[SeaBIOS] [SeaBIOS PATCH] xen: require Xen info structure at 0x1000 to detect Xen

David Woodhouse posted 1 patch 1 year, 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/seabios tags/patchew/feef99dd2e1a5dce004d22baf07d716d6ea1344c.camel@infradead.org
There is a newer version of this series
src/fw/xen.c | 45 ++++++++++++++++++++++++++++++++-------------
1 file changed, 32 insertions(+), 13 deletions(-)
[SeaBIOS] [SeaBIOS PATCH] xen: require Xen info structure at 0x1000 to detect Xen
Posted by David Woodhouse 1 year, 3 months ago
From: David Woodhouse <dwmw@amazon.co.uk>

When running under Xen, hvmloader places a table at 0x1000 with the e820
information and BIOS tables. If this isn't present, SeaBIOS will 
currently panic.

We now have support for running Xen guests natively in QEMU/KVM, which
boots SeaBIOS directly instead of via hvmloader, and does not provide
the same structure.

As it happens, this doesn't matter on first boot. because although we
set PlatformRunningOn to PF_QEMU|PF_XEN, reading it back again still
gives zero. Presumably because in true Xen, this is all already RAM. But
in QEMU with a faithfully-emulated PAM config in the host bridge, it's
still in ROM mode at this point so we don't see what we've just written.

On reboot, however, the region *is* set to RAM mode and we do see the
updated value of PlatformRunningOn, do manage to remember that we've
detected Xen in CPUID, and hit the panic.

It's not trivial to detect QEMU vs. real Xen at the time xen_preinit()
runs, because it's so early. We can't even make a XENVER_extraversion
hypercall to look for hints, because we haven't set up the hypercall
page (and don't have an allocator to give us a page in which to do so).

So just make Xen detection contingent on the info structure being
present. If it wasn't, we were going to panic anyway. That leaves us
taking the standard QEMU init path for Xen guests in native QEMU,
which is just fine.

Untested on actual Xen but ObviouslyCorrect™.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 src/fw/xen.c | 45 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 32 insertions(+), 13 deletions(-)

diff --git a/src/fw/xen.c b/src/fw/xen.c
index a215b9e..00e4b0c 100644
--- a/src/fw/xen.c
+++ b/src/fw/xen.c
@@ -40,16 +40,25 @@ struct xen_seabios_info {
     u32 e820_nr;
 } PACKED;
 
-static void validate_info(struct xen_seabios_info *t)
+static struct xen_seabios_info *validate_info(void)
 {
-    if ( memcmp(t->signature, "XenHVMSeaBIOS", 14) )
-        panic("Bad Xen info signature\n");
+    struct xen_seabios_info *t = (void *)INFO_PHYSICAL_ADDRESS;
 
-    if ( t->length < sizeof(struct xen_seabios_info) )
-        panic("Bad Xen info length\n");
+    if ( memcmp(t->signature, "XenHVMSeaBIOS", 14) ) {
+        dprintf(1, "Bad Xen info signature\n");
+        return NULL;
+    }
+
+    if ( t->length < sizeof(struct xen_seabios_info) ) {
+        dprintf(1, "Bad Xen info length\n");
+        return NULL;
+    }
 
-    if (checksum(t, t->length) != 0)
-        panic("Bad Xen info checksum\n");
+    if (checksum(t, t->length) != 0) {
+        dprintf(1, "Bad Xen info checksum\n");
+        return NULL;
+    }
+    return t;
 }
 
 void xen_preinit(void)
@@ -86,7 +95,10 @@ void xen_preinit(void)
         dprintf(1, "No Xen hypervisor found.\n");
         return;
     }
-    PlatformRunningOn = PF_QEMU|PF_XEN;
+    if (validate_info())
+        PlatformRunningOn = PF_QEMU|PF_XEN;
+    else
+        dprintf(1, "Not enabling Xen support due to lack of Xen info\n");
 }
 
 static int hypercall_xen_version( int cmd, void *arg)
