powerpc: Update AT_HWCAP2 bits

  This patch will wait for the acceptance of the kernel patch [1] into
Linus' tree.

[1] http://patchwork.ozlabs.org/patch/824764/

--- 8< ---

Linux commit ID XXXXXXXXX reserved a new bit for a scenario where
transactional memory is available, but the suspended state is disabled.

2017-10-18  Tulio Magno Quites Machado Filho  <tuliom@linux.vnet.ibm.com>

	* sysdeps/powerpc/bits/hwcap.h (PPC_FEATURE2_HTM_NO_SUSPEND): New
	macro.
	* sysdeps/powerpc/dl-procinfo.c (_dl_powerpc_cap_flags): Add
	htm-no-suspend.

Signed-off-by: Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com>
---
 sysdeps/powerpc/bits/hwcap.h  | 2 ++
 sysdeps/powerpc/dl-procinfo.c | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)
  

On 10/18/2017 09:15 PM, Tulio Magno Quites Machado Filho wrote:
> Linux commit ID XXXXXXXXX reserved a new bit for a scenario where
> transactional memory is available, but the suspended state is disabled.

The semantics of this bit are wrong.  If the suspended state is not 
available, the old hwcap bit needs to be cleared, and a new flag needs 
to be set which documents the availability of the different for, of 
transactional memory.  Otherwise, old applications which assume the 
presence of the suspended state when the old hwcap bit is set will break.

(I think we should have this discussion on the kernel list.)

Thanks,
Florian
  

Florian Weimer <fweimer@redhat.com> writes:

> On 10/18/2017 09:15 PM, Tulio Magno Quites Machado Filho wrote:
>> Linux commit ID XXXXXXXXX reserved a new bit for a scenario where
>> transactional memory is available, but the suspended state is disabled.
>
> The semantics of this bit are wrong.  If the suspended state is not 
> available, the old hwcap bit needs to be cleared, and a new flag needs 
> to be set which documents the availability of the different for, of 
> transactional memory.  Otherwise, old applications which assume the 
> presence of the suspended state when the old hwcap bit is set will break.
>
> (I think we should have this discussion on the kernel list.)

Agreed.  Especially because I don't think the points you're raising will affect
the glibc patch unless it's necessary to change the name of the bit.
  

On 19/10/2017 07:57, Florian Weimer wrote:
> On 10/18/2017 09:15 PM, Tulio Magno Quites Machado Filho wrote:
>> Linux commit ID XXXXXXXXX reserved a new bit for a scenario where
>> transactional memory is available, but the suspended state is disabled.
> 
> The semantics of this bit are wrong.  If the suspended state is not available, the old hwcap bit needs to be cleared, and a new flag needs to be set which documents the availability of the different for, of transactional memory.  Otherwise, old applications which assume the presence of the suspended state when the old hwcap bit is set will break.
> 
> (I think we should have this discussion on the kernel list.)
> 
> Thanks,
> Florian

How exactly this kind of hardware handles the 'tsr.' instruction
state change? Assuming the kernel submission proposal it seems
an ABI extension, since MSR[TS] will be change from 0xb10 to 0b00
in case of a transaction (instead of expected 0b10 to 0x01).  
Will it also execute the failure handling in this case, as for a 
'tresume' in the case a failure in suspended state? Also, will
it also change TEXARS with the abort information?

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 Zanella <adhemerval.zanella@linaro.org> writes:

> On 19/10/2017 07:57, Florian Weimer wrote:
>> On 10/18/2017 09:15 PM, Tulio Magno Quites Machado Filho wrote:
>>> Linux commit ID XXXXXXXXX reserved a new bit for a scenario where
>>> transactional memory is available, but the suspended state is disabled.
>> 
>> The semantics of this bit are wrong.  If the suspended state is not available, the old hwcap bit needs to be cleared, and a new flag needs to be set which documents the availability of the different for, of transactional memory.  Otherwise, old applications which assume the presence of the suspended state when the old hwcap bit is set will break.
>> 
>> (I think we should have this discussion on the kernel list.)

I'd appreciate if we could follow Florian's suggestion and keep this
discussion in linuxppc-dev mailing list, with the authors of these patches
involved too.  ;-)

For the record, the last version of the patch series is being reviewed at [1]
[2].

