Re: [PATCH 08/23] KVM: x86 emulator: cleanup some direct calls into kvm to use existing callbacks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/28/2010 12:33 PM, Gleb Natapov wrote:
On Wed, Apr 28, 2010 at 11:59:54AM +0300, Avi Kivity wrote:
On 04/27/2010 03:15 PM, Gleb Natapov wrote:
Use callbacks from x86_emulate_ops to access segments instead of calling
into kvm directly.



-static unsigned long seg_base(struct x86_emulate_ctxt *ctxt, int seg)
+static unsigned long seg_base(struct x86_emulate_ctxt *ctxt,
+			      struct x86_emulate_ops *ops, int seg)
  {
-	if (ctxt->mode == X86EMUL_MODE_PROT64&&   seg<   VCPU_SREG_FS)
-		return 0;
+	unsigned long base;

-	return kvm_x86_ops->get_segment_base(ctxt->vcpu, seg);
get_segment_base() is only one vmread on intel, but you replace it
with reading the entire segment.

Didn't what to have separate x86_emulate_ops callback for reading
segment base. If this is serious performance concern it can be added

It will definitely hurt nested vmx. For ordinary vmx perhaps not so much, but better not to regress.

+	if (ctxt->mode == X86EMUL_MODE_PROT64) {
+		u64 val;
+		switch (seg) {
+		case VCPU_SREG_FS:
+			ops->get_msr(ctxt->vcpu, MSR_FS_BASE,&val);
+			break;
+		case VCPU_SREG_GS:
+			ops->get_msr(ctxt->vcpu, MSR_GS_BASE,&val);
+			break;
+		default:
+			val = 0;
+			break;
+		}
Why this ugliness?  get_cached_descriptor() should do this.

get_cached_descriptor() returns struct desc_struct which is 32 bit only.
There is not 64bit segment descriptors.

Ah. Well either extend desc_struct, or return something which works everywhere.

  static unsigned long seg_override_base(struct x86_emulate_ctxt *ctxt,
+				       struct x86_emulate_ops *ops,
  				       struct decode_cache *c)
  {
  	if (!c->has_seg_override)
  		return 0;

-	return seg_base(ctxt, c->seg_override);
+	return seg_base(ctxt, ops, c->seg_override);
  }
Sticking ops into ctxt would reduce the size of these patches.

But it will introduce intermediate huge patch. But I agree that this
should be done eventually. Planned to do it somewhere at the end. Near
the patch that change x86_emulate_ops callbacks to get ctxt instead of
vcpu as a parameter.

Would have been better up front IMO, but don't destroy your patchset just for this.

--
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux