On Fri, Jan 30, 2026 at 03:45:09PM -0600, Cheatham, Benjamin wrote:
> On 1/30/2026 2:58 PM, Verma, Vishal L wrote:
> > On Fri, 2026-01-30 at 13:59 -0600, Cheatham, Benjamin wrote:
> >>>
> >>> It feels to me like the two injection 'modes' should really be two
> >>> separate commands, especially since they act on different classes of
> >>> targets.
> >>>
> >>> So essentially, split both the injection and clear commands into:
> >>>
> >>> inject-protocol-error
> >>> inject-media-error
> >>> clear-protocol-error
> >>> clear-media-error.
> >>
> >> This seems reasonable but I should clarify it would only be 3 commands,
> >> clear-protocol-error wouldn't be a thing since there's only an injection
> >> action for protocol errors.
> >
> > Ah I see, makes sense on 3 commands. I assume the clear command would
> > still be clear-media-error for symmetry.
> >
> >>
> >> Should I keep this all in one file or split into two separate files
> >> on protocol/media error lines? Could also do inject/clear files if that
> >> seems more logical.
> >
> > For the code - a single inject.c file probably seems fine. There's
> > precedent of implementing multiple related commands in one file, and it
> > makes sense to me here.
>
> Ok, sounds good.
>
> >
> >>>
> >>> That way the target operands for them are well defined - i.e. port
> >>> objects for protocol errors and memdevs for media errors.
> >>>
> >>>
> >>> Another thing - and I'm not too attached to either way for this -
> >>>
> >>> The -t 'long-string' feels a bit awkward. Could it be split into
> >>> something like:
> >>>
> >>> --target={mem,cache} --type={correctable,uncorrectable,fatal}
> >>>
> >>> And then 'compose' the actual thing being injected from those options?
> >>> Or is that unnecessary gymnastics?
> >>>
> >>
> >> No, I like that idea. I do think the argument names could be better though.
> >> What about:
> >>
> >> # inject-protocol-error <port> --protocol={mem,cache}
> >> --severity={correctable,uncorrectable,fatal}
> >>
> >> with the short flags for --protocol and --severity being -p and -s,
> >> respectively?
> >
> > Yes, I like those better!
> >
> >>
> >> For inject/clear-media-error it could stay as-is, i.e.:
> >>
> >> # inject-media-error <memdev> -t={poison} -a=<device physical address>
> >>
> >> or I could update it to be something like:
> >>
> >> # inject-media-error <memdev> --poison -a=<device physical address>
> >
> > Either of those seems reasonable to me. What's the possibility of a
> > /lot/ of types getting added? Probably small? If so, maybe --poison is
> > cleanest, no string parsing needed.
>
> I'm not sure, but I doubt it's many. I'll look into it and if it's less than
> ~5 or
> so I'll change it to --poison.
I too like separating the commands. Before we wrap this one up,
here's another flavor to consider:
inject-media-poison
clear-media-poison
There is not another type, nor a placeholder for another type in CXL
spec. The media-errors are poison only. The only variability is in
the error source.
If we do 'inject-media-poison' today, and then another inject type comes
along, we can always do 'inject-media-blah'
Maybe I'm missing where the other -t choice might appear?
-- Alison
>
> Thanks,
> Ben
