Oliver <ooh...@gmail.com> writes: > On Sun, Jun 2, 2019 at 2:44 PM Aneesh Kumar K.V > <aneesh.ku...@linux.ibm.com> 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. > > You should probably fold the second paragraph here into the first. > >> 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) >> + 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); >> + 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; >> + 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; >> -- >> 2.21.0 > > I assume you got the qemu bits sorted out with Shiva? Looks good otherwise.
That is correct. I also tested with different xfer values (1, 2, 4, 8) on both Qemu and PowerVM. > > Reviewed-by: Oliver O'Halloran <ooh...@gmail.com> Thanks -aneesh