From: Baolu Lu <baolu.lu@linux.intel.com>
To: Fenghua Yu <fenghua.yu@intel.com>,
Jean-Philippe Brucker <jean-philippe@linaro.org>
Cc: Dave Hansen <dave.hansen@intel.com>,
Tony Luck <tony.luck@intel.com>, Ashok Raj <ashok.raj@intel.com>,
Ravi V Shankar <ravi.v.shankar@intel.com>,
Peter Zijlstra <peterz@infradead.org>,
robin.murphy@arm.com, Dave Hansen <dave.hansen@linux.intel.com>,
x86 <x86@kernel.org>, linux-kernel <linux-kernel@vger.kernel.org>,
iommu <iommu@lists.linux-foundation.org>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
Andy Lutomirski <luto@kernel.org>,
Josh Poimboeuf <jpoimboe@redhat.com>,
zhangfei.gao@linaro.org, Thomas Gleixner <tglx@linutronix.de>,
will@kernel.org
Subject: Re: [PATCH v4 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit
Date: Sat, 30 Apr 2022 15:33:17 +0800 [thread overview]
Message-ID: <3ea91623-97ea-f318-70db-55f08922d7ce@linux.intel.com> (raw)
In-Reply-To: <Ymxke+ihgwNy3BCE@fyu1.sc.intel.com>
On 2022/4/30 06:19, Fenghua Yu wrote:
> Hi, Jean and Baolu,
>
> On Fri, Apr 29, 2022 at 03:34:36PM +0100, Jean-Philippe Brucker wrote:
>> On Fri, Apr 29, 2022 at 06:51:17AM -0700, Fenghua Yu wrote:
>>> Hi, Baolu,
>>>
>>> On Fri, Apr 29, 2022 at 03:53:57PM +0800, Baolu Lu wrote:
>>>> On 2022/4/28 16:39, Jean-Philippe Brucker wrote:
>>>>>> The address space is what the OOM killer is after. That gets refcounted
>>>>>> with mmget()/mmput()/mm->mm_users. The OOM killer is satiated by the
>>>>>> page freeing done in __mmput()->exit_mmap().
>>>>>>
>>>>>> Also, all the VMAs should be gone after exit_mmap(). So, even if
>>>>>> vma->vm_file was holding a reference to a device driver, that reference
>>>>>> should be gone by the time __mmdrop() is actually freeing the PASID.
>>>>>
>>>>> I agree with all that. The concern was about tearing down the PASID in the
>>>>> IOMMU and device from the release() MMU notifier, which would happen in
>>>>> exit_mmap(). But doing the teardown at or before __mmdrop() is fine. And
>>>>> since the IOMMU drivers need to hold mm->mm_count anyway between bind()
>>>>> and unbind(), I think Fenghua's fix works.
>>>>
>>>> But I didn't find mmgrab()/mmdrop() get called in both arm and intel
>>>> IOMMU drivers.
>>>>
>>>> $ git grep mmgrab drivers/iommu/
>>>> [no output]
>>>>
>>>> Do we need to add these in a separated fix patch, or I missed anything
>>>> here?
>>>
>>> On both ARM and X86, sva_bind() calls mmu_notifier_register()->mmgrab() and
>>> sva_unbind() calls mmu_notifier_unregister()/mmu_notifier_put()->mmdrop().
>>
>> Yes, although for Arm I realized the mmu_notifier grab wasn't sufficient
>> so I sent a separate fix that should go in 5.18 as well
>> https://lore.kernel.org/linux-iommu/20220426130444.300556-1-jean-philippe@linaro.org/
>> The Arm driver still touches the arch mm context after mmu_notifier_put().
>> I don't think X86 has that problem.
>
> I think so too. On X86, the mm is not used after mmu_notifier_unregister().
>
> In case of supervisor mode SVM (i.e. svm is bound to init_mm), the code
> is right too because init_mm and its PASID cannot be dropped and
> mmu_notifier_register()/mmu_notifier_unregister() are not called.
>
> So I think no extra fix patch is needed on X86.
Thanks, Fenghua and Jean. It's clear to me now.
Jean, another quick question about the iommu_sva_bind_device()
/**
* iommu_sva_bind_device() - Bind a process address space to a device
* @dev: the device
* @mm: the mm to bind, caller must hold a reference to it
* @drvdata: opaque data pointer to pass to bind callback
This interface requires the caller to take a reference to mm. Which
reference should it take, mm->mm_count or mm->mm_users? It's better to
make it explicit in this comment.
Best regards,
baolu
WARNING: multiple messages have this Message-ID (diff)
From: Baolu Lu <baolu.lu@linux.intel.com>
To: Fenghua Yu <fenghua.yu@intel.com>,
Jean-Philippe Brucker <jean-philippe@linaro.org>
Cc: Ravi V Shankar <ravi.v.shankar@intel.com>,
Tony Luck <tony.luck@intel.com>, Ashok Raj <ashok.raj@intel.com>,
Peter Zijlstra <peterz@infradead.org>,
will@kernel.org, Dave Hansen <dave.hansen@linux.intel.com>,
x86 <x86@kernel.org>, linux-kernel <linux-kernel@vger.kernel.org>,
Dave Hansen <dave.hansen@intel.com>,
iommu <iommu@lists.linux-foundation.org>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
Andy Lutomirski <luto@kernel.org>,
Josh Poimboeuf <jpoimboe@redhat.com>,
zhangfei.gao@linaro.org, Thomas Gleixner <tglx@linutronix.de>,
robin.murphy@arm.com
Subject: Re: [PATCH v4 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit
Date: Sat, 30 Apr 2022 15:33:17 +0800 [thread overview]
Message-ID: <3ea91623-97ea-f318-70db-55f08922d7ce@linux.intel.com> (raw)
In-Reply-To: <Ymxke+ihgwNy3BCE@fyu1.sc.intel.com>
On 2022/4/30 06:19, Fenghua Yu wrote:
> Hi, Jean and Baolu,
>
> On Fri, Apr 29, 2022 at 03:34:36PM +0100, Jean-Philippe Brucker wrote:
>> On Fri, Apr 29, 2022 at 06:51:17AM -0700, Fenghua Yu wrote:
>>> Hi, Baolu,
>>>
>>> On Fri, Apr 29, 2022 at 03:53:57PM +0800, Baolu Lu wrote:
>>>> On 2022/4/28 16:39, Jean-Philippe Brucker wrote:
>>>>>> The address space is what the OOM killer is after. That gets refcounted
>>>>>> with mmget()/mmput()/mm->mm_users. The OOM killer is satiated by the
>>>>>> page freeing done in __mmput()->exit_mmap().
>>>>>>
>>>>>> Also, all the VMAs should be gone after exit_mmap(). So, even if
>>>>>> vma->vm_file was holding a reference to a device driver, that reference
>>>>>> should be gone by the time __mmdrop() is actually freeing the PASID.
>>>>>
>>>>> I agree with all that. The concern was about tearing down the PASID in the
>>>>> IOMMU and device from the release() MMU notifier, which would happen in
>>>>> exit_mmap(). But doing the teardown at or before __mmdrop() is fine. And
>>>>> since the IOMMU drivers need to hold mm->mm_count anyway between bind()
>>>>> and unbind(), I think Fenghua's fix works.
>>>>
>>>> But I didn't find mmgrab()/mmdrop() get called in both arm and intel
>>>> IOMMU drivers.
>>>>
>>>> $ git grep mmgrab drivers/iommu/
>>>> [no output]
>>>>
>>>> Do we need to add these in a separated fix patch, or I missed anything
>>>> here?
>>>
>>> On both ARM and X86, sva_bind() calls mmu_notifier_register()->mmgrab() and
>>> sva_unbind() calls mmu_notifier_unregister()/mmu_notifier_put()->mmdrop().
>>
>> Yes, although for Arm I realized the mmu_notifier grab wasn't sufficient
>> so I sent a separate fix that should go in 5.18 as well
>> https://lore.kernel.org/linux-iommu/20220426130444.300556-1-jean-philippe@linaro.org/
>> The Arm driver still touches the arch mm context after mmu_notifier_put().
>> I don't think X86 has that problem.
>
> I think so too. On X86, the mm is not used after mmu_notifier_unregister().
>
> In case of supervisor mode SVM (i.e. svm is bound to init_mm), the code
> is right too because init_mm and its PASID cannot be dropped and
> mmu_notifier_register()/mmu_notifier_unregister() are not called.
>
> So I think no extra fix patch is needed on X86.
Thanks, Fenghua and Jean. It's clear to me now.
Jean, another quick question about the iommu_sva_bind_device()
/**
* iommu_sva_bind_device() - Bind a process address space to a device
* @dev: the device
* @mm: the mm to bind, caller must hold a reference to it
* @drvdata: opaque data pointer to pass to bind callback
This interface requires the caller to take a reference to mm. Which
reference should it take, mm->mm_count or mm->mm_users? It's better to
make it explicit in this comment.
Best regards,
baolu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2022-04-30 7:33 UTC|newest]
Thread overview: 214+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-07 23:02 [PATCH v4 00/11] Re-enable ENQCMD and PASID MSR Fenghua Yu
2022-02-07 23:02 ` Fenghua Yu
2022-02-07 23:02 ` [PATCH v4 01/11] iommu/sva: Rename CONFIG_IOMMU_SVA_LIB to CONFIG_IOMMU_SVA Fenghua Yu
2022-02-07 23:02 ` Fenghua Yu
2022-02-08 2:39 ` Lu Baolu
2022-02-08 2:39 ` Lu Baolu
2022-02-15 10:54 ` [tip: x86/pasid] " tip-bot2 for Fenghua Yu
2022-02-07 23:02 ` [PATCH v4 02/11] mm: Change CONFIG option for mm->pasid field Fenghua Yu
2022-02-07 23:02 ` Fenghua Yu
2022-02-08 2:40 ` Lu Baolu
2022-02-08 2:40 ` Lu Baolu
2022-02-15 10:54 ` [tip: x86/pasid] " tip-bot2 for Fenghua Yu
2022-02-07 23:02 ` [PATCH v4 03/11] iommu/ioasid: Introduce a helper to check for valid PASIDs Fenghua Yu
2022-02-07 23:02 ` Fenghua Yu
2022-02-08 2:40 ` Lu Baolu
2022-02-08 2:40 ` Lu Baolu
2022-02-15 10:54 ` [tip: x86/pasid] " tip-bot2 for Fenghua Yu
2022-02-07 23:02 ` [PATCH v4 04/11] kernel/fork: Initialize mm's PASID Fenghua Yu
2022-02-07 23:02 ` Fenghua Yu
2022-02-14 17:23 ` Thomas Gleixner
2022-02-14 17:23 ` Thomas Gleixner
2022-02-15 10:54 ` [tip: x86/pasid] " tip-bot2 for Fenghua Yu
2022-02-07 23:02 ` [PATCH v4 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit Fenghua Yu
2022-02-07 23:02 ` Fenghua Yu
2022-02-08 2:41 ` Lu Baolu
2022-02-08 2:41 ` Lu Baolu
2022-02-08 15:01 ` Fenghua Yu
2022-02-08 15:01 ` Fenghua Yu
2022-02-10 3:16 ` Jacob Pan
2022-02-10 3:16 ` Jacob Pan
2022-02-10 16:27 ` Fenghua Yu
2022-02-10 16:27 ` Fenghua Yu
2022-02-10 17:24 ` Luck, Tony
2022-02-10 17:24 ` Luck, Tony
2022-02-10 18:31 ` Fenghua Yu
2022-02-10 18:31 ` Fenghua Yu
2022-02-10 23:52 ` Fenghua Yu
2022-02-10 23:52 ` Fenghua Yu
2022-02-10 18:49 ` Jacob Pan
2022-02-10 18:49 ` Jacob Pan
2022-02-10 23:15 ` Fenghua Yu
2022-02-10 23:15 ` Fenghua Yu
2022-02-11 22:00 ` Dave Hansen
2022-02-11 22:00 ` Dave Hansen
2022-02-14 17:24 ` Thomas Gleixner
2022-02-14 17:24 ` Thomas Gleixner
2022-02-15 9:55 ` Joerg Roedel
2022-02-15 9:55 ` Joerg Roedel
2022-04-11 14:00 ` Zhangfei Gao
2022-04-11 14:10 ` Dave Hansen
2022-04-11 14:10 ` Dave Hansen
2022-04-11 14:20 ` zhangfei.gao
2022-04-11 14:20 ` zhangfei.gao
2022-04-11 14:36 ` Dave Hansen
2022-04-11 14:36 ` Dave Hansen
2022-04-11 14:44 ` zhangfei.gao
2022-04-11 14:44 ` zhangfei.gao
2022-04-11 14:52 ` Dave Hansen
2022-04-11 14:52 ` Dave Hansen
2022-04-11 15:13 ` zhangfei.gao
2022-04-11 15:13 ` zhangfei.gao
2022-04-12 7:04 ` zhangfei.gao
2022-04-12 7:04 ` zhangfei.gao
2022-04-12 13:41 ` Fenghua Yu
2022-04-12 13:41 ` Fenghua Yu
2022-04-12 14:39 ` Dave Hansen
2022-04-12 14:39 ` Dave Hansen
2022-04-15 9:59 ` Fenghua Yu
2022-04-15 9:59 ` Fenghua Yu
2022-04-12 15:35 ` zhangfei.gao
2022-04-12 15:35 ` zhangfei.gao
2022-04-14 10:08 ` zhangfei.gao
2022-04-14 10:08 ` zhangfei.gao
2022-04-15 9:51 ` Fenghua Yu
2022-04-15 9:51 ` Fenghua Yu
2022-04-15 10:14 ` zhangfei.gao
2022-04-15 10:14 ` zhangfei.gao
2022-04-15 10:14 ` zhangfei.gao
2022-04-15 10:50 ` Fenghua Yu
2022-04-15 10:50 ` Fenghua Yu
2022-04-15 11:52 ` zhangfei.gao
2022-04-15 11:52 ` zhangfei.gao
2022-04-15 12:37 ` Fenghua Yu
2022-04-15 12:37 ` Fenghua Yu
2022-04-16 1:30 ` zhangfei.gao
2022-04-16 1:30 ` zhangfei.gao
2022-04-15 19:07 ` Fenghua Yu
2022-04-15 19:07 ` Fenghua Yu
2022-04-15 21:00 ` Jacob Pan
2022-04-15 21:00 ` Jacob Pan
2022-04-16 1:43 ` zhangfei.gao
2022-04-16 1:43 ` zhangfei.gao
2022-04-18 18:14 ` Jacob Pan
2022-04-18 18:14 ` Jacob Pan
2022-04-19 1:02 ` zhangfei.gao
2022-04-19 1:02 ` zhangfei.gao
2022-04-18 6:34 ` Tian, Kevin
2022-04-18 6:34 ` Tian, Kevin
2022-04-18 18:11 ` Jacob Pan
2022-04-18 18:11 ` Jacob Pan
2022-04-20 16:45 ` Jean-Philippe Brucker
2022-04-20 16:45 ` Jean-Philippe Brucker
2022-04-21 6:47 ` zhangfei.gao
2022-04-21 6:47 ` zhangfei.gao
2022-04-22 9:03 ` zhangfei.gao
2022-04-22 9:03 ` zhangfei.gao
2022-04-22 10:11 ` Jean-Philippe Brucker
2022-04-22 10:11 ` Jean-Philippe Brucker
2022-04-22 13:15 ` zhangfei.gao
2022-04-22 13:15 ` zhangfei.gao
2022-04-22 15:50 ` Jean-Philippe Brucker
2022-04-22 15:50 ` Jean-Philippe Brucker
2022-04-23 11:13 ` zhangfei.gao
2022-04-23 11:13 ` zhangfei.gao
2022-04-24 2:58 ` Zhangfei Gao
2022-04-24 2:58 ` Zhangfei Gao
2022-04-24 9:52 ` Zhangfei Gao
2022-04-24 9:52 ` Zhangfei Gao
2022-04-25 13:53 ` Jean-Philippe Brucker
2022-04-25 13:53 ` Jean-Philippe Brucker
2022-04-25 14:18 ` Dave Hansen
2022-04-25 14:18 ` Dave Hansen
2022-04-25 14:26 ` Jean-Philippe Brucker
2022-04-25 14:26 ` Jean-Philippe Brucker
2022-04-25 15:34 ` Jacob Pan
2022-04-25 15:34 ` Jacob Pan
2022-04-25 16:13 ` Jean-Philippe Brucker
2022-04-25 16:13 ` Jean-Philippe Brucker
2022-04-25 22:32 ` Jacob Pan
2022-04-25 22:32 ` Jacob Pan
2022-04-26 4:20 ` Fenghua Yu
2022-04-26 4:20 ` Fenghua Yu
2022-04-26 5:04 ` Zhangfei Gao
2022-04-26 5:04 ` Zhangfei Gao
2022-04-28 0:54 ` Fenghua Yu
2022-04-28 0:54 ` Fenghua Yu
2022-04-28 8:43 ` Jean-Philippe Brucker
2022-04-28 8:43 ` Jean-Philippe Brucker
2022-04-28 15:09 ` Dave Hansen
2022-04-28 15:09 ` Dave Hansen
2022-04-28 15:28 ` Fenghua Yu
2022-04-28 15:28 ` Fenghua Yu
2022-04-28 15:42 ` Dave Hansen
2022-04-28 15:42 ` Dave Hansen
2022-04-28 16:01 ` Jean-Philippe Brucker
2022-04-28 16:01 ` Jean-Philippe Brucker
2022-04-28 16:35 ` Dave Hansen
2022-04-28 16:35 ` Dave Hansen
2022-04-26 4:28 ` Zhangfei Gao
2022-04-26 4:28 ` Zhangfei Gao
2022-04-26 4:36 ` Fenghua Yu
2022-04-26 4:36 ` Fenghua Yu
2022-04-26 5:19 ` Zhangfei Gao
2022-04-26 5:19 ` Zhangfei Gao
2022-04-25 15:55 ` Dave Hansen
2022-04-25 15:55 ` Dave Hansen
2022-04-25 16:40 ` Jean-Philippe Brucker
2022-04-25 16:40 ` Jean-Philippe Brucker
2022-04-26 15:27 ` Dave Hansen
2022-04-26 15:27 ` Dave Hansen
2022-04-26 16:48 ` Jean-Philippe Brucker
2022-04-26 16:48 ` Jean-Philippe Brucker
2022-04-26 23:31 ` Dave Hansen
2022-04-26 23:31 ` Dave Hansen
2022-04-28 8:39 ` Jean-Philippe Brucker
2022-04-28 8:39 ` Jean-Philippe Brucker
2022-04-29 7:53 ` Baolu Lu
2022-04-29 7:53 ` Baolu Lu
2022-04-29 13:51 ` Fenghua Yu
2022-04-29 13:51 ` Fenghua Yu
2022-04-29 14:34 ` Jean-Philippe Brucker
2022-04-29 14:34 ` Jean-Philippe Brucker
2022-04-29 22:19 ` Fenghua Yu
2022-04-29 22:19 ` Fenghua Yu
2022-04-30 7:33 ` Baolu Lu [this message]
2022-04-30 7:33 ` Baolu Lu
2022-05-03 7:49 ` Jean-Philippe Brucker
2022-05-03 7:49 ` Jean-Philippe Brucker
2022-05-06 5:36 ` Baolu Lu
2022-05-06 5:36 ` Baolu Lu
2022-04-12 14:36 ` Dave Hansen
2022-04-12 14:36 ` Dave Hansen
2022-04-12 15:10 ` Jean-Philippe Brucker
2022-04-12 15:10 ` Jean-Philippe Brucker
2022-04-12 15:35 ` Dave Hansen
2022-04-12 15:35 ` Dave Hansen
2022-04-13 11:14 ` Lu Baolu
2022-04-13 11:14 ` Lu Baolu
2022-04-25 2:57 ` zhangfei.gao
2022-04-25 2:57 ` zhangfei.gao
2022-02-15 10:54 ` [tip: x86/pasid] " tip-bot2 for Fenghua Yu
2022-02-07 23:02 ` [PATCH v4 06/11] x86/fpu: Clear PASID when copying fpstate Fenghua Yu
2022-02-07 23:02 ` Fenghua Yu
2022-02-15 10:54 ` [tip: x86/pasid] " tip-bot2 for Fenghua Yu
2022-02-07 23:02 ` [PATCH v4 07/11] sched: Define and initialize a flag to identify valid PASID in the task Fenghua Yu
2022-02-07 23:02 ` Fenghua Yu
2022-02-15 10:54 ` [tip: x86/pasid] " tip-bot2 for Peter Zijlstra
2022-02-07 23:02 ` [PATCH v4 08/11] x86/traps: Demand-populate PASID MSR via #GP Fenghua Yu
2022-02-07 23:02 ` Fenghua Yu
2022-02-15 10:54 ` [tip: x86/pasid] " tip-bot2 for Fenghua Yu
2022-02-07 23:02 ` [PATCH v4 09/11] x86/cpufeatures: Re-enable ENQCMD Fenghua Yu
2022-02-07 23:02 ` Fenghua Yu
2022-02-15 10:54 ` [tip: x86/pasid] " tip-bot2 for Fenghua Yu
2022-02-07 23:02 ` [PATCH v4 10/11] tools/objtool: Check for use of the ENQCMD instruction in the kernel Fenghua Yu
2022-02-07 23:02 ` Fenghua Yu
2022-02-15 10:54 ` [tip: x86/pasid] " tip-bot2 for Fenghua Yu
2022-03-09 7:55 ` [tip: x86/core] " tip-bot2 for Fenghua Yu
2022-03-15 10:44 ` tip-bot2 for Fenghua Yu
2022-02-07 23:02 ` [PATCH v4 11/11] docs: x86: Change documentation for SVA (Shared Virtual Addressing) Fenghua Yu
2022-02-07 23:02 ` Fenghua Yu
2022-02-14 17:25 ` Thomas Gleixner
2022-02-14 17:25 ` Thomas Gleixner
2022-02-15 10:54 ` [tip: x86/pasid] Documentation/x86: Update " tip-bot2 for Fenghua Yu
2022-02-11 20:04 ` [PATCH v4 00/11] Re-enable ENQCMD and PASID MSR Fenghua Yu
2022-02-11 20:04 ` Fenghua Yu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3ea91623-97ea-f318-70db-55f08922d7ce@linux.intel.com \
--to=baolu.lu@linux.intel.com \
--cc=ashok.raj@intel.com \
--cc=bp@alien8.de \
--cc=dave.hansen@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=fenghua.yu@intel.com \
--cc=iommu@lists.linux-foundation.org \
--cc=jean-philippe@linaro.org \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=ravi.v.shankar@intel.com \
--cc=robin.murphy@arm.com \
--cc=tglx@linutronix.de \
--cc=tony.luck@intel.com \
--cc=will@kernel.org \
--cc=x86@kernel.org \
--cc=zhangfei.gao@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.