2015-05-13 05:24:13

by Maninder Singh

[permalink] [raw]
Subject: [EDT][PATCH 1/1] hw_breakpoint.c :cpu hotplug handling

EP-2DAD0AFA905A4ACB804C4F82A001242F

Subject: [PATCH 1/1] hw_breakpoint.c :cpu hotplug handling

This patch adds support for CPU hotplug, It re-installl all installed watchpoints and breakpoints
back on H/W in case of cpu-hot plug.

Signed-off-by: Vaneet Narang <[email protected]>
Signed-off-by: Maninder Singh <[email protected]>
Reviewed-by: Amit Arora <[email protected]>
Reviewed-by: Ajeet Yadav <[email protected]>
---
arch/arm/kernel/hw_breakpoint.c | 56 +++++++++++++++++++++++++++++++++++++++
1 files changed, 56 insertions(+), 0 deletions(-)

diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
index dc7d0a9..172bfa8 100644
--- a/arch/arm/kernel/hw_breakpoint.c
+++ b/arch/arm/kernel/hw_breakpoint.c
@@ -388,6 +388,56 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
return 0;
}

+/*
+ * reInstall a perf counter breakpoint.
+ */
+void arch_reinstall_hw_breakpoint(int index, int type)
+{
+ struct arch_hw_breakpoint *info;
+ struct perf_event **slots;
+ int ctrl_base, val_base;
+ u32 addr, ctrl;
+ struct perf_event *bp;
+
+ if (type == ARM_ENTRY_BREAKPOINT) {
+ /* Breakpoint */
+ ctrl_base = ARM_BASE_BCR;
+ val_base = ARM_BASE_BVR;
+ slots = (struct perf_event **)__get_cpu_var(bp_on_reg);
+ } else {
+ /* Watchpoint */
+ ctrl_base = ARM_BASE_WCR;
+ val_base = ARM_BASE_WVR;
+ slots = (struct perf_event **)__get_cpu_var(wp_on_reg);
+ }
+
+ bp = slots[index];
+ if (!bp)
+ return;
+
+ info = counter_arch_bp(bp);
+ addr = info->address;
+ ctrl = encode_ctrl_reg(info->ctrl) | 0x1;
+
+
+ /* Override the breakpoint data with the step data. */
+ if (info->step_ctrl.enabled) {
+ addr = info->trigger & ~0x3;
+ ctrl = encode_ctrl_reg(info->step_ctrl);
+ if (info->ctrl.type != ARM_BREAKPOINT_EXECUTE) {
+ index = 0;
+ ctrl_base = ARM_BASE_BCR + core_num_brps;
+ val_base = ARM_BASE_BVR + core_num_brps;
+ }
+ }
+
+ /* Setup the address register. */
+ write_wb_reg(val_base + index, addr);
+
+ /* Setup the control register. */
+ write_wb_reg(ctrl_base + index, ctrl);
+}
+
void arch_uninstall_hw_breakpoint(struct perf_event *bp)
{
struct arch_hw_breakpoint *info = counter_arch_bp(bp);
@@ -1019,6 +1069,12 @@ clear_vcr:
out_mdbgen:
if (enable_monitor_mode())
cpumask_or(&debug_err_mask, &debug_err_mask, cpumask_of(cpu));
+
+ for (i = 0; i < core_num_brps; ++i)
+ arch_reinstall_hw_breakpoint(i, ARM_ENTRY_BREAKPOINT);
+ for (i = 0; i < core_num_wrps; ++i)
+ arch_reinstall_hw_breakpoint(i, ARM_ENTRY_SYNC_WATCHPOINT);
+
}

static int dbg_reset_notify(struct notifier_block *self,
--
1.7.1


Thanks
Maninder Singh????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?


2015-05-13 05:33:19

by Baruch Siach

[permalink] [raw]
Subject: Re: [EDT][PATCH 1/1] hw_breakpoint.c :cpu hotplug handling

Hi Maninder Singh,

On Wed, May 13, 2015 at 05:24:07AM +0000, Maninder Singh wrote:
> EP-2DAD0AFA905A4ACB804C4F82A001242F
>
> Subject: [PATCH 1/1] hw_breakpoint.c :cpu hotplug handling
>
> This patch adds support for CPU hotplug, It re-installl all installed watchpoints and breakpoints
> back on H/W in case of cpu-hot plug.
>
> Signed-off-by: Vaneet Narang <[email protected]>
> Signed-off-by: Maninder Singh <[email protected]>
> Reviewed-by: Amit Arora <[email protected]>
> Reviewed-by: Ajeet Yadav <[email protected]>
> ---
> arch/arm/kernel/hw_breakpoint.c | 56 +++++++++++++++++++++++++++++++++++++++
> 1 files changed, 56 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
> index dc7d0a9..172bfa8 100644
> --- a/arch/arm/kernel/hw_breakpoint.c
> +++ b/arch/arm/kernel/hw_breakpoint.c
> @@ -388,6 +388,56 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
> return 0;
> }
>
> +/*
> + * reInstall a perf counter breakpoint.
> + */
> +void arch_reinstall_hw_breakpoint(int index, int type)

Static?

baruch

--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- [email protected] - tel: +972.2.679.5364, http://www.tkos.co.il -

2015-05-13 16:08:54

by Will Deacon

[permalink] [raw]
Subject: Re: [EDT][PATCH 1/1] hw_breakpoint.c :cpu hotplug handling

On Wed, May 13, 2015 at 06:24:06AM +0100, Maninder Singh wrote:
> EP-2DAD0AFA905A4ACB804C4F82A001242F
>
> Subject: [PATCH 1/1] hw_breakpoint.c :cpu hotplug handling
>
> This patch adds support for CPU hotplug, It re-installl all installed watchpoints and breakpoints
> back on H/W in case of cpu-hot plug.

Not sure why this is needed -- the scheduler should reinstall the
breakpoints when the debugged task gets scheduled in via
arch_install_hw_breakpoint.

Will

2015-05-18 04:46:54

by Vaneet Narang

[permalink] [raw]
Subject: Re: [EDT][PATCH 1/1] hw_breakpoint.c :cpu hotplug handling

EP-2DAD0AFA905A4ACB804C4F82A001242F

On Wed, May 13, 2015 at 06:24:06AM +0100, Maninder Singh wrote:
>> EP-2DAD0AFA905A4ACB804C4F82A001242F
>>
>> Subject: [PATCH 1/1] hw_breakpoint.c :cpu hotplug handling
>>
>> This patch adds support for CPU hotplug, It re-installl all installed watchpoints and breakpoints
>> back on H/W in case of cpu-hot plug.

>Not sure why this is needed -- the scheduler should reinstall the
>breakpoints when the debugged task gets scheduled in via
>arch_install_hw_breakpoint.
>
>Will

I agree with you this reinstalling has to be either take care by scheduler or Debug tool.
In current implementation we clear H/W registers but we don't clear slots (wp_on_reg / bp_on_reg) for both watchpoint or breakpoint.
So it makes mandatory for debug tool to uninstall breakpoints when CPU goes offline, because if we don't
uninstall which I think is not required and when CPU comes online, we will not be able to reinstall them back because there will be no free slots.
Despite of the fact that H/W registers are free but still we wouldn't be able to install breakpoints.
Logically we should clear these slots or we should reinstall but if you think reinstall has to be taken care by scheduler or debugger
then at least we should clear these slots.

Regards,
Vaneet Narang
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2015-05-20 16:50:30

by Will Deacon

[permalink] [raw]
Subject: Re: [EDT][PATCH 1/1] hw_breakpoint.c :cpu hotplug handling

On Mon, May 18, 2015 at 05:46:31AM +0100, Vaneet Narang wrote:
> EP-2DAD0AFA905A4ACB804C4F82A001242F
>
> On Wed, May 13, 2015 at 06:24:06AM +0100, Maninder Singh wrote:
> >> EP-2DAD0AFA905A4ACB804C4F82A001242F
> >>
> >> Subject: [PATCH 1/1] hw_breakpoint.c :cpu hotplug handling
> >>
> >> This patch adds support for CPU hotplug, It re-installl all installed watchpoints and breakpoints
> >> back on H/W in case of cpu-hot plug.
>
> >Not sure why this is needed -- the scheduler should reinstall the
> >breakpoints when the debugged task gets scheduled in via
> >arch_install_hw_breakpoint.
> >
> >Will
>
> I agree with you this reinstalling has to be either take care by scheduler
> or Debug tool.

Which debug tool? If it's a ptrace user, then the scheduler should still
manage this, as ptrace will call into perf to create the breakpoints.

> In current implementation we clear H/W registers but we don't clear slots
> (wp_on_reg / bp_on_reg) for both watchpoint or breakpoint.

Ok, so are you saying that when the CPU is hotplugged off, the active
breakpoints aren't being uninstalled by perf? I thought this would have
happened as a result of the scheduler migrating the current task away
from the dying CPU.

Will