[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH 02/10 v2] xen/arm: vpl011: Add new vuart domctl interface to setup pfn and evtchn
Hi Bhupinder,
Title: Please avoid the term pfn and use gfn or mfn.
On 28/04/17 17:01, Bhupinder Thakur wrote:
1. Add two new domctl API to:
- Allocate a new event channel for sending/receiving events to/from Xen.
- Map the PFN allocted by the toolstack to be used as IN/OUT ring buffers.
Ditto and s/allocted/allocated/
Xen will communicate with xenconsole over the ring buffer and the event
channel to transmit and receive pl011 data on guest domain's behalf.
Signed-off-by: Bhupinder Thakur <bhupinder.thakur@xxxxxxxxxx>
---
Changes since v1:
- Replaced the hvm interface with domctl interface
tools/libxc/include/xenctrl.h | 26 ++++++++++++++++++++++++++
tools/libxc/xc_domain.c | 39 +++++++++++++++++++++++++++++++++++++++
xen/arch/arm/domctl.c | 20 ++++++++++++++++++++
xen/arch/x86/domctl.c | 4 ++++
xen/include/public/domctl.h | 11 +++++++++++
5 files changed, 100 insertions(+)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 1629f41..bebfe7d 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -886,6 +886,32 @@ int xc_vcpu_getcontext(xc_interface *xch,
vcpu_guest_context_any_t *ctxt);
/**
+ * This function returns information about the pfn and the event channel
Ditto.
+ * to be used for setting up a virtual uart console.
s/uart/UART/
+ *
+ * @parm xch a handle to an open hypervisor interface
+ * @parm domid the domain to get information from
+ * @parm vuart_pfn the pfn to be used for the ring buffer
Ditto for pfn.
+ * @return 0 on success, negative error on failure
+ */
+int xc_domain_vuart_set_pfn(xc_interface *xch,
s/pfn/gfn/
+ uint32_t domid,
+ uint32_t vuart_pfn);
s/pfn/gfn/ also a frame number may not fit in uint32_t. You want to use
xen_pfn_t.
+
+/**
+ * This function returns information about the pfn and the event channel
+ * to be used for setting up a virtual console.
+ *
+ * @parm xch a handle to an open hypervisor interface
+ * @parm domid the domain to get information from
+ * @parm vuart_evtchn the event channel to be used for console events
+ * @return 0 on success, negative error on failure
+ */
+int xc_domain_vuart_get_evtchn(xc_interface *xch,
+ uint32_t domid,
+ uint32_t *vuart_evtchn);
You want to use evtchn_port_t here.
+
+/**
* This function returns information about the XSAVE state of a particular
* vcpu of a domain. If extstate->size and extstate->xfeature_mask are 0,
* the call is considered a query to retrieve them and the buffer is not
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 00909ad4..943f202 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -343,6 +343,45 @@ int xc_domain_get_guest_width(xc_interface *xch, uint32_t
domid,
return 0;
}
+int xc_domain_vuart_set_pfn(xc_interface *xch,
+ uint32_t domid,
+ uint32_t vuart_pfn)
See all my remarks above.
+{
+ DECLARE_DOMCTL;
+ int rc = 0;
+
+ domctl.cmd = XEN_DOMCTL_vuart_op;
+ domctl.domain = (domid_t)domid;
+ domctl.u.vuart_op.cmd = XEN_DOMCTL_VUART_OP_SET_PFN;
+ domctl.u.vuart_op.pfn = vuart_pfn;
+
+ if ( (rc = do_domctl(xch, &domctl)) < 0 )
+ return rc;
+
+ return rc;
+}
+
+int xc_domain_vuart_get_evtchn(xc_interface *xch,
+ uint32_t domid,
+ uint32_t *vuart_evtchn)
See all my remarks above.
+{
+ DECLARE_DOMCTL;
+ int rc = 0;
+
+ *vuart_evtchn = -1;
+
+ domctl.cmd = XEN_DOMCTL_vuart_op;
+ domctl.domain = (domid_t)domid;
+ domctl.u.vuart_op.cmd = XEN_DOMCTL_VUART_OP_GET_EVTCHN;
+
+ if ( (rc = do_domctl(xch, &domctl)) < 0 )
+ return rc;
+
+ *vuart_evtchn = domctl.u.vuart_op.evtchn;
+
+ return rc;
+}
+
int xc_domain_getinfo(xc_interface *xch,
uint32_t first_domid,
unsigned int max_doms,
diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
index 971caec..e400f87 100644
--- a/xen/arch/arm/domctl.c
+++ b/xen/arch/arm/domctl.c
@@ -10,6 +10,7 @@
#include <xen/sched.h>
#include <xen/hypercall.h>
#include <xen/iocap.h>
+#include <xen/guest_access.h>
#include <xsm/xsm.h>
#include <public/domctl.h>
@@ -119,6 +120,25 @@ 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;
+ if ( d->arch.vpl011.initialized )
You can avoid one level of "if" if you do:
if ( !d->arch.vpl011.initialized )
return -EINVAL.
if ( domctl->u.vuart_op.cmd == .. )
+ {
+ if ( domctl->u.vuart_op.cmd == XEN_DOMCTL_VUART_OP_SET_PFN )
I would prefer if you use a switch case for the cmd, this would make
easier to return an error if the command is not correct rather assuming
that all others commands means "getting the event channel".
+ {
+ rc = vpl011_map_guest_page(d, domctl->u.vuart_op.pfn);
There is nothing to prevent vpl011_map_guest_page to be called twice. So
you would end up to leak memory if XEN_DOMCTL_VUART_OP_SET_PFN is called
twice.
+ }
+ else
+ {
+ domctl->u.vuart_op.evtchn = d->arch.vpl011.evtchn;
+ rc = __copy_to_guest(u_domctl, domctl, 1);
+ }
+ return rc;
+ }
+ else
+ return -EINVAL;
I don't think -EINVAL is the correct value if not initialized. It would
be better to be -ENOSYS.
+ }
default:
{
int rc;
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index e104be2..49e907d 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1465,6 +1465,10 @@ long arch_do_domctl(
recalculate_cpuid_policy(d);
break;
+ case XEN_DOMCTL_vuart_op:
+ ret = -EOPNOTSUPP;
+ break;
+
default:
ret = iommu_do_domctl(domctl, d, u_domctl);
break;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index e6cf211..8bee0c3 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1138,6 +1138,15 @@ struct xen_domctl_psr_cat_op {
uint32_t target; /* IN */
uint64_t data; /* IN/OUT */
};
+
+struct xen_domctl_vuart_op {
+#define XEN_DOMCTL_VUART_OP_SET_PFN 0
+#define XEN_DOMCTL_VUART_OP_GET_EVTCHN 1
+ uint32_t cmd; /* XEN_DOMCTL_VUART_OP_* */
+ uint32_t pfn; /* IN */
This should be xen_pfn_t and ...
+ uint32_t evtchn; /* OUT */
... evtchn_port_t.
+};
+
typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
@@ -1218,6 +1227,7 @@ struct xen_domctl {
#define XEN_DOMCTL_monitor_op 77
#define XEN_DOMCTL_psr_cat_op 78
#define XEN_DOMCTL_soft_reset 79
+#define XEN_DOMCTL_vuart_op 80
#define XEN_DOMCTL_gdbsx_guestmemio 1000
#define XEN_DOMCTL_gdbsx_pausevcpu 1001
#define XEN_DOMCTL_gdbsx_unpausevcpu 1002
@@ -1280,6 +1290,7 @@ struct xen_domctl {
struct xen_domctl_psr_cmt_op psr_cmt_op;
struct xen_domctl_monitor_op monitor_op;
struct xen_domctl_psr_cat_op psr_cat_op;
+ struct xen_domctl_vuart_op vuart_op;
uint8_t pad[128];
} u;
};
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
