Re: [PATCH v1 07/15] xen/riscv: introduce tracking of pending vCPU interrupts, part 1

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 1/15/26 11:59 AM, Jan Beulich wrote:
> On 15.01.2026 11:55, Oleksii Kurochko wrote:
> > On 1/15/26 10:52 AM, Jan Beulich wrote:
> > > On 15.01.2026 10:14, Oleksii Kurochko wrote:
> > > > On 1/14/26 4:56 PM, Jan Beulich wrote:
> > > > > On 14.01.2026 16:39, Oleksii Kurochko wrote:
> > > > > > If a guest will do "That (the 1 -> 0 transitions) could be (guest) writes
> > > > > > to SVIP, for example." then the correspondent HVIP (and HIP as usually
> > > > > > they are aliasis of HVIP) bits will be updated. And that is why we need
> > > > > > vcpu_sync_interrupts() I've mentioned in one of replies and sync VSSIP:
> > > > > > +void vcpu_sync_interrupts(struct vcpu *v)
> > > > > > +{
> > > > > > +    unsigned long hvip;
> > > > > > +
> > > > > > +    /* Read current HVIP and VSIE CSRs */
> > > > > > +    v->arch.vsie = csr_read(CSR_VSIE);
> > > > > > +
> > > > > > +    /* Sync-up HVIP.VSSIP bit changes does by Guest */
> > > > > > +    hvip = csr_read(CSR_HVIP);
> > > > > > +    if ( (v->arch.hvip ^ hvip) & BIT(IRQ_VS_SOFT, UL) )
> > > > > > +    {
> > > > > > +        if ( hvip & BIT(IRQ_VS_SOFT, UL) )
> > > > > > +        {
> > > > > > +            if ( !test_and_set_bit(IRQ_VS_SOFT,
> > > > > > +                                   &v->arch.irqs_pending_mask) )
> > > > > > +                set_bit(IRQ_VS_SOFT, &v->arch.irqs_pending);
> > > > > > +        }
> > > > > > +        else
> > > > > > +        {
> > > > > > +            if ( !test_and_set_bit(IRQ_VS_SOFT,
> > > > > > +                                   &v->arch.irqs_pending_mask) )
> > > > > > +                clear_bit(IRQ_VS_SOFT, &v->arch.irqs_pending);
> > > > > > +        }
> > > > > > +    }
> > > > > I fear I don't understand this at all. Why would the guest having set a
> > > > > pending bit not result in the IRQ to be marked pending?
> > > > Maybe it is wrong assumption but based on the spec:
> > > >      Bits sip.SSIP and sie.SSIE are the interrupt-pending and interrupt-enable
> > > >      bits  for supervisor-level software interrupts. If implemented, SSIP is
> > > >      writable in sip and may also be set to 1 by a platform-specific interrupt
> > > >      controller.
> > > > and:
> > > >      Interprocessor interrupts are sent to other harts by 
> > > > implementation-specific
> > > >      means, which will ultimately cause the SSIP bit to be set in the recipient
> > > >      hart’s sip register.
> > > >
> > > > Meaning that sending an IPI to self by writing 1 to sip.SSIP is
> > > > well-defined. The same should be true of vsip.SSIP while in VS mode.
> > > I can't read that out of the text above. To the contrary, "will ultimately cause
> > > the SSIP bit to be set" suggests to me that the bit is not to be set by writing
> > > the CSR. Things still may work like this for self-IPI, but that wouldn't follow
> > > from the quotation above.
> > Why not that wouldn't follow from the quotation above?
> >
> > The first quotation tells that we can do self-IPI so VSSIP.SSIP will set to 1
> > what we could miss SSIP bit if won't explicitly try to read h/w HVIP (or VSSIP,
> > or whatever other alias of the SSIP bit) and sync with what we have cached
> > in hypervisor.
> The bit being writable doesn't imply that it being written with 1 would also
> trigger an interruption. If that's indeed the behavior, it surely is being
> said elsewhere.
According to the spec it will trap to S-mode (VS-mode in our context) if both of
the following are true: (a) either the current privilege mode is S and the SIE
bit in the sstatus register is set, or the current privilege mode has less
privilege than S-mode; and (b) bit i is set in both sip and sie.

Even without a triggering an interrupt I think it we can still lose set bit in
VSSIP register (if Im not mistaken something). If we won't do a sync of cached
hvip and h/w hvip then it could lead to the issue we lost a real SSIP bit value.
For example, guest before entering hypervisor set VSSIP.SSIP to 1 what
means what means that hip.VSSIP will be also set to 1 as:
  When bit 2 of hideleg is zero, vsip.SSIP and vsie.SSIE are read-only zeros.
  Else, vsip.SSIP and vsie.SSIE are aliases of hip.VSSIP and hie.VSSIE.
And so hvip.SSIP will be set to 1 as:
  Bits hip.VSSIP and hie.VSSIE are the interrupt-pending and interrupt-enable
  bits for VS-level software interrupts. VSSIP in hip is an alias (writable)
  of the same bit in hvip.
And then if we don't sync cached hvip with h/w hvip, it could lead to then
when we will put cached hvip (which has .VSSIP set to 0) overwrite h/w 
hvip.VSSIP
which was set to 1.

~ Oleksii