On 12/14/20 6:13 PM, Lorenzo Pieralisi wrote:
[...]
Is the device_initcall() vs builtin_platform_driver() something
related to the hook_fault_code()? What would break if this were
always builtin_platform_driver()?
rcar_pcie_init() would not be called before probe.
Sorry to be slow, but why does it need to be called before probe?
Obviously software isn't putting the controller in D3 or enabling ASPM
before probe.
I don't understand it either so it would be good to clarify.
The hook_fault_code() is marked __init, so if probe() was deferred and the
kernel __init memory was free'd, attempt to call hook_fault_code() from
probe would lead to a crash.
Understood - I don't think there is a point though in keeping
the builtin_platform_driver() call then, something like:
#ifdef CONFIG_ARM
...
static __init void init_platform_hook_fault(void) {
if (of_find_matching_node(NULL, rcar_pcie_abort_handler_of_match)) {
#ifdef CONFIG_ARM_LPAE
hook_fault_code(17, rcar_pcie_aarch32_abort_handler, SIGBUS, 0,
"asynchronous external abort");
#else
hook_fault_code(22, rcar_pcie_aarch32_abort_handler, SIGBUS, 0,
"imprecise external abort");
#endif
}
}
#else
static inline void init_platform_hook_fault(void)
{}
#endif
static int __init rcar_pcie_init(void)
{
init_platform_hook_fault();
return platform_driver_register(&rcar_pcie_driver);
}
device_initcall(rcar_pcie_init);
Does this look simpler than the code in this patch ?
Or we remove the __init marker from hook_fault_code().
This is a bugfix, it should be possible to backport this through the
stable tree easily, without changing core architecture code.
Also, some of these platforms are SMP systems, I don't understand
what prevents multiple cores to fault at once given that the faults
can happen for config/io/mem accesses alike.
I understand that the immediate fix is for S2R, that is single
threaded but I would like to understand how comprehensive this fix
is.
Are you suggesting to add some sort of locking ?
If we merge a fix the fix has to work, by reading the code if multiple
cores fault at once this fix seems to have an issue that's why I asked,
you may still end up with an unhandled fault by reading the code.
So, are you suggesting the hook needs some locking ?
