Re: [Xen-devel] [PATCH v4 28/31] libxc/xen: introduce HVM_PARAM_CMDLINE_PFN

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 07/08/15 11:18, Roger Pau Monne wrote:
> This HVM parameter returns a PFN that contains the address of the memory
> page where the guest command line has been placed.
>
> Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
>  tools/libxc/xc_dom_x86.c        | 17 +++++++++++++++++
>  xen/arch/x86/hvm/hvm.c          |  2 ++
>  xen/include/public/hvm/params.h |  5 ++++-
>  3 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
> index 87bce6e..369745d 100644
> --- a/tools/libxc/xc_dom_x86.c
> +++ b/tools/libxc/xc_dom_x86.c
> @@ -562,6 +562,23 @@ static int alloc_magic_pages_hvm(struct xc_dom_image 
> *dom)
>      xc_hvm_param_set(xch, domid, HVM_PARAM_SHARING_RING_PFN,
>                       special_pfn(SPECIALPAGE_SHARING));
>  
> +    if ( dom->cmdline )
> +    {
> +        xen_pfn_t cmdline_pfn = xc_dom_alloc_page(dom, "command line");
> +        char *cmdline = xc_map_foreign_range(xch, domid, PAGE_SIZE,
> +                                             PROT_READ | PROT_WRITE,
> +                                             cmdline_pfn);
> +        if ( cmdline == NULL ) {

Mix of coding styles.

> +            DOMPRINTF("Unable to map command line page");
> +            goto error_out;
> +        }
> +
> +        strncpy(cmdline, dom->cmdline, MAX_GUEST_CMDLINE);
> +        cmdline[MAX_GUEST_CMDLINE - 1] = '\0';
> +        munmap(cmdline, PAGE_SIZE);
> +        xc_hvm_param_set(xch, domid, HVM_PARAM_CMDLINE_PFN, cmdline_pfn);

I am still very much on the fence about the use of HVM parameters here,
especially as this is one-shot information which will be freed by the
guest boot process.

How much of an inconvenience would it be to specify:

%ebx is a phyaddr of a structure looking like:

struct dmlike_start_info {
    uint32_t magic;
    uint32_t flags; /* so as not to shoot ourselves in the foot */
    uint32_t cmdline_paddr;
    uint32_t nr_modules;
    uint32_t modlist_paddr;
};

And a

struct dmlite_modlist_entry {
    uint64_t paddr;
    uint64_t size;
};

This avoids enforcing page alignment on the two bits of information.  I
don't forsee the structures needing to change in the future, and in the
typical case can fit in a few hundred bytes rather than two pages.

~Andrew

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