On 7/6/21 7:36 PM, Marc Zyngier wrote:
On Tue, 06 Jul 2021 15:52:46 +0100,
Marc Zyngier <maz@xxxxxxxxxx> 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.
Please try this[1] for size, which is on top of Linus' tree as of this
morning.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/pmu/reset-values
I have tried [1] and it works fine. Also tested with my patch on top (without
masking) and it also works fine.
Thanks,
alex.
