diff mbox series

[1/3] user-mode: Use QEMU_ALIGNED() on TaskState structure

Message ID 20210304152607.1817648-2-f4bug@amsat.org
State New
Headers show
Series user-mode: Rename CPUState::opaque as CPUState::task_state | expand

Commit Message

Philippe Mathieu-Daudé March 4, 2021, 3:26 p.m. UTC
The rest of the codebase uses the QEMU_ALIGNED() introduced in
commit 911a4d2215b ("compiler.h: add QEMU_ALIGNED() to enforce
struct alignment"). Use it for the TaskState structure too.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 bsd-user/qemu.h   | 2 +-
 linux-user/qemu.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Peter Maydell March 4, 2021, 5:08 p.m. UTC | #1
On Thu, 4 Mar 2021 at 15:31, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> The rest of the codebase uses the QEMU_ALIGNED() introduced in
> commit 911a4d2215b ("compiler.h: add QEMU_ALIGNED() to enforce
> struct alignment"). Use it for the TaskState structure too.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  bsd-user/qemu.h   | 2 +-
>  linux-user/qemu.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index d2bcaab7413..1ec14010216 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -96,7 +96,7 @@ typedef struct TaskState {
>      int signal_pending; /* non zero if a signal may be pending */
>
>      uint8_t stack[];
> -} __attribute__((aligned(16))) TaskState;
> +} QEMU_ALIGNED(16) TaskState;
>
>  void init_task_state(TaskState *ts);
>  extern const char *qemu_uname_release;
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index 52c981710b4..d7815bfb845 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -157,7 +157,7 @@ typedef struct TaskState {
>
>      /* This thread's sigaltstack, if it has one */
>      struct target_sigaltstack sigaltstack_used;
> -} __attribute__((aligned(16))) TaskState;
> +} QEMU_ALIGNED(16) TaskState;

Does this struct need to be 16-aligned these days? When it was
first added in commit 851e67a1b46f in 2003, there was
a justification in a comment (still present in the source today):
/* NOTE: we force a big alignment so that the stack stored after is
   aligned too */
because the final field in the struct was "uint8_t stack[0];"
But that field was removed in commit 48e15fc2d in 2010 which
switched us to allocating the stack and the TaskState separately.

So I think that at least for linux-user the alignment attribute
is no longer needed.

bsd-user's struct still has the 'stack' field but as far
as I can tell it is never used, so it and the alignment
could be deleted there too. (bsd-user is missing the changes
that 48e15fc2d makes for linux-user, but less harmfully since
it appears not to implement either threads or fork (?!). In
any case the plan at the moment is to blow away bsd-user/
entirely so spending much thought on it is wasted effort.)

thanks
-- PMM
diff mbox series

Patch

diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index d2bcaab7413..1ec14010216 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -96,7 +96,7 @@  typedef struct TaskState {
     int signal_pending; /* non zero if a signal may be pending */
 
     uint8_t stack[];
-} __attribute__((aligned(16))) TaskState;
+} QEMU_ALIGNED(16) TaskState;
 
 void init_task_state(TaskState *ts);
 extern const char *qemu_uname_release;
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 52c981710b4..d7815bfb845 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -157,7 +157,7 @@  typedef struct TaskState {
 
     /* This thread's sigaltstack, if it has one */
     struct target_sigaltstack sigaltstack_used;
-} __attribute__((aligned(16))) TaskState;
+} QEMU_ALIGNED(16) TaskState;
 
 extern char *exec_path;
 void init_task_state(TaskState *ts);