Re: [RFC PATCH] cxl/pci: Set default timeout for background operations

Jonathan Cameron
2024-02-15T12:34:14Z

On Wed, 14 Feb 2024 19:31:23 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> sthanneeru.opensrc@ wrote:
> > From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
> >
> > The CXL 3.0 specification outlines background operations,
> > and support for handling these operations was added in following patch.
> >
> > Link: https://lore.kernel.org/all/20230523170927.20685-5-dave@stgolabs.net/
> >
> > Mailbox commands like ‘Log Populate’ use background operations
> > to complete the execution of the command.
> > This can lead to a timeout, since there is currently no option
> > in the ioctl cxl_send_command structure to specify
> > a timeout value. The default values being zero can lead
> > to the driver reporting false negatives to the application.
> >
> > This patch aims to establish default values, enabling mailbox commands
> > that operate in the background to continue functioning even
> > if a timeout is not set in the userspace application.
>
> The reason there are no defaults is because userspace is not allowed to
> issue background commands. The CXL background command definition is
> awkward in that it allows a single command to monopolize the mailbox for
> an indefinite amount of time.
>
> Instead, the approach taken with Firmware Update and Sanitize is that a
> kernel sysfs ABI mediates access to the mailbox and facilitates bounded
> timeslices between command submissions. It effectively allows the kernel
> to manage fairness and more importantly preempt userspace if it needs to
> issue its own commands.
>
> I assume you are only seeing this lack of a default due to building with
> CONFIG_CXL_MEM_RAW_COMMANDS=y? If yes, "raw" means "raw" and the kernel
> is mostly taken out of the loop of saving userspace from itself.
>
> All that said, ugh, "Log Populate" has no facility to time bound the
> population of the log. I do not think it is tenable for Linux to
> surrender mailbox access for an indefinite uninterruptible amount of
> time... unless you want to handle "Log Populate" like Sanitize where the
> unbounded background operation is tolerated because the device is taken
> offline?

It may be pointless to do a component state dump only on an offline device.
My assumption is this one is hardware debug only. Patches out of
tree or behind a really scary sounding config variable perhaps?
Other than vendor log I don't think populate log applies to the other
log types yet (they don't mention it anyway!)

Jonathan