Re: [PATCH 3/4] powerpc/powernv: Enable TM without suspend if possible

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 19/10/2017 11:34, Tulio Magno Quites Machado Filho wrote:
> Forwarding some comments from Adhemerval sent to libc-alpha [1]...
> 
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>> Florian Weimer <fweimer@redhat.com> writes:
>>
>>> On 10/12/2017 12:17 PM, Michael Ellerman wrote:
>>>> +	pr_info("Enabling TM (Transactional Memory) with Suspend Disabled\n");
>>>> +	cur_cpu_spec->cpu_features |= CPU_FTR_TM;
>>>> +	cur_cpu_spec->cpu_user_features2 |= PPC_FEATURE2_HTM_NO_SUSPEND;
>>>> +	tm_suspend_disabled = true;
>>>
>>> This doesn't look right because if suspend is not available, you need to 
>>> clear the original PPC_FEATURE2_HTM flag because the semantics are not 
>>> right, so that applications can use fallback code.  Otherwise, 
>>> applications may incorrectly select the HTM code and break if running on 
>>> a system which supports HTM, but without the suspend state.
>>>
>>> The new flag should say that HTM is supported, but without the suspend 
>>> state, and it should be always set if PPC_FEATURE2_HTM is set.
>>
>> Will it also change TEXARS with the abort information?
> 
> It should, with a permanent error cause so that old applications entering
> suspended state can adopt another technique.
> Michael, could you clarify if this is indeed happening, please?
> 
>> I completely agree with Florian here, this is as *ABI* change
>> and the kernel need to advertise a different TM ABI instead
>> of as an extension.
> 
> Adhemerval, could you elaborate which problems you're foreseeing, please?
> 

Pretty much the same Florian already stated: an application can not any
more assume for instance:

        tsr. 	0
        mfcr	r9,128
        andis. 	r10,r9,0x4000
        be 	cr0,L(suspend)
	andis.	r10,r9,0x2000
	be	cr0,L(transactional)

However thinking more about it I am not sure if this should be really a
problem: on default HTM mode the program must handle self-induced failures
as the tbegin. failure path and I assume trying to suspend/resume in this
case will trigger this.  For instance:

   if (__builtin_tbegin (0))
     {
       /* some transactional stuff.  */
       __builtin_tsuspend ();
       /* non transactional stuff.  */
       __builtin_tresume ();
       /* more transactional stuff.  */
     }
   else
     {
       /* fall-out code.  */
     }

So I assume for these chips without suspend/resume support the example
code will always run the fall-out code.