Re: [PATCH v2] UIO: Let uio_pdrv_genirq support cached memory

[Hongren Zheng]

On Thu, May 05, 2022 at 02:15:19PM +0200, Indan Zupancic wrote:
> If 'cacheable' DT property exists, the memory mapping will be
> done with UIO_MEM_IOVA instead of the default UIO_MEM_PHYS.
> 
> Signed-off-by: Indan Zupancic <Indan.Zupancic@xxxxxxxxxxxx>
> ---
>  Documentation/driver-api/uio-howto.rst | 3 ++-
>  drivers/uio/uio_pdrv_genirq.c          | 8 ++++++--
>  2 files changed, 8 insertions(+), 3 deletions(-)

I assume this is fundamentally OK? What else should be done to get this merged?

I plan to add similar functionality for uio_pci_generic,
especially for the RedHat IVSHMEM pcie device where
bar2 is just normal memory and should be marked
as UIO_MEM_IOVA as this has performance implication,
is that acceptable?

Cc'ed uio_pci_generic maintainer here.

> 
> diff --git a/Documentation/driver-api/uio-howto.rst b/Documentation/driver-api/uio-howto.rst
> index 907ffa3b38f5..3cb48fc757d9 100644
> --- a/Documentation/driver-api/uio-howto.rst
> +++ b/Documentation/driver-api/uio-howto.rst
> @@ -413,7 +413,8 @@ probed with the ``"of_id"`` module parameter set to the ``"compatible"``
>  string of the node the driver is supposed to handle. By default, the
>  node's name (without the unit address) is exposed as name for the
>  UIO device in userspace. To set a custom name, a property named
> -``"linux,uio-name"`` may be specified in the DT node.
> +``"linux,uio-name"`` may be specified in the DT node. If a property
> +named ``cacheable`` exists, the memory will be mapped cacheable.
>  
>  Using uio_dmem_genirq for platform devices
>  ------------------------------------------
> diff --git a/drivers/uio/uio_pdrv_genirq.c b/drivers/uio/uio_pdrv_genirq.c
> index 63258b6accc4..269784ce0ba3 100644
> --- a/drivers/uio/uio_pdrv_genirq.c
> +++ b/drivers/uio/uio_pdrv_genirq.c
> @@ -115,6 +115,7 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
>  	struct uio_mem *uiomem;
>  	int ret = -EINVAL;
>  	int i;
> +	int memtype = UIO_MEM_PHYS;
>  
>  	if (node) {
>  		const char *name;
> @@ -132,7 +133,10 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
>  		else
>  			uioinfo->name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
>  						       "%pOFn", node);
> -
> +		if (of_get_property(node, "cacheable", NULL)) {
> +			dev_info(&pdev->dev, "%s cacheable\n", uioinfo->name);
> +			memtype = UIO_MEM_IOVA;
> +		}
>  		uioinfo->version = "devicetree";
>  		/* Multiple IRQs are not supported */
>  	}
> @@ -204,7 +208,7 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
>  			break;
>  		}
>  
> -		uiomem->memtype = UIO_MEM_PHYS;
> +		uiomem->memtype = memtype;
>  		uiomem->addr = r->start & PAGE_MASK;
>  		uiomem->offs = r->start & ~PAGE_MASK;
>  		uiomem->size = (uiomem->offs + resource_size(r)
> -- 
> 2.30.2
> 
>