Re: [Qemu-devel] [PATCH for-3.1] hw/arm/virt-acpi-build: Fix SMMUv3 ACPI integration

[Auger Eric <eric.auger@redhat.com>]

Hi Shannon,

On 11/29/18 3:24 AM, Shannon Zhao wrote:
> 
> 
> On 2018/11/29 1:26, Auger Eric wrote:
>>>>>    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;
>>> So if we use this field ,we need to set the flags with
>>> ACPI_IORT_SMMU_V3_PXM_VALID
>>>>> +    uint32_t id_mapping_index;
>>> And if we use this field, it needs to set the revision to at least 1.
>> But is it harmful to add those fields in the struct as this patch does?
>>
>> - in our case ACPI_IORT_SMMU_V3_PXM_VALID flag is not set so the field
>> value is ignored according to the spec and arm_smmu_v3_set_proximity()
>> will not do anything.
>>
>> - SMMU control interrupts are all GSIV based so spec says that deviceID
>> index is ignored.
>>
>> So eventually the fact the struct size is changing over the revisions
>> does not look a problem because the node length is part of the struct
>> and the offset to the ID array also is part of the structure.
>>
>> So I could have left the structure as before (because we don't use the
>> functionalities associated to those fields). But on the other hand it's
>> good to upgrade the struct according to Rev D now.
>>
>> So I think the patch is correct, isn't?
> Yes, I think it's not harmful but it would be better to add some
> comments to explain why we don't increase the revision number ATM.
OK thank you for your input. I will repost for 3.2 and will upgrade the
table structs and their associated revision fields.

Thanks

Eric
> 
> Thanks,
> Shannon
>