diff mbox series

[1/2] memory: introduce memory_region_init_ram_protected()

Message ID 20230620195054.23929-2-lvivier@redhat.com (mailing list archive)
State New, archived
Headers show
Series vhost-vdpa: skip TPM CRB memory section | expand

Commit Message

Laurent Vivier June 20, 2023, 7:50 p.m. UTC
Commit 56918a126a ("memory: Add RAM_PROTECTED flag to skip IOMMU mappings")
has introduced the RAM_PROTECTED flag to denote "protected" memory.

This flags is only used with qemu_ram_alloc_from_fd() for now.

To be able to register memory region with this flag, define
memory_region_init_ram_protected() and declare the flag as valid in
qemu_ram_alloc_internal() and qemu_ram_alloc().

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 include/exec/memory.h | 33 +++++++++++++++++++++++++++++++++
 softmmu/memory.c      | 33 +++++++++++++++++++++++++++------
 softmmu/physmem.c     |  4 ++--
 3 files changed, 62 insertions(+), 8 deletions(-)

Comments

Stefan Berger June 21, 2023, 12:27 p.m. UTC | #1
On 6/20/23 15:50, Laurent Vivier wrote:
> Commit 56918a126a ("memory: Add RAM_PROTECTED flag to skip IOMMU mappings")
> has introduced the RAM_PROTECTED flag to denote "protected" memory.
> 
> This flags is only used with qemu_ram_alloc_from_fd() for now.
> 
> To be able to register memory region with this flag, define
> memory_region_init_ram_protected() and declare the flag as valid in
> qemu_ram_alloc_internal() and qemu_ram_alloc().
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>   include/exec/memory.h | 33 +++++++++++++++++++++++++++++++++
>   softmmu/memory.c      | 33 +++++++++++++++++++++++++++------
>   softmmu/physmem.c     |  4 ++--
>   3 files changed, 62 insertions(+), 8 deletions(-)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 47c2e0221c35..d8760015c381 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -1520,6 +1520,39 @@ void memory_region_init_iommu(void *_iommu_mr,
>                                 const char *name,
>                                 uint64_t size);
> 
> +/**
> + * memory_region_init_ram_protected - Initialize RAM memory region.  Accesses
> + *                                    into the region will modify memory
> + *                                    directly.
> + *
> + * The memory is created with the RAM_PROTECTED flag, for memory that
> + * looks and acts like RAM but inaccessible via normal mechanisms,

but is inaccessible


> + * including DMA.
> + *
> + * @mr: the #MemoryRegion to be initialized
> + * @owner: the object that tracks the region's reference count (must be
> + *         TYPE_DEVICE or a subclass of TYPE_DEVICE, or NULL)
> + * @name: name of the memory region
> + * @size: size of the region in bytes
> + * @errp: pointer to Error*, to store an error if it happens.
> + *
> + * This function allocates RAM for a board model or device, and
> + * arranges for it to be migrated (by calling vmstate_register_ram()
> + * if @owner is a DeviceState, or vmstate_register_ram_global() if
> + * @owner is NULL).
> + *
> + * TODO: Currently we restrict @owner to being either NULL (for
> + * global RAM regions with no owner) or devices, so that we can
> + * give the RAM block a unique name for migration purposes.
> + * We should lift this restriction and allow arbitrary Objects.
> + * If you pass a non-NULL non-device @owner then we will assert.
> + */
> +void memory_region_init_ram_protected(MemoryRegion *mr,
> +                                      Object *owner,
> +                                      const char *name,
> +                                      uint64_t size,
> +                                      Error **errp);
> +
>   /**
>    * memory_region_init_ram - Initialize RAM memory region.  Accesses into the
>    *                          region will modify memory directly.
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index 7d9494ce7028..952c87277353 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -3551,16 +3551,18 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
>       }
>   }
> 
> -void memory_region_init_ram(MemoryRegion *mr,
> -                            Object *owner,
> -                            const char *name,
> -                            uint64_t size,
> -                            Error **errp)
> +static void memory_region_init_ram_flags(MemoryRegion *mr,
> +                                         Object *owner,
> +                                         const char *name,
> +                                         uint64_t size,
> +                                         uint32_t ram_flags,
> +                                         Error **errp)
>   {
>       DeviceState *owner_dev;
>       Error *err = NULL;
> 
> -    memory_region_init_ram_nomigrate(mr, owner, name, size, &err);
> +    memory_region_init_ram_flags_nomigrate(mr, owner, name, size, ram_flags,
> +                                           &err);
>       if (err) {
>           error_propagate(errp, err);
>           return;
> @@ -3575,6 +3577,25 @@ void memory_region_init_ram(MemoryRegion *mr,
>       vmstate_register_ram(mr, owner_dev);
>   }
> 
> +void memory_region_init_ram_protected(MemoryRegion *mr,
> +                                      Object *owner,
> +                                      const char *name,
> +                                      uint64_t size,
> +                                      Error **errp)
> +{
> +        memory_region_init_ram_flags(mr, owner, name, size, RAM_PROTECTED,
> +                                     errp);
> +}
> +
> +void memory_region_init_ram(MemoryRegion *mr,
> +                            Object *owner,
> +                            const char *name,
> +                            uint64_t size,
> +                            Error **errp)
> +{
> +        memory_region_init_ram_flags(mr, owner, name, size, 0, errp);
> +}
> +
>   void memory_region_init_rom(MemoryRegion *mr,
>                               Object *owner,
>                               const char *name,
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index 6bdd944fe880..bf66c81e7255 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -1978,7 +1978,7 @@ RAMBlock *qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size,
>       Error *local_err = NULL;
> 
>       assert((ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC |
> -                          RAM_NORESERVE)) == 0);
> +                          RAM_NORESERVE | RAM_PROTECTED)) == 0);
>       assert(!host ^ (ram_flags & RAM_PREALLOC));
> 
>       size = HOST_PAGE_ALIGN(size);
> @@ -2012,7 +2012,7 @@ RAMBlock *qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
>   RAMBlock *qemu_ram_alloc(ram_addr_t size, uint32_t ram_flags,
>                            MemoryRegion *mr, Error **errp)
>   {
> -    assert((ram_flags & ~(RAM_SHARED | RAM_NORESERVE)) == 0);
> +    assert((ram_flags & ~(RAM_SHARED | RAM_NORESERVE | RAM_PROTECTED)) == 0);
>       return qemu_ram_alloc_internal(size, size, NULL, NULL, ram_flags, mr, errp);
>   }
> 

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
David Hildenbrand June 22, 2023, 1:05 p.m. UTC | #2
On 20.06.23 21:50, Laurent Vivier wrote:
> Commit 56918a126a ("memory: Add RAM_PROTECTED flag to skip IOMMU mappings")
> has introduced the RAM_PROTECTED flag to denote "protected" memory.
> 
> This flags is only used with qemu_ram_alloc_from_fd() for now.
> 
> To be able to register memory region with this flag, define
> memory_region_init_ram_protected() and declare the flag as valid in
> qemu_ram_alloc_internal() and qemu_ram_alloc().
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>   include/exec/memory.h | 33 +++++++++++++++++++++++++++++++++
>   softmmu/memory.c      | 33 +++++++++++++++++++++++++++------
>   softmmu/physmem.c     |  4 ++--
>   3 files changed, 62 insertions(+), 8 deletions(-)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 47c2e0221c35..d8760015c381 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -1520,6 +1520,39 @@ void memory_region_init_iommu(void *_iommu_mr,
>                                 const char *name,
>                                 uint64_t size);
>   
> +/**
> + * memory_region_init_ram_protected - Initialize RAM memory region.  Accesses
> + *                                    into the region will modify memory
> + *                                    directly.
> + *
> + * The memory is created with the RAM_PROTECTED flag, for memory that
> + * looks and acts like RAM but inaccessible via normal mechanisms,
> + * including DMA.
> + *
> + * @mr: the #MemoryRegion to be initialized
> + * @owner: the object that tracks the region's reference count (must be
> + *         TYPE_DEVICE or a subclass of TYPE_DEVICE, or NULL)
> + * @name: name of the memory region
> + * @size: size of the region in bytes
> + * @errp: pointer to Error*, to store an error if it happens.
> + *
> + * This function allocates RAM for a board model or device, and
> + * arranges for it to be migrated (by calling vmstate_register_ram()
> + * if @owner is a DeviceState, or vmstate_register_ram_global() if
> + * @owner is NULL).
> + *
> + * TODO: Currently we restrict @owner to being either NULL (for
> + * global RAM regions with no owner) or devices, so that we can
> + * give the RAM block a unique name for migration purposes.
> + * We should lift this restriction and allow arbitrary Objects.
> + * If you pass a non-NULL non-device @owner then we will assert.
> + */
> +void memory_region_init_ram_protected(MemoryRegion *mr,
> +                                      Object *owner,
> +                                      const char *name,
> +                                      uint64_t size,
> +                                      Error **errp);
> +
>   /**
>    * memory_region_init_ram - Initialize RAM memory region.  Accesses into the
>    *                          region will modify memory directly.
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index 7d9494ce7028..952c87277353 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -3551,16 +3551,18 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
>       }
>   }
>   
> -void memory_region_init_ram(MemoryRegion *mr,
> -                            Object *owner,
> -                            const char *name,
> -                            uint64_t size,
> -                            Error **errp)
> +static void memory_region_init_ram_flags(MemoryRegion *mr,
> +                                         Object *owner,
> +                                         const char *name,
> +                                         uint64_t size,
> +                                         uint32_t ram_flags,
> +                                         Error **errp)
>   {
>       DeviceState *owner_dev;
>       Error *err = NULL;
>   
> -    memory_region_init_ram_nomigrate(mr, owner, name, size, &err);
> +    memory_region_init_ram_flags_nomigrate(mr, owner, name, size, ram_flags,
> +                                           &err);
>       if (err) {
>           error_propagate(errp, err);
>           return;
> @@ -3575,6 +3577,25 @@ void memory_region_init_ram(MemoryRegion *mr,
>       vmstate_register_ram(mr, owner_dev);
>   }
>   
> +void memory_region_init_ram_protected(MemoryRegion *mr,
> +                                      Object *owner,
> +                                      const char *name,
> +                                      uint64_t size,
> +                                      Error **errp)
> +{
> +        memory_region_init_ram_flags(mr, owner, name, size, RAM_PROTECTED,
> +                                     errp);
> +}
> +
> +void memory_region_init_ram(MemoryRegion *mr,
> +                            Object *owner,
> +                            const char *name,
> +                            uint64_t size,
> +                            Error **errp)
> +{
> +        memory_region_init_ram_flags(mr, owner, name, size, 0, errp);
> +}
> +
>   void memory_region_init_rom(MemoryRegion *mr,
>                               Object *owner,
>                               const char *name,
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index 6bdd944fe880..bf66c81e7255 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -1978,7 +1978,7 @@ RAMBlock *qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size,
>       Error *local_err = NULL;
>   
>       assert((ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC |
> -                          RAM_NORESERVE)) == 0);
> +                          RAM_NORESERVE | RAM_PROTECTED)) == 0);
>       assert(!host ^ (ram_flags & RAM_PREALLOC));
>   
>       size = HOST_PAGE_ALIGN(size);
> @@ -2012,7 +2012,7 @@ RAMBlock *qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
>   RAMBlock *qemu_ram_alloc(ram_addr_t size, uint32_t ram_flags,
>                            MemoryRegion *mr, Error **errp)
>   {
> -    assert((ram_flags & ~(RAM_SHARED | RAM_NORESERVE)) == 0);
> +    assert((ram_flags & ~(RAM_SHARED | RAM_NORESERVE | RAM_PROTECTED)) == 0);
>       return qemu_ram_alloc_internal(size, size, NULL, NULL, ram_flags, mr, errp);
>   }
>   

Reviewed-by: David Hildenbrand <david@redhat.com>
Peter Maydell June 22, 2023, 1:16 p.m. UTC | #3
On Tue, 20 Jun 2023 at 20:51, Laurent Vivier <lvivier@redhat.com> wrote:
>
> Commit 56918a126a ("memory: Add RAM_PROTECTED flag to skip IOMMU mappings")
> has introduced the RAM_PROTECTED flag to denote "protected" memory.
>
> This flags is only used with qemu_ram_alloc_from_fd() for now.
>
> To be able to register memory region with this flag, define
> memory_region_init_ram_protected() and declare the flag as valid in
> qemu_ram_alloc_internal() and qemu_ram_alloc().
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  include/exec/memory.h | 33 +++++++++++++++++++++++++++++++++
>  softmmu/memory.c      | 33 +++++++++++++++++++++++++++------
>  softmmu/physmem.c     |  4 ++--
>  3 files changed, 62 insertions(+), 8 deletions(-)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 47c2e0221c35..d8760015c381 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -1520,6 +1520,39 @@ void memory_region_init_iommu(void *_iommu_mr,
>                                const char *name,
>                                uint64_t size);
>
> +/**
> + * memory_region_init_ram_protected - Initialize RAM memory region.  Accesses
> + *                                    into the region will modify memory
> + *                                    directly.
> + *
> + * The memory is created with the RAM_PROTECTED flag, for memory that
> + * looks and acts like RAM but inaccessible via normal mechanisms,
> + * including DMA.

This doesn't really tell me why you might want to mark
a region as RAM_PROTECTED. What kind of memory region is
not DMAable to? What are "normal mechanisms" here?
What are the "non-normal mechanisms" that you *can* use on
this memory region?

At the moment we only seem to use RAM_PROTECTED for
the SGX EPC memory backend. The commit message adding
that flag is pretty vague about what it means...

thanks
-- PMM
diff mbox series

Patch

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 47c2e0221c35..d8760015c381 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1520,6 +1520,39 @@  void memory_region_init_iommu(void *_iommu_mr,
                               const char *name,
                               uint64_t size);
 
+/**
+ * memory_region_init_ram_protected - Initialize RAM memory region.  Accesses
+ *                                    into the region will modify memory
+ *                                    directly.
+ *
+ * The memory is created with the RAM_PROTECTED flag, for memory that
+ * looks and acts like RAM but inaccessible via normal mechanisms,
+ * including DMA.
+ *
+ * @mr: the #MemoryRegion to be initialized
+ * @owner: the object that tracks the region's reference count (must be
+ *         TYPE_DEVICE or a subclass of TYPE_DEVICE, or NULL)
+ * @name: name of the memory region
+ * @size: size of the region in bytes
+ * @errp: pointer to Error*, to store an error if it happens.
+ *
+ * This function allocates RAM for a board model or device, and
+ * arranges for it to be migrated (by calling vmstate_register_ram()
+ * if @owner is a DeviceState, or vmstate_register_ram_global() if
+ * @owner is NULL).
+ *
+ * TODO: Currently we restrict @owner to being either NULL (for
+ * global RAM regions with no owner) or devices, so that we can
+ * give the RAM block a unique name for migration purposes.
+ * We should lift this restriction and allow arbitrary Objects.
+ * If you pass a non-NULL non-device @owner then we will assert.
+ */
+void memory_region_init_ram_protected(MemoryRegion *mr,
+                                      Object *owner,
+                                      const char *name,
+                                      uint64_t size,
+                                      Error **errp);
+
 /**
  * memory_region_init_ram - Initialize RAM memory region.  Accesses into the
  *                          region will modify memory directly.
diff --git a/softmmu/memory.c b/softmmu/memory.c
index 7d9494ce7028..952c87277353 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -3551,16 +3551,18 @@  void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
     }
 }
 
-void memory_region_init_ram(MemoryRegion *mr,
-                            Object *owner,
-                            const char *name,
-                            uint64_t size,
-                            Error **errp)
+static void memory_region_init_ram_flags(MemoryRegion *mr,
+                                         Object *owner,
+                                         const char *name,
+                                         uint64_t size,
+                                         uint32_t ram_flags,
+                                         Error **errp)
 {
     DeviceState *owner_dev;
     Error *err = NULL;
 
-    memory_region_init_ram_nomigrate(mr, owner, name, size, &err);
+    memory_region_init_ram_flags_nomigrate(mr, owner, name, size, ram_flags,
+                                           &err);
     if (err) {
         error_propagate(errp, err);
         return;
@@ -3575,6 +3577,25 @@  void memory_region_init_ram(MemoryRegion *mr,
     vmstate_register_ram(mr, owner_dev);
 }
 
+void memory_region_init_ram_protected(MemoryRegion *mr,
+                                      Object *owner,
+                                      const char *name,
+                                      uint64_t size,
+                                      Error **errp)
+{
+        memory_region_init_ram_flags(mr, owner, name, size, RAM_PROTECTED,
+                                     errp);
+}
+
+void memory_region_init_ram(MemoryRegion *mr,
+                            Object *owner,
+                            const char *name,
+                            uint64_t size,
+                            Error **errp)
+{
+        memory_region_init_ram_flags(mr, owner, name, size, 0, errp);
+}
+
 void memory_region_init_rom(MemoryRegion *mr,
                             Object *owner,
                             const char *name,
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 6bdd944fe880..bf66c81e7255 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -1978,7 +1978,7 @@  RAMBlock *qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size,
     Error *local_err = NULL;
 
     assert((ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC |
-                          RAM_NORESERVE)) == 0);
+                          RAM_NORESERVE | RAM_PROTECTED)) == 0);
     assert(!host ^ (ram_flags & RAM_PREALLOC));
 
     size = HOST_PAGE_ALIGN(size);
@@ -2012,7 +2012,7 @@  RAMBlock *qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
 RAMBlock *qemu_ram_alloc(ram_addr_t size, uint32_t ram_flags,
                          MemoryRegion *mr, Error **errp)
 {
-    assert((ram_flags & ~(RAM_SHARED | RAM_NORESERVE)) == 0);
+    assert((ram_flags & ~(RAM_SHARED | RAM_NORESERVE | RAM_PROTECTED)) == 0);
     return qemu_ram_alloc_internal(size, size, NULL, NULL, ram_flags, mr, errp);
 }