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

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

[Jason Gunthorpe]

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.

Jason

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

[Nathan Chancellor]

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

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

[Jason Gunthorpe]

On Tue, Sep 16, 2025 at 07:20:43PM -0700, Nathan Chancellor wrote:
> 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)."

That makes sense, I did not know it had changed like that

But still, I'd expect the compiler to retain the original behavior
unless std23/gnu23 is specified given that it does change codegen and
causes errors.

Though that is even more perplexing if gcc has been working this way
since 13 why does it avoid the udiv..

Jason

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

[Nathan Chancellor]

On Wed, Sep 17, 2025 at 08:22:14AM -0300, Jason Gunthorpe wrote:
> On Tue, Sep 16, 2025 at 07:20:43PM -0700, Nathan Chancellor wrote:
> > 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)."
> 
> That makes sense, I did not know it had changed like that
> 
> But still, I'd expect the compiler to retain the original behavior
> unless std23/gnu23 is specified given that it does change codegen and
> causes errors.
> 
> Though that is even more perplexing if gcc has been working this way
> since 13 why does it avoid the udiv..

Looking at the preprocessed source and making some educated guesses, I
am guessing with inlining and constant folding, GCC turn division into
multiplication by the reciprocal. Clang implements some of these
optimizations but not all, if I remember correctly.

Cheers,
Nathan

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

[Jason Gunthorpe]

On Wed, Sep 17, 2025 at 03:12:17PM -0700, Nathan Chancellor wrote:
> Looking at the preprocessed source and making some educated guesses, I
> am guessing with inlining and constant folding, GCC turn division into
> multiplication by the reciprocal. Clang implements some of these
> optimizations but not all, if I remember correctly.

Oh interesting, I noticed that clang was measurably slower than gcc
for this code, maybe this is some of why

Jason