| Message ID | 1491212673-13476-3-git-send-email-bhupinder.thakur@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 3 Apr 2017, Bhupinder Thakur wrote: > 1. Add two new HVM param handlers for: > - 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. > > 2. Add validation to disallow get/set of these HVM params from guest > domain. > > 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@linaro.org> > --- > xen/arch/arm/hvm.c | 112 ++++++++++++++++++++++++++++++++++++++++ > xen/include/public/hvm/params.h | 10 ++++ > 2 files changed, 122 insertions(+) > > diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c > index d999bde..c1fed45 100644 > --- a/xen/arch/arm/hvm.c > +++ b/xen/arch/arm/hvm.c > @@ -23,6 +23,8 @@ > #include <xen/guest_access.h> > #include <xen/sched.h> > #include <xen/monitor.h> > +#include <xen/event.h> > +#include <xen/vmap.h> > > #include <xsm/xsm.h> > > @@ -31,6 +33,80 @@ > #include <public/hvm/hvm_op.h> > > #include <asm/hypercall.h> > +#include <xen/vpl011.h> > + > +static bool vpl011_built(void) > +{ > +#ifdef CONFIG_VPL011_CONSOLE > + return true; > +#else > + return false; > +#endif > +} > + > +static int hvm_allow_set_param(struct domain *d, > + const struct xen_hvm_param *a) > +{ > + uint64_t value = d->arch.hvm_domain.params[a->index]; > + int rc; > + > + rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_set_param); > + if ( rc ) > + return rc; > + > + switch ( a->index ) > + { > + /* The following parameters should not be set by the guest. */ > + case HVM_PARAM_VCONSOLE_PFN: > + case HVM_PARAM_VCONSOLE_EVTCHN: > + if ( d == current->domain ) > + rc = -EPERM; > + break; > + default: > + break; > + } > + > + if ( rc ) > + return rc; > + > + switch ( a->index ) > + { > + /* The following parameters should only be changed once. */ > + case HVM_PARAM_VCONSOLE_PFN: > + case HVM_PARAM_VCONSOLE_EVTCHN: > + if ( value != 0 && a->value != value ) > + rc = -EEXIST; > + break; > + default: > + break; > + } > + > + return rc; > +} > + > +static int hvm_allow_get_param(struct domain *d, > + const struct xen_hvm_param *a) > +{ > + int rc; > + > + rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_get_param); > + if ( rc ) > + return rc; > + > + switch ( a->index ) > + { > + /* The remaining parameters should not be read by the guest. */ > + case HVM_PARAM_VCONSOLE_PFN: > + case HVM_PARAM_VCONSOLE_EVTCHN: > + if ( d == current->domain ) > + rc = -EPERM; > + break; > + default: > + break; > + } > + > + return rc; > +} > > long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) > { > @@ -61,9 +137,45 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) > if ( op == HVMOP_set_param ) > { > d->arch.hvm_domain.params[a.index] = a.value; > + > + if ( a.index == HVM_PARAM_VCONSOLE_PFN || > + a.index == HVM_PARAM_VCONSOLE_EVTCHN ) > + { > + if ( vpl011_built() ) Code style: we only use spaces for indentation in Xen. Please fix across the series. Also, I would move the "if ( vpl011_built() )" check to hvm_allow_set_param and hvm_allow_get_param. > + { > + rc = hvm_allow_set_param(d, &a); > + if ( rc ) > + goto param_fail; > + > + if ( a.index == HVM_PARAM_VCONSOLE_PFN ) > + { > + rc = vpl011_map_guest_page(d); > + if ( rc ) > + goto param_fail; > + } > + } > + else > + { > + rc = -1; > + goto param_fail; > + } > + } > } > else > { > + if ( a.index == HVM_PARAM_VCONSOLE_PFN || > + a.index == HVM_PARAM_VCONSOLE_EVTCHN ) > + { > + if ( !vpl011_built() ) > + { > + rc = -1; > + goto param_fail; > + } > + } > + rc = hvm_allow_get_param(d, &a); > + if ( rc ) > + goto param_fail; > + > a.value = d->arch.hvm_domain.params[a.index]; > rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0; > } > diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h > index 3f54a49..15d37e5 100644 > --- a/xen/include/public/hvm/params.h > +++ b/xen/include/public/hvm/params.h > @@ -203,10 +203,20 @@ > */ > #define HVM_PARAM_ACPI_IOPORTS_LOCATION 19 > > +#if defined(__arm__) || defined(__aarch64__) > +/* Virtual console (VC) shared memory ring and event channel. */ > +#define HVM_PARAM_VCONSOLE_PFN 20 > +#define HVM_PARAM_VCONSOLE_EVTCHN 21 > +#else > /* Deprecated */ > #define HVM_PARAM_MEMORY_EVENT_CR0 20 > #define HVM_PARAM_MEMORY_EVENT_CR3 21 > +#endif > + > +/* Deprecated */ > #define HVM_PARAM_MEMORY_EVENT_CR4 22 > + > +/* Deprecated */ > #define HVM_PARAM_MEMORY_EVENT_INT3 23 > #define HVM_PARAM_MEMORY_EVENT_SINGLE_STEP 25 > #define HVM_PARAM_MEMORY_EVENT_MSR 30 > -- > 2.7.4 >
Hi Stefano, On 19 April 2017 at 05:52, Stefano Stabellini <sstabellini@kernel.org> wrote: >> long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) >> { >> @@ -61,9 +137,45 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) >> if ( op == HVMOP_set_param ) >> { >> d->arch.hvm_domain.params[a.index] = a.value; >> + >> + if ( a.index == HVM_PARAM_VCONSOLE_PFN || >> + a.index == HVM_PARAM_VCONSOLE_EVTCHN ) >> + { >> + if ( vpl011_built() ) > > Code style: we only use spaces for indentation in Xen. Please fix across > the series. > I think this was introduced by mistake as I had to keep switching the tab settings while modifying Xen code and xenconsoled code. I will fix it. > Also, I would move the "if ( vpl011_built() )" check to > hvm_allow_set_param and hvm_allow_get_param. I will move vpl011_built() inside the hvm_allow functions. Regards, Bhupinder
diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c index d999bde..c1fed45 100644 --- a/xen/arch/arm/hvm.c +++ b/xen/arch/arm/hvm.c @@ -23,6 +23,8 @@ #include <xen/guest_access.h> #include <xen/sched.h> #include <xen/monitor.h> +#include <xen/event.h> +#include <xen/vmap.h> #include <xsm/xsm.h> @@ -31,6 +33,80 @@ #include <public/hvm/hvm_op.h> #include <asm/hypercall.h> +#include <xen/vpl011.h> + +static bool vpl011_built(void) +{ +#ifdef CONFIG_VPL011_CONSOLE + return true; +#else + return false; +#endif +} + +static int hvm_allow_set_param(struct domain *d, + const struct xen_hvm_param *a) +{ + uint64_t value = d->arch.hvm_domain.params[a->index]; + int rc; + + rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_set_param); + if ( rc ) + return rc; + + switch ( a->index ) + { + /* The following parameters should not be set by the guest. */ + case HVM_PARAM_VCONSOLE_PFN: + case HVM_PARAM_VCONSOLE_EVTCHN: + if ( d == current->domain ) + rc = -EPERM; + break; + default: + break; + } + + if ( rc ) + return rc; + + switch ( a->index ) + { + /* The following parameters should only be changed once. */ + case HVM_PARAM_VCONSOLE_PFN: + case HVM_PARAM_VCONSOLE_EVTCHN: + if ( value != 0 && a->value != value ) + rc = -EEXIST; + break; + default: + break; + } + + return rc; +} + +static int hvm_allow_get_param(struct domain *d, + const struct xen_hvm_param *a) +{ + int rc; + + rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_get_param); + if ( rc ) + return rc; + + switch ( a->index ) + { + /* The remaining parameters should not be read by the guest. */ + case HVM_PARAM_VCONSOLE_PFN: + case HVM_PARAM_VCONSOLE_EVTCHN: + if ( d == current->domain ) + rc = -EPERM; + break; + default: + break; + } + + return rc; +} long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) { @@ -61,9 +137,45 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) if ( op == HVMOP_set_param ) { d->arch.hvm_domain.params[a.index] = a.value; + + if ( a.index == HVM_PARAM_VCONSOLE_PFN || + a.index == HVM_PARAM_VCONSOLE_EVTCHN ) + { + if ( vpl011_built() ) + { + rc = hvm_allow_set_param(d, &a); + if ( rc ) + goto param_fail; + + if ( a.index == HVM_PARAM_VCONSOLE_PFN ) + { + rc = vpl011_map_guest_page(d); + if ( rc ) + goto param_fail; + } + } + else + { + rc = -1; + goto param_fail; + } + } } else { + if ( a.index == HVM_PARAM_VCONSOLE_PFN || + a.index == HVM_PARAM_VCONSOLE_EVTCHN ) + { + if ( !vpl011_built() ) + { + rc = -1; + goto param_fail; + } + } + rc = hvm_allow_get_param(d, &a); + if ( rc ) + goto param_fail; + a.value = d->arch.hvm_domain.params[a.index]; rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0; } diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h index 3f54a49..15d37e5 100644 --- a/xen/include/public/hvm/params.h +++ b/xen/include/public/hvm/params.h @@ -203,10 +203,20 @@ */ #define HVM_PARAM_ACPI_IOPORTS_LOCATION 19 +#if defined(__arm__) || defined(__aarch64__) +/* Virtual console (VC) shared memory ring and event channel. */ +#define HVM_PARAM_VCONSOLE_PFN 20 +#define HVM_PARAM_VCONSOLE_EVTCHN 21 +#else /* Deprecated */ #define HVM_PARAM_MEMORY_EVENT_CR0 20 #define HVM_PARAM_MEMORY_EVENT_CR3 21 +#endif + +/* Deprecated */ #define HVM_PARAM_MEMORY_EVENT_CR4 22 + +/* Deprecated */ #define HVM_PARAM_MEMORY_EVENT_INT3 23 #define HVM_PARAM_MEMORY_EVENT_SINGLE_STEP 25 #define HVM_PARAM_MEMORY_EVENT_MSR 30
1. Add two new HVM param handlers for: - 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. 2. Add validation to disallow get/set of these HVM params from guest domain. 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@linaro.org> --- xen/arch/arm/hvm.c | 112 ++++++++++++++++++++++++++++++++++++++++ xen/include/public/hvm/params.h | 10 ++++ 2 files changed, 122 insertions(+)