[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <00a5fe10-98bb-f018-06b6-29af22d014eb@arm.com>
Date: Wed, 28 Jun 2017 18:07:32 +0100
From: Jean-Philippe Brucker <jean-philippe.brucker@....com>
To: Jacob Pan <jacob.jun.pan@...ux.intel.com>,
Joerg Roedel <joro@...tes.org>
Cc: Lan Tianyu <tianyu.lan@...el.com>,
"Liu, Yi L" <yi.l.liu@...ux.intel.com>,
"Tian, Kevin" <kevin.tian@...el.com>,
LKML <linux-kernel@...r.kernel.org>,
iommu@...ts.linux-foundation.org,
Jean Delvare <khali@...ux-fr.org>,
David Woodhouse <dwmw2@...radead.org>
Subject: Re: [PATCH 3/9] iommu: Introduce iommu do invalidate API function
On 28/06/17 17:09, Jacob Pan wrote:
> On Wed, 28 Jun 2017 12:08:23 +0200
> Joerg Roedel <joro@...tes.org> wrote:
>
>> On Tue, Jun 27, 2017 at 12:47:57PM -0700, Jacob Pan wrote:
>>> From: "Liu, Yi L" <yi.l.liu@...ux.intel.com>
>>>
>>> When a SVM capable device is assigned to a guest, the first level
>>> page tables are owned by the guest and the guest PASID table
>>> pointer is linked to the device context entry of the physical IOMMU.
>>>
>>> Host IOMMU driver has no knowledge of caching structure updates
>>> unless the guest invalidation activities are passed down to the
>>> host. The primary usage is derived from emulated IOMMU in the
>>> guest, where QEMU can trap invalidation activities before pass them
>>> down the host/physical IOMMU. There are IOMMU architectural
>>> specific actions need to be taken which requires the generic APIs
>>> introduced in this patch to have opaque data in the
>>> tlb_invalidate_info argument.
>>
>> Which "IOMMU architectural specific actions" are you thinking of?
>>
> construction of queued invalidation descriptors, then submit them to
> the IOMMU QI interface.
>>> +int iommu_invalidate(struct iommu_domain *domain,
>>> + struct device *dev, struct tlb_invalidate_info
>>> *inv_info) +{
>>> + int ret = 0;
>>> +
>>> + if (unlikely(!domain->ops->invalidate))
>>> + return -ENODEV;
>>> +
>>> + ret = domain->ops->invalidate(domain, dev, inv_info);
>>> +
>>> + return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(iommu_invalidate);
>>
>> [...]
>>
>>> +struct tlb_invalidate_info {
>>> + __u32 model;
>>> + __u32 length;
>>> + __u8 opaque[];
>>> +};
>>
>> This interface is aweful. It requires the user of a generic api to
>> know details about the implementation behind to do anything useful.
>>
>> Please explain in more detail why this is needed. My feeling is that
>> we can make this more generic with a small set of invalidation
>> functions in the iommu-api.
>>
> My thinking was that via configuration control, there will be unlikely
> any mixed IOMMU models between pIOMMU and vIOMMU. We could have just
> model specific data pass through layers of SW (QEMU, VFIO) for
> performance reasons. We do have an earlier hybrid version that has
> generic data and opaque raw data. Would the below work for all IOMMU
> models?
For reference, this was also discussed in the initial posting of the series:
https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg03452.html
At least for ARM SMMUv2 and v3, I think the invalidation format you
propose should be sufficient, although "device_selective" should probably
be "domain_selective". And maybe a flag field could contain relatively
generic hints such as "only invalidate leaf table when page_selective".
Thanks,
Jean
> https://www.spinics.net/lists/kvm/msg148798.html
>
> struct tlb_invalidate_info
> {
> __u32 model; /* Vendor number */
> __u8 granularity
> #define DEVICE_SELECTVIE_INV (1 << 0)
> #define PAGE_SELECTIVE_INV (1 << 0)
> #define PASID_SELECTIVE_INV (1 << 1)
> __u32 pasid;
> __u64 addr;
> __u64 size;
>
> /* Since IOMMU format has already been validated for this table,
> the IOMMU driver knows that the following structure is in a
> format it knows */
> __u8 opaque[];
> };
>
>>
>>
>> Joerg
>>
>
> [Jacob Pan]
> _______________________________________________
> iommu mailing list
> iommu@...ts.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>
Powered by blists - more mailing lists