[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC v2 1/2] memory: allow MemoryRegion's priorit
From: |
Marcel Apfelbaum |
Subject: |
Re: [Qemu-devel] [PATCH RFC v2 1/2] memory: allow MemoryRegion's priority field to accept negative values |
Date: |
Mon, 09 Sep 2013 15:03:11 +0300 |
On Mon, 2013-09-09 at 12:28 +0100, Peter Maydell wrote:
> On 9 September 2013 12:11, Marcel Apfelbaum <address@hidden> 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 <address@hidden>
> > ---
> > 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;
>
> You haven't addressed any of the points I made reviewing
> the first version of this. Please don't just ignore
> code review, or people will stop reviewing your code.
Hi Peter,
I really value your comments and I did acted upon them.
Basically all the changes of this version are based on your comments, thanks!
I did answer to your comment and I was going to remove it,
but I missed it again :(. Sorry for that.
I will remove it of course in the following version.
>
> I think it would also be nice to update docs/memory.txt
> to say explicitly that priority is signed and why this
> is useful, something like this:
Of course I will, thanks!
>
> ====begin====
> 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 that must sit 'above' any others (with a
> positive priority) and also a background region that
> sits 'below' others (with a negative priority).
> ====endit====
>
> (in the 'Overlapping regions and priority' section
> of that document).
>
> thanks
> -- PMM
>
- [Qemu-devel] [PATCH RFC v2 0/3] pci: complete master abort protocol, Marcel Apfelbaum, 2013/09/09
- [Qemu-devel] [PATCH RFC v2 1/2] memory: allow MemoryRegion's priority field to accept negative values, Marcel Apfelbaum, 2013/09/09
- [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Marcel Apfelbaum, 2013/09/09
- Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Michael S. Tsirkin, 2013/09/09
- Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Marcel Apfelbaum, 2013/09/09
- Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Michael S. Tsirkin, 2013/09/09
- Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Marcel Apfelbaum, 2013/09/09
- Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Peter Maydell, 2013/09/09
- Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Michael S. Tsirkin, 2013/09/09
- Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Peter Maydell, 2013/09/09
- Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Marcel Apfelbaum, 2013/09/09
- Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Peter Maydell, 2013/09/09