Re: [PATCH v2 33/39] xen/riscv: dump old interrupt file to memory

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

On 9/23/26 5:15 PM, Baptiste Le Duc wrote:
>> At the old interrupt file, dump to memory all the eip and eie arrays).
> Typo `)`

Will drop `)`.

>> After this step is done, the old interrupt file is no longer in use so
>> old intrrupt file VGEIN could be released.
>>
>> Restoring of old interrupt file state will be done in follow-up
>> patch.
>>
> 
> 
>> There are cases where it is needed to specify on which cpu it is
>> necessary to VGEIN should be released so update vgein_release() to
> 
> The sentence miss a verb, here is a proposal:
> ```
> There are cases where the cpu on which the VGEIN is released needs to
> be specified, so update vgein_release() to deal with that.
> ```

I think it could be dropped at all as vgein_release() stub is just 
introduced here and not updated.

> Moreover, could you explain me the cases you are talking about? It's not
> clear by reading the commit message in the first place.

For example, during migration of vCPU, vCPU->processor points to new CPU 
where it will be run but we still have to free VGEIN on the prev. 
->processor.

> 
> 
>> deal with that.
> 
> 
> 
> 
>>
> 
> 
>> Keep BUG_ON("unimplemented") placeholder in imsic_migrate_vcpu() to guard
> 
> 
>> against silent incorrect behaviour or unexpected panics in guest VMs until
>> the function is fully implemented.
>>
> 
> 
>> vgein_release() is stub for now and will be introduced later.
> 
> 
>>
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>
>> diff --git a/xen/arch/riscv/aia.c b/xen/arch/riscv/aia.c
>> index 75c82bcfa1..be3901ec0c 100644
>> --- a/xen/arch/riscv/aia.c
>> +++ b/xen/arch/riscv/aia.c
>> @@ -30,3 +30,8 @@ unsigned int vgein_assign(struct vcpu *v)
>>   
>>       return 0;
>>   }
>> +
>> +void vgein_release(struct vcpu *v, unsigned int vgein_id, unsigned int cpu)
>> +{
>> +    BUG_ON("unimplemented\n");
>> +}
>> diff --git a/xen/arch/riscv/imsic.c b/xen/arch/riscv/imsic.c
>> index 5e9f6995e4..3cba58e0c1 100644
>> --- a/xen/arch/riscv/imsic.c
>> +++ b/xen/arch/riscv/imsic.c
>> @@ -56,6 +56,24 @@ static unsigned int __ro_after_init guest_num_msis;
>>    */
>>   #define GUEST_IMSIC_MAX_MSIS 255U
>>   
>> +/*
>> + * The interrupt identities an IMSIC interrupt file provides are 0 (which is
>> + * never valid, but still occupies a bit) up to IMSIC_MAX_ID inclusive, so
>> + * IMSIC_MAX_ID + 1 bits have to be covered.
>> + */
>> +#define IMSIC_MAX_EIX DIV_ROUND_UP(IMSIC_MAX_ID + 1, BITS_PER_TYPE(uint64_t))
> 
> 
>> +
>> +struct imsic_mrif_eix {
>> +    unsigned long eip[BITS_PER_TYPE(uint64_t) / BITS_PER_LONG];
>> +    unsigned long eie[BITS_PER_TYPE(uint64_t) / BITS_PER_LONG];
> 
> 
>> +};
>> +
>> +struct imsic_mrif {
>> +    struct imsic_mrif_eix eix[IMSIC_MAX_EIX];
>> +    unsigned long eithreshold;
>> +    unsigned long eidelivery;
>> +};
>> +
> Maybe I didn't get something but I couldn't find anything in the commit
> message explaining why do we use mrif here.

It is just convenient way to temporary store h/w interrupt file. Also it 
could be used not only for ...

> 
> Moreover, mrif, as described in aia spec (8.3 Memory-resident interrupt
> files), seems to be only usable with IOMMU that Xen doesn't support.

... IOMMU but also to support more guest interrupts file implemented by 
IMSIC (basically what I am calling as software interrupt file). Without 
memory-resident interrupt files, the number of virtual RISC-V harts that 
can directly receive MSIs from devices is limited by the total number of 
guest interrupt files implemented by all IMSICs in the system, because 
all MSIs to RISC-V harts must go through IMSICs. For a single RISC-V 
hart, the number of guest interrupt files is the GEILEN parameter 
defined by the Privileged Architecture, which can be at most 31 for RV32 
and 63 for RV64.

> 
> If you want to have something in memory that could store some interrupt
> file info, we should take another name to not be confusing.

It seems like it is okay to use memory residential interrupt file (mrif) 
here based on KVM's code who are using mrif for the same purpose I 
described above.

In short, MRIF is a joint virtualization technology shared between the 
IOMMU and the hypervisor. The IOMMU uses the MRIF as a memory target to 
land incoming hardware MSIs, while the hypervisor manages these MRIFs in 
RAM as software data structures to support an effectively unlimited 
number of vCPUs that don't currently hold a physical IMSIC guest file slot.

~ Oleksii