[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250304110032.GEZ8bdUOg2WLUrhMcm@fat_crate.local>
Date: Tue, 4 Mar 2025 12:00:32 +0100
From: Borislav Petkov <bp@...en8.de>
To: Dave Hansen <dave.hansen@...el.com>
Cc: Rik van Riel <riel@...riel.com>, x86@...nel.org,
linux-kernel@...r.kernel.org, peterz@...radead.org,
dave.hansen@...ux.intel.com, zhengqi.arch@...edance.com,
nadav.amit@...il.com, thomas.lendacky@....com, kernel-team@...a.com,
linux-mm@...ck.org, akpm@...ux-foundation.org, jackmanb@...gle.com,
jannh@...gle.com, mhklinux@...look.com, andrew.cooper3@...rix.com,
Manali.Shukla@....com, mingo@...nel.org
Subject: Re: [PATCH v14 03/13] x86/mm: add INVLPGB support code
On Mon, Mar 03, 2025 at 11:23:58AM -0800, Dave Hansen wrote:
> Here's a plain diff if you just want to squish it in.
> diff --git a/arch/x86/include/asm/tlb.h b/arch/x86/include/asm/tlb.h
> index 5375145eb9596..3bd617c204346 100644
> --- a/arch/x86/include/asm/tlb.h
> +++ b/arch/x86/include/asm/tlb.h
> @@ -28,6 +28,11 @@ static inline void invlpg(unsigned long addr)
> asm volatile("invlpg (%0)" ::"r" (addr) : "memory");
> }
>
> +enum invlpgb_stride {
Right, this is an address stride, as the text calls it.
> + NO_STRIDE = 0,
> + PTE_STRIDE = 0,
Ok, so those are confusing. No stride is PTE stride so let's just zap
NO_STRIDE.
> + PMD_STRIDE = 1
> +};
>
> /*
> * INVLPGB does broadcast TLB invalidation across all the CPUs in the system.
...
> static inline void invlpgb_flush_user_nr_nosync(unsigned long pcid,
> unsigned long addr,
> u16 nr,
> bool pmd_stride)
You're relying on the fact that true == PMD_STRIDE and false to PTE_STRIDE but
let's make it Right(tm), see below.
Rest looks ok.
IOW, I'm merging this into patch 3:
diff --git a/arch/x86/include/asm/tlb.h b/arch/x86/include/asm/tlb.h
index 5375145eb959..6718835c3b0c 100644
--- a/arch/x86/include/asm/tlb.h
+++ b/arch/x86/include/asm/tlb.h
@@ -28,6 +28,10 @@ static inline void invlpg(unsigned long addr)
asm volatile("invlpg (%0)" ::"r" (addr) : "memory");
}
+enum addr_stride {
+ PTE_STRIDE = 0,
+ PMD_STRIDE = 1
+};
/*
* INVLPGB does broadcast TLB invalidation across all the CPUs in the system.
@@ -48,10 +52,10 @@ static inline void invlpg(unsigned long addr)
*/
static inline void __invlpgb(unsigned long asid, unsigned long pcid,
unsigned long addr, u16 nr_pages,
- bool pmd_stride, u8 flags)
+ enum addr_stride stride, u8 flags)
{
u32 edx = (pcid << 16) | asid;
- u32 ecx = (pmd_stride << 31) | (nr_pages - 1);
+ u32 ecx = (stride << 31) | (nr_pages - 1);
u64 rax = addr | flags;
/* The low bits in rax are for flags. Verify addr is clean. */
@@ -78,33 +82,38 @@ static inline void __tlbsync(void)
/*
* INVLPGB can be targeted by virtual address, PCID, ASID, or any combination
* of the three. For example:
- * - INVLPGB_VA | INVLPGB_INCLUDE_GLOBAL: invalidate all TLB entries at the address
- * - INVLPGB_PCID: invalidate all TLB entries matching the PCID
+ * - FLAG_VA | FLAG_INCLUDE_GLOBAL: invalidate all TLB entries at the address
+ * - FLAG_PCID: invalidate all TLB entries matching the PCID
*
- * The first can be used to invalidate (kernel) mappings at a particular
+ * The first is used to invalidate (kernel) mappings at a particular
* address across all processes.
*
* The latter invalidates all TLB entries matching a PCID.
*/
-#define INVLPGB_VA BIT(0)
-#define INVLPGB_PCID BIT(1)
-#define INVLPGB_ASID BIT(2)
-#define INVLPGB_INCLUDE_GLOBAL BIT(3)
-#define INVLPGB_FINAL_ONLY BIT(4)
-#define INVLPGB_INCLUDE_NESTED BIT(5)
+#define INVLPGB_FLAG_VA BIT(0)
+#define INVLPGB_FLAG_PCID BIT(1)
+#define INVLPGB_FLAG_ASID BIT(2)
+#define INVLPGB_FLAG_INCLUDE_GLOBAL BIT(3)
+#define INVLPGB_FLAG_FINAL_ONLY BIT(4)
+#define INVLPGB_FLAG_INCLUDE_NESTED BIT(5)
+
+/* The implied mode when all bits are clear: */
+#define INVLPGB_MODE_ALL_NONGLOBALS 0UL
static inline void invlpgb_flush_user_nr_nosync(unsigned long pcid,
unsigned long addr,
- u16 nr,
- bool pmd_stride)
+ u16 nr, bool stride)
{
- __invlpgb(0, pcid, addr, nr, pmd_stride, INVLPGB_PCID | INVLPGB_VA);
+ enum addr_stride str = stride ? PMD_STRIDE : PTE_STRIDE;
+ u8 flags = INVLPGB_FLAG_PCID | INVLPGB_FLAG_VA;
+
+ __invlpgb(0, pcid, addr, nr, str, flags);
}
/* Flush all mappings for a given PCID, not including globals. */
static inline void invlpgb_flush_single_pcid_nosync(unsigned long pcid)
{
- __invlpgb(0, pcid, 0, 1, 0, INVLPGB_PCID);
+ __invlpgb(0, pcid, 0, 1, PTE_STRIDE, INVLPGB_FLAG_PCID);
}
/* Flush all mappings, including globals, for all PCIDs. */
@@ -117,21 +126,21 @@ static inline void invlpgb_flush_all(void)
* as it is cheaper.
*/
guard(preempt)();
- __invlpgb(0, 0, 0, 1, 0, INVLPGB_INCLUDE_GLOBAL);
+ __invlpgb(0, 0, 0, 1, PTE_STRIDE, INVLPGB_FLAG_INCLUDE_GLOBAL);
__tlbsync();
}
/* Flush addr, including globals, for all PCIDs. */
static inline void invlpgb_flush_addr_nosync(unsigned long addr, u16 nr)
{
- __invlpgb(0, 0, addr, nr, 0, INVLPGB_INCLUDE_GLOBAL);
+ __invlpgb(0, 0, addr, nr, PTE_STRIDE, INVLPGB_FLAG_INCLUDE_GLOBAL);
}
/* Flush all mappings for all PCIDs except globals. */
static inline void invlpgb_flush_all_nonglobals(void)
{
guard(preempt)();
- __invlpgb(0, 0, 0, 1, 0, 0);
+ __invlpgb(0, 0, 0, 1, PTE_STRIDE, INVLPGB_MODE_ALL_NONGLOBALS);
__tlbsync();
}
#endif /* _ASM_X86_TLB_H */
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Powered by blists - more mailing lists