Re: [Xen-devel] [PATCH RFC v1 21/74] x86/entry: Early PVH boot code

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Jan 05, 2018 at 06:32:56AM -0700, Jan Beulich wrote:
> > +    module_t *mod;
> > +    unsigned int i;
> > +
> > +    ASSERT(pvh_info->magic == XEN_HVM_START_MAGIC_VALUE);
> > +
> > +    /*
> > +     * Turn hvm_start_info into mbi. Luckily all modules are placed under 
> > 4GB
> > +     * boundary on x86.
> 
> ISTR having that discussion relatively recently in another context:
> All the header states is "NB: Xen on x86 will always try to place all
> the data below the 4GiB boundary." Note the "try to". Hence I
> think ...
> 
> > +     */
> > +    pvh_mbi.flags = MBI_CMDLINE | MBI_MODULES | MBI_LOADERNAME;
> > +
> > +    ASSERT(!(pvh_info->cmdline_paddr >> 32));
> 
> ... this, if we don't want to handle the case, should be BUG_ON() or
> panic() (same further down).
> 
> > +    pvh_mbi.cmdline = pvh_info->cmdline_paddr;
> > +    pvh_mbi.boot_loader_name = __pa(pvh_loader);
> > +
> > +    ASSERT(pvh_info->nr_modules < 32);
> 
> ARRAY_SIZE(pvh_mbi_mods) and perhaps again BUG_ON() or
> panic().
> 
> > +    pvh_mbi.mods_count = pvh_info->nr_modules;
> > +    pvh_mbi.mods_addr = __pa(pvh_mbi_mods);
> > +
> > +    mod = pvh_mbi_mods;
> > +    entry = __va(pvh_info->modlist_paddr);
> 
> How come __va() already works at this point in time? And what about
> this address being beyond 4Gb?
> 

The original code uses __va at the beginning of __start_xen so this is
no more erroneous than what we originally have.

We shall BUG_ON address beyond 4Gb for the time being.

> > +    for ( i = 0; i < pvh_info->nr_modules; i++ )
> > +    {
> > +        ASSERT(!(entry[i].paddr >> 32));
> 
> To relax this condition (in particular to allow huge initrd), how
> about ...
> 
> > +        mod[i].mod_start = entry[i].paddr;
> > +        mod[i].mod_end   = entry[i].paddr + entry[i].size;
> 
> ... using the EFI approach here and store the PFN in mod_start
> and the size in mod_end?


This function turns pvh_info into multiboot info. I'm afraid I don't
follow you suggestion here. The best approach now is to BUG_ON here and
consider huge initrd later.

(I will try to fix other comments where I can)

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel