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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

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