diff mbox

[V3,8/29] tools/libxl: create vIOMMU during domain construction

Message ID 1506049330-11196-9-git-send-email-tianyu.lan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

lan,Tianyu Sept. 22, 2017, 3:01 a.m. UTC
From: Chao Gao <chao.gao@intel.com>

If guest is configured to have a vIOMMU, create it during domain construction.

Signed-off-by: Chao Gao <chao.gao@intel.com>
Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>

---
v3:
 - Remove the process of querying capabilities.
---
 tools/libxl/libxl_x86.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Roger Pau Monne Oct. 19, 2017, 10:13 a.m. UTC | #1
On Thu, Sep 21, 2017 at 11:01:49PM -0400, Lan Tianyu wrote:
> From: Chao Gao <chao.gao@intel.com>
> 
> If guest is configured to have a vIOMMU, create it during domain construction.
> 
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> 
> ---
> v3:
>  - Remove the process of querying capabilities.
> ---
>  tools/libxl/libxl_x86.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
> index 23c9a55..25cae5f 100644
> --- a/tools/libxl/libxl_x86.c
> +++ b/tools/libxl/libxl_x86.c
> @@ -341,8 +341,25 @@ int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
>      if (d_config->b_info.type == LIBXL_DOMAIN_TYPE_HVM) {
>          unsigned long shadow = DIV_ROUNDUP(d_config->b_info.shadow_memkb,
>                                             1024);
> +        int i;

unsigned int.

> +
>          xc_shadow_control(ctx->xch, domid, XEN_DOMCTL_SHADOW_OP_SET_ALLOCATION,
>                            NULL, 0, &shadow, 0, NULL);
> +
> +        for (i = 0; i < d_config->b_info.num_viommus; i++) {
> +            uint32_t id;
> +            libxl_viommu_info *viommu = d_config->b_info.viommu + i;

Since this is an array I would rather prefer that you use
&d_config->b_info.viommu[i].

> +
> +            if (viommu->type == LIBXL_VIOMMU_TYPE_INTEL_VTD) {
> +                ret = xc_viommu_create(ctx->xch, domid, VIOMMU_TYPE_INTEL_VTD,
> +                                       viommu->base_addr, viommu->cap, &id);

As said in another patch: this will break compilation because
xc_viommu_create is introduced in patch 9.

Please organize the patches in a way that the code always compiles and
works fine. Keep in mind that the Xen tree should be bisectable
always.

Roger.
Wei Liu Oct. 26, 2017, 12:05 p.m. UTC | #2
On Thu, Oct 19, 2017 at 11:13:57AM +0100, Roger Pau Monné wrote:
> > +
> > +            if (viommu->type == LIBXL_VIOMMU_TYPE_INTEL_VTD) {
> > +                ret = xc_viommu_create(ctx->xch, domid, VIOMMU_TYPE_INTEL_VTD,
> > +                                       viommu->base_addr, viommu->cap, &id);
> 
> As said in another patch: this will break compilation because
> xc_viommu_create is introduced in patch 9.
> 
> Please organize the patches in a way that the code always compiles and
> works fine. Keep in mind that the Xen tree should be bisectable
> always.
> 

+10 to this.

We rely heavily on our test system's bisector to tell us what is wrong.
The bisector works on patch level. Please make sure every patch builds,
otherwise the test system will just give up.

If triaging can be done automatically by computers, maintainers can
spend less time doing tedious work and more time reviewing patches
(yours included).

Normally I use git-rebase to build every commit, but I figured that's a
bit too dangerous so I wrote a script.

Please check out:

  [PATCH v3 for-4.10] scripts: introduce a script for build test

It is still under review, but you can fish out some of the runes to do
build tests.

Wei.
lan,Tianyu Oct. 27, 2017, 1:58 a.m. UTC | #3
On 2017年10月26日 20:05, Wei Liu wrote:
> On Thu, Oct 19, 2017 at 11:13:57AM +0100, Roger Pau Monné wrote:
>>> +
>>> +            if (viommu->type == LIBXL_VIOMMU_TYPE_INTEL_VTD) {
>>> +                ret = xc_viommu_create(ctx->xch, domid, VIOMMU_TYPE_INTEL_VTD,
>>> +                                       viommu->base_addr, viommu->cap, &id);
>>
>> As said in another patch: this will break compilation because
>> xc_viommu_create is introduced in patch 9.
>>
>> Please organize the patches in a way that the code always compiles and
>> works fine. Keep in mind that the Xen tree should be bisectable
>> always.
>>
> 
> +10 to this.
> 
> We rely heavily on our test system's bisector to tell us what is wrong.
> The bisector works on patch level. Please make sure every patch builds,
> otherwise the test system will just give up.
> 
> If triaging can be done automatically by computers, maintainers can
> spend less time doing tedious work and more time reviewing patches
> (yours included).

Sure. Will pay more attention on this.

> 
> Normally I use git-rebase to build every commit, but I figured that's a
> bit too dangerous so I wrote a script.
> 
> Please check out:
> 
>   [PATCH v3 for-4.10] scripts: introduce a script for build test
> 
> It is still under review, but you can fish out some of the runes to do
> build tests.

This is very helpful. Thanks.
diff mbox

Patch

diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index 23c9a55..25cae5f 100644
--- a/tools/libxl/libxl_x86.c
+++ b/tools/libxl/libxl_x86.c
@@ -341,8 +341,25 @@  int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
     if (d_config->b_info.type == LIBXL_DOMAIN_TYPE_HVM) {
         unsigned long shadow = DIV_ROUNDUP(d_config->b_info.shadow_memkb,
                                            1024);
+        int i;
+
         xc_shadow_control(ctx->xch, domid, XEN_DOMCTL_SHADOW_OP_SET_ALLOCATION,
                           NULL, 0, &shadow, 0, NULL);
+
+        for (i = 0; i < d_config->b_info.num_viommus; i++) {
+            uint32_t id;
+            libxl_viommu_info *viommu = d_config->b_info.viommu + i;
+
+            if (viommu->type == LIBXL_VIOMMU_TYPE_INTEL_VTD) {
+                ret = xc_viommu_create(ctx->xch, domid, VIOMMU_TYPE_INTEL_VTD,
+                                       viommu->base_addr, viommu->cap, &id);
+                if (ret) {
+                    LOGED(ERROR, domid, "create vIOMMU fail");
+                    ret = ERROR_FAIL;
+                    goto out;
+                }
+            }
+        }
     }
 
     if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV &&