From: Jason Gunthorpe <[email protected]>
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 <[email protected]>
---
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(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 12d51495caec..35b9fd559bb6 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -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
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 66bfabae8814..afd9c2dc228b 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -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
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index c876f1d36a81..2b62dbb3396a 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -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;
/*
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 3199fd54b462..52fa02f1b675 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -156,6 +156,7 @@ config IOMMU_DMA
# Shared Virtual Addressing
config IOMMU_SVA
+ select IOMMU_MM_DATA
bool
config FSL_PAMU
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index b5b254e205c6..c1f2b6f9a3d0 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -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;
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 36c5b43999e6..330f3cd8d5ad 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -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
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 77f01ac385f7..3ac8e8556c3d 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -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
diff --git a/kernel/fork.c b/kernel/fork.c
index 3b6d20dfb9a8..d28f0d4582dc 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -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
diff --git a/mm/Kconfig b/mm/Kconfig
index 264a2df5ecf5..fee4a15e444b 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -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
diff --git a/mm/init-mm.c b/mm/init-mm.c
index cfd367822cdd..c52dc2740a3d 100644
--- a/mm/init-mm.c
+++ b/mm/init-mm.c
@@ -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)
--
2.39.3
Hi Tina,
On Fri, Oct 27, 2023 at 08:05:20AM +0800, Tina Zhang wrote:
> From: Jason Gunthorpe <[email protected]>
>
> 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 <[email protected]>
If you send it, you also need to add your Signed-off-by.
Regards,
Joerg
Hi Joerg,
> -----Original Message-----
> From: Joerg Roedel <[email protected]>
> Sent: Friday, October 27, 2023 3:12 PM
> To: Zhang, Tina <[email protected]>
> Cc: [email protected]; [email protected]; David Woodhouse
> <[email protected]>; Lu Baolu <[email protected]>; Will Deacon
> <[email protected]>; Robin Murphy <[email protected]>; Jason
> Gunthorpe <[email protected]>; Tian, Kevin <[email protected]>; Nicolin Chen
> <[email protected]>; Michael Shavit <[email protected]>; Vasant
> Hegde <[email protected]>; Jason Gunthorpe <[email protected]>
> 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 <[email protected]>
> >
> > 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
> > [email protected]/
> >
> > 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 <[email protected]>
>
> 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 <[email protected]>
> Cc: [email protected]; [email protected]; David Woodhouse
> <[email protected]>; Lu Baolu <[email protected]>; Will Deacon
> <[email protected]>; Robin Murphy <[email protected]>; Jason
> Gunthorpe <[email protected]>; Tian, Kevin <[email protected]>; Nicolin Chen
> <[email protected]>; Michael Shavit <[email protected]>; Vasant
> Hegde <[email protected]>; Jason Gunthorpe <[email protected]>
> Subject: RE: [PATCH v10 1/6] iommu: Change kconfig around IOMMU_SVA
>
> Hi Joerg,
>
> > -----Original Message-----
> > From: Joerg Roedel <[email protected]>
> > Sent: Friday, October 27, 2023 3:12 PM
> > To: Zhang, Tina <[email protected]>
> > Cc: [email protected]; [email protected]; David
> > Woodhouse <[email protected]>; Lu Baolu
> <[email protected]>;
> > Will Deacon <[email protected]>; Robin Murphy <[email protected]>;
> > Jason Gunthorpe <[email protected]>; Tian, Kevin <[email protected]>;
> > Nicolin Chen <[email protected]>; Michael Shavit
> > <[email protected]>; Vasant Hegde <[email protected]>; Jason
> > Gunthorpe <[email protected]>
> > 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 <[email protected]>
> > >
> > > 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/[email protected]/ 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 <[email protected]>
> > > >
> > > > 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/[email protected]/
> 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 <[email protected]>
> > > > >
> > > > > 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/[email protected]/
> > 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 <[email protected]>
> Sent: Tuesday, November 28, 2023 2:25 AM
> To: Zhang, Tina <[email protected]>
> Cc: Joerg Roedel <[email protected]>; [email protected]; linux-
> [email protected]; David Woodhouse <[email protected]>; Lu
> Baolu <[email protected]>; Will Deacon <[email protected]>; Robin
> Murphy <[email protected]>; Tian, Kevin <[email protected]>;
> Nicolin Chen <[email protected]>; Michael Shavit <[email protected]>;
> Vasant Hegde <[email protected]>
> 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 <[email protected]>
> > > > > >
> > > > > > 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