2021-08-09 20:00:24

by Quentin Perret

[permalink] [raw]
Subject: [PATCH v4 00/21] Track shared pages at EL2 in protected mode

Hi all,

This is v4 of the patch series previously posted here:

https://lore.kernel.org/lkml/[email protected]/

This series aims to improve how the nVHE hypervisor tracks ownership of
memory pages when running in protected mode ("kvm-arm.mode=protected" on
the kernel command line).

The main issue with the existing ownership tracking code is that it is
completely binary: a page is either owned by an entity (e.g. the host)
or not. However, we'll need something smarter to track shared pages, as
is needed for virtio, or even just host/hypervisor communications.

This series introduces a few changes to the kvm page-table library to
allow annotating shared pages in ignored bits (a.k.a. software bits) of
leaf entries, and makes use of that infrastructure to track all pages
that are shared between the host and the hypervisor. We will obviously
want to apply the same treatment to guest stage-2 page-tables, but that
is not really possible to do until EL2 manages them directly, so I'll
keep that for another series.

The series is based on the 5.14-rc5, and has been tested on
AML-S905X-CC (Le Potato) and using various Qemu configurations.

Changes since v3
- Fixed typos in comments / commit messages;
- Various small cleanups and refactoring;
- Rebased on 5.14-rc5.

Marc Zyngier (1):
KVM: arm64: Introduce helper to retrieve a PTE and its level

Quentin Perret (19):
KVM: arm64: Introduce hyp_assert_lock_held()
KVM: arm64: Provide the host_stage2_try() helper macro
KVM: arm64: Expose page-table helpers
KVM: arm64: Optimize host memory aborts
KVM: arm64: Rename KVM_PTE_LEAF_ATTR_S2_IGNORED
KVM: arm64: Don't overwrite software bits with owner id
KVM: arm64: Tolerate re-creating hyp mappings to set software bits
KVM: arm64: Enable forcing page-level stage-2 mappings
KVM: arm64: Allow populating software bits
KVM: arm64: Add helpers to tag shared pages in SW bits
KVM: arm64: Expose host stage-2 manipulation helpers
KVM: arm64: Expose pkvm_hyp_id
KVM: arm64: Introduce addr_is_memory()
KVM: arm64: Enable retrieving protections attributes of PTEs
KVM: arm64: Mark host bss and rodata section as shared
KVM: arm64: Remove __pkvm_mark_hyp
KVM: arm64: Refactor protected nVHE stage-1 locking
KVM: arm64: Restrict EL2 stage-1 changes in protected mode
KVM: arm64: Make __pkvm_create_mappings static

Will Deacon (1):
KVM: arm64: Add hyp_spin_is_locked() for basic locking assertions at
EL2

arch/arm64/include/asm/kvm_asm.h | 5 +-
arch/arm64/include/asm/kvm_pgtable.h | 167 ++++++++----
arch/arm64/kvm/Kconfig | 9 +
arch/arm64/kvm/arm.c | 46 ----
arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 33 ++-
arch/arm64/kvm/hyp/include/nvhe/mm.h | 3 +-
arch/arm64/kvm/hyp/include/nvhe/spinlock.h | 25 ++
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 20 +-
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 225 +++++++++++++---
arch/arm64/kvm/hyp/nvhe/mm.c | 22 +-
arch/arm64/kvm/hyp/nvhe/setup.c | 82 +++++-
arch/arm64/kvm/hyp/pgtable.c | 247 +++++++++---------
arch/arm64/kvm/mmu.c | 28 +-
13 files changed, 628 insertions(+), 284 deletions(-)

--
2.32.0.605.g8dce9f2422-goog


2021-08-09 20:00:48

by Quentin Perret

[permalink] [raw]
Subject: [PATCH v4 17/21] KVM: arm64: Mark host bss and rodata section as shared

As the hypervisor maps the host's .bss and .rodata sections in its
stage-1, make sure to tag them as shared in hyp and host page-tables.

But since the hypervisor relies on the presence of these mappings, we
cannot let the host in complete control of the memory regions -- it
must not unshare or donate them to another entity for example. To
prevent this, let's transfer the ownership of those ranges to the
hypervisor itself, and share the pages back with the host.

Signed-off-by: Quentin Perret <[email protected]>
---
arch/arm64/kvm/hyp/nvhe/setup.c | 82 +++++++++++++++++++++++++++++----
1 file changed, 74 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/setup.c b/arch/arm64/kvm/hyp/nvhe/setup.c
index 0b574d106519..57c27846320f 100644
--- a/arch/arm64/kvm/hyp/nvhe/setup.c
+++ b/arch/arm64/kvm/hyp/nvhe/setup.c
@@ -58,6 +58,7 @@ static int recreate_hyp_mappings(phys_addr_t phys, unsigned long size,
{
void *start, *end, *virt = hyp_phys_to_virt(phys);
unsigned long pgt_size = hyp_s1_pgtable_pages() << PAGE_SHIFT;
+ enum kvm_pgtable_prot prot;
int ret, i;

/* Recreate the hyp page-table using the early page allocator */
@@ -83,10 +84,6 @@ static int recreate_hyp_mappings(phys_addr_t phys, unsigned long size,
if (ret)
return ret;

- ret = pkvm_create_mappings(__start_rodata, __end_rodata, PAGE_HYP_RO);
- if (ret)
- return ret;
-
ret = pkvm_create_mappings(__hyp_rodata_start, __hyp_rodata_end, PAGE_HYP_RO);
if (ret)
return ret;
@@ -95,10 +92,6 @@ static int recreate_hyp_mappings(phys_addr_t phys, unsigned long size,
if (ret)
return ret;

- ret = pkvm_create_mappings(__hyp_bss_end, __bss_stop, PAGE_HYP_RO);
- if (ret)
- return ret;
-
ret = pkvm_create_mappings(virt, virt + size, PAGE_HYP);
if (ret)
return ret;
@@ -117,6 +110,24 @@ static int recreate_hyp_mappings(phys_addr_t phys, unsigned long size,
return ret;
}

+ /*
+ * Map the host's .bss and .rodata sections RO in the hypervisor, but
+ * transfer the ownership from the host to the hypervisor itself to
+ * make sure it can't be donated or shared with another entity.
+ *
+ * The ownership transition requires matching changes in the host
+ * stage-2. This will be done later (see finalize_host_mappings()) once
+ * the hyp_vmemmap is addressable.
+ */
+ prot = pkvm_mkstate(PAGE_HYP_RO, PKVM_PAGE_SHARED_OWNED);
+ ret = pkvm_create_mappings(__start_rodata, __end_rodata, prot);
+ if (ret)
+ return ret;
+
+ ret = pkvm_create_mappings(__hyp_bss_end, __bss_stop, prot);
+ if (ret)
+ return ret;
+
return 0;
}

@@ -148,6 +159,57 @@ static void hpool_put_page(void *addr)
hyp_put_page(&hpool, addr);
}

+static int finalize_host_mappings_walker(u64 addr, u64 end, u32 level,
+ kvm_pte_t *ptep,
+ enum kvm_pgtable_walk_flags flag,
+ void * const arg)
+{
+ enum kvm_pgtable_prot prot;
+ enum pkvm_page_state state;
+ kvm_pte_t pte = *ptep;
+ phys_addr_t phys;
+
+ if (!kvm_pte_valid(pte))
+ return 0;
+
+ if (level != (KVM_PGTABLE_MAX_LEVELS - 1))
+ return -EINVAL;
+
+ phys = kvm_pte_to_phys(pte);
+ if (!addr_is_memory(phys))
+ return 0;
+
+ /*
+ * Adjust the host stage-2 mappings to match the ownership attributes
+ * configured in the hypervisor stage-1.
+ */
+ state = pkvm_getstate(kvm_pgtable_hyp_pte_prot(pte));
+ switch (state) {
+ case PKVM_PAGE_OWNED:
+ return host_stage2_set_owner_locked(phys, PAGE_SIZE, pkvm_hyp_id);
+ case PKVM_PAGE_SHARED_OWNED:
+ prot = pkvm_mkstate(PKVM_HOST_MEM_PROT, PKVM_PAGE_SHARED_BORROWED);
+ break;
+ case PKVM_PAGE_SHARED_BORROWED:
+ prot = pkvm_mkstate(PKVM_HOST_MEM_PROT, PKVM_PAGE_SHARED_OWNED);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return host_stage2_idmap_locked(phys, PAGE_SIZE, prot);
+}
+
+static int finalize_host_mappings(void)
+{
+ struct kvm_pgtable_walker walker = {
+ .cb = finalize_host_mappings_walker,
+ .flags = KVM_PGTABLE_WALK_LEAF,
+ };
+
+ return kvm_pgtable_walk(&pkvm_pgtable, 0, BIT(pkvm_pgtable.ia_bits), &walker);
+}
+
void __noreturn __pkvm_init_finalise(void)
{
struct kvm_host_data *host_data = this_cpu_ptr(&kvm_host_data);
@@ -167,6 +229,10 @@ void __noreturn __pkvm_init_finalise(void)
if (ret)
goto out;

+ ret = finalize_host_mappings();
+ if (ret)
+ goto out;
+
pkvm_pgtable_mm_ops = (struct kvm_pgtable_mm_ops) {
.zalloc_page = hyp_zalloc_hyp_page,
.phys_to_virt = hyp_phys_to_virt,
--
2.32.0.605.g8dce9f2422-goog

2021-08-09 20:09:44

by Quentin Perret

[permalink] [raw]
Subject: [PATCH v4 11/21] KVM: arm64: Allow populating software bits

Introduce infrastructure allowing to manipulate software bits in stage-1
and stage-2 page-tables using additional entries in the kvm_pgtable_prot
enum.

This is heavily inspired by Marc's implementation of a similar feature
in the NV patch series, but adapted to allow stage-1 changes as well:

https://lore.kernel.org/kvmarm/[email protected]/

Suggested-by: Marc Zyngier <[email protected]>
Signed-off-by: Quentin Perret <[email protected]>
---
arch/arm64/include/asm/kvm_pgtable.h | 12 +++++++++++-
arch/arm64/kvm/hyp/pgtable.c | 5 +++++
2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index 2c090b0eee77..ff9d52f8073a 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -121,6 +121,10 @@ enum kvm_pgtable_stage2_flags {
* @KVM_PGTABLE_PROT_W: Write permission.
* @KVM_PGTABLE_PROT_R: Read permission.
* @KVM_PGTABLE_PROT_DEVICE: Device attributes.
+ * @KVM_PGTABLE_PROT_SW0: Software bit 0.
+ * @KVM_PGTABLE_PROT_SW1: Software bit 1.
+ * @KVM_PGTABLE_PROT_SW2: Software bit 2.
+ * @KVM_PGTABLE_PROT_SW3: Software bit 3.
*/
enum kvm_pgtable_prot {
KVM_PGTABLE_PROT_X = BIT(0),
@@ -128,6 +132,11 @@ enum kvm_pgtable_prot {
KVM_PGTABLE_PROT_R = BIT(2),

KVM_PGTABLE_PROT_DEVICE = BIT(3),
+
+ KVM_PGTABLE_PROT_SW0 = BIT(55),
+ KVM_PGTABLE_PROT_SW1 = BIT(56),
+ KVM_PGTABLE_PROT_SW2 = BIT(57),
+ KVM_PGTABLE_PROT_SW3 = BIT(58),
};

#define KVM_PGTABLE_PROT_RW (KVM_PGTABLE_PROT_R | KVM_PGTABLE_PROT_W)
@@ -420,7 +429,8 @@ kvm_pte_t kvm_pgtable_stage2_mkold(struct kvm_pgtable *pgt, u64 addr);
* If there is a valid, leaf page-table entry used to translate @addr, then
* relax the permissions in that entry according to the read, write and
* execute permissions specified by @prot. No permissions are removed, and
- * TLB invalidation is performed after updating the entry.
+ * TLB invalidation is performed after updating the entry. Software bits cannot
+ * be set or cleared using kvm_pgtable_stage2_relax_perms().
*
* Return: 0 on success, negative error code on failure.
*/
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index e25d829587b9..cff744136044 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -357,6 +357,7 @@ static int hyp_set_prot_attr(enum kvm_pgtable_prot prot, kvm_pte_t *ptep)
attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S1_AP, ap);
attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S1_SH, sh);
attr |= KVM_PTE_LEAF_ATTR_LO_S1_AF;
+ attr |= prot & KVM_PTE_LEAF_ATTR_HI_SW;
*ptep = attr;

return 0;
@@ -558,6 +559,7 @@ static int stage2_set_prot_attr(struct kvm_pgtable *pgt, enum kvm_pgtable_prot p

attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S2_SH, sh);
attr |= KVM_PTE_LEAF_ATTR_LO_S2_AF;
+ attr |= prot & KVM_PTE_LEAF_ATTR_HI_SW;
*ptep = attr;

return 0;
@@ -1025,6 +1027,9 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr,
u32 level;
kvm_pte_t set = 0, clr = 0;

+ if (prot & KVM_PTE_LEAF_ATTR_HI_SW)
+ return -EINVAL;
+
if (prot & KVM_PGTABLE_PROT_R)
set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;

--
2.32.0.605.g8dce9f2422-goog

2021-08-09 20:13:21

by Quentin Perret

[permalink] [raw]
Subject: [PATCH v4 20/21] KVM: arm64: Restrict EL2 stage-1 changes in protected mode

The host kernel is currently able to change EL2 stage-1 mappings without
restrictions thanks to the __pkvm_create_mappings() hypercall. But in a
world where the host is no longer part of the TCB, this clearly poses a
problem.

To fix this, introduce a new hypercall to allow the host to share a
physical memory page with the hypervisor, and remove the
__pkvm_create_mappings() variant. The new hypercall implements
ownership and permission checks before allowing the sharing operation,
and it annotates the shared page in the hypervisor stage-1 and host
stage-2 page-tables.

Signed-off-by: Quentin Perret <[email protected]>
---
arch/arm64/include/asm/kvm_asm.h | 2 +-
arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 1 +
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 11 +--
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 88 +++++++++++++++++++
arch/arm64/kvm/mmu.c | 28 +++++-
5 files changed, 118 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index 432a9ea1f02e..aed2aa61766a 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -59,7 +59,7 @@
#define __KVM_HOST_SMCCC_FUNC___vgic_v3_save_aprs 13
#define __KVM_HOST_SMCCC_FUNC___vgic_v3_restore_aprs 14
#define __KVM_HOST_SMCCC_FUNC___pkvm_init 15
-#define __KVM_HOST_SMCCC_FUNC___pkvm_create_mappings 16
+#define __KVM_HOST_SMCCC_FUNC___pkvm_host_share_hyp 16
#define __KVM_HOST_SMCCC_FUNC___pkvm_create_private_mapping 17
#define __KVM_HOST_SMCCC_FUNC___pkvm_cpu_set_vector 18
#define __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize 19
diff --git a/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h b/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h
index 0118527b07b0..03e604f842e2 100644
--- a/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h
+++ b/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h
@@ -49,6 +49,7 @@ extern struct host_kvm host_kvm;
extern const u8 pkvm_hyp_id;

int __pkvm_prot_finalize(void);
+int __pkvm_host_share_hyp(u64 pfn);

bool addr_is_memory(phys_addr_t phys);
int host_stage2_idmap_locked(phys_addr_t addr, u64 size, enum kvm_pgtable_prot prot);
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index 7900d5b66ba3..2da6aa8da868 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -140,14 +140,11 @@ static void handle___pkvm_cpu_set_vector(struct kvm_cpu_context *host_ctxt)
cpu_reg(host_ctxt, 1) = pkvm_cpu_set_vector(slot);
}

-static void handle___pkvm_create_mappings(struct kvm_cpu_context *host_ctxt)
+static void handle___pkvm_host_share_hyp(struct kvm_cpu_context *host_ctxt)
{
- DECLARE_REG(unsigned long, start, host_ctxt, 1);
- DECLARE_REG(unsigned long, size, host_ctxt, 2);
- DECLARE_REG(unsigned long, phys, host_ctxt, 3);
- DECLARE_REG(enum kvm_pgtable_prot, prot, host_ctxt, 4);
+ DECLARE_REG(u64, pfn, host_ctxt, 1);

- cpu_reg(host_ctxt, 1) = __pkvm_create_mappings(start, size, phys, prot);
+ cpu_reg(host_ctxt, 1) = __pkvm_host_share_hyp(pfn);
}

static void handle___pkvm_create_private_mapping(struct kvm_cpu_context *host_ctxt)
@@ -185,7 +182,7 @@ static const hcall_t host_hcall[] = {
HANDLE_FUNC(__vgic_v3_restore_aprs),
HANDLE_FUNC(__pkvm_init),
HANDLE_FUNC(__pkvm_cpu_set_vector),
- HANDLE_FUNC(__pkvm_create_mappings),
+ HANDLE_FUNC(__pkvm_host_share_hyp),
HANDLE_FUNC(__pkvm_create_private_mapping),
HANDLE_FUNC(__pkvm_prot_finalize),
};
diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
index 2991dc6996b9..8165390d3ec9 100644
--- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
+++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
@@ -339,6 +339,94 @@ static int host_stage2_idmap(u64 addr)
return ret;
}

+static inline bool check_prot(enum kvm_pgtable_prot prot,
+ enum kvm_pgtable_prot required,
+ enum kvm_pgtable_prot denied)
+{
+ return (prot & (required | denied)) == required;
+}
+
+int __pkvm_host_share_hyp(u64 pfn)
+{
+ phys_addr_t addr = hyp_pfn_to_phys(pfn);
+ enum kvm_pgtable_prot prot, cur;
+ void *virt = __hyp_va(addr);
+ enum pkvm_page_state state;
+ kvm_pte_t pte;
+ int ret;
+
+ if (!addr_is_memory(addr))
+ return -EINVAL;
+
+ hyp_spin_lock(&host_kvm.lock);
+ hyp_spin_lock(&pkvm_pgd_lock);
+
+ ret = kvm_pgtable_get_leaf(&host_kvm.pgt, addr, &pte, NULL);
+ if (ret)
+ goto unlock;
+ if (!pte)
+ goto map_shared;
+
+ /*
+ * Check attributes in the host stage-2 PTE. We need the page to be:
+ * - mapped RWX as we're sharing memory;
+ * - not borrowed, as that implies absence of ownership.
+ * Otherwise, we can't let it got through
+ */
+ cur = kvm_pgtable_stage2_pte_prot(pte);
+ prot = pkvm_mkstate(0, PKVM_PAGE_SHARED_BORROWED);
+ if (!check_prot(cur, PKVM_HOST_MEM_PROT, prot)) {
+ ret = -EPERM;
+ goto unlock;
+ }
+
+ state = pkvm_getstate(cur);
+ if (state == PKVM_PAGE_OWNED)
+ goto map_shared;
+
+ /*
+ * Tolerate double-sharing the same page, but this requires
+ * cross-checking the hypervisor stage-1.
+ */
+ if (state != PKVM_PAGE_SHARED_OWNED) {
+ ret = -EPERM;
+ goto unlock;
+ }
+
+ ret = kvm_pgtable_get_leaf(&pkvm_pgtable, (u64)virt, &pte, NULL);
+ if (ret)
+ goto unlock;
+
+ /*
+ * If the page has been shared with the hypervisor, it must be
+ * already mapped as SHARED_BORROWED in its stage-1.
+ */
+ cur = kvm_pgtable_hyp_pte_prot(pte);
+ prot = pkvm_mkstate(PAGE_HYP, PKVM_PAGE_SHARED_BORROWED);
+ if (!check_prot(cur, prot, ~prot))
+ ret = EPERM;
+ goto unlock;
+
+map_shared:
+ /*
+ * If the page is not yet shared, adjust mappings in both page-tables
+ * while both locks are held.
+ */
+ prot = pkvm_mkstate(PAGE_HYP, PKVM_PAGE_SHARED_BORROWED);
+ ret = pkvm_create_mappings_locked(virt, virt + PAGE_SIZE, prot);
+ BUG_ON(ret);
+
+ prot = pkvm_mkstate(PKVM_HOST_MEM_PROT, PKVM_PAGE_SHARED_OWNED);
+ ret = host_stage2_idmap_locked(addr, PAGE_SIZE, prot);
+ BUG_ON(ret);
+
+unlock:
+ hyp_spin_unlock(&pkvm_pgd_lock);
+ hyp_spin_unlock(&host_kvm.lock);
+
+ return ret;
+}
+
void handle_host_mem_abort(struct kvm_cpu_context *host_ctxt)
{
struct kvm_vcpu_fault_info fault;
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 0625bf2353c2..cbab146cda6a 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -259,10 +259,8 @@ static int __create_hyp_mappings(unsigned long start, unsigned long size,
{
int err;

- if (!kvm_host_owns_hyp_mappings()) {
- return kvm_call_hyp_nvhe(__pkvm_create_mappings,
- start, size, phys, prot);
- }
+ if (WARN_ON(!kvm_host_owns_hyp_mappings()))
+ return -EINVAL;

mutex_lock(&kvm_hyp_pgd_mutex);
err = kvm_pgtable_hyp_map(hyp_pgtable, start, size, phys, prot);
@@ -282,6 +280,21 @@ static phys_addr_t kvm_kaddr_to_phys(void *kaddr)
}
}

+static int pkvm_share_hyp(phys_addr_t start, phys_addr_t end)
+{
+ phys_addr_t addr;
+ int ret;
+
+ for (addr = ALIGN_DOWN(start, PAGE_SIZE); addr < end; addr += PAGE_SIZE) {
+ ret = kvm_call_hyp_nvhe(__pkvm_host_share_hyp,
+ __phys_to_pfn(addr));
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
/**
* create_hyp_mappings - duplicate a kernel virtual address range in Hyp mode
* @from: The virtual kernel start address of the range
@@ -302,6 +315,13 @@ int create_hyp_mappings(void *from, void *to, enum kvm_pgtable_prot prot)
if (is_kernel_in_hyp_mode())
return 0;

+ if (!kvm_host_owns_hyp_mappings()) {
+ if (WARN_ON(prot != PAGE_HYP))
+ return -EPERM;
+ return pkvm_share_hyp(kvm_kaddr_to_phys(from),
+ kvm_kaddr_to_phys(to));
+ }
+
start = start & PAGE_MASK;
end = PAGE_ALIGN(end);

--
2.32.0.605.g8dce9f2422-goog

2021-08-09 20:13:23

by Quentin Perret

[permalink] [raw]
Subject: [PATCH v4 14/21] KVM: arm64: Expose pkvm_hyp_id

Allow references to the hypervisor's owner id from outside
mem_protect.c.

Signed-off-by: Quentin Perret <[email protected]>
---
arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 2 ++
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h b/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h
index 0849ee8fa260..23316a021880 100644
--- a/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h
+++ b/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h
@@ -46,6 +46,8 @@ struct host_kvm {
};
extern struct host_kvm host_kvm;

+extern const u8 pkvm_hyp_id;
+
int __pkvm_prot_finalize(void);
int __pkvm_mark_hyp(phys_addr_t start, phys_addr_t end);

diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
index f95a5a4aa09c..ee255171945c 100644
--- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
+++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
@@ -31,7 +31,7 @@ static struct hyp_pool host_s2_pool;
u64 id_aa64mmfr0_el1_sys_val;
u64 id_aa64mmfr1_el1_sys_val;

-static const u8 pkvm_hyp_id = 1;
+const u8 pkvm_hyp_id = 1;

static void *host_s2_zalloc_pages_exact(size_t size)
{
--
2.32.0.605.g8dce9f2422-goog

2021-08-10 06:53:15

by Fuad Tabba

[permalink] [raw]
Subject: Re: [PATCH v4 14/21] KVM: arm64: Expose pkvm_hyp_id

Hi Quentin,

On Mon, Aug 9, 2021 at 5:25 PM Quentin Perret <[email protected]> wrote:
>
> Allow references to the hypervisor's owner id from outside
> mem_protect.c.
>
> Signed-off-by: Quentin Perret <[email protected]>
> ---

Reviewed-by: Fuad Tabba <[email protected]>

Thanks,
/fuad

> arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 2 ++
> arch/arm64/kvm/hyp/nvhe/mem_protect.c | 2 +-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h b/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h
> index 0849ee8fa260..23316a021880 100644
> --- a/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h
> +++ b/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h
> @@ -46,6 +46,8 @@ struct host_kvm {
> };
> extern struct host_kvm host_kvm;
>
> +extern const u8 pkvm_hyp_id;
> +
> int __pkvm_prot_finalize(void);
> int __pkvm_mark_hyp(phys_addr_t start, phys_addr_t end);
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> index f95a5a4aa09c..ee255171945c 100644
> --- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> +++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> @@ -31,7 +31,7 @@ static struct hyp_pool host_s2_pool;
> u64 id_aa64mmfr0_el1_sys_val;
> u64 id_aa64mmfr1_el1_sys_val;
>
> -static const u8 pkvm_hyp_id = 1;
> +const u8 pkvm_hyp_id = 1;
>
> static void *host_s2_zalloc_pages_exact(size_t size)
> {
> --
> 2.32.0.605.g8dce9f2422-goog
>

2021-08-10 08:21:20

by Fuad Tabba

[permalink] [raw]
Subject: Re: [PATCH v4 17/21] KVM: arm64: Mark host bss and rodata section as shared

Hi Quentin,

On Mon, Aug 9, 2021 at 5:25 PM Quentin Perret <[email protected]> wrote:
>
> As the hypervisor maps the host's .bss and .rodata sections in its
> stage-1, make sure to tag them as shared in hyp and host page-tables.
>
> But since the hypervisor relies on the presence of these mappings, we
> cannot let the host in complete control of the memory regions -- it
> must not unshare or donate them to another entity for example. To
> prevent this, let's transfer the ownership of those ranges to the
> hypervisor itself, and share the pages back with the host.
>
> Signed-off-by: Quentin Perret <[email protected]>
> ---

Reviewed-by: Fuad Tabba <[email protected]>

Thanks,
/fuad


> arch/arm64/kvm/hyp/nvhe/setup.c | 82 +++++++++++++++++++++++++++++----
> 1 file changed, 74 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/setup.c b/arch/arm64/kvm/hyp/nvhe/setup.c
> index 0b574d106519..57c27846320f 100644
> --- a/arch/arm64/kvm/hyp/nvhe/setup.c
> +++ b/arch/arm64/kvm/hyp/nvhe/setup.c
> @@ -58,6 +58,7 @@ static int recreate_hyp_mappings(phys_addr_t phys, unsigned long size,
> {
> void *start, *end, *virt = hyp_phys_to_virt(phys);
> unsigned long pgt_size = hyp_s1_pgtable_pages() << PAGE_SHIFT;
> + enum kvm_pgtable_prot prot;
> int ret, i;
>
> /* Recreate the hyp page-table using the early page allocator */
> @@ -83,10 +84,6 @@ static int recreate_hyp_mappings(phys_addr_t phys, unsigned long size,
> if (ret)
> return ret;
>
> - ret = pkvm_create_mappings(__start_rodata, __end_rodata, PAGE_HYP_RO);
> - if (ret)
> - return ret;
> -
> ret = pkvm_create_mappings(__hyp_rodata_start, __hyp_rodata_end, PAGE_HYP_RO);
> if (ret)
> return ret;
> @@ -95,10 +92,6 @@ static int recreate_hyp_mappings(phys_addr_t phys, unsigned long size,
> if (ret)
> return ret;
>
> - ret = pkvm_create_mappings(__hyp_bss_end, __bss_stop, PAGE_HYP_RO);
> - if (ret)
> - return ret;
> -
> ret = pkvm_create_mappings(virt, virt + size, PAGE_HYP);
> if (ret)
> return ret;
> @@ -117,6 +110,24 @@ static int recreate_hyp_mappings(phys_addr_t phys, unsigned long size,
> return ret;
> }
>
> + /*
> + * Map the host's .bss and .rodata sections RO in the hypervisor, but
> + * transfer the ownership from the host to the hypervisor itself to
> + * make sure it can't be donated or shared with another entity.
> + *
> + * The ownership transition requires matching changes in the host
> + * stage-2. This will be done later (see finalize_host_mappings()) once
> + * the hyp_vmemmap is addressable.
> + */
> + prot = pkvm_mkstate(PAGE_HYP_RO, PKVM_PAGE_SHARED_OWNED);
> + ret = pkvm_create_mappings(__start_rodata, __end_rodata, prot);
> + if (ret)
> + return ret;
> +
> + ret = pkvm_create_mappings(__hyp_bss_end, __bss_stop, prot);
> + if (ret)
> + return ret;
> +
> return 0;
> }
>
> @@ -148,6 +159,57 @@ static void hpool_put_page(void *addr)
> hyp_put_page(&hpool, addr);
> }
>
> +static int finalize_host_mappings_walker(u64 addr, u64 end, u32 level,
> + kvm_pte_t *ptep,
> + enum kvm_pgtable_walk_flags flag,
> + void * const arg)
> +{
> + enum kvm_pgtable_prot prot;
> + enum pkvm_page_state state;
> + kvm_pte_t pte = *ptep;
> + phys_addr_t phys;
> +
> + if (!kvm_pte_valid(pte))
> + return 0;
> +
> + if (level != (KVM_PGTABLE_MAX_LEVELS - 1))
> + return -EINVAL;
> +
> + phys = kvm_pte_to_phys(pte);
> + if (!addr_is_memory(phys))
> + return 0;
> +
> + /*
> + * Adjust the host stage-2 mappings to match the ownership attributes
> + * configured in the hypervisor stage-1.
> + */
> + state = pkvm_getstate(kvm_pgtable_hyp_pte_prot(pte));
> + switch (state) {
> + case PKVM_PAGE_OWNED:
> + return host_stage2_set_owner_locked(phys, PAGE_SIZE, pkvm_hyp_id);
> + case PKVM_PAGE_SHARED_OWNED:
> + prot = pkvm_mkstate(PKVM_HOST_MEM_PROT, PKVM_PAGE_SHARED_BORROWED);
> + break;
> + case PKVM_PAGE_SHARED_BORROWED:
> + prot = pkvm_mkstate(PKVM_HOST_MEM_PROT, PKVM_PAGE_SHARED_OWNED);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return host_stage2_idmap_locked(phys, PAGE_SIZE, prot);
> +}
> +
> +static int finalize_host_mappings(void)
> +{
> + struct kvm_pgtable_walker walker = {
> + .cb = finalize_host_mappings_walker,
> + .flags = KVM_PGTABLE_WALK_LEAF,
> + };
> +
> + return kvm_pgtable_walk(&pkvm_pgtable, 0, BIT(pkvm_pgtable.ia_bits), &walker);
> +}
> +
> void __noreturn __pkvm_init_finalise(void)
> {
> struct kvm_host_data *host_data = this_cpu_ptr(&kvm_host_data);
> @@ -167,6 +229,10 @@ void __noreturn __pkvm_init_finalise(void)
> if (ret)
> goto out;
>
> + ret = finalize_host_mappings();
> + if (ret)
> + goto out;
> +
> pkvm_pgtable_mm_ops = (struct kvm_pgtable_mm_ops) {
> .zalloc_page = hyp_zalloc_hyp_page,
> .phys_to_virt = hyp_phys_to_virt,
> --
> 2.32.0.605.g8dce9f2422-goog
>

2021-08-10 08:25:17

by Fuad Tabba

[permalink] [raw]
Subject: Re: [PATCH v4 20/21] KVM: arm64: Restrict EL2 stage-1 changes in protected mode

Hi Quentin,

On Mon, Aug 9, 2021 at 5:25 PM Quentin Perret <[email protected]> wrote:
>
> The host kernel is currently able to change EL2 stage-1 mappings without
> restrictions thanks to the __pkvm_create_mappings() hypercall. But in a
> world where the host is no longer part of the TCB, this clearly poses a
> problem.
>
> To fix this, introduce a new hypercall to allow the host to share a
> physical memory page with the hypervisor, and remove the
> __pkvm_create_mappings() variant. The new hypercall implements
> ownership and permission checks before allowing the sharing operation,
> and it annotates the shared page in the hypervisor stage-1 and host
> stage-2 page-tables.
>
> Signed-off-by: Quentin Perret <[email protected]>
> ---

Reviewed-by: Fuad Tabba <[email protected]>

Thanks,
/fuad

> arch/arm64/include/asm/kvm_asm.h | 2 +-
> arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 1 +
> arch/arm64/kvm/hyp/nvhe/hyp-main.c | 11 +--
> arch/arm64/kvm/hyp/nvhe/mem_protect.c | 88 +++++++++++++++++++
> arch/arm64/kvm/mmu.c | 28 +++++-
> 5 files changed, 118 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> index 432a9ea1f02e..aed2aa61766a 100644
> --- a/arch/arm64/include/asm/kvm_asm.h
> +++ b/arch/arm64/include/asm/kvm_asm.h
> @@ -59,7 +59,7 @@
> #define __KVM_HOST_SMCCC_FUNC___vgic_v3_save_aprs 13
> #define __KVM_HOST_SMCCC_FUNC___vgic_v3_restore_aprs 14
> #define __KVM_HOST_SMCCC_FUNC___pkvm_init 15
> -#define __KVM_HOST_SMCCC_FUNC___pkvm_create_mappings 16
> +#define __KVM_HOST_SMCCC_FUNC___pkvm_host_share_hyp 16
> #define __KVM_HOST_SMCCC_FUNC___pkvm_create_private_mapping 17
> #define __KVM_HOST_SMCCC_FUNC___pkvm_cpu_set_vector 18
> #define __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize 19
> diff --git a/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h b/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h
> index 0118527b07b0..03e604f842e2 100644
> --- a/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h
> +++ b/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h
> @@ -49,6 +49,7 @@ extern struct host_kvm host_kvm;
> extern const u8 pkvm_hyp_id;
>
> int __pkvm_prot_finalize(void);
> +int __pkvm_host_share_hyp(u64 pfn);
>
> bool addr_is_memory(phys_addr_t phys);
> int host_stage2_idmap_locked(phys_addr_t addr, u64 size, enum kvm_pgtable_prot prot);
> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> index 7900d5b66ba3..2da6aa8da868 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> @@ -140,14 +140,11 @@ static void handle___pkvm_cpu_set_vector(struct kvm_cpu_context *host_ctxt)
> cpu_reg(host_ctxt, 1) = pkvm_cpu_set_vector(slot);
> }
>
> -static void handle___pkvm_create_mappings(struct kvm_cpu_context *host_ctxt)
> +static void handle___pkvm_host_share_hyp(struct kvm_cpu_context *host_ctxt)
> {
> - DECLARE_REG(unsigned long, start, host_ctxt, 1);
> - DECLARE_REG(unsigned long, size, host_ctxt, 2);
> - DECLARE_REG(unsigned long, phys, host_ctxt, 3);
> - DECLARE_REG(enum kvm_pgtable_prot, prot, host_ctxt, 4);
> + DECLARE_REG(u64, pfn, host_ctxt, 1);
>
> - cpu_reg(host_ctxt, 1) = __pkvm_create_mappings(start, size, phys, prot);
> + cpu_reg(host_ctxt, 1) = __pkvm_host_share_hyp(pfn);
> }
>
> static void handle___pkvm_create_private_mapping(struct kvm_cpu_context *host_ctxt)
> @@ -185,7 +182,7 @@ static const hcall_t host_hcall[] = {
> HANDLE_FUNC(__vgic_v3_restore_aprs),
> HANDLE_FUNC(__pkvm_init),
> HANDLE_FUNC(__pkvm_cpu_set_vector),
> - HANDLE_FUNC(__pkvm_create_mappings),
> + HANDLE_FUNC(__pkvm_host_share_hyp),
> HANDLE_FUNC(__pkvm_create_private_mapping),
> HANDLE_FUNC(__pkvm_prot_finalize),
> };
> diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> index 2991dc6996b9..8165390d3ec9 100644
> --- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> +++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> @@ -339,6 +339,94 @@ static int host_stage2_idmap(u64 addr)
> return ret;
> }
>
> +static inline bool check_prot(enum kvm_pgtable_prot prot,
> + enum kvm_pgtable_prot required,
> + enum kvm_pgtable_prot denied)
> +{
> + return (prot & (required | denied)) == required;
> +}
> +
> +int __pkvm_host_share_hyp(u64 pfn)
> +{
> + phys_addr_t addr = hyp_pfn_to_phys(pfn);
> + enum kvm_pgtable_prot prot, cur;
> + void *virt = __hyp_va(addr);
> + enum pkvm_page_state state;
> + kvm_pte_t pte;
> + int ret;
> +
> + if (!addr_is_memory(addr))
> + return -EINVAL;
> +
> + hyp_spin_lock(&host_kvm.lock);
> + hyp_spin_lock(&pkvm_pgd_lock);
> +
> + ret = kvm_pgtable_get_leaf(&host_kvm.pgt, addr, &pte, NULL);
> + if (ret)
> + goto unlock;
> + if (!pte)
> + goto map_shared;
> +
> + /*
> + * Check attributes in the host stage-2 PTE. We need the page to be:
> + * - mapped RWX as we're sharing memory;
> + * - not borrowed, as that implies absence of ownership.
> + * Otherwise, we can't let it got through
> + */
> + cur = kvm_pgtable_stage2_pte_prot(pte);
> + prot = pkvm_mkstate(0, PKVM_PAGE_SHARED_BORROWED);
> + if (!check_prot(cur, PKVM_HOST_MEM_PROT, prot)) {
> + ret = -EPERM;
> + goto unlock;
> + }
> +
> + state = pkvm_getstate(cur);
> + if (state == PKVM_PAGE_OWNED)
> + goto map_shared;
> +
> + /*
> + * Tolerate double-sharing the same page, but this requires
> + * cross-checking the hypervisor stage-1.
> + */
> + if (state != PKVM_PAGE_SHARED_OWNED) {
> + ret = -EPERM;
> + goto unlock;
> + }
> +
> + ret = kvm_pgtable_get_leaf(&pkvm_pgtable, (u64)virt, &pte, NULL);
> + if (ret)
> + goto unlock;
> +
> + /*
> + * If the page has been shared with the hypervisor, it must be
> + * already mapped as SHARED_BORROWED in its stage-1.
> + */
> + cur = kvm_pgtable_hyp_pte_prot(pte);
> + prot = pkvm_mkstate(PAGE_HYP, PKVM_PAGE_SHARED_BORROWED);
> + if (!check_prot(cur, prot, ~prot))
> + ret = EPERM;
> + goto unlock;
> +
> +map_shared:
> + /*
> + * If the page is not yet shared, adjust mappings in both page-tables
> + * while both locks are held.
> + */
> + prot = pkvm_mkstate(PAGE_HYP, PKVM_PAGE_SHARED_BORROWED);
> + ret = pkvm_create_mappings_locked(virt, virt + PAGE_SIZE, prot);
> + BUG_ON(ret);
> +
> + prot = pkvm_mkstate(PKVM_HOST_MEM_PROT, PKVM_PAGE_SHARED_OWNED);
> + ret = host_stage2_idmap_locked(addr, PAGE_SIZE, prot);
> + BUG_ON(ret);
> +
> +unlock:
> + hyp_spin_unlock(&pkvm_pgd_lock);
> + hyp_spin_unlock(&host_kvm.lock);
> +
> + return ret;
> +}
> +
> void handle_host_mem_abort(struct kvm_cpu_context *host_ctxt)
> {
> struct kvm_vcpu_fault_info fault;
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 0625bf2353c2..cbab146cda6a 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -259,10 +259,8 @@ static int __create_hyp_mappings(unsigned long start, unsigned long size,
> {
> int err;
>
> - if (!kvm_host_owns_hyp_mappings()) {
> - return kvm_call_hyp_nvhe(__pkvm_create_mappings,
> - start, size, phys, prot);
> - }
> + if (WARN_ON(!kvm_host_owns_hyp_mappings()))
> + return -EINVAL;
>
> mutex_lock(&kvm_hyp_pgd_mutex);
> err = kvm_pgtable_hyp_map(hyp_pgtable, start, size, phys, prot);
> @@ -282,6 +280,21 @@ static phys_addr_t kvm_kaddr_to_phys(void *kaddr)
> }
> }
>
> +static int pkvm_share_hyp(phys_addr_t start, phys_addr_t end)
> +{
> + phys_addr_t addr;
> + int ret;
> +
> + for (addr = ALIGN_DOWN(start, PAGE_SIZE); addr < end; addr += PAGE_SIZE) {
> + ret = kvm_call_hyp_nvhe(__pkvm_host_share_hyp,
> + __phys_to_pfn(addr));
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> /**
> * create_hyp_mappings - duplicate a kernel virtual address range in Hyp mode
> * @from: The virtual kernel start address of the range
> @@ -302,6 +315,13 @@ int create_hyp_mappings(void *from, void *to, enum kvm_pgtable_prot prot)
> if (is_kernel_in_hyp_mode())
> return 0;
>
> + if (!kvm_host_owns_hyp_mappings()) {
> + if (WARN_ON(prot != PAGE_HYP))
> + return -EPERM;
> + return pkvm_share_hyp(kvm_kaddr_to_phys(from),
> + kvm_kaddr_to_phys(to));
> + }
> +
> start = start & PAGE_MASK;
> end = PAGE_ALIGN(end);
>
> --
> 2.32.0.605.g8dce9f2422-goog
>

2021-08-10 08:52:53

by Fuad Tabba

[permalink] [raw]
Subject: Re: [PATCH v4 11/21] KVM: arm64: Allow populating software bits

Hi Quentin,

On Mon, Aug 9, 2021 at 5:25 PM Quentin Perret <[email protected]> wrote:
>
> Introduce infrastructure allowing to manipulate software bits in stage-1
> and stage-2 page-tables using additional entries in the kvm_pgtable_prot
> enum.
>
> This is heavily inspired by Marc's implementation of a similar feature
> in the NV patch series, but adapted to allow stage-1 changes as well:
>
> https://lore.kernel.org/kvmarm/[email protected]/
>
> Suggested-by: Marc Zyngier <[email protected]>
> Signed-off-by: Quentin Perret <[email protected]>
> ---

Reviewed-by: Fuad Tabba <[email protected]>

Thanks,
/fuad

> arch/arm64/include/asm/kvm_pgtable.h | 12 +++++++++++-
> arch/arm64/kvm/hyp/pgtable.c | 5 +++++
> 2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> index 2c090b0eee77..ff9d52f8073a 100644
> --- a/arch/arm64/include/asm/kvm_pgtable.h
> +++ b/arch/arm64/include/asm/kvm_pgtable.h
> @@ -121,6 +121,10 @@ enum kvm_pgtable_stage2_flags {
> * @KVM_PGTABLE_PROT_W: Write permission.
> * @KVM_PGTABLE_PROT_R: Read permission.
> * @KVM_PGTABLE_PROT_DEVICE: Device attributes.
> + * @KVM_PGTABLE_PROT_SW0: Software bit 0.
> + * @KVM_PGTABLE_PROT_SW1: Software bit 1.
> + * @KVM_PGTABLE_PROT_SW2: Software bit 2.
> + * @KVM_PGTABLE_PROT_SW3: Software bit 3.
> */
> enum kvm_pgtable_prot {
> KVM_PGTABLE_PROT_X = BIT(0),
> @@ -128,6 +132,11 @@ enum kvm_pgtable_prot {
> KVM_PGTABLE_PROT_R = BIT(2),
>
> KVM_PGTABLE_PROT_DEVICE = BIT(3),
> +
> + KVM_PGTABLE_PROT_SW0 = BIT(55),
> + KVM_PGTABLE_PROT_SW1 = BIT(56),
> + KVM_PGTABLE_PROT_SW2 = BIT(57),
> + KVM_PGTABLE_PROT_SW3 = BIT(58),
> };
>
> #define KVM_PGTABLE_PROT_RW (KVM_PGTABLE_PROT_R | KVM_PGTABLE_PROT_W)
> @@ -420,7 +429,8 @@ kvm_pte_t kvm_pgtable_stage2_mkold(struct kvm_pgtable *pgt, u64 addr);
> * If there is a valid, leaf page-table entry used to translate @addr, then
> * relax the permissions in that entry according to the read, write and
> * execute permissions specified by @prot. No permissions are removed, and
> - * TLB invalidation is performed after updating the entry.
> + * TLB invalidation is performed after updating the entry. Software bits cannot
> + * be set or cleared using kvm_pgtable_stage2_relax_perms().
> *
> * Return: 0 on success, negative error code on failure.
> */
> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> index e25d829587b9..cff744136044 100644
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
> @@ -357,6 +357,7 @@ static int hyp_set_prot_attr(enum kvm_pgtable_prot prot, kvm_pte_t *ptep)
> attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S1_AP, ap);
> attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S1_SH, sh);
> attr |= KVM_PTE_LEAF_ATTR_LO_S1_AF;
> + attr |= prot & KVM_PTE_LEAF_ATTR_HI_SW;
> *ptep = attr;
>
> return 0;
> @@ -558,6 +559,7 @@ static int stage2_set_prot_attr(struct kvm_pgtable *pgt, enum kvm_pgtable_prot p
>
> attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S2_SH, sh);
> attr |= KVM_PTE_LEAF_ATTR_LO_S2_AF;
> + attr |= prot & KVM_PTE_LEAF_ATTR_HI_SW;
> *ptep = attr;
>
> return 0;
> @@ -1025,6 +1027,9 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr,
> u32 level;
> kvm_pte_t set = 0, clr = 0;
>
> + if (prot & KVM_PTE_LEAF_ATTR_HI_SW)
> + return -EINVAL;
> +
> if (prot & KVM_PGTABLE_PROT_R)
> set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
>
> --
> 2.32.0.605.g8dce9f2422-goog
>

2021-08-11 11:27:22

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v4 00/21] Track shared pages at EL2 in protected mode

On Mon, 9 Aug 2021 16:24:27 +0100, Quentin Perret wrote:
> This is v4 of the patch series previously posted here:
>
> https://lore.kernel.org/lkml/[email protected]/
>
> This series aims to improve how the nVHE hypervisor tracks ownership of
> memory pages when running in protected mode ("kvm-arm.mode=protected" on
> the kernel command line).
>
> [...]

Applied to next, thanks!

[01/21] KVM: arm64: Add hyp_spin_is_locked() for basic locking assertions at EL2
commit: d21292f13f1f0721d60e8122e2db46bea8cf6950
[02/21] KVM: arm64: Introduce hyp_assert_lock_held()
commit: 8e049e0daf23aa380c264e5e15e4c64ea5497ed7
[03/21] KVM: arm64: Provide the host_stage2_try() helper macro
commit: 1bac49d490cbc813f407a5c9806e464bf4a300c9
[05/21] KVM: arm64: Expose page-table helpers
commit: 51add457733bbc4a442fc280d73d14bfe262e4a0
[06/21] KVM: arm64: Optimize host memory aborts
commit: c4f0935e4d957bfcea25ad76860445660a60f3fd
[07/21] KVM: arm64: Rename KVM_PTE_LEAF_ATTR_S2_IGNORED
commit: 178cac08d588e7406a09351a992f57892d8d9cc9
[08/21] KVM: arm64: Don't overwrite software bits with owner id
commit: 8a0282c68121e53ab17413283cfed408a47e1a2a
[09/21] KVM: arm64: Tolerate re-creating hyp mappings to set software bits
commit: b53846c5f279cb5329b82f19a7d313f02cb9d21c
[10/21] KVM: arm64: Enable forcing page-level stage-2 mappings
commit: 5651311941105ca077d3ab74dd4a92e646ecf7fb
[11/21] KVM: arm64: Allow populating software bits
commit: 4505e9b624cefafa4b75d8a28e72f32076c33375
[12/21] KVM: arm64: Add helpers to tag shared pages in SW bits
commit: ec250a67ea8db6209918a389554cf3aec0395b1f
[13/21] KVM: arm64: Expose host stage-2 manipulation helpers
commit: 39257da0e04e5cdb1e4a3ca715dc3d949fe8b059
[14/21] KVM: arm64: Expose pkvm_hyp_id
commit: 2d77e238badb022adb364332b7d6a1d627f77145
[15/21] KVM: arm64: Introduce addr_is_memory()
commit: e009dce1292c37cf8ee7c33e0887ad3c642f980f
[16/21] KVM: arm64: Enable retrieving protections attributes of PTEs
commit: 9024b3d0069ab4b8ef70cf55f0ee09e61f3a0747
[17/21] KVM: arm64: Mark host bss and rodata section as shared
commit: 2c50166c62ba7f3c23c1bbdbb9324db462ddc97b
[18/21] KVM: arm64: Remove __pkvm_mark_hyp
commit: ad0e0139a8e163245d8f44ab4f6ec3bc9b08034d
[19/21] KVM: arm64: Refactor protected nVHE stage-1 locking
commit: f9370010e92638f66473baf342e19de940403362
[20/21] KVM: arm64: Restrict EL2 stage-1 changes in protected mode
commit: 66c57edd3bc79e3527daaae8123f72ecd1e3fa25
[21/21] KVM: arm64: Make __pkvm_create_mappings static
commit: 64a80fb766f9a91e26930bfc56d8e7c12425df12

Note that patch #4 has been used as the base for this series,
and is already part of the mapping level rework.

Cheers,

M.
--
Without deviation from the norm, progress is not possible.