Re: [RFC] PPC: MPIC: necessary readback after EOI?

On Mon, 2015-01-05 at 19:43 +0100, Andreas Mohr wrote:

[CC related ppl]

On Mon, Jan 05, 2015 at 12:10:54PM -0600, Scott Wood wrote:

quoted

On Mon, 2015-01-05 at 18:46 +0100, Andreas Mohr wrote:

quoted

Hi,

quoted

I was curious why the mpic_cpu_read(MPIC_INFO(CPU_WHOAMI)) was there in
the first place and if it's still needed. If it's still required, I
guess a better approach is to eliminate the call only if the kernel is
running on the KVM guest side, where the MPIC is emulated and no longer
requires a readback.

"Why not?"

A mechanism being "emulated"/"virtual" or not
may not necessarily be much of a distinction (if at all!).
The readback might be required
to properly fulfill all requirements
of a full state change protocol specification,
which might easily be the case for both RS(*) and virtual hardware.
And especially for virtual hardware
such a "readback" event
might be an extremely important "end of transaction" marker
which may often be needed for freeing of temporary resources etc.

I'm not convinced that it's required in real silicon (though there are
many MPIC implementations which have their own quirks...), and I'm 100%
sure that it's not required in the QEMU/KVM implementation of MPIC.

It would have been nice if a code comment explained why it was doing the
readback...  I don't see any particular need to wait for EOI completion
here (unlike when masking).

Hmm, yeah.

git clone git://git.kernel.org/pub/scm/linux/kernel/git/history/history.git
git checkout v2.6.11
git blame ./ppc64/kernel/mpic.c
git show 378193eb
    [PATCH] ppc64: Rewrite the openpic driver

+/* Send an EOI */
+static inline void mpic_eoi(struct mpic *mpic)
+{
+       mpic_cpu_write(MPIC_CPU_EOI, 0);
+       (void)mpic_cpu_read(MPIC_CPU_WHOAMI);
+}


-static void openpic_eoi(void)
-{
-       DECL_THIS_CPU;
-
-       CHECK_THIS_CPU;
-       openpic_write(&OpenPIC->THIS_CPU.EOI, 0);
-       /* Handle PCI write posting */
-       (void)openpic_read(&OpenPIC->THIS_CPU.EOI);
-}


So, this does seem to be about PCI posted writes after all.
Which begs the question whether all PIC hardware is connected via PCI bus,
which... is not the case for emulated hardware at least, I'd think.

And it's somewhat unfortunate
that the comment in fact was removed in that commit
(perhaps reinstate this comment in all of the various mpic.c life forms?).

But even for PCI, why do we need to wait for this write to complete?
Back in the arch/ppc days ppc_md.get_irq() was called in a loop, so it
would make sense that we'd need the next IRQ to be ready by the time
it's called again, but now that there's no get_irq() loop we shouldn't
need to wait.

-Scott