subject:"Re\: \[PATCH v2 05\/30\] KVM\: Provide callback to get\/set control registers in emulator ops."

Re: [PATCH v2 05/30] KVM: Provide callback to get/set control registers in emulator ops.

2010-03-15 Thread Andre Przywara


Gleb Natapov wrote:

Use this callback instead of directly call kvm function. Also rename
realmode_(set|get)_cr to emulator_(set|get)_cr since function has nothing
to do with real mode.
Do you mind removing the static before emulator_{set,get}_cr and marking 
it EXPORT_SYMBOL? Then one could use it in vmx.c (and soon in svm.c ;-) 
while handling MOV-CR intercepts. Currently most of the code is actually 
duplicated.


Also, shouldn't mk_cr_64() not be called mask_cr_64() for better 
readability?


Regards,
Andre.


Signed-off-by: Gleb Natapov g...@redhat.com
---
 arch/x86/include/asm/kvm_emulate.h |3 +-
 arch/x86/include/asm/kvm_host.h|2 -
 arch/x86/kvm/emulate.c |7 +-
 arch/x86/kvm/x86.c |  114 ++--
 4 files changed, 63 insertions(+), 63 deletions(-)

diff --git a/arch/x86/include/asm/kvm_emulate.h 
b/arch/x86/include/asm/kvm_emulate.h
index 2666d7a..0c5caa4 100644
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -108,7 +108,8 @@ struct x86_emulate_ops {
const void *new,
unsigned int bytes,
struct kvm_vcpu *vcpu);
-
+   ulong (*get_cr)(int cr, struct kvm_vcpu *vcpu);
+   void (*set_cr)(int cr, ulong val, struct kvm_vcpu *vcpu);
 };
 
 /* Type, address-of, and value of an instruction's operand. */

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 3b178d8..e8e108a 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -585,8 +585,6 @@ void realmode_lidt(struct kvm_vcpu *vcpu, u16 size, 
unsigned long address);
 void realmode_lmsw(struct kvm_vcpu *vcpu, unsigned long msw,
   unsigned long *rflags);
 
-unsigned long realmode_get_cr(struct kvm_vcpu *vcpu, int cr);

-void realmode_set_cr(struct kvm_vcpu *vcpu, int cr, unsigned long value);
 void kvm_enable_efer_bits(u64);
 int kvm_get_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *data);
 int kvm_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data);
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 91450b5..5b060e4 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2483,7 +2483,7 @@ twobyte_insn:
break;
case 4: /* smsw */
c-dst.bytes = 2;
-   c-dst.val = realmode_get_cr(ctxt-vcpu, 0);
+   c-dst.val = ops-get_cr(0, ctxt-vcpu);
break;
case 6: /* lmsw */
realmode_lmsw(ctxt-vcpu, (u16)c-src.val,
@@ -2519,8 +2519,7 @@ twobyte_insn:
case 0x20: /* mov cr, reg */
if (c-modrm_mod != 3)
goto cannot_emulate;
-   c-regs[c-modrm_rm] =
-   realmode_get_cr(ctxt-vcpu, c-modrm_reg);
+   c-regs[c-modrm_rm] = ops-get_cr(c-modrm_reg, ctxt-vcpu);
c-dst.type = OP_NONE;   /* no writeback */
break;
case 0x21: /* mov from dr to reg */
@@ -2534,7 +2533,7 @@ twobyte_insn:
case 0x22: /* mov reg, cr */
if (c-modrm_mod != 3)
goto cannot_emulate;
-   realmode_set_cr(ctxt-vcpu, c-modrm_reg, c-modrm_val);
+   ops-set_cr(c-modrm_reg, c-modrm_val, ctxt-vcpu);
c-dst.type = OP_NONE;
break;
case 0x23: /* mov from reg to dr */
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index a1e671a..bf714df 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3370,12 +3370,70 @@ void kvm_report_emulation_failure(struct kvm_vcpu 
*vcpu, const char *context)
 }
 EXPORT_SYMBOL_GPL(kvm_report_emulation_failure);
 
+static u64 mk_cr_64(u64 curr_cr, u32 new_val)

