Re: [PATCH 1/3] soc cache: L3 cache driver for HiSilicon SoC

["Arnd Bergmann" <arnd@arndb.de>]

On Tue, Feb 3, 2026, at 17:18, Yushan Wang wrote:
> The driver will create a file of `/dev/hisi_l3c` on init, mmap
> operations to it will allocate a memory region that is guaranteed to be
> placed in L3 cache.
>
> The driver also provides unmap() to deallocated the locked memory.
>
> The driver also provides an ioctl interface for user to get cache lock
> information, such as lock restrictions and locked sizes.
>
> Signed-off-by: Yushan Wang <wangyushan12@huawei.com>

Hi Yushan,

Thanks for your submission. Since we are in the last week of
the merge window, this is not going to be linux-7.0 material,
but I'll have a quick look for now.

>  .../userspace-api/ioctl/ioctl-number.rst      |   1 +
>  MAINTAINERS                                   |   6 +
>  drivers/soc/hisilicon/Kconfig                 |  11 +
>  drivers/soc/hisilicon/Makefile                |   2 +
>  drivers/soc/hisilicon/hisi_soc_l3c.c          | 357 ++++++++++++++++++
>  include/uapi/misc/hisi_l3c.h                  |  28 ++

I don't think this should be in drivers/soc/, since I want
to reserve that for internal drivers without a user visible
interface other than the soc_device information. (yes, there
are a few historic counterexamples)

I also don't think this should be a hilicon specific interface,
if possible. The functionality is not that unusual in the end.

We had similar concepts using the numactl system calls in the
part, but I don't think we should do that here because you may
need the numa interfaces for other purposes as well, and it
may be confusing to existing callers.

Having a generic madvise() based interface would be great,
not sure if hardware support for that is common enough for that.

> +	/* Continuous physical memory is required for L3 cache lock. */
> +	pg = alloc_contig_pages(1 << order, GFP_KERNEL | __GFP_NOWARN | 
> __GFP_ZERO,
> +				cpu_to_node(smp_processor_id()), NULL);

Since this is a user allocation, should that be GFP_USER instead
of GFP_KERNEL?

> +/* HISI_L3C_INFO: cache lock info for HiSilicon SoC */
> +#define HISI_L3C_LOCK_INFO	_IOW(0xBB, 1, unsigned long)

The specification here looks wrong, please see
Documentation/driver-api/ioctl.rst

I think for your implementation it should be

#define HISI_L3C_LOCK_INFO	_IOR(0xBB, 1, hisi_l3c_lock_info)

> +struct hisi_l3c_lock_info {
> +	__u32 lock_region_num;
> +	__u64 lock_size;
> +	__u8 address_alignment;
> +	__u64 max_lock_size;
> +	__u64 min_lock_size;
> +};

You are leaking kernel data because of the padding in this structure,
please rearrange the members to avoid padding.

It may be better to use a different interface instead of ioctl(),
possibly exporting global data in sysfs.

       Arnd