Re: [jgunthorpe:iommu_pt_vtd 8/34] ERROR: modpost: "__udivdi3" [drivers/iommu/generic_pt/fmt/iommu_amdv1.ko] undefined!

[Nathan Chancellor <nathan@kernel.org>]

Hi Jason,

On Tue, Sep 16, 2025 at 11:12:56AM -0300, Jason Gunthorpe wrote:
> On Tue, Sep 16, 2025 at 01:38:56PM +0800, Oliver Sang wrote:
> > hi, Jason,
> > 
> > On Tue, Sep 02, 2025 at 09:48:56AM -0300, Jason Gunthorpe wrote:
> > > On Wed, Aug 27, 2025 at 06:53:24PM +0800, kernel test robot wrote:
> > > > tree:   https://github.com/jgunthorpe/linux iommu_pt_vtd
> > > > head:   fbb816ad5a7780a27bde18e3090b30552b6068f9
> > > > commit: 9722034883c205c7cfc7e395cb9a44221f010fc3 [8/34] iommupt: Add map_pages op
> > > > config: i386-randconfig-r111-20250827 (https://download.01.org/0day-ci/archive/20250827/202508271856.ixwxgh3g-lkp@intel.com/config)
> > > > compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
> > > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250827/202508271856.ixwxgh3g-lkp@intel.com/reproduce)
> > > > 
> > > > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > > > the same patch/commit), kindly add following tags
> > > > | Reported-by: kernel test robot <lkp@intel.com>
> > > > | Closes: https://lore.kernel.org/oe-kbuild-all/202508271856.ixwxgh3g-lkp@intel.com/
> > > > 
> > > > All errors (new ones prefixed by >>, old ones prefixed by <<):
> > > > 
> > > > >> ERROR: modpost: "__udivdi3" [drivers/iommu/generic_pt/fmt/iommu_amdv1.ko] undefined!
> > > 
> > > I could not reproduce this..
> > 
> > sorry for late.
> > 
> > not sure how you reproduce? the reroducer in
> > https://download.01.org/0day-ci/archive/20250827/202508271856.ixwxgh3g-lkp@intel.com/reproduce
> > is wrong. could you try below?
> 
> I used the instructions with the ubuntu clang 20 build.
> 
> Your instructions with the 0day clang build do reproduce.
> 
> This seems like a bug in that 0day clang compiler build:
> 
> static inline unsigned int pt_pgsz_lg2_to_level(struct pt_common *common,
> 						unsigned int pgsize_lg2)
> {
> 	static_assert(PT_GRANULE_LG2SZ < 256);
> 	return (pgsize_lg2 - PT_GRANULE_LG2SZ) /
> 	       ((unsigned int)(PT_TABLEMEM_LG2SZ - ilog2(PT_ITEM_WORD_SIZE)));
> 
> Is the source of the issue.
> 
> I added this:
> 
> 	typeof(PT_GRANULE_LG2SZ) x = PT_GRANULE_LG2SZ;
> 
> 	static_assert(sizeof(x) == 8);
> 
> And my compilers, and the 0day sparse, all fail that static_assert, x
> is 'int'.
> 
> ../drivers/iommu/generic_pt/fmt/../pt_fmt_defaults.h:36:9: sparse: error: static assertion failed: "sizeof(x) == 8"
> 
> This is correct as in C the type of an enum value is always int and it
> is a GCC extension that if int is not big enough for the constant the
> type is promoted till it is.
> 
> But for some reason your clang 20 build is saying it is not int. That
> then triggers incorrect 64 bit division.

I spent some time looking into this today and I ran out of time before
coming to any real conclusion but I wanted to dump what I had so far.

This error is not reproducible with LLVM 19.1.7 but it is with LLVM
20.1.8. Running a bisect between those two versions reveals

  https://github.com/llvm/llvm-project/commit/a0f88901a4e6a6618c3ec02108103d0415e28834

as the bad commit, which certainly makes sense, as C23 made this GCC
extension as part of the standard so clang needed to account for that.
As far as I can tell, GCC did the same thing in

  https://gcc.gnu.org/cgit/gcc/commit/?id=3b3083a598ca3f4b6203284e01ed39ab6ff0844f

  "The standard feature gives all the values of such an enum the enumerated
  type (while keeping type int if that can represent all values of the
  enumeration), where previously GCC only gave those values outside the range
  of int the enumerated type... this patch makes the change to types of
  enumerators unconditionally (if that causes problems in practice we could
  always make it conditional on C2x mode instead)."

so I am somewhat surprised GCC does not exhibit similar behavior since I
would expect the underlying type of this enum to be 'unsigned long long'
from the GENMASK_ULL usage to define PT_TOP_PHYS_MASK... but I have not
been able to figure out why yet.

This could be easily worked around by moving PT_TOP_PHYS_MASK to its own
enum like has been done in the past like ff1cc97b1f4c ("block/blk-iocost
(gcc13): keep large values in a new enum") did.

Cheers,
Nathan