Re: [Xen-devel] Ping: [PATCH v3] x86/HVM: correct page dirty marking in hvm_map_guest_frame_rw()

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Oct 26, 2015 at 07:09:06AM -0600, Jan Beulich wrote:
> >>> On 19.10.15 at 16:53, <JBeulich@xxxxxxxx> wrote:
> > --- a/tools/libxc/xc_sr_save.c
> > +++ b/tools/libxc/xc_sr_save.c
> > @@ -537,7 +537,8 @@ static int suspend_and_send_dirty(struct
> >      if ( xc_shadow_control(
> >               xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_CLEAN,
> >               HYPERCALL_BUFFER(dirty_bitmap), ctx->save.p2m_size,
> > -             NULL, 0, &stats) != ctx->save.p2m_size )
> > +             NULL, XEN_DOMCTL_SHADOW_LOGDIRTY_FINAL, &stats) !=
> > +         ctx->save.p2m_size )
> >      {
> >          PERROR("Failed to retrieve logdirty bitmap");
> >          rc = -1;
> 
> Tools maintainers, could I please get an ack or otherwise on this
> adjustment?
> 

Er, there seems to be behaviour change that would cause external users
of xc_shadow_control cease to function properly.

Does it make sense to use inverse logic that you set a flag to not
mark permanent pages dirty? I think that's a bit awkward, not sure if it
is worth it.

I also realise that libxenctrl doesn't have stable interface so I'm a
bit two-minded for this.

Ian and Ian, what do you think?

Wei.

> > --- a/xen/include/public/domctl.h
> > +++ b/xen/include/public/domctl.h
> > @@ -208,6 +208,13 @@ struct xen_domctl_getpageframeinfo3 {
> >    */
> >  #define XEN_DOMCTL_SHADOW_ENABLE_EXTERNAL  (1 << 4)
> >  
> > +/* Mode flags for XEN_DOMCTL_SHADOW_OP_{CLEAN,PEEK}. */
> > + /*
> > +  * This is the final iteration: Requesting to include pages mapped
> > +  * writably by the hypervisor in the dirty bitmap.
> > +  */
> > +#define XEN_DOMCTL_SHADOW_LOGDIRTY_FINAL   (1 << 0)
> > +
> >  struct xen_domctl_shadow_op_stats {
> >      uint32_t fault_count;
> >      uint32_t dirty_count;
> > @@ -219,8 +226,9 @@ struct xen_domctl_shadow_op {
> >      /* IN variables. */
> >      uint32_t       op;       /* XEN_DOMCTL_SHADOW_OP_* */
> >  
> > -    /* OP_ENABLE */
> > -    uint32_t       mode;     /* XEN_DOMCTL_SHADOW_ENABLE_* */
> > +    /* OP_ENABLE: XEN_DOMCTL_SHADOW_ENABLE_* */
> > +    /* OP_PEAK / OP_CLEAN: XEN_DOMCTL_SHADOW_LOGDIRTY_* */
> > +    uint32_t       mode;
> >  
> >      /* OP_GET_ALLOCATION / OP_SET_ALLOCATION */
> >      uint32_t       mb;       /* Shadow memory allocation in MB */
> 
> "REST" maintainers, could I please get an ack or otherwise on this
> interface change?
> 
> Thanks, Jan
> 

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