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: <BN9PR11MB5276DEEEA205B267192FC01B8C079@BN9PR11MB5276.namprd11.prod.outlook.com>
Date:   Wed, 16 Nov 2022 09:15:15 +0000
From:   "Tian, Kevin" <kevin.tian@...el.com>
To:     Baolu Lu <baolu.lu@...ux.intel.com>,
        "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>
CC:     Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>,
        "Robin Murphy" <robin.murphy@....com>,
        "Liu, Yi L" <yi.l.liu@...el.com>,
        "Pan, Jacob jun" <jacob.jun.pan@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v3 4/7] iommu/vt-d: Fold dmar_remove_one_dev_info() into
 its caller

> From: Baolu Lu <baolu.lu@...ux.intel.com>
> Sent: Wednesday, November 16, 2022 4:03 PM
> 
> On 2022/11/16 13:35, Tian, Kevin wrote:
> >> From: Baolu Lu<baolu.lu@...ux.intel.com>
> >> Sent: Wednesday, November 16, 2022 12:36 PM
> >>
> >> On 11/16/22 11:53 AM, Tian, Kevin wrote:
> >>>> From: Lu Baolu<baolu.lu@...ux.intel.com>
> >>>> Sent: Monday, November 14, 2022 9:41 AM
> >>>> @@ -4562,7 +4538,10 @@ static void
> intel_iommu_release_device(struct
> >>>> device *dev)
> >>>>    {
> >>>>    	struct device_domain_info *info = dev_iommu_priv_get(dev);
> >>>>
> >>>> -	dmar_remove_one_dev_info(dev);
> >>>> +	iommu_disable_pci_caps(info);
> >>>> +	domain_context_clear(info);
> >>>> +	device_block_translation(dev);
> >>> clear context after blocking translation.
> >> Unfortunately domain_context_clear() needs reference to info->domain
> >> (for domain id when flushing cache), which is cleared in
> >> device_block_translation().
> >>
> > this sounds an ordering problem. clearing context should be after
> > blocking translation in concept.
> 
> At present, when the default domain is attached to the device, we first
> populate the pasid table entry, and then populate the device context
> entry. Above code is just the reverse operation.
> 
> Can you see any practical problems caused by this sequence? If so, it
> seems that we should carefully consider whether such problems already
> exist.
> 

there is no problem with existing code. Just after this patch the order
looks weird based on the literal name of those functions.

domain_context_clear() is a big hammer to disable the context entry,
implying translation must be blocked. Then calling another block
translation afterwards becomes unnecessary.

Probably it should be split into two functions with one requiring 
info->domain called before block translation and the rest which
actually clears the context entry being the last step?

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