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).
Thanks,
Alex
>
> Thanks,
>
> M.
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/commit/?h=kvm-arm64/pmu-undef&id=af7eff70eaf8f28179334f5aeabb70a306242c83
