Re: [Qemu-devel] [PATCH] softfloat: don't execute ppc64 ISA 3.0B instruction if it is not supported

[Laurent Vivier <laurent@vivier.eu>]

[-- Attachment #1: Type: text/plain, Size: 2196 bytes --]

On 03/11/2018 13:57, David Gibson wrote:
> On Thu, Nov 01, 2018 at 06:54:59PM +0100, Laurent Vivier wrote:
>> On 01/11/2018 18:49, Peter Maydell wrote:
>>> On 1 November 2018 at 17:38, Laurent Vivier <laurent@vivier.eu> wrote:
>>>> commit 27ae5109a2 has introduced an assembly instruction only supported
>>>> by ISA 3.0B and it fails to execute on previous versions of the POWER
>>>> CPU (like PowerPC G5).
>>>>
>>>> This patch fixes that by checking the ISA level, and falls back to
>>>> the default C function if the instruction is not supported.
>>>>
>>>> Fixes: 27ae5109a2ba8b6b679cce3e03e16570a34390a0
>>>>        (softfloat: Specialize udiv_qrnnd for ppc64)
>>>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>>>> ---
>>>>  include/fpu/softfloat-macros.h | 39 ++++++++++++++++++++--------------
>>>>  1 file changed, 23 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/include/fpu/softfloat-macros.h b/include/fpu/softfloat-macros.h
>>>> index c86687fa5e..fe98b33df9 100644
>>>> --- a/include/fpu/softfloat-macros.h
>>>> +++ b/include/fpu/softfloat-macros.h
>>>> @@ -78,6 +78,9 @@ this code that are retained.
>>>>  /* Portions of this work are licensed under the terms of the GNU GPL,
>>>>   * version 2 or later. See the COPYING file in the top-level directory.
>>>>   */
>>>> +#if defined(_ARCH_PPC64)
>>>> +extern bool have_isa_3_00;
>>>> +#endif
>>>
>>> I was wondering where this bool came from. The answer is
>>> that it's defined in tcg/ppc/tcg-target.inc.c. It's ok to
>>> use it here because fpu/softfloat.c is only compiled if
>>> CONFIG_TCG is true, so the tcg code will be present. The
>>> other user of this include file is target/m68k/softfloat.c,
>>> which also will only be compiled for a ppc host if CONFIG_TCG.
>>>
>>> It's a little awkward to be borrowing this tcg/ppc internal
>>> flag into the softfloat code, though.
>>
>> I agree, I was hopping Richard can advice another way to do that.
> 
> We already have ISA version flags in insns_flags & insns_flags2, so
> I'm hoping we can use those rather than introduce a new bool.
> 

Richard has sent another patch using "defined(_ARCH_PWR7)"

Thanks,
Laurent


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]