Re: [PATCH v4 20/22] iommu/vt-d: Add bind guest PASID support

From: Auger Eric
Date: Thu Jul 18 2019 - 03:47:14 EST


Hi Ashok,

On 7/16/19 7:04 PM, Raj, Ashok wrote:
> Hi Eric
>
> Jacob is on sabbatical, so i'll give it my best shot :-)
>
> Yi/Kevin can jump in...
>
> On Tue, Jul 16, 2019 at 06:45:51PM +0200, Auger Eric wrote:
>> Hi Jacob,
>>
>> On 6/9/19 3:44 PM, Jacob Pan wrote:
>>> When supporting guest SVA with emulated IOMMU, the guest PASID
>>> table is shadowed in VMM. Updates to guest vIOMMU PASID table
>>> will result in PASID cache flush which will be passed down to
>>> the host as bind guest PASID calls.
>>>
>>> For the SL page tables, it will be harvested from device's
>>> default domain (request w/o PASID), or aux domain in case of
>>> mediated device.
>>>
>>> .-------------. .---------------------------.
>>> | vIOMMU | | Guest process CR3, FL only|
>>> | | '---------------------------'
>>> .----------------/
>>> | PASID Entry |--- PASID cache flush -
>>> '-------------' |
>>> | | V
>>> | | CR3 in GPA
>>> '-------------'
>>> Guest
>>> ------| Shadow |--------------------------|--------
>>> v v v
>>> Host
>>> .-------------. .----------------------.
>>> | pIOMMU | | Bind FL for GVA-GPA |
>>> | | '----------------------'
>>> .----------------/ |
>>> | PASID Entry | V (Nested xlate)
>>> '----------------\.------------------------------.
>>> | | |SL for GPA-HPA, default domain|
>>> | | '------------------------------'
>>> '-------------'
>>> Where:
>>> - FL = First level/stage one page tables
>>> - SL = Second level/stage two page tables
>>>
>>> Signed-off-by: Jacob Pan <jacob.jun.pan@xxxxxxxxxxxxxxx>
>>> Signed-off-by: Liu, Yi L <yi.l.liu@xxxxxxxxxxxxxxx>
>>> ---
>>> drivers/iommu/intel-iommu.c | 4 +
>>> drivers/iommu/intel-svm.c | 187 ++++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/intel-iommu.h | 13 ++-
>>> include/linux/intel-svm.h | 17 ++++
>>> 4 files changed, 219 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
>>> index 7cfa0eb..3b4d712 100644
>>> --- a/drivers/iommu/intel-iommu.c
>>> +++ b/drivers/iommu/intel-iommu.c
>>> @@ -5782,6 +5782,10 @@ const struct iommu_ops intel_iommu_ops = {
>>> .dev_enable_feat = intel_iommu_dev_enable_feat,
>>> .dev_disable_feat = intel_iommu_dev_disable_feat,
>>> .pgsize_bitmap = INTEL_IOMMU_PGSIZES,
>>> +#ifdef CONFIG_INTEL_IOMMU_SVM
>>> + .sva_bind_gpasid = intel_svm_bind_gpasid,
>>> + .sva_unbind_gpasid = intel_svm_unbind_gpasid,
>>> +#endif
>>> };
>>>
>>> static void quirk_iommu_g4x_gfx(struct pci_dev *dev)
>>> diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
>>> index 66d98e1..f06a82f 100644
>>> --- a/drivers/iommu/intel-svm.c
>>> +++ b/drivers/iommu/intel-svm.c
>>> @@ -229,6 +229,193 @@ static LIST_HEAD(global_svm_list);
>>> list_for_each_entry(sdev, &svm->devs, list) \
>>> if (dev == sdev->dev) \
>>>
>>> +int intel_svm_bind_gpasid(struct iommu_domain *domain,
>>> + struct device *dev,
>>> + struct gpasid_bind_data *data)
>>> +{
>>> + struct intel_iommu *iommu = intel_svm_device_to_iommu(dev);
>>> + struct intel_svm_dev *sdev;
>>> + struct intel_svm *svm = NULL;
>> not requested
>>> + struct dmar_domain *ddomain;
>>> + int ret = 0;
>>> +
>>> + if (WARN_ON(!iommu) || !data)
>>> + return -EINVAL;
>>> +
>>> + if (data->version != IOMMU_GPASID_BIND_VERSION_1 ||
>>> + data->format != IOMMU_PASID_FORMAT_INTEL_VTD)
>>> + return -EINVAL;
>>> +
>>> + if (dev_is_pci(dev)) {
>>> + /* VT-d supports devices with full 20 bit PASIDs only */
>>> + if (pci_max_pasids(to_pci_dev(dev)) != PASID_MAX)
>>> + return -EINVAL;
>>> + }
>>> +
>>> + /*
>>> + * We only check host PASID range, we have no knowledge to check
>>> + * guest PASID range nor do we use the guest PASID.
>> guest pasid is set below in svm->gpasid.
>> So I am confused, do you handle gpasid or not? If you don't use gpasid
>> at the moment, then you may return -EINVAL if data->flags &
>> IOMMU_SVA_GPASID_VAL.
>>
>> I confess I don't really understand gpasid as I thought you use
>> enlightened PASID allocation to use a system wide PASID allocator on
>> host so in which case guest and host PASID do differ?
>
> Correct, from within the guest, we only use the enlightned allocator.
>
> Guest PASID can be managed via Qemu, and it will also associate
> guest pasid's with appropriate host pasids. Sort of how guest bdf
> and host bdf are managed for page-request etc.

OK I will look at the QEMU side then.

Thanks

Eric
>