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