* Re: [RFC 1/5] seq_file: introduce seq_open_data helper
From: Andreas Dilger @ 2018-03-01 23:44 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Jonathan Corbet, Alexander Viro, Tony Luck, linux-ia64,
Michael Ellerman, linuxppc-dev, linux-doc, linux-kernel,
linux-fsdevel
In-Reply-To: <20180301233724.20440-1-linux@rasmusvillemoes.dk>
[-- Attachment #1: Type: text/plain, Size: 4152 bytes --]
On Mar 1, 2018, at 4:37 PM, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
>
> There are quite a few callers of seq_open that could be simplified by
> setting the ->private member via the seq_open call instead of fetching
> file->private_data afterwards.
>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
> I've just included a few examples of possible users of this helper,
> there are many more similar cases. As a bonus, the first two fix
> potential NULL derefs (if one believes that seq_open can actually
> fail).
>
> seq_open_private would have been a better name, but that one is
> already taken...
>
> Documentation/filesystems/seq_file.txt | 9 +++++----
> fs/seq_file.c | 9 ++++++++-
> include/linux/seq_file.h | 1 +
> 3 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/filesystems/seq_file.txt b/Documentation/filesystems/seq_file.txt
> index 9de4303201e1..68571b8275d8 100644
> --- a/Documentation/filesystems/seq_file.txt
> +++ b/Documentation/filesystems/seq_file.txt
> @@ -234,10 +234,11 @@ Here, the call to seq_open() takes the seq_operations structure we created
> before, and gets set up to iterate through the virtual file.
>
> On a successful open, seq_open() stores the struct seq_file pointer in
> -file->private_data. If you have an application where the same iterator can
> -be used for more than one file, you can store an arbitrary pointer in the
> -private field of the seq_file structure; that value can then be retrieved
> -by the iterator functions.
> +file->private_data. If you have an application where the same iterator
> +can be used for more than one file, you can store an arbitrary pointer
> +in the private field of the seq_file structure; that value can then be
> +retrieved by the iterator functions. Using the wrapper seq_open_data()
> +allows you to set the initial value for that field.
>
> There is also a wrapper function to seq_open() called seq_open_private(). It
> kmallocs a zero filled block of memory and stores a pointer to it in the
> diff --git a/fs/seq_file.c b/fs/seq_file.c
> index eea09f6d8830..f2145cb6e23d 100644
> --- a/fs/seq_file.c
> +++ b/fs/seq_file.c
> @@ -45,7 +45,7 @@ static void *seq_buf_alloc(unsigned long size)
> * Note: seq_open() will allocate a struct seq_file and store its
> * pointer in @file->private_data. This pointer should not be modified.
> */
> -int seq_open(struct file *file, const struct seq_operations *op)
> +int seq_open_data(struct file *file, const struct seq_operations *op, void *data)
> {
> struct seq_file *p;
>
> @@ -59,6 +59,7 @@ int seq_open(struct file *file, const struct seq_operations *op)
>
> mutex_init(&p->lock);
> p->op = op;
> + p->private = data;
>
> // No refcounting: the lifetime of 'p' is constrained
> // to the lifetime of the file.
> @@ -85,6 +86,12 @@ int seq_open(struct file *file, const struct seq_operations *op)
> }
> EXPORT_SYMBOL(seq_open);
>
> +int seq_open(struct file *file, const struct seq_operations *op)
> +{
> + return seq_open_data(file, op, NULL);
> +}
> +EXPORT_SYMBOL(seq_open_data);
This is a bit confusing. You export "seq_open" after seq_open_data(),
and export "seq_open_data" here after seq_open(). Not strictly a bug,
but could become one in the future.
Cheers, Andreas
> +
> static int traverse(struct seq_file *m, loff_t offset)
> {
> loff_t pos = 0, index;
> diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h
> index ab437dd2e3b9..f5ff376fa62b 100644
> --- a/include/linux/seq_file.h
> +++ b/include/linux/seq_file.h
> @@ -107,6 +107,7 @@ void seq_pad(struct seq_file *m, char c);
>
> char *mangle_path(char *s, const char *p, const char *esc);
> int seq_open(struct file *, const struct seq_operations *);
> +int seq_open_data(struct file *, const struct seq_operations *, void *);
> ssize_t seq_read(struct file *, char __user *, size_t, loff_t *);
> loff_t seq_lseek(struct file *, loff_t, int);
> int seq_release(struct inode *, struct file *);
> --
> 2.15.1
>
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply
* [RFC 3/5] powerpc/pseries: use seq_open_data in hcall_inst_seq_open
From: Rasmus Villemoes @ 2018-03-01 23:37 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: Rasmus Villemoes, linuxppc-dev, linux-kernel
In-Reply-To: <20180301233724.20440-1-linux@rasmusvillemoes.dk>
This code should check the return value of seq_open(); if it failed,
file->private_data is NULL. But we can avoid the issue entirely and
simplify the code by letting seq_open_data() set the ->private member.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
arch/powerpc/platforms/pseries/hvCall_inst.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/hvCall_inst.c b/arch/powerpc/platforms/pseries/hvCall_inst.c
index 89b7ce807e70..05990c9fe264 100644
--- a/arch/powerpc/platforms/pseries/hvCall_inst.c
+++ b/arch/powerpc/platforms/pseries/hvCall_inst.c
@@ -92,14 +92,7 @@ static const struct seq_operations hcall_inst_seq_ops = {
static int hcall_inst_seq_open(struct inode *inode, struct file *file)
{
- int rc;
- struct seq_file *seq;
-
- rc = seq_open(file, &hcall_inst_seq_ops);
- seq = file->private_data;
- seq->private = file_inode(file)->i_private;
-
- return rc;
+ return seq_open_data(file, &hcall_inst_seq_ops, file_inode(file)->i_private);
}
static const struct file_operations hcall_inst_seq_fops = {
--
2.15.1
^ permalink raw reply related
* [RFC 1/5] seq_file: introduce seq_open_data helper
From: Rasmus Villemoes @ 2018-03-01 23:37 UTC (permalink / raw)
To: Jonathan Corbet, Alexander Viro
Cc: Rasmus Villemoes, Tony Luck, linux-ia64, Michael Ellerman,
linuxppc-dev, linux-doc, linux-kernel, linux-fsdevel
There are quite a few callers of seq_open that could be simplified by
setting the ->private member via the seq_open call instead of fetching
file->private_data afterwards.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
I've just included a few examples of possible users of this helper,
there are many more similar cases. As a bonus, the first two fix
potential NULL derefs (if one believes that seq_open can actually
fail).
seq_open_private would have been a better name, but that one is
already taken...
Documentation/filesystems/seq_file.txt | 9 +++++----
fs/seq_file.c | 9 ++++++++-
include/linux/seq_file.h | 1 +
3 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/Documentation/filesystems/seq_file.txt b/Documentation/filesystems/seq_file.txt
index 9de4303201e1..68571b8275d8 100644
--- a/Documentation/filesystems/seq_file.txt
+++ b/Documentation/filesystems/seq_file.txt
@@ -234,10 +234,11 @@ Here, the call to seq_open() takes the seq_operations structure we created
before, and gets set up to iterate through the virtual file.
On a successful open, seq_open() stores the struct seq_file pointer in
-file->private_data. If you have an application where the same iterator can
-be used for more than one file, you can store an arbitrary pointer in the
-private field of the seq_file structure; that value can then be retrieved
-by the iterator functions.
+file->private_data. If you have an application where the same iterator
+can be used for more than one file, you can store an arbitrary pointer
+in the private field of the seq_file structure; that value can then be
+retrieved by the iterator functions. Using the wrapper seq_open_data()
+allows you to set the initial value for that field.
There is also a wrapper function to seq_open() called seq_open_private(). It
kmallocs a zero filled block of memory and stores a pointer to it in the
diff --git a/fs/seq_file.c b/fs/seq_file.c
index eea09f6d8830..f2145cb6e23d 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -45,7 +45,7 @@ static void *seq_buf_alloc(unsigned long size)
* Note: seq_open() will allocate a struct seq_file and store its
* pointer in @file->private_data. This pointer should not be modified.
*/
-int seq_open(struct file *file, const struct seq_operations *op)
+int seq_open_data(struct file *file, const struct seq_operations *op, void *data)
{
struct seq_file *p;
@@ -59,6 +59,7 @@ int seq_open(struct file *file, const struct seq_operations *op)
mutex_init(&p->lock);
p->op = op;
+ p->private = data;
// No refcounting: the lifetime of 'p' is constrained
// to the lifetime of the file.
@@ -85,6 +86,12 @@ int seq_open(struct file *file, const struct seq_operations *op)
}
EXPORT_SYMBOL(seq_open);
+int seq_open(struct file *file, const struct seq_operations *op)
+{
+ return seq_open_data(file, op, NULL);
+}
+EXPORT_SYMBOL(seq_open_data);
+
static int traverse(struct seq_file *m, loff_t offset)
{
loff_t pos = 0, index;
diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h
index ab437dd2e3b9..f5ff376fa62b 100644
--- a/include/linux/seq_file.h
+++ b/include/linux/seq_file.h
@@ -107,6 +107,7 @@ void seq_pad(struct seq_file *m, char c);
char *mangle_path(char *s, const char *p, const char *esc);
int seq_open(struct file *, const struct seq_operations *);
+int seq_open_data(struct file *, const struct seq_operations *, void *);
ssize_t seq_read(struct file *, char __user *, size_t, loff_t *);
loff_t seq_lseek(struct file *, loff_t, int);
int seq_release(struct inode *, struct file *);
--
2.15.1
^ permalink raw reply related
* [PATCH] powerpc: boot: add strrchr function
From: Rob Herring @ 2018-03-01 15:26 UTC (permalink / raw)
To: Michael Ellerman
Cc: linux-kernel, linuxppc-dev, Benjamin Herrenschmidt,
Paul Mackerras
libfdt gained a new dependency on strrchr, so copy the implementation
from lib/string.c. Most of the string functions are in assembly, but
stdio.c already has strnlen, so add strrchr there.
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Rob Herring <robh@kernel.org>
---
Please ack. This is a dependency for dtc/libfdt sync with upstream.
arch/powerpc/boot/stdio.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/powerpc/boot/stdio.c b/arch/powerpc/boot/stdio.c
index a701261b1781..98042eff7b26 100644
--- a/arch/powerpc/boot/stdio.c
+++ b/arch/powerpc/boot/stdio.c
@@ -21,6 +21,16 @@ size_t strnlen(const char * s, size_t count)
return sc - s;
}
+char *strrchr(const char *s, int c)
+{
+ const char *last = NULL;
+ do {
+ if (*s == (char)c)
+ last = s;
+ } while (*s++);
+ return (char *)last;
+}
+
#ifdef __powerpc64__
# define do_div(n, base) ({ \
--
2.14.1
^ permalink raw reply related
* Re: [PATCH] powerpc: dts: replace 'linux, stdout-path' with 'stdout-path'
From: Rob Herring @ 2018-03-01 13:39 UTC (permalink / raw)
To: Michael Ellerman
Cc: devicetree, linux-kernel@vger.kernel.org, Mark Rutland,
Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev
In-Reply-To: <87606ga8bv.fsf@concordia.ellerman.id.au>
On Wed, Feb 28, 2018 at 7:33 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Rob Herring <robh@kernel.org> writes:
>
>> 'linux,stdout-path' has been deprecated for some time in favor of
>> 'stdout-path'. Now dtc will warn on occurrences of 'linux,stdout-path'.
>> Search and replace all the of occurrences with 'stdout-path'.
>
> This patch looks OK.
>
> But please remember that not all device trees are generated with dtc, we
> still have machines in the wild that have firmware which use
> "linux,stdout-path" and may never be updated.
Absolutely. The core code still supports both.
The only scenario I could come up with is someone has an old
bootloader that only understands linux,stdout-path and modifies it and
they update the dtb. We may end up with both properties with
stdout-path taking preference.
Rob
^ permalink raw reply
* Re: [RFC PATCH 1/6] powerpc: Add security feature flags for Spectre/Meltdown
From: Michael Ellerman @ 2018-03-01 13:20 UTC (permalink / raw)
To: Daniel Axtens, linuxppc-dev
In-Reply-To: <87606geir8.fsf@linkitivity.dja.id.au>
Daniel Axtens <dja@axtens.net> writes:
> Michael Ellerman <mpe@ellerman.id.au> writes:
>> diff --git a/arch/powerpc/include/asm/security_features.h b/arch/powerpc/include/asm/security_features.h
>> new file mode 100644
>> index 000000000000..3b690de8b0e8
>> --- /dev/null
>> +++ b/arch/powerpc/include/asm/security_features.h
>> @@ -0,0 +1,65 @@
...
>> +// Features indicating support for Spectre/Meltdown mitigations
>> +
>> +// The L1-D cache can be flushed with ori r30,r30,0
>> +#define SEC_FTR_L1D_FLUSH_ORI30 0x0000000000000001ull
>> +
>> +// The L1-D cache can be flushed with mtspr 882,r0 (aka SPRN_TRIG2ull
> This looks like some sort of search-replace gone wrong? ------------^^^^
Sure is! Oops.
Thanks for reviewing.
cheers
^ permalink raw reply
* Re: [RFC PATCH 1/6] powerpc: Add security feature flags for Spectre/Meltdown
From: Michael Ellerman @ 2018-03-01 13:19 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <20180228150427.GA21977@gate.crashing.org>
Segher Boessenkool <segher@kernel.crashing.org> writes:
> Hi!
>
> On Thu, Mar 01, 2018 at 01:53:11AM +1100, Michael Ellerman wrote:
>> +// A speculation barrier should be used for bounds checks (Spectre variant 1ull
>
> s/1ull/1)/ ?
Haha, oops. Thanks for spotting it.
Result of replacing ASM_CONST(x) with xull.
cheers
^ permalink raw reply
* Re: [PATCH v2] powerpc/npu-dma.c: Fix deadlock in mmio_invalidate
From: Michael Ellerman @ 2018-03-01 13:16 UTC (permalink / raw)
To: Alistair Popple; +Cc: linuxppc-dev, Balbir Singh, Mark Hairgrove
In-Reply-To: <7309481.ksyhQnkyZL@new-mexico>
Alistair Popple <alistair@popple.id.au> writes:
>> > diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
>> > index 0a253b64ac5f..2fed4b116e19 100644
>> > --- a/arch/powerpc/platforms/powernv/npu-dma.c
>> > +++ b/arch/powerpc/platforms/powernv/npu-dma.c
>> > @@ -726,7 +749,7 @@ struct npu_context *pnv_npu2_init_context(struct pci_dev *gpdev,
>> > if (WARN_ON(of_property_read_u32(nvlink_dn, "ibm,npu-link-index",
>> > &nvlink_index)))
>> > return ERR_PTR(-ENODEV);
>> > - npu_context->npdev[npu->index][nvlink_index] = npdev;
>> > + WRITE_ONCE(npu_context->npdev[npu->index][nvlink_index], npdev);
>>
>> When you're publishing a struct via a pointer you would typically have a
>> barrier between the stores that set the fields of the struct, and the
>> store that publishes the struct. Otherwise the reader can see a
>> partially setup struct.
>
> npdev is just a pointer to a pci_dev setup by the PCI code. We assign it to
> npdev[][] to indicate to the mmu notifiers that an invalidation should also
> be sent to this nvlink. However I don't think there is any ordering
> required wrt to the rest of the npu_context setup - so long as when the
> notifiers deference npu_context->npdev[i][j] it either sees a valid
> non-NULL pointer or NULL assigned to npdev[][] everything should be ok.
Yeah OK. You do then dereference npdev->bus, so you depend on that being
setup prior to the store that makes the npdev visible. But I guess we're
saying that happened eons ago somewhere in the PCI code and will have
happened by now.
>> I think here the npdev was setup somewhere else, and maybe there has
>> been an intervening barrier, but it's not clear. A comment at least
>> would be good.
>
> Yes it has been. I will submit a v3 with some more comments incorporating
> the above. Unless you think my argument is wrong?
If you can comment it then I think I'm happy with it.
>> In general I feel like a spinlock or two could significantly simply the
>> locking/ordering in this code, and given we're doing MMIOs anyway would
>> not affect performance.
>
> Indeed, I am working on a patch to add a spinlock around the allocation of
> the npu_context as pnv_npu2_destroy_context() needs to be serialised with
> respect to pnv_npu2_init_context() on the same mm_struct. I'd incorrectly
> assumed the driver would do this serialisation but apparently it can't so
> we need to ensure kref_get(npu_context) can't race with it's destruction
> (concurrent init_context() is protected by mmap_sem). I could fold that
> (unposted) patch into this one if you think that would be better?
No this is big enough already, I almost asked you to split it to make
the diff more readable :)
So just send the spinlock patch when it's ready as a separate patch.
cheers
^ permalink raw reply
* Re: [PATCH 3/3] powerpc/64s/idle: POWER9 ESL=0 stop avoid save/restore overhead
From: Nicholas Piggin @ 2018-03-01 11:57 UTC (permalink / raw)
To: Vaidyanathan Srinivasan; +Cc: linuxppc-dev, Gautham R . Shenoy, Paul Mackerras
In-Reply-To: <20180228183439.GC20168@drishya.in.ibm.com>
On Thu, 1 Mar 2018 00:04:39 +0530
Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> wrote:
> * Nicholas Piggin <npiggin@gmail.com> [2017-11-18 00:08:07]:
>
> > When stop is executed with EC=ESL=0, it appears to execute like a
> > normal instruction (resuming from NIP when woken by interrupt). So all
> > the save/restore handling can be avoided completely. In particular NV
> > GPRs do not have to be saved, and MSR does not have to be switched
> > back to kernel MSR.
> >
> > So move the test for EC=ESL=0 sleep states out to power9_idle_stop,
> > and return directly to the caller after stop in that case. The mtspr
> > to PSSCR is moved to the top of power9_offline_stop just so it matches
> > power9_idle_stop.
> >
> > This improves performance for ping-pong benchmark with the stop0_lite
> > idle state by 2.54% for 2 threads in the same core, and 2.57% for
> > different cores.
> >
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>
> Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
>
> > ---
> > arch/powerpc/kernel/idle_book3s.S | 43 +++++++++++------------------------
> > arch/powerpc/platforms/powernv/idle.c | 7 +++++-
> > 2 files changed, 19 insertions(+), 31 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> > index 07a306173c5a..6243da99b26c 100644
> > --- a/arch/powerpc/kernel/idle_book3s.S
> > +++ b/arch/powerpc/kernel/idle_book3s.S
> > @@ -324,31 +324,8 @@ enter_winkle:
> > /*
> > * r3 - PSSCR value corresponding to the requested stop state.
> > */
> > -power_enter_stop:
> > -/*
> > - * Check if we are executing the lite variant with ESL=EC=0
> > - */
> > - andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
> > +power_enter_stop_esl:
> > clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */
> > - bne .Lhandle_esl_ec_set
> > - PPC_STOP
> > - li r3,0 /* Since we didn't lose state, return 0 */
> > -
> > - /*
> > - * pnv_wakeup_noloss() expects r12 to contain the SRR1 value so
> > - * it can determine if the wakeup reason is an HMI in
> > - * CHECK_HMI_INTERRUPT.
> > - *
> > - * However, when we wakeup with ESL=0, SRR1 will not contain the wakeup
> > - * reason, so there is no point setting r12 to SRR1.
> > - *
> > - * Further, we clear r12 here, so that we don't accidentally enter the
> > - * HMI in pnv_wakeup_noloss() if the value of r12[42:45] == WAKE_HMI.
> > - */
> > - li r12, 0
> > - b pnv_wakeup_noloss
> > -
> > -.Lhandle_esl_ec_set:
> > BEGIN_FTR_SECTION
> > /*
> > * POWER9 DD2.0 or earlier can incorrectly set PMAO when waking up after
> > @@ -423,26 +400,32 @@ ALT_FTR_SECTION_END_NESTED_IFSET(CPU_FTR_ARCH_207S, 66); \
> > * r3 contains desired PSSCR register value.
> > */
> > _GLOBAL(power9_idle_stop)
> > - std r3, PACA_REQ_PSSCR(r13)
> > mtspr SPRN_PSSCR,r3
> > - LOAD_REG_ADDR(r4,power_enter_stop)
> > + andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
> > + bne 1f
> > + PPC_STOP
> > + li r3,0 /* Since we didn't lose state, return 0 */
> > + blr
> > +
> > +1: std r3, PACA_REQ_PSSCR(r13)
> > + LOAD_REG_ADDR(r4,power_enter_stop_esl)
> > b pnv_powersave_common
> > /* No return */
>
> Good optimization to skip the context save and directly execute stop
> for ESL=EC=0 case.
Yep we should now be getting pretty close to optimal for ESL=EC=0.
About the only other thing we could do is maintain PSSCR state and
only change it when going to a different idle state. May not be
worth worrying about but I haven't benchmarked it.
>
> > /*
> > - * Entered with MSR[EE]=0 and no soft-masked interrupts pending.
> > - * r3 contains desired PSSCR register value.
> > + * This is the same as the above, but it sets KVM state for secondaries,
> > + * and it must have PSSCR[EC]=1
> > */
> > _GLOBAL(power9_offline_stop)
> > - std r3, PACA_REQ_PSSCR(r13)
> > mtspr SPRN_PSSCR,r3
> > + std r3, PACA_REQ_PSSCR(r13)
> > #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> > /* Tell KVM we're entering idle */
> > li r4,KVM_HWTHREAD_IN_IDLE
> > /* DO THIS IN REAL MODE! See comment above. */
> > stb r4,HSTATE_HWTHREAD_STATE(r13)
> > #endif
> > - LOAD_REG_ADDR(r4,power_enter_stop)
> > + LOAD_REG_ADDR(r4,power_enter_stop_esl)
> > b pnv_powersave_common
> > /* No return */
> >
> > diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> > index a921d5428d76..610b1637c16f 100644
> > --- a/arch/powerpc/platforms/powernv/idle.c
> > +++ b/arch/powerpc/platforms/powernv/idle.c
> > @@ -621,7 +621,12 @@ static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags,
> > continue;
> > }
> >
> > - if (max_residency_ns < residency_ns[i]) {
> > + /*
> > + * Deepest stop for unplug must be PSSCR[EC]=1 (wakeup at
> > + * 0x100.
> > + */
> > + if ((max_residency_ns < residency_ns[i])&&
> > + (psscr_val[i] & PSSCR_EC)) {
> > max_residency_ns = residency_ns[i];
> > pnv_deepest_stop_psscr_val = psscr_val[i];
> > pnv_deepest_stop_psscr_mask = psscr_mask[i];
>
> If firmware did not provide any ESL=EC=1 state, we can still leave
> threads in stop ESL=0 state. This is just a corner case or random
> test scenario. Why do we want to enforce that offline cpus really use
> a ESL=0 state or just spin?
It's because power9_offline_stop only has cases for EC=ESL=1
states now.
It actually looks like EC=ESL=0 unplug today is broken KVM, because
the wakeup side does not check HWTHREAD_REQ, and yet they do set
HWTHREAD_IN_IDLE. That would probably hang in KVM if we run with
dependent threads, wouldn't it?
I think banning it for now should be okay.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 2/3] powerpc/64s/idle: avoid sync for KVM state when waking from idle
From: Nicholas Piggin @ 2018-03-01 11:38 UTC (permalink / raw)
To: Vaidyanathan Srinivasan; +Cc: linuxppc-dev, Gautham R . Shenoy, Paul Mackerras
In-Reply-To: <20180228181623.GA20168@drishya.in.ibm.com>
On Wed, 28 Feb 2018 23:46:23 +0530
Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> wrote:
> * Nicholas Piggin <npiggin@gmail.com> [2017-11-18 00:08:06]:
>
> > When waking from a CPU idle instruction (e.g., nap or stop), the sync
> > for ordering the KVM secondary thread state can be avoided if there
> > wakeup is coming from a kernel context rather than KVM context.
> >
> > This improves performance for ping-pong benchmark with the stop0 idle
> > state by 0.46% for 2 threads in the same core, and 1.02% for different
> > cores.
>
> Cool, the improvement comes from avoiding the "sync" alone?
Yes, they can be pretty costly.
>
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> > arch/powerpc/kernel/idle_book3s.S | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> > index 2f8364e7b489..07a306173c5a 100644
> > --- a/arch/powerpc/kernel/idle_book3s.S
> > +++ b/arch/powerpc/kernel/idle_book3s.S
> > @@ -532,6 +532,9 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_ARCH_300)
> > mr r3,r12
> >
> > #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> > + lbz r0,HSTATE_HWTHREAD_STATE(r13)
> > + cmpwi r0,KVM_HWTHREAD_IN_KERNEL
> > + beq 1f
> > li r0,KVM_HWTHREAD_IN_KERNEL
> > stb r0,HSTATE_HWTHREAD_STATE(r13)
> > /* Order setting hwthread_state vs. testing hwthread_req */
>
> With this change, we will not check for HSTATE_HWTHREAD_REQ != 0
> condition but unconditionally goto host kernel if
> HSTATE_HWTHREAD_STATE == KVM_HWTHREAD_IN_KERNEL at wakeup.
That's right.
> Host is in ST mode and sibling thread got a wakeup event (door bell) to execute
> a new vcpu by calling kvm_start_guest, what will HSTATE_HWTHREAD_STATE be?
It should be KVM_HWTHREAD_IN_IDLE.
> Just to clarify, what will the flags looks like for
>
> (a) Host cpu sibling thread is offline and need to execute guest
> (b) Host cpu sibling thread is idle and need to execute guest
In the idle case we are running with independent threads mode and
siblings not unplugged, so we should not get KVM wake-up requests
come through this path. I'm not fluent in KVM though, so I could
be wrong.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 01/21] powerpc: Remove warning on array size when empty
From: Michael Ellerman @ 2018-03-01 9:33 UTC (permalink / raw)
To: Mathieu Malaterre
Cc: Benjamin Herrenschmidt, Paul Mackerras, Jiri Slaby, linuxppc-dev,
LKML
In-Reply-To: <CA+7wUsxsZrjHY8=TSUiMEgGMm72XyQRW47b7oqNjFGeFKLMd+w@mail.gmail.com>
Mathieu Malaterre <malat@debian.org> writes:
> On Thu, Mar 1, 2018 at 3:55 AM, Michael Ellerman <mpe@ellerman.id.au> wro=
te:
>> Mathieu Malaterre <malat@debian.org> writes:
>>
>>> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined,=
the
>>> array feature_properties is defined as an empty array, which in turn
>>> triggers the following warning (treated as error on W=3D1):
>>>
>>> CC arch/powerpc/kernel/prom.o
>>> arch/powerpc/kernel/prom.c: In function =E2=80=98check_cpu_feature_prop=
erties=E2=80=99:
>>> arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expres=
sion < 0 is always false [-Werror=3Dtype-limits]
>>> for (i =3D 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>>> ^
>>> cc1: all warnings being treated as errors
>>
>> Ugh, that's annoying.
>>
>> This seems to work?
>>
>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
>> index 4dffef947b8a..5215119e249c 100644
>> --- a/arch/powerpc/kernel/prom.c
>> +++ b/arch/powerpc/kernel/prom.c
>> @@ -291,11 +291,11 @@ static inline void identical_pvr_fixup(unsigned lo=
ng node)
>>
>> static void __init check_cpu_feature_properties(unsigned long node)
>> {
>> - unsigned long i;
>> struct feature_property *fp =3D feature_properties;
>> const __be32 *prop;
>> + int i;
>>
>> - for (i =3D 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>> + for (i =3D 0; i < (int)ARRAY_SIZE(feature_properties); ++i, ++fp=
) {
>> prop =3D of_get_flat_dt_prop(node, fp->name, NULL);
>> if (prop && be32_to_cpup(prop) >=3D fp->min_value) {
>> cur_cpu_spec->cpu_features |=3D fp->cpu_feature;
>>
>
> Indeed that looks like the less invasive solution, I'll re-submit.
Thanks.
> Should I resubmit the entire patch series (21 indep patches) or
> re-submit only the 3 patches that were discussed (as part of a
> different series) ?
Just resubmit the ones that need changes. You can either send them as a
new series of 3, or post each as a reply to the original patch with v2
in the subject.
cheers
^ permalink raw reply
* Re: [RFC REBASED 5/5] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations
From: Nicholas Piggin @ 2018-03-01 9:22 UTC (permalink / raw)
To: Christophe LEROY
Cc: Aneesh Kumar K.V, Michael Ellerman, linux-kernel, linuxppc-dev
In-Reply-To: <4e3c5281-427b-abe8-9c80-f2c1a0247e28@c-s.fr>
On Thu, 1 Mar 2018 08:09:55 +0100
Christophe LEROY <christophe.leroy@c-s.fr> wrote:
> Le 28/02/2018 à 07:53, Nicholas Piggin a écrit :
> > On Tue, 27 Feb 2018 18:11:07 +0530
> > "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> >
> >> Nicholas Piggin <npiggin@gmail.com> writes:
> >>
> >>> On Tue, 27 Feb 2018 14:31:07 +0530
> >>> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> >>>
> >>>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
> >>>>
> >>>>> The number of high slices a process might use now depends on its
> >>>>> address space size, and what allocation address it has requested.
> >>>>>
> >>>>> This patch uses that limit throughout call chains where possible,
> >>>>> rather than use the fixed SLICE_NUM_HIGH for bitmap operations.
> >>>>> This saves some cost for processes that don't use very large address
> >>>>> spaces.
> >>>>
> >>>> I haven't really looked at the final code. One of the issue we had was
> >>>> with the below scenario.
> >>>>
> >>>> mmap(addr, len) where addr < 128TB and addr+len > 128TB We want to make
> >>>> sure we build the mask such that we don't find the addr available.
> >>>
> >>> We should run it through the mmap regression tests. I *think* we moved
> >>> all of that logic from the slice code to get_ummapped_area before going
> >>> in to slices. I may have missed something though, it would be good to
> >>> have more eyes on it.
> >>>
> >>
> >> mmap(-1,...) failed with the test. Something like below fix it
> >>
> >> @@ -756,7 +770,7 @@ void slice_set_user_psize(struct mm_struct *mm, unsigned int psize)
> >> mm->context.low_slices_psize = lpsizes;
> >>
> >> hpsizes = mm->context.high_slices_psize;
> >> - high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
> >> + high_slices = SLICE_NUM_HIGH;
> >> for (i = 0; i < high_slices; i++) {
> >> mask_index = i & 0x1;
> >> index = i >> 1;
> >>
> >> I guess for everything in the mm_context_t, we should compute it till
> >> SLICE_NUM_HIGH. The reason for failure was, even though we recompute the
> >> slice mask cached in mm_context on slb_addr_limit, it was still derived
> >> from the high_slices_psizes which was computed with lower value.
> >
> > Okay thanks for catching that Aneesh. I guess that's a slow path so it
> > should be okay. Christophe if you're taking care of the series can you
> > fold it in? Otherwise I'll do that after yours gets merged.
> >
>
> I'm not really taking care of your serie, just took it once to see how
> it fits on the 8xx.
> I prefer if you can handle them. If you need my help for any test on
> PPC32 don't hesitate to ask.
No problem I can do that. Thanks for rebasing them.
Thanks,
Nick
^ permalink raw reply
* Re: [RFC][PATCH bpf] tools: bpftool: Fix tags for bpf-to-bpf calls
From: Naveen N. Rao @ 2018-03-01 8:51 UTC (permalink / raw)
To: Daniel Borkmann, Sandipan Das
Cc: ast, jakub.kicinski, linuxppc-dev, mpe, netdev
In-Reply-To: <4cdcc751-d830-51ce-23a0-62f773dc015e@iogearbox.net>
Daniel Borkmann wrote:
> On 02/27/2018 01:13 PM, Sandipan Das wrote:
>> With this patch, it will look like this:
>> 0: (85) call pc+2#bpf_prog_8f85936f29a7790a+3
>=20
> (Note the +2 is the insn->off already.)
>=20
>> 1: (b7) r0 =3D 1
>> 2: (95) exit
>> 3: (b7) r0 =3D 2
>> 4: (95) exit
>>=20
>> where 8f85936f29a7790a is the tag of the bpf program and 3 is
>> the offset to the start of the subprog from the start of the
>> program.
>=20
> The problem with this approach would be that right now the name is
> something like bpf_prog_5f76847930402518_F where the subprog tag is
> just a placeholder so in future, this may well adapt to e.g. the actual
> function name from the elf file. Note that when kallsyms is enabled
> then a name like bpf_prog_5f76847930402518_F will also appear in stack
> traces, perf records, etc, so for correlation/debugging it would really
> help to have them the same everywhere.
>=20
> Worst case if there's nothing better, potentially what one could do in
> bpf_prog_get_info_by_fd() is to dump an array of full addresses and
> have the imm part as the index pointing to one of them, just unfortunate
> that it's likely only needed in ppc64.
Ok. We seem to have discussed a few different aspects in this thread. =20
Let me summarize the different aspects we have discussed:
1. Passing address of JIT'ed function to the JIT engines:
Two approaches discussed:
a. Existing approach, where the subprog address is encoded as an=20
offset from __bpf_call_base() in imm32 field of the BPF call=20
instruction. This requires the JIT'ed function to be within 2GB of=20
__bpf_call_base(), which won't be true on ppc64, at the least. So,=20
this won't on ppc64 (and any other architectures where vmalloc'ed=20
(module_alloc()) memory is from a different, far, address range).
=20
[As a side note, is it _actually_ guaranteed that JIT'ed functions=20
will be within 2GB (signed 32-bit...) on all other architectures=20
where BPF JIT is supported? I'm not quite sure how memory allocation=20
works on other architectures, but it looks like this can fail if=20
there are other larger allocations.]
b. Pass the full 64-bit address of the call target in an auxiliary=20
field for the JIT engine to use (as implemented in this mail chain). =20
We can then use this to determine the call target if this is a=20
pseudo call.
There is a third option we can consider:
c. Convert BPF pseudo call instruction into a 2-instruction sequence=20
(similar to BPF_DW) and encode the full 64-bit call target in the=20
second bpf instruction. To distinguish this from other instruction=20
forms, we can set imm32 to -1.
If we go with (b) or (c), we will need to take a call on whether we=20
will implement this in the same manner across all architectures, or=20
if we should have ppc64 (and any other affected architectures) work=20
differently from the rest.
Further more, for (b), bpftool won't be able to derive the target=20
function call address, but approaches (a) and (c) are fine. More=20
about that below...
2. Indicating target function in bpftool:
In the existing approach, bpftool can determine target address since=20
the offset is encoded in imm32 and is able to lookup the name from=20
kallsyms, if enabled.
If we go with approach (b) for ppc64, this won't work and we will=20
have to minimally update bpftool to detect that the target address=20
is not available on ppc64.
If we go with approach (c), the target address will be available and=20
we should be able to update bpftool to look that up.
=20
[As a side note, I suppose part of Sandipan's point with the=20
previous patch was to make the bpftool output consistent whether or=20
not JIT is enabled. It does look a bit weird that bpftool shows the=20
address of a JIT'ed function when asked to print the BPF bytecode.]
Thoughts?
- Naveen
=
^ permalink raw reply
* Re: [PATCH v4] watchdog: add SPDX identifiers for watchdog subsystem
From: Marcus Folkesson @ 2018-03-01 7:24 UTC (permalink / raw)
To: Florian Fainelli
Cc: Wim Van Sebroeck, Guenter Roeck, Joel Stanley, Nicolas Ferre,
Alexandre Belloni, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Eric Anholt, Stefan Wahren,
Support Opensource, Baruch Siach, William Breathitt Gray,
Jimmy Vance, Keguang Zhang, Joachim Eastwood, Tomas Winkler,
Johannes Thumshirn, Andreas Werner, Carlo Caione, Kevin Hilman,
Matthias Brugger, Wan ZongShun, Michal Simek, Vladimir Zapolskiy,
Sylvain Lemieux, Kukjin Kim, Krzysztof Kozlowski, Zwane Mwaikambo,
Jim Cromie, Barry Song, Patrice Chotard, Maxime Ripard,
Chen-Yu Tsai, Marc Gonzalez, Mans Rullgard, Thierry Reding,
Jonathan Hunter, Masahiro Yamada, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Jun Nie, Baoyou Xie, Shawn Guo,
linux-watchdog, linux-kernel, linux-arm-kernel, linux-rpi-kernel,
adi-buildroot-devel, linux-mips, linux-amlogic, linux-mediatek,
linux-samsung-soc, linux-tegra, linuxppc-dev, patches
In-Reply-To: <786d740d-f29c-8f09-4648-95244c06e59b@gmail.com>
Florian,
On Wed, Feb 28, 2018 at 03:45:43PM -0800, Florian Fainelli wrote:
> On 02/28/2018 07:01 AM, Marcus Folkesson wrote:
> > - Add SPDX identifier
> > - Remove boiler plate license text
> > - If MODULE_LICENSE and boiler plate does not match, go for boiler plate
> > license
> >
> > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> > Acked-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> > Acked-by: Baruch Siach <baruch@tkos.co.il>
> > Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> > Acked-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
> > Acked-by: Johannes Thumshirn <jth@kernel.org>
> > Acked-by: Mans Rullgard <mans@mansr.com>
> > Acked-by: Matthias Brugger <matthias.bgg@gmail.com>
> > Acked-by: Michal Simek <michal.simek@xilinx.com>
> > Acked-by: Neil Armstrong <narmstrong@baylibre.com>
> > Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> > Acked-by: Thierry Reding <treding@nvidia.com>
> > Acked-by: Tomas Winkler <tomas.winkler@intel.com>
> > Acked-by: Patrice Chotard <patrice.chotard@st.com>
> > Acked-by: William Breathitt Gray <vilhelm.gray@gmail.com>
> > Reviewed-by: Eric Anholt <eric@anholt.net>
> > ---
> >
> > Notes:
> > v4:
> > - Drop coh901327_wdt since it allready is a pending patch
> > v3:
> > - Keep license text for ebc-c384_wdt
> > v2:
> > - Put back removed copyright texts for meson_gxbb_wdt and coh901327_wdt
> > - Change to BSD-3-Clause for meson_gxbb_wdt
> > v1: Please have an extra look at meson_gxbb_wdt.c
> >
>
> > drivers/watchdog/ar7_wdt.c | 14 +---------
>
> > drivers/watchdog/bcm2835_wdt.c | 5 +---
> > drivers/watchdog/bcm47xx_wdt.c | 5 +---
> > drivers/watchdog/bcm63xx_wdt.c | 5 +---
> > drivers/watchdog/bcm7038_wdt.c | 12 ++------
> > drivers/watchdog/bcm_kona_wdt.c | 9 +-----
>
> > drivers/watchdog/mtx-1_wdt.c | 11 +-------
>
> For these drivers above:
>
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
>
> and it looks like you missed rdc321x_wdt.c, is there a specific reason?
Good catch!
I will fix that, thank you.
> --
> Florian
Best regards
Marcus Folkesson
^ permalink raw reply
* Re: [RFC REBASED 5/5] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations
From: Christophe LEROY @ 2018-03-01 7:09 UTC (permalink / raw)
To: Nicholas Piggin, Aneesh Kumar K.V, Michael Ellerman
Cc: linux-kernel, linuxppc-dev
In-Reply-To: <20180228165331.6e09959d@roar.ozlabs.ibm.com>
Le 28/02/2018 à 07:53, Nicholas Piggin a écrit :
> On Tue, 27 Feb 2018 18:11:07 +0530
> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>
>> Nicholas Piggin <npiggin@gmail.com> writes:
>>
>>> On Tue, 27 Feb 2018 14:31:07 +0530
>>> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>>>
>>>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>>>
>>>>> The number of high slices a process might use now depends on its
>>>>> address space size, and what allocation address it has requested.
>>>>>
>>>>> This patch uses that limit throughout call chains where possible,
>>>>> rather than use the fixed SLICE_NUM_HIGH for bitmap operations.
>>>>> This saves some cost for processes that don't use very large address
>>>>> spaces.
>>>>
>>>> I haven't really looked at the final code. One of the issue we had was
>>>> with the below scenario.
>>>>
>>>> mmap(addr, len) where addr < 128TB and addr+len > 128TB We want to make
>>>> sure we build the mask such that we don't find the addr available.
>>>
>>> We should run it through the mmap regression tests. I *think* we moved
>>> all of that logic from the slice code to get_ummapped_area before going
>>> in to slices. I may have missed something though, it would be good to
>>> have more eyes on it.
>>>
>>
>> mmap(-1,...) failed with the test. Something like below fix it
>>
>> @@ -756,7 +770,7 @@ void slice_set_user_psize(struct mm_struct *mm, unsigned int psize)
>> mm->context.low_slices_psize = lpsizes;
>>
>> hpsizes = mm->context.high_slices_psize;
>> - high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
>> + high_slices = SLICE_NUM_HIGH;
>> for (i = 0; i < high_slices; i++) {
>> mask_index = i & 0x1;
>> index = i >> 1;
>>
>> I guess for everything in the mm_context_t, we should compute it till
>> SLICE_NUM_HIGH. The reason for failure was, even though we recompute the
>> slice mask cached in mm_context on slb_addr_limit, it was still derived
>> from the high_slices_psizes which was computed with lower value.
>
> Okay thanks for catching that Aneesh. I guess that's a slow path so it
> should be okay. Christophe if you're taking care of the series can you
> fold it in? Otherwise I'll do that after yours gets merged.
>
I'm not really taking care of your serie, just took it once to see how
it fits on the 8xx.
I prefer if you can handle them. If you need my help for any test on
PPC32 don't hesitate to ask.
Christophe
^ permalink raw reply
* Re: [PATCH 01/21] powerpc: Remove warning on array size when empty
From: Mathieu Malaterre @ 2018-03-01 7:01 UTC (permalink / raw)
To: Michael Ellerman
Cc: Benjamin Herrenschmidt, Paul Mackerras, Jiri Slaby, linuxppc-dev,
LKML
In-Reply-To: <87woyw8pxe.fsf@concordia.ellerman.id.au>
On Thu, Mar 1, 2018 at 3:55 AM, Michael Ellerman <mpe@ellerman.id.au> wrote=
:
> Mathieu Malaterre <malat@debian.org> writes:
>
>> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, =
the
>> array feature_properties is defined as an empty array, which in turn
>> triggers the following warning (treated as error on W=3D1):
>>
>> CC arch/powerpc/kernel/prom.o
>> arch/powerpc/kernel/prom.c: In function =E2=80=98check_cpu_feature_prope=
rties=E2=80=99:
>> arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned express=
ion < 0 is always false [-Werror=3Dtype-limits]
>> for (i =3D 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>> ^
>> cc1: all warnings being treated as errors
>
> Ugh, that's annoying.
>
> This seems to work?
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 4dffef947b8a..5215119e249c 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -291,11 +291,11 @@ static inline void identical_pvr_fixup(unsigned lon=
g node)
>
> static void __init check_cpu_feature_properties(unsigned long node)
> {
> - unsigned long i;
> struct feature_property *fp =3D feature_properties;
> const __be32 *prop;
> + int i;
>
> - for (i =3D 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
> + for (i =3D 0; i < (int)ARRAY_SIZE(feature_properties); ++i, ++fp)=
{
> prop =3D of_get_flat_dt_prop(node, fp->name, NULL);
> if (prop && be32_to_cpup(prop) >=3D fp->min_value) {
> cur_cpu_spec->cpu_features |=3D fp->cpu_feature;
>
Indeed that looks like the less invasive solution, I'll re-submit.
Should I resubmit the entire patch series (21 indep patches) or
re-submit only the 3 patches that were discussed (as part of a
different series) ?
Thanks
^ permalink raw reply
* Re: [PATCH] xmon: Setup xmon debugger hooks when first break-point is set
From: Michael Ellerman @ 2018-03-01 3:00 UTC (permalink / raw)
To: Vaibhav Jain, linuxppc-dev
Cc: Vaibhav Jain, Benjamin Herrenschmidt, Paul Mackerras,
Balbir Singh, Nicholas Piggin, Douglas Miller, Frederic Barrat
In-Reply-To: <20180226113607.20321-1-vaibhav@linux.vnet.ibm.com>
Vaibhav Jain <vaibhav@linux.vnet.ibm.com> writes:
> Presently sysrq key for xmon('x') is registered during kernel init
> irrespective of the value of kernel param 'xmon'. Thus xmon is enabled
> even if 'xmon=off' is passed on the kernel command line. However this
> doesn't enable the kernel debugger hooks needed for instruction or data
> breakpoints. Thus when a break-point is hit with xmon=off a kernel oops
> of the form below is reported:
>
> Oops: Exception in kernel mode, sig: 5 [#1]
> < snip >
> Trace/breakpoint trap
>
> To fix this the patch checks and enables debugger hooks when an
> instruction or data break-point is set via xmon console.
> It also clears
^^^^
> all breakpoints when xmon is disabled via debugfs.
Can you split that into a separate patch please.
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 82e1a3ee6e0f..3679f5417a7e 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -1295,6 +1295,7 @@ bpt_cmds(void)
> switch (cmd) {
> #ifndef CONFIG_PPC_8xx
> static const char badaddr[] = "Only kernel addresses are permitted for breakpoints\n";
> + static const char warnxmon[] = "xmon: Enabling debugger hooks\n";
> int mode;
> case 'd': /* bd - hardware data breakpoint */
> mode = 7;
> @@ -1315,6 +1316,11 @@ bpt_cmds(void)
> dabr.address &= ~HW_BRK_TYPE_DABR;
> dabr.enabled = mode | BP_DABR;
> }
> + /* Enable xmon hooks if needed */
> + if (!xmon_on) {
> + printf(warnxmon);
> + xmon_on = 1;
> + }
> break;
>
> case 'i': /* bi - hardware instr breakpoint */
> @@ -1335,6 +1341,12 @@ bpt_cmds(void)
> if (bp != NULL) {
> bp->enabled |= BP_CIABR;
> iabr = bp;
> +
> + /* Enable xmon hooks if needed */
> + if (!xmon_on) {
> + printf(warnxmon);
> + xmon_on = 1;
> + }
> }
> break;
> #endif
> @@ -1399,8 +1411,15 @@ bpt_cmds(void)
> if (!check_bp_loc(a))
> break;
> bp = new_breakpoint(a);
> - if (bp != NULL)
> + if (bp != NULL) {
> bp->enabled |= BP_TRAP;
> +
> + /* Enable xmon hooks if needed */
> + if (!xmon_on) {
> + printf(warnxmon);
> + xmon_on = 1;
> + }
> + }
Yeah Balbir is right, repeating that three times is ugly. A static
function would do, and also avoids you feeling like you need to make
warnxmon a static char[].
cheers
^ permalink raw reply
* Re: [PATCH 01/21] powerpc: Remove warning on array size when empty
From: Michael Ellerman @ 2018-03-01 2:55 UTC (permalink / raw)
To: Mathieu Malaterre
Cc: Benjamin Herrenschmidt, Paul Mackerras, Jiri Slaby, linuxppc-dev,
linux-kernel, Mathieu Malaterre
In-Reply-To: <20180225172236.29650-2-malat@debian.org>
Mathieu Malaterre <malat@debian.org> writes:
> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, t=
he
> array feature_properties is defined as an empty array, which in turn
> triggers the following warning (treated as error on W=3D1):
>
> CC arch/powerpc/kernel/prom.o
> arch/powerpc/kernel/prom.c: In function =E2=80=98check_cpu_feature_proper=
ties=E2=80=99:
> arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expressi=
on < 0 is always false [-Werror=3Dtype-limits]
> for (i =3D 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
> ^
> cc1: all warnings being treated as errors
Ugh, that's annoying.
This seems to work?
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 4dffef947b8a..5215119e249c 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -291,11 +291,11 @@ static inline void identical_pvr_fixup(unsigned long =
node)
=20
static void __init check_cpu_feature_properties(unsigned long node)
{
- unsigned long i;
struct feature_property *fp =3D feature_properties;
const __be32 *prop;
+ int i;
=20
- for (i =3D 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
+ for (i =3D 0; i < (int)ARRAY_SIZE(feature_properties); ++i, ++fp) {
prop =3D of_get_flat_dt_prop(node, fp->name, NULL);
if (prop && be32_to_cpup(prop) >=3D fp->min_value) {
cur_cpu_spec->cpu_features |=3D fp->cpu_feature;
cheers
^ permalink raw reply related
* Re: [PATCH v2] powerpc/npu-dma.c: Fix deadlock in mmio_invalidate
From: Alistair Popple @ 2018-03-01 2:55 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, Balbir Singh, Mark Hairgrove
In-Reply-To: <877eqx4e07.fsf@concordia.ellerman.id.au>
> > diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
> > index 0a253b64ac5f..2fed4b116e19 100644
> > --- a/arch/powerpc/platforms/powernv/npu-dma.c
> > +++ b/arch/powerpc/platforms/powernv/npu-dma.c
> > @@ -726,7 +749,7 @@ struct npu_context *pnv_npu2_init_context(struct pci_dev *gpdev,
> > if (WARN_ON(of_property_read_u32(nvlink_dn, "ibm,npu-link-index",
> > &nvlink_index)))
> > return ERR_PTR(-ENODEV);
> > - npu_context->npdev[npu->index][nvlink_index] = npdev;
> > + WRITE_ONCE(npu_context->npdev[npu->index][nvlink_index], npdev);
>
> When you're publishing a struct via a pointer you would typically have a
> barrier between the stores that set the fields of the struct, and the
> store that publishes the struct. Otherwise the reader can see a
> partially setup struct.
npdev is just a pointer to a pci_dev setup by the PCI code. We assign it to
npdev[][] to indicate to the mmu notifiers that an invalidation should also
be sent to this nvlink. However I don't think there is any ordering
required wrt to the rest of the npu_context setup - so long as when the
notifiers deference npu_context->npdev[i][j] it either sees a valid
non-NULL pointer or NULL assigned to npdev[][] everything should be ok.
> I think here the npdev was setup somewhere else, and maybe there has
> been an intervening barrier, but it's not clear. A comment at least
> would be good.
Yes it has been. I will submit a v3 with some more comments incorporating
the above. Unless you think my argument is wrong?
> In general I feel like a spinlock or two could significantly simply the
> locking/ordering in this code, and given we're doing MMIOs anyway would
> not affect performance.
Indeed, I am working on a patch to add a spinlock around the allocation of
the npu_context as pnv_npu2_destroy_context() needs to be serialised with
respect to pnv_npu2_init_context() on the same mm_struct. I'd incorrectly
assumed the driver would do this serialisation but apparently it can't so
we need to ensure kref_get(npu_context) can't race with it's destruction
(concurrent init_context() is protected by mmap_sem). I could fold that
(unposted) patch into this one if you think that would be better?
Thanks!
- Alistair
> </rant>
>
> cheers
>
^ permalink raw reply
* Re: [PATCH] cpuidle/powernv : Restore different PSSCR for idle and hotplug
From: Michael Ellerman @ 2018-03-01 1:43 UTC (permalink / raw)
To: Stewart Smith, Akshay Adiga, linux-kernel, linuxppc-dev
Cc: npiggin, Akshay Adiga
In-Reply-To: <87vaekmk6n.fsf@linux.vnet.ibm.com>
Stewart Smith <stewart@linux.vnet.ibm.com> writes:
> Akshay Adiga <akshay.adiga@linux.vnet.ibm.com> writes:
>> commit 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
>> states via stop API.") uses stop-api provided by the firmware to restore
>> PSSCR. PSSCR restore is required for handling special wakeup. When special
>> wakeup is completed, the core enters stop state based on restored PSSCR.
>>
>> Currently PSSCR is restored to deepest available stop state, causing
>> a idle cpu to enter deeper stop state on a special wakeup, which causes
>> the cpu to hang on wakeup.
>>
>> A "sensors" command which reads temperature (through DTS sensors) on idle
>> cpu can trigger special wakeup.
>>
>> Failed Scenario :
>> Request restore of PSSCR with RL = 11
>> cpu enters idle state (stop5)
>> user triggers "sensors" command
>> Assert special wakeup on cpu
>> Restores PSSCR with RL = 11 <---- Done by firmware
>> Read DTS sensor
>> Deassert special wakeup
>> cpu enters idle state (stop11) <-- Instead of stop5
>>
>> Cpu hang is caused because cpu ended up in a deeper state than it requested
>>
>> This patch fixes instability caused by special wakeup when stop11 is
>> enabled. Requests restore of PSSCR to deepest stop state used by cpuidle.
>> Only when offlining cpu, request restore of PSSCR to deepest stop state.
>> On onlining cpu, request restore of PSSCR to deepest stop state used by
>> cpuidle.
>>
>> Fixes : 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
>> states via stop API.")
>
> This should CC stable ?
>
> We'll need this to enable stop11 in firmware and not break things, right?
But is Cc'ing this to stable even sufficient to enable stop11?
If firmware starts enabling stop11 then every existing kernel will be
broken, so eg. bisecting anything prior to now will be impossible.
cheers
^ permalink raw reply
* Re: [PATCH] cpuidle/powernv : Restore different PSSCR for idle and hotplug
From: Michael Ellerman @ 2018-03-01 1:37 UTC (permalink / raw)
To: Akshay Adiga, Stewart Smith; +Cc: linuxppc-dev, linux-kernel, npiggin
In-Reply-To: <20180228191903.23jmawvfqzbcwi2r@aksadiga.ibm>
Akshay Adiga <akshay.adiga@linux.vnet.ibm.com> writes:
> On Mon, Feb 26, 2018 at 03:47:12PM +1100, Stewart Smith wrote:
>> Akshay Adiga <akshay.adiga@linux.vnet.ibm.com> writes:
>> > commit 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
>> > states via stop API.") uses stop-api provided by the firmware to restore
>> > PSSCR. PSSCR restore is required for handling special wakeup. When special
>> > wakeup is completed, the core enters stop state based on restored PSSCR.
>> >
>> > Currently PSSCR is restored to deepest available stop state, causing
>> > a idle cpu to enter deeper stop state on a special wakeup, which causes
>> > the cpu to hang on wakeup.
>> >
>> > A "sensors" command which reads temperature (through DTS sensors) on idle
>> > cpu can trigger special wakeup.
>> >
>> > Failed Scenario :
>> > Request restore of PSSCR with RL = 11
>> > cpu enters idle state (stop5)
>> > user triggers "sensors" command
>> > Assert special wakeup on cpu
>> > Restores PSSCR with RL = 11 <---- Done by firmware
>> > Read DTS sensor
>> > Deassert special wakeup
>> > cpu enters idle state (stop11) <-- Instead of stop5
>> >
>> > Cpu hang is caused because cpu ended up in a deeper state than it requested
>> >
>> > This patch fixes instability caused by special wakeup when stop11 is
>> > enabled. Requests restore of PSSCR to deepest stop state used by cpuidle.
>> > Only when offlining cpu, request restore of PSSCR to deepest stop state.
>> > On onlining cpu, request restore of PSSCR to deepest stop state used by
>> > cpuidle.
>> >
>> > Fixes : 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
>> > states via stop API.")
>>
>> This should CC stable ?
>>
>> We'll need this to enable stop11 in firmware and not break things, right?
>
> Yes I will resend and CC it to stable.
That's not how patches get to stable.
You tag it with "Cc: stable@vger ...", you don't actually email it to
stable@vger.kernel.org.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc: dts: replace 'linux, stdout-path' with 'stdout-path'
From: Michael Ellerman @ 2018-03-01 1:33 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, linux-kernel, Mark Rutland, Benjamin Herrenschmidt,
Paul Mackerras, linuxppc-dev
In-Reply-To: <20180228224406.7049-1-robh@kernel.org>
Rob Herring <robh@kernel.org> writes:
> 'linux,stdout-path' has been deprecated for some time in favor of
> 'stdout-path'. Now dtc will warn on occurrences of 'linux,stdout-path'.
> Search and replace all the of occurrences with 'stdout-path'.
This patch looks OK.
But please remember that not all device trees are generated with dtc, we
still have machines in the wild that have firmware which use
"linux,stdout-path" and may never be updated.
cheers
^ permalink raw reply
* [PATCH] powerpc: Keep const vars out of writable .sdata
From: Kees Cook @ 2018-03-01 1:02 UTC (permalink / raw)
To: Michael Ellerman
Cc: Christophe Leroy, linux-kernel, Benjamin Herrenschmidt,
Paul Mackerras, linuxppc-dev, Segher Boessenkool
From: Segher Boessenkool <segher@kernel.crashing.org>
Newer gcc will support "-mno-readonly-in-sdata"[1], which makes sure that
the optimization on PPC32 for variables getting moved into the .sdata
section will not apply to const variables (which must be in .rodata).
This was originally noticed in mm/rodata_test.c when rodata_test_data
was not static:
c0695034 g O .data 00000004 rodata_test_data
After this patch with an updated compiler, this is correctly in .rodata.
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82411
Reported-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Segher Boessenkool <segher@kernel.crashing.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
arch/powerpc/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index ccd2556bdb53..c7628e973084 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -141,7 +141,9 @@ AFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv1)
endif
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mcmodel=medium,$(call cc-option,-mminimal-toc))
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mno-pointers-to-nested-functions)
+
CFLAGS-$(CONFIG_PPC32) := -ffixed-r2 $(MULTIPLEWORD)
+CFLAGS-$(CONFIG_PPC32) += $(call cc-option,-mno-readonly-in-sdata)
ifeq ($(CONFIG_PPC_BOOK3S_64),y)
CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=power7,-mtune=power4)
--
2.7.4
--
Kees Cook
Pixel Security
^ permalink raw reply related
* Re: [RFC PATCH 1/6] powerpc: Add security feature flags for Spectre/Meltdown
From: Daniel Axtens @ 2018-03-01 0:34 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
In-Reply-To: <20180228145316.11347-1-mpe@ellerman.id.au>
Michael Ellerman <mpe@ellerman.id.au> writes:
> This commit adds security feature flags to reflect the settings we
> receive from firmware regarding Spectre/Meltdown mitigations.
>
> The feature names reflect the names we are given by firmware on bare
> metal machines. See the hostboot source for details.
>
> Arguably these could be firmware features, but that then requires them
> to be read early in boot so they're available prior to asm feature
> patching, but we don't actually want to use them for patching. We may
> also want to dynamically update them in future, which would be
> incompatible with the way firmware features work (at the moment at
> least). So for now just make them separate flags.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> arch/powerpc/include/asm/security_features.h | 65 ++++++++++++++++++++++++++++
> arch/powerpc/kernel/Makefile | 2 +-
> arch/powerpc/kernel/security.c | 14 ++++++
> 3 files changed, 80 insertions(+), 1 deletion(-)
> create mode 100644 arch/powerpc/include/asm/security_features.h
> create mode 100644 arch/powerpc/kernel/security.c
>
> diff --git a/arch/powerpc/include/asm/security_features.h b/arch/powerpc/include/asm/security_features.h
> new file mode 100644
> index 000000000000..3b690de8b0e8
> --- /dev/null
> +++ b/arch/powerpc/include/asm/security_features.h
> @@ -0,0 +1,65 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Security related feature bit definitions.
> + *
> + * Copyright 2018, Michael Ellerman, IBM Corporation.
> + */
> +
> +#ifndef _ASM_POWERPC_SECURITY_FEATURES_H
> +#define _ASM_POWERPC_SECURITY_FEATURES_H
> +
> +
> +extern unsigned long powerpc_security_features;
> +
> +static inline void security_ftr_set(unsigned long feature)
> +{
> + powerpc_security_features |= feature;
> +}
> +
> +static inline void security_ftr_clear(unsigned long feature)
> +{
> + powerpc_security_features &= ~feature;
> +}
> +
> +static inline bool security_ftr_enabled(unsigned long feature)
> +{
> + return !!(powerpc_security_features & feature);
> +}
> +
> +
> +// Features indicating support for Spectre/Meltdown mitigations
> +
> +// The L1-D cache can be flushed with ori r30,r30,0
> +#define SEC_FTR_L1D_FLUSH_ORI30 0x0000000000000001ull
> +
> +// The L1-D cache can be flushed with mtspr 882,r0 (aka SPRN_TRIG2ull
This looks like some sort of search-replace gone wrong? ------------^^^^
> +#define SEC_FTR_L1D_FLUSH_TRIG2 0x0000000000000002ull
> +
> +// ori r31,r31,0 acts as a speculation barrier
> +#define SEC_FTR_SPEC_BAR_ORI31 0x0000000000000004ull
> +
> +// Speculation past bctr is disabled
> +#define SEC_FTR_BCCTRL_SERIALISED 0x0000000000000008ull
> +
> +// Entries in L1-D are private to a SMT thread
> +#define SEC_FTR_L1D_THREAD_PRIV 0x0000000000000010ull
> +
> +// Indirect branch prediction cache disabled
> +#define SEC_FTR_COUNT_CACHE_DISABLED 0x0000000000000020ull
> +
> +
> +// Features indicating need for Spectre/Meltdown mitigations
> +
> +// The L1-D cache should be flushed on MSR[HV] 1->0 transition (hypervisor to guest)
> +#define SEC_FTR_L1D_FLUSH_HV 0x0000000000000040ull
> +
> +// The L1-D cache should be flushed on MSR[PR] 0->1 transition (kernel to userspace)
> +#define SEC_FTR_L1D_FLUSH_PR 0x0000000000000080ull
> +
> +// A speculation barrier should be used for bounds checks (Spectre variant 1ull
Likewise here? ---------------------------------------------------------------^^^^
Regards,
Daniel
> +#define SEC_FTR_BNDS_CHK_SPEC_BAR 0x0000000000000100ull
> +
> +// Firmware configuration indicates user favours security over performance
> +#define SEC_FTR_FAVOUR_SECURITY 0x0000000000000200ull
> +
> +#endif /* _ASM_POWERPC_SECURITY_FEATURES_H */
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 1b6bc7fba996..d458c45e5004 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -42,7 +42,7 @@ obj-$(CONFIG_VDSO32) += vdso32/
> obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
> obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
> obj-$(CONFIG_PPC_BOOK3S_64) += cpu_setup_ppc970.o cpu_setup_pa6t.o
> -obj-$(CONFIG_PPC_BOOK3S_64) += cpu_setup_power.o
> +obj-$(CONFIG_PPC_BOOK3S_64) += cpu_setup_power.o security.o
> obj-$(CONFIG_PPC_BOOK3S_64) += mce.o mce_power.o
> obj-$(CONFIG_PPC_BOOK3E_64) += exceptions-64e.o idle_book3e.o
> obj-$(CONFIG_PPC64) += vdso64/
> diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
> new file mode 100644
> index 000000000000..c62a5d7196e3
> --- /dev/null
> +++ b/arch/powerpc/kernel/security.c
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +//
> +// Security related flags and so on.
> +//
> +// Copyright 2018, Michael Ellerman, IBM Corporation.
> +
> +#include <asm/security_features.h>
> +
> +
> +unsigned long powerpc_security_features __read_mostly = \
> + SEC_FTR_L1D_FLUSH_HV | \
> + SEC_FTR_L1D_FLUSH_PR | \
> + SEC_FTR_BNDS_CHK_SPEC_BAR | \
> + SEC_FTR_FAVOUR_SECURITY;
> --
> 2.14.1
^ permalink raw reply
* Re: [PATCH v4] watchdog: add SPDX identifiers for watchdog subsystem
From: Florian Fainelli @ 2018-02-28 23:45 UTC (permalink / raw)
To: Marcus Folkesson, Wim Van Sebroeck, Guenter Roeck, Joel Stanley,
Nicolas Ferre, Alexandre Belloni, Florian Fainelli, Ray Jui,
Scott Branden, bcm-kernel-feedback-list, Eric Anholt,
Stefan Wahren, Support Opensource, Baruch Siach,
William Breathitt Gray, Jimmy Vance, Keguang Zhang,
Joachim Eastwood, Tomas Winkler, Johannes Thumshirn,
Andreas Werner, Carlo Caione, Kevin Hilman, Matthias Brugger,
Wan ZongShun, Michal Simek, Vladimir Zapolskiy, Sylvain Lemieux,
Kukjin Kim, Krzysztof Kozlowski, Zwane Mwaikambo, Jim Cromie,
Barry Song, Patrice Chotard, Maxime Ripard, Chen-Yu Tsai,
Marc Gonzalez, Mans Rullgard, Thierry Reding, Jonathan Hunter,
Masahiro Yamada, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Jun Nie, Baoyou Xie, Shawn Guo
Cc: linux-watchdog, linux-kernel, linux-arm-kernel, linux-rpi-kernel,
adi-buildroot-devel, linux-mips, linux-amlogic, linux-mediatek,
linux-samsung-soc, linux-tegra, linuxppc-dev, patches
In-Reply-To: <20180228150209.2525-1-marcus.folkesson@gmail.com>
On 02/28/2018 07:01 AM, Marcus Folkesson wrote:
> - Add SPDX identifier
> - Remove boiler plate license text
> - If MODULE_LICENSE and boiler plate does not match, go for boiler plate
> license
>
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> Acked-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Acked-by: Baruch Siach <baruch@tkos.co.il>
> Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> Acked-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
> Acked-by: Johannes Thumshirn <jth@kernel.org>
> Acked-by: Mans Rullgard <mans@mansr.com>
> Acked-by: Matthias Brugger <matthias.bgg@gmail.com>
> Acked-by: Michal Simek <michal.simek@xilinx.com>
> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> Acked-by: Thierry Reding <treding@nvidia.com>
> Acked-by: Tomas Winkler <tomas.winkler@intel.com>
> Acked-by: Patrice Chotard <patrice.chotard@st.com>
> Acked-by: William Breathitt Gray <vilhelm.gray@gmail.com>
> Reviewed-by: Eric Anholt <eric@anholt.net>
> ---
>
> Notes:
> v4:
> - Drop coh901327_wdt since it allready is a pending patch
> v3:
> - Keep license text for ebc-c384_wdt
> v2:
> - Put back removed copyright texts for meson_gxbb_wdt and coh901327_wdt
> - Change to BSD-3-Clause for meson_gxbb_wdt
> v1: Please have an extra look at meson_gxbb_wdt.c
>
> drivers/watchdog/ar7_wdt.c | 14 +---------
> drivers/watchdog/bcm2835_wdt.c | 5 +---
> drivers/watchdog/bcm47xx_wdt.c | 5 +---
> drivers/watchdog/bcm63xx_wdt.c | 5 +---
> drivers/watchdog/bcm7038_wdt.c | 12 ++------
> drivers/watchdog/bcm_kona_wdt.c | 9 +-----
> drivers/watchdog/mtx-1_wdt.c | 11 +-------
For these drivers above:
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
and it looks like you missed rdc321x_wdt.c, is there a specific reason?
--
Florian
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox