On 2021-05-02 6:32 p.m., John Hubbard wrote:
> On 4/8/21 10:01 AM, Logan Gunthorpe wrote:
>> Add a flags member to the dma_map_ops structure with one flag to
>> indicate support for PCI P2PDMA.
>>
>> Also, add a helper to check if a device supports PCI P2PDMA.
>>
>> Signed-off-by: Logan Gunthorpe <logang@xxxxxxxxxxxx>
>> ---
>> include/linux/dma-map-ops.h | 3 +++
>> include/linux/dma-mapping.h | 5 +++++
>> kernel/dma/mapping.c | 18 ++++++++++++++++++
>> 3 files changed, 26 insertions(+)
>>
>> diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
>> index 51872e736e7b..481892822104 100644
>> --- a/include/linux/dma-map-ops.h
>> +++ b/include/linux/dma-map-ops.h
>> @@ -12,6 +12,9 @@
>> struct cma;
>>
>> struct dma_map_ops {
>> + unsigned int flags;
>> +#define DMA_F_PCI_P2PDMA_SUPPORTED (1 << 0)
>> +
>
> Can we move this up and out of the struct member area, so that it looks
> more like this:
>
> /*
> * Values for struct dma_map_ops.flags:
> *
> * DMA_F_PCI_P2PDMA_SUPPORTED: <documentation here...this is a good place to
> * explain exactly what this flag is for.>
> */
> #define DMA_F_PCI_P2PDMA_SUPPORTED (1 << 0)
>
> struct dma_map_ops {
> unsigned int flags;
>
Sure, I don't care that much. I was just following the style in nvme.h.
>> void *(*alloc)(struct device *dev, size_t size,
>> dma_addr_t *dma_handle, gfp_t gfp,
>> unsigned long attrs);
>> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
>> index 50b8f586cf59..c31980ecca62 100644
>> --- a/include/linux/dma-mapping.h
>> +++ b/include/linux/dma-mapping.h
>> @@ -146,6 +146,7 @@ int dma_mmap_attrs(struct device *dev, struct vm_area_struct *vma,
>> unsigned long attrs);
>> bool dma_can_mmap(struct device *dev);
>> int dma_supported(struct device *dev, u64 mask);
>> +bool dma_pci_p2pdma_supported(struct device *dev);
>> int dma_set_mask(struct device *dev, u64 mask);
>> int dma_set_coherent_mask(struct device *dev, u64 mask);
>> u64 dma_get_required_mask(struct device *dev);
>> @@ -247,6 +248,10 @@ static inline int dma_supported(struct device *dev, u64 mask)
>> {
>> return 0;
>> }
>> +static inline bool dma_pci_p2pdma_supported(struct device *dev)
>> +{
>> + return 0;
>
> Should be:
>
> return false;
Yup, will fix.
>> +}
>> static inline int dma_set_mask(struct device *dev, u64 mask)
>> {
>> return -EIO;
>> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
>> index 923089c4267b..ce44a0fcc4e5 100644
>> --- a/kernel/dma/mapping.c
>> +++ b/kernel/dma/mapping.c
>> @@ -573,6 +573,24 @@ int dma_supported(struct device *dev, u64 mask)
>> }
>> EXPORT_SYMBOL(dma_supported);
>>
>> +bool dma_pci_p2pdma_supported(struct device *dev)
>> +{
>> + const struct dma_map_ops *ops = get_dma_ops(dev);
>> +
>> + /* if ops is not set, dma direct will be used which supports P2PDMA */
>> + if (!ops)
>> + return true;
>> +
>> + /*
>> + * Note: dma_ops_bypass is not checked here because P2PDMA should
>> + * not be used with dma mapping ops that do not have support even
>> + * if the specific device is bypassing them.
>> + */
>> +
>> + return ops->flags & DMA_F_PCI_P2PDMA_SUPPORTED;
>
> Wow, rather unusual combination of things in order decide this. It feels
> a bit over-complicated to have flags and ops and a bool function all
> dealing with the same 1-bit answer, but there is no caller shown here,
> so I'll have to come back to this after reviewing subsequent patches.
Yeah, I originally had it much simpler, but it confused Ira and it was
clear it had to be written more explicitly and commented better.
Logan
