iommu.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] iommu/amd: Add PPR overflow support
@ 2023-06-09 10:17 Vasant Hegde
  2023-06-09 10:17 ` [PATCH 1/2] iommu/amd: Generalize log overflow handling Vasant Hegde
  2023-06-09 10:17 ` [PATCH 2/2] iommu/amd: Handle PPR log overflow Vasant Hegde
  0 siblings, 2 replies; 9+ messages in thread
From: Vasant Hegde @ 2023-06-09 10:17 UTC (permalink / raw)
  To: iommu, joro; +Cc: suravee.suthikulpanit, joao.m.martins, Vasant Hegde

Current IOMMU driver is lacking PPR log overflow handling. Add logic to
restart PPR log after an overflow has occurred.

This patch applies on top of cleanup series:
  https://lore.kernel.org/linux-iommu/20230609090631.6052-1-vasant.hegde@amd.com/T/#t

Vasant Hegde (2):
  iommu/amd: Generalize log overflow handling
  iommu/amd: Handle PPR log overflow

 drivers/iommu/amd/amd_iommu.h       |  1 +
 drivers/iommu/amd/amd_iommu_types.h |  5 ++-
 drivers/iommu/amd/init.c            | 60 ++++++++++++++++++++---------
 drivers/iommu/amd/iommu.c           | 13 +++++--
 4 files changed, 57 insertions(+), 22 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] iommu/amd: Generalize log overflow handling
  2023-06-09 10:17 [PATCH 0/2] iommu/amd: Add PPR overflow support Vasant Hegde
@ 2023-06-09 10:17 ` Vasant Hegde
  2023-06-13 20:22   ` Jerry Snitselaar
  2023-06-20 14:51   ` Joao Martins
  2023-06-09 10:17 ` [PATCH 2/2] iommu/amd: Handle PPR log overflow Vasant Hegde
  1 sibling, 2 replies; 9+ messages in thread
From: Vasant Hegde @ 2023-06-09 10:17 UTC (permalink / raw)
  To: iommu, joro; +Cc: suravee.suthikulpanit, joao.m.martins, Vasant Hegde

IOMMU has three different logs (Event, GA and PPR log) and each uses
different buffer for logging. Once buffer becomes full IOMMU generates
overflow interrupt and we have to restart the logs. Log restart procedure
is same for all three logs except it uses different bits in 'MMIO Offset
2020h IOMMU Status Register'. Hence lets move common code to generic
function and individual log overflow handler will call generic function
with appropriate parameters.

Also rename MMIO_STATUS_EVT_OVERFLOW_INT_MASK as
MMIO_STATUS_EVT_OVERFLOW_MASK.

Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/amd_iommu_types.h |  3 +-
 drivers/iommu/amd/init.c            | 51 ++++++++++++++++++-----------
 drivers/iommu/amd/iommu.c           |  4 +--
 3 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index 364fdaa52e74..318b84cf47f6 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -120,9 +120,10 @@
 #define PASID_MASK		0x0000ffff
 
 /* MMIO status bits */
-#define MMIO_STATUS_EVT_OVERFLOW_INT_MASK	BIT(0)
+#define MMIO_STATUS_EVT_OVERFLOW_MASK		BIT(0)
 #define MMIO_STATUS_EVT_INT_MASK		BIT(1)
 #define MMIO_STATUS_COM_WAIT_INT_MASK		BIT(2)
+#define MMIO_STATUS_EVT_RUN_MASK		BIT(3)
 #define MMIO_STATUS_PPR_INT_MASK		BIT(6)
 #define MMIO_STATUS_GALOG_RUN_MASK		BIT(8)
 #define MMIO_STATUS_GALOG_OVERFLOW_MASK		BIT(9)
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index c2d80a4e5fb0..3c21e9333899 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -748,38 +748,51 @@ static int __init alloc_command_buffer(struct amd_iommu *iommu)
 	return iommu->cmd_buf ? 0 : -ENOMEM;
 }
 
+/*
+ * Interrupt handler has processed all pending events and adjusted head
+ * and tail pointer. Reset overflow mask and restart logging again.
+ */
+static void amd_iommu_restart_log(struct amd_iommu *iommu, const char *evt_type,
+				  u8 cntrl_intr, u8 cntrl_log,
+				  u32 status_run_mask, u32 status_overflow_mask)
+{
+	u32 status;
+
+	status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
+	if (status & status_run_mask)
+		return;
+
+	pr_info_ratelimited("IOMMU %s log restarting\n", evt_type);
+
+	iommu_feature_disable(iommu, cntrl_log);
+	iommu_feature_disable(iommu, cntrl_intr);
+
+	writel(status_overflow_mask, iommu->mmio_base + MMIO_STATUS_OFFSET);
+
+	iommu_feature_enable(iommu, cntrl_intr);
+	iommu_feature_enable(iommu, cntrl_log);
+}
+
 /*
  * This function restarts event logging in case the IOMMU experienced
  * an event log buffer overflow.
  */
 void amd_iommu_restart_event_logging(struct amd_iommu *iommu)
 {
-	iommu_feature_disable(iommu, CONTROL_EVT_LOG_EN);
-	iommu_feature_enable(iommu, CONTROL_EVT_LOG_EN);
+	amd_iommu_restart_log(iommu, "Event", CONTROL_EVT_INT_EN,
+			      CONTROL_EVT_LOG_EN, MMIO_STATUS_EVT_RUN_MASK,
+			      MMIO_STATUS_EVT_OVERFLOW_MASK);
 }
 
 /*
  * This function restarts event logging in case the IOMMU experienced
- * an GA log overflow.
+ * GA log overflow.
  */
 void amd_iommu_restart_ga_log(struct amd_iommu *iommu)
 {
-	u32 status;
-
-	status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
-	if (status & MMIO_STATUS_GALOG_RUN_MASK)
-		return;
-
-	pr_info_ratelimited("IOMMU GA Log restarting\n");
-
-	iommu_feature_disable(iommu, CONTROL_GALOG_EN);
-	iommu_feature_disable(iommu, CONTROL_GAINT_EN);
-
-	writel(MMIO_STATUS_GALOG_OVERFLOW_MASK,
-	       iommu->mmio_base + MMIO_STATUS_OFFSET);
-
-	iommu_feature_enable(iommu, CONTROL_GAINT_EN);
-	iommu_feature_enable(iommu, CONTROL_GALOG_EN);
+	amd_iommu_restart_log(iommu, "GA", CONTROL_GAINT_EN,
+			      CONTROL_GALOG_EN, MMIO_STATUS_GALOG_RUN_MASK,
+			      MMIO_STATUS_GALOG_OVERFLOW_MASK);
 }
 
 /*
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index ad501944d77c..a29548d98b6a 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -842,7 +842,7 @@ amd_iommu_set_pci_msi_domain(struct device *dev, struct amd_iommu *iommu) { }
 #endif /* !CONFIG_IRQ_REMAP */
 
 #define AMD_IOMMU_INT_MASK	\
