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

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 09/11/2014 10:55 AM, Jan Beulich wrote:
> > > > On 11.09.14 at 15:54, <boris.ostrovsky@xxxxxxxxxx> wrote:
> > On 09/11/2014 02:39 AM, Jan Beulich wrote:
> > > > > > On 10.09.14 at 19:23, <boris.ostrovsky@xxxxxxxxxx> wrote:
> > > > On 09/10/2014 10:45 AM, Jan Beulich wrote:
> > > > > > > > On 04.09.14 at 05:41, <boris.ostrovsky@xxxxxxxxxx> wrote:
> > > > > > +struct xen_pmu_arch {
> > > > > > +    union {
> > > > > > +        struct cpu_user_regs regs;
> > > > > > +        uint8_t pad[256];
> > > > > > +    } r;
> > > > > Can you remind me again what you need the union and padding for
> > > > > here?
> > > > This structure is laid out in a shared page with a (possibly 32-bit)
> > > > guest who need to access fields that follow this union.
> > > Hmm, okay. But how would such a guest make reasonable use of
> > > the regs field then?
> > When hypervisor is preparing this data for 32-bit consumer in
> > vpmu_do_interrupts() it translates registers to 32-bit version:
> >
> >           struct compat_cpu_user_regs *cmp;
> >           gregs = guest_cpu_user_regs();
> >           cmp = (void *)&vpmu->xenpmu_data->pmu.r.regs;
> >           XLAT_cpu_user_regs(cmp, gregs);
> >
> > I remember struggling trying to figure a better way of presenting this
> > but ended up with the (void *) cast. IIRC I tried putting
> > compat_cpu_user_regs into the union but something didn't quite work
> > (with compilation).
> Of course that can't work - the compat structure simply doesn't
> exist for public headers.
>
> > > And then - why 256 and not 200? struct
> > > cpu_user_regs can't change size anyway. Plus, finally, why do
> > > you expose the GPRs but not any of the other register state?
> > I wanted to leave some padding in case we decide to add non-GPR
> > registers and keep major version of the interface unchanged (only minor
> > version will bumped). TBH though, I can't think of any non-GPR registers
> > to be ever useful.
> Then what do you need the GPRs for here? I don't think they're
> any better or worse than, say, XMM ones. I could see you needing/
> wanting some basic stuff like CS:RIP and SS:RSP and maybe EFLAGS,
> but that's about it.
I believe some perf sub-tools (tracing-related if I am not mistaken) 
want to have access to traced function's arguments.
This is obviously not supported by series but one day it may be. (This 
day will be far in the future)
-boris


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