| Message ID | 1549054223-12220-1-git-send-email-zub@linux.fjfi.cvut.cz (mailing list archive) |
|---|---|
| Headers | show |
| Series | block: sed-opal: support shadow MBR done flag and write | expand |
On Fri, 1 Feb 2019, David Kozub wrote: > This patch series extends SED 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] > > There is a fork of sed-opal-temp that can use these new IOCTLs.[2] I tested > these on Samsung 840 EVO and 850 EVO drives, on x86-64 and arm64 systems. > > The series applies on v5.0-rc4. > > I'm resending as v4 as suggested by Scott Bauer.[3] > > Changes from v3 to v4: > * added Reviewed-by from Scott, including for the patch 16/16 (details in > [3]) So this time really the 16th patch got lost somewhere - while I have received it, I could not see it in https://lore.kernel.org/lkml/. So I re-sent it now. Best regards, David
On Fri, Feb 01, 2019 at 09:50:07PM +0100, David Kozub wrote: > This patch series extends SED 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. Actually most of it is really useful cleanups and small fixes for the existing OPAL code. Any chance you could resend just those bits as a first series ASAP? I'll try to spend some time to to review the actual feature additions in the meantime. > There is a fork of sed-opal-temp that can use these new IOCTLs.[2] I tested > these on Samsung 840 EVO and 850 EVO drives, on x86-64 and arm64 systems. Which brings up another question: how do we get a properly maintained version of the sed-opal tool up ASAP? It's been a bit bitrotting unfortunately, and the documentation and error handling hasn't been all that great to start with. Are we going to find a good home for it? Util-linux for example?
On Mon, Feb 04, 2019 at 07:04:15AM -0800, Christoph Hellwig wrote: > On Fri, Feb 01, 2019 at 09:50:07PM +0100, David Kozub wrote: > > This patch series extends SED 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. > > Actually most of it is really useful cleanups and small fixes for the > existing OPAL code. Any chance you could resend just those bits as > a first series ASAP? I'll try to spend some time to to review the > actual feature additions in the meantime. > > > There is a fork of sed-opal-temp that can use these new IOCTLs.[2] I tested > > these on Samsung 840 EVO and 850 EVO drives, on x86-64 and arm64 systems. > > Which brings up another question: how do we get a properly maintained > version of the sed-opal tool up ASAP? It's been a bit bitrotting > unfortunately, and the documentation and error handling hasn't been all > that great to start with. Are we going to find a good home for it? > Util-linux for example? I would also like to get it off my github and into a proper location as well. Christoph, do you know off hand how we would get it in util-linux?
On Mon, Feb 04, 2019 at 10:36:10AM -0500, Scott Bauer wrote: > > Which brings up another question: how do we get a properly maintained > > version of the sed-opal tool up ASAP? It's been a bit bitrotting > > unfortunately, and the documentation and error handling hasn't been all > > that great to start with. Are we going to find a good home for it? > > Util-linux for example? > > I would also like to get it off my github and into a proper location as well. > > Christoph, do you know off hand how we would get it in util-linux? Maybe just ask util-linux@vger.kernel.org if they are interested?
On Mon, 4 Feb 2019, Christoph Hellwig wrote: > On Fri, Feb 01, 2019 at 09:50:07PM +0100, David Kozub wrote: >> This patch series extends SED 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. > > Actually most of it is really useful cleanups and small fixes for the > existing OPAL code. Any chance you could resend just those bits as > a first series ASAP? I'll try to spend some time to to review the > actual feature additions in the meantime. OK, I'll try to separate these into two sets. This will unfortunately trigger some changes (conflict resolving - e.g. if I move the last two patches in the current series forward, in front of the patches with new functionality). What is the proper procedure w.r.t. Reviewed-by tags which were already given? Common sense would make me keep them for trivial changes and remove them if the patch should be re-reviewed. Is that correct? >> There is a fork of sed-opal-temp that can use these new IOCTLs.[2] I tested >> these on Samsung 840 EVO and 850 EVO drives, on x86-64 and arm64 systems. > > Which brings up another question: how do we get a properly maintained > version of the sed-opal tool up ASAP? It's been a bit bitrotting > unfortunately, and the documentation and error handling hasn't been all > that great to start with. Are we going to find a good home for it? > Util-linux for example? FWIW I also hacked together a tool to cover my usecase: https://gitlab.com/zub2/opalctl The selling point is that it can handle passwords the same way that sedutil (https://github.com/Drive-Trust-Alliance/sedutil) does. Best regards, David
On Tue, Feb 05, 2019 at 12:06:54AM +0100, David Kozub wrote: > This will unfortunately trigger some changes (conflict resolving - e.g. if I > move the last two patches in the current series forward, in front of the > patches with new functionality). What is the proper procedure w.r.t. > Reviewed-by tags which were already given? Common sense would make me keep > them for trivial changes and remove them if the patch should be re-reviewed. > Is that correct? I don't think there is a general rule. As long as the change is trivial I keep them personally, otherwise I drop them. E.g. if you just remove a few hunk because of new code that isn't present now I'd keep them, if there are more complex changes to the code flow I would consider dropping them. > FWIW I also hacked together a tool to cover my usecase: > https://gitlab.com/zub2/opalctl > > The selling point is that it can handle passwords the same way that sedutil > (https://github.com/Drive-Trust-Alliance/sedutil) does. That actually is pretty nice, as it allows people to migrate over (not that I've heard of a whole lot of usage of sedutil, but being compatible to some extent is always nice)