Xen devel - Mar 2013 - [PATCH 4/18 V2]: PVH xen: add params to read_segment_register

In this patch, read_segment_register macro is changed to take vcpu and
regs parameters so it can check if it''s PVH guest (change in upcoming
patches). No functionality change. Also, make emulate_privileged_op()
public for later while changing this file.

Changes in V2:  None

Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
---
 xen/arch/x86/domain.c        |    8 ++++----
 xen/arch/x86/traps.c         |   28 ++++++++++++++--------------
 xen/arch/x86/x86_64/traps.c  |   16 ++++++++--------
 xen/include/asm-x86/system.h |    2 +-
 xen/include/asm-x86/traps.h  |    1 +
 5 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index ea1381c..e9549e0 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1340,10 +1340,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(ds);
-    regs->es = read_segment_register(es);
-    regs->fs = read_segment_register(fs);
-    regs->gs = read_segment_register(gs);
+    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);
 
     if ( regs->ds )
         dirty_segment_mask |= DIRTY_DS;
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index d36eddd..ab3e814 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1823,7 +1823,7 @@ static inline uint64_t guest_misc_enable(uint64_t val)
     }                                                                       \
     (eip) += sizeof(_x); _x; })
 
-#define read_sreg(regs, sr) read_segment_register(sr)
+#define read_sreg(vcpu, regs, sr) read_segment_register(vcpu, regs, sr)
 
 static int is_cpufreq_controller(struct domain *d)
 {
@@ -1833,7 +1833,7 @@ static int is_cpufreq_controller(struct domain *d)
 
 #include "x86_64/mmconfig.h"
 
-static int emulate_privileged_op(struct cpu_user_regs *regs)
+int emulate_privileged_op(struct cpu_user_regs *regs)
 {
     struct vcpu *v = current;
     unsigned long *reg, eip = regs->eip;
@@ -1869,7 +1869,7 @@ static int emulate_privileged_op(struct cpu_user_regs
*regs)
         goto fail;
 
     /* emulating only opcodes not allowing SS to be default */
-    data_sel = read_sreg(regs, ds);
+    data_sel = read_sreg(v, regs, ds);
 
     /* Legacy prefixes. */
     for ( i = 0; i < 8; i++, rex == opcode || (rex = 0) )
@@ -1887,17 +1887,17 @@ static int emulate_privileged_op(struct cpu_user_regs
*regs)
             data_sel = regs->cs;
             continue;
         case 0x3e: /* DS override */
-            data_sel = read_sreg(regs, ds);
+            data_sel = read_sreg(v, regs, ds);
             continue;
         case 0x26: /* ES override */
-            data_sel = read_sreg(regs, es);
+            data_sel = read_sreg(v, regs, es);
             continue;
         case 0x64: /* FS override */
-            data_sel = read_sreg(regs, fs);
+            data_sel = read_sreg(v, regs, fs);
             lm_ovr = lm_seg_fs;
             continue;
         case 0x65: /* GS override */
-            data_sel = read_sreg(regs, gs);
+            data_sel = read_sreg(v, regs, gs);
             lm_ovr = lm_seg_gs;
             continue;
         case 0x36: /* SS override */
@@ -1944,7 +1944,7 @@ static int emulate_privileged_op(struct cpu_user_regs
*regs)
 
         if ( !(opcode & 2) )
         {
-            data_sel = read_sreg(regs, es);
+            data_sel = read_sreg(v, regs, es);
             lm_ovr = lm_seg_none;
         }
 
@@ -2677,22 +2677,22 @@ static void emulate_gate_op(struct cpu_user_regs *regs)
             ASSERT(opnd_sel);
             continue;
         case 0x3e: /* DS override */
-            opnd_sel = read_sreg(regs, ds);
+            opnd_sel = read_sreg(v, regs, ds);
             if ( !opnd_sel )
                 opnd_sel = dpl;
             continue;
         case 0x26: /* ES override */
-            opnd_sel = read_sreg(regs, es);
+            opnd_sel = read_sreg(v, regs, es);
             if ( !opnd_sel )
                 opnd_sel = dpl;
             continue;
         case 0x64: /* FS override */
-            opnd_sel = read_sreg(regs, fs);
+            opnd_sel = read_sreg(v, regs, fs);
             if ( !opnd_sel )
                 opnd_sel = dpl;
             continue;
         case 0x65: /* GS override */
-            opnd_sel = read_sreg(regs, gs);
+            opnd_sel = read_sreg(v, regs, gs);
             if ( !opnd_sel )
                 opnd_sel = dpl;
             continue;
@@ -2745,7 +2745,7 @@ static void emulate_gate_op(struct cpu_user_regs *regs)
                             switch ( modrm & 7 )
                             {
                             default:
-                                opnd_sel = read_sreg(regs, ds);
+                                opnd_sel = read_sreg(v, regs, ds);
                                 break;
                             case 4: case 5:
                                 opnd_sel = regs->ss;
@@ -2773,7 +2773,7 @@ static void emulate_gate_op(struct cpu_user_regs *regs)
                             break;
                         }
                         if ( !opnd_sel )
-                            opnd_sel = read_sreg(regs, ds);
+                            opnd_sel = read_sreg(v, regs, 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 eec919a..d2f7209 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(ds);
-        fault_regs.es = read_segment_register(es);
-        fault_regs.fs = read_segment_register(fs);
-        fault_regs.gs = read_segment_register(gs);
+        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);
     }
 
     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(ds);
-    regs->es = read_segment_register(es);
-    regs->fs = read_segment_register(fs);
-    regs->gs = read_segment_register(gs);
+    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);
 
     printk("CPU:    %d\n", cpu);
     _show_registers(regs, crs, CTXT_hypervisor, NULL);
diff --git a/xen/include/asm-x86/system.h b/xen/include/asm-x86/system.h
index b0876d6..d8dc6f2 100644
--- a/xen/include/asm-x86/system.h
+++ b/xen/include/asm-x86/system.h
@@ -4,7 +4,7 @@
 #include <xen/lib.h>
 #include <asm/bitops.h>
 
-#define read_segment_register(name)                             \
+#define read_segment_register(vcpu, regs, name)                 \
 ({  u16 __sel;                                                  \
     asm volatile ( "movw %%" STR(name) ",%0" :
"=r" (__sel) );  \
     __sel;                                                      \
diff --git a/xen/include/asm-x86/traps.h b/xen/include/asm-x86/traps.h
index 82cbcee..202e3be 100644
--- a/xen/include/asm-x86/traps.h
+++ b/xen/include/asm-x86/traps.h
@@ -49,4 +49,5 @@ extern int guest_has_trap_callback(struct domain *d, uint16_t
vcpuid,
 extern int send_guest_trap(struct domain *d, uint16_t vcpuid,
 				unsigned int trap_nr);
 
+int emulate_privileged_op(struct cpu_user_regs *regs);
 #endif /* ASM_TRAP_H */
-- 
1.7.2.3

>>> On 16.03.13 at 01:28, Mukesh Rathor
<mukesh.rathor@oracle.com> wrote:
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -1823,7 +1823,7 @@ static inline uint64_t guest_misc_enable(uint64_t
val)
>      }                                                                     
\
>      (eip) += sizeof(_x); _x; })
>  
> -#define read_sreg(regs, sr) read_segment_register(sr)
> +#define read_sreg(vcpu, regs, sr) read_segment_register(vcpu, regs, sr)
I don''t see why you would need to retain this macro now that all
arguments get simply passed through.

On Fri, Mar 15, 2013 at 05:28:25PM -0700, Mukesh Rathor
wrote:
> In this patch, read_segment_register macro is changed to take vcpu and
> regs parameters so it can check if it''s PVH guest (change in
upcoming
> patches). No functionality change. Also, make emulate_privileged_op()
> public for later while changing this file.
> 
> Changes in V2:  None
> 
> Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
Pretty simple.

Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

> ---
>  xen/arch/x86/domain.c        |    8 ++++----
>  xen/arch/x86/traps.c         |   28 ++++++++++++++--------------
>  xen/arch/x86/x86_64/traps.c  |   16 ++++++++--------
>  xen/include/asm-x86/system.h |    2 +-
>  xen/include/asm-x86/traps.h  |    1 +
>  5 files changed, 28 insertions(+), 27 deletions(-)
> 
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index ea1381c..e9549e0 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1340,10 +1340,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(ds);
> -    regs->es = read_segment_register(es);
> -    regs->fs = read_segment_register(fs);
> -    regs->gs = read_segment_register(gs);
> +    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);
>  
>      if ( regs->ds )
>          dirty_segment_mask |= DIRTY_DS;
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index d36eddd..ab3e814 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -1823,7 +1823,7 @@ static inline uint64_t guest_misc_enable(uint64_t
val)
>      }                                                                     
\
>      (eip) += sizeof(_x); _x; })
>  
> -#define read_sreg(regs, sr) read_segment_register(sr)
> +#define read_sreg(vcpu, regs, sr) read_segment_register(vcpu, regs, sr)
>  
>  static int is_cpufreq_controller(struct domain *d)
>  {
> @@ -1833,7 +1833,7 @@ static int is_cpufreq_controller(struct domain *d)
>  
>  #include "x86_64/mmconfig.h"
>  
> -static int emulate_privileged_op(struct cpu_user_regs *regs)
> +int emulate_privileged_op(struct cpu_user_regs *regs)
>  {
>      struct vcpu *v = current;
>      unsigned long *reg, eip = regs->eip;
> @@ -1869,7 +1869,7 @@ static int emulate_privileged_op(struct cpu_user_regs
*regs)
>          goto fail;
>  
>      /* emulating only opcodes not allowing SS to be default */
> -    data_sel = read_sreg(regs, ds);
> +    data_sel = read_sreg(v, regs, ds);
>  
>      /* Legacy prefixes. */
>      for ( i = 0; i < 8; i++, rex == opcode || (rex = 0) )
> @@ -1887,17 +1887,17 @@ static int emulate_privileged_op(struct
cpu_user_regs *regs)
>              data_sel = regs->cs;
>              continue;
>          case 0x3e: /* DS override */
> -            data_sel = read_sreg(regs, ds);
> +            data_sel = read_sreg(v, regs, ds);
>              continue;
>          case 0x26: /* ES override */
> -            data_sel = read_sreg(regs, es);
> +            data_sel = read_sreg(v, regs, es);
>              continue;
>          case 0x64: /* FS override */
> -            data_sel = read_sreg(regs, fs);
> +            data_sel = read_sreg(v, regs, fs);
>              lm_ovr = lm_seg_fs;
>              continue;
>          case 0x65: /* GS override */
> -            data_sel = read_sreg(regs, gs);
> +            data_sel = read_sreg(v, regs, gs);
>              lm_ovr = lm_seg_gs;
>              continue;
>          case 0x36: /* SS override */
> @@ -1944,7 +1944,7 @@ static int emulate_privileged_op(struct cpu_user_regs
*regs)
>  
>          if ( !(opcode & 2) )
>          {
> -            data_sel = read_sreg(regs, es);
> +            data_sel = read_sreg(v, regs, es);
>              lm_ovr = lm_seg_none;
>          }
>  
> @@ -2677,22 +2677,22 @@ static void emulate_gate_op(struct cpu_user_regs
*regs)
>              ASSERT(opnd_sel);
>              continue;
>          case 0x3e: /* DS override */
> -            opnd_sel = read_sreg(regs, ds);
> +            opnd_sel = read_sreg(v, regs, ds);
>              if ( !opnd_sel )
>                  opnd_sel = dpl;
>              continue;
>          case 0x26: /* ES override */
> -            opnd_sel = read_sreg(regs, es);
> +            opnd_sel = read_sreg(v, regs, es);
>              if ( !opnd_sel )
>                  opnd_sel = dpl;
>              continue;
>          case 0x64: /* FS override */
> -            opnd_sel = read_sreg(regs, fs);
> +            opnd_sel = read_sreg(v, regs, fs);
>              if ( !opnd_sel )
>                  opnd_sel = dpl;
>              continue;
>          case 0x65: /* GS override */
> -            opnd_sel = read_sreg(regs, gs);
> +            opnd_sel = read_sreg(v, regs, gs);
>              if ( !opnd_sel )
>                  opnd_sel = dpl;
>              continue;
> @@ -2745,7 +2745,7 @@ static void emulate_gate_op(struct cpu_user_regs
*regs)
>                              switch ( modrm & 7 )
>                              {
>                              default:
> -                                opnd_sel = read_sreg(regs, ds);
> +                                opnd_sel = read_sreg(v, regs, ds);
>                                  break;
>                              case 4: case 5:
>                                  opnd_sel = regs->ss;
> @@ -2773,7 +2773,7 @@ static void emulate_gate_op(struct cpu_user_regs
*regs)
>                              break;
>                          }
>                          if ( !opnd_sel )
> -                            opnd_sel = read_sreg(regs, ds);
> +                            opnd_sel = read_sreg(v, regs, 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 eec919a..d2f7209 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(ds);
> -        fault_regs.es = read_segment_register(es);
> -        fault_regs.fs = read_segment_register(fs);
> -        fault_regs.gs = read_segment_register(gs);
> +        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);
>      }
>  
>      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(ds);
> -    regs->es = read_segment_register(es);
> -    regs->fs = read_segment_register(fs);
> -    regs->gs = read_segment_register(gs);
> +    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);
>  
>      printk("CPU:    %d\n", cpu);
>      _show_registers(regs, crs, CTXT_hypervisor, NULL);
> diff --git a/xen/include/asm-x86/system.h b/xen/include/asm-x86/system.h
> index b0876d6..d8dc6f2 100644
> --- a/xen/include/asm-x86/system.h
> +++ b/xen/include/asm-x86/system.h
> @@ -4,7 +4,7 @@
>  #include <xen/lib.h>
>  #include <asm/bitops.h>
>  
> -#define read_segment_register(name)                             \
> +#define read_segment_register(vcpu, regs, name)                 \
>  ({  u16 __sel;                                                  \
>      asm volatile ( "movw %%" STR(name) ",%0" :
"=r" (__sel) );  \
>      __sel;                                                      \
> diff --git a/xen/include/asm-x86/traps.h b/xen/include/asm-x86/traps.h
> index 82cbcee..202e3be 100644
> --- a/xen/include/asm-x86/traps.h
> +++ b/xen/include/asm-x86/traps.h
> @@ -49,4 +49,5 @@ extern int guest_has_trap_callback(struct domain *d,
uint16_t vcpuid,
>  extern int send_guest_trap(struct domain *d, uint16_t vcpuid,
>  				unsigned int trap_nr);
>  
> +int emulate_privileged_op(struct cpu_user_regs *regs);
>  #endif /* ASM_TRAP_H */
> -- 
> 1.7.2.3
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>

On Mon, 18 Mar 2013 11:44:29 +0000
"Jan Beulich" <JBeulich@suse.com> wrote:
> >>> On 16.03.13 at 01:28, Mukesh Rathor
<mukesh.rathor@oracle.com>
> >>> wrote:
> > --- a/xen/arch/x86/traps.c
> > +++ b/xen/arch/x86/traps.c
> > @@ -1823,7 +1823,7 @@ static inline uint64_t
> > guest_misc_enable(uint64_t
> > val) }
> > \ (eip) += sizeof(_x); _x; }) 
> > -#define read_sreg(regs, sr) read_segment_register(sr)
> > +#define read_sreg(vcpu, regs, sr) read_segment_register(vcpu,
> > regs, sr)
> 
> I don''t see why you would need to retain this macro now that all
> arguments get simply passed through.
I figured they didn''t use read_segment_register() in the first place,
so may be will need to be changed again in future. I''ll just get rid of
it then and use read_segment_register().

thanks
m