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)
-- 
2.25.1
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org

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
_______________________________________________
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
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org