Linux userland API discussions
 help / color / mirror / Atom feed
* Re: [PATCHv3 0/8] CGroup Namespaces
From: Aditya Kali @ 2014-12-05  3:20 UTC (permalink / raw)
  To: Tejun Heo, Li Zefan, Serge Hallyn, Andy Lutomirski,
	Eric W. Biederman, cgroups mailinglist,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux API,
	Ingo Molnar
  Cc: Linux Containers, Rohit Jnagal, Vivek Goyal, Richard Weinberger
In-Reply-To: <1417744550-6461-1-git-send-email-adityakali-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

These patches are now also hosted on github at
https://github.com/adityakali/linux/tree/cgroupns_v3.

Thanks,

On Thu, Dec 4, 2014 at 5:55 PM, Aditya Kali <adityakali-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
> Another spin for CGroup Namespaces feature.
>
> Changes from V2:
> 1. Added documentation in Documentation/cgroups/namespace.txt
> 2. Fixed a bug that caused crash
> 3. Incorporated some other suggestions from last patchset:
>    - removed use of threadgroup_lock() while creating new cgroupns
>    - use task_lock() instead of rcu_read_lock() while accessing
>      task->nsproxy
>    - optimized setns() to own cgroupns
>    - simplified code around sane-behavior mount option parsing
> 4. Restored ACKs from Serge Hallyn from v1 on few patches that have
>    not changed since then.
>
> Changes from V1:
> 1. No pinning of processes within cgroupns. Tasks can be freely moved
>    across cgroups even outside of their cgroupns-root. Usual DAC/MAC policies
>    apply as before.
> 2. Path in /proc/<pid>/cgroup is now always shown and is relative to
>    cgroupns-root. So path can contain '/..' strings depending on cgroupns-root
>    of the reader and cgroup of <pid>.
> 3. setns() does not require the process to first move under target
>    cgroupns-root.
>
> Changes form RFC (V0):
> 1. setns support for cgroupns
> 2. 'mount -t cgroup cgroup <mntpt>' from inside a cgroupns now
>    mounts the cgroup hierarcy with cgroupns-root as the filesystem root.
> 3. writes to cgroup files outside of cgroupns-root are not allowed
> 4. visibility of /proc/<pid>/cgroup is further restricted by not showing
>    anything if the <pid> is in a sibling cgroupns and its cgroup falls outside
>    your cgroupns-root.
>
> ---
>  Documentation/cgroups/namespace.txt | 147 +++++++++++++++++++++++++++
>  fs/kernfs/dir.c                     | 195 ++++++++++++++++++++++++++++++++----
>  fs/kernfs/mount.c                   |  48 +++++++++
>  fs/proc/namespaces.c                |   1 +
>  include/linux/cgroup.h              |  52 +++++++++-
>  include/linux/cgroup_namespace.h    |  36 +++++++
>  include/linux/kernfs.h              |   5 +
>  include/linux/nsproxy.h             |   2 +
>  include/linux/proc_ns.h             |   4 +
>  include/uapi/linux/sched.h          |   3 +-
>  kernel/Makefile                     |   2 +-
>  kernel/cgroup.c                     | 106 +++++++++++++++-----
>  kernel/cgroup_namespace.c           | 140 ++++++++++++++++++++++++++
>  kernel/fork.c                       |   2 +-
>  kernel/nsproxy.c                    |  19 +++-
>  15 files changed, 711 insertions(+), 51 deletions(-)
>  create mode 100644 Documentation/cgroups/namespace.txt
>  create mode 100644 include/linux/cgroup_namespace.h
>  create mode 100644 kernel/cgroup_namespace.c
>
> [PATCHv3 1/8] kernfs: Add API to generate relative kernfs path
> [PATCHv3 2/8] sched: new clone flag CLONE_NEWCGROUP for cgroup
> [PATCHv3 3/8] cgroup: add function to get task's cgroup on default
> [PATCHv3 4/8] cgroup: export cgroup_get() and cgroup_put()
> [PATCHv3 5/8] cgroup: introduce cgroup namespaces
> [PATCHv3 6/8] cgroup: cgroup namespace setns support
> [PATCHv3 7/8] cgroup: mount cgroupns-root when inside non-init cgroupns
> [PATCHv3 8/8] cgroup: Add documentation for cgroup namespaces



-- 
Aditya

^ permalink raw reply

* Re: [PATCH] arch: uapi: asm: mman.h: Let MADV_FREE have same value for all architectures
From: Minchan Kim @ 2014-12-05  6:54 UTC (permalink / raw)
  To: Chen Gang
  Cc: akpm@linux-foundation.org, rth@twiddle.net,
	ink@jurassic.park.msu.ru, mattst88@gmail.com, Ralf Baechle,
	jejb@parisc-linux.org, deller@gmx.de, chris@zankel.net,
	jcmvbkbc@gmail.com, Arnd Bergmann, linux-kernel@vger.kernel.org,
	linux-arch, linux-api
In-Reply-To: <5480E715.3020900@gmail.com>

On Fri, Dec 05, 2014 at 06:58:29AM +0800, Chen Gang wrote:
> For uapi, need try to let all macros have same value, and MADV_FREE is
> added into main branch recently, so need redefine MADV_FREE for it.
> 
> At present, '8' can be shared with all architectures, so redefine it to
> '8'.
> 
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>


Hello Chen,

Thanks for looking at this.
Feel free to add my sign.

Acked-by: Minchan Kim <minchan@kernel.org>

-- 
Kind regards,
Minchan Kim

^ permalink raw reply

* Re: [PATCH v17 1/7] mm: support madvise(MADV_FREE)
From: Minchan Kim @ 2014-12-05  7:08 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Andrew Morton, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Michael Kerrisk,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Hugh Dickins, Johannes Weiner,
	Rik van Riel, KOSAKI Motohiro, Mel Gorman, Jason Evans,
	zhangyanfei-BthXqXjhjHXQFUHtdCDX3A, Kirill A. Shutemov,
	Kirill A. Shutemov
In-Reply-To: <20141203101329.GB23236-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>

On Wed, Dec 03, 2014 at 11:13:29AM +0100, Michal Hocko wrote:
> On Wed 03-12-14 09:00:26, Minchan Kim wrote:
> > On Tue, Dec 02, 2014 at 11:01:25AM +0100, Michal Hocko wrote:
> > > On Mon 01-12-14 08:56:52, Minchan Kim wrote:
> > > [...]
> > > > From 2edd6890f92fa4943ce3c452194479458582d88c Mon Sep 17 00:00:00 2001
> > > > From: Minchan Kim <minchan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > > > Date: Mon, 1 Dec 2014 08:53:55 +0900
> > > > Subject: [PATCH] madvise.2: Document MADV_FREE
> > > > 
> > > > Signed-off-by: Minchan Kim <minchan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > > > ---
> > > >  man2/madvise.2 | 13 +++++++++++++
> > > >  1 file changed, 13 insertions(+)
> > > > 
> > > > diff --git a/man2/madvise.2 b/man2/madvise.2
> > > > index 032ead7..33aa936 100644
> > > > --- a/man2/madvise.2
> > > > +++ b/man2/madvise.2
> > > > @@ -265,6 +265,19 @@ file (see
> > > >  .BR MADV_DODUMP " (since Linux 3.4)"
> > > >  Undo the effect of an earlier
> > > >  .BR MADV_DONTDUMP .
> > > > +.TP
> > > > +.BR MADV_FREE " (since Linux 3.19)"
> > > > +Gives the VM system the freedom to free pages, and tells the system that
> > > > +information in the specified page range is no longer important.
> > > > +This is an efficient way of allowing
> > > > +.BR malloc (3)
> > > 
> > > This might be rather misleading. Only some malloc implementations are
> > > using this feature (jemalloc, right?). So either be specific about which
> > > implementation or do not add it at all.
> > 
> > Make sense. I don't think it's a good idea to say specific example
> > in man page, which is rather arguable and limit the idea.
> > 
> > > 
> > > > +to free pages anywhere in the address space, while keeping the address space
> > > > +valid. The next time that the page is referenced, the page might be demand
> > > > +zeroed, or might contain the data that was there before the MADV_FREE call.
> > > > +References made to that address space range will not make the VM system page the
> > > > +information back in from backing store until the page is modified again.
> > > 
> > > I am not sure I understand the last sentence. So say I did MADV_FREE and
> > > the reclaim has dropped that page. I know that the file backed mappings
> > > are not supported yet but assume they were for a second... Now, I do
> > > read from that location again what is the result?
> > 
> > Zero page.
> 
> OK, it felt strange at first but now that I am thinking about it some
> more it starts making sense. So the semantic is: Either zero page
> (disconnected from the backing store) or the original content after
> madvise(MADV_FREE). The page gets connected to the backing store after
> it gets modified again. If this is the case then the sentence in the man
> page makes perfect sense.
> 
> What made me confused was that I expected file backed pages would get a
> fresh page from the origin but this would be awkward I guess. 
> 
> > > If we consider anon mappings then the backing store is misleading as
> > > well because memory was dropped and so always newly allocated.
> > 
> > When I read the sentence at first, I thought backing store means swap
> > so I don't have any trouble to understand it. But I agree your opinion.
> > Target for man page is not a kernel developer but application developer.
> > 
> > > I would rather drop the whole sentence and rather see an explanation
> > > what is the difference between to MADV_DONT_NEED.
> > > "
> > > Unlike MADV_DONT_NEED the memory is freed lazily e.g. when the VM system
> > > is under memory pressure.
> > > "
> > 
> > It's a good idea but I don't think it's enough. At least we should explan
> > cancel of delay free logic(ie, write). So, How about this?
> > 
> > MADV_FREE " (since Linux 3.19)"
> > 
> > Gives the VM system the freedom to free pages, and tells the system that
> > it's okay to free pages if the VM system has reasons(e.g., memory pressure).
> > So, it looks like delayed MADV_DONTNEED.
> > The next time that the page is referenced, the page might be demand
> > zeroed if the VM system freed the page. Otherwise, it might contain the data
> > that was there before the MADV_FREE call if the VM system didn't free the page.
> > New write in the page after the MADV_FREE call makes the VM system not free
> > the page any more.
> 
> Dunno, I guess the original content was slightly better. Or the
> following wording from UNIX man pages is even more descriptive
> (http://www.lehman.cuny.edu/cgi-bin/man-cgi?madvise+3)
> "
> Tell the kernel that contents in the specified address range are no
> longer important and the range will be overwritten. When there is
> demand for memory, the system will free pages associated with the
> specified address range. In this instance, the next time a page in the
> address range is referenced, it will contain all zeroes.  Otherwise,
> it will con- tain the data that was there prior to the MADV_FREE
> call. References made to the address range will not make the system read
> from backing store (swap space) until the page is modified again.
> 
> This value cannot be used on mappings that have underlying file objects.
> "

For me, it would be better.
Thanks for the heads up.

> 
> I would just clarify the last sentence with addition
> (MAP_PRIVATE|MAP_ANONYMOUS mappings in this implementation). The

I want to be consistent with KSM/THP which used "private anonymous pages".
So, I guess man page maintainer already acked the term so I want to use it,
too.


> difference to MADV_DONTNEED is more complicated now so I wouldn't make
> the text even more confusing.
> 
> Anyway the confusion started on my end so feel free to stick with the
> BSD wording (modulo malloc note which is really confusing as the default
> glibc allocator doesn't do that AFAIK).



>From cfa212d4fb307ae772b08cf564cab7e6adb8f4fc Mon Sep 17 00:00:00 2001
From: Minchan Kim <minchan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Date: Mon, 1 Dec 2014 08:53:55 +0900
Subject: [PATCH] madvise.2: Document MADV_FREE

Signed-off-by: Minchan Kim <minchan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 man2/madvise.2 | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/man2/madvise.2 b/man2/madvise.2
index 032ead7..fc1aaca 100644
--- a/man2/madvise.2
+++ b/man2/madvise.2
@@ -265,6 +265,18 @@ file (see
 .BR MADV_DODUMP " (since Linux 3.4)"
 Undo the effect of an earlier
 .BR MADV_DONTDUMP .
+.TP
+.BR MADV_FREE " (since Linux 3.19)"
+Tell the kernel that contents in the specified address range are no
+longer important and the range will be overwritten. When there is
+demand for memory, the system will free pages associated with the
+specified address range. In this instance, the next time a page in the
+address range is referenced, it will contain all zeroes.  Otherwise,
+it will contain the data that was there prior to the MADV_FREE call.
+References made to the address range will not make the system read
+from backing store (swap space) until the page is modified again.
+It works only with private anonymous pages (see
+.BR mmap (2)).
 .SH RETURN VALUE
 On success
 .BR madvise ()
-- 
2.0.0

-- 
Kind regards,
Minchan Kim

^ permalink raw reply related

* Re: [PATCH v4 0/5] Add Spreadtrum Sharkl64 Platform support
From: Olof Johansson @ 2014-12-05  7:24 UTC (permalink / raw)
  To: Chunyan Zhang
  Cc: gregkh, mark.rutland, arnd, gnomes, broonie, robh+dt, pawel.moll,
	ijc+devicetree, galak, will.deacon, catalin.marinas, jslaby,
	jason, heiko, florian.vaussard, andrew, rrichter, hytszk,
	grant.likely, orsonzhai, geng.ren, zhizhou.zhang, lanqing.liu,
	zhang.lyra, wei.qiao, devicetree, linux-doc, linux-api,
	linux-kernel, arm, linux-serial, sprdlinux, linux-arm-kernel
In-Reply-To: <1417692860-18841-1-git-send-email-chunyan.zhang@spreadtrum.com>

On Thu, Dec 04, 2014 at 07:34:15PM +0800, Chunyan Zhang wrote:
> Spreadtrum is a rapid growing chip vendor providing smart phone total solutions.
> 
> Sharkl64 Platform is nominated as a SoC infrastructure that supports 4G/3G/2G
> standards based on ARMv8 multiple core architecture.Now we have only one
> SoC(SC9836) based on this Platform in developing.
> 
> This patchset adds Sharkl64 support in arm64 device tree and the serial driver
> of SC9836-UART.
> 
> This patchset also has patches which address "sprd" prefix and DT compatible
> strings for nodes which appear un-documented.
> 
> This version code was tesed both on Fast Mode and sc9836-fpga board.
> We use the latest boot-wrapper-aarch64 as the bootloader.

Hi,

We only got 3 of the 5 patches of this version (1, 2 and 4). Can you
resend the dts patch to us (I'm guessing the serial one will go through Greg).


Thanks,

-Olof

^ permalink raw reply

* Re: [PATCH v15 0/3] Add drm driver for Rockchip Socs
From: Dave Airlie @ 2014-12-05  7:26 UTC (permalink / raw)
  To: Daniel Kurtz
  Cc: Heiko Stübner, Mark Yao, Mark Rutland, linux-doc, Kever Yang,
	dri-devel, linux-kernel@vger.kernel.org, linux-api,
	Boris BREZILLON, simon xue, open list:ARM/Rockchip SoC...,
	Grant Likely, Tao Huang, open list:OPEN FIRMWARE AND...,
	Pawel Moll, Ian Campbell, Dominik Behr, 闫孝军,
	Eddie Cai, Rob Herring, John Stultz
In-Reply-To: <CAGS+omAqkQaK99OX_uAf2Z6-fep2Euwn112Oj5gFpRca8ZAgew@mail.gmail.com>

On 5 December 2014 at 05:29, Daniel Kurtz <djkurtz@chromium.org> wrote:
> On Tue, Dec 2, 2014 at 1:31 AM, Heiko Stübner <heiko@sntech.de> wrote:
>> Hi Mark,
>>
>> Am Dienstag, 2. Dezember 2014, 17:13:20 schrieb Mark Yao:
>>> This a series of patches is a DRM Driver for Rockchip Socs, add support
>>> for vop devices. Future patches will add additional encoders/connectors,
>>> such as eDP, HDMI.
>>>
>>> The basic "crtc" for rockchip is a "VOP" - Video Output Processor.
>>> the vop devices found on Rockchip rk3288 Soc, rk3288 soc have two similar
>>> Vop devices. Vop devices support iommu mapping, we use dma-mapping API with
>>> ARM_DMA_USE_IOMMU.
>>
>> [...]
>>
>>> Changes in v15:
>>> - remove depends on ARM_DMA_USE_IOMMU & IOMMU_API which cause
>>>   recursive dependency problem
>
> I thought the recommended solution for this was to fix OMAP3 to not
> select  ARM_DMA_USE_IOMMU and OMAP_IOMMU, not to drop the 'depends on'
> in drm/rockchip?

Lets fix that up once we get this merged, if I had a tree based on the
iommu I'd have
it -next now.

can I get a pull for that?
Dave.

^ permalink raw reply

* Re: [PATCH v6 0/7] vfs: Non-blockling buffered fs read (page cache only)
From: Volker Lendecke @ 2014-12-05  8:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Milosz Tanski, LKML, Christoph Hellwig,
	linux-fsdevel@vger.kernel.org, linux-aio@kvack.org, Mel Gorman,
	Tejun Heo, Jeff Moyer, Theodore Ts'o, Al Viro, Linux API,
	Michael Kerrisk, linux-arch
In-Reply-To: <20141204151102.2d7e11dca39f130c2dff2294@linux-foundation.org>

On Thu, Dec 04, 2014 at 03:11:02PM -0800, Andrew Morton wrote:
> I can see all that, but it's handwaving.  Yes, preadv2() will perform
> better in some circumstances than fincore+pread.  But how much better? 
> Enough to justify this approach, or not?
> 
> Alas, the only way to really settle that is to implement fincore() and
> to subject it to a decent amount of realistic quantitative testing.
> 
> Ho hum.
> 
> Could you please hunt down some libuv developers, see if we can solicit
> some quality input from them?  As I said, we really don't want to merge
> this then find that people don't use it for some reason, or that it
> needs changes.

All I can say from a Samba perspective is that none of the ARM based
Storage boxes I have seen so far do AIO because of the base footprint
for every read. For sequential reads kernel-level readahead could kick
in properly and we should be able to give them the best of both worlds:
No context switches in the default case but also good parallel behaviour
for other workloads.  The most important benchmark for those guys is to
read a DVD image, whether it makes sense or not.

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt@sernet.de

--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>

^ permalink raw reply

* Re: [PATCH v17 1/7] mm: support madvise(MADV_FREE)
From: Michal Hocko @ 2014-12-05  8:32 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, linux-kernel, linux-mm, Michael Kerrisk, linux-api,
	Hugh Dickins, Johannes Weiner, Rik van Riel, KOSAKI Motohiro,
	Mel Gorman, Jason Evans, zhangyanfei, Kirill A. Shutemov,
	Kirill A. Shutemov
In-Reply-To: <20141205070816.GB3358@bbox>

On Fri 05-12-14 16:08:16, Minchan Kim wrote:
[...]
> From cfa212d4fb307ae772b08cf564cab7e6adb8f4fc Mon Sep 17 00:00:00 2001
> From: Minchan Kim <minchan@kernel.org>
> Date: Mon, 1 Dec 2014 08:53:55 +0900
> Subject: [PATCH] madvise.2: Document MADV_FREE
> 
> Signed-off-by: Minchan Kim <minchan@kernel.org>

Reviewed-by: Michal Hocko <mhocko@suse.cz>

Thanks!

> ---
>  man2/madvise.2 | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/man2/madvise.2 b/man2/madvise.2
> index 032ead7..fc1aaca 100644
> --- a/man2/madvise.2
> +++ b/man2/madvise.2
> @@ -265,6 +265,18 @@ file (see
>  .BR MADV_DODUMP " (since Linux 3.4)"
>  Undo the effect of an earlier
>  .BR MADV_DONTDUMP .
> +.TP
> +.BR MADV_FREE " (since Linux 3.19)"
> +Tell the kernel that contents in the specified address range are no
> +longer important and the range will be overwritten. When there is
> +demand for memory, the system will free pages associated with the
> +specified address range. In this instance, the next time a page in the
> +address range is referenced, it will contain all zeroes.  Otherwise,
> +it will contain the data that was there prior to the MADV_FREE call.
> +References made to the address range will not make the system read
> +from backing store (swap space) until the page is modified again.
> +It works only with private anonymous pages (see
> +.BR mmap (2)).
>  .SH RETURN VALUE
>  On success
>  .BR madvise ()
> -- 
> 2.0.0
> 
> -- 
> Kind regards,
> Minchan Kim
> --
> To unsubscribe from this list: send the line "unsubscribe linux-api" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v15 0/3] Add drm driver for Rockchip Socs
From: Heiko Stübner @ 2014-12-05  8:33 UTC (permalink / raw)
  To: Dave Airlie
  Cc: Daniel Kurtz, Mark Yao, Mark Rutland,
	linux-doc-u79uwXL29TY76Z2rM5mHXA, Kever Yang, dri-devel,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Boris BREZILLON, simon xue,
	open list:ARM/Rockchip SoC..., Grant Likely, Tao Huang,
	open list:OPEN FIRMWARE AND..., Pawel Moll, Ian Campbell,
	Dominik Behr, 闫孝军, Eddie Cai, Rob Herring,
	John Stultz
In-Reply-To: <CAPM=9tzcB0XaenK1RjJoOBpJjrq84atJP59=kh6WMA2-OcFpEA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Dave,

Am Freitag, 5. Dezember 2014, 17:26:04 schrieb Dave Airlie:
> On 5 December 2014 at 05:29, Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> > I thought the recommended solution for this was to fix OMAP3 to not
> > select  ARM_DMA_USE_IOMMU and OMAP_IOMMU, not to drop the 'depends on'
> > in drm/rockchip?
> 
> Lets fix that up once we get this merged, if I had a tree based on the
> iommu I'd have
> it -next now.
> 
> can I get a pull for that?

Mark sent a pull on tuesday as reply to the cover-letter.
Message-ID: <547D88D7.5080005-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

Replicated here, if you don't want to search.


Heiko

---------------- 8< -------------------------------------

Hi Dave

The following changes since commit 656d7077d8ffd1c2492d4a0a354367ab2e545059:

   dt-bindings: iommu: Add documentation for rockchip iommu (2014-11-03 
17:29:09 +0100)

are available in the git repository at:

   https://github.com/markyzq/kernel-drm-rockchip.git drm_iommu_v15

for you to fetch changes up to 5ac4837b12f533de5d9f8f66b45494c58e805536:

   dt-bindings: video: Add documentation for rockchip vop (2014-12-02 
17:29:33 +0800)

----------------------------------------------------------------
Mark Yao (3):
       drm: rockchip: Add basic drm driver
       dt-bindings: video: Add for rockchip display subsytem
       dt-bindings: video: Add documentation for rockchip vop

  .../devicetree/bindings/video/rockchip-drm.txt     |   19 +
  .../devicetree/bindings/video/rockchip-vop.txt     |   58 +
  drivers/gpu/drm/Kconfig                            |    2 +
  drivers/gpu/drm/Makefile                           |    1 +
  drivers/gpu/drm/rockchip/Kconfig                   |   17 +
  drivers/gpu/drm/rockchip/Makefile                  |    8 +
  drivers/gpu/drm/rockchip/rockchip_drm_drv.c        |  551 ++++++++
  drivers/gpu/drm/rockchip/rockchip_drm_drv.h        |   68 +
  drivers/gpu/drm/rockchip/rockchip_drm_fb.c         |  201 +++
  drivers/gpu/drm/rockchip/rockchip_drm_fb.h         |   28 +
  drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c      |  210 +++
  drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h      |   21 +
  drivers/gpu/drm/rockchip/rockchip_drm_gem.c        |  294 ++++
  drivers/gpu/drm/rockchip/rockchip_drm_gem.h        |   54 +
  drivers/gpu/drm/rockchip/rockchip_drm_vop.c        | 1455 ++++++++++++++++++++
  drivers/gpu/drm/rockchip/rockchip_drm_vop.h        |  201 +++
  16 files changed, 3188 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/video/rockchip-drm.txt
  create mode 100644 Documentation/devicetree/bindings/video/rockchip-vop.txt
  create mode 100644 drivers/gpu/drm/rockchip/Kconfig
  create mode 100644 drivers/gpu/drm/rockchip/Makefile
  create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_drv.c
  create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_drv.h
  create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fb.c
  create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fb.h
  create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c
  create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h
  create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_gem.c
  create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_gem.h
  create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_vop.c
  create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_vop.h

^ permalink raw reply

* Re: [PATCH v2 00/13] Add kdbus implementation
From: Hillf Danton @ 2014-12-05  8:49 UTC (permalink / raw)
  To: 'Greg Kroah-Hartman'
  Cc: arnd-r2nGTMty4D4, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io, teg-B22kvLQNl6c,
	jkosina-AlSwsSmVLrQ, luto-kltTT9wpgjJwATOyAt5JVQ,
	linux-api-u79uwXL29TY76Z2rM5mHXA, linux-kernel,
	daniel-cYrQPVfZoowdnm+yROfE0A, dh.herrmann-Re5JQEeQqe8AvxtiuMwx3w,
	tixxdz-Umm1ozX2/EEdnm+yROfE0A, Hillf Danton,
	赵东(辅周)

Hey all
> 
> kdbus is a kernel-level IPC implementation that aims for resemblance to
> the the protocol layer with the existing userspace D-Bus daemon while
> enabling some features that couldn't be implemented before in userspace.
> 
[...]
> 
> This can also be found in a git tree, the kdbus branch of char-misc.git at:
>         https://git.kernel.org/cgit/linux/kernel/git/gregkh/char-misc.git/
> 
In the environment:
	Android Kitkat
	Linux-3.4.67
	CPU MTK MT6582 ARMv7 Processor rev 3 (v7l)

root@cwet_td_a800:/ # kdbus-test 
Testing bus make functions (bus-make) .................................. OK
Testing the HELLO command (hello) ...................................... OK
Testing the BYEBYE command (byebye) .................................... OK
Testing a chat pattern (chat) .......................................... OK
Testing a simple dameon (daemon) ....................................... OK
Testing file descriptor passing (fd-passing) ........................... OK
Testing custom endpoint (endpoint) ..................................... OK
Testing monitor functionality (monitor) ................................ OK
Testing basic name registry functions (name-basics) .................... OK
Testing name registry conflict details (name-conflict) ................. OK
Testing queuing of names (name-queue) .................................. OK
Testing basic message handling (message-basic) ......................... OK
Testing handling of messages with priority (message-prio) .............. OK
Testing message quotas are enforced (message-quota) .................... OK
Testing timeout (timeout) .............................................. OK
Testing synchronous replies vs. BYEBYE (sync-byebye) ................... OK
Testing synchronous replies (sync-reply) ............................... OK
Testing freeing of memory (message-free) ............................... OK
Testing retrieving connection information (connection-info) ............ OK
Testing updating connection information (connection-update) ............ OK
Testing verifying pools are never writable (writable-pool) ............. OK
Testing policy (policy) ................................................ OK
Testing unprivileged bus access (policy-priv) .......................... OK
Testing policy in user namespaces (policy-ns) .......................... OK
Testing metadata in user namespaces (metadata-ns) ...................... OK
Testing adding of matches by id (match-id-add) ......................... OK
Testing removing of matches by id (match-id-remove) .................... OK
Testing replace of matches with the same cookie (match-replace) ........ OK
Testing adding of matches by name (match-name-add) ..................... OK
Testing removing of matches by name (match-name-remove) ................ OK
Testing matching for name changes (match-name-change) .................. OK
Testing matching with bloom filters (match-bloom) ...................... OK
Testing activator connections (activator) .............................. OK
Testing benchmark (benchmark) .......................................... OK
Testing race multiple byebyes (race-byebye) ............................ OK
Testing race byebye vs match removal (race-byebye-match) ............... OK

SUMMARY: 36 tests passed, 0 skipped, 0 failed


And we like to test newer versions, if any, of kdbus with our phone.

Thanks
Hillf

^ permalink raw reply

* Re: [PATCH v4 0/5] Add Spreadtrum Sharkl64 Platform support
From: Mark Rutland @ 2014-12-05 10:40 UTC (permalink / raw)
  To: Chunyan Zhang
  Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
	arnd-r2nGTMty4D4@public.gmane.org,
	gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org,
	broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Pawel Moll,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org,
	galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org, Will Deacon,
	Catalin Marinas, jslaby-AlSwsSmVLrQ@public.gmane.org,
	jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org,
	heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org,
	florian.vaussard-p8DiymsW2f8@public.gmane.org,
	andrew-g2DYL2Zd6BY@public.gmane.org,
	rrichter-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org,
	hytszk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	orsonzhai-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	geng.ren-lxIno14LUO0EEoCn2XhGlw
In-Reply-To: <1417692860-18841-1-git-send-email-chunyan.zhang-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>

Hi,

On Thu, Dec 04, 2014 at 11:34:15AM +0000, Chunyan Zhang wrote:
> Spreadtrum is a rapid growing chip vendor providing smart phone total solutions.
> 
> Sharkl64 Platform is nominated as a SoC infrastructure that supports 4G/3G/2G
> standards based on ARMv8 multiple core architecture.Now we have only one
> SoC(SC9836) based on this Platform in developing.
> 
> This patchset adds Sharkl64 support in arm64 device tree and the serial driver
> of SC9836-UART.
> 
> This patchset also has patches which address "sprd" prefix and DT compatible
> strings for nodes which appear un-documented.
> 
> This version code was tesed both on Fast Mode and sc9836-fpga board.
> We use the latest boot-wrapper-aarch64 as the bootloader.
> 
> Changes from v3:
> * Addressed review comments:
> 	- Added the description of clock property for sc9836-uart
> 	- Revised the size of GICC to be 8KiB
> 	- Added another compatible string for psci-0.1

I had open questions on v3 regarding your PSCI imlpementation. You
mentioned that you are using the aarch64 bootwrapper, but your DT
describes PSCI 0.2, and the (upstream) bootwrapper does not implement
PSCI 0.2. Adding the old PSCI compatible string is _not_ sufficient if
you do not have a full PSCI 0.2 implementation.

Given that PSCI 0.2 requires more functionality to be implemented, I'd
like to know that your implementation is spec-compliant (implementing
the mandatory functions, nters the kernel in the correct state, etc),
and that it has been tested.

Would you be able to look at my comments from the last posting please?

Thanks,
Mark.

^ permalink raw reply

* Re: [PATCH v6 4/7] fs/fuse: support compiling out splice
From: Miklos Szeredi @ 2014-12-05 10:44 UTC (permalink / raw)
  To: Pieter Smith
  Cc: Kernel Mailing List, open list:FUSE: FILESYSTEM...,
	open list:ABI/API, Linux-Fsdevel
In-Reply-To: <1417715473-24110-5-git-send-email-pieter-qeJ+1H9vRZbz+pZb47iToQ@public.gmane.org>

On Thu, Dec 4, 2014 at 6:50 PM, Pieter Smith <pieter-qeJ+1H9vRZbz+pZb47iToQ@public.gmane.org> wrote:
> To implement splice support, fs/fuse makes use of nosteal_pipe_buf_ops. This
> struct is exported by fs/splice. The goal of the larger patch set is to
> completely compile out fs/splice, so uses of the exported struct need to be
> compiled out along with fs/splice.
>
> This patch therefore compiles out splice support in fs/fuse when
> CONFIG_SYSCALL_SPLICE is undefined.
>
> Signed-off-by: Pieter Smith <pieter-qeJ+1H9vRZbz+pZb47iToQ@public.gmane.org>


In the future could you PLEASE PLEASE cut the fuse-devel Cc from the
non-fuse specific patches (and I guess that goes for any other
subsystem specific lists and persons as well)?

Otherwise:

Acked-by: Miklos Szeredi <mszeredi-AlSwsSmVLrQ@public.gmane.org>

Thanks,
Miklos

^ permalink raw reply

* Re: [PATCH v3] media: platform: add VPFE capture driver support for AM437X
From: Hans Verkuil @ 2014-12-05 12:24 UTC (permalink / raw)
  To: Lad, Prabhakar, LMML, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-api
  Cc: LKML, Hans Verkuil
In-Reply-To: <1417648378-18271-1-git-send-email-prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Hi Prabhakar,

Sorry, there are still a few items that need to be fixed.
If you can make a v4 with these issues addressed, then I can still make a
pull request, although it depends on Mauro whether it is still accepted for
3.19.

On 12/04/2014 12:12 AM, Lad, Prabhakar wrote:
> From: Benoit Parrot <bparrot-l0cyMroinI0@public.gmane.org>
> 
> This patch adds Video Processing Front End (VPFE) driver for
> AM437X family of devices
> Driver supports the following:
> - V4L2 API using MMAP buffer access based on videobuf2 api
> - Asynchronous sensor/decoder sub device registration
> - DT support

Just to confirm: this driver only supports SDTV formats? No HDTV?
I didn't see any VIDIOC_*_DV_TIMINGS support, so I assume it really
isn't supported.

> 
> Signed-off-by: Benoit Parrot <bparrot-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Darren Etheridge <detheridge-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Lad, Prabhakar <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---

<snip>

> diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c
> new file mode 100644
> index 0000000..25863e8
> --- /dev/null
> +++ b/drivers/media/platform/am437x/am437x-vpfe.c

<snip>

> +
> +static int
> +cmp_v4l2_format(const struct v4l2_format *lhs, const struct v4l2_format *rhs)
> +{
> +	return lhs->type == rhs->type &&
> +		lhs->fmt.pix.width == rhs->fmt.pix.width &&
> +		lhs->fmt.pix.height == rhs->fmt.pix.height &&
> +		lhs->fmt.pix.pixelformat == rhs->fmt.pix.pixelformat &&
> +		lhs->fmt.pix.field == rhs->fmt.pix.field &&
> +		lhs->fmt.pix.colorspace == rhs->fmt.pix.colorspace;

Add a check for pix.ycbcr_enc and pix.quantization.

<snip>

> +/*
> + * vpfe_release : This function is based on the vb2_fop_release
> + * helper function.
> + * It has been augmented to handle module power management,
> + * by disabling/enabling h/w module fcntl clock when necessary.
> + */
> +static int vpfe_release(struct file *file)
> +{
> +	struct vpfe_device *vpfe = video_drvdata(file);
> +	int ret;
> +
> +	vpfe_dbg(2, vpfe, "vpfe_release\n");
> +
> +	ret = _vb2_fop_release(file, NULL);

This isn't going to work. _vb2_fop_release calls v4l2_fh_release(), so
the v4l2_fh_is_singular_file(file) will be wrong and you release the fh
once too many.

I would do this:

	if (!v4l2_fh_is_singular_file(file))
		return vb2_fop_release(file);
	mutex_lock(&vpfe->lock);
	ret = _vb2_fop_release(file, NULL);
	vpfe_ccdc_close(&vpfe->ccdc, vpfe->pdev);
	mutex_unlock(&vpfe->lock);
	return ret;

> +
> +	if (v4l2_fh_is_singular_file(file)) {
> +		mutex_lock(&vpfe->lock);
> +		vpfe_ccdc_close(&vpfe->ccdc, vpfe->pdev);
> +		v4l2_fh_release(file);
> +		mutex_unlock(&vpfe->lock);
> +	}
> +
> +	return ret;
> +}

<snip>

> +static int vpfe_enum_size(struct file *file, void  *priv,
> +			  struct v4l2_frmsizeenum *fsize)
> +{
> +	struct vpfe_device *vpfe = video_drvdata(file);
> +	struct v4l2_subdev_frame_size_enum fse;
> +	struct vpfe_subdev_info *sdinfo;
> +	struct v4l2_mbus_framefmt mbus;
> +	struct v4l2_pix_format pix;
> +	struct vpfe_fmt *fmt;
> +	int ret;
> +
> +	vpfe_dbg(2, vpfe, "vpfe_enum_size\n");
> +
> +	/* check for valid format */
> +	fmt = find_format_by_pix(fsize->pixel_format);
> +	if (!fmt) {
> +		vpfe_dbg(3, vpfe, "Invalid pixel code: %x, default used instead\n",
> +			fsize->pixel_format);
> +		return -EINVAL;
> +	}
> +
> +	memset(fsize->reserved, 0x0, sizeof(fsize->reserved));
> +
> +	sdinfo = vpfe->current_subdev;
> +	if (!sdinfo->sd)
> +		return -EINVAL;
> +
> +	memset(&pix, 0x0, sizeof(pix));
> +	/* Construct pix from parameter and use default for the rest */
> +	pix.pixelformat = fsize->pixel_format;
> +	pix.width = 640;
> +	pix.height = 480;
> +	pix.colorspace = V4L2_COLORSPACE_SRGB;
> +	pix.field = V4L2_FIELD_NONE;
> +	pix_to_mbus(vpfe, &pix, &mbus);
> +
> +	memset(&fse, 0x0, sizeof(fse));
> +	fse.index = fsize->index;
> +	fse.pad = 0;
> +	fse.code = mbus.code;
> +	ret = v4l2_subdev_call(sdinfo->sd, pad, enum_frame_size, NULL, &fse);

FYI: strictly speaking this is wrong since this op theoretically expects a
v4l2_subdev_fh pointer instead of a NULL argument. However, you do not have
an alternative right now. As you know, I've been working on fixing this, so
if that gets accepted, then you need to update this code as well in a later
patch.

> +	if (ret)
> +		return -EINVAL;
> +
> +	vpfe_dbg(1, vpfe, "vpfe_enum_size: index: %d code: %x W:[%d,%d] H:[%d,%d]\n",
> +		fse.index, fse.code, fse.min_width, fse.max_width,
> +		fse.min_height, fse.max_height);
> +
> +	fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
> +	fsize->discrete.width = fse.max_width;
> +	fsize->discrete.height = fse.max_height;
> +
> +	vpfe_dbg(1, vpfe, "vpfe_enum_size: index: %d pixformat: %s size: %dx%d\n",
> +		fsize->index, print_fourcc(fsize->pixel_format),
> +		fsize->discrete.width, fsize->discrete.height);
> +
> +	return 0;
> +}
> +

<snip>

> +static int
> +vpfe_g_selection(struct file *file, void *fh, struct v4l2_selection *s)
> +{
> +	struct vpfe_device *vpfe = video_drvdata(file);
> +
> +	switch (s->target) {
> +	case V4L2_SEL_TGT_COMPOSE_DEFAULT:
> +	case V4L2_SEL_TGT_COMPOSE_BOUNDS:

These two COMPOSE cases should be dropped, since there is no compose support!

> +	case V4L2_SEL_TGT_CROP_BOUNDS:
> +	case V4L2_SEL_TGT_CROP_DEFAULT:
> +		s->r.left = s->r.top = 0;
> +		s->r.width = vpfe->crop.width;
> +		s->r.height = vpfe->crop.height;
> +		break;
> +
> +	case V4L2_SEL_TGT_CROP:
> +		s->r = vpfe->crop;
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}

<snip>

Regards,

	Hans

^ permalink raw reply

* Re: [PATCH v4 0/5] Add Spreadtrum Sharkl64 Platform support
From: Lyra Zhang @ 2014-12-05 12:27 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Chunyan Zhang,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
	arnd-r2nGTMty4D4@public.gmane.org,
	gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org,
	broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Pawel Moll,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org,
	galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org, Will Deacon,
	Catalin Marinas, jslaby-AlSwsSmVLrQ@public.gmane.org,
	jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org,
	heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org,
	florian.vaussard-p8DiymsW2f8@public.gmane.org,
	andrew-g2DYL2Zd6BY@public.gmane.org,
	rrichter-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org,
	hytszk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org
In-Reply-To: <20141205104009.GE11889@leverpostej>

On Fri, Dec 5, 2014 at 6:40 PM, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
> Hi,
>
> On Thu, Dec 04, 2014 at 11:34:15AM +0000, Chunyan Zhang wrote:
>> Spreadtrum is a rapid growing chip vendor providing smart phone total solutions.
>>
>> Sharkl64 Platform is nominated as a SoC infrastructure that supports 4G/3G/2G
>> standards based on ARMv8 multiple core architecture.Now we have only one
>> SoC(SC9836) based on this Platform in developing.
>>
>> This patchset adds Sharkl64 support in arm64 device tree and the serial driver
>> of SC9836-UART.
>>
>> This patchset also has patches which address "sprd" prefix and DT compatible
>> strings for nodes which appear un-documented.
>>
>> This version code was tesed both on Fast Mode and sc9836-fpga board.
>> We use the latest boot-wrapper-aarch64 as the bootloader.
>>
>> Changes from v3:
>> * Addressed review comments:
>>       - Added the description of clock property for sc9836-uart
>>       - Revised the size of GICC to be 8KiB
>>       - Added another compatible string for psci-0.1
>
> I had open questions on v3 regarding your PSCI imlpementation. You
> mentioned that you are using the aarch64 bootwrapper, but your DT
> describes PSCI 0.2, and the (upstream) bootwrapper does not implement
> PSCI 0.2. Adding the old PSCI compatible string is _not_ sufficient if
> you do not have a full PSCI 0.2 implementation.
>
> Given that PSCI 0.2 requires more functionality to be implemented, I'd
> like to know that your implementation is spec-compliant (implementing
> the mandatory functions, nters the kernel in the correct state, etc),
> and that it has been tested.
>
> Would you be able to look at my comments from the last posting please?
>
> Thanks,
> Mark.
> --

Hi, Mark

Ok, I'll check it again with our related engineers.

Actually, I had read all of your comments carefully before sending
each version of patches, and I replied you a few days early, I guess
you may miss it :)

If we just implemented psci-0.1 until now, can we submit this path
without "compatible = "arm,psci-0.2"", but only with " compatible =
"arm,psci" ".



Thanks,
Chunyan

^ permalink raw reply

* Re: [PATCH v3] media: platform: add VPFE capture driver support for AM437X
From: Prabhakar Lad @ 2014-12-05 12:54 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: LMML, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-api, LKML, Hans Verkuil
In-Reply-To: <5481A3EC.8020803-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>

Hi Hans,

On Fri, Dec 5, 2014 at 12:24 PM, Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> wrote:
> Hi Prabhakar,
>
> Sorry, there are still a few items that need to be fixed.
> If you can make a v4 with these issues addressed, then I can still make a
> pull request, although it depends on Mauro whether it is still accepted for
> 3.19.
>
OK will post a v4 tonight fixing all the below issues.

FYI: Looking at the response of Mauro on 'soc-camera: 1st set for 3.19'
he wont accept it!

Thanks,
--Prabhakar Lad

> On 12/04/2014 12:12 AM, Lad, Prabhakar wrote:
>> From: Benoit Parrot <bparrot-l0cyMroinI0@public.gmane.org>
>>
>> This patch adds Video Processing Front End (VPFE) driver for
>> AM437X family of devices
>> Driver supports the following:
>> - V4L2 API using MMAP buffer access based on videobuf2 api
>> - Asynchronous sensor/decoder sub device registration
>> - DT support
>
> Just to confirm: this driver only supports SDTV formats? No HDTV?
> I didn't see any VIDIOC_*_DV_TIMINGS support, so I assume it really
> isn't supported.
>
>>
>> Signed-off-by: Benoit Parrot <bparrot-l0cyMroinI0@public.gmane.org>
>> Signed-off-by: Darren Etheridge <detheridge-l0cyMroinI0@public.gmane.org>
>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>
> <snip>
>
>> diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c
>> new file mode 100644
>> index 0000000..25863e8
>> --- /dev/null
>> +++ b/drivers/media/platform/am437x/am437x-vpfe.c
>
> <snip>
>
>> +
>> +static int
>> +cmp_v4l2_format(const struct v4l2_format *lhs, const struct v4l2_format *rhs)
>> +{
>> +     return lhs->type == rhs->type &&
>> +             lhs->fmt.pix.width == rhs->fmt.pix.width &&
>> +             lhs->fmt.pix.height == rhs->fmt.pix.height &&
>> +             lhs->fmt.pix.pixelformat == rhs->fmt.pix.pixelformat &&
>> +             lhs->fmt.pix.field == rhs->fmt.pix.field &&
>> +             lhs->fmt.pix.colorspace == rhs->fmt.pix.colorspace;
>
> Add a check for pix.ycbcr_enc and pix.quantization.
>
OK

> <snip>
>
>> +/*
>> + * vpfe_release : This function is based on the vb2_fop_release
>> + * helper function.
>> + * It has been augmented to handle module power management,
>> + * by disabling/enabling h/w module fcntl clock when necessary.
>> + */
>> +static int vpfe_release(struct file *file)
>> +{
>> +     struct vpfe_device *vpfe = video_drvdata(file);
>> +     int ret;
>> +
>> +     vpfe_dbg(2, vpfe, "vpfe_release\n");
>> +
>> +     ret = _vb2_fop_release(file, NULL);
>
> This isn't going to work. _vb2_fop_release calls v4l2_fh_release(), so
> the v4l2_fh_is_singular_file(file) will be wrong and you release the fh
> once too many.
>
> I would do this:
>
>         if (!v4l2_fh_is_singular_file(file))
>                 return vb2_fop_release(file);
>         mutex_lock(&vpfe->lock);
>         ret = _vb2_fop_release(file, NULL);
>         vpfe_ccdc_close(&vpfe->ccdc, vpfe->pdev);
>         mutex_unlock(&vpfe->lock);
>         return ret;
>
>> +
>> +     if (v4l2_fh_is_singular_file(file)) {
>> +             mutex_lock(&vpfe->lock);
>> +             vpfe_ccdc_close(&vpfe->ccdc, vpfe->pdev);
>> +             v4l2_fh_release(file);
>> +             mutex_unlock(&vpfe->lock);
>> +     }
>> +
>> +     return ret;
>> +}
>
> <snip>
>
>> +static int vpfe_enum_size(struct file *file, void  *priv,
>> +                       struct v4l2_frmsizeenum *fsize)
>> +{
>> +     struct vpfe_device *vpfe = video_drvdata(file);
>> +     struct v4l2_subdev_frame_size_enum fse;
>> +     struct vpfe_subdev_info *sdinfo;
>> +     struct v4l2_mbus_framefmt mbus;
>> +     struct v4l2_pix_format pix;
>> +     struct vpfe_fmt *fmt;
>> +     int ret;
>> +
>> +     vpfe_dbg(2, vpfe, "vpfe_enum_size\n");
>> +
>> +     /* check for valid format */
>> +     fmt = find_format_by_pix(fsize->pixel_format);
>> +     if (!fmt) {
>> +             vpfe_dbg(3, vpfe, "Invalid pixel code: %x, default used instead\n",
>> +                     fsize->pixel_format);
>> +             return -EINVAL;
>> +     }
>> +
>> +     memset(fsize->reserved, 0x0, sizeof(fsize->reserved));
>> +
>> +     sdinfo = vpfe->current_subdev;
>> +     if (!sdinfo->sd)
>> +             return -EINVAL;
>> +
>> +     memset(&pix, 0x0, sizeof(pix));
>> +     /* Construct pix from parameter and use default for the rest */
>> +     pix.pixelformat = fsize->pixel_format;
>> +     pix.width = 640;
>> +     pix.height = 480;
>> +     pix.colorspace = V4L2_COLORSPACE_SRGB;
>> +     pix.field = V4L2_FIELD_NONE;
>> +     pix_to_mbus(vpfe, &pix, &mbus);
>> +
>> +     memset(&fse, 0x0, sizeof(fse));
>> +     fse.index = fsize->index;
>> +     fse.pad = 0;
>> +     fse.code = mbus.code;
>> +     ret = v4l2_subdev_call(sdinfo->sd, pad, enum_frame_size, NULL, &fse);
>
> FYI: strictly speaking this is wrong since this op theoretically expects a
> v4l2_subdev_fh pointer instead of a NULL argument. However, you do not have
> an alternative right now. As you know, I've been working on fixing this, so
> if that gets accepted, then you need to update this code as well in a later
> patch.
>
>> +     if (ret)
>> +             return -EINVAL;
>> +
>> +     vpfe_dbg(1, vpfe, "vpfe_enum_size: index: %d code: %x W:[%d,%d] H:[%d,%d]\n",
>> +             fse.index, fse.code, fse.min_width, fse.max_width,
>> +             fse.min_height, fse.max_height);
>> +
>> +     fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
>> +     fsize->discrete.width = fse.max_width;
>> +     fsize->discrete.height = fse.max_height;
>> +
>> +     vpfe_dbg(1, vpfe, "vpfe_enum_size: index: %d pixformat: %s size: %dx%d\n",
>> +             fsize->index, print_fourcc(fsize->pixel_format),
>> +             fsize->discrete.width, fsize->discrete.height);
>> +
>> +     return 0;
>> +}
>> +
>
> <snip>
>
>> +static int
>> +vpfe_g_selection(struct file *file, void *fh, struct v4l2_selection *s)
>> +{
>> +     struct vpfe_device *vpfe = video_drvdata(file);
>> +
>> +     switch (s->target) {
>> +     case V4L2_SEL_TGT_COMPOSE_DEFAULT:
>> +     case V4L2_SEL_TGT_COMPOSE_BOUNDS:
>
> These two COMPOSE cases should be dropped, since there is no compose support!
>
>> +     case V4L2_SEL_TGT_CROP_BOUNDS:
>> +     case V4L2_SEL_TGT_CROP_DEFAULT:
>> +             s->r.left = s->r.top = 0;
>> +             s->r.width = vpfe->crop.width;
>> +             s->r.height = vpfe->crop.height;
>> +             break;
>> +
>> +     case V4L2_SEL_TGT_CROP:
>> +             s->r = vpfe->crop;
>> +             break;
>> +
>> +     default:
>> +             return -EINVAL;
>> +     }
>> +
>> +     return 0;
>> +}
>
> <snip>
>
> Regards,
>
>         Hans

^ permalink raw reply

* Re: [tpmdd-devel] [PATCH v9 8/8] tpm: TPM 2.0 FIFO Interface
From: Jarkko Sakkinen @ 2014-12-05 13:05 UTC (permalink / raw)
  To: Scot Doyle
  Cc: Peter Huewe, Ashley Lai, Marcel Selhorst,
	christophe.ricard-Re5JQEeQqe8AvxtiuMwx3w,
	josh.triplett-ral2JQCrhuEAvxtiuMwx3w,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Will Arthur,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	jason.gunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
	trousers-tech-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <alpine.DEB.2.11.1412041459470.5705-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>

On Thu, Dec 04, 2014 at 03:25:12PM +0000, Scot Doyle wrote:
> Hi Jarkko,
> 
> On Thu, 4 Dec 2014, Jarkko Sakkinen wrote:
> > From: Will Arthur <will.c.arthur-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > 
> > Detect TPM 2.0 by using the extended STS (STS3) register. For TPM 2.0,
> > instead of calling tpm_get_timeouts(), assign duration and timeout
> > values defined in the TPM 2.0 PTP specification.
> > 
> > Signed-off-by: Will Arthur <will.c.arthur-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > ---
> >  drivers/char/tpm/tpm_tis.c | 80 ++++++++++++++++++++++++++++++++++++++--------
> >  1 file changed, 67 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> > index 89e1abb..71cbb2d 100644
> > --- a/drivers/char/tpm/tpm_tis.c
> > +++ b/drivers/char/tpm/tpm_tis.c
> > @@ -1,5 +1,6 @@
> >  /*
> >   * Copyright (C) 2005, 2006 IBM Corporation
> > + * Copyright (C) 2014 Intel Corporation
> >   *
> >   * Authors:
> >   * Leendert van Doorn <leendert-aZOuKsOsJu3MbYB6QlFGEg@public.gmane.org>
> > @@ -44,6 +45,10 @@ enum tis_status {
> >  	TPM_STS_DATA_EXPECT = 0x08,
> >  };
> >  
> > +enum tis_status3 {
> > +	TPM_STS3_TPM2_FAM = 0x04,
> > +};
> > +
> >  enum tis_int_flags {
> >  	TPM_GLOBAL_INT_ENABLE = 0x80000000,
> >  	TPM_INTF_BURST_COUNT_STATIC = 0x100,
> > @@ -70,6 +75,7 @@ enum tis_defaults {
> >  #define	TPM_INT_STATUS(l)		(0x0010 | ((l) << 12))
> >  #define	TPM_INTF_CAPS(l)		(0x0014 | ((l) << 12))
> >  #define	TPM_STS(l)			(0x0018 | ((l) << 12))
> > +#define	TPM_STS3(l)			(0x001b | ((l) << 12))
> >  #define	TPM_DATA_FIFO(l)		(0x0024 | ((l) << 12))
> >  
> >  #define	TPM_DID_VID(l)			(0x0F00 | ((l) << 12))
> > @@ -363,6 +369,7 @@ static int tpm_tis_send_main(struct tpm_chip *chip, u8 *buf, size_t len)
> >  {
> >  	int rc;
> >  	u32 ordinal;
> > +	unsigned long dur;
> >  
> >  	rc = tpm_tis_send_data(chip, buf, len);
> >  	if (rc < 0)
> > @@ -374,9 +381,14 @@ static int tpm_tis_send_main(struct tpm_chip *chip, u8 *buf, size_t len)
> >  
> >  	if (chip->vendor.irq) {
> >  		ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
> > +
> > +		if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > +			dur = tpm2_calc_ordinal_duration(chip, ordinal);
> > +		else
> > +			dur = tpm_calc_ordinal_duration(chip, ordinal);
> > +
> >  		if (wait_for_tpm_stat
> > -		    (chip, TPM_STS_DATA_AVAIL | TPM_STS_VALID,
> > -		     tpm_calc_ordinal_duration(chip, ordinal),
> > +		    (chip, TPM_STS_DATA_AVAIL | TPM_STS_VALID, dur,
> >  		     &chip->vendor.read_queue, false) < 0) {
> >  			rc = -ETIME;
> >  			goto out_err;
> > @@ -588,6 +600,7 @@ static int tpm_tis_init(struct device *dev, acpi_handle acpi_dev_handle,
> >  	int rc, i, irq_s, irq_e, probe;
> >  	struct tpm_chip *chip;
> >  	struct priv_data *priv;
> > +	u8 sts3;
> >  
> >  	priv = devm_kzalloc(dev, sizeof(struct priv_data), GFP_KERNEL);
> >  	if (priv == NULL)
> > @@ -604,11 +617,28 @@ static int tpm_tis_init(struct device *dev, acpi_handle acpi_dev_handle,
> >  	if (!chip->vendor.iobase)
> >  		return -EIO;
> >  
> > +	sts3 = ioread8(chip->vendor.iobase + TPM_STS3(1));
> > +	if ((sts3 & TPM_STS3_TPM2_FAM) == TPM_STS3_TPM2_FAM)
> > +		chip->flags = TPM_CHIP_FLAG_TPM2;
> > +
> >  	/* Default timeouts */
> > -	chip->vendor.timeout_a = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
> > -	chip->vendor.timeout_b = msecs_to_jiffies(TIS_LONG_TIMEOUT);
> > -	chip->vendor.timeout_c = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
> > -	chip->vendor.timeout_d = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
> > +	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> > +		chip->vendor.timeout_a = usecs_to_jiffies(TPM2_TIMEOUT_A);
> > +		chip->vendor.timeout_b = usecs_to_jiffies(TPM2_TIMEOUT_B);
> > +		chip->vendor.timeout_c = usecs_to_jiffies(TPM2_TIMEOUT_C);
> > +		chip->vendor.timeout_d = usecs_to_jiffies(TPM2_TIMEOUT_D);
> > +		chip->vendor.duration[TPM_SHORT] =
> > +			usecs_to_jiffies(TPM2_DURATION_SHORT);
> > +		chip->vendor.duration[TPM_MEDIUM] =
> > +			usecs_to_jiffies(TPM2_DURATION_MEDIUM);
> > +		chip->vendor.duration[TPM_LONG] =
> > +			usecs_to_jiffies(TPM2_DURATION_LONG);
> > +	} else {
> > +		chip->vendor.timeout_a = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
> > +		chip->vendor.timeout_b = msecs_to_jiffies(TIS_LONG_TIMEOUT);
> > +		chip->vendor.timeout_c = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
> > +		chip->vendor.timeout_d = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
> > +	}
> >  
> >  	if (wait_startup(chip, 0) != 0) {
> >  		rc = -ENODEV;
> > @@ -623,8 +653,8 @@ static int tpm_tis_init(struct device *dev, acpi_handle acpi_dev_handle,
> >  	vendor = ioread32(chip->vendor.iobase + TPM_DID_VID(0));
> >  	chip->vendor.manufacturer_id = vendor;
> >  
> > -	dev_info(dev,
> > -		 "1.2 TPM (device-id 0x%X, rev-id %d)\n",
> > +	dev_info(dev, "%s TPM (device-id 0x%X, rev-id %d)\n",
> > +		 (chip->flags & TPM_CHIP_FLAG_TPM2) ? "2.0" : "1.2",
> >  		 vendor >> 16, ioread8(chip->vendor.iobase + TPM_RID(0)));
> >  
> >  	if (!itpm) {
> > @@ -665,6 +695,23 @@ static int tpm_tis_init(struct device *dev, acpi_handle acpi_dev_handle,
> >  	if (intfcaps & TPM_INTF_DATA_AVAIL_INT)
> >  		dev_dbg(dev, "\tData Avail Int Support\n");
> >  
> > +	/* get the timeouts before testing for irqs */
> > +	if (!(chip->flags & TPM_CHIP_FLAG_TPM2) && tpm_get_timeouts(chip)) {
> > +		dev_err(dev, "Could not get TPM timeouts and durations\n");
> > +		rc = -ENODEV;
> > +		goto out_err;
> > +	}
> > +
> > +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > +		rc = tpm2_do_selftest(chip);
> > +	else
> > +		rc = tpm_do_selftest(chip);
> > +	if (rc) {
> > +		dev_err(dev, "TPM self test failed\n");
> > +		rc = -ENODEV;
> > +		goto out_err;
> > +	}
> > +
> >  	/* INTERRUPT Setup */
> >  	init_waitqueue_head(&chip->vendor.read_queue);
> >  	init_waitqueue_head(&chip->vendor.int_queue);
> 
> In Peter's for-james branch, commit 0e6cb01, the v1.2 calls to
> tpm_get_timeouts and tpm_do_selftest were moved toward the end of 
> tpm_tis_init.

Right. My bad. Could this be reverted with a separate fix or do
I prepare a new patch set? I would propose the former...

> > @@ -720,7 +767,10 @@ static int tpm_tis_init(struct device *dev, acpi_handle acpi_dev_handle,
> >  			chip->vendor.probed_irq = 0;
> >  
> >  			/* Generate Interrupts */
> > -			tpm_gen_interrupt(chip);
> > +			if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > +				tpm2_gen_interrupt(chip);
> > +			else
> > +				tpm_gen_interrupt(chip);
> >  
> >  			chip->vendor.irq = chip->vendor.probed_irq;
> >  
> > @@ -808,14 +858,18 @@ static void tpm_tis_reenable_interrupts(struct tpm_chip *chip)
> >  static int tpm_tis_resume(struct device *dev)
> >  {
> >  	struct tpm_chip *chip = dev_get_drvdata(dev);
> > -	int ret;
> > +	int ret = 0;
> >  
> >  	if (chip->vendor.irq)
> >  		tpm_tis_reenable_interrupts(chip);
> >  
> > -	ret = tpm_pm_resume(dev);
> > -	if (!ret)
> > -		tpm_do_selftest(chip);
> > +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > +		tpm2_do_selftest(chip);
> > +	else {
> > +		ret = tpm_pm_resume(dev);
> > +		if (!ret)
> > +			tpm_do_selftest(chip);
> > +	}
> >  
> >  	return ret;
> >  }
> > -- 
> > 2.1.0
> > 

/Jarkko

^ permalink raw reply

* Re: [PATCH v9 8/8] tpm: TPM 2.0 FIFO Interface
From: Jarkko Sakkinen @ 2014-12-05 13:06 UTC (permalink / raw)
  To: Peter Hüwe
  Cc: Ashley Lai, Marcel Selhorst,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	josh.triplett-ral2JQCrhuEAvxtiuMwx3w,
	christophe.ricard-Re5JQEeQqe8AvxtiuMwx3w,
	jason.gunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	trousers-tech-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Will Arthur
In-Reply-To: <201412042246.25662.PeterHuewe-Mmb7MZpHnFY@public.gmane.org>

On Thu, Dec 04, 2014 at 10:46:25PM +0100, Peter Hüwe wrote:
> Am Donnerstag, 4. Dezember 2014, 06:55:18 schrieb Jarkko Sakkinen:
> > From: Will Arthur <will.c.arthur-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > 
> > Detect TPM 2.0 by using the extended STS (STS3) register. For TPM 2.0,
> > instead of calling tpm_get_timeouts(), assign duration and timeout
> > values defined in the TPM 2.0 PTP specification.
> > 
> > Signed-off-by: Will Arthur <will.c.arthur-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > ---
> Should we change the KConfig string?
>  TPM Interface Specification 1.2 Interface
> is not quite accurate anymore after applying this patch.

Would make sense. Does this require a new patch set?

> Thanks,
> Peter

/Jarkko

^ permalink raw reply

* Re: [PATCH v9 7/8] tpm: TPM 2.0 CRB Interface
From: Jarkko Sakkinen @ 2014-12-05 13:10 UTC (permalink / raw)
  To: Peter Hüwe
  Cc: Ashley Lai, Marcel Selhorst,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	josh.triplett-ral2JQCrhuEAvxtiuMwx3w,
	christophe.ricard-Re5JQEeQqe8AvxtiuMwx3w,
	jason.gunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	trousers-tech-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <201412042119.21493.PeterHuewe-Mmb7MZpHnFY@public.gmane.org>

I'll fix these (an run sparse).

/Jarkko

On Thu, Dec 04, 2014 at 09:19:21PM +0100, Peter Hüwe wrote:
> Am Donnerstag, 4. Dezember 2014, 06:55:17 schrieb Jarkko Sakkinen:
> > tpm_crb is a driver for TPM 2.0 Command Response Buffer (CRB) Interface
> > as defined in PC Client Platform TPM Profile (PTP) Specification.
> > 
> > Only polling and single locality is supported as these are the limitations
> > of the available hardware, Platform Trust Techonlogy (PTT) in Haswell
> > CPUs.
> > 
> > The driver always applies CRB with ACPI start because PTT reports using
> > only ACPI start as start method but as a result of my testing it requires
> > also CRB start.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> 
> :(
> 
>  make -C /data/data-old/linux-2.6/ M=$(pwd) modules C=1 CHECK=sparse
> make: Entering directory '/data/data-old/linux-2.6'
>   CHECK   /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:99:5: warning: symbol 'crb_suspend' was not declared. Should it be static?
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:121:14: warning: incorrect type in argument 1 (different address spaces)
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:121:14:    expected void [noderef] <asn:2>*<noident>
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:121:14:    got unsigned int *<noident>
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:137:13: warning: incorrect type in argument 1 (different address spaces)
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:137:13:    expected void [noderef] <asn:2>*<noident>
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:137:13:    got unsigned int *<noident>
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:173:19: warning: incorrect type in argument 1 (different address spaces)
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:173:19:    expected void [noderef] <asn:2>*<noident>
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:173:19:    got unsigned int *<noident>
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:177:34: warning: incorrect type in argument 1 (different address spaces)
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:177:34:    expected void [noderef] <asn:2>*<noident>
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:177:34:    got unsigned int *<noident>
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:187:63: warning: incorrect type in argument 2 (different address spaces)
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:187:63:    expected void [noderef] <asn:2>*<noident>
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:187:63:    got unsigned int *<noident>
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:199:56: warning: incorrect type in argument 2 (different address spaces)
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:199:56:    expected void [noderef] <asn:2>*<noident>
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:199:56:    got unsigned int *<noident>
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:207:27: warning: incorrect type in argument 2 (different address spaces)
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:207:27:    expected void [noderef] <asn:2>*<noident>
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:207:27:    got unsigned int *<noident>
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:277:22: warning: cast removes address space of expression
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:284:33: warning: incorrect type in argument 2 (different address spaces)
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:284:33:    expected void const volatile [noderef] <asn:2>*src
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:284:33:    got unsigned long long *<noident>
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:287:56: warning: incorrect type in argument 1 (different address spaces)
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:287:56:    expected void [noderef] <asn:2>*<noident>
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:287:56:    got unsigned int *<noident>
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:286:19: warning: incorrect type in assignment (different address spaces)
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:286:19:    expected unsigned char [usertype] *cmd
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:286:19:    got void [noderef] <asn:2>*
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:293:33: warning: incorrect type in argument 2 (different address spaces)
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:293:33:    expected void const volatile [noderef] <asn:2>*src
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:293:33:    got unsigned long long *<noident>
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:296:56: warning: incorrect type in argument 1 (different address spaces)
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:296:56:    expected void [noderef] <asn:2>*<noident>
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:296:56:    got unsigned int *<noident>
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:295:19: warning: incorrect type in assignment (different address spaces)
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:295:19:    expected unsigned char [usertype] *rsp
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:295:19:    got void [noderef] <asn:2>*
> /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.c:325:5: warning: symbol 'crb_acpi_remove' was not declared. Should it be static?
>   CC [M]  /data/data-old/linux-2.6/drivers/char/tpm/tpm_crb.o
> 
> :(
> Peter

^ permalink raw reply

* Re: [PATCH v9 6/8] tpm: TPM 2.0 baseline support
From: Jarkko Sakkinen @ 2014-12-05 14:13 UTC (permalink / raw)
  To: Peter Hüwe
  Cc: Ashley Lai, Marcel Selhorst,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	josh.triplett-ral2JQCrhuEAvxtiuMwx3w,
	christophe.ricard-Re5JQEeQqe8AvxtiuMwx3w,
	jason.gunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	trousers-tech-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Will Arthur
In-Reply-To: <201412050017.40668.PeterHuewe-Mmb7MZpHnFY@public.gmane.org>

Hey

is it cool if I prepare a separate set of fixes for all issues
in v9? I do not see any problem that could not be fixed without
major structural changes.

/Jarkko

On Fri, Dec 05, 2014 at 12:17:40AM +0100, Peter Hüwe wrote:
> Am Donnerstag, 4. Dezember 2014, 06:55:16 schrieb Jarkko Sakkinen:
> > TPM 2.0 devices are separated by adding a field 'flags' to struct
> > tpm_chip and defining a flag TPM_CHIP_FLAG_TPM2 for tagging them.
> > 
> > This patch adds the following internal functions:
> > 
> > - tpm2_get_random()
> > - tpm2_get_tpm_pt()
> > - tpm2_pcr_extend()
> > - tpm2_pcr_read()
> > - tpm2_startup()
> > 
> > Additionally, the following exported functions are implemented for
> > implementing TPM 2.0 device drivers:
> > 
> > - tpm2_do_selftest()
> > - tpm2_calc_ordinal_durations()
> > - tpm2_gen_interrupt()
> > 
> > The existing functions that are exported for the use for existing
> > subsystems have been changed to check the flags field in struct
> > tpm_chip and use appropriate TPM 2.0 counterpart if
> > TPM_CHIP_FLAG_TPM2 is est.
> > 
> > The code for tpm2_calc_ordinal_duration() and tpm2_startup() were
> > originally written by Will Arthur.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > Signed-off-by: Will Arthur <will.c.arthur-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > ---
> >  drivers/char/tpm/Makefile        |   2 +-
> >  drivers/char/tpm/tpm-chip.c      |  27 +-
> >  drivers/char/tpm/tpm-interface.c |  24 +-
> >  drivers/char/tpm/tpm.h           |  61 +++++
> >  drivers/char/tpm/tpm2-cmd.c      | 542
> > +++++++++++++++++++++++++++++++++++++++ 5 files changed, 641
> > insertions(+), 15 deletions(-)
> >  create mode 100644 drivers/char/tpm/tpm2-cmd.c
> > 
> > diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> > index c715596..88848ed 100644
> > --- a/drivers/char/tpm/Makefile
> > +++ b/drivers/char/tpm/Makefile
> > @@ -2,7 +2,7 @@
> >  # Makefile for the kernel tpm device drivers.
> >  #
> >  obj-$(CONFIG_TCG_TPM) += tpm.o
> > -tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o
> > +tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o
> >  tpm-$(CONFIG_ACPI) += tpm_ppi.o
> > 
> >  ifdef CONFIG_ACPI
> > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > index 7741e28..3f3f2de 100644
> > --- a/drivers/char/tpm/tpm-chip.c
> > +++ b/drivers/char/tpm/tpm-chip.c
> > @@ -195,15 +195,18 @@ int tpm_chip_register(struct tpm_chip *chip)
> >  	if (rc)
> >  		return rc;
> > 
> > -	rc = tpm_sysfs_add_device(chip);
> > -	if (rc)
> > -		goto del_misc;
> > +	/* Populate sysfs for TPM1 devices. */
> > +	if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> > +		rc = tpm_sysfs_add_device(chip);
> > +		if (rc)
> > +			goto del_misc;
> > 
> > -	rc = tpm_add_ppi(chip);
> > -	if (rc)
> > -		goto del_sysfs;
> > +		rc = tpm_add_ppi(chip);
> > +		if (rc)
> > +			goto del_sysfs;
> > 
> > -	chip->bios_dir = tpm_bios_log_setup(chip->devname);
> > +		chip->bios_dir = tpm_bios_log_setup(chip->devname);
> > +	}
> > 
> >  	/* Make the chip available. */
> >  	spin_lock(&driver_lock);
> > @@ -236,10 +239,12 @@ void tpm_chip_unregister(struct tpm_chip *chip)
> >  	spin_unlock(&driver_lock);
> >  	synchronize_rcu();
> > 
> > -	if (chip->bios_dir)
> > -		tpm_bios_log_teardown(chip->bios_dir);
> > -	tpm_remove_ppi(chip);
> > -	tpm_sysfs_del_device(chip);
> > +	if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> > +		if (chip->bios_dir)
> > +			tpm_bios_log_teardown(chip->bios_dir);
> > +		tpm_remove_ppi(chip);
> > +		tpm_sysfs_del_device(chip);
> > +	}
> > 
> >  	tpm_dev_del_device(chip);
> >  }
> > diff --git a/drivers/char/tpm/tpm-interface.c
> > b/drivers/char/tpm/tpm-interface.c index b6f6b17..8a14887 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -360,7 +360,10 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const char
> > *buf, if (chip->vendor.irq)
> >  		goto out_recv;
> > 
> > -	stop = jiffies + tpm_calc_ordinal_duration(chip, ordinal);
> > +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > +		stop = jiffies + tpm2_calc_ordinal_duration(chip, ordinal);
> > +	else
> > +		stop = jiffies + tpm_calc_ordinal_duration(chip, ordinal);
> >  	do {
> >  		u8 status = chip->ops->status(chip);
> >  		if ((status & chip->ops->req_complete_mask) ==
> > @@ -483,7 +486,7 @@ static const struct tpm_input_header tpm_startup_header
> > = { static int tpm_startup(struct tpm_chip *chip, __be16 startup_type) {
> >  	struct tpm_cmd_t start_cmd;
> > -	start_cmd.header.in = tpm_startup_header;
> > +
> WHY?!? This renders tpm_startup useless.
> 
> So NACK for this part.
> >  	start_cmd.params.startup_in.startup_type = startup_type;
> >  	return tpm_transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE,
> >  				"attempting to start the TPM");
> 
> I'll get you an TPM1.2 :)
> 
> Thanks
> Peter

^ permalink raw reply

* Aw: Re: [PATCH v9 6/8] tpm: TPM 2.0 baseline support
From: Peter Huewe @ 2014-12-05 14:35 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Ashley Lai, Marcel Selhorst,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	josh.triplett-ral2JQCrhuEAvxtiuMwx3w,
	christophe.ricard-Re5JQEeQqe8AvxtiuMwx3w,
	jason.gunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	trousers-tech-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Will Arthur
In-Reply-To: <20141205141303.GF6993-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Hi Jarkko,
  

>is it cool if I prepare a separate set of fixes for all issues
>in v9? I do not see any problem that could not be fixed without
>major structural changes.

 

to be honest - I would prefer a v10 with the fixes included, especially since there were quite a few issues/remarks.
If you already have fixes post them as patches on top of v9 so we can test, and then squash them in.




I think we are still in schedule if you do fixes over the weekend - so we hopefully have a _great_ v10 on monday/tuesday
 
I'll send out the first pull request without TPM2.0 support later this evening,
and then send out another one by next week - this should be fine.
 
The next testing slot is Sunday evening since I'm travelling myself (and don't have TPM hardware with me).
 
 
Thanks,
Peter
 

^ permalink raw reply

* Aw: Re: [tpmdd-devel] [PATCH v9 8/8] tpm: TPM 2.0 FIFO Interface
From: Peter Huewe @ 2014-12-05 15:01 UTC (permalink / raw)
  To: "Peter Hüwe"
  Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	christophe.ricard-Re5JQEeQqe8AvxtiuMwx3w,
	josh.triplett-ral2JQCrhuEAvxtiuMwx3w,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Ashley Lai,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Will Arthur,
	jason.gunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
	trousers-tech-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <201412042328.29578.PeterHuewe-Mmb7MZpHnFY@public.gmane.org>

> 
> > Am Donnerstag, 4. Dezember 2014, 06:55:18 schrieb Jarkko Sakkinen:
> > > From: Will Arthur <will.c.arthur-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > >
> > > Detect TPM 2.0 by using the extended STS (STS3) register. For TPM 2.0,
> > > instead of calling tpm_get_timeouts(), assign duration and timeout
> > > values defined in the TPM 2.0 PTP specification.
> > >
> > > Signed-off-by: Will Arthur <will.c.arthur-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> 
> > >
> > > + sts3 = ioread8(chip->vendor.iobase + TPM_STS3(1));
> > > + if ((sts3 & TPM_STS3_TPM2_FAM) == TPM_STS3_TPM2_FAM)
> > > + chip->flags = TPM_CHIP_FLAG_TPM2;
> > > +
> > >
> 
> >
> > When loading tpm_tis force=1 with my tpm1.2 chip on a machine without bios
> > integration, it fets detected as a TPM2.0 chip :/
> >
> > sudo rmmod tpm_tis
> > # modprobe tpm_tis force=1
> > modprobe: ERROR: could not insert 'tpm_tis': No such device
> > # dmesg
> > [ 263.903828] tpm_tis tpm_tis: 2.0 TPM (device-id 0xB, rev-id 16)
> > [ 263.948049] tpm_tis tpm_tis: A TPM error (10) occurred continue selftest
> > [ 263.948120] tpm_tis tpm_tis: TPM self test failed
> >
> >
> > sts3 is reported as 0xff from my TPM1.2
> >
> 
> 
> Hmm,
> my TPM2.0 chip also reports sts3 as 0xff (when loading with force=1 on a
> machine without bios integration)
> 
> [ 307.095344] sts3 ff
> [ 307.095366] tpm_tis tpm_tis: 2.0 TPM (device-id 0x1A, rev-id 16)
> [ 307.140047] tpm_tis tpm_tis: A TPM error (256) occurred continue selftest
> [ 307.140056] tpm_tis tpm_tis: TPM self test failed


You are reading "sts3" - before requesting the locality and thus it returns 0xff for a TPM20 chip as well.
--> You have to have an active locality first.


For a TPM2.0 0xFF is not a valid value (if active locality is set), since reading commandCancel and resetEstablishment bit always return 0 on reads (according to spec).

--> 0xFF should be treated as a TPM1.2 (older tpms with TIS 1.2)
--> 0x04 should be treated as TPM 2.0
--> 0x08 should be treated as TPM1.2 (newer tpms with TIS1.3 enhanced)



Thanks,
Peter

^ permalink raw reply

* Re: [PATCH v4 2/5] crypto: AF_ALG: add AEAD support
From: Herbert Xu @ 2014-12-05 15:46 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: Daniel Borkmann, 'Quentin Gouchet', 'LKML',
	linux-crypto, linux-api
In-Reply-To: <4875720.jRoMDtjHB4@tachyon.chronox.de>

On Wed, Dec 03, 2014 at 08:57:24PM +0100, Stephan Mueller wrote:
>
> +	if (ctx->merge) {
> +		sg = sgl->sg + sgl->cur - 1;
> +		len = min_t(unsigned long, len,
> +			    PAGE_SIZE - sg->offset - sg->length);
> +
> +		err = memcpy_fromiovec(page_address(sg_page(sg)) +
> +				       sg->offset + sg->length,
> +				       msg->msg_iov, len);
> +		if (err)
> +			goto unlock;
> +
> +		sg->length += len;
> +		ctx->merge = (sg->offset + sg->length) & (PAGE_SIZE - 1);
> +
> +		ctx->used += len;
> +		copied += len;
> +		size -= len;
> +	}

Any reason why you got rid of the outer loop here? This will cause
short writes I think.

> +static struct proto_ops algif_aead_ops = {
> +	.family		=	PF_ALG,
> +
> +	.connect	=	sock_no_connect,
> +	.socketpair	=	sock_no_socketpair,
> +	.getname	=	sock_no_getname,
> +	.ioctl		=	sock_no_ioctl,
> +	.listen		=	sock_no_listen,
> +	.shutdown	=	sock_no_shutdown,
> +	.getsockopt	=	sock_no_getsockopt,
> +	.mmap		=	sock_no_mmap,
> +	.bind		=	sock_no_bind,
> +	.accept		=	sock_no_accept,
> +
> +	.release	=	af_alg_release,
> +	.sendmsg	=	aead_sendmsg,
> +	.sendpage	=	aead_sendpage,
> +	.recvmsg	=	aead_recvmsg,
> +	.poll		=	aead_poll,
> +	.setsockopt	=	aead_setsockopt,

No it should go into the parent setsockopt.  Perhaps add a setsockopt
to af_alg_type in order to keep this out of the generic code.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH v4 4/5] crypto: AF_ALG: add random number generator support
From: Herbert Xu @ 2014-12-05 15:53 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: Daniel Borkmann, 'Quentin Gouchet', 'LKML',
	linux-crypto, linux-api
In-Reply-To: <1480776.TeURZV8VLa@tachyon.chronox.de>

On Wed, Dec 03, 2014 at 08:59:01PM +0100, Stephan Mueller wrote:
>
> +static int rng_recvmsg(struct kiocb *unused, struct socket *sock,
> +		       struct msghdr *msg, size_t len, int flags)
> +{
> +	struct sock *sk = sock->sk;
> +	struct alg_sock *ask = alg_sk(sk);
> +	struct rng_ctx *ctx = ask->private;
> +	int err = -EFAULT;
> +
> +	if (len == 0)
> +		return 0;
> +	if (len > MAXSIZE)
> +		len = MAXSIZE;
> +
> +	lock_sock(sk);

This lock simply protects ctx->result.  Since you're using a
tiny buffer why not just put it on the stack?

> +		u8 *buf = kmalloc(seedsize, GFP_KERNEL);
> +		if (!buf)
> +			goto err;
> +		get_random_bytes(buf, seedsize);
> +		ret = crypto_rng_reset(private, buf, len);

I think you should leave the seeding and the seed to the user.
Perhaps do it through setsockopt (on the parent socket).

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH v4 1/5] crypto: AF_ALG: add user space interface for AEAD
From: Herbert Xu @ 2014-12-05 16:01 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: Daniel Borkmann, 'Quentin Gouchet', 'LKML',
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4746255.Vltg6sM4x3-PJstQz4BMNNP20K/wil9xYQuADTiUCJX@public.gmane.org>

On Wed, Dec 03, 2014 at 08:55:42PM +0100, Stephan Mueller wrote:
> AEAD requires the caller to specify the following information separate
> from the data stream. This information allows the AEAD interface handler
> to identify the AAD, ciphertext/plaintext and the authentication tag:
> 
>         * Associated authentication data of arbitrary length and
>           length
> 
>         * Length of authentication tag for encryption
> 
> Signed-off-by: Stephan Mueller <smueller-T9tCv8IpfcWELgA04lAiVw@public.gmane.org>

Patch applied.
-- 
Email: Herbert Xu <herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [tpmdd-devel] [PATCH v9 8/8] tpm: TPM 2.0 FIFO Interface
From: Stefan Berger @ 2014-12-05 20:44 UTC (permalink / raw)
  To: Peter Huewe
  Cc: christophe.ricard-Re5JQEeQqe8AvxtiuMwx3w,
	josh.triplett-ral2JQCrhuEAvxtiuMwx3w,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Ashley Lai,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Will Arthur,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	jason.gunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
	trousers-tech-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <trinity-0fea8cea-4011-4d04-af4a-e7583a7184b4-1417791661803@3capp-gmx-bs52>

On 12/05/2014 10:01 AM, Peter Huewe wrote:
>>> Am Donnerstag, 4. Dezember 2014, 06:55:18 schrieb Jarkko Sakkinen:
>>>> From: Will Arthur <will.c.arthur-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>>>>
>>>> Detect TPM 2.0 by using the extended STS (STS3) register. For TPM 2.0,
>>>> instead of calling tpm_get_timeouts(), assign duration and timeout
>>>> values defined in the TPM 2.0 PTP specification.
>>>>
>>>> Signed-off-by: Will Arthur <will.c.arthur-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>>>> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>>>> + sts3 = ioread8(chip->vendor.iobase + TPM_STS3(1));
>>>> + if ((sts3 & TPM_STS3_TPM2_FAM) == TPM_STS3_TPM2_FAM)
>>>> + chip->flags = TPM_CHIP_FLAG_TPM2;
>>>> +
>>>>
>>> When loading tpm_tis force=1 with my tpm1.2 chip on a machine without bios
>>> integration, it fets detected as a TPM2.0 chip :/
>>>
>>> sudo rmmod tpm_tis
>>> # modprobe tpm_tis force=1
>>> modprobe: ERROR: could not insert 'tpm_tis': No such device
>>> # dmesg
>>> [ 263.903828] tpm_tis tpm_tis: 2.0 TPM (device-id 0xB, rev-id 16)
>>> [ 263.948049] tpm_tis tpm_tis: A TPM error (10) occurred continue selftest
>>> [ 263.948120] tpm_tis tpm_tis: TPM self test failed
>>>
>>>
>>> sts3 is reported as 0xff from my TPM1.2
>>>
>>
>> Hmm,
>> my TPM2.0 chip also reports sts3 as 0xff (when loading with force=1 on a
>> machine without bios integration)
>>
>> [ 307.095344] sts3 ff
>> [ 307.095366] tpm_tis tpm_tis: 2.0 TPM (device-id 0x1A, rev-id 16)
>> [ 307.140047] tpm_tis tpm_tis: A TPM error (256) occurred continue selftest
>> [ 307.140056] tpm_tis tpm_tis: TPM self test failed
>
> You are reading "sts3" - before requesting the locality and thus it returns 0xff for a TPM20 chip as well.
> --> You have to have an active locality first.
>
>
> For a TPM2.0 0xFF is not a valid value (if active locality is set), since reading commandCancel and resetEstablishment bit always return 0 on reads (according to spec).
>
> --> 0xFF should be treated as a TPM1.2 (older tpms with TIS 1.2)
> --> 0x04 should be treated as TPM 2.0
> --> 0x08 should be treated as TPM1.2 (newer tpms with TIS1.3 enhanced)

> + sts3 = ioread8(chip->vendor.iobase + TPM_STS3(1));
> + if ((sts3 & TPM_STS3_TPM2_FAM) == TPM_STS3_TPM2_FAM)
> + chip->flags = TPM_CHIP_FLAG_TPM2;

#define TPM_STS3_TPM2_FAM_MASK = 0x0c

then:

if ((sts3 & TPM_STS3_TPM2_FAM_MASK) == TPM_STS3_TPM2_FAM) ...

[see my previous review on this]

    Stefan






>
>
>
> Thanks,
> Peter
>
> ------------------------------------------------------------------------------
> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
> from Actuate! Instantly Supercharge Your Business Reports and Dashboards
> with Interactivity, Sharing, Native Excel Exports, App Integration & more
> Get technology previously reserved for billion-dollar corporations, FREE
> http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
> _______________________________________________
> tpmdd-devel mailing list
> tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
>

^ permalink raw reply

* Re: [PATCH] arch: uapi: asm: mman.h: Let MADV_FREE have same value for all architectures
From: Chen Gang @ 2014-12-05 21:51 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org,
	rth-hL46jP5Bxq7R7s880joybQ@public.gmane.org,
	ink-biIs/Y0ymYJMZLIVYojuPNP0rXTJTi09@public.gmane.org,
	mattst88-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, Ralf Baechle,
	jejb-6jwH94ZQLHl74goWV3ctuw@public.gmane.org,
	deller-Mmb7MZpHnFY@public.gmane.org,
	chris-YvXeqwSYzG2sTnJN9+BGXg@public.gmane.org,
	jcmvbkbc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, Arnd Bergmann,
	minchan-DgEjT+Ai2ygdnm+yROfE0A,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arch-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20141205001446.GC10064-PTl6brltDGh4DFYR7WNSRA@public.gmane.org>

On 12/05/2014 08:14 AM, Darrick J. Wong wrote:
> On Fri, Dec 05, 2014 at 06:58:29AM +0800, Chen Gang wrote:
>> For uapi, need try to let all macros have same value, and MADV_FREE is
>> added into main branch recently, so need redefine MADV_FREE for it.
>>
>> At present, '8' can be shared with all architectures, so redefine it to
>> '8'.
> 
> Hmmm... you might want to talk to the dietlibc people if you do this:
> 
> $ grep -rn MADV_FREE /usr/include/
> /usr/include/diet/sys/mman.h:101:#define MADV_FREE      0x5             /* (Solaris) contents can be freed */
> 
> Though the sole definition only exists on sparc/powerpc/powerpc64, so who
> knows...
>

Excuse me, I am not quite familiar about dietlibc. If possible, please
help forward this mail to them, and Cc me (if necessary).

Thanks.
-- 
Chen Gang

Open share and attitude like air water and life which God blessed

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox