Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <f1b190cb-3af7-178b-baeb-b59363868779@amd.com>
Date:   Tue, 4 Jul 2023 14:33:57 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH] drm/amdgpu: avoid integer overflow warning in
 amdgpu_device_resize_fb_bar()
Content-Language: en-US
To:     Arnd Bergmann <arnd@arndb.de>, Arnd Bergmann <arnd@kernel.org>,
        Alex Deucher <alexander.deucher@amd.com>,
        "Pan, Xinhui" <Xinhui.Pan@amd.com>,
        Dave Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>
Cc:     Hawking Zhang <Hawking.Zhang@amd.com>,
        Lijo Lazar <lijo.lazar@amd.com>,
        Mario Limonciello <mario.limonciello@amd.com>,
        YiPeng Chai <YiPeng.Chai@amd.com>, Le Ma <le.ma@amd.com>,
        Bokun Zhang <Bokun.Zhang@amd.com>,
        Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>,
        Shiwu Zhang <shiwu.zhang@amd.com>,
        amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
        linux-kernel@vger.kernel.org
References: <20230703123557.3355657-1-arnd@kernel.org>
 <f4bc3739-3ff1-6fa7-5d7d-890f451315bf@amd.com>
 <788147c6-defd-496a-8174-1571b73d1a71@app.fastmail.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <788147c6-defd-496a-8174-1571b73d1a71@app.fastmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UEpjUlJPUGx1MTJFTklGeFNGUHZFOVBZODBtZFc1QjU2VnQwL2lMM1p6YzZr?=
 =?utf-8?B?WG9tSUV3Wm4rOUZUZWIzNFF3ckdjbW8wTjRnckFYS0ErT29LWktVa2dTN2py?=
 =?utf-8?B?MmNmcS8zaGVHYUJ4cGc0elFDSFkxRElUTDR1YzF0Z0FOaUJXLzdUVmpzWDhl?=
 =?utf-8?B?OXluVVVrcUNMMTBLUkJQQ24xZzN5Z0VDL1N2OVVIY0hiaUN3bXd1dWQ5TkxP?=
 =?utf-8?B?UXRRNWtpMmhIQXNLb1VMeFhXQ2pvWGJzM3hmazhQL2NtamVvaHBoQVZQZU4r?=
 =?utf-8?B?cGZibVhheTRDN3IrUXB1TnJlUE9XTEZ2UytLV3FpaEI2cWhINlQ4VUJodElF?=
 =?utf-8?B?Tmp1NTA3ZjVSOEV5WGRkSnRabHZHcDByU3RINWZCUjJoa1k5Z2Vla1pwakJm?=
 =?utf-8?B?dHMrdUl3c1FzdXRoVWR3cnBacVIweStKbnJSUlcrdjlpVXJHT25CVC9kd3hY?=
 =?utf-8?B?ejhINUduU1djV2hMNG1tNjFLNENLSjkwOGt0bm83YmNDeCt5cFJ3bzFWaktF?=
 =?utf-8?B?UTRPYi9BRUtRcjUrMTJGQzlVeTJabkhIL3F3azg3QVBEMVUvcWFUNnhpaFha?=
 =?utf-8?B?R21XN1k3TjdqaXRjY0hSNGpVZUpkOEVtZU41cTFMUXdUSzQ1d1JYT2JleUNY?=
 =?utf-8?B?RGd1aFY2eVJydTRyZEo4Q3MzTzZoZStNRmNFY1JXZ242WTdQdytJKzVHOHl5?=
 =?utf-8?B?R0t3Y1I4YzVsVTN0bTAvVllSMEk0SDA5U0c3eDFjT1llaGxjZmxCSFhlVlAv?=
 =?utf-8?B?MWRVUDhrNFl4Q0VYWitrV2kvcHBldXZyOHEzY1VhS1ZFcXJIbmQwU1pJMVVN?=
 =?utf-8?B?b1dZS1ZDYkEwaStjR1BYWXVGQkJvcDd0Sk9LZVA1dXhaUENwTHJpTTBvOUVW?=
 =?utf-8?B?cDVMY3hBVUZrZjNKL2Q1UTlzV0dzVGdsWjdNcFJHaC9QQ2s0TytEQkZjQ0pQ?=
 =?utf-8?B?Nkl6MHV2SnViRW9OeWwzNkxWVk1MVzFwM0J3bE1ZZ3BsL0JyLytsUGZra3hJ?=
 =?utf-8?B?TldCbGwvSngrRDI0TGVuTWdROUNPM3lyVTRhZVN1V3dCSzdRNU5uTllodXVJ?=
 =?utf-8?B?cXpGcU5tS1JsdXREa0hEeDI5UUFzbGZxNGpTbEJqcnFySzFPNHBDWGV4VUJ2?=
 =?utf-8?B?a3k2c1l0bmVrdGxYNWFwL09pOFNZU2VKN25xMkR3NGJZKzNod3MwUGhvVy9J?=
 =?utf-8?B?empkKzh0alEvdTBLRDJmUDBQQ01zMnJadHRUcS82aWwzeklHNXMxWVZWRC93?=
 =?utf-8?B?ekJpdDZLcGZuellDY3BOOU1oWEZLSWc3TW4vZGpiRjVZL01yaXhNWGpVOXRs?=
 =?utf-8?B?YjI4WVAyUktYY2dBdEVjTWtUczRPQXExMC9Uc3NBKzY2a0ZyaWhIRExCY2R2?=
 =?utf-8?B?R2tUaFFndkMzZFBZZllvUk9scUtkTFBjYVJBL3hNZ2RFZm1VN3NSNW5JMzZ0?=
 =?utf-8?B?Ykc0NXNPT0dSTVZZQUpBQ1RNVjFqMmtZLzZQWHZ1RE96SHZjVTl6YWRsZitT?=
 =?utf-8?B?cnRFeUdyMVJVUDgzek1VSGd3bmdTVmJMZ2J1Zjc0VlhYeUpjRWZXWjdMbUFP?=
 =?utf-8?B?dVdiZ2lyL29QKzd2SXNkd29kTlJXUHRRM0wrSCttNUpMRmNiVGZ3Q0VrdlJR?=
 =?utf-8?B?SmgwTmx1aVZrQkMrRDVRcEYwU09SWEZVcElmYW9HZWlDSVdybndqUWZPcjMv?=
 =?utf-8?B?MkFDdFpQd1NPRWJHRWZJSjk3OTBQWU8yb3NiajNHYk5vTDZRa1Y2akVSN3h5?=
 =?utf-8?B?amc2YlljYjhWYTBnTENjenRqcWJwbVdXRFl2VzZZZ3UyUHF0VjgwYWRBZGQv?=
 =?utf-8?B?eHFkdzU2ODFFWFcvWVZZT3RZYjFaZWgvRlAvSlZXaEFrczV5MmkyNGVMbWdi?=
 =?utf-8?B?SUtjZVZJcWdIeEVFN2pNWnA1TEh5cG5ZeXJ0V1VQYlB1dzVCZEx6WjNsd3VM?=
 =?utf-8?B?K043MHhEVFpHaXpENVNTbEJBTjVsam80aEd2NVBXZ0pEZDdLT2ZlL3BxVWNX?=
 =?utf-8?B?UElTK3NjU3l5MVZKSGJNUXQrK0VmNE9OK05HVkY1d1puSWRKcDRxeUc1ZGZk?=
 =?utf-8?B?L21FaUl2SVN0SVNvWVhDZDdDMFM5NmViWTRLbGhXdFg1VHNCMmRuTEk4V2Fl?=
 =?utf-8?B?VFhjb0pKd1lveUZWK2pTRU14WEliT3FmZjJ2UGtiMlpwMGxLK1NUZDI4clRS?=
 =?utf-8?Q?uLp3BMXLk68aj3R8u7egiVH69HAeGAwT7aGmV0yywtzn?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c2672766-70b0-4028-9f6d-08db7c8af3b5
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Jul 2023 12:34:04.1375
 (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: WsijI8iz5/aQflP/FJJWtPQs4/xUZ0yY2g9WA4GvlowNRmHzSf9rVs4EJE99gNhk
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB5735
X-Spam-Status: No, score=-1.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,NICE_REPLY_A,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=no autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Am 04.07.23 um 14:24 schrieb Arnd Bergmann:
> On Tue, Jul 4, 2023, at 08:54, Christian König wrote:
>> Am 03.07.23 um 14:35 schrieb Arnd Bergmann:
>>> From: Arnd Bergmann <arnd@arndb.de>
>>>
>>> On 32-bit architectures comparing a resource against a value larger than
>>> U32_MAX can cause a warning:
>>>
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:1344:18: error: result of comparison of constant 4294967296 with expression of type 'resource_size_t' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>>>                       res->start > 0x100000000ull)
>>>                       ~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
>>>
>>> The compiler is right that this cannot happen in this configuration, which
>>> is ok, so just add a cast to shut up the warning.
>> Well it doesn't make sense to compile that driver on systems with only
>> 32bit phys_addr_t in the first place.
> Not sure I understand the specific requirement. Do you mean the entire
> amdgpu driver requires 64-bit BAR addressing, or just the bits that
> resize the BARs?

Well a bit of both.

Modern AMD GPUs have 16GiB of local memory (VRAM), making those 
accessible to a CPU which can only handle 32bit addresses by resizing 
the BAR is impossible to begin with.

But going a step further even without resizing it is pretty hard to get 
that hardware working on such an architecture.

>> It might be cleaner to just not build the whole driver on such systems
>> or at least leave out this function.
> How about this version? This also addresses the build failure, but
> I don't know if this makes any sense:
>
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1325,6 +1325,9 @@ int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev)
>          u16 cmd;
>          int r;
>   
> +       if (!IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT))
> +               return 0;
> +

Yes, if that suppresses the warning as well then that makes perfect 
sense to me.

Regards,
Christian.

>          /* Bypass for VF */
>          if (amdgpu_sriov_vf(adev))
>                  return 0;
>
>       Arnd