On 08/09/2023 07:23, Duan, Zhenzhong wrote:
>
> On 6/23/2023 5:48 AM, Joao Martins wrote:
>> Implement IOMMU MR get_attr() method and use the dma_translation
>> property to report the IOMMU_ATTR_DMA_TRANSLATION attribute.
>> Additionally add the necessary get_iommu_attr into the PCIIOMMUOps to
>> support pci_device_iommu_get_attr().
>>
>> The callback in there acts as a IOMMU-specific address space walker
>> which will call get_attr in the IOMMUMemoryRegion backing the device to
>> fetch the desired attribute.
>>
>> Signed-off-by: Avihai Horon <avih...@nvidia.com>
>> Signed-off-by: Joao Martins <joao.m.mart...@oracle.com>
>> ---
>> hw/i386/intel_iommu.c | 40 ++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 40 insertions(+)
>>
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 1606d1b952d0..ed2a46e008df 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -3861,6 +3861,29 @@ static void vtd_iommu_replay(IOMMUMemoryRegion
>> *iommu_mr, IOMMUNotifier *n)
>> return;
>> }
>> +static int vtd_iommu_get_attr(IOMMUMemoryRegion *iommu_mr,
>> + enum IOMMUMemoryRegionAttr attr, void *data)
>> +{
>> + VTDAddressSpace *vtd_as = container_of(iommu_mr, VTDAddressSpace,
>> iommu);
>> + IntelIOMMUState *s = vtd_as->iommu_state;
>> + int ret = 0;
>> +
>> + switch (attr) {
>> + case IOMMU_ATTR_DMA_TRANSLATION:
>> + {
>> + bool *enabled = (bool *)(uintptr_t) data;
>> +
>> + *enabled = s->dma_translation;
>> + break;
>> + }
>> + default:
>> + ret = -EINVAL;
>> + break;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> /* Do the initialization. It will also be called when reset, so pay
>> * attention when adding new initialization stuff.
>> */
>> @@ -4032,8 +4055,24 @@ static AddressSpace *vtd_host_dma_iommu(PCIBus *bus,
>> void *opaque, int devfn)
>> return &vtd_as->as;
>> }
>> +static int vtd_get_iommu_attr(PCIBus *bus, void *opaque, int32_t devfn,
>> + enum IOMMUMemoryRegionAttr attr, void *data)
>> +{
>> + IntelIOMMUState *s = opaque;
>> + VTDAddressSpace *vtd_as;
>> +
>> + assert(0 <= devfn && devfn < PCI_DEVFN_MAX);
>> +
>> + vtd_as = vtd_find_add_as(s, bus, devfn, PCI_NO_PASID);
>> + if (!vtd_as)
>> + return -EINVAL;
>> +
>> + return memory_region_iommu_get_attr(&vtd_as->iommu, attr, data);
>> +}
>> +
>> static PCIIOMMUOps vtd_iommu_ops = {
>> .get_address_space = vtd_host_dma_iommu,
>> + .get_iommu_attr = vtd_get_iommu_attr,
>> };
>> static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)
>> @@ -4197,6 +4236,7 @@ static void
>> vtd_iommu_memory_region_class_init(ObjectClass *klass,
>> imrc->translate = vtd_iommu_translate;
>> imrc->notify_flag_changed = vtd_iommu_notify_flag_changed;
>> imrc->replay = vtd_iommu_replay;
>> + imrc->get_attr = vtd_iommu_get_attr;
>
> Do we have other user of imrc->get_attr? If not, what about squashing
>
> vtd_iommu_get_attr into vtd_get_iommu_attr and have imrc->get_attr removed?
>
There's an user, and that's VFIO, but it's later in the series. It felt more
natural for bisection to do things this way.
