> On 16 Jul 2019, at 23:02, Singh, Brijesh <brijesh.singh@xxxxxxx> wrote: > > > > 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. I don’t see the connection between a reserved #NPF and the need to be at CPL3. A vCPU can execute at CPL<3 a page that is mapped user-accessible in guest page-tables in case CR4.SMEP=0 and then instruction will execute successfully and can dereference a page that is mapped in NPT using an entry with a reserved bit set. Thus, reserved #NPF will be raised while vCPU is at CPL<3 and DecodeAssist microcode will still raise SMAP violation as CR4.SMAP=1 and microcode perform data-fetch with CPL<3. This leading exactly to Errata condition as far as I understand. > >>> >>> 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. Ok. Will write such kvm-unit-test and Cc 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. >>>>> >>>> >>
