Vaibhav Jain <vaib...@linux.ibm.com> writes:

....

 +
> +/* Papr-scm-header + payload expected with ND_CMD_CALL ioctl from libnvdimm 
> */
> +struct nd_pdsm_cmd_pkg {
> +     struct nd_cmd_pkg hdr;  /* Package header containing sub-cmd */
> +     __s32 cmd_status;       /* Out: Sub-cmd status returned back */
> +     __u16 payload_offset;   /* In: offset from start of struct */
> +     __u16 payload_version;  /* In/Out: version of the payload */
> +     __u8 payload[];         /* In/Out: Sub-cmd data buffer */
> +} __packed;

that payload_offset can be avoided if we prevent userspace to user a
different variant of nd_pdsm_cmd_pkg which different header. We can keep
things simpler if we can always find payload at
nd_pdsm_cmd_pkg->payload.

> +
> +/*
> + * Methods to be embedded in ND_CMD_CALL request. These are sent to the 
> kernel
> + * via 'nd_pdsm_cmd_pkg.hdr.nd_command' member of the ioctl struct
> + */
> +enum papr_scm_pdsm {
> +     PAPR_SCM_PDSM_MIN = 0x0,
> +     PAPR_SCM_PDSM_MAX,
> +};
> +
> +/* Convert a libnvdimm nd_cmd_pkg to pdsm specific pkg */
> +static inline struct nd_pdsm_cmd_pkg *nd_to_pdsm_cmd_pkg(struct nd_cmd_pkg 
> *cmd)
> +{
> +     return (struct nd_pdsm_cmd_pkg *) cmd;
> +}
> +
> +/* Return the payload pointer for a given pcmd */
> +static inline void *pdsm_cmd_to_payload(struct nd_pdsm_cmd_pkg *pcmd)
> +{
> +     if (pcmd->hdr.nd_size_in == 0 && pcmd->hdr.nd_size_out == 0)
> +             return NULL;
> +     else
> +             return (void *)((__u8 *) pcmd + pcmd->payload_offset);
> +}
> +

we need to make sure userspace is not passing a wrong payload_offset.

and in the next patch you do

+       /* Copy the health struct to the payload */
+       memcpy(pdsm_cmd_to_payload(pkg), &p->health, copysize);
+       pkg->hdr.nd_fw_size = copysize;
+

All this can be simplified if you can keep payload at
nd_pdsm_cmd_pkg->payload.

If you still want to have the ability to extend the header, then added a
reserved field similar to nd_cmd_pkg.


-aneesh

Reply via email to