Re: [PATCH v2 1/2] xen: x86: copy back tsc info, not pointer to tsc info in domctl

[Ian Campbell <ian.campbell@citrix.com>]

On Tue, 2015-05-26 at 14:11 +0100, Jan Beulich wrote:
> >>> On 26.05.15 at 14:56, <ian.campbell@citrix.com> wrote:
> > On Tue, 2015-05-26 at 13:25 +0100, Jan Beulich wrote:
> >> >>> On 26.05.15 at 13:14, <ian.campbell@citrix.com> wrote:
> >> > --- a/xen/arch/x86/domctl.c
> >> > +++ b/xen/arch/x86/domctl.c
> >> > @@ -856,13 +856,16 @@ long arch_do_domctl(
> >> >              ret = -EINVAL;
> >> >          else
> >> >          {
> >> > +            xen_guest_tsc_info_t info = { 0 };
> >> > +
> >> >              domain_pause(d);
> >> > -            tsc_get_info(d, &domctl->u.tsc_info.info.tsc_mode,
> >> > -                         &domctl->u.tsc_info.info.elapsed_nsec,
> >> > -                         &domctl->u.tsc_info.info.gtsc_khz,
> >> > -                         &domctl->u.tsc_info.info.incarnation);
> >> > +            tsc_get_info(d, &info.tsc_mode,
> >> > +                            &info.elapsed_nsec,
> >> > +                            &info.gtsc_khz,
> >> > +                            &info.incarnation);
> >> >              domain_unpause(d);
> >> > -            copyback = 1;
> >> > +            if ( copy_to_guest(domctl->u.tsc_info.out_info, &info, 1) )
> >> > +                ret = -EFAULT;
> >> >          }
> >> >          break;
> >> >  
> >> 
> >> I have to admit that I don't see the point of this change when patch
> >> 2 basically undoes it all.
> > 
> > I thought so at first but the restructuring in the second patch was
> > large enough that I didn't want to mix it in with an actual functional
> > change. Plus the second patch does more than undo it, it removes "info."
> > from the domctl access.
> 
> Hmm, yeah. I guess in that case the fix (patch 1) could have been
> to drop out_info, and the cleanup (patch 2) to eliminate struct
> xen_guest_tsc_info. But anyway, feel free to commit as is, taking
> this as an ack for patch 2 even in its current shape.

Done, thanks.