Date:   Tue, 22 Oct 2019 10:32:11 +0200
From:   David Hildenbrand <david@...hat.com>
To:     Michal Hocko <mhocko@...nel.org>
Cc:     linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        Alexander Duyck <alexander.h.duyck@...ux.intel.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Anshuman Khandual <anshuman.khandual@....com>,
        Dan Williams <dan.j.williams@...el.com>,
        Mel Gorman <mgorman@...hsingularity.net>,
        Mike Rapoport <rppt@...ux.ibm.com>,
        Mike Rapoport <rppt@...ux.vnet.ibm.com>,
        Oscar Salvador <osalvador@...e.de>,
        Pavel Tatashin <pasha.tatashin@...een.com>,
        Pavel Tatashin <pavel.tatashin@...rosoft.com>,
        Pingfan Liu <kernelfans@...il.com>, Qian Cai <cai@....pw>,
        Vlastimil Babka <vbabka@...e.cz>,
        Wei Yang <richard.weiyang@...il.com>
Subject: Re: [PATCH v2 0/2] mm: Memory offlining + page isolation cleanups

On 22.10.19 10:21, Michal Hocko wrote:
> On Tue 22-10-19 10:15:07, David Hildenbrand wrote:
>> On 22.10.19 10:08, Michal Hocko wrote:
>>> On Tue 22-10-19 08:52:28, David Hildenbrand wrote:
>>>> On 21.10.19 19:23, David Hildenbrand wrote:
>>>>> Two cleanups that popped up while working on (and discussing) virtio-mem:
>>>>>     https://lkml.org/lkml/2019/9/19/463
>>>>>
>>>>> Tested with DIMMs on x86.
>>>>>
>>>>> As discussed with michal in v1, I'll soon look into removing the use
>>>>> of PG_reserved during memory onlining completely - most probably
>>>>> disallowing to offline memory blocks with holes, cleaning up the
>>>>> onlining+offlining code.
>>>>
>>>> BTW, I remember that ZONE_DEVICE pages are still required to be set
>>>> PG_reserved. That has to be sorted out first.
>>>
>>> Do they?
>>
>> Yes, especially KVM code :/
> 
> Details please?
> 


E.g., arch/x86/kvm/mmu.c:kvm_is_mmio_pfn()

And I currently have


>From 55606751b67989bd06d17844a6bcfbf85d44ee69 Mon Sep 17 00:00:00 2001
From: David Hildenbrand <david@...hat.com>
Date: Tue, 22 Oct 2019 10:08:04 +0200
Subject: [PATCH v1] KVM: x86/mmu: Prepare kvm_is_mmio_pfn() for PG_reserved
 changes

Right now, ZONE_DEVICE memory is always set PG_reserved. We want to
change that in the future.

KVM has this weird use case that you can map anything from /dev/mem
into the guest. pfn_valid() is not a reliable check whether the memmap
was initialized and can be touched. pfn_to_online_page() makes sure
that we have an initialized memmap - however, there is no reliable and
fast check to detect memmaps that were initialized and are ZONE_DEVICE.

Let's rewrite kvm_is_mmio_pfn() so we really only touch initialized
memmaps that are guaranteed to not contain garbage. Make sure that
RAM without a memmap is still not detected as MMIO and that ZONE_DEVICE
that is not UC/UC-/WC is not detected as MMIO.

Signed-off-by: David Hildenbrand <david@...hat.com>
---
 arch/x86/kvm/mmu.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 24c23c66b226..c91c9a5d14dc 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2962,23 +2962,29 @@ static bool mmu_need_write_protect(struct kvm_vcpu *vcpu, gfn_t gfn,
 
 static bool kvm_is_mmio_pfn(kvm_pfn_t pfn)
 {
-	if (pfn_valid(pfn))
-		return !is_zero_pfn(pfn) && PageReserved(pfn_to_page(pfn)) &&
-			/*
-			 * Some reserved pages, such as those from NVDIMM
-			 * DAX devices, are not for MMIO, and can be mapped
-			 * with cached memory type for better performance.
-			 * However, the above check misconceives those pages
-			 * as MMIO, and results in KVM mapping them with UC
-			 * memory type, which would hurt the performance.
-			 * Therefore, we check the host memory type in addition
-			 * and only treat UC/UC-/WC pages as MMIO.
-			 */
-			(!pat_enabled() || pat_pfn_immune_to_uc_mtrr(pfn));
+	struct page *page = pfn_to_online_page(pfn);
+
+	/*
+	 * Online pages consist of pages managed by the buddy. Especially,
+	 * ZONE_DEVICE pages are never online. Online pages that are reserved
+	 * indicate the zero page and MMIO pages.
+	 */
+	if (page)
+		return !is_zero_pfn(pfn) && PageReserved(pfn_to_page(pfn));
 
-	return !e820__mapped_raw_any(pfn_to_hpa(pfn),
-				     pfn_to_hpa(pfn + 1) - 1,
-				     E820_TYPE_RAM);
+	/*
+	 * Any RAM that is not online (e.g., mapped via /dev/mem without
+	 * a memmap or with an uninitialized memmap) is not MMIO.
+	 */
+	if (e820__mapped_raw_any(pfn_to_hpa(pfn), pfn_to_hpa(pfn + 1) - 1,
+				 E820_TYPE_RAM))
+		return false;
+
+	/*
+	 * Finally, anything with a valid memmap could be ZONE_DEVICE - or the
+	 * memmap could be uninitialized. Treat only UC/UC-/WC pages as MMIO.
+	 */
+	return pfn_valid() && !pat_enabled() || pat_pfn_immune_to_uc_mtrr(pfn);
 }
 
 /* Bits which may be returned by set_spte() */
-- 
2.21.0




And also virt/kvm/kvm_main.c:kvm_is_reserved_pfn()


>From 928be02b293750e6076c8622268ba41ecd8819e9 Mon Sep 17 00:00:00 2001
From: David Hildenbrand <david@...hat.com>
Date: Tue, 22 Oct 2019 10:26:46 +0200
Subject: [PATCH v1] KVM:Prepare kvm_is_reserved_pfn() for PG_reserved changes

Right now, ZONE_DEVICE memory is always set PG_reserved. We want to
change that in the future.

KVM has this weird use case that you can map anything from /dev/mem
into the guest. pfn_valid() is not a reliable check whether the memmap
was initialized and can be touched. pfn_to_online_page() makes sure
that we have an initialized memmap. Note that ZONE_DEVICE memory is
never online (IOW, managed by the buddy).

Switching to pfn_to_online_page() keeps the existing behavior for
PFNs without a memmap and for ZONE_DEVICE memory. They are treated as
reserved and the page is not touched (e.g., to set it dirty or accessed).

Signed-off-by: David Hildenbrand <david@...hat.com>
---
 virt/kvm/kvm_main.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 66a977472a1c..b98d5d44c2b8 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -151,9 +151,15 @@ __weak int kvm_arch_mmu_notifier_invalidate_range(struct kvm *kvm,
 
 bool kvm_is_reserved_pfn(kvm_pfn_t pfn)
 {
-	if (pfn_valid(pfn))
-		return PageReserved(pfn_to_page(pfn));
+	struct page *page = pfn_to_online_page(pfn);
 
+	/*
+	 * We treat any pages that are not online  (not managed by the buddy)
+	 * as reserved - this includes ZONE_DEVICE pages and pages without
+	 * a memmap (e.g.., mapped via /dev/mem).
+	 */
+	if (page)
+		return PageReserved(pfn_to_page(pfn));
 	return true;
 }
 
-- 
2.21.0



I'd like to note that the pfn_valid() check in __kvm_map_gfn() is also bogus,
but switching pfn_to_online_page() is not possible, as we would treat
ZONE_DEVICE memory differently then.

-- 

Thanks,

David / dhildenb

Powered by blists - more mailing lists