Re: [Xen-devel] [PATCH v2 3/6] libxc: allow HVM guest to have modules

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Jun 25, 2018 at 12:35:30PM +0100, Wei Liu wrote:
> Lift the loading code out of PVH specific branch. Take the chance to
> make the debug message more useful.

Would be nice to mention that you also account for virt_base being
unset. 

> IPXE will be loaded as a module of Rombios.
> 
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

> ---
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> ---
>  tools/libxc/xc_dom_x86.c | 32 ++++++++++++++++++--------------
>  1 file changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
> index e33a28847d..ed4973a997 100644
> --- a/tools/libxc/xc_dom_x86.c
> +++ b/tools/libxc/xc_dom_x86.c
> @@ -1698,20 +1698,6 @@ static int bootlate_hvm(struct xc_dom_image *dom)
>                                  ((uintptr_t)cmdline - (uintptr_t)start_info);
>          }
>  
> -        for ( i = 0; i < dom->num_modules; i++ )
> -        {
> -            struct xc_hvm_firmware_module mod;
> -
> -            DOMPRINTF("Adding module %u", i);
> -            mod.guest_addr_out =
> -                dom->modules[i].seg.vstart - dom->parms.virt_base;
> -            mod.length =
> -                dom->modules[i].seg.vend - dom->modules[i].seg.vstart;
> -
> -            add_module_to_list(dom, &mod, dom->modules[i].cmdline,
> -                               modlist, start_info);
> -        }
> -
>          /* ACPI module 0 is the RSDP */
>          start_info->rsdp_paddr = dom->acpi_modules[0].guest_addr_out ? : 0;
>      }
> @@ -1721,6 +1707,24 @@ static int bootlate_hvm(struct xc_dom_image *dom)
>                             modlist, start_info);
>      }
>  
> +    for ( i = 0; i < dom->num_modules; i++ )
> +    {
> +        struct xc_hvm_firmware_module mod;
> +        uint64_t base = dom->parms.virt_base != UNSET_ADDR ?
> +            dom->parms.virt_base : 0;

For PVH/HVM virt_base should always be UNSET_ADDR and vstart/vend
should be a physical address (ie: rename to pstart/pend). I guess
other bits of the code already use virt_base if present so we need to
use it here to in order to compensate.

Roger.

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