Re: [PATCH v10 02/25] x86/mm: define ARCH_SUPPORTS_SPECULATIVE_PAGE_FAULT
From: Punit Agrawal
Date: Mon May 14 2018 - 11:05:18 EST
Laurent Dufour <ldufour@xxxxxxxxxxxxxxxxxx> writes:
> On 08/05/2018 13:04, Punit Agrawal wrote:
>> Hi Laurent,
>> Laurent Dufour <ldufour@xxxxxxxxxxxxxxxxxx> writes:
>>> Set ARCH_SUPPORTS_SPECULATIVE_PAGE_FAULT which turns on the
>>> Speculative Page Fault handler when building for 64bit.
>>> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>>> Signed-off-by: Laurent Dufour <ldufour@xxxxxxxxxxxxxxxxxx>
>>> arch/x86/Kconfig | 1 +
>>> 1 file changed, 1 insertion(+)
>>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>>> index d8983df5a2bc..ebdeb48e4a4a 100644
>>> --- a/arch/x86/Kconfig
>>> +++ b/arch/x86/Kconfig
>>> @@ -30,6 +30,7 @@ config X86_64
>>> select MODULES_USE_ELF_RELA
>>> select X86_DEV_DMA_OPS
>>> select ARCH_HAS_SYSCALL_WRAPPER
>>> + select ARCH_SUPPORTS_SPECULATIVE_PAGE_FAULT
>> I'd suggest merging this patch with the one making changes to the
>> architectural fault handler towards the end of the series.
>> The Kconfig change is closely tied to the architectural support for SPF
>> and makes sense to be in a single patch.
>> If there's a good reason to keep them as separate patches, please move
>> the architecture Kconfig changes after the patch adding fault handler
>> It's better to enable the feature once the core infrastructure is merged
>> rather than at the beginning of the series to avoid potential bad
>> fallout from incomplete functionality during bisection.
> Indeed bisection was the reason why Andrew asked me to push the configuration
> enablement on top of the series (https://lkml.org/lkml/2017/10/10/1229).
The config options have gone through another round of splitting (between
core and architecture) since that comment. I agree that it still makes
sense to define the core config - CONFIG_SPECULATIVE_PAGE_FAULT early
Just to clarify, my suggestion was to only move the architecture configs
> I also think it would be better to have the architecture enablement in on patch
> but that would mean that the code will not be build when bisecting without the
> latest patch adding the per architecture code.
I don't see that as a problem. But if I'm in the minority, I am OK with
leaving things as they are as well.