| Message ID | 1354209111-18110-1-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 29.11.2012, at 18:11, Paolo Bonzini wrote: > The e801 memory sizes in the multiboot structures hard-code the available > low memory to 640. However, the value should not include the size of the > EBDA. Fill the value in the option ROM, getting the size of low memory > from the BIOS. > > Cc: Alexander Graf <agraf@suse.de> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > Alex, can you ack this patch for 1.3? Do you have a test case (OS that fails for example)? Alex > > pc-bios/multiboot.bin | Bin 1024 -> 1024 bytes > pc-bios/optionrom/multiboot.S | 7 +++++++ > 2 files changed, 7 insertions(+) > > diff --git a/pc-bios/multiboot.bin b/pc-bios/multiboot.bin > index f74a6e142fddc054d7f40ab346a108532afac40f..7b3c1745a430ea5e0e15b9aa817d1cbbaa40db14 100644 > GIT binary patch > delta 81 > zcmZqRXyBNj#q7o8KT+38L4+xd@o;KdryK*rZVrajPB|8aZaEhwAcKt|ty|7*VtW(k > kS)sHUDQSyY7$$Qv{%2ueV91@!$z;eVv)P(y2P2~%0BkrF9smFU > > delta 72 > zcmZqRXyBNj#capqJW<z5frBZH@o;KdryK*rZVrajPB|8aZaE7kAcKt|ty|7&VtdnM > bA;$lVDU(H+3>gJByEE-zoLs=f!*~_|qXQE} > > diff --git a/pc-bios/optionrom/multiboot.S b/pc-bios/optionrom/multiboot.S > index f08222a..003bcfb 100644 > --- a/pc-bios/optionrom/multiboot.S > +++ b/pc-bios/optionrom/multiboot.S > @@ -75,6 +75,13 @@ run_multiboot: > shr $4, %eax > mov %ax, %fs > > + /* Account for the EBDA in the multiboot structure's e801 > + * map. > + */ > + int $0x12 > + cwtl > + movl %eax, %fs:4 > + > /* ES = mmap_addr */ > mov %fs:48, %eax > shr $4, %eax > -- > 1.8.0 >
On 29.11.2012, at 18:11, Paolo Bonzini wrote: > The e801 memory sizes in the multiboot structures hard-code the available > low memory to 640. However, the value should not include the size of the > EBDA. Fill the value in the option ROM, getting the size of low memory > from the BIOS. The description mentions that instead of hard coding, you fetch the value dynamically via a BIOS call. However, I don't see the code that hard codes it changed. Where is that one? Alex > > Cc: Alexander Graf <agraf@suse.de> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > Alex, can you ack this patch for 1.3? > > pc-bios/multiboot.bin | Bin 1024 -> 1024 bytes > pc-bios/optionrom/multiboot.S | 7 +++++++ > 2 files changed, 7 insertions(+) > > diff --git a/pc-bios/multiboot.bin b/pc-bios/multiboot.bin > index f74a6e142fddc054d7f40ab346a108532afac40f..7b3c1745a430ea5e0e15b9aa817d1cbbaa40db14 100644 > GIT binary patch > delta 81 > zcmZqRXyBNj#q7o8KT+38L4+xd@o;KdryK*rZVrajPB|8aZaEhwAcKt|ty|7*VtW(k > kS)sHUDQSyY7$$Qv{%2ueV91@!$z;eVv)P(y2P2~%0BkrF9smFU > > delta 72 > zcmZqRXyBNj#capqJW<z5frBZH@o;KdryK*rZVrajPB|8aZaE7kAcKt|ty|7&VtdnM > bA;$lVDU(H+3>gJByEE-zoLs=f!*~_|qXQE} > > diff --git a/pc-bios/optionrom/multiboot.S b/pc-bios/optionrom/multiboot.S > index f08222a..003bcfb 100644 > --- a/pc-bios/optionrom/multiboot.S > +++ b/pc-bios/optionrom/multiboot.S > @@ -75,6 +75,13 @@ run_multiboot: > shr $4, %eax > mov %ax, %fs > > + /* Account for the EBDA in the multiboot structure's e801 > + * map. > + */ > + int $0x12 > + cwtl > + movl %eax, %fs:4 > + > /* ES = mmap_addr */ > mov %fs:48, %eax > shr $4, %eax > -- > 1.8.0 >
----- Messaggio originale ----- > Da: "Alexander Graf" <agraf@suse.de> > A: "Paolo Bonzini" <pbonzini@redhat.com> > Cc: qemu-devel@nongnu.org > Inviato: Giovedì, 29 novembre 2012 19:51:07 > Oggetto: Re: [PATCH 1.3?] multiboot: fix e801 memory map > > > On 29.11.2012, at 18:11, Paolo Bonzini wrote: > > > The e801 memory sizes in the multiboot structures hard-code the available > > low memory to 640. However, the value should not include the size > > of the EBDA. Fill the value in the option ROM, getting the size of low > > memory from the BIOS. > > The description mentions that instead of hard coding, you fetch the > value dynamically via a BIOS call. However, I don't see the code > that hard codes it changed. Where is that one? It is in hw/multiboot.c: stl_p(bootinfo + MBI_MEM_LOWER, 640); Regarding the testcase, Xen will touch the EBDA without this patch and with http://permalink.gmane.org/gmane.comp.emulators.xen.devel/144855. However, SeaBIOS does not complain. Paolo
On 29.11.2012, at 20:21, Paolo Bonzini wrote: > > > ----- Messaggio originale ----- >> Da: "Alexander Graf" <agraf@suse.de> >> A: "Paolo Bonzini" <pbonzini@redhat.com> >> Cc: qemu-devel@nongnu.org >> Inviato: Giovedì, 29 novembre 2012 19:51:07 >> Oggetto: Re: [PATCH 1.3?] multiboot: fix e801 memory map >> >> >> On 29.11.2012, at 18:11, Paolo Bonzini wrote: >> >>> The e801 memory sizes in the multiboot structures hard-code the available >>> low memory to 640. However, the value should not include the size >>> of the EBDA. Fill the value in the option ROM, getting the size of low >>> memory from the BIOS. >> >> The description mentions that instead of hard coding, you fetch the >> value dynamically via a BIOS call. However, I don't see the code >> that hard codes it changed. Where is that one? > > It is in hw/multiboot.c: > > stl_p(bootinfo + MBI_MEM_LOWER, 640); You want to remove that one then. > Regarding the testcase, Xen will touch the EBDA without this patch and > with http://permalink.gmane.org/gmane.comp.emulators.xen.devel/144855. > However, SeaBIOS does not complain. I see :). Well, the patch is unintrusive enough for be ok for 1.3 IMHO. Alex
Il 29/11/2012 20:24, Alexander Graf ha scritto: >> > It is in hw/multiboot.c: >> > >> > stl_p(bootinfo + MBI_MEM_LOWER, 640); > You want to remove that one then. I wasn't sure of what happens if the multiboot option ROM is old. Do we support that to any extent? > > Regarding the testcase, Xen will touch the EBDA without this patch and > > with http://permalink.gmane.org/gmane.comp.emulators.xen.devel/144855. > > However, SeaBIOS does not complain. > > I see :). Well, the patch is unintrusive enough for be ok for 1.3 IMHO. Actually it's relatively easy to see a failure with Xen and a need to PCI passthrough something with a bulky boot ROM; iSCSI or FC cards will do. You'll need both the Xen patch above, and this patch to boot successfully. Otherwise, it will fail to boot. Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > Il 29/11/2012 20:24, Alexander Graf ha scritto: >>> > It is in hw/multiboot.c: >>> > >>> > stl_p(bootinfo + MBI_MEM_LOWER, 640); >> You want to remove that one then. > > I wasn't sure of what happens if the multiboot option ROM is old. Do we > support that to any extent? Option ROMs are migrated but reset after reboot. I guess you could migrate before the option ROM is executed but that seems extraordinarily unlikely. Regards, Anthony Liguori > >> > Regarding the testcase, Xen will touch the EBDA without this patch and >> > with http://permalink.gmane.org/gmane.comp.emulators.xen.devel/144855. >> > However, SeaBIOS does not complain. >> >> I see :). Well, the patch is unintrusive enough for be ok for 1.3 IMHO. > > Actually it's relatively easy to see a failure with Xen and a need to > PCI passthrough something with a bulky boot ROM; iSCSI or FC cards will > do. You'll need both the Xen patch above, and this patch to boot > successfully. Otherwise, it will fail to boot. > > Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > The e801 memory sizes in the multiboot structures hard-code the available > low memory to 640. However, the value should not include the size of the > EBDA. Fill the value in the option ROM, getting the size of low memory > from the BIOS. > > Cc: Alexander Graf <agraf@suse.de> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > Alex, can you ack this patch for 1.3? Applied. Thanks. Didn't see a clear ack from Alex but since this does fix a bug, I figured we should carry it for 1.3. Regards, Anthony Liguori > > pc-bios/multiboot.bin | Bin 1024 -> 1024 bytes > pc-bios/optionrom/multiboot.S | 7 +++++++ > 2 files changed, 7 insertions(+) > > diff --git a/pc-bios/multiboot.bin b/pc-bios/multiboot.bin > index f74a6e142fddc054d7f40ab346a108532afac40f..7b3c1745a430ea5e0e15b9aa817d1cbbaa40db14 100644 > GIT binary patch > delta 81 > zcmZqRXyBNj#q7o8KT+38L4+xd@o;KdryK*rZVrajPB|8aZaEhwAcKt|ty|7*VtW(k > kS)sHUDQSyY7$$Qv{%2ueV91@!$z;eVv)P(y2P2~%0BkrF9smFU > > delta 72 > zcmZqRXyBNj#capqJW<z5frBZH@o;KdryK*rZVrajPB|8aZaE7kAcKt|ty|7&VtdnM > bA;$lVDU(H+3>gJByEE-zoLs=f!*~_|qXQE} > > diff --git a/pc-bios/optionrom/multiboot.S b/pc-bios/optionrom/multiboot.S > index f08222a..003bcfb 100644 > --- a/pc-bios/optionrom/multiboot.S > +++ b/pc-bios/optionrom/multiboot.S > @@ -75,6 +75,13 @@ run_multiboot: > shr $4, %eax > mov %ax, %fs > > + /* Account for the EBDA in the multiboot structure's e801 > + * map. > + */ > + int $0x12 > + cwtl > + movl %eax, %fs:4 > + > /* ES = mmap_addr */ > mov %fs:48, %eax > shr $4, %eax > -- > 1.8.0
diff --git a/pc-bios/multiboot.bin b/pc-bios/multiboot.bin index f74a6e142fddc054d7f40ab346a108532afac40f..7b3c1745a430ea5e0e15b9aa817d1cbbaa40db14 100644 GIT binary patch delta 81 zcmZqRXyBNj#q7o8KT+38L4+xd@o;KdryK*rZVrajPB|8aZaEhwAcKt|ty|7*VtW(k kS)sHUDQSyY7$$Qv{%2ueV91@!$z;eVv)P(y2P2~%0BkrF9smFU delta 72 zcmZqRXyBNj#capqJW<z5frBZH@o;KdryK*rZVrajPB|8aZaE7kAcKt|ty|7&VtdnM bA;$lVDU(H+3>gJByEE-zoLs=f!*~_|qXQE} diff --git a/pc-bios/optionrom/multiboot.S b/pc-bios/optionrom/multiboot.S index f08222a..003bcfb 100644 --- a/pc-bios/optionrom/multiboot.S +++ b/pc-bios/optionrom/multiboot.S @@ -75,6 +75,13 @@ run_multiboot: shr $4, %eax mov %ax, %fs + /* Account for the EBDA in the multiboot structure's e801 + * map. + */ + int $0x12 + cwtl + movl %eax, %fs:4 + /* ES = mmap_addr */ mov %fs:48, %eax shr $4, %eax
The e801 memory sizes in the multiboot structures hard-code the available low memory to 640. However, the value should not include the size of the EBDA. Fill the value in the option ROM, getting the size of low memory from the BIOS. Cc: Alexander Graf <agraf@suse.de> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- Alex, can you ack this patch for 1.3? pc-bios/multiboot.bin | Bin 1024 -> 1024 bytes pc-bios/optionrom/multiboot.S | 7 +++++++ 2 files changed, 7 insertions(+)