linux-kernel - RE: [PATCH v6 02/10] scsi: ufshpb: Add host control mode support to rsp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 24 Mar 2021 12:44:49 +0000
From:   Avri Altman <Avri.Altman@....com>
To:     Can Guo <cang@...eaurora.org>,
        Zang Leigang <zangleigang@...ilicon.com>
CC:     "James E . J . Bottomley" <jejb@...ux.vnet.ibm.com>,
        "Martin K . Petersen" <martin.petersen@...cle.com>,
        "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        Bart Van Assche <bvanassche@....org>,
        yongmyung lee <ymhungry.lee@...sung.com>,
        Daejun Park <daejun7.park@...sung.com>,
        "alim.akhtar@...sung.com" <alim.akhtar@...sung.com>,
        "asutoshd@...eaurora.org" <asutoshd@...eaurora.org>,
        Avi Shchislowski <Avi.Shchislowski@....com>,
        Bean Huo <beanhuo@...ron.com>,
        "stanley.chu@...iatek.com" <stanley.chu@...iatek.com>
Subject: RE: [PATCH v6 02/10] scsi: ufshpb: Add host control mode support to
 rsp_upiu

> >> @@ -1245,6 +1257,18 @@ static void
> ufshpb_rsp_req_region_update(struct
> >> ufshpb_lu *hpb,
> >>              srgn_i =
> >>                      be16_to_cpu(rsp_field->hpb_active_field[i].active_srgn);
> >>
> >> +            rgn = hpb->rgn_tbl + rgn_i;
> >> +            if (hpb->is_hcm &&
> >> +                (rgn->rgn_state != HPB_RGN_ACTIVE || is_rgn_dirty(rgn))) {
> >> +                    /*
> >> +                     * in host control mode, subregion activation
> >> +                     * recommendations are only allowed to active regions.
> >> +                     * Also, ignore recommendations for dirty regions - the
> >> +                     * host will make decisions concerning those by himself
> >> +                     */
> >> +                    continue;
> >> +            }
> >> +
> >
> > Hi Avri, host control mode also need the recommendations from device,
> > because the bkops would make the ppn invalid, is that right?
> 
> Right, but ONLY recommandations to ACTIVE regions are of host's interest
> in host control mode. For those inactive regions, host makes its own
> decisions.
Correct.
Generally speaking, in host control mode, the host may ignore the device recommendation altogether.
In host mode the device is not allowed to send recommendation to Inactive regions.
Furthermore, as the comment above indicates, we *chose* to ignore recommendation for DIRTY regions.
We do so, because the host is aware of dirty regions and can make the decision per its own internal logic.
ONLY if the region is ACTIVE and CLEAN - the host is unaware that bkops took place,
So we *chose* to act per the device recommendation.  

Thanks,
Avri
> 
> Can Guo.
> 
> >
> >>              dev_dbg(&hpb->sdev_ufs_lu->sdev_dev,
> >>                      "activate(%d) region %d - %d\n", i, rgn_i, srgn_i);
> >>
> >> @@ -1252,7 +1276,6 @@ static void ufshpb_rsp_req_region_update(struct
> >> ufshpb_lu *hpb,
> >>              ufshpb_update_active_info(hpb, rgn_i, srgn_i);
> >>              spin_unlock(&hpb->rsp_list_lock);
> >>
> >> -            rgn = hpb->rgn_tbl + rgn_i;
> >>              srgn = rgn->srgn_tbl + srgn_i;
> >>
> >>              /* blocking HPB_READ */
> >> @@ -1263,6 +1286,14 @@ static void
> ufshpb_rsp_req_region_update(struct
> >> ufshpb_lu *hpb,
> >>              hpb->stats.rb_active_cnt++;
> >>      }
> >>
> >> +    if (hpb->is_hcm) {
> >> +            /*
> >> +             * in host control mode the device is not allowed to inactivate
> >> +             * regions
> >> +             */
> >> +            goto out;
> >> +    }
> >> +
> >>      for (i = 0; i < rsp_field->inactive_rgn_cnt; i++) {
> >>              rgn_i = be16_to_cpu(rsp_field->hpb_inactive_field[i]);
> >>              dev_dbg(&hpb->sdev_ufs_lu->sdev_dev,
> >> @@ -1287,6 +1318,7 @@ static void ufshpb_rsp_req_region_update(struct
> >> ufshpb_lu *hpb,
> >>              hpb->stats.rb_inactive_cnt++;
> >>      }
> >>
> >> +out:
> >>      dev_dbg(&hpb->sdev_ufs_lu->sdev_dev, "Noti: #ACT %u #INACT %u\n",
> >>              rsp_field->active_rgn_cnt, rsp_field->inactive_rgn_cnt);
> >>
> >> diff --git a/drivers/scsi/ufs/ufshpb.h b/drivers/scsi/ufs/ufshpb.h
> >> index 7df30340386a..032672114881 100644
> >> --- a/drivers/scsi/ufs/ufshpb.h
> >> +++ b/drivers/scsi/ufs/ufshpb.h
> >> @@ -121,6 +121,8 @@ struct ufshpb_region {
> >>
> >>      /* below information is used by lru */
> >>      struct list_head list_lru_rgn;
> >> +    unsigned long rgn_flags;
> >> +#define RGN_FLAG_DIRTY 0
> >>  };
> >>
> >>  #define for_each_sub_region(rgn, i, srgn)                           \
> >> --
> >> 2.25.1
> >>