[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v5 2/6] x86: allow reading MSR_IA32_TSC with XENPF_resource_op



>>> On 22.01.15 at 15:03, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 22/01/15 13:36, Jan Beulich wrote:
>>>>> On 22.01.15 at 13:53, <chao.p.peng@xxxxxxxxxxxxxxx> wrote:
>>> On Thu, Jan 22, 2015 at 11:20:15AM +0000, Jan Beulich wrote:
>>>>>>> On 21.01.15 at 12:19, <chao.p.peng@xxxxxxxxxxxxxxx> wrote:
>>>>> --- a/xen/arch/x86/platform_hypercall.c
>>>>> +++ b/xen/arch/x86/platform_hypercall.c
>>>>> @@ -61,7 +61,7 @@ long cpu_down_helper(void *data);
>>>>>  long core_parking_helper(void *data);
>>>>>  uint32_t get_cur_idle_nums(void);
>>>>>  
>>>>> -#define RESOURCE_ACCESS_MAX_ENTRIES 2
>>>>> +#define RESOURCE_ACCESS_MAX_ENTRIES 3
>>>> See my comment on an earlier version.
>>> The new added MSR_IA32_TSC and existed MSR_IA32_CMT_CTR should be
>>> read in an atomic unit. How to achieve this if not increase
>>> MAX_ENTRIES?
>> These are just two entries nevertheless.
> 
> The reasons for two in the first place is that it is an indirect MSR read.

Oh, right - I somehow thought this was needed for a plain MSR
read only.

> Upping MAX_ENTRIES to 3 and allowing the operation to get a timestamp as
> is the only way to synchronously perform the indirect register read and
> timestamp.

I agree then.

> Having said this, the only useful timestamp will be at the same point as
> performing the MSR read.  Having a 3rd operation tacked on the end to
> get a timestamp will be some arbitrary time later, especially if
> interrupts are enabled.

Perhaps the latching of NOW() could happen with the MSR read, and
the latched value then be stored upon encountering the respective
slot? That would also allow further limiting the interrupts disabled
period.

Jan


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.