Re: [Qemu-devel] [RFC] acpi: add reset register to fadt

[Igor Mammedov]

On Sat, 28 Mar 2015 15:46:53 +0100
Reza Jelveh <address@hidden> wrote:

> Some operating systems such as FreeBSD and Mac OSX need the reset_register
> section of the FADT filled to know which port to write to for a system reset.
> 
> What is the right way to set the reset_val and the reset addr in this case?
> ---
>  hw/i386/acpi-build.c | 5 +++++
>  hw/i386/acpi-defs.h  | 2 ++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index d0a5c85..21c1453 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -361,6 +361,11 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, 
> AcpiPmInfo *pm)
>                                (1 << ACPI_FADT_F_SLP_BUTTON) |
>                                (1 << ACPI_FADT_F_RTC_S4));
>      fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_USE_PLATFORM_CLOCK);
> +    fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_RESET_REG_SUP);
> +    fadt->reset_val = 0xf;
> +    fadt->reset_reg.address_space_id   = aml_system_io;
> +    fadt->reset_reg.register_bit_width = 8;
> +    fadt->reset_reg.address            = ICH9_RST_CNT_IOPORT;
>      /* APIC destination mode ("Flat Logical") has an upper limit of 8 CPUs
>       * For more than 8 CPUs, "Clustered Logical" mode has to be used
>       */
> diff --git a/hw/i386/acpi-defs.h b/hw/i386/acpi-defs.h
> index c4468f8..960c833 100644
> --- a/hw/i386/acpi-defs.h
> +++ b/hw/i386/acpi-defs.h
> @@ -132,6 +132,8 @@ struct AcpiFadtDescriptorRev1
>      uint8_t  reserved4a;             /* Reserved */
>      uint8_t  reserved4b;             /* Reserved */
>      uint32_t flags;
> +    Acpi20GenericAddress reset_reg;
> +    uint8_t reset_val;
you are extending structure beyond of what specified by ACPI 1.0b spec,
that might break guests.
We probably can't change revision since Windows ACPI implementation
is mostly 1.0b based so we are stuck with it.
Patch needs to be tested with Windows guests starting with XP.

related issue,
adding fields without changing major version to a corresponding
one doesn't look correct.

>  } QEMU_PACKED;
>  typedef struct AcpiFadtDescriptorRev1 AcpiFadtDescriptorRev1;
>