On 2021-03-23 20:48, Avri Altman wrote:
> > On 2021-03-23 14:37, Daejun Park wrote:
> > > > On 2021-03-23 14:19, Daejun Park wrote:
> > > > > > On 2021-03-23 13:37, Daejun Park wrote:
> > > > > > > > On 2021-03-23 12:22, Can Guo wrote:
> > > > > > > > > On 2021-03-22 17:11, Bean Huo wrote:
> > > > > > > > > > On Mon, 2021-03-22 at 15:54 +0900, Daejun Park
> > > > > > > > > > wrote:
> > > > > > > > > > > + switch (rsp_field->hpb_op) {
> > > > > > > > > > > + case HPB_RSP_REQ_REGION_UPDATE:
> > > > > > > > > > > + if (data_seg_len !=
> > > > > > > > > > > DEV_DATA_SEG_LEN)
> > > > > > > > > > > + dev_warn(&hpb-
> > > > > > > > > > > >sdev_ufs_lu->sdev_dev,
> > > > > > > > > > > + "%s: data seg
> > > > > > > > > > > length is not
> > > > > > > > > > > same.\n",
> > > > > > > > > > > + __func__);
> > > > > > > > > > > +
> > > > > > > > > > > ufshpb_rsp_req_region_update(hpb, rsp_field);
> > > > > > > > > > > + break;
> > > > > > > > > > > + case HPB_RSP_DEV_RESET:
> > > > > > > > > > > + dev_warn(&hpb->sdev_ufs_lu-
> > > > > > > > > > > >sdev_dev,
> > > > > > > > > > > + "UFS device lost HPB
> > > > > > > > > > > information
> > > > > > > > > > > during
> > > > > > > > > > > PM.\n");
> > > > > > > > > > > + break;
> > > > > > > > > > Hi Deajun,
> > > > > > > > > > This series looks good to me. Just here I have
> > > > > > > > > > one question. You
> > > > > > > > > > didn't
> > > > > > > > > > handle HPB_RSP_DEV_RESET, just a warning. Based
> > > > > > > > > > on your SS UFS,
> > > > > > > > > > how
> > > > > > > > > > to
> > > > > > > > > > handle HPB_RSP_DEV_RESET from the host side? Do
> > > > > > > > > > you think we
> > > > > > > > > > shoud
> > > > > > > > > > reset host side HPB entry as well or what else?
> > > > > > > > > > Bean
> > > > > > > > > Same question here - I am still collecting
> > > > > > > > > feedbacks from flash
> > > > > > > > > vendors
> > > > > > > > > about
> > > > > > > > > what is recommanded host behavior on reception of
> > > > > > > > > HPB Op code
> > > > > > > > > 0x2,
> > > > > > > > > since it
> > > > > > > > > is not cleared defined in HPB2.0 specs.
> > > > > > > > > Can Guo.
> > > > > > > > I think the question should be asked in the HPB2.0
> > > > > > > > patch, since in
> > > > > > > > HPB1.0 device
> > > > > > > > control mode, a HPB reset in device side does not
> > > > > > > > impact anything
> > > > > > > > in
> > > > > > > > host side -
> > > > > > > > host is not writing back any HPB entries to device
> > > > > > > > anyways and HPB
> > > > > > > > Read
> > > > > > > > cmd with
> > > > > > > > invalid HPB entries shall be treated as normal
> > > > > > > > Read(10) cmd
> > > > > > > > without
> > > > > > > > any
> > > > > > > > problems.
> > > > > > > Yes, UFS device will process read command even the HPB
> > > > > > > entries are
> > > > > > > valid or
> > > > > > > not. So it is warning about read performance drop by
> > > > > > > dev reset.
> > > > > > Yeah, but still I am 100% sure about what should host do
> > > > > > in case of
> > > > > > HPB2.0
> > > > > > when it receives HPB Op code 0x2, I am waiting for
> > > > > > feedbacks.
> > > > > I think the host has two choices when it receives 0x2.
> > > > > One is nothing on host.
> > > > > The other is discarding all HPB entries in the host.
> > > > > In the JEDEC HPB spec, it as follows:
> > > > > When the device is powered off by the host, the device may
> > > > > restore
> > > > > L2P
> > > > > map
> > > > > data upon power up or build from the host’s HPB READ
> > > > > command.
> > > > > If some UFS builds L2P map data from the host's HPB READ
> > > > > commands, we
> > > > > don't
> > > > > have to discard HPB entries in the host.
> > > > > So I thinks there is nothing to do when it receives 0x2.
> > > > But in HPB2.0, if we do nothing to active regions in host
> > > > side, host
> > > > can
> > > > write
> > > > HPB entries (which host thinks valid, but actually invalid in
> > > > device
> > > > side since
> > > > reset happened) back to device through HPB Write Buffer cmds
> > > > (BUFFER
> > > > ID
> > > > = 0x2).
> > > > My question is that are all UFSs OK with this?
> > > Yes, it must be OK.
> > > Please refer the following the HPB 2.0 spec:
> > > If the HPB Entries sent by HPB WRITE BUFFER are removed by the
> > > device,
> > > for example, because they are not consumed for a long enough
> > > period of
> > > time,
> > > then the HPB READ command for the removed HPB entries shall be
> > > handled
> > > as a
> > > normal READ command.
> > No, it is talking about the subsequent HPB READ cmd sent after a
> > HPB
> > WRITE BUFFER cmd,
> > but not the HPB WRITE BUFFER cmd itself...
> Looks like this discussion is going the same way as we had in host
> mode.
> HPB-WRITE-BUFFER 0x2, if exist, is always a companion to HPB-READ.
> You shouldn't consider them separately.
> The device is expected to handle invalid ppn by itself, and
> specifically for this case,
> As Daejun explained, Handle each HPB-READ (and its companion
> HPB-WRITE-BUFFER) like READ10.
> For device mode, doing nothing in case of dev reset, seems to me
> like
> the right thing to do.
I just got some feedbacks from other flash vendors, they all commit
that
their devices can work well in this scenario [1]. Some of them
proposed
even complicated (maybe better) principles of handling the "HPB
reset",
but since the device works well in [1], I am OK with current
(simpler)
handling of "HPB reset" - in device mode doing nothing, in host mode
re-activate regions that host is trying to do a read to.
