Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Nikolay Borisov <nik.borisov@suse.com>, x86@kernel.org
Cc:     linux-kernel@vger.kernel.org, mhocko@suse.com, jslaby@suse.cz,
        Nikolay Borisov <nik.borisov@suse.com>
Subject: Re: [PATCH v3 3/5] x86/entry: Compile entry_SYSCALL32_ignore
 unconditionally
In-Reply-To: <20230616125730.1164989-4-nik.borisov@suse.com>
References: <20230616125730.1164989-1-nik.borisov@suse.com>
 <20230616125730.1164989-4-nik.borisov@suse.com>
Date:   Sun, 18 Jun 2023 23:11:28 +0200
Message-ID: <87pm5sqxvj.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

On Fri, Jun 16 2023 at 15:57, Nikolay Borisov wrote:
> In upcomiing patches entry_SYSCALL32_ignore() could be used even if

comiing?

> CONFIG_IA32_EMULATION is selected but IA32 support is disabled either
> via CONFIG_IA32_EMULATION_DEFAULT_DISABLED or the runtime override.i

override.i ?

Aside of the lack of a spell checker, this sentence is not really
well structured and helpful.

You already introduced CONFIG_IA32_EMULATION_DEFAULT_DISABLED in the
first patch, which is questionable to start with because the config
switch is only fully functional when everything is in place.

So up to that point the command line option and the config switch can
create inconsistent state, no?

So the right thing to do is to introduce the global variable which
controls that first. As it is 'false' it won't do anything, but allows
you to build up the code. The last step adds the config option and the
command line parsing.

Then the changelog might read like this:

 "To limit the IA32 exposure on 64bit kernels while keeping the
  flexibility for the user to enable it when required, the compile time
  enable/disable via CONFIG_IA32_EMULATION is not good enough and will
  be complemented with a kernel command line option.

  Right now entry_SYSCALL32_ignore() is only compiled when
  CONFIG_IA32_EMULATION=n, but boot-time enable- / disablement obviously
  requires it to be unconditionally available.

  Remove the #ifndef CONFIG_IA32_EMULATION guard"

The point is that changelogs need to convey enough information on their
own that they make sense without having access to the full context of the
patch series.

Thanks,

        tglx