Re: [PATCH] powerpc/nvdimm: Add support for multibyte read/write for metadata

["Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>]

Michael Ellerman <mpe@ellerman.id.au> writes:

> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> Oliver <oohall@gmail.com> writes:
>>> On Sun, Jun 2, 2019 at 2:44 PM Aneesh Kumar K.V
>>> <aneesh.kumar@linux.ibm.com> wrote:
> ...
>>>> 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;
> ...
>>>
>>> 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.
>
> With a big endian kernel?

I completed this testing and found new bugs in other parts of the code.

Thanks for the sugestion.

-aneesh