* [PATCH v3 6/8] crypto: qce - Fix xts-aes-qce for weak keys
From: Bartosz Golaszewski @ 2026-06-17 15:49 UTC (permalink / raw)
To: Thara Gopinath, Herbert Xu, David S. Miller, Stanimir Varbanov,
Eneas U de Queiroz, Kuldeep Singh, Eric Biggers
Cc: linux-crypto, linux-arm-msm, linux-kernel, brgl,
Bartosz Golaszewski, stable
In-Reply-To: <20260617-qce-fix-self-tests-v3-0-ecc2b4dedcfd@oss.qualcomm.com>
From: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
The QCE hardware does not support AES XTS mode when key1 and key2 are
equal. The driver was handling this by unconditionally rejecting the
keys with -ENOKEY(-126), regardless of whether FIPS mode is active or
the FORBID_WEAK_KEYS flag is set.
[ 5.599170] alg: skcipher: xts-aes-qce setkey failed on test vector 0; expected_error=0, actual_error=-126, flags=0x1
[ 5.599184] alg: self-tests for xts(aes) using xts-aes-qce failed (rc=-126)
In general for weak keys,
- If FIPS mode is active or FORBID_WEAK_KEYS is set: return -EINVAL.
- In non-FIPS mode, Accept the key and encrypt successfully.
Since QCE was returning -ENOKEY for non-FIPS mode whereas the
expectation is to encrypt content and return success, the selftest saw a
mismatch and failed.
There are two problems in QCE behavior:
* -ENOKEY is returned instead of -EINVAL for the FIPS/weak-key
rejection case.
* key1 == key2 is rejected even in non-FIPS mode
Fix xts-aes-qce behavior by using generic helper xts_verify_key() to
reject keys early with -EINVAL for FIPS mode active(or FORBID_WEAK_KEYS
set). For non-FIPS mode, since QCE hardware cannot accept the keys, use
software fallback mechanism to encrypt the data.
Cc: stable@vger.kernel.org
Fixes: f0d078dd6c49 ("crypto: qce - Return unsupported if key1 and key 2 are same for AES XTS algorithm")
Signed-off-by: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/crypto/qce/cipher.h | 1 +
drivers/crypto/qce/skcipher.c | 20 +++++++++++++-------
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/crypto/qce/cipher.h b/drivers/crypto/qce/cipher.h
index 850f257d00f3aca0397adc1f703aea690c754d60..daea07551118d444d2f749588bdfe2ae2c6c553f 100644
--- a/drivers/crypto/qce/cipher.h
+++ b/drivers/crypto/qce/cipher.h
@@ -14,6 +14,7 @@
struct qce_cipher_ctx {
u8 enc_key[QCE_MAX_KEY_SIZE];
unsigned int enc_keylen;
+ bool use_fallback;
struct crypto_skcipher *fallback;
};
diff --git a/drivers/crypto/qce/skcipher.c b/drivers/crypto/qce/skcipher.c
index 118a6878a76b1e86534f60e5d2058b99a689302e..9c1ce69adab8309737e15a50826505898340bcd9 100644
--- a/drivers/crypto/qce/skcipher.c
+++ b/drivers/crypto/qce/skcipher.c
@@ -12,6 +12,7 @@
#include <linux/errno.h>
#include <crypto/aes.h>
#include <crypto/internal/skcipher.h>
+#include <crypto/xts.h>
#include "cipher.h"
@@ -194,14 +195,17 @@ static int qce_skcipher_setkey(struct crypto_skcipher *ablk, const u8 *key,
if (!key || !keylen)
return -EINVAL;
- /*
- * AES XTS key1 = key2 not supported by crypto engine.
- * Revisit to request a fallback cipher in this case.
- */
if (IS_XTS(flags)) {
+ ret = xts_verify_key(ablk, key, keylen);
+ if (ret)
+ return ret;
__keylen = keylen >> 1;
- if (!memcmp(key, key + __keylen, __keylen))
- return -ENOKEY;
+ /*
+ * QCE does not support key1 == key2 for XTS.
+ * Use fallback cipher in this case.
+ */
+ ctx->use_fallback = !crypto_memneq(key, key + __keylen,
+ __keylen);
} else {
__keylen = keylen;
}
@@ -262,13 +266,15 @@ static int qce_skcipher_crypt(struct skcipher_request *req, int encrypt)
* needed in all versions of CE)
* AES-CTR with a partial final block (the CE stalls waiting for a full
* block of input).
+ * AES-XTS with key1 == key2 (not supported by the CE).
*/
if (IS_AES(rctx->flags) &&
((keylen != AES_KEYSIZE_128 && keylen != AES_KEYSIZE_256) ||
(IS_CTR(rctx->flags) && !IS_ALIGNED(req->cryptlen, AES_BLOCK_SIZE)) ||
(IS_XTS(rctx->flags) && ((req->cryptlen <= aes_sw_max_len) ||
(req->cryptlen > QCE_SECTOR_SIZE &&
- req->cryptlen % QCE_SECTOR_SIZE))))) {
+ req->cryptlen % QCE_SECTOR_SIZE))) ||
+ (IS_XTS(rctx->flags) && ctx->use_fallback))) {
skcipher_request_set_tfm(&rctx->fallback_req, ctx->fallback);
skcipher_request_set_callback(&rctx->fallback_req,
req->base.flags,
--
2.47.3
^ permalink raw reply related
* [PATCH v3 7/8] crypto: qce - Use a fallback for CCM with a partial final block
From: Bartosz Golaszewski @ 2026-06-17 15:49 UTC (permalink / raw)
To: Thara Gopinath, Herbert Xu, David S. Miller, Stanimir Varbanov,
Eneas U de Queiroz, Kuldeep Singh, Eric Biggers
Cc: linux-crypto, linux-arm-msm, linux-kernel, brgl,
Bartosz Golaszewski, stable
In-Reply-To: <20260617-qce-fix-self-tests-v3-0-ecc2b4dedcfd@oss.qualcomm.com>
CCM builds on AES-CTR for encryption, and the crypto engine stalls on a
partial final block just as it does for plain ctr(aes): a payload whose
length is not a multiple of the AES block size leaves the operation
incomplete and fails with a hardware operation error. This was caught by
the ccm(aes) crypto self-tests.
Force the software fallback for CCM requests whose message length is not
block aligned, reusing the driver's existing need_fallback mechanism.
Cc: stable@vger.kernel.org
Fixes: 9363efb4181c ("crypto: qce - Add support for AEAD algorithms")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/crypto/qce/aead.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/crypto/qce/aead.c b/drivers/crypto/qce/aead.c
index 336614a11377e0be246817da584296124f4de5d8..4fa018204cb628c112f64c45ff6c7407df73b945 100644
--- a/drivers/crypto/qce/aead.c
+++ b/drivers/crypto/qce/aead.c
@@ -514,6 +514,14 @@ static int qce_aead_crypt(struct aead_request *req, int encrypt)
ctx->need_fallback = true;
}
+ /*
+ * CCM uses AES-CTR internally and the CE stalls on a partial final
+ * block, so a payload that is not a multiple of the block size has to
+ * be handled by the fallback.
+ */
+ if (IS_CCM(rctx->flags) && !IS_ALIGNED(rctx->cryptlen, AES_BLOCK_SIZE))
+ ctx->need_fallback = true;
+
/* If fallback is needed, schedule and exit */
if (ctx->need_fallback) {
/* Reset need_fallback in case the same ctx is used for another transaction */
--
2.47.3
^ permalink raw reply related
* [PATCH v3 8/8] crypto: qce - Use fallback for CCM with a fragmented payload
From: Bartosz Golaszewski @ 2026-06-17 15:49 UTC (permalink / raw)
To: Thara Gopinath, Herbert Xu, David S. Miller, Stanimir Varbanov,
Eneas U de Queiroz, Kuldeep Singh, Eric Biggers
Cc: linux-crypto, linux-arm-msm, linux-kernel, brgl,
Bartosz Golaszewski, stable
In-Reply-To: <20260617-qce-fix-self-tests-v3-0-ecc2b4dedcfd@oss.qualcomm.com>
The crypto engine reliably processes CCM only when the message payload
is a single contiguous buffer. The associated data is already linearized
into a bounce buffer before being submitted, but when the payload itself
is split across multiple scatterlist entries the engine stalls waiting
for input and the request fails with a hardware operation error. This
was uncovered by the crypto self-tests, which feed the algorithms
randomly fragmented buffers.
Detect a payload that spans more than one scatterlist entry (in either
the source or the destination, skipping past the associated data) and
route the request to the software fallback.
Cc: stable@vger.kernel.org
Fixes: 9363efb4181c ("crypto: qce - Add support for AEAD algorithms")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/crypto/qce/aead.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/qce/aead.c b/drivers/crypto/qce/aead.c
index 4fa018204cb628c112f64c45ff6c7407df73b945..9ff8fe2a7efcd2734e4ff029744961a7b1101013 100644
--- a/drivers/crypto/qce/aead.c
+++ b/drivers/crypto/qce/aead.c
@@ -498,7 +498,8 @@ static int qce_aead_crypt(struct aead_request *req, int encrypt)
struct qce_aead_reqctx *rctx = aead_request_ctx_dma(req);
struct qce_aead_ctx *ctx = crypto_aead_ctx(tfm);
struct qce_alg_template *tmpl = to_aead_tmpl(tfm);
- unsigned int blocksize = crypto_aead_blocksize(tfm);
+ unsigned int blocksize = crypto_aead_blocksize(tfm), authsize;
+ struct scatterlist __sg[2], *msg_sg;
rctx->flags = tmpl->alg_flags;
rctx->flags |= encrypt ? QCE_ENCRYPT : QCE_DECRYPT;
@@ -522,6 +523,27 @@ static int qce_aead_crypt(struct aead_request *req, int encrypt)
if (IS_CCM(rctx->flags) && !IS_ALIGNED(rctx->cryptlen, AES_BLOCK_SIZE))
ctx->need_fallback = true;
+ /*
+ * The CE reliably processes CCM only when the message payload is a
+ * single contiguous buffer. The associated data is linearized into a
+ * bounce buffer before being handed to the engine, but a fragmented
+ * payload makes the engine stall waiting for input, so route those
+ * requests to the fallback.
+ */
+ if (IS_CCM(rctx->flags) && rctx->cryptlen) {
+ authsize = ctx->authsize;
+
+ msg_sg = scatterwalk_ffwd(__sg, req->src, req->assoclen);
+ if (sg_nents_for_len(msg_sg, rctx->cryptlen +
+ (encrypt ? 0 : authsize)) > 1)
+ ctx->need_fallback = true;
+
+ msg_sg = scatterwalk_ffwd(__sg, req->dst, req->assoclen);
+ if (sg_nents_for_len(msg_sg, rctx->cryptlen +
+ (encrypt ? authsize : 0)) > 1)
+ ctx->need_fallback = true;
+ }
+
/* If fallback is needed, schedule and exit */
if (ctx->need_fallback) {
/* Reset need_fallback in case the same ctx is used for another transaction */
--
2.47.3
^ permalink raw reply related
* Re: [PATCH v3 1/8] crypto: qce - Remove unsafe/deprecated algorithms
From: Eric Biggers @ 2026-06-17 16:05 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Thara Gopinath, Herbert Xu, David S. Miller, Stanimir Varbanov,
Eneas U de Queiroz, Kuldeep Singh, linux-crypto, linux-arm-msm,
linux-kernel, brgl, stable
In-Reply-To: <20260617-qce-fix-self-tests-v3-1-ecc2b4dedcfd@oss.qualcomm.com>
On Wed, Jun 17, 2026 at 05:49:30PM +0200, Bartosz Golaszewski wrote:
> Remove algorithms that are either unsafe or deprecated and have no
> in-kernel users that cannot be served by the ARM CE implementations.
>
> AES-ECB reveals plaintext patterns (identical plaintext blocks produce
> identical ciphertext blocks) and should not be exposed as a hardware-
> accelerated primitive. DES, Triple DES and HMAC-SHA1 have been
> deprecated for years.
>
> Remove sha1, ecb(aes), ecb(des), cbc(des), ecb(des3_ede), cbc(des3_ede),
> hmac(sha1) and all AEAD variants built on these primitives as well as
> authenc(hmac(sha256),cbc(des)). Also clean up the - now dead - code,
> flags and constants.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Acked-by: Eric Biggers <ebiggers@kernel.org>
Looks pretty comprehensive, but I did notice a few leftovers: a comment
still mentions DES3_EDE_BLOCK_SIZE, and there's still some ECB-related
code (grep for ENCR_MODE_ECB, QCE_MODE_ECB, and IS_ECB).
- Eric
^ permalink raw reply
* Re: [RFC PATCH 1/2] rust: crypto: add library AES-128 / SHA-256 / HMAC-SHA256 bindings
From: Eric Biggers @ 2026-06-17 17:18 UTC (permalink / raw)
To: Mike Lothian
Cc: rust-for-linux, linux-crypto, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Daniel Almeida,
Greg Kroah-Hartman, Yury Norov (NVIDIA), Asahi Lina,
Lorenzo Stoakes, Joel Fernandes, Alexandre Courbot,
FUJITA Tomonori, Krishna Ketan Rai, linux-kernel
In-Reply-To: <20260617150143.2152-2-mike@fireburn.co.uk>
On Wed, Jun 17, 2026 at 04:01:32PM +0100, Mike Lothian wrote:
> +/*
> + * AES-128 single-block ECB encryption: out = AES(key, in).
> + *
> + * A helper because aes_encrypt() takes a transparent union (aes_encrypt_arg)
> + * that bindgen cannot express. SHA-256 and HMAC-SHA256 are plain extern
> + * functions and are bound directly.
> + */
> +__rust_helper int
> +rust_helper_aes128_encrypt_block(const u8 *key, const u8 *in, u8 *out)
> +{
> + struct aes_enckey enckey;
> + int ret;
> +
> + ret = aes_prepareenckey(&enckey, key, AES_KEYSIZE_128);
> + if (ret)
> + return ret;
> + aes_encrypt(&enckey, out, in);
> + memzero_explicit(&enckey, sizeof(enckey));
> + return 0;
> +}
This is kind of an anti-pattern, both in expanding the key for every
block and also exposing bare AES instead of AES modes of operation.
It's true that lib/crypto/ is missing a lot of AES modes (I'm working on
that), but AES-CMAC is there already which is one of the two you need.
- Eric
^ permalink raw reply
* Re: [RFC PATCH 2/2] rust: crypto: add RSA public-key encryption via crypto_akcipher
From: Eric Biggers @ 2026-06-17 17:52 UTC (permalink / raw)
To: Mike Lothian
Cc: rust-for-linux, linux-crypto, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Daniel Almeida,
Greg Kroah-Hartman, Yury Norov (NVIDIA), Asahi Lina,
Lorenzo Stoakes, Joel Fernandes, Alexandre Courbot,
FUJITA Tomonori, Krishna Ketan Rai, linux-kernel
In-Reply-To: <20260617150143.2152-3-mike@fireburn.co.uk>
On Wed, Jun 17, 2026 at 04:01:33PM +0100, Mike Lothian wrote:
> Add a Rust binding for the asynchronous public-key cipher API
> (crypto_akcipher), driven synchronously, and a crypto::rsa_pubkey_encrypt()
> convenience built on it.
Could you keep the crypto_akcipher support private to the file and just
expose rsa_pubkey_encrypt()? I don't want the use of crypto_akcipher to
be growing significantly. It's a very bad API, as I think is clear by
all the workarounds you had to implement to use it. I'd like to replace
it with lib/crypto/ APIs for RSA eventually.
- Eric
^ permalink raw reply
* Re: [PATCH v8 4/7] x86/sev: Add support to perform RMP optimizations asynchronously
From: Kalra, Ashish @ 2026-06-17 21:57 UTC (permalink / raw)
To: K Prateek Nayak, tglx, mingo, bp, dave.hansen, x86, hpa, seanjc,
peterz, thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, Tycho.Andersen, Nathan.Fontenot,
ackerleytng, jackyli, pgonda, rientjes, jacobhxu, xin,
pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <75cf11f1-51fc-4f1a-a9a7-4b9403d2bb8b@amd.com>
Hello Prateek,
On 6/16/2026 11:20 PM, K Prateek Nayak wrote:
> Hello Ashish,
>
> On 6/17/2026 1:26 AM, Kalra, Ashish wrote:
>> Hello Prateek,
>>
>> On 6/16/2026 2:27 AM, K Prateek Nayak wrote:
>>> Hello Ashish,
>>>
>>> On 6/16/2026 1:19 AM, Ashish Kalra wrote:
>>>> + /*
>>>> + * RMPOPT scans the RMP table, stores the result of the scan in the
>>>> + * reserved processor memory. The RMP scan is the most expensive
>>>> + * part. If a second RMPOPT occurs, it can skip the expensive scan
>>>> + * if they can see a cached result in the reserved processor memory.
>>>> + *
>>>> + * Do RMPOPT on one CPU alone. Then, follow that up with RMPOPT
>>>> + * on every other primary thread. Followers are "designed to"
>>>> + * skip the scan if they see the "cached" scan results.
>>>> + */
>>>> + cpumask_copy(follower_mask, &rmpopt_cpumask);
>>>
>>> rmpopt_cpumask is constructed after hotplug is disabled but ...
>>>
>>>> +
>>>> + /*
>>>> + * Pin the worker to the current CPU for the leader loop so that
>>>> + * this_cpu remains valid and the RMPOPT instruction executes on
>>>> + * the correct CPU.
>>>> + *
>>>> + * Use migrate_disable() rather than get_cpu() to prevent
>>>> + * migration while still allowing preemption.
>>>> + */
>>>> + migrate_disable();
>>>> + this_cpu = smp_processor_id();
>>>> +
>>>> + if (cpumask_test_cpu(this_cpu, follower_mask)) {
>>>> + /*
>>>> + * Current CPU is a primary thread in rmpopt_cpumask.
>>>> + * Run leader locally and remove from follower mask.
>>>> + */
>>>> + cpumask_clear_cpu(this_cpu, follower_mask);
>>>> +
>>>> + for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G) {
>>>> + rmpopt(pa);
>>>> + cond_resched();
>>>> + }
>>>> + } else if (cpumask_intersects(topology_sibling_cpumask(this_cpu),
>>>> + follower_mask)) {
>>>> + /*
>>>> + * Current CPU is a sibling thread whose primary is in
>>>> + * rmpopt_cpumask. RMPOPT_BASE MSR is per-core, so it
>>>> + * is safe to run the leader locally. Remove the sibling's
>>>> + * primary from the follower mask as this core is already
>>>> + * covered by the leader.
>>>> + */
>>>> + cpumask_andnot(follower_mask, follower_mask,
>>>> + topology_sibling_cpumask(this_cpu));
>>>> +
>>>> + for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G) {
>>>> + rmpopt(pa);
>>>> + cond_resched();
>>>> + }
>>>> + } else {
>>>> + /*
>>>> + * Current CPU does not have RMPOPT_BASE MSR programmed.
>>>> + * Pick an explicit leader from the cpumask to avoid #UD.
>>>> + * Use work_on_cpu() to run in process context on the leader,
>>>> + * avoiding IPI latency.
>>>> + */
>>>
>>> ... this_cpu is neither in the "rmpopt_cpumask", nor is any of its
>>> siblings on "rmpopt_cpumask".
>>>
>>> How does that happen?
>>
>> Actually, this was the implementation before the CPU hotplug disable enforcement code was implemented and added in v8,
>> and i should have fixed this rmpopt_work_handler() accordingly for v8.
>>
>> With the enforced cpu hotplug disable support, case #3 here (above) is now dead code, and removing it lets
>> cases #1 and #2 collapse too.
>>
>> snp_prepare() requires cpu_online_mask == cpu_present_mask before SNP init — so when snp_setup_rmpopt() programs the MSRs, every
>> core's primary is online -> every core is in rmpopt_cpumask.
>>
>> So now the work handler always runs on a CPU whose core is programmed. topology_sibling_cpumask(this_cpu) therefore always intersects
>> rmpopt_cpumask -> case #1 or #2 always matches.
>>
>> So i should actually drop case #3 here - which is: "this_cpu is neither in the "rmpopt_cpumask", nor is any of its
>> siblings on rmpopt_cpumask"
>
> Ack.
>
> Also the fact that cpu_mark_primary_thread() uses LSBs of APICID and if
> you have some insanely weird configuration - like boot with maxcpus=1,
> online all the secondary threads (CPUs 256-511 on a 256C/512T system),
> launch an SNP guest - it can actually leave everything except CORE0 out
> of the "rmpopt_cpumask".
>
>>
>>
>>>
>>>> + int leader_cpu = cpumask_first(follower_mask);
>>>> +
>>>> + if (WARN_ON_ONCE(leader_cpu >= nr_cpu_ids)) {
>>>> + migrate_enable();
>>>> + goto out;
>>>> + }
>>>> +
>>>> + cpumask_clear_cpu(leader_cpu, follower_mask);
>>>> +
>>>> + /* Release migration pin before work_on_cpu(). */
>>>> + migrate_enable();
>>>> +
>>>> + work_on_cpu(leader_cpu, rmpopt_leader_fn, NULL);
>>>
>>> This creates a delayed work and also waits for it to finish execution
>>> which will add more latency than a simple IPI if the comment about IPI
>>> latency above is accurate.
>>>
>>> I think there is some corner case in construction of the
>>> "rmpopt_cpumask" that requires this not-so-pretty else block. Can you
>>> elaborate why this is required?
>>>
>>> Perhaps the "rmpopt_cpumask" construction needs:
>>>
>>> for_each_online_cpu(cpu) {
>>> /* Nominate the first CPU on the sibling mask for RMPOPT */
>>> if (cpu != cpumask_first(topology_sibling_cpumask(cpu)))
>>> continue;
>>> cpumask_set_cpu(cpu, &rmpopt_cpumask);
>>> }
>>>
>>>
>>> and all you need here is:
>>>
>>> /* Do RMPOPt for local core */
>>> for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
>>> rmpopt(pa);
>>>
>>> /* Skip this core from concurrent RMPOPT */
>>> cpumask_and_not(follower_mask, &rmpopt_cpumask, topology_sibling_cpumask(cpu));
>>>
>>> No?
>>>
>>
>> Yes, a simpler implementation will be like this:
>> ...
>>
>> if (!alloc_cpumask_var(&follower_mask, GFP_KERNEL))
>> return;
>>
>
> If you move the migrate_disable() here, you can simply do an andnot
> without needing to copy the rmpopt_cpumask beforehand and save on one
> cpumask iteration.
Yes, that's a nice optimization, we can read directly from rmpopt_cpumask and write follower_mask in one pass.
>
>> cpumask_copy(follower_mask, &rmpopt_cpumask);
>>
>> /*
>> * The current CPU's core always has RMPOPT_BASE programmed
>> * (snp_prepare() required all CPUs online at setup and CPU hotplug
>> * is disabled while SNP is active), so it can always be the leader.
>> * RMPOPT_BASE is per-core; exclude this core from the followers.
>> */
>> migrate_disable();
>> cpumask_andnot(follower_mask, follower_mask,
>> topology_sibling_cpumask(smp_processor_id()));
>>
>> for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G) {
>> rmpopt(pa);
>> cond_resched();
>> }
>> migrate_enable();
>>
>> cpus_read_lock();
>
> I think you can even skip the cpus_read_lock() since we know for a
> fact that hotplug is disabled when we are here.
>
> Perhaps we can have a lockdep_assert_cpu_hotplug_disabled() which
> ensures we'll get a splat if that assumption ever changes when
> running with LOCKDEP?
Yes, that is true when we have made sure that hotplug is disabled, but i think it is Ok
to keep cpus_read_lock() here as it keeps Sashiko happy.
>
> I'll let others comment if that is a good idea or not.
>
>> for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G) {
>> on_each_cpu_mask(follower_mask, rmpopt_smp, (void *)pa, true);
>> cond_resched();
>> }
>> cpus_read_unlock();
>>
>> free_cpumask_var(follower_mask);
>>
>>
>> Here, the leader exclusion must use the sibling mask, not clear_cpu(this_cpu). That's why my collapsed version uses:
>>
>> cpumask_andnot(follower_mask, follower_mask,
>> topology_sibling_cpumask(smp_processor_id()));
>>
>> - If this_cpu is a primary: its sibling mask contains itself (the primary) -> andnot removes this core's primary from the followers.
>>
>> - If this_cpu is a secondary: it isn't in follower_mask at all, but its sibling mask contains its primary, which is in
>> follower_mask -> andnot still removes this core's primary.
>>
>> So either way the current core is dropped from the followers. (The old code needed two branches because case #1 used
>> clear_cpu(this_cpu) — only correct when this_cpu is the primary — while case #2 used the sibling andnot. The single andnot works for
>> both cases).
>
> Ack! And I think this looks much cleaner (to my eyes at least ;-)
>
Thanks,
Ashish
^ permalink raw reply
* Re: [PATCH v8 3/7] crypto/ccp: Disable CPU hotplug while SNP is active
From: Kalra, Ashish @ 2026-06-17 22:23 UTC (permalink / raw)
To: K Prateek Nayak, tglx, mingo, bp, dave.hansen, x86, hpa, seanjc,
peterz, thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, Tycho.Andersen, Nathan.Fontenot,
ackerleytng, jackyli, pgonda, rientjes, jacobhxu, xin,
pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <763bff29-e737-4033-ab30-cec8fd3e7438@amd.com>
Hello Prateek,
On 6/16/2026 11:33 PM, K Prateek Nayak wrote:
> Hello Ashish,
>
> On 6/16/2026 1:19 AM, Ashish Kalra wrote:
>> From: Ashish Kalra <ashish.kalra@amd.com>
>>
>> The SEV firmware enumerates the CPUs at SNP initialization and is not
>> aware of the OS bringing CPUs online or offline afterwards, so OS CPU
>> hotplug can diverge from the firmware's expectations and break SNP.
>> Disable CPU hotplug while SNP is active.
>
> Dumb question: Is this specific to RMPOPT? Otherwise ...
The actual reason is purely about the SEV firmware: it enumerates the BIOS-enabled CPUs at SNP_INIT_EX
and has no knowledge of OS hotplug afterward. That's true whether or not RMPOPT exists.
RMPOPT only benefits from the side effect, which is a stable rmpopt_cpumask and an uncontended cpus_read_lock()
in the work handler.
So it is specific to SNP, but RMPOPT patches that come later in the series rely on it, therefore it
is a pre-patch here.
>
>>
>> SNP is fully torn down only on the SNP_SHUTDOWN_EX x86_snp_shutdown
>> path; the legacy path leaves SNP enabled in hardware while clearing
>> snp_initialized, so __sev_snp_init_locked() can run again. Track the
>> disable with a flag so it is balanced by a matching enable rather than
>> stacked, and re-enable hotplug only on the x86_snp_shutdown path, after
>> snp_shutdown() has cleared the per-core RMPOPT_BASE MSRs with hotplug
>> still disabled.
>>
>> This also keeps the CPU set stable for the asynchronous RMPOPT scan
>> added later in this series, and ensures cpus_read_lock() in the scan
>> is uncontended.
>>
>> Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>> ---
>> drivers/crypto/ccp/sev-dev.c | 29 ++++++++++++++++++++++++++++-
>> 1 file changed, 28 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
>> index 217b6b19802e..c8c3c577463c 100644
>> --- a/drivers/crypto/ccp/sev-dev.c
>> +++ b/drivers/crypto/ccp/sev-dev.c
>> @@ -106,6 +106,9 @@ struct snp_hv_fixed_pages_entry {
>>
>> static LIST_HEAD(snp_hv_fixed_pages);
>>
>> +/* Set while SNP has CPU hotplug disabled. */
>> +static bool snp_cpu_hotplug_disabled;
>> +
>> /* Trusted Memory Region (TMR):
>> * The TMR is a 1MB area that must be 1MB aligned. Use the page allocator
>> * to allocate the memory, which will return aligned memory for the specified
>> @@ -1479,6 +1482,17 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
>>
>> snp_hv_fixed_pages_state_update(sev, HV_FIXED);
>>
>> + /*
>> + * Disable CPU hotplug while SNP is active. Guard against stacking
>> + * the disable count: the legacy SNP_SHUTDOWN_EX path clears
>> + * snp_initialized without re-enabling hotplug, so this can run
>> + * again while hotplug is already disabled.
>> + */
>> + if (!snp_cpu_hotplug_disabled) {
>> + cpu_hotplug_disable();
>> + snp_cpu_hotplug_disabled = true;
>> + }
>> +
>
> ... should this be done before __sev_do_cmd_locked(SEV_CMD_SNP_INIT_EX)
> is issued?
>
> I'm assuming that is when the firmware enumerates the CPUs during SNP
> initialization and any hotplug after that should be disallowed?
Yes, it makes sense to do it before SNP_INIT_EX is issued.
Thanks,
Ashish
>
>> snp_setup_rmpopt();
>>
>> sev->snp_initialized = true;
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: rng: timeriomem_rng: add reg-io-width and mask properties
From: Krzysztof Kozlowski @ 2026-06-18 4:11 UTC (permalink / raw)
To: Jad Keskes, Krzysztof Kozlowski
Cc: Olivia Mackall, Herbert Xu, Rob Herring, Conor Dooley,
Alexander Clouter, linux-crypto, devicetree, linux-kernel
In-Reply-To: <20260617114436.1909659-1-inasj268@gmail.com>
On 17/06/2026 13:44, Jad Keskes wrote:
> Add optional reg-io-width (1, 2, or 4 bytes) and mask properties to the
> binding. reg-io-width selects the bus access size, mask is ANDed with
> the raw register value to allow only the entropy-bearing bits through.
>
> Update the example to show a typical 1-byte configuration.
> Update SPDX to dual license to match kernel convention.
And did you Cc all of the copyright holders?
> Drop the misleading '32-bit aligned' constraint from the reg
> description since alignment now depends on the configured width.
>
> Signed-off-by: Jad Keskes <inasj268@gmail.com>
> ---
> .../bindings/rng/timeriomem_rng.yaml | 48 +++++++++++++++----
> 1 file changed, 40 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml b/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml
> index 4754174e9849..740bc52bf474 100644
> --- a/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml
> +++ b/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml
> @@ -1,10 +1,16 @@
> -# SPDX-License-Identifier: GPL-2.0-only
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
Don't mix multiple changes into one commit.
> %YAML 1.2
> ---
> $id: http://devicetree.org/schemas/rng/timeriomem_rng.yaml#
> $schema: http://devicetree.org/meta-schemas/core.yaml#
>
> -title: TimerIO Random Number Generator
> +title: Timer IOMEM Hardware Random Number Generator
> +
> +description: |
> + This binding covers platforms that have a single IO memory address which
Do not describe the binding. Describe the hardware.
> + provides periodic random data. The driver reads from the address at a
Do not describe drivers. Describe the hardware.
> + fixed interval, returning a configurable-width value masked to the desired
> + bits.
>
> maintainers:
> - Krzysztof Kozlowski <krzk@kernel.org>
> @@ -13,9 +19,17 @@ properties:
> compatible:
> const: timeriomem_rng
>
> + reg:
> + maxItems: 1
> + description:
> + Base address to sample from. Must be aligned to the configured access
> + width (1, 2, or 4 bytes) and at least that wide.
> +
> period:
> $ref: /schemas/types.yaml#/definitions/uint32
> - description: wait time in microseconds to use between samples
> + description:
> + Interval in microseconds between reads. New random data is expected to
> + be available at this rate.
>
> quality:
> $ref: /schemas/types.yaml#/definitions/uint32
> @@ -26,16 +40,26 @@ properties:
> instead. Note that the default quality is usually zero which disables
> using this rng to automatically fill the kernel's entropy pool.
>
> - reg:
> - maxItems: 1
> + reg-io-width:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + default: 4
> + enum: [1, 2, 4]
> description:
> - Base address to sample from. Currently 'reg' must be at least four bytes
> - wide and 32-bit aligned.
> + Access width in bytes. Determines whether the read is performed as
> + an 8-bit, 16-bit, or 32-bit bus access.
> +
> + mask:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + default: 0xFFFFFFFF
> + description:
> + Mask applied to the value read from the register. Bits set to 0 in
> + the mask are cleared in the output data. Default (no mask) passes
> + all bits through.
>
> required:
> - compatible
> - - period
> - reg
> + - period
>
> additionalProperties: false
>
> @@ -46,3 +70,11 @@ examples:
> reg = <0x44 0x04>;
> period = <1000000>;
> };
> +
> + rng@64 {
> + compatible = "timeriomem_rng";
> + reg = <0x64 0x01>;
> + period = <50000>;
> + reg-io-width = <1>;
> + mask = <0xFF>;
> + };
Grow existing example. Or why can't it grow?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: rng: timeriomem_rng: add reg-io-width and mask properties
From: Krzysztof Kozlowski @ 2026-06-18 4:12 UTC (permalink / raw)
To: Jad Keskes, Krzysztof Kozlowski
Cc: Olivia Mackall, Herbert Xu, Rob Herring, Conor Dooley,
Alexander Clouter, linux-crypto, devicetree, linux-kernel
In-Reply-To: <20260617114436.1909659-1-inasj268@gmail.com>
On 17/06/2026 13:44, Jad Keskes wrote:
> Add optional reg-io-width (1, 2, or 4 bytes) and mask properties to the
> binding. reg-io-width selects the bus access size, mask is ANDed with
> the raw register value to allow only the entropy-bearing bits through.
>
> Update the example to show a typical 1-byte configuration.
> Update SPDX to dual license to match kernel convention.
> Drop the misleading '32-bit aligned' constraint from the reg
> description since alignment now depends on the configured width.
>
> Signed-off-by: Jad Keskes <inasj268@gmail.com>
> ---
> .../bindings/rng/timeriomem_rng.yaml | 48 +++++++++++++++----
> 1 file changed, 40 insertions(+), 8 deletions(-)
... And where is any changelog?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3] lib/raid/xor: x86: Add AVX-512 optimized xor_gen()
From: Christoph Hellwig @ 2026-06-18 9:25 UTC (permalink / raw)
To: David Laight
Cc: Christoph Hellwig, Eric Biggers, Andrew Morton, linux-kernel,
linux-crypto, x86, linux-raid
In-Reply-To: <20260617110516.0a70950e@pumpkin>
On Wed, Jun 17, 2026 at 11:05:16AM +0100, David Laight wrote:
> > FYI, one or 2 sources are basically useless as they RAID5 configs
> > that have no benefits over simple mirroring and thus the numbers
> > aren't too interesting.
>
> With three disks you xor two buffers (src_count == 1) to get the parity
> to write to the third - so that is a valid RAID5 config.
Sure. It did not say it is invalid, it just isn't very useful.
^ permalink raw reply
* Re: [PATCH v3] lib/raid/xor: x86: Add AVX-512 optimized xor_gen()
From: David Laight @ 2026-06-18 9:59 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Eric Biggers, Andrew Morton, linux-kernel, linux-crypto, x86,
linux-raid
In-Reply-To: <20260618092500.GC17530@lst.de>
On Thu, 18 Jun 2026 11:25:00 +0200
Christoph Hellwig <hch@lst.de> wrote:
> On Wed, Jun 17, 2026 at 11:05:16AM +0100, David Laight wrote:
> > > FYI, one or 2 sources are basically useless as they RAID5 configs
> > > that have no benefits over simple mirroring and thus the numbers
> > > aren't too interesting.
> >
> > With three disks you xor two buffers (src_count == 1) to get the parity
> > to write to the third - so that is a valid RAID5 config.
>
> Sure. It did not say it is invalid, it just isn't very useful.
>
It gives you 2/3 the total capacity, mirroring only gives 1/2.
But for a lot of uses I suspect disks are cheap enough that mirroring
is usually the 'best bet'.
It also means that there are a set of catastrophic failures that can
be more easily recovered.
It is probably more important to ensure all the disks in a raid set
come from different production lines - otherwise they are likely to
fail at the same time.
At least modern disks don't (seem to) suffer from 'sticky oil' which
would tend to stop them restarting after they got cold while turned off.
Raid doesn't help when multiple disks fail to start!
David
^ permalink raw reply
* Re: [PATCH v3] mfd: loongson-se: Add multi-node support
From: Lee Jones @ 2026-06-18 9:59 UTC (permalink / raw)
To: Qunqin Zhao; +Cc: chenhuacai, xry111, linux-kernel, loongarch, linux-crypto
In-Reply-To: <20260610151354.32617-1-zhaoqunqin@loongson.cn>
/* Sashiko Automation: Issues Found (7 Findings) */
Worth chewing through.
> On the Loongson platform, each node is equipped with a security engine
> device. However, due to a hardware flaw, only the device on node 0 can
> trigger interrupts. Therefore, interrupts from other nodes are forwarded
> by node 0. We need to check in the interrupt handler of node 0 whether
> this interrupt is intended for other nodes, this can be accomplished via
> shared interrupt handling.
>
> Signed-off-by: Qunqin Zhao <zhaoqunqin@loongson.cn>
> ---
> V3:
> Using shared interrupts (IRQF_SHARED) instead of manually
> iterating through all devices to check for interrupts.
>
> Link to v2:
> https://lore.kernel.org/all/20260427165133.23350-1-zhaoqunqin@loongson.cn/
>
> drivers/mfd/loongson-se.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mfd/loongson-se.c b/drivers/mfd/loongson-se.c
> index 3902ba377d6..e63ea40d5db 100644
> --- a/drivers/mfd/loongson-se.c
> +++ b/drivers/mfd/loongson-se.c
> @@ -219,7 +219,7 @@ static int loongson_se_probe(struct platform_device *pdev)
>
> for (i = 0; i < nr_irq; i++) {
> irq = platform_get_irq(pdev, i);
> - err = devm_request_irq(dev, irq, se_irq_handler, 0, "loongson-se", se);
> + err = devm_request_irq(dev, irq, se_irq_handler, IRQF_SHARED, "loongson-se", se);
[Severity: High]
With IRQF_SHARED now being used, does se_irq_handler() need to return
IRQ_NONE when it doesn't process any interrupts?
drivers/mfd/loongson-se.c:se_irq_handler() {
...
/* For engines */
while (int_status) {
id = __ffs(int_status);
complete(&se->engines[id].completion);
int_status &= ~BIT(id);
writel(BIT(id), se->base + SE_S2LINT_CL);
}
spin_unlock(&se->dev_lock);
return IRQ_HANDLED;
}
If another device sharing the IRQ line triggers an interrupt and
int_status is 0, the handler will skip processing but unconditionally
return IRQ_HANDLED.
This defeats the spurious interrupt detection mechanism in
note_interrupt(), which could lead to a system lockup if an interrupt
storm occurs on the shared line.
[Severity: Critical]
This is a pre-existing issue, but does se_irq_handler() lack bounds
checking when accessing the engines array?
drivers/mfd/loongson-se.c:se_irq_handler() {
...
/* For engines */
while (int_status) {
id = __ffs(int_status);
complete(&se->engines[id].completion);
...
The driver enables all 32 possible interrupts by writing SE_INT_ALL
(0xffffffff) to SE_S2LINT_EN in loongson_se_probe(). The engines array
has a fixed size of SE_ENGINE_MAX (16).
If the hardware sets any interrupt bit from 16 to 31, id will be >= 16,
resulting in an out-of-bounds memory write when calling complete().
Could this corrupt adjacent heap memory?
[Severity: Critical]
This isn't a bug introduced by this patch, but is it safe to use
devm_kmalloc() with GFP_KERNEL here instead of devm_kzalloc()?
drivers/mfd/loongson-se.c:loongson_se_probe() {
...
se = devm_kmalloc(dev, sizeof(*se), GFP_KERNEL);
...
writel(SE_INT_ALL, se->base + SE_S2LINT_EN);
...
err = devm_request_irq(dev, irq, se_irq_handler, IRQF_SHARED, "loongson-se", se);
...
The se->engines array contains uninitialized garbage because it is
allocated with devm_kmalloc(). Hardware interrupts are globally enabled
and the IRQ handler is registered before the engine completions are
initialized.
If a leftover interrupt from a prior hardware state is pending when the
handler registers, se_irq_handler() could execute
complete(&se->engines[id].completion) on random, uninitialized memory.
> if (err)
> dev_err(dev, "failed to request IRQ: %d\n", irq);
> }
[Severity: High]
This is a pre-existing issue, but should the probe function return an
error if devm_request_irq() fails?
If devm_request_irq() fails, the loop continues without returning an
error code. Immediately after, loongson_se_init() issues a hardware
command and calls wait_for_completion_interruptible(&se->cmd_completion).
Since no interrupt handler is registered to signal complete(), the
worker thread executing the probe will hang indefinitely.
> @@ -228,7 +228,7 @@ static int loongson_se_probe(struct platform_device *pdev)
> if (err)
> return err;
>
> - return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, engines,
> + return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, engines,
> ARRAY_SIZE(engines), NULL, 0, NULL);
> }
[Severity: High]
This isn't a bug introduced by this patch, but is a reinit_completion()
needed before waiting on the completions?
drivers/mfd/loongson-se.c:loongson_se_send_engine_cmd() {
...
return wait_for_completion_interruptible(&engine->completion);
}
If a user process executing this is interrupted by a signal, it returns
-ERESTARTSYS immediately while the hardware command continues executing.
When the hardware eventually finishes, the interrupt handler calls
complete(), incrementing the completion counter.
Because reinit_completion() is never called before initiating the next
command, the subsequent wait_for_completion_interruptible() will
immediately return success before the new hardware command has actually
finished, breaking hardware synchronization.
[Severity: High]
This is a pre-existing issue, but does loongson_se_init() write
uninitialized stack memory to the device registers?
drivers/mfd/loongson-se.c:loongson_se_init() {
struct loongson_se_controller_cmd cmd;
int err;
cmd.command_id = SE_CMD_START;
err = loongson_se_send_controller_cmd(se, &cmd);
...
For SE_CMD_START, only cmd.command_id is initialized, leaving the
cmd.info array uninitialized. loongson_se_send_controller_cmd() then
unconditionally writes SE_SEND_CMD_REG_LEN (8) u32 words to the device
via writel().
This writes 28 bytes of uninitialized kernel stack contents directly to
the device MMIO registers.
[Severity: High]
This isn't a bug introduced by this patch, but does the platform driver
declaration lack a .remove callback to properly halt the hardware?
drivers/mfd/loongson-se.c:
...
static struct platform_driver loongson_se_driver = {
.probe = loongson_se_probe,
.driver = {
.name = "loongson-se",
.acpi_match_table = loongson_se_acpi_match,
},
};
During driver unbind or module removal, devres will automatically free
the DMA buffer and the IRQ. Because there is no explicit teardown step
to send a stop command or disable hardware interrupts, the controller
remains fully active.
If the hardware continues to process and writes to the now-freed DMA
memory, could it cause memory corruption or trigger IOMMU faults?
>
>
> base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
> --
> 2.47.2
>
--
Lee Jones
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: dts: qcom: glymur: add TRNG node
From: Harshal Dev @ 2026-06-18 11:58 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Kuldeep Singh, linux-arm-msm, linux-crypto, devicetree,
linux-kernel, Konrad Dybcio, Herbert Xu, David S. Miller,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Vinod Koul,
Konrad Dybcio, Dmitry Baryshkov
In-Reply-To: <0debc1fb-f6ae-44c6-aa87-d5ef3e39b47d@oss.qualcomm.com>
Hi Bjorn,
On 6/9/2026 12:06 PM, Harshal Dev wrote:
> Hello Bjorn,
>
> On 5/18/2026 2:06 PM, Harshal Dev wrote:
>> Hi Bjorn,
>>
>> On 4/24/2026 2:05 PM, Harshal Dev wrote:
>>> Glymur has a True Random Number Generator, add the node with the correct
>>> compatible set.
>>>
>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>> Signed-off-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
>>> ---
>>> arch/arm64/boot/dts/qcom/glymur.dtsi | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/glymur.dtsi b/arch/arm64/boot/dts/qcom/glymur.dtsi
>>> index f23cf81ddb77..64bbd5691229 100644
>>> --- a/arch/arm64/boot/dts/qcom/glymur.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/glymur.dtsi
>>> @@ -3675,6 +3675,11 @@ pcie3b_phy: phy@f10000 {
>>> status = "disabled";
>>> };
>>>
>>> + rng: rng@10c3000 {
>>> + compatible = "qcom,glymur-trng", "qcom,trng";
>>> + reg = <0x0 0x010c3000 0x0 0x1000>;
>>> + };
>>> +
>>> tcsr_mutex: hwlock@1f40000 {
>>> compatible = "qcom,tcsr-mutex";
>>> reg = <0x0 0x01f40000 0x0 0x20000>;
>>>
>>
>> A gentle reminder to pick this patch for the 7.2 merge window.
>>
>
> Another reminder to pick this patch up in-case you've missed it.
>
Gentle reminder.
Thanks,
Harshal
> Thanks,
> Harshal
>
>> Regards,
>> Harshal
>
^ permalink raw reply
* [PATCH v5 1/2] dt-bindings: rng: timeriomem_rng: add reg-io-width and mask properties
From: Jad Keskes @ 2026-06-18 12:01 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivia Mackall,
Herbert Xu
Cc: Alexander Clouter, devicetree, linux-crypto, linux-kernel,
Jad Keskes
Add optional reg-io-width (1, 2, or 4 bytes) and mask properties.
reg-io-width selects the bus access size. mask is ANDed with the raw
register value to allow only the entropy-bearing bits through.
Update the example to show a 1-byte configuration.
Signed-off-by: Jad Keskes <inasj268@gmail.com>
---
v5:
- Fix description to describe hardware, not the binding
- Drop SPDX dual-license change
- Merge examples into one
v4: Initial version with reg-io-width (replaced custom width from v3)
v3: Changed from custom width to reg-io-width per dt-bindings convention
v2: Split DT binding and driver into separate patches
v1: Initial submission
---
.../bindings/rng/timeriomem_rng.yaml | 50 ++++++++++++++-----
1 file changed, 37 insertions(+), 13 deletions(-)
diff --git a/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml b/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml
index 4754174e9849..7f0068f785b7 100644
--- a/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml
+++ b/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml
@@ -4,7 +4,11 @@
$id: http://devicetree.org/schemas/rng/timeriomem_rng.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
-title: TimerIO Random Number Generator
+title: Timer IOMEM hardware random number generator
+
+description: |
+ A device that provides random data via a single memory-mapped IO register.
+ A new value becomes available at a fixed interval.
maintainers:
- Krzysztof Kozlowski <krzk@kernel.org>
@@ -13,29 +17,47 @@ properties:
compatible:
const: timeriomem_rng
+ reg:
+ maxItems: 1
+ description:
+ Base address to sample from. Must be aligned to the configured access
+ width (1, 2, or 4 bytes) and at least that wide.
+
period:
$ref: /schemas/types.yaml#/definitions/uint32
- description: wait time in microseconds to use between samples
+ description:
+ Interval in microseconds between random value updates.
quality:
$ref: /schemas/types.yaml#/definitions/uint32
default: 0
description:
- Estimated number of bits of true entropy per 1024 bits read from the rng.
- Defaults to zero which causes the kernel's default quality to be used
- instead. Note that the default quality is usually zero which disables
- using this rng to automatically fill the kernel's entropy pool.
+ Estimated number of bits of true entropy per 1024 bits read from the
+ device. Defaults to zero which causes the kernel's default quality to
+ be used instead. Note that the default quality is usually zero which
+ disables using this RNG to automatically fill the kernel's entropy
+ pool.
- reg:
- maxItems: 1
+ reg-io-width:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 4
+ enum: [1, 2, 4]
description:
- Base address to sample from. Currently 'reg' must be at least four bytes
- wide and 32-bit aligned.
+ Access width in bytes. Determines whether the read is performed as
+ an 8-bit, 16-bit, or 32-bit bus access.
+
+ mask:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 0xFFFFFFFF
+ description:
+ Mask applied to the value read from the register. Bits set to 0 in
+ the mask are cleared in the output data. Default (no mask) passes
+ all bits through.
required:
- compatible
- - period
- reg
+ - period
additionalProperties: false
@@ -43,6 +65,8 @@ examples:
- |
rng@44 {
compatible = "timeriomem_rng";
- reg = <0x44 0x04>;
- period = <1000000>;
+ reg = <0x44 0x01>;
+ period = <50000>;
+ reg-io-width = <1>;
+ mask = <0xFF>;
};
--
2.54.0
^ permalink raw reply related
* [PATCH v5 2/2] hw_random: timeriomem-rng: add configurable read width and data mask
From: Jad Keskes @ 2026-06-18 12:01 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivia Mackall,
Herbert Xu
Cc: Alexander Clouter, devicetree, linux-crypto, linux-kernel,
Jad Keskes
In-Reply-To: <20260618120110.36439-1-inasj268@gmail.com>
The TODO for supporting read sizes other than 32 bits and masking has
been sitting in this driver since 2009. Implement it.
Add reg-io-width (1, 2, or 4 bytes) and mask support. The read loop
dispatches on width using readb/readw/readl so a configured 1-byte
access does not trigger a bus error on hardware that rejects 32-bit
reads to that address. The mask is ANDed with the value before storing.
These are platform properties, not runtime policy -- width depends on
SoC integration, mask reflects which output bits carry entropy.
The alignment check in probe is updated to verify the resource is
aligned to the configured width instead of hardcoding 4-byte alignment.
Signed-off-by: Jad Keskes <inasj268@gmail.com>
---
v5: No changes since v4
v4: Initial version with reg-io-width (bytes) and readb/readw/readl
v3: Added configurable width (bits) with mask
v2: Rebased
v1: Initial submission
---
drivers/char/hw_random/timeriomem-rng.c | 77 ++++++++++++++++++++-----
include/linux/timeriomem-rng.h | 12 ++++
2 files changed, 76 insertions(+), 13 deletions(-)
diff --git a/drivers/char/hw_random/timeriomem-rng.c b/drivers/char/hw_random/timeriomem-rng.c
index e61f06393209..42393409f22a 100644
--- a/drivers/char/hw_random/timeriomem-rng.c
+++ b/drivers/char/hw_random/timeriomem-rng.c
@@ -14,7 +14,9 @@
* has to do is provide the address and 'wait time' that new data becomes
* available.
*
- * TODO: add support for reading sizes other than 32bits and masking
+ * The read width (8, 16, or 32 bits) and an optional data mask can be
+ * configured through platform data or device tree properties. Default is
+ * 32-bit reads with no mask.
*/
#include <linux/completion.h>
@@ -34,6 +36,8 @@ struct timeriomem_rng_private {
void __iomem *io_base;
ktime_t period;
unsigned int present:1;
+ unsigned int reg_io_width;
+ u32 mask;
struct hrtimer timer;
struct completion completion;
@@ -48,6 +52,7 @@ static int timeriomem_rng_read(struct hwrng *hwrng, void *data,
container_of(hwrng, struct timeriomem_rng_private, rng_ops);
int retval = 0;
int period_us = ktime_to_us(priv->period);
+ int chunk = priv->reg_io_width;
/*
* There may not have been enough time for new data to be generated
@@ -71,11 +76,28 @@ static int timeriomem_rng_read(struct hwrng *hwrng, void *data,
usleep_range(period_us,
period_us + max(1, period_us / 100));
- *(u32 *)data = readl(priv->io_base);
- retval += sizeof(u32);
- data += sizeof(u32);
- max -= sizeof(u32);
- } while (wait && max > sizeof(u32));
+ switch (priv->reg_io_width) {
+ case 1: {
+ u8 val = readb(priv->io_base) & priv->mask;
+ *(u8 *)data = val;
+ break;
+ }
+ case 2: {
+ u16 val = readw(priv->io_base) & priv->mask;
+ *(u16 *)data = val;
+ break;
+ }
+ case 4: {
+ u32 val = readl(priv->io_base) & priv->mask;
+ *(u32 *)data = val;
+ break;
+ }
+ }
+
+ retval += chunk;
+ data += chunk;
+ max -= chunk;
+ } while (wait && max > chunk);
/*
* Block any new callers until the RNG has had time to generate new
@@ -125,11 +147,8 @@ static int timeriomem_rng_probe(struct platform_device *pdev)
if (IS_ERR(priv->io_base))
return PTR_ERR(priv->io_base);
- if (res->start % 4 != 0 || resource_size(res) < 4) {
- dev_err(&pdev->dev,
- "address must be at least four bytes wide and 32-bit aligned\n");
- return -EINVAL;
- }
+ priv->reg_io_width = 4;
+ priv->mask = 0xFFFFFFFF;
if (pdev->dev.of_node) {
int i;
@@ -145,9 +164,41 @@ static int timeriomem_rng_probe(struct platform_device *pdev)
if (!of_property_read_u32(pdev->dev.of_node,
"quality", &i))
priv->rng_ops.quality = i;
+
+ of_property_read_u32(pdev->dev.of_node,
+ "reg-io-width", &priv->reg_io_width);
+ of_property_read_u32(pdev->dev.of_node,
+ "mask", &priv->mask);
} else {
period = pdata->period;
priv->rng_ops.quality = pdata->quality;
+
+ if (pdata->reg_io_width_set)
+ priv->reg_io_width = pdata->reg_io_width;
+ if (pdata->mask_set)
+ priv->mask = pdata->mask;
+ }
+
+ if (priv->reg_io_width == 0)
+ priv->reg_io_width = 4;
+
+ switch (priv->reg_io_width) {
+ case 1:
+ case 2:
+ case 4:
+ break;
+ default:
+ dev_err(&pdev->dev, "invalid reg-io-width %u, must be 1, 2, or 4\n",
+ priv->reg_io_width);
+ return -EINVAL;
+ }
+
+ if (!IS_ALIGNED(res->start, priv->reg_io_width) ||
+ resource_size(res) < priv->reg_io_width) {
+ dev_err(&pdev->dev,
+ "address must be %u-byte aligned\n",
+ priv->reg_io_width);
+ return -EINVAL;
}
priv->period = us_to_ktime(period);
@@ -167,8 +218,8 @@ static int timeriomem_rng_probe(struct platform_device *pdev)
return err;
}
- dev_info(&pdev->dev, "32bits from 0x%p @ %dus\n",
- priv->io_base, period);
+ dev_info(&pdev->dev, "%u-byte from %p @ %dus\n",
+ priv->reg_io_width, priv->io_base, period);
return 0;
}
diff --git a/include/linux/timeriomem-rng.h b/include/linux/timeriomem-rng.h
index 672df7fbf6c1..5732489a17a1 100644
--- a/include/linux/timeriomem-rng.h
+++ b/include/linux/timeriomem-rng.h
@@ -16,6 +16,18 @@ struct timeriomem_rng_data {
/* bits of entropy per 1024 bits read */
unsigned int quality;
+
+ /* read width (1, 2, or 4 bytes), 0 means 4 */
+ unsigned int reg_io_width;
+
+ /* set to true if reg-io-width is explicitly provided */
+ bool reg_io_width_set;
+
+ /* mask applied to raw read value */
+ u32 mask;
+
+ /* set to true if mask is explicitly provided */
+ bool mask_set;
};
#endif /* _LINUX_TIMERIOMEM_RNG_H */
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v2 2/2] arm64: dts: qcom: glymur: add TRNG node
From: Konrad Dybcio @ 2026-06-18 12:20 UTC (permalink / raw)
To: Harshal Dev, Bjorn Andersson
Cc: Kuldeep Singh, linux-arm-msm, linux-crypto, devicetree,
linux-kernel, Herbert Xu, David S. Miller, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Konrad Dybcio,
Dmitry Baryshkov
In-Reply-To: <b4794e93-0fd3-4559-9ecd-02d679bd06b5@oss.qualcomm.com>
On 6/18/26 1:58 PM, Harshal Dev wrote:
> Hi Bjorn,
>
> On 6/9/2026 12:06 PM, Harshal Dev wrote:
>> Hello Bjorn,
>>
>> On 5/18/2026 2:06 PM, Harshal Dev wrote:
>>> Hi Bjorn,
>>>
>>> On 4/24/2026 2:05 PM, Harshal Dev wrote:
>>>> Glymur has a True Random Number Generator, add the node with the correct
>>>> compatible set.
>>>>
>>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>> Signed-off-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
>>>> ---
>>>> arch/arm64/boot/dts/qcom/glymur.dtsi | 5 +++++
>>>> 1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/glymur.dtsi b/arch/arm64/boot/dts/qcom/glymur.dtsi
>>>> index f23cf81ddb77..64bbd5691229 100644
>>>> --- a/arch/arm64/boot/dts/qcom/glymur.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/glymur.dtsi
>>>> @@ -3675,6 +3675,11 @@ pcie3b_phy: phy@f10000 {
>>>> status = "disabled";
>>>> };
>>>>
>>>> + rng: rng@10c3000 {
>>>> + compatible = "qcom,glymur-trng", "qcom,trng";
>>>> + reg = <0x0 0x010c3000 0x0 0x1000>;
>>>> + };
>>>> +
>>>> tcsr_mutex: hwlock@1f40000 {
>>>> compatible = "qcom,tcsr-mutex";
>>>> reg = <0x0 0x01f40000 0x0 0x20000>;
>>>>
>>>
>>> A gentle reminder to pick this patch for the 7.2 merge window.
>>>
>>
>> Another reminder to pick this patch up in-case you've missed it.
>>
>
> Gentle reminder.
Bjorn and I were both out at the time, after we returned it was too
late to accept new patches.
Currently we're halfway through the merge window (where Torvalds
receives pull requests to create 7.2-rc1 out of), during which
contributions are not accepted. They will resume in ~1.5wk after
7.2-rc1 is tagged, targetting 7.3
Konrad
^ permalink raw reply
* loop-AES util-linux CVE-2026-27456 update
From: Jari Ruusu @ 2026-06-18 17:25 UTC (permalink / raw)
To: linux-crypto@vger.kernel.org
Earlier loop-AES patched versions of util-linux have incomplete fix for
CVE-2026-27456. Newer versions of loop-AES patched versions that have more
complete fix are here:
https://loop-aes.sourceforge.net/updates/util-linux-2.41.5.diff.bz2
https://loop-aes.sourceforge.net/updates/util-linux-2.41.5.diff.bz2.sign
and
https://loop-aes.sourceforge.net/updates/util-linux-2.42.2.diff.bz2
https://loop-aes.sourceforge.net/updates/util-linux-2.42.2.diff.bz2.sign
--
Jari Ruusu 4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD ACDF F073 3C80 8132 F189
^ permalink raw reply
* Re: [PATCH v8 7/7] x86/sev: Add debugfs support for RMPOPT
From: Borislav Petkov @ 2026-06-18 18:08 UTC (permalink / raw)
To: Ashish Kalra
Cc: tglx, mingo, dave.hansen, x86, hpa, seanjc, peterz,
thomas.lendacky, herbert, davem, ardb, pbonzini, aik,
Michael.Roth, KPrateek.Nayak, Tycho.Andersen, Nathan.Fontenot,
ackerleytng, jackyli, pgonda, rientjes, jacobhxu, xin,
pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <cc9aa9b6cfa2ce826f2ad53f8a13d3b7bf0790b6.1781419998.git.ashish.kalra@amd.com>
On Mon, Jun 15, 2026 at 07:50:56PM +0000, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
>
> Add a debugfs interface to report per-CPU RMPOPT status across all
> system RAM.
>
> To dump the per-CPU RMPOPT status for all system RAM:
>
> /sys/kernel/debug/rmpopt# cat rmpopt-table
>
> Memory @ 0GB: CPU(s): none
> Memory @ 1GB: CPU(s): none
> Memory @ 2GB: CPU(s): 0-1023
> Memory @ 3GB: CPU(s): 0-1023
> Memory @ 4GB: CPU(s): none
> Memory @ 5GB: CPU(s): 0-1023
> Memory @ 6GB: CPU(s): 0-1023
> Memory @ 7GB: CPU(s): 0-1023
> ...
> Memory @1025GB: CPU(s): 0-1023
> Memory @1026GB: CPU(s): 0-1023
> Memory @1027GB: CPU(s): 0-1023
> Memory @1028GB: CPU(s): 0-1023
> Memory @1029GB: CPU(s): 0-1023
> Memory @1030GB: CPU(s): 0-1023
> Memory @1031GB: CPU(s): 0-1023
> Memory @1032GB: CPU(s): 0-1023
> Memory @1033GB: CPU(s): 0-1023
> Memory @1034GB: CPU(s): 0-1023
> Memory @1035GB: CPU(s): 0-1023
> Memory @1036GB: CPU(s): 0-1023
> Memory @1037GB: CPU(s): 0-1023
> Memory @1038GB: CPU(s): none
>
> Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
> arch/x86/virt/svm/sev.c | 128 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 128 insertions(+)
https://lwn.net/Articles/309298/
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH v8 2/7] x86/sev: Initialize RMPOPT configuration MSRs
From: Kalra, Ashish @ 2026-06-18 18:23 UTC (permalink / raw)
To: K Prateek Nayak, tglx, mingo, bp, dave.hansen, x86, hpa, seanjc,
peterz, thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, Tycho.Andersen, Nathan.Fontenot,
ackerleytng, jackyli, pgonda, rientjes, jacobhxu, xin,
pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <fb2f1105-3bef-4197-bccd-865c013ce712@amd.com>
Hello Prateek,
On 6/16/2026 1:03 AM, K Prateek Nayak wrote:
> Hello Ashish,
>
> On 6/16/2026 1:18 AM, Ashish Kalra wrote:
>> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
>> index 8bcdce98f6dc..1b5c18408f0b 100644
>> --- a/arch/x86/virt/svm/sev.c
>> +++ b/arch/x86/virt/svm/sev.c
>> @@ -124,6 +124,10 @@ static void *rmp_bookkeeping __ro_after_init;
>>
>> static u64 probed_rmp_base, probed_rmp_size;
>>
>> +static cpumask_t rmpopt_cpumask;
>
> nit.
>
> I believe you can use cpumask_var_t here and do a zalloc_cpumask_var()
> during snp_setup_rmpopt(). That way !X86_FEATURE_RMPOPT configs don't
> have to needlessly waste space to keep a redundant cpumask around.
>
> Same comment for rmpopt_report_cpumask in Patch 7 which can be
> allocated dynamically during rmpopt_debugfs_setup().
>
Yes.
>> +static phys_addr_t rmpopt_pa_start;
>> +static bool rmpopt_configured;
>> +
>> static LIST_HEAD(snp_leaked_pages_list);
>> static DEFINE_SPINLOCK(snp_leaked_pages_list_lock);
>>
>> @@ -490,7 +494,12 @@ static bool __init setup_rmptable(void)
>> if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED) {
>> if (!setup_segmented_rmptable())
>> return false;
>> + rmpopt_configured = true;
>> } else {
>> + /*
>> + * RMPOPT requires a segmented RMP table, so leave
>> + * rmpopt_configured clear on contiguous RMP systems.
>> + */
>> if (!setup_contiguous_rmptable())
>> return false;
>> }
>> @@ -555,6 +564,21 @@ int snp_prepare(void)
>> }
>> EXPORT_SYMBOL_FOR_MODULES(snp_prepare, "ccp");
>>
>> +static void rmpopt_cleanup(void)
>> +{
>> + int cpu;
>> +
>> + cpus_read_lock();
>
> nit.
>
> You can use guard(cpus_read_lock)() unless there is a complicated
> locking pattern where you need to drop and re-acquire the read lock.
But if i use guard(cpus_read_lock)(), cpus_read_lock stays held across as it is
function-scope, so it will be still held for code following the wrmsrq_on_cpu(),
which is harmless but still changes code behavior.
Probably, the other option is to use scoped_guard form ?
Thanks,
Ashish
>
>> +
>> + for_each_cpu(cpu, &rmpopt_cpumask)
>> + WARN_ON_ONCE(wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, 0));
>> +
>> + cpus_read_unlock();
>> +
>> + cpumask_clear(&rmpopt_cpumask);
>> + rmpopt_pa_start = 0;
>> +}
>> +
>> void snp_shutdown(void)
>> {
>> u64 syscfg;
>
^ permalink raw reply
* Re: [PATCH v8 7/7] x86/sev: Add debugfs support for RMPOPT
From: Kalra, Ashish @ 2026-06-18 19:57 UTC (permalink / raw)
To: Borislav Petkov
Cc: tglx, mingo, dave.hansen, x86, hpa, seanjc, peterz,
thomas.lendacky, herbert, davem, ardb, pbonzini, aik,
Michael.Roth, KPrateek.Nayak, Tycho.Andersen, Nathan.Fontenot,
ackerleytng, jackyli, pgonda, rientjes, jacobhxu, xin,
pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <20260618180814.GCajQ0Dv0CoRMJxbP0@fat_crate.local>
On 6/18/2026 1:08 PM, Borislav Petkov wrote:
> On Mon, Jun 15, 2026 at 07:50:56PM +0000, Ashish Kalra wrote:
>> From: Ashish Kalra <ashish.kalra@amd.com>
>>
>> Add a debugfs interface to report per-CPU RMPOPT status across all
>> system RAM.
>>
>> To dump the per-CPU RMPOPT status for all system RAM:
>>
>> /sys/kernel/debug/rmpopt# cat rmpopt-table
>>
>> Memory @ 0GB: CPU(s): none
>> Memory @ 1GB: CPU(s): none
>> Memory @ 2GB: CPU(s): 0-1023
>> Memory @ 3GB: CPU(s): 0-1023
>> Memory @ 4GB: CPU(s): none
>> Memory @ 5GB: CPU(s): 0-1023
>> Memory @ 6GB: CPU(s): 0-1023
>> Memory @ 7GB: CPU(s): 0-1023
>> ...
>> Memory @1025GB: CPU(s): 0-1023
>> Memory @1026GB: CPU(s): 0-1023
>> Memory @1027GB: CPU(s): 0-1023
>> Memory @1028GB: CPU(s): 0-1023
>> Memory @1029GB: CPU(s): 0-1023
>> Memory @1030GB: CPU(s): 0-1023
>> Memory @1031GB: CPU(s): 0-1023
>> Memory @1032GB: CPU(s): 0-1023
>> Memory @1033GB: CPU(s): 0-1023
>> Memory @1034GB: CPU(s): 0-1023
>> Memory @1035GB: CPU(s): 0-1023
>> Memory @1036GB: CPU(s): 0-1023
>> Memory @1037GB: CPU(s): 0-1023
>> Memory @1038GB: CPU(s): none
>>
>> Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>> ---
>> arch/x86/virt/svm/sev.c | 128 ++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 128 insertions(+)
>
> https://lwn.net/Articles/309298/
>
Since the RMPOPT file is a diagnostic (verify the optimization took effect), debugfs is
arguably the right home for it and we are not claiming it to be an API (there is no
Documentation/ABI entry for it) and we are not presenting it as something tools should
depend on, it is a self-contained diagnostic/debug interface.
Maybe i can add a line to this patch's commit message stating it's a debug-only interface
with no stability guarantee.
We have to provide some method/interface for users to verify if RMP optimizations
are enabled for a GB range of memory.
Thanks,
Ashish
^ permalink raw reply
* Re: [PATCH v8 7/7] x86/sev: Add debugfs support for RMPOPT
From: Borislav Petkov @ 2026-06-18 20:10 UTC (permalink / raw)
To: Kalra, Ashish
Cc: tglx, mingo, dave.hansen, x86, hpa, seanjc, peterz,
thomas.lendacky, herbert, davem, ardb, pbonzini, aik,
Michael.Roth, KPrateek.Nayak, Tycho.Andersen, Nathan.Fontenot,
ackerleytng, jackyli, pgonda, rientjes, jacobhxu, xin,
pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <5849645c-f701-4768-8cdf-1f9032e3226f@amd.com>
On Thu, Jun 18, 2026 at 02:57:45PM -0500, Kalra, Ashish wrote:
> Maybe i can add a line to this patch's commit message stating it's a debug-only interface
> with no stability guarantee.
Sounds to me like you didn't really read that article.
> We have to provide some method/interface for users to verify if RMP optimizations
> are enabled for a GB range of memory.
Sounds to me like this wants to be a facility which is present in the kernel
and it is going to be an ABI.
I am unclear on the real use case but I'm open to being persuaded otherwise.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH v8 2/7] x86/sev: Initialize RMPOPT configuration MSRs
From: Tom Lendacky @ 2026-06-18 21:08 UTC (permalink / raw)
To: Ashish Kalra, tglx, mingo, bp, dave.hansen, x86, hpa, seanjc,
peterz, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, ackerleytng, jackyli, pgonda, rientjes, jacobhxu,
xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen,
darwi, linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <6a4d0ec9e037d91c0fdba9c525942ca288e1b1b2.1781419998.git.ashish.kalra@amd.com>
On 6/15/26 14:48, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
>
> The new RMPOPT instruction helps manage per-CPU RMP optimization
> structures inside the CPU. It takes a 1GB-aligned physical address
> and either returns the status of the optimizations or tries to enable
> the optimizations.
>
> Per-CPU RMPOPT tables support at most 2 TB of addressable memory for
> RMP optimizations.
>
> Initialize the per-CPU RMPOPT table base to the starting physical
> address. This enables RMP optimization for up to 2 TB of system RAM on
> all CPUs.
>
> Additionally, add support to setup and enable RMPOPT once SNP is
> enabled and initialized.
>
> Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
> Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
> Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
> arch/x86/coco/core.c | 2 +
> arch/x86/include/asm/msr-index.h | 3 ++
> arch/x86/include/asm/sev.h | 4 ++
> arch/x86/virt/svm/sev.c | 70 ++++++++++++++++++++++++++++++++
> drivers/crypto/ccp/sev-dev.c | 3 ++
> 5 files changed, 82 insertions(+)
>
> diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c
> index 989ca9f72ba3..8c1393ddc5df 100644
> --- a/arch/x86/coco/core.c
> +++ b/arch/x86/coco/core.c
> @@ -16,6 +16,7 @@
> #include <asm/archrandom.h>
> #include <asm/coco.h>
> #include <asm/processor.h>
> +#include <asm/sev.h>
>
> enum cc_vendor cc_vendor __ro_after_init = CC_VENDOR_NONE;
> SYM_PIC_ALIAS(cc_vendor);
> @@ -172,6 +173,7 @@ static void amd_cc_platform_clear(enum cc_attr attr)
> switch (attr) {
> case CC_ATTR_HOST_SEV_SNP:
> cc_flags.host_sev_snp = 0;
> + snp_clear_rmpopt_configured();
> break;
> default:
> break;
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index 86554de9a3f5..28540744f1eb 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -761,6 +761,9 @@
> #define MSR_AMD64_SEG_RMP_ENABLED_BIT 0
> #define MSR_AMD64_SEG_RMP_ENABLED BIT_ULL(MSR_AMD64_SEG_RMP_ENABLED_BIT)
> #define MSR_AMD64_RMP_SEGMENT_SHIFT(x) (((x) & GENMASK_ULL(13, 8)) >> 8)
> +#define MSR_AMD64_RMPOPT_BASE 0xc0010139
> +#define MSR_AMD64_RMPOPT_ENABLE_BIT 0
> +#define MSR_AMD64_RMPOPT_ENABLE BIT_ULL(MSR_AMD64_RMPOPT_ENABLE_BIT)
>
> #define MSR_SVSM_CAA 0xc001f000
>
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index 594cfa19cbd4..0d662221615a 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -662,6 +662,8 @@ static inline void snp_leak_pages(u64 pfn, unsigned int pages)
> __snp_leak_pages(pfn, pages, true);
> }
> int snp_prepare(void);
> +void snp_setup_rmpopt(void);
> +void snp_clear_rmpopt_configured(void);
> void snp_shutdown(void);
> #else
> static inline bool snp_probe_rmptable_info(void) { return false; }
> @@ -680,6 +682,8 @@ static inline void snp_leak_pages(u64 pfn, unsigned int npages) {}
> static inline void kdump_sev_callback(void) { }
> static inline void snp_fixup_e820_tables(void) {}
> static inline int snp_prepare(void) { return -ENODEV; }
> +static inline void snp_setup_rmpopt(void) {}
> +static inline void snp_clear_rmpopt_configured(void) {}
> static inline void snp_shutdown(void) {}
> #endif
>
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index 8bcdce98f6dc..1b5c18408f0b 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
> @@ -124,6 +124,10 @@ static void *rmp_bookkeeping __ro_after_init;
>
> static u64 probed_rmp_base, probed_rmp_size;
>
> +static cpumask_t rmpopt_cpumask;
> +static phys_addr_t rmpopt_pa_start;
> +static bool rmpopt_configured;
The usage of this isn't doesn't imply what the name says. How about
changing it to rmpopt_capable ?
> +
> static LIST_HEAD(snp_leaked_pages_list);
> static DEFINE_SPINLOCK(snp_leaked_pages_list_lock);
>
> @@ -490,7 +494,12 @@ static bool __init setup_rmptable(void)
> if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED) {
> if (!setup_segmented_rmptable())
> return false;
> + rmpopt_configured = true;
> } else {
> + /*
> + * RMPOPT requires a segmented RMP table, so leave
> + * rmpopt_configured clear on contiguous RMP systems.
> + */
I think this comment should be above where rmpopt_configured is set,
slightly changed to
RMPOPT requires a segmented RMP, so indicate that the system
is capable of configuring and running RMPOPT.
Thanks,
Tom
> if (!setup_contiguous_rmptable())
> return false;
> }
> @@ -555,6 +564,21 @@ int snp_prepare(void)
> }
> EXPORT_SYMBOL_FOR_MODULES(snp_prepare, "ccp");
>
> +static void rmpopt_cleanup(void)
> +{
> + int cpu;
> +
> + cpus_read_lock();
> +
> + for_each_cpu(cpu, &rmpopt_cpumask)
> + WARN_ON_ONCE(wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, 0));
> +
> + cpus_read_unlock();
> +
> + cpumask_clear(&rmpopt_cpumask);
> + rmpopt_pa_start = 0;
> +}
> +
> void snp_shutdown(void)
> {
> u64 syscfg;
> @@ -563,11 +587,57 @@ void snp_shutdown(void)
> if (syscfg & MSR_AMD64_SYSCFG_SNP_EN)
> return;
>
> + rmpopt_cleanup();
> +
> clear_rmp();
> on_each_cpu(mfd_reconfigure, NULL, 1);
> }
> EXPORT_SYMBOL_FOR_MODULES(snp_shutdown, "ccp");
>
> +void snp_clear_rmpopt_configured(void)
> +{
> + rmpopt_configured = false;
> +}
> +
> +void snp_setup_rmpopt(void)
> +{
> + u64 rmpopt_base;
> + int cpu;
> +
> + if (!cpu_feature_enabled(X86_FEATURE_RMPOPT) || !rmpopt_configured)
> + return;
> +
> + cpus_read_lock();
> +
> + /*
> + * The RMPOPT_BASE MSR is per-core, so only one thread per core needs
> + * to set up the RMPOPT_BASE MSR.
> + *
> + * Note: only online primary threads are included. If a core's
> + * primary thread is offline, that core is not covered. CPU hotplug
> + * is not currently supported with SNP enabled.
> + */
> +
> + for_each_online_cpu(cpu)
> + if (topology_is_primary_thread(cpu))
> + cpumask_set_cpu(cpu, &rmpopt_cpumask);
> +
> + rmpopt_pa_start = ALIGN_DOWN(PFN_PHYS(min_low_pfn), SZ_1G);
> + rmpopt_base = rmpopt_pa_start | MSR_AMD64_RMPOPT_ENABLE;
> +
> + /*
> + * Per-CPU RMPOPT tables support at most 2 TB of addressable memory
> + * for RMP optimizations. Initialize the per-CPU RMPOPT table base
> + * to the starting physical address to enable RMP optimizations for
> + * up to 2 TB of system RAM on all CPUs.
> + */
> + for_each_cpu(cpu, &rmpopt_cpumask)
> + WARN_ON_ONCE(wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, rmpopt_base));
> +
> + cpus_read_unlock();
> +}
> +EXPORT_SYMBOL_FOR_MODULES(snp_setup_rmpopt, "ccp");
> +
> /*
> * Do the necessary preparations which are verified by the firmware as
> * described in the SNP_INIT_EX firmware command description in the SNP
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 78f98aee7a66..217b6b19802e 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -1478,6 +1478,9 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
> }
>
> snp_hv_fixed_pages_state_update(sev, HV_FIXED);
> +
> + snp_setup_rmpopt();
> +
> sev->snp_initialized = true;
> dev_dbg(sev->dev, "SEV-SNP firmware initialized, SEV-TIO is %s\n",
> data.tio_en ? "enabled" : "disabled");
^ permalink raw reply
* Re: [PATCH v8 3/7] crypto/ccp: Disable CPU hotplug while SNP is active
From: Dave Hansen @ 2026-06-18 21:35 UTC (permalink / raw)
To: Ashish Kalra, tglx, mingo, bp, dave.hansen, x86, hpa, seanjc,
peterz, thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, ackerleytng, jackyli, pgonda, rientjes, jacobhxu,
xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen,
darwi, linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <1feccf6e2a56d949b30f403c0ca7949f580e5982.1781419998.git.ashish.kalra@amd.com>
On 6/15/26 12:49, Ashish Kalra wrote:
> + /*
> + * Disable CPU hotplug while SNP is active. Guard against stacking
> + * the disable count: the legacy SNP_SHUTDOWN_EX path clears
> + * snp_initialized without re-enabling hotplug, so this can run
> + * again while hotplug is already disabled.
> + */
> + if (!snp_cpu_hotplug_disabled) {
> + cpu_hotplug_disable();
> + snp_cpu_hotplug_disabled = true;
> + }
This seems like a hack, guys.
cpu_hotplug_disable() seems like more of a temporary lock than enforcing
basically permanent system state.
This seems like it would be better implemented by registering a CPU
hotplug callback and then refusing to offline if sev->snp_initialized is
set.
snp_setup_rmpopt() can be run any time, right? It doesn't need to be
after sev->snp_initialized=1.
^ permalink raw reply
* Re: [PATCH v8 5/7] x86/sev: Add interface to re-enable RMP optimizations.
From: Dave Hansen @ 2026-06-18 21:41 UTC (permalink / raw)
To: Ashish Kalra, tglx, mingo, bp, dave.hansen, x86, hpa, seanjc,
peterz, thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, ackerleytng, jackyli, pgonda, rientjes, jacobhxu,
xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen,
darwi, linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <cdb8098074de8e150dcf534ab806e38744325a57.1781419998.git.ashish.kalra@amd.com>
On 6/15/26 12:49, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
>
> RMPOPT table is a per-CPU table which indicates if 1GB regions of
> physical memory are entirely hypervisor-owned or not.
>
> When performing host memory accesses in hypervisor mode as well as
> non-SNP guest mode, the processor may consult the RMPOPT table to
> potentially skip an RMP access and improve performance.
>
> Events such as RMPUPDATE can clear RMP optimizations. Add an interface
> to re-enable those optimizations.
This doesn't really help me understand when or how this function might
be called.
Normal guest evens like splitting and collapsing large pages can
clear RMP optimizations. Without some intervention, all RMP
optimizations would eventually be lost. Periodically re-optimize
the system.
> The interface uses mod_delayed_work() instead of queue_delayed_work()
> so that the delay timer is reset on each call. This provides proper
> batching semantics: re-optimization runs 10 seconds after the *last*
> VM termination rather than after the first. mod_delayed_work() also
> re-queues work that is already in-flight, so a re-scan request
> during an active scan is not silently dropped.
This seems sane.
> +void snp_rmpopt_all_physmem(void)
> +{
> + if (!cpu_feature_enabled(X86_FEATURE_RMPOPT) || !rmpopt_configured)
> + return;
> +
> + guard(mutex)(&rmpopt_wq_mutex);
> +
> + if (!rmpopt_wq)
> + return;
> +
> + mod_delayed_work(rmpopt_wq, &rmpopt_delayed_work,
> + msecs_to_jiffies(RMPOPT_WORK_TIMEOUT));
> +}
> +EXPORT_SYMBOL_GPL(snp_rmpopt_all_physmem);
Does this need to be globally exported? Or can it be exported to a
single module namespace?
I'm close to being able to ack this, but it's still got a few too many
nits to ack.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox