[PATCH] powerpc/pseries: Add papr-platform-dump character driver for dump retrieval

[Mahesh J Salgaonkar]

On 2024-11-23 21:20:39 Sat, Haren Myneni wrote:
[...]
> +static ssize_t papr_platform_dump_handle_read(struct file *file,
> +		char __user *buf, size_t size, loff_t *off)
> +{
> +	struct ibm_platform_dump_params *params = file->private_data;
> +	u64 total_bytes;
> +	s32 fwrc;
> +
> +	/*
> +	 * Dump already completed with the previous read calls.
> +	 * In case if the user space issues further reads, returns
> +	 * -EINVAL.
> +	 */
> +	if (!params->buf_length) {
> +		pr_warn_once("Platform dump completed for dump ID %llu\n",
> +			(u64) (((u64)params->dump_tag_hi << 32) |
> +				params->dump_tag_lo));
> +		return -EINVAL;
> +	}
> +
> +	if (size < SZ_1K) {
> +		pr_err_once("Buffer length should be minimum 1024 bytes\n");
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * The hypervisor returns status 0 if no more data available to
> +	 * download. Then expects the last RTAS call with NULL buffer
> +	 * to invalidate the dump which means dump will be freed in the
> +	 * hypervisor.
> +	 */
> +	if (params->status == RTAS_IBM_PLATFORM_DUMP_COMPLETE) {
> +		params->buf_length = 0;
> +		fwrc = rtas_ibm_platform_dump(params, 0, 0);
> +		/*
> +		 * Returns 0 (success) to the user space so that user
> +		 * space read stops.

Does this implicitly invalidates the dump irrespective of whether
userspace has requested or not ?

Copy-pasting bellow code snippet from librtas side patch posted by you to
librtas-devel mailing list:
+ /*
+ * rtas_platform_dump() is called with buf = NULL and length = 0
+ * for "dump complete" RTAS call to invalidate dump.
+ * For kernel interface, read() will be continued until the
+ * return value = 0. Means kernel API will return this value only
+ * after issued "dump complete" call. So nothing to do further
+ * for the last RTAS call.
+ */
+ if (buffer == NULL)
+ return 0;

If I understand this correctly, it looks like calling
rtas_platform_dump() with buf = NULL and length = 0, now does nothing.
Dump is already invalidated even before userspace makes this call.
Wouldn't this break ABI ? Correct me if I am wrong.

Consider a scenario where userspace tool using librtas interface
rtas_platform_dump() reads the last set of data but unable to write it
to the disk due to insufficient disk space. In that case, tool may error
out without invalidating the dump and expect user to cleanup the disk
space, re-run the tool to save platform dump to disk. If last read
implicitly invalidates the dump, then in this scenario user will loose
the platform dump. Shouldn't we wait for explicit request from user to
invalidate the dump to avoid this ?

Thanks,
-Mahesh.