* [PATCH v7 0/6] Implement qspinlock/pv-qspinlock on ppc
From: Pan Xinhui @ 2016-09-19 9:23 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev
Cc: xinhui.pan, peterz, benh, waiman.long, virtualization, mingo,
paulus, mpe, paulmck
Hi All,
this is the fairlock patchset. You can apply them and build successfully.
patches are based on 4.8-rc4.
qspinlock can avoid waiter starved issue. It has about the same speed in
single-thread and it can be much faster in high contention situations
especially when the spinlock is embedded within the data structure to be
protected.
v6 -> v7:
rebase onto 4.8-rc4
no changelog anymore, sorry for that. I hope there is a very careful review.
Todo:
we can save one function call overhead. As we can use feature-fixup to patch
the binary code. Currently there is pv_lock_ops->lock(lock) and ->unlock(lock) to acquire/release the lock.
some benchmark result below
perf bench
these numbers are ops per sec, So the higher the better.
*******************************************
on pSeries with 32 vcpus, 32Gb memory, pHyp.
------------------------------------------------------------------------------------
test case | pv-qspinlock | qspinlock | current-spinlock
------------------------------------------------------------------------------------
futex hash | 618572 | 552332 | 553788
futex lock-pi | 364 | 364 | 364
sched pipe | 78984 | 76060 | 81454
------------------------------------------------------------------------------------
unix bench:
these numbers are scores, So the higher the better.
************************************************
on PowerNV with 16 cores(cpus) (smt off), 32Gb memory:
-------------
pv-qspinlock and qspinlock have very similar results because pv-qspinlock use native version
which is only having one callback overhead
------------------------------------------------------------------------------------
test case | pv-qspinlock and qspinlock | current-spinlock
------------------------------------------------------------------------------------
Execl Throughput 761.1 761.4
File Copy 1024 bufsize 2000 maxblocks 1259.8 1286.6
File Copy 256 bufsize 500 maxblocks 782.2 790.3
File Copy 4096 bufsize 8000 maxblocks 2741.5 2817.4
Pipe Throughput 1063.2 1036.7
Pipe-based Context Switching 284.7 281.1
Process Creation 679.6 649.1
Shell Scripts (1 concurrent) 1933.2 1922.9
Shell Scripts (8 concurrent) 5003.3 4899.8
System Call Overhead 900.6 896.8
==========================
System Benchmarks Index Score 1139.3 1133.0
--------------------------------------------------------------------------- ---------
*******************************************
on pSeries with 32 vcpus, 32Gb memory, pHyp.
------------------------------------------------------------------------------------
test case | pv-qspinlock | qspinlock | current-spinlock
------------------------------------------------------------------------------------
Execl Throughput 877.1 891.2 872.8
File Copy 1024 bufsize 2000 maxblocks 1390.4 1399.2 1395.0
File Copy 256 bufsize 500 maxblocks 882.4 889.5 881.8
File Copy 4096 bufsize 8000 maxblocks 3112.3 3113.4 3121.7
Pipe Throughput 1095.8 1162.6 1158.5
Pipe-based Context Switching 194.9 192.7 200.7
Process Creation 518.4 526.4 509.1
Shell Scripts (1 concurrent) 1401.9 1413.9 1402.2
Shell Scripts (8 concurrent) 3215.6 3246.6 3229.1
System Call Overhead 833.2 892.4 888.1
====================================
System Benchmarks Index Score 1033.7 1052.5 1047.8
------------------------------------------------------------------------------------
******************************************
on pSeries with 32 vcpus, 16Gb memory, KVM.
------------------------------------------------------------------------------------
test case | pv-qspinlock | qspinlock | current-spinlock
------------------------------------------------------------------------------------
Execl Throughput 497.4 518.7 497.8
File Copy 1024 bufsize 2000 maxblocks 1368.8 1390.1 1343.3
File Copy 256 bufsize 500 maxblocks 857.7 859.8 831.4
File Copy 4096 bufsize 8000 maxblocks 2851.7 2838.1 2785.5
Pipe Throughput 1221.9 1265.3 1250.4
Pipe-based Context Switching 529.8 578.1 564.2
Process Creation 408.4 421.6 287.6
Shell Scripts (1 concurrent) 1201.8 1215.3 1185.8
Shell Scripts (8 concurrent) 3758.4 3799.3 3878.9
System Call Overhead 1008.3 1122.6 1134.2
=====================================
System Benchmarks Index Score 1072.0 1108.9 1050.6
------------------------------------------------------------------------------------
Pan Xinhui (6):
pv-qspinlock: use cmpxchg_release in __pv_queued_spin_unlock
powerpc/qspinlock: powerpc support qspinlock
powerpc: pseries/Kconfig: Add qspinlock build config
powerpc: lib/locks.c: Add cpu yield/wake helper function
powerpc/pv-qspinlock: powerpc support pv-qspinlock
powerpc: pSeries: Add pv-qspinlock build config/make
arch/powerpc/include/asm/qspinlock.h | 93 +++++++++++++
arch/powerpc/include/asm/qspinlock_paravirt.h | 36 +++++
.../powerpc/include/asm/qspinlock_paravirt_types.h | 13 ++
arch/powerpc/include/asm/spinlock.h | 35 +++--
arch/powerpc/include/asm/spinlock_types.h | 4 +
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/kernel/paravirt.c | 153 +++++++++++++++++++++
arch/powerpc/lib/locks.c | 122 ++++++++++++++++
arch/powerpc/platforms/pseries/Kconfig | 9 ++
arch/powerpc/platforms/pseries/setup.c | 5 +
kernel/locking/qspinlock_paravirt.h | 2 +-
11 files changed, 459 insertions(+), 14 deletions(-)
create mode 100644 arch/powerpc/include/asm/qspinlock.h
create mode 100644 arch/powerpc/include/asm/qspinlock_paravirt.h
create mode 100644 arch/powerpc/include/asm/qspinlock_paravirt_types.h
create mode 100644 arch/powerpc/kernel/paravirt.c
--
2.4.11
^ permalink raw reply
* Re: [PATCH v7 6/6] powerpc: pSeries: Add pv-qspinlock build config/make
From: kbuild test robot @ 2016-09-19 8:58 UTC (permalink / raw)
Cc: xinhui.pan, peterz, mpe, linux-kernel, waiman.long,
virtualization, mingo, paulus, kbuild-all, benh, paulmck,
linuxppc-dev
In-Reply-To: <1474277037-15200-7-git-send-email-xinhui.pan@linux.vnet.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 10966 bytes --]
Hi Pan,
[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.8-rc7 next-20160916]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Pan-Xinhui/Implement-qspinlock-pv-qspinlock-on-ppc/20160919-133130
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc
All errors (new ones prefixed by >>):
include/linux/compiler.h:491:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/bug.h:51:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/cmpxchg.h:326:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(1, "Unsupported size for __cmpxchg");
^~~~~~~~~~~~~~~~
In function '__cmpxchg',
inlined from 'pv_wait_node' at kernel/locking/qspinlock_paravirt.h:328:3,
inlined from '__pv_queued_spin_lock_slowpath' at kernel/locking/qspinlock.c:538:3:
include/linux/compiler.h:491:38: error: call to '__compiletime_assert_326' declared with attribute error: Unsupported size for __cmpxchg
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^
include/linux/compiler.h:474:4: note: in definition of macro '__compiletime_assert'
prefix ## suffix(); \
^~~~~~
include/linux/compiler.h:491:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/bug.h:51:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/cmpxchg.h:326:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(1, "Unsupported size for __cmpxchg");
^~~~~~~~~~~~~~~~
In function '__cmpxchg',
inlined from 'pv_wait_head_or_lock' at kernel/locking/qspinlock_paravirt.h:109:10,
inlined from '__pv_queued_spin_lock_slowpath' at kernel/locking/qspinlock.c:573:5:
include/linux/compiler.h:491:38: error: call to '__compiletime_assert_326' declared with attribute error: Unsupported size for __cmpxchg
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^
include/linux/compiler.h:474:4: note: in definition of macro '__compiletime_assert'
prefix ## suffix(); \
^~~~~~
include/linux/compiler.h:491:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/bug.h:51:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/cmpxchg.h:326:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(1, "Unsupported size for __cmpxchg");
^~~~~~~~~~~~~~~~
In function '__xchg_relaxed',
inlined from 'pv_wait_head_or_lock' at kernel/locking/qspinlock_paravirt.h:442:8,
inlined from '__pv_queued_spin_lock_slowpath' at kernel/locking/qspinlock.c:573:5:
include/linux/compiler.h:491:38: error: call to '__compiletime_assert_113' declared with attribute error: Unsupported size for __xchg_local
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^
include/linux/compiler.h:474:4: note: in definition of macro '__compiletime_assert'
prefix ## suffix(); \
^~~~~~
include/linux/compiler.h:491:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/bug.h:51:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/cmpxchg.h:113:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(1, "Unsupported size for __xchg_local");
^~~~~~~~~~~~~~~~
In function '__cmpxchg',
inlined from 'pv_kick_node' at kernel/locking/qspinlock_paravirt.h:366:6,
inlined from '__pv_queued_spin_lock_slowpath' at kernel/locking/qspinlock.c:616:2:
include/linux/compiler.h:491:38: error: call to '__compiletime_assert_326' declared with attribute error: Unsupported size for __cmpxchg
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^
include/linux/compiler.h:474:4: note: in definition of macro '__compiletime_assert'
prefix ## suffix(); \
^~~~~~
include/linux/compiler.h:491:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/bug.h:51:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/cmpxchg.h:326:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(1, "Unsupported size for __cmpxchg");
^~~~~~~~~~~~~~~~
In function '__xchg_relaxed',
inlined from '__pv_queued_spin_lock_slowpath' at kernel/locking/qspinlock.c:184:14:
include/linux/compiler.h:491:38: error: call to '__compiletime_assert_113' declared with attribute error: Unsupported size for __xchg_local
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^
include/linux/compiler.h:474:4: note: in definition of macro '__compiletime_assert'
prefix ## suffix(); \
^~~~~~
include/linux/compiler.h:491:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/bug.h:51:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/cmpxchg.h:113:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(1, "Unsupported size for __xchg_local");
^~~~~~~~~~~~~~~~
In function '__cmpxchg_relaxed',
inlined from '__pv_queued_spin_unlock' at kernel/locking/qspinlock_paravirt.h:547:11:
>> include/linux/compiler.h:491:38: error: call to '__compiletime_assert_358' declared with attribute error: Unsupported size for __cmpxchg_relaxed
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^
include/linux/compiler.h:474:4: note: in definition of macro '__compiletime_assert'
prefix ## suffix(); \
^~~~~~
include/linux/compiler.h:491:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/bug.h:51:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/cmpxchg.h:358:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(1, "Unsupported size for __cmpxchg_relaxed");
^~~~~~~~~~~~~~~~
vim +/__compiletime_assert_358 +491 include/linux/compiler.h
9a8ab1c3 Daniel Santos 2013-02-21 475 __compiletime_error_fallback(__cond); \
9a8ab1c3 Daniel Santos 2013-02-21 476 } while (0)
9a8ab1c3 Daniel Santos 2013-02-21 477
9a8ab1c3 Daniel Santos 2013-02-21 478 #define _compiletime_assert(condition, msg, prefix, suffix) \
9a8ab1c3 Daniel Santos 2013-02-21 479 __compiletime_assert(condition, msg, prefix, suffix)
9a8ab1c3 Daniel Santos 2013-02-21 480
9a8ab1c3 Daniel Santos 2013-02-21 481 /**
9a8ab1c3 Daniel Santos 2013-02-21 482 * compiletime_assert - break build and emit msg if condition is false
9a8ab1c3 Daniel Santos 2013-02-21 483 * @condition: a compile-time constant condition to check
9a8ab1c3 Daniel Santos 2013-02-21 484 * @msg: a message to emit if condition is false
9a8ab1c3 Daniel Santos 2013-02-21 485 *
9a8ab1c3 Daniel Santos 2013-02-21 486 * In tradition of POSIX assert, this macro will break the build if the
9a8ab1c3 Daniel Santos 2013-02-21 487 * supplied condition is *false*, emitting the supplied error message if the
9a8ab1c3 Daniel Santos 2013-02-21 488 * compiler has support to do so.
9a8ab1c3 Daniel Santos 2013-02-21 489 */
9a8ab1c3 Daniel Santos 2013-02-21 490 #define compiletime_assert(condition, msg) \
9a8ab1c3 Daniel Santos 2013-02-21 @491 _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
9a8ab1c3 Daniel Santos 2013-02-21 492
47933ad4 Peter Zijlstra 2013-11-06 493 #define compiletime_assert_atomic_type(t) \
47933ad4 Peter Zijlstra 2013-11-06 494 compiletime_assert(__native_word(t), \
47933ad4 Peter Zijlstra 2013-11-06 495 "Need native word sized stores/loads for atomicity.")
47933ad4 Peter Zijlstra 2013-11-06 496
9c3cdc1f Linus Torvalds 2008-05-10 497 /*
9c3cdc1f Linus Torvalds 2008-05-10 498 * Prevent the compiler from merging or refetching accesses. The compiler
9c3cdc1f Linus Torvalds 2008-05-10 499 * is also forbidden from reordering successive instances of ACCESS_ONCE(),
:::::: The code at line 491 was first introduced by commit
:::::: 9a8ab1c39970a4938a72d94e6fd13be88a797590 bug.h, compiler.h: introduce compiletime_assert & BUILD_BUG_ON_MSG
:::::: TO: Daniel Santos <daniel.santos@pobox.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 49942 bytes --]
[-- Attachment #3: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v7 3/6] powerpc: pseries/Kconfig: Add qspinlock build config
From: kbuild test robot @ 2016-09-19 8:40 UTC (permalink / raw)
Cc: xinhui.pan, peterz, mpe, linux-kernel, waiman.long,
virtualization, mingo, paulus, kbuild-all, benh, paulmck,
linuxppc-dev
In-Reply-To: <1474277037-15200-4-git-send-email-xinhui.pan@linux.vnet.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 5009 bytes --]
Hi Pan,
[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.8-rc7 next-20160916]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Pan-Xinhui/Implement-qspinlock-pv-qspinlock-on-ppc/20160919-133130
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc
All errors (new ones prefixed by >>):
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/linux/smp.h:10,
from kernel/locking/qspinlock.c:25:
In function '__xchg_relaxed',
inlined from 'queued_spin_lock_slowpath' at kernel/locking/qspinlock.c:184:14:
>> include/linux/compiler.h:491:38: error: call to '__compiletime_assert_113' declared with attribute error: Unsupported size for __xchg_local
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^
include/linux/compiler.h:474:4: note: in definition of macro '__compiletime_assert'
prefix ## suffix(); \
^~~~~~
include/linux/compiler.h:491:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/bug.h:51:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/cmpxchg.h:113:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(1, "Unsupported size for __xchg_local");
^~~~~~~~~~~~~~~~
vim +/__compiletime_assert_113 +491 include/linux/compiler.h
9a8ab1c3 Daniel Santos 2013-02-21 475 __compiletime_error_fallback(__cond); \
9a8ab1c3 Daniel Santos 2013-02-21 476 } while (0)
9a8ab1c3 Daniel Santos 2013-02-21 477
9a8ab1c3 Daniel Santos 2013-02-21 478 #define _compiletime_assert(condition, msg, prefix, suffix) \
9a8ab1c3 Daniel Santos 2013-02-21 479 __compiletime_assert(condition, msg, prefix, suffix)
9a8ab1c3 Daniel Santos 2013-02-21 480
9a8ab1c3 Daniel Santos 2013-02-21 481 /**
9a8ab1c3 Daniel Santos 2013-02-21 482 * compiletime_assert - break build and emit msg if condition is false
9a8ab1c3 Daniel Santos 2013-02-21 483 * @condition: a compile-time constant condition to check
9a8ab1c3 Daniel Santos 2013-02-21 484 * @msg: a message to emit if condition is false
9a8ab1c3 Daniel Santos 2013-02-21 485 *
9a8ab1c3 Daniel Santos 2013-02-21 486 * In tradition of POSIX assert, this macro will break the build if the
9a8ab1c3 Daniel Santos 2013-02-21 487 * supplied condition is *false*, emitting the supplied error message if the
9a8ab1c3 Daniel Santos 2013-02-21 488 * compiler has support to do so.
9a8ab1c3 Daniel Santos 2013-02-21 489 */
9a8ab1c3 Daniel Santos 2013-02-21 490 #define compiletime_assert(condition, msg) \
9a8ab1c3 Daniel Santos 2013-02-21 @491 _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
9a8ab1c3 Daniel Santos 2013-02-21 492
47933ad4 Peter Zijlstra 2013-11-06 493 #define compiletime_assert_atomic_type(t) \
47933ad4 Peter Zijlstra 2013-11-06 494 compiletime_assert(__native_word(t), \
47933ad4 Peter Zijlstra 2013-11-06 495 "Need native word sized stores/loads for atomicity.")
47933ad4 Peter Zijlstra 2013-11-06 496
9c3cdc1f Linus Torvalds 2008-05-10 497 /*
9c3cdc1f Linus Torvalds 2008-05-10 498 * Prevent the compiler from merging or refetching accesses. The compiler
9c3cdc1f Linus Torvalds 2008-05-10 499 * is also forbidden from reordering successive instances of ACCESS_ONCE(),
:::::: The code at line 491 was first introduced by commit
:::::: 9a8ab1c39970a4938a72d94e6fd13be88a797590 bug.h, compiler.h: introduce compiletime_assert & BUILD_BUG_ON_MSG
:::::: TO: Daniel Santos <daniel.santos@pobox.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 49942 bytes --]
[-- Attachment #3: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Call for Papers - WorldCIST'17 - 5th World Conference on Information Systems and Technologies (Published by Springer)
From: ML @ 2016-09-18 11:35 UTC (permalink / raw)
To: virtualization
[-- Attachment #1: Type: text/plain, Size: 7664 bytes --]
*
** Apologize if you receive multiple copies of this email, or if its content is irrelevant for you.
*
** Please forward for your contacts. Thank you very much!
*
---------
WorldCIST'17 - 5th World Conference on Information Systems and Technologies
Porto santo Isalnd, Madeira, Portugal
11th-13th of April 2017
http://www.worldcist.org/
-------------------------------------------
SCOPE
The WorldCist'17 - 5th World Conference on Information Systems and Technologies, to be held at Porto Santo Island, Madeira, Portugal, 11 - 13 April 2017, is a global forum for researchers and practitioners to present and discuss the most recent innovations, trends, results, experiences and concerns in the several perspectives of Information Systems and Technologies.
We are pleased to invite you to submit your papers to WorldCist'17 (http://www.worldcist.org/). All submissions will be reviewed on the basis of relevance, originality, importance and clarity.
THEMES
Submitted papers should be related with one or more of the main themes proposed for the Conference:
A) Information and Knowledge Management (IKM);
B) Organizational Models and Information Systems (OMIS);
C) Software and Systems Modeling (SSM);
D) Software Systems, Architectures, Applications and Tools (SSAAT);
E) Multimedia Systems and Applications (MSA);
F) Computer Networks, Mobility and Pervasive Systems (CNMPS);
G) Intelligent and Decision Support Systems (IDSS);
H) Big Data Analytics and Applications (BDAA);
I) Human-Computer Interaction (HCI);
J) Ethics, Computers and Security (ECS)
K) Health Informatics (HIS);
L) Information Technologies in Education (ITE);
M) Information Technologies in Radiocommunications (ITR).
TYPES of SUBMISSIONS AND DECISIONS
Four types of papers can be submitted:
- Full paper: Finished or consolidated R&D works, to be included in one of the Conference themes. These papers are assigned a 10-page limit.
- Short paper: Ongoing works with relevant preliminary results, open to discussion. These papers are assigned a 7-page limit.
- Poster paper: Initial work with relevant ideas, open to discussion. These papers are assigned to a 4-page limit.
- Company paper: Companies' papers that show practical experience, R & D, tools, etc., focused on some topics of the conference. These papers are assigned to a 4-page limit.
Submitted papers must comply with the format of Advances in Intelligent Systems and Computing Series (see Instructions for Authors at Springer Website or download a DOC example) be written in English, must not have been published before, not be under review for any other conference or publication and not include any information leading to the authors’ identification. Therefore, the authors’ names, affiliations and bibliographic references should not be included in the version for evaluation by the Program Committee. This information should only be included in the camera-ready version, saved in Word or Latex format and also in PDF format. These files must be accompanied by the Consent to Publication form filled out, in a ZIP file, and uploaded at the conference management system.
All papers will be subjected to a “double-blind review” by at least two members of the Program Committee.
Based on Program Committee evaluation, a paper can be rejected or accepted by the Conference Chairs. In the later case, it can be accepted as the type originally submitted or as another type. Thus, full papers can be accepted as short papers or poster papers only. Similarly, short papers can be accepted as poster papers only. In these cases, the authors will be allowed to maintain the original number of pages in the camera-ready version.
The authors of accepted poster papers must also build and print a poster to be exhibited during the Conference. This poster must follow an A1 or A2 vertical format. The Conference can includes Work Sessions where these posters are presented and orally discussed, with a 5 minute limit per poster.
The authors of accepted full papers will have 15 minutes to present their work in a Conference Work Session; approximately 5 minutes of discussion will follow each presentation. The authors of accepted short papers and company papers will have 11 minutes to present their work in a Conference Work Session; approximately 4 minutes of discussion will follow each presentation.
PUBLICATION & INDEXING
To ensure that a full paper, short paper, poster paper or company paper is published in the Proceedings, at least one of the authors must be fully registered by the 8th of January 2017, and the paper must comply with the suggested layout and page-limit. Additionally, all recommended changes must be addressed by the authors before they submit the camera-ready version.
No more than one paper per registration will be published in the Conference Proceedings. An extra fee must be paid for publication of additional papers, with a maximum of one additional paper per registration. One registration permits only the participation of one author in the conference.
Full and short papers will be published in Proceedings by Springer, in Advances in Intelligent Systems and Computing Series. Poster and company papers will be published by AISTI.
Published full and short papers will be submitted for indexation by ISI, EI-Compendex, SCOPUS, DBLP and Google Scholar, among others, and will be available in the SpringerLink Digital Library.
The authors of the best selected papers will be invited to extend them for publication in international journals indexed by ISI/SCI, SCOPUS and DBLP, among others, such as:
- International Journal of Neural Systems (IF: 6.085 / Q1)
- Integrated Computer-Aided Engineering (IF: 4.981 / Q1)
- International Journal of Information Management (IF: 2.692 / Q1)
- Electronic Commerce Research and Applications (IF: 2.139 / Q1)
- Computers, Environment and Urban Systems (IF: 2.092 / Q1)
- Data Mining and Knowledge Discovery (IF: 1.759 / Q1)
- Journal of Medical Systems (IF: 2.213 / Q2)
- Journal of Business Research (IF: 2.129 / Q2)
- Pervasive and Mobile Computing (IF: 1.719 / Q2)
- Knowledge and Information Systems (IF: 1.702 / Q2)
- Journal of Grid Computing (IF: 1.561 / Q2) - Special Issue on "Big Data"
- Cluster Computing (IF:1.514 / Q2) - Special Issue on "Advanced Machine Learning in Parallel and Distributed Knowledge Discovery"
- International Journal of Critical Infrastructure Protection (IF: 1.351 / Q2)
- Expert Systems - Journal of Knowledge Engineering (IF: 0.947 / Q3)
- Concurrency and Computation: Practice and Experience (IF: 0.942 / Q3)
- Ethics and Information Technology (IF: 0.739 / Q3)
- Engineering Computations (IF: 0.691 / Q3)
- Advances in Complex Systems (IF: 0.461 / Q3)
- Computing and Informatics (IF: 0.504 / Q4)
- AI Communications (IF: 0.364 / Q4)
- Journal of Hospitality and Tourism Technology (SR: 0.672 / Q2)
- Transforming Government: People, Process and Policy (SR: 0.642 / Q2)
- TEM Journal - Technology, Education, Management, Informatics (ISI - Emerging Sources Citation Index)
- Computer Methods in Biomechanics and Biomedical Engineering - Imaging & Visualization (ISI - Emerging Sources Citation Index)
- Journal of Information Systems Engineering & Management
IMPORTANT DATES
Paper Submission: November 13, 2016
Notification of Acceptance: December 25, 20156
Payment of Registration, to ensure the inclusion of an accepted paper in the conference proceedings: January 8, 2017.
Camera-ready Submission: January 8, 2017
-
WorldCIST'17
http://www.worldcist.org/
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 2/3] qemu: Implement virtio-pstore device
From: Namhyung Kim @ 2016-09-16 10:05 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: virtio-dev, Tony Luck, Daniel P . Berrange, Kees Cook, kvm,
Radim Krčmář, Anton Vorontsov, LKML,
Steven Rostedt, qemu-devel, Minchan Kim, Anthony Liguori,
Colin Cross, Paolo Bonzini, virtualization, Ingo Molnar
In-Reply-To: <20160913155710.3q6yj6yqdyozemul@redhat.com>
On Tue, Sep 13, 2016 at 06:57:10PM +0300, Michael S. Tsirkin wrote:
> On Sat, Aug 20, 2016 at 05:07:43PM +0900, Namhyung Kim wrote:
> > Add virtio pstore device to allow kernel log files saved on the host.
> > It will save the log files on the directory given by pstore device
> > option.
> >
> > $ qemu-system-x86_64 -device virtio-pstore,directory=dir-xx ...
> >
> > (guest) # echo c > /proc/sysrq-trigger
> >
> > $ ls dir-xx
> > dmesg-1.enc.z dmesg-2.enc.z
> >
> > The log files are usually compressed using zlib. Users can see the log
> > messages directly on the host or on the guest (using pstore filesystem).
> >
> > The 'directory' property is required for virtio-pstore device to work.
> > It also adds 'bufsize' property to set size of pstore bufer.
> >
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Radim Krčmář <rkrcmar@redhat.com>
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: Anthony Liguori <aliguori@amazon.com>
> > Cc: Anton Vorontsov <anton@enomsg.org>
> > Cc: Colin Cross <ccross@android.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Tony Luck <tony.luck@intel.com>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Minchan Kim <minchan@kernel.org>
> > Cc: Daniel P. Berrange <berrange@redhat.com>
> > Cc: kvm@vger.kernel.org
> > Cc: qemu-devel@nongnu.org
> > Cc: virtualization@lists.linux-foundation.org
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > hw/virtio/Makefile.objs | 2 +-
> > hw/virtio/virtio-pci.c | 52 ++
> > hw/virtio/virtio-pci.h | 14 +
> > hw/virtio/virtio-pstore.c | 699 +++++++++++++++++++++++++
> > include/hw/pci/pci.h | 1 +
> > include/hw/virtio/virtio-pstore.h | 36 ++
> > include/standard-headers/linux/virtio_ids.h | 1 +
> > include/standard-headers/linux/virtio_pstore.h | 76 +++
> > qdev-monitor.c | 1 +
> > 9 files changed, 881 insertions(+), 1 deletion(-)
> > create mode 100644 hw/virtio/virtio-pstore.c
> > create mode 100644 include/hw/virtio/virtio-pstore.h
> > create mode 100644 include/standard-headers/linux/virtio_pstore.h
> >
> > diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
> > index 3e2b175..aae7082 100644
> > --- a/hw/virtio/Makefile.objs
> > +++ b/hw/virtio/Makefile.objs
> > @@ -4,4 +4,4 @@ common-obj-y += virtio-bus.o
> > common-obj-y += virtio-mmio.o
> >
> > obj-y += virtio.o virtio-balloon.o
> > -obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o
> > +obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o virtio-pstore.o
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index 755f921..c184823 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -2416,6 +2416,57 @@ static const TypeInfo virtio_host_pci_info = {
> > };
> > #endif
> >
> > +/* virtio-pstore-pci */
> > +
> > +static void virtio_pstore_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
> > +{
> > + VirtIOPstorePCI *vps = VIRTIO_PSTORE_PCI(vpci_dev);
> > + DeviceState *vdev = DEVICE(&vps->vdev);
> > + Error *err = NULL;
> > +
> > + qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
> > + object_property_set_bool(OBJECT(vdev), true, "realized", &err);
> > + if (err) {
> > + error_propagate(errp, err);
> > + return;
> > + }
> > +}
> > +
> > +static void virtio_pstore_pci_class_init(ObjectClass *klass, void *data)
> > +{
> > + DeviceClass *dc = DEVICE_CLASS(klass);
> > + VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
> > + PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
> > +
> > + k->realize = virtio_pstore_pci_realize;
> > + set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> > +
> > + pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
> > + pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_PSTORE;
> > + pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
> > + pcidev_k->class_id = PCI_CLASS_OTHERS;
> > +}
> > +
> > +static void virtio_pstore_pci_instance_init(Object *obj)
> > +{
> > + VirtIOPstorePCI *dev = VIRTIO_PSTORE_PCI(obj);
> > +
> > + virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
> > + TYPE_VIRTIO_PSTORE);
> > + object_property_add_alias(obj, "directory", OBJECT(&dev->vdev),
> > + "directory", &error_abort);
> > + object_property_add_alias(obj, "bufsize", OBJECT(&dev->vdev),
> > + "bufsize", &error_abort);
> > +}
> > +
> > +static const TypeInfo virtio_pstore_pci_info = {
> > + .name = TYPE_VIRTIO_PSTORE_PCI,
> > + .parent = TYPE_VIRTIO_PCI,
> > + .instance_size = sizeof(VirtIOPstorePCI),
> > + .instance_init = virtio_pstore_pci_instance_init,
> > + .class_init = virtio_pstore_pci_class_init,
> > +};
> > +
> > /* virtio-pci-bus */
> >
> > static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size,
> > @@ -2485,6 +2536,7 @@ static void virtio_pci_register_types(void)
> > #ifdef CONFIG_VHOST_SCSI
> > type_register_static(&vhost_scsi_pci_info);
> > #endif
> > + type_register_static(&virtio_pstore_pci_info);
> > }
> >
> > type_init(virtio_pci_register_types)
> > diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
> > index 25fbf8a..354b2b7 100644
> > --- a/hw/virtio/virtio-pci.h
> > +++ b/hw/virtio/virtio-pci.h
> > @@ -31,6 +31,7 @@
> > #ifdef CONFIG_VHOST_SCSI
> > #include "hw/virtio/vhost-scsi.h"
> > #endif
> > +#include "hw/virtio/virtio-pstore.h"
> >
> > typedef struct VirtIOPCIProxy VirtIOPCIProxy;
> > typedef struct VirtIOBlkPCI VirtIOBlkPCI;
> > @@ -44,6 +45,7 @@ typedef struct VirtIOInputPCI VirtIOInputPCI;
> > typedef struct VirtIOInputHIDPCI VirtIOInputHIDPCI;
> > typedef struct VirtIOInputHostPCI VirtIOInputHostPCI;
> > typedef struct VirtIOGPUPCI VirtIOGPUPCI;
> > +typedef struct VirtIOPstorePCI VirtIOPstorePCI;
> >
> > /* virtio-pci-bus */
> >
> > @@ -324,6 +326,18 @@ struct VirtIOGPUPCI {
> > VirtIOGPU vdev;
> > };
> >
> > +/*
> > + * virtio-pstore-pci: This extends VirtioPCIProxy.
> > + */
> > +#define TYPE_VIRTIO_PSTORE_PCI "virtio-pstore-pci"
> > +#define VIRTIO_PSTORE_PCI(obj) \
> > + OBJECT_CHECK(VirtIOPstorePCI, (obj), TYPE_VIRTIO_PSTORE_PCI)
> > +
> > +struct VirtIOPstorePCI {
> > + VirtIOPCIProxy parent_obj;
> > + VirtIOPstore vdev;
> > +};
> > +
> > /* Virtio ABI version, if we increment this, we break the guest driver. */
> > #define VIRTIO_PCI_ABI_VERSION 0
> >
> > diff --git a/hw/virtio/virtio-pstore.c b/hw/virtio/virtio-pstore.c
> > new file mode 100644
> > index 0000000..b8fb4be
> > --- /dev/null
> > +++ b/hw/virtio/virtio-pstore.c
> > @@ -0,0 +1,699 @@
> > +/*
> > + * Virtio Pstore Device
> > + *
> > + * Copyright (C) 2016 LG Electronics
> > + *
> > + * Authors:
> > + * Namhyung Kim <namhyung@gmail.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#include <stdio.h>
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/iov.h"
> > +#include "qemu-common.h"
> > +#include "qemu/cutils.h"
> > +#include "qemu/error-report.h"
> > +#include "sysemu/kvm.h"
> > +#include "qapi/visitor.h"
> > +#include "qapi-event.h"
> > +#include "io/channel-util.h"
> > +#include "trace.h"
> > +
> > +#include "hw/virtio/virtio.h"
> > +#include "hw/virtio/virtio-bus.h"
> > +#include "hw/virtio/virtio-access.h"
> > +#include "hw/virtio/virtio-pstore.h"
> > +
> > +#define PSTORE_DEFAULT_BUFSIZE (16 * 1024)
> > +#define PSTORE_DEFAULT_FILE_MAX 5
> > +
> > +/* the index should match to the type value */
> > +static const char *virtio_pstore_file_prefix[] = {
> > + "unknown-", /* VIRTIO_PSTORE_TYPE_UNKNOWN */
>
> Is there value in treating everything unexpected as "unknown"
> and rotating them as if they were logs?
> It might be better to treat everything that's not known
> as guest error.
I was thinking about the version mismatch between the kernel and qemu.
I'd like to make the device can deal with a new kernel version which
might implement a new pstore message type. It will be saved as
unknown but the kernel can read it properly later.
>
>
> > + "dmesg-", /* VIRTIO_PSTORE_TYPE_DMESG */
>
> use named initializers for this instead of comments.
Ok.
>
> > +};
> > +
> > +static char *virtio_pstore_to_filename(VirtIOPstore *s,
> > + struct virtio_pstore_req *req)
> > +{
> > + const char *basename;
> > + unsigned long long id;
> > + unsigned int type = le16_to_cpu(req->type);
> > + unsigned int flags = le32_to_cpu(req->flags);
> > +
> > + if (type < ARRAY_SIZE(virtio_pstore_file_prefix)) {
> > + basename = virtio_pstore_file_prefix[type];
> > + } else {
> > + basename = "unknown-";
> > + }
> > +
> > + id = s->id++;
> > + return g_strdup_printf("%s/%s%llu%s", s->directory, basename, id,
> > + flags & VIRTIO_PSTORE_FL_COMPRESSED ? ".enc.z" : "");
> > +}
> > +
> > +static char *virtio_pstore_from_filename(VirtIOPstore *s, char *name,
> > + struct virtio_pstore_fileinfo *info)
> > +{
> > + char *filename;
> > + unsigned int idx;
> > +
> > + filename = g_strdup_printf("%s/%s", s->directory, name);
> > + if (filename == NULL)
> > + return NULL;
> > +
> > + for (idx = 0; idx < ARRAY_SIZE(virtio_pstore_file_prefix); idx++) {
> > + if (g_str_has_prefix(name, virtio_pstore_file_prefix[idx])) {
> > + info->type = idx;
> > + name += strlen(virtio_pstore_file_prefix[idx]);
> > + break;
> > + }
> > + }
> > +
> > + if (idx == ARRAY_SIZE(virtio_pstore_file_prefix)) {
> > + g_free(filename);
> > + return NULL;
> > + }
> > +
> > + qemu_strtoull(name, NULL, 0, &info->id);
>
> What if this fails?
Hmm.. will add a check for return value then.
>
> > +
> > + info->flags = 0;
> > + if (g_str_has_suffix(name, ".enc.z")) {
> > + info->flags |= VIRTIO_PSTORE_FL_COMPRESSED;
> > + }
> > +
> > + return filename;
> > +}
> > +
> > +static int prefix_idx;
> > +static int prefix_count;
> > +static int prefix_len;
> This does not work properly if there are multiple instances
> of it. Pls move everything into device state.
Kernel (currently?) allows only a single pstore device active. But I
think it'd be better to move them into device state anyway.
>
> > +
> > +static int filter_pstore(const struct dirent *de)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < prefix_count; i++) {
> > + const char *prefix = virtio_pstore_file_prefix[prefix_idx + i];
> > +
> > + if (g_str_has_prefix(de->d_name, prefix)) {
> > + return 1;
> > + }
> > + }
> > + return 0;
> > +}
> > +
> > +static int sort_pstore(const struct dirent **a, const struct dirent **b)
> > +{
> > + uint64_t id_a, id_b;
> > +
> > + qemu_strtoull((*a)->d_name + prefix_len, NULL, 0, &id_a);
> > + qemu_strtoull((*b)->d_name + prefix_len, NULL, 0, &id_b);
> > +
> > + return id_a - id_b;
> > +}
> > +
> > +static int rotate_pstore_file(VirtIOPstore *s, unsigned short type)
> > +{
> > + int ret = 0;
> > + int i, num;
> > + char *filename;
> > + struct dirent **files;
> > +
> > + if (type >= ARRAY_SIZE(virtio_pstore_file_prefix)) {
> > + type = VIRTIO_PSTORE_TYPE_UNKNOWN;
> > + }
> > +
> > + prefix_idx = type;
> > + prefix_len = strlen(virtio_pstore_file_prefix[type]);
> > + prefix_count = 1; /* only scan current type */
> > +
> > + /* delete the oldest file in the same type */
> > + num = scandir(s->directory, &files, filter_pstore, sort_pstore);
> > + if (num < 0)
> > + return num;
> > + if (num < (int)s->file_max)
> > + goto out;
> > +
> > + filename = g_strdup_printf("%s/%s", s->directory, files[0]->d_name);
> > + if (filename == NULL) {
> > + ret = -1;
> > + goto out;
> > + }
> > +
> > + ret = unlink(filename);
> > +
> > +out:
> > + for (i = 0; i < num; i++) {
> > + g_free(files[i]);
> > + }
> > + g_free(files);
> > +
> > + return ret;
> > +}
>
> Pls prefix everything with virtio_pstore or another
> unique prefix. also below.
Ok.
>
> > +
> > +static ssize_t virtio_pstore_do_open(VirtIOPstore *s)
> > +{
> > + /* scan all pstore files */
> > + prefix_idx = 0;
> > + prefix_count = ARRAY_SIZE(virtio_pstore_file_prefix);
> > +
> > + s->file_idx = 0;
> > + s->num_file = scandir(s->directory, &s->files, filter_pstore, alphasort);
> > +
> > + return s->num_file >= 0 ? 0 : -1;
> > +}
> > +
> > +static ssize_t virtio_pstore_do_close(VirtIOPstore *s)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < s->num_file; i++) {
> > + g_free(s->files[i]);
> > + }
> > + g_free(s->files);
> > + s->files = NULL;
> > +
> > + s->num_file = 0;
> > + return 0;
> > +}
> > +
> > +static ssize_t virtio_pstore_do_erase(VirtIOPstore *s,
> > + struct virtio_pstore_req *req)
> > +{
> > + char *filename;
> > + int ret;
> > +
> > + filename = virtio_pstore_to_filename(s, req);
> > + if (filename == NULL)
> > + return -1;
>
> this can't happen.
Why? The virtio_pstore_to_filename() calls g_strdup_printf(). That
means I don't need to worry about the memory allocation failure?
>
> also this is a coding style violation.
Oh, I missed the add {}, will fix.
>
> > +
> > + ret = unlink(filename);
> > +
> > + g_free(filename);
> > + return ret;
> > +}
> > +
> > +struct pstore_read_arg {
> > + VirtIOPstore *vps;
> > + VirtQueueElement *elem;
> > + struct virtio_pstore_fileinfo info;
> > + QIOChannel *ioc;
> > +};
> > +
> > +static gboolean pstore_async_read_fn(QIOChannel *ioc, GIOCondition condition,
> > + gpointer data)
> > +{
> > + struct pstore_read_arg *rarg = data;
> > + struct virtio_pstore_fileinfo *info = &rarg->info;
> > + VirtIOPstore *vps = rarg->vps;
> > + VirtQueueElement *elem = rarg->elem;
> > + struct virtio_pstore_res res;
> > + size_t offset = sizeof(res) + sizeof(*info);
> > + struct iovec *sg = elem->in_sg;
> > + unsigned int sg_num = elem->in_num;
> > + Error *err = NULL;
> > + ssize_t len;
> > + int ret;
> > +
> > + /* skip res and fileinfo */
> > + iov_discard_front(&sg, &sg_num, sizeof(res) + sizeof(*info));
> > +
> > + len = qio_channel_readv(rarg->ioc, sg, sg_num, &err);
> > + if (len < 0) {
> > + if (errno == EAGAIN) {
> > + len = 0;
> > + }
> > + ret = -1;
> > + } else {
> > + info->len = cpu_to_le32(len);
> > + ret = 0;
> > + }
> > +
> > + res.cmd = cpu_to_le16(VIRTIO_PSTORE_CMD_READ);
> > + res.type = cpu_to_le16(VIRTIO_PSTORE_TYPE_UNKNOWN);
> > + res.ret = cpu_to_le32(ret);
> > +
> > + /* now copy res and fileinfo */
> > + iov_from_buf(elem->in_sg, elem->in_num, 0, &res, sizeof(res));
> > + iov_from_buf(elem->in_sg, elem->in_num, sizeof(res), info, sizeof(*info));
> > +
> > + len += offset;
> > + virtqueue_push(vps->rvq, elem, len);
> > + virtio_notify(VIRTIO_DEVICE(vps), vps->rvq);
> > +
> > + return G_SOURCE_REMOVE;
> > +}
> > +
> > +static void free_rarg_fn(gpointer data)
> > +{
> > + struct pstore_read_arg *rarg = data;
> > +
> > + qio_channel_close(rarg->ioc, NULL);
> > +
> > + g_free(rarg->elem);
> > + g_free(rarg);
> > +}
> > +
> > +static ssize_t virtio_pstore_do_read(VirtIOPstore *s, VirtQueueElement *elem)
> > +{
> > + char *filename = NULL;
> > + int fd, idx;
> > + struct stat stbuf;
> > + struct pstore_read_arg *rarg = NULL;
> > + Error *err = NULL;
> > + int ret = -1;
> > +
> > + if (s->file_idx >= s->num_file) {
> > + return 0;
> > + }
> > +
> > + rarg = g_malloc(sizeof(*rarg));
> > + if (rarg == NULL) {
> > + return -1;
> > + }
> > +
> > + idx = s->file_idx++;
> > + filename = virtio_pstore_from_filename(s, s->files[idx]->d_name,
> > + &rarg->info);
> > + if (filename == NULL) {
> > + goto out;
> > + }
> > +
> > + fd = open(filename, O_RDONLY);
> > + if (fd < 0) {
> > + error_report("cannot open %s", filename);
> > + goto out;
> > + }
>
> I see open here but close nowhere. Does this leak fds?
I guess so. But this is changed to use qio_channel_file API in v5 and
I hope doing it right.
>
> > +
> > + if (fstat(fd, &stbuf) < 0) {
>
> So we can stat, but can we e.g. read?
It's just being a paranoid, I think it should succeed, no?
>
>
> > + goto out;
> > + }
> > +
> > + rarg->vps = s;
> > + rarg->elem = elem;
> > + rarg->info.id = cpu_to_le64(rarg->info.id);
> > + rarg->info.type = cpu_to_le16(rarg->info.type);
> > + rarg->info.flags = cpu_to_le32(rarg->info.flags);
> > + rarg->info.time_sec = cpu_to_le64(stbuf.st_ctim.tv_sec);
>
> Is this seconds since epoch?
> Why ctim specifically?
> Pls add comments.
I think it doesn't matter either ctim or mtim.
>
> > + rarg->info.time_nsec = cpu_to_le32(stbuf.st_ctim.tv_nsec);
>
> Not all hosts support nanosecond precision.
> Do we need some way to tell guest what's reliable?
In fact I'm not sure how much it affects users. The pstore messages
are occasional and AFAIK pstore keeps it only for users' information.
>
> Unless you limit this to linux host, you should care about things
> like this (in man fstat)
>
> Since kernel 2.5.48, the stat structure supports nanosecond
> resolution for the three file timestamp fields. The nanosecond compo‐
> nents of each timestamp are available via names of the form
> st_atim.tv_nsec if the _BSD_SOURCE or _SVID_SOURCE feature test macro
> is defined. Nanosecond timestamps are nowadays standardized,
> starting with POSIX.1-2008, and, starting with version 2.12, glibc also
> exposes the nanosecond component names if _POSIX_C_SOURCE is defined
> with the value 200809L or greater, or _XOPEN_SOURCE is defined with
> the value 700 or greater. If none of the aforementioned macros are
> defined, then the nanosecond values are exposed with names of the form
> st_atimensec.
Thanks for the info.
>
>
>
>
> > +
> > + rarg->ioc = qio_channel_new_fd(fd, &err);
> > + if (err) {
> > + error_reportf_err(err, "cannot create io channel: ");
> > + goto out;
> > + }
> > +
> > + qio_channel_set_blocking(rarg->ioc, false, &err);
> > + qio_channel_add_watch(rarg->ioc, G_IO_IN, pstore_async_read_fn, rarg,
> > + free_rarg_fn);
> > + g_free(filename);
> > + return 1;
> > +
> > +out:
> > + g_free(filename);
> > + g_free(rarg);
> > +
> > + return ret;
> > +}
> > +
> > +struct pstore_write_arg {
> > + VirtIOPstore *vps;
> > + VirtQueueElement *elem;
> > + struct virtio_pstore_req *req;
> > + QIOChannel *ioc;
> > +};
> > +
> > +static gboolean pstore_async_write_fn(QIOChannel *ioc, GIOCondition condition,
> > + gpointer data)
> > +{
> > + struct pstore_write_arg *warg = data;
> > + VirtIOPstore *vps = warg->vps;
> > + VirtQueueElement *elem = warg->elem;
> > + struct iovec *sg = elem->out_sg;
> > + unsigned int sg_num = elem->out_num;
> > + struct virtio_pstore_res res;
> > + Error *err = NULL;
> > + ssize_t len;
> > + int ret;
> > +
> > + /* we already consumed the req */
> > + iov_discard_front(&sg, &sg_num, sizeof(*warg->req));
> > +
> > + len = qio_channel_writev(warg->ioc, sg, sg_num, &err);
> > + if (len < 0) {
> > + ret = -1;
> > + } else {
> > + ret = 0;
> > + }
>
> This can discard part of the data written.
> Don't we care?
Doing partial write is better than failing out. But if it's
meaningful to add a retry loop, I'd like to do so.
>
> > +
> > + res.cmd = cpu_to_le16(VIRTIO_PSTORE_CMD_WRITE);
> > + res.type = warg->req->type;
> > + res.ret = cpu_to_le32(ret);
> > +
> > + /* tell the result to guest */
> > + iov_from_buf(elem->in_sg, elem->in_num, 0, &res, sizeof(res));
> > +
> > + virtqueue_push(vps->wvq, elem, sizeof(res));
> > + virtio_notify(VIRTIO_DEVICE(vps), vps->wvq);
> > +
> > + return G_SOURCE_REMOVE;
> > +}
> > +
> > +static void free_warg_fn(gpointer data)
> > +{
> > + struct pstore_write_arg *warg = data;
> > +
> > + qio_channel_close(warg->ioc, NULL);
> > +
> > + g_free(warg->elem);
> > + g_free(warg);
> > +}
> > +
> > +static ssize_t virtio_pstore_do_write(VirtIOPstore *s, VirtQueueElement *elem,
> > + struct virtio_pstore_req *req)
> > +{
> > + unsigned short type = le16_to_cpu(req->type);
> > + char *filename = NULL;
> > + int fd;
> > + int flags = O_WRONLY | O_CREAT | O_TRUNC;
> > + struct pstore_write_arg *warg = NULL;
> > + Error *err = NULL;
> > + int ret = -1;
> > +
> > + /* do not keep same type of files more than 'file-max' */
> > + rotate_pstore_file(s, type);
>
> If you don't care about failures, should this function
> return a value? How about reporting it to the user?
Did you mean when it failed to delete the oldest file (FYI it's not
really 'rotate'). Hmm.. will add error check and report.
>
>
> > +
> > + filename = virtio_pstore_to_filename(s, req);
> > + if (filename == NULL) {
> > + return -1;
> > + }
>
> this can't happen
>
> > +
> > + warg = g_malloc(sizeof(*warg));
> > + if (warg == NULL) {
> > + goto out;
> > + }
> > +
> > + fd = open(filename, flags, 0644);
> > + if (fd < 0) {
> > + error_report("cannot open %s", filename);
> > + ret = fd;
> > + goto out;
> > + }
> > +
> > + warg->vps = s;
> > + warg->elem = elem;
> > + warg->req = req;
> > +
> > + warg->ioc = qio_channel_new_fd(fd, &err);
> > + if (err) {
> > + error_reportf_err(err, "cannot create io channel: ");
> > + goto out;
> > + }
> > +
> > + qio_channel_set_blocking(warg->ioc, false, &err);
> > + qio_channel_add_watch(warg->ioc, G_IO_OUT, pstore_async_write_fn, warg,
> > + free_warg_fn);
> > + g_free(filename);
> > + return 1;
> > +
> > +out:
> > + g_free(filename);
> > + g_free(warg);
> > + return ret;
> > +}
> > +
> > +static void virtio_pstore_handle_io(VirtIODevice *vdev, VirtQueue *vq)
> > +{
> > + VirtIOPstore *s = VIRTIO_PSTORE(vdev);
> > + VirtQueueElement *elem;
> > + struct virtio_pstore_req req;
> > + struct virtio_pstore_res res;
> > + ssize_t len = 0;
> > + int ret;
> > +
> > + for (;;) {
> > + elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> > + if (!elem) {
> > + return;
> > + }
> > +
> > + if (elem->out_num < 1 || elem->in_num < 1) {
> > + error_report("request or response buffer is missing");
> > + exit(1);
> > + }
> > +
> > + if (elem->out_num > 2 || elem->in_num > 3) {
> > + error_report("invalid number of input/output buffer");
> > + exit(1);
> > + }
> > +
> > + len = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
> > + if (len != (ssize_t)sizeof(req)) {
> > + error_report("invalid request size: %ld", (long)len);
> > + exit(1);
> > + }
> > + res.cmd = req.cmd;
> > + res.type = req.type;
> > +
> > + switch (le16_to_cpu(req.cmd)) {
> > + case VIRTIO_PSTORE_CMD_OPEN:
> > + ret = virtio_pstore_do_open(s);
> > + break;
> > + case VIRTIO_PSTORE_CMD_CLOSE:
> > + ret = virtio_pstore_do_close(s);
> > + break;
> > + case VIRTIO_PSTORE_CMD_ERASE:
> > + ret = virtio_pstore_do_erase(s, &req);
> > + break;
> > + case VIRTIO_PSTORE_CMD_READ:
> > + ret = virtio_pstore_do_read(s, elem);
> > + if (ret == 1) {
> > + /* async channel io */
> > + continue;
> > + }
> > + break;
> > + case VIRTIO_PSTORE_CMD_WRITE:
> > + ret = virtio_pstore_do_write(s, elem, &req);
> > + if (ret == 1) {
> > + /* async channel io */
> > + continue;
> > + }
> > + break;
> > + default:
> > + ret = -1;
> > + break;
> > + }
> > +
> > + res.ret = ret;
> > +
> > + iov_from_buf(elem->in_sg, elem->in_num, 0, &res, sizeof(res));
> > + virtqueue_push(vq, elem, sizeof(res) + len);
> > +
> > + virtio_notify(vdev, vq);
> > + g_free(elem);
> > +
> > + if (ret < 0) {
> > + return;
>
> what does this do?
If it failed on any processing, reports it to the kernel and stop
processing later commands. The kernel won't send same kind of command
later.
>
> > + }
> > + }
> > +}
> > +
> > +static void virtio_pstore_device_realize(DeviceState *dev, Error **errp)
> > +{
> > + VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> > + VirtIOPstore *s = VIRTIO_PSTORE(dev);
> > +
> > + virtio_init(vdev, "virtio-pstore", VIRTIO_ID_PSTORE,
> > + sizeof(struct virtio_pstore_config));
> > +
> > + s->id = 1;
> > +
> > + if (!s->bufsize)
> > + s->bufsize = PSTORE_DEFAULT_BUFSIZE;
> > + if (!s->file_max)
> > + s->file_max = PSTORE_DEFAULT_FILE_MAX;
> > +
> > + s->rvq = virtio_add_queue(vdev, 128, virtio_pstore_handle_io);
> > + s->wvq = virtio_add_queue(vdev, 128, virtio_pstore_handle_io);
> > +}
> > +
> > +static void virtio_pstore_device_unrealize(DeviceState *dev, Error **errp)
> > +{
> > + VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> > +
> > + virtio_cleanup(vdev);
> > +}
> > +
> > +static void virtio_pstore_get_config(VirtIODevice *vdev, uint8_t *config_data)
> > +{
> > + VirtIOPstore *dev = VIRTIO_PSTORE(vdev);
> > + struct virtio_pstore_config config;
>
> Add {} here - you want all fields initialized
> if you add them, to avoid leaking them to guest.
Ok.
>
> > +
> > + config.bufsize = cpu_to_le32(dev->bufsize);
> > +
> > + memcpy(config_data, &config, sizeof(struct virtio_pstore_config));
> > +}
> > +
> > +static void virtio_pstore_set_config(VirtIODevice *vdev,
> > + const uint8_t *config_data)
> > +{
> > + VirtIOPstore *dev = VIRTIO_PSTORE(vdev);
> > + struct virtio_pstore_config config;
> > +
> > + memcpy(&config, config_data, sizeof(struct virtio_pstore_config));
> > +
> > + dev->bufsize = le32_to_cpu(config.bufsize);
> > +}
> > +
> > +static uint64_t get_features(VirtIODevice *vdev, uint64_t f, Error **errp)
> > +{
> > + return f;
> > +}
> > +
> > +static void pstore_get_directory(Object *obj, Visitor *v,
> > + const char *name, void *opaque,
> > + Error **errp)
> > +{
> > + VirtIOPstore *s = opaque;
> > +
> > + visit_type_str(v, name, &s->directory, errp);
> > +}
> > +
> > +static void pstore_set_directory(Object *obj, Visitor *v,
> > + const char *name, void *opaque,
> > + Error **errp)
> > +{
> > + VirtIOPstore *s = opaque;
> > + Error *local_err = NULL;
> > + char *value;
> > +
> > + visit_type_str(v, name, &value, &local_err);
> > + if (local_err) {
> > + error_propagate(errp, local_err);
> > + return;
> > + }
> > +
> > + g_free(s->directory);
> > + s->directory = value;
> > +}
> > +
> > +static void pstore_release_directory(Object *obj, const char *name,
> > + void *opaque)
> > +{
> > + VirtIOPstore *s = opaque;
> > +
> > + g_free(s->directory);
> > + s->directory = NULL;
> > +}
> > +
> > +static void pstore_get_bufsize(Object *obj, Visitor *v,
> > + const char *name, void *opaque,
> > + Error **errp)
> > +{
> > + VirtIOPstore *s = opaque;
> > + uint64_t value = s->bufsize;
> > +
> > + visit_type_size(v, name, &value, errp);
> > +}
> > +
> > +static void pstore_set_bufsize(Object *obj, Visitor *v,
> > + const char *name, void *opaque,
> > + Error **errp)
> > +{
> > + VirtIOPstore *s = opaque;
> > + Error *error = NULL;
> > + uint64_t value;
> > +
> > + visit_type_size(v, name, &value, &error);
> > + if (error) {
> > + error_propagate(errp, error);
> > + return;
> > + }
> > +
> > + if (value < 4096) {
> > + error_setg(&error, "Warning: too small buffer size: %"PRIu64, value);
> > + error_propagate(errp, error);
> > + return;
> > + }
> > +
> > + s->bufsize = value;
> > +}
> > +
> > +static void pstore_get_file_max(Object *obj, Visitor *v,
> > + const char *name, void *opaque,
> > + Error **errp)
> > +{
> > + VirtIOPstore *s = opaque;
> > + int64_t value = s->file_max;
> > +
> > + visit_type_int(v, name, &value, errp);
> > +}
> > +
> > +static void pstore_set_file_max(Object *obj, Visitor *v,
> > + const char *name, void *opaque,
> > + Error **errp)
> > +{
> > + VirtIOPstore *s = opaque;
> > + Error *error = NULL;
> > + int64_t value;
> > +
> > + visit_type_int(v, name, &value, &error);
> > + if (error) {
> > + error_propagate(errp, error);
> > + return;
> > + }
> > +
> > + s->file_max = value;
> > +}
>
> Do you need dynamic properties? There are easier ways
> to define an int property. Same for others.
It was due to my insufficient knowledge about the qemu code base. I
don't think it needs to be dynamic.
Thanks,
Namhyung
>
> > +
> > +static Property virtio_pstore_properties[] = {
> > + DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> > +static void virtio_pstore_instance_init(Object *obj)
> > +{
> > + VirtIOPstore *s = VIRTIO_PSTORE(obj);
> > +
> > + object_property_add(obj, "directory", "str",
> > + pstore_get_directory, pstore_set_directory,
> > + pstore_release_directory, s, NULL);
> > + object_property_add(obj, "bufsize", "size",
> > + pstore_get_bufsize, pstore_set_bufsize, NULL, s, NULL);
> > + object_property_add(obj, "file-max", "int",
> > + pstore_get_file_max, pstore_set_file_max, NULL, s, NULL);
> > +}
> > +
> > +static void virtio_pstore_class_init(ObjectClass *klass, void *data)
> > +{
> > + DeviceClass *dc = DEVICE_CLASS(klass);
> > + VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> > +
> > + dc->props = virtio_pstore_properties;
> > + set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> > + vdc->realize = virtio_pstore_device_realize;
> > + vdc->unrealize = virtio_pstore_device_unrealize;
> > + vdc->get_config = virtio_pstore_get_config;
> > + vdc->set_config = virtio_pstore_set_config;
> > + vdc->get_features = get_features;
> > +}
> > +
> > +static const TypeInfo virtio_pstore_info = {
> > + .name = TYPE_VIRTIO_PSTORE,
> > + .parent = TYPE_VIRTIO_DEVICE,
> > + .instance_size = sizeof(VirtIOPstore),
> > + .instance_init = virtio_pstore_instance_init,
> > + .class_init = virtio_pstore_class_init,
> > +};
> > +
> > +static void virtio_register_types(void)
> > +{
> > + type_register_static(&virtio_pstore_info);
> > +}
> > +
> > +type_init(virtio_register_types)
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index 929ec2f..b31774a 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -79,6 +79,7 @@
> > #define PCI_DEVICE_ID_VIRTIO_SCSI 0x1004
> > #define PCI_DEVICE_ID_VIRTIO_RNG 0x1005
> > #define PCI_DEVICE_ID_VIRTIO_9P 0x1009
> > +#define PCI_DEVICE_ID_VIRTIO_PSTORE 0x100a
> >
> > #define PCI_VENDOR_ID_REDHAT 0x1b36
> > #define PCI_DEVICE_ID_REDHAT_BRIDGE 0x0001
> > diff --git a/include/hw/virtio/virtio-pstore.h b/include/hw/virtio/virtio-pstore.h
> > new file mode 100644
> > index 0000000..85b1828
> > --- /dev/null
> > +++ b/include/hw/virtio/virtio-pstore.h
> > @@ -0,0 +1,36 @@
> > +/*
> > + * Virtio Pstore Support
> > + *
> > + * Authors:
> > + * Namhyung Kim <namhyung@gmail.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2. See
> > + * the COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#ifndef _QEMU_VIRTIO_PSTORE_H
> > +#define _QEMU_VIRTIO_PSTORE_H
> > +
> > +#include "standard-headers/linux/virtio_pstore.h"
> > +#include "hw/virtio/virtio.h"
> > +#include "hw/pci/pci.h"
> > +
> > +#define TYPE_VIRTIO_PSTORE "virtio-pstore-device"
> > +#define VIRTIO_PSTORE(obj) \
> > + OBJECT_CHECK(VirtIOPstore, (obj), TYPE_VIRTIO_PSTORE)
> > +
> > +typedef struct VirtIOPstore {
> > + VirtIODevice parent_obj;
> > + VirtQueue *rvq;
> > + VirtQueue *wvq;
> > + char *directory;
> > + int file_idx;
> > + int num_file;
> > + struct dirent **files;
> > + uint64_t id;
> > + uint64_t bufsize;
> > + uint64_t file_max;
> > +} VirtIOPstore;
> > +
> > +#endif
> > diff --git a/include/standard-headers/linux/virtio_ids.h b/include/standard-headers/linux/virtio_ids.h
> > index 77925f5..c72a9ab 100644
> > --- a/include/standard-headers/linux/virtio_ids.h
> > +++ b/include/standard-headers/linux/virtio_ids.h
> > @@ -41,5 +41,6 @@
> > #define VIRTIO_ID_CAIF 12 /* Virtio caif */
> > #define VIRTIO_ID_GPU 16 /* virtio GPU */
> > #define VIRTIO_ID_INPUT 18 /* virtio input */
> > +#define VIRTIO_ID_PSTORE 22 /* virtio pstore */
> >
> > #endif /* _LINUX_VIRTIO_IDS_H */
> > diff --git a/include/standard-headers/linux/virtio_pstore.h b/include/standard-headers/linux/virtio_pstore.h
> > new file mode 100644
> > index 0000000..2f91839
> > --- /dev/null
> > +++ b/include/standard-headers/linux/virtio_pstore.h
> > @@ -0,0 +1,76 @@
> > +#ifndef _LINUX_VIRTIO_PSTORE_H
> > +#define _LINUX_VIRTIO_PSTORE_H
> > +/* This header is BSD licensed so anyone can use the definitions to implement
> > + * compatible drivers/servers.
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions
> > + * are met:
> > + * 1. Redistributions of source code must retain the above copyright
> > + * notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above copyright
> > + * notice, this list of conditions and the following disclaimer in the
> > + * documentation and/or other materials provided with the distribution.
> > + * 3. Neither the name of IBM nor the names of its contributors
> > + * may be used to endorse or promote products derived from this software
> > + * without specific prior written permission.
> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS IS'' AND
> > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> > + * ARE DISCLAIMED. IN NO EVENT SHALL IBM OR CONTRIBUTORS BE LIABLE
> > + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> > + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> > + * SUCH DAMAGE. */
> > +#include "standard-headers/linux/types.h"
> > +#include "standard-headers/linux/virtio_types.h"
> > +#include "standard-headers/linux/virtio_ids.h"
> > +#include "standard-headers/linux/virtio_config.h"
> > +
> > +#define VIRTIO_PSTORE_CMD_NULL 0
> > +#define VIRTIO_PSTORE_CMD_OPEN 1
> > +#define VIRTIO_PSTORE_CMD_READ 2
> > +#define VIRTIO_PSTORE_CMD_WRITE 3
> > +#define VIRTIO_PSTORE_CMD_ERASE 4
> > +#define VIRTIO_PSTORE_CMD_CLOSE 5
> > +
> > +#define VIRTIO_PSTORE_TYPE_UNKNOWN 0
> > +#define VIRTIO_PSTORE_TYPE_DMESG 1
> > +
> > +#define VIRTIO_PSTORE_FL_COMPRESSED 1
> > +
> > +struct virtio_pstore_req {
> > + __virtio16 cmd;
> > + __virtio16 type;
> > + __virtio32 flags;
> > + __virtio64 id;
> > + __virtio32 count;
> > + __virtio32 reserved;
> > +};
> > +
> > +struct virtio_pstore_res {
> > + __virtio16 cmd;
> > + __virtio16 type;
> > + __virtio32 ret;
> > +};
> > +
> > +struct virtio_pstore_fileinfo {
> > + __virtio64 id;
> > + __virtio32 count;
> > + __virtio16 type;
> > + __virtio16 unused;
> > + __virtio32 flags;
> > + __virtio32 len;
> > + __virtio64 time_sec;
> > + __virtio32 time_nsec;
> > + __virtio32 reserved;
> > +};
> > +
> > +struct virtio_pstore_config {
> > + __virtio32 bufsize;
> > +};
> > +
> > +#endif /* _LINUX_VIRTIO_PSTORE_H */
> > diff --git a/qdev-monitor.c b/qdev-monitor.c
> > index e19617f..e1df5a9 100644
> > --- a/qdev-monitor.c
> > +++ b/qdev-monitor.c
> > @@ -73,6 +73,7 @@ static const QDevAlias qdev_alias_table[] = {
> > { "virtio-serial-pci", "virtio-serial", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
> > { "virtio-tablet-ccw", "virtio-tablet", QEMU_ARCH_S390X },
> > { "virtio-tablet-pci", "virtio-tablet", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
> > + { "virtio-pstore-pci", "virtio-pstore" },
> > { }
> > };
> >
> > --
> > 2.9.3
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 1/3] virtio: Basic implementation of virtio pstore driver
From: Namhyung Kim @ 2016-09-16 9:05 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: virtio-dev, Tony Luck, Kees Cook, kvm,
Radim Krčmář, Anton Vorontsov, LKML,
Steven Rostedt, qemu-devel, Minchan Kim, Anthony Liguori,
Colin Cross, Paolo Bonzini, virtualization, Ingo Molnar
In-Reply-To: <20160913173924-mutt-send-email-mst@kernel.org>
Hello Michael,
Thanks for your detailed review. Btw are you ok with the overall
direction of the patch?
On Tue, Sep 13, 2016 at 06:19:41PM +0300, Michael S. Tsirkin wrote:
> On Sat, Aug 20, 2016 at 05:07:42PM +0900, Namhyung Kim wrote:
> > The virtio pstore driver provides interface to the pstore subsystem so
> > that the guest kernel's log/dump message can be saved on the host
> > machine. Users can access the log file directly on the host, or on the
> > guest at the next boot using pstore filesystem. It currently deals with
> > kernel log (printk) buffer only, but we can extend it to have other
> > information (like ftrace dump) later.
> >
> > It supports legacy PCI device using single order-2 page buffer. It uses
> > two virtqueues - one for (sync) read and another for (async) write.
> > Since it cannot wait for write finished, it supports up to 128
> > concurrent IO. The buffer size is configurable now.
> >
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Radim Krčmář <rkrcmar@redhat.com>
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: Anthony Liguori <aliguori@amazon.com>
> > Cc: Anton Vorontsov <anton@enomsg.org>
> > Cc: Colin Cross <ccross@android.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Tony Luck <tony.luck@intel.com>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Minchan Kim <minchan@kernel.org>
> > Cc: kvm@vger.kernel.org
> > Cc: qemu-devel@nongnu.org
> > Cc: virtualization@lists.linux-foundation.org
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > drivers/virtio/Kconfig | 10 +
> > drivers/virtio/Makefile | 1 +
> > drivers/virtio/virtio_pstore.c | 417 +++++++++++++++++++++++++++++++++++++
> > include/uapi/linux/Kbuild | 1 +
> > include/uapi/linux/virtio_ids.h | 1 +
> > include/uapi/linux/virtio_pstore.h | 74 +++++++
> > 6 files changed, 504 insertions(+)
> > create mode 100644 drivers/virtio/virtio_pstore.c
> > create mode 100644 include/uapi/linux/virtio_pstore.h
> >
> > diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> > index 77590320d44c..8f0e6c796c12 100644
> > --- a/drivers/virtio/Kconfig
> > +++ b/drivers/virtio/Kconfig
> > @@ -58,6 +58,16 @@ config VIRTIO_INPUT
> >
> > If unsure, say M.
> >
> > +config VIRTIO_PSTORE
> > + tristate "Virtio pstore driver"
> > + depends on VIRTIO
> > + depends on PSTORE
> > + ---help---
> > + This driver supports virtio pstore devices to save/restore
> > + panic and oops messages on the host.
> > +
> > + If unsure, say M.
> > +
> > config VIRTIO_MMIO
> > tristate "Platform bus driver for memory mapped virtio devices"
> > depends on HAS_IOMEM && HAS_DMA
> > diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
> > index 41e30e3dc842..bee68cb26d48 100644
> > --- a/drivers/virtio/Makefile
> > +++ b/drivers/virtio/Makefile
> > @@ -5,3 +5,4 @@ virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
> > virtio_pci-$(CONFIG_VIRTIO_PCI_LEGACY) += virtio_pci_legacy.o
> > obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o
> > obj-$(CONFIG_VIRTIO_INPUT) += virtio_input.o
> > +obj-$(CONFIG_VIRTIO_PSTORE) += virtio_pstore.o
> > diff --git a/drivers/virtio/virtio_pstore.c b/drivers/virtio/virtio_pstore.c
> > new file mode 100644
> > index 000000000000..0a63c7db4278
> > --- /dev/null
> > +++ b/drivers/virtio/virtio_pstore.c
> > @@ -0,0 +1,417 @@
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/pstore.h>
> > +#include <linux/virtio.h>
> > +#include <linux/virtio_config.h>
> > +#include <uapi/linux/virtio_ids.h>
> > +#include <uapi/linux/virtio_pstore.h>
> > +
> > +#define VIRT_PSTORE_ORDER 2
> > +#define VIRT_PSTORE_BUFSIZE (4096 << VIRT_PSTORE_ORDER)
> > +#define VIRT_PSTORE_NR_REQ 128
>
> where are these numbers from?
The buffer size was chosen to be larger than default kmsg_bytes
(10240) in the pstore platform code and the request count is a
arbitrary value.
>
>
> > +
> > +struct virtio_pstore {
> > + struct virtio_device *vdev;
> > + struct virtqueue *vq[2];
> > + struct pstore_info pstore;
> > + struct virtio_pstore_req req[VIRT_PSTORE_NR_REQ];
> > + struct virtio_pstore_res res[VIRT_PSTORE_NR_REQ];
> > + unsigned int req_id;
> > +
> > + /* Waiting for host to ack */
> > + wait_queue_head_t acked;
> > + int failed;
> > +};
> > +
> > +#define TYPE_TABLE_ENTRY(_entry) \
> > + { PSTORE_TYPE_##_entry, VIRTIO_PSTORE_TYPE_##_entry }
> > +
> > +struct type_table {
> > + int pstore;
> > + u16 virtio;
> > +} type_table[] = {
> > + TYPE_TABLE_ENTRY(DMESG),
> > +};
> > +
> > +#undef TYPE_TABLE_ENTRY
>
> Let's not play preprocessor games until this becomes
> a big issue. Simple
> { PSTORE_TYPE_DMESG, VIRTIO_PSTORE_TYPE_DMESG}
> does the trick just as well for now.
> Also see below.
>
>
>
> > +
> > +
>
> single empty line pls.
Ok.
>
> > +static u16 to_virtio_type(struct virtio_pstore *vps, enum pstore_type_id type)
> > +{
> > + unsigned int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(type_table); i++) {
> > + if (type == type_table[i].pstore)
> > + return cpu_to_virtio16(vps->vdev, type_table[i].virtio);
> > + }
>
> Rather complex for something that always returns a single value.
> why do we need a table at all?
> How about a switch statement?
The pstore has 4 message types and I'd like to add a few more. This
patch only implements the most popular dmesg type and others will be
added later.
But I'm ok with the switch too.
>
> static u16 to_virtio_type(struct virtio_pstore *vps, enum pstore_type_id type)
> {
> switch (type) {
> case PSTORE_TYPE_DMESG:
> return VIRTIO_PSTORE_TYPE_DMESG;
> default:
> return VIRTIO_PSTORE_TYPE_UNKNOWN;
> }
> }
>
>
> > +
> > + return cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_TYPE_UNKNOWN);
> > +}
>
> This returns an incorrect type.
Right. But it was fixed in v5 and you seem to review an earlier
version unfortunately.
>
>
> > +
> > +static enum pstore_type_id from_virtio_type(struct virtio_pstore *vps, u16 type)
> > +{
> > + unsigned int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(type_table); i++) {
> > + if (virtio16_to_cpu(vps->vdev, type) == type_table[i].virtio)
> > + return type_table[i].pstore;
> > + }
> > +
> > + return PSTORE_TYPE_UNKNOWN;
> > +}
> > +
> > +static void virtpstore_ack(struct virtqueue *vq)
> > +{
> > + struct virtio_pstore *vps = vq->vdev->priv;
> > +
> > + wake_up(&vps->acked);
> > +}
> > +
> > +static void virtpstore_check(struct virtqueue *vq)
> > +{
> > + struct virtio_pstore *vps = vq->vdev->priv;
> > + struct virtio_pstore_res *res;
> > + unsigned int len;
> > +
> > + res = virtqueue_get_buf(vq, &len);
> > + if (res == NULL)
> > + return;
> > +
> > + if (virtio32_to_cpu(vq->vdev, res->ret) < 0)
> > + vps->failed = 1;
> > +}
> > +
> > +static void virt_pstore_get_reqs(struct virtio_pstore *vps,
> > + struct virtio_pstore_req **preq,
> > + struct virtio_pstore_res **pres)
> > +{
> > + unsigned int idx = vps->req_id++ % VIRT_PSTORE_NR_REQ;
> > +
> > + *preq = &vps->req[idx];
> > + *pres = &vps->res[idx];
> > +
> > + memset(*preq, 0, sizeof(**preq));
> > + memset(*pres, 0, sizeof(**pres));
> > +}
> > +
> > +static int virt_pstore_open(struct pstore_info *psi)
> > +{
> > + struct virtio_pstore *vps = psi->data;
> > + struct virtio_pstore_req *req;
> > + struct virtio_pstore_res *res;
> > + struct scatterlist sgo[1], sgi[1];
> > + struct scatterlist *sgs[2] = { sgo, sgi };
> > + unsigned int len;
> > +
> > + virt_pstore_get_reqs(vps, &req, &res);
> > +
> > + req->cmd = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_OPEN);
> > +
> > + sg_init_one(sgo, req, sizeof(*req));
> > + sg_init_one(sgi, res, sizeof(*res));
> > + virtqueue_add_sgs(vps->vq[0], sgs, 1, 1, vps, GFP_KERNEL);
> > + virtqueue_kick(vps->vq[0]);
> > +
> > + wait_event(vps->acked, virtqueue_get_buf(vps->vq[0], &len));
> > + return virtio32_to_cpu(vps->vdev, res->ret);
> > +}
> > +
> > +static int virt_pstore_close(struct pstore_info *psi)
> > +{
> > + struct virtio_pstore *vps = psi->data;
> > + struct virtio_pstore_req *req = &vps->req[vps->req_id];
> > + struct virtio_pstore_res *res = &vps->res[vps->req_id];
> > + struct scatterlist sgo[1], sgi[1];
> > + struct scatterlist *sgs[2] = { sgo, sgi };
> > + unsigned int len;
> > +
> > + virt_pstore_get_reqs(vps, &req, &res);
> > +
> > + req->cmd = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_CLOSE);
> > +
> > + sg_init_one(sgo, req, sizeof(*req));
> > + sg_init_one(sgi, res, sizeof(*res));
> > + virtqueue_add_sgs(vps->vq[0], sgs, 1, 1, vps, GFP_KERNEL);
> > + virtqueue_kick(vps->vq[0]);
> > +
> > + wait_event(vps->acked, virtqueue_get_buf(vps->vq[0], &len));
> > + return virtio32_to_cpu(vps->vdev, res->ret);
> > +}
> > +
> > +static ssize_t virt_pstore_read(u64 *id, enum pstore_type_id *type,
> > + int *count, struct timespec *time,
> > + char **buf, bool *compressed,
> > + ssize_t *ecc_notice_size,
> > + struct pstore_info *psi)
> > +{
> > + struct virtio_pstore *vps = psi->data;
> > + struct virtio_pstore_req *req;
> > + struct virtio_pstore_res *res;
> > + struct virtio_pstore_fileinfo info;
> > + struct scatterlist sgo[1], sgi[3];
> > + struct scatterlist *sgs[2] = { sgo, sgi };
> > + unsigned int len;
> > + unsigned int flags;
> > + int ret;
> > + void *bf;
> > +
> > + virt_pstore_get_reqs(vps, &req, &res);
> > +
> > + req->cmd = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_READ);
> > +
> > + sg_init_one(sgo, req, sizeof(*req));
> > + sg_init_table(sgi, 3);
> > + sg_set_buf(&sgi[0], res, sizeof(*res));
> > + sg_set_buf(&sgi[1], &info, sizeof(info));
> > + sg_set_buf(&sgi[2], psi->buf, psi->bufsize);
> > + virtqueue_add_sgs(vps->vq[0], sgs, 1, 1, vps, GFP_KERNEL);
> > + virtqueue_kick(vps->vq[0]);
> > +
> > + wait_event(vps->acked, virtqueue_get_buf(vps->vq[0], &len));
> > + if (len < sizeof(*res) + sizeof(info))
> > + return -1;
> > +
> > + ret = virtio32_to_cpu(vps->vdev, res->ret);
> > + if (ret < 0)
> > + return ret;
> > +
> > + len = virtio32_to_cpu(vps->vdev, info.len);
> > +
> > + bf = kmalloc(len, GFP_KERNEL);
> > + if (bf == NULL)
> > + return -ENOMEM;
> > +
> > + *id = virtio64_to_cpu(vps->vdev, info.id);
> > + *type = from_virtio_type(vps, info.type);
> > + *count = virtio32_to_cpu(vps->vdev, info.count);
> > +
> > + flags = virtio32_to_cpu(vps->vdev, info.flags);
> > + *compressed = flags & VIRTIO_PSTORE_FL_COMPRESSED;
> > +
> > + time->tv_sec = virtio64_to_cpu(vps->vdev, info.time_sec);
> > + time->tv_nsec = virtio32_to_cpu(vps->vdev, info.time_nsec);
> > +
> > + memcpy(bf, psi->buf, len);
> > + *buf = bf;
> > +
> > + return len;
> > +}
> > +
> > +static int notrace virt_pstore_write(enum pstore_type_id type,
> > + enum kmsg_dump_reason reason,
> > + u64 *id, unsigned int part, int count,
> > + bool compressed, size_t size,
> > + struct pstore_info *psi)
> > +{
> > + struct virtio_pstore *vps = psi->data;
> > + struct virtio_pstore_req *req;
> > + struct virtio_pstore_res *res;
> > + struct scatterlist sgo[2], sgi[1];
> > + struct scatterlist *sgs[2] = { sgo, sgi };
> > + unsigned int flags = compressed ? VIRTIO_PSTORE_FL_COMPRESSED : 0;
> > +
> > + if (vps->failed)
> > + return -1;
> > +
> > + *id = vps->req_id;
> > + virt_pstore_get_reqs(vps, &req, &res);
> > +
> > + req->cmd = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_WRITE);
> > + req->type = to_virtio_type(vps, type);
> > + req->flags = cpu_to_virtio32(vps->vdev, flags);
> > +
> > + sg_init_table(sgo, 2);
> > + sg_set_buf(&sgo[0], req, sizeof(*req));
> > + sg_set_buf(&sgo[1], pstore_get_buf(psi), size);
> > + sg_init_one(sgi, res, sizeof(*res));
> > + virtqueue_add_sgs(vps->vq[1], sgs, 1, 1, vps, GFP_ATOMIC);
> > + virtqueue_kick(vps->vq[1]);
> > +
> > + return 0;
> > +}
> > +
> > +static int virt_pstore_erase(enum pstore_type_id type, u64 id, int count,
> > + struct timespec time, struct pstore_info *psi)
> > +{
> > + struct virtio_pstore *vps = psi->data;
> > + struct virtio_pstore_req *req;
> > + struct virtio_pstore_res *res;
> > + struct scatterlist sgo[1], sgi[1];
> > + struct scatterlist *sgs[2] = { sgo, sgi };
> > + unsigned int len;
> > +
> > + virt_pstore_get_reqs(vps, &req, &res);
> > +
> > + req->cmd = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_ERASE);
> > + req->type = to_virtio_type(vps, type);
> > + req->id = cpu_to_virtio64(vps->vdev, id);
> > + req->count = cpu_to_virtio32(vps->vdev, count);
> > +
> > + sg_init_one(sgo, req, sizeof(*req));
> > + sg_init_one(sgi, res, sizeof(*res));
> > + virtqueue_add_sgs(vps->vq[0], sgs, 1, 1, vps, GFP_KERNEL);
> > + virtqueue_kick(vps->vq[0]);
> > +
> > + wait_event(vps->acked, virtqueue_get_buf(vps->vq[0], &len));
> > + return virtio32_to_cpu(vps->vdev, res->ret);
> > +}
> > +
> > +static int virt_pstore_init(struct virtio_pstore *vps)
> > +{
> > + struct pstore_info *psinfo = &vps->pstore;
> > + int err;
> > +
> > + if (!psinfo->bufsize)
> > + psinfo->bufsize = VIRT_PSTORE_BUFSIZE;
> > +
> > + psinfo->buf = alloc_pages_exact(psinfo->bufsize, GFP_KERNEL);
> > + if (!psinfo->buf) {
> > + pr_err("cannot allocate pstore buffer\n");
> > + return -ENOMEM;
> > + }
> > +
> > + psinfo->owner = THIS_MODULE;
> > + psinfo->name = "virtio";
> > + psinfo->open = virt_pstore_open;
> > + psinfo->close = virt_pstore_close;
> > + psinfo->read = virt_pstore_read;
> > + psinfo->erase = virt_pstore_erase;
> > + psinfo->write = virt_pstore_write;
> > + psinfo->flags = PSTORE_FLAGS_DMESG;
> > +
> > + psinfo->data = vps;
> > + spin_lock_init(&psinfo->buf_lock);
> > +
> > + err = pstore_register(psinfo);
> > + if (err)
> > + kfree(psinfo->buf);
> > +
> > + return err;
> > +}
> > +
> > +static int virt_pstore_exit(struct virtio_pstore *vps)
> > +{
> > + struct pstore_info *psinfo = &vps->pstore;
> > +
> > + pstore_unregister(psinfo);
>
> I don't know enough about pstore - does this
> actually ensure that
> 1. all existing users close the device
> 2. no new users can open it
> somehow?
The pstore driver doesn't create a device node (except the pmsg device
which this patch doesn't deal with) so it doesn't need to worry about
the user AFAIK. It just calls the pstore callbacks (if any) on some
system events (e.g. kmsg dump, console write and so on).
In fact, pstore is a pseudo file system, works as read-only mode.
>
> > +
> > + free_pages_exact(psinfo->buf, psinfo->bufsize);
> > + psinfo->buf = NULL;
> > + psinfo->bufsize = 0;
> > +
> > + return 0;
> > +}
> > +
> > +static int virtpstore_init_vqs(struct virtio_pstore *vps)
> > +{
> > + vq_callback_t *callbacks[] = { virtpstore_ack, virtpstore_check };
> > + const char *names[] = { "pstore_read", "pstore_write" };
> > +
> > + return vps->vdev->config->find_vqs(vps->vdev, 2, vps->vq,
> > + callbacks, names);
> > +}
> > +
> > +static void virtpstore_init_config(struct virtio_pstore *vps)
> > +{
> > + u32 bufsize;
> > +
> > + virtio_cread(vps->vdev, struct virtio_pstore_config, bufsize, &bufsize);
> > +
> > + vps->pstore.bufsize = PAGE_ALIGN(bufsize);
> > +}
> > +
> > +static void virtpstore_confirm_config(struct virtio_pstore *vps)
> > +{
> > + u32 bufsize = vps->pstore.bufsize;
> > +
> > + virtio_cwrite(vps->vdev, struct virtio_pstore_config, bufsize,
> > + &bufsize);
> > +}
> > +
> > +static int virtpstore_probe(struct virtio_device *vdev)
> > +{
> > + struct virtio_pstore *vps;
> > + int err;
> > +
> > + if (!vdev->config->get) {
> > + dev_err(&vdev->dev, "driver init: config access disabled\n");
> > + return -EINVAL;
> > + }
> > +
> > + vdev->priv = vps = kzalloc(sizeof(*vps), GFP_KERNEL);
> > + if (!vps) {
> > + err = -ENOMEM;
> > + goto out;
> > + }
> > + vps->vdev = vdev;
> > +
> > + err = virtpstore_init_vqs(vps);
> > + if (err < 0)
> > + goto out_free;
> > +
> > + virtpstore_init_config(vps);
> > +
> > + err = virt_pstore_init(vps);
> > + if (err)
> > + goto out_del_vq;
> > +
> > + virtpstore_confirm_config(vps);
> > +
> > + init_waitqueue_head(&vps->acked);
> > +
> > + virtio_device_ready(vdev);
> > +
> > + dev_info(&vdev->dev, "driver init: ok (bufsize = %luK, flags = %x)\n",
> > + vps->pstore.bufsize >> 10, vps->pstore.flags);
> > +
> > + return 0;
> > +
> > +out_del_vq:
> > + vdev->config->del_vqs(vdev);
> > +out_free:
> > + kfree(vps);
> > +out:
> > + dev_err(&vdev->dev, "driver init: failed with %d\n", err);
> > + return err;
> > +}
> > +
> > +static void virtpstore_remove(struct virtio_device *vdev)
> > +{
> > + struct virtio_pstore *vps = vdev->priv;
> > +
> > + virt_pstore_exit(vps);
> > +
> > + /* Now we reset the device so we can clean up the queues. */
> > + vdev->config->reset(vdev);
> > +
> > + vdev->config->del_vqs(vdev);
> > +
> > + kfree(vps);
> > +}
> > +
> > +static unsigned int features[] = {
> > +};
> > +
> > +static struct virtio_device_id id_table[] = {
> > + { VIRTIO_ID_PSTORE, VIRTIO_DEV_ANY_ID },
> > + { 0 },
> > +};
> > +
> > +static struct virtio_driver virtio_pstore_driver = {
>
> We need some way to avoid trying to load this
> as a legacy device. There isn't a way to do it yet
> so I won't block your patch on this but pls try to
> come up with something, and I'll do, too.
I have no idea what I can do. Also it seems the kvmtools supports
only legacy devices (please correct me I was wrong).
>
>
> > + .driver.name = KBUILD_MODNAME,
> > + .driver.owner = THIS_MODULE,
> > + .feature_table = features,
> > + .feature_table_size = ARRAY_SIZE(features),
> > + .id_table = id_table,
> > + .probe = virtpstore_probe,
> > + .remove = virtpstore_remove,
>
> Won't this need freeze/restore callbacks?
Probably.. :) Will add.
>
> > +};
> > +
> > +module_virtio_driver(virtio_pstore_driver);
> > +MODULE_DEVICE_TABLE(virtio, id_table);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Namhyung Kim <namhyung@kernel.org>");
> > +MODULE_DESCRIPTION("Virtio pstore driver");
> > diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
> > index 6d4e92ccdc91..9bbb1554d8b2 100644
> > --- a/include/uapi/linux/Kbuild
> > +++ b/include/uapi/linux/Kbuild
> > @@ -449,6 +449,7 @@ header-y += virtio_ids.h
> > header-y += virtio_input.h
> > header-y += virtio_net.h
> > header-y += virtio_pci.h
> > +header-y += virtio_pstore.h
> > header-y += virtio_ring.h
> > header-y += virtio_rng.h
> > header-y += virtio_scsi.h
> > diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
> > index 77925f587b15..c72a9ab588c0 100644
> > --- a/include/uapi/linux/virtio_ids.h
> > +++ b/include/uapi/linux/virtio_ids.h
> > @@ -41,5 +41,6 @@
> > #define VIRTIO_ID_CAIF 12 /* Virtio caif */
> > #define VIRTIO_ID_GPU 16 /* virtio GPU */
> > #define VIRTIO_ID_INPUT 18 /* virtio input */
> > +#define VIRTIO_ID_PSTORE 22 /* virtio pstore */
> >
> > #endif /* _LINUX_VIRTIO_IDS_H */
> > diff --git a/include/uapi/linux/virtio_pstore.h b/include/uapi/linux/virtio_pstore.h
> > new file mode 100644
> > index 000000000000..f4b0d204d8ae
> > --- /dev/null
> > +++ b/include/uapi/linux/virtio_pstore.h
> > @@ -0,0 +1,74 @@
> > +#ifndef _LINUX_VIRTIO_PSTORE_H
> > +#define _LINUX_VIRTIO_PSTORE_H
> > +/* This header is BSD licensed so anyone can use the definitions to implement
> > + * compatible drivers/servers.
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions
> > + * are met:
> > + * 1. Redistributions of source code must retain the above copyright
> > + * notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above copyright
> > + * notice, this list of conditions and the following disclaimer in the
> > + * documentation and/or other materials provided with the distribution.
> > + * 3. Neither the name of IBM nor the names of its contributors
> > + * may be used to endorse or promote products derived from this software
> > + * without specific prior written permission.
> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS IS'' AND
> > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> > + * ARE DISCLAIMED. IN NO EVENT SHALL IBM OR CONTRIBUTORS BE LIABLE
> > + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> > + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> > + * SUCH DAMAGE. */
> > +#include <linux/types.h>
> > +#include <linux/virtio_types.h>
> > +
> > +#define VIRTIO_PSTORE_CMD_NULL 0
> > +#define VIRTIO_PSTORE_CMD_OPEN 1
> > +#define VIRTIO_PSTORE_CMD_READ 2
> > +#define VIRTIO_PSTORE_CMD_WRITE 3
> > +#define VIRTIO_PSTORE_CMD_ERASE 4
> > +#define VIRTIO_PSTORE_CMD_CLOSE 5
> > +
> > +#define VIRTIO_PSTORE_TYPE_UNKNOWN 0
> > +#define VIRTIO_PSTORE_TYPE_DMESG 1
> > +
> > +#define VIRTIO_PSTORE_FL_COMPRESSED 1
>
> Most other headers use _F_ and not _FL_
> Also, we specify bit number and not the
> bitmask. So:
>
> #define VIRTIO_PSTORE_F_COMPRESSED 0
>
> and
>
> (0x1 << VIRTIO_PSTORE_F_COMPRESSED)
Ok.
>
>
> > +
> > +struct virtio_pstore_req {
> > + __virtio16 cmd;
> > + __virtio16 type;
> > + __virtio32 flags;
> > + __virtio64 id;
> > + __virtio32 count;
> > + __virtio32 reserved;
> > +};
> > +
> > +struct virtio_pstore_res {
> > + __virtio16 cmd;
> > + __virtio16 type;
> > + __virtio32 ret;
> > +};
> > +
> > +struct virtio_pstore_fileinfo {
> > + __virtio64 id;
> > + __virtio32 count;
> > + __virtio16 type;
> > + __virtio16 unused;
> > + __virtio32 flags;
> > + __virtio32 len;
> > + __virtio64 time_sec;
> > + __virtio32 time_nsec;
> > + __virtio32 reserved;
>
> Any reason one is reserved the other is unused?
> If not just calls them pad1, pad2?
No specific reason, will change.
>
> > +};
> > +
> > +struct virtio_pstore_config {
> > + __virtio32 bufsize;
> > +};
> > +
>
> __virtio things are for compatibility things.
> New devices should just use __le everywhere.
Right. Again, already fixed in v5. :)
>
> Let me post a patch that adds config space accessors
> so you can do this.
Please CC me on the patch.
Thanks,
Namhyung
>
>
> > +#endif /* _LINUX_VIRTIO_PSTORE_H */
> > --
> > 2.9.3
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v9 00/19] Add support for FDMA DMA controller and slim core rproc found on STi chipsets
From: Vinod Koul @ 2016-09-15 16:20 UTC (permalink / raw)
To: Bjorn Andersson
Cc: devicetree, kernel, airlied, linux-remoteproc, patrice.chotard,
dri-devel, linux-kernel, Peter Griffin, dmaengine, dan.j.williams,
virtualization, lee.jones, linux-arm-kernel
In-Reply-To: <20160914130739.GE13920@localhost>
On Wed, Sep 14, 2016 at 06:37:40PM +0530, Vinod Koul wrote:
> On Tue, Sep 13, 2016 at 11:06:16AM -0700, Bjorn Andersson wrote:
> > > I hate to send a ping,
> >
> > Sorry about that.
> >
> > > but do you think we can merge this fdma series? It has gone
> > > through quite a few review rounds now.
> > >
> >
> > I think the remoteproc part looks good.
>
> yeah I was waiting for ack on other patches. But looks like at least
> remoteproc ones have it
>
>
> > Vinod, I don't have any changes queued in remoteproc that should cause
> > merge issues. If you want to you could take the remoteproc patch
> > through your tree.
>
> I rechecked the dma part, they look good to me, so I should be able to apply
> these. I will wait a day for ack/nacks. It is the time to speak up :)
And I have applied thru 9th patch. Others are applied by Patrice.
Btw you should send drm ones to drm folks separately and not in this
series..
Thanks
--
~Vinod
^ permalink raw reply
* [PATCH v3] virtio_pci: Limit DMA mask to 44 bits for legacy virtio devices
From: Will Deacon @ 2016-09-14 16:33 UTC (permalink / raw)
To: virtualization, kvm; +Cc: Will Deacon, Andy Lutomirski, Michael S. Tsirkin
Legacy virtio defines the virtqueue base using a 32-bit PFN field, with
a read-only register indicating a fixed page size of 4k.
This can cause problems for DMA allocators that allocate top down from
the DMA mask, which is set to 64 bits. In this case, the addresses are
silently truncated to 44-bit, leading to IOMMU faults, failure to read
from the queue or data corruption.
This patch restricts the coherent DMA mask for legacy PCI virtio devices
to 44 bits, which matches the specification.
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Benjamin Serebrin <serebrin@google.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
drivers/virtio/virtio_pci_legacy.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
index 8c4e61783441..6d9e5173d5fa 100644
--- a/drivers/virtio/virtio_pci_legacy.c
+++ b/drivers/virtio/virtio_pci_legacy.c
@@ -212,10 +212,18 @@ int virtio_pci_legacy_probe(struct virtio_pci_device *vp_dev)
return -ENODEV;
}
- rc = dma_set_mask_and_coherent(&pci_dev->dev, DMA_BIT_MASK(64));
- if (rc)
- rc = dma_set_mask_and_coherent(&pci_dev->dev,
- DMA_BIT_MASK(32));
+ rc = dma_set_mask(&pci_dev->dev, DMA_BIT_MASK(64));
+ if (rc) {
+ rc = dma_set_mask_and_coherent(&pci_dev->dev, DMA_BIT_MASK(32));
+ } else {
+ /*
+ * The virtio ring base address is expressed as a 32-bit PFN,
+ * with a page size of 1 << VIRTIO_PCI_QUEUE_ADDR_SHIFT.
+ */
+ dma_set_coherent_mask(&pci_dev->dev,
+ DMA_BIT_MASK(32 + VIRTIO_PCI_QUEUE_ADDR_SHIFT));
+ }
+
if (rc)
dev_warn(&pci_dev->dev, "Failed to enable 64-bit or 32-bit DMA. Trying to continue, but this might not work.\n");
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v2] virtio_pci: Limit DMA mask to 44 bits for legacy virtio devices
From: Will Deacon @ 2016-09-14 16:03 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Andy Lutomirski, kvm, virtualization
In-Reply-To: <20160914154034-mutt-send-email-mst@kernel.org>
Hi Michael,
On Wed, Sep 14, 2016 at 03:42:25PM +0300, Michael S. Tsirkin wrote:
> On Wed, Sep 14, 2016 at 12:16:28PM +0100, Will Deacon wrote:
> > Legacy virtio defines the virtqueue base using a 32-bit PFN field, with
> > a read-only register indicating a fixed page size of 4k.
> >
> > This can cause problems for DMA allocators that allocate top down from
> > the DMA mask, which is set to 64 bits. In this case, the addresses are
> > silently truncated to 44-bit, leading to IOMMU faults, failure to read
> > from the queue or data corruption.
> >
> > This patch restricts the DMA mask for legacy PCI virtio devices to
> > 44 bits, which matches the specification.
> >
> > Cc: Andy Lutomirski <luto@kernel.org>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Benjamin Serebrin <serebrin@google.com>
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > ---
> > drivers/virtio/virtio_pci_legacy.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
> > index 8c4e61783441..efb3f5dff4b7 100644
> > --- a/drivers/virtio/virtio_pci_legacy.c
> > +++ b/drivers/virtio/virtio_pci_legacy.c
> > @@ -212,12 +212,17 @@ int virtio_pci_legacy_probe(struct virtio_pci_device *vp_dev)
> > return -ENODEV;
> > }
> >
> > - rc = dma_set_mask_and_coherent(&pci_dev->dev, DMA_BIT_MASK(64));
> > + /*
> > + * The virtio ring base address is expressed as a 32-bit PFN, with a
> > + * page size of 1 << VIRTIO_PCI_QUEUE_ADDR_SHIFT.
> > + */
> > + rc = dma_set_mask_and_coherent(&pci_dev->dev,
> > + DMA_BIT_MASK(32 + VIRTIO_PCI_QUEUE_ADDR_SHIFT));
>
> So why not limit just the coherent mask, as I suggested in a comment on v1?
Sorry, I completely missed that suggestion. I'll roll a v3 with that
included, although I'll rejig things a bit to make the error handling
a bit more bearable.
> > if (rc)
> > rc = dma_set_mask_and_coherent(&pci_dev->dev,
> > DMA_BIT_MASK(32));
> > if (rc)
> > - dev_warn(&pci_dev->dev, "Failed to enable 64-bit or 32-bit DMA. Trying to continue, but this might not work.\n");
> > + dev_warn(&pci_dev->dev, "Failed to enable %d-bit or 32-bit DMA. Trying to continue, but this might not work.\n", 32 + VIRTIO_PCI_QUEUE_ADDR_SHIFT);
>
>
> I'd rather we split this.
I'll actually just leave like it was in the first place if we're trying a
64-bit streaming mask.
Stay tuned,
Will
^ permalink raw reply
* [PATCH 11/11] virtio_console: Rename a jump label in five functions
From: SF Markus Elfring @ 2016-09-14 14:10 UTC (permalink / raw)
To: virtualization, Amit Shah, Arnd Bergmann, Greg Kroah-Hartman,
Michael S. Tsirkin, Rusty Russell
Cc: Julia Lawall, kernel-janitors, LKML
In-Reply-To: <020438b9-a7f8-0050-04c1-43382ba60b75@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 14 Sep 2016 15:37:52 +0200
Adjust a jump label according to the current Linux coding style convention.
Thus replace the identifier "out" by "unlock".
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/char/virtio_console.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 0c4d4e7..6c90c9c 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -257,11 +257,11 @@ static struct port *find_port_by_vtermno(u32 vtermno)
list_for_each_entry(cons, &pdrvdata.consoles, list) {
if (cons->vtermno == vtermno) {
port = container_of(cons, struct port, cons);
- goto out;
+ goto unlock;
}
}
port = NULL;
-out:
+ unlock:
spin_unlock_irqrestore(&pdrvdata_lock, flags);
return port;
}
@@ -276,11 +276,11 @@ static struct port *find_port_by_devt_in_portdev(struct ports_device *portdev,
list_for_each_entry(port, &portdev->ports, list) {
if (port->cdev->dev == dev) {
kref_get(&port->kref);
- goto out;
+ goto unlock;
}
}
port = NULL;
-out:
+ unlock:
spin_unlock_irqrestore(&portdev->ports_lock, flags);
return port;
@@ -296,10 +296,10 @@ static struct port *find_port_by_devt(dev_t dev)
list_for_each_entry(portdev, &pdrvdata.portdevs, list) {
port = find_port_by_devt_in_portdev(portdev, dev);
if (port)
- goto out;
+ goto unlock;
}
port = NULL;
-out:
+ unlock:
spin_unlock_irqrestore(&pdrvdata_lock, flags);
return port;
}
@@ -312,9 +312,9 @@ static struct port *find_port_by_id(struct ports_device *portdev, u32 id)
spin_lock_irqsave(&portdev->ports_lock, flags);
list_for_each_entry(port, &portdev->ports, list)
if (port->id == id)
- goto out;
+ goto unlock;
port = NULL;
-out:
+ unlock:
spin_unlock_irqrestore(&portdev->ports_lock, flags);
return port;
@@ -329,9 +329,9 @@ static struct port *find_port_by_vq(struct ports_device *portdev,
spin_lock_irqsave(&portdev->ports_lock, flags);
list_for_each_entry(port, &portdev->ports, list)
if (port->in_vq == vq || port->out_vq == vq)
- goto out;
+ goto unlock;
port = NULL;
-out:
+ unlock:
spin_unlock_irqrestore(&portdev->ports_lock, flags);
return port;
}
--
2.10.0
^ permalink raw reply related
* [PATCH 10/11] virtio_console: Rename jump labels in alloc_buf()
From: SF Markus Elfring @ 2016-09-14 14:09 UTC (permalink / raw)
To: virtualization, Amit Shah, Arnd Bergmann, Greg Kroah-Hartman,
Michael S. Tsirkin, Rusty Russell
Cc: Julia Lawall, kernel-janitors, LKML
In-Reply-To: <020438b9-a7f8-0050-04c1-43382ba60b75@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 14 Sep 2016 15:20:30 +0200
Adjust jump labels according to the current Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/char/virtio_console.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 69c6718..0c4d4e7 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -431,7 +431,7 @@ static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size,
buf = kmalloc(sizeof(*buf) + sizeof(struct scatterlist) * pages,
GFP_KERNEL);
if (!buf)
- goto fail;
+ goto exit;
buf->sgpages = pages;
if (pages > 0) {
@@ -451,7 +451,7 @@ static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size,
* in dma-coherent.c
*/
if (!vq->vdev->dev.parent || !vq->vdev->dev.parent->parent)
- goto free_buf;
+ goto free_buffer;
buf->dev = vq->vdev->dev.parent->parent;
/* Increase device refcnt to avoid freeing it */
@@ -464,15 +464,14 @@ static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size,
}
if (!buf->buf)
- goto free_buf;
+ goto free_buffer;
buf->len = 0;
buf->offset = 0;
buf->size = buf_size;
return buf;
-
-free_buf:
+ free_buffer:
kfree(buf);
-fail:
+ exit:
return NULL;
}
--
2.10.0
^ permalink raw reply related
* [PATCH 09/11] virtio_console: Rename a jump label in __send_to_port()
From: SF Markus Elfring @ 2016-09-14 14:08 UTC (permalink / raw)
To: virtualization, Amit Shah, Arnd Bergmann, Greg Kroah-Hartman,
Michael S. Tsirkin, Rusty Russell
Cc: Julia Lawall, kernel-janitors, LKML
In-Reply-To: <020438b9-a7f8-0050-04c1-43382ba60b75@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 14 Sep 2016 15:15:06 +0200
Adjust a jump label according to the current Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/char/virtio_console.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index babc812..69c6718 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -634,14 +634,14 @@ static ssize_t __send_to_port(struct port *port, struct scatterlist *sg,
if (err) {
in_count = 0;
- goto done;
+ goto unlock;
}
if (out_vq->num_free == 0)
port->outvq_full = true;
if (nonblock)
- goto done;
+ goto unlock;
/*
* Wait till the host acknowledges it pushed out the data we
@@ -655,7 +655,7 @@ static ssize_t __send_to_port(struct port *port, struct scatterlist *sg,
while (!virtqueue_get_buf(out_vq, &len)
&& !virtqueue_is_broken(out_vq))
cpu_relax();
-done:
+ unlock:
spin_unlock_irqrestore(&port->outvq_lock, flags);
port->stats.bytes_sent += in_count;
--
2.10.0
^ permalink raw reply related
* [PATCH 08/11] virtio_console: Rename jump labels in port_fops_write()
From: SF Markus Elfring @ 2016-09-14 14:07 UTC (permalink / raw)
To: virtualization, Amit Shah, Arnd Bergmann, Greg Kroah-Hartman,
Michael S. Tsirkin, Rusty Russell
Cc: Julia Lawall, kernel-janitors, LKML
In-Reply-To: <020438b9-a7f8-0050-04c1-43382ba60b75@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 14 Sep 2016 15:07:42 +0200
Adjust jump labels according to the current Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/char/virtio_console.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index d8681d9..babc812 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -842,7 +842,7 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
ret = copy_from_user(buf->buf, ubuf, count);
if (ret) {
ret = -EFAULT;
- goto free_buf;
+ goto free_buffer;
}
/*
@@ -857,11 +857,10 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
ret = __send_to_port(port, sg, 1, count, buf, nonblock);
if (nonblock && ret > 0)
- goto out;
-
-free_buf:
+ goto exit;
+ free_buffer:
free_buf(buf, true);
-out:
+ exit:
return ret;
}
--
2.10.0
^ permalink raw reply related
* [PATCH 07/11] virtio_console: Rename a jump label in port_fops_splice_write()
From: SF Markus Elfring @ 2016-09-14 14:06 UTC (permalink / raw)
To: virtualization, Amit Shah, Arnd Bergmann, Greg Kroah-Hartman,
Michael S. Tsirkin, Rusty Russell
Cc: Julia Lawall, kernel-janitors, LKML
In-Reply-To: <020438b9-a7f8-0050-04c1-43382ba60b75@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 14 Sep 2016 15:01:51 +0200
Adjust a jump label according to the current Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/char/virtio_console.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 99dc659..d8681d9 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -947,17 +947,17 @@ static ssize_t port_fops_splice_write(struct pipe_inode_info *pipe,
pipe_lock(pipe);
if (!pipe->nrbufs) {
ret = 0;
- goto error_out;
+ goto unlock;
}
ret = wait_port_writable(port, filp->f_flags & O_NONBLOCK);
if (ret < 0)
- goto error_out;
+ goto unlock;
buf = alloc_buf(port->out_vq, 0, pipe->nrbufs);
if (!buf) {
ret = -ENOMEM;
- goto error_out;
+ goto unlock;
}
sgl.n = 0;
@@ -973,8 +973,7 @@ static ssize_t port_fops_splice_write(struct pipe_inode_info *pipe,
if (unlikely(ret <= 0))
free_buf(buf, true);
return ret;
-
-error_out:
+ unlock:
pipe_unlock(pipe);
return ret;
}
--
2.10.0
^ permalink raw reply related
* [PATCH 06/11] virtio_console: Rename a jump label in port_fops_open()
From: SF Markus Elfring @ 2016-09-14 14:05 UTC (permalink / raw)
To: virtualization, Amit Shah, Arnd Bergmann, Greg Kroah-Hartman,
Michael S. Tsirkin, Rusty Russell
Cc: Julia Lawall, kernel-janitors, LKML
In-Reply-To: <020438b9-a7f8-0050-04c1-43382ba60b75@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 14 Sep 2016 14:58:24 +0200
Adjust a jump label according to the current Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/char/virtio_console.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 40b8775..99dc659 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1058,7 +1058,7 @@ static int port_fops_open(struct inode *inode, struct file *filp)
*/
if (is_console_port(port)) {
ret = -ENXIO;
- goto out;
+ goto put_ref;
}
/* Allow only one process to open a particular port at a time */
@@ -1066,7 +1066,7 @@ static int port_fops_open(struct inode *inode, struct file *filp)
if (port->guest_connected) {
spin_unlock_irq(&port->inbuf_lock);
ret = -EBUSY;
- goto out;
+ goto put_ref;
}
port->guest_connected = true;
@@ -1087,7 +1087,7 @@ static int port_fops_open(struct inode *inode, struct file *filp)
send_control_msg(filp->private_data, VIRTIO_CONSOLE_PORT_OPEN, 1);
return 0;
-out:
+ put_ref:
kref_put(&port->kref, remove_port);
return ret;
}
--
2.10.0
^ permalink raw reply related
* [PATCH 05/11] virtio_console: Rename jump labels in add_port()
From: SF Markus Elfring @ 2016-09-14 14:04 UTC (permalink / raw)
To: virtualization, Amit Shah, Arnd Bergmann, Greg Kroah-Hartman,
Michael S. Tsirkin, Rusty Russell
Cc: Julia Lawall, kernel-janitors, LKML
In-Reply-To: <020438b9-a7f8-0050-04c1-43382ba60b75@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 14 Sep 2016 14:53:00 +0200
Adjust jump labels according to the current Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/char/virtio_console.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 768bbb7..40b8775 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1399,7 +1399,7 @@ static int add_port(struct ports_device *portdev, u32 id)
port = kmalloc(sizeof(*port), GFP_KERNEL);
if (!port) {
err = -ENOMEM;
- goto fail;
+ goto send_control_message;
}
kref_init(&port->kref);
@@ -1434,7 +1434,7 @@ static int add_port(struct ports_device *portdev, u32 id)
if (err < 0) {
dev_err(&port->portdev->vdev->dev,
"Error %d adding cdev for port %u\n", err, id);
- goto free_cdev;
+ goto delete_cdev;
}
port->dev = device_create(pdrvdata.class, &port->portdev->vdev->dev,
devt, port, "vport%up%u",
@@ -1444,7 +1444,7 @@ static int add_port(struct ports_device *portdev, u32 id)
dev_err(&port->portdev->vdev->dev,
"Error %d creating device for port %u\n",
err, id);
- goto free_cdev;
+ goto delete_cdev;
}
spin_lock_init(&port->inbuf_lock);
@@ -1456,7 +1456,7 @@ static int add_port(struct ports_device *portdev, u32 id)
if (!nr_added_bufs) {
dev_err(port->dev, "Error allocating inbufs\n");
err = -ENOMEM;
- goto free_device;
+ goto destroy_device;
}
if (is_rproc_serial(port->portdev->vdev))
@@ -1473,7 +1473,7 @@ static int add_port(struct ports_device *portdev, u32 id)
*/
err = init_port_console(port);
if (err)
- goto free_inbufs;
+ goto free_buffers;
}
spin_lock_irq(&portdev->ports_lock);
@@ -1500,17 +1500,16 @@ static int add_port(struct ports_device *portdev, u32 id)
&port_debugfs_ops);
}
return 0;
-
-free_inbufs:
+ free_buffers:
while ((buf = virtqueue_detach_unused_buf(port->in_vq)))
free_buf(buf, true);
-free_device:
+ destroy_device:
device_destroy(pdrvdata.class, port->dev->devt);
-free_cdev:
+ delete_cdev:
cdev_del(port->cdev);
-free_port:
+ free_port:
kfree(port);
-fail:
+ send_control_message:
/* The host might want to notify management sw about port add failure */
__send_control_msg(portdev, id, VIRTIO_CONSOLE_PORT_READY, 0);
return err;
--
2.10.0
^ permalink raw reply related
* [PATCH 04/11] virtio_console: Rename jump labels in virtcons_probe()
From: SF Markus Elfring @ 2016-09-14 14:03 UTC (permalink / raw)
To: virtualization, Amit Shah, Arnd Bergmann, Greg Kroah-Hartman,
Michael S. Tsirkin, Rusty Russell
Cc: Julia Lawall, kernel-janitors, LKML
In-Reply-To: <020438b9-a7f8-0050-04c1-43382ba60b75@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 14 Sep 2016 14:24:05 +0200
Adjust jump labels according to the current Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/char/virtio_console.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 004314e..768bbb7 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -2037,7 +2037,7 @@ static int virtcons_probe(struct virtio_device *vdev)
portdev = kmalloc(sizeof(*portdev), GFP_KERNEL);
if (!portdev) {
err = -ENOMEM;
- goto fail;
+ goto exit;
}
/* Attach this portdev to this virtio_device, and vice-versa. */
@@ -2051,7 +2051,7 @@ static int virtcons_probe(struct virtio_device *vdev)
"Error %d registering chrdev for device %u\n",
portdev->chr_major, vdev->index);
err = portdev->chr_major;
- goto free;
+ goto free_port;
}
multiport = false;
@@ -2068,7 +2068,7 @@ static int virtcons_probe(struct virtio_device *vdev)
err = init_vqs(portdev);
if (err < 0) {
dev_err(&vdev->dev, "Error %d initializing vqs\n", err);
- goto free_chrdev;
+ goto unregister;
}
spin_lock_init(&portdev->ports_lock);
@@ -2091,7 +2091,7 @@ static int virtcons_probe(struct virtio_device *vdev)
dev_err(&vdev->dev,
"Error allocating buffers for control queue\n");
err = -ENOMEM;
- goto free_vqs;
+ goto send_control_message;
}
} else {
/*
@@ -2121,17 +2121,16 @@ static int virtcons_probe(struct virtio_device *vdev)
wait_for_completion(&early_console_added);
return 0;
-
-free_vqs:
+ send_control_message:
/* The host might want to notify mgmt sw about device add failure */
__send_control_msg(portdev, VIRTIO_CONSOLE_BAD_ID,
VIRTIO_CONSOLE_DEVICE_READY, 0);
remove_vqs(portdev);
-free_chrdev:
+ unregister:
unregister_chrdev(portdev->chr_major, "virtio-portsdev");
-free:
+ free_port:
kfree(portdev);
-fail:
+ exit:
return err;
}
--
2.10.0
^ permalink raw reply related
* [PATCH 03/11] virtio_console: Rename a jump label in init()
From: SF Markus Elfring @ 2016-09-14 14:02 UTC (permalink / raw)
To: virtualization, Amit Shah, Arnd Bergmann, Greg Kroah-Hartman,
Michael S. Tsirkin, Rusty Russell
Cc: Julia Lawall, kernel-janitors, LKML
In-Reply-To: <020438b9-a7f8-0050-04c1-43382ba60b75@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 14 Sep 2016 14:10:24 +0200
Adjust jump labels according to the current Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/char/virtio_console.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index bf0ad57..004314e 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -2309,7 +2309,7 @@ static int __init init(void)
err = register_virtio_driver(&virtio_console);
if (err < 0) {
pr_err("Error %d registering virtio driver\n", err);
- goto free;
+ goto remove;
}
err = register_virtio_driver(&virtio_rproc_serial);
if (err < 0) {
@@ -2318,9 +2318,9 @@ static int __init init(void)
goto unregister;
}
return 0;
-unregister:
+ unregister:
unregister_virtio_driver(&virtio_console);
-free:
+ remove:
debugfs_remove_recursive(pdrvdata.debugfs_dir);
class_destroy(pdrvdata.class);
return err;
--
2.10.0
^ permalink raw reply related
* [PATCH 02/11] virtio_console: Less function calls in init_vqs() after error detection
From: SF Markus Elfring @ 2016-09-14 14:01 UTC (permalink / raw)
To: virtualization, Amit Shah, Arnd Bergmann, Greg Kroah-Hartman,
Michael S. Tsirkin, Rusty Russell
Cc: Julia Lawall, kernel-janitors, LKML
In-Reply-To: <020438b9-a7f8-0050-04c1-43382ba60b75@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 14 Sep 2016 14:00:35 +0200
The kfree() function was called in up to five cases
by the init_vqs() function during error handling even if
the passed variable contained a null pointer.
* Return directly after a call of the function "kmalloc_array" failed
at the beginning.
* Split a condition check for memory allocation failures so that
each pointer from these function calls will be checked immediately.
See also background information:
Topic "CWE-754: Improper check for unusual or exceptional conditions"
Link: https://cwe.mitre.org/data/definitions/754.html
* Adjust jump targets according to the Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/char/virtio_console.c | 32 ++++++++++++++++++++++++++------
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 325ebc6..bf0ad57 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1882,20 +1882,37 @@ static int init_vqs(struct ports_device *portdev)
nr_queues = use_multiport(portdev) ? (nr_ports + 1) * 2 : 2;
vqs = kmalloc_array(nr_queues, sizeof(*vqs), GFP_KERNEL);
+ if (!vqs)
+ return -ENOMEM;
+
io_callbacks = kmalloc_array(nr_queues,
sizeof(*io_callbacks),
GFP_KERNEL);
+ if (!io_callbacks) {
+ err = -ENOMEM;
+ goto free_vqs;
+ }
+
io_names = kmalloc_array(nr_queues, sizeof(*io_names), GFP_KERNEL);
+ if (!io_names) {
+ err = -ENOMEM;
+ goto free_callbacks;
+ }
+
portdev->in_vqs = kmalloc_array(nr_ports,
sizeof(*portdev->in_vqs),
GFP_KERNEL);
+ if (!portdev->in_vqs) {
+ err = -ENOMEM;
+ goto free_names;
+ }
+
portdev->out_vqs = kmalloc_array(nr_ports,
sizeof(*portdev->out_vqs),
GFP_KERNEL);
- if (!vqs || !io_callbacks || !io_names || !portdev->in_vqs ||
- !portdev->out_vqs) {
+ if (!portdev->out_vqs) {
err = -ENOMEM;
- goto free;
+ goto free_in_vqs;
}
/*
@@ -1929,7 +1946,7 @@ static int init_vqs(struct ports_device *portdev)
io_callbacks,
(const char **)io_names);
if (err)
- goto free;
+ goto free_out_vqs;
j = 0;
portdev->in_vqs[0] = vqs[0];
@@ -1950,12 +1967,15 @@ static int init_vqs(struct ports_device *portdev)
kfree(vqs);
return 0;
-
-free:
+ free_out_vqs:
kfree(portdev->out_vqs);
+ free_in_vqs:
kfree(portdev->in_vqs);
+ free_names:
kfree(io_names);
+ free_callbacks:
kfree(io_callbacks);
+ free_vqs:
kfree(vqs);
return err;
--
2.10.0
^ permalink raw reply related
* [PATCH 01/11] virtio_console: Use kmalloc_array() in init_vqs()
From: SF Markus Elfring @ 2016-09-14 14:00 UTC (permalink / raw)
To: virtualization, Amit Shah, Arnd Bergmann, Greg Kroah-Hartman,
Michael S. Tsirkin, Rusty Russell
Cc: Julia Lawall, kernel-janitors, LKML
In-Reply-To: <020438b9-a7f8-0050-04c1-43382ba60b75@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 14 Sep 2016 11:23:59 +0200
* Multiplications for the size determination of memory allocations
indicated that array data structures should be processed.
Thus use the corresponding function "kmalloc_array".
This issue was detected by using the Coccinelle software.
* Replace the specifications of data types by pointer dereferences
to make the corresponding size determination a bit safer according to
the Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/char/virtio_console.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index d2406fe..325ebc6 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1881,13 +1881,17 @@ static int init_vqs(struct ports_device *portdev)
nr_ports = portdev->config.max_nr_ports;
nr_queues = use_multiport(portdev) ? (nr_ports + 1) * 2 : 2;
- vqs = kmalloc(nr_queues * sizeof(struct virtqueue *), GFP_KERNEL);
- io_callbacks = kmalloc(nr_queues * sizeof(vq_callback_t *), GFP_KERNEL);
- io_names = kmalloc(nr_queues * sizeof(char *), GFP_KERNEL);
- portdev->in_vqs = kmalloc(nr_ports * sizeof(struct virtqueue *),
- GFP_KERNEL);
- portdev->out_vqs = kmalloc(nr_ports * sizeof(struct virtqueue *),
- GFP_KERNEL);
+ vqs = kmalloc_array(nr_queues, sizeof(*vqs), GFP_KERNEL);
+ io_callbacks = kmalloc_array(nr_queues,
+ sizeof(*io_callbacks),
+ GFP_KERNEL);
+ io_names = kmalloc_array(nr_queues, sizeof(*io_names), GFP_KERNEL);
+ portdev->in_vqs = kmalloc_array(nr_ports,
+ sizeof(*portdev->in_vqs),
+ GFP_KERNEL);
+ portdev->out_vqs = kmalloc_array(nr_ports,
+ sizeof(*portdev->out_vqs),
+ GFP_KERNEL);
if (!vqs || !io_callbacks || !io_names || !portdev->in_vqs ||
!portdev->out_vqs) {
err = -ENOMEM;
--
2.10.0
^ permalink raw reply related
* [PATCH 00/11] virtio-console: Fine-tuning for 14 function implementations
From: SF Markus Elfring @ 2016-09-14 13:56 UTC (permalink / raw)
To: virtualization, Amit Shah, Arnd Bergmann, Greg Kroah-Hartman,
Michael S. Tsirkin, Rusty Russell
Cc: Julia Lawall, kernel-janitors, LKML
In-Reply-To: <566ABCD9.1060404@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 14 Sep 2016 15:43:21 +0200
Several update suggestions were taken into account
from static source code analysis.
Markus Elfring (11):
Use kmalloc_array() in init_vqs()
Less function calls in init_vqs() after error detection
Rename a jump label in init()
Rename jump labels in virtcons_probe()
Rename jump labels in add_port()
Rename a jump label in port_fops_open()
Rename a jump label in port_fops_splice_write()
Rename jump labels in port_fops_write()
Rename a jump label in __send_to_port()
Rename jump labels in alloc_buf()
Rename a jump label in five functions
drivers/char/virtio_console.c | 155 ++++++++++++++++++++++++------------------
1 file changed, 87 insertions(+), 68 deletions(-)
--
2.10.0
^ permalink raw reply
* Re: [PATCH v9 00/19] Add support for FDMA DMA controller and slim core rproc found on STi chipsets
From: Vinod Koul @ 2016-09-14 13:07 UTC (permalink / raw)
To: Bjorn Andersson
Cc: devicetree, kernel, airlied, linux-remoteproc, patrice.chotard,
dri-devel, linux-kernel, Peter Griffin, dmaengine, dan.j.williams,
virtualization, lee.jones, linux-arm-kernel
In-Reply-To: <20160913180616.GD21438@tuxbot>
On Tue, Sep 13, 2016 at 11:06:16AM -0700, Bjorn Andersson wrote:
> > I hate to send a ping,
>
> Sorry about that.
>
> > but do you think we can merge this fdma series? It has gone
> > through quite a few review rounds now.
> >
>
> I think the remoteproc part looks good.
yeah I was waiting for ack on other patches. But looks like at least
remoteproc ones have it
> Vinod, I don't have any changes queued in remoteproc that should cause
> merge issues. If you want to you could take the remoteproc patch
> through your tree.
I rechecked the dma part, they look good to me, so I should be able to apply
these. I will wait a day for ack/nacks. It is the time to speak up :)
--
~Vinod
^ permalink raw reply
* Re: [PATCH v2] virtio_pci: Limit DMA mask to 44 bits for legacy virtio devices
From: Michael S. Tsirkin @ 2016-09-14 12:42 UTC (permalink / raw)
To: Will Deacon; +Cc: Andy Lutomirski, kvm, virtualization
In-Reply-To: <1473851788-31504-1-git-send-email-will.deacon@arm.com>
On Wed, Sep 14, 2016 at 12:16:28PM +0100, Will Deacon wrote:
> Legacy virtio defines the virtqueue base using a 32-bit PFN field, with
> a read-only register indicating a fixed page size of 4k.
>
> This can cause problems for DMA allocators that allocate top down from
> the DMA mask, which is set to 64 bits. In this case, the addresses are
> silently truncated to 44-bit, leading to IOMMU faults, failure to read
> from the queue or data corruption.
>
> This patch restricts the DMA mask for legacy PCI virtio devices to
> 44 bits, which matches the specification.
>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Benjamin Serebrin <serebrin@google.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
> drivers/virtio/virtio_pci_legacy.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
> index 8c4e61783441..efb3f5dff4b7 100644
> --- a/drivers/virtio/virtio_pci_legacy.c
> +++ b/drivers/virtio/virtio_pci_legacy.c
> @@ -212,12 +212,17 @@ int virtio_pci_legacy_probe(struct virtio_pci_device *vp_dev)
> return -ENODEV;
> }
>
> - rc = dma_set_mask_and_coherent(&pci_dev->dev, DMA_BIT_MASK(64));
> + /*
> + * The virtio ring base address is expressed as a 32-bit PFN, with a
> + * page size of 1 << VIRTIO_PCI_QUEUE_ADDR_SHIFT.
> + */
> + rc = dma_set_mask_and_coherent(&pci_dev->dev,
> + DMA_BIT_MASK(32 + VIRTIO_PCI_QUEUE_ADDR_SHIFT));
So why not limit just the coherent mask, as I suggested in a comment on v1?
> if (rc)
> rc = dma_set_mask_and_coherent(&pci_dev->dev,
> DMA_BIT_MASK(32));
> if (rc)
> - dev_warn(&pci_dev->dev, "Failed to enable 64-bit or 32-bit DMA. Trying to continue, but this might not work.\n");
> + dev_warn(&pci_dev->dev, "Failed to enable %d-bit or 32-bit DMA. Trying to continue, but this might not work.\n", 32 + VIRTIO_PCI_QUEUE_ADDR_SHIFT);
I'd rather we split this.
>
> rc = pci_request_region(pci_dev, 0, "virtio-pci-legacy");
> if (rc)
> --
> 2.1.4
^ permalink raw reply
* Re: [PATCH v9 16/19] ARM: DT: STi: stihxxx-b2120: Add DT nodes for STi audio card
From: Patrice Chotard @ 2016-09-14 12:01 UTC (permalink / raw)
To: Peter Griffin, linux-arm-kernel, linux-kernel, kernel, vinod.koul,
dan.j.williams, airlied, kraxel, ohad, bjorn.andersson
Cc: devicetree, Arnaud Pouliquen, linux-remoteproc, dri-devel,
virtualization, dmaengine, lee.jones
In-Reply-To: <1473081421-16555-17-git-send-email-peter.griffin@linaro.org>
Hi Peter
On 09/05/2016 03:16 PM, Peter Griffin wrote:
> This patch enables the uniperif players 2 & 3 for b2120 boards
> and also adds the "simple-audio-card" device node to interconnect
> the SoC sound device and the codec.
>
> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
> arch/arm/boot/dts/stihxxx-b2120.dtsi | 45 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stihxxx-b2120.dtsi b/arch/arm/boot/dts/stihxxx-b2120.dtsi
> index 722c63f..4939501 100644
> --- a/arch/arm/boot/dts/stihxxx-b2120.dtsi
> +++ b/arch/arm/boot/dts/stihxxx-b2120.dtsi
> @@ -131,5 +131,50 @@
> dvb-card = <STV0367_TDA18212_NIMA_1>;
> };
> };
> +
> + sti_uni_player2: sti-uni-player@8d82000 {
> + status = "okay";
> + };
> +
> + sti_uni_player3: sti-uni-player@8d85000 {
> + status = "okay";
> + };
> +
> + sti_sasg_codec: sti-sasg-codec {
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_spdif_out>;
> + };
> +
> + sound {
> + compatible = "simple-audio-card";
> + simple-audio-card,name = "sti audio card";
> + status = "okay";
> +
> + simple-audio-card,dai-link@0 {
> + /* DAC */
> + format = "i2s";
> + mclk-fs = <256>;
> + cpu {
> + sound-dai = <&sti_uni_player2>;
> + };
> +
> + codec {
> + sound-dai = <&sti_sasg_codec 1>;
> + };
> + };
> + simple-audio-card,dai-link@1 {
> + /* SPDIF */
> + format = "left_j";
> + mclk-fs = <128>;
> + cpu {
> + sound-dai = <&sti_uni_player3>;
> + };
> +
> + codec {
> + sound-dai = <&sti_sasg_codec 0>;
> + };
> + };
> + };
> };
> };
>
Applied for STi next
Thanks
^ permalink raw reply
* Re: [PATCH v9 15/19] ARM: STi: DT: STiH407: Add uniperif reader dt nodes
From: Patrice Chotard @ 2016-09-14 12:01 UTC (permalink / raw)
To: Peter Griffin, linux-arm-kernel, linux-kernel, kernel, vinod.koul,
dan.j.williams, airlied, kraxel, ohad, bjorn.andersson
Cc: devicetree, Arnaud Pouliquen, linux-remoteproc, dri-devel,
virtualization, dmaengine, lee.jones
In-Reply-To: <1473081421-16555-16-git-send-email-peter.griffin@linaro.org>
Hi Peter
On 09/05/2016 03:16 PM, Peter Griffin wrote:
> This patch adds the DT node for the uniperif reader
> IP block found on STiH407 family silicon.
>
> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
> arch/arm/boot/dts/stih407-family.dtsi | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
> index 1edc36c..883019a 100644
> --- a/arch/arm/boot/dts/stih407-family.dtsi
> +++ b/arch/arm/boot/dts/stih407-family.dtsi
> @@ -960,5 +960,33 @@
>
> status = "disabled";
> };
> +
> + sti_uni_reader0: sti-uni-reader@8d83000 {
> + compatible = "st,sti-uni-reader";
> + #sound-dai-cells = <0>;
> + st,syscfg = <&syscfg_core>;
> + reg = <0x8d83000 0x158>;
> + interrupts = <GIC_SPI 87 IRQ_TYPE_NONE>;
> + dmas = <&fdma0 5 0 1>;
> + dma-names = "rx";
> + dai-name = "Uni Reader #0 (PCM IN)";
> + st,version = <3>;
> +
> + status = "disabled";
> + };
> +
> + sti_uni_reader1: sti-uni-reader@8d84000 {
> + compatible = "st,sti-uni-reader";
> + #sound-dai-cells = <0>;
> + st,syscfg = <&syscfg_core>;
> + reg = <0x8d84000 0x158>;
> + interrupts = <GIC_SPI 88 IRQ_TYPE_NONE>;
> + dmas = <&fdma0 6 0 1>;
> + dma-names = "rx";
> + dai-name = "Uni Reader #1 (HDMI RX)";
> + st,version = <3>;
> +
> + status = "disabled";
> + };
> };
> };
>
Applied for STi next
Thanks
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox