[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [v3 2/5] Qemu-Xen-vTPM: Xen frontend driver infrastruct
From: |
Stefano Stabellini |
Subject: |
Re: [Qemu-devel] [v3 2/5] Qemu-Xen-vTPM: Xen frontend driver infrastructure |
Date: |
Thu, 29 Jan 2015 17:32:37 +0000 |
User-agent: |
Alpine 2.02 (DEB 1266 2009-07-14) |
On Thu, 29 Jan 2015, Xu, Quan wrote:
> > -----Original Message-----
> > From: Stefano Stabellini [mailto:address@hidden
> > Sent: Tuesday, January 20, 2015 1:15 AM
> > To: Xu, Quan
> > Cc: address@hidden; address@hidden;
> > address@hidden
> > Subject: Re: [v3 2/5] Qemu-Xen-vTPM: Xen frontend driver infrastructure
> >
> > On Tue, 30 Dec 2014, Quan Xu wrote:
> > > This patch adds infrastructure for xen front drivers living in qemu,
> > > so drivers don't need to implement common stuff on their own. It's
> > > mostly xenbus management stuff: some functions to access XenStore,
> > > setting up XenStore watches, callbacks on device discovery and state
> > > changes, and handle event channel between the virtual machines.
> > >
> > > Call xen_fe_register() function to register XenDevOps, and make sure,
> > > XenDevOps's flags is DEVOPS_FLAG_FE, which is flag bit to point out
> > > the XenDevOps is Xen frontend.
> > >
> > > --Changes in v3:
> > > -New xen_frontend.c file
> > > -Move xenbus_switch_state() to xen_frontend.c -Move xen_stubdom_be()
> > > to xenstore_fe_read_be_str() -Move *_stubdom_*() to *_fe_*()
> > >
> > > Signed-off-by: Quan Xu <address@hidden>
> > > ---
> > > hw/xen/Makefile.objs | 2 +-
> > > hw/xen/xen_backend.c | 11 +-
> > > hw/xen/xen_frontend.c | 323
> > +++++++++++++++++++++++++++++++++++++++++++
> > > include/hw/xen/xen_backend.h | 14 ++
> > > 4 files changed, 348 insertions(+), 2 deletions(-) create mode
> > > 100644 hw/xen/xen_frontend.c
> > >
> > > diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs index
> > > a0ca0aa..b0bb065 100644
> > > --- a/hw/xen/Makefile.objs
> > > +++ b/hw/xen/Makefile.objs
> > > @@ -1,5 +1,5 @@
> > > # xen backend driver support
> > > -common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o
> > > +common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o
> > > +xen_frontend.o
> > >
> > > obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
> > > obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o
> > > xen_pt_msi.o diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
> > > index b2cb22b..ad6e324 100644
> > > --- a/hw/xen/xen_backend.c
> > > +++ b/hw/xen/xen_backend.c
> > > @@ -275,7 +275,7 @@ static struct XenDevice *xen_be_get_xendev(const
> > > char *type, int dom, int dev,
> > > /*
> > > * release xen backend device.
> > > */
> > > -static struct XenDevice *xen_be_del_xendev(int dom, int dev)
> > > +struct XenDevice *xen_be_del_xendev(int dom, int dev)
> > > {
> > > struct XenDevice *xendev, *xnext;
> > >
> > > @@ -681,6 +681,10 @@ static void xenstore_update(void *unused)
> > > if (sscanf(vec[XS_WATCH_TOKEN], "fe:%" PRIxPTR, &ptr) == 1) {
> > > xenstore_update_fe(vec[XS_WATCH_PATH], (void*)ptr);
> > > }
> > > + if (sscanf(vec[XS_WATCH_TOKEN], "stub:%" PRIxPTR ":%d:%" PRIxPTR,
> > > + &type, &dom, &ops) == 3) {
> > > + xenstore_fe_update(vec[XS_WATCH_PATH], (void *)type, dom,
> > (void *)ops);
> > > + }
> >
> > Why "stub:"? Where is it coming from?
> >
> > I think that the xenstore_update function should be moved to a new file:
> > we don't want xen_backend.c to be handling any frontend updates.
> > Maybe you could create a new file, hw/xen/xen_pvdev.c?
> >
>
> Stefano,
> I'd like to create hw/xen/xen_pvdev.c , include/hw/xen/ xen_pvdev.h and
> include/hw/xen/xen_frontend.h
>
> include/hw/xen/ xen_pvdev.h is for
> struct XenDevOps{} and struct XenDevice {} definition and some other macro.
>
> include/hw/xen/xen_frontend.h is for
> xen frontend interface, such as xen_fe_register() .etc
>
> hw/xen/xen_pvdev.c is for
> some general parts of xen_backend/xen_frontend.
> It includes xenstore_update function xendevs management, such as
> xen_qtail_insert_xendev(..),
> xenstore_update(...) .etc.
>
> do you agree with it?
It seems reasonable.