Re: [Qemu-devel] [PATCH-for-4.0 v2 1/2] hw/arm/virt-acpi-build: Set COHACC override flag in IORT SMMUv3 node

[Andrew Jones]

On Thu, Dec 06, 2018 at 06:07:32PM +0100, Eric Auger wrote:
> Let's report IO-coherent access is supported for translation
> table walks, descriptor fetches and queues by setting the COHACC
> override flag. Without that, we observe wrong command opcodes.
> The DT description also advertises the dma coherency.
> 
> Fixes a703b4f6c1ee ("hw/arm/virt-acpi-build: Add smmuv3 node in IORT table")
> 
> Signed-off-by: Eric Auger <address@hidden>
> Reported-by: Shameerali Kolothum Thodi <address@hidden>
> Tested-by: Shameer Kolothum <address@hidden>
> 
> ---
> 
> - change the commit title
> - addition of new fields (pxm and id_mapping_index) done in a
>   separate patch
> ---
>  hw/arm/virt-acpi-build.c    | 1 +
>  include/hw/acpi/acpi-defs.h | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 5785fb697c..aa177ba64d 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -448,6 +448,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, 
> VirtMachineState *vms)
>          smmu->mapping_count = cpu_to_le32(1);
>          smmu->mapping_offset = cpu_to_le32(sizeof(*smmu));
>          smmu->base_address = cpu_to_le64(vms->memmap[VIRT_SMMU].base);
> +        smmu->flags = ACPI_IORT_SMMU_V3_COHACC_OVERRIDE;

Flags are 4 bytes in length, so you need cpu_to_le32()

>          smmu->event_gsiv = cpu_to_le32(irq);
>          smmu->pri_gsiv = cpu_to_le32(irq + 1);
>          smmu->gerr_gsiv = cpu_to_le32(irq + 2);
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index af8e023968..532eaf79bd 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -628,6 +628,8 @@ struct AcpiIortItsGroup {
>  } QEMU_PACKED;
>  typedef struct AcpiIortItsGroup AcpiIortItsGroup;
>  
> +#define ACPI_IORT_SMMU_V3_COHACC_OVERRIDE 1
> +
>  struct AcpiIortSmmu3 {
>      ACPI_IORT_NODE_HEADER_DEF
>      uint64_t base_address;
> -- 
> 2.17.2
> 
> 

Probably not necessary for this series, but we should really switch all
table generators to the aml_append* API.

Besides the missing cpu_to_le32(),

Reviewed-by: Andrew Jones <address@hidden>