Looks good

Acked-by Jon Derrick: <[email protected]>


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 <[email protected]>
> ---
>  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;
> 

Reply via email to