Hi Marc,
On Sat, May 28, 2022 at 4:38 AM Marc Zyngier <maz@xxxxxxxxxx> wrote:
>
> Careful analysis of the vcpu flags show that this is a mix of
> configuration, communication between the host and the hypervisor,
> as well as anciliary state that has no consistency. It'd be a lot
> better if we could split these flags into consistent categories.
>
> However, even if we split these flags apart, we want to make sure
> that each flag can only be applied to its own set, and not across
> sets.
>
> To achieve this, use a preprocessor hack so that each flag is always
> associated with:
>
> - the set that contains it,
>
> - a mask that describe all the bits that contain it (for a simple
> flag, this is the same thing as the flag itself, but we will
> eventually have values that cover multiple bits at once).
>
> Each flag is thus a triplet that is not directly usable as a value,
> but used by three helpers that allow the flag to be set, cleared,
> and fetched. By mandating the use of such helper, we can easily
> enforce that a flag can only be used with the set it belongs to.
>
> Finally, one last helper "unpacks" the raw value from the triplet
> that represents a flag, which is useful for multi-bit values that
> need to be enumerated (in a switch statement, for example).
>
> Further patches will start making use of this infrastructure.
>
> Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
> ---
> arch/arm64/include/asm/kvm_host.h | 33 +++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index a46f952b97f6..5eb6791df608 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -418,6 +418,39 @@ struct kvm_vcpu_arch {
> } steal;
> };
>
> +#define __vcpu_get_flag(v, flagset, f, m) \
> + ({ \
> + v->arch.flagset & (m); \
> + })
> +
> +#define __vcpu_set_flag(v, flagset, f, m) \
> + do { \
> + typeof(v->arch.flagset) *fset; \
> + \
> + fset = &v->arch.flagset; \
> + if (HWEIGHT(m) > 1) \
> + *fset &= ~(m); \
> + *fset |= (f); \
> + } while (0)
> +
> +#define __vcpu_clear_flag(v, flagset, f, m) \
> + do { \
> + typeof(v->arch.flagset) *fset; \
> + \
> + fset = &v->arch.flagset; \
> + *fset &= ~(m); \
> + } while (0)
I think 'v' should be enclosed in parentheses in those three macros.
> +
> +#define vcpu_get_flag(v, ...) __vcpu_get_flag(v, __VA_ARGS__)
> +#define vcpu_set_flag(v, ...) __vcpu_set_flag(v, __VA_ARGS__)
> +#define vcpu_clear_flag(v, ...) __vcpu_clear_flag(v, __VA_ARGS__)
> +
> +#define __vcpu_single_flag(_set, _f) _set, (_f), (_f)
> +
> +#define __flag_unpack(_set, _f, _m) _f
Nit: Probably it might be worth adding a comment that explains the
above two macros ? (e.g. what is each element of the triplets ?)
> +#define vcpu_flag_unpack(...) __flag_unpack(__VA_ARGS__)
Minor nit: KVM Functions and macros whose names begin with "vcpu_"
make me think that they are the operations for a vCPU specified in
the argument, but this macro is not (this might just my own
assumption?). So, IMHO I would prefer a name whose prefix is not
"vcpu_". Having said that, I don't have any good suggestions though...
Perhaps I might prefer "unpack_vcpu_flag" a bit instead?
Thanks,
Reiji
> +
> +
> /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */
> #define vcpu_sve_pffr(vcpu) (kern_hyp_va((vcpu)->arch.sve_state) + \
> sve_ffr_offset((vcpu)->arch.sve_max_vl))
> --
> 2.34.1
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@xxxxxxxxxxxxxxxxxxxxx
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
