Re: [Xen-devel] [PATCH v2 2/2] xen: implement VCPUOP_register_runstate_phys_memory_area

[Julien Grall <julien.grall@xxxxxxx>]

Hi Jan,

On 09/05/2019 10:27, Jan Beulich wrote:
> > > > On 08.05.19 at 17:40, <julien.grall@xxxxxxx> wrote:
> > On 23/04/2019 09:10, Andrii Anisov wrote:
> > > --- a/xen/include/xen/sched.h
> > > +++ b/xen/include/xen/sched.h
> > > @@ -163,15 +163,23 @@ struct vcpu
> > >        void            *sched_priv;    /* scheduler-specific data */
> > >    
> > >        struct vcpu_runstate_info runstate;
> > > +
> > > +    spinlock_t      mapped_runstate_lock;
> > > +
> > >    #ifndef CONFIG_COMPAT
> > >    # define runstate_guest(v) ((v)->runstate_guest)
> > >        XEN_GUEST_HANDLE(vcpu_runstate_info_t) runstate_guest; /* guest address 
> > > */
> > > +    vcpu_runstate_info_t *mapped_runstate;
> > >    #else
> > >    # define runstate_guest(v) ((v)->runstate_guest.native)
> > >        union {
> > >            XEN_GUEST_HANDLE(vcpu_runstate_info_t) native;
> > >            XEN_GUEST_HANDLE(vcpu_runstate_info_compat_t) compat;
> > >        } runstate_guest; /* guest address */
> > > +    union {
> > > +        vcpu_runstate_info_t* native;
> > > +        vcpu_runstate_info_compat_t* compat;
> > > +    } mapped_runstate; /* guest address */
> > The combination of mapped_runstate and runstate_guest is a bit confusing. I
> > think you want to rework the interface to show that only one is possible at the
> > time and make clear which one is used by who. Maybe:
> >
> > union
> > {
> >      /* Legacy interface to be used when the guest provides a virtual address */
> >      union {
> >         XEN_GUEST_HANDLE(vcpu_runstate_info_t) native;
> >         ...
> >      } virt;
> >
> >      /* Interface used when the guest provides a physical address */
> >      union {
> >      } phys;
> > } runstate_guest.
> >
> > runstate_guest_type /* could be a bool or enum */
> >
> > Jan what do you think?
> I fully agree - no mixing of approaches here, if possible. However,
> care needs to be taken that after a domain reset the new kernel
> can chose the opposite model. Question is whether there are even
> other cases where independent components (say boot loader and
> OS) may need to be permitted to chose models independently of
> one another.
Good point. On a similar topic, how does Kexec works on Xen? Do we reset the 
domain as well?
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel