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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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@xxxxxxxxxx>
> ---
>  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
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel