[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Reorganize option rom (+linux kernel) loading.
From: |
Blue Swirl |
Subject: |
Re: [Qemu-devel] [PATCH] Reorganize option rom (+linux kernel) loading. |
Date: |
Wed, 30 Sep 2009 22:51:06 +0300 |
On Wed, Sep 30, 2009 at 6:43 PM, Gerd Hoffmann <address@hidden> wrote:
> This patch adds infrastructure to maintain memory regions which must be
> restored on reset. That includes roms (vga bios and option roms on pc),
> but is also used when loading linux kernels directly. Features:
>
> - loading files is supported.
> - passing blobs is supported.
> - target address range is supported (for optionrom area).
> - fixed target memory address is supported (linux kernel).
>
> The final memory layout is created once all memory regions are
> registered. The option roms get addresses assigned and the
> registered regions are checked against overlaps. Finally all data
> is copyed to the guest memory.
>
> Advantages:
>
> (1) Filling memory on initial boot and on reset takes the same
> code path, making reset more robust.
> (2) The need to keep track of the option rom load address is gone.
> (3) Due to (2) option roms can be loaded outside pc_init(). This
> allows to move the pxe rom loading into the nic drivers for
> example.
>
> Additional bonus: There is a 'info roms' monitor command now.
>
> The patch also switches over pc.c and removes the
> option_rom_setup_reset() and load_option_rom() functions.
Nice idea. The implementation seems to be buggy, I only ever see one
rom and for example sparc-test crashes when issuing 'info roms'.
Perhaps 'info roms' could also check whether the rom has been changed?
On most platforms roms are really read only memories.
> + fd = open(rom->path, O_RDONLY);
> + if (-1 == fd) {
Not again!
> + rom->align = align;
> + rom->min = min;
> + rom->max = max;
> + rom->romsize = lseek(fd, 0, SEEK_END);
> + rom->data = qemu_mallocz(rom->romsize);
There are plenty of extra spaces.