Subject: Re: [PATCH 6/8] KVM: arm64: Remove dead PMU sysreg decoding code

From: Marc Zyngier <[email-masked]>
Date: Thu, 26 Nov 2020 15:57:30 +0000

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...

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel