Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1460221 > unrolled thread

Re: [PATCH 05/19] arm64: rename COMPAT to AARCH32_EL0 in Kconfig

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 05/19] arm64: rename COMPAT to AARCH32_EL0 in Kconfig "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> - 2016-08-11 09:40 +0200
    Re: [PATCH 05/19] arm64: rename COMPAT to AARCH32_EL0 in Kconfig Arnd Bergmann <arnd@arndb.de> - 2016-08-11 11:00 +0200
      Re: [PATCH 05/19] arm64: rename COMPAT to AARCH32_EL0 in Kconfig Catalin Marinas <catalin.marinas@arm.com> - 2016-08-11 17:00 +0200
        Re: [PATCH 05/19] arm64: rename COMPAT to AARCH32_EL0 in Kconfig Arnd Bergmann <arnd@arndb.de> - 2016-08-11 17:20 +0200
          Re: [PATCH 05/19] arm64: rename COMPAT to AARCH32_EL0 in Kconfig Catalin Marinas <catalin.marinas@arm.com> - 2016-08-11 18:40 +0200
            Re: [PATCH 05/19] arm64: rename COMPAT to AARCH32_EL0 in Kconfig Arnd Bergmann <arnd@arndb.de> - 2016-08-11 22:40 +0200
              Re: [PATCH 05/19] arm64: rename COMPAT to AARCH32_EL0 in Kconfig Catalin Marinas <catalin.marinas@arm.com> - 2016-08-12 16:40 +0200
                Re: [PATCH 05/19] arm64: rename COMPAT to AARCH32_EL0 in Kconfig Catalin Marinas <catalin.marinas@arm.com> - 2016-08-15 11:40 +0200

#1460221 — Re: [PATCH 05/19] arm64: rename COMPAT to AARCH32_EL0 in Kconfig

Hi, Yury

On 2016/6/18 7:54, Yury Norov wrote:
> From: Andrew Pinski <apinski@cavium.com>
>
> In this patchset  ILP32 ABI support is added. Additionally to AARCH32,
> which is binary-compatible with ARM, ILP32 is (mostly) ABI-compatible.
>
>  From now, AARCH32_EL0 (former COMPAT) config option means the support of
> AARCH32 userspace, ARM64_ILP32 - support of ILP32 ABI (see next patches),
> and COMPAT indicates that one of them, or both, is enabled.
>
> Where needed, CONFIG_COMPAT is changed over to use CONFIG_AARCH32_EL0 instead
>
> Reviewed-by: David Daney <ddaney@caviumnetworks.com>
> Signed-off-by: Andrew Pinski <Andrew.Pinski@caviumnetworks.com>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
> Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
...
> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> index c173d32..af200a8 100644
> --- a/arch/arm64/kernel/cpuinfo.c
> +++ b/arch/arm64/kernel/cpuinfo.c
> @@ -134,15 +134,17 @@ static int c_show(struct seq_file *m, void *v)
>   		 */
>   		seq_puts(m, "Features\t:");
>   		if (compat) {
> -#ifdef CONFIG_COMPAT
> -			for (j = 0; compat_hwcap_str[j]; j++)
> -				if (compat_elf_hwcap & (1 << j))
> -					seq_printf(m, " %s", compat_hwcap_str[j]);
> -
> -			for (j = 0; compat_hwcap2_str[j]; j++)
> -				if (compat_elf_hwcap2 & (1 << j))
> -					seq_printf(m, " %s", compat_hwcap2_str[j]);
> -#endif /* CONFIG_COMPAT */
> +#ifdef CONFIG_AARCH32_EL0
I saw that compat_hwcap_str and compat_hwcap2_str is defined when
"CONFIG_COMPAT" is true. Why we only change it to CONFIG_AARCH32_EL0
in c show()?
> +			if (personality(current->personality) == PER_LINUX32) {
And "compat" is "personality(current->personality) == PER_LINUX32;",
it seems that there is no need to add this twice.

Regards

Bamvor
> +				for (j = 0; compat_hwcap_str[j]; j++)
> +					if (compat_elf_hwcap & (1 << j))
> +						seq_printf(m, " %s", compat_hwcap_str[j]);
> +
> +				for (j = 0; compat_hwcap2_str[j]; j++)
> +					if (compat_elf_hwcap2 & (1 << j))
> +						seq_printf(m, " %s", compat_hwcap2_str[j]);
> +			}
> +#endif /* CONFIG_AARCH32_EL0 */
>   		} else {
>   			for (j = 0; hwcap_str[j]; j++)
>   				if (elf_hwcap & (1 << j))

[toc] | [next] | [standalone]

#1460295

On Thursday, August 11, 2016 3:35:01 PM CEST Zhangjian (Bamvor) wrote:
> On 2016/6/18 7:54, Yury Norov wrote:
> > From: Andrew Pinski <apinski@cavium.com>
> >
> > In this patchset  ILP32 ABI support is added. Additionally to AARCH32,
> > which is binary-compatible with ARM, ILP32 is (mostly) ABI-compatible.
> >
> >  From now, AARCH32_EL0 (former COMPAT) config option means the support of
> > AARCH32 userspace, ARM64_ILP32 - support of ILP32 ABI (see next patches),
> > and COMPAT indicates that one of them, or both, is enabled.
> >
> > Where needed, CONFIG_COMPAT is changed over to use CONFIG_AARCH32_EL0 instead
> >
> > Reviewed-by: David Daney <ddaney@caviumnetworks.com>
> > Signed-off-by: Andrew Pinski <Andrew.Pinski@caviumnetworks.com>
> > Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> > Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> > Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
> > Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
> ...
> > diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> > index c173d32..af200a8 100644
> > --- a/arch/arm64/kernel/cpuinfo.c
> > +++ b/arch/arm64/kernel/cpuinfo.c
> > @@ -134,15 +134,17 @@ static int c_show(struct seq_file *m, void *v)
> >   		 */
> >   		seq_puts(m, "Features\t:");
> >   		if (compat) {
> > -#ifdef CONFIG_COMPAT
> > -			for (j = 0; compat_hwcap_str[j]; j++)
> > -				if (compat_elf_hwcap & (1 << j))
> > -					seq_printf(m, " %s", compat_hwcap_str[j]);
> > -
> > -			for (j = 0; compat_hwcap2_str[j]; j++)
> > -				if (compat_elf_hwcap2 & (1 << j))
> > -					seq_printf(m, " %s", compat_hwcap2_str[j]);
> > -#endif /* CONFIG_COMPAT */
> > +#ifdef CONFIG_AARCH32_EL0
> I saw that compat_hwcap_str and compat_hwcap2_str is defined when
> "CONFIG_COMPAT" is true. Why we only change it to CONFIG_AARCH32_EL0
> in c show()?
> > +			if (personality(current->personality) == PER_LINUX32) {
> And "compat" is "personality(current->personality) == PER_LINUX32;",
> it seems that there is no need to add this twice.

I think it would be best to remove the #ifdef here completely,
the PER_LINUX32 concept is not strictly tied to the emulation
of ARM binaries, it literally just changes the output of
/proc/cpuinfo and 'uname', and you can have ARM binaries with
PER_LINUX (using the arm64 uname) just like you can have
arm64 binaries running with PER_LINUX32.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1460586

On Thu, Aug 11, 2016 at 10:53:01AM +0200, Arnd Bergmann wrote:
> On Thursday, August 11, 2016 3:35:01 PM CEST Zhangjian (Bamvor) wrote:
> > On 2016/6/18 7:54, Yury Norov wrote:
> > > From: Andrew Pinski <apinski@cavium.com>
> > >
> > > In this patchset  ILP32 ABI support is added. Additionally to AARCH32,
> > > which is binary-compatible with ARM, ILP32 is (mostly) ABI-compatible.
> > >
> > >  From now, AARCH32_EL0 (former COMPAT) config option means the support of
> > > AARCH32 userspace, ARM64_ILP32 - support of ILP32 ABI (see next patches),
> > > and COMPAT indicates that one of them, or both, is enabled.
> > >
> > > Where needed, CONFIG_COMPAT is changed over to use CONFIG_AARCH32_EL0 instead
> > >
> > > Reviewed-by: David Daney <ddaney@caviumnetworks.com>
> > > Signed-off-by: Andrew Pinski <Andrew.Pinski@caviumnetworks.com>
> > > Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> > > Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> > > Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
> > > Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
> > ...
> > > diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> > > index c173d32..af200a8 100644
> > > --- a/arch/arm64/kernel/cpuinfo.c
> > > +++ b/arch/arm64/kernel/cpuinfo.c
> > > @@ -134,15 +134,17 @@ static int c_show(struct seq_file *m, void *v)
> > >   		 */
> > >   		seq_puts(m, "Features\t:");
> > >   		if (compat) {
> > > -#ifdef CONFIG_COMPAT
> > > -			for (j = 0; compat_hwcap_str[j]; j++)
> > > -				if (compat_elf_hwcap & (1 << j))
> > > -					seq_printf(m, " %s", compat_hwcap_str[j]);
> > > -
> > > -			for (j = 0; compat_hwcap2_str[j]; j++)
> > > -				if (compat_elf_hwcap2 & (1 << j))
> > > -					seq_printf(m, " %s", compat_hwcap2_str[j]);
> > > -#endif /* CONFIG_COMPAT */
> > > +#ifdef CONFIG_AARCH32_EL0
> > I saw that compat_hwcap_str and compat_hwcap2_str is defined when
> > "CONFIG_COMPAT" is true. Why we only change it to CONFIG_AARCH32_EL0
> > in c show()?
> > > +			if (personality(current->personality) == PER_LINUX32) {
> > And "compat" is "personality(current->personality) == PER_LINUX32;",
> > it seems that there is no need to add this twice.
> 
> I think it would be best to remove the #ifdef here completely,
> the PER_LINUX32 concept is not strictly tied to the emulation
> of ARM binaries, it literally just changes the output of
> /proc/cpuinfo and 'uname',

It's not strictly related to ARM binaries, however it is related to
AArch32 CPU features being supported and detected by the kernel.
Currently, with CONFIG_COMPAT disabled, we won't have access to a
(meaningful) compat_elf_hwcap.

> and you can have ARM binaries with
> PER_LINUX (using the arm64 uname) just like you can have
> arm64 binaries running with PER_LINUX32.

I was actually looking to enforce the 32-bit binaries to only see
PER_LINUX32, though with a risk of breaking the ABI. OTOH, people are
abusing this and write 32-bit apps relying on the 64-bit /proc/cpuinfo:

http://lkml.kernel.org/g/1464706504-25224-3-git-send-email-catalin.marinas@arm.com

(you were summoned on that discussion couple of times ;))

-- 
Catalin

[toc] | [prev] | [next] | [standalone]

#1460603

On Thursday, August 11, 2016 3:50:00 PM CEST Catalin Marinas wrote:
> On Thu, Aug 11, 2016 at 10:53:01AM +0200, Arnd Bergmann wrote:
> > On Thursday, August 11, 2016 3:35:01 PM CEST Zhangjian (Bamvor) wrote:
> > > On 2016/6/18 7:54, Yury Norov wrote:
> > > > From: Andrew Pinski <apinski@cavium.com>
> > > >
> > > > In this patchset  ILP32 ABI support is added. Additionally to AARCH32,
> > > > which is binary-compatible with ARM, ILP32 is (mostly) ABI-compatible.
> > > >
> > > >  From now, AARCH32_EL0 (former COMPAT) config option means the support of
> > > > AARCH32 userspace, ARM64_ILP32 - support of ILP32 ABI (see next patches),
> > > > and COMPAT indicates that one of them, or both, is enabled.
> > > >
> > > > Where needed, CONFIG_COMPAT is changed over to use CONFIG_AARCH32_EL0 instead
> > > >
> > > > Reviewed-by: David Daney <ddaney@caviumnetworks.com>
> > > > Signed-off-by: Andrew Pinski <Andrew.Pinski@caviumnetworks.com>
> > > > Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> > > > Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> > > > Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
> > > > Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
> > > ...
> > > > diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> > > > index c173d32..af200a8 100644
> > > > --- a/arch/arm64/kernel/cpuinfo.c
> > > > +++ b/arch/arm64/kernel/cpuinfo.c
> > > > @@ -134,15 +134,17 @@ static int c_show(struct seq_file *m, void *v)
> > > >   		 */
> > > >   		seq_puts(m, "Features\t:");
> > > >   		if (compat) {
> > > > -#ifdef CONFIG_COMPAT
> > > > -			for (j = 0; compat_hwcap_str[j]; j++)
> > > > -				if (compat_elf_hwcap & (1 << j))
> > > > -					seq_printf(m, " %s", compat_hwcap_str[j]);
> > > > -
> > > > -			for (j = 0; compat_hwcap2_str[j]; j++)
> > > > -				if (compat_elf_hwcap2 & (1 << j))
> > > > -					seq_printf(m, " %s", compat_hwcap2_str[j]);
> > > > -#endif /* CONFIG_COMPAT */
> > > > +#ifdef CONFIG_AARCH32_EL0
> > > I saw that compat_hwcap_str and compat_hwcap2_str is defined when
> > > "CONFIG_COMPAT" is true. Why we only change it to CONFIG_AARCH32_EL0
> > > in c show()?
> > > > +			if (personality(current->personality) == PER_LINUX32) {
> > > And "compat" is "personality(current->personality) == PER_LINUX32;",
> > > it seems that there is no need to add this twice.
> > 
> > I think it would be best to remove the #ifdef here completely,
> > the PER_LINUX32 concept is not strictly tied to the emulation
> > of ARM binaries, it literally just changes the output of
> > /proc/cpuinfo and 'uname',
> 
> It's not strictly related to ARM binaries, however it is related to
> AArch32 CPU features being supported and detected by the kernel.
> Currently, with CONFIG_COMPAT disabled, we won't have access to a
> (meaningful) compat_elf_hwcap.

Ah, makes sense. In that case, using CONFIG_AARCH32_EL0 sounds like
the right thing to do here, though I guess we can just drop the
"if (compat)" check, as we specifically want to print the supported
features of the CPU, and they are still present even if a
process with PER_LINUX reads them.

> > and you can have ARM binaries with
> > PER_LINUX (using the arm64 uname) just like you can have
> > arm64 binaries running with PER_LINUX32.
> 
> I was actually looking to enforce the 32-bit binaries to only see
> PER_LINUX32, though with a risk of breaking the ABI. OTOH, people are
> abusing this and write 32-bit apps relying on the 64-bit /proc/cpuinfo:
> 
> http://lkml.kernel.org/g/1464706504-25224-3-git-send-email-catalin.marinas@arm.com
> 
> (you were summoned on that discussion couple of times ;))

Hmm, I thought I saw the thread and didn't have any good idea for
the uname information, but didn't notice it was for /proc/cpuinfo.

What's wrong with always showing both the 32-bit and the 64-bit
hwcap strings here (minus the duplicates, which hopefully have
the same meaning here)?

	Arnd

[toc] | [prev] | [next] | [standalone]

#1460677

On Thu, Aug 11, 2016 at 05:16:45PM +0200, Arnd Bergmann wrote:
> On Thursday, August 11, 2016 3:50:00 PM CEST Catalin Marinas wrote:
> > On Thu, Aug 11, 2016 at 10:53:01AM +0200, Arnd Bergmann wrote:
> > > On Thursday, August 11, 2016 3:35:01 PM CEST Zhangjian (Bamvor) wrote:
> > > > On 2016/6/18 7:54, Yury Norov wrote:
> > > > > From: Andrew Pinski <apinski@cavium.com>
> > > > >
> > > > > In this patchset  ILP32 ABI support is added. Additionally to AARCH32,
> > > > > which is binary-compatible with ARM, ILP32 is (mostly) ABI-compatible.
> > > > >
> > > > >  From now, AARCH32_EL0 (former COMPAT) config option means the support of
> > > > > AARCH32 userspace, ARM64_ILP32 - support of ILP32 ABI (see next patches),
> > > > > and COMPAT indicates that one of them, or both, is enabled.
> > > > >
> > > > > Where needed, CONFIG_COMPAT is changed over to use CONFIG_AARCH32_EL0 instead
> > > > >
> > > > > Reviewed-by: David Daney <ddaney@caviumnetworks.com>
> > > > > Signed-off-by: Andrew Pinski <Andrew.Pinski@caviumnetworks.com>
> > > > > Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> > > > > Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> > > > > Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
> > > > > Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
> > > > ...
> > > > > diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> > > > > index c173d32..af200a8 100644
> > > > > --- a/arch/arm64/kernel/cpuinfo.c
> > > > > +++ b/arch/arm64/kernel/cpuinfo.c
> > > > > @@ -134,15 +134,17 @@ static int c_show(struct seq_file *m, void *v)
> > > > >   		 */
> > > > >   		seq_puts(m, "Features\t:");
> > > > >   		if (compat) {
> > > > > -#ifdef CONFIG_COMPAT
> > > > > -			for (j = 0; compat_hwcap_str[j]; j++)
> > > > > -				if (compat_elf_hwcap & (1 << j))
> > > > > -					seq_printf(m, " %s", compat_hwcap_str[j]);
> > > > > -
> > > > > -			for (j = 0; compat_hwcap2_str[j]; j++)
> > > > > -				if (compat_elf_hwcap2 & (1 << j))
> > > > > -					seq_printf(m, " %s", compat_hwcap2_str[j]);
> > > > > -#endif /* CONFIG_COMPAT */
> > > > > +#ifdef CONFIG_AARCH32_EL0
> > > > I saw that compat_hwcap_str and compat_hwcap2_str is defined when
> > > > "CONFIG_COMPAT" is true. Why we only change it to CONFIG_AARCH32_EL0
> > > > in c show()?
> > > > > +			if (personality(current->personality) == PER_LINUX32) {
> > > > And "compat" is "personality(current->personality) == PER_LINUX32;",
> > > > it seems that there is no need to add this twice.
> > > 
> > > I think it would be best to remove the #ifdef here completely,
> > > the PER_LINUX32 concept is not strictly tied to the emulation
> > > of ARM binaries, it literally just changes the output of
> > > /proc/cpuinfo and 'uname',
> > 
> > It's not strictly related to ARM binaries, however it is related to
> > AArch32 CPU features being supported and detected by the kernel.
> > Currently, with CONFIG_COMPAT disabled, we won't have access to a
> > (meaningful) compat_elf_hwcap.
> 
> Ah, makes sense. In that case, using CONFIG_AARCH32_EL0 sounds like
> the right thing to do here, though I guess we can just drop the
> "if (compat)" check, as we specifically want to print the supported
> features of the CPU, and they are still present even if a
> process with PER_LINUX reads them.

Do you mean always printing both compat and native hwcaps in
/proc/cpuinfo? We discussed this in the past and it's not something we
can easily fix at this stage without breaking the ABI. If we noticed
this before, we could have used distinct feature strings for AArch32 and
AArch64 (e.g. crypto stuff like aes32 and aes64 but we only have aes for
both).

> > > and you can have ARM binaries with
> > > PER_LINUX (using the arm64 uname) just like you can have
> > > arm64 binaries running with PER_LINUX32.
> > 
> > I was actually looking to enforce the 32-bit binaries to only see
> > PER_LINUX32, though with a risk of breaking the ABI. OTOH, people are
> > abusing this and write 32-bit apps relying on the 64-bit /proc/cpuinfo:
> > 
> > http://lkml.kernel.org/g/1464706504-25224-3-git-send-email-catalin.marinas@arm.com
> > 
> > (you were summoned on that discussion couple of times ;))
> 
> Hmm, I thought I saw the thread and didn't have any good idea for
> the uname information, but didn't notice it was for /proc/cpuinfo.
> 
> What's wrong with always showing both the 32-bit and the 64-bit
> hwcap strings here (minus the duplicates, which hopefully have
> the same meaning here)?

As I said above, some of them have the same name (which may be a good
thing at a first look) but we don't have an architecture guarantee that
the feature is present in both AArch32 and AArch64 modes (e.g. AES may
only be available in AArch64).

-- 
Catalin

[toc] | [prev] | [next] | [standalone]

#1460788

On Thursday, August 11, 2016 5:30:03 PM CEST Catalin Marinas wrote:

> > > > and you can have ARM binaries with
> > > > PER_LINUX (using the arm64 uname) just like you can have
> > > > arm64 binaries running with PER_LINUX32.
> > > 
> > > I was actually looking to enforce the 32-bit binaries to only see
> > > PER_LINUX32, though with a risk of breaking the ABI. OTOH, people are
> > > abusing this and write 32-bit apps relying on the 64-bit /proc/cpuinfo:
> > > 
> > > http://lkml.kernel.org/g/1464706504-25224-3-git-send-email-catalin.marinas@arm.com
> > > 
> > > (you were summoned on that discussion couple of times ;))
> > 
> > Hmm, I thought I saw the thread and didn't have any good idea for
> > the uname information, but didn't notice it was for /proc/cpuinfo.
> > 
> > What's wrong with always showing both the 32-bit and the 64-bit
> > hwcap strings here (minus the duplicates, which hopefully have
> > the same meaning here)?
> 
> As I said above, some of them have the same name (which may be a good
> thing at a first look) but we don't have an architecture guarantee that
> the feature is present in both AArch32 and AArch64 modes (e.g. AES may
> only be available in AArch64).

Is this the case on actual implementations that exist today? If they
are actually always both present, we might be able to get away with it.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1461230

On Thu, Aug 11, 2016 at 10:29:03PM +0200, Arnd Bergmann wrote:
> On Thursday, August 11, 2016 5:30:03 PM CEST Catalin Marinas wrote:
> > > > > and you can have ARM binaries with
> > > > > PER_LINUX (using the arm64 uname) just like you can have
> > > > > arm64 binaries running with PER_LINUX32.
> > > > 
> > > > I was actually looking to enforce the 32-bit binaries to only see
> > > > PER_LINUX32, though with a risk of breaking the ABI. OTOH, people are
> > > > abusing this and write 32-bit apps relying on the 64-bit /proc/cpuinfo:
> > > > 
> > > > http://lkml.kernel.org/g/1464706504-25224-3-git-send-email-catalin.marinas@arm.com
> > > > 
> > > > (you were summoned on that discussion couple of times ;))
> > > 
> > > Hmm, I thought I saw the thread and didn't have any good idea for
> > > the uname information, but didn't notice it was for /proc/cpuinfo.
> > > 
> > > What's wrong with always showing both the 32-bit and the 64-bit
> > > hwcap strings here (minus the duplicates, which hopefully have
> > > the same meaning here)?
> > 
> > As I said above, some of them have the same name (which may be a good
> > thing at a first look) but we don't have an architecture guarantee that
> > the feature is present in both AArch32 and AArch64 modes (e.g. AES may
> > only be available in AArch64).
> 
> Is this the case on actual implementations that exist today? If they
> are actually always both present, we might be able to get away with it.

It may be fine on current implementations but what would we do when/if
we actually find such discrepancy? It's not just ARM Ltd designing the
chips, so as long as the architecture doesn't mandate it you may find
strange implementations.

Imposing such restriction in the architecture doesn't make sense if the
only reason is the /proc/cpuinfo file (and I can't think of any other
reason why this should be enforced).

What I'm worried about is 32-bit apps running on an arm64 kernel and
making use of the 64-bit /proc/cpuinfo without any guarantee that the
AArch32 state has such features. In my patch proposal linked above I
wanted to always force the compat /proc/cpuinfo for 32-bit tasks.

-- 
Catalin

[toc] | [prev] | [next] | [standalone]

#1462677

On Sat, Aug 13, 2016 at 06:17:03PM +0300, Yury Norov wrote:
> On Fri, Aug 12, 2016 at 03:36:12PM +0100, Catalin Marinas wrote:
> > On Thu, Aug 11, 2016 at 10:29:03PM +0200, Arnd Bergmann wrote:
> > > On Thursday, August 11, 2016 5:30:03 PM CEST Catalin Marinas wrote:
> > > > > > > and you can have ARM binaries with
> > > > > > > PER_LINUX (using the arm64 uname) just like you can have
> > > > > > > arm64 binaries running with PER_LINUX32.
> > > > > > 
> > > > > > I was actually looking to enforce the 32-bit binaries to only see
> > > > > > PER_LINUX32, though with a risk of breaking the ABI. OTOH, people are
> > > > > > abusing this and write 32-bit apps relying on the 64-bit /proc/cpuinfo:
> > > > > > 
> > > > > > http://lkml.kernel.org/g/1464706504-25224-3-git-send-email-catalin.marinas@arm.com
> > > > > > 
> > > > > > (you were summoned on that discussion couple of times ;))
> > > > > 
> > > > > Hmm, I thought I saw the thread and didn't have any good idea for
> > > > > the uname information, but didn't notice it was for /proc/cpuinfo.
> > > > > 
> > > > > What's wrong with always showing both the 32-bit and the 64-bit
> > > > > hwcap strings here (minus the duplicates, which hopefully have
> > > > > the same meaning here)?
> > > > 
> > > > As I said above, some of them have the same name (which may be a good
> > > > thing at a first look) but we don't have an architecture guarantee that
> > > > the feature is present in both AArch32 and AArch64 modes (e.g. AES may
> > > > only be available in AArch64).
> > > 
> > > Is this the case on actual implementations that exist today? If they
> > > are actually always both present, we might be able to get away with it.
> > 
> > It may be fine on current implementations but what would we do when/if
> > we actually find such discrepancy? It's not just ARM Ltd designing the
> > chips, so as long as the architecture doesn't mandate it you may find
> > strange implementations.
> > 
> > Imposing such restriction in the architecture doesn't make sense if the
> > only reason is the /proc/cpuinfo file (and I can't think of any other
> > reason why this should be enforced).
> > 
> > What I'm worried about is 32-bit apps running on an arm64 kernel and
> > making use of the 64-bit /proc/cpuinfo without any guarantee that the
> > AArch32 state has such features. In my patch proposal linked above I
> > wanted to always force the compat /proc/cpuinfo for 32-bit tasks.
> 
> The link doesn't work for me. Is there other link, or what's the
> maillist there?

With lkml.kernel.org, just change the 'g' with an 'r':

http://lkml.kernel.org/r/1464706504-25224-3-git-send-email-catalin.marinas@arm.com

It was on linux-arm-kernel.

> So, what we decided finally? Is my understanding correct that we leave
> everything as is in ilp32 series, and it will be resolved separately?

ILP32 is not affected by this since the hwcap for ILP32 should match
native. It was more a question about whether AArch32 tasks should ever
have access to AArch64 hwcaps (and potential misuse).

-- 
Catalin

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web