Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1335225
| From | Marc Zyngier <marc.zyngier@arm.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v4 21/23] arm64: hw_breakpoint: Allow EL2 breakpoints if running in HYP |
| Date | 2016-02-16 11:00 +0100 |
| Message-ID | <r2KAi-685-5@gated-at.bofh.it> (permalink) |
| References | <r14tr-4eC-3@gated-at.bofh.it> <r14tu-4eC-59@gated-at.bofh.it> <r2vrB-4dy-33@gated-at.bofh.it> <r2wH2-5lc-75@gated-at.bofh.it> |
| Organization | ARM Ltd |
On 15/02/16 19:07, Will Deacon wrote:
> On Mon, Feb 15, 2016 at 05:46:56PM +0000, Will Deacon wrote:
>> On Thu, Feb 11, 2016 at 06:40:02PM +0000, Marc Zyngier wrote:
>>> With VHE, we place kernel {watch,break}-points at EL2 to get things
>>> like kgdb and "perf -e mem:..." working.
>>>
>>> This requires a bit of repainting in the low-level encore/decode,
>>> but is otherwise pretty simple.
>>>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>> ---
>>> arch/arm64/include/asm/hw_breakpoint.h | 49 +++++++++++++++++++++-------------
>>> 1 file changed, 31 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
>>> index 9732908..4d8d5a8 100644
>>> --- a/arch/arm64/include/asm/hw_breakpoint.h
>>> +++ b/arch/arm64/include/asm/hw_breakpoint.h
>>> @@ -18,6 +18,7 @@
>>>
>>> #include <asm/cputype.h>
>>> #include <asm/cpufeature.h>
>>> +#include <asm/virt.h>
>>>
>>> #ifdef __KERNEL__
>>>
>>> @@ -35,24 +36,6 @@ struct arch_hw_breakpoint {
>>> struct arch_hw_breakpoint_ctrl ctrl;
>>> };
>>>
>>> -static inline u32 encode_ctrl_reg(struct arch_hw_breakpoint_ctrl ctrl)
>>> -{
>>> - return (ctrl.len << 5) | (ctrl.type << 3) | (ctrl.privilege << 1) |
>>> - ctrl.enabled;
>>> -}
>>> -
>>> -static inline void decode_ctrl_reg(u32 reg,
>>> - struct arch_hw_breakpoint_ctrl *ctrl)
>>> -{
>>> - ctrl->enabled = reg & 0x1;
>>> - reg >>= 1;
>>> - ctrl->privilege = reg & 0x3;
>>> - reg >>= 2;
>>> - ctrl->type = reg & 0x3;
>>> - reg >>= 2;
>>> - ctrl->len = reg & 0xff;
>>> -}
>>> -
>>> /* Breakpoint */
>>> #define ARM_BREAKPOINT_EXECUTE 0
>>>
>>> @@ -62,6 +45,7 @@ static inline void decode_ctrl_reg(u32 reg,
>>> #define AARCH64_ESR_ACCESS_MASK (1 << 6)
>>>
>>> /* Privilege Levels */
>>> +#define AARCH64_BREAKPOINT_EL2 0
>>> #define AARCH64_BREAKPOINT_EL1 1
>>> #define AARCH64_BREAKPOINT_EL0 2
>>>
>>> @@ -76,6 +60,35 @@ static inline void decode_ctrl_reg(u32 reg,
>>> #define ARM_KERNEL_STEP_ACTIVE 1
>>> #define ARM_KERNEL_STEP_SUSPEND 2
>>>
>>> +#define DBG_HMC_HYP (1 << 13)
>>> +#define DBG_SSC_HYP (3 << 14)
>>
>> Why do we need to touch the SSC field at all?
>>
>>> +
>>> +static inline u32 encode_ctrl_reg(struct arch_hw_breakpoint_ctrl ctrl)
>>> +{
>>> + u32 val = (ctrl.len << 5) | (ctrl.type << 3) | ctrl.enabled;
>>> +
>>> + if (is_kernel_in_hyp_mode() && ctrl.privilege == AARCH64_BREAKPOINT_EL1)
>>> + val |= DBG_HMC_HYP | DBG_SSC_HYP | (AARCH64_BREAKPOINT_EL2 << 1);
>>
>> I don't think this is correct. We want to allow, for example, a userspace
>> watchpoint to fire thanks to something like put_user, so the encoding
>> really needs to build up the PMC field (like we do already), then orr in
>> the HMC field.
>
> Hmm, I got my arm and my arm64 mixed up here. For the latter, we don't
> actually support EL0+EL1 watchpoints, but I still think that the
> {HMC,SSC,PMC} encoding of {1,00,xx} is cleaner.
Indeed. So with that in mind, I ended up with the following patch, which is much simpler than the previous one:
diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
index 9732908..115ea2a 100644
--- a/arch/arm64/include/asm/hw_breakpoint.h
+++ b/arch/arm64/include/asm/hw_breakpoint.h
@@ -18,6 +18,7 @@
#include <asm/cputype.h>
#include <asm/cpufeature.h>
+#include <asm/virt.h>
#ifdef __KERNEL__
@@ -35,10 +36,21 @@ struct arch_hw_breakpoint {
struct arch_hw_breakpoint_ctrl ctrl;
};
+/* Privilege Levels */
+#define AARCH64_BREAKPOINT_EL1 1
+#define AARCH64_BREAKPOINT_EL0 2
+
+#define DBG_HMC_HYP (1 << 13)
+
static inline u32 encode_ctrl_reg(struct arch_hw_breakpoint_ctrl ctrl)
{
- return (ctrl.len << 5) | (ctrl.type << 3) | (ctrl.privilege << 1) |
+ u32 val = (ctrl.len << 5) | (ctrl.type << 3) | (ctrl.privilege << 1) |
ctrl.enabled;
+
+ if (is_kernel_in_hyp_mode() && ctrl.privilege == AARCH64_BREAKPOINT_EL1)
+ val |= DBG_HMC_HYP;
+
+ return val;
}
static inline void decode_ctrl_reg(u32 reg,
@@ -61,10 +73,6 @@ static inline void decode_ctrl_reg(u32 reg,
#define ARM_BREAKPOINT_STORE 2
#define AARCH64_ESR_ACCESS_MASK (1 << 6)
-/* Privilege Levels */
-#define AARCH64_BREAKPOINT_EL1 1
-#define AARCH64_BREAKPOINT_EL0 2
-
/* Lengths */
#define ARM_BREAKPOINT_LEN_1 0x1
#define ARM_BREAKPOINT_LEN_2 0x3
Thanks,
M.
--
Jazz is not dead. It just smells funny...
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH v4 00/23] arm64: Virtualization Host Extension support Marc Zyngier <marc.zyngier@arm.com> - 2016-02-11 19:50 +0100
[PATCH v4 21/23] arm64: hw_breakpoint: Allow EL2 breakpoints if running in HYP Marc Zyngier <marc.zyngier@arm.com> - 2016-02-11 19:50 +0100
Re: [PATCH v4 21/23] arm64: hw_breakpoint: Allow EL2 breakpoints if running in HYP Catalin Marinas <catalin.marinas@arm.com> - 2016-02-15 11:30 +0100
Re: [PATCH v4 21/23] arm64: hw_breakpoint: Allow EL2 breakpoints if running in HYP Will Deacon <will.deacon@arm.com> - 2016-02-15 18:50 +0100
Re: [PATCH v4 21/23] arm64: hw_breakpoint: Allow EL2 breakpoints if running in HYP Will Deacon <will.deacon@arm.com> - 2016-02-15 20:10 +0100
Re: [PATCH v4 21/23] arm64: hw_breakpoint: Allow EL2 breakpoints if running in HYP Marc Zyngier <marc.zyngier@arm.com> - 2016-02-16 11:00 +0100
[PATCH v4.1] arm64: perf: Count EL2 events if the kernel is running in HYP Marc Zyngier <marc.zyngier@arm.com> - 2016-02-17 19:00 +0100
Re: [PATCH v4.1] arm64: perf: Count EL2 events if the kernel is running in HYP Will Deacon <will.deacon@arm.com> - 2016-02-19 15:40 +0100
[PATCH v4 19/23] arm64: KVM: Move most of the fault decoding to C Marc Zyngier <marc.zyngier@arm.com> - 2016-02-11 19:50 +0100
[PATCH v4 20/23] arm64: perf: Count EL2 events if the kernel is running in HYP Marc Zyngier <marc.zyngier@arm.com> - 2016-02-11 19:50 +0100
Re: [PATCH v4 20/23] arm64: perf: Count EL2 events if the kernel is running in HYP Will Deacon <will.deacon@arm.com> - 2016-02-15 18:30 +0100
Re: [PATCH v4 20/23] arm64: perf: Count EL2 events if the kernel is running in HYP Will Deacon <will.deacon@arm.com> - 2016-02-15 19:30 +0100
Re: [PATCH v4 20/23] arm64: perf: Count EL2 events if the kernel is running in HYP Marc Zyngier <marc.zyngier@arm.com> - 2016-02-15 19:30 +0100
[PATCH v4 14/23] arm64: KVM: VHE: Make __fpsimd_enabled VHE aware Marc Zyngier <marc.zyngier@arm.com> - 2016-02-11 19:50 +0100
[PATCH v4 13/23] arm64: KVM: VHE: Enable minimal sysreg save/restore Marc Zyngier <marc.zyngier@arm.com> - 2016-02-11 19:50 +0100
Re: [PATCH v4 00/23] arm64: Virtualization Host Extension support Christoffer Dall <christoffer.dall@linaro.org> - 2016-02-11 20:10 +0100
Re: [PATCH v4 00/23] arm64: Virtualization Host Extension support Marc Zyngier <marc.zyngier@arm.com> - 2016-02-11 20:20 +0100
Re: [PATCH v4 00/23] arm64: Virtualization Host Extension support Catalin Marinas <catalin.marinas@arm.com> - 2016-02-15 11:30 +0100
Re: [PATCH v4 00/23] arm64: Virtualization Host Extension support Marc Zyngier <marc.zyngier@arm.com> - 2016-02-15 11:40 +0100
csiph-web