On Thu, Mar 09, 2017 at 04:52:18AM -0800, Christoffer Dall wrote:
> On Mon, Jan 16, 2017 at 05:33:34PM +0800, Shannon Zhao wrote:
> > From: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
> >
> > Check if the configuration is fine.
>
> This commit message really needs some love and attention.
>
> >
> > Signed-off-by: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
> > ---
> > arch/arm64/kvm/sys_regs.c | 32 +++++++++++++++++++++++++++++++-
> > 1 file changed, 31 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index f613e29..9763b79 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -1493,6 +1493,35 @@ static bool access_id_reg(struct kvm_vcpu *vcpu,
> > return true;
> > }
> >
> > +static int set_id_aa64mmfr0_el1(struct kvm_vcpu *vcpu,
> > + const struct sys_reg_desc *rd,
> > + const struct kvm_one_reg *reg,
> > + void __user *uaddr)
> > +{
> > + u64 val, id_aa64mmfr0;
> > +
> > + if (copy_from_user(&val, uaddr, KVM_REG_SIZE(reg->id)) != 0)
> > + return -EFAULT;
> > +
> > + asm volatile("mrs %0, id_aa64mmfr0_el1\n" : "=r" (id_aa64mmfr0));
>
> Doesn't the kernel have an abstraction for this already or a cached
> value?
Certainly we shouldn't be using a raw mrs instruction. We have
read_sysreg() or read_cpuid() for that.
The cpufeature code has a cached, system-wide safe value cached for each
system register. The cpuid_feature_extract_field() helper uses that.
> > + if ((val & GENMASK(3, 0)) > (id_aa64mmfr0 & GENMASK(3, 0)) ||
> > + (val & GENMASK(7, 4)) > (id_aa64mmfr0 & GENMASK(7, 4)) ||
> > + (val & GENMASK(11, 8)) > (id_aa64mmfr0 & GENMASK(11, 8)) ||
> > + (val & GENMASK(15, 12)) > (id_aa64mmfr0 & GENMASK(15, 12)) ||
> > + (val & GENMASK(19, 16)) > (id_aa64mmfr0 & GENMASK(19, 16)) ||
> > + (val & GENMASK(23, 20)) > (id_aa64mmfr0 & GENMASK(23, 20)) ||
> > + (val & GENMASK(27, 24)) < (id_aa64mmfr0 & GENMASK(27, 24)) ||
> > + (val & GENMASK(31, 28)) < (id_aa64mmfr0 & GENMASK(31, 28))) {
Please use mnemonics. For example, we have ID_AA64MMFR0_TGRAN*_SHIFT
defined in <asm/sysreg.h>.
We also have extraction helpers, see
cpuid_feature_extract_unsigned_field(), as used in
id_aa64mmfr0_mixed_endian_el0().
Thanks,
Mark.
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
