Re: [PATCH 29/38] powerpc: Move 32-bit probe() machine to later in the boot process

On Mon, 2016-06-27 at 23:40 +0200, Benjamin Herrenschmidt wrote:

On Mon, 2016-06-27 at 22:42 +0200, Gerhard Pircher wrote:

quoted

This patch series reminds me of a long-standing issue with the
AmigaOne platform code, which would have to clear the coherence
(M) flag for every memory mapping to avoid lockups (especially
on G4 CPUs) - as you can read in the comment below your patch
above.

Now the follow-up code in amigaone/setup.c to clear the
corresponding CPU feature flag is essentially dead code (it used
to work with a second call of do_feature_fixups() before the
handling of the CPU_FTR_NEED_COHERENT flag was changed/inverted
somewhere around v2.6.30). Also it doesn't seem to fix all
deadlocks, as e.g. KVM virtual machines can still inject memory
mappings with the M flag set AFAICT.

Now I wonder, if there is a reasonably clean way to clear the M
flag in all hardware page table and BAT entries without defacing
generic kernel code too much. Any ideas?

Other than a compile time option ?

Well, I guess a runtime option would always be preferable, but I don't
see how this could be implemented without moving probe_machine() in
front of identify_cpu() (which doesn't look easy either). On the other
side the AmigaOne also requires CONFIG_NOT_COHERENT_CACHE set and this
one is still a compile time option, too.
The question is, if a compile time option that simply clears
CPU_FTR_NEED_COHERENT after identify_cpu() would be acceptable. And
then I still wonder why KVM needs its own similar fix to work on the
AmigaOne. I specifically had to remove/clear the PTE_M bit in
arch/powerpc/kvm/book3s_32_mmu_host.c for this:

206         pteg0 = ((eaddr & 0x0fffffff) >> 22) | (vsid << 7) | PTE_V |
207                 (primary ? 0 : PTE_SEC);
208         pteg1 = hpaddr | PTE_M | PTE_R | PTE_C;
                             ^^^^^

Do we know where the lockups come from btw ? A problem with the
northbridge ?

Yes, it's a problem with the northbridge...as usual. :-(

Cheers,
Ben.

Thanks!

BR,
Gerhard