On 10.09.2026 12:37, Oleksii Kurochko wrote:
> On 9/9/26 4:26 PM, Jan Beulich wrote:
>> On 27.08.2026 17:20, Oleksii Kurochko wrote:
>>> +uint32_t aplic_msi_target_gen(const struct vcpu *target_vcpu,
>>> + uint32_t base_val)
>>> +{
>>> + unsigned int guest_id = vcpu_guest_file_id(target_vcpu);
>>> + unsigned long hart_field = aplic_hart_field(target_vcpu->processor);
>>
>> What guarantees target_vcpu's ->processor field to be meaningful at this
>> point?
>
> Good question. Considering the places where it is called, I would expect
> target_vcpu->processor to have something meaningful, since it is called
> from a place that can only be reached when the guest is running.
>
> Even without that, I don't think there is a big issue here, as
> ->processor is initialized to 0 at allocation time. This means that the
> target register will be configured in such a way that CPU 0 will handle
> such IRQs.
I may not have been explicit enough then: Whether the guest (as a whole)
is running is of no interest. If the specific vCPU is running, all is fine.
If the specific vCPU is in the process of being moved to a different CPU,
and if you read ->processor just before the new value is put there, is
all going to be fine as well? I doubt that.
>>> --- a/xen/arch/riscv/include/asm/vaplic.h
>>> +++ b/xen/arch/riscv/include/asm/vaplic.h
>>> @@ -21,11 +21,16 @@ struct domain;
>>>
>>> struct vaplic_regs {
>>> uint32_t domaincfg;
>>> +
>>> + uint32_t *target;
>>> };
>>
>> A pointer in this structure is odd, as this (supposedly) is a set of
>> guest register values. The field name also doesn't clarify its purpose.
>> All in all: Likely a comment is needed here.
>
> It is a pointer because I tried to save some memory and allocate
> register target based on how many irqs a guest really needed.
>
> As an option I can allocate that statically and use the value from AIA
> spec: target[1]-target[1023]. Would it better?
No, I think dynamic allocation is justified here. (Question is whether it
should be struct vaplic_regs as a whole, but that depends on whether
another runtime-sized array may need adding to it later on.)
Jan
