* Re: [PATCH RFC v5 01/53] KVM: guest_memfd: Introduce per-gmem attributes, use to guard user mappings
From: Ackerley Tng @ 2026-05-07 18:31 UTC (permalink / raw)
To: Liam R. Howlett
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
ira.weiny, jmattson, jthoughton, michael.roth, 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, Youngjun Park, Qi Zheng,
Shakeel Butt, Kiryl Shutsemau, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <byywwfin2aenobdwuesqihm6nzxyx6ecedwgbt7f5tvaaul6fi@u7bmexpavwdn>
"Liam R. Howlett" <liam@infradead.org> writes:
>
> [...snip...]
>
>> +static u64 kvm_gmem_get_attributes(struct inode *inode, pgoff_t index)
>> +{
>> + struct maple_tree *mt = &GMEM_I(inode)->attributes;
>> + void *entry = mtree_load(mt, index);
>> +
>> + /*
>> + * The lock _must_ be held for lookups, as some maple tree operations,
>> + * e.g. append, are unsafe (return inaccurate information) with respect
>> + * to concurrent RCU-protected lookups.
>> + */
>
> Can you please elaborate how you see inaccurate information and which
> information is inaccurate?
>
> Your comment is incorrect and misleading as append will not be used in
> rcu mode. Note that you have not set this tree up in rcu mode.
>
My bad. Thanks for clarifying about usage of rcu mode.
>> + lockdep_assert(mt_lock_is_held(mt));
>> +
In the next revision I'll remove this lockdep and use RCU mode, and
kvm_gmem_get_memory_attributes() should get a stable result.
The other lookups using mt_for_each() in kvm_gmem_range_has_attributes()
and kvm_gmem_get_invalidate_filter() retain the lockdep since those
operate over multiple ranges. Those are called from paths that require
holding the lock to exclude other operations anyway, so the lockdep
requirement does not cost anything more.
>> + return WARN_ON_ONCE(!entry) ? 0 : xa_to_value(entry);
>> +}
>> +
>>
>> [...snip...]
>>
^ permalink raw reply
* Re: [PATCH v17 10/11] PCI/CXL: Mask/Unmask CXL protocol errors
From: Jonathan Cameron @ 2026-05-07 18:29 UTC (permalink / raw)
To: Terry Bowman
Cc: dave, dave.jiang, alison.schofield, djbw, bhelgaas, shiju.jose,
ming.li, Smita.KoralahalliChannabasappa, rrichter, dan.carpenter,
PradeepVineshReddy.Kodamati, lukas, Benjamin.Cheatham,
sathyanarayanan.kuppuswamy, vishal.l.verma, alucerop, ira.weiny,
corbet, rafael, xueshuai, linux-cxl, linux-kernel, linux-pci,
linux-acpi, linux-doc
In-Reply-To: <20260505173029.2718246-11-terry.bowman@amd.com>
On Tue, 5 May 2026 12:30:28 -0500
Terry Bowman <terry.bowman@amd.com> wrote:
> CXL protocol errors are not enabled for all CXL devices after boot. They
> must be enabled in order to process CXL protocol errors. Provide matching
> teardown helpers so the masks are restored when a CXL Port or Downstream
> Port goes away.
>
> Add pci_aer_mask_internal_errors() as the symmetric counterpart to
> pci_aer_unmask_internal_errors() and export both for the cxl_core module.
>
> Introduce cxl_unmask_proto_interrupts() and cxl_mask_proto_interrupts()
> in cxl_core to wrap the PCI helpers with the dev_is_pci() and
> pcie_aer_is_native() gating CXL needs. Both helpers tolerate a NULL
> @dev so teardown callers do not have to special-case it.
>
> Wire cxl_unmask_proto_interrupts() into the success path of
> cxl_dport_map_ras() and devm_cxl_port_ras_setup() so the unmask only
> runs when the RAS register block was actually mapped. Pair each unmask
> with a devm_add_action_or_reset() registration of
> cxl_mask_proto_interrupts() scoped to the cxl_port device. The mask is
> then restored when the cxl_port device releases its devres. This
> applies to Endpoints, Upstream Switch Ports, Downstream Switch Ports,
> and Root Ports.
>
> Co-developed-by: Dan Williams <djbw@kernel.org>
> Signed-off-by: Dan Williams <djbw@kernel.org>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
I'd have definitely preferred if the ras code generally reported
errors on setup and then we decided whether to taken any notice
at the top of the stack, but that's existing code so not your problem.
Reviewed-by: Jonathan Cameron <jic23@kernel.org>
Subject to Dave's interesting suggestion...
^ permalink raw reply
* Re: [PATCH v17 01/11] PCI/AER: Introduce AER-CXL Kfifo
From: Bowman, Terry @ 2026-05-07 18:26 UTC (permalink / raw)
To: Jonathan Cameron
Cc: dave, dave.jiang, alison.schofield, djbw, bhelgaas, shiju.jose,
ming.li, Smita.KoralahalliChannabasappa, rrichter, dan.carpenter,
PradeepVineshReddy.Kodamati, lukas, Benjamin.Cheatham,
sathyanarayanan.kuppuswamy, vishal.l.verma, alucerop, ira.weiny,
corbet, rafael, xueshuai, linux-cxl, linux-kernel, linux-pci,
linux-acpi, linux-doc
In-Reply-To: <20260507185303.329cf964@jic23-huawei>
On 5/7/2026 12:53 PM, Jonathan Cameron wrote:
> On Tue, 5 May 2026 12:30:19 -0500
> Terry Bowman <terry.bowman@amd.com> wrote:
>
>> CXL virtual hierarchy (VH) native RAS handling for CXL Port devices will be
>> added soon. This requires a notification mechanism for the AER driver to
>> share the AER interrupt with the CXL driver. The CXL drivers use the
>> notification to handle and log the CXL RAS errors.
>>
>> Note, 'CXL protocol error' terminology refers to CXL VH and not CXL RCH
>> errors unless specifically noted going forward.
>>
>> Introduce a new file in the AER driver to handle the CXL protocol
>> errors: pci/pcie/aer_cxl_vh.c.
>>
>> Add a kfifo work queue to be used by the AER and CXL drivers. Multiple
>> AER IRQ worker threads can be running and enqueueing concurrently, so
>> include write path synchronization. Pack the kfifo, the spinlock, the
>> rwsem, and the work pointer into a single structure. Initialize the
>> kfifo with INIT_KFIFO() from a subsys_initcall so its mask, esize and
>> data fields are valid before any producer or consumer runs.
>>
>> Add CXL work queue handler registration functions in the AER driver.
>> Export them so the CXL driver can assign or clear the work handler.
>>
>> Introduce 'struct cxl_proto_err_work_data' to serve as the kfifo work
>> data. It contains a reference to the PCI error source device and the
>> error severity. The cxl_core driver uses this when dequeuing the work.
>>
>> Introduce cxl_forward_error() to add a given CXL protocol error to a
>> work structure and push it onto the AER-CXL kfifo. This function takes
>> a pci_dev_get() on the source device. The kfifo consumer is responsible
>> for the matching pci_dev_put() after dequeue. On enqueue failure
>> cxl_forward_error() does the put itself.
>>
>> Synchronize accesses to the work function pointer during registration,
>> deregistration, enqueue, and dequeue.
>>
>> handle_error_source() is intentionally not changed here. The is_cxl_error()
>> switch that routes errors to cxl_forward_error() is added in a later patch
>> together with the kfifo consumer registration. This way the producer and
>> consumer land in the same commit, so CXL errors are not silently dropped
>> during bisect.
>>
>> Also add MAINTAINERS entries for both drivers/pci/pcie/aer_cxl_vh.c
>> (new in this patch) and drivers/pci/pcie/aer_cxl_rch.c (already in tree
>> but previously unlisted) under the existing CXL entry. This way the CXL
>> maintainers are CC'd on changes to the AER-CXL bridging code.
>>
>> Co-developed-by: Dan Williams <djbw@kernel.org>
>> Signed-off-by: Dan Williams <djbw@kernel.org>
>> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
>
> Sashiko did have one comment on what happens if there are multiple things
> in the kfifo and fn fails. At that point I think we are in the all
> bets are off corner and stranding a driver is fine, but open to other opinions!
>
> https://sashiko.dev/#/patchset/20260505173029.2718246-1-terry.bowman%40amd.com
>
> So with that in mind
>
> Reviewed-by: Jonathan Cameron <jic23@kernel.org>
>
Hi Jonathan,
I resolved this for next series by changing __cxl_proto_err_work_fn() to return void
as the error case was unnecessary and only added complexity.
>> diff --git a/drivers/pci/pcie/aer_cxl_vh.c b/drivers/pci/pcie/aer_cxl_vh.c
>> new file mode 100644
>> index 000000000000..c0fea2c2b9bc
>> --- /dev/null
>> +++ b/drivers/pci/pcie/aer_cxl_vh.c
>
>
>> +int for_each_cxl_proto_err(struct cxl_proto_err_work_data *wd,
>> + cxl_proto_err_fn_t fn)
>> +{
>> + int rc;
>> +
>> + guard(rwsem_read)(&cxl_proto_err_kfifo.rwsem);
>> + while (kfifo_get(&cxl_proto_err_kfifo.fifo, wd)) {
>> + rc = fn(wd);
>> + pci_dev_put(wd->pdev);
>> + if (rc)
>> + return rc;
> This is where Sashiko complains. Specifically:
> "If the consumer callback fn() returns an error, does this early return
> strand the remaining items in the kfifo?
> Because cxl_forward_error() takes a pci_dev reference for each enqueued
> item, it looks like these stranded items might leak their pci_dev references
> and prevent clean unbinding or hot-unplug until a new error triggers the
> queue again."
>
> I'd go with indeed it does, but there is no right thing to do here. I guess
> we could flush the kfifo and call pci_dev_put() on each of them, but that's horrible.
> Would basically mean calling the same stuff you have for cancelling outstanding
> entrees on exit().
>
>
Yes, that is an idea. But, until error discriminator is needed this can return void.
Clearing will be necessary but I think that will fit within the call path.
-Terry
>> + }
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_FOR_MODULES(for_each_cxl_proto_err, "cxl_core");
^ permalink raw reply
* Re: [PATCH v17 09/11] cxl: Update Endpoint AER uncorrectable handler
From: Jonathan Cameron @ 2026-05-07 18:25 UTC (permalink / raw)
To: Dave Jiang
Cc: Terry Bowman, dave, alison.schofield, djbw, bhelgaas, shiju.jose,
ming.li, Smita.KoralahalliChannabasappa, rrichter, dan.carpenter,
PradeepVineshReddy.Kodamati, lukas, Benjamin.Cheatham,
sathyanarayanan.kuppuswamy, vishal.l.verma, alucerop, ira.weiny,
corbet, rafael, xueshuai, linux-cxl, linux-kernel, linux-pci,
linux-acpi, linux-doc
In-Reply-To: <0cda6505-a217-4c75-b3cb-51a8c396793c@intel.com>
On Wed, 6 May 2026 10:43:02 -0700
Dave Jiang <dave.jiang@intel.com> wrote:
> On 5/5/26 10:30 AM, Terry Bowman wrote:
> > The CXL cxl_core driver now implements protocol RAS support. PCI
> > uncorrectable (UCE) protocol errors, however, continue to be reported via
> > the AER capability and must still be handled by a PCI error recovery callback.
> > UCE handling is required to provide direction for recovery.
> >
> > Replace the existing cxl_error_detected() callback in cxl/pci.c with a new
> > cxl_pci_error_detected() implementation that handles uncorrectable AER PCI
> > protocol errors.
> >
> > The handler decides solely based on the pci_channel_state_t parameter and
> > does not access PCIe AER capability registers from .error_detected, matching
> > the pattern used by other drivers including the NVMe and ixgbe drivers.
> > CXL.cachemem-corrupting protocol errors are routed separately through the
> > AER-CXL kfifo to cxl_handle_proto_error(), so cxl_pci does not need to
> > second-guess the AER core's classification.
> >
> > claude-opus-4.7 was used for research on PCI error state transitions and
> > requirements.
> >
> > Assisted-by: Claude:claude-opus-4.7
> > Signed-off-by: Terry Bowman <terry.bowman@amd.com>
...
> > ---
> > drivers/cxl/core/ras.c | 43 ++++++++++++++++--------------------------
> > drivers/cxl/cxlpci.h | 8 ++++----
> > drivers/cxl/pci.c | 6 +++---
> > 3 files changed, 23 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
> > index 5cc4087c2807..a98ce0f412ad 100644
> > --- a/drivers/cxl/core/ras.c
> > +++ b/drivers/cxl/core/ras.c
> > @@ -253,38 +253,27 @@ bool cxl_handle_ras(struct device *dev, u64 serial, void __iomem *ras_base)
> > return true;
> > }
> >
> > -pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
> > - pci_channel_state_t state)
> > +pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
> > + pci_channel_state_t state)
> > {
> > - struct cxl_dev_state *cxlds = pci_get_drvdata(pdev);
> > - struct cxl_memdev *cxlmd = cxlds->cxlmd;
> > - struct device *dev = &cxlmd->dev;
> > - bool ue;
> > + struct cxl_dport *dport;
> > + struct cxl_port *port __free(put_cxl_port) =
> > + find_cxl_port_by_dev(&pdev->dev, &dport);
>
> Move this to right before 'port' is being checked. It's ok to do inline var declaration with __free().
With that done LGTM
Reviewed-by: Jonathan Cameron <jic23@kernel.org>
^ permalink raw reply
* Re: [PATCH v17 06/11] PCI: Establish common CXL Port protocol error flow
From: Jonathan Cameron @ 2026-05-07 18:22 UTC (permalink / raw)
To: Terry Bowman
Cc: dave, dave.jiang, alison.schofield, djbw, bhelgaas, shiju.jose,
ming.li, Smita.KoralahalliChannabasappa, rrichter, dan.carpenter,
PradeepVineshReddy.Kodamati, lukas, Benjamin.Cheatham,
sathyanarayanan.kuppuswamy, vishal.l.verma, alucerop, ira.weiny,
corbet, rafael, xueshuai, linux-cxl, linux-kernel, linux-pci,
linux-acpi, linux-doc
In-Reply-To: <20260505173029.2718246-7-terry.bowman@amd.com>
On Tue, 5 May 2026 12:30:24 -0500
Terry Bowman <terry.bowman@amd.com> wrote:
> Add CXL Port protocol error handling callbacks to unify detection,
> logging, and recovery across CXL Ports and Endpoints. Establish a
> common flow for correctable and uncorrectable CXL protocol errors.
> RCH Downstream Port error handling is added in a following patch.
>
> Add cxl_handle_proto_error() to dispatch correctable and uncorrectable
> errors through the CXL RAS helpers. Add cxl_do_recovery() to coordinate
> uncorrectable recovery. Panic via panic() on any uncorrectable CXL RAS
> error. CXL.cachemem traffic cannot be safely recovered from an
> uncorrectable protocol error in software, so panic regardless of the
> AER severity reported. Gate error handling on the port driver being
> bound to avoid processing errors on disabled devices.
>
> Panic explicitly on pci_dev_is_disconnected() before accessing the RAS
> registers. A CXL device disconnecting during an uncorrectable error event
> is itself unrecoverable, particularly for devices in interleaved HDM
> regions. Relying on the status readl() returning ~0u to trip the existing
> panic path leaves the cause ambiguous.
>
> The panic policy applies to the RAS register block of the device whose
> error triggered the recovery: Root/Downstream Port RAS for VH Ports,
> Endpoint Port RAS for VH Endpoints and RCDs. Upstream RCH Downstream
> Port RAS UEs handled via cxl_handle_rdport_errors() are logged only, as
> before this series. Only the RCD Endpoint's own RAS UE drives the panic.
>
> Add to_ras_base() to centralize the RAS base lookup. It selects
> dport->regs.ras for Root/Downstream Ports and port->regs.ras for
> Upstream Ports and Endpoints.
>
> Export pcie_clear_device_status() and pci_aer_clear_fatal_status() so
> cxl_core can clear PCIe/AER state during recovery.
>
> Wire the AER core to the kfifo in this commit by adding the
> is_cxl_error() switch in handle_error_source() alongside the consumer
> registration. This way the producer and consumer go live in the same
> commit, so CXL errors are not silently dropped during bisect.
>
> The correctable AER status is cleared by the producer in
> cxl_forward_error().
>
> Co-developed-by: Dan Williams <djbw@kernel.org>
> Signed-off-by: Dan Williams <djbw@kernel.org>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
>
A few trivial things inline. With those tidied up
Reviewed-by: Jonathan Cameron <jic23@kernel.org>
> + * find_cxl_port_by_dev - Use @dev as hint to do a _by_dport or _by_uport lookup
> + * @dev: generic device that may either be a companion of port or target dport
> + * @dport: output parameter; set to the matched dport for dport-class
> + * lookups (Root Port, Downstream Port), NULL otherwise.
> + *
> + * Return a 'struct cxl_port' with an elevated reference if found. Use
> + * __free(put_cxl_port) to release.
> + */
> +static struct cxl_port *find_cxl_port_by_dev(struct device *dev, struct cxl_dport **dport)
> +{
> + struct pci_dev *pdev;
> +
> + *dport = NULL;
> + if (!dev_is_pci(dev))
> + return NULL;
> +
> + pdev = to_pci_dev(dev);
Only used once. So little point in this step...
> +
> + switch (pci_pcie_type(pdev)) {
switch (pci_pcie_type(to_pci_dev(dev))) {
looks readable enough to me.
> + case PCI_EXP_TYPE_ROOT_PORT:
> + case PCI_EXP_TYPE_DOWNSTREAM:
> + return find_cxl_port_by_dport(dev, dport);
> + case PCI_EXP_TYPE_UPSTREAM:
> + case PCI_EXP_TYPE_ENDPOINT:
> + case PCI_EXP_TYPE_RC_END:
> + return find_cxl_port_by_uport(dev);
> + }
> +
> + return NULL;
> +}
> +
> +static void cxl_do_recovery(struct pci_dev *pdev, struct cxl_port *port, struct cxl_dport *dport)
> +{
> + struct device *dev = &pdev->dev;
> + bool ue;
> +
> + if (pci_dev_is_disconnected(pdev))
> + panic("CXL cachemem error: device disconnected during UE recovery");
> +
> + ue = cxl_handle_ras(dev, pci_get_dsn(pdev),
> + to_ras_base(port, dport));
My lazy (or maybe busy) nature means I haven't checked, but if this remains
the same for rest of series it fits on one line of around 78 chars.
> + if (ue)
> + panic("CXL cachemem error.");
> +
> + pcie_clear_device_status(pdev);
> + pci_aer_clear_nonfatal_status(pdev);
> + pci_aer_clear_fatal_status(pdev);
> +}
> +int cxl_ras_init(void)
> +{
> + cxl_cper_register_prot_err_work(&cxl_cper_prot_err_work);
> + cxl_register_proto_err_work(&cxl_proto_err_work);
> +
> + return 0;
void cxl_ras_init() as per earlier suggestion still looks good ;)
> +}
> +
> +void cxl_ras_exit(void)
> +{
> + cxl_cper_unregister_prot_err_work();
> + cxl_unregister_proto_err_work();
> +}
^ permalink raw reply
* Re: [PATCH v17 05/11] cxl: Limit CXL-CPER kfifo registration functions scope
From: Jonathan Cameron @ 2026-05-07 18:14 UTC (permalink / raw)
To: Terry Bowman
Cc: dave, dave.jiang, alison.schofield, djbw, bhelgaas, shiju.jose,
ming.li, Smita.KoralahalliChannabasappa, rrichter, dan.carpenter,
PradeepVineshReddy.Kodamati, lukas, Benjamin.Cheatham,
sathyanarayanan.kuppuswamy, vishal.l.verma, alucerop, ira.weiny,
corbet, rafael, xueshuai, linux-cxl, linux-kernel, linux-pci,
linux-acpi, linux-doc
In-Reply-To: <20260505173029.2718246-6-terry.bowman@amd.com>
On Tue, 5 May 2026 12:30:23 -0500
Terry Bowman <terry.bowman@amd.com> wrote:
> From: Dan Williams <djbw@kernel.org>
>
> Some CPER functions used by CXL drivers are exported using the
> EXPORT_SYMBOL_NS_GPL(fn, ns) macro. This doesn't provide compile time
> enforcement or visibility of the consumers.
>
> This can be improved by using EXPORT_SYMBOL_FOR_MODULES() instead.
> EXPORT_SYMBOL_FOR_MODULES() explicitly names the modules that can access
> the function. This provides more precise control and visibility of symbol
> exposure than the namespace macro. It also provides compile time checking.
>
> To improve control and clarity, update cxl_cper_register_prot_err_work(),
> cxl_cper_unregister_prot_err_work(), and cxl_cper_prot_err_kfifo_get()
> to use EXPORT_SYMBOL_FOR_MODULES(). Also, update the register and unregister
> functions to return void type.
>
> Update the CPER kfifo unregister to cancel work while using
> synchronization.
>
> Co-developed-by: Terry Bowman <terry.bowman@amd.com>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Signed-off-by: Dan Williams <djbw@kernel.org>
One suggestion on simplifying the code by removing any assumption that
cxl_ras_init() can fail.
> diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
> index 56611da8357a..9193dac4e507 100644
> --- a/drivers/cxl/core/ras.c
> +++ b/drivers/cxl/core/ras.c
> @@ -68,13 +68,13 @@ static DECLARE_WORK(cxl_cper_prot_err_work, cxl_cper_prot_err_work_fn);
>
> int cxl_ras_init(void)
> {
> - return cxl_cper_register_prot_err_work(&cxl_cper_prot_err_work);
> + cxl_cper_register_prot_err_work(&cxl_cper_prot_err_work);
> + return 0;
I'm lazy and haven't read on, but if this never gains a failure
path can we change the signature whilst we are here to stop
pretending it does?
> }
^ permalink raw reply
* Re: [PATCH v17 04/11] cxl: Rename find_cxl_port() to find_cxl_port_by_dport()
From: Jonathan Cameron @ 2026-05-07 18:11 UTC (permalink / raw)
To: Dave Jiang
Cc: Terry Bowman, dave, alison.schofield, djbw, bhelgaas, ming.li,
Smita.KoralahalliChannabasappa, rrichter,
PradeepVineshReddy.Kodamati, lukas, Benjamin.Cheatham,
sathyanarayanan.kuppuswamy, vishal.l.verma, alucerop, ira.weiny,
corbet, rafael, xueshuai, linux-cxl, linux-kernel, linux-pci,
linux-acpi, linux-doc
In-Reply-To: <c5d40fc5-e8e0-4fa4-94af-4ea08c2f370b@intel.com>
On Tue, 5 May 2026 15:06:42 -0700
Dave Jiang <dave.jiang@intel.com> wrote:
> On 5/5/26 10:30 AM, Terry Bowman wrote:
> > From: Dan Williams <djbw@kernel.org>
> >
> > find_cxl_port() and find_cxl_port_by_uport() are internal port lookup
> > functions that search the CXL bus by dport and uport respectively, but
> > their names do not make the lookup method clear.
> >
> > Rename find_cxl_port() to find_cxl_port_by_dport() to make the lookup
> > method explicit and consistent with find_cxl_port_by_uport(). Both
> > functions remain static to port.c; the upcoming patch that adds the
> > first cross-file caller will widen their scope.
> >
> > Co-developed-by: Terry Bowman <terry.bowman@amd.com>
> > Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> > Signed-off-by: Dan Williams <djbw@kernel.org>
>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Jonathan Cameron <jic23@kernel.org>
Could pick this one up independently / early given it's useful for
readability.
^ permalink raw reply
* Re: [PATCH v17 02/11] cxl/ras: Unify Endpoint and Port AER trace events
From: Jonathan Cameron @ 2026-05-07 18:08 UTC (permalink / raw)
To: Terry Bowman
Cc: dave, dave.jiang, alison.schofield, djbw, bhelgaas, shiju.jose,
ming.li, Smita.KoralahalliChannabasappa, rrichter, dan.carpenter,
PradeepVineshReddy.Kodamati, lukas, Benjamin.Cheatham,
sathyanarayanan.kuppuswamy, vishal.l.verma, alucerop, ira.weiny,
corbet, rafael, xueshuai, linux-cxl, linux-kernel, linux-pci,
linux-acpi, linux-doc, Mauro Carvalho Chehab
In-Reply-To: <20260505173029.2718246-3-terry.bowman@amd.com>
On Tue, 5 May 2026 12:30:20 -0500
Terry Bowman <terry.bowman@amd.com> wrote:
> From: Dan Williams <djbw@kernel.org>
+CC Mauro - rasdaemon related - see below.
>
> CXL protocol error logging uses two parallel sets of trace events. The
> cxl_port_aer_correctable_error() and cxl_port_aer_uncorrectable_error()
> events are used by CPER for CXL Port devices. The cxl_aer_correctable_error()
> and cxl_aer_uncorrectable_error() events are used for CXL Endpoints. Update
> the trace routines to use the latter for all CXL devices on both the CPER
> and native AER paths.
>
> Generalize cxl_aer_correctable_error()/cxl_aer_uncorrectable_error to
> take a struct device * and a u64 serial argument supplied by the caller.
> cxl_handle_ras() and cxl_handle_cor_ras() gain the new u64 serial parameter,
> sourced from pci_get_dsn().
>
> The CPER path keeps its existing Port-vs-Endpoint dispatch and passes the
> new arguments to the unified trace events. The CPER path will be folded
> together in a following patch.
>
> Remove the now-unused cxl_port_aer_correctable_error() and
> cxl_port_aer_uncorrectable_error().
>
> **WARNING: ABI BREAK**
> Rename the trace event field "memdev" to "device" so all CXL device types
> (Ports and Endpoints) can be reported under a common field name. Note this
> is an ABI break for userspace tools that key off the old "memdev" field.
> Specifically, rasdaemon's ras-cxl-handler.c looks up "memdev" and bails on
> NULL, so an unmodified rasdaemon will drop every CXL CE/UCE event once this
> kernel ships. A rasdaemon update is needed in a separate series.
>
> The need for the field rename was discussed in v16 review [1].
This concerns me (sorry I wasn't paying attention to the v16 thread).
It is a userspace regression against code that is out in the wild and typically
not updated in sync with the kernel.
If you are suggesting breaking ras-daemon at the very least +CC the maintainer.
To get to a unified tracepoint add a new one that does what you want, but
maintain the existing ones as well. Userspace can then migrate and maybe
in 5+ years time we can delete the non unified ones.
No actually comments on the code, just left it all here for Mauro,
Thanks,
Jonathan
>
> Also, for CXL Upstream Switch Port (USP) and Endpoint (EP) fatal UCE,
> the cxl_aer_uncorrectable_error trace event is not emitted. The AER core
> only retrieves PCI_ERR_UNCOR_STATUS for Root Ports, RCECs, and Downstream
> Ports, or for non-fatal severities. PCI config reads to the source device
> are expected to fail otherwise, so the AER core never reads the status
> word, is_cxl_error() does not classify the event as CXL, and the AER path
> handles it instead. In this case the AER handler consumes the event and
> logs it as an AER error without calling the CXL RAS handlers or trace
> logging.
>
> Before this patch, Endpoint and Port devices emitted different events:
>
> # Endpoint (cxl_aer_*):
> cxl_aer_correctable_error: memdev=mem0 host=0000:0c:00.0 serial=0: status: 'CRC Threshold Hit'
> cxl_aer_uncorrectable_error: memdev=mem0 host=0000:0c:00.0 serial=0: status: 'Cache Data ECC Error | Memory Data ECC Error' first_error: 'Cache Data ECC Error'
>
> # Port (cxl_port_aer_*, no serial field):
> cxl_port_aer_correctable_error: device=0000:0c:00.0 host=pci0000:0c status='CRC Threshold Hit'
> cxl_port_aer_uncorrectable_error: device=0000:0c:00.0 host=pci0000:0c status: 'Cache Data ECC Error | Memory Data ECC Error' first_error: 'Cache Data ECC Error'
>
> After this patch, all CXL devices emit the unified cxl_aer_* events
> with the same field layout:
>
> cxl_aer_correctable_error: device=0000:0c:00.0 host=pci0000:0c serial=0 status: 'CRC Threshold Hit'
> cxl_aer_uncorrectable_error: device=0000:0c:00.0 host=pci0000:0c serial=0 status: 'Cache Data ECC Error | Memory Data ECC Error' first_error: 'Cache Data ECC Error'
>
> [1] https://lore.kernel.org/linux-cxl/69cb2d5ba3111_178904100b7@dwillia2-mobl4.notmuch/
>
> Co-developed-by: Terry Bowman <terry.bowman@amd.com>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Signed-off-by: Dan Williams <djbw@kernel.org>
>
> ---
>
> Changes in v16->v17:
> - Replace cxlds->serial with pci_get_dsn()
> - Change 'memdev' to 'device' (Dan)
> - Updated Commit message
>
> Changes in v15->v16:
> - Add Dan's review-by
> - Incorporate Dan's comment into commit message:
> "Add the serial number at the end to preserve compatibility with
> libtraceevent parsing of the parameters."
>
> Changes in v14->v15:
> - Update commit message.
> - Moved cxl_handle_ras/cxl_handle_cor_ras() changes to future patch (terry)
>
> Changes in v13->v14:
> - Update commit headline (Bjorn)
>
> Changes in v12->v13:
> - Added Dave Jiang's review-by
>
> Changes in v11 -> v12:
> - Correct parameters to call trace_cxl_aer_correctable_error()
> - Add reviewed-by for Jonathan and Shiju
>
> Changes in v10->v11:
> - Updated CE and UCE trace routines to maintain consistent TP_Struct ABI
> and unchanged TP_printk() logging.
> ---
> drivers/cxl/core/core.h | 11 ++++--
> drivers/cxl/core/ras.c | 39 +++++++++++--------
> drivers/cxl/core/ras_rch.c | 6 ++-
> drivers/cxl/core/trace.h | 76 ++++++++------------------------------
> 4 files changed, 49 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index 82ca3a476708..132ac9c1ebf4 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -183,8 +183,9 @@ static inline struct device *dport_to_host(struct cxl_dport *dport)
> #ifdef CONFIG_CXL_RAS
> int cxl_ras_init(void);
> void cxl_ras_exit(void);
> -bool cxl_handle_ras(struct device *dev, void __iomem *ras_base);
> -void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base);
> +bool cxl_handle_ras(struct device *dev, u64 serial, void __iomem *ras_base);
> +void cxl_handle_cor_ras(struct device *dev, u64 serial,
> + void __iomem *ras_base);
> void cxl_dport_map_rch_aer(struct cxl_dport *dport);
> void cxl_disable_rch_root_ints(struct cxl_dport *dport);
> void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds);
> @@ -195,11 +196,13 @@ static inline int cxl_ras_init(void)
> return 0;
> }
> static inline void cxl_ras_exit(void) { }
> -static inline bool cxl_handle_ras(struct device *dev, void __iomem *ras_base)
> +static inline bool cxl_handle_ras(struct device *dev, u64 serial,
> + void __iomem *ras_base)
> {
> return false;
> }
> -static inline void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base) { }
> +static inline void cxl_handle_cor_ras(struct device *dev, u64 serial,
> + void __iomem *ras_base) { }
> static inline void cxl_dport_map_rch_aer(struct cxl_dport *dport) { }
> static inline void cxl_disable_rch_root_ints(struct cxl_dport *dport) { }
> static inline void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds) { }
> diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
> index 006c6ffc2f56..d7081caaf5d3 100644
> --- a/drivers/cxl/core/ras.c
> +++ b/drivers/cxl/core/ras.c
> @@ -13,7 +13,7 @@ static void cxl_cper_trace_corr_port_prot_err(struct pci_dev *pdev,
> {
> u32 status = ras_cap.cor_status & ~ras_cap.cor_mask;
>
> - trace_cxl_port_aer_correctable_error(&pdev->dev, status);
> + trace_cxl_aer_correctable_error(&pdev->dev, status, pci_get_dsn(pdev));
> }
>
> static void cxl_cper_trace_uncorr_port_prot_err(struct pci_dev *pdev,
> @@ -28,20 +28,24 @@ static void cxl_cper_trace_uncorr_port_prot_err(struct pci_dev *pdev,
> else
> fe = status;
>
> - trace_cxl_port_aer_uncorrectable_error(&pdev->dev, status, fe,
> - ras_cap.header_log);
> + trace_cxl_aer_uncorrectable_error(&pdev->dev, status, fe,
> + ras_cap.header_log,
> + pci_get_dsn(pdev));
> }
>
> -static void cxl_cper_trace_corr_prot_err(struct cxl_memdev *cxlmd,
> +static void cxl_cper_trace_corr_prot_err(struct pci_dev *pdev,
> + struct cxl_memdev *cxlmd,
> struct cxl_ras_capability_regs ras_cap)
> {
> u32 status = ras_cap.cor_status & ~ras_cap.cor_mask;
>
> - trace_cxl_aer_correctable_error(cxlmd, status);
> + trace_cxl_aer_correctable_error(&cxlmd->dev, status,
> + pci_get_dsn(pdev));
> }
>
> static void
> -cxl_cper_trace_uncorr_prot_err(struct cxl_memdev *cxlmd,
> +cxl_cper_trace_uncorr_prot_err(struct pci_dev *pdev,
> + struct cxl_memdev *cxlmd,
> struct cxl_ras_capability_regs ras_cap)
> {
> u32 status = ras_cap.uncor_status & ~ras_cap.uncor_mask;
> @@ -53,8 +57,9 @@ cxl_cper_trace_uncorr_prot_err(struct cxl_memdev *cxlmd,
> else
> fe = status;
>
> - trace_cxl_aer_uncorrectable_error(cxlmd, status, fe,
> - ras_cap.header_log);
> + trace_cxl_aer_uncorrectable_error(&cxlmd->dev, status, fe,
> + ras_cap.header_log,
> + pci_get_dsn(pdev));
> }
>
> static int match_memdev_by_parent(struct device *dev, const void *uport)
> @@ -101,9 +106,9 @@ void cxl_cper_handle_prot_err(struct cxl_cper_prot_err_work_data *data)
>
> cxlmd = to_cxl_memdev(mem_dev);
> if (data->severity == AER_CORRECTABLE)
> - cxl_cper_trace_corr_prot_err(cxlmd, data->ras_cap);
> + cxl_cper_trace_corr_prot_err(pdev, cxlmd, data->ras_cap);
> else
> - cxl_cper_trace_uncorr_prot_err(cxlmd, data->ras_cap);
> + cxl_cper_trace_uncorr_prot_err(pdev, cxlmd, data->ras_cap);
> }
> EXPORT_SYMBOL_GPL(cxl_cper_handle_prot_err);
>
> @@ -183,7 +188,7 @@ void devm_cxl_port_ras_setup(struct cxl_port *port)
> }
> EXPORT_SYMBOL_NS_GPL(devm_cxl_port_ras_setup, "CXL");
>
> -void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base)
> +void cxl_handle_cor_ras(struct device *dev, u64 serial, void __iomem *ras_base)
> {
> void __iomem *addr;
> u32 status;
> @@ -195,7 +200,7 @@ void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base)
> status = readl(addr);
> if (status & CXL_RAS_CORRECTABLE_STATUS_MASK) {
> writel(status & CXL_RAS_CORRECTABLE_STATUS_MASK, addr);
> - trace_cxl_aer_correctable_error(to_cxl_memdev(dev), status);
> + trace_cxl_aer_correctable_error(dev, status, serial);
> }
> }
>
> @@ -220,7 +225,7 @@ static void header_log_copy(void __iomem *ras_base, u32 *log)
> * Log the state of the RAS status registers and prepare them to log the
> * next error status. Return 1 if reset needed.
> */
> -bool cxl_handle_ras(struct device *dev, void __iomem *ras_base)
> +bool cxl_handle_ras(struct device *dev, u64 serial, void __iomem *ras_base)
> {
> u32 hl[CXL_HEADERLOG_SIZE_U32];
> void __iomem *addr;
> @@ -247,7 +252,7 @@ bool cxl_handle_ras(struct device *dev, void __iomem *ras_base)
> }
>
> header_log_copy(ras_base, hl);
> - trace_cxl_aer_uncorrectable_error(to_cxl_memdev(dev), status, fe, hl);
> + trace_cxl_aer_uncorrectable_error(dev, status, fe, hl, serial);
> writel(status & CXL_RAS_UNCORRECTABLE_STATUS_MASK, addr);
>
> return true;
> @@ -270,7 +275,8 @@ void cxl_cor_error_detected(struct pci_dev *pdev)
> if (cxlds->rcd)
> cxl_handle_rdport_errors(cxlds);
>
> - cxl_handle_cor_ras(&cxlds->cxlmd->dev, cxlmd->endpoint->regs.ras);
> + cxl_handle_cor_ras(&cxlds->cxlmd->dev, pci_get_dsn(pdev),
> + cxlmd->endpoint->regs.ras);
> }
> }
> EXPORT_SYMBOL_NS_GPL(cxl_cor_error_detected, "CXL");
> @@ -299,7 +305,8 @@ pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
> * chance the situation is recoverable dump the status of the RAS
> * capability registers and bounce the active state of the memdev.
> */
> - ue = cxl_handle_ras(&cxlds->cxlmd->dev, cxlmd->endpoint->regs.ras);
> + ue = cxl_handle_ras(&cxlds->cxlmd->dev, pci_get_dsn(pdev),
> + cxlmd->endpoint->regs.ras);
> }
>
> switch (state) {
> diff --git a/drivers/cxl/core/ras_rch.c b/drivers/cxl/core/ras_rch.c
> index 0a8b3b9b6388..61835fbafc0f 100644
> --- a/drivers/cxl/core/ras_rch.c
> +++ b/drivers/cxl/core/ras_rch.c
> @@ -115,7 +115,9 @@ void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds)
>
> pci_print_aer(pdev, severity, &aer_regs);
> if (severity == AER_CORRECTABLE)
> - cxl_handle_cor_ras(&cxlds->cxlmd->dev, dport->regs.ras);
> + cxl_handle_cor_ras(&cxlds->cxlmd->dev, pci_get_dsn(pdev),
> + dport->regs.ras);
> else
> - cxl_handle_ras(&cxlds->cxlmd->dev, dport->regs.ras);
> + cxl_handle_ras(&cxlds->cxlmd->dev, pci_get_dsn(pdev),
> + dport->regs.ras);
> }
> diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
> index a972e4ef1936..6f3957b3c3af 100644
> --- a/drivers/cxl/core/trace.h
> +++ b/drivers/cxl/core/trace.h
> @@ -48,49 +48,22 @@
> { CXL_RAS_UC_IDE_RX_ERR, "IDE Rx Error" } \
> )
>
> -TRACE_EVENT(cxl_port_aer_uncorrectable_error,
> - TP_PROTO(struct device *dev, u32 status, u32 fe, u32 *hl),
> - TP_ARGS(dev, status, fe, hl),
> +TRACE_EVENT(cxl_aer_uncorrectable_error,
> + TP_PROTO(const struct device *dev, u32 status, u32 fe, u32 *hl,
> + u64 serial),
> + TP_ARGS(dev, status, fe, hl, serial),
> TP_STRUCT__entry(
> __string(device, dev_name(dev))
> __string(host, dev_name(dev->parent))
> - __field(u32, status)
> - __field(u32, first_error)
> - __array(u32, header_log, CXL_HEADERLOG_SIZE_U32)
> - ),
> - TP_fast_assign(
> - __assign_str(device);
> - __assign_str(host);
> - __entry->status = status;
> - __entry->first_error = fe;
> - /*
> - * Embed the 512B headerlog data for user app retrieval and
> - * parsing, but no need to print this in the trace buffer.
> - */
> - memcpy(__entry->header_log, hl, CXL_HEADERLOG_SIZE);
> - ),
> - TP_printk("device=%s host=%s status: '%s' first_error: '%s'",
> - __get_str(device), __get_str(host),
> - show_uc_errs(__entry->status),
> - show_uc_errs(__entry->first_error)
> - )
> -);
> -
> -TRACE_EVENT(cxl_aer_uncorrectable_error,
> - TP_PROTO(const struct cxl_memdev *cxlmd, u32 status, u32 fe, u32 *hl),
> - TP_ARGS(cxlmd, status, fe, hl),
> - TP_STRUCT__entry(
> - __string(memdev, dev_name(&cxlmd->dev))
> - __string(host, dev_name(cxlmd->dev.parent))
> __field(u64, serial)
> __field(u32, status)
> __field(u32, first_error)
> __array(u32, header_log, CXL_HEADERLOG_SIZE_U32)
> ),
> TP_fast_assign(
> - __assign_str(memdev);
> + __assign_str(device);
> __assign_str(host);
> - __entry->serial = cxlmd->cxlds->serial;
> + __entry->serial = serial;
> __entry->status = status;
> __entry->first_error = fe;
> /*
> @@ -99,8 +72,8 @@ TRACE_EVENT(cxl_aer_uncorrectable_error,
> */
> memcpy(__entry->header_log, hl, CXL_HEADERLOG_SIZE);
> ),
> - TP_printk("memdev=%s host=%s serial=%lld: status: '%s' first_error: '%s'",
> - __get_str(memdev), __get_str(host), __entry->serial,
> + TP_printk("device=%s host=%s serial=%lld status: '%s' first_error: '%s'",
> + __get_str(device), __get_str(host), __entry->serial,
> show_uc_errs(__entry->status),
> show_uc_errs(__entry->first_error)
> )
> @@ -124,42 +97,23 @@ TRACE_EVENT(cxl_aer_uncorrectable_error,
> { CXL_RAS_CE_PHYS_LAYER_ERR, "Received Error From Physical Layer" } \
> )
>
> -TRACE_EVENT(cxl_port_aer_correctable_error,
> - TP_PROTO(struct device *dev, u32 status),
> - TP_ARGS(dev, status),
> +TRACE_EVENT(cxl_aer_correctable_error,
> + TP_PROTO(const struct device *dev, u32 status, u64 serial),
> + TP_ARGS(dev, status, serial),
> TP_STRUCT__entry(
> __string(device, dev_name(dev))
> __string(host, dev_name(dev->parent))
> - __field(u32, status)
> - ),
> - TP_fast_assign(
> - __assign_str(device);
> - __assign_str(host);
> - __entry->status = status;
> - ),
> - TP_printk("device=%s host=%s status='%s'",
> - __get_str(device), __get_str(host),
> - show_ce_errs(__entry->status)
> - )
> -);
> -
> -TRACE_EVENT(cxl_aer_correctable_error,
> - TP_PROTO(const struct cxl_memdev *cxlmd, u32 status),
> - TP_ARGS(cxlmd, status),
> - TP_STRUCT__entry(
> - __string(memdev, dev_name(&cxlmd->dev))
> - __string(host, dev_name(cxlmd->dev.parent))
> __field(u64, serial)
> __field(u32, status)
> ),
> TP_fast_assign(
> - __assign_str(memdev);
> + __assign_str(device);
> __assign_str(host);
> - __entry->serial = cxlmd->cxlds->serial;
> + __entry->serial = serial;
> __entry->status = status;
> ),
> - TP_printk("memdev=%s host=%s serial=%lld: status: '%s'",
> - __get_str(memdev), __get_str(host), __entry->serial,
> + TP_printk("device=%s host=%s serial=%lld status: '%s'",
> + __get_str(device), __get_str(host), __entry->serial,
> show_ce_errs(__entry->status)
> )
> );
^ permalink raw reply
* Re: [PATCH v17 01/11] PCI/AER: Introduce AER-CXL Kfifo
From: Jonathan Cameron @ 2026-05-07 17:53 UTC (permalink / raw)
To: Terry Bowman
Cc: dave, dave.jiang, alison.schofield, djbw, bhelgaas, shiju.jose,
ming.li, Smita.KoralahalliChannabasappa, rrichter, dan.carpenter,
PradeepVineshReddy.Kodamati, lukas, Benjamin.Cheatham,
sathyanarayanan.kuppuswamy, vishal.l.verma, alucerop, ira.weiny,
corbet, rafael, xueshuai, linux-cxl, linux-kernel, linux-pci,
linux-acpi, linux-doc
In-Reply-To: <20260505173029.2718246-2-terry.bowman@amd.com>
On Tue, 5 May 2026 12:30:19 -0500
Terry Bowman <terry.bowman@amd.com> wrote:
> CXL virtual hierarchy (VH) native RAS handling for CXL Port devices will be
> added soon. This requires a notification mechanism for the AER driver to
> share the AER interrupt with the CXL driver. The CXL drivers use the
> notification to handle and log the CXL RAS errors.
>
> Note, 'CXL protocol error' terminology refers to CXL VH and not CXL RCH
> errors unless specifically noted going forward.
>
> Introduce a new file in the AER driver to handle the CXL protocol
> errors: pci/pcie/aer_cxl_vh.c.
>
> Add a kfifo work queue to be used by the AER and CXL drivers. Multiple
> AER IRQ worker threads can be running and enqueueing concurrently, so
> include write path synchronization. Pack the kfifo, the spinlock, the
> rwsem, and the work pointer into a single structure. Initialize the
> kfifo with INIT_KFIFO() from a subsys_initcall so its mask, esize and
> data fields are valid before any producer or consumer runs.
>
> Add CXL work queue handler registration functions in the AER driver.
> Export them so the CXL driver can assign or clear the work handler.
>
> Introduce 'struct cxl_proto_err_work_data' to serve as the kfifo work
> data. It contains a reference to the PCI error source device and the
> error severity. The cxl_core driver uses this when dequeuing the work.
>
> Introduce cxl_forward_error() to add a given CXL protocol error to a
> work structure and push it onto the AER-CXL kfifo. This function takes
> a pci_dev_get() on the source device. The kfifo consumer is responsible
> for the matching pci_dev_put() after dequeue. On enqueue failure
> cxl_forward_error() does the put itself.
>
> Synchronize accesses to the work function pointer during registration,
> deregistration, enqueue, and dequeue.
>
> handle_error_source() is intentionally not changed here. The is_cxl_error()
> switch that routes errors to cxl_forward_error() is added in a later patch
> together with the kfifo consumer registration. This way the producer and
> consumer land in the same commit, so CXL errors are not silently dropped
> during bisect.
>
> Also add MAINTAINERS entries for both drivers/pci/pcie/aer_cxl_vh.c
> (new in this patch) and drivers/pci/pcie/aer_cxl_rch.c (already in tree
> but previously unlisted) under the existing CXL entry. This way the CXL
> maintainers are CC'd on changes to the AER-CXL bridging code.
>
> Co-developed-by: Dan Williams <djbw@kernel.org>
> Signed-off-by: Dan Williams <djbw@kernel.org>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Sashiko did have one comment on what happens if there are multiple things
in the kfifo and fn fails. At that point I think we are in the all
bets are off corner and stranding a driver is fine, but open to other opinions!
https://sashiko.dev/#/patchset/20260505173029.2718246-1-terry.bowman%40amd.com
So with that in mind
Reviewed-by: Jonathan Cameron <jic23@kernel.org>
> diff --git a/drivers/pci/pcie/aer_cxl_vh.c b/drivers/pci/pcie/aer_cxl_vh.c
> new file mode 100644
> index 000000000000..c0fea2c2b9bc
> --- /dev/null
> +++ b/drivers/pci/pcie/aer_cxl_vh.c
> +int for_each_cxl_proto_err(struct cxl_proto_err_work_data *wd,
> + cxl_proto_err_fn_t fn)
> +{
> + int rc;
> +
> + guard(rwsem_read)(&cxl_proto_err_kfifo.rwsem);
> + while (kfifo_get(&cxl_proto_err_kfifo.fifo, wd)) {
> + rc = fn(wd);
> + pci_dev_put(wd->pdev);
> + if (rc)
> + return rc;
This is where Sashiko complains. Specifically:
"If the consumer callback fn() returns an error, does this early return
strand the remaining items in the kfifo?
Because cxl_forward_error() takes a pci_dev reference for each enqueued
item, it looks like these stranded items might leak their pci_dev references
and prevent clean unbinding or hot-unplug until a new error triggers the
queue again."
I'd go with indeed it does, but there is no right thing to do here. I guess
we could flush the kfifo and call pci_dev_put() on each of them, but that's horrible.
Would basically mean calling the same stuff you have for cancelling outstanding
entrees on exit().
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_FOR_MODULES(for_each_cxl_proto_err, "cxl_core");
^ permalink raw reply
* Re: [PATCH] Documentation: fix typo and formattting in security/credentials.rst
From: John Doe @ 2026-05-07 17:48 UTC (permalink / raw)
To: Randy Dunlap
Cc: Paul Moore, Serge Hallyn, Jonathan Corbet, Shuah Khan,
linux-security-module, linux-doc, linux-kernel
In-Reply-To: <cbe6e145-9d2f-47bb-8d22-1599cfd1a9d4@infradead.org>
On Wed, May 06, 2026 at 05:36:53PM -0700, Randy Dunlap wrote:
> Hi,
>
> On 5/6/26 3:59 PM, Mayank Gite wrote:
> > - Fixes a typo in "Keys and keyrings" section. Replaces "keying" with
> > "keyring".
> > - Updates formatting of keyring types.
> >
> > Signed-off-by: Mayank Gite <drapl0n.kernel@gmail.com>
>
> Acked-by: Randy Dunlap <rdunlap@infradead.org>
> Tested-by: Randy Dunlap <rdunlap@infradead.org>
>
> > ---
> In the future, right here, after that "---" line, it should say
> something like
>
> v2: use 'git send-email' so that that patch isn't corrupted
>
> and in the Subject: it should say
> [PATCH v2] ...
>
> Thanks.
>
> > Documentation/security/credentials.rst | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/security/credentials.rst b/Documentation/security/credentials.rst
> > index d0191c8b8060..4996838491b1 100644
> > --- a/Documentation/security/credentials.rst
> > +++ b/Documentation/security/credentials.rst
> > @@ -189,9 +189,9 @@ The Linux kernel supports the following types of credentials:
> > be searched for the desired key. Each process may subscribe to a number
> > of keyrings:
> >
> > - Per-thread keying
> > - Per-process keyring
> > - Per-session keyring
> > + - Per-thread keyring
> > + - Per-process keyring
> > + - Per-session keyring
> >
> > When a process accesses a key, if not already present, it will normally be
> > cached on one of these keyrings for future accesses to find.
>
> --
> ~Randy
Hi,
Thank you for the feedback and acknowledgment. I thought that since my
earlier patch was invalid/corrupted, I should submit a new one rather than
version 2.
I understand the procedure now and will ensure that future revisions follow
the guidelines.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: iio: dac: Add AD5529R
From: Conor Dooley @ 2026-05-07 17:35 UTC (permalink / raw)
To: Janani Sunil
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Jonathan Corbet,
Shuah Khan, linux-iio, devicetree, linux-kernel, linux-doc,
Janani Sunil
In-Reply-To: <20260507-ad5529r-driver-v1-1-b4460f3cb44f@analog.com>
[-- Attachment #1: Type: text/plain, Size: 380 bytes --]
On Thu, May 07, 2026 at 06:28:09PM +0200, Janani Sunil wrote:
> Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> buffered voltage output digital-to-analog converter (DAC) with an
> integrated precision reference.
>
> Signed-off-by: Janani Sunil <janani.sunil@analog.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v13 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
From: Mathieu Poirier @ 2026-05-07 17:12 UTC (permalink / raw)
To: Arnaud POULIQUEN
Cc: Beleswar Prasad Padhi, Shenwei Wang, Andrew Lunn, Linus Walleij,
Bartosz Golaszewski, Jonathan Corbet, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Frank Li,
Sascha Hauer, Shuah Khan, linux-gpio@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
Pengutronix Kernel Team, Fabio Estevam, Peng Fan,
devicetree@vger.kernel.org, linux-remoteproc@vger.kernel.org,
imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
dl-linux-imx, Bartosz Golaszewski
In-Reply-To: <6917e3d7-8c6c-4e63-8eca-5308621ec3e8@foss.st.com>
On Tue, May 05, 2026 at 10:46:11AM +0200, Arnaud POULIQUEN wrote:
> Hi Beleswar
>
> On 5/5/26 07:25, Beleswar Prasad Padhi wrote:
> > Hi Arnaud,
> >
> > On 04/05/26 22:34, Arnaud POULIQUEN wrote:
> > > Hi Beleswar,
> > >
> > > On 5/4/26 10:17, Beleswar Prasad Padhi wrote:
> > >
> >
> > [...]
> >
> > > >
> > > > >
> > > > > I may have misunderstood your solution. Could you please help me
> > > > > understand your proposal by explaining how you would handle three
> > > > > GPIO ports defined in the DT, considering that the endpoint
> > > > > addresses on the Linux side can be random?
> > > > > If I assume there is a unique endpoint on the remote side,
> > > > > I do not understand how you can match, on the firmware side,
> > > > > the Linux endpoint address to the GPIO port.
> > > >
> > > >
> > > > Sure, let me take an example:
> > > > Assumptions: 3 GPIO ports in DT, 3 endpoints in Linux (one per port),
> > > > 1 endpoint in remote (0xd) and 1 rpmsg channel (rpmsg-io)
> > > >
> > > > rpmsg {
> > > > rpmsg-io {
> > > > #address-cells = <1>;
> > > > #size-cells = <0>;
> > > >
> > > > gpio@25 {
> > > > compatible = "rpmsg-gpio";
> > > > reg = <25>;
> > > > gpio-controller;
> > > > #gpio-cells = <2>;
> > > > #interrupt-cells = <2>;
> > > > interrupt-controller;
> > > > };
> > > >
> > > > gpio@32 {
> > > > compatible = "rpmsg-gpio";
> > > > reg = <32>;
> > > > gpio-controller;
> > > > #gpio-cells = <2>;
> > > > #interrupt-cells = <2>;
> > > > interrupt-controller;
> > > > };
> > > >
> > > > gpio@35 {
> > > > compatible = "rpmsg-gpio";
> > > > reg = <35>;
> > > > gpio-controller;
> > > > #gpio-cells = <2>;
> > > > #interrupt-cells = <2>;
> > > > interrupt-controller;
> > > > };
> > > > };
> > > > };
> > > >
> > > > Code Flow:
> > > > 1. "rpmsg-io" channel is announced from remote firmware with unique dst
> > > > ept = 0xd.
> > > >
> > > > 2. rpmsg_core.c creates the default dynamic local ept for the channel
> > > > ept = 0x405.
> > > >
> > > > 3. rpmsg_core.c assigns the allocated addr to rpdev device:
> > > > rpdev->src = 0x405 and rpdev->dst = 0xd.
> > > >
> > > > 4. rpmsg_gpio_channel_probe() is triggered. For *each* of the GPIO ports
> > > > in DT, it will trigger rpmsg_gpiochip_register() which will now:
> > > > a. Call port->ept = rpmsg_create_ept(rpdev,
> > > > rpmsg_gpio_channel_callback,
> > > > port,
> > > > {rpdev.id.name,
> > > > RPMSG_ADDR_ANY,
> > > > RPMSG_ADDR_ANY});
> > > > Ex- port->ept->addr = 0x408
> > > >
> > > > b. Prepare a 8-byte message having 2 fields:
> > > > port->ept->addr (0x408) and port->idx (25)
> > > >
> > > > c. Send this message to remote firmware on default channel ept
> > > > (0x405 -> 0xd) by:
> > > > rpmsg_send(rpdev->ept, &message, sizeof(message));
> > > >
> > > > d. Remote side receives this message and creates a map of the
> > > > linux_ept_addr to gpio_port. (0x408 <-> 25)
> > > >
> > > > 5. After this point, any gpio messages sent from Linux from gpio port
> > > > endpoints (Ex- 0x408) can be decoded at remote side by looking up
> > > > its map (Ex- map[0x408] = 25).
> > > >
> > > > 6. Any messages sent from remote to Linux for a particular gpio port can
> > > > also be decoded at Linux by simply fetching the priv pointer to get
> > > > the per-port device:
> > > > struct rpmsg_gpio_port *port = priv;
> > > >
> > >
> > > Thanks for the details!
> > >
> > > To sum up:
> > > - the default endpoint acts as the GPIO controller (0x405),
> > > - one extra Linux endpoint is created per port defined in DT.
> > >
> > > This should work, but my concerns remain the same:
> > >
> > > 1) This implementation forces the remote processor to handle a single
> > > endpoint instead of one endpoint per port. This may add complexity to
> > > the remote firmware if each port is managed in a separate thread.
> >
> >
> > A. Not really, I just chose 1 remote endpoint for this example as you
> > suggested to. We can scale it for two-way communication via the
> > get_config message like you suggested below.
> >
> > B. Isn't it a bad design of the firmware if it is handling 10 gpio ports
> > in 10 threads? The logic to handle all the ports is the same, only
> > the parameters (e.g. line number, msg) is different.
> >
> > >
> > > 2) Linux, as a consumer, should not expose its capabilities to the remote
> > > side (in your proposal it enumerates the ports defined in the DT). In my view, the remote processor should expose its capabilities as the
> > > provider.
> >
> >
> > Agreed on this.
> >
> > >
> > > From my perspective, based on your proposal:
> > > 1) Linux should send a get_config message to the remote proc (0x405 -> 0xD). 2) The remote processor would respond with the list of ports, associated
> > > with an remote endpoint addresses.
> >
> >
> > Agreed, we can scale it for multiple remote endpoints like this.
> >
> > > 3) Linux would parse the response, compare it with the DT, enable the GPIO
> > > ports accordingly, creating it local endpoint and associating it with
> > > the remote endpoint.
> > > Using name service to identify the ports should avoid step 1 & 2 ...
> >
> >
> > Yes, but won't that make a lot of hard-codings in the driver?
> >
> > +static struct rpmsg_device_id rpmsg_gpio_channel_id_table[] = {
> > + { .name = "rpmsg-io-25" },
> > + { .name = "rpmsg-io-32" },
> > + { .name = "rpmsg-io-35" },
> > + { },
> > +};
> >
> > What if tomorrow another vendor decides to add more remoteproc
> > controlled GPIO ports to Linux, they would have to update this struct in
> > the driver everytime. And the port indexes (25/32/35) could also differ
> > between vendors. We should make the driver dynamic i.e. vendor
> > agnostic.
> >
> > I think querying the remote firmware at runtime (step 1 & 2 above) is a
> > common design pattern and makes the driver vendor agnostic. But feel
> > free to correct me.
> >
>
> You are right. My proposal would require a patch in rpmsg-core. The idea of
> allowing a postfix in the compatible string has been discussed before, but,
> if I remember correctly, it was not concluded.
>
I also remember discussing this. I even reviewed one of Arnaud's patch
and submitted one myself. This must have been in 2020 and the reason why it
wasn't merged has escaped my memory.
> /* rpmsg devices and drivers are matched using the service name */
> static inline int rpmsg_id_match(const struct rpmsg_device *rpdev,
> const struct rpmsg_device_id *id)
> {
> size_t len;
>
> + len = strnlen(id->name, RPMSG_NAME_SIZE);
> + if (len && id->name[len - 1] == '*')
> + return !strncmp(id->name, rpdev->id.name, len - 1);
>
> return strncmp(id->name, rpdev->id.name, RPMSG_NAME_SIZE) == 0;
> }
>
> Then, in rpmsg-gpio, and possibly in other drivers such as rpmsg-tty and
> a future rpmsg-i2c, we could use:
> static struct rpmsg_device_id rpmsg_gpio_channel_id_table[] = {
> { .name = "rpmsg-io" },
> { .name = "rpmsg-io-*" },
> { },
> };
That was my initial approach. We don't even need an additional "rpmsg-io-*" in
rpmsg_gpio_channel_id_table[]. All we need is:
/* rpmsg devices and drivers are matched using the service name */
static inline int rpmsg_id_match(const struct rpmsg_device *rpdev,
const struct rpmsg_device_id *id)
{
+ size_t len = strnlen(id->name, RPMSG_NAME_SIZE);
- return strncmp(id->name, rpdev->id.name, RPMSG_NAME_SIZE) == 0;
+ return strncmp(id->name, rpdev->id.name, len) == 0;
}
And let the rpmsg-virtio-gpio driver parse @rpdev->id.name to match with a
GPIO controller in the DT.
>
> If exact name matching is strongly required, then this proposal would not be
> suitablea.
>
> A third option would be a combination of both approaches: instantiate the
> device using the same name service from the remote side, as done in
> rpmsg-tty. In that case, a get_config message, or a similar mechanism, would
> also be needed to retrieve the port information from the remote side.
>
I'm not overly fond of a get_config message because it is one more thing we
have to define and maintain.
Arnaud: is there a get_config message already defined for rpmsg_tty?
Beleswar: Can you provide a link to a virtio device that would use a get_config
message?
> Tanmaya also proposed another alternative based on reserved addresses.
>
> At this point, I suggest letting Mathieu review the discussion and recommend
> the most suitable approach.
>
> Thanks,
> Arnaud
>
> > >
> > > At the end, whatever solution is implemented, my main concern is that the
> > > Linux driver design should, if possible, avoid adding unnecessary complexity
> > > or limitations on the remote side (for instance in openAMP project).
> >
> >
> > Yes definitely, I want the same. Feel free to let me know if this does
> > not suit with the OpenAMP project.
> >
> > Thanks,
> > Beleswar
> >
> > >
> > > Thanks,
> > > Arnaud
> > >
> > >
> > > > So Linux does not need to send the port idx everytime while sending a
> > > > gpio message anymore.
> > > >
> > > > Thanks,
> > > > Beleswar
> > > >
> > > > [...]
> > > >
> > >
>
^ permalink raw reply
* Re: [PATCH v4 1/3] dt-bindings: hwmon: pmbus: add Delta E50SN12051 binding
From: Conor Dooley @ 2026-05-07 17:07 UTC (permalink / raw)
To: u8813345
Cc: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Kevin Chang, Jonathan Corbet, Shuah Khan, linux-hwmon, devicetree,
linux-kernel, linux-doc, Colin Huang
In-Reply-To: <20260507-add-e50sn12051-v4-1-ff2b3768ac7e@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1930 bytes --]
On Thu, May 07, 2026 at 01:12:26PM +0800, Colin Huang via B4 Relay wrote:
> From: Colin Huang <u8813345@gmail.com>
>
> Add devicetree binding documentation for the Delta E50SN12051
> PMBus-compliant device.
>
> Signed-off-by: Colin Huang <u8813345@gmail.com>
> ---
> .../bindings/hwmon/pmbus/delta,e50sn12051.yaml | 42 ++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/delta,e50sn12051.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/delta,e50sn12051.yaml
> new file mode 100644
> index 000000000000..72aefe212d17
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/delta,e50sn12051.yaml
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/pmbus/delta,e50sn12051.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Delta E50SN12051 PMBus Sensor
> +
> +maintainers:
> + - Kevin Chang <kevin.chang2@amd.com>
> +
> +description: |
> + Delta E50SN12051 is a non-isolated 1/8th brick DC-DC power module.
> + It is a PMBus-compliant device accessible via an I2C/SMBus interface
> + and provides standard telemetry such as voltage, current, and
> + temperature measurements.
> +
> +properties:
> + compatible:
> + const: delta,e50sn12051
> +
> + reg:
> + maxItems: 1
> + description: I2C bus address of the PMBus device
> +
> +required:
> + - compatible
> + - reg
With only these two properties, shouldn't this be in trivial-devices?
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + power-module@40 {
> + compatible = "delta,e50sn12051";
> + reg = <0x40>;
> + };
> + };
>
> --
> 2.34.1
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH RFC v5 10/53] KVM: guest_memfd: Add basic support for KVM_SET_MEMORY_ATTRIBUTES2
From: Ackerley Tng @ 2026-05-07 16:56 UTC (permalink / raw)
To: Liam R. Howlett
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
ira.weiny, jmattson, jthoughton, michael.roth, 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, Youngjun Park, Qi Zheng,
Shakeel Butt, Kiryl Shutsemau, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <n5ce32wumzeiqqyqutom4apy2kqfetbvusc6j4k2xarsska5mw@klp5bmy7qhfm>
"Liam R. Howlett" <liam@infradead.org> writes:
> On 26/04/28 04:25PM, Ackerley Tng via B4 Relay wrote:
>>
>> [...snip...]
>>
>> +/*
>> + * Preallocate memory for attributes to be stored on a maple tree, pointed to
>> + * by mas. Adjacent ranges with attributes identical to the new attributes
>> + * will be merged. Also sets mas's bounds up for storing attributes.
>> + *
>> + * This maintains the invariant that ranges with the same attributes will
>> + * always be merged.
>> + */
>> +static int kvm_gmem_mas_preallocate(struct ma_state *mas, u64 attributes,
>> + pgoff_t start, size_t nr_pages)
>> +{
>> + pgoff_t end = start + nr_pages;
>> + pgoff_t last = end - 1;
>> + void *entry;
>> +
>> + /* Try extending range. entry is NULL on overflow/wrap-around. */
>> + mas_set_range(mas, end, end);
>> + entry = mas_find(mas, end);
Thank you for your reviews!
>
> Please read the documentation as I believe you have a bug here. What
> happens if there is another range stored higher than end + 1?
>
The invariant in this maple tree is that contiguous ranges with the same
attribute are stored as a single range.
The goal of this first part is to get the entry at the index just after
the requested range, and see what the attribute there is. If that
attribute is what we're about to set, extend the requested range for
storing to the end of that range.
If there is another range higher than end + 1, with the invariant
maintained, that attribute has to be different than the attribute stored
at end. Hence, we only want to extend this requested range up till end.
> Do you have testing of these functions somewhere?
>
GMEM_CONVERSION_MULTIPAGE_TEST_INIT_SHARED(indexing, 4) tests setting
attributes in ranges. If test_page is 2,
1. [0, 4) starts off shared (4 is the number of pages in the guest_memfd)
2. [2, 3) is converted to private
=> so the ranges should now be [0, 2), [2, 3), [3, 4)
3. [2, 3) is converted back to shared
=> so the ranges should now be [0, 4)
I verified this by inserting some trace_printk()s and inspecting manually.
>> + if (entry && xa_to_value(entry) == attributes)
>> + last = mas->last;
>> +
>> + if (start > 0) {
>> + mas_set_range(mas, start - 1, start - 1);
>> + entry = mas_find(mas, start - 1);
>> + if (entry && xa_to_value(entry) == attributes)
>> + start = mas->index;
>> + }
>> +
>> + mas_set_range(mas, start, last);
>> + return mas_preallocate(mas, xa_mk_value(attributes), GFP_KERNEL);
>> +}
>> +
>>
>> [...snip...]
>>
^ permalink raw reply
* [PATCH v5 28/28] mtd: spi-nor: winbond: Add W25Q02NWxxIM CMP locking support
From: Miquel Raynal @ 2026-05-07 16:47 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
Tudor Ambarus, Shuah Khan
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>
This chip has support for the locking complement (CMP) feature. Add
the relevant bit to enable it.
Unfortunately, this chip also comes with an incorrect BFPT table,
indicating the Control Register cannot be read back. This is wrong,
reading back the register works and has no (observed) side effect. The
datasheet clearly indicates supporting the 35h command and all bits from
the CR are marked readable. QE and CMP bits are inside, and can be
properly read back.
Add a fixup for this, otherwise it would defeat the use of the CMP
feature.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
Test run with W25Q02NWxxIM:
$ flash_lock -u /dev/mtd0
$ flash_lock -l /dev/mtd0 $bs $all_but_one # all but the first
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-0000ffff | unlocked | 1
00010000-0fffffff | locked | 4095
$ flash_lock -u /dev/mtd0 $bs 1 # all but the two first
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-0001ffff | unlocked | 2
00020000-0fffffff | locked | 4094
$ flash_lock -u /dev/mtd0
$ flash_lock -l /dev/mtd0 0 $all_but_one # same from the other side
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-0ffeffff | locked | 4095
0fff0000-0fffffff | unlocked | 1
$ flash_lock -u /dev/mtd0 $(($size - (2 * $bs))) 1 # all but two
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-0ffdffff | locked | 4094
0ffe0000-0fffffff | unlocked | 2
---
drivers/mtd/spi-nor/winbond.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
index 3a3b7f2f1659..8ebdbcec0b3f 100644
--- a/drivers/mtd/spi-nor/winbond.c
+++ b/drivers/mtd/spi-nor/winbond.c
@@ -379,7 +379,9 @@ static const struct flash_info winbond_nor_parts[] = {
}, {
/* W25Q02NWxxIM */
.id = SNOR_ID(0xef, 0x80, 0x22),
- .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6 | SPI_NOR_4BIT_BP,
+ .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6 |
+ SPI_NOR_4BIT_BP | SPI_NOR_HAS_CMP,
+ .fixups = &winbond_rdcr_fixup,
}, {
/* W25H512NWxxAM */
.id = SNOR_ID(0xef, 0xa0, 0x20),
--
2.53.0
^ permalink raw reply related
* [PATCH v5 27/28] mtd: spi-nor: winbond: Add W25Q01NWxxIM CMP locking support
From: Miquel Raynal @ 2026-05-07 16:47 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
Tudor Ambarus, Shuah Khan
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>
This chip has support for the locking complement (CMP) feature. Add
the relevant bit to enable it.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
Test run with W25Q01NWxxIM:
$ flash_lock -u /dev/mtd0
$ flash_lock -l /dev/mtd0 $bs $all_but_one # all but the first
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-0000ffff | unlocked | 1
00010000-0fffffff | locked | 4095
$ flash_lock -u /dev/mtd0 $bs 1 # all but the two first
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-0001ffff | unlocked | 2
00020000-0fffffff | locked | 4094
$ flash_lock -u /dev/mtd0
$ flash_lock -l /dev/mtd0 0 $all_but_one # same from the other side
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-0ffeffff | locked | 4095
0fff0000-0fffffff | unlocked | 1
$ flash_lock -u /dev/mtd0 $(($size - (2 * $bs))) 1 # all but two
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-0ffdffff | locked | 4094
0ffe0000-0fffffff | unlocked | 2
---
drivers/mtd/spi-nor/winbond.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
index 3eca7baa4d5a..3a3b7f2f1659 100644
--- a/drivers/mtd/spi-nor/winbond.c
+++ b/drivers/mtd/spi-nor/winbond.c
@@ -374,7 +374,8 @@ static const struct flash_info winbond_nor_parts[] = {
}, {
/* W25Q01NWxxIM */
.id = SNOR_ID(0xef, 0x80, 0x21),
- .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6 | SPI_NOR_4BIT_BP,
+ .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6 |
+ SPI_NOR_4BIT_BP | SPI_NOR_HAS_CMP,
}, {
/* W25Q02NWxxIM */
.id = SNOR_ID(0xef, 0x80, 0x22),
--
2.53.0
^ permalink raw reply related
* [PATCH v5 26/28] mtd: spi-nor: winbond: Add W25H01NWxxIQ CMP locking support
From: Miquel Raynal @ 2026-05-07 16:47 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
Tudor Ambarus, Shuah Khan
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>
This chip has support for the locking complement (CMP) feature. Add
the relevant bit to enable it.
Unfortunately, this chip also comes with an incorrect BFPT table,
indicating the Control Register cannot be read back. This is wrong,
reading back the register works and has no (observed) side effect. The
datasheet clearly indicates supporting the 35h command and all bits from
the CR are marked readable. QE and CMP bits are inside, and can be
properly read back.
Add a fixup for this, otherwise it would defeat the use of the CMP
feature.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
Test run with W25H01NWxxIQ:
$ flash_lock -u /dev/mtd0
$ flash_lock -l /dev/mtd0 $bs $all_but_one # all but the first
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-0000ffff | unlocked | 1
00010000-07ffffff | locked | 2047
$ flash_lock -u /dev/mtd0 $bs 1 # all but the two first
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-0001ffff | unlocked | 2
00020000-07ffffff | locked | 2046
$ flash_lock -u /dev/mtd0
$ flash_lock -l /dev/mtd0 0 $all_but_one # same from the other side
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-07feffff | locked | 2047
07ff0000-07ffffff | unlocked | 1
$ flash_lock -u /dev/mtd0 $(($size - (2 * $bs))) 1 # alll but two
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-07fdffff | locked | 2046
07fe0000-07ffffff | unlocked | 2
---
drivers/mtd/spi-nor/winbond.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
index 2c48d6c4a0aa..3eca7baa4d5a 100644
--- a/drivers/mtd/spi-nor/winbond.c
+++ b/drivers/mtd/spi-nor/winbond.c
@@ -368,7 +368,9 @@ static const struct flash_info winbond_nor_parts[] = {
}, {
/* W25Q01NWxxIQ */
.id = SNOR_ID(0xef, 0x60, 0x21),
- .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6 | SPI_NOR_4BIT_BP,
+ .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6 |
+ SPI_NOR_4BIT_BP | SPI_NOR_HAS_CMP,
+ .fixups = &winbond_rdcr_fixup,
}, {
/* W25Q01NWxxIM */
.id = SNOR_ID(0xef, 0x80, 0x21),
--
2.53.0
^ permalink raw reply related
* [PATCH v5 25/28] mtd: spi-nor: winbond: Add W25H02NWxxAM CMP locking support
From: Miquel Raynal @ 2026-05-07 16:47 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
Tudor Ambarus, Shuah Khan
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>
This chip has support for the locking complement (CMP) feature. Add
the relevant bit to enable it.
Unfortunately, this chip also comes with an incorrect BFPT table,
indicating the Control Register cannot be read back. This is wrong,
reading back the register works and has no (observed) side effect. The
datasheet clearly indicates supporting the 35h command and all bits from
the CR are marked readable. QE and CMP bits are inside, and can be
properly read back.
Add a fixup for this, otherwise it would defeat the use of the CMP
feature.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
Test run with W25H02NWxxAM:
$ flash_lock -u /dev/mtd0
$ flash_lock -l /dev/mtd0 $bs $all_but_one # all but the first
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-0000ffff | unlocked | 1
00010000-0fffffff | locked | 4095
$ flash_lock -u /dev/mtd0 $bs 1 # all but the two first
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-0001ffff | unlocked | 2
00020000-0fffffff | locked | 4094
$ flash_lock -u /dev/mtd0
$ flash_lock -l /dev/mtd0 0 $all_but_one # same from the other side
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-0ffeffff | locked | 4095
0fff0000-0fffffff | unlocked | 1
$ flash_lock -u /dev/mtd0 $(($size - (2 * $bs))) 1 # all but two
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-0ffdffff | locked | 4094
0ffe0000-0fffffff | unlocked | 2
---
drivers/mtd/spi-nor/winbond.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
index 7628fa7fb64f..2c48d6c4a0aa 100644
--- a/drivers/mtd/spi-nor/winbond.c
+++ b/drivers/mtd/spi-nor/winbond.c
@@ -73,6 +73,26 @@ static const struct spi_nor_fixups w25q256_fixups = {
.post_bfpt = w25q256_post_bfpt_fixups,
};
+static int
+winbond_rdcr_post_bfpt_fixup(struct spi_nor *nor,
+ const struct sfdp_parameter_header *bfpt_header,
+ const struct sfdp_bfpt *bfpt)
+{
+ /*
+ * W25H02NW, unlike its W25H512NW nor W25H01NW cousins, improperly sets
+ * the QE BFPT configuration bits, indicating a non readable CR. This is
+ * both incorrect and impractical, as the chip features a CMP bit for its
+ * locking scheme that lays in the Control Register, and needs to be read.
+ */
+ nor->flags &= ~SNOR_F_NO_READ_CR;
+
+ return 0;
+}
+
+static const struct spi_nor_fixups winbond_rdcr_fixup = {
+ .post_bfpt = winbond_rdcr_post_bfpt_fixup,
+};
+
/**
* winbond_nor_select_die() - Set active die.
* @nor: pointer to 'struct spi_nor'.
@@ -370,7 +390,9 @@ static const struct flash_info winbond_nor_parts[] = {
}, {
/* W25H02NWxxAM */
.id = SNOR_ID(0xef, 0xa0, 0x22),
- .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6 | SPI_NOR_4BIT_BP,
+ .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6 |
+ SPI_NOR_4BIT_BP | SPI_NOR_HAS_CMP,
+ .fixups = &winbond_rdcr_fixup,
},
};
--
2.53.0
^ permalink raw reply related
* [PATCH v5 24/28] mtd: spi-nor: winbond: Add W25H01NWxxAM CMP locking support
From: Miquel Raynal @ 2026-05-07 16:47 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
Tudor Ambarus, Shuah Khan
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>
This chip has support for the locking complement (CMP) feature. Add
the relevant bit to enable it.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
Test run with W25H01NWxxAM:
$ flash_lock -u /dev/mtd0
$ flash_lock -l /dev/mtd0 $bs $all_but_one # all but the first
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-0000ffff | unlocked | 1
00010000-07ffffff | locked | 2047
$ flash_lock -u /dev/mtd0 $bs 1 # all but the two first
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-0001ffff | unlocked | 2
00020000-07ffffff | locked | 2046
$ flash_lock -u /dev/mtd0
$ flash_lock -l /dev/mtd0 0 $all_but_one # same from the other side
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-07feffff | locked | 2047
07ff0000-07ffffff | unlocked | 1
$ flash_lock -u /dev/mtd0 $(($size - (2 * $bs))) 1 # all but two
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-07fdffff | locked | 2046
07fe0000-07ffffff | unlocked | 2
---
drivers/mtd/spi-nor/winbond.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
index ef73c855cedb..7628fa7fb64f 100644
--- a/drivers/mtd/spi-nor/winbond.c
+++ b/drivers/mtd/spi-nor/winbond.c
@@ -365,7 +365,8 @@ static const struct flash_info winbond_nor_parts[] = {
}, {
/* W25H01NWxxAM */
.id = SNOR_ID(0xef, 0xa0, 0x21),
- .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6 | SPI_NOR_4BIT_BP,
+ .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6 |
+ SPI_NOR_4BIT_BP | SPI_NOR_HAS_CMP,
}, {
/* W25H02NWxxAM */
.id = SNOR_ID(0xef, 0xa0, 0x22),
--
2.53.0
^ permalink raw reply related
* [PATCH v5 23/28] mtd: spi-nor: winbond: Add W25H512NWxxAM CMP locking support
From: Miquel Raynal @ 2026-05-07 16:47 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
Tudor Ambarus, Shuah Khan
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>
This chip has support for the locking complement (CMP) feature. Add
the relevant bit to enable it.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
Test run following the freshly written documentation:
$ flash_lock -u /dev/mtd0
$ flash_lock -l /dev/mtd0 $bs $all_but_one # all but the first
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-0000ffff | unlocked | 1
00010000-03ffffff | locked | 1023
$ flash_lock -u /dev/mtd0 $bs 1 # all but the two first
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-0001ffff | unlocked | 2
00020000-03ffffff | locked | 1022
$ flash_lock -u /dev/mtd0
$ flash_lock -l /dev/mtd0 0 $all_but_one # same from the other side
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-03feffff | locked | 1023
03ff0000-03ffffff | unlocked | 1
$ flash_lock -u /dev/mtd0 $(($size - (2 * $bs))) 1 # all but two
$ show_sectors
locked sectors
region (in hex) | status | #blocks
------------------+----------+--------
00000000-03fdffff | locked | 1022
03fe0000-03ffffff | unlocked | 2
---
drivers/mtd/spi-nor/winbond.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
index eaa547d36aad..ef73c855cedb 100644
--- a/drivers/mtd/spi-nor/winbond.c
+++ b/drivers/mtd/spi-nor/winbond.c
@@ -360,7 +360,8 @@ static const struct flash_info winbond_nor_parts[] = {
}, {
/* W25H512NWxxAM */
.id = SNOR_ID(0xef, 0xa0, 0x20),
- .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6 | SPI_NOR_4BIT_BP,
+ .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6 |
+ SPI_NOR_4BIT_BP | SPI_NOR_HAS_CMP,
}, {
/* W25H01NWxxAM */
.id = SNOR_ID(0xef, 0xa0, 0x21),
--
2.53.0
^ permalink raw reply related
* [PATCH v5 22/28] mtd: spi-nor: Add steps for testing locking with CMP
From: Miquel Raynal @ 2026-05-07 16:47 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
Tudor Ambarus, Shuah Khan
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>
Extend the test coverage by giving guidelines to verify the CMP bit acts
according to our expectations.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
The instructions listed in this file target people adding support for
new chips, however here are below extra steps that I also ran with the
same W25H512NWxxAM chip. They are here to prove core correctness.
$ flash_lock -u /dev/mtd0
$ flash_lock -l /dev/mtd0 0 1008
$ show_sectors
software locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-03efffff | locked | 1008
03f00000-03ffffff | unlocked | 16
$ flash_lock -l /dev/mtd0 0 1009
$ show_sectors # should not change
software locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-03efffff | locked | 1008
03f00000-03ffffff | unlocked | 16
$ flash_lock -l /dev/mtd0 0 1015
$ show_sectors # should not change
software locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-03efffff | locked | 1008
03f00000-03ffffff | unlocked | 16
$ flash_lock -l /dev/mtd0 0 1016
$ show_sectors # should cover more
software locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-03f7ffff | locked | 1016
03f80000-03ffffff | unlocked | 8
$ flash_lock -u /dev/mtd0 $((1015 * $bs)) 1
$ show_sectors # should not change
software locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-03f7ffff | locked | 1016
03f80000-03ffffff | unlocked | 8
$ flash_lock -u /dev/mtd0 $((1009 * $bs)) 7
$ show_sectors # should not change
software locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-03f7ffff | locked | 1016
03f80000-03ffffff | unlocked | 8
$ flash_lock -u /dev/mtd0 $((1008 * $bs)) 8
$ show_sectors # range should reduce down to initial value
software locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-03efffff | locked | 1008
03f00000-03ffffff | unlocked | 16
[Similar situations, on the other side of the device]
$ flash_lock -u /dev/mtd0
$ flash_lock -l /dev/mtd0 $((16 * $bs)) 1008
$ show_sectors
software locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-000fffff | unlocked | 16
00100000-03ffffff | locked | 1008
$ flash_lock -l /dev/mtd0 $((15 * $bs)) 1009
$ show_sectors # should not change
software locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-000fffff | unlocked | 16
00100000-03ffffff | locked | 1008
$ flash_lock -l /dev/mtd0 $((9 * $bs)) 1015
$ show_sectors # should not change
software locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-000fffff | unlocked | 16
00100000-03ffffff | locked | 1008
$ flash_lock -l /dev/mtd0 $((8 * $bs)) 1016
$ show_sectors # should cover more
software locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-0007ffff | unlocked | 8
00080000-03ffffff | locked | 1016
$ flash_lock -u /dev/mtd0 $((8 * $bs)) 1
$ show_sectors # should not change
software locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-0007ffff | unlocked | 8
00080000-03ffffff | locked | 1016
$ flash_lock -u /dev/mtd0 $((8 * $bs)) 7
$ show_sectors # should not change
software locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-0007ffff | unlocked | 8
00080000-03ffffff | locked | 1016
$ flash_lock -u /dev/mtd0 $((8 * $bs)) 8
$ show_sectors # range should reduce down to initial value
software locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-000fffff | unlocked | 16
00100000-03ffffff | locked | 1008
---
Documentation/driver-api/mtd/spi-nor.rst | 37 ++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/Documentation/driver-api/mtd/spi-nor.rst b/Documentation/driver-api/mtd/spi-nor.rst
index b42de32c6eba..896cff9c7675 100644
--- a/Documentation/driver-api/mtd/spi-nor.rst
+++ b/Documentation/driver-api/mtd/spi-nor.rst
@@ -335,3 +335,40 @@ section, after the ``---`` marker.
------------------+----------+---------
00000000-0000ffff | locked | 1
00010000-03ffffff | unlocked | 1023
+
+ If the flash features a Complement (CMP) bit, we can protect with
+ more granularity above half of the capacity. Let's lock all but one
+ block, then unlock one more block::
+
+ root@1:~# all_but_one=$((($size / $bs) - ($ss / $bs)))
+
+ root@1:~# flash_lock -u /dev/mtd0
+ root@1:~# flash_lock -l /dev/mtd0 $ss $all_but_one # all but the first
+ root@1:~# show_sectors
+ software locked sectors
+ region (in hex) | status | #sectors
+ ------------------+----------+---------
+ 00000000-0000ffff | unlocked | 1
+ 00010000-03ffffff | locked | 1023
+ root@1:~# flash_lock -u /dev/mtd0 $ss $(($ss / $bs)) # all but the two first
+ root@1:~# show_sectors
+ software locked sectors
+ region (in hex) | status | #sectors
+ ------------------+----------+---------
+ 00000000-0001ffff | unlocked | 2
+ 00020000-03ffffff | locked | 1022
+ root@1:~# flash_lock -u /dev/mtd0
+ root@1:~# flash_lock -l /dev/mtd0 0 $all_but_one # same from the other side
+ root@1:~# show_sectors
+ software locked sectors
+ region (in hex) | status | #sectors
+ ------------------+----------+---------
+ 00000000-03feffff | locked | 1023
+ 03ff0000-03ffffff | unlocked | 1
+ root@1:~# flash_lock -u /dev/mtd0 $(($size - (2 * $ss))) $(($ss / $bs)) # all but two
+ root@1:~# show_sectors
+ software locked sectors
+ region (in hex) | status | #sectors
+ ------------------+----------+---------
+ 00000000-03fdffff | locked | 1022
+ 03fe0000-03ffffff | unlocked | 2
--
2.53.0
^ permalink raw reply related
* [PATCH v5 21/28] mtd: spi-nor: swp: Add support for the complement feature
From: Miquel Raynal @ 2026-05-07 16:47 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
Tudor Ambarus, Shuah Khan
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>
The current locking implementation allows to select a power of two
number of blocks, which is going to be the protected amount, as well as
telling whether this is the data at the top (end of the device) or the
bottom (beginning of the device). This means at most we can cover half
of the device or the entire device, but nothing in between.
The complement feature allows a much finer grain of configuration, by
allowing to invert what is considered locked and unlocked.
Add support for this feature. The only known position for the CMP bit is
bit 6 of the configuration register.
The locking and unlocking logics are kept unchanged if the CMP bit is
unavailable. Otherwise, once the regular logic has been applied, we
check if we already found an optimal configuration. If not, we try with
the CMP bit set. If the coverage is closer to the request, we use it.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/spi-nor/core.c | 3 +
drivers/mtd/spi-nor/core.h | 4 +
drivers/mtd/spi-nor/debugfs.c | 1 +
drivers/mtd/spi-nor/swp.c | 202 +++++++++++++++++++++++++++++++++++-------
include/linux/mtd/spi-nor.h | 1 +
5 files changed, 179 insertions(+), 32 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index cdca0fd881a3..e78f709d3f5a 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2974,6 +2974,9 @@ static void spi_nor_init_flags(struct spi_nor *nor)
nor->flags |= SNOR_F_HAS_SR_BP3_BIT6;
}
+ if (flags & SPI_NOR_HAS_CMP)
+ nor->flags |= SNOR_F_HAS_SR2_CMP_BIT6;
+
if (flags & SPI_NOR_RWW && nor->params->n_banks > 1 &&
!nor->controller_ops)
nor->flags |= SNOR_F_RWW;
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 552e734c7107..3ec36103ebab 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -141,6 +141,7 @@ enum spi_nor_option_flags {
SNOR_F_ECC = BIT(15),
SNOR_F_NO_WP = BIT(16),
SNOR_F_SWAP16 = BIT(17),
+ SNOR_F_HAS_SR2_CMP_BIT6 = BIT(18),
};
struct spi_nor_read_command {
@@ -491,6 +492,8 @@ struct spi_nor_id {
* SPI_NOR_NO_ERASE: no erase command needed.
* SPI_NOR_QUAD_PP: flash supports Quad Input Page Program.
* SPI_NOR_RWW: flash supports reads while write.
+ * SPI_NOR_HAS_CMP: flash SR2 has complement (CMP) protect bit. Must
+ * be used with SPI_NOR_HAS_LOCK.
*
* @no_sfdp_flags: flags that indicate support that can be discovered via SFDP.
* Used when SFDP tables are not defined in the flash. These
@@ -539,6 +542,7 @@ struct flash_info {
#define SPI_NOR_NO_ERASE BIT(6)
#define SPI_NOR_QUAD_PP BIT(8)
#define SPI_NOR_RWW BIT(9)
+#define SPI_NOR_HAS_CMP BIT(10)
u8 no_sfdp_flags;
#define SPI_NOR_SKIP_SFDP BIT(0)
diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
index a07e879bfa25..635ed1cfb3fa 100644
--- a/drivers/mtd/spi-nor/debugfs.c
+++ b/drivers/mtd/spi-nor/debugfs.c
@@ -30,6 +30,7 @@ static const char *const snor_f_names[] = {
SNOR_F_NAME(ECC),
SNOR_F_NAME(NO_WP),
SNOR_F_NAME(SWAP16),
+ SNOR_F_NAME(HAS_SR2_CMP_BIT6),
};
#undef SNOR_F_NAME
diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index 5d6c3afa36e3..6b301d04ff23 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -34,6 +34,15 @@ static u8 spi_nor_get_sr_tb_mask(struct spi_nor *nor)
return 0;
}
+static u8 spi_nor_get_sr_cmp_mask(struct spi_nor *nor)
+{
+ if (!(nor->flags & SNOR_F_NO_READ_CR) &&
+ nor->flags & SNOR_F_HAS_SR2_CMP_BIT6)
+ return SR2_CMP_BIT6;
+ else
+ return 0;
+}
+
u64 spi_nor_get_min_prot_length_sr(struct spi_nor *nor)
{
unsigned int bp_slots, bp_slots_needed;
@@ -61,8 +70,10 @@ void spi_nor_get_locked_range_sr(struct spi_nor *nor, const u8 *sr, loff_t *ofs,
u64 min_prot_len;
u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
+ u8 cmp_mask = spi_nor_get_sr_cmp_mask(nor);
u8 bp, val = sr[0] & bp_mask;
bool tb = (nor->flags & SNOR_F_HAS_SR_TB) ? sr[0] & tb_mask : 0;
+ bool cmp = sr[1] & cmp_mask;
if (nor->flags & SNOR_F_HAS_SR_BP3_BIT6 && val & SR_BP3_BIT6)
val = (val & ~SR_BP3_BIT6) | SR_BP3;
@@ -70,22 +81,37 @@ void spi_nor_get_locked_range_sr(struct spi_nor *nor, const u8 *sr, loff_t *ofs,
bp = val >> SR_BP_SHIFT;
if (!bp) {
- /* No protection */
- *ofs = 0;
- *len = 0;
- return;
+ if (!cmp) {
+ /* No protection */
+ *ofs = 0;
+ *len = 0;
+ return;
+ } else {
+ /* Full protection */
+ *ofs = 0;
+ *len = nor->params->size;
+ }
}
min_prot_len = spi_nor_get_min_prot_length_sr(nor);
*len = min_prot_len << (bp - 1);
-
if (*len > nor->params->size)
*len = nor->params->size;
- if (tb)
- *ofs = 0;
- else
- *ofs = nor->params->size - *len;
+ if (cmp)
+ *len = nor->params->size - *len;
+
+ if (!cmp) {
+ if (tb)
+ *ofs = 0;
+ else
+ *ofs = nor->params->size - *len;
+ } else {
+ if (tb)
+ *ofs = nor->params->size - *len;
+ else
+ *ofs = 0;
+ }
}
/*
@@ -142,13 +168,15 @@ static int spi_nor_sr_set_bp_mask(struct spi_nor *nor, u8 *sr, u8 pow)
}
static int spi_nor_build_sr(struct spi_nor *nor, const u8 *old_sr, u8 *new_sr,
- u8 pow, bool use_top)
+ u8 pow, bool use_top, bool cmp)
{
u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
+ u8 cmp_mask = spi_nor_get_sr_cmp_mask(nor);
int ret;
new_sr[0] = old_sr[0] & ~bp_mask & ~tb_mask;
+ new_sr[1] = old_sr[1] & ~cmp_mask;
/* Build BP field */
ret = spi_nor_sr_set_bp_mask(nor, &new_sr[0], pow);
@@ -156,9 +184,13 @@ static int spi_nor_build_sr(struct spi_nor *nor, const u8 *old_sr, u8 *new_sr,
return ret;
/* Build TB field */
- if (!use_top)
+ if ((!cmp && !use_top) || (cmp && use_top))
new_sr[0] |= tb_mask;
+ /* Build CMP field */
+ if (cmp)
+ new_sr[1] |= cmp_mask;
+
return 0;
}
@@ -170,15 +202,22 @@ void spi_nor_cache_sr_lock_bits(struct spi_nor *nor, u8 *sr)
{
u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
+ u8 cmp_mask = spi_nor_get_sr_cmp_mask(nor);
if (!sr) {
if (spi_nor_read_sr(nor, nor->bouncebuf))
return;
+ if (!(nor->flags & SNOR_F_NO_READ_CR)) {
+ if (spi_nor_read_cr(nor, nor->bouncebuf + 1))
+ return;
+ }
+
sr = nor->bouncebuf;
}
nor->dfs_sr_cache[0] = sr[0] & (bp_mask | tb_mask | SR_SRWD);
+ nor->dfs_sr_cache[1] = sr[1] & cmp_mask;
}
/*
@@ -187,10 +226,11 @@ void spi_nor_cache_sr_lock_bits(struct spi_nor *nor, u8 *sr)
* register
* (SR). Does not support these features found in newer SR bitfields:
* - SEC: sector/block protect - only handle SEC=0 (block protect)
- * - CMP: complement protect - only support CMP=0 (range is not complemented)
*
* Support for the following is provided conditionally for some flash:
* - TB: top/bottom protect
+ * - CMP: complement protect (BP and TP describe the unlocked part, while
+ * the reminder is locked)
*
* Sample table portion for 8MB flash (Winbond w25q64fw):
*
@@ -217,11 +257,13 @@ void spi_nor_cache_sr_lock_bits(struct spi_nor *nor, u8 *sr)
static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
{
u64 min_prot_len = spi_nor_get_min_prot_length_sr(nor);
- u8 status_old[1] = {}, status_new[1] = {};
- loff_t ofs_old, ofs_new;
- u64 len_old, len_new;
+ u8 status_old[2] = {}, status_new[2] = {}, status_new_cmp[2] = {};
+ u8 *best_status_new = status_new;
+ loff_t ofs_old, ofs_new, ofs_new_cmp;
+ u64 len_old, len_new, len_new_cmp;
loff_t lock_len;
- bool can_be_top = true, can_be_bottom = nor->flags & SNOR_F_HAS_SR_TB;
+ bool can_be_top = true, can_be_bottom = nor->flags & SNOR_F_HAS_SR_TB,
+ can_be_cmp = spi_nor_get_sr_cmp_mask(nor);
bool use_top;
int ret;
u8 pow;
@@ -232,6 +274,14 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
status_old[0] = nor->bouncebuf[0];
+ if (!(nor->flags & SNOR_F_NO_READ_CR)) {
+ ret = spi_nor_read_cr(nor, nor->bouncebuf + 1);
+ if (ret)
+ return ret;
+
+ status_old[1] = nor->bouncebuf[1];
+ }
+
/* 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))
return 0;
@@ -262,24 +312,56 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
else
pow = ilog2(lock_len) - ilog2(min_prot_len) + 1;
- ret = spi_nor_build_sr(nor, status_old, status_new, pow, use_top);
+ ret = spi_nor_build_sr(nor, status_old, status_new, pow, use_top, false);
if (ret)
return ret;
+ /*
+ * In case the region asked is not fully met, maybe we can try with the
+ * complement feature
+ */
+ spi_nor_get_locked_range_sr(nor, status_new, &ofs_new, &len_new);
+ if (can_be_cmp && len_new != lock_len) {
+ pow = ilog2(nor->params->size - lock_len) - ilog2(min_prot_len) + 1;
+ ret = spi_nor_build_sr(nor, status_old, status_new_cmp, pow, use_top, true);
+ if (ret)
+ return ret;
+
+ /*
+ * ilog2() "floors" the result, which means in some cases we may have to
+ * manually reduce the scope when the complement feature is used.
+ * The uAPI is to never lock more than what is requested, but less is accepted.
+ * Make sure we are not covering a too wide range, reduce it otherwise.
+ */
+ spi_nor_get_locked_range_sr(nor, status_new_cmp, &ofs_new_cmp, &len_new_cmp);
+ if (len_new_cmp > lock_len) {
+ pow++;
+ ret = spi_nor_build_sr(nor, status_old, status_new_cmp, pow, use_top, true);
+ if (ret)
+ return ret;
+ }
+
+ /* Pick the CMP configuration if we cover a closer range */
+ spi_nor_get_locked_range_sr(nor, status_new, &ofs_new, &len_new);
+ spi_nor_get_locked_range_sr(nor, status_new_cmp, &ofs_new_cmp, &len_new_cmp);
+ if (len_new_cmp > len_new)
+ best_status_new = status_new_cmp;
+ }
+
/*
* Disallow further writes if WP# pin is neither left floating nor
* wrongly tied to GND (that includes internal pull-downs).
* WP# pin hard strapped to GND can be a valid use case.
*/
if (!(nor->flags & SNOR_F_NO_WP))
- status_new[0] |= SR_SRWD;
+ best_status_new[0] |= SR_SRWD;
/* Don't bother if they're the same */
- if (status_new[0] == status_old[0])
+ if (best_status_new[0] == status_old[0] && best_status_new[1] == status_old[1])
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);
+ spi_nor_get_locked_range_sr(nor, best_status_new, &ofs_new, &len_new);
/* Don't "lock" with no region! */
if (!len_new)
@@ -290,11 +372,14 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
(ofs_old < ofs_new || (ofs_new + len_new) < (ofs_old + len_old)))
return -EINVAL;
- ret = spi_nor_write_sr_and_check(nor, status_new[0]);
+ if (nor->flags & SNOR_F_NO_READ_CR)
+ ret = spi_nor_write_sr_and_check(nor, best_status_new[0]);
+ else
+ ret = spi_nor_write_sr_cr_and_check(nor, best_status_new);
if (ret)
return ret;
- spi_nor_cache_sr_lock_bits(nor, status_new);
+ spi_nor_cache_sr_lock_bits(nor, best_status_new);
return 0;
}
@@ -308,11 +393,13 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
{
u64 min_prot_len = spi_nor_get_min_prot_length_sr(nor);
int ret;
- u8 status_old[1], status_new[1];
- loff_t ofs_old, ofs_new;
- u64 len_old, len_new;
+ u8 status_old[2], status_new[2], status_new_cmp[2];
+ u8 *best_status_new = status_new;
+ loff_t ofs_old, ofs_new, ofs_new_cmp;
+ u64 len_old, len_new, len_new_cmp;
loff_t lock_len;
- bool can_be_top = true, can_be_bottom = nor->flags & SNOR_F_HAS_SR_TB;
+ bool can_be_top = true, can_be_bottom = nor->flags & SNOR_F_HAS_SR_TB,
+ can_be_cmp = spi_nor_get_sr_cmp_mask(nor);
bool use_top;
u8 pow;
@@ -322,6 +409,14 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
status_old[0] = nor->bouncebuf[0];
+ if (!(nor->flags & SNOR_F_NO_READ_CR)) {
+ ret = spi_nor_read_cr(nor, nor->bouncebuf + 1);
+ if (ret)
+ return ret;
+
+ status_old[1] = nor->bouncebuf[1];
+ }
+
/* 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))
return 0;
@@ -359,30 +454,65 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
else
pow = ilog2(lock_len) - ilog2(min_prot_len) + 1;
- ret = spi_nor_build_sr(nor, status_old, status_new, pow, use_top);
+ ret = spi_nor_build_sr(nor, status_old, status_new, pow, use_top, false);
if (ret)
return ret;
+ /*
+ * In case the region asked is not fully met, maybe we can try with the
+ * complement feature
+ */
+ spi_nor_get_locked_range_sr(nor, status_new, &ofs_new, &len_new);
+ if (can_be_cmp && len_new != lock_len) {
+ pow = ilog2(nor->params->size - lock_len) - ilog2(min_prot_len) + 1;
+ ret = spi_nor_build_sr(nor, status_old, status_new_cmp, pow, use_top, true);
+ if (ret)
+ return ret;
+
+ /*
+ * ilog2() "floors" the result, which means in some cases we may have to
+ * manually reduce the scope when the complement feature is used.
+ * The uAPI is to never unlock more than what is requested, but less is accepted.
+ * Make sure we are not covering a too small range, increase it otherwise.
+ */
+ spi_nor_get_locked_range_sr(nor, status_new_cmp, &ofs_new_cmp, &len_new_cmp);
+ if (len_new_cmp < lock_len) {
+ pow--;
+ ret = spi_nor_build_sr(nor, status_old, status_new_cmp, pow, use_top, true);
+ if (ret)
+ return ret;
+ }
+
+ /* Pick the CMP configuration if we cover a closer range */
+ spi_nor_get_locked_range_sr(nor, status_new, &ofs_new, &len_new);
+ spi_nor_get_locked_range_sr(nor, status_new_cmp, &ofs_new_cmp, &len_new_cmp);
+ if (len_new_cmp > len_new)
+ best_status_new = status_new_cmp;
+ }
+
/* Don't protect status register if we're fully unlocked */
if (lock_len == 0)
- status_new[0] &= ~SR_SRWD;
+ best_status_new[0] &= ~SR_SRWD;
/* Don't bother if they're the same */
- if (status_new[0] == status_old[0])
+ if (best_status_new[0] == status_old[0] && best_status_new[1] == status_old[1])
return 0;
/* Only modify protection if it will not lock other areas */
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);
+ spi_nor_get_locked_range_sr(nor, best_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;
- ret = spi_nor_write_sr_and_check(nor, status_new[0]);
+ if (nor->flags & SNOR_F_NO_READ_CR)
+ ret = spi_nor_write_sr_and_check(nor, best_status_new[0]);
+ else
+ ret = spi_nor_write_sr_cr_and_check(nor, best_status_new);
if (ret)
return ret;
- spi_nor_cache_sr_lock_bits(nor, status_new);
+ spi_nor_cache_sr_lock_bits(nor, best_status_new);
return 0;
}
@@ -402,6 +532,14 @@ static int spi_nor_sr_is_locked(struct spi_nor *nor, loff_t ofs, u64 len)
if (ret)
return ret;
+ if (!(nor->flags & SNOR_F_NO_READ_CR)) {
+ ret = spi_nor_read_cr(nor, nor->bouncebuf + 1);
+ if (ret)
+ return ret;
+ } else {
+ nor->bouncebuf[1] = 0;
+ }
+
return spi_nor_is_locked_sr(nor, ofs, len, nor->bouncebuf);
}
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 9ad77f9e76c2..4b92494827b1 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -125,6 +125,7 @@
#define SR2_LB1 BIT(3) /* Security Register Lock Bit 1 */
#define SR2_LB2 BIT(4) /* Security Register Lock Bit 2 */
#define SR2_LB3 BIT(5) /* Security Register Lock Bit 3 */
+#define SR2_CMP_BIT6 BIT(6)
#define SR2_QUAD_EN_BIT7 BIT(7)
/* Supported SPI protocols */
--
2.53.0
^ permalink raw reply related
* [PATCH v5 20/28] mtd: spi-nor: Add steps for testing locking support
From: Miquel Raynal @ 2026-05-07 16:47 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
Tudor Ambarus, Shuah Khan
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>
As recently raised on the mailing list, it may be useful to propose a
list of steps to go through in order to proove the devices have been
described correctly, especially since all the block protection
information is not stored in any kind of table and is instead filled
manually by developpers.
Use the debugfs output to ease the comparison between expectations and
reality.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
Documentation/driver-api/mtd/spi-nor.rst | 132 +++++++++++++++++++++++++++++++
1 file changed, 132 insertions(+)
diff --git a/Documentation/driver-api/mtd/spi-nor.rst b/Documentation/driver-api/mtd/spi-nor.rst
index 148fa4288760..b42de32c6eba 100644
--- a/Documentation/driver-api/mtd/spi-nor.rst
+++ b/Documentation/driver-api/mtd/spi-nor.rst
@@ -203,3 +203,135 @@ section, after the ``---`` marker.
mtd.writesize = 1
mtd.oobsize = 0
regions = 0
+
+5) If your flash supports locking, please go through the following test
+ procedure to make sure it correctly behaves. The below example
+ expects the typical situation where eraseblocks and lock sectors have
+ the same size. In case you enabled MTD_SPI_NOR_USE_4K_SECTORS, you
+ must adapt `bs` accordingly.
+
+ Warning: These tests may hard lock your device! Make sure:
+ - The device is not hard locked already (#WP strapped to low and
+ SR_SRWD bit set)
+ - If you have a WPn pin, you may want to set `no-wp` in your DT for
+ the time of the test, to only make use of software protection.
+ Otherwise, clearing the locking state depends on the WPn
+ signal and if it is tied to low, the flash will be permanently
+ locked.
+
+ Test full chip locking and make sure expectations, the MEMISLOCKED
+ ioctl output, the debugfs output and experimental results are all
+ aligned::
+
+ root@1:~# alias show_sectors='grep -A4 "locked sectors" /sys/kernel/debug/spi-nor/spi0.0/params'
+ root@1:~# flash_lock -u /dev/mtd0
+ root@1:~# flash_lock -i /dev/mtd0
+ Device: /dev/mtd0
+ Start: 0
+ Len: 0x4000000
+ Lock status: unlocked
+ Return code: 0
+ root@1:~# mtd_debug erase /dev/mtd0 0 2097152
+ Erased 2097152 bytes from address 0x00000000 in flash
+ root@1:~# mtd_debug write /dev/mtd0 0 2097152 spi_test
+ Copied 2097152 bytes from spi_test to address 0x00000000 in flash
+ root@1:~# mtd_debug read /dev/mtd0 0 2097152 spi_read
+ Copied 2097152 bytes from address 0x00000000 in flash to spi_read
+ root@1:~# sha256sum spi*
+ c444216a6ba2a4a66cccd60a0dd062bce4b865dd52b200ef5e21838c4b899ac8 spi_read
+ c444216a6ba2a4a66cccd60a0dd062bce4b865dd52b200ef5e21838c4b899ac8 spi_test
+ root@1:~# show_sectors
+ software locked sectors
+ region (in hex) | status | #sectors
+ ------------------+----------+---------
+ 00000000-03ffffff | unlocked | 1024
+
+ root@1:~# flash_lock -l /dev/mtd0
+ root@1:~# flash_lock -i /dev/mtd0
+ Device: /dev/mtd0
+ Start: 0
+ Len: 0x4000000
+ Lock status: locked
+ Return code: 1
+ root@1:~# mtd_debug erase /dev/mtd0 0 2097152
+ Erased 2097152 bytes from address 0x00000000 in flash
+ root@1:~# mtd_debug read /dev/mtd0 0 2097152 spi_read
+ Copied 2097152 bytes from address 0x00000000 in flash to spi_read
+ root@1:~# sha256sum spi*
+ c444216a6ba2a4a66cccd60a0dd062bce4b865dd52b200ef5e21838c4b899ac8 spi_read
+ c444216a6ba2a4a66cccd60a0dd062bce4b865dd52b200ef5e21838c4b899ac8 spi_test
+ root@1:~# dd if=/dev/urandom of=./spi_test2 bs=1M count=2
+ 2+0 records in
+ 2+0 records out
+ root@1:~# mtd_debug write /dev/mtd0 0 2097152 spi_test2
+ Copied 2097152 bytes from spi_test to address 0x00000000 in flash
+ root@1:~# mtd_debug read /dev/mtd0 0 2097152 spi_read2
+ Copied 2097152 bytes from address 0x00000000 in flash to spi_read
+ root@1:~# sha256sum spi*
+ c444216a6ba2a4a66cccd60a0dd062bce4b865dd52b200ef5e21838c4b899ac8 spi_read
+ c444216a6ba2a4a66cccd60a0dd062bce4b865dd52b200ef5e21838c4b899ac8 spi_read2
+ c444216a6ba2a4a66cccd60a0dd062bce4b865dd52b200ef5e21838c4b899ac8 spi_test
+ bea9334df51c620440f86751cba0799214a016329f1736f9456d40cf40efdc88 spi_test2
+ root@1:~# show_sectors
+ software locked sectors
+ region (in hex) | status | #sectors
+ ------------------+----------+---------
+ 00000000-03ffffff | locked | 1024
+ root@1:~# flash_lock -u /dev/mtd0
+
+ Once we trust the debugfs output we can use it to test various
+ situations. Check top locking/unlocking (end of the device)::
+
+ root@1:~# size=$(cat /sys/class/mtd/mtd0/size)
+ root@1:~# bs=$(cat /sys/class/mtd/mtd0/erasesize)
+ root@1:~# nsectors=$(grep unlocked /sys/kernel/debug/spi-nor/spi0.0/params | sed -e 's/.*unlocked | //')
+ root@1:~# ss=$(($size / $nsectors))
+ root@1:~# bps=$(($ss / $bs))
+
+ root@1:~# flash_lock -u /dev/mtd0
+ root@1:~# flash_lock -l /dev/mtd0 $(($size - (2 * $ss))) $((2 * $bps)) # last two
+ root@1:~# show_sectors
+ software locked sectors
+ region (in hex) | status | #sectors
+ ------------------+----------+---------
+ 00000000-03fdffff | unlocked | 1022
+ 03fe0000-03ffffff | locked | 2
+ root@1:~# flash_lock -u /dev/mtd0 $(($size - (2 * $ss))) $((1 * $bps)) # last one
+ root@1:~# show_sectors
+ software locked sectors
+ region (in hex) | status | #sectors
+ ------------------+----------+---------
+ 00000000-03feffff | unlocked | 1023
+ 03ff0000-03ffffff | locked | 1
+
+ If the flash features 4 block protection bits (BP), we can protect
+ more than 4MB (typically 128 64kiB-blocks or more), with a finer
+ grain than locking the entire device::
+
+ root@1:~# flash_lock -u /dev/mtd0
+ root@1:~# flash_lock -l /dev/mtd0 $(($size - (2**7 * $ss))) $((2**7 * $bps))
+ root@1:~# show_sectors
+ software locked sectors
+ region (in hex) | status | #sectors
+ ------------------+----------+---------
+ 00000000-037fffff | unlocked | 896
+ 03800000-03ffffff | locked | 128
+
+ If the flash features a Top/Bottom (TB) bit, we can protect the
+ beginning of the flash::
+
+ root@1:~# flash_lock -u /dev/mtd0
+ root@1:~# flash_lock -l /dev/mtd0 0 $((2 * $bps)) # first two
+ root@1:~# show_sectors
+ software locked sectors
+ region (in hex) | status | #sectors
+ ------------------+----------+---------
+ 00000000-0001ffff | locked | 2
+ 00020000-03ffffff | unlocked | 1022
+ root@1:~# flash_lock -u /dev/mtd0 $ss $((1 * $bps)) # first one
+ root@1:~# show_sectors
+ software locked sectors
+ region (in hex) | status | #sectors
+ ------------------+----------+---------
+ 00000000-0000ffff | locked | 1
+ 00010000-03ffffff | unlocked | 1023
--
2.53.0
^ permalink raw reply related
* [PATCH v5 19/28] mtd: spi-nor: debugfs: Add a locked sectors map
From: Miquel Raynal @ 2026-05-07 16:47 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
Tudor Ambarus, Shuah Khan
Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>
In order to get a very clear view of the sectors being locked, besides
the `params` output giving the ranges, we may want to see a proper map
of the sectors and for each of them, their status. Depending on the use
case, this map may be easier to parse by humans and gives a more acurate
feeling of the situation. At least myself, for the few locking-related
developments I recently went through, I found it very useful to get a
clearer mental model of what was locked/unlocked.
Here is an example of output:
$ cat /sys/kernel/debug/spi-nor/spi0.0/locked-sectors-map
Locked sectors map (x: locked, .: unlocked, unit: 64kiB)
0x00000000 (# 0): ................ ................ ................ ................
0x00400000 (# 64): ................ ................ ................ ................
0x00800000 (# 128): ................ ................ ................ ................
0x00c00000 (# 192): ................ ................ ................ ................
0x01000000 (# 256): ................ ................ ................ ................
0x01400000 (# 320): ................ ................ ................ ................
0x01800000 (# 384): ................ ................ ................ ................
0x01c00000 (# 448): ................ ................ ................ ................
0x02000000 (# 512): ................ ................ ................ ................
0x02400000 (# 576): ................ ................ ................ ................
0x02800000 (# 640): ................ ................ ................ ................
0x02c00000 (# 704): ................ ................ ................ ................
0x03000000 (# 768): ................ ................ ................ ................
0x03400000 (# 832): ................ ................ ................ ................
0x03800000 (# 896): ................ ................ ................ ................
0x03c00000 (# 960): ................ ................ ................ ..............xx
The output is wrapped at 64 sectors, spaces every 16 sectors are
improving the readability, every line starts by the first sector
offset (hex) and number (decimal).
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/spi-nor/debugfs.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
index 36ec35d6b2dc..a07e879bfa25 100644
--- a/drivers/mtd/spi-nor/debugfs.c
+++ b/drivers/mtd/spi-nor/debugfs.c
@@ -190,6 +190,40 @@ static int spi_nor_params_show(struct seq_file *s, void *data)
}
DEFINE_SHOW_ATTRIBUTE(spi_nor_params);
+static int spi_nor_locked_sectors_map_show(struct seq_file *s, void *data)
+{
+ struct spi_nor *nor = s->private;
+ struct spi_nor_flash_parameter *params = nor->params;
+ unsigned int min_prot_len = spi_nor_get_min_prot_length_sr(nor);
+ unsigned int offset = 0, sector = 0;
+ bool locked;
+ int i;
+
+ seq_printf(s, "Locked sectors map (x: locked, .: unlocked, unit: %dkiB)\n",
+ min_prot_len / 1024);
+ while (offset < params->size) {
+ seq_printf(s, " 0x%08x (#%5d): ", offset, sector);
+ for (i = 0; i < 64 && offset < params->size; i++) {
+ locked = spi_nor_is_locked_sr(nor, offset, min_prot_len,
+ nor->dfs_sr_cache);
+ if (locked)
+ seq_puts(s, "x");
+ else
+ seq_puts(s, ".");
+
+ if (((i + 1) % 16) == 0)
+ seq_puts(s, " ");
+
+ offset += min_prot_len;
+ sector++;
+ }
+ seq_puts(s, "\n");
+ }
+
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(spi_nor_locked_sectors_map);
+
static void spi_nor_print_read_cmd(struct seq_file *s, u32 cap,
struct spi_nor_read_command *cmd)
{
@@ -275,6 +309,8 @@ void spi_nor_debugfs_register(struct spi_nor *nor)
debugfs_create_file("params", 0444, d, nor, &spi_nor_params_fops);
debugfs_create_file("capabilities", 0444, d, nor,
&spi_nor_capabilities_fops);
+ if (spi_nor_has_default_locking_ops(nor))
+ debugfs_create_file("locked-sectors-map", 0444, d, nor, &spi_nor_locked_sectors_map_fops);
}
void spi_nor_debugfs_shutdown(void)
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v5 00/13] ima: Introduce staging mechanism
From: steven chen @ 2026-05-07 16:47 UTC (permalink / raw)
To: Roberto Sassu, corbet, skhan, zohar, dmitry.kasatkin,
eric.snowberg, paul, jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, nramas, Roberto Sassu, steven chen
In-Reply-To: <20260429160319.4162918-1-roberto.sassu@huaweicloud.com>
On 4/29/2026 9:03 AM, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Introduction
> ============
>
> The IMA measurements list is currently stored in the kernel memory.
> Memory occupation grows linearly with the number of entries, and can
> become a problem especially in environments with reduced resources.
>
> While there is an advantage in keeping the IMA measurements list in
> kernel memory, so that it is always available for reading from the
> securityfs interfaces, storing it elsewhere would make it possible to
> free precious memory for other kernel components.
>
> Storing the IMA measurements list outside the kernel does not introduce
> security issues, since its integrity is anyway protected by the TPM.
>
> Hence, the new IMA staging mechanism is introduced to allow user space
> to remove the desired portion of the measurements list from the kernel.
>
>
> Usage
> =====
>
> The IMA staging mechanism can be enabled from the kernel configuration
> with the CONFIG_IMA_STAGING option.
>
> If it is enabled, IMA duplicates the current measurements interfaces
> (both binary and ASCII), by adding the _staged file suffix. Both the
> original and the staging interfaces gain the write permission for the
> root user and group, but require the process to have CAP_SYS_ADMIN set.
>
> The staging mechanism supports two flavors.
>
> Staging with prompt
> ~~~~~~~~~~~~~~~~~~~
>
> The current measurements list is moved to a temporary staging area, and
> staged measurements are deleted upon confirmation.
>
> This staging process is achieved with the following steps.
>
> 1. echo A > <original interface>: the user requests IMA to stage the
> entire measurements list;
> 2. cat <_staged interface>: the user reads the staged measurements;
> 3. echo D > <_staged interface>: the user requests IMA to delete
> staged measurements.
>
> Staging and deleting
> ~~~~~~~~~~~~~~~~~~~~
>
> N measurements are staged to a temporary staging area, and immediately
> deleted without further confirmation.
>
> This staging process is achieved with the following steps.
>
> 1. cat <original interface>: the user reads the current measurements
> list and determines what the value N for staging should be;
> 2. echo N > <original interface>: the user requests IMA to delete N
> measurements from the current measurements list.
This submission proposes two ways for log trimming:
*Favour 1:* Staging with prompt
*Favour 2:* stage and delete N
Functionally, both approaches address the same problem, but *Favour 2
*is the
stronger design and should be preferred. There is no good reason to keep
*Favour 1.*
From a kernel implementation perspective, *Favour 2 *is more efficient
because it
minimizes the time spent holding the list lock (can’t be shorter). It
also substantially
reduces the amount of kernel-side logic, removing nearly half of the
code required
by the alternative approach.
From a user-space perspective, *Favour 2 *results in a much cleaner
model. It avoids
the need to track and reconcile both old and staged lists in user space
as well as
two lists (cur and staged) in the kernel space, which simplifies log
trimming logic
and reduces maintenance overhead. In addition, it preserves the existing
external
behavior by not exposing any staged list to user space.
Overall, *Favour 2 *provides the same functional result with lower
kernel complexity,
shorter kernel list lock hold time, and a simpler user-space interface.
For those
reasons, it is the preferable approach and *Favour 1* does not appear to
offer sufficient
justification to keep both implementations.
Steven
>
> Management of Staged Measurements
> =================================
>
> Since with the staging mechanism measurement entries are removed from
> the kernel, the user needs to save the staged ones in a storage and
> concatenate them together, so that it can present them to remote
> attestation agents as if staging was never done.
>
>
> Patch set content
> =================
>
> Patches 1-8 are preparatory patches to quickly replace the hash table,
> maintain separate counters for the different measurements list types,
> mediate access to the measurements list interface, and simplify the staging
> patches.
>
> Patch 9 introduces the staging with prompt flavor. Patch 10 makes it
> possible to flush the hash table when deleting all the staged measurements.
> Patch 11 introduces the staging and deleting flavor. Patch 12 avoids
> measurements entries to be stored twice if there is contention between the
> measurements interfaces and kexec. Patch 13 adds the documentation of the
> staging mechanism.
>
>
> Changelog
> =========
>
> v4:
> - Add write permission to the original measurement interface, and move
> the A and N staging commands to that interface
> - Explain better the two staging flavors and highlight that the staging
> and delete only stages measurements internally
> - Rename ima_queue_staged_delete_partial() to ima_queue_delete_partial()
> - Replace ima_staged_measurements_prepended with per measurements list
> flag to avoid copying staged and active list measurements twice
> - Optimize the staging and deleting flavor by locklessly determining the
> cut position in the active list, and immediately deleting entries
> without explicit staging and splicing (suggested by Steven Chen)
>
> v3:
> - Add Kconfig option to enable the staging mechanism (suggested by Mimi)
> - Change the meaning of BINARY_STAGED to be just the staged measurements
> - Separate the two staging flavors in two different functions:
> ima_queue_staged_delete_all() for staging with prompt,
> ima_queue_staged_delete_partial() for staging and deleting
> - Delete N entries without staging first (suggested by Mimi)
> - Avoid duplicate staged entries if there is contention between the
> measurements list interfaces and kexec
>
> v2:
> - New patch to move measurements and violation counters outside the
> ima_h_table structure
> - New patch to quickly replace the hash table
> - Forbid partial deletion when flushing hash table (suggested by Mimi)
> - Ignore ima_flush_htable if CONFIG_IMA_DISABLE_HTABLE is enabled
> - BINARY_SIZE_* renamed to BINARY_* for better clarity
> - Removed ima_measurements_staged_exist and testing list empty instead
> - ima_queue_stage_trim() and ima_queue_delete_staged_trimmed() renamed to
> ima_queue_stage() and ima_queue_delete_staged()
> - New delete interval [1, ULONG_MAX - 1]
> - Rename ima_measure_lock to ima_measure_mutex
> - Move seq_open() and seq_release() outside the ima_measure_mutex lock
> - Drop ima_measurements_staged_read() and use seq_read() instead
> - Optimize create_securityfs_measurement_lists() changes
> - New file name format with _staged suffix at the end of the file name
> - Use _rcu list variant in ima_dump_measurement_list()
> - Remove support for direct trimming and splice the remaining entries to
> the active list (suggested by Mimi)
> - Hot swap the hash table if flushing is requested
>
> v1:
> - Support for direct trimming without staging
> - Support unstaging on kexec (requested by Gregory Lumen)
>
> Roberto Sassu (13):
> ima: Remove ima_h_table structure
> ima: Replace static htable queue with dynamically allocated array
> ima: Introduce per binary measurements list type ima_num_entries
> counter
> ima: Introduce per binary measurements list type binary_runtime_size
> value
> ima: Introduce _ima_measurements_start() and _ima_measurements_next()
> ima: Mediate open/release method of the measurements list
> ima: Use snprintf() in create_securityfs_measurement_lists
> ima: Introduce ima_dump_measurement()
> ima: Add support for staging measurements with prompt
> ima: Add support for flushing the hash table when staging measurements
> ima: Support staging and deleting N measurements entries
> ima: Return error on deleting measurements already copied during kexec
> doc: security: Add documentation of the IMA staging mechanism
>
> .../admin-guide/kernel-parameters.txt | 4 +
> Documentation/security/IMA-staging.rst | 163 +++++++++
> Documentation/security/index.rst | 1 +
> MAINTAINERS | 2 +
> security/integrity/ima/Kconfig | 16 +
> security/integrity/ima/ima.h | 32 +-
> security/integrity/ima/ima_api.c | 2 +-
> security/integrity/ima/ima_fs.c | 315 ++++++++++++++++--
> security/integrity/ima/ima_init.c | 5 +
> security/integrity/ima/ima_kexec.c | 53 ++-
> security/integrity/ima/ima_queue.c | 283 ++++++++++++++--
> 11 files changed, 803 insertions(+), 73 deletions(-)
> create mode 100644 Documentation/security/IMA-staging.rst
>
^ 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