Re: [PATCH 1/2] LoongArch: Add pad structure members for explicit alignment
From: WANG Xuerui
Date: Wed Apr 19 2023 - 13:24:34 EST
On 4/19/23 19:00, WANG Xuerui wrote:
On 2023/4/19 18:42, Xi Ruoyao wrote:
On Tue, 2023-04-18 at 17:13 +0800, Qing Zhang wrote:
This is done in order to easily calculate the number of breakpoints
in hw_break_get.
Signed-off-by: Qing Zhang <zhangqing@xxxxxxxxxxx>
---
arch/loongarch/include/uapi/asm/ptrace.h | 3 ++-
arch/loongarch/kernel/ptrace.c | 13 +++++++++----
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/arch/loongarch/include/uapi/asm/ptrace.h
b/arch/loongarch/include/uapi/asm/ptrace.h
index 2282ae1fd3b6..06e3be52cb04 100644
--- a/arch/loongarch/include/uapi/asm/ptrace.h
+++ b/arch/loongarch/include/uapi/asm/ptrace.h
@@ -57,11 +57,12 @@ struct user_lasx_state {
Drive-by comment to the patch author: there is no "user_lasx_state"
yet. Please always state your base commit if not obvious, or you
should start from some well-known upstream HEAD (e.g. Linus' rc tags,
loongarch-fixes, or loongarch-next).
};
struct user_watch_state {
- uint16_t dbg_info;
+ uint64_t dbg_info;
Ouch. This is a breaking change when we consider user code like
`printf(PRIu16 "\n", ptr->dbg_info);`. Is it really necessary?
Ah right. This is UAPI so without *very* concrete and convicing reason
why the change is not going to impact any potential users, it's gonna
be a presumed NAK. In other words you must demonstrate (1) why it's
absolutely necessary to make the change and (2) that it's impossible
to impact anyone, before any such changes can even be considered.
Please ignore all of this. The memory layout is actually the same after
the change due to the padding, I was somehow thinking in big-endian a
few hours ago. (The commit message didn't help either, I think both
Ruoyao and me got into the habitual thinking that changes like this are
most likely just churn without real benefits, after *not* seeing the
rationale in the commit message which was kinda expected.)
struct {
uint64_t addr;
uint64_t mask;
uint32_t ctrl;
+ uint32_t pad;
} dbg_regs[8];
};
diff --git a/arch/loongarch/kernel/ptrace.c
b/arch/loongarch/kernel/ptrace.c
index 0c7c41e41cad..9c3bc1bbf2ff 100644
--- a/arch/loongarch/kernel/ptrace.c
+++ b/arch/loongarch/kernel/ptrace.c
@@ -475,10 +475,10 @@ static int ptrace_hbp_fill_attr_ctrl(unsigned
int note_type,
return 0;
}
-static int ptrace_hbp_get_resource_info(unsigned int note_type, u16
*info)
+static int ptrace_hbp_get_resource_info(unsigned int note_type, u64
*info)
{
u8 num;
- u16 reg = 0;
+ u64 reg = 0;
switch (note_type) {
case NT_LOONGARCH_HW_BREAK:
@@ -616,7 +616,7 @@ static int hw_break_get(struct task_struct
*target,
const struct user_regset *regset,
struct membuf to)
{
- u16 info;
+ u64 info;
u32 ctrl;
u64 addr, mask;
int ret, idx = 0;
@@ -646,6 +646,7 @@ static int hw_break_get(struct task_struct
*target,
membuf_store(&to, addr);
membuf_store(&to, mask);
membuf_store(&to, ctrl);
+ membuf_zero(&to, sizeof(u32));
idx++;
}
@@ -662,7 +663,7 @@ static int hw_break_set(struct task_struct
*target,
int ret, idx = 0, offset, limit;
unsigned int note_type = regset->core_note_type;
- /* Resource info */
+ /* Resource info and pad */
offset = offsetof(struct user_watch_state, dbg_regs);
user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf, 0,
offset);
@@ -704,6 +705,10 @@ static int hw_break_set(struct task_struct
*target,
if (ret)
return ret;
offset += PTRACE_HBP_CTRL_SZ;
+
+ user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf,
+ offset, offset +
PTRACE_HBP_PAD_SZ);
+ offset += PTRACE_HBP_PAD_SZ;
idx++;
}
--
WANG "xen0n" Xuerui
Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/