On 7/16/19 2:34 PM, Liran Alon wrote: > > >> On 16 Jul 2019, at 22:28, Singh, Brijesh <brijesh.singh@xxxxxxx> wrote: >> >> >> >> On 7/16/19 12:35 PM, Liran Alon wrote: >>> >>> >>>> On 16 Jul 2019, at 20:27, Paolo Bonzini <pbonzini@xxxxxxxxxx> 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. >>>> >>> >