xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Mukesh Rathor <mukesh.rathor@oracle.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xen.org>
Subject: Re: [PATCH 09/18] PVH xen: Support privileged op emulation for PVH
Date: Wed, 3 Jul 2013 19:00:16 -0700	[thread overview]
Message-ID: <20130703190016.1a20e418@mantra.us.oracle.com> (raw)
In-Reply-To: <51D4174002000078000E26E3@nat28.tlf.novell.com>

On Wed, 03 Jul 2013 11:21:20 +0100
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 03.07.13 at 03:38, Mukesh Rathor <mukesh.rathor@oracle.com>
> >>> wrote:
> > On Fri, 28 Jun 2013 10:20:47 +0100
> > "Jan Beulich" <JBeulich@suse.com> wrote:
> > 
> >> >>> On 28.06.13 at 01:43, Mukesh Rathor <mukesh.rathor@oracle.com>
> >> >>> wrote:
.............
> >> The problem is that you think that now and forever this macro
> >> will only be used from the MMIO emulation path (or some such, in
> >> any case - just from one very specific path). This is an assumption
> >> you may make while in an early development phase, but not in
> >> patches that you intend to be committed: Someone adding another
> >> use of the macro is _very_ unlikely to go and check what contraints
> >> apply to that use. The macro has to work in the general case.
> > 
> > Hmm.. Ok, I still fail to see the difference, caching upfront
> > always is such a low overhead.
> 
> Even if it really is (which I doubt), you still would make PVH
> different from both PV and HVM, which both don't populate the
> selector fields of the frame (PV obviously has ->cs and ->ss
> populated [by the CPU], but HVM avoids even that).

And what's wrong with PVH being little different? If anything, that
makes the code easier than to make it like PVH/HVM IMHO. HVM may not
cache selectors, but caches other things like CR3 upfront:

vmx_vmexit_handler():
        v->arch.hvm_vcpu.guest_cr[3] = v->arch.hvm_vcpu.hw_cr[3] =
                    __vmread(GUEST_CR3);

> We'll have to see - at the first glance I don't follow...

Here's what I am talking about:

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 16e25e9..ab1953f 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1241,10 +1241,10 @@ static void save_segments(struct vcpu *v)
     struct cpu_user_regs *regs = &v->arch.user_regs;
     unsigned int dirty_segment_mask = 0;
 
-    regs->ds = read_segment_register(v, regs, ds);
-    regs->es = read_segment_register(v, regs, es);
-    regs->fs = read_segment_register(v, regs, fs);
-    regs->gs = read_segment_register(v, regs, gs);
+    regs->ds = read_segment_register(v, regs, x86_seg_ds);
+    regs->es = read_segment_register(v, regs, x86_seg_es);
+    regs->fs = read_segment_register(v, regs, x86_seg_fs);
+    regs->gs = read_segment_register(v, regs, x86_seg_gs);
 
     if ( regs->ds )
         dirty_segment_mask |= DIRTY_DS;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index e0d84af..5318126 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -676,6 +676,45 @@ static void vmx_ctxt_switch_to(struct vcpu *v)
         .fields = { .type = 0xb, .s = 0, .dpl = 0, .p = 1, .avl = 0,    \
                     .l = 0, .db = 0, .g = 0, .pad = 0 } }).bytes)
 
