linux-kernel - Re: [git pull] IOMMU Updates for Linux v6.4

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: <CAHk-=wiriLmq6OgLLF9seANqCJqjCrgUC384zcJUFtv3xJgVkQ@mail.gmail.com>
Date:   Sun, 30 Apr 2023 13:07:45 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Joerg Roedel <joro@...tes.org>, Jason Gunthorpe <jgg@...pe.ca>
Cc:     Will Deacon <will@...nel.org>, linux-kernel@...r.kernel.org,
        iommu@...ts.linux.dev
Subject: Re: [git pull] IOMMU Updates for Linux v6.4

On Sun, Apr 30, 2023 at 4:13 AM Joerg Roedel <joro@...tes.org> wrote:
>
> this pull-request is somewhat messier than usual because it has a lot of
> conflicts with your tree. I resolved them in a test-merge and sorted it out
> for you to compare your solution to mine (mine is also mostly similar to
> the one in linux-next).

Your resolution is different from mine.

Some of it is just white-space differences etc, but some of it is meaningful.

For example, you have

                if (mm->pasid < min || mm->pasid >= max)

in your iommu_sva_alloc_pasid(), which seems to have undone the change
in commit 4e14176ab13f ("iommu/sva: Stop using ioasid_set for SVA"),
which changed it to check for

           .. mm->pasid > max)

instead (which seems also consistent with what ida_alloc_range() does:
'max' is inclusive).

You also seem to have kept the deleted <linux/ioasid.h> header file.

I'm also a bit unsure about what the intent with mm_valid_pasid() is.
In commit cd3891158a77 ("iommu/sva: Move PASID helpers to sva code")
that helper (under the previous name) got moved to a different header
file, but in the process it also got done unconditionally as

  static inline bool pasid_valid(ioasid_t ioasid)
  {
         return ioasid != INVALID_IOASID;
  }

and didn't have a "ioasid is disabled in the config, so have an
alternate helper that always returns false".

But in your merge, you ended up splitting it into two versions again.

I don't think that's technically the "right" merge (it basically
changes things wrt the two branches), but I do think it's nicer.

So I edited my merge to follow that lead.

Finally, I'm not happy with the Kconfig situation here. Commit
99b5726b4423 ("iommu: Remove ioasid infrastructure") removed
CONFIG_IOASID, but left the

        select IOASID

in the 'config INTEL_IOMMU' Kconfig case. I removed that as dead, but
now we have that

        select IOMMU_SVA

in the 'config INTEL_IOMMU_SVM' case instead. So it's a very different
Kconfig setup.

Anyway, I'm not super-happy with how this all turned out. The example
merge seems to be wrong, and the Kconfig situation is confusing.

Somebody should double-check my result, in other words.

                Linus