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

[PATCH v2] 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>
---
v2: Drop ifdef __powerpc__

 linux-user/include/host/ppc/host-signal.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/linux-user/include/host/ppc/host-signal.h b/linux-user/include/host/ppc/host-signal.h
index b80384d135..ec6166ed66 100644
--- a/linux-user/include/host/ppc/host-signal.h
+++ b/linux-user/include/host/ppc/host-signal.h
@@ -11,6 +11,9 @@
 #ifndef PPC_HOST_SIGNAL_H
 #define PPC_HOST_SIGNAL_H
 
+/* needed for pt_regs */
+#include <asm/ptrace.h>
+
 /* The third argument to a SA_SIGINFO handler is ucontext_t. */
 typedef ucontext_t host_sigcontext;
 
-- 
2.35.1

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

Posted by Richard Henderson 2 years, 1 month ago

On 3/14/22 10:25, Khem Raj 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>
> ---
> v2: Drop ifdef __powerpc__
> 
>   linux-user/include/host/ppc/host-signal.h | 3 +++
>   1 file changed, 3 insertions(+)

As per late conversation vs version 1, instead of the include, we should update the code 
to avoid the pt_regs indirection and instead reference gp_regs directly.

We should also move the file to ../ppc64/, leaving ppc/ empty.


r~

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

Posted by Daniel Henrique Barboza 2 years, 1 month ago


On 3/14/22 14:25, Khem Raj 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>
> ---
> v2: Drop ifdef __powerpc__
> 
>   linux-user/include/host/ppc/host-signal.h | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/linux-user/include/host/ppc/host-signal.h b/linux-user/include/host/ppc/host-signal.h
> index b80384d135..ec6166ed66 100644
> --- a/linux-user/include/host/ppc/host-signal.h
> +++ b/linux-user/include/host/ppc/host-signal.h
> @@ -11,6 +11,9 @@
>   #ifndef PPC_HOST_SIGNAL_H
>   #define PPC_HOST_SIGNAL_H
>   
> +/* needed for pt_regs */
> +#include <asm/ptrace.h>
> +

I am intrigued about why we didn't hit this before, especially considering that ppc64 header is just a
pointer to this file.

I looked it up and didn't find why. This change seems harmless to me though.


Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>




>   /* The third argument to a SA_SIGINFO handler is ucontext_t. */
>   typedef ucontext_t host_sigcontext;
>

Re: [PATCH v2] 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:59, Daniel Henrique Barboza
<danielhb413@gmail.com> wrote:
> I am intrigued about why we didn't hit this before, especially considering that ppc64 header is just a
> pointer to this file.

It's specific to musl, which does different things with its
system includes than glibc does.

-- PMM

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

Posted by Daniel P. Berrangé 2 years, 1 month ago

On Mon, Mar 14, 2022 at 06:06:40PM +0000, Peter Maydell wrote:
> On Mon, 14 Mar 2022 at 17:59, Daniel Henrique Barboza
> <danielhb413@gmail.com> wrote:
> > I am intrigued about why we didn't hit this before, especially considering that ppc64 header is just a
> > pointer to this file.
> 
> It's specific to musl, which does different things with its
> system includes than glibc does.

And while we have CI testing for QEMU with Alpine that uses musl, this
is only x86_64 host. IOW we don't have CI coverage of ppc64 host
with musl, only glibc

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|