[Xen-devel,02/10] xen/arm: vpl011: Add new virtual console hvm params in Xen

Message ID 1491212673-13476-3-git-send-email-bhupinder.thakur@linaro.org
State New
Headers show
Series
  • pl011 emulation support in Xen
Related show

Commit Message

Bhupinder Thakur April 3, 2017, 9:44 a.m.
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(+)

Comments

Stefano Stabellini April 19, 2017, 12:22 a.m. | #1
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
>
Bhupinder Thakur April 19, 2017, 8:48 a.m. | #2
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

Patch

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