On 7/6/21 4:52 PM, Marc Zyngier wrote:
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.
Right, I meant I will resubmit after -rc1 as you requested.
Thanks,
alex.
