date:20231025

From: Nathan Lynch 

Consistently testing system parameter access is a bit difficult by
nature -- the set of parameters available depends on the model and
system configuration, and updating a parameter should be considered a
destructive operation reserved for the admin.

So we validate some of the error paths and retrieve the SPLPAR
characteristics string, but not much else.

Signed-off-by: Nathan Lynch 
---
 tools/testing/selftests/powerpc/Makefile   |   1 +
 .../selftests/powerpc/papr_sysparm/.gitignore  |   1 +
 .../selftests/powerpc/papr_sysparm/Makefile|  12 ++
 .../selftests/powerpc/papr_sysparm/papr_sysparm.c  | 164 +
 4 files changed, 178 insertions(+)

diff --git a/tools/testing/selftests/powerpc/Makefile 
b/tools/testing/selftests/powerpc/Makefile
index 05fc68d446c2..c376151982c4 100644
--- a/tools/testing/selftests/powerpc/Makefile
+++ b/tools/testing/selftests/powerpc/Makefile
@@ -33,6 +33,7 @@ SUB_DIRS = alignment  \
   math \
   papr_attributes  \
   papr_vpd \
+  papr_sysparm \
   ptrace   \
   security \
   mce
diff --git a/tools/testing/selftests/powerpc/papr_sysparm/.gitignore 
b/tools/testing/selftests/powerpc/papr_sysparm/.gitignore
new file mode 100644
index ..f2a69bf59d40
--- /dev/null
+++ b/tools/testing/selftests/powerpc/papr_sysparm/.gitignore
@@ -0,0 +1 @@
+/papr_sysparm
diff --git a/tools/testing/selftests/powerpc/papr_sysparm/Makefile 
b/tools/testing/selftests/powerpc/papr_sysparm/Makefile
new file mode 100644
index ..7f79e437634a
--- /dev/null
+++ b/tools/testing/selftests/powerpc/papr_sysparm/Makefile
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0
+noarg:
+   $(MAKE) -C ../
+
+TEST_GEN_PROGS := papr_sysparm
+
+top_srcdir = ../../../../..
+include ../../lib.mk
+
+$(TEST_GEN_PROGS): ../harness.c ../utils.c
+
+$(OUTPUT)/papr_sysparm: CFLAGS += $(KHDR_INCLUDES)
diff --git a/tools/testing/selftests/powerpc/papr_sysparm/papr_sysparm.c 
b/tools/testing/selftests/powerpc/papr_sysparm/papr_sysparm.c
new file mode 100644
index ..fc25c03e8bc7
--- /dev/null
+++ b/tools/testing/selftests/powerpc/papr_sysparm/papr_sysparm.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "utils.h"
+
+#define DEVPATH "/dev/papr-sysparm"
+
+static int open_close(void)
+{
+   const int devfd = open(DEVPATH, O_RDONLY);
+
+   SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+   DEVPATH " not present");
+
+   FAIL_IF(devfd < 0);
+   FAIL_IF(close(devfd) != 0);
+
+   return 0;
+}
+
+static int get_splpar(void)
+{
+   struct papr_sysparm_io_block sp = {
+   .parameter = 20, // SPLPAR characteristics
+   };
+   const int devfd = open(DEVPATH, O_RDONLY);
+
+   SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+   DEVPATH " not present");
+
+   FAIL_IF(devfd < 0);
+   FAIL_IF(ioctl(devfd, PAPR_SYSPARM_IOC_GET, ) != 0);
+   FAIL_IF(sp.length == 0);
+   FAIL_IF(sp.length > sizeof(sp.data));
+   FAIL_IF(close(devfd) != 0);
+
+   return 0;
+}
+
+static int get_bad_parameter(void)
+{
+   struct papr_sysparm_io_block sp = {
+   .parameter = UINT32_MAX, // there are only ~60 specified 
parameters
+   };
+   const int devfd = open(DEVPATH, O_RDONLY);
+
+   SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+   DEVPATH " not present");
+
+   FAIL_IF(devfd < 0);
+
+   // Ensure expected error
+   FAIL_IF(ioctl(devfd, PAPR_SYSPARM_IOC_GET, ) != -1);
+   FAIL_IF(errno != EOPNOTSUPP);
+
+   // Ensure the buffer is unchanged
+   FAIL_IF(sp.length != 0);
+   for (size_t i = 0; i < ARRAY_SIZE(sp.data); ++i)
+   FAIL_IF(sp.data[i] != 0);
+
+   FAIL_IF(close(devfd) != 0);
+
+   return 0;
+}
+
+static int check_efault_common(unsigned long cmd)
+{
+   const int devfd = open(DEVPATH, O_RDONLY);
+
+   SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+   DEVPATH " not present");
+
+   FAIL_IF(devfd < 0);
+
+   // Ensure expected error
+   FAIL_IF(ioctl(devfd, cmd, NULL) != -1);
+   FAIL_IF(errno != EFAULT);
+
+   FAIL_IF(close(devfd) != 0);
+
+   return 0;
+}
+
+static int check_efault_get(void)
+{
+   return check_efault_common(PAPR_SYSPARM_IOC_GET);
+}
+
+static int check_efault_set(void)
+{
+   return check_efault_common(PAPR_SYSPARM_IOC_SET);
+}
+
+static int set_hmc0(void)
+{
+   struct papr_sysparm_io_block sp = {
+   .parameter = 0, // HMC0, not a settable parameter
+   };
+   const int devfd = open(DEVPATH, O_RDONLY);
+
+   SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+   DEVPATH " not present");
+
+   FAIL_IF(devfd < 0);
+
+   // Ensure expected error
+   FAIL_IF(ioctl(devfd,

[PATCH v3 04/10] powerpc/rtas: Warn if per-function lock isn't held

From: Nathan Lynch 

If the function descriptor has a populated lock member, then callers
are required to hold it across calls. Now that the firmware activation
sequence is appropriately guarded, we can warn when the requirement
isn't satisfied.

__do_enter_rtas_trace() gets reorganized a bit as a result of
performing the function descriptor lookup unconditionally now.

Signed-off-by: Nathan Lynch 
---
 arch/powerpc/kernel/rtas.c | 21 +
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 864d140e7247..0a0ae81fac99 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -670,28 +670,25 @@ static void __do_enter_rtas(struct rtas_args *args)
 
 static void __do_enter_rtas_trace(struct rtas_args *args)
 {
-   const char *name = NULL;
+   struct rtas_function *func = 
rtas_token_to_function(be32_to_cpu(args->token));
 
-   if (args == _args)
-   lockdep_assert_held(_lock);
/*
-* If the tracepoints that consume the function name aren't
-* active, avoid the lookup.
+* If there is a per-function lock, it must be held by the
+* caller.
 */
-   if ((trace_rtas_input_enabled() || trace_rtas_output_enabled())) {
-   const s32 token = be32_to_cpu(args->token);
-   const struct rtas_function *func = 
rtas_token_to_function(token);
+   if (func->lock)
+   WARN_ON(!mutex_is_locked(func->lock));
 
-   name = func->name;
-   }
+   if (args == _args)
+   lockdep_assert_held(_lock);
 
-   trace_rtas_input(args, name);
+   trace_rtas_input(args, func->name);
trace_rtas_ll_entry(args);
 
__do_enter_rtas(args);
 
trace_rtas_ll_exit(args);
-   trace_rtas_output(args, name);
+   trace_rtas_output(args, func->name);
 }
 
 static void do_enter_rtas(struct rtas_args *args)

-- 
2.41.0

[PATCH v3 06/10] powerpc/pseries: Add papr-vpd character driver for VPD retrieval

From: Nathan Lynch 

PowerVM LPARs may retrieve Vital Product Data (VPD) for system
components using the ibm,get-vpd RTAS function.

We can expose this to user space with a /dev/papr-vpd character
device, where the programming model is:

  struct papr_location_code plc = { .str = "", }; /* obtain all VPD */
  int devfd = open("/dev/papr-vpd", O_RDONLY);
  int vpdfd = ioctl(devfd, PAPR_VPD_CREATE_HANDLE, );
  size_t size = lseek(vpdfd, 0, SEEK_END);
  char *buf = malloc(size);
  pread(devfd, buf, size, 0);

When a file descriptor is obtained from ioctl(PAPR_VPD_CREATE_HANDLE),
the file contains the result of a complete ibm,get-vpd sequence. The
file contents are immutable from the POV of user space. To get a new
view of the VPD, the client must create a new handle.

This design choice insulates user space from most of the complexities
that ibm,get-vpd brings:

* ibm,get-vpd must be called more than once to obtain complete
  results.

* Only one ibm,get-vpd call sequence should be in progress at a time;
  interleaved sequences will disrupt each other. Callers must have a
  protocol for serializing their use of the function.

* A call sequence in progress may receive a "VPD changed, try again"
  status, requiring the client to abandon the sequence and start
  over.

The memory required for the VPD buffers seems acceptable, around 20KB
for all VPD on one of my systems. And the value of the
/rtas/ibm,vpd-size DT property (the estimated maximum size of VPD) is
consistently 300KB across various systems I've checked.

I've implemented support for this new ABI in the rtas_get_vpd()
function in librtas, which the vpdupdate command currently uses to
populate its VPD database. I've verified that an unmodified vpdupdate
binary generates an identical database when using a librtas.so that
prefers the new ABI.

Note that the driver needs to serialize its call sequences with legacy
sys_rtas(ibm,get-vpd) callers, so it exposes its internal lock for
sys_rtas.

Signed-off-by: Nathan Lynch 
---
 Documentation/userspace-api/ioctl/ioctl-number.rst |   2 +
 arch/powerpc/include/uapi/asm/papr-vpd.h   |  22 +
 arch/powerpc/platforms/pseries/Makefile|   1 +
 arch/powerpc/platforms/pseries/papr-vpd.c  | 522 +
 4 files changed, 547 insertions(+)

diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst 
b/Documentation/userspace-api/ioctl/ioctl-number.rst
index 4ea5b837399a..a950545bf7cd 100644
--- a/Documentation/userspace-api/ioctl/ioctl-number.rst
+++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
@@ -349,6 +349,8 @@ Code  Seq#Include File  
 Comments
  

 0xB1  00-1F  PPPoX
  

+0xB2  00 arch/powerpc/include/uapi/asm/papr-vpd.h
powerpc/pseries VPD API
+ 

 0xB3  00 linux/mmc/ioctl.h
 0xB4  00-0F  linux/gpio.h

 0xB5  00-0F  uapi/linux/rpmsg.h  

diff --git a/arch/powerpc/include/uapi/asm/papr-vpd.h 
b/arch/powerpc/include/uapi/asm/papr-vpd.h
new file mode 100644
index ..b62e4f897a70
--- /dev/null
+++ b/arch/powerpc/include/uapi/asm/papr-vpd.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_PAPR_VPD_H_
+#define _UAPI_PAPR_VPD_H_
+
+#include 
+#include 
+
+struct papr_location_code {
+   /*
+* PAPR+ 12.3.2.4 Converged Location Code Rules - Length
+* Restrictions. 79 characters plus nul.
+*/
+   char str[80];
+};
+
+/*
+ * ioctl for /dev/papr-vpd. Returns a VPD handle fd corresponding to
+ * the location code.
+ */
+#define PAPR_VPD_IOC_CREATE_HANDLE _IOW(PAPR_MISCDEV_IOC_ID, 0, struct 
papr_location_code)
+
+#endif /* _UAPI_PAPR_VPD_H_ */
diff --git a/arch/powerpc/platforms/pseries/Makefile 
b/arch/powerpc/platforms/pseries/Makefile
index 53c3b91af2f7..cbcaa102e2b4 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -4,6 +4,7 @@ ccflags-$(CONFIG_PPC_PSERIES_DEBUG) += -DDEBUG
 
 obj-y  := lpar.o hvCall.o nvram.o reconfig.o \
   of_helpers.o rtas-work-area.o papr-sysparm.o \
+  papr-vpd.o \
   setup.o iommu.o event_sources.o ras.o \
   firmware.o power.o dlpar.o mobility.o rng.o \
   pci.o pci_dlpar.o eeh_pseries.o msi.o \
diff --git a/arch/powerpc/platforms/pseries/papr-vpd.c 
b/arch/powerpc/platforms/pseries/papr-vpd.c
new file mode 100644

[PATCH v3 03/10] powerpc/rtas: Serialize firmware activation sequences

From: Nathan Lynch 

Use the function lock API to prevent interleaving call sequences of
the ibm,activate-firmware RTAS function, which typically requires
multiple calls to complete the update. While the spec does not
specifically prohibit interleaved sequences, there's almost certainly
no advantage to allowing them.

Signed-off-by: Nathan Lynch 
---
 arch/powerpc/kernel/rtas.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 579a2c475bb6..864d140e7247 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -1738,10 +1738,14 @@ void rtas_activate_firmware(void)
return;
}
 
+   rtas_function_lock(RTAS_FN_IBM_ACTIVATE_FIRMWARE);
+
do {
fwrc = rtas_call(token, 0, 1, NULL);
} while (rtas_busy_delay(fwrc));
 
+   rtas_function_unlock(RTAS_FN_IBM_ACTIVATE_FIRMWARE);
+
if (fwrc)
pr_err("ibm,activate-firmware failed (%i)\n", fwrc);
 }

-- 
2.41.0

[PATCH v3 00/10] powerpc/pseries: New character devices for system parameters and VPD

Add character devices that expose PAPR-specific system parameters and
VPD to user space.

The problem: important platform features are enabled on Linux VMs
through the powerpc-specific rtas() syscall in combination with
writeable mappings of /dev/mem. In typical usage, this is encapsulated
behind APIs provided by the librtas library. This paradigm is
incompatible with lockdown, which prohibits /dev/mem access. It also
is too low-level in many cases: a single logical operation may require
multiple sys_rtas() calls in succession to complete. This carries the
risk that a process may exit while leaving an operation unfinished. It
also means that callers must coordinate their use of the syscall for
functions that cannot tolerate multiple concurrent clients, such as
ibm,get-vpd.

