Hi Ira, Thanks for reviewing this patch.
Ira Weiny <ira.we...@intel.com> writes: > On Thu, Jan 13, 2022 at 05:32:52PM +0530, Vaibhav Jain wrote: > [snip] > >> >> +/* Inject a smart error Add the dirty-shutdown-counter value to the pdsm */ >> +static int papr_pdsm_smart_inject(struct papr_scm_priv *p, >> + union nd_pdsm_payload *payload) >> +{ >> + int rc; >> + u32 supported_flags = 0; >> + u64 mask = 0, override = 0; >> + >> + /* Check for individual smart error flags and update mask and override >> */ >> + if (payload->smart_inject.flags & PDSM_SMART_INJECT_HEALTH_FATAL) { >> + supported_flags |= PDSM_SMART_INJECT_HEALTH_FATAL; >> + mask |= PAPR_PMEM_HEALTH_FATAL; >> + override |= payload->smart_inject.fatal_enable ? >> + PAPR_PMEM_HEALTH_FATAL : 0; >> + } >> + >> + if (payload->smart_inject.flags & PDSM_SMART_INJECT_BAD_SHUTDOWN) { >> + supported_flags |= PDSM_SMART_INJECT_BAD_SHUTDOWN; >> + mask |= PAPR_PMEM_SHUTDOWN_DIRTY; >> + override |= payload->smart_inject.unsafe_shutdown_enable ? >> + PAPR_PMEM_SHUTDOWN_DIRTY : 0; >> + } >> + > > I'm struggling to see why there is a need for both a flag and an 8 bit > 'enable' > value? > This is to enable the inject/uninject error usecase with ndctl which lets user select individual error conditions like bad_shutdown or fatal-health state. The nd_papr_pdsm_smart_inject.flag field indicates which error conditions needs to be tweaked and individual __u8 fields like 'fatal_enable' are boolean values to indicate the inject/uninject state of that error condition. For e.g to uninject fatal-health and inject unsafe-shutdown following nd_papr_pdsm_smart_inject payload can be sent: { .flags = PDSM_SMART_INJECT_HEALTH_FATAL | PDSM_SMART_INJECT_BAD_SHUTDOWN, .fatal_enable = 0, .unsafe_shutdown_enable = 1, } To just to inject fatal-health following nd_papr_pdsm_smart_inject payload can be sent: { .flags = PDSM_SMART_INJECT_HEALTH_FATAL, .fatal_enable = 1, .unsafe_shutdown_enable = <dont-care>, } > Ira > -- Cheers ~ Vaibhav