Re: [Part2 PATCH v9 38/38] KVM: X86: Restart the guest when insn_len is zero and SEV is enabled

[Singh, Brijesh]

On 2/1/19 2:21 PM, Jim Mattson wrote:
> On Mon, Dec 4, 2017 at 5:07 PM Brijesh Singh <brijesh.singh@xxxxxxx> wrote:
>>
>> On AMD platforms, under certain conditions insn_len may be zero on #NPF.
>> This can happen if a guest gets a page-fault on data access but the HW
>> table walker is not able to read the instruction page (e.g instruction
>> page is not present in memory).
>>
>> Typically, when insn_len is zero, x86_emulate_instruction() walks the
>> guest page table and fetches the instruction bytes from guest memory.
>> When SEV is enabled, the guest memory is encrypted with guest-specific
>> key hence hypervisor will not able to fetch the instruction bytes.
>> In those cases we simply restart the guest.
>>
>> I have encountered this issue when running kernbench inside the guest.
>>
>> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
>> Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
>> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
>> Cc: "Radim KrÄmÃÅ" <rkrcmar@xxxxxxxxxx>
>> Cc: Joerg Roedel <joro@xxxxxxxxxx>
>> Cc: Borislav Petkov <bp@xxxxxxx>
>> Cc: Tom Lendacky <thomas.lendacky@xxxxxxx>
>> Cc: x86@xxxxxxxxxx
>> Cc: kvm@xxxxxxxxxxxxxxx
>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>> Signed-off-by: Brijesh Singh <brijesh.singh@xxxxxxx>
>> ---
>>   arch/x86/kvm/mmu.c | 10 ++++++++++
>>   arch/x86/kvm/svm.c |  6 ++++--
>>   2 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
>> index e5e66e5c6640..d5e5dbd0e5ad 100644
>> --- a/arch/x86/kvm/mmu.c
>> +++ b/arch/x86/kvm/mmu.c
>> @@ -4950,6 +4950,16 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, u64 error_code,
>>          if (mmio_info_in_cache(vcpu, cr2, direct))
>>                  emulation_type = 0;
>>   emulate:
>> +       /*
>> +        * On AMD platforms, under certain conditions insn_len may be zero on #NPF.
>> +        * This can happen if a guest gets a page-fault on data access but the HW
>> +        * table walker is not able to read the instruction page (e.g instruction
>> +        * page is not present in memory). In those cases we simply restart the
>> +        * guest.
>> +        */
>> +       if (unlikely(insn && !insn_len))
>> +               return 1;
>> +
> 
> How does this work, for instance, with MMIO at CPL3 with SMAP enabled?
> 


The processor will still attempt to supply correct instruction
bytes in this case (basically it disables SMAP temporarily during
this read). *HOWEVER*, looking through Zen docs, there is an
errata about this on Zen. See errata 1096 in
https://www.amd.com/system/files/TechDocs/55449_Fam_17h_M_00h0Fh_Rev_Guide.pdf

Looking at errata it seems on Zen processors, the CPU will not
supply instruction bytes in this scenario (MMIO at CPL3 with
SMAP=1). I will reach out to HW folks to see if there is any
plans to fix in upcoming CPU cores.

We probably need to workaround for this errata. In non SEV case
we can let the host read the instruction bytes but in SEV case
we can disable SMAP temporarily in CR4, then retry the guest,
and then restore the CR4.SMAP settings. I will work on patch
and post on mailing list.

-Brijesh