8:47 p.m.
On Wed, Mar 16, 2016 at 1:21 PM, Jerry Hoemann <jerry.hoemann(a)hpe.com> wrote:
On Tue, Mar 15, 2016 at 03:08:14PM -0700, Dan Williams wrote:
[..]
The above touches upon multiple things, and I'm not sure I
understand
all your comments.
1. nvdimm_bus_descriptor.dsm_mask has been around for quite a while.
I think you simply want me to rename this to cmd_mask and leave
it in nvdimm_bus_descriptor.
however, it is still a mask of acceptable dsm functions. we test
the pass thru nd_command against it.
2. nvdimm_bus_descriptor.call_dsm:1;
This was introduced in:
https://lists.01.org/pipermail/linux-nvdimm/2016-January/004052.html
This flag didn't really change the system behavior. So, I don't
think the sense of the tests on call_dsm in core.c/commands_show() and
dimm_devs.c/commands_show() is what you intended.
In the respective commands_show function, I think we only want to
call nvdimm_bus_cmd_name or nvdimm_cmd_name if and only if the underlying
DSM matches the intel example dsm. (i.e. nvdimm_bus_cmd_name and nvdimm_cmd_name
describes function names from the intel example dsm, not hpe's example.)
For root commands, there is currently no divergence, so we could leave
it as proposed, or drop it as you suggest above.
For the non-root commands, if we're using the intel dsm, we want to call
nvdimm_cmd_name. But if not, then the only command available is the pass
thru "call_dsm".
This is where I think we diverge. There's nothing stopping the kernel
from translating a
ND_IOCTL_SET_CONFIG_DATA into an "Intel - Set Namespace Label Data" or
"HP - NVDIMM-N Set OS data". In all cases the kernel will want to
intercept that command whether it comes in as the generic
ND_IOCTL_SET_CONFIG_DATA or via the ND_IOCTL_CALL interface.
So, I'll move a flag to the nvdimm struct and set it only if
we're
using NFIT_DEV_DIMM, not for NFIT_DEV_DIMM_N_HPE[1|2].
NFIT_DEV_DIMM_N_HPE* can support more than just ND_IOCTL_CALL. Having
the kernel do ND_IOCTL_<GENERIC> conversion to the family specific
command allows old ndctl binaries to run against new kernels.
3. sysfs extensions
I had planned to do some type of extension for sysfs for passthru
after getting basic features agreed upon. But, i hadn't thought
it through deeply. As for:
/sys/bus/nd/devices/nmem0/nfit/dsm_mask
/sys/bus/nd/devices/nmem0/nfit/family
Don't we want:
/sys/bus/nd/devices/nmem0/dsm_mask
/sys/bus/nd/devices/nmem0/family
As the mask/family is associated with the dimm.
Hmm, I think more like this:
/sys/bus/nd/devices/nmem0/nfit/dsm_mask
/sys/bus/nd/devices/nmem0/family
The mask is associated with the dimm, but it is ACPI specific.
However, you're right that the family id is truly generic and should
be at the dimm level because the family type could span across
non-ACPI architectures.
Do you consider this sysfs change as a requirement for accepting
the
rest of the patch set?
You can defer it to me. I need the family information for ndctl to
start converting from the old-style to the new-style ioctl interface.