Re: [Qemu-devel] [PATCH] arm-linux-user: fix elfload.c's AT_HWCAP reflected cpu features.

[Andreas Färber]

Am 09.11.2011 14:04, schrieb Benoît Canet:
> The cpu capabilities passed by the elf loader in AT_HWCAP where
> a constant.
> Make AT_HWCAP reflect the emulated cpu features in order to give
> correct clues to eglibc.
> 
> Fix :  [Bug 887516] [NEW] VFP support reported for the PXA270
> 
> Signed-off-by: Benoit Canet <address@hidden>
> ---
>  linux-user/elfload.c |   31 +++++++++++++++++++++++++++----
>  1 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index a413976..5d81ec1 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -375,10 +375,33 @@ bool guest_validate_base(unsigned long guest_base)
>      return 1; /* All good */
>  }
>  
> -#define ELF_HWCAP (ARM_HWCAP_ARM_SWP | ARM_HWCAP_ARM_HALF               \
> -                   | ARM_HWCAP_ARM_THUMB | ARM_HWCAP_ARM_FAST_MULT      \
> -                   | ARM_HWCAP_ARM_FPA | ARM_HWCAP_ARM_VFP              \
> -                   | ARM_HWCAP_ARM_NEON | ARM_HWCAP_ARM_VFPv3 )
> +
> +#define ELF_HWCAP get_elf_hwcap()

I assume ELF_HWCAP is being used in architecture-independent code? Or
would it be feasible to replace all occurrences with the function call?

> +
> +static uint32_t get_elf_hwcap(void)
> +{
> +    CPUState *e = thread_env;
> +    uint32_t hwcaps = 0;
> +
> +    hwcaps |= ARM_HWCAP_ARM_SWP;
> +    hwcaps |= ARM_HWCAP_ARM_HALF;
> +    hwcaps |= ARM_HWCAP_ARM_THUMB;
> +    hwcaps |= ARM_HWCAP_ARM_FAST_MULT;
> +    hwcaps |= ARM_HWCAP_ARM_FPA;
> +
> +    /* prove for the extra features */

probe?

> +#define GET_FEATURE(feat, hwcap) \
> +    do {if (arm_feature(e, feat)) { hwcaps |= hwcap; } } while (0)

This doesn't return anything, it sets the hwcap flag. SET_HWCAP maybe?

> +    GET_FEATURE(ARM_FEATURE_VFP, ARM_HWCAP_ARM_VFP);
> +    GET_FEATURE(ARM_FEATURE_IWMMXT, ARM_HWCAP_ARM_IWMMXT);
> +    GET_FEATURE(ARM_FEATURE_THUMB2EE, ARM_HWCAP_ARM_THUMBEE);
> +    GET_FEATURE(ARM_FEATURE_NEON, ARM_HWCAP_ARM_NEON);
> +    GET_FEATURE(ARM_FEATURE_VFP3, ARM_HWCAP_ARM_VFPv3);
> +    GET_FEATURE(ARM_FEATURE_VFP_FP16, ARM_HWCAP_ARM_VFPv3D16);

I'm wondering if this translation table were better placed in
target-arm/helper.c where we fiddle with the features in the first
place. We could loop through all features here and call a function that
returns the hwcap or 0 and |= it. Me and others will be adding new
features and we'll risk adapting this here.

I was told that VFP_FP16 is *not* VFPv3-D16. Please remove that for now.

> +#undef GET_FEATURE
> +
> +    return hwcaps;
> +}
>  
>  #endif
>  

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg