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

[Shannon Zhao]

On 2015/4/9 2:53, Michael S. Tsirkin wrote:
> On Fri, Apr 03, 2015 at 06:03:39PM +0800, Shannon Zhao wrote:
>> @@ -135,6 +138,43 @@ struct AcpiFadtDescriptorRev1
>>  } QEMU_PACKED;
>>  typedef struct AcpiFadtDescriptorRev1 AcpiFadtDescriptorRev1;
>>  
>> +struct acpi_generic_address {
>> +    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;
> 
> Pls use standard QEMU style for structs.
> There are more like this in the patchset, pls find and fix them.
> 

Ok, thanks.

> 
>> +
>> +struct AcpiFadtDescriptorRev5_1 {
>> +    ACPI_FADT_COMMON_DEF
>> +    uint16_t boot_flags;     /* IA-PC Boot Architecture Flags (see below 
>> for individual flags) */
>> +    uint8_t reserved;        /* Reserved, must be zero */
>> +    uint32_t flags;      /* Miscellaneous flag bits (see below for 
>> individual flags) */
>> +    struct acpi_generic_address reset_register; /* 64-bit address of the 
>> Reset register */
>> +    uint8_t reset_value;     /* Value to write to the reset_register port 
>> to reset the system */
>> +    uint16_t arm_boot_flags; /* ARM-Specific Boot Flags (see below for 
>> individual flags) (ACPI 5.1) */
>> +    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 */
>> +    struct acpi_generic_address xpm1a_event_block;  /* 64-bit Extended 
>> Power Mgt 1a Event Reg Blk address */
>> +    struct acpi_generic_address xpm1b_event_block;  /* 64-bit Extended 
>> Power Mgt 1b Event Reg Blk address */
>> +    struct acpi_generic_address xpm1a_control_block;    /* 64-bit Extended 
>> Power Mgt 1a Control Reg Blk address */
>> +    struct acpi_generic_address xpm1b_control_block;    /* 64-bit Extended 
>> Power Mgt 1b Control Reg Blk address */
>> +    struct acpi_generic_address xpm2_control_block; /* 64-bit Extended 
>> Power Mgt 2 Control Reg Blk address */
>> +    struct acpi_generic_address xpm_timer_block;    /* 64-bit Extended 
>> Power Mgt Timer Ctrl Reg Blk address */
>> +    struct acpi_generic_address xgpe0_block;    /* 64-bit Extended General 
>> Purpose Event 0 Reg Blk address */
>> +    struct acpi_generic_address xgpe1_block;    /* 64-bit Extended General 
>> Purpose Event 1 Reg Blk address */
>> +    struct acpi_generic_address sleep_control;  /* 64-bit Sleep Control 
>> register (ACPI 5.0) */
>> +    struct acpi_generic_address sleep_status;   /* 64-bit Sleep Status 
>> register (ACPI 5.0) */
>> +} QEMU_PACKED;
> 
> empty line missing.
> 

ok.

>> +typedef struct AcpiFadtDescriptorRev5_1 AcpiFadtDescriptorRev5_1;
>> +
>> +enum {
>> +    ACPI_FADT_ARM_USE_PSCI_G_0_2,
>> +    ACPI_FADT_ARM_PSCI_USE_HVC,
>> +};
> 
> These are part of tables, are they not?

They are the values of arm_boot_flags in AcpiFadtDescriptorRev5_1.

> Pls add = 0, = 1, so we don't change them by mistake.

Ok, thanks.

> 
>> +
>>  /*
>>   * ACPI 1.0 Root System Description Table (RSDT)
>>   */
>> -- 
>> 2.0.4
>>
> 
> .
>