On 3/4/25 08:19, Borislav Petkov wrote:
> +static inline void __invlpgb_all(unsigned long asid, unsigned long pcid,
> + unsigned long addr, u16 nr_pages, u8 flags)
> +{
> + __invlpgb(asid, pcid, addr, nr_pages, 0, flags);
> +}
Why would __invlpg_all() need an 'addr' or 'nr_pages'? Shouldn't those be 0?
It's _better_ of course when it happens at a single site and it's close
to a prototype for __invlpgb(). But it's still a magic '0' that it's
impossible to make sense of without looking at the prototype.
Looking at the APM again... there really are three possible values for
ECX[31]:
0: increment by 4k
1: increment by 2M
X: Don't care, no increment is going to happen
What you wrote above could actually be written:
__invlpgb(asid, pcid, addr, nr_pages, 1, flags);
so the 0/1 is _actually_ completely random and arbitrary as far as the
spec goes.
Why does it matter?
It enables you to do sanity checking. For example, we could actually
enforce a rule that "no stride" can't be paired with any of the
per-address invalidation characteristics:
if (stride == NO_STRIDE) {
WARN_ON(flags & INVLPGB_FLAG_VA);
WARN_ON(addr);
WARN_ON(nr_pages);
}
That's impossible if you pass a 'bool' in.
But, honestly, I'm deep into nitpick mode here. I think differentiating
the three cases is worth it, but it's also not the hill I'm going to die
on. ;)
