On 13.11.2025 13:20, Alejandro Vallejo wrote:
> On Thu Nov 13, 2025 at 12:42 PM CET, Jan Beulich wrote:
>> On 12.11.2025 16:22, Alejandro Vallejo wrote:
>>> --- a/xen/arch/x86/cpuid.c
>>> +++ b/xen/arch/x86/cpuid.c
>>> @@ -297,7 +297,7 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
>>> if ( v->arch.hvm.guest_cr[4] & X86_CR4_OSXSAVE )
>>> res->c |= cpufeat_mask(X86_FEATURE_OSXSAVE);
>>> }
>>> - else /* PV domain */
>>> + else if ( IS_ENABLED(CONFIG_PV) )
>>> {
>>> regs = guest_cpu_user_regs();
>>>
>>> @@ -509,7 +509,7 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
>>> if ( !hap_enabled(d) && !hvm_pae_enabled(v) )
>>> res->d &= ~cpufeat_mask(X86_FEATURE_PSE36);
>>> }
>>> - else /* PV domain */
>>> + else if ( IS_ENABLED(CONFIG_PV) )
>>
>> Maybe better leave the "else"-s as is and, ahead of them, insert
>>
>> else if ( !IS_ENABLED(CONFIG_PV) )
>> ASSERT_UNREACHABLE();
>>
>> Happy to make the adjustment while committing, provided you'd be happy with
>> me
>> doing so.
>
> Should I understand that as an Acked-by?
You may, yes (implicitly).
Jan
> I think it'd be marginally clearer with the assert added to my code as an else
> branch at the end, but either form works. I'm fine with it being committed
> in the form I originally sent, what you proposed, or the ASSERT as an else
> branch.
>
> They all have the same effect, after all.
>
> Cheers,
> Alejandro
