drivers/gpu//drm/i915/i915_gem_gtt.c:947: error: 'iter.sg' is used uninitialized in this function

[kbuild test robot]

Hi Chris,

FYI, the error/warning still remains.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   249f1efd8e3d58f86469fc305b0eda39db18d7ce
commit: 894ccebee2b0e606ba9638d20dd87b33568482d7 drm/i915: Micro-optimise 
gen8_ppgtt_insert_entries()
date:   3 months ago
config: x86_64-randconfig-b0-05281108 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
git checkout 894ccebee2b0e606ba9638d20dd87b33568482d7
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   cc1: warnings being treated as errors
   drivers/gpu//drm/i915/i915_gem_gtt.c: In function 'gen8_ppgtt_insert_3lvl':
>> drivers/gpu//drm/i915/i915_gem_gtt.c:947: error: 'iter.sg' is used 
>> uninitialized in this function
>> drivers/gpu//drm/i915/i915_gem_gtt.c:948: error: 'iter.dma' is used 
>> uninitialized in this function
   drivers/gpu//drm/i915/i915_gem_gtt.c: In function 'gen8_ppgtt_insert_4lvl':
   drivers/gpu//drm/i915/i915_gem_gtt.c:964: error: 'iter.sg' is used 
uninitialized in this function
   drivers/gpu//drm/i915/i915_gem_gtt.c:965: error: 'iter.dma' is used 
uninitialized in this function

vim +947 drivers/gpu//drm/i915/i915_gem_gtt.c

   941 enum i915_cache_level cache_level,
   942 u32 unused)
   943  {
   944  struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
   945  struct sgt_dma iter = {
   946  .sg = pages->sgl,
 > 947  .dma = sg_dma_address(iter.sg),
 > 948  .max = iter.dma + iter.sg->length,
   949  };
   950  
   951  gen8_ppgtt_insert_pte_entries(ppgtt, >pdp, ,

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

drivers/gpu//drm/i915/i915_gem_gtt.c:947: error: 'iter.sg' is used uninitialized in this function

[kbuild test robot]

Hi Chris,

FYI, the error/warning still remains.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   249f1efd8e3d58f86469fc305b0eda39db18d7ce
commit: 894ccebee2b0e606ba9638d20dd87b33568482d7 drm/i915: Micro-optimise 
gen8_ppgtt_insert_entries()
date:   3 months ago
config: x86_64-randconfig-b0-05281108 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
git checkout 894ccebee2b0e606ba9638d20dd87b33568482d7
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   cc1: warnings being treated as errors
   drivers/gpu//drm/i915/i915_gem_gtt.c: In function 'gen8_ppgtt_insert_3lvl':
>> drivers/gpu//drm/i915/i915_gem_gtt.c:947: error: 'iter.sg' is used 
>> uninitialized in this function
>> drivers/gpu//drm/i915/i915_gem_gtt.c:948: error: 'iter.dma' is used 
>> uninitialized in this function
   drivers/gpu//drm/i915/i915_gem_gtt.c: In function 'gen8_ppgtt_insert_4lvl':
   drivers/gpu//drm/i915/i915_gem_gtt.c:964: error: 'iter.sg' is used 
uninitialized in this function
   drivers/gpu//drm/i915/i915_gem_gtt.c:965: error: 'iter.dma' is used 
uninitialized in this function

vim +947 drivers/gpu//drm/i915/i915_gem_gtt.c

   941 enum i915_cache_level cache_level,
   942 u32 unused)
   943  {
   944  struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
   945  struct sgt_dma iter = {
   946  .sg = pages->sgl,
 > 947  .dma = sg_dma_address(iter.sg),
 > 948  .max = iter.dma + iter.sg->length,
   949  };
   950  
   951  gen8_ppgtt_insert_pte_entries(ppgtt, >pdp, ,

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v5 1/2] mfd: intel_soc_pmic: Select designware i2c-bus driver

[kbuild test robot]

Hi Hans,

[auto build test ERROR on ljones-mfd/for-mfd-next]
[also build test ERROR on v4.12-rc2 next-20170526]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Hans-de-Goede/mfd-intel_soc_pmic-Select-designware-i2c-bus-driver/20170526-211338
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: frv-allmodconfig (attached as .config)
compiler: frv-linux-gcc (GCC) 6.2.0
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=frv 

All errors (new ones prefixed by >>):

>> drivers//clocksource/timer-sun5i.c:52:21: error: field 'clksrc' has 
>> incomplete type
 struct clocksource clksrc;
^~
>> drivers//clocksource/timer-sun5i.c:60:28: error: field 'clkevt' has 
>> incomplete type
 struct clock_event_device clkevt;
   ^~
   In file included from include/linux/clk.h:16:0,
from drivers//clocksource/timer-sun5i.c:13:
   drivers//clocksource/timer-sun5i.c: In function 'sun5i_clkevt_shutdown':
   include/linux/kernel.h:854:48: error: initialization from incompatible 
pointer type [-Werror=incompatible-pointer-types]
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
   drivers//clocksource/timer-sun5i.c:64:2: note: in expansion of macro 
'container_of'
 container_of(x, struct sun5i_timer_clkevt, clkevt)
 ^~~~
   drivers//clocksource/timer-sun5i.c:108:34: note: in expansion of macro 
'to_sun5i_timer_clkevt'
 struct sun5i_timer_clkevt *ce = to_sun5i_timer_clkevt(clkevt);
 ^
   include/linux/kernel.h:854:48: note: (near initialization for 'ce')
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
   drivers//clocksource/timer-sun5i.c:64:2: note: in expansion of macro 
'container_of'
 container_of(x, struct sun5i_timer_clkevt, clkevt)
 ^~~~
   drivers//clocksource/timer-sun5i.c:108:34: note: in expansion of macro 
'to_sun5i_timer_clkevt'
 struct sun5i_timer_clkevt *ce = to_sun5i_timer_clkevt(clkevt);
 ^
   drivers//clocksource/timer-sun5i.c: In function 'sun5i_clkevt_set_oneshot':
   include/linux/kernel.h:854:48: error: initialization from incompatible 
pointer type [-Werror=incompatible-pointer-types]
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
   drivers//clocksource/timer-sun5i.c:64:2: note: in expansion of macro 
'container_of'
 container_of(x, struct sun5i_timer_clkevt, clkevt)
 ^~~~
   drivers//clocksource/timer-sun5i.c:116:34: note: in expansion of macro 
'to_sun5i_timer_clkevt'
 struct sun5i_timer_clkevt *ce = to_sun5i_timer_clkevt(clkevt);
 ^
   include/linux/kernel.h:854:48: note: (near initialization for 'ce')
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
   drivers//clocksource/timer-sun5i.c:64:2: note: in expansion of macro 
'container_of'
 container_of(x, struct sun5i_timer_clkevt, clkevt)
 ^~~~
   drivers//clocksource/timer-sun5i.c:116:34: note: in expansion of macro 
'to_sun5i_timer_clkevt'
 struct sun5i_timer_clkevt *ce = to_sun5i_timer_clkevt(clkevt);
 ^
   drivers//clocksource/timer-sun5i.c: In function 'sun5i_clkevt_set_periodic':
   include/linux/kernel.h:854:48: error: initialization from incompatible 
pointer type [-Werror=incompatible-pointer-types]
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
   drivers//clocksource/timer-sun5i.c:64:2: note: in expansion of macro 
'container_of'
 container_of(x, struct sun5i_timer_clkevt, clkevt)
 ^~~~
   drivers//clocksource/timer-sun5i.c:125:34: note: in expansion of macro 
'to_sun5i_timer_clkevt'
 struct sun5i_timer_clkevt *ce = to_sun5i_timer_clkevt(clkevt);
 ^
   include/linux/kernel.h:854:48: note: (near initialization for 'ce')
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
   drivers//clocksource/timer-sun5i.c:64:2: note: in expansion of macro 
'container_of'
 container_of(x, struct sun5i_timer_clkevt, clkevt)
 ^~~~
   drivers//clocksource/timer-sun5i.c:125:34: note: in expansion of macro 
'to_sun5i_timer_clkevt'
 struct sun5i_timer_clkevt *ce = to_sun5i_timer_clkevt(clkevt);
 

Re: [PATCH v5 1/2] mfd: intel_soc_pmic: Select designware i2c-bus driver

[kbuild test robot]

Hi Hans,

[auto build test ERROR on ljones-mfd/for-mfd-next]
[also build test ERROR on v4.12-rc2 next-20170526]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Hans-de-Goede/mfd-intel_soc_pmic-Select-designware-i2c-bus-driver/20170526-211338
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: frv-allmodconfig (attached as .config)
compiler: frv-linux-gcc (GCC) 6.2.0
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=frv 

All errors (new ones prefixed by >>):

>> drivers//clocksource/timer-sun5i.c:52:21: error: field 'clksrc' has 
>> incomplete type
 struct clocksource clksrc;
^~
>> drivers//clocksource/timer-sun5i.c:60:28: error: field 'clkevt' has 
>> incomplete type
 struct clock_event_device clkevt;
   ^~
   In file included from include/linux/clk.h:16:0,
from drivers//clocksource/timer-sun5i.c:13:
   drivers//clocksource/timer-sun5i.c: In function 'sun5i_clkevt_shutdown':
   include/linux/kernel.h:854:48: error: initialization from incompatible 
pointer type [-Werror=incompatible-pointer-types]
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
   drivers//clocksource/timer-sun5i.c:64:2: note: in expansion of macro 
'container_of'
 container_of(x, struct sun5i_timer_clkevt, clkevt)
 ^~~~
   drivers//clocksource/timer-sun5i.c:108:34: note: in expansion of macro 
'to_sun5i_timer_clkevt'
 struct sun5i_timer_clkevt *ce = to_sun5i_timer_clkevt(clkevt);
 ^
   include/linux/kernel.h:854:48: note: (near initialization for 'ce')
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
   drivers//clocksource/timer-sun5i.c:64:2: note: in expansion of macro 
'container_of'
 container_of(x, struct sun5i_timer_clkevt, clkevt)
 ^~~~
   drivers//clocksource/timer-sun5i.c:108:34: note: in expansion of macro 
'to_sun5i_timer_clkevt'
 struct sun5i_timer_clkevt *ce = to_sun5i_timer_clkevt(clkevt);
 ^
   drivers//clocksource/timer-sun5i.c: In function 'sun5i_clkevt_set_oneshot':
   include/linux/kernel.h:854:48: error: initialization from incompatible 
pointer type [-Werror=incompatible-pointer-types]
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
   drivers//clocksource/timer-sun5i.c:64:2: note: in expansion of macro 
'container_of'
 container_of(x, struct sun5i_timer_clkevt, clkevt)
 ^~~~
   drivers//clocksource/timer-sun5i.c:116:34: note: in expansion of macro 
'to_sun5i_timer_clkevt'
 struct sun5i_timer_clkevt *ce = to_sun5i_timer_clkevt(clkevt);
 ^
   include/linux/kernel.h:854:48: note: (near initialization for 'ce')
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
   drivers//clocksource/timer-sun5i.c:64:2: note: in expansion of macro 
'container_of'
 container_of(x, struct sun5i_timer_clkevt, clkevt)
 ^~~~
   drivers//clocksource/timer-sun5i.c:116:34: note: in expansion of macro 
'to_sun5i_timer_clkevt'
 struct sun5i_timer_clkevt *ce = to_sun5i_timer_clkevt(clkevt);
 ^
   drivers//clocksource/timer-sun5i.c: In function 'sun5i_clkevt_set_periodic':
   include/linux/kernel.h:854:48: error: initialization from incompatible 
pointer type [-Werror=incompatible-pointer-types]
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
   drivers//clocksource/timer-sun5i.c:64:2: note: in expansion of macro 
'container_of'
 container_of(x, struct sun5i_timer_clkevt, clkevt)
 ^~~~
   drivers//clocksource/timer-sun5i.c:125:34: note: in expansion of macro 
'to_sun5i_timer_clkevt'
 struct sun5i_timer_clkevt *ce = to_sun5i_timer_clkevt(clkevt);
 ^
   include/linux/kernel.h:854:48: note: (near initialization for 'ce')
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
   drivers//clocksource/timer-sun5i.c:64:2: note: in expansion of macro 
'container_of'
 container_of(x, struct sun5i_timer_clkevt, clkevt)
 ^~~~
   drivers//clocksource/timer-sun5i.c:125:34: note: in expansion of macro 
'to_sun5i_timer_clkevt'
 struct sun5i_timer_clkevt *ce = to_sun5i_timer_clkevt(clkevt);
 

drivers/gpu/drm/i915/selftests/i915_gem_request.c:280: error: 'request' may be used uninitialized in this function

[kbuild test robot]

Hi Chris,

FYI, the error/warning still remains.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   249f1efd8e3d58f86469fc305b0eda39db18d7ce
commit: b348090d6758cc391dc91f8a8233b18f0acc8458 drm/i915: Simple selftest to 
exercise live requests
date:   3 months ago
config: x86_64-randconfig-b0-05281108 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
git checkout b348090d6758cc391dc91f8a8233b18f0acc8458
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All errors (new ones prefixed by >>):

   cc1: warnings being treated as errors
   In file included from drivers/gpu/drm/i915/i915_gem_request.c:1204:
   drivers/gpu/drm/i915/selftests/i915_gem_request.c: In function 
'live_nop_request':
>> drivers/gpu/drm/i915/selftests/i915_gem_request.c:280: error: 'request' may 
>> be used uninitialized in this function

vim +/request +280 drivers/gpu/drm/i915/selftests/i915_gem_request.c

   274   */
   275  
   276  mutex_lock(>drm.struct_mutex);
   277  
   278  for_each_engine(engine, i915, id) {
   279  IGT_TIMEOUT(end_time);
 > 280  struct drm_i915_gem_request *request;
   281  unsigned long n, prime;
   282  ktime_t times[2] = {};
   283  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

drivers/gpu/drm/i915/selftests/i915_gem_request.c:280: error: 'request' may be used uninitialized in this function

[kbuild test robot]

Hi Chris,

FYI, the error/warning still remains.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   249f1efd8e3d58f86469fc305b0eda39db18d7ce
commit: b348090d6758cc391dc91f8a8233b18f0acc8458 drm/i915: Simple selftest to 
exercise live requests
date:   3 months ago
config: x86_64-randconfig-b0-05281108 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
git checkout b348090d6758cc391dc91f8a8233b18f0acc8458
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All errors (new ones prefixed by >>):

   cc1: warnings being treated as errors
   In file included from drivers/gpu/drm/i915/i915_gem_request.c:1204:
   drivers/gpu/drm/i915/selftests/i915_gem_request.c: In function 
'live_nop_request':
>> drivers/gpu/drm/i915/selftests/i915_gem_request.c:280: error: 'request' may 
>> be used uninitialized in this function

vim +/request +280 drivers/gpu/drm/i915/selftests/i915_gem_request.c

   274   */
   275  
   276  mutex_lock(>drm.struct_mutex);
   277  
   278  for_each_engine(engine, i915, id) {
   279  IGT_TIMEOUT(end_time);
 > 280  struct drm_i915_gem_request *request;
   281  unsigned long n, prime;
   282  ktime_t times[2] = {};
   283  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v4] sgi-xp: Use designated initializers

[Christoph Hellwig]

On Sat, May 27, 2017 at 05:39:14PM -0700, Kees Cook wrote:
> Prepare to mark sensitive kernel structures for randomization by making
> sure they're using designated initializers. In this case, no initializers
> are needed (they can be NULL initialized and callers adjusted to check
> for NULL, which is more efficient than an indirect call).

I'd say this looks good:

Reviewed-by: Christoph Hellwig 

Until I noticed all this is unused, so we should just remove it
entirely.  I now remember the weird SGI excuse, but given that they
haven't managed to get their code upstream in more than 10 years there
is no valid reason at all to keep this around.

Re: [PATCH v4] sgi-xp: Use designated initializers

[Christoph Hellwig]

On Sat, May 27, 2017 at 05:39:14PM -0700, Kees Cook wrote:
> Prepare to mark sensitive kernel structures for randomization by making
> sure they're using designated initializers. In this case, no initializers
> are needed (they can be NULL initialized and callers adjusted to check
> for NULL, which is more efficient than an indirect call).

I'd say this looks good:

Reviewed-by: Christoph Hellwig 

Until I noticed all this is unused, so we should just remove it
entirely.  I now remember the weird SGI excuse, but given that they
haven't managed to get their code upstream in more than 10 years there
is no valid reason at all to keep this around.

Re: [PATCH v2 11/20] randstruct: Disable randomization of ACPICA structs

[Christoph Hellwig]

On Sat, May 27, 2017 at 01:03:23PM -0700, Kees Cook wrote:
> On Sat, May 27, 2017 at 1:42 AM, Christoph Hellwig  wrote:
> > On Fri, May 26, 2017 at 01:17:15PM -0700, Kees Cook wrote:
> >> Since the ACPICA source is maintained externally to the kernel, we can
> >> neither switch it to designated initializers nor mark it
> >> __no_randomize_layout. Until ACPICA-upstream changes[1] land to handle the
> >> designated initialization, explicitly skip it in the plugin.
> >>
> >> [1] https://github.com/acpica/acpica/pull/248
> >
> > I'd just overried the ACPIA bullshit process and just include these
> > changes, as they are a major improvement independent of any
> > reandomization.
> 
> Well... I'd rather not. It's been explicitly NAKed by them already,
> which is why I sent the upstream solution (though it's being ignored
> currently).

Send it to linux-keernel again and we'll override their bulllshit.
We can make exceptions from our coding style and preference just because
they are idiots.  And the whole ACPI mess needs urgent de-obsfucation
anyway.

Re: [PATCH v2 11/20] randstruct: Disable randomization of ACPICA structs

[Christoph Hellwig]

On Sat, May 27, 2017 at 01:03:23PM -0700, Kees Cook wrote:
> On Sat, May 27, 2017 at 1:42 AM, Christoph Hellwig  wrote:
> > On Fri, May 26, 2017 at 01:17:15PM -0700, Kees Cook wrote:
> >> Since the ACPICA source is maintained externally to the kernel, we can
> >> neither switch it to designated initializers nor mark it
> >> __no_randomize_layout. Until ACPICA-upstream changes[1] land to handle the
> >> designated initialization, explicitly skip it in the plugin.
> >>
> >> [1] https://github.com/acpica/acpica/pull/248
> >
> > I'd just overried the ACPIA bullshit process and just include these
> > changes, as they are a major improvement independent of any
> > reandomization.
> 
> Well... I'd rather not. It's been explicitly NAKed by them already,
> which is why I sent the upstream solution (though it's being ignored
> currently).

Send it to linux-keernel again and we'll override their bulllshit.
We can make exceptions from our coding style and preference just because
they are idiots.  And the whole ACPI mess needs urgent de-obsfucation
anyway.

panel-sitronix-st7789v.c:undefined reference to `of_find_backlight_by_node'

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   249f1efd8e3d58f86469fc305b0eda39db18d7ce
commit: 7142afb3a186ed2cd028318205a4852f04283380 drm/panel: Add driver for 
sitronix ST7789V LCD controller
date:   7 weeks ago
config: x86_64-randconfig-s3-05280946 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
git checkout 7142afb3a186ed2cd028318205a4852f04283380
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/built-in.o: In function `s6e3ha2_remove':
   panel-samsung-s6e3ha2.c:(.text+0x1cc45f): undefined reference to 
`backlight_device_unregister'
   drivers/built-in.o: In function `s6e3ha2_probe':
   panel-samsung-s6e3ha2.c:(.text+0x1cd24c): undefined reference to 
`backlight_device_register'
   panel-samsung-s6e3ha2.c:(.text+0x1cd34d): undefined reference to 
`backlight_device_unregister'
   drivers/built-in.o: In function `st7789v_probe':
>> panel-sitronix-st7789v.c:(.text+0x1cef6c): undefined reference to 
>> `of_find_backlight_by_node'

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

panel-sitronix-st7789v.c:undefined reference to `of_find_backlight_by_node'

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   249f1efd8e3d58f86469fc305b0eda39db18d7ce
commit: 7142afb3a186ed2cd028318205a4852f04283380 drm/panel: Add driver for 
sitronix ST7789V LCD controller
date:   7 weeks ago
config: x86_64-randconfig-s3-05280946 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
git checkout 7142afb3a186ed2cd028318205a4852f04283380
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/built-in.o: In function `s6e3ha2_remove':
   panel-samsung-s6e3ha2.c:(.text+0x1cc45f): undefined reference to 
`backlight_device_unregister'
   drivers/built-in.o: In function `s6e3ha2_probe':
   panel-samsung-s6e3ha2.c:(.text+0x1cd24c): undefined reference to 
`backlight_device_register'
   panel-samsung-s6e3ha2.c:(.text+0x1cd34d): undefined reference to 
`backlight_device_unregister'
   drivers/built-in.o: In function `st7789v_probe':
>> panel-sitronix-st7789v.c:(.text+0x1cef6c): undefined reference to 
>> `of_find_backlight_by_node'

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH 6/7] clocksource: Rename CLKSRC_OF to TIMER_OF

[Shawn Guo]

On Sat, May 27, 2017 at 11:58:47AM +0200, Daniel Lezcano wrote:
> diff --git a/arch/arm/mach-imx/epit.c b/arch/arm/mach-imx/epit.c
> index fb9a73a..4a4d2e2 100644
> --- a/arch/arm/mach-imx/epit.c
> +++ b/arch/arm/mach-imx/epit.c
> @@ -39,7 +39,7 @@
>  #define EPITCR_OM_TOGGLE (1 << 22)
>  #define EPITCR_OM_CLEAR  (2 << 22)
>  #define EPITCR_OM_SET(3 << 22)
> -#define EPITCR_CLKSRC_OFF(0 << 24)
> +#define EPITCR_TIMER_OFF (0 << 24)
>  #define EPITCR_CLKSRC_PERIPHERAL (1 << 24)
>  #define EPITCR_CLKSRC_REF_HIGH   (1 << 24)
>  #define EPITCR_CLKSRC_REF_LOW(3 << 24)

I do not think this change should be there.

Shawn

Re: [PATCH 6/7] clocksource: Rename CLKSRC_OF to TIMER_OF

[Shawn Guo]

On Sat, May 27, 2017 at 11:58:47AM +0200, Daniel Lezcano wrote:
> diff --git a/arch/arm/mach-imx/epit.c b/arch/arm/mach-imx/epit.c
> index fb9a73a..4a4d2e2 100644
> --- a/arch/arm/mach-imx/epit.c
> +++ b/arch/arm/mach-imx/epit.c
> @@ -39,7 +39,7 @@
>  #define EPITCR_OM_TOGGLE (1 << 22)
>  #define EPITCR_OM_CLEAR  (2 << 22)
>  #define EPITCR_OM_SET(3 << 22)
> -#define EPITCR_CLKSRC_OFF(0 << 24)
> +#define EPITCR_TIMER_OFF (0 << 24)
>  #define EPITCR_CLKSRC_PERIPHERAL (1 << 24)
>  #define EPITCR_CLKSRC_REF_HIGH   (1 << 24)
>  #define EPITCR_CLKSRC_REF_LOW(3 << 24)

I do not think this change should be there.

Shawn

Re: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP counter

[kbuild test robot]

Hi Kan,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.12-rc2 next-20170526]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/kan-liang-intel-com/perf-x86-intel-enable-CPU-ref_cycles-for-GP-counter/20170522-111500
config: i386-randconfig-x0-05280944 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   arch/x86/built-in.o: In function `nhm_ref_cycles_rep':
>> (.text+0x64ff): undefined reference to `__udivdi3'

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP counter

[kbuild test robot]

Hi Kan,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.12-rc2 next-20170526]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/kan-liang-intel-com/perf-x86-intel-enable-CPU-ref_cycles-for-GP-counter/20170522-111500
config: i386-randconfig-x0-05280944 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   arch/x86/built-in.o: In function `nhm_ref_cycles_rep':
>> (.text+0x64ff): undefined reference to `__udivdi3'

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

panel-samsung-s6e3ha2.c:undefined reference to `backlight_device_unregister'

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   249f1efd8e3d58f86469fc305b0eda39db18d7ce
commit: ed29f9426d9bf1b5ec7e79dfb28f39de7b3a0069 drm/panel: Add support for 
S6E3HA2 panel driver on TM2 board
date:   7 weeks ago
config: x86_64-randconfig-s3-05280946 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
git checkout ed29f9426d9bf1b5ec7e79dfb28f39de7b3a0069
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/built-in.o: In function `s6e3ha2_remove':
>> panel-samsung-s6e3ha2.c:(.text+0x1cc45f): undefined reference to 
>> `backlight_device_unregister'
   drivers/built-in.o: In function `s6e3ha2_probe':
>> panel-samsung-s6e3ha2.c:(.text+0x1cd24c): undefined reference to 
>> `backlight_device_register'
   panel-samsung-s6e3ha2.c:(.text+0x1cd34d): undefined reference to 
`backlight_device_unregister'

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

panel-samsung-s6e3ha2.c:undefined reference to `backlight_device_unregister'

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   249f1efd8e3d58f86469fc305b0eda39db18d7ce
commit: ed29f9426d9bf1b5ec7e79dfb28f39de7b3a0069 drm/panel: Add support for 
S6E3HA2 panel driver on TM2 board
date:   7 weeks ago
config: x86_64-randconfig-s3-05280946 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
git checkout ed29f9426d9bf1b5ec7e79dfb28f39de7b3a0069
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/built-in.o: In function `s6e3ha2_remove':
>> panel-samsung-s6e3ha2.c:(.text+0x1cc45f): undefined reference to 
>> `backlight_device_unregister'
   drivers/built-in.o: In function `s6e3ha2_probe':
>> panel-samsung-s6e3ha2.c:(.text+0x1cd24c): undefined reference to 
>> `backlight_device_register'
   panel-samsung-s6e3ha2.c:(.text+0x1cd34d): undefined reference to 
`backlight_device_unregister'

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v5 3/3] drm: mali-dp: Add writeback connector

[kbuild test robot]

Hi Brian,

[auto build test ERROR on drm/drm-next]
[also build test ERROR on v4.12-rc2 next-20170526]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Liviu-Dudau/drm-mali-dp-Add-support-for-writeback-on-DP550-DP650/20170516-013246
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: arm-allyesconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   In file included from drivers/gpu/drm/arm/malidp_drv.c:31:0:
>> drivers/gpu/drm/arm/malidp_drv.h:16:31: fatal error: drm/drm_writeback.h: No 
>> such file or directory
#include 
  ^
   compilation terminated.
--
>> drivers/gpu/drm/arm/malidp_mw.c:19:31: fatal error: drm/drm_writeback.h: No 
>> such file or directory
#include 
  ^
   compilation terminated.

vim +16 drivers/gpu/drm/arm/malidp_drv.h

10   * ARM Mali DP500/DP550/DP650 KMS/DRM driver structures
11   */
12  
13  #ifndef __MALIDP_DRV_H__
14  #define __MALIDP_DRV_H__
15  
  > 16  #include 
17  #include 
18  #include 
19  #include 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v5 3/3] drm: mali-dp: Add writeback connector

[kbuild test robot]

Hi Brian,

[auto build test ERROR on drm/drm-next]
[also build test ERROR on v4.12-rc2 next-20170526]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Liviu-Dudau/drm-mali-dp-Add-support-for-writeback-on-DP550-DP650/20170516-013246
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: arm-allyesconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   In file included from drivers/gpu/drm/arm/malidp_drv.c:31:0:
>> drivers/gpu/drm/arm/malidp_drv.h:16:31: fatal error: drm/drm_writeback.h: No 
>> such file or directory
#include 
  ^
   compilation terminated.
