On 30.11.2021 22:18, Andrew Cooper wrote:
> On 29/11/2021 09:10, Jan Beulich wrote:
>> --- a/xen/arch/x86/cpu/vpmu.c
>> +++ b/xen/arch/x86/cpu/vpmu.c
>> @@ -480,12 +470,17 @@ static int vpmu_arch_initialise(struct v
>> return -EINVAL;
>> }
>>
>> - vpmu->hw_lapic_lvtpc = PMU_APIC_VECTOR | APIC_LVT_MASKED;
>> -
>> + ret = alternative_call(vpmu_ops.initialise, v);
>> if ( ret )
>> + {
>> printk(XENLOG_G_WARNING "VPMU: Initialization failed for %pv\n", v);
>> + return ret;
>> + }
>> +
>> + vpmu->hw_lapic_lvtpc = PMU_APIC_VECTOR | APIC_LVT_MASKED;
>> + vpmu_set(vpmu, VPMU_INITIALIZED);
>
> It occurs to me that if, in previous patch, you do:
>
> if ( ret )
> printk(XENLOG_G_WARNING "VPMU: Initialization failed for %pv\n", v);
> + else
> + vpmu_set(vpmu, VPMU_INITIALIZED);
>
> then you don't need to undo the adjustments in
> {svm,vmx}_vpmu_initialise() in this patch.
I actually had it that way first, until noticing it was wrong. It can
be done only here because it if only here where the XENPMU_MODE_OFF
checks move from the vendor functions into here.
> Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>, although
> preferably with the VPMU_INITIALIZED adjustment.
Thanks; as said, that adjustment can't be done in patch 1 just yet.
But I did add the missing trailing commas.
Jan
