Re: [PATCH ats_vtd v5 20/22] pci: add a pci-level API for ATS

[CLEMENT MATHIEU--DRIF]

On 09/07/2024 12:15, Minwoo Im wrote:
> Caution: External email. Do not open attachments or click links, unless this 
> email comes from a known sender and you know the content is safe.
>
>
> On 24-07-02 05:52:45, CLEMENT MATHIEU--DRIF wrote:
>> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>>
>> Devices implementing ATS can send translation requests using
>> pci_ats_request_translation_pasid.
>>
>> The invalidation events are sent back to the device using the iommu
>> notifier managed with pci_register_iommu_tlb_event_notifier and
>> pci_unregister_iommu_tlb_event_notifier
>>
>> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>> ---
>>   hw/pci/pci.c         | 44 +++++++++++++++++++++++++++++++++++++
>>   include/hw/pci/pci.h | 52 ++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 96 insertions(+)
>>
>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>> index 7a483dd05d..93b816aff2 100644
>> --- a/hw/pci/pci.c
>> +++ b/hw/pci/pci.c
>> @@ -2833,6 +2833,50 @@ void pci_device_unset_iommu_device(PCIDevice *dev)
>>       }
>>   }
>>
>> +ssize_t pci_ats_request_translation_pasid(PCIDevice *dev, uint32_t pasid,
>> +                                          bool priv_req, bool exec_req,
>> +                                          hwaddr addr, size_t length,
>> +                                          bool no_write, IOMMUTLBEntry 
>> *result,
>> +                                          size_t result_length,
>> +                                          uint32_t *err_count)
>> +{
>> +    assert(result_length);
>> +    IOMMUMemoryRegion *iommu_mr = pci_device_iommu_memory_region_pasid(dev,
>> +                                                                        
>> pasid);
>> +    if (!iommu_mr || !pcie_ats_enabled(dev)) {
>> +        return -EPERM;
>> +    }
>> +    return memory_region_iommu_ats_request_translation(iommu_mr, priv_req,
>> +                                                       exec_req, addr, 
>> length,
>> +                                                       no_write, result,
>> +                                                       result_length,
>> +                                                       err_count);
>> +}
> Can we use this function not from the endpoint PCI device, but inside of the 
> pci
> subsystem (hw/pci/pci.c) to make transparent abstraction for ATS request from
> PCI endpoint device POV?  I guess it would be better to have PCI subsystem to
> issue ATS request if pcie_ats_enabled(dev) rather than calling from the 
> endpoint
> side.
Hi,

This series aims to bring support for SVM (we are trying to integrate 
the patches bit by bit).
 From a spec point of view, I don't know if it would make sense to 
implement the SVM logic at the PCI level
as it's supposed to be implemented by endpoint devices.
However, we could consider providing a reference/reusable/encapsulated 
implementation of SVM with a simplified API
that would call the pci_* functions under the hood.

Do you have a specific use case in mind?

 >cmd

>