| Message ID | feef99dd2e1a5dce004d22baf07d716d6ea1344c.camel@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [SeaBIOS] xen: require Xen info structure at 0x1000 to detect Xen | expand |
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>
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
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
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
> 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
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
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
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
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);