* [PATCH net-next 7/7] net: openvswitch: add likely in flow_lookup
From: xiangxia.m.yue @ 2019-09-19 16:54 UTC (permalink / raw)
To: pshelar, gvrose8192; +Cc: netdev, Tonghao Zhang
In-Reply-To: <1568912093-68535-1-git-send-email-xiangxia.m.yue@gmail.com>
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
The most case *index < ma->max, we add likely for performance.
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
net/openvswitch/flow_table.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
index 223470a..bd7e976 100644
--- a/net/openvswitch/flow_table.c
+++ b/net/openvswitch/flow_table.c
@@ -526,7 +526,7 @@ static struct sw_flow *flow_lookup(struct flow_table *tbl,
struct sw_flow_mask *mask;
int i;
- if (*index < ma->max) {
+ if (likely(*index < ma->max)) {
mask = rcu_dereference_ovsl(ma->masks[*index]);
if (mask) {
flow = masked_flow_lookup(ti, key, mask, n_mask_hit);
--
1.8.3.1
^ permalink raw reply related
* [PATCH 1/2] can: D_CAN: perform a sofware reset on open
From: Jeroen Hofstee @ 2019-09-26 8:50 UTC (permalink / raw)
To: linux-can@vger.kernel.org
Cc: Jeroen Hofstee, Wolfgang Grandegger, Marc Kleine-Budde,
David S. Miller, open list:NETWORKING DRIVERS, open list
In-Reply-To: <20190926085005.24805-1-jhofstee@victronenergy.com>
When the C_CAN interface is closed it is put in power down mode, but
does not reset the error counters / state. So reset the D_CAN on open,
so the reported state and the actual state match.
According to [1], the C_CAN module doesn't have the software reset.
[1] http://www.bosch-semiconductors.com/media/ip_modules/pdf_2/c_can_fd8/users_manual_c_can_fd8_r210_1.pdf
Signed-off-by: Jeroen Hofstee <jhofstee@victronenergy.com>
---
drivers/net/can/c_can/c_can.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 606b7d8ffe13..502a181d02e7 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -52,6 +52,7 @@
#define CONTROL_EX_PDR BIT(8)
/* control register */
+#define CONTROL_SWR BIT(15)
#define CONTROL_TEST BIT(7)
#define CONTROL_CCE BIT(6)
#define CONTROL_DISABLE_AR BIT(5)
@@ -569,6 +570,26 @@ static void c_can_configure_msg_objects(struct net_device *dev)
IF_MCONT_RCV_EOB);
}
+static int software_reset(struct net_device *dev)
+{
+ struct c_can_priv *priv = netdev_priv(dev);
+ int retry = 0;
+
+ if (priv->type != BOSCH_D_CAN)
+ return 0;
+
+ priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_SWR | CONTROL_INIT);
+ while (priv->read_reg(priv, C_CAN_CTRL_REG) & CONTROL_SWR) {
+ msleep(20);
+ if (retry++ > 100) {
+ netdev_err(dev, "CCTRL: software reset failed\n");
+ return -EIO;
+ }
+ }
+
+ return 0;
+}
+
/*
* Configure C_CAN chip:
* - enable/disable auto-retransmission
@@ -578,6 +599,11 @@ static void c_can_configure_msg_objects(struct net_device *dev)
static int c_can_chip_config(struct net_device *dev)
{
struct c_can_priv *priv = netdev_priv(dev);
+ int err;
+
+ err = software_reset(dev);
+ if (err)
+ return err;
/* enable automatic retransmission */
priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_ENABLE_AR);
--
2.17.1
^ permalink raw reply related
* [PATCH 2/2] can: C_CAN: add bus recovery events
From: Jeroen Hofstee @ 2019-09-26 8:50 UTC (permalink / raw)
To: linux-can@vger.kernel.org
Cc: Jeroen Hofstee, Wolfgang Grandegger, Marc Kleine-Budde,
David S. Miller, open list:NETWORKING DRIVERS, open list
In-Reply-To: <20190926085005.24805-1-jhofstee@victronenergy.com>
While the state is update when the error counters increase and decrease,
there is no event when the bus recovers and the error counters decrease
again. So add that event as well.
Change the state going downward to be ERROR_PASSIVE -> ERROR_WARNING ->
ERROR_ACTIVE instead of directly to ERROR_ACTIVE again.
Signed-off-by: Jeroen Hofstee <jhofstee@victronenergy.com>
---
drivers/net/can/c_can/c_can.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 502a181d02e7..5cfaab18e10b 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -912,6 +912,9 @@ static int c_can_handle_state_change(struct net_device *dev,
struct can_berr_counter bec;
switch (error_type) {
+ case C_CAN_NO_ERROR:
+ priv->can.state = CAN_STATE_ERROR_ACTIVE;
+ break;
case C_CAN_ERROR_WARNING:
/* error warning state */
priv->can.can_stats.error_warning++;
@@ -942,6 +945,13 @@ static int c_can_handle_state_change(struct net_device *dev,
ERR_CNT_RP_SHIFT;
switch (error_type) {
+ case C_CAN_NO_ERROR:
+ /* error warning state */
+ cf->can_id |= CAN_ERR_CRTL;
+ cf->data[1] = CAN_ERR_CRTL_ACTIVE;
+ cf->data[6] = bec.txerr;
+ cf->data[7] = bec.rxerr;
+ break;
case C_CAN_ERROR_WARNING:
/* error warning state */
cf->can_id |= CAN_ERR_CRTL;
@@ -1080,11 +1090,17 @@ static int c_can_poll(struct napi_struct *napi, int quota)
/* handle bus recovery events */
if ((!(curr & STATUS_BOFF)) && (last & STATUS_BOFF)) {
netdev_dbg(dev, "left bus off state\n");
- priv->can.state = CAN_STATE_ERROR_ACTIVE;
+ work_done += c_can_handle_state_change(dev, C_CAN_ERROR_PASSIVE);
}
+
if ((!(curr & STATUS_EPASS)) && (last & STATUS_EPASS)) {
netdev_dbg(dev, "left error passive state\n");
- priv->can.state = CAN_STATE_ERROR_ACTIVE;
+ work_done += c_can_handle_state_change(dev, C_CAN_ERROR_WARNING);
+ }
+
+ if ((!(curr & STATUS_EWARN)) && (last & STATUS_EWARN)) {
+ netdev_dbg(dev, "left error warning state\n");
+ work_done += c_can_handle_state_change(dev, C_CAN_NO_ERROR);
}
/* handle lec errors on the bus */
--
2.17.1
^ permalink raw reply related
* [PATCH net] net: dsa: qca8k: Use up to 7 ports for all operations
From: Michal Vokáč @ 2019-09-26 8:59 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, netdev
Cc: Vivien Didelot, linux-kernel, Florian Fainelli, Heiner Kallweit,
Michal Vokáč
The QCA8K family supports up to 7 ports. So use the existing
QCA8K_NUM_PORTS define to allocate the switch structure and limit all
operations with the switch ports.
This was not an issue until commit 0394a63acfe2 ("net: dsa: enable and
disable all ports") disabled all unused ports. Since the unused ports 7-11
are outside of the correct register range on this switch some registers
were rewritten with invalid content.
Fixes: 6b93fb46480a ("net-next: dsa: add new driver for qca8xxx family")
Fixes: a0c02161ecfc ("net: dsa: variable number of ports")
Fixes: 0394a63acfe2 ("net: dsa: enable and disable all ports")
Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
---
I am not sure which of the fixes tags should be used but this definetelly
fixes something..
IMHO the 0394a63acfe2 ("net: dsa: enable and disable all ports") did not
cause the issue but made it visible.
drivers/net/dsa/qca8k.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 16f15c93a102..bbeeb8618c80 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -705,7 +705,7 @@ qca8k_setup(struct dsa_switch *ds)
BIT(0) << QCA8K_GLOBAL_FW_CTRL1_UC_DP_S);
/* Setup connection between CPU port & user ports */
- for (i = 0; i < DSA_MAX_PORTS; i++) {
+ for (i = 0; i < QCA8K_NUM_PORTS; i++) {
/* CPU port gets connected to all user ports of the switch */
if (dsa_is_cpu_port(ds, i)) {
qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(QCA8K_CPU_PORT),
@@ -1074,7 +1074,7 @@ qca8k_sw_probe(struct mdio_device *mdiodev)
if (id != QCA8K_ID_QCA8337)
return -ENODEV;
- priv->ds = dsa_switch_alloc(&mdiodev->dev, DSA_MAX_PORTS);
+ priv->ds = dsa_switch_alloc(&mdiodev->dev, QCA8K_NUM_PORTS);
if (!priv->ds)
return -ENOMEM;
--
2.1.4
^ permalink raw reply related
* [PATCH net] devlink: Fix error handling in param and info_get dumpit cb
From: Vasundhara Volam @ 2019-09-26 9:35 UTC (permalink / raw)
To: davem; +Cc: netdev, Vasundhara Volam, Jiri Pirko, Michael Chan
If any of the param or info_get op returns error, dumpit cb is
skipping to dump remaining params or info_get ops for all the
drivers.
Instead skip only for the param/info_get op which returned error
and continue to dump remaining information, except if the return
code is EMSGSIZE.
Cc: Jiri Pirko <jiri@mellanox.com>
Cc: Michael Chan <michael.chan@broadcom.com>
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
---
net/core/devlink.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index e48680e..a1dd1b8 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -3172,7 +3172,7 @@ static int devlink_nl_cmd_param_get_dumpit(struct sk_buff *msg,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq,
NLM_F_MULTI);
- if (err) {
+ if (err == -EMSGSIZE) {
mutex_unlock(&devlink->lock);
goto out;
}
@@ -3432,7 +3432,7 @@ static int devlink_nl_cmd_port_param_get_dumpit(struct sk_buff *msg,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq,
NLM_F_MULTI);
- if (err) {
+ if (err == -EMSGSIZE) {
mutex_unlock(&devlink->lock);
goto out;
}
@@ -4088,7 +4088,7 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
cb->nlh->nlmsg_seq, NLM_F_MULTI,
cb->extack);
mutex_unlock(&devlink->lock);
- if (err)
+ if (err == -EMSGSIZE)
break;
idx++;
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next 0/7] optimize openvswitch flow looking up
From: David Miller @ 2019-09-26 10:14 UTC (permalink / raw)
To: xiangxia.m.yue; +Cc: pshelar, gvrose8192, netdev
In-Reply-To: <1568912093-68535-1-git-send-email-xiangxia.m.yue@gmail.com>
From: xiangxia.m.yue@gmail.com
Date: Fri, 20 Sep 2019 00:54:46 +0800
> This series patch optimize openvswitch.
Please resubmit this when net-next opens back up.
Thnak you.
^ permalink raw reply
* chapoly acceleration hardware [Was: Re: [RFC PATCH 00/18] crypto: wireguard using the existing crypto API]
From: Jason A. Donenfeld @ 2019-09-26 11:06 UTC (permalink / raw)
To: Pascal Van Leeuwen
Cc: Ard Biesheuvel, Linux Crypto Mailing List, linux-arm-kernel,
Herbert Xu, David Miller, Greg KH, Linus Torvalds, Samuel Neves,
Dan Carpenter, Arnd Bergmann, Eric Biggers, Andy Lutomirski,
Will Deacon, Marc Zyngier, Catalin Marinas, Willy Tarreau, Netdev,
Toke Høiland-Jørgensen, Dave Taht
In-Reply-To: <MN2PR20MB29733663686FB38153BAE7EACA860@MN2PR20MB2973.namprd20.prod.outlook.com>
[CC +willy, toke, dave, netdev]
Hi Pascal
On Thu, Sep 26, 2019 at 12:19 PM Pascal Van Leeuwen
<pvanleeuwen@verimatrix.com> wrote:
> Actually, that assumption is factually wrong. I don't know if anything
> is *publicly* available, but I can assure you the silicon is running in
> labs already. And something will be publicly available early next year
> at the latest. Which could nicely coincide with having Wireguard support
> in the kernel (which I would also like to see happen BTW) ...
>
> Not "at some point". It will. Very soon. Maybe not in consumer or server
> CPUs, but definitely in the embedded (networking) space.
> And it *will* be much faster than the embedded CPU next to it, so it will
> be worth using it for something like bulk packet encryption.
Super! I was wondering if you could speak a bit more about the
interface. My biggest questions surround latency. Will it be
synchronous or asynchronous? If the latter, why? What will its
latencies be? How deep will its buffers be? The reason I ask is that a
lot of crypto acceleration hardware of the past has been fast and
having very deep buffers, but at great expense of latency. In the
networking context, keeping latency low is pretty important. Already
WireGuard is multi-threaded which isn't super great all the time for
latency (improvements are a work in progress). If you're involved with
the design of the hardware, perhaps this is something you can help
ensure winds up working well? For example, AES-NI is straightforward
and good, but Intel can do that because they are the CPU. It sounds
like your silicon will be adjacent. How do you envision this working
in a low latency environment?
Jason
^ permalink raw reply
* [PATCH] NFC: st95hf: clean up indentation issue
From: Colin King @ 2019-09-26 11:13 UTC (permalink / raw)
To: David S . Miller, netdev; +Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The return statement is indented incorrectly, add in a missing
tab and remove an extraneous space after the return
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/nfc/st95hf/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nfc/st95hf/core.c b/drivers/nfc/st95hf/core.c
index ce38782ebf80..291efff048af 100644
--- a/drivers/nfc/st95hf/core.c
+++ b/drivers/nfc/st95hf/core.c
@@ -661,7 +661,7 @@ static int st95hf_error_handling(struct st95hf_context *stcontext,
result = -ETIMEDOUT;
else
result = -EIO;
- return result;
+ return result;
}
/* Check for CRC err only if CRC is present in the tag response */
--
2.20.1
^ permalink raw reply related
* [PATCH] net: ena: clean up indentation issue
From: Colin King @ 2019-09-26 11:22 UTC (permalink / raw)
To: Netanel Belgazal, Saeed Bishara, Zorik Machulsky,
David S . Miller, Arthur Kiyanovski, Sameeh Jubran, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
There memset is indented incorrectly, remove the extraneous tabs.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/amazon/ena/ena_eth_com.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/amazon/ena/ena_eth_com.c b/drivers/net/ethernet/amazon/ena/ena_eth_com.c
index 38046bf0ff44..2845ac277724 100644
--- a/drivers/net/ethernet/amazon/ena/ena_eth_com.c
+++ b/drivers/net/ethernet/amazon/ena/ena_eth_com.c
@@ -211,8 +211,8 @@ static int ena_com_sq_update_llq_tail(struct ena_com_io_sq *io_sq)
pkt_ctrl->curr_bounce_buf =
ena_com_get_next_bounce_buffer(&io_sq->bounce_buf_ctrl);
- memset(io_sq->llq_buf_ctrl.curr_bounce_buf,
- 0x0, llq_info->desc_list_entry_size);
+ memset(io_sq->llq_buf_ctrl.curr_bounce_buf,
+ 0x0, llq_info->desc_list_entry_size);
pkt_ctrl->idx = 0;
if (unlikely(llq_info->desc_stride_ctrl == ENA_ADMIN_SINGLE_DESC_PER_ENTRY))
--
2.20.1
^ permalink raw reply related
* Are BPF tail calls only supposed to work with pinned maps?
From: Toke Høiland-Jørgensen @ 2019-09-26 11:23 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: netdev, bpf
Hi Daniel
While working on a prototype of the XDP chain call feature, I ran into
some strange behaviour with tail calls: If I create a userspace program
that loads two XDP programs, one of which tail calls the other, the tail
call map would appear to be empty even though the userspace program
populates it as part of the program loading.
I eventually tracked this down to this commit:
c9da161c6517 ("bpf: fix clearing on persistent program array maps")
Which clears PROG_ARRAY maps whenever the last uref to it disappears
(which it does when my loader exits after attaching the XDP program).
This effectively means that tail calls only work if the PROG_ARRAY map
is pinned (or the process creating it keeps running). And as far as I
can tell, the inner_map reference in bpf_map_fd_get_ptr() doesn't bump
the uref either, so presumably if one were to create a map-in-map
construct with tail call pointer in the inner map(s), each inner map
would also need to be pinned (haven't tested this case)?
Is this really how things are supposed to work? From an XDP use case PoV
this seems somewhat surprising...
Or am I missing something obvious here?
-Toke
^ permalink raw reply
* Re: chapoly acceleration hardware [Was: Re: [RFC PATCH 00/18] crypto: wireguard using the existing crypto API]
From: Toke Høiland-Jørgensen @ 2019-09-26 11:38 UTC (permalink / raw)
To: Jason A. Donenfeld, Pascal Van Leeuwen
Cc: Ard Biesheuvel, Linux Crypto Mailing List, linux-arm-kernel,
Herbert Xu, David Miller, Greg KH, Linus Torvalds, Samuel Neves,
Dan Carpenter, Arnd Bergmann, Eric Biggers, Andy Lutomirski,
Will Deacon, Marc Zyngier, Catalin Marinas, Willy Tarreau, Netdev,
Dave Taht
In-Reply-To: <CAHmME9r5m7D-oMU6Lv_ZhEyWmrNscMr5HokzdK0wg2Ayzzbeow@mail.gmail.com>
"Jason A. Donenfeld" <Jason@zx2c4.com> writes:
> [CC +willy, toke, dave, netdev]
>
> Hi Pascal
>
> On Thu, Sep 26, 2019 at 12:19 PM Pascal Van Leeuwen
> <pvanleeuwen@verimatrix.com> wrote:
>> Actually, that assumption is factually wrong. I don't know if anything
>> is *publicly* available, but I can assure you the silicon is running in
>> labs already. And something will be publicly available early next year
>> at the latest. Which could nicely coincide with having Wireguard support
>> in the kernel (which I would also like to see happen BTW) ...
>>
>> Not "at some point". It will. Very soon. Maybe not in consumer or server
>> CPUs, but definitely in the embedded (networking) space.
>> And it *will* be much faster than the embedded CPU next to it, so it will
>> be worth using it for something like bulk packet encryption.
>
> Super! I was wondering if you could speak a bit more about the
> interface. My biggest questions surround latency. Will it be
> synchronous or asynchronous? If the latter, why? What will its
> latencies be? How deep will its buffers be? The reason I ask is that a
> lot of crypto acceleration hardware of the past has been fast and
> having very deep buffers, but at great expense of latency. In the
> networking context, keeping latency low is pretty important. Already
> WireGuard is multi-threaded which isn't super great all the time for
> latency (improvements are a work in progress). If you're involved with
> the design of the hardware, perhaps this is something you can help
> ensure winds up working well? For example, AES-NI is straightforward
> and good, but Intel can do that because they are the CPU. It sounds
> like your silicon will be adjacent. How do you envision this working
> in a low latency environment?
Being asynchronous doesn't *necessarily* have to hurt latency; you just
need the right queue back-pressure.
We already have multiple queues in the stack. With an async crypto
engine we would go from something like:
stack -> [qdisc] -> wg if -> [wireguard buffer] -> netdev driver ->
device -> [device buffer] -> wire
to
stack -> [qdisc] -> wg if -> [wireguard buffer] -> crypto stack ->
crypto device -> [crypto device buffer] -> wg post-crypto -> netdev
driver -> device -> [device buffer] -> wire
(where everything in [] is a packet queue).
The wireguard buffer is the source of the latency you're alluding to
above (the comment about multi-threaded behaviour), so we probably need
to fix that anyway. For the device buffer we have BQL to keep it at a
minimum. So that leaves the buffering in the crypto offload device. If
we add something like BQL to the crypto offload drivers, we could
conceivably avoid having that add a significant amount of latency. In
fact, doing so may benefit other users of crypto offloads as well, no?
Presumably ipsec has this same issue?
Caveat: I am fairly ignorant about the inner workings of the crypto
subsystem, so please excuse any inaccuracies in the above; the diagrams
are solely for illustrative purposes... :)
-Toke
^ permalink raw reply
* [RFC PATCH v4 0/6] Enable ptp_kvm for arm64
From: Jianyong Wu @ 2019-09-26 11:42 UTC (permalink / raw)
To: netdev, yangbo.lu, john.stultz, tglx, pbonzini,
sean.j.christopherson, maz, richardcochran, Mark.Rutland,
Will.Deacon, suzuki.poulose
Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, Steve.Capper,
Kaly.Xin, justin.he, jianyong.wu, nd
kvm ptp targets to provide high precision time sync between guest
and host in virtualization environment. This patch enable kvm ptp
for arm64.
This patch set base on [1][2][3]
change log:
from v3 to v4:
(1) fix clocksource of ptp_kvm to arch_sys_counter.
(2) move kvm_arch_ptp_get_clock_fn into arm_arch_timer.c
(3) subtract cntvoff before return cycles from host.
(4) use ktime_get_snapshot instead of getnstimeofday and
get_current_counterval to return time and counter value.
(5) split ktime and counter into two 32-bit block respectively
to avoid Y2038-safe issue.
(6) set time compensation to device time as half of the delay of hvc call.
(7) add ARM_ARCH_TIMER as dependency of ptp_kvm for
arm64.
from v2 to v3:
(1) fix some issues in commit log.
(2) add some receivers in send list.
from v1 to v2:
(1) move arch-specific code from arch/ to driver/ptp/
(2) offer mechanism to inform userspace if ptp_kvm service is
available.
(3) separate ptp_kvm code for arm64 into hypervisor part and
guest part.
(4) add API to expose monotonic clock and counter value.
(5) refine code: remove no necessary part and reconsitution.
[1]https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/
commit/?h=kvm/hvc&id=125ea89e4a21e2fc5235410f966a996a1a7148bf
[2]https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/
commit/?h=kvm/hvc&id=464f5a1741e5959c3e4d2be1966ae0093b4dce06
[3]https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/
commit/?h=kvm/hvc&id=6597490e005d0eeca8ed8c1c1d7b4318ee014681
Jianyong Wu (5):
psci: Export psci_ops.conduit symbol as modules will use it.
ptp: Reorganize ptp_kvm modules to make it arch-independent.
psci: Add hvc call service for ptp_kvm.
ptp: arm64: Enable ptp_kvm for arm64
kvm: arm64: Add capability check extension for ptp_kvm
drivers/clocksource/arm_arch_timer.c | 28 +++++++++
drivers/firmware/psci/psci.c | 6 ++
drivers/ptp/Kconfig | 2 +-
drivers/ptp/Makefile | 1 +
drivers/ptp/{ptp_kvm.c => kvm_ptp.c} | 79 +++++++------------------
drivers/ptp/ptp_kvm_arm64.c | 73 +++++++++++++++++++++++
drivers/ptp/ptp_kvm_x86.c | 87 ++++++++++++++++++++++++++++
include/asm-generic/ptp_kvm.h | 12 ++++
include/linux/arm-smccc.h | 14 ++++-
include/linux/psci.h | 1 +
include/uapi/linux/kvm.h | 1 +
virt/kvm/arm/arm.c | 1 +
virt/kvm/arm/psci.c | 18 ++++++
13 files changed, 262 insertions(+), 61 deletions(-)
rename drivers/ptp/{ptp_kvm.c => kvm_ptp.c} (63%)
create mode 100644 drivers/ptp/ptp_kvm_arm64.c
create mode 100644 drivers/ptp/ptp_kvm_x86.c
create mode 100644 include/asm-generic/ptp_kvm.h
--
2.17.1
^ permalink raw reply
* [RFC PATCH v4 1/5] psci: Export psci_ops.conduit symbol as modules will use it.
From: Jianyong Wu @ 2019-09-26 11:42 UTC (permalink / raw)
To: netdev, yangbo.lu, john.stultz, tglx, pbonzini,
sean.j.christopherson, maz, richardcochran, Mark.Rutland,
Will.Deacon, suzuki.poulose
Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, Steve.Capper,
Kaly.Xin, justin.he, jianyong.wu, nd
In-Reply-To: <20190926114212.5322-1-jianyong.wu@arm.com>
If arm_smccc_1_1_invoke used in modules, psci_ops.conduit should
be export.
Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
---
drivers/firmware/psci/psci.c | 6 ++++++
include/linux/arm-smccc.h | 2 +-
include/linux/psci.h | 1 +
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index f82ccd39a913..35c4eaab1451 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -212,6 +212,12 @@ static unsigned long psci_migrate_info_up_cpu(void)
0, 0, 0);
}
+enum psci_conduit psci_get_conduit(void)
+{
+ return psci_ops.conduit;
+}
+EXPORT_SYMBOL(psci_get_conduit);
+
static void set_conduit(enum psci_conduit conduit)
{
switch (conduit) {
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 552cbd49abe8..a6e4d3e3d10a 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -357,7 +357,7 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
* The return value also provides the conduit that was used.
*/
#define arm_smccc_1_1_invoke(...) ({ \
- int method = psci_ops.conduit; \
+ int method = psci_get_conduit(); \
switch (method) { \
case PSCI_CONDUIT_HVC: \
arm_smccc_1_1_hvc(__VA_ARGS__); \
diff --git a/include/linux/psci.h b/include/linux/psci.h
index a8a15613c157..e5cedc986049 100644
--- a/include/linux/psci.h
+++ b/include/linux/psci.h
@@ -42,6 +42,7 @@ struct psci_operations {
enum smccc_version smccc_version;
};
+extern enum psci_conduit psci_get_conduit(void);
extern struct psci_operations psci_ops;
#if defined(CONFIG_ARM_PSCI_FW)
--
2.17.1
^ permalink raw reply related
* [RFC PATCH v4 2/5] ptp: Reorganize ptp_kvm modules to make it arch-independent.
From: Jianyong Wu @ 2019-09-26 11:42 UTC (permalink / raw)
To: netdev, yangbo.lu, john.stultz, tglx, pbonzini,
sean.j.christopherson, maz, richardcochran, Mark.Rutland,
Will.Deacon, suzuki.poulose
Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, Steve.Capper,
Kaly.Xin, justin.he, jianyong.wu, nd
In-Reply-To: <20190926114212.5322-1-jianyong.wu@arm.com>
Currently, ptp_kvm modules implementation is only for x86 which includs
large part of arch-specific code. This patch move all of those code
into new arch related file in the same directory.
Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
---
drivers/ptp/Makefile | 1 +
drivers/ptp/{ptp_kvm.c => kvm_ptp.c} | 77 ++++++------------------
drivers/ptp/ptp_kvm_x86.c | 87 ++++++++++++++++++++++++++++
include/asm-generic/ptp_kvm.h | 12 ++++
4 files changed, 118 insertions(+), 59 deletions(-)
rename drivers/ptp/{ptp_kvm.c => kvm_ptp.c} (63%)
create mode 100644 drivers/ptp/ptp_kvm_x86.c
create mode 100644 include/asm-generic/ptp_kvm.h
diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile
index 677d1d178a3e..8f27ba302e31 100644
--- a/drivers/ptp/Makefile
+++ b/drivers/ptp/Makefile
@@ -4,6 +4,7 @@
#
ptp-y := ptp_clock.o ptp_chardev.o ptp_sysfs.o
+ptp_kvm-y := ptp_kvm_$(ARCH).o kvm_ptp.o
obj-$(CONFIG_PTP_1588_CLOCK) += ptp.o
obj-$(CONFIG_PTP_1588_CLOCK_DTE) += ptp_dte.o
obj-$(CONFIG_PTP_1588_CLOCK_IXP46X) += ptp_ixp46x.o
diff --git a/drivers/ptp/ptp_kvm.c b/drivers/ptp/kvm_ptp.c
similarity index 63%
rename from drivers/ptp/ptp_kvm.c
rename to drivers/ptp/kvm_ptp.c
index fc7d0b77e118..d8f215186904 100644
--- a/drivers/ptp/ptp_kvm.c
+++ b/drivers/ptp/kvm_ptp.c
@@ -8,12 +8,12 @@
#include <linux/err.h>
#include <linux/init.h>
#include <linux/kernel.h>
+#include <linux/slab.h>
#include <linux/module.h>
#include <uapi/linux/kvm_para.h>
#include <asm/kvm_para.h>
-#include <asm/pvclock.h>
-#include <asm/kvmclock.h>
#include <uapi/asm/kvm_para.h>
+#include <asm-generic/ptp_kvm.h>
#include <linux/ptp_clock_kernel.h>
@@ -24,56 +24,29 @@ struct kvm_ptp_clock {
DEFINE_SPINLOCK(kvm_ptp_lock);
-static struct pvclock_vsyscall_time_info *hv_clock;
-
-static struct kvm_clock_pairing clock_pair;
-static phys_addr_t clock_pair_gpa;
-
static int ptp_kvm_get_time_fn(ktime_t *device_time,
struct system_counterval_t *system_counter,
void *ctx)
{
- unsigned long ret;
+ unsigned long ret, cycle;
struct timespec64 tspec;
- unsigned version;
- int cpu;
- struct pvclock_vcpu_time_info *src;
+ struct clocksource *cs;
spin_lock(&kvm_ptp_lock);
preempt_disable_notrace();
- cpu = smp_processor_id();
- src = &hv_clock[cpu].pvti;
-
- do {
- /*
- * We are using a TSC value read in the hosts
- * kvm_hc_clock_pairing handling.
- * So any changes to tsc_to_system_mul
- * and tsc_shift or any other pvclock
- * data invalidate that measurement.
- */
- version = pvclock_read_begin(src);
-
- ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING,
- clock_pair_gpa,
- KVM_CLOCK_PAIRING_WALLCLOCK);
- if (ret != 0) {
- pr_err_ratelimited("clock pairing hypercall ret %lu\n", ret);
- spin_unlock(&kvm_ptp_lock);
- preempt_enable_notrace();
- return -EOPNOTSUPP;
- }
-
- tspec.tv_sec = clock_pair.sec;
- tspec.tv_nsec = clock_pair.nsec;
- ret = __pvclock_read_cycles(src, clock_pair.tsc);
- } while (pvclock_read_retry(src, version));
+ ret = kvm_arch_ptp_get_clock_fn(&cycle, &tspec, &cs);
+ if (ret != 0) {
+ pr_err_ratelimited("clock pairing hypercall ret %lu\n", ret);
+ spin_unlock(&kvm_ptp_lock);
+ preempt_enable_notrace();
+ return -EOPNOTSUPP;
+ }
preempt_enable_notrace();
- system_counter->cycles = ret;
- system_counter->cs = &kvm_clock;
+ system_counter->cycles = cycle;
+ system_counter->cs = cs;
*device_time = timespec64_to_ktime(tspec);
@@ -116,17 +89,13 @@ static int ptp_kvm_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
spin_lock(&kvm_ptp_lock);
- ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING,
- clock_pair_gpa,
- KVM_CLOCK_PAIRING_WALLCLOCK);
+ ret = kvm_arch_ptp_get_clock(&tspec);
if (ret != 0) {
pr_err_ratelimited("clock offset hypercall ret %lu\n", ret);
spin_unlock(&kvm_ptp_lock);
return -EOPNOTSUPP;
}
- tspec.tv_sec = clock_pair.sec;
- tspec.tv_nsec = clock_pair.nsec;
spin_unlock(&kvm_ptp_lock);
memcpy(ts, &tspec, sizeof(struct timespec64));
@@ -166,21 +135,11 @@ static void __exit ptp_kvm_exit(void)
static int __init ptp_kvm_init(void)
{
- long ret;
-
- if (!kvm_para_available())
- return -ENODEV;
-
- clock_pair_gpa = slow_virt_to_phys(&clock_pair);
- hv_clock = pvclock_get_pvti_cpu0_va();
+ int ret;
- if (!hv_clock)
- return -ENODEV;
-
- ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING, clock_pair_gpa,
- KVM_CLOCK_PAIRING_WALLCLOCK);
- if (ret == -KVM_ENOSYS || ret == -KVM_EOPNOTSUPP)
- return -ENODEV;
+ ret = kvm_arch_ptp_init();
+ if (!ret)
+ return -EOPNOTSUPP;
kvm_ptp_clock.caps = ptp_kvm_caps;
diff --git a/drivers/ptp/ptp_kvm_x86.c b/drivers/ptp/ptp_kvm_x86.c
new file mode 100644
index 000000000000..a52cf1c2990c
--- /dev/null
+++ b/drivers/ptp/ptp_kvm_x86.c
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Virtual PTP 1588 clock for use with KVM guests
+ *
+ * Copyright (C) 2017 Red Hat Inc.
+ */
+
+#include <asm/pvclock.h>
+#include <asm/kvmclock.h>
+#include <linux/module.h>
+#include <uapi/asm/kvm_para.h>
+#include <uapi/linux/kvm_para.h>
+#include <linux/ptp_clock_kernel.h>
+
+phys_addr_t clock_pair_gpa;
+struct kvm_clock_pairing clock_pair;
+struct pvclock_vsyscall_time_info *hv_clock;
+
+int kvm_arch_ptp_init(void)
+{
+ int ret;
+
+ if (!kvm_para_available())
+ return -ENODEV;
+
+ clock_pair_gpa = slow_virt_to_phys(&clock_pair);
+ hv_clock = pvclock_get_pvti_cpu0_va();
+ if (!hv_clock)
+ return -ENODEV;
+
+ ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING, clock_pair_gpa,
+ KVM_CLOCK_PAIRING_WALLCLOCK);
+ if (ret == -KVM_ENOSYS || ret == -KVM_EOPNOTSUPP)
+ return -ENODEV;
+
+ return 0;
+}
+
+int kvm_arch_ptp_get_clock(struct timespec64 *ts)
+{
+ long ret;
+
+ ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING,
+ clock_pair_gpa,
+ KVM_CLOCK_PAIRING_WALLCLOCK);
+ if (ret != 0)
+ return -EOPNOTSUPP;
+
+ ts->tv_sec = clock_pair.sec;
+ ts->tv_nsec = clock_pair.nsec;
+
+ return 0;
+}
+
+int kvm_arch_ptp_get_clock_fn(unsigned long *cycle, struct timespec64 *tspec,
+ struct clocksource **cs)
+{
+ unsigned long ret;
+ unsigned int version;
+ int cpu;
+ struct pvclock_vcpu_time_info *src;
+
+ cpu = smp_processor_id();
+ src = &hv_clock[cpu].pvti;
+
+ do {
+ /*
+ * We are using a TSC value read in the hosts
+ * kvm_hc_clock_pairing handling.
+ * So any changes to tsc_to_system_mul
+ * and tsc_shift or any other pvclock
+ * data invalidate that measurement.
+ */
+ version = pvclock_read_begin(src);
+
+ ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING,
+ clock_pair_gpa,
+ KVM_CLOCK_PAIRING_WALLCLOCK);
+ tspec->tv_sec = clock_pair.sec;
+ tspec->tv_nsec = clock_pair.nsec;
+ *cycle = __pvclock_read_cycles(src, clock_pair.tsc);
+ } while (pvclock_read_retry(src, version));
+
+ *cs = &kvm_clock;
+
+ return 0;
+}
diff --git a/include/asm-generic/ptp_kvm.h b/include/asm-generic/ptp_kvm.h
new file mode 100644
index 000000000000..208e842bfa64
--- /dev/null
+++ b/include/asm-generic/ptp_kvm.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Virtual PTP 1588 clock for use with KVM guests
+ *
+ * Copyright (C) 2019 ARM Ltd.
+ * All Rights Reserved
+ */
+
+int kvm_arch_ptp_init(void);
+int kvm_arch_ptp_get_clock(struct timespec64 *ts);
+int kvm_arch_ptp_get_clock_fn(long *cycle,
+ struct timespec64 *tspec, void *cs);
--
2.17.1
^ permalink raw reply related
* [RFC PATCH v4 3/5] psci: Add hvc call service for ptp_kvm.
From: Jianyong Wu @ 2019-09-26 11:42 UTC (permalink / raw)
To: netdev, yangbo.lu, john.stultz, tglx, pbonzini,
sean.j.christopherson, maz, richardcochran, Mark.Rutland,
Will.Deacon, suzuki.poulose
Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, Steve.Capper,
Kaly.Xin, justin.he, jianyong.wu, nd
In-Reply-To: <20190926114212.5322-1-jianyong.wu@arm.com>
This patch is the base of ptp_kvm for arm64.
ptp_kvm modules will call hvc to get this service.
The service offers real time and counter cycle of host for guest.
Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
---
include/linux/arm-smccc.h | 12 ++++++++++++
virt/kvm/arm/psci.c | 18 ++++++++++++++++++
2 files changed, 30 insertions(+)
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index a6e4d3e3d10a..bc0cdad10f35 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -94,6 +94,7 @@
/* KVM "vendor specific" services */
#define ARM_SMCCC_KVM_FUNC_FEATURES 0
+#define ARM_SMCCC_KVM_PTP 1
#define ARM_SMCCC_KVM_FUNC_FEATURES_2 127
#define ARM_SMCCC_KVM_NUM_FUNCS 128
@@ -103,6 +104,17 @@
ARM_SMCCC_OWNER_VENDOR_HYP, \
ARM_SMCCC_KVM_FUNC_FEATURES)
+/*
+ * This ID used for virtual ptp kvm clock and it will pass second value
+ * and nanosecond value of host real time and system counter by vcpu
+ * register to guest.
+ */
+#define ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID \
+ ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
+ ARM_SMCCC_SMC_32, \
+ ARM_SMCCC_OWNER_VENDOR_HYP, \
+ ARM_SMCCC_KVM_PTP)
+
#ifndef __ASSEMBLY__
#include <linux/linkage.h>
diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
index 0debf49bf259..3f30fc42a5ca 100644
--- a/virt/kvm/arm/psci.c
+++ b/virt/kvm/arm/psci.c
@@ -392,6 +392,8 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
u32 func_id = smccc_get_function(vcpu);
u32 val[4] = {};
u32 option;
+ u64 cycles;
+ struct system_time_snapshot systime_snapshot;
val[0] = SMCCC_RET_NOT_SUPPORTED;
@@ -431,6 +433,22 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES);
break;
+ /*
+ * This will used for virtual ptp kvm clock. three
+ * values will be passed back.
+ * reg0 stores high 32-bit host ktime;
+ * reg1 stores low 32-bit host ktime;
+ * reg2 stores high 32-bit difference of host cycles and cntvoff;
+ * reg3 stores low 32-bit difference of host cycles and cntvoff.
+ */
+ case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
+ ktime_get_snapshot(&systime_snapshot);
+ val[0] = systime_snapshot.real >> 32;
+ val[1] = systime_snapshot.real << 32 >> 32;
+ cycles = systime_snapshot.cycles - vcpu_vtimer(vcpu)->cntvoff;
+ val[2] = cycles >> 32;
+ val[3] = cycles << 32 >> 32;
+ break;
default:
return kvm_psci_call(vcpu);
}
--
2.17.1
^ permalink raw reply related
* [RFC PATCH v4 4/5] ptp: arm64: Enable ptp_kvm for arm64
From: Jianyong Wu @ 2019-09-26 11:42 UTC (permalink / raw)
To: netdev, yangbo.lu, john.stultz, tglx, pbonzini,
sean.j.christopherson, maz, richardcochran, Mark.Rutland,
Will.Deacon, suzuki.poulose
Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, Steve.Capper,
Kaly.Xin, justin.he, jianyong.wu, nd
In-Reply-To: <20190926114212.5322-1-jianyong.wu@arm.com>
Currently in arm64 virtualization environment, there is no mechanism to
keep time sync between guest and host. Time in guest will drift compared
with host after boot up as they may both use third party time sources
to correct their time respectively. The time deviation will be in order
of milliseconds but some scenarios ask for higher time precision, like
in cloud envirenment, we want all the VMs running in the host aquire the
same level accuracy from host clock.
Use of kvm ptp clock, which choose the host clock source clock as a
reference clock to sync time clock between guest and host has been adopted
by x86 which makes the time sync order from milliseconds to nanoseconds.
This patch enable kvm ptp on arm64 and we get the similar clock drift as
found with x86 with kvm ptp.
Test result comparison between with kvm ptp and without it in arm64 are
as follows. This test derived from the result of command 'chronyc
sources'. we should take more cure of the last sample column which shows
the offset between the local clock and the source at the last measurement.
no kvm ptp in guest:
MS Name/IP address Stratum Poll Reach LastRx Last sample
========================================================================
^* dns1.synet.edu.cn 2 6 377 13 +1040us[+1581us] +/- 21ms
^* dns1.synet.edu.cn 2 6 377 21 +1040us[+1581us] +/- 21ms
^* dns1.synet.edu.cn 2 6 377 29 +1040us[+1581us] +/- 21ms
^* dns1.synet.edu.cn 2 6 377 37 +1040us[+1581us] +/- 21ms
^* dns1.synet.edu.cn 2 6 377 45 +1040us[+1581us] +/- 21ms
^* dns1.synet.edu.cn 2 6 377 53 +1040us[+1581us] +/- 21ms
^* dns1.synet.edu.cn 2 6 377 61 +1040us[+1581us] +/- 21ms
^* dns1.synet.edu.cn 2 6 377 4 -130us[ +796us] +/- 21ms
^* dns1.synet.edu.cn 2 6 377 12 -130us[ +796us] +/- 21ms
^* dns1.synet.edu.cn 2 6 377 20 -130us[ +796us] +/- 21ms
in host:
MS Name/IP address Stratum Poll Reach LastRx Last sample
========================================================================
^* 120.25.115.20 2 7 377 72 -470us[ -603us] +/- 18ms
^* 120.25.115.20 2 7 377 92 -470us[ -603us] +/- 18ms
^* 120.25.115.20 2 7 377 112 -470us[ -603us] +/- 18ms
^* 120.25.115.20 2 7 377 2 +872ns[-6808ns] +/- 17ms
^* 120.25.115.20 2 7 377 22 +872ns[-6808ns] +/- 17ms
^* 120.25.115.20 2 7 377 43 +872ns[-6808ns] +/- 17ms
^* 120.25.115.20 2 7 377 63 +872ns[-6808ns] +/- 17ms
^* 120.25.115.20 2 7 377 83 +872ns[-6808ns] +/- 17ms
^* 120.25.115.20 2 7 377 103 +872ns[-6808ns] +/- 17ms
^* 120.25.115.20 2 7 377 123 +872ns[-6808ns] +/- 17ms
The dns1.synet.edu.cn is the network reference clock for guest and
120.25.115.20 is the network reference clock for host. we can't get the
clock error between guest and host directly, but a roughly estimated value
will be in order of hundreds of us to ms.
with kvm ptp in guest:
chrony has been disabled in host to remove the disturb by network clock.
MS Name/IP address Stratum Poll Reach LastRx Last sample
========================================================================
* PHC0 0 3 377 8 -7ns[ +1ns] +/- 3ns
* PHC0 0 3 377 8 +1ns[ +16ns] +/- 3ns
* PHC0 0 3 377 6 -4ns[ -0ns] +/- 6ns
* PHC0 0 3 377 6 -8ns[ -12ns] +/- 5ns
* PHC0 0 3 377 5 +2ns[ +4ns] +/- 4ns
* PHC0 0 3 377 13 +2ns[ +4ns] +/- 4ns
* PHC0 0 3 377 12 -4ns[ -6ns] +/- 4ns
* PHC0 0 3 377 11 -8ns[ -11ns] +/- 6ns
* PHC0 0 3 377 10 -14ns[ -20ns] +/- 4ns
* PHC0 0 3 377 8 +4ns[ +5ns] +/- 4ns
The PHC0 is the ptp clock which choose the host clock as its source
clock. So we can be sure to say that the clock error between host and guest
is in order of ns.
Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
---
drivers/clocksource/arm_arch_timer.c | 28 +++++++++++
drivers/ptp/Kconfig | 2 +-
drivers/ptp/kvm_ptp.c | 4 +-
drivers/ptp/ptp_kvm_arm64.c | 73 ++++++++++++++++++++++++++++
include/asm-generic/ptp_kvm.h | 2 +-
5 files changed, 106 insertions(+), 3 deletions(-)
create mode 100644 drivers/ptp/ptp_kvm_arm64.c
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 07e57a49d1e8..7a99673e4149 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -1633,4 +1633,32 @@ static int __init arch_timer_acpi_init(struct acpi_table_header *table)
return arch_timer_common_init();
}
TIMER_ACPI_DECLARE(arch_timer, ACPI_SIG_GTDT, arch_timer_acpi_init);
+
+#if IS_ENABLED(CONFIG_PTP_1588_CLOCK_KVM)
+#include <linux/arm-smccc.h>
+int kvm_arch_ptp_get_clock_fn(long *cycle, struct timespec64 *ts,
+ struct clocksource **cs)
+{
+ u64 t1, t2;
+ struct arm_smccc_res hvc_res;
+ ktime_t ktime_overall;
+
+ t1 = sched_clock();
+ arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID, &hvc_res);
+ t2 = sched_clock();
+ t2 -= t1;
+ t2 /= 2;
+ if ((long)(hvc_res.a0) < 0)
+ return -EOPNOTSUPP;
+
+ ktime_overall = hvc_res.a0 << 32 | hvc_res.a1;
+ *ts = ktime_to_timespec64(ktime_overall);
+ timespec64_add_ns(ts, t2);
+ *cycle = hvc_res.a2 << 32 | hvc_res.a3;
+ *cs = &clocksource_counter;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(kvm_arch_ptp_get_clock_fn);
+#endif
#endif
diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
index 9b8fee5178e8..3c31ff8eb05f 100644
--- a/drivers/ptp/Kconfig
+++ b/drivers/ptp/Kconfig
@@ -110,7 +110,7 @@ config PTP_1588_CLOCK_PCH
config PTP_1588_CLOCK_KVM
tristate "KVM virtual PTP clock"
depends on PTP_1588_CLOCK
- depends on KVM_GUEST && X86
+ depends on KVM_GUEST && X86 || ARM64 && ARM_ARCH_TIMER
default y
help
This driver adds support for using kvm infrastructure as a PTP
diff --git a/drivers/ptp/kvm_ptp.c b/drivers/ptp/kvm_ptp.c
index d8f215186904..b467e1674a71 100644
--- a/drivers/ptp/kvm_ptp.c
+++ b/drivers/ptp/kvm_ptp.c
@@ -22,6 +22,8 @@ struct kvm_ptp_clock {
struct ptp_clock_info caps;
};
+//extern int kvm_arch_ptp_get_clock_fn(long *cycle, struct timespec64 *ts,
+// struct clocksource **cs)
DEFINE_SPINLOCK(kvm_ptp_lock);
static int ptp_kvm_get_time_fn(ktime_t *device_time,
@@ -138,7 +140,7 @@ static int __init ptp_kvm_init(void)
int ret;
ret = kvm_arch_ptp_init();
- if (!ret)
+ if (ret)
return -EOPNOTSUPP;
kvm_ptp_clock.caps = ptp_kvm_caps;
diff --git a/drivers/ptp/ptp_kvm_arm64.c b/drivers/ptp/ptp_kvm_arm64.c
new file mode 100644
index 000000000000..e0c993847293
--- /dev/null
+++ b/drivers/ptp/ptp_kvm_arm64.c
@@ -0,0 +1,73 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Virtual PTP 1588 clock for use with KVM guests
+ * Copyright (C) 2019 ARM Ltd.
+ * All Rights Reserved
+ */
+
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <asm/hypervisor.h>
+#include <linux/module.h>
+#include <linux/psci.h>
+#include <linux/arm-smccc.h>
+#include <linux/timecounter.h>
+#include <linux/sched/clock.h>
+#include <asm/arch_timer.h>
+
+struct system_counterval_t ptp_sc;
+
+/*
+ * as trap call cause delay, this function will return the delay in nanosecond
+ */
+static u64 arm_smccc_1_1_invoke_delay(u32 id, struct arm_smccc_res *res)
+{
+ u64 t1, t2;
+
+ t1 = sched_clock();
+ arm_smccc_1_1_invoke(id, res);
+ t2 = sched_clock();
+ t2 -= t1;
+ t2 /= 2;
+
+ return t2;
+}
+
+int kvm_arch_ptp_init(void)
+{
+ struct arm_smccc_res hvc_res;
+
+ arm_smccc_1_1_invoke_delay(ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID,
+ &hvc_res);
+ if ((long)(hvc_res.a0) < 0)
+ return -EOPNOTSUPP;
+
+ return 0;
+}
+
+int kvm_arch_ptp_get_clock_generic(struct timespec64 *ts,
+ struct arm_smccc_res *hvc_res)
+{
+ u64 ns;
+ ktime_t ktime_overall;
+
+ ns = arm_smccc_1_1_invoke_delay(ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID,
+ hvc_res);
+ if ((long)(hvc_res->a0) < 0)
+ return -EOPNOTSUPP;
+
+ ktime_overall = hvc_res->a0 << 32 | hvc_res->a1;
+ *ts = ktime_to_timespec64(ktime_overall);
+ timespec64_add_ns(ts, ns);
+
+ return 0;
+}
+
+int kvm_arch_ptp_get_clock(struct timespec64 *ts)
+{
+ struct arm_smccc_res hvc_res;
+
+ kvm_arch_ptp_get_clock_generic(ts, &hvc_res);
+
+ return 0;
+}
diff --git a/include/asm-generic/ptp_kvm.h b/include/asm-generic/ptp_kvm.h
index 208e842bfa64..829d76bdb905 100644
--- a/include/asm-generic/ptp_kvm.h
+++ b/include/asm-generic/ptp_kvm.h
@@ -8,5 +8,5 @@
int kvm_arch_ptp_init(void);
int kvm_arch_ptp_get_clock(struct timespec64 *ts);
-int kvm_arch_ptp_get_clock_fn(long *cycle,
+int kvm_arch_ptp_get_clock_fn(unsigned long *cycle,
struct timespec64 *tspec, void *cs);
--
2.17.1
^ permalink raw reply related
* [RFC PATCH v4 5/5] kvm: arm64: Add capability check extension for ptp_kvm
From: Jianyong Wu @ 2019-09-26 11:42 UTC (permalink / raw)
To: netdev, yangbo.lu, john.stultz, tglx, pbonzini,
sean.j.christopherson, maz, richardcochran, Mark.Rutland,
Will.Deacon, suzuki.poulose
Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, Steve.Capper,
Kaly.Xin, justin.he, jianyong.wu, nd
In-Reply-To: <20190926114212.5322-1-jianyong.wu@arm.com>
Let userspace check if there is kvm ptp service in host.
before VMs migrate to a another host, VMM may check if this
cap is available to determine the migration behaviour.
Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
Suggested-by: Marc Zyngier <maz@kernel.org>
---
include/uapi/linux/kvm.h | 1 +
virt/kvm/arm/arm.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 2fe12b40d503..a0bff6002bd9 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -993,6 +993,7 @@ struct kvm_ppc_resize_hpt {
#define KVM_CAP_ARM_SVE 170
#define KVM_CAP_ARM_PTRAUTH_ADDRESS 171
#define KVM_CAP_ARM_PTRAUTH_GENERIC 172
+#define KVM_CAP_ARM_KVM_PTP 173
#ifdef KVM_CAP_IRQ_ROUTING
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index bd5c55916d0d..80999985160b 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -201,6 +201,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
case KVM_CAP_MP_STATE:
case KVM_CAP_IMMEDIATE_EXIT:
case KVM_CAP_VCPU_EVENTS:
+ case KVM_CAP_ARM_KVM_PTP:
r = 1;
break;
case KVM_CAP_ARM_SET_DEVICE_ADDR:
--
2.17.1
^ permalink raw reply related
* [PATCH net 0/3] mlxsw: Various fixes
From: Ido Schimmel @ 2019-09-26 11:43 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, alexanderk, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
This patchset includes two small fixes for the mlxsw driver and one
patch which clarifies recently introduced devlink-trap documentation.
Patch #1 clears the port's VLAN filters during port initialization. This
ensures that the drop reason reported to the user is consistent. The
problem is explained in detail in the commit message.
Patch #2 clarifies the description of one of the traps exposed via
devlink-trap.
Patch #3 from Danielle forbids the installation of a tc filter with
multiple mirror actions since this is not supported by the device. The
failure is communicated to the user via extack.
Danielle Ratson (1):
mlxsw: spectrum_flower: Fail in case user specifies multiple mirror
actions
Ido Schimmel (2):
mlxsw: spectrum: Clear VLAN filters during port initialization
Documentation: Clarify trap's description
Documentation/networking/devlink-trap.rst | 3 ++-
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 9 +++++++++
drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c | 6 ++++++
.../selftests/drivers/net/mlxsw/devlink_trap_l2_drops.sh | 7 -------
4 files changed, 17 insertions(+), 8 deletions(-)
--
2.21.0
^ permalink raw reply
* [PATCH net 1/3] mlxsw: spectrum: Clear VLAN filters during port initialization
From: Ido Schimmel @ 2019-09-26 11:43 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, alexanderk, mlxsw, Ido Schimmel
In-Reply-To: <20190926114340.9483-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
When a port is created, its VLAN filters are not cleared by the
firmware. This causes tagged packets to be later dropped by the ingress
STP filters, which default to DISCARD state.
The above did not matter much until commit b5ce611fd96e ("mlxsw:
spectrum: Add devlink-trap support") where we exposed the drop reason to
users.
Without this patch, the drop reason users will see is not consistent. If
a port is enslaved to a VLAN-aware bridge and a packet with an invalid
VLAN tries to ingress the bridge, it will be dropped due to ingress STP
filter. If the VLAN is later enabled and then disabled, the packet will
be dropped by the ingress VLAN filter despite the above being a
seemingly NOP operation.
Fix this by clearing all the VLAN filters during port initialization.
Adjust the test accordingly.
Fixes: b5ce611fd96e ("mlxsw: spectrum: Add devlink-trap support")
Reported-by: Alex Kushnarov <alexanderk@mellanox.com>
Tested-by: Alex Kushnarov <alexanderk@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 9 +++++++++
.../selftests/drivers/net/mlxsw/devlink_trap_l2_drops.sh | 7 -------
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index dd234cf7b39d..dcf9562bce8a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3771,6 +3771,14 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
goto err_port_qdiscs_init;
}
+ err = mlxsw_sp_port_vlan_set(mlxsw_sp_port, 0, VLAN_N_VID - 1, false,
+ false);
+ if (err) {
+ dev_err(mlxsw_sp->bus_info->dev, "Port %d: Failed to clear VLAN filter\n",
+ mlxsw_sp_port->local_port);
+ goto err_port_vlan_clear;
+ }
+
err = mlxsw_sp_port_nve_init(mlxsw_sp_port);
if (err) {
dev_err(mlxsw_sp->bus_info->dev, "Port %d: Failed to initialize NVE\n",
@@ -3818,6 +3826,7 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
err_port_pvid_set:
mlxsw_sp_port_nve_fini(mlxsw_sp_port);
err_port_nve_init:
+err_port_vlan_clear:
mlxsw_sp_tc_qdisc_fini(mlxsw_sp_port);
err_port_qdiscs_init:
mlxsw_sp_port_fids_fini(mlxsw_sp_port);
diff --git a/tools/testing/selftests/drivers/net/mlxsw/devlink_trap_l2_drops.sh b/tools/testing/selftests/drivers/net/mlxsw/devlink_trap_l2_drops.sh
index 5dcdfa20fc6c..126caf28b529 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/devlink_trap_l2_drops.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/devlink_trap_l2_drops.sh
@@ -224,13 +224,6 @@ ingress_vlan_filter_test()
local vid=10
bridge vlan add vid $vid dev $swp2 master
- # During initialization the firmware enables all the VLAN filters and
- # the driver does not turn them off since the traffic will be discarded
- # by the STP filter whose default is DISCARD state. Add the VID on the
- # ingress bridge port and then remove it to make sure it is not member
- # in the VLAN.
- bridge vlan add vid $vid dev $swp1 master
- bridge vlan del vid $vid dev $swp1 master
RET=0
--
2.21.0
^ permalink raw reply related
* [PATCH net 2/3] Documentation: Clarify trap's description
From: Ido Schimmel @ 2019-09-26 11:43 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, alexanderk, mlxsw, Ido Schimmel
In-Reply-To: <20190926114340.9483-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
Alex noted that the below description might not be obvious to all users.
Clarify it by adding an example.
Fixes: f3047ca01f12 ("Documentation: Add devlink-trap documentation")
Reported-by: Alex Kushnarov <alexanderk@mellanox.com>
Reviewed-by: Alex Kushnarov <alexanderk@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
Documentation/networking/devlink-trap.rst | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/networking/devlink-trap.rst b/Documentation/networking/devlink-trap.rst
index c20c7c483664..8e90a85f3bd5 100644
--- a/Documentation/networking/devlink-trap.rst
+++ b/Documentation/networking/devlink-trap.rst
@@ -143,7 +143,8 @@ be added to the following table:
* - ``port_list_is_empty``
- ``drop``
- Traps packets that the device decided to drop in case they need to be
- flooded and the flood list is empty
+ flooded (e.g., unknown unicast, unregistered multicast) and there are
+ no ports the packets should be flooded to
* - ``port_loopback_filter``
- ``drop``
- Traps packets that the device decided to drop in case after layer 2
--
2.21.0
^ permalink raw reply related
* [PATCH net 3/3] mlxsw: spectrum_flower: Fail in case user specifies multiple mirror actions
From: Ido Schimmel @ 2019-09-26 11:43 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, alexanderk, mlxsw, Danielle Ratson, Ido Schimmel
In-Reply-To: <20190926114340.9483-1-idosch@idosch.org>
From: Danielle Ratson <danieller@mellanox.com>
The ASIC can only mirror a packet to one port, but when user is trying
to set more than one mirror action, it doesn't fail.
Add a check if more than one mirror action was specified per rule and if so,
fail for not being supported.
Fixes: d0d13c1858a11 ("mlxsw: spectrum_acl: Add support for mirror action")
Signed-off-by: Danielle Ratson <danieller@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
index 0ad1a24abfc6..b607919c8ad0 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
@@ -21,6 +21,7 @@ static int mlxsw_sp_flower_parse_actions(struct mlxsw_sp *mlxsw_sp,
struct netlink_ext_ack *extack)
{
const struct flow_action_entry *act;
+ int mirror_act_count = 0;
int err, i;
if (!flow_action_has_entries(flow_action))
@@ -105,6 +106,11 @@ static int mlxsw_sp_flower_parse_actions(struct mlxsw_sp *mlxsw_sp,
case FLOW_ACTION_MIRRED: {
struct net_device *out_dev = act->dev;
+ if (mirror_act_count++) {
+ NL_SET_ERR_MSG_MOD(extack, "Multiple mirror actions per rule are not supported");
+ return -EOPNOTSUPP;
+ }
+
err = mlxsw_sp_acl_rulei_act_mirror(mlxsw_sp, rulei,
block, out_dev,
extack);
--
2.21.0
^ permalink raw reply related
* Re: [PATCH RFC] cfg80211: add new command for reporting wiphy crashes
From: Rafał Miłecki @ 2019-09-26 11:55 UTC (permalink / raw)
To: Jouni Malinen
Cc: Johannes Berg, David S . Miller, linux-wireless@vger.kernel.org,
Network Development, Linux Kernel Mailing List, wpa_supplicant,
OpenWrt Development List, Rafał Miłecki
In-Reply-To: <20190920140143.GA30514@w1.fi>
Resending from my subscribed e-mail
On 20.09.2019 16:01, Jouni Malinen wrote:
> On Fri, Sep 20, 2019 at 03:37:08PM +0200, Rafał Miłecki wrote:
>> Hardware or firmware instability may result in unusable wiphy. In such
>> cases usually a hardware reset is needed. To allow a full recovery
>> kernel has to indicate problem to the user space.
>
> Why? Shouldn't the driver be able to handle this on its own since all
> the previous configuration was done through the driver anyway. As far as
> I know, there are drivers that do indeed try to do this and handle it
> successfully at least for station mode. AP mode may be more complex, but
> for that one, I guess it would be fine to drop all associations (and
> provide indication of that to user space) and just restart the BSS.
Indeed my main concert is AP mode. I'm afraid that cfg80211 doesn't
cache all settings, consider e.g. nl80211_start_ap(). It builds
struct cfg80211_ap_settings using info from nl80211 message and
passes it to the driver (rdev_start_ap()). Once it's done it
caches only a small subset of all setup data.
In other words driver doesn't have enough info to recover interfaces
setup.
>> This new nl80211 command lets user space known wiphy has crashed and has
>> been just recovered. When applicable it should result in supplicant or
>> authenticator reconfiguring all interfaces.
>
> For me, that is not really "recovered" if some additional
> reconfiguration steps are needed.. I'd like to get a more detailed view
> on what exactly might need to be reconfigured and how would user space
> know what exactly to do. Or would the plan here be that the driver would
> not even indicate this crash if it is actually able to internally
> recover fully from the firmware restart?
I meant that hardware has been recovered & is operational again (driver
can talk to it). I expected user space to reconfigure all interfaces
using the same settings that were used on previous run.
If driver were able to recover interfaces setup on its own (with a help
of cfg80211) then user space wouldn't need to be involved.
>> I'd like to use this new cfg80211_crash_report() in brcmfmac after a
>> successful recovery from a FullMAC firmware crash.
>>
>> Later on I'd like to modify hostapd to reconfigure wiphy using a
>> previously used setup.
>
> So this implies that at least something would need to happen in AP mode.
> Do you have a list of items that the driver cannot do on its own and why
> it would be better to do them from user space?
First of all I was wondering how to handle interfaces creation. After a
firmware crash we have:
1) Interfaces created in Linux
2) No corresponsing interfaces in firmware
Syncing that (re-creating in-firmware firmwares) may be a bit tricky
depending on a driver and hardware. For some cases it could be easier to
delete all interfaces and ask user space to setup wiphy (create required
interfaces) again. I'm not sure if that's acceptable though?
If we agree interfaces should stay and driver simply should configure
firmware properly, then we need all data as explained earlier. struct
cfg80211_ap_settings is not available during runtime. How should we
handle that problem?
>> I'm OpenWrt developer & user and I got annoyed by my devices not auto
>> recovering after various failures. There are things I cannot fix (hw
>> failures or closed fw crashes) but I still expect my devices to get
>> back to operational state as soon as possible on their own.
>
> I fully agree with the auto recovery being important thing to cover for
> this, but I'm not yet convinced that this needs user space action. Or if
> it does, there would need to be more detailed way of indicating what
> exactly is needed for user space to do. The proposed change here is just
> saying "hey, I crashed and did something to get the hardware/firmware
> responding again" which does not really tell much to user space other
> than potentially requiring full disable + re-enable for the related
> interfaces. And that is something that should not actually be done in
> all cases of firmware crashes since there are drivers that handle
> recovery in a manner that is in practice completely transparent to user
> space.
I was aiming for a brutal force solution: just make user space
interfaces need a full setup just at they were just created.
^ permalink raw reply
* Re: [PATCH RFC] cfg80211: add new command for reporting wiphy crashes
From: Johannes Berg @ 2019-09-26 11:55 UTC (permalink / raw)
To: Rafał Miłecki, Jouni Malinen, Rafał Miłecki
Cc: David S . Miller, linux-wireless, netdev, linux-kernel, hostap,
openwrt-devel
In-Reply-To: <4f6f37e5-802c-4504-3dcb-c4a640d138bd@milecki.pl>
On Thu, 2019-09-26 at 13:52 +0200, Rafał Miłecki wrote:
>
> Indeed my main concert is AP mode. I'm afraid that cfg80211 doesn't
> cache all settings, consider e.g. nl80211_start_ap(). It builds
> struct cfg80211_ap_settings using info from nl80211 message and
> passes it to the driver (rdev_start_ap()). Once it's done it
> caches only a small subset of all setup data.
>
> In other words driver doesn't have enough info to recover interfaces
> setup.
So the driver can cache it, just like mac80211.
You can't seriously be suggesting that the driver doesn't *have* enough
information - everything passed through it :)
> I meant that hardware has been recovered & is operational again (driver
> can talk to it). I expected user space to reconfigure all interfaces
> using the same settings that were used on previous run.
>
> If driver were able to recover interfaces setup on its own (with a help
> of cfg80211) then user space wouldn't need to be involved.
The driver can do it, mac80211 does. It's just a matter of what the
driver will do or not.
> First of all I was wondering how to handle interfaces creation. After a
> firmware crash we have:
> 1) Interfaces created in Linux
> 2) No corresponsing interfaces in firmware
> Syncing that (re-creating in-firmware firmwares) may be a bit tricky
> depending on a driver and hardware.
We do that in mac80211, it works fine. Why would it be tricky?
If something fails, I think we force that interface to go down.
> For some cases it could be easier to
> delete all interfaces and ask user space to setup wiphy (create required
> interfaces) again. I'm not sure if that's acceptable though?
>
> If we agree interfaces should stay and driver simply should configure
> firmware properly, then we need all data as explained earlier. struct
> cfg80211_ap_settings is not available during runtime. How should we
> handle that problem?
You can cache it in the driver in whatever format makes sense.
> I was aiming for a brutal force solution: just make user space
> interfaces need a full setup just at they were just created.
You can still do that btw, just unregister and re-register the wiphy.
johannes
^ permalink raw reply
* Re: [PATCH RFC] cfg80211: add new command for reporting wiphy crashes
From: Rafał Miłecki @ 2019-09-26 12:00 UTC (permalink / raw)
To: Johannes Berg, Jouni Malinen, Rafał Miłecki
Cc: David S . Miller, linux-wireless, netdev, linux-kernel, hostap,
openwrt-devel
In-Reply-To: <9ece533700be8237699881312a99cc91c6a71d36.camel@sipsolutions.net>
On 26.09.2019 13:55, Johannes Berg wrote:
> On Thu, 2019-09-26 at 13:52 +0200, Rafał Miłecki wrote:
>>
>> Indeed my main concert is AP mode. I'm afraid that cfg80211 doesn't
>> cache all settings, consider e.g. nl80211_start_ap(). It builds
>> struct cfg80211_ap_settings using info from nl80211 message and
>> passes it to the driver (rdev_start_ap()). Once it's done it
>> caches only a small subset of all setup data.
>>
>> In other words driver doesn't have enough info to recover interfaces
>> setup.
>
> So the driver can cache it, just like mac80211.
>
> You can't seriously be suggesting that the driver doesn't *have* enough
> information - everything passed through it :)
Precisely: it doesn't store (cache) enough info.
>> I meant that hardware has been recovered & is operational again (driver
>> can talk to it). I expected user space to reconfigure all interfaces
>> using the same settings that were used on previous run.
>>
>> If driver were able to recover interfaces setup on its own (with a help
>> of cfg80211) then user space wouldn't need to be involved.
>
> The driver can do it, mac80211 does. It's just a matter of what the
> driver will do or not.
>
>> First of all I was wondering how to handle interfaces creation. After a
>> firmware crash we have:
>> 1) Interfaces created in Linux
>> 2) No corresponsing interfaces in firmware
>
>> Syncing that (re-creating in-firmware firmwares) may be a bit tricky
>> depending on a driver and hardware.
>
> We do that in mac80211, it works fine. Why would it be tricky?
In brcmfmac on .add_virtual_intf() we:
1) Send request to the FullMAC firmware
2) Wait for a reply
3) On success we create interface
4) We wake up .add_virtual_intf() handler
I'll need to find a way to skip step 3 in recovery path since interface
on host side already exists.
> If something fails, I think we force that interface to go down.
>
>> For some cases it could be easier to
>> delete all interfaces and ask user space to setup wiphy (create required
>> interfaces) again. I'm not sure if that's acceptable though?
>>
>> If we agree interfaces should stay and driver simply should configure
>> firmware properly, then we need all data as explained earlier. struct
>> cfg80211_ap_settings is not available during runtime. How should we
>> handle that problem?
>
> You can cache it in the driver in whatever format makes sense.
>
>> I was aiming for a brutal force solution: just make user space
>> interfaces need a full setup just at they were just created.
>
> You can still do that btw, just unregister and re-register the wiphy.
OK, so basically I need to work on caching setup data. Should I try
doing that at my selected driver level (brcmfmac)? Or should I focus on
generic solution (cfg80211) and consider offloading mac80211 if
possible?
^ permalink raw reply
* Re: [PATCH RFC] cfg80211: add new command for reporting wiphy crashes
From: Johannes Berg @ 2019-09-26 12:04 UTC (permalink / raw)
To: Rafał Miłecki, Jouni Malinen
Cc: David S . Miller, linux-wireless, netdev, linux-kernel, hostap,
openwrt-devel
In-Reply-To: <09503390-91f0-3789-496a-6e9891156c5e@gmail.com>
On Thu, 2019-09-26 at 14:00 +0200, Rafał Miłecki wrote:
> > You can't seriously be suggesting that the driver doesn't *have* enough
> > information - everything passed through it :)
>
> Precisely: it doesn't store (cache) enough info.
But nothing stops it (the driver) from doing so!
> In brcmfmac on .add_virtual_intf() we:
> 1) Send request to the FullMAC firmware
> 2) Wait for a reply
> 3) On success we create interface
> 4) We wake up .add_virtual_intf() handler
>
> I'll need to find a way to skip step 3 in recovery path since interface
> on host side already exists.
Sure, we skip lots of things in all drivers, look at iwlwifi for example
with IWL_MVM_STATUS_IN_HW_RESTART.
> OK, so basically I need to work on caching setup data. Should I try
> doing that at my selected driver level (brcmfmac)? Or should I focus on
> generic solution (cfg80211) and consider offloading mac80211 if
> possible?
I think doing it generically will not work well, you have different code
paths and different formats, different data that you need etc.
Sometimes there's information cfg80211 doesn't even *have*, because the
driver is responsible for things (e.g. elements). I guess you can try,
but my gut feeling is that it'd simpler in the driver.
Now you can argue that everything passes through cfg80211 so it must
have enough data too (just like I'm arguing the driver certainly has
enough data), but ... it seems to me the cfg80211 is usually more
action-based, where the restore flow needs to keep the *state*, not
replay the series of actions that happened.
johannes
^ permalink raw reply
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