-	(MMIO_STATUS_EVT_OVERFLOW_INT_MASK | \
+	(MMIO_STATUS_EVT_OVERFLOW_MASK | \
 	 MMIO_STATUS_EVT_INT_MASK | \
 	 MMIO_STATUS_PPR_INT_MASK | \
 	 MMIO_STATUS_GALOG_OVERFLOW_MASK | \
@@ -881,7 +881,7 @@ irqreturn_t amd_iommu_int_thread(int irq, void *data)
 		}
 #endif
 
-		if (status & MMIO_STATUS_EVT_OVERFLOW_INT_MASK) {
+		if (status & MMIO_STATUS_EVT_OVERFLOW_MASK) {
 			pr_info_ratelimited("IOMMU event log overflow\n");
 			amd_iommu_restart_event_logging(iommu);
 		}
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] iommu/amd: Handle PPR log overflow
  2023-06-09 10:17 [PATCH 0/2] iommu/amd: Add PPR overflow support Vasant Hegde
  2023-06-09 10:17 ` [PATCH 1/2] iommu/amd: Generalize log overflow handling Vasant Hegde
@ 2023-06-09 10:17 ` Vasant Hegde
  2023-06-13 20:29   ` Jerry Snitselaar
  2023-06-20 14:53   ` Joao Martins
  1 sibling, 2 replies; 9+ messages in thread
From: Vasant Hegde @ 2023-06-09 10:17 UTC (permalink / raw)
  To: iommu, joro; +Cc: suravee.suthikulpanit, joao.m.martins, Vasant Hegde

Some ATS-capable peripherals can issue requests to the processor to service
peripheral page requests using PCIe PRI (the Page Request Interface). IOMMU
supports PRI using PPR log buffer. IOMMU writes PRI request to PPR log
buffer and sends PPR interrupt to host. When there is no space in the
PPR log buffer (PPR log overflow) it will set PprOverflow bit in 'MMIO
Offset 2020h IOMMU Status Register'. When this happens PPR log needs to be
restarted as specified in IOMMU spec [1] section 2.6.2.

When handling the event it just resumes the PPR log without resizing
(similar to the way event and GA log overflow is handled).

Failing to handle PPR overflow means device may not work properly as
IOMMU stops processing new PPR events from device.

[1] https://www.amd.com/system/files/TechDocs/48882_3.07_PUB.pdf

Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/amd_iommu.h       |  1 +
 drivers/iommu/amd/amd_iommu_types.h |  2 ++
 drivers/iommu/amd/init.c            | 11 +++++++++++
 drivers/iommu/amd/iommu.c           |  9 ++++++++-
 4 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index 317d5c26c614..156f57b4f78c 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -16,6 +16,7 @@ irqreturn_t amd_iommu_int_handler(int irq, void *data);
 void amd_iommu_apply_erratum_63(struct amd_iommu *iommu, u16 devid);
 void amd_iommu_restart_event_logging(struct amd_iommu *iommu);
 void amd_iommu_restart_ga_log(struct amd_iommu *iommu);
+void amd_iommu_restart_ppr_log(struct amd_iommu *iommu);
 void amd_iommu_set_rlookup_table(struct amd_iommu *iommu, u16 devid);
 
 #ifdef CONFIG_AMD_IOMMU_DEBUGFS
diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index 318b84cf47f6..a993e1bdb70b 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -124,7 +124,9 @@
 #define MMIO_STATUS_EVT_INT_MASK		BIT(1)
 #define MMIO_STATUS_COM_WAIT_INT_MASK		BIT(2)
 #define MMIO_STATUS_EVT_RUN_MASK		BIT(3)
+#define MMIO_STATUS_PPR_OVERFLOW_MASK		BIT(5)
 #define MMIO_STATUS_PPR_INT_MASK		BIT(6)
+#define MMIO_STATUS_PPR_RUN_MASK		BIT(7)
 #define MMIO_STATUS_GALOG_RUN_MASK		BIT(8)
 #define MMIO_STATUS_GALOG_OVERFLOW_MASK		BIT(9)
 #define MMIO_STATUS_GALOG_INT_MASK		BIT(10)
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 3c21e9333899..9908cd4f1c31 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -795,6 +795,17 @@ void amd_iommu_restart_ga_log(struct amd_iommu *iommu)
 			      MMIO_STATUS_GALOG_OVERFLOW_MASK);
 }
 
+/*
+ * This function restarts ppr logging in case the IOMMU experienced
+ * PPR log overflow.
+ */
+void amd_iommu_restart_ppr_log(struct amd_iommu *iommu)
+{
+	amd_iommu_restart_log(iommu, "PPR", CONTROL_PPRINT_EN,
+			      CONTROL_PPRLOG_EN, MMIO_STATUS_PPR_RUN_MASK,
+			      MMIO_STATUS_PPR_OVERFLOW_MASK);
+}
+
 /*
  * This function resets the command buffer if the IOMMU stopped fetching
  * commands from it.
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index a29548d98b6a..3c179d548ecd 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -844,6 +844,7 @@ amd_iommu_set_pci_msi_domain(struct device *dev, struct amd_iommu *iommu) { }
 #define AMD_IOMMU_INT_MASK	\
 	(MMIO_STATUS_EVT_OVERFLOW_MASK | \
 	 MMIO_STATUS_EVT_INT_MASK | \
+	 MMIO_STATUS_PPR_OVERFLOW_MASK | \
 	 MMIO_STATUS_PPR_INT_MASK | \
 	 MMIO_STATUS_GALOG_OVERFLOW_MASK | \
 	 MMIO_STATUS_GALOG_INT_MASK)
@@ -863,11 +864,17 @@ irqreturn_t amd_iommu_int_thread(int irq, void *data)
 			iommu_poll_events(iommu);
 		}
 
-		if (status & MMIO_STATUS_PPR_INT_MASK) {
+		if (status & (MMIO_STATUS_PPR_INT_MASK |
+			      MMIO_STATUS_PPR_OVERFLOW_MASK)) {
 			pr_devel("Processing IOMMU PPR Log\n");
 			iommu_poll_ppr_log(iommu);
 		}
 
+		if (status & MMIO_STATUS_PPR_OVERFLOW_MASK) {
+			pr_info_ratelimited("IOMMU PPR log overflow\n");
+			amd_iommu_restart_ppr_log(iommu);
+		}
+
 #ifdef CONFIG_IRQ_REMAP
 		if (status & (MMIO_STATUS_GALOG_INT_MASK |
 			      MMIO_STATUS_GALOG_OVERFLOW_MASK)) {
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] iommu/amd: Generalize log overflow handling
  2023-06-09 10:17 ` [PATCH 1/2] iommu/amd: Generalize log overflow handling Vasant Hegde
@ 2023-06-13 20:22   ` Jerry Snitselaar
  2023-06-20 14:51   ` Joao Martins
  1 sibling, 0 replies; 9+ messages in thread
From: Jerry Snitselaar @ 2023-06-13 20:22 UTC (permalink / raw)
  To: Vasant Hegde; +Cc: iommu, joro, suravee.suthikulpanit, joao.m.martins

On Fri, Jun 09, 2023 at 10:17:45AM +0000, Vasant Hegde wrote:
> IOMMU has three different logs (Event, GA and PPR log) and each uses
> different buffer for logging. Once buffer becomes full IOMMU generates
> overflow interrupt and we have to restart the logs. Log restart procedure
> is same for all three logs except it uses different bits in 'MMIO Offset
> 2020h IOMMU Status Register'. Hence lets move common code to generic
> function and individual log overflow handler will call generic function
> with appropriate parameters.
> 
> Also rename MMIO_STATUS_EVT_OVERFLOW_INT_MASK as
> MMIO_STATUS_EVT_OVERFLOW_MASK.
> 
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>

Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>

> ---
>  drivers/iommu/amd/amd_iommu_types.h |  3 +-
>  drivers/iommu/amd/init.c            | 51 ++++++++++++++++++-----------
>  drivers/iommu/amd/iommu.c           |  4 +--
>  3 files changed, 36 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 364fdaa52e74..318b84cf47f6 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -120,9 +120,10 @@
>  #define PASID_MASK		0x0000ffff
>  
>  /* MMIO status bits */
> -#define MMIO_STATUS_EVT_OVERFLOW_INT_MASK	BIT(0)
> +#define MMIO_STATUS_EVT_OVERFLOW_MASK		BIT(0)
>  #define MMIO_STATUS_EVT_INT_MASK		BIT(1)
>  #define MMIO_STATUS_COM_WAIT_INT_MASK		BIT(2)
> +#define MMIO_STATUS_EVT_RUN_MASK		BIT(3)
>  #define MMIO_STATUS_PPR_INT_MASK		BIT(6)
>  #define MMIO_STATUS_GALOG_RUN_MASK		BIT(8)
>  #define MMIO_STATUS_GALOG_OVERFLOW_MASK		BIT(9)
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index c2d80a4e5fb0..3c21e9333899 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -748,38 +748,51 @@ static int __init alloc_command_buffer(struct amd_iommu *iommu)
>  	return iommu->cmd_buf ? 0 : -ENOMEM;
>  }
>  
> +/*
> + * Interrupt handler has processed all pending events and adjusted head
> + * and tail pointer. Reset overflow mask and restart logging again.
> + */
> +static void amd_iommu_restart_log(struct amd_iommu *iommu, const char *evt_type,
> +				  u8 cntrl_intr, u8 cntrl_log,
> +				  u32 status_run_mask, u32 status_overflow_mask)
> +{
> +	u32 status;
> +
> +	status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
> +	if (status & status_run_mask)
> +		return;
> +
> +	pr_info_ratelimited("IOMMU %s log restarting\n", evt_type);
> +
> +	iommu_feature_disable(iommu, cntrl_log);
> +	iommu_feature_disable(iommu, cntrl_intr);
> +
> +	writel(status_overflow_mask, iommu->mmio_base + MMIO_STATUS_OFFSET);
> +
> +	iommu_feature_enable(iommu, cntrl_intr);
> +	iommu_feature_enable(iommu, cntrl_log);
> +}
> +
>  /*
>   * This function restarts event logging in case the IOMMU experienced
>   * an event log buffer overflow.
>   */
>  void amd_iommu_restart_event_logging(struct amd_iommu *iommu)
>  {
> -	iommu_feature_disable(iommu, CONTROL_EVT_LOG_EN);
> -	iommu_feature_enable(iommu, CONTROL_EVT_LOG_EN);
> +	amd_iommu_restart_log(iommu, "Event", CONTROL_EVT_INT_EN,
> +			      CONTROL_EVT_LOG_EN, MMIO_STATUS_EVT_RUN_MASK,
> +			      MMIO_STATUS_EVT_OVERFLOW_MASK);
>  }
>  
>  /*
>   * This function restarts event logging in case the IOMMU experienced
> - * an GA log overflow.
> + * GA log overflow.
>   */
>  void amd_iommu_restart_ga_log(struct amd_iommu *iommu)
>  {
> -	u32 status;
> -
> -	status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
> -	if (status & MMIO_STATUS_GALOG_RUN_MASK)
> -		return;
> -
> -	pr_info_ratelimited("IOMMU GA Log restarting\n");
> -
> -	iommu_feature_disable(iommu, CONTROL_GALOG_EN);
> -	iommu_feature_disable(iommu, CONTROL_GAINT_EN);
> -
> -	writel(MMIO_STATUS_GALOG_OVERFLOW_MASK,
> -	       iommu->mmio_base + MMIO_STATUS_OFFSET);
> -
> -	iommu_feature_enable(iommu, CONTROL_GAINT_EN);
> -	iommu_feature_enable(iommu, CONTROL_GALOG_EN);
> +	amd_iommu_restart_log(iommu, "GA", CONTROL_GAINT_EN,
> +			      CONTROL_GALOG_EN, MMIO_STATUS_GALOG_RUN_MASK,
> +			      MMIO_STATUS_GALOG_OVERFLOW_MASK);
>  }
>  
>  /*
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index ad501944d77c..a29548d98b6a 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -842,7 +842,7 @@ amd_iommu_set_pci_msi_domain(struct device *dev, struct amd_iommu *iommu) { }
>  #endif /* !CONFIG_IRQ_REMAP */
>  
>  #define AMD_IOMMU_INT_MASK	\
> -	(MMIO_STATUS_EVT_OVERFLOW_INT_MASK | \
> +	(MMIO_STATUS_EVT_OVERFLOW_MASK | \
>  	 MMIO_STATUS_EVT_INT_MASK | \
>  	 MMIO_STATUS_PPR_INT_MASK | \
>  	 MMIO_STATUS_GALOG_OVERFLOW_MASK | \
> @@ -881,7 +881,7 @@ irqreturn_t amd_iommu_int_thread(int irq, void *data)
>  		}
>  #endif
>  
> -		if (status & MMIO_STATUS_EVT_OVERFLOW_INT_MASK) {
> +		if (status & MMIO_STATUS_EVT_OVERFLOW_MASK) {
>  			pr_info_ratelimited("IOMMU event log overflow\n");
>  			amd_iommu_restart_event_logging(iommu);
>  		}
> -- 
> 2.31.1
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] iommu/amd: Handle PPR log overflow
  2023-06-09 10:17 ` [PATCH 2/2] iommu/amd: Handle PPR log overflow Vasant Hegde
@ 2023-06-13 20:29   ` Jerry Snitselaar
  2023-06-14  7:28     ` Vasant Hegde
  2023-06-20 14:53   ` Joao Martins
  1 sibling, 1 reply; 9+ messages in thread
From: Jerry Snitselaar @ 2023-06-13 20:29 UTC (permalink / raw)
  To: Vasant Hegde; +Cc: iommu, joro, suravee.suthikulpanit, joao.m.martins

On Fri, Jun 09, 2023 at 10:17:46AM +0000, Vasant Hegde wrote:
> Some ATS-capable peripherals can issue requests to the processor to service
> peripheral page requests using PCIe PRI (the Page Request Interface). IOMMU
> supports PRI using PPR log buffer. IOMMU writes PRI request to PPR log
> buffer and sends PPR interrupt to host. When there is no space in the
> PPR log buffer (PPR log overflow) it will set PprOverflow bit in 'MMIO
> Offset 2020h IOMMU Status Register'. When this happens PPR log needs to be
> restarted as specified in IOMMU spec [1] section 2.6.2.
> 
> When handling the event it just resumes the PPR log without resizing
> (similar to the way event and GA log overflow is handled).
> 
> Failing to handle PPR overflow means device may not work properly as
> IOMMU stops processing new PPR events from device.
> 
> [1] https://www.amd.com/system/files/TechDocs/48882_3.07_PUB.pdf
> 
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>

Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>

Will the code re-org you are doing move the event log overflow check
next to the event log processing check, like is done with the ga log
and ppr log?

Regards,
Jerry

> ---
>  drivers/iommu/amd/amd_iommu.h       |  1 +
>  drivers/iommu/amd/amd_iommu_types.h |  2 ++
>  drivers/iommu/amd/init.c            | 11 +++++++++++
>  drivers/iommu/amd/iommu.c           |  9 ++++++++-
>  4 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
> index 317d5c26c614..156f57b4f78c 100644
> --- a/drivers/iommu/amd/amd_iommu.h
> +++ b/drivers/iommu/amd/amd_iommu.h
> @@ -16,6 +16,7 @@ irqreturn_t amd_iommu_int_handler(int irq, void *data);
>  void amd_iommu_apply_erratum_63(struct amd_iommu *iommu, u16 devid);
>  void amd_iommu_restart_event_logging(struct amd_iommu *iommu);
>  void amd_iommu_restart_ga_log(struct amd_iommu *iommu);
> +void amd_iommu_restart_ppr_log(struct amd_iommu *iommu);
>  void amd_iommu_set_rlookup_table(struct amd_iommu *iommu, u16 devid);
>  
>  #ifdef CONFIG_AMD_IOMMU_DEBUGFS
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 318b84cf47f6..a993e1bdb70b 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -124,7 +124,9 @@
>  #define MMIO_STATUS_EVT_INT_MASK		BIT(1)
>  #define MMIO_STATUS_COM_WAIT_INT_MASK		BIT(2)
>  #define MMIO_STATUS_EVT_RUN_MASK		BIT(3)
> +#define MMIO_STATUS_PPR_OVERFLOW_MASK		BIT(5)
>  #define MMIO_STATUS_PPR_INT_MASK		BIT(6)
> +#define MMIO_STATUS_PPR_RUN_MASK		BIT(7)
>  #define MMIO_STATUS_GALOG_RUN_MASK		BIT(8)
>  #define MMIO_STATUS_GALOG_OVERFLOW_MASK		BIT(9)
>  #define MMIO_STATUS_GALOG_INT_MASK		BIT(10)
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 3c21e9333899..9908cd4f1c31 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -795,6 +795,17 @@ void amd_iommu_restart_ga_log(struct amd_iommu *iommu)
>  			      MMIO_STATUS_GALOG_OVERFLOW_MASK);
>  }
>  
> +/*
> + * This function restarts ppr logging in case the IOMMU experienced
> + * PPR log overflow.
> + */
> +void amd_iommu_restart_ppr_log(struct amd_iommu *iommu)
> +{
> +	amd_iommu_restart_log(iommu, "PPR", CONTROL_PPRINT_EN,
> +			      CONTROL_PPRLOG_EN, MMIO_STATUS_PPR_RUN_MASK,
> +			      MMIO_STATUS_PPR_OVERFLOW_MASK);
> +}
> +
>  /*
>   * This function resets the command buffer if the IOMMU stopped fetching
>   * commands from it.
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index a29548d98b6a..3c179d548ecd 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -844,6 +844,7 @@ amd_iommu_set_pci_msi_domain(struct device *dev, struct amd_iommu *iommu) { }
>  #define AMD_IOMMU_INT_MASK	\
>  	(MMIO_STATUS_EVT_OVERFLOW_MASK | \
>  	 MMIO_STATUS_EVT_INT_MASK | \
> +	 MMIO_STATUS_PPR_OVERFLOW_MASK | \
>  	 MMIO_STATUS_PPR_INT_MASK | \
>  	 MMIO_STATUS_GALOG_OVERFLOW_MASK | \
>  	 MMIO_STATUS_GALOG_INT_MASK)
> @@ -863,11 +864,17 @@ irqreturn_t amd_iommu_int_thread(int irq, void *data)
>  			iommu_poll_events(iommu);
>  		}
>  
> -		if (status & MMIO_STATUS_PPR_INT_MASK) {
> +		if (status & (MMIO_STATUS_PPR_INT_MASK |
> +			      MMIO_STATUS_PPR_OVERFLOW_MASK)) {
>  			pr_devel("Processing IOMMU PPR Log\n");
>  			iommu_poll_ppr_log(iommu);
>  		}
>  
> +		if (status & MMIO_STATUS_PPR_OVERFLOW_MASK) {
> +			pr_info_ratelimited("IOMMU PPR log overflow\n");
> +			amd_iommu_restart_ppr_log(iommu);
> +		}
> +
>  #ifdef CONFIG_IRQ_REMAP
>  		if (status & (MMIO_STATUS_GALOG_INT_MASK |
>  			      MMIO_STATUS_GALOG_OVERFLOW_MASK)) {
> -- 
> 2.31.1
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] iommu/amd: Handle PPR log overflow
  2023-06-13 20:29   ` Jerry Snitselaar
@ 2023-06-14  7:28     ` Vasant Hegde
  0 siblings, 0 replies; 9+ messages in thread
From: Vasant Hegde @ 2023-06-14  7:28 UTC (permalink / raw)
  To: Jerry Snitselaar; +Cc: iommu, joro, suravee.suthikulpanit, joao.m.martins

Hi Jerry,

On 6/14/2023 1:59 AM, Jerry Snitselaar wrote:
> On Fri, Jun 09, 2023 at 10:17:46AM +0000, Vasant Hegde wrote:
>> Some ATS-capable peripherals can issue requests to the processor to service
>> peripheral page requests using PCIe PRI (the Page Request Interface). IOMMU
>> supports PRI using PPR log buffer. IOMMU writes PRI request to PPR log
>> buffer and sends PPR interrupt to host. When there is no space in the
>> PPR log buffer (PPR log overflow) it will set PprOverflow bit in 'MMIO
>> Offset 2020h IOMMU Status Register'. When this happens PPR log needs to be
>> restarted as specified in IOMMU spec [1] section 2.6.2.
>>
>> When handling the event it just resumes the PPR log without resizing
>> (similar to the way event and GA log overflow is handled).
>>
>> Failing to handle PPR overflow means device may not work properly as
>> IOMMU stops processing new PPR events from device.
>>
>> [1] https://www.amd.com/system/files/TechDocs/48882_3.07_PUB.pdf
>>
>> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> 
> Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>

Thanks for the review.

> 
> Will the code re-org you are doing move the event log overflow check
> next to the event log processing check, like is done with the ga log
> and ppr log?

Yes. Its handled as part of IOMMU interrupt split series.

-Vasant


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] iommu/amd: Generalize log overflow handling
  2023-06-09 10:17 ` [PATCH 1/2] iommu/amd: Generalize log overflow handling Vasant Hegde
  2023-06-13 20:22   ` Jerry Snitselaar
@ 2023-06-20 14:51   ` Joao Martins
  2023-06-20 16:01     ` Vasant Hegde
  1 sibling, 1 reply; 9+ messages in thread
From: Joao Martins @ 2023-06-20 14:51 UTC (permalink / raw)
  To: Vasant Hegde; +Cc: suravee.suthikulpanit, iommu, joro



On 09/06/2023 11:17, Vasant Hegde wrote:
> IOMMU has three different logs (Event, GA and PPR log) and each uses
> different buffer for logging. Once buffer becomes full IOMMU generates
> overflow interrupt and we have to restart the logs. Log restart procedure
> is same for all three logs except it uses different bits in 'MMIO Offset
> 2020h IOMMU Status Register'. Hence lets move common code to generic
> function and individual log overflow handler will call generic function
> with appropriate parameters.
> 
> Also rename MMIO_STATUS_EVT_OVERFLOW_INT_MASK as
> MMIO_STATUS_EVT_OVERFLOW_MASK.
> 
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>

Really nice rework, fwiw:

	Reviewed-by: Joao Martins <joao.m.martins@oracle.com>

Just a though below (you don't need to address it, just an idea)

> ---
>  drivers/iommu/amd/amd_iommu_types.h |  3 +-
>  drivers/iommu/amd/init.c            | 51 ++++++++++++++++++-----------
>  drivers/iommu/amd/iommu.c           |  4 +--
>  3 files changed, 36 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 364fdaa52e74..318b84cf47f6 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -120,9 +120,10 @@
>  #define PASID_MASK		0x0000ffff
>  
>  /* MMIO status bits */
> -#define MMIO_STATUS_EVT_OVERFLOW_INT_MASK	BIT(0)
> +#define MMIO_STATUS_EVT_OVERFLOW_MASK		BIT(0)
>  #define MMIO_STATUS_EVT_INT_MASK		BIT(1)
>  #define MMIO_STATUS_COM_WAIT_INT_MASK		BIT(2)
> +#define MMIO_STATUS_EVT_RUN_MASK		BIT(3)
>  #define MMIO_STATUS_PPR_INT_MASK		BIT(6)
>  #define MMIO_STATUS_GALOG_RUN_MASK		BIT(8)
>  #define MMIO_STATUS_GALOG_OVERFLOW_MASK		BIT(9)
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index c2d80a4e5fb0..3c21e9333899 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -748,38 +748,51 @@ static int __init alloc_command_buffer(struct amd_iommu *iommu)
>  	return iommu->cmd_buf ? 0 : -ENOMEM;
>  }
>  
> +/*
> + * Interrupt handler has processed all pending events and adjusted head
> + * and tail pointer. Reset overflow mask and restart logging again.
> + */
> +static void amd_iommu_restart_log(struct amd_iommu *iommu, const char *evt_type,
> +				  u8 cntrl_intr, u8 cntrl_log,
> +				  u32 status_run_mask, u32 status_overflow_mask)
> +{
> +	u32 status;
> +
> +	status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
> +	if (status & status_run_mask)
> +		return;
> +
> +	pr_info_ratelimited("IOMMU %s log restarting\n", evt_type);
> +
> +	iommu_feature_disable(iommu, cntrl_log);
> +	iommu_feature_disable(iommu, cntrl_intr);
> +
> +	writel(status_overflow_mask, iommu->mmio_base + MMIO_STATUS_OFFSET);
> +
> +	iommu_feature_enable(iommu, cntrl_intr);
> +	iommu_feature_enable(iommu, cntrl_log);
> +}
> +
>  /*
>   * This function restarts event logging in case the IOMMU experienced
>   * an event log buffer overflow.
>   */
>  void amd_iommu_restart_event_logging(struct amd_iommu *iommu)
>  {
> -	iommu_feature_disable(iommu, CONTROL_EVT_LOG_EN);
> -	iommu_feature_enable(iommu, CONTROL_EVT_LOG_EN);
> +	amd_iommu_restart_log(iommu, "Event", CONTROL_EVT_INT_EN,
> +			      CONTROL_EVT_LOG_EN, MMIO_STATUS_EVT_RUN_MASK,
> +			      MMIO_STATUS_EVT_OVERFLOW_MASK);
>  }
>  

If CONTROL_EVT_{INT,LOG} was instead EVT{INT,LOG} (like it's PPR and GA
counterparts), I do wonder if a macro is better placed here considering that the
only think that changes is the string name and whether it's EVT, PPR or GA is
used. But perhaps that might be too obscure.

>  /*
>   * This function restarts event logging in case the IOMMU experienced
> - * an GA log overflow.
> + * GA log overflow.
>   */
>  void amd_iommu_restart_ga_log(struct amd_iommu *iommu)
>  {
> -	u32 status;
> -
> -	status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
> -	if (status & MMIO_STATUS_GALOG_RUN_MASK)
> -		return;
> -
> -	pr_info_ratelimited("IOMMU GA Log restarting\n");
> -
> -	iommu_feature_disable(iommu, CONTROL_GALOG_EN);
> -	iommu_feature_disable(iommu, CONTROL_GAINT_EN);
> -
> -	writel(MMIO_STATUS_GALOG_OVERFLOW_MASK,
> -	       iommu->mmio_base + MMIO_STATUS_OFFSET);
> -
> -	iommu_feature_enable(iommu, CONTROL_GAINT_EN);
> -	iommu_feature_enable(iommu, CONTROL_GALOG_EN);
> +	amd_iommu_restart_log(iommu, "GA", CONTROL_GAINT_EN,
> +			      CONTROL_GALOG_EN, MMIO_STATUS_GALOG_RUN_MASK,
> +			      MMIO_STATUS_GALOG_OVERFLOW_MASK);
>  }
>  
>  /*
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index ad501944d77c..a29548d98b6a 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -842,7 +842,7 @@ amd_iommu_set_pci_msi_domain(struct device *dev, struct amd_iommu *iommu) { }
>  #endif /* !CONFIG_IRQ_REMAP */
>  
>  #define AMD_IOMMU_INT_MASK	\
> -	(MMIO_STATUS_EVT_OVERFLOW_INT_MASK | \
> +	(MMIO_STATUS_EVT_OVERFLOW_MASK | \
>  	 MMIO_STATUS_EVT_INT_MASK | \
>  	 MMIO_STATUS_PPR_INT_MASK | \
>  	 MMIO_STATUS_GALOG_OVERFLOW_MASK | \
> @@ -881,7 +881,7 @@ irqreturn_t amd_iommu_int_thread(int irq, void *data)
>  		}
>  #endif
>  
> -		if (status & MMIO_STATUS_EVT_OVERFLOW_INT_MASK) {
> +		if (status & MMIO_STATUS_EVT_OVERFLOW_MASK) {
>  			pr_info_ratelimited("IOMMU event log overflow\n");
>  			amd_iommu_restart_event_logging(iommu);
>  		}

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] iommu/amd: Handle PPR log overflow
  2023-06-09 10:17 ` [PATCH 2/2] iommu/amd: Handle PPR log overflow Vasant Hegde
  2023-06-13 20:29   ` Jerry Snitselaar
@ 2023-06-20 14:53   ` Joao Martins
  1 sibling, 0 replies; 9+ messages in thread
From: Joao Martins @ 2023-06-20 14:53 UTC (permalink / raw)
  To: Vasant Hegde; +Cc: suravee.suthikulpanit, iommu, joro

On 09/06/2023 11:17, Vasant Hegde wrote:
> Some ATS-capable peripherals can issue requests to the processor to service
> peripheral page requests using PCIe PRI (the Page Request Interface). IOMMU
> supports PRI using PPR log buffer. IOMMU writes PRI request to PPR log
> buffer and sends PPR interrupt to host. When there is no space in the
> PPR log buffer (PPR log overflow) it will set PprOverflow bit in 'MMIO
> Offset 2020h IOMMU Status Register'. When this happens PPR log needs to be
> restarted as specified in IOMMU spec [1] section 2.6.2.
> 
> When handling the event it just resumes the PPR log without resizing
> (similar to the way event and GA log overflow is handled).
> 
> Failing to handle PPR overflow means device may not work properly as
> IOMMU stops processing new PPR events from device.
> 
> [1] https://www.amd.com/system/files/TechDocs/48882_3.07_PUB.pdf
> 
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>

Reviewed-by: Joao Martins <joao.m.martins@oracle.com>

> ---
>  drivers/iommu/amd/amd_iommu.h       |  1 +
>  drivers/iommu/amd/amd_iommu_types.h |  2 ++
>  drivers/iommu/amd/init.c            | 11 +++++++++++
>  drivers/iommu/amd/iommu.c           |  9 ++++++++-
>  4 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
> index 317d5c26c614..156f57b4f78c 100644
> --- a/drivers/iommu/amd/amd_iommu.h
> +++ b/drivers/iommu/amd/amd_iommu.h
> @@ -16,6 +16,7 @@ irqreturn_t amd_iommu_int_handler(int irq, void *data);
>  void amd_iommu_apply_erratum_63(struct amd_iommu *iommu, u16 devid);
>  void amd_iommu_restart_event_logging(struct amd_iommu *iommu);
>  void amd_iommu_restart_ga_log(struct amd_iommu *iommu);
> +void amd_iommu_restart_ppr_log(struct amd_iommu *iommu);
>  void amd_iommu_set_rlookup_table(struct amd_iommu *iommu, u16 devid);
>  
>  #ifdef CONFIG_AMD_IOMMU_DEBUGFS
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 318b84cf47f6..a993e1bdb70b 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -124,7 +124,9 @@
>  #define MMIO_STATUS_EVT_INT_MASK		BIT(1)
>  #define MMIO_STATUS_COM_WAIT_INT_MASK		BIT(2)
>  #define MMIO_STATUS_EVT_RUN_MASK		BIT(3)
> +#define MMIO_STATUS_PPR_OVERFLOW_MASK		BIT(5)
>  #define MMIO_STATUS_PPR_INT_MASK		BIT(6)
> +#define MMIO_STATUS_PPR_RUN_MASK		BIT(7)
>  #define MMIO_STATUS_GALOG_RUN_MASK		BIT(8)
>  #define MMIO_STATUS_GALOG_OVERFLOW_MASK		BIT(9)
>  #define MMIO_STATUS_GALOG_INT_MASK		BIT(10)
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 3c21e9333899..9908cd4f1c31 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -795,6 +795,17 @@ void amd_iommu_restart_ga_log(struct amd_iommu *iommu)
>  			      MMIO_STATUS_GALOG_OVERFLOW_MASK);
>  }
>  
> +/*
> + * This function restarts ppr logging in case the IOMMU experienced
> + * PPR log overflow.
> + */
> +void amd_iommu_restart_ppr_log(struct amd_iommu *iommu)
> +{
> +	amd_iommu_restart_log(iommu, "PPR", CONTROL_PPRINT_EN,
> +			      CONTROL_PPRLOG_EN, MMIO_STATUS_PPR_RUN_MASK,
> +			      MMIO_STATUS_PPR_OVERFLOW_MASK);
> +}
> +
>  /*
>   * This function resets the command buffer if the IOMMU stopped fetching
>   * commands from it.
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index a29548d98b6a..3c179d548ecd 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -844,6 +844,7 @@ amd_iommu_set_pci_msi_domain(struct device *dev, struct amd_iommu *iommu) { }
>  #define AMD_IOMMU_INT_MASK	\
>  	(MMIO_STATUS_EVT_OVERFLOW_MASK | \
>  	 MMIO_STATUS_EVT_INT_MASK | \
> +	 MMIO_STATUS_PPR_OVERFLOW_MASK | \
>  	 MMIO_STATUS_PPR_INT_MASK | \
>  	 MMIO_STATUS_GALOG_OVERFLOW_MASK | \
>  	 MMIO_STATUS_GALOG_INT_MASK)
> @@ -863,11 +864,17 @@ irqreturn_t amd_iommu_int_thread(int irq, void *data)
>  			iommu_poll_events(iommu);
>  		}
>  
> -		if (status & MMIO_STATUS_PPR_INT_MASK) {
> +		if (status & (MMIO_STATUS_PPR_INT_MASK |
> +			      MMIO_STATUS_PPR_OVERFLOW_MASK)) {
>  			pr_devel("Processing IOMMU PPR Log\n");
>  			iommu_poll_ppr_log(iommu);
>  		}
>  
> +		if (status & MMIO_STATUS_PPR_OVERFLOW_MASK) {
> +			pr_info_ratelimited("IOMMU PPR log overflow\n");
> +			amd_iommu_restart_ppr_log(iommu);
> +		}
> +
>  #ifdef CONFIG_IRQ_REMAP
>  		if (status & (MMIO_STATUS_GALOG_INT_MASK |
>  			      MMIO_STATUS_GALOG_OVERFLOW_MASK)) {

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] iommu/amd: Generalize log overflow handling
  2023-06-20 14:51   ` Joao Martins
@ 2023-06-20 16:01     ` Vasant Hegde
  0 siblings, 0 replies; 9+ messages in thread
From: Vasant Hegde @ 2023-06-20 16:01 UTC (permalink / raw)
  To: Joao Martins; +Cc: suravee.suthikulpanit, iommu, joro

Hi Joao,


On 6/20/2023 8:21 PM, Joao Martins wrote:
> 
> 
> On 09/06/2023 11:17, Vasant Hegde wrote:
>> IOMMU has three different logs (Event, GA and PPR log) and each uses
>> different buffer for logging. Once buffer becomes full IOMMU generates
>> overflow interrupt and we have to restart the logs. Log restart procedure
>> is same for all three logs except it uses different bits in 'MMIO Offset
>> 2020h IOMMU Status Register'. Hence lets move common code to generic
>> function and individual log overflow handler will call generic function
>> with appropriate parameters.
>>
>> Also rename MMIO_STATUS_EVT_OVERFLOW_INT_MASK as
>> MMIO_STATUS_EVT_OVERFLOW_MASK.
>>
>> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> 
> Really nice rework, fwiw:
> 
> 	Reviewed-by: Joao Martins <joao.m.martins@oracle.com>

I did post V2 version, but forgot to CC you. Sorry for that.
Can you please look into V2?

V2 ->
https://lore.kernel.org/linux-iommu/20230619132346.6021-1-vasant.hegde@amd.com/



> Just a though below (you don't need to address it, just an idea)
> 
>> ---
>>  drivers/iommu/amd/amd_iommu_types.h |  3 +-
>>  drivers/iommu/amd/init.c            | 51 ++++++++++++++++++-----------
>>  drivers/iommu/amd/iommu.c           |  4 +--
>>  3 files changed, 36 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
>> index 364fdaa52e74..318b84cf47f6 100644
>> --- a/drivers/iommu/amd/amd_iommu_types.h
>> +++ b/drivers/iommu/amd/amd_iommu_types.h
>> @@ -120,9 +120,10 @@
>>  #define PASID_MASK		0x0000ffff
>>  
>>  /* MMIO status bits */
>> -#define MMIO_STATUS_EVT_OVERFLOW_INT_MASK	BIT(0)
>> +#define MMIO_STATUS_EVT_OVERFLOW_MASK		BIT(0)
>>  #define MMIO_STATUS_EVT_INT_MASK		BIT(1)
>>  #define MMIO_STATUS_COM_WAIT_INT_MASK		BIT(2)
>> +#define MMIO_STATUS_EVT_RUN_MASK		BIT(3)
>>  #define MMIO_STATUS_PPR_INT_MASK		BIT(6)
>>  #define MMIO_STATUS_GALOG_RUN_MASK		BIT(8)
>>  #define MMIO_STATUS_GALOG_OVERFLOW_MASK		BIT(9)
>> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
>> index c2d80a4e5fb0..3c21e9333899 100644
>> --- a/drivers/iommu/amd/init.c
>> +++ b/drivers/iommu/amd/init.c
>> @@ -748,38 +748,51 @@ static int __init alloc_command_buffer(struct amd_iommu *iommu)
>>  	return iommu->cmd_buf ? 0 : -ENOMEM;
>>  }
>>  
>> +/*
>> + * Interrupt handler has processed all pending events and adjusted head
>> + * and tail pointer. Reset overflow mask and restart logging again.
>> + */
>> +static void amd_iommu_restart_log(struct amd_iommu *iommu, const char *evt_type,
>> +				  u8 cntrl_intr, u8 cntrl_log,
>> +				  u32 status_run_mask, u32 status_overflow_mask)
>> +{
>> +	u32 status;
>> +
>> +	status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
>> +	if (status & status_run_mask)
>> +		return;
>> +
>> +	pr_info_ratelimited("IOMMU %s log restarting\n", evt_type);
>> +
>> +	iommu_feature_disable(iommu, cntrl_log);
>> +	iommu_feature_disable(iommu, cntrl_intr);
>> +
>> +	writel(status_overflow_mask, iommu->mmio_base + MMIO_STATUS_OFFSET);
>> +
>> +	iommu_feature_enable(iommu, cntrl_intr);
>> +	iommu_feature_enable(iommu, cntrl_log);
>> +}
>> +
>>  /*
>>   * This function restarts event logging in case the IOMMU experienced
>>   * an event log buffer overflow.
>>   */
>>  void amd_iommu_restart_event_logging(struct amd_iommu *iommu)
>>  {
>> -	iommu_feature_disable(iommu, CONTROL_EVT_LOG_EN);
>> -	iommu_feature_enable(iommu, CONTROL_EVT_LOG_EN);
>> +	amd_iommu_restart_log(iommu, "Event", CONTROL_EVT_INT_EN,
>> +			      CONTROL_EVT_LOG_EN, MMIO_STATUS_EVT_RUN_MASK,
>> +			      MMIO_STATUS_EVT_OVERFLOW_MASK);
>>  }
>>  
> 
> If CONTROL_EVT_{INT,LOG} was instead EVT{INT,LOG} (like it's PPR and GA
> counterparts), I do wonder if a macro is better placed here considering that the
> only think that changes is the string name and whether it's EVT, PPR or GA is
> used. But perhaps that might be too obscure.

Sure. I am touching some of the init code in next series. I will try to fix it
as part of that.

-Vasant


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-06-20 16:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-09 10:17 [PATCH 0/2] iommu/amd: Add PPR overflow support Vasant Hegde
2023-06-09 10:17 ` [PATCH 1/2] iommu/amd: Generalize log overflow handling Vasant Hegde
2023-06-13 20:22   ` Jerry Snitselaar
2023-06-20 14:51   ` Joao Martins
2023-06-20 16:01     ` Vasant Hegde
2023-06-09 10:17 ` [PATCH 2/2] iommu/amd: Handle PPR log overflow Vasant Hegde
2023-06-13 20:29   ` Jerry Snitselaar
2023-06-14  7:28     ` Vasant Hegde
2023-06-20 14:53   ` Joao Martins

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).