* [PATCH v2 0/2] iommu/vt-d: Some misc tweaks in SVA
@ 2021-01-26 8:07 ` Lu Baolu
0 siblings, 0 replies; 7+ messages in thread
From: Lu Baolu @ 2021-01-26 8:07 UTC (permalink / raw)
To: Joerg Roedel, Will Deacon; +Cc: Kevin Tian, Ashok Raj, linux-kernel, iommu
This includes some misc tweaks in the VT-d SVA implementation. I will
plan them for v5.12 if no objections.
Change log:
v1->v2:
- v1:
https://lore.kernel.org/linux-iommu/20210121014505.1659166-1-baolu.lu@linux.intel.com/
- Keep the logic of clearing PRO sane
- Drop the device outstanding PRQ number patch. Need more test and
will repost later.
Best regards,
baolu
Lu Baolu (2):
iommu/vt-d: Clear PRQ overflow only when PRQ is empty
iommu/vt-d: Use INVALID response code instead of FAILURE
drivers/iommu/intel/svm.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
--
2.25.1
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2 1/2] iommu/vt-d: Clear PRQ overflow only when PRQ is empty
2021-01-26 8:07 ` Lu Baolu
@ 2021-01-26 8:07 ` Lu Baolu
-1 siblings, 0 replies; 7+ messages in thread
From: Lu Baolu @ 2021-01-26 8:07 UTC (permalink / raw)
To: Joerg Roedel, Will Deacon
Cc: Ashok Raj, Jacob Pan, Kevin Tian, Liu Yi L, iommu, linux-kernel,
Lu Baolu
It is incorrect to always clear PRO when it's set w/o first checking
whether the overflow condition has been cleared. Current code assumes
that if an overflow condition occurs it must have been cleared by earlier
loop. However since the code runs in a threaded context, the overflow
condition could occur even after setting the head to the tail under some
extreme condition. To be sane, we should read both head/tail again when
seeing a pending PRO and only clear PRO after all pending PRs have been
handled.
Suggested-by: Kevin Tian <kevin.tian@intel.com>
Link: https://lore.kernel.org/linux-iommu/MWHPR11MB18862D2EA5BD432BF22D99A48CA09@MWHPR11MB1886.namprd11.prod.outlook.com/
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
drivers/iommu/intel/svm.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 033b25886e57..d7c98c5fa4e7 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -1042,8 +1042,17 @@ static irqreturn_t prq_event_thread(int irq, void *d)
* Clear the page request overflow bit and wake up all threads that
* are waiting for the completion of this handling.
*/
- if (readl(iommu->reg + DMAR_PRS_REG) & DMA_PRS_PRO)
- writel(DMA_PRS_PRO, iommu->reg + DMAR_PRS_REG);
+ if (readl(iommu->reg + DMAR_PRS_REG) & DMA_PRS_PRO) {
+ pr_info_ratelimited("IOMMU: %s: PRQ overflow detected\n",
+ iommu->name);
+ head = dmar_readq(iommu->reg + DMAR_PQH_REG) & PRQ_RING_MASK;
+ tail = dmar_readq(iommu->reg + DMAR_PQT_REG) & PRQ_RING_MASK;
+ if (head == tail) {
+ writel(DMA_PRS_PRO, iommu->reg + DMAR_PRS_REG);
+ pr_info_ratelimited("IOMMU: %s: PRQ overflow cleared",
+ iommu->name);
+ }
+ }
if (!completion_done(&iommu->prq_complete))
complete(&iommu->prq_complete);
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 1/2] iommu/vt-d: Clear PRQ overflow only when PRQ is empty
@ 2021-01-26 8:07 ` Lu Baolu
0 siblings, 0 replies; 7+ messages in thread
From: Lu Baolu @ 2021-01-26 8:07 UTC (permalink / raw)
To: Joerg Roedel, Will Deacon; +Cc: Kevin Tian, Ashok Raj, linux-kernel, iommu
It is incorrect to always clear PRO when it's set w/o first checking
whether the overflow condition has been cleared. Current code assumes
that if an overflow condition occurs it must have been cleared by earlier
loop. However since the code runs in a threaded context, the overflow
condition could occur even after setting the head to the tail under some
extreme condition. To be sane, we should read both head/tail again when
seeing a pending PRO and only clear PRO after all pending PRs have been
handled.
Suggested-by: Kevin Tian <kevin.tian@intel.com>
Link: https://lore.kernel.org/linux-iommu/MWHPR11MB18862D2EA5BD432BF22D99A48CA09@MWHPR11MB1886.namprd11.prod.outlook.com/
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
drivers/iommu/intel/svm.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 033b25886e57..d7c98c5fa4e7 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -1042,8 +1042,17 @@ static irqreturn_t prq_event_thread(int irq, void *d)
* Clear the page request overflow bit and wake up all threads that
* are waiting for the completion of this handling.
*/
- if (readl(iommu->reg + DMAR_PRS_REG) & DMA_PRS_PRO)
- writel(DMA_PRS_PRO, iommu->reg + DMAR_PRS_REG);
+ if (readl(iommu->reg + DMAR_PRS_REG) & DMA_PRS_PRO) {
+ pr_info_ratelimited("IOMMU: %s: PRQ overflow detected\n",
+ iommu->name);
+ head = dmar_readq(iommu->reg + DMAR_PQH_REG) & PRQ_RING_MASK;
+ tail = dmar_readq(iommu->reg + DMAR_PQT_REG) & PRQ_RING_MASK;
+ if (head == tail) {
+ writel(DMA_PRS_PRO, iommu->reg + DMAR_PRS_REG);
+ pr_info_ratelimited("IOMMU: %s: PRQ overflow cleared",
+ iommu->name);
+ }
+ }
if (!completion_done(&iommu->prq_complete))
complete(&iommu->prq_complete);
--
2.25.1
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] iommu/vt-d: Use INVALID response code instead of FAILURE
2021-01-26 8:07 ` Lu Baolu
@ 2021-01-26 8:07 ` Lu Baolu
-1 siblings, 0 replies; 7+ messages in thread
From: Lu Baolu @ 2021-01-26 8:07 UTC (permalink / raw)
To: Joerg Roedel, Will Deacon
Cc: Ashok Raj, Jacob Pan, Kevin Tian, Liu Yi L, iommu, linux-kernel,
Lu Baolu
The VT-d IOMMU response RESPONSE_FAILURE for a page request in below
cases:
- When it gets a Page_Request with no PASID;
- When it gets a Page_Request with PASID that is not in use for this
device.
This is allowed by the spec, but IOMMU driver doesn't support such cases
today. When the device receives RESPONSE_FAILURE, it sends the device
state machine to HALT state. Now if we try to unload the driver, it hangs
since the device doesn't send any outbound transactions to host when the
driver is trying to clear things up. The only possible responses would be
for invalidation requests.
Let's use RESPONSE_INVALID instead for now, so that the device state
machine doesn't enter HALT state.
Suggested-by: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
drivers/iommu/intel/svm.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index d7c98c5fa4e7..574a7e657a9a 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -911,10 +911,8 @@ static irqreturn_t prq_event_thread(int irq, void *d)
u64 address;
handled = 1;
-
req = &iommu->prq[head / sizeof(*req)];
-
- result = QI_RESP_FAILURE;
+ result = QI_RESP_INVALID;
address = (u64)req->addr << VTD_PAGE_SHIFT;
if (!req->pasid_present) {
pr_err("%s: Page request without PASID: %08llx %08llx\n",
@@ -952,7 +950,6 @@ static irqreturn_t prq_event_thread(int irq, void *d)
rcu_read_unlock();
}
- result = QI_RESP_INVALID;
/* Since we're using init_mm.pgd directly, we should never take
* any faults on kernel addresses. */
if (!svm->mm)
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 2/2] iommu/vt-d: Use INVALID response code instead of FAILURE
@ 2021-01-26 8:07 ` Lu Baolu
0 siblings, 0 replies; 7+ messages in thread
From: Lu Baolu @ 2021-01-26 8:07 UTC (permalink / raw)
To: Joerg Roedel, Will Deacon; +Cc: Kevin Tian, Ashok Raj, linux-kernel, iommu
The VT-d IOMMU response RESPONSE_FAILURE for a page request in below
cases:
- When it gets a Page_Request with no PASID;
- When it gets a Page_Request with PASID that is not in use for this
device.
This is allowed by the spec, but IOMMU driver doesn't support such cases
today. When the device receives RESPONSE_FAILURE, it sends the device
state machine to HALT state. Now if we try to unload the driver, it hangs
since the device doesn't send any outbound transactions to host when the
driver is trying to clear things up. The only possible responses would be
for invalidation requests.
Let's use RESPONSE_INVALID instead for now, so that the device state
machine doesn't enter HALT state.
Suggested-by: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
drivers/iommu/intel/svm.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index d7c98c5fa4e7..574a7e657a9a 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -911,10 +911,8 @@ static irqreturn_t prq_event_thread(int irq, void *d)
u64 address;
handled = 1;
-
req = &iommu->prq[head / sizeof(*req)];
-
- result = QI_RESP_FAILURE;
+ result = QI_RESP_INVALID;
address = (u64)req->addr << VTD_PAGE_SHIFT;
if (!req->pasid_present) {
pr_err("%s: Page request without PASID: %08llx %08llx\n",
@@ -952,7 +950,6 @@ static irqreturn_t prq_event_thread(int irq, void *d)
rcu_read_unlock();
}
- result = QI_RESP_INVALID;
/* Since we're using init_mm.pgd directly, we should never take
* any faults on kernel addresses. */
if (!svm->mm)
--
2.25.1
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 0/2] iommu/vt-d: Some misc tweaks in SVA
2021-01-26 8:07 ` Lu Baolu
` (2 preceding siblings ...)
(?)
@ 2021-01-29 8:25 ` Joerg Roedel
-1 siblings, 0 replies; 7+ messages in thread
From: Joerg Roedel @ 2021-01-29 8:25 UTC (permalink / raw)
To: Lu Baolu; +Cc: Kevin Tian, Ashok Raj, linux-kernel, iommu, Will Deacon
On Tue, Jan 26, 2021 at 04:07:28PM +0800, Lu Baolu wrote:
> Lu Baolu (2):
> iommu/vt-d: Clear PRQ overflow only when PRQ is empty
> iommu/vt-d: Use INVALID response code instead of FAILURE
>
> drivers/iommu/intel/svm.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
Applied, thanks.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 7+ messages in thread