From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f178.google.com (mail-pl1-f178.google.com [209.85.214.178]) (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 E3ABC2E7185 for ; Tue, 24 Feb 2026 19:03:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.178 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771959790; cv=none; b=Beb585ngpXgne/sfdEJ2Ms9hn8ZrdKU6l2PaFDcbaSC1FmvjhQIb2a7nD4jfAbSZgWCvAWY2cCBdLuaITt+doYN4h3rOmaLIPliyK9/BUrXfVz7VLs91NyWWqLNu7nl3gkxpkSbosDVwSo9lHCAnCWdf/L+5atwc0fcV21sWfU8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771959790; c=relaxed/simple; bh=BPzlPvPufPvOXP3pdB2UQNj5Uct014GMnFkHawwDFwY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=grrbVqxZIanDmnY/GAMa7CG9BfRLDkMuPQXcpHrtJFNJJwdif2wOjJJdv/p26h28o353El+D9o08HoGHiyGJgiqXfsBIemxifD6k72GjK+TE/b/QYRD5fzjQ7HREvQU6iWAafItFhd/ekH5XtL2PDXN6htUfL9jyszUd3nPDSpI= 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=C5BRflp7; arc=none smtp.client-ip=209.85.214.178 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="C5BRflp7" Received: by mail-pl1-f178.google.com with SMTP id d9443c01a7336-2aad8123335so10155ad.1 for ; Tue, 24 Feb 2026 11:03:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1771959788; x=1772564588; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=X93b96xDCWqbNSn/Osk0k6D0QiHzoDXCBhSgmB8C344=; b=C5BRflp7hBmvCOLgrrC9JjY3twQOp4qCXCTstDD4ptm7eSR2E39Bks8f90j+qmqyxI bjtiYPVwZe4NHktOTYXcr0b8fH5F8qYur5JC/kMuGDe6cPPGvS8fW92HDMornloiLHh/ wHyxwKd4palG6NtIRkDe5crr/Ut5WU9PuGXX16ETlj0XPCLKUB1NakSUvLXhHbzUBLS+ bYCF1ZPn3cEbF9lllRIOwLpr5Kuq3R7mKBAsKWT5JVone/CG6Y0QazPDWDLyCqos0fBn 0Zg8CwS2nVL64xRMZmqKPXpQgwI2QthEsePUWb2xp9uFFsR5WidLPPMzgnw18m2A5BSO HJBA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1771959788; x=1772564588; h=in-reply-to:content-transfer-encoding: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=X93b96xDCWqbNSn/Osk0k6D0QiHzoDXCBhSgmB8C344=; b=LnCfyhpvvbUs6/IT1e14pRv4mt06ay6sw1S+PIIYNI67qz3FEOe8DRwLEgge8KOOWX pweTbA0n2dMGiTJ+JkRaiNSbShbI/H57r/l4ra5Mo99zUB58U661KoCecZxEBERVCE25 rPxG1s/QuxJ8a7sdS8Pb4IHvkvirBC0t2a4Z5DhO60cr9knNEkOnD3Em7f6eywkYO+VS 5BEyx+d82UC+cZhrWVh3yOCadvK8Xqsi3eZTp6dpW5ab2NyY89JYiDgVgrnzEZmAfI2f p6BpfrPXawrthC0ShCsn9D6G7gjtqg6bu9WABrxPiPKfX7oLKNlnjBgLS4DzWv9oEllV Ffvw== X-Forwarded-Encrypted: i=1; AJvYcCXGyez81NtjIMVp7aGkB0acW9/ifSetWicX03VqzSFBMAK6b4yjBaf4LNwQ6AGgk7cG5YryY24muhQv8rQ=@vger.kernel.org X-Gm-Message-State: AOJu0Yz1iYcr8AS1nPIvcFTW04NEFFICVioQrh9Ma7x8lxEkKjWbgDnD o8rp2epcPW2jOhuFJpZ55Z/yv4i34A2BxewTtbLF+Ax19mHGsXT4GIyqCSDo1vnY8Q== X-Gm-Gg: ATEYQzzkSgtfR3oWec+3B0wNjfgHc4WhR/yALPU821aEXVkahWyy2PzOuuPqDhYHNaq 0GBVlmJWQx4MiZ0anj6rNXwUr8hu73tkUlFJQ7AuUasJNAkHrAwSxbzXhlvrD9IYh6qfH6Lsp1m FOBuF18aDP6wp2/P6Hwl80LVfFvqEWEom3V/GdAqFcF0aBk/dRy/5iYvkKUthZsw2EDTX2gAPEA eHmGX3WLeODSmsYUJ00GuxIS5+wXgy7QosHzOIlNtg21+qDBwgmPNJOs9Ho6agIFEAkN0OUhC5M wEnmXim94w8g/64Rochupo13zGYLUJA3pO/Dg0bvyr6IxBn3IGxDzoRNTN0lZRKnkwFDXbn2Yxq riSDfTBOuNqLDlNU/qPFtUmmszGkbBXzeeUWGQ41530sWF0EISyRKCKRR7g40ZqqEC943uRQaDU rV0nWLw/IUSPuH9aRcCzEozid/3GW1mjCrgkAdXDcJW3Tu27iwODB4D2ljy6Z0 X-Received: by 2002:a17:902:c412:b0:2a9:5bfa:54ef with SMTP id d9443c01a7336-2adca6c8e0amr234085ad.10.1771959787309; Tue, 24 Feb 2026 11:03:07 -0800 (PST) Received: from google.com (222.245.187.35.bc.googleusercontent.com. [35.187.245.222]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-2ad74d36911sm114902575ad.0.2026.02.24.11.02.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 24 Feb 2026 11:03:06 -0800 (PST) Date: Tue, 24 Feb 2026 19:02:56 +0000 From: Pranjal Shrivastava To: David Matlack Cc: Alex Williamson , Adithya Jayachandran , Alexander Graf , Alex Mastro , Alistair Popple , Andrew Morton , Ankit Agrawal , Bjorn Helgaas , Chris Li , David Rientjes , Jacob Pan , Jason Gunthorpe , Jason Gunthorpe , Jonathan Corbet , Josh Hilke , Kevin Tian , kexec@lists.infradead.org, kvm@vger.kernel.org, Leon Romanovsky , Leon Romanovsky , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-mm@kvack.org, linux-pci@vger.kernel.org, Lukas Wunner , =?utf-8?Q?Micha=C5=82?= Winiarski , Mike Rapoport , Parav Pandit , Pasha Tatashin , Pratyush Yadav , Raghavendra Rao Ananta , Rodrigo Vivi , Saeed Mahameed , Samiullah Khawaja , Shuah Khan , Thomas =?iso-8859-1?Q?Hellstr=F6m?= , Tomita Moeko , Vipin Sharma , Vivek Kasireddy , William Tu , Yi Liu , Zhu Yanjun Subject: Re: [PATCH v2 02/22] PCI: Add API to track PCI devices preserved across Live Update Message-ID: References: <20260129212510.967611-1-dmatlack@google.com> <20260129212510.967611-3-dmatlack@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=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Tue, Feb 24, 2026 at 09:33:28AM -0800, David Matlack wrote: > On Tue, Feb 24, 2026 at 1:18 AM Pranjal Shrivastava wrote: > > On Thu, Jan 29, 2026 at 09:24:49PM +0000, David Matlack wrote: > > > + * Copyright (c) 2025, Google LLC. > > > > Nit: Should these be 2026 now? > > Yes! Thanks for catching that. > > > > +int pci_liveupdate_outgoing_preserve(struct pci_dev *dev) > > > +{ > > > + struct pci_dev_ser new = INIT_PCI_DEV_SER(dev); > > > + struct pci_ser *ser; > > > + int i, ret; > > > + > > > + /* Preserving VFs is not supported yet. */ > > > + if (dev->is_virtfn) > > > + return -EINVAL; > > > + > > > + guard(mutex)(&pci_flb_outgoing_lock); > > > + > > > + if (dev->liveupdate_outgoing) > > > + return -EBUSY; > > > + > > > + ret = liveupdate_flb_get_outgoing(&pci_liveupdate_flb, (void **)&ser); > > > + if (ret) > > > + return ret; > > > + > > > + if (ser->nr_devices == ser->max_nr_devices) > > > + return -E2BIG; > > > > I'm wondering how (or if) this handles hot-plugged devices? > > max_nr_devices is calculated based on for_each_pci_dev at the time of > > the first preservation.. what happens if a device is hotplugged after > > the first device is preserved but before the second one is, does > > max_nr_devices become stale? Since ser->max_nr_devices will not reflect > > the actual possible device count, potentially leading to an unnecessary > > -E2BIG failure? > > Yes, it's possible to run out space to preserve devices if devices are > hot-plugged and then preserved. But I think it's better to defer > handling such a use-case exists (unless you see an obvious simple > solution). So far I am not seeing preserving hot-plugged devices > across Live Update as a high priority use-case to support. > Ack. If we aren't supporting preservation for hot-plug at this point. Let's mention that somewhere? Maybe just a little comment or the kdoc? > > > +u32 pci_liveupdate_incoming_nr_devices(void) > > > +{ > > > + struct pci_ser *ser; > > > + int ret; > > > + > > > + ret = liveupdate_flb_get_incoming(&pci_liveupdate_flb, (void **)&ser); > > > + if (ret) > > > + return 0; > > > > Masking this error looks troubled, in the following patch, I see that > > the retval 0 is treated as a fresh boot, but the IOMMU mappings for that > > BDF might still be preserved? Which could lead to DMA aliasing issues, > > without a hint of what happened since we don't even log anything. > > All fo the non-0 errors indicate there are 0 incoming devices at the > time of the call, so I think returning 0 is appropriate. > > - EOPNOTSUPP: Live Update is not enabled. > - ENODATA: Live Update is finished (all incoming devices have been restored). > - ENOTENT: No PCI data was preserved across the Live Update. > > None of these cover the case where an IOMMU mapping for BDF X is > preserved, but device X is not preserved. This is a case we should > handle in some way... but here is not that place. > > > > > Maybe we could have something like the following: > > > > int pci_liveupdate_incoming_nr_devices(void) > > { > > struct pci_ser *ser; > > int ret; > > > > ret = liveupdate_flb_get_incoming(&pci_liveupdate_flb, (void **)&ser); > > if (ret) { > > if (ret != -ENOENT) > > pr_warn("PCI: Failed to retrieve preservation list: %d\n", ret); > > This would cause this warning to get printed if Live Update was > disabled, or if no PCI devices were preserved. But both of those are > not error scenarios. > I agree, the snippet was just an example. What I'm trying to say here is, what if the retval is -ENOMEM / -ENODATA, the existing code will treat it as a fresh boot because it believes there are no incoming devices. However, since this was an incoming device which failed to be retrieved, there's a chance that it's IOMMU mapping was preserved too. By returning 0, the PCI core will feel free to rebalance bus numbers or reassign BARs. For instance, if the IOMMU already inherited mappings for BDF 02:00.0, but the PCI core (due to this masked error) reassigns a different device to that BDF, we face DMA aliasing or IOMMU faults. Am I missing some context here? > > > +void pci_liveupdate_setup_device(struct pci_dev *dev) > > > +{ > > > + struct pci_ser *ser; > > > + int ret; > > > + > > > + ret = liveupdate_flb_get_incoming(&pci_liveupdate_flb, (void **)&ser); > > > + if (ret) > > > + return; > > > > We should log something here either at info / debug level since the > > error isn't bubbled up and the luo_core doesn't scream about it either. > > Any error from liveupdate_flb_get_incoming() simply means there are no > incoming devices. So I don't think there's any error to report in > dmesg. > > > > + dev->liveupdate_incoming = !!pci_ser_find(ser, dev); > > > > This feels a little hacky, shall we go for something like: > > > > dev->liveupdate_incoming = (pci_ser_find(ser, dev) != NULL); ? > > In my experience in the kernel (mostly from KVM), explicity comparison > to NULL is less preferred to treating a pointer as a boolean. But I'm > ok with following whatever is the locally preferred style for this > kind of check. > No strong feelings there, I see both being used in drivers/pci. > > > @@ -582,6 +583,10 @@ struct pci_dev { > > > u8 tph_mode; /* TPH mode */ > > > u8 tph_req_type; /* TPH requester type */ > > > #endif > > > +#ifdef CONFIG_LIVEUPDATE > > > + unsigned int liveupdate_incoming:1; /* Preserved by previous kernel */ > > > + unsigned int liveupdate_outgoing:1; /* Preserved for next kernel */ > > > +#endif > > > }; > > > > This would start another anon bitfield container, should we move this > > above within the existing bitfield? If we've run pahole and found this > > to be better, then this should be fine. > > Yeah I simply appended these new fields to the very end of the struct. > If we care about optimizing the packing of struct pci_dev I can find a > better place to put it. If you have pahole handy, it would be great to see if these can slide into an existing hole. If not, no big deal for v3.. we can keep it as is Thanks, Praan