Message-ID: <CAHp75VfVhVGFEQGeUKajrUKmkx_et_KakDDgroC2BrMMhd62yg@mail.gmail.com>
Date:   Fri, 27 Aug 2021 09:58:32 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Claire Chang <tientzu@...omium.org>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>,
        Frank Rowand <frowand.list@...il.com>,
        Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Juergen Gross <jgross@...e.com>,
        Christoph Hellwig <hch@....de>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        benh@...nel.crashing.org, paulus@...ba.org,
        "list@....net:IOMMU DRIVERS" <iommu@...ts.linux-foundation.org>,
        sstabellini@...nel.org, Robin Murphy <robin.murphy@....com>,
        grant.likely@....com, xypron.glpk@....de,
        Thierry Reding <treding@...dia.com>, mingo@...nel.org,
        bauerman@...ux.ibm.com, peterz@...radead.org,
        Greg KH <gregkh@...uxfoundation.org>,
        Saravana Kannan <saravanak@...gle.com>,
        "Rafael J . Wysocki" <rafael.j.wysocki@...el.com>,
        heikki.krogerus@...ux.intel.com,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Randy Dunlap <rdunlap@...radead.org>,
        Dan Williams <dan.j.williams@...el.com>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        linux-devicetree <devicetree@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        linuxppc-dev@...ts.ozlabs.org, xen-devel@...ts.xenproject.org,
        Nicolas Boichat <drinkcat@...omium.org>,
        Jim Quinlan <james.quinlan@...adcom.com>, tfiga@...omium.org,
        bskeggs@...hat.com, bhelgaas@...gle.com, chris@...is-wilson.co.uk,
        daniel@...ll.ch, airlied@...ux.ie, dri-devel@...ts.freedesktop.org,
        intel-gfx@...ts.freedesktop.org, jani.nikula@...ux.intel.com,
        jxgao@...gle.com, joonas.lahtinen@...ux.intel.com,
        linux-pci@...r.kernel.org, maarten.lankhorst@...ux.intel.com,
        matthew.auld@...el.com, rodrigo.vivi@...el.com,
        thomas.hellstrom@...ux.intel.com, thomas.lendacky@....com,
        quic_qiancai@...cinc.com
Subject: Re: [PATCH v15 10/12] swiotlb: Add restricted DMA pool initialization

On Thu, Jun 24, 2021 at 6:59 PM Claire Chang <tientzu@...omium.org> wrote:
>
> Add the initialization function to create restricted DMA pools from
> matching reserved-memory nodes.
>
> Regardless of swiotlb setting, the restricted DMA pool is preferred if
> available.
>
> The restricted DMA pools provide a basic level of protection against the
> DMA overwriting buffer contents at unexpected times. However, to protect
> against general data leakage and system memory corruption, the system
> needs to provide a way to lock down the memory access, e.g., MPU.





> +static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
> +                                   struct device *dev)
> +{
> +       struct io_tlb_mem *mem = rmem->priv;
> +       unsigned long nslabs = rmem->size >> IO_TLB_SHIFT;
> +
> +       /*
> +        * Since multiple devices can share the same pool, the private data,
> +        * io_tlb_mem struct, will be initialized by the first device attached
> +        * to it.
> +        */

> +       if (!mem) {

Can it be rather

if (mem)
  goto out_assign;

or so?

> +               mem = kzalloc(struct_size(mem, slots, nslabs), GFP_KERNEL);
> +               if (!mem)
> +                       return -ENOMEM;
> +
> +               set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
> +                                    rmem->size >> PAGE_SHIFT);

Below you are using a macro from pfn.h, but not here, I think it's PFN_DOWN().

> +               swiotlb_init_io_tlb_mem(mem, rmem->base, nslabs, false);
> +               mem->force_bounce = true;
> +               mem->for_alloc = true;
> +
> +               rmem->priv = mem;
> +
> +               if (IS_ENABLED(CONFIG_DEBUG_FS)) {
> +                       mem->debugfs =
> +                               debugfs_create_dir(rmem->name, debugfs_dir);
> +                       swiotlb_create_debugfs_files(mem);
> +               }
> +       }
> +
> +       dev->dma_io_tlb_mem = mem;
> +
> +       return 0;
> +}
> +
> +static void rmem_swiotlb_device_release(struct reserved_mem *rmem,
> +                                       struct device *dev)
> +{
> +       dev->dma_io_tlb_mem = io_tlb_default_mem;
> +}
> +
> +static const struct reserved_mem_ops rmem_swiotlb_ops = {
> +       .device_init = rmem_swiotlb_device_init,
> +       .device_release = rmem_swiotlb_device_release,
> +};
> +
> +static int __init rmem_swiotlb_setup(struct reserved_mem *rmem)
> +{
> +       unsigned long node = rmem->fdt_node;
> +
> +       if (of_get_flat_dt_prop(node, "reusable", NULL) ||
> +           of_get_flat_dt_prop(node, "linux,cma-default", NULL) ||
> +           of_get_flat_dt_prop(node, "linux,dma-default", NULL) ||
> +           of_get_flat_dt_prop(node, "no-map", NULL))
> +               return -EINVAL;
> +
> +       if (PageHighMem(pfn_to_page(PHYS_PFN(rmem->base)))) {
> +               pr_err("Restricted DMA pool must be accessible within the linear mapping.");
> +               return -EINVAL;
> +       }
> +
> +       rmem->ops = &rmem_swiotlb_ops;
> +       pr_info("Reserved memory: created restricted DMA pool at %pa, size %ld MiB\n",
> +               &rmem->base, (unsigned long)rmem->size / SZ_1M);

Oh là là, besides explicit casting that I believe can be avoided, %ld
!= unsigned long. Can you check the printk-formats.rst document?

> +       return 0;
> +}
> +
> +RESERVEDMEM_OF_DECLARE(dma, "restricted-dma-pool", rmem_swiotlb_setup);
>  #endif /* CONFIG_DMA_RESTRICTED_POOL */

-- 
With Best Regards,
Andy Shevchenko

Powered by blists - more mailing lists