linux-kernel - Re: [PATH v5 3/3] iommu/vt-d: debugfs: Support dumping a specified page table

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fd56b69d-d9ea-411c-9eaa-454b3d64f51e@linux.intel.com>
Date:   Mon, 16 Oct 2023 11:04:25 +0800
From:   Baolu Lu <baolu.lu@...ux.intel.com>
To:     Jingqi Liu <Jingqi.liu@...el.com>, iommu@...ts.linux.dev,
        Tian Kevin <kevin.tian@...el.com>,
        Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>,
        Robin Murphy <robin.murphy@....com>
Cc:     baolu.lu@...ux.intel.com, linux-kernel@...r.kernel.org
Subject: Re: [PATH v5 3/3] iommu/vt-d: debugfs: Support dumping a specified
 page table

On 10/13/23 9:58 PM, Jingqi Liu wrote:
> The original debugfs only dumps all page tables without pasid. With
> pasid supported, the page table with pasid also needs to be dumped.
> 
> This patch supports dumping a specified page table in legacy mode or
> scalable mode with or without a specified pasid.
> 
> For legacy mode, according to bus number and DEVFN, traverse the root
> table and context table to get the pointer of page table in the
> context table entry, then dump the specified page table.
> 
> For scalable mode, according to bus number, DEVFN and pasid, traverse
> the root table, context table, pasid directory and pasid table to get
> the pointer of page table in the pasid table entry, then dump the
> specified page table..
> 
> Examples are as follows:
> 1) Dump the page table of device "0000:00:1f.0" that only supports
>     legacy mode.
>     $ sudo cat
>     /sys/kernel/debug/iommu/intel/0000:00:1f.0/domain_translation_struct
> 
> 2) Dump the page table of device "0000:00:0a.0" with PASID "1" that
>     supports scalable mode.
>     $ sudo cat
>     /sys/kernel/debug/iommu/intel/0000:00:0a.0/1/domain_translation_struct
> 
> Suggested-by: Kevin Tian <kevin.tian@...el.com>
> Signed-off-by: Jingqi Liu <Jingqi.liu@...el.com>
> ---
>   drivers/iommu/intel/debugfs.c | 154 ++++++++++++++++++++++++++--------
>   1 file changed, 120 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/iommu/intel/debugfs.c b/drivers/iommu/intel/debugfs.c
> index 8a18a7be5215..239136727ac4 100644
> --- a/drivers/iommu/intel/debugfs.c
> +++ b/drivers/iommu/intel/debugfs.c
> @@ -347,58 +347,141 @@ static void pgtable_walk_level(struct seq_file *m, struct dma_pte *pde,
>   	}
>   }
>   
> -static int __show_device_domain_translation(struct device *dev, void *data)
> +static int domain_translation_struct_show(struct seq_file *m,
> +					  struct device_domain_info *info,
> +					  ioasid_t pasid)
>   {
> -	struct dmar_domain *domain;
> -	struct seq_file *m = data;
> -	u64 path[6] = { 0 };
> -
> -	domain = to_dmar_domain(iommu_get_domain_for_dev(dev));
> -	if (!domain)
> -		return 0;
> +	bool scalable, found = false;
> +	struct dmar_drhd_unit *drhd;
> +	struct intel_iommu *iommu;
> +	u16 devfn, bus, seg;
>   
> -	seq_printf(m, "Device %s @0x%llx\n", dev_name(dev),
> -		   (u64)virt_to_phys(domain->pgd));
> -	seq_puts(m, "IOVA_PFN\t\tPML5E\t\t\tPML4E\t\t\tPDPE\t\t\tPDE\t\t\tPTE\n");
> +	bus = info->bus;
> +	devfn = info->devfn;
> +	seg = info->segment;
>   
> -	pgtable_walk_level(m, domain->pgd, domain->agaw + 2, 0, path);
> -	seq_putc(m, '\n');
> +	rcu_read_lock();
> +	for_each_active_iommu(iommu, drhd) {
> +		struct context_entry *context;
> +		u64 pgd, path[6] = { 0 };
> +		u32 sts, agaw;
>   
> -	/* Don't iterate */
> -	return 1;
> -}
> +		if (seg != iommu->segment)
> +			continue;
>   
> -static int show_device_domain_translation(struct device *dev, void *data)
> -{
> -	struct iommu_group *group;
> +		sts = dmar_readl(iommu->reg + DMAR_GSTS_REG);
> +		if (!(sts & DMA_GSTS_TES)) {
> +			seq_printf(m, "DMA Remapping is not enabled on %s\n",
> +				   iommu->name);
> +			continue;
> +		}
> +		if (dmar_readq(iommu->reg + DMAR_RTADDR_REG) & DMA_RTADDR_SMT)
> +			scalable = true;
> +		else
> +			scalable = false;
>   
> -	group = iommu_group_get(dev);
> -	if (group) {
>   		/*
> -		 * The group->mutex is held across the callback, which will
> -		 * block calls to iommu_attach/detach_group/device. Hence,
> +		 * The iommu->lock is held across the callback, which will
> +		 * block calls to domain_attach/domain_detach. Hence,
>   		 * the domain of the device will not change during traversal.
>   		 *
> -		 * All devices in an iommu group share a single domain, hence
> -		 * we only dump the domain of the first device. Even though,
> -		 * this code still possibly races with the iommu_unmap()
> +		 * Traversing page table possibly races with the iommu_unmap()
>   		 * interface. This could be solved by RCU-freeing the page
>   		 * table pages in the iommu_unmap() path.
>   		 */
> -		iommu_group_for_each_dev(group, data,
> -					 __show_device_domain_translation);
> -		iommu_group_put(group);
> +		spin_lock(&iommu->lock);
> +
> +		context = iommu_context_addr(iommu, bus, devfn, 0);
> +		if (!context || !context_present(context))
> +			goto iommu_unlock;
> +
> +		if (scalable) {	/* scalable mode */
> +			struct pasid_dir_entry *dir_tbl, *dir_entry;
> +			struct pasid_entry *pasid_tbl, *pasid_tbl_entry;
> +			u16 pasid_dir_size, dir_idx, tbl_idx, pgtt;
> +			u64 pasid_dir_ptr;

0day robot complained:

    drivers/iommu/intel/debugfs.c: In function 
'domain_translation_struct_show':
 >> drivers/iommu/intel/debugfs.c:401:29: warning: variable 
'pasid_dir_size' set but not used [-Wunused-but-set-variable]
      401 |                         u16 pasid_dir_size, dir_idx, 
tbl_idx, pgtt;
          |                             ^~~~~~~~~~~~~~

I have removed pasid_dir_size and queue the whole series for v6.7.

Thank you!

Best regards,
baolu