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
| ||
|
Message-ID: <20221215101303.6rezz5mqjwupdaqe@skbuf> Date: Thu, 15 Dec 2022 12:13:03 +0200 From: Vladimir Oltean <vladimir.oltean@....com> To: Robin Murphy <robin.murphy@....com> Cc: iommu@...ts.linux.dev, Will Deacon <will@...nel.org>, Joerg Roedel <joro@...tes.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, Michael Walle <michael@...le.cc>, Laurentiu Tudor <laurentiu.tudor@....com>, Claudiu Manoil <claudiu.manoil@....com>, netdev@...r.kernel.org Subject: Re: [PATCH] iommu/arm-smmu: don't unregister on shutdown On Wed, Dec 14, 2022 at 09:25:45PM +0000, Robin Murphy wrote: > > > The change itself looks sensible. The point of this shutdown hook is simply > > > not to leave active translations in place that might confuse future software > > > after reboot/kexec; any housekeeping in the current kernel state is a waste > > > of time anyway. Fancy doing the same for SMMUv3 as well? > > > > I can try, but I won't have hardware to test. > > > > Basically the only thing truly relevant for shutdown from arm_smmu_device_remove() > > is arm_smmu_device_disable(), would you agree to a patch which changes > > things as below? > > > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > > index 6d5df91c5c46..d4d8bfee9feb 100644 > > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > > @@ -3854,7 +3854,9 @@ static int arm_smmu_device_remove(struct platform_device *pdev) > > static void arm_smmu_device_shutdown(struct platform_device *pdev) > > { > > - arm_smmu_device_remove(pdev); > > + struct arm_smmu_device *smmu = platform_get_drvdata(pdev); > > + > > + arm_smmu_device_disable(smmu); > > } > > static const struct of_device_id arm_smmu_of_match[] = { > > > Looks fine to me! I'll let Will decide if he'd still prefer to do the full > remove-calls-shutdown reversal here as well for complete consistency, but I > reckon the minimal diff is no bad thing :) The reason why I did it this way is that if remove() still called shutdown(), it would have looked like this here: static void arm_smmu_device_shutdown(struct platform_device *pdev) { struct arm_smmu_device *smmu = platform_get_drvdata(pdev); arm_smmu_device_disable(smmu); } static int arm_smmu_device_remove(struct platform_device *pdev) { struct arm_smmu_device *smmu = platform_get_drvdata(pdev); iommu_device_unregister(&smmu->iommu); iommu_device_sysfs_remove(&smmu->iommu); arm_smmu_device_shutdown(pdev); iopf_queue_free(smmu->evtq.iopf); return 0; } Not really that beneficial. I also didn't want to reorder any operations, they seem to be done in reverse order of what is being done in arm_smmu_device_probe().
Powered by blists - more mailing lists