Re: [Xen-devel] [PATCH v4 2/9] xen/arm: Add more registers for saving and restoring vcpu registers

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2013-10-11 at 17:30 +0900, Jaeyong Yoo wrote:
> > -----Original Message-----
> > From: xen-devel-bounces@xxxxxxxxxxxxx [mailto:xen-devel-
> > bounces@xxxxxxxxxxxxx] On Behalf Of Ian Campbell
> > Sent: Thursday, October 10, 2013 7:41 PM
> > To: Jaeyong Yoo
> > Cc: Tim Deegan; xen-devel@xxxxxxxxxxxxx
> > Subject: Re: [Xen-devel] [PATCH v4 2/9] xen/arm: Add more registers for
> > saving and restoring vcpu registers
> > 
> > On Fri, 2013-10-04 at 13:43 +0900, Jaeyong Yoo wrote:
> > > diff --git a/xen/include/public/arch-arm.h
> > > b/xen/include/public/arch-arm.h index 5d359af..bf6dc9a 100644
> > > --- a/xen/include/public/arch-arm.h
> > > +++ b/xen/include/public/arch-arm.h
> > > @@ -253,6 +253,41 @@ struct vcpu_guest_context {
> > >
> > >      uint32_t sctlr, ttbcr;
> > >      uint64_t ttbr0, ttbr1;
> > > +    uint32_t ifar, dfar;
> > > +    uint32_t ifsr, dfsr;
> > > +    uint32_t dacr;
> > > +    uint64_t par;
> > > +
> > > +#ifdef CONFIG_ARM_32
> > 
> > I'm afraid a per arch ifdef isn't allowed in the include/public tree.
> > The interface should be identical for both 32 and 64 bit callers. Also
> > think of 32-on-64 guests etc.
> > 
> > Also, this struct is guest facing (via VCPUOP_initialise) but many/all of
> > these new registers are not things which a guest needs to specify via a
> > hypercall. IOW I think many of them should be part of some toolstack
> > private save/restore interface.
> 
> I see, the guest can specify something like sctlr, and ttbr/ttbcr, and the
> others should be hidden inside hvm save/restore. 

Right, the important thing is that all that additional state is only
visible to the toolstack and the hypervisor, not to guests.

Actually the guest shouldn't really see this interface anyway, that's
really a hold over from x86. On ARM only the toolstack really needs to
use this struct.

I wonder if we can drop struct vcpu_guest_context from the guest facing
ABI on ARM. I see that we already don't expose VCPUOP_initialise and the
only other user is XEN_DOMCTL_(sg)etvcpucontext.

I wonder if we could on x86 do:
        typedef vcpu_guest_context_t domctl_guest_context_t
and on ARM s/vcpu_guest_context/domtctl_guest_context/ (plus appropriate
__XEN_TOOLS__ ifdeffery) and make the domctl's use the new name only
while VCPUOP uses the old name?

Can anyone think of a reason why a guest might need an interface to
set/get VCPU state directly like this on ARM? Or why this might not work
for x86?

Ian.



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