| Message ID | 20201027215118.27003-6-will@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | An alternative series for asymmetric AArch32 systems | expand |
On Tue, Oct 27, 2020 at 09:51:17PM +0000, Will Deacon wrote: > Since 32-bit applications will be killed if they are caught trying to > execute on a 64-bit-only CPU in a mismatched system, advertise the set > of 32-bit capable CPUs to userspace in sysfs. > > Signed-off-by: Will Deacon <will@kernel.org> > --- > .../ABI/testing/sysfs-devices-system-cpu | 8 ++++++++ > arch/arm64/kernel/cpufeature.c | 19 +++++++++++++++++++ > 2 files changed, 27 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu > index b555df825447..19893fb8e870 100644 > --- a/Documentation/ABI/testing/sysfs-devices-system-cpu > +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu > @@ -472,6 +472,14 @@ Description: AArch64 CPU registers > 'identification' directory exposes the CPU ID registers for > identifying model and revision of the CPU. > > +What: /sys/devices/system/cpu/aarch32_el0 > +Date: October 2020 > +Contact: Linux ARM Kernel Mailing list <linux-arm-kernel@lists.infradead.org> > +Description: Identifies the subset of CPUs in the system that can execute > + AArch32 (32-bit ARM) applications. If absent, then all or none > + of the CPUs can execute AArch32 applications and execve() will > + behave accordingly. How is this value represented? A hint here would be nice. > + > What: /sys/devices/system/cpu/cpu#/cpu_capacity > Date: December 2016 > Contact: Linux kernel mailing list <linux-kernel@vger.kernel.org> > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > index 2e2219cbd54c..9f29d4d1ef7e 100644 > --- a/arch/arm64/kernel/cpufeature.c > +++ b/arch/arm64/kernel/cpufeature.c > @@ -67,6 +67,7 @@ > #include <linux/crash_dump.h> > #include <linux/sort.h> > #include <linux/stop_machine.h> > +#include <linux/sysfs.h> > #include <linux/types.h> > #include <linux/mm.h> > #include <linux/cpu.h> > @@ -1236,6 +1237,24 @@ bool system_has_mismatched_32bit_el0(void) > return fld == ID_AA64PFR0_EL0_64BIT_ONLY; > } > > +static ssize_t aarch32_el0_show(struct kobject *kobj, > + struct kobj_attribute *attr, char *buf) > +{ > + const struct cpumask *mask = system_32bit_el0_cpumask(); > + return sprintf(buf, "%*pbl\n", cpumask_pr_args(mask)); sysfs_emit()? And a blank line to make checkpatch.pl happy :) > +} > +static const struct kobj_attribute aarch32_el0_attr = __ATTR_RO(aarch32_el0); DEVICE_ATTR_RO()? > + > +static int __init aarch32_el0_sysfs_init(void) > +{ > + if (!__allow_mismatched_32bit_el0) > + return 0; > + > + return sysfs_create_file(&cpu_subsys.dev_root->kobj, > + &aarch32_el0_attr.attr); device_create_file() please, dev_root is a struct device, no need to "thunk" down to a "raw" sysfs call. thanks, greg k-h
On Wed, Oct 28, 2020 at 09:37:46AM +0100, Greg Kroah-Hartman wrote: > On Tue, Oct 27, 2020 at 09:51:17PM +0000, Will Deacon wrote: > > Since 32-bit applications will be killed if they are caught trying to > > execute on a 64-bit-only CPU in a mismatched system, advertise the set > > of 32-bit capable CPUs to userspace in sysfs. > > > > Signed-off-by: Will Deacon <will@kernel.org> > > --- > > .../ABI/testing/sysfs-devices-system-cpu | 8 ++++++++ > > arch/arm64/kernel/cpufeature.c | 19 +++++++++++++++++++ > > 2 files changed, 27 insertions(+) > > > > diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu > > index b555df825447..19893fb8e870 100644 > > --- a/Documentation/ABI/testing/sysfs-devices-system-cpu > > +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu > > @@ -472,6 +472,14 @@ Description: AArch64 CPU registers > > 'identification' directory exposes the CPU ID registers for > > identifying model and revision of the CPU. > > > > +What: /sys/devices/system/cpu/aarch32_el0 > > +Date: October 2020 > > +Contact: Linux ARM Kernel Mailing list <linux-arm-kernel@lists.infradead.org> > > +Description: Identifies the subset of CPUs in the system that can execute > > + AArch32 (32-bit ARM) applications. If absent, then all or none > > + of the CPUs can execute AArch32 applications and execve() will > > + behave accordingly. > > How is this value represented? A hint here would be nice. It's in the same format as /sys/devices/system/cpu/{online,offline,possible,present}, so I'll just say that (although the text for those doesn't seem to specify it either...). > > What: /sys/devices/system/cpu/cpu#/cpu_capacity > > Date: December 2016 > > Contact: Linux kernel mailing list <linux-kernel@vger.kernel.org> > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > > index 2e2219cbd54c..9f29d4d1ef7e 100644 > > --- a/arch/arm64/kernel/cpufeature.c > > +++ b/arch/arm64/kernel/cpufeature.c > > @@ -67,6 +67,7 @@ > > #include <linux/crash_dump.h> > > #include <linux/sort.h> > > #include <linux/stop_machine.h> > > +#include <linux/sysfs.h> > > #include <linux/types.h> > > #include <linux/mm.h> > > #include <linux/cpu.h> > > @@ -1236,6 +1237,24 @@ bool system_has_mismatched_32bit_el0(void) > > return fld == ID_AA64PFR0_EL0_64BIT_ONLY; > > } > > > > +static ssize_t aarch32_el0_show(struct kobject *kobj, > > + struct kobj_attribute *attr, char *buf) > > +{ > > + const struct cpumask *mask = system_32bit_el0_cpumask(); > > + return sprintf(buf, "%*pbl\n", cpumask_pr_args(mask)); > > sysfs_emit()? > > And a blank line to make checkpatch.pl happy :) Hehe, yeah ok. > > +} > > +static const struct kobj_attribute aarch32_el0_attr = __ATTR_RO(aarch32_el0); > > DEVICE_ATTR_RO()? > > > + > > +static int __init aarch32_el0_sysfs_init(void) > > +{ > > + if (!__allow_mismatched_32bit_el0) > > + return 0; > > + > > + return sysfs_create_file(&cpu_subsys.dev_root->kobj, > > + &aarch32_el0_attr.attr); > > device_create_file() please, dev_root is a struct device, no need to > "thunk" down to a "raw" sysfs call. Totally missed I had a struct device in my hand, so hopefully that will tidy things up a little bit. Cheers, Will
On Tue, Oct 27, 2020 at 09:51:17PM +0000, Will Deacon wrote: > diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu > index b555df825447..19893fb8e870 100644 > --- a/Documentation/ABI/testing/sysfs-devices-system-cpu > +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu > @@ -472,6 +472,14 @@ Description: AArch64 CPU registers > 'identification' directory exposes the CPU ID registers for > identifying model and revision of the CPU. > > +What: /sys/devices/system/cpu/aarch32_el0 Nitpick: should we call this aarch32_el0_present? It's not exactly present as we populate it as CPUs come online but it's closer to this mask than to the online one. > +Date: October 2020 > +Contact: Linux ARM Kernel Mailing list <linux-arm-kernel@lists.infradead.org> > +Description: Identifies the subset of CPUs in the system that can execute > + AArch32 (32-bit ARM) applications. If absent, then all or none > + of the CPUs can execute AArch32 applications and execve() will > + behave accordingly. What does "accordingly" mean? Normally, we'd get ENOEXEC but here the execve() "succeeds" followed by a SIGKILL if it ends up on the wrong CPU.
On Wed, Oct 28, 2020 at 12:15:07PM +0000, Catalin Marinas wrote: > On Tue, Oct 27, 2020 at 09:51:17PM +0000, Will Deacon wrote: > > diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu > > index b555df825447..19893fb8e870 100644 > > --- a/Documentation/ABI/testing/sysfs-devices-system-cpu > > +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu > > @@ -472,6 +472,14 @@ Description: AArch64 CPU registers > > 'identification' directory exposes the CPU ID registers for > > identifying model and revision of the CPU. > > > > +What: /sys/devices/system/cpu/aarch32_el0 > > Nitpick: should we call this aarch32_el0_present? It's not exactly > present as we populate it as CPUs come online but it's closer to this > mask than to the online one. I don't think so, because a CPU could be set in this mask but not in the present mask, which is hugely confusing it it has "present" in the name! > > +Date: October 2020 > > +Contact: Linux ARM Kernel Mailing list <linux-arm-kernel@lists.infradead.org> > > +Description: Identifies the subset of CPUs in the system that can execute > > + AArch32 (32-bit ARM) applications. If absent, then all or none > > + of the CPUs can execute AArch32 applications and execve() will > > + behave accordingly. > > What does "accordingly" mean? Normally, we'd get ENOEXEC but here the > execve() "succeeds" followed by a SIGKILL if it ends up on the wrong > CPU. No; if the file is absent then execve() behaves as it always has. Will
On Wed, Oct 28, 2020 at 12:27:59PM +0000, Will Deacon wrote: > On Wed, Oct 28, 2020 at 12:15:07PM +0000, Catalin Marinas wrote: > > On Tue, Oct 27, 2020 at 09:51:17PM +0000, Will Deacon wrote: > > > diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu > > > index b555df825447..19893fb8e870 100644 > > > --- a/Documentation/ABI/testing/sysfs-devices-system-cpu > > > +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu > > > @@ -472,6 +472,14 @@ Description: AArch64 CPU registers > > > 'identification' directory exposes the CPU ID registers for > > > identifying model and revision of the CPU. > > > > > > +What: /sys/devices/system/cpu/aarch32_el0 > > > > Nitpick: should we call this aarch32_el0_present? It's not exactly > > present as we populate it as CPUs come online but it's closer to this > > mask than to the online one. > > I don't think so, because a CPU could be set in this mask but not in the > present mask, which is hugely confusing it it has "present" in the name! How can it end up here but not in the present mask? We populate present early if they have a corresponding DT entry. > > > +Date: October 2020 > > > +Contact: Linux ARM Kernel Mailing list <linux-arm-kernel@lists.infradead.org> > > > +Description: Identifies the subset of CPUs in the system that can execute > > > + AArch32 (32-bit ARM) applications. If absent, then all or none > > > + of the CPUs can execute AArch32 applications and execve() will > > > + behave accordingly. > > > > What does "accordingly" mean? Normally, we'd get ENOEXEC but here the > > execve() "succeeds" followed by a SIGKILL if it ends up on the wrong > > CPU. > > No; if the file is absent then execve() behaves as it always has. Ah, I missed the "absent" part and got confused.
On Wed, Oct 28, 2020 at 03:14:43PM +0000, Catalin Marinas wrote: > On Wed, Oct 28, 2020 at 12:27:59PM +0000, Will Deacon wrote: > > On Wed, Oct 28, 2020 at 12:15:07PM +0000, Catalin Marinas wrote: > > > On Tue, Oct 27, 2020 at 09:51:17PM +0000, Will Deacon wrote: > > > > diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu > > > > index b555df825447..19893fb8e870 100644 > > > > --- a/Documentation/ABI/testing/sysfs-devices-system-cpu > > > > +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu > > > > @@ -472,6 +472,14 @@ Description: AArch64 CPU registers > > > > 'identification' directory exposes the CPU ID registers for > > > > identifying model and revision of the CPU. > > > > > > > > +What: /sys/devices/system/cpu/aarch32_el0 > > > > > > Nitpick: should we call this aarch32_el0_present? It's not exactly > > > present as we populate it as CPUs come online but it's closer to this > > > mask than to the online one. > > > > I don't think so, because a CPU could be set in this mask but not in the > > present mask, which is hugely confusing it it has "present" in the name! > > How can it end up here but not in the present mask? We populate present > early if they have a corresponding DT entry. I was under the impression that physical CPU hotplug with ACPI would clear the entry in the present mask, but I can't say I have any machines that I can test that with and it looks like it might only be implemented for x86 at the moment. That said, it looks like cpu_die_early() also marks CPUs as not being present and this can happen due to a late capability conflict. Will
diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu index b555df825447..19893fb8e870 100644 --- a/Documentation/ABI/testing/sysfs-devices-system-cpu +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu @@ -472,6 +472,14 @@ Description: AArch64 CPU registers 'identification' directory exposes the CPU ID registers for identifying model and revision of the CPU. +What: /sys/devices/system/cpu/aarch32_el0 +Date: October 2020 +Contact: Linux ARM Kernel Mailing list <linux-arm-kernel@lists.infradead.org> +Description: Identifies the subset of CPUs in the system that can execute + AArch32 (32-bit ARM) applications. If absent, then all or none + of the CPUs can execute AArch32 applications and execve() will + behave accordingly. + What: /sys/devices/system/cpu/cpu#/cpu_capacity Date: December 2016 Contact: Linux kernel mailing list <linux-kernel@vger.kernel.org> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index 2e2219cbd54c..9f29d4d1ef7e 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -67,6 +67,7 @@ #include <linux/crash_dump.h> #include <linux/sort.h> #include <linux/stop_machine.h> +#include <linux/sysfs.h> #include <linux/types.h> #include <linux/mm.h> #include <linux/cpu.h> @@ -1236,6 +1237,24 @@ bool system_has_mismatched_32bit_el0(void) return fld == ID_AA64PFR0_EL0_64BIT_ONLY; } +static ssize_t aarch32_el0_show(struct kobject *kobj, + struct kobj_attribute *attr, char *buf) +{ + const struct cpumask *mask = system_32bit_el0_cpumask(); + return sprintf(buf, "%*pbl\n", cpumask_pr_args(mask)); +} +static const struct kobj_attribute aarch32_el0_attr = __ATTR_RO(aarch32_el0); + +static int __init aarch32_el0_sysfs_init(void) +{ + if (!__allow_mismatched_32bit_el0) + return 0; + + return sysfs_create_file(&cpu_subsys.dev_root->kobj, + &aarch32_el0_attr.attr); +} +device_initcall(aarch32_el0_sysfs_init); + static bool has_32bit_el0(const struct arm64_cpu_capabilities *entry, int scope) { return has_cpuid_feature(entry, scope) || __allow_mismatched_32bit_el0;
Since 32-bit applications will be killed if they are caught trying to execute on a 64-bit-only CPU in a mismatched system, advertise the set of 32-bit capable CPUs to userspace in sysfs. Signed-off-by: Will Deacon <will@kernel.org> --- .../ABI/testing/sysfs-devices-system-cpu | 8 ++++++++ arch/arm64/kernel/cpufeature.c | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+)