| Message ID | 20170811000000.18672-1-jerry.hoemann@hpe.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Aug 10, 2017 at 5:00 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote: > Add structure definitions newly published/modified in v0.85: > > https://github.com/HewlettPackard/hpe-nvm/blob/master/Documentation/NFIT_DSM_DDR4_NVDIMM-N_v85.pdf Are there going to be follow-on patches that make use of these definitions? I would save this update for when those follow-on patches are ready.
On Thu, Aug 10, 2017 at 05:47:10PM -0700, Dan Williams wrote: > On Thu, Aug 10, 2017 at 5:00 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote: > > Add structure definitions newly published/modified in v0.85: > > > > https://github.com/HewlettPackard/hpe-nvm/blob/master/Documentation/NFIT_DSM_DDR4_NVDIMM-N_v85.pdf > > Are there going to be follow-on patches that make use of these > definitions? I would save this update for when those follow-on patches > are ready. There are multiple projects both inside and outside of HPE that need to make DSM calls. The DSM ioctl is an interface exported to user applications. Customers and vendors can and will use this interface for their own tools and this use need not result in patches sent back to the ndctl project.
On Thu, Aug 10, 2017 at 7:12 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote: > On Thu, Aug 10, 2017 at 05:47:10PM -0700, Dan Williams wrote: >> On Thu, Aug 10, 2017 at 5:00 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote: >> > Add structure definitions newly published/modified in v0.85: >> > >> > https://github.com/HewlettPackard/hpe-nvm/blob/master/Documentation/NFIT_DSM_DDR4_NVDIMM-N_v85.pdf >> >> Are there going to be follow-on patches that make use of these >> definitions? I would save this update for when those follow-on patches >> are ready. > > There are multiple projects both inside and outside of HPE that need to > make DSM calls. > > The DSM ioctl is an interface exported to user applications. Customers > and vendors can and will use this interface for their own tools and this > use need not result in patches sent back to the ndctl project. My hope was that the existing extra definitions in ndctl-hpe1.h would gain some users at some point in the future, but there's otherwise no need to add dead code to ndctl-hpe1.h. That header is only used at compile time it's not shipped so it doesn't provide any benefit to those third party applications that you mention.
On Thu, Aug 10, 2017 at 07:27:58PM -0700, Dan Williams wrote: > On Thu, Aug 10, 2017 at 7:12 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote: > > On Thu, Aug 10, 2017 at 05:47:10PM -0700, Dan Williams wrote: > >> On Thu, Aug 10, 2017 at 5:00 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote: > >> > Add structure definitions newly published/modified in v0.85: > >> > > >> > https://github.com/HewlettPackard/hpe-nvm/blob/master/Documentation/NFIT_DSM_DDR4_NVDIMM-N_v85.pdf > >> > >> Are there going to be follow-on patches that make use of these > >> definitions? I would save this update for when those follow-on patches > >> are ready. > > > > There are multiple projects both inside and outside of HPE that need to > > make DSM calls. > > > > The DSM ioctl is an interface exported to user applications. Customers > > and vendors can and will use this interface for their own tools and this > > use need not result in patches sent back to the ndctl project. > > My hope was that the existing extra definitions in ndctl-hpe1.h would > gain some users at some point in the future, but there's otherwise no > need to add dead code to ndctl-hpe1.h. That header is only used at > compile time it's not shipped so it doesn't provide any benefit to > those third party applications that you mention. AFAIK ndctl-hpe1.h is the only "upstream" place where the definition of the HPE1 non-root function structures reside. Having the definitions in the kernel supplied header ndctl.h would be much better, but you pushed back on that two years ago. Have you changed your mind? Do you want me to add the HPE nvdimm-n non root structure definitions to ndctl.h like the Intel non-root definitions?
On Thu, Aug 10, 2017 at 8:41 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote: > On Thu, Aug 10, 2017 at 07:27:58PM -0700, Dan Williams wrote: >> On Thu, Aug 10, 2017 at 7:12 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote: >> > On Thu, Aug 10, 2017 at 05:47:10PM -0700, Dan Williams wrote: >> >> On Thu, Aug 10, 2017 at 5:00 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote: >> >> > Add structure definitions newly published/modified in v0.85: >> >> > >> >> > https://github.com/HewlettPackard/hpe-nvm/blob/master/Documentation/NFIT_DSM_DDR4_NVDIMM-N_v85.pdf >> >> >> >> Are there going to be follow-on patches that make use of these >> >> definitions? I would save this update for when those follow-on patches >> >> are ready. >> > >> > There are multiple projects both inside and outside of HPE that need to >> > make DSM calls. >> > >> > The DSM ioctl is an interface exported to user applications. Customers >> > and vendors can and will use this interface for their own tools and this >> > use need not result in patches sent back to the ndctl project. >> >> My hope was that the existing extra definitions in ndctl-hpe1.h would >> gain some users at some point in the future, but there's otherwise no >> need to add dead code to ndctl-hpe1.h. That header is only used at >> compile time it's not shipped so it doesn't provide any benefit to >> those third party applications that you mention. > > AFAIK ndctl-hpe1.h is the only "upstream" place where the definition of > the HPE1 non-root function structures reside. For a third-party utility I don't see how ndctl is its upstream. > > Having the definitions in the kernel supplied header ndctl.h would be > much better, but you pushed back on that two years ago. > > Have you changed your mind? Do you want me to add the HPE nvdimm-n > non root structure definitions to ndctl.h like the Intel non-root > definitions? No, they don't do the kernel any good either. ND_CMD_CALL is meant to be the last of the ioctl payload definitions we need to add to ndctl.h. For these to be useful to the ndctl utilty they need libndctl.h wrappers similar to what we've done for handling SMART payloads across vendors. I.e. hide the vendor-specific aspect and make a generic interface. For example, these commands seem to have several similarities with the Microsoft format. I'm planning to do the same with new additions to the Intel DSM command set.
On 8/10/2017 10:27 PM, Dan Williams wrote: > On Thu, Aug 10, 2017 at 7:12 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote: >> On Thu, Aug 10, 2017 at 05:47:10PM -0700, Dan Williams wrote: >>> On Thu, Aug 10, 2017 at 5:00 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote: >>>> Add structure definitions newly published/modified in v0.85: >>>> >>>> https://github.com/HewlettPackard/hpe-nvm/blob/master/Documentation/NFIT_DSM_DDR4_NVDIMM-N_v85.pdf >>> >>> Are there going to be follow-on patches that make use of these >>> definitions? I would save this update for when those follow-on patches >>> are ready. >> >> There are multiple projects both inside and outside of HPE that need to >> make DSM calls. >> >> The DSM ioctl is an interface exported to user applications. Customers >> and vendors can and will use this interface for their own tools and this >> use need not result in patches sent back to the ndctl project. > > My hope was that the existing extra definitions in ndctl-hpe1.h would > gain some users at some point in the future, It hasn't been easy to add things to ndctl but we would like to add more. Otherwise we'll have to create yet another tool. > but there's otherwise no > need to add dead code to ndctl-hpe1.h. That header is only used at > compile time it's not shipped so it doesn't provide any benefit to > those third party applications that you mention. We've thought about shipping this include file. I think there's a library or developer package for ndctl? -- ljk > _______________________________________________ > Linux-nvdimm mailing list > Linux-nvdimm@lists.01.org > https://lists.01.org/mailman/listinfo/linux-nvdimm >
On Fri, Aug 11, 2017 at 6:36 AM, Linda Knippers <linda.knippers@hpe.com> wrote: > > > On 8/10/2017 10:27 PM, Dan Williams wrote: >> >> On Thu, Aug 10, 2017 at 7:12 PM, Jerry Hoemann <jerry.hoemann@hpe.com> >> wrote: >>> >>> On Thu, Aug 10, 2017 at 05:47:10PM -0700, Dan Williams wrote: >>>> >>>> On Thu, Aug 10, 2017 at 5:00 PM, Jerry Hoemann <jerry.hoemann@hpe.com> >>>> wrote: >>>>> >>>>> Add structure definitions newly published/modified in v0.85: >>>>> >>>>> >>>>> https://github.com/HewlettPackard/hpe-nvm/blob/master/Documentation/NFIT_DSM_DDR4_NVDIMM-N_v85.pdf >>>> >>>> >>>> Are there going to be follow-on patches that make use of these >>>> definitions? I would save this update for when those follow-on patches >>>> are ready. >>> >>> >>> There are multiple projects both inside and outside of HPE that need to >>> make DSM calls. >>> >>> The DSM ioctl is an interface exported to user applications. Customers >>> and vendors can and will use this interface for their own tools and this >>> use need not result in patches sent back to the ndctl project. >> >> >> My hope was that the existing extra definitions in ndctl-hpe1.h would >> gain some users at some point in the future, > > > It hasn't been easy to add things to ndctl but we would like to add more. > Otherwise we'll have to create yet another tool. Understood. The goal I've had with ndctl is to be vendor agnostic out of the gate. The trajectory of vendor tools is that each vendor releases their own special sauce and sometime later someone who is tired of dealing with that differentiation writes a library to undo it, like system-storage-manager or smartmontools, and provide a unified interface. If you need to do truly vendor specific things that probably needs a new tool, but if there's any commonality for a feature across vendors then I think we should look to add a common ndctl front-end for it. For example, I'm looking to add a firmware update mechanism that provides a framework to hide all the "if (vendor == X)" details behind the libndctl api. >> but there's otherwise no >> need to add dead code to ndctl-hpe1.h. That header is only used at >> compile time it's not shipped so it doesn't provide any benefit to >> those third party applications that you mention. > > > We've thought about shipping this include file. I think there's > a library or developer package for ndctl? It does have a devel package for users to write against the libndctl.h api. There's also general if 'platform == NFIT' vs 'platform == e820' differentiation. What it doesn't have is any api that externally presents a vendor specific interface.
On 8/11/2017 12:48 PM, Dan Williams wrote: > On Fri, Aug 11, 2017 at 6:36 AM, Linda Knippers <linda.knippers@hpe.com> wrote: >> >> >> On 8/10/2017 10:27 PM, Dan Williams wrote: >>> >>> On Thu, Aug 10, 2017 at 7:12 PM, Jerry Hoemann <jerry.hoemann@hpe.com> >>> wrote: >>>> >>>> On Thu, Aug 10, 2017 at 05:47:10PM -0700, Dan Williams wrote: >>>>> >>>>> On Thu, Aug 10, 2017 at 5:00 PM, Jerry Hoemann <jerry.hoemann@hpe.com> >>>>> wrote: >>>>>> >>>>>> Add structure definitions newly published/modified in v0.85: >>>>>> >>>>>> >>>>>> https://github.com/HewlettPackard/hpe-nvm/blob/master/Documentation/NFIT_DSM_DDR4_NVDIMM-N_v85.pdf >>>>> >>>>> >>>>> Are there going to be follow-on patches that make use of these >>>>> definitions? I would save this update for when those follow-on patches >>>>> are ready. >>>> >>>> >>>> There are multiple projects both inside and outside of HPE that need to >>>> make DSM calls. >>>> >>>> The DSM ioctl is an interface exported to user applications. Customers >>>> and vendors can and will use this interface for their own tools and this >>>> use need not result in patches sent back to the ndctl project. >>> >>> >>> My hope was that the existing extra definitions in ndctl-hpe1.h would >>> gain some users at some point in the future, >> >> >> It hasn't been easy to add things to ndctl but we would like to add more. >> Otherwise we'll have to create yet another tool. > > Understood. The goal I've had with ndctl is to be vendor agnostic out > of the gate. The trajectory of vendor tools is that each vendor > releases their own special sauce and sometime later someone who is > tired of dealing with that differentiation writes a library to undo > it, like system-storage-manager or smartmontools, and provide a > unified interface. If you need to do truly vendor specific things that > probably needs a new tool, but if there's any commonality for a > feature across vendors then I think we should look to add a common > ndctl front-end for it. For example, I'm looking to add a firmware > update mechanism that provides a framework to hide all the "if (vendor > == X)" details behind the libndctl api. > >>> but there's otherwise no >>> need to add dead code to ndctl-hpe1.h. That header is only used at >>> compile time it's not shipped so it doesn't provide any benefit to >>> those third party applications that you mention. >> >> >> We've thought about shipping this include file. I think there's >> a library or developer package for ndctl? > > It does have a devel package for users to write against the libndctl.h > api. There's also general if 'platform == NFIT' vs 'platform == e820' > differentiation. What it doesn't have is any api that externally > presents a vendor specific interface. I don't think we actually need a vendor-specific interface. We just need an easy/common way to pass through the commands. That's why it seemed natural to add the new DSM structure definitions to the ones that are already there.
On Fri, Aug 11, 2017 at 10:58 AM, Linda Knippers <linda.knippers@hpe.com> wrote: > > On 8/11/2017 12:48 PM, Dan Williams wrote: >> >> On Fri, Aug 11, 2017 at 6:36 AM, Linda Knippers <linda.knippers@hpe.com> >> wrote: >>> >>> >>> >>> On 8/10/2017 10:27 PM, Dan Williams wrote: >>>> >>>> >>>> On Thu, Aug 10, 2017 at 7:12 PM, Jerry Hoemann <jerry.hoemann@hpe.com> >>>> wrote: >>>>> >>>>> >>>>> On Thu, Aug 10, 2017 at 05:47:10PM -0700, Dan Williams wrote: >>>>>> >>>>>> >>>>>> On Thu, Aug 10, 2017 at 5:00 PM, Jerry Hoemann <jerry.hoemann@hpe.com> >>>>>> wrote: >>>>>>> >>>>>>> >>>>>>> Add structure definitions newly published/modified in v0.85: >>>>>>> >>>>>>> >>>>>>> >>>>>>> https://github.com/HewlettPackard/hpe-nvm/blob/master/Documentation/NFIT_DSM_DDR4_NVDIMM-N_v85.pdf >>>>>> >>>>>> >>>>>> >>>>>> Are there going to be follow-on patches that make use of these >>>>>> definitions? I would save this update for when those follow-on patches >>>>>> are ready. >>>>> >>>>> >>>>> >>>>> There are multiple projects both inside and outside of HPE that need to >>>>> make DSM calls. >>>>> >>>>> The DSM ioctl is an interface exported to user applications. Customers >>>>> and vendors can and will use this interface for their own tools and >>>>> this >>>>> use need not result in patches sent back to the ndctl project. >>>> >>>> >>>> >>>> My hope was that the existing extra definitions in ndctl-hpe1.h would >>>> gain some users at some point in the future, >>> >>> >>> >>> It hasn't been easy to add things to ndctl but we would like to add more. >>> Otherwise we'll have to create yet another tool. >> >> >> Understood. The goal I've had with ndctl is to be vendor agnostic out >> of the gate. The trajectory of vendor tools is that each vendor >> releases their own special sauce and sometime later someone who is >> tired of dealing with that differentiation writes a library to undo >> it, like system-storage-manager or smartmontools, and provide a >> unified interface. If you need to do truly vendor specific things that >> probably needs a new tool, but if there's any commonality for a >> feature across vendors then I think we should look to add a common >> ndctl front-end for it. For example, I'm looking to add a firmware >> update mechanism that provides a framework to hide all the "if (vendor >> == X)" details behind the libndctl api. >> >>>> but there's otherwise no >>>> need to add dead code to ndctl-hpe1.h. That header is only used at >>>> compile time it's not shipped so it doesn't provide any benefit to >>>> those third party applications that you mention. >>> >>> >>> >>> We've thought about shipping this include file. I think there's >>> a library or developer package for ndctl? >> >> >> It does have a devel package for users to write against the libndctl.h >> api. There's also general if 'platform == NFIT' vs 'platform == e820' >> differentiation. What it doesn't have is any api that externally >> presents a vendor specific interface. > > > I don't think we actually need a vendor-specific interface. We just > need an easy/common way to pass through the commands. That's why it > seemed natural to add the new DSM structure definitions to the ones > that are already there. That sounds fine to me, just need ndctl_dimm_cmd_new_* wrappers in front of them with an ops structure to redirect to the vendor specific implementation. Just like we have for ndctl_dimm_cmd_new_smart().
diff --git a/ndctl/lib/ndctl-hpe1.h b/ndctl/lib/ndctl-hpe1.h index b050831..ba071fd 100644 --- a/ndctl/lib/ndctl-hpe1.h +++ b/ndctl/lib/ndctl-hpe1.h @@ -30,6 +30,10 @@ enum { NDN_HPE1_CMD_ERRINJ_QUERY = 18, NDN_HPE1_CMD_ERRINJ_INJECT = 19, NDN_HPE1_CMD_ERRINJ_STATUS = 20, + NDN_HPE1_CMD_I2C_READ = 21, + NDN_HPE1_CMD_I2C_WRITE = 22, + NDN_HPE1_CMD_I2C_BLK_READ = 23, + NDN_HPE1_CMD_I2C_BLK_WRITE = 24, }; /* NDN_HPE1_CMD_SMART */ @@ -103,7 +107,11 @@ struct ndn_hpe1_smart_data { __u8 res2; __u16 energy_src_total_runtime; __u16 vndr_spec_data_size; - __u8 vnd_spec_data[60]; + __u8 vnd_spec_data[28]; + __u64 failed_save_cnt; + __u64 failed_restore_cnt; + __u64 failed_arm_cnt; + __u64 failed_flush_cnt; } __attribute__((packed)); struct ndn_hpe1_smart { @@ -312,6 +320,42 @@ struct ndn_hpe1_get_inj_err_status { __u8 err_inj_opt; } __attribute__((packed)); +/* NDN_HPE1_CMD_I2C_READ */ +struct ndn_hpe1_i2c_read { + __u8 in_page; + __u8 in_offset; + __u32 status; + __u8 reg_value; +} __attribute__((packed)); + +/* NDN_HPE1_CMD_I2C_WRITE */ +struct ndn_hpe1_i2c_write { + __u8 in_page; + __u8 in_offset; + __u8 in_value; + __u32 status; +} __attribute__((packed)); + + +/* NDN_HPE1_CMD_I2C_BLK_READ */ +struct ndn_hpe1_i2c_blk_read { + __u8 in_type; + __u16 in_reg_id; + __u8 in_blk_id; + __u32 status; + __u8 data[32]; +} __attribute__((packed)); + +/* NDN_HPE1_CMD_I2C_BLK_WRITE */ +struct ndn_hpe1_i2c_blk_write { + __u8 in_type; + __u16 in_reg_id; + __u8 in_blk_id; + __u8 in_data[32]; + __u32 status; +} __attribute__((packed)); + + union ndn_hpe1_cmd { __u64 query; struct ndn_hpe1_smart smart; @@ -326,6 +370,10 @@ union ndn_hpe1_cmd { struct ndn_hpe1_query_err_inj_cap err_cap; struct ndn_hpe1_inj_err inj_err; struct ndn_hpe1_get_inj_err_status inj_err_stat; + struct ndn_hpe1_i2c_read i2c_read; + struct ndn_hpe1_i2c_write i2c_write; + struct ndn_hpe1_i2c_blk_read i2c_blk_read; + struct ndn_hpe1_i2c_blk_write i2c_blk_write; unsigned char buf[128]; };
Add structure definitions newly published/modified in v0.85: https://github.com/HewlettPackard/hpe-nvm/blob/master/Documentation/NFIT_DSM_DDR4_NVDIMM-N_v85.pdf Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com> --- ndctl/lib/ndctl-hpe1.h | 50 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-)