--
>> drivers/gpu/drm/arm/malidp_mw.c:19:31: fatal error: drm/drm_writeback.h: No 
>> such file or directory
#include 
  ^
   compilation terminated.

vim +16 drivers/gpu/drm/arm/malidp_drv.h

10   * ARM Mali DP500/DP550/DP650 KMS/DRM driver structures
11   */
12  
13  #ifndef __MALIDP_DRV_H__
14  #define __MALIDP_DRV_H__
15  
  > 16  #include 
17  #include 
18  #include 
19  #include 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH] fs: warn in case userspace lied about modprobe return

[Jessica Yu]

+++ Luis R. Rodriguez [25/05/17 17:44 -0700]:

kmod <= v19 was broken -- it could return 0 to modprobe calls,
incorrectly assuming that a kernel module was built-in, whereas in
reality the module was just forming in the kernel. The reason for this
is an incorrect userspace heuristics. A userspace kmod fix is available
for it [0], however should userspace break again we could go on with
an failed get_fs_type() which is hard to debug as the request_module()
is detected as returning 0. The first suspect would be that there is
something worth with the kernel's module loader and obviously in this
case that is not the issue.

Since these issues are painful to debug complain when we know userspace
has  outright lied to us.

[0] 
http://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/libkmod/libkmod-module.c?id=fd44a98ae2eb5eb32161088954ab21e58e19dfc4

Suggested-by: Rusty Russell 
Cc: Jessica Yu 
Signed-off-by: Luis R. Rodriguez 
---
fs/filesystems.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/filesystems.c b/fs/filesystems.c
index cac75547d35c..0f477a5de6ea 100644
--- a/fs/filesystems.c
+++ b/fs/filesystems.c
@@ -275,8 +275,10 @@ struct file_system_type *get_fs_type(const char *name)
int len = dot ? dot - name : strlen(name);

fs = __get_fs_type(name, len);
-   if (!fs && (request_module("fs-%.*s", len, name) == 0))
+   if (!fs && (request_module("fs-%.*s", len, name) == 0)) {
+   WARN_ONCE(!fs, "request_module fs-%.*s succeeded, but still no 
fs?\n", len, name);


The WARN needs to go below the second __get_fs_type() attempt, no?
Because we want to try __get_fs_type() again right after
request_module(), to see if the fs loaded, and _then_ WARN if it
doesn't appear to be loaded.

Jessica


fs = __get_fs_type(name, len);
+   }

if (dot && fs && !(fs->fs_flags & FS_HAS_SUBTYPE)) {
put_filesystem(fs);
--
2.11.0

Re: [PATCH] fs: warn in case userspace lied about modprobe return

[Jessica Yu]

+++ Luis R. Rodriguez [25/05/17 17:44 -0700]:

kmod <= v19 was broken -- it could return 0 to modprobe calls,
incorrectly assuming that a kernel module was built-in, whereas in
reality the module was just forming in the kernel. The reason for this
is an incorrect userspace heuristics. A userspace kmod fix is available
for it [0], however should userspace break again we could go on with
an failed get_fs_type() which is hard to debug as the request_module()
is detected as returning 0. The first suspect would be that there is
something worth with the kernel's module loader and obviously in this
case that is not the issue.

Since these issues are painful to debug complain when we know userspace
has  outright lied to us.

[0] 
http://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/libkmod/libkmod-module.c?id=fd44a98ae2eb5eb32161088954ab21e58e19dfc4

Suggested-by: Rusty Russell 
Cc: Jessica Yu 
Signed-off-by: Luis R. Rodriguez 
---
fs/filesystems.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/filesystems.c b/fs/filesystems.c
index cac75547d35c..0f477a5de6ea 100644
--- a/fs/filesystems.c
+++ b/fs/filesystems.c
@@ -275,8 +275,10 @@ struct file_system_type *get_fs_type(const char *name)
int len = dot ? dot - name : strlen(name);

fs = __get_fs_type(name, len);
-   if (!fs && (request_module("fs-%.*s", len, name) == 0))
+   if (!fs && (request_module("fs-%.*s", len, name) == 0)) {
+   WARN_ONCE(!fs, "request_module fs-%.*s succeeded, but still no 
fs?\n", len, name);


The WARN needs to go below the second __get_fs_type() attempt, no?
Because we want to try __get_fs_type() again right after
request_module(), to see if the fs loaded, and _then_ WARN if it
doesn't appear to be loaded.

Jessica


fs = __get_fs_type(name, len);
+   }

if (dot && fs && !(fs->fs_flags & FS_HAS_SUBTYPE)) {
put_filesystem(fs);
--
2.11.0

Re: [BUGFIX PATCH] kprobes/x86: Fix to set RWX bits correctly before releasing trampoline

[Jessica Yu]

+++ Masami Hiramatsu [26/05/17 09:24 +0900]:

On Thu, 25 May 2017 19:24:26 +0200
"Luis R. Rodriguez"  wrote:


On Thu, May 25, 2017 at 07:38:17PM +0900, Masami Hiramatsu wrote:
> Fix kprobes to set(recover) RWX bits correctly on trampoline
> buffer before releasing it. Releasing readonly page to
> module_memfree() crash the kernel.
>
> Without this fix, if kprobes user register a bunch of kprobes
> in function body (since kprobes on function entry usually
> use ftrace) and unregister it, kernel hits a BUG and crash.
>
> Signed-off-by: Masami Hiramatsu 
> Fixes: d0381c81c2f7 ("kprobes/x86: Set kprobes pages read-only")
> ---
>  arch/x86/kernel/kprobes/core.c |9 +
>  kernel/kprobes.c   |2 +-
>  2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> index 5b2bbfb..6b87780 100644
> --- a/arch/x86/kernel/kprobes/core.c
> +++ b/arch/x86/kernel/kprobes/core.c
> @@ -52,6 +52,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include 
>  #include 
> @@ -417,6 +418,14 @@ static void prepare_boost(struct kprobe *p, struct insn 
*insn)
>}
>  }
>
> +/* Recover page to RW mode before releasing it */
> +void free_insn_page(void *page)
> +{
> +  set_memory_nx((unsigned long)page & PAGE_MASK, 1);
> +  set_memory_rw((unsigned long)page & PAGE_MASK, 1);
> +  module_memfree(page);
> +}

Is this needed for all module_memfree() ? If so should / could it just do it
for alloc users ?


Hmm, would you mean setting those bits in module_memfree()?
I think it should be discussed with other users, kmodule, bpf and ftrace.
It could be, but I'm not so sure about that because setting nx
timing would be critical for some users. As far as I can see,
for ftrace and kprobes, that is OK.


Memory does need to be rw before calling module_memfree(), although I
think it might be better leave that responsibility/flexibility to the
callers, instead of blanket calls to set_memory_rw/x. At least in the
case of the module loader, we have finer-grained control of page
protections; not all pages within the module_alloc'd region need
set_memory_rw/x to be called before freeing (see disable_ro_nx() in
module.c).

Jessica   

Re: [BUGFIX PATCH] kprobes/x86: Fix to set RWX bits correctly before releasing trampoline

[Jessica Yu]

+++ Masami Hiramatsu [26/05/17 09:24 +0900]:

On Thu, 25 May 2017 19:24:26 +0200
"Luis R. Rodriguez"  wrote:


On Thu, May 25, 2017 at 07:38:17PM +0900, Masami Hiramatsu wrote:
> Fix kprobes to set(recover) RWX bits correctly on trampoline
> buffer before releasing it. Releasing readonly page to
> module_memfree() crash the kernel.
>
> Without this fix, if kprobes user register a bunch of kprobes
> in function body (since kprobes on function entry usually
> use ftrace) and unregister it, kernel hits a BUG and crash.
>
> Signed-off-by: Masami Hiramatsu 
> Fixes: d0381c81c2f7 ("kprobes/x86: Set kprobes pages read-only")
> ---
>  arch/x86/kernel/kprobes/core.c |9 +
>  kernel/kprobes.c   |2 +-
>  2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> index 5b2bbfb..6b87780 100644
> --- a/arch/x86/kernel/kprobes/core.c
> +++ b/arch/x86/kernel/kprobes/core.c
> @@ -52,6 +52,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include 
>  #include 
> @@ -417,6 +418,14 @@ static void prepare_boost(struct kprobe *p, struct insn 
*insn)
>}
>  }
>
> +/* Recover page to RW mode before releasing it */
> +void free_insn_page(void *page)
> +{
> +  set_memory_nx((unsigned long)page & PAGE_MASK, 1);
> +  set_memory_rw((unsigned long)page & PAGE_MASK, 1);
> +  module_memfree(page);
> +}

Is this needed for all module_memfree() ? If so should / could it just do it
for alloc users ?


Hmm, would you mean setting those bits in module_memfree()?
I think it should be discussed with other users, kmodule, bpf and ftrace.
It could be, but I'm not so sure about that because setting nx
timing would be critical for some users. As far as I can see,
for ftrace and kprobes, that is OK.


Memory does need to be rw before calling module_memfree(), although I
think it might be better leave that responsibility/flexibility to the
callers, instead of blanket calls to set_memory_rw/x. At least in the
case of the module loader, we have finer-grained control of page
protections; not all pages within the module_alloc'd region need
set_memory_rw/x to be called before freeing (see disable_ro_nx() in
module.c).

Jessica   

Re: [PATCH] LSM: Convert security_hook_heads into explicit array of struct list_head

[Tetsuo Handa]

