* [Qemu-devel] [PATCH] multiboot: fix e801 memory map
2012-11-29 17:11 [Qemu-devel] [PATCH 1.3?] multiboot: fix e801 memory map Paolo Bonzini
@ 2012-11-29 17:11 ` Paolo Bonzini
2012-11-29 18:49 ` [Qemu-devel] [PATCH 1.3?] " Alexander Graf
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Paolo Bonzini @ 2012-11-29 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexander Graf
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>
---
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH 1.3?] multiboot: fix e801 memory map
2012-11-29 17:11 [Qemu-devel] [PATCH 1.3?] multiboot: fix e801 memory map Paolo Bonzini
2012-11-29 17:11 ` [Qemu-devel] [PATCH] " Paolo Bonzini
@ 2012-11-29 18:49 ` Alexander Graf
2012-11-29 18:51 ` Alexander Graf
2012-11-30 16:15 ` Anthony Liguori
3 siblings, 0 replies; 9+ messages in thread
From: Alexander Graf @ 2012-11-29 18:49 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH 1.3?] multiboot: fix e801 memory map
2012-11-29 17:11 [Qemu-devel] [PATCH 1.3?] multiboot: fix e801 memory map Paolo Bonzini
2012-11-29 17:11 ` [Qemu-devel] [PATCH] " Paolo Bonzini
2012-11-29 18:49 ` [Qemu-devel] [PATCH 1.3?] " Alexander Graf
@ 2012-11-29 18:51 ` Alexander Graf
2012-11-29 19:21 ` Paolo Bonzini
2012-11-30 16:15 ` Anthony Liguori
3 siblings, 1 reply; 9+ messages in thread
From: Alexander Graf @ 2012-11-29 18:51 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH 1.3?] multiboot: fix e801 memory map
2012-11-29 18:51 ` Alexander Graf
@ 2012-11-29 19:21 ` Paolo Bonzini
2012-11-29 19:24 ` Alexander Graf
0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2012-11-29 19:21 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-devel
----- 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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH 1.3?] multiboot: fix e801 memory map
2012-11-29 19:21 ` Paolo Bonzini
@ 2012-11-29 19:24 ` Alexander Graf
2012-11-29 19:40 ` Paolo Bonzini
0 siblings, 1 reply; 9+ messages in thread
From: Alexander Graf @ 2012-11-29 19:24 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 1.3?] multiboot: fix e801 memory map
2012-11-29 19:24 ` Alexander Graf
@ 2012-11-29 19:40 ` Paolo Bonzini
2012-11-29 21:12 ` Anthony Liguori
0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2012-11-29 19:40 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-devel
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 1.3?] multiboot: fix e801 memory map
2012-11-29 19:40 ` Paolo Bonzini
@ 2012-11-29 21:12 ` Anthony Liguori
0 siblings, 0 replies; 9+ messages in thread
From: Anthony Liguori @ 2012-11-29 21:12 UTC (permalink / raw)
To: Paolo Bonzini, Alexander Graf; +Cc: qemu-devel
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 1.3?] multiboot: fix e801 memory map
2012-11-29 17:11 [Qemu-devel] [PATCH 1.3?] multiboot: fix e801 memory map Paolo Bonzini
` (2 preceding siblings ...)
2012-11-29 18:51 ` Alexander Graf
@ 2012-11-30 16:15 ` Anthony Liguori
3 siblings, 0 replies; 9+ messages in thread
From: Anthony Liguori @ 2012-11-30 16:15 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: Alexander Graf
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
^ permalink raw reply [flat|nested] 9+ messages in thread