diff mbox series

[13/14] bsd-user: Make compile for non-linux user-mode stuff

Message ID 20240722214313.89503-14-imp@bsdimp.com (mailing list archive)
State New, archived
Headers show
Series bsd-user: Misc changes for 9.1 (I hope) | expand

Commit Message

Warner Losh July 22, 2024, 9:43 p.m. UTC
We include the files that define PR_MTE_TCF_SHIFT only on Linux, but use
them unconditionally. Restrict its use to Linux-only.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 target/arm/gdbstub64.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Philippe Mathieu-Daudé July 22, 2024, 10:01 p.m. UTC | #1
Hi Warner,

On 22/7/24 23:43, Warner Losh wrote:
> We include the files that define PR_MTE_TCF_SHIFT only on Linux, but use
> them unconditionally. Restrict its use to Linux-only.

We should check that in meson, i.e.:

config_host_data.set('CONFIG_PRCTL_PR_MTE_TCF_SHIFT',
                      cc.has_header_symbol('sys/prctl.h', 
'PR_MTE_TCF_SHIFT'))

(like we do for CONFIG_PRCTL_PR_SET_TIMERSLACK), then rework
linux-user/aarch64/mte_user_helper.h (and possibly
tests/tcg/aarch64/mte.h), moving in a common directory.

That said, your patch matches this file header:

   #if defined(CONFIG_USER_ONLY) && defined(CONFIG_LINUX)
   #include <sys/prctl.h>
   #include "mte_user_helper.h"
   #endif

Even if this #ifdef'ry is ugly, it isn't wrong, so:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

¯\_(ツ)_/¯

> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>   target/arm/gdbstub64.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
> index 2e2bc2700b8..6dc81aecb2a 100644
> --- a/target/arm/gdbstub64.c
> +++ b/target/arm/gdbstub64.c
> @@ -404,6 +404,7 @@ int aarch64_gdb_get_tag_ctl_reg(CPUState *cs, GByteArray *buf, int reg)
>   
>   int aarch64_gdb_set_tag_ctl_reg(CPUState *cs, uint8_t *buf, int reg)
>   {
> +#if defined(CONFIG_LINUX)
>       ARMCPU *cpu = ARM_CPU(cs);
>       CPUARMState *env = &cpu->env;
>   
> @@ -425,6 +426,9 @@ int aarch64_gdb_set_tag_ctl_reg(CPUState *cs, uint8_t *buf, int reg)
>       arm_set_mte_tcf0(env, tcf);
>   
>       return 1;
> +#else
> +    return 0;
> +#endif
>   }
>   
>   static void handle_q_memtag(GArray *params, void *user_ctx)
Alex Bennée July 22, 2024, 11:09 p.m. UTC | #2
Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Hi Warner,
>
> On 22/7/24 23:43, Warner Losh wrote:
>> We include the files that define PR_MTE_TCF_SHIFT only on Linux, but use
>> them unconditionally. Restrict its use to Linux-only.
>
> We should check that in meson, i.e.:
>
> config_host_data.set('CONFIG_PRCTL_PR_MTE_TCF_SHIFT',
>                      cc.has_header_symbol('sys/prctl.h',
>                      'PR_MTE_TCF_SHIFT'))

Surely this fails for non-aarch64 hosts?

> (like we do for CONFIG_PRCTL_PR_SET_TIMERSLACK), then rework
> linux-user/aarch64/mte_user_helper.h (and possibly
> tests/tcg/aarch64/mte.h), moving in a common directory.
>
> That said, your patch matches this file header:
>
>   #if defined(CONFIG_USER_ONLY) && defined(CONFIG_LINUX)
>   #include <sys/prctl.h>
>   #include "mte_user_helper.h"
>   #endif
>
> Even if this #ifdef'ry is ugly, it isn't wrong, so:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> ¯\_(ツ)_/¯
>
>> Signed-off-by: Warner Losh <imp@bsdimp.com>
>> ---
>>   target/arm/gdbstub64.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>> diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
>> index 2e2bc2700b8..6dc81aecb2a 100644
>> --- a/target/arm/gdbstub64.c
>> +++ b/target/arm/gdbstub64.c
>> @@ -404,6 +404,7 @@ int aarch64_gdb_get_tag_ctl_reg(CPUState *cs, GByteArray *buf, int reg)
>>     int aarch64_gdb_set_tag_ctl_reg(CPUState *cs, uint8_t *buf, int
>> reg)
>>   {
>> +#if defined(CONFIG_LINUX)
>>       ARMCPU *cpu = ARM_CPU(cs);
>>       CPUARMState *env = &cpu->env;
>>   @@ -425,6 +426,9 @@ int aarch64_gdb_set_tag_ctl_reg(CPUState *cs,
>> uint8_t *buf, int reg)
>>       arm_set_mte_tcf0(env, tcf);
>>         return 1;
>> +#else
>> +    return 0;
>> +#endif
>>   }
>>     static void handle_q_memtag(GArray *params, void *user_ctx)
Richard Henderson July 23, 2024, 1:15 a.m. UTC | #3
On 7/23/24 07:43, Warner Losh wrote:
> We include the files that define PR_MTE_TCF_SHIFT only on Linux, but use
> them unconditionally. Restrict its use to Linux-only.
> 
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   target/arm/gdbstub64.c | 4 ++++
>   1 file changed, 4 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
Warner Losh July 23, 2024, 5:12 a.m. UTC | #4
On Mon, Jul 22, 2024 at 4:01 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> Hi Warner,
>
> On 22/7/24 23:43, Warner Losh wrote:
> > We include the files that define PR_MTE_TCF_SHIFT only on Linux, but use
> > them unconditionally. Restrict its use to Linux-only.
>
> We should check that in meson, i.e.:
>
> config_host_data.set('CONFIG_PRCTL_PR_MTE_TCF_SHIFT',
>                       cc.has_header_symbol('sys/prctl.h',
> 'PR_MTE_TCF_SHIFT'))
>
> (like we do for CONFIG_PRCTL_PR_SET_TIMERSLACK), then rework
> linux-user/aarch64/mte_user_helper.h (and possibly
> tests/tcg/aarch64/mte.h), moving in a common directory.
>

Hmmm... I'll have to check, but since I didn't add the incompatibility, it
will likely be on my backlog for a while...


> That said, your patch matches this file header:
>
>    #if defined(CONFIG_USER_ONLY) && defined(CONFIG_LINUX)
>    #include <sys/prctl.h>
>    #include "mte_user_helper.h"
>    #endif
>
> Even if this #ifdef'ry is ugly, it isn't wrong, so:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> ¯\_(ツ)_/¯
>

Thanks!

Warner


> > Signed-off-by: Warner Losh <imp@bsdimp.com>
> > ---
> >   target/arm/gdbstub64.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
> > index 2e2bc2700b8..6dc81aecb2a 100644
> > --- a/target/arm/gdbstub64.c
> > +++ b/target/arm/gdbstub64.c
> > @@ -404,6 +404,7 @@ int aarch64_gdb_get_tag_ctl_reg(CPUState *cs,
> GByteArray *buf, int reg)
> >
> >   int aarch64_gdb_set_tag_ctl_reg(CPUState *cs, uint8_t *buf, int reg)
> >   {
> > +#if defined(CONFIG_LINUX)
> >       ARMCPU *cpu = ARM_CPU(cs);
> >       CPUARMState *env = &cpu->env;
> >
> > @@ -425,6 +426,9 @@ int aarch64_gdb_set_tag_ctl_reg(CPUState *cs,
> uint8_t *buf, int reg)
> >       arm_set_mte_tcf0(env, tcf);
> >
> >       return 1;
> > +#else
> > +    return 0;
> > +#endif
> >   }
> >
> >   static void handle_q_memtag(GArray *params, void *user_ctx)
>
>
diff mbox series

Patch

diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 2e2bc2700b8..6dc81aecb2a 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -404,6 +404,7 @@  int aarch64_gdb_get_tag_ctl_reg(CPUState *cs, GByteArray *buf, int reg)
 
 int aarch64_gdb_set_tag_ctl_reg(CPUState *cs, uint8_t *buf, int reg)
 {
+#if defined(CONFIG_LINUX)
     ARMCPU *cpu = ARM_CPU(cs);
     CPUARMState *env = &cpu->env;
 
@@ -425,6 +426,9 @@  int aarch64_gdb_set_tag_ctl_reg(CPUState *cs, uint8_t *buf, int reg)
     arm_set_mte_tcf0(env, tcf);
 
     return 1;
+#else
+    return 0;
+#endif
 }
 
 static void handle_q_memtag(GArray *params, void *user_ctx)