Re: [ 102/127] iommu/amd: Workaround for ERBT1312

Re: [ 102/127] iommu/amd: Workaround for ERBT1312

[Andreas Hartmann]

Hello Joerg, hello Alex,

the subsequent patch and the patch "iommu/amd: Re-enable IOMMU event log
interrupt after handling." 925fe08bce38d1ff052fe2209b9e2b8d5fbb7f98
spread /var/log/messages with the following line (> 700 lines/second)
right after loading vfio:

AMD-Vi: Event logged [IO_PAGE_FAULT device=00:14.0 domain=0x0000 address=0x000000fdf9103300 flags=0x0600]

lspci -vvvs 0:14.0
00:14.0 SMBus: Advanced Micro Devices [AMD] nee ATI SBx00 SMBus Controller (rev 42)
        Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
        Status: Cap- 66MHz+ UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-


Besides the enormous pollution I couldn't see any malfunction at all.
At first, I didn't realised it at all (-> the SSD was fast enough to
cover it silently). I saw it the first time I rebooted because X didn't start any more because
the /var partition was completely full. 

I removed the two mentioned patches and all is working
fine again as before.

Any idea?


Thanks,
kind regards,
Andreas


Greg Kroah-Hartman wrote:
> 3.9-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Joerg Roedel <joro@8bytes.org>
> 
> commit d3263bc29706e42f74d8800807c2dedf320d77f1 upstream.
> 
> Work around an IOMMU  hardware bug where clearing the
> EVT_INT or PPR_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 or ppr
> interrupts.
> 
> Reported-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Signed-off-by: Joerg Roedel <joro@8bytes.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> ---
>  drivers/iommu/amd_iommu.c |   34 ++++++++++++++++++++++++++--------
>  1 file changed, 26 insertions(+), 8 deletions(-)
> 
> --- 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);
>  
> @@ -744,16 +753,25 @@ static void iommu_handle_ppr_entry(struc
>  static void iommu_poll_ppr_log(struct amd_iommu *iommu)
>  {
>  	unsigned long flags;
> -	u32 head, tail;
> +	u32 head, tail, status;
>  
>  	if (iommu->ppr_log == NULL)
>  		return;
>  
> -	/* enable ppr interrupts again */
> -	writel(MMIO_STATUS_PPR_INT_MASK, iommu->mmio_base + MMIO_STATUS_OFFSET);
> -
>  	spin_lock_irqsave(&iommu->lock, flags);
>  
> +	/* enable ppr interrupts again */
> +	do {
> +		/*
> +		 * Workaround for Erratum ERBT1312
> +		 * Clearing the PPR_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_PPR_INT_MASK,
> +		       iommu->mmio_base + MMIO_STATUS_OFFSET);
> +	} while (status & MMIO_STATUS_PPR_INT_MASK);
> +
>  	head = readl(iommu->mmio_base + MMIO_PPR_HEAD_OFFSET);
>  	tail = readl(iommu->mmio_base + MMIO_PPR_TAIL_OFFSET);
>  

Re: [ 102/127] iommu/amd: Workaround for ERBT1312

[Alex Williamson]

On Fri, 2013-06-28 at 18:11 +0200, Andreas Hartmann wrote:
> Hello Joerg, hello Alex,
> 
> the subsequent patch and the patch "iommu/amd: Re-enable IOMMU event log
> interrupt after handling." 925fe08bce38d1ff052fe2209b9e2b8d5fbb7f98
> spread /var/log/messages with the following line (> 700 lines/second)
> right after loading vfio:
> 
> AMD-Vi: Event logged [IO_PAGE_FAULT device=00:14.0 domain=0x0000 address=0x000000fdf9103300 flags=0x0600]

That's interesting, I PXE boot my system from one NIC then use a
different NIC for the iSCSI root.  The PXE boot NIC now screams like
this, _until_ I attach it to vfio, then it quiets down.

> lspci -vvvs 0:14.0
> 00:14.0 SMBus: Advanced Micro Devices [AMD] nee ATI SBx00 SMBus Controller (rev 42)
>         Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
>         Status: Cap- 66MHz+ UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
> 
> 
> Besides the enormous pollution I couldn't see any malfunction at all.
> At first, I didn't realised it at all (-> the SSD was fast enough to
> cover it silently). I saw it the first time I rebooted because X didn't start any more because
> the /var partition was completely full. 
> 
> I removed the two mentioned patches and all is working
> fine again as before.
> 
> Any idea?

Not really without some digging.  I wonder if it's a new event each time
or if something is just not clearing a previous event.  ISTR that a boot
used to often, but not always, generate a couple faults between the
IOMMU being enabled and the NIC driver being loaded.  All the faults I
see are to the same address, so my guess is that it's getting replayed.
Thanks,

Alex

> Greg Kroah-Hartman wrote:
> > 3.9-stable review patch.  If anyone has any objections, please let me know.
> > 
> > ------------------
> > 
> > From: Joerg Roedel <joro@8bytes.org>
> > 
> > commit d3263bc29706e42f74d8800807c2dedf320d77f1 upstream.
> > 
> > Work around an IOMMU  hardware bug where clearing the
> > EVT_INT or PPR_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 or ppr
> > interrupts.
> > 
> > Reported-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> > Signed-off-by: Joerg Roedel <joro@8bytes.org>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > 
> > ---
> >  drivers/iommu/amd_iommu.c |   34 ++++++++++++++++++++++++++--------
> >  1 file changed, 26 insertions(+), 8 deletions(-)
> > 
> > --- 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);
> >  
> > @@ -744,16 +753,25 @@ static void iommu_handle_ppr_entry(struc
> >  static void iommu_poll_ppr_log(struct amd_iommu *iommu)
> >  {
> >  	unsigned long flags;
> > -	u32 head, tail;
> > +	u32 head, tail, status;
> >  
> >  	if (iommu->ppr_log == NULL)
> >  		return;
> >  
> > -	/* enable ppr interrupts again */
> > -	writel(MMIO_STATUS_PPR_INT_MASK, iommu->mmio_base + MMIO_STATUS_OFFSET);
> > -
> >  	spin_lock_irqsave(&iommu->lock, flags);
> >  
> > +	/* enable ppr interrupts again */
> > +	do {
> > +		/*
> > +		 * Workaround for Erratum ERBT1312
> > +		 * Clearing the PPR_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_PPR_INT_MASK,
> > +		       iommu->mmio_base + MMIO_STATUS_OFFSET);
> > +	} while (status & MMIO_STATUS_PPR_INT_MASK);
> > +
> >  	head = readl(iommu->mmio_base + MMIO_PPR_HEAD_OFFSET);
> >  	tail = readl(iommu->mmio_base + MMIO_PPR_TAIL_OFFSET);
> >  

Re: [ 102/127] iommu/amd: Workaround for ERBT1312

[Joerg Roedel]

Hi Andreas,

On Fri, Jun 28, 2013 at 06:11:36PM +0200, Andreas Hartmann wrote:
> Hello Joerg, hello Alex,
> 
> the subsequent patch and the patch "iommu/amd: Re-enable IOMMU event log
> interrupt after handling." 925fe08bce38d1ff052fe2209b9e2b8d5fbb7f98
> spread /var/log/messages with the following line (> 700 lines/second)
> right after loading vfio:
> 
> AMD-Vi: Event logged [IO_PAGE_FAULT device=00:14.0 domain=0x0000 address=0x000000fdf9103300 flags=0x0600]
> 
> lspci -vvvs 0:14.0
> 00:14.0 SMBus: Advanced Micro Devices [AMD] nee ATI SBx00 SMBus Controller (rev 42)
>         Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
>         Status: Cap- 66MHz+ UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-

Most likely a BIOS issue that is uncovered by re-enabling the event-log
interrupt patch. The device itself is only used by the BIOS and not by
the Linux kernel

> Besides the enormous pollution I couldn't see any malfunction at all.
> At first, I didn't realised it at all (-> the SSD was fast enough to
> cover it silently). I saw it the first time I rebooted because X didn't start any more because
> the /var partition was completely full. 
> 
> I removed the two mentioned patches and all is working
> fine again as before.

Without these two patches, can you check dmesg after boot if there are
other lines which report IO_PAGE_FAULTs?


	Joerg

Re: [ 102/127] iommu/amd: Workaround for ERBT1312

[Joerg Roedel]

Hi Alex,

On Fri, Jun 28, 2013 at 11:49:04AM -0600, Alex Williamson wrote:
> That's interesting, I PXE boot my system from one NIC then use a
> different NIC for the iSCSI root.  The PXE boot NIC now screams like
> this, _until_ I attach it to vfio, then it quiets down.

Can you please send an example line of the reported fault? The addresses
it faults on would be interesting.

> > Any idea?
> 
> Not really without some digging.  I wonder if it's a new event each time
> or if something is just not clearing a previous event.  ISTR that a boot
> used to often, but not always, generate a couple faults between the
> IOMMU being enabled and the NIC driver being loaded.  All the faults I
> see are to the same address, so my guess is that it's getting replayed.

Well, I think it is a problem uncovered by the patch that re-enables the
event-log interrupt after it happened once. We need to find a strategy
to cope with those problems.

To my mind as a quick-fix comes rate-limiting for the printks. Or we use
the suppress-pf bit in the DTE to suppress all page-faults after the
first one.


	Joerg

Re: [ 102/127] iommu/amd: Workaround for ERBT1312

[Andreas Hartmann]

Hello Joerg,

Joerg Roedel wrote:
> Hi Andreas,
> 
> On Fri, Jun 28, 2013 at 06:11:36PM +0200, Andreas Hartmann wrote:
>> Hello Joerg, hello Alex,
>>
>> the subsequent patch and the patch "iommu/amd: Re-enable IOMMU event log
>> interrupt after handling." 925fe08bce38d1ff052fe2209b9e2b8d5fbb7f98
>> spread /var/log/messages with the following line (> 700 lines/second)
>> right after loading vfio:
>>
>> AMD-Vi: Event logged [IO_PAGE_FAULT device=00:14.0 domain=0x0000 address=0x000000fdf9103300 flags=0x0600]
>>
>> lspci -vvvs 0:14.0
>> 00:14.0 SMBus: Advanced Micro Devices [AMD] nee ATI SBx00 SMBus Controller (rev 42)
>>         Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
>>         Status: Cap- 66MHz+ UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
> 
> Most likely a BIOS issue that is uncovered by re-enabling the event-log
> interrupt patch. The device itself is only used by the BIOS and not by
> the Linux kernel

Thanks for this info! Good to know.

[...]

>> I removed the two mentioned patches and all is working
>> fine again as before.
> 
> Without these two patches, can you check dmesg after boot if there are
> other lines which report IO_PAGE_FAULTs?

You're right, there is exactly one entry directly after loading of vfio.
I can see this message, too, with linux 3.4.43.


Regards,
Andreas

Re: [ 102/127] iommu/amd: Workaround for ERBT1312

[Alex Williamson]

On Fri, 2013-06-28 at 20:29 +0200, Joerg Roedel wrote:
> Hi Alex,
> 
> On Fri, Jun 28, 2013 at 11:49:04AM -0600, Alex Williamson wrote:
> > That's interesting, I PXE boot my system from one NIC then use a
> > different NIC for the iSCSI root.  The PXE boot NIC now screams like
> > this, _until_ I attach it to vfio, then it quiets down.
> 
> Can you please send an example line of the reported fault? The addresses
> it faults on would be interesting.

[   99.613489] AMD-Vi: Event logged [IO_PAGE_FAULT device=02:00.0
domain=0x0000 address=0x000000000008f880 flags=0x0050]

> > > Any idea?
> > 
> > Not really without some digging.  I wonder if it's a new event each time
> > or if something is just not clearing a previous event.  ISTR that a boot
> > used to often, but not always, generate a couple faults between the
> > IOMMU being enabled and the NIC driver being loaded.  All the faults I
> > see are to the same address, so my guess is that it's getting replayed.
> 
> Well, I think it is a problem uncovered by the patch that re-enables the
> event-log interrupt after it happened once. We need to find a strategy
> to cope with those problems.
> 
> To my mind as a quick-fix comes rate-limiting for the printks. Or we use
> the suppress-pf bit in the DTE to suppress all page-faults after the
> first one.
> 
> 
> 	Joerg
> 
> 

Re: [ 102/127] iommu/amd: Workaround for ERBT1312

[Joerg Roedel]

Alex, Andreas,

On Fri, Jun 28, 2013 at 08:42:05PM +0200, Andreas Hartmann wrote:
> You're right, there is exactly one entry directly after loading of vfio.
> I can see this message, too, with linux 3.4.43.

Can you please test this patch? It should reduce the noise
significantly, but a few of those error messages are still expected.

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index e3c2d74..74e1d1c 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1431,6 +1431,7 @@ static void init_device_table_dma(void)
 	for (devid = 0; devid <= amd_iommu_last_bdf; ++devid) {
 		set_dev_entry_bit(devid, DEV_ENTRY_VALID);
 		set_dev_entry_bit(devid, DEV_ENTRY_TRANSLATION);
+		set_dev_entry_bit(devid, DEV_ENTRY_ONE_FAULT);
 	}
 }
 
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 083f98c..4872b68 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -173,6 +173,7 @@
 #define DEV_ENTRY_TRANSLATION   0x01
 #define DEV_ENTRY_IR            0x3d
 #define DEV_ENTRY_IW            0x3e
+#define DEV_ENTRY_ONE_FAULT	0x61
 #define DEV_ENTRY_NO_PAGE_FAULT	0x62
 #define DEV_ENTRY_EX            0x67
 #define DEV_ENTRY_SYSMGT1       0x68

Re: [ 102/127] iommu/amd: Workaround for ERBT1312

[Andreas Hartmann]

Alex Williamson wrote:
> On Fri, 2013-06-28 at 18:11 +0200, Andreas Hartmann wrote:
>> Hello Joerg, hello Alex,
>>
>> the subsequent patch and the patch "iommu/amd: Re-enable IOMMU event log
>> interrupt after handling." 925fe08bce38d1ff052fe2209b9e2b8d5fbb7f98
>> spread /var/log/messages with the following line (> 700 lines/second)
>> right after loading vfio:
>>
>> AMD-Vi: Event logged [IO_PAGE_FAULT device=00:14.0 domain=0x0000 address=0x000000fdf9103300 flags=0x0600]
> 
> That's interesting, I PXE boot my system from one NIC then use a
> different NIC for the iSCSI root.  The PXE boot NIC now screams like
> this, _until_ I attach it to vfio, then it quiets down.

Hmm, I just remembered an active workaround I implemented to "resolve"
an error like this when starting my VM to passthrough my intel pci
ethernet device since I applied a new kvm version:


qemu-kvm: -device vfio-pci,host=06:06.0: vfio: failed to set iommu for
container: Device or resource busy

qemu-kvm: -device vfio-pci,host=06:06.0: vfio: failed to setup container
for group 12

qemu-kvm: -device vfio-pci,host=06:06.0: vfio: failed to get group 12

qemu-kvm: -device vfio-pci,host=06:06.0: Device 'vfio-pci' could not be
initialized


The workaround was to bind the individual multifunction devices during
boot one time to vfio and release them after 2 seconds again and rebind
them to the original drivers as they where bound before (if it was bound
to any).

I did this with a script beginning like this:

#!/bin/sh
modprobe vfio-pci

echo "1002 4385" > /sys/bus/pci/drivers/vfio-pci/new_id
echo 0000:00:14.0 > /sys/bus/pci/devices/0000:00:14.0/driver/unbind
echo 0000:00:14.0 > /sys/bus/pci/drivers/vfio-pci/bind
...

sleep 2

echo 0000:00:14.0 > /sys/bus/pci/drivers/vfio-pci/unbind
echo "1002 4385" > /sys/bus/pci/drivers/vfio-pci/remove_id
...

The logs in messages:

Jun 28 15:54:12 . kernel: [   48.860147] VFIO - User Level meta-driver version: 0.3
Jun 28 15:54:12 . kernel: [   48.875243] AMD-Vi: Event logged [IO_PAGE_FAULT device=00:14.0 domain=0x0000 address=0x000000fdf9103300 flags=0x0600]
...

Therefore, the logoutput most probably started after device 14.0 was
bound to vfio. If it would have started after removing vfio, I would
have expected 2 seconds between the start messages of vfio and the first
occurrence of the IO_PAGE_FAULT.

Today, I'm using kvm 1.3.1 and it isn't necessary to use the complete
workaround anymore. It is enough to bind / unbind the pci bridge
as described above before starting the VM with the passed through pci
ethernet device.
Because I now don't touch the 14.0 device any more, the IO_PAGE_FAULT
messages disappeared completely.

@Joerg:
Anyway, I'm going to test your provided patch tomorrow!

BTW: what does it mean: IO_PAGE_FAULT - what do I have to expect if I
see this message?



Thanks,
regards,
Andreas

Re: [ 102/127] iommu/amd: Workaround for ERBT1312

[Alex Williamson]

On Fri, 2013-06-28 at 21:23 +0200, Joerg Roedel wrote:
> Alex, Andreas,
> 
> On Fri, Jun 28, 2013 at 08:42:05PM +0200, Andreas Hartmann wrote:
> > You're right, there is exactly one entry directly after loading of vfio.
> > I can see this message, too, with linux 3.4.43.
> 
> Can you please test this patch? It should reduce the noise
> significantly, but a few of those error messages are still expected.

Seems to work for me.  I generated a few errors, but each one seems
unique and is more like what I would expect if the PXE ROM left the NIC
running:

[    2.574155] AMD-Vi: Event logged [IO_PAGE_FAULT device=02:00.0 domain=0x001b address=0x00000000000946b0 flags=0x0020]                                        
[    2.574157] AMD-Vi: Event logged [IO_PAGE_FAULT device=02:00.0 domain=0x001b address=0x000000000008f8d0 flags=0x0020]                                        
[    4.992074] AMD-Vi: Event logged [IO_PAGE_FAULT device=02:00.0 domain=0x001b address=0x000000000008f880 flags=0x0000] 

I booted the system four times and twice got just three log messages,
like above, and twice got no faults.  Thanks,

Alex

> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
> index e3c2d74..74e1d1c 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -1431,6 +1431,7 @@ static void init_device_table_dma(void)
>  	for (devid = 0; devid <= amd_iommu_last_bdf; ++devid) {
>  		set_dev_entry_bit(devid, DEV_ENTRY_VALID);
>  		set_dev_entry_bit(devid, DEV_ENTRY_TRANSLATION);
> +		set_dev_entry_bit(devid, DEV_ENTRY_ONE_FAULT);
>  	}
>  }
>  
> diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
> index 083f98c..4872b68 100644
> --- a/drivers/iommu/amd_iommu_types.h
> +++ b/drivers/iommu/amd_iommu_types.h
> @@ -173,6 +173,7 @@
>  #define DEV_ENTRY_TRANSLATION   0x01
>  #define DEV_ENTRY_IR            0x3d
>  #define DEV_ENTRY_IW            0x3e
> +#define DEV_ENTRY_ONE_FAULT	0x61
>  #define DEV_ENTRY_NO_PAGE_FAULT	0x62
>  #define DEV_ENTRY_EX            0x67
>  #define DEV_ENTRY_SYSMGT1       0x68
> 

Re: [ 102/127] iommu/amd: Workaround for ERBT1312

[Andreas Hartmann]

Joerg Roedel wrote:
> Alex, Andreas,
> 
> On Fri, Jun 28, 2013 at 08:42:05PM +0200, Andreas Hartmann wrote:
>> You're right, there is exactly one entry directly after loading of vfio.
>> I can see this message, too, with linux 3.4.43.
> 
> Can you please test this patch? It should reduce the noise
> significantly, but a few of those error messages are still expected.

Sorry, but it doesn't work for me at all :-(. Behaviour is unchanged. It
is exactly as described in the other mail: at the moment of binding vfio
to 14.0, the fire begins.

echo "1002 4385" > /sys/bus/pci/drivers/vfio-pci/new_id
echo 0000:00:14.0 > /sys/bus/pci/devices/0000:00:14.0/driver/unbind
echo 0000:00:14.0 > /sys/bus/pci/drivers/vfio-pci/bind


Regards,
Andreas

Re: [ 102/127] iommu/amd: Workaround for ERBT1312

[Joerg Roedel]

On Sat, Jun 29, 2013 at 07:54:20AM +0200, Andreas Hartmann wrote:
> Sorry, but it doesn't work for me at all :-(. Behaviour is unchanged. It
> is exactly as described in the other mail: at the moment of binding vfio
> to 14.0, the fire begins.

Hmm, VFIO attaches the device to a new domain. That clears the bit, how
about this patch:

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 1a5285b..cdf346f 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2111,6 +2111,8 @@ static void set_dte_entry(u16 devid, struct protection_domain *domain, bool ats)
 
 		tmp = DTE_GCR3_VAL_C(gcr3) << DTE_GCR3_SHIFT_C;
 		flags    |= tmp;
+
+		flags	 |= DTE_FLAG_SE;
 	}
 
 	flags &= ~(0xffffUL);
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index e3c2d74..74e1d1c 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1431,6 +1431,7 @@ static void init_device_table_dma(void)
 	for (devid = 0; devid <= amd_iommu_last_bdf; ++devid) {
 		set_dev_entry_bit(devid, DEV_ENTRY_VALID);
 		set_dev_entry_bit(devid, DEV_ENTRY_TRANSLATION);
+		set_dev_entry_bit(devid, DEV_ENTRY_ONE_FAULT);
 	}
 }
 
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 083f98c..2104ca4 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -173,6 +173,7 @@
 #define DEV_ENTRY_TRANSLATION   0x01
 #define DEV_ENTRY_IR            0x3d
 #define DEV_ENTRY_IW            0x3e
+#define DEV_ENTRY_ONE_FAULT	0x61
 #define DEV_ENTRY_NO_PAGE_FAULT	0x62
 #define DEV_ENTRY_EX            0x67
 #define DEV_ENTRY_SYSMGT1       0x68
@@ -282,6 +283,7 @@
 #define IOMMU_PTE_IW (1ULL << 62)
 
 #define DTE_FLAG_IOTLB	(0x01UL << 32)
+#define DTE_FLAG_SE	(0x01UL << 33)
 #define DTE_FLAG_GV	(0x01ULL << 55)
 #define DTE_GLX_SHIFT	(56)
 #define DTE_GLX_MASK	(3)

Re: [ 102/127] iommu/amd: Workaround for ERBT1312

[Andreas Hartmann]

Joerg Roedel schrieb:
> On Sat, Jun 29, 2013 at 07:54:20AM +0200, Andreas Hartmann wrote:
>> Sorry, but it doesn't work for me at all :-(. Behaviour is unchanged. It
>> is exactly as described in the other mail: at the moment of binding vfio
>> to 14.0, the fire begins.
>
> Hmm, VFIO attaches the device to a new domain. That clears the bit, how
> about this patch:

Didn't help, too :-(


Regards,
Andreas

[ 102/127] iommu/amd: Workaround for ERBT1312

[Greg Kroah-Hartman]

3.9-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Joerg Roedel <joro@8bytes.org>

commit d3263bc29706e42f74d8800807c2dedf320d77f1 upstream.

Work around an IOMMU  hardware bug where clearing the
EVT_INT or PPR_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 or ppr
interrupts.

Reported-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Joerg Roedel <joro@8bytes.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/iommu/amd_iommu.c |   34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)

--- 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);
 
@@ -744,16 +753,25 @@ static void iommu_handle_ppr_entry(struc
 static void iommu_poll_ppr_log(struct amd_iommu *iommu)
 {
 	unsigned long flags;
-	u32 head, tail;
+	u32 head, tail, status;
 
 	if (iommu->ppr_log == NULL)
 		return;
 
-	/* enable ppr interrupts again */
-	writel(MMIO_STATUS_PPR_INT_MASK, iommu->mmio_base + MMIO_STATUS_OFFSET);
-
 	spin_lock_irqsave(&iommu->lock, flags);
 
+	/* enable ppr interrupts again */
+	do {
+		/*
+		 * Workaround for Erratum ERBT1312
+		 * Clearing the PPR_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_PPR_INT_MASK,
+		       iommu->mmio_base + MMIO_STATUS_OFFSET);
+	} while (status & MMIO_STATUS_PPR_INT_MASK);
+
 	head = readl(iommu->mmio_base + MMIO_PPR_HEAD_OFFSET);
 	tail = readl(iommu->mmio_base + MMIO_PPR_TAIL_OFFSET);