Re: [RFC PATCH 05/17] KVM: arm64: Take an argument to indicate parallel walk

[Marc Zyngier <maz@kernel.org>]

Hi Oliver,

On Fri, 15 Apr 2022 22:58:49 +0100,
Oliver Upton <oupton@google.com> wrote:
> 
> It is desirable to reuse the same page walkers for serial and parallel
> faults. Take an argument to kvm_pgtable_walk() (and throughout) to
> indicate whether or not a walk might happen in parallel with another.
>
> No functional change intended.
> 
> Signed-off-by: Oliver Upton <oupton@google.com>
> ---
>  arch/arm64/include/asm/kvm_pgtable.h  |  5 +-
>  arch/arm64/kvm/hyp/nvhe/mem_protect.c |  4 +-
>  arch/arm64/kvm/hyp/nvhe/setup.c       |  4 +-
>  arch/arm64/kvm/hyp/pgtable.c          | 91 ++++++++++++++-------------
>  4 files changed, 54 insertions(+), 50 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> index ea818a5f7408..74955aba5918 100644
> --- a/arch/arm64/include/asm/kvm_pgtable.h
> +++ b/arch/arm64/include/asm/kvm_pgtable.h
> @@ -194,7 +194,7 @@ enum kvm_pgtable_walk_flags {
>  typedef int (*kvm_pgtable_visitor_fn_t)(u64 addr, u64 end, u32 level,
>  					kvm_pte_t *ptep, kvm_pte_t *old,
>  					enum kvm_pgtable_walk_flags flag,
> -					void * const arg);
> +					void * const arg, bool shared);

Am I the only one who find this really ugly? Sprinkling this all over
the shop makes the code rather unreadable. It seems to me that having
some sort of more general context would make more sense.

For example, I would fully expect the walk context to tell us whether
this walker is willing to share its walk. Add a predicate to that,
which would conveniently expand to 'false' for contexts where we don't
have RCU (such as the pKVM HYP PT management, and you should get
something that is more manageable.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm