From a508d5afb70894ab50ccc4678f55ff801468182b Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Tue, 1 Jul 2025 16:16:47 +0100 Subject: [PATCH] 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 a25be111cd8f8..a26e377a36171 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) { -- 2.47.2