[PATCH 0/3] iommu/amd: Consolidate PPR log handling

[PATCH 0/3] iommu/amd: Consolidate PPR log handling

[Vasant Hegde]

Consolidate PPR log enablement related code and also enable PPR/GA
interrupt after setting up the interrupt handler.

This patch series applies on of "Interrupt handling improvements"
series.
  [1] https://lore.kernel.org/linux-iommu/20230628053222.5962-1-vasant.hegde@amd.com/T/#t

Vasant Hegde (3):
  iommu/amd: Disable PPR log/interrupt in iommu_disable()
  iommu/amd: Consolidate PPR log enablement
  iommu/amd: Enable PPR/GA interrupt after interrupt handler setup

 drivers/iommu/amd/init.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

-- 
2.31.1

[PATCH 1/3] iommu/amd: Disable PPR log/interrupt in iommu_disable()

[Vasant Hegde]

Similar to other logs, disable PPR log/interrupt in
iommu_disable() path.

Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/init.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 30f09e786f7b..94c91b6ee603 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -483,6 +483,10 @@ static void iommu_disable(struct amd_iommu *iommu)
 	iommu_feature_disable(iommu, CONTROL_GALOG_EN);
 	iommu_feature_disable(iommu, CONTROL_GAINT_EN);
 
+	/* Disable IOMMU PPR logging */
+	iommu_feature_disable(iommu, CONTROL_PPRLOG_EN);
+	iommu_feature_disable(iommu, CONTROL_PPRINT_EN);
+
 	/* Disable IOMMU hardware itself */
 	iommu_feature_disable(iommu, CONTROL_IOMMU_EN);
 
-- 
2.31.1

Re: [PATCH 1/3] iommu/amd: Disable PPR log/interrupt in iommu_disable()

[Jerry Snitselaar]

On Wed, Jun 28, 2023 at 05:45:52AM +0000, Vasant Hegde wrote:
> Similar to other logs, disable PPR log/interrupt in
> iommu_disable() path.
> 
> 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/init.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 30f09e786f7b..94c91b6ee603 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -483,6 +483,10 @@ static void iommu_disable(struct amd_iommu *iommu)
>  	iommu_feature_disable(iommu, CONTROL_GALOG_EN);
>  	iommu_feature_disable(iommu, CONTROL_GAINT_EN);
>  
> +	/* Disable IOMMU PPR logging */
> +	iommu_feature_disable(iommu, CONTROL_PPRLOG_EN);
> +	iommu_feature_disable(iommu, CONTROL_PPRINT_EN);
> +
>  	/* Disable IOMMU hardware itself */
>  	iommu_feature_disable(iommu, CONTROL_IOMMU_EN);
>  
> -- 
> 2.31.1
> 

[PATCH 2/3] iommu/amd: Consolidate PPR log enablement

[Vasant Hegde]

Move PPR log interrupt bit setting to iommu_enable_ppr_log(). Also
rearrange iommu_enable_ppr_log() such that PPREn bit is enabled
before enabling PPRLog and PPRInt bits. So that when PPRLog bit is
set it will clear the PPRLogOverflow bit and sets the PPRLogRun bit
in the IOMMU Status Register [MMIO Offset 2020h].

Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/init.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 94c91b6ee603..f26c077eb9ea 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -934,6 +934,8 @@ static void iommu_enable_ppr_log(struct amd_iommu *iommu)
 	if (iommu->ppr_log == NULL)
 		return;
 
+	iommu_feature_enable(iommu, CONTROL_PPR_EN);
+
 	entry = iommu_virt_to_phys(iommu->ppr_log) | PPR_LOG_SIZE_512;
 
 	memcpy_toio(iommu->mmio_base + MMIO_PPR_LOG_OFFSET,
@@ -944,7 +946,7 @@ static void iommu_enable_ppr_log(struct amd_iommu *iommu)
 	writel(0x00, iommu->mmio_base + MMIO_PPR_TAIL_OFFSET);
 
 	iommu_feature_enable(iommu, CONTROL_PPRLOG_EN);
-	iommu_feature_enable(iommu, CONTROL_PPR_EN);
+	iommu_feature_enable(iommu, CONTROL_PPRINT_EN);
 }
 
 static void __init free_ppr_log(struct amd_iommu *iommu)
@@ -2526,8 +2528,6 @@ static int iommu_init_irq(struct amd_iommu *iommu)
 
 	iommu_feature_enable(iommu, CONTROL_EVT_INT_EN);
 
-	if (iommu->ppr_log != NULL)
-		iommu_feature_enable(iommu, CONTROL_PPRINT_EN);
 	return 0;
 }
 
-- 
2.31.1

Re: [PATCH 2/3] iommu/amd: Consolidate PPR log enablement

[Jerry Snitselaar]

