Re: [PATCH 3/6] hvmloader: add helper functions to get/set HVM params

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

On Tue, Jun 03, 2014 at 02:15:37PM +0100, David Vrabel wrote:
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  tools/firmware/hvmloader/Makefile    |    1 +
>  tools/firmware/hvmloader/hvm_param.c |   36 ++++++++++++++++++++++++++++++++++
>  tools/firmware/hvmloader/hvmloader.c |   14 ++-----------
>  tools/firmware/hvmloader/util.h      |    9 +++++++++
>  tools/firmware/hvmloader/xenbus.c    |   14 +++++--------
>  5 files changed, 53 insertions(+), 21 deletions(-)
>  create mode 100644 tools/firmware/hvmloader/hvm_param.c
> 
> diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
> index 00ee952..46a79c5 100644
> --- a/tools/firmware/hvmloader/Makefile
> +++ b/tools/firmware/hvmloader/Makefile
> @@ -31,6 +31,7 @@ CFLAGS += $(CFLAGS_xeninclude)
>  OBJS  = hvmloader.o mp_tables.o util.o smbios.o 
>  OBJS += smp.o cacheattr.o xenbus.o
>  OBJS += e820.o pci.o pir.o ctype.o
> +OBJS += hvm_param.o
>  ifeq ($(debug),y)
>  OBJS += tests.o
>  endif
> diff --git a/tools/firmware/hvmloader/hvm_param.c b/tools/firmware/hvmloader/hvm_param.c
> new file mode 100644
> index 0000000..f7d8720
> --- /dev/null
> +++ b/tools/firmware/hvmloader/hvm_param.c
> @@ -0,0 +1,36 @@
> +/*
> + * hvm_param.c: get/set HVM params.
> + *
> + * Copyright (C) 2014 Citrix Systems R&D Ltd.
> + */
> +#include "util.h"
> +#include "config.h"
> +#include "hypercall.h"
> +
> +#include <xen/hvm/params.h>
> +
> +int hvm_param_get(uint32_t index, uint64_t *value)
> +{
> +    struct xen_hvm_param p;
> +    int ret;
> +
> +    p.domid = DOMID_SELF;
> +    p.index = index;
> +
> +    ret = hypercall_hvm_op(HVMOP_get_param, &p);
> +    if (ret == 0)
> +        *value = p.value;
> +
> +    return ret;
> +}
> +
> +int hvm_param_set(uint32_t index, uint64_t value)
> +{
> +    struct xen_hvm_param p;
> +
> +    p.domid = DOMID_SELF;
> +    p.index = index;
> +    p.value = value;
> +
> +    return hypercall_hvm_op(HVMOP_set_param, &p);
> +}
> diff --git a/tools/firmware/hvmloader/hvmloader.c b/tools/firmware/hvmloader/hvmloader.c
> index 1cc8cf2..7b0da38 100644
> --- a/tools/firmware/hvmloader/hvmloader.c
> +++ b/tools/firmware/hvmloader/hvmloader.c
> @@ -176,14 +176,10 @@ static void cmos_write_memory_size(void)
>  static void init_vm86_tss(void)
>  {
>      void *tss;
> -    struct xen_hvm_param p;
>  
>      tss = mem_alloc(128, 128);
>      memset(tss, 0, 128);
> -    p.domid = DOMID_SELF;
> -    p.index = HVM_PARAM_VM86_TSS;
> -    p.value = virt_to_phys(tss);
> -    hypercall_hvm_op(HVMOP_set_param, &p);
> +    hvm_param_set(HVM_PARAM_VM86_TSS, virt_to_phys(tss));
>      printf("vm86 TSS at %08lx\n", virt_to_phys(tss));
>  }
>  
> @@ -314,12 +310,6 @@ int main(void)
>  
>      if ( acpi_enabled )
>      {
> -        struct xen_hvm_param p = {
> -            .domid = DOMID_SELF,
> -            .index = HVM_PARAM_ACPI_IOPORTS_LOCATION,
> -            .value = 1,
> -        };
> -
>          if ( bios->acpi_build_tables )
>          {
>              printf("Loading ACPI ...\n");
> @@ -328,7 +318,7 @@ int main(void)
>  
>          acpi_enable_sci();
>  
> -        hypercall_hvm_op(HVMOP_set_param, &p);
> +        hvm_param_set(HVM_PARAM_ACPI_IOPORTS_LOCATION, 1);
>      }
>  
>      init_vm86_tss();
> diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h
> index 9ccb905..e4b6be4 100644
> --- a/tools/firmware/hvmloader/util.h
> +++ b/tools/firmware/hvmloader/util.h
> @@ -210,6 +210,15 @@ const char *xenstore_read(const char *path, const char *default_resp);
>   */
>  int xenstore_write(const char *path, const char *value);
>  
> +
> +/* Get a HVM param.
> + */
> +int hvm_param_get(uint32_t index, uint64_t *value);
> +
> +/* Set a HVM param.
> + */
> +int hvm_param_set(uint32_t index, uint64_t value);
> +
>  /* Setup PCI bus */
>  void pci_setup(void);
>  
> diff --git a/tools/firmware/hvmloader/xenbus.c b/tools/firmware/hvmloader/xenbus.c
> index fe72e97..64c2176 100644
> --- a/tools/firmware/hvmloader/xenbus.c
> +++ b/tools/firmware/hvmloader/xenbus.c
> @@ -41,21 +41,17 @@ static char payload[XENSTORE_PAYLOAD_MAX + 1];  /* Unmarshalling area */
>   * Call once, before any other xenbus actions. */
>  void xenbus_setup(void)
>  {
> -    xen_hvm_param_t param;
> +    uint64_t val;
>  
>      /* Ask Xen where the xenbus shared page is. */
> -    param.domid = DOMID_SELF;
> -    param.index = HVM_PARAM_STORE_PFN;
> -    if ( hypercall_hvm_op(HVMOP_get_param, &param) )
> +    if ( hvm_param_get(HVM_PARAM_STORE_PFN, &val) )
>          BUG();
> -    rings = (void *) (unsigned long) (param.value << PAGE_SHIFT);
> +    rings = (void *) (unsigned long) (val << PAGE_SHIFT);
>  
>      /* Ask Xen where the xenbus event channel is. */
> -    param.domid = DOMID_SELF;
> -    param.index = HVM_PARAM_STORE_EVTCHN;
> -    if ( hypercall_hvm_op(HVMOP_get_param, &param) )
> +    if ( hvm_param_get(HVM_PARAM_STORE_EVTCHN, &val) )
>          BUG();
> -    event = param.value;
> +    event = val;
>  
>      printf("Xenbus rings @0x%lx, event channel %lu\n",
>             (unsigned long) rings, (unsigned long) event);
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel