On 2021-04-13 10:53:39 Tue, Oliver O'Halloran wrote:
On Mon, Apr 12, 2021 at 5:52 PM Mahesh Salgaonkar mahesh@linux.ibm.com wrote:
During the EEH MMIO error checking, the current implementation fails to map the (virtual) MMIO address back to the pci device on radix with hugepage mappings for I/O. This results into failure to dispatch EEH event with no recovery even when EEH capability has been enabled on the device.
eeh_check_failure(token) # token = virtual MMIO address addr = eeh_token_to_phys(token); edev = eeh_addr_cache_get_dev(addr); if (!edev) return 0; eeh_dev_check_failure(edev); <= Dispatch the EEH event
In case of hugepage mappings, eeh_token_to_phys() has a bug in virt -> phys translation that results in wrong physical address, which is then passed to eeh_addr_cache_get_dev() to match it against cached pci I/O address ranges to get to a PCI device. Hence, it fails to find a match and the EEH event never gets dispatched leaving the device in failed state.
The commit 33439620680be ("powerpc/eeh: Handle hugepages in ioremap space") introduced following logic to translate virt to phys for hugepage mappings:
eeh_token_to_phys():
pa = pte_pfn(*ptep);/* On radix we can do hugepage mappings for io, so handle that */if (hugepage_shift) {pa <<= hugepage_shift; <= This is wrongpa |= token & ((1ul << hugepage_shift) - 1);}I think I vaguely remember thinking "is this right?" at the time. Apparently not!
Reviewed-by: Oliver O'Halloran oohall@gmail.com
Thanks for the review.
It would probably be a good idea to add a debugfs interface to help with testing the address translation. Maybe something like:
echo <mmio addr> > /sys/kernel/debug/powerpc/eeh_addr_check
Then in the kernel:
struct resource *r = lookup_resource(mmio_addr); void *virt = ioremap_resource(r); ret = eeh_check_failure(virt); iounmap(virt)
return ret;
A little tedious, but then you can write a selftest :)
Sure, will give a try.
Thanks, -Mahesh.