Re: [PATCH v2 3/5] PCI: Introduce direct dma alias

From: Bjorn Helgaas <helgaas@kernel.org>
To: Jon Derrick <jonathan.derrick@intel.com>
Cc: iommu@lists.linux-foundation.org, linux-pci@vger.kernel.org,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Keith Busch <kbusch@kernel.org>, Joerg Roedel <joro@8bytes.org>,
	Christoph Hellwig <hch@lst.de>,
	David Woodhouse <dwmw2@infradead.org>,
	Lu Baolu <baolu.lu@linux.intel.com>
Subject: Re: [PATCH v2 3/5] PCI: Introduce direct dma alias
Date: Thu, 9 Jan 2020 17:11:41 -0600	[thread overview]
Message-ID: <20200109231141.GA41540@google.com> (raw)
In-Reply-To: <1578580256-3483-4-git-send-email-jonathan.derrick@intel.com>

In subject:
s/Introduce direct dma alias/Add pci_direct_dma_alias()/

On Thu, Jan 09, 2020 at 07:30:54AM -0700, Jon Derrick wrote:
> The current dma alias implementation requires the aliased device be on
> the same bus as the dma parent. This introduces an arch-specific
> mechanism to point to an arbitrary struct device when doing mapping and
> pci alias search.

"arbitrary struct device" is a little weird since an arbitrary device
doesn't have to be a PCI device, but these mappings and aliases only
make sense in the PCI domain.

Maybe it has something to do with pci_sysdata.vmd_dev being a
"struct device *" rather than a "struct pci_dev *"?  I don't know why
that is, because it looks like every place you use it, you use
to_pci_dev() to get the pci_dev pointer back anyway.  But I assume you
have some good reason for that.

s/dma/DMA/
s/pci/PCI/
(above and also in code comments below)

> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
>  arch/x86/pci/common.c |  7 +++++++
>  drivers/pci/pci.c     | 17 ++++++++++++++++-
>  drivers/pci/search.c  |  9 +++++++++
>  include/linux/pci.h   |  1 +
>  4 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
> index 1e59df0..565cc17 100644
> --- a/arch/x86/pci/common.c
> +++ b/arch/x86/pci/common.c
> @@ -736,3 +736,10 @@ int pci_ext_cfg_avail(void)
>  	else
>  		return 0;
>  }
> +
> +#if IS_ENABLED(CONFIG_VMD)
> +struct device *pci_direct_dma_alias(struct pci_dev *dev)
> +{
> +	return to_pci_sysdata(dev->bus)->vmd_dev;
> +}
> +#endif
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index ad746d9..e4269e9 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -6034,7 +6034,9 @@ bool pci_devs_are_dma_aliases(struct pci_dev *dev1, struct pci_dev *dev2)
>  	return (dev1->dma_alias_mask &&
>  		test_bit(dev2->devfn, dev1->dma_alias_mask)) ||
>  	       (dev2->dma_alias_mask &&
> -		test_bit(dev1->devfn, dev2->dma_alias_mask));
> +		test_bit(dev1->devfn, dev2->dma_alias_mask)) ||
> +	       (pci_direct_dma_alias(dev1) == &dev2->dev) ||
> +	       (pci_direct_dma_alias(dev2) == &dev1->dev);
>  }
>  
>  bool pci_device_is_present(struct pci_dev *pdev)
> @@ -6058,6 +6060,19 @@ void pci_ignore_hotplug(struct pci_dev *dev)
>  }
>  EXPORT_SYMBOL_GPL(pci_ignore_hotplug);
>  
> +/**
> + * pci_direct_dma_alias - Get dma alias for pci device
> + * @dev: the PCI device that may have a dma alias
> + *
> + * Permits the platform to provide architecture-specific functionality to
> + * devices needing to alias dma to another device. This is the default
> + * implementation. Architecture implementations can override this.
> + */
> +struct device __weak *pci_direct_dma_alias(struct pci_dev *dev)
> +{
> +	return NULL;
> +}
> +
>  resource_size_t __weak pcibios_default_alignment(void)
>  {
>  	return 0;
> diff --git a/drivers/pci/search.c b/drivers/pci/search.c
> index bade140..6d61209 100644
> --- a/drivers/pci/search.c
> +++ b/drivers/pci/search.c
> @@ -32,6 +32,15 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
>  	struct pci_bus *bus;
>  	int ret;
>  
> +	if (unlikely(pci_direct_dma_alias(pdev))) {
> +		struct device *dev = pci_direct_dma_alias(pdev);
> +
> +		if (dev_is_pci(dev))
> +			pdev = to_pci_dev(dev);
> +		return fn(pdev, PCI_DEVID(pdev->bus->number, pdev->devfn),
> +			  data);
> +	}
> +
>  	ret = fn(pdev, pci_dev_id(pdev), data);
>  	if (ret)
>  		return ret;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index c393dff..82494d3 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1202,6 +1202,7 @@ u32 pcie_bandwidth_available(struct pci_dev *dev, struct pci_dev **limiting_dev,
>  int pci_select_bars(struct pci_dev *dev, unsigned long flags);
>  bool pci_device_is_present(struct pci_dev *pdev);
>  void pci_ignore_hotplug(struct pci_dev *dev);
> +struct device *pci_direct_dma_alias(struct pci_dev *dev);
>  
>  int __printf(6, 7) pci_request_irq(struct pci_dev *dev, unsigned int nr,
>  		irq_handler_t handler, irq_handler_t thread_fn, void *dev_id,
> -- 
> 1.8.3.1
> 