| Message ID | 97917e065d84b45174271d618572ce862a3d80ae.1455055858.git.alistair.francis@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Alistair Francis <alistair.francis@xilinx.com> writes: > Add a function called memory_region_add_subregion_no_print() that > creates memory subregions that won't be printed when running > the 'info mtree' command. > > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > Reviewed-by: KONRAD Frederic <fred.konrad@greensocs.com> > --- > > include/exec/memory.h | 17 +++++++++++++++++ > memory.c | 10 +++++++++- > 2 files changed, 26 insertions(+), 1 deletion(-) > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index c92734a..25353df 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -186,6 +186,7 @@ struct MemoryRegion { > bool skip_dump; > bool enabled; > bool warning_printed; /* For reservations */ > + bool do_not_print; > uint8_t vga_logging_count; > MemoryRegion *alias; > hwaddr alias_offset; > @@ -954,6 +955,22 @@ void memory_region_del_eventfd(MemoryRegion *mr, > void memory_region_add_subregion(MemoryRegion *mr, > hwaddr offset, > MemoryRegion *subregion); > + > +/** > + * memory_region_add_subregion_no_print: Add a subregion to a container. > + * > + * The same functionality as memory_region_add_subregion except that any > + * memory regions added by this function are not printed by 'info mtree'. > + * > + * @mr: the region to contain the new subregion; must be a container > + * initialized with memory_region_init(). > + * @offset: the offset relative to @mr where @subregion is added. > + * @subregion: the subregion to be added. > + */ > +void memory_region_add_subregion_no_print(MemoryRegion *mr, > + hwaddr offset, > + MemoryRegion *subregion); > + I think this needlessly complicates the memory region code and I'm not sure what is too be gained for the register code. The only usage of the code is inside a loop in register_init_block32. In each case the region has the same set of ops. Why isn't a single region being created with an indirect handler which can dispatch to the individual register handling code? While its true some drivers create individual IO regions by an large most are creating a block with a common handler. > /** > * memory_region_add_subregion_overlap: Add a subregion to a container > * with overlap. > diff --git a/memory.c b/memory.c > index 09041ed..228a8a7 100644 > --- a/memory.c > +++ b/memory.c > @@ -1829,6 +1829,14 @@ void memory_region_add_subregion(MemoryRegion *mr, > memory_region_add_subregion_common(mr, offset, subregion); > } > > +void memory_region_add_subregion_no_print(MemoryRegion *mr, > + hwaddr offset, > + MemoryRegion *subregion) > +{ > + memory_region_add_subregion(mr, offset, subregion); > + subregion->do_not_print = true; > +} > + > void memory_region_add_subregion_overlap(MemoryRegion *mr, > hwaddr offset, > MemoryRegion *subregion, > @@ -2219,7 +2227,7 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f, > const MemoryRegion *submr; > unsigned int i; > > - if (!mr) { > + if (!mr || mr->do_not_print) { > return; > } -- Alex Bennée
On 26 February 2016 at 16:51, Alex Bennée <alex.bennee@linaro.org> wrote: > I think this needlessly complicates the memory region code and I'm not > sure what is too be gained for the register code. The only usage of the > code is inside a loop in register_init_block32. In each case the region > has the same set of ops. Why isn't a single region being created with an > indirect handler which can dispatch to the individual register handling > code? > > While its true some drivers create individual IO regions by an large > most are creating a block with a common handler. Yeah, I have to say I'm not really convinced about having one MR per register -- the MR code was never intended to be used that way, and it seems like a good way to find nasty performance or memory usage surprises. thanks -- PMM
On Fri, Feb 26, 2016 at 8:54 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 26 February 2016 at 16:51, Alex Bennée <alex.bennee@linaro.org> wrote: >> I think this needlessly complicates the memory region code and I'm not >> sure what is too be gained for the register code. The only usage of the >> code is inside a loop in register_init_block32. In each case the region >> has the same set of ops. Why isn't a single region being created with an >> indirect handler which can dispatch to the individual register handling >> code? >> >> While its true some drivers create individual IO regions by an large >> most are creating a block with a common handler. > > Yeah, I have to say I'm not really convinced about having one MR > per register -- the MR code was never intended to be used that way, > and it seems like a good way to find nasty performance or memory > usage surprises. I have removed the one memory region per register. Thanks, Alistair > > thanks > -- PMM >
diff --git a/include/exec/memory.h b/include/exec/memory.h index c92734a..25353df 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -186,6 +186,7 @@ struct MemoryRegion { bool skip_dump; bool enabled; bool warning_printed; /* For reservations */ + bool do_not_print; uint8_t vga_logging_count; MemoryRegion *alias; hwaddr alias_offset; @@ -954,6 +955,22 @@ void memory_region_del_eventfd(MemoryRegion *mr, void memory_region_add_subregion(MemoryRegion *mr, hwaddr offset, MemoryRegion *subregion); + +/** + * memory_region_add_subregion_no_print: Add a subregion to a container. + * + * The same functionality as memory_region_add_subregion except that any + * memory regions added by this function are not printed by 'info mtree'. + * + * @mr: the region to contain the new subregion; must be a container + * initialized with memory_region_init(). + * @offset: the offset relative to @mr where @subregion is added. + * @subregion: the subregion to be added. + */ +void memory_region_add_subregion_no_print(MemoryRegion *mr, + hwaddr offset, + MemoryRegion *subregion); + /** * memory_region_add_subregion_overlap: Add a subregion to a container * with overlap. diff --git a/memory.c b/memory.c index 09041ed..228a8a7 100644 --- a/memory.c +++ b/memory.c @@ -1829,6 +1829,14 @@ void memory_region_add_subregion(MemoryRegion *mr, memory_region_add_subregion_common(mr, offset, subregion); } +void memory_region_add_subregion_no_print(MemoryRegion *mr, + hwaddr offset, + MemoryRegion *subregion) +{ + memory_region_add_subregion(mr, offset, subregion); + subregion->do_not_print = true; +} + void memory_region_add_subregion_overlap(MemoryRegion *mr, hwaddr offset, MemoryRegion *subregion, @@ -2219,7 +2227,7 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f, const MemoryRegion *submr; unsigned int i; - if (!mr) { + if (!mr || mr->do_not_print) { return; }