Received-SPF: pass (google.com: domain of linux-kernel+bounces-85260-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Wed, 28 Feb 2024 09:28:28 -0600
From: Lucas De Marchi <lucas.demarchi@intel.com>
To: Arnd Bergmann <arnd@arndb.de>
CC: Arnd Bergmann <arnd@kernel.org>, Oded Gabbay <ogabbay@kernel.org>, Thomas
 =?utf-8?Q?Hellstr=C3=B6m?= <thomas.hellstrom@linux.intel.com>, "Maarten
 Lankhorst" <maarten.lankhorst@linux.intel.com>, Maxime Ripard
	<mripard@kernel.org>, Thomas Zimmermann <tzimmermann@suse.de>, Dave Airlie
	<airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>, Rodrigo Vivi
	<rodrigo.vivi@intel.com>, Matt Roper <matthew.d.roper@intel.com>, "Matthew
 Brost" <matthew.brost@intel.com>, <intel-xe@lists.freedesktop.org>,
	<dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/3] [v2] drm/xe/xe2: fix 64-bit division in
 pte_update_size
Message-ID: <zft2yp3y4gtqrypbvs6qhuqfpkctitxyg2bbmnb7omk4nwv6nb@wkusd6zjwx4t>
References: <20240226124736.1272949-1-arnd@kernel.org>
 <20240226124736.1272949-3-arnd@kernel.org>
 <kq3cpbz4ctqvfhtlh7f7wxd7ub3izdjovhv2jqkjasre7u6y2k@exaw42ber3f6>
 <0c6127da-53d8-4c37-8337-e64e3e91bbaf@app.fastmail.com>
Content-Type: text/plain; charset="us-ascii"; format=flowed
Content-Disposition: inline
In-Reply-To: <0c6127da-53d8-4c37-8337-e64e3e91bbaf@app.fastmail.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?YmzF7+lZ3QCjcOhUdnnqJdXON+IGJGLCpuKuTXn5Wq0pPXgdMv7L0oJ9MfKO?=
 =?us-ascii?Q?h7B4O5ZZSLRqZQ8F8GG3ydPCcS5y9qrLzV1cOMFacGg4ymxFHdhP6JhdA28u?=
 =?us-ascii?Q?Q4DgC6bHHMyewUbc11meAgisYaemhVhmT5axfBVf74yqYRC6afb5Ao/hP/n/?=
 =?us-ascii?Q?7kOE0iDX+OHe4jm1lRag1cClfzQJgGSaVcRoHFTh8oabQUOAxvxG+rM532Hl?=
 =?us-ascii?Q?x/q8Gp1tBR0uGot/yTPHfiXXYZL/xi4cMU5TKEheSoDHuvJcKy5Fp/9NiN5F?=
 =?us-ascii?Q?5rKIDaZvIbPbUUuU6C/zTeoZEczUO3OnhomBh+JH+ig9/J2YmkyZccjDkvMB?=
 =?us-ascii?Q?qtPsW3v3AfGuAL3bmFZCKbZZNfeBmnZxLhAWzXXCw+JXbE3e9CwqgI4ghmtR?=
 =?us-ascii?Q?8HOWZItfJeHN6UJnVs7nsM03W37VtdVTB3tSY8DSDv3S7dV8/yXmUHS/nf2k?=
 =?us-ascii?Q?c0i3mdGgNnpGuDTFl+pUmt8vWLoO2HUAlWW4jEnmkPIzRvyqz1Dum943xbtT?=
 =?us-ascii?Q?rfmrhbjp1PBLCthLdbup2/dSdPnFqoyzhq5XPpDSnpHmdY1xl8hGsuNpGKjJ?=
 =?us-ascii?Q?vHSjBlCQ8sO/2RHOc+FiJqIc4VCcj6KqWdr3w+oIJVHj+u6M8jj9Q2d2lXyk?=
 =?us-ascii?Q?kJKOwiIHHqjA1oajLviU9egytUoDtgZKAmLeDDFzBkrfg8eXL9030VyiRbKC?=
 =?us-ascii?Q?Q5s8i7wbsgbTiPLRueu1BLRf4TtvyOZeK5guWS3HQHC2Dh7jBggLK4m1Ce6v?=
 =?us-ascii?Q?EKSsmg4WegsSmpcscCyyz2UU6aBRk6mbasdai8QnRgqnm1HFAJTq4V1rcWAw?=
 =?us-ascii?Q?v24vcou86hD9CpPnnrUfNdiTJORrP7V3iIt6JCO0j3cshNb0E0Eq53sHOO4t?=
 =?us-ascii?Q?hiILpGDqAbZlzE0CjUmJKudyFOekZjV9guJdkHdcCxZ3RumHul27fYj9PFNp?=
 =?us-ascii?Q?NsZ34kvPDoGnZjIqZ1udUFvGgVVNIJvwo2LudSKpluJvQoN8oEjRcNgm4jAy?=
 =?us-ascii?Q?tsnNkbbGX/BoUKmbNRrqUL1gIbHumtjY2T0Lm8abQSclfsvzFLk7m/3DuzmQ?=
 =?us-ascii?Q?WucNTkQoSDET9vJ9UxNsB23emB1DmIyXHqU8XIXJ3yJtLYpx+IUgSoHW+N+q?=
 =?us-ascii?Q?7Anzk3j6niZ0pSdQk5oMnVTqQGKeT+rps2UMaJqrnUT3bTwb7Hr8f2OA9VqC?=
 =?us-ascii?Q?S42BAI2dnBKhHapUe6eIxmvYNBgaIJG2TZ+QIhntKEhe3l1v4wqVTGxKdZ0n?=
 =?us-ascii?Q?rjVgLfwkjJkMkvN0tLYOu9jzd/D4VOPS3gvVJ25achYEOT/WaTw5tgTviqSB?=
 =?us-ascii?Q?nrz3PB1G747XnVsBj69naYa4NjpFf6WTd685R/79Z36QBDrfhBPtkDFYrP88?=
 =?us-ascii?Q?u66U5r37J7b76ubhxACXkjULddObifi39wt7OwgHQfpIKDS8TjvtD7rWyvO0?=
 =?us-ascii?Q?RmRErg+H/qK7PES0BWuo3qCMKBDA8NbO/ghjUmzhIPNfp7Y2LH18fv4pQ8jP?=
 =?us-ascii?Q?Mg9TMWHfv6z7BkdztnQA8H7t7Rby1x94/leQitG77atr7wzccx6IUuGXz1oc?=
 =?us-ascii?Q?PITpjGHqBa40fhxAYTj3zwTr7r/a93iLw3/gOFlJdKRc0gatavc5YvYg9dZY?=
 =?us-ascii?Q?9w=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: c38970bf-620f-4b73-3ea6-08dc3871eba1
X-MS-Exchange-CrossTenant-AuthSource: CY5PR11MB6139.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Feb 2024 15:28:31.7194
 (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: ucpV6zu9VfGwvxBRLJbASEi9e2fFoDepJs9XjA/rILvvJ+wu75pk5AhyVdvh5PR8g8zbn5NpIlRjR8qL/L6uSnQYikoItBTnUAGWUXqDa7k=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR11MB8413
X-OriginatorOrg: intel.com

On Wed, Feb 28, 2024 at 01:26:29PM +0100, Arnd Bergmann wrote:
>On Mon, Feb 26, 2024, at 17:40, Lucas De Marchi wrote:
>> On Mon, Feb 26, 2024 at 01:46:38PM +0100, Arnd Bergmann wrote:
>>>
>>>Fixes: 237412e45390 ("drm/xe: Enable 32bits build")
>>>Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>>---
>>>v2: use correct Fixes tag
>>
>> but what about the other comment? How are we supposed to use
>> DIV_ROUND_UP() but then in some places (which?) have to open code it?
>
>The problem is not DIV_ROUND_UP() but the division but the 64-bit
>division itself. There is a DIV_ROUND_UP_ULL() macro that would
>address the build failure as well, but doing the shift is much
>more efficient here since it can be done in a couple of instructions.
>
>> What compiler does this fail on?
>
>I saw it with clang-19 on 32-bit arm, but I assume it happens
>on others as well.

somehow it passed on x86 :-/

>
>>> drivers/gpu/drm/xe/xe_migrate.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>>diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
>>>index a66fdf2d2991..ee1bb938c493 100644
>>>--- a/drivers/gpu/drm/xe/xe_migrate.c
>>>+++ b/drivers/gpu/drm/xe/xe_migrate.c
>>>@@ -462,7 +462,7 @@ static u32 pte_update_size(struct xe_migrate *m,
>>> 	} else {
>>> 		/* Clip L0 to available size */
>>> 		u64 size = min(*L0, (u64)avail_pts * SZ_2M);
>>>-		u64 num_4k_pages = DIV_ROUND_UP(size, XE_PAGE_SIZE);
>>>+		u32 num_4k_pages = (size + XE_PAGE_SIZE - 1) >> XE_PTE_SHIFT;
>>
>> also the commit message doesn't seem to match the patch as you are only
>> changing one instance.
>
>Not sure what you mean. As I wrote in the changelog, the
>second instance is fixed by using a 32-bit division here,
>which does not cause link failures.

I missed the type conversion to u32 and was thinking there was another
hunk missing for the second change.

all looks good to me now and I will apply later today to drm-xe-next.
Thanks.

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Lucas De Marchi

>
>      Arnd