Re: [PATCH 1/2] KVM: SVM: Fix workaround for AMD Errata 1096

["Singh, Brijesh" <brijesh.singh@amd.com>]

On 7/16/19 2:34 PM, Liran Alon wrote:
> 
> 
>> On 16 Jul 2019, at 22:28, Singh, Brijesh <brijesh.singh@amd.com> wrote:
>>
>>
>>
>> On 7/16/19 12:35 PM, Liran Alon wrote:
>>>
>>>
>>>> On 16 Jul 2019, at 20:27, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>
>>>> On 16/07/19 18:56, Liran Alon wrote:
>>>>> If the CPU performs the VMExit transition of state before doing the data read for DecodeAssist,
>>>>> then I agree that CPL will be 0 on data-access regardless of vCPU CPL. But this also means that SMAP
>>>>> violation should be raised based on host CR4.SMAP value and not vCPU CR4.SMAP value as KVM code checks.
>>>>>
>>>>> Furthermore, vCPU CPL of guest doesn’t need to be 3 in order to trigger this Errata.
>>>>
>>>> Under the conditions in the code, if CPL were <3 then the SMAP fault
>>>> would have been sent to the guest.
>>>> But I agree that if we need to
>>>> change it to check host CR4, then the CPL of the guest should not be
>>>> checked.
>>>
>>> Yep.
>>> Well it all depends on how AMD CPU actually works.
>>> We need some clarification from AMD but for sure the current code in KVM is not only wrong, but probably have never been tested. :P
>>>
>>> Looking for further clarifications from AMD before submitting v2…
>>>
>>
>> When this errata is hit, the CPU will be at CPL3. From hardware
>> point-of-view the below sequence happens:
>>
>> 1. CPL3 guest hits reserved bit NPT fault (MMIO access)
> 
> Why CPU needs to be at CPL3?
> The requirement for SMAP should be that this page is user-accessible in guest page-tables.
> Think on a case where guest have CR4.SMAP=1 and CR4.SMEP=0.
> 

We are discussing reserved NPF so we need to be at CPL3.

>>
>> 2. Microcode uses special opcode which attempts to read data using the
>> CPL0 privileges. The microcode read CS:RIP, when it hits SMAP fault,
>> it gives up and returns no instruction bytes.
>>
>> (Note: vCPU is still at CPL3)
> 
> So at this point guest vCPU CR4.SMAP is what matters right? Not host CR4.SMAP.
> 

Yes, its guest vCPU SMAP.

>>
>> 3. CPU causes #VMEXIT for original fault address.
>>
>> The SMAP fault occurred while we are still in guest context. It will be
>> nice to have code test example to triggers this errata.
> 
> I can write such code in kvm-unit-tests for you to run on relevant hardware if you have such a machine present.
> I don’t have relevant machine with me and therefore I wrote a disclaimer I couldn’t test it in cover letter.
> 

I have required hardware and should be able to run some test for you.

> So to sum-up what KVM needs to do:
> 1) Check guest vCPU CR4.SMAP is set to 1. (As I fixed in this commit).
> 2) Remove the check for CPL==3. If we really want to be pedantic, we can parse guest page-tables to see if PTE have U/S bit set to 1.


Remember in the case of SEV guest, the page-tables are encrypted with
the guest specific key and we will not be able to walk it to inspect
the U/S bit. We want to detect the errata and if its SEV guest then
we can't do much, for non-SEV fallback to instruction decode which
will walk the guest page-tables to fetch the instruction bytes.


> What do you think?
> 
> -Liran
> 
>>
>>> -Liran
>>>
>>>>
>>>> Paolo
>>>>
>>>>> It’s only important that guest page-tables maps the guest RIP as user-accessible. i.e. U/S bit in PTE set to 1.
>>>>
>>>
>