xen_preinit() runs early and changes DebugOutputPort. qemu_preinit() runs soon after. inb on DebugOutputPort doesn't work on Xen, so the check will always fail and DebugOutputPort will be cleared to 0 disabling output.
Quick exit the function when running on Xen to preserve the modified DebugOutputPort.
Signed-off-by: Jason Andryuk jandryuk@gmail.com --- src/hw/serialio.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/hw/serialio.c b/src/hw/serialio.c index 3163344..c6297bc 100644 --- a/src/hw/serialio.c +++ b/src/hw/serialio.c @@ -106,6 +106,10 @@ u16 DebugOutputPort VARFSEG = 0x402; void qemu_debug_preinit(void) { + /* Xen already overrode DebugOutputPort in xen_preinit(). */ + if (runningOnXen()) + return; + /* Check if the QEMU debug output port is active */ if (CONFIG_DEBUG_IO && inb(GET_GLOBAL(DebugOutputPort)) != QEMU_DEBUGCON_READBACK)
On Tue, Jun 16, 2020 at 10:31:17PM -0400, Jason Andryuk wrote:
xen_preinit() runs early and changes DebugOutputPort. qemu_preinit() runs soon after. inb on DebugOutputPort doesn't work on Xen, so the check will always fail and DebugOutputPort will be cleared to 0 disabling output.
Quick exit the function when running on Xen to preserve the modified DebugOutputPort.
Signed-off-by: Jason Andryuk jandryuk@gmail.com
src/hw/serialio.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/hw/serialio.c b/src/hw/serialio.c index 3163344..c6297bc 100644 --- a/src/hw/serialio.c +++ b/src/hw/serialio.c @@ -106,6 +106,10 @@ u16 DebugOutputPort VARFSEG = 0x402; void qemu_debug_preinit(void) {
- /* Xen already overrode DebugOutputPort in xen_preinit(). */
- if (runningOnXen())
return;
Thanks. The code looks fine to me. However, I find the comment confusing as this function only checks if output is enabled - it doesn't check the validity of the port itself. Perhaps a comment like "Xen doesn't support checking if debug output is active".
-Kevin
- /* Check if the QEMU debug output port is active */ if (CONFIG_DEBUG_IO && inb(GET_GLOBAL(DebugOutputPort)) != QEMU_DEBUGCON_READBACK)
-- 2.25.1 _______________________________________________ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-leave@seabios.org
On Mon, Jun 22, 2020 at 8:40 PM Kevin O'Connor kevin@koconnor.net wrote:
On Tue, Jun 16, 2020 at 10:31:17PM -0400, Jason Andryuk wrote:
xen_preinit() runs early and changes DebugOutputPort. qemu_preinit() runs soon after. inb on DebugOutputPort doesn't work on Xen, so the check will always fail and DebugOutputPort will be cleared to 0 disabling output.
Quick exit the function when running on Xen to preserve the modified DebugOutputPort.
Signed-off-by: Jason Andryuk jandryuk@gmail.com
src/hw/serialio.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/hw/serialio.c b/src/hw/serialio.c index 3163344..c6297bc 100644 --- a/src/hw/serialio.c +++ b/src/hw/serialio.c @@ -106,6 +106,10 @@ u16 DebugOutputPort VARFSEG = 0x402; void qemu_debug_preinit(void) {
- /* Xen already overrode DebugOutputPort in xen_preinit(). */
- if (runningOnXen())
return;Thanks. The code looks fine to me. However, I find the comment confusing as this function only checks if output is enabled - it doesn't check the validity of the port itself. Perhaps a comment like "Xen doesn't support checking if debug output is active".
Sure, I can change it. The other part I was trying to convey is that even though Xen uses QEMU, and this function is prefixed with qemu_, DebugOutputPort is different and cannot be checked. Since DebugOutputPort is set right above this function, a reader may not expect it to have changed. Having said that, your wording clearly explains why Xen is leaving this function early.
Thanks, Jason
-
Jason Andryuk -
Kevin O'Connor