[1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2017-October/164701.html
[2] http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=7773

> How exactly this kind of hardware handles the 'tsr.' instruction
> state change? Assuming the kernel submission proposal it seems
> an ABI extension, since MSR[TS] will be change from 0xb10 to 0b00
> in case of a transaction (instead of expected 0b10 to 0x01).

Indeed.  It traps and aborts the transaction.

> Will it also execute the failure handling in this case, as for a 
> 'tresume' in the case a failure in suspended state?

When suspend is not available, it will never enter suspended state.

> Also, 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.

> 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.

I'm forwarding your last question and this comment to linuxppc-dev and Cc'ing
you.  Would you mind to elaborate what you're mean there, please?

By the way, you may want to read my reply to Florian there [3] before posting.

[3] https://lists.ozlabs.org/pipermail/linuxppc-dev/2017-October/165124.html

Thank you all for these comments!
  

Hi Tulio,

On 18-10-2017 17:15, Tulio Magno Quites Machado Filho wrote:
> This patch will wait for the acceptance of the kernel patch [1] into
> Linus' tree.
> 
> [1] http://patchwork.ozlabs.org/patch/824764/

Is it really necessary to wait the TM patches into Linus' tree to push your
patch as-is, given that from discussion @linuxppc-dev with Ellerman it's
now clear that 'htm' feature and 'htm-no-suspend' feature are mutually
exclusive?

I'm just asking b/c I don't see any harm on running your patch with a kernel
without TM no suspend mode support and it looks correct as-is.


Regards,
Gustavo

> --- 8< ---
> 
> Linux commit ID XXXXXXXXX reserved a new bit for a scenario where
> transactional memory is available, but the suspended state is disabled.
> 
> 2017-10-18  Tulio Magno Quites Machado Filho  <tuliom@linux.vnet.ibm.com>
> 
> 	* sysdeps/powerpc/bits/hwcap.h (PPC_FEATURE2_HTM_NO_SUSPEND): New
> 	macro.
> 	* sysdeps/powerpc/dl-procinfo.c (_dl_powerpc_cap_flags): Add
> 	htm-no-suspend.
> 
> Signed-off-by: Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com>
> ---
>  sysdeps/powerpc/bits/hwcap.h  | 2 ++
>  sysdeps/powerpc/dl-procinfo.c | 4 ++--
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/sysdeps/powerpc/bits/hwcap.h b/sysdeps/powerpc/bits/hwcap.h
> index dfc71c2..0668ca0 100644
> --- a/sysdeps/powerpc/bits/hwcap.h
> +++ b/sysdeps/powerpc/bits/hwcap.h
> @@ -72,3 +72,5 @@
>  						 128-bit */
>  #define PPC_FEATURE2_DARN	   0x00200000 /* darn instruction.  */
>  #define PPC_FEATURE2_SCV	   0x00100000 /* scv syscall.  */
> +#define PPC_FEATURE2_HTM_NO_SUSPEND  0x00080000 /* TM without suspended
> +						   state.  */
> diff --git a/sysdeps/powerpc/dl-procinfo.c b/sysdeps/powerpc/dl-procinfo.c
> index 4dac16d..55a6e78 100644
> --- a/sysdeps/powerpc/dl-procinfo.c
> +++ b/sysdeps/powerpc/dl-procinfo.c
> @@ -45,7 +45,7 @@
>  #if !defined PROCINFO_DECL && defined SHARED
>    ._dl_powerpc_cap_flags
>  #else
> -PROCINFO_CLASS const char _dl_powerpc_cap_flags[64][10]
> +PROCINFO_CLASS const char _dl_powerpc_cap_flags[64][15]
>  #endif
>  #ifndef PROCINFO_DECL
>  = {
> @@ -61,7 +61,7 @@ PROCINFO_CLASS const char _dl_powerpc_cap_flags[64][10]
>      "", "", "", "",
>      "", "", "", "",
>      "", "", "", "",
> -    "", "", "", "",
> +    "", "", "", "htm-no-suspend",
>      "scv", "darn", "ieee128", "arch_3_00",
>      "htm-nosc", "vcrypto", "tar", "isel",
>      "ebb", "dscr", "htm", "arch_2_07",
>
  

On 08/11/2017 14:11, Gustavo Romero wrote:
> Hi Tulio,
> 
> On 18-10-2017 17:15, Tulio Magno Quites Machado Filho wrote:
>> This patch will wait for the acceptance of the kernel patch [1] into
>> Linus' tree.
>>
>> [1] http://patchwork.ozlabs.org/patch/824764/
> 
> Is it really necessary to wait the TM patches into Linus' tree to push your
> patch as-is, given that from discussion @linuxppc-dev with Ellerman it's
> now clear that 'htm' feature and 'htm-no-suspend' feature are mutually
> exclusive?
> 
> I'm just asking b/c I don't see any harm on running your patch with a kernel
> without TM no suspend mode support and it looks correct as-is.

Yes, we only assume that it is de facto canonical kernel ABI if it
has been accepted on Linus' tree.
  

Hi Zanella,

On 08-11-2017 14:46, Adhemerval Zanella wrote:
> 
> 
> On 08/11/2017 14:11, Gustavo Romero wrote:
>> Hi Tulio,
>>
>> On 18-10-2017 17:15, Tulio Magno Quites Machado Filho wrote:
>>> This patch will wait for the acceptance of the kernel patch [1] into
>>> Linus' tree.
>>>
>>> [1] http://patchwork.ozlabs.org/patch/824764/
>>
>> Is it really necessary to wait the TM patches into Linus' tree to push your
>> patch as-is, given that from discussion @linuxppc-dev with Ellerman it's
>> now clear that 'htm' feature and 'htm-no-suspend' feature are mutually
>> exclusive?
>>
>> I'm just asking b/c I don't see any harm on running your patch with a kernel
>> without TM no suspend mode support and it looks correct as-is.
> 
> Yes, we only assume that it is de facto canonical kernel ABI if it
> has been accepted on Linus' tree.

I see. Thanks for the info.


Regards,
Gustavo
  

Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com> writes:

> This patch will wait for the acceptance of the kernel patch [1] into
> Linus' tree.

Kernel patch has been merged, so I'm merging the glibc patch too:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5b0e2cb020085efe202123162502e0b551e49a0e

> Linux commit ID XXXXXXXXX reserved a new bit for a scenario where
> transactional memory is available, but the suspended state is disabled.
>
> 2017-10-18  Tulio Magno Quites Machado Filho  <tuliom@linux.vnet.ibm.com>
>
> 	* sysdeps/powerpc/bits/hwcap.h (PPC_FEATURE2_HTM_NO_SUSPEND): New
> 	macro.
> 	* sysdeps/powerpc/dl-procinfo.c (_dl_powerpc_cap_flags): Add
> 	htm-no-suspend.
>
> Signed-off-by: Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com>
  

On 11/17/2017 12:00 PM, Tulio Magno Quites Machado Filho wrote:
> Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com> writes:
> 
>> This patch will wait for the acceptance of the kernel patch [1] into
>> Linus' tree.
> 
> Kernel patch has been merged, so I'm merging the glibc patch too:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5b0e2cb020085efe202123162502e0b551e49a0e

Note that while merging to Linus tree marks the point at which glibc will
accept changes per:
https://sourceware.org/glibc/wiki/Style_and_Conventions#Support_for_features_not_yet_in_the_mainstream_Linux_kernel.3F

... it is not a stable ABI until you see a Linux 4.15 with that constant
released.

Likewise the constant in glibc master is not stable until we release this
glibc on February 1st 2018.

So one should be careful that this constant is *close* to being frozen
forever by the glibc release and cannot be changed in the kernel, even
if you wanted to.
  

Carlos O'Donell <carlos@redhat.com> writes:

> On 11/17/2017 12:00 PM, Tulio Magno Quites Machado Filho wrote:
>> Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com> writes:
>> 
>>> This patch will wait for the acceptance of the kernel patch [1] into
>>> Linus' tree.
>> 
>> Kernel patch has been merged, so I'm merging the glibc patch too:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5b0e2cb020085efe202123162502e0b551e49a0e
>
> Note that while merging to Linus tree marks the point at which glibc will
> accept changes per:
> https://sourceware.org/glibc/wiki/Style_and_Conventions#Support_for_features_not_yet_in_the_mainstream_Linux_kernel.3F
>
> ... it is not a stable ABI until you see a Linux 4.15 with that constant
> released.
>
> Likewise the constant in glibc master is not stable until we release this
> glibc on February 1st 2018.
>
> So one should be careful that this constant is *close* to being frozen
> forever by the glibc release and cannot be changed in the kernel, even
> if you wanted to.

Ack.
  

On 11/17/2017 09:00 PM, Tulio Magno Quites Machado Filho wrote:
> Tulio Magno Quites Machado Filho<tuliom@linux.vnet.ibm.com>  writes:
> 
>> This patch will wait for the acceptance of the kernel patch [1] into
>> Linus' tree.
> Kernel patch has been merged, so I'm merging the glibc patch too:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5b0e2cb020085efe202123162502e0b551e49a0e

Could we backport this to 2.26?  It's essentially a hardware errata 
workaround, so perhaps it qualifies as a bugfix?

If not, I think we'd carry it as a customer downstream patch.

Thanks,
Florian
  

On Nov 20 2017, Florian Weimer <fweimer@redhat.com> wrote:

> On 11/17/2017 09:00 PM, Tulio Magno Quites Machado Filho wrote:
>> Tulio Magno Quites Machado Filho<tuliom@linux.vnet.ibm.com>  writes:
>>
>>> This patch will wait for the acceptance of the kernel patch [1] into
>>> Linus' tree.
>> Kernel patch has been merged, so I'm merging the glibc patch too:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5b0e2cb020085efe202123162502e0b551e49a0e
>
> Could we backport this to 2.26?  It's essentially a hardware errata
> workaround, so perhaps it qualifies as a bugfix?

Doesn't the _dl_powerpc_cap_flags change mean that this is a (libc
internal) ABI change?

Andreas.
  

On 11/20/2017 04:53 PM, Andreas Schwab wrote:
> On Nov 20 2017, Florian Weimer <fweimer@redhat.com> wrote:
> 
>> On 11/17/2017 09:00 PM, Tulio Magno Quites Machado Filho wrote:
>>> Tulio Magno Quites Machado Filho<tuliom@linux.vnet.ibm.com>  writes:
>>>
>>>> This patch will wait for the acceptance of the kernel patch [1] into
>>>> Linus' tree.
>>> Kernel patch has been merged, so I'm merging the glibc patch too:
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5b0e2cb020085efe202123162502e0b551e49a0e
>>
>> Could we backport this to 2.26?  It's essentially a hardware errata
>> workaround, so perhaps it qualifies as a bugfix?
> 
> Doesn't the _dl_powerpc_cap_flags change mean that this is a (libc
> internal) ABI change?

