linux-kernel - RE: [PATCH 0/2] Fix issues with untrusted devices and AMD IOMMU

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:   Mon, 4 Apr 2022 17:05:00 +0000
From:   "Limonciello, Mario" <Mario.Limonciello@....com>
To:     Christoph Hellwig <hch@...radead.org>
CC:     Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Robin Murphy <robin.murphy@....com>,
        "open list:IOMMU DRIVERS" <iommu@...ts.linux-foundation.org>,
        "Suthikulpanit, Suravee" <Suravee.Suthikulpanit@....com>,
        "Hegde, Vasant" <Vasant.Hegde@....com>,
        open list <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 0/2] Fix issues with untrusted devices and AMD IOMMU

[AMD Official Use Only]

> On Mon, Apr 04, 2022 at 11:47:05AM -0500, Mario Limonciello wrote:
> > The bounce buffers were originally set up, but torn down during
> > the boot process.
> > * This happens because as part of IOMMU initialization
> >   `amd_iommu_init_dma_ops` gets called and resets the global swiotlb to 0.
> > * When late_init gets called `pci_swiotlb_late_init` `swiotlb_exit` is
> >   called and the buffers are torn down.
> 
> I think the proper thing is to do this:
> 
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index a1ada7bff44e6..079694f894b85 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -1838,17 +1838,10 @@ void amd_iommu_domain_update(struct
> protection_domain *domain)
>  	amd_iommu_domain_flush_complete(domain);
>  }
> 
> -static void __init amd_iommu_init_dma_ops(void)
> -{
> -	swiotlb = (iommu_default_passthrough() || sme_me_mask) ? 1 : 0;
> -}
> -
>  int __init amd_iommu_init_api(void)
>  {
>  	int err;
> 
> -	amd_iommu_init_dma_ops();
> -
>  	err = bus_set_iommu(&pci_bus_type, &amd_iommu_ops);
>  	if (err)
>  		return err;

I do expect that solves it as well.  The reason I submitted the way I did is
that there seemed to be a strong affinity for having swiotlb disabled when IOMMU
is enabled on AMD IOMMU.  The original code that disabled SWIOTLB in AMD IOMMU
dates all the way back to 2.6.33 (commit 75f1cdf1dda92cae037ec848ae63690d91913eac)
and it has ping ponged around since then to add more criteria that it would be or wouldn't
be disabled, but was never actually dropped until your suggestion.

If the consensus is that it should be dropped, I'll validate it does help and then send that out
as a v2.  I do think that my messaging patch (1/2) may still be useful for debugging in the
future if for another reason SWIOTLB is disabled.