Hi all,
This is the v3 of the series previously posted here:
https://lore.kernel.org/kvmarm/20201117181607.1761516-1-qperret@google.com/
This basically allows us to wrap the host with a stage 2 when running in
nVHE, hence paving the way for protecting guest memory from the host in
the future (among other use-cases). For more details about the
motivation and the design angle taken here, I would recommend to have a
look at the cover letter of v1, and/or to watch these presentations at
LPC [1] and KVM forum 2020 [2].
V3 includes a bunch of clean-ups and small refactorings all over the
place as well as a few new features. Specifically, this now allows us to
remove memory pages from the host stage 2 cleanly, and this series does
so for all the .hyp memory sections (which has uncovered existing bugs
upstream and in v2 of this series -- see [3] and [4]). This also now
makes good use of block mappings whenever that is possible, and has
gotten a bit more testing on real hardware (which helped uncover other
bugs [5]).
The other changes to v3 include:
- clean-ups, refactoring and extra comments all over the place (Will);
- dropped fdt hook in favor of memblock API now that the relevant
patches ([6]) are merged (Rob);
- moved the CPU feature copy stuff to __init/__initdata (Marc);
- fixed FWB support (Mate);
- rebased on v5.12-rc1.
This series depends on Will's vCPU context fix ([5]) and Marc's PMU
fixes ([7]). And here's a branch with all the goodies applied:
https://android-kvm.googlesource.com/linux qperret/host-stage2-v3
Thanks,
Quentin
[1] https://youtu.be/54q6RzS9BpQ?t=10859
[2] https://youtu.be/wY-u6n75iXc
[3] https://lore.kernel.org/kvmarm/20210203141931.615898-1-qperret@google.com/
[4] https://lore.kernel.org/kvmarm/20210128173850.2478161-1-qperret@google.com/
[5] https://lore.kernel.org/kvmarm/20210226181211.14542-1-will@kernel.org/
[6] https://lore.kernel.org/lkml/20210115114544.1830068-1-qperret@google.com/
[7] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/pmu-undef-NV
Quentin Perret (29):
KVM: arm64: Initialize kvm_nvhe_init_params early
KVM: arm64: Avoid free_page() in page-table allocator
KVM: arm64: Factor memory allocation out of pgtable.c
KVM: arm64: Introduce a BSS section for use at Hyp
KVM: arm64: Make kvm_call_hyp() a function call at Hyp
KVM: arm64: Allow using kvm_nvhe_sym() in hyp code
KVM: arm64: Introduce an early Hyp page allocator
KVM: arm64: Stub CONFIG_DEBUG_LIST at Hyp
KVM: arm64: Introduce a Hyp buddy page allocator
KVM: arm64: Enable access to sanitized CPU features at EL2
KVM: arm64: Factor out vector address calculation
KVM: arm64: Prepare the creation of s1 mappings at EL2
KVM: arm64: Elevate hypervisor mappings creation at EL2
KVM: arm64: Use kvm_arch for stage 2 pgtable
KVM: arm64: Use kvm_arch in kvm_s2_mmu
KVM: arm64: Set host stage 2 using kvm_nvhe_init_params
KVM: arm64: Refactor kvm_arm_setup_stage2()
KVM: arm64: Refactor __load_guest_stage2()
KVM: arm64: Refactor __populate_fault_info()
KVM: arm64: Make memcache anonymous in pgtable allocator
KVM: arm64: Reserve memory for host stage 2
KVM: arm64: Sort the hypervisor memblocks
KVM: arm64: Introduce PROT_NONE mappings for stage 2
KVM: arm64: Refactor stage2_map_set_prot_attr()
KVM: arm64: Add kvm_pgtable_stage2_idmap_greedy()
KVM: arm64: Wrap the host with a stage 2
KVM: arm64: Page-align the .hyp sections
KVM: arm64: Disable PMU support in protected mode
KVM: arm64: Protect the .hyp sections from the host
Will Deacon (3):
arm64: lib: Annotate {clear,copy}_page() as position-independent
KVM: arm64: Link position-independent string routines into .hyp.text
arm64: kvm: Add standalone ticket spinlock implementation for use at
hyp
arch/arm64/include/asm/cpufeature.h | 1 +
arch/arm64/include/asm/hyp_image.h | 7 +
arch/arm64/include/asm/kvm_asm.h | 9 +
arch/arm64/include/asm/kvm_cpufeature.h | 19 ++
arch/arm64/include/asm/kvm_host.h | 19 +-
arch/arm64/include/asm/kvm_hyp.h | 8 +
arch/arm64/include/asm/kvm_mmu.h | 23 +-
arch/arm64/include/asm/kvm_pgtable.h | 117 ++++++-
arch/arm64/include/asm/sections.h | 1 +
arch/arm64/kernel/asm-offsets.c | 3 +
arch/arm64/kernel/cpufeature.c | 13 +
arch/arm64/kernel/image-vars.h | 30 ++
arch/arm64/kernel/vmlinux.lds.S | 74 +++--
arch/arm64/kvm/arm.c | 199 ++++++++++--
arch/arm64/kvm/hyp/Makefile | 2 +-
arch/arm64/kvm/hyp/include/hyp/switch.h | 37 ++-
arch/arm64/kvm/hyp/include/nvhe/early_alloc.h | 14 +
arch/arm64/kvm/hyp/include/nvhe/gfp.h | 55 ++++
arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 36 +++
arch/arm64/kvm/hyp/include/nvhe/memory.h | 52 +++
arch/arm64/kvm/hyp/include/nvhe/mm.h | 92 ++++++
arch/arm64/kvm/hyp/include/nvhe/spinlock.h | 92 ++++++
arch/arm64/kvm/hyp/nvhe/Makefile | 9 +-
arch/arm64/kvm/hyp/nvhe/cache.S | 13 +
arch/arm64/kvm/hyp/nvhe/cpufeature.c | 8 +
arch/arm64/kvm/hyp/nvhe/early_alloc.c | 54 ++++
arch/arm64/kvm/hyp/nvhe/hyp-init.S | 46 ++-
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 69 ++++
arch/arm64/kvm/hyp/nvhe/hyp.lds.S | 1 +
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 235 ++++++++++++++
arch/arm64/kvm/hyp/nvhe/mm.c | 173 ++++++++++
arch/arm64/kvm/hyp/nvhe/page_alloc.c | 195 ++++++++++++
arch/arm64/kvm/hyp/nvhe/psci-relay.c | 4 +-
arch/arm64/kvm/hyp/nvhe/setup.c | 212 +++++++++++++
arch/arm64/kvm/hyp/nvhe/stub.c | 22 ++
arch/arm64/kvm/hyp/nvhe/switch.c | 12 +-
arch/arm64/kvm/hyp/nvhe/tlb.c | 4 +-
arch/arm64/kvm/hyp/pgtable.c | 298 ++++++++++++++----
arch/arm64/kvm/hyp/reserved_mem.c | 113 +++++++
arch/arm64/kvm/mmu.c | 115 ++++++-
arch/arm64/kvm/perf.c | 3 +-
arch/arm64/kvm/pmu.c | 8 +-
arch/arm64/kvm/reset.c | 42 +--
arch/arm64/kvm/sys_regs.c | 21 ++
arch/arm64/lib/clear_page.S | 4 +-
arch/arm64/lib/copy_page.S | 4 +-
arch/arm64/mm/init.c | 3 +
47 files changed, 2356 insertions(+), 215 deletions(-)
create mode 100644 arch/arm64/include/asm/kvm_cpufeature.h
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/early_alloc.h
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/gfp.h
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/mem_protect.h
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/memory.h
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/mm.h
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/spinlock.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/cache.S
create mode 100644 arch/arm64/kvm/hyp/nvhe/cpufeature.c
create mode 100644 arch/arm64/kvm/hyp/nvhe/early_alloc.c
create mode 100644 arch/arm64/kvm/hyp/nvhe/mem_protect.c
create mode 100644 arch/arm64/kvm/hyp/nvhe/mm.c
create mode 100644 arch/arm64/kvm/hyp/nvhe/page_alloc.c
create mode 100644 arch/arm64/kvm/hyp/nvhe/setup.c
create mode 100644 arch/arm64/kvm/hyp/nvhe/stub.c
create mode 100644 arch/arm64/kvm/hyp/reserved_mem.c
--
2.30.1.766.gb4fecdf3b7-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
In order to use the kernel list library at EL2, introduce stubs for the
CONFIG_DEBUG_LIST out-of-lines calls.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
arch/arm64/kvm/hyp/nvhe/stub.c | 22 ++++++++++++++++++++++
2 files changed, 23 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/kvm/hyp/nvhe/stub.c
With nVHE, the host currently creates all stage 1 hypervisor mappings at
EL1 during boot, installs them at EL2, and extends them as required
(e.g. when creating a new VM). But in a world where the host is no
longer trusted, it cannot have full control over the code mapped in the
hypervisor.
In preparation for enabling the hypervisor to create its own stage 1
mappings during boot, introduce an early page allocator, with minimal
functionality. This allocator is designed to be used only during early
bootstrap of the hyp code when memory protection is enabled, which will
then switch to using a full-fledged page allocator after init.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/hyp/include/nvhe/early_alloc.h | 14 +++++
arch/arm64/kvm/hyp/include/nvhe/memory.h | 24 +++++++++
arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
arch/arm64/kvm/hyp/nvhe/early_alloc.c | 54 +++++++++++++++++++
arch/arm64/kvm/hyp/nvhe/psci-relay.c | 4 +-
5 files changed, 94 insertions(+), 4 deletions(-)
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/early_alloc.h
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/memory.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/early_alloc.c
In order to make use of the stage 2 pgtable code for the host stage 2,
change kvm_s2_mmu to use a kvm_arch pointer in lieu of the kvm pointer,
as the host will have the former but not the latter.
Acked-by: Will Deacon <will@kernel.org>
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_host.h | 2 +-
arch/arm64/include/asm/kvm_mmu.h | 6 +++++-
arch/arm64/kvm/mmu.c | 8 ++++----
3 files changed, 10 insertions(+), 6 deletions(-)
@@ -94,7 +94,7 @@ struct kvm_s2_mmu {/* The last vcpu id that ran on each physical CPU */int__percpu*last_vcpu_ran;-structkvm*kvm;+structkvm_arch*arch;};structkvm_arch_memory_slot{
In order to re-map the guest vectors at EL2 when pKVM is enabled,
refactor __kvm_vector_slot2idx() and kvm_init_vector_slot() to move all
the address calculation logic in a static inline function.
Acked-by: Will Deacon <will@kernel.org>
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_mmu.h | 8 ++++++++
arch/arm64/kvm/arm.c | 9 +--------
2 files changed, 9 insertions(+), 8 deletions(-)
@@ -1345,16 +1345,9 @@ static unsigned long nvhe_percpu_order(void)/* A lookup table holding the hypervisor VA for each vector slot */staticvoid*hyp_spectre_vector_selector[BP_HARDEN_EL2_SLOTS];-staticint__kvm_vector_slot2idx(enumarm64_hyp_spectre_vectorslot)-{-returnslot-(slot!=HYP_VECTOR_DIRECT);-}-staticvoidkvm_init_vector_slot(void*base,enumarm64_hyp_spectre_vectorslot){-intidx=__kvm_vector_slot2idx(slot);--hyp_spectre_vector_selector[slot]=base+(idx*SZ_2K);+hyp_spectre_vector_selector[slot]=__kvm_vector_slot2addr(base,slot);}staticintkvm_init_vector_slots(void)
--
2.30.1.766.gb4fecdf3b7-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
When memory protection is enabled, the hyp code will require a basic
form of memory management in order to allocate and free memory pages at
EL2. This is needed for various use-cases, including the creation of hyp
mappings or the allocation of stage 2 page tables.
To address these use-case, introduce a simple memory allocator in the
hyp code. The allocator is designed as a conventional 'buddy allocator',
working with a page granularity. It allows to allocate and free
physically contiguous pages from memory 'pools', with a guaranteed order
alignment in the PA space. Each page in a memory pool is associated
with a struct hyp_page which holds the page's metadata, including its
refcount, as well as its current order, hence mimicking the kernel's
buddy system in the GFP infrastructure. The hyp_page metadata are made
accessible through a hyp_vmemmap, following the concept of
SPARSE_VMEMMAP in the kernel.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/hyp/include/nvhe/gfp.h | 55 +++++++
arch/arm64/kvm/hyp/include/nvhe/memory.h | 28 ++++
arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
arch/arm64/kvm/hyp/nvhe/page_alloc.c | 195 +++++++++++++++++++++++
4 files changed, 279 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/gfp.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/page_alloc.c
@@ -0,0 +1,195 @@+// SPDX-License-Identifier: GPL-2.0-only+/*+*Copyright(C)2020GoogleLLC+*Author:QuentinPerret<qperret@google.com>+*/++#include<asm/kvm_hyp.h>+#include<nvhe/gfp.h>++u64__hyp_vmemmap;++/*+*Indexthehyp_vmemmaptofindapotentialbuddypage,butmakenoassumption+*aboutitscurrentstate.+*+*Examplebuddy-treefora4-pagesphysicallycontiguouspool:+*+*o:Page3+*/+*o-o:Page2+*/+*/o:Page1+*//+*o---o-o:Page0+*Order210+*+*Exampleofrequestsonthispool:+*__find_buddy_nocheck(pool,page0,order0)=>page1+*__find_buddy_nocheck(pool,page0,order1)=>page2+*__find_buddy_nocheck(pool,page1,order0)=>page0+*__find_buddy_nocheck(pool,page2,order0)=>page3+*/+staticstructhyp_page*__find_buddy_nocheck(structhyp_pool*pool,+structhyp_page*p,+unsignedintorder)+{+phys_addr_taddr=hyp_page_to_phys(p);++addr^=(PAGE_SIZE<<order);++/*+*Don'treturnapageoutsidethepoolrange--itbelongsto+*somethingelseandmaynotbemappedinhyp_vmemmap.+*/+if(addr<pool->range_start||addr>=pool->range_end)+returnNULL;++returnhyp_phys_to_page(addr);+}++/* Find a buddy page currently available for allocation */+staticstructhyp_page*__find_buddy_avail(structhyp_pool*pool,+structhyp_page*p,+unsignedintorder)+{+structhyp_page*buddy=__find_buddy_nocheck(pool,p,order);++if(!buddy||buddy->order!=order||list_empty(&buddy->node))+returnNULL;++returnbuddy;++}++staticvoid__hyp_attach_page(structhyp_pool*pool,+structhyp_page*p)+{+unsignedintorder=p->order;+structhyp_page*buddy;++memset(hyp_page_to_virt(p),0,PAGE_SIZE<<p->order);++/*+*Onlythefirststructhyp_pageofahigh-orderpage(otherwiseknown+*asthe'head')shouldhavep->orderset.Thenon-headpagesshould+*havep->order=HYP_NO_ORDER.Here@pmaynolongerbethehead+*aftercoallescing,somakesuretomarkitHYP_NO_ORDERproactively.+*/+p->order=HYP_NO_ORDER;+for(;(order+1)<pool->max_order;order++){+buddy=__find_buddy_avail(pool,p,order);+if(!buddy)+break;++/* Take the buddy out of its list, and coallesce with @p */+list_del_init(&buddy->node);+buddy->order=HYP_NO_ORDER;+p=(p<buddy)?p:buddy;+}++/* Mark the new head, and insert it */+p->order=order;+list_add_tail(&p->node,&pool->free_area[order]);+}++staticvoidhyp_attach_page(structhyp_page*p)+{+structhyp_pool*pool=hyp_page_to_pool(p);++hyp_spin_lock(&pool->lock);+__hyp_attach_page(pool,p);+hyp_spin_unlock(&pool->lock);+}++staticstructhyp_page*__hyp_extract_page(structhyp_pool*pool,+structhyp_page*p,+unsignedintorder)+{+structhyp_page*buddy;++list_del_init(&p->node);+while(p->order>order){+/*+*Thebuddyofordern-1currentlyhasHYP_NO_ORDERasit+*iscoveredbyahigher-levelpage(whoseheadis@p).Use+*__find_buddy_nocheck()tofinditandinjectitinthe+*free_list[n-1],effectivelysplitting@pinhalf.+*/+p->order--;+buddy=__find_buddy_nocheck(pool,p,p->order);+buddy->order=p->order;+list_add_tail(&buddy->node,&pool->free_area[buddy->order]);+}++returnp;+}++voidhyp_put_page(void*addr)+{+structhyp_page*p=hyp_virt_to_page(addr);++if(hyp_page_ref_dec_and_test(p))+hyp_attach_page(p);+}++voidhyp_get_page(void*addr)+{+structhyp_page*p=hyp_virt_to_page(addr);++hyp_page_ref_inc(p);+}++void*hyp_alloc_pages(structhyp_pool*pool,unsignedintorder)+{+unsignedinti=order;+structhyp_page*p;++hyp_spin_lock(&pool->lock);++/* Look for a high-enough-order page */+while(i<pool->max_order&&list_empty(&pool->free_area[i]))+i++;+if(i>=pool->max_order){+hyp_spin_unlock(&pool->lock);+returnNULL;+}++/* Extract it from the tree at the right order */+p=list_first_entry(&pool->free_area[i],structhyp_page,node);+p=__hyp_extract_page(pool,p,order);++hyp_spin_unlock(&pool->lock);+hyp_page_ref_inc(p);++returnp?hyp_page_to_virt(p):NULL;+}++inthyp_pool_init(structhyp_pool*pool,u64pfn,unsignedintnr_pages,+unsignedintreserved_pages)+{+phys_addr_tphys=hyp_pfn_to_phys(pfn);+structhyp_page*p;+inti;++hyp_spin_lock_init(&pool->lock);+pool->max_order=min(MAX_ORDER,get_order(nr_pages<<PAGE_SHIFT));+for(i=0;i<pool->max_order;i++)+INIT_LIST_HEAD(&pool->free_area[i]);+pool->range_start=phys;+pool->range_end=phys+(nr_pages<<PAGE_SHIFT);++/* Init the vmemmap portion */+p=hyp_phys_to_page(phys);+memset(p,0,sizeof(*p)*nr_pages);+for(i=0;i<nr_pages;i++){+p[i].pool=pool;+INIT_LIST_HEAD(&p[i].node);+}++/* Attach the unused pages to the buddy tree */+for(i=reserved_pages;i<nr_pages;i++)+__hyp_attach_page(pool,&p[i]);++return0;+}
--
2.30.1.766.gb4fecdf3b7-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Refactor __load_guest_stage2() to introduce __load_stage2() which will
be re-used when loading the host stage 2.
Acked-by: Will Deacon <will@kernel.org>
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_mmu.h | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
In order to re-use some of the stage 2 setup code at EL2, factor parts
of kvm_arm_setup_stage2() out into separate functions.
No functional change intended.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_pgtable.h | 26 +++++++++++++++++
arch/arm64/kvm/hyp/pgtable.c | 32 +++++++++++++++++++++
arch/arm64/kvm/reset.c | 42 +++-------------------------
3 files changed, 62 insertions(+), 38 deletions(-)
@@ -332,19 +332,10 @@ int kvm_set_ipa_limit(void)return0;}-/*-*ConfiguretheVTCR_EL2forthisVM.TheVTCRvalueiscommon-*acrossallthephysicalCPUsonthesystem.Weusesystemwide-*sanitisedvaluestofillindifferentfields,exceptforHardware-*ManagementofAccessFlags.HAFlagissetunconditionallyon-*allCPUs,asitissafetorunwithorwithoutthefeatureand-*thebitisRES0onCPUsthatdon'tsupportit.-*/intkvm_arm_setup_stage2(structkvm*kvm,unsignedlongtype){-u64vtcr=VTCR_EL2_FLAGS,mmfr0;-u32parange,phys_shift;-u8lvls;+u64mmfr0,mmfr1;+u32phys_shift;if(type&~KVM_VM_TYPE_ARM_IPA_SIZE_MASK)return-EINVAL;
@@ -359,33 +350,8 @@ int kvm_arm_setup_stage2(struct kvm *kvm, unsigned long type)}mmfr0=read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);-parange=cpuid_feature_extract_unsigned_field(mmfr0,-ID_AA64MMFR0_PARANGE_SHIFT);-if(parange>ID_AA64MMFR0_PARANGE_MAX)-parange=ID_AA64MMFR0_PARANGE_MAX;-vtcr|=parange<<VTCR_EL2_PS_SHIFT;--vtcr|=VTCR_EL2_T0SZ(phys_shift);-/*-*Useaminimum2levelpagetabletopreventsplitting-*hostPMDhugepagesatstage2.-*/-lvls=stage2_pgtable_levels(phys_shift);-if(lvls<2)-lvls=2;-vtcr|=VTCR_EL2_LVLS_TO_SL0(lvls);--/*-*EnabletheHardwareAccessFlagmanagement,unconditionally-*onallCPUs.ThefeaturesisRES0onCPUswithoutthesupport-*andmustbeignoredbytheCPUs.-*/-vtcr|=VTCR_EL2_HA;+mmfr1=read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);+kvm->arch.vtcr=kvm_get_vtcr(mmfr0,mmfr1,phys_shift);-/* Set the vmid bits */-vtcr|=(kvm_get_vmid_bits()==16)?-VTCR_EL2_VS_16BIT:-VTCR_EL2_VS_8BIT;-kvm->arch.vtcr=vtcr;return0;}
--
2.30.1.766.gb4fecdf3b7-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
The current stage2 page-table allocator uses a memcache to get
pre-allocated pages when it needs any. To allow re-using this code at
EL2 which uses a concept of memory pools, make the memcache argument of
kvm_pgtable_stage2_map() anonymous, and let the mm_ops zalloc_page()
callbacks use it the way they need to.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_pgtable.h | 6 +++---
arch/arm64/kvm/hyp/pgtable.c | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
Extend the memory pool allocated for the hypervisor to include enough
pages to map all of memory at page granularity for the host stage 2.
While at it, also reserve some memory for device mappings.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/hyp/include/nvhe/mm.h | 23 ++++++++++++++++++++++-
arch/arm64/kvm/hyp/nvhe/setup.c | 12 ++++++++++++
arch/arm64/kvm/hyp/reserved_mem.c | 2 ++
3 files changed, 36 insertions(+), 1 deletion(-)
Add a new map function to the KVM page-table library that allows to
greedily create block identity-mappings. This will be useful to create
lazily the host stage 2 page-table as it will own most of memory and
will always be identity mapped.
The new helper function creates the mapping in 2 steps: it first walks
the page-table to compute the largest possible granule that can be used
to idmap a given address without overriding existing incompatible
mappings; and then creates a mapping accordingly.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_pgtable.h | 37 +++++++++
arch/arm64/kvm/hyp/pgtable.c | 119 +++++++++++++++++++++++++++
2 files changed, 156 insertions(+)
@@ -987,3 +987,122 @@ void kvm_pgtable_stage2_destroy(struct kvm_pgtable *pgt)pgt->mm_ops->free_pages_exact(pgt->pgd,pgd_sz);pgt->pgd=NULL;}++structstage2_reduce_range_data{+kvm_pte_tattr;+u64target_addr;+u32start_level;+structkvm_mem_range*range;+};++staticint__stage2_reduce_range(structstage2_reduce_range_data*data,u64addr)+{+u32level=data->start_level;++for(;level<KVM_PGTABLE_MAX_LEVELS;level++){+u64granule=kvm_granule_size(level);+u64start=ALIGN_DOWN(data->target_addr,granule);+u64end=start+granule;++/*+*Thepinnedaddressisinthecurrentrange,tryonelevel+*deeper.+*/+if(start==ALIGN_DOWN(addr,granule))+continue;++/*+*Makesurethecurrentrangeisareductionoftheexisting+*rangebeforeupdatingit.+*/+if(data->range->start<=start&&end<=data->range->end){+data->start_level=level;+data->range->start=start;+data->range->end=end;+return0;+}+}++return-EINVAL;+}++#define KVM_PTE_LEAF_S2_COMPAT_MASK (KVM_PTE_LEAF_ATTR_S2_PERMS | \+KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR|\+KVM_PTE_LEAF_SW_BIT_PROT_NONE)++staticintstage2_reduce_range_walker(u64addr,u64end,u32level,+kvm_pte_t*ptep,+enumkvm_pgtable_walk_flagsflag,+void*constarg)+{+structstage2_reduce_range_data*data=arg;+kvm_pte_tattr;+intret;++if(addr<data->range->start||addr>=data->range->end)+return0;++attr=*ptep&KVM_PTE_LEAF_S2_COMPAT_MASK;+if(!attr||attr==data->attr)+return0;++/*+*Anexistingmappingwithincompatibleprotectionattributesis+*'pinned',soreducetherangeifwehitone.+*/+ret=__stage2_reduce_range(data,addr);+if(ret)+returnret;++return-EAGAIN;+}++staticintstage2_reduce_range(structkvm_pgtable*pgt,u64addr,+enumkvm_pgtable_protprot,+structkvm_mem_range*range)+{+structstage2_reduce_range_datadata={+.start_level=pgt->start_level,+.range=range,+.target_addr=addr,+};+structkvm_pgtable_walkerwalker={+.cb=stage2_reduce_range_walker,+.flags=KVM_PGTABLE_WALK_LEAF,+.arg=&data,+};+intret;++data.attr=stage2_get_prot_attr(prot)&KVM_PTE_LEAF_S2_COMPAT_MASK;+if(!data.attr)+return-EINVAL;++/* Reduce the kvm_mem_range to a granule size */+ret=__stage2_reduce_range(&data,range->end);+if(ret)+returnret;++/* Walk the range to check permissions and reduce further if needed */+do{+ret=kvm_pgtable_walk(pgt,range->start,range->end,&walker);+}while(ret==-EAGAIN);++returnret;+}++intkvm_pgtable_stage2_idmap_greedy(structkvm_pgtable*pgt,u64addr,+enumkvm_pgtable_protprot,+structkvm_mem_range*range,+void*mc)+{+u64size;+intret;++ret=stage2_reduce_range(pgt,addr,prot,range);+if(ret)+returnret;++size=range->end-range->start;+returnkvm_pgtable_stage2_map(pgt,range->start,size,range->start,+prot,mc);+}
--
2.30.1.766.gb4fecdf3b7-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
When KVM runs in nVHE protected mode, use the host stage 2 to unmap the
hypervisor sections. The long-term goal is to ensure the EL2 code can
remain robust regardless of the host's state, so this starts by making
sure the host cannot e.g. write to the .hyp sections directly.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_asm.h | 1 +
arch/arm64/kvm/arm.c | 46 +++++++++++++++++++
arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 2 +
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 9 ++++
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 22 +++++++++
5 files changed, 80 insertions(+)
When KVM runs in protected nVHE mode, make use of a stage 2 page-table
to give the hypervisor some control over the host memory accesses. The
host stage 2 is created lazily using large block mappings if possible,
and will default to page mappings in absence of a better solution.
From this point on, memory accesses from the host to protected memory
regions (e.g. marked PROT_NONE) are fatal and lead to hyp_panic().
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_asm.h | 1 +
arch/arm64/include/asm/kvm_cpufeature.h | 2 +
arch/arm64/kernel/image-vars.h | 3 +
arch/arm64/kvm/arm.c | 10 +
arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 34 +++
arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
arch/arm64/kvm/hyp/nvhe/hyp-init.S | 1 +
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 11 +
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 213 ++++++++++++++++++
arch/arm64/kvm/hyp/nvhe/setup.c | 5 +
arch/arm64/kvm/hyp/nvhe/switch.c | 7 +-
arch/arm64/kvm/hyp/nvhe/tlb.c | 4 +-
12 files changed, 286 insertions(+), 7 deletions(-)
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/mem_protect.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/mem_protect.c
@@ -0,0 +1,213 @@+// SPDX-License-Identifier: GPL-2.0-only+/*+*Copyright(C)2020GoogleLLC+*Author:QuentinPerret<qperret@google.com>+*/++#include<linux/kvm_host.h>+#include<asm/kvm_cpufeature.h>+#include<asm/kvm_emulate.h>+#include<asm/kvm_hyp.h>+#include<asm/kvm_mmu.h>+#include<asm/kvm_pgtable.h>+#include<asm/stage2_pgtable.h>++#include<hyp/switch.h>++#include<nvhe/gfp.h>+#include<nvhe/memory.h>+#include<nvhe/mem_protect.h>+#include<nvhe/mm.h>++externunsignedlonghyp_nr_cpus;+structhost_kvmhost_kvm;++structhyp_poolhost_s2_mem;+structhyp_poolhost_s2_dev;++staticvoid*host_s2_zalloc_pages_exact(size_tsize)+{+returnhyp_alloc_pages(&host_s2_mem,get_order(size));+}++staticvoid*host_s2_zalloc_page(void*pool)+{+returnhyp_alloc_pages(pool,0);+}++staticintprepare_s2_pools(void*mem_pgt_pool,void*dev_pgt_pool)+{+unsignedlongnr_pages,pfn;+intret;++pfn=hyp_virt_to_pfn(mem_pgt_pool);+nr_pages=host_s2_mem_pgtable_pages();+ret=hyp_pool_init(&host_s2_mem,pfn,nr_pages,0);+if(ret)+returnret;++pfn=hyp_virt_to_pfn(dev_pgt_pool);+nr_pages=host_s2_dev_pgtable_pages();+ret=hyp_pool_init(&host_s2_dev,pfn,nr_pages,0);+if(ret)+returnret;++host_kvm.mm_ops.zalloc_pages_exact=host_s2_zalloc_pages_exact;+host_kvm.mm_ops.zalloc_page=host_s2_zalloc_page;+host_kvm.mm_ops.phys_to_virt=hyp_phys_to_virt;+host_kvm.mm_ops.virt_to_phys=hyp_virt_to_phys;+host_kvm.mm_ops.page_count=hyp_page_count;+host_kvm.mm_ops.get_page=hyp_get_page;+host_kvm.mm_ops.put_page=hyp_put_page;++return0;+}++staticvoidprepare_host_vtcr(void)+{+u32parange,phys_shift;+u64mmfr0,mmfr1;++mmfr0=arm64_ftr_reg_id_aa64mmfr0_el1.sys_val;+mmfr1=arm64_ftr_reg_id_aa64mmfr1_el1.sys_val;++/* The host stage 2 is id-mapped, so use parange for T0SZ */+parange=kvm_get_parange(mmfr0);+phys_shift=id_aa64mmfr0_parange_to_phys_shift(parange);++host_kvm.arch.vtcr=kvm_get_vtcr(mmfr0,mmfr1,phys_shift);+}++intkvm_host_prepare_stage2(void*mem_pgt_pool,void*dev_pgt_pool)+{+structkvm_s2_mmu*mmu=&host_kvm.arch.mmu;+intret;++prepare_host_vtcr();+hyp_spin_lock_init(&host_kvm.lock);++ret=prepare_s2_pools(mem_pgt_pool,dev_pgt_pool);+if(ret)+returnret;++ret=kvm_pgtable_stage2_init(&host_kvm.pgt,&host_kvm.arch,+&host_kvm.mm_ops);+if(ret)+returnret;++mmu->pgd_phys=__hyp_pa(host_kvm.pgt.pgd);+mmu->arch=&host_kvm.arch;+mmu->pgt=&host_kvm.pgt;+mmu->vmid.vmid_gen=0;+mmu->vmid.vmid=0;++return0;+}++int__pkvm_prot_finalize(void)+{+structkvm_s2_mmu*mmu=&host_kvm.arch.mmu;+structkvm_nvhe_init_params*params=this_cpu_ptr(&kvm_init_params);++params->vttbr=kvm_get_vttbr(mmu);+params->vtcr=host_kvm.arch.vtcr;+params->hcr_el2|=HCR_VM;+if(cpus_have_const_cap(ARM64_HAS_STAGE2_FWB))+params->hcr_el2|=HCR_FWB;+kvm_flush_dcache_to_poc(params,sizeof(*params));++write_sysreg(params->hcr_el2,hcr_el2);+__load_stage2(&host_kvm.arch.mmu,host_kvm.arch.vtcr);++__tlbi(vmalls12e1is);+dsb(ish);+isb();++return0;+}++staticvoidhost_stage2_unmap_dev_all(void)+{+structkvm_pgtable*pgt=&host_kvm.pgt;+structmemblock_region*reg;+u64addr=0;+inti;++/* Unmap all non-memory regions to recycle the pages */+for(i=0;i<hyp_memblock_nr;i++,addr=reg->base+reg->size){+reg=&hyp_memory[i];+kvm_pgtable_stage2_unmap(pgt,addr,reg->base-addr);+}+kvm_pgtable_stage2_unmap(pgt,addr,ULONG_MAX);+}++staticboolfind_mem_range(phys_addr_taddr,structkvm_mem_range*range)+{+intcur,left=0,right=hyp_memblock_nr;+structmemblock_region*reg;+phys_addr_tend;++range->start=0;+range->end=ULONG_MAX;++/* The list of memblock regions is sorted, binary search it */+while(left<right){+cur=(left+right)>>1;+reg=&hyp_memory[cur];+end=reg->base+reg->size;+if(addr<reg->base){+right=cur;+range->end=reg->base;+}elseif(addr>=end){+left=cur+1;+range->start=end;+}else{+range->start=reg->base;+range->end=end;+returntrue;+}+}++returnfalse;+}++staticinthost_stage2_idmap(u64addr)+{+enumkvm_pgtable_protprot=KVM_PGTABLE_PROT_R|KVM_PGTABLE_PROT_W;+structkvm_mem_rangerange;+boolis_memory=find_mem_range(addr,&range);+structhyp_pool*pool=is_memory?&host_s2_mem:&host_s2_dev;+intret;++if(is_memory)+prot|=KVM_PGTABLE_PROT_X;++hyp_spin_lock(&host_kvm.lock);+ret=kvm_pgtable_stage2_idmap_greedy(&host_kvm.pgt,addr,prot,+&range,pool);+if(is_memory||ret!=-ENOMEM)+gotounlock;+host_stage2_unmap_dev_all();+ret=kvm_pgtable_stage2_idmap_greedy(&host_kvm.pgt,addr,prot,+&range,pool);+unlock:+hyp_spin_unlock(&host_kvm.lock);++returnret;+}++voidhandle_host_mem_abort(structkvm_cpu_context*host_ctxt)+{+structkvm_vcpu_fault_infofault;+u64esr,addr;+intret=0;++esr=read_sysreg_el2(SYS_ESR);+if(!__get_fault_info(esr,&fault))+hyp_panic();++addr=(fault.hpfar_el2&HPFAR_MASK)<<8;+ret=host_stage2_idmap(addr);+if(ret&&ret!=-EAGAIN)+hyp_panic();+}
@@ -28,6 +28,8 @@#include<asm/processor.h>#include<asm/thread_info.h>+#include<nvhe/mem_protect.h>+/* Non-VHE specific context */DEFINE_PER_CPU(structkvm_host_data,kvm_host_data);DEFINE_PER_CPU(structkvm_cpu_context,kvm_hyp_ctxt);
@@ -102,11 +104,6 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu)write_sysreg(__kvm_hyp_host_vector,vbar_el2);}-staticvoid__load_host_stage2(void)-{-write_sysreg(0,vttbr_el2);-}-/* Save VGICv3 state on non-VHE systems */staticvoid__hyp_vgic_save_state(structkvm_vcpu*vcpu){
Once we start unmapping portions of memory from the host stage 2 (such
as e.g. the hypervisor memory sections, or pages that belong to
protected guests), we will need a way to track page ownership. And
given that all mappings in the host stage 2 will be identity-mapped, we
can use the host stage 2 page-table itself as a simplistic rmap.
As a first step towards this, introduce a new protection attribute
in the stage 2 page table code, called KVM_PGTABLE_PROT_NONE, which
allows to annotate portions of the IPA space as inaccessible. For
simplicity, PROT_NONE mappings are created as invalid mappings with a
software bit set.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_pgtable.h | 2 ++
arch/arm64/kvm/hyp/pgtable.c | 26 ++++++++++++++++++++++++--
2 files changed, 26 insertions(+), 2 deletions(-)
Refactor __populate_fault_info() to introduce __get_fault_info() which
will be used once the host is wrapped in a stage 2.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/hyp/include/hyp/switch.h | 37 ++++++++++++++-----------
1 file changed, 21 insertions(+), 16 deletions(-)
We will soon need to check if a Physical Address belongs to a memblock
at EL2, so make sure to sort them so this can be done efficiently.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/hyp/reserved_mem.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
@@ -158,6 +158,9 @@ struct kvm_nvhe_init_params {unsignedlongtpidr_el2;unsignedlongstack_hyp_va;phys_addr_tpgd_pa;+unsignedlonghcr_el2;+unsignedlongvttbr;+unsignedlongvtcr;};/* Translate a kernel address @ptr into its equivalent linear mapping */
In order to make use of the stage 2 pgtable code for the host stage 2,
use struct kvm_arch in lieu of struct kvm as the host will have the
former but not the latter.
Acked-by: Will Deacon <will@kernel.org>
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_pgtable.h | 5 +++--
arch/arm64/kvm/hyp/pgtable.c | 6 +++---
arch/arm64/kvm/mmu.c | 2 +-
3 files changed, 7 insertions(+), 6 deletions(-)
When memory protection is enabled, the EL2 code needs the ability to
create and manage its own page-table. To do so, introduce a new set of
hypercalls to bootstrap a memory management system at EL2.
This leads to the following boot flow in nVHE Protected mode:
1. the host allocates memory for the hypervisor very early on, using
the memblock API;
2. the host creates a set of stage 1 page-table for EL2, installs the
EL2 vectors, and issues the __pkvm_init hypercall;
3. during __pkvm_init, the hypervisor re-creates its stage 1 page-table
and stores it in the memory pool provided by the host;
4. the hypervisor then extends its stage 1 mappings to include a
vmemmap in the EL2 VA space, hence allowing to use the buddy
allocator introduced in a previous patch;
5. the hypervisor jumps back in the idmap page, switches from the
host-provided page-table to the new one, and wraps up its
initialization by enabling the new allocator, before returning to
the host.
6. the host can free the now unused page-table created for EL2, and
will now need to issue hypercalls to make changes to the EL2 stage 1
mappings instead of modifying them directly.
Note that for the sake of simplifying the review, this patch focuses on
the hypervisor side of things. In other words, this only implements the
new hypercalls, but does not make use of them from the host yet. The
host-side changes will follow in a subsequent patch.
Credits to Will for __pkvm_init_switch_pgd.
Co-authored-by: Will Deacon [off-list ref]
Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_asm.h | 4 +
arch/arm64/include/asm/kvm_host.h | 7 +
arch/arm64/include/asm/kvm_hyp.h | 8 ++
arch/arm64/include/asm/kvm_pgtable.h | 2 +
arch/arm64/kernel/image-vars.h | 16 +++
arch/arm64/kvm/hyp/Makefile | 2 +-
arch/arm64/kvm/hyp/include/nvhe/mm.h | 71 ++++++++++
arch/arm64/kvm/hyp/nvhe/Makefile | 4 +-
arch/arm64/kvm/hyp/nvhe/hyp-init.S | 31 +++++
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 49 +++++++
arch/arm64/kvm/hyp/nvhe/mm.c | 173 ++++++++++++++++++++++++
arch/arm64/kvm/hyp/nvhe/setup.c | 195 +++++++++++++++++++++++++++
arch/arm64/kvm/hyp/pgtable.c | 2 -
arch/arm64/kvm/hyp/reserved_mem.c | 92 +++++++++++++
arch/arm64/mm/init.c | 3 +
15 files changed, 654 insertions(+), 5 deletions(-)
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/mm.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/mm.c
create mode 100644 arch/arm64/kvm/hyp/nvhe/setup.c
create mode 100644 arch/arm64/kvm/hyp/reserved_mem.c
@@ -0,0 +1,173 @@+// SPDX-License-Identifier: GPL-2.0-only+/*+*Copyright(C)2020GoogleLLC+*Author:QuentinPerret<qperret@google.com>+*/++#include<linux/kvm_host.h>+#include<asm/kvm_hyp.h>+#include<asm/kvm_mmu.h>+#include<asm/kvm_pgtable.h>+#include<asm/spectre.h>++#include<nvhe/early_alloc.h>+#include<nvhe/gfp.h>+#include<nvhe/memory.h>+#include<nvhe/mm.h>+#include<nvhe/spinlock.h>++structkvm_pgtablepkvm_pgtable;+hyp_spinlock_tpkvm_pgd_lock;+u64__io_map_base;++structmemblock_regionhyp_memory[HYP_MEMBLOCK_REGIONS];+unsignedinthyp_memblock_nr;++int__pkvm_create_mappings(unsignedlongstart,unsignedlongsize,+unsignedlongphys,enumkvm_pgtable_protprot)+{+interr;++hyp_spin_lock(&pkvm_pgd_lock);+err=kvm_pgtable_hyp_map(&pkvm_pgtable,start,size,phys,prot);+hyp_spin_unlock(&pkvm_pgd_lock);++returnerr;+}++unsignedlong__pkvm_create_private_mapping(phys_addr_tphys,size_tsize,+enumkvm_pgtable_protprot)+{+unsignedlongaddr;+interr;++hyp_spin_lock(&pkvm_pgd_lock);++size=PAGE_ALIGN(size+offset_in_page(phys));+addr=__io_map_base;+__io_map_base+=size;++/* Are we overflowing on the vmemmap ? */+if(__io_map_base>__hyp_vmemmap){+__io_map_base-=size;+addr=(unsignedlong)ERR_PTR(-ENOMEM);+gotoout;+}++err=kvm_pgtable_hyp_map(&pkvm_pgtable,addr,size,phys,prot);+if(err){+addr=(unsignedlong)ERR_PTR(err);+gotoout;+}++addr=addr+offset_in_page(phys);+out:+hyp_spin_unlock(&pkvm_pgd_lock);++returnaddr;+}++intpkvm_create_mappings(void*from,void*to,enumkvm_pgtable_protprot)+{+unsignedlongstart=(unsignedlong)from;+unsignedlongend=(unsignedlong)to;+unsignedlongvirt_addr;+phys_addr_tphys;++start=start&PAGE_MASK;+end=PAGE_ALIGN(end);++for(virt_addr=start;virt_addr<end;virt_addr+=PAGE_SIZE){+interr;++phys=hyp_virt_to_phys((void*)virt_addr);+err=__pkvm_create_mappings(virt_addr,PAGE_SIZE,phys,prot);+if(err)+returnerr;+}++return0;+}++inthyp_back_vmemmap(phys_addr_tphys,unsignedlongsize,phys_addr_tback)+{+unsignedlongstart,end;++hyp_vmemmap_range(phys,size,&start,&end);++return__pkvm_create_mappings(start,end-start,back,PAGE_HYP);+}++staticvoid*__hyp_bp_vect_base;+intpkvm_cpu_set_vector(enumarm64_hyp_spectre_vectorslot)+{+void*vector;++switch(slot){+caseHYP_VECTOR_DIRECT:{+vector=__kvm_hyp_vector;+break;+}+caseHYP_VECTOR_SPECTRE_DIRECT:{+vector=__bp_harden_hyp_vecs;+break;+}+caseHYP_VECTOR_INDIRECT:+caseHYP_VECTOR_SPECTRE_INDIRECT:{+vector=(void*)__hyp_bp_vect_base;+break;+}+default:+return-EINVAL;+}++vector=__kvm_vector_slot2addr(vector,slot);+*this_cpu_ptr(&kvm_hyp_vector)=(unsignedlong)vector;++return0;+}++inthyp_map_vectors(void)+{+phys_addr_tphys;+void*bp_base;++if(!cpus_have_const_cap(ARM64_SPECTRE_V3A))+return0;++phys=__hyp_pa(__bp_harden_hyp_vecs);+bp_base=(void*)__pkvm_create_private_mapping(phys,+__BP_HARDEN_HYP_VECS_SZ,+PAGE_HYP_EXEC);+if(IS_ERR_OR_NULL(bp_base))+returnPTR_ERR(bp_base);++__hyp_bp_vect_base=bp_base;++return0;+}++inthyp_create_idmap(u32hyp_va_bits)+{+unsignedlongstart,end;++start=hyp_virt_to_phys((void*)__hyp_idmap_text_start);+start=ALIGN_DOWN(start,PAGE_SIZE);++end=hyp_virt_to_phys((void*)__hyp_idmap_text_end);+end=ALIGN(end,PAGE_SIZE);++/*+*OnehalfoftheVAspaceisreservedtolinearlymapportionsof+*memory--seeva_layout.cformoredetails.TheotherhalfoftheVA+*spacecontainsthetrampolinepage,andneedssomecare.Splitthat+*secondhalfintwoandfindthequarterofVAspacenotconflicting+*withtheidmaptoplacetheIOsandthevmemmap.IOsusethelower+*halfofthequarterandthevmemmaptheupperhalf.+*/+__io_map_base=start&BIT(hyp_va_bits-2);+__io_map_base^=BIT(hyp_va_bits-2);+__hyp_vmemmap=__io_map_base|BIT(hyp_va_bits-3);++return__pkvm_create_mappings(start,end-start,start,PAGE_HYP_EXEC);+}
@@ -0,0 +1,195 @@+// SPDX-License-Identifier: GPL-2.0-only+/*+*Copyright(C)2020GoogleLLC+*Author:QuentinPerret<qperret@google.com>+*/++#include<linux/kvm_host.h>+#include<asm/kvm_hyp.h>+#include<asm/kvm_mmu.h>+#include<asm/kvm_pgtable.h>++#include<nvhe/early_alloc.h>+#include<nvhe/gfp.h>+#include<nvhe/memory.h>+#include<nvhe/mm.h>+#include<nvhe/trap_handler.h>++structhyp_poolhpool;+structkvm_pgtable_mm_opspkvm_pgtable_mm_ops;+unsignedlonghyp_nr_cpus;++#define hyp_percpu_size ((unsigned long)__per_cpu_end - \+(unsignedlong)__per_cpu_start)++staticvoid*vmemmap_base;+staticvoid*hyp_pgt_base;++staticintdivide_memory_pool(void*virt,unsignedlongsize)+{+unsignedlongvstart,vend,nr_pages;++hyp_early_alloc_init(virt,size);++hyp_vmemmap_range(__hyp_pa(virt),size,&vstart,&vend);+nr_pages=(vend-vstart)>>PAGE_SHIFT;+vmemmap_base=hyp_early_alloc_contig(nr_pages);+if(!vmemmap_base)+return-ENOMEM;++nr_pages=hyp_s1_pgtable_pages();+hyp_pgt_base=hyp_early_alloc_contig(nr_pages);+if(!hyp_pgt_base)+return-ENOMEM;++return0;+}++staticintrecreate_hyp_mappings(phys_addr_tphys,unsignedlongsize,+unsignedlong*per_cpu_base,+u32hyp_va_bits)+{+void*start,*end,*virt=hyp_phys_to_virt(phys);+unsignedlongpgt_size=hyp_s1_pgtable_pages()<<PAGE_SHIFT;+intret,i;++/* Recreate the hyp page-table using the early page allocator */+hyp_early_alloc_init(hyp_pgt_base,pgt_size);+ret=kvm_pgtable_hyp_init(&pkvm_pgtable,hyp_va_bits,+&hyp_early_alloc_mm_ops);+if(ret)+returnret;++ret=hyp_create_idmap(hyp_va_bits);+if(ret)+returnret;++ret=hyp_map_vectors();+if(ret)+returnret;++ret=hyp_back_vmemmap(phys,size,hyp_virt_to_phys(vmemmap_base));+if(ret)+returnret;++ret=pkvm_create_mappings(__hyp_text_start,__hyp_text_end,PAGE_HYP_EXEC);+if(ret)+returnret;++ret=pkvm_create_mappings(__start_rodata,__end_rodata,PAGE_HYP_RO);+if(ret)+returnret;++ret=pkvm_create_mappings(__hyp_rodata_start,__hyp_rodata_end,PAGE_HYP_RO);+if(ret)+returnret;++ret=pkvm_create_mappings(__hyp_bss_start,__hyp_bss_end,PAGE_HYP);+if(ret)+returnret;++ret=pkvm_create_mappings(__hyp_bss_end,__bss_stop,PAGE_HYP_RO);+if(ret)+returnret;++ret=pkvm_create_mappings(virt,virt+size,PAGE_HYP);+if(ret)+returnret;++for(i=0;i<hyp_nr_cpus;i++){+start=(void*)kern_hyp_va(per_cpu_base[i]);+end=start+PAGE_ALIGN(hyp_percpu_size);+ret=pkvm_create_mappings(start,end,PAGE_HYP);+if(ret)+returnret;++end=(void*)per_cpu_ptr(&kvm_init_params,i)->stack_hyp_va;+start=end-PAGE_SIZE;+ret=pkvm_create_mappings(start,end,PAGE_HYP);+if(ret)+returnret;+}++return0;+}++staticvoidupdate_nvhe_init_params(void)+{+structkvm_nvhe_init_params*params;+unsignedlongi;++for(i=0;i<hyp_nr_cpus;i++){+params=per_cpu_ptr(&kvm_init_params,i);+params->pgd_pa=__hyp_pa(pkvm_pgtable.pgd);+__flush_dcache_area(params,sizeof(*params));+}+}++staticvoid*hyp_zalloc_hyp_page(void*arg)+{+returnhyp_alloc_pages(&hpool,0);+}++void__noreturn__pkvm_init_finalise(void)+{+structkvm_host_data*host_data=this_cpu_ptr(&kvm_host_data);+structkvm_cpu_context*host_ctxt=&host_data->host_ctxt;+unsignedlongnr_pages,reserved_pages,pfn;+intret;++/* Now that the vmemmap is backed, install the full-fledged allocator */+pfn=hyp_virt_to_pfn(hyp_pgt_base);+nr_pages=hyp_s1_pgtable_pages();+reserved_pages=hyp_early_alloc_nr_used_pages();+ret=hyp_pool_init(&hpool,pfn,nr_pages,reserved_pages);+if(ret)+gotoout;++pkvm_pgtable_mm_ops.zalloc_page=hyp_zalloc_hyp_page;+pkvm_pgtable_mm_ops.phys_to_virt=hyp_phys_to_virt;+pkvm_pgtable_mm_ops.virt_to_phys=hyp_virt_to_phys;+pkvm_pgtable_mm_ops.get_page=hyp_get_page;+pkvm_pgtable_mm_ops.put_page=hyp_put_page;+pkvm_pgtable.mm_ops=&pkvm_pgtable_mm_ops;++out:+/*+*Wetail-calledtoherefromhandle___pkvm_init()andwillnotreturn,+*somakesuretopropagatethereturnvaluetothehost.+*/+cpu_reg(host_ctxt,1)=ret;++__host_enter(host_ctxt);+}++int__pkvm_init(phys_addr_tphys,unsignedlongsize,unsignedlongnr_cpus,+unsignedlong*per_cpu_base,u32hyp_va_bits)+{+structkvm_nvhe_init_params*params;+void*virt=hyp_phys_to_virt(phys);+void(*fn)(phys_addr_tparams_pa,void*finalize_fn_va);+intret;++if(phys%PAGE_SIZE||size%PAGE_SIZE)+return-EINVAL;++hyp_spin_lock_init(&pkvm_pgd_lock);+hyp_nr_cpus=nr_cpus;++ret=divide_memory_pool(virt,size);+if(ret)+returnret;++ret=recreate_hyp_mappings(phys,size,per_cpu_base,hyp_va_bits);+if(ret)+returnret;++update_nvhe_init_params();++/* Jump in the idmap page to switch to the new page-tables */+params=this_cpu_ptr(&kvm_init_params);+fn=(typeof(fn))__hyp_pa(__pkvm_init_switch_pgd);+fn(__hyp_pa(params),__pkvm_init_finalise);++unreachable();+}
Previous commits have introduced infrastructure to enable the EL2 code
to manage its own stage 1 mappings. However, this was preliminary work,
and none of it is currently in use.
Put all of this together by elevating the mapping creation at EL2 when
memory protection is enabled. In this case, the host kernel running
at EL1 still creates _temporary_ EL2 mappings, only used while
initializing the hypervisor, but frees them right after.
As such, all calls to create_hyp_mappings() after kvm init has finished
turn into hypercalls, as the host now has no 'legal' way to modify the
hypevisor page tables directly.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_mmu.h | 2 +-
arch/arm64/kvm/arm.c | 87 +++++++++++++++++++++++++++++---
arch/arm64/kvm/mmu.c | 43 ++++++++++++++--
3 files changed, 120 insertions(+), 12 deletions(-)
Introduce the infrastructure in KVM enabling to copy CPU feature
registers into EL2-owned data-structures, to allow reading sanitised
values directly at EL2 in nVHE.
Given that only a subset of these features are being read by the
hypervisor, the ones that need to be copied are to be listed under
<asm/kvm_cpufeature.h> together with the name of the nVHE variable that
will hold the copy.
While at it, introduce the first user of this infrastructure by
implementing __flush_dcache_area at EL2, which needs
arm64_ftr_reg_ctrel0.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/cpufeature.h | 1 +
arch/arm64/include/asm/kvm_cpufeature.h | 17 +++++++++++++++++
arch/arm64/include/asm/kvm_host.h | 4 ++++
arch/arm64/kernel/cpufeature.c | 13 +++++++++++++
arch/arm64/kvm/hyp/nvhe/Makefile | 3 ++-
arch/arm64/kvm/hyp/nvhe/cache.S | 13 +++++++++++++
arch/arm64/kvm/hyp/nvhe/cpufeature.c | 8 ++++++++
arch/arm64/kvm/sys_regs.c | 21 +++++++++++++++++++++
8 files changed, 79 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/include/asm/kvm_cpufeature.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/cache.S
create mode 100644 arch/arm64/kvm/hyp/nvhe/cpufeature.c
The host currently writes directly in EL2 per-CPU data sections from
the PMU code when running in nVHE. In preparation for unmapping the EL2
sections from the host stage 2, disable PMU support in protected mode as
we currently do not have a use-case for it.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/perf.c | 3 ++-
arch/arm64/kvm/pmu.c | 8 ++++----
2 files changed, 6 insertions(+), 5 deletions(-)
In order to ease its re-use in other code paths, refactor
stage2_map_set_prot_attr() to not depend on a stage2_map_data struct.
No functional change intended.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/hyp/pgtable.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
We will soon unmap the .hyp sections from the host stage 2 in Protected
nVHE mode, which obvisouly works with at least page granularity, so make
sure to align them correctly.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kernel/vmlinux.lds.S | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
@@ -322,11 +317,12 @@ SECTIONS#include "image-vars.h"/*-*TheHYPinitcodeandIDmaptextcan't be longer than a page each,-*andshouldnotcrossapageboundary.+*TheHYPinitcodeandIDmaptextcan't be longer than a page each. The+*formerispage-aligned,butthelattermaynotbewith16Kor64Kpages,so+*itshouldalsonotcrossapageboundary.*/-ASSERT(__hyp_idmap_text_end-(__hyp_idmap_text_start&~(SZ_4K-1))<=SZ_4K,-"HYP init code too big or misaligned")+ASSERT(__hyp_idmap_text_end-__hyp_idmap_text_start<=PAGE_SIZE,+"HYP init code too big")ASSERT(__idmap_text_end-(__idmap_text_start&~(SZ_4K-1))<=SZ_4K,"ID map text too big or misaligned")#ifdef CONFIG_HIBERNATION
--
2.30.1.766.gb4fecdf3b7-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Currently, the hyp code cannot make full use of a bss, as the kernel
section is mapped read-only.
While this mapping could simply be changed to read-write, it would
intermingle even more the hyp and kernel state than they currently are.
Instead, introduce a __hyp_bss section, that uses reserved pages, and
create the appropriate RW hyp mappings during KVM init.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/sections.h | 1 +
arch/arm64/kernel/vmlinux.lds.S | 52 ++++++++++++++++++++-----------
arch/arm64/kvm/arm.c | 14 ++++++++-
arch/arm64/kvm/hyp/nvhe/hyp.lds.S | 1 +
4 files changed, 49 insertions(+), 19 deletions(-)
Move the initialization of kvm_nvhe_init_params in a dedicated function
that is run early, and only once during KVM init, rather than every time
the KVM vectors are set and reset.
This also opens the opportunity for the hypervisor to change the init
structs during boot, hence simplifying the replacement of host-provided
page-table by the one the hypervisor will create for itself.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/arm.c | 30 ++++++++++++++++++------------
1 file changed, 18 insertions(+), 12 deletions(-)
@@ -1383,22 +1383,18 @@ static int kvm_init_vector_slots(void)return0;}-staticvoidcpu_init_hyp_mode(void)+staticvoidcpu_prepare_hyp_mode(intcpu){-structkvm_nvhe_init_params*params=this_cpu_ptr_nvhe_sym(kvm_init_params);-structarm_smccc_resres;+structkvm_nvhe_init_params*params=per_cpu_ptr_nvhe_sym(kvm_init_params,cpu);unsignedlongtcr;-/* Switch from the HYP stub to our own HYP init vector */-__hyp_set_vectors(kvm_get_idmap_vector());-/**Calculatetherawper-cpuoffsetwithoutatranslationfromthe*kernel'smappingtothelinearmapping,andstoreitintpidr_el2*sothatwecanuseadr_ltoaccessper-cpuvariablesinEL2.*AlsodroptheKASANtagwhichgetsintheway...*/-params->tpidr_el2=(unsignedlong)kasan_reset_tag(this_cpu_ptr_nvhe_sym(__per_cpu_start))-+params->tpidr_el2=(unsignedlong)kasan_reset_tag(per_cpu_ptr_nvhe_sym(__per_cpu_start,cpu))-(unsignedlong)kvm_ksym_ref(CHOOSE_NVHE_SYM(__per_cpu_start));params->mair_el2=read_sysreg(mair_el1);
@@ -1430,6 +1426,15 @@ static void cpu_init_hyp_mode(void)*bereadwhiletheMMUisoff.*/kvm_flush_dcache_to_poc(params,sizeof(*params));+}++staticvoidcpu_init_hyp_mode(void)+{+structkvm_nvhe_init_params*params;+structarm_smccc_resres;++/* Switch from the HYP stub to our own HYP init vector */+__hyp_set_vectors(kvm_get_idmap_vector());/**Callinitializationcode,andswitchtothefullblownHYPcode.
From: Will Deacon <will@kernel.org>
Pull clear_page(), copy_page(), memcpy() and memset() into the nVHE hyp
code and ensure that we always execute the '__pi_' entry point on the
offchance that it changes in future.
[ qperret: Commit title nits and added linker script alias ]
Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/hyp_image.h | 3 +++
arch/arm64/kernel/image-vars.h | 11 +++++++++++
arch/arm64/kvm/hyp/nvhe/Makefile | 4 ++++
3 files changed, 18 insertions(+)
kvm_call_hyp() has some logic to issue a function call or a hypercall
depending on the EL at which the kernel is running. However, all the
code compiled under __KVM_NVHE_HYPERVISOR__ is guaranteed to only run
at EL2 which allows us to simplify.
Add ifdefery to kvm_host.h to simplify kvm_call_hyp() in .hyp.text.
Acked-by: Will Deacon <will@kernel.org>
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_host.h | 6 ++++++
1 file changed, 6 insertions(+)
Currently, the KVM page-table allocator uses a mix of put_page() and
free_page() calls depending on the context even though page-allocation
is always achieved using variants of __get_free_page().
Make the code consistent by using put_page() throughout, and reduce the
memory management API surface used by the page-table code. This will
ease factoring out page-allocation from pgtable.c, which is a
pre-requisite to creating page-tables at EL2.
Acked-by: Will Deacon <will@kernel.org>
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/hyp/pgtable.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
From: Will Deacon <will@kernel.org>
clear_page() and copy_page() are suitable for use outside of the kernel
address space, so annotate them as position-independent code.
Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/lib/clear_page.S | 4 ++--
arch/arm64/lib/copy_page.S | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
In preparation for enabling the creation of page-tables at EL2, factor
all memory allocation out of the page-table code, hence making it
re-usable with any compatible memory allocator.
No functional changes intended.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_pgtable.h | 41 +++++++++++-
arch/arm64/kvm/hyp/pgtable.c | 98 +++++++++++++++++-----------
arch/arm64/kvm/mmu.c | 66 ++++++++++++++++++-
3 files changed, 163 insertions(+), 42 deletions(-)
@@ -88,6 +88,44 @@ static bool kvm_is_device_pfn(unsigned long pfn)return!pfn_valid(pfn);}+staticvoid*stage2_memcache_zalloc_page(void*arg)+{+structkvm_mmu_memory_cache*mc=arg;++/* Allocated with __GFP_ZERO, so no need to zero */+returnkvm_mmu_memory_cache_alloc(mc);+}++staticvoid*kvm_host_zalloc_pages_exact(size_tsize)+{+returnalloc_pages_exact(size,GFP_KERNEL_ACCOUNT|__GFP_ZERO);+}++staticvoidkvm_host_get_page(void*addr)+{+get_page(virt_to_page(addr));+}++staticvoidkvm_host_put_page(void*addr)+{+put_page(virt_to_page(addr));+}++staticintkvm_host_page_count(void*addr)+{+returnpage_count(virt_to_page(addr));+}++staticphys_addr_tkvm_host_pa(void*addr)+{+return__pa(addr);+}++staticvoid*kvm_host_va(phys_addr_tphys)+{+return__va(phys);+}+/**Unmappingvsdcachemanagement:*
@@ -351,6 +389,17 @@ int create_hyp_exec_mappings(phys_addr_t phys_addr, size_t size,return0;}+staticstructkvm_pgtable_mm_opskvm_s2_mm_ops={+.zalloc_page=stage2_memcache_zalloc_page,+.zalloc_pages_exact=kvm_host_zalloc_pages_exact,+.free_pages_exact=free_pages_exact,+.get_page=kvm_host_get_page,+.put_page=kvm_host_put_page,+.page_count=kvm_host_page_count,+.phys_to_virt=kvm_host_va,+.virt_to_phys=kvm_host_pa,+};+/***kvm_init_stage2_mmu-InitialiseaS2MMUstrucrure*@kvm:ThepointertotheKVMstructure
In order to allow the usage of code shared by the host and the hyp in
static inline library functions, allow the usage of kvm_nvhe_sym() at
EL2 by defaulting to the raw symbol name.
Acked-by: Will Deacon <will@kernel.org>
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/hyp_image.h | 4 ++++
1 file changed, 4 insertions(+)
From: Will Deacon <will@kernel.org>
We will soon need to synchronise multiple CPUs in the hyp text at EL2.
The qspinlock-based locking used by the host is overkill for this purpose
and relies on the kernel's "percpu" implementation for the MCS nodes.
Implement a simple ticket locking scheme based heavily on the code removed
by commit c11090474d70 ("arm64: locking: Replace ticket lock implementation
with qspinlock").
Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/hyp/include/nvhe/spinlock.h | 92 ++++++++++++++++++++++
1 file changed, 92 insertions(+)
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/spinlock.h
From: Will Deacon <will@kernel.org> Date: 2021-03-04 13:40:50
On Tue, Mar 02, 2021 at 02:59:34PM +0000, Quentin Perret wrote:
Move the initialization of kvm_nvhe_init_params in a dedicated function
that is run early, and only once during KVM init, rather than every time
the KVM vectors are set and reset.
This also opens the opportunity for the hypervisor to change the init
structs during boot, hence simplifying the replacement of host-provided
page-table by the one the hypervisor will create for itself.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/arm.c | 30 ++++++++++++++++++------------
1 file changed, 18 insertions(+), 12 deletions(-)
From: Will Deacon <will@kernel.org> Date: 2021-03-04 14:08:02
On Tue, Mar 02, 2021 at 02:59:36PM +0000, Quentin Perret wrote:
In preparation for enabling the creation of page-tables at EL2, factor
all memory allocation out of the page-table code, hence making it
re-usable with any compatible memory allocator.
No functional changes intended.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_pgtable.h | 41 +++++++++++-
arch/arm64/kvm/hyp/pgtable.c | 98 +++++++++++++++++-----------
arch/arm64/kvm/mmu.c | 66 ++++++++++++++++++-
3 files changed, 163 insertions(+), 42 deletions(-)
It's not clear to me whether "it is automatically rounded to PAGE_SIZE"
means that the caller or the callee does the rounding. If it's the caller,
maybe it would be better to pass the number of pages as an 'npages' argument
instead of the size in bytes?
+ * @free_pages_exact: Free an exact number of memory pages, to free memory
+ * allocated with zalloc_pages_exact.
"Free an exact number of memory pages previously allocated by
zalloc_pages_exact()"
+ * @get_page: Increment the refcount on a page.
+ * @put_page: Decrement the refcount on a page. When the refcount
+ * reaches 0 the page is automatically freed.
+ * @page_count: Return the refcount of a page.
+ * @phys_to_virt: Convert a physical address into a virtual address as
+ * accessible in the current context.
s/as accessible/mapped/
With those changes:
Acked-by: Will Deacon <will@kernel.org>
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-03-04 14:10:59
On Tue, Mar 02, 2021 at 02:59:37PM +0000, Quentin Perret wrote:
Currently, the hyp code cannot make full use of a bss, as the kernel
section is mapped read-only.
While this mapping could simply be changed to read-write, it would
intermingle even more the hyp and kernel state than they currently are.
Instead, introduce a __hyp_bss section, that uses reserved pages, and
create the appropriate RW hyp mappings during KVM init.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/sections.h | 1 +
arch/arm64/kernel/vmlinux.lds.S | 52 ++++++++++++++++++++-----------
arch/arm64/kvm/arm.c | 14 ++++++++-
arch/arm64/kvm/hyp/nvhe/hyp.lds.S | 1 +
4 files changed, 49 insertions(+), 19 deletions(-)
From: Will Deacon <will@kernel.org> Date: 2021-03-04 14:39:48
On Tue, Mar 02, 2021 at 02:59:40PM +0000, Quentin Perret wrote:
With nVHE, the host currently creates all stage 1 hypervisor mappings at
EL1 during boot, installs them at EL2, and extends them as required
(e.g. when creating a new VM). But in a world where the host is no
longer trusted, it cannot have full control over the code mapped in the
hypervisor.
In preparation for enabling the hypervisor to create its own stage 1
mappings during boot, introduce an early page allocator, with minimal
functionality. This allocator is designed to be used only during early
bootstrap of the hyp code when memory protection is enabled, which will
then switch to using a full-fledged page allocator after init.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/hyp/include/nvhe/early_alloc.h | 14 +++++
arch/arm64/kvm/hyp/include/nvhe/memory.h | 24 +++++++++
arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
arch/arm64/kvm/hyp/nvhe/early_alloc.c | 54 +++++++++++++++++++
arch/arm64/kvm/hyp/nvhe/psci-relay.c | 4 +-
5 files changed, 94 insertions(+), 4 deletions(-)
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/early_alloc.h
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/memory.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/early_alloc.c
From: Will Deacon <will@kernel.org> Date: 2021-03-04 15:32:04
On Tue, Mar 02, 2021 at 02:59:42PM +0000, Quentin Perret wrote:
When memory protection is enabled, the hyp code will require a basic
form of memory management in order to allocate and free memory pages at
EL2. This is needed for various use-cases, including the creation of hyp
mappings or the allocation of stage 2 page tables.
To address these use-case, introduce a simple memory allocator in the
hyp code. The allocator is designed as a conventional 'buddy allocator',
working with a page granularity. It allows to allocate and free
physically contiguous pages from memory 'pools', with a guaranteed order
alignment in the PA space. Each page in a memory pool is associated
with a struct hyp_page which holds the page's metadata, including its
refcount, as well as its current order, hence mimicking the kernel's
buddy system in the GFP infrastructure. The hyp_page metadata are made
accessible through a hyp_vmemmap, following the concept of
SPARSE_VMEMMAP in the kernel.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/hyp/include/nvhe/gfp.h | 55 +++++++
arch/arm64/kvm/hyp/include/nvhe/memory.h | 28 ++++
arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
arch/arm64/kvm/hyp/nvhe/page_alloc.c | 195 +++++++++++++++++++++++
4 files changed, 279 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/gfp.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/page_alloc.c
[...]
+static void __hyp_attach_page(struct hyp_pool *pool,
+ struct hyp_page *p)
+{
+ unsigned int order = p->order;
+ struct hyp_page *buddy;
+
+ memset(hyp_page_to_virt(p), 0, PAGE_SIZE << p->order);
+
+ /*
+ * Only the first struct hyp_page of a high-order page (otherwise known
+ * as the 'head') should have p->order set. The non-head pages should
+ * have p->order = HYP_NO_ORDER. Here @p may no longer be the head
+ * after coallescing, so make sure to mark it HYP_NO_ORDER proactively.
+ */
+ p->order = HYP_NO_ORDER;
+ for (; (order + 1) < pool->max_order; order++) {
+ buddy = __find_buddy_avail(pool, p, order);
+ if (!buddy)
+ break;
+
+ /* Take the buddy out of its list, and coallesce with @p */
+ list_del_init(&buddy->node);
+ buddy->order = HYP_NO_ORDER;
+ p = (p < buddy) ? p : buddy;
nit: this is min()
+ }
+
+ /* Mark the new head, and insert it */
+ p->order = order;
+ list_add_tail(&p->node, &pool->free_area[order]);
+}
+
+static void hyp_attach_page(struct hyp_page *p)
+{
+ struct hyp_pool *pool = hyp_page_to_pool(p);
+
+ hyp_spin_lock(&pool->lock);
+ __hyp_attach_page(pool, p);
+ hyp_spin_unlock(&pool->lock);
+}
+
+static struct hyp_page *__hyp_extract_page(struct hyp_pool *pool,
+ struct hyp_page *p,
+ unsigned int order)
+{
+ struct hyp_page *buddy;
+
+ list_del_init(&p->node);
+ while (p->order > order) {
+ /*
+ * The buddy of order n - 1 currently has HYP_NO_ORDER as it
+ * is covered by a higher-level page (whose head is @p). Use
+ * __find_buddy_nocheck() to find it and inject it in the
+ * free_list[n - 1], effectively splitting @p in half.
+ */
+ p->order--;
+ buddy = __find_buddy_nocheck(pool, p, p->order);
+ buddy->order = p->order;
+ list_add_tail(&buddy->node, &pool->free_area[buddy->order]);
+ }
+
+ return p;
+}
+
+void hyp_put_page(void *addr)
+{
+ struct hyp_page *p = hyp_virt_to_page(addr);
+
+ if (hyp_page_ref_dec_and_test(p))
+ hyp_attach_page(p);
+}
+
+void hyp_get_page(void *addr)
+{
+ struct hyp_page *p = hyp_virt_to_page(addr);
+
+ hyp_page_ref_inc(p);
+}
+
+void *hyp_alloc_pages(struct hyp_pool *pool, unsigned int order)
+{
+ unsigned int i = order;
+ struct hyp_page *p;
+
+ hyp_spin_lock(&pool->lock);
+
+ /* Look for a high-enough-order page */
+ while (i < pool->max_order && list_empty(&pool->free_area[i]))
+ i++;
+ if (i >= pool->max_order) {
+ hyp_spin_unlock(&pool->lock);
+ return NULL;
+ }
+
+ /* Extract it from the tree at the right order */
+ p = list_first_entry(&pool->free_area[i], struct hyp_page, node);
+ p = __hyp_extract_page(pool, p, order);
+
+ hyp_spin_unlock(&pool->lock);
+ hyp_page_ref_inc(p);
I find this a little scary, as we momentarily drop the lock. It think
it's ok because the reference count on the page must be 0 at this point,
but actually then I think it would be clearer to have a
hyp_page_ref_init() function which could take the lock, check that the
refcount is indeed 0 and then set it to 1.
What do you think?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thursday 04 Mar 2021 at 15:30:36 (+0000), Will Deacon wrote:
On Tue, Mar 02, 2021 at 02:59:42PM +0000, Quentin Perret wrote:
quoted
+void *hyp_alloc_pages(struct hyp_pool *pool, unsigned int order)
+{
+ unsigned int i = order;
+ struct hyp_page *p;
+
+ hyp_spin_lock(&pool->lock);
+
+ /* Look for a high-enough-order page */
+ while (i < pool->max_order && list_empty(&pool->free_area[i]))
+ i++;
+ if (i >= pool->max_order) {
+ hyp_spin_unlock(&pool->lock);
+ return NULL;
+ }
+
+ /* Extract it from the tree at the right order */
+ p = list_first_entry(&pool->free_area[i], struct hyp_page, node);
+ p = __hyp_extract_page(pool, p, order);
+
+ hyp_spin_unlock(&pool->lock);
+ hyp_page_ref_inc(p);
I find this a little scary, as we momentarily drop the lock. It think
it's ok because the reference count on the page must be 0 at this point,
Yep, @p shouldn't be visible to the caller yet so this should be fine.
but actually then I think it would be clearer to have a
hyp_page_ref_init() function which could take the lock, check that the
refcount is indeed 0 and then set it to 1.
Works for me. Maybe I'll use another name for the API to stay consistent
with the kernel gfp code (hyp_page_ref_inc() and friends are inspired
from their kernel counterpart). And I guess I can hyp_panic() if the
refcount is not 0 at this point to match the VM_BUG_ON_PAGE() in
set_page_refcounted().
Thanks!
Quentin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-03-04 18:48:52
Hi Quentin,
On Tue, Mar 02, 2021 at 02:59:45PM +0000, Quentin Perret wrote:
When memory protection is enabled, the EL2 code needs the ability to
create and manage its own page-table. To do so, introduce a new set of
hypercalls to bootstrap a memory management system at EL2.
This leads to the following boot flow in nVHE Protected mode:
1. the host allocates memory for the hypervisor very early on, using
the memblock API;
2. the host creates a set of stage 1 page-table for EL2, installs the
EL2 vectors, and issues the __pkvm_init hypercall;
3. during __pkvm_init, the hypervisor re-creates its stage 1 page-table
and stores it in the memory pool provided by the host;
4. the hypervisor then extends its stage 1 mappings to include a
vmemmap in the EL2 VA space, hence allowing to use the buddy
allocator introduced in a previous patch;
5. the hypervisor jumps back in the idmap page, switches from the
host-provided page-table to the new one, and wraps up its
initialization by enabling the new allocator, before returning to
the host.
6. the host can free the now unused page-table created for EL2, and
will now need to issue hypercalls to make changes to the EL2 stage 1
mappings instead of modifying them directly.
Note that for the sake of simplifying the review, this patch focuses on
the hypervisor side of things. In other words, this only implements the
new hypercalls, but does not make use of them from the host yet. The
host-side changes will follow in a subsequent patch.
Credits to Will for __pkvm_init_switch_pgd.
Co-authored-by: Will Deacon [off-list ref]
Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_asm.h | 4 +
arch/arm64/include/asm/kvm_host.h | 7 +
arch/arm64/include/asm/kvm_hyp.h | 8 ++
arch/arm64/include/asm/kvm_pgtable.h | 2 +
arch/arm64/kernel/image-vars.h | 16 +++
arch/arm64/kvm/hyp/Makefile | 2 +-
arch/arm64/kvm/hyp/include/nvhe/mm.h | 71 ++++++++++
arch/arm64/kvm/hyp/nvhe/Makefile | 4 +-
arch/arm64/kvm/hyp/nvhe/hyp-init.S | 31 +++++
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 49 +++++++
arch/arm64/kvm/hyp/nvhe/mm.c | 173 ++++++++++++++++++++++++
arch/arm64/kvm/hyp/nvhe/setup.c | 195 +++++++++++++++++++++++++++
arch/arm64/kvm/hyp/pgtable.c | 2 -
arch/arm64/kvm/hyp/reserved_mem.c | 92 +++++++++++++
arch/arm64/mm/init.c | 3 +
15 files changed, 654 insertions(+), 5 deletions(-)
This mostly looks good to me, but in a patch this size I was bound to spot
a few niggles. It is _huge_!
Comparing with the new-fangled set_sctlr_el1 macro we have, this sequence
isn't quite right. Probably best to introduce set_sctlr_el2, and implement
that and the existing macro in terms of set_sctlr_elX or something like
that.
+void __noreturn __pkvm_init_finalise(void)
+{
+ struct kvm_host_data *host_data = this_cpu_ptr(&kvm_host_data);
+ struct kvm_cpu_context *host_ctxt = &host_data->host_ctxt;
+ unsigned long nr_pages, reserved_pages, pfn;
+ int ret;
+
+ /* Now that the vmemmap is backed, install the full-fledged allocator */
+ pfn = hyp_virt_to_pfn(hyp_pgt_base);
+ nr_pages = hyp_s1_pgtable_pages();
+ reserved_pages = hyp_early_alloc_nr_used_pages();
+ ret = hyp_pool_init(&hpool, pfn, nr_pages, reserved_pages);
+ if (ret)
+ goto out;
+
+ pkvm_pgtable_mm_ops.zalloc_page = hyp_zalloc_hyp_page;
+ pkvm_pgtable_mm_ops.phys_to_virt = hyp_phys_to_virt;
+ pkvm_pgtable_mm_ops.virt_to_phys = hyp_virt_to_phys;
+ pkvm_pgtable_mm_ops.get_page = hyp_get_page;
+ pkvm_pgtable_mm_ops.put_page = hyp_put_page;
+ pkvm_pgtable.mm_ops = &pkvm_pgtable_mm_ops;
Can you do:
pkvm_pgtable_mm_ops = (struct kvm_pgtable_mm_ops) {
.zalloc_page = hyp_zalloc_hyp_page,
.phys_to_virt = ...,
...
};
here?
+
+out:
+ /*
+ * We tail-called to here from handle___pkvm_init() and will not return,
+ * so make sure to propagate the return value to the host.
+ */
+ cpu_reg(host_ctxt, 1) = ret;
+
+ __host_enter(host_ctxt);
+}
+
+int __pkvm_init(phys_addr_t phys, unsigned long size, unsigned long nr_cpus,
+ unsigned long *per_cpu_base, u32 hyp_va_bits)
+{
+ struct kvm_nvhe_init_params *params;
+ void *virt = hyp_phys_to_virt(phys);
+ void (*fn)(phys_addr_t params_pa, void *finalize_fn_va);
+ int ret;
+
+ if (phys % PAGE_SIZE || size % PAGE_SIZE)
+ return -EINVAL;
Either PAGE_ALIGNED or '& ~PAGE_MASK' would be better than spelling this
with '%', I reckon.
Anyway, other than these nits:
Acked-by: Will Deacon <will@kernel.org>
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-03-04 19:27:15
On Tue, Mar 02, 2021 at 02:59:46PM +0000, Quentin Perret wrote:
Previous commits have introduced infrastructure to enable the EL2 code
to manage its own stage 1 mappings. However, this was preliminary work,
and none of it is currently in use.
Put all of this together by elevating the mapping creation at EL2 when
memory protection is enabled. In this case, the host kernel running
at EL1 still creates _temporary_ EL2 mappings, only used while
initializing the hypervisor, but frees them right after.
As such, all calls to create_hyp_mappings() after kvm init has finished
turn into hypercalls, as the host now has no 'legal' way to modify the
hypevisor page tables directly.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_mmu.h | 2 +-
arch/arm64/kvm/arm.c | 87 +++++++++++++++++++++++++++++---
arch/arm64/kvm/mmu.c | 43 ++++++++++++++--
3 files changed, 120 insertions(+), 12 deletions(-)
[...]
quoted hunk
@@ -1489,13 +1497,14 @@ static void cpu_hyp_reinit(void) kvm_init_host_cpu_context(&this_cpu_ptr_hyp_sym(kvm_host_data)->host_ctxt); cpu_hyp_reset();- cpu_set_hyp_vector(); if (is_kernel_in_hyp_mode()) kvm_timer_init_vhe(); else cpu_init_hyp_mode();+ cpu_set_hyp_vector();+ kvm_arm_init_debug(); if (vgic_present)
@@ -1691,18 +1700,59 @@ static void teardown_hyp_mode(void) } }+static int do_pkvm_init(u32 hyp_va_bits)+{+ void *per_cpu_base = kvm_ksym_ref(kvm_arm_hyp_percpu_base);+ int ret;++ preempt_disable();+ hyp_install_host_vector();
It's a shame we need this both here _and_ on the reinit path, but it looks
like it's necessary.
/**
* Inits Hyp-mode on all online CPUs
*/
static int init_hyp_mode(void)
{
+ u32 hyp_va_bits;
int cpu;
- int err = 0;
+ int err = -ENOMEM;
+
+ /*
+ * The protected Hyp-mode cannot be initialized if the memory pool
+ * allocation has failed.
+ */
+ if (is_protected_kvm_enabled() && !hyp_mem_base)
+ return err;
This skips the error message you get on the out_err path.
From: Will Deacon <will@kernel.org> Date: 2021-03-04 19:36:51
On Tue, Mar 02, 2021 at 02:59:50PM +0000, Quentin Perret wrote:
In order to re-use some of the stage 2 setup code at EL2, factor parts
of kvm_arm_setup_stage2() out into separate functions.
No functional change intended.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_pgtable.h | 26 +++++++++++++++++
arch/arm64/kvm/hyp/pgtable.c | 32 +++++++++++++++++++++
arch/arm64/kvm/reset.c | 42 +++-------------------------
3 files changed, 62 insertions(+), 38 deletions(-)
Looks much better than the big header in v2, thanks!
Acked-by: Will Deacon <will@kernel.org>
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-03-04 19:40:35
On Tue, Mar 02, 2021 at 02:59:52PM +0000, Quentin Perret wrote:
quoted hunk
Refactor __populate_fault_info() to introduce __get_fault_info() which
will be used once the host is wrapped in a stage 2.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/hyp/include/hyp/switch.h | 37 ++++++++++++++-----------
1 file changed, 21 insertions(+), 16 deletions(-)
Just return __get_fault_info(esr, &vcpu->arch.fault); here.
With that:
Acked-by: Will Deacon <will@kernel.org>
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-03-04 19:45:55
On Tue, Mar 02, 2021 at 02:59:53PM +0000, Quentin Perret wrote:
The current stage2 page-table allocator uses a memcache to get
pre-allocated pages when it needs any. To allow re-using this code at
EL2 which uses a concept of memory pools, make the memcache argument of
kvm_pgtable_stage2_map() anonymous, and let the mm_ops zalloc_page()
callbacks use it the way they need to.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_pgtable.h | 6 +++---
arch/arm64/kvm/hyp/pgtable.c | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
From: Will Deacon <will@kernel.org> Date: 2021-03-04 19:51:16
On Tue, Mar 02, 2021 at 02:59:54PM +0000, Quentin Perret wrote:
quoted hunk
Extend the memory pool allocated for the hypervisor to include enough
pages to map all of memory at page granularity for the host stage 2.
While at it, also reserve some memory for device mappings.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/hyp/include/nvhe/mm.h | 23 ++++++++++++++++++++++-
arch/arm64/kvm/hyp/nvhe/setup.c | 12 ++++++++++++
arch/arm64/kvm/hyp/reserved_mem.c | 2 ++
3 files changed, 36 insertions(+), 1 deletion(-)
@@ -53,7 +53,7 @@ static inline unsigned long __hyp_pgtable_max_pages(unsigned long nr_pages)returntotal;}-staticinlineunsignedlonghyp_s1_pgtable_pages(void)+staticinlineunsignedlong__hyp_pgtable_total_pages(void){unsignedlongres=0,i;
@@ -63,9 +63,30 @@ static inline unsigned long hyp_s1_pgtable_pages(void)res+=__hyp_pgtable_max_pages(reg->size>>PAGE_SHIFT);}+returnres;+}++staticinlineunsignedlonghyp_s1_pgtable_pages(void)+{+unsignedlongres;++res=__hyp_pgtable_total_pages();+/* Allow 1 GiB for private mappings */res+=__hyp_pgtable_max_pages(SZ_1G>>PAGE_SHIFT);returnres;}++staticinlineunsignedlonghost_s2_mem_pgtable_pages(void)+{+return__hyp_pgtable_total_pages()+16;
Is this 16 due to the possibility of a concatenated pgd? If so, please add
a comment to that effect.
With that:
Acked-by: Will Deacon <will@kernel.org>
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-03-04 19:52:52
On Tue, Mar 02, 2021 at 02:59:55PM +0000, Quentin Perret wrote:
We will soon need to check if a Physical Address belongs to a memblock
at EL2, so make sure to sort them so this can be done efficiently.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/hyp/reserved_mem.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
From: Will Deacon <will@kernel.org> Date: 2021-03-04 20:02:27
On Tue, Mar 02, 2021 at 02:59:56PM +0000, Quentin Perret wrote:
Once we start unmapping portions of memory from the host stage 2 (such
as e.g. the hypervisor memory sections, or pages that belong to
protected guests), we will need a way to track page ownership. And
given that all mappings in the host stage 2 will be identity-mapped, we
can use the host stage 2 page-table itself as a simplistic rmap.
As a first step towards this, introduce a new protection attribute
in the stage 2 page table code, called KVM_PGTABLE_PROT_NONE, which
allows to annotate portions of the IPA space as inaccessible. For
simplicity, PROT_NONE mappings are created as invalid mappings with a
software bit set.
Just as an observation, but given that they're invalid we can use any bit
from [63:2] to indicate that it's a PROT_NONE mapping, and that way we
can keep the real "software bits" for live mappings.
But we can of course change that later when we need the bit for something
else.
I think it would be a good idea to check !kvm_pte_valid() in here too,
since it doesn't make sense to report true for valid (or table) entries.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-03-04 20:05:22
On Tue, Mar 02, 2021 at 02:59:57PM +0000, Quentin Perret wrote:
quoted hunk
In order to ease its re-use in other code paths, refactor
stage2_map_set_prot_attr() to not depend on a stage2_map_data struct.
No functional change intended.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/hyp/pgtable.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
@@ -504,15 +503,15 @@ static int stage2_map_set_prot_attr(enum kvm_pgtable_prot prot,if(prot&KVM_PGTABLE_PROT_NONE){if(prot!=KVM_PGTABLE_PROT_NONE)-return-EINVAL;+return0;
Hmm, does the architecture actually say that having all these attributes
as 0 is illegal? If not, I think it would be better to keep the int return
code and replace the 'data' parameter with a pointer to a kvm_pte_t.
Does that work?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-03-04 20:07:30
On Tue, Mar 02, 2021 at 03:00:00PM +0000, Quentin Perret wrote:
We will soon unmap the .hyp sections from the host stage 2 in Protected
nVHE mode, which obvisouly works with at least page granularity, so make
sure to align them correctly.
With the typo fixed:
Acked-by: Will Deacon <will@kernel.org>
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thursday 04 Mar 2021 at 19:25:41 (+0000), Will Deacon wrote:
quoted
+static int do_pkvm_init(u32 hyp_va_bits)
+{
+ void *per_cpu_base = kvm_ksym_ref(kvm_arm_hyp_percpu_base);
+ int ret;
+
+ preempt_disable();
+ hyp_install_host_vector();
It's a shame we need this both here _and_ on the reinit path, but it looks
like it's necessary.
Right and I want this before the KVM vectors are installed on secondary
CPUs, to make sure they get the new pgtable from the start. Otherwise
I'd need to do the same dance on all of them to go a switch TTBR0_EL2
and such.
/**
* Inits Hyp-mode on all online CPUs
*/
static int init_hyp_mode(void)
{
+ u32 hyp_va_bits;
int cpu;
- int err = 0;
+ int err = -ENOMEM;
+
+ /*
+ * The protected Hyp-mode cannot be initialized if the memory pool
+ * allocation has failed.
+ */
+ if (is_protected_kvm_enabled() && !hyp_mem_base)
+ return err;
This skips the error message you get on the out_err path.
Wouldn't this WARN when I have !hyp_pgtable && is_protected_kvm_enabled()
but the static key is still off (which can happen, sadly, as per the
comment above)?
Thanks,
Quentin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thursday 04 Mar 2021 at 19:49:53 (+0000), Will Deacon wrote:
On Tue, Mar 02, 2021 at 02:59:54PM +0000, Quentin Perret wrote:
quoted
Extend the memory pool allocated for the hypervisor to include enough
pages to map all of memory at page granularity for the host stage 2.
While at it, also reserve some memory for device mappings.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/hyp/include/nvhe/mm.h | 23 ++++++++++++++++++++++-
arch/arm64/kvm/hyp/nvhe/setup.c | 12 ++++++++++++
arch/arm64/kvm/hyp/reserved_mem.c | 2 ++
3 files changed, 36 insertions(+), 1 deletion(-)
On Thursday 04 Mar 2021 at 20:03:36 (+0000), Will Deacon wrote:
On Tue, Mar 02, 2021 at 02:59:57PM +0000, Quentin Perret wrote:
quoted
In order to ease its re-use in other code paths, refactor
stage2_map_set_prot_attr() to not depend on a stage2_map_data struct.
No functional change intended.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/hyp/pgtable.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
On Thursday 04 Mar 2021 at 20:00:45 (+0000), Will Deacon wrote:
On Tue, Mar 02, 2021 at 02:59:56PM +0000, Quentin Perret wrote:
quoted
Once we start unmapping portions of memory from the host stage 2 (such
as e.g. the hypervisor memory sections, or pages that belong to
protected guests), we will need a way to track page ownership. And
given that all mappings in the host stage 2 will be identity-mapped, we
can use the host stage 2 page-table itself as a simplistic rmap.
As a first step towards this, introduce a new protection attribute
in the stage 2 page table code, called KVM_PGTABLE_PROT_NONE, which
allows to annotate portions of the IPA space as inaccessible. For
simplicity, PROT_NONE mappings are created as invalid mappings with a
software bit set.
Just as an observation, but given that they're invalid we can use any bit
from [63:2] to indicate that it's a PROT_NONE mapping, and that way we
can keep the real "software bits" for live mappings.
But we can of course change that later when we need the bit for something
else.
Right, so I used this approach for consistency with the kernel's
PROT_NONE mappings:
#define PTE_PROT_NONE (_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */
And in fact now that I think about it, it might be worth re-using the
same bit in stage 2.
But yes it would be pretty neat to use the other bits of invalid
mappings to add metadata about the pages. I could even drop the
PROT_NONE stuff straight away in favor of a more extensive mechanism for
tracking page ownership...
Thinking about it, it should be relatively straightforward to construct
the host stage 2 with the following invariants:
1. everything is identity-mapped in the host stage 2;
2. all valid mappings imply the underlying PA range belongs to the
host;
3. bits [63:32] (say) of all invalid mappings are used to store a
unique identifier for the owner of the underlying PA range;
4. the host identifier is 0, such that it owns all of memory by
default at boot as its pgd is zeroed;
And then I could replace my PROT_NONE permission stuff by an ownership
change. E.g. the hypervisor would have its own identifier, and I could
use it to mark the .hyp memory sections as owned by the hyp (which
implies inaccessible by the host). And that should scale quite easily
when we start running protected guests as we'll assign them their own
identifiers. Sharing pages between guests (or even worse, between host
and guests) is a bit trickier, but maybe that is for later.
Thoughts?
Why do we need an extra entry here? Couldn't we just create PROT_NONE
entries when none of R,W or X are set?
The kernel has an explicit PAGE_NONE permission, so I followed the same
idea, but that could work as well. Now, as per the above discussion that
may not be relevant if we implement the page ownership thing.
From: Will Deacon <will@kernel.org> Date: 2021-03-05 14:40:38
On Tue, Mar 02, 2021 at 02:59:58PM +0000, Quentin Perret wrote:
quoted hunk
Add a new map function to the KVM page-table library that allows to
greedily create block identity-mappings. This will be useful to create
lazily the host stage 2 page-table as it will own most of memory and
will always be identity mapped.
The new helper function creates the mapping in 2 steps: it first walks
the page-table to compute the largest possible granule that can be used
to idmap a given address without overriding existing incompatible
mappings; and then creates a mapping accordingly.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_pgtable.h | 37 +++++++++
arch/arm64/kvm/hyp/pgtable.c | 119 +++++++++++++++++++++++++++
2 files changed, 156 insertions(+)
Not sure it's worth mentioning "highest possible level" here, as
realistically the caller still has to provide a memcache to deal with the
worst case and the structure of the page-table shouldn't matter.
+ * @pgt: Page-table structure initialised by kvm_pgtable_*_init().
+ * @addr: Input address to identity-map.
+ * @prot: Permissions and attributes for the mapping.
+ * @range: Boundaries of the maximum memory region to map.
+ * @mc: Cache of pre-allocated memory from which to allocate page-table
+ * pages.
+ *
+ * This function attempts to install high-level identity-mappings covering @addr
"high-level"? (again, I think I'd just drop this)
+ * without overriding existing mappings with incompatible permissions or
+ * attributes. An existing table entry may be coalesced into a block mapping
+ * if and only if it covers @addr and all its leafs are either invalid and/or
s/leafs/leaf entries/
quoted hunk
+ * have permissions and attributes strictly matching @prot. The mapping is
+ * guaranteed to be contained within the boundaries specified by @range at call
+ * time. If only a subset of the memory specified by @range is mapped (because
+ * of e.g. alignment issues or existing incompatible mappings), @range will be
+ * updated accordingly.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+int kvm_pgtable_stage2_idmap_greedy(struct kvm_pgtable *pgt, u64 addr,
+ enum kvm_pgtable_prot prot,
+ struct kvm_mem_range *range,
+ void *mc);
#endif /* __ARM64_KVM_PGTABLE_H__ */
(this will need updating based on the other discussion we had)
+ /* Reduce the kvm_mem_range to a granule size */
+ ret = __stage2_reduce_range(&data, range->end);
+ if (ret)
+ return ret;
+
+ /* Walk the range to check permissions and reduce further if needed */
+ do {
+ ret = kvm_pgtable_walk(pgt, range->start, range->end, &walker);
(we spent some time debugging an issue here and you spotted that you're
passing range->end instead of the size ;)
+ } while (ret == -EAGAIN);
I'm a bit nervous about this loop -- what guarantees forward progress here?
Can we return to the host after a few tries instead?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Friday 05 Mar 2021 at 14:39:42 (+0000), Will Deacon wrote:
On Tue, Mar 02, 2021 at 02:59:58PM +0000, Quentin Perret wrote:
quoted
+/**
+ * kvm_pgtable_stage2_idmap_greedy() - Identity-map an Intermediate Physical
+ * Address with a leaf entry at the highest
+ * possible level.
Not sure it's worth mentioning "highest possible level" here, as
realistically the caller still has to provide a memcache to deal with the
worst case and the structure of the page-table shouldn't matter.
Right, we need to pass a range so I suppose that should be enough to
say 'this tries to cover large portions of memory'.
quoted
+ * @pgt: Page-table structure initialised by kvm_pgtable_*_init().
+ * @addr: Input address to identity-map.
+ * @prot: Permissions and attributes for the mapping.
+ * @range: Boundaries of the maximum memory region to map.
+ * @mc: Cache of pre-allocated memory from which to allocate page-table
+ * pages.
+ *
+ * This function attempts to install high-level identity-mappings covering @addr
"high-level"? (again, I think I'd just drop this)
quoted
+ * without overriding existing mappings with incompatible permissions or
+ * attributes. An existing table entry may be coalesced into a block mapping
+ * if and only if it covers @addr and all its leafs are either invalid and/or
s/leafs/leaf entries/
Ack for both.
quoted
+ * have permissions and attributes strictly matching @prot. The mapping is
+ * guaranteed to be contained within the boundaries specified by @range at call
+ * time. If only a subset of the memory specified by @range is mapped (because
+ * of e.g. alignment issues or existing incompatible mappings), @range will be
+ * updated accordingly.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+int kvm_pgtable_stage2_idmap_greedy(struct kvm_pgtable *pgt, u64 addr,
+ enum kvm_pgtable_prot prot,
+ struct kvm_mem_range *range,
+ void *mc);
#endif /* __ARM64_KVM_PGTABLE_H__ */
(this will need updating based on the other discussion we had)
Ack.
quoted
+ /* Reduce the kvm_mem_range to a granule size */
+ ret = __stage2_reduce_range(&data, range->end);
+ if (ret)
+ return ret;
+
+ /* Walk the range to check permissions and reduce further if needed */
+ do {
+ ret = kvm_pgtable_walk(pgt, range->start, range->end, &walker);
(we spent some time debugging an issue here and you spotted that you're
passing range->end instead of the size ;)
Yep, I have the fix applied locally, and ready to fly in v4 :)
quoted
+ } while (ret == -EAGAIN);
I'm a bit nervous about this loop -- what guarantees forward progress here?
Can we return to the host after a few tries instead?
-EAGAIN only happens when we've been able to successfully reduce the
range to a potentially valid granule size. That can't happen infinitely.
We're guaranteed to fail when trying to reduce the range to a
granularity smaller than PAGE_SIZE (the -EINVAL case of
__stage2_reduce_range), which is indicative of a host memory abort in a
page it should not access (because marked PROT_NONE for instance).
Cheers,
Quentin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-03-05 16:59:49
On Fri, Mar 05, 2021 at 03:03:36PM +0000, Quentin Perret wrote:
On Friday 05 Mar 2021 at 14:39:42 (+0000), Will Deacon wrote:
quoted
On Tue, Mar 02, 2021 at 02:59:58PM +0000, Quentin Perret wrote:
quoted
+ /* Reduce the kvm_mem_range to a granule size */
+ ret = __stage2_reduce_range(&data, range->end);
+ if (ret)
+ return ret;
+
+ /* Walk the range to check permissions and reduce further if needed */
+ do {
+ ret = kvm_pgtable_walk(pgt, range->start, range->end, &walker);
(we spent some time debugging an issue here and you spotted that you're
passing range->end instead of the size ;)
Yep, I have the fix applied locally, and ready to fly in v4 :)
quoted
quoted
+ } while (ret == -EAGAIN);
I'm a bit nervous about this loop -- what guarantees forward progress here?
Can we return to the host after a few tries instead?
-EAGAIN only happens when we've been able to successfully reduce the
range to a potentially valid granule size. That can't happen infinitely.
We're guaranteed to fail when trying to reduce the range to a
granularity smaller than PAGE_SIZE (the -EINVAL case of
__stage2_reduce_range), which is indicative of a host memory abort in a
page it should not access (because marked PROT_NONE for instance).
Can you compute an upper bound on the number of iterations based on the
number of page-table levels then? I'm just conscious that all of this is
effectively running with irqs disabled, and so being able to reason about
the amount of work we're going to do makes me much more comfortable.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-03-05 19:03:07
On Tue, Mar 02, 2021 at 03:00:01PM +0000, Quentin Perret wrote:
The host currently writes directly in EL2 per-CPU data sections from
the PMU code when running in nVHE. In preparation for unmapping the EL2
sections from the host stage 2, disable PMU support in protected mode as
we currently do not have a use-case for it.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/kvm/perf.c | 3 ++-
arch/arm64/kvm/pmu.c | 8 ++++----
2 files changed, 6 insertions(+), 5 deletions(-)
From: Will Deacon <will@kernel.org> Date: 2021-03-05 19:04:44
On Fri, Mar 05, 2021 at 09:52:12AM +0000, Quentin Perret wrote:
On Thursday 04 Mar 2021 at 20:00:45 (+0000), Will Deacon wrote:
quoted
On Tue, Mar 02, 2021 at 02:59:56PM +0000, Quentin Perret wrote:
quoted
Once we start unmapping portions of memory from the host stage 2 (such
as e.g. the hypervisor memory sections, or pages that belong to
protected guests), we will need a way to track page ownership. And
given that all mappings in the host stage 2 will be identity-mapped, we
can use the host stage 2 page-table itself as a simplistic rmap.
As a first step towards this, introduce a new protection attribute
in the stage 2 page table code, called KVM_PGTABLE_PROT_NONE, which
allows to annotate portions of the IPA space as inaccessible. For
simplicity, PROT_NONE mappings are created as invalid mappings with a
software bit set.
Just as an observation, but given that they're invalid we can use any bit
from [63:2] to indicate that it's a PROT_NONE mapping, and that way we
can keep the real "software bits" for live mappings.
But we can of course change that later when we need the bit for something
else.
Right, so I used this approach for consistency with the kernel's
PROT_NONE mappings:
#define PTE_PROT_NONE (_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */
And in fact now that I think about it, it might be worth re-using the
same bit in stage 2.
But yes it would be pretty neat to use the other bits of invalid
mappings to add metadata about the pages. I could even drop the
PROT_NONE stuff straight away in favor of a more extensive mechanism for
tracking page ownership...
Thinking about it, it should be relatively straightforward to construct
the host stage 2 with the following invariants:
1. everything is identity-mapped in the host stage 2;
2. all valid mappings imply the underlying PA range belongs to the
host;
3. bits [63:32] (say) of all invalid mappings are used to store a
unique identifier for the owner of the underlying PA range;
4. the host identifier is 0, such that it owns all of memory by
default at boot as its pgd is zeroed;
And then I could replace my PROT_NONE permission stuff by an ownership
change. E.g. the hypervisor would have its own identifier, and I could
use it to mark the .hyp memory sections as owned by the hyp (which
implies inaccessible by the host). And that should scale quite easily
when we start running protected guests as we'll assign them their own
identifiers. Sharing pages between guests (or even worse, between host
and guests) is a bit trickier, but maybe that is for later.
Thoughts?
I think this sounds like a worthwhile generalisation to me, although virtio
brings an immediate need for shared pages and so we'll still need a software
bit for those so that we e.g. prevent the host from donating such a shared
page to the hypervisor.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-03-05 19:14:32
On Tue, Mar 02, 2021 at 03:00:02PM +0000, Quentin Perret wrote:
When KVM runs in nVHE protected mode, use the host stage 2 to unmap the
hypervisor sections. The long-term goal is to ensure the EL2 code can
remain robust regardless of the host's state, so this starts by making
sure the host cannot e.g. write to the .hyp sections directly.
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_asm.h | 1 +
arch/arm64/kvm/arm.c | 46 +++++++++++++++++++
arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 2 +
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 9 ++++
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 22 +++++++++
5 files changed, 80 insertions(+)
@@ -196,6 +196,28 @@ static int host_stage2_idmap(u64 addr)returnret;}+int__pkvm_host_unmap(phys_addr_tstart,phys_addr_tend)+{+structkvm_mem_ranger1,r2;+intret;++/*+*host_stage2_unmap_dev_all()currentlyreliesonMMIOmappingsbeing+*non-persistent,sodon'tallowPROT_NONEinMMIOrange.+*/+if(!find_mem_range(start,&r1)||!find_mem_range(end,&r2))+return-EINVAL;+if(r1.start!=r2.start)+return-EINVAL;
Feels like this should be in a helper to determine whether or not a range is
solely covered by memory.
Either way:
Acked-by: Will Deacon <will@kernel.org>
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-03-05 19:29:59
On Tue, Mar 02, 2021 at 02:59:59PM +0000, Quentin Perret wrote:
When KVM runs in protected nVHE mode, make use of a stage 2 page-table
to give the hypervisor some control over the host memory accesses. The
host stage 2 is created lazily using large block mappings if possible,
and will default to page mappings in absence of a better solution.
From this point on, memory accesses from the host to protected memory
regions (e.g. marked PROT_NONE) are fatal and lead to hyp_panic().
Signed-off-by: Quentin Perret <redacted>
---
arch/arm64/include/asm/kvm_asm.h | 1 +
arch/arm64/include/asm/kvm_cpufeature.h | 2 +
arch/arm64/kernel/image-vars.h | 3 +
arch/arm64/kvm/arm.c | 10 +
arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 34 +++
arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
arch/arm64/kvm/hyp/nvhe/hyp-init.S | 1 +
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 11 +
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 213 ++++++++++++++++++
arch/arm64/kvm/hyp/nvhe/setup.c | 5 +
arch/arm64/kvm/hyp/nvhe/switch.c | 7 +-
arch/arm64/kvm/hyp/nvhe/tlb.c | 4 +-
12 files changed, 286 insertions(+), 7 deletions(-)
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/mem_protect.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/mem_protect.c
AFAICT, there's no ISB here. Do we need one before the TLB invalidation?
+ __tlbi(vmalls12e1is);
+ dsb(ish);
Given that the TLB is invalidated on the boot path, please can you add
a comment here about the stale entries which you need to invalidate?
Also, does this need to be inner-shareable? I thought this function ran on
each CPU.
+ isb();
+
+ return 0;
+}
+
+static void host_stage2_unmap_dev_all(void)
+{
+ struct kvm_pgtable *pgt = &host_kvm.pgt;
+ struct memblock_region *reg;
+ u64 addr = 0;
+ int i;
+
+ /* Unmap all non-memory regions to recycle the pages */
+ for (i = 0; i < hyp_memblock_nr; i++, addr = reg->base + reg->size) {
+ reg = &hyp_memory[i];
+ kvm_pgtable_stage2_unmap(pgt, addr, reg->base - addr);
+ }
+ kvm_pgtable_stage2_unmap(pgt, addr, ULONG_MAX);
I find this _really_ hard to reason about, as range is passed by reference
and we don't reset it after the first call returns -ENOMEM for an MMIO
mapping. Maybe some commentary on why it's still valid here?
Other than that, this is looking really good to me.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
I find this _really_ hard to reason about, as range is passed by reference
and we don't reset it after the first call returns -ENOMEM for an MMIO
mapping. Maybe some commentary on why it's still valid here?
Sure, I'll add something. FWIW, that is intended -- -ENOMEM can only be
caused by the call to kvm_pgtable_stage2_map() which leaves the range
untouched. So, as long as we don't release the lock, the range returned
by the first call to kvm_pgtable_stage2_idmap_greedy() should still be
valid. I suppose I could call kvm_pgtable_stage2_map() directly the
second time to make it obvious but I thought this would expose the
internal of kvm_pgtable_stage2_idmap_greedy() a little bit too much.
Thanks,
Quentin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
AFAICT, there's no ISB here. Do we need one before the TLB invalidation?
You mean for the ARM64_WORKAROUND_SPECULATIVE_AT case? __load_stage2()
should already add one for me no?
__load_stage2() _only_ has the ISB if ARM64_WORKAROUND_SPECULATIVE_AT is
present, whereas I think you need one unconditionall here so that the
system register write has taken effect before the TLB invalidation.
It's similar to the comment at the end of __tlb_switch_to_guest().
Having said that, I do worry that ARM64_WORKAROUND_SPECULATIVE_AT probably
needs a closer look in the world of pKVM, since it currently special-cases
the host.
quoted
quoted
+ __tlbi(vmalls12e1is);
+ dsb(ish);
Given that the TLB is invalidated on the boot path, please can you add
a comment here about the stale entries which you need to invalidate?
Sure -- that is for HCR bits cached in TLBs for VMID 0. Thinking about
it I could probably reduce the tlbi scope as well.
quoted
Also, does this need to be inner-shareable? I thought this function ran on
each CPU.
Hmm, correct, nsh should do.
Cool, then you can do that for both the TLBI and the DSB instructions (and
please add a comment that the invalidation is due to the HCR bits).
quoted
quoted
+static void host_stage2_unmap_dev_all(void)
+{
+ struct kvm_pgtable *pgt = &host_kvm.pgt;
+ struct memblock_region *reg;
+ u64 addr = 0;
+ int i;
+
+ /* Unmap all non-memory regions to recycle the pages */
+ for (i = 0; i < hyp_memblock_nr; i++, addr = reg->base + reg->size) {
+ reg = &hyp_memory[i];
+ kvm_pgtable_stage2_unmap(pgt, addr, reg->base - addr);
+ }
+ kvm_pgtable_stage2_unmap(pgt, addr, ULONG_MAX);
Is this just going to return -ERANGE?
Hrmpf, yes, that wants BIT(pgt->ia_bits) I think. And that wants testing
as well, clearly.
Agreed, maybe it's worth checking the return value.
I find this _really_ hard to reason about, as range is passed by reference
and we don't reset it after the first call returns -ENOMEM for an MMIO
mapping. Maybe some commentary on why it's still valid here?
Sure, I'll add something. FWIW, that is intended -- -ENOMEM can only be
caused by the call to kvm_pgtable_stage2_map() which leaves the range
untouched. So, as long as we don't release the lock, the range returned
by the first call to kvm_pgtable_stage2_idmap_greedy() should still be
valid. I suppose I could call kvm_pgtable_stage2_map() directly the
second time to make it obvious but I thought this would expose the
internal of kvm_pgtable_stage2_idmap_greedy() a little bit too much.
I can see it both ways, but updating the kerneldoc for
kvm_pgtable_stage2_idmap_greedy() to state in which cases the range is
updated and how would be helpful. It just says "negative error code on
failure" at the moment.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Monday 08 Mar 2021 at 12:46:07 (+0000), Will Deacon wrote:
__load_stage2() _only_ has the ISB if ARM64_WORKAROUND_SPECULATIVE_AT is
present, whereas I think you need one unconditionall here so that the
system register write has taken effect before the TLB invalidation.
It's similar to the comment at the end of __tlb_switch_to_guest().
Having said that, I do worry that ARM64_WORKAROUND_SPECULATIVE_AT probably
needs a closer look in the world of pKVM, since it currently special-cases
the host.
Yes, I see that now. I'll start looking into this.
quoted
quoted
quoted
+ __tlbi(vmalls12e1is);
+ dsb(ish);
Given that the TLB is invalidated on the boot path, please can you add
a comment here about the stale entries which you need to invalidate?
Sure -- that is for HCR bits cached in TLBs for VMID 0. Thinking about
it I could probably reduce the tlbi scope as well.
quoted
Also, does this need to be inner-shareable? I thought this function ran on
each CPU.
Hmm, correct, nsh should do.
Cool, then you can do that for both the TLBI and the DSB instructions (and
please add a comment that the invalidation is due to the HCR bits).
Sure.
quoted
quoted
quoted
+static void host_stage2_unmap_dev_all(void)
+{
+ struct kvm_pgtable *pgt = &host_kvm.pgt;
+ struct memblock_region *reg;
+ u64 addr = 0;
+ int i;
+
+ /* Unmap all non-memory regions to recycle the pages */
+ for (i = 0; i < hyp_memblock_nr; i++, addr = reg->base + reg->size) {
+ reg = &hyp_memory[i];
+ kvm_pgtable_stage2_unmap(pgt, addr, reg->base - addr);
+ }
+ kvm_pgtable_stage2_unmap(pgt, addr, ULONG_MAX);
Is this just going to return -ERANGE?
Hrmpf, yes, that wants BIT(pgt->ia_bits) I think. And that wants testing
as well, clearly.
Agreed, maybe it's worth checking the return value.
Ack, and hyp_panic if != 0, but that is probably preferable anyway.
I find this _really_ hard to reason about, as range is passed by reference
and we don't reset it after the first call returns -ENOMEM for an MMIO
mapping. Maybe some commentary on why it's still valid here?
Sure, I'll add something. FWIW, that is intended -- -ENOMEM can only be
caused by the call to kvm_pgtable_stage2_map() which leaves the range
untouched. So, as long as we don't release the lock, the range returned
by the first call to kvm_pgtable_stage2_idmap_greedy() should still be
valid. I suppose I could call kvm_pgtable_stage2_map() directly the
second time to make it obvious but I thought this would expose the
internal of kvm_pgtable_stage2_idmap_greedy() a little bit too much.
I can see it both ways, but updating the kerneldoc for
kvm_pgtable_stage2_idmap_greedy() to state in which cases the range is
updated and how would be helpful. It just says "negative error code on
failure" at the moment.
Alternatively I could expose the 'reduce' path (maybe with another name
e.g. stage2_find_compatible_range() or so) and remove the
kvm_pgtable_stage2_idmap_greedy() wrapper. So it'd be the caller's
responsibility to not release the lock in between
stage2_find_compatible_range() and kvm_pgtable_stage2_map() for
instance, but that sounds reasonable to me. And that would make it
explicit it's the _map() path that failed with -ENOMEM, and that the
range can be re-used the second time.
Thoughts?
Thanks,
Quentin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
I find this _really_ hard to reason about, as range is passed by reference
and we don't reset it after the first call returns -ENOMEM for an MMIO
mapping. Maybe some commentary on why it's still valid here?
Sure, I'll add something. FWIW, that is intended -- -ENOMEM can only be
caused by the call to kvm_pgtable_stage2_map() which leaves the range
untouched. So, as long as we don't release the lock, the range returned
by the first call to kvm_pgtable_stage2_idmap_greedy() should still be
valid. I suppose I could call kvm_pgtable_stage2_map() directly the
second time to make it obvious but I thought this would expose the
internal of kvm_pgtable_stage2_idmap_greedy() a little bit too much.
I can see it both ways, but updating the kerneldoc for
kvm_pgtable_stage2_idmap_greedy() to state in which cases the range is
updated and how would be helpful. It just says "negative error code on
failure" at the moment.
Alternatively I could expose the 'reduce' path (maybe with another name
e.g. stage2_find_compatible_range() or so) and remove the
kvm_pgtable_stage2_idmap_greedy() wrapper. So it'd be the caller's
responsibility to not release the lock in between
stage2_find_compatible_range() and kvm_pgtable_stage2_map() for
instance, but that sounds reasonable to me. And that would make it
explicit it's the _map() path that failed with -ENOMEM, and that the
range can be re-used the second time.
Thoughts?
I suppose it depends on whether or not you reckon this could be optimised
into a single-pass of the page-table. If not, then splitting it up makes
sense to me (and actually, it's not like this has tonnes of callers so
even if we changed things in future it wouldn't be too hard to fix them up).
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel