Re: [PATCH 02/11] xen/arm: vpl011: Add new hvm params in Xen for ring buffer/event setup

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Feb 21, 2017 at 04:55:59PM +0530, Bhupinder Thakur wrote:
> Three new HVM param handlers added for:
>     - allocating a new VIRQ and return to the toolstack
>     - allocating a new event channel for sending/receiving events from Xen and return
>       to the toolstack
>     - mapping the PFN allocted by the toolstack to be used as IN/OUT ring buffers
> 
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
> ---
>  xen/arch/arm/hvm.c              | 39 +++++++++++++++++++++++++++++++++++++++
>  xen/include/public/hvm/params.h | 10 +++++++++-
>  2 files changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
> index d999bde..f3b9eb1 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,7 @@
>  #include <public/hvm/hvm_op.h>
>  
>  #include <asm/hypercall.h>
> +#include "vpl011.h"
>  
>  long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
>  {
> @@ -61,9 +64,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;
> +
> +#ifdef CONFIG_VPL011_CONSOLE
> +            /*
> +             * if it is a vpl011 console pfn then map it to its

s/if/If/
> +             * own address space

And you can also add an . here.

> +             */
> +            if ( a.index == HVM_PARAM_VPL011_CONSOLE_PFN )
> +            {
> +                vpl011_map_guest_page(d);
> +            }
> +#else
> +            /* 
> +             * if VPL011 is not compiled in then disallow setting of any 
> +             * related HVM params
> +             */
> +            if ( a.index == HVM_PARAM_VPL011_CONSOLE_PFN ||
> +                 a.index == HVM_PARAM_VPL011_CONSOLE_EVTCHN ||
> +                 a.index == HVM_PARAM_VPL011_VIRQ )
> +            {
> +                rc = -1;
> +                goto param_fail;
> +            }
> +#endif
>          }
>          else
>          {
> +#ifndef CONFIG_VPL011_CONSOLE
> +            /* 
> +             * if VPL011 is not compiled in then disallow setting of any 
> +             * related HVM params
> +             */
> +            if ( a.index == HVM_PARAM_VPL011_CONSOLE_PFN ||
> +                 a.index == HVM_PARAM_VPL011_CONSOLE_EVTCHN ||
> +                 a.index == HVM_PARAM_VPL011_VIRQ )
> +            {
> +                rc = -1;
> +                goto param_fail;

This and the above look like it could be nicely folded in a function?

Say:

static bool vpl011_built(unsigned int idx)
{
#ifdef CONFIG_VPL011_CONSOLE
	if ( idx == ..
		return true;
#else
	return false;
}

And here you can just do:
	if (vpl011_built())
	{
		rc = -1;
	.. and so on.
		
> +            }
> +#endif
>              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..13bf719 100644
> --- a/xen/include/public/hvm/params.h
> +++ b/xen/include/public/hvm/params.h
> @@ -203,10 +203,17 @@
>   */
>  #define HVM_PARAM_ACPI_IOPORTS_LOCATION 19
>  
> -/* Deprecated */
> +#if defined(__arm__) || defined(__aarch64__)
> +#define HVM_PARAM_VPL011_CONSOLE_PFN    20
> +#define HVM_PARAM_VPL011_CONSOLE_EVTCHN 21
> +#define HVM_PARAM_VPL011_VIRQ           22
> +#else
>  #define HVM_PARAM_MEMORY_EVENT_CR0          20
>  #define HVM_PARAM_MEMORY_EVENT_CR3          21
>  #define HVM_PARAM_MEMORY_EVENT_CR4          22
> +#endif
> +
> +/* Deprecated */
>  #define HVM_PARAM_MEMORY_EVENT_INT3         23
>  #define HVM_PARAM_MEMORY_EVENT_SINGLE_STEP  25
>  #define HVM_PARAM_MEMORY_EVENT_MSR          30
> @@ -253,6 +260,7 @@
>   */
>  #define HVM_PARAM_X87_FIP_WIDTH 36
>  
> +


???
>  #define HVM_NR_PARAMS 37
>  
>  #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel

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