mbox series

[0/2] target/arm: Always build Aarch64 gdbstub helpers

Message ID 20240619124903.56898-1-philmd@linaro.org
Headers show
Series target/arm: Always build Aarch64 gdbstub helpers | expand

Message

Philippe Mathieu-Daudé June 19, 2024, 12:49 p.m. UTC
Merge gdbstub64.c in gdbstub.c and remove uses of
target specific TARGET_AARCH64 definition.
Small step toward single ARM/Aarch64 binary.

Philippe Mathieu-Daudé (2):
  target/arm: Merge gdbstub64.c within gdbstub.c
  target/arm: Always build Aarch64 gdbstub helpers

 target/arm/cpu.h       |   8 +-
 target/arm/internals.h |   2 -
 target/arm/gdbstub.c   | 363 +++++++++++++++++++++++++++++++++++++-
 target/arm/gdbstub64.c | 383 -----------------------------------------
 target/arm/meson.build |   1 -
 5 files changed, 364 insertions(+), 393 deletions(-)
 delete mode 100644 target/arm/gdbstub64.c

Comments

Philippe Mathieu-Daudé June 28, 2024, 6:19 a.m. UTC | #1
ping?

On 19/6/24 14:49, Philippe Mathieu-Daudé wrote:
> Merge gdbstub64.c in gdbstub.c and remove uses of
> target specific TARGET_AARCH64 definition.
> Small step toward single ARM/Aarch64 binary.
> 
> Philippe Mathieu-Daudé (2):
>    target/arm: Merge gdbstub64.c within gdbstub.c
>    target/arm: Always build Aarch64 gdbstub helpers
> 
>   target/arm/cpu.h       |   8 +-
>   target/arm/internals.h |   2 -
>   target/arm/gdbstub.c   | 363 +++++++++++++++++++++++++++++++++++++-
>   target/arm/gdbstub64.c | 383 -----------------------------------------
>   target/arm/meson.build |   1 -
>   5 files changed, 364 insertions(+), 393 deletions(-)
>   delete mode 100644 target/arm/gdbstub64.c
>
Richard Henderson June 28, 2024, 2:31 p.m. UTC | #2
On 6/19/24 05:49, Philippe Mathieu-Daudé wrote:
> Merge gdbstub64.c in gdbstub.c and remove uses of
> target specific TARGET_AARCH64 definition.
> Small step toward single ARM/Aarch64 binary.
> 
> Philippe Mathieu-Daudé (2):
>    target/arm: Merge gdbstub64.c within gdbstub.c
>    target/arm: Always build Aarch64 gdbstub helpers
> 
>   target/arm/cpu.h       |   8 +-
>   target/arm/internals.h |   2 -
>   target/arm/gdbstub.c   | 363 +++++++++++++++++++++++++++++++++++++-
>   target/arm/gdbstub64.c | 383 -----------------------------------------
>   target/arm/meson.build |   1 -
>   5 files changed, 364 insertions(+), 393 deletions(-)
>   delete mode 100644 target/arm/gdbstub64.c
> 

Are we attempting a single binary for user-only as well?


r~
Philippe Mathieu-Daudé June 28, 2024, 4:37 p.m. UTC | #3
On 28/6/24 16:31, Richard Henderson wrote:
> On 6/19/24 05:49, Philippe Mathieu-Daudé wrote:
>> Merge gdbstub64.c in gdbstub.c and remove uses of
>> target specific TARGET_AARCH64 definition.
>> Small step toward single ARM/Aarch64 binary.
>>
>> Philippe Mathieu-Daudé (2):
>>    target/arm: Merge gdbstub64.c within gdbstub.c
>>    target/arm: Always build Aarch64 gdbstub helpers
>>
>>   target/arm/cpu.h       |   8 +-
>>   target/arm/internals.h |   2 -
>>   target/arm/gdbstub.c   | 363 +++++++++++++++++++++++++++++++++++++-
>>   target/arm/gdbstub64.c | 383 -----------------------------------------
>>   target/arm/meson.build |   1 -
>>   5 files changed, 364 insertions(+), 393 deletions(-)
>>   delete mode 100644 target/arm/gdbstub64.c
>>
> 
> Are we attempting a single binary for user-only as well?

No, due to ABI constraints, right? I did a user-emulation
smoke build, no failure, did I miss something?
Richard Henderson June 28, 2024, 4:50 p.m. UTC | #4
On 6/28/24 09:37, Philippe Mathieu-Daudé wrote:
> On 28/6/24 16:31, Richard Henderson wrote:
>> On 6/19/24 05:49, Philippe Mathieu-Daudé wrote:
>>> Merge gdbstub64.c in gdbstub.c and remove uses of
>>> target specific TARGET_AARCH64 definition.
>>> Small step toward single ARM/Aarch64 binary.
>>>
>>> Philippe Mathieu-Daudé (2):
>>>    target/arm: Merge gdbstub64.c within gdbstub.c
>>>    target/arm: Always build Aarch64 gdbstub helpers
>>>
>>>   target/arm/cpu.h       |   8 +-
>>>   target/arm/internals.h |   2 -
>>>   target/arm/gdbstub.c   | 363 +++++++++++++++++++++++++++++++++++++-
>>>   target/arm/gdbstub64.c | 383 -----------------------------------------
>>>   target/arm/meson.build |   1 -
>>>   5 files changed, 364 insertions(+), 393 deletions(-)
>>>   delete mode 100644 target/arm/gdbstub64.c
>>>
>>
>> Are we attempting a single binary for user-only as well?
> 
> No, due to ABI constraints, right? I did a user-emulation
> smoke build, no failure, did I miss something?

