On Tue, 06 Jul 2021 14:50:35 +0100, Alexandre Chartre <alexandre.chartre@xxxxxxxxxx> wrote: > > > Hi Marc, > > On 6/29/21 3:16 PM, Alexandre Chartre wrote: > > On 6/29/21 11:06 AM, Marc Zyngier wrote > > [...] > >> So the sysreg is the only thing we should consider, and I think we > >> should drop the useless masking. There is at least another instance of > >> this in the PMU code (kvm_pmu_overflow_status()), and apart from > >> kvm_pmu_vcpu_reset(), only the sysreg accessors should care about the > >> masking to sanitise accesses. > >> > >> What do you think? > >> > > > > I think you are right. PMCNTENSET_EL0 is already masked with kvm_pmu_valid_counter_mask() > > so there's effectively no need to mask it again when we use it. I will send an additional > > patch (on top of this one) to remove useless masking. Basically, changes would be: > > I had a closer look and we can't remove the mask. The access > functions (for pmcnten, pminten, pmovs), clear or set only the > specified valid counter bits. This means that bits other than the > valid counter bits never change in __vcpu_sys_reg(), and those bits > are not necessarily zero because the initial value is > 0x1de7ec7edbadc0deULL (set by reset_unknown()). That's a bug that should be fixed on its own. Bits that are RAZ/WI in the architecture shouldn't be kept in the shadow registers the first place. I'll have a look. > So I will resubmit initial patch, with just the commit message > changes. Please don't. I'm not papering over this kind of bug. Thanks, M. -- Without deviation from the norm, progress is not possible.
