On Thu, Sep 17, 2020 at 08:00:56PM +0800, Peng Liang wrote:
> To emulate ID registers, we need to validate the value of the register
> defined by user space. For most ID registers, we need to check whether
> each field defined by user space is no more than that of host (whether
> host support the corresponding features) and whether the fields are
> supposed to be exposed to guest. Introduce check_features to do those
> jobs.
>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@xxxxxxxxxx>
> Signed-off-by: Peng Liang <liangpeng10@xxxxxxxxxx>
> ---
> arch/arm64/include/asm/cpufeature.h | 2 ++
> arch/arm64/kernel/cpufeature.c | 23 +++++++++++++++++++++++
> 2 files changed, 25 insertions(+)
>
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index 2ba7c4f11d8a..954adc5ca72f 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -579,6 +579,8 @@ void check_local_cpu_capabilities(void);
>
> u64 read_sanitised_ftr_reg(u32 id);
>
> +int check_features(u32 sys_reg, u64 val);
> +
> static inline bool cpu_supports_mixed_endian_el0(void)
> {
> return id_aa64mmfr0_mixed_endian_el0(read_cpuid(ID_AA64MMFR0_EL1));
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 698b32705544..e58926992a70 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -2850,3 +2850,26 @@ ssize_t cpu_show_meltdown(struct device *dev, struct device_attribute *attr,
>
> return sprintf(buf, "Vulnerable\n");
> }
> +
> +int check_features(u32 sys_reg, u64 val)
> +{
> + struct arm64_ftr_reg *reg = get_arm64_ftr_reg(sys_reg);
> + const struct arm64_ftr_bits *ftrp;
> + u64 exposed_mask = 0;
> +
> + if (!reg)
> + return -ENOENT;
> +
> + for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) {
> + if (arm64_ftr_value(ftrp, reg->sys_val) <
> + arm64_ftr_value(ftrp, val)) {
> + return -EINVAL;
This assumes that 0b1111 is invalid if the host has e.g. 0b0001,
but, IIRC, there are some ID registers where 0b1111 means the
feature is disabled.
> + }
> + exposed_mask |= arm64_ftr_mask(ftrp);
> + }
> +
> + if (val & ~exposed_mask)
> + return -EINVAL;
> +
> + return 0;
> +}
> --
> 2.26.2
>
I don't think we should be trying to do the verification at the ftr_bits
level, at least not generally. Trying to handle all ID registers the
same way is bound to fail, for the 0b1111 vs. 0b0000 reason pointed
out above, and probably other reasons. As I stated before, we should be
validating each feature of each ID register on a case by case basis,
and we should be using higher level CPU feature checking APIs to get
that right.
Also, what about validating that all VCPUs have consistent features
exposed? Each VCPU could select a valid feature mask by this check,
but different ones, which will obviously create a completely broken
guest.
Thanks,
drew
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
