From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57375) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aZLcU-000499-7A for qemu-devel@nongnu.org; Fri, 26 Feb 2016 11:51:59 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aZLcQ-0005Tx-Vr for qemu-devel@nongnu.org; Fri, 26 Feb 2016 11:51:58 -0500 Received: from mail-wm0-x231.google.com ([2a00:1450:400c:c09::231]:38726) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aZLcQ-0005TO-M4 for qemu-devel@nongnu.org; Fri, 26 Feb 2016 11:51:54 -0500 Received: by mail-wm0-x231.google.com with SMTP id a4so77665000wme.1 for ; Fri, 26 Feb 2016 08:51:54 -0800 (PST) References: <97917e065d84b45174271d618572ce862a3d80ae.1455055858.git.alistair.francis@xilinx.com> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: <97917e065d84b45174271d618572ce862a3d80ae.1455055858.git.alistair.francis@xilinx.com> Date: Fri, 26 Feb 2016 16:51:51 +0000 Message-ID: <87io1b76u0.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v4 01/16] memory: Allow subregions to not be printed by info mtree List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alistair Francis Cc: edgar.iglesias@xilinx.com, peter.maydell@linaro.org, qemu-devel@nongnu.org, crosthwaitepeter@gmail.com, edgar.iglesias@gmail.com, afaerber@suse.de, fred.konrad@greensocs.com Alistair Francis 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 > Reviewed-by: KONRAD Frederic > --- > > 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