On Wed, Jan 19, 2022 at 9:43 PM Bjorn Helgaas [off-list ref] wrote:

On Wed, Jan 19, 2022 at 02:40:11PM +0800, Shuai Xue wrote:

quoted

[+to Rafael, question about HEST/GHES/SDEI init]

Hi, Bjorn,

Thank you for your comments and quick reply.

在 2022/1/19 AM6:49, Bjorn Helgaas 写道:

quoted

On Sun, Jan 16, 2022 at 04:43:10PM +0800, Shuai Xue wrote:

quoted

On an ACPI system, ACPI is initialised very early from a
subsys_initcall(), while SDEI is not ready until a
subsys_initcall_sync(). This patch is to reduce the time before GHES
initialization.

The SDEI driver provides functions (e.g. apei_sdei_register_ghes(),
apei_sdei_unregister_ghes()) to register or unregister event callback
for dispatcher in firmware. When the GHES driver probing, it registers
the corresponding callback according to the notification type specified
by GHES. If the GHES notification type is SDEI, the GHES driver will
call apei_sdei_register_ghes() to register event call.

When the firmware emits an event, it migrates the handling of the event
into the kernel at the registered entry-point __sdei_asm_handler. And
finally, the kernel will call the registered event callback and return
status_code to indicate the status of event handling. SDEI_EV_FAILED
indicates that the kernel failed to handle the event.

Consequently, when an error occurs during kernel booting, the kernel is
unable to handle and report errors until the GHES driver is initialized
by device_initcall(), in which the event callback is registered.  For
example, when the kernel booting, the console logs many times from
firmware before GHES drivers init in our platform:

   Trip in MM PCIe RAS handle(Intr:910)
   Clean PE[1.1.1] ERR_STS:0x4000100 -> 0 INT_STS:F0000000
   Find RP(98:1.0)
   --Walk dev(98:1.0) CE:0 UCE:4000
   ...
   ERROR:   sdei_dispatch_event(32a) ret:-1
   --handler(910) end

If I understand correctly, the firmware noticed an error, tried to
report it to the kernel, and is complaining because the kernel isn't
ready to handle it yet.  And the reason for this patch is to reduce
these complaints from the firmware.

My thoughts exactly :)

quoted

That doesn't seem like a very good reason for this patch.  There is
*always* a window before the kernel is ready to handle events from the
firmware.

Yes, there is always a window. But if we could do better in kernel that
reduces the window by 90% (from 33 seconds to 3 second), why not?

quoted

Why is the firmware noticing these errors in the first place?  If
you're seeing these complaints regularly, my guess is that either you
have some terrible hardware or (more likely) the firmware isn't
clearing some expected error condition correctly.  For example, maybe
the Unsupported Request errors that happen while enumerating PCIe
devices are being reported.

If you register the callback function, the kernel will now start
seeing these error reports.  What happens then?  Does the kernel log
the errors somewhere?  Is that better than the current situation where
the firmware logs them?

Yep, it is a hardware issue. The firmware only logs in console
(ttyAMA0) and we can not see it in kernel side. After the kernel
starts seeing these error reports, we could see EDAC/ghes and
efi/cper detailed logs in dmesg. We did not notice the problem until
we check the console log, which inspired us to reduce the window
when kernel startup, so that we can see the message clearly and
properly. I think the intuition is to check the log of dmesg, not
the console.

quoted

quoted

However, I DO think that:

  - Removing acpi_hest_init() from acpi_pci_root_init(), and

  - Converting ghes_init() and sdei_init() from initcalls to explicit
    calls

are very good reasons to do something like this patch because HEST is
not PCI-specific, and IMO, explicit calls are better than initcalls
because initcall ordering is implicit and not well-defined within a
level.

Haha, if the above reasons still don't convince you, I would like to
accept yours :) Should we do it in one patch or separate it into two
patches?

IMO, this can be done in one patch, but this would probably go via
Rafael.

Yes, that would make sense IMO.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel