On 2020-11-26 15:54, Alexandru Elisei wrote:
Hi Marc,
On 11/26/20 3:34 PM, Marc Zyngier wrote:
Hi Alex,
On 2020-11-26 15:18, Alexandru Elisei wrote:
Hi Marc,
I checked and indeed the remaining cases cover all registers that use
this accessor.
However, I'm a bit torn here. The warning that I got when trying to
run a guest
with the PMU feature flag set, but not initialized (reported at [1])
was also not
supposed to ever be reached:
static u32 kvm_pmu_event_mask(struct kvm *kvm)
{
switch (kvm->arch.pmuver) {
case 1: /* ARMv8.0 */
return GENMASK(9, 0);
case 4: /* ARMv8.1 */
case 5: /* ARMv8.4 */
case 6: /* ARMv8.5 */
return GENMASK(15, 0);
default: /* Shouldn't be here, just for sanity */
WARN_ONCE(1, "Unknown PMU version %d\n", kvm->arch.pmuver);
return 0;
}
}
I realize it's not exactly the same thing and I'll leave it up to you
if you want
to add a warning for the cases that should never happen. I'm fine
either way:
I already have queued such a warning[1]. It turns out that LLVM warns
idx can be left uninitialized, and shouts. Let me know if that works
for you.
Looks good to me, unsigned long is 64 bits and instructions are 32
bits, so we'll never run into a situation where a valid encoding is
~0UL.
You can add my Reviewed-by to this patch (and to the one at [1] if it's
still
possible).
It's a fixup, so it will get folded into the original patch.
Thanks for spending time reviewing (and fixing) this!
M.
--
Jazz is not dead. It just smells funny...