Oops.  I probably replied to the wrong patch then.  I meant the related 
patch which adds the flag to an installed header.

Thanks,
Florian
  

On Nov 20 2017, Florian Weimer <fweimer@redhat.com> wrote:

> On 11/20/2017 04:53 PM, Andreas Schwab wrote:
>> On Nov 20 2017, Florian Weimer <fweimer@redhat.com> wrote:
>>
>>> On 11/17/2017 09:00 PM, Tulio Magno Quites Machado Filho wrote:
>>>> Tulio Magno Quites Machado Filho<tuliom@linux.vnet.ibm.com>  writes:
>>>>
>>>>> This patch will wait for the acceptance of the kernel patch [1] into
>>>>> Linus' tree.
>>>> Kernel patch has been merged, so I'm merging the glibc patch too:
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5b0e2cb020085efe202123162502e0b551e49a0e
>>>
>>> Could we backport this to 2.26?  It's essentially a hardware errata
>>> workaround, so perhaps it qualifies as a bugfix?
>>
>> Doesn't the _dl_powerpc_cap_flags change mean that this is a (libc
>> internal) ABI change?
>
> Oops.  I probably replied to the wrong patch then.  I meant the related
> patch which adds the flag to an installed header.

It does both, so you would have to split that off.

Andreas.
  

