* [PATCH v3 0/2] iommu/amd: Interrupt handling improvements
@ 2023-06-28 5:32 Vasant Hegde
2023-06-28 5:32 ` [PATCH v3 1/2] iommu/amd: Refactor IOMMU interrupt handling logic for Event, PPR, and GA logs Vasant Hegde
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Vasant Hegde @ 2023-06-28 5:32 UTC (permalink / raw
To: iommu, joro; +Cc: suravee.suthikulpanit, joao.m.martins, jsnitsel, Vasant Hegde
AMD IOMMU has three different logs/interrupts (Event, PPR and GA).
Currently, the IOMMU driver consolidates interrupts for all three logs to
a single interrupt. This could become a bottleneck when enable GA and PPR
logs simultaneously. Hence separate these interrupt to prevent potential
performance issue.
This series applies on top of PPR overflow support patchset [1].
[1] https://lore.kernel.org/linux-iommu/20230628051624.5792-1-vasant.hegde@amd.com/T/#t
Changes from v2 -> v3:
- Moved log message to amd_iommu_handle_irq() - Thanks Joao
V2 : https://lore.kernel.org/linux-iommu/20230619133008.6221-1-vasant.hegde@amd.com/
Changes from v1 -> v2:
- Moved GA log interrupt under CONFIG_IRQ_REMAP
- Updated patch description
V1 : https://lore.kernel.org/linux-iommu/20230609102025.6498-1-vasant.hegde@amd.com/
Vasant Hegde (2):
iommu/amd: Refactor IOMMU interrupt handling logic for Event, PPR, and GA logs
iommu/amd: Enable separate interrupt for PPR and GA log
drivers/iommu/amd/amd_iommu.h | 3 +
drivers/iommu/amd/amd_iommu_types.h | 9 +++
drivers/iommu/amd/init.c | 50 ++++++++++++----
drivers/iommu/amd/iommu.c | 93 ++++++++++++++++-------------
4 files changed, 100 insertions(+), 55 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/2] iommu/amd: Refactor IOMMU interrupt handling logic for Event, PPR, and GA logs
2023-06-28 5:32 [PATCH v3 0/2] iommu/amd: Interrupt handling improvements Vasant Hegde
@ 2023-06-28 5:32 ` Vasant Hegde
2023-06-29 16:36 ` Jerry Snitselaar
2023-06-28 5:32 ` [PATCH v3 2/2] iommu/amd: Enable separate interrupt for PPR and GA log Vasant Hegde
2023-07-14 14:20 ` [PATCH v3 0/2] iommu/amd: Interrupt handling improvements Joerg Roedel
2 siblings, 1 reply; 6+ messages in thread
From: Vasant Hegde @ 2023-06-28 5:32 UTC (permalink / raw
To: iommu, joro; +Cc: suravee.suthikulpanit, joao.m.martins, jsnitsel, Vasant Hegde
The AMD IOMMU has three log buffers (i.e. Event, PPR, and GA). The IOMMU
driver processes these log entries when it receive an IOMMU interrupt.
Then, it needs to clear the corresponding interrupt status bits. Also, when
an overflow occurs, it needs to handle the log overflow by clearing the
specific overflow status bit and restart the log.
Since, logic for handling these logs is the same, refactor the code into a
helper function called amd_iommu_handle_irq(), which handles the steps
described. Then, reuse it for all types of log.
Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Vasant Hegde<vasant.hegde@amd.com>
---
drivers/iommu/amd/amd_iommu.h | 3 ++
drivers/iommu/amd/iommu.c | 93 +++++++++++++++++++----------------
2 files changed, 53 insertions(+), 43 deletions(-)
diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index 8c61c19dabc4..f23ad6043f04 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -12,6 +12,9 @@
#include "amd_iommu_types.h"
irqreturn_t amd_iommu_int_thread(int irq, void *data);
+irqreturn_t amd_iommu_int_thread_evtlog(int irq, void *data);
+irqreturn_t amd_iommu_int_thread_pprlog(int irq, void *data);
+irqreturn_t amd_iommu_int_thread_galog(int irq, void *data);
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);
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 8f299bb0f3ed..5e9c287b10e0 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -841,57 +841,27 @@ static inline void
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_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)
-
-irqreturn_t amd_iommu_int_thread(int irq, void *data)
+static void amd_iommu_handle_irq(void *data, const char *evt_type,
+ u32 int_mask, u32 overflow_mask,
+ void (*int_handler)(struct amd_iommu *),
+ void (*overflow_handler)(struct amd_iommu *))
{
struct amd_iommu *iommu = (struct amd_iommu *) data;
u32 status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
+ u32 mask = int_mask | overflow_mask;
- while (status & AMD_IOMMU_INT_MASK) {
+ while (status & mask) {
/* Enable interrupt sources again */
- writel(AMD_IOMMU_INT_MASK,
- iommu->mmio_base + MMIO_STATUS_OFFSET);
-
- if (status & MMIO_STATUS_EVT_INT_MASK) {
- pr_devel("Processing IOMMU Event Log\n");
- iommu_poll_events(iommu);
- }
-
- 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)) {
- pr_devel("Processing IOMMU GA Log\n");
- iommu_poll_ga_log(iommu);
- }
+ writel(mask, iommu->mmio_base + MMIO_STATUS_OFFSET);
- if (status & MMIO_STATUS_GALOG_OVERFLOW_MASK) {
- pr_info_ratelimited("IOMMU GA Log overflow\n");
- amd_iommu_restart_ga_log(iommu);
+ if (int_handler) {
+ pr_devel("Processing IOMMU (ivhd%d) %s Log\n",
+ iommu->index, evt_type);
+ int_handler(iommu);
}
-#endif
- if (status & MMIO_STATUS_EVT_OVERFLOW_MASK) {
- pr_info_ratelimited("IOMMU event log overflow\n");
- amd_iommu_restart_event_logging(iommu);
- }
+ if ((status & overflow_mask) && overflow_handler)
+ overflow_handler(iommu);
/*
* Hardware bug: ERBT1312
@@ -908,6 +878,43 @@ irqreturn_t amd_iommu_int_thread(int irq, void *data)
*/
status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
}
+}
+
+irqreturn_t amd_iommu_int_thread_evtlog(int irq, void *data)
+{
+ amd_iommu_handle_irq(data, "Evt", MMIO_STATUS_EVT_INT_MASK,
+ MMIO_STATUS_EVT_OVERFLOW_MASK,
+ iommu_poll_events, amd_iommu_restart_event_logging);
+
+ return IRQ_HANDLED;
+}
+
+irqreturn_t amd_iommu_int_thread_pprlog(int irq, void *data)
+{
+ amd_iommu_handle_irq(data, "PPR", MMIO_STATUS_PPR_INT_MASK,
+ MMIO_STATUS_PPR_OVERFLOW_MASK,
+ iommu_poll_ppr_log, amd_iommu_restart_ppr_log);
+
+ return IRQ_HANDLED;
+}
+
+irqreturn_t amd_iommu_int_thread_galog(int irq, void *data)
+{
+#ifdef CONFIG_IRQ_REMAP
+ amd_iommu_handle_irq(data, "GA", MMIO_STATUS_GALOG_INT_MASK,
+ MMIO_STATUS_GALOG_OVERFLOW_MASK,
+ iommu_poll_ga_log, amd_iommu_restart_ga_log);
+#endif
+
+ return IRQ_HANDLED;
+}
+
+irqreturn_t amd_iommu_int_thread(int irq, void *data)
+{
+ amd_iommu_int_thread_evtlog(irq, data);
+ amd_iommu_int_thread_pprlog(irq, data);
+ amd_iommu_int_thread_galog(irq, data);
+
return IRQ_HANDLED;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] iommu/amd: Enable separate interrupt for PPR and GA log
2023-06-28 5:32 [PATCH v3 0/2] iommu/amd: Interrupt handling improvements Vasant Hegde
2023-06-28 5:32 ` [PATCH v3 1/2] iommu/amd: Refactor IOMMU interrupt handling logic for Event, PPR, and GA logs Vasant Hegde
@ 2023-06-28 5:32 ` Vasant Hegde
2023-06-29 16:35 ` Jerry Snitselaar
2023-07-14 14:20 ` [PATCH v3 0/2] iommu/amd: Interrupt handling improvements Joerg Roedel
2 siblings, 1 reply; 6+ messages in thread
From: Vasant Hegde @ 2023-06-28 5:32 UTC (permalink / raw
To: iommu, joro
Cc: suravee.suthikulpanit, joao.m.martins, jsnitsel, Vasant Hegde,
Alexey Kardashevskiy
AMD IOMMU has three log buffers (i.e. Event, PPR, and GA). These logs can
be configured to generate different interrupts when an entry is inserted
into a log buffer.
However, current implementation share single interrupt to handle all three
logs. With increasing usages of the GA (for IOMMU AVIC) and PPR logs (for
IOMMUv2 APIs and SVA), interrupt sharing could potentially become
performance bottleneck.
Hence, separate IOMMU interrupt into use three separate vectors and irq
threads with corresponding name, which will be displayed in the
/proc/interrupts as "AMD-Vi<x>-[Evt/PPR/GA]", where "x" is an IOMMU id.
Note that this patch changes interrupt handling only in IOMMU x2apic mode
(MMIO 0x18[IntCapXTEn]=1). In legacy mode it will continue to use single
MSI interrupt.
Signed-off-by: Vasant Hegde<vasant.hegde@amd.com>
Reviewed-by: Alexey Kardashevskiy<aik@amd.com>
---
drivers/iommu/amd/amd_iommu_types.h | 9 ++++++
drivers/iommu/amd/init.c | 50 ++++++++++++++++++++++-------
2 files changed, 47 insertions(+), 12 deletions(-)
diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index 2266badc6d0a..7011b746d38e 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -705,12 +705,21 @@ struct amd_iommu {
/* event buffer virtual address */
u8 *evt_buf;
+ /* Name for event log interrupt */
+ unsigned char evt_irq_name[16];
+
/* Base of the PPR log, if present */
u8 *ppr_log;
+ /* Name for PPR log interrupt */
+ unsigned char ppr_irq_name[16];
+
/* Base of the GA log, if present */
u8 *ga_log;
+ /* Name for GA log interrupt */
+ unsigned char ga_irq_name[16];
+
/* Tail of the GA log, if present */
u8 *ga_log_tail;
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index e78d7c4f41bd..30f09e786f7b 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -2335,6 +2335,7 @@ static int intcapxt_irqdomain_alloc(struct irq_domain *domain, unsigned int virq
struct irq_data *irqd = irq_domain_get_irq_data(domain, i);
irqd->chip = &intcapxt_controller;
+ irqd->hwirq = info->hwirq;
irqd->chip_data = info->data;
__irq_set_handler(i, handle_edge_irq, 0, "edge");
}
@@ -2361,22 +2362,14 @@ static void intcapxt_unmask_irq(struct irq_data *irqd)
xt.destid_0_23 = cfg->dest_apicid & GENMASK(23, 0);
xt.destid_24_31 = cfg->dest_apicid >> 24;
- /**
- * Current IOMMU implementation uses the same IRQ for all
- * 3 IOMMU interrupts.
- */
- writeq(xt.capxt, iommu->mmio_base + MMIO_INTCAPXT_EVT_OFFSET);
- writeq(xt.capxt, iommu->mmio_base + MMIO_INTCAPXT_PPR_OFFSET);
- writeq(xt.capxt, iommu->mmio_base + MMIO_INTCAPXT_GALOG_OFFSET);
+ writeq(xt.capxt, iommu->mmio_base + irqd->hwirq);
}
static void intcapxt_mask_irq(struct irq_data *irqd)
{
struct amd_iommu *iommu = irqd->chip_data;
- writeq(0, iommu->mmio_base + MMIO_INTCAPXT_EVT_OFFSET);
- writeq(0, iommu->mmio_base + MMIO_INTCAPXT_PPR_OFFSET);
- writeq(0, iommu->mmio_base + MMIO_INTCAPXT_GALOG_OFFSET);
+ writeq(0, iommu->mmio_base + irqd->hwirq);
}
@@ -2439,7 +2432,8 @@ static struct irq_domain *iommu_get_irqdomain(void)
return iommu_irqdomain;
}
-static int iommu_setup_intcapxt(struct amd_iommu *iommu)
+static int __iommu_setup_intcapxt(struct amd_iommu *iommu, const char *devname,
+ int hwirq, irq_handler_t thread_fn)
{
struct irq_domain *domain;
struct irq_alloc_info info;
@@ -2453,6 +2447,7 @@ static int iommu_setup_intcapxt(struct amd_iommu *iommu)
init_irq_alloc_info(&info, NULL);
info.type = X86_IRQ_ALLOC_TYPE_AMDVI;
info.data = iommu;
+ info.hwirq = hwirq;
irq = irq_domain_alloc_irqs(domain, 1, node, &info);
if (irq < 0) {
@@ -2461,7 +2456,7 @@ static int iommu_setup_intcapxt(struct amd_iommu *iommu)
}
ret = request_threaded_irq(irq, amd_iommu_int_handler,
- amd_iommu_int_thread, 0, "AMD-Vi", iommu);
+ thread_fn, 0, devname, iommu);
if (ret) {
irq_domain_free_irqs(irq, 1);
irq_domain_remove(domain);
@@ -2471,6 +2466,37 @@ static int iommu_setup_intcapxt(struct amd_iommu *iommu)
return 0;
}
+static int iommu_setup_intcapxt(struct amd_iommu *iommu)
+{
+ int ret;
+
+ snprintf(iommu->evt_irq_name, sizeof(iommu->evt_irq_name),
+ "AMD-Vi%d-Evt", iommu->index);
+ ret = __iommu_setup_intcapxt(iommu, iommu->evt_irq_name,
+ MMIO_INTCAPXT_EVT_OFFSET,
+ amd_iommu_int_thread_evtlog);
+ if (ret)
+ return ret;
+
+ snprintf(iommu->ppr_irq_name, sizeof(iommu->ppr_irq_name),
+ "AMD-Vi%d-PPR", iommu->index);
+ ret = __iommu_setup_intcapxt(iommu, iommu->ppr_irq_name,
+ MMIO_INTCAPXT_PPR_OFFSET,
+ amd_iommu_int_thread_pprlog);
+ if (ret)
+ return ret;
+
+#ifdef CONFIG_IRQ_REMAP
+ snprintf(iommu->ga_irq_name, sizeof(iommu->ga_irq_name),
+ "AMD-Vi%d-GA", iommu->index);
+ ret = __iommu_setup_intcapxt(iommu, iommu->ga_irq_name,
+ MMIO_INTCAPXT_GALOG_OFFSET,
+ amd_iommu_int_thread_galog);
+#endif
+
+ return ret;
+}
+
static int iommu_init_irq(struct amd_iommu *iommu)
{
int ret;
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] iommu/amd: Enable separate interrupt for PPR and GA log
2023-06-28 5:32 ` [PATCH v3 2/2] iommu/amd: Enable separate interrupt for PPR and GA log Vasant Hegde
@ 2023-06-29 16:35 ` Jerry Snitselaar
0 siblings, 0 replies; 6+ messages in thread
From: Jerry Snitselaar @ 2023-06-29 16:35 UTC (permalink / raw
To: Vasant Hegde
Cc: iommu, joro, suravee.suthikulpanit, joao.m.martins,
Alexey Kardashevskiy
On Wed, Jun 28, 2023 at 05:32:22AM +0000, Vasant Hegde wrote:
> AMD IOMMU has three log buffers (i.e. Event, PPR, and GA). These logs can
> be configured to generate different interrupts when an entry is inserted
> into a log buffer.
>
> However, current implementation share single interrupt to handle all three
> logs. With increasing usages of the GA (for IOMMU AVIC) and PPR logs (for
> IOMMUv2 APIs and SVA), interrupt sharing could potentially become
> performance bottleneck.
>
> Hence, separate IOMMU interrupt into use three separate vectors and irq
> threads with corresponding name, which will be displayed in the
> /proc/interrupts as "AMD-Vi<x>-[Evt/PPR/GA]", where "x" is an IOMMU id.
>
> Note that this patch changes interrupt handling only in IOMMU x2apic mode
> (MMIO 0x18[IntCapXTEn]=1). In legacy mode it will continue to use single
> MSI interrupt.
>
> Signed-off-by: Vasant Hegde<vasant.hegde@amd.com>
> Reviewed-by: Alexey Kardashevskiy<aik@amd.com>
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
> ---
> drivers/iommu/amd/amd_iommu_types.h | 9 ++++++
> drivers/iommu/amd/init.c | 50 ++++++++++++++++++++++-------
> 2 files changed, 47 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 2266badc6d0a..7011b746d38e 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -705,12 +705,21 @@ struct amd_iommu {
> /* event buffer virtual address */
> u8 *evt_buf;
>
> + /* Name for event log interrupt */
> + unsigned char evt_irq_name[16];
> +
> /* Base of the PPR log, if present */
> u8 *ppr_log;
>
> + /* Name for PPR log interrupt */
> + unsigned char ppr_irq_name[16];
> +
> /* Base of the GA log, if present */
> u8 *ga_log;
>
> + /* Name for GA log interrupt */
> + unsigned char ga_irq_name[16];
> +
> /* Tail of the GA log, if present */
> u8 *ga_log_tail;
>
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index e78d7c4f41bd..30f09e786f7b 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -2335,6 +2335,7 @@ static int intcapxt_irqdomain_alloc(struct irq_domain *domain, unsigned int virq
> struct irq_data *irqd = irq_domain_get_irq_data(domain, i);
>
> irqd->chip = &intcapxt_controller;
> + irqd->hwirq = info->hwirq;
> irqd->chip_data = info->data;
> __irq_set_handler(i, handle_edge_irq, 0, "edge");
> }
> @@ -2361,22 +2362,14 @@ static void intcapxt_unmask_irq(struct irq_data *irqd)
> xt.destid_0_23 = cfg->dest_apicid & GENMASK(23, 0);
> xt.destid_24_31 = cfg->dest_apicid >> 24;
>
> - /**
> - * Current IOMMU implementation uses the same IRQ for all
> - * 3 IOMMU interrupts.
> - */
> - writeq(xt.capxt, iommu->mmio_base + MMIO_INTCAPXT_EVT_OFFSET);
> - writeq(xt.capxt, iommu->mmio_base + MMIO_INTCAPXT_PPR_OFFSET);
> - writeq(xt.capxt, iommu->mmio_base + MMIO_INTCAPXT_GALOG_OFFSET);
> + writeq(xt.capxt, iommu->mmio_base + irqd->hwirq);
> }
>
> static void intcapxt_mask_irq(struct irq_data *irqd)
> {
> struct amd_iommu *iommu = irqd->chip_data;
>
> - writeq(0, iommu->mmio_base + MMIO_INTCAPXT_EVT_OFFSET);
> - writeq(0, iommu->mmio_base + MMIO_INTCAPXT_PPR_OFFSET);
> - writeq(0, iommu->mmio_base + MMIO_INTCAPXT_GALOG_OFFSET);
> + writeq(0, iommu->mmio_base + irqd->hwirq);
> }
>
>
> @@ -2439,7 +2432,8 @@ static struct irq_domain *iommu_get_irqdomain(void)
> return iommu_irqdomain;
> }
>
> -static int iommu_setup_intcapxt(struct amd_iommu *iommu)
> +static int __iommu_setup_intcapxt(struct amd_iommu *iommu, const char *devname,
> + int hwirq, irq_handler_t thread_fn)
> {
> struct irq_domain *domain;
> struct irq_alloc_info info;
> @@ -2453,6 +2447,7 @@ static int iommu_setup_intcapxt(struct amd_iommu *iommu)
> init_irq_alloc_info(&info, NULL);
> info.type = X86_IRQ_ALLOC_TYPE_AMDVI;
> info.data = iommu;
> + info.hwirq = hwirq;
>
> irq = irq_domain_alloc_irqs(domain, 1, node, &info);
> if (irq < 0) {
> @@ -2461,7 +2456,7 @@ static int iommu_setup_intcapxt(struct amd_iommu *iommu)
> }
>
> ret = request_threaded_irq(irq, amd_iommu_int_handler,
> - amd_iommu_int_thread, 0, "AMD-Vi", iommu);
> + thread_fn, 0, devname, iommu);
> if (ret) {
> irq_domain_free_irqs(irq, 1);
> irq_domain_remove(domain);
> @@ -2471,6 +2466,37 @@ static int iommu_setup_intcapxt(struct amd_iommu *iommu)
> return 0;
> }
>
> +static int iommu_setup_intcapxt(struct amd_iommu *iommu)
> +{
> + int ret;
> +
> + snprintf(iommu->evt_irq_name, sizeof(iommu->evt_irq_name),
> + "AMD-Vi%d-Evt", iommu->index);
> + ret = __iommu_setup_intcapxt(iommu, iommu->evt_irq_name,
> + MMIO_INTCAPXT_EVT_OFFSET,
> + amd_iommu_int_thread_evtlog);
> + if (ret)
> + return ret;
> +
> + snprintf(iommu->ppr_irq_name, sizeof(iommu->ppr_irq_name),
> + "AMD-Vi%d-PPR", iommu->index);
> + ret = __iommu_setup_intcapxt(iommu, iommu->ppr_irq_name,
> + MMIO_INTCAPXT_PPR_OFFSET,
> + amd_iommu_int_thread_pprlog);
> + if (ret)
> + return ret;
> +
> +#ifdef CONFIG_IRQ_REMAP
> + snprintf(iommu->ga_irq_name, sizeof(iommu->ga_irq_name),
> + "AMD-Vi%d-GA", iommu->index);
> + ret = __iommu_setup_intcapxt(iommu, iommu->ga_irq_name,
> + MMIO_INTCAPXT_GALOG_OFFSET,
> + amd_iommu_int_thread_galog);
> +#endif
> +
> + return ret;
> +}
> +
> static int iommu_init_irq(struct amd_iommu *iommu)
> {
> int ret;
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] iommu/amd: Refactor IOMMU interrupt handling logic for Event, PPR, and GA logs
2023-06-28 5:32 ` [PATCH v3 1/2] iommu/amd: Refactor IOMMU interrupt handling logic for Event, PPR, and GA logs Vasant Hegde
@ 2023-06-29 16:36 ` Jerry Snitselaar
0 siblings, 0 replies; 6+ messages in thread
From: Jerry Snitselaar @ 2023-06-29 16:36 UTC (permalink / raw
To: Vasant Hegde; +Cc: iommu, joro, suravee.suthikulpanit, joao.m.martins
On Wed, Jun 28, 2023 at 05:32:21AM +0000, Vasant Hegde wrote:
> The AMD IOMMU has three log buffers (i.e. Event, PPR, and GA). The IOMMU
> driver processes these log entries when it receive an IOMMU interrupt.
> Then, it needs to clear the corresponding interrupt status bits. Also, when
> an overflow occurs, it needs to handle the log overflow by clearing the
> specific overflow status bit and restart the log.
>
> Since, logic for handling these logs is the same, refactor the code into a
> helper function called amd_iommu_handle_irq(), which handles the steps
> described. Then, reuse it for all types of log.
>
> Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Signed-off-by: Vasant Hegde<vasant.hegde@amd.com>
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
> ---
> drivers/iommu/amd/amd_iommu.h | 3 ++
> drivers/iommu/amd/iommu.c | 93 +++++++++++++++++++----------------
> 2 files changed, 53 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
> index 8c61c19dabc4..f23ad6043f04 100644
> --- a/drivers/iommu/amd/amd_iommu.h
> +++ b/drivers/iommu/amd/amd_iommu.h
> @@ -12,6 +12,9 @@
> #include "amd_iommu_types.h"
>
> irqreturn_t amd_iommu_int_thread(int irq, void *data);
> +irqreturn_t amd_iommu_int_thread_evtlog(int irq, void *data);
> +irqreturn_t amd_iommu_int_thread_pprlog(int irq, void *data);
> +irqreturn_t amd_iommu_int_thread_galog(int irq, void *data);
> 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);
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 8f299bb0f3ed..5e9c287b10e0 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -841,57 +841,27 @@ static inline void
> 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_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)
> -
> -irqreturn_t amd_iommu_int_thread(int irq, void *data)
> +static void amd_iommu_handle_irq(void *data, const char *evt_type,
> + u32 int_mask, u32 overflow_mask,
> + void (*int_handler)(struct amd_iommu *),
> + void (*overflow_handler)(struct amd_iommu *))
> {
> struct amd_iommu *iommu = (struct amd_iommu *) data;
> u32 status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
> + u32 mask = int_mask | overflow_mask;
>
> - while (status & AMD_IOMMU_INT_MASK) {
> + while (status & mask) {
> /* Enable interrupt sources again */
> - writel(AMD_IOMMU_INT_MASK,
> - iommu->mmio_base + MMIO_STATUS_OFFSET);
> -
> - if (status & MMIO_STATUS_EVT_INT_MASK) {
> - pr_devel("Processing IOMMU Event Log\n");
> - iommu_poll_events(iommu);
> - }
> -
> - 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)) {
> - pr_devel("Processing IOMMU GA Log\n");
> - iommu_poll_ga_log(iommu);
> - }
> + writel(mask, iommu->mmio_base + MMIO_STATUS_OFFSET);
>
> - if (status & MMIO_STATUS_GALOG_OVERFLOW_MASK) {
> - pr_info_ratelimited("IOMMU GA Log overflow\n");
> - amd_iommu_restart_ga_log(iommu);
> + if (int_handler) {
> + pr_devel("Processing IOMMU (ivhd%d) %s Log\n",
> + iommu->index, evt_type);
> + int_handler(iommu);
> }
> -#endif
>
> - if (status & MMIO_STATUS_EVT_OVERFLOW_MASK) {
> - pr_info_ratelimited("IOMMU event log overflow\n");
> - amd_iommu_restart_event_logging(iommu);
> - }
> + if ((status & overflow_mask) && overflow_handler)
> + overflow_handler(iommu);
>
> /*
> * Hardware bug: ERBT1312
> @@ -908,6 +878,43 @@ irqreturn_t amd_iommu_int_thread(int irq, void *data)
> */
> status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
> }
> +}
> +
> +irqreturn_t amd_iommu_int_thread_evtlog(int irq, void *data)
> +{
> + amd_iommu_handle_irq(data, "Evt", MMIO_STATUS_EVT_INT_MASK,
> + MMIO_STATUS_EVT_OVERFLOW_MASK,
> + iommu_poll_events, amd_iommu_restart_event_logging);
> +
> + return IRQ_HANDLED;
> +}
> +
> +irqreturn_t amd_iommu_int_thread_pprlog(int irq, void *data)
> +{
> + amd_iommu_handle_irq(data, "PPR", MMIO_STATUS_PPR_INT_MASK,
> + MMIO_STATUS_PPR_OVERFLOW_MASK,
> + iommu_poll_ppr_log, amd_iommu_restart_ppr_log);
> +
> + return IRQ_HANDLED;
> +}
> +
> +irqreturn_t amd_iommu_int_thread_galog(int irq, void *data)
> +{
> +#ifdef CONFIG_IRQ_REMAP
> + amd_iommu_handle_irq(data, "GA", MMIO_STATUS_GALOG_INT_MASK,
> + MMIO_STATUS_GALOG_OVERFLOW_MASK,
> + iommu_poll_ga_log, amd_iommu_restart_ga_log);
> +#endif
> +
> + return IRQ_HANDLED;
> +}
> +
> +irqreturn_t amd_iommu_int_thread(int irq, void *data)
> +{
> + amd_iommu_int_thread_evtlog(irq, data);
> + amd_iommu_int_thread_pprlog(irq, data);
> + amd_iommu_int_thread_galog(irq, data);
> +
> return IRQ_HANDLED;
> }
>
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 0/2] iommu/amd: Interrupt handling improvements
2023-06-28 5:32 [PATCH v3 0/2] iommu/amd: Interrupt handling improvements Vasant Hegde
2023-06-28 5:32 ` [PATCH v3 1/2] iommu/amd: Refactor IOMMU interrupt handling logic for Event, PPR, and GA logs Vasant Hegde
2023-06-28 5:32 ` [PATCH v3 2/2] iommu/amd: Enable separate interrupt for PPR and GA log Vasant Hegde
@ 2023-07-14 14:20 ` Joerg Roedel
2 siblings, 0 replies; 6+ messages in thread
From: Joerg Roedel @ 2023-07-14 14:20 UTC (permalink / raw
To: Vasant Hegde; +Cc: iommu, suravee.suthikulpanit, joao.m.martins, jsnitsel
On Wed, Jun 28, 2023 at 05:32:20AM +0000, Vasant Hegde wrote:
> Vasant Hegde (2):
> iommu/amd: Refactor IOMMU interrupt handling logic for Event, PPR, and GA logs
> iommu/amd: Enable separate interrupt for PPR and GA log
>
> drivers/iommu/amd/amd_iommu.h | 3 +
> drivers/iommu/amd/amd_iommu_types.h | 9 +++
> drivers/iommu/amd/init.c | 50 ++++++++++++----
> drivers/iommu/amd/iommu.c | 93 ++++++++++++++++-------------
> 4 files changed, 100 insertions(+), 55 deletions(-)
Applied, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-07-14 14:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-28 5:32 [PATCH v3 0/2] iommu/amd: Interrupt handling improvements Vasant Hegde
2023-06-28 5:32 ` [PATCH v3 1/2] iommu/amd: Refactor IOMMU interrupt handling logic for Event, PPR, and GA logs Vasant Hegde
2023-06-29 16:36 ` Jerry Snitselaar
2023-06-28 5:32 ` [PATCH v3 2/2] iommu/amd: Enable separate interrupt for PPR and GA log Vasant Hegde
2023-06-29 16:35 ` Jerry Snitselaar
2023-07-14 14:20 ` [PATCH v3 0/2] iommu/amd: Interrupt handling improvements Joerg Roedel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.