Re: [Xen-devel] [PATCH v3 10/13] xen/arm: support VCPUOP_register_vcpu_info.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-04-25 at 12:38 +0100, Stefano Stabellini wrote:
> On Thu, 25 Apr 2013, Ian Campbell wrote:
> > On Wed, 2013-04-24 at 20:07 +0100, Stefano Stabellini wrote:
> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > > CC: keir@xxxxxxx
> > > CC: JBeulich@xxxxxxxx
> > > 
> > > Changes in v3:
> > > - do not export all the vcpu_op hypercalls to ARM guests, only
> > > VCPUOP_register_vcpu_info.
> > > ---
> > >  xen/arch/arm/domain.c           |   13 +++++++++++++
> > >  xen/arch/arm/traps.c            |    1 +
> > >  xen/include/asm-arm/hypercall.h |    3 +++
> > >  3 files changed, 17 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> > > index fee3790..a676441 100644
> > > --- a/xen/arch/arm/domain.c
> > > +++ b/xen/arch/arm/domain.c
> > > @@ -10,6 +10,7 @@
> > >   * GNU General Public License for more details.
> > >   */
> > >  #include <xen/config.h>
> > > +#include <xen/hypercall.h>
> > >  #include <xen/init.h>
> > >  #include <xen/lib.h>
> > >  #include <xen/sched.h>
> > > @@ -628,6 +629,18 @@ void arch_dump_domain_info(struct domain *d)
> > >   }
> > >  }
> > >  
> > > +
> > > +long do_restricted_vcpu_op(int cmd, int vcpuid, 
> > > XEN_GUEST_HANDLE_PARAM(void) arg)
> > 
> > This is a bit fugly but I suppose it's no worse than the other
> > alternatives I can think of.
> > 
> > I don't really like the "restricted" name but the other obvious
> > alternative do_arch_vcpu_op is out because typically that's called
> > *from* do_foo_op not instead of.
> > 
> > Is renaming do_vcpu_op to do_common_vcpu_op and adding do_vcpu_op as
> > per-arch on all architectures (basically a nop on x86) an option?
> 
> This is a question for the x86 maintainers.

Yeah, sorry it was I just didn't address it to them ;-)

> > > +    }
> > > +}
> > > +
> > >  long arch_do_vcpu_op(int cmd, struct vcpu *v, 
> > > XEN_GUEST_HANDLE_PARAM(void) arg)
> > >  {
> > >      return -ENOSYS;
> > > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> > > index 733099a..d69231c 100644
> > > --- a/xen/arch/arm/traps.c
> > > +++ b/xen/arch/arm/traps.c
> > > @@ -617,6 +617,7 @@ static arm_hypercall_t arm_hypercall_table[] = {
> > >      HYPERCALL(sysctl, 2),
> > >      HYPERCALL(hvm_op, 2),
> > >      HYPERCALL(grant_table_op, 3),
> > > +    HYPERCALL(restricted_vcpu_op, 3),
> > >  };
> > >  
> > >  #define __PSCI_cpu_suspend 0
> > > diff --git a/xen/include/asm-arm/hypercall.h 
> > > b/xen/include/asm-arm/hypercall.h
> > > index 0833ec4..8ab0cc4 100644
> > > --- a/xen/include/asm-arm/hypercall.h
> > > +++ b/xen/include/asm-arm/hypercall.h
> > > @@ -4,6 +4,9 @@
> > >  #include <public/domctl.h> /* for arch_do_domctl */
> > >  int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
> > >  
> > > +#define __HYPERVISOR_restricted_vcpu_op __HYPERVISOR_vcpu_op
> > 
> > I don't think this needs it's own #define, does it? (maybe that requires
> > an alternative HYPERCALL macro, that would be fine IMHO).
> 
> It does because of the way the HYPERCALL macro builds the
> arm_hypercall_table.

Right, hence my call for an alternative macro, e.g. #define
RESTICTED_HYPERCALL

Ian.



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