Re: [PATCH 2/5] soc: ti: Add IOMPU-like PVU driver

[Krzysztof Kozlowski <krzk@kernel.org>]

On 26/08/2024 19:56, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> The TI Peripheral Virtualization Unit (PVU) permits to define a limited
> set of mappings for DMA requests on the system memory. Unlike with an
> IOMMU, there is no fallback to a memory-backed page table, only a fixed
> set of register-backed TLBs. Emulating an IOMMU behavior appears to be
> the more fragile the more fragmentation of pending requests occur.
> 
> Therefore, this driver does not expose the PVU as an IOMMU. It rather
> introduces a simple, static interface to devices that are under
> restricted-dma-pool constraints. They can register their pools with the
> PVUs, enabling only those pools to work for DMA. As also MSI is issued
> as DMA, the PVU already register the related translator region of the
> AM654 as valid DMA target.
> 
> This driver is the essential building block for limiting DMA from
> untrusted devices to clearly defined memory regions in the absence of a
> real IOMMU (SMMU).
> 
> Co-developed-by: Diogo Ivo <diogo.ivo@siemens.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  drivers/soc/ti/Kconfig  |   4 +
>  drivers/soc/ti/Makefile |   1 +
>  drivers/soc/ti/ti-pvu.c | 487 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/ti-pvu.h  |  11 +
>  4 files changed, 503 insertions(+)
>  create mode 100644 drivers/soc/ti/ti-pvu.c
>  create mode 100644 include/linux/ti-pvu.h
> 
> diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
> index 1a93001c9e36..af7173ad84de 100644
> --- a/drivers/soc/ti/Kconfig
> +++ b/drivers/soc/ti/Kconfig
> @@ -82,6 +82,10 @@ config TI_PRUSS
>  	  processors on various TI SoCs. It's safe to say N here if you're
>  	  not interested in the PRU or if you are unsure.
>  

...

> +
> +static int ti_pvu_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *its_node;
> +	void __iomem *base;
> +	struct ti_pvu *pvu;
> +	u32 val;
> +	int ret;
> +
> +	pvu = devm_kzalloc(dev, sizeof(struct ti_pvu), GFP_KERNEL);

sizeof(*)

> +	if (!pvu)
> +		return -ENOMEM;
> +
> +	pvu->pdev = pdev;
> +
> +	base = devm_platform_ioremap_resource_byname(pdev, "cfg");
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	pvu->cfg = devm_regmap_init_mmio(dev, base, &pvu_cfg_regmap_cfg);
> +	if (IS_ERR(pvu->cfg))
> +		return dev_err_probe(dev, PTR_ERR(pvu->cfg), "failed to init cfg regmap");
> +
> +	ret = devm_regmap_field_bulk_alloc(dev, pvu->cfg, pvu->cfg_fields,
> +					   pvu_cfg_reg_fields, PVU_MAX_CFG_FIELDS);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to alloc cfg regmap fields");
> +
> +	pvu->num_tlbs = pvu_field_read(pvu, PVU_TLBS);
> +	pvu->num_entries = pvu_field_read(pvu, PVU_TLB_ENTRIES);
> +	dev_info(dev, "TLBs: %d, entries per TLB: %d\n", pvu->num_tlbs,
> +		 pvu->num_entries);
> +
> +	pvu->tlbif_base = devm_platform_ioremap_resource_byname(pdev, "tlbif");
> +	if (IS_ERR(pvu->tlbif_base))
> +		return PTR_ERR(pvu->tlbif_base);
> +
> +	its_node = of_find_compatible_node(0, 0, "arm,gic-v3-its");
> +	if (its_node) {
> +		u32 pre_its_window[2];
> +
> +		ret = of_property_read_u32_array(its_node,
> +						 "socionext,synquacer-pre-its",
> +						 pre_its_window,
> +						 ARRAY_SIZE(pre_its_window));
> +		if (ret) {
> +			dev_err(dev, "failed to read pre-its property\n");
> +			return ret;
> +		}
> +
> +		ret = pvu_create_region(pvu, pre_its_window[0],
> +					pre_its_window[1]);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	val = readl(pvu->tlbif_base + PVU_CHAIN);
> +	val |= PVU_CHAIN_EN;
> +	writel(val, pvu->tlbif_base + PVU_CHAIN);
> +
> +	pvu_field_write(pvu, PVU_DMA_CNT, 0);
> +	pvu_field_write(pvu, PVU_DMA_CL0, 0);
> +	pvu_field_write(pvu, PVU_DMA_CL1, 0);
> +	pvu_field_write(pvu, PVU_DMA_CL2, 0);
> +	pvu_field_write(pvu, PVU_DMA_CL3, 0);
> +	pvu_field_write(pvu, PVU_MAX_VIRTID, NUM_VIRTIDS);
> +
> +	ret = platform_get_irq(pdev, 0);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to get irq\n");
> +
> +	ret = devm_request_irq(dev, ret, pvu_fault_isr, 0, dev_name(dev), pvu);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to request irq\n");
> +
> +	pvu_field_write(pvu, PVU_EXC_ENABLE, 1);
> +	pvu_field_write(pvu, PVU_ENABLED, 1);
> +
> +	dev_set_drvdata(dev, pvu);
> +
> +	mutex_lock(&ti_pvu_lock);
> +	list_add(&pvu->entry, &ti_pvu_list);
> +	mutex_unlock(&ti_pvu_lock);
> +
> +	return 0;
> +}
> +
> +static void ti_pvu_remove(struct platform_device *pdev)
> +{
> +	struct ti_pvu *pvu = dev_get_drvdata(&pdev->dev);
> +
> +	mutex_lock(&ti_pvu_lock);
> +	list_del(&pvu->entry);
> +	mutex_unlock(&ti_pvu_lock);
> +}
> +
> +static const struct of_device_id ti_pvu_of_match[] = {
> +	{ .compatible = "ti,am654-pvu", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, ti_pvu_of_match);
> +
> +static struct platform_driver ti_pvu_driver = {
> +	.driver = {
> +		.name = "ti-pvu",
> +		.of_match_table = ti_pvu_of_match,
> +	},
> +	.probe = ti_pvu_probe,
> +	.remove_new = ti_pvu_remove,
> +};
> +module_platform_driver(ti_pvu_driver);
> diff --git a/include/linux/ti-pvu.h b/include/linux/ti-pvu.h
> new file mode 100644
> index 000000000000..d40642522cf0
> --- /dev/null
> +++ b/include/linux/ti-pvu.h
> @@ -0,0 +1,11 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * TI Peripheral Virtualization Unit driver for static DMA isolation
> + *
> + * Copyright (c) 2024, Siemens AG
> + */
> +

Missing guards.

> +#include <linux/ioport.h>
> +
> +int ti_pvu_create_region(unsigned int virt_id, const struct resource *region);
> +int ti_pvu_remove_region(unsigned int virt_id, const struct resource *region);


Best regards,
Krzysztof