diff mbox series

[for-next,1/2] RDMA/hns: Support query information of functions from FW

Message ID 1615542507-40018-2-git-send-email-liweihang@huawei.com (mailing list archive)
State Superseded
Headers show
Series RDMA/hns: Support to select congestion control algorithm | expand

Commit Message

Weihang Li March 12, 2021, 9:48 a.m. UTC 
From: Wei Xu <xuwei5@hisilicon.com>

Add a new type of command to query mac id of functions from the firmware,
it is used to select the template of congestion algorithm. More info will
be supported in the future.

Signed-off-by: Wei Xu <xuwei5@hisilicon.com>
Signed-off-by: Shengming Shu <shushengming1@huawei.com>
Signed-off-by: Weihang Li <liweihang@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_device.h |  1 +
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c  | 28 ++++++++++++++++++++++++++++
 drivers/infiniband/hw/hns/hns_roce_hw_v2.h  |  7 +++++++
 3 files changed, 36 insertions(+)

Comments

Jason Gunthorpe March 23, 2021, 7:56 p.m. UTC | #1 
On Fri, Mar 12, 2021 at 05:48:26PM +0800, Weihang Li wrote:

> +static int hns_roce_query_func_info(struct hns_roce_dev *hr_dev)
> +{
> +	struct hns_roce_pf_func_info *resp;
> +	struct hns_roce_cmq_desc desc;
> +	int ret;
> +
> +	if (hr_dev->pci_dev->revision < PCI_REVISION_ID_HIP09)
> +		return 0;
> +
> +	hns_roce_cmq_setup_basic_desc(&desc, HNS_ROCE_OPC_QUERY_FUNC_INFO,
> +				      true);
> +	ret = hns_roce_cmq_send(hr_dev, &desc, 1);
> +	if (ret)
> +		return ret;
> +
> +	resp = (struct hns_roce_pf_func_info *)desc.data;

WTF is this cast?

struct hns_roce_cmq_desc {
        __le16 opcode;
        __le16 flag;
        __le16 retval;
        __le16 rsv;
        __le32 data[6];
};

Casting __le32 to a pointer is wrong

Jason
Weihang Li March 24, 2021, 1:54 a.m. UTC | #2 
On 2021/3/24 3:56, Jason Gunthorpe wrote:
> On Fri, Mar 12, 2021 at 05:48:26PM +0800, Weihang Li wrote:
> 
>> +static int hns_roce_query_func_info(struct hns_roce_dev *hr_dev)
>> +{
>> +	struct hns_roce_pf_func_info *resp;
>> +	struct hns_roce_cmq_desc desc;
>> +	int ret;
>> +
>> +	if (hr_dev->pci_dev->revision < PCI_REVISION_ID_HIP09)
>> +		return 0;
>> +
>> +	hns_roce_cmq_setup_basic_desc(&desc, HNS_ROCE_OPC_QUERY_FUNC_INFO,
>> +				      true);
>> +	ret = hns_roce_cmq_send(hr_dev, &desc, 1);
>> +	if (ret)
>> +		return ret;
>> +
>> +	resp = (struct hns_roce_pf_func_info *)desc.data;
> 
> WTF is this cast?
> 
> struct hns_roce_cmq_desc {
>         __le16 opcode;
>         __le16 flag;
>         __le16 retval;
>         __le16 rsv;
>         __le32 data[6];
> };
> 
> Casting __le32 to a pointer is wrong
> 
> Jason
> 

Hi Jason

desc.data is the address of array 'data[6]', it is got from the firmware, we
cast it to 'struct hns_roce_pf_func_info *' to parse its contents. I think this
is a cast from '__le32 *' to 'struct hns_roce_pf_func_info *'.

Thanks
Weihang
Weihang Li March 24, 2021, 3:07 a.m. UTC | #3 
On 2021/3/24 9:55, liweihang wrote:
> On 2021/3/24 3:56, Jason Gunthorpe wrote:
>> On Fri, Mar 12, 2021 at 05:48:26PM +0800, Weihang Li wrote:
>>
>>> +static int hns_roce_query_func_info(struct hns_roce_dev *hr_dev)
>>> +{
>>> +	struct hns_roce_pf_func_info *resp;
>>> +	struct hns_roce_cmq_desc desc;
>>> +	int ret;
>>> +
>>> +	if (hr_dev->pci_dev->revision < PCI_REVISION_ID_HIP09)
>>> +		return 0;
>>> +
>>> +	hns_roce_cmq_setup_basic_desc(&desc, HNS_ROCE_OPC_QUERY_FUNC_INFO,
>>> +				      true);
>>> +	ret = hns_roce_cmq_send(hr_dev, &desc, 1);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	resp = (struct hns_roce_pf_func_info *)desc.data;
>>
>> WTF is this cast?
>>
>> struct hns_roce_cmq_desc {
>>         __le16 opcode;
>>         __le16 flag;
>>         __le16 retval;
>>         __le16 rsv;
>>         __le32 data[6];
>> };
>>
>> Casting __le32 to a pointer is wrong
>>
>> Jason
>>
> 
> Hi Jason
> 
> desc.data is the address of array 'data[6]', it is got from the firmware, we
> cast it to 'struct hns_roce_pf_func_info *' to parse its contents. I think this
> is a cast from '__le32 *' to 'struct hns_roce_pf_func_info *'.
> 
> Thanks
> Weihang
> 
> 

Jason, do you mean the current code is not written correctly? Do you have any
suggestions for achieving our purpose?

Weihang
Jason Gunthorpe March 24, 2021, 12:06 p.m. UTC | #4 
On Wed, Mar 24, 2021 at 03:07:47AM +0000, liweihang wrote:
> On 2021/3/24 9:55, liweihang wrote:
> > On 2021/3/24 3:56, Jason Gunthorpe wrote:
> >> On Fri, Mar 12, 2021 at 05:48:26PM +0800, Weihang Li wrote:
> >>
> >>> +static int hns_roce_query_func_info(struct hns_roce_dev *hr_dev)
> >>> +{
> >>> +	struct hns_roce_pf_func_info *resp;
> >>> +	struct hns_roce_cmq_desc desc;
> >>> +	int ret;
> >>> +
> >>> +	if (hr_dev->pci_dev->revision < PCI_REVISION_ID_HIP09)
> >>> +		return 0;
> >>> +
> >>> +	hns_roce_cmq_setup_basic_desc(&desc, HNS_ROCE_OPC_QUERY_FUNC_INFO,
> >>> +				      true);
> >>> +	ret = hns_roce_cmq_send(hr_dev, &desc, 1);
> >>> +	if (ret)
> >>> +		return ret;
> >>> +
> >>> +	resp = (struct hns_roce_pf_func_info *)desc.data;
> >>
> >> WTF is this cast?
> >>
> >> struct hns_roce_cmq_desc {
> >>         __le16 opcode;
> >>         __le16 flag;
> >>         __le16 retval;
> >>         __le16 rsv;
> >>         __le32 data[6];
> >> };
> >>
> >> Casting __le32 to a pointer is wrong
> >>
> >> Jason
> >>
> > 
> > Hi Jason
> > 
> > desc.data is the address of array 'data[6]', it is got from the firmware, we
> > cast it to 'struct hns_roce_pf_func_info *' to parse its contents. I think this
> > is a cast from '__le32 *' to 'struct hns_roce_pf_func_info *'.
> > 
> > Thanks
> > Weihang
> > 
> > 
> 
> Jason, do you mean the current code is not written correctly? Do you have any
> suggestions for achieving our purpose?

Use a union

Jason
diff mbox series

Patch

diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index 1a95fb7..869548e 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -1001,6 +1001,7 @@  struct hns_roce_dev {
 	void			*priv;
 	struct workqueue_struct *irq_workq;
 	const struct hns_roce_dfx_hw *dfx;
+	u32 cong_algo_tmpl_id;
 };
 
 static inline struct hns_roce_dev *to_hr_dev(struct ib_device *ib_dev)
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 8f73d006..816006d 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -1537,6 +1537,27 @@  static int hns_roce_query_fw_ver(struct hns_roce_dev *hr_dev)
 	return 0;
 }
 
+static int hns_roce_query_func_info(struct hns_roce_dev *hr_dev)
+{
+	struct hns_roce_pf_func_info *resp;
+	struct hns_roce_cmq_desc desc;
+	int ret;
+
+	if (hr_dev->pci_dev->revision < PCI_REVISION_ID_HIP09)
+		return 0;
+
+	hns_roce_cmq_setup_basic_desc(&desc, HNS_ROCE_OPC_QUERY_FUNC_INFO,
+				      true);
+	ret = hns_roce_cmq_send(hr_dev, &desc, 1);
+	if (ret)
+		return ret;
+
+	resp = (struct hns_roce_pf_func_info *)desc.data;
+	hr_dev->cong_algo_tmpl_id = le32_to_cpu(resp->own_mac_id);
+
+	return 0;
+}
+
 static int hns_roce_config_global_param(struct hns_roce_dev *hr_dev)
 {
 	struct hns_roce_cfg_global_param *req;
@@ -2279,6 +2300,13 @@  static int hns_roce_v2_profile(struct hns_roce_dev *hr_dev)
 		return ret;
 	}
 
+	ret = hns_roce_query_func_info(hr_dev);
+	if (ret) {
+		dev_err(hr_dev->dev, "Query function info fail, ret = %d.\n",
+			ret);
+		return ret;
+	}
+
 	ret = hns_roce_config_global_param(hr_dev);
 	if (ret) {
 		dev_err(hr_dev->dev, "Configure global param fail, ret = %d.\n",
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
index ffdae15..74a1c15 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
@@ -235,6 +235,7 @@  enum hns_roce_opcode_type {
 	HNS_ROCE_OPC_CFG_EXT_LLM			= 0x8403,
 	HNS_ROCE_OPC_CFG_TMOUT_LLM			= 0x8404,
 	HNS_ROCE_OPC_QUERY_PF_TIMER_RES			= 0x8406,
+	HNS_ROCE_OPC_QUERY_FUNC_INFO			= 0x8407,
 	HNS_ROCE_OPC_QUERY_PF_CAPS_NUM                  = 0x8408,
 	HNS_ROCE_OPC_CFG_ENTRY_SIZE			= 0x8409,
 	HNS_ROCE_OPC_CFG_SGID_TB			= 0x8500,
@@ -1469,6 +1470,12 @@  struct hns_roce_pf_timer_res_a {
 #define PF_RES_DATA_2_PF_CQC_TIMER_BT_NUM_S 16
 #define PF_RES_DATA_2_PF_CQC_TIMER_BT_NUM_M GENMASK(27, 16)
 
+struct hns_roce_pf_func_info {
+	__le32 rsv1;
+	__le32 own_mac_id;
+	__le32 rsv2[4];
+};
+
 struct hns_roce_vf_res_a {
 	__le32 vf_id;
 	__le32 vf_qpc_bt_idx_num;