Re: [PATCH 06/14] iommu/vt-d: Implement device and iommu preserve/unpreserve ops

[Pranjal Shrivastava <praan@google.com>]

On Fri, Mar 20, 2026 at 11:01:59PM +0000, Pranjal Shrivastava wrote:
> On Tue, Feb 03, 2026 at 10:09:40PM +0000, Samiullah Khawaja wrote:
> > Add implementation of the device and iommu presevation in a separate
> > file. Also set the device and iommu preserve/unpreserve ops in the
> > struct iommu_ops.
> > 
> > During normal shutdown the iommu translation is disabled. Since the root
> > table is preserved during live update, it needs to be cleaned up and the
> > context entries of the unpreserved devices need to be cleared.
> > 
> > Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
> > ---
> >  drivers/iommu/intel/Makefile     |   1 +
> >  drivers/iommu/intel/iommu.c      |  47 ++++++++++-
> >  drivers/iommu/intel/iommu.h      |  27 +++++++
> >  drivers/iommu/intel/liveupdate.c | 134 +++++++++++++++++++++++++++++++
> >  4 files changed, 205 insertions(+), 4 deletions(-)
> >  create mode 100644 drivers/iommu/intel/liveupdate.c
> > 
> > diff --git a/drivers/iommu/intel/Makefile b/drivers/iommu/intel/Makefile
> > index ada651c4a01b..d38fc101bc35 100644
> > --- a/drivers/iommu/intel/Makefile
> > +++ b/drivers/iommu/intel/Makefile
> > @@ -6,3 +6,4 @@ obj-$(CONFIG_INTEL_IOMMU_DEBUGFS) += debugfs.o
> >  obj-$(CONFIG_INTEL_IOMMU_SVM) += svm.o
> >  obj-$(CONFIG_IRQ_REMAP) += irq_remapping.o
> >  obj-$(CONFIG_INTEL_IOMMU_PERF_EVENTS) += perfmon.o
> > +obj-$(CONFIG_IOMMU_LIVEUPDATE) += liveupdate.o
> > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> > index 134302fbcd92..c95de93fb72f 100644
> > --- a/drivers/iommu/intel/iommu.c
> > +++ b/drivers/iommu/intel/iommu.c
> > @@ -16,6 +16,7 @@
> >  #include <linux/crash_dump.h>
> >  #include <linux/dma-direct.h>
> >  #include <linux/dmi.h>
> > +#include <linux/iommu-lu.h>
> >  #include <linux/memory.h>
> >  #include <linux/pci.h>
> >  #include <linux/pci-ats.h>
> > @@ -52,6 +53,8 @@ static int rwbf_quirk;
> >  

[ ---- snip >8 ----- ]
 
> > +
> > +int intel_iommu_preserve(struct iommu_device *iommu_dev, struct iommu_ser *ser)
> > +{
> > +	struct intel_iommu *iommu;
> > +	int ret;
> > +
> > +	iommu = container_of(iommu_dev, struct intel_iommu, iommu);
> > +
> > +	spin_lock(&iommu->lock);
> 
> Minor note: We call this with the session->mutex acquired which is a
> sleepable/preemptable context whereas spin_lock disables preemption and
> puts us in an atomic context. 
> 
> While iommu_context_addr() happens to be safe here because it uses
> GFP_ATOMIC & we pass alloc=0, we are heavily relying on the assumption
> that iommu_context_addr() or other helpers will never sleep or attempt
> a GFP_KERNEL allocation under the hood.
> 
> Given how easy it would be for a future refactor to accidentally
> introduce a sleeping lock inside any of the other helpers, I'd recommend
> adding an explicit comment here warning that this runs in an atomic
> context (same for unpreserve below).
> 

Thinking a little more about this, I think we shall instead add a
comment in the core API docs, mentioning that these helpers (like
iommu_preserve_page etc.) can be called from atomic context and hence
should not acquire any mutexes as we can't be certain of the context the
drivers call them in. 
 
Thanks,
Praan