[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 5 May 2022 20:12:24 +0800
From: Baolu Lu <baolu.lu@...ux.intel.com>
To: "Tian, Kevin" <kevin.tian@...el.com>,
Joerg Roedel <joro@...tes.org>,
Jason Gunthorpe <jgg@...dia.com>,
Alex Williamson <alex.williamson@...hat.com>
Cc: "Pan, Jacob jun" <jacob.jun.pan@...el.com>,
"Liu, Yi L" <yi.l.liu@...el.com>,
"iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/4] iommu/vt-d: Check domain force_snooping against
attached devices
On 2022/5/5 16:43, Tian, Kevin wrote:
>> From: Lu Baolu <baolu.lu@...ux.intel.com>
>> Sent: Thursday, May 5, 2022 9:07 AM
>>
>> As domain->force_snooping only impacts the devices attached with the
>> domain, there's no need to check against all IOMMU units. At the same
>> time, for a brand new domain (hasn't been attached to any device), the
>> force_snooping field could be set, but the attach_dev callback will
>> return failure if it wants to attach to a device which IOMMU has no
>> snoop control capability.
>
> The description about brand new domain is not very clear. I think the
> point here is that force_snooping could be set on a domain no matter
> whether it has been attached or not and once set it is an immutable
> flag. If no device attached the operation always succeeds then this
> empty domain can be only attached to a device of which the IOMMU
> supports snoop control.
Exactly. Will update this description.
>
>> static bool intel_iommu_enforce_cache_coherency(struct iommu_domain
>> *domain)
>> {
>> struct dmar_domain *dmar_domain = to_dmar_domain(domain);
>>
>> - if (!domain_update_iommu_snooping(NULL))
>> + if (dmar_domain->force_snooping)
>> + return true;
>> +
>> + if (!domain_support_force_snooping(dmar_domain))
>> return false;
>> +
>
> Who guarantees that domain->devices won't change between
> above condition check and following set operation?
Good catch. Should lift the lock up here.
>
>> + domain_set_force_snooping(dmar_domain);
>> dmar_domain->force_snooping = true;
>> +
>> return true;
>> }
>>
>
> Thanks
> Kevin
Best regards,
baolu
Powered by blists - more mailing lists