@@ -122,10 +134,14 @@ void xen_hypercall_setup(void)
 
 void xen_biostable_setup(void)
 {
-    struct xen_seabios_info *info = (void *)INFO_PHYSICAL_ADDRESS;
-    void **tables = (void*)info->tables;
+    struct xen_seabios_info *info = validate_info();
+    void **tables;
     int i;
 
+    if (!info)
+        panic("Xen info corrupted\n");
+
+    tables = (void*)info->tables;
     dprintf(1, "xen: copy BIOS tables...\n");
     for (i=0; i<info->tables_nr; i++)
         copy_table(tables[i]);
@@ -136,12 +152,15 @@ void xen_biostable_setup(void)
 void xen_ramsize_preinit(void)
 {
     int i;
-    struct xen_seabios_info *info = (void *)INFO_PHYSICAL_ADDRESS;
-    struct e820entry *e820 = (struct e820entry *)info->e820;
-    validate_info(info);
+    struct xen_seabios_info *info = validate_info();
+    struct e820entry *e820;
+
+    if (!info)
+        panic("Xen info corrupted\n");
 
     dprintf(1, "xen: copy e820...\n");
 
+    e820 = (struct e820entry *)info->e820;
     for (i = 0; i < info->e820_nr; i++) {
         struct e820entry *e = &e820[i];
         e820_add(e->start, e->size, e->type);
-- 
2.34.1


_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [SeaBIOS PATCH] xen: require Xen info structure at 0x1000 to detect Xen
Posted by Kevin O'Connor 1 year, 2 months ago
On Fri, Jan 20, 2023 at 11:33:19AM +0000, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> When running under Xen, hvmloader places a table at 0x1000 with the e820
> information and BIOS tables. If this isn't present, SeaBIOS will 
> currently panic.
> 
> We now have support for running Xen guests natively in QEMU/KVM, which
> boots SeaBIOS directly instead of via hvmloader, and does not provide
> the same structure.
> 
> As it happens, this doesn't matter on first boot. because although we
> set PlatformRunningOn to PF_QEMU|PF_XEN, reading it back again still
> gives zero. Presumably because in true Xen, this is all already RAM. But
> in QEMU with a faithfully-emulated PAM config in the host bridge, it's
> still in ROM mode at this point so we don't see what we've just written.
> 
> On reboot, however, the region *is* set to RAM mode and we do see the
> updated value of PlatformRunningOn, do manage to remember that we've
> detected Xen in CPUID, and hit the panic.
> 
> It's not trivial to detect QEMU vs. real Xen at the time xen_preinit()
> runs, because it's so early. We can't even make a XENVER_extraversion
> hypercall to look for hints, because we haven't set up the hypercall
> page (and don't have an allocator to give us a page in which to do so).
> 
> So just make Xen detection contingent on the info structure being
> present. If it wasn't, we were going to panic anyway. That leaves us
> taking the standard QEMU init path for Xen guests in native QEMU,
> which is just fine.

Thanks.  I committed this change.

-Kevin
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
Re: [SeaBIOS] [SeaBIOS PATCH] xen: require Xen info structure at 0x1000 to detect Xen
Posted by David Woodhouse 1 year, 2 months ago
On Wed, 2023-02-01 at 21:13 -0500, Kevin O'Connor wrote:
> On Fri, Jan 20, 2023 at 11:33:19AM +0000, David Woodhouse wrote:
> > From: David Woodhouse <dwmw@amazon.co.uk>
> > 
> > When running under Xen, hvmloader places a table at 0x1000 with the e820
> > information and BIOS tables. If this isn't present, SeaBIOS will 
> > currently panic.
> > 
> > We now have support for running Xen guests natively in QEMU/KVM, which
> > boots SeaBIOS directly instead of via hvmloader, and does not provide
> > the same structure.
> > 
> > As it happens, this doesn't matter on first boot. because although we
> > set PlatformRunningOn to PF_QEMU|PF_XEN, reading it back again still
> > gives zero. Presumably because in true Xen, this is all already RAM. But
> > in QEMU with a faithfully-emulated PAM config in the host bridge, it's
> > still in ROM mode at this point so we don't see what we've just written.
> > 
> > On reboot, however, the region *is* set to RAM mode and we do see the
> > updated value of PlatformRunningOn, do manage to remember that we've
> > detected Xen in CPUID, and hit the panic.
> > 
> > It's not trivial to detect QEMU vs. real Xen at the time xen_preinit()
> > runs, because it's so early. We can't even make a XENVER_extraversion
> > hypercall to look for hints, because we haven't set up the hypercall
> > page (and don't have an allocator to give us a page in which to do so).
> > 
> > So just make Xen detection contingent on the info structure being
> > present. If it wasn't, we were going to panic anyway. That leaves us
> > taking the standard QEMU init path for Xen guests in native QEMU,
> > which is just fine.
> 
> Thanks.  I committed this change.
> 
> -Kevin

Thanks, Kevin.

I'd like to get the rest of the Xen platform support in to qemu 8.0 if
possible. Which is currently scheduled for March.

Is there likely to be a SeaBIOS release before then which Gerd would
pull into qemu anyway, or should I submit a submodule update to a
snapshot of today's tree? That would just pull in this commit, and the
one other fix that's in the SeaBIOS tree since 1.16.1?


 $ git shortlog rel-1.16.1..
David Woodhouse (1):
      xen: require Xen info structure at 0x1000 to detect Xen

Qi Zhou (1):
      usb: fix wrong init of keyboard/mouse's if first interface is not boot protocol



Re: [SeaBIOS] Re: [SeaBIOS PATCH] xen: require Xen info structure at 0x1000 to detect Xen
Posted by Gerd Hoffmann 1 year, 2 months ago
> Thanks, Kevin.
> 
> I'd like to get the rest of the Xen platform support in to qemu 8.0 if
> possible. Which is currently scheduled for March.
> 
> Is there likely to be a SeaBIOS release before then which Gerd would
> pull into qemu anyway, or should I submit a submodule update to a
> snapshot of today's tree? That would just pull in this commit, and the
> one other fix that's in the SeaBIOS tree since 1.16.1?

Tagging 1.16.2 in time for the qemu 8.0 should not be a problem given
that we have only bugfixes in master.  Roughly around soft freeze is
probably a good time for that.

take care,
  Gerd
[SeaBIOS] Re: [SeaBIOS PATCH] xen: require Xen info structure at 0x1000 to detect Xen
Posted by David Woodhouse 1 year, 1 month ago
On Thu, 2023-02-02 at 10:10 +0100, Gerd Hoffmann wrote:
> > Thanks, Kevin.
> > 
> > I'd like to get the rest of the Xen platform support in to qemu 8.0
> > if
> > possible. Which is currently scheduled for March.
> > 
> > Is there likely to be a SeaBIOS release before then which Gerd
> > would
> > pull into qemu anyway, or should I submit a submodule update to a
> > snapshot of today's tree? That would just pull in this commit, and
> > the
> > one other fix that's in the SeaBIOS tree since 1.16.1?
> 
> Tagging 1.16.2 in time for the qemu 8.0 should not be a problem given
> that we have only bugfixes in master.  Roughly around soft freeze is
> probably a good time for that.

That's, erm, at the *end* of today 2023-03-07, and the freeze happens
only *after* Paul has reviewed the phase 2 Xen PV back end support,
right?

https://wiki.qemu.org/Planning/8.0
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [SeaBIOS PATCH] xen: require Xen info structure at 0x1000 to detect Xen
Posted by Gerd Hoffmann 1 year, 1 month ago
On Tue, Mar 07, 2023 at 08:42:18AM +0000, David Woodhouse wrote:
> On Thu, 2023-02-02 at 10:10 +0100, Gerd Hoffmann wrote:
> > > Thanks, Kevin.
> > > 
> > > I'd like to get the rest of the Xen platform support in to qemu 8.0
> > > if
> > > possible. Which is currently scheduled for March.
> > > 
> > > Is there likely to be a SeaBIOS release before then which Gerd
> > > would
> > > pull into qemu anyway, or should I submit a submodule update to a
> > > snapshot of today's tree? That would just pull in this commit, and
> > > the
> > > one other fix that's in the SeaBIOS tree since 1.16.1?
> > 
> > Tagging 1.16.2 in time for the qemu 8.0 should not be a problem given
> > that we have only bugfixes in master.  Roughly around soft freeze is
> > probably a good time for that.
> 
> That's, erm, at the *end* of today 2023-03-07, and the freeze happens
> only *after* Paul has reviewed the phase 2 Xen PV back end support,
> right?

Ok, we have as of today two changes:

  kraxel@sirius ~/projects/seabios (master)# git log --oneline rel-1.16.1..
  ea1b7a073390 xen: require Xen info structure at 0x1000 to detect Xen
  645a64b4911d usb: fix wrong init of keyboard/mouse's if first interface is not boot protocol

With no changes since January and no known issues.
I think we can safely tag the current state as 1.16.2.

I'll do that next monday (plus qemu pull request) unless
there are objections until then.

take care,
  Gerd

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] seabios 1.16.2 release tagged (was: Re: Re: [SeaBIOS PATCH] xen: require Xen info structure at). 0x1000 to detect Xen
Posted by Gerd Hoffmann 1 year, 1 month ago
  Hi,

> Ok, we have as of today two changes:
> 
>   kraxel@sirius ~/projects/seabios (master)# git log --oneline rel-1.16.1..
>   ea1b7a073390 xen: require Xen info structure at 0x1000 to detect Xen
>   645a64b4911d usb: fix wrong init of keyboard/mouse's if first interface is not boot protocol
> 
> With no changes since January and no known issues.
> I think we can safely tag the current state as 1.16.2.
> 
> I'll do that next monday (plus qemu pull request) unless
> there are objections until then.

Oops, totally forgot it.  Three days after the deadline, still no
objections raised, so I tagged the release today and uploaded the
source tarball.  qemu pull request for the update is out of the
door too.

take care,
  Gerd

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [SeaBIOS PATCH] xen: require Xen info structure at 0x1000 to detect Xen
Posted by Kevin O'Connor 1 year, 2 months ago
On Fri, Jan 20, 2023 at 11:33:19AM +0000, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> When running under Xen, hvmloader places a table at 0x1000 with the e820
> information and BIOS tables. If this isn't present, SeaBIOS will 
> currently panic.
> 
> We now have support for running Xen guests natively in QEMU/KVM, which
> boots SeaBIOS directly instead of via hvmloader, and does not provide
> the same structure.
> 
> As it happens, this doesn't matter on first boot. because although we
> set PlatformRunningOn to PF_QEMU|PF_XEN, reading it back again still
> gives zero. Presumably because in true Xen, this is all already RAM. But
> in QEMU with a faithfully-emulated PAM config in the host bridge, it's
> still in ROM mode at this point so we don't see what we've just written.
> 
> On reboot, however, the region *is* set to RAM mode and we do see the
> updated value of PlatformRunningOn, do manage to remember that we've
> detected Xen in CPUID, and hit the panic.
> 
> It's not trivial to detect QEMU vs. real Xen at the time xen_preinit()
> runs, because it's so early. We can't even make a XENVER_extraversion
> hypercall to look for hints, because we haven't set up the hypercall
> page (and don't have an allocator to give us a page in which to do so).
> 
> So just make Xen detection contingent on the info structure being
> present. If it wasn't, we were going to panic anyway. That leaves us
> taking the standard QEMU init path for Xen guests in native QEMU,
> which is just fine.
> 
> Untested on actual Xen but ObviouslyCorrect™.

Thanks.  I don't have a way to test this, but it looks fine to me.
I'll give a few more days to see if others have comments, and
otherwise look to commit.

Cheers,
-Kevin
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
Re: [SeaBIOS PATCH] xen: require Xen info structure at 0x1000 to detect Xen
Posted by Paul Durrant 1 year, 2 months ago
On 20/01/2023 11:33, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> When running under Xen, hvmloader places a table at 0x1000 with the e820
> information and BIOS tables. If this isn't present, SeaBIOS will
> currently panic.
> 
> We now have support for running Xen guests natively in QEMU/KVM, which
> boots SeaBIOS directly instead of via hvmloader, and does not provide
> the same structure.
> 
> As it happens, this doesn't matter on first boot. because although we
> set PlatformRunningOn to PF_QEMU|PF_XEN, reading it back again still
> gives zero. Presumably because in true Xen, this is all already RAM. But
> in QEMU with a faithfully-emulated PAM config in the host bridge, it's
> still in ROM mode at this point so we don't see what we've just written.
> 
> On reboot, however, the region *is* set to RAM mode and we do see the
> updated value of PlatformRunningOn, do manage to remember that we've
> detected Xen in CPUID, and hit the panic.
> 
> It's not trivial to detect QEMU vs. real Xen at the time xen_preinit()
> runs, because it's so early. We can't even make a XENVER_extraversion
> hypercall to look for hints, because we haven't set up the hypercall
> page (and don't have an allocator to give us a page in which to do so).
> 
> So just make Xen detection contingent on the info structure being
> present. If it wasn't, we were going to panic anyway. That leaves us
> taking the standard QEMU init path for Xen guests in native QEMU,
> which is just fine.
> 
> Untested on actual Xen but ObviouslyCorrect™.

Works for me...

Tested-by: Paul Durrant <paul@xen.org>

> 
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>   src/fw/xen.c | 45 ++++++++++++++++++++++++++++++++-------------
>   1 file changed, 32 insertions(+), 13 deletions(-)
> 

Also...

Reviewed-by: Paul Durrant <paul@xen.org>