Is there any limitation on the firmware size in Xen?

Is there any limitation on the firmware size in Xen?

[Charles Arnold]

I was asked the following question which I am posting to the list.

"
My name is Gary Lin, and I am the maintainer of the OVMF package, a UEFI
implementation for the virtual machines.

Recently, I was testing an upstream patchset[*] and encountered some
problems in Xen. Maybe you can help me or give me some hints.

To be short, the edk2/ovmf upstream is going to increase the firmware
size from 2MB to 4MB to fulfill windows HCK, and we have to test
different types of VM to make sure the patchset really work. When I was
using the 2MB build, my Xen HVM worked as expected and showed the boot
menu. However, if I use the 4MB build, I got something like this from
"xl dmesg":

(d32)  - CPU0 ... 39-bit phys ... fixed MTRRs ... var MTRRs [1/8] ... done.
(d32) Testing HVM environment:
(d32)  - REP INSB across page boundaries ... passed
(d32)  - GS base MSRs and SWAPGS ... passed
(d32) Passed 2 of 2 tests
(d32) Writing SMBIOS tables ...
(d32) Loading OVMF ...
(d32) no BIOS ROM image found
(d32) *** HVMLoader bug at hvmloader.c:389
(d32) *** HVMLoader crashed.

I tried to trace the code and found that in libxl__load_hvm_firmware_module()
in tools/libxl/libxl_dom.c actully loaded the file and add_module_to_list()
in tools/libxc/xc_dom_x86.c was loading a firmware "module" with 4194304
bytes. However, when hvmloader loaded "bios_module" with get_module_entry(),
modlist is NULL. It seems the firmware data was removed for some reason.

Here are my questions:

1. Is there any limitation on the firmware size in Xen?

2. How is hvm_start_info passed to hvmloader?

Thanks,

Gary Lin

[*] https://github.com/lersek/edk2.git branch emu4k
"

- Charles



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Is there any limitation on the firmware size in Xen?

[Jan Beulich]

>>> On 10.05.17 at 16:29, <CARNOLD@suse.com> wrote:
> I was asked the following question which I am posting to the list.
> 
> "
> My name is Gary Lin, and I am the maintainer of the OVMF package, a UEFI
> implementation for the virtual machines.
> 
> Recently, I was testing an upstream patchset[*] and encountered some
> problems in Xen. Maybe you can help me or give me some hints.
> 
> To be short, the edk2/ovmf upstream is going to increase the firmware
> size from 2MB to 4MB to fulfill windows HCK, and we have to test
> different types of VM to make sure the patchset really work. When I was
> using the 2MB build, my Xen HVM worked as expected and showed the boot
> menu. However, if I use the 4MB build, I got something like this from
> "xl dmesg":
> 
> (d32)  - CPU0 ... 39-bit phys ... fixed MTRRs ... var MTRRs [1/8] ... done.
> (d32) Testing HVM environment:
> (d32)  - REP INSB across page boundaries ... passed
> (d32)  - GS base MSRs and SWAPGS ... passed
> (d32) Passed 2 of 2 tests
> (d32) Writing SMBIOS tables ...
> (d32) Loading OVMF ...
> (d32) no BIOS ROM image found
> (d32) *** HVMLoader bug at hvmloader.c:389
> (d32) *** HVMLoader crashed.
> 
> I tried to trace the code and found that in 
> libxl__load_hvm_firmware_module()
> in tools/libxl/libxl_dom.c actully loaded the file and add_module_to_list()
> in tools/libxc/xc_dom_x86.c was loading a firmware "module" with 4194304
> bytes. However, when hvmloader loaded "bios_module" with get_module_entry(),
> modlist is NULL. It seems the firmware data was removed for some reason.
> 
> Here are my questions:
> 
> 1. Is there any limitation on the firmware size in Xen?

I can't recall an explicit limit on the hvmloader side, but there are a
number of other restrictions. I'd suggest adding a few more printf()s
in relevant hvmloader functions (like get_module_entry()) to learn
whether the image doesn't get passed, or whether instead hvmloader
doesn't like where it sits (or whether something else has been
corrupted).

> 2. How is hvm_start_info passed to hvmloader?

A pointer to it is in register EBX. But maybe that's no what you really
meant o ask?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Is there any limitation on the firmware size in Xen?

[Wei Liu]

On Wed, May 10, 2017 at 08:29:35AM -0600, Charles Arnold wrote:
> I was asked the following question which I am posting to the list.
> 
> "
> My name is Gary Lin, and I am the maintainer of the OVMF package, a UEFI
> implementation for the virtual machines.
> 
> Recently, I was testing an upstream patchset[*] and encountered some
> problems in Xen. Maybe you can help me or give me some hints.
> 
> To be short, the edk2/ovmf upstream is going to increase the firmware
> size from 2MB to 4MB to fulfill windows HCK, and we have to test
> different types of VM to make sure the patchset really work. When I was
> using the 2MB build, my Xen HVM worked as expected and showed the boot
> menu. However, if I use the 4MB build, I got something like this from
> "xl dmesg":
> 
> (d32)  - CPU0 ... 39-bit phys ... fixed MTRRs ... var MTRRs [1/8] ... done.
> (d32) Testing HVM environment:
> (d32)  - REP INSB across page boundaries ... passed
> (d32)  - GS base MSRs and SWAPGS ... passed
> (d32) Passed 2 of 2 tests
> (d32) Writing SMBIOS tables ...
> (d32) Loading OVMF ...
> (d32) no BIOS ROM image found
> (d32) *** HVMLoader bug at hvmloader.c:389
> (d32) *** HVMLoader crashed.
> 
> I tried to trace the code and found that in libxl__load_hvm_firmware_module()
> in tools/libxl/libxl_dom.c actully loaded the file and add_module_to_list()
> in tools/libxc/xc_dom_x86.c was loading a firmware "module" with 4194304
> bytes. However, when hvmloader loaded "bios_module" with get_module_entry(),
> modlist is NULL. It seems the firmware data was removed for some reason.
> 
> Here are my questions:
> 
> 1. Is there any limitation on the firmware size in Xen?
> 

OVMF is loaded into 4GB - ovmf_size. There shouldn't be limitation in
that regard. HVMloader should be happy with that address range.

Like Jan suggested, it would be worth investigating if it is corrupted
by something else.


Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Is there any limitation on the firmware size in Xen?

[Wei Liu]

On Wed, May 10, 2017 at 04:39:32PM +0100, Wei Liu wrote:
> On Wed, May 10, 2017 at 08:29:35AM -0600, Charles Arnold wrote:
> > I was asked the following question which I am posting to the list.
> > 
> > "
> > My name is Gary Lin, and I am the maintainer of the OVMF package, a UEFI
> > implementation for the virtual machines.
> > 
> > Recently, I was testing an upstream patchset[*] and encountered some
> > problems in Xen. Maybe you can help me or give me some hints.
> > 
> > To be short, the edk2/ovmf upstream is going to increase the firmware
> > size from 2MB to 4MB to fulfill windows HCK, and we have to test
> > different types of VM to make sure the patchset really work. When I was
> > using the 2MB build, my Xen HVM worked as expected and showed the boot
> > menu. However, if I use the 4MB build, I got something like this from
> > "xl dmesg":
> > 
> > (d32)  - CPU0 ... 39-bit phys ... fixed MTRRs ... var MTRRs [1/8] ... done.
> > (d32) Testing HVM environment:
> > (d32)  - REP INSB across page boundaries ... passed
> > (d32)  - GS base MSRs and SWAPGS ... passed
> > (d32) Passed 2 of 2 tests
> > (d32) Writing SMBIOS tables ...
> > (d32) Loading OVMF ...
> > (d32) no BIOS ROM image found
> > (d32) *** HVMLoader bug at hvmloader.c:389
> > (d32) *** HVMLoader crashed.
> > 
> > I tried to trace the code and found that in libxl__load_hvm_firmware_module()
> > in tools/libxl/libxl_dom.c actully loaded the file and add_module_to_list()
> > in tools/libxc/xc_dom_x86.c was loading a firmware "module" with 4194304
> > bytes. However, when hvmloader loaded "bios_module" with get_module_entry(),
> > modlist is NULL. It seems the firmware data was removed for some reason.
> > 
> > Here are my questions:
> > 
> > 1. Is there any limitation on the firmware size in Xen?
> > 
> 
> OVMF is loaded into 4GB - ovmf_size. There shouldn't be limitation in
> that regard. HVMloader should be happy with that address range.

Oh wait, it hasn't gone that far into loading OVMF.

IT would be useful, as a starting point, to go through modlist, print
out and compare module loading addresses and lengths from both libxc and
hvmloader.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Is there any limitation on the firmware size in Xen?

[Gary Lin]

On Thu, May 11, 2017 at 01:23:47AM +1000, Jan Beulich wrote:
> >>> On 10.05.17 at 16:29, <CARNOLD@suse.com> wrote:
> > I was asked the following question which I am posting to the list.
> > 
> > "
> > My name is Gary Lin, and I am the maintainer of the OVMF package, a UEFI
> > implementation for the virtual machines.
> > 
> > Recently, I was testing an upstream patchset[*] and encountered some
> > problems in Xen. Maybe you can help me or give me some hints.
> > 
> > To be short, the edk2/ovmf upstream is going to increase the firmware
> > size from 2MB to 4MB to fulfill windows HCK, and we have to test
> > different types of VM to make sure the patchset really work. When I was
> > using the 2MB build, my Xen HVM worked as expected and showed the boot
> > menu. However, if I use the 4MB build, I got something like this from
> > "xl dmesg":
> > 
> > (d32)  - CPU0 ... 39-bit phys ... fixed MTRRs ... var MTRRs [1/8] ... done.
> > (d32) Testing HVM environment:
> > (d32)  - REP INSB across page boundaries ... passed
> > (d32)  - GS base MSRs and SWAPGS ... passed
> > (d32) Passed 2 of 2 tests
> > (d32) Writing SMBIOS tables ...
> > (d32) Loading OVMF ...
> > (d32) no BIOS ROM image found
> > (d32) *** HVMLoader bug at hvmloader.c:389
> > (d32) *** HVMLoader crashed.
> > 
> > I tried to trace the code and found that in 
> > libxl__load_hvm_firmware_module()
> > in tools/libxl/libxl_dom.c actully loaded the file and add_module_to_list()
> > in tools/libxc/xc_dom_x86.c was loading a firmware "module" with 4194304
> > bytes. However, when hvmloader loaded "bios_module" with get_module_entry(),
> > modlist is NULL. It seems the firmware data was removed for some reason.
> > 
> > Here are my questions:
> > 
> > 1. Is there any limitation on the firmware size in Xen?
> 
> I can't recall an explicit limit on the hvmloader side, but there are a
> number of other restrictions. I'd suggest adding a few more printf()s
> in relevant hvmloader functions (like get_module_entry()) to learn
> whether the image doesn't get passed, or whether instead hvmloader
> doesn't like where it sits (or whether something else has been
> corrupted).
I added a check in the beginning of get_module_entry() like this:

 if (!modlist) {
     printf("null modlist\n");
 }

then I got "null modlist" right after "xl create -c ovmf.cfg". That's
why I want to know how hvm_start_info is passed to hvmloader.

