diff mbox

write option roms in pc-bios/

Message ID 1261664861-15493-1-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Dec. 24, 2009, 2:27 p.m. UTC
>> The gdt address calculation in linuxboot.bin is broken in two ways: first
>> it loads %cs into %eax, but that instruction leaves the high bits of %eax
>> undefined and we did not clear them. Secondly, we completely ignore the
>> incorrect %eax, and use the undefined %ebx instead.
>>
>> With these issues fixed, linuxboot works again.
>
> Wow, I wonder how it worked for me...

Got it.  The option roms are built in pc-bios/optionrom, but QEMU loads
the ones in pc-bios.  So actually I was always testing Alexander's code
even though I had done a "make -B" to feel safer. :-(

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 pc-bios/optionrom/Makefile |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Comments

Avi Kivity Dec. 24, 2009, 2:50 p.m. UTC | #1
On 12/24/2009 04:27 PM, Paolo Bonzini wrote:
>
> Got it.  The option roms are built in pc-bios/optionrom, but QEMU loads
> the ones in pc-bios.  So actually I was always testing Alexander's code
> even though I had done a "make -B" to feel safer. :-(
>
>    

Ah.  When things work straight off for me I always suspect something, so 
I introduce a bug deliberately to make sure I'm testing what I think I'm 
testing.

Of course, that happens only very rarely.
Avi Kivity Dec. 24, 2009, 3:03 p.m. UTC | #2
On 12/24/2009 04:27 PM, Paolo Bonzini wrote:
>>
>> Wow, I wonder how it worked for me...
>>      
> Got it.  The option roms are built in pc-bios/optionrom, but QEMU loads
> the ones in pc-bios.  So actually I was always testing Alexander's code
> even though I had done a "make -B" to feel safer. :-(
>
>    

Hm, I used make && make install to test, and it definitely changed from 
non-working to working.
Anthony Liguori Jan. 7, 2010, 8:03 p.m. UTC | #3
On 12/24/2009 08:27 AM, Paolo Bonzini wrote:
>>> The gdt address calculation in linuxboot.bin is broken in two ways: first
>>> it loads %cs into %eax, but that instruction leaves the high bits of %eax
>>> undefined and we did not clear them. Secondly, we completely ignore the
>>> incorrect %eax, and use the undefined %ebx instead.
>>>
>>> With these issues fixed, linuxboot works again.
>>>        
>> Wow, I wonder how it worked for me...
>>      
> Got it.  The option roms are built in pc-bios/optionrom, but QEMU loads
> the ones in pc-bios.  So actually I was always testing Alexander's code
> even though I had done a "make -B" to feel safer. :-(
>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
>    

This will wreak havoc on the tree since these are built by default and 
they are replacing files in revision control.

Regards,

Anthony Liguori
Paolo Bonzini Jan. 8, 2010, 7:31 a.m. UTC | #4
On 01/07/2010 09:03 PM, Anthony Liguori wrote:
>
> This will wreak havoc on the tree since these are built by default and
> they are replacing files in revision control.

... the next question is why are they (linuxboot.bin and multiboot.bin) 
in revision control since they do not require any strange tool to be built.

Paolo
Aurelien Jarno Jan. 8, 2010, 7:57 a.m. UTC | #5
Paolo Bonzini a écrit :
> On 01/07/2010 09:03 PM, Anthony Liguori wrote:
>> This will wreak havoc on the tree since these are built by default and
>> they are replacing files in revision control.
> 
> ... the next question is why are they (linuxboot.bin and multiboot.bin) 
> in revision control since they do not require any strange tool to be built.
> 

They require an i386 cross compiler, which is not available on all
installations.
Paolo Bonzini Jan. 8, 2010, 8:19 a.m. UTC | #6
On 01/08/2010 08:57 AM, Aurelien Jarno wrote:
> Paolo Bonzini a écrit :
>> On 01/07/2010 09:03 PM, Anthony Liguori wrote:
>>> This will wreak havoc on the tree since these are built by default and
>>> they are replacing files in revision control.
>>
>> ... the next question is why are they (linuxboot.bin and multiboot.bin)
>> in revision control since they do not require any strange tool to be built.
>
> They require an i386 cross compiler, which is not available on all
> installations.

They are being built always anyway now, since their target 
pc-bios/optionrom/linuxboot.bin does not exist on a checked out tree or 
even a release tree.  But no one has complained.

Paolo
Aurelien Jarno Jan. 8, 2010, 9:12 a.m. UTC | #7
Paolo Bonzini a écrit :
> On 01/08/2010 08:57 AM, Aurelien Jarno wrote:
>> Paolo Bonzini a écrit :
>>> On 01/07/2010 09:03 PM, Anthony Liguori wrote:
>>>> This will wreak havoc on the tree since these are built by default and
>>>> they are replacing files in revision control.
>>> ... the next question is why are they (linuxboot.bin and multiboot.bin)
>>> in revision control since they do not require any strange tool to be built.
>> They require an i386 cross compiler, which is not available on all
>> installations.
> 
> They are being built always anyway now, since their target 
> pc-bios/optionrom/linuxboot.bin does not exist on a checked out tree or 
> even a release tree.  But no one has complained.

They are only built on i386 and x86_64 hosts, look at the configure
script. We should continue providing all the ROM binaries, even the x86
ones. We can't require that every installation has a (cross-)compiler
for x86, the same way that we don't require a sparc or powerpc cross
compiler for x86 installations.
Anthony Liguori Jan. 8, 2010, 12:29 p.m. UTC | #8
On 01/08/2010 01:31 AM, Paolo Bonzini wrote:
> On 01/07/2010 09:03 PM, Anthony Liguori wrote:
>>
>> This will wreak havoc on the tree since these are built by default and
>> they are replacing files in revision control.
>
> ... the next question is why are they (linuxboot.bin and 
> multiboot.bin) in revision control since they do not require any 
> strange tool to be built.

If you're just thinking about x86 and kvm, sure, but when you consider a 
default build creates tcg targets for arm, sparc, mips, etc, and that 
they all need firmware, it's extremely unlikely that any given user has 
all of the cross compilers available to build all of the roms.

Yes, we could and should be smarter about building roms when we can and 
making sure we install and use those whenever possible.  We still need 
to ship roms though for the foreseeable future.

Regards,

Anthony Liguori



> Paolo
diff mbox

Patch

diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
index 54db882..bf5a2f3 100644
--- a/pc-bios/optionrom/Makefile
+++ b/pc-bios/optionrom/Makefile
@@ -13,7 +13,7 @@  CFLAGS += -I$(SRC_PATH)
 CFLAGS += $(call cc-option, $(CFLAGS), -fno-stack-protector)
 QEMU_CFLAGS = $(CFLAGS)
 
-build-all: multiboot.bin linuxboot.bin
+build-all: ../multiboot.bin ../linuxboot.bin
 
 %.img: %.o
 	$(call quiet-command,$(LD) -Ttext 0 -e _start -s -o $@ $<,"  Building $(TARGET_DIR)$@")
@@ -21,7 +21,7 @@  build-all: multiboot.bin linuxboot.bin
 %.raw: %.img
 	$(call quiet-command,$(OBJCOPY) -O binary -j .text $< $@,"  Building $(TARGET_DIR)$@")
 
-%.bin: %.raw
+../%.bin: %.raw
 	$(call quiet-command,$(SHELL) $(SRC_PATH)/pc-bios/optionrom/signrom.sh $< $@,"  Signing $(TARGET_DIR)$@")
 
 clean: