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 2C7A7409604 for ; Wed, 12 Aug 2026 23:23:22 +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=1786577005; cv=none; b=ZCCp8JhSxgsusQLbkuKMYfExSVLwUOQYAbD+WXf3fgNW3eqOO9HCdMeuLMaZiZCvvtbZBsAew4yHUGZFDZJk7ZhHo6CwT5cGfS5anA+HF1DH/Iv65onXKYHlUrkb9ZmkjO8skyK50EC+RR87EAC5WTAZTggraaKH9wO30+dyocw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786577005; c=relaxed/simple; bh=7cpnZ4/+ceUOXND5CI1ohyxaSHOso6PtTWdS3LWSlE4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Ys3rFJ4N4dD2GByumZDWk4+njkUznfG8gic19DcqBQ0gvoZN7P4Ufox9X1JV396LotIfenVT9uPOyDYzkx6S3Q/F+XYKzdt7Y6UQFHydl85mvGFi8xQ0q8fdqvduX9DyVfN1RVTuaq/ldYpOPZ+EywR7o1cab0TwSpoTRPoGWIo= 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=LT3w/dw9; 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="LT3w/dw9" Received: by mail-pl1-f172.google.com with SMTP id d9443c01a7336-2cab97c86bdso20215ad.1 for ; Wed, 12 Aug 2026 16:23:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1786577001; x=1787181801; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=htZ7B2PCM+RfMEpW8esUs7JsYRPq+p1qH0eY16r8P3Y=; b=LT3w/dw9QkX8Mxh8L6NhW7z9HDvKbcOTqVmDYG2/m9KZHHhtcQS6j15y4niK+kTMrr m8Y6nfvHVs/VrXvu4RPNqDao6fEWPeBUeQ+GFl/O4GdomKl68mwRhp9c4n/el8eM/FuQ qTCTkGTbGPV28l4395o28C7pFKbqE0O8P5Fv3+Y1/Nd4owcBk7oOu/Xi2LiMpsk1AddR +Y6ENE40PmNbtBm02qAvwtp3jAjgCDB9WQcNxPQVB8FiFW/xZrf+gUiBgTls/BPNMKgv 2ZNd/TimxvvqDV7+hq+C7VNQRHzED0xnw1iep3q20e60mG0/4xN7ioldZWvuGeFSR5N6 oGIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1786577001; x=1787181801; h=in-reply-to:content-disposition:content-type: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 :content-type; bh=htZ7B2PCM+RfMEpW8esUs7JsYRPq+p1qH0eY16r8P3Y=; b=Cj2n1bW1z3t78f8r45nVem0hR0JlvSAl6LdOc/ANA86POjT2+rgQW8vOs4BnXJ9bIj CHRAQ4debMEiieuj15rg/ofqgC7vlDb8kSosCp/qH0DRxtVr9JNVK33lFTfOuWCpKwpC tM+FlihMfzt2q5h3p4ENG6Ym/nv8HuXGl2sKsM4ik7rWuQTK4UkZa4J1qhFAkTBNX37C LquMZSb5vckbT+32YxUT0LI4DDxshbnkz3ljN22KCa5mUAphTiIrTNUtybMhb8ePKj3F 8DVDzC8kS8lepO8G8+hqf+FAIT+uWqxY7K4XRY92BHZm1anmSia8OOyaD4bPCurNCgM8 3Y+A== X-Forwarded-Encrypted: i=1; AHgh+Rp7QtgkZe245c1TwQuXh0MRCmxEwwCKXbWzONxMjlPr/HxNM/8iUwwUhW7ZT0yJ0jjx+x0MR22BRPICahs=@vger.kernel.org X-Gm-Message-State: AOJu0Yyuj2Nsn08v+KKsSB5YE4DUvKtToxl7txm0Cm7UuoE55RucrBJ4 hXsVWIxNF2HkpUjLZs4VgD6ZKdBX2K6VJJnzW9+Xfwu3IbyWGNNvkxQKypRI5W515g== X-Gm-Gg: AR+sD11Jn08VfU3fI0GI+Pa/TZr4wXdSJWV7oPu8xfZzkJgU6+ojhh0APbZJZFpSemR MJsJRb46HtE/tjl/uV/SlUHKUn90sI+ljzlRYRbxQozPQAh6ZADjOHZgwmkxJs4/WHwgNNLnFms umbZs++xB5/rbDIXNX4bGM5mQZzZMpHYjxuT4Kxyo0dRbO6hCSTSxDmuAv+Ofl/KfRfDU0naBeG ksLa3Qx1YqNlNbHRPFM0GhOOHsjr0tL01DK/VC8QHT6NUni9XIN3mGqHjBTPTRtpPPf/N3ruj9t CWDngAEveCqyGGeSXIc6mdeOZi1oU/nUluyUycKPSX7OoRYuY7jQvyJU6Eh7UgRPmmCUpUIyjR7 jjfT0ANJgkCZIKWRQB9iGO4s0t0Yh0TNvtPtJwCfUjZH3S4ZLe9qztsTJw6+RluSrmLuxZYF/tK BGg2uJiJYiTKgDs86Ovxn1SEChT3zdUpoyu1DtCwTzbuu3Kn+X3oo5ojOXUjtTbJ+sN5EyRFSRR JDZ+ks2yEP5s29ISTJkwhFm8SGyPA+mEpXpw1qh+EH+WUTQFNkoqdRiMNo= X-Received: by 2002:a17:903:37cd:b0:2c7:f0cc:dafa with SMTP id d9443c01a7336-2d389d983d6mr866935ad.5.1786577000833; Wed, 12 Aug 2026 16:23:20 -0700 (PDT) Received: from google.com (210.87.127.34.bc.googleusercontent.com. [34.127.87.210]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-3931f44ddf3sm569830a91.14.2026.08.12.16.23.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 12 Aug 2026 16:23:19 -0700 (PDT) Date: Wed, 12 Aug 2026 23:23:16 +0000 From: Samiullah Khawaja To: Ankit Soni 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, Pratyush Yadav , Pasha Tatashin , David Matlack , Andrew Morton , Pranjal Shrivastava , Vipin Sharma Subject: Re: [PATCH v4 09/18] iommu: Add APIs to get iommu and device preserved state Message-ID: References: <20260808022723.3893618-1-skhawaja@google.com> <20260808022723.3893618-10-skhawaja@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: On Wed, Aug 12, 2026 at 06:29:18AM +0000, Ankit Soni wrote: >On Sat, Aug 08, 2026 at 02:27:14AM +0000, Samiullah Khawaja wrote: >> The preserved state of the device and IOMMU needs to be fetched during >> shutdown and boot in the next kernel. Add APIs that can be used to fetch >> the preserved state of a device and IOMMU. The APIs will only be used >> during shutdown and after liveupdate so no locking needed. >> >> Reviewed-by: Pranjal Shrivastava >> Signed-off-by: Samiullah Khawaja >> --- >> drivers/iommu/liveupdate.c | 122 +++++++++++++++++++++++++++++++ >> include/linux/iommu-liveupdate.h | 45 ++++++++++++ >> 2 files changed, 167 insertions(+) >> >> diff --git a/drivers/iommu/liveupdate.c b/drivers/iommu/liveupdate.c >> index 7f349ae5a124..20acf123b47a 100644 >> --- a/drivers/iommu/liveupdate.c >> +++ b/drivers/iommu/liveupdate.c > >../.. > >> @@ -262,6 +273,117 @@ void iommu_liveupdate_unregister_flb(struct liveupdate_file_handler *handler) >> } >> EXPORT_SYMBOL(iommu_liveupdate_unregister_flb); >> >> +/* >> + * iommu_liveupdate_flb_get_incoming() - Helper function to get FLB state >> + * @flb_objp: Pointer to get the restored FLB object >> + * >> + * Return: 0 if FLB state found and restored, error if no data found >> + */ >> +static int iommu_liveupdate_flb_get_incoming(struct iommu_flb_obj **flb_objp) >> +{ >> + struct iommu_flb_obj *flb_obj; >> + int ret; >> + >> + ret = liveupdate_flb_get_incoming(&iommu_flb, (void **)flb_objp); >> + if (ret) >> + return ret; >> + >> + flb_obj = *flb_objp; >> mutex_lock(&flb_obj->lock); >> + >> + /* >> + * FLB version mismatch is considered fatal for security reasons for >> + * now. >> + */ >> + if (flb_obj->ser->version != IOMMU_LUO_FLB_VERSION) >> + goto err_fatal; >> + >> + /* >> + * Array Phys of each type should be valid if the FLB was created for >> + * preservation. This is true even if no devices, iommus or domains were >> + * preserved. >> + */ >> + if (!flb_obj->ser->iommu_array_phys || >> + !flb_obj->ser->device_array_phys || >> + !flb_obj->ser->iommu_domain_array_phys) >> + goto err_fatal; >> + >> + mutex_unlock(&flb_obj->lock); >> + return 0; >> + >> +err_fatal: >> + panic("Failed to restore IOMMU Live Update FLB\n"); >> +} > >../.. > >> +/** >> + * iommu_get_preserved_data() - Get preserved data for an IOMMU HW >> + * @token: Token used to preserve this IOMMU HW >> + * @type: IOMMU type in preserved state >> + * >> + * Gets the preserved state of an IOMMU HW using token and the IOMMU type. >> + * >> + * Return: struct iommu_hw_ser on success, NULL if no preserved state found. >> + */ >> +struct iommu_hw_ser *iommu_get_preserved_data(u64 token, enum iommu_type_ser type) >> +{ >> + struct iommu_hw_ser *iommu_ser = NULL; >> + struct iommu_hw_array_ser *array; >> + struct iommu_flb_obj *flb_obj; >> + int ret, idx; >> + >> + ret = iommu_liveupdate_flb_get_incoming(&flb_obj); >> + if (ret) >> + return NULL; > >Hi, Hi, Thanks for looking at this. > >Returning NULL for every failure loses a distinction the caller needs. >"Nothing was handed over" and "something was handed over and the lookup >failed" are different states, and this helper is the only place that can >tell them apart. > My intention is to handle these details in the helper function iommu_liveupdate_flb_get_incoming() to keep the driver code clean. Basically to allow the caller to decide whether the state is available or not. The various error handling cases can be covered internally, and I based it on the errors that the liveupdate_flb_get_incoming() can return. >Of the errors reachable here, three mean nothing was handed over: > > -EOPNOTSUPP live update disabled or not built This means that the is state not available so returning NULL > -ENODATA no incoming handover data > -ENOENT the handover carried no IOMMU FLB Both mean that no data is available from IOMMU point of view as version is moved into FLB state, so returning NULL. Now down the road we might have cases where the available data has a different version as compared to what this kernel expects. We can handle all those things in this function and if possible hand over data to the caller in the right format it expects. Otherwise we panic() considering mismatch fatal. I will add these details in the documentation. > >An -ENOMEM out of a .retrieve() callback means the opposite, and returns >the same NULL. Ah yes... This needs to be handled internally also. Probably in the .retrieve() callback as -ENOMEM when getting the FLB should be fatal. > >The distinction matters because callers read NULL as a cold boot and act >on it. Both VT-d call sites in 10/18 do. init_dmars(): > > if (!iommu_ser) > init_translation_status(iommu); > > if (translation_pre_enabled(iommu) && !is_kdump_kernel()) { > iommu_disable_translation(iommu); > clear_translation_pre_enabled(iommu); > pr_warn("Translation was enabled for %s but we are not in kdump mode\n", > iommu->name); > } > >and intel_iommu_add(): > > if (!iommu_ser && iommu->gcmd & DMA_GCMD_TE) > iommu_disable_translation(iommu); > >On an -ENOMEM both disable an IOMMU that the previous kernel left >translating, underneath devices still doing DMA through it. The warning >attributes it to a stale enable outside kdump, so the log points away from >the actual cause. > >The helper already treats this class of failure as fatal a few lines >lower. A version mismatch and a missing array pointer both reach err_fatal >and panic(), because data was handed over and cannot be trusted. A failed >fetch is the same class -- something was handed over and could not be read >back -- but it returns an error that becomes NULL, and callers read that >as a cold boot. > >A single failure also spreads. luo_flb_retrieve_one() caches a failed >.retrieve() in incoming.retrieve_status and returns it directly on every >later call, so one -ENOMEM makes the lookup return NULL for every IOMMU in >the system, not just the one that hit it. > >Is returning NULL for all of them intentional? If callers are meant to >treat every failure as "nothing was handed over", that is worth saying in >a comment here. > >If not, the smallest fix is to return NULL for the absent cases only and >an ERR_PTR for everything else: This is what I was doing in previous version, but it gets too messy. Moving it into the helper handles fatal errors, and versioning down the road, cleanly. Thanks, Sami