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

["Roger Pau Monné" <roger.pau@citrix.com>]

El 07/08/15 a les 19.30, Andrew Cooper ha escrit:
> 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@citrix.com>
>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> Cc: Ian Campbell <ian.campbell@citrix.com>
>> Cc: Wei Liu <wei.liu2@citrix.com>
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>>  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.

I agree, using two pages was overkill for this, specially taking into
account that the command line cannot exceed 1024 chars.

The above structure looks fine, I would note that the command line is
limited to 1024 characters, so that leaves us with a maximum of 190
modules if we want to limit all this metainfo to one page only, which
IMHO is more than enough.

Roger.

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