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

Re: [Xen-devel] [PATCH v6 15/19] x86/VPMU: Add privileged PMU mode

On 05/27/2014 05:10 AM, Jan Beulich wrote:
On 27.05.14 at 04:08, <boris.ostrovsky@xxxxxxxxxx> wrote:
On 05/26/2014 07:48 AM, Jan Beulich wrote:
+                gregs = guest_cpu_user_regs();
+                memcpy(&v->arch.vpmu.xenpmu_data->pmu.r.regs,
+                       gregs, sizeof(struct cpu_user_regs));
+            }
+            else
+                memcpy(&v->arch.vpmu.xenpmu_data->pmu.r.regs,
+                       regs, sizeof(struct cpu_user_regs));
- cmp = (void *)&v->arch.vpmu.xenpmu_data->pmu.r.regs;
-            XLAT_cpu_user_regs(cmp, gregs);
-            memcpy(&v->arch.vpmu.xenpmu_data->pmu.r.regs,
-                   &cmp, sizeof(struct compat_cpu_user_regs));
+            gregs = &v->arch.vpmu.xenpmu_data->pmu.r.regs;
+            gregs->cs = (current->arch.flags & TF_kernel_mode) ? 0 : 0x3;
Ah, no - you want to modify the structure here. But you could do this
directly on the ->pmu.r.regs field rather than first latching the pointer.

And as said before, it doesn't really look correct to simply set ->cs to
just the RPL, especially without any comment explaining why this is
(a) being done and (b) correct.
The reason for only passing up the RPL is that's the only field that the
guest is interested in (whether the interrupt happened in kernel or user
space). I added a comment in the code to this effect.

Do you think that all fields need to be passed?
How do you know what a guest is interested in? Namely 32-bit OSes
may have uses for more than a single flat code segment, and hence
may have an interest in knowing the full selector.

The registers are used only by the PMU handler in the guest and that code only wants to know where the (v)cpu was sampled.

OTOH there is really no reason *not* to pass full selector value. So I'll fix this.


Xen-devel mailing list



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