Re: MSI interrupts

On Wed, 26 Mar 2025 4:08:57 +0000, Dan Cross wrote:

In article <b1c74762d01f71cc1b8ac838dcf6d4fa@www.novabbs.org>,
MitchAlsup1 <mitchalsup@aol.com> wrote:

-----------------------

>
You (well Chris) can write critical sections where a low priority
holder can give way to the higher priority requestor such that
the low priority holder reverts back to before it entered CS,
and does not see the lock being stolen.

>
You still have not defined what you mean by, "the lock being
stolen."
>
Tell you what.  Earlier, you posted some code for a critical
section that inserts a node into a linked list.  That code is
written in terms of some primitives (presumably intrinsics,
though you call them macros); `esmLOCKload`, `esmLOCKprefetch`,
`esmINTERFERENCE` and `esmLOCKstore`.  Here's that code, slightly
reformatted:
>
// Removes the element `from` from a doubly linked list, and
// inserts it into a (possibly different) doubly linked list,
// after the element pointed to be `to`.
// Assumes <stdbool.h>
bool
MoveElement(Element *from, Element *to)
{
assert(from != NULL);
assert(to != NULL);
>
Element *from_next = esmLOCKload(from->next);
Element *from_prev = esmLOCKload(from->prev);
Element *to_next = esmLOCKload(to->next);
esmLOCKprefetch(from_next);
esmLOCKprefetch(from_prev);
esmLOCKprefetch(to_next);
if (!esmINTERFERENCE()) {
// Unlink the "from" node from its list.
from_prev->next = from_next;  // XXX: `from_prev` may be nil?
from_next->prev = from_prev;  // XXX: `from_next` may be nil?
// Insert the node after "to".
to->next = from;
to_next->prev = from;         // XXX: `to_net` may be nil?
from->prev = to;
esmLOCKstore(from->next, to_next);
return true;
}
return false;
}
>
Leaving aside the unchecked boundary conditions I highlighted
above, there are other questions that this raises.  You
mentioned that the ESM stuff can use up to 8 cache lines in a
single atomic "event"; let's count how many this code may
encounter:
>
1. `from` may point to a cache line
2. `to` may point to a different cache line.
3. Does your architecture use a stack?

No

   What sort of alignment criteria do you impose?

On the participating data none. On the cache line--cache line.

   At first blush, it seems to me that
   the pointers `from_next`, `from_prev`, and `to_next` could be
   on the stack and if so, will be on at least one cache line,
   and possibly 2, if the call frame for `MoveElement` spans
   more than one.

The illustrated code is using 6 participating cache lines.
Where local variables are kept (stack, registers) does not
count against the tally of participating cache lines.

                 Of course, it's impossible to say when this
   is compiled, but maybe you require that the stack is always
   aligned to a cache line boundary or something.

Data stack is DoubleWord aligned at all times.

                                                  Or maybe
   these are all in registers and it's fine.

Local variables are not participating in the event--they are just
around to guide the event towards conclusion.

4. Depending on the size and alignment criteria of the the
   `Element` structure and the placement of the `next` and
   `prev` elements in the struct (unknowable from this example),
   `to->next` may be on another cache line.
5. Similarly, `from->next`
6. And `from->prev`
7. And `from_prev->next`
8. And `from_next->prev`
9. And `to_next->prev`.
>
So potentially this code _could_ touch 9 or 10 cache lines, more
than you said are supported for a single atomic event.  What
happens in that case?  Does the code just always return false?
The programmer best make sure the optimizer is keeping those
temporary pointers off the stack; good luck with a debug build.
>
Anyway, let's assume that's not the case, and we don't hit the
pathological case with cache lines, and we're not dealing with
edge cases at the front or end of the lists (so various next or
prev pointers are not NULL).  Why don't I try to explain what I
think this code is doing, and you tell me whether I'm right or
wrong.
>
If I understand this correctly, the `esmLOCKload`s are simply
loads, but they make sure that the compiler emits instructions
that tie into your atomic support framework.

So far so good.

                                             Earlier you wrote,
"esmINTERFERENCE is a query on the state of th event and whether
a 2nd party has performed a memory action that kills the event."
Ok, I take this to mean _at the time of the store_, which
appears later in the program.  If it were simply a query at the
time it was written in the code, then it seems like it opens a
TOCTOU bug (what if something interfers after the check, but
before the the locked store at the end?).  Or perhaps it signals
to the architecture that this is the point to which the
processor should rollback, if anything subsequently fails; sort
of a `setjmp` kind of thing that conceptually returns twice.

The "if( esmINTERFERENCE() )" not only queries the state of the
participating cache lines, it sets up a guarding-shadow over
the STs to follow such that if killing-interference is detected
none of the STs are performed AND control is transferred to the
branch label (outside of the event). So not only does it query
the state at it execution, it monitors that over the rest of
the event.
BUT ALSO: it initializes the ability to NAK SNOOPs. The thought
is that if we have reached the manifestation phase of the event
(that is ready to do all the stores) then we should be able to
drive the event forward by not sending the data in response to
the SNOOP. We send a Nak (based on priority or the data)
If the SNOOPing core receives a NaK and it is not attempting an
event the request is simply reissued. If it is attempting, then
its event fails. So a core benignly accessing data that another
core is attempting an event upon, is delayed by an interconnect
round trip--but if it is attempting, it knows its event will
fail and it will either retry from before the event started,
or it will goto the interferrence-label where other facilities
\are at its disposal.

Assuming nothing has poisoned the event, the various pointer
stores inside the conditional are conditionally performed; the
`esmLOCKstore` to `from->next` commits the event, at which
point all side effects are exposed and durable.

From a SW perspective, that is accurate enough.
From a HW perspective, the participating STs are performed and
held in a buffer, and when it is known the event has succeeded,
a bit is flipped and these writes to memory all appear to have
been performed in the same cycle. One cycle earlier and no
interested 2nd party has seen any of them, the subsequent cycle
any interested 2nd party will see them all.
I should also note: ESM works even when there are no caches in
the system. ...

>
Is that correct?  Let's suppose that it is.
>
But later you wrote,
>

So, if you want the property whereby the lock disappears on any
control transfer out of the event {exception, interrupt, SVC, SVR, ...};
then you want to use my ATOMIC stuff; otherwise, you can use the
normal ATOMIC primitives everyone and his brother provide.

>
Well, what precisely do you mean here when you say, "if you want
the property whereby the lock disappears on any control transfer
out of the event"?

If you want that property--you use the tools at hand.
If you don't just use them as primitive generators.

What I _hope_ you mean is that if you transfer "out of the
event" before the `esmLOCKstore` then anything that's been done
since the "event" started is rolled back, but NOT if control
transfer happens _after_ the `esmLOCKstore`.  If THAT were the
case, then this entire mechanism is useless.

One can use a esmLOCKstore( *SP, 0 ) to abandon an event. HW
detects that *SP is not participating in the event, and uses
the lock bit in the instruction to know it is time to leave
the event in the failed manner. No ST is performed to the
non-participating cache line. {{I guess I could wrap the
abandonment into a macro ...}}

Perhaps you would clarify?
>
But for the time being, let's assume that `esmLOCKstore` is
conceptually a commit that ends the atomic event, and the store
is durable and observable after that.  Let's see if we can
construct a simple spinlock from these primitives.

Notice that only the last ST has the esmLOCKstore() applied.
HW knows that STs to participating lines have the ATOMIC
property. The final ST uses the LOCK bit to denote the end
of the event.

>
typedef struct Spinlock Spinlock;
struct Spinlock {
uint32_t lock;
};
>
uint32_t
xcas(volatile Spinlock *lk, uint32_t expected, uint32_t val)
{
uint32_t v = esmLOCKload(&lk->lock);
if (v == expected && !esmINTERFERENCE()) {
esmLOCKstore(&lk->lock, val)
}
return v;
}

If the interference check is desired at all:: (see below)
I think you should move interference check in front of the
expected check, but that is a first impression--have to think
about it more.

void
spin_acquire(volatile Spinlock *lk)
{
splhi();
while (xcas(lk, 0, 1) != 0)
relax(); // Spin loop intrinsic.
}
>
void
spin_release(volatile Spinlock *lk)
{
xcas(lk, 1, 0);
// Maybe we should `spllo` here, maybe not.  Either choice
// reflects lots of assumptions about whether or not we might
// take and hold multiple locks concurrently, etc.
spllo();
}
>
Does this seem correct?  It is not immediately clear to me
whether `esmINTERFERENCE` is actually needed here, but
avoiding the store seemed fine.

There are MANY cases where the interference query is not needed.
Without the interference query, deadly interference backs the
core up to the first instruction of the event. With the interference
check, if deadly interference happens before the query, control
reverts to before the event, and afterwards, control is transferred
to the interference-label.

                                   One could imagine shorting
`xcas` to simply:
>
uint32_t
xcas(volatile Spinlock *lk, uint32_t expected, uint32_t val)
{
uint32_t v = esmLOCKload(&lk->lock);
if (v == expected) esmLOCKstore(&lk->lock, val);
return v;
}

Yes, unless you want the alternate exit strategy, this works fine.
In fact test and set is written::
boolean char TestAndSet( type char *p )
{
     boolean t = esmLOCKload ( *p );
                 esmLOCKstore( *p, 1 );
     return t;
}
Should interference transpire between load and store control
reverts back to the LD again, so the return value is the LD
which did not encounter interference.

or perhaps:
>
uint32_t
xswap(volatile Spinlock *lk, uint32_t val)
{
uint32_t v = esmLOCKload(&lk->lock);
esmLOCKstore(&lk->lock, val);
return v;
}
>
But that all depends on the specifics of the atomics.
>

For the sizes of CSs this covers--it prevent priority inversion.

>
Except you've admitted that this is not a general solution, as
it cannot "cover" arbitrary software-defined critical sections:
the window isn't big enough and we've seen how even a fairly
small problem may exceed the number of cache lines supported by
one of these events.

Remember only the lines participating in the event count. For
example, say one had sprintf() inside the event. None of the
lines touched by springt() are participating in the event,
and are not counted against the 8 cache lines available.
---------------------

So now I, as a systems software person, have to ask design
question around how I want to use these architectural features,
if at all, weighing various pros and cons.  This is where I'm
struggling with your proposal: given that I'm going to have to
implement more general support for the things that the
architecture gives me anyway, what do I gain by also supporting
the specialized mechanisms?

All I can say about this, was that when I presented ASF (ESM's
predecessor) to Microsoft and spend a couple hours explaining
to them how it works, MS wanted it implemented as rapidly as
AMD could pull it off. Dave Cutler was particularly impressed.
----------------------

And here I thought you had an imagination .....

>
You admitted you're not an OS person and asked other people to
poke holes in your proposal.  I'm sorry if you find the holes
I'm poking upsetting.

I enjoy your poking of holes--I really do
But you seem like the calculus student that keeps asking what
problem can I solve now that I can differentiate.