Received-SPF: pass (google.com: domain of linux-kernel+bounces-80094-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Sun, 25 Feb 2024 15:21:29 +0100
From: Robert Richter <rrichter@amd.com>
To: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Alison Schofield <alison.schofield@intel.com>,
	Vishal Verma <vishal.l.verma@intel.com>,
	Ira Weiny <ira.weiny@intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Dave Jiang <dave.jiang@intel.com>,
	Davidlohr Bueso <dave@stgolabs.net>, linux-cxl@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Len Brown <lenb@kernel.org>, Lukas Wunner <lukas@wunner.de>,
	Fan Ni <nifan.cxl@gmail.com>
Subject: Re: [PATCH v4 2/3] cxl/pci: Get rid of pointer arithmetic reading
 CDAT table
Message-ID: <ZdtM6UJM733qAukn@rric.localdomain>
References: <20240216155844.406996-1-rrichter@amd.com>
 <20240216155844.406996-3-rrichter@amd.com>
 <20240219125059.0000737c@Huawei.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240219125059.0000737c@Huawei.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?IaYsqFGsEO9KxwK9DEuRPWGLBC7Ns/VSVxOCF4mFqgVED6+3A/xjPhohfrSQ?=
 =?us-ascii?Q?BcxOdvTpz6ojPgnXOBIBXsuv8skSofMSPZydu/Xfxd6IdGoTDlC50YZtTyt/?=
 =?us-ascii?Q?I1WEKrOZWWn2KoecvUedUMLA2g909esZndN1vdqsZ7udGyI1gr7RQ9LSfDrm?=
 =?us-ascii?Q?GnfT0hozaIx0kXoMwaenW0RarHyLPbQp94fr/EpjgmJd0g3f3KHAdayznNOC?=
 =?us-ascii?Q?KlizaeH5y2/I5w/iEjX2nnGnxw1VEtfQ/Slg/vwpSe2Lf3cgXyPpKGO78kEB?=
 =?us-ascii?Q?Edhm330YLEgxwDoe8F7nYJygvG7d1ztotkB+X7pHl8EFRSDb6kQ4HDG9YNsc?=
 =?us-ascii?Q?xunrEixj/OlAWUGwRuoch+J7kCniHD4M6MR/3IYIpR5+n9WW8TSbRWqU8yUT?=
 =?us-ascii?Q?wSIEVZBeo/AmfLoU5OLJ/TR/jjRH2LxRPWd4F549Gt0wePM8bKbx7kfwe0DQ?=
 =?us-ascii?Q?aBkjjbjqnyVB2UxXcm5gW+20I2uDDZzLEakU4PbgxqZljAQ4be2r6m8e43HU?=
 =?us-ascii?Q?H6nzNHjijpRE263cUYbQWLfdUn49T5P0K/1lJLrgOGzUjS4KJCekXAFUdHA8?=
 =?us-ascii?Q?FoqFoRiILm2+R/nFGC0n5TNMdLJ4l2rV0vKtzszEzOS8N1wRvbrCzo9T3xFX?=
 =?us-ascii?Q?jM9foUhqy2yTMxTMbFigwpoWGOyAsmlwz93URoUnGin0xNbL4+u1XTbFT6sH?=
 =?us-ascii?Q?01+ts+OlTL9WKrvh3yE4oh/LaukxMlIjk/r5oG7AP3Qt0JjqEK51imvBULFl?=
 =?us-ascii?Q?4hY0P9BMzjSObBCflJ3M1XYgp+ZemaauSoYLEABatPGhdqddEmTP4qXy7sml?=
 =?us-ascii?Q?vr+/qFB/Y9UsVVvM/KAKgxfFlJi96QcI6RgXxB6lleJftgeqhK9rKxzjdhdX?=
 =?us-ascii?Q?ieq+PC4UcRIwG9GyLnU9BY/ilhI6OrTq26K2Jk1P1rZxmEvoVmRrfMn/6f3O?=
 =?us-ascii?Q?7WxxSABFMmuej5/zHYtXSZ7rfI+leR4WDDOTKeKlxxPTIX00POjk5L7A392p?=
 =?us-ascii?Q?WavDN+eOH6AHLwMtMbO5Nx45bXLBlcBVxWC3aCiSSJbh2hpLWmc0udstbao0?=
 =?us-ascii?Q?JAUizTCHH/4s1zrUSZNI01nR7ZVVrOi6RjZtRsyk8GxqmL0vpBSJlKExkrDe?=
 =?us-ascii?Q?F0jWcweW07qqPYwbQsEHBn3q5OLE4rWTWpXpyWJ18kpUH4VPMc93hmQydx02?=
 =?us-ascii?Q?/nrCJp73ifvn/xS3FiPBVNjATXz/O0+LmKdLnIZpoLY04W4TR6gP3bXh5YZr?=
 =?us-ascii?Q?G3zOfro1uKa9I4w2ibwbKBzPdd81e84uft9kqOwttN+FQFuxUrDj97JfpGNd?=
 =?us-ascii?Q?YOYpchnTKy8cAY3korMAw7pnEuxUyJWqIuE1BT9ZqpUX/m+S1k1n6+V8syWO?=
 =?us-ascii?Q?Z1iS5A9qcq+3xsgmvuxsZ+hml6aN7zS6dCsyiZgQzxp20IXZjAW6KJHajSXm?=
 =?us-ascii?Q?czcQLPWm/cKvSZE2w1H8ENCIkMY4L/ul80giUkTsIlV3p0aCBJ1GpRsPtGtg?=
 =?us-ascii?Q?rjVrT+omQW3Ork/m3CT+ii7HpV7oVrZls6gCT7OsZgL6Fo2gKojtWced7RVZ?=
 =?us-ascii?Q?LuwqrISEOix7AV9Ki18=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: bed342b6-6d79-4086-a183-08dc360d128d
X-MS-Exchange-CrossTenant-AuthSource: IA0PR12MB8746.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Feb 2024 14:21:36.5788
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: EDubTbRmmJtDOi4ShmbVSO5cIyiJ45b0rWC26Z2YISEkNmrWYNUyrcGWGFCi2lbYkIMriVnX68unaFsDvIQ1sw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR12MB5371

On 19.02.24 12:50:59, Jonathan Cameron wrote:
> On Fri, 16 Feb 2024 16:58:43 +0100
> Robert Richter <rrichter@amd.com> wrote:
> 
> > 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. E.g., see fix of devm_kfree()
> > in read_cdat_data():
> > 
> >  c65efe3685f5 cxl/cdat: Free correct buffer on checksum error
> > 
> > Rework code to avoid calculations with sizeof(__le32). Introduce
> > struct cdat_doe_rsp 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.
> > 
> > Cc: Lukas Wunner <lukas@wunner.de>
> > Cc: Fan Ni <nifan.cxl@gmail.com>
> > Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> > Signed-off-by: Robert Richter <rrichter@amd.com>
> Ok. I suspect we could fine tune this for ever but changes here look good
> enough to me and definitely nicer than the original ;)
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Thanks for your review of this series and also the v5 update for patch
#3.

-Robert