Alexey Kardashevskiy wrote:
> PCIe 6.0 introduces the "Integrity & Data Encryption (IDE)" feature which
> adds a new capability with id=0x30.
>
> Add the new id to the list of capabilities. Add new flags from pciutils.
> Add a module with a helper to control selective IDE capability.
>
> TODO: get rid of lots of magic numbers. It is one annoying flexible
> capability to deal with.
This changelog needs a theory of operation to explain how it is used and
to give some chance of reviewing whether this is implementing more than
is required to get the job done.
> Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxx>
> ---
> drivers/pci/Makefile | 1 +
> include/linux/pci-ide.h | 18 ++
> include/uapi/linux/pci_regs.h | 76 +++++++-
> drivers/pci/ide.c | 186 ++++++++++++++++++++
> drivers/pci/Kconfig | 4 +
> 5 files changed, 284 insertions(+), 1 deletion(-)
>
[..]
> diff --git a/include/linux/pci-ide.h b/include/linux/pci-ide.h
> new file mode 100644
> index 000000000000..983a8daf1199
> --- /dev/null
> +++ b/include/linux/pci-ide.h
[..]
> diff --git a/drivers/pci/ide.c b/drivers/pci/ide.c
> new file mode 100644
> index 000000000000..dc0632e836e7
> --- /dev/null
> +++ b/drivers/pci/ide.c
> @@ -0,0 +1,186 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Integrity & Data Encryption (IDE)
> + * PCIe r6.0, sec 6.33 DOE
> + */
> +
> +#define dev_fmt(fmt) "IDE: " fmt
> +
> +#include <linux/pci.h>
> +#include <linux/pci-ide.h>
> +#include <linux/bitfield.h>
> +#include <linux/module.h>
> +
> +#define DRIVER_VERSION "0.1"
> +#define DRIVER_AUTHOR "aik@xxxxxxx"
> +#define DRIVER_DESC "Integrity and Data Encryption driver"
This is not a driver. It is a helper library for the TSM core and maybe
native PCI IDE establishment in the future.
I am not what purpose AUTHOR, VERSION and DESC, serve as I do not see
how this can get away with being a module when the TSM core is built-in
to be initialized via pci_init_capabilities().
[..]
Difficult to review the implementation without a clear idea of the user,
and the exports are not necessary if the only consumer is the PCI core.
> +
> +static int __init ide_init(void)
> +{
> + int ret = 0;
> +
> + pr_info(DRIVER_DESC " version: " DRIVER_VERSION "\n");
> + return ret;
> +}
> +
> +static void __exit ide_cleanup(void)
> +{
> +}
> +
> +module_init(ide_init);
> +module_exit(ide_cleanup);
> +
> +MODULE_VERSION(DRIVER_VERSION);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR(DRIVER_AUTHOR);
> +MODULE_DESCRIPTION(DRIVER_DESC);
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index b0b14468ba5d..8e908d684c77 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -137,6 +137,10 @@ config PCI_CMA
> config PCI_DOE
> bool
>
> +config PCI_IDE
> + tristate
> + default m
Setting aside the tristate, no new kernel code should unconditionally
enable itself by default.
