[PATCH] pci: Reenable the AMD IOMMU if it's mysteriously vanished over suspend

[PATCH] pci: Reenable the AMD IOMMU if it's mysteriously vanished over suspend

[Matthew Garrett]

AMD's reference BIOS code had a bug that could result in the firmware
failing to reenable the iommu on resume. It transpires that this causes
certain less than desirable behaviour when it comes to PCI accesses, to
whit them ending up somewhere near Bristol when the more desirable outcome
was Edinburgh. Sadness ensues, perhaps along with filesystem corruption.
Let's make sure that it gets turned back on.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/pci/quirks.c |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 89ed181..dcf9832 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -2695,6 +2695,39 @@ DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_RICOH, PCI_DEVICE_ID_RICOH_R5C832,
 #endif /*CONFIG_MMC_RICOH_MMC*/
 
 
+#ifdef CONFIG_AMD_IOMMU
+
+/* Some AMD BIOSes fail to reenable the iommu on resume */
+
+static void amd_iommu_reenable(struct pci_dev *dev)
+{
+	u32 ioc_feature_control;
+	struct pci_dev *iommu;
+
+	iommu = pci_get_device(PCI_VENDOR_ID_ATI, 0x5a23, NULL);
+
+	if (!iommu)
+		return;
+
+	/* Select Northbridge indirect register 0x75 and enable writing */
+	pci_write_config_dword(dev, 0x60, 0x75 | (1 << 7));
+	pci_read_config_dword(dev, 0x64, &ioc_feature_control);
+
+	/* Enable the iommu if it's vanished */
+	if (!(ioc_feature_control & 0x1))
+		pci_write_config_dword(dev, 0x64, ioc_feature_control | 1);
+
+	/* Disable writing again */
+	pci_write_config_dword(dev, 0x60, 0x75);
+
+	pci_dev_put(iommu);
+}
+
+DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_ATI, 0x5a10, amd_iommu_reenable);
+DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_ATI, 0x5a12, amd_iommu_reenable);
+DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_ATI, 0x5a13, amd_iommu_reenable);
+#endif
+
 static void pci_do_fixups(struct pci_dev *dev, struct pci_fixup *f,
 			  struct pci_fixup *end)
 {
-- 
1.7.3

Re: [PATCH] pci: Reenable the AMD IOMMU if it's mysteriously vanished over suspend

[Matthew Garrett]

On Thu, Sep 30, 2010 at 11:38:28AM -0400, Matthew Garrett wrote:
> AMD's reference BIOS code had a bug that could result in the firmware
> failing to reenable the iommu on resume. It transpires that this causes
> certain less than desirable behaviour when it comes to PCI accesses, to
> whit them ending up somewhere near Bristol when the more desirable outcome
> was Edinburgh. Sadness ensues, perhaps along with filesystem corruption.
> Let's make sure that it gets turned back on.

Of course, this isn't actually sufficient - nothing in the iommu code 
appears to reprogram the BAR, for instance. Joerg, how is this meant to 
work?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] pci: Reenable the AMD IOMMU if it's mysteriously vanished over suspend

[Joerg Roedel]

On Thu, Sep 30, 2010 at 06:39:45PM +0100, Matthew Garrett wrote:
> On Thu, Sep 30, 2010 at 11:38:28AM -0400, Matthew Garrett wrote:
> > AMD's reference BIOS code had a bug that could result in the firmware
> > failing to reenable the iommu on resume. It transpires that this causes
> > certain less than desirable behaviour when it comes to PCI accesses, to
> > whit them ending up somewhere near Bristol when the more desirable outcome
> > was Edinburgh. Sadness ensues, perhaps along with filesystem corruption.
> > Let's make sure that it gets turned back on.
> 
> Of course, this isn't actually sufficient - nothing in the iommu code 
> appears to reprogram the BAR, for instance. Joerg, how is this meant to 
> work?

