Re: [PATCH 2/2] add sysenter/syscall emulation for 32bit compat mode

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

 



Andre Przywara wrote:


@@ -1985,10 +1992,114 @@ twobyte_insn:
             goto cannot_emulate;
         }
         break;
+    case 0x05: { /* syscall */
+        unsigned long cr0 = ctxt->vcpu->arch.cr0;
+        struct kvm_segment cs, ss;
+
+        memset(&cs, 0, sizeof(struct kvm_segment));
+        memset(&ss, 0, sizeof(struct kvm_segment));
+
+        /* inject #UD if
+         * 1. we are in real mode
+         * 2. protected mode is not enabled
+         * 3. LOCK prefix is used
+         */
+        if ((ctxt->mode == X86EMUL_MODE_REAL)
+            || (!(cr0 & X86_CR0_PE))
+            || (c->lock_prefix)) {
+            /* we don't need to inject #UD here, because
+             * when emulate_instruction() returns something else
+             * than EMULATE_DONE, then svm.c:ud_interception()
+             * will do that for us.
+             */
+            goto cannot_emulate;

I prefer explicit injection, relying on the caller is tricky and may change.
I don't agree. If this function cannot emulate an instruction, it returns -1 and lets the upper levels handle this. If we cannot rely on this, what else can we rely on? I could remove the comment in case this is confusing. The same functionality (return -1 to inject UD into the guest) is used in other places in this same file.

We return -1, but that doesn't mean we inject #UD. For some cases (page table emulation), we unshadow the page and retry (letting the guest execute natively).

We only inject #UD from that specific call site. If we somehow emulated from another call site, we'd get different behaviour.



+            cs.limit = 0xffffffff;
+            ss.base = 0;
+            ss.limit = 0xffffffff;

Once is enough.
You are right about the ss.base assignment. But the limit goes from five f's to eight f's. On a first glance this should not matter (as the granularity bit is set), but exactly here are differences between VMX and SVM, so I'd like to leave it this way.

I misread, I thought you were setting ss.limit twice. Certainly the code is correct as is and should not be modified. Sorry about the confusion.

--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

--
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