RE: [EXT] RE: [PATCH v3 0/2] Add log related mailbox commands

[ajay.opensrc <ajay.opensrc@micron.com>]

Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
> Srinivasulu Opensrc <sthanneeru.opensrc@micron.com> wrote:
>

> > >-----Original Message-----
> > >From: Dan Williams <dan.j.williams@intel.com>
> > >Sent: Wednesday, March 27, 2024 5:07 AM
> > >To: Srinivasulu Opensrc <sthanneeru.opensrc@micron.com>; linux-
> > >cxl@vger.kernel.org
> > >Cc: Jonathan.Cameron@huawei.com; dan.j.williams@intel.com;
> > >john@jagalactic.com; Eishan Mirakhur <emirakhur@micron.com>; Ajay
> > >Joshi <ajayjoshi@micron.com>; Ravis OpenSrc
> > ><Ravis.OpenSrc@micron.com>; Srinivasulu Thanneeru
> > ><sthanneeru@micron.com>
> > >Subject: [EXT] RE: [PATCH v3 0/2] Add log related mailbox commands
> > >
> > >CAUTION: EXTERNAL EMAIL. Do not click links or open attachments
> > >unless you recognize the sender and were expecting this message.
> > >
> > >
> > >sthanneeru.opensrc@ wrote:
> > >> From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
> > >>
> > >> Add support to expose following mailbox commands to userspace for
> > >> clearing and populating the Vendor debug log in certain scenarios,
> > >> allowing for the aggregation of results over time.
> > >>
> > >> 1. CXL r3.1 8.2.9.5.3 Get Log Capabilities.
> > >> 2. CXL r3.1 8.2.9.5.4 Clear Log commands.
> > >> 3. CXL r3.1 8.2.9.5.6 Get Supported Logs Sub-List.
> > >>
> > >> ---
> > >> Changes in v3:
> > >> - 'Component State Dump log' has several caveats for ioctl()
> > >>    not being a suitable ABI as pointed in v2.(Dan Williams)
> > >> - Remove Component State Dump from Clear log filter.
> > >> - Implement a seperate patch(yet to do) to address issues as pointed in
> v2.
> > >
> > >Circling back to this question... I had overlooked the fact that in
> > >v3.1 the "Request Abort Background Operation" command was added
> (8.2.9.1.5).
> > >With that the kernel can safely support background commands with
> > >indefinite residency. So as long as the device supports that command
> > >and advertises that Log populate requests can be cancelled then we
> > >can build a facility to cancel any user-submitted background commands
> > >when a kernel internal need for the background command slot arises.
> >
> > I don't have the access to test "Request Abort Background Operation."
> > Previously, we posted RFC for default time for background operations.
> >
> https://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> > .kernel.org%2Flinux-mm%2F20240207105349.301-1-
> sthanneeru.opensrc%40mic
> >
> ron.com%2F&data=05%7C02%7Cajayjoshi%40micron.com%7Cabe61f69d40
> 2460e2f2
> >
> b08dc55964860%7Cf38a5ecd28134862b11bac1d563c806f%7C0%7C0%7C
> 63847935119
> >
> 7749869%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi
> V2luMzIiLC
> >
> JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=QyxjJi2iLMTSTl
> R46rv9k5
> > U%2FDZVrKruaaunew8HMR9E%3D&reserved=0
> >
> >  Could you please guide me on how to proceed with this current patch
> series?
>
> Whilst QEMU emulation doesn't yet support background command aborting,
> it wouldn't be that hard to add and would provide a route to test this
> functionality.
> We've done similar in a few other cases where no one had any hardware yet.
>
> Of course that might not help you if you have silicon that doesn't implement it
> but I agree with Dan that it is a lot less problematic to allow for unbounded
> background ops if we can stop them for other urgent activity.
>

Thanks Jonathan and Dan. Agree with you on the background command handling.
The series which Srini (sthanneeru.opensrc) has submitted has 2 patches:
cxl/mbox: Add Get Log Capabilities and Get Supported Logs Sub-List
    commands
cxl/mbox: Add Clear Log mailbox command
The commands in these patches(opcodes: 0402h, 0403h, 0405h) don't seem to 
have any background handling as of 3.1.
Do you think these patches seem ok or do you feel something
is missing here?


Ajay

> Jonathan