* [PATCH v4 14/27] mtd: spi-nor: swp: Simplify checking the locked/unlocked range
From: Miquel Raynal @ 2026-04-03 16:09 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260403-winbond-v6-18-rc1-spi-nor-swp-v4-0-833dab5e7288@bootlin.com>
In both the locking/unlocking steps, at the end we verify whether we do
not lock/unlock more than requested (in which case an error must be
returned).
While being possible to do that with very simple mask comparisons, it
does not scale when adding extra locking features such as the CMP
possibility. In order to make these checks slightly easier to read and
more future proof, use existing helpers to read the (future) status
register, extract the covered range, and compare it with very usual
algebric comparisons.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/spi-nor/swp.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index e2e423b20989..c45a9ddd5788 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -198,7 +198,8 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
u64 min_prot_len;
int ret;
u8 status_old[1] = {}, status_new[1] = {};
- u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
+ loff_t ofs_old, ofs_new;
+ u64 len_old, len_new;
loff_t lock_len;
bool can_be_top = true, can_be_bottom = nor->flags & SNOR_F_HAS_SR_TB;
bool use_top;
@@ -246,10 +247,6 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
if (ret)
return ret;
- /* Don't "lock" with no region! */
- if (!(status_new[0] & bp_mask))
- return -EINVAL;
-
/*
* Disallow further writes if WP# pin is neither left floating nor
* wrongly tied to GND (that includes internal pull-downs).
@@ -262,8 +259,16 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
if (status_new[0] == status_old[0])
return 0;
+ spi_nor_get_locked_range_sr(nor, status_old, &ofs_old, &len_old);
+ spi_nor_get_locked_range_sr(nor, status_new, &ofs_new, &len_new);
+
+ /* Don't "lock" with no region! */
+ if (!len_new)
+ return -EINVAL;
+
/* Only modify protection if it will not unlock other areas */
- if ((status_new[0] & bp_mask) < (status_old[0] & bp_mask))
+ if (len_old &&
+ (ofs_old < ofs_new || (ofs_new + len_new) < (ofs_old + len_old)))
return -EINVAL;
return spi_nor_write_sr_and_check(nor, status_new[0]);
@@ -279,7 +284,8 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
u64 min_prot_len;
int ret;
u8 status_old[1], status_new[1];
- u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
+ loff_t ofs_old, ofs_new;
+ u64 len_old, len_new;
loff_t lock_len;
bool can_be_top = true, can_be_bottom = nor->flags & SNOR_F_HAS_SR_TB;
bool use_top;
@@ -344,7 +350,10 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
return 0;
/* Only modify protection if it will not lock other areas */
- if ((status_new[0] & bp_mask) > (status_old[0] & bp_mask))
+ spi_nor_get_locked_range_sr(nor, status_old, &ofs_old, &len_old);
+ spi_nor_get_locked_range_sr(nor, status_new, &ofs_new, &len_new);
+ if (len_old && len_new &&
+ (ofs_new < ofs_old || (ofs_old + len_old) < (ofs_new + len_new)))
return -EINVAL;
return spi_nor_write_sr_and_check(nor, status_new[0]);
--
2.53.0
^ permalink raw reply related
* [PATCH v4 13/27] mtd: spi-nor: swp: Create helpers for building the SR register
From: Miquel Raynal @ 2026-04-03 16:09 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260403-winbond-v6-18-rc1-spi-nor-swp-v4-0-833dab5e7288@bootlin.com>
The status register contains 3 or 4 BP (Block Protect) bits, 0 or 1
TB (Top/Bottom) bit, soon 0 or 1 CMP (Complement) bit. The last BP bit
and the TB bit locations change between vendors. The whole logic of
buildling the content of the status register based on some input
conditions is used two times and soon will be used 4 times.
Create dedicated helpers for these steps.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/spi-nor/swp.c | 83 +++++++++++++++++++++++++++++------------------
1 file changed, 51 insertions(+), 32 deletions(-)
diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index f068cb9c8f6d..e2e423b20989 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -123,6 +123,43 @@ static bool spi_nor_is_unlocked_sr(struct spi_nor *nor, loff_t ofs, u64 len,
return spi_nor_check_lock_status_sr(nor, ofs, len, sr, false);
}
+static int spi_nor_sr_set_bp_mask(struct spi_nor *nor, u8 *sr, u8 pow)
+{
+ u8 mask = spi_nor_get_sr_bp_mask(nor);
+ u8 val = pow << SR_BP_SHIFT;
+
+ if (nor->flags & SNOR_F_HAS_SR_BP3_BIT6 && val & SR_BP3)
+ val = (val & ~SR_BP3) | SR_BP3_BIT6;
+
+ if (val & ~mask)
+ return -EINVAL;
+
+ sr[0] = val;
+
+ return 0;
+}
+
+static int spi_nor_build_sr(struct spi_nor *nor, const u8 *old_sr, u8 *new_sr,
+ u8 pow, bool use_top)
+{
+ u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
+ u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
+ int ret;
+
+ new_sr[0] = old_sr[0] & ~bp_mask & ~tb_mask;
+
+ /* Build BP field */
+ ret = spi_nor_sr_set_bp_mask(nor, &new_sr[0], pow);
+ if (ret)
+ return ret;
+
+ /* Build TB field */
+ if (!use_top)
+ new_sr[0] |= tb_mask;
+
+ return 0;
+}
+
/*
* Lock a region of the flash. Compatible with ST Micro and similar flash.
* Supports the block protection bits BP{0,1,2}/BP{0,1,2,3} in the status
@@ -162,11 +199,10 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
int ret;
u8 status_old[1] = {}, status_new[1] = {};
u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
- u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
- u8 pow, val;
loff_t lock_len;
bool can_be_top = true, can_be_bottom = nor->flags & SNOR_F_HAS_SR_TB;
bool use_top;
+ u8 pow;
ret = spi_nor_read_sr(nor, nor->bouncebuf);
if (ret)
@@ -200,24 +236,19 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
lock_len = ofs + len;
if (lock_len == nor->params->size) {
- val = bp_mask;
+ pow = (nor->flags & SNOR_F_HAS_4BIT_BP) ? GENMASK(3, 0) : GENMASK(2, 0);
} else {
min_prot_len = spi_nor_get_min_prot_length_sr(nor);
pow = ilog2(lock_len) - ilog2(min_prot_len) + 1;
- val = pow << SR_BP_SHIFT;
-
- if (nor->flags & SNOR_F_HAS_SR_BP3_BIT6 && val & SR_BP3)
- val = (val & ~SR_BP3) | SR_BP3_BIT6;
-
- if (val & ~bp_mask)
- return -EINVAL;
-
- /* Don't "lock" with no region! */
- if (!(val & bp_mask))
- return -EINVAL;
}
- status_new[0] = (status_old[0] & ~bp_mask & ~tb_mask) | val;
+ ret = spi_nor_build_sr(nor, status_old, status_new, pow, use_top);
+ if (ret)
+ return ret;
+
+ /* Don't "lock" with no region! */
+ if (!(status_new[0] & bp_mask))
+ return -EINVAL;
/*
* Disallow further writes if WP# pin is neither left floating nor
@@ -227,9 +258,6 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
if (!(nor->flags & SNOR_F_NO_WP))
status_new[0] |= SR_SRWD;
- if (!use_top)
- status_new[0] |= tb_mask;
-
/* Don't bother if they're the same */
if (status_new[0] == status_old[0])
return 0;
@@ -252,11 +280,10 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
int ret;
u8 status_old[1], status_new[1];
u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
- u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
- u8 pow, val;
loff_t lock_len;
bool can_be_top = true, can_be_bottom = nor->flags & SNOR_F_HAS_SR_TB;
bool use_top;
+ u8 pow;
ret = spi_nor_read_sr(nor, nor->bouncebuf);
if (ret)
@@ -297,29 +324,21 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
lock_len = ofs;
if (lock_len == 0) {
- val = 0; /* fully unlocked */
+ pow = 0; /* fully unlocked */
} else {
min_prot_len = spi_nor_get_min_prot_length_sr(nor);
pow = ilog2(lock_len) - ilog2(min_prot_len) + 1;
- val = pow << SR_BP_SHIFT;
- if (nor->flags & SNOR_F_HAS_SR_BP3_BIT6 && val & SR_BP3)
- val = (val & ~SR_BP3) | SR_BP3_BIT6;
-
- /* Some power-of-two sizes may not be supported */
- if (val & ~bp_mask)
- return -EINVAL;
}
- status_new[0] = (status_old[0] & ~bp_mask & ~tb_mask) | val;
+ ret = spi_nor_build_sr(nor, status_old, status_new, pow, use_top);
+ if (ret)
+ return ret;
/* Don't protect status register if we're fully unlocked */
if (lock_len == 0)
status_new[0] &= ~SR_SRWD;
- if (!use_top)
- status_new[0] |= tb_mask;
-
/* Don't bother if they're the same */
if (status_new[0] == status_old[0])
return 0;
--
2.53.0
^ permalink raw reply related
* [PATCH v4 12/27] mtd: spi-nor: swp: Create a TB intermediate variable
From: Miquel Raynal @ 2026-04-03 16:09 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260403-winbond-v6-18-rc1-spi-nor-swp-v4-0-833dab5e7288@bootlin.com>
Ease the future reuse of the tb (Top/Bottom) boolean by creating an
intermediate variable.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/spi-nor/swp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index 66f85826ba0d..f068cb9c8f6d 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -60,6 +60,7 @@ static void spi_nor_get_locked_range_sr(struct spi_nor *nor, const u8 *sr, loff_
u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
u8 bp, val = sr[0] & bp_mask;
+ bool tb = (nor->flags & SNOR_F_HAS_SR_TB) ? sr[0] & tb_mask : 0;
if (nor->flags & SNOR_F_HAS_SR_BP3_BIT6 && val & SR_BP3_BIT6)
val = (val & ~SR_BP3_BIT6) | SR_BP3;
@@ -79,7 +80,7 @@ static void spi_nor_get_locked_range_sr(struct spi_nor *nor, const u8 *sr, loff_
if (*len > nor->params->size)
*len = nor->params->size;
- if (nor->flags & SNOR_F_HAS_SR_TB && sr[0] & tb_mask)
+ if (tb)
*ofs = 0;
else
*ofs = nor->params->size - *len;
--
2.53.0
^ permalink raw reply related
* [PATCH v4 11/27] mtd: spi-nor: swp: Rename a mask
From: Miquel Raynal @ 2026-04-03 16:09 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260403-winbond-v6-18-rc1-spi-nor-swp-v4-0-833dab5e7288@bootlin.com>
"mask" is not very descriptive when we already manipulate two masks, and
soon will manipulate three. Rename it "bp_mask" to align with the
existing "tb_mask" and soon "cmp_mask".
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/spi-nor/swp.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index af6e577d9b42..66f85826ba0d 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -57,9 +57,9 @@ static void spi_nor_get_locked_range_sr(struct spi_nor *nor, const u8 *sr, loff_
u64 *len)
{
u64 min_prot_len;
- u8 mask = spi_nor_get_sr_bp_mask(nor);
+ u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
- u8 bp, val = sr[0] & mask;
+ u8 bp, val = sr[0] & bp_mask;
if (nor->flags & SNOR_F_HAS_SR_BP3_BIT6 && val & SR_BP3_BIT6)
val = (val & ~SR_BP3_BIT6) | SR_BP3;
@@ -160,7 +160,7 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
u64 min_prot_len;
int ret;
u8 status_old[1] = {}, status_new[1] = {};
- u8 mask = spi_nor_get_sr_bp_mask(nor);
+ u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
u8 pow, val;
loff_t lock_len;
@@ -199,7 +199,7 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
lock_len = ofs + len;
if (lock_len == nor->params->size) {
- val = mask;
+ val = bp_mask;
} else {
min_prot_len = spi_nor_get_min_prot_length_sr(nor);
pow = ilog2(lock_len) - ilog2(min_prot_len) + 1;
@@ -208,15 +208,15 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
if (nor->flags & SNOR_F_HAS_SR_BP3_BIT6 && val & SR_BP3)
val = (val & ~SR_BP3) | SR_BP3_BIT6;
- if (val & ~mask)
+ if (val & ~bp_mask)
return -EINVAL;
/* Don't "lock" with no region! */
- if (!(val & mask))
+ if (!(val & bp_mask))
return -EINVAL;
}
- status_new[0] = (status_old[0] & ~mask & ~tb_mask) | val;
+ status_new[0] = (status_old[0] & ~bp_mask & ~tb_mask) | val;
/*
* Disallow further writes if WP# pin is neither left floating nor
@@ -234,7 +234,7 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
return 0;
/* Only modify protection if it will not unlock other areas */
- if ((status_new[0] & mask) < (status_old[0] & mask))
+ if ((status_new[0] & bp_mask) < (status_old[0] & bp_mask))
return -EINVAL;
return spi_nor_write_sr_and_check(nor, status_new[0]);
@@ -250,7 +250,7 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
u64 min_prot_len;
int ret;
u8 status_old[1], status_new[1];
- u8 mask = spi_nor_get_sr_bp_mask(nor);
+ u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
u8 pow, val;
loff_t lock_len;
@@ -306,11 +306,11 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
val = (val & ~SR_BP3) | SR_BP3_BIT6;
/* Some power-of-two sizes may not be supported */
- if (val & ~mask)
+ if (val & ~bp_mask)
return -EINVAL;
}
- status_new[0] = (status_old[0] & ~mask & ~tb_mask) | val;
+ status_new[0] = (status_old[0] & ~bp_mask & ~tb_mask) | val;
/* Don't protect status register if we're fully unlocked */
if (lock_len == 0)
@@ -324,7 +324,7 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
return 0;
/* Only modify protection if it will not lock other areas */
- if ((status_new[0] & mask) > (status_old[0] & mask))
+ if ((status_new[0] & bp_mask) > (status_old[0] & bp_mask))
return -EINVAL;
return spi_nor_write_sr_and_check(nor, status_new[0]);
--
2.53.0
^ permalink raw reply related
* [PATCH v4 10/27] mtd: spi-nor: swp: Create a helper that writes SR, CR and checks
From: Miquel Raynal @ 2026-04-03 16:09 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260403-winbond-v6-18-rc1-spi-nor-swp-v4-0-833dab5e7288@bootlin.com>
There are many helpers already to either read and/or write SR and/or CR,
as well as sometimes check the returned values. In order to be able to
switch from a 1 byte status register to a 2 bytes status register while
keeping the same level of verification, let's introduce a new helper
that writes them both (atomically) and then reads them back (separated)
to compare the values.
In case 2 bytes registers are not supported, we still have the usual
fallback available in the helper being exported to the rest of the core.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/spi-nor/core.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/mtd/spi-nor/core.h | 1 +
2 files changed, 66 insertions(+)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 1eee519c01e5..f007bd6b389a 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -976,6 +976,54 @@ int spi_nor_write_16bit_cr_and_check(struct spi_nor *nor, u8 cr)
return 0;
}
+/**
+ * spi_nor_write_16bit_sr_cr_and_check() - Write the Status Register 1 and the
+ * Configuration Register in one shot. Ensure that the bytes written in both
+ * registers match the received value.
+ * @nor: pointer to a 'struct spi_nor'.
+ * @regs: two-byte array with values to be written to the status and
+ * configuration registers.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_16bit_sr_cr_and_check(struct spi_nor *nor, const u8 *regs)
+{
+ u8 written_regs[2];
+ int ret;
+
+ written_regs[0] = regs[0];
+ written_regs[1] = regs[1];
+ nor->bouncebuf[0] = regs[0];
+ nor->bouncebuf[1] = regs[1];
+
+ ret = spi_nor_write_sr(nor, nor->bouncebuf, 2);
+ if (ret)
+ return ret;
+
+ ret = spi_nor_read_sr(nor, &nor->bouncebuf[0]);
+ if (ret)
+ return ret;
+
+ if (written_regs[0] != nor->bouncebuf[0]) {
+ dev_dbg(nor->dev, "SR: Read back test failed\n");
+ return -EIO;
+ }
+
+ if (nor->flags & SNOR_F_NO_READ_CR)
+ return 0;
+
+ ret = spi_nor_read_cr(nor, &nor->bouncebuf[1]);
+ if (ret)
+ return ret;
+
+ if (written_regs[1] != nor->bouncebuf[1]) {
+ dev_dbg(nor->dev, "CR: read back test failed\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
/**
* spi_nor_write_sr_and_check() - Write the Status Register 1 and ensure that
* the byte written match the received value without affecting other bits in the
@@ -993,6 +1041,23 @@ int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1)
return spi_nor_write_sr1_and_check(nor, sr1);
}
+/**
+ * spi_nor_write_sr_cr_and_check() - Write the Status Register 1 and ensure that
+ * the byte written match the received value. Same for the Control Register if
+ * available.
+ * @nor: pointer to a 'struct spi_nor'.
+ * @regs: byte array to be written to the registers.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+int spi_nor_write_sr_cr_and_check(struct spi_nor *nor, const u8 *regs)
+{
+ if (nor->flags & SNOR_F_HAS_16BIT_SR)
+ return spi_nor_write_16bit_sr_cr_and_check(nor, regs);
+
+ return spi_nor_write_sr1_and_check(nor, regs[0]);
+}
+
/**
* spi_nor_write_sr2() - Write the Status Register 2 using the
* SPINOR_OP_WRSR2 (3eh) command.
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 16b382d4f04f..3dc9ba3bc6da 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -632,6 +632,7 @@ int spi_nor_read_cr(struct spi_nor *nor, u8 *cr);
int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len);
int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1);
int spi_nor_write_16bit_cr_and_check(struct spi_nor *nor, u8 cr);
+int spi_nor_write_sr_cr_and_check(struct spi_nor *nor, const u8 *regs);
ssize_t spi_nor_read_data(struct spi_nor *nor, loff_t from, size_t len,
u8 *buf);
--
2.53.0
^ permalink raw reply related
* [PATCH v4 09/27] mtd: spi-nor: swp: Use a pointer for SR instead of a single byte
From: Miquel Raynal @ 2026-04-03 16:09 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260403-winbond-v6-18-rc1-spi-nor-swp-v4-0-833dab5e7288@bootlin.com>
At this stage, the Status Register is most often seen as a single
byte. This is subject to change when we will need to read the CMP bit
which is located in the Control Register (kind of secondary status
register). Both will need to be carried.
Change a few prototypes to carry a u8 pointer. This way it also makes it
very clear where we access the first register, and where we will access
the second.
There is no functional change.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/spi-nor/swp.c | 48 ++++++++++++++++++++++++-----------------------
1 file changed, 25 insertions(+), 23 deletions(-)
diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index 246f6d5ca8dd..af6e577d9b42 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -53,13 +53,13 @@ static u64 spi_nor_get_min_prot_length_sr(struct spi_nor *nor)
return sector_size;
}
-static void spi_nor_get_locked_range_sr(struct spi_nor *nor, u8 sr, loff_t *ofs,
+static void spi_nor_get_locked_range_sr(struct spi_nor *nor, const u8 *sr, loff_t *ofs,
u64 *len)
{
u64 min_prot_len;
u8 mask = spi_nor_get_sr_bp_mask(nor);
u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
- u8 bp, val = sr & mask;
+ u8 bp, val = sr[0] & mask;
if (nor->flags & SNOR_F_HAS_SR_BP3_BIT6 && val & SR_BP3_BIT6)
val = (val & ~SR_BP3_BIT6) | SR_BP3;
@@ -79,7 +79,7 @@ static void spi_nor_get_locked_range_sr(struct spi_nor *nor, u8 sr, loff_t *ofs,
if (*len > nor->params->size)
*len = nor->params->size;
- if (nor->flags & SNOR_F_HAS_SR_TB && sr & tb_mask)
+ if (nor->flags & SNOR_F_HAS_SR_TB && sr[0] & tb_mask)
*ofs = 0;
else
*ofs = nor->params->size - *len;
@@ -90,7 +90,7 @@ static void spi_nor_get_locked_range_sr(struct spi_nor *nor, u8 sr, loff_t *ofs,
* (if @locked is false); false otherwise.
*/
static bool spi_nor_check_lock_status_sr(struct spi_nor *nor, loff_t ofs,
- u64 len, u8 sr, bool locked)
+ u64 len, const u8 *sr, bool locked)
{
loff_t lock_offs, lock_offs_max, offs_max;
u64 lock_len;
@@ -111,13 +111,13 @@ static bool spi_nor_check_lock_status_sr(struct spi_nor *nor, loff_t ofs,
return (ofs >= lock_offs_max) || (offs_max <= lock_offs);
}
-static bool spi_nor_is_locked_sr(struct spi_nor *nor, loff_t ofs, u64 len, u8 sr)
+static bool spi_nor_is_locked_sr(struct spi_nor *nor, loff_t ofs, u64 len, const u8 *sr)
{
return spi_nor_check_lock_status_sr(nor, ofs, len, sr, true);
}
static bool spi_nor_is_unlocked_sr(struct spi_nor *nor, loff_t ofs, u64 len,
- u8 sr)
+ const u8 *sr)
{
return spi_nor_check_lock_status_sr(nor, ofs, len, sr, false);
}
@@ -158,7 +158,8 @@ static bool spi_nor_is_unlocked_sr(struct spi_nor *nor, loff_t ofs, u64 len,
static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
{
u64 min_prot_len;
- int ret, status_old, status_new;
+ int ret;
+ u8 status_old[1] = {}, status_new[1] = {};
u8 mask = spi_nor_get_sr_bp_mask(nor);
u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
u8 pow, val;
@@ -170,7 +171,7 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
if (ret)
return ret;
- status_old = nor->bouncebuf[0];
+ status_old[0] = nor->bouncebuf[0];
/* If nothing in our range is unlocked, we don't need to do anything */
if (spi_nor_is_locked_sr(nor, ofs, len, status_old))
@@ -215,7 +216,7 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
return -EINVAL;
}
- status_new = (status_old & ~mask & ~tb_mask) | val;
+ status_new[0] = (status_old[0] & ~mask & ~tb_mask) | val;
/*
* Disallow further writes if WP# pin is neither left floating nor
@@ -223,20 +224,20 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
* WP# pin hard strapped to GND can be a valid use case.
*/
if (!(nor->flags & SNOR_F_NO_WP))
- status_new |= SR_SRWD;
+ status_new[0] |= SR_SRWD;
if (!use_top)
- status_new |= tb_mask;
+ status_new[0] |= tb_mask;
/* Don't bother if they're the same */
- if (status_new == status_old)
+ if (status_new[0] == status_old[0])
return 0;
/* Only modify protection if it will not unlock other areas */
- if ((status_new & mask) < (status_old & mask))
+ if ((status_new[0] & mask) < (status_old[0] & mask))
return -EINVAL;
- return spi_nor_write_sr_and_check(nor, status_new);
+ return spi_nor_write_sr_and_check(nor, status_new[0]);
}
/*
@@ -247,7 +248,8 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
{
u64 min_prot_len;
- int ret, status_old, status_new;
+ int ret;
+ u8 status_old[1], status_new[1];
u8 mask = spi_nor_get_sr_bp_mask(nor);
u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
u8 pow, val;
@@ -259,7 +261,7 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
if (ret)
return ret;
- status_old = nor->bouncebuf[0];
+ status_old[0] = nor->bouncebuf[0];
/* If nothing in our range is locked, we don't need to do anything */
if (spi_nor_is_unlocked_sr(nor, ofs, len, status_old))
@@ -308,24 +310,24 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
return -EINVAL;
}
- status_new = (status_old & ~mask & ~tb_mask) | val;
+ status_new[0] = (status_old[0] & ~mask & ~tb_mask) | val;
/* Don't protect status register if we're fully unlocked */
if (lock_len == 0)
- status_new &= ~SR_SRWD;
+ status_new[0] &= ~SR_SRWD;
if (!use_top)
- status_new |= tb_mask;
+ status_new[0] |= tb_mask;
/* Don't bother if they're the same */
- if (status_new == status_old)
+ if (status_new[0] == status_old[0])
return 0;
/* Only modify protection if it will not lock other areas */
- if ((status_new & mask) > (status_old & mask))
+ if ((status_new[0] & mask) > (status_old[0] & mask))
return -EINVAL;
- return spi_nor_write_sr_and_check(nor, status_new);
+ return spi_nor_write_sr_and_check(nor, status_new[0]);
}
/*
@@ -343,7 +345,7 @@ static int spi_nor_sr_is_locked(struct spi_nor *nor, loff_t ofs, u64 len)
if (ret)
return ret;
- return spi_nor_is_locked_sr(nor, ofs, len, nor->bouncebuf[0]);
+ return spi_nor_is_locked_sr(nor, ofs, len, nor->bouncebuf);
}
/*
--
2.53.0
^ permalink raw reply related
* [PATCH v4 08/27] mtd: spi-nor: swp: Clarify a comment
From: Miquel Raynal @ 2026-04-03 16:09 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260403-winbond-v6-18-rc1-spi-nor-swp-v4-0-833dab5e7288@bootlin.com>
The comment states that some power of two sizes are not supported. This
is very device dependent (based on the size), so modulate a bit the
sentence to make it more accurate.
Reviewed-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/spi-nor/swp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index 64a917543928..246f6d5ca8dd 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -303,7 +303,7 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
if (nor->flags & SNOR_F_HAS_SR_BP3_BIT6 && val & SR_BP3)
val = (val & ~SR_BP3) | SR_BP3_BIT6;
- /* Some power-of-two sizes are not supported */
+ /* Some power-of-two sizes may not be supported */
if (val & ~mask)
return -EINVAL;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v4 07/27] mtd: spi-nor: swp: Explain the MEMLOCK ioctl implementation behaviour
From: Miquel Raynal @ 2026-04-03 16:09 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260403-winbond-v6-18-rc1-spi-nor-swp-v4-0-833dab5e7288@bootlin.com>
Add comments about how these requests are actually handled in the SPI
NOR core. Their behaviour was not entirely clear to me at first, and
explaining them in plain English sounds the way to go.
Reviewed-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/spi-nor/swp.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index 1d50db1ef1a0..64a917543928 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -346,6 +346,14 @@ static int spi_nor_sr_is_locked(struct spi_nor *nor, loff_t ofs, u64 len)
return spi_nor_is_locked_sr(nor, ofs, len, nor->bouncebuf[0]);
}
+/*
+ * These ioctls behave according to the following rules:
+ * ->lock(): Never locks more than what is requested, ie. may lock less
+ * ->unlock(): Never unlocks more than what is requested, ie. may unlock less
+ * -is_locked(): Checks if the region is *fully* locked, returns false otherwise.
+ * This feeback may be misleading because users may get an "unlocked"
+ * status even though a subpart of the region is effectively locked.
+ */
static const struct spi_nor_locking_ops spi_nor_sr_locking_ops = {
.lock = spi_nor_sr_lock,
.unlock = spi_nor_sr_unlock,
--
2.53.0
^ permalink raw reply related
* [PATCH v4 06/27] mtd: spi-nor: debugfs: Enhance output
From: Miquel Raynal @ 2026-04-03 16:09 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260403-winbond-v6-18-rc1-spi-nor-swp-v4-0-833dab5e7288@bootlin.com>
Align the number of dashes to the bigger column width (the title in this
case) to make the output more pleasant and aligned with what is done
in the "params" file output.
Reviewed-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/spi-nor/debugfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
index 69830ad43990..d0191eb9f879 100644
--- a/drivers/mtd/spi-nor/debugfs.c
+++ b/drivers/mtd/spi-nor/debugfs.c
@@ -144,7 +144,7 @@ static int spi_nor_params_show(struct seq_file *s, void *data)
seq_puts(s, "\nsector map\n");
seq_puts(s, " region (in hex) | erase mask | overlaid\n");
- seq_puts(s, " ------------------+------------+----------\n");
+ seq_puts(s, " ------------------+------------+---------\n");
for (i = 0; i < erase_map->n_regions; i++) {
u64 start = region[i].offset;
u64 end = start + region[i].size - 1;
--
2.53.0
^ permalink raw reply related
* [PATCH v4 05/27] mtd: spi-nor: debugfs: Align variable access with the rest of the file
From: Miquel Raynal @ 2026-04-03 16:09 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260403-winbond-v6-18-rc1-spi-nor-swp-v4-0-833dab5e7288@bootlin.com>
The "params" variable is used everywhere else, align this particular
line of the file to use "params" directly rather than the "nor" pointer.
Reviewed-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/spi-nor/debugfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
index d700e0b27182..69830ad43990 100644
--- a/drivers/mtd/spi-nor/debugfs.c
+++ b/drivers/mtd/spi-nor/debugfs.c
@@ -139,7 +139,7 @@ static int spi_nor_params_show(struct seq_file *s, void *data)
if (!(nor->flags & SNOR_F_NO_OP_CHIP_ERASE)) {
string_get_size(params->size, 1, STRING_UNITS_2, buf, sizeof(buf));
- seq_printf(s, " %02x (%s)\n", nor->params->die_erase_opcode, buf);
+ seq_printf(s, " %02x (%s)\n", params->die_erase_opcode, buf);
}
seq_puts(s, "\nsector map\n");
--
2.53.0
^ permalink raw reply related
* [PATCH v4 04/27] mtd: spi-nor: Improve opcodes documentation
From: Miquel Raynal @ 2026-04-03 16:09 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260403-winbond-v6-18-rc1-spi-nor-swp-v4-0-833dab5e7288@bootlin.com>
There are two status registers, named 1 and 2. The current wording is
misleading as "1" may refer to the status register ID as well as the
number of bytes required (which, in this case can be 1 or 2).
Clarify the comments by aligning them on the same pattern:
"{read,write} status {1,2} register"
Reviewed-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
include/linux/mtd/spi-nor.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index cdcfe0fd2e7d..90a0cf583512 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -21,8 +21,8 @@
/* Flash opcodes. */
#define SPINOR_OP_WRDI 0x04 /* Write disable */
#define SPINOR_OP_WREN 0x06 /* Write enable */
-#define SPINOR_OP_RDSR 0x05 /* Read status register */
-#define SPINOR_OP_WRSR 0x01 /* Write status register 1 byte */
+#define SPINOR_OP_RDSR 0x05 /* Read status register 1 */
+#define SPINOR_OP_WRSR 0x01 /* Write status register 1 */
#define SPINOR_OP_RDSR2 0x3f /* Read status register 2 */
#define SPINOR_OP_WRSR2 0x3e /* Write status register 2 */
#define SPINOR_OP_READ 0x03 /* Read data bytes (low frequency) */
--
2.53.0
^ permalink raw reply related
* [PATCH v4 03/27] mtd: spi-nor: swp: Improve locking user experience
From: Miquel Raynal @ 2026-04-03 16:09 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal, stable
In-Reply-To: <20260403-winbond-v6-18-rc1-spi-nor-swp-v4-0-833dab5e7288@bootlin.com>
In the case of the first block being locked (or the few first blocks),
if the user want to fully unlock the device it has two possibilities:
- either it asks to unlock the entire device, and this works;
- or it asks to unlock just the block(s) that are currently locked,
which fails.
It fails because the conditions "can_be_top" and "can_be_bottom" are
true. Indeed, in this case, we unlock everything, so the TB bit does not
matter. However in the current implementation, use_top would be true (as
this is the favourite option) and lock_len, which in practice should be
reduced down to 0, is set to "nor->params->size - (ofs + len)" which is
a positive number. This is wrong.
An easy way is to simply add an extra condition. In the unlock() path,
if we can achieve the same result from both sides, it means we unlock
everything and lock_len must simply be 0. A comment is added to clarify
that logic.
Fixes: 3dd8012a8eeb ("mtd: spi-nor: add TB (Top/Bottom) protect support")
Cc: stable@kernel.org
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Michael Walle <mwalle@kernel.org>
---
drivers/mtd/spi-nor/swp.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index 9b07f83aeac7..1d50db1ef1a0 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -280,8 +280,15 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
/* Prefer top, if both are valid */
use_top = can_be_top;
- /* lock_len: length of region that should remain locked */
- if (use_top)
+ /*
+ * lock_len: length of region that should remain locked.
+ *
+ * When can_be_top and can_be_bottom booleans are true, both adjacent
+ * regions are unlocked, thus the entire flash can be unlocked.
+ */
+ if (can_be_top && can_be_bottom)
+ lock_len = 0;
+ else if (use_top)
lock_len = nor->params->size - (ofs + len);
else
lock_len = ofs;
--
2.53.0
^ permalink raw reply related
* [PATCH v4 02/27] mtd: spi-nor: debugfs: Fix the flags list
From: Miquel Raynal @ 2026-04-03 16:09 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260403-winbond-v6-18-rc1-spi-nor-swp-v4-0-833dab5e7288@bootlin.com>
As mentioned above the spi_nor_option_flags enumeration in core.h, this
list should be kept in sync with the one in the core.
Add the missing flag.
Fixes: 6a42bc97ccda ("mtd: spi-nor: core: Allow specifying the byte order in Octal DTR mode")
Reviewed-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/spi-nor/debugfs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
index fa6956144d2e..d700e0b27182 100644
--- a/drivers/mtd/spi-nor/debugfs.c
+++ b/drivers/mtd/spi-nor/debugfs.c
@@ -28,6 +28,7 @@ static const char *const snor_f_names[] = {
SNOR_F_NAME(RWW),
SNOR_F_NAME(ECC),
SNOR_F_NAME(NO_WP),
+ SNOR_F_NAME(SWAP16),
};
#undef SNOR_F_NAME
--
2.53.0
^ permalink raw reply related
* [PATCH v4 01/27] mtd: spi-nor: Drop duplicate Kconfig dependency
From: Miquel Raynal @ 2026-04-03 16:09 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260403-winbond-v6-18-rc1-spi-nor-swp-v4-0-833dab5e7288@bootlin.com>
I do not think the MTD dependency is needed twice. This is likely a
duplicate coming from a former rebase when the spi-nor core got cleaned
up a while ago. Remove the extra line.
Fixes: b35b9a10362d ("mtd: spi-nor: Move m25p80 code in spi-nor.c")
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Michael Walle <mwalle@kernel.org>
---
drivers/mtd/spi-nor/Kconfig | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 24cd25de2b8b..fd05a24d64a9 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -1,7 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
menuconfig MTD_SPI_NOR
tristate "SPI NOR device support"
- depends on MTD
depends on MTD && SPI_MASTER
select SPI_MEM
help
--
2.53.0
^ permalink raw reply related
* [PATCH v4 00/27] mtd: spi-nor: Enhance software protection
From: Miquel Raynal @ 2026-04-03 16:09 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal, stable
Hello,
As recently raised on the mailing-list (link below), it seems that the
"locking" support in SPI NOR could benefit from some enhancements. As I
myself had to dig into it recently, here is a proposal.
First issue that I see, the MEMLOCK ioctl is not behaving correctly
in some cases, as addressed in:
mtd: spi-nor: swp: Improve locking user experience
Then there is no clear explanation of the shortcuts taken by the kernel
in terms of uAPI, so there is an attempt to list them in:
mtd: spi-nor: swp: Explain the MEMLOCK ioctl implementation behaviour
Plus, Tudor also asked if we could cover locking in the testing
procedure, which is done in:
mtd: spi-nor: Add steps for testing locking support
In order to simplify this procedure, and because it got very helpful
during my testing/development, I want to propose additions to the
debugfs output:
mtd: spi-nor: debugfs: Add locking support TODO: make the captures again
Finally, I am providing an implementation for the complement (CMP)
feature in order to allow finer control of the regions locked. This
feature is for instance available on Winbond chips:
[core] mtd: spi-nor: swp: Add support for the complement feature
[doc] mtd: spi-nor: Add steps for testing locking with CMP
[use] mtd: spi-nor: winbond: Add CMP locking support
Disclaimer: it was much less straightforward than I initially thought to
get the CMP feature working correctly. I tested it with as much focus as
I could, and I am improving the test coverage for the new cases, I am
also providing extra test cases in the metadata of the commit (which do
not make sense to test for chip additions, but may be sensible show when
making core additions like this one), but honestly there are so many
possibilities, I may still be missing corner cases. I hope this will
anyway be helpful to others!
All the other patches are misc improvements or style fixes which I faced
and fixed during my development.
Link: https://lore.kernel.org/linux-mtd/92e99a96-5582-48a5-a4f9-e9b33fcff171@linux.dev/
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
Changes in v4:
- Make sure we don't try to show the (SR specific) debugfs info if the
chip does not support an SR based locking scheme. For this, add a new
helper to derive whether we are using the default ops or not.
- Fix compilation issue on arm32, by using div_u64.
- Link to v3: https://lore.kernel.org/r/20260317-winbond-v6-18-rc1-spi-nor-swp-v3-0-2ca9ea4e7b9b@bootlin.com
Changes in v3:
- No change at all, just rebased on top of v7.0-rc1.
- Collected 2 R-by from M. Walle.
- Link to v2: https://lore.kernel.org/r/20260108-winbond-v6-18-rc1-spi-nor-swp-v2-0-c462ef806130@bootlin.com
Changes in v2:
- Collect tags.
- Add missing Fixes/Cc: stable tags.
- Add a comment explaining why can_be_top && can_be_bottom is a specific
condition.
- Fix commit logs following Michael Walle's reviews.
- Amend the documentation following our discussion with Michael Walle as
well.
- Cache the SR register for debugfs use.
- Create a locked sector map file instead of dumping it as part of the
`params` file output.
- Improved greatly the output of the map as suggested by Michael.
- Add a patch fixing a duplicate dependency in Kconfig.
- Add an important comment in the doc about the small 4kiB erase size
choice.
- Add test runs for each and every chip for which the CMP feature is
added. This prove me that testing of each and every chip was needed,
as some of them seem to feature a broken BFPT table which does not
advertise a working 35h (Read CR) command.
- Added a condition on which the CMP feature is enabled: RDCR must be
possible.
- Link to v1: https://lore.kernel.org/r/20251114-winbond-v6-18-rc1-spi-nor-swp-v1-0-487bc7129931@bootlin.com
---
Miquel Raynal (27):
mtd: spi-nor: Drop duplicate Kconfig dependency
mtd: spi-nor: debugfs: Fix the flags list
mtd: spi-nor: swp: Improve locking user experience
mtd: spi-nor: Improve opcodes documentation
mtd: spi-nor: debugfs: Align variable access with the rest of the file
mtd: spi-nor: debugfs: Enhance output
mtd: spi-nor: swp: Explain the MEMLOCK ioctl implementation behaviour
mtd: spi-nor: swp: Clarify a comment
mtd: spi-nor: swp: Use a pointer for SR instead of a single byte
mtd: spi-nor: swp: Create a helper that writes SR, CR and checks
mtd: spi-nor: swp: Rename a mask
mtd: spi-nor: swp: Create a TB intermediate variable
mtd: spi-nor: swp: Create helpers for building the SR register
mtd: spi-nor: swp: Simplify checking the locked/unlocked range
mtd: spi-nor: swp: Cosmetic changes
mtd: spi-nor: Create a local SR cache
mtd: spi-nor: debugfs: Add locking support
mtd: spi-nor: debugfs: Add a locked sectors map
mtd: spi-nor: Add steps for testing locking support
mtd: spi-nor: swp: Add support for the complement feature
mtd: spi-nor: Add steps for testing locking with CMP
mtd: spi-nor: winbond: Add W25H512NWxxAM CMP locking support
mtd: spi-nor: winbond: Add W25H01NWxxAM CMP locking support
mtd: spi-nor: winbond: Add W25H02NWxxAM CMP locking support
mtd: spi-nor: winbond: Add W25H01NWxxIQ CMP locking support
mtd: spi-nor: winbond: Add W25Q01NWxxIM CMP locking support
mtd: spi-nor: winbond: Add W25Q02NWxxIM CMP locking support
Documentation/driver-api/mtd/spi-nor.rst | 164 ++++++++++++++
drivers/mtd/spi-nor/Kconfig | 1 -
drivers/mtd/spi-nor/core.c | 74 ++++++-
drivers/mtd/spi-nor/core.h | 11 +
drivers/mtd/spi-nor/debugfs.c | 69 +++++-
drivers/mtd/spi-nor/swp.c | 364 ++++++++++++++++++++++++-------
drivers/mtd/spi-nor/winbond.c | 41 +++-
include/linux/mtd/spi-nor.h | 7 +-
8 files changed, 643 insertions(+), 88 deletions(-)
---
base-commit: 2cd9033836346e630afaf847c4f1ced08c4d1846
change-id: 20251114-winbond-v6-18-rc1-spi-nor-swp-865d36f4f695
Best regards,
--
Miquel Raynal <miquel.raynal@bootlin.com>
^ permalink raw reply
* Re: [PATCH v3 17/27] mtd: spi-nor: debugfs: Add locking support
From: Miquel Raynal @ 2026-04-03 16:06 UTC (permalink / raw)
To: Takahiro.Kuwano
Cc: pratyush, mwalle, richard, vigneshr, corbet, tudor.ambarus,
sean.anderson, thomas.petazzoni, STLin2, linux-mtd, linux-kernel,
linux-doc
In-Reply-To: <ee2057a104d44d3dbfbedcf5b1ed80b2@infineon.com>
Hi Takahiro,
>> +u64 spi_nor_get_min_prot_length_sr(struct spi_nor *nor);
>> +void spi_nor_get_locked_range_sr(struct spi_nor *nor, const u8 *sr, loff_t *ofs, u64 *len);
>> +bool spi_nor_is_locked_sr(struct spi_nor *nor, loff_t ofs, u64 len, const u8 *sr);
>> +
>
> It would be better to have generic helper functions rather than using SR-based
> functions directly. The locking_ops is vendor/chip specific and can provide
> other locking mechanism than SR-based block protection. For instance, Infineon,
> Micron, Macronix (and maybe other vendors) offer protection mechanisms that can
> protect sectors individually with volatile or non-volatile manner.
I get what you mean, thanks for pointing this out. Unfortunately it's
not that straightforward.
As of today, there are the "default" locking ops (SR based) and a few
others, chip specific. For debugging purposes, these "other" ops do not
provide any kind of useful feedback regarding what has actually been
locked. Only SR based chips provide this, it is useful, I want to use it.
So I'm going to expose a new boolean to filter out whether we have
locking support *and* if yes, whether we can use SR based functions, this
should also address this concern:
> Don't we need to check 'SNOR_F_HAS_LOCK' flag here?
We will still need to expose the SR specific helpers, though, but at
least we will no longer get inconsistencies with chips not featuring the
default SR approach.
>> + seq_puts(s, "\nlocked sectors\n");
>> + seq_puts(s, " region (in hex) | status | #blocks\n");
>> + seq_puts(s, " ------------------+----------+--------\n");
>> +
>> + spi_nor_get_locked_range_sr(nor, nor->dfs_sr_cache, &lock_start, &lock_length);
>> + if (!lock_length || lock_length == params->size) {
>> + seq_printf(s, " %08llx-%08llx | %s | %llu\n", 0ULL, params->size - 1,
>> + lock_length ? " locked" : "unlocked", params->size / min_prot_len);
>
> div_u64() is needed.
> I got undefined reference to `__aeabi_uldivmod' for my 32-bit ARM platform.
> Same for following four seq_printf().
Good catch, I will fix this too.
Thanks,
Miquèl
^ permalink raw reply
* Re: [PATCH v2 2/3] Documentation: explain how to find maintainers addresses for security reports
From: Kees Cook @ 2026-04-03 15:48 UTC (permalink / raw)
To: Willy Tarreau
Cc: greg, edumazet, rdunlap, Jonathan Corbet, skhan, workflows,
linux-doc, linux-kernel
In-Reply-To: <20260403062018.31080-3-w@1wt.eu>
On Fri, Apr 03, 2026 at 08:20:17AM +0200, Willy Tarreau wrote:
> [...]
> +One difficulty for most first-time reporters is to figure the right list of
> +recipients to send a report to. In the Linux kernel, all official maintainers
> +are trusted, so the consequences of accidentally including the wrong maintainer
> +are essentially a bit more noise for that person, i.e. nothing dramatic. As
Yeah, this is the central point: we already trust maintainers; there is
nothing "special" about security@kernel.org.
> [...]
> +single line suitable for use in the To: field of a mailer like this::
> +
> + $ ./scripts/get_maintainer.pl --no-tree --no-l --no-r --no-n --m \
> + --no-git-fallback --no-substatus --no-rolestats --no-multiline \
> + --pattern-depth 1 drivers/example.c
> + dev1@example.com, dev2@example.org
To echo Greg, yeah, this is great, and has been an implicit action we've
done for years, so there's every reason to delegate it to the reporter
to avoid the round-trip.
Though I guess we'll see if these new instructions actually change
anything -- we still have people asking for CVE assignments. :P
Reviewed-by: Kees Cook <kees@kernel.org>
--
Kees Cook
^ permalink raw reply
* Re: [net-next,13/15] ice: add support for unmanaged DPLL on E830 NIC
From: Tony Nguyen @ 2026-04-03 15:44 UTC (permalink / raw)
To: Paolo Abeni
Cc: andriy.shevchenko, pmenzel, aleksandr.loktionov, mika.westerberg,
mbloch, saeedm, piotr.kwapulinski, sx.rinitha, kuba,
grzegorz.nitka, netdev, przemyslaw.kitszel, arkadiusz.kubalewski,
tariqt, davem, dima.ruinskiy, bhelgaas, andrew+netdev, lukas,
jacob.e.keller, richardcochran, takkozu, enjuk,
maximilianpezzullo, joe, przemyslaw.korba, leon, vgrinber,
linux-pci, sunithax.d.mekala, corbet, vinicius.gomes,
ilpo.jarvinen, edumazet, linux-doc, kohei.enju, kohei, horms,
avigailx.dahan
In-Reply-To: <20260402102616.177883-1-pabeni@redhat.com>
On 4/2/2026 3:26 AM, Paolo Abeni wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
>
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> ice: add support for unmanaged DPLL on E830 NIC
>
> Hardware variants of E830 may support an unmanaged DPLL where the
> configuration is hardcoded within the hardware and firmware, meaning
> users cannot modify settings. However, users are able to check the DPLL
> lock status and obtain configuration information through the Linux DPLL
> and devlink health subsystem.
>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
>> index 62f75701d652..2a9eb233dbf4 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_dpll.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
>
> [ ... ]
>
>> @@ -3148,12 +3176,15 @@ ice_dpll_init_direct_pins(struct ice_pf *pf, bool cgu,
>> int ret;
>>
>> ret = ice_dpll_get_pins(pf, pins, start_idx, count, pf->dplls.clock_id);
>> - if (ret)
>> + if (!cgu || ret)
>> return ret;
>> - if (cgu) {
>> +
>> + if (first) {
>> ret = ice_dpll_register_pins(first, pins, ops, count);
>> if (ret)
>> goto release_pins;
>> + }
>> + if (second) {
>> ret = ice_dpll_register_pins(second, pins, ops, count);
>> if (ret)
>> goto unregister_first;
>> @@ -3162,7 +3193,8 @@ ice_dpll_init_direct_pins(struct ice_pf *pf, bool cgu,
>> return 0;
>>
>> unregister_first:
>> - ice_dpll_unregister_pins(first, pins, ops, count);
>> + if (first)
>> + ice_dpll_unregister_pins(first, pins, ops, count);
>> release_pins:
>> ice_dpll_release_pins(pins, count);
>> return ret;
>
> NULL checks were added to ice_dpll_init_direct_pins for handling unmanaged
> DPLL initialization where only one DPLL device is active. However, is the
> corresponding cleanup function ice_dpll_deinit_direct_pins missing the same
> NULL checks?
>
> In unmanaged mode, only pf->dplls.pps.dpll is initialized while
> pf->dplls.eec.dpll remains NULL. When ice_dpll_init_pins calls
> ice_dpll_init_direct_pins(pf, true, outputs, count, num_outputs,
> output_ops, eec.dpll, pps.dpll), if this second call (for output pins)
> fails, the error path calls ice_dpll_deinit_direct_pins(pf, true, ...,
> pps.dpll, eec.dpll).
>
> Looking at ice_dpll_deinit_direct_pins:
>
> static void ice_dpll_deinit_direct_pins(struct ice_pf *pf, bool cgu,
> struct ice_dpll_pin *pins, int count,
> const struct dpll_pin_ops *ops,
> struct dpll_device *first,
> struct dpll_device *second)
> {
> if (cgu) {
> ice_dpll_unregister_pins(first, pins, ops, count);
> ice_dpll_unregister_pins(second, pins, ops, count);
> }
> ice_dpll_release_pins(pins, count);
> }
>
> When cgu=true and second=NULL, ice_dpll_unregister_pins is called with
> dpll=NULL. Then dpll_pin_unregister(dpll, pin) dereferences dpll with
> xa_empty(&dpll->pin_refs), causing a NULL pointer dereference.
>
> Shouldn't ice_dpll_deinit_direct_pins add NULL checks for first and second,
> matching the guards you added in ice_dpll_init_direct_pins?
>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.h b/drivers/net/ethernet/intel/ice/ice_dpll.h
>> index ae42cdea0ee1..2c98b6c6deb0 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_dpll.h
>> +++ b/drivers/net/ethernet/intel/ice/ice_dpll.h
>
> [ ... ]
>
>> @@ -173,3 +181,5 @@ static inline void ice_dpll_deinit(struct ice_pf *pf) { }
>> #define ICE_CGU_R11_SYNCE_S_BYP_CLK GENMASK(6, 1)
>>
>> #define ICE_CGU_BYPASS_MUX_OFFSET_E825C 3
>> +#define ICE_DPLL_UNMANAGED_PIN_NUM 4
>> +#define ICE_DPLL_IN_ESYNC_ENABLED ICE_AQC_GET_CGU_IN_CFG_FLG2_ESYNC_EN
>
> This isn't a bug, but ICE_DPLL_UNMANAGED_PIN_NUM is defined but never
> used. Looking at the unmanaged pin descriptors, ice_e830_unmanaged_inputs
> has 1 entry and ice_e830_unmanaged_outputs has 2 entries (totaling 3 pins,
> not 4).
Hi Paolo,
We're working on changes to resolve these issues.
Thanks,
Tony
^ permalink raw reply
* [PATCH v8 0/3] Support runtime configuration for per-VM's HGATP mode
From: fangyu.yu @ 2026-04-03 15:30 UTC (permalink / raw)
To: pbonzini, corbet, anup, atish.patra, pjw, palmer, aou, alex,
skhan
Cc: guoren, radim.krcmar, andrew.jones, linux-doc, kvm, kvm-riscv,
linux-riscv, linux-kernel, Fangyu Yu
From: Fangyu Yu <fangyu.yu@linux.alibaba.com>
Currently, RISC-V KVM hardcodes the G-stage page table format (HGATP mode)
to the maximum mode detected at boot time (e.g., SV57x4 if supported). but
often such a wide GPA is unnecessary, just as a host sometimes doesn't need
sv57.
This patch reuse KVM_CAP_VM_GPA_BITS to select HGATP.MODE. User-space can
now explicitly request a specific HGATP mode (SV39x4, SV48x4, SV57x4 or
SV32x4) during VM creation.
---
Changes in v8:
- Reuse KVM_CAP_VM_GPA_BITS to advertise and select the effective GPA width
for a VM (Anup's suggestion).
- Handle the kvm == NULL case and task kvm->lock and kvm->slots_lock to serialize
against concurrent vCPU creation and memslot updates (Radim's suggestion).
- Link to v7:
https://lore.kernel.org/linux-riscv/20260402132303.6252-1-fangyu.yu@linux.alibaba.com/
---
Changes in v7 (Anup's suggestions):
- Keep the original HGATP mode probing logic.
- Link to v6:
https://lore.kernel.org/linux-riscv/20260330122601.22140-1-fangyu.yu@linux.alibaba.com/
---
Changes in v6 (Anup's suggestions):
- Reworked kvm_riscv_gstage_gpa_bits() and kvm_riscv_gstage_gpa_size() to
take "unsigned long pgd_levels" instead of "struct kvm_arch *".
- Moved kvm_riscv_gstage_mode() helper from kvm_host.h to kvm_gstage.h.
- Renamed kvm->arch.kvm_riscv_gstage_pgd_levels to kvm->arch.pgd_levels.
- Added pgd_levels to struct kvm_gstage to avoid repeated
gstage->kvm->arch pointer chasing.
- Link to v5:
https://lore.kernel.org/linux-riscv/20260204134507.33912-1-fangyu.yu@linux.alibaba.com/
---
Changes in v5:
- Use architectural HGATP.MODE encodings as the bit index for the supported-mode
bitmap and for the VM-mode selection UAPI; no new UAPI mode/bit defines are
introduced(per Radim).
- Allow KVM_CAP_RISCV_SET_HGATP_MODE on RV32 as well(per Drew).
- Link to v4:
https://lore.kernel.org/linux-riscv/20260202140716.34323-1-fangyu.yu@linux.alibaba.com/
---
Changes in v4:
- Extend kvm_riscv_gstage_mode_detect() to probe all HGATP.MODE values
supported by the host and record them in a bitmask.
- Treat unexpected pgd_levels in kvm_riscv_gstage_mode() as an internal error
(e.g. WARN_ON_ONCE())(per Radim).
- Move kvm_riscv_gstage_gpa_bits() and kvm_riscv_gstage_gpa_size() to header
as static inline helpers(per Radim).
- Drop gstage_mode_user_initialized and Remove the kvm_debug() message from
KVM_CAP_RISCV_SET_HGATP_MODE(per Radim).
- Link to v3:
https://lore.kernel.org/linux-riscv/20260125150450.27068-1-fangyu.yu@linux.alibaba.com/
---
Changes in v3:
- Reworked the patch formatting (per Drew).
- Dropped kvm->arch.kvm_riscv_gstage_mode and derive HGATP.MODE from
kvm_riscv_gstage_pgd_levels via a helper, avoiding redundant per-VM state(per Drew).
- Removed kvm_riscv_gstage_max_mode and keep only kvm_riscv_gstage_max_pgd_levels
for host capability detection(per Drew).
- Other initialization and return value issues(per Drew).
- Enforce that KVM_CAP_RISCV_SET_HGATP_MODE can only be enabled before any vCPUs
are created by rejecting the ioctl once kvm->created_vcpus is non-zero(per Radim).
- Add a memslot safety check and reject the capability unless
kvm_are_all_memslots_empty(kvm) is true, ensuring the G-stage format is not
changed after any memslots have been installed(per Radim).
- Link to v2:
https://lore.kernel.org/linux-riscv/20260105143232.76715-1-fangyu.yu@linux.alibaba.com/
Fangyu Yu (3):
RISC-V: KVM: Support runtime configuration for per-VM's HGATP mode
RISC-V: KVM: Cache gstage pgd_levels in struct kvm_gstage
RISC-V: KVM: Reuse KVM_CAP_VM_GPA_BITS to select HGATP.MODE
arch/riscv/include/asm/kvm_gstage.h | 47 ++++++++++++++++---
arch/riscv/include/asm/kvm_host.h | 1 +
arch/riscv/kvm/gstage.c | 65 +++++++++++++--------------
arch/riscv/kvm/main.c | 12 ++---
arch/riscv/kvm/mmu.c | 70 +++++++++--------------------
arch/riscv/kvm/vm.c | 49 ++++++++++++++++++--
arch/riscv/kvm/vmid.c | 3 +-
7 files changed, 148 insertions(+), 99 deletions(-)
--
2.50.1
^ permalink raw reply
* [PATCH v8 2/3] RISC-V: KVM: Cache gstage pgd_levels in struct kvm_gstage
From: fangyu.yu @ 2026-04-03 15:30 UTC (permalink / raw)
To: pbonzini, corbet, anup, atish.patra, pjw, palmer, aou, alex,
skhan
Cc: guoren, radim.krcmar, andrew.jones, linux-doc, kvm, kvm-riscv,
linux-riscv, linux-kernel, Fangyu Yu
In-Reply-To: <20260403153019.9916-1-fangyu.yu@linux.alibaba.com>
From: Fangyu Yu <fangyu.yu@linux.alibaba.com>
Gstage page-table helpers frequently chase gstage->kvm->arch to
fetch pgd_levels. This adds noise and repeats the same dereference
chain in hot paths.
Add pgd_levels to struct kvm_gstage and initialize it from kvm->arch
when setting up a gstage instance. Introduce kvm_riscv_gstage_init()
to centralize initialization and switch gstage code to use
gstage->pgd_levels.
Suggested-by: Anup Patel <anup@brainfault.org>
Signed-off-by: Fangyu Yu <fangyu.yu@linux.alibaba.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
arch/riscv/include/asm/kvm_gstage.h | 10 ++++++
arch/riscv/kvm/gstage.c | 10 +++---
arch/riscv/kvm/mmu.c | 50 ++++++-----------------------
3 files changed, 25 insertions(+), 45 deletions(-)
diff --git a/arch/riscv/include/asm/kvm_gstage.h b/arch/riscv/include/asm/kvm_gstage.h
index 5aa58d1f692a..70d9d483365e 100644
--- a/arch/riscv/include/asm/kvm_gstage.h
+++ b/arch/riscv/include/asm/kvm_gstage.h
@@ -15,6 +15,7 @@ struct kvm_gstage {
#define KVM_GSTAGE_FLAGS_LOCAL BIT(0)
unsigned long vmid;
pgd_t *pgd;
+ unsigned long pgd_levels;
};
struct kvm_gstage_mapping {
@@ -92,4 +93,13 @@ static inline unsigned long kvm_riscv_gstage_mode(unsigned long pgd_levels)
}
}
+static inline void kvm_riscv_gstage_init(struct kvm_gstage *gstage, struct kvm *kvm)
+{
+ gstage->kvm = kvm;
+ gstage->flags = 0;
+ gstage->vmid = READ_ONCE(kvm->arch.vmid.vmid);
+ gstage->pgd = kvm->arch.pgd;
+ gstage->pgd_levels = kvm->arch.pgd_levels;
+}
+
#endif
diff --git a/arch/riscv/kvm/gstage.c b/arch/riscv/kvm/gstage.c
index 4beb9322fe76..7c4c34bc191b 100644
--- a/arch/riscv/kvm/gstage.c
+++ b/arch/riscv/kvm/gstage.c
@@ -26,7 +26,7 @@ static inline unsigned long gstage_pte_index(struct kvm_gstage *gstage,
unsigned long mask;
unsigned long shift = HGATP_PAGE_SHIFT + (kvm_riscv_gstage_index_bits * level);
- if (level == gstage->kvm->arch.pgd_levels - 1)
+ if (level == gstage->pgd_levels - 1)
mask = (PTRS_PER_PTE * (1UL << kvm_riscv_gstage_pgd_xbits)) - 1;
else
mask = PTRS_PER_PTE - 1;
@@ -45,7 +45,7 @@ static int gstage_page_size_to_level(struct kvm_gstage *gstage, unsigned long pa
u32 i;
unsigned long psz = 1UL << 12;
- for (i = 0; i < gstage->kvm->arch.pgd_levels; i++) {
+ for (i = 0; i < gstage->pgd_levels; i++) {
if (page_size == (psz << (i * kvm_riscv_gstage_index_bits))) {
*out_level = i;
return 0;
@@ -58,7 +58,7 @@ static int gstage_page_size_to_level(struct kvm_gstage *gstage, unsigned long pa
static int gstage_level_to_page_order(struct kvm_gstage *gstage, u32 level,
unsigned long *out_pgorder)
{
- if (gstage->kvm->arch.pgd_levels < level)
+ if (gstage->pgd_levels < level)
return -EINVAL;
*out_pgorder = 12 + (level * kvm_riscv_gstage_index_bits);
@@ -83,7 +83,7 @@ bool kvm_riscv_gstage_get_leaf(struct kvm_gstage *gstage, gpa_t addr,
pte_t **ptepp, u32 *ptep_level)
{
pte_t *ptep;
- u32 current_level = gstage->kvm->arch.pgd_levels - 1;
+ u32 current_level = gstage->pgd_levels - 1;
*ptep_level = current_level;
ptep = (pte_t *)gstage->pgd;
@@ -127,7 +127,7 @@ int kvm_riscv_gstage_set_pte(struct kvm_gstage *gstage,
struct kvm_mmu_memory_cache *pcache,
const struct kvm_gstage_mapping *map)
{
- u32 current_level = gstage->kvm->arch.pgd_levels - 1;
+ u32 current_level = gstage->pgd_levels - 1;
pte_t *next_ptep = (pte_t *)gstage->pgd;
pte_t *ptep = &next_ptep[gstage_pte_index(gstage, map->addr, current_level)];
diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
index fbcdd75cb9af..2d3def024270 100644
--- a/arch/riscv/kvm/mmu.c
+++ b/arch/riscv/kvm/mmu.c
@@ -24,10 +24,7 @@ static void mmu_wp_memory_region(struct kvm *kvm, int slot)
phys_addr_t end = (memslot->base_gfn + memslot->npages) << PAGE_SHIFT;
struct kvm_gstage gstage;
- gstage.kvm = kvm;
- gstage.flags = 0;
- gstage.vmid = READ_ONCE(kvm->arch.vmid.vmid);
- gstage.pgd = kvm->arch.pgd;
+ kvm_riscv_gstage_init(&gstage, kvm);
spin_lock(&kvm->mmu_lock);
kvm_riscv_gstage_wp_range(&gstage, start, end);
@@ -49,10 +46,7 @@ int kvm_riscv_mmu_ioremap(struct kvm *kvm, gpa_t gpa, phys_addr_t hpa,
struct kvm_gstage_mapping map;
struct kvm_gstage gstage;
- gstage.kvm = kvm;
- gstage.flags = 0;
- gstage.vmid = READ_ONCE(kvm->arch.vmid.vmid);
- gstage.pgd = kvm->arch.pgd;
+ kvm_riscv_gstage_init(&gstage, kvm);
end = (gpa + size + PAGE_SIZE - 1) & PAGE_MASK;
pfn = __phys_to_pfn(hpa);
@@ -89,10 +83,7 @@ void kvm_riscv_mmu_iounmap(struct kvm *kvm, gpa_t gpa, unsigned long size)
{
struct kvm_gstage gstage;
- gstage.kvm = kvm;
- gstage.flags = 0;
- gstage.vmid = READ_ONCE(kvm->arch.vmid.vmid);
- gstage.pgd = kvm->arch.pgd;
+ kvm_riscv_gstage_init(&gstage, kvm);
spin_lock(&kvm->mmu_lock);
kvm_riscv_gstage_unmap_range(&gstage, gpa, size, false);
@@ -109,10 +100,7 @@ void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm,
phys_addr_t end = (base_gfn + __fls(mask) + 1) << PAGE_SHIFT;
struct kvm_gstage gstage;
- gstage.kvm = kvm;
- gstage.flags = 0;
- gstage.vmid = READ_ONCE(kvm->arch.vmid.vmid);
- gstage.pgd = kvm->arch.pgd;
+ kvm_riscv_gstage_init(&gstage, kvm);
kvm_riscv_gstage_wp_range(&gstage, start, end);
}
@@ -141,10 +129,7 @@ void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
phys_addr_t size = slot->npages << PAGE_SHIFT;
struct kvm_gstage gstage;
- gstage.kvm = kvm;
- gstage.flags = 0;
- gstage.vmid = READ_ONCE(kvm->arch.vmid.vmid);
- gstage.pgd = kvm->arch.pgd;
+ kvm_riscv_gstage_init(&gstage, kvm);
spin_lock(&kvm->mmu_lock);
kvm_riscv_gstage_unmap_range(&gstage, gpa, size, false);
@@ -250,10 +235,7 @@ bool kvm_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range)
if (!kvm->arch.pgd)
return false;
- gstage.kvm = kvm;
- gstage.flags = 0;
- gstage.vmid = READ_ONCE(kvm->arch.vmid.vmid);
- gstage.pgd = kvm->arch.pgd;
+ kvm_riscv_gstage_init(&gstage, kvm);
mmu_locked = spin_trylock(&kvm->mmu_lock);
kvm_riscv_gstage_unmap_range(&gstage, range->start << PAGE_SHIFT,
(range->end - range->start) << PAGE_SHIFT,
@@ -275,10 +257,7 @@ bool kvm_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
WARN_ON(size != PAGE_SIZE && size != PMD_SIZE && size != PUD_SIZE);
- gstage.kvm = kvm;
- gstage.flags = 0;
- gstage.vmid = READ_ONCE(kvm->arch.vmid.vmid);
- gstage.pgd = kvm->arch.pgd;
+ kvm_riscv_gstage_init(&gstage, kvm);
if (!kvm_riscv_gstage_get_leaf(&gstage, range->start << PAGE_SHIFT,
&ptep, &ptep_level))
return false;
@@ -298,10 +277,7 @@ bool kvm_test_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
WARN_ON(size != PAGE_SIZE && size != PMD_SIZE && size != PUD_SIZE);
- gstage.kvm = kvm;
- gstage.flags = 0;
- gstage.vmid = READ_ONCE(kvm->arch.vmid.vmid);
- gstage.pgd = kvm->arch.pgd;
+ kvm_riscv_gstage_init(&gstage, kvm);
if (!kvm_riscv_gstage_get_leaf(&gstage, range->start << PAGE_SHIFT,
&ptep, &ptep_level))
return false;
@@ -463,10 +439,7 @@ int kvm_riscv_mmu_map(struct kvm_vcpu *vcpu, struct kvm_memory_slot *memslot,
struct kvm_gstage gstage;
struct page *page;
- gstage.kvm = kvm;
- gstage.flags = 0;
- gstage.vmid = READ_ONCE(kvm->arch.vmid.vmid);
- gstage.pgd = kvm->arch.pgd;
+ kvm_riscv_gstage_init(&gstage, kvm);
/* Setup initial state of output mapping */
memset(out_map, 0, sizeof(*out_map));
@@ -587,10 +560,7 @@ void kvm_riscv_mmu_free_pgd(struct kvm *kvm)
spin_lock(&kvm->mmu_lock);
if (kvm->arch.pgd) {
- gstage.kvm = kvm;
- gstage.flags = 0;
- gstage.vmid = READ_ONCE(kvm->arch.vmid.vmid);
- gstage.pgd = kvm->arch.pgd;
+ kvm_riscv_gstage_init(&gstage, kvm);
kvm_riscv_gstage_unmap_range(&gstage, 0UL,
kvm_riscv_gstage_gpa_size(kvm->arch.pgd_levels), false);
pgd = READ_ONCE(kvm->arch.pgd);
--
2.50.1
^ permalink raw reply related
* [PATCH v8 3/3] RISC-V: KVM: Reuse KVM_CAP_VM_GPA_BITS to select HGATP.MODE
From: fangyu.yu @ 2026-04-03 15:30 UTC (permalink / raw)
To: pbonzini, corbet, anup, atish.patra, pjw, palmer, aou, alex,
skhan
Cc: guoren, radim.krcmar, andrew.jones, linux-doc, kvm, kvm-riscv,
linux-riscv, linux-kernel, Fangyu Yu
In-Reply-To: <20260403153019.9916-1-fangyu.yu@linux.alibaba.com>
From: Fangyu Yu <fangyu.yu@linux.alibaba.com>
Reuse KVM_CAP_VM_GPA_BITS to advertise and select the effective
G-stage GPA width for a VM.
KVM_CHECK_EXTENSION(KVM_CAP_VM_GPA_BITS) returns the effective GPA
bits for a VM, KVM_ENABLE_CAP(KVM_CAP_VM_GPA_BITS) allows userspace
to downsize the effective GPA width by selecting a smaller G-stage
page table format:
- gpa_bits <= 41 selects Sv39x4 (pgd_levels=3)
- gpa_bits <= 50 selects Sv48x4 (pgd_levels=4)
- gpa_bits <= 59 selects Sv57x4 (pgd_levels=5)
Reject the request with -EINVAL for unsupported values and with -EBUSY
if vCPUs have been created or any memslot is populated.
Signed-off-by: Fangyu Yu <fangyu.yu@linux.alibaba.com>
Reviewed-by: Andrew Jones <andrew.jones@oss.qualcomm.com>
Reviewed-by: Guo Ren <guoren@kernel.org>
---
arch/riscv/kvm/vm.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 42 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/kvm/vm.c b/arch/riscv/kvm/vm.c
index fb7c4e07961f..a9f083feeb76 100644
--- a/arch/riscv/kvm/vm.c
+++ b/arch/riscv/kvm/vm.c
@@ -214,12 +214,52 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
int kvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
{
+ if (cap->flags)
+ return -EINVAL;
+
switch (cap->cap) {
case KVM_CAP_RISCV_MP_STATE_RESET:
- if (cap->flags)
- return -EINVAL;
kvm->arch.mp_state_reset = true;
return 0;
+ case KVM_CAP_VM_GPA_BITS: {
+ unsigned long gpa_bits = cap->args[0];
+ unsigned long new_levels;
+ int r = 0;
+
+ /* Decide target pgd levels from requested gpa_bits */
+#ifdef CONFIG_64BIT
+ if (gpa_bits <= 41)
+ new_levels = 3; /* Sv39x4 */
+ else if (gpa_bits <= 50)
+ new_levels = 4; /* Sv48x4 */
+ else if (gpa_bits <= 59)
+ new_levels = 5; /* Sv57x4 */
+ else
+ return -EINVAL;
+#else
+ /* 32-bit: only Sv32x4*/
+ if (gpa_bits <= 34)
+ new_levels = 2;
+ else
+ return -EINVAL;
+#endif
+ if (new_levels > kvm_riscv_gstage_max_pgd_levels)
+ return -EINVAL;
+
+ /* Follow KVM's lock ordering: kvm->lock -> kvm->slots_lock. */
+ mutex_lock(&kvm->lock);
+ mutex_lock(&kvm->slots_lock);
+
+ if (kvm->created_vcpus || !kvm_are_all_memslots_empty(kvm))
+ r = -EBUSY;
+ else
+ kvm->arch.pgd_levels = new_levels;
+
+ mutex_unlock(&kvm->slots_lock);
+ mutex_unlock(&kvm->lock);
+
+ return r;
+ }
default:
return -EINVAL;
}
--
2.50.1
^ permalink raw reply related
* [PATCH v8 1/3] RISC-V: KVM: Support runtime configuration for per-VM's HGATP mode
From: fangyu.yu @ 2026-04-03 15:30 UTC (permalink / raw)
To: pbonzini, corbet, anup, atish.patra, pjw, palmer, aou, alex,
skhan
Cc: guoren, radim.krcmar, andrew.jones, linux-doc, kvm, kvm-riscv,
linux-riscv, linux-kernel, Fangyu Yu
In-Reply-To: <20260403153019.9916-1-fangyu.yu@linux.alibaba.com>
From: Fangyu Yu <fangyu.yu@linux.alibaba.com>
Introduces one per-VM architecture-specific fields to support runtime
configuration of the G-stage page table format:
- kvm->arch.pgd_levels: the corresponding number of page table levels
for the selected mode.
These fields replace the previous global variables
kvm_riscv_gstage_mode and kvm_riscv_gstage_pgd_levels, enabling different
virtual machines to independently select their G-stage page table format
instead of being forced to share the maximum mode detected by the kernel
at boot time.
Signed-off-by: Fangyu Yu <fangyu.yu@linux.alibaba.com>
Reviewed-by: Andrew Jones <andrew.jones@oss.qualcomm.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Reviewed-by: Guo Ren <guoren@kernel.org>
---
arch/riscv/include/asm/kvm_gstage.h | 37 ++++++++++++----
arch/riscv/include/asm/kvm_host.h | 1 +
arch/riscv/kvm/gstage.c | 65 ++++++++++++++---------------
arch/riscv/kvm/main.c | 12 +++---
arch/riscv/kvm/mmu.c | 20 +++++----
arch/riscv/kvm/vm.c | 5 ++-
arch/riscv/kvm/vmid.c | 3 +-
7 files changed, 86 insertions(+), 57 deletions(-)
diff --git a/arch/riscv/include/asm/kvm_gstage.h b/arch/riscv/include/asm/kvm_gstage.h
index 595e2183173e..5aa58d1f692a 100644
--- a/arch/riscv/include/asm/kvm_gstage.h
+++ b/arch/riscv/include/asm/kvm_gstage.h
@@ -29,16 +29,22 @@ struct kvm_gstage_mapping {
#define kvm_riscv_gstage_index_bits 10
#endif
-extern unsigned long kvm_riscv_gstage_mode;
-extern unsigned long kvm_riscv_gstage_pgd_levels;
+extern unsigned long kvm_riscv_gstage_max_pgd_levels;
#define kvm_riscv_gstage_pgd_xbits 2
#define kvm_riscv_gstage_pgd_size (1UL << (HGATP_PAGE_SHIFT + kvm_riscv_gstage_pgd_xbits))
-#define kvm_riscv_gstage_gpa_bits (HGATP_PAGE_SHIFT + \
- (kvm_riscv_gstage_pgd_levels * \
- kvm_riscv_gstage_index_bits) + \
- kvm_riscv_gstage_pgd_xbits)
-#define kvm_riscv_gstage_gpa_size ((gpa_t)(1ULL << kvm_riscv_gstage_gpa_bits))
+
+static inline unsigned long kvm_riscv_gstage_gpa_bits(unsigned long pgd_levels)
+{
+ return (HGATP_PAGE_SHIFT +
+ pgd_levels * kvm_riscv_gstage_index_bits +
+ kvm_riscv_gstage_pgd_xbits);
+}
+
+static inline gpa_t kvm_riscv_gstage_gpa_size(unsigned long pgd_levels)
+{
+ return BIT_ULL(kvm_riscv_gstage_gpa_bits(pgd_levels));
+}
bool kvm_riscv_gstage_get_leaf(struct kvm_gstage *gstage, gpa_t addr,
pte_t **ptepp, u32 *ptep_level);
@@ -69,4 +75,21 @@ void kvm_riscv_gstage_wp_range(struct kvm_gstage *gstage, gpa_t start, gpa_t end
void kvm_riscv_gstage_mode_detect(void);
+static inline unsigned long kvm_riscv_gstage_mode(unsigned long pgd_levels)
+{
+ switch (pgd_levels) {
+ case 2:
+ return HGATP_MODE_SV32X4;
+ case 3:
+ return HGATP_MODE_SV39X4;
+ case 4:
+ return HGATP_MODE_SV48X4;
+ case 5:
+ return HGATP_MODE_SV57X4;
+ default:
+ WARN_ON_ONCE(1);
+ return HGATP_MODE_OFF;
+ }
+}
+
#endif
diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h
index 24585304c02b..478f699e9dec 100644
--- a/arch/riscv/include/asm/kvm_host.h
+++ b/arch/riscv/include/asm/kvm_host.h
@@ -94,6 +94,7 @@ struct kvm_arch {
/* G-stage page table */
pgd_t *pgd;
phys_addr_t pgd_phys;
+ unsigned long pgd_levels;
/* Guest Timer */
struct kvm_guest_timer timer;
diff --git a/arch/riscv/kvm/gstage.c b/arch/riscv/kvm/gstage.c
index b67d60d722c2..4beb9322fe76 100644
--- a/arch/riscv/kvm/gstage.c
+++ b/arch/riscv/kvm/gstage.c
@@ -12,22 +12,21 @@
#include <asm/kvm_gstage.h>
#ifdef CONFIG_64BIT
-unsigned long kvm_riscv_gstage_mode __ro_after_init = HGATP_MODE_SV39X4;
-unsigned long kvm_riscv_gstage_pgd_levels __ro_after_init = 3;
+unsigned long kvm_riscv_gstage_max_pgd_levels __ro_after_init = 3;
#else
-unsigned long kvm_riscv_gstage_mode __ro_after_init = HGATP_MODE_SV32X4;
-unsigned long kvm_riscv_gstage_pgd_levels __ro_after_init = 2;
+unsigned long kvm_riscv_gstage_max_pgd_levels __ro_after_init = 2;
#endif
#define gstage_pte_leaf(__ptep) \
(pte_val(*(__ptep)) & (_PAGE_READ | _PAGE_WRITE | _PAGE_EXEC))
-static inline unsigned long gstage_pte_index(gpa_t addr, u32 level)
+static inline unsigned long gstage_pte_index(struct kvm_gstage *gstage,
+ gpa_t addr, u32 level)
{
unsigned long mask;
unsigned long shift = HGATP_PAGE_SHIFT + (kvm_riscv_gstage_index_bits * level);
- if (level == (kvm_riscv_gstage_pgd_levels - 1))
+ if (level == gstage->kvm->arch.pgd_levels - 1)
mask = (PTRS_PER_PTE * (1UL << kvm_riscv_gstage_pgd_xbits)) - 1;
else
mask = PTRS_PER_PTE - 1;
@@ -40,12 +39,13 @@ static inline unsigned long gstage_pte_page_vaddr(pte_t pte)
return (unsigned long)pfn_to_virt(__page_val_to_pfn(pte_val(pte)));
}
-static int gstage_page_size_to_level(unsigned long page_size, u32 *out_level)
+static int gstage_page_size_to_level(struct kvm_gstage *gstage, unsigned long page_size,
+ u32 *out_level)
{
u32 i;
unsigned long psz = 1UL << 12;
- for (i = 0; i < kvm_riscv_gstage_pgd_levels; i++) {
+ for (i = 0; i < gstage->kvm->arch.pgd_levels; i++) {
if (page_size == (psz << (i * kvm_riscv_gstage_index_bits))) {
*out_level = i;
return 0;
@@ -55,21 +55,23 @@ static int gstage_page_size_to_level(unsigned long page_size, u32 *out_level)
return -EINVAL;
}
-static int gstage_level_to_page_order(u32 level, unsigned long *out_pgorder)
+static int gstage_level_to_page_order(struct kvm_gstage *gstage, u32 level,
+ unsigned long *out_pgorder)
{
- if (kvm_riscv_gstage_pgd_levels < level)
+ if (gstage->kvm->arch.pgd_levels < level)
return -EINVAL;
*out_pgorder = 12 + (level * kvm_riscv_gstage_index_bits);
return 0;
}
-static int gstage_level_to_page_size(u32 level, unsigned long *out_pgsize)
+static int gstage_level_to_page_size(struct kvm_gstage *gstage, u32 level,
+ unsigned long *out_pgsize)
{
int rc;
unsigned long page_order = PAGE_SHIFT;
- rc = gstage_level_to_page_order(level, &page_order);
+ rc = gstage_level_to_page_order(gstage, level, &page_order);
if (rc)
return rc;
@@ -81,11 +83,11 @@ bool kvm_riscv_gstage_get_leaf(struct kvm_gstage *gstage, gpa_t addr,
pte_t **ptepp, u32 *ptep_level)
{
pte_t *ptep;
- u32 current_level = kvm_riscv_gstage_pgd_levels - 1;
+ u32 current_level = gstage->kvm->arch.pgd_levels - 1;
*ptep_level = current_level;
ptep = (pte_t *)gstage->pgd;
- ptep = &ptep[gstage_pte_index(addr, current_level)];
+ ptep = &ptep[gstage_pte_index(gstage, addr, current_level)];
while (ptep && pte_val(ptep_get(ptep))) {
if (gstage_pte_leaf(ptep)) {
*ptep_level = current_level;
@@ -97,7 +99,7 @@ bool kvm_riscv_gstage_get_leaf(struct kvm_gstage *gstage, gpa_t addr,
current_level--;
*ptep_level = current_level;
ptep = (pte_t *)gstage_pte_page_vaddr(ptep_get(ptep));
- ptep = &ptep[gstage_pte_index(addr, current_level)];
+ ptep = &ptep[gstage_pte_index(gstage, addr, current_level)];
} else {
ptep = NULL;
}
@@ -110,7 +112,7 @@ static void gstage_tlb_flush(struct kvm_gstage *gstage, u32 level, gpa_t addr)
{
unsigned long order = PAGE_SHIFT;
- if (gstage_level_to_page_order(level, &order))
+ if (gstage_level_to_page_order(gstage, level, &order))
return;
addr &= ~(BIT(order) - 1);
@@ -125,9 +127,9 @@ int kvm_riscv_gstage_set_pte(struct kvm_gstage *gstage,
struct kvm_mmu_memory_cache *pcache,
const struct kvm_gstage_mapping *map)
{
- u32 current_level = kvm_riscv_gstage_pgd_levels - 1;
+ u32 current_level = gstage->kvm->arch.pgd_levels - 1;
pte_t *next_ptep = (pte_t *)gstage->pgd;
- pte_t *ptep = &next_ptep[gstage_pte_index(map->addr, current_level)];
+ pte_t *ptep = &next_ptep[gstage_pte_index(gstage, map->addr, current_level)];
if (current_level < map->level)
return -EINVAL;
@@ -151,7 +153,7 @@ int kvm_riscv_gstage_set_pte(struct kvm_gstage *gstage,
}
current_level--;
- ptep = &next_ptep[gstage_pte_index(map->addr, current_level)];
+ ptep = &next_ptep[gstage_pte_index(gstage, map->addr, current_level)];
}
if (pte_val(*ptep) != pte_val(map->pte)) {
@@ -175,7 +177,7 @@ int kvm_riscv_gstage_map_page(struct kvm_gstage *gstage,
out_map->addr = gpa;
out_map->level = 0;
- ret = gstage_page_size_to_level(page_size, &out_map->level);
+ ret = gstage_page_size_to_level(gstage, page_size, &out_map->level);
if (ret)
return ret;
@@ -217,7 +219,7 @@ void kvm_riscv_gstage_op_pte(struct kvm_gstage *gstage, gpa_t addr,
u32 next_ptep_level;
unsigned long next_page_size, page_size;
- ret = gstage_level_to_page_size(ptep_level, &page_size);
+ ret = gstage_level_to_page_size(gstage, ptep_level, &page_size);
if (ret)
return;
@@ -229,7 +231,7 @@ void kvm_riscv_gstage_op_pte(struct kvm_gstage *gstage, gpa_t addr,
if (ptep_level && !gstage_pte_leaf(ptep)) {
next_ptep = (pte_t *)gstage_pte_page_vaddr(ptep_get(ptep));
next_ptep_level = ptep_level - 1;
- ret = gstage_level_to_page_size(next_ptep_level, &next_page_size);
+ ret = gstage_level_to_page_size(gstage, next_ptep_level, &next_page_size);
if (ret)
return;
@@ -263,7 +265,7 @@ void kvm_riscv_gstage_unmap_range(struct kvm_gstage *gstage,
while (addr < end) {
found_leaf = kvm_riscv_gstage_get_leaf(gstage, addr, &ptep, &ptep_level);
- ret = gstage_level_to_page_size(ptep_level, &page_size);
+ ret = gstage_level_to_page_size(gstage, ptep_level, &page_size);
if (ret)
break;
@@ -297,7 +299,7 @@ void kvm_riscv_gstage_wp_range(struct kvm_gstage *gstage, gpa_t start, gpa_t end
while (addr < end) {
found_leaf = kvm_riscv_gstage_get_leaf(gstage, addr, &ptep, &ptep_level);
- ret = gstage_level_to_page_size(ptep_level, &page_size);
+ ret = gstage_level_to_page_size(gstage, ptep_level, &page_size);
if (ret)
break;
@@ -319,39 +321,34 @@ void __init kvm_riscv_gstage_mode_detect(void)
/* Try Sv57x4 G-stage mode */
csr_write(CSR_HGATP, HGATP_MODE_SV57X4 << HGATP_MODE_SHIFT);
if ((csr_read(CSR_HGATP) >> HGATP_MODE_SHIFT) == HGATP_MODE_SV57X4) {
- kvm_riscv_gstage_mode = HGATP_MODE_SV57X4;
- kvm_riscv_gstage_pgd_levels = 5;
+ kvm_riscv_gstage_max_pgd_levels = 5;
goto done;
}
/* Try Sv48x4 G-stage mode */
csr_write(CSR_HGATP, HGATP_MODE_SV48X4 << HGATP_MODE_SHIFT);
if ((csr_read(CSR_HGATP) >> HGATP_MODE_SHIFT) == HGATP_MODE_SV48X4) {
- kvm_riscv_gstage_mode = HGATP_MODE_SV48X4;
- kvm_riscv_gstage_pgd_levels = 4;
+ kvm_riscv_gstage_max_pgd_levels = 4;
goto done;
}
/* Try Sv39x4 G-stage mode */
csr_write(CSR_HGATP, HGATP_MODE_SV39X4 << HGATP_MODE_SHIFT);
if ((csr_read(CSR_HGATP) >> HGATP_MODE_SHIFT) == HGATP_MODE_SV39X4) {
- kvm_riscv_gstage_mode = HGATP_MODE_SV39X4;
- kvm_riscv_gstage_pgd_levels = 3;
+ kvm_riscv_gstage_max_pgd_levels = 3;
goto done;
}
#else /* CONFIG_32BIT */
/* Try Sv32x4 G-stage mode */
csr_write(CSR_HGATP, HGATP_MODE_SV32X4 << HGATP_MODE_SHIFT);
if ((csr_read(CSR_HGATP) >> HGATP_MODE_SHIFT) == HGATP_MODE_SV32X4) {
- kvm_riscv_gstage_mode = HGATP_MODE_SV32X4;
- kvm_riscv_gstage_pgd_levels = 2;
+ kvm_riscv_gstage_max_pgd_levels = 2;
goto done;
}
#endif
/* KVM depends on !HGATP_MODE_OFF */
- kvm_riscv_gstage_mode = HGATP_MODE_OFF;
- kvm_riscv_gstage_pgd_levels = 0;
+ kvm_riscv_gstage_max_pgd_levels = 0;
done:
csr_write(CSR_HGATP, 0);
diff --git a/arch/riscv/kvm/main.c b/arch/riscv/kvm/main.c
index 0f3fe3986fc0..90ee0a032b9a 100644
--- a/arch/riscv/kvm/main.c
+++ b/arch/riscv/kvm/main.c
@@ -105,17 +105,17 @@ static int __init riscv_kvm_init(void)
return rc;
kvm_riscv_gstage_mode_detect();
- switch (kvm_riscv_gstage_mode) {
- case HGATP_MODE_SV32X4:
+ switch (kvm_riscv_gstage_max_pgd_levels) {
+ case 2:
str = "Sv32x4";
break;
- case HGATP_MODE_SV39X4:
+ case 3:
str = "Sv39x4";
break;
- case HGATP_MODE_SV48X4:
+ case 4:
str = "Sv48x4";
break;
- case HGATP_MODE_SV57X4:
+ case 5:
str = "Sv57x4";
break;
default:
@@ -164,7 +164,7 @@ static int __init riscv_kvm_init(void)
(rc) ? slist : "no features");
}
- kvm_info("using %s G-stage page table format\n", str);
+ kvm_info("highest G-stage page table mode is %s\n", str);
kvm_info("VMID %ld bits available\n", kvm_riscv_gstage_vmid_bits());
diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
index 088d33ba90ed..fbcdd75cb9af 100644
--- a/arch/riscv/kvm/mmu.c
+++ b/arch/riscv/kvm/mmu.c
@@ -67,7 +67,7 @@ int kvm_riscv_mmu_ioremap(struct kvm *kvm, gpa_t gpa, phys_addr_t hpa,
if (!writable)
map.pte = pte_wrprotect(map.pte);
- ret = kvm_mmu_topup_memory_cache(&pcache, kvm_riscv_gstage_pgd_levels);
+ ret = kvm_mmu_topup_memory_cache(&pcache, kvm->arch.pgd_levels);
if (ret)
goto out;
@@ -186,7 +186,7 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,
* space addressable by the KVM guest GPA space.
*/
if ((new->base_gfn + new->npages) >=
- (kvm_riscv_gstage_gpa_size >> PAGE_SHIFT))
+ kvm_riscv_gstage_gpa_size(kvm->arch.pgd_levels) >> PAGE_SHIFT)
return -EFAULT;
hva = new->userspace_addr;
@@ -472,7 +472,7 @@ int kvm_riscv_mmu_map(struct kvm_vcpu *vcpu, struct kvm_memory_slot *memslot,
memset(out_map, 0, sizeof(*out_map));
/* We need minimum second+third level pages */
- ret = kvm_mmu_topup_memory_cache(pcache, kvm_riscv_gstage_pgd_levels);
+ ret = kvm_mmu_topup_memory_cache(pcache, kvm->arch.pgd_levels);
if (ret) {
kvm_err("Failed to topup G-stage cache\n");
return ret;
@@ -575,6 +575,7 @@ int kvm_riscv_mmu_alloc_pgd(struct kvm *kvm)
return -ENOMEM;
kvm->arch.pgd = page_to_virt(pgd_page);
kvm->arch.pgd_phys = page_to_phys(pgd_page);
+ kvm->arch.pgd_levels = kvm_riscv_gstage_max_pgd_levels;
return 0;
}
@@ -590,10 +591,12 @@ void kvm_riscv_mmu_free_pgd(struct kvm *kvm)
gstage.flags = 0;
gstage.vmid = READ_ONCE(kvm->arch.vmid.vmid);
gstage.pgd = kvm->arch.pgd;
- kvm_riscv_gstage_unmap_range(&gstage, 0UL, kvm_riscv_gstage_gpa_size, false);
+ kvm_riscv_gstage_unmap_range(&gstage, 0UL,
+ kvm_riscv_gstage_gpa_size(kvm->arch.pgd_levels), false);
pgd = READ_ONCE(kvm->arch.pgd);
kvm->arch.pgd = NULL;
kvm->arch.pgd_phys = 0;
+ kvm->arch.pgd_levels = 0;
}
spin_unlock(&kvm->mmu_lock);
@@ -603,11 +606,12 @@ void kvm_riscv_mmu_free_pgd(struct kvm *kvm)
void kvm_riscv_mmu_update_hgatp(struct kvm_vcpu *vcpu)
{
- unsigned long hgatp = kvm_riscv_gstage_mode << HGATP_MODE_SHIFT;
- struct kvm_arch *k = &vcpu->kvm->arch;
+ struct kvm_arch *ka = &vcpu->kvm->arch;
+ unsigned long hgatp = kvm_riscv_gstage_mode(ka->pgd_levels)
+ << HGATP_MODE_SHIFT;
- hgatp |= (READ_ONCE(k->vmid.vmid) << HGATP_VMID_SHIFT) & HGATP_VMID;
- hgatp |= (k->pgd_phys >> PAGE_SHIFT) & HGATP_PPN;
+ hgatp |= (READ_ONCE(ka->vmid.vmid) << HGATP_VMID_SHIFT) & HGATP_VMID;
+ hgatp |= (ka->pgd_phys >> PAGE_SHIFT) & HGATP_PPN;
ncsr_write(CSR_HGATP, hgatp);
diff --git a/arch/riscv/kvm/vm.c b/arch/riscv/kvm/vm.c
index 13c63ae1a78b..fb7c4e07961f 100644
--- a/arch/riscv/kvm/vm.c
+++ b/arch/riscv/kvm/vm.c
@@ -199,7 +199,10 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
r = KVM_USER_MEM_SLOTS;
break;
case KVM_CAP_VM_GPA_BITS:
- r = kvm_riscv_gstage_gpa_bits;
+ if (!kvm)
+ r = kvm_riscv_gstage_gpa_bits(kvm_riscv_gstage_max_pgd_levels);
+ else
+ r = kvm_riscv_gstage_gpa_bits(kvm->arch.pgd_levels);
break;
default:
r = 0;
diff --git a/arch/riscv/kvm/vmid.c b/arch/riscv/kvm/vmid.c
index cf34d448289d..c15bdb1dd8be 100644
--- a/arch/riscv/kvm/vmid.c
+++ b/arch/riscv/kvm/vmid.c
@@ -26,7 +26,8 @@ static DEFINE_SPINLOCK(vmid_lock);
void __init kvm_riscv_gstage_vmid_detect(void)
{
/* Figure-out number of VMID bits in HW */
- csr_write(CSR_HGATP, (kvm_riscv_gstage_mode << HGATP_MODE_SHIFT) | HGATP_VMID);
+ csr_write(CSR_HGATP, (kvm_riscv_gstage_mode(kvm_riscv_gstage_max_pgd_levels) <<
+ HGATP_MODE_SHIFT) | HGATP_VMID);
vmid_bits = csr_read(CSR_HGATP);
vmid_bits = (vmid_bits & HGATP_VMID) >> HGATP_VMID_SHIFT;
vmid_bits = fls_long(vmid_bits);
--
2.50.1
^ permalink raw reply related
* Re: [PATCH v9 01/10] x86/bhi: x86/vmscape: Move LFENCE out of clear_bhb_loop()
From: Borislav Petkov @ 2026-04-03 15:16 UTC (permalink / raw)
To: Pawan Gupta
Cc: x86, Jon Kohler, Nikolay Borisov, H. Peter Anvin, Josh Poimboeuf,
David Kaplan, Sean Christopherson, Dave Hansen, Peter Zijlstra,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, KP Singh,
Jiri Olsa, David S. Miller, David Laight, Andy Lutomirski,
Thomas Gleixner, Ingo Molnar, David Ahern, Martin KaFai Lau,
Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend,
Stanislav Fomichev, Hao Luo, Paolo Bonzini, Jonathan Corbet,
linux-kernel, kvm, Asit Mallick, Tao Zhang, bpf, netdev,
linux-doc
In-Reply-To: <20260402-vmscape-bhb-v9-1-94d16bc29774@linux.intel.com>
On Thu, Apr 02, 2026 at 05:30:47PM -0700, Pawan Gupta wrote:
> Currently, the BHB clearing sequence is followed by an LFENCE to prevent
> transient execution of subsequent indirect branches prematurely. However,
> the LFENCE barrier could be unnecessary in certain cases. For example, when
> the kernel is using the BHI_DIS_S mitigation, and BHB clearing is only
> needed for userspace. In such cases, the LFENCE is redundant because ring
> transitions would provide the necessary serialization.
>
> Below is a quick recap of BHI mitigation options:
>
> On Alder Lake and newer
>
> BHI_DIS_S: Hardware control to mitigate BHI in ring0. This has low
> performance overhead.
>
> Long loop: Alternatively, a longer version of the BHB clearing sequence
> can be used to mitigate BHI. It can also be used to mitigate the BHI
> variant of VMSCAPE. This is not yet implemented in Linux.
>
> On older CPUs
>
> Short loop: Clears BHB at kernel entry and VMexit. The "Long loop" is
> effective on older CPUs as well, but should be avoided because of
> unnecessary overhead.
>
> On Alder Lake and newer CPUs, eIBRS isolates the indirect targets between
> guest and host. But when affected by the BHI variant of VMSCAPE, a guest's
> branch history may still influence indirect branches in userspace. This
> also means the big hammer IBPB could be replaced with a cheaper option that
> clears the BHB at exit-to-userspace after a VMexit.
>
> In preparation for adding the support for the BHB sequence (without LFENCE)
> on newer CPUs, move the LFENCE to the caller side after clear_bhb_loop() is
> executed. Allow callers to decide whether they need the LFENCE or not. This
> adds a few extra bytes to the call sites, but it obviates the need for
> multiple variants of clear_bhb_loop().
>
> Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
> Tested-by: Jon Kohler <jon@nutanix.com>
> Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> ---
> arch/x86/entry/entry_64.S | 5 ++++-
> arch/x86/include/asm/nospec-branch.h | 4 ++--
> arch/x86/net/bpf_jit_comp.c | 2 ++
> 3 files changed, 8 insertions(+), 3 deletions(-)
Acked-by: Borislav Petkov (AMD) <bp@alien8.de>
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH v9 07/10] x86/vmscape: Use static_call() for predictor flush
From: Sean Christopherson @ 2026-04-03 14:52 UTC (permalink / raw)
To: Pawan Gupta
Cc: x86, Jon Kohler, Nikolay Borisov, H. Peter Anvin, Josh Poimboeuf,
David Kaplan, Borislav Petkov, Dave Hansen, Peter Zijlstra,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, KP Singh,
Jiri Olsa, David S. Miller, David Laight, Andy Lutomirski,
Thomas Gleixner, Ingo Molnar, David Ahern, Martin KaFai Lau,
Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend,
Stanislav Fomichev, Hao Luo, Paolo Bonzini, Jonathan Corbet,
linux-kernel, kvm, Asit Mallick, Tao Zhang, bpf, netdev,
linux-doc
In-Reply-To: <20260402-vmscape-bhb-v9-7-94d16bc29774@linux.intel.com>
On Thu, Apr 02, 2026, Pawan Gupta wrote:
> Adding more mitigation options at exit-to-userspace for VMSCAPE would
> usually require a series of checks to decide which mitigation to use. In
> this case, the mitigation is done by calling a function, which is decided
> at boot. So, adding more feature flags and multiple checks can be avoided
> by using static_call() to the mitigating function.
>
> Replace the flag-based mitigation selector with a static_call(). This also
> frees the existing X86_FEATURE_IBPB_EXIT_TO_USER.
...
> @@ -3133,8 +3139,14 @@ static void __init vmscape_update_mitigation(void)
> static void __init vmscape_apply_mitigation(void)
> {
> if (vmscape_mitigation == VMSCAPE_MITIGATION_IBPB_EXIT_TO_USER)
> - setup_force_cpu_cap(X86_FEATURE_IBPB_EXIT_TO_USER);
> + static_call_update(vmscape_predictor_flush, write_ibpb);
> +}
> +
> +bool vmscape_mitigation_enabled(void)
> +{
> + return !!static_call_query(vmscape_predictor_flush);
> }
> +EXPORT_SYMBOL_FOR_KVM(vmscape_mitigation_enabled);
>
> #undef pr_fmt
> #define pr_fmt(fmt) fmt
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 45d7cfedc507..e204482e64f3 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -11463,7 +11463,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> * set for the CPU that actually ran the guest, and not the CPU that it
> * may migrate to.
> */
> - if (cpu_feature_enabled(X86_FEATURE_IBPB_EXIT_TO_USER))
> + if (vmscape_mitigation_enabled())
This is pretty lame. It turns a statically patched MOV
11548 if (cpu_feature_enabled(X86_FEATURE_IBPB_EXIT_TO_USER))
11549 this_cpu_write(x86_ibpb_exit_to_user, true);
0x000000000003c57a <+858>: movb $0x1,%gs:0x0(%rip) # 0x3c582 <vcpu_enter_guest+866>
into a function call and two sets of conditional branches. And with mitigations
enabled, that function call may trigger the wonderful unret insanity
11548 if (vmscape_mitigation_enabled())
0x000000000003c575 <+853>: call 0x3c57a <vcpu_enter_guest+858>
0x000000000003c57a <+858>: test %al,%al
0x000000000003c57c <+860>: je 0x3c586 <vcpu_enter_guest+870>
11549 this_cpu_write(x86_predictor_flush_exit_to_user, true);
0x000000000003c57e <+862>: movb $0x1,%gs:0x0(%rip) # 0x3c586 <vcpu_enter_guest+870>
3166 {
0xffffffff81285320 <+0>: endbr64
0xffffffff81285324 <+4>: call 0xffffffff812aa5a0 <__fentry__>
3167 return !!static_call_query(vmscape_predictor_flush);
0xffffffff81285329 <+9>: mov 0x13a4f30(%rip),%rax # 0xffffffff8262a260 <__SCK__vmscape_predictor_flush>
0xffffffff81285330 <+16>: test %rax,%rax
0xffffffff81285333 <+19>: setne %al
3168 }
0xffffffff81285336 <+22>: jmp 0xffffffff81db1e30 <__x86_return_thunk>
While this isn't KVM's super hot inner run loop, it's still very much a hot path.
Even more annoying, KVM will eat the function call on kernels with CPU_MITIGATIONS=n.
I'd like to at least do something like the below to make the common case of
multiple guest entry/exits more or less free, and to avoid the CALL+(UN)RET
overhead, but trying to include linux/static_call.h in processor.h (or any other
core x86 header) creates a cyclical dependency :-/
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 20ab4dd588c6..0dc0680a80f8 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -36,6 +36,7 @@ struct vm86;
#include <linux/err.h>
#include <linux/irqflags.h>
#include <linux/mem_encrypt.h>
+#include <linux/static_call.h>
/*
* We handle most unaligned accesses in hardware. On the other hand
@@ -753,7 +754,11 @@ enum mds_mitigations {
};
extern bool gds_ucode_mitigated(void);
-extern bool vmscape_mitigation_enabled(void);
+
+static inline bool vmscape_mitigation_enabled(void)
+{
+ return !!static_call_query(vmscape_predictor_flush);
+}
/*
* Make previous memory operations globally visible before
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 366ebe1e1fb9..02bf626f0773 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -148,6 +148,7 @@ DEFINE_STATIC_KEY_FALSE(switch_mm_cond_l1d_flush);
* sequence. This defaults to no mitigation.
*/
DEFINE_STATIC_CALL_NULL(vmscape_predictor_flush, write_ibpb);
+EXPORT_STATIC_CALL_GPL(vmscape_predictor_flush);
#undef pr_fmt
#define pr_fmt(fmt) "mitigations: " fmt
@@ -3162,12 +3163,6 @@ static void __init vmscape_apply_mitigation(void)
static_call_update(vmscape_predictor_flush, clear_bhb_loop_nofence);
}
-bool vmscape_mitigation_enabled(void)
-{
- return !!static_call_query(vmscape_predictor_flush);
-}
-EXPORT_SYMBOL_FOR_KVM(vmscape_mitigation_enabled);
-
#undef pr_fmt
#define pr_fmt(fmt) fmt
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index a1fbbab08291..117c60d00758 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -11545,7 +11545,9 @@ static noinline int vcpu_enter_guest(struct kvm_vcpu *vcpu)
* set for the CPU that actually ran the guest, and not the CPU that it
* may migrate to.
*/
- if (vmscape_mitigation_enabled())
+ if (IS_ENABLED(CONFIG_CPU_MITIGATIONS) &&
+ !this_cpu_read(x86_predictor_flush_exit_to_user) &&
+ vmscape_mitigation_enabled())
this_cpu_write(x86_predictor_flush_exit_to_user, true);
/*
^ permalink raw reply related
* Re: [PATCH RFC v4 10/44] KVM: guest_memfd: Add support for KVM_SET_MEMORY_ATTRIBUTES2
From: Ackerley Tng @ 2026-04-03 14:50 UTC (permalink / raw)
To: Michael Roth
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
ira.weiny, jmattson, jthoughton, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, tabba, willy,
wyihan, yan.y.zhao, forkloop, pratyush, suzuki.poulose,
aneesh.kumar, Paolo Bonzini, Sean Christopherson, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Jonathan Corbet, Shuah Khan, Shuah Khan, Vishal Annapurve,
Andrew Morton, Chris Li, Kairui Song, Kemeng Shi, Nhat Pham,
Baoquan He, Barry Song, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest, linux-mm
In-Reply-To: <CAEvNRgGm9icDK8sK5ZfqHEOEqSbvjwtihE4p9d3vpBq-NfVjmw@mail.gmail.com>
Ackerley Tng <ackerleytng@google.com> writes:
>
> [...snip...]
>
> guest_memfd's populate will first check that the memory is shared, then
> also set the memory to private after the populate.
>
> [...snip...]
>
Looking at this again, the above basically means that the entire
conversion process needs to be performed within populate.
In addition to setting the attributes in guest_memfd as private, for
consistency, populate will also have to do all the associated
operations, especially unmapping from the host, checking refcounts,
and the list of work in conversion will only increase in future with
direct map removal/restoration and huge page merging.
The complexity of conversion also means possible errors (EAGAIN for
elevated refcounts and ENOMEM when we're out of memory), and error
information like the offset where the elevated refcount was.
It doesn't look like there's room for this information to be plumbed out
through the platform-specific ioctls, and even if we make space, it
seems odd to have conversion-related error information returned through
the platform-specific call.
I agree with the goal of not having KVM touch private memory contents as
tracked by guest_memfd, so I'd like to propose that we distinguish:
1. private as tracked by KVM (guest_memfd/vm_memory_attributes)
2. private as tracked by trusted entity
Currently, in TDX's populate flow, KVM doesn't do any copying, it only
instructs TDX to do the copying.
"TDH.MEM.PAGE.ADD Operands Information Definition" in the TDX module
ABI spec says the source is accessed as shared, the destination is
accessed as private, and in the case that source and destination are
the same, the "In-Place Add" section says the source will be converted
to private.
In SNP's populate flow, SNP-specific code in KVM does the copying from
shared memory into the destination, then makes the destination address
private in the RMP table before telling the firmware to do the
in-place encryption.
I think we should think of the populate ioctls as being
platform-specific ioctls that, when called, accept
+ destination address: private (as tracked by guest_memfd)
+ source address: shared (as tracked by guest_memfd) or NULL
KVM doesn't touch private memory contents, even in this case, because
it's really a platform-specific ioctl that handles loading, and the
platform does expect the destination is private for both TDX and SNP
at the firmware boundary.
Since SNP (platform-specific) only allows in-place launch update, and
KVM had to provide an interface that allows a different source address
for support before in-place conversion, then SNP has to continue
supporting the to-be-deprecated path by doing the copying within
platform-specific code.
For consistency, guest_memfd can continue to check that it tracks the
destination address as private, and sev_gmem_populate will then hide
the copying code away just to support the legacy case.
The flow before in-place conversion is
1. Load memory (shared or non-guest_memfd memory)
2. KVM_SEV_SNP_LAUNCH_UPDATE or KVM_TDX_INIT_MEM_REGION (destination:
gfn for separate private memory, source: shared memory)
The proposed flow for in-place conversion is
1. INIT_SHARED or convert to shared
2. Load memory while it is shared
3. Convert to private (PRESERVE, or new flag?)
4. KVM_SEV_SNP_LAUNCH_UPDATE or KVM_TDX_INIT_MEM_REGION (destination:
gfn for converted private memory, source: NULL)
TLDR:
+ Think of populate ioctls not as KVM touching memory, but platform
handling population.
+ KVM code (kvm_gmem_populate) still doesn't touch memory contents
+ post_populate is platform-specific code that handles loading into
private destination memory just to support legacy non-in-place
conversion.
+ Don't complicate populate ioctls by doing conversion just to support
legacy use-cases where platform-specific code has to do copying on
the host.
>>>
>>> [...snip...]
>>>
^ 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