[PATCH 0/5] docs: improve the memory API documentation

[PATCH 0/5] docs: improve the memory API documentation

[Alex Bennée]

As I've been looking through the Memory API for our Xen work I thought
I should take the time to clean it up. I needed to teach kdoc about
our QLIST_ macros and I found at least one unused field in the
structure.

Looking through the definitions I do wander if the meaning of
romd_mode and ram_device could be cleaned up to a single bool
(directly_mapped or use_ops?).

Anyway for now just cosmetic improvements to the docs as we are close
to softfreeze.

Alex.

Alex Bennée (5):
  scripts/kernel-doc: teach kdoc about QLIST_ macros
  docs: include ramblock.h in the memory API docs
  include/exec: remove warning_printed from MemoryRegion
  include/exec: annotate all the MemoryRegion fields
  docs/devel: mark out defined functions and structures

 docs/devel/memory.rst   | 49 +++++++++++++-------------
 include/exec/memory.h   | 48 +++++++++++++++++++++++---
 include/exec/ramblock.h | 76 +++++++++++++++++++++++++++--------------
 scripts/kernel-doc      |  9 ++++-
 4 files changed, 127 insertions(+), 55 deletions(-)

-- 
2.39.2

[PATCH 1/5] scripts/kernel-doc: teach kdoc about QLIST_ macros

[Alex Bennée]

The kernel-doc script does some pre-processing on structure
definitions before parsing for names. Teach it about QLIST and replace
with simplified structures representing the base type.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 scripts/kernel-doc | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 240923d509a..26c47562e79 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1226,7 +1226,14 @@ sub dump_struct($$) {
 	# replace DECLARE_KFIFO_PTR
 	$members =~ s/DECLARE_KFIFO_PTR\s*\(([^,)]+),\s*([^,)]+)\)/$2 \*$1/gos;
 
-	my $declaration = $members;
+        # QEMU Specific Macros
+
+        # replace QLIST_ENTRY with base type and variable name
+        $members =~ s/QLIST_ENTRY\(([^)]+)\)\s+([^;]+)/$1 \*$2/gos;
+        # replace QLIST_HEAD, optionally capturing an anonymous struct marker, and capture type and variable name
+        $members =~ s/QLIST_HEAD\(\s*,\s*([^)]+)\)\s+([^;]+)/struct { $1 *lh_first; } $2/gos;
+
+        my $declaration = $members;
 
 	# Split nested struct/union elements as newer ones
 	while ($members =~ m/(struct|union)([^\{\};]+)\{([^\{\}]*)\}([^\{\}\;]*)\;/) {
-- 
2.39.2

Re: [PATCH 1/5] scripts/kernel-doc: teach kdoc about QLIST_ macros

[Peter Xu]

On Thu, Mar 07, 2024 at 06:11:01PM +0000, Alex Bennée wrote:
> The kernel-doc script does some pre-processing on structure
> definitions before parsing for names. Teach it about QLIST and replace
> with simplified structures representing the base type.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  scripts/kernel-doc | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index 240923d509a..26c47562e79 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -1226,7 +1226,14 @@ sub dump_struct($$) {
>  	# replace DECLARE_KFIFO_PTR
>  	$members =~ s/DECLARE_KFIFO_PTR\s*\(([^,)]+),\s*([^,)]+)\)/$2 \*$1/gos;
>  
> -	my $declaration = $members;
> +        # QEMU Specific Macros
> +
> +        # replace QLIST_ENTRY with base type and variable name
> +        $members =~ s/QLIST_ENTRY\(([^)]+)\)\s+([^;]+)/$1 \*$2/gos;
> +        # replace QLIST_HEAD, optionally capturing an anonymous struct marker, and capture type and variable name
> +        $members =~ s/QLIST_HEAD\(\s*,\s*([^)]+)\)\s+([^;]+)/struct { $1 *lh_first; } $2/gos;
> +
> +        my $declaration = $members;

May need a "tabify" here..

-- 
Peter Xu

Re: [PATCH 1/5] scripts/kernel-doc: teach kdoc about QLIST_ macros

[Alex Bennée]

Peter Xu <peterx@redhat.com> writes:

> On Thu, Mar 07, 2024 at 06:11:01PM +0000, Alex Bennée wrote:
>> The kernel-doc script does some pre-processing on structure
>> definitions before parsing for names. Teach it about QLIST and replace
>> with simplified structures representing the base type.
>> 
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  scripts/kernel-doc | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>> 
>> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
>> index 240923d509a..26c47562e79 100755
>> --- a/scripts/kernel-doc
>> +++ b/scripts/kernel-doc
>> @@ -1226,7 +1226,14 @@ sub dump_struct($$) {
>>  	# replace DECLARE_KFIFO_PTR
>>  	$members =~ s/DECLARE_KFIFO_PTR\s*\(([^,)]+),\s*([^,)]+)\)/$2 \*$1/gos;
>>  
>> -	my $declaration = $members;
>> +        # QEMU Specific Macros
>> +
>> +        # replace QLIST_ENTRY with base type and variable name
>> +        $members =~ s/QLIST_ENTRY\(([^)]+)\)\s+([^;]+)/$1 \*$2/gos;
>> +        # replace QLIST_HEAD, optionally capturing an anonymous struct marker, and capture type and variable name
>> +        $members =~ s/QLIST_HEAD\(\s*,\s*([^)]+)\)\s+([^;]+)/struct { $1 *lh_first; } $2/gos;
>> +
>> +        my $declaration = $members;
>
> May need a "tabify" here..

Ugg that file is a mess. Any idea what we should use for perl, tabs or
spaces? I can update editorconfig.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 1/5] scripts/kernel-doc: teach kdoc about QLIST_ macros

[Peter Xu]

On Fri, Mar 08, 2024 at 08:09:15AM +0000, Alex Bennée wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > On Thu, Mar 07, 2024 at 06:11:01PM +0000, Alex Bennée wrote:
> >> The kernel-doc script does some pre-processing on structure
> >> definitions before parsing for names. Teach it about QLIST and replace
> >> with simplified structures representing the base type.
> >> 
> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >> ---
> >>  scripts/kernel-doc | 9 ++++++++-
> >>  1 file changed, 8 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> >> index 240923d509a..26c47562e79 100755
> >> --- a/scripts/kernel-doc
> >> +++ b/scripts/kernel-doc
> >> @@ -1226,7 +1226,14 @@ sub dump_struct($$) {
> >>  	# replace DECLARE_KFIFO_PTR
> >>  	$members =~ s/DECLARE_KFIFO_PTR\s*\(([^,)]+),\s*([^,)]+)\)/$2 \*$1/gos;
> >>  
> >> -	my $declaration = $members;
> >> +        # QEMU Specific Macros
> >> +
> >> +        # replace QLIST_ENTRY with base type and variable name
> >> +        $members =~ s/QLIST_ENTRY\(([^)]+)\)\s+([^;]+)/$1 \*$2/gos;
> >> +        # replace QLIST_HEAD, optionally capturing an anonymous struct marker, and capture type and variable name
> >> +        $members =~ s/QLIST_HEAD\(\s*,\s*([^)]+)\)\s+([^;]+)/struct { $1 *lh_first; } $2/gos;
> >> +
> >> +        my $declaration = $members;
> >
> > May need a "tabify" here..
> 
> Ugg that file is a mess. Any idea what we should use for perl, tabs or
> spaces? I can update editorconfig.

Indeed.. not perl expert here.

For this one it might be still good to keep the same with the code around
before an attempt to clean it up.

Thanks,

-- 
Peter Xu

Re: [PATCH 1/5] scripts/kernel-doc: teach kdoc about QLIST_ macros

[Thomas Huth]

On 08/03/2024 09.22, Peter Xu wrote:
> On Fri, Mar 08, 2024 at 08:09:15AM +0000, Alex Bennée wrote:
>> Peter Xu <peterx@redhat.com> writes:
>>
>>> On Thu, Mar 07, 2024 at 06:11:01PM +0000, Alex Bennée wrote:
>>>> The kernel-doc script does some pre-processing on structure
>>>> definitions before parsing for names. Teach it about QLIST and replace
>>>> with simplified structures representing the base type.
>>>>
>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>>> ---
>>>>   scripts/kernel-doc | 9 ++++++++-
>>>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
>>>> index 240923d509a..26c47562e79 100755
>>>> --- a/scripts/kernel-doc
>>>> +++ b/scripts/kernel-doc
>>>> @@ -1226,7 +1226,14 @@ sub dump_struct($$) {
>>>>   	# replace DECLARE_KFIFO_PTR
>>>>   	$members =~ s/DECLARE_KFIFO_PTR\s*\(([^,)]+),\s*([^,)]+)\)/$2 \*$1/gos;
>>>>   
>>>> -	my $declaration = $members;
>>>> +        # QEMU Specific Macros
>>>> +
>>>> +        # replace QLIST_ENTRY with base type and variable name
>>>> +        $members =~ s/QLIST_ENTRY\(([^)]+)\)\s+([^;]+)/$1 \*$2/gos;
>>>> +        # replace QLIST_HEAD, optionally capturing an anonymous struct marker, and capture type and variable name
>>>> +        $members =~ s/QLIST_HEAD\(\s*,\s*([^)]+)\)\s+([^;]+)/struct { $1 *lh_first; } $2/gos;
>>>> +
>>>> +        my $declaration = $members;
>>>
>>> May need a "tabify" here..
>>
>> Ugg that file is a mess. Any idea what we should use for perl, tabs or
>> spaces? I can update editorconfig.
> 
> Indeed.. not perl expert here.
> 
> For this one it might be still good to keep the same with the code around
> before an attempt to clean it up.

I think this file originate from the Linux kernel repo, so it might be a 
good idea to keep it in sync with the version from there?

  Thomas

Re: [PATCH 1/5] scripts/kernel-doc: teach kdoc about QLIST_ macros

[Peter Maydell]

On Fri, 8 Mar 2024 at 08:49, Thomas Huth <thuth@redhat.com> wrote:
>
> On 08/03/2024 09.22, Peter Xu wrote:
> > On Fri, Mar 08, 2024 at 08:09:15AM +0000, Alex Bennée wrote:
> >> Peter Xu <peterx@redhat.com> writes:
> >>
> >>> On Thu, Mar 07, 2024 at 06:11:01PM +0000, Alex Bennée wrote:
> >>>> The kernel-doc script does some pre-processing on structure
> >>>> definitions before parsing for names. Teach it about QLIST and replace
> >>>> with simplified structures representing the base type.
> >>>>
> >>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >>>> ---
> >>>>   scripts/kernel-doc | 9 ++++++++-
> >>>>   1 file changed, 8 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> >>>> index 240923d509a..26c47562e79 100755
> >>>> --- a/scripts/kernel-doc
> >>>> +++ b/scripts/kernel-doc
> >>>> @@ -1226,7 +1226,14 @@ sub dump_struct($$) {
> >>>>    # replace DECLARE_KFIFO_PTR
> >>>>    $members =~ s/DECLARE_KFIFO_PTR\s*\(([^,)]+),\s*([^,)]+)\)/$2 \*$1/gos;
> >>>>
> >>>> -  my $declaration = $members;
> >>>> +        # QEMU Specific Macros
> >>>> +
> >>>> +        # replace QLIST_ENTRY with base type and variable name
> >>>> +        $members =~ s/QLIST_ENTRY\(([^)]+)\)\s+([^;]+)/$1 \*$2/gos;
> >>>> +        # replace QLIST_HEAD, optionally capturing an anonymous struct marker, and capture type and variable name
> >>>> +        $members =~ s/QLIST_HEAD\(\s*,\s*([^)]+)\)\s+([^;]+)/struct { $1 *lh_first; } $2/gos;
> >>>> +
> >>>> +        my $declaration = $members;
> >>>
> >>> May need a "tabify" here..
> >>
> >> Ugg that file is a mess. Any idea what we should use for perl, tabs or
> >> spaces? I can update editorconfig.
> >
> > Indeed.. not perl expert here.
> >
> > For this one it might be still good to keep the same with the code around
> > before an attempt to clean it up.
>
> I think this file originate from the Linux kernel repo, so it might be a
> good idea to keep it in sync with the version from there?

Yes; we should avoid divergence where we can. We already
have one script (checkpatch) that has diverged too far
for easy resyncing, it would be good to avoid this one
going in the same direction. We last did a resync in 2020.

thanks
-- PMM

[PATCH 2/5] docs: include ramblock.h in the memory API docs

[Alex Bennée]

The RAMBlock concept is fairly central to RAM-like MemoryRegions so
lets update the structure documentation and include in the docs.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 docs/devel/memory.rst   |  1 +
 include/exec/ramblock.h | 76 +++++++++++++++++++++++++++--------------
 2 files changed, 52 insertions(+), 25 deletions(-)

diff --git a/docs/devel/memory.rst b/docs/devel/memory.rst
index 69c5e3f914a..ed24708fce3 100644
--- a/docs/devel/memory.rst
+++ b/docs/devel/memory.rst
@@ -369,4 +369,5 @@ callbacks are called:
 API Reference
 -------------
 
+.. kernel-doc:: include/exec/ramblock.h
 .. kernel-doc:: include/exec/memory.h
diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h
index 848915ea5bf..eb2416b6f66 100644
--- a/include/exec/ramblock.h
+++ b/include/exec/ramblock.h
@@ -24,68 +24,94 @@
 #include "qemu/rcu.h"
 #include "exec/ramlist.h"
 
+/**
+ * struct RAMBlock - represents a chunk of RAM
+ *
+ * RAMBlocks can be backed by allocated RAM or a file-descriptor. See
+ * @flags for the details. For the purposes of migration various book
+ * keeping and dirty state tracking elements are also tracked in this
+ * structure.
+ */
 struct RAMBlock {
+    /** @rcu: used for lazy free under RCU */
     struct rcu_head rcu;
+    /** @mr: parent MemoryRegion the block belongs to */
     struct MemoryRegion *mr;
+    /** @host: pointer to host address of RAM */
     uint8_t *host;
-    uint8_t *colo_cache; /* For colo, VM's ram cache */
+    /** @colo_cache: For colo, VM's ram cache */
+    uint8_t *colo_cache;
+    /** @offset: offset into host backing store??? or guest address space? */
     ram_addr_t offset;
+    /** @used_length: amount of store used */
     ram_addr_t used_length;
+    /** @max_length: for blocks that can be resized the max possible */
     ram_addr_t max_length;
+    /** @resized: callback notifier when block resized  */
     void (*resized)(const char*, uint64_t length, void *host);
+    /** @flags: see RAM_* flags in memory.h  */
     uint32_t flags;
-    /* Protected by the BQL.  */
+    /** @idstr: Protected by the BQL.  */
     char idstr[256];
-    /* RCU-enabled, writes protected by the ramlist lock */
+    /**
+     * @next: next RAMBlock, RCU-enabled, writes protected by the
+     * ramlist lock
+     */
     QLIST_ENTRY(RAMBlock) next;
+    /** @ramblock_notifiers: list of RAMBlockNotifier notifiers */
     QLIST_HEAD(, RAMBlockNotifier) ramblock_notifiers;
+    /** @fd: fd of backing store if used */
     int fd;
+    /** @fd_offset: offset into the fd based backing store */
     uint64_t fd_offset;
+    /** @page_size: ideal page size of backing store*/
     size_t page_size;
-    /* dirty bitmap used during migration */
+    /** @bmap:  dirty bitmap used during migration */
     unsigned long *bmap;
 
     /*
      * Below fields are only used by mapped-ram migration
      */
-    /* bitmap of pages present in the migration file */
+
+    /** @file_bmap: bitmap of pages present in the migration file  */
     unsigned long *file_bmap;
-    /*
-     * offset in the file pages belonging to this ramblock are saved,
-     * used only during migration to a file.
-     */
+    /** @bitmap_offset: offset in the migration file of the bitmaps */
     off_t bitmap_offset;
+    /** @pages_offset: offset in the migration file of the pages */
     uint64_t pages_offset;
 
-    /* bitmap of already received pages in postcopy */
+    /** @receivedmap: bitmap of already received pages in postcopy */
     unsigned long *receivedmap;
 
-    /*
-     * bitmap to track already cleared dirty bitmap.  When the bit is
-     * set, it means the corresponding memory chunk needs a log-clear.
-     * Set this up to non-NULL to enable the capability to postpone
-     * and split clearing of dirty bitmap on the remote node (e.g.,
-     * KVM).  The bitmap will be set only when doing global sync.
+    /**
+     * @clear_bmap: bitmap to track already cleared dirty bitmap. When
+     * the bit is set, it means the corresponding memory chunk needs a
+     * log-clear. Set this up to non-NULL to enable the capability to
+     * postpone and split clearing of dirty bitmap on the remote node
+     * (e.g., KVM). The bitmap will be set only when doing global
+     * sync.
      *
      * It is only used during src side of ram migration, and it is
      * protected by the global ram_state.bitmap_mutex.
      *
      * NOTE: this bitmap is different comparing to the other bitmaps
      * in that one bit can represent multiple guest pages (which is
-     * decided by the `clear_bmap_shift' variable below).  On
+     * decided by the @clear_bmap_shift variable below).  On
      * destination side, this should always be NULL, and the variable
-     * `clear_bmap_shift' is meaningless.
+     * @clear_bmap_shift is meaningless.
      */
     unsigned long *clear_bmap;
+    /** @clear_bmap_shift: number pages each @clear_bmap bit represents */
     uint8_t clear_bmap_shift;
 
-    /*
-     * RAM block length that corresponds to the used_length on the migration
-     * source (after RAM block sizes were synchronized). Especially, after
-     * starting to run the guest, used_length and postcopy_length can differ.
-     * Used to register/unregister uffd handlers and as the size of the received
-     * bitmap. Receiving any page beyond this length will bail out, as it
-     * could not have been valid on the source.
+    /**
+     * @postcopy_length: RAM block length that corresponds to the
+     * @used_length on the migration source (after RAM block sizes
+     * were synchronized). Especially, after starting to run the
+     * guest, @used_length and @postcopy_length can differ. Used to
+     * register/unregister uffd handlers and as the size of the
+     * received bitmap. Receiving any page beyond this length will
+     * bail out, as it could not have been valid on the source.
      */
     ram_addr_t postcopy_length;
 };
