Re: [Xen-devel] [PATCH] SVM: correct CPUID event processing

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.09.2019 10:39, Alexandru Stefan ISAILA wrote:
> 
> 
> On 19.09.2019 13:37, Jan Beulich wrote:
>> hvm_monitor_cpuid() expects the input registers, not two of the outputs.
>>
>> However, once having made the necessary adjustment, the SVM and VMX
>> functions are so similar that they should be folded (thus avoiding
>> further similar asymmetries to get introduced). Use the best of both
>> worlds by e.g. using "curr" consistently. This then being the only
>> caller of hvm_check_cpuid_faulting(), fold in that function as well.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Reviewed-by: Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>

Thanks.

>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -3349,14 +3349,28 @@ unsigned long copy_from_user_hvm(void *t
>>       return rc ? len : 0; /* fake a copy_from_user() return code */
>>   }
>>
> 
> Useful fold. Maybe a small comment with the reason to have one function 
> would help. Something like both SVM and VMX do the same thing, but it's 
> up to you if it is necessary.
> 
>> -bool hvm_check_cpuid_faulting(struct vcpu *v)
>> +int hvm_vmexit_cpuid(struct cpu_user_regs *regs, unsigned int inst_len)

The patch description explicitly says so. Since having common code
for things not differing between vendors is the rule rather than
an exception, commenting individual functions to this effect would
seem rather odd to me.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel