Message ID | 1378725696-13590-2-git-send-email-marcel.a@redhat.com |
---|---|
State | New |
Headers | show |
On Mon, Sep 09, 2013 at 02:21:35PM +0300, Marcel Apfelbaum wrote: > Priority is used to make visible some subregions by obscuring > the parent MemoryRegion addresses overlapping with the subregion. > > By allowing the priority to be negative the opposite can be done: > Allow a subregion to be visible on all the addresses not covered > by the parent MemoryRegion or other subregions. > > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> Seems harmless enough. Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > --- > include/exec/memory.h | 6 +++--- > memory.c | 2 +- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index ebe0d24..6995087 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -153,7 +153,7 @@ struct MemoryRegion { > bool flush_coalesced_mmio; > MemoryRegion *alias; > hwaddr alias_offset; > - unsigned priority; > + int priority; > bool may_overlap; > QTAILQ_HEAD(subregions, MemoryRegion) subregions; > QTAILQ_ENTRY(MemoryRegion) subregions_link; > @@ -193,7 +193,7 @@ struct MemoryListener { > void (*coalesced_mmio_del)(MemoryListener *listener, MemoryRegionSection *section, > hwaddr addr, hwaddr len); > /* Lower = earlier (during add), later (during del) */ > - unsigned priority; > + int priority; > AddressSpace *address_space_filter; > QTAILQ_ENTRY(MemoryListener) link; > }; > @@ -779,7 +779,7 @@ void memory_region_add_subregion(MemoryRegion *mr, > void memory_region_add_subregion_overlap(MemoryRegion *mr, > hwaddr offset, > MemoryRegion *subregion, > - unsigned priority); > + int priority); > > /** > * memory_region_get_ram_addr: Get the ram address associated with a memory > diff --git a/memory.c b/memory.c > index 5a10fd0..984a3dc 100644 > --- a/memory.c > +++ b/memory.c > @@ -1473,7 +1473,7 @@ void memory_region_add_subregion(MemoryRegion *mr, > void memory_region_add_subregion_overlap(MemoryRegion *mr, > hwaddr offset, > MemoryRegion *subregion, > - unsigned priority) > + int priority) > { > subregion->may_overlap = true; > subregion->priority = priority; > -- > 1.8.3.1
On 9 September 2013 12:41, Michael S. Tsirkin <mst@redhat.com> wrote: > On Mon, Sep 09, 2013 at 02:21:35PM +0300, Marcel Apfelbaum wrote: >> Priority is used to make visible some subregions by obscuring >> the parent MemoryRegion addresses overlapping with the subregion. >> >> By allowing the priority to be negative the opposite can be done: >> Allow a subregion to be visible on all the addresses not covered >> by the parent MemoryRegion or other subregions. >> >> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> > > Seems harmless enough. > > Reviewed-by: Michael S. Tsirkin <mst@redhat.com> No, the idea is good but this version is just broken. See the comments I made on the previous version which Marcel ignored :-( -- PMM
On Mon, Sep 09, 2013 at 12:45:12PM +0100, Peter Maydell wrote: > On 9 September 2013 12:41, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Mon, Sep 09, 2013 at 02:21:35PM +0300, Marcel Apfelbaum wrote: > >> Priority is used to make visible some subregions by obscuring > >> the parent MemoryRegion addresses overlapping with the subregion. > >> > >> By allowing the priority to be negative the opposite can be done: > >> Allow a subregion to be visible on all the addresses not covered > >> by the parent MemoryRegion or other subregions. > >> > >> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> > > > > Seems harmless enough. > > > > Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > > No, the idea is good but this version is just broken. > See the comments I made on the previous version which > Marcel ignored :-( > > -- PMM You are right, I missed the bugs. Good catch, thanks.
On Mon, Sep 09, 2013 at 02:48:55PM +0300, Michael S. Tsirkin wrote: > On Mon, Sep 09, 2013 at 12:45:12PM +0100, Peter Maydell wrote: > > On 9 September 2013 12:41, Michael S. Tsirkin <mst@redhat.com> wrote: > > > On Mon, Sep 09, 2013 at 02:21:35PM +0300, Marcel Apfelbaum wrote: > > >> Priority is used to make visible some subregions by obscuring > > >> the parent MemoryRegion addresses overlapping with the subregion. > > >> > > >> By allowing the priority to be negative the opposite can be done: > > >> Allow a subregion to be visible on all the addresses not covered > > >> by the parent MemoryRegion or other subregions. > > >> > > >> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> > > > > > > Seems harmless enough. > > > > > > Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > > > > No, the idea is good but this version is just broken. > > See the comments I made on the previous version which > > Marcel ignored :-( > > > > -- PMM > > You are right, I missed the bugs. > Good catch, thanks. > So I guess it's only an Acked-by: Michael S. Tsirkin <mst@redhat.com> at this point :)
diff --git a/include/exec/memory.h b/include/exec/memory.h index ebe0d24..6995087 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -153,7 +153,7 @@ struct MemoryRegion { bool flush_coalesced_mmio; MemoryRegion *alias; hwaddr alias_offset; - unsigned priority; + int priority; bool may_overlap; QTAILQ_HEAD(subregions, MemoryRegion) subregions; QTAILQ_ENTRY(MemoryRegion) subregions_link; @@ -193,7 +193,7 @@ struct MemoryListener { void (*coalesced_mmio_del)(MemoryListener *listener, MemoryRegionSection *section, hwaddr addr, hwaddr len); /* Lower = earlier (during add), later (during del) */ - unsigned priority; + int priority; AddressSpace *address_space_filter; QTAILQ_ENTRY(MemoryListener) link; }; @@ -779,7 +779,7 @@ void memory_region_add_subregion(MemoryRegion *mr, void memory_region_add_subregion_overlap(MemoryRegion *mr, hwaddr offset, MemoryRegion *subregion, - unsigned priority); + int priority); /** * memory_region_get_ram_addr: Get the ram address associated with a memory diff --git a/memory.c b/memory.c index 5a10fd0..984a3dc 100644 --- a/memory.c +++ b/memory.c @@ -1473,7 +1473,7 @@ void memory_region_add_subregion(MemoryRegion *mr, void memory_region_add_subregion_overlap(MemoryRegion *mr, hwaddr offset, MemoryRegion *subregion, - unsigned priority) + int priority) { subregion->may_overlap = true; subregion->priority = priority;
Priority is used to make visible some subregions by obscuring the parent MemoryRegion addresses overlapping with the subregion. By allowing the priority to be negative the opposite can be done: Allow a subregion to be visible on all the addresses not covered by the parent MemoryRegion or other subregions. Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> --- include/exec/memory.h | 6 +++--- memory.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)