On 04/26, Jacob Shin wrote: > > Implement hardware breakpoint address mask for AMD Family 16h and > above processors. CPUID feature bit indicates hardware support for > DRn_ADDR_MASK MSRs. These masks further qualify DRn/DR7 hardware > breakpoint addresses to allow matching of larger addresses ranges.
Imho, looks good. Just one nit and one question below. > @@ -163,29 +165,8 @@ void arch_uninstall_hw_breakpoint(struct perf_event *bp) > *dr7 &= ~__encode_dr7(i, info->len, info->type); ... > + if (info->mask) > + set_dr_addr_mask(0, i); I agree we should clear addr_mask anyway. But I am just curious, what if we do not? I mean what will the hardware do if this breakpoint was already disabled but the mask wasn't cleared? > @@ -314,11 +300,14 @@ int arch_validate_hwbkpt_settings(struct perf_event *bp) > if (ret) > return ret; > > - ret = -EINVAL; > - > switch (info->len) { > case X86_BREAKPOINT_LEN_1: > align = 0; > + if (info->mask) { > + if (!cpu_has_bpext) > + return -EOPNOTSUPP; > + align = info->mask; OK. But it seems we need a CONFIG_CPU_SUP_AMD-dependant helper for this cpu_has_bpext check? (like we have the nop set_dr_addr_mask() variant if !CONFIG_CPU_SUP_AMD). Suppose that the kernel was compiled without CONFIG_CPU_SUP_AMD. Then perf_event_open(attr => { .bp_len == 16 }) will succeed, but this breakpoint won't actually work as expected? Oleg. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/