diff mbox

[RFC] Fix Intel IOMMU support for Marvell 88SE91xx SATA controllers.

Message ID 5127C716.6050903@linux-ipv6.be (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Stijn Tintel Feb. 22, 2013, 7:29 p.m. UTC 
On 19-12-12 11:58, Andrew Cooks wrote:
> This is my second attempt to make Marvell 88SE91xx SATA controllers work when IOMMU is enabled.[1][2] 
> As suggested, it no longer tries to add support for phantom functions.
> 
> What's missing:
> * No AMD support. I need some help with this.
> * Table of affected chip IDs is incomplete. I think 0x9123, 0x9125, 0x9128 are also affected.
This one is also affected: 08:00.0 0106: 1b4b:9130 (rev 11), this is in
dmesg:

[    1.914086] dmar: DMAR:[DMA Read] Request device [08:00.1] fault addr
fff00000
> 
> Patch is against 3.7.1
> 
> Review and feedback would be appreciated.
I changed your patch to check for my chip ID, and it solves my problem:
no more hang during boot, and the HDD connected to this controller is
now detected with IOMMU enabled.

Also see 1 minor comment inline.
> 
> 1. https://bugzilla.redhat.com/show_bug.cgi?id=757166
> 2. https://bugzilla.kernel.org/show_bug.cgi?id=42679
> 
> Signed-off-by: Andrew Cooks <acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/iommu/intel-iommu.c |   36 ++++++++++++++++++++++++++++++++++--
>  drivers/pci/quirks.c        |   34 ++++++++++++++++++++++++++++++++++
>  include/linux/pci.h         |    1 +
>  3 files changed, 66 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 0badfa4..17e64c0 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -1672,6 +1674,31 @@ static int domain_context_mapping_one(struct dmar_domain *domain, int segment,
>  	return 0;
>  }
>  
> +/* For buggy devices like Marvell 88SE91xx chips that use unclaimed
> + * functions.
> + */
> +static int map_quirky_dma_fn(struct dmar_domain *domain,
> +		struct pci_dev *pdev,
> +		int translation)
> +{
> +	u8 fn;
> +	int err = 0;
> +
> +	for (fn = 1; fn < 8; fn++) {
> +		if (pci_func_is_dma_source(pdev, fn)) {
> +			err = domain_context_mapping_one(domain,
> +					pci_domain_nr(pdev->bus),
> +					pdev->bus->number,
> +					PCI_DEVFN(PCI_SLOT(pdev->devfn), fn),
> +					translation);
> +			if (err)
> +				return err;
> +			dev_dbg(&pdev->dev, "func: %d mapped dma quirk", fn);
> +		}
> +	}
> +	return 0;
> +}
> +
>  static int
>  domain_context_mapping(struct dmar_domain *domain, struct pci_dev *pdev,
>  			int translation)
> @@ -1685,6 +1712,11 @@ domain_context_mapping(struct dmar_domain *domain, struct pci_dev *pdev,
>  	if (ret)
>  		return ret;
>  
> +	/* quirk for undeclared pci functions */
> +	ret = map_quirky_dma_fn(domain, pdev, translation);
> +	if (ret)
> +		return ret;
> +
>  	/* dependent device mapping */
>  	tmp = pci_find_upstream_pcie_bridge(pdev);
>  	if (!tmp)
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 7a451ff..8d02bac 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3227,6 +3227,40 @@ static const struct pci_dev_dma_source {
>  	{ 0 }
>  };
>  
> +static const struct pci_dev_dma_source_funcs {
> +	u16 vendor;
> +	u16 device;
> +	u8 func_map;	/* bit map. lsb is fn 0. */
> +} pci_dev_dma_source_funcs[] = {
> +	 {0x1b4b, 0x9172, (1<<0)|(1<<1)},
May I suggest to apply the attached patch first, and check for
PCI_VENDOR_ID_MARVELL_2 instead?
> +	 { 0 },	
> +};
> +
> +static u8 pci_get_dma_source_map(struct pci_dev *dev)
> +{
> +	const struct pci_dev_dma_source_funcs *i;
> +
> +	for (i = pci_dev_dma_source_funcs; i->func_map; i++) {
> +		if ((i->vendor == dev->vendor ||
> +		     i->vendor == (u16)PCI_ANY_ID) &&
> +		    (i->device == dev->device ||
> +		     i->device == (u16)PCI_ANY_ID)) {
> +			return i->func_map;			
> +		}
> +	}
> +	return 0;
> +}
> +
> +int pci_func_is_dma_source(struct pci_dev *dev, int fn)
> +{
> +	u8 fn_map = pci_get_dma_source_map(dev);
> +
> +	if (fn_map & (1 << fn))
> +		return 1;
> +
> +	return 0;
> +}
> +
>  /*
>   * IOMMUs with isolation capabilities need to be programmed with the
>   * correct source ID of a device.  In most cases, the source ID matches
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index ee21795..8f0fa7f 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1546,6 +1546,7 @@ enum pci_fixup_pass {
>  #ifdef CONFIG_PCI_QUIRKS
>  void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev);
>  struct pci_dev *pci_get_dma_source(struct pci_dev *dev);
> +int pci_func_is_dma_source(struct pci_dev *dev, int fn);
>  int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags);
>  #else
>  static inline void pci_fixup_device(enum pci_fixup_pass pass,
>

Comments

Andrew Cooks Feb. 25, 2013, 8:37 a.m. UTC | #1 
On Sat, Feb 23, 2013 at 3:29 AM, Stijn Tintel <stijn@linux-ipv6.be> wrote:
> On 19-12-12 11:58, Andrew Cooks wrote:
>> This is my second attempt to make Marvell 88SE91xx SATA controllers work when IOMMU is enabled.[1][2]
>> As suggested, it no longer tries to add support for phantom functions.
>>
>> What's missing:
>> * No AMD support. I need some help with this.
>> * Table of affected chip IDs is incomplete. I think 0x9123, 0x9125, 0x9128 are also affected.
> This one is also affected: 08:00.0 0106: 1b4b:9130 (rev 11), this is in
> dmesg:
>
> [    1.914086] dmar: DMAR:[DMA Read] Request device [08:00.1] fault addr
> fff00000

Handling specific chip revisions is another issue I'm not sure how to
handle. We could use another bitmap field, but we need more vendor
cooperation to know exactly which chip model and revision combinations
are affected.

>>
>> Patch is against 3.7.1
>>
>> Review and feedback would be appreciated.
> I changed your patch to check for my chip ID, and it solves my problem:
> no more hang during boot, and the HDD connected to this controller is
> now detected with IOMMU enabled.
>
> Also see 1 minor comment inline.

Thanks for the feedback!  I'll include your PCI_VENDOR_ID change in
the next iteration of the patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

commit 05183aa996b2971884a60041e995b470b8224574
Author: Stijn Tintel <stijn@linux-ipv6.be>
Date:   Sun Feb 10 01:01:16 2013 +0100

    Move PCI_VENDOR_ID_MARVELL_2 to pci_ids.h
    
    Signed-off-by: Stijn Tintel <stijn@linux-ipv6.be>

diff --git a/drivers/scsi/mvumi.h b/drivers/scsi/mvumi.h
index e360135..41f1687 100644
--- a/drivers/scsi/mvumi.h
+++ b/drivers/scsi/mvumi.h
@@ -32,7 +32,6 @@ 
 #define VER_BUILD		1500
 
 #define MV_DRIVER_NAME			"mvumi"
-#define PCI_VENDOR_ID_MARVELL_2		0x1b4b
 #define PCI_DEVICE_ID_MARVELL_MV9143	0x9143
 #define PCI_DEVICE_ID_MARVELL_MV9580	0x9580
 
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 0eb6579..60c4ff4 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -1608,6 +1608,7 @@ 
 #define PCI_DEVICE_ID_MARVELL_GT64260	0x6430
 #define PCI_DEVICE_ID_MARVELL_MV64360	0x6460
 #define PCI_DEVICE_ID_MARVELL_MV64460	0x6480
+#define PCI_VENDOR_ID_MARVELL_2		0x1b4b
 #define PCI_DEVICE_ID_MARVELL_88ALP01_NAND	0x4100
 #define PCI_DEVICE_ID_MARVELL_88ALP01_SD	0x4101
 #define PCI_DEVICE_ID_MARVELL_88ALP01_CCIC	0x4102