| Message ID | 20201126155421.14901-4-dbrazdil@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Opt-in always-on nVHE hypervisor | expand |
On Thu, Nov 26, 2020 at 03:54:01PM +0000, David Brazdil wrote: > CPU index should never be negative. Change the signature of > (set_)cpu_logical_map to take an unsigned int. > > Signed-off-by: David Brazdil <dbrazdil@google.com> Is there a function problem here, or is this just cleanup from inspection? Core code including the cpuhp_*() callbacks uses an int, so if there's a strong justification to change this, it suggests there's some treewide cleanup that should be done. I don't have strong feelings on the matter, but I'd like to understand the rationale. Thanks, Mark. > --- > arch/arm64/include/asm/smp.h | 4 ++-- > arch/arm64/kernel/setup.c | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h > index 2e7f529ec5a6..bcb01ca15325 100644 > --- a/arch/arm64/include/asm/smp.h > +++ b/arch/arm64/include/asm/smp.h > @@ -46,9 +46,9 @@ DECLARE_PER_CPU_READ_MOSTLY(int, cpu_number); > * Logical CPU mapping. > */ > extern u64 __cpu_logical_map[NR_CPUS]; > -extern u64 cpu_logical_map(int cpu); > +extern u64 cpu_logical_map(unsigned int cpu); > > -static inline void set_cpu_logical_map(int cpu, u64 hwid) > +static inline void set_cpu_logical_map(unsigned int cpu, u64 hwid) > { > __cpu_logical_map[cpu] = hwid; > } > diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c > index 133257ffd859..2f2973bc67c7 100644 > --- a/arch/arm64/kernel/setup.c > +++ b/arch/arm64/kernel/setup.c > @@ -276,7 +276,7 @@ arch_initcall(reserve_memblock_reserved_regions); > > u64 __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = INVALID_HWID }; > > -u64 cpu_logical_map(int cpu) > +u64 cpu_logical_map(unsigned int cpu) > { > return __cpu_logical_map[cpu]; > } > -- > 2.29.2.454.gaff20da3a2-goog >
On Thu, Nov 26, 2020 at 05:28:38PM +0000, Mark Rutland wrote: > On Thu, Nov 26, 2020 at 03:54:01PM +0000, David Brazdil wrote: > > CPU index should never be negative. Change the signature of > > (set_)cpu_logical_map to take an unsigned int. > > > > Signed-off-by: David Brazdil <dbrazdil@google.com> > > Is there a function problem here, or is this just cleanup from > inspection? > > Core code including the cpuhp_*() callbacks uses an int, so if there's a > strong justification to change this, it suggests there's some treewide > cleanup that should be done. > > I don't have strong feelings on the matter, but I'd like to understand > the rationale. Yeah, it's a mess. Marc and I felt that using a uint was less error-prone wrt bounds checks. If this gets an int, it still works and only checking the upper bound is required. Does that make sense? David
diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h index 2e7f529ec5a6..bcb01ca15325 100644 --- a/arch/arm64/include/asm/smp.h +++ b/arch/arm64/include/asm/smp.h @@ -46,9 +46,9 @@ DECLARE_PER_CPU_READ_MOSTLY(int, cpu_number); * Logical CPU mapping. */ extern u64 __cpu_logical_map[NR_CPUS]; -extern u64 cpu_logical_map(int cpu); +extern u64 cpu_logical_map(unsigned int cpu); -static inline void set_cpu_logical_map(int cpu, u64 hwid) +static inline void set_cpu_logical_map(unsigned int cpu, u64 hwid) { __cpu_logical_map[cpu] = hwid; } diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c index 133257ffd859..2f2973bc67c7 100644 --- a/arch/arm64/kernel/setup.c +++ b/arch/arm64/kernel/setup.c @@ -276,7 +276,7 @@ arch_initcall(reserve_memblock_reserved_regions); u64 __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = INVALID_HWID }; -u64 cpu_logical_map(int cpu) +u64 cpu_logical_map(unsigned int cpu) { return __cpu_logical_map[cpu]; }
CPU index should never be negative. Change the signature of (set_)cpu_logical_map to take an unsigned int. Signed-off-by: David Brazdil <dbrazdil@google.com> --- arch/arm64/include/asm/smp.h | 4 ++-- arch/arm64/kernel/setup.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)