Re: [PATCH v5 30/46] regulator: pwm: retrieve correct voltage
Hi Mark, On Tue, 12 Apr 2016 05:42:03 +0100 Mark Brown wrote: > On Thu, Apr 07, 2016 at 11:54:31PM +0200, Boris Brezillon wrote: > > > Is there any reason for calling set_machine_constraints() after > > device_register() in regulator_register()? > > I'm not sure there's a strong one, we don't really use the class device > for anything, but without doing a full audit I couldn't guarantee that. At first glance I don't see any problem (even the rdev_err/info/...() functions do not use dev_err/info/...()). The patch will be part of v6 (unless you want me to send it independently). Thanks, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel docs: muddying the waters a bit
Hi Markus, On 04/08/16 17:12, Markus Heiser wrote: > Hi kernel-doc authors, > > motivated by this MT, I implemented a toolchain to migrate the kernel’s > DocBook XML documentation to reST markup. > > It converts 99% of the docs well ... to gain an impression how > kernel-docs could benefit from, visit my sphkerneldoc project page > on github: > > http://return42.github.io/sphkerneldoc/ > > The sources available at: > > https://github.com/return42/sphkerneldoc > > The work is underway, suggestions are welcome! I have to admit that this looks pretty good :-) My main remark based on my quick scan through the docs is that anything in typewriter font seems to be shown as red text with a rectangle around it. That's quite jarring for me and I think it should be just shown as normal text, just using a non-proportional font, just like in the original. I also noticed that the 'title' of tables ends with a '¶' character for some reason. See e.g. the struct v4l2_audioout table in http://return42.github.io/sphkerneldoc/books/linux_tv/media/v4l/vidioc-g-audioout.html Regards, Hans > > .. have a nice weekend .. > > --M-- > > > Am 13.03.2016 um 16:33 schrieb Markus Heiser : > >> >> Am 10.03.2016 um 16:21 schrieb Mauro Carvalho Chehab >> : >> >>> Em Thu, 10 Mar 2016 12:25:58 +0200 >>> Jani Nikula escreveu: >>> TL;DR? Skip to the last paragraph. On Wed, 09 Mar 2016, Mauro Carvalho Chehab wrote: > I guess the conversion to asciidoc format is now in good shape, > at least to demonstrate that it is possible to use this format for the > media docbook. Still, there are lots of broken references. Getting references right with asciidoc is a big problem in the kernel-doc side. As I wrote before, the proofs of concept only worked because everything was processed as one big file (via includes). The Asciidoctor inter-document references won't help, because we won't know the target document name while processing kernel-doc. >>> >>> I was able to produce chunked htmls here with: >>> >>> asciidoctor -b docbook45 media_api.adoc >>> xmlto -o html-dir html media_api.xml >>> >>> The results are at: >>> >>> https://mchehab.fedorapeople.org/media-kabi-docs-test/asciidoc_tests/chunked/ >>> >>> But yeah, all references seem to be broken there. It could be due to some >>> conversion issue (I didn't actually tried to check what's wrong there), >>> but I think that there's something not ok with docbook45 >>> output for multi-part documents (on both AsciiDoc and Asciidoctor). >>> Sphinx is massively better at handling cross references for kernel-doc. We can use domains (C language) and roles (e.g. functions, types, etc.) for the references, which provide kind of namespaces. Sphinx warns for referencing non-existing targets, but doesn't generate broken links in the result like Asciidoctor does. For example, in the documentation for a function that has struct foo as parameter or return type, a cross reference to struct foo is added automagically, but only if documentation for struct foo actually exists. In Asciidoctor, we would have to blindly generate the references ourselves, and try to resolve broken links ourselves by somehow post-processing the result. > Yet, from my side, if we're willing to get rid of DocBook, then > Asciidoctor seems to be the *only* alternative so far to parse the > complex media documents. I think you mean, "get rid of DocBook as source format", not altogether? I'm yet to be convinved we could rely on Asciidoctor's native formats. >>> >>> What I mean is that, right now, I see only two alternatives for the >>> media uAPI documentation: >>> 1) keep using DocBook; >>> 2) AsciiDoc/Asciidoctor. >>> >>> Sphinx doesn't have what's needed to support the complexity of the >>> media books, specially since cell span seems to be possible only >>> by using asciiArt formats. Writing a big table using asciiArt is >>> something that is a *real pain*. Also, as tested, if the table is >>> too big, it fails to parse such asciiArt tables. So, while Sphinx >>> doesn't have a decent way to describe tables, we can't use it. >> >> >> Huge tables and cell-spans are the *real pain* ;-) ... with sphinx-doc, >> (mostly) you have more then one choice .. e.g. import csv tables .. >> but this should be discussed by example ... >> >> >>> If it starts implementing it, then we can check if the other >>> features used by the media documentation are also supported. >>> Probably, multi-part books would be another pain with Sphinx. >>> We have actually 4 books inside a common body. A few chapters >>> (like book licensing, bibliography, error codes) are shared >>> by all 4 documents. >>> >>> But, so far, I can't see any way to port media books without >>> lots of lot of work to develop new features at the Sphinx code. >> >> >> may I can help you ... >> >>
Re: [PATCH v5 30/46] regulator: pwm: retrieve correct voltage
On Tue, Apr 12, 2016 at 10:37:22AM +0200, Boris Brezillon wrote: > Mark Brown wrote: > > On Thu, Apr 07, 2016 at 11:54:31PM +0200, Boris Brezillon wrote: > > I'm not sure there's a strong one, we don't really use the class device > > for anything, but without doing a full audit I couldn't guarantee that. > At first glance I don't see any problem (even the rdev_err/info/...() > functions do not use dev_err/info/...()). The patch will be part of v6 > (unless you want me to send it independently). I'd rather it didn't get sucked into this series since it seems that it's getting delayed indefinitely - I can apply it to the regulator tree and create a tag that can be pulled into other trees as needed if things do get applied. signature.asc Description: PGP signature
Re: [PATCH v5 30/46] regulator: pwm: retrieve correct voltage
On Tue, 12 Apr 2016 11:09:38 +0100 Mark Brown wrote: > On Tue, Apr 12, 2016 at 10:37:22AM +0200, Boris Brezillon wrote: > > Mark Brown wrote: > > > On Thu, Apr 07, 2016 at 11:54:31PM +0200, Boris Brezillon wrote: > > > > I'm not sure there's a strong one, we don't really use the class device > > > for anything, but without doing a full audit I couldn't guarantee that. > > > At first glance I don't see any problem (even the rdev_err/info/...() > > functions do not use dev_err/info/...()). The patch will be part of v6 > > (unless you want me to send it independently). > > I'd rather it didn't get sucked into this series since it seems that > it's getting delayed indefinitely - I can apply it to the regulator tree > and create a tag that can be pulled into other trees as needed if things > do get applied. Done. -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 01/46] pwm: rcar: make use of pwm_is_enabled()
On Wed, Mar 30, 2016 at 10:03:24PM +0200, Boris Brezillon wrote:
> Commit 5c31252c4a86 ("pwm: Add the pwm_is_enabled() helper") introduced a
> new function to test whether a PWM device is enabled or not without
> manipulating PWM internal fields.
> Hiding this is necessary if we want to smoothly move to the atomic PWM
> config approach without impacting PWM drivers.
> Fix this driver to use pwm_is_enabled() instead of directly accessing the
> ->flags field.
>
> Signed-off-by: Boris Brezillon
> ---
> drivers/pwm/pwm-rcar.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Applied, thanks.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v5 02/46] backlight: pwm_bl: remove useless call to pwm_set_period()
On Wed, Mar 30, 2016 at 10:03:25PM +0200, Boris Brezillon wrote: > The PWM period will be set when calling pwm_config. Remove this useless > call to pwm_set_period(), which might mess up with the internal PWM state. > > Signed-off-by: Boris Brezillon > Acked-by: Lee Jones > --- > drivers/video/backlight/pwm_bl.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) Applied, thanks. Thierry signature.asc Description: PGP signature
Re: [PATCH v5 03/46] backlight: lm3630a_bl: stop messing with the pwm->period field
On Wed, Mar 30, 2016 at 10:03:26PM +0200, Boris Brezillon wrote: > pwm->period field is not supposed to be changed by PWM users. The only > ones authorized to change it are the PWM core and PWM drivers. > > Signed-off-by: Boris Brezillon > --- > drivers/video/backlight/lm3630a_bl.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) Applied, thanks. Thierry signature.asc Description: PGP signature
Re: [PATCH v5 04/46] pwm: get rid of pwm->lock
On Wed, Mar 30, 2016 at 10:03:27PM +0200, Boris Brezillon wrote: > PWM devices are not protected against concurrent accesses. The lock in > pwm_device might let PWM users think it is, but it's actually only > protecting the enabled state. > > Removing this lock should be fine as long as all PWM users are aware that > accesses to the PWM device have to be serialized, which seems to be the > case for all of them except the sysfs interface. > Patch the sysfs code by adding a lock to the pwm_export struct and making > sure it's taken for all accesses to the exported PWM device. > > Signed-off-by: Boris Brezillon > --- > drivers/pwm/core.c | 19 -- > drivers/pwm/sysfs.c | 57 > ++--- > include/linux/pwm.h | 2 -- > 3 files changed, 50 insertions(+), 28 deletions(-) This is a little overzealous. Only accesses that can cause races need to be protected by the lock. All of the *_show() callbacks don't modify the PWM device in any way, so there is no need to protect them against concurrent accesses. I've dropped the changes when applying. Thanks, Thierry signature.asc Description: PGP signature
Re: [PATCH v5 04/46] pwm: get rid of pwm->lock
Hi Thierry, On Tue, 12 Apr 2016 13:22:46 +0200 Thierry Reding wrote: > On Wed, Mar 30, 2016 at 10:03:27PM +0200, Boris Brezillon wrote: > > PWM devices are not protected against concurrent accesses. The lock in > > pwm_device might let PWM users think it is, but it's actually only > > protecting the enabled state. > > > > Removing this lock should be fine as long as all PWM users are aware that > > accesses to the PWM device have to be serialized, which seems to be the > > case for all of them except the sysfs interface. > > Patch the sysfs code by adding a lock to the pwm_export struct and making > > sure it's taken for all accesses to the exported PWM device. > > > > Signed-off-by: Boris Brezillon > > --- > > drivers/pwm/core.c | 19 -- > > drivers/pwm/sysfs.c | 57 > > ++--- > > include/linux/pwm.h | 2 -- > > 3 files changed, 50 insertions(+), 28 deletions(-) > > This is a little overzealous. Only accesses that can cause races need to > be protected by the lock. All of the *_show() callbacks don't modify the > PWM device in any way, so there is no need to protect them against > concurrent accesses. This is probably true for this set of changes, but what will happen when we'll switch to the atomic API? There's no guarantee that pwm->state = *newstate is done atomically, and you may see a partially updated state when calling pwm_get_state() while another thread is calling pwm_apply_state(). -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 05/46] pwm: introduce the pwm_args concept
On Wed, Mar 30, 2016 at 10:03:28PM +0200, Boris Brezillon wrote:
> Currently the PWM core mixes the current PWM state with the per-platform
> reference config (specified through the PWM lookup table, DT definition or
> directly hardcoded in PWM drivers).
>
> Create a pwm_args struct to store this reference config, so that PWM users
> can differentiate the current config from the reference one.
>
> Patch all places where pwm->args should be initialized. We keep the
> pwm_set_polarity/period() calls until all PWM users are patched to
> use pwm_args instead of pwm_get_period/polarity().
Perhaps a helper would be useful? Something like:
static inline void
pwm_apply_args(struct pwm_device *pwm, const struct pwm_args *args)
{
pwm_set_duty_cycle(pwm, args->duty_cycle);
pwm_set_period(pwm, args->period);
}
? That would make it slightly easier to get rid of it again after all
clients have been converted.
With the exception of pwm-clps711x all of these args are set at of_xlate
time (for DT) or from the lookup table in pwm_get() (for non-DT), so it
might even be possible to move this call to the core, so that removal of
it will be a one-liner.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v5 04/46] pwm: get rid of pwm->lock
On Tue, Apr 12, 2016 at 01:32:55PM +0200, Boris Brezillon wrote: > Hi Thierry, > > On Tue, 12 Apr 2016 13:22:46 +0200 > Thierry Reding wrote: > > > On Wed, Mar 30, 2016 at 10:03:27PM +0200, Boris Brezillon wrote: > > > PWM devices are not protected against concurrent accesses. The lock in > > > pwm_device might let PWM users think it is, but it's actually only > > > protecting the enabled state. > > > > > > Removing this lock should be fine as long as all PWM users are aware that > > > accesses to the PWM device have to be serialized, which seems to be the > > > case for all of them except the sysfs interface. > > > Patch the sysfs code by adding a lock to the pwm_export struct and making > > > sure it's taken for all accesses to the exported PWM device. > > > > > > Signed-off-by: Boris Brezillon > > > --- > > > drivers/pwm/core.c | 19 -- > > > drivers/pwm/sysfs.c | 57 > > > ++--- > > > include/linux/pwm.h | 2 -- > > > 3 files changed, 50 insertions(+), 28 deletions(-) > > > > This is a little overzealous. Only accesses that can cause races need to > > be protected by the lock. All of the *_show() callbacks don't modify the > > PWM device in any way, so there is no need to protect them against > > concurrent accesses. > > This is probably true for this set of changes, but what will happen > when we'll switch to the atomic API? There's no guarantee that > pwm->state = *newstate is done atomically, and you may see a partially > updated state when calling pwm_get_state() while another thread is > calling pwm_apply_state(). I'd argue that for sysfs it doesn't matter since the userspace API is non-atomic anyway. As such it doesn't really matter which part of the state you're getting because only one field from the query is exposed to userspace, hence the coherence of the state is irrelevant. All other users should be taking care of the serialization themselves already. Thierry signature.asc Description: PGP signature
Re: [PATCH v5 15/46] pwm: introduce the pwm_state concept
On Wed, Mar 30, 2016 at 10:03:38PM +0200, Boris Brezillon wrote: > The PWM state, represented by its period, duty_cycle and polarity, > is currently directly stored in the PWM device. > Declare a pwm_state structure embedding those field so that we can later > use this struct to atomically update all the PWM parameters at once. > > All pwm_get_xxx() helpers are now implemented as wrappers around > pwm_get_state(). > > Signed-off-by: Boris Brezillon > --- > drivers/pwm/core.c | 8 > include/linux/pwm.h | 54 > + > 2 files changed, 46 insertions(+), 16 deletions(-) > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > index 6433059..f3f91e7 100644 > --- a/drivers/pwm/core.c > +++ b/drivers/pwm/core.c > @@ -268,7 +268,7 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip, > pwm->chip = chip; > pwm->pwm = chip->base + i; > pwm->hwpwm = i; > - pwm->polarity = polarity; > + pwm->state.polarity = polarity; Would this not more correctly be assigned to pwm->args.polarity? After all this is setting up the "initial" state, much like DT or the lookup tables would for duty cycle and period. Thierry signature.asc Description: PGP signature
Re: [PATCH v5 16/46] pwm: move the enabled/disabled info into pwm_state
On Wed, Mar 30, 2016 at 10:03:39PM +0200, Boris Brezillon wrote:
[...]
> @@ -145,7 +146,11 @@ static inline void pwm_get_state(const struct pwm_device
> *pwm,
>
> static inline bool pwm_is_enabled(const struct pwm_device *pwm)
> {
> - return test_bit(PWMF_ENABLED, &pwm->flags);
> + struct pwm_state pstate;
No need for the p prefix, in my opinion.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v5 05/46] pwm: introduce the pwm_args concept
On Tue, 12 Apr 2016 13:39:12 +0200
Thierry Reding wrote:
> On Wed, Mar 30, 2016 at 10:03:28PM +0200, Boris Brezillon wrote:
> > Currently the PWM core mixes the current PWM state with the per-platform
> > reference config (specified through the PWM lookup table, DT definition or
> > directly hardcoded in PWM drivers).
> >
> > Create a pwm_args struct to store this reference config, so that PWM users
> > can differentiate the current config from the reference one.
> >
> > Patch all places where pwm->args should be initialized. We keep the
> > pwm_set_polarity/period() calls until all PWM users are patched to
> > use pwm_args instead of pwm_get_period/polarity().
>
> Perhaps a helper would be useful? Something like:
>
> static inline void
> pwm_apply_args(struct pwm_device *pwm, const struct pwm_args *args)
> {
> pwm_set_duty_cycle(pwm, args->duty_cycle);
> pwm_set_period(pwm, args->period);
> }
>
> ? That would make it slightly easier to get rid of it again after all
> clients have been converted.
Sure. I'll add this helper.
>
> With the exception of pwm-clps711x all of these args are set at of_xlate
> time (for DT) or from the lookup table in pwm_get() (for non-DT), so it
> might even be possible to move this call to the core, so that removal of
> it will be a one-liner.
Not sure I get that one. Some drivers are implementing their own
->of_xlate() method, how would you get rid of this pwm_apply_args() in
those custom implementations?
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 15/46] pwm: introduce the pwm_state concept
On Tue, 12 Apr 2016 13:49:04 +0200 Thierry Reding wrote: > On Wed, Mar 30, 2016 at 10:03:38PM +0200, Boris Brezillon wrote: > > The PWM state, represented by its period, duty_cycle and polarity, > > is currently directly stored in the PWM device. > > Declare a pwm_state structure embedding those field so that we can later > > use this struct to atomically update all the PWM parameters at once. > > > > All pwm_get_xxx() helpers are now implemented as wrappers around > > pwm_get_state(). > > > > Signed-off-by: Boris Brezillon > > --- > > drivers/pwm/core.c | 8 > > include/linux/pwm.h | 54 > > + > > 2 files changed, 46 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > > index 6433059..f3f91e7 100644 > > --- a/drivers/pwm/core.c > > +++ b/drivers/pwm/core.c > > @@ -268,7 +268,7 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip, > > pwm->chip = chip; > > pwm->pwm = chip->base + i; > > pwm->hwpwm = i; > > - pwm->polarity = polarity; > > + pwm->state.polarity = polarity; > > Would this not more correctly be assigned to pwm->args.polarity? After > all this is setting up the "initial" state, much like DT or the lookup > tables would for duty cycle and period. Yes, I wasn't sure about the pwm_add_with_polarity() meaning. To me, all the reference info should be extracted from DT, PWM lookup table or driver specific ->request() implementation, but I can definitely initialize the args.polarity here too. Should I keep the pwm->state.polarity assignment (to set the initial polarity when the driver does not support hardware readout)? -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 05/46] pwm: introduce the pwm_args concept
On Tue, Apr 12, 2016 at 02:04:12PM +0200, Boris Brezillon wrote:
> On Tue, 12 Apr 2016 13:39:12 +0200
> Thierry Reding wrote:
>
> > On Wed, Mar 30, 2016 at 10:03:28PM +0200, Boris Brezillon wrote:
> > > Currently the PWM core mixes the current PWM state with the per-platform
> > > reference config (specified through the PWM lookup table, DT definition or
> > > directly hardcoded in PWM drivers).
> > >
> > > Create a pwm_args struct to store this reference config, so that PWM users
> > > can differentiate the current config from the reference one.
> > >
> > > Patch all places where pwm->args should be initialized. We keep the
> > > pwm_set_polarity/period() calls until all PWM users are patched to
> > > use pwm_args instead of pwm_get_period/polarity().
> >
> > Perhaps a helper would be useful? Something like:
> >
> > static inline void
> > pwm_apply_args(struct pwm_device *pwm, const struct pwm_args *args)
> > {
> > pwm_set_duty_cycle(pwm, args->duty_cycle);
> > pwm_set_period(pwm, args->period);
> > }
> >
> > ? That would make it slightly easier to get rid of it again after all
> > clients have been converted.
>
> Sure. I'll add this helper.
>
> >
> > With the exception of pwm-clps711x all of these args are set at of_xlate
> > time (for DT) or from the lookup table in pwm_get() (for non-DT), so it
> > might even be possible to move this call to the core, so that removal of
> > it will be a one-liner.
>
> Not sure I get that one. Some drivers are implementing their own
> ->of_xlate() method, how would you get rid of this pwm_apply_args() in
> those custom implementations?
I was proposing to have pwm_apply_args() called from the core.
of_pwm_get() is where ->of_xlate() is called from, and the lookup table
arguments would be applied in pwm_get(). Taking into account clps711x,
which sets the arguments in ->request() it might be possible to simply
call pwm_apply_args() from pwm_device_request(), since that's also
called by all other request functions, even the legacy ones.
That said, the amount of code to modify isn't that large, so I'm fine if
you want to keep sprinkling the calls across multiple files, especially
since it's temporary.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v5 15/46] pwm: introduce the pwm_state concept
On Tue, Apr 12, 2016 at 02:17:18PM +0200, Boris Brezillon wrote: > On Tue, 12 Apr 2016 13:49:04 +0200 > Thierry Reding wrote: > > > On Wed, Mar 30, 2016 at 10:03:38PM +0200, Boris Brezillon wrote: > > > The PWM state, represented by its period, duty_cycle and polarity, > > > is currently directly stored in the PWM device. > > > Declare a pwm_state structure embedding those field so that we can later > > > use this struct to atomically update all the PWM parameters at once. > > > > > > All pwm_get_xxx() helpers are now implemented as wrappers around > > > pwm_get_state(). > > > > > > Signed-off-by: Boris Brezillon > > > --- > > > drivers/pwm/core.c | 8 > > > include/linux/pwm.h | 54 > > > + > > > 2 files changed, 46 insertions(+), 16 deletions(-) > > > > > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > > > index 6433059..f3f91e7 100644 > > > --- a/drivers/pwm/core.c > > > +++ b/drivers/pwm/core.c > > > @@ -268,7 +268,7 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip, > > > pwm->chip = chip; > > > pwm->pwm = chip->base + i; > > > pwm->hwpwm = i; > > > - pwm->polarity = polarity; > > > + pwm->state.polarity = polarity; > > > > Would this not more correctly be assigned to pwm->args.polarity? After > > all this is setting up the "initial" state, much like DT or the lookup > > tables would for duty cycle and period. > > Yes, I wasn't sure about the pwm_add_with_polarity() meaning. To me, > all the reference info should be extracted from DT, PWM lookup table or > driver specific ->request() implementation, but I can definitely > initialize the args.polarity here too. > > Should I keep the pwm->state.polarity assignment (to set the initial > polarity when the driver does not support hardware readout)? Wouldn't this work automatically as part of the pwm_apply_args() helper if we extended it with this setting? Thierry signature.asc Description: PGP signature
Re: [PATCH v5 15/46] pwm: introduce the pwm_state concept
On Tue, 12 Apr 2016 14:21:41 +0200 Thierry Reding wrote: > On Tue, Apr 12, 2016 at 02:17:18PM +0200, Boris Brezillon wrote: > > On Tue, 12 Apr 2016 13:49:04 +0200 > > Thierry Reding wrote: > > > > > On Wed, Mar 30, 2016 at 10:03:38PM +0200, Boris Brezillon wrote: > > > > The PWM state, represented by its period, duty_cycle and polarity, > > > > is currently directly stored in the PWM device. > > > > Declare a pwm_state structure embedding those field so that we can later > > > > use this struct to atomically update all the PWM parameters at once. > > > > > > > > All pwm_get_xxx() helpers are now implemented as wrappers around > > > > pwm_get_state(). > > > > > > > > Signed-off-by: Boris Brezillon > > > > --- > > > > drivers/pwm/core.c | 8 > > > > include/linux/pwm.h | 54 > > > > + > > > > 2 files changed, 46 insertions(+), 16 deletions(-) > > > > > > > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > > > > index 6433059..f3f91e7 100644 > > > > --- a/drivers/pwm/core.c > > > > +++ b/drivers/pwm/core.c > > > > @@ -268,7 +268,7 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip, > > > > pwm->chip = chip; > > > > pwm->pwm = chip->base + i; > > > > pwm->hwpwm = i; > > > > - pwm->polarity = polarity; > > > > + pwm->state.polarity = polarity; > > > > > > Would this not more correctly be assigned to pwm->args.polarity? After > > > all this is setting up the "initial" state, much like DT or the lookup > > > tables would for duty cycle and period. > > > > Yes, I wasn't sure about the pwm_add_with_polarity() meaning. To me, > > all the reference info should be extracted from DT, PWM lookup table or > > driver specific ->request() implementation, but I can definitely > > initialize the args.polarity here too. > > > > Should I keep the pwm->state.polarity assignment (to set the initial > > polarity when the driver does not support hardware readout)? > > Wouldn't this work automatically as part of the pwm_apply_args() helper > if we extended it with this setting? Well, as you explained in you answer to patch 5, pwm_apply_args() should be called on a per-request basis (each time a PWM device is requested), while the initial polarity setting should only be applied when registering the PWM chip (and its devices). After that, the framework takes care of keeping the PWM state in sync with the hardware state. Let's take a real (though a bit unusual) example. Say you have a single PWM device referenced by two different users. Only one user can be enabled at a time, but each of them has its own reference config (different polarity, different period). User1 calls pwm_get() and applies its own polarity and period. Then user1 is unregistered and release the PWM device, leaving the polarity and period untouched. User2 is registered and request the same PWM device, but user2 is smarter and tries to extract the current PWM state before adapting the config according to pwm_args. If you just reset pwm->state.polarity each time pwm_apply_args() is called (and you suggested to call it as part of the request procedure), then this means the PWM state is no longer in sync with the hardware state. -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 05/46] pwm: introduce the pwm_args concept
On Tue, 12 Apr 2016 14:20:29 +0200
Thierry Reding wrote:
> On Tue, Apr 12, 2016 at 02:04:12PM +0200, Boris Brezillon wrote:
> > On Tue, 12 Apr 2016 13:39:12 +0200
> > Thierry Reding wrote:
> >
> > > On Wed, Mar 30, 2016 at 10:03:28PM +0200, Boris Brezillon wrote:
> > > > Currently the PWM core mixes the current PWM state with the per-platform
> > > > reference config (specified through the PWM lookup table, DT definition
> > > > or
> > > > directly hardcoded in PWM drivers).
> > > >
> > > > Create a pwm_args struct to store this reference config, so that PWM
> > > > users
> > > > can differentiate the current config from the reference one.
> > > >
> > > > Patch all places where pwm->args should be initialized. We keep the
> > > > pwm_set_polarity/period() calls until all PWM users are patched to
> > > > use pwm_args instead of pwm_get_period/polarity().
> > >
> > > Perhaps a helper would be useful? Something like:
> > >
> > > static inline void
> > > pwm_apply_args(struct pwm_device *pwm, const struct pwm_args *args)
> > > {
> > > pwm_set_duty_cycle(pwm, args->duty_cycle);
> > > pwm_set_period(pwm, args->period);
> > > }
> > >
> > > ? That would make it slightly easier to get rid of it again after all
> > > clients have been converted.
> >
> > Sure. I'll add this helper.
> >
> > >
> > > With the exception of pwm-clps711x all of these args are set at of_xlate
> > > time (for DT) or from the lookup table in pwm_get() (for non-DT), so it
> > > might even be possible to move this call to the core, so that removal of
> > > it will be a one-liner.
> >
> > Not sure I get that one. Some drivers are implementing their own
> > ->of_xlate() method, how would you get rid of this pwm_apply_args() in
> > those custom implementations?
>
> I was proposing to have pwm_apply_args() called from the core.
> of_pwm_get() is where ->of_xlate() is called from, and the lookup table
> arguments would be applied in pwm_get(). Taking into account clps711x,
> which sets the arguments in ->request() it might be possible to simply
> call pwm_apply_args() from pwm_device_request(), since that's also
> called by all other request functions, even the legacy ones.
>
> That said, the amount of code to modify isn't that large, so I'm fine if
> you want to keep sprinkling the calls across multiple files, especially
> since it's temporary.
No, I'm fine addressing that, but I just don't get where you'd get the
pwm_args to apply. Do you suggest to pass 'struct pwm_args *' to the
->of_xlate() and ->request() methods?
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 05/46] pwm: introduce the pwm_args concept
On Tue, 12 Apr 2016 13:39:12 +0200
Thierry Reding wrote:
> On Wed, Mar 30, 2016 at 10:03:28PM +0200, Boris Brezillon wrote:
> > Currently the PWM core mixes the current PWM state with the per-platform
> > reference config (specified through the PWM lookup table, DT definition or
> > directly hardcoded in PWM drivers).
> >
> > Create a pwm_args struct to store this reference config, so that PWM users
> > can differentiate the current config from the reference one.
> >
> > Patch all places where pwm->args should be initialized. We keep the
> > pwm_set_polarity/period() calls until all PWM users are patched to
> > use pwm_args instead of pwm_get_period/polarity().
>
> Perhaps a helper would be useful? Something like:
>
> static inline void
> pwm_apply_args(struct pwm_device *pwm, const struct pwm_args *args)
> {
> pwm_set_duty_cycle(pwm, args->duty_cycle);
> pwm_set_period(pwm, args->period);
> }
>
> ? That would make it slightly easier to get rid of it again after all
> clients have been converted.
>
> With the exception of pwm-clps711x all of these args are set at of_xlate
> time (for DT) or from the lookup table in pwm_get() (for non-DT), so it
> might even be possible to move this call to the core, so that removal of
> it will be a one-liner.
Okay, I think I misunderstood your suggestion. I thought you wanted
this helper to set the reference config, but you actually want to apply
a new state based on the PWM reference values.
Except that pwm_args does not contain all the required information to
apply a full config (args->duty_cycle and args->enable do not exist).
This being said, in my v6 I moved the content of
pwm_regulator_adjust_pwm_config() (patch 27) into a generic helper
(pwm_adjust_config()). This helper is doing pretty much what you're
suggesting here (but again, I'm not sure I correctly understood your
suggestion :-/).
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 15/46] pwm: introduce the pwm_state concept
On Tue, Apr 12, 2016 at 02:45:08PM +0200, Boris Brezillon wrote: > On Tue, 12 Apr 2016 14:21:41 +0200 > Thierry Reding wrote: > > > On Tue, Apr 12, 2016 at 02:17:18PM +0200, Boris Brezillon wrote: > > > On Tue, 12 Apr 2016 13:49:04 +0200 > > > Thierry Reding wrote: > > > > > > > On Wed, Mar 30, 2016 at 10:03:38PM +0200, Boris Brezillon wrote: > > > > > The PWM state, represented by its period, duty_cycle and polarity, > > > > > is currently directly stored in the PWM device. > > > > > Declare a pwm_state structure embedding those field so that we can > > > > > later > > > > > use this struct to atomically update all the PWM parameters at once. > > > > > > > > > > All pwm_get_xxx() helpers are now implemented as wrappers around > > > > > pwm_get_state(). > > > > > > > > > > Signed-off-by: Boris Brezillon > > > > > --- > > > > > drivers/pwm/core.c | 8 > > > > > include/linux/pwm.h | 54 > > > > > + > > > > > 2 files changed, 46 insertions(+), 16 deletions(-) > > > > > > > > > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > > > > > index 6433059..f3f91e7 100644 > > > > > --- a/drivers/pwm/core.c > > > > > +++ b/drivers/pwm/core.c > > > > > @@ -268,7 +268,7 @@ int pwmchip_add_with_polarity(struct pwm_chip > > > > > *chip, > > > > > pwm->chip = chip; > > > > > pwm->pwm = chip->base + i; > > > > > pwm->hwpwm = i; > > > > > - pwm->polarity = polarity; > > > > > + pwm->state.polarity = polarity; > > > > > > > > Would this not more correctly be assigned to pwm->args.polarity? After > > > > all this is setting up the "initial" state, much like DT or the lookup > > > > tables would for duty cycle and period. > > > > > > Yes, I wasn't sure about the pwm_add_with_polarity() meaning. To me, > > > all the reference info should be extracted from DT, PWM lookup table or > > > driver specific ->request() implementation, but I can definitely > > > initialize the args.polarity here too. > > > > > > Should I keep the pwm->state.polarity assignment (to set the initial > > > polarity when the driver does not support hardware readout)? > > > > Wouldn't this work automatically as part of the pwm_apply_args() helper > > if we extended it with this setting? > > Well, as you explained in you answer to patch 5, pwm_apply_args() > should be called on a per-request basis (each time a PWM device is > requested), while the initial polarity setting should only be applied > when registering the PWM chip (and its devices). After that, the > framework takes care of keeping the PWM state in sync with the hardware > state. > > Let's take a real (though a bit unusual) example. Say you have a single > PWM device referenced by two different users. Only one user can be > enabled at a time, but each of them has its own reference config > (different polarity, different period). > > User1 calls pwm_get() and applies its own polarity and period. Then > user1 is unregistered and release the PWM device, leaving the polarity > and period untouched. > > User2 is registered and request the same PWM device, but user2 is > smarter and tries to extract the current PWM state before adapting the > config according to pwm_args. If you just reset pwm->state.polarity > each time pwm_apply_args() is called (and you suggested to call it as > part of the request procedure), then this means the PWM state is no > longer in sync with the hardware state. In that case neither will be the period or duty cycle. Essentially this gets us back to square one where we need to decide how to handle current state vs. initial arguments. But I don't think this is really going to be an issue because this is all moot until we've moved over to the atomic API, at which point this is all going to go away anyway. Thierry signature.asc Description: PGP signature
Re: [PATCH v5 05/46] pwm: introduce the pwm_args concept
On Tue, Apr 12, 2016 at 03:06:27PM +0200, Boris Brezillon wrote:
> On Tue, 12 Apr 2016 13:39:12 +0200
> Thierry Reding wrote:
>
> > On Wed, Mar 30, 2016 at 10:03:28PM +0200, Boris Brezillon wrote:
> > > Currently the PWM core mixes the current PWM state with the per-platform
> > > reference config (specified through the PWM lookup table, DT definition or
> > > directly hardcoded in PWM drivers).
> > >
> > > Create a pwm_args struct to store this reference config, so that PWM users
> > > can differentiate the current config from the reference one.
> > >
> > > Patch all places where pwm->args should be initialized. We keep the
> > > pwm_set_polarity/period() calls until all PWM users are patched to
> > > use pwm_args instead of pwm_get_period/polarity().
> >
> > Perhaps a helper would be useful? Something like:
> >
> > static inline void
> > pwm_apply_args(struct pwm_device *pwm, const struct pwm_args *args)
> > {
> > pwm_set_duty_cycle(pwm, args->duty_cycle);
> > pwm_set_period(pwm, args->period);
> > }
> >
> > ? That would make it slightly easier to get rid of it again after all
> > clients have been converted.
> >
> > With the exception of pwm-clps711x all of these args are set at of_xlate
> > time (for DT) or from the lookup table in pwm_get() (for non-DT), so it
> > might even be possible to move this call to the core, so that removal of
> > it will be a one-liner.
>
> Okay, I think I misunderstood your suggestion. I thought you wanted
> this helper to set the reference config, but you actually want to apply
> a new state based on the PWM reference values.
>
> Except that pwm_args does not contain all the required information to
> apply a full config (args->duty_cycle and args->enable do not exist).
>
> This being said, in my v6 I moved the content of
> pwm_regulator_adjust_pwm_config() (patch 27) into a generic helper
> (pwm_adjust_config()). This helper is doing pretty much what you're
> suggesting here (but again, I'm not sure I correctly understood your
> suggestion :-/).
I'm not suggesting that pwm_apply_args() apply any state. I think we
both agreed earlier that the initial state (represented by pwm_args) was
never to be automatically applied.
What I was suggesting is that we move all the calls to pwm_set_period()
and pwm_set_duty_cycle() into a central location to make it easier to
remove them later in the series. This is really only temporary, so I
don't mind if we leave the calls sprinkled all over the place. At least
that way I hope we'll avoid confusion about what we're talking about =)
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v5 15/46] pwm: introduce the pwm_state concept
On Tue, 12 Apr 2016 15:11:18 +0200 Thierry Reding wrote: > On Tue, Apr 12, 2016 at 02:45:08PM +0200, Boris Brezillon wrote: > > On Tue, 12 Apr 2016 14:21:41 +0200 > > Thierry Reding wrote: > > > > > On Tue, Apr 12, 2016 at 02:17:18PM +0200, Boris Brezillon wrote: > > > > On Tue, 12 Apr 2016 13:49:04 +0200 > > > > Thierry Reding wrote: > > > > > > > > > On Wed, Mar 30, 2016 at 10:03:38PM +0200, Boris Brezillon wrote: > > > > > > The PWM state, represented by its period, duty_cycle and polarity, > > > > > > is currently directly stored in the PWM device. > > > > > > Declare a pwm_state structure embedding those field so that we can > > > > > > later > > > > > > use this struct to atomically update all the PWM parameters at once. > > > > > > > > > > > > All pwm_get_xxx() helpers are now implemented as wrappers around > > > > > > pwm_get_state(). > > > > > > > > > > > > Signed-off-by: Boris Brezillon > > > > > > --- > > > > > > drivers/pwm/core.c | 8 > > > > > > include/linux/pwm.h | 54 > > > > > > + > > > > > > 2 files changed, 46 insertions(+), 16 deletions(-) > > > > > > > > > > > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > > > > > > index 6433059..f3f91e7 100644 > > > > > > --- a/drivers/pwm/core.c > > > > > > +++ b/drivers/pwm/core.c > > > > > > @@ -268,7 +268,7 @@ int pwmchip_add_with_polarity(struct pwm_chip > > > > > > *chip, > > > > > > pwm->chip = chip; > > > > > > pwm->pwm = chip->base + i; > > > > > > pwm->hwpwm = i; > > > > > > - pwm->polarity = polarity; > > > > > > + pwm->state.polarity = polarity; > > > > > > > > > > Would this not more correctly be assigned to pwm->args.polarity? After > > > > > all this is setting up the "initial" state, much like DT or the lookup > > > > > tables would for duty cycle and period. > > > > > > > > Yes, I wasn't sure about the pwm_add_with_polarity() meaning. To me, > > > > all the reference info should be extracted from DT, PWM lookup table or > > > > driver specific ->request() implementation, but I can definitely > > > > initialize the args.polarity here too. > > > > > > > > Should I keep the pwm->state.polarity assignment (to set the initial > > > > polarity when the driver does not support hardware readout)? > > > > > > Wouldn't this work automatically as part of the pwm_apply_args() helper > > > if we extended it with this setting? > > > > Well, as you explained in you answer to patch 5, pwm_apply_args() > > should be called on a per-request basis (each time a PWM device is > > requested), while the initial polarity setting should only be applied > > when registering the PWM chip (and its devices). After that, the > > framework takes care of keeping the PWM state in sync with the hardware > > state. > > > > Let's take a real (though a bit unusual) example. Say you have a single > > PWM device referenced by two different users. Only one user can be > > enabled at a time, but each of them has its own reference config > > (different polarity, different period). > > > > User1 calls pwm_get() and applies its own polarity and period. Then > > user1 is unregistered and release the PWM device, leaving the polarity > > and period untouched. > > > > User2 is registered and request the same PWM device, but user2 is > > smarter and tries to extract the current PWM state before adapting the > > config according to pwm_args. If you just reset pwm->state.polarity > > each time pwm_apply_args() is called (and you suggested to call it as > > part of the request procedure), then this means the PWM state is no > > longer in sync with the hardware state. > > In that case neither will be the period or duty cycle. Essentially this > gets us back to square one where we need to decide how to handle current > state vs. initial arguments. That's not true. Now we clearly differentiate the reference config (content of pwm_args which is only a subset of what you'll find in pwm_state) and the PWM state (represented by pwm_state). We should be safe as long as we keep those 2 elements as 2 orthogonal concepts: - pwm_args is supposed to give some hint to the PWM user to help him configure it's PWM appropriately - pwm_state is here to reflect the real PWM state, and apply new configs > > But I don't think this is really going to be an issue because this is > all moot until we've moved over to the atomic API, at which point this > is all going to go away anyway. As stated in my answer to patch 5, I think I misunderstood your suggestion. pwm_apply_args() is supposed to adjust the PWM config to match the period and polarity specified in pwm_args, right? If that's the case, my question is, should we really call this function each time a new user requests a PWM instead of letting those users call the function on-demand (not all users want to adapt the current PWM config to the pwm_args, some may just want to apply a completely new
Re: [PATCH] arm64: erratum: Workaround for Kryo reserved system register read
On Thu, Apr 07, 2016 at 09:54:16AM -0600, Naveen Kaje wrote: > The ARMv8.0 architecture reserves several system register > encodings for future use. These encodings should behave > as read-only and always return zero on a read. The Kryo core > errantly causes an instruction abort upon an AArch64 > read attempt to the following system register encodings using > the MRS instruction: >3, 0, C0, [C4-C7], [2-3, 6-7] >3, 0, C0, C3, [3-7] >3, 0, C0, [C4,C6,C7], [4-5] >3, 0, C0, C2, [6-7] > All system register encodings above use the following form > Op0, Op1, CRn, CRm, Op2. > Note that some of the encodings listed above include the system > register space reserved for the following identification registers > which may appear in future revisions of the ARM architecture beyond > ARMv8.0. Is this bug affecting test or production silicon? If the former (which I presume is the case since such chip wouldn't have passed the ARM validation suite), I won't merge the workaround into mainline. It is, however, fine by me to give it to your early testers. We have a similar approach with the ARM Ltd CPUs and don't upstream any CPU errata workaround unless a partner goes with the silicon into production. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 15/46] pwm: introduce the pwm_state concept
On Tue, Apr 12, 2016 at 03:26:44PM +0200, Boris Brezillon wrote: > On Tue, 12 Apr 2016 15:11:18 +0200 > Thierry Reding wrote: > > > On Tue, Apr 12, 2016 at 02:45:08PM +0200, Boris Brezillon wrote: > > > On Tue, 12 Apr 2016 14:21:41 +0200 > > > Thierry Reding wrote: > > > > > > > On Tue, Apr 12, 2016 at 02:17:18PM +0200, Boris Brezillon wrote: > > > > > On Tue, 12 Apr 2016 13:49:04 +0200 > > > > > Thierry Reding wrote: > > > > > > > > > > > On Wed, Mar 30, 2016 at 10:03:38PM +0200, Boris Brezillon wrote: > > > > > > > The PWM state, represented by its period, duty_cycle and polarity, > > > > > > > is currently directly stored in the PWM device. > > > > > > > Declare a pwm_state structure embedding those field so that we > > > > > > > can later > > > > > > > use this struct to atomically update all the PWM parameters at > > > > > > > once. > > > > > > > > > > > > > > All pwm_get_xxx() helpers are now implemented as wrappers around > > > > > > > pwm_get_state(). > > > > > > > > > > > > > > Signed-off-by: Boris Brezillon > > > > > > > > > > > > > > --- > > > > > > > drivers/pwm/core.c | 8 > > > > > > > include/linux/pwm.h | 54 > > > > > > > + > > > > > > > 2 files changed, 46 insertions(+), 16 deletions(-) > > > > > > > > > > > > > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > > > > > > > index 6433059..f3f91e7 100644 > > > > > > > --- a/drivers/pwm/core.c > > > > > > > +++ b/drivers/pwm/core.c > > > > > > > @@ -268,7 +268,7 @@ int pwmchip_add_with_polarity(struct pwm_chip > > > > > > > *chip, > > > > > > > pwm->chip = chip; > > > > > > > pwm->pwm = chip->base + i; > > > > > > > pwm->hwpwm = i; > > > > > > > - pwm->polarity = polarity; > > > > > > > + pwm->state.polarity = polarity; > > > > > > > > > > > > Would this not more correctly be assigned to pwm->args.polarity? > > > > > > After > > > > > > all this is setting up the "initial" state, much like DT or the > > > > > > lookup > > > > > > tables would for duty cycle and period. > > > > > > > > > > Yes, I wasn't sure about the pwm_add_with_polarity() meaning. To me, > > > > > all the reference info should be extracted from DT, PWM lookup table > > > > > or > > > > > driver specific ->request() implementation, but I can definitely > > > > > initialize the args.polarity here too. > > > > > > > > > > Should I keep the pwm->state.polarity assignment (to set the initial > > > > > polarity when the driver does not support hardware readout)? > > > > > > > > Wouldn't this work automatically as part of the pwm_apply_args() helper > > > > if we extended it with this setting? > > > > > > Well, as you explained in you answer to patch 5, pwm_apply_args() > > > should be called on a per-request basis (each time a PWM device is > > > requested), while the initial polarity setting should only be applied > > > when registering the PWM chip (and its devices). After that, the > > > framework takes care of keeping the PWM state in sync with the hardware > > > state. > > > > > > Let's take a real (though a bit unusual) example. Say you have a single > > > PWM device referenced by two different users. Only one user can be > > > enabled at a time, but each of them has its own reference config > > > (different polarity, different period). > > > > > > User1 calls pwm_get() and applies its own polarity and period. Then > > > user1 is unregistered and release the PWM device, leaving the polarity > > > and period untouched. > > > > > > User2 is registered and request the same PWM device, but user2 is > > > smarter and tries to extract the current PWM state before adapting the > > > config according to pwm_args. If you just reset pwm->state.polarity > > > each time pwm_apply_args() is called (and you suggested to call it as > > > part of the request procedure), then this means the PWM state is no > > > longer in sync with the hardware state. > > > > In that case neither will be the period or duty cycle. Essentially this > > gets us back to square one where we need to decide how to handle current > > state vs. initial arguments. > > That's not true. Now we clearly differentiate the reference config > (content of pwm_args which is only a subset of what you'll find in > pwm_state) and the PWM state (represented by pwm_state). > > We should be safe as long as we keep those 2 elements as 2 orthogonal > concepts: > - pwm_args is supposed to give some hint to the PWM user to help him > configure it's PWM appropriately > - pwm_state is here to reflect the real PWM state, and apply new > configs > > > > > But I don't think this is really going to be an issue because this is > > all moot until we've moved over to the atomic API, at which point this > > is all going to go away anyway. > > As stated in my answer to patch 5, I think I misunderstood your > suggestion. pwm_apply_args() is supposed to adjust the PWM config to > match the period
Re: [PATCH v5 02/46] backlight: pwm_bl: remove useless call to pwm_set_period()
On Tue, 12 Apr 2016, Thierry Reding wrote: > On Wed, Mar 30, 2016 at 10:03:25PM +0200, Boris Brezillon wrote: > > The PWM period will be set when calling pwm_config. Remove this useless > > call to pwm_set_period(), which might mess up with the internal PWM state. > > > > Signed-off-by: Boris Brezillon > > Acked-by: Lee Jones > > --- > > drivers/video/backlight/pwm_bl.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > Applied, thanks. Applied? -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 15/46] pwm: introduce the pwm_state concept
On Tue, 12 Apr 2016 16:05:46 +0200 Thierry Reding wrote: > On Tue, Apr 12, 2016 at 03:26:44PM +0200, Boris Brezillon wrote: > > On Tue, 12 Apr 2016 15:11:18 +0200 > > Thierry Reding wrote: > > > > > On Tue, Apr 12, 2016 at 02:45:08PM +0200, Boris Brezillon wrote: > > > > On Tue, 12 Apr 2016 14:21:41 +0200 > > > > Thierry Reding wrote: > > > > > > > > > On Tue, Apr 12, 2016 at 02:17:18PM +0200, Boris Brezillon wrote: > > > > > > On Tue, 12 Apr 2016 13:49:04 +0200 > > > > > > Thierry Reding wrote: > > > > > > > > > > > > > On Wed, Mar 30, 2016 at 10:03:38PM +0200, Boris Brezillon wrote: > > > > > > > > The PWM state, represented by its period, duty_cycle and > > > > > > > > polarity, > > > > > > > > is currently directly stored in the PWM device. > > > > > > > > Declare a pwm_state structure embedding those field so that we > > > > > > > > can later > > > > > > > > use this struct to atomically update all the PWM parameters at > > > > > > > > once. > > > > > > > > > > > > > > > > All pwm_get_xxx() helpers are now implemented as wrappers around > > > > > > > > pwm_get_state(). > > > > > > > > > > > > > > > > Signed-off-by: Boris Brezillon > > > > > > > > > > > > > > > > --- > > > > > > > > drivers/pwm/core.c | 8 > > > > > > > > include/linux/pwm.h | 54 > > > > > > > > + > > > > > > > > 2 files changed, 46 insertions(+), 16 deletions(-) > > > > > > > > > > > > > > > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > > > > > > > > index 6433059..f3f91e7 100644 > > > > > > > > --- a/drivers/pwm/core.c > > > > > > > > +++ b/drivers/pwm/core.c > > > > > > > > @@ -268,7 +268,7 @@ int pwmchip_add_with_polarity(struct > > > > > > > > pwm_chip *chip, > > > > > > > > pwm->chip = chip; > > > > > > > > pwm->pwm = chip->base + i; > > > > > > > > pwm->hwpwm = i; > > > > > > > > - pwm->polarity = polarity; > > > > > > > > + pwm->state.polarity = polarity; > > > > > > > > > > > > > > Would this not more correctly be assigned to pwm->args.polarity? > > > > > > > After > > > > > > > all this is setting up the "initial" state, much like DT or the > > > > > > > lookup > > > > > > > tables would for duty cycle and period. > > > > > > > > > > > > Yes, I wasn't sure about the pwm_add_with_polarity() meaning. To me, > > > > > > all the reference info should be extracted from DT, PWM lookup > > > > > > table or > > > > > > driver specific ->request() implementation, but I can definitely > > > > > > initialize the args.polarity here too. > > > > > > > > > > > > Should I keep the pwm->state.polarity assignment (to set the initial > > > > > > polarity when the driver does not support hardware readout)? > > > > > > > > > > Wouldn't this work automatically as part of the pwm_apply_args() > > > > > helper > > > > > if we extended it with this setting? > > > > > > > > Well, as you explained in you answer to patch 5, pwm_apply_args() > > > > should be called on a per-request basis (each time a PWM device is > > > > requested), while the initial polarity setting should only be applied > > > > when registering the PWM chip (and its devices). After that, the > > > > framework takes care of keeping the PWM state in sync with the hardware > > > > state. > > > > > > > > Let's take a real (though a bit unusual) example. Say you have a single > > > > PWM device referenced by two different users. Only one user can be > > > > enabled at a time, but each of them has its own reference config > > > > (different polarity, different period). > > > > > > > > User1 calls pwm_get() and applies its own polarity and period. Then > > > > user1 is unregistered and release the PWM device, leaving the polarity > > > > and period untouched. > > > > > > > > User2 is registered and request the same PWM device, but user2 is > > > > smarter and tries to extract the current PWM state before adapting the > > > > config according to pwm_args. If you just reset pwm->state.polarity > > > > each time pwm_apply_args() is called (and you suggested to call it as > > > > part of the request procedure), then this means the PWM state is no > > > > longer in sync with the hardware state. > > > > > > In that case neither will be the period or duty cycle. Essentially this > > > gets us back to square one where we need to decide how to handle current > > > state vs. initial arguments. > > > > That's not true. Now we clearly differentiate the reference config > > (content of pwm_args which is only a subset of what you'll find in > > pwm_state) and the PWM state (represented by pwm_state). > > > > We should be safe as long as we keep those 2 elements as 2 orthogonal > > concepts: > > - pwm_args is supposed to give some hint to the PWM user to help him > > configure it's PWM appropriately > > - pwm_state is here to reflect the real PWM state, and apply new > > configs > > > > > > > > But I don't think thi
Re: [PATCH v5 03/46] backlight: lm3630a_bl: stop messing with the pwm->period field
On Tue, 12 Apr 2016, Thierry Reding wrote: > On Wed, Mar 30, 2016 at 10:03:26PM +0200, Boris Brezillon wrote: > > pwm->period field is not supposed to be changed by PWM users. The only > > ones authorized to change it are the PWM core and PWM drivers. > > > > Signed-off-by: Boris Brezillon > > --- > > drivers/video/backlight/lm3630a_bl.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > Applied, thanks. Applied? -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 02/46] backlight: pwm_bl: remove useless call to pwm_set_period()
On Tue, Apr 12, 2016 at 03:16:53PM +0100, Lee Jones wrote: > On Tue, 12 Apr 2016, Thierry Reding wrote: > > > On Wed, Mar 30, 2016 at 10:03:25PM +0200, Boris Brezillon wrote: > > > The PWM period will be set when calling pwm_config. Remove this useless > > > call to pwm_set_period(), which might mess up with the internal PWM state. > > > > > > Signed-off-by: Boris Brezillon > > > Acked-by: Lee Jones > > > --- > > > drivers/video/backlight/pwm_bl.c | 4 +--- > > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > Applied, thanks. > > Applied? You Acked it, so I assumed you were fine with me taking it through the PWM tree to take care of the dependencies. Did I assume wrongly? Thierry signature.asc Description: PGP signature
Re: [PATCH v5 03/46] backlight: lm3630a_bl: stop messing with the pwm->period field
On Tue, Apr 12, 2016 at 03:16:13PM +0100, Lee Jones wrote: > On Tue, 12 Apr 2016, Thierry Reding wrote: > > > On Wed, Mar 30, 2016 at 10:03:26PM +0200, Boris Brezillon wrote: > > > pwm->period field is not supposed to be changed by PWM users. The only > > > ones authorized to change it are the PWM core and PWM drivers. > > > > > > Signed-off-by: Boris Brezillon > > > --- > > > drivers/video/backlight/lm3630a_bl.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > Applied, thanks. > > Applied? You didn't specifically Ack this one, but I presumed that since the change is essentially the same as for pwm-backlight, and this is another prerequisite for the remainder of the series it should go in through the PWM tree as well. Thierry signature.asc Description: PGP signature
Re: Kernel docs: muddying the waters a bit
On Fri, 8 Apr 2016 17:12:27 +0200 Markus Heiser wrote: > motivated by this MT, I implemented a toolchain to migrate the kernel’s > DocBook XML documentation to reST markup. > > It converts 99% of the docs well ... to gain an impression how > kernel-docs could benefit from, visit my sphkerneldoc project page > on github: > > http://return42.github.io/sphkerneldoc/ So I've obviously been pretty quiet on this recently. Apologies...I've been dealing with an extended death-in-the-family experience, and there is still a fair amount of cleanup to be done. Looking quickly at this work, it seems similar to the results I got. But there's a lot of code there that came from somewhere? I'd put together a fairly simple conversion using pandoc and a couple of short sed scripts; is there a reason for a more complex solution? Thanks for looking into this, anyway; I hope to be able to focus more on it shortly. jon -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 00/10] Add DRM Driver for HiSilicon Kirin hi6220 SoC
On Mon, Apr 11, 2016 at 04:55:33PM +0800, Xinliang Liu wrote: > This patch set adds a new drm driver for HiSilicon Kirin hi6220 SoC. > Current testing and support board is Hikey board which is one of Linaro > 96boards. It is an arm64 open source board. For more information about > this board, please access https://www.96boards.org. > > Hardware Detail > --- > The display subsystem of Hi6220 SoC is shown as bellow: > +-+ +--+ +-+ +-+ > | | | | | | | | > | FB |-->| ADE|>| DSI |>| External | > | | | | | | | HDMI/panel | > +-+ +--+ +-+ +-+ > > - ADE(Advanced Display Engine) is the display controller. It contains 7 > channels, 3 overlay compositors and a LDI. > - A channel looks like: DMA-->clip-->scale-->ctrans(or called csc). > - Overlay compositor is response to compose planes which come from 7 > channels and pass composed image to LDI. > - LDI is response to generate timings and RGB data stream. > - DSI converts the RGB data stream from ADE to DSI packets. > - External HDMI/panel module is connected with DSI bus. Now Hikey use a > ADI's ADV7533 external HDMI chip. I haven't looked at the details again, but seems it's all ready. Please create a pull request for the entire pile against drm-next and submit it to Dave Airlie. Thanks, Daniel > > Change History > - > Changes in v8: > - Rebase to v4.6-rc3 > > Changes in v7: > - Add config.mutex protection when accessing mode_config.connector_list. > - Clean up match data getting of kirin_drm_drv.c. > - A few Regs define clean up and typo fixs for ADE crtc and DSI encoder > driver. > - Fix vblank irq flag "DRIVER_IRQF_SHARED" to "IRQF_SHARED". > > Changes in v6: > - Cleanup values part of reg and clocks relevant properties. > - Change "pclk_dsi" clock name to "pclk". > > Changes in v5: > - Remove endpoint unit address of dsi output port. > - Use syscon to access ADE media NOC QoS registers instread of directly > writing registers. > - Use reset controller to reset ADE instead of directly writing registers. > > Changes in v4: > - Describe more specific of clocks and ports of binding docs. > - Fix indentation of binding docs. > > Changes in v3: > - Move and rename all the files to kirin sub-directory. > So that we could separate different seires SoCs' driver. > - Make ade as the drm master node. > - Replace drm_platform_init, load, unload implementation. > - Use assigned-clocks to set clock rate. > - Use ports to connect display relevant nodes. > - Rename hisi_drm_dsi.c to dw_drm_dsi.c > - Make encoder type as DRM_MODE_ENCODER_DSI. > - A few cleanup on regs and code. > > Changes in v2: > - Remove abtraction layer of plane/crtc/encoder/connector. > - Refactor atomic implementation according to Daniel Vetter's guides: > http://blog.ffwll.ch/2014/11/atomic-modeset-support-for-kms-drivers.html > http://blog.ffwll.ch/2015/09/xdc-2015-atomic-modesetting-for-drivers.html > http://blog.ffwll.ch/2015/08/atomic-modesetting-design-overview.html > - Use bridge instead of slave encoder to connect external HDMI. > - Move dt binding docs to bindings/display/hisilicon directory. > > Xinliang Liu (10): > drm/hisilicon: Add device tree binding for hi6220 display subsystem > drm/hisilicon: Add hisilicon kirin drm master driver > drm/hisilicon: Add crtc driver for ADE > drm/hisilicon: Add plane driver for ADE > drm/hisilicon: Add vblank driver for ADE > drm/hisilicon: Add cma fbdev and hotplug > drm/hisilicon: Add designware dsi encoder driver > drm/hisilicon: Add designware dsi host driver > drm/hisilicon: Add support for external bridge > MAINTAINERS: Add maintainer for hisilicon DRM driver > > .../bindings/display/hisilicon/dw-dsi.txt | 72 ++ > .../bindings/display/hisilicon/hisi-ade.txt| 64 ++ > MAINTAINERS| 10 + > drivers/gpu/drm/Kconfig|2 + > drivers/gpu/drm/Makefile |1 + > drivers/gpu/drm/hisilicon/Kconfig |5 + > drivers/gpu/drm/hisilicon/Makefile |5 + > drivers/gpu/drm/hisilicon/kirin/Kconfig| 10 + > drivers/gpu/drm/hisilicon/kirin/Makefile |5 + > drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 857 > drivers/gpu/drm/hisilicon/kirin/dw_dsi_reg.h | 103 ++ > drivers/gpu/drm/hisilicon/kirin/kirin_ade_reg.h| 230 + > drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c| 1057 > > drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c| 367 +++ > drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h| 31 + > 15 files changed, 2819 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/hisilicon/dw-dsi.txt > create mode
[PATCH v4 2/6] string_helpers: add kstrdup_quotable_cmdline
Provide an escaped (but readable: no inter-argument NULLs) commandline
safe for logging.
Signed-off-by: Kees Cook
---
include/linux/string_helpers.h | 1 +
lib/string_helpers.c | 34 ++
2 files changed, 35 insertions(+)
diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
index 9de228af00c1..684d2695fc36 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -69,5 +69,6 @@ static inline int string_escape_str_any_np(const char *src,
char *dst,
}
char *kstrdup_quotable(const char *src, gfp_t gfp);
+char *kstrdup_quotable_cmdline(struct task_struct *task, gfp_t gfp);
#endif
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index aa00c9f989ee..b16ee85aaf87 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -562,3 +563,36 @@ char *kstrdup_quotable(const char *src, gfp_t gfp)
return dst;
}
EXPORT_SYMBOL_GPL(kstrdup_quotable);
+
+/*
+ * Returns allocated NULL-terminated string containing process
+ * command line, with inter-argument NULLs replaced with spaces,
+ * and other special characters escaped.
+ */
+char *kstrdup_quotable_cmdline(struct task_struct *task, gfp_t gfp)
+{
+ char *buffer, *quoted;
+ int i, res;
+
+ buffer = kmalloc(PAGE_SIZE, GFP_TEMPORARY);
+ if (!buffer)
+ return NULL;
+
+ res = get_cmdline(task, buffer, PAGE_SIZE - 1);
+ buffer[res] = '\0';
+
+ /* Collapse trailing NULLs, leave res pointing to last non-NULL. */
+ while (--res >= 0 && buffer[res] == '\0')
+ ;
+
+ /* Replace inter-argument NULLs. */
+ for (i = 0; i <= res; i++)
+ if (buffer[i] == '\0')
+ buffer[i] = ' ';
+
+ /* Make sure result is printable. */
+ quoted = kstrdup_quotable(buffer, gfp);
+ kfree(buffer);
+ return quoted;
+}
+EXPORT_SYMBOL_GPL(kstrdup_quotable_cmdline);
--
2.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/6] string_helpers: add kstrdup_quotable
Handle allocating and escaping a string safe for logging.
Signed-off-by: Kees Cook
---
include/linux/string_helpers.h | 2 ++
lib/string_helpers.c | 28
2 files changed, 30 insertions(+)
diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
index dabe643eb5fa..9de228af00c1 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -68,4 +68,6 @@ static inline int string_escape_str_any_np(const char *src,
char *dst,
return string_escape_str(src, dst, sz, ESCAPE_ANY_NP, only);
}
+char *kstrdup_quotable(const char *src, gfp_t gfp);
+
#endif
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 5c88204b6f1f..aa00c9f989ee 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
@@ -534,3 +535,30 @@ int string_escape_mem(const char *src, size_t isz, char
*dst, size_t osz,
return p - dst;
}
EXPORT_SYMBOL(string_escape_mem);
+
+/*
+ * Return an allocated string that has been escaped of special characters
+ * and double quotes, making it safe to log in quotes.
+ */
+char *kstrdup_quotable(const char *src, gfp_t gfp)
+{
+ size_t slen, dlen;
+ char *dst;
+ const int flags = ESCAPE_HEX;
+ const char esc[] = "\f\n\r\t\v\a\e\\\"";
+
+ if (!src)
+ return NULL;
+ slen = strlen(src);
+
+ dlen = string_escape_mem(src, slen, NULL, 0, flags, esc);
+ dst = kmalloc(dlen + 1, gfp);
+ if (!dst)
+ return NULL;
+
+ WARN_ON(string_escape_mem(src, slen, dst, dlen, flags, esc) != dlen);
+ dst[dlen] = '\0';
+
+ return dst;
+}
+EXPORT_SYMBOL_GPL(kstrdup_quotable);
--
2.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 6/6] LSM: LoadPin for kernel file loading restrictions
This LSM enforces that kernel-loaded files (modules, firmware, etc)
must all come from the same filesystem, with the expectation that
such a filesystem is backed by a read-only device such as dm-verity
or CDROM. This allows systems that have a verified and/or unchangeable
filesystem to enforce module and firmware loading restrictions without
needing to sign the files individually.
Signed-off-by: Kees Cook
---
Documentation/security/LoadPin.txt | 17
MAINTAINERS| 6 ++
include/linux/lsm_hooks.h | 5 +
security/Kconfig | 1 +
security/Makefile | 2 +
security/loadpin/Kconfig | 10 ++
security/loadpin/Makefile | 1 +
security/loadpin/loadpin.c | 190 +
security/security.c| 1 +
9 files changed, 233 insertions(+)
create mode 100644 Documentation/security/LoadPin.txt
create mode 100644 security/loadpin/Kconfig
create mode 100644 security/loadpin/Makefile
create mode 100644 security/loadpin/loadpin.c
diff --git a/Documentation/security/LoadPin.txt
b/Documentation/security/LoadPin.txt
new file mode 100644
index ..e11877f5d3d4
--- /dev/null
+++ b/Documentation/security/LoadPin.txt
@@ -0,0 +1,17 @@
+LoadPin is a Linux Security Module that ensures all kernel-loaded files
+(modules, firmware, etc) all originate from the same filesystem, with
+the expectation that such a filesystem is backed by a read-only device
+such as dm-verity or CDROM. This allows systems that have a verified
+and/or unchangeable filesystem to enforce module and firmware loading
+restrictions without needing to sign the files individually.
+
+The LSM is selectable at build-time with CONFIG_SECURITY_LOADPIN, and
+can be controlled at boot-time with the kernel command line option
+"loadpin.enabled". By default, it is enabled, but can be disabled at
+boot ("loadpin.enabled=0").
+
+LoadPin starts pinning when it sees the first file loaded. If the
+block device backing the filesystem is not read-only, a sysctl is
+created to toggle pinning: /proc/sys/kernel/loadpin/enabled. (Having
+a mutable filesystem means pinning is mutable too, but having the
+sysctl allows for easy testing on systems with a mutable filesystem.)
diff --git a/MAINTAINERS b/MAINTAINERS
index 40eb1dbe2ae5..de4cf8e9247e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9964,6 +9964,12 @@ T: git
git://git.kernel.org/pub/scm/linux/kernel/git/jj/apparmor-dev.git
S: Supported
F: security/apparmor/
+LOADPIN SECURITY MODULE
+M: Kees Cook
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git
lsm/loadpin
+S: Supported
+F: security/loadpin/
+
YAMA SECURITY MODULE
M: Kees Cook
T: git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git
yama/tip
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index cdee11cbcdf1..f3402aab1927 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1893,5 +1893,10 @@ extern void __init yama_add_hooks(void);
#else
static inline void __init yama_add_hooks(void) { }
#endif
+#ifdef CONFIG_SECURITY_LOADPIN
+void __init loadpin_add_hooks(void);
+#else
+static inline void loadpin_add_hooks(void) { };
+#endif
#endif /* ! __LINUX_LSM_HOOKS_H */
diff --git a/security/Kconfig b/security/Kconfig
index e45237897b43..176758cdfa57 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -122,6 +122,7 @@ source security/selinux/Kconfig
source security/smack/Kconfig
source security/tomoyo/Kconfig
source security/apparmor/Kconfig
+source security/loadpin/Kconfig
source security/yama/Kconfig
source security/integrity/Kconfig
diff --git a/security/Makefile b/security/Makefile
index c9bfbc84ff50..f2d71cdb8e19 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -8,6 +8,7 @@ subdir-$(CONFIG_SECURITY_SMACK) += smack
subdir-$(CONFIG_SECURITY_TOMOYO)+= tomoyo
subdir-$(CONFIG_SECURITY_APPARMOR) += apparmor
subdir-$(CONFIG_SECURITY_YAMA) += yama
+subdir-$(CONFIG_SECURITY_LOADPIN) += loadpin
# always enable default capabilities
obj-y += commoncap.o
@@ -22,6 +23,7 @@ obj-$(CONFIG_AUDIT) += lsm_audit.o
obj-$(CONFIG_SECURITY_TOMOYO) += tomoyo/
obj-$(CONFIG_SECURITY_APPARMOR)+= apparmor/
obj-$(CONFIG_SECURITY_YAMA)+= yama/
+obj-$(CONFIG_SECURITY_LOADPIN) += loadpin/
obj-$(CONFIG_CGROUP_DEVICE)+= device_cgroup.o
# Object integrity file lists
diff --git a/security/loadpin/Kconfig b/security/loadpin/Kconfig
new file mode 100644
index ..c668ac4eda65
--- /dev/null
+++ b/security/loadpin/Kconfig
@@ -0,0 +1,10 @@
+config SECURITY_LOADPIN
+ bool "Pin load of kernel files (modules, fw, etc) to one filesystem"
+ depends on SECURITY && BLOCK
+ help
+ Any files read through the kernel file reading interface
+
[PATCH v4 3/6] string_helpers: add kstrdup_quotable_file
Allocate a NULL-terminated file path with special characters escaped,
safe for logging.
Signed-off-by: Kees Cook
---
include/linux/string_helpers.h | 3 +++
lib/string_helpers.c | 30 ++
2 files changed, 33 insertions(+)
diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
index 684d2695fc36..5ce9538f290e 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -3,6 +3,8 @@
#include
+struct file;
+
/* Descriptions of the types of units to
* print in */
enum string_size_units {
@@ -70,5 +72,6 @@ static inline int string_escape_str_any_np(const char *src,
char *dst,
char *kstrdup_quotable(const char *src, gfp_t gfp);
char *kstrdup_quotable_cmdline(struct task_struct *task, gfp_t gfp);
+char *kstrdup_quotable_file(struct file *file, gfp_t gfp);
#endif
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index b16ee85aaf87..ecaac2c0526f 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -10,6 +10,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -596,3 +598,31 @@ char *kstrdup_quotable_cmdline(struct task_struct *task,
gfp_t gfp)
return quoted;
}
EXPORT_SYMBOL_GPL(kstrdup_quotable_cmdline);
+
+/*
+ * Returns allocated NULL-terminated string containing pathname,
+ * with special characters escaped, able to be safely logged. If
+ * there is an error, the leading character will be "<".
+ */
+char *kstrdup_quotable_file(struct file *file, gfp_t gfp)
+{
+ char *temp, *pathname;
+
+ if (!file)
+ return kstrdup("", gfp);
+
+ /* We add 11 spaces for ' (deleted)' to be appended */
+ temp = kmalloc(PATH_MAX + 11, GFP_TEMPORARY);
+ if (!temp)
+ return kstrdup("", gfp);
+
+ pathname = file_path(file, temp, PATH_MAX + 11);
+ if (IS_ERR(pathname))
+ pathname = kstrdup("", gfp);
+ else
+ pathname = kstrdup_quotable(pathname, gfp);
+
+ kfree(temp);
+ return pathname;
+}
+EXPORT_SYMBOL_GPL(kstrdup_quotable_file);
--
2.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 5/6] fs: provide function to report enum strings
Providing human-readable (and audit-parsable) strings for the READING_*
enums is needed by some LSMs.
Signed-off-by: Kees Cook
---
fs/exec.c | 19 +++
include/linux/fs.h | 1 +
2 files changed, 20 insertions(+)
diff --git a/fs/exec.c b/fs/exec.c
index c4010b8207a1..05e71b6c0ef0 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -819,6 +819,25 @@ struct file *open_exec(const char *name)
}
EXPORT_SYMBOL(open_exec);
+const char *kernel_read_file_id_str(enum kernel_read_file_id id)
+{
+ switch (id) {
+ case READING_FIRMWARE:
+ return "firmware";
+ case READING_MODULE:
+ return "kernel-module";
+ case READING_KEXEC_IMAGE:
+ return "kexec-image";
+ case READING_KEXEC_INITRAMFS:
+ return "kexec-initramfs";
+ case READING_POLICY:
+ return "security-policy";
+ default:
+ return "unknown";
+ }
+}
+EXPORT_SYMBOL(kernel_read_file_id_str);
+
int kernel_read(struct file *file, loff_t offset,
char *addr, unsigned long count)
{
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 304991a80e23..596b403d5a28 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2589,6 +2589,7 @@ enum kernel_read_file_id {
READING_MAX_ID
};
+extern const char *kernel_read_file_id_str(enum kernel_read_file_id id);
extern int kernel_read(struct file *, loff_t, char *, unsigned long);
extern int kernel_read_file(struct file *, void **, loff_t *, loff_t,
enum kernel_read_file_id);
--
2.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 4/6] Yama: consolidate error reporting
Use a common error reporting function for Yama violation reports, and give
more detail into the process command lines.
Signed-off-by: Kees Cook
---
security/yama/yama_lsm.c | 31 +--
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
index cb6ed10816d4..c19f6e5df9a3 100644
--- a/security/yama/yama_lsm.c
+++ b/security/yama/yama_lsm.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#define YAMA_SCOPE_DISABLED0
#define YAMA_SCOPE_RELATIONAL 1
@@ -41,6 +42,22 @@ static DEFINE_SPINLOCK(ptracer_relations_lock);
static void yama_relation_cleanup(struct work_struct *work);
static DECLARE_WORK(yama_relation_work, yama_relation_cleanup);
+static void report_access(const char *access, struct task_struct *target,
+ struct task_struct *agent)
+{
+ char *target_cmd, *agent_cmd;
+
+ target_cmd = kstrdup_quotable_cmdline(target, GFP_KERNEL);
+ agent_cmd = kstrdup_quotable_cmdline(agent, GFP_KERNEL);
+
+ pr_notice_ratelimited(
+ "ptrace %s of \"%s\"[%d] was attempted by \"%s\"[%d]\n",
+ access, target_cmd, target->pid, agent_cmd, agent->pid);
+
+ kfree(agent_cmd);
+ kfree(target_cmd);
+}
+
/**
* yama_relation_cleanup - remove invalid entries from the relation list
*
@@ -307,11 +324,8 @@ static int yama_ptrace_access_check(struct task_struct
*child,
}
}
- if (rc && (mode & PTRACE_MODE_NOAUDIT) == 0) {
- printk_ratelimited(KERN_NOTICE
- "ptrace of pid %d was attempted by: %s (pid %d)\n",
- child->pid, current->comm, current->pid);
- }
+ if (rc && (mode & PTRACE_MODE_NOAUDIT) == 0)
+ report_access("attach", child, current);
return rc;
}
@@ -337,11 +351,8 @@ int yama_ptrace_traceme(struct task_struct *parent)
break;
}
- if (rc) {
- printk_ratelimited(KERN_NOTICE
- "ptraceme of pid %d was attempted by: %s (pid %d)\n",
- current->pid, parent->comm, parent->pid);
- }
+ if (rc)
+ report_access("traceme", current, parent);
return rc;
}
--
2.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 0/6] LSM: LoadPin for kernel file loading restrictions
This provides the mini-LSM "loadpin" that intercepts the now consolidated kernel_file_read LSM hook so that a system can keep all loads coming from a single trusted filesystem. This is what Chrome OS uses to pin kernel module and firmware loading to the read-only crypto-verified dm-verity partition so that kernel module signing is not needed. -Kees v4: - add missing "const" to char * src, joe v3: - changed module parameter to "loadpin.enabled" - add sysctl docs, akpm - add general use function for enum, zohar - add gfp_t, joe - clean up loops, andriy.shevchenko - reduce BUG_ON to WARN_ON, joe v2: - break out utility helpers into separate functions - have Yama use new helpers too -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] doc: usb: Fix typo in gadget_multi documentation
It tries to "match" drivers for each interface (not "much"). Signed-off-by: Diego Herranz --- Documentation/usb/gadget_multi.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/usb/gadget_multi.txt b/Documentation/usb/gadget_multi.txt index 7d66a86..5faf514 100644 --- a/Documentation/usb/gadget_multi.txt +++ b/Documentation/usb/gadget_multi.txt @@ -43,7 +43,7 @@ For the gadget two work under Windows two conditions have to be met: First of all, Windows need to detect the gadget as an USB composite gadget which on its own have some conditions[4]. If they are met, Windows lets USB Generic Parent Driver[5] handle the device which then -tries to much drivers for each individual interface (sort of, don't +tries to match drivers for each individual interface (sort of, don't get into too many details). The good news is: you do not have to worry about most of the -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Prefer kASLR over Hibernation
On 11/04/16 19:03, Kees Cook wrote: > On Mon, Apr 11, 2016 at 1:00 AM, James Morse wrote: >> On 06/04/16 20:44, Kees Cook wrote: >>> When building with both CONFIG_HIBERNATION and CONFIG_RANDOMIZE_BASE, >>> one or the other must be chosen at boot-time. Until now, hibernation >>> was selected when no choice was made on the command line. >>> >>> To make the security benefits of kASLR more widely available to end >>> users (since the use of hibernation is becoming more rare and kASLR, >>> already available on x86, will be available on arm64 and MIPS soon), >>> this changes the default to preferring kASLR over hibernation. Users >>> wanting hibernation can turn off kASLR by adding "nokaslr" to the kernel >>> command line. >> >> While hibernate isn't yet merged for arm64, it does work with kASLR in >> v4.6-rc*, >> it would be a shame to have to choose at boot time, (but that's my problem to >> fix if/when its merged). > > Ah, interesting, so they work together on arm64? (i.e. you've actually > tested a boot loader that provides the seed for kASLR to operate?) Almost: due to a lack of firmware support I hacked the efi stub to read a 'seed' from a system counter. To check it works I printed the address of 'panic' out during boot: > [0.353712] DEBUG: &panic == ff960819a4e8 Then hibernated, and powered the board back on, the resume kernel gives: > [0.353528] DEBUG: &panic == ff840819a4e8 But after it has restored the hibernate image, I can dig in /proc/kallsyms to see the original value: > root@localhost:~# cat /proc/kallsyms | grep "T panic" > ff960819a4e8 T panic > Maybe RANDOMIZE_BASE_DEFAULT (default to y) and if x86 && hibernation, > select =n. and use that for selecting it? Looks good to me. Thanks, James -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 04/13] task_isolation: add initial support
On 4/8/2016 12:34 PM, Chris Metcalf wrote: However, this makes me wonder if "strict" mode should be the default for task isolation?? That way task isolation really doesn't conflict semantically with migration. And we could provide a "weak" mode, or a "kernel-friendly" mode, or some such nomenclature, and define the migration semantics just for that case, where it makes it clear it's a bit unusual. I noodled around with this and decided it was a better default, so I've made the changes and pushed it up to the branch: git://git.kernel.org/pub/scm/linux/kernel/git/cmetcalf/linux-tile.git dataplane Now, by default when you enter task isolation mode, you are in what I used to call "strict" mode, i.e. you can't use the kernel. You can select a user-specified signal you want to deliver instead of the default SIGKILL, and if you select signal 0, then you don't get a signal at all and instead you get to keep running in task isolation mode after making a syscall, page fault, etc. Thus the API now looks like this in : #define PR_SET_TASK_ISOLATION 48 #define PR_GET_TASK_ISOLATION 49 # define PR_TASK_ISOLATION_ENABLE (1 << 0) # define PR_TASK_ISOLATION_USERSIG (1 << 1) # define PR_TASK_ISOLATION_SET_SIG(sig) (((sig) & 0x7f) << 8) # define PR_TASK_ISOLATION_GET_SIG(bits) (((bits) >> 8) & 0x7f) # define PR_TASK_ISOLATION_NOSIG \ (PR_TASK_ISOLATION_USERSIG | PR_TASK_ISOLATION_SET_SIG(0)) I think this better matches what people should want to do in their applications, and also matches the expectations people have about what it means to go into task isolation mode in the first place. I got rid of the ONESHOT mode that I added in the v12 series, since it didn't seem like it was what Frederic had been asking for anyway, and it didn't seem particularly useful on its own. Frederic, how does this align with your intuition for this stuff? -- Chris Metcalf, Mellanox Technologies http://www.mellanox.com -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/6] string_helpers: add kstrdup_quotable
Quoting Kees Cook (keesc...@chromium.org):
> Handle allocating and escaping a string safe for logging.
>
> Signed-off-by: Kees Cook
Acked-by: Serge Hallyn
> ---
> include/linux/string_helpers.h | 2 ++
> lib/string_helpers.c | 28
> 2 files changed, 30 insertions(+)
>
> diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
> index dabe643eb5fa..9de228af00c1 100644
> --- a/include/linux/string_helpers.h
> +++ b/include/linux/string_helpers.h
> @@ -68,4 +68,6 @@ static inline int string_escape_str_any_np(const char *src,
> char *dst,
> return string_escape_str(src, dst, sz, ESCAPE_ANY_NP, only);
> }
>
> +char *kstrdup_quotable(const char *src, gfp_t gfp);
> +
> #endif
> diff --git a/lib/string_helpers.c b/lib/string_helpers.c
> index 5c88204b6f1f..aa00c9f989ee 100644
> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c
> @@ -10,6 +10,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -534,3 +535,30 @@ int string_escape_mem(const char *src, size_t isz, char
> *dst, size_t osz,
> return p - dst;
> }
> EXPORT_SYMBOL(string_escape_mem);
> +
> +/*
> + * Return an allocated string that has been escaped of special characters
> + * and double quotes, making it safe to log in quotes.
> + */
> +char *kstrdup_quotable(const char *src, gfp_t gfp)
> +{
> + size_t slen, dlen;
> + char *dst;
> + const int flags = ESCAPE_HEX;
> + const char esc[] = "\f\n\r\t\v\a\e\\\"";
> +
> + if (!src)
> + return NULL;
> + slen = strlen(src);
> +
> + dlen = string_escape_mem(src, slen, NULL, 0, flags, esc);
> + dst = kmalloc(dlen + 1, gfp);
> + if (!dst)
> + return NULL;
> +
> + WARN_ON(string_escape_mem(src, slen, dst, dlen, flags, esc) != dlen);
> + dst[dlen] = '\0';
> +
> + return dst;
> +}
> +EXPORT_SYMBOL_GPL(kstrdup_quotable);
> --
> 2.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/6] string_helpers: add kstrdup_quotable_cmdline
Quoting Kees Cook (keesc...@chromium.org):
> Provide an escaped (but readable: no inter-argument NULLs) commandline
> safe for logging.
>
> Signed-off-by: Kees Cook
Acked-by: Serge Hallyn
> ---
> include/linux/string_helpers.h | 1 +
> lib/string_helpers.c | 34 ++
> 2 files changed, 35 insertions(+)
>
> diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
> index 9de228af00c1..684d2695fc36 100644
> --- a/include/linux/string_helpers.h
> +++ b/include/linux/string_helpers.h
> @@ -69,5 +69,6 @@ static inline int string_escape_str_any_np(const char *src,
> char *dst,
> }
>
> char *kstrdup_quotable(const char *src, gfp_t gfp);
> +char *kstrdup_quotable_cmdline(struct task_struct *task, gfp_t gfp);
>
> #endif
> diff --git a/lib/string_helpers.c b/lib/string_helpers.c
> index aa00c9f989ee..b16ee85aaf87 100644
> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c
> @@ -10,6 +10,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -562,3 +563,36 @@ char *kstrdup_quotable(const char *src, gfp_t gfp)
> return dst;
> }
> EXPORT_SYMBOL_GPL(kstrdup_quotable);
> +
> +/*
> + * Returns allocated NULL-terminated string containing process
> + * command line, with inter-argument NULLs replaced with spaces,
> + * and other special characters escaped.
> + */
> +char *kstrdup_quotable_cmdline(struct task_struct *task, gfp_t gfp)
> +{
> + char *buffer, *quoted;
> + int i, res;
> +
> + buffer = kmalloc(PAGE_SIZE, GFP_TEMPORARY);
> + if (!buffer)
> + return NULL;
> +
> + res = get_cmdline(task, buffer, PAGE_SIZE - 1);
> + buffer[res] = '\0';
> +
> + /* Collapse trailing NULLs, leave res pointing to last non-NULL. */
> + while (--res >= 0 && buffer[res] == '\0')
> + ;
> +
> + /* Replace inter-argument NULLs. */
> + for (i = 0; i <= res; i++)
> + if (buffer[i] == '\0')
> + buffer[i] = ' ';
> +
> + /* Make sure result is printable. */
> + quoted = kstrdup_quotable(buffer, gfp);
> + kfree(buffer);
> + return quoted;
> +}
> +EXPORT_SYMBOL_GPL(kstrdup_quotable_cmdline);
> --
> 2.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/6] string_helpers: add kstrdup_quotable_file
Quoting Kees Cook (keesc...@chromium.org):
> Allocate a NULL-terminated file path with special characters escaped,
> safe for logging.
>
> Signed-off-by: Kees Cook
Acked-by: Serge Hallyn
> ---
> include/linux/string_helpers.h | 3 +++
> lib/string_helpers.c | 30 ++
> 2 files changed, 33 insertions(+)
>
> diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
> index 684d2695fc36..5ce9538f290e 100644
> --- a/include/linux/string_helpers.h
> +++ b/include/linux/string_helpers.h
> @@ -3,6 +3,8 @@
>
> #include
>
> +struct file;
> +
> /* Descriptions of the types of units to
> * print in */
> enum string_size_units {
> @@ -70,5 +72,6 @@ static inline int string_escape_str_any_np(const char *src,
> char *dst,
>
> char *kstrdup_quotable(const char *src, gfp_t gfp);
> char *kstrdup_quotable_cmdline(struct task_struct *task, gfp_t gfp);
> +char *kstrdup_quotable_file(struct file *file, gfp_t gfp);
>
> #endif
> diff --git a/lib/string_helpers.c b/lib/string_helpers.c
> index b16ee85aaf87..ecaac2c0526f 100644
> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c
> @@ -10,6 +10,8 @@
> #include
> #include
> #include
> +#include
> +#include
> #include
> #include
> #include
> @@ -596,3 +598,31 @@ char *kstrdup_quotable_cmdline(struct task_struct *task,
> gfp_t gfp)
> return quoted;
> }
> EXPORT_SYMBOL_GPL(kstrdup_quotable_cmdline);
> +
> +/*
> + * Returns allocated NULL-terminated string containing pathname,
> + * with special characters escaped, able to be safely logged. If
> + * there is an error, the leading character will be "<".
> + */
> +char *kstrdup_quotable_file(struct file *file, gfp_t gfp)
> +{
> + char *temp, *pathname;
> +
> + if (!file)
> + return kstrdup("", gfp);
> +
> + /* We add 11 spaces for ' (deleted)' to be appended */
> + temp = kmalloc(PATH_MAX + 11, GFP_TEMPORARY);
> + if (!temp)
> + return kstrdup("", gfp);
> +
> + pathname = file_path(file, temp, PATH_MAX + 11);
> + if (IS_ERR(pathname))
> + pathname = kstrdup("", gfp);
> + else
> + pathname = kstrdup_quotable(pathname, gfp);
> +
> + kfree(temp);
> + return pathname;
> +}
> +EXPORT_SYMBOL_GPL(kstrdup_quotable_file);
> --
> 2.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/6] Yama: consolidate error reporting
Quoting Kees Cook (keesc...@chromium.org):
> Use a common error reporting function for Yama violation reports, and give
> more detail into the process command lines.
>
> Signed-off-by: Kees Cook
Acked-by: Serge Hallyn
> ---
> security/yama/yama_lsm.c | 31 +--
> 1 file changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
> index cb6ed10816d4..c19f6e5df9a3 100644
> --- a/security/yama/yama_lsm.c
> +++ b/security/yama/yama_lsm.c
> @@ -18,6 +18,7 @@
> #include
> #include
> #include
> +#include
>
> #define YAMA_SCOPE_DISABLED 0
> #define YAMA_SCOPE_RELATIONAL1
> @@ -41,6 +42,22 @@ static DEFINE_SPINLOCK(ptracer_relations_lock);
> static void yama_relation_cleanup(struct work_struct *work);
> static DECLARE_WORK(yama_relation_work, yama_relation_cleanup);
>
> +static void report_access(const char *access, struct task_struct *target,
> + struct task_struct *agent)
> +{
> + char *target_cmd, *agent_cmd;
> +
> + target_cmd = kstrdup_quotable_cmdline(target, GFP_KERNEL);
> + agent_cmd = kstrdup_quotable_cmdline(agent, GFP_KERNEL);
> +
> + pr_notice_ratelimited(
> + "ptrace %s of \"%s\"[%d] was attempted by \"%s\"[%d]\n",
> + access, target_cmd, target->pid, agent_cmd, agent->pid);
> +
> + kfree(agent_cmd);
> + kfree(target_cmd);
> +}
> +
> /**
> * yama_relation_cleanup - remove invalid entries from the relation list
> *
> @@ -307,11 +324,8 @@ static int yama_ptrace_access_check(struct task_struct
> *child,
> }
> }
>
> - if (rc && (mode & PTRACE_MODE_NOAUDIT) == 0) {
> - printk_ratelimited(KERN_NOTICE
> - "ptrace of pid %d was attempted by: %s (pid %d)\n",
> - child->pid, current->comm, current->pid);
> - }
> + if (rc && (mode & PTRACE_MODE_NOAUDIT) == 0)
> + report_access("attach", child, current);
>
> return rc;
> }
> @@ -337,11 +351,8 @@ int yama_ptrace_traceme(struct task_struct *parent)
> break;
> }
>
> - if (rc) {
> - printk_ratelimited(KERN_NOTICE
> - "ptraceme of pid %d was attempted by: %s (pid %d)\n",
> - current->pid, parent->comm, parent->pid);
> - }
> + if (rc)
> + report_access("traceme", current, parent);
>
> return rc;
> }
> --
> 2.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 5/6] fs: provide function to report enum strings
Quoting Kees Cook (keesc...@chromium.org):
> Providing human-readable (and audit-parsable) strings for the READING_*
> enums is needed by some LSMs.
>
> Signed-off-by: Kees Cook
Acked-by: Serge Hallyn
> ---
> fs/exec.c | 19 +++
> include/linux/fs.h | 1 +
> 2 files changed, 20 insertions(+)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index c4010b8207a1..05e71b6c0ef0 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -819,6 +819,25 @@ struct file *open_exec(const char *name)
> }
> EXPORT_SYMBOL(open_exec);
>
> +const char *kernel_read_file_id_str(enum kernel_read_file_id id)
> +{
> + switch (id) {
> + case READING_FIRMWARE:
> + return "firmware";
> + case READING_MODULE:
> + return "kernel-module";
> + case READING_KEXEC_IMAGE:
> + return "kexec-image";
> + case READING_KEXEC_INITRAMFS:
> + return "kexec-initramfs";
> + case READING_POLICY:
> + return "security-policy";
> + default:
> + return "unknown";
> + }
> +}
> +EXPORT_SYMBOL(kernel_read_file_id_str);
> +
> int kernel_read(struct file *file, loff_t offset,
> char *addr, unsigned long count)
> {
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 304991a80e23..596b403d5a28 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2589,6 +2589,7 @@ enum kernel_read_file_id {
> READING_MAX_ID
> };
>
> +extern const char *kernel_read_file_id_str(enum kernel_read_file_id id);
> extern int kernel_read(struct file *, loff_t, char *, unsigned long);
> extern int kernel_read_file(struct file *, void **, loff_t *, loff_t,
> enum kernel_read_file_id);
> --
> 2.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 6/6] LSM: LoadPin for kernel file loading restrictions
Quoting Kees Cook (keesc...@chromium.org):
> This LSM enforces that kernel-loaded files (modules, firmware, etc)
> must all come from the same filesystem, with the expectation that
> such a filesystem is backed by a read-only device such as dm-verity
> or CDROM. This allows systems that have a verified and/or unchangeable
> filesystem to enforce module and firmware loading restrictions without
> needing to sign the files individually.
>
> Signed-off-by: Kees Cook
Acked-by: Serge Hallyn
> ---
> Documentation/security/LoadPin.txt | 17
> MAINTAINERS| 6 ++
> include/linux/lsm_hooks.h | 5 +
> security/Kconfig | 1 +
> security/Makefile | 2 +
> security/loadpin/Kconfig | 10 ++
> security/loadpin/Makefile | 1 +
> security/loadpin/loadpin.c | 190
> +
> security/security.c| 1 +
> 9 files changed, 233 insertions(+)
> create mode 100644 Documentation/security/LoadPin.txt
> create mode 100644 security/loadpin/Kconfig
> create mode 100644 security/loadpin/Makefile
> create mode 100644 security/loadpin/loadpin.c
>
> diff --git a/Documentation/security/LoadPin.txt
> b/Documentation/security/LoadPin.txt
> new file mode 100644
> index ..e11877f5d3d4
> --- /dev/null
> +++ b/Documentation/security/LoadPin.txt
> @@ -0,0 +1,17 @@
> +LoadPin is a Linux Security Module that ensures all kernel-loaded files
> +(modules, firmware, etc) all originate from the same filesystem, with
> +the expectation that such a filesystem is backed by a read-only device
> +such as dm-verity or CDROM. This allows systems that have a verified
> +and/or unchangeable filesystem to enforce module and firmware loading
> +restrictions without needing to sign the files individually.
> +
> +The LSM is selectable at build-time with CONFIG_SECURITY_LOADPIN, and
> +can be controlled at boot-time with the kernel command line option
> +"loadpin.enabled". By default, it is enabled, but can be disabled at
> +boot ("loadpin.enabled=0").
> +
> +LoadPin starts pinning when it sees the first file loaded. If the
> +block device backing the filesystem is not read-only, a sysctl is
> +created to toggle pinning: /proc/sys/kernel/loadpin/enabled. (Having
> +a mutable filesystem means pinning is mutable too, but having the
> +sysctl allows for easy testing on systems with a mutable filesystem.)
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 40eb1dbe2ae5..de4cf8e9247e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9964,6 +9964,12 @@ T: git
> git://git.kernel.org/pub/scm/linux/kernel/git/jj/apparmor-dev.git
> S: Supported
> F: security/apparmor/
>
> +LOADPIN SECURITY MODULE
> +M: Kees Cook
> +T: git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git
> lsm/loadpin
> +S: Supported
> +F: security/loadpin/
> +
> YAMA SECURITY MODULE
> M: Kees Cook
> T: git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git
> yama/tip
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index cdee11cbcdf1..f3402aab1927 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1893,5 +1893,10 @@ extern void __init yama_add_hooks(void);
> #else
> static inline void __init yama_add_hooks(void) { }
> #endif
> +#ifdef CONFIG_SECURITY_LOADPIN
> +void __init loadpin_add_hooks(void);
> +#else
> +static inline void loadpin_add_hooks(void) { };
> +#endif
>
> #endif /* ! __LINUX_LSM_HOOKS_H */
> diff --git a/security/Kconfig b/security/Kconfig
> index e45237897b43..176758cdfa57 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -122,6 +122,7 @@ source security/selinux/Kconfig
> source security/smack/Kconfig
> source security/tomoyo/Kconfig
> source security/apparmor/Kconfig
> +source security/loadpin/Kconfig
> source security/yama/Kconfig
>
> source security/integrity/Kconfig
> diff --git a/security/Makefile b/security/Makefile
> index c9bfbc84ff50..f2d71cdb8e19 100644
> --- a/security/Makefile
> +++ b/security/Makefile
> @@ -8,6 +8,7 @@ subdir-$(CONFIG_SECURITY_SMACK) += smack
> subdir-$(CONFIG_SECURITY_TOMOYO)+= tomoyo
> subdir-$(CONFIG_SECURITY_APPARMOR) += apparmor
> subdir-$(CONFIG_SECURITY_YAMA) += yama
> +subdir-$(CONFIG_SECURITY_LOADPIN)+= loadpin
>
> # always enable default capabilities
> obj-y+= commoncap.o
> @@ -22,6 +23,7 @@ obj-$(CONFIG_AUDIT) += lsm_audit.o
> obj-$(CONFIG_SECURITY_TOMOYO)+= tomoyo/
> obj-$(CONFIG_SECURITY_APPARMOR) += apparmor/
> obj-$(CONFIG_SECURITY_YAMA) += yama/
> +obj-$(CONFIG_SECURITY_LOADPIN) += loadpin/
> obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o
>
> # Object integrity file lists
> diff --git a/security/loadpin/Kconfig b/security/loadpin/Kconfig
> new file mode 100644
> index 00
[PATCH] Add EDAC peripheral init functions & Ethernet EDAC.
This patch set adds the memory initialization functions for Altera's Arria10 peripherals, the first of which is the Ethernet EDAC. The first 3 patches add the memory initialization functionality. The last 3 patches add Ethernet EDAC support. [PATCH 1/6] EDAC, altera: Check parent status for Arria10 EDAC block [PATCH 2/6] EDAC, altera: Move IRQ function declaration [PATCH 3/6] EDAC, altera: Add Arria10 ECC memory init functions [PATCH 4/6] Documentation: dt: socfpga: Add Arria10 Ethernet binding [PATCH 5/6] EDAC, altera: Add Arria10 Ethernet EDAC support [PATCH 6/6] ARM: dts: Add Arria10 Ethernet EDAC devicetree entry -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] EDAC, altera: Check parent status for Arria10 EDAC block
From: Thor Thayer
In preparation for the Arria10 ECC modules, check the status
of the parent in the device tree to ensure the block is enabled.
Skip if no parent phandle is set in the device tree.
Signed-off-by: Thor Thayer
---
drivers/edac/altera_edac.c |9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 11775dc..c3e040d 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -1115,6 +1115,7 @@ static int altr_edac_a10_device_add(struct
altr_arria10_edac *edac,
struct resource res;
int edac_idx;
int rc = 0;
+ struct device_node *parent;
const struct edac_device_prv_data *prv;
/* Get matching node and check for valid result */
const struct of_device_id *pdev_id =
@@ -1127,6 +1128,14 @@ static int altr_edac_a10_device_add(struct
altr_arria10_edac *edac,
if (IS_ERR_OR_NULL(prv))
return -ENODEV;
+ /* If there is a parent parameter, exit if it is not available. */
+ parent = of_parse_phandle(np, "parent", 0);
+ if (parent && !of_device_is_available(parent)) {
+ of_node_put(parent);
+ return -ENODEV;
+ }
+ of_node_put(parent);
+
if (!devres_open_group(edac->dev, altr_edac_a10_device_add, GFP_KERNEL))
return -ENOMEM;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] EDAC, altera: Add Arria10 Ethernet EDAC support
From: Thor Thayer
Add Altera Arria10 Ethernet FIFO memory EDAC support.
Signed-off-by: Thor Thayer
---
drivers/edac/Kconfig |7 ++
drivers/edac/altera_edac.c | 153
drivers/edac/altera_edac.h | 14
3 files changed, 174 insertions(+)
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 6ca7474..d0c1dab 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -391,6 +391,13 @@ config EDAC_ALTERA_OCRAM
Support for error detection and correction on the
Altera On-Chip RAM Memory for Altera SoCs.
+config EDAC_ALTERA_ETHERNET
+ bool "Altera Ethernet FIFO ECC"
+ depends on EDAC_ALTERA=y
+ help
+ Support for error detection and correction on the
+ Altera Ethernet FIFO Memory for Altera SoCs.
+
config EDAC_SYNOPSYS
tristate "Synopsys DDR Memory Controller"
depends on EDAC_MM_EDAC && ARCH_ZYNQ
diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 0955ab0..43b6f36 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -553,6 +553,12 @@ const struct edac_device_prv_data ocramecc_data;
const struct edac_device_prv_data l2ecc_data;
const struct edac_device_prv_data a10_ocramecc_data;
const struct edac_device_prv_data a10_l2ecc_data;
+const struct edac_device_prv_data a10_enet0rxecc_data;
+const struct edac_device_prv_data a10_enet0txecc_data;
+const struct edac_device_prv_data a10_enet1rxecc_data;
+const struct edac_device_prv_data a10_enet1txecc_data;
+const struct edac_device_prv_data a10_enet2rxecc_data;
+const struct edac_device_prv_data a10_enet2txecc_data;
static irqreturn_t altr_edac_device_handler(int irq, void *dev_id)
{
@@ -716,6 +722,20 @@ static const struct of_device_id
altr_edac_device_of_match[] = {
{ .compatible = "altr,socfpga-a10-ocram-ecc",
.data = (void *)&a10_ocramecc_data },
#endif
+#ifdef CONFIG_EDAC_ALTERA_ETHERNET
+ { .compatible = "altr,socfpga-a10-emac0-rx-ecc",
+ .data = (void *)&a10_enet0rxecc_data },
+ { .compatible = "altr,socfpga-a10-emac0-tx-ecc",
+ .data = (void *)&a10_enet0txecc_data },
+ { .compatible = "altr,socfpga-a10-emac1-rx-ecc",
+ .data = (void *)&a10_enet1rxecc_data },
+ { .compatible = "altr,socfpga-a10-emac1-tx-ecc",
+ .data = (void *)&a10_enet1txecc_data },
+ { .compatible = "altr,socfpga-a10-emac2-rx-ecc",
+ .data = (void *)&a10_enet2rxecc_data },
+ { .compatible = "altr,socfpga-a10-emac2-tx-ecc",
+ .data = (void *)&a10_enet2txecc_data },
+#endif
{},
};
MODULE_DEVICE_TABLE(of, altr_edac_device_of_match);
@@ -1033,6 +1053,126 @@ const struct edac_device_prv_data a10_l2ecc_data = {
#endif /* CONFIG_EDAC_ALTERA_L2C */
+/* Ethernet Device Functions /
+
+#ifdef CONFIG_EDAC_ALTERA_ETHERNET
+
+const struct edac_device_prv_data a10_enet0rxecc_data = {
+ .setup = altr_check_ecc_deps,
+ .ce_clear_mask = ALTR_A10_ECC_SERRPENA,
+ .ue_clear_mask = ALTR_A10_ECC_DERRPENA,
+ .irq_status_mask = A10_SYSMGR_ECC_INTSTAT_EMAC0RX,
+ .dbgfs_name = "altr_emac0rx_trigger",
+ .ecc_enable_mask = ALTR_A10_ETHERNET_ECC_EN_CTL,
+ .ecc_en_ofst = ALTR_A10_ECC_CTRL_OFST,
+ .ce_set_mask = ALTR_A10_ECC_TSERRA,
+ .ue_set_mask = ALTR_A10_ECC_TDERRA,
+ .set_err_ofst = ALTR_A10_ECC_INTTEST_OFST,
+ .ecc_irq_handler = altr_edac_a10_ecc_irq,
+ .inject_fops = &altr_edac_a10_device_inject_fops,
+};
+
+const struct edac_device_prv_data a10_enet0txecc_data = {
+ .setup = altr_check_ecc_deps,
+ .ce_clear_mask = ALTR_A10_ECC_SERRPENA,
+ .ue_clear_mask = ALTR_A10_ECC_DERRPENA,
+ .irq_status_mask = A10_SYSMGR_ECC_INTSTAT_EMAC0TX,
+ .dbgfs_name = "altr_emac0tx_trigger",
+ .ecc_enable_mask = ALTR_A10_ETHERNET_ECC_EN_CTL,
+ .ecc_en_ofst = ALTR_A10_ECC_CTRL_OFST,
+ .ce_set_mask = ALTR_A10_ECC_TSERRA,
+ .ue_set_mask = ALTR_A10_ECC_TDERRA,
+ .set_err_ofst = ALTR_A10_ECC_INTTEST_OFST,
+ .ecc_irq_handler = altr_edac_a10_ecc_irq,
+ .inject_fops = &altr_edac_a10_device_inject_fops,
+};
+
+const struct edac_device_prv_data a10_enet1rxecc_data = {
+ .setup = altr_check_ecc_deps,
+ .ce_clear_mask = ALTR_A10_ECC_SERRPENA,
+ .ue_clear_mask = ALTR_A10_ECC_DERRPENA,
+ .irq_status_mask = A10_SYSMGR_ECC_INTSTAT_EMAC1RX,
+ .dbgfs_name = "altr_emac1rx_trigger",
+ .ecc_enable_mask = ALTR_A10_ETHERNET_ECC_EN_CTL,
+ .ecc_en_ofst = ALTR_A10_ECC_CTRL_OFST,
+ .ce_set_mask = ALTR_A10_ECC_TSERRA,
+ .ue_set_mask = ALTR_A10_ECC_TDERRA,
+ .set_err_ofst = ALTR_A10_ECC_INTTEST_OFST,
+ .ecc_irq_handler = altr_edac_a10_ecc_irq,
+ .inject_fops = &altr_edac_a10_device_inject_fops,
+};
+
+const struct edac_device_prv_data a10_enet1txecc_data = {
+ .setup = altr_check_ecc_deps,
+
[PATCH 6/6] ARM: dts: Add Arria10 Ethernet EDAC devicetree entry
From: Thor Thayer
Add the device tree entries needed to support the Altera Ethernet
FIFO buffer EDAC on the Arria10 chip.
Signed-off-by: Thor Thayer
---
arch/arm/boot/dts/socfpga_arria10.dtsi | 36
1 file changed, 36 insertions(+)
diff --git a/arch/arm/boot/dts/socfpga_arria10.dtsi
b/arch/arm/boot/dts/socfpga_arria10.dtsi
index 27cc497..6195ade 100644
--- a/arch/arm/boot/dts/socfpga_arria10.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria10.dtsi
@@ -617,6 +617,42 @@
compatible = "altr,socfpga-a10-ocram-ecc";
reg = <0xff8c3000 0x400>;
};
+
+ emac0-rx-ecc@ff8c0800 {
+ compatible = "altr,socfpga-a10-emac0-rx-ecc";
+ reg = <0xff8c0800 0x400>;
+ parent = <&gmac0>;
+ };
+
+ emac0-tx-ecc@ff8c0c00 {
+ compatible = "altr,socfpga-a10-emac0-tx-ecc";
+ reg = <0xff8c0c00 0x400>;
+ parent = <&gmac0>;
+ };
+
+ emac1-rx-ecc@ff8c1000 {
+ compatible = "altr,socfpga-a10-emac1-rx-ecc";
+ reg = <0xff8c1000 0x400>;
+ parent = <&gmac1>;
+ };
+
+ emac1-tx-ecc@ff8c1400 {
+ compatible = "altr,socfpga-a10-emac1-tx-ecc";
+ reg = <0xff8c1400 0x400>;
+ parent = <&gmac1>;
+ };
+
+ emac2-rx-ecc@ff8c1800 {
+ compatible = "altr,socfpga-a10-emac2-rx-ecc";
+ reg = <0xff8c1800 0x400>;
+ parent = <&gmac2>;
+ };
+
+ emac2-tx-ecc@ff8c1c00 {
+ compatible = "altr,socfpga-a10-emac2-tx-ecc";
+ reg = <0xff8c1c00 0x400>;
+ parent = <&gmac2>;
+ };
};
rst: rstmgr@ffd05000 {
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] Documentation: dt: socfpga: Add Arria10 Ethernet binding
From: Thor Thayer
Add the device tree bindings needed to support the Altera Ethernet
FIFO buffers on the Arria10 chip.
Signed-off-by: Thor Thayer
---
.../bindings/arm/altera/socfpga-eccmgr.txt | 24
1 file changed, 24 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt
b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt
index 5a6b160..aa1c593 100644
--- a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt
+++ b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt
@@ -76,6 +76,18 @@ Required Properties:
- compatible : Should be "altr,socfpga-a10-ocram-ecc"
- reg: Address and size for ECC block registers.
+Ethernet FIFO ECC
+Required Properties:
+- compatible : Should be "altr,socfpga-a10-emac0-rx-ecc" for the 1st EMAC
+ Receive buffer
+ or "altr,socfpga-a10-emac0-tx-ecc" for the 1st EMAC Transmit buffer
+ or "altr,socfpga-a10-emac1-rx-ecc" for the 2nd EMAC Receive buffer
+ or "altr,socfpga-a10-emac1-tx-ecc" for the 2nd EMAC Transmit buffer
+ or "altr,socfpga-a10-emac2-rx-ecc" for the 3rd EMAC Receive buffer
+ or "altr,socfpga-a10-emac2-tx-ecc" for the 3rd EMAC Transmit buffer
+- reg: Address and size for ECC block registers.
+- parent : phandle to parent Ethernet node.
+
Example:
eccmgr: eccmgr@ffd06000 {
@@ -96,4 +108,16 @@ Example:
compatible = "altr,socfpga-a10-ocram-ecc";
reg = <0xff8c3000 0x90>;
};
+
+ emac0-rx-ecc@ff8c0800 {
+ compatible = "altr,socfpga-a10-emac0-rx-ecc";
+ reg = <0xff8c0800 0x400>;
+ parent = <&gmac0>;
+ };
+
+ emac0-tx-ecc@ff8c0c00 {
+ compatible = "altr,socfpga-a10-emac0-tx-ecc";
+ reg = <0xff8c0c00 0x400>;
+ parent = <&gmac0>;
+ };
};
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] EDAC, altera: Move IRQ function declaration
From: Thor Thayer
In preparation for additional memory module ECCs, the
IRQ declaration is being made available to everyone.
Move it outside of the OCRAM only area.
Signed-off-by: Thor Thayer
---
drivers/edac/altera_edac.c |7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index c3e040d..226e650 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -848,6 +848,10 @@ static struct platform_driver altr_edac_device_driver = {
};
module_platform_driver(altr_edac_device_driver);
+/* Arria10 Function Declarations */
+static irqreturn_t altr_edac_a10_ecc_irq(struct altr_edac_device_dev *dci,
+bool sberr);
+
/*** OCRAM EDAC Device Functions */
#ifdef CONFIG_EDAC_ALTERA_OCRAM
@@ -902,9 +906,6 @@ const struct edac_device_prv_data ocramecc_data = {
.inject_fops = &altr_edac_device_inject_fops,
};
-static irqreturn_t altr_edac_a10_ecc_irq(struct altr_edac_device_dev *dci,
-bool sberr);
-
const struct edac_device_prv_data a10_ocramecc_data = {
.setup = altr_check_ecc_deps,
.ce_clear_mask = ALTR_A10_ECC_SERRPENA,
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] EDAC, altera: Add Arria10 ECC memory init functions
From: Thor Thayer
In preparation for additional memory module ECCs, add the
memory initialization functions.
Signed-off-by: Thor Thayer
---
drivers/edac/altera_edac.c | 152
drivers/edac/altera_edac.h |3 +
2 files changed, 155 insertions(+)
diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 226e650..0955ab0 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -19,6 +19,7 @@
#include
#include
+#include
#include
#include
#include
@@ -851,6 +852,8 @@ module_platform_driver(altr_edac_device_driver);
/* Arria10 Function Declarations */
static irqreturn_t altr_edac_a10_ecc_irq(struct altr_edac_device_dev *dci,
bool sberr);
+static int altr_init_a10_ecc_block(const char *compat, u32 irq_mask,
+ u32 ecc_ctrl_en_mask, bool dual_port);
/*** OCRAM EDAC Device Functions */
@@ -1039,6 +1042,155 @@ const struct edac_device_prv_data a10_l2ecc_data = {
* Based on xgene_edac.c peripheral code.
*/
+static inline void ecc_set_bits(u32 bit_mask, void __iomem *ioaddr)
+{
+ u32 value = readl(ioaddr);
+
+ value |= bit_mask;
+ writel(value, ioaddr);
+}
+
+static inline void ecc_clear_bits(u32 bit_mask, void __iomem *ioaddr)
+{
+ u32 value = readl(ioaddr);
+
+ value &= ~bit_mask;
+ writel(value, ioaddr);
+}
+
+static inline int ecc_test_bits(u32 bit_mask, void __iomem *ioaddr)
+{
+ u32 value = readl(ioaddr);
+
+ return (value & bit_mask) ? 1 : 0;
+}
+
+/*
+ * This function uses the memory initialization block in the Arria10 ECC
+ * controller to initialize/clear the entire memory data and ECC data.
+ */
+static int altr_init_memory_port(void __iomem *ioaddr, int port)
+{
+ int limit = ALTR_A10_ECC_INIT_WATCHDOG_10US;
+ u32 init_mask = ALTR_A10_ECC_INITA;
+ u32 stat_mask = ALTR_A10_ECC_INITCOMPLETEA;
+ u32 clear_mask = ALTR_A10_ECC_ERRPENA_MASK;
+ int ret = 0;
+
+ if (port) {
+ init_mask = ALTR_A10_ECC_INITB;
+ stat_mask = ALTR_A10_ECC_INITCOMPLETEB;
+ clear_mask = ALTR_A10_ECC_ERRPENB_MASK;
+ }
+
+ ecc_set_bits(init_mask, (ioaddr + ALTR_A10_ECC_CTRL_OFST));
+ while (limit--) {
+ if (ecc_test_bits(stat_mask,
+ (ioaddr + ALTR_A10_ECC_INITSTAT_OFST)))
+ break;
+ udelay(1);
+ }
+ if (limit < 0)
+ ret = -EBUSY;
+
+ /* Clear any pending ECC interrupts */
+ writel(clear_mask, (ioaddr + ALTR_A10_ECC_INTSTAT_OFST));
+
+ return ret;
+}
+
+/*
+ * Aside from the L2 ECC, the Arria10 ECC memories have a common register
+ * layout so the following functions can be shared between all peripherals.
+ */
+static int altr_init_a10_ecc_block(const char *compat, u32 irq_mask,
+ u32 ecc_ctrl_en_mask, bool dual_port)
+{
+ int ret = 0;
+ void __iomem *ecc_block_base;
+ struct regmap *ecc_mgr_map;
+ char *ecc_name;
+ struct device_node *np, *parent, *np_eccmgr;
+
+ np = of_find_compatible_node(NULL, NULL, compat);
+ if (!np) {
+ pr_err("SOCFPGA: Unable to find %s in dtb\n", compat);
+ ret = -ENODEV;
+ goto out;
+ }
+ ecc_name = (char *)np->name;
+
+ /* Ensure device is enabled before calling init, otherwise exit */
+ parent = of_parse_phandle(np, "parent", 0);
+ if (!parent || !of_device_is_available(parent)) {
+ ret = -ENODEV;
+ goto out1;
+ }
+
+ /* Get the ECC Manager - parent of the device EDACs */
+ np_eccmgr = of_get_parent(np);
+ ecc_mgr_map = syscon_regmap_lookup_by_phandle(np_eccmgr,
+ "altr,sysmgr-syscon");
+ of_node_put(np_eccmgr);
+ if (IS_ERR(ecc_mgr_map)) {
+ edac_printk(KERN_ERR, EDAC_DEVICE,
+ "Unable to get syscon altr,sysmgr-syscon\n");
+ ret = -ENODEV;
+ goto out1;
+ }
+
+ /* Map the ECC Block */
+ ecc_block_base = of_iomap(np, 0);
+ if (!ecc_block_base) {
+ edac_printk(KERN_ERR, EDAC_DEVICE,
+ "Unable to map %s ECC block\n", ecc_name);
+ ret = -ENODEV;
+ goto out1;
+ }
+
+ /* Disable ECC */
+ regmap_write(ecc_mgr_map, A10_SYSMGR_ECC_INTMASK_SET_OFST, irq_mask);
+ ecc_clear_bits(ALTR_A10_ECC_SERRINTEN,
+ (ecc_block_base + ALTR_A10_ECC_ERRINTEN_OFST));
+ ecc_clear_bits(ecc_ctrl_en_mask,
+ (ecc_block_base + ALTR_A10_ECC_CTRL_OFST));
+ /* Ensure all writes complete */
+ wmb();
+ /* Use HW initi
[PATCH v2] kaslr: allow kASLR to be default over Hibernation
Since kASLR and Hibernation can not currently coexist at runtime
on x86, the default behavior was to disable kASLR by default when
CONFIG_HIBERNATION was present (to retain original behavior).
The behavior of kASLR on arm64 (and soon MIPS) is to be enabled by
default when selected at build time. Since arm64 Hibernation does not
conflict with kASLR, this fixes the hibernation argument parsing to be
x86-specific. Additionally, since end users want to be able to select
kASLR on x86 by default at build time, create CONFIG_RANDOMIZE_BASE_ON
that is present only on x86.
Signed-off-by: Kees Cook
---
v2:
- make this x86-specific selectable, rather than global default
---
Documentation/kernel-parameters.txt | 9 +++--
arch/x86/Kconfig| 16 +++-
arch/x86/boot/compressed/aslr.c | 2 +-
kernel/power/hibernate.c| 31 +--
4 files changed, 44 insertions(+), 14 deletions(-)
diff --git a/Documentation/kernel-parameters.txt
b/Documentation/kernel-parameters.txt
index ecc74fa4bfde..282e5c826c32 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1770,12 +1770,9 @@ bytes respectively. Such letter suffixes can also be
entirely omitted.
js= [HW,JOY] Analog joystick
See Documentation/input/joystick.txt.
- kaslr/nokaslr [X86]
- Enable/disable kernel and module base offset ASLR
- (Address Space Layout Randomization) if built into
- the kernel. When CONFIG_HIBERNATION is selected,
- kASLR is disabled by default. When kASLR is enabled,
- hibernation will be disabled.
+ kaslr/nokaslr [KNL] When CONFIG_RANDOMIZE_BASE is set, this
+ enables/disables kernel and module base offset ASLR
+ (Address Space Layout Randomization).
keepinitrd [HW,ARM]
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 2dc18605831f..e0fb1717fe3c 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1932,7 +1932,7 @@ config RELOCATABLE
(CONFIG_PHYSICAL_START) is used as the minimum location.
config RANDOMIZE_BASE
- bool "Randomize the address of the kernel image"
+ bool "Randomize the address of the kernel image (kASLR)"
depends on RELOCATABLE
default n
---help---
@@ -1955,6 +1955,20 @@ config RANDOMIZE_BASE
If unsure, say N.
+config RANDOMIZE_BASE_ON
+ bool "Prefer kASLR over Hibernation"
+ depends on RANDOMIZE_BASE
+ depends on HIBERNATION
+ default n
+ ---help---
+ Currently Hibernation and kASLR are not compatible at runtime
+ on x86. To enable kASLR by default (and disable Hibernation),
+ enable this option. To enable Hibernation by default (and
+ disable kASLR), disable this option. Regardless of this
+ setting, the availability of kASLR (and therefore Hibernation)
+ can be chosen at boot time with the "kaslr" or "nokaslr"
+ kernel argument.
+
config RANDOMIZE_BASE_MAX_OFFSET
hex "Maximum kASLR offset allowed" if EXPERT
depends on RANDOMIZE_BASE
diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
index 6a9b96b4624d..8214b174b9bd 100644
--- a/arch/x86/boot/compressed/aslr.c
+++ b/arch/x86/boot/compressed/aslr.c
@@ -304,7 +304,7 @@ unsigned char *choose_kernel_location(struct boot_params
*boot_params,
unsigned long choice = (unsigned long)output;
unsigned long random;
-#ifdef CONFIG_HIBERNATION
+#ifndef CONFIG_RANDOMIZE_BASE_ON
if (!cmdline_find_option_bool("kaslr")) {
debug_putstr("KASLR disabled by default...\n");
goto out;
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index fca9254280ee..526a6403fb2e 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -35,8 +35,13 @@
static int nocompress;
+#ifndef CONFIG_RANDOMIZE_BASE_ON
static int noresume;
static int nohibernate;
+#else
+static int noresume = 1;
+static int nohibernate = 1;
+#endif
static int resume_wait;
static unsigned int resume_delay;
static char resume_file[256] = CONFIG_PM_STD_PARTITION;
@@ -1154,11 +1159,6 @@ static int __init nohibernate_setup(char *str)
return 1;
}
-static int __init kaslr_nohibernate_setup(char *str)
-{
- return nohibernate_setup(str);
-}
-
static int __init page_poison_nohibernate_setup(char *str)
{
#ifdef CONFIG_PAGE_POISONING_ZERO
@@ -1175,6 +1175,26 @@ static int __init page_poison_nohibernate_setup(char
*str)
return 1;
}
+/*
+ * Hibernation on x86 currently conflicts with kASLR, so only change
+ * hibernation boot defaults when seeing kaslr arguments on x86.
+ */
+#if defined(CONFIG_X86) && defined(CONFIG_RANDOMIZE_BASE)
+static int __init kaslr_nohibernate_setup(char *str)
+{
+
Re: [PATCH v4 5/6] fs: provide function to report enum strings
On Tue, Apr 12, 2016 at 09:54:44AM -0700, Kees Cook wrote: > Providing human-readable (and audit-parsable) strings for the READING_* > enums is needed by some LSMs. > > Signed-off-by: Kees Cook > --- > fs/exec.c | 19 +++ > include/linux/fs.h | 1 + > 2 files changed, 20 insertions(+) What the devil is that doing in fs/exec.c, of all places? -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 5/6] fs: provide function to report enum strings
On Tue, Apr 12, 2016 at 3:31 PM, Al Viro wrote: > On Tue, Apr 12, 2016 at 09:54:44AM -0700, Kees Cook wrote: >> Providing human-readable (and audit-parsable) strings for the READING_* >> enums is needed by some LSMs. >> >> Signed-off-by: Kees Cook >> --- >> fs/exec.c | 19 +++ >> include/linux/fs.h | 1 + >> 2 files changed, 20 insertions(+) > > What the devil is that doing in fs/exec.c, of all places? Since that's where the kernel_read* functions that use the enum live, it seemed like the right place to put the string function too. I'm happy to move it where ever folks think it's best to live. -Kees -- Kees Cook Chrome OS & Brillo Security -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 00/10] Add DRM Driver for HiSilicon Kirin hi6220 SoC
On 13 April 2016 at 00:08, Daniel Vetter wrote: > On Mon, Apr 11, 2016 at 04:55:33PM +0800, Xinliang Liu wrote: >> This patch set adds a new drm driver for HiSilicon Kirin hi6220 SoC. >> Current testing and support board is Hikey board which is one of Linaro >> 96boards. It is an arm64 open source board. For more information about >> this board, please access https://www.96boards.org. >> >> Hardware Detail >> --- >> The display subsystem of Hi6220 SoC is shown as bellow: >> +-+ +--+ +-+ +-+ >> | | | | | | | | >> | FB |-->| ADE|>| DSI |>| External | >> | | | | | | | HDMI/panel | >> +-+ +--+ +-+ +-+ >> >> - ADE(Advanced Display Engine) is the display controller. It contains 7 >> channels, 3 overlay compositors and a LDI. >> - A channel looks like: DMA-->clip-->scale-->ctrans(or called csc). >> - Overlay compositor is response to compose planes which come from 7 >> channels and pass composed image to LDI. >> - LDI is response to generate timings and RGB data stream. >> - DSI converts the RGB data stream from ADE to DSI packets. >> - External HDMI/panel module is connected with DSI bus. Now Hikey use a >> ADI's ADV7533 external HDMI chip. > > I haven't looked at the details again, but seems it's all ready. Please > create a pull request for the entire pile against drm-next and submit it > to Dave Airlie. Thanks Daniel, will send pull request to Dave soon. Best, -xinliang > > Thanks, Daniel > >> >> Change History >> - >> Changes in v8: >> - Rebase to v4.6-rc3 >> >> Changes in v7: >> - Add config.mutex protection when accessing mode_config.connector_list. >> - Clean up match data getting of kirin_drm_drv.c. >> - A few Regs define clean up and typo fixs for ADE crtc and DSI encoder >> driver. >> - Fix vblank irq flag "DRIVER_IRQF_SHARED" to "IRQF_SHARED". >> >> Changes in v6: >> - Cleanup values part of reg and clocks relevant properties. >> - Change "pclk_dsi" clock name to "pclk". >> >> Changes in v5: >> - Remove endpoint unit address of dsi output port. >> - Use syscon to access ADE media NOC QoS registers instread of directly >> writing registers. >> - Use reset controller to reset ADE instead of directly writing registers. >> >> Changes in v4: >> - Describe more specific of clocks and ports of binding docs. >> - Fix indentation of binding docs. >> >> Changes in v3: >> - Move and rename all the files to kirin sub-directory. >> So that we could separate different seires SoCs' driver. >> - Make ade as the drm master node. >> - Replace drm_platform_init, load, unload implementation. >> - Use assigned-clocks to set clock rate. >> - Use ports to connect display relevant nodes. >> - Rename hisi_drm_dsi.c to dw_drm_dsi.c >> - Make encoder type as DRM_MODE_ENCODER_DSI. >> - A few cleanup on regs and code. >> >> Changes in v2: >> - Remove abtraction layer of plane/crtc/encoder/connector. >> - Refactor atomic implementation according to Daniel Vetter's guides: >> http://blog.ffwll.ch/2014/11/atomic-modeset-support-for-kms-drivers.html >> http://blog.ffwll.ch/2015/09/xdc-2015-atomic-modesetting-for-drivers.html >> http://blog.ffwll.ch/2015/08/atomic-modesetting-design-overview.html >> - Use bridge instead of slave encoder to connect external HDMI. >> - Move dt binding docs to bindings/display/hisilicon directory. >> >> Xinliang Liu (10): >> drm/hisilicon: Add device tree binding for hi6220 display subsystem >> drm/hisilicon: Add hisilicon kirin drm master driver >> drm/hisilicon: Add crtc driver for ADE >> drm/hisilicon: Add plane driver for ADE >> drm/hisilicon: Add vblank driver for ADE >> drm/hisilicon: Add cma fbdev and hotplug >> drm/hisilicon: Add designware dsi encoder driver >> drm/hisilicon: Add designware dsi host driver >> drm/hisilicon: Add support for external bridge >> MAINTAINERS: Add maintainer for hisilicon DRM driver >> >> .../bindings/display/hisilicon/dw-dsi.txt | 72 ++ >> .../bindings/display/hisilicon/hisi-ade.txt| 64 ++ >> MAINTAINERS| 10 + >> drivers/gpu/drm/Kconfig|2 + >> drivers/gpu/drm/Makefile |1 + >> drivers/gpu/drm/hisilicon/Kconfig |5 + >> drivers/gpu/drm/hisilicon/Makefile |5 + >> drivers/gpu/drm/hisilicon/kirin/Kconfig| 10 + >> drivers/gpu/drm/hisilicon/kirin/Makefile |5 + >> drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 857 >> drivers/gpu/drm/hisilicon/kirin/dw_dsi_reg.h | 103 ++ >> drivers/gpu/drm/hisilicon/kirin/kirin_ade_reg.h| 230 + >> drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c| 1057 >> >> drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c| 367 +++ >> dr
RE: [PATCH v2] can: rcar_canfd: Add Renesas R-Car CAN FD driver
HI Marc,
Gentle reminder!
Are you happy with the open comment's disposition? I can send a next version of
patch if we have a closure on current set of comments.
Thanks,
Ramesh
> -Original Message-
> From: Ramesh Shanmugasundaram
> Sent: 01 April 2016 13:49
> To: Ramesh Shanmugasundaram ;
> w...@grandegger.com; robh...@kernel.org; pawel.m...@arm.com;
> mark.rutl...@arm.com; ijc+devicet...@hellion.org.uk; ga...@codeaurora.org;
> cor...@lwn.net
> Cc: linux-renesas-...@vger.kernel.org; devicet...@vger.kernel.org; linux-
> c...@vger.kernel.org; net...@vger.kernel.org; linux-doc@vger.kernel.org;
> geert+rene...@glider.be; Chris Paterson
> Subject: RE: [PATCH v2] can: rcar_canfd: Add Renesas R-Car CAN FD driver
>
> Hi Marc,
>
> Thanks for your time & review comments.
>
> > -Original Message-
> (...)
> > > +#define RCANFD_NAPI_WEIGHT 8 /* Rx poll quota */
> > > +
> > > +#define RCANFD_NUM_CHANNELS 2
> > > +#define RCANFD_CHANNELS_MASK 0x3 /* Two channels max */
> >
> > (BIT(RCANFD_NUM_CHANNELS) - 1
>
> OK
>
> >
> > > +
> > > +/* Rx FIFO is a global resource of the controller. There are 8 such
> > FIFOs
> > > + * available. Each channel gets a dedicated Rx FIFO (i.e.) the
> > > + channel
> (...)
> > > +#define RCANFD_CMFIFO_CFDLC(x) (((x) & 0xf) << 28)
> > > +#define RCANFD_CMFIFO_CFPTR(x) (((x) & 0xfff) << 16)
> > > +#define RCANFD_CMFIFO_CFTS(x)(((x) & 0xff) << 0)
> > > +
> > > +/* Global Test Config register */
> > > +#define RCANFD_GTSTCFG_C0CBCEBIT(0)
> > > +#define RCANFD_GTSTCFG_C1CBCEBIT(1)
> > > +
> > > +#define RCANFD_GTSTCTR_ICBCTME BIT(0)
> > > +
> > > +/* AFL Rx rules registers */
> > > +#define RCANFD_AFLCFG_SETRNC0(x) (((x) & 0xff) << 24)
> > > +#define RCANFD_AFLCFG_SETRNC1(x) (((x) & 0xff) << 16)
> >
> > What about something like:
> >
> > #define RCANFD_AFLCFG_SETRNC(n, x) (((x) & 0xff) << (24 - n * 8))
> >
> > This will save some if()s in the code
>
> Nice :-). Will update.
>
> >
> > > +#define RCANFD_AFLCFG_GETRNC0(x) (((x) >> 24) & 0xff)
> > > +#define RCANFD_AFLCFG_GETRNC1(x) (((x) >> 16) & 0xff)
> > > +
> > > +#define RCANFD_AFL_PAGENUM(entry)((entry) / 16)
> (...)
> > > +#define rcar_canfd_read(priv, offset)\
> > > + readl(priv->base + (offset))
> > > +#define rcar_canfd_write(priv, offset, val) \
> > > + writel(val, priv->base + (offset))
> > > +#define rcar_canfd_set_bit(priv, reg, val) \
> > > + rcar_canfd_update(val, val, priv->base + (reg))
> > > +#define rcar_canfd_clear_bit(priv, reg, val) \
> > > + rcar_canfd_update(val, 0, priv->base + (reg))
> > > +#define rcar_canfd_update_bit(priv, reg, mask, val) \
> > > + rcar_canfd_update(mask, val, priv->base + (reg))
> >
> > please use static inline functions instead of defines.
>
> OK.
>
> >
> > > +
> > > +static void rcar_canfd_get_data(struct canfd_frame *cf,
> > > + struct rcar_canfd_channel *priv, u32 off)
> >
> > Please use "struct rcar_canfd_channel *priv" as first argument, struct
> > canfd_frame *cf as second. Remove off, as the offset is already
> > defined by the channel.
>
> I'll re-order priv, cf as you mentioned. I'll leave "off" arg as it is
> because it is based on FIFO number of channel + mode (CAN only or CANFD
> only). Otherwise, I will have to add another check inside this function
> for mode.
>
> > > +{
> > > + u32 i, lwords;
> > > +
> > > + lwords = cf->len / sizeof(u32);
> > > + if (cf->len % sizeof(u32))
> > > + lwords++;
> >
> > Use DIV_ROUND_UP() instead of open coding it.
>
> Agreed. Thanks.
>
> >
> > > + for (i = 0; i < lwords; i++)
> > > + *((u32 *)cf->data + i) =
> > > + rcar_canfd_read(priv, off + (i * sizeof(u32))); }
> > > +
> > > +static void rcar_canfd_put_data(struct canfd_frame *cf,
> > > + struct rcar_canfd_channel *priv, u32 off)
> >
> > same here
>
> Yes (same as _get_data)
>
> >
> > > +{
> > > + u32 i, j, lwords, leftover;
> > > + u32 data = 0;
> > > +
> > > + lwords = cf->len / sizeof(u32);
> > > + leftover = cf->len % sizeof(u32);
> > > + for (i = 0; i < lwords; i++)
> > > + rcar_canfd_write(priv, off + (i * sizeof(u32)),
> > > + *((u32 *)cf->data + i));
> >
> > Here you don't convert the endianess...
>
> Yes
>
> > > +
> > > + if (leftover) {
> > > + u8 *p = (u8 *)((u32 *)cf->data + i);
> > > +
> > > + for (j = 0; j < leftover; j++)
> > > + data |= p[j] << (j * 8);
> >
> > ...here you do an implicit endianess conversion. "data" is little
> > endian, while p[j] is big endian.
>
> Not sure I got the question correctly.
> Controller expectation of data bytes in 32bit register is bits[7:0] =
> byte0, bits[15:8] = byte1 and so on - little endian.
> For e.g. if cf->data points to byte stream H'112233445566 (cf->data[0] =
> 0x11), first rcar_
