Re: [PATCH 2/3] x86/vPMU: invoke <vendor>_vpmu_initialise() through a hook as well

[Andrew Cooper <amc96@xxxxxxxx>]

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 <andrew.cooper3@xxxxxxxxxx>, 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