Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     "Bae, Chang Seok" <chang.seok.bae@intel.com>
Cc:     "bp@suse.de" <bp@suse.de>, "Lutomirski, Andy" <luto@kernel.org>,
        "mingo@kernel.org" <mingo@kernel.org>,
        "x86@kernel.org" <x86@kernel.org>,
        "Brown, Len" <len.brown@intel.com>,
        "lenb@kernel.org" <lenb@kernel.org>,
        "Hansen, Dave" <dave.hansen@intel.com>,
        "Macieira, Thiago" <thiago.macieira@intel.com>,
        "Liu, Jing2" <jing2.liu@intel.com>,
        "Shankar, Ravi V" <ravi.v.shankar@intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v10 13/28] x86/fpu/xstate: Use feature disable (XFD) to
 protect dynamic user state
In-Reply-To: <E5C66E6E-8AB8-4258-814D-C8E94446E2C4@intel.com>
References: <20210825155413.19673-1-chang.seok.bae@intel.com>
 <20210825155413.19673-14-chang.seok.bae@intel.com> <871r546b52.ffs@tglx>
 <87wnmw4w80.ffs@tglx> <E5C66E6E-8AB8-4258-814D-C8E94446E2C4@intel.com>
Date:   Mon, 04 Oct 2021 14:35:18 +0200
Message-ID: <87bl45ge7t.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Sun, Oct 03 2021 at 22:38, Chang Seok Bae wrote:
> On Oct 1, 2021, at 08:10, Thomas Gleixner <tglx@linutronix.de> wrote:
>> 	local_irq_enable();
>
> v1 had some similar ones (not the same though) [1]. FWIW, I think Andy=E2=
=80=99s point
> is worth to be noted here:
>
>     First, you can't just enable IRQs here. If IRQs are off, they're off =
for a
>     reason. Secondly, if they're *on*, you just forgot that fact.

The #NM comes from user space where interrupts are always enabled. So we
can enable interrupts _after_ doing the sanity checks.

Also we reenable interrupts in various other trap handlers when the trap
comes from user space as well. That's perfectly fine and required. How
would e.g. fault handling or single stepping ever work otherwise?

I have no idea where you had places the local_irq_enable(), but the code
I outlined is correct.

Thanks,

        tglx