> From: Jason Gunthorpe <[email protected]>
> Sent: Saturday, September 20, 2025 2:20 AM
>
> On Thu, Sep 18, 2025 at 07:05:50AM +0000, Tian, Kevin wrote:
> > > 256*2^30, 396,5665 , 389,5258 , 92.92
> >
> > data here mismatches those in coverletter, though the difference
> > didn't affect the conclusion. 😊
>
> I ran it twice, it isn't stable in micro :)
reasonable, but better to use a same set all over the place.
>
> > > +if IOMMU_PT
> > > +config IOMMU_PT_AMDV1
> > > + tristate "IOMMU page table for 64-bit AMD IOMMU v1"
> >
> > remove "64-bit"? I don't think there is a 32-bit format ever.
>
> I am marking the 64 bit vs 32 bit in the kconfig descriptions since
> that seemed to be a pattern from iopgtable
>
> > > +
> > > +static inline unsigned int amdv1pt_table_item_lg2sz(const struct
> pt_state
> > > *pts)
> > > +{
> > > + return PT_GRANULE_LG2SZ +
> > > + (PT_TABLEMEM_LG2SZ - ilog2(PT_ITEM_WORD_SIZE)) * pts-
> > > >level;
> > > +}
> > > +#define pt_table_item_lg2sz amdv1pt_table_item_lg2sz
> >
> > this is the same as in pt_fmt_defaults.h
>
> Yeah, I left these behind because the order of declaration is
> inconvenient, the format itself wants to call this function a few
> lines down.
>
> But I just realized it can be forward declared:
>
> /* Body in pt_fmt_defaults.h */
> static inline unsigned int pt_table_item_lg2sz(const struct pt_state *pts);
>
> So that's fixed.
>
> > > +static inline void
> > > +amdv1pt_install_leaf_entry(struct pt_state *pts, pt_oaddr_t oa,
> > > + unsigned int oasz_lg2,
> > > + const struct pt_write_attrs *attrs)
> > > +{
> > > + unsigned int isz_lg2 = pt_table_item_lg2sz(pts);
> > > + u64 *tablep = pt_cur_table(pts, u64) + pts->index;
> >
> > check that the index is aligned to oasz_log2
>
> I see the various formats have a mismatched collection of assertions,
> I added this function:
>
> /*
> * Format can call in the pt_install_leaf_entry() to check the arguments are
> all
> * aligned correctly.
> */
> static inline bool pt_check_install_leaf_args(struct pt_state *pts,
> pt_oaddr_t oa,
> unsigned int oasz_lg2)
> {
> unsigned int isz_lg2 = pt_table_item_lg2sz(pts);
>
> if (PT_WARN_ON(oalog2_mod(oa, oasz_lg2)))
> return false;
>
> #ifdef pt_possible_sizes
> if (PT_WARN_ON(isz_lg2 < oasz_lg2 ||
> oasz_lg2 > isz_lg2 + pt_num_items_lg2(pts)))
> return false;
> #else
> if (PT_WARN_ON(oasz_lg2 != isz_lg2 &&
> oasz_lg2 != isz_lg2 + pt_contig_count_lg2(pts)))
> return false;
> #endif
>
> if (PT_WARN_ON(oalog2_mod(pts->index, oasz_lg2 - isz_lg2)))
> return false;
> return true;
> }
>
> And the format can just call it.
>
> Made both these edits to all the formats..
>
much better.
