Date:   Mon, 20 Dec 2021 17:52:40 +0800
From:   Chen Yu <yu.c.chen@intel.com>
To:     linux-acpi@vger.kernel.org
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Ard Biesheuvel <ardb@kernel.org>, Len Brown <lenb@kernel.org>,
        Ashok Raj <ashok.raj@intel.com>,
        Andy Shevchenko <andriy.shevchenko@intel.com>,
        Mike Rapoport <rppt@kernel.org>, linux-kernel@vger.kernel.org,
        Hongyu Ning <hongyu.ning@intel.com>
Subject: Re: [PATCH v12 2/4] drivers/acpi: Introduce Platform Firmware
 Runtime Update device driver
Message-ID: <20211220095240.GA582440@chenyu-desktop>
References: <cover.1639669829.git.yu.c.chen@intel.com>
 <11441d3b36608e0840c46eb45e6d75ffc1c020ec.1639669829.git.yu.c.chen@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <11441d3b36608e0840c46eb45e6d75ffc1c020ec.1639669829.git.yu.c.chen@intel.com>
Precedence: bulk

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@intel.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Ashok Raj <ashok.raj@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Len Brown <lenb@kernel.org>
> Cc: Mike Rapoport <rppt@kernel.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Signed-off-by: Chen Yu <yu.c.chen@intel.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