Re: [PATCH v2 2/3] hw/arm/virt-acpi-build: IORT upgrade up to revision E.b

[Jean-Philippe Brucker]

On Tue, Oct 05, 2021 at 10:38:04AM +0200, Eric Auger wrote:
> Upgrade the IORT table from B to E.b specification
> revision (ARM DEN 0049E.b).
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>

Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

Two nits below

> 
> ---
> 
> v1 -> v2:
> - Fix Revision value for ITS node and SMMUv3 node
> - increment an identifier
> ---
>  hw/arm/virt-acpi-build.c | 48 ++++++++++++++++++++++++----------------
>  1 file changed, 29 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 257d0fee17..789bac3134 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -241,19 +241,20 @@ static void acpi_dsdt_add_tpm(Aml *scope, 
> VirtMachineState *vms)
>  #endif
>  
>  #define ID_MAPPING_ENTRY_SIZE 20
> -#define SMMU_V3_ENTRY_SIZE 60
> -#define ROOT_COMPLEX_ENTRY_SIZE 32
> +#define SMMU_V3_ENTRY_SIZE 68
> +#define ROOT_COMPLEX_ENTRY_SIZE 36
>  #define IORT_NODE_OFFSET 48
>  
>  static void build_iort_id_mapping(GArray *table_data, uint32_t input_base,
>                                    uint32_t id_count, uint32_t out_ref)
>  {
> -    /* Identity RID mapping covering the whole input RID range */
> +    /* Table 4 ID mapping format */
>      build_append_int_noprefix(table_data, input_base, 4); /* Input base */
>      build_append_int_noprefix(table_data, id_count, 4); /* Number of IDs */
>      build_append_int_noprefix(table_data, input_base, 4); /* Output base */
>      build_append_int_noprefix(table_data, out_ref, 4); /* Output Reference */
> -    build_append_int_noprefix(table_data, 0, 4); /* Flags */
> +    /* Flags */
> +    build_append_int_noprefix(table_data, 0 /* Single mapping */, 4);

The comment is a bit confusing, seems to indicate that "Single mapping" is
enabled by value 0

>  }
>  
>  struct AcpiIortIdMapping {
> @@ -298,7 +299,7 @@ static int iort_idmap_compare(gconstpointer a, 
> gconstpointer b)
>  /*
>   * Input Output Remapping Table (IORT)
>   * Conforms to "IO Remapping Table System Software on ARM Platforms",
> - * Document number: ARM DEN 0049B, October 2015
> + * Document number: ARM DEN 0049E, Feb 2021

0049E.b?

>   */
>  static void
>  build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)