linux-kernel - Re: [PATCH 1/3] iommu/vt-d: Setup scalable mode context entry in probe path

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: <3ad0e349-0ba4-4a89-a664-800c5615ef77@linux.intel.com>
Date: Mon, 4 Mar 2024 16:23:35 +0800
From: Baolu Lu <baolu.lu@...ux.intel.com>
To: "Tian, Kevin" <kevin.tian@...el.com>, Joerg Roedel <joro@...tes.org>,
 Will Deacon <will@...nel.org>, Robin Murphy <robin.murphy@....com>,
 Jason Gunthorpe <jgg@...pe.ca>, "Badger, Eric" <ebadger@...estorage.com>
Cc: baolu.lu@...ux.intel.com, "iommu@...ts.linux.dev"
 <iommu@...ts.linux.dev>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] iommu/vt-d: Setup scalable mode context entry in
 probe path

On 2024/3/4 15:48, Tian, Kevin wrote:
>> From: Lu Baolu <baolu.lu@...ux.intel.com>
>> Sent: Thursday, February 29, 2024 5:48 PM
>>
>> +static int device_pasid_table_setup(struct device *dev, u8 bus, u8 devfn)
>> +{
>> +	struct device_domain_info *info = dev_iommu_priv_get(dev);
>> +	struct intel_iommu *iommu = info->iommu;
>> +	struct context_entry *context;
>> +
>> +	spin_lock(&iommu->lock);
>> +	context = iommu_context_addr(iommu, bus, devfn, true);
>> +	if (!context) {
>> +		spin_unlock(&iommu->lock);
>> +		return -ENOMEM;
>> +	}
>> +
>> +	if (context_present(context) && !context_copied(iommu, bus, devfn))
>> {
>> +		spin_unlock(&iommu->lock);
>> +		return 0;
>> +	}
>> +
>> +	/*
>> +	 * For kdump case, at this point, the device is supposed to finish
>> +	 * reset at its driver probe stage, so no in-flight DMA will exist,
>> +	 * and we don't need to worry anymore hereafter.
>> +	 */
>> +	if (context_copied(iommu, bus, devfn)) {
>> +		context_clear_entry(context);
>> +		if (!ecap_coherent(iommu->ecap))
>> +			clflush_cache_range(context, sizeof(*context));
>> +		sm_context_flush_caches(dev);
>> +		clear_context_copied(iommu, bus, devfn);
>> +	}
> 
> it's unclear to me why this doesn't need refer to old did as done in the
> existing code. If scalable mode makes any difference could you extend
> the comment to explain so people can avoid similar confusion when
> comparing the different paths between legacy and sm?

The previous code gets the domain ID from the copied context entry:

u16 did_old = context_domain_id(context);

This makes no sense for scalable mode, as the domain ID has been moved
to the PASID entry in scalable mode. As the result, did_old always gets
0.

> anyway it's kind of a semantics change probably worth a separate patch
> to special case sm for bisect and then doing cleanup...

This change doesn't impact anything as the hardware will skip domain id
field in the Context-cache Invalidate Descriptor in scalable mode.

Spec section 6.5.2.1 Context-cache Invalidate Descriptor:

Domain-ID (DID): ... This field is ignored by hardware when operating in
scalable mode (RTADDR_REG.TTM=01b).

Best regards,
baolu