From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f172.google.com (mail-pl1-f172.google.com [209.85.214.172]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E0150313520 for ; Tue, 24 Mar 2026 19:45:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774381518; cv=none; b=dS3zOnbhnvlNtojWUQI3wba3ONkf8b58VgT/LXXjDOiLsxmcNWE8pabZVXG0gDb9rxvPSVgEmCRsSGh0kesqZVwrHWD19ITpojQAiMxl+Xm9IMvSk8iOje82N6NUx6VXBcdzEY+Xcgu9Rh89GUR1oC3/FvgopvXvi9PW8c/KLlY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774381518; c=relaxed/simple; bh=cElslhTE6m9W0YImaAUJVkSxCsiQPpNOSWv/SF1fcSE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=rxP/O8zKXvprL8vh557ClFRgsmPanhE64KwN3trSpYoZBeVRYsJ63gMWWNAqaa0EfkG1BVeRHkS9fRZ+2aIdq+kDXsZJnEe/zy8Lu4KRb6V4dnN5XYLGXCYJKEHRtiar4hiC0avT7g3F72xuJuxmQ+ztpzFaVtmsQfKtzFhGc8M= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=dLfeLan/; arc=none smtp.client-ip=209.85.214.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="dLfeLan/" Received: by mail-pl1-f172.google.com with SMTP id d9443c01a7336-2b0b260d309so2435ad.1 for ; Tue, 24 Mar 2026 12:45:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1774381516; x=1774986316; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=Lk/w0o7W4j7sim/BVALDWnCOxHkFQz3/Bh51qF9Rkjg=; b=dLfeLan/mOfYbtbBWE5UG9L8b+WjDqAvKGiLdSsDRLRiRA+kza2wYzfDWqvD1jdOYo Z3leRpyc+7V3oS0vjQ0gLxVKIDkK6FSrL1/A4am94WY249jgRdBFCZiOi4E9ZVQIleHc S8WBlpc1HgGkpQ6nDpB3SNvt5BoH2qdIiRfPeMptwZKLPWJtZBzOa8N+K9CRmU4BbYuR uTX5TTGMBf5XvljL2hUu5JCXoeXllTT3DL6rWQmxCAcMY+A3CDsAGoFjELFJszPMTunE 6OdNKNMK27Mj27fWFwQfAdh8DzHoj6n5zIBzRi8RkY4i0lVQ+eHD4VUY2UWTdLk9C0tK oHLA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774381516; x=1774986316; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Lk/w0o7W4j7sim/BVALDWnCOxHkFQz3/Bh51qF9Rkjg=; b=qfOYqxfyeQwj5XikNnA0vagrwKguFFkY5GuFo7/qodIs3GeDDEugodUjM7tLD0AWaD ktoEx8AZHY0TG+dCQUlTVI+Z0undc6ImW7/bZToZG/pkLuGNZdk1jGkk/9NuHd9XLHnc LTCxvdFDROxWVcI4WkuO2ZmU51aTptZMObpbJHBAe2nmexNbWnxJHW0AmtDcWcUr9VgZ Xp52DM1PHdcWOpWXw5xRSTySKCvAc+EDpEmhthEvVinqb2iK7iVmE5SPY7Xo2JOmHQYM ZC+iPB+HJozjetu+2FyKvkhslguWHLwlRw7kvcXPKAi5m+OwvQpdTc1gVl+YyXC8+V+X sduA== X-Forwarded-Encrypted: i=1; AJvYcCVYHI6QCKzKur0I1Z/9mZS/xaSpK4WZOaWJmQ//zDpOul9ihIJnEDLo5HgSnZTTS8/wDr4Kjb3igf5iz3w=@vger.kernel.org X-Gm-Message-State: AOJu0Yxhz6qVFNIc7VH4nhljl6WK2ABNWX0wgZdvEgibN53cyaWdylxV 6k9XAQrl+45f5hEMsqDbXWQDNLXjqKknYrhPBg2aNxqk4z+ht6NYKL0ucGGWXlklgw== X-Gm-Gg: ATEYQzxIT5jcZ6lUglIC30xC85hzxtL+cVgUVU2BQkviCJXTX0kqw0A+ZD656bpE9A7 JdmOz8GSIDuHtqUn8jqduNiJc+JJrIzsYQpQ4WSTbvXjK/gtF5Ce/228U6ZNChuctdp8Vy0enRe osjLeAejYYTDOcq5QGn/v++mSg1iwKfaqBDNSqB0Fywa4ovhWXLt3ueELEOcrY5Hmzo/BtWak6P 6hB8SFYnRDjvPHsFUEnrOaseOq8DFC0kysOzn7cCwGyxJ90heAgEFhtcw5dcz+d15si4RcFgqI+ DOeT0M58uFUN942l+h4K+S1pPLwGbR0M6/Z8OJ4/6J1GMHINvScT8iRjGR2Nc8W26tK9PE/aH7q zJukzjEjI22sYKqpeHh5Puf+zWXInznms0P4EOVyfs/t0Lg9YETuszLqW0PfZAhgPSFY6HP8Ve3 IU5buQUC2oQAOjQzgVnYb8rwDEAv6WQ0UsdcxE5ggNjk8N8zRmk5pBzRwZK6U0Cw== X-Received: by 2002:a17:903:b47:b0:2b0:aee4:afe7 with SMTP id d9443c01a7336-2b0b1568b56mr739105ad.10.1774381515550; Tue, 24 Mar 2026 12:45:15 -0700 (PDT) Received: from google.com (168.136.83.34.bc.googleusercontent.com. [34.83.136.168]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-82b03aa5aa7sm13255806b3a.1.2026.03.24.12.45.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 24 Mar 2026 12:45:14 -0700 (PDT) Date: Tue, 24 Mar 2026 19:45:11 +0000 From: Samiullah Khawaja To: Vipin Sharma Cc: David Woodhouse , Lu Baolu , Joerg Roedel , Will Deacon , Jason Gunthorpe , Robin Murphy , Kevin Tian , Alex Williamson , Shuah Khan , iommu@lists.linux.dev, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Saeed Mahameed , Adithya Jayachandran , Parav Pandit , Leon Romanovsky , William Tu , Pratyush Yadav , Pasha Tatashin , David Matlack , Andrew Morton , Chris Li , Pranjal Shrivastava , YiFei Zhu Subject: Re: [PATCH 02/14] iommu: Implement IOMMU core liveupdate skeleton Message-ID: References: <20260203220948.2176157-1-skhawaja@google.com> <20260203220948.2176157-3-skhawaja@google.com> <20260316221854.GC1768676.vipinsh@google.com> <20260324184809.GA190066.vipinsh@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <20260324184809.GA190066.vipinsh@google.com> On Tue, Mar 24, 2026 at 12:06:10PM -0700, Vipin Sharma wrote: >On Tue, Mar 17, 2026 at 08:33:39PM +0000, Samiullah Khawaja wrote: >> On Tue, Mar 17, 2026 at 12:58:27PM -0700, Vipin Sharma wrote: >> > On Tue, Feb 03, 2026 at 10:09:36PM +0000, Samiullah Khawaja wrote: >> > > diff --git a/drivers/iommu/liveupdate.c b/drivers/iommu/liveupdate.c >> > > +int iommu_for_each_preserved_device(iommu_preserved_device_iter_fn fn, >> > > + void *arg) >> > > +{ >> > > + struct iommu_lu_flb_obj *obj; >> > > + struct devices_ser *devices; >> > > + int ret, i, idx; >> > > + >> > > + ret = liveupdate_flb_get_incoming(&iommu_flb, (void **)&obj); >> > > + if (ret) >> > > + return -ENOENT; >> > > + >> > > + devices = __va(obj->ser->devices_phys); >> > > + for (i = 0, idx = 0; i < obj->ser->nr_devices; ++i, ++idx) { >> > > + if (idx >= MAX_DEVICE_SERS) { >> > > + devices = __va(devices->objs.next_objs); >> > > + idx = 0; >> > > + } >> > > + >> > > + if (devices->devices[idx].obj.deleted) >> > > + continue; >> > > + >> > > + ret = fn(&devices->devices[idx], arg); >> > > + if (ret) >> > > + return ret; >> > > + } >> > > + >> > > + return 0; >> > > +} >> > > +EXPORT_SYMBOL(iommu_for_each_preserved_device); >> > Also, should this function be introduced in the patch where it is >> > getting used? Other changes in this patch are already big and complex. >> > Same for iommu_get_device_preserved_data() and >> > iommu_get_preserved_data(). >> >> These are used by the drivers, but part of core. So need to be in >> this patch :(. > >Sorry, I am not understanding why it has to be in this patch? Can it be >its own patch? I will move it to a separate patch as per discussion in other thread. See: https://lore.kernel.org/all/abrk39_M8k45myXJ@google.com/ >> >> Note that this patch is adding core skeleton only, focusing on helpers >> for the serialized state. This patch is not preserving any real state of >> iommu, domain or devices. For example, the domains are saved through >> generic page table in a separate patch, and the drivers preserve the >> state of devices and associated iommu in separate patches. >> >> I will add this text in the commit message to clarify the purpose of >> this patch. >> > >> > I think this patch can be split in three. >> > Patch 1: Preserve iommu_domain >> > Patch 2: Preserve pci device and iommu device >> > Patch 3: The helper functions I mentioned above. > >I understand that this patch is adding some helper functions and not >doing any actual preservation. I am suggesting to split this helper >function patch into three for easier review based on the above suggestion. >If I am not wrong this is biggest patch in series of approx 500 line >changes. After moving the getter helpers out to the separate patch as mentioned above, the size of this patch should significantly reduce. I will split the remaining into domain and device + iommu preservation helper patches as you suggested. > >> > > +static void iommu_unpreserve_locked(struct iommu_device *iommu) >> > > +{ >> > > + struct iommu_ser *iommu_ser = iommu->outgoing_preserved_state; >> > > + >> > > + iommu_ser->obj.ref_count--; >> > >> > Should there be a null check? >> >> Hmm.. There is a dependency of unpreservation of iommus with devices, so >> this should never be NULL unless used independently. >> >> But I think I will add it here to protect against that. > >Okay. Since, it is a static function, I am fine either way. > >> > > +void iommu_unpreserve_device(struct iommu_domain *domain, struct device *dev) >> > > +{ >> > > + struct iommu_lu_flb_obj *flb_obj; >> > > + struct device_ser *device_ser; >> > > + struct dev_iommu *iommu; >> > > + struct pci_dev *pdev; >> > > + int ret; >> > > + >> > > + if (!dev_is_pci(dev)) >> > > + return; >> > > + >> > > + pdev = to_pci_dev(dev); >> > > + iommu = dev->iommu; >> > > + if (!iommu->iommu_dev->ops->unpreserve_device || >> > > + !iommu->iommu_dev->ops->unpreserve) >> > > + return; >> > > + >> > > + ret = liveupdate_flb_get_outgoing(&iommu_flb, (void **)&flb_obj); >> > > + if (WARN_ON(ret)) >> > >> > Why WARN_ON here and not other places? Do we need it? >> >> Basically this means that the upper layer (iommufd/vfio) is asking to >> unpreserve a device, but there is no FLB found. This should not happen >> and should generate a warning. > >Yeah, but other places iommu_domain_[preserve|unpreserve](), >iommu_presreve_locked(), and iommu_preserve_device() are also using this >function. I am having a confusion on why it is important in his >function and not others. Those functions are also called by upper layer. Ah yes.. That makes sense. I will add it to the unpreserve variants of functions you listed, as during preservation the FLB obj should have been created. >