On Wed, Jun 28, 2023 at 05:45:53AM +0000, Vasant Hegde wrote:
> Move PPR log interrupt bit setting to iommu_enable_ppr_log(). Also
> rearrange iommu_enable_ppr_log() such that PPREn bit is enabled
> before enabling PPRLog and PPRInt bits. So that when PPRLog bit is
> set it will clear the PPRLogOverflow bit and sets the PPRLogRun bit
> in the IOMMU Status Register [MMIO Offset 2020h].
> 
> 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/init.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 94c91b6ee603..f26c077eb9ea 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -934,6 +934,8 @@ static void iommu_enable_ppr_log(struct amd_iommu *iommu)
>  	if (iommu->ppr_log == NULL)
>  		return;
>  
> +	iommu_feature_enable(iommu, CONTROL_PPR_EN);
> +
>  	entry = iommu_virt_to_phys(iommu->ppr_log) | PPR_LOG_SIZE_512;
>  
>  	memcpy_toio(iommu->mmio_base + MMIO_PPR_LOG_OFFSET,
> @@ -944,7 +946,7 @@ static void iommu_enable_ppr_log(struct amd_iommu *iommu)
>  	writel(0x00, iommu->mmio_base + MMIO_PPR_TAIL_OFFSET);
>  
>  	iommu_feature_enable(iommu, CONTROL_PPRLOG_EN);
> -	iommu_feature_enable(iommu, CONTROL_PPR_EN);
> +	iommu_feature_enable(iommu, CONTROL_PPRINT_EN);
>  }
>  
>  static void __init free_ppr_log(struct amd_iommu *iommu)
> @@ -2526,8 +2528,6 @@ static int iommu_init_irq(struct amd_iommu *iommu)
>  
>  	iommu_feature_enable(iommu, CONTROL_EVT_INT_EN);
>  
> -	if (iommu->ppr_log != NULL)
> -		iommu_feature_enable(iommu, CONTROL_PPRINT_EN);
>  	return 0;
>  }
>  
> -- 
> 2.31.1
> 

[PATCH 3/3] iommu/amd: Enable PPR/GA interrupt after interrupt handler setup

[Vasant Hegde]

Current code enables PPR and GA interrupts before setting up the
interrupt handler (in state_next()). Make sure interrupt handler
is in place before enabling these interrupt.

amd_iommu_enable_interrupts() gets called in normal boot, kdump as well
as in suspend/resume path. Hence moving interrupt enablement to this
function works fine.

Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/init.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index f26c077eb9ea..45efb7e5d725 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -2943,8 +2943,6 @@ static void enable_iommus_vapic(void)
 static void enable_iommus(void)
 {
 	early_enable_iommus();
-	enable_iommus_vapic();
-	enable_iommus_v2();
 }
 
 static void disable_iommus(void)
@@ -3208,6 +3206,13 @@ static int amd_iommu_enable_interrupts(void)
 			goto out;
 	}
 
+	/*
+	 * Interrupt handler is ready to process interrupts. Enable
+	 * PPR and GA log interrupt for all IOMMUs.
+	 */
+	enable_iommus_vapic();
+	enable_iommus_v2();
+
 out:
 	return ret;
 }
@@ -3287,8 +3292,6 @@ static int __init state_next(void)
 		register_syscore_ops(&amd_iommu_syscore_ops);
 		ret = amd_iommu_init_pci();
 		init_state = ret ? IOMMU_INIT_ERROR : IOMMU_PCI_INIT;
-		enable_iommus_vapic();
-		enable_iommus_v2();
 		break;
 	case IOMMU_PCI_INIT:
 		ret = amd_iommu_enable_interrupts();
-- 
2.31.1

Re: [PATCH 3/3] iommu/amd: Enable PPR/GA interrupt after interrupt handler setup

[Jerry Snitselaar]

On Wed, Jun 28, 2023 at 05:45:54AM +0000, Vasant Hegde wrote:
> Current code enables PPR and GA interrupts before setting up the
> interrupt handler (in state_next()). Make sure interrupt handler
> is in place before enabling these interrupt.
> 
> amd_iommu_enable_interrupts() gets called in normal boot, kdump as well
> as in suspend/resume path. Hence moving interrupt enablement to this
> function works fine.
> 
> 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/init.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index f26c077eb9ea..45efb7e5d725 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -2943,8 +2943,6 @@ static void enable_iommus_vapic(void)
>  static void enable_iommus(void)
>  {
>  	early_enable_iommus();
> -	enable_iommus_vapic();
> -	enable_iommus_v2();
>  }
>  
>  static void disable_iommus(void)
> @@ -3208,6 +3206,13 @@ static int amd_iommu_enable_interrupts(void)
>  			goto out;
>  	}
>  
> +	/*
> +	 * Interrupt handler is ready to process interrupts. Enable
> +	 * PPR and GA log interrupt for all IOMMUs.
> +	 */
> +	enable_iommus_vapic();
> +	enable_iommus_v2();
> +
>  out:
>  	return ret;
>  }
> @@ -3287,8 +3292,6 @@ static int __init state_next(void)
>  		register_syscore_ops(&amd_iommu_syscore_ops);
>  		ret = amd_iommu_init_pci();
>  		init_state = ret ? IOMMU_INIT_ERROR : IOMMU_PCI_INIT;
> -		enable_iommus_vapic();
> -		enable_iommus_v2();
>  		break;
>  	case IOMMU_PCI_INIT:
>  		ret = amd_iommu_enable_interrupts();
> -- 
> 2.31.1
> 

Re: [PATCH 0/3] iommu/amd: Consolidate PPR log handling

[Joerg Roedel]

On Wed, Jun 28, 2023 at 05:45:51AM +0000, Vasant Hegde wrote:
> Vasant Hegde (3):
>   iommu/amd: Disable PPR log/interrupt in iommu_disable()
>   iommu/amd: Consolidate PPR log enablement
>   iommu/amd: Enable PPR/GA interrupt after interrupt handler setup
> 
>  drivers/iommu/amd/init.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)

Applied, thanks.