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

[Michal Suchánek]

On Tue, Nov 26, 2024 at 12:40:20PM -0800, Haren Myneni wrote:
> On Wed, 2024-11-27 at 00:42 +0530, Mahesh J Salgaonkar wrote:
> > 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 ?
> 
> No, the RTAS call from the last read() will invalidate the dump. Expect
> the user space make the read() until returns 0 which means the last
> read() will return 0 after invalidate the dump. 
> 
> size_t read() 
> {
>    if (params->status == RTAS_IBM_PLATFORM_DUMP_COMPLETE) {
>            RTAS call to invalidate dump
>            return 0;
>    }
>    get the data from RTAS call
>    If the RTAS call return status is DUMP_COMPLETE
>              params->status = RTAS_IBM_PLATFORM_DUMP_COMPLETE
>    return bytes_written
> }
> 
> If the RTAS call returns DUMP_COMPLETE, the hypervisor expects one more
> RTAS call to invalidate the dump which is done as part of the last
> read()
> 
> > 
> > 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.
> Following the same read() ABI - expects to make calls until returns
> size 0.
>
> The current usage of rtas_platform_dump() in ppc64-
> diag/rtas_errd/extract_platdump.c
> 
> dump_complete = rtas_platform_dump(dump_tag, 0, dump_buf, DUMP_BUF_SZ,
>                                         &seq_next, &bytes);
> 
> while (dump_complete) {
> 
>    dump_complete = rtas_platform_dump(dump_tag, seq, dump_buf,
>  				DUMP_BUF_SZ, &seq_next, &bytes);
>                                 
> }
> 
> ret = rtas_platform_dump(dump_tag, seq, NULL, 0, 
>                                 &seq_next, &bytes);
> 
> we need to support both new and old interfaces and not changing the
> above code which uses librtas API.
> 
> So the new rtas_platform_dump() interface
> {
>  if the buffer == NULL 
>      return - nothing to do here. Dump is invalidated with the previous
> rtas_platform_dump()   
>  
>  size = read()
>  if size == 0 
>       dump complete and invalidate the dump
>       return 0
> 
>   return 1;
> }

No EOF?

So no standard file handling code can use this FD?

But also the size 0 read both indicates the EOF and invalidates the
dump, these should be separate.

Which differs from the hypervisor API that makes it impossible to save
the dump without deleting it, and introduces a regression.

If you are doing IOCTLs anyway the invalidation could be an IOCTL. Or
you could really follow the RTAS ABI and only incalidate if the user
passes NULL and 0.

But more generally the previously added RTAS wrappers do not closely
follow the RTAS ABI, and do accumulation of read data in the kernel,
passing the whole buffer to userspace afterwards.

Why cannot it be done in this case?

Even more generally if the dump IDs are stable these could be listed in
some sysfs or debugfs directory, and provide standard file operations,
including unlink() to remove the dump.

With the bonus that at least one of these filesystems has some
provisions for confidentiality lockdown. The implementation could use
that to make the dumps unavailable in confidentiality lockdown level if
the dumps are considered confidential without reimplementing the check.

Thanks

Michal