[v10,1/6] iommu: Change kconfig around IOMMU_SVA
Commit Message
From: Jason Gunthorpe <jgg@nvidia.com>
Linus suggested that the kconfig here is confusing:
https://lore.kernel.org/all/CAHk-=wgUiAtiszwseM1p2fCJ+sC4XWQ+YN4TanFhUgvUqjr9Xw@mail.gmail.com/
Let's break it into three kconfigs controlling distinct things:
- CONFIG_IOMMU_MM_DATA controls if the mm_struct has the additional
fields for the IOMMU. Currently only PASID, but later patches store
a struct iommu_mm_data *
- CONFIG_ARCH_HAS_CPU_PASID controls if the arch needs the scheduling bit
for keeping track of the ENQCMD instruction. x86 will select this if
IOMMU_SVA is enabled
- IOMMU_SVA controls if the IOMMU core compiles in the SVA support code
for iommu driver use and the IOMMU exported API
This way ARM will not enable CONFIG_ARCH_HAS_CPU_PASID
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
arch/Kconfig | 5 +++++
arch/x86/Kconfig | 1 +
arch/x86/kernel/traps.c | 2 +-
drivers/iommu/Kconfig | 1 +
include/linux/iommu.h | 2 +-
include/linux/mm_types.h | 2 +-
include/linux/sched.h | 2 +-
kernel/fork.c | 2 +-
mm/Kconfig | 3 +++
mm/init-mm.c | 2 +-
10 files changed, 16 insertions(+), 6 deletions(-)
Comments
Hi Tina,
On Fri, Oct 27, 2023 at 08:05:20AM +0800, Tina Zhang wrote:
> From: Jason Gunthorpe <jgg@nvidia.com>
>
> Linus suggested that the kconfig here is confusing:
While this patch looks decent to me, you forgot to Cc Linus on it. In
general, given that it touches a couple of core files, I'd like to wait
for some more people to have a look at it and not rushing anything in.
>
> https://lore.kernel.org/all/CAHk-=wgUiAtiszwseM1p2fCJ+sC4XWQ+YN4TanFhUgvUqjr9Xw@mail.gmail.com/
>
> Let's break it into three kconfigs controlling distinct things:
>
> - CONFIG_IOMMU_MM_DATA controls if the mm_struct has the additional
> fields for the IOMMU. Currently only PASID, but later patches store
> a struct iommu_mm_data *
>
> - CONFIG_ARCH_HAS_CPU_PASID controls if the arch needs the scheduling bit
> for keeping track of the ENQCMD instruction. x86 will select this if
> IOMMU_SVA is enabled
>
> - IOMMU_SVA controls if the IOMMU core compiles in the SVA support code
> for iommu driver use and the IOMMU exported API
>
> This way ARM will not enable CONFIG_ARCH_HAS_CPU_PASID
>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
If you send it, you also need to add your Signed-off-by.
Regards,
Joerg
Hi Joerg,
> -----Original Message-----
> From: Joerg Roedel <joro@8bytes.org>
> Sent: Friday, October 27, 2023 3:12 PM
> To: Zhang, Tina <tina.zhang@intel.com>
> Cc: iommu@lists.linux.dev; linux-kernel@vger.kernel.org; David Woodhouse
> <dwmw2@infradead.org>; Lu Baolu <baolu.lu@linux.intel.com>; Will Deacon
> <will@kernel.org>; Robin Murphy <robin.murphy@arm.com>; Jason
> Gunthorpe <jgg@ziepe.ca>; Tian, Kevin <kevin.tian@intel.com>; Nicolin Chen
> <nicolinc@nvidia.com>; Michael Shavit <mshavit@google.com>; Vasant
> Hegde <vasant.hegde@amd.com>; Jason Gunthorpe <jgg@nvidia.com>
> Subject: Re: [PATCH v10 1/6] iommu: Change kconfig around IOMMU_SVA
>
> Hi Tina,
>
> On Fri, Oct 27, 2023 at 08:05:20AM +0800, Tina Zhang wrote:
> > From: Jason Gunthorpe <jgg@nvidia.com>
> >
> > Linus suggested that the kconfig here is confusing:
>
> While this patch looks decent to me, you forgot to Cc Linus on it. In general,
> given that it touches a couple of core files, I'd like to wait for some more
> people to have a look at it and not rushing anything in.
Make sense. I'll CC Linus. Comments are welcome.
> >
> > https://lore.kernel.org/all/CAHk-
> =wgUiAtiszwseM1p2fCJ+sC4XWQ+YN4TanFhU
> > gvUqjr9Xw@mail.gmail.com/
> >
> > Let's break it into three kconfigs controlling distinct things:
> >
> > - CONFIG_IOMMU_MM_DATA controls if the mm_struct has the additional
> > fields for the IOMMU. Currently only PASID, but later patches store
> > a struct iommu_mm_data *
> >
> > - CONFIG_ARCH_HAS_CPU_PASID controls if the arch needs the scheduling
> bit
> > for keeping track of the ENQCMD instruction. x86 will select this if
> > IOMMU_SVA is enabled
> >
> > - IOMMU_SVA controls if the IOMMU core compiles in the SVA support
> code
> > for iommu driver use and the IOMMU exported API
> >
> > This way ARM will not enable CONFIG_ARCH_HAS_CPU_PASID
> >
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
>
> If you send it, you also need to add your Signed-off-by.
OK. I'll add my Signed-off-by.
Thanks,
-Tina
>
> Regards,
>
> Joerg
Hi Joerg and Jason,
> -----Original Message-----
> From: Zhang, Tina
> Sent: Friday, October 27, 2023 3:37 PM
> To: Joerg Roedel <joro@8bytes.org>
> Cc: iommu@lists.linux.dev; linux-kernel@vger.kernel.org; David Woodhouse
> <dwmw2@infradead.org>; Lu Baolu <baolu.lu@linux.intel.com>; Will Deacon
> <will@kernel.org>; Robin Murphy <robin.murphy@arm.com>; Jason
> Gunthorpe <jgg@ziepe.ca>; Tian, Kevin <kevin.tian@intel.com>; Nicolin Chen
> <nicolinc@nvidia.com>; Michael Shavit <mshavit@google.com>; Vasant
> Hegde <vasant.hegde@amd.com>; Jason Gunthorpe <jgg@nvidia.com>
> Subject: RE: [PATCH v10 1/6] iommu: Change kconfig around IOMMU_SVA
>
> Hi Joerg,
>
> > -----Original Message-----
> > From: Joerg Roedel <joro@8bytes.org>
> > Sent: Friday, October 27, 2023 3:12 PM
> > To: Zhang, Tina <tina.zhang@intel.com>
> > Cc: iommu@lists.linux.dev; linux-kernel@vger.kernel.org; David
> > Woodhouse <dwmw2@infradead.org>; Lu Baolu
> <baolu.lu@linux.intel.com>;
> > Will Deacon <will@kernel.org>; Robin Murphy <robin.murphy@arm.com>;
> > Jason Gunthorpe <jgg@ziepe.ca>; Tian, Kevin <kevin.tian@intel.com>;
> > Nicolin Chen <nicolinc@nvidia.com>; Michael Shavit
> > <mshavit@google.com>; Vasant Hegde <vasant.hegde@amd.com>; Jason
> > Gunthorpe <jgg@nvidia.com>
> > Subject: Re: [PATCH v10 1/6] iommu: Change kconfig around IOMMU_SVA
> >
> > Hi Tina,
> >
> > On Fri, Oct 27, 2023 at 08:05:20AM +0800, Tina Zhang wrote:
> > > From: Jason Gunthorpe <jgg@nvidia.com>
> > >
> > > Linus suggested that the kconfig here is confusing:
> >
> > While this patch looks decent to me, you forgot to Cc Linus on it. In
> > general, given that it touches a couple of core files, I'd like to
> > wait for some more people to have a look at it and not rushing anything in.
> Make sense. I'll CC Linus. Comments are welcome.
Can we separate this patch? I'm thinking about CC more people for review. At least, we need to CC all the reviewers of https://lore.kernel.org/all/20230506133134.1492395-1-jacob.jun.pan@linux.intel.com/ to this patch. So, it seems more reasonable to separate this patch. What do you think?
Regards,
-Tina
On Fri, Oct 27, 2023 at 09:14:17AM +0000, Zhang, Tina wrote:
> > > On Fri, Oct 27, 2023 at 08:05:20AM +0800, Tina Zhang wrote:
> > > > From: Jason Gunthorpe <jgg@nvidia.com>
> > > >
> > > > Linus suggested that the kconfig here is confusing:
> > >
> > > While this patch looks decent to me, you forgot to Cc Linus on it. In
> > > general, given that it touches a couple of core files, I'd like to
> > > wait for some more people to have a look at it and not rushing anything in.
> > Make sense. I'll CC Linus. Comments are welcome.
> Can we separate this patch? I'm thinking about CC more people for
> review. At least, we need to CC all the reviewers of
> https://lore.kernel.org/all/20230506133134.1492395-1-jacob.jun.pan@linux.intel.com/
> to this patch. So, it seems more reasonable to separate this
> patch. What do you think?
I think that makes sense, let's go ahead with the rest of the SVA
series and we can do this next cycle. Bikeshedding config names isn't
really critical.
Jason
On Fri, Oct 27, 2023 at 08:20:13AM -0300, Jason Gunthorpe wrote:
> On Fri, Oct 27, 2023 at 09:14:17AM +0000, Zhang, Tina wrote:
> > > > On Fri, Oct 27, 2023 at 08:05:20AM +0800, Tina Zhang wrote:
> > > > > From: Jason Gunthorpe <jgg@nvidia.com>
> > > > >
> > > > > Linus suggested that the kconfig here is confusing:
> > > >
> > > > While this patch looks decent to me, you forgot to Cc Linus on it. In
> > > > general, given that it touches a couple of core files, I'd like to
> > > > wait for some more people to have a look at it and not rushing anything in.
> > > Make sense. I'll CC Linus. Comments are welcome.
>
> > Can we separate this patch? I'm thinking about CC more people for
> > review. At least, we need to CC all the reviewers of
> > https://lore.kernel.org/all/20230506133134.1492395-1-jacob.jun.pan@linux.intel.com/
> > to this patch. So, it seems more reasonable to separate this
> > patch. What do you think?
>
> I think that makes sense, let's go ahead with the rest of the SVA
> series and we can do this next cycle. Bikeshedding config names isn't
> really critical.
So this didn't make it even without the config change :(
Can you please resend it again and CC the right please please. This
needs to go into this cycle for sure
Thanks
Jason
> -----Original Message-----
> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Tuesday, November 28, 2023 2:25 AM
> To: Zhang, Tina <tina.zhang@intel.com>
> Cc: Joerg Roedel <joro@8bytes.org>; iommu@lists.linux.dev; linux-
> kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>; Lu
> Baolu <baolu.lu@linux.intel.com>; Will Deacon <will@kernel.org>; Robin
> Murphy <robin.murphy@arm.com>; Tian, Kevin <kevin.tian@intel.com>;
> Nicolin Chen <nicolinc@nvidia.com>; Michael Shavit <mshavit@google.com>;
> Vasant Hegde <vasant.hegde@amd.com>
> Subject: Re: [PATCH v10 1/6] iommu: Change kconfig around IOMMU_SVA
>
> On Fri, Oct 27, 2023 at 08:20:13AM -0300, Jason Gunthorpe wrote:
> > On Fri, Oct 27, 2023 at 09:14:17AM +0000, Zhang, Tina wrote:
> > > > > On Fri, Oct 27, 2023 at 08:05:20AM +0800, Tina Zhang wrote:
> > > > > > From: Jason Gunthorpe <jgg@nvidia.com>
> > > > > >
> > > > > > Linus suggested that the kconfig here is confusing:
> > > > >
> > > > > While this patch looks decent to me, you forgot to Cc Linus on
> > > > > it. In general, given that it touches a couple of core files,
> > > > > I'd like to wait for some more people to have a look at it and not rushing
> anything in.
> > > > Make sense. I'll CC Linus. Comments are welcome.
> >
> > > Can we separate this patch? I'm thinking about CC more people for
> > > review. At least, we need to CC all the reviewers of
> > > https://lore.kernel.org/all/20230506133134.1492395-1-
> jacob.jun.pan@l
> > > inux.intel.com/ to this patch. So, it seems more reasonable to
> > > separate this patch. What do you think?
> >
> > I think that makes sense, let's go ahead with the rest of the SVA
> > series and we can do this next cycle. Bikeshedding config names isn't
> > really critical.
>
> So this didn't make it even without the config change :(
>
> Can you please resend it again and CC the right please please. This needs to go
> into this cycle for sure
Sure.
Regards,
-Tina
>
> Thanks
> Jason
@@ -301,6 +301,11 @@ config ARCH_HAS_DMA_CLEAR_UNCACHED
config ARCH_HAS_CPU_FINALIZE_INIT
bool
+# The architecture has a per-task state that includes the mm's PASID
+config ARCH_HAS_CPU_PASID
+ bool
+ select IOMMU_MM_DATA
+
# Select if arch init_task must go in the __init_task_data section
config ARCH_TASK_STRUCT_ON_STACK
bool
@@ -72,6 +72,7 @@ config X86
select ARCH_HAS_CACHE_LINE_SIZE
select ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION
select ARCH_HAS_CPU_FINALIZE_INIT
+ select ARCH_HAS_CPU_PASID if IOMMU_SVA
select ARCH_HAS_CURRENT_STACK_POINTER
select ARCH_HAS_DEBUG_VIRTUAL
select ARCH_HAS_DEBUG_VM_PGTABLE if !X86_PAE
@@ -565,7 +565,7 @@ static bool fixup_iopl_exception(struct pt_regs *regs)
*/
static bool try_fixup_enqcmd_gp(void)
{
-#ifdef CONFIG_IOMMU_SVA
+#ifdef CONFIG_ARCH_HAS_CPU_PASID
u32 pasid;
/*
@@ -156,6 +156,7 @@ config IOMMU_DMA
# Shared Virtual Addressing
config IOMMU_SVA
+ select IOMMU_MM_DATA
bool
config FSL_PAMU
@@ -1189,7 +1189,7 @@ static inline bool tegra_dev_iommu_get_stream_id(struct device *dev, u32 *stream
return false;
}
-#ifdef CONFIG_IOMMU_SVA
+#ifdef CONFIG_IOMMU_MM_DATA
static inline void mm_pasid_init(struct mm_struct *mm)
{
mm->pasid = IOMMU_PASID_INVALID;
@@ -881,7 +881,7 @@ struct mm_struct {
#endif
struct work_struct async_put_work;
-#ifdef CONFIG_IOMMU_SVA
+#ifdef CONFIG_IOMMU_MM_DATA
u32 pasid;
#endif
#ifdef CONFIG_KSM
@@ -949,7 +949,7 @@ struct task_struct {
/* Recursion prevention for eventfd_signal() */
unsigned in_eventfd:1;
#endif
-#ifdef CONFIG_IOMMU_SVA
+#ifdef CONFIG_ARCH_HAS_CPU_PASID
unsigned pasid_activated:1;
#endif
#ifdef CONFIG_CPU_SUP_INTEL
@@ -1179,7 +1179,7 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
tsk->use_memdelay = 0;
#endif
-#ifdef CONFIG_IOMMU_SVA
+#ifdef CONFIG_ARCH_HAS_CPU_PASID
tsk->pasid_activated = 0;
#endif
@@ -1258,6 +1258,9 @@ config LOCK_MM_AND_FIND_VMA
bool
depends on !STACK_GROWSUP
+config IOMMU_MM_DATA
+ bool
+
source "mm/damon/Kconfig"
endmenu
@@ -44,7 +44,7 @@ struct mm_struct init_mm = {
#endif
.user_ns = &init_user_ns,
.cpu_bitmap = CPU_BITS_NONE,
-#ifdef CONFIG_IOMMU_SVA
+#ifdef CONFIG_IOMMU_MM_DATA
.pasid = IOMMU_PASID_INVALID,
#endif
INIT_MM_CONTEXT(init_mm)