From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55545) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gRWJU-0004WQ-EJ for qemu-devel@nongnu.org; Tue, 27 Nov 2018 00:53:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gRWJT-0005kn-ET for qemu-devel@nongnu.org; Tue, 27 Nov 2018 00:53:36 -0500 References: <20181126154616.18173-1-eric.auger@redhat.com> From: Auger Eric Message-ID: Date: Tue, 27 Nov 2018 06:53:25 +0100 MIME-Version: 1.0 In-Reply-To: <20181126154616.18173-1-eric.auger@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-3.1] hw/arm/virt-acpi-build: Fix SMMUv3 ACPI integration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: eric.auger.pro@gmail.com, qemu-devel@nongnu.org, qemu-arm@nongnu.org, peter.maydell@linaro.org, shannon.zhaosl@gmail.com Cc: shameerali.kolothum.thodi@huawei.com Hi Shannon, On 11/26/18 4:46 PM, Eric Auger wrote: > The AcpiIortSmmu3 misses 2 32b fields corresponding to the > proximity domain and the device id mapping index. I fail to understand how we currently track the evolutions of the IORT structures: Looking at the smmuv3 node in kernel include/acpi/actbl2.h - the following fields were added in c944230064eb ACPICA: iasl: Update to IORT SMMUv3 disassembling (1 year, 4 months ago) u8 pxm; u8 reserved1; u16 reserved2; - id_mapping_index was added in 4c106aa411ee ACPICA: iasl: Add SMMUv3 device ID mapping index support (1 year ago) - current u32 pxm was introduced in d87be0438e3d ACPICA: IORT: Update for revision D (6 months ago) I would have expected each of those evolutions to be tagged by a revision increment in the acpi_iort_node.revision field but this is not done. Also I fail to see any enum for encoding the revision. The smmuv3 struct that has been exposed until now corresponds to Rev C https://lists.linuxfoundation.org/pipermail/iommu/2017-May/022000.html (0c2021c047ba ACPICA: IORT: Update SMMU models for revision C (1 year, 4 months ago) Also I noticed that in rev D, new fields were added in struct acpi_iort_root_complex. We miss them at the moment in the IORT description. How does the guest kernel know which revision of the IORT is exposed? What do I miss? Thanks Eric > > Also 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 > Reported-by: Shameerali Kolothum Thodi > --- > hw/arm/virt-acpi-build.c | 1 + > include/hw/acpi/acpi-defs.h | 8 ++++++++ > 2 files changed, 9 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; > 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..c3ee1f517b 100644 > --- a/include/hw/acpi/acpi-defs.h > +++ b/include/hw/acpi/acpi-defs.h > @@ -628,6 +628,12 @@ struct AcpiIortItsGroup { > } QEMU_PACKED; > typedef struct AcpiIortItsGroup AcpiIortItsGroup; > > +enum { > + ACPI_IORT_SMMU_V3_COHACC_OVERRIDE = 1 << 0, > + ACPI_IORT_SMMU_V3_HTTU_OVERRIDE = 3 << 1, > + ACPI_IORT_SMMU_V3_PXM_VALID = 1 << 3 > +}; > + > struct AcpiIortSmmu3 { > ACPI_IORT_NODE_HEADER_DEF > uint64_t base_address; > @@ -639,6 +645,8 @@ struct AcpiIortSmmu3 { > uint32_t pri_gsiv; > uint32_t gerr_gsiv; > uint32_t sync_gsiv; > + uint32_t pxm; > + uint32_t id_mapping_index; > AcpiIortIdMapping id_mapping_array[0]; > } QEMU_PACKED; > typedef struct AcpiIortSmmu3 AcpiIortSmmu3; >