Re: [PATCH v1 08/17] xen/riscv: add IMSIC state save/restore

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

On 8/13/26 11:30 AM, Baptiste Le Duc wrote:
>> IMSIC state is currently needed only to track which physical CPU owns a
>> vCPU's IMSIC interrupt file. This is required because the physical CPU
>> ID is part of the physical address used to map the IMSIC file.
>>
>> Add imsic_state_save() to record the current pCPU for a vCPU. When the
>> vCPU is migrated to a different pCPU, the mapping will need to be updated.
>>
>> When imsic_state_restore() is called, VGEIN is already assigned to the
>> vCPU and the guest interrupt file is already mapped, and, as only h/w
>> interrupt files are used for now, nothing specific needs to be done.
>> Action is only required when the vCPU is moved to a different pCPU, which
>> requires recalculating VGEIN and the mapping for the new guest interrupt
>> file. That will be handled separately by vcpu_move_irqs(), which is
>> introduced in a follow-up patch; until then this case is guarded by a
>> BUG_ON(), which is fine.
>>
>> Co-developed-by: Romain Caritey <Romain.Caritey@microchip.com>
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>
>> diff --git a/xen/arch/riscv/imsic.c b/xen/arch/riscv/imsic.c
>> index 2a792e756c..406bc68cbc 100644
>> --- a/xen/arch/riscv/imsic.c
>> +++ b/xen/arch/riscv/imsic.c
>> @@ -20,6 +20,7 @@
>>   #include <xen/init.h>
>>   #include <xen/libfdt/libfdt.h>
>>   #include <xen/macros.h>
>> +#include <xen/rwlock.h>
>>   #include <xen/sched.h>
>>   #include <xen/smp.h>
>>   #include <xen/spinlock.h>
>> @@ -418,6 +419,28 @@ int imsic_map_guest_file(struct vcpu *v, unsigned int vsfile_id)
>>       return res;
>>   }
>>   
>> +void imsic_state_save(struct vcpu *v)
>> +{
>> +    struct vimsic_state *imsic_state = v->arch.vimsic_state;
>> +    unsigned long flags;
>> +
>> +    /*
>> +     * SW interrupt file always has ->vsfile_pcpu = NR_CPUS so nothing specific
>> +     * should be done in this case.
>> +     */
>> +    if ( !vcpu_guest_file_id(v) )
>> +        return;
> 
> 
>> +
>> +    write_lock_irqsave(&imsic_state->vsfile_lock, flags);
>> +    imsic_state->vsfile_pcpu = cpuid_to_hartid(v->processor);
> 
> How will you detect a migration is needed? Don't you need to first know
> if ->vsfile_pcpu is different to cpuid_to_hartid(v->processor)? (I
> didn't take a look to other patchs for the moment, so the
> explanations might be later.)
> 

Migration (if you are speaking about migration of vCPU from one pCPU to 
another) is completely different path. Look at sched_move_irqs().

~ Oleksii