+{
+   return (curr_cr  ~((1ULL  32) - 1)) | new_val;
+}
+
+static unsigned long emulator_get_cr(int cr, struct kvm_vcpu *vcpu)
+{
+   unsigned long value;
+
+   switch (cr) {
+   case 0:
+   value = kvm_read_cr0(vcpu);
+   break;
+   case 2:
+   value = vcpu-arch.cr2;
+   break;
+   case 3:
+   value = vcpu-arch.cr3;
+   break;
+   case 4:
+   value = kvm_read_cr4(vcpu);
+   break;
+   case 8:
+   value = kvm_get_cr8(vcpu);
+   break;
+   default:
+   vcpu_printf(vcpu, %s: unexpected cr %u\n, __func__, cr);
+   return 0;
+   }
+
+   return value;
+}
+
+static void emulator_set_cr(int cr, unsigned long val, struct kvm_vcpu *vcpu)
+{
+   switch (cr) {
+   case 0:
+   kvm_set_cr0(vcpu, mk_cr_64(kvm_read_cr0(vcpu), val));
+   break;
+   case 2:
+   vcpu-arch.cr2 = val;
+   break;
+   case 3:
+   kvm_set_cr3(vcpu, val);
+   break;

Re: [PATCH v2 05/30] KVM: Provide callback to get/set control registers in emulator ops.

2010-03-15 Thread Gleb Natapov

On Mon, Mar 15, 2010 at 02:06:48PM +0100, Andre Przywara wrote:
 Gleb Natapov wrote:
 Use this callback instead of directly call kvm function. Also rename
 realmode_(set|get)_cr to emulator_(set|get)_cr since function has nothing
 to do with real mode.
 Do you mind removing the static before emulator_{set,get}_cr and
I don't, but this is not the goal of this patch series.

 marking it EXPORT_SYMBOL? Then one could use it in vmx.c (and soon
 in svm.c ;-) while handling MOV-CR intercepts. Currently most of the
 code is actually duplicated.
 
 Also, shouldn't mk_cr_64() not be called mask_cr_64() for better
 readability?
This is how it is called now, the patch only moves it. But this code
will be reworked by later patches anyway since functions called from
emulator should not inject exceptions behind emulator's back.

 
 Regards,
 Andre.
 
 Signed-off-by: Gleb Natapov g...@redhat.com
 ---
  arch/x86/include/asm/kvm_emulate.h |3 +-
  arch/x86/include/asm/kvm_host.h|2 -
  arch/x86/kvm/emulate.c |7 +-
  arch/x86/kvm/x86.c |  114 
  ++--
  4 files changed, 63 insertions(+), 63 deletions(-)
 
 diff --git a/arch/x86/include/asm/kvm_emulate.h 
 b/arch/x86/include/asm/kvm_emulate.h
 index 2666d7a..0c5caa4 100644
 --- a/arch/x86/include/asm/kvm_emulate.h
 +++ b/arch/x86/include/asm/kvm_emulate.h
 @@ -108,7 +108,8 @@ struct x86_emulate_ops {
  const void *new,
  unsigned int bytes,
  struct kvm_vcpu *vcpu);
 -
 +ulong (*get_cr)(int cr, struct kvm_vcpu *vcpu);
 +void (*set_cr)(int cr, ulong val, struct kvm_vcpu *vcpu);
  };
  /* Type, address-of, and value of an instruction's operand. */
 diff --git a/arch/x86/include/asm/kvm_host.h 
 b/arch/x86/include/asm/kvm_host.h
 index 3b178d8..e8e108a 100644
 --- a/arch/x86/include/asm/kvm_host.h
 +++ b/arch/x86/include/asm/kvm_host.h
 @@ -585,8 +585,6 @@ void realmode_lidt(struct kvm_vcpu *vcpu, u16 size, 
 unsigned long address);
  void realmode_lmsw(struct kvm_vcpu *vcpu, unsigned long msw,
 unsigned long *rflags);
 -unsigned long realmode_get_cr(struct kvm_vcpu *vcpu, int cr);
 -void realmode_set_cr(struct kvm_vcpu *vcpu, int cr, unsigned long value);
  void kvm_enable_efer_bits(u64);
  int kvm_get_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *data);
  int kvm_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data);
 diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
 index 91450b5..5b060e4 100644
 --- a/arch/x86/kvm/emulate.c
 +++ b/arch/x86/kvm/emulate.c
 @@ -2483,7 +2483,7 @@ twobyte_insn:
  break;
  case 4: /* smsw */
  c-dst.bytes = 2;
 -c-dst.val = realmode_get_cr(ctxt-vcpu, 0);
 +c-dst.val = ops-get_cr(0, ctxt-vcpu);
  break;
  case 6: /* lmsw */
  realmode_lmsw(ctxt-vcpu, (u16)c-src.val,
 @@ -2519,8 +2519,7 @@ twobyte_insn:
  case 0x20: /* mov cr, reg */
  if (c-modrm_mod != 3)
  goto cannot_emulate;
 -c-regs[c-modrm_rm] =
 -realmode_get_cr(ctxt-vcpu, c-modrm_reg);
 +c-regs[c-modrm_rm] = ops-get_cr(c-modrm_reg, ctxt-vcpu);
  c-dst.type = OP_NONE;  /* no writeback */
  break;
  case 0x21: /* mov from dr to reg */
 @@ -2534,7 +2533,7 @@ twobyte_insn:
  case 0x22: /* mov reg, cr */
  if (c-modrm_mod != 3)
  goto cannot_emulate;
 -realmode_set_cr(ctxt-vcpu, c-modrm_reg, c-modrm_val);
 +ops-set_cr(c-modrm_reg, c-modrm_val, ctxt-vcpu);
  c-dst.type = OP_NONE;
  break;
  case 0x23: /* mov from reg to dr */
 diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
 index a1e671a..bf714df 100644
 --- a/arch/x86/kvm/x86.c
 +++ b/arch/x86/kvm/x86.c
 @@ -3370,12 +3370,70 @@ void kvm_report_emulation_failure(struct kvm_vcpu 
 *vcpu, const char *context)
  }
  EXPORT_SYMBOL_GPL(kvm_report_emulation_failure);
 +static u64 mk_cr_64(u64 curr_cr, u32 new_val)
 +{
 +return (curr_cr  ~((1ULL  32) - 1)) | new_val;
 +}
 +
 +static unsigned long emulator_get_cr(int cr, struct kvm_vcpu *vcpu)
 +{
 +unsigned long value;
 +
 +switch (cr) {
 +case 0:
 +value = kvm_read_cr0(vcpu);
 +break;
 +case 2:
 +value = vcpu-arch.cr2;
 +break;
 +case 3:
 +value = vcpu-arch.cr3;
 +break;
 +case 4:
 +value = kvm_read_cr4(vcpu);
 +break;
 +case 8:
 +value = kvm_get_cr8(vcpu);
 +break;
 +default:
 +vcpu_printf(vcpu, %s: unexpected cr %u\n, __func__, cr);
 +return 0;
 +}
 +
 +return value;
 +}
 +
 +static void emulator_set_cr(int cr, unsigned long val, struct kvm_vcpu 
 *vcpu)
 +{
 +switch

Re: [PATCH v2 05/30] KVM: Provide callback to get/set control registers in emulator ops.

2010-03-15 Thread Avi Kivity


On 03/15/2010 03:06 PM, Andre Przywara wrote:

Gleb Natapov wrote:

Use this callback instead of directly call kvm function. Also rename
realmode_(set|get)_cr to emulator_(set|get)_cr since function has 
nothing

to do with real mode.
Do you mind removing the static before emulator_{set,get}_cr and 
marking it EXPORT_SYMBOL? Then one could use it in vmx.c (and soon in 
svm.c ;-) while handling MOV-CR intercepts. Currently most of the code 
is actually duplicated.


Just do that in your patch, that's standard practice.

--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 05/30] KVM: Provide callback to get/set control registers in emulator ops.

Re: [PATCH v2 05/30] KVM: Provide callback to get/set control registers in emulator ops.

Re: [PATCH v2 05/30] KVM: Provide callback to get/set control registers in emulator ops.

3 matches

Site Navigation

Mail list logo

Footer information