Re: [PATCH v3 05/16] xen/riscv: introduce tracking of pending vCPU interrupts, part 1

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.02.2026 10:37, Oleksii Kurochko wrote:
> On 2/11/26 3:16 PM, Jan Beulich wrote:
>> On 09.02.2026 17:52, Oleksii Kurochko wrote:
>>> Based on Linux kernel v6.16.0.
>>> Note that smp_wmb() is used instead of smp_mb__before_atomic() as what
>>> we want to guarantee that if a bit in irqs_pending_mask is obversable
>>> that the correspondent bit in irqs_pending is observable too.
>> And the counterpart of this barrier is the one encoded implicitly in
>> xchg()? Could do with making more explicit, e.g. by way of adding a
>> code comment there.
> 
> I thought it would be clear from the paragraph below where xchng_acquire()
> is mentioned. I'll update the comment to make it more explicit.

I'm confused. The (bogus) mentioning of xchg_acquire() is in the patch
description, whereas I suggested a code comment.

>>> @@ -124,3 +125,72 @@ void arch_vcpu_destroy(struct vcpu *v)
>>>   {
>>>       vfree((char *)v->arch.cpu_info + sizeof(struct cpu_info) - 
>>> STACK_SIZE);
>>>   }
>>> +
>>> +int vcpu_set_interrupt(struct vcpu *v, unsigned int irq)
>>> +{
>>> +    /*
>>> +     * We only allow VS-mode software, timer, and external
>>> +     * interrupts when irq is one of the local interrupts
>>> +     * defined by RISC-V privilege specification.
>>> +     */
>> What is "when irq is one ..." intended to be telling me? There's no ...
>>
>>> +    if ( irq != IRQ_VS_SOFT &&
>>> +         irq != IRQ_VS_TIMER &&
>>> +         irq != IRQ_VS_EXT )
>>> +        return -EINVAL;
>> ... corresponding code (anymore), afaict.
> 
> That part should be removed, there is no any sense for it anymore.
> 
>> Further, who are the prospected callers of this function and its sibling
>> below? If they're all internal (i.e. not reachable via hypercalls or
>> emulation on behalf of the guest), this may want to be assertions.
> 
> Considering your further reply:
>    Having seen a use in patch 08, I should clarify the "reachable" part here:
>    It's not the "reachable" alone, but whether the guest has control over the
>    "irq" value passed. For the example in patch 08 this isn't the case.
> 
> I think I did not fully understand the part about “the guest has control over
> the ‘irq’ value passed”, but at the moment I do not have any plans for the 
> guest
> to pass the irq value directly. (Do you have any examples where it should be
> needed?).

No, I don't. This is all for you to sort out.

>>> --- a/xen/arch/riscv/include/asm/domain.h
>>> +++ b/xen/arch/riscv/include/asm/domain.h
>>> @@ -54,6 +54,25 @@ struct arch_vcpu {
>>>       register_t henvcfg;
>>>       register_t hideleg;
>>>       register_t hstateen0;
>>> +    register_t hvip;
>>> +
>>> +    register_t vsie;
>>> +
>>> +    /*
>>> +     * VCPU interrupts
>>> +     *
>>> +     * We have a lockless approach for tracking pending VCPU interrupts
>>> +     * implemented using atomic bitops. The irqs_pending bitmap represent
>>> +     * pending interrupts whereas irqs_pending_mask represent bits changed
>>> +     * in irqs_pending. Our approach is modeled around multiple producer
>>> +     * and single consumer problem where the consumer is the VCPU itself.
>>> +     *
>>> +     * DECLARE_BITMAP() is needed here to support 64 vCPU local interrupts
>>> +     * on RV32 host.
>>> +     */
>>> +#define RISCV_VCPU_NR_IRQS 64
>> What is this 64 deriving from? IOW - can it be some kind of expression,
>> helping the reader?
> 
> Originally it derives from the width of mideleg, mie, mvien, mvip, and mip 
> (and
> their counterpars for other modes) what means that RV32 will have no more then
> 32 local interrutps, but considering that RISC-V AIA spec tells ...:
> 
>    Table 2.1 lists both the CSRs added for machine level and existing 
> machine-level
>    CSRs whose size is changed by the Advanced Interrupt Architecture. 
> Existing CSRs
>    mie, mip, and mideleg are widended to 64 bits to support a total of 64 
> interrupt
>    causes.
>    For RV32, the high-half CSRs listed in the table allow access to the upper 
> 32
>    bits of registers mideleg, mie, mvien, mvip, and mip. The Advanced 
> Interrupt
>    Architecture requires that these high-half CSRs exist for RV32, but the 
> bits they
>    access may all be merely read-only zeros.
> 
> ... that for RV32 it was widened to 64, so 64 appears here. I haven't used 
> some AIA
> specific name for constant 64 as in case if AIA isn't used it is more then 
> enough
> to cover PLIC case, for example.

Thing is that with RV128 in mind I wonder whether 64 is correct, or whether it
e.g. wants to be max(BITS_PER_LONG, 64).

Jan