On 2/11/26 3:26 PM, Jan Beulich wrote:
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.
Yes, only acquire part is relevant here. release mentioned here as xchg use
.aqrl
prefix.
--- 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?
Missed that during vcpu_update_hvip() inside if().
+ 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?
Good point. I will do that.
+ *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?
Agree, it would be more correct to have outside if().
--- 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).
It makes sense. Ill do that.
Thanks.
~ Oleksii
