Messages in this thread | | | Subject | Re: [PATCH 1/2] KVM: x86: Add emulation support for #GP triggered by VM instructions | From | Wei Huang <> | Date | Tue, 12 Jan 2021 22:55:39 -0600 |
| |
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@redhat.com> wrote: >>> >>> Andy Lutomirski <luto@amacapital.net> 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()?
| |