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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-10-17 at 11:14 +0900, Jaeyong Yoo wrote:
> > -----Original Message-----
> > From: xen-devel-bounces@xxxxxxxxxxxxx [mailto:xen-devel-
> > bounces@xxxxxxxxxxxxx] On Behalf Of Jaeyong Yoo
> > Sent: Monday, October 14, 2013 1:39 PM
> > To: 'Ian Campbell'; 'Tim Deegan'
> > Cc: 'Stefano Stabellini'; 'Keir Fraser'; 'Jan Beulich'; xen-
> > devel@xxxxxxxxxxxxx
> > Subject: Re: [Xen-devel] [PATCH v4 2/9] xen/arm: Add more registers for
> > saving and restoring vcpu registers
> > 
> > > -----Original Message-----
> > > From: xen-devel-bounces@xxxxxxxxxxxxx [mailto:xen-devel-
> > > bounces@xxxxxxxxxxxxx] On Behalf Of Ian Campbell
> > > Sent: Friday, October 11, 2013 7:26 PM
> > > To: Tim Deegan
> > > Cc: Keir Fraser; Stefano Stabellini; Jan Beulich; Jaeyong Yoo; 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-11 at 11:22 +0100, Tim Deegan wrote:
> > > > At 09:43 +0100 on 11 Oct (1381484614), Ian Campbell wrote:
> > > > > 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.
> > > >
> > > > Does the guest need to have those on ARM? How are you arranging SMP
> > > > guest AP bringup?  If it's using SCI then maybe that hypercall
> > > > interface can be dropped.
> > >
> > > SMP bring up is done via PSCI, which is the firmware "hypercall"
> > > interface, defined by ARM for bringing up physucal CPUs which we
> > > implement within Xen for the guests' benefit.
> > 
> > So, could I remove XEN_DOMCTL_(sg)etvcpucontext and use HVM save/restore
> > for migrating vcpu registers?
> > 
> > And, if I could, arch_get_info_guest becomes dangling function and would
> > it be better to remove this function too? or better to keep it for
> > symmetry?
> 
> Are these questions missed?

Yes, sorry!

In principal we might be able to remove XEN_DOMCTL_(sg)etvcpucontext as
you suggest but I'm not sure to what extent tools are at liberty to poke
inside HVM save/restore blobs? I suppose they are allowed to and can
ignore compatibility stuff like supporting older version's layouts?

But to be honest I think it's probably easier to keep the domctls, so
that common(ish) code like xenctx.c continues to work for dumping basic
processor state to distinguish between platforms which use domctl and
those which use hvmsave.

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



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