On Thu, 25 Jul 2024 15:16:12 +0100,
Alexandru Elisei <alexandru.elisei@xxxxxxx> wrote:
>
> Hi Marc,
>
> On Mon, Jul 08, 2024 at 05:57:58PM +0100, Marc Zyngier wrote:
> > + if (perm_fail) {
> > + struct s1_walk_result tmp;
>
> I was wondering if you would consider initializing 'tmp' to the empty struct
> here. That makes it consistent with the initialization of 'wr' in the !perm_fail
> case and I think it will make the code more robust wrt to changes to
> compute_par_s1() and what fields it accesses.
I think there is a slightly better way, with something like this:
diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
index b02d8dbffd209..36fa2801ab4ef 100644
--- a/arch/arm64/kvm/at.c
+++ b/arch/arm64/kvm/at.c
@@ -803,12 +803,12 @@ static u64 handle_at_slow(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)
}
if (perm_fail) {
- struct s1_walk_result tmp;
-
- tmp.failed = true;
- tmp.fst = ESR_ELx_FSC_PERM | wr.level;
- tmp.s2 = false;
- tmp.ptw = false;
+ struct s1_walk_result tmp = (struct s1_walk_result){
+ .failed = true,
+ .fst = ESR_ELx_FSC_PERM | wr.level,
+ .s2 = false,
+ .ptw = false,
+ };
wr = tmp;
}
Thoughts?
M.
--
Without deviation from the norm, progress is not possible.
