Re: [Qemu-arm] [PATCH v4 4/8] bcm2835_peripherals: add rollup device for bcm2835 peripherals

[Peter Maydell]

On 26 January 2016 at 19:12, Andrew Baumann
<address@hidden> wrote:
>> From: Peter Crosthwaite [mailto:address@hidden
>> Sent: Tuesday, 26 January 2016 00:03
>> On Mon, Jan 25, 2016 at 10:23 PM, Andrew Baumann
>> <address@hidden> wrote:
>> >> > +    /* Memory region for peripheral devices, which we export to our
>> >> parent */
>> >> > +    memory_region_init_io(&s->peri_mr, obj, NULL, s, "bcm2835-
>> >> peripherals",
>> >> > +                          0x1000000);
>> >>
>> >> Should this just be normal memory_region_init?
>> >
>> > I think so -- it's just a container region, and I probably copy and pasted 
>> > the
>> API here. The two MR init APIs seem almost but not-quite identical when
>> NULL callbacks are used. Can you briefly explain the difference?

NULL callbacks are the same thing as memory_region_init_reservation(),
which is for defining a "you can't get here" part of the address space.
This is used only when KVM is enabled, by devices whose real implementation
of a bit of memory mapped IO is in the host kernel. The host kernel will
intercept accesses to that region and QEMU never sees accesses to them
(exception: emulated DMA, which is the only reason there's a behaviour
at all rather than an assertion). This is why such regions get marked as
mr->terminates: they really do have no "holes" in them.

So the general rules are:
 * use a plain container (created via memory_region_init())
   for almost any situation where you're creating something whose
   behaviour is built up by adding together subregions
 * use memory_region_init_io() with non-NULL ops for creating straightforward
   "leaf" regions which have defined device behaviour
 * similarly, memory_region_init_ram() for ram, etc
 * use memory_region_init_reservation() or memory_region_init_io() with a
   NULL ops pointer if you're the QEMU part of a KVM in-kernel
   irqchip/etc.
 * If you have a container memory region, but you need a "behaviour for
   the bits that aren't covered by specific subregions" that's specific to
   this device, then you have two choices:
   (a) use a "background" subregion that covers the whole space and
       has a lower priority than any of the other subregions
   (b) create your container with one of the other memory_region_init
       functions and use the IO callbacks/ram/etc to define the background
       behaviour. Such an object will still support subregions being added.
   I think (a) is generally cleaner, but the API permits adding
   subregions to MRs other than pure containers for the benefit of
   the odd few cases where (b) is done.

docs/memory.txt could probably use an update to:
(a) specifically mention the functions to use to create the various
regions listed in the 'Types of regions' section
(b) describe the 'reservation' kind of memory region

thanks
-- PMM