Re: [PATCH v3 39/39] xen/riscv: introduce IMSIC h/w interrupt file attaching to vcpu

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

On 11/4/26 4:06 PM, Baptiste Le Duc wrote:
>> Introduce imsic_vsfile_attach() to give a vCPU a working guest
>> interrupt file.
>>
>> A guest (VS) interrupt file has to be backed by one of the hardware
>> guest interrupt files of the pCPU the vCPU runs on, so that pCPU has to
>> be known first. arch_vcpu_create() is therefore not a suitable place:
>> the pCPU assigned to a vCPU can still change before the vCPU is first
>> scheduled, which would mean taking another file and remapping the
>> guest's IMSIC page to it. Call imsic_vsfile_attach() from
>> continue_new_vcpu() instead, i.e. when the vCPU is scheduled for the
>> first time; as that isn't an __init context, neither is
>> imsic_vsfile_attach().
>>
>> Giving a vCPU its first file shares most of the steps with moving it to
>> a new one on migration, so move the body of imsic_migrate_vcpu() to
>> imsic_vsfile_move() and use it for both. Its first_attach argument skips
>> the part which moves the interrupt state out of the old file: the vCPU
>> has never run at that point, so the freshly acquired and zeroed file is
>> all the interrupt state it has, and hstatus.VGEIN can be pointed at it
>> right away.
>>
>> vAPLIC target registers are reconfigured on the first attach as well: a
>> target the guest writes for a vCPU which has no h/w guest interrupt file
>> yet only updates the stored copy, which reaches the h/w at this point.
>>
> Nit: I didn't quite understand this paragraph, "only updates"
> leaves out that the h/w APLIC write is skipped, because there is no file
> to point the target at yet. Maybe something like:
> 
>      The guest may program a vAPLIC TARGET register for a vCPU which has
>      not run yet. That vCPU has no h/w guest interrupt file yet, so
>      vaplic_mmio_write() only stores the value. imsic_vsfile_move() then
>      writes it to the h/w APLIC on the first attach, pointing to the newly
>      acquired file.
> 

Agreed, I'll reword it along those lines, naming vaplic_target_write(),
which is where the value gets stored, and vaplic_reconfigure_target(),
which is what writes it to the h/w APLIC on the first attach:

```
The guest may program the vAPLIC target register of a h/w source for a
vCPU which hasn't run yet. That vCPU has no h/w guest interrupt file
yet, so there is nothing to point the h/w target register at, and
vaplic_target_write() only stores the value. Hence imsic_vsfile_move()
calls vaplic_reconfigure_target() on the first attach as well, which
writes the stored value to the h/w APLIC, pointing it at the newly
acquired file.
```

> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
> 

Thanks for the review!

~ Oleksii