RE: [PATCH v3 10/10] x86/split_lock: Handle #AC exception for split lock
> From: Ingo Molnar [mailto:mingo.kernel@gmail.com] On Behalf Of Ingo Molnar > > On Mon, Feb 11, 2019 at 11:53:39AM +0100, Ingo Molnar wrote: > > > > + do_trap(trapnr, signr, str, regs, error_code, > > > > BUS_ADRALN, > NULL); > > > > + } > > > > +} > > > > > > Is there any experience with how frequently this signal is killing > > > user-space processes on a modern distro? Any expectation of how > > > frequent such SIGBUS task terminations are going to be in the field? > > > > We did pretty intensive testing internally (zero day tests, many > > engineers and testers use the patches in their dailly work, etc). So > > far I'm not aware of any user space process hiting split lock issue. I > > guess GCC or other compilers takes care of the issue already. Inline > > assembly code may hit the issue if code is not right, but there are > > fewer inline assembly code in user space. > > > > So far we only find two kernel split lock issues and fix them in the > > first two patches in this patch set. We also find one BIOS split lock > > issue internally which has been fixed in production BIOS. > > > > Thanks. > > Ok, still, for binary compatibility's sake, could you please add a sysctl knob > (root-only, etc.) and a kernel boot parameter that disables this check? In this patch set, we already extend kernel option "clearcpuid=" to support CPU cap flags. So "clearcpuid=split_lock_detection" can disable this split lock check during boot time. In next version, I will add "/sys/kernel/split_lock_detection" to enable or disable the split lock check during run time. Hopefully these work. Thanks. -Fenghua
Re: [PATCH v3 10/10] x86/split_lock: Handle #AC exception for split lock
* Fenghua Yu wrote: > On Mon, Feb 11, 2019 at 11:53:39AM +0100, Ingo Molnar wrote: > > > + do_trap(trapnr, signr, str, regs, error_code, BUS_ADRALN, NULL); > > > + } > > > +} > > > > Is there any experience with how frequently this signal is killing > > user-space processes on a modern distro? Any expectation of how frequent > > such SIGBUS task terminations are going to be in the field? > > We did pretty intensive testing internally (zero day tests, many engineers > and testers use the patches in their dailly work, etc). So far I'm not > aware of any user space process hiting split lock issue. I guess GCC or > other compilers takes care of the issue already. Inline assembly code may > hit the issue if code is not right, but there are fewer inline assembly > code in user space. > > So far we only find two kernel split lock issues and fix them in the first > two patches in this patch set. We also find one BIOS split lock issue > internally which has been fixed in production BIOS. > > Thanks. Ok, still, for binary compatibility's sake, could you please add a sysctl knob (root-only, etc.) and a kernel boot parameter that disables this check? Looks good otherwise. Thanks, Ingo
Re: [PATCH v3 10/10] x86/split_lock: Handle #AC exception for split lock
On Mon, Feb 11, 2019 at 11:53:39AM +0100, Ingo Molnar wrote: > > + do_trap(trapnr, signr, str, regs, error_code, BUS_ADRALN, NULL); > > + } > > +} > > Is there any experience with how frequently this signal is killing > user-space processes on a modern distro? Any expectation of how frequent > such SIGBUS task terminations are going to be in the field? We did pretty intensive testing internally (zero day tests, many engineers and testers use the patches in their dailly work, etc). So far I'm not aware of any user space process hiting split lock issue. I guess GCC or other compilers takes care of the issue already. Inline assembly code may hit the issue if code is not right, but there are fewer inline assembly code in user space. So far we only find two kernel split lock issues and fix them in the first two patches in this patch set. We also find one BIOS split lock issue internally which has been fixed in production BIOS. Thanks. -Fenghua
Re: [PATCH v3 10/10] x86/split_lock: Handle #AC exception for split lock
* Fenghua Yu wrote:
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -61,6 +61,7 @@
> #include
> #include
> #include
> +#include
>
> #ifdef CONFIG_X86_64
> #include
> @@ -292,9 +293,36 @@ DO_ERROR(X86_TRAP_OLD_MF, SIGFPE, 0, NULL,
> "coprocessor segment overru
> DO_ERROR(X86_TRAP_TS, SIGSEGV, 0, NULL, "invalid TSS",
> invalid_TSS)
> DO_ERROR(X86_TRAP_NP, SIGBUS, 0, NULL, "segment not present",
> segment_not_present)
> DO_ERROR(X86_TRAP_SS, SIGBUS, 0, NULL, "stack segment",
> stack_segment)
> -DO_ERROR(X86_TRAP_AC, SIGBUS, BUS_ADRALN, NULL, "alignment check",
> alignment_check)
> #undef IP
>
> +dotraplinkage void do_alignment_check(struct pt_regs *regs, long error_code)
> +{
> + unsigned int trapnr = X86_TRAP_AC;
> + char str[] = "alignment check";
> + int signr = SIGBUS;
> + int ret;
> +
> + RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
> +
> + if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, signr) !=
> + NOTIFY_STOP) {
> + /* #AC exception could be handled by split lock handler. */
> + ret = do_ac_split_lock(regs);
> + if (ret) {
> + cond_local_irq_enable(regs);
> +
> + return;
> + }
> +
> + cond_local_irq_enable(regs);
> + /*
> + * If not processed by split lock handler, go to generic
> + * #AC handler.
> + */
> + do_trap(trapnr, signr, str, regs, error_code, BUS_ADRALN, NULL);
> + }
> +}
Is there any experience with how frequently this signal is killing
user-space processes on a modern distro? Any expectation of how frequent
such SIGBUS task terminations are going to be in the field?
Thanks,
Ingo
Re: [PATCH v3 10/10] x86/split_lock: Handle #AC exception for split lock
Hi Fenghua,
I love your patch! Yet something to improve:
[auto build test ERROR on tip/auto-latest]
[also build test ERROR on v5.0-rc4 next-20190204]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Fenghua-Yu/x86-split_lock-Enable-AC-exception-for-split-locked-accesses/20190204-162843
config: x86_64-randconfig-k3-02041323 (attached as .config)
compiler: gcc-8 (Debian 8.2.0-14) 8.2.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
ld: arch/x86/kernel/traps.o: in function `do_alignment_check':
arch/x86/kernel/traps.c:310: undefined reference to `do_ac_split_lock'
ld: arch/x86/kernel/setup.o: in function `setup_arch':
arch/x86/kernel/setup.c:965: undefined reference to `set_ac_split_lock'
ld: arch/x86/kernel/smpboot.o: in function `start_secondary':
>> arch/x86/kernel/smpboot.c:235: undefined reference to `set_ac_split_lock'
vim +235 arch/x86/kernel/smpboot.c
206
207 static int cpu0_logical_apicid;
208 static int enable_start_cpu0;
209 /*
210 * Activate a secondary processor.
211 */
212 static void notrace start_secondary(void *unused)
213 {
214 /*
215 * Don't put *anything* except direct CPU state initialization
216 * before cpu_init(), SMP booting is too fragile that we want to
217 * limit the things done here to the most necessary things.
218 */
219 if (boot_cpu_has(X86_FEATURE_PCID))
220 __write_cr4(__read_cr4() | X86_CR4_PCIDE);
221
222 #ifdef CONFIG_X86_32
223 /* switch away from the initial page table */
224 load_cr3(swapper_pg_dir);
225 /*
226 * Initialize the CR4 shadow before doing anything that could
227 * try to read it.
228 */
229 cr4_init_shadow();
230 __flush_tlb_all();
231 #endif
232 load_current_idt();
233 cpu_init();
234
> 235 set_ac_split_lock();
236
237 x86_cpuinit.early_percpu_clock_init();
238 preempt_disable();
239 smp_callin();
240
241 enable_start_cpu0 = 0;
242
243 /* otherwise gcc will move up smp_processor_id before the
cpu_init */
244 barrier();
245 /*
246 * Check TSC synchronization with the boot CPU:
247 */
248 check_tsc_sync_target();
249
250 speculative_store_bypass_ht_init();
251
252 /*
253 * Lock vector_lock, set CPU online and bring the vector
254 * allocator online. Online must be set with vector_lock held
255 * to prevent a concurrent irq setup/teardown from seeing a
256 * half valid vector space.
257 */
258 lock_vector_lock();
259 set_cpu_online(smp_processor_id(), true);
260 lapic_online();
261 unlock_vector_lock();
262 cpu_set_state_online(smp_processor_id());
263 x86_platform.nmi_init();
264
265 /* enable local interrupts */
266 local_irq_enable();
267
268 /* to prevent fake stack check failure in clock setup */
269 boot_init_stack_canary();
270
271 x86_cpuinit.setup_percpu_clockev();
272
273 wmb();
274 cpu_startup_entry(CPUHP_AP_ONLINE_IDLE);
275 }
276
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [PATCH v3 10/10] x86/split_lock: Handle #AC exception for split lock
Hi Fenghua,
I love your patch! Yet something to improve:
[auto build test ERROR on tip/auto-latest]
[also build test ERROR on v5.0-rc4 next-20190204]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Fenghua-Yu/x86-split_lock-Enable-AC-exception-for-split-locked-accesses/20190204-162843
config: i386-randconfig-a2-02040849 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
ld: arch/x86/kernel/traps.o: in function `do_alignment_check':
arch/x86/kernel/traps.c:310: undefined reference to `do_ac_split_lock'
ld: arch/x86/kernel/setup.o: in function `setup_arch':
>> arch/x86/kernel/setup.c:965: undefined reference to `set_ac_split_lock'
vim +965 arch/x86/kernel/setup.c
949
950 strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
951 *cmdline_p = command_line;
952
953 /*
954 * x86_configure_nx() is called before parse_early_param() to
detect
955 * whether hardware doesn't support NX (so that the early EHCI
debug
956 * console setup can safely call set_fixmap()). It may then be
called
957 * again from within noexec_setup() during parsing early
parameters
958 * to honor the respective command line option.
959 */
960 x86_configure_nx();
961
962 parse_early_param();
963
964 /* Set up #AC for split lock at the earliest phase. */
> 965 set_ac_split_lock();
966
967 if (efi_enabled(EFI_BOOT))
968 efi_memblock_x86_reserve_range();
969 #ifdef CONFIG_MEMORY_HOTPLUG
970 /*
971 * Memory used by the kernel cannot be hot-removed because Linux
972 * cannot migrate the kernel pages. When memory hotplug is
973 * enabled, we should prevent memblock from allocating memory
974 * for the kernel.
975 *
976 * ACPI SRAT records all hotpluggable memory ranges. But before
977 * SRAT is parsed, we don't know about it.
978 *
979 * The kernel image is loaded into memory at very early time. We
980 * cannot prevent this anyway. So on NUMA system, we set any
981 * node the kernel resides in as un-hotpluggable.
982 *
983 * Since on modern servers, one node could have double-digit
984 * gigabytes memory, we can assume the memory around the kernel
985 * image is also un-hotpluggable. So before SRAT is parsed, just
986 * allocate memory near the kernel image to try the best to keep
987 * the kernel away from hotpluggable memory.
988 */
989 if (movable_node_is_enabled())
990 memblock_set_bottom_up(true);
991 #endif
992
993 x86_report_nx();
994
995 /* after early param, so could get panic from serial */
996 memblock_x86_reserve_range_setup_data();
997
998 if (acpi_mps_check()) {
999 #ifdef CONFIG_X86_LOCAL_APIC
1000 disable_apic = 1;
1001 #endif
1002 setup_clear_cpu_cap(X86_FEATURE_APIC);
1003 }
1004
1005 e820__reserve_setup_data();
1006 e820__finish_early_params();
1007
1008 if (efi_enabled(EFI_BOOT))
1009 efi_init();
1010
1011 dmi_scan_machine();
1012 dmi_memdev_walk();
1013 dmi_set_dump_stack_arch_desc();
1014
1015 /*
1016 * VMware detection requires dmi to be available, so this
1017 * needs to be done after dmi_scan_machine(), for the boot CPU.
1018 */
1019 init_hypervisor_platform();
1020
1021 tsc_early_init();
1022 x86_init.resources.probe_roms();
1023
1024 /* after parse_early_param, so could debug it */
1025 insert_resource(_resource, _resource);
1026 insert_resource(_resource, _resource);
1027 insert_resource(_resource, _resource);
1028
1029 e820_add_kernel_range();
1030 trim_bios_range();
1031 #ifdef CONFIG_X86_32
1032 if (ppro_with_ram_bug()) {
1033 e820__range_update(0x7000ULL, 0x4ULL,
E820_TYPE_RAM,
1034E820_TYPE_RESERVED);
1035 e820__update_table(e820_table);
1036 printk(KERN_INFO "fixed physical RAM map:\n");
1037 e820__print_table("bad_ppro");
1038 }
1039 #else
1040 early_gart_iommu_check();
1041 #endif
1042
1043 /*
1044
