Re: [PATCH 1/2] LoongArch: Add pad structure members for explicit alignment

From: Xi Ruoyao
Date: Wed Apr 19 2023 - 06:42:49 EST


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 {
>  };
>  
>  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?

>         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++;
>         }
>  

--
Xi Ruoyao <xry111@xxxxxxxxxxx>
School of Aerospace Science and Technology, Xidian University