tree: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/cache
head: f61050aefc0ca1c0b3e93114eadd0a910a66202b
commit: 0438fb1aebf428efcdce64ef4ec610e93e0006f9 [29/38] x86/intel_rdt: Pseudo-lock region creation/removal core
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
git checkout 0438fb1aebf428efcdce64ef4ec610e93e0006f9
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c: In function 'pseudo_lock_fn':
>> arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c:363:1: warning: unsupported size for integer register
}
^
>> arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c:363:1: warning: unsupported size for integer register
vim +363 arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
280
281 /*
282 * Make sure none of the allocated memory is cached. If it is we
283 * will get a cache hit in below loop from outside of pseudo-locked
284 * region.
285 * wbinvd (as opposed to clflush/clflushopt) is required to
286 * increase likelihood that allocated cache portion will be filled
287 * with associated memory.
288 */
289 native_wbinvd();
290
291 /*
292 * Always called with interrupts enabled. By disabling interrupts
293 * ensure that we will not be preempted during this critical section.
294 */
295 local_irq_disable();
296
297 /*
298 * Call wrmsr and rdmsr as directly as possible to avoid tracing
299 * clobbering local register variables or affecting cache accesses.
300 *
301 * Disable the hardware prefetcher so that when the end of the memory
302 * being pseudo-locked is reached the hardware will not read beyond
303 * the buffer and evict pseudo-locked memory read earlier from the
304 * cache.
305 */
306 __wrmsr(MSR_MISC_FEATURE_CONTROL, prefetch_disable_bits, 0x0);
307 closid_p = this_cpu_read(pqr_state.cur_closid);
308 rmid_p = this_cpu_read(pqr_state.cur_rmid);
309 mem_r = plr->kmem;
310 size = plr->size;
311 line_size = plr->line_size;
312 /*
313 * Critical section begin: start by writing the closid associated
314 * with the capacity bitmask of the cache region being
315 * pseudo-locked followed by reading of kernel memory to load it
316 * into the cache.
317 */
318 __wrmsr(IA32_PQR_ASSOC, rmid_p, rdtgrp->closid);
319 /*
320 * Cache was flushed earlier. Now access kernel memory to read it
321 * into cache region associated with just activated plr->closid.
322 * Loop over data twice:
323 * - In first loop the cache region is shared with the page walker
324 * as it populates the paging structure caches (including TLB).
325 * - In the second loop the paging structure caches are used and
326 * cache region is populated with the memory being referenced.
327 */
328 for (i = 0; i < size; i += PAGE_SIZE) {
329 /*
330 * Add a barrier to prevent speculative execution of this
331 * loop reading beyond the end of the buffer.
332 */
333 rmb();
334 asm volatile("mov (%0,%1,1), %%eax\n\t"
335 :
336 : "r" (mem_r), "r" (i)
337 : "%eax", "memory");
338 }
339 for (i = 0; i < size; i += line_size) {
340 /*
341 * Add a barrier to prevent speculative execution of this
342 * loop reading beyond the end of the buffer.
343 */
344 rmb();
345 asm volatile("mov (%0,%1,1), %%eax\n\t"
346 :
347 : "r" (mem_r), "r" (i)
348 : "%eax", "memory");
349 }
350 /*
351 * Critical section end: restore closid with capacity bitmask that
352 * does not overlap with pseudo-locked region.
353 */
354 __wrmsr(IA32_PQR_ASSOC, rmid_p, closid_p);
355
356 /* Re-enable the hardware prefetcher(s) */
357 wrmsr(MSR_MISC_FEATURE_CONTROL, 0x0, 0x0);
358 local_irq_enable();
359
360 plr->thread_done = 1;
361 wake_up_interruptible(&plr->lock_thread_wq);
362 return 0;
> 363 }
364
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
0-day kbuild test robot reported the following issue:
arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c: In function 'pseudo_lock_fn':
>> arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c:363:1: warning: unsupported size for integer register
}
^
>> arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c:363:1: warning: unsupported size for integer register
vim +363 arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
361 wake_up_interruptible(&plr->lock_thread_wq);
362 return 0;
> 363 }
364
The issue is earlier in the code, and in more locations, where (in 32-bit)
a u64 variable (i below) is used as the value of a 32-bit register:
asm volatile("mov (%0,%1,1), %%eax\n\t"
:
: "r" (mem_r), "r" (i)
: "%eax", "memory");
The behavior in this case would be that only the 32 bit lower part of the
variable is passed. The variable is used as a counter in a for loop that
iterates over the pseudo-locked region and its maximum value is thus the
largest size of a pseudo-locked region. No pseudo-locked region currently
supported would need more than 32-bits for its size. There is thus no
potential for harm in the current state when using the u64 variable, but
this should be fixed.
Modify the variable with which the registers are initialized to be 64-bit
when used for 64-bit register and 32-bit when used for 32-bit register.
Signed-off-by: Reinette Chatre <[email protected]>
---
Thank you 0-day!
arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c b/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
index 3c24752873e5..c95de5bc45a1 100644
--- a/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
+++ b/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
@@ -421,7 +421,11 @@ static int pseudo_lock_fn(void *_rdtgrp)
struct rdtgroup *rdtgrp = _rdtgrp;
struct pseudo_lock_region *plr = rdtgrp->plr;
u32 rmid_p, closid_p;
+#ifdef CONFIG_X86_64
u64 i;
+#else
+ u32 i;
+#endif
#ifdef CONFIG_KASAN
/*
* The registers used for local register variables are also used
@@ -877,7 +881,11 @@ static int measure_cycles_lat_fn(void *_plr)
{
struct pseudo_lock_region *plr = _plr;
u64 start, end;
+#ifdef CONFIG_X86_64
u64 i;
+#else
+ u32 i;
+#endif
#ifdef CONFIG_KASAN
/*
* The registers used for local register variables are also used
@@ -931,7 +939,11 @@ static int measure_cycles_perf_fn(void *_plr)
struct pseudo_lock_region *plr = _plr;
unsigned long long l2_hits, l2_miss;
u64 l2_hit_bits, l2_miss_bits;
+#ifdef CONFIG_X86_64
u64 i;
+#else
+ u32 i;
+#endif
#ifdef CONFIG_KASAN
/*
* The registers used for local register variables are also used
--
2.17.0
Commit-ID: a9347363748fbfc25d0ba7b60d866b8a10ca4561
Gitweb: https://git.kernel.org/tip/a9347363748fbfc25d0ba7b60d866b8a10ca4561
Author: Reinette Chatre <[email protected]>
AuthorDate: Tue, 19 Jun 2018 23:19:27 -0700
Committer: Thomas Gleixner <[email protected]>
CommitDate: Wed, 20 Jun 2018 10:26:52 +0200
x86/intel_rdt: Fix passing of value to 32-bit register
0-day kbuild test robot reported the following issue:
arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c: In function 'pseudo_lock_fn':
>> arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c:363:1: warning: unsupported size for integer register
}
^
>> arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c:363:1: warning: unsupported size for integer register
vim +363 arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
361 wake_up_interruptible(&plr->lock_thread_wq);
362 return 0;
> 363 }
364
The issue is earlier in the code, and in more locations, where (in 32-bit)
a u64 variable (i below) is used as the value of a 32-bit register:
asm volatile("mov (%0,%1,1), %%eax\n\t"
:
: "r" (mem_r), "r" (i)
: "%eax", "memory");
The behavior in this case would be that only the 32 bit lower part of the
variable is passed. The variable is used as a counter in a for loop that
iterates over the pseudo-locked region and its maximum value is thus the
largest size of a pseudo-locked region. No pseudo-locked region currently
supported would need more than 32-bits for its size. There is thus no
potential for harm in the current state when using the u64 variable, but
this should be fixed.
Modify the variable with which the registers are initialized to be 64-bit
when used for 64-bit register and 32-bit when used for 32-bit register.
Fixes: 0438fb1aebf4 ("x86/intel_rdt: Pseudo-lock region creation/removal core")
Reported-by: [email protected]
Signed-off-by: Reinette Chatre <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Link: https://lkml.kernel.org/r/5773274f9947c4d8becbabd2655bd1628f060147.1529474468.git.reinette.chatre@intel.com
---
arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c b/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
index 0d44dc1f7146..a1670e50d6ce 100644
--- a/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
+++ b/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
@@ -415,7 +415,11 @@ static int pseudo_lock_fn(void *_rdtgrp)
struct rdtgroup *rdtgrp = _rdtgrp;
struct pseudo_lock_region *plr = rdtgrp->plr;
u32 rmid_p, closid_p;
+#ifdef CONFIG_X86_64
u64 i;
+#else
+ u32 i;
+#endif
#ifdef CONFIG_KASAN
/*
* The registers used for local register variables are also used
@@ -870,7 +874,11 @@ static int measure_cycles_lat_fn(void *_plr)
{
struct pseudo_lock_region *plr = _plr;
u64 start, end;
+#ifdef CONFIG_X86_64
u64 i;
+#else
+ u32 i;
+#endif
#ifdef CONFIG_KASAN
/*
* The registers used for local register variables are also used
@@ -924,7 +932,11 @@ static int measure_cycles_perf_fn(void *_plr)
struct pseudo_lock_region *plr = _plr;
unsigned long long l2_hits, l2_miss;
u64 l2_hit_bits, l2_miss_bits;
+#ifdef CONFIG_X86_64
u64 i;
+#else
+ u32 i;
+#endif
#ifdef CONFIG_KASAN
/*
* The registers used for local register variables are also used
Commit-ID: a449a5325528c1ef2c5efae0fa422bf1c4a270e6
Gitweb: https://git.kernel.org/tip/a449a5325528c1ef2c5efae0fa422bf1c4a270e6
Author: Ingo Molnar <[email protected]>
AuthorDate: Tue, 19 Jun 2018 23:19:27 -0700
Committer: Ingo Molnar <[email protected]>
CommitDate: Thu, 21 Jun 2018 11:21:35 +0200
x86/intel_rdt: Simplify index type
Simplify this pattern:
#ifdef CONFIG_X86_64
u64 i;
#else
u32 i;
#endif
... to the more natural and shorter one:
unsigned long i;
No change in functionality.
Acked-by Thomas Gleixner <[email protected]>
Cc: Reinette Chatre <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Fixes: 0438fb1aebf4 ("x86/intel_rdt: Pseudo-lock region creation/removal core")
Link: https://lkml.kernel.org/r/5773274f9947c4d8becbabd2655bd1628f060147.1529474468.git.reinette.chatre@intel.com
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c | 18 +++---------------
1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c b/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
index a1670e50d6ce..df68972d5e3e 100644
--- a/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
+++ b/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
@@ -415,11 +415,7 @@ static int pseudo_lock_fn(void *_rdtgrp)
struct rdtgroup *rdtgrp = _rdtgrp;
struct pseudo_lock_region *plr = rdtgrp->plr;
u32 rmid_p, closid_p;
-#ifdef CONFIG_X86_64
- u64 i;
-#else
- u32 i;
-#endif
+ unsigned long i;
#ifdef CONFIG_KASAN
/*
* The registers used for local register variables are also used
@@ -874,11 +870,7 @@ static int measure_cycles_lat_fn(void *_plr)
{
struct pseudo_lock_region *plr = _plr;
u64 start, end;
-#ifdef CONFIG_X86_64
- u64 i;
-#else
- u32 i;
-#endif
+ unsigned long i;
#ifdef CONFIG_KASAN
/*
* The registers used for local register variables are also used
@@ -932,11 +924,7 @@ static int measure_cycles_perf_fn(void *_plr)
struct pseudo_lock_region *plr = _plr;
unsigned long long l2_hits, l2_miss;
u64 l2_hit_bits, l2_miss_bits;
-#ifdef CONFIG_X86_64
- u64 i;
-#else
- u32 i;
-#endif
+ unsigned long i;
#ifdef CONFIG_KASAN
/*
* The registers used for local register variables are also used