* [PATCH net-next 5/7] net: hns3: modify some logs format
From: Huazhong Tan @ 2019-09-10 8:58 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
jakub.kicinski, Guangbin Huang, Huazhong Tan
In-Reply-To: <1568105908-60983-1-git-send-email-tanhuazhong@huawei.com>
From: Guangbin Huang <huangguangbin2@huawei.com>
The pfc_en and pfc_map need to be displayed in hexadecimal notation,
printing dma address should use %pad, and the end of printed string
needs to be add "\n".
This patch modifies them.
Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c | 7 +++++--
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c | 2 +-
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 2 +-
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
index 5cf4c1e..28961a6 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
@@ -166,6 +166,7 @@ static int hns3_dbg_bd_info(struct hnae3_handle *h, const char *cmd_buf)
struct hns3_enet_ring *ring;
u32 tx_index, rx_index;
u32 q_num, value;
+ dma_addr_t addr;
int cnt;
cnt = sscanf(&cmd_buf[8], "%u %u", &q_num, &tx_index);
@@ -194,8 +195,9 @@ static int hns3_dbg_bd_info(struct hnae3_handle *h, const char *cmd_buf)
}
tx_desc = &ring->desc[tx_index];
+ addr = le64_to_cpu(tx_desc->addr);
dev_info(dev, "TX Queue Num: %u, BD Index: %u\n", q_num, tx_index);
- dev_info(dev, "(TX)addr: 0x%llx\n", tx_desc->addr);
+ dev_info(dev, "(TX)addr: %pad\n", &addr);
dev_info(dev, "(TX)vlan_tag: %u\n", tx_desc->tx.vlan_tag);
dev_info(dev, "(TX)send_size: %u\n", tx_desc->tx.send_size);
dev_info(dev, "(TX)vlan_tso: %u\n", tx_desc->tx.type_cs_vlan_tso);
@@ -217,8 +219,9 @@ static int hns3_dbg_bd_info(struct hnae3_handle *h, const char *cmd_buf)
rx_index = (cnt == 1) ? value : tx_index;
rx_desc = &ring->desc[rx_index];
+ addr = le64_to_cpu(rx_desc->addr);
dev_info(dev, "RX Queue Num: %u, BD Index: %u\n", q_num, rx_index);
- dev_info(dev, "(RX)addr: 0x%llx\n", rx_desc->addr);
+ dev_info(dev, "(RX)addr: %pad\n", &addr);
dev_info(dev, "(RX)l234_info: %u\n", rx_desc->rx.l234_info);
dev_info(dev, "(RX)pkt_len: %u\n", rx_desc->rx.pkt_len);
dev_info(dev, "(RX)size: %u\n", rx_desc->rx.size);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
index 816f920..c063301 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
@@ -342,7 +342,7 @@ static int hclge_ieee_setpfc(struct hnae3_handle *h, struct ieee_pfc *pfc)
hdev->tm_info.pfc_en = pfc->pfc_en;
netif_dbg(h, drv, netdev,
- "set pfc: pfc_en=%u, pfc_map=%u, num_tc=%u\n",
+ "set pfc: pfc_en=%x, pfc_map=%x, num_tc=%u\n",
pfc->pfc_en, pfc_map, hdev->tm_info.num_tc);
hclge_tm_pfc_info_update(hdev);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 8d4dc1b..bc5bad3 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -3751,7 +3751,7 @@ static void hclge_reset_event(struct pci_dev *pdev, struct hnae3_handle *handle)
else if (time_after(jiffies, (hdev->last_reset_time + 4 * 5 * HZ)))
hdev->reset_level = HNAE3_FUNC_RESET;
- dev_info(&hdev->pdev->dev, "received reset event , reset type is %d",
+ dev_info(&hdev->pdev->dev, "received reset event, reset type is %d\n",
hdev->reset_level);
/* request reset & schedule reset task */
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 3/7] net: hns3: fix shaper parameter algorithm
From: Huazhong Tan @ 2019-09-10 8:58 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
jakub.kicinski, Yonglong Liu, Huazhong Tan
In-Reply-To: <1568105908-60983-1-git-send-email-tanhuazhong@huawei.com>
From: Yonglong Liu <liuyonglong@huawei.com>
Currently when hns3 driver configures the tm shaper to limit
bandwidth below 20Mbit using the parameters calculated by
hclge_shaper_para_calc(), the actual bandwidth limited by tm
hardware module is not accurate enough, for example, 1.28 Mbit
when the user is configuring 1 Mbit.
This patch adjusts the ir_calc to be closer to ir, and
always calculate the ir_b parameter when user is configuring
a small bandwidth. Also, removes an unnecessary parenthesis
when calculating denominator.
Fixes: 848440544b41 ("net: hns3: Add support of TX Scheduler & Shaper to HNS3 driver")
Signed-off-by: Yonglong Liu <liuyonglong@huawei.com>
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
index e829101..9f0e35f 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
@@ -81,16 +81,13 @@ static int hclge_shaper_para_calc(u32 ir, u8 shaper_level,
return 0;
} else if (ir_calc > ir) {
/* Increasing the denominator to select ir_s value */
- while (ir_calc > ir) {
+ while (ir_calc >= ir && ir) {
ir_s_calc++;
ir_calc = DIVISOR_IR_B_126 / (tick * (1 << ir_s_calc));
}
- if (ir_calc == ir)
- *ir_b = 126;
- else
- *ir_b = (ir * tick * (1 << ir_s_calc) +
- (DIVISOR_CLK >> 1)) / DIVISOR_CLK;
+ *ir_b = (ir * tick * (1 << ir_s_calc) + (DIVISOR_CLK >> 1)) /
+ DIVISOR_CLK;
} else {
/* Increasing the numerator to select ir_u value */
u32 numerator;
@@ -104,7 +101,7 @@ static int hclge_shaper_para_calc(u32 ir, u8 shaper_level,
if (ir_calc == ir) {
*ir_b = 126;
} else {
- u32 denominator = (DIVISOR_CLK * (1 << --ir_u_calc));
+ u32 denominator = DIVISOR_CLK * (1 << --ir_u_calc);
*ir_b = (ir * tick + (denominator >> 1)) / denominator;
}
}
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 2/7] net: hns3: revert to old channel when setting new channel num fail
From: Huazhong Tan @ 2019-09-10 8:58 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
jakub.kicinski, Peng Li, Huazhong Tan
In-Reply-To: <1568105908-60983-1-git-send-email-tanhuazhong@huawei.com>
From: Peng Li <lipeng321@huawei.com>
After setting new channel num, it needs free old ring memory and
allocate new ring memory. If there is no enough memory and allocate
new ring memory fail, the ring may initialize fail. To make sure
the network interface can work normally, driver should revert the
channel to the old configuration.
Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 51 ++++++++++++++++++-------
1 file changed, 37 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 9f3f8e3..8dbaf36 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -4410,6 +4410,30 @@ static int hns3_reset_notify(struct hnae3_handle *handle,
return ret;
}
+static int hns3_change_channels(struct hnae3_handle *handle, u32 new_tqp_num,
+ bool rxfh_configured)
+{
+ int ret;
+
+ ret = handle->ae_algo->ops->set_channels(handle, new_tqp_num,
+ rxfh_configured);
+ if (ret) {
+ dev_err(&handle->pdev->dev,
+ "Change tqp num(%u) fail.\n", new_tqp_num);
+ return ret;
+ }
+
+ ret = hns3_reset_notify(handle, HNAE3_INIT_CLIENT);
+ if (ret)
+ return ret;
+
+ ret = hns3_reset_notify(handle, HNAE3_UP_CLIENT);
+ if (ret)
+ hns3_reset_notify(handle, HNAE3_UNINIT_CLIENT);
+
+ return ret;
+}
+
int hns3_set_channels(struct net_device *netdev,
struct ethtool_channels *ch)
{
@@ -4450,24 +4474,23 @@ int hns3_set_channels(struct net_device *netdev,
return ret;
org_tqp_num = h->kinfo.num_tqps;
- ret = h->ae_algo->ops->set_channels(h, new_tqp_num, rxfh_configured);
+ ret = hns3_change_channels(h, new_tqp_num, rxfh_configured);
if (ret) {
- ret = h->ae_algo->ops->set_channels(h, org_tqp_num,
- rxfh_configured);
- if (ret) {
- /* If revert to old tqp failed, fatal error occurred */
- dev_err(&netdev->dev,
- "Revert to old tqp num fail, ret=%d", ret);
- return ret;
+ int ret1;
+
+ netdev_warn(netdev,
+ "Change channels fail, revert to old value\n");
+ ret1 = hns3_change_channels(h, org_tqp_num, rxfh_configured);
+ if (ret1) {
+ netdev_err(netdev,
+ "revert to old channel fail\n");
+ return ret1;
}
- dev_info(&netdev->dev,
- "Change tqp num fail, Revert to old tqp num");
- }
- ret = hns3_reset_notify(h, HNAE3_INIT_CLIENT);
- if (ret)
+
return ret;
+ }
- return hns3_reset_notify(h, HNAE3_UP_CLIENT);
+ return 0;
}
static const struct hns3_hw_error_info hns3_hw_err[] = {
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 0/7] net: hns3: add a feature & bugfixes & cleanups
From: Huazhong Tan @ 2019-09-10 8:58 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
jakub.kicinski, Huazhong Tan
This patch-set includes a VF feature, bugfixes and cleanups for the HNS3
ethernet controller driver.
[patch 01/07] adds ethtool_ops.set_channels support for HNS3 VF driver
[patch 02/07] adds a recovery for setting channel fail.
[patch 03/07] fixes an error related to shaper parameter algorithm.
[patch 04/07] fixes an error related to ksetting.
[patch 05/07] adds cleanups for some log pinting.
[patch 06/07] adds a NULL pointer check before function calling.
[patch 07/07] adds some debugging information for reset issue.
Guangbin Huang (4):
net: hns3: add ethtool_ops.set_channels support for HNS3 VF driver
net: hns3: fix port setting handle for fibre port
net: hns3: modify some logs format
net: hns3: check NULL pointer before use
Huazhong Tan (1):
net: hns3: add some DFX info for reset issue
Peng Li (1):
net: hns3: revert to old channel when setting new channel num fail
Yonglong Liu (1):
net: hns3: fix shaper parameter algorithm
drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c | 7 +-
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 54 ++++++++++----
drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 16 ++++
.../net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c | 2 +-
.../ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c | 32 +++++---
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 13 ++--
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.h | 2 +-
.../net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c | 11 +--
.../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 87 ++++++++++++++++++++--
9 files changed, 178 insertions(+), 46 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH net-next 6/7] net: hns3: check NULL pointer before use
From: Huazhong Tan @ 2019-09-10 8:58 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
jakub.kicinski, Guangbin Huang, Huazhong Tan
In-Reply-To: <1568105908-60983-1-git-send-email-tanhuazhong@huawei.com>
From: Guangbin Huang <huangguangbin2@huawei.com>
This patch checks ops->set_default_reset_request whether is NULL
before using it in function hns3_slot_reset.
Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 8dbaf36..616cad0 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -2006,7 +2006,8 @@ static pci_ers_result_t hns3_slot_reset(struct pci_dev *pdev)
ops = ae_dev->ops;
/* request the reset */
- if (ops->reset_event && ops->get_reset_level) {
+ if (ops->reset_event && ops->get_reset_level &&
+ ops->set_default_reset_request) {
if (ae_dev->hw_err_reset_req) {
reset_type = ops->get_reset_level(ae_dev,
&ae_dev->hw_err_reset_req);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 7/7] net: hns3: add some DFX info for reset issue
From: Huazhong Tan @ 2019-09-10 8:58 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
jakub.kicinski, Huazhong Tan
In-Reply-To: <1568105908-60983-1-git-send-email-tanhuazhong@huawei.com>
This patch adds more information for reset DFX. Also, adds some
cleanups to reset info, move reset_fail_cnt into struct
hclge_rst_stats, and modifies some print formats.
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
.../ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c | 32 ++++++++++++++++------
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 11 ++++----
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.h | 2 +-
3 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
index 6dcce48..d0128d7 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
@@ -931,22 +931,36 @@ static void hclge_dbg_fd_tcam(struct hclge_dev *hdev)
static void hclge_dbg_dump_rst_info(struct hclge_dev *hdev)
{
- dev_info(&hdev->pdev->dev, "PF reset count: %d\n",
+ dev_info(&hdev->pdev->dev, "PF reset count: %u\n",
hdev->rst_stats.pf_rst_cnt);
- dev_info(&hdev->pdev->dev, "FLR reset count: %d\n",
+ dev_info(&hdev->pdev->dev, "FLR reset count: %u\n",
hdev->rst_stats.flr_rst_cnt);
- dev_info(&hdev->pdev->dev, "CORE reset count: %d\n",
- hdev->rst_stats.core_rst_cnt);
- dev_info(&hdev->pdev->dev, "GLOBAL reset count: %d\n",
+ dev_info(&hdev->pdev->dev, "GLOBAL reset count: %u\n",
hdev->rst_stats.global_rst_cnt);
- dev_info(&hdev->pdev->dev, "IMP reset count: %d\n",
+ dev_info(&hdev->pdev->dev, "IMP reset count: %u\n",
hdev->rst_stats.imp_rst_cnt);
- dev_info(&hdev->pdev->dev, "reset done count: %d\n",
+ dev_info(&hdev->pdev->dev, "reset done count: %u\n",
hdev->rst_stats.reset_done_cnt);
- dev_info(&hdev->pdev->dev, "HW reset done count: %d\n",
+ dev_info(&hdev->pdev->dev, "HW reset done count: %u\n",
hdev->rst_stats.hw_reset_done_cnt);
- dev_info(&hdev->pdev->dev, "reset count: %d\n",
+ dev_info(&hdev->pdev->dev, "reset count: %u\n",
hdev->rst_stats.reset_cnt);
+ dev_info(&hdev->pdev->dev, "reset count: %u\n",
+ hdev->rst_stats.reset_cnt);
+ dev_info(&hdev->pdev->dev, "reset fail count: %u\n",
+ hdev->rst_stats.reset_fail_cnt);
+ dev_info(&hdev->pdev->dev, "vector0 interrupt enable status: 0x%x\n",
+ hclge_read_dev(&hdev->hw, HCLGE_MISC_VECTOR_REG_BASE));
+ dev_info(&hdev->pdev->dev, "reset interrupt source: 0x%x\n",
+ hclge_read_dev(&hdev->hw, HCLGE_MISC_RESET_STS_REG));
+ dev_info(&hdev->pdev->dev, "reset interrupt status: 0x%x\n",
+ hclge_read_dev(&hdev->hw, HCLGE_MISC_VECTOR_INT_STS));
+ dev_info(&hdev->pdev->dev, "hardware reset status: 0x%x\n",
+ hclge_read_dev(&hdev->hw, HCLGE_GLOBAL_RESET_REG));
+ dev_info(&hdev->pdev->dev, "handshake status: 0x%x\n",
+ hclge_read_dev(&hdev->hw, HCLGE_NIC_CSQ_DEPTH_REG));
+ dev_info(&hdev->pdev->dev, "function reset status: 0x%x\n",
+ hclge_read_dev(&hdev->hw, HCLGE_FUN_RST_ING));
}
static void hclge_dbg_get_m7_stats_info(struct hclge_dev *hdev)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index bc5bad3..fd7f943 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -3547,12 +3547,12 @@ static bool hclge_reset_err_handle(struct hclge_dev *hdev)
"reset failed because new reset interrupt\n");
hclge_clear_reset_cause(hdev);
return false;
- } else if (hdev->reset_fail_cnt < MAX_RESET_FAIL_CNT) {
- hdev->reset_fail_cnt++;
+ } else if (hdev->rst_stats.reset_fail_cnt < MAX_RESET_FAIL_CNT) {
+ hdev->rst_stats.reset_fail_cnt++;
set_bit(hdev->reset_type, &hdev->reset_pending);
dev_info(&hdev->pdev->dev,
"re-schedule reset task(%d)\n",
- hdev->reset_fail_cnt);
+ hdev->rst_stats.reset_fail_cnt);
return true;
}
@@ -3679,7 +3679,8 @@ static void hclge_reset(struct hclge_dev *hdev)
/* ignore RoCE notify error if it fails HCLGE_RESET_MAX_FAIL_CNT - 1
* times
*/
- if (ret && hdev->reset_fail_cnt < HCLGE_RESET_MAX_FAIL_CNT - 1)
+ if (ret &&
+ hdev->rst_stats.reset_fail_cnt < HCLGE_RESET_MAX_FAIL_CNT - 1)
goto err_reset;
rtnl_lock();
@@ -3695,7 +3696,7 @@ static void hclge_reset(struct hclge_dev *hdev)
goto err_reset;
hdev->last_reset_time = jiffies;
- hdev->reset_fail_cnt = 0;
+ hdev->rst_stats.reset_fail_cnt = 0;
hdev->rst_stats.reset_done_cnt++;
ae_dev->reset_type = HNAE3_NONE_RESET;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
index 870550f..3e9574a 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
@@ -659,6 +659,7 @@ struct hclge_rst_stats {
u32 global_rst_cnt; /* the number of GLOBAL */
u32 imp_rst_cnt; /* the number of IMP reset */
u32 reset_cnt; /* the number of reset */
+ u32 reset_fail_cnt; /* the number of reset fail */
};
/* time and register status when mac tunnel interruption occur */
@@ -725,7 +726,6 @@ struct hclge_dev {
unsigned long reset_request; /* reset has been requested */
unsigned long reset_pending; /* client rst is pending to be served */
struct hclge_rst_stats rst_stats;
- u32 reset_fail_cnt;
u32 fw_version;
u16 num_vmdq_vport; /* Num vmdq vport this PF has set up */
u16 num_tqps; /* Num task queue pairs of this PF */
--
2.7.4
^ permalink raw reply related
* Re: Default qdisc not correctly initialized with custom MTU
From: Holger Hoffstätte @ 2019-09-10 9:14 UTC (permalink / raw)
To: Cong Wang; +Cc: Netdev
In-Reply-To: <CAM_iQpWKsSWDZ55kMO6mzDe5C7tHW-ub_eH91hRzZMdUtKJtfA@mail.gmail.com>
On 9/10/19 12:52 AM, Cong Wang wrote:
> On Mon, Sep 9, 2019 at 5:44 AM Holger Hoffstätte
> <holger@applied-asynchrony.com> wrote:
>> I can't help but feel this is a slight bug in terms of initialization order,
>> and that the default qdisc should only be created when it's first being
>> used/attached to a link, not when the sysctls are configured.
>
> Yeah, this is because the fq_codel qdisc is initialized once and
> doesn't get any notification when the netdev's MTU get changed.
My point was that it shouldn't be created or initialized at all when
the sysctl is configured, only the name should be validated/stored and
queried when needed. If any interface is brought up before that point,
no value (yet) would just mean "trod along with the defaults" to whoever
is doing the work.
> We can "fix" this by adding a NETDEV_CHANGEMTU notifier to
> qdisc's, but I don't know if it is really worth the effort.
This is essentially the opposite of what I had in mind. The problem is
that the entity was created, not that it needs to be notified.
Also I don't think that would work for scenarios with multiple links
using different MTUs.
> Is there any reason you can't change that order?
Yes, because that wouldn't solve anything?
Like i said I can just kick the root qdisc to update itself in
a post interface-setup script, and that works fine. Since I need
that script anyway for setting several other parameters for
the device it's no big deal - just another workaround.
A brief look at the initialization in sch_mq/sch_generic unfortunately
didn't really help clear things up for me, hence I guess my real
question is whether a qdisc *must* be created early for some reason
(assuming sysctls come before link setup), or whether this is something
that could be delayed and done on-demand.
thanks,
Holger
^ permalink raw reply
* [PATCH v7 1/7] nfc: pn533: i2c: "pn532" as dt compatible string
From: Lars Poeschel @ 2019-09-10 9:31 UTC (permalink / raw)
To: Allison Randal, Greg Kroah-Hartman, Jilayne Lovejoy, Kate Stewart,
Thomas Gleixner, Lars Poeschel, open list:NFC SUBSYSTEM,
open list
Cc: Johan Hovold
It is favourable to have one unified compatible string for devices that
have multiple interfaces. So this adds simply "pn532" as the devicetree
binding compatible string and makes a note that the old ones are
deprecated.
Cc: Johan Hovold <johan@kernel.org>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v6:
- Rebased the patch series on v5.3-rc5
Changes in v3:
- This patch is new in v3
drivers/nfc/pn533/i2c.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/nfc/pn533/i2c.c b/drivers/nfc/pn533/i2c.c
index 1832cd921ea7..1abd40398a5a 100644
--- a/drivers/nfc/pn533/i2c.c
+++ b/drivers/nfc/pn533/i2c.c
@@ -245,6 +245,11 @@ static int pn533_i2c_remove(struct i2c_client *client)
}
static const struct of_device_id of_pn533_i2c_match[] = {
+ { .compatible = "nxp,pn532", },
+ /*
+ * NOTE: The use of the compatibles with the trailing "...-i2c" is
+ * deprecated and will be removed.
+ */
{ .compatible = "nxp,pn533-i2c", },
{ .compatible = "nxp,pn532-i2c", },
{},
--
2.23.0
^ permalink raw reply related
* [PATCH v7 3/7] nfc: pn533: Add dev_up/dev_down hooks to phy_ops
From: Lars Poeschel @ 2019-09-10 9:33 UTC (permalink / raw)
To: Kate Stewart, Gustavo A. R. Silva, Kees Cook, Allison Randal,
Greg Kroah-Hartman, Lars Poeschel, Thomas Gleixner,
Jilayne Lovejoy, Steve Winslow, open list:NFC SUBSYSTEM,
open list
Cc: Johan Hovold
This adds hooks for dev_up and dev_down to the phy_ops. They are
optional.
The idea is to inform the phy driver when the nfc chip is really going
to be used. When it is not used, the phy driver can suspend it's
interface to the nfc chip to save some power. The nfc chip is considered
not in use before dev_up and after dev_down.
Cc: Johan Hovold <johan@kernel.org>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v6:
- Rebased the patch series on v5.3-rc5
Changes in v5:
- (dev->phy_ops->dev_up) instead (dev->phy_ops)
Changes in v4:
- This patch is new in v4
drivers/nfc/pn533/pn533.c | 12 +++++++++++-
drivers/nfc/pn533/pn533.h | 9 +++++++++
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index a172a32aa9d9..64836c727aee 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -2458,6 +2458,9 @@ static int pn533_dev_up(struct nfc_dev *nfc_dev)
{
struct pn533 *dev = nfc_get_drvdata(nfc_dev);
+ if (dev->phy_ops->dev_up)
+ dev->phy_ops->dev_up(dev);
+
if (dev->device_type == PN533_DEVICE_PN532) {
int rc = pn532_sam_configuration(nfc_dev);
@@ -2470,7 +2473,14 @@ static int pn533_dev_up(struct nfc_dev *nfc_dev)
static int pn533_dev_down(struct nfc_dev *nfc_dev)
{
- return pn533_rf_field(nfc_dev, 0);
+ struct pn533 *dev = nfc_get_drvdata(nfc_dev);
+ int ret;
+
+ ret = pn533_rf_field(nfc_dev, 0);
+ if (dev->phy_ops->dev_down && !ret)
+ dev->phy_ops->dev_down(dev);
+
+ return ret;
}
static struct nfc_ops pn533_nfc_ops = {
diff --git a/drivers/nfc/pn533/pn533.h b/drivers/nfc/pn533/pn533.h
index 8bf9d6ece0f5..570ee0a3e832 100644
--- a/drivers/nfc/pn533/pn533.h
+++ b/drivers/nfc/pn533/pn533.h
@@ -207,6 +207,15 @@ struct pn533_phy_ops {
struct sk_buff *out);
int (*send_ack)(struct pn533 *dev, gfp_t flags);
void (*abort_cmd)(struct pn533 *priv, gfp_t flags);
+ /*
+ * dev_up and dev_down are optional.
+ * They are used to inform the phy layer that the nfc chip
+ * is going to be really used very soon. The phy layer can then
+ * bring up it's interface to the chip and have it suspended for power
+ * saving reasons otherwise.
+ */
+ void (*dev_up)(struct pn533 *priv);
+ void (*dev_down)(struct pn533 *priv);
};
--
2.23.0
^ permalink raw reply related
* [PATCH v7 4/7] nfc: pn533: Split pn533 init & nfc_register
From: Lars Poeschel @ 2019-09-10 9:33 UTC (permalink / raw)
To: Thomas Gleixner, Jilayne Lovejoy, Greg Kroah-Hartman,
Allison Randal, Kate Stewart, Lars Poeschel, Kees Cook,
Steve Winslow, Gustavo A. R. Silva, open list:NFC SUBSYSTEM,
open list
Cc: Johan Hovold, Claudiu Beznea
There is a problem in the initialisation and setup of the pn533: It
registers with nfc too early. It could happen, that it finished
registering with nfc and someone starts using it. But setup of the pn533
is not yet finished. Bad or at least unintended things could happen.
So I split out nfc registering (and unregistering) to seperate functions
that have to be called late in probe then.
Cc: Johan Hovold <johan@kernel.org>
Cc: Claudiu Beznea <Claudiu.Beznea@microchip.com>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v7:
- Remove an unneeded rc variable initialization
- Corrected goto error to err_clean in pn533_usb_probe
Changes in v6:
- Rebased the patch series on v5.3-rc5
Changes in v5:
- This patch is new in v5
drivers/nfc/pn533/i2c.c | 17 +++++-----
drivers/nfc/pn533/pn533.c | 66 ++++++++++++++++++++-------------------
drivers/nfc/pn533/pn533.h | 11 ++++---
drivers/nfc/pn533/usb.c | 14 ++++++---
4 files changed, 60 insertions(+), 48 deletions(-)
diff --git a/drivers/nfc/pn533/i2c.c b/drivers/nfc/pn533/i2c.c
index 1abd40398a5a..e9e5a1ec8857 100644
--- a/drivers/nfc/pn533/i2c.c
+++ b/drivers/nfc/pn533/i2c.c
@@ -193,12 +193,10 @@ static int pn533_i2c_probe(struct i2c_client *client,
phy->i2c_dev = client;
i2c_set_clientdata(client, phy);
- priv = pn533_register_device(PN533_DEVICE_PN532,
- PN533_NO_TYPE_B_PROTOCOLS,
+ priv = pn53x_common_init(PN533_DEVICE_PN532,
PN533_PROTO_REQ_ACK_RESP,
phy, &i2c_phy_ops, NULL,
- &phy->i2c_dev->dev,
- &client->dev);
+ &phy->i2c_dev->dev);
if (IS_ERR(priv)) {
r = PTR_ERR(priv);
@@ -220,13 +218,17 @@ static int pn533_i2c_probe(struct i2c_client *client,
if (r)
goto fn_setup_err;
- return 0;
+ r = pn53x_register_nfc(priv, PN533_NO_TYPE_B_PROTOCOLS, &client->dev);
+ if (r)
+ goto fn_setup_err;
+
+ return r;
fn_setup_err:
free_irq(client->irq, phy);
irq_rqst_err:
- pn533_unregister_device(phy->priv);
+ pn53x_common_clean(phy->priv);
return r;
}
@@ -239,7 +241,8 @@ static int pn533_i2c_remove(struct i2c_client *client)
free_irq(client->irq, phy);
- pn533_unregister_device(phy->priv);
+ pn53x_unregister_nfc(phy->priv);
+ pn53x_common_clean(phy->priv);
return 0;
}
diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index 64836c727aee..e5d5e4c83a04 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -2590,14 +2590,12 @@ int pn533_finalize_setup(struct pn533 *dev)
}
EXPORT_SYMBOL_GPL(pn533_finalize_setup);
-struct pn533 *pn533_register_device(u32 device_type,
- u32 protocols,
+struct pn533 *pn53x_common_init(u32 device_type,
enum pn533_protocol_type protocol_type,
void *phy,
struct pn533_phy_ops *phy_ops,
struct pn533_frame_ops *fops,
- struct device *dev,
- struct device *parent)
+ struct device *dev)
{
struct pn533 *priv;
int rc = -ENOMEM;
@@ -2638,43 +2636,18 @@ struct pn533 *pn533_register_device(u32 device_type,
skb_queue_head_init(&priv->fragment_skb);
INIT_LIST_HEAD(&priv->cmd_queue);
-
- priv->nfc_dev = nfc_allocate_device(&pn533_nfc_ops, protocols,
- priv->ops->tx_header_len +
- PN533_CMD_DATAEXCH_HEAD_LEN,
- priv->ops->tx_tail_len);
- if (!priv->nfc_dev) {
- rc = -ENOMEM;
- goto destroy_wq;
- }
-
- nfc_set_parent_dev(priv->nfc_dev, parent);
- nfc_set_drvdata(priv->nfc_dev, priv);
-
- rc = nfc_register_device(priv->nfc_dev);
- if (rc)
- goto free_nfc_dev;
-
return priv;
-free_nfc_dev:
- nfc_free_device(priv->nfc_dev);
-
-destroy_wq:
- destroy_workqueue(priv->wq);
error:
kfree(priv);
return ERR_PTR(rc);
}
-EXPORT_SYMBOL_GPL(pn533_register_device);
+EXPORT_SYMBOL_GPL(pn53x_common_init);
-void pn533_unregister_device(struct pn533 *priv)
+void pn53x_common_clean(struct pn533 *priv)
{
struct pn533_cmd *cmd, *n;
- nfc_unregister_device(priv->nfc_dev);
- nfc_free_device(priv->nfc_dev);
-
flush_delayed_work(&priv->poll_work);
destroy_workqueue(priv->wq);
@@ -2689,8 +2662,37 @@ void pn533_unregister_device(struct pn533 *priv)
kfree(priv);
}
-EXPORT_SYMBOL_GPL(pn533_unregister_device);
+EXPORT_SYMBOL_GPL(pn53x_common_clean);
+
+int pn53x_register_nfc(struct pn533 *priv, u32 protocols,
+ struct device *parent)
+{
+ int rc;
+
+ priv->nfc_dev = nfc_allocate_device(&pn533_nfc_ops, protocols,
+ priv->ops->tx_header_len +
+ PN533_CMD_DATAEXCH_HEAD_LEN,
+ priv->ops->tx_tail_len);
+ if (!priv->nfc_dev)
+ return -ENOMEM;
+
+ nfc_set_parent_dev(priv->nfc_dev, parent);
+ nfc_set_drvdata(priv->nfc_dev, priv);
+
+ rc = nfc_register_device(priv->nfc_dev);
+ if (rc)
+ nfc_free_device(priv->nfc_dev);
+
+ return rc;
+}
+EXPORT_SYMBOL_GPL(pn53x_register_nfc);
+void pn53x_unregister_nfc(struct pn533 *priv)
+{
+ nfc_unregister_device(priv->nfc_dev);
+ nfc_free_device(priv->nfc_dev);
+}
+EXPORT_SYMBOL_GPL(pn53x_unregister_nfc);
MODULE_AUTHOR("Lauro Ramos Venancio <lauro.venancio@openbossa.org>");
MODULE_AUTHOR("Aloisio Almeida Jr <aloisio.almeida@openbossa.org>");
diff --git a/drivers/nfc/pn533/pn533.h b/drivers/nfc/pn533/pn533.h
index 570ee0a3e832..510ddebbd896 100644
--- a/drivers/nfc/pn533/pn533.h
+++ b/drivers/nfc/pn533/pn533.h
@@ -219,18 +219,19 @@ struct pn533_phy_ops {
};
-struct pn533 *pn533_register_device(u32 device_type,
- u32 protocols,
+struct pn533 *pn53x_common_init(u32 device_type,
enum pn533_protocol_type protocol_type,
void *phy,
struct pn533_phy_ops *phy_ops,
struct pn533_frame_ops *fops,
- struct device *dev,
- struct device *parent);
+ struct device *dev);
int pn533_finalize_setup(struct pn533 *dev);
-void pn533_unregister_device(struct pn533 *priv);
+void pn53x_common_clean(struct pn533 *priv);
void pn533_recv_frame(struct pn533 *dev, struct sk_buff *skb, int status);
+int pn53x_register_nfc(struct pn533 *priv, u32 protocols,
+ struct device *parent);
+void pn53x_unregister_nfc(struct pn533 *priv);
bool pn533_rx_frame_is_cmd_response(struct pn533 *dev, void *frame);
bool pn533_rx_frame_is_ack(void *_frame);
diff --git a/drivers/nfc/pn533/usb.c b/drivers/nfc/pn533/usb.c
index c5289eaf17ee..8511fd089970 100644
--- a/drivers/nfc/pn533/usb.c
+++ b/drivers/nfc/pn533/usb.c
@@ -534,9 +534,9 @@ static int pn533_usb_probe(struct usb_interface *interface,
goto error;
}
- priv = pn533_register_device(id->driver_info, protocols, protocol_type,
+ priv = pn53x_common_init(id->driver_info, protocol_type,
phy, &usb_phy_ops, fops,
- &phy->udev->dev, &interface->dev);
+ &phy->udev->dev);
if (IS_ERR(priv)) {
rc = PTR_ERR(priv);
@@ -547,12 +547,17 @@ static int pn533_usb_probe(struct usb_interface *interface,
rc = pn533_finalize_setup(priv);
if (rc)
- goto error;
+ goto err_clean;
usb_set_intfdata(interface, phy);
+ rc = pn53x_register_nfc(priv, protocols, &interface->dev);
+ if (rc)
+ goto err_clean;
return 0;
+err_clean:
+ pn53x_common_clean(priv);
error:
usb_free_urb(phy->in_urb);
usb_free_urb(phy->out_urb);
@@ -570,7 +575,8 @@ static void pn533_usb_disconnect(struct usb_interface *interface)
if (!phy)
return;
- pn533_unregister_device(phy->priv);
+ pn53x_unregister_nfc(phy->priv);
+ pn53x_common_clean(phy->priv);
usb_set_intfdata(interface, NULL);
--
2.23.0
^ permalink raw reply related
* [PATCH v7 5/7] nfc: pn533: add UART phy driver
From: Lars Poeschel @ 2019-09-10 9:33 UTC (permalink / raw)
To: Greg Kroah-Hartman, Thomas Gleixner, Lars Poeschel, Kate Stewart,
Steve Winslow, Allison Randal, open list, open list:NFC SUBSYSTEM
Cc: Johan Hovold, Claudiu Beznea
This adds the UART phy interface for the pn533 driver.
The pn533 driver can be used through UART interface this way.
It is implemented as a serdev device.
Cc: Johan Hovold <johan@kernel.org>
Cc: Claudiu Beznea <Claudiu.Beznea@microchip.com>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v7:
- Add comment at send_wakeup variable to document a possible and
harmless concurrency issue
Changes in v6:
- Rebased the patch series on v5.3-rc5
Changes in v5:
- Use the splitted pn53x_common_init and pn53x_register_nfc
and pn53x_common_clean and pn53x_unregister_nfc alike
Changes in v4:
- SPDX-License-Identifier: GPL-2.0+
- Source code comments above refering items
- Error check for serdev_device_write's
- Change if (xxx == NULL) to if (!xxx)
- Remove device name from a dev_err
- move pn533_register in _probe a bit towards the end of _probe
- make use of newly added dev_up / dev_down phy_ops
- control send_wakeup variable from dev_up / dev_down
Changes in v3:
- depend on SERIAL_DEV_BUS in Kconfig
Changes in v2:
- switched from tty line discipline to serdev, resulting in many
simplifications
- SPDX License Identifier
drivers/nfc/pn533/Kconfig | 11 ++
drivers/nfc/pn533/Makefile | 2 +
drivers/nfc/pn533/pn533.h | 8 +
drivers/nfc/pn533/uart.c | 324 +++++++++++++++++++++++++++++++++++++
4 files changed, 345 insertions(+)
create mode 100644 drivers/nfc/pn533/uart.c
diff --git a/drivers/nfc/pn533/Kconfig b/drivers/nfc/pn533/Kconfig
index f6d6b345ba0d..7fe1bbe26568 100644
--- a/drivers/nfc/pn533/Kconfig
+++ b/drivers/nfc/pn533/Kconfig
@@ -26,3 +26,14 @@ config NFC_PN533_I2C
If you choose to build a module, it'll be called pn533_i2c.
Say N if unsure.
+
+config NFC_PN532_UART
+ tristate "NFC PN532 device support (UART)"
+ depends on SERIAL_DEV_BUS
+ select NFC_PN533
+ ---help---
+ This module adds support for the NXP pn532 UART interface.
+ Select this if your platform is using the UART bus.
+
+ If you choose to build a module, it'll be called pn532_uart.
+ Say N if unsure.
diff --git a/drivers/nfc/pn533/Makefile b/drivers/nfc/pn533/Makefile
index 43c25b4f9466..b9648337576f 100644
--- a/drivers/nfc/pn533/Makefile
+++ b/drivers/nfc/pn533/Makefile
@@ -4,7 +4,9 @@
#
pn533_usb-objs = usb.o
pn533_i2c-objs = i2c.o
+pn532_uart-objs = uart.o
obj-$(CONFIG_NFC_PN533) += pn533.o
obj-$(CONFIG_NFC_PN533_USB) += pn533_usb.o
obj-$(CONFIG_NFC_PN533_I2C) += pn533_i2c.o
+obj-$(CONFIG_NFC_PN532_UART) += pn532_uart.o
diff --git a/drivers/nfc/pn533/pn533.h b/drivers/nfc/pn533/pn533.h
index 510ddebbd896..6541088fad73 100644
--- a/drivers/nfc/pn533/pn533.h
+++ b/drivers/nfc/pn533/pn533.h
@@ -43,6 +43,11 @@
/* Preamble (1), SoPC (2), ACK Code (2), Postamble (1) */
#define PN533_STD_FRAME_ACK_SIZE 6
+/*
+ * Preamble (1), SoPC (2), Packet Length (1), Packet Length Checksum (1),
+ * Specific Application Level Error Code (1) , Postamble (1)
+ */
+#define PN533_STD_ERROR_FRAME_SIZE 8
#define PN533_STD_FRAME_CHECKSUM(f) (f->data[f->datalen])
#define PN533_STD_FRAME_POSTAMBLE(f) (f->data[f->datalen + 1])
/* Half start code (3), LEN (4) should be 0xffff for extended frame */
@@ -84,6 +89,9 @@
#define PN533_CMD_MI_MASK 0x40
#define PN533_CMD_RET_SUCCESS 0x00
+#define PN533_FRAME_DATALEN_ACK 0x00
+#define PN533_FRAME_DATALEN_ERROR 0x01
+#define PN533_FRAME_DATALEN_EXTENDED 0xFF
enum pn533_protocol_type {
PN533_PROTO_REQ_ACK_RESP = 0,
diff --git a/drivers/nfc/pn533/uart.c b/drivers/nfc/pn533/uart.c
new file mode 100644
index 000000000000..cce8f29eda8b
--- /dev/null
+++ b/drivers/nfc/pn533/uart.c
@@ -0,0 +1,324 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for NXP PN532 NFC Chip - UART transport layer
+ *
+ * Copyright (C) 2018 Lemonage Software GmbH
+ * Author: Lars Pöschel <poeschel@lemonage.de>
+ * All rights reserved.
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/nfc.h>
+#include <linux/netdevice.h>
+#include <linux/of.h>
+#include <linux/serdev.h>
+#include "pn533.h"
+
+#define PN532_UART_SKB_BUFF_LEN (PN533_CMD_DATAEXCH_DATA_MAXLEN * 2)
+
+enum send_wakeup {
+ PN532_SEND_NO_WAKEUP = 0,
+ PN532_SEND_WAKEUP,
+ PN532_SEND_LAST_WAKEUP,
+};
+
+
+struct pn532_uart_phy {
+ struct serdev_device *serdev;
+ struct sk_buff *recv_skb;
+ struct pn533 *priv;
+ /*
+ * send_wakeup variable is used to control if we need to send a wakeup
+ * request to the pn532 chip prior to our actual command. There is a
+ * little propability of a race condition. We decided to not mutex the
+ * variable as the worst that could happen is, that we send a wakeup
+ * to the chip that is already awake. This does not hurt. It is a
+ * no-op to the chip.
+ */
+ enum send_wakeup send_wakeup;
+ struct timer_list cmd_timeout;
+ struct sk_buff *cur_out_buf;
+};
+
+static int pn532_uart_send_frame(struct pn533 *dev,
+ struct sk_buff *out)
+{
+ /* wakeup sequence and dummy bytes for waiting time */
+ static const u8 wakeup[] = {
+ 0x55, 0x55, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+ struct pn532_uart_phy *pn532 = dev->phy;
+ int err;
+
+ print_hex_dump_debug("PN532_uart TX: ", DUMP_PREFIX_NONE, 16, 1,
+ out->data, out->len, false);
+
+ pn532->cur_out_buf = out;
+ if (pn532->send_wakeup) {
+ err= serdev_device_write(pn532->serdev,
+ wakeup, sizeof(wakeup),
+ MAX_SCHEDULE_TIMEOUT);
+ if (err < 0)
+ return err;
+ }
+
+ if (pn532->send_wakeup == PN532_SEND_LAST_WAKEUP) {
+ pn532->send_wakeup = PN532_SEND_NO_WAKEUP;
+ }
+
+ err = serdev_device_write(pn532->serdev, out->data, out->len,
+ MAX_SCHEDULE_TIMEOUT);
+ if (err < 0)
+ return err;
+
+ mod_timer(&pn532->cmd_timeout, HZ / 40 + jiffies);
+ return 0;
+}
+
+static int pn532_uart_send_ack(struct pn533 *dev, gfp_t flags)
+{
+ struct pn532_uart_phy *pn532 = dev->phy;
+ /* spec 7.1.1.3: Preamble, SoPC (2), ACK Code (2), Postamble */
+ static const u8 ack[PN533_STD_FRAME_ACK_SIZE] = {
+ 0x00, 0x00, 0xff, 0x00, 0xff, 0x00};
+ int err;
+
+ err = serdev_device_write(pn532->serdev, ack, sizeof(ack),
+ MAX_SCHEDULE_TIMEOUT);
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
+static void pn532_uart_abort_cmd(struct pn533 *dev, gfp_t flags)
+{
+ /* An ack will cancel the last issued command */
+ pn532_uart_send_ack(dev, flags);
+ /* schedule cmd_complete_work to finish current command execution */
+ pn533_recv_frame(dev, NULL, -ENOENT);
+}
+
+static void pn532_dev_up(struct pn533 *dev)
+{
+ struct pn532_uart_phy *pn532 = dev->phy;
+
+ serdev_device_open(pn532->serdev);
+ pn532->send_wakeup = PN532_SEND_LAST_WAKEUP;
+}
+
+static void pn532_dev_down(struct pn533 *dev)
+{
+ struct pn532_uart_phy *pn532 = dev->phy;
+
+ serdev_device_close(pn532->serdev);
+ pn532->send_wakeup = PN532_SEND_WAKEUP;
+}
+
+static struct pn533_phy_ops uart_phy_ops = {
+ .send_frame = pn532_uart_send_frame,
+ .send_ack = pn532_uart_send_ack,
+ .abort_cmd = pn532_uart_abort_cmd,
+ .dev_up = pn532_dev_up,
+ .dev_down = pn532_dev_down,
+};
+
+static void pn532_cmd_timeout(struct timer_list *t)
+{
+ struct pn532_uart_phy *dev = from_timer(dev, t, cmd_timeout);
+
+ pn532_uart_send_frame(dev->priv, dev->cur_out_buf);
+}
+
+/*
+ * scans the buffer if it contains a pn532 frame. It is not checked if the
+ * frame is really valid. This is later done with pn533_rx_frame_is_valid.
+ * This is useful for malformed or errornous transmitted frames. Adjusts the
+ * bufferposition where the frame starts, since pn533_recv_frame expects a
+ * well formed frame.
+ */
+static int pn532_uart_rx_is_frame(struct sk_buff *skb)
+{
+ int i;
+ u16 frame_len;
+ struct pn533_std_frame *std;
+ struct pn533_ext_frame *ext;
+
+ for (i = 0; i + PN533_STD_FRAME_ACK_SIZE <= skb->len; i++) {
+ std = (struct pn533_std_frame *)&skb->data[i];
+ /* search start code */
+ if (std->start_frame != cpu_to_be16(PN533_STD_FRAME_SOF))
+ continue;
+
+ /* frame type */
+ switch (std->datalen) {
+ case PN533_FRAME_DATALEN_ACK:
+ if (std->datalen_checksum == 0xff) {
+ skb_pull(skb, i);
+ return 1;
+ }
+
+ break;
+ case PN533_FRAME_DATALEN_ERROR:
+ if ((std->datalen_checksum == 0xff) &&
+ (skb->len >=
+ PN533_STD_ERROR_FRAME_SIZE)) {
+ skb_pull(skb, i);
+ return 1;
+ }
+
+ break;
+ case PN533_FRAME_DATALEN_EXTENDED:
+ ext = (struct pn533_ext_frame *)&skb->data[i];
+ frame_len = ext->datalen;
+ if (skb->len >= frame_len +
+ sizeof(struct pn533_ext_frame) +
+ 2 /* CKS + Postamble */) {
+ skb_pull(skb, i);
+ return 1;
+ }
+
+ break;
+ default: /* normal information frame */
+ frame_len = std->datalen;
+ if (skb->len >= frame_len +
+ sizeof(struct pn533_std_frame) +
+ 2 /* CKS + Postamble */) {
+ skb_pull(skb, i);
+ return 1;
+ }
+
+ break;
+ }
+ }
+
+ return 0;
+}
+
+static int pn532_receive_buf(struct serdev_device *serdev,
+ const unsigned char *data, size_t count)
+{
+ struct pn532_uart_phy *dev = serdev_device_get_drvdata(serdev);
+ size_t i;
+
+ del_timer(&dev->cmd_timeout);
+ for (i = 0; i < count; i++) {
+ skb_put_u8(dev->recv_skb, *data++);
+ if (!pn532_uart_rx_is_frame(dev->recv_skb))
+ continue;
+
+ pn533_recv_frame(dev->priv, dev->recv_skb, 0);
+ dev->recv_skb = alloc_skb(PN532_UART_SKB_BUFF_LEN, GFP_KERNEL);
+ if (!dev->recv_skb)
+ return 0;
+ }
+
+ return i;
+}
+
+static struct serdev_device_ops pn532_serdev_ops = {
+ .receive_buf = pn532_receive_buf,
+ .write_wakeup = serdev_device_write_wakeup,
+};
+
+static const struct of_device_id pn532_uart_of_match[] = {
+ { .compatible = "nxp,pn532", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, pn532_uart_of_match);
+
+static int pn532_uart_probe(struct serdev_device *serdev)
+{
+ struct pn532_uart_phy *pn532;
+ struct pn533 *priv;
+ int err;
+
+ err = -ENOMEM;
+ pn532 = kzalloc(sizeof(*pn532), GFP_KERNEL);
+ if (!pn532)
+ goto err_exit;
+
+ pn532->recv_skb = alloc_skb(PN532_UART_SKB_BUFF_LEN, GFP_KERNEL);
+ if (!pn532->recv_skb)
+ goto err_free;
+
+ pn532->serdev = serdev;
+ serdev_device_set_drvdata(serdev, pn532);
+ serdev_device_set_client_ops(serdev, &pn532_serdev_ops);
+ err = serdev_device_open(serdev);
+ if (err) {
+ dev_err(&serdev->dev, "Unable to open device\n");
+ goto err_skb;
+ }
+
+ err = serdev_device_set_baudrate(serdev, 115200);
+ if (err != 115200) {
+ err = -EINVAL;
+ goto err_serdev;
+ }
+
+ serdev_device_set_flow_control(serdev, false);
+ pn532->send_wakeup = PN532_SEND_WAKEUP;
+ timer_setup(&pn532->cmd_timeout, pn532_cmd_timeout, 0);
+ priv = pn53x_common_init(PN533_DEVICE_PN532,
+ PN533_PROTO_REQ_ACK_RESP,
+ pn532, &uart_phy_ops, NULL,
+ &pn532->serdev->dev);
+ if (IS_ERR(priv)) {
+ err = PTR_ERR(priv);
+ goto err_serdev;
+ }
+
+ pn532->priv = priv;
+ err = pn533_finalize_setup(pn532->priv);
+ if (err)
+ goto err_clean;
+
+ serdev_device_close(serdev);
+ err = pn53x_register_nfc(priv, PN533_NO_TYPE_B_PROTOCOLS, &serdev->dev);
+ if (err) {
+ pn53x_common_clean(pn532->priv);
+ goto err_skb;
+ }
+
+ return err;
+
+err_clean:
+ pn53x_common_clean(pn532->priv);
+err_serdev:
+ serdev_device_close(serdev);
+err_skb:
+ kfree_skb(pn532->recv_skb);
+err_free:
+ kfree(pn532);
+err_exit:
+ return err;
+}
+
+static void pn532_uart_remove(struct serdev_device *serdev)
+{
+ struct pn532_uart_phy *pn532 = serdev_device_get_drvdata(serdev);
+
+ pn53x_unregister_nfc(pn532->priv);
+ serdev_device_close(serdev);
+ pn53x_common_clean(pn532->priv);
+ kfree_skb(pn532->recv_skb);
+ kfree(pn532);
+}
+
+static struct serdev_device_driver pn532_uart_driver = {
+ .probe = pn532_uart_probe,
+ .remove = pn532_uart_remove,
+ .driver = {
+ .name = "pn532_uart",
+ .of_match_table = of_match_ptr(pn532_uart_of_match),
+ },
+};
+
+module_serdev_device_driver(pn532_uart_driver);
+
+MODULE_AUTHOR("Lars Pöschel <poeschel@lemonage.de>");
+MODULE_DESCRIPTION("PN532 UART driver");
+MODULE_LICENSE("GPL");
--
2.23.0
^ permalink raw reply related
* [PATCH v7 6/7] nfc: pn533: Add autopoll capability
From: Lars Poeschel @ 2019-09-10 9:34 UTC (permalink / raw)
To: Allison Randal, Kees Cook, Jilayne Lovejoy, Steve Winslow,
Greg Kroah-Hartman, Lars Poeschel, Gustavo A. R. Silva,
Thomas Gleixner, Kate Stewart, open list:NFC SUBSYSTEM, open list
Cc: Johan Hovold
pn532 devices support an autopoll command, that lets the chip
automatically poll for selected nfc technologies instead of manually
looping through every single nfc technology the user is interested in.
This is faster and less cpu and bus intensive than manually polling.
This adds this autopoll capability to the pn533 driver.
Cc: Johan Hovold <johan@kernel.org>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v7:
- Remove __packed attribute at struct pn532_autopoll_resp
- Add missing '\n' at the end of dev_dbg and nfc_err strings
Changes in v6:
- Rebased the patch series on v5.3-rc5
drivers/nfc/pn533/pn533.c | 193 +++++++++++++++++++++++++++++++++++++-
drivers/nfc/pn533/pn533.h | 10 +-
2 files changed, 197 insertions(+), 6 deletions(-)
diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index e5d5e4c83a04..518f3dd5faf7 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -185,6 +185,32 @@ struct pn533_cmd_jump_dep_response {
u8 gt[];
} __packed;
+struct pn532_autopoll_resp {
+ u8 type;
+ u8 ln;
+ u8 tg;
+ u8 tgdata[];
+};
+
+/* PN532_CMD_IN_AUTOPOLL */
+#define PN532_AUTOPOLL_POLLNR_INFINITE 0xff
+#define PN532_AUTOPOLL_PERIOD 0x03 /* in units of 150 ms */
+
+#define PN532_AUTOPOLL_TYPE_GENERIC_106 0x00
+#define PN532_AUTOPOLL_TYPE_GENERIC_212 0x01
+#define PN532_AUTOPOLL_TYPE_GENERIC_424 0x02
+#define PN532_AUTOPOLL_TYPE_JEWEL 0x04
+#define PN532_AUTOPOLL_TYPE_MIFARE 0x10
+#define PN532_AUTOPOLL_TYPE_FELICA212 0x11
+#define PN532_AUTOPOLL_TYPE_FELICA424 0x12
+#define PN532_AUTOPOLL_TYPE_ISOA 0x20
+#define PN532_AUTOPOLL_TYPE_ISOB 0x23
+#define PN532_AUTOPOLL_TYPE_DEP_PASSIVE_106 0x40
+#define PN532_AUTOPOLL_TYPE_DEP_PASSIVE_212 0x41
+#define PN532_AUTOPOLL_TYPE_DEP_PASSIVE_424 0x42
+#define PN532_AUTOPOLL_TYPE_DEP_ACTIVE_106 0x80
+#define PN532_AUTOPOLL_TYPE_DEP_ACTIVE_212 0x81
+#define PN532_AUTOPOLL_TYPE_DEP_ACTIVE_424 0x82
/* PN533_TG_INIT_AS_TARGET */
#define PN533_INIT_TARGET_PASSIVE 0x1
@@ -1389,6 +1415,101 @@ static int pn533_poll_dep(struct nfc_dev *nfc_dev)
return rc;
}
+static int pn533_autopoll_complete(struct pn533 *dev, void *arg,
+ struct sk_buff *resp)
+{
+ u8 nbtg;
+ int rc;
+ struct pn532_autopoll_resp *apr;
+ struct nfc_target nfc_tgt;
+
+ if (IS_ERR(resp)) {
+ rc = PTR_ERR(resp);
+
+ nfc_err(dev->dev, "%s autopoll complete error %d\n",
+ __func__, rc);
+
+ if (rc == -ENOENT) {
+ if (dev->poll_mod_count != 0)
+ return rc;
+ goto stop_poll;
+ } else if (rc < 0) {
+ nfc_err(dev->dev,
+ "Error %d when running autopoll\n", rc);
+ goto stop_poll;
+ }
+ }
+
+ nbtg = resp->data[0];
+ if ((nbtg > 2) || (nbtg <= 0))
+ return -EAGAIN;
+
+ apr = (struct pn532_autopoll_resp *)&resp->data[1];
+ while (nbtg--) {
+ memset(&nfc_tgt, 0, sizeof(struct nfc_target));
+ switch (apr->type) {
+ case PN532_AUTOPOLL_TYPE_ISOA:
+ dev_dbg(dev->dev, "ISOA\n");
+ rc = pn533_target_found_type_a(&nfc_tgt, apr->tgdata,
+ apr->ln - 1);
+ break;
+ case PN532_AUTOPOLL_TYPE_FELICA212:
+ case PN532_AUTOPOLL_TYPE_FELICA424:
+ dev_dbg(dev->dev, "FELICA\n");
+ rc = pn533_target_found_felica(&nfc_tgt, apr->tgdata,
+ apr->ln - 1);
+ break;
+ case PN532_AUTOPOLL_TYPE_JEWEL:
+ dev_dbg(dev->dev, "JEWEL\n");
+ rc = pn533_target_found_jewel(&nfc_tgt, apr->tgdata,
+ apr->ln - 1);
+ break;
+ case PN532_AUTOPOLL_TYPE_ISOB:
+ dev_dbg(dev->dev, "ISOB\n");
+ rc = pn533_target_found_type_b(&nfc_tgt, apr->tgdata,
+ apr->ln - 1);
+ break;
+ case PN532_AUTOPOLL_TYPE_MIFARE:
+ dev_dbg(dev->dev, "Mifare\n");
+ rc = pn533_target_found_type_a(&nfc_tgt, apr->tgdata,
+ apr->ln - 1);
+ break;
+ default:
+ nfc_err(dev->dev,
+ "Unknown current poll modulation\n");
+ rc = -EPROTO;
+ }
+
+ if (rc)
+ goto done;
+
+ if (!(nfc_tgt.supported_protocols & dev->poll_protocols)) {
+ nfc_err(dev->dev,
+ "The Tg found doesn't have the desired protocol\n");
+ rc = -EAGAIN;
+ goto done;
+ }
+
+ dev->tgt_available_prots = nfc_tgt.supported_protocols;
+ apr = (struct pn532_autopoll_resp *)
+ (apr->tgdata + (apr->ln - 1));
+ }
+
+ pn533_poll_reset_mod_list(dev);
+ nfc_targets_found(dev->nfc_dev, &nfc_tgt, 1);
+
+done:
+ dev_kfree_skb(resp);
+ return rc;
+
+stop_poll:
+ nfc_err(dev->dev, "autopoll operation has been stopped\n");
+
+ pn533_poll_reset_mod_list(dev);
+ dev->poll_protocols = 0;
+ return rc;
+}
+
static int pn533_poll_complete(struct pn533 *dev, void *arg,
struct sk_buff *resp)
{
@@ -1534,6 +1655,7 @@ static int pn533_start_poll(struct nfc_dev *nfc_dev,
struct pn533_poll_modulations *cur_mod;
u8 rand_mod;
int rc;
+ struct sk_buff *skb;
dev_dbg(dev->dev,
"%s: im protocols 0x%x tm protocols 0x%x\n",
@@ -1557,9 +1679,73 @@ static int pn533_start_poll(struct nfc_dev *nfc_dev,
tm_protocols = 0;
}
- pn533_poll_create_mod_list(dev, im_protocols, tm_protocols);
dev->poll_protocols = im_protocols;
dev->listen_protocols = tm_protocols;
+ if (dev->device_type == PN533_DEVICE_PN532_AUTOPOLL) {
+ skb = pn533_alloc_skb(dev, 4 + 6);
+ if (!skb)
+ return -ENOMEM;
+
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_POLLNR_INFINITE;
+ *((u8 *)skb_put(skb, sizeof(u8))) = PN532_AUTOPOLL_PERIOD;
+
+ if ((im_protocols & NFC_PROTO_MIFARE_MASK) &&
+ (im_protocols & NFC_PROTO_ISO14443_MASK) &&
+ (im_protocols & NFC_PROTO_NFC_DEP_MASK))
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_GENERIC_106;
+ else {
+ if (im_protocols & NFC_PROTO_MIFARE_MASK)
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_MIFARE;
+
+ if (im_protocols & NFC_PROTO_ISO14443_MASK)
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_ISOA;
+
+ if (im_protocols & NFC_PROTO_NFC_DEP_MASK) {
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_DEP_PASSIVE_106;
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_DEP_PASSIVE_212;
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_DEP_PASSIVE_424;
+ }
+ }
+
+ if (im_protocols & NFC_PROTO_FELICA_MASK ||
+ im_protocols & NFC_PROTO_NFC_DEP_MASK) {
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_FELICA212;
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_FELICA424;
+ }
+
+ if (im_protocols & NFC_PROTO_JEWEL_MASK)
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_JEWEL;
+
+ if (im_protocols & NFC_PROTO_ISO14443_B_MASK)
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_ISOB;
+
+ if (tm_protocols)
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_DEP_ACTIVE_106;
+
+ rc = pn533_send_cmd_async(dev, PN533_CMD_IN_AUTOPOLL, skb,
+ pn533_autopoll_complete, NULL);
+
+ if (rc < 0)
+ dev_kfree_skb(skb);
+ else
+ dev->poll_mod_count++;
+
+ return rc;
+ }
+
+ pn533_poll_create_mod_list(dev, im_protocols, tm_protocols);
/* Do not always start polling from the same modulation */
get_random_bytes(&rand_mod, sizeof(rand_mod));
@@ -2461,7 +2647,8 @@ static int pn533_dev_up(struct nfc_dev *nfc_dev)
if (dev->phy_ops->dev_up)
dev->phy_ops->dev_up(dev);
- if (dev->device_type == PN533_DEVICE_PN532) {
+ if ((dev->device_type == PN533_DEVICE_PN532) ||
+ (dev->device_type == PN533_DEVICE_PN532_AUTOPOLL)) {
int rc = pn532_sam_configuration(nfc_dev);
if (rc)
@@ -2508,6 +2695,7 @@ static int pn533_setup(struct pn533 *dev)
case PN533_DEVICE_PASORI:
case PN533_DEVICE_ACR122U:
case PN533_DEVICE_PN532:
+ case PN533_DEVICE_PN532_AUTOPOLL:
max_retries.mx_rty_atr = 0x2;
max_retries.mx_rty_psl = 0x1;
max_retries.mx_rty_passive_act =
@@ -2544,6 +2732,7 @@ static int pn533_setup(struct pn533 *dev)
switch (dev->device_type) {
case PN533_DEVICE_STD:
case PN533_DEVICE_PN532:
+ case PN533_DEVICE_PN532_AUTOPOLL:
break;
case PN533_DEVICE_PASORI:
diff --git a/drivers/nfc/pn533/pn533.h b/drivers/nfc/pn533/pn533.h
index 6541088fad73..388fc1b4fcc1 100644
--- a/drivers/nfc/pn533/pn533.h
+++ b/drivers/nfc/pn533/pn533.h
@@ -6,10 +6,11 @@
* Copyright (C) 2012-2013 Tieto Poland
*/
-#define PN533_DEVICE_STD 0x1
-#define PN533_DEVICE_PASORI 0x2
-#define PN533_DEVICE_ACR122U 0x3
-#define PN533_DEVICE_PN532 0x4
+#define PN533_DEVICE_STD 0x1
+#define PN533_DEVICE_PASORI 0x2
+#define PN533_DEVICE_ACR122U 0x3
+#define PN533_DEVICE_PN532 0x4
+#define PN533_DEVICE_PN532_AUTOPOLL 0x5
#define PN533_ALL_PROTOCOLS (NFC_PROTO_JEWEL_MASK | NFC_PROTO_MIFARE_MASK |\
NFC_PROTO_FELICA_MASK | NFC_PROTO_ISO14443_MASK |\
@@ -75,6 +76,7 @@
#define PN533_CMD_IN_ATR 0x50
#define PN533_CMD_IN_RELEASE 0x52
#define PN533_CMD_IN_JUMP_FOR_DEP 0x56
+#define PN533_CMD_IN_AUTOPOLL 0x60
#define PN533_CMD_TG_INIT_AS_TARGET 0x8c
#define PN533_CMD_TG_GET_DATA 0x86
--
2.23.0
^ permalink raw reply related
* [PATCH v7 7/7] nfc: pn532_uart: Make use of pn532 autopoll
From: Lars Poeschel @ 2019-09-10 9:34 UTC (permalink / raw)
To: GitAuthor: Lars Poeschel, open list:NFC SUBSYSTEM, open list; +Cc: Johan Hovold
This switches the pn532 UART phy driver from manually polling to the new
autopoll mechanism.
Cc: Johan Hovold <johan@kernel.org>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v6:
- Rebased the patch series on v5.3-rc5
drivers/nfc/pn533/uart.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nfc/pn533/uart.c b/drivers/nfc/pn533/uart.c
index cce8f29eda8b..deb553017d82 100644
--- a/drivers/nfc/pn533/uart.c
+++ b/drivers/nfc/pn533/uart.c
@@ -262,7 +262,7 @@ static int pn532_uart_probe(struct serdev_device *serdev)
serdev_device_set_flow_control(serdev, false);
pn532->send_wakeup = PN532_SEND_WAKEUP;
timer_setup(&pn532->cmd_timeout, pn532_cmd_timeout, 0);
- priv = pn53x_common_init(PN533_DEVICE_PN532,
+ priv = pn53x_common_init(PN533_DEVICE_PN532_AUTOPOLL,
PN533_PROTO_REQ_ACK_RESP,
pn532, &uart_phy_ops, NULL,
&pn532->serdev->dev);
--
2.23.0
^ permalink raw reply related
* [PATCH] net/mlx5: Declare 'rt' as corresponding enum type
From: Austin Kim @ 2019-09-10 9:27 UTC (permalink / raw)
To: saeedm, leon
Cc: davem, valex, erezsh, markb, netdev, linux-rdma, linux-kernel,
austindh.kim
When building kernel with clang, we can observe below warning message:
drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c:1080:9:
warning: implicit conversion from enumeration type 'enum mlx5_reformat_ctx_type'
to different enumeration type 'enum mlx5dr_action_type' [- Wenum-conversion]
rt = MLX5_REFORMAT_TYPE_L2_TO_L2_TUNNEL;
~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c:1082:9:
warning: implicit conversion from enumeration type 'enum mlx5_reformat_ctx_type'
to different enumeration type 'enum mlx5dr_action_type' [- Wenum-conversion]
rt = MLX5_REFORMAT_TYPE_L2_TO_L3_TUNNEL;
~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c:1084:51:
warning: implicit conversion from enumeration type 'enum mlx5dr_action_type'
to different enumeration type 'enum mlx5_reformat_ctx_type' [- Wenum-conversion]
ret = mlx5dr_cmd_create_reformat_ctx(dmn->mdev, rt, data_sz, data,
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~
Declare 'rt' as corresponding enum mlx5_reformat_ctx_type type.
Signed-off-by: Austin Kim <austindh.kim@gmail.com>
---
drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c
index a02f87f..7d81a77 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c
@@ -1074,7 +1074,7 @@ dr_action_create_reformat_action(struct mlx5dr_domain *dmn,
case DR_ACTION_TYP_L2_TO_TNL_L2:
case DR_ACTION_TYP_L2_TO_TNL_L3:
{
- enum mlx5dr_action_type rt;
+ enum mlx5_reformat_ctx_type rt;
if (action->action_type == DR_ACTION_TYP_L2_TO_TNL_L2)
rt = MLX5_REFORMAT_TYPE_L2_TO_L2_TUNNEL;
--
2.6.2
^ permalink raw reply related
* Re: ❌ FAIL: Stable queue: queue-5.2
From: Hangbin Liu @ 2019-09-10 9:30 UTC (permalink / raw)
To: Greg KH
Cc: CKI Project, Linux Stable maillist, netdev, Jan Stancek,
Xiumei Mu, David Howells, linux-afs, Sasha Levin
In-Reply-To: <20190910085810.GA3593@kroah.com>
On Tue, Sep 10, 2019 at 09:58:10AM +0100, Greg KH wrote:
> On Tue, Sep 10, 2019 at 04:19:56PM +0800, Hangbin Liu wrote:
> > On Wed, Aug 28, 2019 at 08:36:14AM -0400, CKI Project wrote:
> > >
> > > Hello,
> > >
> > > We ran automated tests on a patchset that was proposed for merging into this
> > > kernel tree. The patches were applied to:
> > >
> > > Kernel repo: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
> > > Commit: f7d5b3dc4792 - Linux 5.2.10
> > >
> > > The results of these automated tests are provided below.
> > >
> > > Overall result: FAILED (see details below)
> > > Merge: OK
> > > Compile: OK
> > > Tests: FAILED
> > >
> > > All kernel binaries, config files, and logs are available for download here:
> > >
> > > https://artifacts.cki-project.org/pipelines/128519
> > >
> > >
> > >
> > > One or more kernel tests failed:
> > >
> > > x86_64:
> > > ❌ Networking socket: fuzz
> >
> > Sorry, maybe the info is a little late, I just found the call traces for this
> > failure.
>
> And this is no longer failing?
I haven't seen this issue later. But you know, this was triggered by a fuzz test,
not sure if the bad code still exists.
>
> What is the "fuzz" test?
It's just a socket test that create all kinds of domains/types/protocols and do
some {set,get}sockopt for TCP/UDP/SCTP
https://github.com/CKI-project/tests-beaker/blob/master/networking/socket/fuzz/socket.c#L155
Xiumei Mu also forwarded me a mail. It looks Sasha has fixed something.
But I don't know the details.
----- Forwarded Message -----
> From: "Sasha Levin" <sashal@kernel.org>
> To: "Greg KH" <greg@kroah.com>
> Cc: "Major Hayden" <major@mhtx.net>, "CKI Project" <cki-project@redhat.com>, "Linux Stable maillist"
> <stable@vger.kernel.org>, "Yi Zhang" <yi.zhang@redhat.com>, "Xiumei Mu" <xmu@redhat.com>, "Hangbin Liu"
> <haliu@redhat.com>, "Ying Xu" <yinxu@redhat.com>
> Sent: Wednesday, August 28, 2019 2:25:36 AM
> Subject: Re: ❌ FAIL: Test report for kernel 5.2.11-rc1-9f63171.cki (stable)
>
> On Tue, Aug 27, 2019 at 07:05:18PM +0200, Greg KH wrote:
> >On Tue, Aug 27, 2019 at 09:35:30AM -0500, Major Hayden wrote:
> >> On 8/27/19 7:31 AM, CKI Project wrote:
> >> > x86_64:
> >> > Host 2:
> >> > ❌ Networking socket: fuzz [9]
> >> > ❌ Networking sctp-auth: sockopts test [10]
> >>
> >> It looks like there was an oops when these tests ran on 5.2.11-rc1 and the
> >> last set of patches in stable-queue:
> >
> >Can you bisect?
>
> I think I've fixed it, let's see what happens next run.
>
> --
> Thanks,
> Sasha
^ permalink raw reply
* Re: ❌ FAIL: Stable queue: queue-5.2
From: Hangbin Liu @ 2019-09-10 9:40 UTC (permalink / raw)
To: Greg KH
Cc: CKI Project, Linux Stable maillist, netdev, Jan Stancek,
Xiumei Mu, David Howells, linux-afs, Sasha Levin
In-Reply-To: <20190910093021.GK22496@dhcp-12-139.nay.redhat.com>
On Tue, Sep 10, 2019 at 05:30:21PM +0800, Hangbin Liu wrote:
> Xiumei Mu also forwarded me a mail. It looks Sasha has fixed something.
> But I don't know the details.
Oh, I checked that thread. It's the same issue. So Sasha should has fixed it. I
just wonder the commit id now.
Thanks
Hangbin
>
> ----- Forwarded Message -----
> > From: "Sasha Levin" <sashal@kernel.org>
> > To: "Greg KH" <greg@kroah.com>
> > Cc: "Major Hayden" <major@mhtx.net>, "CKI Project" <cki-project@redhat.com>, "Linux Stable maillist"
> > <stable@vger.kernel.org>, "Yi Zhang" <yi.zhang@redhat.com>, "Xiumei Mu" <xmu@redhat.com>, "Hangbin Liu"
> > <haliu@redhat.com>, "Ying Xu" <yinxu@redhat.com>
> > Sent: Wednesday, August 28, 2019 2:25:36 AM
> > Subject: Re: ❌ FAIL: Test report for kernel 5.2.11-rc1-9f63171.cki (stable)
> >
> > On Tue, Aug 27, 2019 at 07:05:18PM +0200, Greg KH wrote:
> > >On Tue, Aug 27, 2019 at 09:35:30AM -0500, Major Hayden wrote:
> > >> On 8/27/19 7:31 AM, CKI Project wrote:
> > >> > x86_64:
> > >> > Host 2:
> > >> > ❌ Networking socket: fuzz [9]
> > >> > ❌ Networking sctp-auth: sockopts test [10]
> > >>
> > >> It looks like there was an oops when these tests ran on 5.2.11-rc1 and the
> > >> last set of patches in stable-queue:
> > >
> > >Can you bisect?
> >
> > I think I've fixed it, let's see what happens next run.
> >
> > --
> > Thanks,
> > Sasha
^ permalink raw reply
* Re: [RFC PATCH 3/4] virtio: introudce a mdev based transport
From: Michael S. Tsirkin @ 2019-09-10 10:01 UTC (permalink / raw)
To: Jason Wang
Cc: kvm, virtualization, netdev, linux-kernel, kwankhede,
alex.williamson, cohuck, tiwei.bie, maxime.coquelin,
cunming.liang, zhihong.wang, rob.miller, idos, xiao.w.wang,
haotian.wang
In-Reply-To: <20190910081935.30516-4-jasowang@redhat.com>
On Tue, Sep 10, 2019 at 04:19:34PM +0800, Jason Wang wrote:
> This path introduces a new mdev transport for virtio. This is used to
> use kernel virtio driver to drive the mediated device that is capable
> of populating virtqueue directly.
>
> A new virtio-mdev driver will be registered to the mdev bus, when a
> new virtio-mdev device is probed, it will register the device with
> mdev based config ops. This means, unlike the exist hardware
> transport, this is a software transport between mdev driver and mdev
> device. The transport was implemented through:
>
> - configuration access was implemented through parent_ops->read()/write()
> - vq/config callback was implemented through parent_ops->ioctl()
>
> This transport is derived from virtio MMIO protocol and was wrote for
> kernel driver. But for the transport itself, but the design goal is to
> be generic enough to support userspace driver (this part will be added
> in the future).
>
> Note:
> - current mdev assume all the parameter of parent_ops was from
> userspace. This prevents us from implementing the kernel mdev
> driver. For a quick POC, this patch just abuse those parameter and
> assume the mdev device implementation will treat them as kernel
> pointer. This should be addressed in the formal series by extending
> mdev_parent_ops.
> - for a quick POC, I just drive the transport from MMIO, I'm pretty
> there's lot of optimization space for this.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/vfio/mdev/Kconfig | 7 +
> drivers/vfio/mdev/Makefile | 1 +
> drivers/vfio/mdev/virtio_mdev.c | 500 +++++++++++++++++++++++++++++++
> include/uapi/linux/virtio_mdev.h | 131 ++++++++
> 4 files changed, 639 insertions(+)
> create mode 100644 drivers/vfio/mdev/virtio_mdev.c
> create mode 100644 include/uapi/linux/virtio_mdev.h
>
> diff --git a/drivers/vfio/mdev/Kconfig b/drivers/vfio/mdev/Kconfig
> index 5da27f2100f9..c488c31fc137 100644
> --- a/drivers/vfio/mdev/Kconfig
> +++ b/drivers/vfio/mdev/Kconfig
> @@ -16,3 +16,10 @@ config VFIO_MDEV_DEVICE
> default n
> help
> VFIO based driver for Mediated devices.
> +
> +config VIRTIO_MDEV_DEVICE
> + tristate "VIRTIO driver for Mediated devices"
> + depends on VFIO_MDEV && VIRTIO
> + default n
> + help
> + VIRTIO based driver for Mediated devices.
> diff --git a/drivers/vfio/mdev/Makefile b/drivers/vfio/mdev/Makefile
> index 101516fdf375..99d31e29c23e 100644
> --- a/drivers/vfio/mdev/Makefile
> +++ b/drivers/vfio/mdev/Makefile
> @@ -4,3 +4,4 @@ mdev-y := mdev_core.o mdev_sysfs.o mdev_driver.o
>
> obj-$(CONFIG_VFIO_MDEV) += mdev.o
> obj-$(CONFIG_VFIO_MDEV_DEVICE) += vfio_mdev.o
> +obj-$(CONFIG_VIRTIO_MDEV_DEVICE) += virtio_mdev.o
> diff --git a/drivers/vfio/mdev/virtio_mdev.c b/drivers/vfio/mdev/virtio_mdev.c
> new file mode 100644
> index 000000000000..5ff09089297e
> --- /dev/null
> +++ b/drivers/vfio/mdev/virtio_mdev.c
> @@ -0,0 +1,500 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * VIRTIO based driver for Mediated device
> + *
> + * Copyright (c) 2019, Red Hat. All rights reserved.
> + * Author: Jason Wang <jasowang@redhat.com>
> + *
> + * Based on Virtio MMIO driver.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/uuid.h>
> +#include <linux/mdev.h>
> +#include <linux/virtio.h>
> +#include <linux/virtio_config.h>
> +#include <linux/virtio_ring.h>
> +#include <uapi/linux/virtio_mdev.h>
> +#include "mdev_private.h"
> +
> +#define DRIVER_VERSION "0.1"
> +#define DRIVER_AUTHOR "Red Hat Corporation"
> +#define DRIVER_DESC "VIRTIO based driver for Mediated device"
> +
> +#define to_virtio_mdev_device(dev) \
> + container_of(dev, struct virtio_mdev_device, vdev)
> +
> +struct virtio_mdev_device {
> + struct virtio_device vdev;
> + struct mdev_device *mdev;
> + unsigned long version;
> +
> + struct virtqueue **vqs;
> + spinlock_t lock;
> +};
> +
> +struct virtio_mdev_vq_info {
> + /* the actual virtqueue */
> + struct virtqueue *vq;
> +
> + /* the list node for the virtqueues list */
> + struct list_head node;
> +};
> +
> +static u32 virtio_mdev_readl(struct mdev_device *mdev,
> + loff_t off)
> +{
> + struct mdev_parent *parent = mdev->parent;
> + ssize_t len;
> + u32 val;
> +
> + if (unlikely(!parent->ops->read))
> + return 0xFFFFFFFF;
> +
> + len = parent->ops->read(mdev, (char *)&val, 4, &off);
> + if (len != 4)
> + return 0xFFFFFFFF;
> +
> + return val;
> +}
> +
> +static void virtio_mdev_writel(struct mdev_device *mdev,
> + loff_t off, u32 val)
> +{
> + struct mdev_parent *parent = mdev->parent;
> +
> + if (unlikely(!parent->ops->write))
> + return;
> +
> + parent->ops->write(mdev, (char *)&val, 4, &off);
> +
> + return;
> +}
> +
> +static void virtio_mdev_get(struct virtio_device *vdev, unsigned offset,
> + void *buf, unsigned len)
> +{
> + struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev);
> + struct mdev_device *mdev = vm_dev->mdev;
> + struct mdev_parent *parent = mdev->parent;
> +
> + loff_t off = offset + VIRTIO_MDEV_CONFIG;
> +
> + switch (len) {
> + case 1:
> + *(u8 *)buf = parent->ops->read(mdev, buf, 1, &off);
> + break;
> + case 2:
> + *(u16 *)buf = parent->ops->read(mdev, buf, 2, &off);
> + break;
> + case 4:
> + *(u32 *)buf = parent->ops->read(mdev, buf, 4, &off);
> + break;
> + case 8:
> + *(u32 *)buf = parent->ops->read(mdev, buf, 4, &off);
> + *((u32 *)buf + 1) = parent->ops->read(mdev, buf, 4, &off);
> + break;
> + default:
> + BUG();
> + }
> +
> + return;
> +}
> +
> +static void virtio_mdev_set(struct virtio_device *vdev, unsigned offset,
> + const void *buf, unsigned len)
> +{
> + struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev);
> + struct mdev_device *mdev = vm_dev->mdev;
> + struct mdev_parent *parent = mdev->parent;
> + loff_t off = offset + VIRTIO_MDEV_CONFIG;
> +
> + switch (len) {
> + case 1:
> + case 2:
> + case 4:
> + case 8:
> + break;
> + default:
> + BUG();
> + }
> +
> + parent->ops->write(mdev, buf, len, &off);
> +
> + return;
> +}
> +
> +static u32 virtio_mdev_generation(struct virtio_device *vdev)
> +{
> + struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev);
> +
> + if (vm_dev->version == 1)
> + return 0;
> + else
> + return virtio_mdev_readl(vm_dev->mdev,
> + VIRTIO_MDEV_CONFIG_GENERATION);
> +}
> +
> +static u8 virtio_mdev_get_status(struct virtio_device *vdev)
> +{
> + struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev);
> +
> + return virtio_mdev_readl(vm_dev->mdev, VIRTIO_MDEV_STATUS) & 0xff;
> +}
> +
> +static void virtio_mdev_set_status(struct virtio_device *vdev, u8 status)
> +{
> + struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev);
> +
> + virtio_mdev_writel(vm_dev->mdev, VIRTIO_MDEV_STATUS, status);
> +}
> +
> +static void virtio_mdev_reset(struct virtio_device *vdev)
> +{
> + struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev);
> +
> + virtio_mdev_writel(vm_dev->mdev, VIRTIO_MDEV_STATUS, 0);
> +}
> +
> +static bool virtio_mdev_notify(struct virtqueue *vq)
> +{
> + struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vq->vdev);
> +
> + /* We write the queue's selector into the notification register to
> + * signal the other end */
> + virtio_mdev_writel(vm_dev->mdev, VIRTIO_MDEV_QUEUE_NOTIFY,
> + vq->index);
> + return true;
> +}
> +
> +static irqreturn_t virtio_mdev_config_cb(void *private)
> +{
> + struct virtio_mdev_device *vm_dev = private;
> +
> + virtio_config_changed(&vm_dev->vdev);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t virtio_mdev_virtqueue_cb(void *private)
> +{
> + struct virtio_mdev_vq_info *info = private;
> +
> + return vring_interrupt(0, info->vq);
> +}
> +
> +static struct virtqueue *
> +virtio_mdev_setup_vq(struct virtio_device *vdev, unsigned index,
> + void (*callback)(struct virtqueue *vq),
> + const char *name, bool ctx)
> +{
> + struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev);
> + struct mdev_device *mdev= vm_dev->mdev;
> + struct mdev_parent *parent = mdev->parent;
> + struct virtio_mdev_vq_info *info;
> + struct virtio_mdev_callback cb;
> + struct virtqueue *vq;
> + unsigned long flags;
> + u32 align, num;
> + u64 addr;
> + int err;
> +
> + if (!name)
> + return NULL;
> +
> + /* Select the queue we're interested in */
> + virtio_mdev_writel(vm_dev->mdev, VIRTIO_MDEV_QUEUE_SEL, index);
> +
> + /* Queue shouldn't already be set up. */
> + if (virtio_mdev_readl(vm_dev->mdev, VIRTIO_MDEV_QUEUE_READY)) {
> + err = -ENOENT;
> + goto error_available;
> + }
> +
> + /* Allocate and fill out our active queue description */
> + info = kmalloc(sizeof(*info), GFP_KERNEL);
> + if (!info) {
> + err = -ENOMEM;
> + goto error_kmalloc;
> + }
> +
> + num = virtio_mdev_readl(vm_dev->mdev, VIRTIO_MDEV_QUEUE_NUM_MAX);
> + if (num == 0) {
> + err = -ENOENT;
> + goto error_new_virtqueue;
> + }
> +
> + /* Create the vring */
> + align = virtio_mdev_readl(vm_dev->mdev, VIRTIO_MDEV_QUEUE_ALIGN);
> + vq = vring_create_virtqueue(index, num, align, vdev,
> + true, true, ctx,
> + virtio_mdev_notify, callback, name);
> + if (!vq) {
> + err = -ENOMEM;
> + goto error_new_virtqueue;
> + }
> +
> + /* Setup virtqueue callback */
> + cb.callback = virtio_mdev_virtqueue_cb;
> + cb.private = info;
> + err = parent->ops->ioctl(mdev, VIRTIO_MDEV_SET_VQ_CALLBACK,
> + (unsigned long)&cb);
> + if (err) {
> + err = -EINVAL;
> + goto error_callback;
> + }
> +
> + virtio_mdev_writel(vm_dev->mdev, VIRTIO_MDEV_QUEUE_NUM,
> + virtqueue_get_vring_size(vq));
> + addr = virtqueue_get_desc_addr(vq);
> + virtio_mdev_writel(vm_dev->mdev, VIRTIO_MDEV_QUEUE_DESC_LOW, (u32)addr);
> + virtio_mdev_writel(vm_dev->mdev, VIRTIO_MDEV_QUEUE_DESC_HIGH,
> + (u32)(addr >> 32));
> +
> + addr = virtqueue_get_avail_addr(vq);
> + virtio_mdev_writel(vm_dev->mdev, VIRTIO_MDEV_QUEUE_AVAIL_LOW, (u32)addr);
> + virtio_mdev_writel(vm_dev->mdev, VIRTIO_MDEV_QUEUE_AVAIL_HIGH,
> + (u32)(addr >> 32));
> +
> + addr = virtqueue_get_used_addr(vq);
> + virtio_mdev_writel(vm_dev->mdev, VIRTIO_MDEV_QUEUE_USED_LOW, (u32)addr);
> + virtio_mdev_writel(vm_dev->mdev, VIRTIO_MDEV_QUEUE_USED_HIGH, (u32)(addr >> 32));
> +
> + virtio_mdev_writel(vm_dev->mdev, VIRTIO_MDEV_QUEUE_READY, 1);
> +
> + vq->priv = info;
> + info->vq = vq;
> +
> + return vq;
> +
> +error_callback:
> + vring_del_virtqueue(vq);
> +error_new_virtqueue:
> + virtio_mdev_writel(vm_dev->mdev, VIRTIO_MDEV_QUEUE_READY, 0);
> + WARN_ON(virtio_mdev_readl(vm_dev->mdev, VIRTIO_MDEV_QUEUE_READY));
> + kfree(info);
> +error_kmalloc:
> +error_available:
> + return ERR_PTR(err);
> +
> +}
> +
> +static void virtio_mdev_del_vq(struct virtqueue *vq)
> +{
> + struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vq->vdev);
> + struct virtio_mdev_vq_info *info = vq->priv;
> + unsigned long flags;
> + unsigned int index = vq->index;
> +
> + /* Select and deactivate the queue */
> + virtio_mdev_writel(vm_dev->mdev, VIRTIO_MDEV_QUEUE_SEL, index);
> + virtio_mdev_writel(vm_dev->mdev,VIRTIO_MDEV_QUEUE_READY, 0);
> + WARN_ON(virtio_mdev_readl(vm_dev->mdev, VIRTIO_MDEV_QUEUE_READY));
> +
> + vring_del_virtqueue(vq);
> +
> + kfree(info);
> +}
> +
> +static void virtio_mdev_del_vqs(struct virtio_device *vdev)
> +{
> + struct virtqueue *vq, *n;
> +
> + list_for_each_entry_safe(vq, n, &vdev->vqs, list)
> + virtio_mdev_del_vq(vq);
> +
> + return;
> +}
> +
> +static int virtio_mdev_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> + struct virtqueue *vqs[],
> + vq_callback_t *callbacks[],
> + const char * const names[],
> + const bool *ctx,
> + struct irq_affinity *desc)
> +{
> + struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev);
> + struct mdev_device *mdev = vm_dev->mdev;
> + struct mdev_parent *parent = mdev->parent;
> + struct virtio_mdev_callback cb;
> + int i, err, queue_idx = 0;
> + vm_dev->vqs = kmalloc_array(queue_idx, sizeof(*vm_dev->vqs),
> + GFP_KERNEL);
> + if (!vm_dev->vqs)
> + return -ENOMEM;
> +
> + for (i = 0; i < nvqs; ++i) {
> + if (!names[i]) {
> + vqs[i] = NULL;
> + continue;
> + }
> +
> + vqs[i] = virtio_mdev_setup_vq(vdev, queue_idx++,
> + callbacks[i], names[i], ctx ?
> + ctx[i] : false);
> + if (IS_ERR(vqs[i])) {
> + err = PTR_ERR(vqs[i]);
> + goto err_setup_vq;
> + }
> + }
> +
> + cb.callback = virtio_mdev_config_cb;
> + cb.private = vm_dev;
> + err = parent->ops->ioctl(mdev, VIRTIO_MDEV_SET_CONFIG_CALLBACK,
> + (unsigned long)&cb);
> + if (err)
> + goto err_setup_vq;
> +
> + return 0;
> +
> +err_setup_vq:
> + kfree(vm_dev->vqs);
> + virtio_mdev_del_vqs(vdev);
> + return err;
> +}
> +
> +static u64 virtio_mdev_get_features(struct virtio_device *vdev)
> +{
> + struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev);
> + u64 features;
> +
> + virtio_mdev_writel(vm_dev->mdev, VIRTIO_MDEV_DEVICE_FEATURES_SEL, 1);
> + features = virtio_mdev_readl(vm_dev->mdev, VIRTIO_MDEV_DEVICE_FEATURES);
> + features <<= 32;
> +
> + virtio_mdev_writel(vm_dev->mdev, VIRTIO_MDEV_DEVICE_FEATURES_SEL, 0);
> + features |= virtio_mdev_readl(vm_dev->mdev, VIRTIO_MDEV_DEVICE_FEATURES);
> +
> + return features;
> +}
> +
> +static int virtio_mdev_finalize_features(struct virtio_device *vdev)
> +{
> + struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev);
> +
> + /* Give virtio_ring a chance to accept features. */
> + vring_transport_features(vdev);
> +
> + virtio_mdev_writel(vm_dev->mdev, VIRTIO_MDEV_DRIVER_FEATURES_SEL, 1);
> + virtio_mdev_writel(vm_dev->mdev, VIRTIO_MDEV_DRIVER_FEATURES,
> + (u32)(vdev->features >> 32));
> +
> + virtio_mdev_writel(vm_dev->mdev, VIRTIO_MDEV_DRIVER_FEATURES_SEL, 0);
> + virtio_mdev_writel(vm_dev->mdev, VIRTIO_MDEV_DRIVER_FEATURES,
> + (u32)vdev->features);
> +
> + return 0;
> +}
> +
> +static const char *virtio_mdev_bus_name(struct virtio_device *vdev)
> +{
> + struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev);
> + struct mdev_device *mdev = vm_dev->mdev;
> +
> + return dev_name(&mdev->dev);
> +}
> +
> +static const struct virtio_config_ops virtio_mdev_config_ops = {
> + .get = virtio_mdev_get,
> + .set = virtio_mdev_set,
> + .generation = virtio_mdev_generation,
> + .get_status = virtio_mdev_get_status,
> + .set_status = virtio_mdev_set_status,
> + .reset = virtio_mdev_reset,
> + .find_vqs = virtio_mdev_find_vqs,
> + .del_vqs = virtio_mdev_del_vqs,
> + .get_features = virtio_mdev_get_features,
> + .finalize_features = virtio_mdev_finalize_features,
> + .bus_name = virtio_mdev_bus_name,
> +};
> +
> +static void virtio_mdev_release_dev(struct device *_d)
> +{
> + struct virtio_device *vdev =
> + container_of(_d, struct virtio_device, dev);
> + struct virtio_mdev_device *vm_dev =
> + container_of(vdev, struct virtio_mdev_device, vdev);
> +
> + devm_kfree(_d, vm_dev);
> +}
> +
> +static int virtio_mdev_probe(struct device *dev)
> +{
> + struct mdev_device *mdev = to_mdev_device(dev);
> + struct virtio_mdev_device *vm_dev;
> + unsigned long magic;
> + int rc;
> +
> + magic = virtio_mdev_readl(mdev, VIRTIO_MDEV_MAGIC_VALUE);
> + if (magic != ('v' | 'i' << 8 | 'r' << 16 | 't' << 24)) {
> + dev_warn(dev, "Wrong magic value 0x%08lx!\n", magic);
> + return -ENODEV;
> + }
> +
> + vm_dev = devm_kzalloc(dev, sizeof(*vm_dev), GFP_KERNEL);
> + if (!vm_dev)
> + return -ENOMEM;
> +
> + vm_dev->vdev.dev.parent = dev;
> + vm_dev->vdev.dev.release = virtio_mdev_release_dev;
> + vm_dev->vdev.config = &virtio_mdev_config_ops;
> + vm_dev->mdev = mdev;
> + vm_dev->vqs = NULL;
> + spin_lock_init(&vm_dev->lock);
> +
> + vm_dev->version = virtio_mdev_readl(mdev, VIRTIO_MDEV_VERSION);
> + if (vm_dev->version != 1) {
> + dev_err(dev, "Version %ld not supported!\n",
> + vm_dev->version);
> + return -ENXIO;
> + }
> +
> + vm_dev->vdev.id.device = virtio_mdev_readl(mdev, VIRTIO_MDEV_DEVICE_ID);
> + if (vm_dev->vdev.id.device == 0)
> + return -ENODEV;
> +
> + vm_dev->vdev.id.vendor = virtio_mdev_readl(mdev, VIRTIO_MDEV_VENDOR_ID);
> + rc = register_virtio_device(&vm_dev->vdev);
> + if (rc)
> + put_device(dev);
> +
> + dev_set_drvdata(dev, vm_dev);
> +
> + return rc;
> +
> +}
> +
> +static void virtio_mdev_remove(struct device *dev)
> +{
> + struct virtio_mdev_device *vm_dev = dev_get_drvdata(dev);
> +
> + unregister_virtio_device(&vm_dev->vdev);
> +}
> +
> +static struct mdev_driver virtio_mdev_driver = {
> + .name = "virtio_mdev",
> + .probe = virtio_mdev_probe,
> + .remove = virtio_mdev_remove,
> +};
> +
> +static int __init virtio_mdev_init(void)
> +{
> + return mdev_register_driver(&virtio_mdev_driver, THIS_MODULE);
> +}
> +
> +static void __exit virtio_mdev_exit(void)
> +{
> + mdev_unregister_driver(&virtio_mdev_driver);
> +}
> +
> +module_init(virtio_mdev_init)
> +module_exit(virtio_mdev_exit)
> +
> +MODULE_VERSION(DRIVER_VERSION);
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR(DRIVER_AUTHOR);
> +MODULE_DESCRIPTION(DRIVER_DESC);
> diff --git a/include/uapi/linux/virtio_mdev.h b/include/uapi/linux/virtio_mdev.h
> new file mode 100644
> index 000000000000..8040de6b960a
> --- /dev/null
> +++ b/include/uapi/linux/virtio_mdev.h
> @@ -0,0 +1,131 @@
> +/*
> + * Virtio mediated device driver
> + *
> + * Copyright 2019, Red Hat Corp.
> + *
> + * Based on Virtio MMIO driver by ARM Ltd, copyright ARM Ltd. 2011
> + *
> + * This header is BSD licensed so anyone can use the definitions to implement
> + * compatible drivers/servers.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. Neither the name of IBM nor the names of its contributors
> + * may be used to endorse or promote products derived from this software
> + * without specific prior written permission.
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL IBM OR CONTRIBUTORS BE LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */
> +#ifndef _LINUX_VIRTIO_MDEV_H
> +#define _LINUX_VIRTIO_MDEV_H
> +
> +#include <linux/interrupt.h>
> +#include <linux/vringh.h>
> +#include <uapi/linux/virtio_net.h>
> +
> +/*
> + * Ioctls
> + */
Pls add a bit more content here. It's redundant to state these
are ioctls. Much better to document what does each one do.
> +
> +struct virtio_mdev_callback {
> + irqreturn_t (*callback)(void *);
> + void *private;
> +};
> +
> +#define VIRTIO_MDEV 0xAF
> +#define VIRTIO_MDEV_SET_VQ_CALLBACK _IOW(VIRTIO_MDEV, 0x00, \
> + struct virtio_mdev_callback)
> +#define VIRTIO_MDEV_SET_CONFIG_CALLBACK _IOW(VIRTIO_MDEV, 0x01, \
> + struct virtio_mdev_callback)
Function pointer in an ioctl parameter? How does this ever make sense?
And can't we use a couple of registers for this, and avoid ioctls?
> +
> +#define VIRTIO_MDEV_DEVICE_API_STRING "virtio-mdev"
> +
> +/*
> + * Control registers
> + */
> +
> +/* Magic value ("virt" string) - Read Only */
> +#define VIRTIO_MDEV_MAGIC_VALUE 0x000
> +
> +/* Virtio device version - Read Only */
> +#define VIRTIO_MDEV_VERSION 0x004
> +
> +/* Virtio device ID - Read Only */
> +#define VIRTIO_MDEV_DEVICE_ID 0x008
> +
> +/* Virtio vendor ID - Read Only */
> +#define VIRTIO_MDEV_VENDOR_ID 0x00c
> +
> +/* Bitmask of the features supported by the device (host)
> + * (32 bits per set) - Read Only */
> +#define VIRTIO_MDEV_DEVICE_FEATURES 0x010
> +
> +/* Device (host) features set selector - Write Only */
> +#define VIRTIO_MDEV_DEVICE_FEATURES_SEL 0x014
> +
> +/* Bitmask of features activated by the driver (guest)
> + * (32 bits per set) - Write Only */
> +#define VIRTIO_MDEV_DRIVER_FEATURES 0x020
> +
> +/* Activated features set selector - Write Only */
> +#define VIRTIO_MDEV_DRIVER_FEATURES_SEL 0x024
> +
> +/* Queue selector - Write Only */
> +#define VIRTIO_MDEV_QUEUE_SEL 0x030
> +
> +/* Maximum size of the currently selected queue - Read Only */
> +#define VIRTIO_MDEV_QUEUE_NUM_MAX 0x034
> +
> +/* Queue size for the currently selected queue - Write Only */
> +#define VIRTIO_MDEV_QUEUE_NUM 0x038
> +
> +/* Ready bit for the currently selected queue - Read Write */
> +#define VIRTIO_MDEV_QUEUE_READY 0x044
Is this same as started?
> +
> +/* Alignment of virtqueue - Read Only */
> +#define VIRTIO_MDEV_QUEUE_ALIGN 0x048
> +
> +/* Queue notifier - Write Only */
> +#define VIRTIO_MDEV_QUEUE_NOTIFY 0x050
> +
> +/* Device status register - Read Write */
> +#define VIRTIO_MDEV_STATUS 0x060
> +
> +/* Selected queue's Descriptor Table address, 64 bits in two halves */
> +#define VIRTIO_MDEV_QUEUE_DESC_LOW 0x080
> +#define VIRTIO_MDEV_QUEUE_DESC_HIGH 0x084
> +
> +/* Selected queue's Available Ring address, 64 bits in two halves */
> +#define VIRTIO_MDEV_QUEUE_AVAIL_LOW 0x090
> +#define VIRTIO_MDEV_QUEUE_AVAIL_HIGH 0x094
> +
> +/* Selected queue's Used Ring address, 64 bits in two halves */
> +#define VIRTIO_MDEV_QUEUE_USED_LOW 0x0a0
> +#define VIRTIO_MDEV_QUEUE_USED_HIGH 0x0a4
> +
> +/* Configuration atomicity value */
> +#define VIRTIO_MDEV_CONFIG_GENERATION 0x0fc
> +
> +/* The config space is defined by each driver as
> + * the per-driver configuration space - Read Write */
> +#define VIRTIO_MDEV_CONFIG 0x100
Mixing device and generic config space is what virtio pci did,
caused lots of problems with extensions.
It would be better to reserve much more space.
> +
> +#endif
> +
> +
> +/* Ready bit for the currently selected queue - Read Write */
> --
> 2.19.1
^ permalink raw reply
* [PATCH] ath9k: remove unneeded variable
From: Ding Xiang @ 2019-09-10 9:55 UTC (permalink / raw)
To: kvalo, davem; +Cc: linux-wireless, netdev, linux-kernel
"len" is unneeded,just return 0
Signed-off-by: Ding Xiang <dingxiang@cmss.chinamobile.com>
---
drivers/net/wireless/ath/ath9k/gpio.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/gpio.c b/drivers/net/wireless/ath/ath9k/gpio.c
index b457e52..f3d1bc0 100644
--- a/drivers/net/wireless/ath/ath9k/gpio.c
+++ b/drivers/net/wireless/ath/ath9k/gpio.c
@@ -498,14 +498,13 @@ static int ath9k_dump_legacy_btcoex(struct ath_softc *sc, u8 *buf, u32 size)
{
struct ath_btcoex *btcoex = &sc->btcoex;
- u32 len = 0;
ATH_DUMP_BTCOEX("Stomp Type", btcoex->bt_stomp_type);
ATH_DUMP_BTCOEX("BTCoex Period (msec)", btcoex->btcoex_period);
ATH_DUMP_BTCOEX("Duty Cycle", btcoex->duty_cycle);
ATH_DUMP_BTCOEX("BT Wait time", btcoex->bt_wait_time);
- return len;
+ return 0;
}
int ath9k_dump_btcoex(struct ath_softc *sc, u8 *buf, u32 size)
--
1.9.1
^ permalink raw reply related
* Re: [RFC PATCH 4/4] docs: Sample driver to demonstrate how to implement virtio-mdev framework
From: Michael S. Tsirkin @ 2019-09-10 10:15 UTC (permalink / raw)
To: Jason Wang
Cc: kvm, virtualization, netdev, linux-kernel, kwankhede,
alex.williamson, cohuck, tiwei.bie, maxime.coquelin,
cunming.liang, zhihong.wang, rob.miller, idos, xiao.w.wang,
haotian.wang
In-Reply-To: <20190910081935.30516-5-jasowang@redhat.com>
On Tue, Sep 10, 2019 at 04:19:35PM +0800, Jason Wang wrote:
> This sample driver creates mdev device that simulate virtio net device
> over virtio mdev transport. The device is implemented through vringh
> and workqueue.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> samples/Kconfig | 7 +
> samples/vfio-mdev/Makefile | 1 +
> samples/vfio-mdev/mvnet.c | 766 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 774 insertions(+)
> create mode 100644 samples/vfio-mdev/mvnet.c
So for a POC, this is a bit too rough to be able to judge
whether the approach is workable.
Can you get it a bit more into shape esp wrt UAPI?
> diff --git a/samples/Kconfig b/samples/Kconfig
> index c8dacb4dda80..a1a1ca2c00b7 100644
> --- a/samples/Kconfig
> +++ b/samples/Kconfig
> @@ -131,6 +131,13 @@ config SAMPLE_VFIO_MDEV_MDPY
> mediated device. It is a simple framebuffer and supports
> the region display interface (VFIO_GFX_PLANE_TYPE_REGION).
>
> +config SAMPLE_VIRTIO_MDEV_NET
> + tristate "Build virtio mdev net example mediated device sample code -- loadable modules only"
> + depends on VIRTIO_MDEV_DEVICE && VHOST_RING && m
> + help
> + Build a networking sample device for use as a virtio
> + mediated device.
> +
> config SAMPLE_VFIO_MDEV_MDPY_FB
> tristate "Build VFIO mdpy example guest fbdev driver -- loadable module only"
> depends on FB && m
> diff --git a/samples/vfio-mdev/Makefile b/samples/vfio-mdev/Makefile
> index 10d179c4fdeb..f34af90ed0a0 100644
> --- a/samples/vfio-mdev/Makefile
> +++ b/samples/vfio-mdev/Makefile
> @@ -3,3 +3,4 @@ obj-$(CONFIG_SAMPLE_VFIO_MDEV_MTTY) += mtty.o
> obj-$(CONFIG_SAMPLE_VFIO_MDEV_MDPY) += mdpy.o
> obj-$(CONFIG_SAMPLE_VFIO_MDEV_MDPY_FB) += mdpy-fb.o
> obj-$(CONFIG_SAMPLE_VFIO_MDEV_MBOCHS) += mbochs.o
> +obj-$(CONFIG_SAMPLE_VIRTIO_MDEV_NET) += mvnet.o
> diff --git a/samples/vfio-mdev/mvnet.c b/samples/vfio-mdev/mvnet.c
> new file mode 100644
> index 000000000000..da295b41955e
> --- /dev/null
> +++ b/samples/vfio-mdev/mvnet.c
> @@ -0,0 +1,766 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Mediated virtual virtio-net device driver.
> + *
> + * Copyright (c) 2019, Red Hat Inc. All rights reserved.
> + * Author: Jason Wang <jasowang@redhat.com>
> + *
> + * Sample driver
Documentation on how to use this?
> that creates mdev device that simulates ethernet
> + * device virtio mdev transport.
Well not exactly. What it seems to do is short-circuit
RX and TX rings.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/fs.h>
> +#include <linux/poll.h>
> +#include <linux/slab.h>
> +#include <linux/sched.h>
> +#include <linux/wait.h>
> +#include <linux/uuid.h>
> +#include <linux/iommu.h>
> +#include <linux/sysfs.h>
> +#include <linux/file.h>
> +#include <linux/etherdevice.h>
> +#include <linux/mdev.h>
> +#include <uapi/linux/virtio_mdev.h>
> +
> +#define VERSION_STRING "0.1"
> +#define DRIVER_AUTHOR "NVIDIA Corporation"
> +
> +#define MVNET_CLASS_NAME "mvnet"
> +
> +#define MVNET_NAME "mvnet"
> +
> +/*
> + * Global Structures
> + */
> +
> +static struct mvnet_dev {
> + struct class *vd_class;
> + struct idr vd_idr;
> + struct device dev;
> +} mvnet_dev;
> +
> +struct mvnet_virtqueue {
> + struct vringh vring;
> + struct vringh_kiov iov;
> + unsigned short head;
> + bool ready;
> + u32 desc_addr_lo;
> + u32 desc_addr_hi;
> + u32 device_addr_lo;
> + u32 device_addr_hi;
> + u32 driver_addr_lo;
> + u32 driver_addr_hi;
> + u64 desc_addr;
> + u64 device_addr;
> + u64 driver_addr;
> + void *private;
> + irqreturn_t (*cb)(void *);
> +};
> +
> +#define MVNET_QUEUE_ALIGN PAGE_SIZE
> +#define MVNET_QUEUE_MAX 256
> +#define MVNET_MAGIC_VALUE ('v' | 'i' << 8 | 'r' << 16 | 't' << 24)
> +#define MVNET_VERSION 0x1 /* Implies virtio 1.0 */
> +#define MVNET_DEVICE_ID 0x1 /* network card */
> +#define MVNET_VENDOR_ID 0 /* is this correct ? */
> +#define MVNET_DEVICE_FEATURES VIRTIO_F_VERSION_1
> +
> +u64 mvnet_features = (1ULL << VIRTIO_F_ANY_LAYOUT) |
> + (1ULL << VIRTIO_F_VERSION_1) |
> + (1ULL << VIRTIO_F_IOMMU_PLATFORM) ;
> +
> +/* State of each mdev device */
> +struct mvnet_state {
> + struct mvnet_virtqueue vqs[2];
> + struct work_struct work;
> + spinlock_t lock;
> + struct mdev_device *mdev;
> + struct virtio_net_config config;
> + struct virtio_mdev_callback *cbs;
> + void *buffer;
> + u32 queue_sel;
> + u32 driver_features_sel;
> + u32 driver_features[2];
> + u32 device_features_sel;
> + u32 status;
> + u32 generation;
> + u32 num;
> + struct list_head next;
> +};
> +
> +static struct mutex mdev_list_lock;
> +static struct list_head mdev_devices_list;
> +
> +static void mvnet_queue_ready(struct mvnet_state *mvnet, unsigned idx)
> +{
> + struct mvnet_virtqueue *vq = &mvnet->vqs[idx];
> + int ret;
> +
> + vq->desc_addr = (u64)vq->desc_addr_hi << 32 | vq->desc_addr_lo;
> + vq->device_addr = (u64)vq->device_addr_hi << 32 | vq->device_addr_lo;
> + vq->driver_addr = (u64)vq->driver_addr_hi << 32 | vq->driver_addr_lo;
> +
> + ret = vringh_init_kern(&vq->vring, mvnet_features, MVNET_QUEUE_MAX,
> + false, (struct vring_desc *)vq->desc_addr,
> + (struct vring_avail *)vq->driver_addr,
> + (struct vring_used *)vq->device_addr);
> +}
> +
> +static ssize_t mvnet_read_config(struct mdev_device *mdev,
> + u32 *val, loff_t pos)
> +{
> + struct mvnet_state *mvnet;
> + struct mvnet_virtqueue *vq;
> + u32 queue_sel;
> +
> + if (!mdev || !val)
> + return -EINVAL;
> +
> + mvnet = mdev_get_drvdata(mdev);
> + if (!mvnet) {
> + pr_err("%s mvnet not found\n", __func__);
> + return -EINVAL;
> + }
> +
> + queue_sel = mvnet->queue_sel;
> + vq = &mvnet->vqs[queue_sel];
> +
> + switch (pos) {
> + case VIRTIO_MDEV_MAGIC_VALUE:
> + *val = MVNET_MAGIC_VALUE;
> + break;
> + case VIRTIO_MDEV_VERSION:
> + *val = MVNET_VERSION;
> + break;
> + case VIRTIO_MDEV_DEVICE_ID:
> + *val = MVNET_DEVICE_ID;
> + break;
> + case VIRTIO_MDEV_VENDOR_ID:
> + *val = MVNET_VENDOR_ID;
> + break;
> + case VIRTIO_MDEV_DEVICE_FEATURES:
> + if (mvnet->device_features_sel)
> + *val = mvnet_features >> 32;
> + else
> + *val = mvnet_features;
> + break;
> + case VIRTIO_MDEV_QUEUE_NUM_MAX:
> + *val = MVNET_QUEUE_MAX;
> + break;
> + case VIRTIO_MDEV_QUEUE_READY:
> + *val = vq->ready;
> + break;
> + case VIRTIO_MDEV_QUEUE_ALIGN:
> + *val = MVNET_QUEUE_ALIGN;
> + break;
> + case VIRTIO_MDEV_STATUS:
> + *val = mvnet->status;
> + break;
> + case VIRTIO_MDEV_QUEUE_DESC_LOW:
> + *val = vq->desc_addr_lo;
> + break;
> + case VIRTIO_MDEV_QUEUE_DESC_HIGH:
> + *val = vq->desc_addr_hi;
> + break;
> + case VIRTIO_MDEV_QUEUE_AVAIL_LOW:
> + *val = vq->driver_addr_lo;
> + break;
> + case VIRTIO_MDEV_QUEUE_AVAIL_HIGH:
> + *val = vq->driver_addr_hi;
> + break;
> + case VIRTIO_MDEV_QUEUE_USED_LOW:
> + *val = vq->device_addr_lo;
> + break;
> + case VIRTIO_MDEV_QUEUE_USED_HIGH:
> + *val = vq->device_addr_hi;
> + break;
> + case VIRTIO_MDEV_CONFIG_GENERATION:
> + *val = 1;
> + break;
> + default:
> + pr_err("Unsupported mdev read offset at 0x%x\n", pos);
> + break;
> + }
> +
> + return 4;
> +}
> +
> +static ssize_t mvnet_read_net_config(struct mdev_device *mdev,
> + char *buf, size_t count, loff_t pos)
> +{
> + struct mvnet_state *mvnet = mdev_get_drvdata(mdev);
> +
> + if (!mvnet) {
> + pr_err("%s mvnet not found\n", __func__);
> + return -EINVAL;
> + }
> +
> + if (pos + count > sizeof(mvnet->config))
> + return -EINVAL;
> +
> + memcpy(buf, &mvnet->config + (unsigned)pos, count);
> +
> + return count;
> +}
> +
> +static void mvnet_vq_reset(struct mvnet_virtqueue *vq)
> +{
> + vq->ready = 0;
> + vq->desc_addr_lo = vq->desc_addr_hi = 0;
> + vq->device_addr_lo = vq->device_addr_hi = 0;
> + vq->driver_addr_lo = vq->driver_addr_hi = 0;
> + vq->desc_addr = 0;
> + vq->driver_addr = 0;
> + vq->device_addr = 0;
> + vringh_init_kern(&vq->vring, mvnet_features, MVNET_QUEUE_MAX,
> + false, 0, 0, 0);
> +}
> +
> +static void mvnet_reset(struct mvnet_state *mvnet)
> +{
> + int i;
> +
> + for (i = 0; i < 2; i++)
> + mvnet_vq_reset(&mvnet->vqs[i]);
> +
> + mvnet->queue_sel = 0;
> + mvnet->driver_features_sel = 0;
> + mvnet->device_features_sel = 0;
> + mvnet->status = 0;
> + ++mvnet->generation;
> +}
> +
> +static ssize_t mvnet_write_config(struct mdev_device *mdev,
> + u32 *val, loff_t pos)
> +{
> + struct mvnet_state *mvnet;
> + struct mvnet_virtqueue *vq;
> + u32 queue_sel;
> +
> + if (!mdev || !val)
> + return -EINVAL;
> +
> + mvnet = mdev_get_drvdata(mdev);
> + if (!mvnet) {
> + pr_err("%s mvnet not found\n", __func__);
> + return -EINVAL;
> + }
> +
> + queue_sel = mvnet->queue_sel;
> + vq = &mvnet->vqs[queue_sel];
> +
> + switch (pos) {
> + case VIRTIO_MDEV_DEVICE_FEATURES_SEL:
> + mvnet->device_features_sel = *val;
> + break;
> + case VIRTIO_MDEV_DRIVER_FEATURES:
> + mvnet->driver_features[mvnet->driver_features_sel] = *val;
> + break;
> + case VIRTIO_MDEV_DRIVER_FEATURES_SEL:
> + mvnet->driver_features_sel = *val;
> + break;
> + case VIRTIO_MDEV_QUEUE_SEL:
> + mvnet->queue_sel = *val;
> + break;
> + case VIRTIO_MDEV_QUEUE_NUM:
> + mvnet->num = *val;
> + break;
> + case VIRTIO_MDEV_QUEUE_READY:
> + vq->ready = *val;
> + if (vq->ready) {
> + spin_lock(&mvnet->lock);
> + mvnet_queue_ready(mvnet, queue_sel);
> + spin_unlock(&mvnet->lock);
> + }
> + break;
> + case VIRTIO_MDEV_QUEUE_NOTIFY:
> + if (vq->ready)
> + schedule_work(&mvnet->work);
> + break;
> + case VIRTIO_MDEV_STATUS:
> + mvnet->status = *val;
> + if (*val == 0) {
> + spin_lock(&mvnet->lock);
> + mvnet_reset(mvnet);
> + spin_unlock(&mvnet->lock);
> + }
> + break;
> + case VIRTIO_MDEV_QUEUE_DESC_LOW:
> + vq->desc_addr_lo = *val;
> + break;
> + case VIRTIO_MDEV_QUEUE_DESC_HIGH:
> + vq->desc_addr_hi = *val;
> + break;
> + case VIRTIO_MDEV_QUEUE_AVAIL_LOW:
> + vq->driver_addr_lo = *val;
> + break;
> + case VIRTIO_MDEV_QUEUE_AVAIL_HIGH:
> + vq->driver_addr_hi = *val;
> + break;
> + case VIRTIO_MDEV_QUEUE_USED_LOW:
> + vq->device_addr_lo = *val;
> + break;
> + case VIRTIO_MDEV_QUEUE_USED_HIGH:
> + vq->device_addr_hi = *val;
> + break;
> + default:
> + pr_err("Unsupported write offset! 0x%x\n", pos);
> + break;
> + }
> + spin_unlock(&mvnet->lock);
> +
> + return 4;
> +}
> +
> +static void mvnet_work(struct work_struct *work)
> +{
> + struct mvnet_state *mvnet = container_of(work, struct
> + mvnet_state, work);
> + struct mvnet_virtqueue *txq = &mvnet->vqs[1];
> + struct mvnet_virtqueue *rxq = &mvnet->vqs[0];
> + size_t read, write, total_write;
> + unsigned long flags;
> + int err;
> + int pkts = 0;
> +
> + spin_lock(&mvnet->lock);
> +
> + if (!txq->ready || !rxq->ready)
> + goto out;
> +
> + while (true) {
> + total_write = 0;
> + err = vringh_getdesc_kern(&txq->vring, &txq->iov, NULL,
> + &txq->head, GFP_KERNEL);
> + if (err <= 0)
> + break;
> +
> + err = vringh_getdesc_kern(&rxq->vring, NULL, &rxq->iov,
> + &rxq->head, GFP_KERNEL);
GFP_KERNEL under a spin_lock will cause deadlocks.
> + if (err <= 0) {
> + vringh_complete_kern(&txq->vring, txq->head, 0);
> + break;
> + }
> +
> + while (true) {
> + read = vringh_iov_pull_kern(&txq->iov, mvnet->buffer,
> + PAGE_SIZE);
> + if (read <= 0)
> + break;
> +
> + write = vringh_iov_push_kern(&rxq->iov, mvnet->buffer,
> + read);
> + if (write <= 0)
> + break;
> +
> + total_write += write;
> + }
> +
> + /* Make sure data is wrote before advancing index */
> + smp_wmb();
> +
> + vringh_complete_kern(&txq->vring, txq->head, 0);
> + vringh_complete_kern(&rxq->vring, rxq->head, total_write);
> +
> + /* Make sure used is visible before rasing the
> + interrupt */
> + smp_wmb();
> +
> + local_bh_disable();
> + if (txq->cb)
> + txq->cb(txq->private);
> + if (rxq->cb)
> + rxq->cb(rxq->private);
> + local_bh_enable();
> +
> + pkts ++;
coding style problem
> + if (pkts > 4) {
> + schedule_work(&mvnet->work);
> + goto out;
> + }
> + }
> +
> +out:
> + spin_unlock(&mvnet->lock);
> +}
> +
> +static dma_addr_t mvnet_map_page(struct device *dev, struct page *page,
> + unsigned long offset, size_t size,
> + enum dma_data_direction dir,
> + unsigned long attrs)
> +{
> + /* Vringh can only use VA */
> + return page_address(page) + offset;
> +}
> +
> +static void mvnet_unmap_page(struct device *dev, dma_addr_t dma_addr,
> + size_t size, enum dma_data_direction dir,
> + unsigned long attrs)
> +{
> + return ;
> +}
> +
> +static void *mvnet_alloc_coherent(struct device *dev, size_t size,
> + dma_addr_t *dma_addr, gfp_t flag,
> + unsigned long attrs)
> +{
> + void *ret = kmalloc(size, flag);
> +
> + if (ret == NULL)
!ret is nicer
> + *dma_addr = DMA_MAPPING_ERROR;
> + else
> + *dma_addr = ret;
Not sure how does this build ... don't we need a cast?
> +
> + return ret;
> +}
> +
> +static void mvnet_free_coherent(struct device *dev, size_t size,
> + void *vaddr, dma_addr_t dma_addr,
> + unsigned long attrs)
> +{
> + kfree(dma_addr);
> +}
> +
> +static const struct dma_map_ops mvnet_dma_ops = {
> + .map_page = mvnet_map_page,
> + .unmap_page = mvnet_unmap_page,
> + .alloc = mvnet_alloc_coherent,
> + .free = mvnet_free_coherent,
> +};
> +
> +static int mvnet_create(struct kobject *kobj, struct mdev_device *mdev)
> +{
> + struct mvnet_state *mvnet;
> + struct virtio_net_config *config;
> +
> + if (!mdev)
> + return -EINVAL;
> +
> + mvnet = kzalloc(sizeof(struct mvnet_state), GFP_KERNEL);
> + if (mvnet == NULL)
> + return -ENOMEM;
> +
> + mvnet->buffer = kmalloc(PAGE_SIZE, GFP_KERNEL);
> + if (!mvnet->buffer) {
> + kfree(mvnet);
> + return -ENOMEM;
> + }
> +
> + config = &mvnet->config;
> + config->mtu = 1500;
> + config->status = VIRTIO_NET_S_LINK_UP;
> + eth_random_addr(config->mac);
> +
> + INIT_WORK(&mvnet->work, mvnet_work);
> +
> + spin_lock_init(&mvnet->lock);
> + mvnet->mdev = mdev;
> + mdev_set_drvdata(mdev, mvnet);
> +
> + mutex_lock(&mdev_list_lock);
> + list_add(&mvnet->next, &mdev_devices_list);
> + mutex_unlock(&mdev_list_lock);
> +
> + mdev_set_dma_ops(mdev, &mvnet_dma_ops);
> +
> + return 0;
> +}
> +
> +static int mvnet_remove(struct mdev_device *mdev)
> +{
> + struct mvnet_state *mds, *tmp_mds;
> + struct mvnet_state *mvnet = mdev_get_drvdata(mdev);
> + int ret = -EINVAL;
> +
> + mutex_lock(&mdev_list_lock);
> + list_for_each_entry_safe(mds, tmp_mds, &mdev_devices_list, next) {
> + if (mvnet == mds) {
> + list_del(&mvnet->next);
> + mdev_set_drvdata(mdev, NULL);
> + kfree(mvnet->buffer);
> + kfree(mvnet);
> + ret = 0;
> + break;
> + }
> + }
> + mutex_unlock(&mdev_list_lock);
> +
> + return ret;
> +}
> +
> +static ssize_t mvnet_read(struct mdev_device *mdev, char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + ssize_t ret;
> +
> + if (*ppos < VIRTIO_MDEV_CONFIG) {
> + if (count == 4)
> + ret = mvnet_read_config(mdev, (u32 *)buf, *ppos);
> + else
> + ret = -EINVAL;
> + *ppos += 4;
> + } else if (*ppos < VIRTIO_MDEV_CONFIG + sizeof(struct virtio_net_config)) {
> + ret = mvnet_read_net_config(mdev, buf, count,
> + *ppos - VIRTIO_MDEV_CONFIG);
> + *ppos += count;
> + } else {
> + ret = -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +static ssize_t mvnet_write(struct mdev_device *mdev, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + int ret;
> +
> + if (*ppos < VIRTIO_MDEV_CONFIG) {
> + if (count == 4)
> + ret = mvnet_write_config(mdev, (u32 *)buf, *ppos);
> + else
> + ret = -EINVAL;
> + *ppos += 4;
> + } else {
> + /* No writable net config */
> + ret = -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +static long mvnet_ioctl(struct mdev_device *mdev, unsigned int cmd,
> + unsigned long arg)
> +{
> + int ret = 0;
> + struct mvnet_state *mvnet;
> + struct virtio_mdev_callback *cb;
> +
> + if (!mdev)
> + return -EINVAL;
> +
> + mvnet = mdev_get_drvdata(mdev);
> + if (!mvnet)
> + return -ENODEV;
> +
> + spin_lock(&mvnet->lock);
> +
> + switch (cmd) {
> + case VIRTIO_MDEV_SET_VQ_CALLBACK:
> + cb = (struct virtio_mdev_callback *)arg;
> + mvnet->vqs[mvnet->queue_sel].cb = cb->callback;
> + mvnet->vqs[mvnet->queue_sel].private = cb->private;
> + break;
> + case VIRTIO_MDEV_SET_CONFIG_CALLBACK:
> + break;
success, but nothing happens.
> + default:
> + pr_err("Not supportted ioctl cmd 0x%x\n", cmd);
> + ret = -ENOTTY;
> + break;
> + }
> +
> + spin_unlock(&mvnet->lock);
> +
> + return ret;
> +}
> +
> +static int mvnet_open(struct mdev_device *mdev)
> +{
> + pr_info("%s\n", __func__);
> + return 0;
> +}
> +
> +static void mvnet_close(struct mdev_device *mdev)
> +{
> + pr_info("%s\n", __func__);
> +}
> +
> +static ssize_t
> +sample_mvnet_dev_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + return sprintf(buf, "This is phy device\n");
what's this?
> +}
> +
> +static DEVICE_ATTR_RO(sample_mvnet_dev);
> +
> +static struct attribute *mvnet_dev_attrs[] = {
> + &dev_attr_sample_mvnet_dev.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group mvnet_dev_group = {
> + .name = "mvnet_dev",
> + .attrs = mvnet_dev_attrs,
> +};
> +
> +static const struct attribute_group *mvnet_dev_groups[] = {
> + &mvnet_dev_group,
> + NULL,
> +};
> +
> +static ssize_t
> +sample_mdev_dev_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + if (mdev_from_dev(dev))
> + return sprintf(buf, "This is MDEV %s\n", dev_name(dev));
> +
> + return sprintf(buf, "\n");
> +}
> +
> +static DEVICE_ATTR_RO(sample_mdev_dev);
> +
> +static struct attribute *mdev_dev_attrs[] = {
> + &dev_attr_sample_mdev_dev.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group mdev_dev_group = {
> + .name = "vendor",
> + .attrs = mdev_dev_attrs,
> +};
> +
> +static const struct attribute_group *mdev_dev_groups[] = {
> + &mdev_dev_group,
> + NULL,
> +};
> +
> +#define MVNET_STRING_LEN 16
> +
> +static ssize_t
> +name_show(struct kobject *kobj, struct device *dev, char *buf)
> +{
> + char name[MVNET_STRING_LEN];
> + const char *name_str = "virtio-net";
> +
> + snprintf(name, MVNET_STRING_LEN, "%s", dev_driver_string(dev));
> + if (!strcmp(kobj->name, name))
> + return sprintf(buf, "%s\n", name_str);
> +
> + return -EINVAL;
> +}
> +
> +static MDEV_TYPE_ATTR_RO(name);
> +
> +static ssize_t
> +available_instances_show(struct kobject *kobj, struct device *dev, char *buf)
> +{
> + return sprintf(buf, "%d\n", INT_MAX);
> +}
> +
> +static MDEV_TYPE_ATTR_RO(available_instances);
?
> +
> +static ssize_t device_api_show(struct kobject *kobj, struct device *dev,
> + char *buf)
> +{
> + return sprintf(buf, "%s\n", VIRTIO_MDEV_DEVICE_API_STRING);
> +}
> +
> +static MDEV_TYPE_ATTR_RO(device_api);
> +
> +static struct attribute *mdev_types_attrs[] = {
> + &mdev_type_attr_name.attr,
> + &mdev_type_attr_device_api.attr,
> + &mdev_type_attr_available_instances.attr,
> + NULL,
> +};
> +
> +static struct attribute_group mdev_type_group = {
> + .name = "",
> + .attrs = mdev_types_attrs,
> +};
> +
> +static struct attribute_group *mdev_type_groups[] = {
> + &mdev_type_group,
> + NULL,
> +};
> +
> +static const struct mdev_parent_ops mdev_fops = {
> + .owner = THIS_MODULE,
> + .dev_attr_groups = mvnet_dev_groups,
> + .mdev_attr_groups = mdev_dev_groups,
> + .supported_type_groups = mdev_type_groups,
> + .create = mvnet_create,
> + .remove = mvnet_remove,
> + .open = mvnet_open,
> + .release = mvnet_close,
> + .read = mvnet_read,
> + .write = mvnet_write,
> + .ioctl = mvnet_ioctl,
> +};
> +
> +static void mvnet_device_release(struct device *dev)
> +{
> + dev_dbg(dev, "mvnet: released\n");
> +}
> +
> +static int __init mvnet_dev_init(void)
> +{
> + int ret = 0;
> +
> + pr_info("mvnet_dev: %s\n", __func__);
> +
> + memset(&mvnet_dev, 0, sizeof(mvnet_dev));
> +
> + idr_init(&mvnet_dev.vd_idr);
> +
> + mvnet_dev.vd_class = class_create(THIS_MODULE, MVNET_CLASS_NAME);
> +
> + if (IS_ERR(mvnet_dev.vd_class)) {
> + pr_err("Error: failed to register mvnet_dev class\n");
> + ret = PTR_ERR(mvnet_dev.vd_class);
> + goto failed1;
> + }
> +
> + mvnet_dev.dev.class = mvnet_dev.vd_class;
> + mvnet_dev.dev.release = mvnet_device_release;
> + dev_set_name(&mvnet_dev.dev, "%s", MVNET_NAME);
> +
> + ret = device_register(&mvnet_dev.dev);
> + if (ret)
> + goto failed2;
> +
> + ret = mdev_register_device(&mvnet_dev.dev, &mdev_fops);
> + if (ret)
> + goto failed3;
> +
> + mutex_init(&mdev_list_lock);
> + INIT_LIST_HEAD(&mdev_devices_list);
> +
> + goto all_done;
> +
> +failed3:
> +
> + device_unregister(&mvnet_dev.dev);
> +failed2:
> + class_destroy(mvnet_dev.vd_class);
> +
> +failed1:
> +all_done:
> + return ret;
> +}
> +
> +static void __exit mvnet_dev_exit(void)
> +{
> + mvnet_dev.dev.bus = NULL;
> + mdev_unregister_device(&mvnet_dev.dev);
> +
> + device_unregister(&mvnet_dev.dev);
> + idr_destroy(&mvnet_dev.vd_idr);
> + class_destroy(mvnet_dev.vd_class);
> + mvnet_dev.vd_class = NULL;
> + pr_info("mvnet_dev: Unloaded!\n");
> +}
> +
> +module_init(mvnet_dev_init)
> +module_exit(mvnet_dev_exit)
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_INFO(supported, "Test driver that simulate serial port over PCI");
> +MODULE_VERSION(VERSION_STRING);
> +MODULE_AUTHOR(DRIVER_AUTHOR);
> --
> 2.19.1
^ permalink raw reply
* Re: [PATCH net] net: sonic: replace dev_kfree_skb in sonic_send_packet
From: Sergei Shtylyov @ 2019-09-10 10:23 UTC (permalink / raw)
To: Mao Wenan, tsbogend, davem; +Cc: netdev, linux-kernel, kernel-janitors
In-Reply-To: <20190910085848.144780-1-maowenan@huawei.com>
Hello!
On 10.09.2019 11:58, Mao Wenan wrote:
> sonic_send_packet will be processed in irq or none
s/none irq/non-irq/?
> irq context, so it would better use dev_kfree_skb_any
> instead of dev_kfree_skb.
>
> Fixes: d9fb9f384292 ("*sonic/natsemi/ns83829: Move the National Semi-conductor drivers")
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
[...]
MBR, Sergei
^ permalink raw reply
* RE: [RFC PATCH 3/3] Enable ptp_kvm for arm64
From: Jianyong Wu (Arm Technology China) @ 2019-09-10 10:29 UTC (permalink / raw)
To: Marc Zyngier
Cc: netdev@vger.kernel.org, pbonzini@redhat.com,
sean.j.christopherson@intel.com, richardcochran@gmail.com,
Mark Rutland, Will Deacon, Suzuki Poulose,
linux-kernel@vger.kernel.org, Steve Capper,
Kaly Xin (Arm Technology China), Justin He (Arm Technology China)
In-Reply-To: <86blvtsodw.wl-maz@kernel.org>
Hi Marc,
> -----Original Message-----
> From: Marc Zyngier <maz@kernel.org>
> Sent: Monday, September 9, 2019 7:25 PM
> To: Jianyong Wu (Arm Technology China) <Jianyong.Wu@arm.com>
> Cc: netdev@vger.kernel.org; pbonzini@redhat.com;
> sean.j.christopherson@intel.com; richardcochran@gmail.com; Mark Rutland
> <Mark.Rutland@arm.com>; Will Deacon <Will.Deacon@arm.com>; Suzuki
> Poulose <Suzuki.Poulose@arm.com>; linux-kernel@vger.kernel.org; Steve
> Capper <Steve.Capper@arm.com>; Kaly Xin (Arm Technology China)
> <Kaly.Xin@arm.com>; Justin He (Arm Technology China)
> <Justin.He@arm.com>
> Subject: Re: [RFC PATCH 3/3] Enable ptp_kvm for arm64
>
> On Mon, 09 Sep 2019 11:17:24 +0100,
> "Jianyong Wu (Arm Technology China)" <Jianyong.Wu@arm.com> wrote:
>
> Hi Jianyoung,
>
> [...]
>
> > > > > I'm definitely not keen on exposing the internals of the
> > > > > arch_timer driver to random subsystems. Furthermore, you seem to
> > > > > expect that the guest kernel will only use the arch timer as a
> > > > > clocksource, and nothing really guarantees that (in which case
> > > get_device_system_crosststamp will fail).
> > > > >
> > > > The code here is really ugly, I need a better solution to offer a
> > > > clock source For the guest.
> > > >
> > > > > It looks to me that we'd be better off exposing a core
> > > > > timekeeping API that populates a struct system_counterval_t
> > > > > based on the
> > > > > *current* timekeeper monotonic clocksource. This would simplify
> > > > > the split between generic and arch-specific code.
> > > > >
> > > > I think it really necessary.
> > > >
> > > > > Whether or not tglx will be happy with the idea is another
> > > > > problem, but I'm certainly not taking any change to the arch
> > > > > timer code based on
> > > this.
> > > > >
> > > > I can have a try, but the detail is not clear for me now.
> > >
> > > Something along those lines:
> > >
> > > From 5f1c061e55c691d64012bc7c1490a1a8c4432c67 Mon Sep 17 00:00:00
> > > 2001
> > > From: Marc Zyngier <maz@kernel.org>
> > > Date: Sat, 7 Sep 2019 10:11:49 +0100
> > > Subject: [PATCH] timekeeping: Expose API allowing retrival of
> > > current clocksource and counter value
> > >
> > > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > > ---
> > > include/linux/timekeeping.h | 5 +++++
> > > kernel/time/timekeeping.c | 12 ++++++++++++
> > > 2 files changed, 17 insertions(+)
> > >
> > > diff --git a/include/linux/timekeeping.h
> > > b/include/linux/timekeeping.h index
> > > b27e2ffa96c1..6df26a913711 100644
> > > --- a/include/linux/timekeeping.h
> > > +++ b/include/linux/timekeeping.h
> > > @@ -275,6 +275,11 @@ extern int get_device_system_crosststamp(
> > > struct system_time_snapshot *history,
> > > struct system_device_crosststamp *xtstamp);
> > >
> > > +/*
> > > + * Obtain current monotonic clock and its counter value */ extern
> > > +void get_current_counterval(struct system_counterval_t *sc);
> > > +
> > > /*
> > > * Simultaneously snapshot realtime and monotonic raw clocks
> > > */
> > > diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> > > index
> > > d911c8470149..de689bbd3808 100644
> > > --- a/kernel/time/timekeeping.c
> > > +++ b/kernel/time/timekeeping.c
> > > @@ -1098,6 +1098,18 @@ static bool cycle_between(u64 before, u64
> > > test,
> > > u64 after)
> > > return false;
> > > }
> > >
> > > +/**
> > > + * get_current_counterval - Snapshot the current clocksource and
> > > +counter
> > > value
> > > + * @sc: Pointer to a struct containing the current clocksource and its
> > > value
> > > + */
> > > +void get_current_counterval(struct system_counterval_t *sc) {
> > > + struct timekeeper *tk = &tk_core.timekeeper;
> > > +
> > > + sc->cs = READ_ONCE(tk->tkr_mono.clock);
> > > + sc->cycles = sc->cs->read(sc->cs); }
> > > +
> > > /**
> > > * get_device_system_crosststamp - Synchronously capture
> > > system/device timestamp
> > > * @get_time_fn: Callback to get simultaneous device time and
> > >
> > > which should do the right thing.
> > >
> > It is a good news for me. These code is indeed what I need! So what's
> > your plan about this patch? Is there any problem with you if I
> > include these code into my patch ?
>
> Just add this patch as part of your series (I'll try to write an actual commit log
> for that).
Very kind of you!
>
> [...]
>
> > > > > Other questions: how does this works with VM migration?
> > > > > Specially when moving from a hypervisor that supports the
> > > > > feature to one that
> > > doesn't?
> > > > >
> > > > I think it won't solve the problem generated by VM migration and
> > > > only for VMs in a single machine. Ptp_kvm only works for VMs in
> > > > the same machine. But using ptp (not ptp_kvm) clock, all the
> > > > machines in a low latency network environment can keep time sync
> > > > in high precision, Then VMs move from one machine to another will
> > > > obtain a high precision time sync.
> > >
> > > That's a problem. Migration must be possible from one host to
> > > another, even if that means temporarily loosing some (or a lot of)
> > > precision. The service must be discoverable from userspace on the
> > > host so that the MVV can decie whether a migration is possible or not.
> > >
> > Don't worry, things will be not that bad. ptp_kvm will not trouble
> > the VM migration. This ptp_kvm is one clocksource of the clock pool
> > for chrony. Chrony will choose the highest precision clock from the
> > pool. If host does not support ptp_kvm, the ptp_kvm will not be chosen
> > as the clocksouce of chrony. We have roughly the same logic of
> > implementation of ptp_kvm with x86, and ptp_kvm works well in x86. so
> > I think that will be the case for arm64.
> >
> > Maybe I miss your point, I have no idea of MVV and can't get related
> > info from google. Also I'm not clear of your last words of how to
> > decide VM migration is possible?
>
> Sorry. s/MVV/VMM/. Basically userspace, such as QEMU.
>
> Here's an example: The guest runs on a PTP aware host, starts using the PTP
> service and uses HVC calls to get its clock. We now migrate the guest to a non
> PTP-aware host. The hypercalls are now going to fail unexpectedly. Is that
> something that is acceptable? I don't think it is. Once you've allowed a guest
> to use a service, this service should be preserved. I'd be more confident if we
> gave to userspace the indication that the hypervisor supports PTP. Userspace
> can then decide whether to perform migration or not.
>
It's really a point we should consider. let me check the behavior of chrony in this scenario first.
Thanks
Jianyong Wu
> Thanks,
>
> M.
>
> --
> Jazz is not dead, it just smells funny.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply
* [PATCH bpf-next 04/11] samples: bpf: use own EXTRA_CFLAGS for clang commands
From: Ivan Khoronzhuk @ 2019-09-10 10:38 UTC (permalink / raw)
To: ast, daniel, yhs, davem, jakub.kicinski, hawk, john.fastabend
Cc: linux-kernel, netdev, bpf, clang-built-linux, Ivan Khoronzhuk
In-Reply-To: <20190910103830.20794-1-ivan.khoronzhuk@linaro.org>
It can overlap with CFLAGS used for libraries built with gcc if
not now then in next patches. Correct it here for simplicity.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
samples/bpf/Makefile | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index b59e77e2250e..8ecc5d0c2d5b 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -218,10 +218,10 @@ BTF_LLVM_PROBE := $(shell echo "int main() { return 0; }" | \
/bin/rm -f ./llvm_btf_verify.o)
ifneq ($(BTF_LLVM_PROBE),)
- EXTRA_CFLAGS += -g
+ CLANG_EXTRA_CFLAGS += -g
else
ifneq ($(and $(BTF_LLC_PROBE),$(BTF_PAHOLE_PROBE),$(BTF_OBJCOPY_PROBE)),)
- EXTRA_CFLAGS += -g
+ CLANG_EXTRA_CFLAGS += -g
LLC_FLAGS += -mattr=dwarfris
DWARF2BTF = y
endif
@@ -280,8 +280,8 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
# useless for BPF samples.
$(obj)/%.o: $(src)/%.c
@echo " CLANG-bpf " $@
- $(Q)$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) -I$(obj) \
- -I$(srctree)/tools/testing/selftests/bpf/ \
+ $(Q)$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(CLANG_EXTRA_CFLAGS) \
+ -I$(obj) -I$(srctree)/tools/testing/selftests/bpf/ \
-D__KERNEL__ -D__BPF_TRACING__ -Wno-unused-value -Wno-pointer-sign \
-D__TARGET_ARCH_$(SRCARCH) -Wno-compare-distinct-pointer-types \
-Wno-gnu-variable-sized-type-not-at-end \
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 11/11] samples: bpf: makefile: add sysroot support
From: Ivan Khoronzhuk @ 2019-09-10 10:38 UTC (permalink / raw)
To: ast, daniel, yhs, davem, jakub.kicinski, hawk, john.fastabend
Cc: linux-kernel, netdev, bpf, clang-built-linux, Ivan Khoronzhuk
In-Reply-To: <20190910103830.20794-1-ivan.khoronzhuk@linaro.org>
Basically it only enables that was added by previous couple fixes.
For sure, just make tools/include to be included after sysroot
headers.
export ARCH=arm
export CROSS_COMPILE=arm-linux-gnueabihf-
make samples/bpf/ SYSROOT="path/to/sysroot"
Sysroot contains correct libs installed and its headers ofc.
Useful when working with NFC or virtual machine.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
samples/bpf/Makefile | 5 +++++
samples/bpf/README.rst | 10 ++++++++++
2 files changed, 15 insertions(+)
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 4edc5232cfc1..68ba78d1dbbe 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -177,6 +177,11 @@ ifeq ($(ARCH), arm)
CLANG_EXTRA_CFLAGS := $(D_OPTIONS)
endif
+ifdef SYSROOT
+ccflags-y += --sysroot=${SYSROOT}
+PROGS_LDFLAGS := -L${SYSROOT}/usr/lib
+endif
+
ccflags-y += -I$(objtree)/usr/include
ccflags-y += -I$(srctree)/tools/lib/bpf/
ccflags-y += -I$(srctree)/tools/testing/selftests/bpf/
diff --git a/samples/bpf/README.rst b/samples/bpf/README.rst
index 5f27e4faca50..786d0ab98e8a 100644
--- a/samples/bpf/README.rst
+++ b/samples/bpf/README.rst
@@ -74,3 +74,13 @@ samples for the cross target.
export ARCH=arm64
export CROSS_COMPILE="aarch64-linux-gnu-"
make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
+
+If need to use environment of target board (headers and libs), the SYSROOT
+also can be set, pointing on FS of target board:
+
+export ARCH=arm64
+export CROSS_COMPILE="aarch64-linux-gnu-"
+make samples/bpf/ SYSROOT=~/some_sdk/linux-devkit/sysroots/aarch64-linux-gnu
+
+Setting LLC and CLANG is not necessarily if it's installed on HOST and have
+in its targets appropriate arch triple (usually it has several arches).
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 10/11] libbpf: makefile: add C/CXX/LDFLAGS to libbpf.so and test_libpf targets
From: Ivan Khoronzhuk @ 2019-09-10 10:38 UTC (permalink / raw)
To: ast, daniel, yhs, davem, jakub.kicinski, hawk, john.fastabend
Cc: linux-kernel, netdev, bpf, clang-built-linux, Ivan Khoronzhuk
In-Reply-To: <20190910103830.20794-1-ivan.khoronzhuk@linaro.org>
In case of LDFLAGS and EXTRA_CC/CXX flags there is no way to pass them
correctly to build command, for instance when --sysroot is used or
external libraries are used, like -lelf, wich can be absent in
toolchain. This is used for samples/bpf cross-compiling allowing to
get elf lib from sysroot.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
samples/bpf/Makefile | 8 +++++++-
tools/lib/bpf/Makefile | 11 ++++++++---
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 79c9aa41832e..4edc5232cfc1 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -186,6 +186,10 @@ ccflags-y += -I$(srctree)/tools/perf
ccflags-y += $(D_OPTIONS)
ccflags-y += -Wall
ccflags-y += -fomit-frame-pointer
+
+EXTRA_CXXFLAGS := $(ccflags-y)
+
+# options not valid for C++
ccflags-y += -Wmissing-prototypes
ccflags-y += -Wstrict-prototypes
@@ -252,7 +256,9 @@ clean:
$(LIBBPF): FORCE
# Fix up variables inherited from Kbuild that tools/ build system won't like
- $(MAKE) -C $(dir $@) RM='rm -rf' LDFLAGS= srctree=$(BPF_SAMPLES_PATH)/../../ O=
+ $(MAKE) -C $(dir $@) RM='rm -rf' EXTRA_CFLAGS="$(PROGS_CFLAGS)" \
+ EXTRA_CXXFLAGS="$(EXTRA_CXXFLAGS)" LDFLAGS=$(PROGS_LDFLAGS) \
+ srctree=$(BPF_SAMPLES_PATH)/../../ O=
$(obj)/syscall_nrs.h: $(obj)/syscall_nrs.s FORCE
$(call filechk,offsets,__SYSCALL_NRS_H__)
diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index c6f94cffe06e..bccfa556ef4e 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -94,6 +94,10 @@ else
CFLAGS := -g -Wall
endif
+ifdef EXTRA_CXXFLAGS
+ CXXFLAGS := $(EXTRA_CXXFLAGS)
+endif
+
ifeq ($(feature-libelf-mmap), 1)
override CFLAGS += -DHAVE_LIBELF_MMAP_SUPPORT
endif
@@ -176,8 +180,9 @@ $(BPF_IN): force elfdep bpfdep
$(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
$(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN)
- $(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
- -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
+ $(QUIET_LINK)$(CC) $(LDFLAGS) \
+ --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
+ -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
@ln -sf $(@F) $(OUTPUT)libbpf.so
@ln -sf $(@F) $(OUTPUT)libbpf.so.$(LIBBPF_MAJOR_VERSION)
@@ -185,7 +190,7 @@ $(OUTPUT)libbpf.a: $(BPF_IN)
$(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
$(OUTPUT)test_libbpf: test_libbpf.cpp $(OUTPUT)libbpf.a
- $(QUIET_LINK)$(CXX) $(INCLUDES) $^ -lelf -o $@
+ $(QUIET_LINK)$(CXX) $(CXXFLAGS) $(LDFLAGS) $(INCLUDES) $^ -lelf -o $@
$(OUTPUT)libbpf.pc:
$(QUIET_GEN)sed -e "s|@PREFIX@|$(prefix)|" \
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 09/11] samples: bpf: makefile: use CC environment for HDR_PROBE
From: Ivan Khoronzhuk @ 2019-09-10 10:38 UTC (permalink / raw)
To: ast, daniel, yhs, davem, jakub.kicinski, hawk, john.fastabend
Cc: linux-kernel, netdev, bpf, clang-built-linux, Ivan Khoronzhuk
In-Reply-To: <20190910103830.20794-1-ivan.khoronzhuk@linaro.org>
No need in hacking HOSTCC to be cross-compiler any more, so drop
this trick and use CC for HDR_PROBE
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
samples/bpf/Makefile | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 625a71f2e9d2..79c9aa41832e 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -209,15 +209,14 @@ BTF_PAHOLE ?= pahole
# Detect that we're cross compiling and use the cross compiler
ifdef CROSS_COMPILE
-HOSTCC = $(CROSS_COMPILE)gcc
CLANG_ARCH_ARGS = --target=$(notdir $(CROSS_COMPILE:%-=%))
endif
# Don't evaluate probes and warnings if we need to run make recursively
ifneq ($(src),)
HDR_PROBE := $(shell printf "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \
- $(HOSTCC) $(KBUILD_HOSTCFLAGS) -x c - -o /dev/null 2>/dev/null && \
- echo okay)
+ $(CC) $(PROGS_CFLAGS) $(PROGS_LDFLAGS) -x c - -o /dev/null 2>/dev/null \
+ && echo okay)
ifeq ($(HDR_PROBE),)
$(warning WARNING: Detected possible issues with include path.)
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox