On 21.11.2013, at 13:08, Michael S. Tsirkin <m...@redhat.com> wrote:
> pc-bios/s390-zipl.rom is a flat image so it's expected that
> loading it as elf will fail.
> It should fall back on loading a flat file, but doesn't
> on 32 bit systems, instead it fails printing:
> qemu: hardware error: could not load bootloader 's390-zipl.rom'
>
> The result is boot failure.
>
> The reason is that a 64 bit unsigned interger which is set
> to -1 on error is compared to -1UL which on a 32 bit system
> with gcc is a 32 bit unsigned interger.
> Since both are unsigned, no sign extension takes place and
> comparison evaluates to non-equal.
>
> There's no reason to do clever tricks: -1 will cause
> sign extension to happen correctly automatically.
>
> Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> ---
> hw/s390x/ipl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index d69adb2..88115e9 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -80,7 +80,7 @@ static int s390_ipl_init(SysBusDevice *dev)
>
> bios_size = load_elf(bios_filename, NULL, NULL, &ipl->start_addr,
> NULL,
> NULL, 1, ELF_MACHINE, 0);
> - if (bios_size == -1UL) {
> + if (bios_size == -1) {
I still dislike that we have 2 completely separate checks for the same thing.
One here, one a few lines below checking for (long)bios_size < 0. I would very
much like to see them at least fail identically :). I also don't think that
comparing to == -1 is really more safe than comparing to -1UL (what is -1UL
anyway? Negatives aren't unsigned, are they?).
But as a quick fix for 1.7 it's good enough IMHO.
Acked-by: Alexander Graf <ag...@suse.de>
Alex
