Re: [PATCH v2 2/3] target/arm/tcg/m_helper.c: Include the full helpers only with CONFIG_ARM_V7M

[Peter Maydell]

On Thu, 1 Feb 2024 at 19:12, Thomas Huth <thuth@redhat.com> wrote:
>
> On 01/02/2024 15.19, Peter Maydell wrote:
> > On Mon, 29 Jan 2024 at 08:18, Thomas Huth <thuth@redhat.com> wrote:
> >>
> >> If CONFIG_ARM_V7M is not set, we don't want to include the full-fledged
> >> helper functions that require additional functions for linking. The
> >> reduced set of the linux-user functions works fine as stubs in this
> >> case, so change the #ifdef statement accordingly.
> >>
> >> Signed-off-by: Thomas Huth <thuth@redhat.com>
> >> ---
> >>   target/arm/tcg/m_helper.c | 3 ++-
> >>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/target/arm/tcg/m_helper.c b/target/arm/tcg/m_helper.c
> >> index d1f1e02acc..a5a6e96fc3 100644
> >> --- a/target/arm/tcg/m_helper.c
> >> +++ b/target/arm/tcg/m_helper.c
> >> @@ -22,6 +22,7 @@
> >>   #endif
> >>   #if !defined(CONFIG_USER_ONLY)
> >>   #include "hw/intc/armv7m_nvic.h"
> >> +#include CONFIG_DEVICES
> >>   #endif
> >>
> >>   static void v7m_msr_xpsr(CPUARMState *env, uint32_t mask,
> >> @@ -69,7 +70,7 @@ uint32_t arm_v7m_mrs_control(CPUARMState *env, uint32_t 
> >> secure)
> >>       return value;
> >>   }
> >>
> >> -#ifdef CONFIG_USER_ONLY
> >> +#if defined(CONFIG_USER_ONLY) || !defined(CONFIG_ARM_V7M)
> >
> > This looks a bit odd. If we don't have CONFIG_ARM_V7M
> > why are we compiling this file at all?
>
> We'll get failures during linking otherwise. target/arm/helper.h still
> defines code that requires the v7m_* helper functions:
>
> /usr/bin/ld: libqemu-arm-softmmu.fa.p/target_arm_tcg_translate.c.o:
> qemu/target/arm/helper.h:76: undefined reference to `helper_v7m_vlldm'
> /usr/bin/ld: libqemu-arm-softmmu.fa.p/target_arm_tcg_translate.c.o:
> qemu/target/arm/helper.h:75: undefined reference to `helper_v7m_vlstm'
> /usr/bin/ld: libqemu-arm-softmmu.fa.p/target_arm_tcg_translate.c.o:
> qemu/target/arm/helper.h:73: undefined reference to
> `helper_v7m_preserve_fp_state'
>
> etc.

OK, but what we want in that case is either (a) avoid referring
to the functions if we're not building for V7M (as you say,
may be awkward) or else (b) stub versions of the functions that
abort() if called. We don't really want the linux-user versions
of the functions.

Does having an m-helper_stub.c which we build if not CONFIG_ARM_V7M
and having m_helper.c only built with CONFIG_ARM_V7M look
feasible?

thanks
-- PMM