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

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Tue, 2015-05-19 at 17:08 +0530, Vijay Kilari wrote:
> Hi Ian,
> 
>    If we want to target for 4.6, then I think we should draw conclusion
> 
> On Sat, May 16, 2015 at 2:19 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> > Hi,
> >
> >
> > On 16/05/2015 05:03, Vijay Kilari wrote:
> >>
> >> On Fri, May 15, 2015 at 11:01 PM, Julien Grall <julien.grall@xxxxxxxxxx>
> >> wrote:
> >>>
> >>> On 15/05/15 16:38, Ian Campbell wrote:
> >>>>
> >>>> On Fri, 2015-05-15 at 16:05 +0100, Julien Grall wrote:
> >>>>>
> >>>>> On 15/05/15 15:04, Vijay Kilari wrote:
> >>>>>>
> >>>>>> On Fri, May 15, 2015 at 7:14 PM, Julien Grall
> >>>>>> <julien.grall@xxxxxxxxxx> wrote:
> >>>>>>>
> >>>>>>> On 15/05/15 14:24, Ian Campbell wrote:
> >>>>>>>>
> >>>>>>>> On Fri, 2015-05-15 at 18:44 +0530, Vijay Kilari wrote:
> >>>>>>>>>
> >>>>>>>>> On Fri, May 15, 2015 at 6:23 PM, Ian Campbell
> >>>>>>>>> <ian.campbell@xxxxxxxxxx> wrote:
> >>>>>>>>>>
> >>>>>>>>>> On Fri, 2015-05-15 at 18:17 +0530, Vijay Kilari wrote:
> >>>>>>>>>>>
> >>>>>>>>>>> On Fri, May 15, 2015 at 5:33 PM, Julien Grall
> >>>>>>>>>>> <julien.grall@xxxxxxxxxx> wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>> On 15/05/15 12:30, Ian Campbell wrote:
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Handling of Single vITS and multipl pITS can be made simple.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> All ITS commands except SYNC & INVALL has device id which will
> >>>>>>>>>>>>>> help us to know to which pITS it should be sent.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> SYNC & INVALL can be dropped by Xen on Guest request
> >>>>>>>>>>>>>>   and let Xen append where ever SYNC & INVALL is required.
> >>>>>>>>>>>>>> (Ex; Linux driver adds SYNC for required commands).
> >>>>>>>>>>>>>> With this assumption, all ITS commands are mapped to pITS
> >>>>>>>>>>>>>> and no need of synchronization across pITS
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> You've ignored the second bullet its three sub-bullets, I
> >>>>>>>>>>>>> think.
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>     Why can't we group the batch of commands based on pITS it has
> >>>>>>>>>>> to be sent?.
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Are you suggesting that each batch we send should be synchronous?
> >>>>>>>>>> (i.e.
> >>>>>>>>>> end with SYNC+INT) That doesn't seem at all desirable.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Not only at the end of batch, SYNC can be appended based on every
> >>>>>>>>> command within the batch.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Could be, but something to avoid I think?
> >>>>>>>
> >>>>>>>
> >>>>>>> That would slow down the ITS processing (SYNC is waiting that the
> >>>>>>> previous command has executed).
> >>>>>>>
> >>>>>>> Also, what about INTALL? Sending it everytime would be horrible for
> >>>>>>> the
> >>>>>>> performance because it flush the ITS cache.
> >>>>>>
> >>>>>>
> >>>>>> INVALL is not required everytime. It can be sent only as mentioned in
> >>>>>> spec Note.
> >>>>>> ex; MOVI
> >>>
> >>>
> >>> BTW, when you quote the spec, can you give the section number/version of
> >>> the spec? So far, I'm not able to find anything about the relation
> >>> between MOVI and INVALL in my spec.
> >>>
> >>
> >> See 5.13.19 INVALL collection of PRD03-GENC-010745 20.0
> >
> >
> > Still nothing about MOVI... How did you deduce it?
> 
>  I have quoted it as an example where INVALL might be needed.
> 
> >
> >
> > The spec only says:
> >
> > "this command is expected to be used by software when it changed the
> > re-configuration of an LPI in memory
> > to ensure any cached copies of the old configuration are discarded."
> >
> >>> INV* commands are sent in order to ask the ITS reloading the
> >>> configuration tables (see 4.8.4 PRD03-GENC-010745 24.0):
> >>>
> >>> "The effects of this caching are not visible to software except when
> >>> reconfiguring an LPI, in which case an explicit invalidate command must
> >>> be issued (e.g. an ITS INV command or a write to GICR_INVLPIR)
> >>> Note: this means hardware must manage its caches automatically when
> >>> moving interrupts"
> >>>
> >>> So, it looks like to me that INV* command are only necessary when
> >>> configuration tables is changed.
> >>>
> >>> FWIW, Linux is using INVALL when a collection is map and INV when the
> >>> LPI configuration is changed. I don't see any INV* command after MOVI.
> >>> So it confirms what the spec says.
> >>>
> >>>>>> Note: this command is expected to be used by software when it changed
> >>>>>> the re-configuration
> >>>>>> of an LPI in memory to ensure any cached copies of the old
> >>>>>> configuration are discarded.
> >>>>>
> >>>>>
> >>>>> INVALL is used when a large number of LPIs has been reconfigured. If
> >>>>> you
> >>>>> send one by MOVI is not efficient at all and will slowdown all the
> >>>>> interrupts for few milliseconds. We need to use them with caution.
> >>>>>
> >>>>> Usually a guest will send one for multiple MOVI command.
> >>>>
> >>>>
> >>>> We should be prepared for a guest which does nothing but send INVALL
> >>>> commands (i.e. trying to DoS the host).
> >>>>
> >>>> I mentioned earlier about maybe needing to track which pITS's a SYNC
> >>>> goes to (based on what SYNC have happened already and what commands the
> >>>> guest has sent since).
> >>>>
> >>>> Do we also need to track which LPIs a guest has fiddled with in order to
> >>>> decide (perhaps via a threshold) whether to use INVALL vs a small number
> >>>> of targeted INVALL?
> >>>
> >>>
> >>> I did some reading about the INV* commands (INV and INVALL). The
> >>> interesting section in GICv3 is 4.8.4 PRD03-GENC-010745 24.0.
> >>>
> >>> They are only used to ensure the ITS re-read the LPIs configuration
> >>> table. I don't speak about the pending table as the spec (4.8.5) says
> >>> that it's maintained solely by a re-distributor. It's up to the
> >>> implementation to provide a mechanism to sync the memory (useful for
> >>> Power Management).
> >>>
> >>> The LPIs configuration tables is used to enable/disable the LPI and set
> >>> the priority. Only the enable/disable bit needs to be replicated to the
> >>> hardware.
> >>>
> >>> The pITS LPIs configuration tables is managed by Xen. Each guest will
> >>> provide to the vITS his own LPIs configuration table.
> >>>
> >>> The emulation of INV* command will depend on how we decide to emulate
> >>> the LPIs configuration table.
> >>>
> >>> Solution 1: Trap every access to the guest LPIs configuration table
> >>>
> >>     Trapping on guest LPI configuration table is mandatory to
> >> enable/disable LPI in LPI pending table. There is no ITS command
> >> for this. In my RFC patches I have done this, where Xen calls
> >> irq_hw_controller's set_affinity which will send INVALL command
> >
> >
> > Trapping is not mandatory. The ITS may not read the LPI configuration table
> > until a INV/INVALL command has been sent.
> >
> > The vITS is not forced to enable/disable the LPIs until one of this command
> > is sent.
> 
>   If so, in case INV/INVALL is not sent, then LPI configuration will
> never be applied.
> Which is slightly different from behaviour without Xen

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?

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?

Ian.


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