The solution presented here is to add a pair of small pseries-specific
"drivers," one for VPD and one for system parameters. The new drivers
expose these facilities to user space in ways that are compatible with
lockdown and require no coordination between their clients.

Since the ibm,get-vpd call sequence performed by the papr-vpd driver
must be serialized against all other uses of the function, the series
begins by adding some new APIs to the core RTAS support code for this
purpose.

Both drivers could potentially support poll() methods to notify
clients of changes to parameters or VPD that happen due to partition
migration and other events. But that should be safe to leave for
later, assuming there's any interest.

I have made changes to librtas to prefer the new interfaces and
verified that existing clients work correctly with the new code. A
draft PR for that work is here:

https://github.com/ibm-power-utilities/librtas/pull/36

The user-space ABI has not changed since v1 of this series.

I expect to propose at least one more small driver in this style for
platform dump retrieval in a separate submission in the future.

---
Changes in v3:
- Add new rtas_function_lock()/unlock() APIs and convert existing code
to use them.
- Convert papr-vpd to use rtas_function_lock()/unlock() instead of
having sys_rtas() obtain a driver-private mutex.
- Rebase on current powerpc/next.
- Link to v2:
https://lore.kernel.org/r/20231013-papr-sys_rtas-vs-lockdown-v2-0-ead01ce01...@linux.ibm.com

Changes in v2:
- Fix unused-but-set variable warning in papr-sysparm code.
- Rebase on powerpc/next branch.
- Link to v1:
https://lore.kernel.org/r/20231006-papr-sys_rtas-vs-lockdown-v1-0-3a36bfb66...@linux.ibm.com

Changes in v1 vs initial RFC:
- Add papr-sysparm driver and tests.
- Add a papr-miscdev.h uapi header.
- Prevent sys_rtas() from interfering with papr-vpd call sequences.
- Handle -4 ("VPD changed") status in papr-vpd.
- Include string_helpers.h in papr-vpd.c, per Michal Suchánek
- Link to RFC:
https://lore.kernel.org/r/20230822-papr-sys_rtas-vs-lockdown-v1-0-932623cf3...@linux.ibm.com

---
Nathan Lynch (10):
powerpc/rtas: Factor out function descriptor lookup
powerpc/rtas: Facilitate high-level call sequences
powerpc/rtas: Serialize firmware activation sequences
powerpc/rtas: Warn if per-function lock isn't held
powerpc/uapi: Export papr-miscdev.h header
powerpc/pseries: Add papr-vpd character driver for VPD retrieval
powerpc/pseries/papr-sysparm: Validate buffer object lengths
powerpc/pseries/papr-sysparm: Expose character device to user space
powerpc/selftests: Add test for papr-vpd
powerpc/selftests: Add test for papr-sysparm

Documentation/userspace-api/ioctl/ioctl-number.rst | 4 +
arch/powerpc/include/asm/papr-sysparm.h| 17 +-
arch/powerpc/include/asm/rtas.h| 2 +
arch/powerpc/include/uapi/asm/papr-miscdev.h | 9 +
arch/powerpc/include/uapi/asm/papr-sysparm.h | 58 +++
arch/powerpc/include/uapi/asm/papr-vpd.h | 22 +
arch/powerpc/kernel/rtas.c | 157 ++-
arch/powerpc/platforms/pseries/Makefile| 1 +
arch/powerpc/platforms/pseries/papr-sysparm.c | 201 +++-
arch/powerpc/platforms/pseries/papr-vpd.c | 522 +
tools/testing/selftests/powerpc/Makefile | 2 +
.../selftests/powerpc/papr_sysparm/.gitignore | 1 +
.../selftests/powerpc/papr_sysparm/Makefile| 12 +
.../selftests/powerpc/papr_sysparm/papr_sysparm.c | 164 +++
.../testing/selftests/powerpc/papr_vpd/.gitignore | 1 +
tools/testing/selftests/powerpc/papr_vpd/Makefile | 12 +
.../testing/selftests/powerpc/papr_vpd/papr_vpd.c | 352 ++
17 files changed, 1503 insertions(+), 34 deletions(-)
---
base-commit: 36e826b568e412f61d68fedc02a67b4d8b7583cc
change-id: 20230817-papr-sys_rtas-vs-lockdown-5c54505db792

Best regards,
--
Nathan Lynch

[PATCH v3 01/10] powerpc/rtas: Factor out function descriptor lookup

From: Nathan Lynch 

Move the function descriptor table lookup out of rtas_function_token()
into a separate routine for use in new code to follow. No functional
change.

Signed-off-by: Nathan Lynch 
---
 arch/powerpc/kernel/rtas.c | 31 +++
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index eddc031c4b95..e2fac171bf69 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -464,29 +464,36 @@ static struct rtas_function rtas_function_table[] 
__ro_after_init = {
 static DEFINE_RAW_SPINLOCK(rtas_lock);
 static struct rtas_args rtas_args;
 
-/**
- * rtas_function_token() - RTAS function token lookup.
- * @handle: Function handle, e.g. RTAS_FN_EVENT_SCAN.
- *
- * Context: Any context.
- * Return: the token value for the function if implemented by this platform,
- * otherwise RTAS_UNKNOWN_SERVICE.
- */
-s32 rtas_function_token(const rtas_fn_handle_t handle)
+static struct rtas_function *rtas_function_lookup(const rtas_fn_handle_t 
handle)
 {
const size_t index = handle.index;
const bool out_of_bounds = index >= ARRAY_SIZE(rtas_function_table);
 
if (WARN_ONCE(out_of_bounds, "invalid function index %zu", index))
-   return RTAS_UNKNOWN_SERVICE;
+   return NULL;
/*
 * Various drivers attempt token lookups on non-RTAS
 * platforms.
 */
if (!rtas.dev)
-   return RTAS_UNKNOWN_SERVICE;
+   return NULL;
+
+   return _function_table[index];
+}
+
+/**
+ * rtas_function_token() - RTAS function token lookup.
+ * @handle: Function handle, e.g. RTAS_FN_EVENT_SCAN.
+ *
+ * Context: Any context.
+ * Return: the token value for the function if implemented by this platform,
+ * otherwise RTAS_UNKNOWN_SERVICE.
+ */
+s32 rtas_function_token(const rtas_fn_handle_t handle)
+{
+   const struct rtas_function *func = rtas_function_lookup(handle);
 
-   return rtas_function_table[index].token;
+   return func ? func->token : RTAS_UNKNOWN_SERVICE;
 }
 EXPORT_SYMBOL_GPL(rtas_function_token);
 

-- 
2.41.0

[PATCH v3 09/10] powerpc/selftests: Add test for papr-vpd

From: Nathan Lynch 

Add selftests for /dev/papr-vpd, exercising the common expected use
cases:

* Retrieve all VPD by passing an empty location code.
* Retrieve the "system VPD" by passing a location code derived from DT
  root node properties, as done by the vpdupdate command.

The tests also verify that certain intended properties of the driver
hold:

* Passing an unterminated location code to PAPR_VPD_CREATE_HANDLE gets
  EINVAL.
* Passing a NULL location code pointer to PAPR_VPD_CREATE_HANDLE gets
  EFAULT.
* Closing the device node without first issuing a
  PAPR_VPD_CREATE_HANDLE command to it succeeds.
* Releasing a handle without first consuming any data from it
  succeeds.
* Re-reading the contents of a handle returns the same data as the
  first time.

Some minimal validation of the returned data is performed.

The tests are skipped on systems where the papr-vpd driver does not
initialize, making this useful only on PowerVM LPARs at this point.

Signed-off-by: Nathan Lynch 
---
 tools/testing/selftests/powerpc/Makefile   |   1 +
 .../testing/selftests/powerpc/papr_vpd/.gitignore  |   1 +
 tools/testing/selftests/powerpc/papr_vpd/Makefile  |  12 +
 .../testing/selftests/powerpc/papr_vpd/papr_vpd.c  | 352 +
 4 files changed, 366 insertions(+)

diff --git a/tools/testing/selftests/powerpc/Makefile 
b/tools/testing/selftests/powerpc/Makefile
index 7ea42fa02eab..05fc68d446c2 100644
--- a/tools/testing/selftests/powerpc/Makefile
+++ b/tools/testing/selftests/powerpc/Makefile
@@ -32,6 +32,7 @@ SUB_DIRS = alignment  \
   vphn \
   math \
   papr_attributes  \
+  papr_vpd \
   ptrace   \
   security \
   mce
diff --git a/tools/testing/selftests/powerpc/papr_vpd/.gitignore 
b/tools/testing/selftests/powerpc/papr_vpd/.gitignore
new file mode 100644
index ..49285031a656
--- /dev/null
+++ b/tools/testing/selftests/powerpc/papr_vpd/.gitignore
@@ -0,0 +1 @@
+/papr_vpd
diff --git a/tools/testing/selftests/powerpc/papr_vpd/Makefile 
b/tools/testing/selftests/powerpc/papr_vpd/Makefile
new file mode 100644
index ..06b719703bfd
--- /dev/null
+++ b/tools/testing/selftests/powerpc/papr_vpd/Makefile
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0
+noarg:
+   $(MAKE) -C ../
+
+TEST_GEN_PROGS := papr_vpd
+
+top_srcdir = ../../../../..
+include ../../lib.mk
+
+$(TEST_GEN_PROGS): ../harness.c ../utils.c
+
+$(OUTPUT)/papr_vpd: CFLAGS += $(KHDR_INCLUDES)
diff --git a/tools/testing/selftests/powerpc/papr_vpd/papr_vpd.c 
b/tools/testing/selftests/powerpc/papr_vpd/papr_vpd.c
new file mode 100644
index ..98cbb9109ee6
--- /dev/null
+++ b/tools/testing/selftests/powerpc/papr_vpd/papr_vpd.c
@@ -0,0 +1,352 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#define _GNU_SOURCE
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "utils.h"
+
+#define DEVPATH "/dev/papr-vpd"
+
+static int dev_papr_vpd_open_close(void)
+{
+   const int devfd = open(DEVPATH, O_RDONLY);
+
+   SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+   DEVPATH " not present");
+
+   FAIL_IF(devfd < 0);
+   FAIL_IF(close(devfd) != 0);
+
+   return 0;
+}
+
+static int dev_papr_vpd_get_handle_all(void)
+{
+   const int devfd = open(DEVPATH, O_RDONLY);
+   struct papr_location_code lc = { .str = "", };
+   off_t size;
+   int fd;
+
+   SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+   DEVPATH " not present");
+
+   FAIL_IF(devfd < 0);
+
+   errno = 0;
+   fd = ioctl(devfd, PAPR_VPD_IOC_CREATE_HANDLE, );
+   FAIL_IF(errno != 0);
+   FAIL_IF(fd < 0);
+
+   FAIL_IF(close(devfd) != 0);
+
+   size = lseek(fd, 0, SEEK_END);
+   FAIL_IF(size <= 0);
+
+   void *buf = malloc((size_t)size);
+   FAIL_IF(!buf);
+
+   ssize_t consumed = pread(fd, buf, size, 0);
+   FAIL_IF(consumed != size);
+
+   /* Ensure EOF */
+   FAIL_IF(read(fd, buf, size) != 0);
+   FAIL_IF(close(fd));
+
+   /* Verify that the buffer looks like VPD */
+   static const char needle[] = "System VPD";
+   FAIL_IF(!memmem(buf, size, needle, strlen(needle)));
+
+   return 0;
+}
+
+static int dev_papr_vpd_get_handle_byte_at_a_time(void)
+{
+   const int devfd = open(DEVPATH, O_RDONLY);
+   struct papr_location_code lc = { .str = "", };
+   int fd;
+
+   SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+   DEVPATH " not present");
+
+   FAIL_IF(devfd < 0);
+
+   errno = 0;
+   fd = ioctl(devfd, PAPR_VPD_IOC_CREATE_HANDLE, );
+   FAIL_IF(errno != 0);
+   FAIL_IF(fd < 0);
+
+   FAIL_IF(close(devfd) != 0);
+
+   size_t consumed = 0;
+   while (1) {
+   ssize_t res;
+   char c;
+
+   errno = 0;
+   res = read(fd, , sizeof(c));
+   FAIL_IF(res > sizeof(c));
+

[PATCH v3 02/10] powerpc/rtas: Facilitate high-level call sequences

From: Nathan Lynch 

On RTAS platforms there is a general restriction that the OS must not
enter RTAS on more than one CPU at a time. This low-level
serialization requirement is satisfied by holding a spin
lock (rtas_lock) across most RTAS function invocations.

However, some pseries RTAS functions require multiple successive calls
to complete a logical operation. Beginning a new call sequence for such a
function may disrupt any other sequences of that function already in
progress. Safe and reliable use of these functions effectively
requires higher-level serialization beyond what is already done at the
level of RTAS entry and exit.

Where a sequence-based RTAS function is invoked only through
sys_rtas(), with no in-kernel users, there is no issue as far as the
kernel is concerned. User space is responsible for appropriately
serializing its call sequences. (Whether user space code actually
takes measures to prevent sequence interleaving is another matter.)
Examples of such functions currently include ibm,platform-dump and
ibm,get-vpd.

But where a sequence-based RTAS function has both user space and
in-kernel uesrs, there is a hazard. Even if the in-kernel call sites
of such a function serialize their sequences correctly, a user of
sys_rtas() can invoke the same function at any time, potentially
disrupting a sequence in progress.

So in order to prevent disruption of kernel-based RTAS call sequences,
they must serialize not only with themselves but also with sys_rtas()
users, somehow. Preferably without adding global locks or adding more
function-specific hacks to sys_rtas(). This is a prerequisite for
adding an in-kernel call sequence of ibm,get-vpd, which is in a change
to follow.

Note that it has never been feasible for the kernel to prevent
sys_rtas()-based sequences from being disrupted because control
returns to user space on every call. sys_rtas()-based users of these
functions have always been, and continue to be, responsible for
coordinating their call sequences with other users, even those which
may invoke the RTAS functions through less direct means than
sys_rtas(). This is an unavoidable consequence of exposing
sequence-based RTAS functions through sys_rtas().

* Add new rtas_function_lock() and rtas_function_unlock() APIs for use
  with sequence-based RTAS functions.

* Add an optional per-function mutex to struct rtas_function. When this
  member is set, kernel-internal callers of the RTAS function are
  required to guard their call sequences with rtas_function_lock() and
  rtas_function_unlock(). This requirement will be enforced in a later
  change, after all affected call sites are updated.

* Populate the lock members of function table entries where
  serialization of call sequences is known to be necessary, along with
  justifying commentary.

* In sys_rtas(), acquire the per-function mutex when it is present.

There should be no perceivable change introduced here except that
concurrent callers of the same RTAS function via sys_rtas() may block
on a mutex instead of spinning on rtas_lock. Changes to follow will
add rtas_function_lock()/unlock() pairs to kernel-based call
sequences.

Signed-off-by: Nathan Lynch 
---
 arch/powerpc/include/asm/rtas.h |   2 +
 arch/powerpc/kernel/rtas.c  | 101 +++-
 2 files changed, 101 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index c697c3c74694..382627854034 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -408,6 +408,8 @@ static inline bool rtas_function_implemented(const 
rtas_fn_handle_t handle)
 {
return rtas_function_token(handle) != RTAS_UNKNOWN_SERVICE;
 }
+void rtas_function_lock(rtas_fn_handle_t handle);
+void rtas_function_unlock(rtas_fn_handle_t handle);
 extern int rtas_token(const char *service);
 extern int rtas_service_present(const char *service);
 extern int rtas_call(int token, int, int, int *, ...);
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index e2fac171bf69..579a2c475bb6 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -70,14 +71,34 @@ struct rtas_filter {
  *ppc64le, and we want to keep it that way. It does
  *not make sense for this to be set when @filter
  *is NULL.
+ * @lock: Pointer to an optional dedicated per-function mutex. This
+ *should be set for functions that require multiple calls in
+ *sequence to complete a single operation, and such sequences
+ *will disrupt each other if allowed to interleave. Users of
+ *this function are required to hold the associated lock for
+ *the duration of the call sequence. Add an explanatory
+ *comment to the function table entry if setting this member.
  */

[PATCH v3 07/10] powerpc/pseries/papr-sysparm: Validate buffer object lengths

From: Nathan Lynch 

The ability to get and set system parameters will be exposed to user
space, so let's get a little more strict about malformed
papr_sysparm_buf objects.

* Create accessors for the length field of struct papr_sysparm_buf.
  The length is always stored in MSB order and this is better than
  spreading the necessary conversions all over.

* Reject attempts to submit invalid buffers to RTAS.

* Warn if RTAS returns a buffer with an invalid length, clamping the
  returned length to a safe value that won't overrun the buffer.

These are meant as precautionary measures to mitigate both firmware
and kernel bugs in this area, should they arise, but I am not aware of
any.

Signed-off-by: Nathan Lynch 
---
 arch/powerpc/platforms/pseries/papr-sysparm.c | 47 +++
 1 file changed, 47 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/papr-sysparm.c 
b/arch/powerpc/platforms/pseries/papr-sysparm.c
index fedc61599e6c..a1e7aeac7416 100644
--- a/arch/powerpc/platforms/pseries/papr-sysparm.c
+++ b/arch/powerpc/platforms/pseries/papr-sysparm.c
@@ -23,6 +23,46 @@ void papr_sysparm_buf_free(struct papr_sysparm_buf *buf)
kfree(buf);
 }
 
+static size_t papr_sysparm_buf_get_length(const struct papr_sysparm_buf *buf)
+{
+   return be16_to_cpu(buf->len);
+}
+
+static void papr_sysparm_buf_set_length(struct papr_sysparm_buf *buf, size_t 
length)
+{
+   WARN_ONCE(length > sizeof(buf->val),
+ "bogus length %zu, clamping to safe value", length);
+   length = min(sizeof(buf->val), length);
+   buf->len = cpu_to_be16(length);
+}
+
+/*
+ * For use on buffers returned from ibm,get-system-parameter before
+ * returning them to callers. Ensures the encoded length of valid data
+ * cannot overrun buf->val[].
+ */
+static void papr_sysparm_buf_clamp_length(struct papr_sysparm_buf *buf)
+{
+   papr_sysparm_buf_set_length(buf, papr_sysparm_buf_get_length(buf));
+}
+
+/*
+ * Perform some basic diligence on the system parameter buffer before
+ * submitting it to RTAS.
+ */
+static bool papr_sysparm_buf_can_submit(const struct papr_sysparm_buf *buf)
+{
+   /*
+* Firmware ought to reject buffer lengths that exceed the
+* maximum specified in PAPR, but there's no reason for the
+* kernel to allow them either.
+*/
+   if (papr_sysparm_buf_get_length(buf) > sizeof(buf->val))
+   return false;
+
+   return true;
+}
+
 /**
  * papr_sysparm_get() - Retrieve the value of a PAPR system parameter.
  * @param: PAPR system parameter token as described in
@@ -63,6 +103,9 @@ int papr_sysparm_get(papr_sysparm_t param, struct 
papr_sysparm_buf *buf)
if (token == RTAS_UNKNOWN_SERVICE)
return -ENOENT;
 
+   if (!papr_sysparm_buf_can_submit(buf))
+   return -EINVAL;
+
work_area = rtas_work_area_alloc(sizeof(*buf));
 
memcpy(rtas_work_area_raw_buf(work_area), buf, sizeof(*buf));
@@ -77,6 +120,7 @@ int papr_sysparm_get(papr_sysparm_t param, struct 
papr_sysparm_buf *buf)
case 0:
ret = 0;
memcpy(buf, rtas_work_area_raw_buf(work_area), sizeof(*buf));
+   papr_sysparm_buf_clamp_length(buf);
break;
case -3: /* parameter not implemented */
ret = -EOPNOTSUPP;
@@ -115,6 +159,9 @@ int papr_sysparm_set(papr_sysparm_t param, const struct 
papr_sysparm_buf *buf)
if (token == RTAS_UNKNOWN_SERVICE)
return -ENOENT;
 
+   if (!papr_sysparm_buf_can_submit(buf))
+   return -EINVAL;
+
work_area = rtas_work_area_alloc(sizeof(*buf));
 
memcpy(rtas_work_area_raw_buf(work_area), buf, sizeof(*buf));

-- 
2.41.0

[PATCH v3 05/10] powerpc/uapi: Export papr-miscdev.h header

From: Nathan Lynch 

Allocate one identifying code (the first column of the ioctl-number
table) for the collection of PAPR miscdev drivers to share.

Signed-off-by: Nathan Lynch 
---
 arch/powerpc/include/uapi/asm/papr-miscdev.h | 9 +
 1 file changed, 9 insertions(+)

diff --git a/arch/powerpc/include/uapi/asm/papr-miscdev.h 
b/arch/powerpc/include/uapi/asm/papr-miscdev.h
new file mode 100644
index ..49a2a270b7f3
--- /dev/null
+++ b/arch/powerpc/include/uapi/asm/papr-miscdev.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_PAPR_MISCDEV_H_
+#define _UAPI_PAPR_MISCDEV_H_
+
+enum {
+   PAPR_MISCDEV_IOC_ID = 0xb2,
+};
+
+#endif /* _UAPI_PAPR_MISCDEV_H_ */

-- 
2.41.0

[PATCH v3 08/10] powerpc/pseries/papr-sysparm: Expose character device to user space

From: Nathan Lynch 

Until now the papr_sysparm APIs have been kernel-internal. But user
space needs access to PAPR system parameters too. The only method
available to user space today to get or set system parameters is using
sys_rtas() and /dev/mem to pass RTAS-addressable buffers between user
space and firmware. This is incompatible with lockdown and should be
deprecated.

So provide an alternative ABI to user space in the form of a
/dev/papr-sysparm character device with just two ioctl commands (get
and set). The data payloads involved are small enough to fit in the
ioctl argument buffer, making the code relatively simple.

Exposing the system parameters through sysfs has been considered but
it would be too awkward:

* The kernel currently does not have to contain an exhaustive list of
  defined system parameters. This is a convenient property to maintain
  because we don't have to update the kernel whenever a new parameter
  is added to PAPR. Exporting a named attribute in sysfs for each
  parameter would negate this.

* Some system parameters are text-based and some are not.

* Retrieval of at least one system parameter requires input data,
  which a simple read-oriented interface can't support.

Signed-off-by: Nathan Lynch 
---
 Documentation/userspace-api/ioctl/ioctl-number.rst |   2 +
 arch/powerpc/include/asm/papr-sysparm.h|  17 ++-
 arch/powerpc/include/uapi/asm/papr-sysparm.h   |  58 
 arch/powerpc/platforms/pseries/papr-sysparm.c  | 154 -
 4 files changed, 223 insertions(+), 8 deletions(-)

diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst 
b/Documentation/userspace-api/ioctl/ioctl-number.rst
index a950545bf7cd..d8b6cb1a3636 100644
--- a/Documentation/userspace-api/ioctl/ioctl-number.rst
+++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
@@ -351,6 +351,8 @@ Code  Seq#Include File  
 Comments
  

 0xB2  00 arch/powerpc/include/uapi/asm/papr-vpd.h
powerpc/pseries VPD API
  

+0xB2  01-02  arch/powerpc/include/uapi/asm/papr-sysparm.h
powerpc/pseries system parameter API
+ 

 0xB3  00 linux/mmc/ioctl.h
 0xB4  00-0F  linux/gpio.h

 0xB5  00-0F  uapi/linux/rpmsg.h  

diff --git a/arch/powerpc/include/asm/papr-sysparm.h 
b/arch/powerpc/include/asm/papr-sysparm.h
index f5fdbd8ae9db..0dbbff59101d 100644
--- a/arch/powerpc/include/asm/papr-sysparm.h
+++ b/arch/powerpc/include/asm/papr-sysparm.h
@@ -2,8 +2,10 @@
 #ifndef _ASM_POWERPC_PAPR_SYSPARM_H
 #define _ASM_POWERPC_PAPR_SYSPARM_H
 
+#include 
+
 typedef struct {
-   const u32 token;
+   u32 token;
 } papr_sysparm_t;
 
 #define mk_papr_sysparm(x_) ((papr_sysparm_t){ .token = x_, })
@@ -20,11 +22,14 @@ typedef struct {
 #define PAPR_SYSPARM_TLB_BLOCK_INVALIDATE_ATTRSmk_papr_sysparm(50)
 #define PAPR_SYSPARM_LPAR_NAME mk_papr_sysparm(55)
 
-enum {
-   PAPR_SYSPARM_MAX_INPUT  = 1024,
-   PAPR_SYSPARM_MAX_OUTPUT = 4000,
-};
-
+/**
+ * struct papr_sysparm_buf - RTAS work area layout for system parameter 
functions.
+ *
+ * This is the memory layout of the buffers passed to/from
+ * ibm,get-system-parameter and ibm,set-system-parameter. It is
+ * distinct from the papr_sysparm_io_block structure that is passed
+ * between user space and the kernel.
+ */
 struct papr_sysparm_buf {
__be16 len;
char val[PAPR_SYSPARM_MAX_OUTPUT];
diff --git a/arch/powerpc/include/uapi/asm/papr-sysparm.h 
b/arch/powerpc/include/uapi/asm/papr-sysparm.h
new file mode 100644
index ..9f9a0f267ea5
--- /dev/null
+++ b/arch/powerpc/include/uapi/asm/papr-sysparm.h
@@ -0,0 +1,58 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_PAPR_SYSPARM_H_
+#define _UAPI_PAPR_SYSPARM_H_
+
+#include 
+#include 
+#include 
+
+enum {
+   PAPR_SYSPARM_MAX_INPUT  = 1024,
+   PAPR_SYSPARM_MAX_OUTPUT = 4000,
+};
+
+struct papr_sysparm_io_block {
+   __u32 parameter;
+   __u16 length;
+   char data[PAPR_SYSPARM_MAX_OUTPUT];
+};
+
+/**
+ * PAPR_SYSPARM_IOC_GET - Retrieve the value of a PAPR system parameter.
+ *
+ * Uses _IOWR because of one corner case: Retrieving the value of the
+ * "OS Service Entitlement Status" parameter (60) requires the caller
+ * to supply input data (a date string) in the buffer passed to
+ * firmware. So the @length and @data of the incoming
+ * papr_sysparm_io_block are always used to initialize the work area
+ * supplied to ibm,get-system-parameter. No other parameters are known
+ * to

Re: [PATCH 10/10] [RFC] wifi: remove ipw2100/ipw2200 drivers

2023-10-25 Thread Philipp Hortmann


On 10/26/23 00:27, Witold Baryluk wrote:
I might be interested in modernizing the driver, but I have no idea how 
much effort it would be (in terms of changed fraction of code). 20k LOC 
is neither small or big, and not obvious (a lot of it would be 
unchanged), if it is a week of work, or months of work.


Hi Witold,

I am trying to do this with rtl8192e.
One possibility is to take the following patch series as a starting point:
https://yhbt.net/lore/all/1414604649-9105-1-git-send-email-tvbox...@gmail.com/

For me as a beginner and hobbyist this is a huge task. I am happy when I 
can finish it until next summer.


In case of questions feel free to ask.

Bye Philipp

Re: [PATCH 10/10] [RFC] wifi: remove ipw2100/ipw2200 drivers

2023-10-25 Thread Witold Baryluk

> From: Arnd Bergmann 
>
> These two drivers were used for the earliest "Centrino" branded Intel
> laptops during the late 32-bit Pentium-M era, roughly 2003 to 2005, which
> probably makes it the most modern platform that still uses the wireless
> extension interface instead of cfg80211. Unlike the other drivers that
> are suggested for removal, this one is still officially maintained.
>
> According to Johannes Berg, there was an effort to finish the move away
> from wext in the past, but the last evidence of this that I could find
> is from commit a3caa99e6c68f ("libipw: initiate cfg80211 API conversion
> (v2)") in 2009.
>
> Link: https://lore.kernel.org/all/87fs2fgals@kernel.org/
> Cc: Stanislav Yakovlev 
> Cc: Linux Wireless 
> Signed-off-by: Arnd Bergmann 
> ---
> I'm not convinced this should be in the same set of drivers as the
> rest, since this is clearly less obsolete than the other hardware
> that I would remove support for.

I still use ipw2200 on Intel PRO/Wireless 2915ABG [Calexico2] Network
Connection card, in my IBM Thinkpad X41 (Pentium-M 1.73GHz, Centrino
platform). The laptop is rock solid, and I use it as a backup for my
other Thinkpad. In fact is sometimes preferable to more modern machines
(IMHO X41 itself is the best laptop ever made in terms of a design).

Never had really issues with WiFi on it. In terms of speed it is neither
far or slow, but does the job anyway.

Now, I do not use this laptop frequently, maybe once or twice a month.
But that is more because in I use laptops less in general these days. Not
because the machine is not usable. I have modern SSD in it, second hard
drive, two USB 3.0 ports via ExpressCard, high res 4:3 (1440x1050)
display, full disk encryption, etc.

I would really like for this driver to stay in the mainline for another 5-10
years.

I might be interested in modernizing the driver, but I have no idea how
much effort it would be (in terms of changed fraction of code). 20k LOC is
neither small or big, and not obvious (a lot of it would be unchanged),
if it is a week of work, or months of work.

I would not have an issue with removing it, and readding back if somebody
(or me) ports it, if not for re-review from scratch concerns. If I port
it, I would not be able to do re-review, 1) out of date coding standards,
2) different reviewers, 3) I would only port needed parts, and keep rest
of the driver intact, so I would not be able to really provide much
insight. So, readding after porting might be harder than keeping and
porting.

I also used in the past other drivers you are removing (zd1201, hostap,
orinoco, rndis, wl3501), but I do not have too much worry about them, as
they are ancient, and also require ancient hardware to run.


Cheers,
Witold


-- 
Witold Baryluk

Re: [PATCH v6 1/3] PCI/AER: Factor out interrupt toggling into helpers

2023-10-25 Thread Bjorn Helgaas

On Fri, May 12, 2023 at 08:00:12AM +0800, Kai-Heng Feng wrote:
> There are many places that enable and disable AER interrupt, so move
> them into helpers.
> 
> Reviewed-by: Mika Westerberg 
> Reviewed-by: Kuppuswamy Sathyanarayanan 
> 
> Reviewed-by: Jonathan Cameron 
> Signed-off-by: Kai-Heng Feng 

I applied this patch (only 1/3) to pci/aer for v6.7.

I'm not clear on the others yet, so let's look at those again after
v6.7-rc1.  It seemed like there's still a question about disabling
interrupts when we're going to D3hot.

>  drivers/pci/pcie/aer.c | 45 +-
>  1 file changed, 27 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index f6c24ded134c..1420e1f27105 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -1227,6 +1227,28 @@ static irqreturn_t aer_irq(int irq, void *context)
>   return IRQ_WAKE_THREAD;
>  }
>  
> +static void aer_enable_irq(struct pci_dev *pdev)
> +{
> + int aer = pdev->aer_cap;
> + u32 reg32;
> +
> + /* Enable Root Port's interrupt in response to error messages */
> + pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, );
> + reg32 |= ROOT_PORT_INTR_ON_MESG_MASK;
> + pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, reg32);
> +}
> +
> +static void aer_disable_irq(struct pci_dev *pdev)
> +{
> + int aer = pdev->aer_cap;
> + u32 reg32;
> +
> + /* Disable Root's interrupt in response to error messages */
> + pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, );
> + reg32 &= ~ROOT_PORT_INTR_ON_MESG_MASK;
> + pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, reg32);
> +}
> +
>  /**
>   * aer_enable_rootport - enable Root Port's interrupts when receiving 
> messages
>   * @rpc: pointer to a Root Port data structure
> @@ -1256,10 +1278,7 @@ static void aer_enable_rootport(struct aer_rpc *rpc)
>   pci_read_config_dword(pdev, aer + PCI_ERR_UNCOR_STATUS, );
>   pci_write_config_dword(pdev, aer + PCI_ERR_UNCOR_STATUS, reg32);
>  
> - /* Enable Root Port's interrupt in response to error messages */
> - pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, );
> - reg32 |= ROOT_PORT_INTR_ON_MESG_MASK;
> - pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, reg32);
> + aer_enable_irq(pdev);
>  }
>  
>  /**
> @@ -1274,10 +1293,7 @@ static void aer_disable_rootport(struct aer_rpc *rpc)
>   int aer = pdev->aer_cap;
>   u32 reg32;
>  
> - /* Disable Root's interrupt in response to error messages */
> - pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, );
> - reg32 &= ~ROOT_PORT_INTR_ON_MESG_MASK;
> - pci_write_config_dword(pdev, aer + PCI_ERR_ROOT_COMMAND, reg32);
> + aer_disable_irq(pdev);
>  
>   /* Clear Root's error status reg */
>   pci_read_config_dword(pdev, aer + PCI_ERR_ROOT_STATUS, );
> @@ -1372,12 +1388,8 @@ static pci_ers_result_t aer_root_reset(struct pci_dev 
> *dev)
>*/
>   aer = root ? root->aer_cap : 0;
>  
> - if ((host->native_aer || pcie_ports_native) && aer) {
> - /* Disable Root's interrupt in response to error messages */
> - pci_read_config_dword(root, aer + PCI_ERR_ROOT_COMMAND, );
> - reg32 &= ~ROOT_PORT_INTR_ON_MESG_MASK;
> - pci_write_config_dword(root, aer + PCI_ERR_ROOT_COMMAND, reg32);
> - }
> + if ((host->native_aer || pcie_ports_native) && aer)
> + aer_disable_irq(root);
>  
>   if (type == PCI_EXP_TYPE_RC_EC || type == PCI_EXP_TYPE_RC_END) {
>   rc = pcie_reset_flr(dev, PCI_RESET_DO_RESET);
> @@ -1396,10 +1408,7 @@ static pci_ers_result_t aer_root_reset(struct pci_dev 
> *dev)
>   pci_read_config_dword(root, aer + PCI_ERR_ROOT_STATUS, );
>   pci_write_config_dword(root, aer + PCI_ERR_ROOT_STATUS, reg32);
>  
> - /* Enable Root Port's interrupt in response to error messages */
> - pci_read_config_dword(root, aer + PCI_ERR_ROOT_COMMAND, );
> - reg32 |= ROOT_PORT_INTR_ON_MESG_MASK;
> - pci_write_config_dword(root, aer + PCI_ERR_ROOT_COMMAND, reg32);
> + aer_enable_irq(root);
>   }
>  
>   return rc ? PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_RECOVERED;
> -- 
> 2.34.1
>

