Anthony PERARD
2011-Jul-26 11:57 UTC
[Xen-devel] [PATCH] hvmloader: Fix FADT table with QEMU Upstream.
When booting a Windows guest, the OS report an issue with the ACPI (in a
BSOD). The exact issue is "SCI_EN never becomes set in PM1 Control
Register." (quoted from WinDbg help).
To fix this, this patch set some value related to the QEMU upstream: The
SMI command port, and the acpi_enable/acpi_disable values.
Reported-by: Tobias Geiger <tobias.geiger@vido.info>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/firmware/hvmloader/seabios.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/tools/firmware/hvmloader/seabios.c
b/tools/firmware/hvmloader/seabios.c
index 8cd0108..1b89ba7 100644
--- a/tools/firmware/hvmloader/seabios.c
+++ b/tools/firmware/hvmloader/seabios.c
@@ -28,6 +28,9 @@
#include "smbios_types.h"
#include "acpi/acpi2_0.h"
+/* Upstream QEMU require to set more field in the FADT */
+extern struct acpi_20_fadt Fadt;
+
#define ROM_INCLUDE_SEABIOS
#include "roms.inc"
@@ -91,6 +94,11 @@ static void add_table(uint32_t t)
static void seabios_acpi_build_tables(void)
{
uint32_t rsdp = (uint32_t)scratch_alloc(sizeof(struct acpi_20_rsdp), 0);
+
+ Fadt.smi_cmd = 0xb2;
+ Fadt.acpi_enable = 0xf1;
+ Fadt.acpi_disable = 0xf0;
+
acpi_build_tables(rsdp);
add_table(rsdp);
}
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jul-26 12:25 UTC
[Xen-devel] Re: [PATCH] hvmloader: Fix FADT table with QEMU Upstream.
On Tue, 2011-07-26 at 07:57 -0400, Anthony PERARD wrote:> When booting a Windows guest, the OS report an issue with the ACPI (in a > BSOD). The exact issue is "SCI_EN never becomes set in PM1 Control > Register." (quoted from WinDbg help). > > To fix this, this patch set some value related to the QEMU upstream: The > SMI command port, and the acpi_enable/acpi_disable values.Would it be harmful to include these new values in the Fadt for every BIOS rather than adding special cases? If this really needs to be Sea BIOS specific then I think it would be better done as a function call (e.g. acpi_enable_whatever()) into the ACPI code (where the rest of the Fadt init lives) from the seabios hooks rather than splitting the initialisation between two locations (or maybe better this should be the default and the ROMBIOS hooks should disable it if necessary). Ian.> Reported-by: Tobias Geiger <tobias.geiger@vido.info> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > tools/firmware/hvmloader/seabios.c | 8 ++++++++ > 1 files changed, 8 insertions(+), 0 deletions(-) > > diff --git a/tools/firmware/hvmloader/seabios.c b/tools/firmware/hvmloader/seabios.c > index 8cd0108..1b89ba7 100644 > --- a/tools/firmware/hvmloader/seabios.c > +++ b/tools/firmware/hvmloader/seabios.c > @@ -28,6 +28,9 @@ > #include "smbios_types.h" > #include "acpi/acpi2_0.h" > > +/* Upstream QEMU require to set more field in the FADT */ > +extern struct acpi_20_fadt Fadt; > + > #define ROM_INCLUDE_SEABIOS > #include "roms.inc" > > @@ -91,6 +94,11 @@ static void add_table(uint32_t t) > static void seabios_acpi_build_tables(void) > { > uint32_t rsdp = (uint32_t)scratch_alloc(sizeof(struct acpi_20_rsdp), 0); > + > + Fadt.smi_cmd = 0xb2; > + Fadt.acpi_enable = 0xf1; > + Fadt.acpi_disable = 0xf0; > + > acpi_build_tables(rsdp); > add_table(rsdp); > }_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2011-Jul-26 12:41 UTC
Re: [Xen-devel] Re: [PATCH] hvmloader: Fix FADT table with QEMU Upstream.
On 26/07/2011 13:25, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:> On Tue, 2011-07-26 at 07:57 -0400, Anthony PERARD wrote: >> When booting a Windows guest, the OS report an issue with the ACPI (in a >> BSOD). The exact issue is "SCI_EN never becomes set in PM1 Control >> Register." (quoted from WinDbg help). >> >> To fix this, this patch set some value related to the QEMU upstream: The >> SMI command port, and the acpi_enable/acpi_disable values. > > Would it be harmful to include these new values in the Fadt for every > BIOS rather than adding special cases? > > If this really needs to be Sea BIOS specific then I think it would be > better done as a function call (e.g. acpi_enable_whatever()) into the > ACPI code (where the rest of the Fadt init lives) from the seabios hooks > rather than splitting the initialisation between two locations (or maybe > better this should be the default and the ROMBIOS hooks should disable > it if necessary).Descriptive macros and/or comments that describe the magic numbers being poked into the FADT would also be helpful. -- Keir> Ian. > >> Reported-by: Tobias Geiger <tobias.geiger@vido.info> >> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> >> --- >> tools/firmware/hvmloader/seabios.c | 8 ++++++++ >> 1 files changed, 8 insertions(+), 0 deletions(-) >> >> diff --git a/tools/firmware/hvmloader/seabios.c >> b/tools/firmware/hvmloader/seabios.c >> index 8cd0108..1b89ba7 100644 >> --- a/tools/firmware/hvmloader/seabios.c >> +++ b/tools/firmware/hvmloader/seabios.c >> @@ -28,6 +28,9 @@ >> #include "smbios_types.h" >> #include "acpi/acpi2_0.h" >> >> +/* Upstream QEMU require to set more field in the FADT */ >> +extern struct acpi_20_fadt Fadt; >> + >> #define ROM_INCLUDE_SEABIOS >> #include "roms.inc" >> >> @@ -91,6 +94,11 @@ static void add_table(uint32_t t) >> static void seabios_acpi_build_tables(void) >> { >> uint32_t rsdp = (uint32_t)scratch_alloc(sizeof(struct acpi_20_rsdp), 0); >> + >> + Fadt.smi_cmd = 0xb2; >> + Fadt.acpi_enable = 0xf1; >> + Fadt.acpi_disable = 0xf0; >> + >> acpi_build_tables(rsdp); >> add_table(rsdp); >> } > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel