linux-kernel - Re: [PATCH v5 02/12] iommu/vt-d: Manage scalalble mode PASID tables

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: <20181203134411.lejlkbnagxml54ro@8bytes.org>
Date:   Mon, 3 Dec 2018 14:44:12 +0100
From:   Joerg Roedel <joro@...tes.org>
To:     Lu Baolu <baolu.lu@...ux.intel.com>
Cc:     David Woodhouse <dwmw2@...radead.org>, ashok.raj@...el.com,
        sanjay.k.kumar@...el.com, jacob.jun.pan@...el.com,
        kevin.tian@...el.com, yi.l.liu@...el.com, yi.y.sun@...el.com,
        peterx@...hat.com,
        Jean-Philippe Brucker <jean-philippe.brucker@....com>,
        iommu@...ts.linux-foundation.org, linux-kernel@...r.kernel.org,
        Jacob Pan <jacob.jun.pan@...ux.intel.com>
Subject: Re: [PATCH v5 02/12] iommu/vt-d: Manage scalalble mode PASID tables

Hi Baolu,

On Wed, Nov 28, 2018 at 11:54:39AM +0800, Lu Baolu wrote:
> @@ -2482,12 +2482,13 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu,
>  	if (dev)
>  		dev->archdata.iommu = info;
>  
> -	if (dev && dev_is_pci(dev) && info->pasid_supported) {
> +	/* PASID table is mandatory for a PCI device in scalable mode. */
> +	if (dev && dev_is_pci(dev) && sm_supported(iommu)) {

This will also allocate a PASID table if the device does not support
PASIDs, right? Will the table not be used in that case or will the
device just use the fallback PASID? Isn't it better in that case to have
no PASID table?

> @@ -143,18 +143,20 @@ int intel_pasid_alloc_table(struct device *dev)
>  		return -ENOMEM;
>  	INIT_LIST_HEAD(&pasid_table->dev);
>  
> -	size = sizeof(struct pasid_entry);
> -	count = min_t(int, pci_max_pasids(to_pci_dev(dev)), intel_pasid_max_id);
> -	order = get_order(size * count);
> +	if (info->pasid_supported)
> +		max_pasid = min_t(int, pci_max_pasids(to_pci_dev(dev)),
> +				  intel_pasid_max_id);
> +
> +	size = max_pasid >> (PASID_PDE_SHIFT - 3);
> +	order = size ? get_order(size) : 0;
>  	pages = alloc_pages_node(info->iommu->node,
> -				 GFP_ATOMIC | __GFP_ZERO,
> -				 order);
> +				 GFP_ATOMIC | __GFP_ZERO, order);

This is a simple data structure allocation path, does it need
GFP_ATOMIC?



	Joerg