Enabling the IOMMU device is only part of what the BIOS is supposed to
do to get the IOMMU ready for the OS. A handful of registers in the
config space of the device and the IOMMU MMIO region must also be
restored. Thats the simple part. The IOMMU device also spans two
indirect register spaces to configure the IOMMU caches. I have no idea
yet how this need to be configured. The plan is definitly to
work-around a missing IOMMU device in the IOMMU driver. But I am still
talking to some people to find out what exactly must be done, especially
with the indirect register spaces.


	Joerg

Re: [PATCH] pci: Reenable the AMD IOMMU if it's mysteriously vanished over suspend

[Matthew Garrett]

On Thu, Sep 30, 2010 at 08:43:01PM +0200, Joerg Roedel wrote:

> Enabling the IOMMU device is only part of what the BIOS is supposed to
> do to get the IOMMU ready for the OS. A handful of registers in the
> config space of the device and the IOMMU MMIO region must also be
> restored. Thats the simple part. The IOMMU device also spans two
> indirect register spaces to configure the IOMMU caches. I have no idea
> yet how this need to be configured. The plan is definitly to
> work-around a missing IOMMU device in the IOMMU driver. But I am still
> talking to some people to find out what exactly must be done, especially
> with the indirect register spaces.

Chapter 8 of the register setup guide seems to have something on this - 
I'll try saving and restoring all of the setup it performs to see if it 
improves things.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] pci: Reenable the AMD IOMMU if it's mysteriously vanished over suspend

[Bjorn Helgaas]

On Thursday, September 30, 2010 09:38:28 am Matthew Garrett wrote:
> AMD's reference BIOS code had a bug that could result in the firmware
> failing to reenable the iommu on resume. It transpires that this causes
> certain less than desirable behaviour when it comes to PCI accesses, to
> whit them ending up somewhere near Bristol when the more desirable outcome
> was Edinburgh. Sadness ensues, perhaps along with filesystem corruption.
> Let's make sure that it gets turned back on.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> ---
>  drivers/pci/quirks.c |   33 +++++++++++++++++++++++++++++++++
>  1 files changed, 33 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 89ed181..dcf9832 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -2695,6 +2695,39 @@ DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_RICOH, PCI_DEVICE_ID_RICOH_R5C832,
>  #endif /*CONFIG_MMC_RICOH_MMC*/
>  
>  
> +#ifdef CONFIG_AMD_IOMMU
> +
> +/* Some AMD BIOSes fail to reenable the iommu on resume */
> +
> +static void amd_iommu_reenable(struct pci_dev *dev)
> +{
> +	u32 ioc_feature_control;
> +	struct pci_dev *iommu;
> +
> +	iommu = pci_get_device(PCI_VENDOR_ID_ATI, 0x5a23, NULL);
> +
> +	if (!iommu)
> +		return;
> +
> +	/* Select Northbridge indirect register 0x75 and enable writing */
> +	pci_write_config_dword(dev, 0x60, 0x75 | (1 << 7));
> +	pci_read_config_dword(dev, 0x64, &ioc_feature_control);
> +
> +	/* Enable the iommu if it's vanished */
> +	if (!(ioc_feature_control & 0x1))
> +		pci_write_config_dword(dev, 0x64, ioc_feature_control | 1);

This sounds important and tricky enough that it'd be nice to have a
note in dmesg about what we're doing.

> +	/* Disable writing again */
> +	pci_write_config_dword(dev, 0x60, 0x75);
> +
> +	pci_dev_put(iommu);
> +}
> +
> +DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_ATI, 0x5a10, amd_iommu_reenable);
> +DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_ATI, 0x5a12, amd_iommu_reenable);
> +DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_ATI, 0x5a13, amd_iommu_reenable);
> +#endif
> +
>  static void pci_do_fixups(struct pci_dev *dev, struct pci_fixup *f,
>  			  struct pci_fixup *end)
>  {
>