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;
Date:   Mon, 19 Jun 2023 10:47:17 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Waiman Long <longman@redhat.com>
Cc:     Robin Jarry <rjarry@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        "H.Peter Anvin" <hpa@zytor.com>,
        Josh Poimboeuf <jpoimboe@kernel.org>,
        Pawan Gupta <pawan.kumar.gupta@linux.intel.com>,
        Juri Lelli <juri.lelli@redhat.com>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
        Daniel Bristot de Oliveira <bristot@redhat.com>,
        Valentin Schneider <vschneid@redhat.com>,
        linux-kernel@vger.kernel.org, x86@kernel.org,
        Joe Mario <jmario@redhat.com>
Subject: Re: [PATCH 0/5] x86/speculation: Disable IBRS when idle
Message-ID: <20230619084717.GJ4253@hirez.programming.kicks-ass.net>
References: <20230616200003.745742-1-longman@redhat.com>
 <CTECMFWMMST3.9FTWRDG7FFKQ@ringo>
 <20230617122115.GA1830050@hirez.programming.kicks-ass.net>
 <55219f3b-992d-ccc3-ba29-7bf33465b5cc@redhat.com>
 <ef2490a0-2768-ba70-e89d-59c0df76f827@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ef2490a0-2768-ba70-e89d-59c0df76f827@redhat.com>
Precedence: bulk

On Sun, Jun 18, 2023 at 11:25:29PM -0400, Waiman Long wrote:

> > We were testing on the RHEL9.2 kernel which doesn't have your

Then keep the tinkering in the RHEL tree, please.

> We may need to extend your current solution to cover more cases.

See below.

> Perhaps adding a module parameter (e.g. idle_no_ibrs) to force the use
> of intel_idle_ibrs(). BTW, is it really the case that we can't disable
> IBRS when irq is enabled?

No, that is an entirely artificial constraint due to not having
intel_idle_ibrs_irq() and having no desire to deal with the
ramifications of such a thing.

That said; it also doesn't make any sense what so ever to add this. The
reason for having this intel_idle_irq() is for C1 state to improve IRQ
response latency. Adding WRMSRs will obviously regress that.

Specifically, we very intentionally did not add CPUIDLE_FLAG_IBRS to the
very shallow idle states to avoid regressions. These WRMSRs are
*EXPENSIVE*.

Additionally, if you were to go do this with IRQs enabled, then you have
to worry about enabling IBRS again on the interrupt path from kernel.

> The idle thread does not really interact with any user
> applications. I don't think there is any risk of information leakage even if
> we disable IBRS with interrupt enabled. Is my assumption incorrect?

Yes:

 - doing the WRMSR on C1 makes no sense, the C1 state is only picked if
   the idle time expectation is *VERY* short, the WRMSR overhead in that
   case is probably more than the expected idle time.

 - doing the WRMSR with IRQs enabled means you now get to touch the
   interrupt/exception from kernel paths, nobody wants more of this
   crap.

 - the whole IBRS thing is a trainwreck, let it be.

 - finally, T0 runs userspace, T1 goes into C1 idle, disables IBRS,
   enables IRQs, takes an IRQ and now T0 can 'see' everything T1 does in
   kernel space, you loose.

Also, did I say that IBRS sucks? Like really? It is horrific -- step
away and let it be.

The possibly better solution is to make sure nothing untrusted what so
ever runs on the DPDK machine, then you can forget about all the
mitigation nonsense.