DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 48066C1E5B1C
Date: Mon, 31 Jul 2017 16:05:42 -0400 (EDT)
From: Paolo Bonzini <pbonzini@redhat.com>
To: Brijesh Singh <brijesh.singh@amd.com>
Cc: kvm@vger.kernel.org, thomas lendacky <thomas.lendacky@amd.com>,
        rkrcmar@redhat.com, joro@8bytes.org, x86@kernel.org,
        linux-kernel@vger.kernel.org, mingo@redhat.com, hpa@zytor.com,
        tglx@linutronix.de, bp@suse.de
Message-ID: <98086274.371452.1501531542630.JavaMail.zimbra@redhat.com>
In-Reply-To: <d7266e60-3bcc-946d-4325-dfd1126c9fc4@amd.com>
References: <147992048887.27638.17559991037474542240.stgit@brijesh-build-machine> <147992049856.27638.17076562184960611399.stgit@brijesh-build-machine> <da3f8d2d-04c2-d2f8-0752-7d484c9bc25b@redhat.com> <e8911a18-ce93-ca01-2876-3f7e90091f92@amd.com> <21b9f4db-f929-80f6-6ad2-6fa3b77f82c0@redhat.com> <d7266e60-3bcc-946d-4325-dfd1126c9fc4@amd.com>
Subject: Re: [PATCH v2 1/3] kvm: svm: Add support for additional SVM NPF
 error codes
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Thread-Topic: Add support for additional SVM NPF error codes
Thread-Index: BA/pH1Byf+GRxHrLyC6qSQcYvYFBKA==
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2424
Lines: 69


> > There can be different cases where an L0->L2 shadow nested page table is
> > marked read only, in particular when a page is read only in L1's nested
> > page tables.  If such a page is accessed by L2 while walking page tables
> > it will cause a nested page fault (page table walks are write accesses).
> >   However, after kvm_mmu_unprotect_page you will get another page fault,
> > and again in an endless stream.
> > 
> > Instead, emulation would have caused a nested page fault vmexit, I think.
> 
> If possible could you please give me some pointer on how to create this use
> case so that we can get definitive answer.
> 
> Looking at the code path is giving me indication that the new code
> (the kvm_mmu_unprotect_page call) only happens if vcpu->arch.mmu_page_fault()
> returns an indication that the instruction should be emulated. I would not
> expect that to be the case scenario you described above since L1 making a page
> read-only (this is a page table for L2) is an error and should result in #NPF
> being injected into L1.

The flow is:

  hardware walks page table; L2 page table points to read only memory
  -> pf_interception (code = 
  -> kvm_handle_page_fault (need_unprotect = false)
  -> kvm_mmu_page_fault
  -> paging64_page_fault (for example)
     -> try_async_pf
        map_writable set to false
     -> paging64_fetch(write_fault = true, map_writable = false, prefault = false)
        -> mmu_set_spte(speculative = false, host_writable = false, write_fault = true)
           -> set_spte
              mmu_need_write_protect returns true
              return true
           write_fault == true -> set emulate = true
           return true
        return true
     return true
  emulate

Without this patch, emulation would have called

  ..._gva_to_gpa_nested
  -> translate_nested_gpa
  -> paging64_gva_to_gpa
  -> paging64_walk_addr
  -> paging64_walk_addr_generic
     set fault (nested_page_fault=true)

and then:

   kvm_propagate_fault
   -> nested_svm_inject_npf_exit

> It's bit hard for me to visualize the code flow and
> figure out exactly how that would happen, but I just tried booting nested
> virtualization and it seem to be working okay.

I don't expect the above to happen when booting a normal guest (usual L1
guests hardly have readonly mappings).

> Is there a kvm-unit-test which I can run to trigger this scenario ? thanks

No, there isn't.

Paolo

> -Brijesh
>