On 01/12/2021 08:31, Jan Beulich wrote:
> 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.
Oh ok. Never mind then.
>> Reviewed-by: Andrew Cooper <[email protected]>, 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.
Thanks.
~Andrew
