linux-kernel - Re: [PATCH v3 02/12] iommu/vt-d: Add cache tag invalidation helpers

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]

Date: Wed, 24 Apr 2024 09:45:56 +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>
Cc: baolu.lu@...ux.intel.com, "Zhang, Tina" <tina.zhang@...el.com>,
 "Liu, Yi L" <yi.l.liu@...el.com>,
 "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 02/12] iommu/vt-d: Add cache tag invalidation helpers

On 4/23/24 4:42 PM, Tian, Kevin wrote:
>> From: Baolu Lu<baolu.lu@...ux.intel.com>
>> Sent: Monday, April 22, 2024 1:30 PM
>>
>> On 4/16/24 4:06 PM, Lu Baolu wrote:
>>> +		case CACHE_TAG_NESTING_DEVTLB:
>>> +			/*
>>> +			 * Address translation cache in device side caches the
>>> +			 * result of nested translation. There is no easy way
>>> +			 * to identify the exact set of nested translations
>>> +			 * affected by a change in S2. So just flush the entire
>>> +			 * device cache.
>>> +			 */
>>> +			addr = 0;
>>> +			mask = MAX_AGAW_PFN_WIDTH;
>>> +			fallthrough;
>> I realized that the logic above is not right. Setting both @addr and
>> @mask to 0 doesn't means flush all caches on the device. I will change
>> it like below:
> I didn't get. Above code doesn't set @mask to 0.

Oh!? I have no idea why I read that as "mask = 0" now. Perhaps my brain
was on vacation earlier. :-)

> 
>> diff --git a/drivers/iommu/intel/cache.c b/drivers/iommu/intel/cache.c
>> index e8418cdd8331..18debb82272a 100644
>> --- a/drivers/iommu/intel/cache.c
>> +++ b/drivers/iommu/intel/cache.c
>> @@ -302,9 +302,14 @@ void cache_tag_flush_range(struct dmar_domain
>> *domain, unsigned long start,
>>                            * affected by a change in S2. So just flush
>> the entire
>>                            * device cache.
>>                            */
>> -                       addr = 0;
>> -                       mask = MAX_AGAW_PFN_WIDTH;
>> -                       fallthrough;
>> +                       info = dev_iommu_priv_get(tag->dev);
>> +                       sid = PCI_DEVID(info->bus, info->devfn);
>> +
>> +                       qi_flush_dev_iotlb(iommu, sid, info->pfsid,
>> info->ats_qdep,
>> +                                          0, MAX_AGAW_PFN_WIDTH);
>> +                       quirk_extra_dev_tlb_flush(info, 0,
>> MAX_AGAW_PFN_WIDTH,
>> +                                                 IOMMU_NO_PASID,
>> info->ats_qdep);
>> +                       break;
> and I didn't get this change. It goes backward by ignoring tag->pasid.
> 
> what's the exact problem of the fallthrough logic in original code?

Sorry! Please ignore this.

Best regards,
baolu