On 02/06/2019 14:43, Aneesh Kumar K.V wrote:
> SCM_READ/WRITE_MEATADATA hcall supports multibyte read/write. This patch
> updates the metadata read/write to use 1, 2, 4 or 8 byte read/write as
> mentioned in PAPR document.
>
> READ/WRITE_METADATA hcall supports the 1, 2, 4, or 8 bytes read/write.
> For other values hcall results H_P3.
>
> Hypervisor stores the metadata contents in big-endian format and in-order
> to enable read/write in different granularity, we need to switch the contents
> to big-endian before calling HCALL.
>
> Based on an patch from Oliver O'Halloran <ooh...@gmail.com>
> Signed-off-by: Aneesh Kumar K.V <aneesh.ku...@linux.ibm.com>
> ---
> arch/powerpc/platforms/pseries/papr_scm.c | 104 +++++++++++++++++-----
> 1 file changed, 82 insertions(+), 22 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c
> b/arch/powerpc/platforms/pseries/papr_scm.c
> index 0176ce66673f..e33cebb8ee6c 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -97,42 +97,102 @@ static int drc_pmem_unbind(struct papr_scm_priv *p)
> }
>
> static int papr_scm_meta_get(struct papr_scm_priv *p,
> - struct nd_cmd_get_config_data_hdr *hdr)
> + struct nd_cmd_get_config_data_hdr *hdr)
> {
> unsigned long data[PLPAR_HCALL_BUFSIZE];
> + unsigned long offset, data_offset;
> + int len, read;
> int64_t ret;
>
> - if (hdr->in_offset >= p->metadata_size || hdr->in_length != 1)
> + if ((hdr->in_offset + hdr->in_length) >= p->metadata_size)
> return -EINVAL;
>
> - ret = plpar_hcall(H_SCM_READ_METADATA, data, p->drc_index,
> - hdr->in_offset, 1);
> -
> - if (ret == H_PARAMETER) /* bad DRC index */
> - return -ENODEV;
> - if (ret)
> - return -EINVAL; /* other invalid parameter */
> -
> - hdr->out_buf[0] = data[0] & 0xff;
> -
> + for (len = hdr->in_length; len; len -= read) {
> +
> + data_offset = hdr->in_length - len;
> + offset = hdr->in_offset + data_offset;
> +
> + if (len >= 8)
> + read = 8;
> + else if (len >= 4)
> + read = 4;
> + else if ( len >= 2)
Do not need a space before "len".
> + read = 2;
> + else
> + read = 1;
> +
> + ret = plpar_hcall(H_SCM_READ_METADATA, data, p->drc_index,
> + offset, read);
> +
> + if (ret == H_PARAMETER) /* bad DRC index */
> + return -ENODEV;
> + if (ret)
> + return -EINVAL; /* other invalid parameter */
> +
> + switch (read) {
> + case 8:
> + *(uint64_t *)(hdr->out_buf + data_offset) =
> be64_to_cpu(data[0]);
> + break;
> + case 4:
> + *(uint32_t *)(hdr->out_buf + data_offset) =
> be32_to_cpu(data[0] & 0xffffffff);
> + break;
> +
> + case 2:
> + *(uint16_t *)(hdr->out_buf + data_offset) =
> be16_to_cpu(data[0] & 0xffff);
> + break;
> +
> + case 1:
> + *(uint32_t *)(hdr->out_buf + data_offset) = (data[0] &
> 0xff);
Memory corruption, should be uint8_t*.
> + break;
> + }
> + }
> return 0;
> }
>
> static int papr_scm_meta_set(struct papr_scm_priv *p,
> - struct nd_cmd_set_config_hdr *hdr)
> + struct nd_cmd_set_config_hdr *hdr)
> {
> + unsigned long offset, data_offset;
> + int len, wrote;
> + unsigned long data;
> + __be64 data_be;
> int64_t ret;
>
> - if (hdr->in_offset >= p->metadata_size || hdr->in_length != 1)
> + if ((hdr->in_offset + hdr->in_length) >= p->metadata_size)
> return -EINVAL;
>
> - ret = plpar_hcall_norets(H_SCM_WRITE_METADATA,
> - p->drc_index, hdr->in_offset, hdr->in_buf[0], 1);
> -
> - if (ret == H_PARAMETER) /* bad DRC index */
> - return -ENODEV;
> - if (ret)
> - return -EINVAL; /* other invalid parameter */
> + for (len = hdr->in_length; len; len -= wrote) {
> +
> + data_offset = hdr->in_length - len;
> + offset = hdr->in_offset + data_offset;
> +
> + if (len >= 8) {
> + data = *(uint64_t *)(hdr->in_buf + data_offset);
> + data_be = cpu_to_be64(data);
> + wrote = 8;
> + } else if (len >= 4) {
> + data = *(uint32_t *)(hdr->in_buf + data_offset);
> + data &= 0xffffffff;
Why do you need &0xffffffff here and below (&0xffff, &0xff)? uint32_t is
unsigned type so the sign bit won't be extended.
> + data_be = cpu_to_be32(data);
> + wrote = 4;
> + } else if (len >= 2) {
> + data = *(uint16_t *)(hdr->in_buf + data_offset);
> + data &= 0xffff;
> + data_be = cpu_to_be16(data);
> + wrote = 2;
> + } else {
> + data_be = *(uint8_t *)(hdr->in_buf + data_offset);
> + data_be &= 0xff;
> + wrote = 1;
> + }
> +
> + ret = plpar_hcall_norets(H_SCM_WRITE_METADATA, p->drc_index,
> + offset, data_be, wrote);
> + if (ret == H_PARAMETER) /* bad DRC index */
> + return -ENODEV;
> + if (ret)
> + return -EINVAL; /* other invalid parameter */
> + }
>
> return 0;
> }
> @@ -154,7 +214,7 @@ int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc,
> struct nvdimm *nvdimm,
> get_size_hdr = buf;
>
> get_size_hdr->status = 0;
> - get_size_hdr->max_xfer = 1;
> + get_size_hdr->max_xfer = 8;
> get_size_hdr->config_size = p->metadata_size;
> *cmd_rc = 0;
> break;
>
--
Alexey
