Hi Boris, On 01/10/18 18:59, Borislav Petkov wrote: > On Fri, Sep 21, 2018 at 11:16:52PM +0100, James Morse wrote: >> Now that there are two users of the estatus queue, and likely to be more, >> make it a Kconfig symbol selected by the appropriate notification. We >> can move the ARCH_HAVE_NMI_SAFE_CMPXCHG checks in here too. > > Ok, question: why do we need to complicate things at all? I mean, why do > we even need a Kconfig symbol? Before patch 4, this was behind CONFIG_HAVE_ACPI_APEI_NMI, (so it made use of an existing kconfig symbol), and there was only one user x86:NMI. The ACPI spec has four ~NMI notifications, so far the support for these in Linux has been selectable separately. If you build the kernel without any of them then this code would be unused, and generate warnings because all those users are behind #ifdef too. > This code is being used by two arches now so why not simply build it in > unconditionally and be done with it. The couple of KB saved are simply > not worth the effort, especially if it is going to end up being enabled > on 99% of the setups... I'm all in favour of letting the compiler work it out, but the existing ghes code has #ifdef/#else all over the place. This is 'keeping the style'. I assumed it was done this way to support an older compiler on x86, (I see that jumped from 3.2 to 4.6 with commit cafa0010cd51) We could strip the lot away to a few IS_ENABLED() in ghes_probe() and the memory_failure()/AER calls if you'd prefer. Thanks, James