Well, no.  But qemu-arm does not need gdbstub64.c.
Given TARGET_AARCH64 will be set on a combined build, I'm not sure what is the fix?


r~
Peter Maydell July 4, 2024, 3:01 p.m. UTC | #5
On Fri, 28 Jun 2024 at 17:50, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 6/28/24 09:37, Philippe Mathieu-Daudé wrote:
> > On 28/6/24 16:31, Richard Henderson wrote:
> >> On 6/19/24 05:49, Philippe Mathieu-Daudé wrote:
> >>> Merge gdbstub64.c in gdbstub.c and remove uses of
> >>> target specific TARGET_AARCH64 definition.
> >>> Small step toward single ARM/Aarch64 binary.
> >>>
> >>> Philippe Mathieu-Daudé (2):
> >>>    target/arm: Merge gdbstub64.c within gdbstub.c
> >>>    target/arm: Always build Aarch64 gdbstub helpers
> >>>
> >>>   target/arm/cpu.h       |   8 +-
> >>>   target/arm/internals.h |   2 -
> >>>   target/arm/gdbstub.c   | 363 +++++++++++++++++++++++++++++++++++++-
> >>>   target/arm/gdbstub64.c | 383 -----------------------------------------
> >>>   target/arm/meson.build |   1 -
> >>>   5 files changed, 364 insertions(+), 393 deletions(-)
> >>>   delete mode 100644 target/arm/gdbstub64.c
> >>>
> >>
> >> Are we attempting a single binary for user-only as well?
> >
> > No, due to ABI constraints, right? I did a user-emulation
> > smoke build, no failure, did I miss something?
>
> Well, no.  But qemu-arm does not need gdbstub64.c.
> Given TARGET_AARCH64 will be set on a combined build, I'm not sure what is the fix?

Richard: I'm a bit confused about where we are with this
patchset. Do your comments mean:
 * this patchset is OK for system emulation but we
   should (later) think also about user-mode ?
 * this patchset has a problem with user-mode so it
   needs rethinking ?
 * something else ?

thanks
-- PMM
Richard Henderson July 4, 2024, 6:24 p.m. UTC | #6
On 7/4/24 08:01, Peter Maydell wrote:
> On Fri, 28 Jun 2024 at 17:50, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> On 6/28/24 09:37, Philippe Mathieu-Daudé wrote:
>>> On 28/6/24 16:31, Richard Henderson wrote:
>>>> On 6/19/24 05:49, Philippe Mathieu-Daudé wrote:
>>>>> Merge gdbstub64.c in gdbstub.c and remove uses of
>>>>> target specific TARGET_AARCH64 definition.
>>>>> Small step toward single ARM/Aarch64 binary.
>>>>>
>>>>> Philippe Mathieu-Daudé (2):
>>>>>     target/arm: Merge gdbstub64.c within gdbstub.c
>>>>>     target/arm: Always build Aarch64 gdbstub helpers
>>>>>
>>>>>    target/arm/cpu.h       |   8 +-
>>>>>    target/arm/internals.h |   2 -
>>>>>    target/arm/gdbstub.c   | 363 +++++++++++++++++++++++++++++++++++++-
>>>>>    target/arm/gdbstub64.c | 383 -----------------------------------------
>>>>>    target/arm/meson.build |   1 -
>>>>>    5 files changed, 364 insertions(+), 393 deletions(-)
>>>>>    delete mode 100644 target/arm/gdbstub64.c
>>>>>
>>>>
>>>> Are we attempting a single binary for user-only as well?
>>>
>>> No, due to ABI constraints, right? I did a user-emulation
>>> smoke build, no failure, did I miss something?
>>
>> Well, no.  But qemu-arm does not need gdbstub64.c.
>> Given TARGET_AARCH64 will be set on a combined build, I'm not sure what is the fix?
> 
> Richard: I'm a bit confused about where we are with this
> patchset. Do your comments mean:
>   * this patchset is OK for system emulation but we
>     should (later) think also about user-mode ?
>   * this patchset has a problem with user-mode so it
>     needs rethinking ?
>   * something else ?

I'm confused about what this patch set improves.

It doesn't remove ifdefs; they're still there in gdbstub.c.  The code that handles aarch64 
is now in gdbstub instead of segregated into gdbstub64.c, so we have one larger file for 
no obvious benefit.

Was there some other build problem that I missed?  Because I don't see how it advances the 
stated goal of "a single ARM/AArch64 binary".


r~