Re: [PATCH] iommu: Allow device driver to use its own PASID space for SVA

[Joonwon Kang <joonwonkang@google.com>]

> On Fri, May 15, 2026 at 09:46:05AM +0000, Joonwon Kang wrote:
> > diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> > index 0ca3912ecb7f..61e2e52105e5 100644
> > --- a/arch/x86/kernel/traps.c
> > +++ b/arch/x86/kernel/traps.c
> > @@ -864,6 +864,8 @@ static bool try_fixup_enqcmd_gp(void)
> >  		return false;
> >  
> >  	pasid = mm_get_enqcmd_pasid(current->mm);
> > +	if (pasid == IOMMU_PASID_INVALID)
> > +		return false;
> 
> If you do this then probably you should get rid of mm_valid_pasid(),
> mm_get_enqcmd_pasid() already has the NULL check so the two functions
> are kind of pointless.
> 
> You also missed the other place calling mm_valid_pasid() that should
> really be sensitive to this as well:
> 
> static int prctl_enable_tagged_addr(struct mm_struct *mm, unsigned long nr_bits)
> {
> [..]
>         if (mm_valid_pasid(mm) &&
>             !test_bit(MM_CONTEXT_FORCE_TAGGED_SVA, &mm->context.flags))
>                 return -EINVAL;
> 
> Make that removal a prep patch
> 

Thanks for pointing this out. I think mm_valid_pasid() is to check if SVA
is currently in action while mm_get_enqcmd_pasid() is to get the PASID for
"ENQCMD instruction execution". What prctl_enable_tagged_addr()
semantically requires here seems to be only checking if SVA is activated
or not regardless of whether EL0 can execute ENQCMD instruction; the
function wants to return error if SVA has been activated when LAM is to be
activated as they are mutually exclusive [1]. So, I think we should leave
it as-is. I am adding the author Kiryl as a reviewer for confirmation.

Since it is now possible with this patch to activate SVA without involving
EL0(for ENQCMD-like instructions), callers should be able to distinguish
between if SVA is activated and if EL0 can execute ENQCMD-like
instructions in addition to the SVA activation. mm_valid_pasid() is for
the former and mm_get_enqcmd_pasid() is for the latter. So, I think there
will be loss if we make the latter API serve for the former.

Or, I guess we could rename mm_valid_pasid() to be more specific like
mm_sva_activated() or mm_sva_in_use(). What do you think?

> I didn't try to check the rest closely but the approach looked sane to
> me

Thanks. Sashiko reported valid points [2] and I will handle them in the
meantime.

[1] https://lore.kernel.org/all/20230312112612.31869-12-kirill.shutemov@linux.intel.com/
[2] https://sashiko.dev/#/patchset/20260515094605.3195841-1-joonwonkang%40google.com

Thanks,
Joonwon Kang