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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.05.15 at 14:56, <ian.campbell@xxxxxxxxxx> wrote:
> On Tue, 2015-05-26 at 13:25 +0100, Jan Beulich wrote:
>> >>> On 26.05.15 at 13:14, <ian.campbell@xxxxxxxxxx> 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.

Jan


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