On 12.11.2025 16:56, Grygorii Strashko wrote:
> On 12.11.25 17:22, Alejandro Vallejo wrote:
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -571,15 +571,17 @@ int arch_vcpu_create(struct vcpu *v)
>>   
>>       if ( is_hvm_domain(d) )
>>           rc = hvm_vcpu_initialise(v);
>> -    else if ( !is_idle_domain(d) )
>> -        rc = pv_vcpu_initialise(v);
>> -    else
>> +    else if ( is_idle_domain(d) )
>>       {
> 
> The is_idle_domain() wants to go first here, i think.
> [1] https://patchwork.kernel.org/comment/26646246/

It's a "positive" check (no !) here, so no, imo the order above is fine.

>>           /* Idle domain */
>>           v->arch.cr3 = __pa(idle_pg_table);
>>           rc = 0;
>>           v->arch.msrs = ZERO_BLOCK_PTR; /* Catch stray misuses */
>>       }
>> +    else if ( IS_ENABLED(CONFIG_PV) )
>> +        rc = pv_vcpu_initialise(v);
>> +    else
>> +        rc = -EOPNOTSUPP;
>>   
>>       if ( rc )
>>           goto fail;
> 
> Actually, if you are here and have time and inspiration :)
> - if ( is_idle_domain(d) ) staff can be consolidated at the
>    beginning of arch_vcpu_create() which will make it much more readable and 
> simple.

This may indeed be possible, but needs to be done with care. Iirc at least
once someone already screwed up there, while trying something along these
lines.

Jan

Reply via email to