From 4530256f3699a053f0b9dc677e231d570bbd0eea Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Mon, 23 Jun 2025 15:22:52 +0200 Subject: KVM: arm64: vgic-its: Return -ENXIO to invalid KVM_DEV_ARM_VGIC_GRP_CTRL attrs A preliminary version of a hack to invoke unmap_all_vpes() from an ioctl didn't work very well. We eventually determined this was because we were invoking it on the wrong file descriptor, but not getting an error. Signed-off-by: David Woodhouse Reviewed-by: Eric Auger Link: https://lore.kernel.org/r/bbbddd56135399baf699bc46ffb6e7f08d9f8c9f.camel@infradead.org Signed-off-by: Oliver Upton --- arch/arm64/kvm/vgic/vgic-its.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c index 534049c7c94b..b34f8976c9cc 100644 --- a/arch/arm64/kvm/vgic/vgic-its.c +++ b/arch/arm64/kvm/vgic/vgic-its.c @@ -2694,6 +2694,9 @@ static int vgic_its_ctrl(struct kvm *kvm, struct vgic_its *its, u64 attr) case KVM_DEV_ARM_ITS_RESTORE_TABLES: ret = abi->restore_tables(its); break; + default: + ret = -ENXIO; + break; } mutex_unlock(&its->its_lock); -- cgit v1.2.3 From a508d5afb70894ab50ccc4678f55ff801468182b Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Tue, 1 Jul 2025 16:16:47 +0100 Subject: KVM: arm64: Remove the wi->{e0,}poe vs wr->{p,u}ov confusion Some of the POE computation is a bit confused. Specifically, there is an element of confusion between what wi->{e0,}poe an wr->{p,u}ov actually represent. - wi->{e0,}poe is an *input* to the walk, and indicates whether POE is enabled at EL0 or EL{1,2} - wr->{p,u}ov is a *result* of the walk, and indicates whether overlays are enabled. Crutially, it is possible to have POE enabled, and yet overlays disabled, while the converse isn't true What this all means is that once the base permissions have been established, checking for wi->{e0,}poe makes little sense, because the truth about overlays resides in wr->{p,u}ov. So constructs checking for (wi->poe && wr->pov) only add perplexity. Refactor compute_s1_overlay_permissions() and the way it is called according to the above principles. Take the opportunity to avoid reading registers that are not strictly required. Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20250701151648.754785-2-maz@kernel.org Signed-off-by: Oliver Upton --- arch/arm64/kvm/at.c | 52 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c index a25be111cd8f..a26e377a3617 100644 --- a/arch/arm64/kvm/at.c +++ b/arch/arm64/kvm/at.c @@ -1047,34 +1047,43 @@ static void compute_s1_overlay_permissions(struct kvm_vcpu *vcpu, idx = FIELD_GET(PTE_PO_IDX_MASK, wr->desc); - switch (wi->regime) { - case TR_EL10: - pov_perms = perm_idx(vcpu, POR_EL1, idx); - uov_perms = perm_idx(vcpu, POR_EL0, idx); - break; - case TR_EL20: - pov_perms = perm_idx(vcpu, POR_EL2, idx); - uov_perms = perm_idx(vcpu, POR_EL0, idx); - break; - case TR_EL2: - pov_perms = perm_idx(vcpu, POR_EL2, idx); - uov_perms = 0; - break; - } + if (wr->pov) { + switch (wi->regime) { + case TR_EL10: + pov_perms = perm_idx(vcpu, POR_EL1, idx); + break; + case TR_EL20: + pov_perms = perm_idx(vcpu, POR_EL2, idx); + break; + case TR_EL2: + pov_perms = perm_idx(vcpu, POR_EL2, idx); + break; + } - if (pov_perms & ~POE_RWX) - pov_perms = POE_NONE; + if (pov_perms & ~POE_RWX) + pov_perms = POE_NONE; - if (wi->poe && wr->pov) { wr->pr &= pov_perms & POE_R; wr->pw &= pov_perms & POE_W; wr->px &= pov_perms & POE_X; } - if (uov_perms & ~POE_RWX) - uov_perms = POE_NONE; + if (wr->uov) { + switch (wi->regime) { + case TR_EL10: + uov_perms = perm_idx(vcpu, POR_EL0, idx); + break; + case TR_EL20: + uov_perms = perm_idx(vcpu, POR_EL0, idx); + break; + case TR_EL2: + uov_perms = 0; + break; + } + + if (uov_perms & ~POE_RWX) + uov_perms = POE_NONE; - if (wi->e0poe && wr->uov) { wr->ur &= uov_perms & POE_R; wr->uw &= uov_perms & POE_W; wr->ux &= uov_perms & POE_X; @@ -1095,8 +1104,7 @@ static void compute_s1_permissions(struct kvm_vcpu *vcpu, if (!wi->hpd) compute_s1_hierarchical_permissions(vcpu, wi, wr); - if (wi->poe || wi->e0poe) - compute_s1_overlay_permissions(vcpu, wi, wr); + compute_s1_overlay_permissions(vcpu, wi, wr); /* R_QXXPC */ if (wr->pwxn) { -- cgit v1.2.3 From 5152977340b6dc54e7c8cc8fe401e89cfa3e6f94 Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Tue, 1 Jul 2025 16:16:48 +0100 Subject: KVM: arm64: Follow specification when implementing WXN The R_QXXPC and R_NPBXC rules have some interesting (and pretty sharp) corners when defining the behaviour of of WXN at S1: - when S1 overlay is enabled, WXN applies to the overlay and will remove W - when S1 overlay is disabled, WXN applies to the base permissions and will remove X. Today, we lumb the two together in a way that doesn't really match the rules, making things awkward to follow what is happening, in particular when overlays are enabled. Split these two rules over two distinct paths, which makes things a lot easier to read and validate against the architecture rules. Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20250701151648.754785-3-maz@kernel.org Signed-off-by: Oliver Upton --- arch/arm64/kvm/at.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c index a26e377a3617..0e5610533949 100644 --- a/arch/arm64/kvm/at.c +++ b/arch/arm64/kvm/at.c @@ -1063,6 +1063,10 @@ static void compute_s1_overlay_permissions(struct kvm_vcpu *vcpu, if (pov_perms & ~POE_RWX) pov_perms = POE_NONE; + /* R_QXXPC, S1PrivOverflow enabled */ + if (wr->pwxn && (pov_perms & POE_X)) + pov_perms &= ~POE_W; + wr->pr &= pov_perms & POE_R; wr->pw &= pov_perms & POE_W; wr->px &= pov_perms & POE_X; @@ -1084,6 +1088,10 @@ static void compute_s1_overlay_permissions(struct kvm_vcpu *vcpu, if (uov_perms & ~POE_RWX) uov_perms = POE_NONE; + /* R_NPBXC, S1UnprivOverlay enabled */ + if (wr->uwxn && (uov_perms & POE_X)) + uov_perms &= ~POE_W; + wr->ur &= uov_perms & POE_R; wr->uw &= uov_perms & POE_W; wr->ux &= uov_perms & POE_X; @@ -1106,21 +1114,13 @@ static void compute_s1_permissions(struct kvm_vcpu *vcpu, compute_s1_overlay_permissions(vcpu, wi, wr); - /* R_QXXPC */ - if (wr->pwxn) { - if (!wr->pov && wr->pw) - wr->px = false; - if (wr->pov && wr->px) - wr->pw = false; - } + /* R_QXXPC, S1PrivOverlay disabled */ + if (!wr->pov) + wr->px &= !(wr->pwxn && wr->pw); - /* R_NPBXC */ - if (wr->uwxn) { - if (!wr->uov && wr->uw) - wr->ux = false; - if (wr->uov && wr->ux) - wr->uw = false; - } + /* R_NPBXC, S1UnprivOverlay disabled */ + if (!wr->uov) + wr->ux &= !(wr->uwxn && wr->uw); pan = wi->pan && (wr->ur || wr->uw || (pan3_enabled(vcpu, wi->regime) && wr->ux)); -- cgit v1.2.3 From 30a597e19f24dc5264954456fe749e26ac67478a Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Fri, 18 Jul 2025 01:50:24 +0000 Subject: arm64: kvm: sys_regs: use string choices helper We can use string choices helper, let's use it. Signed-off-by: Kuninori Morimoto Link: https://lore.kernel.org/r/87pldy5ktb.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Oliver Upton --- arch/arm64/kvm/sys_regs.c | 2 +- arch/arm64/kvm/sys_regs.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 76c2f0da821f..c131d6b0d35b 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -4475,7 +4475,7 @@ static bool kvm_esr_cp10_id_to_sys64(u64 esr, struct sys_reg_params *params) return true; kvm_pr_unimpl("Unhandled cp10 register %s: %u\n", - params->is_write ? "write" : "read", reg_id); + str_write_read(params->is_write), reg_id); return false; } diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h index ef97d9fc67cc..317abc490368 100644 --- a/arch/arm64/kvm/sys_regs.h +++ b/arch/arm64/kvm/sys_regs.h @@ -108,7 +108,7 @@ inline void print_sys_reg_msg(const struct sys_reg_params *p, /* Look, we even formatted it for you to paste into the table! */ kvm_pr_unimpl("%pV { Op0(%2u), Op1(%2u), CRn(%2u), CRm(%2u), Op2(%2u), func_%s },\n", &(struct va_format){ fmt, &va }, - p->Op0, p->Op1, p->CRn, p->CRm, p->Op2, p->is_write ? "write" : "read"); + p->Op0, p->Op1, p->CRn, p->CRm, p->Op2, str_write_read(p->is_write)); va_end(va); } -- cgit v1.2.3 From f509de1b0f8959ae4d90c6eeb13dee145fbb0127 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Fri, 18 Jul 2025 01:50:38 +0000 Subject: arm64: kvm: trace_handle_exit: use string choices helper We can use string choices helper, let's use it. Signed-off-by: Kuninori Morimoto Link: https://lore.kernel.org/r/87o6ti5ksx.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Oliver Upton --- arch/arm64/kvm/trace_handle_exit.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/kvm/trace_handle_exit.h b/arch/arm64/kvm/trace_handle_exit.h index f85415db7713..a7ab9a3bbed0 100644 --- a/arch/arm64/kvm/trace_handle_exit.h +++ b/arch/arm64/kvm/trace_handle_exit.h @@ -113,7 +113,7 @@ TRACE_EVENT(kvm_sys_access, __entry->vcpu_pc, __entry->name ?: "UNKN", __entry->Op0, __entry->Op1, __entry->CRn, __entry->CRm, __entry->Op2, - __entry->is_write ? "write" : "read") + str_write_read(__entry->is_write)) ); TRACE_EVENT(kvm_set_guest_debug, -- cgit v1.2.3