Re: [PATCH v2 34/39] xen/riscv: restore register state in the new IMSIC VS-file

[Oleksii Kurochko <oleksii.kurochko@gmail.com>]

On 9/14/26 5:21 PM, Jan Beulich wrote:
> On 27.08.2026 17:21, Oleksii Kurochko wrote:
>> @@ -630,13 +649,13 @@ static void cf_check imsic_vsfile_local_read_clear(void *data)
>>       old_vsiselect = csr_read(CSR_VSISELECT);
>>       old_hstatus = csr_read(CSR_HSTATUS);
>>       new_hstatus = old_hstatus & ~HSTATUS_VGEIN;
>> -    new_hstatus |= ((unsigned long)idata->hgei) << HSTATUS_VGEIN_SHIFT;
>> +    new_hstatus |= MASK_INSR(idata->hgei, HSTATUS_VGEIN);
> 
> Please put into final shape upon introduction.

Oh, right, this ...

> 
>>       csr_write(CSR_HSTATUS, new_hstatus);
>>   
>>       /*
>> -     * There is no need to use atomic functions version to store
>> -     * values in MRIF because imsic_vsfile_read_clear() is always called
>> -     * with pointer to temporary MRIF on stack.
>> +     * No atomic accessors are needed to store the values into the MRIF here,
>> +     * as imsic_vsfile_read_clear() is always called with a pointer to a
>> +     * temporary MRIF on the stack.
>>        */
> 
> Same for this comment perhaps.

... and this should be part of prev. patch.

> 
>> @@ -1077,5 +1140,12 @@ void imsic_migrate_vcpu(struct vcpu *v)
>>       /* Free-up old IMSIC VS-file */
>>       vgein_release(v, old_vsfile_id, old_vsfile_cpu);
>>   
>> -    BUG_ON("unimplemented");
>> +    /* Restore register state in the new IMSIC VS-file */
>> +    vsfile_data.mrif = &tmrif;
> 
> Ah, here &tmrif is used a 2nd time.

I think it could be dropped and just properly init vsfile_data.mrif 
during declaration:

     struct imsic_vsfile_data vsfile_data = {
         .nr_eix = imsic_nr_eix(),
         .mrif = &(struct imsic_mrif){ },
     };

and then use vsfile.mrif instead of &tmrif.

> 
>> +    imsic_call_on_cpu(new_vsfile_cpu, imsic_vsfile_local_update, &vsfile_data);
>> +
>> +    /* Set VCPU HSTATUS.VGEIN to new IMSIC VS-file */
>> +    vcpu_guest_cpu_user_regs(v)->hstatus &= ~HSTATUS_VGEIN;
>> +    vcpu_guest_cpu_user_regs(v)->hstatus |=
>> +            MASK_INSR(new_vsfile_hgei, HSTATUS_VGEIN);
> 
> Nit: Indentation.
> 
> Other comments on earlier patches apply here (and possibly elsewhere) as
> well. Just ftaod.

I will fix them.

Thanks.

~ Oleksii