Kees Cook wrote:
> On Sat, May 27, 2017 at 4:17 AM, Tetsuo Handa
>  wrote:
> > Commit 3dfc9b02864b19f4 ("LSM: Initialize security_hook_heads upon
> > registration.") treats "struct security_hook_heads" as an implicit array
> > of "struct list_head" so that we can eliminate code for static
> > initialization. Although we haven't encountered compilers which do not
> > treat sizeof(security_hook_heads) != sizeof(struct list_head) *
> > (sizeof(security_hook_heads) / sizeof(struct list_head)), Casey does not
> > like the assumption that a structure of N elements can be assumed to be
> > the same as an array of N elements.
> >
> > Now that Kees found that randstruct complains such casting
> >
> >   security/security.c: In function 'security_init':
> >   security/security.c:59:20: note: found mismatched op0 struct pointer 
> > types: 'struct list_head' and 'struct security_hook_heads'
> >
> > struct list_head *list = (struct list_head *) _hook_heads;
> >
> > and Christoph thinks that we should fix it rather than make randstruct
> > whitelist it, this patch fixes it.
> >
> > It would be possible to revert commit 3dfc9b02864b19f4, but this patch
> > converts security_hook_heads into an explicit array of struct list_head
> > by introducing an enum, due to reasons explained below.
> 
> Like Casey, I had confused this patch with the other(?) that resulted
> in dropped type checking. This just switches from named list_heads to
> indexed list_heads, which is fine now that the BUG_ON exists to
> sanity-check the index being used.

Casey, are you just confused as well?

> 
> > In MM subsystem, a sealable memory allocator patch was proposed, and
> > the LSM hooks ("struct security_hook_heads security_hook_heads" and
> > "struct security_hook_list ...[]") will benefit from this allocator via
> > protection using set_memory_ro()/set_memory_rw(), and that allocator
> > will remove CONFIG_SECURITY_WRITABLE_HOOKS config option. Thus, we will
> > likely be moving to that direction.
> 
> It's unlikely that smalloc will allow unsealing after initialization,
> so the SELinux disabling case will remain, IIUC.

LKM-based LSM modules will need it. Look at the result of a recent poll at
https://distrowatch.com/weekly.php?pollnumber=102=SeeVote=20170522#poll
 .
We are still failing to provide users "a security module that individual user
can afford enabling". And we know that we cannot merge all security modules
into mainline. Thus, allowing LKM-based LSM modules is inevitable.

> > @@ -179,7 +182,8 @@ void __init security_add_hooks(struct 
> > security_hook_list *hooks, int count,
> > do {\
> > struct security_hook_list *P;   \
> > \
> > -   list_for_each_entry(P, _hook_heads.FUNC, list) \
> > +   list_for_each_entry(P, _hook_heads \
> > +   [LSM_##FUNC], list) \
> 
> Can this be unsplit so the [...] remains next to security_hook_heads?

These are needed for passing 80 columns check by scripts/checkpatch.pl .
Should we ignore that warning or rename security_hook_heads to e.g. SHH ?

> Otherwise, yeah, I can be convinced to take this. :) Thanks for
> persisting with this, I think it makes sense now.

Thank you.

Re: [PATCH] LSM: Convert security_hook_heads into explicit array of struct list_head

[Tetsuo Handa]

Kees Cook wrote:
> On Sat, May 27, 2017 at 4:17 AM, Tetsuo Handa
>  wrote:
> > Commit 3dfc9b02864b19f4 ("LSM: Initialize security_hook_heads upon
> > registration.") treats "struct security_hook_heads" as an implicit array
> > of "struct list_head" so that we can eliminate code for static
> > initialization. Although we haven't encountered compilers which do not
> > treat sizeof(security_hook_heads) != sizeof(struct list_head) *
> > (sizeof(security_hook_heads) / sizeof(struct list_head)), Casey does not
> > like the assumption that a structure of N elements can be assumed to be
> > the same as an array of N elements.
> >
> > Now that Kees found that randstruct complains such casting
> >
> >   security/security.c: In function 'security_init':
> >   security/security.c:59:20: note: found mismatched op0 struct pointer 
> > types: 'struct list_head' and 'struct security_hook_heads'
> >
> > struct list_head *list = (struct list_head *) _hook_heads;
> >
> > and Christoph thinks that we should fix it rather than make randstruct
> > whitelist it, this patch fixes it.
> >
> > It would be possible to revert commit 3dfc9b02864b19f4, but this patch
> > converts security_hook_heads into an explicit array of struct list_head
> > by introducing an enum, due to reasons explained below.
> 
> Like Casey, I had confused this patch with the other(?) that resulted
> in dropped type checking. This just switches from named list_heads to
> indexed list_heads, which is fine now that the BUG_ON exists to
> sanity-check the index being used.

Casey, are you just confused as well?

> 
> > In MM subsystem, a sealable memory allocator patch was proposed, and
> > the LSM hooks ("struct security_hook_heads security_hook_heads" and
> > "struct security_hook_list ...[]") will benefit from this allocator via
> > protection using set_memory_ro()/set_memory_rw(), and that allocator
> > will remove CONFIG_SECURITY_WRITABLE_HOOKS config option. Thus, we will
> > likely be moving to that direction.
> 
> It's unlikely that smalloc will allow unsealing after initialization,
> so the SELinux disabling case will remain, IIUC.

LKM-based LSM modules will need it. Look at the result of a recent poll at
https://distrowatch.com/weekly.php?pollnumber=102=SeeVote=20170522#poll
 .
We are still failing to provide users "a security module that individual user
can afford enabling". And we know that we cannot merge all security modules
into mainline. Thus, allowing LKM-based LSM modules is inevitable.

> > @@ -179,7 +182,8 @@ void __init security_add_hooks(struct 
> > security_hook_list *hooks, int count,
> > do {\
> > struct security_hook_list *P;   \
> > \
> > -   list_for_each_entry(P, _hook_heads.FUNC, list) \
> > +   list_for_each_entry(P, _hook_heads \
> > +   [LSM_##FUNC], list) \
> 
> Can this be unsplit so the [...] remains next to security_hook_heads?

These are needed for passing 80 columns check by scripts/checkpatch.pl .
Should we ignore that warning or rename security_hook_heads to e.g. SHH ?

> Otherwise, yeah, I can be convinced to take this. :) Thanks for
> persisting with this, I think it makes sense now.

Thank you.

Re: [PATCH] LSM: Convert security_hook_heads into explicit array of struct list_head

[Kees Cook]

On Sat, May 27, 2017 at 4:17 AM, Tetsuo Handa
 wrote:
> Commit 3dfc9b02864b19f4 ("LSM: Initialize security_hook_heads upon
> registration.") treats "struct security_hook_heads" as an implicit array
> of "struct list_head" so that we can eliminate code for static
> initialization. Although we haven't encountered compilers which do not
> treat sizeof(security_hook_heads) != sizeof(struct list_head) *
> (sizeof(security_hook_heads) / sizeof(struct list_head)), Casey does not
> like the assumption that a structure of N elements can be assumed to be
> the same as an array of N elements.
>
> Now that Kees found that randstruct complains such casting
>
>   security/security.c: In function 'security_init':
>   security/security.c:59:20: note: found mismatched op0 struct pointer types: 
> 'struct list_head' and 'struct security_hook_heads'
>
> struct list_head *list = (struct list_head *) _hook_heads;
>
> and Christoph thinks that we should fix it rather than make randstruct
> whitelist it, this patch fixes it.
>
> It would be possible to revert commit 3dfc9b02864b19f4, but this patch
> converts security_hook_heads into an explicit array of struct list_head
> by introducing an enum, due to reasons explained below.

Like Casey, I had confused this patch with the other(?) that resulted
in dropped type checking. This just switches from named list_heads to
indexed list_heads, which is fine now that the BUG_ON exists to
sanity-check the index being used.

> In MM subsystem, a sealable memory allocator patch was proposed, and
> the LSM hooks ("struct security_hook_heads security_hook_heads" and
> "struct security_hook_list ...[]") will benefit from this allocator via
> protection using set_memory_ro()/set_memory_rw(), and that allocator
> will remove CONFIG_SECURITY_WRITABLE_HOOKS config option. Thus, we will
> likely be moving to that direction.

It's unlikely that smalloc will allow unsealing after initialization,
so the SELinux disabling case will remain, IIUC.

> This means that these structures will be allocated at run time using
> this allocator, and therefore the address of these structures will be
> determined at run time rather than compile time.
>
> But currently, LSM_HOOK_INIT() macro depends on the address of
> security_hook_heads being known at compile time. If we use an enum
> so that LSM_HOOK_INIT() macro does not need to know absolute address of
> security_hook_heads, it will help us to use that allocator for LSM hooks.
>
> As a result of introducing an enum, security_hook_heads becomes a local
> variable, making it easier to allocate security_hook_heads at run time.
>
> Signed-off-by: Tetsuo Handa 
> Cc: Kees Cook 
> Cc: Paul Moore 
> Cc: Stephen Smalley 
> Cc: Casey Schaufler 
> Cc: James Morris 
> Cc: Igor Stoppa 
> Cc: Christoph Hellwig 
> ---
>  include/linux/lsm_hooks.h | 412 
> +++---
>  security/security.c   |  38 +++--
>  2 files changed, 229 insertions(+), 221 deletions(-)
>
> diff --git a/security/security.c b/security/security.c
> index 38316bb..bd3c07e 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -179,7 +182,8 @@ void __init security_add_hooks(struct security_hook_list 
> *hooks, int count,
> do {\
> struct security_hook_list *P;   \
> \
> -   list_for_each_entry(P, _hook_heads.FUNC, list) \
> +   list_for_each_entry(P, _hook_heads \
> +   [LSM_##FUNC], list) \

Can this be unsplit so the [...] remains next to security_hook_heads?

> P->hook.FUNC(__VA_ARGS__);  \
> } while (0)
>
> @@ -188,7 +192,8 @@ void __init security_add_hooks(struct security_hook_list 
> *hooks, int count,
> do {\
> struct security_hook_list *P;   \
> \
> -   list_for_each_entry(P, _hook_heads.FUNC, list) { \
> +   list_for_each_entry(P, _hook_heads \
> +   [LSM_##FUNC], list) {   \

Same

> RC = P->hook.FUNC(__VA_ARGS__); \
> if (RC != 0)\
> break;  \
> @@ -1587,8 +1595,8 @@ int security_xfrm_state_pol_flow_match(struct 
> xfrm_state *x,
>  * For speed optimization, we explicitly break the loop rather than
>  * using the macro
>  */
> -   list_for_each_entry(hp, 
> 

Re: [PATCH] LSM: Convert security_hook_heads into explicit array of struct list_head

[Kees Cook]

On Sat, May 27, 2017 at 4:17 AM, Tetsuo Handa
 wrote:
> Commit 3dfc9b02864b19f4 ("LSM: Initialize security_hook_heads upon
> registration.") treats "struct security_hook_heads" as an implicit array
> of "struct list_head" so that we can eliminate code for static
> initialization. Although we haven't encountered compilers which do not
> treat sizeof(security_hook_heads) != sizeof(struct list_head) *
> (sizeof(security_hook_heads) / sizeof(struct list_head)), Casey does not
> like the assumption that a structure of N elements can be assumed to be
> the same as an array of N elements.
>
> Now that Kees found that randstruct complains such casting
>
>   security/security.c: In function 'security_init':
>   security/security.c:59:20: note: found mismatched op0 struct pointer types: 
> 'struct list_head' and 'struct security_hook_heads'
>
> struct list_head *list = (struct list_head *) _hook_heads;
>
> and Christoph thinks that we should fix it rather than make randstruct
> whitelist it, this patch fixes it.
>
> It would be possible to revert commit 3dfc9b02864b19f4, but this patch
> converts security_hook_heads into an explicit array of struct list_head
> by introducing an enum, due to reasons explained below.

Like Casey, I had confused this patch with the other(?) that resulted
in dropped type checking. This just switches from named list_heads to
indexed list_heads, which is fine now that the BUG_ON exists to
sanity-check the index being used.

> In MM subsystem, a sealable memory allocator patch was proposed, and
> the LSM hooks ("struct security_hook_heads security_hook_heads" and
> "struct security_hook_list ...[]") will benefit from this allocator via
> protection using set_memory_ro()/set_memory_rw(), and that allocator
> will remove CONFIG_SECURITY_WRITABLE_HOOKS config option. Thus, we will
> likely be moving to that direction.

It's unlikely that smalloc will allow unsealing after initialization,
so the SELinux disabling case will remain, IIUC.

> This means that these structures will be allocated at run time using
> this allocator, and therefore the address of these structures will be
> determined at run time rather than compile time.
>
> But currently, LSM_HOOK_INIT() macro depends on the address of
> security_hook_heads being known at compile time. If we use an enum
> so that LSM_HOOK_INIT() macro does not need to know absolute address of
> security_hook_heads, it will help us to use that allocator for LSM hooks.
>
> As a result of introducing an enum, security_hook_heads becomes a local
> variable, making it easier to allocate security_hook_heads at run time.
>
> Signed-off-by: Tetsuo Handa 
> Cc: Kees Cook 
> Cc: Paul Moore 
> Cc: Stephen Smalley 
> Cc: Casey Schaufler 
> Cc: James Morris 
> Cc: Igor Stoppa 
> Cc: Christoph Hellwig 
> ---
>  include/linux/lsm_hooks.h | 412 
> +++---
>  security/security.c   |  38 +++--
>  2 files changed, 229 insertions(+), 221 deletions(-)
>
> diff --git a/security/security.c b/security/security.c
> index 38316bb..bd3c07e 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -179,7 +182,8 @@ void __init security_add_hooks(struct security_hook_list 
> *hooks, int count,
> do {\
> struct security_hook_list *P;   \
> \
> -   list_for_each_entry(P, _hook_heads.FUNC, list) \
> +   list_for_each_entry(P, _hook_heads \
> +   [LSM_##FUNC], list) \

Can this be unsplit so the [...] remains next to security_hook_heads?

> P->hook.FUNC(__VA_ARGS__);  \
> } while (0)
>
> @@ -188,7 +192,8 @@ void __init security_add_hooks(struct security_hook_list 
> *hooks, int count,
> do {\
> struct security_hook_list *P;   \
> \
> -   list_for_each_entry(P, _hook_heads.FUNC, list) { \
> +   list_for_each_entry(P, _hook_heads \
> +   [LSM_##FUNC], list) {   \

Same

> RC = P->hook.FUNC(__VA_ARGS__); \
> if (RC != 0)\
> break;  \
> @@ -1587,8 +1595,8 @@ int security_xfrm_state_pol_flow_match(struct 
> xfrm_state *x,
>  * For speed optimization, we explicitly break the loop rather than
>  * using the macro
>  */
> -   list_for_each_entry(hp, 
> _hook_heads.xfrm_state_pol_flow_match,
> -   list) {
> +   list_for_each_entry(hp, _hook_heads
> +   [LSM_xfrm_state_pol_flow_match], list) {

Same

> rc = 

Re: [kernel-hardening] Re: [PATCH v2 05/20] randstruct: Whitelist struct security_hook_heads cast

[Kees Cook]

On Sat, May 27, 2017 at 3:04 PM, Tetsuo Handa
 wrote:
> Kees Cook wrote:
>> On Sat, May 27, 2017 at 1:41 AM, Christoph Hellwig  
>> wrote:
>> > On Fri, May 26, 2017 at 01:17:09PM -0700, Kees Cook wrote:
>> >> The LSM initialization routines walk security_hook_heads as an array
>> >> of struct list_head instead of via names to avoid a ton of needless
>> >> source. Whitelist this to avoid the false positive warning from the
>> >> plugin:
>> >
>> > I think this crap just needs to be fixed properly.  If not it almost
>> > defeats the protections as the "security" ops are just about everywhere.
>>
>> There's nothing unsafe about 3dfc9b02864b19f4dab376f14479ee4ad1de6c9e,
>> it just avoids tons of needless code. Tetsuo has some other ideas for
>> cleaning it up further, but I don't like it because it removes
>> compile-time verification of function types.
>
> Excuse me, but why you think that compile-time verification of function
> types is removed?
>
> -   { .head = _hook_heads.HEAD, .hook = { .HEAD = HOOK } }
> +   { .idx = LSM_##HEAD, .hook = { .HEAD = HOOK } }
>
> This change removes dependency on absolute address of security_hook_heads
> being known at compile-time. If function types of .hook.HEAD and HOOK
> mismatches, the compiler can still warn it.

Sorry, misremembered, that was the other patch. I'll go review this
current one...

-Kees

-- 
Kees Cook
Pixel Security

Re: [kernel-hardening] Re: [PATCH v2 05/20] randstruct: Whitelist struct security_hook_heads cast

[Kees Cook]

On Sat, May 27, 2017 at 3:04 PM, Tetsuo Handa
 wrote:
> Kees Cook wrote:
>> On Sat, May 27, 2017 at 1:41 AM, Christoph Hellwig  
>> wrote:
>> > On Fri, May 26, 2017 at 01:17:09PM -0700, Kees Cook wrote:
>> >> The LSM initialization routines walk security_hook_heads as an array
>> >> of struct list_head instead of via names to avoid a ton of needless
>> >> source. Whitelist this to avoid the false positive warning from the
>> >> plugin:
>> >
>> > I think this crap just needs to be fixed properly.  If not it almost
>> > defeats the protections as the "security" ops are just about everywhere.
>>
>> There's nothing unsafe about 3dfc9b02864b19f4dab376f14479ee4ad1de6c9e,
>> it just avoids tons of needless code. Tetsuo has some other ideas for
>> cleaning it up further, but I don't like it because it removes
>> compile-time verification of function types.
>
> Excuse me, but why you think that compile-time verification of function
> types is removed?
>
> -   { .head = _hook_heads.HEAD, .hook = { .HEAD = HOOK } }
> +   { .idx = LSM_##HEAD, .hook = { .HEAD = HOOK } }
>
> This change removes dependency on absolute address of security_hook_heads
> being known at compile-time. If function types of .hook.HEAD and HOOK
> mismatches, the compiler can still warn it.

Sorry, misremembered, that was the other patch. I'll go review this
current one...

-Kees

-- 
Kees Cook
Pixel Security

[PATCH v2] ipc: Convert ipc_kern_perm.refcount to refcount_t

[Kees Cook]

From: Elena Reshetova 

refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.

Cc: Manfred Spraul 
Signed-off-by: Elena Reshetova 
Signed-off-by: Hans Liljestrand 
Signed-off-by: David Windsor 
Signed-off-by: Kees Cook 
---
Brown paper bag update: forgot to commit a missing hunk...
---
 include/linux/ipc.h | 3 ++-
 ipc/util.c  | 6 +++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/linux/ipc.h b/include/linux/ipc.h
index 5591f055e13f..a7fecbc647d8 100644
--- a/include/linux/ipc.h
+++ b/include/linux/ipc.h
@@ -3,6 +3,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #define IPCMNI 32768  /* <= MAX_INT limit for ipc arrays (including sysctl 
changes) */
@@ -22,7 +23,7 @@ struct kern_ipc_perm {
void*security;
 
struct rcu_head rcu;
-   atomic_t refcount;
+   refcount_t  refcount;
 } cacheline_aligned_in_smp;
 
 #endif /* _LINUX_IPC_H */
diff --git a/ipc/util.c b/ipc/util.c
index 1a2cb02467ab..069bb22c9f64 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -232,7 +232,7 @@ int ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm 
*new, int size)
 
idr_preload(GFP_KERNEL);
 
-   atomic_set(>refcount, 1);
+   refcount_set(>refcount, 1);
spin_lock_init(>lock);
new->deleted = false;
rcu_read_lock();
@@ -397,13 +397,13 @@ void ipc_rmid(struct ipc_ids *ids, struct kern_ipc_perm 
*ipcp)
 
 int ipc_rcu_getref(struct kern_ipc_perm *ptr)
 {
-   return atomic_inc_not_zero(>refcount);
+   return refcount_inc_not_zero(>refcount);
 }
 
 void ipc_rcu_putref(struct kern_ipc_perm *ptr,
void (*func)(struct rcu_head *head))
 {
-   if (!atomic_dec_and_test(>refcount))
+   if (!refcount_dec_and_test(>refcount))
return;
 
call_rcu(>rcu, func);
-- 
2.7.4


-- 
Kees Cook
Pixel Security

[PATCH v2] ipc: Convert ipc_kern_perm.refcount to refcount_t

[Kees Cook]

From: Elena Reshetova 

refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.

Cc: Manfred Spraul 
Signed-off-by: Elena Reshetova 
Signed-off-by: Hans Liljestrand 
Signed-off-by: David Windsor 
Signed-off-by: Kees Cook 
---
Brown paper bag update: forgot to commit a missing hunk...
---
 include/linux/ipc.h | 3 ++-
 ipc/util.c  | 6 +++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/linux/ipc.h b/include/linux/ipc.h
index 5591f055e13f..a7fecbc647d8 100644
--- a/include/linux/ipc.h
+++ b/include/linux/ipc.h
@@ -3,6 +3,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #define IPCMNI 32768  /* <= MAX_INT limit for ipc arrays (including sysctl 
changes) */
@@ -22,7 +23,7 @@ struct kern_ipc_perm {
void*security;
 
struct rcu_head rcu;
-   atomic_t refcount;
+   refcount_t  refcount;
 } cacheline_aligned_in_smp;
 
 #endif /* _LINUX_IPC_H */
diff --git a/ipc/util.c b/ipc/util.c
index 1a2cb02467ab..069bb22c9f64 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -232,7 +232,7 @@ int ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm 
*new, int size)
 
idr_preload(GFP_KERNEL);
 
-   atomic_set(>refcount, 1);
+   refcount_set(>refcount, 1);
spin_lock_init(>lock);
new->deleted = false;
rcu_read_lock();
@@ -397,13 +397,13 @@ void ipc_rmid(struct ipc_ids *ids, struct kern_ipc_perm 
*ipcp)
 
 int ipc_rcu_getref(struct kern_ipc_perm *ptr)
 {
-   return atomic_inc_not_zero(>refcount);
+   return refcount_inc_not_zero(>refcount);
 }
 
 void ipc_rcu_putref(struct kern_ipc_perm *ptr,
void (*func)(struct rcu_head *head))
 {
-   if (!atomic_dec_and_test(>refcount))
+   if (!refcount_dec_and_test(>refcount))
return;
 
call_rcu(>rcu, func);
-- 
2.7.4


-- 
Kees Cook
Pixel Security

[GIT PULL] Thermal SoC management fixes for v4.12-rc3

[Eduardo Valentin]

Hello Rui,

I know this is late, but please send these six fixes for next rc3.

The following changes since commit 08332893e37af6ae779367e78e444f8f9571511d:

  Linux 4.12-rc2 (2017-05-21 19:30:23 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal fixes

for you to fetch changes up to a54c51863ed1294078a435151e625313b4365ac5:

  thermal: broadcom: ns-thermal: default on iProc SoCs (2017-05-23 20:09:34 
-0700)


Colin Ian King (1):
  thermal: core: make thermal_emergency_poweroff static

Jon Mason (1):
  thermal: broadcom: ns-thermal: default on iProc SoCs

Markus Elfring (3):
  ti-soc-thermal: Use devm_kcalloc() in ti_bandgap_build()
  ti-soc-thermal: Delete error messages for failed memory allocations in 
ti_bandgap_build()
  ti-soc-thermal: Fix a typo in a comment line

Masahiro Yamada (1):
  thermal: qoriq: remove useless call for of_thermal_get_trip_points()

 drivers/thermal/broadcom/Kconfig|  9 +
 drivers/thermal/qoriq_thermal.c |  3 ---
 drivers/thermal/thermal_core.c  |  2 +-
 drivers/thermal/ti-soc-thermal/ti-bandgap.c | 14 +-
 4 files changed, 11 insertions(+), 17 deletions(-)


signature.asc
Description: Digital signature

[GIT PULL] Thermal SoC management fixes for v4.12-rc3

[Eduardo Valentin]

Hello Rui,

I know this is late, but please send these six fixes for next rc3.

The following changes since commit 08332893e37af6ae779367e78e444f8f9571511d:

  Linux 4.12-rc2 (2017-05-21 19:30:23 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal fixes

for you to fetch changes up to a54c51863ed1294078a435151e625313b4365ac5:

  thermal: broadcom: ns-thermal: default on iProc SoCs (2017-05-23 20:09:34 
-0700)


Colin Ian King (1):
  thermal: core: make thermal_emergency_poweroff static

Jon Mason (1):
  thermal: broadcom: ns-thermal: default on iProc SoCs

Markus Elfring (3):
  ti-soc-thermal: Use devm_kcalloc() in ti_bandgap_build()
  ti-soc-thermal: Delete error messages for failed memory allocations in 
ti_bandgap_build()
  ti-soc-thermal: Fix a typo in a comment line

Masahiro Yamada (1):
  thermal: qoriq: remove useless call for of_thermal_get_trip_points()

 drivers/thermal/broadcom/Kconfig|  9 +
 drivers/thermal/qoriq_thermal.c |  3 ---
 drivers/thermal/thermal_core.c  |  2 +-
 drivers/thermal/ti-soc-thermal/ti-bandgap.c | 14 +-
 4 files changed, 11 insertions(+), 17 deletions(-)


signature.asc
Description: Digital signature

[PATCH v4] sgi-xp: Use designated initializers

[Kees Cook]

Prepare to mark sensitive kernel structures for randomization by making
sure they're using designated initializers. In this case, no initializers
are needed (they can be NULL initialized and callers adjusted to check
for NULL, which is more efficient than an indirect call).

Cc: Robin Holt 
Cc: Christoph Hellwig 
Signed-off-by: Kees Cook 
---
Brown paper bag release: forgot to actually commit a hunk...
---
 drivers/misc/sgi-xp/xp.h  | 12 +++-
 drivers/misc/sgi-xp/xp_main.c | 36 +++-
 2 files changed, 18 insertions(+), 30 deletions(-)

diff --git a/drivers/misc/sgi-xp/xp.h b/drivers/misc/sgi-xp/xp.h
index c862cd4583cc..b8069eec18cb 100644
--- a/drivers/misc/sgi-xp/xp.h
+++ b/drivers/misc/sgi-xp/xp.h
@@ -309,6 +309,9 @@ static inline enum xp_retval
 xpc_send(short partid, int ch_number, u32 flags, void *payload,
 u16 payload_size)
 {
+   if (!xpc_interface.send)
+   return xpNotLoaded;
+
return xpc_interface.send(partid, ch_number, flags, payload,
  payload_size);
 }
@@ -317,6 +320,9 @@ static inline enum xp_retval
 xpc_send_notify(short partid, int ch_number, u32 flags, void *payload,
u16 payload_size, xpc_notify_func func, void *key)
 {
+   if (!xpc_interface.send_notify)
+   return xpNotLoaded;
+
return xpc_interface.send_notify(partid, ch_number, flags, payload,
 payload_size, func, key);
 }
@@ -324,12 +330,16 @@ xpc_send_notify(short partid, int ch_number, u32 flags, 
void *payload,
 static inline void
 xpc_received(short partid, int ch_number, void *payload)
 {
-   return xpc_interface.received(partid, ch_number, payload);
+   if (xpc_interface.received)
+   xpc_interface.received(partid, ch_number, payload);
 }
 
 static inline enum xp_retval
 xpc_partid_to_nasids(short partid, void *nasids)
 {
+   if (!xpc_interface.partid_to_nasids)
+   return xpNotLoaded;
+
return xpc_interface.partid_to_nasids(partid, nasids);
 }
 
diff --git a/drivers/misc/sgi-xp/xp_main.c b/drivers/misc/sgi-xp/xp_main.c
index 01be66d02ca8..6d7f557fd1c1 100644
--- a/drivers/misc/sgi-xp/xp_main.c
+++ b/drivers/misc/sgi-xp/xp_main.c
@@ -69,23 +69,9 @@ struct xpc_registration xpc_registrations[XPC_MAX_NCHANNELS];
 EXPORT_SYMBOL_GPL(xpc_registrations);
 
 /*
- * Initialize the XPC interface to indicate that XPC isn't loaded.
+ * Initialize the XPC interface to NULL to indicate that XPC isn't loaded.
  */
-static enum xp_retval
-xpc_notloaded(void)
-{
-   return xpNotLoaded;
-}
-
-struct xpc_interface xpc_interface = {
-   (void (*)(int))xpc_notloaded,
-   (void (*)(int))xpc_notloaded,
-   (enum xp_retval(*)(short, int, u32, void *, u16))xpc_notloaded,
-   (enum xp_retval(*)(short, int, u32, void *, u16, xpc_notify_func,
-  void *))xpc_notloaded,
-   (void (*)(short, int, void *))xpc_notloaded,
-   (enum xp_retval(*)(short, void *))xpc_notloaded
-};
+struct xpc_interface xpc_interface = { };
 EXPORT_SYMBOL_GPL(xpc_interface);
 
 /*
@@ -115,17 +101,7 @@ EXPORT_SYMBOL_GPL(xpc_set_interface);
 void
 xpc_clear_interface(void)
 {
-   xpc_interface.connect = (void (*)(int))xpc_notloaded;
-   xpc_interface.disconnect = (void (*)(int))xpc_notloaded;
-   xpc_interface.send = (enum xp_retval(*)(short, int, u32, void *, u16))
-   xpc_notloaded;
-   xpc_interface.send_notify = (enum xp_retval(*)(short, int, u32, void *,
-  u16, xpc_notify_func,
-  void *))xpc_notloaded;
-   xpc_interface.received = (void (*)(short, int, void *))
-   xpc_notloaded;
-   xpc_interface.partid_to_nasids = (enum xp_retval(*)(short, void *))
-   xpc_notloaded;
+   memset(_interface, 0, sizeof(xpc_interface));
 }
 EXPORT_SYMBOL_GPL(xpc_clear_interface);
 
@@ -188,7 +164,8 @@ xpc_connect(int ch_number, xpc_channel_func func, void 
*key, u16 payload_size,
 
mutex_unlock(>mutex);
 
-   xpc_interface.connect(ch_number);
+   if (xpc_interface.connect)
+   xpc_interface.connect(ch_number);
 
return xpSuccess;
 }
@@ -237,7 +214,8 @@ xpc_disconnect(int ch_number)
registration->assigned_limit = 0;
registration->idle_limit = 0;
 
-   xpc_interface.disconnect(ch_number);
+   if (xpc_interface.disconnect)
+   xpc_interface.disconnect(ch_number);
 
mutex_unlock(>mutex);
 
-- 
2.7.4


-- 
Kees Cook
Pixel Security

Re: [PATCH] LSM: Convert security_hook_heads into explicit array of struct list_head

[Tetsuo Handa]

Casey Schaufler wrote:
> > But currently, LSM_HOOK_INIT() macro depends on the address of
> > security_hook_heads being known at compile time. If we use an enum
> > so that LSM_HOOK_INIT() macro does not need to know absolute address of
> > security_hook_heads, it will help us to use that allocator for LSM hooks.
> >
> > As a result of introducing an enum, security_hook_heads becomes a local
> > variable, making it easier to allocate security_hook_heads at run time.
> 
> You loose the type checking in security.c. This is the same
> objection I had before to this approach. It's why I objected
> to 3dfc9b02864b19f4 and why I didn't adopt the array approach
> in the first place. If it's so important that randstruct not
> complain about the unnatural cast, revert the patch that
> introduced it. I see no net benefit in hiding the symbol over
> loosing the typing. You trade a list of typed function
> pointers for an enumerated list of values. It doesn't even
> make the code look smaller!

I still cannot understand what you are referring by "type checking".
Please explain me what the type checking in security/security.c is.

[PATCH v4] sgi-xp: Use designated initializers

[Kees Cook]

Prepare to mark sensitive kernel structures for randomization by making
sure they're using designated initializers. In this case, no initializers
are needed (they can be NULL initialized and callers adjusted to check
for NULL, which is more efficient than an indirect call).

Cc: Robin Holt 
Cc: Christoph Hellwig 
Signed-off-by: Kees Cook 
---
Brown paper bag release: forgot to actually commit a hunk...
---
 drivers/misc/sgi-xp/xp.h  | 12 +++-
 drivers/misc/sgi-xp/xp_main.c | 36 +++-
 2 files changed, 18 insertions(+), 30 deletions(-)

diff --git a/drivers/misc/sgi-xp/xp.h b/drivers/misc/sgi-xp/xp.h
index c862cd4583cc..b8069eec18cb 100644
--- a/drivers/misc/sgi-xp/xp.h
+++ b/drivers/misc/sgi-xp/xp.h
@@ -309,6 +309,9 @@ static inline enum xp_retval
 xpc_send(short partid, int ch_number, u32 flags, void *payload,
 u16 payload_size)
 {
+   if (!xpc_interface.send)
+   return xpNotLoaded;
+
return xpc_interface.send(partid, ch_number, flags, payload,
  payload_size);
 }
@@ -317,6 +320,9 @@ static inline enum xp_retval
 xpc_send_notify(short partid, int ch_number, u32 flags, void *payload,
u16 payload_size, xpc_notify_func func, void *key)
 {
+   if (!xpc_interface.send_notify)
+   return xpNotLoaded;
+
return xpc_interface.send_notify(partid, ch_number, flags, payload,
 payload_size, func, key);
 }
@@ -324,12 +330,16 @@ xpc_send_notify(short partid, int ch_number, u32 flags, 
void *payload,
 static inline void
 xpc_received(short partid, int ch_number, void *payload)
 {
-   return xpc_interface.received(partid, ch_number, payload);
+   if (xpc_interface.received)
+   xpc_interface.received(partid, ch_number, payload);
 }
 
 static inline enum xp_retval
 xpc_partid_to_nasids(short partid, void *nasids)
 {
+   if (!xpc_interface.partid_to_nasids)
+   return xpNotLoaded;
+
return xpc_interface.partid_to_nasids(partid, nasids);
 }
 
diff --git a/drivers/misc/sgi-xp/xp_main.c b/drivers/misc/sgi-xp/xp_main.c
index 01be66d02ca8..6d7f557fd1c1 100644
--- a/drivers/misc/sgi-xp/xp_main.c
+++ b/drivers/misc/sgi-xp/xp_main.c
@@ -69,23 +69,9 @@ struct xpc_registration xpc_registrations[XPC_MAX_NCHANNELS];
 EXPORT_SYMBOL_GPL(xpc_registrations);
 
 /*
- * Initialize the XPC interface to indicate that XPC isn't loaded.
+ * Initialize the XPC interface to NULL to indicate that XPC isn't loaded.
  */
-static enum xp_retval
-xpc_notloaded(void)
-{
-   return xpNotLoaded;
-}
-
-struct xpc_interface xpc_interface = {
-   (void (*)(int))xpc_notloaded,
-   (void (*)(int))xpc_notloaded,
-   (enum xp_retval(*)(short, int, u32, void *, u16))xpc_notloaded,
-   (enum xp_retval(*)(short, int, u32, void *, u16, xpc_notify_func,
-  void *))xpc_notloaded,
-   (void (*)(short, int, void *))xpc_notloaded,
-   (enum xp_retval(*)(short, void *))xpc_notloaded
-};
+struct xpc_interface xpc_interface = { };
 EXPORT_SYMBOL_GPL(xpc_interface);
 
 /*
@@ -115,17 +101,7 @@ EXPORT_SYMBOL_GPL(xpc_set_interface);
 void
 xpc_clear_interface(void)
 {
-   xpc_interface.connect = (void (*)(int))xpc_notloaded;
-   xpc_interface.disconnect = (void (*)(int))xpc_notloaded;
-   xpc_interface.send = (enum xp_retval(*)(short, int, u32, void *, u16))
-   xpc_notloaded;
-   xpc_interface.send_notify = (enum xp_retval(*)(short, int, u32, void *,
-  u16, xpc_notify_func,
-  void *))xpc_notloaded;
-   xpc_interface.received = (void (*)(short, int, void *))
-   xpc_notloaded;
-   xpc_interface.partid_to_nasids = (enum xp_retval(*)(short, void *))
-   xpc_notloaded;
+   memset(_interface, 0, sizeof(xpc_interface));
 }
 EXPORT_SYMBOL_GPL(xpc_clear_interface);
 
@@ -188,7 +164,8 @@ xpc_connect(int ch_number, xpc_channel_func func, void 
*key, u16 payload_size,
 
mutex_unlock(>mutex);
 
-   xpc_interface.connect(ch_number);
+   if (xpc_interface.connect)
+   xpc_interface.connect(ch_number);
 
return xpSuccess;
 }
@@ -237,7 +214,8 @@ xpc_disconnect(int ch_number)
registration->assigned_limit = 0;
registration->idle_limit = 0;
 
-   xpc_interface.disconnect(ch_number);
+   if (xpc_interface.disconnect)
+   xpc_interface.disconnect(ch_number);
 
mutex_unlock(>mutex);
 
-- 
2.7.4


-- 
Kees Cook
Pixel Security

Re: [PATCH] LSM: Convert security_hook_heads into explicit array of struct list_head

[Tetsuo Handa]

Casey Schaufler wrote:
> > But currently, LSM_HOOK_INIT() macro depends on the address of
> > security_hook_heads being known at compile time. If we use an enum
> > so that LSM_HOOK_INIT() macro does not need to know absolute address of
> > security_hook_heads, it will help us to use that allocator for LSM hooks.
> >
> > As a result of introducing an enum, security_hook_heads becomes a local
> > variable, making it easier to allocate security_hook_heads at run time.
> 
> You loose the type checking in security.c. This is the same
> objection I had before to this approach. It's why I objected
> to 3dfc9b02864b19f4 and why I didn't adopt the array approach
> in the first place. If it's so important that randstruct not
> complain about the unnatural cast, revert the patch that
> introduced it. I see no net benefit in hiding the symbol over
> loosing the typing. You trade a list of typed function
> pointers for an enumerated list of values. It doesn't even
> make the code look smaller!

I still cannot understand what you are referring by "type checking".
Please explain me what the type checking in security/security.c is.

[PATCH v3] sgi-xp: Use designated initializers

[Kees Cook]

Prepare to mark sensitive kernel structures for randomization by making
sure they're using designated initializers. In this case, no initializers
are needed (they can be NULL initialized and callers adjusted to check
for NULL, which is more efficient than an indirect call).

Cc: Robin Holt 
Cc: Christoph Hellwig 
Signed-off-by: Kees Cook 
---
 drivers/misc/sgi-xp/xp.h  | 12 
 drivers/misc/sgi-xp/xp_main.c | 36 +++-
 2 files changed, 19 insertions(+), 29 deletions(-)

diff --git a/drivers/misc/sgi-xp/xp.h b/drivers/misc/sgi-xp/xp.h
index c862cd4583cc..39c9e9cf4b2a 100644
--- a/drivers/misc/sgi-xp/xp.h
+++ b/drivers/misc/sgi-xp/xp.h
@@ -309,6 +309,9 @@ static inline enum xp_retval
 xpc_send(short partid, int ch_number, u32 flags, void *payload,
 u16 payload_size)
 {
+   if (!xpc_interface.send)
+   return xpNotLoaded;
+
return xpc_interface.send(partid, ch_number, flags, payload,
  payload_size);
 }
@@ -317,6 +320,9 @@ static inline enum xp_retval
 xpc_send_notify(short partid, int ch_number, u32 flags, void *payload,
u16 payload_size, xpc_notify_func func, void *key)
 {
+   if (!xpc_interface.send_notify)
+   return xpNotLoaded;
+
return xpc_interface.send_notify(partid, ch_number, flags, payload,
 payload_size, func, key);
 }
@@ -324,12 +330,18 @@ xpc_send_notify(short partid, int ch_number, u32 flags, 
void *payload,
 static inline void
 xpc_received(short partid, int ch_number, void *payload)
 {
+   if (!xpc_interface.received)
+   return xpNotLoaded;
+
return xpc_interface.received(partid, ch_number, payload);
 }
 
 static inline enum xp_retval
 xpc_partid_to_nasids(short partid, void *nasids)
 {
+   if (!xpc_interface.partid_to_nasids)
+   return xpNotLoaded;
+
return xpc_interface.partid_to_nasids(partid, nasids);
 }
 
diff --git a/drivers/misc/sgi-xp/xp_main.c b/drivers/misc/sgi-xp/xp_main.c
index 01be66d02ca8..6d7f557fd1c1 100644
--- a/drivers/misc/sgi-xp/xp_main.c
+++ b/drivers/misc/sgi-xp/xp_main.c
@@ -69,23 +69,9 @@ struct xpc_registration xpc_registrations[XPC_MAX_NCHANNELS];
 EXPORT_SYMBOL_GPL(xpc_registrations);
 
 /*
- * Initialize the XPC interface to indicate that XPC isn't loaded.
+ * Initialize the XPC interface to NULL to indicate that XPC isn't loaded.
  */
-static enum xp_retval
-xpc_notloaded(void)
-{
-   return xpNotLoaded;
-}
-
-struct xpc_interface xpc_interface = {
-   (void (*)(int))xpc_notloaded,
-   (void (*)(int))xpc_notloaded,
-   (enum xp_retval(*)(short, int, u32, void *, u16))xpc_notloaded,
-   (enum xp_retval(*)(short, int, u32, void *, u16, xpc_notify_func,
-  void *))xpc_notloaded,
-   (void (*)(short, int, void *))xpc_notloaded,
-   (enum xp_retval(*)(short, void *))xpc_notloaded
-};
+struct xpc_interface xpc_interface = { };
 EXPORT_SYMBOL_GPL(xpc_interface);
 
 /*
@@ -115,17 +101,7 @@ EXPORT_SYMBOL_GPL(xpc_set_interface);
 void
 xpc_clear_interface(void)
 {
-   xpc_interface.connect = (void (*)(int))xpc_notloaded;
-   xpc_interface.disconnect = (void (*)(int))xpc_notloaded;
-   xpc_interface.send = (enum xp_retval(*)(short, int, u32, void *, u16))
-   xpc_notloaded;
-   xpc_interface.send_notify = (enum xp_retval(*)(short, int, u32, void *,
-  u16, xpc_notify_func,
-  void *))xpc_notloaded;
-   xpc_interface.received = (void (*)(short, int, void *))
-   xpc_notloaded;
-   xpc_interface.partid_to_nasids = (enum xp_retval(*)(short, void *))
-   xpc_notloaded;
+   memset(_interface, 0, sizeof(xpc_interface));
 }
 EXPORT_SYMBOL_GPL(xpc_clear_interface);
 
@@ -188,7 +164,8 @@ xpc_connect(int ch_number, xpc_channel_func func, void 
*key, u16 payload_size,
 
mutex_unlock(>mutex);
 
-   xpc_interface.connect(ch_number);
+   if (xpc_interface.connect)
+   xpc_interface.connect(ch_number);
 
return xpSuccess;
 }
@@ -237,7 +214,8 @@ xpc_disconnect(int ch_number)
registration->assigned_limit = 0;
registration->idle_limit = 0;
 
-   xpc_interface.disconnect(ch_number);
+   if (xpc_interface.disconnect)
+   xpc_interface.disconnect(ch_number);
 
mutex_unlock(>mutex);
 
-- 
2.7.4


-- 
Kees Cook
Pixel Security

[PATCH v3] sgi-xp: Use designated initializers

[Kees Cook]

Prepare to mark sensitive kernel structures for randomization by making
sure they're using designated initializers. In this case, no initializers
are needed (they can be NULL initialized and callers adjusted to check
for NULL, which is more efficient than an indirect call).

Cc: Robin Holt 
Cc: Christoph Hellwig 
Signed-off-by: Kees Cook 
---
 drivers/misc/sgi-xp/xp.h  | 12 
 drivers/misc/sgi-xp/xp_main.c | 36 +++-
 2 files changed, 19 insertions(+), 29 deletions(-)

diff --git a/drivers/misc/sgi-xp/xp.h b/drivers/misc/sgi-xp/xp.h
index c862cd4583cc..39c9e9cf4b2a 100644
--- a/drivers/misc/sgi-xp/xp.h
+++ b/drivers/misc/sgi-xp/xp.h
@@ -309,6 +309,9 @@ static inline enum xp_retval
 xpc_send(short partid, int ch_number, u32 flags, void *payload,
 u16 payload_size)
 {
+   if (!xpc_interface.send)
+   return xpNotLoaded;
+
return xpc_interface.send(partid, ch_number, flags, payload,
  payload_size);
 }
@@ -317,6 +320,9 @@ static inline enum xp_retval
 xpc_send_notify(short partid, int ch_number, u32 flags, void *payload,
u16 payload_size, xpc_notify_func func, void *key)
 {
+   if (!xpc_interface.send_notify)
+   return xpNotLoaded;
+
return xpc_interface.send_notify(partid, ch_number, flags, payload,
 payload_size, func, key);
 }
@@ -324,12 +330,18 @@ xpc_send_notify(short partid, int ch_number, u32 flags, 
void *payload,
 static inline void
 xpc_received(short partid, int ch_number, void *payload)
 {
+   if (!xpc_interface.received)
+   return xpNotLoaded;
+
return xpc_interface.received(partid, ch_number, payload);
 }
 
 static inline enum xp_retval
 xpc_partid_to_nasids(short partid, void *nasids)
 {
+   if (!xpc_interface.partid_to_nasids)
+   return xpNotLoaded;
+
return xpc_interface.partid_to_nasids(partid, nasids);
 }
 
diff --git a/drivers/misc/sgi-xp/xp_main.c b/drivers/misc/sgi-xp/xp_main.c
index 01be66d02ca8..6d7f557fd1c1 100644
--- a/drivers/misc/sgi-xp/xp_main.c
+++ b/drivers/misc/sgi-xp/xp_main.c
@@ -69,23 +69,9 @@ struct xpc_registration xpc_registrations[XPC_MAX_NCHANNELS];
 EXPORT_SYMBOL_GPL(xpc_registrations);
 
 /*
- * Initialize the XPC interface to indicate that XPC isn't loaded.
+ * Initialize the XPC interface to NULL to indicate that XPC isn't loaded.
  */
-static enum xp_retval
-xpc_notloaded(void)
-{
-   return xpNotLoaded;
-}
-
-struct xpc_interface xpc_interface = {
-   (void (*)(int))xpc_notloaded,
-   (void (*)(int))xpc_notloaded,
-   (enum xp_retval(*)(short, int, u32, void *, u16))xpc_notloaded,
-   (enum xp_retval(*)(short, int, u32, void *, u16, xpc_notify_func,
-  void *))xpc_notloaded,
-   (void (*)(short, int, void *))xpc_notloaded,
-   (enum xp_retval(*)(short, void *))xpc_notloaded
-};
+struct xpc_interface xpc_interface = { };
 EXPORT_SYMBOL_GPL(xpc_interface);
 
 /*
@@ -115,17 +101,7 @@ EXPORT_SYMBOL_GPL(xpc_set_interface);
 void
 xpc_clear_interface(void)
 {
-   xpc_interface.connect = (void (*)(int))xpc_notloaded;
-   xpc_interface.disconnect = (void (*)(int))xpc_notloaded;
-   xpc_interface.send = (enum xp_retval(*)(short, int, u32, void *, u16))
-   xpc_notloaded;
-   xpc_interface.send_notify = (enum xp_retval(*)(short, int, u32, void *,
-  u16, xpc_notify_func,
-  void *))xpc_notloaded;
-   xpc_interface.received = (void (*)(short, int, void *))
-   xpc_notloaded;
-   xpc_interface.partid_to_nasids = (enum xp_retval(*)(short, void *))
-   xpc_notloaded;
+   memset(_interface, 0, sizeof(xpc_interface));
 }
 EXPORT_SYMBOL_GPL(xpc_clear_interface);
 
@@ -188,7 +164,8 @@ xpc_connect(int ch_number, xpc_channel_func func, void 
*key, u16 payload_size,
 
mutex_unlock(>mutex);
 
-   xpc_interface.connect(ch_number);
+   if (xpc_interface.connect)
+   xpc_interface.connect(ch_number);
 
return xpSuccess;
 }
@@ -237,7 +214,8 @@ xpc_disconnect(int ch_number)
registration->assigned_limit = 0;
registration->idle_limit = 0;
 
-   xpc_interface.disconnect(ch_number);
+   if (xpc_interface.disconnect)
+   xpc_interface.disconnect(ch_number);
 
mutex_unlock(>mutex);
 
-- 
2.7.4


-- 
Kees Cook
Pixel Security

Re: [PATCH] FS-Cache: print hexadecimal value for special cookies type

[Jérémy Lefaure]

On Thu, 27 Apr 2017 16:03:45 +0100
David Howells  wrote:

> Jérémy Lefaure  wrote:
> 
> > When building object-list.o, gcc 6 raises a warning on the sprintf call
> > in fscache_objlist_show:
> > 
> >   CC  fs/fscache/object-list.o
> > fs/fscache/object-list.c: In function ‘fscache_objlist_show’:
> > fs/fscache/object-list.c:265:19: warning: ‘sprintf’ may write a
> > terminating nul past the end of the destination [-Wformat-overflow=]
> > sprintf(_type, "%02u", cookie->def->type);
> >^~
> > fs/fscache/object-list.c:265:4: note: ‘sprintf’ output between 3 and 4
> > bytes into a destination of size 3
> > sprintf(_type, "%02u", cookie->def->type);
> > ^
> > 
> > Moreover, the documentation says that we should have an hex value for
> > special cookies (see Documentation/filesystems/caching/fscache.txt).
> > 
> > Printing hexadecimal value for special cookies fixes the overflow
> > warning and complies with the documentation.  
> 
> Fine by me.  We don't actually handle special type cookies at the moment, so
> you're not going to see anything other than DT or IX for now anyway.
> 
> I'll push this in the next merge window if that's okay with you.
> 

Hi David,
I didn't clearly say that I'm okay about pushing this patch in my last
answer. I am okay with that.

I don't see this patch in v4.12-rc2. Is there an issue with this patch
or are you waiting for my answer ?

Thanks,
Jérémy

Re: [PATCH] FS-Cache: print hexadecimal value for special cookies type

[Jérémy Lefaure]

On Thu, 27 Apr 2017 16:03:45 +0100
David Howells  wrote:

> Jérémy Lefaure  wrote:
> 
> > When building object-list.o, gcc 6 raises a warning on the sprintf call
> > in fscache_objlist_show:
> > 
> >   CC  fs/fscache/object-list.o
> > fs/fscache/object-list.c: In function ‘fscache_objlist_show’:
> > fs/fscache/object-list.c:265:19: warning: ‘sprintf’ may write a
> > terminating nul past the end of the destination [-Wformat-overflow=]
> > sprintf(_type, "%02u", cookie->def->type);
> >^~
> > fs/fscache/object-list.c:265:4: note: ‘sprintf’ output between 3 and 4
> > bytes into a destination of size 3
> > sprintf(_type, "%02u", cookie->def->type);
> > ^
> > 
> > Moreover, the documentation says that we should have an hex value for
> > special cookies (see Documentation/filesystems/caching/fscache.txt).
> > 
> > Printing hexadecimal value for special cookies fixes the overflow
> > warning and complies with the documentation.  
> 
> Fine by me.  We don't actually handle special type cookies at the moment, so
> you're not going to see anything other than DT or IX for now anyway.
> 
> I'll push this in the next merge window if that's okay with you.
> 

Hi David,
I didn't clearly say that I'm okay about pushing this patch in my last
answer. I am okay with that.

I don't see this patch in v4.12-rc2. Is there an issue with this patch
or are you waiting for my answer ?

Thanks,
Jérémy

Re: [PATCH V4 00/17] thermal: cpu_cooling: improve interaction with cpufreq core

[Eduardo Valentin]

On Fri, May 26, 2017 at 10:27:18AM +0530, Viresh Kumar wrote:
> On 26-04-17, 11:41, Lukasz Luba wrote:
> > Hi Viresh,
> > 
> > I went through the v4 code and it looks good to me.
> > Feel free to add for the v4 series
> > Reviewed-by: Lukasz Luba 
> 
> @Eduardo: You missed adding this to the patches.

This is true. I picked your patches before the previous merge window and
they were locally in my tree. Then I pushed them to k.org and missed
this. That is fine, only your patches are for next merge window and it
doesnt hurt too much to rebuild the branch.

Thanks for noticing.

> 
> -- 
> viresh


signature.asc
Description: Digital signature

Re: [PATCH V4 00/17] thermal: cpu_cooling: improve interaction with cpufreq core

[Eduardo Valentin]

On Fri, May 26, 2017 at 10:27:18AM +0530, Viresh Kumar wrote:
> On 26-04-17, 11:41, Lukasz Luba wrote:
> > Hi Viresh,
> > 
> > I went through the v4 code and it looks good to me.
> > Feel free to add for the v4 series
> > Reviewed-by: Lukasz Luba 
> 
> @Eduardo: You missed adding this to the patches.

This is true. I picked your patches before the previous merge window and
they were locally in my tree. Then I pushed them to k.org and missed
this. That is fine, only your patches are for next merge window and it
doesnt hurt too much to rebuild the branch.

Thanks for noticing.

> 
> -- 
> viresh


signature.asc
Description: Digital signature

Re: [PATCH 6/6] tty: serial: lpuart: add a more accurate baud rate calculation method

[Andy Shevchenko]

On Tue, May 9, 2017 at 10:50 AM, Dong Aisheng  wrote:
> On new LPUART versions, the oversampling ratio for the receiver can be
> changed from 4x (00011) to 32x (1) which could help us get a more
> accurate baud rate divider.
>
> The idea is to use the best OSR (over-sampling rate) possible.
> Note, OSR is typically hard-set to 16 in other LPUART instantiations.
> Loop to find the best OSR value possible, one that generates minimum
> baud diff iterate through the rest of the supported values of OSR.

> +lpuart32_serial_setbrg(struct lpuart_port *sport, unsigned int baudrate)
> +{
> +   u32 sbr, osr, baud_diff, tmp_osr, tmp_sbr, tmp_diff, tmp;
> +   u32 clk = sport->port.uartclk;
> +
> +   /*
> +* The idea is to use the best OSR (over-sampling rate) possible.
> +* Note, OSR is typically hard-set to 16 in other LPUART 
> instantiations.
> +* Loop to find the best OSR value possible, one that generates 
> minimum
> +* baud_diff iterate through the rest of the supported values of OSR.
> +*
> +* Calculation Formula:
> +*  Baud Rate = baud clock / ((OSR+1) × SBR)
> +*/
> +   baud_diff = baudrate;
> +   osr = 0;
> +   sbr = 0;
> +

> +   for (tmp_osr = 4; tmp_osr <= 32; tmp_osr++) {

I _think_ you may simplify this and avoid for-loop if you reconsider approach.

> +   /* calculate the temporary sbr value  */
> +   tmp_sbr = (clk / (baudrate * tmp_osr));
> +   if (tmp_sbr == 0)
> +   tmp_sbr = 1;
> +
> +   /*
> +* calculate the baud rate difference based on the temporary
> +* osr and sbr values
> +*/

> +   tmp_diff = clk / (tmp_osr * tmp_sbr) - baudrate;

(32 - 4 + 1) times division is called...

> +
> +   /* select best values between sbr and sbr+1 */
> +   tmp = clk / (tmp_osr * (tmp_sbr + 1));
> +   if (tmp_diff > (baudrate - tmp)) {
> +   tmp_diff = baudrate - tmp;
> +   tmp_sbr++;
> +   }
> +
> +   if (tmp_diff <= baud_diff) {
> +   baud_diff = tmp_diff;
> +   osr = tmp_osr;
> +   sbr = tmp_sbr;
> +   }
> +   }

> +   /* handle buadrate outside acceptable rate */
> +   if (baud_diff > ((baudrate / 100) * 3))
> +   dev_warn(sport->port.dev,
> +"unacceptable baud rate difference of more than 
> 3%%\n");

Shouldn't you fall back to previous setting?

> +
> +   tmp = lpuart32_read(sport->port.membase + UARTBAUD);
> +

> +   if ((osr > 3) && (osr < 8))

Isn't it

if (osr ^ BIT(2) < BIT(2))

?

> +   tmp |= UARTBAUD_BOTHEDGE;

> +}

> +   if (of_device_is_compatible(port->dev->of_node, 
> "fsl,imx7ulp-lpuart")) {
> +   lpuart32_serial_setbrg(sport, baud);

> +   } else {
> +   sbr = sport->port.uartclk / (16 * baud);
> +   bd &= ~UARTBAUD_SBR_MASK;
> +   bd |= sbr & UARTBAUD_SBR_MASK;
> +   bd |= UARTBAUD_BOTHEDGE;
> +   bd &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
> +   lpuart32_write(bd, sport->port.membase + UARTBAUD);
> +   }

Perhaps it makes sense to split this to a helper function as well (in
a separate patch).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 6/6] tty: serial: lpuart: add a more accurate baud rate calculation method

[Andy Shevchenko]

On Tue, May 9, 2017 at 10:50 AM, Dong Aisheng  wrote:
> On new LPUART versions, the oversampling ratio for the receiver can be
> changed from 4x (00011) to 32x (1) which could help us get a more
> accurate baud rate divider.
>
> The idea is to use the best OSR (over-sampling rate) possible.
> Note, OSR is typically hard-set to 16 in other LPUART instantiations.
> Loop to find the best OSR value possible, one that generates minimum
> baud diff iterate through the rest of the supported values of OSR.

> +lpuart32_serial_setbrg(struct lpuart_port *sport, unsigned int baudrate)
> +{
> +   u32 sbr, osr, baud_diff, tmp_osr, tmp_sbr, tmp_diff, tmp;
> +   u32 clk = sport->port.uartclk;
> +
> +   /*
> +* The idea is to use the best OSR (over-sampling rate) possible.
> +* Note, OSR is typically hard-set to 16 in other LPUART 
> instantiations.
> +* Loop to find the best OSR value possible, one that generates 
> minimum
> +* baud_diff iterate through the rest of the supported values of OSR.
> +*
> +* Calculation Formula:
> +*  Baud Rate = baud clock / ((OSR+1) × SBR)
> +*/
> +   baud_diff = baudrate;
> +   osr = 0;
> +   sbr = 0;
> +

> +   for (tmp_osr = 4; tmp_osr <= 32; tmp_osr++) {

I _think_ you may simplify this and avoid for-loop if you reconsider approach.

> +   /* calculate the temporary sbr value  */
> +   tmp_sbr = (clk / (baudrate * tmp_osr));
> +   if (tmp_sbr == 0)
> +   tmp_sbr = 1;
> +
> +   /*
> +* calculate the baud rate difference based on the temporary
> +* osr and sbr values
> +*/

> +   tmp_diff = clk / (tmp_osr * tmp_sbr) - baudrate;

(32 - 4 + 1) times division is called...

> +
> +   /* select best values between sbr and sbr+1 */
> +   tmp = clk / (tmp_osr * (tmp_sbr + 1));
> +   if (tmp_diff > (baudrate - tmp)) {
> +   tmp_diff = baudrate - tmp;
> +   tmp_sbr++;
> +   }
> +
> +   if (tmp_diff <= baud_diff) {
> +   baud_diff = tmp_diff;
> +   osr = tmp_osr;
> +   sbr = tmp_sbr;
> +   }
> +   }

> +   /* handle buadrate outside acceptable rate */
> +   if (baud_diff > ((baudrate / 100) * 3))
> +   dev_warn(sport->port.dev,
> +"unacceptable baud rate difference of more than 
> 3%%\n");

Shouldn't you fall back to previous setting?

> +
> +   tmp = lpuart32_read(sport->port.membase + UARTBAUD);
> +

> +   if ((osr > 3) && (osr < 8))

Isn't it

if (osr ^ BIT(2) < BIT(2))

?

> +   tmp |= UARTBAUD_BOTHEDGE;

> +}

> +   if (of_device_is_compatible(port->dev->of_node, 
> "fsl,imx7ulp-lpuart")) {
> +   lpuart32_serial_setbrg(sport, baud);

> +   } else {
> +   sbr = sport->port.uartclk / (16 * baud);
> +   bd &= ~UARTBAUD_SBR_MASK;
> +   bd |= sbr & UARTBAUD_SBR_MASK;
> +   bd |= UARTBAUD_BOTHEDGE;
> +   bd &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
> +   lpuart32_write(bd, sport->port.membase + UARTBAUD);
> +   }

Perhaps it makes sense to split this to a helper function as well (in
a separate patch).

-- 
With Best Regards,
Andy Shevchenko

[PATCH v7 1/2] soc: qcom: Add device tree binding for GLINK RPM

[Bjorn Andersson]

Add device tree binding documentation for the Qualcomm GLINK RPM, used
for communication with the Resource Power Management subsystem in
various Qualcomm SoCs.

Acked-by: Rob Herring 
Signed-off-by: Bjorn Andersson 
---

Changes since v6:
- Change node in example to "mailbox"
- Add Rob's ack.

 .../devicetree/bindings/soc/qcom/qcom,glink.txt| 73 ++
 1 file changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt 
b/Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt
new file mode 100644
index ..50fc20c6ce91
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt
@@ -0,0 +1,73 @@
+Qualcomm RPM GLINK binding
+
+This binding describes the Qualcomm RPM GLINK, a fifo based mechanism for
+communication with the Resource Power Management system on various Qualcomm
+platforms.
+
+- compatible:
+   Usage: required
+   Value type: 
+   Definition: must be "qcom,glink-rpm"
+
+- interrupts:
+   Usage: required
+   Value type: 
+   Definition: should specify the IRQ used by the remote processor to
+   signal this processor about communication related events
+
+- qcom,rpm-msg-ram:
+   Usage: required
+   Value type: 
+   Definition: handle to RPM message memory resource
+
+- mboxes:
+   Usage: required
+   Value type: 
+   Definition: reference to the "rpm_hlos" mailbox in APCS, as described
+   in mailbox/mailbox.txt
+
+= GLINK DEVICES
+Each subnode of the GLINK node represent function tied to a virtual
+communication channel. The name of the nodes are not important. The properties
+of these nodes are defined by the individual bindings for the specific function
+- but must contain the following property:
+
+- qcom,glink-channels:
+   Usage: required
+   Value type: 
+   Definition: a list of channels tied to this function, used for matching
+   the function to a set of virtual channels
+
+= EXAMPLE
+The following example represents the GLINK RPM node on a MSM8996 device, with
+the function for the "rpm_request" channel defined, which is used for
+regualtors and root clocks.
+
+   apcs_glb: mailbox@982 {
+   compatible = "qcom,msm8996-apcs-hmss-global";
+   reg = <0x982 0x1000>;
+
+   #mbox-cells = <1>;
+   };
+
+   rpm_msg_ram: memory@68000 {
+   compatible = "qcom,rpm-msg-ram";
+   reg = <0x68000 0x6000>;
+   };
+
+   rpm-glink {
+   compatible = "qcom,glink-rpm";
+
+   interrupts = ;
+
+   qcom,rpm-msg-ram = <_msg_ram>;
+
+   mboxes = <_glb 0>;
+
+   rpm-requests {
+   compatible = "qcom,rpm-msm8996";
+   qcom,glink-channels = "rpm_requests";
+
+   ...
+   };
+   };
-- 
2.12.0

[PATCH v7 2/2] rpmsg: Introduce Qualcomm RPM glink driver

[Bjorn Andersson]

This introduces a basic driver for communicating over "native glink"
with the RPM found in Qualcomm platforms.

Signed-off-by: Bjorn Andersson 
---

Changes since v6:
- Call mbox_client_txdone() after mbox_send_message()
- Depend on MAILBOX, rather than the APCS_IPC driver

 drivers/rpmsg/Kconfig  |   10 +
 drivers/rpmsg/Makefile |1 +
 drivers/rpmsg/qcom_glink_rpm.c | 1233 
 3 files changed, 1244 insertions(+)
 create mode 100644 drivers/rpmsg/qcom_glink_rpm.c

diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
index edc008f55663..2a5d2b446de2 100644
--- a/drivers/rpmsg/Kconfig
+++ b/drivers/rpmsg/Kconfig
@@ -13,6 +13,16 @@ config RPMSG_CHAR
  in /dev. They make it possible for user-space programs to send and
  receive rpmsg packets.
 
+config RPMSG_QCOM_GLINK_RPM
+   tristate "Qualcomm RPM Glink driver"
+   select RPMSG
+   depends on HAS_IOMEM
+   depends on MAILBOX
+   help
+ Say y here to enable support for the GLINK RPM communication driver,
+ which serves as a channel for communication with the RPM in GLINK
+ enabled systems.
+
 config RPMSG_QCOM_SMD
tristate "Qualcomm Shared Memory Driver (SMD)"
depends on QCOM_SMEM
diff --git a/drivers/rpmsg/Makefile b/drivers/rpmsg/Makefile
index fae9a6d548fb..28cc19088cc0 100644
--- a/drivers/rpmsg/Makefile
+++ b/drivers/rpmsg/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_RPMSG)+= rpmsg_core.o
 obj-$(CONFIG_RPMSG_CHAR)   += rpmsg_char.o
+obj-$(CONFIG_RPMSG_QCOM_GLINK_RPM) += qcom_glink_rpm.o
 obj-$(CONFIG_RPMSG_QCOM_SMD)   += qcom_smd.o
 obj-$(CONFIG_RPMSG_VIRTIO) += virtio_rpmsg_bus.o
diff --git a/drivers/rpmsg/qcom_glink_rpm.c b/drivers/rpmsg/qcom_glink_rpm.c
new file mode 100644
index ..3559a3e84c1e
--- /dev/null
+++ b/drivers/rpmsg/qcom_glink_rpm.c
@@ -0,0 +1,1233 @@
+/*
+ * Copyright (c) 2016-2017, Linaro Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "rpmsg_internal.h"
+
+#define RPM_TOC_SIZE   256
+#define RPM_TOC_MAGIC  0x67727430 /* grt0 */
+#define RPM_TOC_MAX_ENTRIES((RPM_TOC_SIZE - sizeof(struct rpm_toc)) / \
+sizeof(struct rpm_toc_entry))
+
+#define RPM_TX_FIFO_ID 0x61703272 /* ap2r */
+#define RPM_RX_FIFO_ID 0x72326170 /* r2ap */
+
+#define GLINK_NAME_SIZE32
+
+#define RPM_GLINK_CID_MIN  1
+#define RPM_GLINK_CID_MAX  65536
+
+struct rpm_toc_entry {
+   __le32 id;
+   __le32 offset;
+   __le32 size;
+} __packed;
+
+struct rpm_toc {
+   __le32 magic;
+   __le32 count;
+
+   struct rpm_toc_entry entries[];
+} __packed;
+
+struct glink_msg {
+   __le16 cmd;
+   __le16 param1;
+   __le32 param2;
+   u8 data[];
+} __packed;
+
+struct glink_rpm_pipe {
+   void __iomem *tail;
+   void __iomem *head;
+
+   void __iomem *fifo;
+
+   size_t length;
+};
+
+/**
+ * struct glink_defer_cmd - deferred incoming control message
+ * @node:  list node
+ * @msg:   message header
+ * data:   payload of the message
+ *
+ * Copy of a received control message, to be added to @rx_queue and processed
+ * by @rx_work of @glink_rpm.
+ */
+struct glink_defer_cmd {
+   struct list_head node;
+
+   struct glink_msg msg;
+   u8 data[];
+};
+
+/**
+ * struct glink_rpm - driver context, relates to one remote subsystem
+ * @dev:   reference to the associated struct device
+ * @doorbell:  "rpm_hlos" ipc doorbell
+ * @rx_pipe:   pipe object for receive FIFO
+ * @tx_pipe:   pipe object for transmit FIFO
+ * @irq:   IRQ for signaling incoming events
+ * @rx_work:   worker for handling received control messages
+ * @rx_lock:   protects the @rx_queue
+ * @rx_queue:  queue of received control messages to be processed in @rx_work
+ * @tx_lock:   synchronizes operations on the tx fifo
+ * @idr_lock:  synchronizes @lcids and @rcids modifications
+ * @lcids: idr of all channels with a known local channel id
+ * @rcids: idr of all channels with a known remote channel id
+ */
+struct glink_rpm {
+   struct device *dev;
+
+   struct mbox_client mbox_client;
+   struct mbox_chan *mbox_chan;
+
+   struct glink_rpm_pipe rx_pipe;
+   struct glink_rpm_pipe tx_pipe;
+
+   int irq;
+
+   struct work_struct 

[PATCH v7 1/2] soc: qcom: Add device tree binding for GLINK RPM

[Bjorn Andersson]

Add device tree binding documentation for the Qualcomm GLINK RPM, used
for communication with the Resource Power Management subsystem in
various Qualcomm SoCs.

Acked-by: Rob Herring 
Signed-off-by: Bjorn Andersson 
---

Changes since v6:
- Change node in example to "mailbox"
- Add Rob's ack.

 .../devicetree/bindings/soc/qcom/qcom,glink.txt| 73 ++
 1 file changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt 
b/Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt
new file mode 100644
index ..50fc20c6ce91
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt
@@ -0,0 +1,73 @@
+Qualcomm RPM GLINK binding
+
+This binding describes the Qualcomm RPM GLINK, a fifo based mechanism for
+communication with the Resource Power Management system on various Qualcomm
+platforms.
+
+- compatible:
+   Usage: required
+   Value type: 
+   Definition: must be "qcom,glink-rpm"
+
+- interrupts:
+   Usage: required
+   Value type: 
+   Definition: should specify the IRQ used by the remote processor to
+   signal this processor about communication related events
+
+- qcom,rpm-msg-ram:
+   Usage: required
+   Value type: 
+   Definition: handle to RPM message memory resource
+
+- mboxes:
+   Usage: required
+   Value type: 
+   Definition: reference to the "rpm_hlos" mailbox in APCS, as described
+   in mailbox/mailbox.txt
+
+= GLINK DEVICES
+Each subnode of the GLINK node represent function tied to a virtual
+communication channel. The name of the nodes are not important. The properties
+of these nodes are defined by the individual bindings for the specific function
+- but must contain the following property:
+
+- qcom,glink-channels:
+   Usage: required
+   Value type: 
+   Definition: a list of channels tied to this function, used for matching
+   the function to a set of virtual channels
+
+= EXAMPLE
+The following example represents the GLINK RPM node on a MSM8996 device, with
+the function for the "rpm_request" channel defined, which is used for
+regualtors and root clocks.
+
+   apcs_glb: mailbox@982 {
+   compatible = "qcom,msm8996-apcs-hmss-global";
+   reg = <0x982 0x1000>;
+
+   #mbox-cells = <1>;
+   };
+
+   rpm_msg_ram: memory@68000 {
+   compatible = "qcom,rpm-msg-ram";
+   reg = <0x68000 0x6000>;
+   };
+
+   rpm-glink {
+   compatible = "qcom,glink-rpm";
+
+   interrupts = ;
+
+   qcom,rpm-msg-ram = <_msg_ram>;
+
+   mboxes = <_glb 0>;
+
+   rpm-requests {
+   compatible = "qcom,rpm-msm8996";
+   qcom,glink-channels = "rpm_requests";
+
+   ...
+   };
+   };
-- 
2.12.0

[PATCH v7 2/2] rpmsg: Introduce Qualcomm RPM glink driver

[Bjorn Andersson]

This introduces a basic driver for communicating over "native glink"
with the RPM found in Qualcomm platforms.

Signed-off-by: Bjorn Andersson 
---

Changes since v6:
- Call mbox_client_txdone() after mbox_send_message()
- Depend on MAILBOX, rather than the APCS_IPC driver

 drivers/rpmsg/Kconfig  |   10 +
 drivers/rpmsg/Makefile |1 +
 drivers/rpmsg/qcom_glink_rpm.c | 1233 
 3 files changed, 1244 insertions(+)
 create mode 100644 drivers/rpmsg/qcom_glink_rpm.c

diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
index edc008f55663..2a5d2b446de2 100644
--- a/drivers/rpmsg/Kconfig
+++ b/drivers/rpmsg/Kconfig
@@ -13,6 +13,16 @@ config RPMSG_CHAR
  in /dev. They make it possible for user-space programs to send and
  receive rpmsg packets.
 
+config RPMSG_QCOM_GLINK_RPM
+   tristate "Qualcomm RPM Glink driver"
+   select RPMSG
+   depends on HAS_IOMEM
+   depends on MAILBOX
+   help
+ Say y here to enable support for the GLINK RPM communication driver,
+ which serves as a channel for communication with the RPM in GLINK
+ enabled systems.
+
 config RPMSG_QCOM_SMD
tristate "Qualcomm Shared Memory Driver (SMD)"
depends on QCOM_SMEM
diff --git a/drivers/rpmsg/Makefile b/drivers/rpmsg/Makefile
index fae9a6d548fb..28cc19088cc0 100644
--- a/drivers/rpmsg/Makefile
+++ b/drivers/rpmsg/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_RPMSG)+= rpmsg_core.o
 obj-$(CONFIG_RPMSG_CHAR)   += rpmsg_char.o
+obj-$(CONFIG_RPMSG_QCOM_GLINK_RPM) += qcom_glink_rpm.o
 obj-$(CONFIG_RPMSG_QCOM_SMD)   += qcom_smd.o
 obj-$(CONFIG_RPMSG_VIRTIO) += virtio_rpmsg_bus.o
diff --git a/drivers/rpmsg/qcom_glink_rpm.c b/drivers/rpmsg/qcom_glink_rpm.c
new file mode 100644
index ..3559a3e84c1e
--- /dev/null
+++ b/drivers/rpmsg/qcom_glink_rpm.c
@@ -0,0 +1,1233 @@
+/*
+ * Copyright (c) 2016-2017, Linaro Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "rpmsg_internal.h"
+
+#define RPM_TOC_SIZE   256
+#define RPM_TOC_MAGIC  0x67727430 /* grt0 */
+#define RPM_TOC_MAX_ENTRIES((RPM_TOC_SIZE - sizeof(struct rpm_toc)) / \
+sizeof(struct rpm_toc_entry))
+
+#define RPM_TX_FIFO_ID 0x61703272 /* ap2r */
+#define RPM_RX_FIFO_ID 0x72326170 /* r2ap */
+
+#define GLINK_NAME_SIZE32
+
+#define RPM_GLINK_CID_MIN  1
+#define RPM_GLINK_CID_MAX  65536
+
+struct rpm_toc_entry {
+   __le32 id;
+   __le32 offset;
+   __le32 size;
+} __packed;
+
+struct rpm_toc {
+   __le32 magic;
+   __le32 count;
+
+   struct rpm_toc_entry entries[];
+} __packed;
+
+struct glink_msg {
+   __le16 cmd;
+   __le16 param1;
+   __le32 param2;
+   u8 data[];
+} __packed;
+
+struct glink_rpm_pipe {
+   void __iomem *tail;
+   void __iomem *head;
+
+   void __iomem *fifo;
+
+   size_t length;
+};
+
+/**
+ * struct glink_defer_cmd - deferred incoming control message
+ * @node:  list node
+ * @msg:   message header
+ * data:   payload of the message
+ *
+ * Copy of a received control message, to be added to @rx_queue and processed
+ * by @rx_work of @glink_rpm.
+ */
+struct glink_defer_cmd {
+   struct list_head node;
+
+   struct glink_msg msg;
+   u8 data[];
+};
+
+/**
+ * struct glink_rpm - driver context, relates to one remote subsystem
+ * @dev:   reference to the associated struct device
+ * @doorbell:  "rpm_hlos" ipc doorbell
+ * @rx_pipe:   pipe object for receive FIFO
+ * @tx_pipe:   pipe object for transmit FIFO
+ * @irq:   IRQ for signaling incoming events
+ * @rx_work:   worker for handling received control messages
+ * @rx_lock:   protects the @rx_queue
+ * @rx_queue:  queue of received control messages to be processed in @rx_work
+ * @tx_lock:   synchronizes operations on the tx fifo
+ * @idr_lock:  synchronizes @lcids and @rcids modifications
+ * @lcids: idr of all channels with a known local channel id
+ * @rcids: idr of all channels with a known remote channel id
+ */
+struct glink_rpm {
+   struct device *dev;
+
+   struct mbox_client mbox_client;
+   struct mbox_chan *mbox_chan;
+
+   struct glink_rpm_pipe rx_pipe;
+   struct glink_rpm_pipe tx_pipe;
+
+   int irq;
+
+   struct work_struct rx_work;
+   spinlock_t 

Re: [PATCH v2 2/7] x86: use long long for 64-bit atomic ops

[hpa]

On May 26, 2017 12:09:04 PM PDT, Dmitry Vyukov  wrote:
>Some 64-bit atomic operations use 'long long' as operand/return type
>(e.g. asm-generic/atomic64.h, arch/x86/include/asm/atomic64_32.h);
>while others use 'long' (e.g. arch/x86/include/asm/atomic64_64.h).
>This makes it impossible to write portable code.
>For example, there is no format specifier that prints result of
>atomic64_read() without warnings. atomic64_try_cmpxchg() is almost
>impossible to use in portable fashion because it requires either
>'long *' or 'long long *' as argument depending on arch.
>
>Switch arch/x86/include/asm/atomic64_64.h to 'long long'.
>
>Signed-off-by: Dmitry Vyukov 
>Cc: Mark Rutland 
>Cc: Peter Zijlstra 
>Cc: Will Deacon 
>Cc: Andrew Morton 
>Cc: Andrey Ryabinin 
>Cc: Ingo Molnar 
>Cc: kasan-...@googlegroups.com
>Cc: linux...@kvack.org
>Cc: linux-kernel@vger.kernel.org
>Cc: x...@kernel.org
>
>---
>Changes since v1:
> - reverted stray s/long/long long/ replace in comment
> - added arch/s390 changes to fix build errors/warnings
>---
> arch/s390/include/asm/atomic_ops.h | 14 +-
> arch/s390/include/asm/bitops.h | 12 -
>arch/x86/include/asm/atomic64_64.h | 52
>+++---
> include/linux/types.h  |  2 +-
> 4 files changed, 40 insertions(+), 40 deletions(-)
>
>diff --git a/arch/s390/include/asm/atomic_ops.h
>b/arch/s390/include/asm/atomic_ops.h
>index ac9e2b939d04..055a9083e52d 100644
>--- a/arch/s390/include/asm/atomic_ops.h
>+++ b/arch/s390/include/asm/atomic_ops.h
>@@ -31,10 +31,10 @@ __ATOMIC_OPS(__atomic_and, int, "lan")
> __ATOMIC_OPS(__atomic_or,  int, "lao")
> __ATOMIC_OPS(__atomic_xor, int, "lax")
> 
>-__ATOMIC_OPS(__atomic64_add, long, "laag")
>-__ATOMIC_OPS(__atomic64_and, long, "lang")
>-__ATOMIC_OPS(__atomic64_or,  long, "laog")
>-__ATOMIC_OPS(__atomic64_xor, long, "laxg")
>+__ATOMIC_OPS(__atomic64_add, long long, "laag")
>+__ATOMIC_OPS(__atomic64_and, long long, "lang")
>+__ATOMIC_OPS(__atomic64_or,  long long, "laog")
>+__ATOMIC_OPS(__atomic64_xor, long long, "laxg")
> 
> #undef __ATOMIC_OPS
> #undef __ATOMIC_OP
>@@ -46,7 +46,7 @@ static inline void __atomic_add_const(int val, int
>*ptr)
>   : [ptr] "+Q" (*ptr) : [val] "i" (val) : "cc");
> }
> 
>-static inline void __atomic64_add_const(long val, long *ptr)
>+static inline void __atomic64_add_const(long val, long long *ptr)
> {
>   asm volatile(
>   "   agsi%[ptr],%[val]\n"
>@@ -82,7 +82,7 @@ __ATOMIC_OPS(__atomic_xor, "xr")
> #undef __ATOMIC_OPS
> 
> #define __ATOMIC64_OP(op_name, op_string) \
>-static inline long op_name(long val, long *ptr)   
>\
>+static inline long op_name(long val, long long *ptr)  \
> { \
>   long old, new;  \
>   \
>@@ -118,7 +118,7 @@ static inline int __atomic_cmpxchg(int *ptr, int
>old, int new)
>   return old;
> }
> 
>-static inline long __atomic64_cmpxchg(long *ptr, long old, long new)
>+static inline long __atomic64_cmpxchg(long long *ptr, long old, long
>new)
> {
>   asm volatile(
>   "   csg %[old],%[new],%[ptr]"
>diff --git a/arch/s390/include/asm/bitops.h
>b/arch/s390/include/asm/bitops.h
>index d92047da5ccb..8912f52bca5d 100644
>--- a/arch/s390/include/asm/bitops.h
>+++ b/arch/s390/include/asm/bitops.h
>@@ -80,7 +80,7 @@ static inline void set_bit(unsigned long nr, volatile
>unsigned long *ptr)
>   }
> #endif
>   mask = 1UL << (nr & (BITS_PER_LONG - 1));
>-  __atomic64_or(mask, addr);
>+  __atomic64_or(mask, (long long *)addr);
> }
> 
>static inline void clear_bit(unsigned long nr, volatile unsigned long
>*ptr)
>@@ -101,7 +101,7 @@ static inline void clear_bit(unsigned long nr,
>volatile unsigned long *ptr)
>   }
> #endif
>   mask = ~(1UL << (nr & (BITS_PER_LONG - 1)));
>-  __atomic64_and(mask, addr);
>+  __atomic64_and(mask, (long long *)addr);
> }
> 
>static inline void change_bit(unsigned long nr, volatile unsigned long
>*ptr)
>@@ -122,7 +122,7 @@ static inline void change_bit(unsigned long nr,
>volatile unsigned long *ptr)
>   }
> #endif
>   mask = 1UL << (nr & (BITS_PER_LONG - 1));
>-  __atomic64_xor(mask, addr);
>+  __atomic64_xor(mask, (long long *)addr);
> }
> 
> static inline int
>@@ -132,7 +132,7 @@ test_and_set_bit(unsigned long nr, volatile
>unsigned long *ptr)
>   unsigned long old, mask;
> 
>   mask = 1UL << (nr & (BITS_PER_LONG - 1));
>-  old = __atomic64_or_barrier(mask, addr);
>+  old = __atomic64_or_barrier(mask, (long long *)addr);
>   return (old & mask) != 0;
> }
> 
>@@ -143,7 +143,7 @@ 

Re: [PATCH v2 2/7] x86: use long long for 64-bit atomic ops

[hpa]

On May 26, 2017 12:09:04 PM PDT, Dmitry Vyukov  wrote:
>Some 64-bit atomic operations use 'long long' as operand/return type
>(e.g. asm-generic/atomic64.h, arch/x86/include/asm/atomic64_32.h);
>while others use 'long' (e.g. arch/x86/include/asm/atomic64_64.h).
>This makes it impossible to write portable code.
>For example, there is no format specifier that prints result of
>atomic64_read() without warnings. atomic64_try_cmpxchg() is almost
>impossible to use in portable fashion because it requires either
>'long *' or 'long long *' as argument depending on arch.
>
>Switch arch/x86/include/asm/atomic64_64.h to 'long long'.
>
>Signed-off-by: Dmitry Vyukov 
>Cc: Mark Rutland 
>Cc: Peter Zijlstra 
>Cc: Will Deacon 
>Cc: Andrew Morton 
>Cc: Andrey Ryabinin 
>Cc: Ingo Molnar 
>Cc: kasan-...@googlegroups.com
>Cc: linux...@kvack.org
>Cc: linux-kernel@vger.kernel.org
>Cc: x...@kernel.org
>
>---
>Changes since v1:
> - reverted stray s/long/long long/ replace in comment
> - added arch/s390 changes to fix build errors/warnings
>---
> arch/s390/include/asm/atomic_ops.h | 14 +-
> arch/s390/include/asm/bitops.h | 12 -
>arch/x86/include/asm/atomic64_64.h | 52
>+++---
> include/linux/types.h  |  2 +-
> 4 files changed, 40 insertions(+), 40 deletions(-)
>
>diff --git a/arch/s390/include/asm/atomic_ops.h
>b/arch/s390/include/asm/atomic_ops.h
>index ac9e2b939d04..055a9083e52d 100644
>--- a/arch/s390/include/asm/atomic_ops.h
>+++ b/arch/s390/include/asm/atomic_ops.h
>@@ -31,10 +31,10 @@ __ATOMIC_OPS(__atomic_and, int, "lan")
> __ATOMIC_OPS(__atomic_or,  int, "lao")
> __ATOMIC_OPS(__atomic_xor, int, "lax")
> 
>-__ATOMIC_OPS(__atomic64_add, long, "laag")
>-__ATOMIC_OPS(__atomic64_and, long, "lang")
>-__ATOMIC_OPS(__atomic64_or,  long, "laog")
>-__ATOMIC_OPS(__atomic64_xor, long, "laxg")
>+__ATOMIC_OPS(__atomic64_add, long long, "laag")
>+__ATOMIC_OPS(__atomic64_and, long long, "lang")
>+__ATOMIC_OPS(__atomic64_or,  long long, "laog")
>+__ATOMIC_OPS(__atomic64_xor, long long, "laxg")
> 
> #undef __ATOMIC_OPS
> #undef __ATOMIC_OP
>@@ -46,7 +46,7 @@ static inline void __atomic_add_const(int val, int
>*ptr)
>   : [ptr] "+Q" (*ptr) : [val] "i" (val) : "cc");
> }
> 
>-static inline void __atomic64_add_const(long val, long *ptr)
>+static inline void __atomic64_add_const(long val, long long *ptr)
> {
>   asm volatile(
>   "   agsi%[ptr],%[val]\n"
>@@ -82,7 +82,7 @@ __ATOMIC_OPS(__atomic_xor, "xr")
> #undef __ATOMIC_OPS
> 
> #define __ATOMIC64_OP(op_name, op_string) \
>-static inline long op_name(long val, long *ptr)   
>\
>+static inline long op_name(long val, long long *ptr)  \
> { \
>   long old, new;  \
>   \
>@@ -118,7 +118,7 @@ static inline int __atomic_cmpxchg(int *ptr, int
>old, int new)
>   return old;
> }
> 
>-static inline long __atomic64_cmpxchg(long *ptr, long old, long new)
>+static inline long __atomic64_cmpxchg(long long *ptr, long old, long
>new)
> {
>   asm volatile(
>   "   csg %[old],%[new],%[ptr]"
>diff --git a/arch/s390/include/asm/bitops.h
>b/arch/s390/include/asm/bitops.h
>index d92047da5ccb..8912f52bca5d 100644
>--- a/arch/s390/include/asm/bitops.h
>+++ b/arch/s390/include/asm/bitops.h
>@@ -80,7 +80,7 @@ static inline void set_bit(unsigned long nr, volatile
>unsigned long *ptr)
>   }
> #endif
>   mask = 1UL << (nr & (BITS_PER_LONG - 1));
>-  __atomic64_or(mask, addr);
>+  __atomic64_or(mask, (long long *)addr);
> }
> 
>static inline void clear_bit(unsigned long nr, volatile unsigned long
>*ptr)
>@@ -101,7 +101,7 @@ static inline void clear_bit(unsigned long nr,
>volatile unsigned long *ptr)
>   }
> #endif
>   mask = ~(1UL << (nr & (BITS_PER_LONG - 1)));
>-  __atomic64_and(mask, addr);
>+  __atomic64_and(mask, (long long *)addr);
> }
> 
>static inline void change_bit(unsigned long nr, volatile unsigned long
>*ptr)
>@@ -122,7 +122,7 @@ static inline void change_bit(unsigned long nr,
>volatile unsigned long *ptr)
>   }
> #endif
>   mask = 1UL << (nr & (BITS_PER_LONG - 1));
>-  __atomic64_xor(mask, addr);
>+  __atomic64_xor(mask, (long long *)addr);
> }
> 
> static inline int
>@@ -132,7 +132,7 @@ test_and_set_bit(unsigned long nr, volatile
>unsigned long *ptr)
>   unsigned long old, mask;
> 
>   mask = 1UL << (nr & (BITS_PER_LONG - 1));
>-  old = __atomic64_or_barrier(mask, addr);
>+  old = __atomic64_or_barrier(mask, (long long *)addr);
>   return (old & mask) != 0;
> }
> 
>@@ -143,7 +143,7 @@ test_and_clear_bit(unsigned long nr, volatile
>unsigned long *ptr)
>   unsigned long old, mask;
> 
>   mask = ~(1UL << (nr & (BITS_PER_LONG - 1)));
>-  old = 

[PATCH v7 2/3] dt-bindings: mailbox: Introduce Qualcomm APCS global binding

[Bjorn Andersson]

Introduce a binding for the Qualcomm APCS global block, exposing a
mailbox for invoking interrupts on remote processors in the system.

Acked-by: Rob Herring 
Signed-off-by: Bjorn Andersson 
---

Changes since v6:
- None

 .../bindings/mailbox/qcom,apcs-kpss-global.txt | 46 ++
 1 file changed, 46 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt

diff --git 
a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt 
b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
new file mode 100644
index ..fb961c310f44
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
@@ -0,0 +1,46 @@
+Binding for the Qualcomm APCS global block
+==
+
+This binding describes the APCS "global" block found in various Qualcomm
+platforms.
+
+- compatible:
+   Usage: required
+   Value type: 
+   Definition: must be one of:
+   "qcom,msm8916-apcs-kpss-global",
+   "qcom,msm8996-apcs-hmss-global"
+
+- reg:
+   Usage: required
+   Value type: 
+   Definition: must specify the base address and size of the global block
+
+- #mbox-cells:
+   Usage: required
+   Value type: 
+   Definition: as described in mailbox.txt, must be 1
+
+
+= EXAMPLE
+The following example describes the APCS HMSS found in MSM8996 and part of the
+GLINK RPM referencing the "rpm_hlos" doorbell therein.
+
+   apcs_glb: mailbox@982 {
+   compatible = "qcom,msm8996-apcs-hmss-global";
+   reg = <0x982 0x1000>;
+
+   #mbox-cells = <1>;
+   };
+
+   rpm-glink {
+   compatible = "qcom,glink-rpm";
+
+   interrupts = ;
+
+   qcom,rpm-msg-ram = <_msg_ram>;
+
+   mboxes = <_glb 0>;
+   mbox-names = "rpm_hlos";
+   };
+
-- 
2.12.0

[PATCH v7 3/3] mailbox: Introduce Qualcomm APCS IPC driver

[Bjorn Andersson]

This implements a driver that exposes the IPC bits found in the APCS
Global block in various Qualcomm platforms. The bits are used to signal
inter-processor communication signals from the application CPU to other
masters.

Signed-off-by: Bjorn Andersson 
---

Changes since v6:
- Add COMPILE_TEST
- Drop "dev" from apcs_ipc struct
- Track only the IPC register instead of base and offset

 drivers/mailbox/Kconfig |   8 ++
 drivers/mailbox/Makefile|   2 +
 drivers/mailbox/qcom-apcs-ipc-mailbox.c | 129 
 3 files changed, 139 insertions(+)
 create mode 100644 drivers/mailbox/qcom-apcs-ipc-mailbox.c

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index ee1a3d9147ef..c5731e5e3c6c 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -124,6 +124,14 @@ config MAILBOX_TEST
  Test client to help with testing new Controller driver
  implementations.
 
+config QCOM_APCS_IPC
+   tristate "Qualcomm APCS IPC driver"
+   depends on ARCH_QCOM || COMPILE_TEST
+   help
+ Say y here to enable support for the APCS IPC mailbox driver,
+ providing an interface for invoking the inter-process communication
+ signals from the application processor to other masters.
+
 config TEGRA_HSP_MBOX
bool "Tegra HSP (Hardware Synchronization Primitives) Driver"
depends on ARCH_TEGRA_186_SOC
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index e2bcb03cd35b..d54e41206e17 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -32,4 +32,6 @@ obj-$(CONFIG_BCM_PDC_MBOX)+= bcm-pdc-mailbox.o
 
 obj-$(CONFIG_BCM_FLEXRM_MBOX)  += bcm-flexrm-mailbox.o
 
+obj-$(CONFIG_QCOM_APCS_IPC)+= qcom-apcs-ipc-mailbox.o
+
 obj-$(CONFIG_TEGRA_HSP_MBOX)   += tegra-hsp.o
diff --git a/drivers/mailbox/qcom-apcs-ipc-mailbox.c 
b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
new file mode 100644
index ..9924c6d7f05d
--- /dev/null
+++ b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
@@ -0,0 +1,129 @@
+/*
+ * Copyright (c) 2017, Linaro Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define QCOM_APCS_IPC_BITS 32
+
+struct qcom_apcs_ipc {
+   struct mbox_controller mbox;
+   struct mbox_chan mbox_chans[QCOM_APCS_IPC_BITS];
+
+   void __iomem *reg;
+   unsigned long offset;
+};
+
+static int qcom_apcs_ipc_send_data(struct mbox_chan *chan, void *data)
+{
+   struct qcom_apcs_ipc *apcs = container_of(chan->mbox,
+ struct qcom_apcs_ipc, mbox);
+   unsigned long idx = (unsigned long)chan->con_priv;
+
+   writel(BIT(idx), apcs->reg);
+
+   return 0;
+}
+
+static const struct mbox_chan_ops qcom_apcs_ipc_ops = {
+   .send_data = qcom_apcs_ipc_send_data,
+};
+
+static int qcom_apcs_ipc_probe(struct platform_device *pdev)
+{
+   struct qcom_apcs_ipc *apcs;
+   struct resource *res;
+   unsigned long offset;
+   void __iomem *base;
+   unsigned long i;
+   int ret;
+
+   apcs = devm_kzalloc(>dev, sizeof(*apcs), GFP_KERNEL);
+   if (!apcs)
+   return -ENOMEM;
+
+   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+   base = devm_ioremap_resource(>dev, res);
+   if (IS_ERR(base))
+   return PTR_ERR(base);
+
+   offset = (unsigned long)of_device_get_match_data(>dev);
+
+   apcs->reg = base + offset;
+
+   /* Initialize channel identifiers */
+   for (i = 0; i < ARRAY_SIZE(apcs->mbox_chans); i++)
+   apcs->mbox_chans[i].con_priv = (void *)i;
+
+   apcs->mbox.dev = >dev;
+   apcs->mbox.ops = _apcs_ipc_ops;
+   apcs->mbox.chans = apcs->mbox_chans;
+   apcs->mbox.num_chans = ARRAY_SIZE(apcs->mbox_chans);
+
+   ret = mbox_controller_register(>mbox);
+   if (ret) {
+   dev_err(>dev, "failed to register APCS IPC controller\n");
+   return ret;
+   }
+
+   platform_set_drvdata(pdev, apcs);
+
+   return 0;
+}
+
+static int qcom_apcs_ipc_remove(struct platform_device *pdev)
+{
+   struct qcom_apcs_ipc *apcs = platform_get_drvdata(pdev);
+
+   mbox_controller_unregister(>mbox);
+
+   return 0;
+}
+
+/* .data is the offset of the ipc register within the global block */
+static const struct of_device_id qcom_apcs_ipc_of_match[] = {
+   { .compatible = "qcom,msm8916-apcs-kpss-global", .data = (void *)8 

[PATCH v7 1/3] mailbox: Make startup and shutdown ops optional

[Bjorn Andersson]

Some mailbox hardware doesn't have to perform any additional operations
on startup of shutdown, so make these optional.

Signed-off-by: Bjorn Andersson 
---

Changes since v6:
- None

 drivers/mailbox/mailbox.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 9dfbf7ea10a2..537f4f6d009b 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -355,11 +355,14 @@ struct mbox_chan *mbox_request_channel(struct mbox_client 
*cl, int index)
 
spin_unlock_irqrestore(>lock, flags);
 
-   ret = chan->mbox->ops->startup(chan);
-   if (ret) {
-   dev_err(dev, "Unable to startup the chan (%d)\n", ret);
-   mbox_free_channel(chan);
-   chan = ERR_PTR(ret);
+   if (chan->mbox->ops->startup) {
+   ret = chan->mbox->ops->startup(chan);
+
+   if (ret) {
+   dev_err(dev, "Unable to startup the chan (%d)\n", ret);
+   mbox_free_channel(chan);
+   chan = ERR_PTR(ret);
+   }
}
 
mutex_unlock(_mutex);
@@ -408,7 +411,8 @@ void mbox_free_channel(struct mbox_chan *chan)
if (!chan || !chan->cl)
return;
 
-   chan->mbox->ops->shutdown(chan);
+   if (chan->mbox->ops->shutdown)
+   chan->mbox->ops->shutdown(chan);
 
/* The queued TX requests are simply aborted, no callbacks are made */
spin_lock_irqsave(>lock, flags);
-- 
2.12.0

[PATCH v7 2/3] dt-bindings: mailbox: Introduce Qualcomm APCS global binding

[Bjorn Andersson]

Introduce a binding for the Qualcomm APCS global block, exposing a
mailbox for invoking interrupts on remote processors in the system.

Acked-by: Rob Herring 
Signed-off-by: Bjorn Andersson 
---

Changes since v6:
- None

 .../bindings/mailbox/qcom,apcs-kpss-global.txt | 46 ++
 1 file changed, 46 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt

diff --git 
a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt 
b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
new file mode 100644
index ..fb961c310f44
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
@@ -0,0 +1,46 @@
+Binding for the Qualcomm APCS global block
+==
+
+This binding describes the APCS "global" block found in various Qualcomm
+platforms.
+
+- compatible:
+   Usage: required
+   Value type: 
+   Definition: must be one of:
+   "qcom,msm8916-apcs-kpss-global",
+   "qcom,msm8996-apcs-hmss-global"
+
+- reg:
+   Usage: required
+   Value type: 
+   Definition: must specify the base address and size of the global block
+
+- #mbox-cells:
+   Usage: required
+   Value type: 
+   Definition: as described in mailbox.txt, must be 1
+
+
+= EXAMPLE
+The following example describes the APCS HMSS found in MSM8996 and part of the
+GLINK RPM referencing the "rpm_hlos" doorbell therein.
+
+   apcs_glb: mailbox@982 {
+   compatible = "qcom,msm8996-apcs-hmss-global";
+   reg = <0x982 0x1000>;
+
+   #mbox-cells = <1>;
+   };
+
+   rpm-glink {
+   compatible = "qcom,glink-rpm";
+
+   interrupts = ;
+
+   qcom,rpm-msg-ram = <_msg_ram>;
+
+   mboxes = <_glb 0>;
+   mbox-names = "rpm_hlos";
+   };
+
-- 
2.12.0

[PATCH v7 3/3] mailbox: Introduce Qualcomm APCS IPC driver

[Bjorn Andersson]

This implements a driver that exposes the IPC bits found in the APCS
Global block in various Qualcomm platforms. The bits are used to signal
inter-processor communication signals from the application CPU to other
masters.

Signed-off-by: Bjorn Andersson 
---

Changes since v6:
- Add COMPILE_TEST
- Drop "dev" from apcs_ipc struct
- Track only the IPC register instead of base and offset

 drivers/mailbox/Kconfig |   8 ++
 drivers/mailbox/Makefile|   2 +
 drivers/mailbox/qcom-apcs-ipc-mailbox.c | 129 
 3 files changed, 139 insertions(+)
 create mode 100644 drivers/mailbox/qcom-apcs-ipc-mailbox.c

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index ee1a3d9147ef..c5731e5e3c6c 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -124,6 +124,14 @@ config MAILBOX_TEST
  Test client to help with testing new Controller driver
  implementations.
 
+config QCOM_APCS_IPC
+   tristate "Qualcomm APCS IPC driver"
+   depends on ARCH_QCOM || COMPILE_TEST
+   help
+ Say y here to enable support for the APCS IPC mailbox driver,
+ providing an interface for invoking the inter-process communication
+ signals from the application processor to other masters.
+
 config TEGRA_HSP_MBOX
bool "Tegra HSP (Hardware Synchronization Primitives) Driver"
depends on ARCH_TEGRA_186_SOC
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index e2bcb03cd35b..d54e41206e17 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -32,4 +32,6 @@ obj-$(CONFIG_BCM_PDC_MBOX)+= bcm-pdc-mailbox.o
 
 obj-$(CONFIG_BCM_FLEXRM_MBOX)  += bcm-flexrm-mailbox.o
 
+obj-$(CONFIG_QCOM_APCS_IPC)+= qcom-apcs-ipc-mailbox.o
+
 obj-$(CONFIG_TEGRA_HSP_MBOX)   += tegra-hsp.o
diff --git a/drivers/mailbox/qcom-apcs-ipc-mailbox.c 
b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
new file mode 100644
index ..9924c6d7f05d
--- /dev/null
+++ b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
@@ -0,0 +1,129 @@
+/*
+ * Copyright (c) 2017, Linaro Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define QCOM_APCS_IPC_BITS 32
+
+struct qcom_apcs_ipc {
+   struct mbox_controller mbox;
+   struct mbox_chan mbox_chans[QCOM_APCS_IPC_BITS];
+
+   void __iomem *reg;
+   unsigned long offset;
+};
+
+static int qcom_apcs_ipc_send_data(struct mbox_chan *chan, void *data)
+{
+   struct qcom_apcs_ipc *apcs = container_of(chan->mbox,
+ struct qcom_apcs_ipc, mbox);
+   unsigned long idx = (unsigned long)chan->con_priv;
+
+   writel(BIT(idx), apcs->reg);
+
+   return 0;
+}
+
+static const struct mbox_chan_ops qcom_apcs_ipc_ops = {
+   .send_data = qcom_apcs_ipc_send_data,
+};
+
+static int qcom_apcs_ipc_probe(struct platform_device *pdev)
+{
+   struct qcom_apcs_ipc *apcs;
+   struct resource *res;
+   unsigned long offset;
+   void __iomem *base;
+   unsigned long i;
+   int ret;
+
+   apcs = devm_kzalloc(>dev, sizeof(*apcs), GFP_KERNEL);
+   if (!apcs)
+   return -ENOMEM;
+
+   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+   base = devm_ioremap_resource(>dev, res);
+   if (IS_ERR(base))
+   return PTR_ERR(base);
+
+   offset = (unsigned long)of_device_get_match_data(>dev);
+
+   apcs->reg = base + offset;
+
+   /* Initialize channel identifiers */
+   for (i = 0; i < ARRAY_SIZE(apcs->mbox_chans); i++)
+   apcs->mbox_chans[i].con_priv = (void *)i;
+
+   apcs->mbox.dev = >dev;
+   apcs->mbox.ops = _apcs_ipc_ops;
+   apcs->mbox.chans = apcs->mbox_chans;
+   apcs->mbox.num_chans = ARRAY_SIZE(apcs->mbox_chans);
+
+   ret = mbox_controller_register(>mbox);
+   if (ret) {
+   dev_err(>dev, "failed to register APCS IPC controller\n");
+   return ret;
+   }
+
+   platform_set_drvdata(pdev, apcs);
+
+   return 0;
+}
+
+static int qcom_apcs_ipc_remove(struct platform_device *pdev)
+{
+   struct qcom_apcs_ipc *apcs = platform_get_drvdata(pdev);
+
+   mbox_controller_unregister(>mbox);
+
+   return 0;
+}
+
+/* .data is the offset of the ipc register within the global block */
+static const struct of_device_id qcom_apcs_ipc_of_match[] = {
+   { .compatible = "qcom,msm8916-apcs-kpss-global", .data = (void *)8 },
+   { .compatible = 

[PATCH v7 1/3] mailbox: Make startup and shutdown ops optional

[Bjorn Andersson]

Some mailbox hardware doesn't have to perform any additional operations
on startup of shutdown, so make these optional.

Signed-off-by: Bjorn Andersson 
---

Changes since v6:
- None

 drivers/mailbox/mailbox.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 9dfbf7ea10a2..537f4f6d009b 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -355,11 +355,14 @@ struct mbox_chan *mbox_request_channel(struct mbox_client 
*cl, int index)
 
spin_unlock_irqrestore(>lock, flags);
 
-   ret = chan->mbox->ops->startup(chan);
-   if (ret) {
-   dev_err(dev, "Unable to startup the chan (%d)\n", ret);
-   mbox_free_channel(chan);
-   chan = ERR_PTR(ret);
+   if (chan->mbox->ops->startup) {
+   ret = chan->mbox->ops->startup(chan);
+
+   if (ret) {
+   dev_err(dev, "Unable to startup the chan (%d)\n", ret);
+   mbox_free_channel(chan);
+   chan = ERR_PTR(ret);
+   }
}
 
mutex_unlock(_mutex);
@@ -408,7 +411,8 @@ void mbox_free_channel(struct mbox_chan *chan)
if (!chan || !chan->cl)
return;
 
-   chan->mbox->ops->shutdown(chan);
+   if (chan->mbox->ops->shutdown)
+   chan->mbox->ops->shutdown(chan);
 
/* The queued TX requests are simply aborted, no callbacks are made */
spin_lock_irqsave(>lock, flags);
-- 
2.12.0

Re: [PATCH net-next] net: dsa: mv88e6xxx: handle SERDES error appropriately

[David Miller]

From: Vivien Didelot 
Date: Fri, 26 May 2017 18:02:42 -0400

> mv88e6xxx_serdes_power returns an error, so no need to print an error
> message inside of it. Rather print it in its caller when the error is
> ignored, which is in the mv88e6xxx_port_disable void function.
> 
> Catch and return its error in the counterpart mv88e6xxx_port_enable.
> 
> Fixes: 04aca9938255 ("dsa: mv88e6xxx: Enable/Disable SERDES on port 
> enable/disable")
> Signed-off-by: Vivien Didelot 

Applied, thanks.

Re: [PATCH net-next] net: dsa: mv88e6xxx: handle SERDES error appropriately

[David Miller]

From: Vivien Didelot 
Date: Fri, 26 May 2017 18:02:42 -0400

> mv88e6xxx_serdes_power returns an error, so no need to print an error
> message inside of it. Rather print it in its caller when the error is
> ignored, which is in the mv88e6xxx_port_disable void function.
> 
> Catch and return its error in the counterpart mv88e6xxx_port_enable.
> 
> Fixes: 04aca9938255 ("dsa: mv88e6xxx: Enable/Disable SERDES on port 
> enable/disable")
> Signed-off-by: Vivien Didelot 

Applied, thanks.

Re: [PATCHv1, RFC 5/8] x86/mm: Fold p4d page table layer at runtime

[Brian Gerst]

On Sat, May 27, 2017 at 6:46 PM, Kirill A. Shutemov
 wrote:
> On Sat, May 27, 2017 at 11:09:54AM -0400, Brian Gerst wrote:
>> >  static inline int pgd_none(pgd_t pgd)
>> >  {
>> > +   if (p4d_folded)
>> > +   return 0;
>> > /*
>> >  * There is no need to do a workaround for the KNL stray
>> >  * A/D bit erratum here.  PGDs only point to page tables
>>
>> These should use static_cpu_has(X86_FEATURE_LA57), so that it gets
>> patched by alternatives.
>
> Right, eventually we would likely need something like this. But at this
> point I'm more worried about correctness than performance. Performance
> will be the next step.
>
> And I haven't tried it yet, but I would expect direct use of alternatives
> wouldn't be possible. If I read code correctly, we enable paging way
> before we apply alternatives. But we need to have something functional in
> between.

static_cpu_has() does the check dynamically before alternatives are
applied, so using it early isn't a problem.

--
Brian Gerst

Re: [PATCHv1, RFC 5/8] x86/mm: Fold p4d page table layer at runtime

[Brian Gerst]

On Sat, May 27, 2017 at 6:46 PM, Kirill A. Shutemov
 wrote:
> On Sat, May 27, 2017 at 11:09:54AM -0400, Brian Gerst wrote:
>> >  static inline int pgd_none(pgd_t pgd)
>> >  {
>> > +   if (p4d_folded)
>> > +   return 0;
>> > /*
>> >  * There is no need to do a workaround for the KNL stray
>> >  * A/D bit erratum here.  PGDs only point to page tables
>>
>> These should use static_cpu_has(X86_FEATURE_LA57), so that it gets
>> patched by alternatives.
>
> Right, eventually we would likely need something like this. But at this
> point I'm more worried about correctness than performance. Performance
> will be the next step.
>
> And I haven't tried it yet, but I would expect direct use of alternatives
> wouldn't be possible. If I read code correctly, we enable paging way
> before we apply alternatives. But we need to have something functional in
> between.

static_cpu_has() does the check dynamically before alternatives are
applied, so using it early isn't a problem.

--
Brian Gerst

Re: [PATCHv1, RFC 5/8] x86/mm: Fold p4d page table layer at runtime

[Kirill A. Shutemov]

On Sat, May 27, 2017 at 11:09:54AM -0400, Brian Gerst wrote:
> >  static inline int pgd_none(pgd_t pgd)
> >  {
> > +   if (p4d_folded)
> > +   return 0;
> > /*
> >  * There is no need to do a workaround for the KNL stray
> >  * A/D bit erratum here.  PGDs only point to page tables
> 
> These should use static_cpu_has(X86_FEATURE_LA57), so that it gets
> patched by alternatives.

Right, eventually we would likely need something like this. But at this
point I'm more worried about correctness than performance. Performance
will be the next step.

And I haven't tried it yet, but I would expect direct use of alternatives
wouldn't be possible. If I read code correctly, we enable paging way
before we apply alternatives. But we need to have something functional in
between.

I guess it will be fun :)

-- 
 Kirill A. Shutemov

Re: [PATCHv1, RFC 5/8] x86/mm: Fold p4d page table layer at runtime

[Kirill A. Shutemov]

On Sat, May 27, 2017 at 11:09:54AM -0400, Brian Gerst wrote:
> >  static inline int pgd_none(pgd_t pgd)
> >  {
> > +   if (p4d_folded)
> > +   return 0;
> > /*
> >  * There is no need to do a workaround for the KNL stray
> >  * A/D bit erratum here.  PGDs only point to page tables
> 
> These should use static_cpu_has(X86_FEATURE_LA57), so that it gets
> patched by alternatives.

Right, eventually we would likely need something like this. But at this
point I'm more worried about correctness than performance. Performance
will be the next step.

And I haven't tried it yet, but I would expect direct use of alternatives
wouldn't be possible. If I read code correctly, we enable paging way
before we apply alternatives. But we need to have something functional in
between.

I guess it will be fun :)

-- 
 Kirill A. Shutemov

Re: [PATCH v3] hdlcdrv: Fix divide by zero in hdlcdrv_ioctl

[David Miller]

From: Firo Yang 
Date: Fri, 26 May 2017 22:37:38 +0800

> syszkaller fuzzer triggered a divide by zero, when set calibration
> through ioctl().
> 
> To fix it, test 'bitrate' if it is negative or 0, just return -EINVAL.
> 
> Reported-by: Andrey Konovalov 
> Signed-off-by: Firo Yang 

Applied, thank you.

Re: [PATCH v3] hdlcdrv: Fix divide by zero in hdlcdrv_ioctl

[David Miller]

From: Firo Yang 
Date: Fri, 26 May 2017 22:37:38 +0800

> syszkaller fuzzer triggered a divide by zero, when set calibration
> through ioctl().
> 
> To fix it, test 'bitrate' if it is negative or 0, just return -EINVAL.
> 
> Reported-by: Andrey Konovalov 
> Signed-off-by: Firo Yang 

Applied, thank you.

Re: [PATCH] i2c: pca954x: Add option to skip disabling PCA954x Mux device

[Andy Shevchenko]

On Tue, May 9, 2017 at 11:20 AM,   wrote:

> On some Layerscape boards like LS2085ARDB/LS2080ARDB,
> input pull-up resistors on PCA954x Mux device are
> missing on board. So, if mux are disabled after powered-on,
> input lines will float leading to incorrect functionality.
>
> Hence, PCA954x Mux device should never be turned-off after
> power-on.
>
> Add option to skip disabling PCA954x Mux device
> if device tree contians "i2c-mux-never-disable" property
> for pca954x device node.

> +#ifdef CONFIG_ARCH_LAYERSCAPE

This is 99.99% is no go.

You need to spend more time to make the driver more generic (perhaps
using device property instead) and not using ugly #ifdefery.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] i2c: pca954x: Add option to skip disabling PCA954x Mux device

[Andy Shevchenko]

On Tue, May 9, 2017 at 11:20 AM,   wrote:

> On some Layerscape boards like LS2085ARDB/LS2080ARDB,
> input pull-up resistors on PCA954x Mux device are
> missing on board. So, if mux are disabled after powered-on,
> input lines will float leading to incorrect functionality.
>
> Hence, PCA954x Mux device should never be turned-off after
> power-on.
>
> Add option to skip disabling PCA954x Mux device
> if device tree contians "i2c-mux-never-disable" property
> for pca954x device node.

> +#ifdef CONFIG_ARCH_LAYERSCAPE

This is 99.99% is no go.

You need to spend more time to make the driver more generic (perhaps
using device property instead) and not using ugly #ifdefery.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] LSM: Convert security_hook_heads into explicit array of struct list_head

[Casey Schaufler]

On 5/27/2017 4:17 AM, Tetsuo Handa wrote:
> Commit 3dfc9b02864b19f4 ("LSM: Initialize security_hook_heads upon
> registration.") treats "struct security_hook_heads" as an implicit array
> of "struct list_head" so that we can eliminate code for static
> initialization. Although we haven't encountered compilers which do not
> treat sizeof(security_hook_heads) != sizeof(struct list_head) *
> (sizeof(security_hook_heads) / sizeof(struct list_head)), Casey does not
> like the assumption that a structure of N elements can be assumed to be
> the same as an array of N elements.
>
> Now that Kees found that randstruct complains such casting
>
>   security/security.c: In function 'security_init':
>   security/security.c:59:20: note: found mismatched op0 struct pointer types: 
> 'struct list_head' and 'struct security_hook_heads'
>
> struct list_head *list = (struct list_head *) _hook_heads;
>
> and Christoph thinks that we should fix it rather than make randstruct
> whitelist it, this patch fixes it.
>
> It would be possible to revert commit 3dfc9b02864b19f4, but this patch
> converts security_hook_heads into an explicit array of struct list_head
> by introducing an enum, due to reasons explained below.
>
> In MM subsystem, a sealable memory allocator patch was proposed, and
> the LSM hooks ("struct security_hook_heads security_hook_heads" and
> "struct security_hook_list ...[]") will benefit from this allocator via
> protection using set_memory_ro()/set_memory_rw(), and that allocator
> will remove CONFIG_SECURITY_WRITABLE_HOOKS config option. Thus, we will
> likely be moving to that direction.
>
> This means that these structures will be allocated at run time using
> this allocator, and therefore the address of these structures will be
> determined at run time rather than compile time.
>
> But currently, LSM_HOOK_INIT() macro depends on the address of
> security_hook_heads being known at compile time. If we use an enum
> so that LSM_HOOK_INIT() macro does not need to know absolute address of
> security_hook_heads, it will help us to use that allocator for LSM hooks.
>
> As a result of introducing an enum, security_hook_heads becomes a local
> variable, making it easier to allocate security_hook_heads at run time.

You loose the type checking in security.c. This is the same
objection I had before to this approach. It's why I objected
to 3dfc9b02864b19f4 and why I didn't adopt the array approach
in the first place. If it's so important that randstruct not
complain about the unnatural cast, revert the patch that
introduced it. I see no net benefit in hiding the symbol over
loosing the typing. You trade a list of typed function
pointers for an enumerated list of values. It doesn't even
make the code look smaller!
 

>
> Signed-off-by: Tetsuo Handa 
> Cc: Kees Cook 
> Cc: Paul Moore 
> Cc: Stephen Smalley 
> Cc: Casey Schaufler 
> Cc: James Morris 
> Cc: Igor Stoppa 
> Cc: Christoph Hellwig 
> ---
>  include/linux/lsm_hooks.h | 412 
> +++---
>  security/security.c   |  38 +++--
>  2 files changed, 229 insertions(+), 221 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 080f34e..1f98d1b 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1663,219 +1663,220 @@
>  #endif /* CONFIG_AUDIT */
>  };
>  
> -struct security_hook_heads {
> - struct list_head binder_set_context_mgr;
> - struct list_head binder_transaction;
> - struct list_head binder_transfer_binder;
> - struct list_head binder_transfer_file;
> - struct list_head ptrace_access_check;
> - struct list_head ptrace_traceme;
> - struct list_head capget;
> - struct list_head capset;
> - struct list_head capable;
> - struct list_head quotactl;
> - struct list_head quota_on;
> - struct list_head syslog;
> - struct list_head settime;
> - struct list_head vm_enough_memory;
> - struct list_head bprm_set_creds;
> - struct list_head bprm_check_security;
> - struct list_head bprm_secureexec;
> - struct list_head bprm_committing_creds;
> - struct list_head bprm_committed_creds;
> - struct list_head sb_alloc_security;
> - struct list_head sb_free_security;
> - struct list_head sb_copy_data;
> - struct list_head sb_remount;
> - struct list_head sb_kern_mount;
> - struct list_head sb_show_options;
> - struct list_head sb_statfs;
> - struct list_head sb_mount;
> - struct list_head sb_umount;
> - struct list_head sb_pivotroot;
> - struct list_head sb_set_mnt_opts;
> - struct list_head sb_clone_mnt_opts;
> - struct list_head sb_parse_opts_str;
> - struct list_head dentry_init_security;
> - struct list_head dentry_create_files_as;
> +enum 

Re: [PATCH] LSM: Convert security_hook_heads into explicit array of struct list_head

[Casey Schaufler]

On 5/27/2017 4:17 AM, Tetsuo Handa wrote:
> Commit 3dfc9b02864b19f4 ("LSM: Initialize security_hook_heads upon
> registration.") treats "struct security_hook_heads" as an implicit array
> of "struct list_head" so that we can eliminate code for static
> initialization. Although we haven't encountered compilers which do not
> treat sizeof(security_hook_heads) != sizeof(struct list_head) *
> (sizeof(security_hook_heads) / sizeof(struct list_head)), Casey does not
> like the assumption that a structure of N elements can be assumed to be
> the same as an array of N elements.
>
> Now that Kees found that randstruct complains such casting
>
>   security/security.c: In function 'security_init':
>   security/security.c:59:20: note: found mismatched op0 struct pointer types: 
> 'struct list_head' and 'struct security_hook_heads'
>
> struct list_head *list = (struct list_head *) _hook_heads;
>
> and Christoph thinks that we should fix it rather than make randstruct
> whitelist it, this patch fixes it.
>
> It would be possible to revert commit 3dfc9b02864b19f4, but this patch
> converts security_hook_heads into an explicit array of struct list_head
> by introducing an enum, due to reasons explained below.
>
> In MM subsystem, a sealable memory allocator patch was proposed, and
> the LSM hooks ("struct security_hook_heads security_hook_heads" and
> "struct security_hook_list ...[]") will benefit from this allocator via
> protection using set_memory_ro()/set_memory_rw(), and that allocator
> will remove CONFIG_SECURITY_WRITABLE_HOOKS config option. Thus, we will
> likely be moving to that direction.
>
> This means that these structures will be allocated at run time using
> this allocator, and therefore the address of these structures will be
> determined at run time rather than compile time.
>
> But currently, LSM_HOOK_INIT() macro depends on the address of
> security_hook_heads being known at compile time. If we use an enum
> so that LSM_HOOK_INIT() macro does not need to know absolute address of
> security_hook_heads, it will help us to use that allocator for LSM hooks.
>
> As a result of introducing an enum, security_hook_heads becomes a local
> variable, making it easier to allocate security_hook_heads at run time.

You loose the type checking in security.c. This is the same
objection I had before to this approach. It's why I objected
to 3dfc9b02864b19f4 and why I didn't adopt the array approach
in the first place. If it's so important that randstruct not
complain about the unnatural cast, revert the patch that
introduced it. I see no net benefit in hiding the symbol over
loosing the typing. You trade a list of typed function
pointers for an enumerated list of values. It doesn't even
make the code look smaller!
 

>
> Signed-off-by: Tetsuo Handa 
> Cc: Kees Cook 
> Cc: Paul Moore 
> Cc: Stephen Smalley 
> Cc: Casey Schaufler 
> Cc: James Morris 
> Cc: Igor Stoppa 
> Cc: Christoph Hellwig 
> ---
>  include/linux/lsm_hooks.h | 412 
> +++---
>  security/security.c   |  38 +++--
>  2 files changed, 229 insertions(+), 221 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 080f34e..1f98d1b 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1663,219 +1663,220 @@
>  #endif /* CONFIG_AUDIT */
>  };
>  
> -struct security_hook_heads {
> - struct list_head binder_set_context_mgr;
> - struct list_head binder_transaction;
> - struct list_head binder_transfer_binder;
> - struct list_head binder_transfer_file;
> - struct list_head ptrace_access_check;
> - struct list_head ptrace_traceme;
> - struct list_head capget;
> - struct list_head capset;
> - struct list_head capable;
> - struct list_head quotactl;
> - struct list_head quota_on;
> - struct list_head syslog;
> - struct list_head settime;
> - struct list_head vm_enough_memory;
> - struct list_head bprm_set_creds;
> - struct list_head bprm_check_security;
> - struct list_head bprm_secureexec;
> - struct list_head bprm_committing_creds;
> - struct list_head bprm_committed_creds;
> - struct list_head sb_alloc_security;
> - struct list_head sb_free_security;
> - struct list_head sb_copy_data;
> - struct list_head sb_remount;
> - struct list_head sb_kern_mount;
> - struct list_head sb_show_options;
> - struct list_head sb_statfs;
> - struct list_head sb_mount;
> - struct list_head sb_umount;
> - struct list_head sb_pivotroot;
> - struct list_head sb_set_mnt_opts;
> - struct list_head sb_clone_mnt_opts;
> - struct list_head sb_parse_opts_str;
> - struct list_head dentry_init_security;
> - struct list_head dentry_create_files_as;
> +enum security_hook_index {
> + LSM_binder_set_context_mgr,
> + LSM_binder_transaction,
> + LSM_binder_transfer_binder,
> + LSM_binder_transfer_file,
> + LSM_ptrace_access_check,
> +   

Re: [PATCH v2 1/2] drivers: pwm: core: implement pwm dead-times

[Andy Shevchenko]

On Tue, May 9, 2017 at 11:19 AM, Claudiu Beznea
 wrote:
> Extends PWM framework to support PWM dead-times.
> The notions introduced are rising edge dead-time
> and falling edge dead-time. These are useful for
> PWM controllers with channels that have more than
> one outputs.
> The implementation add sysfs interface for
> configuration. It extends the pwm_state structure
> with two new members which keeps the values for
> dead-times.
> There were no additions in device tree for PWM channels
> initialized by device tree.

AFAIU it's effectively called phase of the signal.
It looks to me much simpler if you allow to have linked / virtual
channels instead of creating a lot of (duplicated) properties.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] drivers: pwm: core: implement pwm dead-times

[Andy Shevchenko]

On Tue, May 9, 2017 at 11:19 AM, Claudiu Beznea
 wrote:
> Extends PWM framework to support PWM dead-times.
> The notions introduced are rising edge dead-time
> and falling edge dead-time. These are useful for
> PWM controllers with channels that have more than
> one outputs.
> The implementation add sysfs interface for
> configuration. It extends the pwm_state structure
> with two new members which keeps the values for
> dead-times.
> There were no additions in device tree for PWM channels
> initialized by device tree.

AFAIU it's effectively called phase of the signal.
It looks to me much simpler if you allow to have linked / virtual
channels instead of creating a lot of (duplicated) properties.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] drivers: pwm: core: implement pwm mode

[Andy Shevchenko]

On Tue, May 9, 2017 at 3:15 PM, Claudiu Beznea
 wrote:
> Extends PWM framework to support PWM modes. The currently
> implemented PWM modes were called PWM complementary mode
> and PWM push-pull mode. For devices that have more than one
> output per PWM channel:
> - PWM complementary mode is standard working mode; in PWM
> complementary mode the rising and falling edges of the
> channels outputs have opposite levels, same duration and
> same starting time.
> - in PWM push-pull mode the channles outputs has same levels,
> same duration and the rising edges are delayed until the
> beginning of the next period.
> A new member was added in pwm_state structure in order to
> keep the new PWM argument.

To me it sound over-engineered.

It looks like polarity type

I dunno if PWM supports linked / virtual channels, but it would be like that

channel X  (polarity P) effectively is

channel Xa (polarity P) / channel Xb (polarity !P)

and vise versa.

Moreover, mode in your case doesn't fit to GPIO style of output which
would be emulated or native mode for PWM (we have already a use case
and one driver implements that).

GPIO type of output is, obviously, duty=100% in case of emulation,
though separate state for HW assisted kind of that.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] drivers: pwm: core: implement pwm mode

[Andy Shevchenko]

On Tue, May 9, 2017 at 3:15 PM, Claudiu Beznea
 wrote:
> Extends PWM framework to support PWM modes. The currently
> implemented PWM modes were called PWM complementary mode
> and PWM push-pull mode. For devices that have more than one
> output per PWM channel:
> - PWM complementary mode is standard working mode; in PWM
> complementary mode the rising and falling edges of the
> channels outputs have opposite levels, same duration and
> same starting time.
> - in PWM push-pull mode the channles outputs has same levels,
> same duration and the rising edges are delayed until the
> beginning of the next period.
> A new member was added in pwm_state structure in order to
> keep the new PWM argument.

To me it sound over-engineered.

It looks like polarity type

I dunno if PWM supports linked / virtual channels, but it would be like that

channel X  (polarity P) effectively is

channel Xa (polarity P) / channel Xb (polarity !P)

and vise versa.

Moreover, mode in your case doesn't fit to GPIO style of output which
would be emulated or native mode for PWM (we have already a use case
and one driver implements that).

GPIO type of output is, obviously, duty=100% in case of emulation,
though separate state for HW assisted kind of that.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 05/20] randstruct: Whitelist struct security_hook_heads cast

[Tetsuo Handa]

Kees Cook wrote:
> On Sat, May 27, 2017 at 1:41 AM, Christoph Hellwig  wrote:
> > On Fri, May 26, 2017 at 01:17:09PM -0700, Kees Cook wrote:
> >> The LSM initialization routines walk security_hook_heads as an array
> >> of struct list_head instead of via names to avoid a ton of needless
> >> source. Whitelist this to avoid the false positive warning from the
> >> plugin:
> >
> > I think this crap just needs to be fixed properly.  If not it almost
> > defeats the protections as the "security" ops are just about everywhere.
> 
> There's nothing unsafe about 3dfc9b02864b19f4dab376f14479ee4ad1de6c9e,
> it just avoids tons of needless code. Tetsuo has some other ideas for
> cleaning it up further, but I don't like it because it removes
> compile-time verification of function types.

Excuse me, but why you think that compile-time verification of function
types is removed?

-   { .head = _hook_heads.HEAD, .hook = { .HEAD = HOOK } }
+   { .idx = LSM_##HEAD, .hook = { .HEAD = HOOK } }

This change removes dependency on absolute address of security_hook_heads
being known at compile-time. If function types of .hook.HEAD and HOOK
mismatches, the compiler can still warn it.

Re: [PATCH v2 05/20] randstruct: Whitelist struct security_hook_heads cast

[Tetsuo Handa]

Kees Cook wrote:
> On Sat, May 27, 2017 at 1:41 AM, Christoph Hellwig  wrote:
> > On Fri, May 26, 2017 at 01:17:09PM -0700, Kees Cook wrote:
> >> The LSM initialization routines walk security_hook_heads as an array
> >> of struct list_head instead of via names to avoid a ton of needless
> >> source. Whitelist this to avoid the false positive warning from the
> >> plugin:
> >
> > I think this crap just needs to be fixed properly.  If not it almost
> > defeats the protections as the "security" ops are just about everywhere.
> 
> There's nothing unsafe about 3dfc9b02864b19f4dab376f14479ee4ad1de6c9e,
> it just avoids tons of needless code. Tetsuo has some other ideas for
> cleaning it up further, but I don't like it because it removes
> compile-time verification of function types.

Excuse me, but why you think that compile-time verification of function
types is removed?

-   { .head = _hook_heads.HEAD, .hook = { .HEAD = HOOK } }
+   { .idx = LSM_##HEAD, .hook = { .HEAD = HOOK } }

This change removes dependency on absolute address of security_hook_heads
being known at compile-time. If function types of .hook.HEAD and HOOK
mismatches, the compiler can still warn it.

[PATCH v2] mac80211: Invoke TX LED in more code paths

[Bjorn Andersson]

ieee80211_tx_status() is only one of the possible ways a driver can
report a handled packet, some drivers call this for every packet while
others calls it rarely or never.

In order to invoke the TX LED in the non-status reporting cases this
patch pushes the call to ieee80211_led_tx() into
ieee80211_report_used_skb(), which is shared between the various code
paths.

Signed-off-by: Bjorn Andersson 
---
 net/mac80211/status.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index be47ac5cd8c8..a9fa6ee57e8f 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -546,6 +546,8 @@ static void ieee80211_report_used_skb(struct 
ieee80211_local *local,
skb->wifi_acked_valid = 1;
skb->wifi_acked = acked;
}
+
+   ieee80211_led_tx(local);
 }
 
 /*
@@ -823,8 +825,6 @@ static void __ieee80211_tx_status(struct ieee80211_hw *hw,
}
}
 
-   ieee80211_led_tx(local);
-
/* SNMP counters
 * Fragments are passed to low-level drivers as separate skbs, so these
 * are actually fragments, not frames. Update frame counters only for
-- 
2.12.0

[PATCH v2] mac80211: Invoke TX LED in more code paths

[Bjorn Andersson]

ieee80211_tx_status() is only one of the possible ways a driver can
report a handled packet, some drivers call this for every packet while
others calls it rarely or never.

In order to invoke the TX LED in the non-status reporting cases this
patch pushes the call to ieee80211_led_tx() into
ieee80211_report_used_skb(), which is shared between the various code
paths.

Signed-off-by: Bjorn Andersson 
---
 net/mac80211/status.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index be47ac5cd8c8..a9fa6ee57e8f 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -546,6 +546,8 @@ static void ieee80211_report_used_skb(struct 
ieee80211_local *local,
skb->wifi_acked_valid = 1;
skb->wifi_acked = acked;
}
+
+   ieee80211_led_tx(local);
 }
 
 /*
@@ -823,8 +825,6 @@ static void __ieee80211_tx_status(struct ieee80211_hw *hw,
}
}
 
-   ieee80211_led_tx(local);
-
/* SNMP counters
 * Fragments are passed to low-level drivers as separate skbs, so these
 * are actually fragments, not frames. Update frame counters only for
-- 
2.12.0

Re: [PATCH net-next] net: stmmac: use correct pointer when printing normal descriptor ring

[Andy Shevchenko]

On Tue, May 9, 2017 at 7:52 PM, Niklas Cassel  wrote:
> From: Niklas Cassel 

Commit message?

> seq_printf(seq, "%d [0x%x]: 0x%x 0x%x 0x%x 0x%x\n",
> -  i, (unsigned int)virt_to_phys(ep),
> +  i, (unsigned int)virt_to_phys(p),

There is should not be casting. Pointer might be 64-bit, thus %pap
must be used instead with a reference to the physical address.

>le32_to_cpu(p->des0), le32_to_cpu(p->des1),
>le32_to_cpu(p->des2), 
> le32_to_cpu(p->des3));
> p++;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH net-next] net: stmmac: use correct pointer when printing normal descriptor ring

[Andy Shevchenko]

On Tue, May 9, 2017 at 7:52 PM, Niklas Cassel  wrote:
> From: Niklas Cassel 

Commit message?

> seq_printf(seq, "%d [0x%x]: 0x%x 0x%x 0x%x 0x%x\n",
> -  i, (unsigned int)virt_to_phys(ep),
> +  i, (unsigned int)virt_to_phys(p),

There is should not be casting. Pointer might be 64-bit, thus %pap
must be used instead with a reference to the physical address.

>le32_to_cpu(p->des0), le32_to_cpu(p->des1),
>le32_to_cpu(p->des2), 
> le32_to_cpu(p->des3));
> p++;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] ipc: Convert kern_ipc_perm.refcount to refcount_t

[kbuild test robot]

Hi Elena,

[auto build test ERROR on next-20170526]
[cannot apply to linus/master linux/master kees/for-next/pstore v4.9-rc8 
v4.9-rc7 v4.9-rc6 v4.12-rc2]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Kees-Cook/ipc-Convert-kern_ipc_perm-refcount-to-refcount_t/20170528-040601
config: m32r-m32104ut_defconfig (attached as .config)
compiler: m32r-linux-gcc (GCC) 6.2.0
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=m32r 

All errors (new ones prefixed by >>):

   In file included from include/uapi/linux/shm.h:4:0,
from include/linux/shm.h:6,
from ipc/util.c:44:
   include/linux/ipc.h:25:2: error: unknown type name 'refcount_t'
 refcount_t refcount;
 ^~
   ipc/util.c: In function 'ipc_addid':
>> ipc/util.c:235:15: error: passing argument 1 of 'refcount_set' from 
>> incompatible pointer type [-Werror=incompatible-pointer-types]
 refcount_set(>refcount, 1);
  ^
   In file included from include/linux/key.h:26:0,
from include/linux/security.h:26,
from ipc/util.c:52:
   include/linux/refcount.h:28:20: note: expected 'refcount_t * {aka struct 
refcount_struct *}' but argument is of type 'int *'
static inline void refcount_set(refcount_t *r, unsigned int n)
   ^~~~
   ipc/util.c: In function 'ipc_rcu_getref':
>> ipc/util.c:400:31: error: passing argument 1 of 'refcount_inc_not_zero' from 
>> incompatible pointer type [-Werror=incompatible-pointer-types]
 return refcount_inc_not_zero(>refcount);
  ^
   In file included from include/linux/key.h:26:0,
from include/linux/security.h:26,
from ipc/util.c:52:
   include/linux/refcount.h:47:26: note: expected 'refcount_t * {aka struct 
refcount_struct *}' but argument is of type 'int *'
extern __must_check bool refcount_inc_not_zero(refcount_t *r);
 ^
   ipc/util.c: In function 'ipc_rcu_putref':
>> ipc/util.c:406:29: error: passing argument 1 of 'refcount_dec_and_test' from 
>> incompatible pointer type [-Werror=incompatible-pointer-types]
 if (!refcount_dec_and_test(>refcount))
^
   In file included from include/linux/key.h:26:0,
from include/linux/security.h:26,
from ipc/util.c:52:
   include/linux/refcount.h:53:26: note: expected 'refcount_t * {aka struct 
refcount_struct *}' but argument is of type 'int *'
extern __must_check bool refcount_dec_and_test(refcount_t *r);
 ^
   cc1: some warnings being treated as errors

vim +/refcount_set +235 ipc/util.c

38   *
39   *  Note that sems have a special fast path that avoids 
kern_ipc_perm.lock -
40   *  see sem_lock().
41   */
42  
43  #include 
  > 44  #include 
45  #include 
46  #include 
47  #include 
48  #include 
49  #include 
50  #include 
51  #include 
  > 52  #include 
53  #include 
54  #include 
55  #include 
56  #include 
57  #include 
58  #include 
59  #include 
60  #include 
61  #include 
62  
63  #include 
64  
65  #include "util.h"
66  
67  struct ipc_proc_iface {
68  const char *path;
69  const char *header;
70  int ids;
71  int (*show)(struct seq_file *, void *);
72  };
73  
74  /**
75   * ipc_init - initialise ipc subsystem
76   *
77   * The various sysv ipc resources (semaphores, messages and shared
78   * memory) are initialised.
79   *
80   * A callback routine is registered into the memory hotplug notifier
81   * chain: since msgmni scales to lowmem this callback routine will be
82   * called upon successful memory add / remove to recompute msmgni.
83   */
84  static int __init ipc_init(void)
85  {
86  sem_init();
87  msg_init();
88  shm_init();
89  return 0;
90  }
91  device_initcall(ipc_init);
92  
93  /**
94   * ipc_init_ids - initialise ipc identifiers
95   * @ids: ipc identifier set
96   *
97   * Set up the sequence range to use for the ipc identifier range 
(limited
98   * below IPCMNI) then initialise the ids idr.
99   */
   100  void ipc_init_ids(struct ipc_ids *ids)
   101  {
   102  ids->in_use = 0;
   103  ids->seq = 0;
   104  ids->next_id = -1;
   105  init_rwsem(>rwsem);
   106  idr_init(>ipcs_idr);
   107  }
   108  
   109  #ifdef CONFIG_PROC_FS
   110  static const struct 

Re: [PATCH] ipc: Convert kern_ipc_perm.refcount to refcount_t

[kbuild test robot]

Hi Elena,

[auto build test ERROR on next-20170526]
[cannot apply to linus/master linux/master kees/for-next/pstore v4.9-rc8 
v4.9-rc7 v4.9-rc6 v4.12-rc2]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Kees-Cook/ipc-Convert-kern_ipc_perm-refcount-to-refcount_t/20170528-040601
config: m32r-m32104ut_defconfig (attached as .config)
compiler: m32r-linux-gcc (GCC) 6.2.0
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=m32r 

All errors (new ones prefixed by >>):

   In file included from include/uapi/linux/shm.h:4:0,
from include/linux/shm.h:6,
from ipc/util.c:44:
   include/linux/ipc.h:25:2: error: unknown type name 'refcount_t'
 refcount_t refcount;
 ^~
   ipc/util.c: In function 'ipc_addid':
>> ipc/util.c:235:15: error: passing argument 1 of 'refcount_set' from 
>> incompatible pointer type [-Werror=incompatible-pointer-types]
 refcount_set(>refcount, 1);
  ^
   In file included from include/linux/key.h:26:0,
from include/linux/security.h:26,
from ipc/util.c:52:
   include/linux/refcount.h:28:20: note: expected 'refcount_t * {aka struct 
refcount_struct *}' but argument is of type 'int *'
static inline void refcount_set(refcount_t *r, unsigned int n)
   ^~~~
   ipc/util.c: In function 'ipc_rcu_getref':
>> ipc/util.c:400:31: error: passing argument 1 of 'refcount_inc_not_zero' from 
>> incompatible pointer type [-Werror=incompatible-pointer-types]
 return refcount_inc_not_zero(>refcount);
  ^
   In file included from include/linux/key.h:26:0,
from include/linux/security.h:26,
from ipc/util.c:52:
   include/linux/refcount.h:47:26: note: expected 'refcount_t * {aka struct 
refcount_struct *}' but argument is of type 'int *'
extern __must_check bool refcount_inc_not_zero(refcount_t *r);
 ^
   ipc/util.c: In function 'ipc_rcu_putref':
>> ipc/util.c:406:29: error: passing argument 1 of 'refcount_dec_and_test' from 
>> incompatible pointer type [-Werror=incompatible-pointer-types]
 if (!refcount_dec_and_test(>refcount))
^
   In file included from include/linux/key.h:26:0,
from include/linux/security.h:26,
from ipc/util.c:52:
   include/linux/refcount.h:53:26: note: expected 'refcount_t * {aka struct 
refcount_struct *}' but argument is of type 'int *'
extern __must_check bool refcount_dec_and_test(refcount_t *r);
 ^
   cc1: some warnings being treated as errors

vim +/refcount_set +235 ipc/util.c

38   *
39   *  Note that sems have a special fast path that avoids 
kern_ipc_perm.lock -
40   *  see sem_lock().
41   */
42  
43  #include 
  > 44  #include 
45  #include 
46  #include 
47  #include 
48  #include 
49  #include 
50  #include 
51  #include 
  > 52  #include 
53  #include 
54  #include 
55  #include 
56  #include 
57  #include 
58  #include 
59  #include 
60  #include 
61  #include 
62  
63  #include 
64  
65  #include "util.h"
66  
67  struct ipc_proc_iface {
68  const char *path;
69  const char *header;
70  int ids;
71  int (*show)(struct seq_file *, void *);
72  };
73  
74  /**
75   * ipc_init - initialise ipc subsystem
76   *
77   * The various sysv ipc resources (semaphores, messages and shared
78   * memory) are initialised.
79   *
80   * A callback routine is registered into the memory hotplug notifier
81   * chain: since msgmni scales to lowmem this callback routine will be
82   * called upon successful memory add / remove to recompute msmgni.
83   */
84  static int __init ipc_init(void)
85  {
86  sem_init();
87  msg_init();
88  shm_init();
89  return 0;
90  }
91  device_initcall(ipc_init);
92  
93  /**
94   * ipc_init_ids - initialise ipc identifiers
95   * @ids: ipc identifier set
96   *
97   * Set up the sequence range to use for the ipc identifier range 
(limited
98   * below IPCMNI) then initialise the ids idr.
99   */
   100  void ipc_init_ids(struct ipc_ids *ids)
   101  {
   102  ids->in_use = 0;
   103  ids->seq = 0;
   104  ids->next_id = -1;
   105  init_rwsem(>rwsem);
   106  idr_init(>ipcs_idr);
   107  }
   108  
   109  #ifdef CONFIG_PROC_FS
   110  static const struct 

GREETINGS FROM MR. MUSTAPHA ALI.

[mustapha ali]

Dear Friend.

Greetings.

My Name is Mustapha Ali, I am a banker by profession. I am from
Ouagadougou, Burkina Faso, West Africa. My reason of contacting you is
to transfer an abandoned fund $5M US Dollars to your account if you
agree with me.

The owner of this fund died since 2003 with his Next Of Kin. I want to
present you to the bank as the Next of Kin/beneficiary of this fund.

Please indicate your interest and willingness by sending the below
information for more clarification and confident to enable me feed you
with more details concerning the business deal.

(1) Your full name...
(2) Your age and sex
(3) Your contact address..
(4) Your private phone no..
(5) Fax number if Any..
(6) Your country of origin..
(7) Your occupation.’
(8) Your photo..

Further details of the transaction shall be forward to you as soon as
I receive your response indicating your interest in handling this
transaction.

 Have a Great Day,

Mustapha Ali.

GREETINGS FROM MR. MUSTAPHA ALI.

[mustapha ali]

Dear Friend.

Greetings.

My Name is Mustapha Ali, I am a banker by profession. I am from
Ouagadougou, Burkina Faso, West Africa. My reason of contacting you is
to transfer an abandoned fund $5M US Dollars to your account if you
agree with me.

The owner of this fund died since 2003 with his Next Of Kin. I want to
present you to the bank as the Next of Kin/beneficiary of this fund.

Please indicate your interest and willingness by sending the below
information for more clarification and confident to enable me feed you
with more details concerning the business deal.

(1) Your full name...
(2) Your age and sex
(3) Your contact address..
(4) Your private phone no..
(5) Fax number if Any..
(6) Your country of origin..
(7) Your occupation.’
(8) Your photo..

Further details of the transaction shall be forward to you as soon as
I receive your response indicating your interest in handling this
transaction.

 Have a Great Day,

Mustapha Ali.

Re: [PATCH 15/16] platform/x86: wmi-mof: New driver to expose embedded WMI MOF metadata

[Andy Lutomirski]

On Sat, May 27, 2017 at 4:14 AM, Pali Rohár  wrote:
> Hi! Note that in WMI is stored binary MOF (BMOF; .bmf file; compiled
> MOF), not ordinary MOF data which are plain text. So maybe it could make
> sense to include "B" into name of sysfs entry? Or not? (Just suggestion)
>
> On Saturday 27 May 2017 07:31:29 Darren Hart wrote:
>> From: Andy Lutomirski 
>>
>> Quite a few laptops (and maybe servers?) have embedded WMI MOF
>
> Not "a few", but "lots of" :-)
>
>> metadata. I think that Samba has tools to interpret it, but there is
>> currently no interface to get the data in the first place.
>
> No, there is no non-ms-windows tool for interpreting those binary MOF
> (BMF) data yet.
>
>> + priv->mofdata = wmidev_block_query(wdev, 0);
>> + if (!priv->mofdata) {
>> + dev_warn(>dev, "failed to read MOF\n");
>> + return -EIO;
>> + }
>> +
>> + if (priv->mofdata->type != ACPI_TYPE_BUFFER) {
>> + dev_warn(>dev, "MOF is not a buffer\n");
>> + ret = -EIO;
>> + goto err_free;
>> + }
>
> Are not those problems fatal for driver and therefore dev_err() better?
>
>> + sysfs_bin_attr_init(>mof_bin_attr);
>> + priv->mof_bin_attr.attr.name = "mof";
>> + priv->mof_bin_attr.attr.mode = 0400;
>
> 0400 means to be readable only by root? Is there then reason why normal
> user should not be able to read it?
>

I have no specific objection to making it 0444, but in general I'd
rather expose less information to unprivileged users rather than more.
I'm also having trouble imagining what an unprivileged user would do
with the MOF -- it's useful for reverse engineering and it may
eventually be useful for making WMI calls from userspace, but neither
of those is particularly useful to unprivileged users.

Re: [PATCH 15/16] platform/x86: wmi-mof: New driver to expose embedded WMI MOF metadata

[Andy Lutomirski]

On Sat, May 27, 2017 at 4:14 AM, Pali Rohár  wrote:
> Hi! Note that in WMI is stored binary MOF (BMOF; .bmf file; compiled
> MOF), not ordinary MOF data which are plain text. So maybe it could make
> sense to include "B" into name of sysfs entry? Or not? (Just suggestion)
>
> On Saturday 27 May 2017 07:31:29 Darren Hart wrote:
>> From: Andy Lutomirski 
>>
>> Quite a few laptops (and maybe servers?) have embedded WMI MOF
>
> Not "a few", but "lots of" :-)
>
>> metadata. I think that Samba has tools to interpret it, but there is
>> currently no interface to get the data in the first place.
>
> No, there is no non-ms-windows tool for interpreting those binary MOF
> (BMF) data yet.
>
>> + priv->mofdata = wmidev_block_query(wdev, 0);
>> + if (!priv->mofdata) {
>> + dev_warn(>dev, "failed to read MOF\n");
>> + return -EIO;
>> + }
>> +
>> + if (priv->mofdata->type != ACPI_TYPE_BUFFER) {
>> + dev_warn(>dev, "MOF is not a buffer\n");
>> + ret = -EIO;
>> + goto err_free;
>> + }
>
> Are not those problems fatal for driver and therefore dev_err() better?
>
>> + sysfs_bin_attr_init(>mof_bin_attr);
>> + priv->mof_bin_attr.attr.name = "mof";
>> + priv->mof_bin_attr.attr.mode = 0400;
>
> 0400 means to be readable only by root? Is there then reason why normal
> user should not be able to read it?
>

I have no specific objection to making it 0444, but in general I'd
rather expose less information to unprivileged users rather than more.
I'm also having trouble imagining what an unprivileged user would do
with the MOF -- it's useful for reverse engineering and it may
eventually be useful for making WMI calls from userspace, but neither
of those is particularly useful to unprivileged users.

Re: [PATCH] Doc*/media/uapi: fix control name

[Pavel Machek]

On Sat 2017-05-27 22:30:35, Sakari Ailus wrote:
> On Sat, May 27, 2017 at 10:12:40AM +0200, Pavel Machek wrote:
> > V4L2_CID_EXPOSURE_BIAS does not exist, fix documentation.
> > 
> > Signed-off-by: Pavel Machek 
> > 
> > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst 
> > b/Documentation/media/uapi/v4l/extended-controls.rst
> > index abb1057..76c5b1a 100644
> > --- a/Documentation/media/uapi/v4l/extended-controls.rst
> > +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> > @@ -2019,7 +2019,7 @@ enum v4l2_exposure_auto_type -
> >  dynamically vary the frame rate. By default this feature is disabled
> >  (0) and the frame rate must remain constant.
> >  
> > -``V4L2_CID_EXPOSURE_BIAS (integer menu)``
> > +``V4L2_CID_AUTO_EXPOSURE_BIAS (integer menu)``
> >  Determines the automatic exposure compensation, it is effective only
> >  when ``V4L2_CID_EXPOSURE_AUTO`` control is set to ``AUTO``,
> >  ``SHUTTER_PRIORITY`` or ``APERTURE_PRIORITY``. It is expressed in
> > 
> 
> Acked-by: Sakari Ailus 
> 
> Generally linux-media is enough, for other lists such as LKML this is just
> noise.

Well, this is what get_maintainer suggested. LKML is used to a lot of
traffic ;-) [I know, I am subscribed.]
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH] Doc*/media/uapi: fix control name

[Pavel Machek]

On Sat 2017-05-27 22:30:35, Sakari Ailus wrote:
> On Sat, May 27, 2017 at 10:12:40AM +0200, Pavel Machek wrote:
> > V4L2_CID_EXPOSURE_BIAS does not exist, fix documentation.
> > 
> > Signed-off-by: Pavel Machek 
> > 
> > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst 
> > b/Documentation/media/uapi/v4l/extended-controls.rst
> > index abb1057..76c5b1a 100644
> > --- a/Documentation/media/uapi/v4l/extended-controls.rst
> > +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> > @@ -2019,7 +2019,7 @@ enum v4l2_exposure_auto_type -
> >  dynamically vary the frame rate. By default this feature is disabled
> >  (0) and the frame rate must remain constant.
> >  
> > -``V4L2_CID_EXPOSURE_BIAS (integer menu)``
> > +``V4L2_CID_AUTO_EXPOSURE_BIAS (integer menu)``
> >  Determines the automatic exposure compensation, it is effective only
> >  when ``V4L2_CID_EXPOSURE_AUTO`` control is set to ``AUTO``,
> >  ``SHUTTER_PRIORITY`` or ``APERTURE_PRIORITY``. It is expressed in
> > 
> 
> Acked-by: Sakari Ailus 
> 
> Generally linux-media is enough, for other lists such as LKML this is just
> noise.

Well, this is what get_maintainer suggested. LKML is used to a lot of
traffic ;-) [I know, I am subscribed.]
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH v5] usb: typec: Add a sysfs node to manage port type

[Guenter Roeck]

On 05/27/2017 01:52 PM, Badhri Jagan Sridharan wrote:

User space applications in some cases have the need to enforce a
specific port type(DFP/UFP/DRP). This change allows userspace to
attempt setting the desired port type. Low level drivers can
however reject the request if the specific port type is not supported.

Signed-off-by: Badhri Jagan Sridharan 


Reviewed-by: Guenter Roeck 


---
Changelog since v1:
- introduced a new variable port_type in struct typec_port to track
   the current port type instead of changing type member in
   typec_capability to address Heikki Krogerus comments.
- changed the output format and strings that would be displayed as
   suggested by Heikki Krogerus.

Changelog since v2:
- introduced a new mutex lock to protect port_type for addressing
   the race conditions identified by Geunter Roeck
- added typec_port_types_drp for printing port_type when cap->type
   is TYPE_PORT_DRP as suggested by Geunter Roeck
- Power role swap and data role swaps would be rejected unless
   port port_type == TYPE_PORT_DRP
- port_type_store would return immediately if the current port_type
   is same as the new port_type as suggested by Geunter Roeck

Changelog since v3:
- Moved as much as code outside port_type_lock as suggested by
   Geunter Roeck
- Removed Change-Id line from commit message identified by
   Greg Kroah-Hartman

Changelog since v4:
- Corrected return value and moved moved one more line of code
   outside log in the port_type_store function as suggested


not that it matters: s/log/lock/


   by Geunter Roeck



and s/Geunter/Guenter/ ;-)


  drivers/usb/typec/typec.c | 106 +-
  include/linux/usb/typec.h |   4 ++
  2 files changed, 100 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/typec/typec.c b/drivers/usb/typec/typec.c
index 89e540bb7ff3..63644785ce31 100644
--- a/drivers/usb/typec/typec.c
+++ b/drivers/usb/typec/typec.c
@@ -11,6 +11,7 @@
  
  #include 

  #include 
+#include 
  #include 
  #include 
  
@@ -69,6 +70,8 @@ struct typec_port {

enum typec_role pwr_role;
enum typec_role vconn_role;
enum typec_pwr_opmode   pwr_opmode;
+   enum typec_port_typeport_type;
+   struct mutexport_type_lock;
  
  	const struct typec_capability	*cap;

  };
@@ -789,6 +792,18 @@ static const char * const typec_data_roles[] = {
[TYPEC_HOST]= "host",
  };
  
+static const char * const typec_port_types[] = {

+   [TYPEC_PORT_DFP] = "source",
+   [TYPEC_PORT_UFP] = "sink",
+   [TYPEC_PORT_DRP] = "dual",
+};
+
+static const char * const typec_port_types_drp[] = {
+   [TYPEC_PORT_DFP] = "dual [source] sink",
+   [TYPEC_PORT_UFP] = "dual source [sink]",
+   [TYPEC_PORT_DRP] = "[dual] source sink",
+};
+
  static ssize_t
  preferred_role_store(struct device *dev, struct device_attribute *attr,
 const char *buf, size_t size)
@@ -846,11 +861,6 @@ static ssize_t data_role_store(struct device *dev,
struct typec_port *port = to_typec_port(dev);
int ret;
  
-	if (port->cap->type != TYPEC_PORT_DRP) {

-   dev_dbg(dev, "data role swap only supported with DRP ports\n");
-   return -EOPNOTSUPP;
-   }
-
if (!port->cap->dr_set) {
dev_dbg(dev, "data role swapping not supported\n");
return -EOPNOTSUPP;
@@ -860,11 +870,22 @@ static ssize_t data_role_store(struct device *dev,
if (ret < 0)
return ret;
  
+	mutex_lock(>port_type_lock);

+   if (port->port_type != TYPEC_PORT_DRP) {
+   dev_dbg(dev, "port type fixed at \"%s\"",
+typec_port_types[port->port_type]);
+   ret = -EOPNOTSUPP;
+   goto unlock_and_ret;
+   }
+
ret = port->cap->dr_set(port->cap, ret);
if (ret)
-   return ret;
+   goto unlock_and_ret;
  
-	return size;

+   ret = size;
+unlock_and_ret:
+   mutex_unlock(>port_type_lock);
+   return ret;
  }
  
  static ssize_t data_role_show(struct device *dev,

@@ -885,7 +906,7 @@ static ssize_t power_role_store(struct device *dev,
const char *buf, size_t size)
  {
struct typec_port *port = to_typec_port(dev);
-   int ret = size;
+   int ret;
  
  	if (!port->cap->pd_revision) {

dev_dbg(dev, "USB Power Delivery not supported\n");
@@ -906,11 +927,22 @@ static ssize_t power_role_store(struct device *dev,
if (ret < 0)
return ret;
  
+	mutex_lock(>port_type_lock);

+   if (port->port_type != TYPEC_PORT_DRP) {
+   dev_dbg(dev, "port type fixed at \"%s\"",
+typec_port_types[port->port_type]);
+   ret = -EOPNOTSUPP;
+   goto unlock_and_ret;
+   }
+
ret = 

Re: [PATCH v5] usb: typec: Add a sysfs node to manage port type

[Guenter Roeck]

On 05/27/2017 01:52 PM, Badhri Jagan Sridharan wrote:

User space applications in some cases have the need to enforce a
specific port type(DFP/UFP/DRP). This change allows userspace to
attempt setting the desired port type. Low level drivers can
however reject the request if the specific port type is not supported.

Signed-off-by: Badhri Jagan Sridharan 


Reviewed-by: Guenter Roeck 


---
Changelog since v1:
- introduced a new variable port_type in struct typec_port to track
   the current port type instead of changing type member in
   typec_capability to address Heikki Krogerus comments.
- changed the output format and strings that would be displayed as
   suggested by Heikki Krogerus.

Changelog since v2:
- introduced a new mutex lock to protect port_type for addressing
   the race conditions identified by Geunter Roeck
- added typec_port_types_drp for printing port_type when cap->type
   is TYPE_PORT_DRP as suggested by Geunter Roeck
- Power role swap and data role swaps would be rejected unless
   port port_type == TYPE_PORT_DRP
- port_type_store would return immediately if the current port_type
   is same as the new port_type as suggested by Geunter Roeck

Changelog since v3:
- Moved as much as code outside port_type_lock as suggested by
   Geunter Roeck
- Removed Change-Id line from commit message identified by
   Greg Kroah-Hartman

Changelog since v4:
- Corrected return value and moved moved one more line of code
   outside log in the port_type_store function as suggested


not that it matters: s/log/lock/


   by Geunter Roeck



and s/Geunter/Guenter/ ;-)


  drivers/usb/typec/typec.c | 106 +-
  include/linux/usb/typec.h |   4 ++
  2 files changed, 100 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/typec/typec.c b/drivers/usb/typec/typec.c
index 89e540bb7ff3..63644785ce31 100644
--- a/drivers/usb/typec/typec.c
+++ b/drivers/usb/typec/typec.c
@@ -11,6 +11,7 @@
  
  #include 

  #include 
+#include 
  #include 
  #include 
  
@@ -69,6 +70,8 @@ struct typec_port {

enum typec_role pwr_role;
enum typec_role vconn_role;
enum typec_pwr_opmode   pwr_opmode;
+   enum typec_port_typeport_type;
+   struct mutexport_type_lock;
  
  	const struct typec_capability	*cap;

  };
@@ -789,6 +792,18 @@ static const char * const typec_data_roles[] = {
[TYPEC_HOST]= "host",
  };
  
+static const char * const typec_port_types[] = {

+   [TYPEC_PORT_DFP] = "source",
+   [TYPEC_PORT_UFP] = "sink",
+   [TYPEC_PORT_DRP] = "dual",
+};
+
+static const char * const typec_port_types_drp[] = {
+   [TYPEC_PORT_DFP] = "dual [source] sink",
+   [TYPEC_PORT_UFP] = "dual source [sink]",
+   [TYPEC_PORT_DRP] = "[dual] source sink",
+};
+
  static ssize_t
  preferred_role_store(struct device *dev, struct device_attribute *attr,
 const char *buf, size_t size)
@@ -846,11 +861,6 @@ static ssize_t data_role_store(struct device *dev,
struct typec_port *port = to_typec_port(dev);
int ret;
  
-	if (port->cap->type != TYPEC_PORT_DRP) {

-   dev_dbg(dev, "data role swap only supported with DRP ports\n");
-   return -EOPNOTSUPP;
-   }
-
if (!port->cap->dr_set) {
dev_dbg(dev, "data role swapping not supported\n");
return -EOPNOTSUPP;
@@ -860,11 +870,22 @@ static ssize_t data_role_store(struct device *dev,
if (ret < 0)
return ret;
  
+	mutex_lock(>port_type_lock);

+   if (port->port_type != TYPEC_PORT_DRP) {
+   dev_dbg(dev, "port type fixed at \"%s\"",
+typec_port_types[port->port_type]);
+   ret = -EOPNOTSUPP;
+   goto unlock_and_ret;
+   }
+
ret = port->cap->dr_set(port->cap, ret);
if (ret)
-   return ret;
+   goto unlock_and_ret;
  
-	return size;

+   ret = size;
+unlock_and_ret:
+   mutex_unlock(>port_type_lock);
+   return ret;
  }
  
  static ssize_t data_role_show(struct device *dev,

@@ -885,7 +906,7 @@ static ssize_t power_role_store(struct device *dev,
const char *buf, size_t size)
  {
struct typec_port *port = to_typec_port(dev);
-   int ret = size;
+   int ret;
  
  	if (!port->cap->pd_revision) {

dev_dbg(dev, "USB Power Delivery not supported\n");
@@ -906,11 +927,22 @@ static ssize_t power_role_store(struct device *dev,
if (ret < 0)
return ret;
  
+	mutex_lock(>port_type_lock);

+   if (port->port_type != TYPEC_PORT_DRP) {
+   dev_dbg(dev, "port type fixed at \"%s\"",
+typec_port_types[port->port_type]);
+   ret = -EOPNOTSUPP;
+   goto unlock_and_ret;
+   }
+
ret = port->cap->pr_set(port->cap, ret);
if (ret)
-  

[PATCH v5] usb: typec: Add a sysfs node to manage port type

[Badhri Jagan Sridharan]

User space applications in some cases have the need to enforce a
specific port type(DFP/UFP/DRP). This change allows userspace to
attempt setting the desired port type. Low level drivers can
however reject the request if the specific port type is not supported.

Signed-off-by: Badhri Jagan Sridharan 
---
Changelog since v1:
- introduced a new variable port_type in struct typec_port to track
  the current port type instead of changing type member in
  typec_capability to address Heikki Krogerus comments.
- changed the output format and strings that would be displayed as
  suggested by Heikki Krogerus.

Changelog since v2:
- introduced a new mutex lock to protect port_type for addressing
  the race conditions identified by Geunter Roeck
- added typec_port_types_drp for printing port_type when cap->type
  is TYPE_PORT_DRP as suggested by Geunter Roeck
- Power role swap and data role swaps would be rejected unless
  port port_type == TYPE_PORT_DRP
- port_type_store would return immediately if the current port_type
  is same as the new port_type as suggested by Geunter Roeck

Changelog since v3:
- Moved as much as code outside port_type_lock as suggested by
  Geunter Roeck
- Removed Change-Id line from commit message identified by
  Greg Kroah-Hartman

Changelog since v4:
- Corrected return value and moved moved one more line of code
  outside log in the port_type_store function as suggested
  by Geunter Roeck

 drivers/usb/typec/typec.c | 106 +-
 include/linux/usb/typec.h |   4 ++
 2 files changed, 100 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/typec/typec.c b/drivers/usb/typec/typec.c
index 89e540bb7ff3..63644785ce31 100644
--- a/drivers/usb/typec/typec.c
+++ b/drivers/usb/typec/typec.c
@@ -11,6 +11,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -69,6 +70,8 @@ struct typec_port {
enum typec_role pwr_role;
enum typec_role vconn_role;
enum typec_pwr_opmode   pwr_opmode;
+   enum typec_port_typeport_type;
+   struct mutexport_type_lock;
 
const struct typec_capability   *cap;
 };
@@ -789,6 +792,18 @@ static const char * const typec_data_roles[] = {
[TYPEC_HOST]= "host",
 };
 
+static const char * const typec_port_types[] = {
+   [TYPEC_PORT_DFP] = "source",
+   [TYPEC_PORT_UFP] = "sink",
+   [TYPEC_PORT_DRP] = "dual",
+};
+
+static const char * const typec_port_types_drp[] = {
+   [TYPEC_PORT_DFP] = "dual [source] sink",
+   [TYPEC_PORT_UFP] = "dual source [sink]",
+   [TYPEC_PORT_DRP] = "[dual] source sink",
+};
+
 static ssize_t
 preferred_role_store(struct device *dev, struct device_attribute *attr,
 const char *buf, size_t size)
@@ -846,11 +861,6 @@ static ssize_t data_role_store(struct device *dev,
struct typec_port *port = to_typec_port(dev);
int ret;
 
-   if (port->cap->type != TYPEC_PORT_DRP) {
-   dev_dbg(dev, "data role swap only supported with DRP ports\n");
-   return -EOPNOTSUPP;
-   }
-
if (!port->cap->dr_set) {
dev_dbg(dev, "data role swapping not supported\n");
return -EOPNOTSUPP;
@@ -860,11 +870,22 @@ static ssize_t data_role_store(struct device *dev,
if (ret < 0)
return ret;
 
+   mutex_lock(>port_type_lock);
+   if (port->port_type != TYPEC_PORT_DRP) {
+   dev_dbg(dev, "port type fixed at \"%s\"",
+typec_port_types[port->port_type]);
+   ret = -EOPNOTSUPP;
+   goto unlock_and_ret;
+   }
+
ret = port->cap->dr_set(port->cap, ret);
if (ret)
-   return ret;
+   goto unlock_and_ret;
 
-   return size;
+   ret = size;
+unlock_and_ret:
+   mutex_unlock(>port_type_lock);
+   return ret;
 }
 
 static ssize_t data_role_show(struct device *dev,
@@ -885,7 +906,7 @@ static ssize_t power_role_store(struct device *dev,
const char *buf, size_t size)
 {
struct typec_port *port = to_typec_port(dev);
-   int ret = size;
+   int ret;
 
if (!port->cap->pd_revision) {
dev_dbg(dev, "USB Power Delivery not supported\n");
@@ -906,11 +927,22 @@ static ssize_t power_role_store(struct device *dev,
if (ret < 0)
return ret;
 
+   mutex_lock(>port_type_lock);
+   if (port->port_type != TYPEC_PORT_DRP) {
+   dev_dbg(dev, "port type fixed at \"%s\"",
+typec_port_types[port->port_type]);
+   ret = -EOPNOTSUPP;
+   goto unlock_and_ret;
+   }
+
ret = port->cap->pr_set(port->cap, ret);
if (ret)
-   return ret;
+   goto unlock_and_ret;
 
-   return size;
+   ret = size;
+unlock_and_ret:
+   

[PATCH v5] usb: typec: Add a sysfs node to manage port type

[Badhri Jagan Sridharan]

User space applications in some cases have the need to enforce a
specific port type(DFP/UFP/DRP). This change allows userspace to
attempt setting the desired port type. Low level drivers can
however reject the request if the specific port type is not supported.

Signed-off-by: Badhri Jagan Sridharan 
---
Changelog since v1:
- introduced a new variable port_type in struct typec_port to track
  the current port type instead of changing type member in
  typec_capability to address Heikki Krogerus comments.
- changed the output format and strings that would be displayed as
  suggested by Heikki Krogerus.

Changelog since v2:
- introduced a new mutex lock to protect port_type for addressing
  the race conditions identified by Geunter Roeck
- added typec_port_types_drp for printing port_type when cap->type
  is TYPE_PORT_DRP as suggested by Geunter Roeck
- Power role swap and data role swaps would be rejected unless
  port port_type == TYPE_PORT_DRP
- port_type_store would return immediately if the current port_type
  is same as the new port_type as suggested by Geunter Roeck

Changelog since v3:
- Moved as much as code outside port_type_lock as suggested by
  Geunter Roeck
- Removed Change-Id line from commit message identified by
  Greg Kroah-Hartman

Changelog since v4:
- Corrected return value and moved moved one more line of code
  outside log in the port_type_store function as suggested
  by Geunter Roeck

 drivers/usb/typec/typec.c | 106 +-
 include/linux/usb/typec.h |   4 ++
 2 files changed, 100 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/typec/typec.c b/drivers/usb/typec/typec.c
index 89e540bb7ff3..63644785ce31 100644
--- a/drivers/usb/typec/typec.c
+++ b/drivers/usb/typec/typec.c
@@ -11,6 +11,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -69,6 +70,8 @@ struct typec_port {
enum typec_role pwr_role;
enum typec_role vconn_role;
enum typec_pwr_opmode   pwr_opmode;
+   enum typec_port_typeport_type;
+   struct mutexport_type_lock;
 
const struct typec_capability   *cap;
 };
@@ -789,6 +792,18 @@ static const char * const typec_data_roles[] = {
[TYPEC_HOST]= "host",
 };
 
+static const char * const typec_port_types[] = {
+   [TYPEC_PORT_DFP] = "source",
+   [TYPEC_PORT_UFP] = "sink",
+   [TYPEC_PORT_DRP] = "dual",
+};
+
+static const char * const typec_port_types_drp[] = {
+   [TYPEC_PORT_DFP] = "dual [source] sink",
+   [TYPEC_PORT_UFP] = "dual source [sink]",
+   [TYPEC_PORT_DRP] = "[dual] source sink",
+};
+
 static ssize_t
 preferred_role_store(struct device *dev, struct device_attribute *attr,
 const char *buf, size_t size)
@@ -846,11 +861,6 @@ static ssize_t data_role_store(struct device *dev,
struct typec_port *port = to_typec_port(dev);
int ret;
 
-   if (port->cap->type != TYPEC_PORT_DRP) {
-   dev_dbg(dev, "data role swap only supported with DRP ports\n");
-   return -EOPNOTSUPP;
-   }
-
if (!port->cap->dr_set) {
dev_dbg(dev, "data role swapping not supported\n");
return -EOPNOTSUPP;
@@ -860,11 +870,22 @@ static ssize_t data_role_store(struct device *dev,
if (ret < 0)
return ret;
 
+   mutex_lock(>port_type_lock);
+   if (port->port_type != TYPEC_PORT_DRP) {
+   dev_dbg(dev, "port type fixed at \"%s\"",
+typec_port_types[port->port_type]);
+   ret = -EOPNOTSUPP;
+   goto unlock_and_ret;
+   }
+
ret = port->cap->dr_set(port->cap, ret);
if (ret)
-   return ret;
+   goto unlock_and_ret;
 
-   return size;
+   ret = size;
+unlock_and_ret:
+   mutex_unlock(>port_type_lock);
+   return ret;
 }
 
 static ssize_t data_role_show(struct device *dev,
@@ -885,7 +906,7 @@ static ssize_t power_role_store(struct device *dev,
const char *buf, size_t size)
 {
struct typec_port *port = to_typec_port(dev);
-   int ret = size;
+   int ret;
 
if (!port->cap->pd_revision) {
dev_dbg(dev, "USB Power Delivery not supported\n");
@@ -906,11 +927,22 @@ static ssize_t power_role_store(struct device *dev,
if (ret < 0)
return ret;
 
+   mutex_lock(>port_type_lock);
+   if (port->port_type != TYPEC_PORT_DRP) {
+   dev_dbg(dev, "port type fixed at \"%s\"",
+typec_port_types[port->port_type]);
+   ret = -EOPNOTSUPP;
+   goto unlock_and_ret;
+   }
+
ret = port->cap->pr_set(port->cap, ret);
if (ret)
-   return ret;
+   goto unlock_and_ret;
 
-   return size;
+   ret = size;
+unlock_and_ret:
+   mutex_unlock(>port_type_lock);
+   

Re: [PATCH] platform/x86: wmi: Fix printing info about WDG structure

[Andy Shevchenko]

On Sat, May 27, 2017 at 11:48 PM, Pali Rohár  wrote:
> On Saturday 27 May 2017 15:33:14 Andy Shevchenko wrote:
>> On Sat, May 27, 2017 at 4:17 PM, Pali Rohár 
>> wrote:

>> Okay, got it. But on your opinion does it make sense to do
>>
>> pr_info("\tobject_id: %2pE\n", g->object_id);
>>
>> instead?
>>
>> For ASCII it will go as is previously, otherwise escaping would be
>> done.
>
> Both is OK for me. Do you want to send a new patch with %2pE?

To me it looks slightly cleaner.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] platform/x86: wmi: Fix printing info about WDG structure

[Andy Shevchenko]

On Sat, May 27, 2017 at 11:48 PM, Pali Rohár  wrote:
> On Saturday 27 May 2017 15:33:14 Andy Shevchenko wrote:
>> On Sat, May 27, 2017 at 4:17 PM, Pali Rohár 
>> wrote:

>> Okay, got it. But on your opinion does it make sense to do
>>
>> pr_info("\tobject_id: %2pE\n", g->object_id);
>>
>> instead?
>>
>> For ASCII it will go as is previously, otherwise escaping would be
>> done.
>
> Both is OK for me. Do you want to send a new patch with %2pE?

To me it looks slightly cleaner.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] platform/x86: wmi: Fix printing info about WDG structure

[Pali Rohár]

On Saturday 27 May 2017 15:33:14 Andy Shevchenko wrote:
> On Sat, May 27, 2017 at 4:17 PM, Pali Rohár 
> wrote:
> > On Saturday 27 May 2017 15:07:09 Andy Shevchenko wrote:
> >> On Sat, May 27, 2017 at 2:51 PM, Pali Rohár 
> >> 
> >> wrote:
> >> > Remove
> >> > also reserved member as it does not have any defined meaning or
> >> > type yet.
> >> > 
> >> > -   pr_info("\treserved: %02X\n", g->reserved);
> >> 
> >> Do we need this? Commit message doesn't clarify.
> > 
> > I wrote to commit message that reserved does not have defined
> > meaning nor type. Also reserved overlap with object_id[1] so for
> > non-event should not be print at all. And as it is reserved, I
> > removed it.
> 
> Oh, indeed.
> 
> >> > +   pr_info("\tobject_id: %c%c\n", g->object_id[0],
> >> > g->object_id[1]);
> >> 
> >> If this can still contain non-printable characters the %*pE can
> >> help instead.
> > 
> > Those are printable ASCII. object_id contains two characters which
> > are suffix for ACPI method.
> > 
> > Problem was only for events when we tried to print notify_id as
> > object_id. notify_id is binary and overlaps with object_id.
> 
> Okay, got it. But on your opinion does it make sense to do
> 
> pr_info("\tobject_id: %2pE\n", g->object_id);
> 
> instead?
> 
> For ASCII it will go as is previously, otherwise escaping would be
> done.

Both is OK for me. Do you want to send a new patch with %2pE?

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH] platform/x86: wmi: Fix printing info about WDG structure

[Pali Rohár]

On Saturday 27 May 2017 15:33:14 Andy Shevchenko wrote:
> On Sat, May 27, 2017 at 4:17 PM, Pali Rohár 
> wrote:
> > On Saturday 27 May 2017 15:07:09 Andy Shevchenko wrote:
> >> On Sat, May 27, 2017 at 2:51 PM, Pali Rohár 
> >> 
> >> wrote:
> >> > Remove
> >> > also reserved member as it does not have any defined meaning or
> >> > type yet.
> >> > 
> >> > -   pr_info("\treserved: %02X\n", g->reserved);
> >> 
> >> Do we need this? Commit message doesn't clarify.
> > 
> > I wrote to commit message that reserved does not have defined
> > meaning nor type. Also reserved overlap with object_id[1] so for
> > non-event should not be print at all. And as it is reserved, I
> > removed it.
> 
> Oh, indeed.
> 
> >> > +   pr_info("\tobject_id: %c%c\n", g->object_id[0],
> >> > g->object_id[1]);
> >> 
> >> If this can still contain non-printable characters the %*pE can
> >> help instead.
> > 
> > Those are printable ASCII. object_id contains two characters which
> > are suffix for ACPI method.
> > 
> > Problem was only for events when we tried to print notify_id as
> > object_id. notify_id is binary and overlaps with object_id.
> 
> Okay, got it. But on your opinion does it make sense to do
> 
> pr_info("\tobject_id: %2pE\n", g->object_id);
> 
> instead?
> 
> For ASCII it will go as is previously, otherwise escaping would be
> done.

Both is OK for me. Do you want to send a new patch with %2pE?

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH v2 24/27] thunderbolt: Add support for Internal Connection Manager (ICM)

[Andy Shevchenko]

On Fri, May 26, 2017 at 7:09 PM, Mika Westerberg
 wrote:

> +static inline u8 dual_link_from_link(u8 link)
> +{
> +   return link ? ((link - 1) ^ 0x01) + 1 : 0;
> +}

I have got better one (IIUC):

  return (link + 1) ^ 0x01) - 1;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 24/27] thunderbolt: Add support for Internal Connection Manager (ICM)

[Andy Shevchenko]

On Fri, May 26, 2017 at 7:09 PM, Mika Westerberg
 wrote:

> +static inline u8 dual_link_from_link(u8 link)
> +{
> +   return link ? ((link - 1) ^ 0x01) + 1 : 0;
> +}

I have got better one (IIUC):

  return (link + 1) ^ 0x01) - 1;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4] usb: typec: Add a sysfs node to manage port type

[Badhri Jagan Sridharan]

Thanks Geunter for comments. Will fix them in my next patch.

On Fri, May 26, 2017 at 7:24 PM, Guenter Roeck  wrote:
> On 05/26/2017 04:07 PM, Badhri Jagan Sridharan wrote:
>>
>> User space applications in some cases have the need to enforce a
>> specific port type(DFP/UFP/DRP). This change allows userspace to
>> attempt setting the desired port type. Low level drivers can
>> however reject the request if the specific port type is not supported.
>>
>> Signed-off-by: Badhri Jagan Sridharan 
>> ---
>> Changelog since v1:
>> - introduced a new variable port_type in struct typec_port to track
>>the current port type instead of changing type member in
>>typec_capability to address Heikki Krogerus comments.
>> - changed the output format and strings that would be displayed as
>>suggested by Heikki Krogerus.
>>
>> Changelog since v2:
>> - introduced a new mutex lock to protect port_type for addressing
>>the race conditions identified by Geunter Roeck
>> - added typec_port_types_drp for printing port_type when cap->type
>>is TYPE_PORT_DRP as suggested by Geunter Roeck
>> - Power role swap and data role swaps would be rejected unless
>>port port_type == TYPE_PORT_DRP
>> - port_type_store would return immediately if the current port_type
>>is same as the new port_type as suggested by Geunter Roeck
>>
>> Changelog since v3:
>> - Moved as much as code outside port_type_lock as suggested by
>>Geunter Roeck
>> - Removed Change-Id line from commit message identified by
>>Greg Kroah-Hartman
>>
>>   drivers/usb/typec/typec.c | 106
>> +-
>>   include/linux/usb/typec.h |   4 ++
>>   2 files changed, 100 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/usb/typec/typec.c b/drivers/usb/typec/typec.c
>> index 89e540bb7ff3..bf1eb11c6646 100644
>> --- a/drivers/usb/typec/typec.c
>> +++ b/drivers/usb/typec/typec.c
>> @@ -11,6 +11,7 @@
>> #include 
>>   #include 
>> +#include 
>>   #include 
>>   #include 
>>   @@ -69,6 +70,8 @@ struct typec_port {
>> enum typec_role pwr_role;
>> enum typec_role vconn_role;
>> enum typec_pwr_opmode   pwr_opmode;
>> +   enum typec_port_typeport_type;
>> +   struct mutexport_type_lock;
>> const struct typec_capability   *cap;
>>   };
>> @@ -789,6 +792,18 @@ static const char * const typec_data_roles[] = {
>> [TYPEC_HOST]= "host",
>>   };
>>   +static const char * const typec_port_types[] = {
>> +   [TYPEC_PORT_DFP] = "source",
>> +   [TYPEC_PORT_UFP] = "sink",
>> +   [TYPEC_PORT_DRP] = "dual",
>> +};
>> +
>> +static const char * const typec_port_types_drp[] = {
>> +   [TYPEC_PORT_DFP] = "dual [source] sink",
>> +   [TYPEC_PORT_UFP] = "dual source [sink]",
>> +   [TYPEC_PORT_DRP] = "[dual] source sink",
>> +};
>> +
>>   static ssize_t
>>   preferred_role_store(struct device *dev, struct device_attribute *attr,
>>  const char *buf, size_t size)
>> @@ -846,11 +861,6 @@ static ssize_t data_role_store(struct device *dev,
>> struct typec_port *port = to_typec_port(dev);
>> int ret;
>>   - if (port->cap->type != TYPEC_PORT_DRP) {
>> -   dev_dbg(dev, "data role swap only supported with DRP
>> ports\n");
>> -   return -EOPNOTSUPP;
>> -   }
>> -
>> if (!port->cap->dr_set) {
>> dev_dbg(dev, "data role swapping not supported\n");
>> return -EOPNOTSUPP;
>> @@ -860,11 +870,22 @@ static ssize_t data_role_store(struct device *dev,
>> if (ret < 0)
>> return ret;
>>   + mutex_lock(>port_type_lock);
>> +   if (port->port_type != TYPEC_PORT_DRP) {
>> +   dev_dbg(dev, "port type fixed at \"%s\"",
>> +typec_port_types[port->port_type]);
>> +   ret = -EOPNOTSUPP;
>> +   goto unlock_and_ret;
>> +   }
>> +
>> ret = port->cap->dr_set(port->cap, ret);
>> if (ret)
>> -   return ret;
>> +   goto unlock_and_ret;
>>   - return size;
>> +   ret = size;
>> +unlock_and_ret:
>> +   mutex_unlock(>port_type_lock);
>> +   return ret;
>>   }
>> static ssize_t data_role_show(struct device *dev,
>> @@ -885,7 +906,7 @@ static ssize_t power_role_store(struct device *dev,
>> const char *buf, size_t size)
>>   {
>> struct typec_port *port = to_typec_port(dev);
>> -   int ret = size;
>> +   int ret;
>> if (!port->cap->pd_revision) {
>> dev_dbg(dev, "USB Power Delivery not supported\n");
>> @@ -906,11 +927,22 @@ static ssize_t power_role_store(struct device *dev,
>> if (ret < 0)
>> return ret;
>>   + mutex_lock(>port_type_lock);
>> +   if (port->port_type != TYPEC_PORT_DRP) {
>> +   dev_dbg(dev, 

Re: [PATCHv4] usb: typec: Add a sysfs node to manage port type

[Badhri Jagan Sridharan]

On Fri, May 26, 2017 at 10:43 PM, Greg Kroah-Hartman
 wrote:
> On Fri, May 26, 2017 at 01:42:57PM -0700, Badhri Jagan Sridharan wrote:
>> User space applications in some cases have the need to enforce a
>> specific port type(DFP/UFP/DRP). This change allows userspace to
>> attempt setting the desired port type. Low level drivers can
>> however reject the request if the specific port type is not supported.
>>
>> Signed-off-by: Badhri Jagan Sridharan 
>> ---
>> Changelog since v1:
>> - introduced a new variable port_type in struct typec_port to track
>>   the current port type instead of changing type member in
>>   typec_capability to address Heikki Krogerus comments.
>> - changed the output format and strings that would be displayed as
>>   suggested by Heikki Krogerus.
>>
>> Changelog since v2:
>> - introduced a new mutex lock to protect port_type for addressing
>>   the race conditions identified by Geunter Roeck
>> - added typec_port_types_drp for printing port_type when cap->type
>>   is TYPE_PORT_DRP as suggested by Geunter Roeck
>> - Power role swap and data role swaps would be rejected unless
>>   port port_type == TYPE_PORT_DRP
>> - port_type_store would return immediately if the current port_type
>>   is same as the new port_type as suggested by Geunter Roeck
>>
>> Changelog since v3:
>> - Moved as much as code outside port_type_lock as suggested by
>>   Geunter Roeck
>> - Removed Change-Id line from commit message identified by
>>   Greg Kroah-Hartman
>
> Ok, this is how you write a changelog for a patch, very nice job!

Thanks Greg :)

>
> greg k-h