[Qemu-devel] [PATCH v1 0/3] Memory: Trivial fixes

[Qemu-devel] [PATCH v1 0/3] Memory: Trivial fixes

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Some trivial fixes to memory API.


Peter Crosthwaite (3):
  memory: Fix comment typo
  memory: as_update_topology_pass: Improve comments
  memory: render_memory_region: factor out fr constant setters

 memory.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

-- 
1.8.3.rc1.44.gb387c77.dirty

[Qemu-devel] [PATCH v1 1/3] memory: Fix comment typo

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

s/ajacent/adjacent

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/memory.c b/memory.c
index 5cb8f4a..60e033b 100644
--- a/memory.c
+++ b/memory.c
@@ -282,7 +282,7 @@ static bool can_merge(FlatRange *r1, FlatRange *r2)
         && r1->readonly == r2->readonly;
 }
 
-/* Attempt to simplify a view by merging ajacent ranges */
+/* Attempt to simplify a view by merging adjacent ranges */
 static void flatview_simplify(FlatView *view)
 {
     unsigned i, j;
-- 
1.8.3.rc1.44.gb387c77.dirty

[Qemu-devel] [PATCH v1 2/3] memory: as_update_topology_pass: Improve comments

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

These comments we're a little difficult to read. First one had
incorrect parenthesis. The part about attributes changing is
really applicable to the region being 'in both' rather than 'in
new'

Second comment has an obscure parenthetic about 'Logging may have
changed'. Made clearer, as this if is supposed to handle the case where
the memory region is unchanged (with the notable exception re logging).

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 memory.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/memory.c b/memory.c
index 60e033b..7e710c4 100644
--- a/memory.c
+++ b/memory.c
@@ -719,7 +719,7 @@ static void address_space_update_topology_pass(AddressSpace *as,
                 || int128_lt(frold->addr.start, frnew->addr.start)
                 || (int128_eq(frold->addr.start, frnew->addr.start)
                     && !flatrange_equal(frold, frnew)))) {
-            /* In old, but (not in new, or in new but attributes changed). */
+            /* In old but not in new, or in both but attributes changed. */
 
             if (!adding) {
                 MEMORY_LISTENER_UPDATE_REGION(frold, as, Reverse, region_del);
@@ -727,7 +727,7 @@ static void address_space_update_topology_pass(AddressSpace *as,
 
             ++iold;
         } else if (frold && frnew && flatrange_equal(frold, frnew)) {
-            /* In both (logging may have changed) */
+            /* In both and unchanged (except logging may have changed) */
 
             if (adding) {
                 MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, region_nop);
-- 
1.8.3.rc1.44.gb387c77.dirty

[Qemu-devel] [PATCH v1 3/3] memory: render_memory_region: factor out fr constant setters

[peter.crosthwaite]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

These 4 replicated lines set properties of fr that are constant over
the course of the function. Factor out their repeated setting (and also
guards against them being set multiple times in the loop below).

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 memory.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/memory.c b/memory.c
index 7e710c4..7bdfc58 100644
--- a/memory.c
+++ b/memory.c
@@ -556,6 +556,11 @@ static void render_memory_region(FlatView *view,
     base = clip.start;
     remain = clip.size;
 
+    fr.mr = mr;
+    fr.dirty_log_mask = mr->dirty_log_mask;
+    fr.romd_mode = mr->romd_mode;
+    fr.readonly = readonly;
+
     /* Render the region itself into any gaps left by the current view. */
     for (i = 0; i < view->nr && int128_nz(remain); ++i) {
         if (int128_ge(base, addrrange_end(view->ranges[i].addr))) {
@@ -564,12 +569,8 @@ static void render_memory_region(FlatView *view,
         if (int128_lt(base, view->ranges[i].addr.start)) {
             now = int128_min(remain,
                              int128_sub(view->ranges[i].addr.start, base));
-            fr.mr = mr;
             fr.offset_in_region = offset_in_region;
             fr.addr = addrrange_make(base, now);
-            fr.dirty_log_mask = mr->dirty_log_mask;
-            fr.romd_mode = mr->romd_mode;
-            fr.readonly = readonly;
             flatview_insert(view, i, &fr);
             ++i;
             int128_addto(&base, now);
@@ -584,12 +585,8 @@ static void render_memory_region(FlatView *view,
         int128_subfrom(&remain, now);
     }
     if (int128_nz(remain)) {
-        fr.mr = mr;
         fr.offset_in_region = offset_in_region;
         fr.addr = addrrange_make(base, remain);
-        fr.dirty_log_mask = mr->dirty_log_mask;
-        fr.romd_mode = mr->romd_mode;
-        fr.readonly = readonly;
         flatview_insert(view, i, &fr);
     }
 }
-- 
1.8.3.rc1.44.gb387c77.dirty

Re: [Qemu-devel] [PATCH v1 0/3] Memory: Trivial fixes

[Paolo Bonzini]

Il 03/06/2013 07:31, peter.crosthwaite@xilinx.com ha scritto:
> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> 
> Some trivial fixes to memory API.
> 
> 
> Peter Crosthwaite (3):
>   memory: Fix comment typo
>   memory: as_update_topology_pass: Improve comments
>   memory: render_memory_region: factor out fr constant setters
> 
>  memory.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
> 

Thanks, applying to my iommu branch.

Re: [Qemu-devel] [PATCH v1 2/3] memory: as_update_topology_pass: Improve comments

[Andreas Färber]

Am 03.06.2013 07:32, schrieb peter.crosthwaite@xilinx.com:
> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> 
> These comments we're a little difficult to read. First one had

While we're picky: "were" - Paolo can you fix up in your queue?

Andreas

> incorrect parenthesis. The part about attributes changing is
> really applicable to the region being 'in both' rather than 'in
> new'
> 
> Second comment has an obscure parenthetic about 'Logging may have
> changed'. Made clearer, as this if is supposed to handle the case where
> the memory region is unchanged (with the notable exception re logging).
> 
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
> 
>  memory.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/memory.c b/memory.c
> index 60e033b..7e710c4 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -719,7 +719,7 @@ static void address_space_update_topology_pass(AddressSpace *as,
>                  || int128_lt(frold->addr.start, frnew->addr.start)
>                  || (int128_eq(frold->addr.start, frnew->addr.start)
>                      && !flatrange_equal(frold, frnew)))) {
> -            /* In old, but (not in new, or in new but attributes changed). */
> +            /* In old but not in new, or in both but attributes changed. */
>  
>              if (!adding) {
>                  MEMORY_LISTENER_UPDATE_REGION(frold, as, Reverse, region_del);
> @@ -727,7 +727,7 @@ static void address_space_update_topology_pass(AddressSpace *as,
>  
>              ++iold;
>          } else if (frold && frnew && flatrange_equal(frold, frnew)) {
> -            /* In both (logging may have changed) */
> +            /* In both and unchanged (except logging may have changed) */
>  
>              if (adding) {
>                  MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, region_nop);
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v1 2/3] memory: as_update_topology_pass: Improve comments

[Paolo Bonzini]

Il 04/06/2013 13:43, Andreas Färber ha scritto:
> Am 03.06.2013 07:32, schrieb peter.crosthwaite@xilinx.com:
>> > From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> > 
>> > These comments we're a little difficult to read. First one had
> While we're picky: "were" - Paolo can you fix up in your queue?

Of course.

Paolo