[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 2/6] vTPM: add HVM_PARAM_STUBDOM_VTPM parameter for HVM virtual machine




> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> Sent: Thursday, October 30, 2014 8:17 PM
> To: Xu, Quan; xen-devel@xxxxxxxxxxxxx
> Cc: keir@xxxxxxx; ian.campbell@xxxxxxxxxx; tim@xxxxxxx;
> ian.jackson@xxxxxxxxxxxxx; jbeulich@xxxxxxxx
> Subject: Re: [Xen-devel] [PATCH 2/6] vTPM: add
> HVM_PARAM_STUBDOM_VTPM parameter for HVM virtual machine
> 
> On 30/10/14 12:05, Xu, Quan wrote:
> >
> >> -----Original Message-----
> >> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> >> Sent: Thursday, October 30, 2014 7:50 PM
> >> To: Xu, Quan; xen-devel@xxxxxxxxxxxxx
> >> Cc: keir@xxxxxxx; ian.campbell@xxxxxxxxxx; tim@xxxxxxx;
> >> ian.jackson@xxxxxxxxxxxxx; jbeulich@xxxxxxxx
> >> Subject: Re: [Xen-devel] [PATCH 2/6] vTPM: add
> HVM_PARAM_STUBDOM_VTPM
> >> parameter for HVM virtual machine
> >>
> >> On 30/10/14 07:38, Quan Xu wrote:
> >>> Signed-off-by: Quan Xu <quan.xu@xxxxxxxxx>
> >> What is the purpose of this parameter?  A patch like this is
> >> currently unacceptable, especially as the libxl hunk indicates that
> >> the parameter name does not match whatever information you are putting
> into it.
> >>
> > Thanks for your suggestion.
> > This parameter tell the Qemu whether to register Qemu vTPM frontend in
> xen_hvm_init().
> > Qemu will get the parameter value by xc_get_hvm_param(). How can I change
> it?
> 
> This is surely something which should be a command line parameter to qemu, or
> perhaps for qemu to read out of xenstore.
> 
> An HVM param is entirely inappropriate for this purpose, in my opinion.

Appreciate your advice, the below is the idea why I enable it with an hvm param.
Actually there are some command line parameter to qemu in patch
*[PATCH 5/6] vTPM: add vTPM device for HVM virtual machine*
+        flexarray_vappend(dm_args, "-tpmdev",
+                          "xenstubdoms,id=xenvtpm0", NULL);
+        flexarray_vappend(dm_args,"-device",
+                          "tpm-tis,tpmdev=xenvtpm0", NULL);

Qemu registers xen backend with 'xen_be_register()' in xen_hvm_init() 
[Qemu/xen-hvm.c]. 
Such as some existing devices,' console'/'vkbd'/'qdisk'.
## Qemu source code in xen_hvm_init() [Qemu/xen-hvm.c]##
    xen_be_register("console", &xen_console_ops);
    xen_be_register("vkbd", &xen_kbdmouse_ops);
    xen_be_register("qdisk", &xen_blkdev_ops);
####
So xen_vtpm_register() is also a xen backend. it should align with it.

Reading out of xenstore is also a common way to get value parameters. Try to 
align with 
Qemu, there are a lot of 'xc_get_hvm_param()'  to get ' HVM_PARAM_IOREQ_PFN ' /
HVM_PARAM_BUFIOREQ_PFN / HVM_PARAM_BUFIOREQ_EVTCHN
in xen_hvm_init() [Qemu/xen-hvm.c]. 

I should commit Qemu patch ASAP too. I will CC you when I commit it in next 
days.
I will modify it if it's better to for qemu to read out of xenstore.


Quan
> 
> ~Andrew

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.