[PATCH] ARM: vfp: fix misspelled identifier in documentation
The documentation comment for vfp_notifier misspelled THREAD_NOTIFY_SWITCH as THREAD_NOFTIFY_SWITCH. Signed-off-by: Jonas Rabenstein --- arch/arm/vfp/vfpmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c index 2cb355c1b5b7..4f8c6f583b79 100644 --- a/arch/arm/vfp/vfpmodule.c +++ b/arch/arm/vfp/vfpmodule.c @@ -140,7 +140,7 @@ static void vfp_thread_copy(struct thread_info *thread) /* * When this function is called with the following 'cmd's, the following * is true while this function is being run: - * THREAD_NOFTIFY_SWTICH: + * THREAD_NOTIFY_SWITCH: * - the previously running thread will not be scheduled onto another CPU. * - the next thread to be run (v) will not be running on another CPU. * - thread->cpu is the local CPU number -- 2.26.3
[tip:perf/core] perf doc: Fix documentation of the Flags section in perf.data
Commit-ID: 8c23a522388b34cd7bc8473987eda0c75eb37c0e
Gitweb: https://git.kernel.org/tip/8c23a522388b34cd7bc8473987eda0c75eb37c0e
Author: Jonas Rabenstein
AuthorDate: Tue, 19 Feb 2019 16:45:15 +0100
Committer: Arnaldo Carvalho de Melo
CommitDate: Tue, 19 Feb 2019 13:39:12 -0300
perf doc: Fix documentation of the Flags section in perf.data
According to the current documentation the flags section is placed after
the file header itself but the code assumes to find the flags section
after the data section. This change updates the documentation to that
assumption.
Signed-off-by: Jonas Rabenstein
Acked-by: Jiri Olsa
Cc: Alexander Shishkin
Cc: Andi Kleen
Cc: Namhyung Kim
Cc: Peter Zijlstra
Cc: Stephane Eranian
Cc: Thomas Richter
Link:
http://lkml.kernel.org/r/20190219154515.3954-2-jonas.rabenst...@studium.uni-erlangen.de
Signed-off-by: Arnaldo Carvalho de Melo
---
tools/perf/Documentation/perf.data-file-format.txt | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/tools/perf/Documentation/perf.data-file-format.txt
b/tools/perf/Documentation/perf.data-file-format.txt
index 5f9a3924830b..593ef49b273c 100644
--- a/tools/perf/Documentation/perf.data-file-format.txt
+++ b/tools/perf/Documentation/perf.data-file-format.txt
@@ -43,11 +43,10 @@ struct perf_file_section {
Flags section:
-The header is followed by different optional headers, described by the bits set
-in flags. Only headers for which the bit is set are included. Each header
-consists of a perf_file_section located after the initial header.
-The respective perf_file_section points to the data of the additional
-header and defines its size.
+For each of the optional features a perf_file_section it placed after the data
+section if the feature bit is set in the perf_header flags bitset. The
+respective perf_file_section points to the data of the additional header and
+defines its size.
Some headers consist of strings, which are defined like this:
[tip:perf/core] perf doc: Fix HEADER_CMDLINE description in perf.data documentation
Commit-ID: 7a663c0ff330a068c088ad46bb0130e651a9fec3 Gitweb: https://git.kernel.org/tip/7a663c0ff330a068c088ad46bb0130e651a9fec3 Author: Jonas Rabenstein AuthorDate: Tue, 19 Feb 2019 16:45:14 +0100 Committer: Arnaldo Carvalho de Melo CommitDate: Tue, 19 Feb 2019 13:39:08 -0300 perf doc: Fix HEADER_CMDLINE description in perf.data documentation The content of the HEADER_CMDLINE feature header is a perf_header_string_list of the argument vector and not a perf_header_string of the commandline. Signed-off-by: Jonas Rabenstein Acked-by: Jiri Olsa Cc: Alexander Shishkin Cc: Andi Kleen Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Thomas Richter Link: http://lkml.kernel.org/r/20190219154515.3954-1-jonas.rabenst...@studium.uni-erlangen.de Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/Documentation/perf.data-file-format.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt index dfb218feaad9..5f9a3924830b 100644 --- a/tools/perf/Documentation/perf.data-file-format.txt +++ b/tools/perf/Documentation/perf.data-file-format.txt @@ -131,7 +131,7 @@ An uint64_t with the total memory in bytes. HEADER_CMDLINE = 11, -A perf_header_string with the perf command line used to collect the data. +A perf_header_string_list with the perf arg-vector used to collect the data. HEADER_EVENT_DESC = 12,
Re: [PATCH 3/3] perf machine: add inline symbols to callchains
On Sat, Feb 23, 2019 at 01:21:34PM +0900, Namhyung Kim wrote:
> Hello,
>
> On Fri, Feb 22, 2019 at 1:07 AM Jonas Rabenstein
> wrote:
> >
> > Use map__inlines to resolve inlined functions for every entry with
> > an symbol that should be added to a callchain.
> >
> > Signed-off-by: Jonas Rabenstein
> > ---
> > tools/perf/util/machine.c | 115 --
> > 1 file changed, 87 insertions(+), 28 deletions(-)
> >
> > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > index dce29c21e4ea..070d074482b4 100644
> > --- a/tools/perf/util/machine.c
> > +++ b/tools/perf/util/machine.c
> > @@ -1908,6 +1908,91 @@ struct iterations {
> > u64 cycles;
> > };
> >
> > +static int __add_callchain_location(struct callchain_cursor *cursor,
> > + struct symbol **parent,
> > + struct addr_location *root_al,
> > + u64 addr, struct addr_location *al,
> > + bool branch, struct branch_flags *flags,
> > + u64 branch_from, struct iterations
> > *iter)
> > +{
> > + int nr_loop_iter = 0;
> > + u64 iter_cycles = 0;
> > +
> > + if (symbol_conf.hide_unresolved && al->sym == NULL)
> > + return 0;
> > +
> > + if (al->sym) {
> > + if (perf_hpp_list.parent && !*parent &&
> > + symbol__match_regex(al->sym, &parent_regex))
> > + *parent = al->sym;
> > + else if (have_ignore_callees && root_al &&
> > + symbol__match_regex(al->sym, &ignore_callees_regex)) {
> > + /* Treat this symbol as the root,
> > + forgetting its callees. */
> > + *root_al = *al;
> > + callchain_cursor_reset(cursor);
> > + }
> > + }
> > +
> > + if (iter) {
> > + nr_loop_iter = iter->nr_loop_iter;
> > + iter_cycles = iter->cycles;
> > + }
> > + return callchain_cursor_append(cursor, addr, al->map, al->sym,
> > branch,
> > + flags, nr_loop_iter, iter_cycles,
> > + branch_from, al->srcline);
> > +}
> > +
> > +static int __add_callchain_ip(struct callchain_cursor *cursor, u64 ip,
> > + struct addr_location *al, bool branch,
> > + struct branch_flags *flags, u64 branch_from,
> > + struct iterations *iter, struct symbol
> > **parent,
> > + struct addr_location *root_al)
> > +{
> > + struct inline_node *inline_node;
> > + struct inline_list *inline_list;
> > + const char *srcline;
> > + struct symbol *symbol;
> > + int err = 0;
> > +
> > + al->srcline = callchain_srcline(al->map, al->sym, al->addr);
> > + if (callchain_param.order == ORDER_CALLER)
> > + err = __add_callchain_location(cursor, parent, root_al, ip,
> > al,
> > + branch, flags, branch_from,
> > iter);
> > + if (err || !al->map || !al->sym)
> > + goto no_inline;
> > +
> > + inline_node = map__inlines(al->map, ip, al->sym);
> > + if (!inline_node || list_empty(&inline_node->val))
> > + goto no_inline;
> > +
> > + symbol = al->sym;
> > + srcline = al->srcline;
> > + list_for_each_entry(inline_list, &inline_node->val, list) {
> > + if (inline_list->symbol == symbol)
> > + continue;
> > + al->sym = inline_list->symbol;
> > + al->srcline = inline_list->srcline;
> > + err = __add_callchain_location(cursor, parent, root_al, ip,
> > + al, branch, flags,
> > + branch_from, iter);
> > + if (err)
> > + break;
> > + }
>
> Does this loop do the job both for ORDER_CALLER and ORDER_CALLEE?
The list of inline symbols is already build in he correct order
depending on callchain_p
[PATCH 0/3] Support inline symbols in callchains
Hi,
This patchset supersedes my previous attempt to add inline symbols to
callchain of perf-script [1] by a more generic attempt to not hook in
the output stage but directly into the callchain generation. By a matter
of fact this adds those inline symbols automatically to other commands
like perf-report.
Additionally this fixes the regression reported by Jiri Olsa [2] that
some entries from previous outputs had been vanished and now only new
lines are added if symbols had been found.
The integration for perf-report is not completely done as there is an
issue if the root for an hist_entry as for inlined symbols there may be
multiple instances (for each address-range) with the same name. But in
util/sort.c:233 only the name is compared for inlined symbols. As a
consequence the returned hist_entry may hold a reference to another
instance for this inlined symbol (with another address range than
requested) we later on fail with -ERANGE in __symbol__inc_addr_samples
(util/annotate.c:857).
This issue does still permit perf-report to be executed without any
problems and the inlined symbols do show up but none of the samples is
actually accounted to them but to the original symbol :(
To further provide information what this changeset is doing, here is a
script-session to show the differences in the output:
[jonas@x60s]$ git reset --hard v5.0-rc7; make -C tools/perf >/dev/null; \
HEAD is now at a3b22b9f11d9 Linux 5.0-rc7
[jonas@x60s]$ git am *.patch; make -C tools/perf >/dev/null; \
mv tools /perf/perf perf-new
Applying: perf map: add function to lookup inline symbols
Applying: perf machine: use map__inlines in append_inlines
Applying: perf machine: add inline symbols to callchains
[jonas@x60s]$ cat test.c
static int foo(int m){int r=0; for(int i=0;iold.script
real0m0.039s
user0m0.012s
sys 0m0.022s
[jonas@x60s]$ time ./perf-new script >new.script
real0m0.045s
user0m0.010s
sys 0m0.032s
[jonas@x60s]$ diff -u old.script new.script
--- old.script 2019-02-21 16:30:47.997194180 +0100
+++ new.script 2019-02-21 16:30:51.757309790 +0100
@@ -23,66 +23,101 @@
7f43b1148090 _start+0x0 (/lib/x86_64-linux-gnu/ld-2.27.so)
test 7579 1470536.968092: 354758 cycles:uppp:
+ 7f43b1148f4b elf_get_dynamic_info+0xab (inlined)
7f43b1148f4b _dl_start+0xab (/lib/x86_64-linux-gnu/ld-2.27.so)
7f43b1148098 _dl_start_user+0x0 (/lib/x86_64-linux-gnu/ld-2.27.so)
[...]
test 7579 1470536.969210:1922435 cycles:uppp:
+ 563f0df9c53f foo+0x4f (inlined)
+ 563f0df9c53f bar+0x4f (inlined)
+ 563f0df9c53f baz+0x4f (inlined)
563f0df9c53f main+0x4f (/home/jonas/linux/test)
7f43b0d77b97 __libc_start_main+0xe7
(/lib/x86_64-linux-gnu/libc-2.27.so)
75e258d4c544155 [unknown] ([unknown])
[jonas@x60s]$ time ./perf-new script --no-inline >new.noinline.script
real0m0.035s
user0m0.012s
sys 0m0.020s
[jonas@x60s]$ diff -u old.script new.noinline.script
[jonas@x60s]$ ./perf-old report --stdio --quiet | sed '/^$/Q'
88.80%88.80% test test [.] main
|
---0x75e258d4c544155
__libc_start_main
main
[jonas@x60s]$ ./perf-new report --stdio --quiet | sed '/^$/Q'
88.80%88.80% test test [.] main
|
---0x75e258d4c544155
__libc_start_main
main
baz (inlined)
bar (inlined)
foo (inlined)
[jonas@x60s]$ ./perf-new report --stdio --quiet --no-inline | sed '/^$/Q'
88.80%88.80% test test [.] main
|
---0x75e258d4c544155
__libc_start_main
main
I am still trying to find a way that in the new output of report the
88.80% 'Self' are not accounted to main itself but split up for the
inlined baz, bar and foo symbols.
I'm open to any help as well as feedback,
Jonas
[1] https://www.spinics.net/lists/linux-perf-users/msg07792.html
[2] https://www.spinics.net/lists/linux-perf-users/msg07798.html
Jonas Rabenstein (3):
perf map: add function to lookup inline symbols
perf machine: use map__inlines in append_inlines
perf machine: add inline symbols to callchains
tools/perf/util/machine.c | 130 ++
tools/perf/util/map.c | 23 +++
tools/perf/util/map.h | 3 +
3 files changed, 115 insertions(+), 41 deletions(-)
--
2.19.2
[PATCH 2/3] perf machine: use map__inlines in append_inlines
The previous patch provides a generic way to lookup the root node of the
inlined symbols for a specific address. Reuse that implementation
instead of a duplicated version of the required steps.
Signed-off-by: Jonas Rabenstein
---
tools/perf/util/machine.c | 15 ++-
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 143f7057d581..dce29c21e4ea 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -2330,23 +2330,12 @@ static int append_inlines(struct callchain_cursor
*cursor,
{
struct inline_node *inline_node;
struct inline_list *ilist;
- u64 addr;
int ret = 1;
- if (!symbol_conf.inline_name || !map || !sym)
+ inline_node = map__inlines(map, ip, sym);
+ if (!inline_node)
return ret;
- addr = map__map_ip(map, ip);
- addr = map__rip_2objdump(map, addr);
-
- inline_node = inlines__tree_find(&map->dso->inlined_nodes, addr);
- if (!inline_node) {
- inline_node = dso__parse_addr_inlines(map->dso, addr, sym);
- if (!inline_node)
- return ret;
- inlines__tree_insert(&map->dso->inlined_nodes, inline_node);
- }
-
list_for_each_entry(ilist, &inline_node->val, list) {
ret = callchain_cursor_append(cursor, ip, map,
ilist->symbol, false,
--
2.19.2
[PATCH 3/3] perf machine: add inline symbols to callchains
Use map__inlines to resolve inlined functions for every entry with
an symbol that should be added to a callchain.
Signed-off-by: Jonas Rabenstein
---
tools/perf/util/machine.c | 115 --
1 file changed, 87 insertions(+), 28 deletions(-)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index dce29c21e4ea..070d074482b4 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1908,6 +1908,91 @@ struct iterations {
u64 cycles;
};
+static int __add_callchain_location(struct callchain_cursor *cursor,
+ struct symbol **parent,
+ struct addr_location *root_al,
+ u64 addr, struct addr_location *al,
+ bool branch, struct branch_flags *flags,
+ u64 branch_from, struct iterations *iter)
+{
+ int nr_loop_iter = 0;
+ u64 iter_cycles = 0;
+
+ if (symbol_conf.hide_unresolved && al->sym == NULL)
+ return 0;
+
+ if (al->sym) {
+ if (perf_hpp_list.parent && !*parent &&
+ symbol__match_regex(al->sym, &parent_regex))
+ *parent = al->sym;
+ else if (have_ignore_callees && root_al &&
+ symbol__match_regex(al->sym, &ignore_callees_regex)) {
+ /* Treat this symbol as the root,
+ forgetting its callees. */
+ *root_al = *al;
+ callchain_cursor_reset(cursor);
+ }
+ }
+
+ if (iter) {
+ nr_loop_iter = iter->nr_loop_iter;
+ iter_cycles = iter->cycles;
+ }
+ return callchain_cursor_append(cursor, addr, al->map, al->sym, branch,
+ flags, nr_loop_iter, iter_cycles,
+ branch_from, al->srcline);
+}
+
+static int __add_callchain_ip(struct callchain_cursor *cursor, u64 ip,
+ struct addr_location *al, bool branch,
+ struct branch_flags *flags, u64 branch_from,
+ struct iterations *iter, struct symbol **parent,
+ struct addr_location *root_al)
+{
+ struct inline_node *inline_node;
+ struct inline_list *inline_list;
+ const char *srcline;
+ struct symbol *symbol;
+ int err = 0;
+
+ al->srcline = callchain_srcline(al->map, al->sym, al->addr);
+ if (callchain_param.order == ORDER_CALLER)
+ err = __add_callchain_location(cursor, parent, root_al, ip, al,
+ branch, flags, branch_from,
iter);
+ if (err || !al->map || !al->sym)
+ goto no_inline;
+
+ inline_node = map__inlines(al->map, ip, al->sym);
+ if (!inline_node || list_empty(&inline_node->val))
+ goto no_inline;
+
+ symbol = al->sym;
+ srcline = al->srcline;
+ list_for_each_entry(inline_list, &inline_node->val, list) {
+ if (inline_list->symbol == symbol)
+ continue;
+ al->sym = inline_list->symbol;
+ al->srcline = inline_list->srcline;
+ err = __add_callchain_location(cursor, parent, root_al, ip,
+ al, branch, flags,
+ branch_from, iter);
+ if (err)
+ break;
+ }
+
+ if (callchain_param.order == ORDER_CALLEE) {
+ al->srcline = srcline;
+ al->sym = symbol;
+ }
+
+no_inline:
+ if (!err && callchain_param.order == ORDER_CALLEE)
+ err = __add_callchain_location(cursor, parent, root_al, ip, al,
+ branch, flags, branch_from,
iter);
+ return err;
+}
+
+
static int add_callchain_ip(struct thread *thread,
struct callchain_cursor *cursor,
struct symbol **parent,
@@ -1920,9 +2005,6 @@ static int add_callchain_ip(struct thread *thread,
u64 branch_from)
{
struct addr_location al;
- int nr_loop_iter = 0;
- u64 iter_cycles = 0;
- const char *srcline = NULL;
al.filtered = 0;
al.sym = NULL;
@@ -1955,31 +2037,8 @@ static int add_callchain_ip(struct thread *thread,
thread__find_symbol(thread, *cpumode, ip, &al);
}
- if (al.sym != NULL) {
- if (perf_hpp_list.parent && !*parent &&
- symbol__match_regex(al.sym, &parent_regex))
- *parent = al.sym;
-
[PATCH 1/3] perf map: add function to lookup inline symbols
Inlined symbols should always be added to the dso's inlined_nodes tree
in order to reuse them for a later lookup for the same address. Instead
of repeating those steps at the different users, provide a central
method to lookup and register inline symbols for a map.
Signed-off-by: Jonas Rabenstein
---
tools/perf/util/map.c | 23 +++
tools/perf/util/map.h | 3 +++
2 files changed, 26 insertions(+)
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 6751301a755c..0fe74e83ca8d 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -988,3 +988,26 @@ struct map_groups *map__kmaps(struct map *map)
}
return kmap->kmaps;
}
+
+struct inline_node *map__inlines(struct map *map, u64 addr, struct symbol *sym)
+{
+ struct inline_node *node;
+
+ if (!symbol_conf.inline_name)
+ return NULL;
+
+ if (!map->dso)
+ return NULL;
+
+ addr = map->map_ip(map, addr);
+ addr = map__rip_2objdump(map, addr);
+
+ node = inlines__tree_find(&map->dso->inlined_nodes, addr);
+ if (node)
+ return node;
+
+ node = dso__parse_addr_inlines(map->dso, addr, sym);
+ if (node)
+ inlines__tree_insert(&map->dso->inlined_nodes, node);
+ return node;
+}
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 09282aa45c80..bdcc6e77e26e 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -270,4 +270,7 @@ static inline bool is_entry_trampoline(const char *name)
return !strcmp(name, ENTRY_TRAMPOLINE_NAME);
}
+struct inline_node *map__inlines(struct map *map, u64 addr,
+struct symbol *sym);
+
#endif /* __PERF_MAP_H */
--
2.19.2
Re: [PATCH] perf hist: fix memory leak if histogram entry is reused
On Thu, Feb 21, 2019 at 01:39:09PM +0100, Jiri Olsa wrote: > On Thu, Feb 21, 2019 at 01:23:06PM +0100, Jonas Rabenstein wrote: > > In __hists__add_entry the srcline of the addr_location is duplicated > > for the hist_entry. If hists__findnew_entry returns an already existing > > hist_entry the srcline has to be freed again as no further reference to > > that duplicated srcline would exists anymore. > > > > Signed-off-by: Jonas Rabenstein > > --- > > tools/perf/util/hist.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c > > index 8aad8330e392..25b8dbdbbe87 100644 > > --- a/tools/perf/util/hist.c > > +++ b/tools/perf/util/hist.c > > @@ -623,6 +623,9 @@ __hists__add_entry(struct hists *hists, > > .ops = ops, > > }, *he = hists__findnew_entry(hists, &entry, al, sample_self); > > > > + if (he && he->srcline != entry.srcline) > > + free(entry.srcline); > > + > > if (!hists->has_callchains && he && he->callchain_size != 0) > > hists->has_callchains = true; > > return he; > > nice, do we have similar issue in here? > > jirka > > > --- > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c > index 216388003dea..e65e6822c848 100644 > --- a/tools/perf/util/hist.c > +++ b/tools/perf/util/hist.c > @@ -966,7 +966,7 @@ iter_add_next_cumulative_entry(struct hist_entry_iter > *iter, > .map = al->map, > .sym = al->sym, > }, > - .srcline = al->srcline ? strdup(al->srcline) : NULL, > + .srcline = al->srcline, While this shouldn't leak the memory, we may end up with an al->srcline to get free afterwards while still having a reference on it within the hist_entry... Also I could not find where/how the hist_entry is freed up so we may get an double free if both of al and he clean the srcline. Of course, with your solution we could spare the useless strdup/free if we find an hist_entry (which should be the typical case for hotspots..). Taking a deeper look thus should be beneficial - but I do not have the time for that right now because I'm still working on the inline-symbol integration which is more important for me... - Jonas > .parent = iter->parent, > .raw_data = sample->raw_data, > .raw_size = sample->raw_size,
[PATCH] perf hist: fix memory leak if histogram entry is reused
In __hists__add_entry the srcline of the addr_location is duplicated for the hist_entry. If hists__findnew_entry returns an already existing hist_entry the srcline has to be freed again as no further reference to that duplicated srcline would exists anymore. Signed-off-by: Jonas Rabenstein --- tools/perf/util/hist.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index 8aad8330e392..25b8dbdbbe87 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -623,6 +623,9 @@ __hists__add_entry(struct hists *hists, .ops = ops, }, *he = hists__findnew_entry(hists, &entry, al, sample_self); + if (he && he->srcline != entry.srcline) + free(entry.srcline); + if (!hists->has_callchains && he && he->callchain_size != 0) hists->has_callchains = true; return he; -- 2.19.2
[PATCHv2 2/2] perf evsel: add inline functions to sample callchain output
Whenever a callchain shall be printed search for each address whether
inline information is available and add those symbols to the output
if symbol_conf.inline_name is enabled.
Signed-off-by: Jonas Rabenstein
---
v2:
- fix handling of static binaries
tools/perf/util/evsel_fprintf.c | 55 +
1 file changed, 49 insertions(+), 6 deletions(-)
diff --git a/tools/perf/util/evsel_fprintf.c b/tools/perf/util/evsel_fprintf.c
index c710f687ddf4..a8d6465af1aa 100644
--- a/tools/perf/util/evsel_fprintf.c
+++ b/tools/perf/util/evsel_fprintf.c
@@ -168,6 +168,52 @@ static int __fprintf_callchain_link(u64 ip, struct map
*map, struct symbol *symb
return printed;
}
+static int __fprintf_callchain(struct callchain_cursor_node *node, bool first,
+ int left_alignment, unsigned int print_opts,
+ FILE *fp)
+{
+ int printed = 0;
+ struct inline_node *inline_node;
+ struct inline_list *ilist;
+ u64 addr;
+
+ if (!symbol_conf.inline_name)
+ goto no_inline;
+
+ if (!node->map || !node->map->dso)
+ goto no_inline;
+
+ addr = node->map->map_ip(node->map, node->ip);
+ addr = map__rip_2objdump(node->map, addr);
+
+ inline_node = inlines__tree_find(&node->map->dso->inlined_nodes,
+addr);
+ if (!inline_node) {
+ inline_node = dso__parse_addr_inlines(node->map->dso,
+ addr, node->sym);
+ if (!inline_node)
+ goto no_inline;
+ inlines__tree_insert(&node->map->dso->inlined_nodes,
+inline_node);
+ }
+
+ list_for_each_entry(ilist, &inline_node->val, list) {
+ printed += __fprintf_callchain_link(node->ip, node->map,
+ ilist->symbol,
+ ilist->srcline,
+ first, left_alignment,
+ print_opts, fp);
+ first = (first && printed == 0);
+ }
+
+ return printed;
+
+no_inline:
+ return __fprintf_callchain_link(node->ip, node->map, node->sym,
+ NULL, first, left_alignment,
+ print_opts, fp);
+}
+
int sample__fprintf_callchain(struct perf_sample *sample, int left_alignment,
unsigned int print_opts, struct callchain_cursor
*cursor,
FILE *fp)
@@ -183,12 +229,9 @@ int sample__fprintf_callchain(struct perf_sample *sample,
int left_alignment,
if (!node)
break;
-
- printed += __fprintf_callchain_link(node->ip, node->map,
- node->sym, NULL,
- (printed == 0),
- left_alignment,
- print_opts, fp);
+ printed += __fprintf_callchain(node, (printed == 0),
+ left_alignment,
+ print_opts, fp);
/* Add srccode here too? */
if (symbol_conf.bt_stop_list &&
--
2.19.2
Re: [PATCH 2/2] perf evsel: add inline functions to sample callchain output
On Tue, Feb 19, 2019 at 07:38:10PM +0100, Jonas Rabenstein wrote:
> Whenever a callchain shall be printed search for each address whether
> inline information is available and add those symbols to the output
> if symbol_conf.inline_name is enabled.
>
> Signed-off-by: Jonas Rabenstein
> ---
> tools/perf/util/evsel_fprintf.c | 46 +
> 1 file changed, 46 insertions(+)
>
> diff --git a/tools/perf/util/evsel_fprintf.c b/tools/perf/util/evsel_fprintf.c
> index c710f687ddf4..9f9ece453dc4 100644
> --- a/tools/perf/util/evsel_fprintf.c
> +++ b/tools/perf/util/evsel_fprintf.c
> @@ -168,6 +168,49 @@ static int __fprintf_callchain_link(u64 ip, struct map
> *map, struct symbol *symb
> return printed;
> }
>
> +static int __fprintf_callchain_inlines(struct callchain_cursor_node *node,
> +bool first, int left_alignment,
> +unsigned int print_opts, FILE *fp)
> +{
> + int printed = 0;
> + struct inline_node *inline_node;
> + struct inline_list *ilist;
> + u64 addr;
> +
> + if (!symbol_conf.inline_name)
> + return 0;
> +
> + if (!node->map || !node->map->dso)
> + return 0;
> +
> + addr = node->map->map_ip(node->map, node->ip);
I stumbled across a problem with the usage of map_ip here. Using the map_ip
on dynamic binaries does resolve the inline symbols but it fails with static
ones. Using node->ip directly works for static binaries but fails for
static ones...
Is there a way to decide whether node->map->dso is a static or dynamic
object file? Furthermore, is it even correct for a static dso to have
map->map_ip set to map__map_ip instead of identity__map_ip?
> +
> + inline_node = inlines__tree_find(&node->map->dso->inlined_nodes,
> + addr);
> + if (!inline_node) {
> + inline_node = dso__parse_addr_inlines(node->map->dso,
> + addr, node->sym);
> + if (!inline_node)
> + return 0;
> + inlines__tree_insert(&node->map->dso->inlined_nodes,
> + inline_node);
> + }
> +
> + list_for_each_entry(ilist, &inline_node->val, list) {
> + if (ilist->symbol == node->sym)
> + break;
> +
> + printed += __fprintf_callchain_link(node->ip, node->map,
> + ilist->symbol,
> + ilist->srcline,
> + first, left_alignment,
> + print_opts, fp);
> + first = (first && printed == 0);
> + }
> +
> + return printed;
> +}
> +
> int sample__fprintf_callchain(struct perf_sample *sample, int left_alignment,
> unsigned int print_opts, struct callchain_cursor
> *cursor,
> FILE *fp)
> @@ -183,6 +226,9 @@ int sample__fprintf_callchain(struct perf_sample *sample,
> int left_alignment,
> if (!node)
> break;
>
> + printed += __fprintf_callchain_inlines(node, (printed
> == 0),
> +left_alignment,
> +print_opts, fp);
>
> printed += __fprintf_callchain_link(node->ip, node->map,
> node->sym, NULL,
> --
> 2.19.2
>
[PATCH 0/2] perf evsel: add support for inlined function in callchains
Hi, sample__fprintf_callchain currently did not use the already available code to get the symbols of an inlined function if such information is available in a dso. This patchset adds the required logic to add appropriate lines. As I am quite new to the code base of perf I am not sure how to test that changeset in a correct way. At least the codes builds and the tools that make use of sample__fprintf_callchain (perf-script, perf-trace and perf-sched as far as I can see) did not fail to run - also I did not get into the details of perf-trace and perf-sched as I have never used them before. Another thing I am not sure how to deal with are some warnings of checkpatch.pl due to the 80 character line limit. Due to the long function names in use the current implementation already exceeded that limit in the same spots by even more characters as I have taken the inner loop and put it into a separate function. I hope to expand my work to perf-report but thought it might be usefull to get already early feedback on those patches. Thank you, Jonas Jonas Rabenstein (2): perf evsel: split sample__fprintf_callchain in output and iteration perf evsel: add inline functions to sample callchain output tools/perf/util/evsel_fprintf.c | 157 ++-- 1 file changed, 109 insertions(+), 48 deletions(-) -- 2.19.2
[PATCH 1/2] perf evsel: split sample__fprintf_callchain in output and iteration
Split the iteration over the callchain and the actual output of an
link in the callchain into separate functions. This allows to reuse
the output function in a follow up patch to add inline symbols to
the output.
Signed-off-by: Jonas Rabenstein
---
tools/perf/util/evsel_fprintf.c | 111 ++--
1 file changed, 63 insertions(+), 48 deletions(-)
diff --git a/tools/perf/util/evsel_fprintf.c b/tools/perf/util/evsel_fprintf.c
index 95ea147f9e18..c710f687ddf4 100644
--- a/tools/perf/util/evsel_fprintf.c
+++ b/tools/perf/util/evsel_fprintf.c
@@ -99,12 +99,12 @@ int perf_evsel__fprintf(struct perf_evsel *evsel,
return ++printed;
}
-int sample__fprintf_callchain(struct perf_sample *sample, int left_alignment,
- unsigned int print_opts, struct callchain_cursor
*cursor,
- FILE *fp)
+static int __fprintf_callchain_link(u64 ip, struct map *map, struct symbol
*symbol,
+ const char *srcline, bool first, int
left_alignment,
+ unsigned int print_opts, FILE *fp)
{
+ u64 addr = 0;
int printed = 0;
- struct callchain_cursor_node *node;
int print_ip = print_opts & EVSEL__PRINT_IP;
int print_sym = print_opts & EVSEL__PRINT_SYM;
int print_dso = print_opts & EVSEL__PRINT_DSO;
@@ -115,63 +115,80 @@ int sample__fprintf_callchain(struct perf_sample *sample,
int left_alignment,
int print_arrow = print_opts & EVSEL__PRINT_CALLCHAIN_ARROW;
int print_skip_ignored = print_opts & EVSEL__PRINT_SKIP_IGNORED;
char s = print_oneline ? ' ' : '\t';
- bool first = true;
+ struct addr_location node_al;
- if (sample->callchain) {
- struct addr_location node_al;
- callchain_cursor_commit(cursor);
+ if (symbol && symbol->ignore && print_skip_ignored)
+ return 0;
- while (1) {
- u64 addr = 0;
+ printed += fprintf(fp, "%-*.*s", left_alignment, left_alignment, " ");
- node = callchain_cursor_current(cursor);
- if (!node)
- break;
+ if (print_arrow && !first)
+ printed += fprintf(fp, " <-");
- if (node->sym && node->sym->ignore &&
print_skip_ignored)
- goto next;
+ if (print_ip)
+ printed += fprintf(fp, "%c%16" PRIx64, s, ip);
- printed += fprintf(fp, "%-*.*s", left_alignment,
left_alignment, " ");
+ if (map)
+ addr = map->map_ip(map, ip);
- if (print_arrow && !first)
- printed += fprintf(fp, " <-");
+ if (print_sym) {
+ printed += fprintf(fp, " ");
+ node_al.addr = addr;
+ node_al.map = map;
- if (print_ip)
- printed += fprintf(fp, "%c%16" PRIx64, s,
node->ip);
+ if (print_symoffset) {
+ printed += __symbol__fprintf_symname_offs(symbol,
&node_al,
+
print_unknown_as_addr,
+ true, fp);
+ } else {
+ printed += __symbol__fprintf_symname(symbol, &node_al,
+
print_unknown_as_addr,
+fp);
+ }
+ }
- if (node->map)
- addr = node->map->map_ip(node->map, node->ip);
+ if (print_dso && (!symbol || !symbol->inlined)) {
+ printed += fprintf(fp, " (");
+ printed += map__fprintf_dsoname(map, fp);
+ printed += fprintf(fp, ")");
+ }
- if (print_sym) {
- printed += fprintf(fp, " ");
- node_al.addr = addr;
- node_al.map = node->map;
+ if (print_srcline && srcline)
+ printed += fprintf(fp, "\n %s", srcline);
+ else if (print_srcline)
+ printed += map__fprintf_srcline(map, addr, "\n ", fp);
- if (print_symoffset) {
- printed +=
__symbol__fprintf_symname_offs(node->sym, &node_al,
-
[PATCH 2/2] perf evsel: add inline functions to sample callchain output
Whenever a callchain shall be printed search for each address whether
inline information is available and add those symbols to the output
if symbol_conf.inline_name is enabled.
Signed-off-by: Jonas Rabenstein
---
tools/perf/util/evsel_fprintf.c | 46 +
1 file changed, 46 insertions(+)
diff --git a/tools/perf/util/evsel_fprintf.c b/tools/perf/util/evsel_fprintf.c
index c710f687ddf4..9f9ece453dc4 100644
--- a/tools/perf/util/evsel_fprintf.c
+++ b/tools/perf/util/evsel_fprintf.c
@@ -168,6 +168,49 @@ static int __fprintf_callchain_link(u64 ip, struct map
*map, struct symbol *symb
return printed;
}
+static int __fprintf_callchain_inlines(struct callchain_cursor_node *node,
+ bool first, int left_alignment,
+ unsigned int print_opts, FILE *fp)
+{
+ int printed = 0;
+ struct inline_node *inline_node;
+ struct inline_list *ilist;
+ u64 addr;
+
+ if (!symbol_conf.inline_name)
+ return 0;
+
+ if (!node->map || !node->map->dso)
+ return 0;
+
+ addr = node->map->map_ip(node->map, node->ip);
+
+ inline_node = inlines__tree_find(&node->map->dso->inlined_nodes,
+addr);
+ if (!inline_node) {
+ inline_node = dso__parse_addr_inlines(node->map->dso,
+ addr, node->sym);
+ if (!inline_node)
+ return 0;
+ inlines__tree_insert(&node->map->dso->inlined_nodes,
+inline_node);
+ }
+
+ list_for_each_entry(ilist, &inline_node->val, list) {
+ if (ilist->symbol == node->sym)
+ break;
+
+ printed += __fprintf_callchain_link(node->ip, node->map,
+ ilist->symbol,
+ ilist->srcline,
+ first, left_alignment,
+ print_opts, fp);
+ first = (first && printed == 0);
+ }
+
+ return printed;
+}
+
int sample__fprintf_callchain(struct perf_sample *sample, int left_alignment,
unsigned int print_opts, struct callchain_cursor
*cursor,
FILE *fp)
@@ -183,6 +226,9 @@ int sample__fprintf_callchain(struct perf_sample *sample,
int left_alignment,
if (!node)
break;
+ printed += __fprintf_callchain_inlines(node, (printed
== 0),
+ left_alignment,
+ print_opts, fp);
printed += __fprintf_callchain_link(node->ip, node->map,
node->sym, NULL,
--
2.19.2
[PATCH] perf: fix documentation of the Flags section in perf.data
According to the current documentation the flags section is placed after
the file header itself but the code assumes to find the flags section
after the data section. This change updates the documentation to that
assumption.
Acked-by: Jiri Olsa
Signed-off-by: Jonas Rabenstein
---
tools/perf/Documentation/perf.data-file-format.txt | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/tools/perf/Documentation/perf.data-file-format.txt
b/tools/perf/Documentation/perf.data-file-format.txt
index 5f9a3924830b..593ef49b273c 100644
--- a/tools/perf/Documentation/perf.data-file-format.txt
+++ b/tools/perf/Documentation/perf.data-file-format.txt
@@ -43,11 +43,10 @@ struct perf_file_section {
Flags section:
-The header is followed by different optional headers, described by the bits set
-in flags. Only headers for which the bit is set are included. Each header
-consists of a perf_file_section located after the initial header.
-The respective perf_file_section points to the data of the additional
-header and defines its size.
+For each of the optional features a perf_file_section it placed after the data
+section if the feature bit is set in the perf_header flags bitset. The
+respective perf_file_section points to the data of the additional header and
+defines its size.
Some headers consist of strings, which are defined like this:
--
2.19.2
[PATCH] perf: fix HEADER_CMDLINE description in perf.data documentation
The content of this feature header is a perf_header_string_list of the argument vector and not a perf_header_string of the commandline. Acked-by: Jiri Olsa Signed-off-by: Jonas Rabenstein --- tools/perf/Documentation/perf.data-file-format.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt index dfb218feaad9..5f9a3924830b 100644 --- a/tools/perf/Documentation/perf.data-file-format.txt +++ b/tools/perf/Documentation/perf.data-file-format.txt @@ -131,7 +131,7 @@ An uint64_t with the total memory in bytes. HEADER_CMDLINE = 11, -A perf_header_string with the perf command line used to collect the data. +A perf_header_string_list with the perf arg-vector used to collect the data. HEADER_EVENT_DESC = 12, -- 2.19.2
[PATCH] perf: fix documentation of the Flags section in perf.data
According to the current documentation the flags section is placed after
the file header itself but the code assumes to find the flags section
after the data section. This change updates the documentation to that
assumption.
---
tools/perf/Documentation/perf.data-file-format.txt | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/tools/perf/Documentation/perf.data-file-format.txt
b/tools/perf/Documentation/perf.data-file-format.txt
index 5f9a3924830b..593ef49b273c 100644
--- a/tools/perf/Documentation/perf.data-file-format.txt
+++ b/tools/perf/Documentation/perf.data-file-format.txt
@@ -43,11 +43,10 @@ struct perf_file_section {
Flags section:
-The header is followed by different optional headers, described by the bits set
-in flags. Only headers for which the bit is set are included. Each header
-consists of a perf_file_section located after the initial header.
-The respective perf_file_section points to the data of the additional
-header and defines its size.
+For each of the optional features a perf_file_section it placed after the data
+section if the feature bit is set in the perf_header flags bitset. The
+respective perf_file_section points to the data of the additional header and
+defines its size.
Some headers consist of strings, which are defined like this:
--
2.19.2
[PATCH] perf: fix HEADER_CMDLINE description in perf.data documentation
The content of this feature header is a perf_header_string_list of the argument vector and not a perf_header_string of the commandline. --- tools/perf/Documentation/perf.data-file-format.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt index dfb218feaad9..5f9a3924830b 100644 --- a/tools/perf/Documentation/perf.data-file-format.txt +++ b/tools/perf/Documentation/perf.data-file-format.txt @@ -131,7 +131,7 @@ An uint64_t with the total memory in bytes. HEADER_CMDLINE = 11, -A perf_header_string with the perf command line used to collect the data. +A perf_header_string_list with the perf arg-vector used to collect the data. HEADER_EVENT_DESC = 12, -- 2.19.2
Re: [PATCH] perf: update perf.data file format documentation
On Mon, Feb 18, 2019 at 12:22:46AM +0100, Jiri Olsa wrote:
> On Fri, Feb 15, 2019 at 07:28:23PM +0100, Jonas Rabenstein wrote:
> > I found that the documentation of the flags section is some how
> > different from the actual format used and expected by the perf
> > tools. In this patch the according section of the file format
> > documentation is updated to conform to the expectations of the
> > perf tool suite.
> >
> > Signed-off-by: Jonas Rabenstein
> > ---
> > .../perf/Documentation/perf.data-file-format.txt | 15 ++-
> > 1 file changed, 6 insertions(+), 9 deletions(-)
> >
> > diff --git a/tools/perf/Documentation/perf.data-file-format.txt
> > b/tools/perf/Documentation/perf.data-file-format.txt
> > index dfb218feaad9..6ea199f28330 100644
> > --- a/tools/perf/Documentation/perf.data-file-format.txt
> > +++ b/tools/perf/Documentation/perf.data-file-format.txt
> > @@ -43,13 +43,10 @@ struct perf_file_section {
> >
> > Flags section:
> >
> > -The header is followed by different optional headers, described by the
> > bits set
> > -in flags. Only headers for which the bit is set are included. Each header
> > -consists of a perf_file_section located after the initial header.
> > -The respective perf_file_section points to the data of the additional
> > -header and defines its size.
> > -
> > -Some headers consist of strings, which are defined like this:
> > +The Flags section is placed directly after the data section and consists
> > of a
> > +variable amount of information described by the flags-bitset in the
> > perf_header.
> > +A lot of the headers in the Flags section are simple strings and are
> > represented
> > +like this:
>
> some how I find this more confusing.. please describe
> what's actualy wrong with the current wording
The difference is that the current wording states "The header is
followed by different optional headers" but the actual placement
of those headers of the flags section is after the data section
(see perf_file_header__read at util/header.c:3108 in v4.20). But
I admit that I shouldn't have removed the perf_file_section's...
> >
> > struct perf_header_string {
> > uint32_t len;
> > @@ -82,7 +79,7 @@ assigned by the linker to an executable.
> > struct build_id_event {
> > struct perf_event_header header;
> > pid_tpid;
> > - uint8_t build_id[24];
> > + uint8_t build_id[PERF_ALIGN(24, sizeof(u64))];
>
> isn't that always 24? I guess u meant:
>
> build_id[PERF_ALIGN(20, sizeof(u64))];
You are right that this should always be 24. I just went over the diff
of my codebase to generate the perf.data file and tried to extract the
relevant format changes. I think I just stumbled across the definition
of build_id_event in util/event.h:229 and added that (irrelevant) change
which finally - also useless - worked as soon as I adjusted the flags
section.
The other way round this is the actual definition in the code base so
I'm not sure whether it should be the same. But as it is useless perhaps
change the definition in the source?
>
>
> > char filename[header.size - offsetof(struct
> > build_id_event, filename)];
> > };
> >
> > @@ -131,7 +128,7 @@ An uint64_t with the total memory in bytes.
> >
> > HEADER_CMDLINE = 11,
> >
> > -A perf_header_string with the perf command line used to collect the data.
> > +A perf_header_string_list with the perf arg-vector used to collect the
> > data.
>
> nice catch
>
> thanks,
> jirka
>
> >
> > HEADER_EVENT_DESC = 12,
> >
> > --
> > 2.17.1
> >
[PATCH] perf: update perf.data file format documentation
I found that the documentation of the flags section is some how
different from the actual format used and expected by the perf
tools. In this patch the according section of the file format
documentation is updated to conform to the expectations of the
perf tool suite.
Signed-off-by: Jonas Rabenstein
---
.../perf/Documentation/perf.data-file-format.txt | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/tools/perf/Documentation/perf.data-file-format.txt
b/tools/perf/Documentation/perf.data-file-format.txt
index dfb218feaad9..6ea199f28330 100644
--- a/tools/perf/Documentation/perf.data-file-format.txt
+++ b/tools/perf/Documentation/perf.data-file-format.txt
@@ -43,13 +43,10 @@ struct perf_file_section {
Flags section:
-The header is followed by different optional headers, described by the bits set
-in flags. Only headers for which the bit is set are included. Each header
-consists of a perf_file_section located after the initial header.
-The respective perf_file_section points to the data of the additional
-header and defines its size.
-
-Some headers consist of strings, which are defined like this:
+The Flags section is placed directly after the data section and consists of a
+variable amount of information described by the flags-bitset in the
perf_header.
+A lot of the headers in the Flags section are simple strings and are
represented
+like this:
struct perf_header_string {
uint32_t len;
@@ -82,7 +79,7 @@ assigned by the linker to an executable.
struct build_id_event {
struct perf_event_header header;
pid_tpid;
- uint8_t build_id[24];
+ uint8_t build_id[PERF_ALIGN(24, sizeof(u64))];
char filename[header.size - offsetof(struct
build_id_event, filename)];
};
@@ -131,7 +128,7 @@ An uint64_t with the total memory in bytes.
HEADER_CMDLINE = 11,
-A perf_header_string with the perf command line used to collect the data.
+A perf_header_string_list with the perf arg-vector used to collect the data.
HEADER_EVENT_DESC = 12,
--
2.17.1
[PATCH 13/16] block: sed-opal: check size of shadow mbr
Check whether the shadow mbr does fit in the provided space on the
target. Also a proper firmware should handle this case and return an
error we may prevent problems or even damage with crappy firmwares.
Signed-off-by: Jonas Rabenstein
---
block/opal_proto.h | 16
block/sed-opal.c | 39 +++
2 files changed, 55 insertions(+)
diff --git a/block/opal_proto.h b/block/opal_proto.h
index b6e352cfe982..5e8df3245eb0 100644
--- a/block/opal_proto.h
+++ b/block/opal_proto.h
@@ -106,6 +106,7 @@ enum opal_uid {
OPAL_ENTERPRISE_BANDMASTER0_UID,
OPAL_ENTERPRISE_ERASEMASTER_UID,
/* tables */
+ OPAL_TABLE_TABLE,
OPAL_LOCKINGRANGE_GLOBAL,
OPAL_LOCKINGRANGE_ACE_RDLOCKED,
OPAL_LOCKINGRANGE_ACE_WRLOCKED,
@@ -160,6 +161,21 @@ enum opal_token {
OPAL_STARTCOLUMN = 0x03,
OPAL_ENDCOLUMN = 0x04,
OPAL_VALUES = 0x01,
+ /* table table */
+ OPAL_TABLE_UID = 0x00,
+ OPAL_TABLE_NAME = 0x01,
+ OPAL_TABLE_COMMON = 0x02,
+ OPAL_TABLE_TEMPLATE = 0x03,
+ OPAL_TABLE_KIND = 0x04,
+ OPAL_TABLE_COLUMN = 0x05,
+ OPAL_TABLE_COLUMNS = 0x06,
+ OPAL_TABLE_ROWS = 0x07,
+ OPAL_TABLE_ROWS_FREE = 0x08,
+ OPAL_TABLE_ROW_BYTES = 0x09,
+ OPAL_TABLE_LASTID = 0x0A,
+ OPAL_TABLE_MIN = 0x0B,
+ OPAL_TABLE_MAX = 0x0C,
+
/* authority table */
OPAL_PIN = 0x03,
/* locking tokens */
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 034cb940b34a..ad52091531f2 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -139,6 +139,8 @@ static const u8 opaluid[][OPAL_UID_LENGTH] = {
/* tables */
+ [OPAL_TABLE_TABLE]
+ { 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01 },
[OPAL_LOCKINGRANGE_GLOBAL] =
{ 0x00, 0x00, 0x08, 0x02, 0x00, 0x00, 0x00, 0x01 },
[OPAL_LOCKINGRANGE_ACE_RDLOCKED] =
@@ -1120,6 +1122,29 @@ static int generic_get_column(struct opal_dev *dev,
const u8 *table,
return finalize_and_send(dev, parse_and_check_status);
}
+/*
+ * see TCG SAS 5.3.2.3 for a description of the available columns
+ *
+ * the result is provided in dev->resp->tok[4]
+ */
+static int generic_get_table_info(struct opal_dev *dev, enum opal_uid table,
+ u64 column)
+{
+ u8 uid[OPAL_UID_LENGTH];
+ const unsigned int half = OPAL_UID_LENGTH/2;
+
+ /* sed-opal UIDs can be split in two halves:
+* first: actual table index
+* second: relative index in the table
+* so we have to get the first half of the OPAL_TABLE_TABLE and use the
+* first part of the target table as relative index into that table
+*/
+ memcpy(uid, opaluid[OPAL_TABLE_TABLE], half);
+ memcpy(uid+half, opaluid[table], half);
+
+ return generic_get_column(dev, uid, column);
+}
+
static int gen_key(struct opal_dev *dev, void *data)
{
u8 uid[OPAL_UID_LENGTH];
@@ -1535,6 +1560,20 @@ static int write_shadow_mbr(struct opal_dev *dev, void
*data)
u64 len;
int err = 0;
+ /* do we fit in the available shadow mbr space? */
+ err = generic_get_table_info(dev, OPAL_MBR, OPAL_TABLE_ROWS);
+ if (err) {
+ pr_debug("MBR: could not get shadow size\n");
+ return err;
+ }
+
+ len = response_get_u64(&dev->parsed, 4);
+ if (shadow->offset + shadow->size > len) {
+ pr_debug("MBR: does not fit in shadow (%llu vs. %llu)\n",
+shadow->offset + shadow->size, len);
+ return -ENOSPC;
+ }
+
/* do the actual transmission(s) */
src = (u8 *) shadow->data;
while (off < shadow->size) {
--
2.20.1
[PATCH 03/16] block: sed-opal: unify space check in add_token_*
All add_token_* functions have a common set of conditions that have to
be checked. Use a common function for those checks in order to avoid
different behaviour as well as code duplication.
Co-authored-by: David Kozub
Signed-off-by: Jonas Rabenstein
Signed-off-by: David Kozub
---
block/sed-opal.c | 30 +-
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 5c123a5b4ab1..980705681806 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -510,15 +510,29 @@ static int opal_discovery0(struct opal_dev *dev, void
*data)
return opal_discovery0_end(dev);
}
-static void add_token_u8(int *err, struct opal_dev *cmd, u8 tok)
+static size_t remaining_size(struct opal_dev *cmd)
+{
+ return IO_BUFFER_LENGTH - cmd->pos;
+}
+
+static bool can_add(int *err, struct opal_dev *cmd, size_t len)
{
if (*err)
- return;
- if (cmd->pos >= IO_BUFFER_LENGTH - 1) {
- pr_debug("Error adding u8: end of buffer.\n");
+ return false;
+
+ if (remaining_size(cmd) < len) {
+ pr_debug("Error adding %zu bytes: end of buffer.\n", len);
*err = -ERANGE;
- return;
+ return false;
}
+
+ return true;
+}
+
+static void add_token_u8(int *err, struct opal_dev *cmd, u8 tok)
+{
+ if (!can_add(err, cmd, 1))
+ return;
cmd->cmd[cmd->pos++] = tok;
}
@@ -563,9 +577,8 @@ static void add_token_u64(int *err, struct opal_dev *cmd,
u64 number)
msb = fls64(number);
len = DIV_ROUND_UP(msb, 8);
- if (cmd->pos >= IO_BUFFER_LENGTH - len - 1) {
+ if (!can_add(err, cmd, len + 1)) {
pr_debug("Error adding u64: end of buffer.\n");
- *err = -ERANGE;
return;
}
add_short_atom_header(cmd, false, false, len);
@@ -587,9 +600,8 @@ static void add_token_bytestring(int *err, struct opal_dev
*cmd,
is_short_atom = false;
}
- if (len >= IO_BUFFER_LENGTH - cmd->pos - header_len) {
+ if (!can_add(err, cmd, header_len + len)) {
pr_debug("Error adding bytestring: end of buffer.\n");
- *err = -ERANGE;
return;
}
--
2.20.1
[PATCH 12/16] block: sed-opal: unify retrieval of table columns
Instead of having multiple places defining the same argument list to get
a specific column of a sed-opal table, provide a generic version and
call it from those functions.
Signed-off-by: Jonas Rabenstein
---
block/opal_proto.h | 2 +
block/sed-opal.c | 132 +
2 files changed, 50 insertions(+), 84 deletions(-)
diff --git a/block/opal_proto.h b/block/opal_proto.h
index e20be8258854..b6e352cfe982 100644
--- a/block/opal_proto.h
+++ b/block/opal_proto.h
@@ -170,6 +170,8 @@ enum opal_token {
OPAL_READLOCKED = 0x07,
OPAL_WRITELOCKED = 0x08,
OPAL_ACTIVEKEY = 0x0A,
+ /* lockingsp table */
+ OPAL_LIFECYCLE = 0x06,
/* locking info table */
OPAL_MAXRANGES = 0x04,
/* mbr control */
diff --git a/block/sed-opal.c b/block/sed-opal.c
index ee24e4e7ad6e..034cb940b34a 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -1089,6 +1089,37 @@ static int finalize_and_send(struct opal_dev *dev,
cont_fn cont)
return opal_send_recv(dev, cont);
}
+/*
+ * request @column from table @table on device @dev. On success, the column
+ * data will be available in dev->resp->tok[4]
+ */
+static int generic_get_column(struct opal_dev *dev, const u8 *table,
+ u64 column)
+{
+ int err;
+
+ err = cmd_start(dev, table, opalmethod[OPAL_GET]);
+
+ add_token_u8(&err, dev, OPAL_STARTLIST);
+
+ add_token_u8(&err, dev, OPAL_STARTNAME);
+ add_token_u8(&err, dev, OPAL_STARTCOLUMN);
+ add_token_u64(&err, dev, column);
+ add_token_u8(&err, dev, OPAL_ENDNAME);
+
+ add_token_u8(&err, dev, OPAL_STARTNAME);
+ add_token_u8(&err, dev, OPAL_ENDCOLUMN);
+ add_token_u64(&err, dev, column);
+ add_token_u8(&err, dev, OPAL_ENDNAME);
+
+ add_token_u8(&err, dev, OPAL_ENDLIST);
+
+ if (err)
+ return err;
+
+ return finalize_and_send(dev, parse_and_check_status);
+}
+
static int gen_key(struct opal_dev *dev, void *data)
{
u8 uid[OPAL_UID_LENGTH];
@@ -1143,23 +1174,11 @@ static int get_active_key(struct opal_dev *dev, void
*data)
if (err)
return err;
- err = cmd_start(dev, uid, opalmethod[OPAL_GET]);
- add_token_u8(&err, dev, OPAL_STARTLIST);
- add_token_u8(&err, dev, OPAL_STARTNAME);
- add_token_u8(&err, dev, 3); /* startCloumn */
- add_token_u8(&err, dev, 10); /* ActiveKey */
- add_token_u8(&err, dev, OPAL_ENDNAME);
- add_token_u8(&err, dev, OPAL_STARTNAME);
- add_token_u8(&err, dev, 4); /* endColumn */
- add_token_u8(&err, dev, 10); /* ActiveKey */
- add_token_u8(&err, dev, OPAL_ENDNAME);
- add_token_u8(&err, dev, OPAL_ENDLIST);
- if (err) {
- pr_debug("Error building get active key command\n");
+ err = generic_get_column(dev, uid, OPAL_ACTIVEKEY);
+ if (err)
return err;
- }
- return finalize_and_send(dev, get_active_key_cont);
+ return get_active_key_cont(dev);
}
static int generic_lr_enable_disable(struct opal_dev *dev,
@@ -1819,17 +1838,19 @@ static int activate_lsp(struct opal_dev *dev, void
*data)
return finalize_and_send(dev, parse_and_check_status);
}
-static int get_lsp_lifecycle_cont(struct opal_dev *dev)
+/* Determine if we're in the Manufactured Inactive or Active state */
+static int get_lsp_lifecycle(struct opal_dev *dev, void *data)
{
u8 lc_status;
- int error = 0;
+ int err;
- error = parse_and_check_status(dev);
- if (error)
- return error;
+ err = generic_get_column(dev, opaluid[OPAL_LOCKINGSP_UID],
+OPAL_LIFECYCLE);
+ if (err)
+ return err;
lc_status = response_get_u64(&dev->parsed, 4);
- /* 0x08 is Manufacured Inactive */
+ /* 0x08 is Manufactured Inactive */
/* 0x09 is Manufactured */
if (lc_status != OPAL_MANUFACTURED_INACTIVE) {
pr_debug("Couldn't determine the status of the Lifecycle
state\n");
@@ -1839,49 +1860,19 @@ static int get_lsp_lifecycle_cont(struct opal_dev *dev)
return 0;
}
-/* Determine if we're in the Manufactured Inactive or Active state */
-static int get_lsp_lifecycle(struct opal_dev *dev, void *data)
-{
- int err;
-
- err = cmd_start(dev, opaluid[OPAL_LOCKINGSP_UID],
- opalmethod[OPAL_GET]);
-
- add_token_u8(&err, dev, OPAL_STARTLIST);
-
- add_token_u8(&err, dev, OPAL_STARTNAME);
- add_token_u8(&err, dev, 3); /* Start Column */
- add_token_u8(&err, dev, 6); /* Lifecycle Column */
- add_token_u8(&err, dev, OPAL_ENDNAME);
-
- add_token_u8(&err, dev, OPAL_STARTNAME);
- add_token_u8(&err, dev, 4);
[PATCH 08/16] block: sed-opal: print failed function address
Add function address (and if available its symbol) to the message if a
step function fails.
Signed-off-by: Jonas Rabenstein
---
block/sed-opal.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 1332547e5a99..4225f23b2165 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -394,8 +394,8 @@ static int next(struct opal_dev *dev)
error = step->fn(dev, step->data);
if (error) {
- pr_debug("Error on step function: %d with error %d:
%s\n",
-state, error,
+ pr_debug("Step %d (%pS) failed wit error %d: %s\n",
+state, step->fn, error,
opal_error_to_human(error));
/* For each OPAL command we do a discovery0 then we
--
2.20.1
[PATCH 10/16] block: sed-opal: add ioctl for done-mark of shadow mbr
Enable users to mark the shadow mbr as done without completely
deactivating the shadow mbr feature. This may be useful on reboots,
when the power to the disk is not disconnected in between and the shadow
mbr stores the required boot files. Of course, this saves also the
(few) commands required to enable the feature if it is already enabled
and one only wants to mark the shadow mbr as done.
Signed-off-by: Jonas Rabenstein
---
block/sed-opal.c | 33 +++--
include/linux/sed-opal.h | 1 +
include/uapi/linux/sed-opal.h | 1 +
3 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index db966bbc5a05..f85d9e53aab8 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -1996,13 +1996,39 @@ static int opal_erase_locking_range(struct opal_dev
*dev,
static int opal_enable_disable_shadow_mbr(struct opal_dev *dev,
struct opal_mbr_data *opal_mbr)
{
+ u8 token = opal_mbr->enable_disable == OPAL_MBR_ENABLE
+ ? OPAL_TRUE : OPAL_FALSE;
const struct opal_step mbr_steps[] = {
{ opal_discovery0, },
{ start_admin1LSP_opal_session, &opal_mbr->key },
- { set_mbr_done, &opal_mbr->enable_disable },
+ { set_mbr_done, &token },
{ end_opal_session, },
{ start_admin1LSP_opal_session, &opal_mbr->key },
- { set_mbr_enable_disable, &opal_mbr->enable_disable },
+ { set_mbr_enable_disable, &token },
+ { end_opal_session, },
+ { NULL, }
+ };
+ int ret;
+
+ if (opal_mbr->enable_disable != OPAL_MBR_ENABLE &&
+ opal_mbr->enable_disable != OPAL_MBR_DISABLE)
+ return -EINVAL;
+
+ mutex_lock(&dev->dev_lock);
+ setup_opal_dev(dev, mbr_steps);
+ ret = next(dev);
+ mutex_unlock(&dev->dev_lock);
+ return ret;
+}
+
+static int opal_mbr_status(struct opal_dev *dev, struct opal_mbr_data
*opal_mbr)
+{
+ u8 token = opal_mbr->enable_disable == OPAL_MBR_ENABLE
+ ? OPAL_TRUE : OPAL_FALSE;
+ const struct opal_step mbr_steps[] = {
+ { opal_discovery0, },
+ { start_admin1LSP_opal_session, &opal_mbr->key },
+ { set_mbr_done, &token },
{ end_opal_session, },
{ NULL, }
};
@@ -2349,6 +2375,9 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd,
void __user *arg)
case IOC_OPAL_ENABLE_DISABLE_MBR:
ret = opal_enable_disable_shadow_mbr(dev, p);
break;
+ case IOC_OPAL_MBR_STATUS:
+ ret = opal_mbr_status(dev, p);
+ break;
case IOC_OPAL_ERASE_LR:
ret = opal_erase_locking_range(dev, p);
break;
diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
index 04b124fca51e..b38dc602cae3 100644
--- a/include/linux/sed-opal.h
+++ b/include/linux/sed-opal.h
@@ -47,6 +47,7 @@ static inline bool is_sed_ioctl(unsigned int cmd)
case IOC_OPAL_ENABLE_DISABLE_MBR:
case IOC_OPAL_ERASE_LR:
case IOC_OPAL_SECURE_ERASE_LR:
+ case IOC_OPAL_MBR_STATUS:
return true;
}
return false;
diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
index 627624d35030..0cb9890cdc04 100644
--- a/include/uapi/linux/sed-opal.h
+++ b/include/uapi/linux/sed-opal.h
@@ -116,5 +116,6 @@ struct opal_mbr_data {
#define IOC_OPAL_ENABLE_DISABLE_MBR _IOW('p', 229, struct opal_mbr_data)
#define IOC_OPAL_ERASE_LR _IOW('p', 230, struct opal_session_info)
#define IOC_OPAL_SECURE_ERASE_LR_IOW('p', 231, struct opal_session_info)
+#define IOC_OPAL_MBR_STATUS _IOW('p', 232, struct opal_mbr_data)
#endif /* _UAPI_SED_OPAL_H */
--
2.20.1
[PATCH 02/16] block: sed-opal: use correct macro for method length
Also the values of OPAL_UID_LENGTH and OPAL_METHOD_LENGTH are the same,
it is weird to use OPAL_UID_LENGTH for the definition of the methods.
Signed-off-by: Jonas Rabenstein
---
block/sed-opal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index c882a193e162..5c123a5b4ab1 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -181,7 +181,7 @@ static const u8 opaluid[][OPAL_UID_LENGTH] = {
* Derived from: TCG_Storage_Architecture_Core_Spec_v2.01_r1.00
* Section: 6.3 Assigned UIDs
*/
-static const u8 opalmethod[][OPAL_UID_LENGTH] = {
+static const u8 opalmethod[][OPAL_METHOD_LENGTH] = {
[OPAL_PROPERTIES] =
{ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x01 },
[OPAL_STARTSESSION] =
--
2.20.1
[PATCH 09/16] block: sed-opal: split generation of bytestring header and content
Split the header generation from the (normal) memcpy part if a
bytestring is copied into the command buffer. This allows in-place
generation of the bytestring content. For example, copy_from_user may be
used without an intermediate buffer.
Signed-off-by: Jonas Rabenstein
---
block/sed-opal.c | 23 +++
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 4225f23b2165..db966bbc5a05 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -586,14 +586,11 @@ static void add_token_u64(int *err, struct opal_dev *cmd,
u64 number)
add_token_u8(err, cmd, number >> (len * 8));
}
-static void add_token_bytestring(int *err, struct opal_dev *cmd,
-const u8 *bytestring, size_t len)
+static u8 *add_bytestring_header(int *err, struct opal_dev *cmd, size_t len)
{
size_t header_len = 1;
bool is_short_atom = true;
-
- if (*err)
- return;
+ char *start;
if (len & ~SHORT_ATOM_LEN_MASK) {
header_len = 2;
@@ -602,17 +599,27 @@ static void add_token_bytestring(int *err, struct
opal_dev *cmd,
if (!can_add(err, cmd, header_len + len)) {
pr_debug("Error adding bytestring: end of buffer.\n");
- return;
+ return NULL;
}
if (is_short_atom)
add_short_atom_header(cmd, true, false, len);
else
add_medium_atom_header(cmd, true, false, len);
-
- memcpy(&cmd->cmd[cmd->pos], bytestring, len);
+ start = &cmd->cmd[cmd->pos];
cmd->pos += len;
+ return start;
+}
+static void add_token_bytestring(int *err, struct opal_dev *cmd,
+const u8 *bytestring, size_t len)
+{
+ u8 *start;
+
+ start = add_bytestring_header(err, cmd, len);
+ if (!start)
+ return;
+ memcpy(start, bytestring, len);
}
static int build_locking_range(u8 *buffer, size_t length, u8 lr)
--
2.20.1
[PATCH 11/16] block: sed-opal: ioctl for writing to shadow mbr
Allow modification of the shadow mbr. If the shadow mbr is not marked as
done, this data will be presented read only as the device content. Only
after marking the shadow mbr as done and unlocking a locking range the
actual content is accessible.
Co-authored-by: David Kozub
Signed-off-by: Jonas Rabenstein
Signed-off-by: David Kozub
---
block/sed-opal.c | 88 ++-
include/linux/sed-opal.h | 1 +
include/uapi/linux/sed-opal.h | 8
3 files changed, 96 insertions(+), 1 deletion(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index f85d9e53aab8..ee24e4e7ad6e 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -34,6 +34,9 @@
#define IO_BUFFER_LENGTH 2048
#define MAX_TOKS 64
+/* Number of bytes needed by cmd_finalize. */
+#define CMD_FINALIZE_BYTES_NEEDED 7
+
struct opal_step {
int (*fn)(struct opal_dev *dev, void *data);
void *data;
@@ -668,7 +671,11 @@ static int cmd_finalize(struct opal_dev *cmd, u32 hsn, u32
tsn)
struct opal_header *hdr;
int err = 0;
- /* close the parameter list opened from cmd_start */
+ /*
+* Close the parameter list opened from cmd_start.
+* The number of bytes added must be equal to
+* CMD_FINALIZE_BYTES_NEEDED.
+*/
add_token_u8(&err, cmd, OPAL_ENDLIST);
add_token_u8(&err, cmd, OPAL_ENDOFDATA);
@@ -1500,6 +1507,57 @@ static int set_mbr_enable_disable(struct opal_dev *dev,
void *data)
return finalize_and_send(dev, parse_and_check_status);
}
+static int write_shadow_mbr(struct opal_dev *dev, void *data)
+{
+ struct opal_shadow_mbr *shadow = data;
+ const u8 __user *src;
+ u8 *dst;
+ size_t off = 0;
+ u64 len;
+ int err = 0;
+
+ /* do the actual transmission(s) */
+ src = (u8 *) shadow->data;
+ while (off < shadow->size) {
+ err = cmd_start(dev, opaluid[OPAL_MBR], opalmethod[OPAL_SET]);
+ add_token_u8(&err, dev, OPAL_STARTNAME);
+ add_token_u8(&err, dev, OPAL_WHERE);
+ add_token_u64(&err, dev, shadow->offset + off);
+ add_token_u8(&err, dev, OPAL_ENDNAME);
+
+ add_token_u8(&err, dev, OPAL_STARTNAME);
+ add_token_u8(&err, dev, OPAL_VALUES);
+
+ /*
+* The bytestring header is either 1 or 2 bytes, so assume 2.
+* There also needs to be enough space to accommodate the
+* trailing OPAL_ENDNAME (1 byte) and tokens added by
+* cmd_finalize.
+*/
+ len = min(remaining_size(dev) - (2+1+CMD_FINALIZE_BYTES_NEEDED),
+ (size_t)(shadow->size - off));
+ pr_debug("MBR: write bytes %zu+%llu/%llu\n",
+off, len, shadow->size);
+
+ dst = add_bytestring_header(&err, dev, len);
+ if (!dst)
+ break;
+ if (copy_from_user(dst, src + off, len))
+ err = -EFAULT;
+
+ add_token_u8(&err, dev, OPAL_ENDNAME);
+ if (err)
+ break;
+
+ err = finalize_and_send(dev, parse_and_check_status);
+ if (err)
+ break;
+
+ off += len;
+ }
+ return err;
+}
+
static int generic_pw_cmd(u8 *key, size_t key_len, u8 *cpin_uid,
struct opal_dev *dev)
{
@@ -2045,6 +2103,31 @@ static int opal_mbr_status(struct opal_dev *dev, struct
opal_mbr_data *opal_mbr)
return ret;
}
+static int opal_write_shadow_mbr(struct opal_dev *dev,
+struct opal_shadow_mbr *info)
+{
+ const struct opal_step mbr_steps[] = {
+ { opal_discovery0, },
+ { start_admin1LSP_opal_session, &info->key },
+ { write_shadow_mbr, info },
+ { end_opal_session, },
+ { NULL, }
+ };
+ int ret;
+
+ if (info->size == 0)
+ return 0;
+
+ if (!access_ok(info->data, info->size))
+ return -EINVAL;
+
+ mutex_lock(&dev->dev_lock);
+ setup_opal_dev(dev, mbr_steps);
+ ret = next(dev);
+ mutex_unlock(&dev->dev_lock);
+ return ret;
+}
+
static int opal_save(struct opal_dev *dev, struct opal_lock_unlock *lk_unlk)
{
struct opal_suspend_data *suspend;
@@ -2378,6 +2461,9 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd,
void __user *arg)
case IOC_OPAL_MBR_STATUS:
ret = opal_mbr_status(dev, p);
break;
+ case IOC_OPAL_WRITE_SHADOW_MBR:
+ ret = opal_write_shadow_mbr(dev, p);
+ break;
case IOC_OPAL_ERASE_LR:
ret = opal_erase_locking_range(dev, p);
break;
Re: [PATCH v2 08/11] block: sed-opal: ioctl for writing to shadow mbr
Hi, On Wed, Mar 21, 2018 at 02:43:21AM +0100, Jonas Rabenstein wrote: > On Tue, Mar 20, 2018 at 04:09:08PM -0600, Scott Bauer wrote: > > On Tue, Mar 20, 2018 at 10:36:04AM +0100, Jonas Rabenstein wrote: > > > On Mon, Mar 19, 2018 at 08:52:24PM +0100, Christoph Hellwig wrote: > > > > On Mon, Mar 19, 2018 at 07:36:50PM +0100, Jonas Rabenstein wrote: > > > > I hate doing this as an ioctls. Can we make this a sysfs binary file > > > > so that people can use dd or cat to write the shadow mbr? > > > I already thought about providing a sysfs interface for all that instead > > > of using ioctls. But as I am pretty new to kernel programming I do not > > > have all the required insight. Especially, as writing the mbr requires > > > the sed-opal password I am unsure how a clean sysfs interface to provide > > > the password together with a simple dd would look like. Just wanted to ask, how to proceed with those patches/what I should do. Using sysfs instead of an ioctl is probably easier to use from userspace _if_ there is a good way to provide the password - which I do not know of :( If nobody else could think of a solution, shall writes to the shadow mbr remain unsupported? I'ld really appreciate feedback and possible solutions, Jonas
Re: [PATCH v2 08/11] block: sed-opal: ioctl for writing to shadow mbr
On Tue, Mar 20, 2018 at 04:09:08PM -0600, Scott Bauer wrote: > On Tue, Mar 20, 2018 at 10:36:04AM +0100, Jonas Rabenstein wrote: > > On Mon, Mar 19, 2018 at 08:52:24PM +0100, Christoph Hellwig wrote: > > > On Mon, Mar 19, 2018 at 07:36:50PM +0100, Jonas Rabenstein wrote: > > > > Allow modification of the shadow mbr. If the shadow mbr is not marked as > > > > done, this data will be presented read only as the device content. Only > > > > after marking the shadow mbr as done and unlocking a locking range the > > > > actual content is accessible. > > > > > > I hate doing this as an ioctls. Can we make this a sysfs binary file > > > so that people can use dd or cat to write the shadow mbr? > > I already thought about providing a sysfs interface for all that instead > > of using ioctls. But as I am pretty new to kernel programming I do not > > have all the required insight. Especially, as writing the mbr requires > > the sed-opal password I am unsure how a clean sysfs interface to provide > > the password together with a simple dd would look like. > > Moreover I already have a patch that changes the 'void *data' argument > > to setup_opal_dev to a kobject pointer. As far as I know, this is the > > first step to get into the sysfs hierarchy. But as I do not have access > > to an NVMe drive and have no idea about its implementation, this change > > works only for the scsi side. > > Post what you have as an RFC (review for comment) and I will test for the NVMe > side, and or start a port for NVMe. It doesn't have to be perfect since you're > sending it out as RFC. It's just a base for us to test/look at to see if we > still like the sysfs way. Seems, like I failed to make my point in the previous message. I do not have more than adding a directory 'sed_opal' in sysfs for each sed-opal enabled disk. But that directory is completely empty. My further plans are, to fill up that directory with some public info like the one that gets printed by a call to TCG's 'sedutil-cli --query' command. The interesting part - where I am clueless how to achieve it - would be to have a binary sysfs attribute (like mentioned by Christoph) for the features (like shadow mbr) requiring some kind of authentication. Until there is any (good) idea, I do not think it is time for an RFC? -- Jonas
Re: [PATCH v2 11/11] block: sed-opal: check size of shadow mbr
On Mon, Mar 19, 2018 at 09:01:51PM +0100, Christoph Hellwig wrote: > On Mon, Mar 19, 2018 at 07:36:53PM +0100, Jonas Rabenstein wrote: > > Check whether the shadow mbr does fit in the provided space on the > > target. Also a proper firmware should handle this case and return an > > error we may prevent problems or even damage with crappy firmwares. > > > > Signed-off-by: Jonas Rabenstein > > > > diff --git a/block/sed-opal.c b/block/sed-opal.c > > index 51f8034edbf7..9c73bd24c55f 100644 > > --- a/block/sed-opal.c > > +++ b/block/sed-opal.c > > @@ -1545,6 +1545,20 @@ static int write_shadow_mbr(struct opal_dev *dev, > > void *data) > > u64 len; > > int err = 0; > > > > + /* do we fit in the available shadow mbr space? */ > > + err = generic_get_table_info(dev, OPAL_MBR, OPAL_TABLE_ROWS); > > And here it gets used. So this should be merged with the previous patch. Thought, as the previous one provides a generic interface which is only used here for this specific use case, separate patches would be better. But I will merge them in a v3 with all the other comments on the other patches.
Re: [PATCH v2 08/11] block: sed-opal: ioctl for writing to shadow mbr
On Mon, Mar 19, 2018 at 08:52:24PM +0100, Christoph Hellwig wrote: > On Mon, Mar 19, 2018 at 07:36:50PM +0100, Jonas Rabenstein wrote: > > Allow modification of the shadow mbr. If the shadow mbr is not marked as > > done, this data will be presented read only as the device content. Only > > after marking the shadow mbr as done and unlocking a locking range the > > actual content is accessible. > > I hate doing this as an ioctls. Can we make this a sysfs binary file > so that people can use dd or cat to write the shadow mbr? I already thought about providing a sysfs interface for all that instead of using ioctls. But as I am pretty new to kernel programming I do not have all the required insight. Especially, as writing the mbr requires the sed-opal password I am unsure how a clean sysfs interface to provide the password together with a simple dd would look like. Moreover I already have a patch that changes the 'void *data' argument to setup_opal_dev to a kobject pointer. As far as I know, this is the first step to get into the sysfs hierarchy. But as I do not have access to an NVMe drive and have no idea about its implementation, this change works only for the scsi side. In other words, if someone could hint me in the right direction, I would be glad to (re)implement the ioctl interface for sysfs. Moreover, this would allow to export some additional information like the current state of the device (is it looke, is sed-opal enabled, whats the current state of mbr, etc.). - Jonas
[PATCH v2 02/11] block: sed-opal: unify space check in add_token_*
All add_token_* functions have a common set of conditions that have to
be checked. Use a common function for those checks in order to avoid
different behaviour as well as code duplication.
Signed-off-by: Jonas Rabenstein
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 5aa41744b8f1..771b4cfff95c 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -510,15 +510,24 @@ static int opal_discovery0(struct opal_dev *dev, void
*data)
return opal_discovery0_end(dev);
}
-static void add_token_u8(int *err, struct opal_dev *cmd, u8 tok)
+static bool can_add(int *err, struct opal_dev *cmd, size_t len)
{
if (*err)
- return;
- if (cmd->pos >= IO_BUFFER_LENGTH - 1) {
- pr_debug("Error adding u8: end of buffer.\n");
+ return false;
+
+ if (len > IO_BUFFER_LENGTH || cmd->pos >= IO_BUFFER_LENGTH - len) {
+ pr_debug("Error adding %zu bytes: end of buffer.\n", len);
*err = -ERANGE;
- return;
+ return false;
}
+
+ return true;
+}
+
+static void add_token_u8(int *err, struct opal_dev *cmd, u8 tok)
+{
+ if (!can_add(err, cmd, 1))
+ return;
cmd->cmd[cmd->pos++] = tok;
}
@@ -563,9 +572,8 @@ static void add_token_u64(int *err, struct opal_dev *cmd,
u64 number)
msb = fls64(number);
len = DIV_ROUND_UP(msb, 8);
- if (cmd->pos >= IO_BUFFER_LENGTH - len - 1) {
+ if (!can_add(err, cmd, len + 1)) {
pr_debug("Error adding u64: end of buffer.\n");
- *err = -ERANGE;
return;
}
add_short_atom_header(cmd, false, false, len);
@@ -587,9 +595,8 @@ static void add_token_bytestring(int *err, struct opal_dev
*cmd,
is_short_atom = false;
}
- if (len >= IO_BUFFER_LENGTH - cmd->pos - header_len) {
+ if (!can_add(err, cmd, header_len + len)) {
pr_debug("Error adding bytestring: end of buffer.\n");
- *err = -ERANGE;
return;
}
--
2.16.1
[PATCH v2 03/11] block: sed-opal: unify cmd start and finalize
Every step starts with resetting the cmd buffer as well as the comid and
constructs the appropriate OPAL_CALL command. Consequently, those
actions may be combined into one generic function. On should take care,
that the opening and closing tokens for the argument list are already
emitted by those functions and thus must not be additionally added.
Signed-off-by: Jonas Rabenstein
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 771b4cfff95c..efe5d2a7f3dc 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -656,6 +656,9 @@ static int cmd_finalize(struct opal_dev *cmd, u32 hsn, u32
tsn)
struct opal_header *hdr;
int err = 0;
+ /* close the parameter list opened from start_opal_cmd */
+ add_token_u8(&err, cmd, OPAL_ENDLIST);
+
add_token_u8(&err, cmd, OPAL_ENDOFDATA);
add_token_u8(&err, cmd, OPAL_STARTLIST);
add_token_u8(&err, cmd, 0);
@@ -998,6 +1001,26 @@ static void clear_opal_cmd(struct opal_dev *dev)
memset(dev->cmd, 0, IO_BUFFER_LENGTH);
}
+static int start_opal_cmd(struct opal_dev *dev, const u8 *uid, const u8
*method)
+{
+ int err = 0;
+
+ clear_opal_cmd(dev);
+ set_comid(dev, dev->comid);
+
+ add_token_u8(&err, dev, OPAL_CALL);
+ add_token_bytestring(&err, dev, uid, OPAL_UID_LENGTH);
+ add_token_bytestring(&err, dev, method, OPAL_METHOD_LENGTH);
+
+ /* every method call is followed by its parameters enclosed within
+* OPAL_STARTLIST and OPAL_ENDLIST tokens. We automatically open the
+* parameter list here and close it later in cmd_finalize
+*/
+ add_token_u8(&err, dev, OPAL_STARTLIST);
+
+ return err;
+}
+
static int start_opal_session_cont(struct opal_dev *dev)
{
u32 hsn, tsn;
@@ -1060,21 +1083,13 @@ static int finalize_and_send(struct opal_dev *dev,
cont_fn cont)
static int gen_key(struct opal_dev *dev, void *data)
{
u8 uid[OPAL_UID_LENGTH];
- int err = 0;
-
- clear_opal_cmd(dev);
- set_comid(dev, dev->comid);
+ int err;
memcpy(uid, dev->prev_data, min(sizeof(uid), dev->prev_d_len));
kfree(dev->prev_data);
dev->prev_data = NULL;
- add_token_u8(&err, dev, OPAL_CALL);
- add_token_bytestring(&err, dev, uid, OPAL_UID_LENGTH);
- add_token_bytestring(&err, dev, opalmethod[OPAL_GENKEY],
-OPAL_UID_LENGTH);
- add_token_u8(&err, dev, OPAL_STARTLIST);
- add_token_u8(&err, dev, OPAL_ENDLIST);
+ err = start_opal_cmd(dev, uid, opalmethod[OPAL_GENKEY]);
if (err) {
pr_debug("Error building gen key command\n");
@@ -1112,21 +1127,14 @@ static int get_active_key_cont(struct opal_dev *dev)
static int get_active_key(struct opal_dev *dev, void *data)
{
u8 uid[OPAL_UID_LENGTH];
- int err = 0;
+ int err;
u8 *lr = data;
- clear_opal_cmd(dev);
- set_comid(dev, dev->comid);
-
err = build_locking_range(uid, sizeof(uid), *lr);
if (err)
return err;
- err = 0;
- add_token_u8(&err, dev, OPAL_CALL);
- add_token_bytestring(&err, dev, uid, OPAL_UID_LENGTH);
- add_token_bytestring(&err, dev, opalmethod[OPAL_GET], OPAL_UID_LENGTH);
- add_token_u8(&err, dev, OPAL_STARTLIST);
+ err = start_opal_cmd(dev, uid, opalmethod[OPAL_GET]);
add_token_u8(&err, dev, OPAL_STARTLIST);
add_token_u8(&err, dev, OPAL_STARTNAME);
add_token_u8(&err, dev, 3); /* startCloumn */
@@ -1137,7 +1145,6 @@ static int get_active_key(struct opal_dev *dev, void
*data)
add_token_u8(&err, dev, 10); /* ActiveKey */
add_token_u8(&err, dev, OPAL_ENDNAME);
add_token_u8(&err, dev, OPAL_ENDLIST);
- add_token_u8(&err, dev, OPAL_ENDLIST);
if (err) {
pr_debug("Error building get active key command\n");
return err;
@@ -1150,13 +1157,10 @@ static int generic_lr_enable_disable(struct opal_dev
*dev,
u8 *uid, bool rle, bool wle,
bool rl, bool wl)
{
- int err = 0;
+ int err;
- add_token_u8(&err, dev, OPAL_CALL);
- add_token_bytestring(&err, dev, uid, OPAL_UID_LENGTH);
- add_token_bytestring(&err, dev, opalmethod[OPAL_SET], OPAL_UID_LENGTH);
+ err = start_opal_cmd(dev, uid, opalmethod[OPAL_SET]);
- add_token_u8(&err, dev, OPAL_STARTLIST);
add_token_u8(&err, dev, OPAL_STARTNAME);
add_token_u8(&err, dev, OPAL_VALUES);
add_token_u8(&err, dev, OPAL_STARTLIST);
@@ -1183,7 +1187,6 @@ static int generic_lr_enable_disable(struct opal_dev *dev,
add_token_u8(&err, dev, OPAL_ENDLIST);
add_token_u8(&err, dev, OPAL_ENDNAME);
- add_tok
[PATCH v2 04/11] block: sed-opal: unify error handling of responses
Also response_get_token had already been in place, its functionality had
been duplicated within response_get_{u64,bytestring} with the same error
handling. Unify the handling by reusing response_get_token within the
other functions.
Signed-off-by: Jonas Rabenstein
diff --git a/block/sed-opal.c b/block/sed-opal.c
index efe5d2a7f3dc..30f6e46518a6 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -697,6 +697,11 @@ static const struct opal_resp_tok *response_get_token(
{
const struct opal_resp_tok *tok;
+ if (!resp) {
+ pr_debug("Response is NULL\n");
+ return ERR_PTR(-EINVAL);
+ }
+
if (n >= resp->num) {
pr_debug("Token number doesn't exist: %d, resp: %d\n",
n, resp->num);
@@ -879,27 +884,19 @@ static size_t response_get_string(const struct
parsed_resp *resp, int n,
const char **store)
{
u8 skip;
- const struct opal_resp_tok *token;
+ const struct opal_resp_tok *tok;
*store = NULL;
- if (!resp) {
- pr_debug("Response is NULL\n");
- return 0;
- }
-
- if (n > resp->num) {
- pr_debug("Response has %d tokens. Can't access %d\n",
-resp->num, n);
+ tok = response_get_token(resp, n);
+ if (IS_ERR(tok))
return 0;
- }
- token = &resp->toks[n];
- if (token->type != OPAL_DTA_TOKENID_BYTESTRING) {
+ if (tok->type != OPAL_DTA_TOKENID_BYTESTRING) {
pr_debug("Token is not a byte string!\n");
return 0;
}
- switch (token->width) {
+ switch (tok->width) {
case OPAL_WIDTH_TINY:
case OPAL_WIDTH_SHORT:
skip = 1;
@@ -915,37 +912,29 @@ static size_t response_get_string(const struct
parsed_resp *resp, int n,
return 0;
}
- *store = token->pos + skip;
- return token->len - skip;
+ *store = tok->pos + skip;
+ return tok->len - skip;
}
static u64 response_get_u64(const struct parsed_resp *resp, int n)
{
- if (!resp) {
- pr_debug("Response is NULL\n");
- return 0;
- }
+ const struct opal_resp_tok *tok;
- if (n > resp->num) {
- pr_debug("Response has %d tokens. Can't access %d\n",
-resp->num, n);
+ tok = response_get_token(resp, n);
+ if (IS_ERR(tok))
return 0;
- }
- if (resp->toks[n].type != OPAL_DTA_TOKENID_UINT) {
- pr_debug("Token is not unsigned it: %d\n",
-resp->toks[n].type);
+ if (tok->type != OPAL_DTA_TOKENID_UINT) {
+ pr_debug("Token is not unsigned it: %d\n", tok->type);
return 0;
}
- if (!(resp->toks[n].width == OPAL_WIDTH_TINY ||
- resp->toks[n].width == OPAL_WIDTH_SHORT)) {
- pr_debug("Atom is not short or tiny: %d\n",
-resp->toks[n].width);
+ if (tok->width != OPAL_WIDTH_TINY && tok->width != OPAL_WIDTH_SHORT) {
+ pr_debug("Atom is not short or tiny: %d\n", tok->width);
return 0;
}
- return resp->toks[n].stored.u;
+ return tok->stored.u;
}
static bool response_token_matches(const struct opal_resp_tok *token, u8 match)
--
2.16.1
[PATCH v2 07/11] block: sed-opal: add ioctl for done-mark of shadow mbr
Enable users to mark the shadow mbr as done without completely
deactivating the shadow mbr feature. This may be useful on reboots,
when the power to the disk is not disconnected in between and the shadow
mbr stores the required boot files. Of course, this saves also the
(few) commands required to enable the feature if it is already enabled
and one only wants to mark the shadow mbr as done.
Signed-off-by: Jonas Rabenstein
diff --git a/block/sed-opal.c b/block/sed-opal.c
index fc10f81d4892..2c8baff8bf67 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -1988,13 +1988,39 @@ static int opal_erase_locking_range(struct opal_dev
*dev,
static int opal_enable_disable_shadow_mbr(struct opal_dev *dev,
struct opal_mbr_data *opal_mbr)
{
+ u8 token = opal_mbr->enable_disable == OPAL_MBR_ENABLE
+ ? OPAL_TRUE : OPAL_FALSE;
const struct opal_step mbr_steps[] = {
{ opal_discovery0, },
{ start_admin1LSP_opal_session, &opal_mbr->key },
- { set_mbr_done, &opal_mbr->enable_disable },
+ { set_mbr_done, &token },
{ end_opal_session, },
{ start_admin1LSP_opal_session, &opal_mbr->key },
- { set_mbr_enable_disable, &opal_mbr->enable_disable },
+ { set_mbr_enable_disable, &token },
+ { end_opal_session, },
+ { NULL, }
+ };
+ int ret;
+
+ if (opal_mbr->enable_disable != OPAL_MBR_ENABLE &&
+ opal_mbr->enable_disable != OPAL_MBR_DISABLE)
+ return -EINVAL;
+
+ mutex_lock(&dev->dev_lock);
+ setup_opal_dev(dev, mbr_steps);
+ ret = next(dev);
+ mutex_unlock(&dev->dev_lock);
+ return ret;
+}
+
+static int opal_mbr_status(struct opal_dev *dev, struct opal_mbr_data
*opal_mbr)
+{
+ u8 token = opal_mbr->enable_disable == OPAL_MBR_ENABLE
+ ? OPAL_TRUE : OPAL_FALSE;
+ const struct opal_step mbr_steps[] = {
+ { opal_discovery0, },
+ { start_admin1LSP_opal_session, &opal_mbr->key },
+ { set_mbr_done, &token },
{ end_opal_session, },
{ NULL, }
};
@@ -2340,6 +2366,9 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd,
void __user *arg)
case IOC_OPAL_ENABLE_DISABLE_MBR:
ret = opal_enable_disable_shadow_mbr(dev, p);
break;
+ case IOC_OPAL_MBR_STATUS:
+ ret = opal_mbr_status(dev, p);
+ break;
case IOC_OPAL_ERASE_LR:
ret = opal_erase_locking_range(dev, p);
break;
diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
index 04b124fca51e..b38dc602cae3 100644
--- a/include/linux/sed-opal.h
+++ b/include/linux/sed-opal.h
@@ -47,6 +47,7 @@ static inline bool is_sed_ioctl(unsigned int cmd)
case IOC_OPAL_ENABLE_DISABLE_MBR:
case IOC_OPAL_ERASE_LR:
case IOC_OPAL_SECURE_ERASE_LR:
+ case IOC_OPAL_MBR_STATUS:
return true;
}
return false;
diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
index 627624d35030..0cb9890cdc04 100644
--- a/include/uapi/linux/sed-opal.h
+++ b/include/uapi/linux/sed-opal.h
@@ -116,5 +116,6 @@ struct opal_mbr_data {
#define IOC_OPAL_ENABLE_DISABLE_MBR _IOW('p', 229, struct opal_mbr_data)
#define IOC_OPAL_ERASE_LR _IOW('p', 230, struct opal_session_info)
#define IOC_OPAL_SECURE_ERASE_LR_IOW('p', 231, struct opal_session_info)
+#define IOC_OPAL_MBR_STATUS _IOW('p', 232, struct opal_mbr_data)
#endif /* _UAPI_SED_OPAL_H */
--
2.16.1
[PATCH v2 08/11] block: sed-opal: ioctl for writing to shadow mbr
Allow modification of the shadow mbr. If the shadow mbr is not marked as
done, this data will be presented read only as the device content. Only
after marking the shadow mbr as done and unlocking a locking range the
actual content is accessible.
Signed-off-by: Jonas Rabenstein
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 2c8baff8bf67..4549fa164e98 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -1492,6 +1492,54 @@ static int set_mbr_enable_disable(struct opal_dev *dev,
void *data)
return finalize_and_send(dev, parse_and_check_status);
}
+static int write_shadow_mbr(struct opal_dev *dev, void *data)
+{
+ struct opal_shadow_mbr *shadow = data;
+ const u8 __user *src;
+ u8 *dst;
+ size_t off;
+ u64 len;
+ int err = 0;
+
+ /* FIXME: this is the maximum we can use for IO_BUFFER_LENGTH=2048.
+*Instead of having a constant value, it would be nice to
+*compute the actual value depending on IO_BUFFER_LENGTH
+*/
+ len = 1950;
+
+ /* do the actual transmission(s) */
+ src = (u8 *) shadow->data;
+ for (off = 0 ; off < shadow->size; off += len) {
+ len = min(len, shadow->size - off);
+
+ pr_debug("MBR: write bytes %zu+%llu/%llu\n",
+off, len, shadow->size);
+ err = start_opal_cmd(dev, opaluid[OPAL_MBR],
+opalmethod[OPAL_SET]);
+ add_token_u8(&err, dev, OPAL_STARTNAME);
+ add_token_u8(&err, dev, OPAL_WHERE);
+ add_token_u64(&err, dev, shadow->offset + off);
+ add_token_u8(&err, dev, OPAL_ENDNAME);
+
+ add_token_u8(&err, dev, OPAL_STARTNAME);
+ add_token_u8(&err, dev, OPAL_VALUES);
+ dst = add_bytestring_header(&err, dev, len);
+ if (!dst)
+ break;
+ if (copy_from_user(dst, src + off, len))
+ err = -EFAULT;
+
+ add_token_u8(&err, dev, OPAL_ENDNAME);
+ if (err)
+ break;
+
+ err = finalize_and_send(dev, parse_and_check_status);
+ if (err)
+ break;
+ }
+ return err;
+}
+
static int generic_pw_cmd(u8 *key, size_t key_len, u8 *cpin_uid,
struct opal_dev *dev)
{
@@ -2037,6 +2085,31 @@ static int opal_mbr_status(struct opal_dev *dev, struct
opal_mbr_data *opal_mbr)
return ret;
}
+static int opal_write_shadow_mbr(struct opal_dev *dev,
+struct opal_shadow_mbr *info)
+{
+ const struct opal_step mbr_steps[] = {
+ { opal_discovery0, },
+ { start_admin1LSP_opal_session, &info->key },
+ { write_shadow_mbr, info },
+ { end_opal_session, },
+ { NULL, }
+ };
+ int ret;
+
+ if (info->size == 0)
+ return 0;
+
+ if (!access_ok(VERIFY_READ, info->data, info->size))
+ return -EINVAL;
+
+ mutex_lock(&dev->dev_lock);
+ setup_opal_dev(dev, mbr_steps);
+ ret = next(dev);
+ mutex_unlock(&dev->dev_lock);
+ return ret;
+}
+
static int opal_save(struct opal_dev *dev, struct opal_lock_unlock *lk_unlk)
{
struct opal_suspend_data *suspend;
@@ -2369,6 +2442,9 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd,
void __user *arg)
case IOC_OPAL_MBR_STATUS:
ret = opal_mbr_status(dev, p);
break;
+ case IOC_OPAL_WRITE_SHADOW_MBR:
+ ret = opal_write_shadow_mbr(dev, p);
+ break;
case IOC_OPAL_ERASE_LR:
ret = opal_erase_locking_range(dev, p);
break;
diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
index b38dc602cae3..cf08cdc13cbd 100644
--- a/include/linux/sed-opal.h
+++ b/include/linux/sed-opal.h
@@ -47,6 +47,7 @@ static inline bool is_sed_ioctl(unsigned int cmd)
case IOC_OPAL_ENABLE_DISABLE_MBR:
case IOC_OPAL_ERASE_LR:
case IOC_OPAL_SECURE_ERASE_LR:
+ case IOC_OPAL_WRITE_SHADOW_MBR:
case IOC_OPAL_MBR_STATUS:
return true;
}
diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
index 0cb9890cdc04..8e84307f66d4 100644
--- a/include/uapi/linux/sed-opal.h
+++ b/include/uapi/linux/sed-opal.h
@@ -104,6 +104,13 @@ struct opal_mbr_data {
__u8 __align[7];
};
+struct opal_shadow_mbr {
+ struct opal_key key;
+ const __u64 data;
+ __u64 offset;
+ __u64 size;
+};
+
#define IOC_OPAL_SAVE _IOW('p', 220, struct opal_lock_unlock)
#define IOC_OPAL_LOCK_UNLOCK _IOW('p', 221, struct opal_lock_unlock)
#define IOC_OPAL_TAKE_OWNERSHIP_IOW(
[PATCH v2 11/11] block: sed-opal: check size of shadow mbr
Check whether the shadow mbr does fit in the provided space on the
target. Also a proper firmware should handle this case and return an
error we may prevent problems or even damage with crappy firmwares.
Signed-off-by: Jonas Rabenstein
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 51f8034edbf7..9c73bd24c55f 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -1545,6 +1545,20 @@ static int write_shadow_mbr(struct opal_dev *dev, void
*data)
u64 len;
int err = 0;
+ /* do we fit in the available shadow mbr space? */
+ err = generic_get_table_info(dev, OPAL_MBR, OPAL_TABLE_ROWS);
+ if (err) {
+ pr_debug("MBR: could not get shadow size\n");
+ return err;
+ }
+
+ len = response_get_u64(&dev->parsed, 4);
+ if (shadow->offset + shadow->size > len) {
+ pr_debug("MBR: does not fit in shadow (%llu vs. %llu)\n",
+shadow->offset + shadow->size, len);
+ return -ENOSPC;
+ }
+
/* FIXME: this is the maximum we can use for IO_BUFFER_LENGTH=2048.
*Instead of having a constant value, it would be nice to
*compute the actual value depending on IO_BUFFER_LENGTH
--
2.16.1
[PATCH v2 10/11] block: sed-opal: get metadata about opal-sed tables
Every opal-sed table is described in the OPAL_TABLE_TABLE. Provide a
function to get desired metadata information out of that table.
Signed-off-by: Jonas Rabenstein
diff --git a/block/opal_proto.h b/block/opal_proto.h
index b6e352cfe982..5e8df3245eb0 100644
--- a/block/opal_proto.h
+++ b/block/opal_proto.h
@@ -106,6 +106,7 @@ enum opal_uid {
OPAL_ENTERPRISE_BANDMASTER0_UID,
OPAL_ENTERPRISE_ERASEMASTER_UID,
/* tables */
+ OPAL_TABLE_TABLE,
OPAL_LOCKINGRANGE_GLOBAL,
OPAL_LOCKINGRANGE_ACE_RDLOCKED,
OPAL_LOCKINGRANGE_ACE_WRLOCKED,
@@ -160,6 +161,21 @@ enum opal_token {
OPAL_STARTCOLUMN = 0x03,
OPAL_ENDCOLUMN = 0x04,
OPAL_VALUES = 0x01,
+ /* table table */
+ OPAL_TABLE_UID = 0x00,
+ OPAL_TABLE_NAME = 0x01,
+ OPAL_TABLE_COMMON = 0x02,
+ OPAL_TABLE_TEMPLATE = 0x03,
+ OPAL_TABLE_KIND = 0x04,
+ OPAL_TABLE_COLUMN = 0x05,
+ OPAL_TABLE_COLUMNS = 0x06,
+ OPAL_TABLE_ROWS = 0x07,
+ OPAL_TABLE_ROWS_FREE = 0x08,
+ OPAL_TABLE_ROW_BYTES = 0x09,
+ OPAL_TABLE_LASTID = 0x0A,
+ OPAL_TABLE_MIN = 0x0B,
+ OPAL_TABLE_MAX = 0x0C,
+
/* authority table */
OPAL_PIN = 0x03,
/* locking tokens */
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 5b7b23cb95a4..51f8034edbf7 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -136,6 +136,8 @@ static const u8 opaluid[][OPAL_UID_LENGTH] = {
/* tables */
+ [OPAL_TABLE_TABLE]
+ { 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01 },
[OPAL_LOCKINGRANGE_GLOBAL] =
{ 0x00, 0x00, 0x08, 0x02, 0x00, 0x00, 0x00, 0x01 },
[OPAL_LOCKINGRANGE_ACE_RDLOCKED] =
@@ -1107,6 +1109,29 @@ static int generic_get_column(struct opal_dev *dev,
const u8 *table,
return finalize_and_send(dev, parse_and_check_status);
}
+/*
+ * see TCG SAS 5.3.2.3 for a description of the available columns
+ *
+ * the result is provided in dev->resp->tok[4]
+ */
+static int generic_get_table_info(struct opal_dev *dev, enum opal_uid table,
+ u64 column)
+{
+ u8 uid[OPAL_UID_LENGTH];
+ const unsigned int half = OPAL_UID_LENGTH/2;
+
+ /* sed-opal UIDs can be split in two halfs:
+* first: actual table index
+* second: relative index in the table
+* so we have to get the first half of the OPAL_TABLE_TABLE and use the
+* first part of the target table as relative index into that table
+*/
+ memcpy(uid, opaluid[OPAL_TABLE_TABLE], half);
+ memcpy(uid+half, opaluid[table], half);
+
+ return generic_get_column(dev, uid, column);
+}
+
static int gen_key(struct opal_dev *dev, void *data)
{
u8 uid[OPAL_UID_LENGTH];
--
2.16.1
[PATCH v2 09/11] block: sed-opal: unify retrieval of table columns
Instead of having multiple places defining the same argument list to get
a specific column of a sed-opal table, provide a generic version and
call it from those functions.
Signed-off-by: Jonas Rabenstein
diff --git a/block/opal_proto.h b/block/opal_proto.h
index e20be8258854..b6e352cfe982 100644
--- a/block/opal_proto.h
+++ b/block/opal_proto.h
@@ -170,6 +170,8 @@ enum opal_token {
OPAL_READLOCKED = 0x07,
OPAL_WRITELOCKED = 0x08,
OPAL_ACTIVEKEY = 0x0A,
+ /* lockingsp table */
+ OPAL_LIFECYCLE = 0x06,
/* locking info table */
OPAL_MAXRANGES = 0x04,
/* mbr control */
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 4549fa164e98..5b7b23cb95a4 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -1076,6 +1076,37 @@ static int finalize_and_send(struct opal_dev *dev,
cont_fn cont)
return opal_send_recv(dev, cont);
}
+/*
+ * request @column from table @table on device @dev. On success, the column
+ * data will be available in dev->resp->tok[4]
+ */
+static int generic_get_column(struct opal_dev *dev, const u8 *table,
+ u64 column)
+{
+ int err;
+
+ err = start_opal_cmd(dev, table, opalmethod[OPAL_GET]);
+
+ add_token_u8(&err, dev, OPAL_STARTLIST);
+
+ add_token_u8(&err, dev, OPAL_STARTNAME);
+ add_token_u8(&err, dev, OPAL_STARTCOLUMN);
+ add_token_u64(&err, dev, column);
+ add_token_u8(&err, dev, OPAL_ENDNAME);
+
+ add_token_u8(&err, dev, OPAL_STARTNAME);
+ add_token_u8(&err, dev, OPAL_ENDCOLUMN);
+ add_token_u64(&err, dev, column);
+ add_token_u8(&err, dev, OPAL_ENDNAME);
+
+ add_token_u8(&err, dev, OPAL_ENDLIST);
+
+ if (err)
+ return err;
+
+ return finalize_and_send(dev, parse_and_check_status);
+}
+
static int gen_key(struct opal_dev *dev, void *data)
{
u8 uid[OPAL_UID_LENGTH];
@@ -1130,23 +1161,11 @@ static int get_active_key(struct opal_dev *dev, void
*data)
if (err)
return err;
- err = start_opal_cmd(dev, uid, opalmethod[OPAL_GET]);
- add_token_u8(&err, dev, OPAL_STARTLIST);
- add_token_u8(&err, dev, OPAL_STARTNAME);
- add_token_u8(&err, dev, 3); /* startCloumn */
- add_token_u8(&err, dev, 10); /* ActiveKey */
- add_token_u8(&err, dev, OPAL_ENDNAME);
- add_token_u8(&err, dev, OPAL_STARTNAME);
- add_token_u8(&err, dev, 4); /* endColumn */
- add_token_u8(&err, dev, 10); /* ActiveKey */
- add_token_u8(&err, dev, OPAL_ENDNAME);
- add_token_u8(&err, dev, OPAL_ENDLIST);
- if (err) {
- pr_debug("Error building get active key command\n");
+ err = generic_get_column(dev, uid, OPAL_ACTIVEKEY);
+ if (err)
return err;
- }
- return finalize_and_send(dev, get_active_key_cont);
+ return get_active_key_cont(dev);
}
static int generic_lr_enable_disable(struct opal_dev *dev,
@@ -1801,14 +1820,16 @@ static int activate_lsp(struct opal_dev *dev, void
*data)
return finalize_and_send(dev, parse_and_check_status);
}
-static int get_lsp_lifecycle_cont(struct opal_dev *dev)
+/* Determine if we're in the Manufactured Inactive or Active state */
+static int get_lsp_lifecycle(struct opal_dev *dev, void *data)
{
u8 lc_status;
- int error = 0;
+ int err;
- error = parse_and_check_status(dev);
- if (error)
- return error;
+ err = generic_get_column(dev, opaluid[OPAL_LOCKINGSP_UID],
+OPAL_LIFECYCLE);
+ if (err)
+ return err;
lc_status = response_get_u64(&dev->parsed, 4);
/* 0x08 is Manufacured Inactive */
@@ -1821,49 +1842,19 @@ static int get_lsp_lifecycle_cont(struct opal_dev *dev)
return 0;
}
-/* Determine if we're in the Manufactured Inactive or Active state */
-static int get_lsp_lifecycle(struct opal_dev *dev, void *data)
-{
- int err;
-
- err = start_opal_cmd(dev, opaluid[OPAL_LOCKINGSP_UID],
-opalmethod[OPAL_GET]);
-
- add_token_u8(&err, dev, OPAL_STARTLIST);
-
- add_token_u8(&err, dev, OPAL_STARTNAME);
- add_token_u8(&err, dev, 3); /* Start Column */
- add_token_u8(&err, dev, 6); /* Lifecycle Column */
- add_token_u8(&err, dev, OPAL_ENDNAME);
-
- add_token_u8(&err, dev, OPAL_STARTNAME);
- add_token_u8(&err, dev, 4); /* End Column */
- add_token_u8(&err, dev, 6); /* Lifecycle Column */
- add_token_u8(&err, dev, OPAL_ENDNAME);
-
- add_token_u8(&err, dev, OPAL_ENDLIST);
-
- if (err) {
- pr_debug("Error Building GET Lifecycle Status command\n");
- return err;
- }
-
- return finalize_and_send(dev, get_
[PATCH v2 06/11] block: sed-opal: split generation of bytestring header and content
Split the header generation from the (normal) memcpy part if a
bytestring is copied into the command buffer. This allows in-place
generation of the bytestring content. For example, copy_from_user may be
used without an intermediate buffer.
Signed-off-by: Jonas Rabenstein
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 9b6f14e7aeb1..fc10f81d4892 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -581,14 +581,11 @@ static void add_token_u64(int *err, struct opal_dev *cmd,
u64 number)
add_token_u8(err, cmd, number >> (len * 8));
}
-static void add_token_bytestring(int *err, struct opal_dev *cmd,
-const u8 *bytestring, size_t len)
+static u8 *add_bytestring_header(int *err, struct opal_dev *cmd, size_t len)
{
size_t header_len = 1;
bool is_short_atom = true;
-
- if (*err)
- return;
+ char *start;
if (len & ~SHORT_ATOM_LEN_MASK) {
header_len = 2;
@@ -597,17 +594,27 @@ static void add_token_bytestring(int *err, struct
opal_dev *cmd,
if (!can_add(err, cmd, header_len + len)) {
pr_debug("Error adding bytestring: end of buffer.\n");
- return;
+ return NULL;
}
if (is_short_atom)
add_short_atom_header(cmd, true, false, len);
else
add_medium_atom_header(cmd, true, false, len);
-
- memcpy(&cmd->cmd[cmd->pos], bytestring, len);
+ start = &cmd->cmd[cmd->pos];
cmd->pos += len;
+ return start;
+}
+static void add_token_bytestring(int *err, struct opal_dev *cmd,
+const u8 *bytestring, size_t len)
+{
+ u8 *start;
+
+ start = add_bytestring_header(err, cmd, len);
+ if (!start)
+ return;
+ memcpy(start, bytestring, len);
}
static int build_locking_range(u8 *buffer, size_t length, u8 lr)
--
2.16.1
[PATCH v2 05/11] block: sed-opal: print failed function address
Add function address (and if available its symbol) to the message if a
step function fails.
Signed-off-by: Jonas Rabenstein
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 30f6e46518a6..9b6f14e7aeb1 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -394,8 +394,8 @@ static int next(struct opal_dev *dev)
error = step->fn(dev, step->data);
if (error) {
- pr_debug("Error on step function: %d with error %d:
%s\n",
-state, error,
+ pr_debug("Step %d (%pS) failed wit error %d: %s\n",
+state, step->fn, error,
opal_error_to_human(error));
/* For each OPAL command we do a discovery0 then we
--
2.16.1
[PATCH v2 00/11] block: sed-opal support write to shadow mbr
Hi, I was advised to resend the patchset as a v2 where all the patches are in a flat hierarchy. So here is a complete set which hopefully pleases all requirements. As the previous fixes have by now all landed into linux-next, no additional patches are required for testing. Thanks, Jonas -- Jonas Rabenstein (11): block: sed-opal: use correct macro for method length block: sed-opal: unify space check in add_token_* block: sed-opal: unify cmd start and finalize block: sed-opal: unify error handling of responses block: sed-opal: print failed function address block: sed-opal: split generation of bytestring header and content block: sed-opal: add ioctl for done-mark of shadow mbr block: sed-opal: ioctl for writing to shadow mbr block: sed-opal: unify retrieval of table columns block: sed-opal: get metadata about opal-sed tables block: sed-opal: check size of shadow mbr block/opal_proto.h| 18 ++ block/sed-opal.c | 619 +- include/linux/sed-opal.h | 2 + include/uapi/linux/sed-opal.h | 9 + 4 files changed, 339 insertions(+), 309 deletions(-) -- 2.16.1
[PATCH v2 01/11] block: sed-opal: use correct macro for method length
Also the values of OPAL_UID_LENGTH and OPAL_METHOD_LENGTH are the same,
it is weird to use OPAL_UID_LENGTH for the definition of the methods.
Signed-off-by: Jonas Rabenstein
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 945f4b8610e0..5aa41744b8f1 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -181,7 +181,7 @@ static const u8 opaluid[][OPAL_UID_LENGTH] = {
* Derived from: TCG_Storage_Architecture_Core_Spec_v2.01_r1.00
* Section: 6.3 Assigned UIDs
*/
-static const u8 opalmethod[][OPAL_UID_LENGTH] = {
+static const u8 opalmethod[][OPAL_METHOD_LENGTH] = {
[OPAL_PROPERTIES] =
{ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x01 },
[OPAL_STARTSESSION] =
--
2.16.1
[PATCH v2 3/8] block: sed-opal: unify cmd start and finalize
Every step starts with resetting the cmd buffer as well as the comid and
constructs the appropriate OPAL_CALL command. Consequently, those
actions may be combined into one generic function.
Signed-off-by: Jonas Rabenstein
---
block/sed-opal.c | 250 ---
1 file changed, 70 insertions(+), 180 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index a228a13f0a08..3bf685884fbf 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -659,6 +659,9 @@ static int cmd_finalize(struct opal_dev *cmd, u32 hsn, u32
tsn)
struct opal_header *hdr;
int err = 0;
+ /* close the parameter list opened from start_opal_cmd */
+ add_token_u8(&err, cmd, OPAL_ENDLIST);
+
add_token_u8(&err, cmd, OPAL_ENDOFDATA);
add_token_u8(&err, cmd, OPAL_STARTLIST);
add_token_u8(&err, cmd, 0);
@@ -1001,6 +1004,26 @@ static void clear_opal_cmd(struct opal_dev *dev)
memset(dev->cmd, 0, IO_BUFFER_LENGTH);
}
+static int start_opal_cmd(struct opal_dev *dev, const u8 *uid, const u8
*method)
+{
+ int err = 0;
+
+ clear_opal_cmd(dev);
+ set_comid(dev, dev->comid);
+
+ add_token_u8(&err, dev, OPAL_CALL);
+ add_token_bytestring(&err, dev, uid, OPAL_UID_LENGTH);
+ add_token_bytestring(&err, dev, method, OPAL_METHOD_LENGTH);
+
+ /* every method call is followed by its parameters enclosed within
+* OPAL_STARTLIST and OPAL_ENDLIST tokens. We automatically open the
+* parameter list here and close it later in cmd_finalize
+*/
+ add_token_u8(&err, dev, OPAL_STARTLIST);
+
+ return err;
+}
+
static int start_opal_session_cont(struct opal_dev *dev)
{
u32 hsn, tsn;
@@ -1063,21 +1086,13 @@ static int finalize_and_send(struct opal_dev *dev,
cont_fn cont)
static int gen_key(struct opal_dev *dev, void *data)
{
u8 uid[OPAL_UID_LENGTH];
- int err = 0;
-
- clear_opal_cmd(dev);
- set_comid(dev, dev->comid);
+ int err;
memcpy(uid, dev->prev_data, min(sizeof(uid), dev->prev_d_len));
kfree(dev->prev_data);
dev->prev_data = NULL;
- add_token_u8(&err, dev, OPAL_CALL);
- add_token_bytestring(&err, dev, uid, OPAL_UID_LENGTH);
- add_token_bytestring(&err, dev, opalmethod[OPAL_GENKEY],
-OPAL_UID_LENGTH);
- add_token_u8(&err, dev, OPAL_STARTLIST);
- add_token_u8(&err, dev, OPAL_ENDLIST);
+ err = start_opal_cmd(dev, uid, opalmethod[OPAL_GENKEY]);
if (err) {
pr_debug("Error building gen key command\n");
@@ -1115,21 +1130,14 @@ static int get_active_key_cont(struct opal_dev *dev)
static int get_active_key(struct opal_dev *dev, void *data)
{
u8 uid[OPAL_UID_LENGTH];
- int err = 0;
+ int err;
u8 *lr = data;
- clear_opal_cmd(dev);
- set_comid(dev, dev->comid);
-
err = build_locking_range(uid, sizeof(uid), *lr);
if (err)
return err;
- err = 0;
- add_token_u8(&err, dev, OPAL_CALL);
- add_token_bytestring(&err, dev, uid, OPAL_UID_LENGTH);
- add_token_bytestring(&err, dev, opalmethod[OPAL_GET], OPAL_UID_LENGTH);
- add_token_u8(&err, dev, OPAL_STARTLIST);
+ err = start_opal_cmd(dev, uid, opalmethod[OPAL_GET]);
add_token_u8(&err, dev, OPAL_STARTLIST);
add_token_u8(&err, dev, OPAL_STARTNAME);
add_token_u8(&err, dev, 3); /* startCloumn */
@@ -1140,7 +1148,6 @@ static int get_active_key(struct opal_dev *dev, void
*data)
add_token_u8(&err, dev, 10); /* ActiveKey */
add_token_u8(&err, dev, OPAL_ENDNAME);
add_token_u8(&err, dev, OPAL_ENDLIST);
- add_token_u8(&err, dev, OPAL_ENDLIST);
if (err) {
pr_debug("Error building get active key command\n");
return err;
@@ -1153,13 +1160,10 @@ static int generic_lr_enable_disable(struct opal_dev
*dev,
u8 *uid, bool rle, bool wle,
bool rl, bool wl)
{
- int err = 0;
+ int err;
- add_token_u8(&err, dev, OPAL_CALL);
- add_token_bytestring(&err, dev, uid, OPAL_UID_LENGTH);
- add_token_bytestring(&err, dev, opalmethod[OPAL_SET], OPAL_UID_LENGTH);
+ err = start_opal_cmd(dev, uid, opalmethod[OPAL_SET]);
- add_token_u8(&err, dev, OPAL_STARTLIST);
add_token_u8(&err, dev, OPAL_STARTNAME);
add_token_u8(&err, dev, OPAL_VALUES);
add_token_u8(&err, dev, OPAL_STARTLIST);
@@ -1186,7 +1190,6 @@ static int generic_lr_enable_disable(struct opal_dev *dev,
add_token_u8(&err, dev, OPAL_ENDLIST);
add_token_u8(&err, dev, OPAL_ENDNAME);
- add_token_u8(&err, d
[PATCH v2 8.3/8.4] block: sed-opal: get metadata about opal-sed tables
Every opal-sed table is described in the OPAL_TABLE_TABLE. Provide a
function to get desired information out of that table.
Signed-off-by: Jonas Rabenstein
---
block/opal_proto.h | 16
block/sed-opal.c | 25 +
2 files changed, 41 insertions(+)
diff --git a/block/opal_proto.h b/block/opal_proto.h
index b6e352cfe982..5e8df3245eb0 100644
--- a/block/opal_proto.h
+++ b/block/opal_proto.h
@@ -106,6 +106,7 @@ enum opal_uid {
OPAL_ENTERPRISE_BANDMASTER0_UID,
OPAL_ENTERPRISE_ERASEMASTER_UID,
/* tables */
+ OPAL_TABLE_TABLE,
OPAL_LOCKINGRANGE_GLOBAL,
OPAL_LOCKINGRANGE_ACE_RDLOCKED,
OPAL_LOCKINGRANGE_ACE_WRLOCKED,
@@ -160,6 +161,21 @@ enum opal_token {
OPAL_STARTCOLUMN = 0x03,
OPAL_ENDCOLUMN = 0x04,
OPAL_VALUES = 0x01,
+ /* table table */
+ OPAL_TABLE_UID = 0x00,
+ OPAL_TABLE_NAME = 0x01,
+ OPAL_TABLE_COMMON = 0x02,
+ OPAL_TABLE_TEMPLATE = 0x03,
+ OPAL_TABLE_KIND = 0x04,
+ OPAL_TABLE_COLUMN = 0x05,
+ OPAL_TABLE_COLUMNS = 0x06,
+ OPAL_TABLE_ROWS = 0x07,
+ OPAL_TABLE_ROWS_FREE = 0x08,
+ OPAL_TABLE_ROW_BYTES = 0x09,
+ OPAL_TABLE_LASTID = 0x0A,
+ OPAL_TABLE_MIN = 0x0B,
+ OPAL_TABLE_MAX = 0x0C,
+
/* authority table */
OPAL_PIN = 0x03,
/* locking tokens */
diff --git a/block/sed-opal.c b/block/sed-opal.c
index ebee06aaadd6..4d93a6097ec0 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -136,6 +136,8 @@ static const u8 opaluid[][OPAL_UID_LENGTH] = {
/* tables */
+ [OPAL_TABLE_TABLE]
+ { 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01 },
[OPAL_LOCKINGRANGE_GLOBAL] =
{ 0x00, 0x00, 0x08, 0x02, 0x00, 0x00, 0x00, 0x01 },
[OPAL_LOCKINGRANGE_ACE_RDLOCKED] =
@@ -1106,6 +1108,29 @@ static int generic_get_column(struct opal_dev *dev,
const u8 *table,
return finalize_and_send(dev, parse_and_check_status);
}
+/*
+ * see TCG SAS 5.3.2.3 for a description of the available columns
+ *
+ * the result is provided in dev->resp->tok[4]
+ */
+static int generic_get_table_info(struct opal_dev *dev, enum opal_uid table,
+ u64 column)
+{
+ u8 uid[OPAL_UID_LENGTH];
+ const unsigned int half = OPAL_UID_LENGTH/2;
+
+ /* sed-opal UIDs can be split in two halfs:
+* first: actual table index
+* second: relative index in the table
+* so we have to get the first half of the OPAL_TABLE_TABLE and use the
+* first part of the target table as relative index into that table
+*/
+ memcpy(uid, opaluid[OPAL_TABLE_TABLE], half);
+ memcpy(uid+half, opaluid[table], half);
+
+ return generic_get_column(dev, uid, column);
+}
+
static int gen_key(struct opal_dev *dev, void *data)
{
u8 uid[OPAL_UID_LENGTH];
--
2.16.1
[PATCH v2 8.4/8.4] block: sed-opal: check size of shadow mbr
Check whether the shadow mbr does fit in the provided space on the
target. Also a proper firmware should handle this case and return an
error we may prevent problem with crappy firmwares.
Signed-off-by: Jonas Rabenstein
---
block/sed-opal.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 4d93a6097ec0..8a08ae91bc25 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -1544,6 +1544,20 @@ static int write_shadow_mbr(struct opal_dev *dev, void
*data)
u64 len;
int err = 0;
+ /* do we fit in the available shadow mbr space? */
+ err = generic_get_table_info(dev, OPAL_MBR, OPAL_TABLE_ROWS);
+ if (err) {
+ pr_debug("MBR: could not get shadow size\n");
+ return err;
+ }
+
+ len = response_get_u64(&dev->parsed, 4);
+ if (shadow->offset + shadow->size > len) {
+ pr_debug("MBR: does not fit in shadow (%llu vs. %llu)\n",
+shadow->offset + shadow->size, len);
+ return -ENOSPC;
+ }
+
/* FIXME: this is the maximum we can use for IO_BUFFER_LENGTH=2048.
*Instead of having constant, it would be nice to compute the
*actual value depending on IO_BUFFER_LENGTH
--
2.16.1
[PATCH v2 8.2/8.4] block: sed-opal: unify retrieval of table columns
instead of having multiple places defining the same argument list to get
a specific column of a sed-opal table, provide a generic version and
call it from those functions.
Signed-off-by: Jonas Rabenstein
---
block/opal_proto.h | 2 +
block/sed-opal.c | 130 +++--
2 files changed, 49 insertions(+), 83 deletions(-)
diff --git a/block/opal_proto.h b/block/opal_proto.h
index e20be8258854..b6e352cfe982 100644
--- a/block/opal_proto.h
+++ b/block/opal_proto.h
@@ -170,6 +170,8 @@ enum opal_token {
OPAL_READLOCKED = 0x07,
OPAL_WRITELOCKED = 0x08,
OPAL_ACTIVEKEY = 0x0A,
+ /* lockingsp table */
+ OPAL_LIFECYCLE = 0x06,
/* locking info table */
OPAL_MAXRANGES = 0x04,
/* mbr control */
diff --git a/block/sed-opal.c b/block/sed-opal.c
index aa49fcc30462..ebee06aaadd6 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -1075,6 +1075,37 @@ static int finalize_and_send(struct opal_dev *dev,
cont_fn cont)
return opal_send_recv(dev, cont);
}
+/*
+ * request @column from table @table on device @dev. On success, the column
+ * data will be available in dev->resp->tok[4]
+ */
+static int generic_get_column(struct opal_dev *dev, const u8 *table,
+ u64 column)
+{
+ int err;
+
+ err = start_opal_cmd(dev, table, opalmethod[OPAL_GET]);
+
+ add_token_u8(&err, dev, OPAL_STARTLIST);
+
+ add_token_u8(&err, dev, OPAL_STARTNAME);
+ add_token_u8(&err, dev, OPAL_STARTCOLUMN);
+ add_token_u64(&err, dev, column);
+ add_token_u8(&err, dev, OPAL_ENDNAME);
+
+ add_token_u8(&err, dev, OPAL_STARTNAME);
+ add_token_u8(&err, dev, OPAL_ENDCOLUMN);
+ add_token_u64(&err, dev, column);
+ add_token_u8(&err, dev, OPAL_ENDNAME);
+
+ add_token_u8(&err, dev, OPAL_ENDLIST);
+
+ if (err)
+ return err;
+
+ return finalize_and_send(dev, parse_and_check_status);
+}
+
static int gen_key(struct opal_dev *dev, void *data)
{
u8 uid[OPAL_UID_LENGTH];
@@ -1129,23 +1160,11 @@ static int get_active_key(struct opal_dev *dev, void
*data)
if (err)
return err;
- err = start_opal_cmd(dev, uid, opalmethod[OPAL_GET]);
- add_token_u8(&err, dev, OPAL_STARTLIST);
- add_token_u8(&err, dev, OPAL_STARTNAME);
- add_token_u8(&err, dev, 3); /* startCloumn */
- add_token_u8(&err, dev, 10); /* ActiveKey */
- add_token_u8(&err, dev, OPAL_ENDNAME);
- add_token_u8(&err, dev, OPAL_STARTNAME);
- add_token_u8(&err, dev, 4); /* endColumn */
- add_token_u8(&err, dev, 10); /* ActiveKey */
- add_token_u8(&err, dev, OPAL_ENDNAME);
- add_token_u8(&err, dev, OPAL_ENDLIST);
- if (err) {
- pr_debug("Error building get active key command\n");
+ err = generic_get_column(dev, uid, OPAL_ACTIVEKEY);
+ if (err)
return err;
- }
- return finalize_and_send(dev, get_active_key_cont);
+ return get_active_key_cont(dev);
}
static int generic_lr_enable_disable(struct opal_dev *dev,
@@ -1800,14 +1819,16 @@ static int activate_lsp(struct opal_dev *dev, void
*data)
return finalize_and_send(dev, parse_and_check_status);
}
-static int get_lsp_lifecycle_cont(struct opal_dev *dev)
+/* Determine if we're in the Manufactured Inactive or Active state */
+static int get_lsp_lifecycle(struct opal_dev *dev, void *data)
{
u8 lc_status;
- int error = 0;
+ int err;
- error = parse_and_check_status(dev);
- if (error)
- return error;
+ err = generic_get_column(dev, opaluid[OPAL_LOCKINGSP_UID],
+OPAL_LIFECYCLE);
+ if (err)
+ return err;
lc_status = response_get_u64(&dev->parsed, 4);
/* 0x08 is Manufacured Inactive */
@@ -1820,49 +1841,19 @@ static int get_lsp_lifecycle_cont(struct opal_dev *dev)
return 0;
}
-/* Determine if we're in the Manufactured Inactive or Active state */
-static int get_lsp_lifecycle(struct opal_dev *dev, void *data)
-{
- int err;
-
- err = start_opal_cmd(dev, opaluid[OPAL_LOCKINGSP_UID],
-opalmethod[OPAL_GET]);
-
- add_token_u8(&err, dev, OPAL_STARTLIST);
-
- add_token_u8(&err, dev, OPAL_STARTNAME);
- add_token_u8(&err, dev, 3); /* Start Column */
- add_token_u8(&err, dev, 6); /* Lifecycle Column */
- add_token_u8(&err, dev, OPAL_ENDNAME);
-
- add_token_u8(&err, dev, OPAL_STARTNAME);
- add_token_u8(&err, dev, 4); /* End Column */
- add_token_u8(&err, dev, 6); /* Lifecycle Column */
- add_token_u8(&err, dev, OPAL_ENDNAME);
-
- add_token_u8(&err, dev, OPAL_ENDLIST);
-
- if (err)
[PATCH v2 8.0/8.4] block: sed-opal: check size of shadow mbr
Hi, I managed to extract the usable shadow mbr size out of my 850Evos OPAL_TABLE_TABLE and added an appropriate check into the write function. As this involves more than just a few lines, I decided to split the v2 of this subpatch into 4 separate patches. I am unsure what whould be the best practice for such an situation but hope it is okay like this. Also the userspace exported pointer is replaced with an u64 value. Jonas Rabenstein (4): block: sed-opal: ioctl for writing to shadow mbr block: sed-opal: unify retrieval of table columns block: sed-opal: get metadata about opal-sed tables block: sed-opal: check size of shadow mbr block/opal_proto.h| 18 block/sed-opal.c | 245 -- include/linux/sed-opal.h | 1 + include/uapi/linux/sed-opal.h | 8 ++ 4 files changed, 189 insertions(+), 83 deletions(-) -- 2.16.1
[PATCH v2 8.1/8.4] block: sed-opal: ioctl for writing to shadow mbr
Allow modification of the shadow mbr. If the shadow mbr is not marked as
done, this data will be presented read only as the device content. Only
after marking the shadow mbr as done and unlocking a locking range the
actual content is accessible.
Signed-off-by: Jonas Rabenstein
---
block/sed-opal.c | 76 +++
include/linux/sed-opal.h | 1 +
include/uapi/linux/sed-opal.h | 8 +
3 files changed, 85 insertions(+)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index b201c96d23a3..aa49fcc30462 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -1491,6 +1491,54 @@ static int set_mbr_enable_disable(struct opal_dev *dev,
void *data)
return finalize_and_send(dev, parse_and_check_status);
}
+static int write_shadow_mbr(struct opal_dev *dev, void *data)
+{
+ struct opal_shadow_mbr *shadow = data;
+ const u8 __user *src;
+ u8 *dst;
+ size_t off;
+ u64 len;
+ int err = 0;
+
+ /* FIXME: this is the maximum we can use for IO_BUFFER_LENGTH=2048.
+*Instead of having constant, it would be nice to compute the
+*actual value depending on IO_BUFFER_LENGTH
+*/
+ len = 1950;
+
+ /* do the actual transmission(s) */
+ src = (u8 *) shadow->data;
+ for (off = 0 ; off < shadow->size; off += len) {
+ len = min(len, shadow->size - off);
+
+ pr_debug("MBR: write bytes %zu+%llu/%llu\n",
+off, len, shadow->size);
+ err = start_opal_cmd(dev, opaluid[OPAL_MBR],
+opalmethod[OPAL_SET]);
+ add_token_u8(&err, dev, OPAL_STARTNAME);
+ add_token_u8(&err, dev, OPAL_WHERE);
+ add_token_u64(&err, dev, shadow->offset + off);
+ add_token_u8(&err, dev, OPAL_ENDNAME);
+
+ add_token_u8(&err, dev, OPAL_STARTNAME);
+ add_token_u8(&err, dev, OPAL_VALUES);
+ dst = add_bytestring_header(&err, dev, len);
+ if (!dst)
+ break;
+ if (copy_from_user(dst, src + off, len))
+ err = -EFAULT;
+
+ add_token_u8(&err, dev, OPAL_ENDNAME);
+ if (err)
+ break;
+
+ err = finalize_and_send(dev, parse_and_check_status);
+ if (err)
+ break;
+ }
+ return err;
+}
+
static int generic_pw_cmd(u8 *key, size_t key_len, u8 *cpin_uid,
struct opal_dev *dev)
{
@@ -2036,6 +2084,31 @@ static int opal_mbr_status(struct opal_dev *dev, struct
opal_mbr_data *opal_mbr)
return ret;
}
+static int opal_write_shadow_mbr(struct opal_dev *dev,
+struct opal_shadow_mbr *info)
+{
+ const struct opal_step mbr_steps[] = {
+ { opal_discovery0, },
+ { start_admin1LSP_opal_session, &info->key },
+ { write_shadow_mbr, info },
+ { end_opal_session, },
+ { NULL, }
+ };
+ int ret;
+
+ if (info->size == 0)
+ return 0;
+
+ if (!access_ok(VERIFY_READ, info->data, info->size))
+ return -EINVAL;
+
+ mutex_lock(&dev->dev_lock);
+ setup_opal_dev(dev, mbr_steps);
+ ret = next(dev);
+ mutex_unlock(&dev->dev_lock);
+ return ret;
+}
+
static int opal_save(struct opal_dev *dev, struct opal_lock_unlock *lk_unlk)
{
struct opal_suspend_data *suspend;
@@ -2368,6 +2441,9 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd,
void __user *arg)
case IOC_OPAL_MBR_STATUS:
ret = opal_mbr_status(dev, p);
break;
+ case IOC_OPAL_WRITE_SHADOW_MBR:
+ ret = opal_write_shadow_mbr(dev, p);
+ break;
case IOC_OPAL_ERASE_LR:
ret = opal_erase_locking_range(dev, p);
break;
diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
index b38dc602cae3..cf08cdc13cbd 100644
--- a/include/linux/sed-opal.h
+++ b/include/linux/sed-opal.h
@@ -47,6 +47,7 @@ static inline bool is_sed_ioctl(unsigned int cmd)
case IOC_OPAL_ENABLE_DISABLE_MBR:
case IOC_OPAL_ERASE_LR:
case IOC_OPAL_SECURE_ERASE_LR:
+ case IOC_OPAL_WRITE_SHADOW_MBR:
case IOC_OPAL_MBR_STATUS:
return true;
}
diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
index 0cb9890cdc04..ab4b69778bd0 100644
--- a/include/uapi/linux/sed-opal.h
+++ b/include/uapi/linux/sed-opal.h
@@ -104,6 +104,13 @@ struct opal_mbr_data {
__u8 __align[7];
};
+struct opal_shadow_mbr {
+ struct opal_key key;
+ const __u64 data;
+ __u64 offset;
+ __u64 size;
+};
+
#define IOC_OPAL_SAVE
[PATCH 2/8] block: sed-opal: unify space check in add_token_*
All add_token_* functions have a common set of conditions that have to
be checked. Use a common function for those checks in order to avoid
different behaviour.
Signed-off-by: Jonas Rabenstein
---
block/sed-opal.c | 25 -
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index f9d94ce64b08..a228a13f0a08 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -510,15 +510,24 @@ static int opal_discovery0(struct opal_dev *dev, void
*data)
return opal_discovery0_end(dev);
}
-static void add_token_u8(int *err, struct opal_dev *cmd, u8 tok)
+static bool can_add(int *err, struct opal_dev *cmd, size_t len)
{
if (*err)
- return;
- if (cmd->pos >= IO_BUFFER_LENGTH - 1) {
- pr_debug("Error adding u8: end of buffer.\n");
+ return false;
+
+ if (len > IO_BUFFER_LENGTH || cmd->pos >= IO_BUFFER_LENGTH - len) {
+ pr_debug("Error adding %zu bytes: end of buffer.\n", len);
*err = -ERANGE;
- return;
+ return false;
}
+
+ return true;
+}
+
+static void add_token_u8(int *err, struct opal_dev *cmd, u8 tok)
+{
+ if (!can_add(err, cmd, 1))
+ return;
cmd->cmd[cmd->pos++] = tok;
}
@@ -564,9 +573,8 @@ static void add_token_u64(int *err, struct opal_dev *cmd,
u64 number)
msb = fls(number);
len = DIV_ROUND_UP(msb, 4);
- if (cmd->pos >= IO_BUFFER_LENGTH - len - 1) {
+ if (!can_add(err, cmd, len + 1)) {
pr_debug("Error adding u64: end of buffer.\n");
- *err = -ERANGE;
return;
}
add_short_atom_header(cmd, false, false, len);
@@ -590,9 +598,8 @@ static void add_token_bytestring(int *err, struct opal_dev
*cmd,
is_short_atom = false;
}
- if (len >= IO_BUFFER_LENGTH - cmd->pos - header_len) {
+ if (!can_add(err, cmd, header_len + len)) {
pr_debug("Error adding bytestring: end of buffer.\n");
- *err = -ERANGE;
return;
}
--
2.16.1
[PATCH 5/8] block: sed-opal: print failed function address
add function address (and if available its symbol) to the message if a
step function fails.
Signed-off-by: Jonas Rabenstein
---
block/sed-opal.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 5a448a3ba1df..f7925e6f607c 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -394,8 +394,8 @@ static int next(struct opal_dev *dev)
error = step->fn(dev, step->data);
if (error) {
- pr_debug("Error on step function: %d with error %d:
%s\n",
-state, error,
+ pr_info("Error on step function %pS: %d with error %d:
%s\n",
+step->fn, state, error,
opal_error_to_human(error));
/* For each OPAL command we do a discovery0 then we
--
2.16.1
[PATCH 8/8] block: sed-opal: ioctl for writing to shadow mbr
Allow modification of the shadow mbr. If the shadow mbr is not marked as
done, this data will be presented read only as the device content. Only
after marking the shadow mbr as done and unlocking a locking range the
actual content is accessible.
Signed-off-by: Jonas Rabenstein
---
block/sed-opal.c | 74 +++
include/linux/sed-opal.h | 1 +
include/uapi/linux/sed-opal.h | 8 +
3 files changed, 83 insertions(+)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index b201c96d23a3..1ec3734af656 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -1491,6 +1491,52 @@ static int set_mbr_enable_disable(struct opal_dev *dev,
void *data)
return finalize_and_send(dev, parse_and_check_status);
}
+static int write_shadow_mbr(struct opal_dev *dev, void *data)
+{
+ struct opal_shadow_mbr *shadow = data;
+ size_t off;
+ u64 len;
+ int err = 0;
+ u8 *payload;
+
+ /* FIXME: this is the maximum we can use for IO_BUFFER_LENGTH=2048.
+*Instead of having constant, it would be nice to compute the
+*actual value depending on IO_BUFFER_LENGTH
+*/
+ len = 1950;
+
+ /* do the actual transmission(s) */
+ for (off = 0 ; off < shadow->size; off += len) {
+ len = min(len, shadow->size - off);
+
+ pr_debug("MBR: write bytes %zu+%llu/%llu\n",
+off, len, shadow->size);
+ err = start_opal_cmd(dev, opaluid[OPAL_MBR],
+opalmethod[OPAL_SET]);
+ add_token_u8(&err, dev, OPAL_STARTNAME);
+ add_token_u8(&err, dev, OPAL_WHERE);
+ add_token_u64(&err, dev, shadow->offset + off);
+ add_token_u8(&err, dev, OPAL_ENDNAME);
+
+ add_token_u8(&err, dev, OPAL_STARTNAME);
+ add_token_u8(&err, dev, OPAL_VALUES);
+ payload = add_bytestring_header(&err, dev, len);
+ if (!payload)
+ break;
+ if (copy_from_user(payload, shadow->data + off, len))
+ err = -EFAULT;
+
+ add_token_u8(&err, dev, OPAL_ENDNAME);
+ if (err)
+ break;
+
+ err = finalize_and_send(dev, parse_and_check_status);
+ if (err)
+ break;
+ }
+ return err;
+}
+
static int generic_pw_cmd(u8 *key, size_t key_len, u8 *cpin_uid,
struct opal_dev *dev)
{
@@ -2036,6 +2082,31 @@ static int opal_mbr_status(struct opal_dev *dev, struct
opal_mbr_data *opal_mbr)
return ret;
}
+static int opal_write_shadow_mbr(struct opal_dev *dev,
+struct opal_shadow_mbr *info)
+{
+ const struct opal_step mbr_steps[] = {
+ { opal_discovery0, },
+ { start_admin1LSP_opal_session, &info->key },
+ { write_shadow_mbr, info },
+ { end_opal_session, },
+ { NULL, }
+ };
+ int ret;
+
+ if (info->size == 0)
+ return 0;
+
+ if (!access_ok(VERIFY_READ, info->data, info->size))
+ return -EINVAL;
+
+ mutex_lock(&dev->dev_lock);
+ setup_opal_dev(dev, mbr_steps);
+ ret = next(dev);
+ mutex_unlock(&dev->dev_lock);
+ return ret;
+}
+
static int opal_save(struct opal_dev *dev, struct opal_lock_unlock *lk_unlk)
{
struct opal_suspend_data *suspend;
@@ -2368,6 +2439,9 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd,
void __user *arg)
case IOC_OPAL_MBR_STATUS:
ret = opal_mbr_status(dev, p);
break;
+ case IOC_OPAL_WRITE_SHADOW_MBR:
+ ret = opal_write_shadow_mbr(dev, p);
+ break;
case IOC_OPAL_ERASE_LR:
ret = opal_erase_locking_range(dev, p);
break;
diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
index b38dc602cae3..cf08cdc13cbd 100644
--- a/include/linux/sed-opal.h
+++ b/include/linux/sed-opal.h
@@ -47,6 +47,7 @@ static inline bool is_sed_ioctl(unsigned int cmd)
case IOC_OPAL_ENABLE_DISABLE_MBR:
case IOC_OPAL_ERASE_LR:
case IOC_OPAL_SECURE_ERASE_LR:
+ case IOC_OPAL_WRITE_SHADOW_MBR:
case IOC_OPAL_MBR_STATUS:
return true;
}
diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
index 0cb9890cdc04..c2669ebff681 100644
--- a/include/uapi/linux/sed-opal.h
+++ b/include/uapi/linux/sed-opal.h
@@ -104,6 +104,13 @@ struct opal_mbr_data {
__u8 __align[7];
};
+struct opal_shadow_mbr {
+ struct opal_key key;
+ const __u8 *data;
+ __u64 offset;
+ __u64 size;
+};
+
#define IOC_OPAL_SAVE _IOW('p', 220, struct op
[PATCH 1/8] block: sed-opal: use correct macro for method length
also the values of OPAL_UID_LENGTH and OPAL_METHOD_LENGTH are the same,
it is weird to use OPAL_UID_LENGTH for the definition of the methods.
Signed-off-by: Jonas Rabenstein
---
block/sed-opal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 2158624013b3..f9d94ce64b08 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -181,7 +181,7 @@ static const u8 opaluid[][OPAL_UID_LENGTH] = {
* Derived from: TCG_Storage_Architecture_Core_Spec_v2.01_r1.00
* Section: 6.3 Assigned UIDs
*/
-static const u8 opalmethod[][OPAL_UID_LENGTH] = {
+static const u8 opalmethod[][OPAL_METHOD_LENGTH] = {
[OPAL_PROPERTIES] =
{ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x01 },
[OPAL_STARTSESSION] =
--
2.16.1
[PATCH 7/8] block: sed-opal: add ioctl for done-mark of shadow mbr
Enable users to mark the shadow mbr as done without completely
deactivating the shadow mbr feature. This may be useful on reboots,
when the power to the disk is not disconnected in between and the shadow
mbr stores the required boot files. Of course, this saves also the
(few) commands required to enable the feature if it is already enabled
and one only wants to mark the shadow mbr as done.
Signed-off-by: Jonas Rabenstein
---
block/sed-opal.c | 33 +++--
include/linux/sed-opal.h | 1 +
include/uapi/linux/sed-opal.h | 1 +
3 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index f169e621279a..b201c96d23a3 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -1987,13 +1987,39 @@ static int opal_erase_locking_range(struct opal_dev
*dev,
static int opal_enable_disable_shadow_mbr(struct opal_dev *dev,
struct opal_mbr_data *opal_mbr)
{
+ u8 token = opal_mbr->enable_disable == OPAL_MBR_ENABLE
+ ? OPAL_TRUE : OPAL_FALSE;
const struct opal_step mbr_steps[] = {
{ opal_discovery0, },
{ start_admin1LSP_opal_session, &opal_mbr->key },
- { set_mbr_done, &opal_mbr->enable_disable },
+ { set_mbr_done, &token },
{ end_opal_session, },
{ start_admin1LSP_opal_session, &opal_mbr->key },
- { set_mbr_enable_disable, &opal_mbr->enable_disable },
+ { set_mbr_enable_disable, &token },
+ { end_opal_session, },
+ { NULL, }
+ };
+ int ret;
+
+ if (opal_mbr->enable_disable != OPAL_MBR_ENABLE &&
+ opal_mbr->enable_disable != OPAL_MBR_DISABLE)
+ return -EINVAL;
+
+ mutex_lock(&dev->dev_lock);
+ setup_opal_dev(dev, mbr_steps);
+ ret = next(dev);
+ mutex_unlock(&dev->dev_lock);
+ return ret;
+}
+
+static int opal_mbr_status(struct opal_dev *dev, struct opal_mbr_data
*opal_mbr)
+{
+ u8 token = opal_mbr->enable_disable == OPAL_MBR_ENABLE
+ ? OPAL_TRUE : OPAL_FALSE;
+ const struct opal_step mbr_steps[] = {
+ { opal_discovery0, },
+ { start_admin1LSP_opal_session, &opal_mbr->key },
+ { set_mbr_done, &token },
{ end_opal_session, },
{ NULL, }
};
@@ -2339,6 +2365,9 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd,
void __user *arg)
case IOC_OPAL_ENABLE_DISABLE_MBR:
ret = opal_enable_disable_shadow_mbr(dev, p);
break;
+ case IOC_OPAL_MBR_STATUS:
+ ret = opal_mbr_status(dev, p);
+ break;
case IOC_OPAL_ERASE_LR:
ret = opal_erase_locking_range(dev, p);
break;
diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
index 04b124fca51e..b38dc602cae3 100644
--- a/include/linux/sed-opal.h
+++ b/include/linux/sed-opal.h
@@ -47,6 +47,7 @@ static inline bool is_sed_ioctl(unsigned int cmd)
case IOC_OPAL_ENABLE_DISABLE_MBR:
case IOC_OPAL_ERASE_LR:
case IOC_OPAL_SECURE_ERASE_LR:
+ case IOC_OPAL_MBR_STATUS:
return true;
}
return false;
diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
index 627624d35030..0cb9890cdc04 100644
--- a/include/uapi/linux/sed-opal.h
+++ b/include/uapi/linux/sed-opal.h
@@ -116,5 +116,6 @@ struct opal_mbr_data {
#define IOC_OPAL_ENABLE_DISABLE_MBR _IOW('p', 229, struct opal_mbr_data)
#define IOC_OPAL_ERASE_LR _IOW('p', 230, struct opal_session_info)
#define IOC_OPAL_SECURE_ERASE_LR_IOW('p', 231, struct opal_session_info)
+#define IOC_OPAL_MBR_STATUS _IOW('p', 232, struct opal_mbr_data)
#endif /* _UAPI_SED_OPAL_H */
--
2.16.1
[PATCH 4/8] block: sed-opal: unify error handling of responses
Also response_get_token had already been in place, its functionality had
been duplicated within response_get_{u64,bytestring} with the same error
handling. Unify the handling by reusing response_get_token within the
other functions.
Signed-off-by: Jonas Rabenstein
---
block/sed-opal.c | 28 ++--
1 file changed, 10 insertions(+), 18 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 22dbea7cf4d1..5a448a3ba1df 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -698,6 +698,11 @@ static const struct opal_resp_tok *response_get_token(
{
const struct opal_resp_tok *tok;
+ if (!resp) {
+ pr_debug("Response is NULL\n");
+ return ERR_PTR(-EINVAL);
+ }
+
if (n >= resp->num) {
pr_debug("Token number doesn't exist: %d, resp: %d\n",
n, resp->num);
@@ -883,18 +888,10 @@ static size_t response_get_string(const struct
parsed_resp *resp, int n,
const struct opal_resp_tok *token;
*store = NULL;
- if (!resp) {
- pr_debug("Response is NULL\n");
+ token = response_get_token(resp, n);
+ if (IS_ERR(token))
return 0;
- }
- if (n > resp->num) {
- pr_debug("Response has %d tokens. Can't access %d\n",
-resp->num, n);
- return 0;
- }
-
- token = &resp->toks[n];
if (token->type != OPAL_DTA_TOKENID_BYTESTRING) {
pr_debug("Token is not a byte string!\n");
return 0;
@@ -922,16 +919,11 @@ static size_t response_get_string(const struct
parsed_resp *resp, int n,
static u64 response_get_u64(const struct parsed_resp *resp, int n)
{
- if (!resp) {
- pr_debug("Response is NULL\n");
- return 0;
- }
+ const struct opal_resp_tok *token;
- if (n > resp->num) {
- pr_debug("Response has %d tokens. Can't access %d\n",
-resp->num, n);
+ token = response_get_token(resp, n);
+ if (IS_ERR(token))
return 0;
- }
if (resp->toks[n].type != OPAL_DTA_TOKENID_UINT) {
pr_debug("Token is not unsigned it: %d\n",
--
2.16.1
[PATCH 3/8] block: sed-opal: unify cmd start and finalize
Every step starts with resetting the cmd buffer as well as the comid and
constructs the appropriate OPAL_CALL command. Consequently, those
actions may be combined into one generic function.
Signed-off-by: Jonas Rabenstein
---
block/sed-opal.c | 243 +++
1 file changed, 63 insertions(+), 180 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index a228a13f0a08..22dbea7cf4d1 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -659,6 +659,7 @@ static int cmd_finalize(struct opal_dev *cmd, u32 hsn, u32
tsn)
struct opal_header *hdr;
int err = 0;
+ add_token_u8(&err, cmd, OPAL_ENDLIST);
add_token_u8(&err, cmd, OPAL_ENDOFDATA);
add_token_u8(&err, cmd, OPAL_STARTLIST);
add_token_u8(&err, cmd, 0);
@@ -1001,6 +1002,21 @@ static void clear_opal_cmd(struct opal_dev *dev)
memset(dev->cmd, 0, IO_BUFFER_LENGTH);
}
+static int start_opal_cmd(struct opal_dev *dev, const u8 *uid, const u8
*method)
+{
+ int err = 0;
+
+ clear_opal_cmd(dev);
+ set_comid(dev, dev->comid);
+
+ add_token_u8(&err, dev, OPAL_CALL);
+ add_token_bytestring(&err, dev, uid, OPAL_UID_LENGTH);
+ add_token_bytestring(&err, dev, method, OPAL_METHOD_LENGTH);
+ add_token_u8(&err, dev, OPAL_STARTLIST);
+
+ return err;
+}
+
static int start_opal_session_cont(struct opal_dev *dev)
{
u32 hsn, tsn;
@@ -1063,21 +1079,13 @@ static int finalize_and_send(struct opal_dev *dev,
cont_fn cont)
static int gen_key(struct opal_dev *dev, void *data)
{
u8 uid[OPAL_UID_LENGTH];
- int err = 0;
-
- clear_opal_cmd(dev);
- set_comid(dev, dev->comid);
+ int err;
memcpy(uid, dev->prev_data, min(sizeof(uid), dev->prev_d_len));
kfree(dev->prev_data);
dev->prev_data = NULL;
- add_token_u8(&err, dev, OPAL_CALL);
- add_token_bytestring(&err, dev, uid, OPAL_UID_LENGTH);
- add_token_bytestring(&err, dev, opalmethod[OPAL_GENKEY],
-OPAL_UID_LENGTH);
- add_token_u8(&err, dev, OPAL_STARTLIST);
- add_token_u8(&err, dev, OPAL_ENDLIST);
+ err = start_opal_cmd(dev, uid, opalmethod[OPAL_GENKEY]);
if (err) {
pr_debug("Error building gen key command\n");
@@ -1115,21 +1123,14 @@ static int get_active_key_cont(struct opal_dev *dev)
static int get_active_key(struct opal_dev *dev, void *data)
{
u8 uid[OPAL_UID_LENGTH];
- int err = 0;
+ int err;
u8 *lr = data;
- clear_opal_cmd(dev);
- set_comid(dev, dev->comid);
-
err = build_locking_range(uid, sizeof(uid), *lr);
if (err)
return err;
- err = 0;
- add_token_u8(&err, dev, OPAL_CALL);
- add_token_bytestring(&err, dev, uid, OPAL_UID_LENGTH);
- add_token_bytestring(&err, dev, opalmethod[OPAL_GET], OPAL_UID_LENGTH);
- add_token_u8(&err, dev, OPAL_STARTLIST);
+ err = start_opal_cmd(dev, uid, opalmethod[OPAL_GET]);
add_token_u8(&err, dev, OPAL_STARTLIST);
add_token_u8(&err, dev, OPAL_STARTNAME);
add_token_u8(&err, dev, 3); /* startCloumn */
@@ -1140,7 +1141,6 @@ static int get_active_key(struct opal_dev *dev, void
*data)
add_token_u8(&err, dev, 10); /* ActiveKey */
add_token_u8(&err, dev, OPAL_ENDNAME);
add_token_u8(&err, dev, OPAL_ENDLIST);
- add_token_u8(&err, dev, OPAL_ENDLIST);
if (err) {
pr_debug("Error building get active key command\n");
return err;
@@ -1153,13 +1153,10 @@ static int generic_lr_enable_disable(struct opal_dev
*dev,
u8 *uid, bool rle, bool wle,
bool rl, bool wl)
{
- int err = 0;
+ int err;
- add_token_u8(&err, dev, OPAL_CALL);
- add_token_bytestring(&err, dev, uid, OPAL_UID_LENGTH);
- add_token_bytestring(&err, dev, opalmethod[OPAL_SET], OPAL_UID_LENGTH);
+ err = start_opal_cmd(dev, uid, opalmethod[OPAL_SET]);
- add_token_u8(&err, dev, OPAL_STARTLIST);
add_token_u8(&err, dev, OPAL_STARTNAME);
add_token_u8(&err, dev, OPAL_VALUES);
add_token_u8(&err, dev, OPAL_STARTLIST);
@@ -1186,7 +1183,6 @@ static int generic_lr_enable_disable(struct opal_dev *dev,
add_token_u8(&err, dev, OPAL_ENDLIST);
add_token_u8(&err, dev, OPAL_ENDNAME);
- add_token_u8(&err, dev, OPAL_ENDLIST);
return err;
}
@@ -1207,10 +1203,7 @@ static int setup_locking_range(struct opal_dev *dev,
void *data)
u8 uid[OPAL_UID_LENGTH];
struct opal_user_lr_setup *setup = data;
u8 lr;
- int err = 0;
-
- clear_opal_cmd(dev);
- set_comid(dev, dev
[PATCH 6/8] block: sed-opal: split generation of bytestring header and content
Split the header generation from the (normal) memcpy part if a
bytestring is copied into the command buffer. This allows in-place
generation of the bytestring content. For example, copy_from_user may be
used without an intermediate buffer.
Signed-off-by: Jonas Rabenstein
---
block/sed-opal.c | 23 +++
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index f7925e6f607c..f169e621279a 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -584,14 +584,11 @@ static void add_token_u64(int *err, struct opal_dev *cmd,
u64 number)
}
}
-static void add_token_bytestring(int *err, struct opal_dev *cmd,
-const u8 *bytestring, size_t len)
+static u8 *add_bytestring_header(int *err, struct opal_dev *cmd, size_t len)
{
size_t header_len = 1;
bool is_short_atom = true;
-
- if (*err)
- return;
+ char *start;
if (len & ~SHORT_ATOM_LEN_MASK) {
header_len = 2;
@@ -600,17 +597,27 @@ static void add_token_bytestring(int *err, struct
opal_dev *cmd,
if (!can_add(err, cmd, header_len + len)) {
pr_debug("Error adding bytestring: end of buffer.\n");
- return;
+ return NULL;
}
if (is_short_atom)
add_short_atom_header(cmd, true, false, len);
else
add_medium_atom_header(cmd, true, false, len);
-
- memcpy(&cmd->cmd[cmd->pos], bytestring, len);
+ start = &cmd->cmd[cmd->pos];
cmd->pos += len;
+ return start;
+}
+static void add_token_bytestring(int *err, struct opal_dev *cmd,
+const u8 *bytestring, size_t len)
+{
+ u8 *start;
+
+ start = add_bytestring_header(err, cmd, len);
+ if (!start)
+ return;
+ memcpy(start, bytestring, len);
}
static int build_locking_range(u8 *buffer, size_t length, u8 lr)
--
2.16.1
[PATCH 0/8] block: sed-opal: support write to shadow mbr
Hi, this patchset adds support to write data into the shadow mbr of sed-opal enabled devices. They apply cleanly on today next-tree (next-20180313) and requires the u64 short atom length fix from [0] as that is still missing in that tree. As I only can test on my only sed-opal enabled Samsung 850 Evo on amd64, tests on other hardware would be appreciated. The first six patches provide helper functions that are used in the following up patches as well as do some refactoring and unification of the existing code. With the seventh patch, a new ioctl is added to mark the shadow mbr as done without having to enable/disable the shadow mbr feature as a whole. Finally, the last patch adds an ioctl to write data into the shadow mbr. A modified version of the user space tools with support for those ioctls can be found at [1]. Looking forward to feedback and suggestions for improvement, Jonas [0] https://lkml.org/lkml/2018/3/7/534 [1] https://github.com/ghostav/sed-opal-temp Jonas Rabenstein (8): block: sed-opal: use correct macro for method length block: sed-opal: unify space check in add_token_* block: sed-opal: unify cmd start and finalize block: sed-opal: unify error handling of responses block: sed-opal: print failed function address block: sed-opal: split generation of bytestring header and content block: sed-opal: add ioctl for done-mark of shadow mbr block: sed-opal: ioctl for writing to shadow mbr block/sed-opal.c | 432 +- include/linux/sed-opal.h | 2 + include/uapi/linux/sed-opal.h | 9 + 3 files changed, 223 insertions(+), 220 deletions(-) -- 2.16.1
Re: [PATCH v2] block: sed-opal: fix u64 short atom length
On Wed, Mar 07, 2018 at 04:24:29PM -0700, Scott Bauer wrote: > On Wed, Mar 07, 2018 at 05:55:56PM +0100, Jonas Rabenstein wrote: > > The length must be given as bytes and not as 4 bit tuples. > > > > Signed-off-by: Jonas Rabenstein > > --- > > v2: > > - use fls64 > > - shorten loop body > > --- > > block/sed-opal.c | 11 --- > > 1 file changed, 4 insertions(+), 7 deletions(-) > > > > Reviewed-by: Scott Bauer > > Your two patches should be sent to stable for 4.14. I can queue those up and > do it, > or if you want to you can do it as well. Let me know what you prefer! As I am quite new to all that kernel patch submitting stuff, I would be glad if you could do it for me so I will not mess it up (; Thanks, Jonas
[PATCH v2] block: sed-opal: fix u64 short atom length
The length must be given as bytes and not as 4 bit tuples.
Signed-off-by: Jonas Rabenstein
---
v2:
- use fls64
- shorten loop body
---
block/sed-opal.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 36842bfa572e..38411c5c477f 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -554,15 +554,14 @@ static void add_token_u64(int *err, struct opal_dev *cmd,
u64 number)
size_t len;
int msb;
- u8 n;
if (!(number & ~TINY_ATOM_DATA_MASK)) {
add_token_u8(err, cmd, number);
return;
}
- msb = fls(number);
- len = DIV_ROUND_UP(msb, 4);
+ msb = fls64(number);
+ len = DIV_ROUND_UP(msb, 8);
if (cmd->pos >= IO_BUFFER_LENGTH - len - 1) {
pr_debug("Error adding u64: end of buffer.\n");
@@ -570,10 +569,8 @@ static void add_token_u64(int *err, struct opal_dev *cmd,
u64 number)
return;
}
add_short_atom_header(cmd, false, false, len);
- while (len--) {
- n = number >> (len * 8);
- add_token_u8(err, cmd, n);
- }
+ while (len--)
+ add_token_u8(err, cmd, number >> (len * 8));
}
static void add_token_bytestring(int *err, struct opal_dev *cmd,
--
2.16.1
[PATCH][RESEND] block: sed-opal: fix response string extraction
Tokens are prefixed by a variable length of bytes. If a bytestring is
not stored in an tiny or short atom, we have to skip more than one byte
in order to have the actual bytes not prefixed by the bytes describing
the actual length of the string.
Signed-off-by: Jonas Rabenstein
---
block/sed-opal.c | 26 +++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 525506bed399..33052d0111de 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -876,6 +876,9 @@ static int response_parse(const u8 *buf, size_t length,
static size_t response_get_string(const struct parsed_resp *resp, int n,
const char **store)
{
+ u8 skip;
+ const struct opal_resp_tok *token;
+
*store = NULL;
if (!resp) {
pr_debug("Response is NULL\n");
@@ -888,13 +891,30 @@ static size_t response_get_string(const struct
parsed_resp *resp, int n,
return 0;
}
- if (resp->toks[n].type != OPAL_DTA_TOKENID_BYTESTRING) {
+ token = &resp->toks[n];
+ if (token->type != OPAL_DTA_TOKENID_BYTESTRING) {
pr_debug("Token is not a byte string!\n");
return 0;
}
- *store = resp->toks[n].pos + 1;
- return resp->toks[n].len - 1;
+ switch (token->width) {
+ case OPAL_WIDTH_TINY:
+ case OPAL_WIDTH_SHORT:
+ skip = 1;
+ break;
+ case OPAL_WIDTH_MEDIUM:
+ skip = 2;
+ break;
+ case OPAL_WIDTH_LONG:
+ skip = 4;
+ break;
+ default:
+ pr_debug("Token has invalid width!\n");
+ return 0;
+ }
+
+ *store = token->pos + skip;
+ return token->len - skip;
}
static u64 response_get_u64(const struct parsed_resp *resp, int n)
--
2.13.6
[PATCH][RESEND] block: sed-opal: fix u64 short atom length
The length must be given as bytes and not as 4 bit tuples.
Signed-off-by: Jonas Rabenstein
---
block/sed-opal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 36842bfa572e..d5f565e1557a 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -562,7 +562,7 @@ static void add_token_u64(int *err, struct opal_dev *cmd,
u64 number)
}
msb = fls(number);
- len = DIV_ROUND_UP(msb, 4);
+ len = DIV_ROUND_UP(msb, 8);
if (cmd->pos >= IO_BUFFER_LENGTH - len - 1) {
pr_debug("Error adding u64: end of buffer.\n");
--
2.13.6
[PATCH] block: sed-opal: fix response string extraction
Tokens are prefixed by a variable length of bytes. If a bytestring is
not stored in an tiny or short atom, we have to skip more than one byte
in order to have the actual bytes not prefixed by the bytes describing
the actual length of the string.
Signed-off-by: Jonas Rabenstein
---
block/sed-opal.c | 26 +++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 525506bed399..33052d0111de 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -876,6 +876,9 @@ static int response_parse(const u8 *buf, size_t length,
static size_t response_get_string(const struct parsed_resp *resp, int n,
const char **store)
{
+ u8 skip;
+ const struct opal_resp_tok *token;
+
*store = NULL;
if (!resp) {
pr_debug("Response is NULL\n");
@@ -888,13 +891,30 @@ static size_t response_get_string(const struct
parsed_resp *resp, int n,
return 0;
}
- if (resp->toks[n].type != OPAL_DTA_TOKENID_BYTESTRING) {
+ token = &resp->toks[n];
+ if (token->type != OPAL_DTA_TOKENID_BYTESTRING) {
pr_debug("Token is not a byte string!\n");
return 0;
}
- *store = resp->toks[n].pos + 1;
- return resp->toks[n].len - 1;
+ switch (token->width) {
+ case OPAL_WIDTH_TINY:
+ case OPAL_WIDTH_SHORT:
+ skip = 1;
+ break;
+ case OPAL_WIDTH_MEDIUM:
+ skip = 2;
+ break;
+ case OPAL_WIDTH_LONG:
+ skip = 4;
+ break;
+ default:
+ pr_debug("Token has invalid width!\n");
+ return 0;
+ }
+
+ *store = token->pos + skip;
+ return token->len - skip;
}
static u64 response_get_u64(const struct parsed_resp *resp, int n)
--
2.13.6
[PATCH] block: sed-opal: fix u64 short atom length
The length must be given as bytes and not as 4 bit tuples.
Signed-off-by: Jonas Rabenstein
---
block/sed-opal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 36842bfa572e..d5f565e1557a 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -562,7 +562,7 @@ static void add_token_u64(int *err, struct opal_dev *cmd,
u64 number)
}
msb = fls(number);
- len = DIV_ROUND_UP(msb, 4);
+ len = DIV_ROUND_UP(msb, 8);
if (cmd->pos >= IO_BUFFER_LENGTH - len - 1) {
pr_debug("Error adding u64: end of buffer.\n");
--
2.13.6
[PATCH 1/3] ia64: remove unused asm/segment.h
From: Jonas Rabenstein The file arch/ia64/include/asm/segment.h contains only a comment that it is still in the source tree as it may be referenced from some files. Actually only 2 files that are not in an arch/* directory include . The first is drivers/lguest/x86/switcher_32.S and obviously is not used by ia64. The second is include/asm-generic/uacess.h and also not used by ia64. Signed-off-by: Jonas Rabenstein --- arch/ia64/include/asm/segment.h | 6 -- 1 file changed, 6 deletions(-) delete mode 100644 arch/ia64/include/asm/segment.h diff --git a/arch/ia64/include/asm/segment.h b/arch/ia64/include/asm/segment.h deleted file mode 100644 index b89e2b3..000 --- a/arch/ia64/include/asm/segment.h +++ /dev/null @@ -1,6 +0,0 @@ -#ifndef _ASM_IA64_SEGMENT_H -#define _ASM_IA64_SEGMENT_H - -/* Only here because we have some old header files that expect it.. */ - -#endif /* _ASM_IA64_SEGMENT_H */ -- 1.9.1
[PATCH 3/3] ia64: remove unused asm/mc146818rtc.h
From: Jonas Rabenstein
The file arch/ia64/include/asm/mc146818rtc.h is not used anymore. It
would be included by include/linuxmc146818rtc.h which in fact is
included by the following files:
* sound/core/rtctimer.c:
depends on SND_RTCTIMER -> RTC -> !IA64
* drivers/char/nvram.c:
depends on NVRAM -> X86
* drivers/char/rtc.c:
depends on RTC -> !IA64
* drivers/block/floppy.c:
depends on BLK_DEV_FD -> ARCH_MAY_HAVE_PC_FDC (not defined for IA64)
* drivers/firmware/dcdbas.c:
depends on DCDBAS -> X86
* include/asm/generic/rtc.h:
- drivers/rtc/rtc-cmos.c
depends on RTC_DRV_CMOS -> (X86||ARM||M32R||PPC||MIPS||SPARC64)
- drivers/acpi/acpi/acpi_cmos_rtc.c:
depends on X86
- drivers/rtc/rtc/mrst.c:
depends on RTC_DRV_VRTC -> X86_INTEL_MID (not defined for IA64)
* drivers/char/mwave/smapi.c
depends on MWAVE -> X86
* drivers/input/misc/wistron_btns.c:
depends on CONFIG_WISTRON_BTNS -> X86_32
* drivers/scsi/gdth.c
depends on SCSI_GDTH -> ISA_DMA_API (not defined for IA64)
Fixes: 7f30491ccd28 ("[IA64] Move include/asm-ia64 to arch/ia64/include/asm")
Signed-off-by: Jonas Rabenstein
---
arch/ia64/include/asm/mc146818rtc.h | 10 --
1 file changed, 10 deletions(-)
delete mode 100644 arch/ia64/include/asm/mc146818rtc.h
diff --git a/arch/ia64/include/asm/mc146818rtc.h
b/arch/ia64/include/asm/mc146818rtc.h
deleted file mode 100644
index 407787a2..000
--- a/arch/ia64/include/asm/mc146818rtc.h
+++ /dev/null
@@ -1,10 +0,0 @@
-#ifndef _ASM_IA64_MC146818RTC_H
-#define _ASM_IA64_MC146818RTC_H
-
-/*
- * Machine dependent access functions for RTC registers.
- */
-
-/* empty include file to satisfy the include in genrtc.c */
-
-#endif /* _ASM_IA64_MC146818RTC_H */
--
1.9.1
[PATCH 0/3] ia64: remove unused header files
From: Jonas Rabenstein This patchset removes three unreferenced and unused header files of the ia64 architecture. Also some #include directives would resolve to the files I propose for removal, the dependencies defined by kconfig for those files do not allow their usage together with ia64. I detected the issue with chimaera, a tool I currently develop for my bachelor thesis extending the undertaker tool suite to take the relation of files based on #include directives into account (https://undertaker.cs.fau.de). Jonas Rabenstein (3): ia64: remove unused asm/segment.h ia64: remove unused asm/idle.h ia64: remove unused asm/mc146818rtc.h arch/ia64/include/asm/idle.h| 7 --- arch/ia64/include/asm/mc146818rtc.h | 10 -- arch/ia64/include/asm/segment.h | 6 -- 3 files changed, 23 deletions(-) delete mode 100644 arch/ia64/include/asm/idle.h delete mode 100644 arch/ia64/include/asm/mc146818rtc.h delete mode 100644 arch/ia64/include/asm/segment.h -- 1.9.1
[PATCH 2/3] ia64: remove unused asm/idle.h
From: Jonas Rabenstein
The file arch/ia64/include/asm/idle.h can not be included by any file
and as a consequence may be removed from the source tree.
Currently five files have #includes for asm/idle.h:
drivers/cpufreq/loongson2_cpufreq.c:
depends on LOONGSON2 (mips)
drivers/cpuidle/cpuidle-cps.c:
depends on MIPS_CPS_CPUIDLE (mips)
drivers/idle/i7300_idle.c:
depends on X86_64
drivers/thermal/intel_powerclamp.c:
depends on X86
drivers/xen/events/events_base.c:
depends on XEN (arm/x86)
Signed-off-by: Jonas Rabenstein
---
arch/ia64/include/asm/idle.h | 7 ---
1 file changed, 7 deletions(-)
delete mode 100644 arch/ia64/include/asm/idle.h
diff --git a/arch/ia64/include/asm/idle.h b/arch/ia64/include/asm/idle.h
deleted file mode 100644
index b768501..000
--- a/arch/ia64/include/asm/idle.h
+++ /dev/null
@@ -1,7 +0,0 @@
-#ifndef _ASM_IA64_IDLE_H
-#define _ASM_IA64_IDLE_H
-
-static inline void enter_idle(void) { }
-static inline void exit_idle(void) { }
-
-#endif /* _ASM_IA64_IDLE_H */
--
1.9.1
[PATCH v3] arm: omap2: remove redundant multiplatform checks
From: Jonas Rabenstein The directory arch/arm/mach-omap2 is only selected for compilation if CONFIG_ARCH_OMAP2PLUS is selected. CONFIG_ARCH_OMAP2PLUS itself is a silent option and all machines selecting this option are multiplatform devices. As a consequence checks for CONFIG_ARCH_MULTIPLATFORM as well as CONFIG_ARCH_OMAP2PLUS within that directory are superfluous and can be removed. Signed-off-by: Jonas Rabenstein --- change in v3: * fix build error with unselected ARCH_OMAP2 and ARCH_OMAP3 arch/arm/mach-omap2/Makefile | 2 +- arch/arm/mach-omap2/soc.h| 140 +++ 2 files changed, 22 insertions(+), 120 deletions(-) diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile index 0ba6a0e..04e276c 100644 --- a/arch/arm/mach-omap2/Makefile +++ b/arch/arm/mach-omap2/Makefile @@ -2,7 +2,7 @@ # Makefile for the linux kernel. # -ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/$(src)/include \ +ccflags-y := -I$(srctree)/$(src)/include \ -I$(srctree)/arch/arm/plat-omap/include # Common support diff --git a/arch/arm/mach-omap2/soc.h b/arch/arm/mach-omap2/soc.h index 364418c..2aa01c2 100644 --- a/arch/arm/mach-omap2/soc.h +++ b/arch/arm/mach-omap2/soc.h @@ -39,82 +39,10 @@ #include /* - * Test if multicore OMAP support is needed + * OMAP2+ is always defined as ARCH_MULTIPLATFORM in Kconfig */ #undef MULTI_OMAP2 -#undef OMAP_NAME - -#ifdef CONFIG_ARCH_MULTIPLATFORM #define MULTI_OMAP2 -#endif -#ifdef CONFIG_SOC_OMAP2420 -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME omap2420 -# endif -#endif -#ifdef CONFIG_SOC_OMAP2430 -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME omap2430 -# endif -#endif -#ifdef CONFIG_ARCH_OMAP3 -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME omap3 -# endif -#endif -#ifdef CONFIG_ARCH_OMAP4 -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME omap4 -# endif -#endif - -#ifdef CONFIG_SOC_OMAP5 -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME omap5 -# endif -#endif - -#ifdef CONFIG_SOC_AM33XX -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME am33xx -# endif -#endif - -#ifdef CONFIG_SOC_AM43XX -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME am43xx -# endif -#endif - -#ifdef CONFIG_SOC_DRA7XX -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME DRA7XX -# endif -#endif /* * Omap device type i.e. EMU/HS/TST/GP/BAD @@ -242,11 +170,6 @@ IS_AM_SUBCLASS(437x, 0x437) IS_DRA_SUBCLASS(75x, 0x75) IS_DRA_SUBCLASS(72x, 0x72) -#define soc_is_omap24xx() 0 -#define soc_is_omap242x() 0 -#define soc_is_omap243x() 0 -#define soc_is_omap34xx() 0 -#define soc_is_omap343x() 0 #define soc_is_ti81xx()0 #define soc_is_ti816x()0 #define soc_is_ti814x()0 @@ -265,46 +188,27 @@ IS_DRA_SUBCLASS(72x, 0x72) #define soc_is_dra74x()0 #define soc_is_dra72x()0 -#if defined(MULTI_OMAP2) -# if defined(CONFIG_ARCH_OMAP2) -# undef soc_is_omap24xx -# define soc_is_omap24xx()is_omap24xx() -# endif -# if defined (CONFIG_SOC_OMAP2420) -# undef soc_is_omap242x -# define soc_is_omap242x()is_omap242x() -# endif -# if defined (CONFIG_SOC_OMAP2430) -# undef soc_is_omap243x -# define soc_is_omap243x()is_omap243x() -# endif -# if defined(CONFIG_ARCH_OMAP3) -# undef soc_is_omap34xx -# undef soc_is_omap343x -# define soc_is_omap34xx()is_omap34xx() -# define soc_is_omap343x()is_omap343x() -# endif +#if defined(CONFIG_ARCH_OMAP2) +# define soc_is_omap24xx() is_omap24xx() #else -# if defined(CONFIG_ARCH_OMAP2) -# undef soc_is_omap24xx -# define soc_is_omap24xx()1 -# endif -# if defined(CONFIG_SOC_OMAP2420) -# undef soc_is_omap242x -# define soc_is_omap242x()1 -# endif -# if defined(CONFIG_SOC_OMAP2430) -# undef soc_is_omap243x -# define soc_is_omap243x()1 -# endif -# if defined(CONFIG_ARCH_OMAP3) -# undef soc_is_omap34xx -# define soc_is_omap34xx()1 -# endif -# if defined(CONFIG_SOC_OMAP3430) -# undef soc_is_omap343x -# define soc_is_omap343x()1 -# endif +# define soc_is_omap24xx() 0 +#endif +#if defined(CONFIG_SOC_OMAP2420) +# define soc_is_omap242x() is_omap242x() +#else +# define soc_is_omap242x() 0 +#endif +#if defined(CONFIG_SOC_OMAP2430) +# define soc_is_omap243x() is_omap243x() +#else +# define soc_is_omap243x() 0 +#endif +#if defined(CONFIG_ARCH_OMAP3) +# define soc_is_omap34xx
[PATCH v2] arm: omap2: remove redundant multiplatform checks
The directory arch/arm/mach-omap2 is only selected for compilation if CONFIG_ARCH_OMAP2PLUS is selected. CONFIG_ARCH_OMAP2PLUS itself is a silent option and all machines selecting this option are multiplatform devices. As a consequence checks for CONFIG_ARCH_MULTIPLATFORM as well as CONFIG_ARCH_OMAP2PLUS within that directory are superfluous and can be removed. Signed-off-by: Jonas Rabenstein --- changes in v2: * missed two #endif's arch/arm/mach-omap2/Makefile | 2 +- arch/arm/mach-omap2/soc.h| 121 +++ 2 files changed, 8 insertions(+), 115 deletions(-) diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile index 0ba6a0e..04e276c 100644 --- a/arch/arm/mach-omap2/Makefile +++ b/arch/arm/mach-omap2/Makefile @@ -2,7 +2,7 @@ # Makefile for the linux kernel. # -ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/$(src)/include \ +ccflags-y := -I$(srctree)/$(src)/include \ -I$(srctree)/arch/arm/plat-omap/include # Common support diff --git a/arch/arm/mach-omap2/soc.h b/arch/arm/mach-omap2/soc.h index 364418c..79f1573 100644 --- a/arch/arm/mach-omap2/soc.h +++ b/arch/arm/mach-omap2/soc.h @@ -42,79 +42,7 @@ * Test if multicore OMAP support is needed */ #undef MULTI_OMAP2 -#undef OMAP_NAME - -#ifdef CONFIG_ARCH_MULTIPLATFORM #define MULTI_OMAP2 -#endif -#ifdef CONFIG_SOC_OMAP2420 -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME omap2420 -# endif -#endif -#ifdef CONFIG_SOC_OMAP2430 -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME omap2430 -# endif -#endif -#ifdef CONFIG_ARCH_OMAP3 -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME omap3 -# endif -#endif -#ifdef CONFIG_ARCH_OMAP4 -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME omap4 -# endif -#endif - -#ifdef CONFIG_SOC_OMAP5 -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME omap5 -# endif -#endif - -#ifdef CONFIG_SOC_AM33XX -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME am33xx -# endif -#endif - -#ifdef CONFIG_SOC_AM43XX -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME am43xx -# endif -#endif - -#ifdef CONFIG_SOC_DRA7XX -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME DRA7XX -# endif -#endif /* * Omap device type i.e. EMU/HS/TST/GP/BAD @@ -242,11 +170,6 @@ IS_AM_SUBCLASS(437x, 0x437) IS_DRA_SUBCLASS(75x, 0x75) IS_DRA_SUBCLASS(72x, 0x72) -#define soc_is_omap24xx() 0 -#define soc_is_omap242x() 0 -#define soc_is_omap243x() 0 -#define soc_is_omap34xx() 0 -#define soc_is_omap343x() 0 #define soc_is_ti81xx()0 #define soc_is_ti816x()0 #define soc_is_ti814x()0 @@ -265,46 +188,18 @@ IS_DRA_SUBCLASS(72x, 0x72) #define soc_is_dra74x()0 #define soc_is_dra72x()0 -#if defined(MULTI_OMAP2) -# if defined(CONFIG_ARCH_OMAP2) -# undef soc_is_omap24xx +#if defined(CONFIG_ARCH_OMAP2) # define soc_is_omap24xx()is_omap24xx() -# endif -# if defined (CONFIG_SOC_OMAP2420) -# undef soc_is_omap242x +#endif +#if defined(CONFIG_SOC_OMAP2420) # define soc_is_omap242x()is_omap242x() -# endif -# if defined (CONFIG_SOC_OMAP2430) -# undef soc_is_omap243x +#endif +#if defined(CONFIG_SOC_OMAP2430) # define soc_is_omap243x()is_omap243x() -# endif -# if defined(CONFIG_ARCH_OMAP3) -# undef soc_is_omap34xx -# undef soc_is_omap343x +#endif +#if defined(CONFIG_ARCH_OMAP3) # define soc_is_omap34xx()is_omap34xx() # define soc_is_omap343x()is_omap343x() -# endif -#else -# if defined(CONFIG_ARCH_OMAP2) -# undef soc_is_omap24xx -# define soc_is_omap24xx()1 -# endif -# if defined(CONFIG_SOC_OMAP2420) -# undef soc_is_omap242x -# define soc_is_omap242x()1 -# endif -# if defined(CONFIG_SOC_OMAP2430) -# undef soc_is_omap243x -# define soc_is_omap243x()1 -# endif -# if defined(CONFIG_ARCH_OMAP3) -# undef soc_is_omap34xx -# define soc_is_omap34xx()1 -# endif -# if defined(CONFIG_SOC_OMAP3430) -# undef soc_is_omap343x -# define soc_is_omap343x()1 -# endif #endif /* @@ -339,7 +234,6 @@ IS_OMAP_TYPE(3430, 0x3430) #define soc_is_omap5430() 0 /* These are needed for the common code */ -#ifdef CONFIG_ARCH_OMAP2PLUS #define soc_is_omap7xx() 0 #define soc_is_omap15xx() 0 #define soc_is_omap16xx() 0 @@ -350,7 +244,6 @@ IS_OMAP_TYPE(3430, 0x3430) #define soc_is_omap1710() 0 #define cpu_class_is_omap1() 0 #define cpu_class_is_omap2()
[PATCH] arm: omap2: remove redundant multiplatform checks
The directory arch/arm/mach-omap2 is only selected for compilation if CONFIG_ARCH_OMAP2PLUS is selected. CONFIG_ARCH_OMAP2PLUS itself is a silent option and all machines selecting this option are multiplatform devices. As a consequence checks for CONFIG_ARCH_MULTIPLATFORM as well as CONFIG_ARCH_OMAP2PLUS within that directory are superfluous and can be removed. Signed-off-by: Jonas Rabenstein --- arch/arm/mach-omap2/Makefile | 2 +- arch/arm/mach-omap2/soc.h| 119 ++- 2 files changed, 6 insertions(+), 115 deletions(-) diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile index 0ba6a0e..04e276c 100644 --- a/arch/arm/mach-omap2/Makefile +++ b/arch/arm/mach-omap2/Makefile @@ -2,7 +2,7 @@ # Makefile for the linux kernel. # -ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/$(src)/include \ +ccflags-y := -I$(srctree)/$(src)/include \ -I$(srctree)/arch/arm/plat-omap/include # Common support diff --git a/arch/arm/mach-omap2/soc.h b/arch/arm/mach-omap2/soc.h index 364418c..f1418d4 100644 --- a/arch/arm/mach-omap2/soc.h +++ b/arch/arm/mach-omap2/soc.h @@ -42,79 +42,7 @@ * Test if multicore OMAP support is needed */ #undef MULTI_OMAP2 -#undef OMAP_NAME - -#ifdef CONFIG_ARCH_MULTIPLATFORM #define MULTI_OMAP2 -#endif -#ifdef CONFIG_SOC_OMAP2420 -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME omap2420 -# endif -#endif -#ifdef CONFIG_SOC_OMAP2430 -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME omap2430 -# endif -#endif -#ifdef CONFIG_ARCH_OMAP3 -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME omap3 -# endif -#endif -#ifdef CONFIG_ARCH_OMAP4 -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME omap4 -# endif -#endif - -#ifdef CONFIG_SOC_OMAP5 -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME omap5 -# endif -#endif - -#ifdef CONFIG_SOC_AM33XX -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME am33xx -# endif -#endif - -#ifdef CONFIG_SOC_AM43XX -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME am43xx -# endif -#endif - -#ifdef CONFIG_SOC_DRA7XX -# ifdef OMAP_NAME -# undef MULTI_OMAP2 -# define MULTI_OMAP2 -# else -# define OMAP_NAME DRA7XX -# endif -#endif /* * Omap device type i.e. EMU/HS/TST/GP/BAD @@ -242,11 +170,6 @@ IS_AM_SUBCLASS(437x, 0x437) IS_DRA_SUBCLASS(75x, 0x75) IS_DRA_SUBCLASS(72x, 0x72) -#define soc_is_omap24xx() 0 -#define soc_is_omap242x() 0 -#define soc_is_omap243x() 0 -#define soc_is_omap34xx() 0 -#define soc_is_omap343x() 0 #define soc_is_ti81xx()0 #define soc_is_ti816x()0 #define soc_is_ti814x()0 @@ -265,46 +188,16 @@ IS_DRA_SUBCLASS(72x, 0x72) #define soc_is_dra74x()0 #define soc_is_dra72x()0 -#if defined(MULTI_OMAP2) -# if defined(CONFIG_ARCH_OMAP2) -# undef soc_is_omap24xx +#if defined(CONFIG_ARCH_OMAP2) # define soc_is_omap24xx()is_omap24xx() -# endif -# if defined (CONFIG_SOC_OMAP2420) -# undef soc_is_omap242x +#endif +#if defined(CONFIG_SOC_OMAP2420) # define soc_is_omap242x()is_omap242x() -# endif -# if defined (CONFIG_SOC_OMAP2430) -# undef soc_is_omap243x +#if defined(CONFIG_SOC_OMAP2430) # define soc_is_omap243x()is_omap243x() -# endif -# if defined(CONFIG_ARCH_OMAP3) -# undef soc_is_omap34xx -# undef soc_is_omap343x +#if defined(CONFIG_ARCH_OMAP3) # define soc_is_omap34xx()is_omap34xx() # define soc_is_omap343x()is_omap343x() -# endif -#else -# if defined(CONFIG_ARCH_OMAP2) -# undef soc_is_omap24xx -# define soc_is_omap24xx()1 -# endif -# if defined(CONFIG_SOC_OMAP2420) -# undef soc_is_omap242x -# define soc_is_omap242x()1 -# endif -# if defined(CONFIG_SOC_OMAP2430) -# undef soc_is_omap243x -# define soc_is_omap243x()1 -# endif -# if defined(CONFIG_ARCH_OMAP3) -# undef soc_is_omap34xx -# define soc_is_omap34xx()1 -# endif -# if defined(CONFIG_SOC_OMAP3430) -# undef soc_is_omap343x -# define soc_is_omap343x()1 -# endif #endif /* @@ -339,7 +232,6 @@ IS_OMAP_TYPE(3430, 0x3430) #define soc_is_omap5430() 0 /* These are needed for the common code */ -#ifdef CONFIG_ARCH_OMAP2PLUS #define soc_is_omap7xx() 0 #define soc_is_omap15xx() 0 #define soc_is_omap16xx() 0 @@ -350,7 +242,6 @@ IS_OMAP_TYPE(3430, 0x3430) #define soc_is_omap1710() 0 #define cpu_class_is_omap1() 0 #define cpu_class_is_omap2() 1 -#endif #if defined(CONFIG_ARCH_OMAP2) # undef
Re: [PATCH] arm: remove tautologic #ifdefs in proc-v7-3level.S
On Tue, Mar 29, 2016 at 10:05:58AM +0200, Arnd Bergmann wrote:
> On Tuesday 29 March 2016 09:37:51 Jonas Rabenstein wrote:
> > The file arch/arm/mm/proc-v7-3level.S is only used by the #include
> > directive in arch/arm/mm/proc-v7.S:23. This #include is conditional and
> > depends on CONFIG_ARM_LPAE (otherwise proc-v7-2level.S is used).
> > CONFIG_ARM_LPAE has a dependency on CONFIG_MMU defined in Kconfig.
> > Consequently, checks for CONFIG_MMU in proc-v7-3level.S are superfluous.
> >
> > Signed-off-by: Jonas Rabenstein
> > ---
> > I detected the issue with chimaera, a tool I currently develop for my
> > bachelor
> > thesis extending the undertaker tool suite (https://undertaker.cs.fau.de).
>
> Nice catch!
>
> Reviewed-by: Arnd Bergmann
>
> I guess you should submit the same thing for the other file as well,
> either in the same patch or as a series.
I do not get, what you mean with 'the other file'? For the
proc-v7-2level.S this precondition does not hold, as proc-v7-2level.S is
included if !CONFIG_ARM_LPAE. Consequently, no evidence about the MMU
state is available in their.
> You can also add
>
> Fixes: 8d2cd3a38fd6 ("ARM: LPAE: Factor out classic-MMU specific code into
> proc-v7-2level.S")
Shouldn't it be:
Fixes: 1b6ba46b7efa ("ARM: LPAE: MMU setup for the 3-level page table
format")?
Thanks for your help and advice,
Jonas
[PATCH] arm: remove tautologic #ifdefs in proc-v7-3level.S
The file arch/arm/mm/proc-v7-3level.S is only used by the #include directive in arch/arm/mm/proc-v7.S:23. This #include is conditional and depends on CONFIG_ARM_LPAE (otherwise proc-v7-2level.S is used). CONFIG_ARM_LPAE has a dependency on CONFIG_MMU defined in Kconfig. Consequently, checks for CONFIG_MMU in proc-v7-3level.S are superfluous. Signed-off-by: Jonas Rabenstein --- I detected the issue with chimaera, a tool I currently develop for my bachelor thesis extending the undertaker tool suite (https://undertaker.cs.fau.de). arch/arm/mm/proc-v7-3level.S | 4 1 file changed, 4 deletions(-) diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S index 5e5720e..6903f34 100644 --- a/arch/arm/mm/proc-v7-3level.S +++ b/arch/arm/mm/proc-v7-3level.S @@ -55,13 +55,11 @@ * the new TTB). */ ENTRY(cpu_v7_switch_mm) -#ifdef CONFIG_MMU mmidr2, r2 asidr2, r2 orr rpgdh, rpgdh, r2, lsl #(48 - 32)@ upper 32-bits of pgd mcrrp15, 0, rpgdl, rpgdh, c2@ set TTB 0 isb -#endif ret lr ENDPROC(cpu_v7_switch_mm) @@ -81,7 +79,6 @@ ENDPROC(cpu_v7_switch_mm) * - pte - PTE value to store (64-bit in r2 and r3) */ ENTRY(cpu_v7_set_pte_ext) -#ifdef CONFIG_MMU tst rl, #L_PTE_VALID beq 1f tst rh, #1 << (57 - 32) @ L_PTE_NONE @@ -97,7 +94,6 @@ ENTRY(cpu_v7_set_pte_ext) 1: strdr2, r3, [r0] ALT_SMP(W(nop)) ALT_UP (mcr p15, 0, r0, c7, c10, 1) @ flush_pte -#endif ret lr ENDPROC(cpu_v7_set_pte_ext) -- 2.7.3
[PATCH v2] arm: remove dead blocks in XIP_KERNEL link script
Commit 538bf4694898 ("ARM: 8513/1: xip: Move XIP linking to a separate
file") separated all CONFIG_XIP_KERNEL related portions of the linker
script into a separate file. The copied parts depending on CONFIG_SMP
may be removed, as CONFIG_SMP (and related options) can never be
selected together with CONFIG_XIP_KERNEL. CONFIG_SMP_ON_UP has a direct
dependency on !CONFIG_XIP_KERNEL. CONFIG_XIP_KERNEL itself depends on
!ARCH_MULTIPLATFORM, but all devices specifying HAVE_SMP are also some
kind of multiplatform devices. Finally, HAVE_TCM is also only selected
by multiplatform devices and may not selected for the same reasons.
Signed-off-by: Jonas Rabenstein
---
Changes in v2:
- do not remove CONFIG_CPU_HOTPLUG related parts
- send a patch with one missing item in the first place (line 36)
arch/arm/kernel/vmlinux-xip.lds.S | 73 +--
1 file changed, 1 insertion(+), 72 deletions(-)
diff --git a/arch/arm/kernel/vmlinux-xip.lds.S
b/arch/arm/kernel/vmlinux-xip.lds.S
index cba1ec8..74e03cc 100644
--- a/arch/arm/kernel/vmlinux-xip.lds.S
+++ b/arch/arm/kernel/vmlinux-xip.lds.S
@@ -33,8 +33,7 @@
#define ARM_CPU_KEEP(x)
#endif
-#if (defined(CONFIG_SMP_ON_UP) && !defined(CONFIG_DEBUG_SPINLOCK)) || \
- defined(CONFIG_GENERIC_BUG)
+#ifdef CONFIG_GENERIC_BUG
#define ARM_EXIT_KEEP(x) x
#define ARM_EXIT_DISCARD(x)
#else
@@ -74,9 +73,7 @@ SECTIONS
*(.text.fixup)
*(__ex_table)
#endif
-#ifndef CONFIG_SMP_ON_UP
*(.alt.smp.init)
-#endif
*(.discard)
*(.discard.*)
}
@@ -177,13 +174,6 @@ SECTIONS
*(.taglist.init)
__tagtable_end = .;
}
-#ifdef CONFIG_SMP_ON_UP
- .init.smpalt : {
- __smpalt_begin = .;
- *(.alt.smp.init)
- __smpalt_end = .;
- }
-#endif
.init.pv_table : {
__pv_table_begin = .;
*(.pv_table)
@@ -197,10 +187,6 @@ SECTIONS
INIT_RAM_FS
}
-#ifdef CONFIG_SMP
- PERCPU_SECTION(L1_CACHE_BYTES)
-#endif
-
_exiprom = .; /* End of XIP ROM area */
__data_loc = ALIGN(4); /* location in binary */
. = PAGE_OFFSET + TEXT_OFFSET;
@@ -236,63 +222,6 @@ SECTIONS
}
_edata_loc = __data_loc + SIZEOF(.data);
-#ifdef CONFIG_HAVE_TCM
-/*
-* We align everything to a page boundary so we can
-* free it after init has commenced and TCM contents have
-* been copied to its destination.
-*/
- .tcm_start : {
- . = ALIGN(PAGE_SIZE);
- __tcm_start = .;
- __itcm_start = .;
- }
-
- /*
-* Link these to the ITCM RAM
-* Put VMA to the TCM address and LMA to the common RAM
-* and we'll upload the contents from RAM to TCM and free
-* the used RAM after that.
-*/
- .text_itcm ITCM_OFFSET : AT(__itcm_start)
- {
- __sitcm_text = .;
- *(.tcm.text)
- *(.tcm.rodata)
- . = ALIGN(4);
- __eitcm_text = .;
- }
-
- /*
-* Reset the dot pointer, this is needed to create the
-* relative __dtcm_start below (to be used as extern in code).
-*/
- . = ADDR(.tcm_start) + SIZEOF(.tcm_start) + SIZEOF(.text_itcm);
-
- .dtcm_start : {
- __dtcm_start = .;
- }
-
- /* TODO: add remainder of ITCM as well, that can be used for data! */
- .data_dtcm DTCM_OFFSET : AT(__dtcm_start)
- {
- . = ALIGN(4);
- __sdtcm_data = .;
- *(.tcm.data)
- . = ALIGN(4);
- __edtcm_data = .;
- }
-
- /* Reset the dot pointer or the linker gets confused */
- . = ADDR(.dtcm_start) + SIZEOF(.data_dtcm);
-
- /* End marker for freeing TCM copy in linked object */
- .tcm_end : AT(ADDR(.dtcm_start) + SIZEOF(.data_dtcm)){
- . = ALIGN(PAGE_SIZE);
- __tcm_end = .;
- }
-#endif
-
BSS_SECTION(0, 0, 0)
_end = .;
--
2.7.3
[PATCH] arm: remove dead blocks in XIP_KERNEL link script
Commit 538bf4694898 ("ARM: 8513/1: xip: Move XIP linking to a separate
file") separated all CONFIG_XIP_KERNEL related portions of the linker
script into a separate file. The copied parts depending on CONFIG_SMP
may be removed, as CONFIG_SMP (and related options) can never be
selected together with CONFIG_XIP_KERNEL. CONFIG_SMP_ON_UP has a direct
dependency on !CONFIG_XIP_KERNEL. CONFIG_XIP_KERNEL itself depends on
!ARCH_MULTIPLATFORM, but all devices specifying HAVE_SMP are also some
kind of multiplatform devices. Finally, HAVE_TCM is also only selected
by multiplatform devices and may not selected for the same reasons.
Signed-off-by: Jonas Rabenstein
---
I detected the issue with chimaera, a tool I currently develop for my bachelor
thesis extending the undertaker tool suite (https://undertaker.cs.fau.de).
arch/arm/kernel/vmlinux-xip.lds.S | 75 ---
1 file changed, 75 deletions(-)
diff --git a/arch/arm/kernel/vmlinux-xip.lds.S
b/arch/arm/kernel/vmlinux-xip.lds.S
index cba1ec8..7da95f8 100644
--- a/arch/arm/kernel/vmlinux-xip.lds.S
+++ b/arch/arm/kernel/vmlinux-xip.lds.S
@@ -25,13 +25,8 @@
*(.hyp.idmap.text) \
VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;
-#ifdef CONFIG_HOTPLUG_CPU
-#define ARM_CPU_DISCARD(x)
-#define ARM_CPU_KEEP(x)x
-#else
#define ARM_CPU_DISCARD(x) x
#define ARM_CPU_KEEP(x)
-#endif
#if (defined(CONFIG_SMP_ON_UP) && !defined(CONFIG_DEBUG_SPINLOCK)) || \
defined(CONFIG_GENERIC_BUG)
@@ -74,9 +69,7 @@ SECTIONS
*(.text.fixup)
*(__ex_table)
#endif
-#ifndef CONFIG_SMP_ON_UP
*(.alt.smp.init)
-#endif
*(.discard)
*(.discard.*)
}
@@ -177,13 +170,6 @@ SECTIONS
*(.taglist.init)
__tagtable_end = .;
}
-#ifdef CONFIG_SMP_ON_UP
- .init.smpalt : {
- __smpalt_begin = .;
- *(.alt.smp.init)
- __smpalt_end = .;
- }
-#endif
.init.pv_table : {
__pv_table_begin = .;
*(.pv_table)
@@ -197,10 +183,6 @@ SECTIONS
INIT_RAM_FS
}
-#ifdef CONFIG_SMP
- PERCPU_SECTION(L1_CACHE_BYTES)
-#endif
-
_exiprom = .; /* End of XIP ROM area */
__data_loc = ALIGN(4); /* location in binary */
. = PAGE_OFFSET + TEXT_OFFSET;
@@ -236,63 +218,6 @@ SECTIONS
}
_edata_loc = __data_loc + SIZEOF(.data);
-#ifdef CONFIG_HAVE_TCM
-/*
-* We align everything to a page boundary so we can
-* free it after init has commenced and TCM contents have
-* been copied to its destination.
-*/
- .tcm_start : {
- . = ALIGN(PAGE_SIZE);
- __tcm_start = .;
- __itcm_start = .;
- }
-
- /*
-* Link these to the ITCM RAM
-* Put VMA to the TCM address and LMA to the common RAM
-* and we'll upload the contents from RAM to TCM and free
-* the used RAM after that.
-*/
- .text_itcm ITCM_OFFSET : AT(__itcm_start)
- {
- __sitcm_text = .;
- *(.tcm.text)
- *(.tcm.rodata)
- . = ALIGN(4);
- __eitcm_text = .;
- }
-
- /*
-* Reset the dot pointer, this is needed to create the
-* relative __dtcm_start below (to be used as extern in code).
-*/
- . = ADDR(.tcm_start) + SIZEOF(.tcm_start) + SIZEOF(.text_itcm);
-
- .dtcm_start : {
- __dtcm_start = .;
- }
-
- /* TODO: add remainder of ITCM as well, that can be used for data! */
- .data_dtcm DTCM_OFFSET : AT(__dtcm_start)
- {
- . = ALIGN(4);
- __sdtcm_data = .;
- *(.tcm.data)
- . = ALIGN(4);
- __edtcm_data = .;
- }
-
- /* Reset the dot pointer or the linker gets confused */
- . = ADDR(.dtcm_start) + SIZEOF(.data_dtcm);
-
- /* End marker for freeing TCM copy in linked object */
- .tcm_end : AT(ADDR(.dtcm_start) + SIZEOF(.data_dtcm)){
- . = ALIGN(PAGE_SIZE);
- __tcm_end = .;
- }
-#endif
-
BSS_SECTION(0, 0, 0)
_end = .;
--
2.7.3
[PATCH v2] Remove unselectable rule for arm-exynos-cpufreq.o
Commit "cpufreq: exynos: remove Exynos4x12 specific cpufreq driver support" deleted option ARM_EXYNOS_CPUFREQ but missed to delete a rule in drivers/cpufreq/Makefile which depends on that option. Remove unselectable rule for arm-exynos-cpufreq.o from drivers/cpufreq/Makefile. Signed-off-by: Jonas Rabenstein --- I found this inconsistency using the undertaker and undertaker-checkpatch tools (https://undertaker.cs.fau.de/). drivers/cpufreq/Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index bbc4a9f..958be2b 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -52,7 +52,6 @@ obj-$(CONFIG_ARM_DT_BL_CPUFREQ) += arm_big_little_dt.o obj-$(CONFIG_ARCH_DAVINCI) += davinci-cpufreq.o obj-$(CONFIG_UX500_SOC_DB8500) += dbx500-cpufreq.o -obj-$(CONFIG_ARM_EXYNOS_CPUFREQ) += arm-exynos-cpufreq.o obj-$(CONFIG_ARM_EXYNOS5440_CPUFREQ) += exynos5440-cpufreq.o obj-$(CONFIG_ARM_HIGHBANK_CPUFREQ) += highbank-cpufreq.o obj-$(CONFIG_ARM_HISI_ACPU_CPUFREQ)+= hisi-acpu-cpufreq.o -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Remove unselectable rule for arm-exynos-cpufreq.o
With commit 2907ed4377d9 ("cpufreq: exynos: remove Exynos4x12 specific
cpufreq driver support"), the last specific Exynos-cpufreq driver
depending on ARM_EXYNOS_CPUFREQ was removed. As there is no
Exynos-specific cpufreq driver depending on the code in
drivers/cpufreq/exynos-cpufreq.c (selected by ARM_EXYNOS_CPUFREQ),
this file was deleted and ARM_EXYNOS_CPUFREQ removed from Kconfig.
After all, the rule in drivers/cpufreq/Makefile, to combine all
specific cpufreq drivers depending on the generic code for Exynos into
one objectfile is obsolete and can be removed.
Remove unselectable rule for arm-exynos-cpufreq.o from
drivers/cpufreq/Makefile, which - if selectable - would generate an
always empty objectfile.
Signed-off-by: Jonas Rabenstein
---
I found this inconsistency using the undertaker and
undertaker-checkpatch tools (https://undertaker.cs.fau.de/).
drivers/cpufreq/Makefile | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index bbc4a9f..958be2b 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -52,7 +52,6 @@ obj-$(CONFIG_ARM_DT_BL_CPUFREQ) +=
arm_big_little_dt.o
obj-$(CONFIG_ARCH_DAVINCI) += davinci-cpufreq.o
obj-$(CONFIG_UX500_SOC_DB8500) += dbx500-cpufreq.o
-obj-$(CONFIG_ARM_EXYNOS_CPUFREQ) += arm-exynos-cpufreq.o
obj-$(CONFIG_ARM_EXYNOS5440_CPUFREQ) += exynos5440-cpufreq.o
obj-$(CONFIG_ARM_HIGHBANK_CPUFREQ) += highbank-cpufreq.o
obj-$(CONFIG_ARM_HISI_ACPU_CPUFREQ)+= hisi-acpu-cpufreq.o
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] arm64: remove dead-code depending on CONFIG_UP_LATE_INIT
Commit 4b3dc9679cf7 ("arm64: force CONFIG_SMP=y and remove redundant
#ifdefs") removed UP_LATE_INIT from Kconfig, as it had !SMP as dependency
and therfore can not be selected anymore.
Remove dead #ifdef-block depending on UP_LATE_INIT in
arch/arm64/kernel/setup.c
Signed-off-by: Jonas Rabenstein
---
I found this inconsistency using the undertaker and
undertaker-checkpatch tools (https://undertaker.cs.fau.de/).
arch/arm64/kernel/setup.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 0533f0b..d512ac8 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -213,13 +213,6 @@ void __init do_post_cpus_up_work(void)
apply_alternatives_all();
}
-#ifdef CONFIG_UP_LATE_INIT
-void __init up_late_init(void)
-{
- do_post_cpus_up_work();
-}
-#endif /* CONFIG_UP_LATE_INIT */
-
static void __init setup_processor(void)
{
u64 features;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] arm64: remove redundant object file list
Commit 4b3dc9679cf7 ("arm64: force CONFIG_SMP=y and remove redundant
#ifdefs") forces SMP on arm64. To build the necessary objects for SMP,
they were added to the arm64-obj-y rule in arch/arm64/kernel/Makefile,
without removing the arm64-obj-$(CONFIG_SMP) rule.
Remove redundant object file list depending on always-yes CONFIG_SMP in
arch/arm64/kernel/Makefile.
Signed-off-by: Jonas Rabenstein
---
I found this inconsistency using the undertaker and
undertaker-checkpatch tools (https://undertaker.cs.fau.de/).
arch/arm64/kernel/Makefile | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index f126cfe..c662197 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -25,7 +25,6 @@ arm64-obj-$(CONFIG_COMPAT)+= sys32.o kuser32.o
signal32.o \
../../arm/kernel/opcodes.o
arm64-obj-$(CONFIG_FUNCTION_TRACER)+= ftrace.o entry-ftrace.o
arm64-obj-$(CONFIG_MODULES)+= arm64ksyms.o module.o
-arm64-obj-$(CONFIG_SMP)+= smp.o smp_spin_table.o
topology.o
arm64-obj-$(CONFIG_PERF_EVENTS)+= perf_regs.o
arm64-obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o perf_callchain.o
arm64-obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
