Re: [Qemu-devel] [RFC PATCH 07/11] hw/arm/virt-acpi-build: Generate FADT table and update ACPI headers

[Shannon Zhao]

On 2015/1/25 6:05, Laszlo Ersek wrote:
> comments below
> 
> On 01/24/15 10:21, Shannon Zhao wrote:
>> FADT points to FACS and DSDT, in the case of mach virt, it is also used
>> to set the Hardware Reduced bit and enable PSCI SMP booting through HVC.
>>
>> Update the header definitions for FADT taking into account the new
>> additions of ACPI v5.1 in `include/hw/acpi/acpi-defs.h`
>>
>> Signed-off-by: Shannon Zhao <address@hidden>
>> ---
>>  hw/arm/virt-acpi-build.c    |   26 ++++++++++
>>  include/hw/acpi/acpi-defs.h |  114 
>> +++++++++++++++++++++++++++++--------------
>>  2 files changed, 103 insertions(+), 37 deletions(-)
>>
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 2872dff..e3c708d 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -218,6 +218,32 @@ build_gtdt(GArray *table_data, GArray *linker, 
>> VirtGuestInfo *guest_info)
>>  static void
>>  build_fadt(GArray *table_data, GArray *linker, uint64_t facs, uint64_t dsdt)
>>  {
>> +    AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data, 
>> sizeof(*fadt));
>> +
>> +    /* Hardware Reduced = 1 and use PSCI 0.2+ and with HVC */
> 
> You set "Hardware Reduced" here.
> 
>> +    fadt->flags = cpu_to_le32(1 << ACPI_FADT_F_HW_REDUCED_ACPI);
>> +    fadt->arm_boot_flags = cpu_to_le16((1 << ACPI_FADT_ARM_USE_PSCI_G_0_2) |
>> +                                       (1 << ACPI_FADT_ARM_PSCI_USE_HVC));
>> +
>> +    /* ACPI v5.1 (fadt->revision.fadt->minor_revision) */
>> +    fadt->minor_revision = 0x1;
>> +
>> +    fadt->Xfacs = cpu_to_le64(facs);
> 
> But you also set up a FACS table.
> 
> Hardware Reduced mode makes Linux ignore the FACS table. Please see
> upstream kernel commit
> 
> commit 22e5b40ab21fcac21db0ff25fbb844ffecc73a4a
> Author: Bob Moore <address@hidden>
> Date:   Wed Nov 16 10:57:28 2011 +0800
> 
>     ACPI 5.0: Implement hardware-reduced option
> 
> You can probably drop the generation of the FACS, unless you intend to
> disable HW reduced mode.
> 

Ok, Thanks for pointing this out :-)
I think we should drop the FACS to make it simple.

Thanks,
Shannon