Re: [Xen-devel] [PATCH v4 1/5] x86: allow reading MSR_IA32_TSC with XENPF_resource_op

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.01.15 at 12:20, <chao.p.peng@xxxxxxxxxxxxxxx> wrote:
> On Tue, Jan 20, 2015 at 10:02:32AM +0000, Jan Beulich wrote:
>> >>> On 20.01.15 at 10:45, <chao.p.peng@xxxxxxxxxxxxxxx> wrote:
>> > Memory bandwidth monitoring requires timestamp returned along with the
>> > monitoring counter to verify the correctness of the counter value.
>> > Allow MSR_IA32_TSC to be read and increase RESOURCE_ACCESS_MAX_ENTRIES
>> > to 3.
>> 
>> Do you really need an MSR access here, i.e. is RDTSC not
>> sufficient?
> 
> RDTSC is enough. The purpose of using MSR is to reuse the existed
> XEN_RESOURCE_OP_MSR_READ command. A new command XEN_RESOURCE_OP_TSC_READ
> (looks like not so generic) is needed If using RDTSC. If itâs not the
> issue, then RDTSC can be used instead.

I don't see the need for a sub-op - just filter out MSR_IA32_TSC
in the existing MSR read/write code paths.

>> > --- a/xen/arch/x86/platform_hypercall.c
>> > +++ b/xen/arch/x86/platform_hypercall.c
>> > @@ -61,14 +61,14 @@ 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
>> 
>> No - the limit on the number of consecutive entries doesn't change
>> because of your addition of another MSR. Actual batching is to be
>> done via multicalls, the multi-entry requests here are meant to be
>> used for successive operations that _must_ be issued without
>> preemption (e.g. an MSR write needed for a successive read).
>> 
> You remind me that the requirement here is more than "without
> preemption", ideally the exact timestamp the moment that the counter
> being read should be returned, which is used by used space to calculate
> the time elapsed between two samplings.  So the MSR read and TSC
> read should be atomic, which means, 1) preemption should not happen,
> 2) interrupt should be disabled. Sounds more complex as we have no existed
> way to disable interrupt, using back-to-back flag again?

Yeah, in that case pairing the access would be necessary. And
we have a rsvd field in struct xenpf_resource_entry, which you
could use to request IRQ disabling across the pair of accesses.

Jan

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