Re: [PATCH 1/2] KVM: x86: Add emulation support for #GP triggered by VM instructions

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

 





On 1/12/21 11:56 AM, Sean Christopherson wrote:
On Tue, Jan 12, 2021, Andy Lutomirski wrote:

On Jan 12, 2021, at 7:46 AM, Bandan Das <bsd@xxxxxxxxxx> wrote:

Andy Lutomirski <luto@xxxxxxxxxxxxxx> writes:
...
#endif diff --git a/arch/x86/kvm/mmu/mmu.c
b/arch/x86/kvm/mmu/mmu.c index 6d16481aa29d..c5c4aaf01a1a 100644
--- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@
-50,6 +50,7 @@ #include <asm/io.h> #include <asm/vmx.h> #include
<asm/kvm_page_track.h> +#include <asm/e820/api.h> #include
"trace.h"

extern bool itlb_multihit_kvm_mitigation; @@ -5675,6 +5676,12 @@
void kvm_mmu_slot_set_dirty(struct kvm *kvm, }
EXPORT_SYMBOL_GPL(kvm_mmu_slot_set_dirty);

+bool kvm_is_host_reserved_region(u64 gpa) +{ + return
e820__mbapped_raw_any(gpa-1, gpa+1, E820_TYPE_RESERVED); +}
While _e820__mapped_any()'s doc says '..  checks if any part of
the range <start,end> is mapped ..' it seems to me that the real
check is [start, end) so we should use 'gpa' instead of 'gpa-1',
no?
Why do you need to check GPA at all?

To reduce the scope of the workaround.

The errata only happens when you use one of SVM instructions in the
guest with EAX that happens to be inside one of the host reserved
memory regions (for example SMM).

This code reduces the scope of the workaround at the cost of
increasing the complexity of the workaround and adding a nonsensical
coupling between KVM and host details and adding an export that really
doesn’t deserve to be exported.

Is there an actual concrete benefit to this check?

Besides reducing the scope, my intention for the check was that we should
know if such exceptions occur for any other undiscovered reasons with other
memory types rather than hiding them under this workaround.

Ask AMD?

There are several checking before VMRUN launch. The function, e820__mapped_raw_any(), was definitely one of the easies way to figure out the problematic regions we had.


I would also believe that someone somewhere has a firmware that simply omits
the problematic region instead of listing it as reserved.

I agree with Andy, odds are very good that attempting to be precise will lead to
pain due to false negatives.

And, KVM's SVM instruction emulation needs to be be rock solid regardless of
this behavior since KVM unconditionally intercepts the instruction, i.e. there's
basically zero risk to KVM.


Are you saying that the instruction decode before kvm_is_host_reserved_region() already guarantee the instructions #GP hit are SVM execution instructions (see below)? If so, I think this argument is fair.

+	switch (ctxt->modrm) {
+	case 0xd8: /* VMRUN */
+	case 0xda: /* VMLOAD */
+	case 0xdb: /* VMSAVE */

Bandan: What is your thoughts about removing kvm_is_host_reserved_region()?




[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