> 
> > 2. How is hvm_start_info passed to hvmloader?
> 
> A pointer to it is in register EBX. But maybe that's no what you really
> meant o ask?
> 
Ya, I found "mov  %ebx, hvm_start_info\n" in the assembly code, but I
don't know how EBX was set. (from an external program?)

Thanks,

Gary Lin

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Is there any limitation on the firmware size in Xen?

[Gary Lin]

On Wed, May 10, 2017 at 04:47:49PM +0100, Wei Liu wrote:
> On Wed, May 10, 2017 at 04:39:32PM +0100, Wei Liu wrote:
> > On Wed, May 10, 2017 at 08:29:35AM -0600, Charles Arnold wrote:
> > > I was asked the following question which I am posting to the list.
> > > 
> > > "
> > > My name is Gary Lin, and I am the maintainer of the OVMF package, a UEFI
> > > implementation for the virtual machines.
> > > 
> > > Recently, I was testing an upstream patchset[*] and encountered some
> > > problems in Xen. Maybe you can help me or give me some hints.
> > > 
> > > To be short, the edk2/ovmf upstream is going to increase the firmware
> > > size from 2MB to 4MB to fulfill windows HCK, and we have to test
> > > different types of VM to make sure the patchset really work. When I was
> > > using the 2MB build, my Xen HVM worked as expected and showed the boot
> > > menu. However, if I use the 4MB build, I got something like this from
> > > "xl dmesg":
> > > 
> > > (d32)  - CPU0 ... 39-bit phys ... fixed MTRRs ... var MTRRs [1/8] ... done.
> > > (d32) Testing HVM environment:
> > > (d32)  - REP INSB across page boundaries ... passed
> > > (d32)  - GS base MSRs and SWAPGS ... passed
> > > (d32) Passed 2 of 2 tests
> > > (d32) Writing SMBIOS tables ...
> > > (d32) Loading OVMF ...
> > > (d32) no BIOS ROM image found
> > > (d32) *** HVMLoader bug at hvmloader.c:389
> > > (d32) *** HVMLoader crashed.
> > > 
> > > I tried to trace the code and found that in libxl__load_hvm_firmware_module()
> > > in tools/libxl/libxl_dom.c actully loaded the file and add_module_to_list()
> > > in tools/libxc/xc_dom_x86.c was loading a firmware "module" with 4194304
> > > bytes. However, when hvmloader loaded "bios_module" with get_module_entry(),
> > > modlist is NULL. It seems the firmware data was removed for some reason.
> > > 
> > > Here are my questions:
> > > 
> > > 1. Is there any limitation on the firmware size in Xen?
> > > 
> > 
> > OVMF is loaded into 4GB - ovmf_size. There shouldn't be limitation in
> > that regard. HVMloader should be happy with that address range.
> 
> Oh wait, it hasn't gone that far into loading OVMF.
> 
> IT would be useful, as a starting point, to go through modlist, print
> out and compare module loading addresses and lengths from both libxc and
> hvmloader.
> 

I printed the address and contents of hvm_start_info in get_module_entry().
Here is the result.

4MB build:

(d39) get_module_entry info 0x588000
(d39) info->magic         0x0
(d39) info->version       0x0
(d39) info->flags         0x0
(d39) info->nr_modules    0x0
(d39) info->modlist_paddr 0x0
(d39) info->cmdline_paddr 0x0
(d39) info->rsdp_paddr    0x0

Obviously, it's corrupted since magic is 0.

2MB build:

(d40) get_module_entry info 0x388000
(d40) info->magic         0x336ec578
(d40) info->version       0x0
(d40) info->flags         0x0
(d40) info->nr_modules    0x1
(d40) info->modlist_paddr 0x388028
(d40) info->cmdline_paddr 0x0
(d40) info->rsdp_paddr    0x0

This looks alright and grub2 menu showed.

It seems the starting address increased 2MB, and I wonder if this
matters.

Thanks,

Gary Lin

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Is there any limitation on the firmware size in Xen?

[Gary Lin]

On Thu, May 11, 2017 at 12:10:56PM +0800, Gary Lin wrote:
> On Wed, May 10, 2017 at 04:47:49PM +0100, Wei Liu wrote:
> > On Wed, May 10, 2017 at 04:39:32PM +0100, Wei Liu wrote:
> > > On Wed, May 10, 2017 at 08:29:35AM -0600, Charles Arnold wrote:
> > > > I was asked the following question which I am posting to the list.
> > > > 
> > > > "
> > > > My name is Gary Lin, and I am the maintainer of the OVMF package, a UEFI
> > > > implementation for the virtual machines.
> > > > 
> > > > Recently, I was testing an upstream patchset[*] and encountered some
> > > > problems in Xen. Maybe you can help me or give me some hints.
> > > > 
> > > > To be short, the edk2/ovmf upstream is going to increase the firmware
> > > > size from 2MB to 4MB to fulfill windows HCK, and we have to test
> > > > different types of VM to make sure the patchset really work. When I was
> > > > using the 2MB build, my Xen HVM worked as expected and showed the boot
> > > > menu. However, if I use the 4MB build, I got something like this from
> > > > "xl dmesg":
> > > > 
> > > > (d32)  - CPU0 ... 39-bit phys ... fixed MTRRs ... var MTRRs [1/8] ... done.
> > > > (d32) Testing HVM environment:
> > > > (d32)  - REP INSB across page boundaries ... passed
> > > > (d32)  - GS base MSRs and SWAPGS ... passed
> > > > (d32) Passed 2 of 2 tests
> > > > (d32) Writing SMBIOS tables ...
> > > > (d32) Loading OVMF ...
> > > > (d32) no BIOS ROM image found
> > > > (d32) *** HVMLoader bug at hvmloader.c:389
> > > > (d32) *** HVMLoader crashed.
> > > > 
> > > > I tried to trace the code and found that in libxl__load_hvm_firmware_module()
> > > > in tools/libxl/libxl_dom.c actully loaded the file and add_module_to_list()
> > > > in tools/libxc/xc_dom_x86.c was loading a firmware "module" with 4194304
> > > > bytes. However, when hvmloader loaded "bios_module" with get_module_entry(),
> > > > modlist is NULL. It seems the firmware data was removed for some reason.
> > > > 
> > > > Here are my questions:
> > > > 
> > > > 1. Is there any limitation on the firmware size in Xen?
> > > > 
> > > 
> > > OVMF is loaded into 4GB - ovmf_size. There shouldn't be limitation in
> > > that regard. HVMloader should be happy with that address range.
> > 
> > Oh wait, it hasn't gone that far into loading OVMF.
> > 
> > IT would be useful, as a starting point, to go through modlist, print
> > out and compare module loading addresses and lengths from both libxc and
> > hvmloader.
> > 
> 
> I printed the address and contents of hvm_start_info in get_module_entry().
> Here is the result.
> 
> 4MB build:
> 
> (d39) get_module_entry info 0x588000
> (d39) info->magic         0x0
> (d39) info->version       0x0
> (d39) info->flags         0x0
> (d39) info->nr_modules    0x0
> (d39) info->modlist_paddr 0x0
> (d39) info->cmdline_paddr 0x0
> (d39) info->rsdp_paddr    0x0
> 
> Obviously, it's corrupted since magic is 0.
> 

In the end of bootlate_hvm() (before munmap(start_info, start_info_size)):

bootlate_hvm start_info->magic 336ec578
bootlate_hvm start_info->nr_modules 1
bootlate_hvm start_info->modlist_paddr 0x588028

So until bootlate_hvm(), it seems alright.

Cheers,

Gary Lin

> 2MB build:
> 
> (d40) get_module_entry info 0x388000
> (d40) info->magic         0x336ec578
> (d40) info->version       0x0
> (d40) info->flags         0x0
> (d40) info->nr_modules    0x1
> (d40) info->modlist_paddr 0x388028
> (d40) info->cmdline_paddr 0x0
> (d40) info->rsdp_paddr    0x0
> 
> This looks alright and grub2 menu showed.
> 
> It seems the starting address increased 2MB, and I wonder if this
> matters.
> 
> Thanks,
> 
> Gary Lin

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Is there any limitation on the firmware size in Xen?

[Jan Beulich]

>>> On 11.05.17 at 06:10, <GLin@suse.com> wrote:
> On Wed, May 10, 2017 at 04:47:49PM +0100, Wei Liu wrote:
>> On Wed, May 10, 2017 at 04:39:32PM +0100, Wei Liu wrote:
>> > On Wed, May 10, 2017 at 08:29:35AM -0600, Charles Arnold wrote:
>> > > I was asked the following question which I am posting to the list.
>> > > 
>> > > "
>> > > My name is Gary Lin, and I am the maintainer of the OVMF package, a UEFI
>> > > implementation for the virtual machines.
>> > > 
>> > > Recently, I was testing an upstream patchset[*] and encountered some
>> > > problems in Xen. Maybe you can help me or give me some hints.
>> > > 
>> > > To be short, the edk2/ovmf upstream is going to increase the firmware
>> > > size from 2MB to 4MB to fulfill windows HCK, and we have to test
>> > > different types of VM to make sure the patchset really work. When I was
>> > > using the 2MB build, my Xen HVM worked as expected and showed the boot
>> > > menu. However, if I use the 4MB build, I got something like this from
>> > > "xl dmesg":
>> > > 
>> > > (d32)  - CPU0 ... 39-bit phys ... fixed MTRRs ... var MTRRs [1/8] ... 
> done.
>> > > (d32) Testing HVM environment:
>> > > (d32)  - REP INSB across page boundaries ... passed
>> > > (d32)  - GS base MSRs and SWAPGS ... passed
>> > > (d32) Passed 2 of 2 tests
>> > > (d32) Writing SMBIOS tables ...
>> > > (d32) Loading OVMF ...
>> > > (d32) no BIOS ROM image found
>> > > (d32) *** HVMLoader bug at hvmloader.c:389
>> > > (d32) *** HVMLoader crashed.
>> > > 
>> > > I tried to trace the code and found that in 
> libxl__load_hvm_firmware_module()
>> > > in tools/libxl/libxl_dom.c actully loaded the file and 
> add_module_to_list()
>> > > in tools/libxc/xc_dom_x86.c was loading a firmware "module" with 4194304
>> > > bytes. However, when hvmloader loaded "bios_module" with 
> get_module_entry(),
>> > > modlist is NULL. It seems the firmware data was removed for some reason.
>> > > 
>> > > Here are my questions:
>> > > 
>> > > 1. Is there any limitation on the firmware size in Xen?
>> > > 
>> > 
>> > OVMF is loaded into 4GB - ovmf_size. There shouldn't be limitation in
>> > that regard. HVMloader should be happy with that address range.
>> 
>> Oh wait, it hasn't gone that far into loading OVMF.
>> 
>> IT would be useful, as a starting point, to go through modlist, print
>> out and compare module loading addresses and lengths from both libxc and
>> hvmloader.
>> 
> 
> I printed the address and contents of hvm_start_info in get_module_entry().
> Here is the result.
> 
> 4MB build:
> 
> (d39) get_module_entry info 0x588000
> (d39) info->magic         0x0
> (d39) info->version       0x0
> (d39) info->flags         0x0
> (d39) info->nr_modules    0x0
> (d39) info->modlist_paddr 0x0
> (d39) info->cmdline_paddr 0x0
> (d39) info->rsdp_paddr    0x0
> 
> Obviously, it's corrupted since magic is 0.

