[PATCH v6 1/3] PCI: Fix off by one in dma_alias_mask allocation size

[PATCH v6 1/3] PCI: Fix off by one in dma_alias_mask allocation size

[James Sewart via iommu]

The number of possible devfns is 256, add def and correct uses.

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
Signed-off-by: James Sewart <jamessewart@arista.com>
---
 drivers/pci/pci.c    | 2 +-
 drivers/pci/search.c | 2 +-
 include/linux/pci.h  | 2 ++
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index a97e2571a527..d3c83248f3ce 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5876,7 +5876,7 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode,
 void pci_add_dma_alias(struct pci_dev *dev, u8 devfn)
 {
 	if (!dev->dma_alias_mask)
-		dev->dma_alias_mask = bitmap_zalloc(U8_MAX, GFP_KERNEL);
+		dev->dma_alias_mask = bitmap_zalloc(MAX_NR_DEVFNS, GFP_KERNEL);
 	if (!dev->dma_alias_mask) {
 		pci_warn(dev, "Unable to allocate DMA alias mask\n");
 		return;
diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index bade14002fd8..9e4dfae47252 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -43,7 +43,7 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
 	if (unlikely(pdev->dma_alias_mask)) {
 		u8 devfn;
 
-		for_each_set_bit(devfn, pdev->dma_alias_mask, U8_MAX) {
+		for_each_set_bit(devfn, pdev->dma_alias_mask, MAX_NR_DEVFNS) {
 			ret = fn(pdev, PCI_DEVID(pdev->bus->number, devfn),
 				 data);
 			if (ret)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 1a6cf19eac2d..6481da29d667 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -57,6 +57,8 @@
 #define PCI_DEVID(bus, devfn)	((((u16)(bus)) << 8) | (devfn))
 /* return bus from PCI devid = ((u16)bus_number) << 8) | devfn */
 #define PCI_BUS_NUM(x) (((x) >> 8) & 0xff)
+/* Number of possible devfns. devfns can be from 0.0 to 1f.7 inclusive */
+#define MAX_NR_DEVFNS 256
 
 /* pci_slot represents a physical slot */
 struct pci_slot {
-- 
2.24.0


_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v6 2/3] PCI: Add parameter nr_devfns to pci_add_dma_alias

[James Sewart via iommu]

pci_add_dma_alias can now be used to create a dma alias for a range of
devfns.

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
Signed-off-by: James Sewart <jamessewart@arista.com>
---
 drivers/pci/pci.c    | 22 +++++++++++++++++-----
 drivers/pci/quirks.c | 14 +++++++-------
 include/linux/pci.h  |  2 +-
 3 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index d3c83248f3ce..dbb01aceafda 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5857,7 +5857,8 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode,
 /**
  * pci_add_dma_alias - Add a DMA devfn alias for a device
  * @dev: the PCI device for which alias is added
- * @devfn: alias slot and function
+ * @devfn_from: alias slot and function
+ * @nr_devfns: Number of subsequent devfns to alias
  *
  * This helper encodes an 8-bit devfn as a bit number in dma_alias_mask
  * which is used to program permissible bus-devfn source addresses for DMA
@@ -5873,8 +5874,13 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode,
  * cannot be left as a userspace activity).  DMA aliases should therefore
  * be configured via quirks, such as the PCI fixup header quirk.
  */
-void pci_add_dma_alias(struct pci_dev *dev, u8 devfn)
+void pci_add_dma_alias(struct pci_dev *dev, u8 devfn_from, unsigned nr_devfns)
 {
+	int devfn_to;
+
+	nr_devfns = min(nr_devfns, (unsigned)MAX_NR_DEVFNS);
+	devfn_to = devfn_from + nr_devfns - 1;
+
 	if (!dev->dma_alias_mask)
 		dev->dma_alias_mask = bitmap_zalloc(MAX_NR_DEVFNS, GFP_KERNEL);
 	if (!dev->dma_alias_mask) {
@@ -5882,9 +5888,15 @@ void pci_add_dma_alias(struct pci_dev *dev, u8 devfn)
 		return;
 	}
 
-	set_bit(devfn, dev->dma_alias_mask);
-	pci_info(dev, "Enabling fixed DMA alias to %02x.%d\n",
-		 PCI_SLOT(devfn), PCI_FUNC(devfn));
+	bitmap_set(dev->dma_alias_mask, devfn_from, nr_devfns);
+
+	if (nr_devfns == 1)
+		pci_info(dev, "Enabling fixed DMA alias to %02x.%d\n",
+				PCI_SLOT(devfn_from), PCI_FUNC(devfn_from));
+	else if(nr_devfns > 1)
+		pci_info(dev, "Enabling fixed DMA alias for devfn range from %02x.%d to %02x.%d\n",
+				PCI_SLOT(devfn_from), PCI_FUNC(devfn_from),
+				PCI_SLOT(devfn_to), PCI_FUNC(devfn_to));
 }
 
 bool pci_devs_are_dma_aliases(struct pci_dev *dev1, struct pci_dev *dev2)
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 320255e5e8f8..0f3f5afc73fd 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3932,7 +3932,7 @@ int pci_dev_specific_reset(struct pci_dev *dev, int probe)
 static void quirk_dma_func0_alias(struct pci_dev *dev)
 {
 	if (PCI_FUNC(dev->devfn) != 0)
-		pci_add_dma_alias(dev, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
+		pci_add_dma_alias(dev, PCI_DEVFN(PCI_SLOT(dev->devfn), 0), 1);
 }
 
 /*
@@ -3946,7 +3946,7 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_RICOH, 0xe476, quirk_dma_func0_alias);
 static void quirk_dma_func1_alias(struct pci_dev *dev)
 {
 	if (PCI_FUNC(dev->devfn) != 1)
-		pci_add_dma_alias(dev, PCI_DEVFN(PCI_SLOT(dev->devfn), 1));
+		pci_add_dma_alias(dev, PCI_DEVFN(PCI_SLOT(dev->devfn), 1), 1);
 }
 
 /*
@@ -4031,7 +4031,7 @@ static void quirk_fixed_dma_alias(struct pci_dev *dev)
 
 	id = pci_match_id(fixed_dma_alias_tbl, dev);
 	if (id)
-		pci_add_dma_alias(dev, id->driver_data);
+		pci_add_dma_alias(dev, id->driver_data, 1);
 }
 
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ADAPTEC2, 0x0285, quirk_fixed_dma_alias);
@@ -4073,9 +4073,9 @@ DECLARE_PCI_FIXUP_HEADER(0x8086, 0x244e, quirk_use_pcie_bridge_dma_alias);
  */
 static void quirk_mic_x200_dma_alias(struct pci_dev *pdev)
 {
-	pci_add_dma_alias(pdev, PCI_DEVFN(0x10, 0x0));
-	pci_add_dma_alias(pdev, PCI_DEVFN(0x11, 0x0));
-	pci_add_dma_alias(pdev, PCI_DEVFN(0x12, 0x3));
+	pci_add_dma_alias(pdev, PCI_DEVFN(0x10, 0x0), 1);
+	pci_add_dma_alias(pdev, PCI_DEVFN(0x11, 0x0), 1);
+	pci_add_dma_alias(pdev, PCI_DEVFN(0x12, 0x3), 1);
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2260, quirk_mic_x200_dma_alias);
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2264, quirk_mic_x200_dma_alias);
@@ -5273,7 +5273,7 @@ static void quirk_switchtec_ntb_dma_alias(struct pci_dev *pdev)
 			pci_dbg(pdev,
 				"Aliasing Partition %d Proxy ID %02x.%d\n",
 				pp, PCI_SLOT(devfn), PCI_FUNC(devfn));
-			pci_add_dma_alias(pdev, devfn);
+			pci_add_dma_alias(pdev, devfn, 1);
 		}
 	}
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 6481da29d667..e7a9c8c92a93 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2325,7 +2325,7 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
 }
 #endif
 
-void pci_add_dma_alias(struct pci_dev *dev, u8 devfn);
+void pci_add_dma_alias(struct pci_dev *dev, u8 devfn_from, unsigned nr_devfns);
 bool pci_devs_are_dma_aliases(struct pci_dev *dev1, struct pci_dev *dev2);
 int pci_for_each_dma_alias(struct pci_dev *pdev,
 			   int (*fn)(struct pci_dev *pdev,
-- 
2.24.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v6 3/3] PCI: Add DMA alias quirk for PLX PEX NTB

[James Sewart via iommu]

The PLX PEX NTB forwards DMA transactions using Requester ID's that
don't exist as PCI devices. The devfn for a transaction is used as an
index into a lookup table storing the origin of a transaction on the
other side of the bridge.

This patch aliases all possible devfn's to the NTB device so that any
transaction coming in is governed by the mappings for the NTB.

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
Signed-off-by: James Sewart <jamessewart@arista.com>
---
 drivers/pci/quirks.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 0f3f5afc73fd..3a67049ca630 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -5315,6 +5315,21 @@ SWITCHTEC_QUIRK(0x8574);  /* PFXI 64XG3 */
 SWITCHTEC_QUIRK(0x8575);  /* PFXI 80XG3 */
 SWITCHTEC_QUIRK(0x8576);  /* PFXI 96XG3 */
 
+/*
+ * PLX NTB uses devfn proxy IDs to move TLPs between NT endpoints. These IDs
+ * are used to forward responses to the originator on the other side of the
+ * NTB. Alias all possible IDs to the NTB to permit access when the IOMMU is
+ * turned on.
+ */
+static void quirk_plx_ntb_dma_alias(struct pci_dev *pdev)
+{
+	pci_info(pdev, "Setting PLX NTB proxy ID aliases\n");
+	/* PLX NTB may use all 256 devfns */
+	pci_add_dma_alias(pdev, 0, 256);
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_PLX, 0x87b0, quirk_plx_ntb_dma_alias);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_PLX, 0x87b1, quirk_plx_ntb_dma_alias);
+
 /*
  * On Lenovo Thinkpad P50 SKUs with a Nvidia Quadro M1000M, the BIOS does
  * not always reset the secondary Nvidia GPU between reboots if the system
-- 
2.24.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v6 2/3] PCI: Add parameter nr_devfns to pci_add_dma_alias

[kbuild test robot]

Hi James,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v5.4-rc8]
[also build test WARNING on next-20191206]
[cannot apply to pci/next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/James-Sewart/PCI-Fix-off-by-one-in-dma_alias_mask-allocation-size/20191204-034421
base:    af42d3466bdc8f39806b26f593604fdc54140bcb
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-91-g817270f-dirty
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> drivers/iommu/amd_iommu.c:288:34: sparse: sparse: not enough arguments for function pci_add_dma_alias

vim +288 drivers/iommu/amd_iommu.c

e3156048346c28 Joerg Roedel   2016-04-08  234  
e3156048346c28 Joerg Roedel   2016-04-08  235  static u16 get_alias(struct device *dev)
e3156048346c28 Joerg Roedel   2016-04-08  236  {
e3156048346c28 Joerg Roedel   2016-04-08  237  	struct pci_dev *pdev = to_pci_dev(dev);
e3156048346c28 Joerg Roedel   2016-04-08  238  	u16 devid, ivrs_alias, pci_alias;
e3156048346c28 Joerg Roedel   2016-04-08  239  
6c0b43df74f900 Joerg Roedel   2016-05-09  240  	/* The callers make sure that get_device_id() does not fail here */
e3156048346c28 Joerg Roedel   2016-04-08  241  	devid = get_device_id(dev);
5ebb1bc2d63d90 Arindam Nath   2018-09-18  242  
5ebb1bc2d63d90 Arindam Nath   2018-09-18  243  	/* For ACPI HID devices, we simply return the devid as such */
5ebb1bc2d63d90 Arindam Nath   2018-09-18  244  	if (!dev_is_pci(dev))
5ebb1bc2d63d90 Arindam Nath   2018-09-18  245  		return devid;
5ebb1bc2d63d90 Arindam Nath   2018-09-18  246  
e3156048346c28 Joerg Roedel   2016-04-08  247  	ivrs_alias = amd_iommu_alias_table[devid];
5ebb1bc2d63d90 Arindam Nath   2018-09-18  248  
e3156048346c28 Joerg Roedel   2016-04-08  249  	pci_for_each_dma_alias(pdev, __last_alias, &pci_alias);
e3156048346c28 Joerg Roedel   2016-04-08  250  
e3156048346c28 Joerg Roedel   2016-04-08  251  	if (ivrs_alias == pci_alias)
e3156048346c28 Joerg Roedel   2016-04-08  252  		return ivrs_alias;
e3156048346c28 Joerg Roedel   2016-04-08  253  
e3156048346c28 Joerg Roedel   2016-04-08  254  	/*
e3156048346c28 Joerg Roedel   2016-04-08  255  	 * DMA alias showdown
e3156048346c28 Joerg Roedel   2016-04-08  256  	 *
e3156048346c28 Joerg Roedel   2016-04-08  257  	 * The IVRS is fairly reliable in telling us about aliases, but it
e3156048346c28 Joerg Roedel   2016-04-08  258  	 * can't know about every screwy device.  If we don't have an IVRS
e3156048346c28 Joerg Roedel   2016-04-08  259  	 * reported alias, use the PCI reported alias.  In that case we may
e3156048346c28 Joerg Roedel   2016-04-08  260  	 * still need to initialize the rlookup and dev_table entries if the
e3156048346c28 Joerg Roedel   2016-04-08  261  	 * alias is to a non-existent device.
e3156048346c28 Joerg Roedel   2016-04-08  262  	 */
e3156048346c28 Joerg Roedel   2016-04-08  263  	if (ivrs_alias == devid) {
e3156048346c28 Joerg Roedel   2016-04-08  264  		if (!amd_iommu_rlookup_table[pci_alias]) {
e3156048346c28 Joerg Roedel   2016-04-08  265  			amd_iommu_rlookup_table[pci_alias] =
e3156048346c28 Joerg Roedel   2016-04-08  266  				amd_iommu_rlookup_table[devid];
e3156048346c28 Joerg Roedel   2016-04-08  267  			memcpy(amd_iommu_dev_table[pci_alias].data,
e3156048346c28 Joerg Roedel   2016-04-08  268  			       amd_iommu_dev_table[devid].data,
e3156048346c28 Joerg Roedel   2016-04-08  269  			       sizeof(amd_iommu_dev_table[pci_alias].data));
e3156048346c28 Joerg Roedel   2016-04-08  270  		}
e3156048346c28 Joerg Roedel   2016-04-08  271  
e3156048346c28 Joerg Roedel   2016-04-08  272  		return pci_alias;
e3156048346c28 Joerg Roedel   2016-04-08  273  	}
e3156048346c28 Joerg Roedel   2016-04-08  274  
5f226da1b1d706 Bjorn Helgaas  2019-02-08  275  	pci_info(pdev, "Using IVRS reported alias %02x:%02x.%d "
5f226da1b1d706 Bjorn Helgaas  2019-02-08  276  		"for device [%04x:%04x], kernel reported alias "
e3156048346c28 Joerg Roedel   2016-04-08  277  		"%02x:%02x.%d\n", PCI_BUS_NUM(ivrs_alias), PCI_SLOT(ivrs_alias),
5f226da1b1d706 Bjorn Helgaas  2019-02-08  278  		PCI_FUNC(ivrs_alias), pdev->vendor, pdev->device,
e3156048346c28 Joerg Roedel   2016-04-08  279  		PCI_BUS_NUM(pci_alias), PCI_SLOT(pci_alias),
e3156048346c28 Joerg Roedel   2016-04-08  280  		PCI_FUNC(pci_alias));
e3156048346c28 Joerg Roedel   2016-04-08  281  
e3156048346c28 Joerg Roedel   2016-04-08  282  	/*
e3156048346c28 Joerg Roedel   2016-04-08  283  	 * If we don't have a PCI DMA alias and the IVRS alias is on the same
e3156048346c28 Joerg Roedel   2016-04-08  284  	 * bus, then the IVRS table may know about a quirk that we don't.
e3156048346c28 Joerg Roedel   2016-04-08  285  	 */
e3156048346c28 Joerg Roedel   2016-04-08  286  	if (pci_alias == devid &&
e3156048346c28 Joerg Roedel   2016-04-08  287  	    PCI_BUS_NUM(ivrs_alias) == pdev->bus->number) {
7afd16f882887c Linus Torvalds 2016-05-19 @288  		pci_add_dma_alias(pdev, ivrs_alias & 0xff);
5f226da1b1d706 Bjorn Helgaas  2019-02-08  289  		pci_info(pdev, "Added PCI DMA alias %02x.%d\n",
5f226da1b1d706 Bjorn Helgaas  2019-02-08  290  			PCI_SLOT(ivrs_alias), PCI_FUNC(ivrs_alias));
e3156048346c28 Joerg Roedel   2016-04-08  291  	}
e3156048346c28 Joerg Roedel   2016-04-08  292  
e3156048346c28 Joerg Roedel   2016-04-08  293  	return ivrs_alias;
e3156048346c28 Joerg Roedel   2016-04-08  294  }
e3156048346c28 Joerg Roedel   2016-04-08  295  

:::::: The code at line 288 was first introduced by commit
:::::: 7afd16f882887c9adc69cd1794f5e57777723217 Merge tag 'pci-v4.7-changes' of git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci

:::::: TO: Linus Torvalds <torvalds@linux-foundation.org>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v6 1/3] PCI: Fix off by one in dma_alias_mask allocation size

