Received-SPF: pass (google.com: domain of linux-kernel+bounces-426-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Thu, 14 Dec 2023 20:34:09 -0800
From: Dan Williams <dan.j.williams@intel.com>
To: Robert Richter <rrichter@amd.com>, Dan Williams <dan.j.williams@intel.com>
CC: Ira Weiny <ira.weiny@intel.com>, Davidlohr Bueso <dave@stgolabs.net>,
	Jonathan Cameron <jonathan.cameron@huawei.com>, Dave Jiang
	<dave.jiang@intel.com>, Alison Schofield <alison.schofield@intel.com>, Vishal
 Verma <vishal.l.verma@intel.com>, <linux-cxl@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, Fan Ni <nifan.cxl@gmail.com>, Lukas Wunner
	<lukas@wunner.de>
Subject: Re: [PATCH] cxl/pci: Get rid of pointer arithmetic reading CDAT table
Message-ID: <657bd741d2961_269bd294d@dwillia2-mobl3.amr.corp.intel.com.notmuch>
References: <20231116-fix-cdat-devm-free-v1-1-b148b40707d7@intel.com>
 <ZVfIaNhiSc-yQZo5@rric.localdomain>
 <ZVfJ6Fxidvw_gz7r@rric.localdomain>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <ZVfJ6Fxidvw_gz7r@rric.localdomain>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?Ey5O0UwWa/i1F8ppDCwK91Wz4u2mCqtNQH48rVAzjV6yz54AzeM183UzL308?=
 =?us-ascii?Q?HmJ6VmUP1gGD2yL88nMU/uXLM6ZGroOGaeOfbgJ3p3yw7sjcf+qcts0pXJek?=
 =?us-ascii?Q?7/HDMHAt3ZCqO9tyeMqzJr3yFPbTRPAMzrA9rVdOG25C6MhJ37o7nGG0N37u?=
 =?us-ascii?Q?MAJXYt6l7HfRD219/D1tqtwKo0Y4oQrILE6ltpFRn8HJOPBAHV0KUjbtM7p9?=
 =?us-ascii?Q?RH7O7WJvXUJYx7jl6y5InW8HLPqsiB8VHoXO+b+g3S629otITbASSZCoKqbI?=
 =?us-ascii?Q?OjQzI2TjCT1wasJuzJeFJxkC6Ft3VhTsiDJrDfOnjtKjHCf57MoJtmnovpxH?=
 =?us-ascii?Q?kRX7VnHtmSR+sUTNxm4tDLB6VyfGB3kSc6VgpY+IcEFlKm06fgHNGA8tu1Qu?=
 =?us-ascii?Q?x7eGkHFHPN5pYR+uEbS162V6nWzd9p7EYH+AVPQAmRxJ4We3mQ/Cgdh8sVUQ?=
 =?us-ascii?Q?cpI0J9Jo0R1XMlA23w2o1+7Mr42lIu6p/9d7j0Ofoh9nHRp+89KHpi/7luGu?=
 =?us-ascii?Q?b5uqTR8Cdygzs4r7H3Xt+rmHiOvB7Jkexpoko3PUFkwtZEgtzzjnaZ8v4FHu?=
 =?us-ascii?Q?+roFAf68dG5yTf2soWD2QShBEJDO4Rryn9gFKKtp5s31nP1uKWQI2H0NhbPU?=
 =?us-ascii?Q?2R4hrQgq5GxZ3iIyZ3wCKRMUcpUuk4wzLA4BAK+/ybJ1WDkAIjeFhjR2QG8Y?=
 =?us-ascii?Q?NOAZYstFNifZWa0sx9C8Q+NG0B6s4YUaVn0Jclmjh3IpDEtFAQA04+hNOpLU?=
 =?us-ascii?Q?HeVeRIXqwzlmrM1uQWOsG9Ak9rGQqo7aqaBbjSXaBsMpLyWnjTZbHqLgkhxL?=
 =?us-ascii?Q?EkfG3AeXIffx9WNpdjVTghnoP/F/a37f3jNmYc3prnwzjFE1UNyxARMw3Q9Q?=
 =?us-ascii?Q?MyvrwLcLP8ZP3jyFa3zw8jTyBYL6CN0XxKzbK1EW7Ul4TCsXwCWPJ7gz8OXf?=
 =?us-ascii?Q?xGEIlZuO5qCaeWZwt/qoUDw8a1848fylD52w1N1IcBm4utl40G/KUxW/uUFp?=
 =?us-ascii?Q?lvZZUH0SKLrOjii8Lo85GoAAxMFGEoQbC8xkzngDhY2OkveJT9GIhAETopxg?=
 =?us-ascii?Q?+QUOrevPyhbgjRx2g4PCYQR9nEEFZVkP5FXSb/RuEBmpv9u2524Zego4/DSp?=
 =?us-ascii?Q?JQQKOJR4kC900TdrrSEmvgockgzXquAOsDhOoHpJdM1B+AX3lvD7kLVNSNPV?=
 =?us-ascii?Q?UxTx3u1NCYxdOHmX3jshtB9OOJjWV3xcfjSUxT0HXDqcd85GcCOM/1MGLV8y?=
 =?us-ascii?Q?0WELd8U0HLxiZxSCcFj8Z45ltR3nmgwM2cTdAu+ovecNC/d7Z834Er94u4j5?=
 =?us-ascii?Q?1bRlnCi1DouYyePZ1C8WDvTrISkNTZPzzt4r3MWdAZs61MBRZ2q3bD1bzmTU?=
 =?us-ascii?Q?brQK5dX9xLTQ2649w4YhdbLH9MR0wGWib2HJlPungT5cKrOdRwQD9qf3QrTo?=
 =?us-ascii?Q?AgluyftFHufK1KDQcgc/lOSnBSGc78kInL1fzvB0w/Wv7hjmK4/MvK4esQwi?=
 =?us-ascii?Q?kTezIcj6kZDE+J46vc2T1Yc6Hmn6+xydw0/qkMCiDkD3yLoU294C7iKvO1I8?=
 =?us-ascii?Q?Y2kXioHAH4sfjzYpzmNbvQr9hOvuQYKkaRgQt/IicG1hvtfk/QI3HLDUORnm?=
 =?us-ascii?Q?6Q=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 0fddf615-e90f-4969-d24d-08dbfd27168a
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB8107.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Dec 2023 04:34:12.8686
 (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: YS0ifDvWPjAfJCZmPoV0uDkaxKnWzsHOSynBrl83gKt55l0mQbZdz3l1t1YXkBmu8D2pRta+mT4kYdNyPY/kJyC6A+XxW0X1uRB3mZsrhTA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR11MB6763
X-OriginatorOrg: intel.com

Robert Richter wrote:
> On 17.11.23 21:09:18, Robert Richter wrote:
> > I will send an on-top patch for 6.8 that reworks that code area to
> > remove the pointer arithmetic.
> 
> Here it is:
> 
> From 13787f72c20b8c54754ae86015d982307eae0397 Mon Sep 17 00:00:00 2001
> From: Robert Richter <rrichter@amd.com>
> Subject: [PATCH] cxl/pci: Get rid of pointer arithmetic reading CDAT table
> 
> Reading the CDAT table using DOE requires a Table Access Response
> Header in addition to the CDAT entry. In current implementation this
> has caused offsets with sizeof(__le32) to the actual buffers. This led
> to hardly readable code and even bugs (see fix of devm_kfree() in
> read_cdat_data()).
> 
> Rework code to avoid calculations with sizeof(__le32). Introduce
> struct cdat_doe for this which contains the Table Access Response
> Header and a variable payload size for various data structures
> afterwards to access the CDAT table and its CDAT Data Structures
> without recalculating buffer offsets.

I like reworking the code to introduce an explicit type for the response
buffer, but as Ira points out, lets call it a "response" not a
"cdat_doe".

The feedback on the flex array is accurate, but I see no reason to have
3 flex arrays vs:

struct cdat_response {
       __le32 doe_header;
       union {
               struct cdat_header header;
               struct cdat_entry_header entry;
               u8 table[];
       };
} __packed;

As far as I can see nothing outside of drivers/cxl/core/pci.c needs
'struct cdat_response', so it can stay local to this C file.

While you are working on that I will do another lead-in cleanup to kill
the goto in cxl_cdat_read_table() and let you come back and kill off the
open-coded "+ sizeof(__le32)" that I will leave behind.