Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Message-ID: <c881f759-617b-4349-b207-934b4c72a1ac@intel.com>
Date:   Thu, 30 Nov 2023 09:20:04 -0700
User-Agent: Betterbird (Linux)
Subject: Re: [PATCH v2 2/2] cxl/cdat: Fix header sum value in CDAT checksum
To:     Ira Weiny <ira.weiny@intel.com>,
        Jonathan Cameron <jonathan.cameron@huawei.com>,
        Fan Ni <nifan.cxl@gmail.com>
CC:     <linux-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        "Huai-Cheng Kuo" <hchkuo@avery-design.com.tw>
References: <20231117-fix-cdat-cs-v2-0-715399976d4d@intel.com>
 <20231117-fix-cdat-cs-v2-2-715399976d4d@intel.com>
Content-Language: en-US
From:   Dave Jiang <dave.jiang@intel.com>
In-Reply-To: <20231117-fix-cdat-cs-v2-2-715399976d4d@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UjVNTERadGhjR1BuYUhoVEFWRU1jNjJ3MjdxdFI2SFVSQ3pJZGpQV0ZwYjdL?=
 =?utf-8?B?YXkyclkweTVrME4wSHZHV1ZQSWxBV0c4UXVXWlM0S1kzallHeHVaaXhaMGFR?=
 =?utf-8?B?dENuTWNxV3RFNnVYbVNRckZJbys3VVJBZzhTV2kzU0duZFdWVDZYNzgrMGly?=
 =?utf-8?B?dzAySmV5N2x4L1RPR1E0b1FmYm1mcTJpTWkzYWcxUllodU5CZE42aXpiRjJT?=
 =?utf-8?B?YlBwMmg4MGljbTluS2JCaHRHQUx6dVI1dUw2bmdIMit4UjFEK2U5STFpMzdm?=
 =?utf-8?B?eXlsSzZKb1RDTTFvajVLTUxIUGVObitNZ0pPWGdmTG8yY2E0Z0JKT3lOR29X?=
 =?utf-8?B?TXVUNlA5SC9YTnlYd21oUHkwTTU1VkRqT1hxWW9ISWNVRjNpNDJyR3lwbFNB?=
 =?utf-8?B?eCtsL1lrZTJ4eTNVbHVac1c0YllNTlZxNUtyU0dEWWdGem9CTGFwMFVNTE1M?=
 =?utf-8?B?N2hhTXQ0bzRPZEwwKzhjRzQ3WlpzQ0FvR3pvZXdLeW5jRUNDcksrWG03RzVW?=
 =?utf-8?B?N3VqOVkvZHN0NGxtNTRyb3UvRFRTVnExU1ZsTklUM0RPUnVSTm93ZWZEYkZC?=
 =?utf-8?B?V283SXUvSjFvTFBxSklsTTFJdWV6UHhDMExiTkt4RGlNT0lneUozdkRLS01P?=
 =?utf-8?B?WUNmbHVta2JwR3N2eldyaGp4cUV2WlpKYUg3V1FUeDJTZ2JRRC9tZlhKK1ZL?=
 =?utf-8?B?MXZlUWRGc3lNeSsraFVwU0NnRlp6WGpzMTlHK3RxM0lMdjVsZlNRNTlRa3Ew?=
 =?utf-8?B?VzU0NENPS0U4T2ZudWFaWDFvMU9xVWFvelRMcDkzNFh1T0xFV2tpOVJ3bWlE?=
 =?utf-8?B?S21kRlZvREROTlFITkpxS0FWcmVsSURObnY3SUhpU3lEcldwNDZXZytzSWVz?=
 =?utf-8?B?bGZYeFc0QzdEQ0swWVFuQkV2c2pqZGVrelo2TTdhVk9MYk91L1N6Sm1BSXJG?=
 =?utf-8?B?YjdQVndISkpMN0lMWG5acGxYeVlJeFkzSWErczNNeG5vcWFtdWgwL2RsZm9I?=
 =?utf-8?B?MHBEYzZtWERNM0kwWS9WSXpYNW5oSk5aNkp4cU5OTUttNytzckdQQkFaRnRo?=
 =?utf-8?B?YlJYT2hWK2tBaFNSSGN0U0dQTHU1ZzJuSEx2bnBzRzg1WkV3ME1aYytHYXBy?=
 =?utf-8?B?T2d6UHNMSkVVTUltdFVWZUtJSU5MQWp3TmN0dFRsK1pBc0pyYkNRK0FZV2h1?=
 =?utf-8?B?WEZMWWk3VEhaMm1jSE1nVEVyR084MzJQeXVwYTdFODRHMjBHU3V3cjRiVU5q?=
 =?utf-8?B?U2UzVDdYeE05Y3lidGp4UDJWVDBkd3Q4bVA3WFVSNW5kK0ZlNTNLQStobHlh?=
 =?utf-8?B?c1JvTVF6T1p5SHBsL3VWRE5nNkkrZ2haRWpRdEVDbElZdGZxSmF1c1pXL0Vl?=
 =?utf-8?B?cHZIUGNrY29EQ0kvZVY0YjBUYU0zSWlVa0xDL0V2bzFVQmE3a0VIcmFSall3?=
 =?utf-8?B?NUJiRyt1c09TME0yUWthVzdrQngxSXlMUDhTVnFGaHQ2TWE2YXRyNThwcEtV?=
 =?utf-8?B?OXJ4SUNCWWpVZW5LYXB5N2t5Tm1TR0ZYS2pjODliSElvK1VOaEFBRFRoUEVw?=
 =?utf-8?B?L2l0cWs4WWZ6Qmp4U1dJaTJWei9iaDhJYTZQTDJ5MnRMOHV0L1A2VDh3M3Bk?=
 =?utf-8?B?Q1FpelNJMnBZMlhrdS95amZqa2VVSFdrKzMwVG51cGhTbTRDV0pXYTVVaHRv?=
 =?utf-8?B?RWNEZ2xNOVZpS3pxaGxwQm4xb3A2R3VWOFNLQXdkK1VwVkE1TVJURzFQVGFi?=
 =?utf-8?B?OWpoWVQxZzZtdTBZYW1QVy9Ua1M5d1B5d3NWUWVEVDFqVjRGbHU2ZUlFYlpH?=
 =?utf-8?B?dHJkc1dVYk9aOVRZbk9CdDhBMjJpZFFDbndvalhCbThYdElCVkg5OVJVQWM4?=
 =?utf-8?B?M2Z2UTM0N1lzODFaeitwbzNtKzU4VnZ2NTJPdVViS3lQcUZHMDBIODlVek5B?=
 =?utf-8?B?R1orbkkzcXJ1OGQwcVJMY283d3dXMjBRZ3AzRjRoSkZpQU5vMFBYZkVUVzhO?=
 =?utf-8?B?TGltcU1EY1VkUExYbWk2bzFGK0Y0alMwVTZMeWYvc1YwdGcvRFE1WE51SmZ0?=
 =?utf-8?B?czRSSWVpWVlzb0JaZCtFSVpsOERFRU94d0pWVzZwUllVZHRDaURWRnJhUUE5?=
 =?utf-8?Q?p71ftnxm+OMvrt+9fNkcVcOQ+?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 3f81e2ea-e5bd-40ee-7088-08dbf1c03782
X-MS-Exchange-CrossTenant-AuthSource: PH7PR11MB5984.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Nov 2023 16:20:07.2478
 (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: x76P26HU/A06na9SbIGrcMc7C+QPK/7OinS7RKSOuM90ACxg5RV7Mvx2IDs30h99PsnrI4q+lzDmfr3MUvdUMA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB7385
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Thu, 30 Nov 2023 08:21:43 -0800 (PST)


On 11/29/23 18:33, Ira Weiny 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>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

Def more future proof if they introduce new fields in later versions of the table.

> 
> ---
> 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;
>      int ent, i;
>  
>      /* Use default table if fopen == NULL */
> @@ -95,8 +96,12 @@ static void ct3_build_cdat(CDATObject *cdat, Error **errp)
>      /* For now, no runtime updates */
>      cdat_header->sequence = 0;
>      cdat_header->length += sizeof(CDATTableHeader);
> -    sum += cdat_header->revision + cdat_header->sequence +
> -        cdat_header->length;
> +
> +    buf = (uint8_t *)cdat_header;
> +    for (i = 0; i < sizeof(*cdat_header); i++) {
> +        sum += buf[i];
> +    }
> +
>      /* Sum of all bytes including checksum must be 0 */
>      cdat_header->checksum = ~sum + 1;
>  
>