RE: [PATCH v4 1/1] scsi: ufs: core: add device level exception support

[Arthur Simchaev]

> Hi Arthur,
> This is not a flags attribute. This is for a Query Read 64-bit Attribute data. In
> the existing code, we do not have a read 64-bit attribute, so adding this new
> code would also allow future re-use.
> 
> The new "struct utp_upiu_query_response_v4_0" would improve readability
> because it is formatted exactly as how the jedec standard defines for Attribute
> Read. We won't need to use type cast to get the 64-bit value.
> There would be no issue with efficiency because the same machine code
> would be generated.
> 
> The existing "struct utp_upiu_query_v4_0" probably has a bug in it. It does
> not use the  __attribute__((__packed__)) attribute. The compiler is free to add
> padding in this structure, resulting in the read attribute value being incorrect. I
> plan to provide a separate patch to fix this issue.

Hi Bao 

Upiu_query can be used for all device management command (descriptions, attributes, flags)
See section 10.7.9 UPIU QUERY RESPONSE in the UFS 4.1 specification.
If "struct utp_upiu_query" was properly defined, according to the UFS specification  (by OSF's),
we would not need to add additional "struct utp_upiu_query_v4_0" structures.
If you think the structure should be packaged, you can fix "struct utp_upiu_query" and 
"struct utp_upiu_query_v4_0".

Regards
Arthur

> -----Original Message-----
> From: Bao D. Nguyen <quic_nguyenb@xxxxxxxxxxx>
> Sent: Wednesday, March 26, 2025 12:16 AM
> To: Arthur Simchaev <Arthur.Simchaev@xxxxxxxxxxx>;
> quic_cang@xxxxxxxxxxx; quic_nitirawa@xxxxxxxxxxx; bvanassche@xxxxxxx;
> avri.altman@xxxxxxx; peter.wang@xxxxxxxxxxxx;
> manivannan.sadhasivam@xxxxxxxxxx; minwoo.im@xxxxxxxxxxx;
> adrian.hunter@xxxxxxxxx; martin.petersen@xxxxxxxxxx
> Cc: linux-scsi@xxxxxxxxxxxxxxx; Alim Akhtar <alim.akhtar@xxxxxxxxxxx>;
> James E.J. Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>; Matthias
> Brugger <matthias.bgg@xxxxxxxxx>; AngeloGioacchino Del Regno
> <angelogioacchino.delregno@xxxxxxxxxxxxx>; Bean Huo
> <beanhuo@xxxxxxxxxx>; Keoseong Park <keosung.park@xxxxxxxxxxx>;
> Ziqi Chen <quic_ziqichen@xxxxxxxxxxx>; Al Viro <viro@xxxxxxxxxxxxxxxxxx>;
> Gwendal Grignou <gwendal@xxxxxxxxxxxx>; Eric Biggers
> <ebiggers@xxxxxxxxxx>; open list <linux-kernel@xxxxxxxxxxxxxxx>; moderated
> list:ARM/Mediatek SoC support:Keyword:mediatek <linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx>; moderated list:ARM/Mediatek SoC
> support:Keyword:mediatek <linux-mediatek@xxxxxxxxxxxxxxxxxxx>
> Subject: Re: [PATCH v4 1/1] scsi: ufs: core: add device level exception support
> 
> On 3/25/2025 9:33 AM, Arthur Simchaev wrote:
> > Hi Bao
> >
> > I think adding "struct utp_upiu_query_response_v4_0" is redundant and not
> correct for flags upiu response .
> > You can use "struct utp_upiu_query_v4_0"
> >
> Hi Arthur,
> This is not a flags attribute. This is for a Query Read 64-bit Attribute data. In
> the existing code, we do not have a read 64-bit attribute, so adding this new
> code would also allow future re-use.
> 
> The new "struct utp_upiu_query_response_v4_0" would improve readability
> because it is formatted exactly as how the jedec standard defines for Attribute
> Read. We won't need to use type cast to get the 64-bit value.
> There would be no issue with efficiency because the same machine code
> would be generated.
> 
> The existing "struct utp_upiu_query_v4_0" probably has a bug in it. It does
> not use the  __attribute__((__packed__)) attribute. The compiler is free to add
> padding in this structure, resulting in the read attribute value being incorrect. I
> plan to provide a separate patch to fix this issue.
> 
> Thanks, Bao