> Avri, > thanks for your comments and review. > > > On Tue, 2022-11-08 at 19:09 +0000, Avri Altman wrote: > > > Add advanced RPMB support in ufs_bsg. For these reasons, we try to > > > implement Advanced RPMB in ufs_bsg: > > > 1. According to the UFS specification, only one RPMB operation can > > > be performed at any time. We can ensure this by using reserved slot > > > and its dev_cmd sync operation protection mechanism. > > > > Regardless of its technical convenience, this approach unfortunately > > breaks the spec. > > > > The spec say (please note the line numbers): > > > > "..... > > > > 5197 12.4.5.1 Advanced RPMB Message > > > > 5198 An Advanced RPMB Message is composed of an Advanced RPMB > Meta > > Information and a MAC/KEY in > > > > 5199 the EHS field in *COMMAND UPIU* and *RESPONSE UPIU*. Advanced > > RPMB Data is delivered through > > > > ....." > > > Moreover, in the examples that are provided, it is still expected to > > be carried via SECURITY PROTOCOL IN and SECURITY PROTOCOL OUT, > > > > See e.g. Figure 12.15 — Authenticated Data Write Flow (in Advanced > > RPMB Mode). > > > > > not quite get what you meant here. I meant that we should still build upius that contains security protocol commands. > > > > > > > Therefore, wrapping the rpmb packets in a query-request upiu and > > query-response upiu is not allowed. > > > > > > no, I didn't wrap RPMB packet in query-request/response, it is inupiu_req > and upiu_rsp, it is upiu command. I was thinking you are using a query request upiu because you are using device management commands flow, And specifically, I didn't see where you are setting ucd_req_ptr->sc.cdb, Which should hold the security protocol cdb. > Based on Bart's suggestion, we shouldn't > change the current ufs_bsg structure. I think his concern is that if we change > ufs_bsg structure, the user space tool also needs to change as well. Adding command upiu support will not break any ufs-bsg tools, e.g. ufs-utils. It would just add this capability. Thanks, Avri > > > > > Still, I agree that the approach you suggested, namely to rely on the > > ufs-bsg driver, is the cleanest way to handle the advance rpmb access. > > > > However, IMHO, you need to do it is by adding command UPIU to the > > ufs-bsg driver. > > > > > > Yes, agree with you on this point. But we still need to use reserved slots for > RPMB or command UPIU, we don't want to affect IO requests on the normal > path. > > One problem is that we didn't split the dev_manage command and the > RPMB command in their completion handlers. I would like to change > dev_man to passthrough or something else, and then split dev_man and > RPMB, otherwise, they would be mixed in one dev_man completion handler. > No technical issues here, just want to make it more readable and > maintainable. > > > > Kind regards, > Bean > > > > > Thanks, > > > > Avri
