* [PATCH 4/4] create context mapping entries for devices that use phantom functions. [not found] ` <1354533387-4110-1-git-send-email-acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2012-12-03 11:16 ` Andrew Cooks 2012-12-19 10:58 ` [RFC PATCH] Fix Intel IOMMU support for Marvell 88SE91xx SATA controllers Andrew Cooks 1 sibling, 0 replies; 17+ messages in thread From: Andrew Cooks @ 2012-12-03 11:16 UTC (permalink / raw) To: Bjorn Helgaas, jgarzik-e+AXbWqSrlAAvxtiuMwx3w, Joerg-i9wRM+HIrmnmtl4Z8vJ8Kg761KYD1DLY, Roedel-i9wRM+HIrmnmtl4Z8vJ8Kg761KYD1DLY, joro-zLv9SwRftAIdnm+yROfE0A, Justin-i9wRM+HIrmnmtl4Z8vJ8Kg761KYD1DLY, Piszcz-i9wRM+HIrmnmtl4Z8vJ8Kg761KYD1DLY, jpiszcz-BP4nVm5VUdNhbmWW9KSYcQ, Robert-i9wRM+HIrmnmtl4Z8vJ8Kg761KYD1DLY, Hancock-i9wRM+HIrmnmtl4Z8vJ8Kg761KYD1DLY, hancockrwd-Re5JQEeQqe8AvxtiuMwx3w, YingChu Cc: open list:INTEL IOMMU VT-d, David Woodhouse, open list Create context mapping for phantom functions. Signed-off-by: Andrew Cooks <acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> --- drivers/iommu/intel-iommu.c | 30 ++++++++++++++++++++++++++++++ 1 files changed, 30 insertions(+), 0 deletions(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 554e6ac..b3c9b55 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -1673,6 +1673,32 @@ static int domain_context_mapping_one(struct dmar_domain *domain, int segment, } static int +context_map_phantoms(struct dmar_domain *domain, struct pci_dev *pdev, + int translation) +{ + unsigned fn; + int ret; + + if (!pdev->phantoms_enabled) + return 0; + + pr_debug("context_map_phantoms()\n"); + + for (fn = 1 ; fn < 8 ; fn++) { + ret = domain_context_mapping_one(domain, + pci_domain_nr(pdev->bus), + pdev->bus->number, + fn, + translation); + if (ret) { + pr_info("function %d phantom mapping failed.\n", fn); + return ret; + } + } + return 0; +} + +static int domain_context_mapping(struct dmar_domain *domain, struct pci_dev *pdev, int translation) { @@ -1685,6 +1711,10 @@ domain_context_mapping(struct dmar_domain *domain, struct pci_dev *pdev, if (ret) return ret; + ret = context_map_phantoms(domain, pdev, translation); + if (ret) + return ret; + /* dependent device mapping */ tmp = pci_find_upstream_pcie_bridge(pdev); if (!tmp) -- 1.7.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH] Fix Intel IOMMU support for Marvell 88SE91xx SATA controllers. [not found] ` <1354533387-4110-1-git-send-email-acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2012-12-03 11:16 ` [PATCH 4/4] create context mapping entries for devices that use phantom functions Andrew Cooks @ 2012-12-19 10:58 ` Andrew Cooks 2012-12-19 13:41 ` Chu Ying ` (2 more replies) 1 sibling, 3 replies; 17+ messages in thread From: Andrew Cooks @ 2012-12-19 10:58 UTC (permalink / raw) To: acooks-Re5JQEeQqe8AvxtiuMwx3w, joro-zLv9SwRftAIdnm+yROfE0A, xjtuychu-PkbjNfxxIARBDgjK7y7TUQ, gm.ychu-Re5JQEeQqe8AvxtiuMwx3w, alex.williamson-H+wXaHxf7aLQT0dZR+AlfA, bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, jpiszcz-BP4nVm5VUdNhbmWW9KSYcQ, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ Cc: open list:PCI SUBSYSTEM, open list:INTEL IOMMU VT-d, open list 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. Patch is against 3.7.1 Review and feedback would be appreciated. 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)}, + { 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, -- 1.7.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [RFC PATCH] Fix Intel IOMMU support for Marvell 88SE91xx SATA controllers. 2012-12-19 10:58 ` [RFC PATCH] Fix Intel IOMMU support for Marvell 88SE91xx SATA controllers Andrew Cooks @ 2012-12-19 13:41 ` Chu Ying [not found] ` <A1185FFE-6B90-4B44-BF8C-082E487AA73D-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> [not found] ` <1355914703-28576-1-git-send-email-acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2013-03-01 8:26 ` [PATCH] Quirk to support Marvell 88SE91xx SATA controllers with Intel IOMMU Andrew Cooks 2 siblings, 1 reply; 17+ messages in thread From: Chu Ying @ 2012-12-19 13:41 UTC (permalink / raw) To: Andrew Cooks Cc: joro@8bytes.org, xjtuychu@hotmail.com, alex.williamson@redhat.com, bhelgaas@google.com, jpiszcz@lucidpixels.com, dwmw2@infradead.org, open list:INTEL IOMMU (VT-d), open list, open list:PCI SUBSYSTEM On 2012-12-19, at 18:58, Andrew Cooks <acooks@gmail.com> 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. That's not that simple, those devices with 2 functions( one for sata and the other for pata) work well under Intel IOMMU, so I need comfirm what devices should be involved the latest patch. > Patch is against 3.7.1 > > Review and feedback would be appreciated. > > 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@gmail.com> > --- > 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)}, > + { 0 }, > +}; Can you confirm function 0 should be marked? Per my PCIe trace, I found no function 0 involved in DMA access? > +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, > -- > 1.7.1 > ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <A1185FFE-6B90-4B44-BF8C-082E487AA73D-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [RFC PATCH] Fix Intel IOMMU support for Marvell 88SE91xx SATA controllers. [not found] ` <A1185FFE-6B90-4B44-BF8C-082E487AA73D-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2012-12-19 15:07 ` Andrew Cooks [not found] ` <CAJtEV7ZmgkJMhFL_2Qzt1YsKnZ40gNi0yHgixVW3yJEfi4QzfA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Andrew Cooks @ 2012-12-19 15:07 UTC (permalink / raw) To: Chu Ying Cc: open list, open list:INTEL IOMMU (VT-d), xjtuychu-PkbjNfxxIARBDgjK7y7TUQ@public.gmane.org, jpiszcz-BP4nVm5VUdNhbmWW9KSYcQ@public.gmane.org, open list:PCI SUBSYSTEM, bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org [-- Attachment #1: Type: text/plain, Size: 829 bytes --] On Wed, Dec 19, 2012 at 9:41 PM, Chu Ying <gm.ychu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > On 2012-12-19, at 18:58, Andrew Cooks <acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > >> This is my second attempt to make Marvell 88SE91xx SATA controllers work when IOMMU is enabled.[1][2] >> +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)}, >> + { 0 }, >> +}; > > Can you confirm function 0 should be marked? Per my PCIe trace, I found no function 0 involved in DMA access? > Yes. The attached patch disables function 0 for the 0x9172 device. The attached dmesg output shows the resulting change at time 1.920163 and fault at time 2.265757. [-- Attachment #2: dmesg_iommu_on_func_1_only.gz --] [-- Type: application/x-gzip, Size: 44752 bytes --] [-- Attachment #3: func_1_only.patch --] [-- Type: application/octet-stream, Size: 845 bytes --] diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 17e64c0..2fa2124 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -1706,11 +1706,16 @@ domain_context_mapping(struct dmar_domain *domain, struct pci_dev *pdev, int ret; struct pci_dev *tmp, *parent; - ret = domain_context_mapping_one(domain, pci_domain_nr(pdev->bus), - pdev->bus->number, pdev->devfn, - translation); - if (ret) - return ret; + if (pdev->vendor == 0x1b4b && pdev->device == 0x9172) { + dev_dbg(&pdev->dev, "skipping function 0."); + } else { + ret = domain_context_mapping_one(domain, + pci_domain_nr(pdev->bus), + pdev->bus->number, pdev->devfn, + translation); + if (ret) + return ret; + } /* quirk for undeclared pci functions */ ret = map_quirky_dma_fn(domain, pdev, translation); [-- Attachment #4: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply related [flat|nested] 17+ messages in thread
[parent not found: <CAJtEV7ZmgkJMhFL_2Qzt1YsKnZ40gNi0yHgixVW3yJEfi4QzfA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [RFC PATCH] Fix Intel IOMMU support for Marvell 88SE91xx SATA controllers. [not found] ` <CAJtEV7ZmgkJMhFL_2Qzt1YsKnZ40gNi0yHgixVW3yJEfi4QzfA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2012-12-20 2:55 ` Ying Chu 0 siblings, 0 replies; 17+ messages in thread From: Ying Chu @ 2012-12-20 2:55 UTC (permalink / raw) To: Andrew Cooks Cc: open list, open list:INTEL IOMMU (VT-d), xjtuychu-PkbjNfxxIARBDgjK7y7TUQ@public.gmane.org, jpiszcz-BP4nVm5VUdNhbmWW9KSYcQ@public.gmane.org, open list:PCI SUBSYSTEM, bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org [-- Attachment #1.1: Type: text/plain, Size: 952 bytes --] Ack. 2012/12/19 Andrew Cooks <acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > On Wed, Dec 19, 2012 at 9:41 PM, Chu Ying <gm.ychu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > > On 2012-12-19, at 18:58, Andrew Cooks <acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > > > >> This is my second attempt to make Marvell 88SE91xx SATA controllers > work when IOMMU is enabled.[1][2] > >> +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)}, > >> + { 0 }, > >> +}; > > > > Can you confirm function 0 should be marked? Per my PCIe trace, I found > no function 0 involved in DMA access? > > > > Yes. The attached patch disables function 0 for the 0x9172 device. The > attached dmesg output shows the resulting change at time 1.920163 and > fault at time 2.265757. > [-- Attachment #1.2: Type: text/html, Size: 1493 bytes --] [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <1355914703-28576-1-git-send-email-acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [RFC PATCH] Fix Intel IOMMU support for Marvell 88SE91xx SATA controllers. [not found] ` <1355914703-28576-1-git-send-email-acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2013-02-22 19:29 ` Stijn Tintel [not found] ` <5127C716.6050903-VfPWfsRibaPZj6PxcwrBaQ@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Stijn Tintel @ 2013-02-22 19:29 UTC (permalink / raw) To: Andrew Cooks Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, xjtuychu-PkbjNfxxIARBDgjK7y7TUQ, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, jpiszcz-BP4nVm5VUdNhbmWW9KSYcQ, linux-pci-u79uwXL29TY76Z2rM5mHXA, bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ [-- Attachment #1: Type: text/plain, Size: 4554 bytes --] 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-XMD5yJDbdMReXY1tMh2IBg@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, > [-- Attachment #2: move_pci_vendor_id_marvell_2.patch --] [-- Type: text/x-patch, Size: 1147 bytes --] commit 05183aa996b2971884a60041e995b470b8224574 Author: Stijn Tintel <stijn-VfPWfsRibaPZj6PxcwrBaQ@public.gmane.org> 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-VfPWfsRibaPZj6PxcwrBaQ@public.gmane.org> 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 [-- Attachment #3: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply related [flat|nested] 17+ messages in thread
[parent not found: <5127C716.6050903-VfPWfsRibaPZj6PxcwrBaQ@public.gmane.org>]
* Re: [RFC PATCH] Fix Intel IOMMU support for Marvell 88SE91xx SATA controllers. [not found] ` <5127C716.6050903-VfPWfsRibaPZj6PxcwrBaQ@public.gmane.org> @ 2013-02-25 8:37 ` Andrew Cooks 0 siblings, 0 replies; 17+ messages in thread From: Andrew Cooks @ 2013-02-25 8:37 UTC (permalink / raw) To: Stijn Tintel Cc: open list, YingChu, open list:INTEL IOMMU (VT-d), Justin Piszcz, open list:PCI SUBSYSTEM, bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org, David Woodhouse On Sat, Feb 23, 2013 at 3:29 AM, Stijn Tintel <stijn-VfPWfsRibaPZj6PxcwrBaQ@public.gmane.org> 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] Quirk to support Marvell 88SE91xx SATA controllers with Intel IOMMU. 2012-12-19 10:58 ` [RFC PATCH] Fix Intel IOMMU support for Marvell 88SE91xx SATA controllers Andrew Cooks 2012-12-19 13:41 ` Chu Ying [not found] ` <1355914703-28576-1-git-send-email-acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2013-03-01 8:26 ` Andrew Cooks 2013-03-01 17:54 ` Justin Piszcz [not found] ` <1362126373-32318-1-git-send-email-acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2 siblings, 2 replies; 17+ messages in thread From: Andrew Cooks @ 2013-03-01 8:26 UTC (permalink / raw) To: acooks, joro, xjtuychu, gm.ychu, alex.williamson, bhelgaas, jpiszcz, dwmw2 Cc: open list:INTEL IOMMU VT-d, open list, open list:PCI SUBSYSTEM This is my third submitted patch to make Marvell 88SE91xx SATA controllers work when IOMMU is enabled.[1][2] What's changed: * Adopt David Woodhouse's terminology by referring to the quirky functions as 'ghost' functions. * Unmap ghost functions when device is detached from IOMMU. * Stub function for when CONFIG_PCI_QUIRKS is not enabled. The bad: * Still no AMD support. * The table of affected chip IDs is as complete as I can make it by googling for bug reports. This patch was generated against commit b0af9cd9aab60ceb17d3ebabb9fdf4ff0a99cf50, but will also apply cleanly to 3.7.10. Bug reports: 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@gmail.com> --- drivers/iommu/intel-iommu.c | 50 +++++++++++++++++++++++++++++++++++++++++++ drivers/pci/quirks.c | 47 +++++++++++++++++++++++++++++++++++++++- include/linux/pci.h | 5 ++++ include/linux/pci_ids.h | 1 + 4 files changed, 102 insertions(+), 1 deletions(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 0099667..13323f2 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -1674,6 +1674,50 @@ static int domain_context_mapping_one(struct dmar_domain *domain, int segment, return 0; } +/* For quirky devices like Marvell 88SE91xx chips that use ghost functions. */ +static int map_ghost_dma_fn(struct dmar_domain *domain, + struct pci_dev *pdev, + int translation) +{ + u8 fn, fn_map; + int err = 0; + + fn_map = pci_get_dma_source_map(pdev); + + for (fn = 1; fn < 8; fn++) { + if (fn_map & (1 << 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, "dma quirk; func %d mapped", fn); + } + } + return 0; +} + +static void iommu_detach_dev(struct intel_iommu *iommu, u8 bus, u8 devfn); + +static void unmap_ghost_dma_fn(struct intel_iommu *iommu, + struct pci_dev *pdev) +{ + u8 fn, fn_map; + + fn_map = pci_get_dma_source_map(pdev); + + for (fn = 1; fn < 8; fn++) { + if (fn_map & (1 << fn)) { + iommu_detach_dev(iommu, + pdev->bus->number, + PCI_DEVFN(PCI_SLOT(pdev->devfn), fn)); + dev_dbg(&pdev->dev, "dma quirk; func %d unmapped", fn); + } + } +} + static int domain_context_mapping(struct dmar_domain *domain, struct pci_dev *pdev, int translation) @@ -1687,6 +1731,11 @@ domain_context_mapping(struct dmar_domain *domain, struct pci_dev *pdev, if (ret) return ret; + /* quirk for undeclared/ghost pci functions */ + ret = map_ghost_dma_fn(domain, pdev, translation); + if (ret) + return ret; + /* dependent device mapping */ tmp = pci_find_upstream_pcie_bridge(pdev); if (!tmp) @@ -3786,6 +3835,7 @@ static void domain_remove_one_dev_info(struct dmar_domain *domain, iommu_disable_dev_iotlb(info); iommu_detach_dev(iommu, info->bus, info->devfn); iommu_detach_dependent_devices(iommu, pdev); + unmap_ghost_dma_fn(iommu, pdev); free_devinfo_mem(info); spin_lock_irqsave(&device_domain_lock, flags); diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index 0369fb6..d311100 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -3249,6 +3249,10 @@ static struct pci_dev *pci_func_0_dma_source(struct pci_dev *dev) return pci_get_slot(dev->bus, PCI_DEVFN(PCI_SLOT(dev->devfn), 0)); } +/* Table of source functions for real devices. The DMA requests for the + * device are tagged with a different real function as source. This is + * relevant to multifunction devices. + */ static const struct pci_dev_dma_source { u16 vendor; u16 device; @@ -3275,7 +3279,8 @@ static const struct pci_dev_dma_source { * the device doing the DMA, but sometimes hardware is broken and will * tag the DMA as being sourced from a different device. This function * allows that translation. Note that the reference count of the - * returned device is incremented on all paths. + * returned device is incremented on all paths. Translation is done when + * the device is added to an IOMMU group. */ struct pci_dev *pci_get_dma_source(struct pci_dev *dev) { @@ -3292,6 +3297,46 @@ struct pci_dev *pci_get_dma_source(struct pci_dev *dev) return pci_dev_get(dev); } +/* Table of multiple (ghost) source functions. This is similar to the + * translated sources above, but with the following differences: + * 1. the device may use multiple functions as DMA sources, + * 2. these functions cannot be assumed to be actual devices, + * 3. the specific ghost function for a request can not be exactly predicted. + * The bitmap only contains the additional quirk functions. + */ +static const struct pci_dev_dma_multi_func_sources { + u16 vendor; + u16 device; + u8 func_map; /* bit map. lsb is fn 0. */ +} pci_dev_dma_multi_func_sources[] = { + { PCI_VENDOR_ID_MARVELL_2, 0x9123, (1<<0)|(1<<1)}, + { PCI_VENDOR_ID_MARVELL_2, 0x9125, (1<<0)|(1<<1)}, + { PCI_VENDOR_ID_MARVELL_2, 0x9128, (1<<0)|(1<<1)}, + { PCI_VENDOR_ID_MARVELL_2, 0x9130, (1<<0)|(1<<1)}, + { PCI_VENDOR_ID_MARVELL_2, 0x9172, (1<<0)|(1<<1)}, + { 0 } +}; + +/* + * The mapping of fake/ghost functions is used when the real device is + * attached to an IOMMU domain. IOMMU groups are not aware of these + * functions, because they're not real devices. + */ +u8 pci_get_dma_source_map(struct pci_dev *dev) +{ + const struct pci_dev_dma_multi_func_sources *i; + + for (i = pci_dev_dma_multi_func_sources; 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; +} + static const struct pci_dev_acs_enabled { u16 vendor; u16 device; diff --git a/include/linux/pci.h b/include/linux/pci.h index 2461033..5ad3822 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1578,6 +1578,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); +u8 pci_get_dma_source_map(struct pci_dev *dev); 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, @@ -1586,6 +1587,10 @@ static inline struct pci_dev *pci_get_dma_source(struct pci_dev *dev) { return pci_dev_get(dev); } +u8 pci_get_dma_source_map(struct pci_dev *dev) +{ + return 0; +} static inline int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags) { diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index f11c1c2..df57496 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -1604,6 +1604,7 @@ #define PCI_SUBDEVICE_ID_KEYSPAN_SX2 0x5334 #define PCI_VENDOR_ID_MARVELL 0x11ab +#define PCI_VENDOR_ID_MARVELL_2 0x1b4b #define PCI_DEVICE_ID_MARVELL_GT64111 0x4146 #define PCI_DEVICE_ID_MARVELL_GT64260 0x6430 #define PCI_DEVICE_ID_MARVELL_MV64360 0x6460 -- 1.7.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* RE: [PATCH] Quirk to support Marvell 88SE91xx SATA controllers with Intel IOMMU. 2013-03-01 8:26 ` [PATCH] Quirk to support Marvell 88SE91xx SATA controllers with Intel IOMMU Andrew Cooks @ 2013-03-01 17:54 ` Justin Piszcz [not found] ` <1362126373-32318-1-git-send-email-acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 1 sibling, 0 replies; 17+ messages in thread From: Justin Piszcz @ 2013-03-01 17:54 UTC (permalink / raw) To: 'Andrew Cooks', joro, xjtuychu, gm.ychu, alex.williamson, bhelgaas, dwmw2 Cc: 'open list:INTEL IOMMU (VT-d)', 'open list', 'open list:PCI SUBSYSTEM' -----Original Message----- From: Andrew Cooks [mailto:acooks@gmail.com] Sent: Friday, March 01, 2013 3:26 AM To: acooks@gmail.com; joro@8bytes.org; xjtuychu@hotmail.com; gm.ychu@gmail.com; alex.williamson@redhat.com; bhelgaas@google.com; jpiszcz@lucidpixels.com; dwmw2@infradead.org Cc: open list:INTEL IOMMU (VT-d); open list; open list:PCI SUBSYSTEM Subject: [PATCH] Quirk to support Marvell 88SE91xx SATA controllers with Intel IOMMU. This is my third submitted patch to make Marvell 88SE91xx SATA controllers work when IOMMU is enabled.[1][2] What's changed: * Adopt David Woodhouse's terminology by referring to the quirky functions as 'ghost' functions. * Unmap ghost functions when device is detached from IOMMU. * Stub function for when CONFIG_PCI_QUIRKS is not enabled. The bad: * Still no AMD support. * The table of affected chip IDs is as complete as I can make it by googling for bug reports. This patch was generated against commit b0af9cd9aab60ceb17d3ebabb9fdf4ff0a99cf50, but will also apply cleanly to 3.7.10. -- Hi, Against 3.7.10: # patch -p1 < ../RFC-Fix-Intel-IOMMU-support-for-Marvell-88SE91xx-SATA-controllers..patch patching file drivers/iommu/intel-iommu.c patching file drivers/pci/quirks.c Hunk #1 succeeded at 3230 (offset 3 lines). patching file include/linux/pci.h # Recompile kernel, reboot.. Shutdown host, re-attach to Marvell Controller w/IOMMU. The host still failed to boot, dmesg/panic here: http://home.comcast.net/~jpiszcz/20130301/boot_failure.JPG (The root disk is /dev/sdc) I recompiled again with IOMMU off and it booted ok: # uname -a Linux host 3.7.10 #2 SMP Fri Mar 1 12:44:25 EST 2013 x86_64 GNU/Linux Here is the part of dmesg (what it looks like when it succeeds with IOMMU=off) [ 4.288113] input: American Megatrends Inc. Virtual Keyboard and Mouse as /devices/pci0000:00/0000:00:1a.1/usb4/4-2/4-2:1.0/input/input3 [ 4.289025] hid-generic 0003:046B:FF10.0001: input,hidraw0: USB HID v1.10 Keyboard [American Megatrends Inc. Virtual Keyboard and Mouse] on usb-0000:00:1a.1-2/input0 [ 4.305993] input: American Megatrends Inc. Virtual Keyboard and Mouse as /devices/pci0000:00/0000:00:1a.1/usb4/4-2/4-2:1.1/input/input4 [ 4.307106] hid-generic 0003:046B:FF10.0002: input,hidraw1: USB HID v1.10 Mouse [American Megatrends Inc. Virtual Keyboard and Mouse] on usb-0000:00:1a.1-2/input1 [ 4.326481] ata6: SATA link down (SStatus 0 SControl 300) [ 4.327324] scsi 7:0:0:0: Direct-Access ATA INTEL SSDSC2MH25 PWG4 PQ: 0 ANSI: 5 [ 4.329953] sd 7:0:0:0: [sdc] 488397168 512-byte logical blocks: (250 GB/232 GiB) [ 4.330639] scsi 14:0:0:0: Processor Marvell 91xx Config 1.01 PQ: 0 ANSI: 5 [ 4.333276] sd 7:0:0:0: [sdc] Write Protect is off [ 4.334746] sd 7:0:0:0: [sdc] Mode Sense: 00 3a 00 00 [ 4.334921] sd 7:0:0:0: [sdc] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA [ 4.345622] sdc: sdc1 sdc2 [ 4.347493] sd 7:0:0:0: [sdc] Attached SCSI disk Justin. ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <1362126373-32318-1-git-send-email-acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH] Quirk to support Marvell 88SE91xx SATA controllers with Intel IOMMU. [not found] ` <1362126373-32318-1-git-send-email-acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2013-03-01 17:51 ` Justin Piszcz 2013-03-01 22:19 ` Andrew Cooks 2013-03-06 4:04 ` Alex Williamson 1 sibling, 1 reply; 17+ messages in thread From: Justin Piszcz @ 2013-03-01 17:51 UTC (permalink / raw) To: Andrew Cooks Cc: gm.ychu-Re5JQEeQqe8AvxtiuMwx3w, open list:INTEL IOMMU (VT-d), xjtuychu-PkbjNfxxIARBDgjK7y7TUQ, open list:PCI SUBSYSTEM, bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ, open list [-- Attachment #1.1: Type: text/plain, Size: 2559 bytes --] On Fri, Mar 1, 2013 at 3:26 AM, Andrew Cooks <acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > This is my third submitted patch to make Marvell 88SE91xx SATA controllers > work when IOMMU is enabled.[1][2] > > What's changed: > * Adopt David Woodhouse's terminology by referring to the quirky functions > as 'ghost' functions. > * Unmap ghost functions when device is detached from IOMMU. > * Stub function for when CONFIG_PCI_QUIRKS is not enabled. > > The bad: > * Still no AMD support. > * The table of affected chip IDs is as complete as I can make it by > googling for bug reports. > > > Hi, Against 3.7.10: # patch -p1 < ../RFC-Fix-Intel-IOMMU-support-for-Marvell-88SE91xx-SATA-controllers..patch patching file drivers/iommu/intel-iommu.c patching file drivers/pci/quirks.c Hunk #1 succeeded at 3230 (offset 3 lines). patching file include/linux/pci.h # Recompile kernel, reboot.. Shutdown host, re-attach to Marvell Controller w/IOMMU. The host still failed to boot, dmesg/panic here: http://home.comcast.net/~jpiszcz/20130301/boot_failure.JPG (The root disk is /dev/sdc) I recompiled again with IOMMU off and it booted ok: # uname -a Linux host 3.7.10 #2 SMP Fri Mar 1 12:44:25 EST 2013 x86_64 GNU/Linux Here is the part of dmesg (what it looks like when it succeeds with IOMMU=off) [ 4.288113] input: American Megatrends Inc. Virtual Keyboard and Mouse as /devices/pci0000:00/0000:00:1a.1/usb4/4-2/4-2:1.0/input/input3 [ 4.289025] hid-generic 0003:046B:FF10.0001: input,hidraw0: USB HID v1.10 Keyboard [American Megatrends Inc. Virtual Keyboard and Mouse] on usb-0000:00:1a.1-2/input0 [ 4.305993] input: American Megatrends Inc. Virtual Keyboard and Mouse as /devices/pci0000:00/0000:00:1a.1/usb4/4-2/4-2:1.1/input/input4 [ 4.307106] hid-generic 0003:046B:FF10.0002: input,hidraw1: USB HID v1.10 Mouse [American Megatrends Inc. Virtual Keyboard and Mouse] on usb-0000:00:1a.1-2/input1 [ 4.326481] ata6: SATA link down (SStatus 0 SControl 300) [ 4.327324] scsi 7:0:0:0: Direct-Access ATA INTEL SSDSC2MH25 PWG4 PQ: 0 ANSI: 5 [ 4.329953] sd 7:0:0:0: [sdc] 488397168 512-byte logical blocks: (250 GB/232 GiB) [ 4.330639] scsi 14:0:0:0: Processor Marvell 91xx Config 1.01 PQ: 0 ANSI: 5 [ 4.333276] sd 7:0:0:0: [sdc] Write Protect is off [ 4.334746] sd 7:0:0:0: [sdc] Mode Sense: 00 3a 00 00 [ 4.334921] sd 7:0:0:0: [sdc] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA [ 4.345622] sdc: sdc1 sdc2 [ 4.347493] sd 7:0:0:0: [sdc] Attached SCSI disk Justin. [-- Attachment #1.2: Type: text/html, Size: 3571 bytes --] [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Quirk to support Marvell 88SE91xx SATA controllers with Intel IOMMU. 2013-03-01 17:51 ` Justin Piszcz @ 2013-03-01 22:19 ` Andrew Cooks [not found] ` <CAJtEV7Zs2BiwJ9jdDeoceh9hiVXvVaSvj=9H+4+vEzhY72AS9g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Andrew Cooks @ 2013-03-01 22:19 UTC (permalink / raw) To: Justin Piszcz Cc: Chu Ying, open list:INTEL IOMMU (VT-d), YingChu, open list:PCI SUBSYSTEM, bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org, David Woodhouse, open list On Sat, Mar 2, 2013 at 1:51 AM, Justin Piszcz <jpiszcz-BP4nVm5VUdNhbmWW9KSYcQ@public.gmane.org> wrote: > > > On Fri, Mar 1, 2013 at 3:26 AM, Andrew Cooks <acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: >> >> This is my third submitted patch to make Marvell 88SE91xx SATA controllers >> work when IOMMU is enabled.[1][2] >> > > Hi, > > Against 3.7.10: > > # patch -p1 < > ../RFC-Fix-Intel-IOMMU-support-for-Marvell-88SE91xx-SATA-controllers..patch > patching file drivers/iommu/intel-iommu.c > patching file drivers/pci/quirks.c > Hunk #1 succeeded at 3230 (offset 3 lines). > patching file include/linux/pci.h > # Thanks for testing! Patching 3.7.10 looks somewhat different here. I'm using the linux-3.7.y branch from git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git at commit 356d8c6fb2a7cf49e836742738a8b9a47e77cfea. The output I get is: $ patch -p1 < ~/devel/lk_patches_dma_source_maps-20130301/marvell_ghost_funcs.patch patching file drivers/iommu/intel-iommu.c Hunk #1 succeeded at 1672 (offset -2 lines). Hunk #2 succeeded at 1729 (offset -2 lines). Hunk #3 succeeded at 3833 (offset -2 lines). patching file drivers/pci/quirks.c Hunk #1 succeeded at 3210 (offset -39 lines). Hunk #2 succeeded at 3240 (offset -39 lines). Hunk #3 succeeded at 3258 (offset -39 lines). patching file include/linux/pci.h Hunk #1 succeeded at 1546 (offset -32 lines). Hunk #2 succeeded at 1555 (offset -32 lines). patching file include/linux/pci_ids.h > > Recompile kernel, reboot.. > > Shutdown host, re-attach to Marvell Controller w/IOMMU. > > The host still failed to boot, dmesg/panic here: > http://home.comcast.net/~jpiszcz/20130301/boot_failure.JPG > > (The root disk is /dev/sdc) Sorry it doesn't work for you, but I can't really tell what's failing from the photo you posted. I'm running 3.7.10 with this patch right now. According to the dmesg output you posted at https://home.comcast.net/~jpiszcz/20121128/dmesg.txt and the lspci output in http://www.kernelhub.org/?p=2&msg=171877, you've run into two separate DMAR issues: Problem A - nvidia graphics: [ 2.968248] dmar: DRHD: handling fault status reg 202 [ 2.968253] dmar: DMAR:[DMA Read] Request device [04:00.0] fault addr 0 [ 2.968253] DMAR:[fault reason 06] PTE Read access is not set Problem B - Marvell 88SE9123: [ 2.974534] ata9: SATA link down (SStatus 0 SControl 300) [ 2.976297] ata7: SATA link down (SStatus 0 SControl 300) [ 2.977939] ata13: SATA link down (SStatus 0 SControl 300) [ 2.979689] ata14: SATA link up 1.5 Gbps (SStatus 113 SControl 300) [ 2.981419] dmar: DRHD: handling fault status reg 2 [ 2.981431] ata8: SATA link down (SStatus 0 SControl 300) [ 2.981449] ata11: SATA link down (SStatus 0 SControl 300) [ 2.988479] dmar: DMAR:[DMA Read] Request device [84:00.1] fault addr fff00000 [ 2.988479] DMAR:[fault reason 02] Present bit in context entry is clear Can you capture a full dmesg with this patch applied? -- a. ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <CAJtEV7Zs2BiwJ9jdDeoceh9hiVXvVaSvj=9H+4+vEzhY72AS9g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* RE: [PATCH] Quirk to support Marvell 88SE91xx SATA controllers with Intel IOMMU. [not found] ` <CAJtEV7Zs2BiwJ9jdDeoceh9hiVXvVaSvj=9H+4+vEzhY72AS9g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2013-03-01 23:18 ` Justin Piszcz 2013-03-04 1:35 ` Andrew Cooks 0 siblings, 1 reply; 17+ messages in thread From: Justin Piszcz @ 2013-03-01 23:18 UTC (permalink / raw) To: 'Andrew Cooks' Cc: 'Chu Ying', 'open list:INTEL IOMMU (VT-d)', 'YingChu', 'open list:PCI SUBSYSTEM', bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, 'David Woodhouse', 'open list' -----Original Message----- From: Andrew Cooks [mailto:acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org] Sent: Friday, March 01, 2013 5:19 PM To: Justin Piszcz Cc: Joerg Roedel; YingChu; Chu Ying; Alex Williamson; bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org; David Woodhouse; open list:INTEL IOMMU (VT-d); open list; open list:PCI SUBSYSTEM Subject: Re: [PATCH] Quirk to support Marvell 88SE91xx SATA controllers with Intel IOMMU. On Sat, Mar 2, 2013 at 1:51 AM, Justin Piszcz <jpiszcz-BP4nVm5VUdNhbmWW9KSYcQ@public.gmane.org> wrote: > > > Thanks for testing! No problem. Against a clean 3.7.10 (from ftp.kernel.org) # patch -p1 < ../patch/RFC-Fix-Intel-IOMMU-support-for-Marvell-88SE91xx-SATA-controllers.. patch patching file drivers/iommu/intel-iommu.c patching file drivers/pci/quirks.c Hunk #1 succeeded at 3230 (offset 3 lines). patching file include/linux/pci.h # pwd /usr/src/linux-3.7.10 Full dmesg with the patch applied: (but with IOMMU off) http://home.comcast.net/~jpiszcz/20130301/dmesg-full.txt Full dmesg (as much as possible through netconsole with IOMMU on) http://home.comcast.net/~jpiszcz/20130301/dmesg-iommu-on.txt Let me know if anything else is needed, thanks. Justin. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Quirk to support Marvell 88SE91xx SATA controllers with Intel IOMMU. 2013-03-01 23:18 ` Justin Piszcz @ 2013-03-04 1:35 ` Andrew Cooks [not found] ` <CAJtEV7bvPORPoXgQY2OahNVMj+SY5z=xxQ=nueX7=kLVfpyF_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Andrew Cooks @ 2013-03-04 1:35 UTC (permalink / raw) To: Justin Piszcz Cc: Joerg Roedel, YingChu, Chu Ying, Alex Williamson, bhelgaas@google.com, David Woodhouse, open list:INTEL IOMMU (VT-d), open list, open list:PCI SUBSYSTEM [-- Attachment #1: Type: text/plain, Size: 1289 bytes --] On Sat, Mar 2, 2013 at 7:18 AM, Justin Piszcz <jpiszcz@lucidpixels.com> wrote: > > Against a clean 3.7.10 (from ftp.kernel.org) > > # patch -p1 < > ../patch/RFC-Fix-Intel-IOMMU-support-for-Marvell-88SE91xx-SATA-controllers.. > patch > patching file drivers/iommu/intel-iommu.c > patching file drivers/pci/quirks.c > Hunk #1 succeeded at 3230 (offset 3 lines). > patching file include/linux/pci.h > # pwd > /usr/src/linux-3.7.10 > I've downloaded and patched the 3.7.10 tarball and still get the same output I got before; different output from yours. I'm not sure the patch is complete or applying correctly, are you? Could you please check whether the patch you're applying is the same as the attached file? > Full dmesg with the patch applied: (but with IOMMU off) > http://home.comcast.net/~jpiszcz/20130301/dmesg-full.txt > > Full dmesg (as much as possible through netconsole with IOMMU on) > http://home.comcast.net/~jpiszcz/20130301/dmesg-iommu-on.txt > > Let me know if anything else is needed, thanks. I think some important error messages might have been lost here. You captured a complete dmesg at https://home.comcast.net/~jpiszcz/20121128/dmesg.txt with iommu on. Are you still able to get the same with 3.7.10 if you exclude this patch, or has something else changed? a. [-- Attachment #2: marvell_ghost_funcs.patch --] [-- Type: application/octet-stream, Size: 7298 bytes --] From: Andrew Cooks <acooks@gmail.com> Date: Fri, 1 Mar 2013 15:25:17 +0800 Subject: [PATCH] Quirk to support Marvell 88SE91xx SATA controllers with Intel IOMMU. This is my third submitted patch to make Marvell 88SE91xx SATA controllers work when IOMMU is enabled.[1][2] What's changed: * Adopt David Woodhouse's terminology by referring to the quirky functions as 'ghost' functions. * Unmap ghost functions when device is detached from IOMMU. * Stub function for when CONFIG_PCI_QUIRKS is not enabled. The bad: * Still no AMD support. * The table of affected chip IDs is as complete as I can make it by googling for bug reports. This patch was generated against commit b0af9cd9aab60ceb17d3ebabb9fdf4ff0a99cf50, but will also apply cleanly to 3.7.10. Bug reports: 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@gmail.com> --- drivers/iommu/intel-iommu.c | 50 +++++++++++++++++++++++++++++++++++++++++++ drivers/pci/quirks.c | 47 +++++++++++++++++++++++++++++++++++++++- include/linux/pci.h | 5 ++++ include/linux/pci_ids.h | 1 + 4 files changed, 102 insertions(+), 1 deletions(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 0099667..13323f2 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -1674,6 +1674,50 @@ static int domain_context_mapping_one(struct dmar_domain *domain, int segment, return 0; } +/* For quirky devices like Marvell 88SE91xx chips that use ghost functions. */ +static int map_ghost_dma_fn(struct dmar_domain *domain, + struct pci_dev *pdev, + int translation) +{ + u8 fn, fn_map; + int err = 0; + + fn_map = pci_get_dma_source_map(pdev); + + for (fn = 1; fn < 8; fn++) { + if (fn_map & (1 << 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, "dma quirk; func %d mapped", fn); + } + } + return 0; +} + +static void iommu_detach_dev(struct intel_iommu *iommu, u8 bus, u8 devfn); + +static void unmap_ghost_dma_fn(struct intel_iommu *iommu, + struct pci_dev *pdev) +{ + u8 fn, fn_map; + + fn_map = pci_get_dma_source_map(pdev); + + for (fn = 1; fn < 8; fn++) { + if (fn_map & (1 << fn)) { + iommu_detach_dev(iommu, + pdev->bus->number, + PCI_DEVFN(PCI_SLOT(pdev->devfn), fn)); + dev_dbg(&pdev->dev, "dma quirk; func %d unmapped", fn); + } + } +} + static int domain_context_mapping(struct dmar_domain *domain, struct pci_dev *pdev, int translation) @@ -1687,6 +1731,11 @@ domain_context_mapping(struct dmar_domain *domain, struct pci_dev *pdev, if (ret) return ret; + /* quirk for undeclared/ghost pci functions */ + ret = map_ghost_dma_fn(domain, pdev, translation); + if (ret) + return ret; + /* dependent device mapping */ tmp = pci_find_upstream_pcie_bridge(pdev); if (!tmp) @@ -3786,6 +3835,7 @@ static void domain_remove_one_dev_info(struct dmar_domain *domain, iommu_disable_dev_iotlb(info); iommu_detach_dev(iommu, info->bus, info->devfn); iommu_detach_dependent_devices(iommu, pdev); + unmap_ghost_dma_fn(iommu, pdev); free_devinfo_mem(info); spin_lock_irqsave(&device_domain_lock, flags); diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index 0369fb6..d311100 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -3249,6 +3249,10 @@ static struct pci_dev *pci_func_0_dma_source(struct pci_dev *dev) return pci_get_slot(dev->bus, PCI_DEVFN(PCI_SLOT(dev->devfn), 0)); } +/* Table of source functions for real devices. The DMA requests for the + * device are tagged with a different real function as source. This is + * relevant to multifunction devices. + */ static const struct pci_dev_dma_source { u16 vendor; u16 device; @@ -3275,7 +3279,8 @@ static const struct pci_dev_dma_source { * the device doing the DMA, but sometimes hardware is broken and will * tag the DMA as being sourced from a different device. This function * allows that translation. Note that the reference count of the - * returned device is incremented on all paths. + * returned device is incremented on all paths. Translation is done when + * the device is added to an IOMMU group. */ struct pci_dev *pci_get_dma_source(struct pci_dev *dev) { @@ -3292,6 +3297,46 @@ struct pci_dev *pci_get_dma_source(struct pci_dev *dev) return pci_dev_get(dev); } +/* Table of multiple (ghost) source functions. This is similar to the + * translated sources above, but with the following differences: + * 1. the device may use multiple functions as DMA sources, + * 2. these functions cannot be assumed to be actual devices, + * 3. the specific ghost function for a request can not be exactly predicted. + * The bitmap only contains the additional quirk functions. + */ +static const struct pci_dev_dma_multi_func_sources { + u16 vendor; + u16 device; + u8 func_map; /* bit map. lsb is fn 0. */ +} pci_dev_dma_multi_func_sources[] = { + { PCI_VENDOR_ID_MARVELL_2, 0x9123, (1<<0)|(1<<1)}, + { PCI_VENDOR_ID_MARVELL_2, 0x9125, (1<<0)|(1<<1)}, + { PCI_VENDOR_ID_MARVELL_2, 0x9128, (1<<0)|(1<<1)}, + { PCI_VENDOR_ID_MARVELL_2, 0x9130, (1<<0)|(1<<1)}, + { PCI_VENDOR_ID_MARVELL_2, 0x9172, (1<<0)|(1<<1)}, + { 0 } +}; + +/* + * The mapping of fake/ghost functions is used when the real device is + * attached to an IOMMU domain. IOMMU groups are not aware of these + * functions, because they're not real devices. + */ +u8 pci_get_dma_source_map(struct pci_dev *dev) +{ + const struct pci_dev_dma_multi_func_sources *i; + + for (i = pci_dev_dma_multi_func_sources; 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; +} + static const struct pci_dev_acs_enabled { u16 vendor; u16 device; diff --git a/include/linux/pci.h b/include/linux/pci.h index 2461033..5ad3822 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1578,6 +1578,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); +u8 pci_get_dma_source_map(struct pci_dev *dev); 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, @@ -1586,6 +1587,10 @@ static inline struct pci_dev *pci_get_dma_source(struct pci_dev *dev) { return pci_dev_get(dev); } +u8 pci_get_dma_source_map(struct pci_dev *dev) +{ + return 0; +} static inline int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags) { diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index f11c1c2..df57496 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -1604,6 +1604,7 @@ #define PCI_SUBDEVICE_ID_KEYSPAN_SX2 0x5334 #define PCI_VENDOR_ID_MARVELL 0x11ab +#define PCI_VENDOR_ID_MARVELL_2 0x1b4b #define PCI_DEVICE_ID_MARVELL_GT64111 0x4146 #define PCI_DEVICE_ID_MARVELL_GT64260 0x6430 #define PCI_DEVICE_ID_MARVELL_MV64360 0x6460 -- 1.7.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
[parent not found: <CAJtEV7bvPORPoXgQY2OahNVMj+SY5z=xxQ=nueX7=kLVfpyF_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* RE: [PATCH] Quirk to support Marvell 88SE91xx SATA controllers with Intel IOMMU. [not found] ` <CAJtEV7bvPORPoXgQY2OahNVMj+SY5z=xxQ=nueX7=kLVfpyF_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2013-03-04 11:32 ` Justin Piszcz 2013-03-29 14:36 ` Justin Piszcz 1 sibling, 0 replies; 17+ messages in thread From: Justin Piszcz @ 2013-03-04 11:32 UTC (permalink / raw) To: 'Andrew Cooks' Cc: 'Chu Ying', 'open list:INTEL IOMMU (VT-d)', 'YingChu', 'open list:PCI SUBSYSTEM', bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, 'David Woodhouse', 'open list' On Sat, Mar 2, 2013 at 7:18 AM, Justin Piszcz <jpiszcz-BP4nVm5VUdNhbmWW9KSYcQ@public.gmane.org> wrote: > > Against a clean 3.7.10 (from ftp.kernel.org) > > # patch -p1 < > ../patch/RFC-Fix-Intel-IOMMU-support-for-Marvell-88SE91xx-SATA-controllers.. > patch > patching file drivers/iommu/intel-iommu.c > patching file drivers/pci/quirks.c > Hunk #1 succeeded at 3230 (offset 3 lines). > patching file include/linux/pci.h > # pwd > /usr/src/linux-3.7.10 > I've downloaded and patched the 3.7.10 tarball and still get the same output I got before; different output from yours. I'm not sure the patch is complete or applying correctly, are you? Could you please check whether the patch you're applying is the same as the attached file? Hi, Success! Patch from e-mail: # md5sum marvell_ghost_funcs.patch 718bfb5876e3538ec23a516ef28d03f5 marvell_ghost_funcs.patch Kernel from ftp.kernel.org: # md5sum linux-3.7.10.tar.bz2 56ec294a922b6112a1ef129668f38a83 linux-3.7.10.tar.bz2 Decompress, patch, re-compile w/IOMMU=on. # tar jxf linux-3.7.10.tar.bz2 ; ln -s linux-3.7.10 linux # cd linux; patch -p1 < ../marvell_ghost_funcs.patch patching file drivers/iommu/intel-iommu.c Hunk #1 succeeded at 1672 (offset -2 lines). Hunk #2 succeeded at 1729 (offset -2 lines). Hunk #3 succeeded at 3833 (offset -2 lines). patching file drivers/pci/quirks.c Hunk #1 succeeded at 3210 (offset -39 lines). Hunk #2 succeeded at 3240 (offset -39 lines). Hunk #3 succeeded at 3258 (offset -39 lines). patching file include/linux/pci.h Hunk #1 succeeded at 1546 (offset -32 lines). Hunk #2 succeeded at 1555 (offset -32 lines). patching file include/linux/pci_ids.h Reboot, re-test. # lilo Added 3.7.7-1 Added 3.7.10-5-ioff Added 3.7.10-7 (iommu=off w/patch) = OK Added 3.7.10-8 * (iommu=on w/patch) = OK dmesg w/patch + iommu http://home.comcast.net/~jpiszcz/20130304/dmesg-success-patch.txt Thanks! Justin. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Quirk to support Marvell 88SE91xx SATA controllers with Intel IOMMU. [not found] ` <CAJtEV7bvPORPoXgQY2OahNVMj+SY5z=xxQ=nueX7=kLVfpyF_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2013-03-04 11:32 ` Justin Piszcz @ 2013-03-29 14:36 ` Justin Piszcz 1 sibling, 0 replies; 17+ messages in thread From: Justin Piszcz @ 2013-03-29 14:36 UTC (permalink / raw) To: Andrew Cooks Cc: Chu Ying, open list:INTEL IOMMU (VT-d), YingChu, open list:PCI SUBSYSTEM, bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org, David Woodhouse, open list On Sun, Mar 3, 2013 at 8:35 PM, Andrew Cooks <acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > On Sat, Mar 2, 2013 at 7:18 AM, Justin Piszcz <jpiszcz-BP4nVm5VUdNhbmWW9KSYcQ@public.gmane.org> wrote: >> >> Against a clean 3.7.10 (from ftp.kernel.org) >> >> # patch -p1 < >> ../patch/RFC-Fix-Intel-IOMMU-support-for-Marvell-88SE91xx-SATA-controllers.. >> patch >> patching file drivers/iommu/intel-iommu.c >> patching file drivers/pci/quirks.c >> Hunk #1 succeeded at 3230 (offset 3 lines). >> patching file include/linux/pci.h >> # pwd >> /usr/src/linux-3.7.10 >> > > I've downloaded and patched the 3.7.10 tarball and still get the same > output I got before; different output from yours. I'm not sure the > patch is complete or applying correctly, are you? > Could you please check whether the patch you're applying is the same Hi, As this patch is now working for some time (against 3.7.x), I was wondering when it was going to be included in mainline? I had upgraded to 3.8.x and rebooted and the same problem recurred and had to revert back to 3.7.x. Thanks, Justin. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Quirk to support Marvell 88SE91xx SATA controllers with Intel IOMMU. [not found] ` <1362126373-32318-1-git-send-email-acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2013-03-01 17:51 ` Justin Piszcz @ 2013-03-06 4:04 ` Alex Williamson [not found] ` <1362542675.22132.86.camel-xdHQ/5r00wBBDLzU/O5InQ@public.gmane.org> 1 sibling, 1 reply; 17+ messages in thread From: Alex Williamson @ 2013-03-06 4:04 UTC (permalink / raw) To: Andrew Cooks Cc: gm.ychu-Re5JQEeQqe8AvxtiuMwx3w, open list, xjtuychu-PkbjNfxxIARBDgjK7y7TUQ, open list:INTEL IOMMU (VT-d), jpiszcz-BP4nVm5VUdNhbmWW9KSYcQ, open list:PCI SUBSYSTEM, bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ On Fri, 2013-03-01 at 16:26 +0800, Andrew Cooks wrote: > This is my third submitted patch to make Marvell 88SE91xx SATA controllers work when IOMMU is enabled.[1][2] > > What's changed: > * Adopt David Woodhouse's terminology by referring to the quirky functions as 'ghost' functions. > * Unmap ghost functions when device is detached from IOMMU. > * Stub function for when CONFIG_PCI_QUIRKS is not enabled. > > The bad: > * Still no AMD support. > * The table of affected chip IDs is as complete as I can make it by googling for bug reports. > > This patch was generated against commit b0af9cd9aab60ceb17d3ebabb9fdf4ff0a99cf50, but will also apply cleanly to 3.7.10. > > Bug reports: > 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 | 50 +++++++++++++++++++++++++++++++++++++++++++ > drivers/pci/quirks.c | 47 +++++++++++++++++++++++++++++++++++++++- > include/linux/pci.h | 5 ++++ > include/linux/pci_ids.h | 1 + > 4 files changed, 102 insertions(+), 1 deletions(-) > > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c > index 0099667..13323f2 100644 > --- a/drivers/iommu/intel-iommu.c > +++ b/drivers/iommu/intel-iommu.c > @@ -1674,6 +1674,50 @@ static int domain_context_mapping_one(struct dmar_domain *domain, int segment, > return 0; > } > > +/* For quirky devices like Marvell 88SE91xx chips that use ghost functions. */ > +static int map_ghost_dma_fn(struct dmar_domain *domain, > + struct pci_dev *pdev, > + int translation) > +{ > + u8 fn, fn_map; > + int err = 0; > + > + fn_map = pci_get_dma_source_map(pdev); if (!fn_map) return 0; > + > + for (fn = 1; fn < 8; fn++) { Wouldn't you want to do 0 to 7, then add: if (fn == PCI_FUNC(pdev->devfn)) continue; You could also get more creative with the loop using shifts and exit when the remaining map is 0. > + if (fn_map & (1 << 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; I'd be worried that there's missing cleanup here, what if you were mapping multiple ghost functions and the 2nd one failed, leaving one attached? > + dev_dbg(&pdev->dev, "dma quirk; func %d mapped", fn); > + } > + } > + return 0; > +} > + > +static void iommu_detach_dev(struct intel_iommu *iommu, u8 bus, u8 devfn); > + > +static void unmap_ghost_dma_fn(struct intel_iommu *iommu, > + struct pci_dev *pdev) > +{ > + u8 fn, fn_map; > + > + fn_map = pci_get_dma_source_map(pdev); > + > + for (fn = 1; fn < 8; fn++) { Same early exit and loop comments as above. > + if (fn_map & (1 << fn)) { > + iommu_detach_dev(iommu, > + pdev->bus->number, > + PCI_DEVFN(PCI_SLOT(pdev->devfn), fn)); > + dev_dbg(&pdev->dev, "dma quirk; func %d unmapped", fn); > + } > + } > +} > + > static int > domain_context_mapping(struct dmar_domain *domain, struct pci_dev *pdev, > int translation) > @@ -1687,6 +1731,11 @@ domain_context_mapping(struct dmar_domain *domain, struct pci_dev *pdev, > if (ret) > return ret; > > + /* quirk for undeclared/ghost pci functions */ > + ret = map_ghost_dma_fn(domain, pdev, translation); > + if (ret) > + return ret; > + > /* dependent device mapping */ > tmp = pci_find_upstream_pcie_bridge(pdev); > if (!tmp) > @@ -3786,6 +3835,7 @@ static void domain_remove_one_dev_info(struct dmar_domain *domain, > iommu_disable_dev_iotlb(info); > iommu_detach_dev(iommu, info->bus, info->devfn); > iommu_detach_dependent_devices(iommu, pdev); > + unmap_ghost_dma_fn(iommu, pdev); > free_devinfo_mem(info); > > spin_lock_irqsave(&device_domain_lock, flags); > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index 0369fb6..d311100 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -3249,6 +3249,10 @@ static struct pci_dev *pci_func_0_dma_source(struct pci_dev *dev) > return pci_get_slot(dev->bus, PCI_DEVFN(PCI_SLOT(dev->devfn), 0)); > } > > +/* Table of source functions for real devices. The DMA requests for the > + * device are tagged with a different real function as source. This is > + * relevant to multifunction devices. > + */ > static const struct pci_dev_dma_source { > u16 vendor; > u16 device; > @@ -3275,7 +3279,8 @@ static const struct pci_dev_dma_source { > * the device doing the DMA, but sometimes hardware is broken and will > * tag the DMA as being sourced from a different device. This function > * allows that translation. Note that the reference count of the > - * returned device is incremented on all paths. > + * returned device is incremented on all paths. Translation is done when > + * the device is added to an IOMMU group. > */ > struct pci_dev *pci_get_dma_source(struct pci_dev *dev) > { > @@ -3292,6 +3297,46 @@ struct pci_dev *pci_get_dma_source(struct pci_dev *dev) > return pci_dev_get(dev); > } > > +/* Table of multiple (ghost) source functions. This is similar to the > + * translated sources above, but with the following differences: > + * 1. the device may use multiple functions as DMA sources, > + * 2. these functions cannot be assumed to be actual devices, > + * 3. the specific ghost function for a request can not be exactly predicted. > + * The bitmap only contains the additional quirk functions. > + */ > +static const struct pci_dev_dma_multi_func_sources { > + u16 vendor; > + u16 device; > + u8 func_map; /* bit map. lsb is fn 0. */ > +} pci_dev_dma_multi_func_sources[] = { > + { PCI_VENDOR_ID_MARVELL_2, 0x9123, (1<<0)|(1<<1)}, > + { PCI_VENDOR_ID_MARVELL_2, 0x9125, (1<<0)|(1<<1)}, > + { PCI_VENDOR_ID_MARVELL_2, 0x9128, (1<<0)|(1<<1)}, > + { PCI_VENDOR_ID_MARVELL_2, 0x9130, (1<<0)|(1<<1)}, > + { PCI_VENDOR_ID_MARVELL_2, 0x9172, (1<<0)|(1<<1)}, Links to bug reports in the comments might be useful for future workarounds. I'm also not sure what you mean in the 3rd bullet, the non-ghost function of some of these is sometimes 0, sometimes 1? And the ghost function is the other? Skipping fn 0 above, I assume all cases are fn 0 exists and fn 1 is the ghost function, right? If so, then we probably only want bit 1 set. I'm afraid to ask whether there are configurations of this vendor/device that have a fn 1. Thanks, Alex > + { 0 } > +}; > + > +/* > + * The mapping of fake/ghost functions is used when the real device is > + * attached to an IOMMU domain. IOMMU groups are not aware of these > + * functions, because they're not real devices. > + */ > +u8 pci_get_dma_source_map(struct pci_dev *dev) > +{ > + const struct pci_dev_dma_multi_func_sources *i; > + > + for (i = pci_dev_dma_multi_func_sources; 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; > +} > + > static const struct pci_dev_acs_enabled { > u16 vendor; > u16 device; > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 2461033..5ad3822 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1578,6 +1578,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); > +u8 pci_get_dma_source_map(struct pci_dev *dev); > 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, > @@ -1586,6 +1587,10 @@ static inline struct pci_dev *pci_get_dma_source(struct pci_dev *dev) > { > return pci_dev_get(dev); > } > +u8 pci_get_dma_source_map(struct pci_dev *dev) > +{ > + return 0; > +} > static inline int pci_dev_specific_acs_enabled(struct pci_dev *dev, > u16 acs_flags) > { > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h > index f11c1c2..df57496 100644 > --- a/include/linux/pci_ids.h > +++ b/include/linux/pci_ids.h > @@ -1604,6 +1604,7 @@ > #define PCI_SUBDEVICE_ID_KEYSPAN_SX2 0x5334 > > #define PCI_VENDOR_ID_MARVELL 0x11ab > +#define PCI_VENDOR_ID_MARVELL_2 0x1b4b > #define PCI_DEVICE_ID_MARVELL_GT64111 0x4146 > #define PCI_DEVICE_ID_MARVELL_GT64260 0x6430 > #define PCI_DEVICE_ID_MARVELL_MV64360 0x6460 ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <1362542675.22132.86.camel-xdHQ/5r00wBBDLzU/O5InQ@public.gmane.org>]
* Re: [PATCH] Quirk to support Marvell 88SE91xx SATA controllers with Intel IOMMU. [not found] ` <1362542675.22132.86.camel-xdHQ/5r00wBBDLzU/O5InQ@public.gmane.org> @ 2013-03-06 6:59 ` Andrew Cooks 0 siblings, 0 replies; 17+ messages in thread From: Andrew Cooks @ 2013-03-06 6:59 UTC (permalink / raw) To: Alex Williamson Cc: Chu Ying, open list, YingChu, open list:INTEL IOMMU (VT-d), Justin Piszcz, open list:PCI SUBSYSTEM, bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org, David Woodhouse On Wed, Mar 6, 2013 at 12:04 PM, Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: > On Fri, 2013-03-01 at 16:26 +0800, Andrew Cooks wrote: > >> + >> + for (fn = 1; fn < 8; fn++) { > > Wouldn't you want to do 0 to 7, then add: > > if (fn == PCI_FUNC(pdev->devfn)) > continue; > > You could also get more creative with the loop using shifts and exit > when the remaining map is 0. Thanks, I'll use a shift instead. Up to now I've assumed that function 0 will always be the real device and that function 1-7 may be ghost functions, but as we saw in the case of the Marvell 88NV9143 in the Super Talent CoreStore MV 64GB mini-PCIe SSD, that assumption is probably wrong. To handle the case where the real device is function 1 and function 0 needs to be mapped as a ghost function, would it be acceptable to iterate over 0 to 7 and let domain_context_mapping_one take care of preventing duplicates, or should I duplicate the device_to_context_entry and context_present function calls? > >> + if (fn_map & (1 << 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; > > I'd be worried that there's missing cleanup here, what if you were > mapping multiple ghost functions and the 2nd one failed, leaving one > attached? I don't understand the failure cases sufficiently, but I understand that it's better to have all mappings succeed or fail together. Will fix it. >> +/* Table of multiple (ghost) source functions. This is similar to the >> + * translated sources above, but with the following differences: >> + * 1. the device may use multiple functions as DMA sources, >> + * 2. these functions cannot be assumed to be actual devices, >> + * 3. the specific ghost function for a request can not be exactly predicted. >> + * The bitmap only contains the additional quirk functions. >> + */ >> +static const struct pci_dev_dma_multi_func_sources { >> + u16 vendor; >> + u16 device; >> + u8 func_map; /* bit map. lsb is fn 0. */ >> +} pci_dev_dma_multi_func_sources[] = { >> + { PCI_VENDOR_ID_MARVELL_2, 0x9123, (1<<0)|(1<<1)}, >> + { PCI_VENDOR_ID_MARVELL_2, 0x9125, (1<<0)|(1<<1)}, >> + { PCI_VENDOR_ID_MARVELL_2, 0x9128, (1<<0)|(1<<1)}, >> + { PCI_VENDOR_ID_MARVELL_2, 0x9130, (1<<0)|(1<<1)}, >> + { PCI_VENDOR_ID_MARVELL_2, 0x9172, (1<<0)|(1<<1)}, > > Links to bug reports in the comments might be useful for future > workarounds. I'm also not sure what you mean in the 3rd bullet, the > non-ghost function of some of these is sometimes 0, sometimes 1? And > the ghost function is the other? The ghost function is the one that doesn't correspond to an actual device, but the actual device could be either 0 or 1 and it could use both 0 and 1 for different requests, with no obvious way to tell when it will use 0 and when it will use 1. I'll reword the bullet. > Skipping fn 0 above, I assume all > cases are fn 0 exists and fn 1 is the ghost function, right? If so, > then we probably only want bit 1 set. I'm afraid to ask whether there > are configurations of this vendor/device that have a fn 1. See comment about Marvell 88NV9143 above. Thanks for reviewing! a. ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2013-03-29 14:36 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1354533387-4110-1-git-send-email-acooks@gmail.com>
[not found] ` <1354533387-4110-1-git-send-email-acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2012-12-03 11:16 ` [PATCH 4/4] create context mapping entries for devices that use phantom functions Andrew Cooks
2012-12-19 10:58 ` [RFC PATCH] Fix Intel IOMMU support for Marvell 88SE91xx SATA controllers Andrew Cooks
2012-12-19 13:41 ` Chu Ying
[not found] ` <A1185FFE-6B90-4B44-BF8C-082E487AA73D-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2012-12-19 15:07 ` Andrew Cooks
[not found] ` <CAJtEV7ZmgkJMhFL_2Qzt1YsKnZ40gNi0yHgixVW3yJEfi4QzfA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-12-20 2:55 ` Ying Chu
[not found] ` <1355914703-28576-1-git-send-email-acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-02-22 19:29 ` Stijn Tintel
[not found] ` <5127C716.6050903-VfPWfsRibaPZj6PxcwrBaQ@public.gmane.org>
2013-02-25 8:37 ` Andrew Cooks
2013-03-01 8:26 ` [PATCH] Quirk to support Marvell 88SE91xx SATA controllers with Intel IOMMU Andrew Cooks
2013-03-01 17:54 ` Justin Piszcz
[not found] ` <1362126373-32318-1-git-send-email-acooks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-03-01 17:51 ` Justin Piszcz
2013-03-01 22:19 ` Andrew Cooks
[not found] ` <CAJtEV7Zs2BiwJ9jdDeoceh9hiVXvVaSvj=9H+4+vEzhY72AS9g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-03-01 23:18 ` Justin Piszcz
2013-03-04 1:35 ` Andrew Cooks
[not found] ` <CAJtEV7bvPORPoXgQY2OahNVMj+SY5z=xxQ=nueX7=kLVfpyF_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-03-04 11:32 ` Justin Piszcz
2013-03-29 14:36 ` Justin Piszcz
2013-03-06 4:04 ` Alex Williamson
[not found] ` <1362542675.22132.86.camel-xdHQ/5r00wBBDLzU/O5InQ@public.gmane.org>
2013-03-06 6:59 ` Andrew Cooks
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).