+u16 vmx_get_selector(struct vcpu *v, enum x86_segment seg)
+{
+    u16 sel = 0;
+
+    vmx_vmcs_enter(v);
+    switch ( seg )
+    {
+    case x86_seg_cs:
+        sel = __vmread(GUEST_CS_SELECTOR);
+        break;
+
+    case x86_seg_ss:
+        sel = __vmread(GUEST_SS_SELECTOR);
+        break;
+
+    case x86_seg_es:
+        sel = __vmread(GUEST_ES_SELECTOR);
+        break;
+
+    case x86_seg_ds:
+        sel = __vmread(GUEST_DS_SELECTOR);
+        break;
+
+    case x86_seg_fs:
+        sel = __vmread(GUEST_FS_SELECTOR);
+        break;
+
+    case x86_seg_gs:
+        sel = __vmread(GUEST_GS_SELECTOR);
+        break;
+
+    default:
+        BUG();
+    }
+    vmx_vmcs_exit(v);
+
+    return sel;
+}
+
 void vmx_get_segment_register(struct vcpu *v, enum x86_segment seg,
                               struct segment_register *reg)
 {
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index b638a6e..6b6989a 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -112,6 +112,52 @@ boolean_param("ler", opt_ler);
 #define stack_words_per_line 4
 #define ESP_BEFORE_EXCEPTION(regs) ((unsigned long *)regs->rsp)
 
+/*
+ * We need vcpu because during context switch, going from pure PV to PVH,
+ * in save_segments(), current has been updated to next, and no longer pointing
+ * to the pure PV.
+ */
+u16 read_segment_register(struct vcpu *vcpu,
+    struct cpu_user_regs *regs, enum x86_segment seg)
+{
+    u16 sel = 0;
+
+    if ( is_pvh_vcpu(vcpu) && guest_mode(regs) )
+        sel = pvh_get_selector(vcpu, seg);
+    else
+    {
+        switch ( seg )
+        {
+        case x86_seg_cs:
+            asm volatile ( "movw %%cs,%0" : "=r" (sel) );
+            break;
+
+        case x86_seg_ss:
+            asm volatile ( "movw %%ss,%0" : "=r" (sel) );
+            break;
+
+        case x86_seg_es:
+            asm volatile ( "movw %%es,%0" : "=r" (sel) );
+            break;
+
+        case x86_seg_ds:
+            asm volatile ( "movw %%ds,%0" : "=r" (sel) );
+            break;
+
+        case x86_seg_fs:
+            asm volatile ( "movw %%fs,%0" : "=r" (sel) );
+            break;
+
+        case x86_seg_gs:
+            asm volatile ( "movw %%gs,%0" : "=r" (sel) );
+            break;
+
+        default:
+            BUG();
+        }
+    }
+    return sel;
+}
 static void show_guest_stack(struct vcpu *v, struct cpu_user_regs *regs)
 {
     int i;
@@ -1940,7 +1986,7 @@ int emulate_privileged_op(struct cpu_user_regs *regs)
         goto fail;
 
     /* emulating only opcodes not allowing SS to be default */
-    data_sel = read_segment_register(v, regs, ds);
+    data_sel = read_segment_register(v, regs, x86_seg_ds);
     which_sel = x86_seg_ds;
 
     /* Legacy prefixes. */
@@ -1960,20 +2006,20 @@ int emulate_privileged_op(struct cpu_user_regs *regs)
             which_sel = x86_seg_cs;
             continue;
         case 0x3e: /* DS override */
-            data_sel = read_segment_register(v, regs, ds);
+            data_sel = read_segment_register(v, regs, x86_seg_ds);
             which_sel = x86_seg_ds;
             continue;
         case 0x26: /* ES override */
-            data_sel = read_segment_register(v, regs, es);
+            data_sel = read_segment_register(v, regs, x86_seg_es);
             which_sel = x86_seg_es;
             continue;
         case 0x64: /* FS override */
-            data_sel = read_segment_register(v, regs, fs);
+            data_sel = read_segment_register(v, regs, x86_seg_fs);
             which_sel = x86_seg_fs;
             lm_ovr = lm_seg_fs;
             continue;
         case 0x65: /* GS override */
-            data_sel = read_segment_register(v, regs, gs);
+            data_sel = read_segment_register(v, regs, x86_seg_gs);
             which_sel = x86_seg_gs;
             lm_ovr = lm_seg_gs;
             continue;
@@ -2022,7 +2068,7 @@ int emulate_privileged_op(struct cpu_user_regs *regs)
 
         if ( !(opcode & 2) )
         {
-            data_sel = read_segment_register(v, regs, es);
+            data_sel = read_segment_register(v, regs, x86_seg_es);
             which_sel = x86_seg_es;
             lm_ovr = lm_seg_none;
         }
@@ -2769,22 +2815,22 @@ static void emulate_gate_op(struct cpu_user_regs *regs)
             ASSERT(opnd_sel);
             continue;
         case 0x3e: /* DS override */
-            opnd_sel = read_segment_register(v, regs, ds);
+            opnd_sel = read_segment_register(v, regs, x86_seg_ds);
             if ( !opnd_sel )
                 opnd_sel = dpl;
             continue;
         case 0x26: /* ES override */
-            opnd_sel = read_segment_register(v, regs, es);
+            opnd_sel = read_segment_register(v, regs, x86_seg_es);
             if ( !opnd_sel )
                 opnd_sel = dpl;
             continue;
         case 0x64: /* FS override */
-            opnd_sel = read_segment_register(v, regs, fs);
+            opnd_sel = read_segment_register(v, regs, x86_seg_fs);
             if ( !opnd_sel )
                 opnd_sel = dpl;
             continue;
         case 0x65: /* GS override */
-            opnd_sel = read_segment_register(v, regs, gs);
+            opnd_sel = read_segment_register(v, regs, x86_seg_gs);
             if ( !opnd_sel )
                 opnd_sel = dpl;
             continue;
@@ -2837,7 +2883,8 @@ static void emulate_gate_op(struct cpu_user_regs *regs)
                             switch ( modrm & 7 )
                             {
                             default:
-                                opnd_sel = read_segment_register(v, regs, ds);
+                                opnd_sel = read_segment_register(v, regs, 
+                                                                 x86_seg_ds);
                                 break;
                             case 4: case 5:
                                 opnd_sel = regs->ss;
@@ -2865,7 +2912,8 @@ static void emulate_gate_op(struct cpu_user_regs *regs)
                             break;
                         }
                         if ( !opnd_sel )
-                            opnd_sel = read_segment_register(v, regs, ds);
+                            opnd_sel = read_segment_register(v, regs, 
+                                                             x86_seg_ds);
                         switch ( modrm & 7 )
                         {
                         case 0: case 2: case 4:
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index 0df1e1c..fbf6506 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -122,10 +122,10 @@ void show_registers(struct cpu_user_regs *regs)
         fault_crs[0] = read_cr0();
         fault_crs[3] = read_cr3();
         fault_crs[4] = read_cr4();
-        fault_regs.ds = read_segment_register(v, regs, ds);
-        fault_regs.es = read_segment_register(v, regs, es);
-        fault_regs.fs = read_segment_register(v, regs, fs);
-        fault_regs.gs = read_segment_register(v, regs, gs);
+        fault_regs.ds = read_segment_register(v, regs, x86_seg_ds);
+        fault_regs.es = read_segment_register(v, regs, x86_seg_es);
+        fault_regs.fs = read_segment_register(v, regs, x86_seg_fs);
+        fault_regs.gs = read_segment_register(v, regs, x86_seg_gs);
     }
 
     print_xen_info();
@@ -240,10 +240,10 @@ void do_double_fault(struct cpu_user_regs *regs)
     crs[2] = read_cr2();
     crs[3] = read_cr3();
     crs[4] = read_cr4();
-    regs->ds = read_segment_register(current, regs, ds);
-    regs->es = read_segment_register(current, regs, es);
-    regs->fs = read_segment_register(current, regs, fs);
-    regs->gs = read_segment_register(current, regs, gs);
+    regs->ds = read_segment_register(current, regs, x86_seg_ds);
+    regs->es = read_segment_register(current, regs, x86_seg_es);
+    regs->fs = read_segment_register(current, regs, x86_seg_fs);
+    regs->gs = read_segment_register(current, regs, x86_seg_gs);
 
     printk("CPU:    %d\n", cpu);
     _show_registers(regs, crs, CTXT_hypervisor, NULL);
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 7e21ee1..c63988d 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -194,6 +194,7 @@ struct hvm_function_table {
                                 bool_t access_w, bool_t access_x);
     int (*pvh_set_vcpu_info)(struct vcpu *v, struct vcpu_guest_context *ctxtp);
+    u16 (*vmx_read_selector)(struct vcpu *v, enum x86_segment seg);
 };
 
 extern struct hvm_function_table hvm_funcs;
@@ -308,6 +309,11 @@ static inline void hvm_flush_guest_tlbs(void)
 void hvm_hypercall_page_initialise(struct domain *d,
                                    void *hypercall_page);
 
+static inline u16 pvh_get_selector(struct vcpu *v, enum x86_segment seg)
+{
+    return hvm_funcs.vmx_read_selector(v, seg);
+}
+
 static inline void
 hvm_get_segment_register(struct vcpu *v, enum x86_segment seg,
                          struct segment_register *reg)
diff --git a/xen/include/asm-x86/system.h b/xen/include/asm-x86/system.h
index 035944f..d5d90ca 100644
--- a/xen/include/asm-x86/system.h
+++ b/xen/include/asm-x86/system.h
@@ -4,22 +4,6 @@
 #include <xen/lib.h>
 #include <xen/bitops.h>
 
-/*
- * We need vcpu because during context switch, going from pure PV to PVH,
- * in save_segments(), current has been updated to next, and no longer pointing
- * to the pure PV. Note: for PVH, we update regs->selectors on each vmexit.
- */
-#define read_segment_register(vcpu, regs, name)                   \
-({  u16 __sel;                                                    \
-    struct cpu_user_regs *_regs = (regs);                         \
-                                                                  \
-    if ( is_pvh_vcpu(vcpu) && guest_mode(regs) )                  \
-        __sel = _regs->name;                                      \
-    else                                                          \
-        asm volatile ( "movw %%" #name ",%0" : "=r" (__sel) );    \
-    __sel;                                                        \
-})
-
 #define wbinvd() \
     asm volatile ( "wbinvd" : : : "memory" )
 
diff --git a/xen/include/asm-x86/traps.h b/xen/include/asm-x86/traps.h
index 202e3be..c4a2e2e 100644
--- a/xen/include/asm-x86/traps.h
+++ b/xen/include/asm-x86/traps.h
@@ -50,4 +50,6 @@ extern int send_guest_trap(struct domain *d, uint16_t vcpuid,
 				unsigned int trap_nr);
 
 int emulate_privileged_op(struct cpu_user_regs *regs);
+u16 read_segment_register(struct vcpu *vcpu,
+    struct cpu_user_regs *regs, enum x86_segment seg);
 #endif /* ASM_TRAP_H */

  reply	other threads:[~2013-07-04  2:00 UTC|newest]

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-25  0:01 [PATCH 00/18][V7]: PVH xen: Phase I, Version 7 patches Mukesh Rathor
2013-06-25  0:01 ` [PATCH 01/18] PVH xen: turn gdb_frames/gdt_ents into union Mukesh Rathor
2013-06-25  8:40   ` Jan Beulich
2013-06-25  0:01 ` [PATCH 02/18] PVH xen: add params to read_segment_register Mukesh Rathor
2013-06-25  0:01 ` [PATCH 03/18] PVH xen: Move e820 fields out of pv_domain struct Mukesh Rathor
2013-06-25  8:44   ` Jan Beulich
2013-06-25  0:01 ` [PATCH 04/18] PVH xen: vmx related preparatory changes for PVH Mukesh Rathor
2013-06-25  8:48   ` Jan Beulich
2013-06-25  0:01 ` [PATCH 05/18] PVH xen: hvm/vmcs " Mukesh Rathor
2013-06-25  8:51   ` Jan Beulich
2013-06-25  0:01 ` [PATCH 06/18] PVH xen: Introduce PVH guest type and some basic changes Mukesh Rathor
2013-06-25  9:01   ` Jan Beulich
2013-06-26  1:14     ` Mukesh Rathor
2013-06-26  8:18       ` Jan Beulich
2013-06-25  0:01 ` [PATCH 07/18] PVH xen: domain create, schedular related code changes Mukesh Rathor
2013-06-25  9:13   ` Jan Beulich
2013-06-25  0:01 ` [PATCH 08/18] PVH xen: support invalid op emulation for PVH Mukesh Rathor
2013-06-25  9:16   ` Jan Beulich
2013-06-25  0:01 ` [PATCH 09/18] PVH xen: Support privileged " Mukesh Rathor
2013-06-25  9:36   ` Jan Beulich
2013-06-26 22:41     ` Mukesh Rathor
2013-06-27  7:22       ` Jan Beulich
2013-06-27 23:43         ` Mukesh Rathor
2013-06-28  9:20           ` Jan Beulich
2013-07-03  1:38             ` Mukesh Rathor
2013-07-03 10:21               ` Jan Beulich
2013-07-04  2:00                 ` Mukesh Rathor [this message]
2013-07-04  8:04                   ` Jan Beulich
2013-07-06  1:43                     ` Mukesh Rathor
2013-06-25  0:01 ` [PATCH 10/18] PVH xen: interrupt/event-channel delivery to PVH Mukesh Rathor
2013-06-25 14:29   ` Konrad Rzeszutek Wilk
2013-07-12  0:29     ` Mukesh Rathor
2013-06-25  0:01 ` [PATCH 11/18] PVH xen: additional changes to support PVH guest creation and execution Mukesh Rathor
2013-06-25  0:01 ` [PATCH 12/18] PVH xen: mapcache and show registers Mukesh Rathor
2013-06-25  9:45   ` Jan Beulich
2013-06-25  0:01 ` [PATCH 13/18] PVH xen: mtrr, tsc, grant changes Mukesh Rathor
2013-06-25 14:30   ` Konrad Rzeszutek Wilk
2013-06-25  0:01 ` [PATCH 14/18] PVH xen: Checks, asserts, and limitations for PVH Mukesh Rathor
2013-06-25  9:54   ` Jan Beulich
2013-06-27  2:43     ` Mukesh Rathor
2013-06-27  7:25       ` Jan Beulich
2013-06-25  0:01 ` [PATCH 15/18] PVH xen: add hypercall support " Mukesh Rathor
2013-06-25 10:12   ` Jan Beulich
2013-06-27  3:09     ` Mukesh Rathor
2013-06-27  7:29       ` Jan Beulich
2013-06-25  0:01 ` [PATCH 16/18] PVH xen: vmcs related changes Mukesh Rathor
2013-06-25 10:17   ` Jan Beulich
2013-06-25  0:01 ` [PATCH 17/18] PVH xen: HVM support of PVH guest creation/destruction Mukesh Rathor
2013-06-25  0:01 ` [PATCH 18/18] PVH xen: introduce vmx_pvh.c Mukesh Rathor
2013-06-25 10:49   ` Jan Beulich
2013-06-27  3:30     ` Mukesh Rathor
2013-06-27  7:41       ` Jan Beulich
2013-06-28  1:28         ` Mukesh Rathor
2013-06-28  9:26           ` Jan Beulich
2013-06-28  1:35     ` Mukesh Rathor
2013-06-28  9:31       ` Jan Beulich
2013-06-29  3:03         ` Mukesh Rathor
2013-07-01  8:49           ` Jan Beulich
2013-07-06  1:31         ` Mukesh Rathor
2013-07-08  8:31           ` Jan Beulich
2013-07-08 23:09             ` Mukesh Rathor
2013-07-09  0:01               ` Mukesh Rathor
2013-07-09  7:31                 ` Jan Beulich
2013-07-10  0:33                   ` Mukesh Rathor
2013-07-10  7:20                     ` Jan Beulich
2013-06-28  2:28     ` Mukesh Rathor
2013-06-28  9:44       ` Jan Beulich
2013-06-29  3:04         ` Mukesh Rathor
2013-07-01  8:54           ` Jan Beulich
2013-07-02  2:01             ` Mukesh Rathor
2013-07-03  1:40         ` Mukesh Rathor
2013-07-03 10:25           ` Jan Beulich
2013-07-04  2:02             ` Mukesh Rathor
2013-07-04  8:07               ` Jan Beulich
2013-07-16  2:00         ` Mukesh Rathor
2013-07-16  6:50           ` Jan Beulich
2013-07-17  0:47             ` Mukesh Rathor
2013-07-17  6:36               ` Jan Beulich
2013-06-25 10:17 ` [PATCH 00/18][V7]: PVH xen: Phase I, Version 7 patches George Dunlap
2013-06-26  0:04   ` Mukesh Rathor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130703190016.1a20e418@mantra.us.oracle.com \
    --to=mukesh.rathor@oracle.com \
    --cc=JBeulich@suse.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).