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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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.


> > +{
> > +    switch ( cmd )
> > +    {
> > +        case VCPUOP_register_vcpu_info:
> > +            return do_vcpu_op(cmd, vcpuid, arg);
> > +        default:
> > +            return -EINVAL;
> 
> ENOSYS I think.

right

> > +    }
> > +}
> > +
> >  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.

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