On Sun, 02 Apr 2023 19:37:34 +0100,
Jing Zhang <jingzhangos@xxxxxxxxxx> wrote:
>
> Introduce an ID feature register specific descriptor to include ID
> register specific fields and callbacks besides its corresponding
> general system register descriptor.
>
> No functional change intended.
>
> Co-developed-by: Reiji Watanabe <reijiw@xxxxxxxxxx>
> Signed-off-by: Reiji Watanabe <reijiw@xxxxxxxxxx>
> Signed-off-by: Jing Zhang <jingzhangos@xxxxxxxxxx>
> ---
> arch/arm64/kvm/id_regs.c | 233 ++++++++++++++++++++++++++++----------
> arch/arm64/kvm/sys_regs.c | 2 +-
> arch/arm64/kvm/sys_regs.h | 1 +
> 3 files changed, 178 insertions(+), 58 deletions(-)
>
> diff --git a/arch/arm64/kvm/id_regs.c b/arch/arm64/kvm/id_regs.c
> index e92eacb0ad32..af86001e2686 100644
> --- a/arch/arm64/kvm/id_regs.c
> +++ b/arch/arm64/kvm/id_regs.c
> @@ -18,6 +18,27 @@
>
> #include "sys_regs.h"
>
> +struct id_reg_desc {
> + const struct sys_reg_desc reg_desc;
> + /*
> + * ftr_bits points to the feature bits array defined in cpufeature.c for
> + * writable CPU ID feature register.
> + */
> + const struct arm64_ftr_bits *ftr_bits;
Why do we need to keep this around? we already have all the required
infrastructure to lookup a full arm64_ftr_reg. So why the added stuff?
> + /*
> + * Only bits with 1 are writable from userspace.
> + * This mask might not be necessary in the future whenever all ID
> + * registers are enabled as writable from userspace.
> + */
> + const u64 writable_mask;
> + /*
> + * This function returns the KVM sanitised register value.
> + * The value would be the same as the host kernel sanitised value if
> + * there is no KVM sanitisation for this id register.
> + */
> + u64 (*read_kvm_sanitised_reg)(const struct id_reg_desc *idr);
Why can't this function return both the required value and a mask?
Why can't it live in the sys_reg_desc structure?
Frankly, I don't see a good reason to have this wrapper structure
which makes things pointlessly complicated and prevent code sharing
with the rest of the infrastructure.
M.
--
Without deviation from the norm, progress is not possible.
