Re: [PATCH 18/18] PVH xen: introduce vmx_pvh.c

[Mukesh Rathor <mukesh.rathor@oracle.com>]

On Fri, 28 Jun 2013 10:31:53 +0100
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 28.06.13 at 03:35, Mukesh Rathor <mukesh.rathor@oracle.com>
> >>> wrote:
> > On Tue, 25 Jun 2013 11:49:57 +0100
> > "Jan Beulich" <JBeulich@suse.com> wrote:
> > 
> >> >>> On 25.06.13 at 02:01, Mukesh Rathor <mukesh.rathor@oracle.com>
> >> >>> wrote:
> >> > --- /dev/null
........
> >> Which raises the question of whether your uses of
> >> guest_kernel_mode() are appropriate in the first place: Before this
> >> series there's no use at all under xen/arch/x86/hvm/.
> >> 
> >> And if it is, I'd like to point out once again that this check
> >> should be looking at SS.DPL, not CS.RPL.
> > 
> > Are you suggesting changing the macro to check for SS.DPL instead of
> > CS.RPL it has always done for PV also? Note, PVH has checks in this
> > patch to enforce long mode execution always, so CS.RPL should always
> > be valid for PVH.
> 
> I'm saying that guest_kernel_mode() should be looking at the
> VMCS for PVH (and, should it happen to be used in HVM code
> paths, for HVM too) rather than struct cpu_user_regs. That
> makes the saving of the CS selector pointless (in line with how
> HVM behaves), and once you're going through
> hvm_get_segment_register(), you can as well do this properly
> (i.e. look at SS.DPL rather than CS.RPL). And no, repeatedly

Ok, lmk if you are ok with following:

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index d502bdf..eb5706e 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -41,6 +41,7 @@ obj-y += numa.o
 obj-y += pci.o
 obj-y += percpu.o
 obj-y += physdev.o
+obj-y += pvh.o
 obj-y += setup.o
 obj-y += shutdown.o
 obj-y += smp.o
diff --git a/xen/arch/x86/pvh.c b/xen/arch/x86/pvh.c
new file mode 100644
index 0000000..db9d434
--- /dev/null
+++ b/xen/arch/x86/pvh.c
@@ -0,0 +1,11 @@
+#include <xen/sched.h>
+#include <asm/hvm/hvm.h>
+
+bool_t pvh_kernel_mode(const struct vcpu *v)
+{
+    struct segment_register seg;
+
+    hvm_get_segment_register((struct vcpu *)v, x86_seg_ss, &seg);
+    return (seg.attr.fields.dpl != 3);
+}
+
diff --git a/xen/include/asm-x86/x86_64/regs.h b/xen/include/asm-x86/x86_64/regs.h
index 2ea49c5..c437a41 100644
--- a/xen/include/asm-x86/x86_64/regs.h
+++ b/xen/include/asm-x86/x86_64/regs.h
@@ -10,8 +10,10 @@
 #define ring_2(r)    (((r)->cs & 3) == 2)
 #define ring_3(r)    (((r)->cs & 3) == 3)
 
+bool_t pvh_kernel_mode(const struct vcpu *);
+
 #define guest_kernel_mode(v, r)                                   \
-    (is_pvh_vcpu(v) ? (ring_0(r)) :                               \
+    (is_pvh_vcpu(v) ? (pvh_kernel_mode(v)) :                      \
      (!is_pv_32bit_vcpu(v) ?                                      \
       (ring_3(r) && ((v)->arch.flags & TF_kernel_mode)) :         \
       (ring_1(r))))