On 11/20/2017 06:17 PM, Andreas Schwab wrote:
> On Nov 20 2017, Florian Weimer <fweimer@redhat.com> wrote:
> 
>> On 11/20/2017 04:53 PM, Andreas Schwab wrote:
>>> On Nov 20 2017, Florian Weimer <fweimer@redhat.com> wrote:
>>>
>>>> On 11/17/2017 09:00 PM, Tulio Magno Quites Machado Filho wrote:
>>>>> Tulio Magno Quites Machado Filho<tuliom@linux.vnet.ibm.com>  writes:
>>>>>
>>>>>> This patch will wait for the acceptance of the kernel patch [1] into
>>>>>> Linus' tree.
>>>>> Kernel patch has been merged, so I'm merging the glibc patch too:
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5b0e2cb020085efe202123162502e0b551e49a0e
>>>>
>>>> Could we backport this to 2.26?  It's essentially a hardware errata
>>>> workaround, so perhaps it qualifies as a bugfix?
>>>
>>> Doesn't the _dl_powerpc_cap_flags change mean that this is a (libc
>>> internal) ABI change?
>>
>> Oops.  I probably replied to the wrong patch then.  I meant the related
>> patch which adds the flag to an installed header.
> 
> It does both, so you would have to split that off.

Sorry, I'm interested in the header file change.  Not sure if this was 
clear.