Note that hvmloader's main() has

    BUG_ON(hvm_start_info->magic != XEN_HVM_START_MAGIC_VALUE);

very early, so you having got past this means the corruption
occurred inside hvmloader (or at least while it was already
running). Could you comment out the call to perform_tests()
and try again?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Is there any limitation on the firmware size in Xen?

[Gary Lin]

On Thu, May 11, 2017 at 06:14:42PM +1000, Jan Beulich wrote:
> >>> On 11.05.17 at 06:10, <GLin@suse.com> wrote:
> > On Wed, May 10, 2017 at 04:47:49PM +0100, Wei Liu wrote:
> >> On Wed, May 10, 2017 at 04:39:32PM +0100, Wei Liu wrote:
> >> > On Wed, May 10, 2017 at 08:29:35AM -0600, Charles Arnold wrote:
> >> > > I was asked the following question which I am posting to the list.
> >> > > 
> >> > > "
> >> > > My name is Gary Lin, and I am the maintainer of the OVMF package, a UEFI
> >> > > implementation for the virtual machines.
> >> > > 
> >> > > Recently, I was testing an upstream patchset[*] and encountered some
> >> > > problems in Xen. Maybe you can help me or give me some hints.
> >> > > 
> >> > > To be short, the edk2/ovmf upstream is going to increase the firmware
> >> > > size from 2MB to 4MB to fulfill windows HCK, and we have to test
> >> > > different types of VM to make sure the patchset really work. When I was
> >> > > using the 2MB build, my Xen HVM worked as expected and showed the boot
> >> > > menu. However, if I use the 4MB build, I got something like this from
> >> > > "xl dmesg":
> >> > > 
> >> > > (d32)  - CPU0 ... 39-bit phys ... fixed MTRRs ... var MTRRs [1/8] ... 
> > done.
> >> > > (d32) Testing HVM environment:
> >> > > (d32)  - REP INSB across page boundaries ... passed
> >> > > (d32)  - GS base MSRs and SWAPGS ... passed
> >> > > (d32) Passed 2 of 2 tests
> >> > > (d32) Writing SMBIOS tables ...
> >> > > (d32) Loading OVMF ...
> >> > > (d32) no BIOS ROM image found
> >> > > (d32) *** HVMLoader bug at hvmloader.c:389
> >> > > (d32) *** HVMLoader crashed.
> >> > > 
> >> > > I tried to trace the code and found that in 
> > libxl__load_hvm_firmware_module()
> >> > > in tools/libxl/libxl_dom.c actully loaded the file and 
> > add_module_to_list()
> >> > > in tools/libxc/xc_dom_x86.c was loading a firmware "module" with 4194304
> >> > > bytes. However, when hvmloader loaded "bios_module" with 
> > get_module_entry(),
> >> > > modlist is NULL. It seems the firmware data was removed for some reason.
> >> > > 
> >> > > Here are my questions:
> >> > > 
> >> > > 1. Is there any limitation on the firmware size in Xen?
> >> > > 
> >> > 
> >> > OVMF is loaded into 4GB - ovmf_size. There shouldn't be limitation in
> >> > that regard. HVMloader should be happy with that address range.
> >> 
> >> Oh wait, it hasn't gone that far into loading OVMF.
> >> 
> >> IT would be useful, as a starting point, to go through modlist, print
> >> out and compare module loading addresses and lengths from both libxc and
> >> hvmloader.
> >> 
> > 
> > I printed the address and contents of hvm_start_info in get_module_entry().
> > Here is the result.
> > 
> > 4MB build:
> > 
> > (d39) get_module_entry info 0x588000
> > (d39) info->magic         0x0
> > (d39) info->version       0x0
> > (d39) info->flags         0x0
> > (d39) info->nr_modules    0x0
> > (d39) info->modlist_paddr 0x0
> > (d39) info->cmdline_paddr 0x0
> > (d39) info->rsdp_paddr    0x0
> > 
> > Obviously, it's corrupted since magic is 0.
> 
> Note that hvmloader's main() has
> 
>     BUG_ON(hvm_start_info->magic != XEN_HVM_START_MAGIC_VALUE);
> 
> very early, so you having got past this means the corruption
> occurred inside hvmloader (or at least while it was already
> running). Could you comment out the call to perform_tests()
> and try again?
> 
You got it. After commenting out perform_tests(), the grub2 menu showed
and the system booted.

It seems that perform_tests() cleared 0x400000~0x800000, and that's why 
the members of hvm_start_info became 0 in my test.

Thanks!

Gary Lin


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Is there any limitation on the firmware size in Xen?

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 5982 bytes --]

>>> On 11.05.17 at 11:02, <GLin@suse.com> wrote:
> On Thu, May 11, 2017 at 06:14:42PM +1000, Jan Beulich wrote:
>> Note that hvmloader's main() has
>> 
>>     BUG_ON(hvm_start_info->magic != XEN_HVM_START_MAGIC_VALUE);
>> 
>> very early, so you having got past this means the corruption
>> occurred inside hvmloader (or at least while it was already
>> running). Could you comment out the call to perform_tests()
>> and try again?
>> 
> You got it. After commenting out perform_tests(), the grub2 menu showed
> and the system booted.
> 
> It seems that perform_tests() cleared 0x400000~0x800000, and that's why 
> the members of hvm_start_info became 0 in my test.

So could you give the below/attached patch a try?

Jan

--- a/tools/firmware/hvmloader/tests.c
+++ b/tools/firmware/hvmloader/tests.c
@@ -19,7 +19,9 @@
  * this program; If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "config.h"
 #include "util.h"
+#include <xen/arch-x86/hvm/start_info.h>
 
 #define TEST_FAIL 0
 #define TEST_PASS 1
@@ -29,10 +31,12 @@
  * Memory layout during tests:
  *  4MB to 8MB is cleared.
  *  Page directory resides at 8MB.
- *  4 page table pages reside at 8MB+4kB to 8MB+20kB.
- *  Pagetables identity-map 0-16MB, except 4kB at va 6MB maps to pa 5MB.
+ *  2 page table pages reside at 4MB+4kB to 4MB+12kB.
+ *  Pagetables identity-map 0-8MB, except 4kB at va 6MB maps to pa 5MB.
  */
-#define PD_START (8ul << 20)
+#define TEST_MEM_BASE (4ul << 20)
+#define TEST_MEM_SIZE (4ul << 20)
+#define PD_START TEST_MEM_BASE
 #define PT_START (PD_START + 4096)
 
 static void setup_paging(void)
@@ -41,10 +45,10 @@ static void setup_paging(void)
     uint32_t *pt = (uint32_t *)PT_START;
     uint32_t i;
 
-    /* Identity map 0-16MB. */
-    for ( i = 0; i < 4; i++ )
+    /* Identity map 0-8MB. */
+    for ( i = 0; i < 2; i++ )
         pd[i] = (unsigned long)pt + (i<<12) + 3;
-    for ( i = 0; i < (4*1024); i++ )
+    for ( i = 0; i < 2 * 1024; i++ )
         pt[i] = (i << 12) + 3;
 
     /* Page at virtual 6MB maps to physical 5MB. */
@@ -112,7 +116,7 @@ static int rep_io_test(void)
     stop_paging();
 
     i = 0;
