Re: [Qemu-devel] [PATCH 04/10] S390: check if BIOS is available and create links

[Dominik Dingel]

On Fri, 26 Apr 2013 17:57:44 +0200
Alexander Graf <address@hidden> wrote:

> 
> On 26.04.2013, at 17:48, Dominik Dingel wrote:
> 
> > On Fri, 26 Apr 2013 17:23:18 +0200
> > Alexander Graf <address@hidden> wrote:
> > 
> >> 
> >> On 26.04.2013, at 14:12, Dominik Dingel wrote:
> >> 
> >>> Signed-off-by: Dominik Dingel <address@hidden>
> >>> 
> >>> Check if the BIOS is available before loading it into the memory.
> >>> Create the needed Links for build.
> >>> 
> >>> Add a if the BIOS is available, also add the needed links for the build
> >>> process.
> >>> 
> >>> diff --git a/configure b/configure
> >>> index 19777de..2bbbd54 100755
> >>> --- a/configure
> >>> +++ b/configure
> >>> @@ -4541,6 +4541,7 @@ for bios_file in \
> >>>    $source_path/pc-bios/*.aml \
> >>>    $source_path/pc-bios/*.rom \
> >>>    $source_path/pc-bios/*.dtb \
> >>> +    $source_path/pc-bios/*.img \
> >>>    $source_path/pc-bios/openbios-* \
> >>>    $source_path/pc-bios/palcode-*
> >>> do
> >>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> >>> index 9758529..36daa67 100644
> >>> --- a/hw/s390x/ipl.c
> >>> +++ b/hw/s390x/ipl.c
> >>> @@ -95,6 +95,10 @@ static int s390_ipl_init(SysBusDevice *dev)
> >>>        }
> >>> 
> >>>        bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> >>> +        if (bios_filename == NULL) {
> >>> +            hw_error("could not find stage1 bootloader\n");
> >>> +        }
> >>> +        
> >>>        bios_size = load_elf(bios_filename, NULL, NULL, &ipl->start_addr, 
> >>> NULL,
> >>>                             NULL, 1, ELF_MACHINE, 0);
> >>>        if (bios_size == -1UL) {
> >>> diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
> >>> index ad55a14..1e6ab1e 100644
> >>> --- a/pc-bios/s390-ccw/Makefile
> >>> +++ b/pc-bios/s390-ccw/Makefile
> >>> @@ -21,6 +21,7 @@ s390-ccw.elf: $(OBJECTS)
> >>> 
> >>> s390-ccw.img: s390-ccw.elf
> >>>   $(call quiet-command,strip $< -o $@,"  Stripping $(TARGET_DIR)$@")
> >>> + ln -s -f  `pwd`/$@ ../$@ 
> >> 
> >> I don't think we do this for any other blobs, so why should we here?
> >> 
> >> Alex
> > 
> > In case we are not running on the s390 platform we need the link to the 
> > shipped binary. 
> > But if we are running on the s390 platform we like to build the rom in the 
> > s390-ccw build folder. After that we either copy or link it to the pc-bios 
> > location. 
> > I personally preferred the link version.
> 
> It's not what users expect when they compile code inside of pc-bios. Please 
> stick to whatever behavior other blobs that we compile on demand have.
> 
> 
> Alex
> 

I checked on my current laptop, the thing is, if we build the blob, we also 
want to use it. If we don't overwrite it the link, we will continue to use the 
old shipped blob.

The linuxboot.img is imho wrong in it's way. As we build it, but the link still 
points to the qemu shipped one.

So you change linuxboot.S. If you know rebuild qemu, do you want to use the 
chaned version of linuxboot.img or the shipped one?

build/pc-bios$ diff ./optionrom/linuxboot.img ../../qemu/pc-bios/linuxboot.bin 
Binary files ./optionrom/linuxboot.img and ../../qemu/pc-bios/linuxboot.bin 
differ

Dominik