-- 
2.39.2

Re: [PATCH 2/5] docs: include ramblock.h in the memory API docs

[Peter Xu]

On Thu, Mar 07, 2024 at 06:11:02PM +0000, Alex Bennée wrote:
> The RAMBlock concept is fairly central to RAM-like MemoryRegions so
> lets update the structure documentation and include in the docs.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  docs/devel/memory.rst   |  1 +
>  include/exec/ramblock.h | 76 +++++++++++++++++++++++++++--------------
>  2 files changed, 52 insertions(+), 25 deletions(-)
> 
> diff --git a/docs/devel/memory.rst b/docs/devel/memory.rst
> index 69c5e3f914a..ed24708fce3 100644
> --- a/docs/devel/memory.rst
> +++ b/docs/devel/memory.rst
> @@ -369,4 +369,5 @@ callbacks are called:
>  API Reference
>  -------------
>  
> +.. kernel-doc:: include/exec/ramblock.h
>  .. kernel-doc:: include/exec/memory.h
> diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h
> index 848915ea5bf..eb2416b6f66 100644
> --- a/include/exec/ramblock.h
> +++ b/include/exec/ramblock.h
> @@ -24,68 +24,94 @@
>  #include "qemu/rcu.h"
>  #include "exec/ramlist.h"
>  
> +/**
> + * struct RAMBlock - represents a chunk of RAM
> + *
> + * RAMBlocks can be backed by allocated RAM or a file-descriptor. See
> + * @flags for the details. For the purposes of migration various book
> + * keeping and dirty state tracking elements are also tracked in this
> + * structure.
> + */
>  struct RAMBlock {
> +    /** @rcu: used for lazy free under RCU */
>      struct rcu_head rcu;
> +    /** @mr: parent MemoryRegion the block belongs to */
>      struct MemoryRegion *mr;
> +    /** @host: pointer to host address of RAM */
>      uint8_t *host;
> -    uint8_t *colo_cache; /* For colo, VM's ram cache */
> +    /** @colo_cache: For colo, VM's ram cache */
> +    uint8_t *colo_cache;
> +    /** @offset: offset into host backing store??? or guest address space? */

I think it's the first, or to be explicit, "ram_addr_t address space"?

>      ram_addr_t offset;
> +    /** @used_length: amount of store used */
>      ram_addr_t used_length;
> +    /** @max_length: for blocks that can be resized the max possible */
>      ram_addr_t max_length;
> +    /** @resized: callback notifier when block resized  */
>      void (*resized)(const char*, uint64_t length, void *host);
> +    /** @flags: see RAM_* flags in memory.h  */
>      uint32_t flags;
> -    /* Protected by the BQL.  */
> +    /** @idstr: Protected by the BQL.  */

Hmm, I think RCU should be enough to read an idstr?  Maybe as simple as:

@idstr: Name of the ramblock

?

>      char idstr[256];
> -    /* RCU-enabled, writes protected by the ramlist lock */
> +    /**
> +     * @next: next RAMBlock, RCU-enabled, writes protected by the
> +     * ramlist lock
> +     */
>      QLIST_ENTRY(RAMBlock) next;
> +    /** @ramblock_notifiers: list of RAMBlockNotifier notifiers */
>      QLIST_HEAD(, RAMBlockNotifier) ramblock_notifiers;
> +    /** @fd: fd of backing store if used */

Can also add: "For anonymous RAMBlocks, it's always -1".

>      int fd;
> +    /** @fd_offset: offset into the fd based backing store */
>      uint64_t fd_offset;
> +    /** @page_size: ideal page size of backing store*/

"ideal" might be a bit ambiguous.  How about "backend page size"?

For anon, it's always PAGE_SIZE, for file, it's the one reported in
fstatfs().  But this might be too verbose.

>      size_t page_size;
> -    /* dirty bitmap used during migration */
> +    /** @bmap:  dirty bitmap used during migration */
>      unsigned long *bmap;
>  
>      /*
>       * Below fields are only used by mapped-ram migration
>       */
> -    /* bitmap of pages present in the migration file */
> +
> +    /** @file_bmap: bitmap of pages present in the migration file  */

Can append "(only used in mapped-ram migrations)".  This may also apply to
below two fields.

>      unsigned long *file_bmap;
> -    /*
> -     * offset in the file pages belonging to this ramblock are saved,
> -     * used only during migration to a file.
> -     */
> +    /** @bitmap_offset: offset in the migration file of the bitmaps */

s/bitmaps/bitmap/, as there's only one for each rb.

>      off_t bitmap_offset;
> +    /** @pages_offset: offset in the migration file of the pages */
>      uint64_t pages_offset;
>  
> -    /* bitmap of already received pages in postcopy */
> +    /** @receivedmap: bitmap of already received pages in postcopy */
>      unsigned long *receivedmap;
>  
> -    /*
> -     * bitmap to track already cleared dirty bitmap.  When the bit is
> -     * set, it means the corresponding memory chunk needs a log-clear.
> -     * Set this up to non-NULL to enable the capability to postpone
> -     * and split clearing of dirty bitmap on the remote node (e.g.,
> -     * KVM).  The bitmap will be set only when doing global sync.
> +    /**
> +     * @clear_bmap: bitmap to track already cleared dirty bitmap. When
> +     * the bit is set, it means the corresponding memory chunk needs a
> +     * log-clear. Set this up to non-NULL to enable the capability to
> +     * postpone and split clearing of dirty bitmap on the remote node
> +     * (e.g., KVM). The bitmap will be set only when doing global
> +     * sync.
>       *
>       * It is only used during src side of ram migration, and it is
>       * protected by the global ram_state.bitmap_mutex.
>       *
>       * NOTE: this bitmap is different comparing to the other bitmaps
>       * in that one bit can represent multiple guest pages (which is
> -     * decided by the `clear_bmap_shift' variable below).  On
> +     * decided by the @clear_bmap_shift variable below).  On
>       * destination side, this should always be NULL, and the variable
> -     * `clear_bmap_shift' is meaningless.
> +     * @clear_bmap_shift is meaningless.
>       */
>      unsigned long *clear_bmap;
> +    /** @clear_bmap_shift: number pages each @clear_bmap bit represents */
>      uint8_t clear_bmap_shift;
>  
> -    /*
> -     * RAM block length that corresponds to the used_length on the migration
> -     * source (after RAM block sizes were synchronized). Especially, after
> -     * starting to run the guest, used_length and postcopy_length can differ.
> -     * Used to register/unregister uffd handlers and as the size of the received
> -     * bitmap. Receiving any page beyond this length will bail out, as it
> -     * could not have been valid on the source.
> +    /**
> +     * @postcopy_length: RAM block length that corresponds to the
> +     * @used_length on the migration source (after RAM block sizes
> +     * were synchronized). Especially, after starting to run the
> +     * guest, @used_length and @postcopy_length can differ. Used to
> +     * register/unregister uffd handlers and as the size of the
> +     * received bitmap. Receiving any page beyond this length will
> +     * bail out, as it could not have been valid on the source.
>       */
>      ram_addr_t postcopy_length;
>  };
> -- 
> 2.39.2
> 

