2021-01-28 00:52:17

by Jianxiong Gao

[permalink] [raw]
Subject: [PATCH 1/3] Adding page_offset_mask to device_dma_parameters

Some devices rely on the address offset in a page to function
correctly (NVMe driver as an example). These devices may use
a different page size than the Linux kernel. The address offset
has to be preserved upon mapping, and in order to do so, we
need to record the page_offset_mask first.

Signed-off-by: Jianxiong Gao <[email protected]>
---
include/linux/device.h | 1 +
include/linux/dma-mapping.h | 17 +++++++++++++++++
2 files changed, 18 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index 1779f90eeb4c..f44e0659fc66 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -292,6 +292,7 @@ struct device_dma_parameters {
*/
unsigned int max_segment_size;
unsigned long segment_boundary_mask;
+ unsigned int page_offset_mask;
};

/**
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 2e49996a8f39..5529a31fefba 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -500,6 +500,23 @@ static inline int dma_set_seg_boundary(struct device *dev, unsigned long mask)
return -EIO;
}

+static inline unsigned int dma_get_page_offset_mask(struct device *dev)
+{
+ if (dev->dma_parms)
+ return dev->dma_parms->page_offset_mask;
+ return 0;
+}
+
+static inline int dma_set_page_offset_mask(struct device *dev,
+ unsigned int page_offset_mask)
+{
+ if (dev->dma_parms) {
+ dev->dma_parms->page_offset_mask = page_offset_mask;
+ return 0;
+ }
+ return -EIO;
+}
+
static inline int dma_get_cache_alignment(void)
{
#ifdef ARCH_DMA_MINALIGN
--
2.27.0


2021-01-28 17:32:49

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH 1/3] Adding page_offset_mask to device_dma_parameters

On 2021-01-28 00:38, Jianxiong Gao wrote:
> Some devices rely on the address offset in a page to function
> correctly (NVMe driver as an example). These devices may use
> a different page size than the Linux kernel. The address offset
> has to be preserved upon mapping, and in order to do so, we
> need to record the page_offset_mask first.
>
> Signed-off-by: Jianxiong Gao <[email protected]>
> ---
> include/linux/device.h | 1 +
> include/linux/dma-mapping.h | 17 +++++++++++++++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 1779f90eeb4c..f44e0659fc66 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -292,6 +292,7 @@ struct device_dma_parameters {
> */
> unsigned int max_segment_size;
> unsigned long segment_boundary_mask;
> + unsigned int page_offset_mask;

Could we call this something more like "min_align_mask" (sorry, I can't
think of a name that's actually good and descriptive right now).
Essentially I worry that having "page" in there is going to be too easy
to misinterpret as having anything to do what "page" means almost
everywhere else (even before you throw IOMMU pages into the mix).

Also note that of all the possible ways to pack two ints and a long,
this one is the worst ;)

Robin.

> };
>
> /**
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 2e49996a8f39..5529a31fefba 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -500,6 +500,23 @@ static inline int dma_set_seg_boundary(struct device *dev, unsigned long mask)
> return -EIO;
> }
>
> +static inline unsigned int dma_get_page_offset_mask(struct device *dev)
> +{
> + if (dev->dma_parms)
> + return dev->dma_parms->page_offset_mask;
> + return 0;
> +}
> +
> +static inline int dma_set_page_offset_mask(struct device *dev,
> + unsigned int page_offset_mask)
> +{
> + if (dev->dma_parms) {
> + dev->dma_parms->page_offset_mask = page_offset_mask;
> + return 0;
> + }
> + return -EIO;
> +}
> +
> static inline int dma_get_cache_alignment(void)
> {
> #ifdef ARCH_DMA_MINALIGN
>

2021-01-28 18:20:48

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 1/3] Adding page_offset_mask to device_dma_parameters

On Thu, Jan 28, 2021 at 05:27:25PM +0000, Robin Murphy wrote:
> On 2021-01-28 00:38, Jianxiong Gao wrote:
>> Some devices rely on the address offset in a page to function
>> correctly (NVMe driver as an example). These devices may use
>> a different page size than the Linux kernel. The address offset
>> has to be preserved upon mapping, and in order to do so, we
>> need to record the page_offset_mask first.
>>
>> Signed-off-by: Jianxiong Gao <[email protected]>
>> ---
>> include/linux/device.h | 1 +
>> include/linux/dma-mapping.h | 17 +++++++++++++++++
>> 2 files changed, 18 insertions(+)
>>
>> diff --git a/include/linux/device.h b/include/linux/device.h
>> index 1779f90eeb4c..f44e0659fc66 100644
>> --- a/include/linux/device.h
>> +++ b/include/linux/device.h
>> @@ -292,6 +292,7 @@ struct device_dma_parameters {
>> */
>> unsigned int max_segment_size;
>> unsigned long segment_boundary_mask;
>> + unsigned int page_offset_mask;
>
> Could we call this something more like "min_align_mask" (sorry, I can't
> think of a name that's actually good and descriptive right now).
> Essentially I worry that having "page" in there is going to be too easy to
> misinterpret as having anything to do what "page" means almost everywhere
> else (even before you throw IOMMU pages into the mix).
>
> Also note that of all the possible ways to pack two ints and a long, this
> one is the worst ;)

The block layer uses virt_boundary for the related concept, but that
is pretty horrible too.