-    for ( p = (uint32_t *)0x400000ul; p < (uint32_t *)0x700000ul; p++ )
+    for ( p = (uint32_t *)0x4ff000ul; p < (uint32_t *)0x602000ul; p++ )
     {
         uint32_t expected = 0;
         if ( check[i].addr == (unsigned long)p )
@@ -144,12 +148,12 @@ static int shadow_gs_test(void)
     if ( !(edx & (1u<<29)) )
         return TEST_SKIP;
 
-    /* Long mode pagetable setup: Identity map 0-16MB with 2MB mappings. */
+    /* Long mode pagetable setup: Identity map 0-8MB with 2MB mappings. */
     *pd = (unsigned long)pd + 0x1007; /* Level 4 */
     pd += 512;
     *pd = (unsigned long)pd + 0x1007; /* Level 3 */
     pd += 512;
-    for ( i = 0; i < 8; i++ )         /* Level 2 */
+    for ( i = 0; i < 4; i++ )         /* Level 2 */
         *pd++ = (i << 21) + 0x1e3;
 
     asm volatile (
@@ -191,8 +195,7 @@ static int shadow_gs_test(void)
 
 void perform_tests(void)
 {
-    int i, passed, skipped;
-
+    unsigned int i, passed, skipped;
     static struct {
         int (* const test)(void);
         const char *description;
@@ -204,12 +207,73 @@ void perform_tests(void)
 
     printf("Testing HVM environment:\n");
 
-    if ( hvm_info->low_mem_pgend < 0x1000 )
+    BUILD_BUG_ON(SCRATCH_PHYSICAL_ADDRESS > HVMLOADER_PHYSICAL_ADDRESS);
+    if ( hvm_info->low_mem_pgend <
+         ((TEST_MEM_BASE + TEST_MEM_SIZE) >> PAGE_SHIFT) )
+    {
+        printf("Skipping tests due to insufficient memory (<%luMB)\n",
+               (TEST_MEM_BASE + TEST_MEM_SIZE) >> 20);
+        return;
+    }
+
+    if ( hvm_start_info->cmdline_paddr &&
+         hvm_start_info->cmdline_paddr < TEST_MEM_BASE + TEST_MEM_SIZE &&
+         ((hvm_start_info->cmdline_paddr +
+           strlen((char *)(uintptr_t)hvm_start_info->cmdline_paddr)) >=
+          TEST_MEM_BASE) )
     {
-        printf("Skipping tests due to insufficient memory (<16MB)\n");
+        printf("Skipping tests due to overlap with command line\n");
         return;
     }
 
+    if ( hvm_start_info->rsdp_paddr )
+    {
+        printf("Skipping tests due to non-zero RSDP address\n");
+        return;
+    }
+
+    if ( hvm_start_info->nr_modules )
+    {
+        const struct hvm_modlist_entry *modlist =
+            (void *)(uintptr_t)hvm_start_info->modlist_paddr;
+
+        if ( hvm_start_info->modlist_paddr > UINTPTR_MAX ||
+             ((UINTPTR_MAX - (uintptr_t)modlist) / sizeof(*modlist) <
+              hvm_start_info->nr_modules) )
+        {
+            printf("Skipping tests due to inaccessible module list\n");
+            return;
+        }
+
+        if ( TEST_MEM_BASE < (uintptr_t)(modlist + i) &&
+             (uintptr_t)modlist < TEST_MEM_BASE + TEST_MEM_SIZE )
+        {
+            printf("Skipping tests due to overlap with module list\n");
+            return;
+        }
+
+        for ( i = 0; i < hvm_start_info->nr_modules; ++i )
+        {
+            if ( TEST_MEM_BASE < modlist[i].paddr + modlist[i].size &&
+                 modlist[i].paddr < TEST_MEM_BASE + TEST_MEM_SIZE )
+            {
+                printf("Skipping tests due to overlap with module %u\n", i);
+                return;
+            }
+
+            if ( modlist[i].cmdline_paddr &&
+                 modlist[i].cmdline_paddr < TEST_MEM_BASE + TEST_MEM_SIZE &&
+                 ((modlist[i].cmdline_paddr +
+                   strlen((char *)(uintptr_t)modlist[i].cmdline_paddr)) >=
+                  TEST_MEM_BASE) )
+            {
+                printf("Skipping tests due to overlap with module %u cmdline\n",
+                       i);
+                return;
+            }
+        }
+    }
+
     passed = skipped = 0;
     for ( i = 0; tests[i].test; i++ )
     {



[-- Attachment #2: hvmloader-constrain-tests.patch --]
[-- Type: text/plain, Size: 5863 bytes --]

hvmloader: avoid tests when they would clobber used memory

First of all limit the memory range used for testing to 4Mb: There's no
point placing page tables right above 8Mb when they can equally well
live at the bottom of the chunk at 4Mb - rep_io_test() cares about the
5Mb...7Mb range only anyway. In a subsequent patch this will then also
allow simply looking for an unused 4Mb range (instead of using a build
time determined one).

Extend the "skip tests" condition beyond the "is there enough memory"
question.

Reported-by: Charles Arnold <carnold@suse.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/tools/firmware/hvmloader/tests.c
+++ b/tools/firmware/hvmloader/tests.c
@@ -19,7 +19,9 @@
  * this program; If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "config.h"
 #include "util.h"
+#include <xen/arch-x86/hvm/start_info.h>
 
 #define TEST_FAIL 0
 #define TEST_PASS 1
@@ -29,10 +31,12 @@
  * Memory layout during tests:
  *  4MB to 8MB is cleared.
  *  Page directory resides at 8MB.
- *  4 page table pages reside at 8MB+4kB to 8MB+20kB.
- *  Pagetables identity-map 0-16MB, except 4kB at va 6MB maps to pa 5MB.
+ *  2 page table pages reside at 4MB+4kB to 4MB+12kB.
+ *  Pagetables identity-map 0-8MB, except 4kB at va 6MB maps to pa 5MB.
  */
-#define PD_START (8ul << 20)
+#define TEST_MEM_BASE (4ul << 20)
+#define TEST_MEM_SIZE (4ul << 20)
+#define PD_START TEST_MEM_BASE
 #define PT_START (PD_START + 4096)
 
 static void setup_paging(void)
@@ -41,10 +45,10 @@ static void setup_paging(void)
     uint32_t *pt = (uint32_t *)PT_START;
     uint32_t i;
 
-    /* Identity map 0-16MB. */
-    for ( i = 0; i < 4; i++ )
+    /* Identity map 0-8MB. */
+    for ( i = 0; i < 2; i++ )
         pd[i] = (unsigned long)pt + (i<<12) + 3;
-    for ( i = 0; i < (4*1024); i++ )
+    for ( i = 0; i < 2 * 1024; i++ )
         pt[i] = (i << 12) + 3;
 
     /* Page at virtual 6MB maps to physical 5MB. */
@@ -112,7 +116,7 @@ static int rep_io_test(void)
     stop_paging();
 
     i = 0;
-    for ( p = (uint32_t *)0x400000ul; p < (uint32_t *)0x700000ul; p++ )
+    for ( p = (uint32_t *)0x4ff000ul; p < (uint32_t *)0x602000ul; p++ )
     {
         uint32_t expected = 0;
         if ( check[i].addr == (unsigned long)p )
@@ -144,12 +148,12 @@ static int shadow_gs_test(void)
     if ( !(edx & (1u<<29)) )
         return TEST_SKIP;
 
-    /* Long mode pagetable setup: Identity map 0-16MB with 2MB mappings. */
+    /* Long mode pagetable setup: Identity map 0-8MB with 2MB mappings. */
     *pd = (unsigned long)pd + 0x1007; /* Level 4 */
     pd += 512;
     *pd = (unsigned long)pd + 0x1007; /* Level 3 */
     pd += 512;
-    for ( i = 0; i < 8; i++ )         /* Level 2 */
+    for ( i = 0; i < 4; i++ )         /* Level 2 */
         *pd++ = (i << 21) + 0x1e3;
 
     asm volatile (
@@ -191,8 +195,7 @@ static int shadow_gs_test(void)
 
 void perform_tests(void)
 {
-    int i, passed, skipped;
-
+    unsigned int i, passed, skipped;
     static struct {
         int (* const test)(void);
         const char *description;
@@ -204,12 +207,73 @@ void perform_tests(void)
 
     printf("Testing HVM environment:\n");
 
-    if ( hvm_info->low_mem_pgend < 0x1000 )
+    BUILD_BUG_ON(SCRATCH_PHYSICAL_ADDRESS > HVMLOADER_PHYSICAL_ADDRESS);
+    if ( hvm_info->low_mem_pgend <
+         ((TEST_MEM_BASE + TEST_MEM_SIZE) >> PAGE_SHIFT) )
+    {
+        printf("Skipping tests due to insufficient memory (<%luMB)\n",
+               (TEST_MEM_BASE + TEST_MEM_SIZE) >> 20);
+        return;
+    }
+
+    if ( hvm_start_info->cmdline_paddr &&
+         hvm_start_info->cmdline_paddr < TEST_MEM_BASE + TEST_MEM_SIZE &&
+         ((hvm_start_info->cmdline_paddr +
+           strlen((char *)(uintptr_t)hvm_start_info->cmdline_paddr)) >=
+          TEST_MEM_BASE) )
     {
-        printf("Skipping tests due to insufficient memory (<16MB)\n");
+        printf("Skipping tests due to overlap with command line\n");
         return;
     }
 
+    if ( hvm_start_info->rsdp_paddr )
+    {
+        printf("Skipping tests due to non-zero RSDP address\n");
+        return;
+    }
+
+    if ( hvm_start_info->nr_modules )
+    {
+        const struct hvm_modlist_entry *modlist =
+            (void *)(uintptr_t)hvm_start_info->modlist_paddr;
+
+        if ( hvm_start_info->modlist_paddr > UINTPTR_MAX ||
+             ((UINTPTR_MAX - (uintptr_t)modlist) / sizeof(*modlist) <
+              hvm_start_info->nr_modules) )
+        {
+            printf("Skipping tests due to inaccessible module list\n");
+            return;
+        }
+
+        if ( TEST_MEM_BASE < (uintptr_t)(modlist + i) &&
+             (uintptr_t)modlist < TEST_MEM_BASE + TEST_MEM_SIZE )
+        {
+            printf("Skipping tests due to overlap with module list\n");
+            return;
+        }
+
+        for ( i = 0; i < hvm_start_info->nr_modules; ++i )
+        {
+            if ( TEST_MEM_BASE < modlist[i].paddr + modlist[i].size &&
+                 modlist[i].paddr < TEST_MEM_BASE + TEST_MEM_SIZE )
+            {
+                printf("Skipping tests due to overlap with module %u\n", i);
+                return;
+            }
+
+            if ( modlist[i].cmdline_paddr &&
+                 modlist[i].cmdline_paddr < TEST_MEM_BASE + TEST_MEM_SIZE &&
+                 ((modlist[i].cmdline_paddr +
+                   strlen((char *)(uintptr_t)modlist[i].cmdline_paddr)) >=
+                  TEST_MEM_BASE) )
+            {
+                printf("Skipping tests due to overlap with module %u cmdline\n",
+                       i);
+                return;
+            }
+        }
+    }
+
     passed = skipped = 0;
     for ( i = 0; tests[i].test; i++ )
     {

[-- Attachment #3: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Is there any limitation on the firmware size in Xen?

[Gary Lin]

On Fri, May 12, 2017 at 07:32:49PM +1000, Jan Beulich wrote:
> >>> On 11.05.17 at 11:02, <GLin@suse.com> wrote:
> > On Thu, May 11, 2017 at 06:14:42PM +1000, Jan Beulich wrote:
> >> Note that hvmloader's main() has
> >> 
> >>     BUG_ON(hvm_start_info->magic != XEN_HVM_START_MAGIC_VALUE);
> >> 
> >> very early, so you having got past this means the corruption
> >> occurred inside hvmloader (or at least while it was already
> >> running). Could you comment out the call to perform_tests()
> >> and try again?
> >> 
> > You got it. After commenting out perform_tests(), the grub2 menu showed
> > and the system booted.
> > 
> > It seems that perform_tests() cleared 0x400000~0x800000, and that's why 
> > the members of hvm_start_info became 0 in my test.
> 
> So could you give the below/attached patch a try?
> 
It won't compile.

tests.c: In function 'perform_tests':
tests.c:248:50: error: 'i' may be used uninitialized in this function [-Werror=maybe-uninitialized]
         if ( TEST_MEM_BASE < (uintptr_t)(modlist + i) &&
                                                  ^
cc1: all warnings being treated as errors

Gary Lin

> Jan
> 
> --- a/tools/firmware/hvmloader/tests.c
> +++ b/tools/firmware/hvmloader/tests.c
> @@ -19,7 +19,9 @@
>   * this program; If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include "config.h"
>  #include "util.h"
> +#include <xen/arch-x86/hvm/start_info.h>
>  
>  #define TEST_FAIL 0
>  #define TEST_PASS 1
> @@ -29,10 +31,12 @@
>   * Memory layout during tests:
>   *  4MB to 8MB is cleared.
>   *  Page directory resides at 8MB.
> - *  4 page table pages reside at 8MB+4kB to 8MB+20kB.
> - *  Pagetables identity-map 0-16MB, except 4kB at va 6MB maps to pa 5MB.
> + *  2 page table pages reside at 4MB+4kB to 4MB+12kB.
> + *  Pagetables identity-map 0-8MB, except 4kB at va 6MB maps to pa 5MB.
>   */
> -#define PD_START (8ul << 20)
> +#define TEST_MEM_BASE (4ul << 20)
> +#define TEST_MEM_SIZE (4ul << 20)
> +#define PD_START TEST_MEM_BASE
>  #define PT_START (PD_START + 4096)
>  
>  static void setup_paging(void)
> @@ -41,10 +45,10 @@ static void setup_paging(void)
>      uint32_t *pt = (uint32_t *)PT_START;
>      uint32_t i;
>  
> -    /* Identity map 0-16MB. */
> -    for ( i = 0; i < 4; i++ )
> +    /* Identity map 0-8MB. */
> +    for ( i = 0; i < 2; i++ )
>          pd[i] = (unsigned long)pt + (i<<12) + 3;
> -    for ( i = 0; i < (4*1024); i++ )
> +    for ( i = 0; i < 2 * 1024; i++ )
>          pt[i] = (i << 12) + 3;
>  
>      /* Page at virtual 6MB maps to physical 5MB. */
> @@ -112,7 +116,7 @@ static int rep_io_test(void)
>      stop_paging();
>  
>      i = 0;
> -    for ( p = (uint32_t *)0x400000ul; p < (uint32_t *)0x700000ul; p++ )
> +    for ( p = (uint32_t *)0x4ff000ul; p < (uint32_t *)0x602000ul; p++ )
>      {
>          uint32_t expected = 0;
>          if ( check[i].addr == (unsigned long)p )
> @@ -144,12 +148,12 @@ static int shadow_gs_test(void)
>      if ( !(edx & (1u<<29)) )
>          return TEST_SKIP;
>  
> -    /* Long mode pagetable setup: Identity map 0-16MB with 2MB mappings. */
> +    /* Long mode pagetable setup: Identity map 0-8MB with 2MB mappings. */
>      *pd = (unsigned long)pd + 0x1007; /* Level 4 */
>      pd += 512;
>      *pd = (unsigned long)pd + 0x1007; /* Level 3 */
>      pd += 512;
> -    for ( i = 0; i < 8; i++ )         /* Level 2 */
> +    for ( i = 0; i < 4; i++ )         /* Level 2 */
>          *pd++ = (i << 21) + 0x1e3;
>  
>      asm volatile (
> @@ -191,8 +195,7 @@ static int shadow_gs_test(void)
>  
>  void perform_tests(void)
>  {
> -    int i, passed, skipped;
> -
> +    unsigned int i, passed, skipped;
>      static struct {
>          int (* const test)(void);
>          const char *description;
> @@ -204,12 +207,73 @@ void perform_tests(void)
>  
>      printf("Testing HVM environment:\n");
>  
> -    if ( hvm_info->low_mem_pgend < 0x1000 )
> +    BUILD_BUG_ON(SCRATCH_PHYSICAL_ADDRESS > HVMLOADER_PHYSICAL_ADDRESS);
> +    if ( hvm_info->low_mem_pgend <
> +         ((TEST_MEM_BASE + TEST_MEM_SIZE) >> PAGE_SHIFT) )
> +    {
> +        printf("Skipping tests due to insufficient memory (<%luMB)\n",
> +               (TEST_MEM_BASE + TEST_MEM_SIZE) >> 20);
> +        return;
> +    }
> +
> +    if ( hvm_start_info->cmdline_paddr &&
> +         hvm_start_info->cmdline_paddr < TEST_MEM_BASE + TEST_MEM_SIZE &&
> +         ((hvm_start_info->cmdline_paddr +
> +           strlen((char *)(uintptr_t)hvm_start_info->cmdline_paddr)) >=
> +          TEST_MEM_BASE) )
>      {
> -        printf("Skipping tests due to insufficient memory (<16MB)\n");
> +        printf("Skipping tests due to overlap with command line\n");
>          return;
>      }
>  
> +    if ( hvm_start_info->rsdp_paddr )
> +    {
> +        printf("Skipping tests due to non-zero RSDP address\n");
> +        return;
> +    }
> +
> +    if ( hvm_start_info->nr_modules )
> +    {
> +        const struct hvm_modlist_entry *modlist =
> +            (void *)(uintptr_t)hvm_start_info->modlist_paddr;
> +
> +        if ( hvm_start_info->modlist_paddr > UINTPTR_MAX ||
> +             ((UINTPTR_MAX - (uintptr_t)modlist) / sizeof(*modlist) <
> +              hvm_start_info->nr_modules) )
> +        {
> +            printf("Skipping tests due to inaccessible module list\n");
> +            return;
> +        }
> +
> +        if ( TEST_MEM_BASE < (uintptr_t)(modlist + i) &&
> +             (uintptr_t)modlist < TEST_MEM_BASE + TEST_MEM_SIZE )
> +        {
> +            printf("Skipping tests due to overlap with module list\n");
> +            return;
> +        }
> +
> +        for ( i = 0; i < hvm_start_info->nr_modules; ++i )
> +        {
> +            if ( TEST_MEM_BASE < modlist[i].paddr + modlist[i].size &&
> +                 modlist[i].paddr < TEST_MEM_BASE + TEST_MEM_SIZE )
> +            {
> +                printf("Skipping tests due to overlap with module %u\n", i);
> +                return;
> +            }
> +
> +            if ( modlist[i].cmdline_paddr &&
> +                 modlist[i].cmdline_paddr < TEST_MEM_BASE + TEST_MEM_SIZE &&
> +                 ((modlist[i].cmdline_paddr +
> +                   strlen((char *)(uintptr_t)modlist[i].cmdline_paddr)) >=
> +                  TEST_MEM_BASE) )
> +            {
> +                printf("Skipping tests due to overlap with module %u cmdline\n",
> +                       i);
> +                return;
> +            }
> +        }
> +    }
> +
>      passed = skipped = 0;
>      for ( i = 0; tests[i].test; i++ )
>      {
> 
> 

> hvmloader: avoid tests when they would clobber used memory
> 
> First of all limit the memory range used for testing to 4Mb: There's no
> point placing page tables right above 8Mb when they can equally well
> live at the bottom of the chunk at 4Mb - rep_io_test() cares about the
> 5Mb...7Mb range only anyway. In a subsequent patch this will then also
> allow simply looking for an unused 4Mb range (instead of using a build
> time determined one).
> 
> Extend the "skip tests" condition beyond the "is there enough memory"
> question.
> 
> Reported-by: Charles Arnold <carnold@suse.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/tools/firmware/hvmloader/tests.c
> +++ b/tools/firmware/hvmloader/tests.c
> @@ -19,7 +19,9 @@
>   * this program; If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include "config.h"
>  #include "util.h"
> +#include <xen/arch-x86/hvm/start_info.h>
>  
>  #define TEST_FAIL 0
>  #define TEST_PASS 1
> @@ -29,10 +31,12 @@
>   * Memory layout during tests:
>   *  4MB to 8MB is cleared.
>   *  Page directory resides at 8MB.
> - *  4 page table pages reside at 8MB+4kB to 8MB+20kB.
> - *  Pagetables identity-map 0-16MB, except 4kB at va 6MB maps to pa 5MB.
> + *  2 page table pages reside at 4MB+4kB to 4MB+12kB.
> + *  Pagetables identity-map 0-8MB, except 4kB at va 6MB maps to pa 5MB.
>   */
> -#define PD_START (8ul << 20)
> +#define TEST_MEM_BASE (4ul << 20)
> +#define TEST_MEM_SIZE (4ul << 20)
> +#define PD_START TEST_MEM_BASE
>  #define PT_START (PD_START + 4096)
>  
>  static void setup_paging(void)
> @@ -41,10 +45,10 @@ static void setup_paging(void)
>      uint32_t *pt = (uint32_t *)PT_START;
>      uint32_t i;
>  
> -    /* Identity map 0-16MB. */
> -    for ( i = 0; i < 4; i++ )
> +    /* Identity map 0-8MB. */
> +    for ( i = 0; i < 2; i++ )
>          pd[i] = (unsigned long)pt + (i<<12) + 3;
> -    for ( i = 0; i < (4*1024); i++ )
> +    for ( i = 0; i < 2 * 1024; i++ )
>          pt[i] = (i << 12) + 3;
>  
>      /* Page at virtual 6MB maps to physical 5MB. */
> @@ -112,7 +116,7 @@ static int rep_io_test(void)
>      stop_paging();
>  
>      i = 0;
> -    for ( p = (uint32_t *)0x400000ul; p < (uint32_t *)0x700000ul; p++ )
> +    for ( p = (uint32_t *)0x4ff000ul; p < (uint32_t *)0x602000ul; p++ )
>      {
>          uint32_t expected = 0;
>          if ( check[i].addr == (unsigned long)p )
> @@ -144,12 +148,12 @@ static int shadow_gs_test(void)
>      if ( !(edx & (1u<<29)) )
>          return TEST_SKIP;
>  
> -    /* Long mode pagetable setup: Identity map 0-16MB with 2MB mappings. */
> +    /* Long mode pagetable setup: Identity map 0-8MB with 2MB mappings. */
>      *pd = (unsigned long)pd + 0x1007; /* Level 4 */
>      pd += 512;
>      *pd = (unsigned long)pd + 0x1007; /* Level 3 */
>      pd += 512;
> -    for ( i = 0; i < 8; i++ )         /* Level 2 */
> +    for ( i = 0; i < 4; i++ )         /* Level 2 */
>          *pd++ = (i << 21) + 0x1e3;
>  
>      asm volatile (
> @@ -191,8 +195,7 @@ static int shadow_gs_test(void)
>  
>  void perform_tests(void)
>  {
> -    int i, passed, skipped;
> -
> +    unsigned int i, passed, skipped;
>      static struct {
>          int (* const test)(void);
>          const char *description;
> @@ -204,12 +207,73 @@ void perform_tests(void)
>  
>      printf("Testing HVM environment:\n");
>  
> -    if ( hvm_info->low_mem_pgend < 0x1000 )
> +    BUILD_BUG_ON(SCRATCH_PHYSICAL_ADDRESS > HVMLOADER_PHYSICAL_ADDRESS);
> +    if ( hvm_info->low_mem_pgend <
> +         ((TEST_MEM_BASE + TEST_MEM_SIZE) >> PAGE_SHIFT) )
> +    {
> +        printf("Skipping tests due to insufficient memory (<%luMB)\n",
> +               (TEST_MEM_BASE + TEST_MEM_SIZE) >> 20);
> +        return;
> +    }
> +
> +    if ( hvm_start_info->cmdline_paddr &&
> +         hvm_start_info->cmdline_paddr < TEST_MEM_BASE + TEST_MEM_SIZE &&
> +         ((hvm_start_info->cmdline_paddr +
> +           strlen((char *)(uintptr_t)hvm_start_info->cmdline_paddr)) >=
> +          TEST_MEM_BASE) )
>      {
> -        printf("Skipping tests due to insufficient memory (<16MB)\n");
> +        printf("Skipping tests due to overlap with command line\n");
>          return;
>      }
>  
> +    if ( hvm_start_info->rsdp_paddr )
> +    {
> +        printf("Skipping tests due to non-zero RSDP address\n");
> +        return;
> +    }
> +
> +    if ( hvm_start_info->nr_modules )
> +    {
> +        const struct hvm_modlist_entry *modlist =
> +            (void *)(uintptr_t)hvm_start_info->modlist_paddr;
> +
> +        if ( hvm_start_info->modlist_paddr > UINTPTR_MAX ||
> +             ((UINTPTR_MAX - (uintptr_t)modlist) / sizeof(*modlist) <
> +              hvm_start_info->nr_modules) )
> +        {
> +            printf("Skipping tests due to inaccessible module list\n");
> +            return;
> +        }
> +
> +        if ( TEST_MEM_BASE < (uintptr_t)(modlist + i) &&
> +             (uintptr_t)modlist < TEST_MEM_BASE + TEST_MEM_SIZE )
> +        {
> +            printf("Skipping tests due to overlap with module list\n");
> +            return;
> +        }
> +
> +        for ( i = 0; i < hvm_start_info->nr_modules; ++i )
> +        {
> +            if ( TEST_MEM_BASE < modlist[i].paddr + modlist[i].size &&
> +                 modlist[i].paddr < TEST_MEM_BASE + TEST_MEM_SIZE )
> +            {
> +                printf("Skipping tests due to overlap with module %u\n", i);
> +                return;
> +            }
> +
> +            if ( modlist[i].cmdline_paddr &&
> +                 modlist[i].cmdline_paddr < TEST_MEM_BASE + TEST_MEM_SIZE &&
> +                 ((modlist[i].cmdline_paddr +
> +                   strlen((char *)(uintptr_t)modlist[i].cmdline_paddr)) >=
> +                  TEST_MEM_BASE) )
> +            {
> +                printf("Skipping tests due to overlap with module %u cmdline\n",
> +                       i);
> +                return;
> +            }
> +        }
> +    }
> +
>      passed = skipped = 0;
>      for ( i = 0; tests[i].test; i++ )
>      {


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Is there any limitation on the firmware size in Xen?

[Andrew Cooper]

On 12/05/17 10:32, Jan Beulich wrote:
>>>> On 11.05.17 at 11:02, <GLin@suse.com> wrote:
>> On Thu, May 11, 2017 at 06:14:42PM +1000, Jan Beulich wrote:
>>> Note that hvmloader's main() has
>>>
>>>     BUG_ON(hvm_start_info->magic != XEN_HVM_START_MAGIC_VALUE);
>>>
>>> very early, so you having got past this means the corruption
>>> occurred inside hvmloader (or at least while it was already
>>> running). Could you comment out the call to perform_tests()
>>> and try again?
>>>
>> You got it. After commenting out perform_tests(), the grub2 menu showed
>> and the system booted.
>>
>> It seems that perform_tests() cleared 0x400000~0x800000, and that's why 
>> the members of hvm_start_info became 0 in my test.
> So could you give the below/attached patch a try?
>
> Jan
>
> --- a/tools/firmware/hvmloader/tests.c
> +++ b/tools/firmware/hvmloader/tests.c

In the presence of XTF regression tests being run in OSSTest, this
entire mechanism in HVMLoader ought to be dropped.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Is there any limitation on the firmware size in Xen?

[Jan Beulich]

>>> On 12.05.17 at 12:11, <andrew.cooper3@citrix.com> wrote:
> On 12/05/17 10:32, Jan Beulich wrote:
>> --- a/tools/firmware/hvmloader/tests.c
>> +++ b/tools/firmware/hvmloader/tests.c
> 
> In the presence of XTF regression tests being run in OSSTest, this
> entire mechanism in HVMLoader ought to be dropped.

Well, maybe, but rather not at this point of the release cycle. Even
after 4.9 was branched I'm not entirely convinced that these aren't
at least reasonable as immediate smoke tests.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Is there any limitation on the firmware size in Xen?

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 6927 bytes --]

>>> On 12.05.17 at 12:06, <GLin@suse.com> wrote:
> On Fri, May 12, 2017 at 07:32:49PM +1000, Jan Beulich wrote:
>> >>> On 11.05.17 at 11:02, <GLin@suse.com> wrote:
>> > On Thu, May 11, 2017 at 06:14:42PM +1000, Jan Beulich wrote:
>> >> Note that hvmloader's main() has
>> >> 
>> >>     BUG_ON(hvm_start_info->magic != XEN_HVM_START_MAGIC_VALUE);
>> >> 
>> >> very early, so you having got past this means the corruption
>> >> occurred inside hvmloader (or at least while it was already
>> >> running). Could you comment out the call to perform_tests()
>> >> and try again?
>> >> 
>> > You got it. After commenting out perform_tests(), the grub2 menu showed
>> > and the system booted.
>> > 
>> > It seems that perform_tests() cleared 0x400000~0x800000, and that's why 
>> > the members of hvm_start_info became 0 in my test.
>> 
>> So could you give the below/attached patch a try?
>> 
> It won't compile.
> 
> tests.c: In function 'perform_tests':
> tests.c:248:50: error: 'i' may be used uninitialized in this function 
> [-Werror=maybe-uninitialized]
>          if ( TEST_MEM_BASE < (uintptr_t)(modlist + i) &&
>                                                   ^
> cc1: all warnings being treated as errors

Oops - quite obviously. No idea why neither of the two gcc versions
I've built this with caught the issue. Below/attached a better one (also
with a few other changes).

Jan

--- a/tools/firmware/hvmloader/tests.c
+++ b/tools/firmware/hvmloader/tests.c
@@ -19,7 +19,9 @@
  * this program; If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "config.h"
 #include "util.h"
+#include <xen/arch-x86/hvm/start_info.h>
 
 #define TEST_FAIL 0
 #define TEST_PASS 1
@@ -28,11 +30,13 @@
 /*
  * Memory layout during tests:
  *  4MB to 8MB is cleared.
- *  Page directory resides at 8MB.
- *  4 page table pages reside at 8MB+4kB to 8MB+20kB.
- *  Pagetables identity-map 0-16MB, except 4kB at va 6MB maps to pa 5MB.
+ *  Page directory resides at 4MB.
+ *  2 page table pages reside at 4MB+4kB to 4MB+12kB.
+ *  Pagetables identity-map 0-8MB, except 4kB at va 6MB maps to pa 5MB.
  */
-#define PD_START (8ul << 20)
+#define TEST_MEM_BASE (4ul << 20)
+#define TEST_MEM_SIZE (4ul << 20)
+#define PD_START TEST_MEM_BASE
 #define PT_START (PD_START + 4096)
 
 static void setup_paging(void)
@@ -41,10 +45,10 @@ static void setup_paging(void)
     uint32_t *pt = (uint32_t *)PT_START;
     uint32_t i;
 
-    /* Identity map 0-16MB. */
-    for ( i = 0; i < 4; i++ )
+    /* Identity map 0-8MB. */
+    for ( i = 0; i < 2; i++ )
         pd[i] = (unsigned long)pt + (i<<12) + 3;
-    for ( i = 0; i < (4*1024); i++ )
+    for ( i = 0; i < 2 * 1024; i++ )
         pt[i] = (i << 12) + 3;
 
     /* Page at virtual 6MB maps to physical 5MB. */
@@ -112,7 +116,7 @@ static int rep_io_test(void)
     stop_paging();
 
     i = 0;
-    for ( p = (uint32_t *)0x400000ul; p < (uint32_t *)0x700000ul; p++ )
+    for ( p = (uint32_t *)0x4ff000ul; p < (uint32_t *)0x602000ul; p++ )
     {
         uint32_t expected = 0;
         if ( check[i].addr == (unsigned long)p )
@@ -144,12 +148,12 @@ static int shadow_gs_test(void)
     if ( !(edx & (1u<<29)) )
         return TEST_SKIP;
 
-    /* Long mode pagetable setup: Identity map 0-16MB with 2MB mappings. */
+    /* Long mode pagetable setup: Identity map 0-8MB with 2MB mappings. */
     *pd = (unsigned long)pd + 0x1007; /* Level 4 */
     pd += 512;
     *pd = (unsigned long)pd + 0x1007; /* Level 3 */
     pd += 512;
-    for ( i = 0; i < 8; i++ )         /* Level 2 */
+    for ( i = 0; i < 4; i++ )         /* Level 2 */
         *pd++ = (i << 21) + 0x1e3;
 
     asm volatile (
@@ -191,8 +195,7 @@ static int shadow_gs_test(void)
 
 void perform_tests(void)
 {
-    int i, passed, skipped;
-
+    unsigned int i, passed, skipped;
     static struct {
         int (* const test)(void);
         const char *description;
@@ -204,12 +207,80 @@ void perform_tests(void)
 
     printf("Testing HVM environment:\n");
 
-    if ( hvm_info->low_mem_pgend < 0x1000 )
+    BUILD_BUG_ON(SCRATCH_PHYSICAL_ADDRESS > HVMLOADER_PHYSICAL_ADDRESS);
+    if ( hvm_info->low_mem_pgend <
+         ((TEST_MEM_BASE + TEST_MEM_SIZE) >> PAGE_SHIFT) )
+    {
+        printf("Skipping tests due to insufficient memory (<%luMB)\n",
+               (TEST_MEM_BASE + TEST_MEM_SIZE) >> 20);
+        return;
+    }
+
+    if ( (unsigned long)_end > TEST_MEM_BASE )
+    {
+        printf("Skipping tests due to overlap with base image\n");
+        return;
+    }
+
+    if ( hvm_start_info->cmdline_paddr &&
+         hvm_start_info->cmdline_paddr < TEST_MEM_BASE + TEST_MEM_SIZE &&
+         ((hvm_start_info->cmdline_paddr +
+           strlen((char *)(uintptr_t)hvm_start_info->cmdline_paddr)) >=
+          TEST_MEM_BASE) )
+    {
+        printf("Skipping tests due to overlap with command line\n");
+        return;
+    }
+
+    if ( hvm_start_info->rsdp_paddr )
     {
-        printf("Skipping tests due to insufficient memory (<16MB)\n");
+        printf("Skipping tests due to non-zero RSDP address\n");
         return;
     }
 
+    if ( hvm_start_info->nr_modules )
+    {
+        const struct hvm_modlist_entry *modlist =
+            (void *)(uintptr_t)hvm_start_info->modlist_paddr;
+
+        if ( hvm_start_info->modlist_paddr > UINTPTR_MAX ||
+             ((UINTPTR_MAX - (uintptr_t)modlist) / sizeof(*modlist) <
+              hvm_start_info->nr_modules) )
+        {
+            printf("Skipping tests due to inaccessible module list\n");
+            return;
+        }
+
+        if ( TEST_MEM_BASE < (uintptr_t)(modlist +
+                                         hvm_start_info->nr_modules) &&
+             (uintptr_t)modlist < TEST_MEM_BASE + TEST_MEM_SIZE )
+        {
+            printf("Skipping tests due to overlap with module list\n");
+            return;
+        }
+
+        for ( i = 0; i < hvm_start_info->nr_modules; ++i )
+        {
+            if ( TEST_MEM_BASE < modlist[i].paddr + modlist[i].size &&
+                 modlist[i].paddr < TEST_MEM_BASE + TEST_MEM_SIZE )
+            {
+                printf("Skipping tests due to overlap with module %u\n", i);
+                return;
+            }
+
+            if ( modlist[i].cmdline_paddr &&
+                 modlist[i].cmdline_paddr < TEST_MEM_BASE + TEST_MEM_SIZE &&
+                 ((modlist[i].cmdline_paddr +
+                   strlen((char *)(uintptr_t)modlist[i].cmdline_paddr)) >=
+                  TEST_MEM_BASE) )
+            {
+                printf("Skipping tests due to overlap with module %u cmdline\n",
+                       i);
+                return;
+            }
+        }
+    }
+
     passed = skipped = 0;
     for ( i = 0; tests[i].test; i++ )
     {



[-- Attachment #2: hvmloader-constrain-tests.patch --]
[-- Type: text/plain, Size: 6128 bytes --]

hvmloader: avoid tests when they would clobber used memory

First of all limit the memory range used for testing to 4Mb: There's no
point placing page tables right above 8Mb when they can equally well
live at the bottom of the chunk at 4Mb - rep_io_test() cares about the
5Mb...7Mb range only anyway. In a subsequent patch this will then also
allow simply looking for an unused 4Mb range (instead of using a build
time determined one).

Extend the "skip tests" condition beyond the "is there enough memory"
question.

Reported-by: Charles Arnold <carnold@suse.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/tools/firmware/hvmloader/tests.c
+++ b/tools/firmware/hvmloader/tests.c
@@ -19,7 +19,9 @@
  * this program; If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "config.h"
 #include "util.h"
+#include <xen/arch-x86/hvm/start_info.h>
 
 #define TEST_FAIL 0
 #define TEST_PASS 1
@@ -28,11 +30,13 @@
 /*
  * Memory layout during tests:
  *  4MB to 8MB is cleared.
- *  Page directory resides at 8MB.
- *  4 page table pages reside at 8MB+4kB to 8MB+20kB.
- *  Pagetables identity-map 0-16MB, except 4kB at va 6MB maps to pa 5MB.
+ *  Page directory resides at 4MB.
+ *  2 page table pages reside at 4MB+4kB to 4MB+12kB.
+ *  Pagetables identity-map 0-8MB, except 4kB at va 6MB maps to pa 5MB.
  */
-#define PD_START (8ul << 20)
+#define TEST_MEM_BASE (4ul << 20)
+#define TEST_MEM_SIZE (4ul << 20)
+#define PD_START TEST_MEM_BASE
 #define PT_START (PD_START + 4096)
 
 static void setup_paging(void)
@@ -41,10 +45,10 @@ static void setup_paging(void)
     uint32_t *pt = (uint32_t *)PT_START;
     uint32_t i;
 
-    /* Identity map 0-16MB. */
-    for ( i = 0; i < 4; i++ )
+    /* Identity map 0-8MB. */
+    for ( i = 0; i < 2; i++ )
         pd[i] = (unsigned long)pt + (i<<12) + 3;
-    for ( i = 0; i < (4*1024); i++ )
+    for ( i = 0; i < 2 * 1024; i++ )
         pt[i] = (i << 12) + 3;
 
     /* Page at virtual 6MB maps to physical 5MB. */
@@ -112,7 +116,7 @@ static int rep_io_test(void)
     stop_paging();
 
     i = 0;
-    for ( p = (uint32_t *)0x400000ul; p < (uint32_t *)0x700000ul; p++ )
+    for ( p = (uint32_t *)0x4ff000ul; p < (uint32_t *)0x602000ul; p++ )
     {
         uint32_t expected = 0;
         if ( check[i].addr == (unsigned long)p )
@@ -144,12 +148,12 @@ static int shadow_gs_test(void)
     if ( !(edx & (1u<<29)) )
         return TEST_SKIP;
 
-    /* Long mode pagetable setup: Identity map 0-16MB with 2MB mappings. */
+    /* Long mode pagetable setup: Identity map 0-8MB with 2MB mappings. */
     *pd = (unsigned long)pd + 0x1007; /* Level 4 */
     pd += 512;
     *pd = (unsigned long)pd + 0x1007; /* Level 3 */
     pd += 512;
-    for ( i = 0; i < 8; i++ )         /* Level 2 */
+    for ( i = 0; i < 4; i++ )         /* Level 2 */
         *pd++ = (i << 21) + 0x1e3;
 
     asm volatile (
@@ -191,8 +195,7 @@ static int shadow_gs_test(void)
 
 void perform_tests(void)
 {
-    int i, passed, skipped;
-
+    unsigned int i, passed, skipped;
     static struct {
         int (* const test)(void);
         const char *description;
@@ -204,12 +207,80 @@ void perform_tests(void)
 
     printf("Testing HVM environment:\n");
 
-    if ( hvm_info->low_mem_pgend < 0x1000 )
+    BUILD_BUG_ON(SCRATCH_PHYSICAL_ADDRESS > HVMLOADER_PHYSICAL_ADDRESS);
+    if ( hvm_info->low_mem_pgend <
+         ((TEST_MEM_BASE + TEST_MEM_SIZE) >> PAGE_SHIFT) )
+    {
+        printf("Skipping tests due to insufficient memory (<%luMB)\n",
+               (TEST_MEM_BASE + TEST_MEM_SIZE) >> 20);
+        return;
+    }
+
+    if ( (unsigned long)_end > TEST_MEM_BASE )
+    {
+        printf("Skipping tests due to overlap with base image\n");
+        return;
+    }
+
+    if ( hvm_start_info->cmdline_paddr &&
+         hvm_start_info->cmdline_paddr < TEST_MEM_BASE + TEST_MEM_SIZE &&
+         ((hvm_start_info->cmdline_paddr +
+           strlen((char *)(uintptr_t)hvm_start_info->cmdline_paddr)) >=
+          TEST_MEM_BASE) )
+    {
+        printf("Skipping tests due to overlap with command line\n");
+        return;
+    }
+
+    if ( hvm_start_info->rsdp_paddr )
     {
-        printf("Skipping tests due to insufficient memory (<16MB)\n");
+        printf("Skipping tests due to non-zero RSDP address\n");
         return;
     }
 
+    if ( hvm_start_info->nr_modules )
+    {
+        const struct hvm_modlist_entry *modlist =
+            (void *)(uintptr_t)hvm_start_info->modlist_paddr;
+
+        if ( hvm_start_info->modlist_paddr > UINTPTR_MAX ||
+             ((UINTPTR_MAX - (uintptr_t)modlist) / sizeof(*modlist) <
+              hvm_start_info->nr_modules) )
+        {
+            printf("Skipping tests due to inaccessible module list\n");
+            return;
+        }
+
+        if ( TEST_MEM_BASE < (uintptr_t)(modlist +
+                                         hvm_start_info->nr_modules) &&
+             (uintptr_t)modlist < TEST_MEM_BASE + TEST_MEM_SIZE )
+        {
+            printf("Skipping tests due to overlap with module list\n");
+            return;
+        }
+
+        for ( i = 0; i < hvm_start_info->nr_modules; ++i )
+        {
+            if ( TEST_MEM_BASE < modlist[i].paddr + modlist[i].size &&
+                 modlist[i].paddr < TEST_MEM_BASE + TEST_MEM_SIZE )
+            {
+                printf("Skipping tests due to overlap with module %u\n", i);
+                return;
+            }
+
+            if ( modlist[i].cmdline_paddr &&
+                 modlist[i].cmdline_paddr < TEST_MEM_BASE + TEST_MEM_SIZE &&
+                 ((modlist[i].cmdline_paddr +
+                   strlen((char *)(uintptr_t)modlist[i].cmdline_paddr)) >=
+                  TEST_MEM_BASE) )
+            {
+                printf("Skipping tests due to overlap with module %u cmdline\n",
+                       i);
+                return;
+            }
+        }
+    }
+
     passed = skipped = 0;
     for ( i = 0; tests[i].test; i++ )
     {

[-- Attachment #3: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Is there any limitation on the firmware size in Xen?

[Gary Lin]

On Fri, May 12, 2017 at 10:31:28PM +1000, Jan Beulich wrote:
> >>> On 12.05.17 at 12:06, <GLin@suse.com> wrote:
> > On Fri, May 12, 2017 at 07:32:49PM +1000, Jan Beulich wrote:
> >> >>> On 11.05.17 at 11:02, <GLin@suse.com> wrote:
> >> > On Thu, May 11, 2017 at 06:14:42PM +1000, Jan Beulich wrote:
> >> >> Note that hvmloader's main() has
> >> >> 
> >> >>     BUG_ON(hvm_start_info->magic != XEN_HVM_START_MAGIC_VALUE);
> >> >> 
> >> >> very early, so you having got past this means the corruption
> >> >> occurred inside hvmloader (or at least while it was already
> >> >> running). Could you comment out the call to perform_tests()
> >> >> and try again?
> >> >> 
> >> > You got it. After commenting out perform_tests(), the grub2 menu showed
> >> > and the system booted.
> >> > 
> >> > It seems that perform_tests() cleared 0x400000~0x800000, and that's why 
> >> > the members of hvm_start_info became 0 in my test.
> >> 
> >> So could you give the below/attached patch a try?
> >> 
> > It won't compile.
> > 
> > tests.c: In function 'perform_tests':
> > tests.c:248:50: error: 'i' may be used uninitialized in this function 
> > [-Werror=maybe-uninitialized]
> >          if ( TEST_MEM_BASE < (uintptr_t)(modlist + i) &&
> >                                                   ^
> > cc1: all warnings being treated as errors
> 
> Oops - quite obviously. No idea why neither of the two gcc versions
> I've built this with caught the issue. Below/attached a better one (also
> with a few other changes).
> 
This patch works for me :)

Thanks,

Gary Lin

> Jan
> 
> --- a/tools/firmware/hvmloader/tests.c
> +++ b/tools/firmware/hvmloader/tests.c
> @@ -19,7 +19,9 @@
>   * this program; If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include "config.h"
>  #include "util.h"
> +#include <xen/arch-x86/hvm/start_info.h>
>  
>  #define TEST_FAIL 0
>  #define TEST_PASS 1
> @@ -28,11 +30,13 @@
>  /*
>   * Memory layout during tests:
>   *  4MB to 8MB is cleared.
> - *  Page directory resides at 8MB.
> - *  4 page table pages reside at 8MB+4kB to 8MB+20kB.
> - *  Pagetables identity-map 0-16MB, except 4kB at va 6MB maps to pa 5MB.
> + *  Page directory resides at 4MB.
> + *  2 page table pages reside at 4MB+4kB to 4MB+12kB.
> + *  Pagetables identity-map 0-8MB, except 4kB at va 6MB maps to pa 5MB.
>   */
> -#define PD_START (8ul << 20)
> +#define TEST_MEM_BASE (4ul << 20)
> +#define TEST_MEM_SIZE (4ul << 20)
> +#define PD_START TEST_MEM_BASE
>  #define PT_START (PD_START + 4096)
>  
>  static void setup_paging(void)
> @@ -41,10 +45,10 @@ static void setup_paging(void)
>      uint32_t *pt = (uint32_t *)PT_START;
>      uint32_t i;
>  
> -    /* Identity map 0-16MB. */
> -    for ( i = 0; i < 4; i++ )
> +    /* Identity map 0-8MB. */
> +    for ( i = 0; i < 2; i++ )
>          pd[i] = (unsigned long)pt + (i<<12) + 3;
> -    for ( i = 0; i < (4*1024); i++ )
> +    for ( i = 0; i < 2 * 1024; i++ )
>          pt[i] = (i << 12) + 3;
>  
>      /* Page at virtual 6MB maps to physical 5MB. */
> @@ -112,7 +116,7 @@ static int rep_io_test(void)
>      stop_paging();
>  
>      i = 0;
> -    for ( p = (uint32_t *)0x400000ul; p < (uint32_t *)0x700000ul; p++ )
> +    for ( p = (uint32_t *)0x4ff000ul; p < (uint32_t *)0x602000ul; p++ )
>      {
>          uint32_t expected = 0;
>          if ( check[i].addr == (unsigned long)p )
> @@ -144,12 +148,12 @@ static int shadow_gs_test(void)
>      if ( !(edx & (1u<<29)) )
>          return TEST_SKIP;
>  
> -    /* Long mode pagetable setup: Identity map 0-16MB with 2MB mappings. */
> +    /* Long mode pagetable setup: Identity map 0-8MB with 2MB mappings. */
>      *pd = (unsigned long)pd + 0x1007; /* Level 4 */
>      pd += 512;
>      *pd = (unsigned long)pd + 0x1007; /* Level 3 */
>      pd += 512;
> -    for ( i = 0; i < 8; i++ )         /* Level 2 */
> +    for ( i = 0; i < 4; i++ )         /* Level 2 */
>          *pd++ = (i << 21) + 0x1e3;
>  
>      asm volatile (
> @@ -191,8 +195,7 @@ static int shadow_gs_test(void)
>  
>  void perform_tests(void)
>  {
> -    int i, passed, skipped;
> -
> +    unsigned int i, passed, skipped;
>      static struct {
>          int (* const test)(void);
>          const char *description;
> @@ -204,12 +207,80 @@ void perform_tests(void)
>  
>      printf("Testing HVM environment:\n");
>  
> -    if ( hvm_info->low_mem_pgend < 0x1000 )
> +    BUILD_BUG_ON(SCRATCH_PHYSICAL_ADDRESS > HVMLOADER_PHYSICAL_ADDRESS);
> +    if ( hvm_info->low_mem_pgend <
> +         ((TEST_MEM_BASE + TEST_MEM_SIZE) >> PAGE_SHIFT) )
> +    {
> +        printf("Skipping tests due to insufficient memory (<%luMB)\n",
> +               (TEST_MEM_BASE + TEST_MEM_SIZE) >> 20);
> +        return;
> +    }
> +
> +    if ( (unsigned long)_end > TEST_MEM_BASE )
> +    {
> +        printf("Skipping tests due to overlap with base image\n");
> +        return;
> +    }
> +
> +    if ( hvm_start_info->cmdline_paddr &&
> +         hvm_start_info->cmdline_paddr < TEST_MEM_BASE + TEST_MEM_SIZE &&
> +         ((hvm_start_info->cmdline_paddr +
> +           strlen((char *)(uintptr_t)hvm_start_info->cmdline_paddr)) >=
> +          TEST_MEM_BASE) )
> +    {
> +        printf("Skipping tests due to overlap with command line\n");
> +        return;
> +    }
> +
> +    if ( hvm_start_info->rsdp_paddr )
>      {
> -        printf("Skipping tests due to insufficient memory (<16MB)\n");
> +        printf("Skipping tests due to non-zero RSDP address\n");
>          return;
>      }
>  
> +    if ( hvm_start_info->nr_modules )
> +    {
> +        const struct hvm_modlist_entry *modlist =
> +            (void *)(uintptr_t)hvm_start_info->modlist_paddr;
> +
> +        if ( hvm_start_info->modlist_paddr > UINTPTR_MAX ||
> +             ((UINTPTR_MAX - (uintptr_t)modlist) / sizeof(*modlist) <
> +              hvm_start_info->nr_modules) )
> +        {
> +            printf("Skipping tests due to inaccessible module list\n");
> +            return;
> +        }
> +
> +        if ( TEST_MEM_BASE < (uintptr_t)(modlist +
> +                                         hvm_start_info->nr_modules) &&
> +             (uintptr_t)modlist < TEST_MEM_BASE + TEST_MEM_SIZE )
> +        {
> +            printf("Skipping tests due to overlap with module list\n");
> +            return;
> +        }
> +
> +        for ( i = 0; i < hvm_start_info->nr_modules; ++i )
> +        {
> +            if ( TEST_MEM_BASE < modlist[i].paddr + modlist[i].size &&
> +                 modlist[i].paddr < TEST_MEM_BASE + TEST_MEM_SIZE )
> +            {
> +                printf("Skipping tests due to overlap with module %u\n", i);
> +                return;
> +            }
> +
> +            if ( modlist[i].cmdline_paddr &&
> +                 modlist[i].cmdline_paddr < TEST_MEM_BASE + TEST_MEM_SIZE &&
> +                 ((modlist[i].cmdline_paddr +
> +                   strlen((char *)(uintptr_t)modlist[i].cmdline_paddr)) >=
> +                  TEST_MEM_BASE) )
> +            {
> +                printf("Skipping tests due to overlap with module %u cmdline\n",
> +                       i);
> +                return;
> +            }
> +        }
> +    }
> +
>      passed = skipped = 0;
>      for ( i = 0; tests[i].test; i++ )
>      {
> 
> 

> hvmloader: avoid tests when they would clobber used memory
> 
> First of all limit the memory range used for testing to 4Mb: There's no
> point placing page tables right above 8Mb when they can equally well
> live at the bottom of the chunk at 4Mb - rep_io_test() cares about the
> 5Mb...7Mb range only anyway. In a subsequent patch this will then also
> allow simply looking for an unused 4Mb range (instead of using a build
> time determined one).
> 
> Extend the "skip tests" condition beyond the "is there enough memory"
> question.
> 
> Reported-by: Charles Arnold <carnold@suse.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/tools/firmware/hvmloader/tests.c
> +++ b/tools/firmware/hvmloader/tests.c
> @@ -19,7 +19,9 @@
>   * this program; If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include "config.h"
>  #include "util.h"
> +#include <xen/arch-x86/hvm/start_info.h>
>  
>  #define TEST_FAIL 0
>  #define TEST_PASS 1
> @@ -28,11 +30,13 @@
>  /*
>   * Memory layout during tests:
>   *  4MB to 8MB is cleared.
> - *  Page directory resides at 8MB.
> - *  4 page table pages reside at 8MB+4kB to 8MB+20kB.
> - *  Pagetables identity-map 0-16MB, except 4kB at va 6MB maps to pa 5MB.
> + *  Page directory resides at 4MB.
> + *  2 page table pages reside at 4MB+4kB to 4MB+12kB.
> + *  Pagetables identity-map 0-8MB, except 4kB at va 6MB maps to pa 5MB.
>   */
> -#define PD_START (8ul << 20)
> +#define TEST_MEM_BASE (4ul << 20)
> +#define TEST_MEM_SIZE (4ul << 20)
> +#define PD_START TEST_MEM_BASE
>  #define PT_START (PD_START + 4096)
>  
>  static void setup_paging(void)
> @@ -41,10 +45,10 @@ static void setup_paging(void)
>      uint32_t *pt = (uint32_t *)PT_START;
>      uint32_t i;
>  
> -    /* Identity map 0-16MB. */
> -    for ( i = 0; i < 4; i++ )
> +    /* Identity map 0-8MB. */
> +    for ( i = 0; i < 2; i++ )
>          pd[i] = (unsigned long)pt + (i<<12) + 3;
> -    for ( i = 0; i < (4*1024); i++ )
> +    for ( i = 0; i < 2 * 1024; i++ )
>          pt[i] = (i << 12) + 3;
>  
>      /* Page at virtual 6MB maps to physical 5MB. */
> @@ -112,7 +116,7 @@ static int rep_io_test(void)
>      stop_paging();
>  
>      i = 0;
> -    for ( p = (uint32_t *)0x400000ul; p < (uint32_t *)0x700000ul; p++ )
> +    for ( p = (uint32_t *)0x4ff000ul; p < (uint32_t *)0x602000ul; p++ )
>      {
>          uint32_t expected = 0;
>          if ( check[i].addr == (unsigned long)p )
> @@ -144,12 +148,12 @@ static int shadow_gs_test(void)
>      if ( !(edx & (1u<<29)) )
>          return TEST_SKIP;
>  
> -    /* Long mode pagetable setup: Identity map 0-16MB with 2MB mappings. */
> +    /* Long mode pagetable setup: Identity map 0-8MB with 2MB mappings. */
>      *pd = (unsigned long)pd + 0x1007; /* Level 4 */
>      pd += 512;
>      *pd = (unsigned long)pd + 0x1007; /* Level 3 */
>      pd += 512;
> -    for ( i = 0; i < 8; i++ )         /* Level 2 */
> +    for ( i = 0; i < 4; i++ )         /* Level 2 */
>          *pd++ = (i << 21) + 0x1e3;
>  
>      asm volatile (
> @@ -191,8 +195,7 @@ static int shadow_gs_test(void)
>  
>  void perform_tests(void)
>  {
> -    int i, passed, skipped;
> -
> +    unsigned int i, passed, skipped;
>      static struct {
>          int (* const test)(void);
>          const char *description;
> @@ -204,12 +207,80 @@ void perform_tests(void)
>  
>      printf("Testing HVM environment:\n");
>  
> -    if ( hvm_info->low_mem_pgend < 0x1000 )
> +    BUILD_BUG_ON(SCRATCH_PHYSICAL_ADDRESS > HVMLOADER_PHYSICAL_ADDRESS);
> +    if ( hvm_info->low_mem_pgend <
> +         ((TEST_MEM_BASE + TEST_MEM_SIZE) >> PAGE_SHIFT) )
> +    {
> +        printf("Skipping tests due to insufficient memory (<%luMB)\n",
> +               (TEST_MEM_BASE + TEST_MEM_SIZE) >> 20);
> +        return;
> +    }
> +
> +    if ( (unsigned long)_end > TEST_MEM_BASE )
> +    {
> +        printf("Skipping tests due to overlap with base image\n");
> +        return;
> +    }
> +
> +    if ( hvm_start_info->cmdline_paddr &&
> +         hvm_start_info->cmdline_paddr < TEST_MEM_BASE + TEST_MEM_SIZE &&
> +         ((hvm_start_info->cmdline_paddr +
> +           strlen((char *)(uintptr_t)hvm_start_info->cmdline_paddr)) >=
> +          TEST_MEM_BASE) )
> +    {
> +        printf("Skipping tests due to overlap with command line\n");
> +        return;
> +    }
> +
> +    if ( hvm_start_info->rsdp_paddr )
>      {
> -        printf("Skipping tests due to insufficient memory (<16MB)\n");
> +        printf("Skipping tests due to non-zero RSDP address\n");
>          return;
>      }
>  
> +    if ( hvm_start_info->nr_modules )
> +    {
> +        const struct hvm_modlist_entry *modlist =
> +            (void *)(uintptr_t)hvm_start_info->modlist_paddr;
> +
> +        if ( hvm_start_info->modlist_paddr > UINTPTR_MAX ||
> +             ((UINTPTR_MAX - (uintptr_t)modlist) / sizeof(*modlist) <
> +              hvm_start_info->nr_modules) )
> +        {
> +            printf("Skipping tests due to inaccessible module list\n");
> +            return;
> +        }
> +
> +        if ( TEST_MEM_BASE < (uintptr_t)(modlist +
> +                                         hvm_start_info->nr_modules) &&
> +             (uintptr_t)modlist < TEST_MEM_BASE + TEST_MEM_SIZE )
> +        {
> +            printf("Skipping tests due to overlap with module list\n");
> +            return;
> +        }
> +
> +        for ( i = 0; i < hvm_start_info->nr_modules; ++i )
> +        {
> +            if ( TEST_MEM_BASE < modlist[i].paddr + modlist[i].size &&
> +                 modlist[i].paddr < TEST_MEM_BASE + TEST_MEM_SIZE )
> +            {
> +                printf("Skipping tests due to overlap with module %u\n", i);
> +                return;
> +            }
> +
> +            if ( modlist[i].cmdline_paddr &&
> +                 modlist[i].cmdline_paddr < TEST_MEM_BASE + TEST_MEM_SIZE &&
> +                 ((modlist[i].cmdline_paddr +
> +                   strlen((char *)(uintptr_t)modlist[i].cmdline_paddr)) >=
> +                  TEST_MEM_BASE) )
> +            {
> +                printf("Skipping tests due to overlap with module %u cmdline\n",
> +                       i);
> +                return;
> +            }
> +        }
> +    }
> +
>      passed = skipped = 0;
>      for ( i = 0; tests[i].test; i++ )
>      {


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel