Re: [Xen-devel] [RFC PATCH 5/23] Tools/libxc: Add viommu operations in libxc

["Lan, Tianyu" <tianyu.lan@xxxxxxxxx>]

Hi Paul:
        Sorry for later response.

On 3/31/2017 3:57 AM, Chao Gao wrote:
> On Wed, Mar 29, 2017 at 09:08:06AM +0000, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxx] On Behalf Of
> > > Chao Gao
> > > Sent: 29 March 2017 01:40
> > > To: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > Cc: Lan Tianyu <tianyu.lan@xxxxxxxxx>; Kevin Tian <kevin.tian@xxxxxxxxx>;
> > > Ian Jackson <Ian.Jackson@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxx
> > > Subject: Re: [Xen-devel] [RFC PATCH 5/23] Tools/libxc: Add viommu
> > > operations in libxc
> > >
> > > Tianyu is on vacation this two weeks, so I will try to address
> > > some comments on this series.
> > >
> > > On Tue, Mar 28, 2017 at 05:24:03PM +0100, Wei Liu wrote:
> > > > On Fri, Mar 17, 2017 at 07:27:05PM +0800, Lan Tianyu wrote:
> > > > > From: Chao Gao <chao.gao@xxxxxxxxx>
> > > > >
> > > > > In previous patch, we introduce a common vIOMMU layer. In our design,
> > > > > we create/destroy vIOMMU through DMOP interface instead of creating
> > > it
> > > > > according to a config flag of domain. It makes it is possible
> > > > > to create vIOMMU in device model or in tool stack.
> > I've not been following this closely so apologies if this has already been 
> > asked...
> >
> > Why would you need to create a vIOMMU instance in an external device model.
> > Since the toolstack should be in control of the device model configuration why 
> > would it not know in advance that one was required?
> I assume your question is why we don't create a vIOMMU instance via hypercall 
> in toolstack.
> I think creating in toolstack is also ok and is easier to be reused by pvh.
>
> If Tianyu has no concern about this, will move this part to toolstack.
We can move create/destroy vIOMMU in the tool stack but we still need to 
add such dummy vIOMMU device model in Qemu to pass virtual device's DMA 
request into Xen hypervisor. Qemu is required to use DMOP hypercall and 
tool stack may use domctl hyercall. vIOMMU hypercalls will be divided 
into two part.
Domctl:
        create, destroy and query.
DMOP:
        vDev's DMA related operations.

Is this OK?

> Thanks,
> Chao
>
> >  Paul
> >
> > > > > The following toolstack code is to add XEN_DMOP_viommu_XXX syscalls:
> > > > Hypercalls, not syscalls.
> > > >
> > > > >  - query capabilities of vIOMMU emulated by Xen
> > > > >  - create vIOMMU in Xen hypervisor with base address, capability
> > > > >  - destroy vIOMMU specified by viommu_id
> > > > >
> > > > > Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
> > > > > Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>
> > > > > ---
> > > > >  tools/libs/devicemodel/core.c                   | 69
> > > +++++++++++++++++++++++++
> > > > >  tools/libs/devicemodel/include/xendevicemodel.h | 35 +++++++++++++
> > > > >  tools/libs/devicemodel/libxendevicemodel.map    |  3 ++
> > > > >  tools/libxc/include/xenctrl_compat.h            |  5 ++
> > > > >  tools/libxc/xc_devicemodel_compat.c             | 18 +++++++
> > > > >  5 files changed, 130 insertions(+)
> > > > >
> > > > > diff --git a/tools/libs/devicemodel/core.c b/tools/libs/devicemodel/core.c
> > > > > index a85cb49..aee1150 100644
> > > > > --- a/tools/libs/devicemodel/core.c
> > > > > +++ b/tools/libs/devicemodel/core.c
> > > > Bear in mind that this library is stable, so whatever ends up here can
> > > > change in the future.
> > > >
> > > > This is not saying the following code is problematic. It is just a
> > > > general FYI.
> > > >
> > > > Obviously the toolstack side is going to follow the hypervisor
> > > > interface, so I will do a detailed review later.
> > > Sure. If the hypervisor interface settles down, we can inform you.
> > >
> > > > > +int xendevicemodel_viommu_destroy(
> > > > > +    xendevicemodel_handle *dmod, domid_t dom, uint32_t viommu_id);
> > > > >  #endif /* __XEN_TOOLS__ */
> > > > >
> > > > >  #endif /* XENDEVICEMODEL_H */
> > > > > diff --git a/tools/libs/devicemodel/libxendevicemodel.map
> > > b/tools/libs/devicemodel/libxendevicemodel.map
> > > > > index 45c773e..c2e0968 100644
> > > > > --- a/tools/libs/devicemodel/libxendevicemodel.map
> > > > > +++ b/tools/libs/devicemodel/libxendevicemodel.map
> > > > > @@ -17,6 +17,9 @@ VERS_1.0 {
> > > > >                 xendevicemodel_modified_memory;
> > > > >                 xendevicemodel_set_mem_type;
> > > > >                 xendevicemodel_inject_event;
> > > > > +               xendevicemodel_viommu_query_cap;
> > > > > +               xendevicemodel_viommu_create;
> > > > > +               xendevicemodel_viommu_destroy;
> > > > >                 xendevicemodel_restrict;
> > > > >                 xendevicemodel_close;
> > > > I suppose this series is going to miss 4.9.
> > > >
> > > > Please add these functions to VERS_1.1.
> > > Yes. We will fix this.
> > >
> > > > >         local: *; /* Do not expose anything by default */
> > > > > diff --git a/tools/libxc/include/xenctrl_compat.h
> > > b/tools/libxc/include/xenctrl_compat.h
> > > > > index 040e7b2..315c45d 100644
> > > > > --- a/tools/libxc/include/xenctrl_compat.h
> > > > > +++ b/tools/libxc/include/xenctrl_compat.h
> > > > > @@ -164,6 +164,11 @@ int xc_hvm_set_mem_type(
> > > > >  int xc_hvm_inject_trap(
> > > > >      xc_interface *xch, domid_t domid, int vcpu, uint8_t vector,
> > > > >      uint8_t type, uint32_t error_code, uint8_t insn_len, uint64_t cr2);
> > > > > +int xc_viommu_query_cap(xc_interface *xch, domid_t dom, uint64_t
> > > *cap);
> > > > > +int xc_viommu_create(
> > > > > +    xc_interface *xch, domid_t dom, uint64_t base_addr, uint64_t cap,
> > > > > +    uint32_t *viommu_id);
> > > > > +int xc_viommu_destroy(xc_interface *xch, domid_t dom, uint32_t
> > > viommu_id);
> > > > >  #endif /* XC_WANT_COMPAT_DEVICEMODEL_API */
> > > > >
> > > > > diff --git a/tools/libxc/xc_devicemodel_compat.c
> > > b/tools/libxc/xc_devicemodel_compat.c
> > > > > index e4edeea..62f703a 100644
> > > > > --- a/tools/libxc/xc_devicemodel_compat.c
> > > > > +++ b/tools/libxc/xc_devicemodel_compat.c
> > > > I don't think you need to provide compat wrappers for them. They are new
> > > > APIs.
> > > OK. Got it.
> > >
> > > Thanks,
> > > Chao
> > > > Wei.
> > > _______________________________________________
> > > Xen-devel mailing list
> > > Xen-devel@xxxxxxxxxxxxx
> > > https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel