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@xxxxxxxxxx> 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@xxxxxxxxxx> --- 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@xxxxxxxxxx> 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@xxxxxxxxxx> --- 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
