Re: [PATCH v4 13/14] dt-bindings: of: Add restricted DMA pool
On 3/10/2021 1:40 PM, Rob Herring wrote: > On Wed, Mar 10, 2021 at 9:08 AM Will Deacon wrote: >> >> Hi Claire, >> >> On Tue, Feb 09, 2021 at 02:21:30PM +0800, Claire Chang wrote: >>> Introduce the new compatible string, restricted-dma-pool, for restricted >>> DMA. One can specify the address and length of the restricted DMA memory >>> region by restricted-dma-pool in the reserved-memory node. >>> >>> Signed-off-by: Claire Chang >>> --- >>> .../reserved-memory/reserved-memory.txt | 24 +++ >>> 1 file changed, 24 insertions(+) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt >>> b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt >>> index e8d3096d922c..fc9a12c2f679 100644 >>> --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt >>> +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt >>> @@ -51,6 +51,20 @@ compatible (optional) - standard definition >>>used as a shared pool of DMA buffers for a set of devices. It can >>>be used by an operating system to instantiate the necessary pool >>>management subsystem if necessary. >>> +- restricted-dma-pool: This indicates a region of memory meant to >>> be >>> + used as a pool of restricted DMA buffers for a set of devices. >>> The >>> + memory region would be the only region accessible to those >>> devices. >>> + When using this, the no-map and reusable properties must not be >>> set, >>> + so the operating system can create a virtual mapping that will >>> be used >>> + for synchronization. The main purpose for restricted DMA is to >>> + mitigate the lack of DMA access control on systems without an >>> IOMMU, >>> + which could result in the DMA accessing the system memory at >>> + unexpected times and/or unexpected addresses, possibly leading >>> to data >>> + leakage or corruption. The feature on its own provides 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 way to >>> lock down >>> + the memory access, e.g., MPU. >> >> As far as I can tell, these pools work with both static allocations (which >> seem to match your use-case where firmware has preconfigured the DMA ranges) >> but also with dynamic allocations where a 'size' property is present instead >> of the 'reg' property and the kernel is responsible for allocating the >> reservation during boot. Am I right and, if so, is that deliberate? > > I believe so. I'm not keen on having size only reservations in DT. > Yes, we allowed that already, but that's back from the days of needing > large CMA carveouts to be reserved early in boot. I've read that the > kernel is much better now at contiguous allocations, so do we really > need this in DT anymore? I would say yes, there can be a number of times where you want to semi statically partition your physical memory and their reserved regions. Be it to pack everything together under the same protection rules or because you need to allocate memory from a particular address range in say a non-uniform memory controller architecture where address windows have different scheduling algorithms. -- Florian
Re: [PATCH v4 13/14] dt-bindings: of: Add restricted DMA pool
On Wed, Mar 10, 2021 at 9:08 AM Will Deacon wrote:
>
> Hi Claire,
>
> On Tue, Feb 09, 2021 at 02:21:30PM +0800, Claire Chang wrote:
> > Introduce the new compatible string, restricted-dma-pool, for restricted
> > DMA. One can specify the address and length of the restricted DMA memory
> > region by restricted-dma-pool in the reserved-memory node.
> >
> > Signed-off-by: Claire Chang
> > ---
> > .../reserved-memory/reserved-memory.txt | 24 +++
> > 1 file changed, 24 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > index e8d3096d922c..fc9a12c2f679 100644
> > --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > @@ -51,6 +51,20 @@ compatible (optional) - standard definition
> >used as a shared pool of DMA buffers for a set of devices. It can
> >be used by an operating system to instantiate the necessary pool
> >management subsystem if necessary.
> > +- restricted-dma-pool: This indicates a region of memory meant to
> > be
> > + used as a pool of restricted DMA buffers for a set of devices.
> > The
> > + memory region would be the only region accessible to those
> > devices.
> > + When using this, the no-map and reusable properties must not be
> > set,
> > + so the operating system can create a virtual mapping that will
> > be used
> > + for synchronization. The main purpose for restricted DMA is to
> > + mitigate the lack of DMA access control on systems without an
> > IOMMU,
> > + which could result in the DMA accessing the system memory at
> > + unexpected times and/or unexpected addresses, possibly leading
> > to data
> > + leakage or corruption. The feature on its own provides 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 way to
> > lock down
> > + the memory access, e.g., MPU.
>
> As far as I can tell, these pools work with both static allocations (which
> seem to match your use-case where firmware has preconfigured the DMA ranges)
> but also with dynamic allocations where a 'size' property is present instead
> of the 'reg' property and the kernel is responsible for allocating the
> reservation during boot. Am I right and, if so, is that deliberate?
I believe so. I'm not keen on having size only reservations in DT.
Yes, we allowed that already, but that's back from the days of needing
large CMA carveouts to be reserved early in boot. I've read that the
kernel is much better now at contiguous allocations, so do we really
need this in DT anymore?
> I ask because I think that would potentially be useful to us for the
> Protected KVM work, where we need to bounce virtio memory accesses via
> guest-determined windows because the guest memory is generally inaccessible
> to the host. We've been hacking this using a combination of "swiotlb=force"
> and set_memory_{decrypted,encrypted}() but it would be much better to
> leverage the stuff you have here.
>
> Also:
>
> > +
> > + restricted_dma_mem_reserved: restricted_dma_mem_reserved {
> > + compatible = "restricted-dma-pool";
> > + reg = <0x5000 0x40>;
> > + };
> > };
> >
> > /* ... */
> > @@ -138,4 +157,9 @@ one for multimedia processing (named
> > multimedia-memory@7700, 64MiB).
> > memory-region = <_reserved>;
> > /* ... */
> > };
> > +
> > + pcie_device: pcie_device@0,0 {
> > + memory-region = <_dma_mem_reserved>;
> > + /* ... */
> > + };
>
> I find this example a bit weird, as I didn't think we usually had DT nodes
> for PCI devices; rather they are discovered as a result of probing config
> space. Is the idea that you have one reserved memory region attached to the
> RC and all the PCI devices below that share the region, or is there a need
> for a mapping mechanism?
We can have DT nodes for PCI. AIUI, IBM power systems always do. For
FDT, it's only if there are extra non-discoverable resources. It's
particularly fun when it's resources which need to be enabled for the
PCI device to be discovered. That seems to be a growing problem as PCI
becomes more common on embedded systems.
Rob
Re: [PATCH v4 13/14] dt-bindings: of: Add restricted DMA pool
Hi Claire,
On Tue, Feb 09, 2021 at 02:21:30PM +0800, Claire Chang wrote:
> Introduce the new compatible string, restricted-dma-pool, for restricted
> DMA. One can specify the address and length of the restricted DMA memory
> region by restricted-dma-pool in the reserved-memory node.
>
> Signed-off-by: Claire Chang
> ---
> .../reserved-memory/reserved-memory.txt | 24 +++
> 1 file changed, 24 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> index e8d3096d922c..fc9a12c2f679 100644
> --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> @@ -51,6 +51,20 @@ compatible (optional) - standard definition
>used as a shared pool of DMA buffers for a set of devices. It can
>be used by an operating system to instantiate the necessary pool
>management subsystem if necessary.
> +- restricted-dma-pool: This indicates a region of memory meant to be
> + used as a pool of restricted DMA buffers for a set of devices. The
> + memory region would be the only region accessible to those devices.
> + When using this, the no-map and reusable properties must not be
> set,
> + so the operating system can create a virtual mapping that will be
> used
> + for synchronization. The main purpose for restricted DMA is to
> + mitigate the lack of DMA access control on systems without an
> IOMMU,
> + which could result in the DMA accessing the system memory at
> + unexpected times and/or unexpected addresses, possibly leading to
> data
> + leakage or corruption. The feature on its own provides 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 way to lock
> down
> + the memory access, e.g., MPU.
As far as I can tell, these pools work with both static allocations (which
seem to match your use-case where firmware has preconfigured the DMA ranges)
but also with dynamic allocations where a 'size' property is present instead
of the 'reg' property and the kernel is responsible for allocating the
reservation during boot. Am I right and, if so, is that deliberate?
I ask because I think that would potentially be useful to us for the
Protected KVM work, where we need to bounce virtio memory accesses via
guest-determined windows because the guest memory is generally inaccessible
to the host. We've been hacking this using a combination of "swiotlb=force"
and set_memory_{decrypted,encrypted}() but it would be much better to
leverage the stuff you have here.
Also:
> +
> + restricted_dma_mem_reserved: restricted_dma_mem_reserved {
> + compatible = "restricted-dma-pool";
> + reg = <0x5000 0x40>;
> + };
> };
>
> /* ... */
> @@ -138,4 +157,9 @@ one for multimedia processing (named
> multimedia-memory@7700, 64MiB).
> memory-region = <_reserved>;
> /* ... */
> };
> +
> + pcie_device: pcie_device@0,0 {
> + memory-region = <_dma_mem_reserved>;
> + /* ... */
> + };
I find this example a bit weird, as I didn't think we usually had DT nodes
for PCI devices; rather they are discovered as a result of probing config
space. Is the idea that you have one reserved memory region attached to the
RC and all the PCI devices below that share the region, or is there a need
for a mapping mechanism?
Will
[PATCH v4 13/14] dt-bindings: of: Add restricted DMA pool
Introduce the new compatible string, restricted-dma-pool, for restricted
DMA. One can specify the address and length of the restricted DMA memory
region by restricted-dma-pool in the reserved-memory node.
Signed-off-by: Claire Chang
---
.../reserved-memory/reserved-memory.txt | 24 +++
1 file changed, 24 insertions(+)
diff --git
a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
index e8d3096d922c..fc9a12c2f679 100644
--- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
@@ -51,6 +51,20 @@ compatible (optional) - standard definition
used as a shared pool of DMA buffers for a set of devices. It can
be used by an operating system to instantiate the necessary pool
management subsystem if necessary.
+- restricted-dma-pool: This indicates a region of memory meant to be
+ used as a pool of restricted DMA buffers for a set of devices. The
+ memory region would be the only region accessible to those devices.
+ When using this, the no-map and reusable properties must not be set,
+ so the operating system can create a virtual mapping that will be
used
+ for synchronization. The main purpose for restricted DMA is to
+ mitigate the lack of DMA access control on systems without an IOMMU,
+ which could result in the DMA accessing the system memory at
+ unexpected times and/or unexpected addresses, possibly leading to
data
+ leakage or corruption. The feature on its own provides 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 way to lock
down
+ the memory access, e.g., MPU.
- vendor specific string in the form ,[-]
no-map (optional) - empty property
- Indicates the operating system must not create a virtual mapping
@@ -120,6 +134,11 @@ one for multimedia processing (named
multimedia-memory@7700, 64MiB).
compatible = "acme,multimedia-memory";
reg = <0x7700 0x400>;
};
+
+ restricted_dma_mem_reserved: restricted_dma_mem_reserved {
+ compatible = "restricted-dma-pool";
+ reg = <0x5000 0x40>;
+ };
};
/* ... */
@@ -138,4 +157,9 @@ one for multimedia processing (named
multimedia-memory@7700, 64MiB).
memory-region = <_reserved>;
/* ... */
};
+
+ pcie_device: pcie_device@0,0 {
+ memory-region = <_dma_mem_reserved>;
+ /* ... */
+ };
};
--
2.30.0.478.g8a0d178c01-goog
