Re: [PATCH] fix extboot from boot with cache=off

[Avi Kivity <avi@redhat.com>]

Nolan wrote:
> Extboot submits requests with whatever buffer alignment the guest gave
> to the BIOS.  This breaks with O_DIRECT disks, as they require 512 byte
> alignment.
>
> Most guest bootloaders sector align their requests out of paranoia, but
> the OpenBSD bootloader does not.
>
> This patch always copies.  Since extboot is only used at boot time to
> load limited amounts of data, the overhead is not problematic.  I also
> switched to using cpu_physical_memory_* instead of groveling around with
> phys_ram_base directly.
>
> Signed-off-by: Nolan Leake <nolan <at> sigbus.net>
>
> ---
>
>  qemu/hw/extboot.c |   35 ++++++++++++++++++++++++-----------
>  1 files changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/qemu/hw/extboot.c b/qemu/hw/extboot.c
> index ada0fdd..ea16b3e 100644
> --- a/qemu/hw/extboot.c
> +++ b/qemu/hw/extboot.c
> @@ -77,19 +77,29 @@ static void extboot_write_cmd(void *opaque, uint32_t addr, uint32_t value)
>      BlockDriverState *bs = opaque;
>      int cylinders, heads, sectors, err;
>      uint64_t nb_sectors;
> -
> -    get_translated_chs(bs, &cylinders, &heads, &sectors);
> +    target_phys_addr_t pa;
> +    int blen;
> +    void *buf = NULL;
>  
>      if (cmd->type == 0x01 || cmd->type == 0x02) {
> -	target_ulong pa = cmd->xfer.segment * 16 + cmd->xfer.segment;
> +	pa = cmd->xfer.segment * 16 + cmd->xfer.offset;
> +        blen = cmd->xfer.nb_sectors * 512;
> +        buf = qemu_memalign(512, blen);
> +        if (!buf) {
> +            printf("qemu_memalign failed\n");
> +            return;
> +        }
>   

Don't check for allocation failures.  qemu_malloc() terminates 
gracefully on failure, qemu_memalign() does not, but should.

>  
>  	/* possible buffer overflow */
> -	if ((pa + cmd->xfer.nb_sectors * 512) > phys_ram_size)
> +	if ((pa + blen) > phys_ram_size) {
> +            qemu_free(buf);
>  	    return;
> +        }
>   

cpu_physical_memory_rw() will check these for you.

All in all, a nice improvement in addition to the bug fix.

-- 
error compiling committee.c: too many arguments to function