Re: [PATCH v2] Documentation: admin-guide: PM: Add cpuidle document

2018-11-29 Thread Ulf Hansson
On Wed, 28 Nov 2018 at 12:47, Rafael J. Wysocki  wrote:
>
> From: Rafael J. Wysocki 
>
> Important information is missing from user/admin cpuidle documentation
> available today, so add a new user/admin document for cpuidle containing
> current and comprehensive information to admin-guide and drop the old
> .txt documents it is replacing.
>
> Signed-off-by: Rafael J. Wysocki 
> ---

Wow! Great work!

[...]

> +.. _idle-states-representation:
> +
> +Representation of Idle States
> +=
> +
> +For the CPU idle time management purposes all of the physical idle states
> +supported by the processor have to be represented as a one-dimensional array 
> of
> +|struct cpuidle_state| objects each allowing an individual (logical) CPU to 
> ask
> +the processor hardware to enter an idle state of certain properties.  If 
> there
> +is a hierarchy of units in the processor, one |struct cpuidle_state| object 
> can
> +cover a combination of idle states supported by the units at different 
> levels of
> +the hierarchy.  In that case, the `target residency and exit latency 
> parameters
> +of it `_, must reflect the properties of the idle state at the
> +deepest level (i.e. the idle state of the unit containing all of the other
> +units).
> +
> +For example, take a processor with two cores in a larger unit referred to as
> +a "module" and suppose that asking the hardware to enter a specific idle 
> state
> +(say "X") at the "core" level by one core will trigger the module to try to
> +enter a specific idle state of its own (say "MX") if the other core is in 
> idle
> +state "X" already.  In other words, asking for idle state "X" at the "core"
> +level gives the hardware a license to go as deep as to idle state "MX" at the
> +"module" level, but there is no guarantee that this is going to happen (the 
> core
> +asking for idle state "X" may just end up in that state by itself instead).

This is a good description of a HW like x86 and ARM64 (using PSCI in
platform coordinated mode).

However, for "legacy" ARM products, having at least two cores in a
module, this description does in many cases *not* match how the HW
works. I wonder about other architectures.

My point is, I think it would be fair to mention that the above
description is specific to certain HWs. In other words, for HWs that
leaves the entire state synchronization of the last man standing
algorithm to be managed by a FW, the cpuidle framework plays well. But
for others it has limitations.

> +Then, the target residency of the |struct cpuidle_state| object representing
> +idle state "X" must reflect the minimum time to spend in idle state "MX" of
> +the module (including the time needed to enter it), because that is the 
> minimum
> +time the CPU needs to be idle to save any energy in case the hardware enters
> +that state.  Analogously, the exit latency parameter of that object must 
> cover
> +the exit time of idle state "MX" of the module (and usually its entry time 
> too),
> +because that is the maximum delay between a wakeup signal and the time the 
> CPU
> +will start to execute the first new instruction (assuming that both cores in 
> the
> +module will always be ready to execute instructions as soon as the module
> +becomes operational as a whole).

For the HW working as you described, this is a nice clarification, thanks!

[...]

> +
> +.. _cpu-pm-qos:
> +
> +Power Management Quality of Service for CPUs
> +
> +
> +The power management quality of service (PM QoS) framework in the Linux 
> kernel
> +allows kernel code and user space processes to set constraints on various
> +energy-efficiency features of the kernel to prevent performance from dropping
> +below a required level.  The PM QoS constraints can be set globally, in
> +predefined categories referred to as PM QoS classes, or against individual
> +devices.
> +
> +CPU idle time management can be affected by PM QoS in two ways, through the
> +global constraint in the ``PM_QOS_CPU_DMA_LATENCY`` class and through the

No objections to the description, it's really nice! However, the
naming of this QOS class, is to me, rather confusing.

I realize renaming the sysfs node is not an option, but maybe we
should make a tree wide change to rename the class to something more
descriptive. What do you think?

[...]

Kind regards
Uffe


Re: [PATCH v2] Documentation: admin-guide: PM: Add cpuidle document

2018-11-29 Thread Rafael J. Wysocki
On Thu, Nov 29, 2018 at 9:07 AM Ulf Hansson  wrote:
>
> On Wed, 28 Nov 2018 at 12:47, Rafael J. Wysocki  wrote:
> >
> > From: Rafael J. Wysocki 
> >
> > Important information is missing from user/admin cpuidle documentation
> > available today, so add a new user/admin document for cpuidle containing
> > current and comprehensive information to admin-guide and drop the old
> > .txt documents it is replacing.
> >
> > Signed-off-by: Rafael J. Wysocki 
> > ---
>
> Wow! Great work!
>
> [...]
>
> > +.. _idle-states-representation:
> > +
> > +Representation of Idle States
> > +=
> > +
> > +For the CPU idle time management purposes all of the physical idle states
> > +supported by the processor have to be represented as a one-dimensional 
> > array of
> > +|struct cpuidle_state| objects each allowing an individual (logical) CPU 
> > to ask
> > +the processor hardware to enter an idle state of certain properties.  If 
> > there
> > +is a hierarchy of units in the processor, one |struct cpuidle_state| 
> > object can
> > +cover a combination of idle states supported by the units at different 
> > levels of
> > +the hierarchy.  In that case, the `target residency and exit latency 
> > parameters
> > +of it `_, must reflect the properties of the idle state at the
> > +deepest level (i.e. the idle state of the unit containing all of the other
> > +units).
> > +
> > +For example, take a processor with two cores in a larger unit referred to 
> > as
> > +a "module" and suppose that asking the hardware to enter a specific idle 
> > state
> > +(say "X") at the "core" level by one core will trigger the module to try to
> > +enter a specific idle state of its own (say "MX") if the other core is in 
> > idle
> > +state "X" already.  In other words, asking for idle state "X" at the "core"
> > +level gives the hardware a license to go as deep as to idle state "MX" at 
> > the
> > +"module" level, but there is no guarantee that this is going to happen 
> > (the core
> > +asking for idle state "X" may just end up in that state by itself instead).
>
> This is a good description of a HW like x86 and ARM64 (using PSCI in
> platform coordinated mode).
>
> However, for "legacy" ARM products, having at least two cores in a
> module, this description does in many cases *not* match how the HW
> works. I wonder about other architectures.

Well, this is an example and it is very clearly presented this way IMO
(it says "for example" upfront and then "suppose" etc).

Basically, the purpose of it is to explain that the parameters shown
in sysfs may be effective.

> My point is, I think it would be fair to mention that the above
> description is specific to certain HWs. In other words, for HWs that
> leaves the entire state synchronization of the last man standing
> algorithm to be managed by a FW, the cpuidle framework plays well. But
> for others it has limitations.

I can add it after this entire paragraph I think.  Something like

"There are processors without direct coordination between different
levels of the hierarchy of units inside them, however.  In those cases
asking for an idle state at the "core" level does not automatically
affect the "module" level, for example, in any way and the CPUIdle
driver is responsible for the entire handling of the hierarchy.  Then,
the definition of the idle state objects is entirely up to the driver,
but still the physical properties of the idle state that the processor
hardware finally goes into must always follow the parameters used by
the governor for idle state selection (for instance, the actual exit
latency of that idle state must not exceed the exit latency parameter
of the idle state object selected by the governor)."

