* Re: [PATCH v2 09/16] block: sed-opal: split generation of bytestring header and content
@ 2019-01-19 16:59 Scott Bauer
0 siblings, 0 replies; 2+ messages in thread
From: Scott Bauer @ 2019-01-19 16:59 UTC (permalink / raw)
To: David Kozub; +Cc: linux-kernel, linux-block, hch, jonathan.derrick
On Thu, Jan 17, 2019 at 09:31:49PM +0000, David Kozub wrote:
> -
> - memcpy(&cmd->cmd[cmd->pos], bytestring, len);
> + start = &cmd->cmd[cmd->pos];
> cmd->pos += len;
This is somewhat pendatic, but it helps me review patches if we keep things together.
Since we're no longer doing the memcpy in this function, can we please move the cmd->pos += len
to the location where we actually do the memcpy.
I'm willing to be told to get over it if other reviewers don't like that approach, but if no one cares
please move it.
> + 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);
Do the above here instead.
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
* [PATCH v2 00/16] block: sed-opal: support shadow MBR done flag and write
@ 2019-01-17 21:31 David Kozub
2019-01-17 21:31 ` [PATCH v2 09/16] block: sed-opal: split generation of bytestring header and content David Kozub
0 siblings, 1 reply; 2+ messages in thread
From: David Kozub @ 2019-01-17 21:31 UTC (permalink / raw)
To: Jens Axboe, Jonathan Derrick, linux-block, linux-kernel
Cc: Jonas Rabenstein, David Kozub
This patch series extends OPAL support: it adds IOCTL for setting the shadow
MBR done flag which can be useful for unlocking an OPAL disk on boot and it adds
IOCTL for writing to the shadow MBR. Also included are some minor fixes and
improvements.
This series is based on the original work done by Jonas Rabenstein which was
submitted in March 2018.[1] I tried to apply suggestions made in review on the
list and do some further improvements.
The most contentious issue in the original series was the IOCTL for shadow MBR
write but I think no better approach was found[2] so this was not changed. I'm
open to suggestions.
There is a fork of sed-opal-temp that can use these new IOCTLs.[3] I tested
these on Samsung 840 EVO and 850 EVO drives, on x86-64 and arm64 systems.
The series applies on v5.0-rc2.
I'm resending this as v2 because I sent the first attempt incorrectly. I
apologize for the noise.
[1] https://lore.kernel.org/lkml/cover.1521482295.git.jonas.rabenstein@studium.uni-erlangen.de/
[2] https://lore.kernel.org/lkml/20180405203443.ye4gnw5aey2exlkn@sbauer-Z170X-UD5/
[3] https://github.com/ghostav/sed-opal-temp
David Kozub (8):
block: sed-opal: fix typos and formatting
block: sed-opal: close parameter list in cmd_finalize
block: sed-opal: unify cmd start
block: sed-opal: unify error handling of responses
block: sed-opal: reuse response_get_token to decrease code duplication
block: sed-opal: pass steps via argument rather than via opal_dev
block: sed-opal: don't repeat opal_discovery0 in each steps array
block: sed-opal: rename next to execute_steps
Jonas Rabenstein (8):
block: sed-opal: use correct macro for method length
block: sed-opal: unify space check in add_token_*
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: check size of shadow mbr
block/opal_proto.h | 18 +
block/sed-opal.c | 843 +++++++++++++++++-----------------
include/linux/sed-opal.h | 2 +
include/uapi/linux/sed-opal.h | 9 +
4 files changed, 449 insertions(+), 423 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 2+ messages in thread
* [PATCH v2 09/16] block: sed-opal: split generation of bytestring header and content
2019-01-17 21:31 [PATCH v2 00/16] block: sed-opal: support shadow MBR done flag and write David Kozub
@ 2019-01-17 21:31 ` David Kozub
0 siblings, 0 replies; 2+ messages in thread
From: David Kozub @ 2019-01-17 21:31 UTC (permalink / raw)
To: Jens Axboe, Jonathan Derrick, linux-block, linux-kernel; +Cc: Jonas Rabenstein
From: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>
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 <jonas.rabenstein@studium.uni-erlangen.de>
---
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-01-19 17:00 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-19 16:59 [PATCH v2 09/16] block: sed-opal: split generation of bytestring header and content Scott Bauer
-- strict thread matches above, loose matches on Subject: below --
2019-01-17 21:31 [PATCH v2 00/16] block: sed-opal: support shadow MBR done flag and write David Kozub
2019-01-17 21:31 ` [PATCH v2 09/16] block: sed-opal: split generation of bytestring header and content David Kozub
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).