| Message ID | 20170901145335.9244-1-scott.bauer@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Looks good Acked-by Jon Derrick: <jonathan.derrick@intel.com> On 09/01/2017 08:53 AM, Scott Bauer wrote: > Users who are booting off their Opal enabled drives are having > issues when they have a shadow MBR set up after s3/resume cycle. > When the Drive has a shadow MBR setup the MBRDone flag is set to > false upon power loss (S3/S4/S5). When the MBRDone flag is false > I/O to LBA 0 -> LBA_END_MBR are remapped to the shadow mbr > of the drive. If the drive contains useful data in the 0 -> end_mbr > range upon s3 resume the user can never get to that data as the > drive will keep remapping it to the MBR. To fix this when we unlock > on S3 resume, we need to tell the drive that we're done with the > shadow mbr (even though we didnt use it) by setting true to MBRDone. > This way the drive will stop the remapping and the user can access > their data. > > Signed-off-by: Scott Bauer <scott.bauer@intel.com> > --- > block/opal_proto.h | 1 + > block/sed-opal.c | 32 ++++++++++++++++++++++++++++++++ > 2 files changed, 33 insertions(+) > > diff --git a/block/opal_proto.h b/block/opal_proto.h > index f40c9acf8895..e20be8258854 100644 > --- a/block/opal_proto.h > +++ b/block/opal_proto.h > @@ -46,6 +46,7 @@ enum opal_response_token { > #define GENERIC_HOST_SESSION_NUM 0x41 > > #define TPER_SYNC_SUPPORTED 0x01 > +#define MBR_ENABLED_MASK 0x10 > > #define TINY_ATOM_DATA_MASK 0x3F > #define TINY_ATOM_SIGNED 0x40 > diff --git a/block/sed-opal.c b/block/sed-opal.c > index 9b30ae5ab843..9ed51d0c6b1d 100644 > --- a/block/sed-opal.c > +++ b/block/sed-opal.c > @@ -80,6 +80,7 @@ struct parsed_resp { > > struct opal_dev { > bool supported; > + bool mbr_enabled; > > void *data; > sec_send_recv *send_recv; > @@ -283,6 +284,14 @@ static bool check_tper(const void *data) > return true; > } > > +static bool check_mbrenabled(const void *data) > +{ > + const struct d0_locking_features *lfeat = data; > + u8 sup_feat = lfeat->supported_features; > + > + return !!(sup_feat & MBR_ENABLED_MASK); > +} > + > static bool check_sum(const void *data) > { > const struct d0_single_user_mode *sum = data; > @@ -417,6 +426,7 @@ static int opal_discovery0_end(struct opal_dev *dev) > u32 hlen = be32_to_cpu(hdr->length); > > print_buffer(dev->resp, hlen); > + dev->mbr_enabled = false; > > if (hlen > IO_BUFFER_LENGTH - sizeof(*hdr)) { > pr_debug("Discovery length overflows buffer (%zu+%u)/%u\n", > @@ -442,6 +452,8 @@ static int opal_discovery0_end(struct opal_dev *dev) > check_geometry(dev, body); > break; > case FC_LOCKING: > + dev->mbr_enabled = check_mbrenabled(body->features); > + break; > case FC_ENTERPRISE: > case FC_DATASTORE: > /* some ignored properties */ > @@ -2190,6 +2202,21 @@ static int __opal_lock_unlock(struct opal_dev *dev, > return next(dev); > } > > +static int __opal_set_mbr_done(struct opal_dev *dev, struct opal_key *key) > +{ > + u8 mbr_done_tf = 1; > + const struct opal_step mbrdone_step [] = { > + { opal_discovery0, }, > + { start_admin1LSP_opal_session, key }, > + { set_mbr_done, &mbr_done_tf }, > + { end_opal_session, }, > + { NULL, } > + }; > + > + dev->steps = mbrdone_step; > + return next(dev); > +} > + > static int opal_lock_unlock(struct opal_dev *dev, > struct opal_lock_unlock *lk_unlk) > { > @@ -2345,6 +2372,11 @@ bool opal_unlock_from_suspend(struct opal_dev *dev) > suspend->unlk.session.sum); > was_failure = true; > } > + if (dev->mbr_enabled) { > + ret = __opal_set_mbr_done(dev, &suspend->unlk.session.opal_key); > + if (ret) > + pr_debug("Failed to set MBR Done in S3 resume\n"); > + } > } > mutex_unlock(&dev->dev_lock); > return was_failure; >
On Tue, Sep 05, 2017 at 04:51:51PM -0600, Jon Derrick wrote: Jens, sorry to bother, can you take this? Or do you have any follow up comments? RHEL Is starting to pull changes in for 7.5 I want to get this landed so they can pick it up. If it's already on your list, sorry for being impatient. > Looks good > > Acked-by Jon Derrick: <jonathan.derrick@intel.com> > > > On 09/01/2017 08:53 AM, Scott Bauer wrote: > > Users who are booting off their Opal enabled drives are having > > issues when they have a shadow MBR set up after s3/resume cycle. > > When the Drive has a shadow MBR setup the MBRDone flag is set to > > false upon power loss (S3/S4/S5). When the MBRDone flag is false > > I/O to LBA 0 -> LBA_END_MBR are remapped to the shadow mbr > > of the drive. If the drive contains useful data in the 0 -> end_mbr > > range upon s3 resume the user can never get to that data as the > > drive will keep remapping it to the MBR. To fix this when we unlock > > on S3 resume, we need to tell the drive that we're done with the > > shadow mbr (even though we didnt use it) by setting true to MBRDone. > > This way the drive will stop the remapping and the user can access > > their data. > >
On 09/08/2017 01:36 PM, Scott Bauer wrote: > On Tue, Sep 05, 2017 at 04:51:51PM -0600, Jon Derrick wrote: > Jens, sorry to bother, can you take this? Or do you have any follow up comments? > > RHEL Is starting to pull changes in for 7.5 I want to get this landed so they can > pick it up. I can pull it in for a pull request later in this merge window.
diff --git a/block/opal_proto.h b/block/opal_proto.h index f40c9acf8895..e20be8258854 100644 --- a/block/opal_proto.h +++ b/block/opal_proto.h @@ -46,6 +46,7 @@ enum opal_response_token { #define GENERIC_HOST_SESSION_NUM 0x41 #define TPER_SYNC_SUPPORTED 0x01 +#define MBR_ENABLED_MASK 0x10 #define TINY_ATOM_DATA_MASK 0x3F #define TINY_ATOM_SIGNED 0x40 diff --git a/block/sed-opal.c b/block/sed-opal.c index 9b30ae5ab843..9ed51d0c6b1d 100644 --- a/block/sed-opal.c +++ b/block/sed-opal.c @@ -80,6 +80,7 @@ struct parsed_resp { struct opal_dev { bool supported; + bool mbr_enabled; void *data; sec_send_recv *send_recv; @@ -283,6 +284,14 @@ static bool check_tper(const void *data) return true; } +static bool check_mbrenabled(const void *data) +{ + const struct d0_locking_features *lfeat = data; + u8 sup_feat = lfeat->supported_features; + + return !!(sup_feat & MBR_ENABLED_MASK); +} + static bool check_sum(const void *data) { const struct d0_single_user_mode *sum = data; @@ -417,6 +426,7 @@ static int opal_discovery0_end(struct opal_dev *dev) u32 hlen = be32_to_cpu(hdr->length); print_buffer(dev->resp, hlen); + dev->mbr_enabled = false; if (hlen > IO_BUFFER_LENGTH - sizeof(*hdr)) { pr_debug("Discovery length overflows buffer (%zu+%u)/%u\n", @@ -442,6 +452,8 @@ static int opal_discovery0_end(struct opal_dev *dev) check_geometry(dev, body); break; case FC_LOCKING: + dev->mbr_enabled = check_mbrenabled(body->features); + break; case FC_ENTERPRISE: case FC_DATASTORE: /* some ignored properties */ @@ -2190,6 +2202,21 @@ static int __opal_lock_unlock(struct opal_dev *dev, return next(dev); } +static int __opal_set_mbr_done(struct opal_dev *dev, struct opal_key *key) +{ + u8 mbr_done_tf = 1; + const struct opal_step mbrdone_step [] = { + { opal_discovery0, }, + { start_admin1LSP_opal_session, key }, + { set_mbr_done, &mbr_done_tf }, + { end_opal_session, }, + { NULL, } + }; + + dev->steps = mbrdone_step; + return next(dev); +} + static int opal_lock_unlock(struct opal_dev *dev, struct opal_lock_unlock *lk_unlk) { @@ -2345,6 +2372,11 @@ bool opal_unlock_from_suspend(struct opal_dev *dev) suspend->unlk.session.sum); was_failure = true; } + if (dev->mbr_enabled) { + ret = __opal_set_mbr_done(dev, &suspend->unlk.session.opal_key); + if (ret) + pr_debug("Failed to set MBR Done in S3 resume\n"); + } } mutex_unlock(&dev->dev_lock); return was_failure;
Users who are booting off their Opal enabled drives are having issues when they have a shadow MBR set up after s3/resume cycle. When the Drive has a shadow MBR setup the MBRDone flag is set to false upon power loss (S3/S4/S5). When the MBRDone flag is false I/O to LBA 0 -> LBA_END_MBR are remapped to the shadow mbr of the drive. If the drive contains useful data in the 0 -> end_mbr range upon s3 resume the user can never get to that data as the drive will keep remapping it to the MBR. To fix this when we unlock on S3 resume, we need to tell the drive that we're done with the shadow mbr (even though we didnt use it) by setting true to MBRDone. This way the drive will stop the remapping and the user can access their data. Signed-off-by: Scott Bauer <scott.bauer@intel.com> --- block/opal_proto.h | 1 + block/sed-opal.c | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+)