Re: [PATCH v10 09/20] x86/VPMU: Add public xenpmu.h

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Fri, Sep 12, 2014 at 11:18:48AM -0400, Boris Ostrovsky wrote:
> On 09/12/2014 10:38 AM, Jan Beulich wrote:
> >>>>On 12.09.14 at 16:21, <boris.ostrovsky@oracle.com> wrote:
> >>On 09/12/2014 02:50 AM, Jan Beulich wrote:
> >>>Okay, so you're tailoring the hypervisor interface to Linux. That's
> >>>not what we generally aim for, and hence I continue to think this
> >>>isn't the right approach.
> >>But we know that at least one consumer of this interface (Linux/perf)
> >>does want to have full GPR set.
> >You said they use a struct pt_regs somewhere - that's not the same
> >as struct cpu_user_regs (and the overlap of registers being in both
> >is not much more than a coincidence), and you didn't clarify yet how
> >much of this information is _actually_ getting consumed (i.e. this may
> >well just happen to be a suitable container for conveying a smaller
> >set).
> 
> I don't know how all registers are consumed in Linux code (except for RIP
> and CS) and I am not sure it's all that important: we know that we need to
> pass GPRs to perf infrastructure which, for the most part, is a black box
> (especially given the insane amount of code churn that happens in perf).
> 
> If and when we find out that something that perf wants is not provided in
> pt_regs (converted from cpu_user_regs) then we will have to deal with it.
> But it's not much different from what would happen if we use specific
> registers in the shared structure.
> 
> >
> >>So why not provide it? Especially since
> >>(I suspect that) doing memcpy may be faster than copying only selected
> >>fields.
> >>
> >>If you are categorically against this I can certainly rework this and
> >>pass RIP, RSP, CS and EFLAGS only.
> >My problem with this is that (a) it exposes an arbitrary subset of
> >registers and (b) in an incompatible way (a 32-bit profiling domain
> >should very well be told all 64-bit registers of a 64-bit guest it
> >profiles). Since b would need addressing by a custom structure
> >anyway, dealing with a at the same time seems reasonable to me.
> 
> Note that (b) is not supported --- 32-bit domains can only profile
> themselves and not the hypervisor (or other domains).

And that is due to the tools limitation of not being able to understand
64-bit values right?

> 
> >
> >But of course it would help if other interested parties would voice
> >their view of this...

My thinking (with the release manager hat) is that:

 1). Adding in the support for 32-bit consumer is nice but won't be
     used that often (64-bit consumer with 32-bit 'producer' - as in
     32-bit guests - will surely be present).

 2). Having the right ABI for these perf counters is hard across
     two set of vendors (and what if we want to expand this for ARM?).

 3). The one 'generic' profiling tool that worked across all OSes
     was 'oprofile' which has hit the dustpan. We still support
     it with the oprofile hypercalls. Doing something generic
     might not be the best way going forward.

 4). The ABI should have means to look in the future in case things
     change (and then there is a method to provide the version information
     in case the ABI alters). Future version of the Intel
     or AMD performance counters architecture might expand the
     amount of registers they expose. Or they might not.
     Either way, a new minor rev would have to be increased to
     benefit from this.

 5). Exposing only CS, RIP, and EFLAGS seems to be tailoring the
     interface for Linux perf use case.

 6). Exposing all of the GPRs, while not needed for the current
     set of Linux patches, could allow other tools to use this
     (Intel's tracing tool for example - which uses the perf
     ABI). But surely there are tools on FreeBSD, NetBSD that
     do profiling.


If I understand Jan's concerns correctly, the question is -
if we are not using all of the GPRs right now in the tools
why even bother expanding it? And if the new version of
the architecture does expose it -  and we have the support
- then we can expand the page structure? (and of course
rev the major).

While Boris's view is that the condition above will happen -
we will expand the registers/use case - so why do the
intermediate step of expanding the page structure - when we can
make the structure bigger now (and rev the minor).

Is that about right?