Re: [PATCH v19 10/20] KVM: arm64: Abstract out memory abort handling

[Suzuki K Poulose]

On 22/09/2026 23:38, Jonathan Cameron wrote:
> On Sun, 20 Sep 2026 22:28:35 +0100
> Suzuki K Poulose <suzuki.poulose@xxxxxxx> wrote:
>
> > Move the memory abort handling under VM specific s2 operation.
> >
> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
>
> Trivial comment + request to follow through on adding the local
> variable for kvm. I think the other places that can be used
> survive the other changes in this patch set.
>
> > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> > index d97a4a1bca23f..cf293d09e940a 100644
> > --- a/arch/arm64/kvm/mmu.c
> > +++ b/arch/arm64/kvm/mmu.c
>
>
> > +static int kvm_vm_mem_abort(const struct kvm_s2_fault_desc *s2fd)
> > +{
> > +	int ret;
> > +	struct kvm_vcpu *vcpu = s2fd->vcpu;
> > +
> > +	VM_WARN_ON_ONCE(kvm_vcpu_trap_is_permission_fault(vcpu) &&
> > +			!kvm_is_write_fault(vcpu) &&
> > +			!kvm_vcpu_trap_is_exec_fault(vcpu));
> > +
> > +	if (kvm_slot_has_gmem(s2fd->memslot))
> > +		ret = gmem_abort(s2fd);
> > +	else
> > +		ret = user_mem_abort(s2fd);
> > +	return ret;
>
> Personal preference but I'd just do
> 		return gmem_abort(s2fd);
> 	else
> 		return user_mem_abort(s2fd);

Ack

> and save a couple of lines. I'd keep the else to make it clear it is
> an either / or choice rather than an if something true one.
>
> > +}
> > +
> >   /* Resolve the access fault by making the page young again. */
> >   static void handle_access_fault(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa)
> >   {
> > @@ -2287,6 +2303,7 @@ int kvm_handle_guest_sea(struct kvm_vcpu *vcpu)
> >   int kvm_handle_guest_abort(struct kvm_vcpu *vcpu)
> >   {
> >   	struct kvm_s2_trans nested_trans, *nested = NULL;
> > +	struct kvm *kvm = vcpu->kvm;
>
> If introducing this, also use it for the other places vcpu->kvm is used
> in this function.  Separate patch though to keep down noise.
> Looks like that's mostly to shorten the chain of dereferences in the
> locks.

Added a preparatory patch to convert the other users to a local "kvm"
variable.

Cheers
Suzuki