[PATCH] powerpc/nvdimm: Add support for multibyte read/write for metadata
Aneesh Kumar K.V
aneesh.kumar at linux.ibm.com
Tue Jun 4 19:06:34 AEST 2019
Oliver <oohall at gmail.com> writes:
> On Sun, Jun 2, 2019 at 2:44 PM Aneesh Kumar K.V
> <aneesh.kumar at 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 <oohall at gmail.com>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar at 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 <oohall at gmail.com>
Thanks
-aneesh
More information about the Linuxppc-dev
mailing list