[Bjorn Helgaas]

[+cc Joerg]

On Tue, Dec 03, 2019 at 03:43:22PM +0000, James Sewart wrote:
> The number of possible devfns is 256, add def and correct uses.
> 
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> Signed-off-by: James Sewart <jamessewart@arista.com>

I applied these three patches to pci/virtualization for v5.6, thanks!

I moved the MAX_NR_DEVFNS from include/linux/pci.h to
drivers/pci/pci.h since nobody outside drivers/pci needs it.

> ---
>  drivers/pci/pci.c    | 2 +-
>  drivers/pci/search.c | 2 +-
>  include/linux/pci.h  | 2 ++
>  3 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index a97e2571a527..d3c83248f3ce 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5876,7 +5876,7 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode,
>  void pci_add_dma_alias(struct pci_dev *dev, u8 devfn)
>  {
>  	if (!dev->dma_alias_mask)
> -		dev->dma_alias_mask = bitmap_zalloc(U8_MAX, GFP_KERNEL);
> +		dev->dma_alias_mask = bitmap_zalloc(MAX_NR_DEVFNS, GFP_KERNEL);
>  	if (!dev->dma_alias_mask) {
>  		pci_warn(dev, "Unable to allocate DMA alias mask\n");
>  		return;
> diff --git a/drivers/pci/search.c b/drivers/pci/search.c
> index bade14002fd8..9e4dfae47252 100644
> --- a/drivers/pci/search.c
> +++ b/drivers/pci/search.c
> @@ -43,7 +43,7 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
>  	if (unlikely(pdev->dma_alias_mask)) {
>  		u8 devfn;
>  
> -		for_each_set_bit(devfn, pdev->dma_alias_mask, U8_MAX) {
> +		for_each_set_bit(devfn, pdev->dma_alias_mask, MAX_NR_DEVFNS) {
>  			ret = fn(pdev, PCI_DEVID(pdev->bus->number, devfn),
>  				 data);
>  			if (ret)
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 1a6cf19eac2d..6481da29d667 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -57,6 +57,8 @@
>  #define PCI_DEVID(bus, devfn)	((((u16)(bus)) << 8) | (devfn))
>  /* return bus from PCI devid = ((u16)bus_number) << 8) | devfn */
>  #define PCI_BUS_NUM(x) (((x) >> 8) & 0xff)
> +/* Number of possible devfns. devfns can be from 0.0 to 1f.7 inclusive */
> +#define MAX_NR_DEVFNS 256
>  
>  /* pci_slot represents a physical slot */
>  struct pci_slot {
> -- 
> 2.24.0
> 
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v6 2/3] PCI: Add parameter nr_devfns to pci_add_dma_alias

[Bjorn Helgaas]

[+cc Joerg]

On Tue, Dec 03, 2019 at 03:43:53PM +0000, James Sewart wrote:
> pci_add_dma_alias can now be used to create a dma alias for a range of
> devfns.
> 
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> Signed-off-by: James Sewart <jamessewart@arista.com>
> ---
>  drivers/pci/pci.c    | 22 +++++++++++++++++-----
>  drivers/pci/quirks.c | 14 +++++++-------
>  include/linux/pci.h  |  2 +-
>  3 files changed, 25 insertions(+), 13 deletions(-)

Heads up Joerg: I also updated drivers/iommu/amd_iommu.c (this is the
one reported by the kbuild test robot) and removed the printk there
that prints the same thing as the one in pci_add_dma_alias(), and I
updated a PCI quirk that was merged after this patch was posted.

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index d3c83248f3ce..dbb01aceafda 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5857,7 +5857,8 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode,
>  /**
>   * pci_add_dma_alias - Add a DMA devfn alias for a device
>   * @dev: the PCI device for which alias is added
> - * @devfn: alias slot and function
> + * @devfn_from: alias slot and function
> + * @nr_devfns: Number of subsequent devfns to alias
>   *
>   * This helper encodes an 8-bit devfn as a bit number in dma_alias_mask
>   * which is used to program permissible bus-devfn source addresses for DMA
> @@ -5873,8 +5874,13 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode,
>   * cannot be left as a userspace activity).  DMA aliases should therefore
>   * be configured via quirks, such as the PCI fixup header quirk.
>   */
> -void pci_add_dma_alias(struct pci_dev *dev, u8 devfn)
> +void pci_add_dma_alias(struct pci_dev *dev, u8 devfn_from, unsigned nr_devfns)
>  {
> +	int devfn_to;
> +
> +	nr_devfns = min(nr_devfns, (unsigned)MAX_NR_DEVFNS);
> +	devfn_to = devfn_from + nr_devfns - 1;

I made this look like:

+       devfn_to = min(devfn_from + nr_devfns - 1,
+                      (unsigned) MAX_NR_DEVFNS - 1);

so devfn_from=0xf0, nr_devfns=0x20 doesn't cause devfn_to to wrap
around.

I did keep Logan's reviewed-by, so let me know if I broke something.

>  	if (!dev->dma_alias_mask)
>  		dev->dma_alias_mask = bitmap_zalloc(MAX_NR_DEVFNS, GFP_KERNEL);
>  	if (!dev->dma_alias_mask) {
> @@ -5882,9 +5888,15 @@ void pci_add_dma_alias(struct pci_dev *dev, u8 devfn)
>  		return;
>  	}
>  
> -	set_bit(devfn, dev->dma_alias_mask);
> -	pci_info(dev, "Enabling fixed DMA alias to %02x.%d\n",
> -		 PCI_SLOT(devfn), PCI_FUNC(devfn));
> +	bitmap_set(dev->dma_alias_mask, devfn_from, nr_devfns);
> +
> +	if (nr_devfns == 1)
> +		pci_info(dev, "Enabling fixed DMA alias to %02x.%d\n",
> +				PCI_SLOT(devfn_from), PCI_FUNC(devfn_from));
> +	else if(nr_devfns > 1)
> +		pci_info(dev, "Enabling fixed DMA alias for devfn range from %02x.%d to %02x.%d\n",
> +				PCI_SLOT(devfn_from), PCI_FUNC(devfn_from),
> +				PCI_SLOT(devfn_to), PCI_FUNC(devfn_to));
>  }
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v6 2/3] PCI: Add parameter nr_devfns to pci_add_dma_alias

[James Sewart via iommu]

> On 10 Dec 2019, at 22:37, Bjorn Helgaas <helgaas@kernel.org> wrote:
> 
> [+cc Joerg]
> 
> On Tue, Dec 03, 2019 at 03:43:53PM +0000, James Sewart wrote:
>> pci_add_dma_alias can now be used to create a dma alias for a range of
>> devfns.
>> 
>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
>> Signed-off-by: James Sewart <jamessewart@arista.com>
>> ---
>> drivers/pci/pci.c    | 22 +++++++++++++++++-----
>> drivers/pci/quirks.c | 14 +++++++-------
>> include/linux/pci.h  |  2 +-
>> 3 files changed, 25 insertions(+), 13 deletions(-)
> 
> Heads up Joerg: I also updated drivers/iommu/amd_iommu.c (this is the
> one reported by the kbuild test robot) and removed the printk there
> that prints the same thing as the one in pci_add_dma_alias(), and I
> updated a PCI quirk that was merged after this patch was posted.
> 
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index d3c83248f3ce..dbb01aceafda 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -5857,7 +5857,8 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode,
>> /**
>>  * pci_add_dma_alias - Add a DMA devfn alias for a device
>>  * @dev: the PCI device for which alias is added
>> - * @devfn: alias slot and function
>> + * @devfn_from: alias slot and function
>> + * @nr_devfns: Number of subsequent devfns to alias
>>  *
>>  * This helper encodes an 8-bit devfn as a bit number in dma_alias_mask
>>  * which is used to program permissible bus-devfn source addresses for DMA
>> @@ -5873,8 +5874,13 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode,
>>  * cannot be left as a userspace activity).  DMA aliases should therefore
>>  * be configured via quirks, such as the PCI fixup header quirk.
>>  */
>> -void pci_add_dma_alias(struct pci_dev *dev, u8 devfn)
>> +void pci_add_dma_alias(struct pci_dev *dev, u8 devfn_from, unsigned nr_devfns)
>> {
>> +	int devfn_to;
>> +
>> +	nr_devfns = min(nr_devfns, (unsigned)MAX_NR_DEVFNS);
>> +	devfn_to = devfn_from + nr_devfns - 1;
> 
> I made this look like:
> 
> +       devfn_to = min(devfn_from + nr_devfns - 1,
> +                      (unsigned) MAX_NR_DEVFNS - 1);
> 
> so devfn_from=0xf0, nr_devfns=0x20 doesn't cause devfn_to to wrap
> around.
> 
> I did keep Logan's reviewed-by, so let me know if I broke something.

I think nr_devfns still needs updating as it is used for bitmap_set. 
Although thinking about it now we should limit the number to alias to be 
maximum (MAX_NR_DEVFNS - devfn_from), so that we don’t set past the end of 
the bitmap:

 nr_devfns = min(nr_devfns, (unsigned) MAX_NR_DEVFNS - devfn_from);

I think with this change we wont need to clip devfn_to.

> 
>> 	if (!dev->dma_alias_mask)
>> 		dev->dma_alias_mask = bitmap_zalloc(MAX_NR_DEVFNS, GFP_KERNEL);
>> 	if (!dev->dma_alias_mask) {
>> @@ -5882,9 +5888,15 @@ void pci_add_dma_alias(struct pci_dev *dev, u8 devfn)
>> 		return;
>> 	}
>> 
>> -	set_bit(devfn, dev->dma_alias_mask);
>> -	pci_info(dev, "Enabling fixed DMA alias to %02x.%d\n",
>> -		 PCI_SLOT(devfn), PCI_FUNC(devfn));
>> +	bitmap_set(dev->dma_alias_mask, devfn_from, nr_devfns);
>> +
>> +	if (nr_devfns == 1)
>> +		pci_info(dev, "Enabling fixed DMA alias to %02x.%d\n",
>> +				PCI_SLOT(devfn_from), PCI_FUNC(devfn_from));
>> +	else if(nr_devfns > 1)
>> +		pci_info(dev, "Enabling fixed DMA alias for devfn range from %02x.%d to %02x.%d\n",
>> +				PCI_SLOT(devfn_from), PCI_FUNC(devfn_from),
>> +				PCI_SLOT(devfn_to), PCI_FUNC(devfn_to));
>> }

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v6 2/3] PCI: Add parameter nr_devfns to pci_add_dma_alias

[Bjorn Helgaas]

On Wed, Dec 11, 2019 at 03:37:30PM +0000, James Sewart wrote:
> > On 10 Dec 2019, at 22:37, Bjorn Helgaas <helgaas@kernel.org> wrote:
> >> -void pci_add_dma_alias(struct pci_dev *dev, u8 devfn)
> >> +void pci_add_dma_alias(struct pci_dev *dev, u8 devfn_from, unsigned nr_devfns)
> >> {
> >> +	int devfn_to;
> >> +
> >> +	nr_devfns = min(nr_devfns, (unsigned)MAX_NR_DEVFNS);
> >> +	devfn_to = devfn_from + nr_devfns - 1;
> > 
> > I made this look like:
> > 
> > +       devfn_to = min(devfn_from + nr_devfns - 1,
> > +                      (unsigned) MAX_NR_DEVFNS - 1);
> > 
> > so devfn_from=0xf0, nr_devfns=0x20 doesn't cause devfn_to to wrap
> > around.
> > 
> > I did keep Logan's reviewed-by, so let me know if I broke something.
> 
> I think nr_devfns still needs updating as it is used for bitmap_set. 
> Although thinking about it now we should limit the number to alias to be 
> maximum (MAX_NR_DEVFNS - devfn_from), so that we don’t set past the end of 
> the bitmap:
> 
>  nr_devfns = min(nr_devfns, (unsigned) MAX_NR_DEVFNS - devfn_from);
> 
> I think with this change we wont need to clip devfn_to.

Indeed, you're right, thanks!  I put this in, so it now looks like
the following.  I dropped Logan's reviewed-by to avoid putting words
in his mouth ;)

commit 5ddcd840395a ("PCI: Add nr_devfns parameter to pci_add_dma_alias()")
Author: James Sewart <jamessewart@arista.com>
Date:   Tue Dec 10 16:07:30 2019 -0600

    PCI: Add nr_devfns parameter to pci_add_dma_alias()
    
    Add a "nr_devfns" parameter to pci_add_dma_alias() so it can be used to
    create DMA aliases for a range of devfns.
    
    [bhelgaas: incorporate nr_devfns fix from James, update
    quirk_pex_vca_alias() and setup_aliases()]
    Signed-off-by: James Sewart <jamessewart@arista.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index bd25674ee4db..7a6c056b9b9c 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -230,11 +230,8 @@ static struct pci_dev *setup_aliases(struct device *dev)
 	 */
 	ivrs_alias = amd_iommu_alias_table[pci_dev_id(pdev)];
 	if (ivrs_alias != pci_dev_id(pdev) &&
-	    PCI_BUS_NUM(ivrs_alias) == pdev->bus->number) {
-		pci_add_dma_alias(pdev, ivrs_alias & 0xff);
-		pci_info(pdev, "Added PCI DMA alias %02x.%d\n",
-			PCI_SLOT(ivrs_alias), PCI_FUNC(ivrs_alias));
-	}
+	    PCI_BUS_NUM(ivrs_alias) == pdev->bus->number)
+		pci_add_dma_alias(pdev, ivrs_alias & 0xff, 1);
 
 	clone_aliases(pdev);
 
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7b5fa2eabe09..951099279192 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5998,7 +5998,8 @@ EXPORT_SYMBOL_GPL(pci_pr3_present);
 /**
  * pci_add_dma_alias - Add a DMA devfn alias for a device
  * @dev: the PCI device for which alias is added
- * @devfn: alias slot and function
+ * @devfn_from: alias slot and function
+ * @nr_devfns: number of subsequent devfns to alias
  *
  * This helper encodes an 8-bit devfn as a bit number in dma_alias_mask
  * which is used to program permissible bus-devfn source addresses for DMA
@@ -6014,8 +6015,13 @@ EXPORT_SYMBOL_GPL(pci_pr3_present);
  * cannot be left as a userspace activity).  DMA aliases should therefore
  * be configured via quirks, such as the PCI fixup header quirk.
  */
-void pci_add_dma_alias(struct pci_dev *dev, u8 devfn)
+void pci_add_dma_alias(struct pci_dev *dev, u8 devfn_from, unsigned nr_devfns)
 {
+	int devfn_to;
+
+	nr_devfns = min(nr_devfns, (unsigned) MAX_NR_DEVFNS - devfn_from);
+	devfn_to = devfn_from + nr_devfns - 1;
+
 	if (!dev->dma_alias_mask)
 		dev->dma_alias_mask = bitmap_zalloc(MAX_NR_DEVFNS, GFP_KERNEL);
 	if (!dev->dma_alias_mask) {
@@ -6023,9 +6029,15 @@ void pci_add_dma_alias(struct pci_dev *dev, u8 devfn)
 		return;
 	}
 
-	set_bit(devfn, dev->dma_alias_mask);
-	pci_info(dev, "Enabling fixed DMA alias to %02x.%d\n",
-		 PCI_SLOT(devfn), PCI_FUNC(devfn));
+	bitmap_set(dev->dma_alias_mask, devfn_from, nr_devfns);
+
+	if (nr_devfns == 1)
+		pci_info(dev, "Enabling fixed DMA alias to %02x.%d\n",
+				PCI_SLOT(devfn_from), PCI_FUNC(devfn_from));
+	else if (nr_devfns > 1)
+		pci_info(dev, "Enabling fixed DMA alias for devfn range from %02x.%d to %02x.%d\n",
+				PCI_SLOT(devfn_from), PCI_FUNC(devfn_from),
+				PCI_SLOT(devfn_to), PCI_FUNC(devfn_to));
 }
 
 bool pci_devs_are_dma_aliases(struct pci_dev *dev1, struct pci_dev *dev2)
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 4937a088d7d8..2fcad18622eb 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3932,7 +3932,7 @@ int pci_dev_specific_reset(struct pci_dev *dev, int probe)
 static void quirk_dma_func0_alias(struct pci_dev *dev)
 {
 	if (PCI_FUNC(dev->devfn) != 0)
-		pci_add_dma_alias(dev, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
+		pci_add_dma_alias(dev, PCI_DEVFN(PCI_SLOT(dev->devfn), 0), 1);
 }
 
 /*
@@ -3946,7 +3946,7 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_RICOH, 0xe476, quirk_dma_func0_alias);
 static void quirk_dma_func1_alias(struct pci_dev *dev)
 {
 	if (PCI_FUNC(dev->devfn) != 1)
-		pci_add_dma_alias(dev, PCI_DEVFN(PCI_SLOT(dev->devfn), 1));
+		pci_add_dma_alias(dev, PCI_DEVFN(PCI_SLOT(dev->devfn), 1), 1);
 }
 
 /*
@@ -4031,7 +4031,7 @@ static void quirk_fixed_dma_alias(struct pci_dev *dev)
 
 	id = pci_match_id(fixed_dma_alias_tbl, dev);
 	if (id)
-		pci_add_dma_alias(dev, id->driver_data);
+		pci_add_dma_alias(dev, id->driver_data, 1);
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ADAPTEC2, 0x0285, quirk_fixed_dma_alias);
 
@@ -4072,9 +4072,9 @@ DECLARE_PCI_FIXUP_HEADER(0x8086, 0x244e, quirk_use_pcie_bridge_dma_alias);
  */
 static void quirk_mic_x200_dma_alias(struct pci_dev *pdev)
 {
-	pci_add_dma_alias(pdev, PCI_DEVFN(0x10, 0x0));
-	pci_add_dma_alias(pdev, PCI_DEVFN(0x11, 0x0));
-	pci_add_dma_alias(pdev, PCI_DEVFN(0x12, 0x3));
+	pci_add_dma_alias(pdev, PCI_DEVFN(0x10, 0x0), 1);
+	pci_add_dma_alias(pdev, PCI_DEVFN(0x11, 0x0), 1);
+	pci_add_dma_alias(pdev, PCI_DEVFN(0x12, 0x3), 1);
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2260, quirk_mic_x200_dma_alias);
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2264, quirk_mic_x200_dma_alias);
@@ -4098,13 +4098,8 @@ static void quirk_pex_vca_alias(struct pci_dev *pdev)
 	const unsigned int num_pci_slots = 0x20;
 	unsigned int slot;
 
-	for (slot = 0; slot < num_pci_slots; slot++) {
-		pci_add_dma_alias(pdev, PCI_DEVFN(slot, 0x0));
-		pci_add_dma_alias(pdev, PCI_DEVFN(slot, 0x1));
-		pci_add_dma_alias(pdev, PCI_DEVFN(slot, 0x2));
-		pci_add_dma_alias(pdev, PCI_DEVFN(slot, 0x3));
-		pci_add_dma_alias(pdev, PCI_DEVFN(slot, 0x4));
-	}
+	for (slot = 0; slot < num_pci_slots; slot++)
+		pci_add_dma_alias(pdev, PCI_DEVFN(slot, 0x0), 5);
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2954, quirk_pex_vca_alias);
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2955, quirk_pex_vca_alias);
@@ -5332,7 +5327,7 @@ static void quirk_switchtec_ntb_dma_alias(struct pci_dev *pdev)
 			pci_dbg(pdev,
 				"Aliasing Partition %d Proxy ID %02x.%d\n",
 				pp, PCI_SLOT(devfn), PCI_FUNC(devfn));
-			pci_add_dma_alias(pdev, devfn);
+			pci_add_dma_alias(pdev, devfn, 1);
 		}
 	}
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index c393dff2d66f..930fab293073 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2310,7 +2310,7 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
 }
 #endif
 
-void pci_add_dma_alias(struct pci_dev *dev, u8 devfn);
+void pci_add_dma_alias(struct pci_dev *dev, u8 devfn_from, unsigned nr_devfns);
 bool pci_devs_are_dma_aliases(struct pci_dev *dev1, struct pci_dev *dev2);
 int pci_for_each_dma_alias(struct pci_dev *pdev,
 			   int (*fn)(struct pci_dev *pdev,
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v6 2/3] PCI: Add parameter nr_devfns to pci_add_dma_alias

[Joerg Roedel]

Hi Bjorn,

On Tue, Dec 10, 2019 at 04:37:45PM -0600, Bjorn Helgaas wrote:
> Heads up Joerg: I also updated drivers/iommu/amd_iommu.c (this is the
> one reported by the kbuild test robot) and removed the printk there
> that prints the same thing as the one in pci_add_dma_alias(), and I
> updated a PCI quirk that was merged after this patch was posted.

Fine with me, thanks for the heads up.

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu