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

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

On Mon, 8 Jul 2013 16:09:55 -0700
Mukesh Rathor <mukesh.rathor@oracle.com> wrote:

> On Mon, 08 Jul 2013 09:31:17 +0100
> "Jan Beulich" <JBeulich@suse.com> wrote:
> 
> > >>> On 06.07.13 at 03:31, Mukesh Rathor <mukesh.rathor@oracle.com>
> > >>> wrote:
> > > Ok, lmk if you are ok with following:
> > 
> > Fundamentally yes. But ...
> > 
> > > --- 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
> > 
> > Does this indeed warrant a separate file?
> 
> yeah, i wasn't sure about that, but was not sure where to put it. I
> think we could just have hvm_kernel_mode() next to
> hvm_get_segment_register in hvm.h. It can then also be used in HVM
> code in various places where it currently checks for dpl/cpl.

Actually, not feasible to put anything in any header since regs.h is a pretty
early-on header include, and can't include any other headers. So:

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 8284b3b..06f9470 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4642,6 +4642,14 @@ enum hvm_intblk nhvm_interrupt_blocked(struct vcpu *v)
     return hvm_funcs.nhvm_intr_blocked(v);
 }
 
+bool_t hvm_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 == 0);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/asm-x86/x86_64/regs.h b/xen/include/asm-x86/x86_64/regs
index 2ea49c5..7a9bc44 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 hvm_kernel_mode(const struct vcpu *);
+
 #define guest_kernel_mode(v, r)                                   \
-    (is_pvh_vcpu(v) ? (ring_0(r)) :                               \
+    (is_pvh_vcpu(v) ? (hvm_kernel_mode(v)) :                      \
      (!is_pv_32bit_vcpu(v) ?                                      \
       (ring_3(r) && ((v)->arch.flags & TF_kernel_mode)) :         \
       (ring_1(r))))


Also,
the cast in hvm_get_segment_register((struct vcpu *)v, x86_seg_ss, &seg);
is difficult to change since it triggers a chain of events down the
stream in *_get_segment_register(). So for now, I'll just leave it. 

thanks
Mukesh