Re: [PATCH for-rc 5/9] RDMA/hns: Fix missing pagesize and alignment check in FRMR

[Junxian Huang <huangjunxian6@hisilicon.com>]

On 2024/7/7 17:16, Zhu Yanjun wrote:
> 在 2024/7/5 16:59, Junxian Huang 写道:
>> From: Chengchang Tang <tangchengchang@huawei.com>
>>
>> The offset requires 128B alignment and the page size ranges from
>> 4K to 128M.
>>
>> Fixes: 68a997c5d28c ("RDMA/hns: Add FRMR support for hip08")
> 
> https://patchwork.kernel.org/project/linux-rdma/patch/2eee7e35-504e-4f2a-a364-527e90669108@CMEXHTCAS1.ad.emulex.com/
> In the above link, from Bart, it seems that FRMR is renamed to FRWR.
> "
> There are already a few drivers upstream in which the fast register
> memory region work request is abbreviated as FRWR. Please consider
> renaming FRMR into FRWR in order to avoid confusion and in order to
> make it easier to find related code with grep in the kernel tree.
> "
> 
> So is it possible to rename FRMR to FRWR?
> 

I think the rename is irrelevant to this bugfix, and if it needs to be done,
we'll need a single patch to rename all existing 'FRMR' in hns driver.

So let's leave it as is for now.

Thanks,
Junxian

>> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
>> Signed-off-by: Junxian Huang <huangjunxian6@hisilicon.com>
>> ---
>>   drivers/infiniband/hw/hns/hns_roce_device.h | 4 ++++
>>   drivers/infiniband/hw/hns/hns_roce_mr.c     | 5 +++++
>>   2 files changed, 9 insertions(+)
>>
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
>> index 5a2445f357ab..15b3b978a601 100644
>> --- a/drivers/infiniband/hw/hns/hns_roce_device.h
>> +++ b/drivers/infiniband/hw/hns/hns_roce_device.h
>> @@ -83,6 +83,7 @@
>>   #define MR_TYPE_DMA                0x03
>>     #define HNS_ROCE_FRMR_MAX_PA            512
>> +#define HNS_ROCE_FRMR_ALIGN_SIZE        128
>>     #define PKEY_ID                    0xffff
>>   #define NODE_DESC_SIZE                64
>> @@ -189,6 +190,9 @@ enum {
>>   #define HNS_HW_PAGE_SHIFT            12
>>   #define HNS_HW_PAGE_SIZE            (1 << HNS_HW_PAGE_SHIFT)
>>   +#define HNS_HW_MAX_PAGE_SHIFT            27
>> +#define HNS_HW_MAX_PAGE_SIZE            (1 << HNS_HW_MAX_PAGE_SHIFT)
>> +
>>   struct hns_roce_uar {
>>       u64        pfn;
>>       unsigned long    index;
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_mr.c b/drivers/infiniband/hw/hns/hns_roce_mr.c
>> index 1a61dceb3319..846da8c78b8b 100644
>> --- a/drivers/infiniband/hw/hns/hns_roce_mr.c
>> +++ b/drivers/infiniband/hw/hns/hns_roce_mr.c
>> @@ -443,6 +443,11 @@ int hns_roce_map_mr_sg(struct ib_mr *ibmr, struct scatterlist *sg, int sg_nents,
>>       struct hns_roce_mtr *mtr = &mr->pbl_mtr;
>>       int ret, sg_num = 0;
>>   +    if (!IS_ALIGNED(*sg_offset, HNS_ROCE_FRMR_ALIGN_SIZE) ||
>> +        ibmr->page_size < HNS_HW_PAGE_SIZE ||
>> +        ibmr->page_size > HNS_HW_MAX_PAGE_SIZE)
>> +        return sg_num;
>> +
>>       mr->npages = 0;
>>       mr->page_list = kvcalloc(mr->pbl_mtr.hem_cfg.buf_pg_count,
>>                    sizeof(dma_addr_t), GFP_KERNEL);
>