From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f54.google.com (mail-wm1-f54.google.com [209.85.128.54]) (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 866143054EB for ; Fri, 2 Jan 2026 18:26:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.54 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767378408; cv=none; b=pNF1Hbavh/5gUYmZcnqpmFd2VKgNnNm36ilRIZONkhjdWaLbZYFna9hd/ioAM8f/5JNods4+2BUm3xEXQ17aNWlbKerA1Wg+fA/2LFXyhUG3AQo/tIbyz8h0Q2ufMweldnftUpstmg+ptjOlhRZksxYgOVl0XkR6/Ob8/CrqRHU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767378408; c=relaxed/simple; bh=5nM+eL5LamiqbxWTUlEwUuzZAZDl05AsANtahARrgQs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=XZMME8ne7STloNCGR+JzaAb13V5D3MSEF8ZRGXgwPNwVldv6QdnGWQiHd8UEYZssywG+vWJxc6QhgDpj1zws4LnzC7K2jGm9idnf9kseSko4tMpS1h/WYe2/4Cdtjqdf6tTv+Y0Kg53co8VshNf/DzIOIVCrzShOMWVOj9nAMzQ= 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=rdhYYcSn; arc=none smtp.client-ip=209.85.128.54 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="rdhYYcSn" Received: by mail-wm1-f54.google.com with SMTP id 5b1f17b1804b1-47a95a96d42so1935e9.1 for ; Fri, 02 Jan 2026 10:26:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1767378405; x=1767983205; 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=C6VmRE84hy1tV7GxjuvF11hL7eqNjgvI2Nm6sb2SoCM=; b=rdhYYcSnbxNcCLm781ryT/yfAZ3fZRady5mSEOGgYz1loFUXVbSRDHpuB/dpohSxpZ 5LwLW5UWQqY4gxY8xRHe5Bno4x9z9Cp0cTQZABaM1JOm73gOPfYK4+uFKFFl2pHIc+f4 obVWdrHgbLpT2VKDgpowu1gCeft/OMVHDrnedwZTUnt63zXocziaCKKdmTpG4IbokUZ8 fsN8viP5nKbYnqdDlp4BZZXPtRCQwhMEQjgN9maedWM85ZShvkRoH7mw0UbkXiS7HbIF 86+LgSoXSMiD6vHaKX/XsuYmdNLDPjBv8aukOD5E5TlSGZg6MAfUa1IwA+SRZ6dcLecs xfVQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1767378405; x=1767983205; 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=C6VmRE84hy1tV7GxjuvF11hL7eqNjgvI2Nm6sb2SoCM=; b=an1iPrQBfkwmueMTOUP4uo/9BJPlcSjfzicwNavgDf7bnvq4w++C+hHqVRCXy5vuZv jrI1mcf8k9xMHwiZn1i6pW+R9gE5x7q/W+gZ/+yQt1b5a8FTjUfbCgqZSDdqdvonYdRC ZGBl40m5o8k+wfqqnM4wykyPE6oKNf3PfJNAsuA3uJhaeWGAq20SBfQSojLmjYIKpjrB N0RotWmoh4QZuseKvqIPALei7rwz1QNZt9364Ak5f8lSCoxPxmAQTxHj7rPNrMPGgUB+ yjvVGaeIDvbWin1w/UFqvFb0ucGj0qZEHWVZnpbF4/BmUk28EDM+vkkUSz0NhDi7qER7 4pJw== X-Forwarded-Encrypted: i=1; AJvYcCVOANqkNk6q37Txih2XVkH7nbfbL1xK/4ulPTsr5TtAj3nI1Huv5HKQH3J2FGARzdPntAbza/QmJt5paao=@vger.kernel.org X-Gm-Message-State: AOJu0Yzofrq+3k1/vgFCBIm5AMPixxxtHTH0gx1ep1Sn4I5Q01o51MhQ 40B6LFeYJicuXYOjowNFobZf0rUbZPDDbRvCFM8EiGT+nUNmFCwX/hQ2NDIRBpDISA== X-Gm-Gg: AY/fxX5l60jtrD+KV588LwdC7KZjJNjq/J3eAcAW2N0iJPbXGXTjwuYtStZnPP2rz6e dKZFj4ok0isD42WLS2W2tF/+gjJJ9ask9syZ45hn36g2VCt38/IwtNxr60A+DV702YW5qBGU6sk y2M/rALiP+BIR21616X8ZSa0hIXWOAu2MUEgOCB99wYOr6PVPM4CUaD6Xbq0ONXpkKDOkGCmhVt p9nVXS3r9bZodrUNnZPyf4p3po3BJVWxVPoRfRDbmzDkbR7VRiEopwzuSTcp0quHQDi2sa7qFnN 5AE8QmbJUU7EhffpYHqtMLVfrFM1r71OjxJeyEuL1M5429ZLU3oko5O/YVyfw3T1Ctfo/GJOEoR sqdcE6XGs5L9CqKtRPqlCRGNuso47BgzFc4s60KpK/DHVIW4M6u0Stsfw674/ZQlAxgK+G3s31u iSYcDZbmhi+NkBX5CRPC6EcuEBEQ+cBdpzAgWhvUor5Xsn1bKnvMLGR374UYzHh2o= X-Google-Smtp-Source: AGHT+IGwg8Fa4ecp7Nausir70D/wkitZs+xtIWbvFB+O66NW8MoGvR5fFj6DnKi9U+mtSuI4iqzZiw== X-Received: by 2002:a05:600c:207:b0:47a:80ec:b2f7 with SMTP id 5b1f17b1804b1-47d6c367ecemr12085e9.14.1767378404531; Fri, 02 Jan 2026 10:26:44 -0800 (PST) Received: from google.com (171.85.155.104.bc.googleusercontent.com. [104.155.85.171]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-47d193621c8sm751447155e9.7.2026.01.02.10.26.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 02 Jan 2026 10:26:43 -0800 (PST) Date: Fri, 2 Jan 2026 18:26:40 +0000 From: Mostafa Saleh To: Nicolin Chen Cc: will@kernel.org, robin.murphy@arm.com, jgg@nvidia.com, joro@8bytes.org, linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev, linux-kernel@vger.kernel.org, skolothumtho@nvidia.com, praan@google.com, xueshuai@linux.alibaba.com Subject: Re: [PATCH rc v5 1/4] iommu/arm-smmu-v3: Add update_safe bits to fix STE update sequence Message-ID: References: <58f5af553fa7c3b5fd16f1eb13a81ae428f85678.1766093909.git.nicolinc@nvidia.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 Content-Disposition: inline In-Reply-To: <58f5af553fa7c3b5fd16f1eb13a81ae428f85678.1766093909.git.nicolinc@nvidia.com> On Thu, Dec 18, 2025 at 01:41:56PM -0800, Nicolin Chen wrote: > From: Jason Gunthorpe > > C_BAD_STE was observed when updating nested STE from an S1-bypass mode to > an S1DSS-bypass mode. As both modes enabled S2, the used bit is slightly > different than the normal S1-bypass and S1DSS-bypass modes. As a result, > fields like MEV and EATS in S2's used list marked the word1 as a critical > word that requested a STE.V=0. This breaks a hitless update. > > However, both MEV and EATS aren't critical in terms of STE update. One > controls the merge of the events and the other controls the ATS that is > managed by the driver at the same time via pci_enable_ats(). > > Add an arm_smmu_get_ste_update_safe() to allow STE update algorithm to > relax those fields, avoiding the STE update breakages. > > After this change, entry_set has no caller checking its return value, so > change it to void. > > Note that this change is required by both MEV and EATS fields, which were > introduced in different kernel versions. So add get_update_safe() first. > MEV and EATS will be added to arm_smmu_get_ste_update_safe() separately. > > Fixes: 1e8be08d1c91 ("iommu/arm-smmu-v3: Support IOMMU_DOMAIN_NESTED") > Cc: stable@vger.kernel.org > Signed-off-by: Jason Gunthorpe > Reviewed-by: Shuai Xue > Signed-off-by: Nicolin Chen Reviewed-by: Mostafa Saleh Thanks, Mostafa > --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 2 ++ > .../iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c | 18 ++++++++++--- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 27 ++++++++++++++----- > 3 files changed, 37 insertions(+), 10 deletions(-) > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > index ae23aacc3840..a6c976fa9df2 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > @@ -900,6 +900,7 @@ struct arm_smmu_entry_writer { > > struct arm_smmu_entry_writer_ops { > void (*get_used)(const __le64 *entry, __le64 *used); > + void (*get_update_safe)(__le64 *safe_bits); > void (*sync)(struct arm_smmu_entry_writer *writer); > }; > > @@ -911,6 +912,7 @@ void arm_smmu_make_s2_domain_ste(struct arm_smmu_ste *target, > > #if IS_ENABLED(CONFIG_KUNIT) > void arm_smmu_get_ste_used(const __le64 *ent, __le64 *used_bits); > +void arm_smmu_get_ste_update_safe(__le64 *safe_bits); > void arm_smmu_write_entry(struct arm_smmu_entry_writer *writer, __le64 *cur, > const __le64 *target); > void arm_smmu_get_cd_used(const __le64 *ent, __le64 *used_bits); > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c > index d2671bfd3798..5db14718fdd6 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c > @@ -38,13 +38,16 @@ enum arm_smmu_test_master_feat { > static bool arm_smmu_entry_differs_in_used_bits(const __le64 *entry, > const __le64 *used_bits, > const __le64 *target, > + const __le64 *safe, > unsigned int length) > { > bool differs = false; > unsigned int i; > > for (i = 0; i < length; i++) { > - if ((entry[i] & used_bits[i]) != target[i]) > + __le64 used = used_bits[i] & ~safe[i]; > + > + if ((entry[i] & used) != (target[i] & used)) > differs = true; > } > return differs; > @@ -56,12 +59,17 @@ arm_smmu_test_writer_record_syncs(struct arm_smmu_entry_writer *writer) > struct arm_smmu_test_writer *test_writer = > container_of(writer, struct arm_smmu_test_writer, writer); > __le64 *entry_used_bits; > + __le64 *safe; > > entry_used_bits = kunit_kzalloc( > test_writer->test, sizeof(*entry_used_bits) * NUM_ENTRY_QWORDS, > GFP_KERNEL); > KUNIT_ASSERT_NOT_NULL(test_writer->test, entry_used_bits); > > + safe = kunit_kzalloc(test_writer->test, > + sizeof(*safe) * NUM_ENTRY_QWORDS, GFP_KERNEL); > + KUNIT_ASSERT_NOT_NULL(test_writer->test, safe); > + > pr_debug("STE value is now set to: "); > print_hex_dump_debug(" ", DUMP_PREFIX_NONE, 16, 8, > test_writer->entry, > @@ -79,14 +87,17 @@ arm_smmu_test_writer_record_syncs(struct arm_smmu_entry_writer *writer) > * configuration. > */ > writer->ops->get_used(test_writer->entry, entry_used_bits); > + if (writer->ops->get_update_safe) > + writer->ops->get_update_safe(safe); > KUNIT_EXPECT_FALSE( > test_writer->test, > arm_smmu_entry_differs_in_used_bits( > test_writer->entry, entry_used_bits, > - test_writer->init_entry, NUM_ENTRY_QWORDS) && > + test_writer->init_entry, safe, > + NUM_ENTRY_QWORDS) && > arm_smmu_entry_differs_in_used_bits( > test_writer->entry, entry_used_bits, > - test_writer->target_entry, > + test_writer->target_entry, safe, > NUM_ENTRY_QWORDS)); > } > } > @@ -106,6 +117,7 @@ arm_smmu_v3_test_debug_print_used_bits(struct arm_smmu_entry_writer *writer, > static const struct arm_smmu_entry_writer_ops test_ste_ops = { > .sync = arm_smmu_test_writer_record_syncs, > .get_used = arm_smmu_get_ste_used, > + .get_update_safe = arm_smmu_get_ste_update_safe, > }; > > static const struct arm_smmu_entry_writer_ops test_cd_ops = { > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > index d16d35c78c06..8dbf4ad5b51e 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > @@ -1082,6 +1082,12 @@ void arm_smmu_get_ste_used(const __le64 *ent, __le64 *used_bits) > } > EXPORT_SYMBOL_IF_KUNIT(arm_smmu_get_ste_used); > > +VISIBLE_IF_KUNIT > +void arm_smmu_get_ste_update_safe(__le64 *safe_bits) > +{ > +} > +EXPORT_SYMBOL_IF_KUNIT(arm_smmu_get_ste_update_safe); > + > /* > * Figure out if we can do a hitless update of entry to become target. Returns a > * bit mask where 1 indicates that qword needs to be set disruptively. > @@ -1094,13 +1100,22 @@ static u8 arm_smmu_entry_qword_diff(struct arm_smmu_entry_writer *writer, > { > __le64 target_used[NUM_ENTRY_QWORDS] = {}; > __le64 cur_used[NUM_ENTRY_QWORDS] = {}; > + __le64 safe[NUM_ENTRY_QWORDS] = {}; > u8 used_qword_diff = 0; > unsigned int i; > > writer->ops->get_used(entry, cur_used); > writer->ops->get_used(target, target_used); > + if (writer->ops->get_update_safe) > + writer->ops->get_update_safe(safe); > > for (i = 0; i != NUM_ENTRY_QWORDS; i++) { > + /* > + * Safe is only used for bits that are used by both entries, > + * otherwise it is sequenced according to the unused entry. > + */ > + safe[i] &= target_used[i] & cur_used[i]; > + > /* > * Check that masks are up to date, the make functions are not > * allowed to set a bit to 1 if the used function doesn't say it > @@ -1109,6 +1124,7 @@ static u8 arm_smmu_entry_qword_diff(struct arm_smmu_entry_writer *writer, > WARN_ON_ONCE(target[i] & ~target_used[i]); > > /* Bits can change because they are not currently being used */ > + cur_used[i] &= ~safe[i]; > unused_update[i] = (entry[i] & cur_used[i]) | > (target[i] & ~cur_used[i]); > /* > @@ -1121,7 +1137,7 @@ static u8 arm_smmu_entry_qword_diff(struct arm_smmu_entry_writer *writer, > return used_qword_diff; > } > > -static bool entry_set(struct arm_smmu_entry_writer *writer, __le64 *entry, > +static void entry_set(struct arm_smmu_entry_writer *writer, __le64 *entry, > const __le64 *target, unsigned int start, > unsigned int len) > { > @@ -1137,7 +1153,6 @@ static bool entry_set(struct arm_smmu_entry_writer *writer, __le64 *entry, > > if (changed) > writer->ops->sync(writer); > - return changed; > } > > /* > @@ -1207,12 +1222,9 @@ void arm_smmu_write_entry(struct arm_smmu_entry_writer *writer, __le64 *entry, > entry_set(writer, entry, target, 0, 1); > } else { > /* > - * No inuse bit changed. Sanity check that all unused bits are 0 > - * in the entry. The target was already sanity checked by > - * compute_qword_diff(). > + * No inuse bit changed, though safe bits may have changed. > */ > - WARN_ON_ONCE( > - entry_set(writer, entry, target, 0, NUM_ENTRY_QWORDS)); > + entry_set(writer, entry, target, 0, NUM_ENTRY_QWORDS); > } > } > EXPORT_SYMBOL_IF_KUNIT(arm_smmu_write_entry); > @@ -1543,6 +1555,7 @@ static void arm_smmu_ste_writer_sync_entry(struct arm_smmu_entry_writer *writer) > static const struct arm_smmu_entry_writer_ops arm_smmu_ste_writer_ops = { > .sync = arm_smmu_ste_writer_sync_entry, > .get_used = arm_smmu_get_ste_used, > + .get_update_safe = arm_smmu_get_ste_update_safe, > }; > > static void arm_smmu_write_ste(struct arm_smmu_master *master, u32 sid, > -- > 2.43.0 >