>
> > +Then, the target residency of the |struct cpuidle_state| object 
> > representing
> > +idle state "X" must reflect the minimum time to spend in idle state "MX" of
> > +the module (including the time needed to enter it), because that is the 
> > minimum
> > +time the CPU needs to be idle to save any energy in case the hardware 
> > enters
> > +that state.  Analogously, the exit latency parameter of that object must 
> > cover
> > +the exit time of idle state "MX" of the module (and usually its entry time 
> > too),
> > +because that is the maximum delay between a wakeup signal and the time the 
> > CPU
> > +will start to execute the first new instruction (assuming that both cores 
> > in the
> > +module will always be ready to execute instructions as soon as the module
> > +becomes operational as a whole).
>
> For the HW working as you described, this is a nice clarification, thanks!
>
> [...]
>
> > +
> > +.. _cpu-pm-qos:
> > +
> > +Power Management Quality of Service for CPUs
> > +
> > +
> > +The power management quality of service (PM QoS) framework in the Linux 
> > kernel
> > +allows kernel code and user space processes to set constraints on various
> > +energy-efficiency features of the kernel to p

Re: [PATCH v4 1/5] cgroup: subtree_control bypass mode for bypassable controllers

2018-11-29 Thread Dan Carpenter
Hi Waiman,

Thank you for the patch! Perhaps something to improve:

url:
https://github.com/0day-ci/linux/commits/Waiman-Long/cgroup-Introducing-bypass-mode/20181123-030552
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git for-next

smatch warnings:
kernel/cgroup/cgroup.c:4893 css_create() error: we previously assumed 'parent' 
could be null (see line 4864)

# 
https://github.com/0day-ci/linux/commit/8b68fd4330e043645667a5d3306398f8f88f9ff2
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 8b68fd4330e043645667a5d3306398f8f88f9ff2
vim +/parent +4893 kernel/cgroup/cgroup.c

a31f2d3ff kernel/cgroup.cTejun Heo2012-11-19  4840  
c81c925ad kernel/cgroup.cTejun Heo2013-12-06  4841  /**
6cd0f5bba kernel/cgroup.cTejun Heo2016-03-03  4842   * 
css_create - create a cgroup_subsys_state
c81c925ad kernel/cgroup.cTejun Heo2013-12-06  4843   * @cgrp: 
the cgroup new css will be associated with
c81c925ad kernel/cgroup.cTejun Heo2013-12-06  4844   * @ss: the 
subsys of new css
c81c925ad kernel/cgroup.cTejun Heo2013-12-06  4845   *
c81c925ad kernel/cgroup.cTejun Heo2013-12-06  4846   * Create a 
new css associated with @cgrp - @ss pair.  On success, the new
6cd0f5bba kernel/cgroup.cTejun Heo2016-03-03  4847   * css is 
online and installed in @cgrp.  This function doesn't create the
6cd0f5bba kernel/cgroup.cTejun Heo2016-03-03  4848   * 
interface files.  Returns 0 on success, -errno on failure.
c81c925ad kernel/cgroup.cTejun Heo2013-12-06  4849   */
6cd0f5bba kernel/cgroup.cTejun Heo2016-03-03  4850  static 
struct cgroup_subsys_state *css_create(struct cgroup *cgrp,
6cd0f5bba kernel/cgroup.cTejun Heo2016-03-03  4851  
  struct cgroup_subsys *ss)
c81c925ad kernel/cgroup.cTejun Heo2013-12-06  4852  {
d51f39b05 kernel/cgroup.cTejun Heo2014-05-16  4853  struct 
cgroup *parent = cgroup_parent(cgrp);
8b68fd433 kernel/cgroup/cgroup.c Waiman Long  2018-11-20  4854  struct 
cgroup_subsys_state *parent_css = NULL;
c81c925ad kernel/cgroup.cTejun Heo2013-12-06  4855  struct 
cgroup_subsys_state *css;
c81c925ad kernel/cgroup.cTejun Heo2013-12-06  4856  int err;
c81c925ad kernel/cgroup.cTejun Heo2013-12-06  4857  
c81c925ad kernel/cgroup.cTejun Heo2013-12-06  4858  
lockdep_assert_held(&cgroup_mutex);
c81c925ad kernel/cgroup.cTejun Heo2013-12-06  4859  
8b68fd433 kernel/cgroup/cgroup.c Waiman Long  2018-11-20  4860  /*
8b68fd433 kernel/cgroup/cgroup.c Waiman Long  2018-11-20  4861   * As 
cgroup may be in bypass mode, need to skip over ancestor
8b68fd433 kernel/cgroup/cgroup.c Waiman Long  2018-11-20  4862   * 
cgroups with NULL CSS.
8b68fd433 kernel/cgroup/cgroup.c Waiman Long  2018-11-20  4863   */
8b68fd433 kernel/cgroup/cgroup.c Waiman Long  2018-11-20 @4864  for (; 
parent && !parent_css; parent = cgroup_parent(parent))
   
^
8b68fd433 kernel/cgroup/cgroup.c Waiman Long  2018-11-20  4865  
parent_css = cgroup_css(parent, ss);

When we exit this loop it means either parent is NULL or parent_css
is non-NULL.

8b68fd433 kernel/cgroup/cgroup.c Waiman Long  2018-11-20  4866  
1fed1b2e3 kernel/cgroup.cTejun Heo2014-05-16  4867  css = 
ss->css_alloc(parent_css);
e7e15b87f kernel/cgroup.cTejun Heo2016-06-21  4868  if 
(!css)
e7e15b87f kernel/cgroup.cTejun Heo2016-06-21  4869  
css = ERR_PTR(-ENOMEM);
c81c925ad kernel/cgroup.cTejun Heo2013-12-06  4870  if 
(IS_ERR(css))
6cd0f5bba kernel/cgroup.cTejun Heo2016-03-03  4871  
return css;
c81c925ad kernel/cgroup.cTejun Heo2013-12-06  4872  
8b68fd433 kernel/cgroup/cgroup.c Waiman Long  2018-11-20  4873  
init_and_link_css(css, ss, cgrp, parent_css);
a2bed8209 kernel/cgroup.cTejun Heo2014-05-04  4874  
2aad2a86f kernel/cgroup.cTejun Heo2014-09-24  4875  err = 
percpu_ref_init(&css->refcnt, css_release, 0, GFP_KERNEL);
c81c925ad kernel/cgroup.cTejun Heo2013-12-06  4876  if (err)
3eb59ec64 kernel/cgroup.cLi Zefan 2014-03-18  4877  
goto err_free_css;
c81c925ad kernel/cgroup.cTejun Heo2013-12-06  4878  
cf780b7dc kernel/cgroup.cVladimir Davydov 2015-08-03  4879  err = 
cgroup_idr_alloc(&ss->css_idr, NULL, 2, 0, GFP_KERNEL);
15a4c835e kernel/cgroup.cTejun Heo2014-05-04  4880  if (err 
< 0)
b00c52dae kernel/cgroup.cWenwei Tao   20

Re: [PATCH 0/9] drm: remove deprecated functions

2018-11-29 Thread Linus Walleij
On Mon, Nov 26, 2018 at 3:12 PM Daniel Vetter  wrote:
> On Sat, Nov 24, 2018 at 10:17:13PM +0100, Linus Walleij wrote:

> > It was especially scary.
> >
> > But I think I managed to apply the patches and push the
> > branch now.
>
> Except when you're racing with someone else you should only see conflicts
> with stuff you've just pushed. Or if someone forgot to fix up their mess.
> What was the conflict?

dim push-branches was complaining that one of the commits was
missing the proper committer sign-off, it was the bottom commit under
mine (IIRC "drm/atomic-helper: WARN if fake_commit->hw_done is not
completed as expected")
and dim update-branches seemed to rebase and fix up my patches
and then everything was fine.

I just felt slightly out of control :D

Yours,
Linus Walleij


Re: [PATCH 0/9] drm: remove deprecated functions

2018-11-29 Thread Daniel Vetter
On Thu, Nov 29, 2018 at 3:45 PM Linus Walleij  wrote:
>
> On Mon, Nov 26, 2018 at 3:12 PM Daniel Vetter  wrote:
> > On Sat, Nov 24, 2018 at 10:17:13PM +0100, Linus Walleij wrote:
>
> > > It was especially scary.
> > >
> > > But I think I managed to apply the patches and push the
> > > branch now.
> >
> > Except when you're racing with someone else you should only see conflicts
> > with stuff you've just pushed. Or if someone forgot to fix up their mess.
> > What was the conflict?
>
> dim push-branches was complaining that one of the commits was
> missing the proper committer sign-off, it was the bottom commit under
> mine (IIRC "drm/atomic-helper: WARN if fake_commit->hw_done is not
> completed as expected")
> and dim update-branches seemed to rebase and fix up my patches
> and then everything was fine.

This sounds like you (or dim?) accidentaly amended that commit (which
changes the committer and results in the warning), and a rebase would
indeed have fixed that. If the first patch conflicts this can happen
because dim apply-branch doesn't bail out correctly. Or at least did
in the past, I recently fixed that in

commit ee299e510ae468aab27610bcbc4fdd4de932f74b
Author: Daniel Vetter 
Date:   Wed Oct 17 08:53:00 2018 +0200

dim: make apply-patch fail again

> I just felt slightly out of control :D

If your dim didn't have the above commit and you had a conflict it's
all explained. Otherwise I'm not sure what's been going on ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch


[PATCH] drm: TODO: Add DRM_MODESET_LOCK_ALL* conversion to todo.rst

2018-11-29 Thread Sean Paul
From: Sean Paul 

We should also get the drivers using the helpers. Seems like a good
intermediate TODO item.

Cc: Daniel Vetter 
Signed-off-by: Sean Paul 
---
 Documentation/gpu/todo.rst | 12 
 1 file changed, 12 insertions(+)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index ab347dec50796..1a6dc586fb156 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -241,6 +241,18 @@ struct drm_gem_object_funcs
 GEM objects can now have a function table instead of having the callbacks on 
the
 DRM driver struct. This is now the preferred way and drivers can be moved over.
 
+Use DRM_MODESET_LOCK_ALL_* helpers instead of boilerplate
+-
+
+For cases where drivers are attempting to grab the modeset locks with a local
+acquire context. Replace the boilerplate code surrounding
+drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN() and
+DRM_MODESET_LOCK_ALL_END() instead.
+
+As a reference, take a look at the conversions already completed in drm core.
+
+Contact: Sean Paul, respective driver maintainers
+
 Core refactorings
 =
 
-- 
Sean Paul, Software Engineer, Google / Chromium OS



[PATCH] Documentation: drm: Remove dangling pointer from drm-mm.rst

2018-11-29 Thread Sean Paul
From: Sean Paul 

Fixes htmldocs build error:
Error: Cannot open file ../drivers/gpu/drm/drm_global.c

Fixes: 2bb42410b1bd ("drm: Remove drm_global.{c,h} v2")
Cc: Thomas Zimmermann 
Cc: Christian König 
Cc: Junwei Zhang 
Cc: Alex Deucher 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Christian Koenig 
Cc: Huang Rui 
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Sean Paul 
---
 Documentation/gpu/drm-mm.rst | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index d0f3c6b032009..54a696d961a7c 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -79,9 +79,6 @@ count for the TTM, which will call your initialization 
function.
 
 See the radeon_ttm.c file for an example of usage.
 
-.. kernel-doc:: drivers/gpu/drm/drm_global.c
-   :export:
-
 
 The Graphics Execution Manager (GEM)
 
-- 
Sean Paul, Software Engineer, Google / Chromium OS



Re: [PATCH] Documentation: drm: Remove dangling pointer from drm-mm.rst

2018-11-29 Thread Koenig, Christian
Am 29.11.18 um 16:55 schrieb Sean Paul:
> From: Sean Paul 
>
> Fixes htmldocs build error:
> Error: Cannot open file ../drivers/gpu/drm/drm_global.c
>
> Fixes: 2bb42410b1bd ("drm: Remove drm_global.{c,h} v2")
> Cc: Thomas Zimmermann 
> Cc: Christian König 
> Cc: Junwei Zhang 
> Cc: Alex Deucher 
> Cc: Maarten Lankhorst 
> Cc: Maxime Ripard 
> Cc: Sean Paul 
> Cc: David Airlie 
> Cc: Christian Koenig 
> Cc: Huang Rui 
> Cc: dri-de...@lists.freedesktop.org
> Signed-off-by: Sean Paul 

Reviewed-by: Christian König 

> ---
>   Documentation/gpu/drm-mm.rst | 3 ---
>   1 file changed, 3 deletions(-)
>
> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> index d0f3c6b032009..54a696d961a7c 100644
> --- a/Documentation/gpu/drm-mm.rst
> +++ b/Documentation/gpu/drm-mm.rst
> @@ -79,9 +79,6 @@ count for the TTM, which will call your initialization 
> function.
>   
>   See the radeon_ttm.c file for an example of usage.
>   
> -.. kernel-doc:: drivers/gpu/drm/drm_global.c
> -   :export:
> -
>   
>   The Graphics Execution Manager (GEM)
>   



Re: [PATCH] Documentation: drm: Remove dangling pointer from drm-mm.rst

2018-11-29 Thread Sean Paul
On Thu, Nov 29, 2018 at 10:58 AM Koenig, Christian
 wrote:
>
> Am 29.11.18 um 16:55 schrieb Sean Paul:
> > From: Sean Paul 
> >
> > Fixes htmldocs build error:
> > Error: Cannot open file ../drivers/gpu/drm/drm_global.c
> >
> > Fixes: 2bb42410b1bd ("drm: Remove drm_global.{c,h} v2")
> > Cc: Thomas Zimmermann 
> > Cc: Christian König 
> > Cc: Junwei Zhang 
> > Cc: Alex Deucher 
> > Cc: Maarten Lankhorst 
> > Cc: Maxime Ripard 
> > Cc: Sean Paul 
> > Cc: David Airlie 
> > Cc: Christian Koenig 
> > Cc: Huang Rui 
> > Cc: dri-de...@lists.freedesktop.org
> > Signed-off-by: Sean Paul 
>
> Reviewed-by: Christian König 
>

Thanks for the review. Perhaps the best course of action for this
patch is to have Dave pick it directly and then we can backmerge it
into amdgpu/drm-misc-next from there? I could also spin a topic branch
that we could all merge.

Thoughts?

Sean

> > ---
> >   Documentation/gpu/drm-mm.rst | 3 ---
> >   1 file changed, 3 deletions(-)
> >
> > diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> > index d0f3c6b032009..54a696d961a7c 100644
> > --- a/Documentation/gpu/drm-mm.rst
> > +++ b/Documentation/gpu/drm-mm.rst
> > @@ -79,9 +79,6 @@ count for the TTM, which will call your initialization 
> > function.
> >
> >   See the radeon_ttm.c file for an example of usage.
> >
> > -.. kernel-doc:: drivers/gpu/drm/drm_global.c
> > -   :export:
> > -
> >
> >   The Graphics Execution Manager (GEM)
> >   
>


Re: [PATCH] Documentation: drm: Remove dangling pointer from drm-mm.rst

2018-11-29 Thread Christian König

Am 29.11.18 um 17:11 schrieb Sean Paul:

On Thu, Nov 29, 2018 at 10:58 AM Koenig, Christian
 wrote:

Am 29.11.18 um 16:55 schrieb Sean Paul:

From: Sean Paul 

Fixes htmldocs build error:
Error: Cannot open file ../drivers/gpu/drm/drm_global.c

Fixes: 2bb42410b1bd ("drm: Remove drm_global.{c,h} v2")
Cc: Thomas Zimmermann 
Cc: Christian König 
Cc: Junwei Zhang 
Cc: Alex Deucher 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Christian Koenig 
Cc: Huang Rui 
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Sean Paul 

Reviewed-by: Christian König 


Thanks for the review. Perhaps the best course of action for this
patch is to have Dave pick it directly and then we can backmerge it
into amdgpu/drm-misc-next from there? I could also spin a topic branch
that we could all merge.

Thoughts?


My preference is that I pick it up and push it to Dave through Alex 
fixes push.


But just because the patch which broke this came the same way as well.

Either way works for me,
Christian.



Sean


---
   Documentation/gpu/drm-mm.rst | 3 ---
   1 file changed, 3 deletions(-)

diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index d0f3c6b032009..54a696d961a7c 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -79,9 +79,6 @@ count for the TTM, which will call your initialization 
function.

   See the radeon_ttm.c file for an example of usage.

-.. kernel-doc:: drivers/gpu/drm/drm_global.c
-   :export:
-

   The Graphics Execution Manager (GEM)
   

___
dri-devel mailing list
dri-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel




Re: [PATCH] Documentation: drm: Remove dangling pointer from drm-mm.rst

2018-11-29 Thread Sean Paul
On Thu, Nov 29, 2018 at 11:14 AM Christian König
 wrote:
>
> Am 29.11.18 um 17:11 schrieb Sean Paul:
> > On Thu, Nov 29, 2018 at 10:58 AM Koenig, Christian
> >  wrote:
> >> Am 29.11.18 um 16:55 schrieb Sean Paul:
> >>> From: Sean Paul 
> >>>
> >>> Fixes htmldocs build error:
> >>> Error: Cannot open file ../drivers/gpu/drm/drm_global.c
> >>>
> >>> Fixes: 2bb42410b1bd ("drm: Remove drm_global.{c,h} v2")
> >>> Cc: Thomas Zimmermann 
> >>> Cc: Christian König 
> >>> Cc: Junwei Zhang 
> >>> Cc: Alex Deucher 
> >>> Cc: Maarten Lankhorst 
> >>> Cc: Maxime Ripard 
> >>> Cc: Sean Paul 
> >>> Cc: David Airlie 
> >>> Cc: Christian Koenig 
> >>> Cc: Huang Rui 
> >>> Cc: dri-de...@lists.freedesktop.org
> >>> Signed-off-by: Sean Paul 
> >> Reviewed-by: Christian König 
> >>
> > Thanks for the review. Perhaps the best course of action for this
> > patch is to have Dave pick it directly and then we can backmerge it
> > into amdgpu/drm-misc-next from there? I could also spin a topic branch
> > that we could all merge.
> >
> > Thoughts?
>
> My preference is that I pick it up and push it to Dave through Alex
> fixes push.
>
> But just because the patch which broke this came the same way as well.
>

Yeah, unfortunately it's proliferated, and I'd rather not add hops to
getting drm-misc-next fixed. Let's just do a topic branch and set a PR
to Dave from that. We can both merge the topic branch asap and let
Dave pick it up at his leisure.

Sean

> Either way works for me,
> Christian.
>
> >
> > Sean
> >
> >>> ---
> >>>Documentation/gpu/drm-mm.rst | 3 ---
> >>>1 file changed, 3 deletions(-)
> >>>
> >>> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> >>> index d0f3c6b032009..54a696d961a7c 100644
> >>> --- a/Documentation/gpu/drm-mm.rst
> >>> +++ b/Documentation/gpu/drm-mm.rst
> >>> @@ -79,9 +79,6 @@ count for the TTM, which will call your initialization 
> >>> function.
> >>>
> >>>See the radeon_ttm.c file for an example of usage.
> >>>
> >>> -.. kernel-doc:: drivers/gpu/drm/drm_global.c
> >>> -   :export:
> >>> -
> >>>
> >>>The Graphics Execution Manager (GEM)
> >>>
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>


Re: [PATCH] Documentation: drm: Remove dangling pointer from drm-mm.rst

2018-11-29 Thread Koenig, Christian
Am 29.11.18 um 17:21 schrieb Sean Paul:
> On Thu, Nov 29, 2018 at 11:14 AM Christian König
>  wrote:
>> Am 29.11.18 um 17:11 schrieb Sean Paul:
>>> On Thu, Nov 29, 2018 at 10:58 AM Koenig, Christian
>>>  wrote:
 Am 29.11.18 um 16:55 schrieb Sean Paul:
> From: Sean Paul 
>
> Fixes htmldocs build error:
> Error: Cannot open file ../drivers/gpu/drm/drm_global.c
>
> Fixes: 2bb42410b1bd ("drm: Remove drm_global.{c,h} v2")
> Cc: Thomas Zimmermann 
> Cc: Christian König 
> Cc: Junwei Zhang 
> Cc: Alex Deucher 
> Cc: Maarten Lankhorst 
> Cc: Maxime Ripard 
> Cc: Sean Paul 
> Cc: David Airlie 
> Cc: Christian Koenig 
> Cc: Huang Rui 
> Cc: dri-de...@lists.freedesktop.org
> Signed-off-by: Sean Paul 
 Reviewed-by: Christian König 

>>> Thanks for the review. Perhaps the best course of action for this
>>> patch is to have Dave pick it directly and then we can backmerge it
>>> into amdgpu/drm-misc-next from there? I could also spin a topic branch
>>> that we could all merge.
>>>
>>> Thoughts?
>> My preference is that I pick it up and push it to Dave through Alex
>> fixes push.
>>
>> But just because the patch which broke this came the same way as well.
>>
> Yeah, unfortunately it's proliferated, and I'd rather not add hops to
> getting drm-misc-next fixed. Let's just do a topic branch and set a PR
> to Dave from that. We can both merge the topic branch asap and let
> Dave pick it up at his leisure.

Works for me as well,
Christian.

>
> Sean
>
>> Either way works for me,
>> Christian.
>>
>>> Sean
>>>
> ---
> Documentation/gpu/drm-mm.rst | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> index d0f3c6b032009..54a696d961a7c 100644
> --- a/Documentation/gpu/drm-mm.rst
> +++ b/Documentation/gpu/drm-mm.rst
> @@ -79,9 +79,6 @@ count for the TTM, which will call your initialization 
> function.
>
> See the radeon_ttm.c file for an example of usage.
>
> -.. kernel-doc:: drivers/gpu/drm/drm_global.c
> -   :export:
> -
>
> The Graphics Execution Manager (GEM)
> 
>>> ___
>>> dri-devel mailing list
>>> dri-de...@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel



Re: [PATCH] drm: TODO: Add DRM_MODESET_LOCK_ALL* conversion to todo.rst

2018-11-29 Thread Daniel Vetter
On Thu, Nov 29, 2018 at 10:52:40AM -0500, Sean Paul wrote:
> From: Sean Paul 
> 
> We should also get the drivers using the helpers. Seems like a good
> intermediate TODO item.
> 
> Cc: Daniel Vetter 
> Signed-off-by: Sean Paul 

Acked-by: Daniel Vetter 
> ---
>  Documentation/gpu/todo.rst | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index ab347dec50796..1a6dc586fb156 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -241,6 +241,18 @@ struct drm_gem_object_funcs
>  GEM objects can now have a function table instead of having the callbacks on 
> the
>  DRM driver struct. This is now the preferred way and drivers can be moved 
> over.
>  
> +Use DRM_MODESET_LOCK_ALL_* helpers instead of boilerplate
> +-
> +
> +For cases where drivers are attempting to grab the modeset locks with a local
> +acquire context. Replace the boilerplate code surrounding
> +drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN() and
> +DRM_MODESET_LOCK_ALL_END() instead.

Maybe add:

"This should also be done for all places where drm_modest_lock_all() is
still used."

Cheers, Daniel

> +
> +As a reference, take a look at the conversions already completed in drm core.
> +
> +Contact: Sean Paul, respective driver maintainers
> +
>  Core refactorings
>  =
>  
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


Re: [PATCH v12 23/25] kasan, arm64: select HAVE_ARCH_KASAN_SW_TAGS

2018-11-29 Thread Will Deacon
On Tue, Nov 27, 2018 at 05:55:41PM +0100, Andrey Konovalov wrote:
> Now, that all the necessary infrastructure code has been introduced,
> select HAVE_ARCH_KASAN_SW_TAGS for arm64 to enable software tag-based
> KASAN mode.
> 
> Signed-off-by: Andrey Konovalov 
> ---
>  arch/arm64/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 787d7850e064..8b331dcfb48e 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -111,6 +111,7 @@ config ARM64
>   select HAVE_ARCH_JUMP_LABEL
>   select HAVE_ARCH_JUMP_LABEL_RELATIVE
>   select HAVE_ARCH_KASAN if !(ARM64_16K_PAGES && ARM64_VA_BITS_48)
> + select HAVE_ARCH_KASAN_SW_TAGS if !(ARM64_16K_PAGES && ARM64_VA_BITS_48)

Can you do if HAVE_ARCH_KASAN instead?

Will


Re: [PATCH v12 20/25] kasan, arm64: add brk handler for inline instrumentation

2018-11-29 Thread Will Deacon
On Tue, Nov 27, 2018 at 05:55:38PM +0100, Andrey Konovalov wrote:
> Tag-based KASAN inline instrumentation mode (which embeds checks of shadow
> memory into the generated code, instead of inserting a callback) generates
> a brk instruction when a tag mismatch is detected.
> 
> This commit adds a tag-based KASAN specific brk handler, that decodes the
> immediate value passed to the brk instructions (to extract information
> about the memory access that triggered the mismatch), reads the register
> values (x0 contains the guilty address) and reports the bug.
> 
> Reviewed-by: Andrey Ryabinin 
> Reviewed-by: Dmitry Vyukov 
> Signed-off-by: Andrey Konovalov 
> ---
>  arch/arm64/include/asm/brk-imm.h |  2 +
>  arch/arm64/kernel/traps.c| 68 +++-
>  include/linux/kasan.h|  3 ++
>  3 files changed, 71 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/brk-imm.h 
> b/arch/arm64/include/asm/brk-imm.h
> index ed693c5bcec0..2945fe6cd863 100644
> --- a/arch/arm64/include/asm/brk-imm.h
> +++ b/arch/arm64/include/asm/brk-imm.h
> @@ -16,10 +16,12 @@
>   * 0x400: for dynamic BRK instruction
>   * 0x401: for compile time BRK instruction
>   * 0x800: kernel-mode BUG() and WARN() traps
> + * 0x9xx: tag-based KASAN trap (allowed values 0x900 - 0x9ff)
>   */
>  #define FAULT_BRK_IMM0x100
>  #define KGDB_DYN_DBG_BRK_IMM 0x400
>  #define KGDB_COMPILED_DBG_BRK_IMM0x401
>  #define BUG_BRK_IMM  0x800
> +#define KASAN_BRK_IMM0x900
>  
>  #endif
> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> index 5f4d9acb32f5..04bdc53716ef 100644
> --- a/arch/arm64/kernel/traps.c
> +++ b/arch/arm64/kernel/traps.c
> @@ -35,6 +35,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -284,10 +285,14 @@ void arm64_notify_die(const char *str, struct pt_regs 
> *regs,
>   }
>  }
>  
> -void arm64_skip_faulting_instruction(struct pt_regs *regs, unsigned long 
> size)
> +void __arm64_skip_faulting_instruction(struct pt_regs *regs, unsigned long 
> size)
>  {
>   regs->pc += size;
> +}
>  
> +void arm64_skip_faulting_instruction(struct pt_regs *regs, unsigned long 
> size)
> +{
> + __arm64_skip_faulting_instruction(regs, size);
>   /*
>* If we were single stepping, we want to get the step exception after
>* we return from the trap.
> @@ -959,7 +964,7 @@ static int bug_handler(struct pt_regs *regs, unsigned int 
> esr)
>   }
>  
>   /* If thread survives, skip over the BUG instruction and continue: */
> - arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE);
> + __arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE);

Why do you want to avoid the single-step logic here? Given that we're
skipping over the brk instruction, why wouldn't you want that to trigger
a step exception if single-step is enabled?

Will


Re: [PATCH v2] Documentation: admin-guide: PM: Add cpuidle document

2018-11-29 Thread Ulf Hansson
On Thu, 29 Nov 2018 at 11:28, Rafael J. Wysocki  wrote:
>
> On Thu, Nov 29, 2018 at 9:07 AM Ulf Hansson  wrote:
> >
> > On Wed, 28 Nov 2018 at 12:47, Rafael J. Wysocki  wrote:
> > >
> > > From: Rafael J. Wysocki 
> > >
> > > Important information is missing from user/admin cpuidle documentation
> > > available today, so add a new user/admin document for cpuidle containing
> > > current and comprehensive information to admin-guide and drop the old
> > > .txt documents it is replacing.
> > >
> > > Signed-off-by: Rafael J. Wysocki 
> > > ---
> >
> > Wow! Great work!
> >
> > [...]
> >
> > > +.. _idle-states-representation:
> > > +
> > > +Representation of Idle States
> > > +=
> > > +
> > > +For the CPU idle time management purposes all of the physical idle states
> > > +supported by the processor have to be represented as a one-dimensional 
> > > array of
> > > +|struct cpuidle_state| objects each allowing an individual (logical) CPU 
> > > to ask
> > > +the processor hardware to enter an idle state of certain properties.  If 
> > > there
> > > +is a hierarchy of units in the processor, one |struct cpuidle_state| 
> > > object can
> > > +cover a combination of idle states supported by the units at different 
> > > levels of
> > > +the hierarchy.  In that case, the `target residency and exit latency 
> > > parameters
> > > +of it `_, must reflect the properties of the idle state at 
> > > the
> > > +deepest level (i.e. the idle state of the unit containing all of the 
> > > other
> > > +units).
> > > +
> > > +For example, take a processor with two cores in a larger unit referred 
> > > to as
> > > +a "module" and suppose that asking the hardware to enter a specific idle 
> > > state
> > > +(say "X") at the "core" level by one core will trigger the module to try 
> > > to
> > > +enter a specific idle state of its own (say "MX") if the other core is 
> > > in idle
> > > +state "X" already.  In other words, asking for idle state "X" at the 
> > > "core"
> > > +level gives the hardware a license to go as deep as to idle state "MX" 
> > > at the
> > > +"module" level, but there is no guarantee that this is going to happen 
> > > (the core
> > > +asking for idle state "X" may just end up in that state by itself 
> > > instead).
> >
> > This is a good description of a HW like x86 and ARM64 (using PSCI in
> > platform coordinated mode).
> >
> > However, for "legacy" ARM products, having at least two cores in a
> > module, this description does in many cases *not* match how the HW
> > works. I wonder about other architectures.
>
> Well, this is an example and it is very clearly presented this way IMO
> (it says "for example" upfront and then "suppose" etc).
>
> Basically, the purpose of it is to explain that the parameters shown
> in sysfs may be effective.
>
> > My point is, I think it would be fair to mention that the above
> > description is specific to certain HWs. In other words, for HWs that
> > leaves the entire state synchronization of the last man standing
> > algorithm to be managed by a FW, the cpuidle framework plays well. But
> > for others it has limitations.
>
> I can add it after this entire paragraph I think.  Something like
>
> "There are processors without direct coordination between different
> levels of the hierarchy of units inside them, however.  In those cases
> asking for an idle state at the "core" level does not automatically
> affect the "module" level, for example, in any way and the CPUIdle
> driver is responsible for the entire handling of the hierarchy.  Then,
> the definition of the idle state objects is entirely up to the driver,
> but still the physical properties of the idle state that the processor
> hardware finally goes into must always follow the parameters used by
> the governor for idle state selection (for instance, the actual exit
> latency of that idle state must not exceed the exit latency parameter
> of the idle state object selected by the governor)."

I like this! This leaves some room for understanding that cpuidle has
limitations when in comes down to these kind of platforms, which seems
like reasonable statement to include.

With that, feel free to add:
Reviewed-by: Ulf Hansson 

>
> >
> > > +Then, the target residency of the |struct cpuidle_state| object 
> > > representing
> > > +idle state "X" must reflect the minimum time to spend in idle state "MX" 
> > > of
> > > +the module (including the time needed to enter it), because that is the 
> > > minimum
> > > +time the CPU needs to be idle to save any energy in case the hardware 
> > > enters
> > > +that state.  Analogously, the exit latency parameter of that object must 
> > > cover
> > > +the exit time of idle state "MX" of the module (and usually its entry 
> > > time too),
> > > +because that is the maximum delay between a wakeup signal and the time 
> > > the CPU
> > > +will start to execute the first new instruction (assuming that both 
> > > cores in the
> > > +module will always be

Re: [PATCH v8 0/8] arm64: untag user pointers passed to the kernel

2018-11-29 Thread Catalin Marinas
Hi Andrey,

On Thu, Nov 08, 2018 at 03:48:10PM +0100, Andrey Konovalov wrote:
> On Thu, Nov 8, 2018 at 3:36 PM, Andrey Konovalov  
> wrote:
> > Changes in v8:
> > - Rebased onto 65102238 (4.20-rc1).
> > - Added a note to the cover letter on why syscall wrappers/shims that untag
> >   user pointers won't work.
> > - Added a note to the cover letter that this patchset has been merged into
> >   the Pixel 2 kernel tree.
> > - Documentation fixes, in particular added a list of syscalls that don't
> >   support tagged user pointers.
> 
> I've changed the documentation to be more specific, please take a look.
> 
> I haven't done anything about adding a way for the user to find out
> that the kernel supports this ABI extension. I don't know what would
> the the preferred way to do this, and we haven't received any comments
> on that from anybody else. Probing "on some innocuous syscall
> currently returning -EFAULT on tagged pointer arguments" works though,
> as you mentioned.

We've had some internal discussions and also talked to some people at
Plumbers. I think the best option is to introduce an AT_FLAGS bit to
describe the ABI relaxation on tagged pointers. Vincenzo is going to
propose a patch on top of this series.

> As mentioned in the cover letter, this patchset has been merged into
> the Pixel 2 kernel tree.

I just hope it's not enabled on production kernels, it would introduce
a user ABI that may differ from what ends up upstream.

-- 
Catalin


Re: [PATCH v8 1/8] arm64: add type casts to untagged_addr macro

2018-11-29 Thread Catalin Marinas
On Thu, Nov 08, 2018 at 03:36:08PM +0100, Andrey Konovalov wrote:
> This patch makes the untagged_addr macro accept all kinds of address types
> (void *, unsigned long, etc.) and allows not to specify type casts in each
> place where it is used. This is done by using __typeof__.
> 
> Signed-off-by: Andrey Konovalov 
> ---
>  arch/arm64/include/asm/uaccess.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/asm/uaccess.h 
> b/arch/arm64/include/asm/uaccess.h
> index 07c34087bd5e..c1325271e368 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -101,7 +101,8 @@ static inline unsigned long __range_ok(const void __user 
> *addr, unsigned long si
>   * up with a tagged userland pointer. Clear the tag to get a sane pointer to
>   * pass on to access_ok(), for instance.
>   */
> -#define untagged_addr(addr)  sign_extend64(addr, 55)
> +#define untagged_addr(addr)  \
> + ((__typeof__(addr))sign_extend64((__u64)(addr), 55))

Nitpick: same comment as here (use u64):

http://lkml.kernel.org/r/20181123173739.osgvnnhmptdgt...@lakrids.cambridge.arm.com

Acked-by: Catalin Marinas 

(not acking the whole series just yet, only specific patches to remember
what I reviewed)


Re: [PATCH v8 2/8] uaccess: add untagged_addr definition for other arches

2018-11-29 Thread Catalin Marinas
On Thu, Nov 08, 2018 at 03:36:09PM +0100, Andrey Konovalov wrote:
> diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
> index efe79c1cdd47..c045b4eff95e 100644
> --- a/include/linux/uaccess.h
> +++ b/include/linux/uaccess.h
> @@ -13,6 +13,10 @@
>  
>  #include 
>  
> +#ifndef untagged_addr
> +#define untagged_addr(addr) addr
> +#endif

Nitpick: add braces around (addr). Otherwise:

Acked-by: Catalin Marinas 


Re: [PATCH v8 3/8] arm64: untag user addresses in access_ok and __uaccess_mask_ptr

2018-11-29 Thread Catalin Marinas
On Thu, Nov 08, 2018 at 03:36:10PM +0100, Andrey Konovalov wrote:
> copy_from_user (and a few other similar functions) are used to copy data
> from user memory into the kernel memory or vice versa. Since a user can
> provided a tagged pointer to one of the syscalls that use copy_from_user,
> we need to correctly handle such pointers.
> 
> Do this by untagging user pointers in access_ok and in __uaccess_mask_ptr,
> before performing access validity checks.
> 
> Signed-off-by: Andrey Konovalov 

Reviewed-by: Catalin Marinas 


[PATCH v2] drm: TODO: Add DRM_MODESET_LOCK_ALL* conversion to todo.rst

2018-11-29 Thread Sean Paul
From: Sean Paul 

We should also get the drivers using the helpers. Seems like a good
intermediate TODO item.

Changes in v2:
- Add line about replacing drm_modest_lock_all() (Daniel)

Acked-by: Daniel Vetter 
Signed-off-by: Sean Paul 
---
 Documentation/gpu/todo.rst | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index ab347dec50796..14191b64446df 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -241,6 +241,21 @@ struct drm_gem_object_funcs
 GEM objects can now have a function table instead of having the callbacks on 
the
 DRM driver struct. This is now the preferred way and drivers can be moved over.
 
+Use DRM_MODESET_LOCK_ALL_* helpers instead of boilerplate
+-
+
+For cases where drivers are attempting to grab the modeset locks with a local
+acquire context. Replace the boilerplate code surrounding
+drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN() and
+DRM_MODESET_LOCK_ALL_END() instead.
+
+This should also be done for all places where drm_modest_lock_all() is still
+used.
+
+As a reference, take a look at the conversions already completed in drm core.
+
+Contact: Sean Paul, respective driver maintainers
+
 Core refactorings
 =
 
-- 
Sean Paul, Software Engineer, Google / Chromium OS



Re: [PATCH v2] drm: TODO: Add DRM_MODESET_LOCK_ALL* conversion to todo.rst

2018-11-29 Thread Sean Paul
On Thu, Nov 29, 2018 at 02:58:33PM -0500, Sean Paul wrote:
> From: Sean Paul 
> 
> We should also get the drivers using the helpers. Seems like a good
> intermediate TODO item.
> 
> Changes in v2:
> - Add line about replacing drm_modest_lock_all() (Daniel)
> 
> Acked-by: Daniel Vetter 

Pushed this to -misc-next

> Signed-off-by: Sean Paul 
> ---
>  Documentation/gpu/todo.rst | 15 +++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index ab347dec50796..14191b64446df 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -241,6 +241,21 @@ struct drm_gem_object_funcs
>  GEM objects can now have a function table instead of having the callbacks on 
> the
>  DRM driver struct. This is now the preferred way and drivers can be moved 
> over.
>  
> +Use DRM_MODESET_LOCK_ALL_* helpers instead of boilerplate
> +-
> +
> +For cases where drivers are attempting to grab the modeset locks with a local
> +acquire context. Replace the boilerplate code surrounding
> +drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN() and
> +DRM_MODESET_LOCK_ALL_END() instead.
> +
> +This should also be done for all places where drm_modest_lock_all() is still
> +used.
> +
> +As a reference, take a look at the conversions already completed in drm core.
> +
> +Contact: Sean Paul, respective driver maintainers
> +
>  Core refactorings
>  =
>  
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS


Re: [PATCH v8 04/14] integrity: Introduce struct evm_xattr

2018-11-29 Thread Mimi Zohar
On Fri, 2018-11-16 at 18:07 -0200, Thiago Jung Bauermann wrote:
> Even though struct evm_ima_xattr_data includes a fixed-size array to hold a
> SHA1 digest, most of the code ignores the array and uses the struct to mean
> "type indicator followed by data of unspecified size" and tracks the real
> size of what the struct represents in a separate length variable.
> 
> The only exception to that is the EVM code, which correctly uses the
> definition of struct evm_ima_xattr_data.
> 
> So make this explicit in the code by removing the length specification from
> the array in struct evm_ima_xattr_data. Also, change the name of the
> element from digest to data since in most places the array doesn't hold a
> digest.
> 
> A separate struct evm_xattr is introduced, with the original definition of
> evm_ima_xattr_data to be used in the places that actually expect that
> definition.

, specifically the EVM HMAC code.

> 
> Signed-off-by: Thiago Jung Bauermann 

Other than commenting the evm_xattr usage is limited to HMAC before
the structure definition, this looks good.

Reviewed-by: Mimi Zohar 

> ---
>  security/integrity/evm/evm_main.c | 8 
>  security/integrity/ima/ima_appraise.c | 7 ---
>  security/integrity/integrity.h| 5 +
>  3 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/security/integrity/evm/evm_main.c 
> b/security/integrity/evm/evm_main.c
> index 7f3f54d89a6e..a1b42d10efc7 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -169,7 +169,7 @@ static enum integrity_status evm_verify_hmac(struct 
> dentry *dentry,
>   /* check value type */
>   switch (xattr_data->type) {
>   case EVM_XATTR_HMAC:
> - if (xattr_len != sizeof(struct evm_ima_xattr_data)) {
> + if (xattr_len != sizeof(struct evm_xattr)) {
>   evm_status = INTEGRITY_FAIL;
>   goto out;
>   }
> @@ -179,7 +179,7 @@ static enum integrity_status evm_verify_hmac(struct 
> dentry *dentry,
>  xattr_value_len, &digest);
>   if (rc)
>   break;
> - rc = crypto_memneq(xattr_data->digest, digest.digest,
> + rc = crypto_memneq(xattr_data->data, digest.digest,
>  SHA1_DIGEST_SIZE);
>   if (rc)
>   rc = -EINVAL;
> @@ -523,7 +523,7 @@ int evm_inode_init_security(struct inode *inode,
>const struct xattr *lsm_xattr,
>struct xattr *evm_xattr)
>  {
> - struct evm_ima_xattr_data *xattr_data;
> + struct evm_xattr *xattr_data;
>   int rc;
>  
>   if (!evm_key_loaded() || !evm_protected_xattr(lsm_xattr->name))
> @@ -533,7 +533,7 @@ int evm_inode_init_security(struct inode *inode,
>   if (!xattr_data)
>   return -ENOMEM;
>  
> - xattr_data->type = EVM_XATTR_HMAC;
> + xattr_data->data.type = EVM_XATTR_HMAC;
>   rc = evm_init_hmac(inode, lsm_xattr, xattr_data->digest);
>   if (rc < 0)
>   goto out;
> diff --git a/security/integrity/ima/ima_appraise.c 
> b/security/integrity/ima/ima_appraise.c
> index deec1804a00a..8bcef90939f8 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -167,7 +167,8 @@ enum hash_algo ima_get_hash_algo(struct 
> evm_ima_xattr_data *xattr_value,
>   return sig->hash_algo;
>   break;
>   case IMA_XATTR_DIGEST_NG:
> - ret = xattr_value->digest[0];
> + /* first byte contains algorithm id */
> + ret = xattr_value->data[0];
>   if (ret < HASH_ALGO__LAST)
>   return ret;
>   break;
> @@ -175,7 +176,7 @@ enum hash_algo ima_get_hash_algo(struct 
> evm_ima_xattr_data *xattr_value,
>   /* this is for backward compatibility */
>   if (xattr_len == 21) {
>   unsigned int zero = 0;
> - if (!memcmp(&xattr_value->digest[16], &zero, 4))
> + if (!memcmp(&xattr_value->data[16], &zero, 4))
>   return HASH_ALGO_MD5;
>   else
>   return HASH_ALGO_SHA1;
> @@ -274,7 +275,7 @@ int ima_appraise_measurement(enum ima_hooks func,
>   /* xattr length may be longer. md5 hash in previous
>  version occupied 20 bytes in xattr, instead of 16
>*/
> - rc = memcmp(&xattr_value->digest[hash_start],
> + rc = memcmp(&xattr_value->data[hash_start],
>   iint->ima_hash->digest,
>   iint->ima_hash->length);
>   else
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index e60473b13a8d..20ac02bf1b84 100644
> --- a/securit

[PATCH v3] Documentation: admin-guide: PM: Add cpuidle document

2018-11-29 Thread Rafael J. Wysocki
From: Rafael J. Wysocki 

Important information is missing from user/admin cpuidle documentation
available today, so add a new user/admin document for cpuidle containing
current and comprehensive information to admin-guide and drop the old
.txt documents it is replacing.

Signed-off-by: Rafael J. Wysocki 
Reviewed-by: Viresh Kumar 
Reviewed-by: Ulf Hansson 
---

v2 -> v3:
   * Add paragraph covering processors without automatic coordination between
 different herarchy levels in hardware (Ulf).
   * Add Reviewed-by tags from Viresh and Ulf.

-> v2:
   * Fix typos.
   * Improve description of the state0, state1, etc directories to cover the
 relationship between their numbers and the represented idle states depth
 (Viresh).

---
 Documentation/admin-guide/pm/cpuidle.rst   |  615 +
 Documentation/admin-guide/pm/working-state.rst |1 
 Documentation/cpuidle/core.txt |   23 
 Documentation/cpuidle/sysfs.txt|   98 ---
 4 files changed, 616 insertions(+), 121 deletions(-)

Index: linux-pm/Documentation/admin-guide/pm/cpuidle.rst
===
--- /dev/null
+++ linux-pm/Documentation/admin-guide/pm/cpuidle.rst
@@ -0,0 +1,615 @@
+.. |struct cpuidle_state| replace:: :c:type:`struct cpuidle_state 
`
+.. |cpufreq| replace:: :doc:`CPU Performance Scaling `
+
+
+CPU Idle Time Management
+
+
+::
+
+ Copyright (c) 2018 Intel Corp., Rafael J. Wysocki 
+
+Concepts
+
+
+Modern processors are generally able to enter states in which the execution of
+a program is suspended and instructions belonging to it are not fetched from
+memory or executed.  Those states are the *idle* states of the processor.
+
+Since part of the processor hardware is not used in idle states, entering them
+generally allows power drawn by the processor to be reduced and, in 
consequence,
+it is an opportunity to save energy.
+
+CPU idle time management is an energy-efficiency feature concerned about using
+the idle states of processors for this purpose.
+
+Logical CPUs
+
+
+CPU idle time management operates on CPUs as seen by the *CPU scheduler* (that
+is the part of the kernel responsible for the distribution of computational
+work in the system).  In its view, CPUs are *logical* units.  That is, they 
need
+not be separate physical entities and may just be interfaces appearing to
+software as individual single-core processors.  In other words, a CPU is an
+entity which appears to be fetching instructions that belong to one sequence
+(program) from memory and executing them, but it need not work this way
+physically.  Generally, three different cases can be consider here.
+
+First, if the whole processor can only follow one sequence of instructions (one
+program) at a time, it is a CPU.  In that case, if the hardware is asked to
+enter an idle state, that applies to the processor as a whole.
+
+Second, if the processor is multi-core, each core in it is able to follow at
+least one program at a time.  The cores need not be entirely independent of 
each
+other (for example, they may share caches), but still most of the time they
+work physically in parallel with each other, so if each of them executes only
+one program, those programs run mostly independently of each other at the same
+time.  The entire cores are CPUs in that case and if the hardware is asked to
+enter an idle state, that applies to the core that asked for it in the first
+place, but it also may apply to a larger unit (say a "package" or a "cluster")
+that the core belongs to (in fact, it may apply to an entire hierarchy of 
larger
+units containing the core).  Namely, if all of the cores in the larger unit
+except for one have been put into idle states at the "core level" and the
+remaining core asks the processor to enter an idle state, that may trigger it
+to put the whole larger unit into an idle state which also will affect the
+other cores in that unit.
+
+Finally, each core in a multi-core processor may be able to follow more than 
one
+program in the same time frame (that is, each core may be able to fetch
+instructions from multiple locations in memory and execute them in the same 
time
+frame, but not necessarily entirely in parallel with each other).  In that case
+the cores present themselves to software as "bundles" each consisting of
+multiple individual single-core "processors", referred to as *hardware threads*
+(or hyper-threads specifically on Intel hardware), that each can follow one
+sequence of instructions.  Then, the hardware threads are CPUs from the CPU 
idle
+time management perspective and if the processor is asked to enter an idle 
state
+by one of them, the hardware thread (or CPU) that asked for it is stopped, but
+nothing more happens, unless all of the other hardware threads within the same
+core also have asked the processor to enter an idle state.  In that situ

Re: [PATCH v3] Documentation: admin-guide: PM: Add cpuidle document

2018-11-29 Thread Jonathan Corbet
On Thu, 29 Nov 2018 22:22:04 +0100
"Rafael J. Wysocki"  wrote:

> Important information is missing from user/admin cpuidle documentation
> available today, so add a new user/admin document for cpuidle containing
> current and comprehensive information to admin-guide and drop the old
> .txt documents it is replacing.

I've not had a chance to read through it in depth, but a first scan looks
good.  I assume this is going up via your tree?

Thanks,

jon


Re: [PATCH v3] Documentation: admin-guide: PM: Add cpuidle document

2018-11-29 Thread Rafael J. Wysocki
On Thu, Nov 29, 2018 at 10:40 PM Jonathan Corbet  wrote:
>
> On Thu, 29 Nov 2018 22:22:04 +0100
> "Rafael J. Wysocki"  wrote:
>
> > Important information is missing from user/admin cpuidle documentation
> > available today, so add a new user/admin document for cpuidle containing
> > current and comprehensive information to admin-guide and drop the old
> > .txt documents it is replacing.
>
> I've not had a chance to read through it in depth, but a first scan looks
> good.

Thanks!

>  I assume this is going up via your tree?

Yes, I would like to take it as there may be new patches on top of it.

Cheers,
Rafael