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

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

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
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)


Xen-devel mailing list



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