[PATCH 3/8] leds: an30259a: explicitly unregister LEDs at module's shutdown

LEDs are registered using devm_led_classdev_register() and automatically
unregistered after module's remove(). led_classdev_unregister() calls
led_set_brightness() to turn off the LEDs and module's appropriate callback
uses resources those were destroyed already in module's remove().
So explicitly unregister LEDs at module shutdown.

Signed-off-by: George Stark 
---
 drivers/leds/leds-an30259a.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/leds/leds-an30259a.c b/drivers/leds/leds-an30259a.c
index 24b1041213c2..4209a407d802 100644
--- a/drivers/leds/leds-an30259a.c
+++ b/drivers/leds/leds-an30259a.c
@@ -324,6 +324,10 @@ static int an30259a_probe(struct i2c_client *client)
 static void an30259a_remove(struct i2c_client *client)
 {
struct an30259a *chip = i2c_get_clientdata(client);
+   int i;
+
+   for (i = 0; i < chip->num_leds; i++)
+   devm_led_classdev_unregister(>dev, >leds[i].cdev);
 
mutex_destroy(>mutex);
 }
-- 
2.38.4

[PATCH 1/8] leds: powernv: explicitly unregister LEDs at module's shutdown

LEDs are registered using devm_led_classdev_register() and automatically
unregistered after module's remove(). led_classdev_unregister() calls
led_set_brightness() to turn off the LEDs and module's appropriate callback
uses resources those were destroyed already in module's remove().
So explicitly unregister LEDs at module shutdown.

Signed-off-by: George Stark 
---
 drivers/leds/leds-powernv.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/leds/leds-powernv.c b/drivers/leds/leds-powernv.c
index 743e2cdd0891..7c7f696c8265 100644
--- a/drivers/leds/leds-powernv.c
+++ b/drivers/leds/leds-powernv.c
@@ -302,7 +302,12 @@ static int powernv_led_probe(struct platform_device *pdev)
 
platform_set_drvdata(pdev, powernv_led_common);
 
+   if (!devres_open_group(>dev, priv, GFP_KERNEL))
+   return -ENOMEM;
+
rc = powernv_led_classdev(pdev, led_node, powernv_led_common);
+   if (rc)
+   devres_remove_group(dev, priv);
 out:
of_node_put(led_node);
return rc;
@@ -313,6 +318,8 @@ static int powernv_led_remove(struct platform_device *pdev)
 {
struct powernv_led_common *powernv_led_common;
 
+   devres_release_group(>dev, powernv_led_common);
+
/* Disable LED operation */
powernv_led_common = platform_get_drvdata(pdev);
powernv_led_common->led_disabled = true;
-- 
2.38.4

[PATCH 2/8] leds: nic78bx: explicitly unregister LEDs at module's shutdown

LEDs are registered using devm_led_classdev_register() and automatically
unregistered after module's remove(). led_classdev_unregister() calls
led_set_brightness() to turn off the LEDs and module's appropriate callback
uses resources those were destroyed already in module's remove().
So explicitly unregister LEDs at module shutdown.

Signed-off-by: George Stark 
---
 drivers/leds/leds-nic78bx.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/leds/leds-nic78bx.c b/drivers/leds/leds-nic78bx.c
index f196f52eec1e..12b70fcad37f 100644
--- a/drivers/leds/leds-nic78bx.c
+++ b/drivers/leds/leds-nic78bx.c
@@ -170,6 +170,10 @@ static int nic78bx_probe(struct platform_device *pdev)
 static int nic78bx_remove(struct platform_device *pdev)
 {
struct nic78bx_led_data *led_data = platform_get_drvdata(pdev);
+   int i;
+
+   for (i = 0; i < ARRAY_SIZE(nic78bx_leds); i++)
+   devm_led_classdev_unregister(>dev, _leds[i].cdev);
 
/* Lock LED register */
outb(NIC78BX_LOCK_VALUE,
-- 
2.38.4

[PATCH 6/8] leds: aw2013: explicitly unregister LEDs at module's shutdown

LEDs are registered using devm_led_classdev_register() and automatically
unregistered after module's remove(). led_classdev_unregister() calls
led_set_brightness() to turn off the LEDs and module's appropriate callback
uses resources those were destroyed already in module's remove().
So explicitly unregister LEDs at module shutdown.

Signed-off-by: George Stark 
---
 drivers/leds/leds-aw2013.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/leds/leds-aw2013.c b/drivers/leds/leds-aw2013.c
index 59765640b70f..bd233500aa2d 100644
--- a/drivers/leds/leds-aw2013.c
+++ b/drivers/leds/leds-aw2013.c
@@ -404,6 +404,10 @@ static int aw2013_probe(struct i2c_client *client)
 static void aw2013_remove(struct i2c_client *client)
 {
struct aw2013 *chip = i2c_get_clientdata(client);
+   int i;
+
+   for (i = 0; i < chip->num_leds; i++)
+   devm_led_classdev_unregister(>dev, >leds[i].cdev);
 
aw2013_chip_disable(chip);
 
-- 
2.38.4

[PATCH 0/8] devm_led_classdev_register() usage problem

Lots of drivers use devm_led_classdev_register() to register their led objects
and let the kernel free those leds at the driver's remove stage.
It can lead to a problem due to led_classdev_unregister()
implementation calls led_set_brightness() to turn off the led.
led_set_brightness() may call one of the module's brightness_set callbacks.
If that callback uses module's resources allocated without using devm funcs()
then those resources will be already freed at module's remove() callback and
we may have use-after-free situation.

Here is an example:

module_probe()
{
devm_led_classdev_register(module_brightness_set_cb);
mutex_init();
}

module_brightness_set_cb()
{
mutex_lock();
do_set_brightness();
mutex_unlock();
}

module_remove()
{
mutex_destroy();
}

at rmmod:
module_remove()
->mutex_destroy();
devres_release_all()
->led_classdev_unregister();
->led_set_brightness();
->module_brightness_set_cb();
 ->mutex_lock();  /* use-after-free */

I think it's an architectural issue and should be discussed thoroughly.
Some thoughts about fixing it as a start:
1) drivers can use devm_led_classdev_unregister() to explicitly free leds before
dependend resources are freed. devm_led_classdev_register() remains being useful
to simplify probe implementation.
As a proof of concept I examined all drivers from drivers/leds and prepared
patches where it's needed. Sometimes it was not as clean as just calling
devm_led_classdev_unregister() because several drivers do not track
their leds object at all - they can call devm_led_classdev_register() and drop 
the
returned pointer. In that case I used devres group API.

Drivers outside drivers/leds should be checked too after discussion.

2) remove led_set_brightness from led_classdev_unregister() and force the 
drivers
to turn leds off at shutdown. May be add check that led's brightness is 0
at led_classdev_unregister() and put a warning to dmesg if it's not.
Actually in many cases it doesn't really need to turn off the leds manually 
one-by-one
if driver shutdowns whole led controller. For the last case to disable the 
warning
new flag can be brought in e.g LED_AUTO_OFF_AT_SHUTDOWN (similar to 
LED_RETAIN_AT_SHUTDOWN).

George Stark (8):
  leds: powernv: explicitly unregister LEDs at module's shutdown
  leds: nic78bx: explicitly unregister LEDs at module's shutdown
  leds: an30259a: explicitly unregister LEDs at module's shutdown
  leds: mlxreg: explicitly unregister LEDs at module's shutdown
  leds: aw200xx: explicitly unregister LEDs at module's shutdown
  leds: aw2013: explicitly unregister LEDs at module's shutdown
  leds: lp3952: explicitly unregister LEDs at module's shutdown
  leds: lm3532: explicitly unregister LEDs at module's shutdown

 drivers/leds/leds-an30259a.c |  4 
 drivers/leds/leds-aw200xx.c  |  4 
 drivers/leds/leds-aw2013.c   |  4 
 drivers/leds/leds-lm3532.c   |  6 ++
 drivers/leds/leds-lp3952.c   |  5 +
 drivers/leds/leds-mlxreg.c   | 12 +++-
 drivers/leds/leds-nic78bx.c  |  4 
 drivers/leds/leds-powernv.c  |  7 +++
 8 files changed, 45 insertions(+), 1 deletion(-)

-- 
2.38.4

[PATCH 7/8] leds: lp3952: explicitly unregister LEDs at module's shutdown

LEDs are registered using devm_led_classdev_register() and automatically
unregistered after module's remove(). led_classdev_unregister() calls
led_set_brightness() to turn off the LEDs and module's appropriate callback
uses resources those were destroyed already in module's remove().
So explicitly unregister LEDs at module shutdown.

Signed-off-by: George Stark 
---
 drivers/leds/leds-lp3952.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/leds/leds-lp3952.c b/drivers/leds/leds-lp3952.c
index 3bd55652a706..2de49192011a 100644
--- a/drivers/leds/leds-lp3952.c
+++ b/drivers/leds/leds-lp3952.c
@@ -257,8 +257,13 @@ static int lp3952_probe(struct i2c_client *client)
 static void lp3952_remove(struct i2c_client *client)
 {
struct lp3952_led_array *priv;
+   int i;
 
priv = i2c_get_clientdata(client);
+   for (i = 0; i < LP3952_LED_ALL; i++)
+   if (priv->leds[i].priv)
+   devm_led_classdev_unregister(>dev,
+>leds[i].cdev);
lp3952_on_off(priv, LP3952_LED_ALL, false);
gpiod_set_value(priv->enable_gpio, 0);
 }
-- 
2.38.4

[PATCH 5/8] leds: aw200xx: explicitly unregister LEDs at module's shutdown

LEDs are registered using devm_led_classdev_register() and automatically
unregistered after module's remove(). led_classdev_unregister() calls
led_set_brightness() to turn off the LEDs and module's appropriate callback
uses resources those were destroyed already in module's remove().
So explicitly unregister LEDs at module shutdown.

Signed-off-by: George Stark 
---
 drivers/leds/leds-aw200xx.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c
index 96979b8e09b7..3da0923507ec 100644
--- a/drivers/leds/leds-aw200xx.c
+++ b/drivers/leds/leds-aw200xx.c
@@ -539,6 +539,10 @@ static int aw200xx_probe(struct i2c_client *client)
 static void aw200xx_remove(struct i2c_client *client)
 {
struct aw200xx *chip = i2c_get_clientdata(client);
+   int i;
+
+   for (i = 0; i < chip->num_leds; i++)
+   devm_led_classdev_unregister(>dev, >leds[i].cdev);
 
aw200xx_chip_reset(chip);
mutex_destroy(>mutex);
-- 
2.38.4

[PATCH 4/8] leds: mlxreg: explicitly unregister LEDs at module's shutdown

LEDs are registered using devm_led_classdev_register() and automatically
unregistered after module's remove(). led_classdev_unregister() calls
led_set_brightness() to turn off the LEDs and module's appropriate callback
uses resources those were destroyed already in module's remove().
So explicitly unregister LEDs at module shutdown.

Signed-off-by: George Stark 
---
 drivers/leds/leds-mlxreg.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/leds-mlxreg.c b/drivers/leds/leds-mlxreg.c
index b7855c93bd72..6d65e39c3372 100644
--- a/drivers/leds/leds-mlxreg.c
+++ b/drivers/leds/leds-mlxreg.c
@@ -258,6 +258,7 @@ static int mlxreg_led_probe(struct platform_device *pdev)
 {
struct mlxreg_core_platform_data *led_pdata;
struct mlxreg_led_priv_data *priv;
+   int res;
 
led_pdata = dev_get_platdata(>dev);
if (!led_pdata) {
@@ -273,13 +274,22 @@ static int mlxreg_led_probe(struct platform_device *pdev)
priv->pdev = pdev;
priv->pdata = led_pdata;
 
-   return mlxreg_led_config(priv);
+   if (!devres_open_group(>dev, priv, GFP_KERNEL))
+   return -ENOMEM;
+
+   res = mlxreg_led_config(priv);
+   if (res)
+   devres_remove_group(>dev, priv);
+
+   return res;
 }
 
 static int mlxreg_led_remove(struct platform_device *pdev)
 {
struct mlxreg_led_priv_data *priv = dev_get_drvdata(>dev);
 
+   devres_release_group(>dev, priv);
+
mutex_destroy(>access_lock);
 
return 0;
-- 
2.38.4

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

2023-10-25 Thread Eric DeVolder

 

On Wednesday, October 25, 2023 at 04:58:20 AM CDT, Baoquan He 
 wrote:  
 
 On 10/24/23 at 03:17pm, Arnd Bergmann wrote:
> On Tue, Oct 24, 2023, at 14:44, Baoquan He wrote:
> > Just add people and mailing list to CC since I didn't find this mail in
> > my box, just drag it via 'b4 am'.
> >
> > On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> > ..
> 
> >> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> >> index 7aff28ded2f48..bfc636d64ff2b 100644
> >> --- a/kernel/Kconfig.kexec
> >> +++ b/kernel/Kconfig.kexec
> >> @@ -36,6 +36,7 @@ config KEXEC
> >>  config KEXEC_FILE
> >>      bool "Enable kexec file based system call"
> >>      depends on ARCH_SUPPORTS_KEXEC_FILE
> >> +    depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
> >
> > I am not sure if the logic is correct. In theory, kexec_file code
> > utilizes purgatory to verify the checksum digested during kernel loading
> > when try to jump to the kernel. That means kexec_file depends on
> > purgatory, but not contrary?
> 
> The expression I wrote is a bit confusing, but I think this just
> keeps the existing behavior:
> 
> - on architectures that select ARCH_SUPPORTS_KEXEC_PURGATORY
>  (powerpc, riscv, s390 and x86), we also require CRYPTO_SHA256
>  to be built-in.
> - on architectures that do not have ARCH_SUPPORTS_KEXEC_PURGATORY
>  (arm64 and parisc), CRYPTO_SHA256 is not used and can be disabled
>  or =m.
> 
> Since ARCH_SUPPORTS_KEXEC_PURGATORY is a 'bool' symbol, it could
> be written as
> 
> depends on (ARCH_SUPPORTS_KEXEC_PURGATORY && CRYPTO_SHA256=y) \
>            || !ARCH_SUPPORTS_KEXEC_PURGATORY

Yes, this seems to be clearer to me. Thanks.

> 
> if you find that clearer. I see that the second patch
> actually gets this wrong, it should actually do
> 
> select CRYPTO if ARCH_SUPPORTS_KEXEC_PURGATORY
> select CRYPTO_SHA256 if ARCH_SUPPORTS_KEXEC_PURGATORY

Yeah, makes sense to me.

Hi Eric,

Do you have comment about these?
[eric]: The original goal of the conversion patches was to consolidate, but 
keep existing behavior.Then I had to break existing behavior a bit by tying 
CRASH to KEXEC. I was hoping to haveavoided introducing new problems, but looks 
like there was an escape. At any rate, I thinkthe ideas here are on track and 
necessary. Unfortunately at the moment I'm not at a placewhere I can test.

> 
> > With these changes, we can achieve the goal to avoid building issue,
> > whereas the code logic becomes confusing. E.g people could disable
> > CONFIG_KEXEC_FILE, but still get purgatory code built in which is
> > totally useless.
> >
> > Not sure if I think too much over this.
> 
> I see your point here, and I would suggest changing the
> CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
> the availability of the purgatory code for the arch, rather
> than actually controlling the code itself. I already mentioned
> this for s390, but riscv would need the same thing on top.
> 
> I think the change below should address your concern.
> 
>      Arnd
> 
> diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
> index e60fbd8660c4..3ac341d296db 100644
> --- a/arch/riscv/kernel/elf_kexec.c
> +++ b/arch/riscv/kernel/elf_kexec.c
> @@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image, char 
> *kernel_buf,
>                cmdline = modified_cmdline;
>        }
>  
> -#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
> +#ifdef CONFIG_KEXEC_FILE
>        /* Add purgatory to the image */
>        kbuf.top_down = true;
>        kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> @@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image, char 
> *kernel_buf,
>                                              sizeof(kernel_start), 0);
>        if (ret)
>                pr_err("Error update purgatory ret=%d\n", ret);
> -#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
> +#endif /* CONFIG_KEXEC_FILE */

If so, we don't need the CONFIG_KEXEC_FILE ifdeffery because the
file elf_kexec.c relied on CONFIG_KEXEC_FILE enabling to build in.
We can just remove the "#ifdef CONFIG_KEXEC_FILE..#endif" as x86 does.

>  
>        /* Add the initrd to the image */
>        if (initrd != NULL) {
> diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
> index d25ad1c19f88..ab181d187c23 100644
> --- a/arch/riscv/Kbuild
> +++ b/arch/riscv/Kbuild
> @@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
>  obj-y += errata/
>  obj-$(CONFIG_KVM) += kvm/
>  
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE) += purgatory/
>  
>  # for cleaning
>  subdir- += boot
> diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
> index a5d3503b353c..361aa01dbd49 100644
> --- a/arch/s390/Kbuild
> +++ b/arch/s390/Kbuild
> @@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)        += hypfs/
>  obj-$(CONFIG_APPLDATA_BASE)    += appldata/
>  obj-y                          += net/
>  obj-$(CONFIG_PCI)              += pci/
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE)

[powerpc:merge] BUILD SUCCESS 4d121328397f83465b6c1f60c5fad93bda9bc90e

2023-10-25 Thread kernel test robot

tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git 
merge
branch HEAD: 4d121328397f83465b6c1f60c5fad93bda9bc90e  Automatic merge of 
'next' into merge (2023-10-23 20:42)

elapsed time: 3247m

configs tested: 129
configs skipped: 2

The following configs have been built successfully.
More configs may be tested in the coming days.

tested configs:
alpha allnoconfig   gcc  
alphaallyesconfig   gcc  
alpha   defconfig   gcc  
arc  allmodconfig   gcc  
arc   allnoconfig   gcc  
arc  allyesconfig   gcc  
arc defconfig   gcc  
arc   randconfig-001-20231024   gcc  
arm   allnoconfig   gcc  
arm  allyesconfig   gcc  
arm defconfig   gcc  
arm   randconfig-001-20231024   gcc  
arm64 allnoconfig   gcc  
arm64   defconfig   gcc  
csky  allnoconfig   gcc  
cskydefconfig   gcc  
i386 allmodconfig   gcc  
i386  allnoconfig   gcc  
i386 allyesconfig   gcc  
i386 buildonly-randconfig-001-20231024   gcc  
i386 buildonly-randconfig-002-20231024   gcc  
i386 buildonly-randconfig-003-20231024   gcc  
i386 buildonly-randconfig-004-20231024   gcc  
i386 buildonly-randconfig-005-20231024   gcc  
i386 buildonly-randconfig-006-20231024   gcc  
i386  debian-10.3   gcc  
i386defconfig   gcc  
i386  randconfig-001-20231024   gcc  
i386  randconfig-002-20231024   gcc  
i386  randconfig-003-20231024   gcc  
i386  randconfig-004-20231024   gcc  
i386  randconfig-005-20231024   gcc  
i386  randconfig-006-20231024   gcc  
i386  randconfig-011-20231024   gcc  
i386  randconfig-012-20231024   gcc  
i386  randconfig-013-20231024   gcc  
i386  randconfig-014-20231024   gcc  
i386  randconfig-015-20231024   gcc  
i386  randconfig-016-20231024   gcc  
loongarchallmodconfig   gcc  
loongarch allnoconfig   gcc  
loongarchallyesconfig   gcc  
loongarch   defconfig   gcc  
loongarch randconfig-001-20231024   gcc  
m68k allmodconfig   gcc  
m68k  allnoconfig   gcc  
m68k allyesconfig   gcc  
m68kdefconfig   gcc  
microblaze   allmodconfig   gcc  
microblazeallnoconfig   gcc  
microblaze   allyesconfig   gcc  
microblaze  defconfig   gcc  
mips allmodconfig   gcc  
mips  allnoconfig   gcc  
mips allyesconfig   gcc  
nios2allmodconfig   gcc  
nios2 allnoconfig   gcc  
nios2allyesconfig   gcc  
nios2   defconfig   gcc  
openrisc allmodconfig   gcc  
openrisc  allnoconfig   gcc  
openrisc allyesconfig   gcc  
openriscdefconfig   gcc  
parisc   allmodconfig   gcc  
pariscallnoconfig   gcc  
parisc   allyesconfig   gcc  
parisc  defconfig   gcc  
parisc64defconfig   gcc  
powerpc  allmodconfig   gcc  
powerpc   allnoconfig   gcc  
powerpc  allyesconfig   gcc  
riscvallmodconfig   gcc  
riscv allnoconfig   gcc  
riscvallyesconfig   gcc  
riscv   defconfig   gcc  
riscv randconfig-001-20231024   gcc  
riscv  rv32_defconfig   gcc  
s390 allmodconfig   gcc  
s390  allnoconfig   gcc  
s390 allyesconfig   gcc  
s390defconfig   gcc  
s390  randconfig-001-20231024   gcc  
sh   allmodconfig   gcc  
shallnoconfig   gcc  
sh   allyesconfig   gcc  
sh

Re: [PATCH v8 00/30] Add support for QMC HDLC, framer infrastructure and PEF2256 framer

2023-10-25 Thread Mark Brown

On Wed, Oct 25, 2023 at 12:32:15PM -0700, Jakub Kicinski wrote:
> On Wed, 25 Oct 2023 17:00:51 +0200 Herve Codina wrote:
> > > Which way will those patches go? Via some FSL SoC tree?  

> > This series seems mature now.
> > What is the plan next in order to have it applied ?

> > Don't hesitate to tell me if you prefer split series.

> FWIW we are happy to take the drivers/net/ parts if there is no hard
> dependency. But there's no point taking that unless the SoC bits
> also go in for 6.7.

> Li Yang, what are your expectations WRT merging this series?

I did previously ask for the generic framer bit as a tag so I could
apply the ASoC part but nobody responded and I've been getting repeated
resends of the series :(


signature.asc
Description: PGP signature

Re: [PATCH v8 00/30] Add support for QMC HDLC, framer infrastructure and PEF2256 framer

2023-10-25 Thread Jakub Kicinski

On Wed, 25 Oct 2023 17:00:51 +0200 Herve Codina wrote:
> > Which way will those patches go? Via some FSL SoC tree?  
> 
> This series seems mature now.
> What is the plan next in order to have it applied ?
> 
> Don't hesitate to tell me if you prefer split series.

FWIW we are happy to take the drivers/net/ parts if there is no hard
dependency. But there's no point taking that unless the SoC bits
also go in for 6.7.

Li Yang, what are your expectations WRT merging this series?

Re: [PATCH v4 2/5] KEYS: trusted: Introduce NXP DCP-backed trusted keys

2023-10-25 Thread Jarkko Sakkinen

On Tue Oct 24, 2023 at 7:20 PM EEST, David Gstir wrote:
> DCP (Data Co-Processor) is the little brother of NXP's CAAM IP.
> Beside of accelerated crypto operations, it also offers support for
> hardware-bound keys. Using this feature it is possible to implement a blob
> mechanism similar to what CAAM offers. Unlike on CAAM, constructing and
> parsing the blob has to happen in software (i.e. the kernel).
>
> The software-based blob format used by DCP trusted keys encrypts
> the payload using AES-128-GCM with a freshly generated random key and nonce.
> The random key itself is AES-128-ECB encrypted using the DCP unique
> or OTP key.
>
> The DCP trusted key blob format is:
> /*
>  * struct dcp_blob_fmt - DCP BLOB format.
>  *
>  * @fmt_version: Format version, currently being %1
>  * @blob_key: Random AES 128 key which is used to encrypt @payload,
>  *@blob_key itself is encrypted with OTP or UNIQUE device key in
>  *AES-128-ECB mode by DCP.
>  * @nonce: Random nonce used for @payload encryption.
>  * @payload_len: Length of the plain text @payload.
>  * @payload: The payload itself, encrypted using AES-128-GCM and @blob_key,
>  *   GCM auth tag of size AES_BLOCK_SIZE is attached at the end of it.
>  *
>  * The total size of a DCP BLOB is sizeof(struct dcp_blob_fmt) + @payload_len 
> +
>  * AES_BLOCK_SIZE.
>  */
> struct dcp_blob_fmt {
>   __u8 fmt_version;
>   __u8 blob_key[AES_KEYSIZE_128];
>   __u8 nonce[AES_KEYSIZE_128];
>   __le32 payload_len;
>   __u8 payload[];
> } __packed;
>
> By default the unique key is used. It is also possible to use the
> OTP key. While the unique key should be unique it is not documented how
> this key is derived. Therefore selection the OTP key is supported as
> well via the use_otp_key module parameter.
>
> Co-developed-by: Richard Weinberger 
> Signed-off-by: Richard Weinberger 
> Co-developed-by: David Oberhollenzer 
> Signed-off-by: David Oberhollenzer 
> Signed-off-by: David Gstir 
> ---
>  include/keys/trusted_dcp.h|  11 +
>  security/keys/trusted-keys/Kconfig|   9 +-
>  security/keys/trusted-keys/Makefile   |   2 +
>  security/keys/trusted-keys/trusted_core.c |   6 +-
>  security/keys/trusted-keys/trusted_dcp.c  | 311 ++
>  5 files changed, 337 insertions(+), 2 deletions(-)
>  create mode 100644 include/keys/trusted_dcp.h
>  create mode 100644 security/keys/trusted-keys/trusted_dcp.c
>
> diff --git a/include/keys/trusted_dcp.h b/include/keys/trusted_dcp.h
> new file mode 100644
> index ..9aaa42075b40
> --- /dev/null
> +++ b/include/keys/trusted_dcp.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2021 sigma star gmbh
> + */
> +
> +#ifndef TRUSTED_DCP_H
> +#define TRUSTED_DCP_H
> +
> +extern struct trusted_key_ops dcp_trusted_key_ops;
> +
> +#endif
> diff --git a/security/keys/trusted-keys/Kconfig 
> b/security/keys/trusted-keys/Kconfig
> index dbfdd8536468..c6b80b7e5c78 100644
> --- a/security/keys/trusted-keys/Kconfig
> +++ b/security/keys/trusted-keys/Kconfig
> @@ -33,6 +33,13 @@ config TRUSTED_KEYS_CAAM
> Enable use of NXP's Cryptographic Accelerator and Assurance Module
> (CAAM) as trusted key backend.
>  
> -if !TRUSTED_KEYS_TPM && !TRUSTED_KEYS_TEE && !TRUSTED_KEYS_CAAM
> +config TRUSTED_KEYS_DCP
> + bool "DCP-based trusted keys"
> + depends on CRYPTO_DEV_MXS_DCP >= TRUSTED_KEYS
> + default y
> + help
> +   Enable use of NXP's DCP (Data Co-Processor) as trusted key backend.
> +
> +if !TRUSTED_KEYS_TPM && !TRUSTED_KEYS_TEE && !TRUSTED_KEYS_CAAM && 
> !TRUSTED_KEYS_DCP

This does not scale tbh.

I'd suggest to add additional patch before adding the new key type,
which clears this up a little bit.

First:

config HAVE_TRUSTED_KEYS
bool

And then following this pattern to all trusted key types:

config TRUSTED_KEYS_DCP
bool "DCP-based trusted keys"
depends on CRYPTO_DEV_MXS_DCP >= TRUSTED_KEYS
default y
select HAVE_TRUSTED_KEYS
help
  Enable use of NXP's DCP (Data Co-Processor) as trusted key backend.

And finally:

if !HAVE_TRUSTED_KEYS
comment "No trust source selected!"
endif

BR, Jarkko

Re: [PATCH v4 1/5] crypto: mxs-dcp: Add support for hardware-bound keys

2023-10-25 Thread Jarkko Sakkinen

On Tue Oct 24, 2023 at 7:20 PM EEST, David Gstir wrote:
> DCP is capable of performing AES with two hardware-bound keys:
>
> - The one-time programmable (OTP) key which is burnt via on-chip fuses
> - The unique key (UK) which is derived from the OTP key
>
> In addition to the two hardware-bound keys, DCP also supports
> storing keys in 4 dedicated key slots within its secure memory area
> (internal SRAM).
>
> These keys are not stored in main memory and are therefore
> not directly accessible by the operating system. To use them
> for AES operations, a one-byte key reference has to supplied
> with the DCP operation descriptor in the control register.
>
> This adds support for using any of these 6 keys through the crypto API
> via their key reference after they have been set up. The main purpose
> is to add support for DCP-backed trusted keys. Other use cases are
> possible too (see similar existing paes implementations), but these
> should carefully be evaluated as e.g. enabling AF_ALG will give
> userspace full access to use keys. In scenarios with untrustworthy
> userspace, this will enable en-/decryption oracles.
>
> Co-developed-by: Richard Weinberger 
> Signed-off-by: Richard Weinberger 
> Co-developed-by: David Oberhollenzer 
> Signed-off-by: David Oberhollenzer 
> Signed-off-by: David Gstir 
> ---
>  drivers/crypto/mxs-dcp.c | 104 ++-
>  include/soc/fsl/dcp.h|  17 +++
>  2 files changed, 110 insertions(+), 11 deletions(-)
>  create mode 100644 include/soc/fsl/dcp.h
>
> diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
> index f6b7bce0e656..2dc664fb2faf 100644
> --- a/drivers/crypto/mxs-dcp.c
> +++ b/drivers/crypto/mxs-dcp.c
> @@ -15,6 +15,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -101,6 +102,7 @@ struct dcp_async_ctx {
>   struct crypto_skcipher  *fallback;
>   unsigned intkey_len;
>   uint8_t key[AES_KEYSIZE_128];
> + boolkey_referenced;
>  };
>  
>  struct dcp_aes_req_ctx {
> @@ -155,6 +157,7 @@ static struct dcp *global_sdcp;
>  #define MXS_DCP_CONTROL0_HASH_TERM   (1 << 13)
>  #define MXS_DCP_CONTROL0_HASH_INIT   (1 << 12)
>  #define MXS_DCP_CONTROL0_PAYLOAD_KEY (1 << 11)
> +#define MXS_DCP_CONTROL0_OTP_KEY (1 << 10)
>  #define MXS_DCP_CONTROL0_CIPHER_ENCRYPT  (1 << 8)
>  #define MXS_DCP_CONTROL0_CIPHER_INIT (1 << 9)
>  #define MXS_DCP_CONTROL0_ENABLE_HASH (1 << 6)
> @@ -168,6 +171,8 @@ static struct dcp *global_sdcp;
>  #define MXS_DCP_CONTROL1_CIPHER_MODE_ECB (0 << 4)
>  #define MXS_DCP_CONTROL1_CIPHER_SELECT_AES128(0 << 0)
>  
> +#define MXS_DCP_CONTROL1_KEY_SELECT_SHIFT8
> +
>  static int mxs_dcp_start_dma(struct dcp_async_ctx *actx)
>  {
>   int dma_err;
> @@ -224,13 +229,16 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
>   struct dcp *sdcp = global_sdcp;
>   struct dcp_dma_desc *desc = >coh->desc[actx->chan];
>   struct dcp_aes_req_ctx *rctx = skcipher_request_ctx(req);
> + bool key_referenced = actx->key_referenced;
>   int ret;
>  
> - key_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_key,
> -   2 * AES_KEYSIZE_128, DMA_TO_DEVICE);
> - ret = dma_mapping_error(sdcp->dev, key_phys);
> - if (ret)
> - return ret;
> + if (!key_referenced) {
> + key_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_key,
> +   2 * AES_KEYSIZE_128, DMA_TO_DEVICE);
> + ret = dma_mapping_error(sdcp->dev, key_phys);
> + if (ret)
> + return ret;
> + }
>  
>   src_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_in_buf,
> DCP_BUF_SZ, DMA_TO_DEVICE);
> @@ -255,8 +263,12 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
>   MXS_DCP_CONTROL0_INTERRUPT |
>   MXS_DCP_CONTROL0_ENABLE_CIPHER;
>  
> - /* Payload contains the key. */
> - desc->control0 |= MXS_DCP_CONTROL0_PAYLOAD_KEY;
> + if (key_referenced)
> + /* Set OTP key bit to select the key via KEY_SELECT. */
> + desc->control0 |= MXS_DCP_CONTROL0_OTP_KEY;
> + else
> + /* Payload contains the key. */
> + desc->control0 |= MXS_DCP_CONTROL0_PAYLOAD_KEY;
>  
>   if (rctx->enc)
>   desc->control0 |= MXS_DCP_CONTROL0_CIPHER_ENCRYPT;
> @@ -270,6 +282,9 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
>   else
>   desc->control1 |= MXS_DCP_CONTROL1_CIPHER_MODE_CBC;
>  
> + if (key_referenced)
> + desc->control1 |= sdcp->coh->aes_key[0] << 
> MXS_DCP_CONTROL1_KEY_SELECT_SHIFT;
> +
>   desc->next_cmd_addr = 0;
>   desc->source = src_phys;
>   desc->destination = dst_phys;
> @@ -284,9 +299,9 @@ static int

Re: [PATCH v2 0/9] iommu: Convert dart & iommufd to the new domain_alloc_paging()

2023-10-25 Thread jgg

On Wed, Sep 27, 2023 at 08:47:30PM -0300, Jason Gunthorpe wrote:
> Continue converting drivers to the new interface. Introduce
> ops->blocked_domain to hold the global static BLOCKED domain and convert
> all drivers supporting BLOCKED to use it.
> 
> This makes it trivial for dart and iommufd to convert over to
> domain_alloc_paging().
> 
> There are six drivers remaining:
> 
> drivers/iommu/amd/iommu.c:  .domain_alloc = amd_iommu_domain_alloc,
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:.domain_alloc   = 
> arm_smmu_domain_alloc,
> drivers/iommu/arm/arm-smmu/arm-smmu.c:  .domain_alloc   = 
> arm_smmu_domain_alloc,
> drivers/iommu/fsl_pamu_domain.c:.domain_alloc   = 
> fsl_pamu_domain_alloc,
> drivers/iommu/intel/iommu.c:.domain_alloc   = 
> intel_iommu_domain_alloc,
> drivers/iommu/virtio-iommu.c:   .domain_alloc   = viommu_domain_alloc,
> 
> v2:
>  - Rebase to Joerg's for-next
>  - New patch to remove force_bypass, as discussed with Janne
>  - Move some hunks between patches to accommodate Robin's change to the
>attach_dev switch
> v1: 
> https://lore.kernel.org/r/0-v1-8060f06462cc+c0a39-dart_paging_...@nvidia.com
> 
> Jason Gunthorpe (9):
>   iommu: Move IOMMU_DOMAIN_BLOCKED global statics to ops->blocked_domain
>   iommu/vt-d: Update the definition of the blocking domain
>   iommu/vt-d: Use ops->blocked_domain
>   iommufd: Convert to alloc_domain_paging()

Joerg can you grab these for this cycle please

>   iommu/dart: Use static global identity domains
>   iommu/dart: Move the blocked domain support to a global static
>   iommu/dart: Convert to domain_alloc_paging()
>   iommu/dart: Call apple_dart_finalize_domain() as part of
> alloc_paging()
>   iommu/dart: Remove the force_bypass variable

I will poke more dart related people to get a tested-by, maybe next
cycle.

The arm patches at least need this

Thanks,
Jason

Re: [PATCH v8 00/30] Add support for QMC HDLC, framer infrastructure and PEF2256 framer

2023-10-25 Thread Herve Codina

Hi All, Maintainers

On Fri, 13 Oct 2023 16:46:47 -0700
Jakub Kicinski  wrote:

> On Wed, 11 Oct 2023 08:14:04 +0200 Herve Codina wrote:
> > Compare to the previous iteration
> >   
> > https://lore.kernel.org/linux-kernel/20230928070652.330429-1-herve.cod...@bootlin.com/
> > This v8 series:
> >  - Fixes a race condition
> >  - Uses menuconfig instead of menu and hides CONFIG_GENERIC_FRAMER
> >  - Performs minor changes  
> 
> Which way will those patches go? Via some FSL SoC tree?

This series seems mature now.
What is the plan next in order to have it applied ?

Don't hesitate to tell me if you prefer split series.

Best regards,
Hervé

Re: [PATCH v3] powerpc/pseries/vas: Migration suspend waits for no in-progress open windows

2023-10-25 Thread Nathan Lynch

Haren Myneni  writes:
> The hypervisor returns migration failure if all VAS windows are not
> closed. During pre-migration stage, vas_migration_handler() sets
> migration_in_progress flag and closes all windows from the list.
> The allocate VAS window routine checks the migration flag, setup
> the window and then add it to the list. So there is possibility of
> the migration handler missing the window that is still in the
> process of setup.
>
> t1: Allocate and open VAS t2: Migration event
> window
>
> lock vas_pseries_mutex
> If migration_in_progress set
>   unlock vas_pseries_mutex
>   return
> open window HCALL
> unlock vas_pseries_mutex
> Modify window HCALL   lock vas_pseries_mutex
> setup window  migration_in_progress=true
>   Closes all windows from
>   the list
>   unlock vas_pseries_mutex
> lock vas_pseries_mutexreturn
> if nr_closed_windows == 0
>   // No DLPAR CPU or migration
>   add to the list
>   unlock vas_pseries_mutex
>   return
> unlock vas_pseries_mutex
> Close VAS window
> // due to DLPAR CPU or migration
> return -EBUSY
>
> This patch resolves the issue with the following steps:
> - Set the migration_in_progress flag without holding mutex.
> - Introduce nr_open_wins_progress counter in VAS capabilities
>   struct
> - This counter tracks the number of open windows are still in
>   progress
> - The allocate setup window thread closes windows if the migration
>   is set and decrements nr_open_window_progress counter
> - The migration handler waits for no in-progress open windows.
>
> Fixes: 37e6764895ef ("powerpc/pseries/vas: Add VAS migration handler")
> Signed-off-by: Haren Myneni 

Thanks for bearing with me Haren. While I still think this code would
benefit from reevaluating the overall locking model, that can be taken
up later. I can't identify any bugs in this version.

Reviewed-by: Nathan Lynch

Re: [RFC PATCH v7 12/13] media: imx-asrc: Add memory to memory driver

On 20/10/2023 11:30, Shengjiu Wang wrote:
> Implement the ASRC memory to memory function using
> the v4l2 framework, user can use this function with
> v4l2 ioctl interface.
> 
> User send the output and capture buffer to driver and
> driver store the converted data to the capture buffer.
> 
> This feature can be shared by ASRC and EASRC drivers
> 
> Signed-off-by: Shengjiu Wang 
> ---
>  drivers/media/platform/nxp/Kconfig|   12 +
>  drivers/media/platform/nxp/Makefile   |1 +
>  drivers/media/platform/nxp/imx-asrc.c | 1207 +
>  3 files changed, 1220 insertions(+)
>  create mode 100644 drivers/media/platform/nxp/imx-asrc.c
> 
> diff --git a/drivers/media/platform/nxp/Kconfig 
> b/drivers/media/platform/nxp/Kconfig
> index 40e3436669e2..8234644ee341 100644
> --- a/drivers/media/platform/nxp/Kconfig
> +++ b/drivers/media/platform/nxp/Kconfig
> @@ -67,3 +67,15 @@ config VIDEO_MX2_EMMAPRP
>  
>  source "drivers/media/platform/nxp/dw100/Kconfig"
>  source "drivers/media/platform/nxp/imx-jpeg/Kconfig"
> +
> +config VIDEO_IMX_ASRC
> + tristate "NXP i.MX ASRC M2M support"
> + depends on V4L_MEM2MEM_DRIVERS
> + depends on MEDIA_SUPPORT
> + select VIDEOBUF2_DMA_CONTIG
> + select V4L2_MEM2MEM_DEV
> + help
> + Say Y if you want to add ASRC M2M support for NXP CPUs.
> + It is a complement for ASRC M2P and ASRC P2M features.
> + This option is only useful for out-of-tree drivers since
> + in-tree drivers select it automatically.
> diff --git a/drivers/media/platform/nxp/Makefile 
> b/drivers/media/platform/nxp/Makefile
> index 4d90eb713652..1325675e34f5 100644
> --- a/drivers/media/platform/nxp/Makefile
> +++ b/drivers/media/platform/nxp/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_VIDEO_IMX8MQ_MIPI_CSI2) += imx8mq-mipi-csi2.o
>  obj-$(CONFIG_VIDEO_IMX_MIPI_CSIS) += imx-mipi-csis.o
>  obj-$(CONFIG_VIDEO_IMX_PXP) += imx-pxp.o
>  obj-$(CONFIG_VIDEO_MX2_EMMAPRP) += mx2_emmaprp.o
> +obj-$(CONFIG_VIDEO_IMX_ASRC) += imx-asrc.o
> diff --git a/drivers/media/platform/nxp/imx-asrc.c 
> b/drivers/media/platform/nxp/imx-asrc.c
> new file mode 100644
> index ..b1b5f1bbdf19
> --- /dev/null
> +++ b/drivers/media/platform/nxp/imx-asrc.c
> @@ -0,0 +1,1207 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Copyright (C) 2014-2016 Freescale Semiconductor, Inc.
> +// Copyright (C) 2019-2023 NXP
> +//
> +// Freescale ASRC Memory to Memory (M2M) driver
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define V4L_CAP OUT
> +#define V4L_OUT IN
> +
> +#define ASRC_xPUT_DMA_CALLBACK(dir) \
> + (((dir) == V4L_OUT) ? asrc_input_dma_callback \
> + : asrc_output_dma_callback)
> +
> +#define DIR_STR(dir) (dir) == V4L_OUT ? "out" : "cap"
> +
> +/* Maximum output and capture buffer size */
> +#define ASRC_M2M_BUFFER_SIZE (512 * 1024)
> +
> +/* Maximum output and capture period size */
> +#define ASRC_M2M_PERIOD_SIZE (48 * 1024)
> +
> +struct asrc_pair_m2m {
> + struct fsl_asrc_pair *pair;
> + struct asrc_m2m *m2m;
> + struct v4l2_fh fh;
> + struct v4l2_ctrl_handler ctrl_handler;
> + int channels[2];
> + s64 src_rate_off_prev;  /* Q31.32 */
> + s64 dst_rate_off_prev;  /* Q31.32 */
> + s64 src_rate_off_cur;   /* Q31.32 */
> + s64 dst_rate_off_cur;   /* Q31.32 */
> +};
> +
> +struct asrc_m2m {
> + struct fsl_asrc_m2m_pdata pdata;
> + struct v4l2_device v4l2_dev;
> + struct v4l2_m2m_dev *m2m_dev;
> + struct video_device *dec_vdev;
> + struct mutex mlock; /* v4l2 ioctls serialization */
> + struct platform_device *pdev;
> +};
> +
> +static u32 formats[] = {
> + V4L2_AUDIO_FMT_S8,
> + V4L2_AUDIO_FMT_S16_LE,
> + V4L2_AUDIO_FMT_U16_LE,
> + V4L2_AUDIO_FMT_S24_LE,
> + V4L2_AUDIO_FMT_S24_3LE,
> + V4L2_AUDIO_FMT_U24_LE,
> + V4L2_AUDIO_FMT_U24_3LE,
> + V4L2_AUDIO_FMT_S32_LE,
> + V4L2_AUDIO_FMT_U32_LE,
> + V4L2_AUDIO_FMT_S20_3LE,
> + V4L2_AUDIO_FMT_U20_3LE,
> + V4L2_AUDIO_FMT_FLOAT_LE,
> + V4L2_AUDIO_FMT_IEC958_SUBFRAME_LE,
> +};
> +
> +#define NUM_FORMATS ARRAY_SIZE(formats)
> +
> +static const s64 asrc_v1_m2m_rates[] = {
> + 5512, 8000, 11025, 12000, 16000,
> + 22050, 24000, 32000, 44100,
> + 48000, 64000, 88200, 96000,
> + 128000, 176400, 192000,
> +};
> +
> +static const s64 asrc_v2_m2m_rates[] = {
> + 8000, 11025, 12000, 16000,
> + 22050, 24000, 32000, 44100,
> + 48000, 64000, 88200, 96000,
> + 128000, 176400, 192000, 256000,
> + 352800, 384000, 705600, 768000,
> +};
> +
> +static u32 find_fourcc(snd_pcm_format_t format)
> +{
> + snd_pcm_format_t fmt;
> + unsigned int k;
> +
> + for (k = 0; k < NUM_FORMATS; k++) {
> + fmt = v4l2_fourcc_to_audfmt(formats[k]);
> + if (fmt == format)
> + break;

Just do 'return formats[k];'

Re: [RFC PATCH v7 11/13] media: uapi: Add audio rate controls support

On 20/10/2023 11:30, Shengjiu Wang wrote:
> Add V4L2_CID_M2M_AUDIO_SOURCE_RATE and V4L2_CID_M2M_AUDIO_DEST_RATE
> new IDs for rate control.
> 
> Add V4L2_CID_M2M_AUDIO_SOURCE_RATE_OFFSET and
> V4L2_CID_M2M_AUDIO_DEST_RATE_OFFSET for clock drift.
> 
> Signed-off-by: Shengjiu Wang 
> ---
>  .../media/v4l/ext-ctrls-audio-m2m.rst  | 18 ++
>  drivers/media/v4l2-core/v4l2-ctrls-defs.c  | 12 
>  include/uapi/linux/v4l2-controls.h |  5 +
>  3 files changed, 35 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-audio-m2m.rst 
> b/Documentation/userspace-api/media/v4l/ext-ctrls-audio-m2m.rst
> index 82d2ecedbfee..e6972a2d3b17 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-audio-m2m.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-audio-m2m.rst
> @@ -19,3 +19,21 @@ Audio M2M Control IDs
>  The Audio M2M class descriptor. Calling
>  :ref:`VIDIOC_QUERYCTRL` for this control will
>  return a description of this control class.
> +
> +.. _v4l2-audio-asrc:
> +
> +``V4L2_CID_M2M_AUDIO_SOURCE_RATE``

Add ' (integer menu)' after the name.

> +Sets the audio source rate, unit is (Hz)

(Hz) -> Hz.

General question: is 'rate' good enough or should it be 'sample rate'?

> +
> +``V4L2_CID_M2M_AUDIO_DEST_RATE``
> +Sets the audio destination rate, unit is (Hz)

Ditto here.

> +
> +``V4L2_CID_M2M_AUDIO_SOURCE_RATE_OFFSET``

Add ' (fixed point)' after the name.

> +Sets the offset for audio source rate, unit is (Hz).

for -> from the

(Hz) -> Hz

> +Offset expresses the drift of clock if there is. It is
> +equal to real rate minus ideal rate.

How about:

The offset compensates for any clock drift. The actual source audio
rate is the ideal source audio rate from ``V4L2_CID_M2M_AUDIO_SOURCE_RATE``
plus this fixed point offset.

> +
> +``V4L2_CID_M2M_AUDIO_DEST_RATE_OFFSET``
> +Sets the offset for audio destination rate, unit is (Hz)
> +Offset expresses the drift of clock if there is. It is
> +equal to real rate minus ideal rate.

Same changes as for V4L2_CID_M2M_AUDIO_SOURCE_RATE_OFFSET.

> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c 
> b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> index 2a85ea3dc92f..b695cbdd1f6e 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> @@ -1245,6 +1245,10 @@ const char *v4l2_ctrl_get_name(u32 id)
>  
>   /* Audio M2M controls */
>   case V4L2_CID_M2M_AUDIO_CLASS:  return "Audio M2M Controls";
> + case V4L2_CID_M2M_AUDIO_SOURCE_RATE:return "Audio Source Sample 
> Rate";
> + case V4L2_CID_M2M_AUDIO_DEST_RATE:  return "Audio Dest Sample Rate";
> + case V4L2_CID_M2M_AUDIO_SOURCE_RATE_OFFSET: return "Audio Source 
> Sample Rate Offset";
> + case V4L2_CID_M2M_AUDIO_DEST_RATE_OFFSET:   return "Audio Dest 
> Sample Rate Offset";

Related to my question above: "Sample Rate" or just "Rate"? Whatever we pick, 
it should
be consistent.

>   default:
>   return NULL;
>   }
> @@ -1606,6 +1610,14 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum 
> v4l2_ctrl_type *type,
>   case V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY:
>   *type = V4L2_CTRL_TYPE_HDR10_MASTERING_DISPLAY;
>   break;
> + case V4L2_CID_M2M_AUDIO_SOURCE_RATE:
> + case V4L2_CID_M2M_AUDIO_DEST_RATE:
> + *type = V4L2_CTRL_TYPE_INTEGER_MENU;
> + break;
> + case V4L2_CID_M2M_AUDIO_SOURCE_RATE_OFFSET:
> + case V4L2_CID_M2M_AUDIO_DEST_RATE_OFFSET:
> + *type = V4L2_CTRL_TYPE_FIXED_POINT;
> + break;
>   default:
>   *type = V4L2_CTRL_TYPE_INTEGER;
>   break;
> diff --git a/include/uapi/linux/v4l2-controls.h 
> b/include/uapi/linux/v4l2-controls.h
> index eb0f0a76f867..d433c6f0b533 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -3498,4 +3498,9 @@ struct v4l2_ctrl_av1_film_grain {
>  #define V4L2_CID_M2M_AUDIO_CLASS_BASE  (V4L2_CTRL_CLASS_M2M_AUDIO | 0x900)
>  #define V4L2_CID_M2M_AUDIO_CLASS   (V4L2_CTRL_CLASS_M2M_AUDIO | 1)
>  
> +#define V4L2_CID_M2M_AUDIO_SOURCE_RATE   (V4L2_CID_M2M_AUDIO_CLASS_BASE 
> + 0)
> +#define V4L2_CID_M2M_AUDIO_DEST_RATE (V4L2_CID_M2M_AUDIO_CLASS_BASE + 1)
> +#define V4L2_CID_M2M_AUDIO_SOURCE_RATE_OFFSET
> (V4L2_CID_M2M_AUDIO_CLASS_BASE + 2)
> +#define V4L2_CID_M2M_AUDIO_DEST_RATE_OFFSET  (V4L2_CID_M2M_AUDIO_CLASS_BASE 
> + 3)
> +
>  #endif

Regards,

Hans

Re: [RFC PATCH v7 10/13] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT

On 20/10/2023 11:30, Shengjiu Wang wrote:
> Fixed point controls are used by the user to configure
> a fixed point value in 64bits, which Q31.32 format.
> 
> Signed-off-by: Shengjiu Wang 
> ---
>  Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst  | 6 ++
>  .../userspace-api/media/videodev2.h.rst.exceptions  | 1 +
>  drivers/media/v4l2-core/v4l2-ctrls-api.c| 5 -
>  drivers/media/v4l2-core/v4l2-ctrls-core.c   | 2 ++
>  include/uapi/linux/videodev2.h  | 1 +
>  5 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst 
> b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
> index 4d38acafe8e1..2e15db0779f2 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
> @@ -549,6 +549,12 @@ See also the examples in :ref:`control`.
>- n/a
>- A struct :c:type:`v4l2_ctrl_av1_film_grain`, containing AV1 Film 
> Grain
>  parameters for stateless video decoders.
> +* - ``V4L2_CTRL_TYPE_FIXED_POINT``
> +  - n/a
> +  - n/a
> +  - n/a
> +  - A struct :c:type:`v4l2_ctrl_fixed_point`, containing parameter which 
> is
> +Q31.32 format.

No, this isn't a struct. It's a 64 bit integer.

Also search for INTEGER64 in this file, it's mentioned in the "default_value"
documentation, and _FIXED_POINT needs to be added to that text.

>  
>  .. raw:: latex
>  

You also need to update 
Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst,
search for INTEGER64 in that file.

> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions 
> b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> index e61152bb80d1..2faa5a2015eb 100644
> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> @@ -167,6 +167,7 @@ replace symbol V4L2_CTRL_TYPE_AV1_SEQUENCE 
> :c:type:`v4l2_ctrl_type`
>  replace symbol V4L2_CTRL_TYPE_AV1_TILE_GROUP_ENTRY :c:type:`v4l2_ctrl_type`
>  replace symbol V4L2_CTRL_TYPE_AV1_FRAME :c:type:`v4l2_ctrl_type`
>  replace symbol V4L2_CTRL_TYPE_AV1_FILM_GRAIN :c:type:`v4l2_ctrl_type`
> +replace symbol V4L2_CTRL_TYPE_FIXED_POINT :c:type:`v4l2_ctrl_type`
>  
>  # V4L2 capability defines
>  replace define V4L2_CAP_VIDEO_CAPTURE device-capabilities
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c 
> b/drivers/media/v4l2-core/v4l2-ctrls-api.c
> index 002ea6588edf..e6a0fb8d6791 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-api.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c
> @@ -57,6 +57,7 @@ static int ptr_to_user(struct v4l2_ext_control *c,
>   return copy_to_user(c->string, ptr.p_char, len + 1) ?
>  -EFAULT : 0;
>   case V4L2_CTRL_TYPE_INTEGER64:
> + case V4L2_CTRL_TYPE_FIXED_POINT:
>   c->value64 = *ptr.p_s64;
>   break;
>   default:
> @@ -132,6 +133,7 @@ static int user_to_new(struct v4l2_ext_control *c, struct 
> v4l2_ctrl *ctrl)
>  
>   switch (ctrl->type) {
>   case V4L2_CTRL_TYPE_INTEGER64:
> + case V4L2_CTRL_TYPE_FIXED_POINT:
>   *ctrl->p_new.p_s64 = c->value64;
>   break;
>   case V4L2_CTRL_TYPE_STRING:
> @@ -540,7 +542,8 @@ static int validate_ctrls(struct v4l2_ext_controls *cs,
>*/
>   if (ctrl->is_ptr)
>   continue;
> - if (ctrl->type == V4L2_CTRL_TYPE_INTEGER64)
> + if (ctrl->type == V4L2_CTRL_TYPE_INTEGER64 ||
> + ctrl->type == V4L2_CTRL_TYPE_FIXED_POINT)
>   p_new.p_s64 = >controls[i].value64;
>   else
>   p_new.p_s32 = >controls[i].value;
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c 
> b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> index a662fb60f73f..9d50df0d9874 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> @@ -1187,6 +1187,7 @@ static int std_validate_elem(const struct v4l2_ctrl 
> *ctrl, u32 idx,
>   case V4L2_CTRL_TYPE_INTEGER:
>   return ROUND_TO_RANGE(ptr.p_s32[idx], u32, ctrl);
>   case V4L2_CTRL_TYPE_INTEGER64:
> + case V4L2_CTRL_TYPE_FIXED_POINT:
>   /*
>* We can't use the ROUND_TO_RANGE define here due to
>* the u64 divide that needs special care.
> @@ -1779,6 +1780,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
> v4l2_ctrl_handler *hdl,
>   /* Prefill elem_size for all types handled by std_type_ops */
>   switch ((u32)type) {
>   case V4L2_CTRL_TYPE_INTEGER64:
> + case V4L2_CTRL_TYPE_FIXED_POINT:
>   elem_size = sizeof(s64);
>   break;
>   case V4L2_CTRL_TYPE_STRING:
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>

Re: [RFC PATCH v7 09/13] media: uapi: Add V4L2_CTRL_CLASS_M2M_AUDIO

On 20/10/2023 11:30, Shengjiu Wang wrote:
> The Audio M2M class includes controls for audio memory-to-memory
> use cases. The controls can be used for audio codecs, audio
> preprocessing, audio postprocessing.
> 
> Signed-off-by: Shengjiu Wang 
> ---
>  .../userspace-api/media/v4l/common.rst|  1 +
>  .../media/v4l/ext-ctrls-audio-m2m.rst | 21 +++
>  .../media/v4l/vidioc-g-ext-ctrls.rst  |  4 
>  drivers/media/v4l2-core/v4l2-ctrls-defs.c |  4 
>  include/uapi/linux/v4l2-controls.h|  4 
>  5 files changed, 34 insertions(+)
>  create mode 100644 
> Documentation/userspace-api/media/v4l/ext-ctrls-audio-m2m.rst
> 
> diff --git a/Documentation/userspace-api/media/v4l/common.rst 
> b/Documentation/userspace-api/media/v4l/common.rst
> index ea0435182e44..d5366e96a596 100644
> --- a/Documentation/userspace-api/media/v4l/common.rst
> +++ b/Documentation/userspace-api/media/v4l/common.rst
> @@ -52,6 +52,7 @@ applicable to all devices.
>  ext-ctrls-fm-rx
>  ext-ctrls-detect
>  ext-ctrls-colorimetry
> +ext-ctrls-audio-m2m
>  fourcc
>  format
>  planar-apis
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-audio-m2m.rst 
> b/Documentation/userspace-api/media/v4l/ext-ctrls-audio-m2m.rst
> new file mode 100644
> index ..82d2ecedbfee
> --- /dev/null
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-audio-m2m.rst
> @@ -0,0 +1,21 @@
> +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
> +
> +.. _audiom2m-controls:
> +
> +***
> +Audio M2M Control Reference
> +***
> +
> +The Audio M2M class includes controls for audio memory-to-memory
> +use cases. The controls can be used for audio codecs, audio
> +preprocessing, audio postprocessing.
> +
> +Audio M2M Control IDs
> +---
> +
> +.. _audiom2m-control-id:
> +
> +``V4L2_CID_M2M_AUDIO_CLASS (class)``
> +The Audio M2M class descriptor. Calling
> +:ref:`VIDIOC_QUERYCTRL` for this control will
> +return a description of this control class.
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst 
> b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> index f9f73530a6be..e8475f9fd2cf 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> @@ -480,6 +480,10 @@ still cause this situation.
>- 0xa5
>- The class containing colorimetry controls. These controls are
>   described in :ref:`colorimetry-controls`.
> +* - ``V4L2_CTRL_CLASS_M2M_AUDIO``
> +  - 0xa6
> +  - The class containing audio m2m controls. These controls are
> + described in :ref:`audiom2m-controls`.
>  
>  Return Value
>  
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c 
> b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> index 8696eb1cdd61..2a85ea3dc92f 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> @@ -1242,6 +1242,9 @@ const char *v4l2_ctrl_get_name(u32 id)
>   case V4L2_CID_COLORIMETRY_CLASS:return "Colorimetry Controls";
>   case V4L2_CID_COLORIMETRY_HDR10_CLL_INFO:   return "HDR10 
> Content Light Info";
>   case V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY:  return "HDR10 
> Mastering Display";
> +
> + /* Audio M2M controls */
> + case V4L2_CID_M2M_AUDIO_CLASS:  return "Audio M2M Controls";
>   default:
>   return NULL;
>   }
> @@ -1451,6 +1454,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum 
> v4l2_ctrl_type *type,
>   case V4L2_CID_DETECT_CLASS:
>   case V4L2_CID_CODEC_STATELESS_CLASS:
>   case V4L2_CID_COLORIMETRY_CLASS:
> + case V4L2_CID_M2M_AUDIO_CLASS:
>   *type = V4L2_CTRL_TYPE_CTRL_CLASS;
>   /* You can neither read nor write these */
>   *flags |= V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_WRITE_ONLY;
> diff --git a/include/uapi/linux/v4l2-controls.h 
> b/include/uapi/linux/v4l2-controls.h
> index 68db66d4aae8..eb0f0a76f867 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -30,6 +30,7 @@
>  #define V4L2_CTRL_CLASS_DETECT   0x00a3  /* Detection 
> controls */
>  #define V4L2_CTRL_CLASS_CODEC_STATELESS 0x00a4   /* Stateless codecs 
> controls */
>  #define V4L2_CTRL_CLASS_COLORIMETRY  0x00a5  /* Colorimetry controls 
> */
> +#define V4L2_CTRL_CLASS_M2M_AUDIO0x00a6  /* Audio M2M controls */
>  
>  /* User-class control IDs */
>  
> @@ -3494,4 +3495,7 @@ struct v4l2_ctrl_av1_film_grain {
>  #define V4L2_CID_MPEG_MFC51_BASEV4L2_CID_CODEC_MFC51_BASE
>  #endif
>  
> +#define V4L2_CID_M2M_AUDIO_CLASS_BASE  (V4L2_CTRL_CLASS_M2M_AUDIO | 0x900)
> +#define V4L2_CID_M2M_AUDIO_CLASS   (V4L2_CTRL_CLASS_M2M_AUDIO | 1)
> +

This should be moved up

Re: [RFC PATCH v7 06/13] media: uapi: Add V4L2_CAP_AUDIO_M2M capability flag