Re: [PATCH v4 01/14] libxc: Rework extra module initialisation

[Anthony PERARD <anthony.perard@citrix.com>]

On Tue, Mar 15, 2016 at 08:06:44PM -0400, Konrad Rzeszutek Wilk wrote:
> On Mon, Mar 14, 2016 at 05:55:36PM +0000, Anthony PERARD wrote:
> > This patch use xc_dom_alloc_segment() to allocate the memory space for the
> > ACPI modules and the SMBIOS modules. This is to replace the arbitrary
> > placement of 1MB after the hvmloader image.
> 
> s/of/at/ ?
> 
> Or are you rather staying that the memory for ACPI modules and such
> is offset by one 1MB after hvmloader image?

I was trying to translate this in to english:
*mstart_out = MKALIGN(elf->pend, MB_ALIGN) + (MB_ALIGN);
which came with that comment:
/* Want to place the modules 1Mb+change behind the loader image. */

[...]

Actually, the current situation before this patch would be, with every bios
compiled in, and with the use of acpi_firmware:
/-----------\ <- 0MB
|   other   |
|   stuff   |
+-----------+ <- 1MB
| hvmloader |
| binary    |
+-----------| <- 3.8MB
|  empty    |
+-----------+ <- 4MB <- begining of memory used by perform_tests()
|  ...      |
+-----------+ <- 5MB
| ACPI DATA |
+-----------+ <- 5.4MB
|   ...     |
\-----------/ <- 8MB <- end of memory used by perform_tests()

I just tried to to use the options acpi_firmware, and that the result.
When hvmloader tried to load the extra acpi data, I have this:
Loading SeaBIOS ...
Creating MP tables ...
Loading ACPI ...
*** HVMLoader bug at util.c:460
*** HVMLoader crashed.

That the BUG_ON in mem_alloc(). I think it's trying to allocate memory of
size 0.



> > In later patches, while trying to load a firmware such as OVMF, the later
> 
> .. what later patches? These ones? You may want to mention which ones.

Ok, I will. That would be "hvmloader: Load OVMF from modules" and previous
ones of the patch series.

> > could easily be loaded past the address 4MB (OVMF is a 2MB binary), but
>  
> Why would it be loaded past 4MB? Why not in the space right after ACPI
> DSDT and such?


> > hvmloader use a range of memory from 4MB to 8MB to perform tests and in the
> > process, clear the memory, before loading the modules.
> 
> s/clear/clears/
> 
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> > ---
> > Changes in V4:
> > - check that guest_addr_out have a reasonable value.
> > 
> > New patch in V3.
> > ---
> >  tools/libxc/xc_dom_hvmloader.c | 131 ++++++++++++-----------------------------
> >  1 file changed, 38 insertions(+), 93 deletions(-)
> > 
> > diff --git a/tools/libxc/xc_dom_hvmloader.c b/tools/libxc/xc_dom_hvmloader.c
> > index 330d5e8..7cf5854 100644
> > --- a/tools/libxc/xc_dom_hvmloader.c
> > +++ b/tools/libxc/xc_dom_hvmloader.c

[...]

> >  
> > - error_out:
> > -    free(entries);
> > +    rc = module_init_one(dom, &dom->acpi_module, "acpi module");
> > +    if ( rc ) goto err;
> > +    rc = module_init_one(dom, &dom->smbios_module, "smbios module");
> > +    if ( rc ) goto err;
> 
> You can't be that code thought. That is some nice deletion of code.
> 
> Now is this safe for migration?
> I presume so since this is all E820_RESERVED righT? No funny business
> there?

The acpi module is moved to another location by hvmloader. I guess that the
same for the smbios module. So I think it's safe for migration, and it
does not change anything once hvmloader has finished.

-- 
Anthony PERARD

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