Re: [PATCH] lib/zlib_inflate/inffast: Check config in C to avoid unused function warning
Le 16/09/2021 à 16:22, Paul Menzel a écrit : Building Linux for ppc64le with Ubuntu clang version 12.0.0-3ubuntu1~21.04.1 shows the warning below. arch/powerpc/boot/inffast.c:20:1: warning: unused function 'get_unaligned16' [-Wunused-function] get_unaligned16(const unsigned short *p) ^ 1 warning generated. Fix it, by moving the check from the preprocessor to C, so the compiler sees the use. Signed-off-by: Paul Menzel --- lib/zlib_inflate/inffast.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/zlib_inflate/inffast.c b/lib/zlib_inflate/inffast.c index f19c4fbe1be7..444ad3c3ccd3 100644 --- a/lib/zlib_inflate/inffast.c +++ b/lib/zlib_inflate/inffast.c @@ -254,11 +254,7 @@ void inflate_fast(z_streamp strm, unsigned start) sfrom = (unsigned short *)(from); loops = len >> 1; do -#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS - *sout++ = *sfrom++; -#else - *sout++ = get_unaligned16(sfrom++); -#endif + *sout++ = CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS ? *sfrom++ : get_unaligned16(sfrom++); You can't do that, CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is not something that have value 0 or 1, it is something which is either defined or not. You have to use IS_ENABLED() macro, so it should become something like: if (IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)) *sout++ = *sfrom++; else *sout++ = get_unaligned16(sfrom++); while (--loops); out = (unsigned char *)sout; from = (unsigned char *)sfrom;
Re: [PATCH v5 6/6] sched/fair: Consider SMT in ASYM_PACKING load balance
On Wed, Sep 15, 2021 at 05:43:44PM +0200, Vincent Guittot wrote:
> On Sat, 11 Sept 2021 at 03:19, Ricardo Neri
> wrote:
> >
> > When deciding to pull tasks in ASYM_PACKING, it is necessary not only to
> > check for the idle state of the destination CPU, dst_cpu, but also of
> > its SMT siblings.
> >
> > If dst_cpu is idle but its SMT siblings are busy, performance suffers
> > if it pulls tasks from a medium priority CPU that does not have SMT
> > siblings.
> >
> > Implement asym_smt_can_pull_tasks() to inspect the state of the SMT
> > siblings of both dst_cpu and the CPUs in the candidate busiest group.
> >
> > Cc: Aubrey Li
> > Cc: Ben Segall
> > Cc: Daniel Bristot de Oliveira
> > Cc: Dietmar Eggemann
> > Cc: Mel Gorman
> > Cc: Quentin Perret
> > Cc: Rafael J. Wysocki
> > Cc: Srinivas Pandruvada
> > Cc: Steven Rostedt
> > Cc: Tim Chen
> > Reviewed-by: Joel Fernandes (Google)
> > Reviewed-by: Len Brown
> > Signed-off-by: Ricardo Neri
> > ---
> > Changes since v4:
> > * Use sg_lb_stats::sum_nr_running the idle state of a scheduling group.
> > (Vincent, Peter)
> > * Do not even idle CPUs in asym_smt_can_pull_tasks(). (Vincent)
> > * Updated function documentation and corrected a typo.
> >
> > Changes since v3:
> > * Removed the arch_asym_check_smt_siblings() hook. Discussions with the
> > powerpc folks showed that this patch should not impact them. Also, more
> > recent powerpc processor no longer use asym_packing. (PeterZ)
> > * Removed unnecessary local variable in asym_can_pull_tasks(). (Dietmar)
> > * Removed unnecessary check for local CPUs when the local group has zero
> > utilization. (Joel)
> > * Renamed asym_can_pull_tasks() as asym_smt_can_pull_tasks() to reflect
> > the fact that it deals with SMT cases.
> > * Made asym_smt_can_pull_tasks() return false for !CONFIG_SCHED_SMT so
> > that callers can deal with non-SMT cases.
> >
> > Changes since v2:
> > * Reworded the commit message to reflect updates in code.
> > * Corrected misrepresentation of dst_cpu as the CPU doing the load
> > balancing. (PeterZ)
> > * Removed call to arch_asym_check_smt_siblings() as it is now called in
> > sched_asym().
> >
> > Changes since v1:
> > * Don't bailout in update_sd_pick_busiest() if dst_cpu cannot pull
> > tasks. Instead, reclassify the candidate busiest group, as it
> > may still be selected. (PeterZ)
> > * Avoid an expensive and unnecessary call to cpumask_weight() when
> > determining if a sched_group is comprised of SMT siblings.
> > (PeterZ).
> > ---
> > kernel/sched/fair.c | 94 +
> > 1 file changed, 94 insertions(+)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 26db017c14a3..8d763dd0174b 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -8597,10 +8597,98 @@ group_type group_classify(unsigned int
> > imbalance_pct,
> > return group_has_spare;
> > }
> >
> > +/**
> > + * asym_smt_can_pull_tasks - Check whether the load balancing CPU can pull
> > tasks
> > + * @dst_cpu: Destination CPU of the load balancing
> > + * @sds: Load-balancing data with statistics of the local group
> > + * @sgs: Load-balancing statistics of the candidate busiest group
> > + * @sg:The candidate busiest group
> > + *
> > + * Check the state of the SMT siblings of both @sds::local and @sg and
> > decide
> > + * if @dst_cpu can pull tasks.
> > + *
> > + * If @dst_cpu does not have SMT siblings, it can pull tasks if two or
> > more of
> > + * the SMT siblings of @sg are busy. If only one CPU in @sg is busy, pull
> > tasks
> > + * only if @dst_cpu has higher priority.
> > + *
> > + * If both @dst_cpu and @sg have SMT siblings, and @sg has exactly one more
> > + * busy CPU than @sds::local, let @dst_cpu pull tasks if it has higher
> > priority.
> > + * Bigger imbalances in the number of busy CPUs will be dealt with in
> > + * update_sd_pick_busiest().
> > + *
> > + * If @sg does not have SMT siblings, only pull tasks if all of the SMT
> > siblings
> > + * of @dst_cpu are idle and @sg has lower priority.
> > + */
> > +static bool asym_smt_can_pull_tasks(int dst_cpu, struct sd_lb_stats *sds,
> > + struct sg_lb_stats *sgs,
> > + struct sched_group *sg)
> > +{
> > +#ifdef CONFIG_SCHED_SMT
> > + bool local_is_smt, sg_is_smt;
> > + int sg_busy_cpus;
> > +
> > + local_is_smt = sds->local->flags & SD_SHARE_CPUCAPACITY;
> > + sg_is_smt = sg->flags & SD_SHARE_CPUCAPACITY;
> > +
> > + sg_busy_cpus = sgs->group_weight - sgs->idle_cpus;
> > +
> > + if (!local_is_smt) {
> > + /*
> > +* If we are here, @dst_cpu is idle and does not have SMT
> > +* siblings. Pull tasks if candidate group has two or more
> > +* busy CPUs.
> > +*/
> > +
Re: [PATCH] lib/zlib_inflate/inffast: Check config in C to avoid unused function warning
Hi Paul,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.15-rc1 next-20210916]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Paul-Menzel/lib-zlib_inflate-inffast-Check-config-in-C-to-avoid-unused-function-warning/20210916-222359
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
ff1ffd71d5f0612cf194f5705c671d6b64bf5f91
config: nds32-defconfig (attached as .config)
compiler: nds32le-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/127eebd64d8e291cf75563499f6c886bd54a99d8
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Paul-Menzel/lib-zlib_inflate-inffast-Check-config-in-C-to-avoid-unused-function-warning/20210916-222359
git checkout 127eebd64d8e291cf75563499f6c886bd54a99d8
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross
O=build_dir ARCH=nds32 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
lib/zlib_inflate/inffast.c: In function 'inflate_fast':
>> lib/zlib_inflate/inffast.c:257:39: error:
>> 'CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS' undeclared (first use in this
>> function)
257 | *sout++ =
CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS ? *sfrom++ : get_unaligned16(sfrom++);
|
^~
lib/zlib_inflate/inffast.c:257:39: note: each undeclared identifier is
reported only once for each function it appears in
vim +/CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS +257 lib/zlib_inflate/inffast.c
29
30 /*
31 Decode literal, length, and distance codes and write out the
resulting
32 literal and match bytes until either not enough input or output is
33 available, an end-of-block is encountered, or a data error is
encountered.
34 When large enough input and output buffers are supplied to
inflate(), for
35 example, a 16K input buffer and a 64K output buffer, more than 95%
of the
36 inflate execution time is spent in this routine.
37
38 Entry assumptions:
39
40 state->mode == LEN
41 strm->avail_in >= 6
42 strm->avail_out >= 258
43 start >= strm->avail_out
44 state->bits < 8
45
46 On return, state->mode is one of:
47
48 LEN -- ran out of enough output space or enough available input
49 TYPE -- reached end of block code, inflate() to interpret next
block
50 BAD -- error in block data
51
52 Notes:
53
54 - The maximum input bits used by a length/distance pair is 15 bits
for the
55length code, 5 bits for the length extra, 15 bits for the
distance code,
56and 13 bits for the distance extra. This totals 48 bits, or six
bytes.
57Therefore if strm->avail_in >= 6, then there is enough input to
avoid
58checking for available input while decoding.
59
60 - The maximum bytes that a single length/distance pair can output
is 258
61bytes, which is the maximum length that can be coded.
inflate_fast()
62requires strm->avail_out >= 258 for each loop to avoid checking
for
63output space.
64
65 - @start: inflate()'s starting value for strm->avail_out
66 */
67 void inflate_fast(z_streamp strm, unsigned start)
68 {
69 struct inflate_state *state;
70 const unsigned char *in;/* local strm->next_in */
71 const unsigned char *last; /* while in < last, enough input
available */
72 unsigned char *out; /* local strm->next_out */
73 unsigned char *beg; /* inflate()'s initial strm->next_out */
74 unsigned char *end; /* while out < end, enough space
available */
75 #ifdef INFLATE_STRICT
76 unsigned dmax; /* maximum distance from zlib header */
77 #endif
78 unsigned wsize; /* window size or zero if not using
window */
79 unsigned whave; /* valid bytes in the window */
80 unsigned write; /* window write index */
81 unsigned char *window; /* allocate
Re: [PATCH] lib/zlib_inflate/inffast: Check config in C to avoid unused function warning
Hi Paul,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.15-rc1 next-20210916]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Paul-Menzel/lib-zlib_inflate-inffast-Check-config-in-C-to-avoid-unused-function-warning/20210916-222359
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
ff1ffd71d5f0612cf194f5705c671d6b64bf5f91
config: hexagon-randconfig-r045-20210916 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project
c8b3d7d6d6de37af68b2f379d0e37304f78e115f)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/127eebd64d8e291cf75563499f6c886bd54a99d8
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Paul-Menzel/lib-zlib_inflate-inffast-Check-config-in-C-to-avoid-unused-function-warning/20210916-222359
git checkout 127eebd64d8e291cf75563499f6c886bd54a99d8
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1
O=build_dir ARCH=hexagon SHELL=/bin/bash lib/zlib_inflate/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
>> lib/zlib_inflate/inffast.c:257:18: error: use of undeclared identifier
>> 'CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS'
*sout++ = CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
? *sfrom++ : get_unaligned16(sfrom++);
^
1 error generated.
vim +/CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS +257 lib/zlib_inflate/inffast.c
29
30 /*
31 Decode literal, length, and distance codes and write out the
resulting
32 literal and match bytes until either not enough input or output is
33 available, an end-of-block is encountered, or a data error is
encountered.
34 When large enough input and output buffers are supplied to
inflate(), for
35 example, a 16K input buffer and a 64K output buffer, more than 95%
of the
36 inflate execution time is spent in this routine.
37
38 Entry assumptions:
39
40 state->mode == LEN
41 strm->avail_in >= 6
42 strm->avail_out >= 258
43 start >= strm->avail_out
44 state->bits < 8
45
46 On return, state->mode is one of:
47
48 LEN -- ran out of enough output space or enough available input
49 TYPE -- reached end of block code, inflate() to interpret next
block
50 BAD -- error in block data
51
52 Notes:
53
54 - The maximum input bits used by a length/distance pair is 15 bits
for the
55length code, 5 bits for the length extra, 15 bits for the
distance code,
56and 13 bits for the distance extra. This totals 48 bits, or six
bytes.
57Therefore if strm->avail_in >= 6, then there is enough input to
avoid
58checking for available input while decoding.
59
60 - The maximum bytes that a single length/distance pair can output
is 258
61bytes, which is the maximum length that can be coded.
inflate_fast()
62requires strm->avail_out >= 258 for each loop to avoid checking
for
63output space.
64
65 - @start: inflate()'s starting value for strm->avail_out
66 */
67 void inflate_fast(z_streamp strm, unsigned start)
68 {
69 struct inflate_state *state;
70 const unsigned char *in;/* local strm->next_in */
71 const unsigned char *last; /* while in < last, enough input
available */
72 unsigned char *out; /* local strm->next_out */
73 unsigned char *beg; /* inflate()'s initial strm->next_out */
74 unsigned char *end; /* while out < end, enough space
available */
75 #ifdef INFLATE_STRICT
76 unsigned dmax; /* maximum distance from zlib header */
77 #endif
78 unsigned wsize; /* window size or zero if not using
window */
79 unsigned whave; /* valid bytes in the window */
80 unsigned write; /* window write index */
81 unsigned char *window; /* allocated sliding window, if wsize
!= 0 */
82 unsigned long hold; /* local strm->hold */
83 unsigned bits;
[PATCH] powerpc/lib/sstep: Don't use __{get/put}_user() on kernel addresses
In the old days, when we didn't have kernel userspace access
protection and had set_fs(), it was wise to use __get_user()
and friends to read kernel memory.
Nowadays, get_user() and put_user() are granting userspace access and
are exclusively for userspace access.
Convert single step emulation functions to user_access_begin() and
friends and use unsafe_get_user() and unsafe_put_user().
When addressing kernel addresses, there is no need to open userspace
access. And for book3s/32 it is particularly important to no try and
open userspace access on kernel address, because that would break the
content of kernel space segment registers. No guard has been put
against that risk in order to avoid degrading performance.
copy_from_kernel_nofault() and copy_to_kernel_nofault() should
be used but they are out-of-line functions which would degrade
performance. Those two functions are making use of
__get_kernel_nofault() and __put_kernel_nofault() macros.
Those two macros are just wrappers behind __get_user_size_goto() and
__put_user_size_goto().
unsafe_get_user() and unsafe_put_user() are also wrappers of
__get_user_size_goto() and __put_user_size_goto(). Use them to
access kernel space. That allows refactoring userspace and
kernelspace access.
Reported-by: Stan Johnson
Cc: Finn Thain
Depends-on: 4fe5cda9f89d ("powerpc/uaccess: Implement user_read_access_begin
and user_write_access_begin")
Signed-off-by: Christophe Leroy
---
arch/powerpc/lib/sstep.c | 197 ---
1 file changed, 140 insertions(+), 57 deletions(-)
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index d8d5f901cee1..86f49e3e7cf5 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -302,33 +302,51 @@ static nokprobe_inline void do_byte_reverse(void *ptr,
int nb)
}
}
-static nokprobe_inline int read_mem_aligned(unsigned long *dest,
- unsigned long ea, int nb,
- struct pt_regs *regs)
+static __always_inline int
+__read_mem_aligned(unsigned long *dest, unsigned long ea, int nb, struct
pt_regs *regs)
{
- int err = 0;
unsigned long x = 0;
switch (nb) {
case 1:
- err = __get_user(x, (unsigned char __user *) ea);
+ unsafe_get_user(x, (unsigned char __user *)ea, Efault);
break;
case 2:
- err = __get_user(x, (unsigned short __user *) ea);
+ unsafe_get_user(x, (unsigned short __user *)ea, Efault);
break;
case 4:
- err = __get_user(x, (unsigned int __user *) ea);
+ unsafe_get_user(x, (unsigned int __user *)ea, Efault);
break;
#ifdef __powerpc64__
case 8:
- err = __get_user(x, (unsigned long __user *) ea);
+ unsafe_get_user(x, (unsigned long __user *)ea, Efault);
break;
#endif
}
- if (!err)
- *dest = x;
- else
+ *dest = x;
+ return 0;
+
+Efault:
+ regs->dar = ea;
+ return -EFAULT;
+}
+
+static nokprobe_inline int
+read_mem_aligned(unsigned long *dest, unsigned long ea, int nb, struct pt_regs
*regs)
+{
+ int err;
+
+ if (is_kernel_addr(ea))
+ return __read_mem_aligned(dest, ea, nb, regs);
+
+ if (user_read_access_begin((void __user *)ea, nb)) {
+ err = __read_mem_aligned(dest, ea, nb, regs);
+ user_read_access_end();
+ } else {
+ err = -EFAULT;
regs->dar = ea;
+ }
+
return err;
}
@@ -336,10 +354,8 @@ static nokprobe_inline int read_mem_aligned(unsigned long
*dest,
* Copy from userspace to a buffer, using the largest possible
* aligned accesses, up to sizeof(long).
*/
-static nokprobe_inline int copy_mem_in(u8 *dest, unsigned long ea, int nb,
- struct pt_regs *regs)
+static __always_inline int __copy_mem_in(u8 *dest, unsigned long ea, int nb,
struct pt_regs *regs)
{
- int err = 0;
int c;
for (; nb > 0; nb -= c) {
@@ -348,31 +364,46 @@ static nokprobe_inline int copy_mem_in(u8 *dest, unsigned
long ea, int nb,
c = max_align(nb);
switch (c) {
case 1:
- err = __get_user(*dest, (unsigned char __user *) ea);
+ unsafe_get_user(*dest, (u8 __user *)ea, Efault);
break;
case 2:
- err = __get_user(*(u16 *)dest,
-(unsigned short __user *) ea);
+ unsafe_get_user(*(u16 *)dest, (u16 __user *)ea, Efault);
break;
case 4:
- err = __get_user(*(u32 *)dest,
-(unsigned int __user *) ea);
+
Re: [PATCH v3 0/8] Implement generic cc_platform_has() helper function
On 9/16/21 8:02 AM, Borislav Petkov wrote: On Wed, Sep 15, 2021 at 10:26:06AM -0700, Kuppuswamy, Sathyanarayanan wrote: I have a Intel variant patch (please check following patch). But it includes TDX changes as well. Shall I move TDX changes to different patch and just create a separate patch for adding intel_cc_platform_has()? Yes, please, so that I can expedite that stuff separately and so that it can go in early in order for future work to be based ontop. Sent it part of TDX patch series. Please check and cherry pick it. https://lore.kernel.org/lkml/20210916183550.15349-2-sathyanarayanan.kuppusw...@linux.intel.com/ Thx. -- Sathyanarayanan Kuppuswamy Linux Kernel Developer
[PATCH 5.14 232/432] hvsi: dont panic on tty_register_driver failure
From: Jiri Slaby
[ Upstream commit 7ccbdcc4d08a6d7041e4849219bbb12ffa45db4c ]
The alloc_tty_driver failure is handled gracefully in hvsi_init. But
tty_register_driver is not. panic is called if that one fails.
So handle the failure of tty_register_driver gracefully too. This will
keep at least the console functional as it was enabled earlier by
console_initcall in hvsi_console_init. Instead of shooting down the
whole system.
This means, we disable interrupts and restore hvsi_wait back to
poll_for_state().
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Jiri Slaby
Link: https://lore.kernel.org/r/20210723074317.32690-3-jsl...@suse.cz
Signed-off-by: Greg Kroah-Hartman
Signed-off-by: Sasha Levin
---
drivers/tty/hvc/hvsi.c | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/hvc/hvsi.c b/drivers/tty/hvc/hvsi.c
index bfc15279d5bc..f0bc8e780051 100644
--- a/drivers/tty/hvc/hvsi.c
+++ b/drivers/tty/hvc/hvsi.c
@@ -1038,7 +1038,7 @@ static const struct tty_operations hvsi_ops = {
static int __init hvsi_init(void)
{
- int i;
+ int i, ret;
hvsi_driver = alloc_tty_driver(hvsi_count);
if (!hvsi_driver)
@@ -1069,12 +1069,25 @@ static int __init hvsi_init(void)
}
hvsi_wait = wait_for_state; /* irqs active now */
- if (tty_register_driver(hvsi_driver))
- panic("Couldn't register hvsi console driver\n");
+ ret = tty_register_driver(hvsi_driver);
+ if (ret) {
+ pr_err("Couldn't register hvsi console driver\n");
+ goto err_free_irq;
+ }
printk(KERN_DEBUG "HVSI: registered %i devices\n", hvsi_count);
return 0;
+err_free_irq:
+ hvsi_wait = poll_for_state;
+ for (i = 0; i < hvsi_count; i++) {
+ struct hvsi_struct *hp = _ports[i];
+
+ free_irq(hp->virq, hp);
+ }
+ tty_driver_kref_put(hvsi_driver);
+
+ return ret;
}
device_initcall(hvsi_init);
--
2.30.2
[PATCH 5.13 205/380] hvsi: dont panic on tty_register_driver failure
From: Jiri Slaby
[ Upstream commit 7ccbdcc4d08a6d7041e4849219bbb12ffa45db4c ]
The alloc_tty_driver failure is handled gracefully in hvsi_init. But
tty_register_driver is not. panic is called if that one fails.
So handle the failure of tty_register_driver gracefully too. This will
keep at least the console functional as it was enabled earlier by
console_initcall in hvsi_console_init. Instead of shooting down the
whole system.
This means, we disable interrupts and restore hvsi_wait back to
poll_for_state().
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Jiri Slaby
Link: https://lore.kernel.org/r/20210723074317.32690-3-jsl...@suse.cz
Signed-off-by: Greg Kroah-Hartman
Signed-off-by: Sasha Levin
---
drivers/tty/hvc/hvsi.c | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/hvc/hvsi.c b/drivers/tty/hvc/hvsi.c
index e8c58f9bd263..d6afaae1729a 100644
--- a/drivers/tty/hvc/hvsi.c
+++ b/drivers/tty/hvc/hvsi.c
@@ -1038,7 +1038,7 @@ static const struct tty_operations hvsi_ops = {
static int __init hvsi_init(void)
{
- int i;
+ int i, ret;
hvsi_driver = alloc_tty_driver(hvsi_count);
if (!hvsi_driver)
@@ -1069,12 +1069,25 @@ static int __init hvsi_init(void)
}
hvsi_wait = wait_for_state; /* irqs active now */
- if (tty_register_driver(hvsi_driver))
- panic("Couldn't register hvsi console driver\n");
+ ret = tty_register_driver(hvsi_driver);
+ if (ret) {
+ pr_err("Couldn't register hvsi console driver\n");
+ goto err_free_irq;
+ }
printk(KERN_DEBUG "HVSI: registered %i devices\n", hvsi_count);
return 0;
+err_free_irq:
+ hvsi_wait = poll_for_state;
+ for (i = 0; i < hvsi_count; i++) {
+ struct hvsi_struct *hp = _ports[i];
+
+ free_irq(hp->virq, hp);
+ }
+ tty_driver_kref_put(hvsi_driver);
+
+ return ret;
}
device_initcall(hvsi_init);
--
2.30.2
[PATCH 5.10 163/306] hvsi: dont panic on tty_register_driver failure
From: Jiri Slaby
[ Upstream commit 7ccbdcc4d08a6d7041e4849219bbb12ffa45db4c ]
The alloc_tty_driver failure is handled gracefully in hvsi_init. But
tty_register_driver is not. panic is called if that one fails.
So handle the failure of tty_register_driver gracefully too. This will
keep at least the console functional as it was enabled earlier by
console_initcall in hvsi_console_init. Instead of shooting down the
whole system.
This means, we disable interrupts and restore hvsi_wait back to
poll_for_state().
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Jiri Slaby
Link: https://lore.kernel.org/r/20210723074317.32690-3-jsl...@suse.cz
Signed-off-by: Greg Kroah-Hartman
Signed-off-by: Sasha Levin
---
drivers/tty/hvc/hvsi.c | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/hvc/hvsi.c b/drivers/tty/hvc/hvsi.c
index e8c58f9bd263..d6afaae1729a 100644
--- a/drivers/tty/hvc/hvsi.c
+++ b/drivers/tty/hvc/hvsi.c
@@ -1038,7 +1038,7 @@ static const struct tty_operations hvsi_ops = {
static int __init hvsi_init(void)
{
- int i;
+ int i, ret;
hvsi_driver = alloc_tty_driver(hvsi_count);
if (!hvsi_driver)
@@ -1069,12 +1069,25 @@ static int __init hvsi_init(void)
}
hvsi_wait = wait_for_state; /* irqs active now */
- if (tty_register_driver(hvsi_driver))
- panic("Couldn't register hvsi console driver\n");
+ ret = tty_register_driver(hvsi_driver);
+ if (ret) {
+ pr_err("Couldn't register hvsi console driver\n");
+ goto err_free_irq;
+ }
printk(KERN_DEBUG "HVSI: registered %i devices\n", hvsi_count);
return 0;
+err_free_irq:
+ hvsi_wait = poll_for_state;
+ for (i = 0; i < hvsi_count; i++) {
+ struct hvsi_struct *hp = _ports[i];
+
+ free_irq(hp->virq, hp);
+ }
+ tty_driver_kref_put(hvsi_driver);
+
+ return ret;
}
device_initcall(hvsi_init);
--
2.30.2
Re: [PATCH v3 0/8] Implement generic cc_platform_has() helper function
On Wed, Sep 15, 2021 at 10:26:06AM -0700, Kuppuswamy, Sathyanarayanan wrote: > I have a Intel variant patch (please check following patch). But it includes > TDX changes as well. Shall I move TDX changes to different patch and just > create a separate patch for adding intel_cc_platform_has()? Yes, please, so that I can expedite that stuff separately and so that it can go in early in order for future work to be based ontop. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] lib/zlib_inflate/inffast: Check config in C to avoid unused function warning
Hi Paul, On 9/16/2021 7:22 AM, Paul Menzel wrote: Building Linux for ppc64le with Ubuntu clang version 12.0.0-3ubuntu1~21.04.1 shows the warning below. arch/powerpc/boot/inffast.c:20:1: warning: unused function 'get_unaligned16' [-Wunused-function] get_unaligned16(const unsigned short *p) ^ 1 warning generated. Fix it, by moving the check from the preprocessor to C, so the compiler sees the use. Signed-off-by: Paul Menzel --- lib/zlib_inflate/inffast.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/zlib_inflate/inffast.c b/lib/zlib_inflate/inffast.c index f19c4fbe1be7..444ad3c3ccd3 100644 --- a/lib/zlib_inflate/inffast.c +++ b/lib/zlib_inflate/inffast.c @@ -254,11 +254,7 @@ void inflate_fast(z_streamp strm, unsigned start) sfrom = (unsigned short *)(from); loops = len >> 1; do -#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS - *sout++ = *sfrom++; -#else - *sout++ = get_unaligned16(sfrom++); -#endif + *sout++ = CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS ? *sfrom++ : get_unaligned16(sfrom++); while (--loops); out = (unsigned char *)sout; from = (unsigned char *)sfrom; Thanks for the patch. This should probably be IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) ? ... which matches the rest of the kernel tree, as certain CONFIG_... values are not guaranteed to always be defined. Cheers, Nathan
[PATCH v2] powerpc: warn on emulation of dcbz instruction in kernel mode
dcbz instruction shouldn't be used on non-cached memory. Using
it on non-cached memory can result in alignment exception and
implies a heavy handling.
Instead of silentely emulating the instruction and resulting in high
performance degradation, warn whenever an alignment exception is
taken in kernel mode due to dcbz, so that the user is made aware that
dcbz instruction has been used unexpectedly by the kernel.
Reported-by: Stan Johnson
Cc: Finn Thain
Signed-off-by: Christophe Leroy
---
v2: Warn only when emulating kernel mode
---
arch/powerpc/kernel/align.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
index bbb4181621dd..bf96b954a4eb 100644
--- a/arch/powerpc/kernel/align.c
+++ b/arch/powerpc/kernel/align.c
@@ -349,6 +349,7 @@ int fix_alignment(struct pt_regs *regs)
if (op.type != CACHEOP + DCBZ)
return -EINVAL;
PPC_WARN_ALIGNMENT(dcbz, regs);
+ WARN_ON_ONCE(!user_mode(regs));
r = emulate_dcbz(op.ea, regs);
} else {
if (type == LARX || type == STCX)
--
2.31.1
Re: [RFC PATCH 1/8] arm64: add CPU field to struct thread_info
On Tue, Sep 14, 2021 at 02:10:29PM +0200, Ard Biesheuvel wrote: > The CPU field will be moved back into thread_info even when > THREAD_INFO_IN_TASK is enabled, so add it back to arm64's definition of > struct thread_info. > > Signed-off-by: Ard Biesheuvel Acked-by: Catalin Marinas
RE: [PATCH] powerpc: warn on emulation of dcbz instruction
From: Christophe Leroy > Sent: 16 September 2021 08:24 > > Le 16/09/2021 à 09:16, Benjamin Herrenschmidt a écrit : > > On Thu, 2021-09-16 at 17:15 +1000, Benjamin Herrenschmidt wrote: > >> On Wed, 2021-09-15 at 16:31 +0200, Christophe Leroy wrote: > >>> dcbz instruction shouldn't be used on non-cached memory. Using > >>> it on non-cached memory can result in alignment exception and > >>> implies a heavy handling. > >>> > >>> Instead of silentely emulating the instruction and resulting in > >>> high > >>> performance degradation, warn whenever an alignment exception is > >>> taken due to dcbz, so that the user is made aware that dcbz > >>> instruction has been used unexpectedly. > >>> > >>> Reported-by: Stan Johnson > >>> Cc: Finn Thain > >>> Signed-off-by: Christophe Leroy > >>> --- > >>> arch/powerpc/kernel/align.c | 1 + > >>> 1 file changed, 1 insertion(+) > >>> > >>> diff --git a/arch/powerpc/kernel/align.c > >>> b/arch/powerpc/kernel/align.c > >>> index bbb4181621dd..adc3a4a9c6e4 100644 > >>> --- a/arch/powerpc/kernel/align.c > >>> +++ b/arch/powerpc/kernel/align.c > >>> @@ -349,6 +349,7 @@ int fix_alignment(struct pt_regs *regs) > >>> if (op.type != CACHEOP + DCBZ) > >>> return -EINVAL; > >>> PPC_WARN_ALIGNMENT(dcbz, regs); > >>> + WARN_ON_ONCE(1); > >> > >> This is heavy handed ... It will be treated as an oops by various > >> things uselessly spit out a kernel backtrace. Isn't > >> PPC_WARN_ALIGNMENT > >> enough ? > > > PPC_WARN_ALIGNMENT() only warns if explicitely activated, I want to > catch uses on 'dcbz' on non-cached memory all the time as they are most > often the result of using memset() instead of memset_io(). > > > > > Ah I saw your other one about fbdev... Ok what about you do that in a > > if (!user_mode(regs)) ? > > Yes I can do WARN_ON_ONCE(!user_mode(regs)); instead. > > > Indeed the kernel should not do that. > > Does userspace accesses non-cached memory directly ? It probably can if a driver mmaps PCI space directly into user space. That certainly works on x86-64. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
[PATCH] lib/zlib_inflate/inffast: Check config in C to avoid unused function warning
Building Linux for ppc64le with Ubuntu clang version 12.0.0-3ubuntu1~21.04.1 shows the warning below. arch/powerpc/boot/inffast.c:20:1: warning: unused function 'get_unaligned16' [-Wunused-function] get_unaligned16(const unsigned short *p) ^ 1 warning generated. Fix it, by moving the check from the preprocessor to C, so the compiler sees the use. Signed-off-by: Paul Menzel --- lib/zlib_inflate/inffast.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/zlib_inflate/inffast.c b/lib/zlib_inflate/inffast.c index f19c4fbe1be7..444ad3c3ccd3 100644 --- a/lib/zlib_inflate/inffast.c +++ b/lib/zlib_inflate/inffast.c @@ -254,11 +254,7 @@ void inflate_fast(z_streamp strm, unsigned start) sfrom = (unsigned short *)(from); loops = len >> 1; do -#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS - *sout++ = *sfrom++; -#else - *sout++ = get_unaligned16(sfrom++); -#endif + *sout++ = CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS ? *sfrom++ : get_unaligned16(sfrom++); while (--loops); out = (unsigned char *)sout; from = (unsigned char *)sfrom; -- 2.33.0
Re: [PATCH] nvmem: NVMEM_NINTENDO_OTP should depend on WII
Hi, thanks for this patch, once the Wii U platform will be added it will
need an additional test for WIIU, but for now this is:
Reviewed-by: Emmanuel Gil Peyrot
On Tue, Sep 14, 2021 at 11:29:49AM +0200, Geert Uytterhoeven wrote:
> The Nintendo Wii and Wii U OTP is only present on Nintendo Wii and Wii U
> consoles. Hence add a dependency on WII, to prevent asking the user
> about this driver when configuring a kernel without Nintendo Wii and Wii
> U console support.
>
> Fixes: 3683b761fe3a10ad ("nvmem: nintendo-otp: Add new driver for the Wii and
> Wii U OTP")
> Signed-off-by: Geert Uytterhoeven
> ---
> drivers/nvmem/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index 39854d43758be3fb..da414617a54d4b99 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -109,6 +109,7 @@ config MTK_EFUSE
>
> config NVMEM_NINTENDO_OTP
> tristate "Nintendo Wii and Wii U OTP Support"
> + depends on WII || COMPILE_TEST
> help
> This is a driver exposing the OTP of a Nintendo Wii or Wii U console.
>
> --
> 2.25.1
--
Emmanuel Gil Peyrot
signature.asc
Description: PGP signature
Re: [PATCH v3 4/8] powerpc/pseries/svm: Add a powerpc version of cc_platform_has()
Christoph Hellwig writes:
> On Wed, Sep 15, 2021 at 07:18:34PM +0200, Christophe Leroy wrote:
>> Could you please provide more explicit explanation why inlining such an
>> helper is considered as bad practice and messy ?
>
> Because now we get architectures to all subly differ. Look at the mess
> for ioremap and the ioremap* variant.
>
> The only good reason to allow for inlines if if they are used in a hot
> path. Which cc_platform_has is not, especially not on powerpc.
Yes I agree, it's not a hot path so it doesn't really matter, which is
why I Acked it.
I think it is possible to do both, share the declaration across arches
but also give arches flexibility to use an inline if they prefer, see
patch below.
I'm not suggesting we actually do that for this series now, but I think
it would solve the problem if we ever needed to in future.
cheers
diff --git a/arch/powerpc/platforms/pseries/cc_platform.c
b/arch/powerpc/include/asm/cc_platform.h
similarity index 74%
rename from arch/powerpc/platforms/pseries/cc_platform.c
rename to arch/powerpc/include/asm/cc_platform.h
index e8021af83a19..6285c3c385a6 100644
--- a/arch/powerpc/platforms/pseries/cc_platform.c
+++ b/arch/powerpc/include/asm/cc_platform.h
@@ -7,13 +7,10 @@
* Author: Tom Lendacky
*/
-#include
#include
-
-#include
#include
-bool cc_platform_has(enum cc_attr attr)
+static inline bool arch_cc_platform_has(enum cc_attr attr)
{
switch (attr) {
case CC_ATTR_MEM_ENCRYPT:
@@ -23,4 +20,3 @@ bool cc_platform_has(enum cc_attr attr)
return false;
}
}
-EXPORT_SYMBOL_GPL(cc_platform_has);
diff --git a/arch/x86/include/asm/cc_platform.h
b/arch/x86/include/asm/cc_platform.h
new file mode 100644
index ..0a4220697043
--- /dev/null
+++ b/arch/x86/include/asm/cc_platform.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_CC_PLATFORM_H_
+#define _ASM_X86_CC_PLATFORM_H_
+
+bool arch_cc_platform_has(enum cc_attr attr);
+
+#endif // _ASM_X86_CC_PLATFORM_H_
diff --git a/arch/x86/kernel/cc_platform.c b/arch/x86/kernel/cc_platform.c
index 3c9bacd3c3f3..77e8c3465979 100644
--- a/arch/x86/kernel/cc_platform.c
+++ b/arch/x86/kernel/cc_platform.c
@@ -11,11 +11,11 @@
#include
#include
-bool cc_platform_has(enum cc_attr attr)
+bool arch_cc_platform_has(enum cc_attr attr)
{
if (sme_me_mask)
return amd_cc_platform_has(attr);
return false;
}
-EXPORT_SYMBOL_GPL(cc_platform_has);
+EXPORT_SYMBOL_GPL(arch_cc_platform_has);
diff --git a/include/linux/cc_platform.h b/include/linux/cc_platform.h
index 253f3ea66cd8..f3306647c5d9 100644
--- a/include/linux/cc_platform.h
+++ b/include/linux/cc_platform.h
@@ -65,6 +65,8 @@ enum cc_attr {
#ifdef CONFIG_ARCH_HAS_CC_PLATFORM
+#include
+
/**
* cc_platform_has() - Checks if the specified cc_attr attribute is active
* @attr: Confidential computing attribute to check
@@ -77,7 +79,10 @@ enum cc_attr {
* * TRUE - Specified Confidential Computing attribute is active
* * FALSE - Specified Confidential Computing attribute is not active
*/
-bool cc_platform_has(enum cc_attr attr);
+static inline bool cc_platform_has(enum cc_attr attr)
+{
+ return arch_cc_platform_has(attr);
+}
#else /* !CONFIG_ARCH_HAS_CC_PLATFORM */
Re: [PATCH 1/3] perf: Add macros to specify onchip L2/L3 accesses
Peter Zijlstra writes: > On Tue, Sep 14, 2021 at 08:40:38PM +1000, Michael Ellerman wrote: >> Peter Zijlstra writes: > >> > I'm thinking we ought to keep hops as steps along the NUMA fabric, with >> > 0 hops being the local node. That only gets us: >> > >> > L2, remote=0, hops=HOPS_0 -- our L2 >> > L2, remote=1, hops=HOPS_0 -- L2 on the local node but not ours >> > L2, remote=1, hops!=HOPS_0 -- L2 on a remote node >> >> Hmm. I'm not sure about tying it directly to NUMA hops. I worry we're >> going to see more and more systems where there's a hierarchy within the >> chip/package, in addition to the traditional NUMA hierarchy. >> >> Although then I guess it becomes a question of what exactly is a NUMA >> hop, maybe the answer is that on those future systems those >> intra-chip/package hops should be represented as NUMA hops. >> >> It's not like we have a hard definition of what a NUMA hop is? > > Not really, typically whatever the BIOS/DT/whatever tables tell us. I > think in case of Power you're mostly making things up in software :-) Firmware is software so yes :) > But yeah, I think we have plenty wriggle room there. OK. cheers
Re: [PATCH] swiotlb: set IO TLB segment size via cmdline
Hi Stefano, > Also, Option 1 listed in the webpage seems to be a lot better. Any > reason you can't do that? Because that option both solves the problem > and increases performance. Yes, Option 1 is probably more efficient. But I use another platform under Xen without DMA adjustment functionality. I pinned this webpage only for example to describe the similar problem I had. Cheers, Roman ср, 15 сент. 2021 г. в 04:51, Stefano Stabellini : > > On Tue, 14 Sep 2021, Christoph Hellwig wrote: > > On Tue, Sep 14, 2021 at 05:29:07PM +0200, Jan Beulich wrote: > > > I'm not convinced the swiotlb use describe there falls under "intended > > > use" - mapping a 1280x720 framebuffer in a single chunk? (As an aside, > > > the bottom of this page is also confusing, as following "Then we can > > > confirm the modified swiotlb size in the boot log:" there is a log > > > fragment showing the same original size of 64Mb. > > > > It doesn't. We also do not add hacks to the kernel for whacky out > > of tree modules. > > Also, Option 1 listed in the webpage seems to be a lot better. Any > reason you can't do that? Because that option both solves the problem > and increases performance. -- Best Regards, Roman.
Re: [PATCH] serial: 8250: SERIAL_8250_FSL should not default to y when compile-testing
On Thu, Sep 16, 2021 at 10:55:49AM +0200, Geert Uytterhoeven wrote:
> Hi Johan,
>
> On Thu, Sep 16, 2021 at 10:46 AM Johan Hovold wrote:
> > On Wed, Sep 15, 2021 at 02:56:52PM +0200, Geert Uytterhoeven wrote:
> > > Commit b1442c55ce8977aa ("serial: 8250: extend compile-test coverage")
> > > added compile-test support to the Freescale 16550 driver. However, as
> > > SERIAL_8250_FSL is an invisible symbol, merely enabling COMPILE_TEST now
> > > enables this driver.
> > >
> > > Fix this by making SERIAL_8250_FSL visible. Tighten the dependencies to
> > > prevent asking the user about this driver when configuring a kernel
> > > without appropriate Freescale SoC or ACPI support.
> >
> > This tightening is arguable a separate change which risk introducing
> > regressions if you get it wrong and should go in a separate patch at
> > least.
>
> Getting it wrong would indeed be a regression, but not tightening
> that at the same time would mean I have to send a separate patch with
> a Fixes tag referring to this fix, following this template:
>
> foo should depend on bar
>
> The foo hardware is only present on bar SoCs. Hence add a
> dependency on bar, to prevent asking the user about this driver
> when configuring a kernel without bar support.
I know this is a pet peeve of yours, but asking users about one more
symbol when configuring their kernels is hardly something that requires
a Fixes tag.
Either way it's a pretty weak argument for not separating the change.
Johan
[PATCH v3 07/30] ABI: sysfs-class-cxl: place "not in a guest" at description
The What: field should have just the location of the ABI. Anything else should be inside the description. This fixes its parsing by get_abi.pl script. Acked-by: Andrew Donnellan Signed-off-by: Mauro Carvalho Chehab --- Documentation/ABI/testing/sysfs-class-cxl | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-class-cxl b/Documentation/ABI/testing/sysfs-class-cxl index 818f55970efb..3c77677e0ca7 100644 --- a/Documentation/ABI/testing/sysfs-class-cxl +++ b/Documentation/ABI/testing/sysfs-class-cxl @@ -166,10 +166,11 @@ Description:read only Decimal value of the Per Process MMIO space length. Users: https://github.com/ibm-capi/libcxl -What: /sys/class/cxl/m/pp_mmio_off (not in a guest) +What: /sys/class/cxl/m/pp_mmio_off Date: September 2014 Contact:linuxppc-dev@lists.ozlabs.org Description:read only +(not in a guest) Decimal value of the Per Process MMIO space offset. Users: https://github.com/ibm-capi/libcxl @@ -190,28 +191,31 @@ Description:read only Identifies the revision level of the PSL. Users: https://github.com/ibm-capi/libcxl -What: /sys/class/cxl//base_image (not in a guest) +What: /sys/class/cxl//base_image Date: September 2014 Contact:linuxppc-dev@lists.ozlabs.org Description:read only +(not in a guest) Identifies the revision level of the base image for devices that support loadable PSLs. For FPGAs this field identifies the image contained in the on-adapter flash which is loaded during the initial program load. Users: https://github.com/ibm-capi/libcxl -What: /sys/class/cxl//image_loaded (not in a guest) +What: /sys/class/cxl//image_loaded Date: September 2014 Contact:linuxppc-dev@lists.ozlabs.org Description:read only +(not in a guest) Will return "user" or "factory" depending on the image loaded onto the card. Users: https://github.com/ibm-capi/libcxl -What: /sys/class/cxl//load_image_on_perst (not in a guest) +What: /sys/class/cxl//load_image_on_perst Date: December 2014 Contact:linuxppc-dev@lists.ozlabs.org Description:read/write +(not in a guest) Valid entries are "none", "user", and "factory". "none" means PERST will not cause image to be loaded to the card. A power cycle is required to load the image. @@ -235,10 +239,11 @@ Description:write only contexts on the card AFUs. Users: https://github.com/ibm-capi/libcxl -What: /sys/class/cxl//perst_reloads_same_image (not in a guest) +What: /sys/class/cxl//perst_reloads_same_image Date: July 2015 Contact: linuxppc-dev@lists.ozlabs.org Description: read/write +(not in a guest) Trust that when an image is reloaded via PERST, it will not have changed. -- 2.31.1
[PATCH v3 00/30]Change wildcards on ABI files
The ABI files are meant to be parsed via a script (scripts/get_abi.pl). A new improvement on it will allow it to help to detect if an ABI description is missing, or if the What: field won't match the actual location of the symbol. In order for get_abi.pl to convert What: into regex, changes are needed on existing ABI files, as the conversion should not be ambiguous. One alternative would be to convert everything into regexes, but this would generate a huge amount of patches/changes. So, instead, let's touch only the ABI files that aren't following the de-facto wildcard standards already found on most of the ABI files, e. g.: /.../ * (option1|option2) X Y Z [0-9] (and variants) --- v3: - Added a new patch for sysfs-class-rapidio; - sysfs-class-typec had a typo, instead of a wildcard; - sysfs-bus-soundwire-* had some additional What to be fixed; - added some reviewed-by/acked-by tags. v2: - Added several patches to address uppercase "N" meaning as a wildcard. Mauro Carvalho Chehab (30): ABI: sysfs-bus-usb: better document variable argument ABI: sysfs-tty: better document module name parameter ABI: sysfs-kernel-slab: use a wildcard for the cache name ABI: security: fix location for evm and ima_policy ABI: sysfs-class-tpm: use wildcards for pcr-* nodes ABI: sysfs-bus-rapidio: use wildcards on What definitions ABI: sysfs-class-cxl: place "not in a guest" at description ABI: sysfs-class-devfreq-event: use the right wildcards on What ABI: sysfs-class-mic: use the right wildcards on What definitions ABI: pstore: Fix What field ABI: fix a typo on a What field ABI: sysfs-ata: use a proper wildcard for ata_* ABI: sysfs-class-infiniband: use wildcards on What definitions ABI: sysfs-bus-pci: use wildcards on What definitions ABI: -master: use wildcards on What definitions ABI: sysfs-bus-soundwire-slave: use wildcards on What definitions ABI: sysfs-class-gnss: use wildcards on What definitions ABI: sysfs-class-mei: use wildcards on What definitions ABI: sysfs-class-mux: use wildcards on What definitions ABI: sysfs-class-pwm: use wildcards on What definitions ABI: sysfs-class-rc: use wildcards on What definitions ABI: sysfs-class-rc-nuvoton: use wildcards on What definitions ABI: sysfs-class-uwb_rc: use wildcards on What definitions ABI: sysfs-class-uwb_rc-wusbhc: use wildcards on What definitions ABI: sysfs-devices-platform-dock: use wildcards on What definitions ABI: sysfs-devices-system-cpu: use wildcards on What definitions ABI: sysfs-firmware-efi-esrt: use wildcards on What definitions ABI: sysfs-platform-sst-atom: use wildcards on What definitions ABI: sysfs-ptp: use wildcards on What definitions ABI: sysfs-class-rapidio: use wildcards on What definitions .../ABI/stable/sysfs-class-infiniband | 64 ++--- Documentation/ABI/stable/sysfs-class-tpm | 2 +- Documentation/ABI/testing/evm | 4 +- Documentation/ABI/testing/ima_policy | 2 +- Documentation/ABI/testing/pstore | 3 +- Documentation/ABI/testing/sysfs-ata | 2 +- Documentation/ABI/testing/sysfs-bus-pci | 2 +- Documentation/ABI/testing/sysfs-bus-rapidio | 32 +++ .../ABI/testing/sysfs-bus-soundwire-master| 20 ++-- .../ABI/testing/sysfs-bus-soundwire-slave | 60 ++-- Documentation/ABI/testing/sysfs-bus-usb | 16 ++-- Documentation/ABI/testing/sysfs-class-cxl | 15 ++- .../ABI/testing/sysfs-class-devfreq-event | 12 +-- Documentation/ABI/testing/sysfs-class-gnss| 2 +- Documentation/ABI/testing/sysfs-class-mei | 18 ++-- Documentation/ABI/testing/sysfs-class-mic | 24 ++--- Documentation/ABI/testing/sysfs-class-mux | 2 +- Documentation/ABI/testing/sysfs-class-pwm | 20 ++-- Documentation/ABI/testing/sysfs-class-rapidio | 4 +- Documentation/ABI/testing/sysfs-class-rc | 14 +-- .../ABI/testing/sysfs-class-rc-nuvoton| 2 +- Documentation/ABI/testing/sysfs-class-typec | 2 +- Documentation/ABI/testing/sysfs-class-uwb_rc | 26 ++--- .../ABI/testing/sysfs-class-uwb_rc-wusbhc | 10 +- .../ABI/testing/sysfs-devices-platform-dock | 10 +- .../ABI/testing/sysfs-devices-system-cpu | 16 ++-- .../ABI/testing/sysfs-firmware-efi-esrt | 16 ++-- Documentation/ABI/testing/sysfs-kernel-slab | 94 +-- .../ABI/testing/sysfs-platform-sst-atom | 2 +- Documentation/ABI/testing/sysfs-ptp | 30 +++--- Documentation/ABI/testing/sysfs-tty | 32 +++ 31 files changed, 282 insertions(+), 276 deletions(-) -- 2.31.1
Re: [PATCH] serial: 8250: SERIAL_8250_FSL should not default to y when compile-testing
Hi Johan,
On Thu, Sep 16, 2021 at 10:46 AM Johan Hovold wrote:
> On Wed, Sep 15, 2021 at 02:56:52PM +0200, Geert Uytterhoeven wrote:
> > Commit b1442c55ce8977aa ("serial: 8250: extend compile-test coverage")
> > added compile-test support to the Freescale 16550 driver. However, as
> > SERIAL_8250_FSL is an invisible symbol, merely enabling COMPILE_TEST now
> > enables this driver.
> >
> > Fix this by making SERIAL_8250_FSL visible. Tighten the dependencies to
> > prevent asking the user about this driver when configuring a kernel
> > without appropriate Freescale SoC or ACPI support.
>
> This tightening is arguable a separate change which risk introducing
> regressions if you get it wrong and should go in a separate patch at
> least.
Getting it wrong would indeed be a regression, but not tightening
that at the same time would mean I have to send a separate patch with
a Fixes tag referring to this fix, following this template:
foo should depend on bar
The foo hardware is only present on bar SoCs. Hence add a
dependency on bar, to prevent asking the user about this driver
when configuring a kernel without bar support.
> > Fixes: b1442c55ce8977aa ("serial: 8250: extend compile-test coverage")
> > Signed-off-by: Geert Uytterhoeven
> > ---
> > Yes, it's ugly, but I see no better solution. Do you?
> >
> > drivers/tty/serial/8250/Kconfig | 8 ++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/tty/serial/8250/Kconfig
> > b/drivers/tty/serial/8250/Kconfig
> > index 808268edd2e82a45..a2978b31144e94f2 100644
> > --- a/drivers/tty/serial/8250/Kconfig
> > +++ b/drivers/tty/serial/8250/Kconfig
> > @@ -361,9 +361,13 @@ config SERIAL_8250_BCM2835AUX
> > If unsure, say N.
> >
> > config SERIAL_8250_FSL
> > - bool
> > + bool "Freescale 16550-style UART support (8250 based driver)"
> > depends on SERIAL_8250_CONSOLE
> > - default PPC || ARM || ARM64 || COMPILE_TEST
> > + depends on FSL_SOC || ARCH_LAYERSCAPE || SOC_LS1021A || (ARM64 &&
> > ACPI) || COMPILE_TEST
> > + default FSL_SOC || ARCH_LAYERSCAPE || SOC_LS1021A || (ARM64 && ACPI)
>
> I'd suggest just doing
>
> bool "Freescale 16550-style UART support (8250 based driver)"
> depends on SERIAL_8250_CONSOLE
> default PPC || ARM || ARM64
>
> Since neither of the symbols you add to that "depends on" line is an
> actual build or runtime dependency.
They are.
> Then you can refine the "default" line in a follow up (or argue why you
> think there should be a "depends on FSL_SOC || ...").
>
> > + help
> > + Selecting this option will add support for the 16550-style serial
> > + port hardware found on Freescale SoCs.
> >
> > config SERIAL_8250_DW
> > tristate "Support for Synopsys DesignWare 8250 quirks"
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH] serial: 8250: SERIAL_8250_FSL should not default to y when compile-testing
On Wed, Sep 15, 2021 at 02:56:52PM +0200, Geert Uytterhoeven wrote:
> Commit b1442c55ce8977aa ("serial: 8250: extend compile-test coverage")
> added compile-test support to the Freescale 16550 driver. However, as
> SERIAL_8250_FSL is an invisible symbol, merely enabling COMPILE_TEST now
> enables this driver.
>
> Fix this by making SERIAL_8250_FSL visible. Tighten the dependencies to
> prevent asking the user about this driver when configuring a kernel
> without appropriate Freescale SoC or ACPI support.
This tightening is arguable a separate change which risk introducing
regressions if you get it wrong and should go in a separate patch at
least.
> Fixes: b1442c55ce8977aa ("serial: 8250: extend compile-test coverage")
> Signed-off-by: Geert Uytterhoeven
> ---
> Yes, it's ugly, but I see no better solution. Do you?
>
> drivers/tty/serial/8250/Kconfig | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index 808268edd2e82a45..a2978b31144e94f2 100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -361,9 +361,13 @@ config SERIAL_8250_BCM2835AUX
> If unsure, say N.
>
> config SERIAL_8250_FSL
> - bool
> + bool "Freescale 16550-style UART support (8250 based driver)"
> depends on SERIAL_8250_CONSOLE
> - default PPC || ARM || ARM64 || COMPILE_TEST
> + depends on FSL_SOC || ARCH_LAYERSCAPE || SOC_LS1021A || (ARM64 && ACPI)
> || COMPILE_TEST
> + default FSL_SOC || ARCH_LAYERSCAPE || SOC_LS1021A || (ARM64 && ACPI)
I'd suggest just doing
bool "Freescale 16550-style UART support (8250 based driver)"
depends on SERIAL_8250_CONSOLE
default PPC || ARM || ARM64
Since neither of the symbols you add to that "depends on" line is an
actual build or runtime dependency.
Then you can refine the "default" line in a follow up (or argue why you
think there should be a "depends on FSL_SOC || ...").
> + help
> + Selecting this option will add support for the 16550-style serial
> + port hardware found on Freescale SoCs.
>
> config SERIAL_8250_DW
> tristate "Support for Synopsys DesignWare 8250 quirks"
Johan
Re: [PATCH v3 4/8] powerpc/pseries/svm: Add a powerpc version of cc_platform_has()
On Wed, Sep 15, 2021 at 07:18:34PM +0200, Christophe Leroy wrote: > Could you please provide more explicit explanation why inlining such an > helper is considered as bad practice and messy ? Because now we get architectures to all subly differ. Look at the mess for ioremap and the ioremap* variant. The only good reason to allow for inlines if if they are used in a hot path. Which cc_platform_has is not, especially not on powerpc.
Re: [PATCH] powerpc: warn on emulation of dcbz instruction
Le 16/09/2021 à 09:16, Benjamin Herrenschmidt a écrit : On Thu, 2021-09-16 at 17:15 +1000, Benjamin Herrenschmidt wrote: On Wed, 2021-09-15 at 16:31 +0200, Christophe Leroy wrote: dcbz instruction shouldn't be used on non-cached memory. Using it on non-cached memory can result in alignment exception and implies a heavy handling. Instead of silentely emulating the instruction and resulting in high performance degradation, warn whenever an alignment exception is taken due to dcbz, so that the user is made aware that dcbz instruction has been used unexpectedly. Reported-by: Stan Johnson Cc: Finn Thain Signed-off-by: Christophe Leroy --- arch/powerpc/kernel/align.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c index bbb4181621dd..adc3a4a9c6e4 100644 --- a/arch/powerpc/kernel/align.c +++ b/arch/powerpc/kernel/align.c @@ -349,6 +349,7 @@ int fix_alignment(struct pt_regs *regs) if (op.type != CACHEOP + DCBZ) return -EINVAL; PPC_WARN_ALIGNMENT(dcbz, regs); + WARN_ON_ONCE(1); This is heavy handed ... It will be treated as an oops by various things uselessly spit out a kernel backtrace. Isn't PPC_WARN_ALIGNMENT enough ? PPC_WARN_ALIGNMENT() only warns if explicitely activated, I want to catch uses on 'dcbz' on non-cached memory all the time as they are most often the result of using memset() instead of memset_io(). Ah I saw your other one about fbdev... Ok what about you do that in a if (!user_mode(regs)) ? Yes I can do WARN_ON_ONCE(!user_mode(regs)); instead. Indeed the kernel should not do that. Does userspace accesses non-cached memory directly ? Christophe
Re: [PATCH] powerpc: warn on emulation of dcbz instruction
On Thu, 2021-09-16 at 17:15 +1000, Benjamin Herrenschmidt wrote: > On Wed, 2021-09-15 at 16:31 +0200, Christophe Leroy wrote: > > dcbz instruction shouldn't be used on non-cached memory. Using > > it on non-cached memory can result in alignment exception and > > implies a heavy handling. > > > > Instead of silentely emulating the instruction and resulting in > > high > > performance degradation, warn whenever an alignment exception is > > taken due to dcbz, so that the user is made aware that dcbz > > instruction has been used unexpectedly. > > > > Reported-by: Stan Johnson > > Cc: Finn Thain > > Signed-off-by: Christophe Leroy > > --- > > arch/powerpc/kernel/align.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/powerpc/kernel/align.c > > b/arch/powerpc/kernel/align.c > > index bbb4181621dd..adc3a4a9c6e4 100644 > > --- a/arch/powerpc/kernel/align.c > > +++ b/arch/powerpc/kernel/align.c > > @@ -349,6 +349,7 @@ int fix_alignment(struct pt_regs *regs) > > if (op.type != CACHEOP + DCBZ) > > return -EINVAL; > > PPC_WARN_ALIGNMENT(dcbz, regs); > > + WARN_ON_ONCE(1); > > This is heavy handed ... It will be treated as an oops by various > things uselessly spit out a kernel backtrace. Isn't > PPC_WARN_ALIGNMENT > enough ? Ah I saw your other one about fbdev... Ok what about you do that in a if (!user_mode(regs)) ? Indeed the kernel should not do that. Cheers, Ben.
Re: [PATCH] powerpc: warn on emulation of dcbz instruction
On Wed, 2021-09-15 at 16:31 +0200, Christophe Leroy wrote: > dcbz instruction shouldn't be used on non-cached memory. Using > it on non-cached memory can result in alignment exception and > implies a heavy handling. > > Instead of silentely emulating the instruction and resulting in high > performance degradation, warn whenever an alignment exception is > taken due to dcbz, so that the user is made aware that dcbz > instruction has been used unexpectedly. > > Reported-by: Stan Johnson > Cc: Finn Thain > Signed-off-by: Christophe Leroy > --- > arch/powerpc/kernel/align.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/powerpc/kernel/align.c > b/arch/powerpc/kernel/align.c > index bbb4181621dd..adc3a4a9c6e4 100644 > --- a/arch/powerpc/kernel/align.c > +++ b/arch/powerpc/kernel/align.c > @@ -349,6 +349,7 @@ int fix_alignment(struct pt_regs *regs) > if (op.type != CACHEOP + DCBZ) > return -EINVAL; > PPC_WARN_ALIGNMENT(dcbz, regs); > + WARN_ON_ONCE(1); This is heavy handed ... It will be treated as an oops by various things uselessly spit out a kernel backtrace. Isn't PPC_WARN_ALIGNMENT enough ? Ben.
Re: [PATCH v4] lockdown,selinux: fix wrong subject in some SELinux lockdown checks
On Thu, Sep 16, 2021 at 4:59 AM Paul Moore wrote:
> On Mon, Sep 13, 2021 at 5:05 PM Paul Moore wrote:
> >
> > On Mon, Sep 13, 2021 at 10:02 AM Ondrej Mosnacek
> > wrote:
> > >
> > > Commit 59438b46471a ("security,lockdown,selinux: implement SELinux
> > > lockdown") added an implementation of the locked_down LSM hook to
> > > SELinux, with the aim to restrict which domains are allowed to perform
> > > operations that would breach lockdown.
> > >
> > > However, in several places the security_locked_down() hook is called in
> > > situations where the current task isn't doing any action that would
> > > directly breach lockdown, leading to SELinux checks that are basically
> > > bogus.
> > >
> > > To fix this, add an explicit struct cred pointer argument to
> > > security_lockdown() and define NULL as a special value to pass instead
> > > of current_cred() in such situations. LSMs that take the subject
> > > credentials into account can then fall back to some default or ignore
> > > such calls altogether. In the SELinux lockdown hook implementation, use
> > > SECINITSID_KERNEL in case the cred argument is NULL.
> > >
> > > Most of the callers are updated to pass current_cred() as the cred
> > > pointer, thus maintaining the same behavior. The following callers are
> > > modified to pass NULL as the cred pointer instead:
> > > 1. arch/powerpc/xmon/xmon.c
> > > Seems to be some interactive debugging facility. It appears that
> > > the lockdown hook is called from interrupt context here, so it
> > > should be more appropriate to request a global lockdown decision.
> > > 2. fs/tracefs/inode.c:tracefs_create_file()
> > > Here the call is used to prevent creating new tracefs entries when
> > > the kernel is locked down. Assumes that locking down is one-way -
> > > i.e. if the hook returns non-zero once, it will never return zero
> > > again, thus no point in creating these files. Also, the hook is
> > > often called by a module's init function when it is loaded by
> > > userspace, where it doesn't make much sense to do a check against
> > > the current task's creds, since the task itself doesn't actually
> > > use the tracing functionality (i.e. doesn't breach lockdown), just
> > > indirectly makes some new tracepoints available to whoever is
> > > authorized to use them.
> > > 3. net/xfrm/xfrm_user.c:copy_to_user_*()
> > > Here a cryptographic secret is redacted based on the value returned
> > > from the hook. There are two possible actions that may lead here:
> > > a) A netlink message XFRM_MSG_GETSA with NLM_F_DUMP set - here the
> > > task context is relevant, since the dumped data is sent back to
> > > the current task.
> > > b) When adding/deleting/updating an SA via XFRM_MSG_xxxSA, the
> > > dumped SA is broadcasted to tasks subscribed to XFRM events -
> > > here the current task context is not relevant as it doesn't
> > > represent the tasks that could potentially see the secret.
> > > It doesn't seem worth it to try to keep using the current task's
> > > context in the a) case, since the eventual data leak can be
> > > circumvented anyway via b), plus there is no way for the task to
> > > indicate that it doesn't care about the actual key value, so the
> > > check could generate a lot of "false alert" denials with SELinux.
> > > Thus, let's pass NULL instead of current_cred() here faute de
> > > mieux.
> > >
> > > Improvements-suggested-by: Casey Schaufler
> > > Improvements-suggested-by: Paul Moore
> > > Fixes: 59438b46471a ("security,lockdown,selinux: implement SELinux
> > > lockdown")
> > > Acked-by: Dan Williams [cxl]
> > > Acked-by: Steffen Klassert [xfrm]
> > > Signed-off-by: Ondrej Mosnacek
> > > ---
> > >
> > > v4:
> > > - rebase on top of TODO
> > > - fix rebase conflicts:
> > > * drivers/cxl/pci.c
> > > - trivial: the lockdown reason was corrected in mainline
> > > * kernel/bpf/helpers.c, kernel/trace/bpf_trace.c
> > > - trivial: LOCKDOWN_BPF_READ was renamed to LOCKDOWN_BPF_READ_KERNEL
> > > in mainline
> > > * kernel/power/hibernate.c
> > > - trivial: !secretmem_active() was added to the condition in
> > > hibernation_available()
> > > - cover new security_locked_down() call in kernel/bpf/helpers.c
> > > (LOCKDOWN_BPF_WRITE_USER in BPF_FUNC_probe_write_user case)
> > >
> > > v3:
> > > https://lore.kernel.org/lkml/20210616085118.1141101-1-omosn...@redhat.com/
> > > - add the cred argument to security_locked_down() and adapt all callers
> > > - keep using current_cred() in BPF, as the hook calls have been shifted
> > > to program load time (commit ff40e51043af ("bpf, lockdown, audit: Fix
> > > buggy SELinux lockdown permission checks"))
> > > - in SELinux, don't ignore hook calls where cred == NULL, but use
> > > SECINITSID_KERNEL as the subject instead
> > > - update explanations in
