Re: [kvm-unit-tests PATCH v2 04/23] lib: Extend the definition of the ACPI table FADT

[Andrew Jones <drjones@redhat.com>]

On Fri, May 06, 2022 at 09:55:46PM +0100, Nikos Nikoleris wrote:
> This change add more fields in the APCI table FADT to allow for the
> discovery of the PSCI conduit in arm64 systems. The definition for
> FADT is similar to the one in include/acpi/actbl.h in Linux.
> 
> Signed-off-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
> ---
>  lib/acpi.h   | 35 ++++++++++++++++++++++++++++++-----
>  lib/acpi.c   |  2 +-
>  x86/s3.c     |  2 +-
>  x86/vmexit.c |  2 +-
>  4 files changed, 33 insertions(+), 8 deletions(-)

Looks fine to me, so

Reviewed-by: Andrew Jones <drjones@redhat.com>

but we should get sign-off from x86 people since they've been expecting
fadt to be rev1 all this time and now it's changing.

Thanks,
drew

> 
> diff --git a/lib/acpi.h b/lib/acpi.h
> index d80b983..9f27eb1 100644
> --- a/lib/acpi.h
> +++ b/lib/acpi.h
> @@ -62,7 +62,15 @@ struct acpi_table_xsdt {
>      u64 table_offset_entry[1];
>  } __attribute__ ((packed));
>  
> -struct fadt_descriptor_rev1
> +struct acpi_generic_address {
> +    u8 space_id;            /* Address space where struct or register exists */
> +    u8 bit_width;           /* Size in bits of given register */
> +    u8 bit_offset;          /* Bit offset within the register */
> +    u8 access_width;        /* Minimum Access size (ACPI 3.0) */
> +    u64 address;            /* 64-bit address of struct or register */
> +} __attribute__ ((packed));
> +
> +struct acpi_table_fadt
>  {
>      ACPI_TABLE_HEADER_DEF     /* ACPI common table header */
>      u32 firmware_ctrl;          /* Physical address of FACS */
> @@ -100,10 +108,27 @@ struct fadt_descriptor_rev1
>      u8  day_alrm;               /* Index to day-of-month alarm in RTC CMOS RAM */
>      u8  mon_alrm;               /* Index to month-of-year alarm in RTC CMOS RAM */
>      u8  century;                /* Index to century in RTC CMOS RAM */
> -    u8  reserved4;              /* Reserved */
> -    u8  reserved4a;             /* Reserved */
> -    u8  reserved4b;             /* Reserved */
> -};
> +    u16 boot_flags;             /* IA-PC Boot Architecture Flags (see below for individual flags) */
> +    u8 reserved;                /* Reserved, must be zero */
> +    u32 flags;                  /* Miscellaneous flag bits (see below for individual flags) */
> +    struct acpi_generic_address reset_register;     /* 64-bit address of the Reset register */
> +    u8 reset_value;             /* Value to write to the reset_register port to reset the system */
> +    u16 arm_boot_flags;         /* ARM-Specific Boot Flags (see below for individual flags) (ACPI 5.1) */
> +    u8 minor_revision;          /* FADT Minor Revision (ACPI 5.1) */
> +    u64 Xfacs;                  /* 64-bit physical address of FACS */
> +    u64 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) */
> +    u64 hypervisor_id;      /* Hypervisor Vendor ID (ACPI 6.0) */
> +}  __attribute__ ((packed));
>  
>  struct facs_descriptor_rev1
>  {
> diff --git a/lib/acpi.c b/lib/acpi.c
> index 9b8700c..e8440ae 100644
> --- a/lib/acpi.c
> +++ b/lib/acpi.c
> @@ -46,7 +46,7 @@ void* find_acpi_table_addr(u32 sig)
>  
>  	/* FACS is special... */
>  	if (sig == FACS_SIGNATURE) {
> -		struct fadt_descriptor_rev1 *fadt;
> +		struct acpi_table_fadt *fadt;
>  
>  		fadt = find_acpi_table_addr(FACP_SIGNATURE);
>  		if (!fadt)
> diff --git a/x86/s3.c b/x86/s3.c
> index 89d69fc..16e79f8 100644
> --- a/x86/s3.c
> +++ b/x86/s3.c
> @@ -30,7 +30,7 @@ extern char resume_start, resume_end;
>  
>  int main(int argc, char **argv)
>  {
> -	struct fadt_descriptor_rev1 *fadt = find_acpi_table_addr(FACP_SIGNATURE);
> +	struct acpi_table_fadt *fadt = find_acpi_table_addr(FACP_SIGNATURE);
>  	struct facs_descriptor_rev1 *facs = find_acpi_table_addr(FACS_SIGNATURE);
>  	char *addr, *resume_vec = (void*)0x1000;
>  
> diff --git a/x86/vmexit.c b/x86/vmexit.c
> index 2bac049..fcc0760 100644
> --- a/x86/vmexit.c
> +++ b/x86/vmexit.c
> @@ -206,7 +206,7 @@ int pm_tmr_blk;
>  static void inl_pmtimer(void)
>  {
>      if (!pm_tmr_blk) {
> -	struct fadt_descriptor_rev1 *fadt;
> +	struct acpi_table_fadt *fadt;
>  
>  	fadt = find_acpi_table_addr(FACP_SIGNATURE);
>  	pm_tmr_blk = fadt->pm_tmr_blk;
> -- 
> 2.25.1
>