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

Re: [Xen-devel] [PATCH 06/25 v7] xen/arm: vpl011: Add a new domctl API to initialize vpl011



Hi Bhupinder,

On 07/08/17 09:52, Bhupinder Thakur wrote:
diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h
index 5e1fc60..784ec7f 100644
--- a/tools/libxl/libxl_arch.h
+++ b/tools/libxl/libxl_arch.h
@@ -44,6 +44,13 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
                                       libxl_domain_build_info *info,
                                       struct xc_dom_image *dom);

+/* perform any pending hardware initialization */
+_hidden
+int libxl__arch_build_dom_finish(libxl__gc *gc,
+                                 libxl_domain_build_info *info,
+                                 struct xc_dom_image *dom,
+                                 libxl__domain_build_state *state);
+
 /* build vNUMA vmemrange with arch specific information */
 _hidden
 int libxl__arch_vnuma_build_vmemrange(libxl__gc *gc,
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index d842d88..a33d3c9 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -1038,6 +1038,27 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc 
*gc,
     return 0;
 }

+int libxl__arch_build_dom_finish(libxl__gc *gc,
+                                 libxl_domain_build_info *info,
+                                 struct xc_dom_image *dom,
+                                 libxl__domain_build_state *state)
+{
+    int ret = 0;
+
+    if (info->arch_arm.vuart == LIBXL_VUART_TYPE_SBSA_UART) {

NIT: You could avoid on level of indentation if you do:

if ( info->arch_arm.vuart != LIBXL_VUART_TYPE_SBSA_UART )
  return 0;

....

+        ret = xc_dom_vuart_init(CTX->xch,
+                                XEN_DOMCTL_VUART_TYPE_VPL011,
+                                dom->guest_domid,
+                                dom->console_domid,
+                                dom->vuart_gfn,
+                                &state->vuart_port);
+        if (ret < 0)
+            LOG(ERROR, "xc_dom_vuart_init failed\n");
+    }
+
+    return ret;
+}
+
 int libxl__arch_vnuma_build_vmemrange(libxl__gc *gc,
                                       uint32_t domid,
                                       libxl_domain_build_info *info,

[...]

diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
index db6838d..c7f650e 100644
--- a/xen/arch/arm/domctl.c
+++ b/xen/arch/arm/domctl.c
@@ -5,9 +5,11 @@
  */

 #include <xen/errno.h>
+#include <xen/guest_access.h>
 #include <xen/hypercall.h>
 #include <xen/iocap.h>
 #include <xen/lib.h>
+#include <xen/mm.h>
 #include <xen/sched.h>
 #include <xen/types.h>
 #include <xsm/xsm.h>
@@ -119,6 +121,46 @@ long arch_do_domctl(struct xen_domctl *domctl, struct 
domain *d,
         d->disable_migrate = domctl->u.disable_migrate.disable;
         return 0;

+    case XEN_DOMCTL_vuart_op:
+    {
+        int rc;
+        struct xen_domctl_vuart_op *vuart_op = &domctl->u.vuart_op;
+
+        switch(vuart_op->cmd)
+        {
+        case XEN_DOMCTL_VUART_OP_INIT:
+
+            if ( !d->creation_finished )
+            {
+                if (vuart_op->type == XEN_DOMCTL_VUART_TYPE_VPL011)

Coding style.

+                {
+                        struct vpl011_init_info info;

The indentation is wrong.

+
+                        info.console_domid = vuart_op->console_domid;
+                        info.gfn = _gfn(vuart_op->gfn);
+
+                        rc = domain_vpl011_init(d, &info);
+                        if ( !rc )
+                        {
+                            vuart_op->evtchn = info.evtchn;
+                            rc = __copy_to_guest(u_domctl, domctl, 1);
+                        }
+                }
+                else
+                        rc = -EINVAL;

I think this one should be -EOPNOTSUPP.

+            }
+            else
+                rc = - EPERM;

Can we please avoid the number of nested if? Maybe by introducing a function to handle the domctl.

+
+            break;
+
+        default:
+            rc = -EINVAL;

Same here.

+            break;
+        }
+
+        return rc;
+    }
     default:
     {
         int rc;

Cheers,

--
Julien Grall

_______________________________________________
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®.