Re: [Xen-devel] Xen/arm: Virtual ITS command queue handling

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Tue, 2015-05-19 at 15:05 +0100, Julien Grall wrote:
> On 19/05/15 13:48, Vijay Kilari wrote:
> > On Tue, May 19, 2015 at 5:49 PM, Ian Campbell <ian.campbell@xxxxxxxxxx> 
> > wrote:
> >> On Tue, 2015-05-19 at 17:40 +0530, Vijay Kilari wrote:
> >>>> If a guest issues (for example) a MOVI which is not followed by an
> >>>> INV/INVALL on native then what would trigger the LPI configuration to be
> >>>> applied by the h/w?
> >>>>
> >>>> If a guest is required to send an INV/INVALL in order for some change to
> >>>> take affect and it does not do so then it is buggy, isn't it?
> >>>
> >>> agreed.
> >>>
> >>>>
> >>>> IOW all Xen needs to do is to propagate any guest initiated INV/INVALL
> >>>> as/when it occurs in the command queue. I don't think we need to
> >>>> fabricate an additional INV/INVALL while emulating a MOVI.
> >>>>
> >>>> What am I missing?
> >>>
> >>> back to point:
> >>>
> >>> INV has device id so not an issue.
> >>> INVALL does not have device id to know pITS to send.
> >>> For that reason Xen is expected to insert INVALL at proper
> >>> places similar to SYNC and ignore INV/INVALL of guest.
> >>
> >> Why wouldn't Xen just insert an INVALL in to all relevant pITS in
> >> response to an INVALL from the guest?
> > 
> > If INVALL is sent on all pITS, then we need to wait for all pITS to complete
> > the command before we update CREADR of vITS.
> > 
> >>
> >> If you are proposing something different then please be explicit by what
> >> you mean by "proper places similar to SYNC". Ideally by proposing some
> >> new text which I can use in the document.
> > 
> > If the platform has more than 1 pITS, The ITS commands are mapped
> > from vITS to pITS using device ID provided with ITS command.
> > 
> > However SYNC and INVALL does not have device ID.
> > In such case there could be two ways to handle
> > 1) SYNC and INVALL of guest will be sent to pITS based on previous ITS 
> > commands
> >     of guest
> > 2) Xen will insert/append SYNC and INVALL to guest ITS commands
> > where-ever required and ignore guest
> >    SYNC and INVALL commands
> > 
> > IMO (2) would be better as approach (1) might fail to handle
> > scenario where-in guest is sending only SYNC & INVALL commands.
> 
> When the guest send a SYNC, it expects all the command to be completed.
> If you send SYNC only when you think it's required we will end up to
> unexpected behavior.
> 
> Now, for INVALL, as said on a previous mail it's never required after an
> instruction. It's used to ask the ITS to invalid his cache of the LPI
> configuration.
> 
> A software would be buggy if no INV/INVALL is sent after change the LPI
> configuration table.

Specifically _guest_ software.

AIUI the ITS is not required to reread the LPI cfg table unless an
INV/INVALL is issued, but it is allowed to do so if it wants, i.e. it
could pickup the config change at any point after the write to the cfg
table. Is that correct?

If so then as long as it cannot blow up in Xen's face (i.e. an interrupt
storm) I think between a write to the LPI config table and the next
associated INV/INVALL we are entitled either continue using the old
config until the INV/INVALL, to immediately enact the change or anything
in the middle. I think this gives a fair bit of flexibility.

You've proposed something at the "immediately enact" end of the
spectrum.

> As suggested on a previous mail, I think we can get rid of sending
> INV/INVALL command to the pITS by trapping the LPI configuration table:

The motivation here is simply to avoid the potential negative impact on
the system of a guest which fills its command queue with INVALL
commands?

I think we don't especially care about INV since they are targeted. We
care about INVALL because they are global. INV handling comes along for
the ride though.

> For every write access, when the vLPIs is valid (i.e associated to a
> device/interrupt), Xen will toggle the enable bit in the hardware LPIs
> configuration table, send an INV * and sync his internal state. This
> requiring to be able to translate the vLPIs to a (device,ID).

"INV *"? You don't mean INVALL I think, but rather INV of the specific
device?

One possible downside is that you will convert this guest vits
interaction:
        for all LPIs
                enable LPI
        INVALL

Into this pits interaction:
        for all LPIs
                enable LPI
                INV LPI

Also sequences of events with toggle things back and forth before
invalidating are similarly made more synchronous. (Such sequences seem
dumb to me, but kernel side abstractions sometimes lead to such things).

> INVALL/INV command could be ignored and directly increment CREADR (with
> some care) because it only ensure that the command has been executed,
> not fully completed. A SYNC would be required from the guest in order to
> ensure the completion.
> 
> Therefore we would need more care for the SYNC. Maybe by injecting a
> SYNC when it's necessary.
> 
> Note that we would need Xen to send command on behalf of the guest (i.e
> not part of the command queue).

A guest may do this:
        Enqueue command A
        Enqueue command B
        Change LPI1 cfg table
        Change LPI2 cfg table
        Enqueue command C
        Enqueue command D
        Enqueue INV LPI2
        Enqueue INV LPI1

With your change this would end up going to the PITS as:
        Enqueue command A
        Enqueue command B
        Change LPI1 cfg table
        Enqueue INV LPI1
        Change LPI2 cfg table
        Enqueue INV LPI2
        Enqueue command C
        Enqueue command D

Note that the INV's have been reordered WRT command C and D as well as
each other. Are there sequences of commands where this may make a
semantic difference?

What if command C is a SYNC for example?

> With this solution, it would be possible to have a small amount of time
> where the pITS doesn't use the correct the configuration (i.e the
> interrupt not yet enabled/disabled). Xen is able to cooperate with that
> and will queue the interrupt to the guest.

I think it is inherent in the h/w design that an LPI may still be
delivered after the cfg table has changed or even the INV enqueued, it
is only guaranteed to take effect with a sync following the INV.

I had in mind a lazier scheme which I'll mention for completeness not
because I necessarily think it is better.

For each vits we maintain a bit map which marks LPI cfg table entries as
dirty. Possibly a count of dirty entries too.

On trap of cfg table write we propagate the change to the physical table
and set the corresponding dirty bit (and count++ if we are doing that)

On INV we insert the corresponding INV to the PITS iff
test_and_clear(dirty, LPI) and count--. If the bit is not set then we
just eat the INV.

On INVALL we insert INVALL iff there are bits set in the bitmap (or use
count is not 0 instead, if we chose to maintain that) and clear the
bitmap. If no bits are set in the bitmap then we eat the INVALL.

Extension: If we are tracking count then we may choose to switch INVAL
into one or more INV's up to some threshold of dirtiness.

I've been trying to think of ways of extending this to reduce the
number/impact of guest SYNC. Other than tracking whether there have been
0 or !0 commands since the last SYNC (and squashing the extras) I
haven't thought of a cunning scheme.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel