Re: [Qemu-devel] [PATCH 2/4] Qemu-Xen-vTPM: Register Xen stubdom vTPM frontend driver

[Stefano Stabellini]

On Thu, 6 Nov 2014, Xu, Quan wrote:
> > -----Original Message-----
> > From: Stefano Stabellini [mailto:address@hidden
> > Sent: Thursday, November 06, 2014 11:42 PM
> > To: Xu, Quan
> > Cc: Stefano Stabellini; address@hidden; address@hidden
> > Subject: RE: [PATCH 2/4] Qemu-Xen-vTPM: Register Xen stubdom vTPM
> > frontend driver
> > 
> > On Thu, 6 Nov 2014, Xu, Quan wrote:
> > > > -----Original Message-----
> > > > From: Stefano Stabellini [mailto:address@hidden
> > > > Sent: Monday, November 03, 2014 7:54 PM
> > > > To: Xu, Quan
> > > > Cc: address@hidden; address@hidden;
> > > > address@hidden
> > > > Subject: Re: [PATCH 2/4] Qemu-Xen-vTPM: Register Xen stubdom vTPM
> > > > frontend driver
> > > >
> > > > On Sun, 2 Nov 2014, Quan Xu wrote:
> > > > > This drvier transfers any request/repond between TPM xenstubdoms
> > > > > driver and Xen vTPM stubdom, and facilitates communications
> > > > > between Xen vTPM stubdom domain and vTPM xenstubdoms driver
> > > > >
> > > > > Signed-off-by: Quan Xu <address@hidden>
> > > >
> > > > Please describe what changes did make to xen_backend.c and why.
> > > > The commit message should contains info on all the changes made by
> > > > the patch below.
> > > >
> > >
> > > The following is code process when Qemu is running with Xen.
> > > ##code process
> > > [...]
> > >  xen_hvm_init()
> > >     -->xen_be_register()
> > >         -->xenstore_scan()
> > >             -->xen_be_check_state()
> > >
> > >     -->xen_vtpm_register()
> > >
> > > ideally, I can register 'vtpm' via xen_vtpm_register() as
> > >
> > > + xen_be_register("console", &xen_console_ops);
> > > + xen_be_register("vkbd", &xen_kbdmouse_ops); xen_be_register("qdisk",
> > > + &xen_blkdev_ops);
> > >
> > > but there are 2 reasons why I add xen_vtpm_register(), instead of
> > > xen_be_register().
> > >
> > > 1. The backend of TPM is runing in a Xen stubDom, not Domain 0.
> > > some functions are not working, for example 'setup watch' and 'look
> > > for backend' in xenstore_scan()
> > >
> > > 2. there is a thread runing in Xen stubDom [event_thread()], it will
> > > handle backend status when the frontend is initialized. It is not
> > > compatible with xen_be_check_state(). xen_be_check_state() always
> > > tries to modify the status of backend.
> > >
> > > as there is always a tradeoff, if I force to integrate this case into
> > > xen_be_register(), there are maybe a lot of 'if ... else.. '. It will
> > > break the code architecture. Also I should leverage existing source
> > > code with minimum modifcation. i add 'DEVOPS_FLAG_STUBDOM_BE' flag
> > in
> > > include/hw/xen/xen_backend.h to indicate that device backend is Xen
> > > stubDom.
> > 
> > Given that xen_vtpm_register is actually registering a frontend, not a
> > backend, you cannot use xen_be_register for it.
> > 
> > However instead of introducing xen_vtpm_register, I think you should be
> > adding a generic xen_fe_register function that handle any Xen PV frontend
> > registration. It should also be able to handle backends not in Dom0. Then 
> > you
> > can call:
> > 
> > xen_fe_register("console", &xen_vtpm_ops);
> 
>   xen_fe_register("vtpm", &xen_vtpm_ops); ?

Yes, that is what I meant, sorry.

> A good solution, I will try to add a generic xen_fe_register function that 
> handle any Xen PV frontend in v2.

Cool, thanks.