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

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



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

 


Rackspace

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