Anthony PERARD
2011-Jul-26 14:23 UTC
[Xen-devel] [PATCH V2] hvmloader: Fix FADT table for 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/acpi/static_tables.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
Change:
- this time, always set, even on old QEMU.
- Add more descriptive macro.
diff --git a/tools/firmware/hvmloader/acpi/static_tables.c
b/tools/firmware/hvmloader/acpi/static_tables.c
index cf4b8dc..b68f6e1 100644
--- a/tools/firmware/hvmloader/acpi/static_tables.c
+++ b/tools/firmware/hvmloader/acpi/static_tables.c
@@ -42,6 +42,10 @@ struct acpi_20_facs Facs = {
#define ACPI_PM_TMR_BLK_BIT_WIDTH 0x20
#define ACPI_PM_TMR_BLK_BIT_OFFSET 0x00
+#define SMI_CMD_IOPORT 0xb2
+#define PIIX4_ACPI_ENABLE 0xf1
+#define PIIX4_ACPI_DISABLE 0xf0
+
struct acpi_20_fadt Fadt = {
.header = {
.signature = ACPI_2_0_FADT_SIGNATURE,
@@ -55,6 +59,10 @@ struct acpi_20_fadt Fadt = {
},
.sci_int = 9,
+ .smi_cmd = SMI_CMD_IOPORT,
+
+ .acpi_enable = PIIX4_ACPI_ENABLE,
+ .acpi_disable = PIIX4_ACPI_DISABLE,
.pm1a_evt_blk = ACPI_PM1A_EVT_BLK_ADDRESS_V1,
.pm1a_cnt_blk = ACPI_PM1A_CNT_BLK_ADDRESS_V1,
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Keir Fraser
2011-Jul-26 17:48 UTC
Re: [Xen-devel] [PATCH V2] hvmloader: Fix FADT table for QEMU Upstream.
Thinking about this some more I wonder whether this will be correct for old
Qemu. Furthermore, we don''t actually have a legacy/SMM component in our
virtual firmware, so advertising the enable/disable mechanism via
SMI_CMD_IOPORT is a bit pointless.
Could we instead dynamically handle this in in hvmloader''s
acpi-handling
code something like:
if ( !SCI_EN ) // must be new qemu if so
outb(SMI_CMD_IOPORT, PIIX4_ACPI_ENABLE)
BUG_ON(!SCI_EN)
Or alternatively, we should at least gate the setting of those FADT fields
on such a check, it seems to me.
-- Keir
On 26/07/2011 15:23, "Anthony PERARD"
<anthony.perard@citrix.com> 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.
>
> Reported-by: Tobias Geiger <tobias.geiger@vido.info>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> tools/firmware/hvmloader/acpi/static_tables.c | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> Change:
> - this time, always set, even on old QEMU.
> - Add more descriptive macro.
>
> diff --git a/tools/firmware/hvmloader/acpi/static_tables.c
> b/tools/firmware/hvmloader/acpi/static_tables.c
> index cf4b8dc..b68f6e1 100644
> --- a/tools/firmware/hvmloader/acpi/static_tables.c
> +++ b/tools/firmware/hvmloader/acpi/static_tables.c
> @@ -42,6 +42,10 @@ struct acpi_20_facs Facs = {
> #define ACPI_PM_TMR_BLK_BIT_WIDTH 0x20
> #define ACPI_PM_TMR_BLK_BIT_OFFSET 0x00
>
> +#define SMI_CMD_IOPORT 0xb2
> +#define PIIX4_ACPI_ENABLE 0xf1
> +#define PIIX4_ACPI_DISABLE 0xf0
> +
> struct acpi_20_fadt Fadt = {
> .header = {
> .signature = ACPI_2_0_FADT_SIGNATURE,
> @@ -55,6 +59,10 @@ struct acpi_20_fadt Fadt = {
> },
>
> .sci_int = 9,
> + .smi_cmd = SMI_CMD_IOPORT,
> +
> + .acpi_enable = PIIX4_ACPI_ENABLE,
> + .acpi_disable = PIIX4_ACPI_DISABLE,
>
> .pm1a_evt_blk = ACPI_PM1A_EVT_BLK_ADDRESS_V1,
> .pm1a_cnt_blk = ACPI_PM1A_CNT_BLK_ADDRESS_V1,
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jul-27 16:00 UTC
Re: [Xen-devel] [PATCH V2] hvmloader: Fix FADT table for QEMU Upstream.
On Tue, Jul 26, 2011 at 18:48, Keir Fraser <keir@xen.org> wrote:> Thinking about this some more I wonder whether this will be correct for old > Qemu. Furthermore, we don''t actually have a legacy/SMM component in our > virtual firmware, so advertising the enable/disable mechanism via > SMI_CMD_IOPORT is a bit pointless. > > Could we instead dynamically handle this in in hvmloader''s acpi-handling > code something like: > if ( !SCI_EN ) // must be new qemu if so > outb(SMI_CMD_IOPORT, PIIX4_ACPI_ENABLE) > BUG_ON(!SCI_EN)I just try that, and that works fine. So it''s fine to me to do that instead of providing an IOPort that is not handle by the qemu-xen-legacy.> Or alternatively, we should at least gate the setting of those FADT fields > on such a check, it seems to me.-- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2011-Jul-27 22:28 UTC
Re: [Xen-devel] [PATCH V2] hvmloader: Fix FADT table for QEMU Upstream.
On 27/07/2011 17:00, "Anthony PERARD" <anthony.perard@citrix.com> wrote:> On Tue, Jul 26, 2011 at 18:48, Keir Fraser <keir@xen.org> wrote: >> Thinking about this some more I wonder whether this will be correct for old >> Qemu. Furthermore, we don''t actually have a legacy/SMM component in our >> virtual firmware, so advertising the enable/disable mechanism via >> SMI_CMD_IOPORT is a bit pointless. >> >> Could we instead dynamically handle this in in hvmloader''s acpi-handling >> code something like: >> if ( !SCI_EN ) // must be new qemu if so >> outb(SMI_CMD_IOPORT, PIIX4_ACPI_ENABLE) >> BUG_ON(!SCI_EN) > > I just try that, and that works fine. So it''s fine to me to do that > instead of providing an IOPort that is not handle by the > qemu-xen-legacy.Send me your tested patch! Thanks, Keir>> Or alternatively, we should at least gate the setting of those FADT fields >> on such a check, it seems to me._______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jul-28 13:35 UTC
[Xen-devel] [PATCH V3] hvmloader: Enable SCI in QEMU if is not.
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).
So this patch enables the flags SCI_EN if it is not yet enabled.
Reported-by: Tobias Geiger <tobias.geiger@vido.info>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/firmware/hvmloader/hvmloader.c | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/tools/firmware/hvmloader/hvmloader.c
b/tools/firmware/hvmloader/hvmloader.c
index f8553bb..0637ac0 100644
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -28,6 +28,7 @@
#include "apic_regs.h"
#include <xen/version.h>
#include <xen/hvm/params.h>
+#include <xen/hvm/ioreq.h>
#define ROM_INCLUDE_VGABIOS
#define ROM_INCLUDE_ETHERBOOT
@@ -381,6 +382,22 @@ static const struct bios_config *detect_bios(void)
return NULL;
}
+#define SCI_EN (1 << 0)
+#define SMI_CMD_IOPORT 0xb2
+#define PIIX4_ACPI_ENABLE 0xf1
+
+static void enable_sci(void)
+{
+ int pm1a_cnt_val = 0;
+
+ pm1a_cnt_val = inb(ACPI_PM1A_CNT_BLK_ADDRESS_V1);
+ if (!(pm1a_cnt_val & SCI_EN)) {
+ outb(SMI_CMD_IOPORT, PIIX4_ACPI_ENABLE);
+ }
+ pm1a_cnt_val = inb(ACPI_PM1A_CNT_BLK_ADDRESS_V1);
+ BUG_ON(!(pm1a_cnt_val & SCI_EN));
+}
+
int main(void)
{
const struct bios_config *bios;
@@ -479,6 +496,7 @@ int main(void)
printf("Loading ACPI ...\n");
bios->acpi_build_tables();
}
+ enable_sci();
hypercall_hvm_op(HVMOP_set_param, &p);
}
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel