[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJZ5v0gvLDu8xGUVR0kATko8M37RiYg63t+1BWFox499QJxSpg@mail.gmail.com>
Date: Tue, 19 Oct 2021 18:33:08 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Chen Yu <yu.c.chen@...el.com>,
ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
Ard Biesheuvel <ardb@...nel.org>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Len Brown <lenb@...nel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Ashok Raj <ashok.raj@...el.com>,
Andy Shevchenko <andriy.shevchenko@...el.com>,
Mike Rapoport <rppt@...nel.org>,
Aubrey Li <aubrey.li@...el.com>,
Jonathan Corbet <corbet@....net>
Subject: Re: [PATCH v4 2/4] drivers/acpi: Introduce Platform Firmware Runtime
Update device driver
On Sat, Oct 16, 2021 at 5:16 PM Greg Kroah-Hartman
<gregkh@...uxfoundation.org> wrote:
>
> On Sat, Oct 16, 2021 at 06:40:51PM +0800, Chen Yu wrote:
> > Introduce the pfru_update driver which can be used for Platform Firmware
> > Runtime code injection and driver update[1]. The user is expected to
> > provide the update firmware in the form of capsule file, and pass it to
> > the driver via ioctl. Then the driver would hand this capsule file to the
> > Platform Firmware Runtime Update via the ACPI device _DSM method. At last
> > the low level Management Mode would do the firmware update.
> >
> > [1] https://uefi.org/sites/default/files/resources/Intel_MM_OS_Interface_Spec_Rev100.pdf
> >
> > Signed-off-by: Chen Yu <yu.c.chen@...el.com>
> > ---
> > v4: Add Documentation/ABI/testing/pfru (Rafael J. Wysocki)
> > Change all pr_debug() to dev_dbg() (Greg Kroah-Hartman,
> > Rafael J. Wysocki)
> > Change the error code ENOIOCTLCMD to ENOTTY in ioctl.
> > (Greg Kroah-Hartman)
> > Remove compat ioctl. (Greg Kroah-Hartman)
> > Change /dev/pfru/update to /dev/acpi_pfru (Greg Kroah-Hartman)
> > Remove valid_cap_type() and do sanity check in query_capability().
> > (Rafael J. Wysocki)
> > Remove the loop in query_capability().
> > (Rafael J. Wysocki)
> > Do not fail if the package has more elements than expected,
> > and return error if the number of package elements is too
> > small. (Rafael J. Wysocki)
> > Return the type or a negative error code in get_image_type()
> > (Rafael J. Wysocki)
> > Put the comment inside the function rather than outside.
> > (Rafael J. Wysocki)
> > Return the size or a negative error code adjust_efi_size()
> > (Rafael J. Wysocki)
> > Return -EINVAL rather than -EFAULT if revison id is incorrect.
> > (Rafael J. Wysocki)
> > Move the an read() of pfru into ioctl(), and using read() for
> > the telemetry retrieval. So as to avoid the telemetry device
> > file, the write() will be the code injection/update, the read()
> > will be telemetry retrieval and all of the rest can be ioctl()s
> > under one special device file.
> > (Rafael J. Wysocki)
> > v3: Use __u32 instead of int and __64 instead of unsigned long
> > in include/uapi/linux/pfru.h (Greg Kroah-Hartman)
> > Rename the structure in uapi to start with a prefix pfru so as
> > to avoid confusing in the global namespace. (Greg Kroah-Hartman)
> > v2: Add sanity check for duplicated instance of ACPI device.
> > Update the driver to work with allocated pfru_device objects.
> > (Mike Rapoport)
> > For each switch case pair, get rid of the magic case numbers
> > and add a default clause with the error handling.
> > (Mike Rapoport)
> > Move the obj->type checks outside the switch to reduce redundancy.
> > (Mike Rapoport)
> > Parse the code_inj_id and drv_update_id at driver initialization time
> > to reduce the re-parsing at runtime.(Mike Rapoport)
> > Explain in detail how the size needs to be adjusted when doing
> > version check.(Mike Rapoport)
> > Rename parse_update_result() to dump_update_result()(Mike Rapoport)
> > Remove redundant return.(Mike Rapoport)
> > Do not expose struct capsulate_buf_info to uapi, since it is
> > not needed in userspace.(Mike Rapoport)
> > ---
> > Documentation/ABI/testing/pfru | 41 ++
> > .../userspace-api/ioctl/ioctl-number.rst | 1 +
> > drivers/acpi/Kconfig | 1 +
> > drivers/acpi/Makefile | 1 +
> > drivers/acpi/pfru/Kconfig | 16 +
> > drivers/acpi/pfru/Makefile | 2 +
> > drivers/acpi/pfru/pfru_update.c | 567 ++++++++++++++++++
> > include/uapi/linux/pfru.h | 102 ++++
> > 8 files changed, 731 insertions(+)
> > create mode 100644 Documentation/ABI/testing/pfru
> > create mode 100644 drivers/acpi/pfru/Kconfig
> > create mode 100644 drivers/acpi/pfru/Makefile
> > create mode 100644 drivers/acpi/pfru/pfru_update.c
> > create mode 100644 include/uapi/linux/pfru.h
> >
> > diff --git a/Documentation/ABI/testing/pfru b/Documentation/ABI/testing/pfru
> > new file mode 100644
> > index 000000000000..b8bc81703f46
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/pfru
> > @@ -0,0 +1,41 @@
> > +What: /dev/acpi_pfru
> > +Date: October 2021
> > +KernelVersion: 5.15
> > +Contact: Chen Yu <yu.c.chen@...el.com>
> > +Description:
> > + The ioctl interface to drivers for platform firmware runtime
> > + update(PFRU). Following actions are supported:
> > +
> > + * PFRU_IOC_QUERY_CAP: Read the PFRU Runtime Update capability.
> > + The value is a structure of pfru_update_cap_info.
> > + See include/uapi/linux/pfru.h for definition.
> > +
> > + * PFRU_SET_REV: Set the Revision ID for PFRU Runtime Update.
> > + It could be either 1 or 2.
> > +
> > + * PFRU_IOC_STAGE: Stage a capsule image from communication
> > + buffer and perform authentication.
> > +
> > + * PFRU_IOC_ACTIVATE: Activate a previous staged capsule image.
> > +
> > + * PFRU_IOC_STAGE_ACTIVATE: Perform both stage and activation
> > + actions.
> > +
> > + * PFRU_LOG_IOC_SET_INFO: set log information in Telemetry
> > + Service. The input is a structure of pfru_log_info.
> > + This structure includes log revision id(1 or 2),
> > + log level(0 : Error Message, 1 : Warning Message,
> > + 2 : Informational Message, 4 : Verbose), log data type
> > + (0 : Execution Log, 1 : History Information).
> > + See include/uapi/linux/pfru.h for definition.
> > +
> > + * PFRU_LOG_IOC_GET_INFO: get log information in Telemetry.
> > + The output is a structure of pfru_log_info.
> > +
> > + * PFRU_LOG_IOC_GET_DATA_INFO: get log data information in
> > + Telemetry. The output is a structure of pfru_log_data_info.
> > + See include/uapi/linux/pfru.h for definition.
> > +
> > + Besides ioctl interface, write() and read() are supported on
> > + /dev/acpi_pfru. The write() will be the code injection/update,
> > + and the read() will be telemetry retrieval.
>
> Do we normally describe ioctl interfaces in Documentation/ABI/? Why not
> just add this to the kernel doc with the structures you are creating?
> Wouldn't that be easier?
It would work I suppose.
> Or are other acpi ioctl interfaces documented here already?
No, they aren't, but there is Documentation/ABI/testing/gpio-cdev, for
example, so there is some practice there. Whether it is good or bad
is a separate topic though.
Powered by blists - more mailing lists