* Re: [PATCH 1/1] kbuild: recursive build of external kernel modules
From: Shaun Tancheff @ 2019-08-12 17:34 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Shaun Tancheff, Linux Kbuild mailing list, Joe Lawrence,
James E . J . Bottomley, Jonathan Corbet, Martin K . Petersen,
Michal Marek, Shuah Khan, Thomas Renninger,
open list:DOCUMENTATION, Linux Kernel Mailing List,
Linux PM mailing list, linux-scsi
In-Reply-To: <CAK7LNAScm9P+QMZiqqSQnOoPsN54OTcTGpaDgxTbjJ_knoeGhA@mail.gmail.com>
On Mon, Aug 12, 2019 at 10:24 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> On Fri, Aug 9, 2019 at 9:21 AM Shaun Tancheff <shaun@tancheff.com> wrote:
> >
> > When building a tree of external modules stage 2 fails
> > silently as the root modules.order is empty.
> >
> > Modify the modules.order location to be fixed to the
> > root when KBUILD_EXTMOD is specified and write all
> > module paths to the single modules.order file.
>
> Could you try v5.3-rc4 please?
So it seems we are using 'subdir-m' but that is now gone?
Is there a recommend pattern for backward compatibility?
Thanks!
>
> > Signed-off-by: Shaun Tancheff <stancheff@cray.com>
> > ---
> > Makefile | 1 +
> > scripts/Makefile.build | 8 +++++++-
> > 2 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 23cdf1f41364..a9964492f47e 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1622,6 +1622,7 @@ $(module-dirs): prepare $(objtree)/Module.symvers
> >
> > modules: $(module-dirs)
> > @$(kecho) ' Building modules, stage 2.';
> > + $(Q)$rm -f $(KBUILD_EXTMOD)/modules.order
> > $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
> >
> > PHONY += modules_install
> > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > index 0d434d0afc0b..f9908b3d59e0 100644
> > --- a/scripts/Makefile.build
> > +++ b/scripts/Makefile.build
> > @@ -64,7 +64,13 @@ builtin-target := $(obj)/built-in.a
> > endif
> >
> > ifeq ($(CONFIG_MODULES)$(need-modorder),y1)
> > +ifneq ($(KBUILD_EXTMOD),)
> > +modorder-target := $(KBUILD_EXTMOD)/modules.order
> > +modorder-add := >>
> > +else
> > modorder-target := $(obj)/modules.order
> > +modorder-add := >
> > +endif
> > endif
> >
> > mod-targets := $(patsubst %.o, %.mod, $(obj-m))
> > @@ -423,7 +429,7 @@ endif # builtin-target
> > $(modorder-target): $(subdir-ym) FORCE
> > $(Q){ $(foreach m, $(modorder), \
> > $(if $(filter %/modules.order, $m), cat $m, echo $m);) :; } \
> > - | $(AWK) '!x[$$0]++' - > $@
> > + | $(AWK) '!x[$$0]++' - $(modorder-add) $@
> >
> > #
> > # Rule to compile a set of .o files into one .a file (with symbol table)
> > --
> > 2.20.1
> >
>
>
> --
> Best Regards
> Masahiro Yamada
^ permalink raw reply
* Re: [PATCH v7 1/2] arm64: Define Documentation/arm64/tagged-address-abi.rst
From: Catalin Marinas @ 2019-08-12 17:17 UTC (permalink / raw)
To: Andrew Murray
Cc: Will Deacon, linux-arch, linux-doc, Szabolcs Nagy,
Andrey Konovalov, Kevin Brodsky, Will Deacon, Dave Hansen,
Vincenzo Frascino, linux-arm-kernel
In-Reply-To: <20190812104606.GY56241@e119886-lin.cambridge.arm.com>
On Mon, Aug 12, 2019 at 11:46:06AM +0100, Andrew Murray wrote:
> On Thu, Aug 08, 2019 at 06:04:24PM +0100, Will Deacon wrote:
> > On Wed, Aug 07, 2019 at 04:53:20PM +0100, Catalin Marinas wrote:
> > > +
> > > +- mmap() addr parameter.
> > > +
> > > +- mremap() new_address parameter.
> > > +
> > > +- prctl(PR_SET_MM, ``*``, ...) other than arg2 PR_SET_MM_MAP and
> > > + PR_SET_MM_MAP_SIZE.
> > > +
> > > +- prctl(PR_SET_MM, PR_SET_MM_MAP{,_SIZE}, ...) struct prctl_mm_map fields.
> >
> > How did you generate this list and who will keep it up to date? How do you
> > know you haven't missed anything?
>
> What about shared memory system calls: shmat, shmdt? The latest "arm64: untag
> user pointers passed to the kernel" series doesn't untag these, thus we should
> indicate here that these too are no supported.
Yes. We dropped them from a previous version of the series but they've
never been documented. I'll add something (unless someone has a real
use-case for using tags with shmat/shmdt).
--
Catalin
^ permalink raw reply
* Re: [PATCH v2 2/2] Documentation: sphinx: Don't parse socket() as identifier reference
From: Mauro Carvalho Chehab @ 2019-08-12 16:11 UTC (permalink / raw)
To: Jonathan Neuschäfer
Cc: linux-doc, Jonathan Corbet, Alexei Starovoitov, Daniel Borkmann,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, linux-kernel, netdev, xdp-newbies, bpf
In-Reply-To: <20190812160708.32172-2-j.neuschaefer@gmx.net>
Em Mon, 12 Aug 2019 18:07:05 +0200
Jonathan Neuschäfer <j.neuschaefer@gmx.net> escreveu:
> With the introduction of Documentation/sphinx/automarkup.py, socket() is
> parsed as a reference to the in-kernel definition of socket. Sphinx then
> decides that struct socket is a good match, which is usually not
> intended, when the syscall is meant instead. This was observed in
> Documentation/networking/af_xdp.rst.
>
> Prevent socket() from being misinterpreted by adding it to the Skipfuncs
> list in automarkup.py.
>
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> ---
>
> v2:
> - block socket() in Documentation/sphinx/automarkup.py, as suggested by
> Jonathan Corbet
>
> v1:
> - https://lore.kernel.org/lkml/20190810121738.19587-1-j.neuschaefer@gmx.net/
> ---
> Documentation/sphinx/automarkup.py | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/sphinx/automarkup.py b/Documentation/sphinx/automarkup.py
> index a8798369e8f7..5b6119ff69f4 100644
> --- a/Documentation/sphinx/automarkup.py
> +++ b/Documentation/sphinx/automarkup.py
> @@ -26,7 +26,8 @@ RE_function = re.compile(r'([\w_][\w\d_]+\(\))')
> # just don't even try with these names.
> #
> Skipfuncs = [ 'open', 'close', 'read', 'write', 'fcntl', 'mmap',
> - 'select', 'poll', 'fork', 'execve', 'clone', 'ioctl']
> + 'select', 'poll', 'fork', 'execve', 'clone', 'ioctl',
> + 'socket' ]
Both patches sound OK on my eyes. Yet, I would just fold them into
a single one.
In any case:
Reviewed-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
>
> #
> # Find all occurrences of function() and try to replace them with
> --
> 2.20.1
>
Thanks,
Mauro
^ permalink raw reply
* [PATCH v2 2/2] Documentation: sphinx: Don't parse socket() as identifier reference
From: Jonathan Neuschäfer @ 2019-08-12 16:07 UTC (permalink / raw)
To: linux-doc
Cc: Jonathan Neuschäfer, Jonathan Corbet, Alexei Starovoitov,
Daniel Borkmann, David S. Miller, Jakub Kicinski,
Jesper Dangaard Brouer, John Fastabend, Mauro Carvalho Chehab,
linux-kernel, netdev, xdp-newbies, bpf
In-Reply-To: <20190812160708.32172-1-j.neuschaefer@gmx.net>
With the introduction of Documentation/sphinx/automarkup.py, socket() is
parsed as a reference to the in-kernel definition of socket. Sphinx then
decides that struct socket is a good match, which is usually not
intended, when the syscall is meant instead. This was observed in
Documentation/networking/af_xdp.rst.
Prevent socket() from being misinterpreted by adding it to the Skipfuncs
list in automarkup.py.
Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
v2:
- block socket() in Documentation/sphinx/automarkup.py, as suggested by
Jonathan Corbet
v1:
- https://lore.kernel.org/lkml/20190810121738.19587-1-j.neuschaefer@gmx.net/
---
Documentation/sphinx/automarkup.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/sphinx/automarkup.py b/Documentation/sphinx/automarkup.py
index a8798369e8f7..5b6119ff69f4 100644
--- a/Documentation/sphinx/automarkup.py
+++ b/Documentation/sphinx/automarkup.py
@@ -26,7 +26,8 @@ RE_function = re.compile(r'([\w_][\w\d_]+\(\))')
# just don't even try with these names.
#
Skipfuncs = [ 'open', 'close', 'read', 'write', 'fcntl', 'mmap',
- 'select', 'poll', 'fork', 'execve', 'clone', 'ioctl']
+ 'select', 'poll', 'fork', 'execve', 'clone', 'ioctl',
+ 'socket' ]
#
# Find all occurrences of function() and try to replace them with
--
2.20.1
^ permalink raw reply related
* [PATCH v2 1/2] Documentation: sphinx: Add missing comma to list of strings
From: Jonathan Neuschäfer @ 2019-08-12 16:07 UTC (permalink / raw)
To: linux-doc
Cc: Jonathan Neuschäfer, Jonathan Corbet, Mauro Carvalho Chehab,
linux-kernel
In Python, like in C, when a comma is omitted in a list of strings, the
two strings around the missing comma are concatenated.
Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
v2:
- new patch
---
Documentation/sphinx/automarkup.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/sphinx/automarkup.py b/Documentation/sphinx/automarkup.py
index 77e89c1956d7..a8798369e8f7 100644
--- a/Documentation/sphinx/automarkup.py
+++ b/Documentation/sphinx/automarkup.py
@@ -25,7 +25,7 @@ RE_function = re.compile(r'([\w_][\w\d_]+\(\))')
# to the creation of incorrect and confusing cross references. So
# just don't even try with these names.
#
-Skipfuncs = [ 'open', 'close', 'read', 'write', 'fcntl', 'mmap'
+Skipfuncs = [ 'open', 'close', 'read', 'write', 'fcntl', 'mmap',
'select', 'poll', 'fork', 'execve', 'clone', 'ioctl']
#
--
2.20.1
^ permalink raw reply related
* [PATCH v1] kernel-doc: Allow anonymous enum
From: Andy Shevchenko @ 2019-08-12 16:06 UTC (permalink / raw)
To: Jonathan Corbet, linux-doc, Mika Westerberg, Linus Walleij
Cc: Andy Shevchenko
In C is a valid construction to have an anonymous enumerator.
Though we have now:
drivers/pinctrl/intel/pinctrl-intel.c:240: error: Cannot parse enum!
Support it in the kernel-doc script.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
scripts/kernel-doc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 6b03012750da..079502bcc5a3 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1245,7 +1245,7 @@ sub dump_enum($$) {
# strip #define macros inside enums
$x =~ s@#\s*((define|ifdef)\s+|endif)[^;]*;@@gos;
- if ($x =~ /enum\s+(\w+)\s*\{(.*)\}/) {
+ if ($x =~ /enum\s+(\w*)\s*\{(.*)\}/) {
$declaration_name = $1;
my $members = $2;
my %_members;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 1/1] kbuild: recursive build of external kernel modules
From: Masahiro Yamada @ 2019-08-12 15:23 UTC (permalink / raw)
To: Shaun Tancheff
Cc: Shaun Tancheff, Linux Kbuild mailing list, Joe Lawrence,
James E . J . Bottomley, Jonathan Corbet, Martin K . Petersen,
Michal Marek, Shuah Khan, Thomas Renninger,
open list:DOCUMENTATION, Linux Kernel Mailing List,
Linux PM mailing list, linux-scsi
In-Reply-To: <20190809002104.18599-2-stancheff@cray.com>
On Fri, Aug 9, 2019 at 9:21 AM Shaun Tancheff <shaun@tancheff.com> wrote:
>
> When building a tree of external modules stage 2 fails
> silently as the root modules.order is empty.
>
> Modify the modules.order location to be fixed to the
> root when KBUILD_EXTMOD is specified and write all
> module paths to the single modules.order file.
Could you try v5.3-rc4 please?
> Signed-off-by: Shaun Tancheff <stancheff@cray.com>
> ---
> Makefile | 1 +
> scripts/Makefile.build | 8 +++++++-
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 23cdf1f41364..a9964492f47e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1622,6 +1622,7 @@ $(module-dirs): prepare $(objtree)/Module.symvers
>
> modules: $(module-dirs)
> @$(kecho) ' Building modules, stage 2.';
> + $(Q)$rm -f $(KBUILD_EXTMOD)/modules.order
> $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
>
> PHONY += modules_install
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 0d434d0afc0b..f9908b3d59e0 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -64,7 +64,13 @@ builtin-target := $(obj)/built-in.a
> endif
>
> ifeq ($(CONFIG_MODULES)$(need-modorder),y1)
> +ifneq ($(KBUILD_EXTMOD),)
> +modorder-target := $(KBUILD_EXTMOD)/modules.order
> +modorder-add := >>
> +else
> modorder-target := $(obj)/modules.order
> +modorder-add := >
> +endif
> endif
>
> mod-targets := $(patsubst %.o, %.mod, $(obj-m))
> @@ -423,7 +429,7 @@ endif # builtin-target
> $(modorder-target): $(subdir-ym) FORCE
> $(Q){ $(foreach m, $(modorder), \
> $(if $(filter %/modules.order, $m), cat $m, echo $m);) :; } \
> - | $(AWK) '!x[$$0]++' - > $@
> + | $(AWK) '!x[$$0]++' - $(modorder-add) $@
>
> #
> # Rule to compile a set of .o files into one .a file (with symbol table)
> --
> 2.20.1
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH v5 1/6] mm/page_idle: Add per-pid idle page tracking using virtual index
From: Joel Fernandes @ 2019-08-12 14:56 UTC (permalink / raw)
To: Michal Hocko
Cc: Andrew Morton, linux-kernel, Alexey Dobriyan, Borislav Petkov,
Brendan Gregg, Catalin Marinas, Christian Hansen, dancol, fmayer,
H. Peter Anvin, Ingo Molnar, Jonathan Corbet, Kees Cook,
kernel-team, linux-api, linux-doc, linux-fsdevel, linux-mm,
Mike Rapoport, minchan, namhyung, paulmck, Robin Murphy,
Roman Gushchin, Stephen Rothwell, surenb, Thomas Gleixner, tkjos,
Vladimir Davydov, Vlastimil Babka, Will Deacon
In-Reply-To: <20190808080044.GA18351@dhcp22.suse.cz>
On Thu, Aug 08, 2019 at 10:00:44AM +0200, Michal Hocko wrote:
> On Wed 07-08-19 17:31:05, Joel Fernandes wrote:
> > On Wed, Aug 07, 2019 at 01:58:40PM -0700, Andrew Morton wrote:
> > > On Wed, 7 Aug 2019 16:45:30 -0400 Joel Fernandes <joel@joelfernandes.org> wrote:
> > >
> > > > On Wed, Aug 07, 2019 at 01:04:02PM -0700, Andrew Morton wrote:
> > > > > On Wed, 7 Aug 2019 13:15:54 -0400 "Joel Fernandes (Google)" <joel@joelfernandes.org> wrote:
> > > > >
> > > > > > In Android, we are using this for the heap profiler (heapprofd) which
> > > > > > profiles and pin points code paths which allocates and leaves memory
> > > > > > idle for long periods of time. This method solves the security issue
> > > > > > with userspace learning the PFN, and while at it is also shown to yield
> > > > > > better results than the pagemap lookup, the theory being that the window
> > > > > > where the address space can change is reduced by eliminating the
> > > > > > intermediate pagemap look up stage. In virtual address indexing, the
> > > > > > process's mmap_sem is held for the duration of the access.
> > > > >
> > > > > So is heapprofd a developer-only thing? Is heapprofd included in
> > > > > end-user android loads? If not then, again, wouldn't it be better to
> > > > > make the feature Kconfigurable so that Android developers can enable it
> > > > > during development then disable it for production kernels?
> > > >
> > > > Almost all of this code is already configurable with
> > > > CONFIG_IDLE_PAGE_TRACKING. If you disable it, then all of this code gets
> > > > disabled.
> > > >
> > > > Or are you referring to something else that needs to be made configurable?
> > >
> > > Yes - the 300+ lines of code which this patchset adds!
> > >
> > > The impacted people will be those who use the existing
> > > idle-page-tracking feature but who will not use the new feature. I
> > > guess we can assume this set is small...
> >
> > Yes, I think this set should be small. The code size increase of page_idle.o
> > is from ~1KB to ~2KB. Most of the extra space is consumed by
> > page_idle_proc_generic() function which this patch adds. I don't think adding
> > another CONFIG option to disable this while keeping existing
> > CONFIG_IDLE_PAGE_TRACKING enabled, is worthwhile but I am open to the
> > addition of such an option if anyone feels strongly about it. I believe that
> > once this patch is merged, most like this new interface being added is what
> > will be used more than the old interface (for some of the usecases) so it
> > makes sense to keep it alive with CONFIG_IDLE_PAGE_TRACKING.
>
> I would tend to agree with Joel here. The functionality falls into an
> existing IDLE_PAGE_TRACKING config option quite nicely. If there really
> are users who want to save some space and this is standing in the way
> then they can easily add a new config option with some justification so
> the savings are clear. Without that an additional config simply adds to
> the already existing configurability complexity and balkanization.
Michal, Andrew, Minchan,
Would you have any other review comments on the v5 series? This is just a new
interface that does not disrupt existing users of the older page-idle
tracking, so as such it is a safe change (as in, doesn't change existing
functionality except for the draining bug fix).
thanks,
- Joel
^ permalink raw reply
* Re: [PATCH v2 1/2] rcu/tree: Add basic support for kfree_rcu batching
From: Joel Fernandes @ 2019-08-12 13:08 UTC (permalink / raw)
To: Byungchul Park
Cc: linux-kernel, Rao Shoaib, max.byungchul.park, kernel-team,
kernel-team, Andrew Morton, Davidlohr Bueso, Josh Triplett,
Lai Jiangshan, linux-doc, Mathieu Desnoyers, Paul E. McKenney,
rcu, Steven Rostedt
In-Reply-To: <20190812102917.GA10624@X58A-UD3R>
On Mon, Aug 12, 2019 at 07:29:17PM +0900, Byungchul Park wrote:
[snip]
> > diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
> > index 8e727f57d814..383f2481750f 100644
> > --- a/include/linux/rcutiny.h
> > +++ b/include/linux/rcutiny.h
> > @@ -39,6 +39,11 @@ static inline void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
> > call_rcu(head, func);
> > }
> >
> > +static inline void kfree_call_rcu_nobatch(struct rcu_head *head, rcu_callback_t func)
> > +{
> > + call_rcu(head, func);
> > +}
> > +
> > void rcu_qs(void);
> >
> > static inline void rcu_softirq_qs(void)
> > diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
> > index 735601ac27d3..7e38b39ec634 100644
> > --- a/include/linux/rcutree.h
> > +++ b/include/linux/rcutree.h
> > @@ -34,6 +34,7 @@ static inline void rcu_virt_note_context_switch(int cpu)
> >
> > void synchronize_rcu_expedited(void);
> > void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
> > +void kfree_call_rcu_nobatch(struct rcu_head *head, rcu_callback_t func);
> >
> > void rcu_barrier(void);
> > bool rcu_eqs_special_set(int cpu);
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index a14e5fbbea46..102a5f606d78 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -2593,17 +2593,204 @@ void call_rcu(struct rcu_head *head, rcu_callback_t func)
> > }
> > EXPORT_SYMBOL_GPL(call_rcu);
> >
> > +
> > +/* Maximum number of jiffies to wait before draining a batch. */
> > +#define KFREE_DRAIN_JIFFIES (HZ / 50)
>
> JFYI, I also can see oom with a larger value of this. I hope this magic
> value works well for all kind of systems.
It seems to work well in my testing. I am glad you could not perceive OOMs at
this value, either.
> > - * Queue an RCU callback for lazy invocation after a grace period.
> > - * This will likely be later named something like "call_rcu_lazy()",
> > - * but this change will require some way of tagging the lazy RCU
> > - * callbacks in the list of pending callbacks. Until then, this
> > - * function may only be called from __kfree_rcu().
> > + * Maximum number of kfree(s) to batch, if this limit is hit then the batch of
> > + * kfree(s) is queued for freeing after a grace period, right away.
> > */
> > -void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
> > +struct kfree_rcu_cpu {
> > + /* The rcu_work node for queuing work with queue_rcu_work(). The work
> > + * is done after a grace period.
> > + */
> > + struct rcu_work rcu_work;
> > +
> > + /* The list of objects being queued in a batch but are not yet
> > + * scheduled to be freed.
> > + */
> > + struct rcu_head *head;
> > +
> > + /* The list of objects that have now left ->head and are queued for
> > + * freeing after a grace period.
> > + */
> > + struct rcu_head *head_free;
> > +
> > + /* Protect concurrent access to this structure. */
> > + spinlock_t lock;
> > +
> > + /* The delayed work that flushes ->head to ->head_free incase ->head
> > + * did not reach a length of KFREE_MAX_BATCH within KFREE_DRAIN_JIFFIES.
> > + * In case flushing cannot be done if RCU is busy, then ->head just
> > + * continues to grow beyond KFREE_MAX_BATCH and we retry flushing later.
>
> Minor one. We don't use KFREE_MAX_BATCH anymore.
Sorry for leaving these KFREE_MAX_BATCH comments stale, I cleaned up many of
them already but some where still left behind. I will fix these in the v3.
thanks for review!
- Joel
[snip]
^ permalink raw reply
* Re: [PATCH 3/3] driver/core: Fix build error when SRCU and lockdep disabled
From: Joel Fernandes @ 2019-08-12 13:03 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel, kbuild test robot, Jonathan Corbet, Josh Triplett,
Lai Jiangshan, linux-doc, Mathieu Desnoyers, Paul E. McKenney,
Rafael J. Wysocki, rcu, Steven Rostedt, Tejun Heo
In-Reply-To: <20190812050256.GC5834@kroah.com>
On Mon, Aug 12, 2019 at 07:02:56AM +0200, Greg Kroah-Hartman wrote:
> On Sun, Aug 11, 2019 at 06:11:11PM -0400, Joel Fernandes (Google) wrote:
> > Properly check if lockdep lock checking is disabled at config time. If
> > so, then lock_is_held() is undefined so don't do any checking.
> >
> > This fix is similar to the pattern used in srcu_read_lock_held().
> >
> > Link: https://lore.kernel.org/lkml/201908080026.WSAFx14k%25lkp@intel.com/
> > Fixes: c9e4d3a2fee8 ("acpi: Use built-in RCU list checking for acpi_ioremaps list")
>
> What tree is this commit in?
>
> > Reported-by: kbuild test robot <lkp@intel.com>
> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > ---
> > This patch is based on the -rcu dev branch.
>
> Ah...
>
> > drivers/base/core.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index 32cf83d1c744..fe25cf690562 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -99,7 +99,11 @@ void device_links_read_unlock(int not_used)
> >
> > int device_links_read_lock_held(void)
> > {
> > - return lock_is_held(&device_links_lock);
> > +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> > + return lock_is_held(&(device_links_lock.dep_map));
> > +#else
> > + return 1;
> > +#endif
>
> return 1? So the lock is always held?
This is just the pattern of an assert that is disabled, so that
false-positives don't happen if lockdep is disabled.
So say someone writes a statement like:
WARN_ON_ONCE(!device_links_read_lock_held());
Since lockdep is disabled, we cannot check whether lock is held or not. Yet,
we don't want false positives by reporting that the lock is not held. In this
case, it is better to report that the lock is held to suppress
false-positives. srcu_read_lock_held() also follows the same pattern.
thanks,
- Joel
^ permalink raw reply
* Re: [PATCH v3 2/2] drivers/perf: Add CCPI2 PMU support in ThunderX2 UNCORE driver.
From: Mark Rutland @ 2019-08-12 12:01 UTC (permalink / raw)
To: Ganapatrao Kulkarni
Cc: linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, will@kernel.org,
corbet@lwn.net, Jayachandran Chandrasekharan Nair, Robert Richter,
Jan Glauber, gklkml16@gmail.com
In-Reply-To: <1563873380-2003-3-git-send-email-gkulkarni@marvell.com>
On Tue, Jul 23, 2019 at 09:16:28AM +0000, Ganapatrao Kulkarni wrote:
> CCPI2 is a low-latency high-bandwidth serial interface for connecting
> ThunderX2 processors. This patch adds support to capture CCPI2 perf events.
It would be worth pointing out in the commit message how the CCPI2
counters differ from the others. I realise you have that in the body of
patch 1, but it's critical information when reviewing this patch...
>
> Signed-off-by: Ganapatrao Kulkarni <gkulkarni@marvell.com>
> ---
> drivers/perf/thunderx2_pmu.c | 248 ++++++++++++++++++++++++++++++-----
> 1 file changed, 214 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/perf/thunderx2_pmu.c b/drivers/perf/thunderx2_pmu.c
> index 43d76c85da56..a4e1273eafa3 100644
> --- a/drivers/perf/thunderx2_pmu.c
> +++ b/drivers/perf/thunderx2_pmu.c
> @@ -17,22 +17,31 @@
> */
>
> #define TX2_PMU_MAX_COUNTERS 4
Shouldn't this be 8 now?
[...]
> /*
> - * pmu on each socket has 2 uncore devices(dmc and l3c),
> - * each device has 4 counters.
> + * pmu on each socket has 3 uncore devices(dmc, l3ci and ccpi2),
> + * dmc and l3c has 4 counters and ccpi2 8.
> */
How about:
/*
* Each socket has 3 uncore device associated with a PMU. The DMC and
* L3C have 4 32-bit counters, and the CCPI2 has 8 64-bit counters.
*/
> struct tx2_uncore_pmu {
> struct hlist_node hpnode;
> @@ -69,12 +86,14 @@ struct tx2_uncore_pmu {
> int node;
> int cpu;
> u32 max_counters;
> + u32 counters_mask;
> u32 prorate_factor;
> u32 max_events;
> + u32 events_mask;
> u64 hrtimer_interval;
> void __iomem *base;
> DECLARE_BITMAP(active_counters, TX2_PMU_MAX_COUNTERS);
This bitmap isn't big enough for the 4 new counters.
> - struct perf_event *events[TX2_PMU_MAX_COUNTERS];
> + struct perf_event **events;
As above, can't we bump TX2_PMU_MAX_COUNTERS to 8 rather than making
this a dynamic allocation?
[...]
> static inline u32 reg_readl(unsigned long addr)
> {
> return readl((void __iomem *)addr);
> }
>
> +static inline u32 reg_readq(unsigned long addr)
> +{
> + return readq((void __iomem *)addr);
> +}
Presumably reg_readq() should return a u64.
[...]
> +static void uncore_start_event_ccpi2(struct perf_event *event, int flags)
> +{
> + u32 emask;
> + struct hw_perf_event *hwc = &event->hw;
> + struct tx2_uncore_pmu *tx2_pmu;
> +
> + tx2_pmu = pmu_to_tx2_pmu(event->pmu);
> + emask = tx2_pmu->events_mask;
> +
> + /* Bit [09:00] to set event id, set level and type to 1 */
> + reg_writel((3 << 10) |
Do you mean that bits [11:10] are level and type?
What exactly are 'level' and 'type'?
Can we give those bits mnemonics?
> + GET_EVENTID(event, emask), hwc->config_base);
> + /* reset[4], enable[0] and start[1] counters */
Rather than using magic numbers everywhere, please give these mnemonics,
e.g.
#define CCPI2_PERF_CTL_ENABLE BIT(0)
#define CCPI2_PERF_CTL_START BIT(1)
#define CCPI2_PERF_CTL_RESET BIT(4)
> + reg_writel(0x13, hwc->event_base + CCPI2_PERF_CTL);
... and then you can OR them in here:
ctl = CCPI2_PERF_CTL_ENABLE |
CCPI2_PERF_CTL_START |
CCPI2_PERF_CTL_RESET;
reg_writel(ctl, hwc->event_base + CCPI2_PERF_CTL);
[...]
> @@ -456,8 +603,9 @@ static void tx2_uncore_event_start(struct perf_event *event, int flags)
> tx2_pmu->start_event(event, flags);
> perf_event_update_userpage(event);
>
> - /* Start timer for first event */
> - if (bitmap_weight(tx2_pmu->active_counters,
> + /* Start timer for first non ccpi2 event */
> + if (tx2_pmu->type != PMU_TYPE_CCPI2 &&
> + bitmap_weight(tx2_pmu->active_counters,
> tx2_pmu->max_counters) == 1) {
> hrtimer_start(&tx2_pmu->hrtimer,
> ns_to_ktime(tx2_pmu->hrtimer_interval),
This would be easier to read as two statements:
/* No hrtimer needed with 64-bit counters */
if (tx2_pmu->type == PMU_TYPE_CCPI2)
return;
/* Start timer for first event */
if (bitmap_weight(tx2_pmu->active_counters,
tx2_pmu->max_counters) != 1) {
...
}
> @@ -495,7 +643,8 @@ static int tx2_uncore_event_add(struct perf_event *event, int flags)
> if (hwc->idx < 0)
> return -EAGAIN;
>
> - tx2_pmu->events[hwc->idx] = event;
> + if (tx2_pmu->events)
> + tx2_pmu->events[hwc->idx] = event;
So this is NULL for CCPI2?
I guess we don't strictly need the if we don't have a hrtimer to update
event counts, but this makes the code more complicated than it needs to
be.
[...]
> @@ -580,8 +732,12 @@ static int tx2_uncore_pmu_add_dev(struct tx2_uncore_pmu *tx2_pmu)
> cpu_online_mask);
>
> tx2_pmu->cpu = cpu;
> - hrtimer_init(&tx2_pmu->hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> - tx2_pmu->hrtimer.function = tx2_hrtimer_callback;
> + /* CCPI2 counters are 64 bit counters, no overflow */
> + if (tx2_pmu->type != PMU_TYPE_CCPI2) {
> + hrtimer_init(&tx2_pmu->hrtimer,
> + CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> + tx2_pmu->hrtimer.function = tx2_hrtimer_callback;
> + }
Hmmm... this means that tx2_pmu->hrtimer.function is NULL for the CCPI2
PMU. I think it would be best to check that when (re)programming the
counters rather than the PMU type. For example, in
tx2_uncore_event_start() we could have:
if (!tx2_pmu->hrtimer.function)
return;
if (bitmap_weight(tx2_pmu->active_counters,
tx2_pmu->max_counters) != 1) {
...
}
Thanks,
Mark.
^ permalink raw reply
* Re: [PATCH v2 1/2] rcu/tree: Add basic support for kfree_rcu batching
From: Byungchul Park @ 2019-08-12 10:29 UTC (permalink / raw)
To: Joel Fernandes (Google)
Cc: linux-kernel, Rao Shoaib, max.byungchul.park, kernel-team,
kernel-team, Andrew Morton, Davidlohr Bueso, Josh Triplett,
Lai Jiangshan, linux-doc, Mathieu Desnoyers, Paul E. McKenney,
rcu, Steven Rostedt
In-Reply-To: <20190811024957.233650-1-joel@joelfernandes.org>
On Sat, Aug 10, 2019 at 10:49:55PM -0400, Joel Fernandes (Google) wrote:
> Recently a discussion about stability and performance of a system
> involving a high rate of kfree_rcu() calls surfaced on the list [1]
> which led to another discussion how to prepare for this situation.
>
> This patch adds basic batching support for kfree_rcu(). It is "basic"
> because we do none of the slab management, dynamic allocation, code
> moving or any of the other things, some of which previous attempts did
> [2]. These fancier improvements can be follow-up patches and there are
> several ideas being discussed in those regards.
>
> Torture tests follow in the next patch and show improvements of around
> 400% in reduction of number of grace periods on a 16 CPU system. More
> details and test data are in that patch.
>
> This is an effort to start simple, and build up from there. In the
> future, an extension to use kfree_bulk and possibly per-slab batching
> could be done to further improve performance due to cache-locality and
> slab-specific bulk free optimizations.
>
> There is an implication with rcu_barrier() with this patch. Since the
> kfree_rcu() calls can be batched, and may not be handed yet to the RCU
> machinery in fact, the monitor may not have even run yet to do the
> queue_rcu_work(), there seems no easy way of implementing rcu_barrier()
> to wait for those kfree_rcu()s that are already made. So this means a
> kfree_rcu() followed by an rcu_barrier() does not imply that memory will
> be freed once rcu_barrier() returns.
>
> Another implication is higher active memory usage (although not
> run-away..) until the kfree_rcu() flooding ends, in comparison to
> without batching. More details about this are in the second patch which
> adds an rcuperf test.
>
> Finally, in the future we should get rid of kfree_rcu special casing
> within RCU such as in rcu_do_batch and switch everything to just
> batching. Currently we don't do that since timer subsystem is not yet up
> and we cannot schedule the kfree_rcu() monitor as the timer subsystem's
> lock are not initialized. That would also mean getting rid of
> kfree_call_rcu_nobatch() entirely.
>
> [1] http://lore.kernel.org/lkml/20190723035725-mutt-send-email-mst@kernel.org
> [2] https://lkml.org/lkml/2017/12/19/824
>
> Cc: Rao Shoaib <rao.shoaib@oracle.com>
> Cc: max.byungchul.park@gmail.com
> Cc: byungchul.park@lge.com
> Cc: kernel-team@android.com
> Cc: kernel-team@lge.com
> Co-developed-by: Byungchul Park <byungchul.park@lge.com>
> Signed-off-by: Byungchul Park <byungchul.park@lge.com>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
>
> ---
> RFC v1-> PATCH v2: Removed limits on the ->head list, just let it grow.
> Dropped KFREE_MAX_JIFFIES to HZ/50 from HZ/20 to reduce OOM occurrence.
> Removed sleeps in rcuperf test, just using cond_resched()in loop.
> Better code comments ;)
>
> .../admin-guide/kernel-parameters.txt | 17 ++
> include/linux/rcutiny.h | 5 +
> include/linux/rcutree.h | 1 +
> kernel/rcu/tree.c | 213 +++++++++++++++++-
> 4 files changed, 230 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 7ccd158b3894..a9156ca5de24 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3895,6 +3895,23 @@
> test until boot completes in order to avoid
> interference.
>
> + rcuperf.kfree_rcu_test= [KNL]
> + Set to measure performance of kfree_rcu() flooding.
> +
> + rcuperf.kfree_nthreads= [KNL]
> + The number of threads running loops of kfree_rcu().
> +
> + rcuperf.kfree_alloc_num= [KNL]
> + Number of allocations and frees done in an iteration.
> +
> + rcuperf.kfree_loops= [KNL]
> + Number of loops doing rcuperf.kfree_alloc_num number
> + of allocations and frees.
> +
> + rcuperf.kfree_no_batch= [KNL]
> + Use the non-batching (slower) version of kfree_rcu.
> + This is useful for comparing with the batched version.
> +
> rcuperf.nreaders= [KNL]
> Set number of RCU readers. The value -1 selects
> N, where N is the number of CPUs. A value
> diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
> index 8e727f57d814..383f2481750f 100644
> --- a/include/linux/rcutiny.h
> +++ b/include/linux/rcutiny.h
> @@ -39,6 +39,11 @@ static inline void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
> call_rcu(head, func);
> }
>
> +static inline void kfree_call_rcu_nobatch(struct rcu_head *head, rcu_callback_t func)
> +{
> + call_rcu(head, func);
> +}
> +
> void rcu_qs(void);
>
> static inline void rcu_softirq_qs(void)
> diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
> index 735601ac27d3..7e38b39ec634 100644
> --- a/include/linux/rcutree.h
> +++ b/include/linux/rcutree.h
> @@ -34,6 +34,7 @@ static inline void rcu_virt_note_context_switch(int cpu)
>
> void synchronize_rcu_expedited(void);
> void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
> +void kfree_call_rcu_nobatch(struct rcu_head *head, rcu_callback_t func);
>
> void rcu_barrier(void);
> bool rcu_eqs_special_set(int cpu);
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index a14e5fbbea46..102a5f606d78 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -2593,17 +2593,204 @@ void call_rcu(struct rcu_head *head, rcu_callback_t func)
> }
> EXPORT_SYMBOL_GPL(call_rcu);
>
> +
> +/* Maximum number of jiffies to wait before draining a batch. */
> +#define KFREE_DRAIN_JIFFIES (HZ / 50)
JFYI, I also can see oom with a larger value of this. I hope this magic
value works well for all kind of systems.
> +
> /*
> - * Queue an RCU callback for lazy invocation after a grace period.
> - * This will likely be later named something like "call_rcu_lazy()",
> - * but this change will require some way of tagging the lazy RCU
> - * callbacks in the list of pending callbacks. Until then, this
> - * function may only be called from __kfree_rcu().
> + * Maximum number of kfree(s) to batch, if this limit is hit then the batch of
> + * kfree(s) is queued for freeing after a grace period, right away.
> */
> -void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
> +struct kfree_rcu_cpu {
> + /* The rcu_work node for queuing work with queue_rcu_work(). The work
> + * is done after a grace period.
> + */
> + struct rcu_work rcu_work;
> +
> + /* The list of objects being queued in a batch but are not yet
> + * scheduled to be freed.
> + */
> + struct rcu_head *head;
> +
> + /* The list of objects that have now left ->head and are queued for
> + * freeing after a grace period.
> + */
> + struct rcu_head *head_free;
> +
> + /* Protect concurrent access to this structure. */
> + spinlock_t lock;
> +
> + /* The delayed work that flushes ->head to ->head_free incase ->head
> + * did not reach a length of KFREE_MAX_BATCH within KFREE_DRAIN_JIFFIES.
> + * In case flushing cannot be done if RCU is busy, then ->head just
> + * continues to grow beyond KFREE_MAX_BATCH and we retry flushing later.
Minor one. We don't use KFREE_MAX_BATCH anymore.
> + */
> + struct delayed_work monitor_work;
> + bool monitor_todo; /* Is a delayed work pending execution? */
> +};
> +
> +static DEFINE_PER_CPU(struct kfree_rcu_cpu, krc);
> +
> +/*
> + * This function is invoked in workqueue context after a grace period.
> + * It frees all the objects queued on ->head_free.
> + */
> +static void kfree_rcu_work(struct work_struct *work)
> +{
> + unsigned long flags;
> + struct rcu_head *head, *next;
> + struct kfree_rcu_cpu *krcp = container_of(to_rcu_work(work),
> + struct kfree_rcu_cpu, rcu_work);
> +
> + spin_lock_irqsave(&krcp->lock, flags);
> + head = krcp->head_free;
> + krcp->head_free = NULL;
> + spin_unlock_irqrestore(&krcp->lock, flags);
> +
> + /*
> + * The head is detached and not referenced from anywhere, so lockless
> + * access is Ok.
> + */
> + for (; head; head = next) {
> + next = head->next;
> + head->next = NULL;
> + /* Could be possible to optimize with kfree_bulk in future */
> + __rcu_reclaim(rcu_state.name, head);
> + cond_resched_tasks_rcu_qs();
> + }
> +}
> +
> +/*
> + * Schedule the kfree batch RCU work to run in workqueue context after a GP.
> + *
> + * This function is invoked when the ->head batch has reached size
> + * KFREE_MAX_BATCH or when kfree_rcu_monitor() sees that the
> + * KFREE_DRAIN_JIFFIES timeout has been reached.
KFREE_MAX_BATCH again.
> + *
> + * It can also be called from IRQ context if ->head fills up completely.
> + * (i.e. ->head reaches KFREE_MAX_BATCH under heavy kfree_rcu() load
> + * and that last kfree_rcu() in the ->head batch happened from IRQ context).
Not true anymore.
> + */
> +static inline bool queue_kfree_rcu_work(struct kfree_rcu_cpu *krcp)
> +{
> + lockdep_assert_held(&krcp->lock);
> +
> + /* If a previous RCU batch work is already in progress, we cannot queue
> + * another one, just refuse the optimization and it will be retried
> + * again in KFREE_DRAIN_JIFFIES time.
> + */
> + if (krcp->head_free)
> + return false;
> +
> + krcp->head_free = krcp->head;
> + krcp->head = NULL;
> + INIT_RCU_WORK(&krcp->rcu_work, kfree_rcu_work);
> + queue_rcu_work(system_wq, &krcp->rcu_work);
> +
> + return true;
> +}
> +
> +static inline void kfree_rcu_drain_unlock(struct kfree_rcu_cpu *krcp,
> + unsigned long flags)
> +{
> + /* Flush ->head to ->head_free, all objects on ->head_free will be
> + * kfree'd after a grace period.
> + */
> + if (queue_kfree_rcu_work(krcp)) {
> + /* Success! Our job is done here. */
> + spin_unlock_irqrestore(&krcp->lock, flags);
> + return;
> + }
> +
> + /* Previous batch did not get free yet, let us try again soon. */
> + if (krcp->monitor_todo == false) {
> + schedule_delayed_work_on(smp_processor_id(),
> + &krcp->monitor_work, KFREE_DRAIN_JIFFIES);
> + krcp->monitor_todo = true;
> + }
> + spin_unlock_irqrestore(&krcp->lock, flags);
> +}
> +
> +/*
> + * This function is invoked after the KFREE_DRAIN_JIFFIES timeout has elapsed,
> + * so it drains the specified kfree_rcu_cpu structure's ->head list.
> + */
> +static void kfree_rcu_monitor(struct work_struct *work)
> +{
> + bool todo;
> + unsigned long flags;
> + struct kfree_rcu_cpu *krcp = container_of(work, struct kfree_rcu_cpu,
> + monitor_work.work);
> +
> + spin_lock_irqsave(&krcp->lock, flags);
> + todo = krcp->monitor_todo;
> + krcp->monitor_todo = false;
> + if (todo)
> + kfree_rcu_drain_unlock(krcp, flags);
> + else
> + spin_unlock_irqrestore(&krcp->lock, flags);
> +}
> +
> +/*
> + * This version of kfree_call_rcu does not do batching of kfree_rcu() requests.
> + * Used only by rcuperf torture test for comparison with kfree_rcu_batch().
> + */
> +void kfree_call_rcu_nobatch(struct rcu_head *head, rcu_callback_t func)
> {
> __call_rcu(head, func, -1, 1);
> }
> +EXPORT_SYMBOL_GPL(kfree_call_rcu_nobatch);
> +
> +/*
> + * Queue a request for lazy invocation of kfree() after a grace period.
> + *
> + * Each kfree_call_rcu() request is added to a batch. Once the batch reaches a
> + * threshold of KFREE_MAX_BATCH, all the objects in the batch will be kfree'd
> + * in workqueue context. This allows us to:
> + *
> + * 1. Batch requests together to reduce the number of grace periods during
> + * heavy kfree_rcu() load.
> + *
> + * 2. In the future, makes it possible to use kfree_bulk() on a large number of
> + * kfree_rcu() requests thus reducing the per-object overhead of kfree().
> + *
> + * If the batch does not reach a threshold of KFREE_MAX_BATCH within
> + * KFREE_DRAIN_JIFFIES, then kfree_rcu_monitor() queues them for freeing in
> + * workqueue context after a grace period.
KFREE_MAX_BATCH again.
Thanks,
Byungchul
> + */
> +void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
> +{
> + unsigned long flags;
> + struct kfree_rcu_cpu *krcp;
> + bool monitor_todo;
> +
> + /* kfree_call_rcu() batching requires timers to be up. If the scheduler
> + * is not yet up, just skip batching and do non-batched kfree_call_rcu().
> + */
> + if (rcu_scheduler_active != RCU_SCHEDULER_RUNNING)
> + return kfree_call_rcu_nobatch(head, func);
> +
> + local_irq_save(flags);
> + krcp = this_cpu_ptr(&krc);
> +
> + spin_lock(&krcp->lock);
> +
> + /* Queue the kfree but don't yet schedule the batch. */
> + head->func = func;
> + head->next = krcp->head;
> + krcp->head = head;
> +
> + /* Schedule monitor for timely drain after KFREE_DRAIN_JIFFIES. */
> + monitor_todo = krcp->monitor_todo;
> + krcp->monitor_todo = true;
> + spin_unlock(&krcp->lock);
> +
> + if (!monitor_todo) {
> + schedule_delayed_work_on(smp_processor_id(),
> + &krcp->monitor_work, KFREE_DRAIN_JIFFIES);
> + }
> + local_irq_restore(flags);
> +}
> EXPORT_SYMBOL_GPL(kfree_call_rcu);
>
> /*
> @@ -3455,10 +3642,24 @@ static void __init rcu_dump_rcu_node_tree(void)
> struct workqueue_struct *rcu_gp_wq;
> struct workqueue_struct *rcu_par_gp_wq;
>
> +static void __init kfree_rcu_batch_init(void)
> +{
> + int cpu;
> +
> + for_each_possible_cpu(cpu) {
> + struct kfree_rcu_cpu *krcp = per_cpu_ptr(&krc, cpu);
> +
> + spin_lock_init(&krcp->lock);
> + INIT_DELAYED_WORK(&krcp->monitor_work, kfree_rcu_monitor);
> + }
> +}
> +
> void __init rcu_init(void)
> {
> int cpu;
>
> + kfree_rcu_batch_init();
> +
> rcu_early_boot_tests();
>
> rcu_bootup_announce();
> --
> 2.23.0.rc1.153.gdeed80330f-goog
^ permalink raw reply
* Re: [PATCH v7 1/2] arm64: Define Documentation/arm64/tagged-address-abi.rst
From: Andrew Murray @ 2019-08-12 10:46 UTC (permalink / raw)
To: Will Deacon
Cc: Catalin Marinas, linux-arch, linux-doc, Szabolcs Nagy,
Andrey Konovalov, Kevin Brodsky, Will Deacon, Dave Hansen,
Vincenzo Frascino, linux-arm-kernel
In-Reply-To: <20190808170424.6td34cpdngkcxxpu@willie-the-truck>
On Thu, Aug 08, 2019 at 06:04:24PM +0100, Will Deacon wrote:
> On Wed, Aug 07, 2019 at 04:53:20PM +0100, Catalin Marinas wrote:
> > From: Vincenzo Frascino <vincenzo.frascino@arm.com>
> >
> > On arm64 the TCR_EL1.TBI0 bit has been always enabled hence
> > the userspace (EL0) is allowed to set a non-zero value in the
> > top byte but the resulting pointers are not allowed at the
> > user-kernel syscall ABI boundary.
> >
> > With the relaxed ABI proposed through this document, it is now possible
> > to pass tagged pointers to the syscalls, when these pointers are in
> > memory ranges obtained by an anonymous (MAP_ANONYMOUS) mmap().
> >
> > This change in the ABI requires a mechanism to requires the userspace
> > to opt-in to such an option.
> >
> > Specify and document the way in which sysctl and prctl() can be used
> > in combination to allow the userspace to opt-in this feature.
> >
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Andrey Konovalov <andreyknvl@google.com>
> > Cc: Szabolcs Nagy <szabolcs.nagy@arm.com>
> > Cc: Kevin Brodsky <kevin.brodsky@arm.com>
> > Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> > [catalin.marinas@arm.com: some rewording, dropped MAP_PRIVATE]
> > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > ---
> > Documentation/arm64/tagged-address-abi.rst | 151 +++++++++++++++++++++
> > 1 file changed, 151 insertions(+)
> > create mode 100644 Documentation/arm64/tagged-address-abi.rst
> >
> > diff --git a/Documentation/arm64/tagged-address-abi.rst b/Documentation/arm64/tagged-address-abi.rst
> > new file mode 100644
> > index 000000000000..f91a5d2ac865
> > --- /dev/null
> > +++ b/Documentation/arm64/tagged-address-abi.rst
> > @@ -0,0 +1,151 @@
> > +==========================
> > +AArch64 TAGGED ADDRESS ABI
> > +==========================
> > +
> > +Author: Vincenzo Frascino <vincenzo.frascino@arm.com>
> > +
> > +Date: 25 July 2019
> > +
> > +This document describes the usage and semantics of the Tagged Address
> > +ABI on AArch64 Linux.
> > +
> > +1. Introduction
> > +---------------
> > +
> > +On AArch64 the TCR_EL1.TBI0 bit has always been enabled, allowing userspace
> > +(EL0) to perform memory accesses through 64-bit pointers with a non-zero
> > +top byte. Such tagged pointers, however, were not allowed at the
> > +user-kernel syscall ABI boundary.
>
> I think we should drop the temporal language, so:
>
> "has always been enabled" => "is set by the kernel"
> "were not allowed" => "are not allowed by default"
>
> > +
> > +This document describes the relaxation of the syscall ABI that allows
> > +userspace to pass certain tagged pointers to kernel syscalls, as described
> > +in section 2.
> > +
> > +2. AArch64 Tagged Address ABI
> > +-----------------------------
> > +
> > +From the kernel syscall interface perspective and for the purposes of this
> > +document, a "valid tagged pointer" is a pointer with a potentially non-zero
> > +top-byte that references an address in the user process address space
> > +obtained in one of the following ways:
> > +
> > +- mmap() done by the process itself (or its parent), where either:
> > +
> > + - flags have the **MAP_ANONYMOUS** bit set
> > + - the file descriptor refers to a regular file (including those returned
> > + by memfd_create()) or **/dev/zero**
> > +
> > +- brk() system call done by the process itself (i.e. the heap area between
> > + the initial location of the program break at process creation and its
> > + current location).
> > +
> > +- any memory mapped by the kernel in the address space of the process
> > + during creation and with the same restrictions as for mmap() above (e.g.
> > + data, bss, stack).
> > +
> > +The AArch64 Tagged Address ABI is an opt-in feature and an application can
> > +control it via **prctl()** as follows:
> > +
> > +- **PR_SET_TAGGED_ADDR_CTRL**: enable or disable the AArch64 Tagged Address
> > + ABI for the calling process.
> > +
> > + The (unsigned int) arg2 argument is a bit mask describing the control mode
> > + used:
> > +
> > + - **PR_TAGGED_ADDR_ENABLE**: enable AArch64 Tagged Address ABI. Default
> > + status is disabled.
> > +
> > + The arguments arg3, arg4, and arg5 are ignored.
> > +
> > +- **PR_GET_TAGGED_ADDR_CTRL**: get the status of the AArch64 Tagged Address
> > + ABI for the calling process.
> > +
> > + The arguments arg2, arg3, arg4, and arg5 are ignored.
>
> I agree with Dave (H) that we should require these to be zero. We may be
> able to use arg2 to namespace things for PR_SET_TAGGED_ADDR_CTRL, but for
> PR_GET_TAGGED_ADDR_CTRL we'd have to add a new prctl if we wanted to extend
> it otherwise.
>
> > +The prctl(PR_SET_TAGGED_ADDR_CTRL, ...) will return -EINVAL if the
>
> *The* prctl? Maybe "Calling prctl(..." is better?
>
> > +AArch64 Tagged Address ABI is not available
> > +(CONFIG_ARM64_TAGGED_ADDR_ABI disabled or sysctl abi.tagged_addr=0).
>
> drop the brackets and say "because CONFIG_... is disabled or ..".
>
> > +
> > +The ABI properties set by the mechanism described above are inherited by
> > +threads of the same application and fork()'ed children but cleared by
> > +execve().
>
> Maybe just exec() here, since there are other flavours we shouldn't need to
> enumerate.
>
> > +Opting in (the prctl() option described above only) to or out of the
> > +AArch64 Tagged Address ABI can be disabled globally at runtime using the
> > +sysctl interface:
>
> This sentence reads really badly thanks to the random bracketed part.
>
> > +
> > +- **abi.tagged_addr**: a new sysctl interface that can be used to prevent
> > + applications from enabling or disabling the relaxed ABI. The sysctl
> > + supports the following configuration options:
> > +
> > + - **0**: disable the prctl(PR_SET_TAGGED_ADDR_CTRL) option to
> > + enable/disable the AArch64 Tagged Address ABI globally
>
> This is clunky because it sounds like we're enabling the ABI for everybody,
> where in actual fact we're enabling the controls for the ABI instead. It
> also applies equally to PR_GET_TAGGED_ADDR_CTRL (but see below). Given that
> we've already defined the prctl() above, I think we can just say:
>
> **0**: AArch64 Tagged Address ABI prctl() calls will return -EINVAL
> **1**: AArch64 Tagged Address ABI prctl() calls will behave as documented above.
>
> > + - **1** (Default): enable the prctl(PR_SET_TAGGED_ADDR_CTRL) option to
> > + enable/disable the AArch64 Tagged Address ABI globally
> > +
> > + Note that this sysctl does not affect the status of the AArch64 Tagged
> > + Address ABI of the running processes.
>
> Hmm, but it does mean that you can no longer ask if a previously running
> process is using tags. Is that intentional?
>
> > +When a process has successfully enabled the new ABI by invoking
> > +prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE), the following
> > +behaviours are guaranteed:
>
> nit: this also applies to processes that have inherited the new ABI
> bevaiour via fork() and haven't invoked the prctl() themselves.
>
> > +- Every currently available syscall, except the cases mentioned in section
>
> "currently available" is meaningless and should be removed
>
> > + 3, can accept any valid tagged pointer. The same rule is applicable to
> > + any syscall introduced in the future.
>
> Delete this last sentence.
>
> > +- The syscall behaviour is undefined for non valid tagged pointers.
>
> non valid => invalid
>
> although this needs to be better defined, I think.
>
> > +
> > +- Every valid tagged pointer is expected to work as an untagged one.
>
> What does that mean? Expected by who? What does "work" mean?
>
> > +A definition of the meaning of tagged pointers on AArch64 can be found in:
> > +Documentation/arm64/tagged-pointers.txt.
>
> .txt => .rst
>
> > +
> > +3. AArch64 Tagged Address ABI Exceptions
> > +-----------------------------------------
> > +
> > +The behaviour described in section 2, with particular reference to the
> > +acceptance by the syscalls of any valid tagged pointer, is not applicable
> > +to the following cases:
>
> Jeez louise...
>
> How about: "The following system call parameters must be untagged, regardless
> of the ABI relaxation:"
>
> > +
> > +- mmap() addr parameter.
> > +
> > +- mremap() new_address parameter.
> > +
> > +- prctl(PR_SET_MM, ``*``, ...) other than arg2 PR_SET_MM_MAP and
> > + PR_SET_MM_MAP_SIZE.
> > +
> > +- prctl(PR_SET_MM, PR_SET_MM_MAP{,_SIZE}, ...) struct prctl_mm_map fields.
>
> How did you generate this list and who will keep it up to date? How do you
> know you haven't missed anything?
What about shared memory system calls: shmat, shmdt? The latest "arm64: untag
user pointers passed to the kernel" series doesn't untag these, thus we should
indicate here that these too are no supported.
Thanks,
Andrew Murray
>
> > +Any attempt to use non-zero tagged pointers will lead to undefined
> > +behaviour.
>
> In the tagged pointer document we're slightly more specific and say that
> using non-zero address tags "may result in an error code being returned, a
> (fatal) signal being rasied, or other modes of failure". Maybe reuse that?
>
> > +4. Example of correct usage
> > +---------------------------
> > +.. code-block:: c
> > +
> > + void main(void)
> > + {
> > + static int tbi_enabled = 0;
> > + unsigned long tag = 0;
> > +
>
> Some comments won't go amiss here.
>
> > + char *ptr = mmap(NULL, PAGE_SIZE, PROT_READ | PROT_WRITE,
> > + MAP_ANONYMOUS, -1, 0);
> > +
> > + if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE,
> > + 0, 0, 0) == 0)
> > + tbi_enabled = 1;
> > +
> > + if (ptr == (void *)-1) /* MAP_FAILED */
> > + return -1;
> > +
> > + if (tbi_enabled)
> > + tag = rand() & 0xff;
> > +
> > + ptr = (char *)((unsigned long)ptr | (tag << TAG_SHIFT));
> > +
> > + *ptr = 'a';
> > +
> > + ...
> > + }
>
> Hmm, doesn't this snippet work today? You're not actually passing the
> tagged pointer back to the kernel...
>
> Will
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 9/9] arm64: Retrieve stolen time as paravirtualized guest
From: Steven Price @ 2019-08-12 10:39 UTC (permalink / raw)
To: Zenghui Yu
Cc: linux-kernel, kvm, Catalin Marinas, linux-doc, Russell King,
linux-arm-kernel, Marc Zyngier, Paolo Bonzini, Will Deacon,
kvmarm
In-Reply-To: <5d763c8e-9c06-c448-2644-25bfa0e57e8c@huawei.com>
On 09/08/2019 14:51, Zenghui Yu wrote:
[...]
> Hi Steven,
>
> Since userspace is not involved yet (right?), no one will create the
> PV_TIME device for guest (and no one will specify the IPA of the shared
> stolen time region), and I guess we will get a "not supported" error
> here.
>
> So what should we do if we want to test this series now? Any userspace
> tools? If no, do you have any plans for userspace developing? ;-)
At the moment I have the following patch to kvmtool which creates the
PV_TIME device - this isn't in a state to go upstream, and Marc has
asked that I rework the memory allocation, so this will need to change.
It's a little ugly as it simply reserves the first page of RAM to use
for the PV time structures.
----8<----
diff --git a/Makefile b/Makefile
index 3862112..a79956b 100644
--- a/Makefile
+++ b/Makefile
@@ -158,7 +158,7 @@ endif
# ARM
OBJS_ARM_COMMON := arm/fdt.o arm/gic.o arm/gicv2m.o arm/ioport.o \
arm/kvm.o arm/kvm-cpu.o arm/pci.o arm/timer.o \
- arm/pmu.o
+ arm/pmu.o arm/pvtime.o
HDRS_ARM_COMMON := arm/include
ifeq ($(ARCH), arm)
DEFINES += -DCONFIG_ARM
diff --git a/arm/fdt.c b/arm/fdt.c
index c80e6da..19eccbc 100644
--- a/arm/fdt.c
+++ b/arm/fdt.c
@@ -119,6 +119,7 @@ static int setup_fdt(struct kvm *kvm)
/* Create new tree without a reserve map */
_FDT(fdt_create(fdt, FDT_MAX_SIZE));
+ _FDT(fdt_add_reservemap_entry(fdt, kvm->arch.memory_guest_start, 4096));
_FDT(fdt_finish_reservemap(fdt));
/* Header */
diff --git a/arm/kvm.c b/arm/kvm.c
index 1f85fc6..8bbfef1 100644
--- a/arm/kvm.c
+++ b/arm/kvm.c
@@ -11,6 +11,8 @@
#include <linux/kvm.h>
#include <linux/sizes.h>
+int pvtime_create(struct kvm *kvm);
+
struct kvm_ext kvm_req_ext[] = {
{ DEFINE_KVM_EXT(KVM_CAP_IRQCHIP) },
{ DEFINE_KVM_EXT(KVM_CAP_ONE_REG) },
@@ -86,6 +88,10 @@ void kvm__arch_init(struct kvm *kvm, const char *hugetlbfs_path, u64 ram_size)
/* Create the virtual GIC. */
if (gic__create(kvm, kvm->cfg.arch.irqchip))
die("Failed to create virtual GIC");
+
+ /* Setup PV time */
+ if (pvtime_create(kvm))
+ die("Failed to initialise PV time");
}
#define FDT_ALIGN SZ_2M
diff --git a/arm/pvtime.c b/arm/pvtime.c
new file mode 100644
index 0000000..abcaab3
--- /dev/null
+++ b/arm/pvtime.c
@@ -0,0 +1,77 @@
+#include "kvm/kvm.h"
+
+#define KVM_DEV_TYPE_ARM_PV_TIME (KVM_DEV_TYPE_ARM_VGIC_ITS+2)
+
+/* Device Control API: PV_TIME */
+#define KVM_DEV_ARM_PV_TIME_PADDR 0
+#define KVM_DEV_ARM_PV_TIME_FREQUENCY 3
+
+#define KVM_DEV_ARM_PV_TIME_ST 0
+#define KVM_DEV_ARM_PV_TIME_LPT 1
+
+static int pvtime_fd;
+
+int pvtime_create(struct kvm *kvm);
+
+int pvtime_create(struct kvm *kvm)
+{
+ int err;
+ u64 lpt_paddr = 0x10000000;
+ u64 st_paddr = lpt_paddr + 4096;
+ u32 frequency = 100 * 1000 * 1000;
+
+ printf("lpt_paddr=%llx\n", lpt_paddr);
+
+ struct kvm_create_device pvtime_device = {
+ .type = KVM_DEV_TYPE_ARM_PV_TIME,
+ .flags = 0,
+ };
+
+ err = ioctl(kvm->vm_fd, KVM_CREATE_DEVICE, &pvtime_device);
+ if (err) {
+ printf("Failed to create PV device\n");
+ return 0;
+ }
+
+ pvtime_fd = pvtime_device.fd;
+
+ struct kvm_device_attr lpt_base = {
+ .group = KVM_DEV_ARM_PV_TIME_PADDR,
+ .attr = KVM_DEV_ARM_PV_TIME_LPT,
+ .addr = (u64)(unsigned long)&lpt_paddr
+ };
+ struct kvm_device_attr st_base = {
+ .group = KVM_DEV_ARM_PV_TIME_PADDR,
+ .attr = KVM_DEV_ARM_PV_TIME_ST,
+ .addr = (u64)(unsigned long)&st_paddr
+ };
+
+ struct kvm_device_attr lpt_freq = {
+ .group = KVM_DEV_ARM_PV_TIME_FREQUENCY,
+ .attr = KVM_DEV_ARM_PV_TIME_LPT,
+ .addr = (u64)(unsigned long)&frequency
+ };
+
+ err = ioctl(pvtime_fd, KVM_SET_DEVICE_ATTR, &lpt_base);
+ if (err) {
+ perror("ioctl lpt_base failed");
+ printf("Ignoring LPT...\n");
+ }
+ err = ioctl(pvtime_fd, KVM_SET_DEVICE_ATTR, &st_base);
+ if (err) {
+ perror("ioctl st_base failed");
+ goto out_err;
+ }
+ err = ioctl(pvtime_fd, KVM_SET_DEVICE_ATTR, &lpt_freq);
+ if (err) {
+ perror("ioctl lpt_freq failed");
+ printf("Ignoring LPT...\n");
+ }
+
+ printf("PV time setup\n");
+
+ return 0;
+out_err:
+ close(pvtime_fd);
+ return err;
+}
^ permalink raw reply related
* Re: [PATCH 3/3] driver/core: Fix build error when SRCU and lockdep disabled
From: Greg Kroah-Hartman @ 2019-08-12 5:02 UTC (permalink / raw)
To: Joel Fernandes (Google)
Cc: linux-kernel, kbuild test robot, Jonathan Corbet, Josh Triplett,
Lai Jiangshan, linux-doc, Mathieu Desnoyers, Paul E. McKenney,
Rafael J. Wysocki, rcu, Steven Rostedt, Tejun Heo
In-Reply-To: <20190811221111.99401-3-joel@joelfernandes.org>
On Sun, Aug 11, 2019 at 06:11:11PM -0400, Joel Fernandes (Google) wrote:
> Properly check if lockdep lock checking is disabled at config time. If
> so, then lock_is_held() is undefined so don't do any checking.
>
> This fix is similar to the pattern used in srcu_read_lock_held().
>
> Link: https://lore.kernel.org/lkml/201908080026.WSAFx14k%25lkp@intel.com/
> Fixes: c9e4d3a2fee8 ("acpi: Use built-in RCU list checking for acpi_ioremaps list")
What tree is this commit in?
> Reported-by: kbuild test robot <lkp@intel.com>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> ---
> This patch is based on the -rcu dev branch.
Ah...
> drivers/base/core.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 32cf83d1c744..fe25cf690562 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -99,7 +99,11 @@ void device_links_read_unlock(int not_used)
>
> int device_links_read_lock_held(void)
> {
> - return lock_is_held(&device_links_lock);
> +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> + return lock_is_held(&(device_links_lock.dep_map));
> +#else
> + return 1;
> +#endif
return 1? So the lock is always held?
confused,
greg k-h
^ permalink raw reply
* Re: [PATCH v3 2/2] hwmon: pmbus: Add Inspur Power System power supply driver
From: John Wang @ 2019-08-12 4:48 UTC (permalink / raw)
To: Guenter Roeck
Cc: jdelvare, corbet, linux-hwmon, linux-doc, linux-kernel, miltonm,
Yu Lei, duanzhijia01, Joel Stanley, OpenBMC Maillist
In-Reply-To: <6cf699d9-6efb-f701-d5ab-6f624e515ab8@roeck-us.net>
On Mon, Aug 12, 2019 at 12:21 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 8/11/19 7:53 PM, John Wang wrote:
> > Add the driver to monitor Inspur Power System power supplies
> > with hwmon over pmbus.
> >
> > This driver adds sysfs attributes for additional power supply data,
> > including vendor, model, part_number, serial number,
> > firmware revision, hardware revision, and psu mode(active/standby).
> >
> > Signed-off-by: John Wang <wangzqbj@inspur.com>
> > ---
> > v3:
> > - Sort kconfig/makefile entries alphabetically
> > - Remove unnecessary initialization
> > - Use ATTRIBUTE_GROUPS instead of expanding directly
> > - Use memscan to avoid reimplementation
> > v2:
> > - Fix typos in commit message
> > - Invert Christmas tree
> > - Configure device with sysfs attrs, not debugfs entries
> > - Fix errno in fw_version_read, ENODATA to EPROTO
> > - Change the print format of fw-version
> > - Use sysfs_streq instead of strcmp("xxx" "\n", "xxx")
> > - Document sysfs attributes
> > ---
> > Documentation/hwmon/inspur-ipsps1.rst | 79 +++++++++
> > drivers/hwmon/pmbus/Kconfig | 9 +
> > drivers/hwmon/pmbus/Makefile | 41 ++---
> > drivers/hwmon/pmbus/inspur-ipsps.c | 226 ++++++++++++++++++++++++++
> > 4 files changed, 335 insertions(+), 20 deletions(-)
> > create mode 100644 Documentation/hwmon/inspur-ipsps1.rst
> > create mode 100644 drivers/hwmon/pmbus/inspur-ipsps.c
> >
> > diff --git a/Documentation/hwmon/inspur-ipsps1.rst b/Documentation/hwmon/inspur-ipsps1.rst
> > new file mode 100644
> > index 000000000000..aa19f0ccc8b0
> > --- /dev/null
> > +++ b/Documentation/hwmon/inspur-ipsps1.rst
> > @@ -0,0 +1,79 @@
> > +Kernel driver inspur-ipsps1
> > +=======================
> > +
> > +Supported chips:
> > +
> > + * Inspur Power System power supply unit
> > +
> > +Author: John Wang <wangzqbj@inspur.com>
> > +
> > +Description
> > +-----------
> > +
> > +This driver supports Inspur Power System power supplies. This driver
> > +is a client to the core PMBus driver.
> > +
> > +Usage Notes
> > +-----------
> > +
> > +This driver does not auto-detect devices. You will have to instantiate the
> > +devices explicitly. Please see Documentation/i2c/instantiating-devices for
> > +details.
> > +
> > +Sysfs entries
> > +-------------
> > +
> > +The following attributes are supported:
> > +
> > +======================= ======================================================
> > +curr1_input Measured input current
> > +curr1_label "iin"
> > +curr1_max Maximum current
> > +curr1_max_alarm Current high alarm
> > +curr2_input Measured output current in mA.
> > +curr2_label "iout1"
> > +curr2_crit Critical maximum current
> > +curr2_crit_alarm Current critical high alarm
> > +curr2_max Maximum current
> > +curr2_max_alarm Current high alarm
> > +
> > +fan1_alarm Fan 1 warning.
> > +fan1_fault Fan 1 fault.
> > +fan1_input Fan 1 speed in RPM.
> > +
> > +in1_alarm Input voltage under-voltage alarm.
> > +in1_input Measured input voltage in mV.
> > +in1_label "vin"
> > +in2_input Measured output voltage in mV.
> > +in2_label "vout1"
> > +in2_lcrit Critical minimum output voltage
> > +in2_lcrit_alarm Output voltage critical low alarm
> > +in2_max Maximum output voltage
> > +in2_max_alarm Output voltage high alarm
> > +in2_min Minimum output voltage
> > +in2_min_alarm Output voltage low alarm
> > +
> > +power1_alarm Input fault or alarm.
> > +power1_input Measured input power in uW.
> > +power1_label "pin"
> > +power1_max Input power limit
> > +power2_max_alarm Output power high alarm
> > +power2_max Output power limit
> > +power2_input Measured output power in uW.
> > +power2_label "pout"
> > +
> > +temp[1-3]_input Measured temperature
> > +temp[1-2]_max Maximum temperature
> > +temp[1-3]_max_alarm Temperature high alarm
> > +
> > +vendor Manufacturer name
> > +model Product model
> > +part_number Product part number
> > +serial_number Product serial number
> > +fw_version Firmware version
> > +hw_version Hardware version
> > +mode Work mode. Can be set to active or
> > + standby, when set to standby, PSU will
> > + automatically switch between standby
> > + and redundancy mode.
> > +======================= ======================================================
> > diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> > index 30751eb9550a..2370fce6e816 100644
> > --- a/drivers/hwmon/pmbus/Kconfig
> > +++ b/drivers/hwmon/pmbus/Kconfig
> > @@ -46,6 +46,15 @@ config SENSORS_IBM_CFFPS
> > This driver can also be built as a module. If so, the module will
> > be called ibm-cffps.
> >
> > +config SENSORS_INSPUR_IPSPS
> > + tristate "INSPUR Power System Power Supply"
> > + help
> > + If you say yes here you get hardware monitoring support for the INSPUR
> > + Power System power supply.
> > +
> > + This driver can also be built as a module. If so, the module will
> > + be called inspur-ipsps.
> > +
> > config SENSORS_IR35221
> > tristate "Infineon IR35221"
> > help
> > diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> > index 2219b9300316..c4f82f65f2ad 100644
> > --- a/drivers/hwmon/pmbus/Makefile
> > +++ b/drivers/hwmon/pmbus/Makefile
> > @@ -3,23 +3,24 @@
> > # Makefile for PMBus chip drivers.
> > #
> >
> > -obj-$(CONFIG_PMBUS) += pmbus_core.o
> > -obj-$(CONFIG_SENSORS_PMBUS) += pmbus.o
> > -obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o
> > -obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o
> > -obj-$(CONFIG_SENSORS_IR35221) += ir35221.o
> > -obj-$(CONFIG_SENSORS_IR38064) += ir38064.o
> > -obj-$(CONFIG_SENSORS_ISL68137) += isl68137.o
> > -obj-$(CONFIG_SENSORS_LM25066) += lm25066.o
> > -obj-$(CONFIG_SENSORS_LTC2978) += ltc2978.o
> > -obj-$(CONFIG_SENSORS_LTC3815) += ltc3815.o
> > -obj-$(CONFIG_SENSORS_MAX16064) += max16064.o
> > -obj-$(CONFIG_SENSORS_MAX20751) += max20751.o
> > -obj-$(CONFIG_SENSORS_MAX31785) += max31785.o
> > -obj-$(CONFIG_SENSORS_MAX34440) += max34440.o
> > -obj-$(CONFIG_SENSORS_MAX8688) += max8688.o
> > -obj-$(CONFIG_SENSORS_TPS40422) += tps40422.o
> > -obj-$(CONFIG_SENSORS_TPS53679) += tps53679.o
> > -obj-$(CONFIG_SENSORS_UCD9000) += ucd9000.o
> > -obj-$(CONFIG_SENSORS_UCD9200) += ucd9200.o
> > -obj-$(CONFIG_SENSORS_ZL6100) += zl6100.o
> > +obj-$(CONFIG_PMBUS) += pmbus_core.o
> > +obj-$(CONFIG_SENSORS_PMBUS) += pmbus.o
> > +obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o
> > +obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o
> > +obj-$(CONFIG_SENSORS_INSPUR_IPSPS) += inspur-ipsps.o
> > +obj-$(CONFIG_SENSORS_IR35221) += ir35221.o
> > +obj-$(CONFIG_SENSORS_IR38064) += ir38064.o
> > +obj-$(CONFIG_SENSORS_ISL68137) += isl68137.o
> > +obj-$(CONFIG_SENSORS_LM25066) += lm25066.o
> > +obj-$(CONFIG_SENSORS_LTC2978) += ltc2978.o
> > +obj-$(CONFIG_SENSORS_LTC3815) += ltc3815.o
> > +obj-$(CONFIG_SENSORS_MAX16064) += max16064.o
> > +obj-$(CONFIG_SENSORS_MAX20751) += max20751.o
> > +obj-$(CONFIG_SENSORS_MAX31785) += max31785.o
> > +obj-$(CONFIG_SENSORS_MAX34440) += max34440.o
> > +obj-$(CONFIG_SENSORS_MAX8688) += max8688.o
> > +obj-$(CONFIG_SENSORS_TPS40422) += tps40422.o
> > +obj-$(CONFIG_SENSORS_TPS53679) += tps53679.o
> > +obj-$(CONFIG_SENSORS_UCD9000) += ucd9000.o
> > +obj-$(CONFIG_SENSORS_UCD9200) += ucd9200.o
> > +obj-$(CONFIG_SENSORS_ZL6100) += zl6100.o
>
> Are you serious ? I am not going to accept this.
Sorry for this commit.
I just want to add SENSORS_INSPUR_IPSPS to the makefile in
alphabetical order. But this will make the makefile look less tidy, so
I added a tab to adjust.
So I should
1. Add SENSOR_INSPUR_IPSPS to the end of the file
2. Add SENSOR_INSPUR_IPSPS in alphabetical order, without additional tab
3. other suggestions
Please let me know which one is better?
Thanks for your review.
>
> Guenter
>
> > diff --git a/drivers/hwmon/pmbus/inspur-ipsps.c b/drivers/hwmon/pmbus/inspur-ipsps.c
> > new file mode 100644
> > index 000000000000..fa981b881a60
> > --- /dev/null
> > +++ b/drivers/hwmon/pmbus/inspur-ipsps.c
> > @@ -0,0 +1,226 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright 2019 Inspur Corp.
> > + */
> > +
> > +#include <linux/debugfs.h>
> > +#include <linux/device.h>
> > +#include <linux/fs.h>
> > +#include <linux/i2c.h>
> > +#include <linux/module.h>
> > +#include <linux/pmbus.h>
> > +#include <linux/hwmon-sysfs.h>
> > +
> > +#include "pmbus.h"
> > +
> > +#define IPSPS_REG_VENDOR_ID 0x99
> > +#define IPSPS_REG_MODEL 0x9A
> > +#define IPSPS_REG_FW_VERSION 0x9B
> > +#define IPSPS_REG_PN 0x9C
> > +#define IPSPS_REG_SN 0x9E
> > +#define IPSPS_REG_HW_VERSION 0xB0
> > +#define IPSPS_REG_MODE 0xFC
> > +
> > +#define MODE_ACTIVE 0x55
> > +#define MODE_STANDBY 0x0E
> > +#define MODE_REDUNDANCY 0x00
> > +
> > +#define MODE_ACTIVE_STRING "active"
> > +#define MODE_STANDBY_STRING "standby"
> > +#define MODE_REDUNDANCY_STRING "redundancy"
> > +
> > +enum ipsps_index {
> > + vendor,
> > + model,
> > + fw_version,
> > + part_number,
> > + serial_number,
> > + hw_version,
> > + mode,
> > + num_regs,
> > +};
> > +
> > +static const u8 ipsps_regs[num_regs] = {
> > + [vendor] = IPSPS_REG_VENDOR_ID,
> > + [model] = IPSPS_REG_MODEL,
> > + [fw_version] = IPSPS_REG_FW_VERSION,
> > + [part_number] = IPSPS_REG_PN,
> > + [serial_number] = IPSPS_REG_SN,
> > + [hw_version] = IPSPS_REG_HW_VERSION,
> > + [mode] = IPSPS_REG_MODE,
> > +};
> > +
> > +static ssize_t ipsps_string_show(struct device *dev,
> > + struct device_attribute *devattr,
> > + char *buf)
> > +{
> > + u8 reg;
> > + int rc;
> > + char *p;
> > + char data[I2C_SMBUS_BLOCK_MAX + 1];
> > + struct i2c_client *client = to_i2c_client(dev->parent);
> > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> > +
> > + reg = ipsps_regs[attr->index];
> > + rc = i2c_smbus_read_block_data(client, reg, data);
> > + if (rc < 0)
> > + return rc;
> > +
> > + /* filled with printable characters, ending with # */
> > + p = memscan(data, '#', rc);
> > + *p = '\0';
> > +
> > + return snprintf(buf, PAGE_SIZE, "%s\n", data);
> > +}
> > +
> > +static ssize_t ipsps_fw_version_show(struct device *dev,
> > + struct device_attribute *devattr,
> > + char *buf)
> > +{
> > + u8 reg;
> > + int rc;
> > + u8 data[I2C_SMBUS_BLOCK_MAX] = { 0 };
> > + struct i2c_client *client = to_i2c_client(dev->parent);
> > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> > +
> > + reg = ipsps_regs[attr->index];
> > + rc = i2c_smbus_read_block_data(client, reg, data);
> > + if (rc < 0)
> > + return rc;
> > +
> > + if (rc != 6)
> > + return -EPROTO;
> > +
> > + return snprintf(buf, PAGE_SIZE, "%u.%02u%u-%u.%02u\n",
> > + data[1], data[2]/* < 100 */, data[3]/*< 10*/,
> > + data[4], data[5]/* < 100 */);
> > +}
> > +
> > +static ssize_t ipsps_mode_show(struct device *dev,
> > + struct device_attribute *devattr, char *buf)
> > +{
> > + u8 reg;
> > + int rc;
> > + struct i2c_client *client = to_i2c_client(dev->parent);
> > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> > +
> > + reg = ipsps_regs[attr->index];
> > + rc = i2c_smbus_read_byte_data(client, reg);
> > + if (rc < 0)
> > + return rc;
> > +
> > + switch (rc) {
> > + case MODE_ACTIVE:
> > + return snprintf(buf, PAGE_SIZE, "[%s] %s %s\n",
> > + MODE_ACTIVE_STRING,
> > + MODE_STANDBY_STRING, MODE_REDUNDANCY_STRING);
> > + case MODE_STANDBY:
> > + return snprintf(buf, PAGE_SIZE, "%s [%s] %s\n",
> > + MODE_ACTIVE_STRING,
> > + MODE_STANDBY_STRING, MODE_REDUNDANCY_STRING);
> > + case MODE_REDUNDANCY:
> > + return snprintf(buf, PAGE_SIZE, "%s %s [%s]\n",
> > + MODE_ACTIVE_STRING,
> > + MODE_STANDBY_STRING, MODE_REDUNDANCY_STRING);
> > + default:
> > + return snprintf(buf, PAGE_SIZE, "unspecified\n");
> > + }
> > +}
> > +
> > +static ssize_t ipsps_mode_store(struct device *dev,
> > + struct device_attribute *devattr,
> > + const char *buf, size_t count)
> > +{
> > + u8 reg;
> > + int rc;
> > + struct i2c_client *client = to_i2c_client(dev->parent);
> > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> > +
> > + reg = ipsps_regs[attr->index];
> > + if (sysfs_streq(MODE_STANDBY_STRING, buf)) {
> > + rc = i2c_smbus_write_byte_data(client, reg,
> > + MODE_STANDBY);
> > + if (rc < 0)
> > + return rc;
> > + return count;
> > + } else if (sysfs_streq(MODE_ACTIVE_STRING, buf)) {
> > + rc = i2c_smbus_write_byte_data(client, reg,
> > + MODE_ACTIVE);
> > + if (rc < 0)
> > + return rc;
> > + return count;
> > + }
> > +
> > + return -EINVAL;
> > +}
> > +
> > +static SENSOR_DEVICE_ATTR_RO(vendor, ipsps_string, vendor);
> > +static SENSOR_DEVICE_ATTR_RO(model, ipsps_string, model);
> > +static SENSOR_DEVICE_ATTR_RO(part_number, ipsps_string, part_number);
> > +static SENSOR_DEVICE_ATTR_RO(serial_number, ipsps_string, serial_number);
> > +static SENSOR_DEVICE_ATTR_RO(hw_version, ipsps_string, hw_version);
> > +static SENSOR_DEVICE_ATTR_RO(fw_version, ipsps_fw_version, fw_version);
> > +static SENSOR_DEVICE_ATTR_RW(mode, ipsps_mode, mode);
> > +
> > +static struct attribute *ipsps_attrs[] = {
> > + &sensor_dev_attr_vendor.dev_attr.attr,
> > + &sensor_dev_attr_model.dev_attr.attr,
> > + &sensor_dev_attr_part_number.dev_attr.attr,
> > + &sensor_dev_attr_serial_number.dev_attr.attr,
> > + &sensor_dev_attr_hw_version.dev_attr.attr,
> > + &sensor_dev_attr_fw_version.dev_attr.attr,
> > + &sensor_dev_attr_mode.dev_attr.attr,
> > + NULL,
> > +};
> > +
> > +ATTRIBUTE_GROUPS(ipsps);
> > +
> > +static struct pmbus_driver_info ipsps_info = {
> > + .pages = 1,
> > + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT |
> > + PMBUS_HAVE_IIN | PMBUS_HAVE_POUT | PMBUS_HAVE_PIN |
> > + PMBUS_HAVE_FAN12 | PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 |
> > + PMBUS_HAVE_TEMP3 | PMBUS_HAVE_STATUS_VOUT |
> > + PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT |
> > + PMBUS_HAVE_STATUS_TEMP | PMBUS_HAVE_STATUS_FAN12,
> > + .groups = ipsps_groups,
> > +};
> > +
> > +static struct pmbus_platform_data ipsps_pdata = {
> > + .flags = PMBUS_SKIP_STATUS_CHECK,
> > +};
> > +
> > +static int ipsps_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + client->dev.platform_data = &ipsps_pdata;
> > + return pmbus_do_probe(client, id, &ipsps_info);
> > +}
> > +
> > +static const struct i2c_device_id ipsps_id[] = {
> > + { "inspur_ipsps1", 0 },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(i2c, ipsps_id);
> > +
> > +static const struct of_device_id ipsps_of_match[] = {
> > + { .compatible = "inspur,ipsps1" },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, ipsps_of_match);
> > +
> > +static struct i2c_driver ipsps_driver = {
> > + .driver = {
> > + .name = "inspur-ipsps",
> > + .of_match_table = ipsps_of_match,
> > + },
> > + .probe = ipsps_probe,
> > + .remove = pmbus_do_remove,
> > + .id_table = ipsps_id,
> > +};
> > +
> > +module_i2c_driver(ipsps_driver);
> > +
> > +MODULE_AUTHOR("John Wang");
> > +MODULE_DESCRIPTION("PMBus driver for Inspur Power System power supplies");
> > +MODULE_LICENSE("GPL");
> >
>
^ permalink raw reply
* Re: [PATCH v3 2/2] hwmon: pmbus: Add Inspur Power System power supply driver
From: Guenter Roeck @ 2019-08-12 4:21 UTC (permalink / raw)
To: John Wang, jdelvare, corbet, linux-hwmon, linux-doc, linux-kernel,
miltonm, mine260309, duanzhijia01, joel, openbmc
In-Reply-To: <20190812025309.15702-1-wangzqbj@inspur.com>
On 8/11/19 7:53 PM, John Wang wrote:
> Add the driver to monitor Inspur Power System power supplies
> with hwmon over pmbus.
>
> This driver adds sysfs attributes for additional power supply data,
> including vendor, model, part_number, serial number,
> firmware revision, hardware revision, and psu mode(active/standby).
>
> Signed-off-by: John Wang <wangzqbj@inspur.com>
> ---
> v3:
> - Sort kconfig/makefile entries alphabetically
> - Remove unnecessary initialization
> - Use ATTRIBUTE_GROUPS instead of expanding directly
> - Use memscan to avoid reimplementation
> v2:
> - Fix typos in commit message
> - Invert Christmas tree
> - Configure device with sysfs attrs, not debugfs entries
> - Fix errno in fw_version_read, ENODATA to EPROTO
> - Change the print format of fw-version
> - Use sysfs_streq instead of strcmp("xxx" "\n", "xxx")
> - Document sysfs attributes
> ---
> Documentation/hwmon/inspur-ipsps1.rst | 79 +++++++++
> drivers/hwmon/pmbus/Kconfig | 9 +
> drivers/hwmon/pmbus/Makefile | 41 ++---
> drivers/hwmon/pmbus/inspur-ipsps.c | 226 ++++++++++++++++++++++++++
> 4 files changed, 335 insertions(+), 20 deletions(-)
> create mode 100644 Documentation/hwmon/inspur-ipsps1.rst
> create mode 100644 drivers/hwmon/pmbus/inspur-ipsps.c
>
> diff --git a/Documentation/hwmon/inspur-ipsps1.rst b/Documentation/hwmon/inspur-ipsps1.rst
> new file mode 100644
> index 000000000000..aa19f0ccc8b0
> --- /dev/null
> +++ b/Documentation/hwmon/inspur-ipsps1.rst
> @@ -0,0 +1,79 @@
> +Kernel driver inspur-ipsps1
> +=======================
> +
> +Supported chips:
> +
> + * Inspur Power System power supply unit
> +
> +Author: John Wang <wangzqbj@inspur.com>
> +
> +Description
> +-----------
> +
> +This driver supports Inspur Power System power supplies. This driver
> +is a client to the core PMBus driver.
> +
> +Usage Notes
> +-----------
> +
> +This driver does not auto-detect devices. You will have to instantiate the
> +devices explicitly. Please see Documentation/i2c/instantiating-devices for
> +details.
> +
> +Sysfs entries
> +-------------
> +
> +The following attributes are supported:
> +
> +======================= ======================================================
> +curr1_input Measured input current
> +curr1_label "iin"
> +curr1_max Maximum current
> +curr1_max_alarm Current high alarm
> +curr2_input Measured output current in mA.
> +curr2_label "iout1"
> +curr2_crit Critical maximum current
> +curr2_crit_alarm Current critical high alarm
> +curr2_max Maximum current
> +curr2_max_alarm Current high alarm
> +
> +fan1_alarm Fan 1 warning.
> +fan1_fault Fan 1 fault.
> +fan1_input Fan 1 speed in RPM.
> +
> +in1_alarm Input voltage under-voltage alarm.
> +in1_input Measured input voltage in mV.
> +in1_label "vin"
> +in2_input Measured output voltage in mV.
> +in2_label "vout1"
> +in2_lcrit Critical minimum output voltage
> +in2_lcrit_alarm Output voltage critical low alarm
> +in2_max Maximum output voltage
> +in2_max_alarm Output voltage high alarm
> +in2_min Minimum output voltage
> +in2_min_alarm Output voltage low alarm
> +
> +power1_alarm Input fault or alarm.
> +power1_input Measured input power in uW.
> +power1_label "pin"
> +power1_max Input power limit
> +power2_max_alarm Output power high alarm
> +power2_max Output power limit
> +power2_input Measured output power in uW.
> +power2_label "pout"
> +
> +temp[1-3]_input Measured temperature
> +temp[1-2]_max Maximum temperature
> +temp[1-3]_max_alarm Temperature high alarm
> +
> +vendor Manufacturer name
> +model Product model
> +part_number Product part number
> +serial_number Product serial number
> +fw_version Firmware version
> +hw_version Hardware version
> +mode Work mode. Can be set to active or
> + standby, when set to standby, PSU will
> + automatically switch between standby
> + and redundancy mode.
> +======================= ======================================================
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 30751eb9550a..2370fce6e816 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -46,6 +46,15 @@ config SENSORS_IBM_CFFPS
> This driver can also be built as a module. If so, the module will
> be called ibm-cffps.
>
> +config SENSORS_INSPUR_IPSPS
> + tristate "INSPUR Power System Power Supply"
> + help
> + If you say yes here you get hardware monitoring support for the INSPUR
> + Power System power supply.
> +
> + This driver can also be built as a module. If so, the module will
> + be called inspur-ipsps.
> +
> config SENSORS_IR35221
> tristate "Infineon IR35221"
> help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index 2219b9300316..c4f82f65f2ad 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -3,23 +3,24 @@
> # Makefile for PMBus chip drivers.
> #
>
> -obj-$(CONFIG_PMBUS) += pmbus_core.o
> -obj-$(CONFIG_SENSORS_PMBUS) += pmbus.o
> -obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o
> -obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o
> -obj-$(CONFIG_SENSORS_IR35221) += ir35221.o
> -obj-$(CONFIG_SENSORS_IR38064) += ir38064.o
> -obj-$(CONFIG_SENSORS_ISL68137) += isl68137.o
> -obj-$(CONFIG_SENSORS_LM25066) += lm25066.o
> -obj-$(CONFIG_SENSORS_LTC2978) += ltc2978.o
> -obj-$(CONFIG_SENSORS_LTC3815) += ltc3815.o
> -obj-$(CONFIG_SENSORS_MAX16064) += max16064.o
> -obj-$(CONFIG_SENSORS_MAX20751) += max20751.o
> -obj-$(CONFIG_SENSORS_MAX31785) += max31785.o
> -obj-$(CONFIG_SENSORS_MAX34440) += max34440.o
> -obj-$(CONFIG_SENSORS_MAX8688) += max8688.o
> -obj-$(CONFIG_SENSORS_TPS40422) += tps40422.o
> -obj-$(CONFIG_SENSORS_TPS53679) += tps53679.o
> -obj-$(CONFIG_SENSORS_UCD9000) += ucd9000.o
> -obj-$(CONFIG_SENSORS_UCD9200) += ucd9200.o
> -obj-$(CONFIG_SENSORS_ZL6100) += zl6100.o
> +obj-$(CONFIG_PMBUS) += pmbus_core.o
> +obj-$(CONFIG_SENSORS_PMBUS) += pmbus.o
> +obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o
> +obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o
> +obj-$(CONFIG_SENSORS_INSPUR_IPSPS) += inspur-ipsps.o
> +obj-$(CONFIG_SENSORS_IR35221) += ir35221.o
> +obj-$(CONFIG_SENSORS_IR38064) += ir38064.o
> +obj-$(CONFIG_SENSORS_ISL68137) += isl68137.o
> +obj-$(CONFIG_SENSORS_LM25066) += lm25066.o
> +obj-$(CONFIG_SENSORS_LTC2978) += ltc2978.o
> +obj-$(CONFIG_SENSORS_LTC3815) += ltc3815.o
> +obj-$(CONFIG_SENSORS_MAX16064) += max16064.o
> +obj-$(CONFIG_SENSORS_MAX20751) += max20751.o
> +obj-$(CONFIG_SENSORS_MAX31785) += max31785.o
> +obj-$(CONFIG_SENSORS_MAX34440) += max34440.o
> +obj-$(CONFIG_SENSORS_MAX8688) += max8688.o
> +obj-$(CONFIG_SENSORS_TPS40422) += tps40422.o
> +obj-$(CONFIG_SENSORS_TPS53679) += tps53679.o
> +obj-$(CONFIG_SENSORS_UCD9000) += ucd9000.o
> +obj-$(CONFIG_SENSORS_UCD9200) += ucd9200.o
> +obj-$(CONFIG_SENSORS_ZL6100) += zl6100.o
Are you serious ? I am not going to accept this.
Guenter
> diff --git a/drivers/hwmon/pmbus/inspur-ipsps.c b/drivers/hwmon/pmbus/inspur-ipsps.c
> new file mode 100644
> index 000000000000..fa981b881a60
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/inspur-ipsps.c
> @@ -0,0 +1,226 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2019 Inspur Corp.
> + */
> +
> +#include <linux/debugfs.h>
> +#include <linux/device.h>
> +#include <linux/fs.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/pmbus.h>
> +#include <linux/hwmon-sysfs.h>
> +
> +#include "pmbus.h"
> +
> +#define IPSPS_REG_VENDOR_ID 0x99
> +#define IPSPS_REG_MODEL 0x9A
> +#define IPSPS_REG_FW_VERSION 0x9B
> +#define IPSPS_REG_PN 0x9C
> +#define IPSPS_REG_SN 0x9E
> +#define IPSPS_REG_HW_VERSION 0xB0
> +#define IPSPS_REG_MODE 0xFC
> +
> +#define MODE_ACTIVE 0x55
> +#define MODE_STANDBY 0x0E
> +#define MODE_REDUNDANCY 0x00
> +
> +#define MODE_ACTIVE_STRING "active"
> +#define MODE_STANDBY_STRING "standby"
> +#define MODE_REDUNDANCY_STRING "redundancy"
> +
> +enum ipsps_index {
> + vendor,
> + model,
> + fw_version,
> + part_number,
> + serial_number,
> + hw_version,
> + mode,
> + num_regs,
> +};
> +
> +static const u8 ipsps_regs[num_regs] = {
> + [vendor] = IPSPS_REG_VENDOR_ID,
> + [model] = IPSPS_REG_MODEL,
> + [fw_version] = IPSPS_REG_FW_VERSION,
> + [part_number] = IPSPS_REG_PN,
> + [serial_number] = IPSPS_REG_SN,
> + [hw_version] = IPSPS_REG_HW_VERSION,
> + [mode] = IPSPS_REG_MODE,
> +};
> +
> +static ssize_t ipsps_string_show(struct device *dev,
> + struct device_attribute *devattr,
> + char *buf)
> +{
> + u8 reg;
> + int rc;
> + char *p;
> + char data[I2C_SMBUS_BLOCK_MAX + 1];
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> +
> + reg = ipsps_regs[attr->index];
> + rc = i2c_smbus_read_block_data(client, reg, data);
> + if (rc < 0)
> + return rc;
> +
> + /* filled with printable characters, ending with # */
> + p = memscan(data, '#', rc);
> + *p = '\0';
> +
> + return snprintf(buf, PAGE_SIZE, "%s\n", data);
> +}
> +
> +static ssize_t ipsps_fw_version_show(struct device *dev,
> + struct device_attribute *devattr,
> + char *buf)
> +{
> + u8 reg;
> + int rc;
> + u8 data[I2C_SMBUS_BLOCK_MAX] = { 0 };
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> +
> + reg = ipsps_regs[attr->index];
> + rc = i2c_smbus_read_block_data(client, reg, data);
> + if (rc < 0)
> + return rc;
> +
> + if (rc != 6)
> + return -EPROTO;
> +
> + return snprintf(buf, PAGE_SIZE, "%u.%02u%u-%u.%02u\n",
> + data[1], data[2]/* < 100 */, data[3]/*< 10*/,
> + data[4], data[5]/* < 100 */);
> +}
> +
> +static ssize_t ipsps_mode_show(struct device *dev,
> + struct device_attribute *devattr, char *buf)
> +{
> + u8 reg;
> + int rc;
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> +
> + reg = ipsps_regs[attr->index];
> + rc = i2c_smbus_read_byte_data(client, reg);
> + if (rc < 0)
> + return rc;
> +
> + switch (rc) {
> + case MODE_ACTIVE:
> + return snprintf(buf, PAGE_SIZE, "[%s] %s %s\n",
> + MODE_ACTIVE_STRING,
> + MODE_STANDBY_STRING, MODE_REDUNDANCY_STRING);
> + case MODE_STANDBY:
> + return snprintf(buf, PAGE_SIZE, "%s [%s] %s\n",
> + MODE_ACTIVE_STRING,
> + MODE_STANDBY_STRING, MODE_REDUNDANCY_STRING);
> + case MODE_REDUNDANCY:
> + return snprintf(buf, PAGE_SIZE, "%s %s [%s]\n",
> + MODE_ACTIVE_STRING,
> + MODE_STANDBY_STRING, MODE_REDUNDANCY_STRING);
> + default:
> + return snprintf(buf, PAGE_SIZE, "unspecified\n");
> + }
> +}
> +
> +static ssize_t ipsps_mode_store(struct device *dev,
> + struct device_attribute *devattr,
> + const char *buf, size_t count)
> +{
> + u8 reg;
> + int rc;
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> +
> + reg = ipsps_regs[attr->index];
> + if (sysfs_streq(MODE_STANDBY_STRING, buf)) {
> + rc = i2c_smbus_write_byte_data(client, reg,
> + MODE_STANDBY);
> + if (rc < 0)
> + return rc;
> + return count;
> + } else if (sysfs_streq(MODE_ACTIVE_STRING, buf)) {
> + rc = i2c_smbus_write_byte_data(client, reg,
> + MODE_ACTIVE);
> + if (rc < 0)
> + return rc;
> + return count;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static SENSOR_DEVICE_ATTR_RO(vendor, ipsps_string, vendor);
> +static SENSOR_DEVICE_ATTR_RO(model, ipsps_string, model);
> +static SENSOR_DEVICE_ATTR_RO(part_number, ipsps_string, part_number);
> +static SENSOR_DEVICE_ATTR_RO(serial_number, ipsps_string, serial_number);
> +static SENSOR_DEVICE_ATTR_RO(hw_version, ipsps_string, hw_version);
> +static SENSOR_DEVICE_ATTR_RO(fw_version, ipsps_fw_version, fw_version);
> +static SENSOR_DEVICE_ATTR_RW(mode, ipsps_mode, mode);
> +
> +static struct attribute *ipsps_attrs[] = {
> + &sensor_dev_attr_vendor.dev_attr.attr,
> + &sensor_dev_attr_model.dev_attr.attr,
> + &sensor_dev_attr_part_number.dev_attr.attr,
> + &sensor_dev_attr_serial_number.dev_attr.attr,
> + &sensor_dev_attr_hw_version.dev_attr.attr,
> + &sensor_dev_attr_fw_version.dev_attr.attr,
> + &sensor_dev_attr_mode.dev_attr.attr,
> + NULL,
> +};
> +
> +ATTRIBUTE_GROUPS(ipsps);
> +
> +static struct pmbus_driver_info ipsps_info = {
> + .pages = 1,
> + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT |
> + PMBUS_HAVE_IIN | PMBUS_HAVE_POUT | PMBUS_HAVE_PIN |
> + PMBUS_HAVE_FAN12 | PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 |
> + PMBUS_HAVE_TEMP3 | PMBUS_HAVE_STATUS_VOUT |
> + PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT |
> + PMBUS_HAVE_STATUS_TEMP | PMBUS_HAVE_STATUS_FAN12,
> + .groups = ipsps_groups,
> +};
> +
> +static struct pmbus_platform_data ipsps_pdata = {
> + .flags = PMBUS_SKIP_STATUS_CHECK,
> +};
> +
> +static int ipsps_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + client->dev.platform_data = &ipsps_pdata;
> + return pmbus_do_probe(client, id, &ipsps_info);
> +}
> +
> +static const struct i2c_device_id ipsps_id[] = {
> + { "inspur_ipsps1", 0 },
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, ipsps_id);
> +
> +static const struct of_device_id ipsps_of_match[] = {
> + { .compatible = "inspur,ipsps1" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, ipsps_of_match);
> +
> +static struct i2c_driver ipsps_driver = {
> + .driver = {
> + .name = "inspur-ipsps",
> + .of_match_table = ipsps_of_match,
> + },
> + .probe = ipsps_probe,
> + .remove = pmbus_do_remove,
> + .id_table = ipsps_id,
> +};
> +
> +module_i2c_driver(ipsps_driver);
> +
> +MODULE_AUTHOR("John Wang");
> +MODULE_DESCRIPTION("PMBus driver for Inspur Power System power supplies");
> +MODULE_LICENSE("GPL");
>
^ permalink raw reply
* [PATCH v5 5/9] fpga: dfl: afu: expose __afu_port_enable/disable function.
From: Wu Hao @ 2019-08-12 2:50 UTC (permalink / raw)
To: gregkh, mdf, linux-fpga
Cc: linux-kernel, linux-api, linux-doc, atull, Wu Hao, Xu Yilun
In-Reply-To: <1565578204-13969-1-git-send-email-hao.wu@intel.com>
As these two functions are used by other private features within the
same driver module but different driver files. e.g. in error reporting
private feature, it requires to clear errors when port is in reset.
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
Acked-by: Moritz Fischer <mdf@kernel.org>
Acked-by: Alan Tull <atull@kernel.org>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
v2: rebased
v5: add afu prefix to __port_enable/disable function to keep
alignment with other func exposed across different afu files.
---
drivers/fpga/dfl-afu-main.c | 26 +++++++++++++++-----------
drivers/fpga/dfl-afu.h | 4 ++++
2 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index f0b45f2..449185c 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -22,14 +22,17 @@
#include "dfl-afu.h"
/**
- * port_enable - enable a port
+ * __afu_port_enable - enable a port by clear reset
* @pdev: port platform device.
*
* Enable Port by clear the port soft reset bit, which is set by default.
* The AFU is unable to respond to any MMIO access while in reset.
- * port_enable function should only be used after port_disable function.
+ * __afu_port_enable function should only be used after __afu_port_disable
+ * function.
+ *
+ * The caller needs to hold lock for protection.
*/
-static void port_enable(struct platform_device *pdev)
+void __afu_port_enable(struct platform_device *pdev)
{
struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
void __iomem *base;
@@ -52,13 +55,14 @@ static void port_enable(struct platform_device *pdev)
#define RST_POLL_TIMEOUT 1000 /* us */
/**
- * port_disable - disable a port
+ * __afu_port_disable - disable a port by hold reset
* @pdev: port platform device.
*
- * Disable Port by setting the port soft reset bit, it puts the port into
- * reset.
+ * Disable Port by setting the port soft reset bit, it puts the port into reset.
+ *
+ * The caller needs to hold lock for protection.
*/
-static int port_disable(struct platform_device *pdev)
+int __afu_port_disable(struct platform_device *pdev)
{
struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
void __iomem *base;
@@ -104,9 +108,9 @@ static int __port_reset(struct platform_device *pdev)
{
int ret;
- ret = port_disable(pdev);
+ ret = __afu_port_disable(pdev);
if (!ret)
- port_enable(pdev);
+ __afu_port_enable(pdev);
return ret;
}
@@ -799,9 +803,9 @@ static int port_enable_set(struct platform_device *pdev, bool enable)
mutex_lock(&pdata->lock);
if (enable)
- port_enable(pdev);
+ __afu_port_enable(pdev);
else
- ret = port_disable(pdev);
+ ret = __afu_port_disable(pdev);
mutex_unlock(&pdata->lock);
return ret;
diff --git a/drivers/fpga/dfl-afu.h b/drivers/fpga/dfl-afu.h
index 0c7630a..83683f2 100644
--- a/drivers/fpga/dfl-afu.h
+++ b/drivers/fpga/dfl-afu.h
@@ -79,6 +79,10 @@ struct dfl_afu {
struct dfl_feature_platform_data *pdata;
};
+/* hold pdata->lock when call __afu_port_enable/disable */
+void __afu_port_enable(struct platform_device *pdev);
+int __afu_port_disable(struct platform_device *pdev);
+
void afu_mmio_region_init(struct dfl_feature_platform_data *pdata);
int afu_mmio_region_add(struct dfl_feature_platform_data *pdata,
u32 region_index, u64 region_size, u64 phys, u32 flags);
--
1.8.3.1
^ permalink raw reply related
* [PATCH v5 4/9] fpga: dfl: afu: add userclock sysfs interfaces.
From: Wu Hao @ 2019-08-12 2:49 UTC (permalink / raw)
To: gregkh, mdf, linux-fpga
Cc: linux-kernel, linux-api, linux-doc, atull, Wu Hao, Ananda Ravuri,
Russ Weight, Xu Yilun
In-Reply-To: <1565578204-13969-1-git-send-email-hao.wu@intel.com>
This patch introduces userclock sysfs interfaces for AFU, user
could use these interfaces for clock setting to AFU.
Please note that, this is only working for port header feature
with revision 0, for later revisions, userclock setting is moved
to a separated private feature, so one revision sysfs interface
is exposed to userspace application for this purpose too.
Signed-off-by: Ananda Ravuri <ananda.ravuri@intel.com>
Signed-off-by: Russ Weight <russell.h.weight@intel.com>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
Acked-by: Alan Tull <atull@kernel.org>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
v2: rebased, and switched to use device_add/remove_groups for sysfs
v3: update kernel version and date in sysfs doc
v4: rebased.
v5: drop sysfs add/remove in init/uinit callback.
add missed locking for sysfs.
use is_visible to decide if hardware supports userclk or not.
remove revision sysfs entry.
---
Documentation/ABI/testing/sysfs-platform-dfl-port | 28 ++++++
drivers/fpga/dfl-afu-main.c | 111 +++++++++++++++++++++-
drivers/fpga/dfl.h | 9 ++
3 files changed, 147 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/sysfs-platform-dfl-port b/Documentation/ABI/testing/sysfs-platform-dfl-port
index 1ab3e6f..c2660e4 100644
--- a/Documentation/ABI/testing/sysfs-platform-dfl-port
+++ b/Documentation/ABI/testing/sysfs-platform-dfl-port
@@ -46,3 +46,31 @@ Contact: Wu Hao <hao.wu@intel.com>
Description: Read-write. Read or set AFU latency tolerance reporting value.
Set ltr to 1 if the AFU can tolerate latency >= 40us or set it
to 0 if it is latency sensitive.
+
+What: /sys/bus/platform/devices/dfl-port.0/userclk_freqcmd
+Date: August 2019
+KernelVersion: 5.4
+Contact: Wu Hao <hao.wu@intel.com>
+Description: Write-only. User writes command to this interface to set
+ userclock to AFU.
+
+What: /sys/bus/platform/devices/dfl-port.0/userclk_freqsts
+Date: August 2019
+KernelVersion: 5.4
+Contact: Wu Hao <hao.wu@intel.com>
+Description: Read-only. Read this file to get the status of issued command
+ to userclck_freqcmd.
+
+What: /sys/bus/platform/devices/dfl-port.0/userclk_freqcntrcmd
+Date: August 2019
+KernelVersion: 5.4
+Contact: Wu Hao <hao.wu@intel.com>
+Description: Write-only. User writes command to this interface to set
+ userclock counter.
+
+What: /sys/bus/platform/devices/dfl-port.0/userclk_freqcntrsts
+Date: August 2019
+KernelVersion: 5.4
+Contact: Wu Hao <hao.wu@intel.com>
+Description: Read-only. Read this file to get the status of issued command
+ to userclck_freqcntrcmd.
diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index e955149..f0b45f2 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -274,17 +274,126 @@ static int port_get_id(struct platform_device *pdev)
}
static DEVICE_ATTR_RO(power_state);
+static ssize_t
+userclk_freqcmd_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+ u64 userclk_freq_cmd;
+ void __iomem *base;
+
+ if (kstrtou64(buf, 0, &userclk_freq_cmd))
+ return -EINVAL;
+
+ base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
+
+ mutex_lock(&pdata->lock);
+ writeq(userclk_freq_cmd, base + PORT_HDR_USRCLK_CMD0);
+ mutex_unlock(&pdata->lock);
+
+ return count;
+}
+static DEVICE_ATTR_WO(userclk_freqcmd);
+
+static ssize_t
+userclk_freqcntrcmd_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+ u64 userclk_freqcntr_cmd;
+ void __iomem *base;
+
+ if (kstrtou64(buf, 0, &userclk_freqcntr_cmd))
+ return -EINVAL;
+
+ base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
+
+ mutex_lock(&pdata->lock);
+ writeq(userclk_freqcntr_cmd, base + PORT_HDR_USRCLK_CMD1);
+ mutex_unlock(&pdata->lock);
+
+ return count;
+}
+static DEVICE_ATTR_WO(userclk_freqcntrcmd);
+
+static ssize_t
+userclk_freqsts_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+ u64 userclk_freqsts;
+ void __iomem *base;
+
+ base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
+
+ mutex_lock(&pdata->lock);
+ userclk_freqsts = readq(base + PORT_HDR_USRCLK_STS0);
+ mutex_unlock(&pdata->lock);
+
+ return sprintf(buf, "0x%llx\n", (unsigned long long)userclk_freqsts);
+}
+static DEVICE_ATTR_RO(userclk_freqsts);
+
+static ssize_t
+userclk_freqcntrsts_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+ u64 userclk_freqcntrsts;
+ void __iomem *base;
+
+ base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
+
+ mutex_lock(&pdata->lock);
+ userclk_freqcntrsts = readq(base + PORT_HDR_USRCLK_STS1);
+ mutex_unlock(&pdata->lock);
+
+ return sprintf(buf, "0x%llx\n",
+ (unsigned long long)userclk_freqcntrsts);
+}
+static DEVICE_ATTR_RO(userclk_freqcntrsts);
+
static struct attribute *port_hdr_attrs[] = {
&dev_attr_id.attr,
&dev_attr_ltr.attr,
&dev_attr_ap1_event.attr,
&dev_attr_ap2_event.attr,
&dev_attr_power_state.attr,
+ &dev_attr_userclk_freqcmd.attr,
+ &dev_attr_userclk_freqcntrcmd.attr,
+ &dev_attr_userclk_freqsts.attr,
+ &dev_attr_userclk_freqcntrsts.attr,
NULL,
};
+static umode_t port_hdr_attrs_visible(struct kobject *kobj,
+ struct attribute *attr, int n)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ umode_t mode = attr->mode;
+ void __iomem *base;
+
+ base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
+
+ if (dfl_feature_revision(base) > 0) {
+ /*
+ * userclk sysfs interfaces are only visible in case port
+ * revision is 0, as hardware with revision >0 doesn't
+ * support this.
+ */
+ if (attr == &dev_attr_userclk_freqcmd.attr ||
+ attr == &dev_attr_userclk_freqcntrcmd.attr ||
+ attr == &dev_attr_userclk_freqsts.attr ||
+ attr == &dev_attr_userclk_freqcntrsts.attr)
+ mode = 0;
+ }
+
+ return mode;
+}
+
static const struct attribute_group port_hdr_group = {
- .attrs = port_hdr_attrs,
+ .attrs = port_hdr_attrs,
+ .is_visible = port_hdr_attrs_visible,
};
static int port_hdr_init(struct platform_device *pdev,
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index 856ea4e..9f0e656 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -120,6 +120,10 @@
#define PORT_HDR_CAP 0x30
#define PORT_HDR_CTRL 0x38
#define PORT_HDR_STS 0x40
+#define PORT_HDR_USRCLK_CMD0 0x50
+#define PORT_HDR_USRCLK_CMD1 0x58
+#define PORT_HDR_USRCLK_STS0 0x60
+#define PORT_HDR_USRCLK_STS1 0x68
/* Port Capability Register Bitfield */
#define PORT_CAP_PORT_NUM GENMASK_ULL(1, 0) /* ID of this port */
@@ -355,6 +359,11 @@ static inline bool dfl_feature_is_port(void __iomem *base)
(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_PORT);
}
+static inline u8 dfl_feature_revision(void __iomem *base)
+{
+ return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
+}
+
/**
* struct dfl_fpga_enum_info - DFL FPGA enumeration information
*
--
1.8.3.1
^ permalink raw reply related
* [PATCH v5 2/9] fpga: dfl: fme: convert platform_driver to use dev_groups
From: Wu Hao @ 2019-08-12 2:49 UTC (permalink / raw)
To: gregkh, mdf, linux-fpga; +Cc: linux-kernel, linux-api, linux-doc, atull, Wu Hao
In-Reply-To: <1565578204-13969-1-git-send-email-hao.wu@intel.com>
This patch takes advantage of driver core which helps to create
and remove sysfs attribute files, so there is no need to register
sysfs entries manually in dfl-fme platform river code.
Signed-off-by: Wu Hao <hao.wu@intel.com>
---
drivers/fpga/dfl-fme-main.c | 29 ++---------------------------
1 file changed, 2 insertions(+), 27 deletions(-)
diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c
index f033f1c..bf8114d 100644
--- a/drivers/fpga/dfl-fme-main.c
+++ b/drivers/fpga/dfl-fme-main.c
@@ -129,30 +129,6 @@ static ssize_t socket_id_show(struct device *dev,
};
ATTRIBUTE_GROUPS(fme_hdr);
-static int fme_hdr_init(struct platform_device *pdev,
- struct dfl_feature *feature)
-{
- void __iomem *base = feature->ioaddr;
- int ret;
-
- dev_dbg(&pdev->dev, "FME HDR Init.\n");
- dev_dbg(&pdev->dev, "FME cap %llx.\n",
- (unsigned long long)readq(base + FME_HDR_CAP));
-
- ret = device_add_groups(&pdev->dev, fme_hdr_groups);
- if (ret)
- return ret;
-
- return 0;
-}
-
-static void fme_hdr_uinit(struct platform_device *pdev,
- struct dfl_feature *feature)
-{
- dev_dbg(&pdev->dev, "FME HDR UInit.\n");
- device_remove_groups(&pdev->dev, fme_hdr_groups);
-}
-
static long fme_hdr_ioctl_release_port(struct dfl_feature_platform_data *pdata,
unsigned long arg)
{
@@ -199,8 +175,6 @@ static long fme_hdr_ioctl(struct platform_device *pdev,
};
static const struct dfl_feature_ops fme_hdr_ops = {
- .init = fme_hdr_init,
- .uinit = fme_hdr_uinit,
.ioctl = fme_hdr_ioctl,
};
@@ -361,7 +335,8 @@ static int fme_remove(struct platform_device *pdev)
static struct platform_driver fme_driver = {
.driver = {
- .name = DFL_FPGA_FEATURE_DEV_FME,
+ .name = DFL_FPGA_FEATURE_DEV_FME,
+ .dev_groups = fme_hdr_groups,
},
.probe = fme_probe,
.remove = fme_remove,
--
1.8.3.1
^ permalink raw reply related
* [PATCH v5 3/9] fpga: dfl: afu: convert platform_driver to use dev_groups
From: Wu Hao @ 2019-08-12 2:49 UTC (permalink / raw)
To: gregkh, mdf, linux-fpga; +Cc: linux-kernel, linux-api, linux-doc, atull, Wu Hao
In-Reply-To: <1565578204-13969-1-git-send-email-hao.wu@intel.com>
This patch takes advantage of driver core which helps to create
and remove sysfs attribute files, so there is no need to register
sysfs entries manually in dfl-afu platform river code.
Signed-off-by: Wu Hao <hao.wu@intel.com>
---
drivers/fpga/dfl-afu-main.c | 69 +++++++++++++++++++++++----------------------
1 file changed, 36 insertions(+), 33 deletions(-)
diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index e50c45e..e955149 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -282,24 +282,17 @@ static int port_get_id(struct platform_device *pdev)
&dev_attr_power_state.attr,
NULL,
};
-ATTRIBUTE_GROUPS(port_hdr);
+
+static const struct attribute_group port_hdr_group = {
+ .attrs = port_hdr_attrs,
+};
static int port_hdr_init(struct platform_device *pdev,
struct dfl_feature *feature)
{
- dev_dbg(&pdev->dev, "PORT HDR Init.\n");
-
port_reset(pdev);
- return device_add_groups(&pdev->dev, port_hdr_groups);
-}
-
-static void port_hdr_uinit(struct platform_device *pdev,
- struct dfl_feature *feature)
-{
- dev_dbg(&pdev->dev, "PORT HDR UInit.\n");
-
- device_remove_groups(&pdev->dev, port_hdr_groups);
+ return 0;
}
static long
@@ -330,7 +323,6 @@ static void port_hdr_uinit(struct platform_device *pdev,
static const struct dfl_feature_ops port_hdr_ops = {
.init = port_hdr_init,
- .uinit = port_hdr_uinit,
.ioctl = port_hdr_ioctl,
};
@@ -361,32 +353,37 @@ static void port_hdr_uinit(struct platform_device *pdev,
&dev_attr_afu_id.attr,
NULL
};
-ATTRIBUTE_GROUPS(port_afu);
-static int port_afu_init(struct platform_device *pdev,
- struct dfl_feature *feature)
+static umode_t port_afu_attrs_visible(struct kobject *kobj,
+ struct attribute *attr, int n)
{
- struct resource *res = &pdev->resource[feature->resource_index];
- int ret;
-
- dev_dbg(&pdev->dev, "PORT AFU Init.\n");
+ struct device *dev = kobj_to_dev(kobj);
- ret = afu_mmio_region_add(dev_get_platdata(&pdev->dev),
- DFL_PORT_REGION_INDEX_AFU, resource_size(res),
- res->start, DFL_PORT_REGION_READ |
- DFL_PORT_REGION_WRITE | DFL_PORT_REGION_MMAP);
- if (ret)
- return ret;
+ /*
+ * sysfs entries are visible only if related private feature is
+ * enumerated.
+ */
+ if (!dfl_get_feature_by_id(dev, PORT_FEATURE_ID_AFU))
+ return 0;
- return device_add_groups(&pdev->dev, port_afu_groups);
+ return attr->mode;
}
-static void port_afu_uinit(struct platform_device *pdev,
- struct dfl_feature *feature)
+static const struct attribute_group port_afu_group = {
+ .attrs = port_afu_attrs,
+ .is_visible = port_afu_attrs_visible,
+};
+
+static int port_afu_init(struct platform_device *pdev,
+ struct dfl_feature *feature)
{
- dev_dbg(&pdev->dev, "PORT AFU UInit.\n");
+ struct resource *res = &pdev->resource[feature->resource_index];
- device_remove_groups(&pdev->dev, port_afu_groups);
+ return afu_mmio_region_add(dev_get_platdata(&pdev->dev),
+ DFL_PORT_REGION_INDEX_AFU,
+ resource_size(res), res->start,
+ DFL_PORT_REGION_MMAP | DFL_PORT_REGION_READ |
+ DFL_PORT_REGION_WRITE);
}
static const struct dfl_feature_id port_afu_id_table[] = {
@@ -396,7 +393,6 @@ static void port_afu_uinit(struct platform_device *pdev,
static const struct dfl_feature_ops port_afu_ops = {
.init = port_afu_init,
- .uinit = port_afu_uinit,
};
static struct dfl_feature_driver port_feature_drvs[] = {
@@ -748,9 +744,16 @@ static int afu_remove(struct platform_device *pdev)
return 0;
}
+static const struct attribute_group *afu_dev_groups[] = {
+ &port_hdr_group,
+ &port_afu_group,
+ NULL
+};
+
static struct platform_driver afu_driver = {
.driver = {
- .name = DFL_FPGA_FEATURE_DEV_PORT,
+ .name = DFL_FPGA_FEATURE_DEV_PORT,
+ .dev_groups = afu_dev_groups,
},
.probe = afu_probe,
.remove = afu_remove,
--
1.8.3.1
^ permalink raw reply related
* [PATCH v5 1/9] fpga: dfl: make init callback optional
From: Wu Hao @ 2019-08-12 2:49 UTC (permalink / raw)
To: gregkh, mdf, linux-fpga; +Cc: linux-kernel, linux-api, linux-doc, atull, Wu Hao
In-Reply-To: <1565578204-13969-1-git-send-email-hao.wu@intel.com>
This patch makes init callback of sub features optional. With
this change, people don't need to prepare any empty init callback.
Signed-off-by: Wu Hao <hao.wu@intel.com>
---
drivers/fpga/dfl.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index c0512af..96a2b82 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -271,11 +271,13 @@ static int dfl_feature_instance_init(struct platform_device *pdev,
struct dfl_feature *feature,
struct dfl_feature_driver *drv)
{
- int ret;
+ int ret = 0;
- ret = drv->ops->init(pdev, feature);
- if (ret)
- return ret;
+ if (drv->ops->init) {
+ ret = drv->ops->init(pdev, feature);
+ if (ret)
+ return ret;
+ }
feature->ops = drv->ops;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v5 0/9] FPGA DFL updates
From: Wu Hao @ 2019-08-12 2:49 UTC (permalink / raw)
To: gregkh, mdf, linux-fpga; +Cc: linux-kernel, linux-api, linux-doc, atull, Wu Hao
Hi Greg,
This is v5 patchset which adds more features to FPGA DFL. Marjor changes
against v4 are sysfs related code rework to address comments on v4.
Please help to take a look. Thanks!
Main changes from v4:
- convert code to use dev_groups for sysfs entries (#2, #3, #4, #6, #8).
- clean up for empty init function after remove sysfs add/remove (#1).
- introduce is_visible for sysfs groups (#3, #4, #6, #8).
- remove revision sysfs entries (#4, #6, #8).
- improve naming on shared functions (#5).
- reorganize sysfs entries for port and fme error reporting (#6, #8).
Main changes from v3:
- drop avx512 partail reconfiguration patch for now.
- split dfl_fpga_cdev_config_port to 2 functions *_release/assign_port
(#1).
- split __dfl_fpga_cdev_config_port_vf into 2 functions with locking
added (#2).
- improve description in sysfs doc to avoid misunderstanding (#3).
- switch to boolean in sysfs entry store function (#3).
- remove dev_dbg in init/uinit callback function (#7, #9, #11).
- remove uinit callback which does does nothing (#8, #9)
Main changes from v2:
- update kernel version/date in sysfs doc (patch #4, #5, #8, #10, #11).
- add back Documentation patch (patch #12).
Main changes from v1:
- remove DRV/MODULE_VERSION modifications. (patch #1, #3, #4, #6)
- remove argsz from new ioctls. (patch #2)
- replace sysfs_create/remove_* with device_add/remove_* for sysfs entries.
(patch #5, #8, #11)
Wu Hao (9):
fpga: dfl: make init callback optional
fpga: dfl: fme: convert platform_driver to use dev_groups
fpga: dfl: afu: convert platform_driver to use dev_groups
fpga: dfl: afu: add userclock sysfs interfaces.
fpga: dfl: afu: expose __afu_port_enable/disable function.
fpga: dfl: afu: add error reporting support.
fpga: dfl: afu: add STP (SignalTap) support
fpga: dfl: fme: add global error reporting support
Documentation: fpga: dfl: add descriptions for virtualization and new
interfaces.
Documentation/ABI/testing/sysfs-platform-dfl-fme | 62 ++++
Documentation/ABI/testing/sysfs-platform-dfl-port | 53 ++++
Documentation/fpga/dfl.rst | 105 +++++++
drivers/fpga/Makefile | 3 +-
drivers/fpga/dfl-afu-error.c | 230 ++++++++++++++
drivers/fpga/dfl-afu-main.c | 230 +++++++++++---
drivers/fpga/dfl-afu.h | 9 +
drivers/fpga/dfl-fme-error.c | 359 ++++++++++++++++++++++
drivers/fpga/dfl-fme-main.c | 42 +--
drivers/fpga/dfl-fme.h | 3 +
drivers/fpga/dfl.c | 10 +-
drivers/fpga/dfl.h | 9 +
12 files changed, 1041 insertions(+), 74 deletions(-)
create mode 100644 drivers/fpga/dfl-afu-error.c
create mode 100644 drivers/fpga/dfl-fme-error.c
--
1.8.3.1
^ permalink raw reply
* [PATCH v5 9/9] Documentation: fpga: dfl: add descriptions for virtualization and new interfaces.
From: Wu Hao @ 2019-08-12 2:50 UTC (permalink / raw)
To: gregkh, mdf, linux-fpga
Cc: linux-kernel, linux-api, linux-doc, atull, Wu Hao, Xu Yilun
In-Reply-To: <1565578204-13969-1-git-send-email-hao.wu@intel.com>
This patch adds virtualization support description for DFL based
FPGA devices (based on PCIe SRIOV), and introductions to new
interfaces added by new dfl private feature drivers.
[mdf@kernel.org: Fixed up to make it work with new reStructuredText docs]
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
Acked-by: Alan Tull <atull@kernel.org>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
Documentation/fpga/dfl.rst | 105 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 105 insertions(+)
diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
index 2f125ab..6fa483f 100644
--- a/Documentation/fpga/dfl.rst
+++ b/Documentation/fpga/dfl.rst
@@ -87,6 +87,8 @@ The following functions are exposed through ioctls:
- Get driver API version (DFL_FPGA_GET_API_VERSION)
- Check for extensions (DFL_FPGA_CHECK_EXTENSION)
- Program bitstream (DFL_FPGA_FME_PORT_PR)
+- Assign port to PF (DFL_FPGA_FME_PORT_ASSIGN)
+- Release port from PF (DFL_FPGA_FME_PORT_RELEASE)
More functions are exposed through sysfs
(/sys/class/fpga_region/regionX/dfl-fme.n/):
@@ -102,6 +104,10 @@ More functions are exposed through sysfs
one FPGA device may have more than one port, this sysfs interface indicates
how many ports the FPGA device has.
+ Global error reporting management (errors/)
+ error reporting sysfs interfaces allow user to read errors detected by the
+ hardware, and clear the logged errors.
+
FIU - PORT
==========
@@ -143,6 +149,10 @@ More functions are exposed through sysfs:
Read Accelerator GUID (afu_id)
afu_id indicates which PR bitstream is programmed to this AFU.
+ Error reporting (errors/)
+ error reporting sysfs interfaces allow user to read port/afu errors
+ detected by the hardware, and clear the logged errors.
+
DFL Framework Overview
======================
@@ -218,6 +228,101 @@ the compat_id exposed by the target FPGA region. This check is usually done by
userspace before calling the reconfiguration IOCTL.
+FPGA virtualization - PCIe SRIOV
+================================
+This section describes the virtualization support on DFL based FPGA device to
+enable accessing an accelerator from applications running in a virtual machine
+(VM). This section only describes the PCIe based FPGA device with SRIOV support.
+
+Features supported by the particular FPGA device are exposed through Device
+Feature Lists, as illustrated below:
+
+::
+
+ +-------------------------------+ +-------------+
+ | PF | | VF |
+ +-------------------------------+ +-------------+
+ ^ ^ ^ ^
+ | | | |
+ +-----|------------|---------|--------------|-------+
+ | | | | | |
+ | +-----+ +-------+ +-------+ +-------+ |
+ | | FME | | Port0 | | Port1 | | Port2 | |
+ | +-----+ +-------+ +-------+ +-------+ |
+ | ^ ^ ^ |
+ | | | | |
+ | +-------+ +------+ +-------+ |
+ | | AFU | | AFU | | AFU | |
+ | +-------+ +------+ +-------+ |
+ | |
+ | DFL based FPGA PCIe Device |
+ +---------------------------------------------------+
+
+FME is always accessed through the physical function (PF).
+
+Ports (and related AFUs) are accessed via PF by default, but could be exposed
+through virtual function (VF) devices via PCIe SRIOV. Each VF only contains
+1 Port and 1 AFU for isolation. Users could assign individual VFs (accelerators)
+created via PCIe SRIOV interface, to virtual machines.
+
+The driver organization in virtualization case is illustrated below:
+::
+
+ +-------++------++------+ |
+ | FME || FME || FME | |
+ | FPGA || FPGA || FPGA | |
+ |Manager||Bridge||Region| |
+ +-------++------++------+ |
+ +-----------------------+ +--------+ | +--------+
+ | FME | | AFU | | | AFU |
+ | Module | | Module | | | Module |
+ +-----------------------+ +--------+ | +--------+
+ +-----------------------+ | +-----------------------+
+ | FPGA Container Device | | | FPGA Container Device |
+ | (FPGA Base Region) | | | (FPGA Base Region) |
+ +-----------------------+ | +-----------------------+
+ +------------------+ | +------------------+
+ | FPGA PCIE Module | | Virtual | FPGA PCIE Module |
+ +------------------+ Host | Machine +------------------+
+ -------------------------------------- | ------------------------------
+ +---------------+ | +---------------+
+ | PCI PF Device | | | PCI VF Device |
+ +---------------+ | +---------------+
+
+FPGA PCIe device driver is always loaded first once a FPGA PCIe PF or VF device
+is detected. It:
+
+* Finishes enumeration on both FPGA PCIe PF and VF device using common
+ interfaces from DFL framework.
+* Supports SRIOV.
+
+The FME device driver plays a management role in this driver architecture, it
+provides ioctls to release Port from PF and assign Port to PF. After release
+a port from PF, then it's safe to expose this port through a VF via PCIe SRIOV
+sysfs interface.
+
+To enable accessing an accelerator from applications running in a VM, the
+respective AFU's port needs to be assigned to a VF using the following steps:
+
+#. The PF owns all AFU ports by default. Any port that needs to be
+ reassigned to a VF must first be released through the
+ DFL_FPGA_FME_PORT_RELEASE ioctl on the FME device.
+
+#. Once N ports are released from PF, then user can use command below
+ to enable SRIOV and VFs. Each VF owns only one Port with AFU.
+
+ ::
+
+ echo N > $PCI_DEVICE_PATH/sriov_numvfs
+
+#. Pass through the VFs to VMs
+
+#. The AFU under VF is accessible from applications in VM (using the
+ same driver inside the VF).
+
+Note that an FME can't be assigned to a VF, thus PR and other management
+functions are only available via the PF.
+
Device enumeration
==================
This section introduces how applications enumerate the fpga device from
--
1.8.3.1
^ permalink raw reply related
* [PATCH v5 8/9] fpga: dfl: fme: add global error reporting support
From: Wu Hao @ 2019-08-12 2:50 UTC (permalink / raw)
To: gregkh, mdf, linux-fpga
Cc: linux-kernel, linux-api, linux-doc, atull, Wu Hao, Luwei Kang,
Ananda Ravuri, Xu Yilun
In-Reply-To: <1565578204-13969-1-git-send-email-hao.wu@intel.com>
This patch adds support for global error reporting for FPGA
Management Engine (FME), it introduces sysfs interfaces to
report different error detected by the hardware, and allow
user to clear errors or inject error for testing purpose.
Signed-off-by: Luwei Kang <luwei.kang@intel.com>
Signed-off-by: Ananda Ravuri <ananda.ravuri@intel.com>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
Acked-by: Alan Tull <atull@kernel.org>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
v2: switch to device_add/remove_groups for sysfs.
v3: update kernel version and date in sysfs doc
v4: rebase, remove dev_dbg in init/uinit callback.
v5: reorganize sysfs entries:
remove "fme-errors" sub folder and related sysfs entries to
upper level folder.
merge WO "clear" to RO "errors" to keep alignment with error
sysfs entries in upper level folder.
remove revision sysfs entry.
add missed locking in sysfs entries.
expose sysfs group with is_visible via dev_groups.
---
Documentation/ABI/testing/sysfs-platform-dfl-fme | 62 ++++
drivers/fpga/Makefile | 2 +-
drivers/fpga/dfl-fme-error.c | 359 +++++++++++++++++++++++
drivers/fpga/dfl-fme-main.c | 17 +-
drivers/fpga/dfl-fme.h | 3 +
5 files changed, 440 insertions(+), 3 deletions(-)
create mode 100644 drivers/fpga/dfl-fme-error.c
diff --git a/Documentation/ABI/testing/sysfs-platform-dfl-fme b/Documentation/ABI/testing/sysfs-platform-dfl-fme
index 65372aa..72634d3 100644
--- a/Documentation/ABI/testing/sysfs-platform-dfl-fme
+++ b/Documentation/ABI/testing/sysfs-platform-dfl-fme
@@ -44,3 +44,65 @@ Description: Read-only. It returns socket_id to indicate which socket
this FPGA belongs to, only valid for integrated solution.
User only needs this information, in case standard numa node
can't provide correct information.
+
+What: /sys/bus/platform/devices/dfl-fme.0/errors/pcie0_errors
+Date: August 2019
+KernelVersion: 5.4
+Contact: Wu Hao <hao.wu@intel.com>
+Description: Read-Write. Read this file for errors detected on pcie0 link.
+ Write this file to clear errors logged in pcie0_errors. Write
+ fails with -EINVAL if input parsing fails or input error code
+ doesn't match.
+
+What: /sys/bus/platform/devices/dfl-fme.0/errors/pcie1_errors
+Date: August 2019
+KernelVersion: 5.4
+Contact: Wu Hao <hao.wu@intel.com>
+Description: Read-Write. Read this file for errors detected on pcie1 link.
+ Write this file to clear errors logged in pcie1_errors. Write
+ fails with -EINVAL if input parsing fails or input error code
+ doesn't match.
+
+What: /sys/bus/platform/devices/dfl-fme.0/errors/nonfatal_errors
+Date: August 2019
+KernelVersion: 5.4
+Contact: Wu Hao <hao.wu@intel.com>
+Description: Read-only. It returns non-fatal errors detected.
+
+What: /sys/bus/platform/devices/dfl-fme.0/errors/catfatal_errors
+Date: August 2019
+KernelVersion: 5.4
+Contact: Wu Hao <hao.wu@intel.com>
+Description: Read-only. It returns catastrophic and fatal errors detected.
+
+What: /sys/bus/platform/devices/dfl-fme.0/errors/inject_errors
+Date: August 2019
+KernelVersion: 5.4
+Contact: Wu Hao <hao.wu@intel.com>
+Description: Read-Write. Read this file to check errors injected. Write this
+ file to inject errors for testing purpose. Write fails with
+ -EINVAL if input parsing fails or input inject error code isn't
+ supported.
+
+What: /sys/bus/platform/devices/dfl-fme.0/errors/fme_errors
+Date: August 2019
+KernelVersion: 5.4
+Contact: Wu Hao <hao.wu@intel.com>
+Description: Read-Write. Read this file to get errors detected on FME.
+ Write this file to clear errors logged in fme_errors. Write
+ fials with -EINVAL if input parsing fails or input error code
+ doesn't match.
+
+What: /sys/bus/platform/devices/dfl-fme.0/errors/first_error
+Date: August 2019
+KernelVersion: 5.4
+Contact: Wu Hao <hao.wu@intel.com>
+Description: Read-only. Read this file to get the first error detected by
+ hardware.
+
+What: /sys/bus/platform/devices/dfl-fme.0/errors/next_error
+Date: August 2019
+KernelVersion: 5.4
+Contact: Wu Hao <hao.wu@intel.com>
+Description: Read-only. Read this file to get the second error detected by
+ hardware.
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 7255891..4865b74 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -39,7 +39,7 @@ obj-$(CONFIG_FPGA_DFL_FME_BRIDGE) += dfl-fme-br.o
obj-$(CONFIG_FPGA_DFL_FME_REGION) += dfl-fme-region.o
obj-$(CONFIG_FPGA_DFL_AFU) += dfl-afu.o
-dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o
+dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o dfl-fme-error.o
dfl-afu-objs := dfl-afu-main.o dfl-afu-region.o dfl-afu-dma-region.o
dfl-afu-objs += dfl-afu-error.o
diff --git a/drivers/fpga/dfl-fme-error.c b/drivers/fpga/dfl-fme-error.c
new file mode 100644
index 0000000..f897d41
--- /dev/null
+++ b/drivers/fpga/dfl-fme-error.c
@@ -0,0 +1,359 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for FPGA Management Engine Error Management
+ *
+ * Copyright 2019 Intel Corporation, Inc.
+ *
+ * Authors:
+ * Kang Luwei <luwei.kang@intel.com>
+ * Xiao Guangrong <guangrong.xiao@linux.intel.com>
+ * Wu Hao <hao.wu@intel.com>
+ * Joseph Grecco <joe.grecco@intel.com>
+ * Enno Luebbers <enno.luebbers@intel.com>
+ * Tim Whisonant <tim.whisonant@intel.com>
+ * Ananda Ravuri <ananda.ravuri@intel.com>
+ * Mitchel, Henry <henry.mitchel@intel.com>
+ */
+
+#include <linux/uaccess.h>
+
+#include "dfl.h"
+#include "dfl-fme.h"
+
+#define FME_ERROR_MASK 0x8
+#define FME_ERROR 0x10
+#define MBP_ERROR BIT_ULL(6)
+#define PCIE0_ERROR_MASK 0x18
+#define PCIE0_ERROR 0x20
+#define PCIE1_ERROR_MASK 0x28
+#define PCIE1_ERROR 0x30
+#define FME_FIRST_ERROR 0x38
+#define FME_NEXT_ERROR 0x40
+#define RAS_NONFAT_ERROR_MASK 0x48
+#define RAS_NONFAT_ERROR 0x50
+#define RAS_CATFAT_ERROR_MASK 0x58
+#define RAS_CATFAT_ERROR 0x60
+#define RAS_ERROR_INJECT 0x68
+#define INJECT_ERROR_MASK GENMASK_ULL(2, 0)
+
+#define ERROR_MASK GENMASK_ULL(63, 0)
+
+static ssize_t pcie0_errors_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+ void __iomem *base;
+ u64 value;
+
+ base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+ mutex_lock(&pdata->lock);
+ value = readq(base + PCIE0_ERROR);
+ mutex_unlock(&pdata->lock);
+
+ return sprintf(buf, "0x%llx\n", (unsigned long long)value);
+}
+
+static ssize_t pcie0_errors_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+ void __iomem *base;
+ int ret = 0;
+ u64 v, val;
+
+ if (kstrtou64(buf, 0, &val))
+ return -EINVAL;
+
+ base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+ mutex_lock(&pdata->lock);
+ writeq(GENMASK_ULL(63, 0), base + PCIE0_ERROR_MASK);
+
+ v = readq(base + PCIE0_ERROR);
+ if (val == v)
+ writeq(v, base + PCIE0_ERROR);
+ else
+ ret = -EINVAL;
+
+ writeq(0ULL, base + PCIE0_ERROR_MASK);
+ mutex_unlock(&pdata->lock);
+ return ret ? ret : count;
+}
+static DEVICE_ATTR_RW(pcie0_errors);
+
+static ssize_t pcie1_errors_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+ void __iomem *base;
+ u64 value;
+
+ base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+ mutex_lock(&pdata->lock);
+ value = readq(base + PCIE1_ERROR);
+ mutex_unlock(&pdata->lock);
+
+ return sprintf(buf, "0x%llx\n", (unsigned long long)value);
+}
+
+static ssize_t pcie1_errors_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+ void __iomem *base;
+ int ret = 0;
+ u64 v, val;
+
+ if (kstrtou64(buf, 0, &val))
+ return -EINVAL;
+
+ base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+ mutex_lock(&pdata->lock);
+ writeq(GENMASK_ULL(63, 0), base + PCIE1_ERROR_MASK);
+
+ v = readq(base + PCIE1_ERROR);
+ if (val == v)
+ writeq(v, base + PCIE1_ERROR);
+ else
+ ret = -EINVAL;
+
+ writeq(0ULL, base + PCIE1_ERROR_MASK);
+ mutex_unlock(&pdata->lock);
+ return ret ? ret : count;
+}
+static DEVICE_ATTR_RW(pcie1_errors);
+
+static ssize_t nonfatal_errors_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ void __iomem *base;
+
+ base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+ return sprintf(buf, "0x%llx\n",
+ (unsigned long long)readq(base + RAS_NONFAT_ERROR));
+}
+static DEVICE_ATTR_RO(nonfatal_errors);
+
+static ssize_t catfatal_errors_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ void __iomem *base;
+
+ base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+ return sprintf(buf, "0x%llx\n",
+ (unsigned long long)readq(base + RAS_CATFAT_ERROR));
+}
+static DEVICE_ATTR_RO(catfatal_errors);
+
+static ssize_t inject_errors_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+ void __iomem *base;
+ u64 v;
+
+ base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+ mutex_lock(&pdata->lock);
+ v = readq(base + RAS_ERROR_INJECT);
+ mutex_unlock(&pdata->lock);
+
+ return sprintf(buf, "0x%llx\n",
+ (unsigned long long)FIELD_GET(INJECT_ERROR_MASK, v));
+}
+
+static ssize_t inject_errors_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+ void __iomem *base;
+ u8 inject_error;
+ u64 v;
+
+ if (kstrtou8(buf, 0, &inject_error))
+ return -EINVAL;
+
+ if (inject_error & ~INJECT_ERROR_MASK)
+ return -EINVAL;
+
+ base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+ mutex_lock(&pdata->lock);
+ v = readq(base + RAS_ERROR_INJECT);
+ v &= ~INJECT_ERROR_MASK;
+ v |= FIELD_PREP(INJECT_ERROR_MASK, inject_error);
+ writeq(v, base + RAS_ERROR_INJECT);
+ mutex_unlock(&pdata->lock);
+
+ return count;
+}
+static DEVICE_ATTR_RW(inject_errors);
+
+static ssize_t fme_errors_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+ void __iomem *base;
+ u64 value;
+
+ base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+ mutex_lock(&pdata->lock);
+ value = readq(base + FME_ERROR);
+ mutex_unlock(&pdata->lock);
+
+ return sprintf(buf, "0x%llx\n", (unsigned long long)value);
+}
+
+static ssize_t fme_errors_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+ void __iomem *base;
+ u64 v, val;
+ int ret = 0;
+
+ if (kstrtou64(buf, 0, &val))
+ return -EINVAL;
+
+ base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+ mutex_lock(&pdata->lock);
+ writeq(GENMASK_ULL(63, 0), base + FME_ERROR_MASK);
+
+ v = readq(base + FME_ERROR);
+ if (val == v)
+ writeq(v, base + FME_ERROR);
+ else
+ ret = -EINVAL;
+
+ /* Workaround: disable MBP_ERROR if feature revision is 0 */
+ writeq(dfl_feature_revision(base) ? 0ULL : MBP_ERROR,
+ base + FME_ERROR_MASK);
+ mutex_unlock(&pdata->lock);
+ return ret ? ret : count;
+}
+static DEVICE_ATTR_RW(fme_errors);
+
+static ssize_t first_error_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+ void __iomem *base;
+ u64 value;
+
+ base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+ mutex_lock(&pdata->lock);
+ value = readq(base + FME_FIRST_ERROR);
+ mutex_unlock(&pdata->lock);
+
+ return sprintf(buf, "0x%llx\n", (unsigned long long)value);
+}
+static DEVICE_ATTR_RO(first_error);
+
+static ssize_t next_error_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+ void __iomem *base;
+ u64 value;
+
+ base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+ mutex_lock(&pdata->lock);
+ value = readq(base + FME_NEXT_ERROR);
+ mutex_unlock(&pdata->lock);
+
+ return sprintf(buf, "0x%llx\n", (unsigned long long)value);
+}
+static DEVICE_ATTR_RO(next_error);
+
+static struct attribute *fme_global_err_attrs[] = {
+ &dev_attr_pcie0_errors.attr,
+ &dev_attr_pcie1_errors.attr,
+ &dev_attr_nonfatal_errors.attr,
+ &dev_attr_catfatal_errors.attr,
+ &dev_attr_inject_errors.attr,
+ &dev_attr_fme_errors.attr,
+ &dev_attr_first_error.attr,
+ &dev_attr_next_error.attr,
+ NULL,
+};
+
+static umode_t fme_global_err_attrs_visible(struct kobject *kobj,
+ struct attribute *attr, int n)
+{
+ struct device *dev = kobj_to_dev(kobj);
+
+ /*
+ * sysfs entries are visible only if related private feature is
+ * enumerated.
+ */
+ if (!dfl_get_feature_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR))
+ return 0;
+
+ return attr->mode;
+}
+
+const struct attribute_group fme_global_err_group = {
+ .name = "errors",
+ .attrs = fme_global_err_attrs,
+ .is_visible = fme_global_err_attrs_visible,
+};
+
+static void fme_err_mask(struct device *dev, bool mask)
+{
+ struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+ void __iomem *base;
+
+ base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+ mutex_lock(&pdata->lock);
+
+ /* Workaround: keep MBP_ERROR always masked if revision is 0 */
+ if (dfl_feature_revision(base))
+ writeq(mask ? ERROR_MASK : 0, base + FME_ERROR_MASK);
+ else
+ writeq(mask ? ERROR_MASK : MBP_ERROR, base + FME_ERROR_MASK);
+
+ writeq(mask ? ERROR_MASK : 0, base + PCIE0_ERROR_MASK);
+ writeq(mask ? ERROR_MASK : 0, base + PCIE1_ERROR_MASK);
+ writeq(mask ? ERROR_MASK : 0, base + RAS_NONFAT_ERROR_MASK);
+ writeq(mask ? ERROR_MASK : 0, base + RAS_CATFAT_ERROR_MASK);
+
+ mutex_unlock(&pdata->lock);
+}
+
+static int fme_global_err_init(struct platform_device *pdev,
+ struct dfl_feature *feature)
+{
+ fme_err_mask(&pdev->dev, false);
+
+ return 0;
+}
+
+static void fme_global_err_uinit(struct platform_device *pdev,
+ struct dfl_feature *feature)
+{
+ fme_err_mask(&pdev->dev, true);
+}
+
+const struct dfl_feature_id fme_global_err_id_table[] = {
+ {.id = FME_FEATURE_ID_GLOBAL_ERR,},
+ {0,}
+};
+
+const struct dfl_feature_ops fme_global_err_ops = {
+ .init = fme_global_err_init,
+ .uinit = fme_global_err_uinit,
+};
diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c
index bf8114d..4d78e18 100644
--- a/drivers/fpga/dfl-fme-main.c
+++ b/drivers/fpga/dfl-fme-main.c
@@ -127,7 +127,10 @@ static ssize_t socket_id_show(struct device *dev,
&dev_attr_socket_id.attr,
NULL,
};
-ATTRIBUTE_GROUPS(fme_hdr);
+
+static const struct attribute_group fme_hdr_group = {
+ .attrs = fme_hdr_attrs,
+};
static long fme_hdr_ioctl_release_port(struct dfl_feature_platform_data *pdata,
unsigned long arg)
@@ -188,6 +191,10 @@ static long fme_hdr_ioctl(struct platform_device *pdev,
.ops = &fme_pr_mgmt_ops,
},
{
+ .id_table = fme_global_err_id_table,
+ .ops = &fme_global_err_ops,
+ },
+ {
.ops = NULL,
},
};
@@ -333,10 +340,16 @@ static int fme_remove(struct platform_device *pdev)
return 0;
}
+static const struct attribute_group *fme_dev_groups[] = {
+ &fme_hdr_group,
+ &fme_global_err_group,
+ NULL
+};
+
static struct platform_driver fme_driver = {
.driver = {
.name = DFL_FPGA_FEATURE_DEV_FME,
- .dev_groups = fme_hdr_groups,
+ .dev_groups = fme_dev_groups,
},
.probe = fme_probe,
.remove = fme_remove,
diff --git a/drivers/fpga/dfl-fme.h b/drivers/fpga/dfl-fme.h
index e4131e8..6685c8e 100644
--- a/drivers/fpga/dfl-fme.h
+++ b/drivers/fpga/dfl-fme.h
@@ -35,5 +35,8 @@ struct dfl_fme {
extern const struct dfl_feature_ops fme_pr_mgmt_ops;
extern const struct dfl_feature_id fme_pr_mgmt_id_table[];
+extern const struct dfl_feature_ops fme_global_err_ops;
+extern const struct dfl_feature_id fme_global_err_id_table[];
+extern const struct attribute_group fme_global_err_group;
#endif /* __DFL_FME_H */
--
1.8.3.1
^ permalink raw reply related
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