| Message ID | 20240507062009.20336-1-yan.y.zhao@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Enforce CPU cache flush for non-coherent device assignment | expand |
> From: Zhao, Yan Y <yan.y.zhao@intel.com> > Sent: Tuesday, May 7, 2024 2:20 PM > @@ -101,9 +101,21 @@ static bool kvm_is_mmio_pfn(kvm_pfn_t pfn) > */ > (!pat_enabled() || > pat_pfn_immune_to_uc_mtrr(pfn)); > > + /* > + * If the PFN is invalid and not RAM in raw e820 table, keep treating it > + * as MMIO. > + * > + * If the PFN is invalid and is RAM in raw e820 table, > + * - if PAT is not enabled, always treat the PFN as MMIO to avoid > futher > + * checking of MTRRs. > + * - if PAT is enabled, treat the PFN as MMIO if its PAT is UC/WC/UC- > in > + * primary MMU. > + * to prevent guest cacheable access to MMIO PFNs. > + */ > return !e820__mapped_raw_any(pfn_to_hpa(pfn), > pfn_to_hpa(pfn + 1) - 1, > - E820_TYPE_RAM); > + E820_TYPE_RAM) ? true : > + (!pat_enabled() || > pat_pfn_immune_to_uc_mtrr(pfn)); Is it for another theoretical problem in case the primary mmu uses a non-WB type on a invalid RAM-type pfn so you want to do additional scrutiny here?
On Tue, May 07, 2024 at 04:39:27PM +0800, Tian, Kevin wrote: > > From: Zhao, Yan Y <yan.y.zhao@intel.com> > > Sent: Tuesday, May 7, 2024 2:20 PM > > @@ -101,9 +101,21 @@ static bool kvm_is_mmio_pfn(kvm_pfn_t pfn) > > */ > > (!pat_enabled() || > > pat_pfn_immune_to_uc_mtrr(pfn)); > > > > + /* > > + * If the PFN is invalid and not RAM in raw e820 table, keep treating it > > + * as MMIO. > > + * > > + * If the PFN is invalid and is RAM in raw e820 table, > > + * - if PAT is not enabled, always treat the PFN as MMIO to avoid > > futher > > + * checking of MTRRs. > > + * - if PAT is enabled, treat the PFN as MMIO if its PAT is UC/WC/UC- > > in > > + * primary MMU. > > + * to prevent guest cacheable access to MMIO PFNs. > > + */ > > return !e820__mapped_raw_any(pfn_to_hpa(pfn), > > pfn_to_hpa(pfn + 1) - 1, > > - E820_TYPE_RAM); > > + E820_TYPE_RAM) ? true : > > + (!pat_enabled() || > > pat_pfn_immune_to_uc_mtrr(pfn)); > > Is it for another theoretical problem in case the primary > mmu uses a non-WB type on a invalid RAM-type pfn so > you want to do additional scrutiny here? Yes. Another untold reason is that patch 3 does not do CLFLUSH to this type of memory since it's mapped as uncacheable in primary MMU. I feel that it's better to ensure guest will not access it in cacheable memory type either.
diff --git a/arch/x86/kvm/mmu/spte.c b/arch/x86/kvm/mmu/spte.c index 4a599130e9c9..5db0fb7b74f5 100644 --- a/arch/x86/kvm/mmu/spte.c +++ b/arch/x86/kvm/mmu/spte.c @@ -101,9 +101,21 @@ static bool kvm_is_mmio_pfn(kvm_pfn_t pfn) */ (!pat_enabled() || pat_pfn_immune_to_uc_mtrr(pfn)); + /* + * If the PFN is invalid and not RAM in raw e820 table, keep treating it + * as MMIO. + * + * If the PFN is invalid and is RAM in raw e820 table, + * - if PAT is not enabled, always treat the PFN as MMIO to avoid futher + * checking of MTRRs. + * - if PAT is enabled, treat the PFN as MMIO if its PAT is UC/WC/UC- in + * primary MMU. + * to prevent guest cacheable access to MMIO PFNs. + */ return !e820__mapped_raw_any(pfn_to_hpa(pfn), pfn_to_hpa(pfn + 1) - 1, - E820_TYPE_RAM); + E820_TYPE_RAM) ? true : + (!pat_enabled() || pat_pfn_immune_to_uc_mtrr(pfn)); } /*
Fine-grained check to decide whether a PFN, which is !pfn_valid() and identified within the raw e820 table as RAM, should be treated as MMIO by KVM in order to prevent guest cachable access. Previously, a PFN that is !pfn_valid() and identified within the raw e820 table as RAM was not considered as MMIO. This is for the scenerio when "mem=" was passed to the kernel, resulting in certain valid pages lacking an associated struct page. See commit 0c55671f84ff ("kvm, x86: Properly check whether a pfn is an MMIO or not"). However, that approach is only based on guest performance perspective and may cause cacheable access to potential MMIO PFNs if pat_pfn_immune_to_uc_mtrr() identifies the PFN as having a PAT type of UC/WC/UC-. Therefore, do a fine-graned check for PAT in primary MMU so that KVM would map the PFN as UC in EPT to prevent cachable access from guest. For the rare case when PAT is not enabled, default the PFN to MMIO to avoid further checking MTRR (since functions for MTRR related checking are not exported now). Cc: Kevin Tian <kevin.tian@intel.com> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com> --- arch/x86/kvm/mmu/spte.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)