> On 10 Dec 2019, at 22:37, Bjorn Helgaas <helgaas@xxxxxxxxxx> 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@xxxxxxxxxxxx>
>> Signed-off-by: James Sewart <jamessewart@xxxxxxxxxx>
>> ---
>> 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));
>> }
