linux-kernel - Re: [PATCH v2 1/3] iommu/io-pgtable-arm: Move selftests to a separate file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250917193818.GK1326709@ziepe.ca>
Date: Wed, 17 Sep 2025 16:38:18 -0300
From: Jason Gunthorpe <jgg@...pe.ca>
To: Mostafa Saleh <smostafa@...gle.com>
Cc: iommu@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, robin.murphy@....com, will@...nel.org,
	joro@...tes.org, praan@...gle.com
Subject: Re: [PATCH v2 1/3] iommu/io-pgtable-arm: Move selftests to a
 separate file

On Wed, Sep 17, 2025 at 07:11:38PM +0000, Mostafa Saleh wrote:
> +static void __init arm_lpae_dump_ops(struct io_pgtable_ops *ops)
> +{
> +	struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
> +	struct io_pgtable_cfg *cfg = &data->iop.cfg;

Can be:

       struct io_pgtable_cfg *cfg =
               &io_pgtable_ops_to_pgtable(pgtbl_ops)->cfg;

> +
> +	pr_err("cfg: pgsize_bitmap 0x%lx, ias %u-bit\n",
> +		cfg->pgsize_bitmap, cfg->ias);
> +	pr_err("data: %d levels, 0x%zx pgd_size, %u pg_shift, %u bits_per_level, pgd @ %p\n",
> +		ARM_LPAE_MAX_LEVELS - data->start_level, ARM_LPAE_PGD_SIZE(data),
> +		ilog2(ARM_LPAE_GRANULE(data)), data->bits_per_level, data->pgd);

The entire struct arm_lpae_io_pgtable is exposed to a public header
just for this one print.. Seems undesirable.

Drop the print? Honestly, I prefer this, given the maturity let's not
compromise modularity to print someting nobody will ever read..

Alternatively call a kunit-only exported function to do the print
directly from ops so. smmuv3 has an example how to do that

Either way a precursor patch to adjust it will allow this patch to
avoid publishing most stuff.

> +/*
> + * Calculate the right shift amount to get to the portion describing level l
> + * in a virtual address mapped by the pagetable in d.
> + */
> +#define ARM_LPAE_LVL_SHIFT(l,d)						\
> +	(((ARM_LPAE_MAX_LEVELS - (l)) * (d)->bits_per_level) +		\
> +	ilog2(sizeof(arm_lpae_iopte)))

Didn't see this being used?

> +#define ARM_LPAE_GRANULE(d)						\
> +	(sizeof(arm_lpae_iopte) << (d)->bits_per_level)

Only used by the above print

> +#define ARM_LPAE_PGD_SIZE(d)						\
> +	(sizeof(arm_lpae_iopte) << (d)->pgd_bits)

Only used by the above print

> +#define ARM_LPAE_PTES_PER_TABLE(d)					\
> +	(ARM_LPAE_GRANULE(d) >> ilog2(sizeof(arm_lpae_iopte)))

Didn't see this being used?

> +typedef u64 arm_lpae_iopte;

Only used by the defines and the above print.

Jason