Re: [Qemu-devel] [PATCH v2 for-1.6 v2 2/2] loader: put FW CFG ROM files into RAM

[Peter Maydell]

On 12 August 2013 19:16, Michael S. Tsirkin <address@hidden> wrote:
> +static void *rom_set_mr(Rom *rom, Object *owner, const char *name)
> +{
> +    /*
> +     * Migration code expects that all RAM blocks are full pages.
> +     * Round MR size up to satisfy this condition.
> +     */
> +    unsigned size = ROUND_UP(rom->datasize, qemu_migration_page_size);
> +    void *data = g_malloc0(size);
> +
> +    memcpy(data, rom->data, rom->datasize);
> +
> +    rom->mr = g_malloc(sizeof(*rom->mr));
> +    memory_region_init_ram_ptr(rom->mr, owner, name, size, data);
> +    memory_region_set_readonly(rom->mr, true);
> +    vmstate_register_ram_global(rom->mr);

So having thought about this a little I think the right answer
here is "don't use memory_region_init_ram_ptr()". At the moment
in-tree we have five users of this function:

hw/display/g364fb.c
hw/i386/kvm/pci-assign.c
hw/misc/ivshmem.c
hw/misc/vfio.c
target-ppc/kvm.c

The last four of these all absolutely have to have the guest
use a specific host pointer, typically the result of mmap()ing
something [shared file, PCI device, KVM_ALLOCATE_RMA fd, etc].
The first one I think should be converted to use
memory_region_init_ram() instead, because it doesn't need
to use a particular buffer.

Similarly, what you're trying to do here doesn't require
that the guest sees any specific host pointer, so you should
just use memory_region_init_ram().

We should add an assert to the _init_ram_ptr functions that
checks that the size is OK, as well.

I seem to recall having a conversation with Paolo along these
lines a few months back (we fixed the exynos devices which
were incorrectly using the _ram_ptr function); he can correct
me if I'm off-base here.

-- PMM