Thanks,
Florian
  

On Nov 20 2017, Florian Weimer <fweimer@redhat.com> wrote:

> Sorry, I'm interested in the header file change.  Not sure if this was
> clear.

Then you need to split that off.

Andreas.
  

Andreas Schwab <schwab@suse.de> writes:

> On Nov 20 2017, Florian Weimer <fweimer@redhat.com> wrote:
>
>> Sorry, I'm interested in the header file change.  Not sure if this was
>> clear.
>
> Then you need to split that off.

This patch is changing an internal structure.
Could you elaborate why you think this is a problem?
  

On Nov 21 2017, "Tulio Magno Quites Machado Filho" <tuliom@linux.vnet.ibm.com> wrote:

> This patch is changing an internal structure.
> Could you elaborate why you think this is a problem?

_rtld_global_ro is part of the ld.so interface.

Andreas.
  

On 11/21/2017 12:21 PM, Andreas Schwab wrote:
> On Nov 21 2017, "Tulio Magno Quites Machado Filho" <tuliom@linux.vnet.ibm.com> wrote:
> 
>> This patch is changing an internal structure.
>> Could you elaborate why you think this is a problem?
> 
> _rtld_global_ro is part of the ld.so interface.

Are you suggesting that we cannot change GLIBC_PRIVATE ABIs on a release 
branch?

Thanks,
Florian
  

On 21/11/17 11:25, Florian Weimer wrote:
> On 11/21/2017 12:21 PM, Andreas Schwab wrote:
>> On Nov 21 2017, "Tulio Magno Quites Machado Filho" <tuliom@linux.vnet.ibm.com> wrote:
>>
>>> This patch is changing an internal structure.
>>> Could you elaborate why you think this is a problem?
>>
>> _rtld_global_ro is part of the ld.so interface.
> 
> Are you suggesting that we cannot change GLIBC_PRIVATE ABIs on a release branch?

anything that changes internal abi between the
modules of the libc should not be observable
externally in principle, but it can break things
during libc update or static linked binaries that
expect a particular abi from dlopened libc modules.

i'd prefer if internal abi was not changed on
release branches (including not breaking malloc
interposition or sanitizers etc that hook into
internals as much as possible).
  

On Nov 21 2017, Florian Weimer <fweimer@redhat.com> wrote:

> On 11/21/2017 12:21 PM, Andreas Schwab wrote:
>> On Nov 21 2017, "Tulio Magno Quites Machado Filho" <tuliom@linux.vnet.ibm.com> wrote:
>>
>>> This patch is changing an internal structure.
>>> Could you elaborate why you think this is a problem?
>>
>> _rtld_global_ro is part of the ld.so interface.
>
> Are you suggesting that we cannot change GLIBC_PRIVATE ABIs on a release
> branch?

You will need to restart all running processes to avoid getting
conflicts after such an update.

Andreas.
  

On 11/21/2017 12:46 PM, Andreas Schwab wrote:
> On Nov 21 2017, Florian Weimer <fweimer@redhat.com> wrote:
> 
>> On 11/21/2017 12:21 PM, Andreas Schwab wrote:
>>> On Nov 21 2017, "Tulio Magno Quites Machado Filho" <tuliom@linux.vnet.ibm.com> wrote:
>>>
>>>> This patch is changing an internal structure.
>>>> Could you elaborate why you think this is a problem?
>>>
>>> _rtld_global_ro is part of the ld.so interface.
>>
>> Are you suggesting that we cannot change GLIBC_PRIVATE ABIs on a release
>> branch?
> 
> You will need to restart all running processes to avoid getting
> conflicts after such an update.

I think _rtld_global_ro is fairly harmless in this regard because there 
is only a small window during process startup, before libc.so.6 is 
loaded.  So this will only affect processes started during the update 
(and perhaps dlmopen).

Other GLIBC_PRIVATE changes are far more invasive because they affect 
anything which does a late dlopen (which can happen an arbitrary time 
span after the update, of course).

Thanks,
Florian
  

On Nov 21 2017, Florian Weimer <fweimer@redhat.com> wrote:

> I think _rtld_global_ro is fairly harmless in this regard because there is
> only a small window during process startup, before libc.so.6 is loaded.
> So this will only affect processes started during the update (and perhaps
> dlmopen).

The problem is that the patch changes the size, and the dl-procinfo.c
parts come first in _rtld_global_ro.

Andreas.