On Fri, Feb 15, 2019 at 05:24:12PM +0000, Singh, Brijesh wrote:
> Errata#1096:
>
> On a nested data page fault when CR.SMAP=1 and the guest data read
> generates a SMAP violation, GuestInstrBytes field of the VMCB on a
> VMEXIT will incorrectly return 0h instead the correct guest
> instruction bytes .
>
> Recommend Workaround:
>
> To determine what instruction the guest was executing the hypervisor
> will have to decode the instruction at the instruction pointer.
>
> The recommended workaround can not be implemented for the SEV
> guest because guest memory is encrypted with the guest specific key,
> and instruction decoder will not be able to decode the instruction
> bytes. If we hit this errata in the SEV guest then log the message
> and request a guest shutdown.
>
> Reported-by: Venkatesh Srinivas <venkateshs@xxxxxxxxxx>
> Cc: Jim Mattson <jmattson@xxxxxxxxxx>
> Cc: Tom Lendacky <thomas.lendacky@xxxxxxx>
> Cc: Borislav Petkov <bp@xxxxxxxxx>
> Cc: Joerg Roedel <joro@xxxxxxxxxx>
> Cc: "Radim Krčmář" <rkrcmar@xxxxxxxxxx>
> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Signed-off-by: Brijesh Singh <brijesh.singh@xxxxxxx>
> ---
>
> Changes since v1:
> * request to shutdown the guest instead of injecting #GP
> * use the pr_err_ratelimited to supress the host dmesg
>
> arch/x86/include/asm/kvm_host.h | 2 ++
> arch/x86/kvm/mmu.c | 6 ++++--
> arch/x86/kvm/svm.c | 32 ++++++++++++++++++++++++++++++++
> arch/x86/kvm/vmx/vmx.c | 6 ++++++
> 4 files changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 4660ce90de7f..536acf622af9 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1194,6 +1194,8 @@ struct kvm_x86_ops {
> int (*nested_enable_evmcs)(struct kvm_vcpu *vcpu,
> uint16_t *vmcs_version);
> uint16_t (*nested_get_evmcs_version)(struct kvm_vcpu *vcpu);
> +
> + bool (*emulate_instruction_possible)(struct kvm_vcpu *vcpu);
> };
>
> struct kvm_arch_async_pf {
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index da9c42349b1f..8ebc1bfcabd3 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -5384,8 +5384,10 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, u64 error_code,
> * page is not present in memory). In those cases we simply restart the
> * guest.
> */
> - if (unlikely(insn && !insn_len))
> - return 1;
> + if (unlikely(insn && !insn_len)) {
> + if (!kvm_x86_ops->emulate_instruction_possible(vcpu))
> + return 1;
> + }
IMO "insn && !insn_len" belongs in emulate_instruction_possible(). The
cost of the indirect function call can be avoided by making the callback
optional and only attaching it when necessary, e.g. if the CPU is
affected by the errata. E.g.:
For brevity, maybe emulation_impossible() as the ops' name? E.g.:
if (kvm_x86_ops->emulation_impossible &&
kvm_x86_ops->emulation_impossible(vcpu, insn, insn_len))
return 1;
>
> er = x86_emulate_instruction(vcpu, cr2, emulation_type, insn, insn_len);
