* [PATCH v1 1/1] iommu/vt-d: Enable PRI only if the device enables PASID.
@ 2019-02-07 18:44 sathyanarayanan.kuppuswamy
2019-02-07 20:08 ` David Woodhouse
0 siblings, 1 reply; 6+ messages in thread
From: sathyanarayanan.kuppuswamy @ 2019-02-07 18:44 UTC (permalink / raw)
To: joro, dwmw2
Cc: iommu, linux-kernel, Jacob Pan, Ashok Raj, Kuppuswamy Sathyanarayanan
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
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 <jacob.jun.pan@linux.intel.com>
Cc: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/1] iommu/vt-d: Enable PRI only if the device enables PASID.
2019-02-07 18:44 [PATCH v1 1/1] iommu/vt-d: Enable PRI only if the device enables PASID sathyanarayanan.kuppuswamy
@ 2019-02-07 20:08 ` David Woodhouse
2019-02-07 21:09 ` Raj, Ashok
0 siblings, 1 reply; 6+ messages in thread
From: David Woodhouse @ 2019-02-07 20:08 UTC (permalink / raw)
To: sathyanarayanan.kuppuswamy, joro
Cc: iommu, linux-kernel, Jacob Pan, Ashok Raj
[-- Attachment #1: Type: text/plain, Size: 1031 bytes --]
On Thu, 2019-02-07 at 10:44 -0800, sathyanarayanan.kuppuswamy@linux.intel.com wrote:
> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>
> 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 <jacob.jun.pan@linux.intel.com>
> Cc: Ashok Raj <ashok.raj@intel.com>
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
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.
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/1] iommu/vt-d: Enable PRI only if the device enables PASID.
2019-02-07 20:08 ` David Woodhouse
@ 2019-02-07 21:09 ` Raj, Ashok
2019-02-07 21:15 ` David Woodhouse
0 siblings, 1 reply; 6+ messages in thread
From: Raj, Ashok @ 2019-02-07 21:09 UTC (permalink / raw)
To: David Woodhouse
Cc: sathyanarayanan.kuppuswamy, joro, iommu, linux-kernel, Jacob Pan,
Ashok Raj
On Thu, Feb 07, 2019 at 08:08:06PM +0000, David Woodhouse wrote:
> On Thu, 2019-02-07 at 10:44 -0800, sathyanarayanan.kuppuswamy@linux.intel.com wrote:
> > From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> >
> > 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 <jacob.jun.pan@linux.intel.com>
> > Cc: Ashok Raj <ashok.raj@intel.com>
> > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>
> 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.
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/1] iommu/vt-d: Enable PRI only if the device enables PASID.
2019-02-07 21:09 ` Raj, Ashok
@ 2019-02-07 21:15 ` David Woodhouse
2019-02-07 21:22 ` Raj, Ashok
2019-02-07 21:30 ` sathyanarayanan kuppuswamy
0 siblings, 2 replies; 6+ messages in thread
From: David Woodhouse @ 2019-02-07 21:15 UTC (permalink / raw)
To: Raj, Ashok
Cc: sathyanarayanan.kuppuswamy, joro, iommu, linux-kernel, Jacob Pan
[-- Attachment #1: Type: text/plain, Size: 825 bytes --]
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?
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/1] iommu/vt-d: Enable PRI only if the device enables PASID.
2019-02-07 21:15 ` David Woodhouse
@ 2019-02-07 21:22 ` Raj, Ashok
2019-02-07 21:30 ` sathyanarayanan kuppuswamy
1 sibling, 0 replies; 6+ messages in thread
From: Raj, Ashok @ 2019-02-07 21:22 UTC (permalink / raw)
To: David Woodhouse
Cc: sathyanarayanan.kuppuswamy, joro, iommu, linux-kernel, Jacob Pan,
Ashok Raj
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?
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/1] iommu/vt-d: Enable PRI only if the device enables PASID.
2019-02-07 21:15 ` David Woodhouse
2019-02-07 21:22 ` Raj, Ashok
@ 2019-02-07 21:30 ` sathyanarayanan kuppuswamy
1 sibling, 0 replies; 6+ messages in thread
From: sathyanarayanan kuppuswamy @ 2019-02-07 21:30 UTC (permalink / raw)
To: David Woodhouse, Raj, Ashok; +Cc: joro, iommu, linux-kernel, Jacob Pan
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-02-07 21:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-07 18:44 [PATCH v1 1/1] iommu/vt-d: Enable PRI only if the device enables PASID sathyanarayanan.kuppuswamy
2019-02-07 20:08 ` David Woodhouse
2019-02-07 21:09 ` Raj, Ashok
2019-02-07 21:15 ` David Woodhouse
2019-02-07 21:22 ` Raj, Ashok
2019-02-07 21:30 ` sathyanarayanan kuppuswamy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).