Received-SPF: pass (google.com: domain of linux-kernel+bounces-6099-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Tue, 19 Dec 2023 15:32:18 -0800
From: Ira Weiny <ira.weiny@intel.com>
To: Jonathan Cameron <Jonathan.Cameron@huawei.com>, Ira Weiny
	<ira.weiny@intel.com>
CC: Fan Ni <nifan.cxl@gmail.com>, Dave Jiang <dave.jiang@intel.com>,
	<linux-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>, Huai-Cheng Kuo
	<hchkuo@avery-design.com.tw>
Subject: Re: [PATCH v2 2/2] cxl/cdat: Fix header sum value in CDAT checksum
Message-ID: <658228023732d_277bd294ae@iweiny-mobl.notmuch>
References: <20231117-fix-cdat-cs-v2-0-715399976d4d@intel.com>
 <20231117-fix-cdat-cs-v2-2-715399976d4d@intel.com>
 <20231218132837.00001424@Huawei.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20231218132837.00001424@Huawei.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?2IFGkqTQVgheX0A1oHNOBZ3gNPyqd6wy4549e4fmnEE7sip48l/hodD7Rec/?=
 =?us-ascii?Q?TkKybRx45NHEQd1W8Hv4G1Uv6XnFZabp5dQ0dd3nbtNgfir2qHUnCrvU5asc?=
 =?us-ascii?Q?jSDCGaqg+UcRxFDRgFA5clC66OcX501xRJjHucXZa0QW0BUk+1T6V5WYiwsG?=
 =?us-ascii?Q?gY67EFItp1KQz0ppWgYyfakczK9GI0ypNHVlIsI/Cmgaf9yF0oAD/zxfEfdy?=
 =?us-ascii?Q?ijADt9VEY9O6qspPxHOcIJyYfbTrUfb615FPXWlyZ9MIjWXAdj6D/62LNTFd?=
 =?us-ascii?Q?m6auUKrwnCSNW0AQr/R7SSB3/J3zSt/DC0CTfoJ2IB7UyGOdjPAebGmZ//0H?=
 =?us-ascii?Q?+3+liThVsTXqsWOPzJtsc7yqxuDDd3n1bCeOuJ62NYhhS4+Sv6y3qN5SVK4d?=
 =?us-ascii?Q?TSAWrizMr47PRZfZMfeGPpioCsR0taR1YIPWUIWWwBaRTqKzwETT71/S8ip1?=
 =?us-ascii?Q?0UlZro84muT7YC0bDeeL87bMacKtF0kqDQcct4ueZOX4/pkkOeuiqt8Wk0ZT?=
 =?us-ascii?Q?/0akLWsxEVQuBacq5jZAalZVgCvz+q8sSR4JuZqCCnqwB5Ix9BExWoq1W6hZ?=
 =?us-ascii?Q?e1qeFVHHIJrVClJ8oNlc5guFa+xXsFIg6KZs03/ZZCGlrxuMkTWvl1gGiIZi?=
 =?us-ascii?Q?/ClXwkGDZ3FFgSxl/axbqCSQbj1goun+62cR6gXpbBUFe4lxuHqciRvW/yB3?=
 =?us-ascii?Q?5LYASRVagaUiFWbksQPuYR/woeFzONf3VCig0TG29WXs1NZ2rCYmG0r0atiQ?=
 =?us-ascii?Q?BDM3mcp7EtutZbQM3E/a7mTebXNo6jcRMYS9t6YXqMm9jLd8A32MhI5BNAbO?=
 =?us-ascii?Q?TyJcMtrsa9miQ5F/eh7wWyBNPeks4Vb4ja/nZzSBysUdMwJKFGezSzWFg0pe?=
 =?us-ascii?Q?vv+4IvVomC0fbLKiY5LLrHS0qVZ/PTovgrUDiWht66gFEaBWzFsTkOQ3HYOL?=
 =?us-ascii?Q?QWThBFv17EWwWYIaXatQV9VOgAwzsXzTbwXF2n5G7hxUpsC3BkWjzas56rnH?=
 =?us-ascii?Q?gNB1aYAPNavExOWM+b40lsKjjpZjwiB4RY3x40Ysawm/FnLj9Q4cTp8LYUGV?=
 =?us-ascii?Q?KyPkuN4cufzkvS+39fvOuNSiO8GPRwbkc14E20LrdzU6ZPRjCsJKLGN48v4P?=
 =?us-ascii?Q?1Ohewet+/QZJkhssYnXWtKt4bsxXyv+sCrb5CF3UGve/43J6JJ/2ASHbxiHP?=
 =?us-ascii?Q?y/1Bt3lREkHUItuBmym6YZkLlkhc7/aSZr6vb9A7JiZsQVaIVsEpVXTyIrmL?=
 =?us-ascii?Q?d+tkIYlaLgjJGb3IpVsnu+IA0hFWF4bouWZ8wQRtPitQMvorTGIJku8ZFxfL?=
 =?us-ascii?Q?Z77N5cMM/QLwuGiQuoQYF6+cWq7tEuvY0lj1cmH1Te5wph+lUiy5NfzUGiQV?=
 =?us-ascii?Q?jlhwLbPRh9Pgl9M8cqrqdOQfZy7ycw5hPr73hipDHVXCSx2blxITvfNIqsA2?=
 =?us-ascii?Q?fTdsCidPagBmly+/CoIniMYnvTrm0RMDRiLyjGXX5LKBFmfzVQSKImL98FO2?=
 =?us-ascii?Q?IC+biokYiBYfzBAZEh5pp60nksxYWPaQIi0U+hzLpm4wQa66vN4mia1hKeVN?=
 =?us-ascii?Q?usPkWxNQ9jJ+E5JNj0AeEraCEaN0EAMsrYdek4reKJGTNvsivv9TLcC8Ddz1?=
 =?us-ascii?Q?ZGmA9SHg43Lv9sSP2qvVylTRbsAjS1t0RXu5wNIjhcJB?=
X-MS-Exchange-CrossTenant-Network-Message-Id: da0cfc56-c656-43fd-0d40-08dc00eabfda
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Dec 2023 23:32:22.1837
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: KlOrTERJSQIARBbKaroTushKdbHHaCuXPUco36NetWJxC05YSgIP4FYMOJ5T32K1reBIuFqrUjww8IW/5YeZ9w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR11MB8372
X-OriginatorOrg: intel.com

Jonathan Cameron wrote:
> On Wed, 29 Nov 2023 17:33:04 -0800
> Ira Weiny <ira.weiny@intel.com> wrote:
> 
> > The addition of the DCD support for CXL type-3 devices extended the CDAT
> > table large enough that the checksum being returned was incorrect.[1]
> > 
> > This was because the checksum value was using the header length field
> > rather than each of the 4 bytes of the length field.  This was
> > previously not seen because the length of the CDAT data was less than
> > 256 thus resulting in an equivalent checksum value.
> > 
> > Properly calculate the checksum for the CDAT header.
> > 
> > [1] https://lore.kernel.org/all/20231116-fix-cdat-devm-free-v1-1-b148b40707d7@intel.com/
> > 
> > Fixes: aba578bdace5 ("hw/cxl/cdat: CXL CDAT Data Object Exchange implementation")
> > Cc: Huai-Cheng Kuo <hchkuo@avery-design.com.tw>
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > 
> > ---
> > Changes from V1:
> > [djiang: Remove do {} while (0);]
> > ---
> >  hw/cxl/cxl-cdat.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/cxl/cxl-cdat.c b/hw/cxl/cxl-cdat.c
> > index 24829cf2428d..67e44a4f992a 100644
> > --- a/hw/cxl/cxl-cdat.c
> > +++ b/hw/cxl/cxl-cdat.c
> > @@ -49,6 +49,7 @@ static void ct3_build_cdat(CDATObject *cdat, Error **errp)
> >      g_autofree CDATTableHeader *cdat_header = NULL;
> >      g_autofree CDATEntry *cdat_st = NULL;
> >      uint8_t sum = 0;
> > +    uint8_t *buf;
> This results in a shadowing variable warning. I'll rename it to hdr_buf or something
> like that.

<sigh>  sorry again...

With all the discussion did you want me to re-roll the set?

AFAICS this is the only actual issue.  So if you want to do it that would
be great.

Thanks,
Ira