Re: [Qemu-devel] [PATCH v2] register reset handler to write image into memory

[Yin Olivia-R63875]

Hi Alex,

I checked all the rom_add_*() functions.
Multiple platforms of different architectures use rom_add_* to save images.
        hw/arm_boot.c
        hw/exynos4210.
        hw/highbank.
        hw/mips_fulong2e.c
        hw/mips_malta.c
        hw/mips_r4k.c
        hw/r2d.c

Even for PowerPC, it also use rom_add_blob() to write dtb in memory.
        hw/ppc/e500.c:  rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, 
fdt_size, addr);
        hw/ppc440_bamboo.c

You also minder that ELF file.
        hw/elf_ops.h:   rom_add_blob_fixed(label, data, mem_size, addr);

pstrcpy_targphys() does also call rom_add_blob_fixed() function, so we need 
also verify
        hw/alpha_dp264.c
        hw/cris-boot.c
        hw/lm32_boards.c
        hw/microblaze_boot.c
        hw/milkymist.c
        hw/ppc.c
        hw/ppc_newworld.c
        hw/ppc_oldworld.c
        hw/sun4m.c
        hw/sun4m.c

Should we register reset handler for each above user?

The callers of rom_ptr() function:
        hw/s390-virtio.c
        hw/sun4m.c
        hw/sun4u.c
        target-arm/cpu.c
But I don't understand why rom_ptr should be changed.

Best Regards,
Olivia


> -----Original Message-----
> From: Alexander Graf [mailto:address@hidden
> Sent: Thursday, September 20, 2012 8:10 PM
> To: Andreas Färber
> Cc: Yin Olivia-R63875; address@hidden; address@hidden
> Subject: Re: [Qemu-devel] [PATCH v2] register reset handler to write
> image into memory
> 
> 
> On 23.08.2012, at 13:38, Andreas Färber wrote:
> 
> > Hi,
> >
> > Am 23.08.2012 11:45, schrieb Yin Olivia-R63875:
> >> Dear All,
> >>
> >> I can't find MAINTAINER of hw/loader.c.
> >> Who can help review and apply this patch?
> >
> > This patch is not a small bugfix so it won't be applied during the
> > v1.2 Hard Freeze. You based it onto ppc-next so the obvious answer is,
> > Alex needs to review it, whom you forgot to cc.
> >
> > This patch does not answer the question why you try to avoid the ROM
> > blobs and what ROM blobs are still being used for after your patch. I
> > don't think it makes much sense to work around them for your use cases
> > and to leave them behind - if there's something fundamentally wrong
> > with them they should be ripped out completely or fixed. But maybe I'm
> > misunderstanding in the absence of explanations?
> 
> The fundamental problem is the memory footprint. We only need ROM blobs
> on reset, where they get copied into guest RAM. That means during the
> lifetime of the VM, we waste host memory for no good reason. Imagine a
> guest that runs with -kernel and -initrd, each 10MB in size. Then that VM
> wastes 20MB of precious host memory.
> 
> Eventually, I don't think we will need the full-blown rom interface with
> in-memory rom blobs anymore. Everything should be constructed on demand
> during reset.
> 
> However, if you look at code like the s390 initialization, some users of
> the rom interface expect changes done once to be persistent. These will
> have to be rewritten to redo their changes on reset.
> 
> So Olivia, please do the following:
> 
>   - Make sure that _no_ persistent rom code is left over eventually. This
> also means that you need to convert ELF.
>   - Go through every user of rom_ptr and write the code differently. For
> now, probably by just registering a reset handler that overwrites the
> respective memory location on every reset, rather than modify the rom
> blob.
> 
> 
> Alex
>