Re: [PATCH 1/2 V2] iommu/amd: Add workaround for ERBT1312

[Suravee Suthikulanit <suravee.suthikulpanit@amd.com>]

Joerg,

This workaround is required for both event log and ppr log.  Your patch 
is only taking care of the event log.

Suravee

On 4/18/2013 11:02 AM, Joerg Roedel wrote:
> On Mon, Apr 15, 2013 at 02:07:46AM -0500, suravee.suthikulpanit@amd.com wrote:
>>   drivers/iommu/amd_iommu.c |  145 +++++++++++++++++++++++++++++----------------
> That is way too much for a simple erratum workaround, and too much for a
> stable backport. I queued the patch below instead, which has a much
> smaller diff and does the same. Please rebase your second patch on-top
> of this one and send it again.
>
>  From 4ba052102863da02db79c03d2483b6ad905737ad Mon Sep 17 00:00:00 2001
> From: Joerg Roedel <joro@8bytes.org>
> Date: Thu, 18 Apr 2013 17:55:04 +0200
> Subject: [PATCH] iommu/amd: Workaround for ERBT1312
>
> Work around an IOMMU  hardware bug where clearing the
> EVT_INT bit in the status register may race with the
> hardware trying to set it again. When not handled the bit
> might not be cleared and we lose all future event
> interrupts.
>
> Reported-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Joerg Roedel <joro@8bytes.org>
> ---
>   drivers/iommu/amd_iommu.c |   17 +++++++++++++----
>   1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index f42793d..de5ae4b 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -700,14 +700,23 @@ retry:
>   
>   static void iommu_poll_events(struct amd_iommu *iommu)
>   {
> -	u32 head, tail;
> +	u32 head, tail, status;
>   	unsigned long flags;
>   
> -	/* enable event interrupts again */
> -	writel(MMIO_STATUS_EVT_INT_MASK, iommu->mmio_base + MMIO_STATUS_OFFSET);
> -
>   	spin_lock_irqsave(&iommu->lock, flags);
>   
> +	/* enable event interrupts again */
> +	do {
> +		/*
> +		 * Workaround for Erratum ERBT1312
> +		 * Clearing the EVT_INT bit may race in the hardware, so read
> +		 * it again and make sure it was really cleared
> +		 */
> +		status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
> +		writel(MMIO_STATUS_EVT_INT_MASK,
> +		       iommu->mmio_base + MMIO_STATUS_OFFSET);
> +	} while (status & MMIO_STATUS_EVT_INT_MASK);
> +
>   	head = readl(iommu->mmio_base + MMIO_EVT_HEAD_OFFSET);
>   	tail = readl(iommu->mmio_base + MMIO_EVT_TAIL_OFFSET);
>