* Re: [PATCH v7 25/27] mm/mmap: Add Shadow stack pages to memory accounting
From: Yu-cheng Yu @ 2019-06-11 19:22 UTC (permalink / raw)
To: Dave Hansen, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Peter Zijlstra, Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue,
Dave Martin
In-Reply-To: <1cfc7396-ca90-1933-34ad-b3d43ae52e08@intel.com>
On Tue, 2019-06-11 at 10:55 -0700, Dave Hansen wrote:
> On 6/6/19 1:06 PM, Yu-cheng Yu wrote:
> > --- a/mm/mmap.c
> > +++ b/mm/mmap.c
> > @@ -1703,6 +1703,9 @@ static inline int accountable_mapping(struct file
> > *file, vm_flags_t vm_flags)
> > if (file && is_file_hugepages(file))
> > return 0;
> >
> > + if (arch_copy_pte_mapping(vm_flags))
> > + return 1;
> > +
> > return (vm_flags & (VM_NORESERVE | VM_SHARED | VM_WRITE)) ==
> > VM_WRITE;
> > }
> >
> > @@ -3319,6 +3322,8 @@ void vm_stat_account(struct mm_struct *mm, vm_flags_t
> > flags, long npages)
> > mm->stack_vm += npages;
> > else if (is_data_mapping(flags))
> > mm->data_vm += npages;
> > + else if (arch_copy_pte_mapping(flags))
> > + mm->data_vm += npages;
> > }
>
> This classifies shadow stack as data instead of stack. That seems a wee
> bit counterintuitive. Why did you make this choice?
I don't recall the reason; I will change it to stack and test it out.
Yu-cheng
^ permalink raw reply
* Re: [PATCH v3 05/33] docs: cgroup-v1: convert docs to ReST and rename to *.rst
From: Tejun Heo @ 2019-06-11 19:03 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
H. Peter Anvin, x86, Jens Axboe, Li Zefan, Johannes Weiner,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, James Morris, Serge E. Hallyn, linux-block,
cgroups, netdev, bpf, linux-security-module
In-Reply-To: <79865a4248ce5b042106e5ec69bb493292a8d392.1560045490.git.mchehab+samsung@kernel.org>
On Sat, Jun 08, 2019 at 11:26:55PM -0300, Mauro Carvalho Chehab wrote:
> Convert the cgroup-v1 files to ReST format, in order to
> allow a later addition to the admin-guide.
>
> The conversion is actually:
> - add blank lines and identation in order to identify paragraphs;
> - fix tables markups;
> - add some lists markups;
> - mark literal blocks;
> - adjust title markups.
>
> At its new index.rst, let's add a :orphan: while this is not linked to
> the main index.rst file, in order to avoid build warnings.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Acked-by: Tejun Heo <tj@kernel.org>
Please feel free to route with the rest of the series. If you want
the patch to be routed through the cgroup tree, please let me know.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v4 17/18] kernel/sysctl-test: Add null pointer test for sysctl.c:proc_dointvec()
From: Stephen Boyd @ 2019-06-11 18:50 UTC (permalink / raw)
To: Brendan Higgins
Cc: Iurii Zaikin, frowand.list, gregkh, jpoimboe, keescook,
kieran.bingham, mcgrof, peterz, robh, shuah, tytso,
yamada.masahiro, devicetree, dri-devel, kunit-dev, linux-doc,
linux-fsdevel, linux-kbuild, linux-kernel, linux-kselftest,
linux-nvdimm, linux-um, Alexander.Levin, Tim.Bird, amir73il,
dan.carpenter, daniel, jdike, joel, julia.lawall, khilman,
knut.omang, logang, mpe, pmladek, rdunlap, richard, rientjes,
rostedt, wfg
In-Reply-To: <20190611175830.GA236872@google.com>
Quoting Brendan Higgins (2019-06-11 10:58:30)
> On Fri, Jun 07, 2019 at 12:00:47PM -0700, Stephen Boyd wrote:
> > Quoting Iurii Zaikin (2019-06-05 18:29:42)
> > > On Fri, May 17, 2019 at 11:22 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > > >
> > > > Quoting Brendan Higgins (2019-05-14 15:17:10)
> > > > > diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c
> > > > > new file mode 100644
> > > > > index 0000000000000..fe0f2bae66085
> > > > > --- /dev/null
> > > > > +++ b/kernel/sysctl-test.c
> > > > > +
> > > > > +
> > > > > +static void sysctl_test_dointvec_happy_single_negative(struct kunit *test)
> > > > > +{
> > > > > + struct ctl_table table = {
> > > > > + .procname = "foo",
> > > > > + .data = &test_data.int_0001,
> > > > > + .maxlen = sizeof(int),
> > > > > + .mode = 0644,
> > > > > + .proc_handler = proc_dointvec,
> > > > > + .extra1 = &i_zero,
> > > > > + .extra2 = &i_one_hundred,
> > > > > + };
> > > > > + char input[] = "-9";
> > > > > + size_t len = sizeof(input) - 1;
> > > > > + loff_t pos = 0;
> > > > > +
> > > > > + table.data = kunit_kzalloc(test, sizeof(int), GFP_USER);
> > > > > + KUNIT_EXPECT_EQ(test, 0, proc_dointvec(&table, 1, input, &len, &pos));
> > > > > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, len);
> > > > > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, pos);
> > > > > + KUNIT_EXPECT_EQ(test, -9, *(int *)table.data);
> > > >
> > > > Is the casting necessary? Or can the macro do a type coercion of the
> > > > second parameter based on the first type?
> > > Data field is defined as void* so I believe casting is necessary to
> > > dereference it as a pointer to an array of ints. I don't think the
> > > macro should do any type coercion that == operator wouldn't do.
> > > I did change the cast to make it more clear that it's a pointer to an
> > > array of ints being dereferenced.
> >
> > Ok, I still wonder if we should make KUNIT_EXPECT_EQ check the types on
> > both sides and cause a build warning/error if the types aren't the same.
> > This would be similar to our min/max macros that complain about
> > mismatched types in the comparisons. Then if a test developer needs to
> > convert one type or the other they could do so with a
> > KUNIT_EXPECT_EQ_T() macro that lists the types to coerce both sides to
> > explicitly.
>
> Do you think it would be better to do a phony compare similar to how
> min/max used to work prior to 4.17, or to use the new __typecheck(...)
> macro? This might seem like a dumb question (and maybe it is), but Iurii
> and I thought the former created an error message that was a bit easier
> to understand, whereas __typecheck is obviously superior in terms of
> code reuse.
>
> This is what we are thinking right now; if you don't have any complaints
> I will squash it into the relevant commits on the next revision:
Can you provide the difference in error messages and describe that in
the commit text? The commit message is where you "sell" the patch, so
being able to compare the tradeoff of having another macro to do type
comparisons vs. reusing the one that's there in kernel.h would be useful
to allay concerns that we're duplicating logic for better error
messages.
Honestly, I'd prefer we just use the macros that we've developed in
kernel.h to do comparisons here so that we can get code reuse, but more
importantly so that we don't trip over problems that caused those macros
to be created in the first place. If the error message is bad, perhaps
that can be fixed with some sort of compiler directive to make the error
message a little more useful, i.e. compiletime_warning() thrown into
__typecheck() or something.
> ---
> From: Iurii Zaikin <yzaikin@google.com>
>
> Adds a warning message when comparing values of different types similar
> to what min() / max() macros do.
>
> Signed-off-by: Iurii Zaikin <yzaikin@google.com>
^ permalink raw reply
* Re: [PATCH v3 06/20] docs: mark orphan documents as such
From: Andy Shevchenko @ 2019-06-11 18:23 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab,
Linux Kernel Mailing List, Jonathan Corbet, Frederic Barrat,
Andrew Donnellan, Maxime Coquelin, Alexandre Torgue, David Airlie,
Daniel Vetter, Maarten Lankhorst, Maxime Ripard, Sean Paul,
Georgi Djakov, Matan Ziv-Av, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman,
open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, linux-stm32,
linux-arm Mailing List, dri-devel, Linux PM, Platform Driver
In-Reply-To: <20190611140501.11ba091b@coco.lan>
On Tue, Jun 11, 2019 at 8:05 PM Mauro Carvalho Chehab
<mchehab+samsung@kernel.org> wrote:
>
> Em Tue, 11 Jun 2019 19:52:04 +0300
> Andy Shevchenko <andy.shevchenko@gmail.com> escreveu:
>
> > On Fri, Jun 7, 2019 at 10:04 PM Mauro Carvalho Chehab
> > <mchehab+samsung@kernel.org> wrote:
> > > Sphinx doesn't like orphan documents:
> >
> > > Documentation/laptops/lg-laptop.rst: WARNING: document isn't included in any toctree
> >
> > > Documentation/laptops/lg-laptop.rst | 2 ++
> >
> > > diff --git a/Documentation/laptops/lg-laptop.rst b/Documentation/laptops/lg-laptop.rst
> > > index aa503ee9b3bc..f2c2ffe31101 100644
> > > --- a/Documentation/laptops/lg-laptop.rst
> > > +++ b/Documentation/laptops/lg-laptop.rst
> > > @@ -1,5 +1,7 @@
> > > .. SPDX-License-Identifier: GPL-2.0+
> > >
> > > +:orphan:
> > > +
> > > LG Gram laptop extra features
> > > =============================
> > >
> >
> > Can we rather create a toc tree there?
> > It was a first document in reST format in that folder.
>
> Sure, but:
>
> 1) I have a patch converting the other files on this dir to rst:
>
> https://git.linuxtv.org/mchehab/experimental.git/commit/?h=convert_rst_renames_v4.1&id=abc13233035fdfdbc5ef2f2fbd3d127a1ab15530
>
> 2) It probably makes sense to move the entire dir to
> Documentation/admin-guide.
>
> So, I would prefer to have the :orphan: here while (1) is not merged.
Fine to me as long as you will drop it by the mentioned effort.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v4 17/18] kernel/sysctl-test: Add null pointer test for sysctl.c:proc_dointvec()
From: Brendan Higgins @ 2019-06-11 17:58 UTC (permalink / raw)
To: Stephen Boyd
Cc: Iurii Zaikin, frowand.list, gregkh, jpoimboe, keescook,
kieran.bingham, mcgrof, peterz, robh, shuah, tytso,
yamada.masahiro, devicetree, dri-devel, kunit-dev, linux-doc,
linux-fsdevel, linux-kbuild, linux-kernel, linux-kselftest,
linux-nvdimm, linux-um, Alexander.Levin, Tim.Bird, amir73il,
dan.carpenter, daniel, jdike, joel, julia.lawall, khilman,
knut.omang, logang, mpe, pmladek, rdunlap, richard, rientjes,
rostedt, wfg
In-Reply-To: <20190607190047.C3E7A20868@mail.kernel.org>
On Fri, Jun 07, 2019 at 12:00:47PM -0700, Stephen Boyd wrote:
> Quoting Iurii Zaikin (2019-06-05 18:29:42)
> > On Fri, May 17, 2019 at 11:22 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > >
> > > Quoting Brendan Higgins (2019-05-14 15:17:10)
> > > > diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c
> > > > new file mode 100644
> > > > index 0000000000000..fe0f2bae66085
> > > > --- /dev/null
> > > > +++ b/kernel/sysctl-test.c
> > > > +
> > > > +
> > > > +static void sysctl_test_dointvec_happy_single_negative(struct kunit *test)
> > > > +{
> > > > + struct ctl_table table = {
> > > > + .procname = "foo",
> > > > + .data = &test_data.int_0001,
> > > > + .maxlen = sizeof(int),
> > > > + .mode = 0644,
> > > > + .proc_handler = proc_dointvec,
> > > > + .extra1 = &i_zero,
> > > > + .extra2 = &i_one_hundred,
> > > > + };
> > > > + char input[] = "-9";
> > > > + size_t len = sizeof(input) - 1;
> > > > + loff_t pos = 0;
> > > > +
> > > > + table.data = kunit_kzalloc(test, sizeof(int), GFP_USER);
> > > > + KUNIT_EXPECT_EQ(test, 0, proc_dointvec(&table, 1, input, &len, &pos));
> > > > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, len);
> > > > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, pos);
> > > > + KUNIT_EXPECT_EQ(test, -9, *(int *)table.data);
> > >
> > > Is the casting necessary? Or can the macro do a type coercion of the
> > > second parameter based on the first type?
> > Data field is defined as void* so I believe casting is necessary to
> > dereference it as a pointer to an array of ints. I don't think the
> > macro should do any type coercion that == operator wouldn't do.
> > I did change the cast to make it more clear that it's a pointer to an
> > array of ints being dereferenced.
>
> Ok, I still wonder if we should make KUNIT_EXPECT_EQ check the types on
> both sides and cause a build warning/error if the types aren't the same.
> This would be similar to our min/max macros that complain about
> mismatched types in the comparisons. Then if a test developer needs to
> convert one type or the other they could do so with a
> KUNIT_EXPECT_EQ_T() macro that lists the types to coerce both sides to
> explicitly.
Do you think it would be better to do a phony compare similar to how
min/max used to work prior to 4.17, or to use the new __typecheck(...)
macro? This might seem like a dumb question (and maybe it is), but Iurii
and I thought the former created an error message that was a bit easier
to understand, whereas __typecheck is obviously superior in terms of
code reuse.
This is what we are thinking right now; if you don't have any complaints
I will squash it into the relevant commits on the next revision:
---
From: Iurii Zaikin <yzaikin@google.com>
Adds a warning message when comparing values of different types similar
to what min() / max() macros do.
Signed-off-by: Iurii Zaikin <yzaikin@google.com>
---
include/kunit/test.h | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/include/kunit/test.h b/include/kunit/test.h
index 511c9e85401a6..791e22fba5620 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -335,6 +335,13 @@ void __printf(3, 4) kunit_printk(const char *level,
#define kunit_err(test, fmt, ...) \
kunit_printk(KERN_ERR, test, fmt, ##__VA_ARGS__)
+/*
+ * 'Unnecessary' cast serves to generate a compile-time warning in case
+ * of comparing incompatible types. Inspired by include/linux/kernel.h
+ */
+#define __kunit_typecheck(lhs, rhs) \
+ ((void) (&(lhs) == &(rhs)))
+
static inline struct kunit_stream *kunit_expect_start(struct kunit *test,
const char *file,
const char *line)
@@ -514,6 +521,7 @@ static inline void kunit_expect_ptr_binary(struct kunit *test,
#define KUNIT_EXPECT_BINARY(test, left, condition, right) do { \
typeof(left) __left = (left); \
typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
kunit_expect_binary(test, \
(long long) __left, #left, \
(long long) __right, #right, \
@@ -524,6 +532,7 @@ static inline void kunit_expect_ptr_binary(struct kunit *test,
#define KUNIT_EXPECT_BINARY_MSG(test, left, condition, right, fmt, ...) do { \
typeof(left) __left = (left); \
typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
kunit_expect_binary_msg(test, \
(long long) __left, #left, \
(long long) __right, #right, \
@@ -538,6 +547,7 @@ static inline void kunit_expect_ptr_binary(struct kunit *test,
#define KUNIT_EXPECT_PTR_BINARY(test, left, condition, right) do { \
typeof(left) __left = (left); \
typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
kunit_expect_ptr_binary(test, \
(void *) __left, #left, \
(void *) __right, #right, \
@@ -553,6 +563,7 @@ static inline void kunit_expect_ptr_binary(struct kunit *test,
...) do { \
typeof(left) __left = (left); \
typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
kunit_expect_ptr_binary_msg(test, \
(void *) __left, #left, \
(void *) __right, #right, \
@@ -1013,6 +1024,7 @@ static inline void kunit_assert_ptr_binary(struct kunit *test,
#define KUNIT_ASSERT_BINARY(test, left, condition, right) do { \
typeof(left) __left = (left); \
typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
kunit_assert_binary(test, \
(long long) __left, #left, \
(long long) __right, #right, \
@@ -1023,6 +1035,7 @@ static inline void kunit_assert_ptr_binary(struct kunit *test,
#define KUNIT_ASSERT_BINARY_MSG(test, left, condition, right, fmt, ...) do { \
typeof(left) __left = (left); \
typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
kunit_assert_binary_msg(test, \
(long long) __left, #left, \
(long long) __right, #right, \
@@ -1037,6 +1050,7 @@ static inline void kunit_assert_ptr_binary(struct kunit *test,
#define KUNIT_ASSERT_PTR_BINARY(test, left, condition, right) do { \
typeof(left) __left = (left); \
typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
kunit_assert_ptr_binary(test, \
(void *) __left, #left, \
(void *) __right, #right, \
@@ -1051,6 +1065,7 @@ static inline void kunit_assert_ptr_binary(struct kunit *test,
fmt, ...) do { \
typeof(left) __left = (left); \
typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
kunit_assert_ptr_binary_msg(test, \
(void *) __left, #left, \
(void *) __right, #right, \
--
2.22.0.rc2.383.gf4fbbf30c2-goog
^ permalink raw reply related
* Re: [PATCH v7 25/27] mm/mmap: Add Shadow stack pages to memory accounting
From: Dave Hansen @ 2019-06-11 17:55 UTC (permalink / raw)
To: Yu-cheng Yu, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Peter Zijlstra, Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue,
Dave Martin
In-Reply-To: <20190606200646.3951-26-yu-cheng.yu@intel.com>
On 6/6/19 1:06 PM, Yu-cheng Yu wrote:
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1703,6 +1703,9 @@ static inline int accountable_mapping(struct file *file, vm_flags_t vm_flags)
> if (file && is_file_hugepages(file))
> return 0;
>
> + if (arch_copy_pte_mapping(vm_flags))
> + return 1;
> +
> return (vm_flags & (VM_NORESERVE | VM_SHARED | VM_WRITE)) == VM_WRITE;
> }
>
> @@ -3319,6 +3322,8 @@ void vm_stat_account(struct mm_struct *mm, vm_flags_t flags, long npages)
> mm->stack_vm += npages;
> else if (is_data_mapping(flags))
> mm->data_vm += npages;
> + else if (arch_copy_pte_mapping(flags))
> + mm->data_vm += npages;
> }
This classifies shadow stack as data instead of stack. That seems a wee
bit counterintuitive. Why did you make this choice?
^ permalink raw reply
* Re: [PATCH v3 06/20] docs: mark orphan documents as such
From: Mauro Carvalho Chehab @ 2019-06-11 17:05 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab,
Linux Kernel Mailing List, Jonathan Corbet, Frederic Barrat,
Andrew Donnellan, Maxime Coquelin, Alexandre Torgue, David Airlie,
Daniel Vetter, Maarten Lankhorst, Maxime Ripard, Sean Paul,
Georgi Djakov, Matan Ziv-Av, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman,
open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, linux-stm32,
linux-arm Mailing List, dri-devel, Linux PM, Platform Driver
In-Reply-To: <CAHp75VfTNJOGZx-PoUXLRvzghqf6bVUdJ+yFjE9hNtDLCQ1=UA@mail.gmail.com>
Em Tue, 11 Jun 2019 19:52:04 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> escreveu:
> On Fri, Jun 7, 2019 at 10:04 PM Mauro Carvalho Chehab
> <mchehab+samsung@kernel.org> wrote:
> > Sphinx doesn't like orphan documents:
>
> > Documentation/laptops/lg-laptop.rst: WARNING: document isn't included in any toctree
>
> > Documentation/laptops/lg-laptop.rst | 2 ++
>
> > diff --git a/Documentation/laptops/lg-laptop.rst b/Documentation/laptops/lg-laptop.rst
> > index aa503ee9b3bc..f2c2ffe31101 100644
> > --- a/Documentation/laptops/lg-laptop.rst
> > +++ b/Documentation/laptops/lg-laptop.rst
> > @@ -1,5 +1,7 @@
> > .. SPDX-License-Identifier: GPL-2.0+
> >
> > +:orphan:
> > +
> > LG Gram laptop extra features
> > =============================
> >
>
> Can we rather create a toc tree there?
> It was a first document in reST format in that folder.
Sure, but:
1) I have a patch converting the other files on this dir to rst:
https://git.linuxtv.org/mchehab/experimental.git/commit/?h=convert_rst_renames_v4.1&id=abc13233035fdfdbc5ef2f2fbd3d127a1ab15530
2) It probably makes sense to move the entire dir to
Documentation/admin-guide.
So, I would prefer to have the :orphan: here while (1) is not merged.
Thanks,
Mauro
^ permalink raw reply
* Re: [PATCH v3 06/20] docs: mark orphan documents as such
From: Andy Shevchenko @ 2019-06-11 16:52 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab,
Linux Kernel Mailing List, Jonathan Corbet, Frederic Barrat,
Andrew Donnellan, Maxime Coquelin, Alexandre Torgue, David Airlie,
Daniel Vetter, Maarten Lankhorst, Maxime Ripard, Sean Paul,
Georgi Djakov, Matan Ziv-Av, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman,
open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, linux-stm32,
linux-arm Mailing List, dri-devel, Linux PM, Platform Driver
In-Reply-To: <0bea1c7c4fc06c7edabbf3185c0cbbc6e85eafd0.1559933665.git.mchehab+samsung@kernel.org>
On Fri, Jun 7, 2019 at 10:04 PM Mauro Carvalho Chehab
<mchehab+samsung@kernel.org> wrote:
> Sphinx doesn't like orphan documents:
> Documentation/laptops/lg-laptop.rst: WARNING: document isn't included in any toctree
> Documentation/laptops/lg-laptop.rst | 2 ++
> diff --git a/Documentation/laptops/lg-laptop.rst b/Documentation/laptops/lg-laptop.rst
> index aa503ee9b3bc..f2c2ffe31101 100644
> --- a/Documentation/laptops/lg-laptop.rst
> +++ b/Documentation/laptops/lg-laptop.rst
> @@ -1,5 +1,7 @@
> .. SPDX-License-Identifier: GPL-2.0+
>
> +:orphan:
> +
> LG Gram laptop extra features
> =============================
>
Can we rather create a toc tree there?
It was a first document in reST format in that folder.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v5 05/11] drm: Add helpers to kick off self refresh mode in drivers
From: Sean Paul @ 2019-06-11 16:08 UTC (permalink / raw)
To: dri-devel
Cc: Sean Paul, Daniel Vetter, Jose Souza, Zain Wang, Tomasz Figa,
Ville Syrjälä, Heiko Stuebner, David Airlie,
Maarten Lankhorst, Maxime Ripard, Sean Paul, Jonathan Corbet,
linux-doc
In-Reply-To: <20190611160844.257498-1-sean@poorly.run>
From: Sean Paul <seanpaul@chromium.org>
This patch adds a new drm helper library to help drivers implement
self refresh. Drivers choosing to use it will register crtcs and
will receive callbacks when it's time to enter or exit self refresh
mode.
In its current form, it has a timer which will trigger after a
driver-specified amount of inactivity. When the timer triggers, the
helpers will submit a new atomic commit to shut the refreshing pipe
off. On the next atomic commit, the drm core will revert the self
refresh state and bring everything back up to be actively driven.
From the driver's perspective, this works like a regular disable/enable
cycle. The driver need only check the 'self_refresh_active' state in
crtc_state. It should initiate self refresh mode on the panel and enter
an off or low-power state.
Changes in v2:
- s/psr/self_refresh/ (Daniel)
- integrated the psr exit into the commit that wakes it up (Jose/Daniel)
- made the psr state per-crtc (Jose/Daniel)
Changes in v3:
- Remove the self_refresh_(active|changed) from connector state (Daniel)
- Simplify loop in drm_self_refresh_helper_alter_state (Daniel)
- Improve self_refresh_aware comment (Daniel)
- s/self_refresh_state/self_refresh_data/ (Daniel)
Changes in v4:
- Move docbook location below panel (Daniel)
- Improve docbook with references and more detailed explanation (Daniel)
- Instead of register/unregister, use init/cleanup (Daniel)
Changes in v5:
- Resolved conflict in drm_atomic_helper.c #include block
- Resolved conflict in rst with HDCP helper docs
Link to v1: https://patchwork.freedesktop.org/patch/msgid/20190228210939.83386-2-sean@poorly.run
Link to v2: https://patchwork.freedesktop.org/patch/msgid/20190326204509.96515-1-sean@poorly.run
Link to v3: https://patchwork.freedesktop.org/patch/msgid/20190502194956.218441-6-sean@poorly.run
Link to v4: https://patchwork.freedesktop.org/patch/msgid/20190508160920.144739-6-sean@poorly.run
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Jose Souza <jose.souza@intel.com>
Cc: Zain Wang <wzz@rock-chips.com>
Cc: Tomasz Figa <tfiga@chromium.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
Documentation/gpu/drm-kms-helpers.rst | 9 +
drivers/gpu/drm/Makefile | 2 +-
drivers/gpu/drm/drm_atomic.c | 2 +
drivers/gpu/drm/drm_atomic_helper.c | 35 +++-
drivers/gpu/drm/drm_atomic_state_helper.c | 4 +
drivers/gpu/drm/drm_atomic_uapi.c | 7 +-
drivers/gpu/drm/drm_self_refresh_helper.c | 213 ++++++++++++++++++++++
include/drm/drm_atomic.h | 15 ++
include/drm/drm_connector.h | 14 ++
include/drm/drm_crtc.h | 19 ++
include/drm/drm_self_refresh_helper.h | 22 +++
11 files changed, 337 insertions(+), 5 deletions(-)
create mode 100644 drivers/gpu/drm/drm_self_refresh_helper.c
create mode 100644 include/drm/drm_self_refresh_helper.h
diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index 0fe726a6ee678..b327bbc111821 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -181,6 +181,15 @@ Panel Helper Reference
.. kernel-doc:: drivers/gpu/drm/drm_panel_orientation_quirks.c
:export:
+Panel Self Refresh Helper Reference
+===================================
+
+.. kernel-doc:: drivers/gpu/drm/drm_self_refresh_helper.c
+ :doc: overview
+
+.. kernel-doc:: drivers/gpu/drm/drm_self_refresh_helper.c
+ :export:
+
HDCP Helper Functions Reference
===============================
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index d36feb4a62330..9d630a28a7880 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -43,7 +43,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_dsc.o drm_probe_helper
drm_simple_kms_helper.o drm_modeset_helper.o \
drm_scdc_helper.o drm_gem_framebuffer_helper.o \
drm_atomic_state_helper.o drm_damage_helper.o \
- drm_format_helper.o
+ drm_format_helper.o drm_self_refresh_helper.o
drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index a4e779deab0fb..419381abbdd16 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -384,6 +384,7 @@ static void drm_atomic_crtc_print_state(struct drm_printer *p,
drm_printf(p, "crtc[%u]: %s\n", crtc->base.id, crtc->name);
drm_printf(p, "\tenable=%d\n", state->enable);
drm_printf(p, "\tactive=%d\n", state->active);
+ drm_printf(p, "\tself_refresh_active=%d\n", state->self_refresh_active);
drm_printf(p, "\tplanes_changed=%d\n", state->planes_changed);
drm_printf(p, "\tmode_changed=%d\n", state->mode_changed);
drm_printf(p, "\tactive_changed=%d\n", state->active_changed);
@@ -999,6 +1000,7 @@ static void drm_atomic_connector_print_state(struct drm_printer *p,
drm_printf(p, "connector[%u]: %s\n", connector->base.id, connector->name);
drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name : "(null)");
+ drm_printf(p, "\tself_refresh_aware=%d\n", state->self_refresh_aware);
if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
if (state->writeback_job && state->writeback_job->fb)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index e58be69960692..bc43cc08cdc11 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -33,6 +33,7 @@
#include <drm/drm_damage_helper.h>
#include <drm/drm_device.h>
#include <drm/drm_plane_helper.h>
+#include <drm/drm_self_refresh_helper.h>
#include <drm/drm_print.h>
#include <drm/drm_vblank.h>
#include <drm/drm_writeback.h>
@@ -953,10 +954,33 @@ int drm_atomic_helper_check(struct drm_device *dev,
if (state->legacy_cursor_update)
state->async_update = !drm_atomic_helper_async_check(dev, state);
+ drm_self_refresh_helper_alter_state(state);
+
return ret;
}
EXPORT_SYMBOL(drm_atomic_helper_check);
+static bool
+crtc_needs_disable(struct drm_crtc_state *old_state,
+ struct drm_crtc_state *new_state)
+{
+ /*
+ * No new_state means the crtc is off, so the only criteria is whether
+ * it's currently active or in self refresh mode.
+ */
+ if (!new_state)
+ return drm_atomic_crtc_effectively_active(old_state);
+
+ /*
+ * We need to run through the crtc_funcs->disable() function if the crtc
+ * is currently on, if it's transitioning to self refresh mode, or if
+ * it's in self refresh mode and needs to be fully disabled.
+ */
+ return old_state->active ||
+ (old_state->self_refresh_active && !new_state->enable) ||
+ new_state->self_refresh_active;
+}
+
static void
disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
{
@@ -977,7 +1001,14 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
old_crtc_state = drm_atomic_get_old_crtc_state(old_state, old_conn_state->crtc);
- if (!old_crtc_state->active ||
+ if (new_conn_state->crtc)
+ new_crtc_state = drm_atomic_get_new_crtc_state(
+ old_state,
+ new_conn_state->crtc);
+ else
+ new_crtc_state = NULL;
+
+ if (!crtc_needs_disable(old_crtc_state, new_crtc_state) ||
!drm_atomic_crtc_needs_modeset(old_conn_state->crtc->state))
continue;
@@ -1023,7 +1054,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
continue;
- if (!old_crtc_state->active)
+ if (!crtc_needs_disable(old_crtc_state, new_crtc_state))
continue;
funcs = crtc->helper_private;
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index 97ab26679b969..7d7347a6f1943 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -129,6 +129,10 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
state->commit = NULL;
state->event = NULL;
state->pageflip_flags = 0;
+
+ /* Self refresh should be canceled when a new update is available */
+ state->active = drm_atomic_crtc_effectively_active(state);
+ state->self_refresh_active = false;
}
EXPORT_SYMBOL(__drm_atomic_helper_crtc_duplicate_state);
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index eb22e8bdd8531..abe38bdf85ae6 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -490,7 +490,7 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
struct drm_mode_config *config = &dev->mode_config;
if (property == config->prop_active)
- *val = state->active;
+ *val = drm_atomic_crtc_effectively_active(state);
else if (property == config->prop_mode_id)
*val = (state->mode_blob) ? state->mode_blob->base.id : 0;
else if (property == config->prop_vrr_enabled)
@@ -788,7 +788,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
if (property == config->prop_crtc_id) {
*val = (state->crtc) ? state->crtc->base.id : 0;
} else if (property == config->dpms_property) {
- *val = connector->dpms;
+ if (state->crtc && state->crtc->state->self_refresh_active)
+ *val = DRM_MODE_DPMS_ON;
+ else
+ *val = connector->dpms;
} else if (property == config->tv_select_subconnector_property) {
*val = state->tv.subconnector;
} else if (property == config->tv_left_margin_property) {
diff --git a/drivers/gpu/drm/drm_self_refresh_helper.c b/drivers/gpu/drm/drm_self_refresh_helper.c
new file mode 100644
index 0000000000000..c7083c5694fee
--- /dev/null
+++ b/drivers/gpu/drm/drm_self_refresh_helper.c
@@ -0,0 +1,213 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright (C) 2019 Google, Inc.
+ *
+ * Authors:
+ * Sean Paul <seanpaul@chromium.org>
+ */
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_connector.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_device.h>
+#include <drm/drm_mode_config.h>
+#include <drm/drm_modeset_lock.h>
+#include <drm/drm_print.h>
+#include <drm/drm_self_refresh_helper.h>
+#include <linux/bitops.h>
+#include <linux/slab.h>
+#include <linux/workqueue.h>
+
+/**
+ * DOC: overview
+ *
+ * This helper library provides an easy way for drivers to leverage the atomic
+ * framework to implement panel self refresh (SR) support. Drivers are
+ * responsible for initializing and cleaning up the SR helpers on load/unload
+ * (see &drm_self_refresh_helper_init/&drm_self_refresh_helper_cleanup).
+ * The connector is responsible for setting
+ * &drm_connector_state.self_refresh_aware to true at runtime if it is SR-aware
+ * (meaning it knows how to initiate self refresh on the panel).
+ *
+ * Once a crtc has enabled SR using &drm_self_refresh_helper_init, the
+ * helpers will monitor activity and call back into the driver to enable/disable
+ * SR as appropriate. The best way to think about this is that it's a DPMS
+ * on/off request with &drm_crtc_state.self_refresh_active set in crtc state
+ * that tells you to disable/enable SR on the panel instead of power-cycling it.
+ *
+ * During SR, drivers may choose to fully disable their crtc/encoder/bridge
+ * hardware (in which case no driver changes are necessary), or they can inspect
+ * &drm_crtc_state.self_refresh_active if they want to enter low power mode
+ * without full disable (in case full disable/enable is too slow).
+ *
+ * SR will be deactivated if there are any atomic updates affecting the
+ * pipe that is in SR mode. If a crtc is driving multiple connectors, all
+ * connectors must be SR aware and all will enter/exit SR mode at the same time.
+ *
+ * If the crtc and connector are SR aware, but the panel connected does not
+ * support it (or is otherwise unable to enter SR), the driver should fail
+ * atomic_check when &drm_crtc_state.self_refresh_active is true.
+ */
+
+struct drm_self_refresh_data {
+ struct drm_crtc *crtc;
+ struct delayed_work entry_work;
+ struct drm_atomic_state *save_state;
+ unsigned int entry_delay_ms;
+};
+
+static void drm_self_refresh_helper_entry_work(struct work_struct *work)
+{
+ struct drm_self_refresh_data *sr_data = container_of(
+ to_delayed_work(work),
+ struct drm_self_refresh_data, entry_work);
+ struct drm_crtc *crtc = sr_data->crtc;
+ struct drm_device *dev = crtc->dev;
+ struct drm_modeset_acquire_ctx ctx;
+ struct drm_atomic_state *state;
+ struct drm_connector *conn;
+ struct drm_connector_state *conn_state;
+ struct drm_crtc_state *crtc_state;
+ int i, ret;
+
+ drm_modeset_acquire_init(&ctx, 0);
+
+ state = drm_atomic_state_alloc(dev);
+ if (!state) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+retry:
+ state->acquire_ctx = &ctx;
+
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state)) {
+ ret = PTR_ERR(crtc_state);
+ goto out;
+ }
+
+ if (!crtc_state->enable)
+ goto out;
+
+ ret = drm_atomic_add_affected_connectors(state, crtc);
+ if (ret)
+ goto out;
+
+ for_each_new_connector_in_state(state, conn, conn_state, i) {
+ if (!conn_state->self_refresh_aware)
+ goto out;
+ }
+
+ crtc_state->active = false;
+ crtc_state->self_refresh_active = true;
+
+ ret = drm_atomic_commit(state);
+ if (ret)
+ goto out;
+
+out:
+ if (ret == -EDEADLK) {
+ drm_atomic_state_clear(state);
+ ret = drm_modeset_backoff(&ctx);
+ if (!ret)
+ goto retry;
+ }
+
+ drm_atomic_state_put(state);
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
+}
+
+/**
+ * drm_self_refresh_helper_alter_state - Alters the atomic state for SR exit
+ * @state: the state currently being checked
+ *
+ * Called at the end of atomic check. This function checks the state for flags
+ * incompatible with self refresh exit and changes them. This is a bit
+ * disingenuous since userspace is expecting one thing and we're giving it
+ * another. However in order to keep self refresh entirely hidden from
+ * userspace, this is required.
+ *
+ * At the end, we queue up the self refresh entry work so we can enter PSR after
+ * the desired delay.
+ */
+void drm_self_refresh_helper_alter_state(struct drm_atomic_state *state)
+{
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *crtc_state;
+ int i;
+
+ if (state->async_update || !state->allow_modeset) {
+ for_each_old_crtc_in_state(state, crtc, crtc_state, i) {
+ if (crtc_state->self_refresh_active) {
+ state->async_update = false;
+ state->allow_modeset = true;
+ break;
+ }
+ }
+ }
+
+ for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
+ struct drm_self_refresh_data *sr_data;
+
+ /* Don't trigger the entry timer when we're already in SR */
+ if (crtc_state->self_refresh_active)
+ continue;
+
+ sr_data = crtc->self_refresh_data;
+ if (!sr_data)
+ continue;
+
+ mod_delayed_work(system_wq, &sr_data->entry_work,
+ msecs_to_jiffies(sr_data->entry_delay_ms));
+ }
+}
+EXPORT_SYMBOL(drm_self_refresh_helper_alter_state);
+
+/**
+ * drm_self_refresh_helper_init - Initializes self refresh helpers for a crtc
+ * @crtc: the crtc which supports self refresh supported displays
+ * @entry_delay_ms: amount of inactivity to wait before entering self refresh
+ */
+int drm_self_refresh_helper_init(struct drm_crtc *crtc,
+ unsigned int entry_delay_ms)
+{
+ struct drm_self_refresh_data *sr_data = crtc->self_refresh_data;
+
+ /* Helper is already initialized */
+ if (WARN_ON(sr_data))
+ return -EINVAL;
+
+ sr_data = kzalloc(sizeof(*sr_data), GFP_KERNEL);
+ if (!sr_data)
+ return -ENOMEM;
+
+ INIT_DELAYED_WORK(&sr_data->entry_work,
+ drm_self_refresh_helper_entry_work);
+ sr_data->entry_delay_ms = entry_delay_ms;
+ sr_data->crtc = crtc;
+
+ crtc->self_refresh_data = sr_data;
+ return 0;
+}
+EXPORT_SYMBOL(drm_self_refresh_helper_init);
+
+/**
+ * drm_self_refresh_helper_cleanup - Cleans up self refresh helpers for a crtc
+ * @crtc: the crtc to cleanup
+ */
+void drm_self_refresh_helper_cleanup(struct drm_crtc *crtc)
+{
+ struct drm_self_refresh_data *sr_data = crtc->self_refresh_data;
+
+ /* Helper is already uninitialized */
+ if (sr_data)
+ return;
+
+ crtc->self_refresh_data = NULL;
+
+ cancel_delayed_work_sync(&sr_data->entry_work);
+ kfree(sr_data);
+}
+EXPORT_SYMBOL(drm_self_refresh_helper_cleanup);
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index f122156478010..927e1205d7aa4 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -957,4 +957,19 @@ drm_atomic_crtc_needs_modeset(const struct drm_crtc_state *state)
state->connectors_changed;
}
+/**
+ * drm_atomic_crtc_effectively_active - compute whether crtc is actually active
+ * @state: &drm_crtc_state for the CRTC
+ *
+ * When in self refresh mode, the crtc_state->active value will be false, since
+ * the crtc is off. However in some cases we're interested in whether the crtc
+ * is active, or effectively active (ie: it's connected to an active display).
+ * In these cases, use this function instead of just checking active.
+ */
+static inline bool
+drm_atomic_crtc_effectively_active(const struct drm_crtc_state *state)
+{
+ return state->active || state->self_refresh_active;
+}
+
#endif /* DRM_ATOMIC_H_ */
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 071143bc0ebd1..c6f8486d8b8f3 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -548,6 +548,20 @@ struct drm_connector_state {
/** @tv: TV connector state */
struct drm_tv_connector_state tv;
+ /**
+ * @self_refresh_aware:
+ *
+ * This tracks whether a connector is aware of the self refresh state.
+ * It should be set to true for those connector implementations which
+ * understand the self refresh state. This is needed since the crtc
+ * registers the self refresh helpers and it doesn't know if the
+ * connectors downstream have implemented self refresh entry/exit.
+ *
+ * Drivers should set this to true in atomic_check if they know how to
+ * handle self_refresh requests.
+ */
+ bool self_refresh_aware;
+
/**
* @picture_aspect_ratio: Connector property to control the
* HDMI infoframe aspect ratio setting.
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index dc42b9e353333..4fda610d76f26 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -54,6 +54,7 @@ struct drm_mode_set;
struct drm_file;
struct drm_clip_rect;
struct drm_printer;
+struct drm_self_refresh_data;
struct device_node;
struct dma_fence;
struct edid;
@@ -300,6 +301,17 @@ struct drm_crtc_state {
*/
bool vrr_enabled;
+ /**
+ * @self_refresh_active:
+ *
+ * Used by the self refresh helpers to denote when a self refresh
+ * transition is occuring. This will be set on enable/disable callbacks
+ * when self refresh is being enabled or disabled. In some cases, it may
+ * not be desirable to fully shut off the crtc during self refresh.
+ * CRTC's can inspect this flag and determine the best course of action.
+ */
+ bool self_refresh_active;
+
/**
* @event:
*
@@ -1088,6 +1100,13 @@ struct drm_crtc {
* The name of the CRTC's fence timeline.
*/
char timeline_name[32];
+
+ /**
+ * @self_refresh_data: Holds the state for the self refresh helpers
+ *
+ * Initialized via drm_self_refresh_helper_register().
+ */
+ struct drm_self_refresh_data *self_refresh_data;
};
/**
diff --git a/include/drm/drm_self_refresh_helper.h b/include/drm/drm_self_refresh_helper.h
new file mode 100644
index 0000000000000..405e86fb8461e
--- /dev/null
+++ b/include/drm/drm_self_refresh_helper.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright (C) 2019 Google, Inc.
+ *
+ * Authors:
+ * Sean Paul <seanpaul@chromium.org>
+ */
+#ifndef DRM_SELF_REFRESH_HELPER_H_
+#define DRM_SELF_REFRESH_HELPER_H_
+
+struct drm_atomic_state;
+struct drm_connector;
+struct drm_device;
+struct drm_modeset_acquire_ctx;
+
+void drm_self_refresh_helper_alter_state(struct drm_atomic_state *state);
+
+int drm_self_refresh_helper_init(struct drm_crtc *crtc,
+ unsigned int entry_delay_ms);
+
+void drm_self_refresh_helper_cleanup(struct drm_crtc *crtc);
+#endif
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply related
* Re: [PATCH v3 33/33] docs: EDID/HOWTO.txt: convert it and rename to howto.rst
From: Jonathan Corbet @ 2019-06-11 15:37 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Daniel Vetter, Linux Doc Mailing List, Mauro Carvalho Chehab,
linux-kernel, Maarten Lankhorst, Maxime Ripard, Sean Paul,
David Airlie, dri-devel
In-Reply-To: <20190611060215.232af2bb@coco.lan>
On Tue, 11 Jun 2019 06:02:15 -0300
Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> Jon, please correct me if I' wrong, bu I guess the plan is to place them
> somewhere under Documentation/admin-guide/.
That makes sense to me.
> If so, perhaps creating a Documentation/admin-guide/drm dir there and
> place docs like EDID/HOWTO.txt, svga.txt, etc would work.
Maybe "graphics" or "display" rather than "drm", which may not entirely
applicable to all of those docs or as familiar to all admins?
> Btw, that's one of the reasons[1] why I opted to keep the files where they
> are: properly organizing the converted documents call for such kind
> of discussions. On my experience, discussing names and directory locations
> can generate warm discussions and take a lot of time to reach consensus.
Moving docs is a pain; my life would certainly be easier if I were happy
to just let everything lie where it fell :) But it's far from the hardest
problem we solve in kernel development, I assume we can figure it out.
Thanks,
jon
^ permalink raw reply
* Re: [PATCH v12 03/13] dt-bindings: Add doc for the Ingenic TCU drivers
From: Rob Herring @ 2019-06-11 14:57 UTC (permalink / raw)
To: Paul Cercueil
Cc: Mark Rutland, Ralf Baechle, Paul Burton, James Hogan,
Jonathan Corbet, Daniel Lezcano, Thomas Gleixner,
Michael Turquette, Stephen Boyd, Jason Cooper, Marc Zyngier,
Lee Jones, Mathieu Malaterre, linux-kernel@vger.kernel.org,
devicetree, linux-mips, Linux Doc Mailing List, linux-clk, od
In-Reply-To: <1558811596.2016.1@crapouillou.net>
On Sat, May 25, 2019 at 1:13 PM Paul Cercueil <paul@crapouillou.net> wrote:
>
>
>
> Le ven. 24 mai 2019 à 22:21, Rob Herring <robh@kernel.org> a écrit :
> > On Tue, May 21, 2019 at 04:51:31PM +0200, Paul Cercueil wrote:
> >> Add documentation about how to properly use the Ingenic TCU
> >> (Timer/Counter Unit) drivers from devicetree.
> >>
> >> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> >> ---
> >>
> >> Notes:
> >> v4: New patch in this series. Corresponds to V2 patches 3-4-5
> >> with
> >> added content.
> >>
> >> v5: - Edited PWM/watchdog DT bindings documentation to point to
> >> the new
> >> document.
> >> - Moved main document to
> >> Documentation/devicetree/bindings/timer/ingenic,tcu.txt
> >> - Updated documentation to reflect the new devicetree bindings.
> >>
> >> v6: - Removed PWM/watchdog documentation files as asked by
> >> upstream
> >> - Removed doc about properties that should be implicit
> >> - Removed doc about ingenic,timer-channel /
> >> ingenic,clocksource-channel as they are gone
> >> - Fix WDT clock name in the binding doc
> >> - Fix lengths of register areas in watchdog/pwm nodes
> >>
> >> v7: No change
> >>
> >> v8: - Fix address of the PWM node
> >> - Added doc about system timer and clocksource children nodes
> >>
> >> v9: - Remove doc about system timer and clocksource children
> >> nodes...
> >> - Add doc about ingenic,pwm-channels-mask property
> >>
> >> v10: No change
> >>
> >> v11: Fix info about default value of ingenic,pwm-channels-mask
> >>
> >> v12: Drop sub-nodes for now; they will be introduced in a
> >> follow-up
> >> patchset.
> >
> > Why? I believe I acked them.
>
> The patchset was too big, and I've already been trying to get it
> upstream for
> more than one year now. So I cut it in half in hope that it'll be
> easier to
> upstream it that way.
You can drop the driver part and keep the binding. Unlike drivers, we
don't want bindings to needlessly evolve, and you don't have to wait
til a driver implements some functionality to add that to the binding.
Rob
^ permalink raw reply
* RE: [PATCH V14 2/5] thermal: of-thermal: add API for getting sensor ID from DT
From: Anson Huang @ 2019-06-11 12:37 UTC (permalink / raw)
To: Aisheng Dong, robh+dt@kernel.org, mark.rutland@arm.com,
corbet@lwn.net, shawnguo@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com,
catalin.marinas@arm.com, will.deacon@arm.com, rui.zhang@intel.com,
edubezval@gmail.com, daniel.lezcano@linaro.org,
ulf.hansson@linaro.org, Peng Fan, mchehab+samsung@kernel.org,
linux@roeck-us.net, Daniel Baluta, maxime.ripard@bootlin.com,
olof@lixom.net, jagan@amarulasolutions.com,
horms+renesas@verge.net.au, Leonard Crestez,
bjorn.andersson@linaro.org, dinguyen@kernel.org,
enric.balletbo@collabora.com, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org
Cc: dl-linux-imx
In-Reply-To: <AM0PR04MB42118F3F8E87A72758D4A7B780ED0@AM0PR04MB4211.eurprd04.prod.outlook.com>
Hi, Aisheng
> -----Original Message-----
> From: Aisheng Dong
> Sent: Tuesday, June 11, 2019 7:47 PM
> To: Anson Huang <anson.huang@nxp.com>; robh+dt@kernel.org;
> mark.rutland@arm.com; corbet@lwn.net; shawnguo@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> catalin.marinas@arm.com; will.deacon@arm.com; rui.zhang@intel.com;
> edubezval@gmail.com; daniel.lezcano@linaro.org; ulf.hansson@linaro.org;
> Peng Fan <peng.fan@nxp.com>; mchehab+samsung@kernel.org;
> linux@roeck-us.net; Daniel Baluta <daniel.baluta@nxp.com>;
> maxime.ripard@bootlin.com; olof@lixom.net; jagan@amarulasolutions.com;
> horms+renesas@verge.net.au; Leonard Crestez <leonard.crestez@nxp.com>;
> bjorn.andersson@linaro.org; dinguyen@kernel.org;
> enric.balletbo@collabora.com; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-doc@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-pm@vger.kernel.org
> Cc: dl-linux-imx <linux-imx@nxp.com>
> Subject: RE: [PATCH V14 2/5] thermal: of-thermal: add API for getting sensor
> ID from DT
>
> Hi Anson,
>
> The implementation looks good to me.
> A few minor comments on the doc:
This patch is pending for so long and even till now I am NOT sure if everyone is OK for
this change, so I will wait for some comments if there is any, and will improve the doc
together in next version.
Thanks,
Anson
>
> > From: Anson.Huang@nxp.com [mailto:Anson.Huang@nxp.com]
> > Sent: Monday, June 10, 2019 10:53 AM
> >
> > On some platforms like i.MX8QXP, the thermal driver needs a real HW
> > sensor ID from DT thermal zone, the HW sensor ID is used to get
> > temperature from SCU firmware, and the virtual sensor ID starting from
> > 0 to N is NOT used at all,
>
> This API is unware of HW ID or Virtual ID. So we probably no need to
> mention It here.
>
> > this patch adds new API thermal_zone_of_get_sensor_id() to provide the
> > feature of getting sensor ID from DT thermal zone's node.
> >
>
> "It's useful for thermal driver to register the specific thermal zone devices
> from DT in a common way."
>
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > Changes since V13:
> > - add new API into the thermal API doc.
> > ---
> > Documentation/thermal/sysfs-api.txt | 8 +++++
> > drivers/thermal/of-thermal.c | 66
> > ++++++++++++++++++++++++++++---------
> > include/linux/thermal.h | 10 ++++++
> > 3 files changed, 68 insertions(+), 16 deletions(-)
> >
> > diff --git a/Documentation/thermal/sysfs-api.txt
> > b/Documentation/thermal/sysfs-api.txt
> > index c3fa500..8d7f1b1 100644
> > --- a/Documentation/thermal/sysfs-api.txt
> > +++ b/Documentation/thermal/sysfs-api.txt
> > @@ -159,6 +159,14 @@ temperature) and throttle appropriate devices.
> > for the thermal zone device, which might be useful for platform
> > drivers for temperature calculations.
> >
> > +1.1.9 int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> > + struct device_node *sensor_np,
> > + u32 *id)
> > +
> > + This interface is used to get the sensor id from thermal sensor's
> > + phandle argument, it might be necessary for some platforms which
> > + have specific sensor ID rather than virtual ID from 0 - N.
>
> Does below one look better?
>
> "This interface is used to get the sensor id from the given thermal zone in DT,
> which might be useful for thermal drivers to register specific thermal zone
> device in a common way."
>
> > +
> > 1.2 thermal cooling device interface
> > 1.2.1 struct thermal_cooling_device
> > *thermal_cooling_device_register(char
> > *name,
> > void *devdata, struct thermal_cooling_device_ops *) diff --git
> > a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index
> > dc5093b..a53792b 100644
> > --- a/drivers/thermal/of-thermal.c
> > +++ b/drivers/thermal/of-thermal.c
> > @@ -449,6 +449,54 @@ thermal_zone_of_add_sensor(struct device_node
> > *zone, }
> >
> > /**
> > + * thermal_zone_of_get_sensor_id - get sensor ID from a DT thermal
> > + zone
> > + * @tz_np: a valid thermal zone device node.
> > + * @sensor_np: a sensor node of a valid sensor device.
> > + * @id: a sensor ID pointer will be passed back.
>
> the sensor ID returned if success
>
> > + *
> > + * This function will get sensor ID from a given thermal zone node,
> > + use
> > + * "thermal-sensors" as list name, and get sensor ID from first
> > + phandle's
> > + * argument.
>
> "This function will get sensor ID from a given thermal zone node and the
> sensor get must match the temperature providers @sensor_np."
>
> Regards
> Dong Aisheng
>
> > + *
> > + * Return: 0 on success, proper error code otherwise.
> > + */
> > +
> > +int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> > + struct device_node *sensor_np,
> > + u32 *id)
> > +{
> > + struct of_phandle_args sensor_specs;
> > + int ret;
> > +
> > + ret = of_parse_phandle_with_args(tz_np,
> > + "thermal-sensors",
> > + "#thermal-sensor-cells",
> > + 0,
> > + &sensor_specs);
> > + if (ret)
> > + return ret;
> > +
> > + if (sensor_specs.np != sensor_np) {
> > + of_node_put(sensor_specs.np);
> > + return -ENODEV;
> > + }
> > +
> > + if (sensor_specs.args_count >= 1) {
> > + *id = sensor_specs.args[0];
> > + WARN(sensor_specs.args_count > 1,
> > + "%pOFn: too many cells in sensor specifier %d\n",
> > + sensor_specs.np, sensor_specs.args_count);
> > + } else {
> > + *id = 0;
> > + }
> > +
> > + of_node_put(sensor_specs.np);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(thermal_zone_of_get_sensor_id);
> > +
> > +/**
> > * thermal_zone_of_sensor_register - registers a sensor to a DT thermal
> zone
> > * @dev: a valid struct device pointer of a sensor device. Must contain
> > * a valid .of_node, for the sensor node.
> > @@ -499,36 +547,22 @@ thermal_zone_of_sensor_register(struct device
> > *dev, int sensor_id, void *data,
> > sensor_np = of_node_get(dev->of_node);
> >
> > for_each_available_child_of_node(np, child) {
> > - struct of_phandle_args sensor_specs;
> > int ret, id;
> >
> > /* For now, thermal framework supports only 1 sensor per
> zone */
> > - ret = of_parse_phandle_with_args(child, "thermal-sensors",
> > - "#thermal-sensor-cells",
> > - 0, &sensor_specs);
> > + ret = thermal_zone_of_get_sensor_id(child, sensor_np, &id);
> > if (ret)
> > continue;
> >
> > - if (sensor_specs.args_count >= 1) {
> > - id = sensor_specs.args[0];
> > - WARN(sensor_specs.args_count > 1,
> > - "%pOFn: too many cells in sensor specifier %d\n",
> > - sensor_specs.np, sensor_specs.args_count);
> > - } else {
> > - id = 0;
> > - }
> > -
> > - if (sensor_specs.np == sensor_np && id == sensor_id) {
> > + if (id == sensor_id) {
> > tzd = thermal_zone_of_add_sensor(child, sensor_np,
> > data, ops);
> > if (!IS_ERR(tzd))
> > tzd->ops->set_mode(tzd,
> THERMAL_DEVICE_ENABLED);
> >
> > - of_node_put(sensor_specs.np);
> > of_node_put(child);
> > goto exit;
> > }
> > - of_node_put(sensor_specs.np);
> > }
> > exit:
> > of_node_put(sensor_np);
> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h index
> > 15a4ca5..5edffe6 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -375,6 +375,9 @@ struct thermal_trip {
> >
> > /* Function declarations */
> > #ifdef CONFIG_THERMAL_OF
> > +int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> > + struct device_node *sensor_np,
> > + u32 *id);
> > struct thermal_zone_device *
> > thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
> > const struct thermal_zone_of_device_ops
> *ops); @@
> > -386,6 +389,13 @@ struct thermal_zone_device
> > *devm_thermal_zone_of_sensor_register(
> > void devm_thermal_zone_of_sensor_unregister(struct device *dev,
> > struct thermal_zone_device *tz);
> #else
> > +
> > +static int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> > + struct device_node *sensor_np,
> > + u32 *id)
> > +{
> > + return -ENOENT;
> > +}
> > static inline struct thermal_zone_device *
> > thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
> > const struct thermal_zone_of_device_ops
> *ops)
> > --
> > 2.7.4
^ permalink raw reply
* RE: [PATCH V14 2/5] thermal: of-thermal: add API for getting sensor ID from DT
From: Aisheng Dong @ 2019-06-11 11:46 UTC (permalink / raw)
To: Anson Huang, robh+dt@kernel.org, mark.rutland@arm.com,
corbet@lwn.net, shawnguo@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com,
catalin.marinas@arm.com, will.deacon@arm.com, rui.zhang@intel.com,
edubezval@gmail.com, daniel.lezcano@linaro.org,
ulf.hansson@linaro.org, Peng Fan, mchehab+samsung@kernel.org,
linux@roeck-us.net, Daniel Baluta, maxime.ripard@bootlin.com,
olof@lixom.net, jagan@amarulasolutions.com,
horms+renesas@verge.net.au, Leonard Crestez,
bjorn.andersson@linaro.org, dinguyen@kernel.org,
enric.balletbo@collabora.com, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org
Cc: dl-linux-imx
In-Reply-To: <20190610025254.23940-2-Anson.Huang@nxp.com>
Hi Anson,
The implementation looks good to me.
A few minor comments on the doc:
> From: Anson.Huang@nxp.com [mailto:Anson.Huang@nxp.com]
> Sent: Monday, June 10, 2019 10:53 AM
>
> On some platforms like i.MX8QXP, the thermal driver needs a real HW sensor
> ID from DT thermal zone, the HW sensor ID is used to get temperature from
> SCU firmware, and the virtual sensor ID starting from 0 to N is NOT used at all,
This API is unware of HW ID or Virtual ID. So we probably no need to mention
It here.
> this patch adds new API thermal_zone_of_get_sensor_id() to provide the
> feature of getting sensor ID from DT thermal zone's node.
>
"It's useful for thermal driver to register the specific thermal zone devices
from DT in a common way."
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> Changes since V13:
> - add new API into the thermal API doc.
> ---
> Documentation/thermal/sysfs-api.txt | 8 +++++
> drivers/thermal/of-thermal.c | 66
> ++++++++++++++++++++++++++++---------
> include/linux/thermal.h | 10 ++++++
> 3 files changed, 68 insertions(+), 16 deletions(-)
>
> diff --git a/Documentation/thermal/sysfs-api.txt
> b/Documentation/thermal/sysfs-api.txt
> index c3fa500..8d7f1b1 100644
> --- a/Documentation/thermal/sysfs-api.txt
> +++ b/Documentation/thermal/sysfs-api.txt
> @@ -159,6 +159,14 @@ temperature) and throttle appropriate devices.
> for the thermal zone device, which might be useful for platform
> drivers for temperature calculations.
>
> +1.1.9 int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> + struct device_node *sensor_np,
> + u32 *id)
> +
> + This interface is used to get the sensor id from thermal sensor's
> + phandle argument, it might be necessary for some platforms which
> + have specific sensor ID rather than virtual ID from 0 - N.
Does below one look better?
"This interface is used to get the sensor id from the given thermal zone in
DT, which might be useful for thermal drivers to register specific thermal
zone device in a common way."
> +
> 1.2 thermal cooling device interface
> 1.2.1 struct thermal_cooling_device *thermal_cooling_device_register(char
> *name,
> void *devdata, struct thermal_cooling_device_ops *) diff --git
> a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index
> dc5093b..a53792b 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -449,6 +449,54 @@ thermal_zone_of_add_sensor(struct device_node
> *zone, }
>
> /**
> + * thermal_zone_of_get_sensor_id - get sensor ID from a DT thermal zone
> + * @tz_np: a valid thermal zone device node.
> + * @sensor_np: a sensor node of a valid sensor device.
> + * @id: a sensor ID pointer will be passed back.
the sensor ID returned if success
> + *
> + * This function will get sensor ID from a given thermal zone node, use
> + * "thermal-sensors" as list name, and get sensor ID from first
> + phandle's
> + * argument.
"This function will get sensor ID from a given thermal zone node and the sensor
get must match the temperature providers @sensor_np."
Regards
Dong Aisheng
> + *
> + * Return: 0 on success, proper error code otherwise.
> + */
> +
> +int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> + struct device_node *sensor_np,
> + u32 *id)
> +{
> + struct of_phandle_args sensor_specs;
> + int ret;
> +
> + ret = of_parse_phandle_with_args(tz_np,
> + "thermal-sensors",
> + "#thermal-sensor-cells",
> + 0,
> + &sensor_specs);
> + if (ret)
> + return ret;
> +
> + if (sensor_specs.np != sensor_np) {
> + of_node_put(sensor_specs.np);
> + return -ENODEV;
> + }
> +
> + if (sensor_specs.args_count >= 1) {
> + *id = sensor_specs.args[0];
> + WARN(sensor_specs.args_count > 1,
> + "%pOFn: too many cells in sensor specifier %d\n",
> + sensor_specs.np, sensor_specs.args_count);
> + } else {
> + *id = 0;
> + }
> +
> + of_node_put(sensor_specs.np);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(thermal_zone_of_get_sensor_id);
> +
> +/**
> * thermal_zone_of_sensor_register - registers a sensor to a DT thermal zone
> * @dev: a valid struct device pointer of a sensor device. Must contain
> * a valid .of_node, for the sensor node.
> @@ -499,36 +547,22 @@ thermal_zone_of_sensor_register(struct device *dev,
> int sensor_id, void *data,
> sensor_np = of_node_get(dev->of_node);
>
> for_each_available_child_of_node(np, child) {
> - struct of_phandle_args sensor_specs;
> int ret, id;
>
> /* For now, thermal framework supports only 1 sensor per zone */
> - ret = of_parse_phandle_with_args(child, "thermal-sensors",
> - "#thermal-sensor-cells",
> - 0, &sensor_specs);
> + ret = thermal_zone_of_get_sensor_id(child, sensor_np, &id);
> if (ret)
> continue;
>
> - if (sensor_specs.args_count >= 1) {
> - id = sensor_specs.args[0];
> - WARN(sensor_specs.args_count > 1,
> - "%pOFn: too many cells in sensor specifier %d\n",
> - sensor_specs.np, sensor_specs.args_count);
> - } else {
> - id = 0;
> - }
> -
> - if (sensor_specs.np == sensor_np && id == sensor_id) {
> + if (id == sensor_id) {
> tzd = thermal_zone_of_add_sensor(child, sensor_np,
> data, ops);
> if (!IS_ERR(tzd))
> tzd->ops->set_mode(tzd, THERMAL_DEVICE_ENABLED);
>
> - of_node_put(sensor_specs.np);
> of_node_put(child);
> goto exit;
> }
> - of_node_put(sensor_specs.np);
> }
> exit:
> of_node_put(sensor_np);
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h index
> 15a4ca5..5edffe6 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -375,6 +375,9 @@ struct thermal_trip {
>
> /* Function declarations */
> #ifdef CONFIG_THERMAL_OF
> +int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> + struct device_node *sensor_np,
> + u32 *id);
> struct thermal_zone_device *
> thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
> const struct thermal_zone_of_device_ops *ops); @@
> -386,6 +389,13 @@ struct thermal_zone_device
> *devm_thermal_zone_of_sensor_register(
> void devm_thermal_zone_of_sensor_unregister(struct device *dev,
> struct thermal_zone_device *tz); #else
> +
> +static int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> + struct device_node *sensor_np,
> + u32 *id)
> +{
> + return -ENOENT;
> +}
> static inline struct thermal_zone_device *
> thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
> const struct thermal_zone_of_device_ops *ops)
> --
> 2.7.4
^ permalink raw reply
* Re: [PATCH v7 22/27] binfmt_elf: Extract .note.gnu.property from an ELF file
From: Dave Martin @ 2019-06-11 11:41 UTC (permalink / raw)
To: Florian Weimer
Cc: Yu-cheng Yu, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue
In-Reply-To: <87lfy9cq04.fsf@oldenburg2.str.redhat.com>
On Mon, Jun 10, 2019 at 07:24:43PM +0200, Florian Weimer wrote:
> * Yu-cheng Yu:
>
> > To me, looking at PT_GNU_PROPERTY and not trying to support anything is a
> > logical choice. And it breaks only a limited set of toolchains.
> >
> > I will simplify the parser and leave this patch as-is for anyone who wants to
> > back-port. Are there any objections or concerns?
>
> Red Hat Enterprise Linux 8 does not use PT_GNU_PROPERTY and is probably
> the largest collection of CET-enabled binaries that exists today.
For clarity, RHEL is actively parsing these properties today?
> My hope was that we would backport the upstream kernel patches for CET,
> port the glibc dynamic loader to the new kernel interface, and be ready
> to run with CET enabled in principle (except that porting userspace
> libraries such as OpenSSL has not really started upstream, so many
> processes where CET is particularly desirable will still run without
> it).
>
> I'm not sure if it is a good idea to port the legacy support if it's not
> part of the mainline kernel because it comes awfully close to creating
> our own private ABI.
I guess we can aim to factor things so that PT_NOTE scanning is
available as a fallback on arches for which the absence of
PT_GNU_PROPERTY is not authoritative.
Can we argue that the lack of PT_GNU_PROPERTY is an ABI bug, fix it
for new binaries and hence limit the efforts we go to to support
theoretical binaries that lack the phdrs entry?
If we can make practical simplifications to the parsing, such as
limiting the maximum PT_NOTE size that we will search for the program
properties to 1K (say), or requiring NT_NOTE_GNU_PROPERTY_TYPE_0 to sit
by itself in a single PT_NOTE then that could help minimse the exec
overheads and the number of places for bugs to hide in the kernel.
What we can do here depends on what the tools currently do and what
binaries are out there.
Cheers
---Dave
^ permalink raw reply
* RE: [PATCH V14 1/5] dt-bindings: fsl: scu: add thermal binding
From: Aisheng Dong @ 2019-06-11 11:00 UTC (permalink / raw)
To: Anson Huang, robh+dt@kernel.org, mark.rutland@arm.com,
corbet@lwn.net, shawnguo@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com,
catalin.marinas@arm.com, will.deacon@arm.com, rui.zhang@intel.com,
edubezval@gmail.com, daniel.lezcano@linaro.org,
ulf.hansson@linaro.org, Peng Fan, mchehab+samsung@kernel.org,
linux@roeck-us.net, Daniel Baluta, maxime.ripard@bootlin.com,
olof@lixom.net, jagan@amarulasolutions.com,
horms+renesas@verge.net.au, Leonard Crestez,
bjorn.andersson@linaro.org, dinguyen@kernel.org,
enric.balletbo@collabora.com, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org
Cc: dl-linux-imx
In-Reply-To: <20190610025254.23940-1-Anson.Huang@nxp.com>
> From: Anson.Huang@nxp.com [mailto:Anson.Huang@nxp.com]
> Sent: Monday, June 10, 2019 10:53 AM
>
> NXP i.MX8QXP is an ARMv8 SoC with a Cortex-M4 core inside as system
> controller, the system controller is in charge of system power, clock and
> thermal sensors etc. management, Linux kernel has to communicate with
> system controller via MU (message unit) IPC to get temperature from thermal
> sensors, this patch adds binding doc for i.MX system controller thermal driver.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
Regards
Dong Aisheng
^ permalink raw reply
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Pavel Machek @ 2019-06-11 10:33 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: Dave Hansen, Peter Zijlstra, x86, H. Peter Anvin, Thomas Gleixner,
Ingo Molnar, linux-kernel, linux-doc, linux-mm, linux-arch,
linux-api, Arnd Bergmann, Andy Lutomirski, Balbir Singh,
Borislav Petkov, Cyrill Gorcunov, Dave Hansen,
Eugene Syromiatnikov, Florian Weimer, H.J. Lu, Jann Horn,
Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue,
Dave Martin
In-Reply-To: <e1543e7beb0eb55d6febcd847ccab9b219e60338.camel@intel.com>
[-- Attachment #1: Type: text/plain, Size: 1873 bytes --]
On Mon 2019-06-10 08:47:45, Yu-cheng Yu wrote:
> On Sat, 2019-06-08 at 22:52 +0200, Pavel Machek wrote:
> > Hi!
> >
> > > > I've no idea what the kernel should do; since you failed to answer the
> > > > question what happens when you point this to garbage.
> > > >
> > > > Does it then fault or what?
> > >
> > > Yeah, I think you'll fault with a rather mysterious CR2 value since
> > > you'll go look at the instruction that faulted and not see any
> > > references to the CR2 value.
> > >
> > > I think this new MSR probably needs to get included in oops output when
> > > CET is enabled.
> > >
> > > Why don't we require that a VMA be in place for the entire bitmap?
> > > Don't we need a "get" prctl function too in case something like a JIT is
> > > running and needs to find the location of this bitmap to set bits itself?
> > >
> > > Or, do we just go whole-hog and have the kernel manage the bitmap
> > > itself. Our interface here could be:
> > >
> > > prctl(PR_MARK_CODE_AS_LEGACY, start, size);
> > >
> > > and then have the kernel allocate and set the bitmap for those code
> > > locations.
> >
> > For the record, that sounds like a better interface than userspace knowing
> > about the bitmap formats...
> > Pavel
>
> Initially we implemented the bitmap that way. To manage the bitmap, every time
> the application issues a syscall for a .so it loads, and the kernel does
> copy_from_user() & copy_to_user() (or similar things). If a system has a few
> legacy .so files and every application does the same, it can take a long time to
> boot up.
Loading .so is already many syscalls, I'd not expect measurable
performance there. Are you sure?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH v3 33/33] docs: EDID/HOWTO.txt: convert it and rename to howto.rst
From: Daniel Vetter @ 2019-06-11 9:40 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Daniel Vetter, Linux Doc Mailing List, Mauro Carvalho Chehab,
linux-kernel, Jonathan Corbet, Maarten Lankhorst, Maxime Ripard,
Sean Paul, David Airlie, dri-devel
In-Reply-To: <20190611060215.232af2bb@coco.lan>
On Tue, Jun 11, 2019 at 06:02:15AM -0300, Mauro Carvalho Chehab wrote:
> Em Tue, 11 Jun 2019 10:37:31 +0200
> Daniel Vetter <daniel@ffwll.ch> escreveu:
>
> > On Sat, Jun 08, 2019 at 11:27:23PM -0300, Mauro Carvalho Chehab wrote:
> > > Sphinx need to know when a paragraph ends. So, do some adjustments
> > > at the file for it to be properly parsed.
> > >
> > > At its new index.rst, let's add a :orphan: while this is not linked to
> > > the main index.rst file, in order to avoid build warnings.
> > >
> > > that's said, I believe that this file should be moved to the
> > > GPU/DRM documentation.
> >
> > Yes, but there's a bit a twist: This is definitely end-user documentation,
> > so maybe should be in admin-guide?
> >
> > Atm all we have in Documentation/gpu/ is internals for drivers + some
> > beginnings of uapi documentation for userspace developers.
>
> On media, we have several different types of documents:
>
> - uAPI - consumed by both userspace and kernelspace developers;
> - kAPI - consumed by Kernel hackers;
> - driver-specific information. Those are usually messy, as some contain
> specific internal details, while others are pure end-user documentation.
>
> there are several cross-references between uAPI and kAPI parts.
>
> I've seem similar patterns on several other driver subsystems.
>
> I agree with Jon's principle that the best is to focus the book per
> audience. Yet, trying to rearrange the documentation means a lot of work,
> specially on those cases where a single file contain different types of
> documentation, like on media driver docs.
Yeah atm we're doing a bad job of keeping the kapi and uapi parts
separate. But the plan at least is to move all the gpu related uapi stuff
into Documentation/gpu/drm-uapi.rst. Not sure there's value in moving that
out of the gpu folder ...
> > Jon, what's your recommendation here for subsystem specific
> > end-user/adming docs?
>
> Jon, please correct me if I' wrong, bu I guess the plan is to place them
> somewhere under Documentation/admin-guide/.
>
> If so, perhaps creating a Documentation/admin-guide/drm dir there and
> place docs like EDID/HOWTO.txt, svga.txt, etc would work.
>
> Btw, that's one of the reasons[1] why I opted to keep the files where they
> are: properly organizing the converted documents call for such kind
> of discussions. On my experience, discussing names and directory locations
> can generate warm discussions and take a lot of time to reach consensus.
>
> [1] The other one is to avoid/simplify merge conflicts.
Oh definitely not asking for moving them at the same time, just wondering
how this should be solved properly.
-Daniel
>
> >
> > Thanks, Daniel
> >
> > >
> > > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> > > ---
> > > Documentation/EDID/{HOWTO.txt => howto.rst} | 31 ++++++++++++-------
> > > .../admin-guide/kernel-parameters.txt | 2 +-
> > > drivers/gpu/drm/Kconfig | 2 +-
> > > 3 files changed, 22 insertions(+), 13 deletions(-)
> > > rename Documentation/EDID/{HOWTO.txt => howto.rst} (83%)
> > >
> > > diff --git a/Documentation/EDID/HOWTO.txt b/Documentation/EDID/howto.rst
> > > similarity index 83%
> > > rename from Documentation/EDID/HOWTO.txt
> > > rename to Documentation/EDID/howto.rst
> > > index 539871c3b785..725fd49a88ca 100644
> > > --- a/Documentation/EDID/HOWTO.txt
> > > +++ b/Documentation/EDID/howto.rst
> > > @@ -1,3 +1,9 @@
> > > +:orphan:
> > > +
> > > +====
> > > +EDID
> > > +====
> > > +
> > > In the good old days when graphics parameters were configured explicitly
> > > in a file called xorg.conf, even broken hardware could be managed.
> > >
> > > @@ -34,16 +40,19 @@ Makefile. Please note that the EDID data structure expects the timing
> > > values in a different way as compared to the standard X11 format.
> > >
> > > X11:
> > > -HTimings: hdisp hsyncstart hsyncend htotal
> > > -VTimings: vdisp vsyncstart vsyncend vtotal
> > > + HTimings:
> > > + hdisp hsyncstart hsyncend htotal
> > > + VTimings:
> > > + vdisp vsyncstart vsyncend vtotal
> > >
> > > -EDID:
> > > -#define XPIX hdisp
> > > -#define XBLANK htotal-hdisp
> > > -#define XOFFSET hsyncstart-hdisp
> > > -#define XPULSE hsyncend-hsyncstart
> > > +EDID::
> > >
> > > -#define YPIX vdisp
> > > -#define YBLANK vtotal-vdisp
> > > -#define YOFFSET vsyncstart-vdisp
> > > -#define YPULSE vsyncend-vsyncstart
> > > + #define XPIX hdisp
> > > + #define XBLANK htotal-hdisp
> > > + #define XOFFSET hsyncstart-hdisp
> > > + #define XPULSE hsyncend-hsyncstart
> > > +
> > > + #define YPIX vdisp
> > > + #define YBLANK vtotal-vdisp
> > > + #define YOFFSET vsyncstart-vdisp
> > > + #define YPULSE vsyncend-vsyncstart
> > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > > index 3d072ca532bb..3faf37b8b001 100644
> > > --- a/Documentation/admin-guide/kernel-parameters.txt
> > > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > > @@ -930,7 +930,7 @@
> > > edid/1680x1050.bin, or edid/1920x1080.bin is given
> > > and no file with the same name exists. Details and
> > > instructions how to build your own EDID data are
> > > - available in Documentation/EDID/HOWTO.txt. An EDID
> > > + available in Documentation/EDID/howto.rst. An EDID
> > > data set will only be used for a particular connector,
> > > if its name and a colon are prepended to the EDID
> > > name. Each connector may use a unique EDID data
> > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > > index 6b34949416b1..c3a6dd284c91 100644
> > > --- a/drivers/gpu/drm/Kconfig
> > > +++ b/drivers/gpu/drm/Kconfig
> > > @@ -141,7 +141,7 @@ config DRM_LOAD_EDID_FIRMWARE
> > > monitor are unable to provide appropriate EDID data. Since this
> > > feature is provided as a workaround for broken hardware, the
> > > default case is N. Details and instructions how to build your own
> > > - EDID data are given in Documentation/EDID/HOWTO.txt.
> > > + EDID data are given in Documentation/EDID/howto.rst.
> > >
> > > config DRM_DP_CEC
> > > bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
> > > --
> > > 2.21.0
> > >
> >
>
>
>
> Thanks,
> Mauro
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH v3 33/33] docs: EDID/HOWTO.txt: convert it and rename to howto.rst
From: Mauro Carvalho Chehab @ 2019-06-11 9:02 UTC (permalink / raw)
To: Daniel Vetter
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet, Maarten Lankhorst, Maxime Ripard, Sean Paul,
David Airlie, dri-devel
In-Reply-To: <20190611083731.GS21222@phenom.ffwll.local>
Em Tue, 11 Jun 2019 10:37:31 +0200
Daniel Vetter <daniel@ffwll.ch> escreveu:
> On Sat, Jun 08, 2019 at 11:27:23PM -0300, Mauro Carvalho Chehab wrote:
> > Sphinx need to know when a paragraph ends. So, do some adjustments
> > at the file for it to be properly parsed.
> >
> > At its new index.rst, let's add a :orphan: while this is not linked to
> > the main index.rst file, in order to avoid build warnings.
> >
> > that's said, I believe that this file should be moved to the
> > GPU/DRM documentation.
>
> Yes, but there's a bit a twist: This is definitely end-user documentation,
> so maybe should be in admin-guide?
>
> Atm all we have in Documentation/gpu/ is internals for drivers + some
> beginnings of uapi documentation for userspace developers.
On media, we have several different types of documents:
- uAPI - consumed by both userspace and kernelspace developers;
- kAPI - consumed by Kernel hackers;
- driver-specific information. Those are usually messy, as some contain
specific internal details, while others are pure end-user documentation.
there are several cross-references between uAPI and kAPI parts.
I've seem similar patterns on several other driver subsystems.
I agree with Jon's principle that the best is to focus the book per
audience. Yet, trying to rearrange the documentation means a lot of work,
specially on those cases where a single file contain different types of
documentation, like on media driver docs.
> Jon, what's your recommendation here for subsystem specific
> end-user/adming docs?
Jon, please correct me if I' wrong, bu I guess the plan is to place them
somewhere under Documentation/admin-guide/.
If so, perhaps creating a Documentation/admin-guide/drm dir there and
place docs like EDID/HOWTO.txt, svga.txt, etc would work.
Btw, that's one of the reasons[1] why I opted to keep the files where they
are: properly organizing the converted documents call for such kind
of discussions. On my experience, discussing names and directory locations
can generate warm discussions and take a lot of time to reach consensus.
[1] The other one is to avoid/simplify merge conflicts.
>
> Thanks, Daniel
>
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> > ---
> > Documentation/EDID/{HOWTO.txt => howto.rst} | 31 ++++++++++++-------
> > .../admin-guide/kernel-parameters.txt | 2 +-
> > drivers/gpu/drm/Kconfig | 2 +-
> > 3 files changed, 22 insertions(+), 13 deletions(-)
> > rename Documentation/EDID/{HOWTO.txt => howto.rst} (83%)
> >
> > diff --git a/Documentation/EDID/HOWTO.txt b/Documentation/EDID/howto.rst
> > similarity index 83%
> > rename from Documentation/EDID/HOWTO.txt
> > rename to Documentation/EDID/howto.rst
> > index 539871c3b785..725fd49a88ca 100644
> > --- a/Documentation/EDID/HOWTO.txt
> > +++ b/Documentation/EDID/howto.rst
> > @@ -1,3 +1,9 @@
> > +:orphan:
> > +
> > +====
> > +EDID
> > +====
> > +
> > In the good old days when graphics parameters were configured explicitly
> > in a file called xorg.conf, even broken hardware could be managed.
> >
> > @@ -34,16 +40,19 @@ Makefile. Please note that the EDID data structure expects the timing
> > values in a different way as compared to the standard X11 format.
> >
> > X11:
> > -HTimings: hdisp hsyncstart hsyncend htotal
> > -VTimings: vdisp vsyncstart vsyncend vtotal
> > + HTimings:
> > + hdisp hsyncstart hsyncend htotal
> > + VTimings:
> > + vdisp vsyncstart vsyncend vtotal
> >
> > -EDID:
> > -#define XPIX hdisp
> > -#define XBLANK htotal-hdisp
> > -#define XOFFSET hsyncstart-hdisp
> > -#define XPULSE hsyncend-hsyncstart
> > +EDID::
> >
> > -#define YPIX vdisp
> > -#define YBLANK vtotal-vdisp
> > -#define YOFFSET vsyncstart-vdisp
> > -#define YPULSE vsyncend-vsyncstart
> > + #define XPIX hdisp
> > + #define XBLANK htotal-hdisp
> > + #define XOFFSET hsyncstart-hdisp
> > + #define XPULSE hsyncend-hsyncstart
> > +
> > + #define YPIX vdisp
> > + #define YBLANK vtotal-vdisp
> > + #define YOFFSET vsyncstart-vdisp
> > + #define YPULSE vsyncend-vsyncstart
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index 3d072ca532bb..3faf37b8b001 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -930,7 +930,7 @@
> > edid/1680x1050.bin, or edid/1920x1080.bin is given
> > and no file with the same name exists. Details and
> > instructions how to build your own EDID data are
> > - available in Documentation/EDID/HOWTO.txt. An EDID
> > + available in Documentation/EDID/howto.rst. An EDID
> > data set will only be used for a particular connector,
> > if its name and a colon are prepended to the EDID
> > name. Each connector may use a unique EDID data
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index 6b34949416b1..c3a6dd284c91 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -141,7 +141,7 @@ config DRM_LOAD_EDID_FIRMWARE
> > monitor are unable to provide appropriate EDID data. Since this
> > feature is provided as a workaround for broken hardware, the
> > default case is N. Details and instructions how to build your own
> > - EDID data are given in Documentation/EDID/HOWTO.txt.
> > + EDID data are given in Documentation/EDID/howto.rst.
> >
> > config DRM_DP_CEC
> > bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
> > --
> > 2.21.0
> >
>
Thanks,
Mauro
^ permalink raw reply
* Re: [PATCH v3 33/33] docs: EDID/HOWTO.txt: convert it and rename to howto.rst
From: Daniel Vetter @ 2019-06-11 8:37 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet, Maarten Lankhorst, Maxime Ripard, Sean Paul,
David Airlie, Daniel Vetter, dri-devel
In-Reply-To: <74bec0b5b7c32c8d84adbaf9ff208803475198e5.1560045490.git.mchehab+samsung@kernel.org>
On Sat, Jun 08, 2019 at 11:27:23PM -0300, Mauro Carvalho Chehab wrote:
> Sphinx need to know when a paragraph ends. So, do some adjustments
> at the file for it to be properly parsed.
>
> At its new index.rst, let's add a :orphan: while this is not linked to
> the main index.rst file, in order to avoid build warnings.
>
> that's said, I believe that this file should be moved to the
> GPU/DRM documentation.
Yes, but there's a bit a twist: This is definitely end-user documentation,
so maybe should be in admin-guide?
Atm all we have in Documentation/gpu/ is internals for drivers + some
beginnings of uapi documentation for userspace developers.
Jon, what's your recommendation here for subsystem specific
end-user/adming docs?
Thanks, Daniel
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> ---
> Documentation/EDID/{HOWTO.txt => howto.rst} | 31 ++++++++++++-------
> .../admin-guide/kernel-parameters.txt | 2 +-
> drivers/gpu/drm/Kconfig | 2 +-
> 3 files changed, 22 insertions(+), 13 deletions(-)
> rename Documentation/EDID/{HOWTO.txt => howto.rst} (83%)
>
> diff --git a/Documentation/EDID/HOWTO.txt b/Documentation/EDID/howto.rst
> similarity index 83%
> rename from Documentation/EDID/HOWTO.txt
> rename to Documentation/EDID/howto.rst
> index 539871c3b785..725fd49a88ca 100644
> --- a/Documentation/EDID/HOWTO.txt
> +++ b/Documentation/EDID/howto.rst
> @@ -1,3 +1,9 @@
> +:orphan:
> +
> +====
> +EDID
> +====
> +
> In the good old days when graphics parameters were configured explicitly
> in a file called xorg.conf, even broken hardware could be managed.
>
> @@ -34,16 +40,19 @@ Makefile. Please note that the EDID data structure expects the timing
> values in a different way as compared to the standard X11 format.
>
> X11:
> -HTimings: hdisp hsyncstart hsyncend htotal
> -VTimings: vdisp vsyncstart vsyncend vtotal
> + HTimings:
> + hdisp hsyncstart hsyncend htotal
> + VTimings:
> + vdisp vsyncstart vsyncend vtotal
>
> -EDID:
> -#define XPIX hdisp
> -#define XBLANK htotal-hdisp
> -#define XOFFSET hsyncstart-hdisp
> -#define XPULSE hsyncend-hsyncstart
> +EDID::
>
> -#define YPIX vdisp
> -#define YBLANK vtotal-vdisp
> -#define YOFFSET vsyncstart-vdisp
> -#define YPULSE vsyncend-vsyncstart
> + #define XPIX hdisp
> + #define XBLANK htotal-hdisp
> + #define XOFFSET hsyncstart-hdisp
> + #define XPULSE hsyncend-hsyncstart
> +
> + #define YPIX vdisp
> + #define YBLANK vtotal-vdisp
> + #define YOFFSET vsyncstart-vdisp
> + #define YPULSE vsyncend-vsyncstart
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 3d072ca532bb..3faf37b8b001 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -930,7 +930,7 @@
> edid/1680x1050.bin, or edid/1920x1080.bin is given
> and no file with the same name exists. Details and
> instructions how to build your own EDID data are
> - available in Documentation/EDID/HOWTO.txt. An EDID
> + available in Documentation/EDID/howto.rst. An EDID
> data set will only be used for a particular connector,
> if its name and a colon are prepended to the EDID
> name. Each connector may use a unique EDID data
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 6b34949416b1..c3a6dd284c91 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -141,7 +141,7 @@ config DRM_LOAD_EDID_FIRMWARE
> monitor are unable to provide appropriate EDID data. Since this
> feature is provided as a workaround for broken hardware, the
> default case is N. Details and instructions how to build your own
> - EDID data are given in Documentation/EDID/HOWTO.txt.
> + EDID data are given in Documentation/EDID/howto.rst.
>
> config DRM_DP_CEC
> bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
> --
> 2.21.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Florian Weimer @ 2019-06-11 7:24 UTC (permalink / raw)
To: Dave Hansen
Cc: Yu-cheng Yu, Andy Lutomirski, Peter Zijlstra, x86, H. Peter Anvin,
Thomas Gleixner, Ingo Molnar, linux-kernel, linux-doc, linux-mm,
linux-arch, linux-api, Arnd Bergmann, Balbir Singh,
Borislav Petkov, Cyrill Gorcunov, Dave Hansen,
Eugene Syromiatnikov, H.J. Lu, Jann Horn, Jonathan Corbet,
Kees Cook, Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <92e56b28-0cd4-e3f4-867b-639d9b98b86c@intel.com>
* Dave Hansen:
> My assumption has always been that these large, potentially sparse
> hardware tables *must* be mmap()'d with MAP_NORESERVE specified. That
> should keep them from being problematic with respect to overcommit.
MAP_NORESERVE pages still count towards the commit limit. The flag only
disables checks at allocation time, for this particular allocation. (At
least this was the behavior the last time I looked into this, I
believe.)
Not sure if this makes a difference here.
Thanks,
Florian
^ permalink raw reply
* [PATCH v11 11/13] ima: Define ima-modsig template
From: Thiago Jung Bauermann @ 2019-06-11 6:28 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-doc, linux-kernel, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro, Thiago Jung Bauermann
In-Reply-To: <20190611062817.18412-1-bauerman@linux.ibm.com>
Define new "d-modsig" template field which holds the digest that is
expected to match the one contained in the modsig, and also new "modsig"
template field which holds the appended file signature.
Add a new "ima-modsig" defined template descriptor with the new fields as
well as the ones from the "ima-sig" descriptor.
Change ima_store_measurement() to accept a struct modsig * argument so that
it can be passed along to the templates via struct ima_event_data.
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
Documentation/security/IMA-templates.rst | 7 ++-
security/integrity/ima/ima.h | 20 +++++++-
security/integrity/ima/ima_api.c | 5 +-
security/integrity/ima/ima_main.c | 2 +-
security/integrity/ima/ima_modsig.c | 19 +++++++
security/integrity/ima/ima_policy.c | 41 ++++++++++++++++
security/integrity/ima/ima_template.c | 7 ++-
security/integrity/ima/ima_template_lib.c | 60 ++++++++++++++++++++++-
security/integrity/ima/ima_template_lib.h | 4 ++
9 files changed, 157 insertions(+), 8 deletions(-)
diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index 2cd0e273cc9a..8da20b444be0 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -68,15 +68,18 @@ descriptors by adding their identifier to the format string
- 'd-ng': the digest of the event, calculated with an arbitrary hash
algorithm (field format: [<hash algo>:]digest, where the digest
prefix is shown only if the hash algorithm is not SHA1 or MD5);
+ - 'd-modsig': the digest of the event without the appended modsig;
- 'n-ng': the name of the event, without size limitations;
- - 'sig': the file signature.
+ - 'sig': the file signature;
+ - 'modsig' the appended file signature.
Below, there is the list of defined template descriptors:
- "ima": its format is ``d|n``;
- "ima-ng" (default): its format is ``d-ng|n-ng``;
- - "ima-sig": its format is ``d-ng|n-ng|sig``.
+ - "ima-sig": its format is ``d-ng|n-ng|sig``;
+ - "ima-modsig": its format is ``d-ng|n-ng|sig|d-modsig|modsig``.
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 0acc8e56ec73..a2b2c13ceda8 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -64,6 +64,7 @@ struct ima_event_data {
const unsigned char *filename;
struct evm_ima_xattr_data *xattr_value;
int xattr_len;
+ const struct modsig *modsig;
const char *violation;
};
@@ -207,7 +208,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
const unsigned char *filename,
struct evm_ima_xattr_data *xattr_value,
- int xattr_len, int pcr,
+ int xattr_len, const struct modsig *modsig, int pcr,
struct ima_template_desc *template_desc);
void ima_audit_measurement(struct integrity_iint_cache *iint,
const unsigned char *filename);
@@ -308,6 +309,10 @@ bool ima_hook_supports_modsig(enum ima_hooks func);
int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
struct modsig **modsig);
void ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size);
+int ima_get_modsig_digest(const struct modsig *modsig, enum hash_algo *algo,
+ const u8 **digest, u32 *digest_size);
+int ima_get_raw_modsig(const struct modsig *modsig, const void **data,
+ u32 *data_len);
void ima_free_modsig(struct modsig *modsig);
#else
static inline bool ima_hook_supports_modsig(enum ima_hooks func)
@@ -326,6 +331,19 @@ static inline void ima_collect_modsig(struct modsig *modsig, const void *buf,
{
}
+static inline int ima_get_modsig_digest(const struct modsig *modsig,
+ enum hash_algo *algo, const u8 **digest,
+ u32 *digest_size)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int ima_get_raw_modsig(const struct modsig *modsig,
+ const void **data, u32 *data_len)
+{
+ return -EOPNOTSUPP;
+}
+
static inline void ima_free_modsig(struct modsig *modsig)
{
}
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c351b8c37278..32297d1e6164 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -291,7 +291,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
void ima_store_measurement(struct integrity_iint_cache *iint,
struct file *file, const unsigned char *filename,
struct evm_ima_xattr_data *xattr_value,
- int xattr_len, int pcr,
+ int xattr_len, const struct modsig *modsig, int pcr,
struct ima_template_desc *template_desc)
{
static const char op[] = "add_template_measure";
@@ -303,7 +303,8 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
.file = file,
.filename = filename,
.xattr_value = xattr_value,
- .xattr_len = xattr_len };
+ .xattr_len = xattr_len,
+ .modsig = modsig };
int violation = 0;
if (iint->measured_pcrs & (0x1 << pcr))
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 2c9d3cf85726..85afb31fafe0 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -323,7 +323,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
if (action & IMA_MEASURE)
ima_store_measurement(iint, file, pathname,
- xattr_value, xattr_len, pcr,
+ xattr_value, xattr_len, modsig, pcr,
template_desc);
if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) {
inode_lock(inode);
diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c
index d438b87dba89..b01bbfeb1d98 100644
--- a/security/integrity/ima/ima_modsig.c
+++ b/security/integrity/ima/ima_modsig.c
@@ -140,6 +140,25 @@ int ima_modsig_verify(struct key *keyring, const struct modsig *modsig)
VERIFYING_MODULE_SIGNATURE, NULL, NULL);
}
+int ima_get_modsig_digest(const struct modsig *modsig, enum hash_algo *algo,
+ const u8 **digest, u32 *digest_size)
+{
+ *algo = modsig->hash_algo;
+ *digest = modsig->digest;
+ *digest_size = modsig->digest_size;
+
+ return 0;
+}
+
+int ima_get_raw_modsig(const struct modsig *modsig, const void **data,
+ u32 *data_len)
+{
+ *data = &modsig->raw_pkcs7;
+ *data_len = modsig->raw_pkcs7_len;
+
+ return 0;
+}
+
void ima_free_modsig(struct modsig *modsig)
{
if (!modsig)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index f64ef84516db..6463ab8921ea 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -10,6 +10,9 @@
* - initialize default measure policy rules
*
*/
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/init.h>
#include <linux/list.h>
#include <linux/fs.h>
@@ -766,6 +769,38 @@ static void ima_log_string(struct audit_buffer *ab, char *key, char *value)
ima_log_string_op(ab, key, value, NULL);
}
+/*
+ * Validating the appended signature included in the measurement list requires
+ * the file hash calculated without the appended signature (i.e., the 'd-modsig'
+ * field). Therefore, notify the user if they have the 'modsig' field but not
+ * the 'd-modsig' field in the template.
+ */
+static void check_template_modsig(const struct ima_template_desc *template)
+{
+#define MSG "template with 'modsig' field also needs 'd-modsig' field\n"
+ bool has_modsig, has_dmodsig;
+ static bool checked;
+ int i;
+
+ /* We only need to notify the user once. */
+ if (checked)
+ return;
+
+ has_modsig = has_dmodsig = false;
+ for (i = 0; i < template->num_fields; i++) {
+ if (!strcmp(template->fields[i]->field_id, "modsig"))
+ has_modsig = true;
+ else if (!strcmp(template->fields[i]->field_id, "d-modsig"))
+ has_dmodsig = true;
+ }
+
+ if (has_modsig && !has_dmodsig)
+ pr_notice(MSG);
+
+ checked = true;
+#undef MSG
+}
+
static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
{
struct audit_buffer *ab;
@@ -1096,6 +1131,12 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
else if (entry->action == APPRAISE)
temp_ima_appraise |= ima_appraise_flag(entry->func);
+ if (!result && entry->flags & IMA_MODSIG_ALLOWED) {
+ template_desc = entry->template ? entry->template :
+ ima_template_desc_current();
+ check_template_modsig(template_desc);
+ }
+
audit_log_format(ab, "res=%d", !result);
audit_log_end(ab);
return result;
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index e6e892f31cbd..e25bef419c87 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -26,6 +26,7 @@ static struct ima_template_desc builtin_templates[] = {
{.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT},
{.name = "ima-ng", .fmt = "d-ng|n-ng"},
{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
+ {.name = "ima-modsig", .fmt = "d-ng|n-ng|sig|d-modsig|modsig"},
{.name = "", .fmt = ""}, /* placeholder for a custom format */
};
@@ -43,8 +44,12 @@ static const struct ima_template_field supported_fields[] = {
.field_show = ima_show_template_string},
{.field_id = "sig", .field_init = ima_eventsig_init,
.field_show = ima_show_template_sig},
+ {.field_id = "d-modsig", .field_init = ima_eventdigest_modsig_init,
+ .field_show = ima_show_template_digest_ng},
+ {.field_id = "modsig", .field_init = ima_eventmodsig_init,
+ .field_show = ima_show_template_sig},
};
-#define MAX_TEMPLATE_NAME_LEN 15
+#define MAX_TEMPLATE_NAME_LEN sizeof("d|n|d-ng|n-ng|sig|d-modisg|modsig")
static struct ima_template_desc *ima_template;
static int template_desc_init_fields(const char *template_fmt,
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 513b457ae900..dacb01fb105f 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -223,7 +223,8 @@ int ima_parse_buf(void *bufstartp, void *bufendp, void **bufcurp,
return 0;
}
-static int ima_eventdigest_init_common(u8 *digest, u32 digestsize, u8 hash_algo,
+static int ima_eventdigest_init_common(const u8 *digest, u32 digestsize,
+ u8 hash_algo,
struct ima_field_data *field_data)
{
/*
@@ -326,6 +327,41 @@ int ima_eventdigest_ng_init(struct ima_event_data *event_data,
hash_algo, field_data);
}
+/*
+ * This function writes the digest of the file which is expected to match the
+ * digest contained in the file's embedded signature.
+ */
+int ima_eventdigest_modsig_init(struct ima_event_data *event_data,
+ struct ima_field_data *field_data)
+{
+ enum hash_algo hash_algo;
+ const u8 *cur_digest;
+ u32 cur_digestsize;
+
+ if (!event_data->modsig)
+ return 0;
+
+ if (event_data->violation) {
+ /* Recording a violation. */
+ hash_algo = HASH_ALGO_SHA1;
+ cur_digest = NULL;
+ cur_digestsize = 0;
+ } else {
+ int rc;
+
+ rc = ima_get_modsig_digest(event_data->modsig, &hash_algo,
+ &cur_digest, &cur_digestsize);
+ if (rc)
+ return rc;
+ else if (hash_algo == HASH_ALGO__LAST || cur_digestsize == 0)
+ /* There was some error collecting the digest. */
+ return -EINVAL;
+ }
+
+ return ima_eventdigest_init_common(cur_digest, cur_digestsize,
+ hash_algo, field_data);
+}
+
static int ima_eventname_init_common(struct ima_event_data *event_data,
struct ima_field_data *field_data,
bool size_limit)
@@ -389,3 +425,25 @@ int ima_eventsig_init(struct ima_event_data *event_data,
return ima_write_template_field_data(xattr_value, event_data->xattr_len,
DATA_FMT_HEX, field_data);
}
+
+int ima_eventmodsig_init(struct ima_event_data *event_data,
+ struct ima_field_data *field_data)
+{
+ const void *data;
+ u32 data_len;
+ int rc;
+
+ if (!event_data->modsig)
+ return 0;
+
+ /*
+ * modsig is a runtime structure containing pointers. Get its raw data
+ * instead.
+ */
+ rc = ima_get_raw_modsig(event_data->modsig, &data, &data_len);
+ if (rc)
+ return rc;
+
+ return ima_write_template_field_data(data, data_len, DATA_FMT_HEX,
+ field_data);
+}
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index 6a3d8b831deb..1d7c690ebae5 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -38,8 +38,12 @@ int ima_eventname_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
int ima_eventdigest_ng_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
+int ima_eventdigest_modsig_init(struct ima_event_data *event_data,
+ struct ima_field_data *field_data);
int ima_eventname_ng_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
int ima_eventsig_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
+int ima_eventmodsig_init(struct ima_event_data *event_data,
+ struct ima_field_data *field_data);
#endif /* __LINUX_IMA_TEMPLATE_LIB_H */
^ permalink raw reply related
* [PATCH v11 13/13] ima: Allow template= option for appraise rules as well
From: Thiago Jung Bauermann @ 2019-06-11 6:28 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-doc, linux-kernel, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro, Thiago Jung Bauermann
In-Reply-To: <20190611062817.18412-1-bauerman@linux.ibm.com>
It's useful being able to specify a different IMA template on appraise
policy rules, so allow it.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
---
security/integrity/ima/ima_policy.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 6463ab8921ea..1ac1ef458f2e 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -1110,7 +1110,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
break;
case Opt_template:
ima_log_string(ab, "template", args[0].from);
- if (entry->action != MEASURE) {
+ if (entry->action != MEASURE &&
+ entry->action != APPRAISE) {
result = -EINVAL;
break;
}
^ permalink raw reply related
* [PATCH v11 12/13] ima: Store the measurement again when appraising a modsig
From: Thiago Jung Bauermann @ 2019-06-11 6:28 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-doc, linux-kernel, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro, Thiago Jung Bauermann
In-Reply-To: <20190611062817.18412-1-bauerman@linux.ibm.com>
If the IMA template contains the "modsig" or "d-modsig" field, then the
modsig should be added to the measurement list when the file is appraised.
And that is what normally happens, but if a measurement rule caused a file
containing a modsig to be measured before a different rule causes it to be
appraised, the resulting measurement entry will not contain the modsig
because it is only fetched during appraisal. When the appraisal rule
triggers, it won't store a new measurement containing the modsig because
the file was already measured.
We need to detect that situation and store an additional measurement with
the modsig. This is done by adding an IMA_MEASURE action flag if we read a
modsig and the IMA template contains a modsig field.
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
security/integrity/ima/ima.h | 1 +
security/integrity/ima/ima_api.c | 19 +++++++++++++++----
security/integrity/ima/ima_main.c | 15 ++++++++++++---
security/integrity/ima/ima_template.c | 19 +++++++++++++++++++
4 files changed, 47 insertions(+), 7 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index a2b2c13ceda8..44f5f60424c2 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -149,6 +149,7 @@ void ima_putc(struct seq_file *m, void *data, int datalen);
void ima_print_digest(struct seq_file *m, u8 *digest, u32 size);
struct ima_template_desc *ima_template_desc_current(void);
struct ima_template_desc *lookup_template_desc(const char *name);
+bool ima_template_has_modsig(const struct ima_template_desc *ima_template);
int ima_restore_measurement_entry(struct ima_template_entry *entry);
int ima_restore_measurement_list(loff_t bufsize, void *buf);
int ima_measurements_show(struct seq_file *m, void *v);
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 32297d1e6164..bb887ed3d8a7 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -222,6 +222,14 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
char digest[IMA_MAX_DIGEST_SIZE];
} hash;
+ /*
+ * Always collect the modsig, because IMA might have already collected
+ * the file digest without collecting the modsig in a previous
+ * measurement rule.
+ */
+ if (modsig)
+ ima_collect_modsig(modsig, buf, size);
+
if (iint->flags & IMA_COLLECTED)
goto out;
@@ -255,9 +263,6 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
memcpy(iint->ima_hash, &hash, length);
iint->version = i_version;
- if (modsig)
- ima_collect_modsig(modsig, buf, size);
-
/* Possibly temporary failure due to type of read (eg. O_DIRECT) */
if (!result)
iint->flags |= IMA_COLLECTED;
@@ -307,7 +312,13 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
.modsig = modsig };
int violation = 0;
- if (iint->measured_pcrs & (0x1 << pcr))
+ /*
+ * We still need to store the measurement in the case of MODSIG because
+ * we only have its contents to put in the list at the time of
+ * appraisal, but a file measurement from earlier might already exist in
+ * the measurement list.
+ */
+ if (iint->measured_pcrs & (0x1 << pcr) && !modsig)
return;
result = ima_alloc_init_template(&event_data, &entry, template_desc);
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 85afb31fafe0..e0ca39f81a59 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -307,9 +307,18 @@ static int process_measurement(struct file *file, const struct cred *cred,
/* read 'security.ima' */
xattr_len = ima_read_xattr(file_dentry(file), &xattr_value);
- /* Read the appended modsig if allowed by the policy. */
- if (iint->flags & IMA_MODSIG_ALLOWED)
- ima_read_modsig(func, buf, size, &modsig);
+ /*
+ * Read the appended modsig if allowed by the policy, and allow
+ * an additional measurement list entry, if needed, based on the
+ * template format and whether the file was already measured.
+ */
+ if (iint->flags & IMA_MODSIG_ALLOWED) {
+ rc = ima_read_modsig(func, buf, size, &modsig);
+
+ if (!rc && ima_template_has_modsig(template_desc) &&
+ iint->flags & IMA_MEASURED)
+ action |= IMA_MEASURE;
+ }
}
hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index e25bef419c87..00d9a6cc8a60 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -56,6 +56,25 @@ static int template_desc_init_fields(const char *template_fmt,
const struct ima_template_field ***fields,
int *num_fields);
+/**
+ * ima_template_has_modsig - Check whether template has modsig-related fields.
+ * @ima_template: IMA template to check.
+ *
+ * Tells whether the given template has fields referencing a file's appended
+ * signature.
+ */
+bool ima_template_has_modsig(const struct ima_template_desc *ima_template)
+{
+ int i;
+
+ for (i = 0; i < ima_template->num_fields; i++)
+ if (!strcmp(ima_template->fields[i]->field_id, "modsig") ||
+ !strcmp(ima_template->fields[i]->field_id, "d-modsig"))
+ return true;
+
+ return false;
+}
+
static int __init ima_template_setup(char *str)
{
struct ima_template_desc *template_desc;
^ permalink raw reply related
* [PATCH v11 08/13] ima: Factor xattr_verify() out of ima_appraise_measurement()
From: Thiago Jung Bauermann @ 2019-06-11 6:28 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-doc, linux-kernel, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro, Thiago Jung Bauermann
In-Reply-To: <20190611062817.18412-1-bauerman@linux.ibm.com>
Verify xattr signature in a separate function so that the logic in
ima_appraise_measurement() remains clear when it gains the ability to also
verify an appended module signature.
The code in the switch statement is unchanged except for having to
dereference the status and cause variables (since they're now pointers),
and fixing the style of a block comment to appease checkpatch.
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
security/integrity/ima/ima_appraise.c | 141 +++++++++++++++-----------
1 file changed, 81 insertions(+), 60 deletions(-)
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 18bbe753421a..5d4772f39757 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -202,6 +202,83 @@ int ima_read_xattr(struct dentry *dentry,
return ret;
}
+/*
+ * xattr_verify - verify xattr digest or signature
+ *
+ * Verify whether the hash or signature matches the file contents.
+ *
+ * Return 0 on success, error code otherwise.
+ */
+static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
+ struct evm_ima_xattr_data *xattr_value, int xattr_len,
+ enum integrity_status *status, const char **cause)
+{
+ int rc = -EINVAL, hash_start = 0;
+
+ switch (xattr_value->type) {
+ case IMA_XATTR_DIGEST_NG:
+ /* first byte contains algorithm id */
+ hash_start = 1;
+ /* fall through */
+ case IMA_XATTR_DIGEST:
+ if (iint->flags & IMA_DIGSIG_REQUIRED) {
+ *cause = "IMA-signature-required";
+ *status = INTEGRITY_FAIL;
+ break;
+ }
+ clear_bit(IMA_DIGSIG, &iint->atomic_flags);
+ if (xattr_len - sizeof(xattr_value->type) - hash_start >=
+ iint->ima_hash->length)
+ /*
+ * xattr length may be longer. md5 hash in previous
+ * version occupied 20 bytes in xattr, instead of 16
+ */
+ rc = memcmp(&xattr_value->data[hash_start],
+ iint->ima_hash->digest,
+ iint->ima_hash->length);
+ else
+ rc = -EINVAL;
+ if (rc) {
+ *cause = "invalid-hash";
+ *status = INTEGRITY_FAIL;
+ break;
+ }
+ *status = INTEGRITY_PASS;
+ break;
+ case EVM_IMA_XATTR_DIGSIG:
+ set_bit(IMA_DIGSIG, &iint->atomic_flags);
+ rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
+ (const char *)xattr_value,
+ xattr_len,
+ iint->ima_hash->digest,
+ iint->ima_hash->length);
+ if (rc == -EOPNOTSUPP) {
+ *status = INTEGRITY_UNKNOWN;
+ break;
+ }
+ if (IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING) && rc &&
+ func == KEXEC_KERNEL_CHECK)
+ rc = integrity_digsig_verify(INTEGRITY_KEYRING_PLATFORM,
+ (const char *)xattr_value,
+ xattr_len,
+ iint->ima_hash->digest,
+ iint->ima_hash->length);
+ if (rc) {
+ *cause = "invalid-signature";
+ *status = INTEGRITY_FAIL;
+ } else {
+ *status = INTEGRITY_PASS;
+ }
+ break;
+ default:
+ *status = INTEGRITY_UNKNOWN;
+ *cause = "unknown-ima-data";
+ break;
+ }
+
+ return rc;
+}
+
/*
* ima_appraise_measurement - appraise file measurement
*
@@ -221,7 +298,7 @@ int ima_appraise_measurement(enum ima_hooks func,
struct dentry *dentry = file_dentry(file);
struct inode *inode = d_backing_inode(dentry);
enum integrity_status status = INTEGRITY_UNKNOWN;
- int rc = xattr_len, hash_start = 0;
+ int rc = xattr_len;
if (!(inode->i_opflags & IOP_XATTR))
return INTEGRITY_UNKNOWN;
@@ -259,65 +336,9 @@ int ima_appraise_measurement(enum ima_hooks func,
WARN_ONCE(true, "Unexpected integrity status %d\n", status);
}
- switch (xattr_value->type) {
- case IMA_XATTR_DIGEST_NG:
- /* first byte contains algorithm id */
- hash_start = 1;
- /* fall through */
- case IMA_XATTR_DIGEST:
- if (iint->flags & IMA_DIGSIG_REQUIRED) {
- cause = "IMA-signature-required";
- status = INTEGRITY_FAIL;
- break;
- }
- clear_bit(IMA_DIGSIG, &iint->atomic_flags);
- if (xattr_len - sizeof(xattr_value->type) - hash_start >=
- iint->ima_hash->length)
- /* xattr length may be longer. md5 hash in previous
- version occupied 20 bytes in xattr, instead of 16
- */
- rc = memcmp(&xattr_value->data[hash_start],
- iint->ima_hash->digest,
- iint->ima_hash->length);
- else
- rc = -EINVAL;
- if (rc) {
- cause = "invalid-hash";
- status = INTEGRITY_FAIL;
- break;
- }
- status = INTEGRITY_PASS;
- break;
- case EVM_IMA_XATTR_DIGSIG:
- set_bit(IMA_DIGSIG, &iint->atomic_flags);
- rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
- (const char *)xattr_value,
- xattr_len,
- iint->ima_hash->digest,
- iint->ima_hash->length);
- if (rc == -EOPNOTSUPP) {
- status = INTEGRITY_UNKNOWN;
- break;
- }
- if (IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING) && rc &&
- func == KEXEC_KERNEL_CHECK)
- rc = integrity_digsig_verify(INTEGRITY_KEYRING_PLATFORM,
- (const char *)xattr_value,
- xattr_len,
- iint->ima_hash->digest,
- iint->ima_hash->length);
- if (rc) {
- cause = "invalid-signature";
- status = INTEGRITY_FAIL;
- } else {
- status = INTEGRITY_PASS;
- }
- break;
- default:
- status = INTEGRITY_UNKNOWN;
- cause = "unknown-ima-data";
- break;
- }
+ if (xattr_value)
+ rc = xattr_verify(func, iint, xattr_value, xattr_len, &status,
+ &cause);
out:
/*
^ permalink raw reply related
* [PATCH v11 05/13] integrity: Select CONFIG_KEYS instead of depending on it
From: Thiago Jung Bauermann @ 2019-06-11 6:28 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-doc, linux-kernel, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro, Thiago Jung Bauermann
In-Reply-To: <20190611062817.18412-1-bauerman@linux.ibm.com>
This avoids a dependency cycle in soon-to-be-introduced
CONFIG_IMA_APPRAISE_MODSIG: it will select CONFIG_MODULE_SIG_FORMAT
which in turn selects CONFIG_KEYS. Kconfig then complains that
CONFIG_INTEGRITY_SIGNATURE depends on CONFIG_KEYS.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
security/integrity/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 3ba1168b1756..93d73902c571 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -17,8 +17,8 @@ if INTEGRITY
config INTEGRITY_SIGNATURE
bool "Digital signature verification using multiple keyrings"
- depends on KEYS
default n
+ select KEYS
select SIGNATURE
help
This option enables digital signature verification support
^ permalink raw reply related
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