From: Will Deacon <will@kernel.org> Date: 2021-06-02 16:47:34
Hi again folks,
Here is v8 of the asymmetric 32-bit support patches that I previously
posted here:
v1: https://lore.kernel.org/r/20201027215118.27003-1-will@kernel.org
v2: https://lore.kernel.org/r/20201109213023.15092-1-will@kernel.org
v3: https://lore.kernel.org/r/20201113093720.21106-1-will@kernel.org
v4: https://lore.kernel.org/r/20201124155039.13804-1-will@kernel.org
v5: https://lore.kernel.org/r/20201208132835.6151-1-will@kernel.org
v6: https://lore.kernel.org/r/20210518094725.7701-1-will@kernel.org
v7: https://lore.kernel.org/r/20210525151432.16875-1-will@kernel.org
There was also a nice LWN writeup in case you've forgotten what this is
about:
https://lwn.net/Articles/838339/
Changes since v7 include:
* Dropped the scheduler migration fix, as Valentin has kindly fixed
this separately in -tip:
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=sched/core&id=475ea6c60279e9f2ddf7e4cf2648cd8ae0608361
* Dropped the freezer/ttwu changes, as Peter is rewriting that:
https://lore.kernel.org/r/YLYZv4v68OnAlx+3@hirez.programming.kicks-ass.net
* Reworded documentation wrt KVM behaviour [maz]
* Tidied up control flow in cpuset_cpus_allowed_fallback() [peterz]
* Fixed interaction with migrate_disable() [peterz]
* Don't ignore allocation failure in restrict_cpus_allowed_ptr() [peterz]
* Reordered the patches to put arm64 prep work first
* Added some more acks/reviewed-by tags
Cheers,
Will
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Morten Rasmussen <redacted>
Cc: Qais Yousef <redacted>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Quentin Perret <redacted>
Cc: Tejun Heo <tj@kernel.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: "Rafael J. Wysocki" <redacted>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Daniel Bristot de Oliveira <redacted>
Cc: Valentin Schneider <redacted>
Cc: kernel-team@android.com
--->8
Will Deacon (19):
arm64: cpuinfo: Split AArch32 registers out into a separate struct
arm64: Allow mismatched 32-bit EL0 support
KVM: arm64: Kill 32-bit vCPUs on systems with mismatched EL0 support
arm64: Kill 32-bit applications scheduled on 64-bit-only CPUs
sched: Introduce task_cpu_possible_mask() to limit fallback rq
selection
cpuset: Don't use the cpu_possible_mask as a last resort for cgroup v1
cpuset: Honour task_cpu_possible_mask() in guarantee_online_cpus()
sched: Reject CPU affinity changes based on task_cpu_possible_mask()
sched: Introduce task_struct::user_cpus_ptr to track requested
affinity
sched: Split the guts of sched_setaffinity() into a helper function
sched: Allow task CPU affinity to be restricted on asymmetric systems
sched: Introduce task_cpus_dl_admissible() to check proposed affinity
arm64: Implement task_cpu_possible_mask()
arm64: exec: Adjust affinity for compat tasks with mismatched 32-bit
EL0
arm64: Prevent offlining first CPU with 32-bit EL0 on mismatched
system
arm64: Advertise CPUs capable of running 32-bit applications in sysfs
arm64: Hook up cmdline parameter to allow mismatched 32-bit EL0
arm64: Remove logic to kill 32-bit tasks on 64-bit-only cores
Documentation: arm64: describe asymmetric 32-bit support
.../ABI/testing/sysfs-devices-system-cpu | 9 +
.../admin-guide/kernel-parameters.txt | 11 +
Documentation/arm64/asymmetric-32bit.rst | 155 ++++++++
Documentation/arm64/index.rst | 1 +
arch/arm64/include/asm/cpu.h | 44 +--
arch/arm64/include/asm/cpufeature.h | 8 +-
arch/arm64/include/asm/elf.h | 6 +-
arch/arm64/include/asm/mmu_context.h | 13 +
arch/arm64/kernel/cpufeature.c | 227 +++++++++---
arch/arm64/kernel/cpuinfo.c | 53 +--
arch/arm64/kernel/process.c | 44 ++-
arch/arm64/kvm/arm.c | 11 +-
arch/arm64/tools/cpucaps | 3 +-
include/linux/cpuset.h | 3 +-
include/linux/mmu_context.h | 11 +
include/linux/sched.h | 21 ++
init/init_task.c | 1 +
kernel/cgroup/cpuset.c | 49 ++-
kernel/fork.c | 2 +
kernel/sched/core.c | 334 ++++++++++++++----
kernel/sched/sched.h | 1 +
21 files changed, 819 insertions(+), 188 deletions(-)
create mode 100644 Documentation/arm64/asymmetric-32bit.rst
--
2.32.0.rc0.204.g9fa02ecfa5-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-06-02 16:47:37
In preparation for late initialisation of the "sanitised" AArch32 register
state, move the AArch32 registers out of 'struct cpuinfo' and into their
own struct definition.
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/cpu.h | 44 +++++++++++----------
arch/arm64/kernel/cpufeature.c | 71 ++++++++++++++++++----------------
arch/arm64/kernel/cpuinfo.c | 53 +++++++++++++------------
3 files changed, 89 insertions(+), 79 deletions(-)
@@ -54,6 +35,29 @@ struct cpuinfo_arm64 {u32reg_mvfr0;u32reg_mvfr1;u32reg_mvfr2;+};++structcpuinfo_arm64{+structcpucpu;+structkobjectkobj;+u32reg_ctr;+u32reg_cntfrq;+u32reg_dczid;+u32reg_midr;+u32reg_revidr;++u64reg_id_aa64dfr0;+u64reg_id_aa64dfr1;+u64reg_id_aa64isar0;+u64reg_id_aa64isar1;+u64reg_id_aa64mmfr0;+u64reg_id_aa64mmfr1;+u64reg_id_aa64mmfr2;+u64reg_id_aa64pfr0;+u64reg_id_aa64pfr1;+u64reg_id_aa64zfr0;++structcpuinfo_32bitaarch32;/* pseudo-ZCR for recording maximum ZCR_EL1 LEN value: */u64reg_zcr;
@@ -863,6 +863,31 @@ static void __init init_cpu_hwcaps_indirect_list(void)staticvoid__initsetup_boot_cpu_capabilities(void);+staticvoid__initinit_32bit_cpu_features(structcpuinfo_32bit*info)+{+init_cpu_ftr_reg(SYS_ID_DFR0_EL1,info->reg_id_dfr0);+init_cpu_ftr_reg(SYS_ID_DFR1_EL1,info->reg_id_dfr1);+init_cpu_ftr_reg(SYS_ID_ISAR0_EL1,info->reg_id_isar0);+init_cpu_ftr_reg(SYS_ID_ISAR1_EL1,info->reg_id_isar1);+init_cpu_ftr_reg(SYS_ID_ISAR2_EL1,info->reg_id_isar2);+init_cpu_ftr_reg(SYS_ID_ISAR3_EL1,info->reg_id_isar3);+init_cpu_ftr_reg(SYS_ID_ISAR4_EL1,info->reg_id_isar4);+init_cpu_ftr_reg(SYS_ID_ISAR5_EL1,info->reg_id_isar5);+init_cpu_ftr_reg(SYS_ID_ISAR6_EL1,info->reg_id_isar6);+init_cpu_ftr_reg(SYS_ID_MMFR0_EL1,info->reg_id_mmfr0);+init_cpu_ftr_reg(SYS_ID_MMFR1_EL1,info->reg_id_mmfr1);+init_cpu_ftr_reg(SYS_ID_MMFR2_EL1,info->reg_id_mmfr2);+init_cpu_ftr_reg(SYS_ID_MMFR3_EL1,info->reg_id_mmfr3);+init_cpu_ftr_reg(SYS_ID_MMFR4_EL1,info->reg_id_mmfr4);+init_cpu_ftr_reg(SYS_ID_MMFR5_EL1,info->reg_id_mmfr5);+init_cpu_ftr_reg(SYS_ID_PFR0_EL1,info->reg_id_pfr0);+init_cpu_ftr_reg(SYS_ID_PFR1_EL1,info->reg_id_pfr1);+init_cpu_ftr_reg(SYS_ID_PFR2_EL1,info->reg_id_pfr2);+init_cpu_ftr_reg(SYS_MVFR0_EL1,info->reg_mvfr0);+init_cpu_ftr_reg(SYS_MVFR1_EL1,info->reg_mvfr1);+init_cpu_ftr_reg(SYS_MVFR2_EL1,info->reg_mvfr2);+}+void__initinit_cpu_features(structcpuinfo_arm64*info){/* Before we start using the tables, make sure it is sorted */
@@ -371,31 +397,8 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)info->reg_id_aa64pfr1=read_cpuid(ID_AA64PFR1_EL1);info->reg_id_aa64zfr0=read_cpuid(ID_AA64ZFR0_EL1);-/* Update the 32bit ID registers only if AArch32 is implemented */-if(id_aa64pfr0_32bit_el0(info->reg_id_aa64pfr0)){-info->reg_id_dfr0=read_cpuid(ID_DFR0_EL1);-info->reg_id_dfr1=read_cpuid(ID_DFR1_EL1);-info->reg_id_isar0=read_cpuid(ID_ISAR0_EL1);-info->reg_id_isar1=read_cpuid(ID_ISAR1_EL1);-info->reg_id_isar2=read_cpuid(ID_ISAR2_EL1);-info->reg_id_isar3=read_cpuid(ID_ISAR3_EL1);-info->reg_id_isar4=read_cpuid(ID_ISAR4_EL1);-info->reg_id_isar5=read_cpuid(ID_ISAR5_EL1);-info->reg_id_isar6=read_cpuid(ID_ISAR6_EL1);-info->reg_id_mmfr0=read_cpuid(ID_MMFR0_EL1);-info->reg_id_mmfr1=read_cpuid(ID_MMFR1_EL1);-info->reg_id_mmfr2=read_cpuid(ID_MMFR2_EL1);-info->reg_id_mmfr3=read_cpuid(ID_MMFR3_EL1);-info->reg_id_mmfr4=read_cpuid(ID_MMFR4_EL1);-info->reg_id_mmfr5=read_cpuid(ID_MMFR5_EL1);-info->reg_id_pfr0=read_cpuid(ID_PFR0_EL1);-info->reg_id_pfr1=read_cpuid(ID_PFR1_EL1);-info->reg_id_pfr2=read_cpuid(ID_PFR2_EL1);--info->reg_mvfr0=read_cpuid(MVFR0_EL1);-info->reg_mvfr1=read_cpuid(MVFR1_EL1);-info->reg_mvfr2=read_cpuid(MVFR2_EL1);-}+if(id_aa64pfr0_32bit_el0(info->reg_id_aa64pfr0))+__cpuinfo_store_cpu_32bit(&info->aarch32);if(IS_ENABLED(CONFIG_ARM64_SVE)&&id_aa64pfr0_sve(info->reg_id_aa64pfr0))
--
2.32.0.rc0.204.g9fa02ecfa5-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-06-02 16:47:43
When confronted with a mixture of CPUs, some of which support 32-bit
applications and others which don't, we quite sensibly treat the system
as 64-bit only for userspace and prevent execve() of 32-bit binaries.
Unfortunately, some crazy folks have decided to build systems like this
with the intention of running 32-bit applications, so relax our
sanitisation logic to continue to advertise 32-bit support to userspace
on these systems and track the real 32-bit capable cores in a cpumask
instead. For now, the default behaviour remains but will be tied to
a command-line option in a later patch.
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/cpufeature.h | 8 +-
arch/arm64/kernel/cpufeature.c | 114 ++++++++++++++++++++++++----
arch/arm64/tools/cpucaps | 3 +-
3 files changed, 110 insertions(+), 15 deletions(-)
@@ -2426,7 +2482,7 @@ static bool cpus_have_elf_hwcap(const struct arm64_cpu_capabilities *cap)returnrc;}-staticvoid__initsetup_elf_hwcaps(conststructarm64_cpu_capabilities*hwcaps)+staticvoidsetup_elf_hwcaps(conststructarm64_cpu_capabilities*hwcaps){/* We support emulation of accesses to CPU ID feature registers */cpu_set_named_feature(CPUID);
From: Will Deacon <will@kernel.org> Date: 2021-06-02 16:47:46
If a vCPU is caught running 32-bit code on a system with mismatched
support at EL0, then we should kill it.
Acked-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/kvm/arm.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
From: Will Deacon <will@kernel.org> Date: 2021-06-02 16:47:50
Scheduling a 32-bit application on a 64-bit-only CPU is a bad idea.
Ensure that 32-bit applications always take the slow-path when returning
to userspace on a system with mismatched support at EL0, so that we can
avoid trying to run on a 64-bit-only CPU and force a SIGKILL instead.
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/kernel/process.c | 19 ++++++++++++++++++-
arch/arm64/kernel/signal.c | 26 ++++++++++++++++++++++++++
2 files changed, 44 insertions(+), 1 deletion(-)
@@ -633,8 +643,15 @@ unsigned long arch_align_stack(unsigned long sp)*/voidarch_setup_new_exec(void){-current->mm->context.flags=is_compat_task()?MMCF_AARCH32:0;+unsignedlongmmflags=0;++if(is_compat_task()){+mmflags=MMCF_AARCH32;+if(static_branch_unlikely(&arm64_mismatched_32bit_el0))+set_tsk_thread_flag(current,TIF_NOTIFY_RESUME);+}+current->mm->context.flags=mmflags;ptrauth_thread_init_user();mte_thread_init_user();
From: Will Deacon <will@kernel.org> Date: 2021-06-02 16:47:53
Asymmetric systems may not offer the same level of userspace ISA support
across all CPUs, meaning that some applications cannot be executed by
some CPUs. As a concrete example, upcoming arm64 big.LITTLE designs do
not feature support for 32-bit applications on both clusters.
On such a system, we must take care not to migrate a task to an
unsupported CPU when forcefully moving tasks in select_fallback_rq()
in response to a CPU hot-unplug operation.
Introduce a task_cpu_possible_mask() hook which, given a task argument,
allows an architecture to return a cpumask of CPUs that are capable of
executing that task. The default implementation returns the
cpu_possible_mask, since sane machines do not suffer from per-cpu ISA
limitations that affect scheduling. The new mask is used when selecting
the fallback runqueue as a last resort before forcing a migration to the
first active CPU.
Reviewed-by: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
---
include/linux/mmu_context.h | 11 +++++++++++
kernel/sched/core.c | 5 ++---
2 files changed, 13 insertions(+), 3 deletions(-)
@@ -1814,7 +1814,7 @@ static inline bool is_cpu_allowed(struct task_struct *p, int cpu)/* Non kernel threads are not allowed during either online or offline. */if(!(p->flags&PF_KTHREAD))-returncpu_active(cpu);+returncpu_active(cpu)&&task_cpu_possible(cpu,p);/* KTHREAD_IS_PER_CPU is always allowed. */if(kthread_is_per_cpu(p))
From: Will Deacon <will@kernel.org> Date: 2021-06-02 16:47:59
If the scheduler cannot find an allowed CPU for a task,
cpuset_cpus_allowed_fallback() will widen the affinity to cpu_possible_mask
if cgroup v1 is in use.
In preparation for allowing architectures to provide their own fallback
mask, just return early if we're either using cgroup v1 or we're using
cgroup v2 with a mask that contains invalid CPUs. This will allow
select_fallback_rq() to figure out the mask by itself.
Cc: Li Zefan <redacted>
Cc: Tejun Heo <tj@kernel.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Reviewed-by: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
---
include/linux/cpuset.h | 1 +
kernel/cgroup/cpuset.c | 8 ++++++--
2 files changed, 7 insertions(+), 2 deletions(-)
From: Will Deacon <will@kernel.org> Date: 2021-06-02 16:48:02
Asymmetric systems may not offer the same level of userspace ISA support
across all CPUs, meaning that some applications cannot be executed by
some CPUs. As a concrete example, upcoming arm64 big.LITTLE designs do
not feature support for 32-bit applications on both clusters.
Modify guarantee_online_cpus() to take task_cpu_possible_mask() into
account when trying to find a suitable set of online CPUs for a given
task. This will avoid passing an invalid mask to set_cpus_allowed_ptr()
during ->attach() and will subsequently allow the cpuset hierarchy to be
taken into account when forcefully overriding the affinity mask for a
task which requires migration to a compatible CPU.
Cc: Li Zefan <redacted>
Cc: Tejun Heo <tj@kernel.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Signed-off-by: Will Deacon <will@kernel.org>
---
include/linux/cpuset.h | 2 +-
kernel/cgroup/cpuset.c | 41 ++++++++++++++++++++++++++---------------
2 files changed, 27 insertions(+), 16 deletions(-)
From: Will Deacon <will@kernel.org> Date: 2021-06-02 16:48:09
Reject explicit requests to change the affinity mask of a task via
set_cpus_allowed_ptr() if the requested mask is not a subset of the
mask returned by task_cpu_possible_mask(). This ensures that the
'cpus_mask' for a given task cannot contain CPUs which are incapable of
executing it, except in cases where the affinity is forced.
Reviewed-by: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
---
kernel/sched/core.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
From: Will Deacon <will@kernel.org> Date: 2021-06-02 16:48:12
In preparation for saving and restoring the user-requested CPU affinity
mask of a task, add a new cpumask_t pointer to 'struct task_struct'.
If the pointer is non-NULL, then the mask is copied across fork() and
freed on task exit.
Signed-off-by: Will Deacon <will@kernel.org>
---
include/linux/sched.h | 13 +++++++++++++
init/init_task.c | 1 +
kernel/fork.c | 2 ++
kernel/sched/core.c | 20 ++++++++++++++++++++
4 files changed, 36 insertions(+)
From: Will Deacon <will@kernel.org> Date: 2021-06-02 16:48:17
In preparation for replaying user affinity requests using a saved mask,
split sched_setaffinity() up so that the initial task lookup and
security checks are only performed when the request is coming directly
from userspace.
Signed-off-by: Will Deacon <will@kernel.org>
---
kernel/sched/core.c | 105 ++++++++++++++++++++++++--------------------
1 file changed, 57 insertions(+), 48 deletions(-)
From: Will Deacon <will@kernel.org> Date: 2021-06-02 16:48:21
Asymmetric systems may not offer the same level of userspace ISA support
across all CPUs, meaning that some applications cannot be executed by
some CPUs. As a concrete example, upcoming arm64 big.LITTLE designs do
not feature support for 32-bit applications on both clusters.
Although userspace can carefully manage the affinity masks for such
tasks, one place where it is particularly problematic is execve()
because the CPU on which the execve() is occurring may be incompatible
with the new application image. In such a situation, it is desirable to
restrict the affinity mask of the task and ensure that the new image is
entered on a compatible CPU. From userspace's point of view, this looks
the same as if the incompatible CPUs have been hotplugged off in the
task's affinity mask. Similarly, if a subsequent execve() reverts to
a compatible image, then the old affinity is restored if it is still
valid.
In preparation for restricting the affinity mask for compat tasks on
arm64 systems without uniform support for 32-bit applications, introduce
{force,relax}_compatible_cpus_allowed_ptr(), which respectively restrict
and restore the affinity mask for a task based on the compatible CPUs.
Reviewed-by: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
---
include/linux/sched.h | 2 +
kernel/sched/core.c | 177 ++++++++++++++++++++++++++++++++++++++----
kernel/sched/sched.h | 1 +
3 files changed, 164 insertions(+), 16 deletions(-)
@@ -2430,20 +2425,170 @@ static int __set_cpus_allowed_ptr(struct task_struct *p,__do_set_cpus_allowed(p,new_mask,flags);-returnaffine_move_task(rq,p,&rf,dest_cpu,flags);+if(flags&SCA_USER)+release_user_cpus_ptr(p);++returnaffine_move_task(rq,p,rf,dest_cpu,flags);out:-task_rq_unlock(rq,p,&rf);+task_rq_unlock(rq,p,rf);returnret;}+/*+*Changeagiventask'sCPUaffinity.Migratethethreadtoa+*properCPUandscheduleitawayiftheCPUit'sexecutingon+*isremovedfromtheallowedbitmask.+*+*NOTE:thecallermusthaveavalidreferencetothetask,the+*taskmustnotexit()&deallocateitselfprematurely.The+*callisnotatomic;nospinlocksmaybeheld.+*/+staticint__set_cpus_allowed_ptr(structtask_struct*p,+conststructcpumask*new_mask,u32flags)+{+structrq_flagsrf;+structrq*rq;++rq=task_rq_lock(p,&rf);+return__set_cpus_allowed_ptr_locked(p,new_mask,flags,rq,&rf);+}+intset_cpus_allowed_ptr(structtask_struct*p,conststructcpumask*new_mask){return__set_cpus_allowed_ptr(p,new_mask,0);}EXPORT_SYMBOL_GPL(set_cpus_allowed_ptr);+/*+*Changeagiventask'sCPUaffinitytotheintersectionofitscurrent+*affinitymaskand@subset_mask,writingtheresultingmaskto@new_mask+*andpointing@p->user_cpus_ptrtoacopyoftheoldmask.+*Iftheresultingmaskisempty,leavetheaffinityunchangedandreturn+*-EINVAL.+*/+staticintrestrict_cpus_allowed_ptr(structtask_struct*p,+structcpumask*new_mask,+conststructcpumask*subset_mask)+{+structrq_flagsrf;+structrq*rq;+interr;+structcpumask*user_mask=NULL;++if(!p->user_cpus_ptr){+user_mask=kmalloc(cpumask_size(),GFP_KERNEL);++if(!user_mask)+return-ENOMEM;+}++rq=task_rq_lock(p,&rf);++/*+*Forcefullyrestrictingtheaffinityofadeadlinetaskis+*likelytocauseproblems,sofailandnoisilyoverridethe+*maskentirely.+*/+if(task_has_dl_policy(p)&&dl_bandwidth_enabled()){+err=-EPERM;+gotoerr_unlock;+}++if(!cpumask_and(new_mask,&p->cpus_mask,subset_mask)){+err=-EINVAL;+gotoerr_unlock;+}++/*+*We'reabouttobutcherthetaskaffinity,sokeeptrackofwhat+*theuseraskedforincasewe'reabletorestoreitlateron.+*/+if(user_mask){+cpumask_copy(user_mask,p->cpus_ptr);+p->user_cpus_ptr=user_mask;+}++return__set_cpus_allowed_ptr_locked(p,new_mask,0,rq,&rf);++err_unlock:+task_rq_unlock(rq,p,&rf);+kfree(user_mask);+returnerr;+}++/*+*RestricttheCPUaffinityoftask@psothatitisasubsetof+*task_cpu_possible_mask()andpoint@p->user_cpu_ptrtoacopyofthe+*oldaffinitymask.Iftheresultingmaskisempty,wewarnandwalk+*upthecpusethierarchyuntilwefindasuitablemask.+*/+voidforce_compatible_cpus_allowed_ptr(structtask_struct*p)+{+cpumask_var_tnew_mask;+conststructcpumask*override_mask=task_cpu_possible_mask(p);++alloc_cpumask_var(&new_mask,GFP_KERNEL);++/*+*__migrate_task()canfailsilentlyinthefaceofconcurrent+*offliningofthechosendestinationCPU,sotakethehotplug+*locktoensurethatthemigrationsucceeds.+*/+cpus_read_lock();+if(!cpumask_available(new_mask))+gotoout_set_mask;++if(!restrict_cpus_allowed_ptr(p,new_mask,override_mask))+gotoout_free_mask;++/*+*Wefailedtofindavalidsubsetoftheaffinitymaskforthe+*task,sooverrideitbasedonitscpusethierarchy.+*/+cpuset_cpus_allowed(p,new_mask);+override_mask=new_mask;++out_set_mask:+if(printk_ratelimit()){+printk_deferred("Overriding affinity for process %d (%s) to CPUs %*pbl\n",+task_pid_nr(p),p->comm,+cpumask_pr_args(override_mask));+}++WARN_ON(set_cpus_allowed_ptr(p,override_mask));+out_free_mask:+cpus_read_unlock();+free_cpumask_var(new_mask);+}++staticint+__sched_setaffinity(structtask_struct*p,conststructcpumask*mask);++/*+*Restoretheaffinityofatask@pwhichwaspreviouslyrestrictedbya+*calltoforce_compatible_cpus_allowed_ptr().Thiswillclear(andfree)+*@p->user_cpus_ptr.+*/+voidrelax_compatible_cpus_allowed_ptr(structtask_struct*p)+{+unsignedlongflags;+structcpumask*mask=p->user_cpus_ptr;++/*+*Trytorestoretheoldaffinitymask.Ifthisfails,then+*wefreethemaskexplicitlytoavoiditbeinginheritedacross+*asubsequentfork().+*/+if(!mask||!__sched_setaffinity(p,mask))+return;++raw_spin_lock_irqsave(&p->pi_lock,flags);+release_user_cpus_ptr(p);+raw_spin_unlock_irqrestore(&p->pi_lock,flags);+}+voidset_task_cpu(structtask_struct*p,unsignedintnew_cpu){#ifdef CONFIG_SCHED_DEBUG
From: Will Deacon <will@kernel.org> Date: 2021-06-02 16:48:29
In preparation for restricting the affinity of a task during execve()
on arm64, introduce a new task_cpus_dl_admissible() helper function to
give an indication as to whether the restricted mask is admissible for
a deadline task.
Signed-off-by: Will Deacon <will@kernel.org>
---
include/linux/sched.h | 6 ++++++
kernel/sched/core.c | 44 +++++++++++++++++++++++++++----------------
2 files changed, 34 insertions(+), 16 deletions(-)
From: Will Deacon <will@kernel.org> Date: 2021-06-02 16:48:37
Provide an implementation of task_cpu_possible_mask() so that we can
prevent 64-bit-only cores being added to the 'cpus_mask' for compat
tasks on systems with mismatched 32-bit support at EL0,
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/mmu_context.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
From: Will Deacon <will@kernel.org> Date: 2021-06-02 16:48:40
When exec'ing a 32-bit task on a system with mismatched support for
32-bit EL0, try to ensure that it starts life on a CPU that can actually
run it.
Similarly, when exec'ing a 64-bit task on such a system, try to restore
the old affinity mask if it was previously restricted.
Reviewed-by: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/elf.h | 6 ++----
arch/arm64/kernel/process.c | 39 +++++++++++++++++++++++++++++++++++-
2 files changed, 40 insertions(+), 5 deletions(-)
@@ -638,6 +639,28 @@ unsigned long arch_align_stack(unsigned long sp)returnsp&~0xf;}+#ifdef CONFIG_COMPAT+intcompat_elf_check_arch(conststructelf32_hdr*hdr)+{+if(!system_supports_32bit_el0())+returnfalse;++if((hdr)->e_machine!=EM_ARM)+returnfalse;++if(!((hdr)->e_flags&EF_ARM_EABI_MASK))+returnfalse;++/*+*Preventexecve()ofa32-bitprogramfromadeadlinetask+*iftherestrictedaffinitymaskwouldbeinadmissibleonan+*asymmetricsystem.+*/+return!static_branch_unlikely(&arm64_mismatched_32bit_el0)||+task_cpus_dl_admissible(current,system_32bit_el0_cpumask());+}+#endif+/**Calledfromsetup_new_exec()after(COMPAT_)SET_PERSONALITY.*/
From: Will Deacon <will@kernel.org> Date: 2021-06-02 16:48:43
If we want to support 32-bit applications, then when we identify a CPU
with mismatched 32-bit EL0 support we must ensure that we will always
have an active 32-bit CPU available to us from then on. This is important
for the scheduler, because is_cpu_allowed() will be constrained to 32-bit
CPUs for compat tasks and forced migration due to a hotplug event will
hang if no 32-bit CPUs are available.
On detecting a mismatch, prevent offlining of either the mismatching CPU
if it is 32-bit capable, or find the first active 32-bit capable CPU
otherwise.
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/kernel/cpufeature.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
@@ -2877,15 +2877,33 @@ void __init setup_cpu_features(void)staticintenable_mismatched_32bit_el0(unsignedintcpu){+staticintlucky_winner=-1;+structcpuinfo_arm64*info=&per_cpu(cpu_data,cpu);boolcpu_32bit=id_aa64pfr0_32bit_el0(info->reg_id_aa64pfr0);if(cpu_32bit){cpumask_set_cpu(cpu,cpu_32bit_el0_mask);static_branch_enable_cpuslocked(&arm64_mismatched_32bit_el0);-setup_elf_hwcaps(compat_elf_hwcaps);}+if(cpumask_test_cpu(0,cpu_32bit_el0_mask)==cpu_32bit)+return0;++if(lucky_winner>=0)+return0;++/*+*We'vedetectedamismatch.WeneedtokeeponeofourCPUswith+*32-bitEL0onlinesothatis_cpu_allowed()doesn'tenduprejecting+*everyCPUinthesystemfora32-bittask.+*/+lucky_winner=cpu_32bit?cpu:cpumask_any_and(cpu_32bit_el0_mask,+cpu_active_mask);+get_cpu_device(lucky_winner)->offline_disabled=true;+setup_elf_hwcaps(compat_elf_hwcaps);+pr_info("Asymmetric 32-bit EL0 support detected on CPU %u; CPU hot-unplug disabled on CPU %u\n",+cpu,lucky_winner);return0;}
--
2.32.0.rc0.204.g9fa02ecfa5-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-06-02 16:48:46
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.
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
.../ABI/testing/sysfs-devices-system-cpu | 9 +++++++++
arch/arm64/kernel/cpufeature.c | 19 +++++++++++++++++++
2 files changed, 28 insertions(+)
@@ -494,6 +494,15 @@ 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: May 2021+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 present, the same format as+ /sys/devices/system/cpu/{offline,online,possible,present} is used.+ 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>
From: Will Deacon <will@kernel.org> Date: 2021-06-02 16:48:49
Allow systems with mismatched 32-bit support at EL0 to run 32-bit
applications based on a new kernel parameter.
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
Documentation/admin-guide/kernel-parameters.txt | 8 ++++++++
arch/arm64/kernel/cpufeature.c | 7 +++++++
2 files changed, 15 insertions(+)
@@ -287,6 +287,14 @@ do not want to use tracing_snapshot_alloc() as it needs to be done where GFP_KERNEL allocations are allowed.+ allow_mismatched_32bit_el0 [ARM64]+ Allow execve() of 32-bit applications and setting of the+ PER_LINUX32 personality on systems where only a strict+ subset of the CPUs support 32-bit EL0. When this+ parameter is present, the set of CPUs supporting 32-bit+ EL0 is indicated by /sys/devices/system/cpu/aarch32_el0+ and hot-unplug operations may be restricted.+ amd_iommu= [HW,X86-64] Pass parameters to the AMD IOMMU driver in the system. Possible values are:
From: Will Deacon <will@kernel.org> Date: 2021-06-02 16:48:54
The scheduler now knows enough about these braindead systems to place
32-bit tasks accordingly, so throw out the safety checks and allow the
ret-to-user path to avoid do_notify_resume() if there is nothing to do.
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/kernel/process.c | 14 +-------------
arch/arm64/kernel/signal.c | 26 --------------------------
2 files changed, 1 insertion(+), 39 deletions(-)
From: Will Deacon <will@kernel.org> Date: 2021-06-02 16:49:00
Document support for running 32-bit tasks on asymmetric 32-bit systems
and its impact on the user ABI when enabled.
Signed-off-by: Will Deacon <will@kernel.org>
---
.../admin-guide/kernel-parameters.txt | 3 +
Documentation/arm64/asymmetric-32bit.rst | 155 ++++++++++++++++++
Documentation/arm64/index.rst | 1 +
3 files changed, 159 insertions(+)
create mode 100644 Documentation/arm64/asymmetric-32bit.rst
@@ -295,6 +295,9 @@ EL0 is indicated by /sys/devices/system/cpu/aarch32_el0 and hot-unplug operations may be restricted.+ See Documentation/arm64/asymmetric-32bit.rst for more+ information.+ amd_iommu= [HW,X86-64] Pass parameters to the AMD IOMMU driver in the system. Possible values are:
@@ -0,0 +1,155 @@+======================+Asymmetric 32-bit SoCs+======================++Author: Will Deacon <will@kernel.org>++This document describes the impact of asymmetric 32-bit SoCs on the+execution of 32-bit (``AArch32``) applications.++Date: 2021-05-17++Introduction+============++Some Armv9 SoCs suffer from a big.LITTLE misfeature where only a subset+of the CPUs are capable of executing 32-bit user applications. On such+a system, Linux by default treats the asymmetry as a "mismatch" and+disables support for both the ``PER_LINUX32`` personality and+``execve(2)`` of 32-bit ELF binaries, with the latter returning+``-ENOEXEC``. If the mismatch is detected during late onlining of a+64-bit-only CPU, then the onlining operation fails and the new CPU is+unavailable for scheduling.++Surprisingly, these SoCs have been produced with the intention of+running legacy 32-bit binaries. Unsurprisingly, that doesn't work very+well with the default behaviour of Linux.++It seems inevitable that future SoCs will drop 32-bit support+altogether, so if you're stuck in the unenviable position of needing to+run 32-bit code on one of these transitionary platforms then you would+be wise to consider alternatives such as recompilation, emulation or+retirement. If neither of those options are practical, then read on.++Enabling kernel support+=======================++Since the kernel support is not completely transparent to userspace,+allowing 32-bit tasks to run on an asymmetric 32-bit system requires an+explicit "opt-in" and can be enabled by passing the+``allow_mismatched_32bit_el0`` parameter on the kernel command-line.++For the remainder of this document we will refer to an *asymmetric+system* to mean an asymmetric 32-bit SoC running Linux with this kernel+command-line option enabled.++Userspace impact+================++32-bit tasks running on an asymmetric system behave in mostly the same+way as on a homogeneous system, with a few key differences relating to+CPU affinity.++sysfs+-----++The subset of CPUs capable of running 32-bit tasks is described in+``/sys/devices/system/cpu/aarch32_el0`` and is documented further in+``Documentation/ABI/testing/sysfs-devices-system-cpu``.++**Note:** CPUs are advertised by this file as they are detected and so+late-onlining of 32-bit-capable CPUs can result in the file contents+being modified by the kernel at runtime. Once advertised, CPUs are never+removed from the file.++``execve(2)``+-------------++On a homogeneous system, the CPU affinity of a task is preserved across+``execve(2)``. This is not always possible on an asymmetric system,+specifically when the new program being executed is 32-bit yet the+affinity mask contains 64-bit-only CPUs. In this situation, the kernel+determines the new affinity mask as follows:++1. If the 32-bit-capable subset of the affinity mask is not empty,+ then the affinity is restricted to that subset and the old affinity+ mask is saved. This saved mask is inherited over ``fork(2)`` and+ preserved across ``execve(2)`` of 32-bit programs.++**Note:** This step does not apply to ``SCHED_DEADLINE`` tasks.+ See `SCHED_DEADLINE`_.++2. Otherwise, the cpuset hierarchy of the task is walked until an+ ancestor is found containing at least one 32-bit-capable CPU. The+ affinity of the task is then changed to match the 32-bit-capable+ subset of the cpuset determined by the walk.++3. On failure (i.e. out of memory), the affinity is changed to the set+ of all 32-bit-capable CPUs of which the kernel is aware.++A subsequent ``execve(2)`` of a 64-bit program by the 32-bit task will+invalidate the affinity mask saved in (1) and attempt to restore the CPU+affinity of the task using the saved mask if it was previously valid.+This restoration may fail due to intervening changes to the deadline+policy or cpuset hierarchy, in which case the ``execve(2)`` continues+with the affinity unchanged.++Calls to ``sched_setaffinity(2)`` for a 32-bit task will consider only+the 32-bit-capable CPUs of the requested affinity mask. On success, the+affinity for the task is updated and any saved mask from a prior+``execve(2)`` is invalidated.++``SCHED_DEADLINE``+------------------++Explicit admission of a 32-bit deadline task to the default root domain+(e.g. by calling ``sched_setattr(2)``) is rejected on an asymmetric+32-bit system unless admission control is disabled by writing -1 to+``/proc/sys/kernel/sched_rt_runtime_us``.++``execve(2)`` of a 32-bit program from a 64-bit deadline task will+return ``-ENOEXEC`` if the root domain for the task contains any+64-bit-only CPUs and admission control is enabled. Concurrent offlining+of 32-bit-capable CPUs may still necessitate the procedure described in+`execve(2)`_, in which case step (1) is skipped and a warning is+emitted on the console.++**Note:** It is recommended that a set of 32-bit-capable CPUs are placed+into a separate root domain if ``SCHED_DEADLINE`` is to be used with+32-bit tasks on an asymmetric system. Failure to do so is likely to+result in missed deadlines.++Cpusets+-------++The affinity of a 32-bit task on an asymmetric system may include CPUs+that are not explicitly allowed by the cpuset to which it is attached.+This can occur as a result of the following two situations:++- A 64-bit task attached to a cpuset which allows only 64-bit CPUs+ executes a 32-bit program.++- All of the 32-bit-capable CPUs allowed by a cpuset containing a+ 32-bit task are offlined.++In both of these cases, the new affinity is calculated according to step+(2) of the process described in `execve(2)`_ and the cpuset hierarchy is+unchanged irrespective of the cgroup version.++CPU hotplug+-----------++On an asymmetric system, the first detected 32-bit-capable CPU is+prevented from being offlined by userspace and any such attempt will+return ``-EPERM``. Note that suspend is still permitted even if the+primary CPU (i.e. CPU 0) is 64-bit-only.++KVM+---++Although KVM will not advertise 32-bit EL0 support to any vCPUs on an+asymmetric system, a broken guest at EL1 could still attempt to execute+32-bit code at EL0. In this case, an exit from a vCPU thread in 32-bit+mode will return to host userspace with an ``exit_reason`` of+``KVM_EXIT_FAIL_ENTRY`` and will remain non-runnable until successfully+re-initialised by a subsequent ``KVM_ARM_VCPU_INIT`` operation.
From: Daniel Bristot de Oliveira <hidden> Date: 2021-06-03 09:43:18
On 6/2/21 6:47 PM, Will Deacon wrote:
quoted hunk
In preparation for restricting the affinity of a task during execve()
on arm64, introduce a new task_cpus_dl_admissible() helper function to
give an indication as to whether the restricted mask is admissible for
a deadline task.
Signed-off-by: Will Deacon <will@kernel.org>
---
include/linux/sched.h | 6 ++++++
kernel/sched/core.c | 44 +++++++++++++++++++++++++++----------------
2 files changed, 34 insertions(+), 16 deletions(-)
Would you mind renaming it to dl_task_check_affinity(), in the case of a v9? It
will look coherent with dl_task_can_attach()...
Reviewed-by: Daniel Bristot de Oliveira <redacted>
Thanks!
-- Daniel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Daniel Bristot de Oliveira <hidden> Date: 2021-06-03 09:45:18
On 6/2/21 6:47 PM, Will Deacon wrote:
When exec'ing a 32-bit task on a system with mismatched support for
32-bit EL0, try to ensure that it starts life on a CPU that can actually
run it.
Similarly, when exec'ing a 64-bit task on such a system, try to restore
the old affinity mask if it was previously restricted.
Reviewed-by: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
---
[...]
+#ifdef CONFIG_COMPAT
+int compat_elf_check_arch(const struct elf32_hdr *hdr)
+{
+ if (!system_supports_32bit_el0())
+ return false;
+
+ if ((hdr)->e_machine != EM_ARM)
+ return false;
+
+ if (!((hdr)->e_flags & EF_ARM_EABI_MASK))
+ return false;
+
+ /*
+ * Prevent execve() of a 32-bit program from a deadline task
+ * if the restricted affinity mask would be inadmissible on an
+ * asymmetric system.
+ */
+ return !static_branch_unlikely(&arm64_mismatched_32bit_el0) ||
+ task_cpus_dl_admissible(current, system_32bit_el0_cpumask());
+}
+#endif
From the DL perspective:
Reviewed-by: Daniel Bristot de Oliveira <redacted>
Thanks!
-- Daniel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-06-03 09:52:16
On Thu, Jun 03, 2021 at 11:43:08AM +0200, Daniel Bristot de Oliveira wrote:
On 6/2/21 6:47 PM, Will Deacon wrote:
quoted
In preparation for restricting the affinity of a task during execve()
on arm64, introduce a new task_cpus_dl_admissible() helper function to
give an indication as to whether the restricted mask is admissible for
a deadline task.
Signed-off-by: Will Deacon <will@kernel.org>
---
include/linux/sched.h | 6 ++++++
kernel/sched/core.c | 44 +++++++++++++++++++++++++++----------------
2 files changed, 34 insertions(+), 16 deletions(-)
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-06-03 12:37:31
On Wed, Jun 02, 2021 at 05:47:02PM +0100, Will Deacon wrote:
quoted hunk
When confronted with a mixture of CPUs, some of which support 32-bit
applications and others which don't, we quite sensibly treat the system
as 64-bit only for userspace and prevent execve() of 32-bit binaries.
Unfortunately, some crazy folks have decided to build systems like this
with the intention of running 32-bit applications, so relax our
sanitisation logic to continue to advertise 32-bit support to userspace
on these systems and track the real 32-bit capable cores in a cpumask
instead. For now, the default behaviour remains but will be tied to
a command-line option in a later patch.
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/cpufeature.h | 8 +-
arch/arm64/kernel/cpufeature.c | 114 ++++++++++++++++++++++++----
arch/arm64/tools/cpucaps | 3 +-
3 files changed, 110 insertions(+), 15 deletions(-)
Note that read_sanitised_ftr_reg() has to do a bsearch() to find the
arm64_ftr_reg, so this will make system_32bit_el0_cpumask() a fair
amount more expensive than it needs to be.
Can we follow the pattern we have for arm64_ftr_reg_ctrel0, and have a
arm64_ftr_reg_id_aa64pfr0_el1 that we can address directly here?
That said. I reckon this could be much cleaner if we maintained separate
caps:
ARM64_ALL_CPUS_HAVE_32BIT_EL0
ARM64_SOME_CPUS_HAVE_32BIT_EL0
... and allow arm64_mismatched_32bit_el0 to be set dependent on
ARM64_SOME_CPUS_HAVE_32BIT_EL0. With that, this can be simplified to:
static inline bool system_supports_32bit_el0(void)
{
return (cpus_have_const_cap(ARM64_ALL_CPUS_HAVE_32BIT_EL0)) ||
static_branch_unlikely(&arm64_mismatched_32bit_el0))
}
@@ -979,6 +997,22 @@ static void relax_cpu_ftr_reg(u32 sys_id, int field)WARN_ON(!ftrp->width);}+staticvoidupdate_mismatched_32bit_el0_cpu_features(structcpuinfo_arm64*info,+structcpuinfo_arm64*boot)
Could we s/update/lazy_init/ here?
IIUC this caters for the case where CPU0 doesn't have AArch32 but a
secondary does. That, and the naming looks odd in update_cpu_features()
when we have:
update_mismatched_32bit_el0_cpu_features(...)
update_32bit_cpu_features(...);
Can't we share this with the boot CPU path if we do:
/*
* Initialize the common AArch32 features on the first CPU with AArch32.
*/
static void lazy_init_32bit_el0_cpu_features(struct cpuinfo_arm64 *info,
struct cpuinfo_arm64 *boot)
{
static bool initialised = false;
if (initialised || !id_aa64pfr0_32bit_el0(info->reg_id_aa64pfr0))
return;
boot->aarch32 = info->aarch32;
init_32bit_cpu_features(&boot->aarch32);
initiaised = true;
}
... or is the allow_mismatched_32bit_el0 check necessary for late
hotplug?
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-06-03 12:39:04
On Wed, Jun 02, 2021 at 05:47:01PM +0100, Will Deacon wrote:
In preparation for late initialisation of the "sanitised" AArch32 register
state, move the AArch32 registers out of 'struct cpuinfo' and into their
own struct definition.
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
Makes sense to me; if it's not too painful to change, I'd suggest
`aarch32` rather than `32bit` in the name, but either way:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Mark.
@@ -54,6 +35,29 @@ struct cpuinfo_arm64 {u32reg_mvfr0;u32reg_mvfr1;u32reg_mvfr2;+};++structcpuinfo_arm64{+structcpucpu;+structkobjectkobj;+u32reg_ctr;+u32reg_cntfrq;+u32reg_dczid;+u32reg_midr;+u32reg_revidr;++u64reg_id_aa64dfr0;+u64reg_id_aa64dfr1;+u64reg_id_aa64isar0;+u64reg_id_aa64isar1;+u64reg_id_aa64mmfr0;+u64reg_id_aa64mmfr1;+u64reg_id_aa64mmfr2;+u64reg_id_aa64pfr0;+u64reg_id_aa64pfr1;+u64reg_id_aa64zfr0;++structcpuinfo_32bitaarch32;/* pseudo-ZCR for recording maximum ZCR_EL1 LEN value: */u64reg_zcr;
@@ -863,6 +863,31 @@ static void __init init_cpu_hwcaps_indirect_list(void)staticvoid__initsetup_boot_cpu_capabilities(void);+staticvoid__initinit_32bit_cpu_features(structcpuinfo_32bit*info)+{+init_cpu_ftr_reg(SYS_ID_DFR0_EL1,info->reg_id_dfr0);+init_cpu_ftr_reg(SYS_ID_DFR1_EL1,info->reg_id_dfr1);+init_cpu_ftr_reg(SYS_ID_ISAR0_EL1,info->reg_id_isar0);+init_cpu_ftr_reg(SYS_ID_ISAR1_EL1,info->reg_id_isar1);+init_cpu_ftr_reg(SYS_ID_ISAR2_EL1,info->reg_id_isar2);+init_cpu_ftr_reg(SYS_ID_ISAR3_EL1,info->reg_id_isar3);+init_cpu_ftr_reg(SYS_ID_ISAR4_EL1,info->reg_id_isar4);+init_cpu_ftr_reg(SYS_ID_ISAR5_EL1,info->reg_id_isar5);+init_cpu_ftr_reg(SYS_ID_ISAR6_EL1,info->reg_id_isar6);+init_cpu_ftr_reg(SYS_ID_MMFR0_EL1,info->reg_id_mmfr0);+init_cpu_ftr_reg(SYS_ID_MMFR1_EL1,info->reg_id_mmfr1);+init_cpu_ftr_reg(SYS_ID_MMFR2_EL1,info->reg_id_mmfr2);+init_cpu_ftr_reg(SYS_ID_MMFR3_EL1,info->reg_id_mmfr3);+init_cpu_ftr_reg(SYS_ID_MMFR4_EL1,info->reg_id_mmfr4);+init_cpu_ftr_reg(SYS_ID_MMFR5_EL1,info->reg_id_mmfr5);+init_cpu_ftr_reg(SYS_ID_PFR0_EL1,info->reg_id_pfr0);+init_cpu_ftr_reg(SYS_ID_PFR1_EL1,info->reg_id_pfr1);+init_cpu_ftr_reg(SYS_ID_PFR2_EL1,info->reg_id_pfr2);+init_cpu_ftr_reg(SYS_MVFR0_EL1,info->reg_mvfr0);+init_cpu_ftr_reg(SYS_MVFR1_EL1,info->reg_mvfr1);+init_cpu_ftr_reg(SYS_MVFR2_EL1,info->reg_mvfr2);+}+void__initinit_cpu_features(structcpuinfo_arm64*info){/* Before we start using the tables, make sure it is sorted */
@@ -371,31 +397,8 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)info->reg_id_aa64pfr1=read_cpuid(ID_AA64PFR1_EL1);info->reg_id_aa64zfr0=read_cpuid(ID_AA64ZFR0_EL1);-/* Update the 32bit ID registers only if AArch32 is implemented */-if(id_aa64pfr0_32bit_el0(info->reg_id_aa64pfr0)){-info->reg_id_dfr0=read_cpuid(ID_DFR0_EL1);-info->reg_id_dfr1=read_cpuid(ID_DFR1_EL1);-info->reg_id_isar0=read_cpuid(ID_ISAR0_EL1);-info->reg_id_isar1=read_cpuid(ID_ISAR1_EL1);-info->reg_id_isar2=read_cpuid(ID_ISAR2_EL1);-info->reg_id_isar3=read_cpuid(ID_ISAR3_EL1);-info->reg_id_isar4=read_cpuid(ID_ISAR4_EL1);-info->reg_id_isar5=read_cpuid(ID_ISAR5_EL1);-info->reg_id_isar6=read_cpuid(ID_ISAR6_EL1);-info->reg_id_mmfr0=read_cpuid(ID_MMFR0_EL1);-info->reg_id_mmfr1=read_cpuid(ID_MMFR1_EL1);-info->reg_id_mmfr2=read_cpuid(ID_MMFR2_EL1);-info->reg_id_mmfr3=read_cpuid(ID_MMFR3_EL1);-info->reg_id_mmfr4=read_cpuid(ID_MMFR4_EL1);-info->reg_id_mmfr5=read_cpuid(ID_MMFR5_EL1);-info->reg_id_pfr0=read_cpuid(ID_PFR0_EL1);-info->reg_id_pfr1=read_cpuid(ID_PFR1_EL1);-info->reg_id_pfr2=read_cpuid(ID_PFR2_EL1);--info->reg_mvfr0=read_cpuid(MVFR0_EL1);-info->reg_mvfr1=read_cpuid(MVFR1_EL1);-info->reg_mvfr2=read_cpuid(MVFR2_EL1);-}+if(id_aa64pfr0_32bit_el0(info->reg_id_aa64pfr0))+__cpuinfo_store_cpu_32bit(&info->aarch32);if(IS_ENABLED(CONFIG_ARM64_SVE)&&id_aa64pfr0_sve(info->reg_id_aa64pfr0))
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-06-03 12:59:06
On Wed, Jun 02, 2021 at 05:47:15PM +0100, Will Deacon wrote:
quoted hunk
If we want to support 32-bit applications, then when we identify a CPU
with mismatched 32-bit EL0 support we must ensure that we will always
have an active 32-bit CPU available to us from then on. This is important
for the scheduler, because is_cpu_allowed() will be constrained to 32-bit
CPUs for compat tasks and forced migration due to a hotplug event will
hang if no 32-bit CPUs are available.
On detecting a mismatch, prevent offlining of either the mismatching CPU
if it is 32-bit capable, or find the first active 32-bit capable CPU
otherwise.
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/kernel/cpufeature.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
This is cute, but could we please give it a meaningful name, e.g.
`pinned_cpu` ?
+
struct cpuinfo_arm64 *info = &per_cpu(cpu_data, cpu);
bool cpu_32bit = id_aa64pfr0_32bit_el0(info->reg_id_aa64pfr0);
if (cpu_32bit) {
cpumask_set_cpu(cpu, cpu_32bit_el0_mask);
static_branch_enable_cpuslocked(&arm64_mismatched_32bit_el0);
- setup_elf_hwcaps(compat_elf_hwcaps);
}
+ if (cpumask_test_cpu(0, cpu_32bit_el0_mask) == cpu_32bit)
+ return 0;
+
+ if (lucky_winner >= 0)
+ return 0;
+
+ /*
+ * We've detected a mismatch. We need to keep one of our CPUs with
+ * 32-bit EL0 online so that is_cpu_allowed() doesn't end up rejecting
+ * every CPU in the system for a 32-bit task.
+ */
+ lucky_winner = cpu_32bit ? cpu : cpumask_any_and(cpu_32bit_el0_mask,
+ cpu_active_mask);
+ get_cpu_device(lucky_winner)->offline_disabled = true;
+ setup_elf_hwcaps(compat_elf_hwcaps);
+ pr_info("Asymmetric 32-bit EL0 support detected on CPU %u; CPU hot-unplug disabled on CPU %u\n",
+ cpu, lucky_winner);
return 0;
}
I guess this is going to play havoc with kexec and hibernate. :/
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-06-03 17:24:53
On Thu, Jun 03, 2021 at 01:38:52PM +0100, Mark Rutland wrote:
On Wed, Jun 02, 2021 at 05:47:01PM +0100, Will Deacon wrote:
quoted
In preparation for late initialisation of the "sanitised" AArch32 register
state, move the AArch32 registers out of 'struct cpuinfo' and into their
own struct definition.
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
Makes sense to me; if it's not too painful to change, I'd suggest
`aarch32` rather than `32bit` in the name, but either way:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Thanks. "32bit" is already pervasive in cpufeature.c and we're using arm64
instead of aarch64 in cpuinfo_arm64, so I'll leave this as-is and offer
somebody else the refactoring opportunity ;)
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-06-03 17:41:06
On Thu, Jun 03, 2021 at 01:58:56PM +0100, Mark Rutland wrote:
On Wed, Jun 02, 2021 at 05:47:15PM +0100, Will Deacon wrote:
quoted
If we want to support 32-bit applications, then when we identify a CPU
with mismatched 32-bit EL0 support we must ensure that we will always
have an active 32-bit CPU available to us from then on. This is important
for the scheduler, because is_cpu_allowed() will be constrained to 32-bit
CPUs for compat tasks and forced migration due to a hotplug event will
hang if no 32-bit CPUs are available.
On detecting a mismatch, prevent offlining of either the mismatching CPU
if it is 32-bit capable, or find the first active 32-bit capable CPU
otherwise.
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/kernel/cpufeature.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
This is cute, but could we please give it a meaningful name, e.g.
`pinned_cpu` ?
I really don't see the problem, nor why it's "cute".
Tell you what, I'll add a comment instead:
/*
* The first 32-bit-capable CPU we detected and so can no longer
* be offlined by userspace. -1 indicates we haven't yet onlined
* a 32-bit-capable CPU.
*/
quoted
struct cpuinfo_arm64 *info = &per_cpu(cpu_data, cpu);
bool cpu_32bit = id_aa64pfr0_32bit_el0(info->reg_id_aa64pfr0);
if (cpu_32bit) {
cpumask_set_cpu(cpu, cpu_32bit_el0_mask);
static_branch_enable_cpuslocked(&arm64_mismatched_32bit_el0);
- setup_elf_hwcaps(compat_elf_hwcaps);
}
+ if (cpumask_test_cpu(0, cpu_32bit_el0_mask) == cpu_32bit)
+ return 0;
+
+ if (lucky_winner >= 0)
+ return 0;
+
+ /*
+ * We've detected a mismatch. We need to keep one of our CPUs with
+ * 32-bit EL0 online so that is_cpu_allowed() doesn't end up rejecting
+ * every CPU in the system for a 32-bit task.
+ */
+ lucky_winner = cpu_32bit ? cpu : cpumask_any_and(cpu_32bit_el0_mask,
+ cpu_active_mask);
+ get_cpu_device(lucky_winner)->offline_disabled = true;
+ setup_elf_hwcaps(compat_elf_hwcaps);
+ pr_info("Asymmetric 32-bit EL0 support detected on CPU %u; CPU hot-unplug disabled on CPU %u\n",
+ cpu, lucky_winner);
return 0;
}
I guess this is going to play havoc with kexec and hibernate. :/
The kernel can still offline the CPUs (see the whole freezer mess that I
linked to in the cover letter). What specific havoc are you thinking of?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-06-03 17:44:24
On Thu, Jun 03, 2021 at 01:37:15PM +0100, Mark Rutland wrote:
On Wed, Jun 02, 2021 at 05:47:02PM +0100, Will Deacon wrote:
quoted
When confronted with a mixture of CPUs, some of which support 32-bit
applications and others which don't, we quite sensibly treat the system
as 64-bit only for userspace and prevent execve() of 32-bit binaries.
Unfortunately, some crazy folks have decided to build systems like this
with the intention of running 32-bit applications, so relax our
sanitisation logic to continue to advertise 32-bit support to userspace
on these systems and track the real 32-bit capable cores in a cpumask
instead. For now, the default behaviour remains but will be tied to
a command-line option in a later patch.
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/cpufeature.h | 8 +-
arch/arm64/kernel/cpufeature.c | 114 ++++++++++++++++++++++++----
arch/arm64/tools/cpucaps | 3 +-
3 files changed, 110 insertions(+), 15 deletions(-)
Note that read_sanitised_ftr_reg() has to do a bsearch() to find the
arm64_ftr_reg, so this will make system_32bit_el0_cpumask() a fair
amount more expensive than it needs to be.
I seriously doubt that it matters, but it did come up before and I proposed
a potential solution if it's actually a concern:
https://lore.kernel.org/r/20201202172727.GC29813@willie-the-truck
so if you can show that it's a problem, we can resurrect something like
that.
Can we follow the pattern we have for arm64_ftr_reg_ctrel0, and have a
arm64_ftr_reg_id_aa64pfr0_el1 that we can address directly here?
I mean, clearly its possible, but based on what data?
That said. I reckon this could be much cleaner if we maintained separate
caps:
ARM64_ALL_CPUS_HAVE_32BIT_EL0
ARM64_SOME_CPUS_HAVE_32BIT_EL0
... and allow arm64_mismatched_32bit_el0 to be set dependent on
ARM64_SOME_CPUS_HAVE_32BIT_EL0. With that, this can be simplified to:
static inline bool system_supports_32bit_el0(void)
{
return (cpus_have_const_cap(ARM64_ALL_CPUS_HAVE_32BIT_EL0)) ||
static_branch_unlikely(&arm64_mismatched_32bit_el0))
Something similar was discussed in November last year but this falls
apart with late onlining because its not generally possible to tell whether
you've seen all the CPUs or not.
I'm mostly reluctant to make significant changes based on cosmetic
preferences because testing and debugging this with all the system
combinations is really difficult. Do you see a functional issue with what
I have?
@@ -979,6 +997,22 @@ static void relax_cpu_ftr_reg(u32 sys_id, int field)WARN_ON(!ftrp->width);}+staticvoidupdate_mismatched_32bit_el0_cpu_features(structcpuinfo_arm64*info,+structcpuinfo_arm64*boot)
Could we s/update/lazy_init/ here?
IIUC this caters for the case where CPU0 doesn't have AArch32 but a
secondary does. That, and the naming looks odd in update_cpu_features()
when we have:
update_mismatched_32bit_el0_cpu_features(...)
update_32bit_cpu_features(...);
Can't we share this with the boot CPU path if we do:
/*
* Initialize the common AArch32 features on the first CPU with AArch32.
*/
static void lazy_init_32bit_el0_cpu_features(struct cpuinfo_arm64 *info,
struct cpuinfo_arm64 *boot)
{
static bool initialised = false;
if (initialised || !id_aa64pfr0_32bit_el0(info->reg_id_aa64pfr0))
return;
boot->aarch32 = info->aarch32;
init_32bit_cpu_features(&boot->aarch32);
initiaised = true;
}
... or is the allow_mismatched_32bit_el0 check necessary for late
hotplug?
Interesting. I think this works, but I'm wary that it results in the
32-bit features of a 64-bit-only boot CPU being populated using the first
32-bit-capable CPU even if we're not running with allow_mismatched_32bit_el0
enabled. That feels like setting ourselves up for future bugs. For example,
compat_has_neon() would unexpectedly return true even though 32-bit execve()
would be forbidden.
quoted
+static int __init init_32bit_el0_mask(void)
+{
+ if (!allow_mismatched_32bit_el0)
+ return 0;
+
+ if (!zalloc_cpumask_var(&cpu_32bit_el0_mask, GFP_KERNEL))
+ return -ENOMEM;
+
+ return cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
+ "arm64/mismatched_32bit_el0:online",
+ enable_mismatched_32bit_el0, NULL);
+}
Shouldn't we clear this on a hot-unplug?
No, the mask is intended to show all of the 32-bit CPUs we've seen, so that
userspace can use it to construct 32-bit-capable affinity masks. Having a
race with hot-unplug doesn't help with that and sched_setaffinity() is quite
happy with offline CPUs in the provided mask.
Additionally, the underlying mask is used to implement task_cpu_possible(),
so if we wanted to remove CPUs then we'd need a separate mask just for
sysfs (which _already_ exposes the online mask separately).
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-06-04 09:38:26
On Thu, Jun 03, 2021 at 06:44:14PM +0100, Will Deacon wrote:
On Thu, Jun 03, 2021 at 01:37:15PM +0100, Mark Rutland wrote:
quoted
On Wed, Jun 02, 2021 at 05:47:02PM +0100, Will Deacon wrote:
quoted
When confronted with a mixture of CPUs, some of which support 32-bit
applications and others which don't, we quite sensibly treat the system
as 64-bit only for userspace and prevent execve() of 32-bit binaries.
Unfortunately, some crazy folks have decided to build systems like this
with the intention of running 32-bit applications, so relax our
sanitisation logic to continue to advertise 32-bit support to userspace
on these systems and track the real 32-bit capable cores in a cpumask
instead. For now, the default behaviour remains but will be tied to
a command-line option in a later patch.
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/cpufeature.h | 8 +-
arch/arm64/kernel/cpufeature.c | 114 ++++++++++++++++++++++++----
arch/arm64/tools/cpucaps | 3 +-
3 files changed, 110 insertions(+), 15 deletions(-)
Note that read_sanitised_ftr_reg() has to do a bsearch() to find the
arm64_ftr_reg, so this will make system_32bit_el0_cpumask() a fair
amount more expensive than it needs to be.
I seriously doubt that it matters, but it did come up before and I proposed
a potential solution if it's actually a concern:
https://lore.kernel.org/r/20201202172727.GC29813@willie-the-truck
so if you can show that it's a problem, we can resurrect something like
that.
I'm happy to leave that for future. I raised this because elsewhere this
is an issue when we need to avoid instrumentation; if that's not a
concern here on any path then I am not aware of a functional issue.
quoted
Can we follow the pattern we have for arm64_ftr_reg_ctrel0, and have a
arm64_ftr_reg_id_aa64pfr0_el1 that we can address directly here?
I mean, clearly its possible, but based on what data?
quoted
That said. I reckon this could be much cleaner if we maintained separate
caps:
ARM64_ALL_CPUS_HAVE_32BIT_EL0
ARM64_SOME_CPUS_HAVE_32BIT_EL0
... and allow arm64_mismatched_32bit_el0 to be set dependent on
ARM64_SOME_CPUS_HAVE_32BIT_EL0. With that, this can be simplified to:
static inline bool system_supports_32bit_el0(void)
{
return (cpus_have_const_cap(ARM64_ALL_CPUS_HAVE_32BIT_EL0)) ||
static_branch_unlikely(&arm64_mismatched_32bit_el0))
Something similar was discussed in November last year but this falls
apart with late onlining because its not generally possible to tell whether
you've seen all the CPUs or not.
Ah; is that for when your boot CPU set is all AArch32-capable, but a
late-onlined CPU is not?
I assume that we require at least one of the set of boot CPUs to be
AArch32 cpable, and don't settle the compat hwcaps after userspace has
started.
I'm mostly reluctant to make significant changes based on cosmetic
preferences because testing and debugging this with all the system
combinations is really difficult.
Can't we share this with the boot CPU path if we do:
/*
* Initialize the common AArch32 features on the first CPU with AArch32.
*/
static void lazy_init_32bit_el0_cpu_features(struct cpuinfo_arm64 *info,
struct cpuinfo_arm64 *boot)
{
static bool initialised = false;
if (initialised || !id_aa64pfr0_32bit_el0(info->reg_id_aa64pfr0))
return;
boot->aarch32 = info->aarch32;
init_32bit_cpu_features(&boot->aarch32);
initiaised = true;
}
... or is the allow_mismatched_32bit_el0 check necessary for late
hotplug?
Interesting. I think this works, but I'm wary that it results in the
32-bit features of a 64-bit-only boot CPU being populated using the first
32-bit-capable CPU even if we're not running with allow_mismatched_32bit_el0
enabled. That feels like setting ourselves up for future bugs. For example,
compat_has_neon() would unexpectedly return true even though 32-bit execve()
would be forbidden.
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-06-04 09:49:38
On Thu, Jun 03, 2021 at 06:40:57PM +0100, Will Deacon wrote:
On Thu, Jun 03, 2021 at 01:58:56PM +0100, Mark Rutland wrote:
quoted
On Wed, Jun 02, 2021 at 05:47:15PM +0100, Will Deacon wrote:
quoted
If we want to support 32-bit applications, then when we identify a CPU
with mismatched 32-bit EL0 support we must ensure that we will always
have an active 32-bit CPU available to us from then on. This is important
for the scheduler, because is_cpu_allowed() will be constrained to 32-bit
CPUs for compat tasks and forced migration due to a hotplug event will
hang if no 32-bit CPUs are available.
On detecting a mismatch, prevent offlining of either the mismatching CPU
if it is 32-bit capable, or find the first active 32-bit capable CPU
otherwise.
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/kernel/cpufeature.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
This is cute, but could we please give it a meaningful name, e.g.
`pinned_cpu` ?
I really don't see the problem, nor why it's "cute".
Tell you what, I'll add a comment instead:
/*
* The first 32-bit-capable CPU we detected and so can no longer
* be offlined by userspace. -1 indicates we haven't yet onlined
* a 32-bit-capable CPU.
*/
Thanks for the comment; that's helpful.
However, my concern here is that when we inevitably have to discuss this
with others in future, "lucky winner" is jarring (and also unclear to
those where English is not their native language). For clarity, it would
be really nice to use a term like "cpu", "chosen_cpu", "pinned_cpu",
etc.
However, you're the maintainer; choose what you think is appropriate.
quoted
quoted
struct cpuinfo_arm64 *info = &per_cpu(cpu_data, cpu);
bool cpu_32bit = id_aa64pfr0_32bit_el0(info->reg_id_aa64pfr0);
if (cpu_32bit) {
cpumask_set_cpu(cpu, cpu_32bit_el0_mask);
static_branch_enable_cpuslocked(&arm64_mismatched_32bit_el0);
- setup_elf_hwcaps(compat_elf_hwcaps);
}
+ if (cpumask_test_cpu(0, cpu_32bit_el0_mask) == cpu_32bit)
+ return 0;
+
+ if (lucky_winner >= 0)
+ return 0;
+
+ /*
+ * We've detected a mismatch. We need to keep one of our CPUs with
+ * 32-bit EL0 online so that is_cpu_allowed() doesn't end up rejecting
+ * every CPU in the system for a 32-bit task.
+ */
+ lucky_winner = cpu_32bit ? cpu : cpumask_any_and(cpu_32bit_el0_mask,
+ cpu_active_mask);
+ get_cpu_device(lucky_winner)->offline_disabled = true;
+ setup_elf_hwcaps(compat_elf_hwcaps);
+ pr_info("Asymmetric 32-bit EL0 support detected on CPU %u; CPU hot-unplug disabled on CPU %u\n",
+ cpu, lucky_winner);
return 0;
}
I guess this is going to play havoc with kexec and hibernate. :/
The kernel can still offline the CPUs (see the whole freezer mess that I
linked to in the cover letter). What specific havoc are you thinking of?
Ah. If this is just inhibiting userspace-driven offlining, that sounds
fine.
For kexec, I was concerned that either this would inhibit kexec, or
smp_shutdown_nonboot_cpus() would fail to offline the pinned CPU, and
that'd trigger a BUG(), which would be unfortunate.
For hibernate, the equivalent is freeze_secondary_cpus(), which I guess
is dealt with by the freezer bits you mention.
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Note that read_sanitised_ftr_reg() has to do a bsearch() to find the
arm64_ftr_reg, so this will make system_32bit_el0_cpumask() a fair
amount more expensive than it needs to be.
I seriously doubt that it matters, but it did come up before and I proposed
a potential solution if it's actually a concern:
https://lore.kernel.org/r/20201202172727.GC29813@willie-the-truck
so if you can show that it's a problem, we can resurrect something like
that.
I'm happy to leave that for future. I raised this because elsewhere this
is an issue when we need to avoid instrumentation; if that's not a
concern here on any path then I am not aware of a functional issue.
I can't think of a reason why instrumentation would be an issue for any of
the current callers, but that's a good point to bear in mind.
quoted
quoted
That said. I reckon this could be much cleaner if we maintained separate
caps:
ARM64_ALL_CPUS_HAVE_32BIT_EL0
ARM64_SOME_CPUS_HAVE_32BIT_EL0
... and allow arm64_mismatched_32bit_el0 to be set dependent on
ARM64_SOME_CPUS_HAVE_32BIT_EL0. With that, this can be simplified to:
static inline bool system_supports_32bit_el0(void)
{
return (cpus_have_const_cap(ARM64_ALL_CPUS_HAVE_32BIT_EL0)) ||
static_branch_unlikely(&arm64_mismatched_32bit_el0))
Something similar was discussed in November last year but this falls
apart with late onlining because its not generally possible to tell whether
you've seen all the CPUs or not.
Ah; is that for when your boot CPU set is all AArch32-capable, but a
late-onlined CPU is not?
I assume that we require at least one of the set of boot CPUs to be
AArch32 cpable, and don't settle the compat hwcaps after userspace has
started.
Heh, you assume wrong :)
When we allow the mismatch, then we do actually defer initialisation of
the compat hwcaps until we see a 32-bit CPU. That's fine, as they won't
be visible to userspace until then anyway (PER_LINUX32 is unavailable).
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
That said. I reckon this could be much cleaner if we maintained separate
caps:
ARM64_ALL_CPUS_HAVE_32BIT_EL0
ARM64_SOME_CPUS_HAVE_32BIT_EL0
... and allow arm64_mismatched_32bit_el0 to be set dependent on
ARM64_SOME_CPUS_HAVE_32BIT_EL0. With that, this can be simplified to:
static inline bool system_supports_32bit_el0(void)
{
return (cpus_have_const_cap(ARM64_ALL_CPUS_HAVE_32BIT_EL0)) ||
static_branch_unlikely(&arm64_mismatched_32bit_el0))
Something similar was discussed in November last year but this falls
apart with late onlining because its not generally possible to tell whether
you've seen all the CPUs or not.
Ah; is that for when your boot CPU set is all AArch32-capable, but a
late-onlined CPU is not?
I assume that we require at least one of the set of boot CPUs to be
AArch32 cpable, and don't settle the compat hwcaps after userspace has
started.
Heh, you assume wrong :)
When we allow the mismatch, then we do actually defer initialisation of
the compat hwcaps until we see a 32-bit CPU. That's fine, as they won't
be visible to userspace until then anyway (PER_LINUX32 is unavailable).
That sounds quite scary, to me, though I don't have a concrete problem
to hand. :/
Do we really need to support initializing that so late? For all other
caps we've settled things when the boot CPUs come up, and it's
unfortunate to have to treat this differently.
I'll go see if there's anything that's liable to break today.
Thanks,
Mark
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, Jun 03, 2021 at 06:40:57PM +0100, Will Deacon wrote:
quoted
On Thu, Jun 03, 2021 at 01:58:56PM +0100, Mark Rutland wrote:
quoted
On Wed, Jun 02, 2021 at 05:47:15PM +0100, Will Deacon wrote:
quoted
If we want to support 32-bit applications, then when we identify a CPU
with mismatched 32-bit EL0 support we must ensure that we will always
have an active 32-bit CPU available to us from then on. This is important
for the scheduler, because is_cpu_allowed() will be constrained to 32-bit
CPUs for compat tasks and forced migration due to a hotplug event will
hang if no 32-bit CPUs are available.
On detecting a mismatch, prevent offlining of either the mismatching CPU
if it is 32-bit capable, or find the first active 32-bit capable CPU
otherwise.
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/kernel/cpufeature.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
This is cute, but could we please give it a meaningful name, e.g.
`pinned_cpu` ?
I really don't see the problem, nor why it's "cute".
Tell you what, I'll add a comment instead:
/*
* The first 32-bit-capable CPU we detected and so can no longer
* be offlined by userspace. -1 indicates we haven't yet onlined
* a 32-bit-capable CPU.
*/
Thanks for the comment; that's helpful.
However, my concern here is that when we inevitably have to discuss this
with others in future, "lucky winner" is jarring (and also unclear to
those where English is not their native language). For clarity, it would
be really nice to use a term like "cpu", "chosen_cpu", "pinned_cpu",
etc.
However, you're the maintainer; choose what you think is appropriate.
quoted
quoted
quoted
struct cpuinfo_arm64 *info = &per_cpu(cpu_data, cpu);
bool cpu_32bit = id_aa64pfr0_32bit_el0(info->reg_id_aa64pfr0);
if (cpu_32bit) {
cpumask_set_cpu(cpu, cpu_32bit_el0_mask);
static_branch_enable_cpuslocked(&arm64_mismatched_32bit_el0);
- setup_elf_hwcaps(compat_elf_hwcaps);
}
+ if (cpumask_test_cpu(0, cpu_32bit_el0_mask) == cpu_32bit)
+ return 0;
+
+ if (lucky_winner >= 0)
+ return 0;
+
+ /*
+ * We've detected a mismatch. We need to keep one of our CPUs with
+ * 32-bit EL0 online so that is_cpu_allowed() doesn't end up rejecting
+ * every CPU in the system for a 32-bit task.
+ */
+ lucky_winner = cpu_32bit ? cpu : cpumask_any_and(cpu_32bit_el0_mask,
+ cpu_active_mask);
+ get_cpu_device(lucky_winner)->offline_disabled = true;
+ setup_elf_hwcaps(compat_elf_hwcaps);
+ pr_info("Asymmetric 32-bit EL0 support detected on CPU %u; CPU hot-unplug disabled on CPU %u\n",
+ cpu, lucky_winner);
return 0;
}
I guess this is going to play havoc with kexec and hibernate. :/
The kernel can still offline the CPUs (see the whole freezer mess that I
linked to in the cover letter). What specific havoc are you thinking of?
Ah. If this is just inhibiting userspace-driven offlining, that sounds
fine.
For kexec, I was concerned that either this would inhibit kexec, or
smp_shutdown_nonboot_cpus() would fail to offline the pinned CPU, and
that'd trigger a BUG(), which would be unfortunate.
For hibernate, the equivalent is freeze_secondary_cpus(), which I guess
is dealt with by the freezer bits you mention.
()->offline_disabled will only block offline requests performed by
device_offline(). kexec, hibernate, suspend/resume use cpu_online/offline()
directly so won't be impacted by that. I have sent patches that make
cpu_online/offline() 'private' and not used or exported outside of cpu
subsystem and the odd support function for arch code. All other users use
device_offline() or add/remove_cpu() now.
I have made sure to test kexec, suspend to disk and ram in my older similar
implementation in the past. So we should be good.
Cheers
--
Qais Yousef
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
That said. I reckon this could be much cleaner if we maintained separate
caps:
ARM64_ALL_CPUS_HAVE_32BIT_EL0
ARM64_SOME_CPUS_HAVE_32BIT_EL0
... and allow arm64_mismatched_32bit_el0 to be set dependent on
ARM64_SOME_CPUS_HAVE_32BIT_EL0. With that, this can be simplified to:
static inline bool system_supports_32bit_el0(void)
{
return (cpus_have_const_cap(ARM64_ALL_CPUS_HAVE_32BIT_EL0)) ||
static_branch_unlikely(&arm64_mismatched_32bit_el0))
Something similar was discussed in November last year but this falls
apart with late onlining because its not generally possible to tell whether
you've seen all the CPUs or not.
Ah; is that for when your boot CPU set is all AArch32-capable, but a
late-onlined CPU is not?
I assume that we require at least one of the set of boot CPUs to be
AArch32 cpable, and don't settle the compat hwcaps after userspace has
started.
Heh, you assume wrong :)
When we allow the mismatch, then we do actually defer initialisation of
the compat hwcaps until we see a 32-bit CPU. That's fine, as they won't
be visible to userspace until then anyway (PER_LINUX32 is unavailable).
That sounds quite scary, to me, though I don't have a concrete problem
to hand. :/
Do we really need to support initializing that so late? For all other
caps we've settled things when the boot CPUs come up, and it's
unfortunate to have to treat this differently.
I think it's the nature of the beast, unfortunately. Since we're talking
about multiple generations of SoCs rather than just one oddball design,
then placing artificial restrictions on the boot CPUs doesn't feel like
it will last very long.
I'll go see if there's anything that's liable to break today.
Asymmetric systems may not offer the same level of userspace ISA support
across all CPUs, meaning that some applications cannot be executed by
some CPUs. As a concrete example, upcoming arm64 big.LITTLE designs do
not feature support for 32-bit applications on both clusters.
On such a system, we must take care not to migrate a task to an
unsupported CPU when forcefully moving tasks in select_fallback_rq()
in response to a CPU hot-unplug operation.
Introduce a task_cpu_possible_mask() hook which, given a task argument,
allows an architecture to return a cpumask of CPUs that are capable of
executing that task. The default implementation returns the
cpu_possible_mask, since sane machines do not suffer from per-cpu ISA
limitations that affect scheduling. The new mask is used when selecting
the fallback runqueue as a last resort before forcing a migration to the
first active CPU.
Nit: Some uses of this mask (cpu_is_allowed(), __set_cpus_allowed_ptr())
don't apply to kthreads. This makes sense for the 32-bit@EL0 faff, but it
wouldn't hurt to point this out somewhere IMO.
Also, that's an odd place for the definitions, but IIRC there isn't a much
better choice.
Reviewed-by: Valentin Schneider <redacted>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Since the task will still go through the is_cpu_allowed() loop in
select_fallback_rq() after this, is the subset check actually required
here?
It would have more merit if cpuset_cpus_allowed_fallback() returned whether
it actually changed the allowed mask or not, in which case we could branch
either to the is_cpu_allowed() loop (as we do unconditionally now), or to
the 'state == possible' switch case.
Asymmetric systems may not offer the same level of userspace ISA support
across all CPUs, meaning that some applications cannot be executed by
some CPUs. As a concrete example, upcoming arm64 big.LITTLE designs do
not feature support for 32-bit applications on both clusters.
Modify guarantee_online_cpus() to take task_cpu_possible_mask() into
account when trying to find a suitable set of online CPUs for a given
task. This will avoid passing an invalid mask to set_cpus_allowed_ptr()
during ->attach() and will subsequently allow the cpuset hierarchy to be
taken into account when forcefully overriding the affinity mask for a
task which requires migration to a compatible CPU.
Cc: Li Zefan <redacted>
Cc: Tejun Heo <tj@kernel.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Signed-off-by: Will Deacon <will@kernel.org>
Reject explicit requests to change the affinity mask of a task via
set_cpus_allowed_ptr() if the requested mask is not a subset of the
mask returned by task_cpu_possible_mask(). This ensures that the
'cpus_mask' for a given task cannot contain CPUs which are incapable of
executing it, except in cases where the affinity is forced.
Reviewed-by: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
One comment/observation below, but regardless:
Reviewed-by: Valentin Schneider <redacted>
@@ -2347,15 +2347,17 @@ static int __set_cpus_allowed_ptr(struct task_struct *p,u32flags){conststructcpumask*cpu_valid_mask=cpu_active_mask;+conststructcpumask*cpu_allowed_mask=task_cpu_possible_mask(p);unsignedintdest_cpu;structrq_flagsrf;structrq*rq;intret=0;+boolkthread=p->flags&PF_KTHREAD;rq=task_rq_lock(p,&rf);update_rq_clock(rq);-if(p->flags&PF_KTHREAD||is_migration_disabled(p)){+if(kthread||is_migration_disabled(p)){/**Kernelthreadsareallowedononline&&!activeCPUs,*however,duringcpu-hot-unplug,eventhesemightgetpushed
@@ -2369,6 +2371,11 @@ static int __set_cpus_allowed_ptr(struct task_struct *p,cpu_valid_mask=cpu_online_mask;}+if(!kthread&&!cpumask_subset(new_mask,cpu_allowed_mask)){+ret=-EINVAL;+gotoout;+}+
IIUC this wouldn't be required if guarantee_online_cpus() couldn't build a
mask that extends beyond task_cpu_possible_mask(p): if the new mask doesn't
intersect with that possible mask, it means we're carrying an empty cpumask
and the cpumask_any_and_distribute() below would return nr_cpu_ids, so we'd
bail with -EINVAL.
I don't really see a way around it though due to the expectations behind
guarantee_online_cpus() :/
/*
* Must re-check here, to close a race against __kthread_bind(),
* sched_setaffinity() is not guaranteed to observe the flag.
--
2.32.0.rc0.204.g9fa02ecfa5-goog
In preparation for saving and restoring the user-requested CPU affinity
mask of a task, add a new cpumask_t pointer to 'struct task_struct'.
If the pointer is non-NULL, then the mask is copied across fork() and
freed on task exit.
Signed-off-by: Will Deacon <will@kernel.org>
In preparation for replaying user affinity requests using a saved mask,
split sched_setaffinity() up so that the initial task lookup and
security checks are only performed when the request is coming directly
from userspace.
Signed-off-by: Will Deacon <will@kernel.org>
+static int restrict_cpus_allowed_ptr(struct task_struct *p,
+ struct cpumask *new_mask,
+ const struct cpumask *subset_mask)
+{
+ struct rq_flags rf;
+ struct rq *rq;
+ int err;
+ struct cpumask *user_mask = NULL;
+
+ if (!p->user_cpus_ptr) {
+ user_mask = kmalloc(cpumask_size(), GFP_KERNEL);
+
+ if (!user_mask)
+ return -ENOMEM;
+ }
+
+ rq = task_rq_lock(p, &rf);
+
+ /*
+ * Forcefully restricting the affinity of a deadline task is
+ * likely to cause problems, so fail and noisily override the
+ * mask entirely.
+ */
+ if (task_has_dl_policy(p) && dl_bandwidth_enabled()) {
+ err = -EPERM;
+ goto err_unlock;
+ }
+
+ if (!cpumask_and(new_mask, &p->cpus_mask, subset_mask)) {
+ err = -EINVAL;
+ goto err_unlock;
+ }
+
+ /*
+ * We're about to butcher the task affinity, so keep track of what
+ * the user asked for in case we're able to restore it later on.
+ */
+ if (user_mask) {
+ cpumask_copy(user_mask, p->cpus_ptr);
+ p->user_cpus_ptr = user_mask;
+ }
+
Shouldn't that be done before any of the bailouts above, so we can
potentially restore the mask even if we end up forcefully expanding the
affinity?
+ return __set_cpus_allowed_ptr_locked(p, new_mask, 0, rq, &rf);
+
+err_unlock:
+ task_rq_unlock(rq, p, &rf);
+ kfree(user_mask);
+ return err;
+}
+
+/*
+ * Restrict the CPU affinity of task @p so that it is a subset of
+ * task_cpu_possible_mask() and point @p->user_cpu_ptr to a copy of the
+ * old affinity mask. If the resulting mask is empty, we warn and walk
+ * up the cpuset hierarchy until we find a suitable mask.
+ */
+void force_compatible_cpus_allowed_ptr(struct task_struct *p)
+{
+ cpumask_var_t new_mask;
+ const struct cpumask *override_mask = task_cpu_possible_mask(p);
+
+ alloc_cpumask_var(&new_mask, GFP_KERNEL);
+
+ /*
+ * __migrate_task() can fail silently in the face of concurrent
+ * offlining of the chosen destination CPU, so take the hotplug
+ * lock to ensure that the migration succeeds.
+ */
+ cpus_read_lock();
+ if (!cpumask_available(new_mask))
+ goto out_set_mask;
+
+ if (!restrict_cpus_allowed_ptr(p, new_mask, override_mask))
+ goto out_free_mask;
+
+ /*
+ * We failed to find a valid subset of the affinity mask for the
+ * task, so override it based on its cpuset hierarchy.
+ */
+ cpuset_cpus_allowed(p, new_mask);
+ override_mask = new_mask;
+
+out_set_mask:
+ if (printk_ratelimit()) {
+ printk_deferred("Overriding affinity for process %d (%s) to CPUs %*pbl\n",
+ task_pid_nr(p), p->comm,
+ cpumask_pr_args(override_mask));
+ }
+
+ WARN_ON(set_cpus_allowed_ptr(p, override_mask));
+out_free_mask:
+ cpus_read_unlock();
+ free_cpumask_var(new_mask);
+}
+
+static int
+__sched_setaffinity(struct task_struct *p, const struct cpumask *mask);
+
+/*
+ * Restore the affinity of a task @p which was previously restricted by a
+ * call to force_compatible_cpus_allowed_ptr(). This will clear (and free)
+ * @p->user_cpus_ptr.
+ */
+void relax_compatible_cpus_allowed_ptr(struct task_struct *p)
+{
+ unsigned long flags;
+ struct cpumask *mask = p->user_cpus_ptr;
+
+ /*
+ * Try to restore the old affinity mask. If this fails, then
+ * we free the mask explicitly to avoid it being inherited across
+ * a subsequent fork().
+ */
+ if (!mask || !__sched_setaffinity(p, mask))
+ return;
+
+ raw_spin_lock_irqsave(&p->pi_lock, flags);
+ release_user_cpus_ptr(p);
+ raw_spin_unlock_irqrestore(&p->pi_lock, flags);
AFAICT an affinity change can happen between __sched_setaffinity() and
reacquiring the ->pi_lock. Right now this can't be another
force_compatible_cpus_allowed_ptr() because this is only driven by
arch_setup_new_exec() against current, so we should be fine, but here be
dragons.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-06-07 17:04:15
On Fri, Jun 04, 2021 at 06:10:46PM +0100, Valentin Schneider wrote:
On 02/06/21 17:47, Will Deacon wrote:
quoted
Asymmetric systems may not offer the same level of userspace ISA support
across all CPUs, meaning that some applications cannot be executed by
some CPUs. As a concrete example, upcoming arm64 big.LITTLE designs do
not feature support for 32-bit applications on both clusters.
On such a system, we must take care not to migrate a task to an
unsupported CPU when forcefully moving tasks in select_fallback_rq()
in response to a CPU hot-unplug operation.
Introduce a task_cpu_possible_mask() hook which, given a task argument,
allows an architecture to return a cpumask of CPUs that are capable of
executing that task. The default implementation returns the
cpu_possible_mask, since sane machines do not suffer from per-cpu ISA
limitations that affect scheduling. The new mask is used when selecting
the fallback runqueue as a last resort before forcing a migration to the
first active CPU.
Nit: Some uses of this mask (cpu_is_allowed(), __set_cpus_allowed_ptr())
don't apply to kthreads. This makes sense for the 32-bit@EL0 faff, but it
wouldn't hurt to point this out somewhere IMO.
That's a good point: even after these patches, we still assume the kernel
(and therefore kthreads) can run on all CPUs. I'll expand the comment.
Also, that's an odd place for the definitions, but IIRC there isn't a much
better choice.
Short of adding a new header just for this, I couldn't find anything, no.
Since the task will still go through the is_cpu_allowed() loop in
select_fallback_rq() after this, is the subset check actually required
here?
Yes, I think it's needed. do_set_cpus_allowed() doesn't do any checking
against the task_cpu_possible_mask, so if we returned to
select_fallback_rq() with a mask containing a mixture of 32-bit-capable and
64-bit-only CPUs then we'd end up setting an affinity mask for a 32-bit
task which contains 64-bit-only cores.
It would have more merit if cpuset_cpus_allowed_fallback() returned whether
it actually changed the allowed mask or not, in which case we could branch
either to the is_cpu_allowed() loop (as we do unconditionally now), or to
the 'state == possible' switch case.
I think this is a cleanup, so I can include it as a separate patch (see
below).
Will
--->8
@@ -2951,8 +2951,7 @@ static int select_fallback_rq(int cpu, struct task_struct *p)/* No more Mr. Nice Guy. */switch(state){casecpuset:-if(IS_ENABLED(CONFIG_CPUSETS)){-cpuset_cpus_allowed_fallback(p);+if(cpuset_cpus_allowed_fallback(p)){state=possible;break;}
From: Will Deacon <will@kernel.org> Date: 2021-06-07 22:44:04
On Fri, Jun 04, 2021 at 06:11:52PM +0100, Valentin Schneider wrote:
On 02/06/21 17:47, Will Deacon wrote:
quoted
Reject explicit requests to change the affinity mask of a task via
set_cpus_allowed_ptr() if the requested mask is not a subset of the
mask returned by task_cpu_possible_mask(). This ensures that the
'cpus_mask' for a given task cannot contain CPUs which are incapable of
executing it, except in cases where the affinity is forced.
Reviewed-by: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
One comment/observation below, but regardless:
Reviewed-by: Valentin Schneider <redacted>
@@ -2347,15 +2347,17 @@ static int __set_cpus_allowed_ptr(struct task_struct *p,u32flags){conststructcpumask*cpu_valid_mask=cpu_active_mask;+conststructcpumask*cpu_allowed_mask=task_cpu_possible_mask(p);unsignedintdest_cpu;structrq_flagsrf;structrq*rq;intret=0;+boolkthread=p->flags&PF_KTHREAD;rq=task_rq_lock(p,&rf);update_rq_clock(rq);-if(p->flags&PF_KTHREAD||is_migration_disabled(p)){+if(kthread||is_migration_disabled(p)){/**Kernelthreadsareallowedononline&&!activeCPUs,*however,duringcpu-hot-unplug,eventhesemightgetpushed
@@ -2369,6 +2371,11 @@ static int __set_cpus_allowed_ptr(struct task_struct *p,cpu_valid_mask=cpu_online_mask;}+if(!kthread&&!cpumask_subset(new_mask,cpu_allowed_mask)){+ret=-EINVAL;+gotoout;+}+
IIUC this wouldn't be required if guarantee_online_cpus() couldn't build a
mask that extends beyond task_cpu_possible_mask(p): if the new mask doesn't
intersect with that possible mask, it means we're carrying an empty cpumask
and the cpumask_any_and_distribute() below would return nr_cpu_ids, so we'd
bail with -EINVAL.
I don't really see a way around it though due to the expectations behind
guarantee_online_cpus() :/
Mostly agreed. I started out hacking SCA and only then started knocking the
callers on the head. However, given how many callers there are for this
thing, I'm much more comfortable having the mask check there to ensure that
we return an error if the requested mask contains CPUs on which we're unable
to run, and yes, guarantee_online_cpus() is one such caller.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-06-07 22:52:11
On Fri, Jun 04, 2021 at 06:12:32PM +0100, Valentin Schneider wrote:
On 02/06/21 17:47, Will Deacon wrote:
quoted
+static int restrict_cpus_allowed_ptr(struct task_struct *p,
+ struct cpumask *new_mask,
+ const struct cpumask *subset_mask)
+{
+ struct rq_flags rf;
+ struct rq *rq;
+ int err;
+ struct cpumask *user_mask = NULL;
+
+ if (!p->user_cpus_ptr) {
+ user_mask = kmalloc(cpumask_size(), GFP_KERNEL);
+
+ if (!user_mask)
+ return -ENOMEM;
+ }
+
+ rq = task_rq_lock(p, &rf);
+
+ /*
+ * Forcefully restricting the affinity of a deadline task is
+ * likely to cause problems, so fail and noisily override the
+ * mask entirely.
+ */
+ if (task_has_dl_policy(p) && dl_bandwidth_enabled()) {
+ err = -EPERM;
+ goto err_unlock;
+ }
+
+ if (!cpumask_and(new_mask, &p->cpus_mask, subset_mask)) {
+ err = -EINVAL;
+ goto err_unlock;
+ }
+
+ /*
+ * We're about to butcher the task affinity, so keep track of what
+ * the user asked for in case we're able to restore it later on.
+ */
+ if (user_mask) {
+ cpumask_copy(user_mask, p->cpus_ptr);
+ p->user_cpus_ptr = user_mask;
+ }
+
Shouldn't that be done before any of the bailouts above, so we can
potentially restore the mask even if we end up forcefully expanding the
affinity?
I don't think so. I deliberately only track the old mask if we've managed
to take a subset for the 32-bit task. If we end up having to override the
mask entirely, then I treat it the same way as an explicit affinity change
(only with a warning printed) and don't then try to restore the old mask --
it feels like we'd be overriding the affinity twice if we tried to do that.
quoted
+ return __set_cpus_allowed_ptr_locked(p, new_mask, 0, rq, &rf);
+
+err_unlock:
+ task_rq_unlock(rq, p, &rf);
+ kfree(user_mask);
+ return err;
+}
+
+/*
+ * Restrict the CPU affinity of task @p so that it is a subset of
+ * task_cpu_possible_mask() and point @p->user_cpu_ptr to a copy of the
+ * old affinity mask. If the resulting mask is empty, we warn and walk
+ * up the cpuset hierarchy until we find a suitable mask.
+ */
+void force_compatible_cpus_allowed_ptr(struct task_struct *p)
+{
+ cpumask_var_t new_mask;
+ const struct cpumask *override_mask = task_cpu_possible_mask(p);
+
+ alloc_cpumask_var(&new_mask, GFP_KERNEL);
+
+ /*
+ * __migrate_task() can fail silently in the face of concurrent
+ * offlining of the chosen destination CPU, so take the hotplug
+ * lock to ensure that the migration succeeds.
+ */
+ cpus_read_lock();
Agreed, if that patch does what it says on the tin ;)
I need to digest your reply to me, as this is mind-bending stuff.
quoted
+static int
+__sched_setaffinity(struct task_struct *p, const struct cpumask *mask);
+
+/*
+ * Restore the affinity of a task @p which was previously restricted by a
+ * call to force_compatible_cpus_allowed_ptr(). This will clear (and free)
+ * @p->user_cpus_ptr.
+ */
+void relax_compatible_cpus_allowed_ptr(struct task_struct *p)
+{
+ unsigned long flags;
+ struct cpumask *mask = p->user_cpus_ptr;
+
+ /*
+ * Try to restore the old affinity mask. If this fails, then
+ * we free the mask explicitly to avoid it being inherited across
+ * a subsequent fork().
+ */
+ if (!mask || !__sched_setaffinity(p, mask))
+ return;
+
+ raw_spin_lock_irqsave(&p->pi_lock, flags);
+ release_user_cpus_ptr(p);
+ raw_spin_unlock_irqrestore(&p->pi_lock, flags);
AFAICT an affinity change can happen between __sched_setaffinity() and
reacquiring the ->pi_lock. Right now this can't be another
force_compatible_cpus_allowed_ptr() because this is only driven by
arch_setup_new_exec() against current, so we should be fine, but here be
dragons.
That's a good point. I'll add a comment for now, since I'm not sure who
else might end up using this in future. Generally it's pretty agnostic to
how it's being used, but we're certainly relying on the serialisation of
restrict/relax calls.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Since the task will still go through the is_cpu_allowed() loop in
select_fallback_rq() after this, is the subset check actually required
here?
Yes, I think it's needed. do_set_cpus_allowed() doesn't do any checking
against the task_cpu_possible_mask, so if we returned to
select_fallback_rq() with a mask containing a mixture of 32-bit-capable and
64-bit-only CPUs then we'd end up setting an affinity mask for a 32-bit
task which contains 64-bit-only cores.
On Fri, Jun 04, 2021 at 06:12:32PM +0100, Valentin Schneider wrote:
quoted
On 02/06/21 17:47, Will Deacon wrote:
quoted
+ /*
+ * Forcefully restricting the affinity of a deadline task is
+ * likely to cause problems, so fail and noisily override the
+ * mask entirely.
+ */
+ if (task_has_dl_policy(p) && dl_bandwidth_enabled()) {
+ err = -EPERM;
+ goto err_unlock;
+ }
+
+ if (!cpumask_and(new_mask, &p->cpus_mask, subset_mask)) {
+ err = -EINVAL;
+ goto err_unlock;
+ }
+
+ /*
+ * We're about to butcher the task affinity, so keep track of what
+ * the user asked for in case we're able to restore it later on.
+ */
+ if (user_mask) {
+ cpumask_copy(user_mask, p->cpus_ptr);
+ p->user_cpus_ptr = user_mask;
+ }
+
Shouldn't that be done before any of the bailouts above, so we can
potentially restore the mask even if we end up forcefully expanding the
affinity?
I don't think so. I deliberately only track the old mask if we've managed
to take a subset for the 32-bit task. If we end up having to override the
mask entirely, then I treat it the same way as an explicit affinity change
(only with a warning printed) and don't then try to restore the old mask --
it feels like we'd be overriding the affinity twice if we tried to do that.