Re: [Qemu-devel] [PATCH v9 08/24] hw/arm/virt-acpi-build: Generate FADT table and update ACPI headers

[Shannon Zhao <zhaoshenglong@huawei.com>]

On 2015/5/27 16:25, Alex Bennée wrote:
> 
> Shannon Zhao <zhaoshenglong@huawei.com> writes:
> 
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> In the case of mach virt, it is used to set the Hardware Reduced bit
>> and enable PSCI SMP booting through HVC. So ignore FACS and FADT
>> points to DSDT.
>>
>> 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 <zhaoshenglong@huawei.com>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>>  hw/arm/virt-acpi-build.c    |  31 ++++++++++
>>  include/hw/acpi/acpi-defs.h | 135 ++++++++++++++++++++++++++++++++------------
>>  2 files changed, 129 insertions(+), 37 deletions(-)
>>
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 2cf2cc5..0791501 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -137,6 +137,31 @@ static void acpi_dsdt_add_virtio(Aml *scope,
>>      }
>>  }
>>  
>> +/* FADT */
>> +static void
>> +build_fadt(GArray *table_data, GArray *linker, unsigned dsdt)
>> +{
>> +    AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
>> +
>> +    /* Hardware Reduced = 1 and use PSCI 0.2+ and with HVC */
>> +    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));
> 
> So the ACPI_FADT_* symbols are actually bit positions. Perhaps naming
> them *_SHIFT will make that clearer else where.
> 
>> +
>> +    /* ACPI v5.1 (fadt->revision.fadt->minor_revision) */
>> +    fadt->minor_revision = 0x1;
>> +
>> +    fadt->dsdt = cpu_to_le32(dsdt);
>> +    /* DSDT address to be filled by Guest linker */
>> +    bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
>> +                                   ACPI_BUILD_TABLE_FILE,
>> +                                   table_data, &fadt->dsdt,
>> +                                   sizeof fadt->dsdt);
>> +
>> +    build_header(linker, table_data,
>> +                 (void *)fadt, "FACP", sizeof(*fadt), 5);
>> +}
>> +
>>  /* DSDT */
>>  static void
>>  build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
>> @@ -183,6 +208,7 @@ static
>>  void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
>>  {
>>      GArray *table_offsets;
>> +    unsigned dsdt;
>>      GArray *tables_blob = tables->table_data;
>>  
>>      table_offsets = g_array_new(false, true /* clear */,
>> @@ -202,8 +228,13 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
>>       */
>>  
>>      /* DSDT is pointed to by FADT */
>> +    dsdt = tables_blob->len;
>>      build_dsdt(tables_blob, tables->linker, guest_info);
>>  
>> +    /* FADT MADT GTDT pointed to by RSDT */
>> +    acpi_add_table(table_offsets, tables_blob);
>> +    build_fadt(tables_blob, tables->linker, dsdt);
>> +
>>      /* Cleanup memory that's no longer used. */
>>      g_array_free(table_offsets, true);
>>  }
>> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
>> index c4468f8..fadcf84 100644
>> --- a/include/hw/acpi/acpi-defs.h
>> +++ b/include/hw/acpi/acpi-defs.h
>> @@ -88,46 +88,54 @@ struct AcpiTableHeader         /* ACPI common table header */
>>  typedef struct AcpiTableHeader AcpiTableHeader;
>>  
>>  /*
>> - * ACPI 1.0 Fixed ACPI Description Table (FADT)
>> + * ACPI Fixed ACPI Description Table (FADT)
>>   */
>> +#define ACPI_FADT_COMMON_DEF /* FADT common definition */ \
>> +    ACPI_TABLE_HEADER_DEF    /* ACPI common table header */ \
>> +    uint32_t firmware_ctrl;  /* Physical address of FACS */ \
>> +    uint32_t dsdt;         /* Physical address of DSDT */ \
>> +    uint8_t  model;        /* System Interrupt Model */ \
>> +    uint8_t  reserved1;    /* Reserved */ \
>> +    uint16_t sci_int;      /* System vector of SCI interrupt */ \
>> +    uint32_t smi_cmd;      /* Port address of SMI command port */ \
>> +    uint8_t  acpi_enable;  /* Value to write to smi_cmd to enable ACPI */ \
>> +    uint8_t  acpi_disable; /* Value to write to smi_cmd to disable ACPI */ \
>> +    /* Value to write to SMI CMD to enter S4BIOS state */ \
>> +    uint8_t  S4bios_req; \
>> +    uint8_t  reserved2;    /* Reserved - must be zero */ \
>> +    /* Port address of Power Mgt 1a acpi_event Reg Blk */ \
>> +    uint32_t pm1a_evt_blk; \
>> +    /* Port address of Power Mgt 1b acpi_event Reg Blk */ \
>> +    uint32_t pm1b_evt_blk; \
>> +    uint32_t pm1a_cnt_blk; /* Port address of Power Mgt 1a Control Reg Blk */ \
>> +    uint32_t pm1b_cnt_blk; /* Port address of Power Mgt 1b Control Reg Blk */ \
>> +    uint32_t pm2_cnt_blk;  /* Port address of Power Mgt 2 Control Reg Blk */ \
>> +    uint32_t pm_tmr_blk;   /* Port address of Power Mgt Timer Ctrl Reg Blk */ \
>> +    /* Port addr of General Purpose acpi_event 0 Reg Blk */ \
>> +    uint32_t gpe0_blk; \
>> +    /* Port addr of General Purpose acpi_event 1 Reg Blk */ \
>> +    uint32_t gpe1_blk; \
>> +    uint8_t  pm1_evt_len;  /* Byte length of ports at pm1_x_evt_blk */ \
>> +    uint8_t  pm1_cnt_len;  /* Byte length of ports at pm1_x_cnt_blk */ \
>> +    uint8_t  pm2_cnt_len;  /* Byte Length of ports at pm2_cnt_blk */ \
>> +    uint8_t  pm_tmr_len;   /* Byte Length of ports at pm_tm_blk */ \
>> +    uint8_t  gpe0_blk_len; /* Byte Length of ports at gpe0_blk */ \
>> +    uint8_t  gpe1_blk_len; /* Byte Length of ports at gpe1_blk */ \
>> +    uint8_t  gpe1_base;    /* Offset in gpe model where gpe1 events start */ \
>> +    uint8_t  reserved3;    /* Reserved */ \
>> +    uint16_t plvl2_lat;    /* Worst case HW latency to enter/exit C2 state */ \
>> +    uint16_t plvl3_lat;    /* Worst case HW latency to enter/exit C3 state */ \
>> +    uint16_t flush_size;   /* Size of area read to flush caches */ \
>> +    uint16_t flush_stride; /* Stride used in flushing caches */ \
>> +    uint8_t  duty_offset;  /* Bit location of duty cycle field in p_cnt reg */ \
>> +    uint8_t  duty_width;   /* Bit width of duty cycle field in p_cnt reg */ \
>> +    uint8_t  day_alrm;     /* Index to day-of-month alarm in RTC CMOS RAM */ \
>> +    uint8_t  mon_alrm;     /* Index to month-of-year alarm in RTC CMOS RAM */ \
>> +    uint8_t  century;      /* Index to century in RTC CMOS RAM */
>> +
>>  struct AcpiFadtDescriptorRev1
>>  {
>> -    ACPI_TABLE_HEADER_DEF     /* ACPI common table header */
>> -    uint32_t firmware_ctrl;          /* Physical address of FACS */
>> -    uint32_t dsdt;                   /* Physical address of DSDT */
>> -    uint8_t  model;                  /* System Interrupt Model */
>> -    uint8_t  reserved1;              /* Reserved */
>> -    uint16_t sci_int;                /* System vector of SCI interrupt */
>> -    uint32_t smi_cmd;                /* Port address of SMI command port */
>> -    uint8_t  acpi_enable;            /* Value to write to smi_cmd to enable ACPI */
>> -    uint8_t  acpi_disable;           /* Value to write to smi_cmd to disable ACPI */
>> -    uint8_t  S4bios_req;             /* Value to write to SMI CMD to enter S4BIOS state */
>> -    uint8_t  reserved2;              /* Reserved - must be zero */
>> -    uint32_t pm1a_evt_blk;           /* Port address of Power Mgt 1a acpi_event Reg Blk */
>> -    uint32_t pm1b_evt_blk;           /* Port address of Power Mgt 1b acpi_event Reg Blk */
>> -    uint32_t pm1a_cnt_blk;           /* Port address of Power Mgt 1a Control Reg Blk */
>> -    uint32_t pm1b_cnt_blk;           /* Port address of Power Mgt 1b Control Reg Blk */
>> -    uint32_t pm2_cnt_blk;            /* Port address of Power Mgt 2 Control Reg Blk */
>> -    uint32_t pm_tmr_blk;             /* Port address of Power Mgt Timer Ctrl Reg Blk */
>> -    uint32_t gpe0_blk;               /* Port addr of General Purpose acpi_event 0 Reg Blk */
>> -    uint32_t gpe1_blk;               /* Port addr of General Purpose acpi_event 1 Reg Blk */
>> -    uint8_t  pm1_evt_len;            /* Byte length of ports at pm1_x_evt_blk */
>> -    uint8_t  pm1_cnt_len;            /* Byte length of ports at pm1_x_cnt_blk */
>> -    uint8_t  pm2_cnt_len;            /* Byte Length of ports at pm2_cnt_blk */
>> -    uint8_t  pm_tmr_len;             /* Byte Length of ports at pm_tm_blk */
>> -    uint8_t  gpe0_blk_len;           /* Byte Length of ports at gpe0_blk */
>> -    uint8_t  gpe1_blk_len;           /* Byte Length of ports at gpe1_blk */
>> -    uint8_t  gpe1_base;              /* Offset in gpe model where gpe1 events start */
>> -    uint8_t  reserved3;              /* Reserved */
>> -    uint16_t plvl2_lat;              /* Worst case HW latency to enter/exit C2 state */
>> -    uint16_t plvl3_lat;              /* Worst case HW latency to enter/exit C3 state */
>> -    uint16_t flush_size;             /* Size of area read to flush caches */
>> -    uint16_t flush_stride;           /* Stride used in flushing caches */
>> -    uint8_t  duty_offset;            /* Bit location of duty cycle field in p_cnt reg */
>> -    uint8_t  duty_width;             /* Bit width of duty cycle field in p_cnt reg */
>> -    uint8_t  day_alrm;               /* Index to day-of-month alarm in RTC CMOS RAM */
>> -    uint8_t  mon_alrm;               /* Index to month-of-year alarm in RTC CMOS RAM */
>> -    uint8_t  century;                /* Index to century in RTC CMOS RAM */
>> +    ACPI_FADT_COMMON_DEF
>>      uint8_t  reserved4;              /* Reserved */
>>      uint8_t  reserved4a;             /* Reserved */
>>      uint8_t  reserved4b;             /* Reserved */
>> @@ -135,6 +143,59 @@ struct AcpiFadtDescriptorRev1
>>  } QEMU_PACKED;
>>  typedef struct AcpiFadtDescriptorRev1 AcpiFadtDescriptorRev1;
>>  
>> +struct AcpiGenericAddress {
>> +    uint8_t space_id;        /* Address space where struct or register exists */
>> +    uint8_t bit_width;       /* Size in bits of given register */
>> +    uint8_t bit_offset;      /* Bit offset within the register */
>> +    uint8_t access_width;    /* Minimum Access size (ACPI 3.0) */
>> +    uint64_t address;        /* 64-bit address of struct or register */
>> +} QEMU_PACKED;
>> +
>> +struct AcpiFadtDescriptorRev5_1 {
>> +    ACPI_FADT_COMMON_DEF
>> +    /* IA-PC Boot Architecture Flags (see below for individual flags) */
>> +    uint16_t boot_flags;
>> +    uint8_t reserved;    /* Reserved, must be zero */
>> +    /* Miscellaneous flag bits (see below for individual flags) */
>> +    uint32_t flags;
>> +    /* 64-bit address of the Reset register */
>> +    struct AcpiGenericAddress reset_register;
>> +    /* Value to write to the reset_register port to reset the system */
>> +    uint8_t reset_value;
>> +    /* ARM-Specific Boot Flags (see below for individual flags) (ACPI 5.1) */
>> +    uint16_t arm_boot_flags;
>> +    uint8_t minor_revision;  /* FADT Minor Revision (ACPI 5.1) */
>> +    uint64_t Xfacs;          /* 64-bit physical address of FACS */
>> +    uint64_t Xdsdt;          /* 64-bit physical address of DSDT */
>> +    /* 64-bit Extended Power Mgt 1a Event Reg Blk address */
>> +    struct AcpiGenericAddress xpm1a_event_block;
>> +    /* 64-bit Extended Power Mgt 1b Event Reg Blk address */
>> +    struct AcpiGenericAddress xpm1b_event_block;
>> +    /* 64-bit Extended Power Mgt 1a Control Reg Blk address */
>> +    struct AcpiGenericAddress xpm1a_control_block;
>> +    /* 64-bit Extended Power Mgt 1b Control Reg Blk address */
>> +    struct AcpiGenericAddress xpm1b_control_block;
>> +    /* 64-bit Extended Power Mgt 2 Control Reg Blk address */
>> +    struct AcpiGenericAddress xpm2_control_block;
>> +    /* 64-bit Extended Power Mgt Timer Ctrl Reg Blk address */
>> +    struct AcpiGenericAddress xpm_timer_block;
>> +    /* 64-bit Extended General Purpose Event 0 Reg Blk address */
>> +    struct AcpiGenericAddress xgpe0_block;
>> +    /* 64-bit Extended General Purpose Event 1 Reg Blk address */
>> +    struct AcpiGenericAddress xgpe1_block;
>> +    /* 64-bit Sleep Control register (ACPI 5.0) */
>> +    struct AcpiGenericAddress sleep_control;
>> +    /* 64-bit Sleep Status register (ACPI 5.0) */
>> +    struct AcpiGenericAddress sleep_status;
>> +} QEMU_PACKED;
>> +
>> +typedef struct AcpiFadtDescriptorRev5_1 AcpiFadtDescriptorRev5_1;
>> +
>> +enum {
>> +    ACPI_FADT_ARM_USE_PSCI_G_0_2 = 0,
>> +    ACPI_FADT_ARM_PSCI_USE_HVC = 1,
>> +};
> 
> Where do these names come from? Are they really ARM specific?
> 

>From ACPI 5.1 spec:

Table 5-37 Fixed ACPI Description Table ARM Boot Architecture Flags

> I'm not sure what the benefit of the enum is here over a #define,
> especially if the values mean something to the outside world. If it was
> being passed around and we were doing type checking it would make more
> sense.
> 
>> +
>>  /*
>>   * ACPI 1.0 Root System Description Table (RSDT)
>>   */
> 

-- 
Shannon