On 2021-05-02 4:44 p.m., John Hubbard wrote: > On 4/8/21 10:01 AM, Logan Gunthorpe wrote: >> pci_p2pdma_map_type() will be needed by the dma-iommu map_sg >> implementation because it will need to determine the mapping type >> ahead of actually doing the mapping to create the actual iommu mapping. >> >> Signed-off-by: Logan Gunthorpe <logang@xxxxxxxxxxxx> >> --- >> drivers/pci/p2pdma.c | 34 +++++++++++++++++++++++----------- >> include/linux/pci-p2pdma.h | 15 +++++++++++++++ >> 2 files changed, 38 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c >> index bcb1a6d6119d..38c93f57a941 100644 >> --- a/drivers/pci/p2pdma.c >> +++ b/drivers/pci/p2pdma.c >> @@ -20,13 +20,6 @@ >> #include <linux/seq_buf.h> >> #include <linux/xarray.h> >> >> -enum pci_p2pdma_map_type { >> - PCI_P2PDMA_MAP_UNKNOWN = 0, >> - PCI_P2PDMA_MAP_NOT_SUPPORTED, >> - PCI_P2PDMA_MAP_BUS_ADDR, >> - PCI_P2PDMA_MAP_THRU_HOST_BRIDGE, >> -}; >> - >> struct pci_p2pdma { >> struct gen_pool *pool; >> bool p2pmem_published; >> @@ -822,13 +815,30 @@ void pci_p2pmem_publish(struct pci_dev *pdev, bool publish) >> } >> EXPORT_SYMBOL_GPL(pci_p2pmem_publish); >> >> -static enum pci_p2pdma_map_type pci_p2pdma_map_type(struct dev_pagemap *pgmap, >> - struct device *dev) >> +/** >> + * pci_p2pdma_map_type - return the type of mapping that should be used for >> + * a given device and pgmap >> + * @pgmap: the pagemap of a page to determine the mapping type for >> + * @dev: device that is mapping the page >> + * @dma_attrs: the attributes passed to the dma_map operation -- >> + * this is so they can be checked to ensure P2PDMA pages were not >> + * introduced into an incorrect interface (like dma_map_sg). * >> + * >> + * Returns one of: >> + * PCI_P2PDMA_MAP_NOT_SUPPORTED - The mapping should not be done >> + * PCI_P2PDMA_MAP_BUS_ADDR - The mapping should use the PCI bus address >> + * PCI_P2PDMA_MAP_THRU_HOST_BRIDGE - The mapping should be done directly >> + */ >> +enum pci_p2pdma_map_type pci_p2pdma_map_type(struct dev_pagemap *pgmap, >> + struct device *dev, unsigned long dma_attrs) >> { >> struct pci_dev *provider = to_p2p_pgmap(pgmap)->provider; >> enum pci_p2pdma_map_type ret; >> struct pci_dev *client; >> >> + WARN_ONCE(!(dma_attrs & __DMA_ATTR_PCI_P2PDMA), >> + "PCI P2PDMA pages were mapped with dma_map_sg!"); > > This really ought to also return -EINVAL, assuming that my review suggestions > about return types, in earlier patches, are acceptable. That can't happen because, by convention, dma_map_sg() cannot return -EINVAL. I think the best we can do is proceed normally and just warn loudly. Logan