[Qemu-devel] [PATCH v2] pc: acpi: fix pvpanic for buggy guests

[Qemu-devel] [PATCH v2] pc: acpi: fix pvpanic for buggy guests

[Radim Krčmář]

In the old times, we always had pvpanic in ACPI and a _STA method told
the guest not to use it.  Automatic generation dropped the _STA method
as the specification says that missing _STA means enabled and working.
Some guests (Linux) had buggy drivers and this change made them unable
to utilize pvpanic.

A Linux patch is posted as well, but I think it's worth to make pvpanic
useable on old guests at the price of three lines and few bytes of SSDT.

The old _STA method was
  Method (_STA, 0, NotSerialized) {
      Store (PEST, Local0)
      If (LEqual (Local0, Zero)) {
          Return (Zero) }
      Else {
          Return (0x0F) }}

Igor pointed out that we don't need to use a method to return a constant
and that 0xB (don't show in UI) is the common definition now.

Also, the device used to be PEVT.  (PEVT as in "panic event"?)

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 v2: Use name_decl instead of method and return 0xb [Igor]
     Improve subject an commit message.
 
 Btw. I dislike the comment, so I've added a personal TODO to write 0xB
 in a programmer-friendly way.

 hw/i386/acpi-build.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 73259e729b9f..50f4951ec79e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -733,7 +733,7 @@ build_ssdt(GArray *table_data, GArray *linker,
     if (misc->pvpanic_port) {
         scope = aml_scope("\\_SB.PCI0.ISA");
 
-        dev = aml_device("PEVR");
+        dev = aml_device("PEVT");
         aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0001")));
 
         crs = aml_resource_template();
@@ -748,6 +748,9 @@ build_ssdt(GArray *table_data, GArray *linker,
         aml_append(field, aml_named_field("PEPT", 8));
         aml_append(dev, field);
 
+        /* device present, functioning, decoding, not shown in UI */
+        aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
+
         method = aml_method("RDPT", 0);
         aml_append(method, aml_store(aml_name("PEPT"), aml_local(0)));
         aml_append(method, aml_return(aml_local(0)));
-- 
2.4.2

Re: [Qemu-devel] [PATCH v2] pc: acpi: fix pvpanic for buggy guests

[Igor Mammedov]

On Fri, 29 May 2015 21:57:32 +0200
Radim Krčmář <rkrcmar@redhat.com> wrote:

> In the old times, we always had pvpanic in ACPI and a _STA method told
> the guest not to use it.  Automatic generation dropped the _STA method
> as the specification says that missing _STA means enabled and working.
> Some guests (Linux) had buggy drivers and this change made them unable
> to utilize pvpanic.
> 
> A Linux patch is posted as well, but I think it's worth to make pvpanic
> useable on old guests at the price of three lines and few bytes of SSDT.
> 
> The old _STA method was
>   Method (_STA, 0, NotSerialized) {
>       Store (PEST, Local0)
>       If (LEqual (Local0, Zero)) {
>           Return (Zero) }
>       Else {
>           Return (0x0F) }}
> 
> Igor pointed out that we don't need to use a method to return a constant
> and that 0xB (don't show in UI) is the common definition now.
> 
> Also, the device used to be PEVT.  (PEVT as in "panic event"?)
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  v2: Use name_decl instead of method and return 0xb [Igor]
>      Improve subject an commit message.
>  
>  Btw. I dislike the comment, so I've added a personal TODO to write 0xB
>  in a programmer-friendly way.
> 
>  hw/i386/acpi-build.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 73259e729b9f..50f4951ec79e 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -733,7 +733,7 @@ build_ssdt(GArray *table_data, GArray *linker,
>      if (misc->pvpanic_port) {
>          scope = aml_scope("\\_SB.PCI0.ISA");
>  
> -        dev = aml_device("PEVR");
> +        dev = aml_device("PEVT");
>          aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0001")));
>  
>          crs = aml_resource_template();
> @@ -748,6 +748,9 @@ build_ssdt(GArray *table_data, GArray *linker,
>          aml_append(field, aml_named_field("PEPT", 8));
>          aml_append(dev, field);
>  
> +        /* device present, functioning, decoding, not shown in UI */
> +        aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
> +
>          method = aml_method("RDPT", 0);
>          aml_append(method, aml_store(aml_name("PEPT"), aml_local(0)));
>          aml_append(method, aml_return(aml_local(0)));