From: Kuppuswamy Sathyanarayanan <[email protected]>
Intel IOMMU Page Request Services (PRS) only works with devices which
supports/uses PASID. So enable PRI only if the device also enables
PASID support. For more details, Please check the implementation of PRQ
handler(prq_event_thread()) in intel-svm driver.
Cc: Jacob Pan <[email protected]>
Cc: Ashok Raj <[email protected]>
Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
---
drivers/iommu/intel-iommu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 1457f931218e..ef1a646e0993 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1402,7 +1402,8 @@ static void iommu_enable_dev_iotlb(struct device_domain_info *info)
if (info->pasid_supported && !pci_enable_pasid(pdev, info->pasid_supported & ~1))
info->pasid_enabled = 1;
- if (info->pri_supported && !pci_reset_pri(pdev) && !pci_enable_pri(pdev, 32))
+ if (info->pri_supported && info->pasid_enabled &&
+ !pci_reset_pri(pdev) && !pci_enable_pri(pdev, 32))
info->pri_enabled = 1;
#endif
if (!pdev->untrusted && info->ats_supported &&
--
2.20.1
On Thu, 2019-02-07 at 10:44 -0800, [email protected] wrote:
> From: Kuppuswamy Sathyanarayanan <[email protected]>
>
> Intel IOMMU Page Request Services (PRS) only works with devices which
> supports/uses PASID. So enable PRI only if the device also enables
> PASID support. For more details, Please check the implementation of PRQ
> handler(prq_event_thread()) in intel-svm driver.
>
> Cc: Jacob Pan <[email protected]>
> Cc: Ashok Raj <[email protected]>
> Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
Hm, that shouldn't be true. PRI and PASID support are orthogonal.
If we get a PRI request without PASID, we should currently report it as
non-serviced which is the right thing to do. We can hook this up to KVM
etc. to actually allow paging of guests with devices attached, *if* the
devices attached to those guests support PRI.
Nothing fundamentally stops us using PRI without PASID support.
On Thu, Feb 07, 2019 at 08:08:06PM +0000, David Woodhouse wrote:
> On Thu, 2019-02-07 at 10:44 -0800, [email protected] wrote:
> > From: Kuppuswamy Sathyanarayanan <[email protected]>
> >
> > Intel IOMMU Page Request Services (PRS) only works with devices which
> > supports/uses PASID. So enable PRI only if the device also enables
> > PASID support. For more details, Please check the implementation of PRQ
> > handler(prq_event_thread()) in intel-svm driver.
> >
> > Cc: Jacob Pan <[email protected]>
> > Cc: Ashok Raj <[email protected]>
> > Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
>
> Hm, that shouldn't be true. PRI and PASID support are orthogonal.
>
> If we get a PRI request without PASID, we should currently report it as
> non-serviced which is the right thing to do. We can hook this up to KVM
> etc. to actually allow paging of guests with devices attached, *if* the
> devices attached to those guests support PRI.
>
> Nothing fundamentally stops us using PRI without PASID support.
You are right.. they are completely orthogonal. We just don't have
a way to handle the page-requests for request without PASID's.
There are some of the vIOMMU work to pass the PRI to who owns
the device, and we can certainly relax it then. This is just to reflect
what support exists today. FWIW, even the native driver maybe be able
to resolve this if supported.
>
>
On Thu, 2019-02-07 at 13:09 -0800, Raj, Ashok wrote:
> You are right.. they are completely orthogonal. We just don't have
> a way to handle the page-requests for request without PASID's.
>
> There are some of the vIOMMU work to pass the PRI to who owns
> the device, and we can certainly relax it then. This is just to reflect
> what support exists today. FWIW, even the native driver maybe be able
> to resolve this if supported.
As things stand, if a device makes a PRI request without a PASID, it'll
get told that we didn't manage to bring the page in for it. Which is
true.
What's the actual problem being fixed by this patch? Yes, we're going
to want to hook up a way to pass the PRI to the right place... but why
add *another* thing that's just going to have to be fixed, by reverting
this patch?
On Thu, Feb 07, 2019 at 09:15:24PM +0000, David Woodhouse wrote:
> On Thu, 2019-02-07 at 13:09 -0800, Raj, Ashok wrote:
> > You are right.. they are completely orthogonal. We just don't have
> > a way to handle the page-requests for request without PASID's.
> >
> > There are some of the vIOMMU work to pass the PRI to who owns
> > the device, and we can certainly relax it then. This is just to reflect
> > what support exists today. FWIW, even the native driver maybe be able
> > to resolve this if supported.
>
> As things stand, if a device makes a PRI request without a PASID, it'll
> get told that we didn't manage to bring the page in for it. Which is
> true.
That's true...it does seem to be covered already.. I can't remember
why I thought this was required :-(..
We can drop this patch.
>
> What's the actual problem being fixed by this patch? Yes, we're going
> to want to hook up a way to pass the PRI to the right place... but why
> add *another* thing that's just going to have to be fixed, by reverting
> this patch?
>
On 2/7/19 1:15 PM, David Woodhouse wrote:
> On Thu, 2019-02-07 at 13:09 -0800, Raj, Ashok wrote:
>> You are right.. they are completely orthogonal. We just don't have
>> a way to handle the page-requests for request without PASID's.
>>
>> There are some of the vIOMMU work to pass the PRI to who owns
>> the device, and we can certainly relax it then. This is just to reflect
>> what support exists today. FWIW, even the native driver maybe be able
>> to resolve this if supported.
> As things stand, if a device makes a PRI request without a PASID, it'll
> get told that we didn't manage to bring the page in for it. Which is
> true.
>
> What's the actual problem being fixed by this patch?
Since the request is going to fail any way why go through the process of
enabling it ? Once the functionality (PRI without PASID) is supported,
then they can revert this patch. Just we are trying to expose whats
currently supported clearly.
> Yes, we're going
> to want to hook up a way to pass the PRI to the right place... but why
> add *another* thing that's just going to have to be fixed, by reverting
> this patch?
>
--
Sathyanarayanan Kuppuswamy
Linux kernel developer