It is not easy for people to know "what this parameter does" before
knowing "what is subpage". Let's use "is_mmio" to make it easier to
understand.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 exec.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/exec.c b/exec.c
index 1d76c63..0adae94 100644
--- a/exec.c
+++ b/exec.c
@@ -403,7 +403,7 @@ bool memory_region_is_unassigned(MemoryRegion *mr)
 /* Called from RCU critical section */
 static MemoryRegionSection *address_space_lookup_region(AddressSpaceDispatch *d,
                                                         hwaddr addr,
-                                                        bool resolve_subpage)
+                                                        bool is_mmio)
 {
     MemoryRegionSection *section = atomic_read(&d->mru_section);
     subpage_t *subpage;
@@ -416,7 +416,7 @@ static MemoryRegionSection *address_space_lookup_region(AddressSpaceDispatch *d,
         section = phys_page_find(d, addr);
         update = true;
     }
-    if (resolve_subpage && section->mr->subpage) {
+    if (is_mmio && section->mr->subpage) {
         subpage = container_of(section->mr, subpage_t, iomem);
         section = &d->map.sections[subpage->sub_section[SUBPAGE_IDX(addr)]];
     }
@@ -429,13 +429,13 @@ static MemoryRegionSection *address_space_lookup_region(AddressSpaceDispatch *d,
 /* Called from RCU critical section */
 static MemoryRegionSection *
 address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *xlat,
-                                 hwaddr *plen, bool resolve_subpage)
+                                 hwaddr *plen, bool is_mmio)
 {
     MemoryRegionSection *section;
     MemoryRegion *mr;
     Int128 diff;
 
-    section = address_space_lookup_region(d, addr, resolve_subpage);
+    section = address_space_lookup_region(d, addr, is_mmio);
     /* Compute offset within MemoryRegionSection */
     addr -= section->offset_within_address_space;
 
-- 
2.7.4

On 15/05/2017 10:50, Peter Xu wrote:
> It is not easy for people to know "what this parameter does" before
> knowing "what is subpage". Let's use "is_mmio" to make it easier to
> understand.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

Maybe invert the direction and call it for_iotlb?

Paolo

> ---
>  exec.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 1d76c63..0adae94 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -403,7 +403,7 @@ bool memory_region_is_unassigned(MemoryRegion *mr)
>  /* Called from RCU critical section */
>  static MemoryRegionSection *address_space_lookup_region(AddressSpaceDispatch *d,
>                                                          hwaddr addr,
> -                                                        bool resolve_subpage)
> +                                                        bool is_mmio)
>  {
>      MemoryRegionSection *section = atomic_read(&d->mru_section);
>      subpage_t *subpage;
> @@ -416,7 +416,7 @@ static MemoryRegionSection *address_space_lookup_region(AddressSpaceDispatch *d,
>          section = phys_page_find(d, addr);
>          update = true;
>      }
> -    if (resolve_subpage && section->mr->subpage) {
> +    if (is_mmio && section->mr->subpage) {
>          subpage = container_of(section->mr, subpage_t, iomem);
>          section = &d->map.sections[subpage->sub_section[SUBPAGE_IDX(addr)]];
>      }
> @@ -429,13 +429,13 @@ static MemoryRegionSection *address_space_lookup_region(AddressSpaceDispatch *d,
>  /* Called from RCU critical section */
>  static MemoryRegionSection *
>  address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *xlat,
> -                                 hwaddr *plen, bool resolve_subpage)
> +                                 hwaddr *plen, bool is_mmio)
>  {
>      MemoryRegionSection *section;
>      MemoryRegion *mr;
>      Int128 diff;
>  
> -    section = address_space_lookup_region(d, addr, resolve_subpage);
> +    section = address_space_lookup_region(d, addr, is_mmio);
>      /* Compute offset within MemoryRegionSection */
>      addr -= section->offset_within_address_space;
>  
> 

On Mon, May 15, 2017 at 11:03:49AM +0200, Paolo Bonzini wrote:
> 
> 
> On 15/05/2017 10:50, Peter Xu wrote:
> > It is not easy for people to know "what this parameter does" before
> > knowing "what is subpage". Let's use "is_mmio" to make it easier to
> > understand.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> 
> Maybe invert the direction and call it for_iotlb?

I wasn't aware that tcg is using resolve_subpage==false always. Then
at least is_mmio does not suite here. I am not sure whether for-iotlb
suites here either... So maybe I will just drop this patch as well,
just like patch 3.

Thanks for reviewing it!

-- 
Peter Xu