[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH linux v3] arm: xen: implement multicall hypercall support.



On Thu, 24 Apr 2014, Ian Campbell wrote:
> On Thu, 2014-04-17 at 13:57 +0100, Ian Campbell wrote:
> 
> Ping? (FYI the Xen side has just been committed)

Sorry for the delay


> > As part of this make the usual change to xen_ulong_t in place of unsigned 
> > long.
> > This change has no impact on x86.
> > 
> > The Linux definition of struct multicall_entry.result differs from the Xen
> > definition, I think for good reasons, and used a long rather than an 
> > unsigned
> > long. Therefore introduce a xen_long_t, which is a long on x86 architectures
> > and a signed 64-bit integer on ARM.
> > 
> > Use uint32_t nr_calls on x86 for consistency with the ARM definition.
> > 
> > Build tested on amd64 and i386 builds. Runtime tested on ARM.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > Cc: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> > Cc: David Vrabel <david.vrabel@xxxxxxxxxx>
> > ---
> > v3: - Add for arm64 too
> > v2: - Typo in commit message.
> >     - Update x86 prototype for HYPERCALL_multicall for consistency (nr_calls
> >       is a uint32_t).
> > 
> > Tested on ARM and arm64 with a stupid patch:
> >     diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> >     index b96723e..8b1dc7a 100644
> >     --- a/arch/arm/xen/enlighten.c
> >     +++ b/arch/arm/xen/enlighten.c
> >     @@ -339,6 +339,63 @@ static int __init xen_pm_init(void)
> >      }
> >      late_initcall(xen_pm_init);
> > 
> >     +static int __init multicall_test(void)
> >     +{
> >     +       struct multicall_entry templ[3];
> >     +       struct multicall_entry call[3];
> >     +       const char *str0 = "This is the first debug string\n";
> >     +       const char *str1 = "This is the second debug string\n";
> >     +       const char *str2 = "This is the third debug string\n";
> >     +       int ret;
> >     +
> >     +       templ[0] = (struct multicall_entry) {
> >     +               .op = __HYPERVISOR_console_io,
> >     +               .args[0] = CONSOLEIO_write,
> >     +               .args[1] = strlen(str0),
> >     +               .args[2] = (uintptr_t)str0
> >     +       };
> >     +       templ[1] = (struct multicall_entry) {
> >     +               .op = __HYPERVISOR_console_io,
> >     +               .args[0] = CONSOLEIO_write,
> >     +               .args[1] = strlen(str1),
> >     +               .args[2] = (uintptr_t)str1
> >     +       };
> >     +       templ[2] = (struct multicall_entry) {
> >     +               .op = __HYPERVISOR_console_io,
> >     +               .args[0] = CONSOLEIO_write,
> >     +               .args[1] = strlen(str2),
> >     +               .args[2] = (uintptr_t)str2
> >     +       };
> >     +
> >     +       memcpy(call, templ, sizeof(templ));
> >     +
> >     +       ret = HYPERVISOR_multicall(call, ARRAY_SIZE(call));
> >     +       printk("1st MULTICALL returned %d\n", ret);
> >     +       if (ret == 0) {
> >     +               printk("call[0].result = %"PRI_xen_long"\n", 
> > call[0].result);
> >     +               printk("call[1].result = %"PRI_xen_long"\n", 
> > call[1].result);
> >     +               printk("call[2].result = %"PRI_xen_long"\n", 
> > call[2].result);
> >     +       }
> >     +
> >     +       memcpy(call, templ, sizeof(templ));
> >     +       call[1].args[0] |= 0xf000000000000000ULL;
> >     +
> >     +       /*
> >     +         * On arm: should die after printing first string, shouldn't 
> > print third
> >     +         * On arm64: should print all three.
> >     +         */
> >     +       ret = HYPERVISOR_multicall(call, ARRAY_SIZE(call));
> >     +       printk("2nd MULTICALL returned %d\n", ret);
> >     +       if (ret == 0) {
> >     +               printk("call[0].result = %"PRI_xen_long"\n", 
> > call[0].result);
> >     +               printk("call[1].result = %"PRI_xen_long"\n", 
> > call[1].result);
> >     +               printk("call[2].result = %"PRI_xen_long"\n", 
> > call[2].result);
> >     +       }
> >     +
> >     +       return 0;
> >     +}
> >     +late_initcall(multicall_test);
> >     +
> >      /* In the hypervisor.S file. */
> >      EXPORT_SYMBOL_GPL(HYPERVISOR_event_channel_op);
> >      EXPORT_SYMBOL_GPL(HYPERVISOR_grant_table_op);
> > ---
> >  arch/arm/include/asm/xen/hypercall.h |    6 +-----
> >  arch/arm/include/asm/xen/interface.h |    2 ++
> >  arch/arm/xen/hypercall.S             |    1 +
> >  arch/arm64/xen/hypercall.S           |    1 +
> >  arch/x86/include/asm/xen/hypercall.h |    2 +-
> >  arch/x86/include/asm/xen/interface.h |    3 +++
> >  include/xen/interface/xen.h          |    6 +++---
> >  7 files changed, 12 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/xen/hypercall.h 
> > b/arch/arm/include/asm/xen/hypercall.h
> > index 7704e28..7658150 100644
> > --- a/arch/arm/include/asm/xen/hypercall.h
> > +++ b/arch/arm/include/asm/xen/hypercall.h
> > @@ -48,6 +48,7 @@ int HYPERVISOR_memory_op(unsigned int cmd, void *arg);
> >  int HYPERVISOR_physdev_op(int cmd, void *arg);
> >  int HYPERVISOR_vcpu_op(int cmd, int vcpuid, void *extra_args);
> >  int HYPERVISOR_tmem_op(void *arg);
> > +int HYPERVISOR_multicall(struct multicall_entry *calls, uint32_t nr);
> >  
> >  static inline void
> >  MULTI_update_va_mapping(struct multicall_entry *mcl, unsigned long va,
> > @@ -63,9 +64,4 @@ MULTI_mmu_update(struct multicall_entry *mcl, struct 
> > mmu_update *req,
> >     BUG();
> >  }
> >  
> > -static inline int
> > -HYPERVISOR_multicall(void *call_list, int nr_calls)
> > -{
> > -   BUG();
> > -}
> >  #endif /* _ASM_ARM_XEN_HYPERCALL_H */
> > diff --git a/arch/arm/include/asm/xen/interface.h 
> > b/arch/arm/include/asm/xen/interface.h
> > index 1151188..5006600 100644
> > --- a/arch/arm/include/asm/xen/interface.h
> > +++ b/arch/arm/include/asm/xen/interface.h
> > @@ -40,6 +40,8 @@ typedef uint64_t xen_pfn_t;
> >  #define PRI_xen_pfn "llx"
> >  typedef uint64_t xen_ulong_t;
> >  #define PRI_xen_ulong "llx"
> > +typedef int64_t xen_long_t;
> > +#define PRI_xen_long "llx"
> >  /* Guest handles for primitive C types. */
> >  __DEFINE_GUEST_HANDLE(uchar, unsigned char);
> >  __DEFINE_GUEST_HANDLE(uint,  unsigned int);
> > diff --git a/arch/arm/xen/hypercall.S b/arch/arm/xen/hypercall.S
> > index d1cf7b7..44e3a5f 100644
> > --- a/arch/arm/xen/hypercall.S
> > +++ b/arch/arm/xen/hypercall.S
> > @@ -89,6 +89,7 @@ HYPERCALL2(memory_op);
> >  HYPERCALL2(physdev_op);
> >  HYPERCALL3(vcpu_op);
> >  HYPERCALL1(tmem_op);
> > +HYPERCALL2(multicall);
> >  
> >  ENTRY(privcmd_call)
> >     stmdb sp!, {r4}
> > diff --git a/arch/arm64/xen/hypercall.S b/arch/arm64/xen/hypercall.S
> > index 531342e..8bbe940 100644
> > --- a/arch/arm64/xen/hypercall.S
> > +++ b/arch/arm64/xen/hypercall.S
> > @@ -80,6 +80,7 @@ HYPERCALL2(memory_op);
> >  HYPERCALL2(physdev_op);
> >  HYPERCALL3(vcpu_op);
> >  HYPERCALL1(tmem_op);
> > +HYPERCALL2(multicall);
> >  
> >  ENTRY(privcmd_call)
> >     mov x16, x0

You need to add an EXPORT_SYMBOL_GPL for it in arch/arm/xen/enlighten.c.

The rest is OK.

David, if you don't mind, I would rather have one updated patch in the
tree rather than two. Could you back out the current version from
devel/for-linus-3.16?



> > diff --git a/arch/x86/include/asm/xen/hypercall.h 
> > b/arch/x86/include/asm/xen/hypercall.h
> > index e709884..ca08a27 100644
> > --- a/arch/x86/include/asm/xen/hypercall.h
> > +++ b/arch/x86/include/asm/xen/hypercall.h
> > @@ -343,7 +343,7 @@ HYPERVISOR_memory_op(unsigned int cmd, void *arg)
> >  }
> >  
> >  static inline int
> > -HYPERVISOR_multicall(void *call_list, int nr_calls)
> > +HYPERVISOR_multicall(void *call_list, uint32_t nr_calls)
> >  {
> >     return _hypercall2(int, multicall, call_list, nr_calls);
> >  }
> > diff --git a/arch/x86/include/asm/xen/interface.h 
> > b/arch/x86/include/asm/xen/interface.h
> > index fd9cb76..3400dba 100644
> > --- a/arch/x86/include/asm/xen/interface.h
> > +++ b/arch/x86/include/asm/xen/interface.h
> > @@ -54,6 +54,9 @@ typedef unsigned long xen_pfn_t;
> >  #define PRI_xen_pfn "lx"
> >  typedef unsigned long xen_ulong_t;
> >  #define PRI_xen_ulong "lx"
> > +typedef long xen_long_t;
> > +#define PRI_xen_long "lx"
> > +
> >  /* Guest handles for primitive C types. */
> >  __DEFINE_GUEST_HANDLE(uchar, unsigned char);
> >  __DEFINE_GUEST_HANDLE(uint,  unsigned int);
> > diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
> > index 0cd5ca3..de08213 100644
> > --- a/include/xen/interface/xen.h
> > +++ b/include/xen/interface/xen.h
> > @@ -275,9 +275,9 @@ DEFINE_GUEST_HANDLE_STRUCT(mmu_update);
> >   * NB. The fields are natural register size for this architecture.
> >   */
> >  struct multicall_entry {
> > -    unsigned long op;
> > -    long result;
> > -    unsigned long args[6];
> > +    xen_ulong_t op;
> > +    xen_long_t result;
> > +    xen_ulong_t args[6];
> >  };
> >  DEFINE_GUEST_HANDLE_STRUCT(multicall_entry);
> >  
> 
> 

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.