Date:   Thu, 2 Dec 2021 13:58:51 +0100
From:   Joerg Roedel <jroedel@suse.de>
To:     Borislav Petkov <bp@alien8.de>
Cc:     Joerg Roedel <joro@8bytes.org>, x86@kernel.org,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, hpa@zytor.com,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Andy Lutomirski <luto@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Mike Rapoport <rppt@kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Brijesh Singh <brijesh.singh@amd.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 3/4] x86/mm: Flush global TLB when switching to
 trampoline page-table
Message-ID: <YajDC/tgx7qR/UcW@suse.de>
References: <20211001154817.29225-1-joro@8bytes.org>
 <20211001154817.29225-4-joro@8bytes.org>
 <YXki1Zx4jCmikBuf@zn.tnic>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <YXki1Zx4jCmikBuf@zn.tnic>
Precedence: bulk

On Wed, Oct 27, 2021 at 11:58:45AM +0200, Borislav Petkov wrote:
> On Fri, Oct 01, 2021 at 05:48:16PM +0200, Joerg Roedel wrote:
> > +void load_trampoline_pgtable(void)
> > +{
> > +#ifdef CONFIG_X86_32
> > +	load_cr3(initial_page_table);
> > +#else
> > +	/* Exiting long mode will fail if CR4.PCIDE is set. */
> 
> So this comment is not valid anymore if this is a separate function - it
> is valid only when that function is called in reboot.c so I guess you
> should leave that comment there.

Okay, but in the caller it is not visible the CR4.PCID is disabled in
this function. I'd rather update the comment to tell that the function
is called before transitioning to real mode?

> 
> > +	if (boot_cpu_has(X86_FEATURE_PCID))
> > +		cr4_clear_bits(X86_CR4_PCIDE);
> > +
> > +	write_cr3(real_mode_header->trampoline_pgd);
> 
> Is there any significance to the reordering of those calls here? The
> commit message doesn't say...

Yes, the call to cr4_clear_bits() is not safe anymore on the trampoline
page-table, because the per-cpu areas are not fully mapped anymore.

This changes with the next patch, but its nevertheless more robust to
minimize the code running on the trampoline page-table.

I will add that to the commit message.

Regards,

-- 
J?rg R?del
jroedel@suse.de

SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 N?rnberg
Germany
 
(HRB 36809, AG N?rnberg)
Gesch?ftsf?hrer: Ivo Totev