Re: [PATCH] drm/amdgpu: avoid integer overflow warning in amdgpu_device_resize_fb_bar()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxx>

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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux