memory_region: Fix name comments

PATCH Superseded Dr. David Alan Gilbert (git) 4 participants 6 messages Mar 8, 2017 – Mar 9, 2017

lore

Share & tools

lore

Dr. David Alan Gilbert (git)

author

2017-03-08 10:54 UTC #0 lore raw

PATCH 1/1 memory_region: Fix name comments

From: "Dr. David Alan Gilbert" <[email protected]>

The 'name' parameter to memory_region_init_* had been marked as debug only, however vmstate_region_ram uses it as a parameter to qemu_ram_set_idstr to set RAMBlock names and these form part of the migration stream.

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
 include/exec/memory.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

include/exec/memory.h +5 -5

index 6911023..de8f69e 100644
@@ -307,7 +307,7 @@ struct MemoryRegionSection {
  *
  * @mr: the #MemoryRegion to be initialized
  * @owner: the object that tracks the region's reference count
- * @name: used for debugging; not visible to the user or ABI
+ * @name: Region name, becomes part of RAMBlock name used in migration stream
  * @size: size of the region; any subregions beyond this size will be clipped
  */
 void memory_region_init(MemoryRegion *mr,

@@ -355,7 +355,7 @@ void memory_region_unref(MemoryRegion *mr);
  * @ops: a structure containing read and write callbacks to be used when
  *       I/O is performed on the region.
  * @opaque: passed to the read and write callbacks of the @ops structure.
- * @name: used for debugging; not visible to the user or ABI
+ * @name: Region name, becomes part of RAMBlock name used in migration stream
  * @size: size of the region.
  */
 void memory_region_init_io(MemoryRegion *mr,

@@ -474,7 +474,7 @@ void memory_region_init_ram_device_ptr(MemoryRegion *mr,
  *
  * @mr: the #MemoryRegion to be initialized.
  * @owner: the object that tracks the region's reference count
- * @name: used for debugging; not visible to the user or ABI
+ * @name: Region name, becomes part of RAMBlock name used in migration stream
  * @orig: the region to be referenced; @mr will be equivalent to
  *        @orig between @offset and @offset + @size - 1.
  * @offset: start of the section in @orig to be referenced.

@@ -537,7 +537,7 @@ void memory_region_init_rom_device(MemoryRegion *mr,
  *
  * @mr: the #MemoryRegion to be initialized
  * @owner: the object that tracks the region's reference count
- * @name: used for debugging; not visible to the user or ABI
+ * @name: Region name, becomes part of RAMBlock name used in migration stream
  * @size: size of the region.
  */
 static inline void memory_region_init_reservation(MemoryRegion *mr,

@@ -558,7 +558,7 @@ static inline void memory_region_init_reservation(MemoryRegion *mr,
  * @mr: the #MemoryRegion to be initialized
  * @owner: the object that tracks the region's reference count
  * @ops: a function that translates addresses into the @target region
- * @name: used for debugging; not visible to the user or ABI
+ * @name: Region name, becomes part of RAMBlock name used in migration stream
  * @size: size of the region.
  */
 void memory_region_init_iommu(MemoryRegion *mr,

Signature · 1 line

2.9.3

Philippe Mathieu-Daudé

Reviewed-by on v1 1/1

2017-03-08 11:15 UTC #1 lore raw

Quoted 8 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>
The 'name' parameter to memory_region_init_* had been marked as debug only, however vmstate_region_ram uses it as a parameter to qemu_ram_set_idstr to set RAMBlock names and these form part of the migration stream.
Signed-off-by Dr. David Alan Gilbert <[email protected]>

Reviewed-by Philippe Mathieu-Daudé <[email protected]>

Quoted 54 lines

---
 include/exec/memory.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

include/exec/memory.h +5 -5

index 6911023..de8f69e 100644
@@ -307,7 +307,7 @@ struct MemoryRegionSection {
  *
  * @mr: the #MemoryRegion to be initialized
  * @owner: the object that tracks the region's reference count
- * @name: used for debugging; not visible to the user or ABI
+ * @name: Region name, becomes part of RAMBlock name used in migration stream
  * @size: size of the region; any subregions beyond this size will be clipped
  */
 void memory_region_init(MemoryRegion *mr,

@@ -355,7 +355,7 @@ void memory_region_unref(MemoryRegion *mr);
  * @ops: a structure containing read and write callbacks to be used when
  *       I/O is performed on the region.
  * @opaque: passed to the read and write callbacks of the @ops structure.
- * @name: used for debugging; not visible to the user or ABI
+ * @name: Region name, becomes part of RAMBlock name used in migration stream
  * @size: size of the region.
  */
 void memory_region_init_io(MemoryRegion *mr,

@@ -474,7 +474,7 @@ void memory_region_init_ram_device_ptr(MemoryRegion *mr,
  *
  * @mr: the #MemoryRegion to be initialized.
  * @owner: the object that tracks the region's reference count
- * @name: used for debugging; not visible to the user or ABI
+ * @name: Region name, becomes part of RAMBlock name used in migration stream
  * @orig: the region to be referenced; @mr will be equivalent to
  *        @orig between @offset and @offset + @size - 1.
  * @offset: start of the section in @orig to be referenced.

@@ -537,7 +537,7 @@ void memory_region_init_rom_device(MemoryRegion *mr,
  *
  * @mr: the #MemoryRegion to be initialized
  * @owner: the object that tracks the region's reference count
- * @name: used for debugging; not visible to the user or ABI
+ * @name: Region name, becomes part of RAMBlock name used in migration stream
  * @size: size of the region.
  */
 static inline void memory_region_init_reservation(MemoryRegion *mr,

@@ -558,7 +558,7 @@ static inline void memory_region_init_reservation(MemoryRegion *mr,
  * @mr: the #MemoryRegion to be initialized
  * @owner: the object that tracks the region's reference count
  * @ops: a function that translates addresses into the @target region
- * @name: used for debugging; not visible to the user or ABI
+ * @name: Region name, becomes part of RAMBlock name used in migration stream
  * @size: size of the region.
  */
 void memory_region_init_iommu(MemoryRegion *mr,

Peter Maydell

2017-03-08 11:48 UTC #2 lore raw

In reply to Dr. David Alan Gilbert (git)

Quoted 17 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>
The 'name' parameter to memory_region_init_* had been marked as debug only, however vmstate_region_ram uses it as a parameter to qemu_ram_set_idstr to set RAMBlock names and these form part of the migration stream.
Signed-off-by Dr. David Alan Gilbert <[email protected]>
---
 include/exec/memory.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
include/exec/memory.h +0 -0
index 6911023..de8f69e 100644

include/exec/memory.h struct MemoryRegionSection { · line 310

Quoted 5 lines

include/exec/memory.h +1 -1

@@ -307,7 +307,7 @@ struct MemoryRegionSection {
  *
  * @mr: the #MemoryRegion to be initialized
  * @owner: the object that tracks the region's reference count
- * @name: used for debugging; not visible to the user or ABI
+ * @name: Region name, becomes part of RAMBlock name used in migration stream

...for RAM-backend MRs only, presumably? Also, what are the uniqueness constraints?

include/exec/memory.h void memory_region_unref(MemoryRegion *mr); · line 358

Quoted 9 lines

include/exec/memory.h +1 -1

  * @size: size of the region; any subregions beyond this size will be clipped
  */
 void memory_region_init(MemoryRegion *mr,

@@ -355,7 +355,7 @@ void memory_region_unref(MemoryRegion *mr);
  * @ops: a structure containing read and write callbacks to be used when
  *       I/O is performed on the region.
  * @opaque: passed to the read and write callbacks of the @ops structure.
- * @name: used for debugging; not visible to the user or ABI
+ * @name: Region name, becomes part of RAMBlock name used in migration stream

ditto.

include/exec/memory.h void memory_region_init_ram_device_ptr(MemoryRegion *mr, · line 480

Quoted 12 lines

include/exec/memory.h +1 -1

  * @size: size of the region.
  */
 void memory_region_init_io(MemoryRegion *mr,

@@ -474,7 +474,7 @@ void memory_region_init_ram_device_ptr(MemoryRegion *mr,
  *
  * @mr: the #MemoryRegion to be initialized.
  * @owner: the object that tracks the region's reference count
- * @name: used for debugging; not visible to the user or ABI
+ * @name: Region name, becomes part of RAMBlock name used in migration stream
  * @orig: the region to be referenced; @mr will be equivalent to
  *        @orig between @offset and @offset + @size - 1.
  * @offset: start of the section in @orig to be referenced.

The diff here is rather lacking in context, but this comment change is for memory_region_init_alias(). Aliases presumably don't get migrated (the thing they alias into will be migrated if it's RAM), so the comment seems out of place here.

include/exec/memory.h void memory_region_init_rom_device(MemoryRegion *mr, · line 543

Quoted 9 lines

include/exec/memory.h +1 -1

@@ -537,7 +537,7 @@ void memory_region_init_rom_device(MemoryRegion *mr,
  *
  * @mr: the #MemoryRegion to be initialized
  * @owner: the object that tracks the region's reference count
- * @name: used for debugging; not visible to the user or ABI
+ * @name: Region name, becomes part of RAMBlock name used in migration stream
  * @size: size of the region.
  */
 static inline void memory_region_init_reservation(MemoryRegion *mr,

Similarly, a reservation MR isn't RAM-backed.

include/exec/memory.h static inline void memory_region_init_reservation(MemoryRegion *mr, · line 564

Quoted 9 lines

include/exec/memory.h +1 -1

@@ -558,7 +558,7 @@ static inline void memory_region_init_reservation(MemoryRegion *mr,
  * @mr: the #MemoryRegion to be initialized
  * @owner: the object that tracks the region's reference count
  * @ops: a function that translates addresses into the @target region
- * @name: used for debugging; not visible to the user or ABI
+ * @name: Region name, becomes part of RAMBlock name used in migration stream
  * @size: size of the region.
  */
 void memory_region_init_iommu(MemoryRegion *mr,

...and nor is an IOMMU MR.

Quoted 3 lines

--
2.9.3

thanks
-- PMM

Signature · 2 lines

12345678901234567890123456789012345678901234567890123456789012345678901234567890
         1         2         3         4         5         6         7         8

Dr. David Alan Gilbert

author

2017-03-09 11:26 UTC #3 lore raw

Quoted 14 lines · * Peter Maydell ([email protected]) wrote:

Quoted 12 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
The 'name' parameter to memory_region_init_* had been marked as debug only, however vmstate_region_ram uses it as a parameter to qemu_ram_set_idstr to set RAMBlock names and these form part of the migration stream.
Signed-off-by Dr. David Alan Gilbert <[email protected]>
---
 include/exec/memory.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Quoted 13 lines

include/exec/memory.h struct MemoryRegionSection { · line 310
Quoted 10 lines
include/exec/memory.h +1 -1
index 6911023..de8f69e 100644
@@ -307,7 +307,7 @@ struct MemoryRegionSection {
  *
  * @mr: the #MemoryRegion to be initialized
  * @owner: the object that tracks the region's reference count
- * @name: used for debugging; not visible to the user or ABI
+ * @name: Region name, becomes part of RAMBlock name used in migration stream
...for RAM-backend MRs only, presumably? Also, what are the uniqueness constraints?

Hmm; yes RAM backed or things that behave like RAM backed (e.g. file-backed). Uniqueness isn't required in all cases; qemu_ram_set_idstr prefixes this name by a device name if it's given a device - which it isn't normally for stuff on boards. However the code will at least fail if there's a clash. How about:

* @name: Region name, becomes part of RAMBlock name used in migration stream
* must be unique within any device

Quoted 42 lines

include/exec/memory.h void memory_region_unref(MemoryRegion *mr); · line 358

Quoted 9 lines

include/exec/memory.h +1 -1

  * @size: size of the region; any subregions beyond this size will be clipped
  */
 void memory_region_init(MemoryRegion *mr,

@@ -355,7 +355,7 @@ void memory_region_unref(MemoryRegion *mr);
  * @ops: a structure containing read and write callbacks to be used when
  *       I/O is performed on the region.
  * @opaque: passed to the read and write callbacks of the @ops structure.
- * @name: used for debugging; not visible to the user or ABI
+ * @name: Region name, becomes part of RAMBlock name used in migration stream

ditto.

include/exec/memory.h void memory_region_init_ram_device_ptr(MemoryRegion *mr, · line 480

Quoted 12 lines

include/exec/memory.h +1 -1

  * @size: size of the region.
  */
 void memory_region_init_io(MemoryRegion *mr,

@@ -474,7 +474,7 @@ void memory_region_init_ram_device_ptr(MemoryRegion *mr,
  *
  * @mr: the #MemoryRegion to be initialized.
  * @owner: the object that tracks the region's reference count
- * @name: used for debugging; not visible to the user or ABI
+ * @name: Region name, becomes part of RAMBlock name used in migration stream
  * @orig: the region to be referenced; @mr will be equivalent to
  *        @orig between @offset and @offset + @size - 1.
  * @offset: start of the section in @orig to be referenced.

include/exec/memory.h void memory_region_init_rom_device(MemoryRegion *mr, · line 543

Quoted 9 lines

include/exec/memory.h +1 -1

@@ -537,7 +537,7 @@ void memory_region_init_rom_device(MemoryRegion *mr,
  *
  * @mr: the #MemoryRegion to be initialized
  * @owner: the object that tracks the region's reference count
- * @name: used for debugging; not visible to the user or ABI
+ * @name: Region name, becomes part of RAMBlock name used in migration stream
  * @size: size of the region.
  */
 static inline void memory_region_init_reservation(MemoryRegion *mr,

Similarly, a reservation MR isn't RAM-backed.

Quoted 11 lines

include/exec/memory.h static inline void memory_region_init_reservation(MemoryRegion *mr, · line 564

Quoted 9 lines

include/exec/memory.h +1 -1

@@ -558,7 +558,7 @@ static inline void memory_region_init_reservation(MemoryRegion *mr,
  * @mr: the #MemoryRegion to be initialized
  * @owner: the object that tracks the region's reference count
  * @ops: a function that translates addresses into the @target region
- * @name: used for debugging; not visible to the user or ABI
+ * @name: Region name, becomes part of RAMBlock name used in migration stream
  * @size: size of the region.
  */
 void memory_region_init_iommu(MemoryRegion *mr,

...and nor is an IOMMU MR.

OK, so I think we need it on:
memory_region_init, memory_region_init_ram, memory_region_init_resizeable_ram, memory_region_init_ram_from_file, memory_region_init_ram_ptr, memory_region_init_rom, memory_region_init_rom_device (?)

I don't think it's needed in memory_region_init_ram_device_ptr

Dave

Quoted 11 lines

Quoted 3 lines

--
2.9.3

thanks
-- PMM

Signature · 2 lines

12345678901234567890123456789012345678901234567890123456789012345678901234567890
         1         2         3         4         5         6         7         8

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

Paolo Bonzini

2017-03-09 12:01 UTC #4 lore raw

Quoted 2 lines · Dr. David Alan Gilbert

* @name: Region name, becomes part of RAMBlock name used in migration stream
* must be unique within any device

Just a note, the uniqueness constraint also applies for RAM regions only.

Paolo

Dr. David Alan Gilbert

author

2017-03-09 12:11 UTC #5 lore raw

Quoted 7 lines · * Paolo Bonzini ([email protected]) wrote:

Quoted 2 lines · Dr. David Alan Gilbert
* @name: Region name, becomes part of RAMBlock name used in migration stream
* must be unique within any device
Just a note, the uniqueness constraint also applies for RAM regions only.

Yes, I'm only adding this comment to the ram/ram like functions now.

Dave

Quoted 1 line

Paolo

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK