On 09.02.2026 17:52, Oleksii Kurochko wrote:
> This patch is based on Linux kernel 6.16.0.
>
> Add the consumer side (vcpu_flush_interrupts()) of the lockless pending
> interrupt tracking introduced in part 1 (for producers). According, to the
> design only one consumer is possible, and it is vCPU itself.
> vcpu_flush_interrupts() is expected to be ran (as guests aren't ran now due
> to the lack of functionality) before the hypervisor returns control to the
> guest.
>
> Producers may set bits in irqs_pending_mask without a lock. Clearing bits in
> irqs_pending_mask is performed only by the consumer via xchg() (with aquire &
> release semantics).
Where the release part isn't relevant here, aiui.
> --- a/xen/arch/riscv/domain.c
> +++ b/xen/arch/riscv/domain.c
> @@ -194,3 +194,36 @@ void vcpu_sync_interrupts(struct vcpu *v)
> # error "Update v->arch.vsieh"
> #endif
> }
> +
> +static void vcpu_update_hvip(const struct vcpu *v)
> +{
> + csr_write(CSR_HVIP, v->arch.hvip);
> +}
> +
> +void vcpu_flush_interrupts(struct vcpu *v)
> +{
> + register_t *hvip = &v->arch.hvip;
Why not in the if()'s scope, when it's used only there?
> + if ( ACCESS_ONCE(v->arch.irqs_pending_mask[0]) )
> + {
> + unsigned long mask, val;
> +
> + mask = xchg(&v->arch.irqs_pending_mask[0], 0UL);
> + val = ACCESS_ONCE(v->arch.irqs_pending[0]) & mask;
Make these the initializers of the variables?
> + *hvip &= ~mask;
> + *hvip |= val;
> +
> + /*
> + * Flush AIA high interrupts.
> + *
> + * It is necessary to do only for CONFIG_RISCV_32 which isn't
> + * supported now.
> + */
> +#ifdef CONFIG_RISCV_32
> + # error "Update v->arch.hviph"
Ehem. I really dislike having to give the same comment over and over again.
Please have the # of a pre-processor directive always in the first column.
Also, isn't this misplaced? The containing if() checks irqs_pending_mask[0],
yet aiui irqs_pending_mask[1] would be of interest for hviph?
> --- a/xen/arch/riscv/traps.c
> +++ b/xen/arch/riscv/traps.c
> @@ -172,6 +172,8 @@ static void do_unexpected_trap(const struct cpu_user_regs
> *regs)
> static void check_for_pcpu_work(void)
> {
> vcpu_sync_interrupts(current);
> +
> + vcpu_flush_interrupts(current);
> }
Ah, okay - here comes a 2nd call from this function. However, please latch
current into a local variable (already in the earlier patch perhaps); no
need to fetch it from per-CPU data twice (or yet more times, if further
stuff was going to appear here).
Jan