-- 
Peter Xu

[PATCH 3/5] include/exec: remove warning_printed from MemoryRegion

[Alex Bennée]

Since d197063fcf9 (memory: move unassigned_mem_ops to memory.c) this
field is unused.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/exec/memory.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 8626a355b31..17b741bc4f5 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -814,7 +814,6 @@ struct MemoryRegion {
     bool terminates;
     bool ram_device;
     bool enabled;
-    bool warning_printed; /* For reservations */
     uint8_t vga_logging_count;
     MemoryRegion *alias;
     hwaddr alias_offset;
-- 
2.39.2

Re: [PATCH 3/5] include/exec: remove warning_printed from MemoryRegion

[Richard Henderson]

On 3/7/24 08:11, Alex Bennée wrote:
> Since d197063fcf9 (memory: move unassigned_mem_ops to memory.c) this
> field is unused.
> 
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> ---
>   include/exec/memory.h | 1 -
>   1 file changed, 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 3/5] include/exec: remove warning_printed from MemoryRegion

[Philippe Mathieu-Daudé]

On 7/3/24 19:11, Alex Bennée wrote:
> Since d197063fcf9 (memory: move unassigned_mem_ops to memory.c) this
> field is unused.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   include/exec/memory.h | 1 -
>   1 file changed, 1 deletion(-)

10+ years ;)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 3/5] include/exec: remove warning_printed from MemoryRegion

[Peter Xu]

On Thu, Mar 07, 2024 at 06:11:03PM +0000, Alex Bennée wrote:
> Since d197063fcf9 (memory: move unassigned_mem_ops to memory.c) this
> field is unused.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

[PATCH 4/5] include/exec: annotate all the MemoryRegion fields

[Alex Bennée]

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/exec/memory.h | 47 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 43 insertions(+), 4 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 17b741bc4f5..312ed564dbe 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -778,9 +778,48 @@ bool memory_get_xlat_addr(IOMMUTLBEntry *iotlb, void **vaddr,
 typedef struct CoalescedMemoryRange CoalescedMemoryRange;
 typedef struct MemoryRegionIoeventfd MemoryRegionIoeventfd;
 
-/** MemoryRegion:
- *
- * A struct representing a memory region.
+/**
+ * struct MemoryRegion - represents a memory region
+ * @parent_obj: parent QOM object for the region
+ * @romd_mode: if true ROM devices accessed directly rather than with @ops
+ * @ram: true if a RAM-type device with a @ram_block
+ * @subpage: true if region covers a subpage
+ * @readonly: true for RAM-type devices that are readonly
+ * @nonvolatile: true for nonvolatile RAM-type devices (e.g. NVDIMM)
+ * @rom_device: true for a ROM device, see also @romd_mode
+ * @flush_coalesced_mmio: true to flush any queued coalesced MMIO
+ * operations before access
+ * @unmergeable: this section should not get merged with adjacent
+ * sections
+ * @dirty_log_mask: dirty events region cares about (see DIRTY_ flags)
+ * @is_iommu: true if part of an IOMMU device
+ * @ram_block: backing @RamBlock if @ram is true
+ * @owner: base QOM object that owns this region
+ * @dev: base Device that owns this region
+ * @ops: access operations for MMIO or @romd_mode devices
+ * @opaque: @dev specific data, passed with @ops
+ * @container: parent `MemoryRegion`
+ * @mapped_via_alias: number of times mapped via @alias, container
+ * might be NULL
+ * @size: size of @MemoryRegion
+ * @addr: physical hwaddr of @MemoryRegion
+ * @destructor: cleanup function when @MemoryRegion finalized
+ * @align: alignment requirements for any physical backing store
+ * @terminates: true if this @MemoryRegion is a leaf node
+ * @ram_device: true if @ram device should use @ops to access
+ * @enabled: true once initialised, false once finalized
+ * @vga_logging_count: count of memory logging clients
+ * @alias: link to aliased @MemoryRegion
+ * @alias_offset: offset into aliased region
+ * @priority: priority when resolving overlapping regions
+ * @subregions: list of subregions in this region
+ * @subregions_link: next subregion in the chain
+ * @coalesced: list of coalesced memory ranges
+ * @name: name of memory region
+ * @ioeventfd_nb: count of @ioeventfds for region
+ * @ioeventfds: ioevent notifiers for region
+ * @rdm: if exists see #RamDiscardManager
+ * @disable_reentrancy_guard: if true don't error if device accesses itself
  */
 struct MemoryRegion {
     Object parent_obj;
@@ -806,7 +845,7 @@ struct MemoryRegion {
     const MemoryRegionOps *ops;
     void *opaque;
     MemoryRegion *container;
-    int mapped_via_alias; /* Mapped via an alias, container might be NULL */
+    int mapped_via_alias;
     Int128 size;
     hwaddr addr;
     void (*destructor)(MemoryRegion *mr);
-- 
2.39.2

Re: [PATCH 4/5] include/exec: annotate all the MemoryRegion fields

[Richard Henderson]

On 3/7/24 08:11, Alex Bennée wrote:
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   include/exec/memory.h | 47 +++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 43 insertions(+), 4 deletions(-)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 17b741bc4f5..312ed564dbe 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -778,9 +778,48 @@ bool memory_get_xlat_addr(IOMMUTLBEntry *iotlb, void **vaddr,
>   typedef struct CoalescedMemoryRange CoalescedMemoryRange;
>   typedef struct MemoryRegionIoeventfd MemoryRegionIoeventfd;
>   
> -/** MemoryRegion:
> - *
> - * A struct representing a memory region.
> +/**
> + * struct MemoryRegion - represents a memory region
> + * @parent_obj: parent QOM object for the region
> + * @romd_mode: if true ROM devices accessed directly rather than with @ops
> + * @ram: true if a RAM-type device with a @ram_block
> + * @subpage: true if region covers a subpage
> + * @readonly: true for RAM-type devices that are readonly
> + * @nonvolatile: true for nonvolatile RAM-type devices (e.g. NVDIMM)
> + * @rom_device: true for a ROM device, see also @romd_mode
> + * @flush_coalesced_mmio: true to flush any queued coalesced MMIO
> + * operations before access
> + * @unmergeable: this section should not get merged with adjacent
> + * sections
> + * @dirty_log_mask: dirty events region cares about (see DIRTY_ flags)
> + * @is_iommu: true if part of an IOMMU device
> + * @ram_block: backing @RamBlock if @ram is true
> + * @owner: base QOM object that owns this region
> + * @dev: base Device that owns this region
> + * @ops: access operations for MMIO or @romd_mode devices
> + * @opaque: @dev specific data, passed with @ops
> + * @container: parent `MemoryRegion`
> + * @mapped_via_alias: number of times mapped via @alias, container
> + * might be NULL
> + * @size: size of @MemoryRegion
> + * @addr: physical hwaddr of @MemoryRegion
> + * @destructor: cleanup function when @MemoryRegion finalized
> + * @align: alignment requirements for any physical backing store
> + * @terminates: true if this @MemoryRegion is a leaf node
> + * @ram_device: true if @ram device should use @ops to access
> + * @enabled: true once initialised, false once finalized
> + * @vga_logging_count: count of memory logging clients
> + * @alias: link to aliased @MemoryRegion
> + * @alias_offset: offset into aliased region
> + * @priority: priority when resolving overlapping regions
> + * @subregions: list of subregions in this region
> + * @subregions_link: next subregion in the chain
> + * @coalesced: list of coalesced memory ranges
> + * @name: name of memory region
> + * @ioeventfd_nb: count of @ioeventfds for region
> + * @ioeventfds: ioevent notifiers for region
> + * @rdm: if exists see #RamDiscardManager
> + * @disable_reentrancy_guard: if true don't error if device accesses itself
>    */

Why as one big block rather than line by line?
The block is less likely to be properly kept up-to-date and is much harder to read.


r~

Re: [PATCH 4/5] include/exec: annotate all the MemoryRegion fields

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 3/7/24 08:11, Alex Bennée wrote:
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>   include/exec/memory.h | 47 +++++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 43 insertions(+), 4 deletions(-)
>> diff --git a/include/exec/memory.h b/include/exec/memory.h
>> index 17b741bc4f5..312ed564dbe 100644
>> --- a/include/exec/memory.h
>> +++ b/include/exec/memory.h
>> @@ -778,9 +778,48 @@ bool memory_get_xlat_addr(IOMMUTLBEntry *iotlb, void **vaddr,
>>   typedef struct CoalescedMemoryRange CoalescedMemoryRange;
>>   typedef struct MemoryRegionIoeventfd MemoryRegionIoeventfd;
>>   -/** MemoryRegion:
>> - *
>> - * A struct representing a memory region.
>> +/**
>> + * struct MemoryRegion - represents a memory region
>> + * @parent_obj: parent QOM object for the region
>> + * @romd_mode: if true ROM devices accessed directly rather than with @ops
>> + * @ram: true if a RAM-type device with a @ram_block
>> + * @subpage: true if region covers a subpage
>> + * @readonly: true for RAM-type devices that are readonly
>> + * @nonvolatile: true for nonvolatile RAM-type devices (e.g. NVDIMM)
>> + * @rom_device: true for a ROM device, see also @romd_mode
>> + * @flush_coalesced_mmio: true to flush any queued coalesced MMIO
>> + * operations before access
>> + * @unmergeable: this section should not get merged with adjacent
>> + * sections
>> + * @dirty_log_mask: dirty events region cares about (see DIRTY_ flags)
>> + * @is_iommu: true if part of an IOMMU device
>> + * @ram_block: backing @RamBlock if @ram is true
>> + * @owner: base QOM object that owns this region
>> + * @dev: base Device that owns this region
>> + * @ops: access operations for MMIO or @romd_mode devices
>> + * @opaque: @dev specific data, passed with @ops
>> + * @container: parent `MemoryRegion`
>> + * @mapped_via_alias: number of times mapped via @alias, container
>> + * might be NULL
>> + * @size: size of @MemoryRegion
>> + * @addr: physical hwaddr of @MemoryRegion
>> + * @destructor: cleanup function when @MemoryRegion finalized
>> + * @align: alignment requirements for any physical backing store
>> + * @terminates: true if this @MemoryRegion is a leaf node
>> + * @ram_device: true if @ram device should use @ops to access
>> + * @enabled: true once initialised, false once finalized
>> + * @vga_logging_count: count of memory logging clients
>> + * @alias: link to aliased @MemoryRegion
>> + * @alias_offset: offset into aliased region
>> + * @priority: priority when resolving overlapping regions
>> + * @subregions: list of subregions in this region
>> + * @subregions_link: next subregion in the chain
>> + * @coalesced: list of coalesced memory ranges
>> + * @name: name of memory region
>> + * @ioeventfd_nb: count of @ioeventfds for region
>> + * @ioeventfds: ioevent notifiers for region
>> + * @rdm: if exists see #RamDiscardManager
>> + * @disable_reentrancy_guard: if true don't error if device accesses itself
>>    */
>
> Why as one big block rather than line by line?
> The block is less likely to be properly kept up-to-date and is much
> harder to read.

The inline syntax is a little more prone to parse failures and
annoyingly can't group multiple fields in one inline comment block. But
I can certainly move it inline if that preferable.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 4/5] include/exec: annotate all the MemoryRegion fields

[Peter Maydell]

On Thu, 7 Mar 2024 at 18:11, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  include/exec/memory.h | 47 +++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 43 insertions(+), 4 deletions(-)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 17b741bc4f5..312ed564dbe 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -778,9 +778,48 @@ bool memory_get_xlat_addr(IOMMUTLBEntry *iotlb, void **vaddr,
>  typedef struct CoalescedMemoryRange CoalescedMemoryRange;
>  typedef struct MemoryRegionIoeventfd MemoryRegionIoeventfd;
>
> -/** MemoryRegion:
> - *
> - * A struct representing a memory region.
> +/**
> + * struct MemoryRegion - represents a memory region
> + * @parent_obj: parent QOM object for the region
> + * @romd_mode: if true ROM devices accessed directly rather than with @ops

If true, read accesses go directly to host memory;
write accesses always go via the MMIO accessor.

You might alternatively say "true if a ROM device is in ROMD mode; see
memory_region_device_set_romd()", since the doc comment for
that function has an explanation of what ROMD mode is.

> + * @ram: true if a RAM-type device with a @ram_block

We call this a "RAM memory region" in memory.rst and in
other doc comments in this file.

> + * @subpage: true if region covers a subpage

This is the kind of doc comment that doesn't tell us anything
we didn't already know from the field name. More useful would
be to explain what the subpage handling is actually doing.

> + * @readonly: true for RAM-type devices that are readonly
> + * @nonvolatile: true for nonvolatile RAM-type devices (e.g. NVDIMM)
> + * @rom_device: true for a ROM device, see also @romd_mode
> + * @flush_coalesced_mmio: true to flush any queued coalesced MMIO
> + * operations before access
> + * @unmergeable: this section should not get merged with adjacent
> + * sections
> + * @dirty_log_mask: dirty events region cares about (see DIRTY_ flags)
> + * @is_iommu: true if part of an IOMMU device
> + * @ram_block: backing @RamBlock if @ram is true
> + * @owner: base QOM object that owns this region
> + * @dev: base Device that owns this region
> + * @ops: access operations for MMIO or @romd_mode devices
> + * @opaque: @dev specific data, passed with @ops
> + * @container: parent `MemoryRegion`
> + * @mapped_via_alias: number of times mapped via @alias, container
> + * might be NULL
> + * @size: size of @MemoryRegion
> + * @addr: physical hwaddr of @MemoryRegion

I think this is not a physical address; it's the address
(i.e. offset) of this MR within its parent MR @container.

> + * @destructor: cleanup function when @MemoryRegion finalized
> + * @align: alignment requirements for any physical backing store
> + * @terminates: true if this @MemoryRegion is a leaf node
> + * @ram_device: true if @ram device should use @ops to access

This doesn't sound right. True if this MR is a RAM device memory
region (i.e. it is backed using a pointer corresponding to a
host physical device); see @memory_region_init_ram_device_ptr().

We should also add "RAM device" to the "Types of regions"
list in docs/devel/memory.rst, which is currently missing it.

> + * @enabled: true once initialised, false once finalized

Not very useful, as it misses entirely the use of the flag.
Enabled MRs appear in the guest's memory space; non-enabled
MRs are ignored. By default MRs are enabled, but they can
be enabled and disabled at runtime using memory_region_set_enabled().

> + * @vga_logging_count: count of memory logging clients
> + * @alias: link to aliased @MemoryRegion

If this is an alias MemoryRegion, pointer to the MR we are aliasing.

> + * @alias_offset: offset into aliased region
> + * @priority: priority when resolving overlapping regions
> + * @subregions: list of subregions in this region
> + * @subregions_link: next subregion in the chain
> + * @coalesced: list of coalesced memory ranges
> + * @name: name of memory region
> + * @ioeventfd_nb: count of @ioeventfds for region
> + * @ioeventfds: ioevent notifiers for region
> + * @rdm: if exists see #RamDiscardManager
> + * @disable_reentrancy_guard: if true don't error if device accesses itself
>   */
>  struct MemoryRegion {
>      Object parent_obj;
> @@ -806,7 +845,7 @@ struct MemoryRegion {
>      const MemoryRegionOps *ops;
>      void *opaque;
>      MemoryRegion *container;
> -    int mapped_via_alias; /* Mapped via an alias, container might be NULL */
> +    int mapped_via_alias;
>      Int128 size;
>      hwaddr addr;
>      void (*destructor)(MemoryRegion *mr);
> --
> 2.39.2

thanks
-- PMM

[PATCH 5/5] docs/devel: mark out defined functions and structures

[Alex Bennée]

This allows sphinx to hyperlink the references to their kdoc
definitions for easy navigation.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 docs/devel/memory.rst | 48 +++++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/docs/devel/memory.rst b/docs/devel/memory.rst
index ed24708fce3..193f31198b0 100644
--- a/docs/devel/memory.rst
+++ b/docs/devel/memory.rst
@@ -16,11 +16,11 @@ The memory model provides support for
 - setting up coalesced memory for kvm
 - setting up ioeventfd regions for kvm
 
-Memory is modelled as an acyclic graph of MemoryRegion objects.  Sinks
+Memory is modelled as an acyclic graph of `MemoryRegion` objects.  Sinks
 (leaves) are RAM and MMIO regions, while other nodes represent
 buses, memory controllers, and memory regions that have been rerouted.
 
-In addition to MemoryRegion objects, the memory API provides AddressSpace
+In addition to `MemoryRegion` objects, the memory API provides `AddressSpace`
 objects for every root and possibly for intermediate MemoryRegions too.
 These represent memory as seen from the CPU or a device's viewpoint.
 
@@ -28,17 +28,17 @@ Types of regions
 ----------------
 
 There are multiple types of memory regions (all represented by a single C type
-MemoryRegion):
+`MemoryRegion`):
 
 - RAM: a RAM region is simply a range of host memory that can be made available
   to the guest.
-  You typically initialize these with memory_region_init_ram().  Some special
-  purposes require the variants memory_region_init_resizeable_ram(),
-  memory_region_init_ram_from_file(), or memory_region_init_ram_ptr().
+  You typically initialize these with `memory_region_init_ram()`.  Some special
+  purposes require the variants `memory_region_init_resizeable_ram()`,
+  `memory_region_init_ram_from_file()`, or `memory_region_init_ram_ptr()`.
 
 - MMIO: a range of guest memory that is implemented by host callbacks;
   each read or write causes a callback to be called on the host.
-  You initialize these with memory_region_init_io(), passing it a
+  You initialize these with `memory_region_init_io()`, passing it a
   MemoryRegionOps structure describing the callbacks.
 
 - ROM: a ROM memory region works like RAM for reads (directly accessing
@@ -53,7 +53,7 @@ MemoryRegion):
 - IOMMU region: an IOMMU region translates addresses of accesses made to it
   and forwards them to some other target memory region.  As the name suggests,
   these are only needed for modelling an IOMMU, not for simple devices.
-  You initialize these with memory_region_init_iommu().
+  You initialize these with `memory_region_init_iommu()`.
 
 - container: a container simply includes other memory regions, each at
   a different offset.  Containers are useful for grouping several regions
@@ -65,7 +65,7 @@ MemoryRegion):
   can overlay a subregion of RAM with MMIO or ROM, or a PCI controller
   that does not prevent card from claiming overlapping BARs.
 
-  You initialize a pure container with memory_region_init().
+  You initialize a pure container with `memory_region_init()`.
 
 - alias: a subsection of another region. Aliases allow a region to be
   split apart into discontiguous regions. Examples of uses are memory
@@ -83,7 +83,7 @@ MemoryRegion):
   It claims I/O space that is not supposed to be handled by QEMU itself.
   The typical use is to track parts of the address space which will be
   handled by the host kernel when KVM is enabled.  You initialize these
-  by passing a NULL callback parameter to memory_region_init_io().
+  by passing a NULL callback parameter to `memory_region_init_io()`.
 
 It is valid to add subregions to a region which is not a pure container
 (that is, to an MMIO, RAM or ROM region). This means that the region
@@ -104,28 +104,28 @@ copied to the destination on migration. These APIs which allocate
 the host memory for you will also register the memory so it is
 migrated:
 
-- memory_region_init_ram()
-- memory_region_init_rom()
-- memory_region_init_rom_device()
+- `memory_region_init_ram()`
+- `memory_region_init_rom()`
+- `memory_region_init_rom_device()`
 
 For most devices and boards this is the correct thing. If you
 have a special case where you need to manage the migration of
 the backing memory yourself, you can call the functions:
 
-- memory_region_init_ram_nomigrate()
-- memory_region_init_rom_nomigrate()
-- memory_region_init_rom_device_nomigrate()
+- `memory_region_init_ram_nomigrate()`
+- `memory_region_init_rom_nomigrate()`
+- `memory_region_init_rom_device_nomigrate()`
 
 which only initialize the MemoryRegion and leave handling
 migration to the caller.
 
 The functions:
 
-- memory_region_init_resizeable_ram()
-- memory_region_init_ram_from_file()
-- memory_region_init_ram_from_fd()
-- memory_region_init_ram_ptr()
-- memory_region_init_ram_device_ptr()
+- `memory_region_init_resizeable_ram()`
+- `memory_region_init_ram_from_file()`
+- `memory_region_init_ram_from_fd()`
+- `memory_region_init_ram_ptr()`
+- `memory_region_init_ram_device_ptr()`
 
 are for special cases only, and so they do not automatically
 register the backing memory for migration; the caller must
@@ -150,8 +150,8 @@ device.  For example, the owner object will not die between an
 address_space_map operation and the corresponding address_space_unmap.
 
 After creation, a region can be added to an address space or a
-container with memory_region_add_subregion(), and removed using
-memory_region_del_subregion().
+container with `memory_region_add_subregion()`, and removed using
+`memory_region_del_subregion()`.
 
 Various region attributes (read-only, dirty logging, coalesced mmio,
 ioeventfd) can be changed during the region lifecycle.  They take effect
@@ -213,7 +213,7 @@ allows the region to overlap any other region in the same container, and
 specifies a priority that allows the core to decide which of two regions at
 the same address are visible (highest wins).
 Priority values are signed, and the default value is zero. This means that
-you can use memory_region_add_subregion_overlap() both to specify a region
+you can use `memory_region_add_subregion_overlap()` both to specify a region
 that must sit 'above' any others (with a positive priority) and also a
 background region that sits 'below' others (with a negative priority).
 
-- 
2.39.2

Re: [PATCH 5/5] docs/devel: mark out defined functions and structures

[Peter Maydell]

On Thu, 7 Mar 2024 at 18:11, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> This allows sphinx to hyperlink the references to their kdoc
> definitions for easy navigation.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  docs/devel/memory.rst | 48 +++++++++++++++++++++----------------------
>  1 file changed, 24 insertions(+), 24 deletions(-)



Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM