linux-kernel - Re: [PATCH v12 2/4] drivers/acpi: Introduce Platform Firmware Runtime Update device driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 20 Dec 2021 17:52:40 +0800
From:   Chen Yu <yu.c.chen@...el.com>
To:     linux-acpi@...r.kernel.org
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Ard Biesheuvel <ardb@...nel.org>, Len Brown <lenb@...nel.org>,
        Ashok Raj <ashok.raj@...el.com>,
        Andy Shevchenko <andriy.shevchenko@...el.com>,
        Mike Rapoport <rppt@...nel.org>, linux-kernel@...r.kernel.org,
        Hongyu Ning <hongyu.ning@...el.com>
Subject: Re: [PATCH v12 2/4] drivers/acpi: Introduce Platform Firmware
 Runtime Update device driver

On Fri, Dec 17, 2021 at 12:03:02AM +0800, Chen Yu wrote:
> Introduce the pfr_update driver which can be used for Platform Firmware
> Runtime code injection and driver update [1]. The user is expected to
> provide the EFI capsule, and pass it to the driver by writing the capsule
> to a device special file. The capsule is transferred by the driver to the
> platform firmware with the help of an ACPI _DSM method under the special
> ACPI Platform Firmware Runtime Update device (INTC1080), and the actual
> firmware update is carried out by the low-level Management Mode code in
> the platform firmware.
> 
> This patch allows certain pieces of the platform firmware to be
> updated on the fly while the system is running (runtime) without the
> need to restart it, which is key in the cases when the system needs to
> be available 100% of the time and it cannot afford the downtime related
> to restarting it, or when the work carried out by the system is
> particularly important, so it cannot be interrupted, and it is not
> practical to wait until it is complete.
> 
> Link: https://uefi.org/sites/default/files/resources/Intel_MM_OS_Interface_Spec_Rev100.pdf # [1]
> Cc: Andy Shevchenko <andriy.shevchenko@...el.com>
> Cc: Ard Biesheuvel <ardb@...nel.org>
> Cc: Ashok Raj <ashok.raj@...el.com>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: Len Brown <lenb@...nel.org>
> Cc: Mike Rapoport <rppt@...nel.org>
> Cc: "Rafael J. Wysocki" <rafael@...nel.org>
> Signed-off-by: Chen Yu <yu.c.chen@...el.com>
> +
[cut]
> +static int start_update(int action, struct pfru_device *pfru_dev)
> +{
> +	union acpi_object *out_obj, in_obj, in_buf;
> +	struct pfru_updated_result update_result;
> +	acpi_handle handle;
> +	int ret = -EINVAL;
> +
> +	memset(&in_obj, 0, sizeof(in_obj));
> +	memset(&in_buf, 0, sizeof(in_buf));
> +	in_obj.type = ACPI_TYPE_PACKAGE;
> +	in_obj.package.count = 1;
> +	in_obj.package.elements = &in_buf;
> +	in_buf.type = ACPI_TYPE_INTEGER;
> +	in_buf.integer.value = action;
> +
> +	handle = ACPI_HANDLE(pfru_dev->parent_dev);
> +	out_obj = acpi_evaluate_dsm_typed(handle, &pfru_guid,
> +					  pfru_dev->rev_id, PFRU_FUNC_START,
> +					  &in_obj, ACPI_TYPE_PACKAGE);
> +	if (!out_obj)
> +		return ret;
> +
> +	if (out_obj->package.count < UPDATE_NR_IDX ||
> +	    out_obj->package.elements[UPDATE_STATUS_IDX].type != ACPI_TYPE_INTEGER ||
> +	    out_obj->package.elements[UPDATE_EXT_STATUS_IDX].type != ACPI_TYPE_INTEGER ||
> +	    out_obj->package.elements[UPDATE_AUTH_TIME_LOW_IDX].type != ACPI_TYPE_INTEGER ||
> +	    out_obj->package.elements[UPDATE_AUTH_TIME_HI_IDX].type != ACPI_TYPE_INTEGER ||
> +	    out_obj->package.elements[UPDATE_EXEC_TIME_LOW_IDX].type != ACPI_TYPE_INTEGER ||
> +	    out_obj->package.elements[UPDATE_EXEC_TIME_HI_IDX].type != ACPI_TYPE_INTEGER)
> +		goto free_acpi_buffer;
> +
> +	update_result.status =
> +		out_obj->package.elements[UPDATE_STATUS_IDX].integer.value;
Thanks for Hongyu's testing, the status should be cascaded to user space.
Will fix it in next version.

Thanks,
Chenyu