[PATCH] ppc: Include asm/ptrace.h for pt_regs struct definition

Khem Raj posted 1 patch 2 years, 1 month ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220314170223.554679-1-raj.khem@gmail.com
Maintainers: Laurent Vivier <laurent@vivier.eu>
There is a newer version of this series
linux-user/include/host/ppc/host-signal.h | 4 ++++
1 file changed, 4 insertions(+)
[PATCH] ppc: Include asm/ptrace.h for pt_regs struct definition
Posted by Khem Raj 2 years, 1 month ago
Fixes
../qemu-6.2.0/linux-user/host/ppc64/../ppc/host-signal.h:16:32: error: incomplete definition of type 'struct pt_regs'
    return uc->uc_mcontext.regs->nip;
           ~~~~~~~~~~~~~~~~~~~~^

Signed-off-by: Khem Raj <raj.khem@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
Cc: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/include/host/ppc/host-signal.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/linux-user/include/host/ppc/host-signal.h b/linux-user/include/host/ppc/host-signal.h
index b80384d135..43ee2eee3b 100644
--- a/linux-user/include/host/ppc/host-signal.h
+++ b/linux-user/include/host/ppc/host-signal.h
@@ -11,6 +11,10 @@
 #ifndef PPC_HOST_SIGNAL_H
 #define PPC_HOST_SIGNAL_H
 
+#if defined(__powerpc__)
+#include <asm/ptrace.h>
+#endif
+
 /* The third argument to a SA_SIGINFO handler is ucontext_t. */
 typedef ucontext_t host_sigcontext;
 
-- 
2.35.1


Re: [PATCH] ppc: Include asm/ptrace.h for pt_regs struct definition
Posted by Peter Maydell 2 years, 1 month ago
On Mon, 14 Mar 2022 at 17:02, Khem Raj <raj.khem@gmail.com> wrote:
>
> Fixes
> ../qemu-6.2.0/linux-user/host/ppc64/../ppc/host-signal.h:16:32: error: incomplete definition of type 'struct pt_regs'
>     return uc->uc_mcontext.regs->nip;
>            ~~~~~~~~~~~~~~~~~~~~^
>
> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/include/host/ppc/host-signal.h | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/linux-user/include/host/ppc/host-signal.h b/linux-user/include/host/ppc/host-signal.h
> index b80384d135..43ee2eee3b 100644
> --- a/linux-user/include/host/ppc/host-signal.h
> +++ b/linux-user/include/host/ppc/host-signal.h
> @@ -11,6 +11,10 @@
>  #ifndef PPC_HOST_SIGNAL_H
>  #define PPC_HOST_SIGNAL_H
>
> +#if defined(__powerpc__)

Why is this #if defined() needed ?

> +#include <asm/ptrace.h>
> +#endif

What host distro are you building with? We do test ppc64be at least
in our CI, and that builds OK.

thanks
-- PMM
Re: [PATCH] ppc: Include asm/ptrace.h for pt_regs struct definition
Posted by Khem Raj 2 years, 1 month ago
On Mon, Mar 14, 2022 at 10:05 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 14 Mar 2022 at 17:02, Khem Raj <raj.khem@gmail.com> wrote:
> >
> > Fixes
> > ../qemu-6.2.0/linux-user/host/ppc64/../ppc/host-signal.h:16:32: error: incomplete definition of type 'struct pt_regs'
> >     return uc->uc_mcontext.regs->nip;
> >            ~~~~~~~~~~~~~~~~~~~~^
> >
> > Signed-off-by: Khem Raj <raj.khem@gmail.com>
> > Cc: Peter Maydell <peter.maydell@linaro.org>
> > Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > Cc: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >  linux-user/include/host/ppc/host-signal.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/linux-user/include/host/ppc/host-signal.h b/linux-user/include/host/ppc/host-signal.h
> > index b80384d135..43ee2eee3b 100644
> > --- a/linux-user/include/host/ppc/host-signal.h
> > +++ b/linux-user/include/host/ppc/host-signal.h
> > @@ -11,6 +11,10 @@
> >  #ifndef PPC_HOST_SIGNAL_H
> >  #define PPC_HOST_SIGNAL_H
> >
> > +#if defined(__powerpc__)
>
> Why is this #if defined() needed ?

yeah the define is redundant. I will send a v2

>
> > +#include <asm/ptrace.h>
> > +#endif
>
> What host distro are you building with? We do test ppc64be at least
> in our CI, and that builds OK.

I have a custom distro building with Yocto which is using musl C
library instead of glibc.
may be adelie linux or alpine will exhibit same issue

>
> thanks
> -- PMM
Re: [PATCH] ppc: Include asm/ptrace.h for pt_regs struct definition
Posted by Peter Maydell 2 years, 1 month ago
On Mon, 14 Mar 2022 at 17:22, Khem Raj <raj.khem@gmail.com> wrote:
>
> On Mon, Mar 14, 2022 at 10:05 AM Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Mon, 14 Mar 2022 at 17:02, Khem Raj <raj.khem@gmail.com> wrote:
> > >
> > > Fixes
> > > ../qemu-6.2.0/linux-user/host/ppc64/../ppc/host-signal.h:16:32: error: incomplete definition of type 'struct pt_regs'
> > >     return uc->uc_mcontext.regs->nip;
> > >            ~~~~~~~~~~~~~~~~~~~~^
> > >
> > > Signed-off-by: Khem Raj <raj.khem@gmail.com>
> > > Cc: Peter Maydell <peter.maydell@linaro.org>
> > > Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > > Cc: Richard Henderson <richard.henderson@linaro.org>
> > > ---
> > >  linux-user/include/host/ppc/host-signal.h | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/linux-user/include/host/ppc/host-signal.h b/linux-user/include/host/ppc/host-signal.h
> > > index b80384d135..43ee2eee3b 100644
> > > --- a/linux-user/include/host/ppc/host-signal.h
> > > +++ b/linux-user/include/host/ppc/host-signal.h
> > > @@ -11,6 +11,10 @@
> > >  #ifndef PPC_HOST_SIGNAL_H
> > >  #define PPC_HOST_SIGNAL_H
> > >
> > > +#if defined(__powerpc__)
> >
> > Why is this #if defined() needed ?
>
> yeah the define is redundant. I will send a v2
>
> >
> > > +#include <asm/ptrace.h>
> > > +#endif
> >
> > What host distro are you building with? We do test ppc64be at least
> > in our CI, and that builds OK.
>
> I have a custom distro building with Yocto which is using musl C
> library instead of glibc.

Ah, musl. I found this gcc mailing list post from one of the musl
upstream developers:
https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg281012.html

where he suggests a different approach involving uc_regs->gregs[32]
for ppc32 and uc_mcontext.gp_regs[32] for ppc64.

Richard: any opinion on that idea ?

-- PMM
Re: [PATCH] ppc: Include asm/ptrace.h for pt_regs struct definition
Posted by Richard Henderson 2 years, 1 month ago
On 3/14/22 11:06, Peter Maydell wrote:
>> I have a custom distro building with Yocto which is using musl C
>> library instead of glibc.
> 
> Ah, musl. I found this gcc mailing list post from one of the musl
> upstream developers:
> https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg281012.html
> 
> where he suggests a different approach involving uc_regs->gregs[32]
> for ppc32 and uc_mcontext.gp_regs[32] for ppc64.
> 
> Richard: any opinion on that idea ?
Reasonable.  I would also remove the indirection between 
linux-user/include/host/ppc64/host-signal.h to linux-user/include/host/ppc/host-signal.h.

We don't support ppc32 anymore -- there are afaik no current distros supporting ppc32 with 
which we could even test.


r~