Re: [PATCH] KVM: x86: Support for disabling quirks

[Nadav Amit <nadav.amit@gmail.com>]

Thanks. If you want a test-case you can apply/try the following on top of
the previous kvm-unit-tests patch-set I sent 
( http://www.spinics.net/lists/kvm/msg115525.html ).

Regards,
Nadav

-- >8 --

From: Nadav Amit <namit@cs.technion.ac.il>
Subject: [PATCH kvm-unit-tests] x86: Test LINT0 is disabled after reset

Requires x2APIC in order to easily save LINT0 during 16-bit code.  For the test
to pass a fix for qemu is required.

Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
---
 x86/init.c        | 13 ++++++++++++-
 x86/unittests.cfg |  1 +
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/x86/init.c b/x86/init.c
index 2d7ea99..23079ad 100644
--- a/x86/init.c
+++ b/x86/init.c
@@ -70,6 +70,7 @@ static struct expected_state expected[] = {
 #define cr2 (*(volatile int*)0x2010)
 #define sysenter_eip (*(volatile int*)0x2014)
 #define boot_apic_id (*(volatile int *)0x2018)
+#define lvt0 (*(volatile int *)0x201c)
 
 static void set_test_regs(void)
 {
@@ -94,7 +95,10 @@ static bool check_test_regs(bool init)
 		printf("wrong sysenter_eip msr: %x\n", sysenter_eip);
 		error = true;
 	}
-
+	if (lvt0 != 0x10000) {
+		printf("wrong lvt0 value: %x\n", lvt0);
+		error = true;
+	}
 	return error;
 }
 
@@ -240,6 +244,13 @@ asm (
 	"cpuid\n"
 	"shrl $24, %ebx\n"
 	"mov %ebx, %cs:0x2018\n"	// apic_id
+	"mov $0x1b, %ecx\n"		// IA32_APIC_BASE
+	"rdmsr\n"
+	"or $0x400, %eax\n"
+	"wrmsr\n"			// Enabling x2apic
+	"mov $0x835, %ecx\n"
+	"rdmsr\n"
+        "mov %eax, %cs:0x201c\n"        // lvt0
 	"mov $0x0f, %al\n"		// rtc_out(0x0f, 0x00);
 	"out %al, $0x70\n"
 	"mov $0x00, %al\n"
diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 2d25801..0d1a42b 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -86,6 +86,7 @@ arch = x86_64
 [init]
 file = init.flat
 smp = 2
+extra_params = -cpu qemu64,+x2apic
 
 [msr]
 file = msr.flat
-- 
1.9.1




Paolo Bonzini <pbonzini@redhat.com> wrote:

> 
> 
> On 13/04/2015 00:53, Nadav Amit wrote:
>> Introducing KVM_CAP_DISABLE_QUIRKS for disabling x86 quirks that were previous
>> created in order to overcome QEMU issues. Those issue were mostly result of
>> invalid VM BIOS.  Currently there are two quirks that can be disabled:
>> 
>> 1. KVM_QUIRK_LINT0_REENABLED - LINT0 was enabled after boot
>> 2. KVM_QUIRK_CD_NW_CLEARED - CD and NW are cleared after boot
>> 
>> These two issues are already resolved in recent releases of QEMU, and would
>> therefore be disabled by QEMU.
>> 
>> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
>> ---
>> Documentation/virtual/kvm/api.txt |  3 ++-
>> arch/x86/include/asm/kvm_host.h   |  2 ++
>> arch/x86/include/uapi/asm/kvm.h   |  3 +++
>> arch/x86/kvm/lapic.c              |  5 +++--
>> arch/x86/kvm/svm.c                |  3 ++-
>> arch/x86/kvm/x86.c                | 29 +++++++++++++++++++++++++++++
>> include/uapi/linux/kvm.h          |  1 +
>> 7 files changed, 42 insertions(+), 4 deletions(-)
>> 
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> index bc9f6fe..3931221 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -959,7 +959,8 @@ documentation when it pops into existence).
>> 4.37 KVM_ENABLE_CAP
>> 
>> Capability: KVM_CAP_ENABLE_CAP, KVM_CAP_ENABLE_CAP_VM
>> -Architectures: ppc, s390
>> +Architectures: x86 (only KVM_CAP_ENABLE_CAP_VM),
>> +	       mips (only KVM_CAP_ENABLE_CAP), ppc, s390
>> Type: vcpu ioctl, vm ioctl (with KVM_CAP_ENABLE_CAP_VM)
>> Parameters: struct kvm_enable_cap (in)
>> Returns: 0 on success; -1 on error
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index dea2e7e..f80ad59 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -635,6 +635,8 @@ struct kvm_arch {
>> 	#endif
>> 
>> 	bool boot_vcpu_runs_old_kvmclock;
>> +
>> +	u64 disabled_quirks;
>> };
>> 
>> struct kvm_vm_stat {
>> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
>> index d7dcef5..2fec75e 100644
>> --- a/arch/x86/include/uapi/asm/kvm.h
>> +++ b/arch/x86/include/uapi/asm/kvm.h
>> @@ -345,4 +345,7 @@ struct kvm_xcrs {
>> struct kvm_sync_regs {
>> };
>> 
>> +#define KVM_QUIRK_LINT0_REENABLED	(1 << 0)
>> +#define KVM_QUIRK_CD_NW_CLEARED		(1 << 1)
>> +
>> #endif /* _ASM_X86_KVM_H */
>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>> index 4a6e58a..fe2d89e 100644
>> --- a/arch/x86/kvm/lapic.c
>> +++ b/arch/x86/kvm/lapic.c
>> @@ -1577,8 +1577,9 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu)
>> 	for (i = 0; i < APIC_LVT_NUM; i++)
>> 		apic_set_reg(apic, APIC_LVTT + 0x10 * i, APIC_LVT_MASKED);
>> 	apic->lapic_timer.timer_mode = 0;
>> -	apic_set_reg(apic, APIC_LVT0,
>> -		     SET_APIC_DELIVERY_MODE(0, APIC_MODE_EXTINT));
>> +	if (!(vcpu->kvm->arch.disabled_quirks & KVM_QUIRK_LINT0_REENABLED))
>> +		apic_set_reg(apic, APIC_LVT0,
>> +			     SET_APIC_DELIVERY_MODE(0, APIC_MODE_EXTINT));
>> 
>> 	apic_set_reg(apic, APIC_DFR, 0xffffffffU);
>> 	apic_set_spiv(apic, 0xff);
>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>> index ce741b8..46299da 100644
>> --- a/arch/x86/kvm/svm.c
>> +++ b/arch/x86/kvm/svm.c
>> @@ -1575,7 +1575,8 @@ static void svm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
>> 	 * does not do it - this results in some delay at
>> 	 * reboot
>> 	 */
>> -	cr0 &= ~(X86_CR0_CD | X86_CR0_NW);
>> +	if (!(vcpu->kvm->arch.disabled_quirks & KVM_QUIRK_CD_NW_CLEARED))
>> +		cr0 &= ~(X86_CR0_CD | X86_CR0_NW);
>> 	svm->vmcb->save.cr0 = cr0;
>> 	mark_dirty(svm->vmcb, VMCB_CR);
>> 	update_cr0_intercept(svm);
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index b8cb1d0..c3859a6 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -2778,6 +2778,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>> 	case KVM_CAP_HYPERV_TIME:
>> 	case KVM_CAP_IOAPIC_POLARITY_IGNORED:
>> 	case KVM_CAP_TSC_DEADLINE_TIMER:
>> +	case KVM_CAP_ENABLE_CAP_VM:
>> #ifdef CONFIG_KVM_DEVICE_ASSIGNMENT
>> 	case KVM_CAP_ASSIGN_DEV_IRQ:
>> 	case KVM_CAP_PCI_2_3:
>> @@ -3825,6 +3826,26 @@ int kvm_vm_ioctl_irq_line(struct kvm *kvm, struct kvm_irq_level *irq_event,
>> 	return 0;
>> }
>> 
>> +static int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
>> +				   struct kvm_enable_cap *cap)
>> +{
>> +	int r;
>> +
>> +	if (cap->flags)
>> +		return -EINVAL;
>> +
>> +	switch (cap->cap) {
>> +	case KVM_CAP_DISABLE_QUIRKS:
>> +		kvm->arch.disabled_quirks = cap->args[0];
>> +		r = 0;
>> +		break;
>> +	default:
>> +		r = -EINVAL;
>> +		break;
>> +	}
>> +	return r;
>> +}
>> +
>> long kvm_arch_vm_ioctl(struct file *filp,
>> 		       unsigned int ioctl, unsigned long arg)
>> {
>> @@ -4077,7 +4098,15 @@ long kvm_arch_vm_ioctl(struct file *filp,
>> 		r = 0;
>> 		break;
>> 	}
>> +	case KVM_ENABLE_CAP: {
>> +		struct kvm_enable_cap cap;
>> 
>> +		r = -EFAULT;
>> +		if (copy_from_user(&cap, argp, sizeof(cap)))
>> +			goto out;
>> +		r = kvm_vm_ioctl_enable_cap(kvm, &cap);
>> +		break;
>> +	}
>> 	default:
>> 		r = kvm_vm_ioctl_assigned_device(kvm, ioctl, arg);
>> 	}
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index f574d7b..01c0a8e 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -813,6 +813,7 @@ struct kvm_ppc_smmu_info {
>> #define KVM_CAP_MIPS_MSA 112
>> #define KVM_CAP_S390_INJECT_IRQ 113
>> #define KVM_CAP_S390_IRQ_STATE 114
>> +#define KVM_CAP_DISABLE_QUIRKS 115
>> 
>> #ifdef KVM_CAP_IRQ_ROUTING
> 
> Applied (locally) for 4.2.
> 
> Paolo
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html