* [PATCH] qlcnic: remove redundant assignment to variable err
From: Colin King @ 2019-07-03 8:32 UTC (permalink / raw)
To: Shahed Shaikh, Manish Chopra, GR-Linux-NIC-Dev, David S . Miller,
netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The variable err is being initialized with a value that is never
read and it is being updated later with a new value. The
initialization is redundant and can be removed.
Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c
index af3b037fa442..5632da05145a 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c
@@ -1066,7 +1066,7 @@ static int qlcnic_sriov_pf_cfg_ip_cmd(struct qlcnic_bc_trans *trans,
{
struct qlcnic_vf_info *vf = trans->vf;
struct qlcnic_adapter *adapter = vf->adapter;
- int err = -EIO;
+ int err;
cmd->req.arg[1] |= vf->vp->handle << 16;
cmd->req.arg[1] |= BIT_31;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next] tipc: use rcu dereference functions properly
From: Xin Long @ 2019-07-03 8:33 UTC (permalink / raw)
To: David Miller; +Cc: network dev, Jon Maloy, Ying Xue, tipc-discussion
In-Reply-To: <20190702.150811.1940085234903099096.davem@davemloft.net>
On Wed, Jul 3, 2019 at 6:08 AM David Miller <davem@davemloft.net> wrote:
>
> From: Xin Long <lucien.xin@gmail.com>
> Date: Tue, 2 Jul 2019 00:54:55 +0800
>
> > For these places are protected by rcu_read_lock, we change from
> > rcu_dereference_rtnl to rcu_dereference, as there is no need to
> > check if rtnl lock is held.
> >
> > For these places are protected by rtnl_lock, we change from
> > rcu_dereference_rtnl to rtnl_dereference/rcu_dereference_protected,
> > as no extra memory barriers are needed under rtnl_lock() which also
> > protects tn->bearer_list[] and dev->tipc_ptr/b->media_ptr updating.
> >
> > rcu_dereference_rtnl will be only used in the places where it could
> > be under rcu_read_lock or rtnl_lock.
> >
> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
>
> In the cases where RTNL is held, even if rcu_read_lock() is also taken,
> we should use rtnl_dereference() because that avoids the READ_ONCE().
Right, that's what I did in this patch.
But for the places where it's sometimes called under rtnl_lock() only and
sometimes called under rcu_read_lock() only, like tipc_udp_is_known_peer()
and tipc_udp_rcast_add(), I kept rcu_dereference_rtnl(). makes sense?
^ permalink raw reply
* Re: [PATCH net-next v5 0/5] Add MPLS actions to TC
From: John Hurley @ 2019-07-03 8:33 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Network Development, David Miller, Jiri Pirko, Cong Wang,
David Ahern, Simon Horman, Jakub Kicinski, oss-drivers
In-Reply-To: <CA+FuTSd3DaYsY1o_GFp-X=uRkfb6i0PUPbUsUagERmAZS+Hd7Q@mail.gmail.com>
On Wed, Jul 3, 2019 at 3:19 AM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Tue, Jul 2, 2019 at 8:32 PM John Hurley <john.hurley@netronome.com> wrote:
> >
> > This patchset introduces a new TC action module that allows the
> > manipulation of the MPLS headers of packets. The code impliments
> > functionality including push, pop, and modify.
> >
> > Also included are tests for the new funtionality. Note that these will
> > require iproute2 changes to be submitted soon.
> >
> > NOTE: these patches are applied to net-next along with the patch:
> > [PATCH net 1/1] net: openvswitch: fix csum updates for MPLS actions
> > This patch has been accepted into net but, at time of posting, is not yet
> > in net-next.
> >
> > v4-v5:
> > - move mpls_hdr() call to after skb_ensure_writable - patch 3
> > (Willem de Bruijn)
> > - move mpls_dec_ttl to helper - patch 4 (Willem de Bruijn)
> > - add iproute2 usage example to commit msg - patch 4 (David Ahern)
> > - align label validation with mpls core code - patch 4 (David Ahern)
> > - improve extack message for no proto in mpls pop - patch 4 (David Ahern)
> > v3-v4:
> > - refactor and reuse OvS code (Cong Wang)
> > - use csum API rather than skb_post*rscum to update skb->csum (Cong Wang)
> > - remove unnecessary warning (Cong Wang)
> > - add comments to uapi attributes (David Ahern)
> > - set strict type policy check for TCA_MPLS_UNSPEC (David Ahern)
> > - expand/improve extack messages (David Ahern)
> > - add option to manually set BOS
> > v2-v3:
> > - remove a few unnecessary line breaks (Jiri Pirko)
> > - retract hw offload patch from set (resubmit with driver changes) (Jiri)
> > v1->v2:
> > - ensure TCA_ID_MPLS does not conflict with TCA_ID_CTINFO (Davide Caratti)
> >
> > John Hurley (5):
> > net: core: move push MPLS functionality from OvS to core helper
> > net: core: move pop MPLS functionality from OvS to core helper
> > net: core: add MPLS update core helper and use in OvS
> > net: sched: add mpls manipulation actions to TC
> > selftests: tc-tests: actions: add MPLS tests
>
> Reviewed-by: Willem de Bruijn <willemb@google.com>
>
> I did have some conflicts applying the patches from patchwork (to diff
> v4 vs v5). Might be my process. This is clean against net-next, right?
Hi Willem, thanks for review.
See the note in the cover letter....
We had a patch accepted into net earlier in the week, these patches
are applied to net-next + that patch.
Unfortunately when we applied the patches direct to net-next and tried
to merge in net then we got merge conflicts that needed manually
fixed.
Basically, the above patches should apply cleanly to net-next once net
has been merged in.
^ permalink raw reply
* Shall we add some note info for tcp_min_snd_mss?
From: ZhangXiao @ 2019-07-03 8:35 UTC (permalink / raw)
To: edumazet, davem; +Cc: netdev
Hi David & Eric,
Commit 5f3e2bf0 (tcp: add tcp_min_snd_mss sysctl) add a new interface to
adjust network. While if this variable been set too large, for example
larger then (MTU - 40), the net link maybe damaged. So, how about adding
some warning messages for the operator/administrator? In document, or in
source code.
Thanks
Xiao
^ permalink raw reply
* linux-next: manual merge of the akpm-current tree with the net-next tree
From: Stephen Rothwell @ 2019-07-03 8:40 UTC (permalink / raw)
To: Andrew Morton, David Miller, Networking
Cc: Linux Next Mailing List, Linux Kernel Mailing List,
Alexander Potapenko, Mahesh Bandewar
[-- Attachment #1: Type: text/plain, Size: 1197 bytes --]
Hi all,
Today's linux-next merge of the akpm-current tree got a conflict in:
lib/Makefile
between commit:
509e56b37cc3 ("blackhole_dev: add a selftest")
from the net-next tree and commit:
c86f3a66de9a ("lib: introduce test_meminit module")
from the akpm-current tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc lib/Makefile
index 0c3c197a7801,05980c802500..000000000000
--- a/lib/Makefile
+++ b/lib/Makefile
@@@ -91,7 -91,7 +91,8 @@@ obj-$(CONFIG_TEST_DEBUG_VIRTUAL) += tes
obj-$(CONFIG_TEST_MEMCAT_P) += test_memcat_p.o
obj-$(CONFIG_TEST_OBJAGG) += test_objagg.o
obj-$(CONFIG_TEST_STACKINIT) += test_stackinit.o
+obj-$(CONFIG_TEST_BLACKHOLE_DEV) += test_blackhole_dev.o
+ obj-$(CONFIG_TEST_MEMINIT) += test_meminit.o
obj-$(CONFIG_TEST_LIVEPATCH) += livepatch/
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v6 04/15] ethtool: introduce ethtool netlink interface
From: Jiri Pirko @ 2019-07-03 8:41 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, David Miller, Jakub Kicinski, Andrew Lunn,
Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg,
linux-kernel
In-Reply-To: <20190702145241.GD20101@unicorn.suse.cz>
Tue, Jul 02, 2019 at 04:52:41PM CEST, mkubecek@suse.cz wrote:
>On Tue, Jul 02, 2019 at 02:25:21PM +0200, Jiri Pirko wrote:
>> Tue, Jul 02, 2019 at 01:49:59PM CEST, mkubecek@suse.cz wrote:
>> >+Request header
>> >+--------------
>> >+
>> >+Each request or reply message contains a nested attribute with common header.
>> >+Structure of this header is
>>
>> Missing ":"
>
>OK
>
>> >+
>> >+ ETHTOOL_A_HEADER_DEV_INDEX (u32) device ifindex
>> >+ ETHTOOL_A_HEADER_DEV_NAME (string) device name
>> >+ ETHTOOL_A_HEADER_INFOMASK (u32) info mask
>> >+ ETHTOOL_A_HEADER_GFLAGS (u32) flags common for all requests
>> >+ ETHTOOL_A_HEADER_RFLAGS (u32) request specific flags
>> >+
>> >+ETHTOOL_A_HEADER_DEV_INDEX and ETHTOOL_A_HEADER_DEV_NAME identify the device
>> >+message relates to. One of them is sufficient in requests, if both are used,
>> >+they must identify the same device. Some requests, e.g. global string sets, do
>> >+not require device identification. Most GET requests also allow dump requests
>> >+without device identification to query the same information for all devices
>> >+providing it (each device in a separate message).
>> >+
>> >+Optional info mask allows to ask only for a part of data provided by GET
>>
>> How this "infomask" works? What are the bits related to? Is that request
>> specific?
>
>The interpretation is request specific, the information returned for
>a GET request is divided into multiple parts and client can choose to
>request one of them (usually one). In the code so far, infomask bits
>correspond to top level (nest) attributes but I would rather not make it
>a strict rule.
Wait, so it is a matter of verbosity? If you have multiple parts and the
user is able to chose one of them, why don't you rather have multiple
get commands, one per bit. This infomask construct seems redundant to me.
>
>I'll make the paragraph more verbose.
>
>> >+request types. If omitted or zero, all data is returned. The two flag bitmaps
>> >+allow enabling requestoptions; ETHTOOL_A_HEADER_GFLAGS are global flags common
>>
>> s/requestoptions;/request options./ ?
>
>Yes.
>
>> >+for all request types, flags recognized in ETHTOOL_A_HEADER_RFLAGS and their
>> >+interpretation are specific for each request type. Global flags are
>> >+
>> >+ ETHTOOL_RF_COMPACT use compact format bitsets in reply
>>
>> Why "RF"? Isn't this "GF"? I would like "ETHTOOL_GFLAG_COMPACT" better.
>
>RF as Request Flags. At the moment, global flags use ETHTOOL_RF_name
>pattern and request specific flags ETHTOOL_RF_msgtype_name. GFLAG and
>RFLAG would probably show the relation better, so how about
>
> ETHTOOL_GFLAG_name for global
> ETHTOOL_RFLAG_msgtype_name for request specific
Yep, as I suggested. Looks fine to me.
>
>> >+ ETHTOOL_RF_REPLY send optional reply (SET and ACT requests)
>> >+
>> >+Request specific flags are described with each request type. For both flag
>> >+attributes, new flags should follow the general idea that if the flag is not
>> >+set, the behaviour is the same as (or closer to) the behaviour before it was
>>
>> "closer to" ? That would be unfortunate I believe...
>
>There may be situations where it cannot be exactly the same, e.g.
>because the flag affects interpretation of an attribute which was
>introduced together with the flag. How about "...the behaviour is
>backward compatible"?
Ok.
>
>
>> >+List of message types
>> >+---------------------
>> >+
>> >+All constants identifying message types use ETHTOOL_CMD_ prefix and suffix
>> >+according to message purpose:
>> >+
>> >+ _GET userspace request to retrieve data
>> >+ _SET userspace request to set data
>> >+ _ACT userspace request to perform an action
>> >+ _GET_REPLY kernel reply to a GET request
>> >+ _SET_REPLY kernel reply to a SET request
>> >+ _ACT_REPLY kernel reply to an ACT request
>> >+ _NTF kernel notification
>> >+
>> >+"GET" requests are sent by userspace applications to retrieve device
>> >+information. They usually do not contain any message specific attributes.
>> >+Kernel replies with corresponding "GET_REPLY" message. For most types, "GET"
>> >+request with NLM_F_DUMP and no device identification can be used to query the
>> >+information for all devices supporting the request.
>> >+
>> >+If the data can be also modified, corresponding "SET" message with the same
>> >+layout as "GET" reply is used to request changes. Only attributes where
>>
>> s/"GET" reply"/"GET_REPLY" ?
>> Maybe better to emphasize that the "GET_REPLY" is the one corresponding
>> with "SET". But perhaps I got this sentence all wrong :/
>
>OK
>
>> >+a change is requested are included in such request (also, not all attributes
>> >+may be changed). Replies to most "SET" request consist only of error code and
>> >+extack; if kernel provides additional data, it is sent in the form of
>> >+corresponding "SET_REPLY" message (if ETHTOOL_RF_REPLY flag was set in request
>> >+header).
>> >+
>> >+Data modification also triggers sending a "NTF" message with a notification.
>> >+These usually bear only a subset of attributes which was affected by the
>> >+change. The same notification is issued if the data is modified using other
>> >+means (mostly ioctl ethtool interface). Unlike notifications from ethtool
>> >+netlink code which are only sent if something actually changed, notifications
>> >+triggered by ioctl interface may be sent even if the request did not actually
>> >+change any data.
>>
>> Interesting. What's the reason for that?
>
>Most setting commands in ioctl interface do not even query the original
>state, they just pass the structure from ioctl() to ethtool_ops handler.
>We could add retrieving the original state first but I suppose we would
>still have to call the handler anyway even if requested values are the
>same (as that's what kernel does now) and it's not clear if omitting the
>notification in such case is the right thing to do.
Okay, got it. Better notification with no change than no notification.
>
>> >diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile
>> >index 3ebfab2bca66..f30e0da88be5 100644
>> >--- a/net/ethtool/Makefile
>> >+++ b/net/ethtool/Makefile
>> >@@ -1,3 +1,7 @@
>> > # SPDX-License-Identifier: GPL-2.0
>> >
>> >-obj-y += ioctl.o
>> >+obj-y += ioctl.o
>> >+
>> >+obj-$(CONFIG_ETHTOOL_NETLINK) += ethtool_nl.o
>>
>> Hmm, I wonder, why not to make this always on? We want users to use
>> it, memory savings in case it is off would be minimal. RTNetlink is also
>> always on. Ethtool ioctl is also always on.
>
>We have already discussed this in the previous version. Someone claimed
>earlier that building a kernel without ethtool interface would make
>sense for some minimalistic systems. My plan is to make the ioctl
>interface also optional once it's possible for (sufficiently new)
>ethtool to work without it.
Okay, pardon me. I don't recall that conversation.
>
>Michal
^ permalink raw reply
* Re: [PATCH next] loopback: fix lockdep splat
From: Geert Uytterhoeven @ 2019-07-03 8:45 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: Netdev, Eric Dumazet, David Miller, Mahesh Bandewar,
Linux-Renesas
In-Reply-To: <20190703061631.84485-1-maheshb@google.com>
Hi Mahesh,
s/lockdep/rcu/ in the subject.
On Wed, Jul 3, 2019 at 8:16 AM Mahesh Bandewar <maheshb@google.com> wrote:
> dev_init_scheduler() and dev_activate() expect the caller to
> hold RTNL. Since we don't want blackhole device to be initialized
> per ns, we are initializing at init.
>
> [ 3.855027] Call Trace:
> [ 3.855034] dump_stack+0x67/0x95
> [ 3.855037] lockdep_rcu_suspicious+0xd5/0x110
> [ 3.855044] dev_init_scheduler+0xe3/0x120
> [ 3.855048] ? net_olddevs_init+0x60/0x60
> [ 3.855050] blackhole_netdev_init+0x45/0x6e
> [ 3.855052] do_one_initcall+0x6c/0x2fa
> [ 3.855058] ? rcu_read_lock_sched_held+0x8c/0xa0
> [ 3.855066] kernel_init_freeable+0x1e5/0x288
> [ 3.855071] ? rest_init+0x260/0x260
> [ 3.855074] kernel_init+0xf/0x180
> [ 3.855076] ? rest_init+0x260/0x260
> [ 3.855078] ret_from_fork+0x24/0x30
>
> Fixes: 4de83b88c66 ("loopback: create blackhole net device similar to loopack.")
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
Thanks, that got rid of the rcu splat.
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH v2 1/4] net: dsa: Change DT bindings for Vitesse VSC73xx switches
From: Pawel Dembicki @ 2019-07-03 8:57 UTC (permalink / raw)
Cc: paweldembicki, linus.walleij, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Rob Herring, Mark Rutland,
netdev, devicetree, linux-kernel
In-Reply-To: <20190701152723.624-1-paweldembicki@gmail.com>
This commit introduce how to use vsc73xx platform driver.
Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
Changes in v2:
- Drop -spi and -platform suffix
- Change commit message
.../bindings/net/dsa/vitesse,vsc73xx.txt | 57 +++++++++++++++++--
1 file changed, 53 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/dsa/vitesse,vsc73xx.txt b/Documentation/devicetree/bindings/net/dsa/vitesse,vsc73xx.txt
index ed4710c40641..c55e0148657d 100644
--- a/Documentation/devicetree/bindings/net/dsa/vitesse,vsc73xx.txt
+++ b/Documentation/devicetree/bindings/net/dsa/vitesse,vsc73xx.txt
@@ -2,8 +2,8 @@ Vitesse VSC73xx Switches
========================
This defines device tree bindings for the Vitesse VSC73xx switch chips.
-The Vitesse company has been acquired by Microsemi and Microsemi in turn
-acquired by Microchip but retains this vendor branding.
+The Vitesse company has been acquired by Microsemi and Microsemi has
+been acquired Microchip but retains this vendor branding.
The currently supported switch chips are:
Vitesse VSC7385 SparX-G5 5+1-port Integrated Gigabit Ethernet Switch
@@ -11,8 +11,13 @@ Vitesse VSC7388 SparX-G8 8-port Integrated Gigabit Ethernet Switch
Vitesse VSC7395 SparX-G5e 5+1-port Integrated Gigabit Ethernet Switch
Vitesse VSC7398 SparX-G8e 8-port Integrated Gigabit Ethernet Switch
-The device tree node is an SPI device so it must reside inside a SPI bus
-device tree node, see spi/spi-bus.txt
+This switch could have two different management interface.
+
+If SPI interface is used, the device tree node is an SPI device so it must
+reside inside a SPI bus device tree node, see spi/spi-bus.txt
+
+If Platform driver is used, the device tree node is an platform device so it
+must reside inside a platform bus device tree node.
Required properties:
@@ -38,6 +43,7 @@ and subnodes of DSA switches.
Examples:
+SPI:
switch@0 {
compatible = "vitesse,vsc7395";
reg = <0>;
@@ -79,3 +85,46 @@ switch@0 {
};
};
};
+
+Platform:
+switch@2,0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "vitesse,vsc7385";
+ reg = <0x2 0x0 0x20000>;
+ reset-gpios = <&gpio0 12 GPIO_ACTIVE_LOW>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ label = "lan1";
+ };
+ port@1 {
+ reg = <1>;
+ label = "lan2";
+ };
+ port@2 {
+ reg = <2>;
+ label = "lan3";
+ };
+ port@3 {
+ reg = <3>;
+ label = "lan4";
+ };
+ vsc: port@6 {
+ reg = <6>;
+ label = "cpu";
+ ethernet = <&enet0>;
+ phy-mode = "rgmii";
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ pause;
+ };
+ };
+ };
+
+};
--
2.20.1
^ permalink raw reply related
* [PATCH v1 net-next] net: stmmac: Enable dwmac4 jumbo frame more than 8KiB
From: Voon Weifeng @ 2019-07-03 16:59 UTC (permalink / raw)
To: David S. Miller, Maxime Coquelin
Cc: netdev, linux-kernel, Jose Abreu, Giuseppe Cavallaro, Andrew Lunn,
Alexandre Torgue, Ong Boon Leong, Voon Weifeng
From: Weifeng Voon <weifeng.voon@intel.com>
Enable GMAC v4.xx and beyond to support 16KiB buffer.
Signed-off-by: Weifeng Voon <weifeng.voon@intel.com>
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
index cf6436d3d6c7..dbde23e7e169 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
@@ -443,6 +443,15 @@ static void dwmac4_clear(struct dma_desc *p)
p->des3 = 0;
}
+static int set_16kib_bfsize(int mtu)
+{
+ int ret = 0;
+
+ if (unlikely(mtu >= BUF_SIZE_8KiB))
+ ret = BUF_SIZE_16KiB;
+ return ret;
+}
+
const struct stmmac_desc_ops dwmac4_desc_ops = {
.tx_status = dwmac4_wrback_get_tx_status,
.rx_status = dwmac4_wrback_get_rx_status,
@@ -469,4 +478,6 @@ static void dwmac4_clear(struct dma_desc *p)
.clear = dwmac4_clear,
};
-const struct stmmac_mode_ops dwmac4_ring_mode_ops = { };
+const struct stmmac_mode_ops dwmac4_ring_mode_ops = {
+ .set_16kib_bfsize = set_16kib_bfsize,
+};
--
1.9.1
^ permalink raw reply related
* [PATCH v2 2/4] net: dsa: vsc73xx: Split vsc73xx driver
From: Pawel Dembicki @ 2019-07-03 9:02 UTC (permalink / raw)
Cc: paweldembicki, linus.walleij, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Rob Herring, Mark Rutland,
netdev, devicetree, linux-kernel
In-Reply-To: <20190701152723.624-2-paweldembicki@gmail.com>
This driver (currently) only takes control of the switch chip over
SPI and configures it to route packages around when connected to a
CPU port. But Vitesse chip support also parallel interface.
This patch split driver into two parts: core and spi. It is required
for add support to another managing interface.
Tested-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
Changes in v2:
- Make patch less invasive
- Move mutex to SPI part of driver
- v2 also tested by Linus
drivers/net/dsa/Kconfig | 11 +-
drivers/net/dsa/Makefile | 3 +-
...tesse-vsc73xx.c => vitesse-vsc73xx-core.c} | 170 +--------------
drivers/net/dsa/vitesse-vsc73xx-spi.c | 203 ++++++++++++++++++
drivers/net/dsa/vitesse-vsc73xx.h | 29 +++
5 files changed, 254 insertions(+), 162 deletions(-)
rename drivers/net/dsa/{vitesse-vsc73xx.c => vitesse-vsc73xx-core.c} (91%)
create mode 100644 drivers/net/dsa/vitesse-vsc73xx-spi.c
create mode 100644 drivers/net/dsa/vitesse-vsc73xx.h
diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index b91e78e3598f..4ab2aa09e2e4 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -99,8 +99,8 @@ config NET_DSA_SMSC_LAN9303_MDIO
for MDIO managed mode.
config NET_DSA_VITESSE_VSC73XX
- tristate "Vitesse VSC7385/7388/7395/7398 support"
- depends on OF && SPI
+ tristate
+ depends on OF
depends on NET_DSA
select FIXED_PHY
select VITESSE_PHY
@@ -109,4 +109,11 @@ config NET_DSA_VITESSE_VSC73XX
This enables support for the Vitesse VSC7385, VSC7388,
VSC7395 and VSC7398 SparX integrated ethernet switches.
+config NET_DSA_VITESSE_VSC73XX_SPI
+ tristate "Vitesse VSC7385/7388/7395/7398 SPI mode support"
+ depends on SPI
+ select NET_DSA_VITESSE_VSC73XX
+ ---help---
+ This enables support for the Vitesse VSC7385, VSC7388, VSC7395
+ and VSC7398 SparX integrated ethernet switches in SPI managed mode.
endmenu
diff --git a/drivers/net/dsa/Makefile b/drivers/net/dsa/Makefile
index fefb6aaa82ba..117bf78be211 100644
--- a/drivers/net/dsa/Makefile
+++ b/drivers/net/dsa/Makefile
@@ -14,7 +14,8 @@ realtek-objs := realtek-smi.o rtl8366.o rtl8366rb.o
obj-$(CONFIG_NET_DSA_SMSC_LAN9303) += lan9303-core.o
obj-$(CONFIG_NET_DSA_SMSC_LAN9303_I2C) += lan9303_i2c.o
obj-$(CONFIG_NET_DSA_SMSC_LAN9303_MDIO) += lan9303_mdio.o
-obj-$(CONFIG_NET_DSA_VITESSE_VSC73XX) += vitesse-vsc73xx.o
+obj-$(CONFIG_NET_DSA_VITESSE_VSC73XX) += vitesse-vsc73xx-core.o
+obj-$(CONFIG_NET_DSA_VITESSE_VSC73XX_SPI) += vitesse-vsc73xx-spi.o
obj-y += b53/
obj-y += microchip/
obj-y += mv88e6xxx/
diff --git a/drivers/net/dsa/vitesse-vsc73xx.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
similarity index 91%
rename from drivers/net/dsa/vitesse-vsc73xx.c
rename to drivers/net/dsa/vitesse-vsc73xx-core.c
index d4780610ea8a..10063f31d9a3 100644
--- a/drivers/net/dsa/vitesse-vsc73xx.c
+++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
@@ -10,10 +10,6 @@
* handling the switch in a memory-mapped manner by connecting to that external
* CPU's memory bus.
*
- * This driver (currently) only takes control of the switch chip over SPI and
- * configures it to route packages around when connected to a CPU port. The
- * chip has embedded PHYs and VLAN support so we model it using DSA.
- *
* Copyright (C) 2018 Linus Wallej <linus.walleij@linaro.org>
* Includes portions of code from the firmware uploader by:
* Copyright (C) 2009 Gabor Juhos <juhosg@openwrt.org>
@@ -24,8 +20,6 @@
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/of_mdio.h>
-#include <linux/platform_device.h>
-#include <linux/spi/spi.h>
#include <linux/bitops.h>
#include <linux/if_bridge.h>
#include <linux/etherdevice.h>
@@ -34,6 +28,8 @@
#include <linux/random.h>
#include <net/dsa.h>
+#include "vitesse-vsc73xx.h"
+
#define VSC73XX_BLOCK_MAC 0x1 /* Subblocks 0-4, 6 (CPU port) */
#define VSC73XX_BLOCK_ANALYZER 0x2 /* Only subblock 0 */
#define VSC73XX_BLOCK_MII 0x3 /* Subblocks 0 and 1 */
@@ -255,13 +251,6 @@
#define VSC73XX_GLORESET_PHY_RESET BIT(1)
#define VSC73XX_GLORESET_MASTER_RESET BIT(0)
-#define VSC73XX_CMD_MODE_READ 0
-#define VSC73XX_CMD_MODE_WRITE 1
-#define VSC73XX_CMD_MODE_SHIFT 4
-#define VSC73XX_CMD_BLOCK_SHIFT 5
-#define VSC73XX_CMD_BLOCK_MASK 0x7
-#define VSC73XX_CMD_SUBBLOCK_MASK 0xf
-
#define VSC7385_CLOCK_DELAY ((3 << 4) | 3)
#define VSC7385_CLOCK_DELAY_MASK ((3 << 4) | 3)
@@ -274,20 +263,6 @@
VSC73XX_ICPU_CTRL_CLK_EN | \
VSC73XX_ICPU_CTRL_SRST)
-/**
- * struct vsc73xx - VSC73xx state container
- */
-struct vsc73xx {
- struct device *dev;
- struct gpio_desc *reset;
- struct spi_device *spi;
- struct dsa_switch *ds;
- struct gpio_chip gc;
- u16 chipid;
- u8 addr[ETH_ALEN];
- struct mutex lock; /* Protects SPI traffic */
-};
-
#define IS_7385(a) ((a)->chipid == VSC73XX_CHIPID_ID_7385)
#define IS_7388(a) ((a)->chipid == VSC73XX_CHIPID_ID_7388)
#define IS_7395(a) ((a)->chipid == VSC73XX_CHIPID_ID_7395)
@@ -365,7 +340,7 @@ static const struct vsc73xx_counter vsc73xx_tx_counters[] = {
{ 29, "TxQoSClass3" }, /* non-standard counter */
};
-static int vsc73xx_is_addr_valid(u8 block, u8 subblock)
+int vsc73xx_is_addr_valid(u8 block, u8 subblock)
{
switch (block) {
case VSC73XX_BLOCK_MAC:
@@ -396,96 +371,18 @@ static int vsc73xx_is_addr_valid(u8 block, u8 subblock)
return 0;
}
-
-static u8 vsc73xx_make_addr(u8 mode, u8 block, u8 subblock)
-{
- u8 ret;
-
- ret = (block & VSC73XX_CMD_BLOCK_MASK) << VSC73XX_CMD_BLOCK_SHIFT;
- ret |= (mode & 1) << VSC73XX_CMD_MODE_SHIFT;
- ret |= subblock & VSC73XX_CMD_SUBBLOCK_MASK;
-
- return ret;
-}
+EXPORT_SYMBOL(vsc73xx_is_addr_valid);
static int vsc73xx_read(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,
u32 *val)
{
- struct spi_transfer t[2];
- struct spi_message m;
- u8 cmd[4];
- u8 buf[4];
- int ret;
-
- if (!vsc73xx_is_addr_valid(block, subblock))
- return -EINVAL;
-
- spi_message_init(&m);
-
- memset(&t, 0, sizeof(t));
-
- t[0].tx_buf = cmd;
- t[0].len = sizeof(cmd);
- spi_message_add_tail(&t[0], &m);
-
- t[1].rx_buf = buf;
- t[1].len = sizeof(buf);
- spi_message_add_tail(&t[1], &m);
-
- cmd[0] = vsc73xx_make_addr(VSC73XX_CMD_MODE_READ, block, subblock);
- cmd[1] = reg;
- cmd[2] = 0;
- cmd[3] = 0;
-
- mutex_lock(&vsc->lock);
- ret = spi_sync(vsc->spi, &m);
- mutex_unlock(&vsc->lock);
-
- if (ret)
- return ret;
-
- *val = (buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3];
-
- return 0;
+ return vsc->ops->read(vsc, block, subblock, reg, val);
}
static int vsc73xx_write(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,
u32 val)
{
- struct spi_transfer t[2];
- struct spi_message m;
- u8 cmd[2];
- u8 buf[4];
- int ret;
-
- if (!vsc73xx_is_addr_valid(block, subblock))
- return -EINVAL;
-
- spi_message_init(&m);
-
- memset(&t, 0, sizeof(t));
-
- t[0].tx_buf = cmd;
- t[0].len = sizeof(cmd);
- spi_message_add_tail(&t[0], &m);
-
- t[1].tx_buf = buf;
- t[1].len = sizeof(buf);
- spi_message_add_tail(&t[1], &m);
-
- cmd[0] = vsc73xx_make_addr(VSC73XX_CMD_MODE_WRITE, block, subblock);
- cmd[1] = reg;
-
- buf[0] = (val >> 24) & 0xff;
- buf[1] = (val >> 16) & 0xff;
- buf[2] = (val >> 8) & 0xff;
- buf[3] = val & 0xff;
-
- mutex_lock(&vsc->lock);
- ret = spi_sync(vsc->spi, &m);
- mutex_unlock(&vsc->lock);
-
- return ret;
+ return vsc->ops->write(vsc, block, subblock, reg, val);
}
static int vsc73xx_update_bits(struct vsc73xx *vsc, u8 block, u8 subblock,
@@ -1245,21 +1142,11 @@ static int vsc73xx_gpio_probe(struct vsc73xx *vsc)
return 0;
}
-static int vsc73xx_probe(struct spi_device *spi)
+int vsc73xx_probe(struct vsc73xx *vsc)
{
- struct device *dev = &spi->dev;
- struct vsc73xx *vsc;
+ struct device *dev = vsc->dev;
int ret;
- vsc = devm_kzalloc(dev, sizeof(*vsc), GFP_KERNEL);
- if (!vsc)
- return -ENOMEM;
-
- spi_set_drvdata(spi, vsc);
- vsc->spi = spi_dev_get(spi);
- vsc->dev = dev;
- mutex_init(&vsc->lock);
-
/* Release reset, if any */
vsc->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
if (IS_ERR(vsc->reset)) {
@@ -1270,14 +1157,6 @@ static int vsc73xx_probe(struct spi_device *spi)
/* Wait 20ms according to datasheet table 245 */
msleep(20);
- spi->mode = SPI_MODE_0;
- spi->bits_per_word = 8;
- ret = spi_setup(spi);
- if (ret < 0) {
- dev_err(dev, "spi setup failed.\n");
- return ret;
- }
-
ret = vsc73xx_detect(vsc);
if (ret) {
dev_err(dev, "no chip found (%d)\n", ret);
@@ -1321,43 +1200,16 @@ static int vsc73xx_probe(struct spi_device *spi)
return 0;
}
+EXPORT_SYMBOL(vsc73xx_probe);
-static int vsc73xx_remove(struct spi_device *spi)
+int vsc73xx_remove(struct vsc73xx *vsc)
{
- struct vsc73xx *vsc = spi_get_drvdata(spi);
-
dsa_unregister_switch(vsc->ds);
gpiod_set_value(vsc->reset, 1);
return 0;
}
-
-static const struct of_device_id vsc73xx_of_match[] = {
- {
- .compatible = "vitesse,vsc7385",
- },
- {
- .compatible = "vitesse,vsc7388",
- },
- {
- .compatible = "vitesse,vsc7395",
- },
- {
- .compatible = "vitesse,vsc7398",
- },
- { },
-};
-MODULE_DEVICE_TABLE(of, vsc73xx_of_match);
-
-static struct spi_driver vsc73xx_driver = {
- .probe = vsc73xx_probe,
- .remove = vsc73xx_remove,
- .driver = {
- .name = "vsc73xx",
- .of_match_table = vsc73xx_of_match,
- },
-};
-module_spi_driver(vsc73xx_driver);
+EXPORT_SYMBOL(vsc73xx_remove);
MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>");
MODULE_DESCRIPTION("Vitesse VSC7385/7388/7395/7398 driver");
diff --git a/drivers/net/dsa/vitesse-vsc73xx-spi.c b/drivers/net/dsa/vitesse-vsc73xx-spi.c
new file mode 100644
index 000000000000..0fe1e6175a4a
--- /dev/null
+++ b/drivers/net/dsa/vitesse-vsc73xx-spi.c
@@ -0,0 +1,203 @@
+// SPDX-License-Identifier: GPL-2.0
+/* DSA driver for:
+ * Vitesse VSC7385 SparX-G5 5+1-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7388 SparX-G8 8-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7395 SparX-G5e 5+1-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7398 SparX-G8e 8-port Integrated Gigabit Ethernet Switch
+ *
+ * This driver takes control of the switch chip over SPI and
+ * configures it to route packages around when connected to a CPU port.
+ *
+ * Copyright (C) 2018 Linus Wallej <linus.walleij@linaro.org>
+ * Includes portions of code from the firmware uploader by:
+ * Copyright (C) 2009 Gabor Juhos <juhosg@openwrt.org>
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/spi/spi.h>
+
+#include "vitesse-vsc73xx.h"
+
+#define VSC73XX_CMD_SPI_MODE_READ 0
+#define VSC73XX_CMD_SPI_MODE_WRITE 1
+#define VSC73XX_CMD_SPI_MODE_SHIFT 4
+#define VSC73XX_CMD_SPI_BLOCK_SHIFT 5
+#define VSC73XX_CMD_SPI_BLOCK_MASK 0x7
+#define VSC73XX_CMD_SPI_SUBBLOCK_MASK 0xf
+
+/**
+ * struct vsc73xx_spi - VSC73xx SPI state container
+ */
+struct vsc73xx_spi {
+ struct spi_device *spi;
+ struct mutex lock; /* Protects SPI traffic */
+ struct vsc73xx vsc;
+};
+
+static const struct vsc73xx_ops vsc73xx_spi_ops;
+
+static u8 vsc73xx_make_addr(u8 mode, u8 block, u8 subblock)
+{
+ u8 ret;
+
+ ret = (block & VSC73XX_CMD_SPI_BLOCK_MASK)
+ << VSC73XX_CMD_SPI_BLOCK_SHIFT;
+ ret |= (mode & 1) << VSC73XX_CMD_SPI_MODE_SHIFT;
+ ret |= subblock & VSC73XX_CMD_SPI_SUBBLOCK_MASK;
+
+ return ret;
+}
+
+static int vsc73xx_spi_read(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,
+ u32 *val)
+{
+ struct vsc73xx_spi *vsc_spi = vsc->priv;
+ struct spi_transfer t[2];
+ struct spi_message m;
+ u8 cmd[4];
+ u8 buf[4];
+ int ret;
+
+ if (!vsc73xx_is_addr_valid(block, subblock))
+ return -EINVAL;
+
+ spi_message_init(&m);
+
+ memset(&t, 0, sizeof(t));
+
+ t[0].tx_buf = cmd;
+ t[0].len = sizeof(cmd);
+ spi_message_add_tail(&t[0], &m);
+
+ t[1].rx_buf = buf;
+ t[1].len = sizeof(buf);
+ spi_message_add_tail(&t[1], &m);
+
+ cmd[0] = vsc73xx_make_addr(VSC73XX_CMD_SPI_MODE_READ, block, subblock);
+ cmd[1] = reg;
+ cmd[2] = 0;
+ cmd[3] = 0;
+
+ mutex_lock(&vsc_spi->lock);
+ ret = spi_sync(vsc_spi->spi, &m);
+ mutex_unlock(&vsc_spi->lock);
+
+ if (ret)
+ return ret;
+
+ *val = (buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3];
+
+ return 0;
+}
+
+static int vsc73xx_spi_write(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,
+ u32 val)
+{
+ struct vsc73xx_spi *vsc_spi = vsc->priv;
+ struct spi_transfer t[2];
+ struct spi_message m;
+ u8 cmd[2];
+ u8 buf[4];
+ int ret;
+
+ if (!vsc73xx_is_addr_valid(block, subblock))
+ return -EINVAL;
+
+ spi_message_init(&m);
+
+ memset(&t, 0, sizeof(t));
+
+ t[0].tx_buf = cmd;
+ t[0].len = sizeof(cmd);
+ spi_message_add_tail(&t[0], &m);
+
+ t[1].tx_buf = buf;
+ t[1].len = sizeof(buf);
+ spi_message_add_tail(&t[1], &m);
+
+ cmd[0] = vsc73xx_make_addr(VSC73XX_CMD_SPI_MODE_WRITE, block, subblock);
+ cmd[1] = reg;
+
+ buf[0] = (val >> 24) & 0xff;
+ buf[1] = (val >> 16) & 0xff;
+ buf[2] = (val >> 8) & 0xff;
+ buf[3] = val & 0xff;
+
+ mutex_lock(&vsc_spi->lock);
+ ret = spi_sync(vsc_spi->spi, &m);
+ mutex_unlock(&vsc_spi->lock);
+
+ return ret;
+}
+
+static int vsc73xx_spi_probe(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct vsc73xx_spi *vsc_spi;
+ int ret;
+
+ vsc_spi = devm_kzalloc(dev, sizeof(*vsc_spi), GFP_KERNEL);
+ if (!vsc_spi)
+ return -ENOMEM;
+
+ spi_set_drvdata(spi, vsc_spi);
+ vsc_spi->spi = spi_dev_get(spi);
+ vsc_spi->vsc.dev = dev;
+ vsc_spi->vsc.priv = vsc_spi;
+ vsc_spi->vsc.ops = &vsc73xx_spi_ops;
+ mutex_init(&vsc_spi->lock);
+
+ spi->mode = SPI_MODE_0;
+ spi->bits_per_word = 8;
+ ret = spi_setup(spi);
+ if (ret < 0) {
+ dev_err(dev, "spi setup failed.\n");
+ return ret;
+ }
+
+ return vsc73xx_probe(&vsc_spi->vsc);
+}
+
+static int vsc73xx_spi_remove(struct spi_device *spi)
+{
+ struct vsc73xx_spi *vsc_spi = spi_get_drvdata(spi);
+
+ return vsc73xx_remove(&vsc_spi->vsc);
+}
+
+static const struct vsc73xx_ops vsc73xx_spi_ops = {
+ .read = vsc73xx_spi_read,
+ .write = vsc73xx_spi_write,
+};
+
+static const struct of_device_id vsc73xx_of_match[] = {
+ {
+ .compatible = "vitesse,vsc7385",
+ },
+ {
+ .compatible = "vitesse,vsc7388",
+ },
+ {
+ .compatible = "vitesse,vsc7395",
+ },
+ {
+ .compatible = "vitesse,vsc7398",
+ },
+ { },
+};
+MODULE_DEVICE_TABLE(of, vsc73xx_of_match);
+
+static struct spi_driver vsc73xx_spi_driver = {
+ .probe = vsc73xx_spi_probe,
+ .remove = vsc73xx_spi_remove,
+ .driver = {
+ .name = "vsc73xx-spi",
+ .of_match_table = vsc73xx_of_match,
+ },
+};
+module_spi_driver(vsc73xx_spi_driver);
+
+MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>");
+MODULE_DESCRIPTION("Vitesse VSC7385/7388/7395/7398 SPI driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/net/dsa/vitesse-vsc73xx.h b/drivers/net/dsa/vitesse-vsc73xx.h
new file mode 100644
index 000000000000..7478f8d4e0a9
--- /dev/null
+++ b/drivers/net/dsa/vitesse-vsc73xx.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include <linux/device.h>
+#include <linux/etherdevice.h>
+#include <linux/gpio/driver.h>
+
+/**
+ * struct vsc73xx - VSC73xx state container
+ */
+struct vsc73xx {
+ struct device *dev;
+ struct gpio_desc *reset;
+ struct dsa_switch *ds;
+ struct gpio_chip gc;
+ u16 chipid;
+ u8 addr[ETH_ALEN];
+ const struct vsc73xx_ops *ops;
+ void *priv;
+};
+
+struct vsc73xx_ops {
+ int (*read)(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,
+ u32 *val);
+ int (*write)(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,
+ u32 val);
+};
+
+int vsc73xx_is_addr_valid(u8 block, u8 subblock);
+int vsc73xx_probe(struct vsc73xx *vsc);
+int vsc73xx_remove(struct vsc73xx *vsc);
--
2.20.1
^ permalink raw reply related
* [PATCH v2 3/4] net: dsa: vsc73xx: add support for parallel mode
From: Pawel Dembicki @ 2019-07-03 9:07 UTC (permalink / raw)
Cc: paweldembicki, linus.walleij, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Rob Herring, Mark Rutland,
netdev, devicetree, linux-kernel
In-Reply-To: <20190701152723.624-3-paweldembicki@gmail.com>
This patch add platform part of vsc73xx driver.
It allows to use chip connected by parallel interface.
Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
Changes in v2:
- drop mutex
- fix indentation
drivers/net/dsa/Kconfig | 8 ++
drivers/net/dsa/Makefile | 1 +
drivers/net/dsa/vitesse-vsc73xx-platform.c | 160 +++++++++++++++++++++
3 files changed, 169 insertions(+)
create mode 100644 drivers/net/dsa/vitesse-vsc73xx-platform.c
diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index 4ab2aa09e2e4..80965808949d 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -116,4 +116,12 @@ config NET_DSA_VITESSE_VSC73XX_SPI
---help---
This enables support for the Vitesse VSC7385, VSC7388, VSC7395
and VSC7398 SparX integrated ethernet switches in SPI managed mode.
+
+config NET_DSA_VITESSE_VSC73XX_PLATFORM
+ tristate "Vitesse VSC7385/7388/7395/7398 Platform mode support"
+ depends on HAS_IOMEM
+ select NET_DSA_VITESSE_VSC73XX
+ ---help---
+ This enables support for the Vitesse VSC7385, VSC7388, VSC7395
+ and VSC7398 SparX integrated ethernet switches in Platform managed mode.
endmenu
diff --git a/drivers/net/dsa/Makefile b/drivers/net/dsa/Makefile
index 117bf78be211..d5e4c668ac03 100644
--- a/drivers/net/dsa/Makefile
+++ b/drivers/net/dsa/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_NET_DSA_SMSC_LAN9303) += lan9303-core.o
obj-$(CONFIG_NET_DSA_SMSC_LAN9303_I2C) += lan9303_i2c.o
obj-$(CONFIG_NET_DSA_SMSC_LAN9303_MDIO) += lan9303_mdio.o
obj-$(CONFIG_NET_DSA_VITESSE_VSC73XX) += vitesse-vsc73xx-core.o
+obj-$(CONFIG_NET_DSA_VITESSE_VSC73XX_PLATFORM) += vitesse-vsc73xx-platform.o
obj-$(CONFIG_NET_DSA_VITESSE_VSC73XX_SPI) += vitesse-vsc73xx-spi.o
obj-y += b53/
obj-y += microchip/
diff --git a/drivers/net/dsa/vitesse-vsc73xx-platform.c b/drivers/net/dsa/vitesse-vsc73xx-platform.c
new file mode 100644
index 000000000000..149d921b8499
--- /dev/null
+++ b/drivers/net/dsa/vitesse-vsc73xx-platform.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: GPL-2.0
+/* DSA driver for:
+ * Vitesse VSC7385 SparX-G5 5+1-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7388 SparX-G8 8-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7395 SparX-G5e 5+1-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7398 SparX-G8e 8-port Integrated Gigabit Ethernet Switch
+ *
+ * This driver takes control of the switch chip over Platform and
+ * configures it to route packages around when connected to a CPU port.
+ *
+ * Copyright (C) 2019 pawel Dembicki <paweldembicki@gmail.com>
+ * Based on vitesse-vsc-spi.c by:
+ * Copyright (C) 2018 Linus Wallej <linus.walleij@linaro.org>
+ * Includes portions of code from the firmware uploader by:
+ * Copyright (C) 2009 Gabor Juhos <juhosg@openwrt.org>
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include "vitesse-vsc73xx.h"
+
+#define VSC73XX_CMD_PLATFORM_BLOCK_SHIFT 14
+#define VSC73XX_CMD_PLATFORM_BLOCK_MASK 0x7
+#define VSC73XX_CMD_PLATFORM_SUBBLOCK_SHIFT 10
+#define VSC73XX_CMD_PLATFORM_SUBBLOCK_MASK 0xf
+#define VSC73XX_CMD_PLATFORM_REGISTER_SHIFT 2
+
+/**
+ * struct vsc73xx_platform - VSC73xx Platform state container
+ */
+struct vsc73xx_platform {
+ struct platform_device *pdev;
+ void __iomem *base_addr;
+ struct vsc73xx vsc;
+};
+
+static const struct vsc73xx_ops vsc73xx_platform_ops;
+
+static u32 vsc73xx_make_addr(u8 block, u8 subblock, u8 reg)
+{
+ u32 ret;
+
+ ret = (block & VSC73XX_CMD_PLATFORM_BLOCK_MASK)
+ << VSC73XX_CMD_PLATFORM_BLOCK_SHIFT;
+ ret |= (subblock & VSC73XX_CMD_PLATFORM_SUBBLOCK_MASK)
+ << VSC73XX_CMD_PLATFORM_SUBBLOCK_SHIFT;
+ ret |= reg << VSC73XX_CMD_PLATFORM_REGISTER_SHIFT;
+
+ return ret;
+}
+
+static int vsc73xx_platform_read(struct vsc73xx *vsc, u8 block, u8 subblock,
+ u8 reg, u32 *val)
+{
+ struct vsc73xx_platform *vsc_platform = vsc->priv;
+ u32 offset;
+
+ if (!vsc73xx_is_addr_valid(block, subblock))
+ return -EINVAL;
+
+ offset = vsc73xx_make_addr(block, subblock, reg);
+ *val = ioread32be(vsc_platform->base_addr + offset);
+
+ return 0;
+}
+
+static int vsc73xx_platform_write(struct vsc73xx *vsc, u8 block, u8 subblock,
+ u8 reg, u32 val)
+{
+ struct vsc73xx_platform *vsc_platform = vsc->priv;
+ u32 offset;
+
+ if (!vsc73xx_is_addr_valid(block, subblock))
+ return -EINVAL;
+
+ offset = vsc73xx_make_addr(block, subblock, reg);
+ iowrite32be(val, vsc_platform->base_addr + offset);
+
+ return 0;
+}
+
+static int vsc73xx_platform_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct vsc73xx_platform *vsc_platform;
+ struct resource *res = NULL;
+ int ret;
+
+ vsc_platform = devm_kzalloc(dev, sizeof(*vsc_platform), GFP_KERNEL);
+ if (!vsc_platform)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, vsc_platform);
+ vsc_platform->pdev = pdev;
+ vsc_platform->vsc.dev = dev;
+ vsc_platform->vsc.priv = vsc_platform;
+ vsc_platform->vsc.ops = &vsc73xx_platform_ops;
+
+ /* obtain I/O memory space */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(&pdev->dev, "cannot obtain I/O memory space\n");
+ ret = -ENXIO;
+ return ret;
+ }
+
+ vsc_platform->base_addr = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(vsc_platform->base_addr)) {
+ dev_err(&pdev->dev, "cannot request I/O memory space\n");
+ ret = -ENXIO;
+ return ret;
+ }
+
+ return vsc73xx_probe(&vsc_platform->vsc);
+}
+
+static int vsc73xx_platform_remove(struct platform_device *pdev)
+{
+ struct vsc73xx_platform *vsc_platform = platform_get_drvdata(pdev);
+
+ return vsc73xx_remove(&vsc_platform->vsc);
+}
+
+static const struct vsc73xx_ops vsc73xx_platform_ops = {
+ .read = vsc73xx_platform_read,
+ .write = vsc73xx_platform_write,
+};
+
+static const struct of_device_id vsc73xx_of_match[] = {
+ {
+ .compatible = "vitesse,vsc7385",
+ },
+ {
+ .compatible = "vitesse,vsc7388",
+ },
+ {
+ .compatible = "vitesse,vsc7395",
+ },
+ {
+ .compatible = "vitesse,vsc7398",
+ },
+ { },
+};
+MODULE_DEVICE_TABLE(of, vsc73xx_of_match);
+
+static struct platform_driver vsc73xx_platform_driver = {
+ .probe = vsc73xx_platform_probe,
+ .remove = vsc73xx_platform_remove,
+ .driver = {
+ .name = "vsc73xx-platform",
+ .of_match_table = vsc73xx_of_match,
+ },
+};
+module_platform_driver(vsc73xx_platform_driver);
+
+MODULE_AUTHOR("Pawel Dembicki <paweldembicki@gmail.com>");
+MODULE_DESCRIPTION("Vitesse VSC7385/7388/7395/7398 Platform driver");
+MODULE_LICENSE("GPL v2");
--
2.20.1
^ permalink raw reply related
* Re: [PATCH bpf-next v2] libbpf: add xsk_ring_prod__nb_free() function
From: Eelco Chaudron @ 2019-07-03 9:10 UTC (permalink / raw)
To: Magnus Karlsson
Cc: Network Development, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko
In-Reply-To: <CAJ8uoz3BoLiM04WW=91wYryrVBqj5GDsL5mvDaAyBAv-6MNbsQ@mail.gmail.com>
On 28 Jun 2019, at 12:14, Magnus Karlsson wrote:
> On Wed, Jun 26, 2019 at 10:33 AM Eelco Chaudron <echaudro@redhat.com>
> wrote:
>>
>> When an AF_XDP application received X packets, it does not mean X
>> frames can be stuffed into the producer ring. To make it easier for
>> AF_XDP applications this API allows them to check how many frames can
>> be added into the ring.
>>
>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>> ---
>>
>> v1 -> v2
>> - Renamed xsk_ring_prod__free() to xsk_ring_prod__nb_free()
>> - Add caching so it will only touch global state when needed
>>
>> tools/lib/bpf/xsk.h | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
>> index 82ea71a0f3ec..6acb81102346 100644
>> --- a/tools/lib/bpf/xsk.h
>> +++ b/tools/lib/bpf/xsk.h
>> @@ -76,11 +76,11 @@ xsk_ring_cons__rx_desc(const struct xsk_ring_cons
>> *rx, __u32 idx)
>> return &descs[idx & rx->mask];
>> }
>>
>> -static inline __u32 xsk_prod_nb_free(struct xsk_ring_prod *r, __u32
>> nb)
>> +static inline __u32 xsk_prod__nb_free(struct xsk_ring_prod *r, __u32
>> nb)
>> {
>> __u32 free_entries = r->cached_cons - r->cached_prod;
>>
>> - if (free_entries >= nb)
>> + if (free_entries >= nb && nb != 0)
>> return free_entries;
>
> Thanks Eelco for the patch. Is the test nb != 0 introduced here so
> that the function will continue with the refresh from the global state
> when nb is set to 0? If so, could a user not instead just set the nb
> parameter to the size of the ring? This would always trigger a
> refresh, except when the number of free entries is equal to the size
> of the ring, but then we do not need the refresh anyway. This would
> eliminate the nb != 0 test that you introduced from the fast path.
Will remove this change from the fast path, and your suggestion can be
used if circumvention of the cache is needed. Will sent out a v3 soon...
>> /* Refresh the local tail pointer.
>> @@ -110,7 +110,7 @@ static inline __u32 xsk_cons_nb_avail(struct
>> xsk_ring_cons *r, __u32 nb)
>> static inline size_t xsk_ring_prod__reserve(struct xsk_ring_prod
>> *prod,
>> size_t nb, __u32 *idx)
>> {
>> - if (xsk_prod_nb_free(prod, nb) < nb)
>> + if (xsk_prod__nb_free(prod, nb) < nb)
>> return 0;
>>
>> *idx = prod->cached_prod;
>> --
>> 2.20.1
>>
^ permalink raw reply
* [PATCH v2 4/4] net: dsa: vsc73xx: Assert reset if iCPU is enabled
From: Pawel Dembicki @ 2019-07-03 9:10 UTC (permalink / raw)
Cc: paweldembicki, linus.walleij, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Rob Herring, Mark Rutland,
netdev, devicetree, linux-kernel
In-Reply-To: <20190701152723.624-4-paweldembicki@gmail.com>
Driver allow to use devices with disabled iCPU only.
Some devices have pre-initialised iCPU by bootloader.
That state make switch unmanaged. This patch force reset
if device is in unmanaged state. In the result chip lost
internal firmware from RAM and it can be managed.
Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
Changes in v2:
- rebase commit after changes 1-3/4 patches
drivers/net/dsa/vitesse-vsc73xx-core.c | 36 ++++++++++++--------------
1 file changed, 17 insertions(+), 19 deletions(-)
diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
index 10063f31d9a3..4525702faf68 100644
--- a/drivers/net/dsa/vitesse-vsc73xx-core.c
+++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
@@ -417,22 +417,8 @@ static int vsc73xx_detect(struct vsc73xx *vsc)
}
if (val == 0xffffffff) {
- dev_info(vsc->dev, "chip seems dead, assert reset\n");
- gpiod_set_value_cansleep(vsc->reset, 1);
- /* Reset pulse should be 20ns minimum, according to datasheet
- * table 245, so 10us should be fine
- */
- usleep_range(10, 100);
- gpiod_set_value_cansleep(vsc->reset, 0);
- /* Wait 20ms according to datasheet table 245 */
- msleep(20);
-
- ret = vsc73xx_read(vsc, VSC73XX_BLOCK_SYSTEM, 0,
- VSC73XX_ICPU_MBOX_VAL, &val);
- if (val == 0xffffffff) {
- dev_err(vsc->dev, "seems not to help, giving up\n");
- return -ENODEV;
- }
+ dev_info(vsc->dev, "chip seems dead.\n");
+ return -EAGAIN;
}
ret = vsc73xx_read(vsc, VSC73XX_BLOCK_SYSTEM, 0,
@@ -483,9 +469,8 @@ static int vsc73xx_detect(struct vsc73xx *vsc)
}
if (icpu_si_boot_en && !icpu_pi_en) {
dev_err(vsc->dev,
- "iCPU enabled boots from SI, no external memory\n");
- dev_err(vsc->dev, "no idea how to deal with this\n");
- return -ENODEV;
+ "iCPU enabled boots from PI/SI, no external memory\n");
+ return -EAGAIN;
}
if (!icpu_si_boot_en && icpu_pi_en) {
dev_err(vsc->dev,
@@ -1158,6 +1143,19 @@ int vsc73xx_probe(struct vsc73xx *vsc)
msleep(20);
ret = vsc73xx_detect(vsc);
+ if (ret == -EAGAIN) {
+ dev_err(vsc->dev,
+ "Chip seams to be out of control. Assert reset and try again.\n");
+ gpiod_set_value_cansleep(vsc->reset, 1);
+ /* Reset pulse should be 20ns minimum, according to datasheet
+ * table 245, so 10us should be fine
+ */
+ usleep_range(10, 100);
+ gpiod_set_value_cansleep(vsc->reset, 0);
+ /* Wait 20ms according to datasheet table 245 */
+ msleep(20);
+ ret = vsc73xx_detect(vsc);
+ }
if (ret) {
dev_err(dev, "no chip found (%d)\n", ret);
return -ENODEV;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2 0/3] vsock/virtio: several fixes in the .probe() and .remove()
From: Stefan Hajnoczi @ 2019-07-03 9:14 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, kvm, Michael S. Tsirkin, linux-kernel, virtualization,
Stefan Hajnoczi, David S. Miller
In-Reply-To: <20190701170357.jtuhy3ank7mv6izb@steredhat>
[-- Attachment #1: Type: text/plain, Size: 2545 bytes --]
On Mon, Jul 01, 2019 at 07:03:57PM +0200, Stefano Garzarella wrote:
> On Mon, Jul 01, 2019 at 04:11:13PM +0100, Stefan Hajnoczi wrote:
> > On Fri, Jun 28, 2019 at 02:36:56PM +0200, Stefano Garzarella wrote:
> > > During the review of "[PATCH] vsock/virtio: Initialize core virtio vsock
> > > before registering the driver", Stefan pointed out some possible issues
> > > in the .probe() and .remove() callbacks of the virtio-vsock driver.
> > >
> > > This series tries to solve these issues:
> > > - Patch 1 adds RCU critical sections to avoid use-after-free of
> > > 'the_virtio_vsock' pointer.
> > > - Patch 2 stops workers before to call vdev->config->reset(vdev) to
> > > be sure that no one is accessing the device.
> > > - Patch 3 moves the works flush at the end of the .remove() to avoid
> > > use-after-free of 'vsock' object.
> > >
> > > v2:
> > > - Patch 1: use RCU to protect 'the_virtio_vsock' pointer
> > > - Patch 2: no changes
> > > - Patch 3: flush works only at the end of .remove()
> > > - Removed patch 4 because virtqueue_detach_unused_buf() returns all the buffers
> > > allocated.
> > >
> > > v1: https://patchwork.kernel.org/cover/10964733/
> >
> > This looks good to me.
>
> Thanks for the review!
>
> >
> > Did you run any stress tests? For example an SMP guest constantly
> > connecting and sending packets together with a script that
> > hotplug/unplugs vhost-vsock-pci from the host side.
>
> Yes, I started an SMP guest (-smp 4 -monitor tcp:127.0.0.1:1234,server,nowait)
> and I run these scripts to stress the .probe()/.remove() path:
>
> - guest
> while true; do
> cat /dev/urandom | nc-vsock -l 4321 > /dev/null &
> cat /dev/urandom | nc-vsock -l 5321 > /dev/null &
> cat /dev/urandom | nc-vsock -l 6321 > /dev/null &
> cat /dev/urandom | nc-vsock -l 7321 > /dev/null &
> wait
> done
>
> - host
> while true; do
> cat /dev/urandom | nc-vsock 3 4321 > /dev/null &
> cat /dev/urandom | nc-vsock 3 5321 > /dev/null &
> cat /dev/urandom | nc-vsock 3 6321 > /dev/null &
> cat /dev/urandom | nc-vsock 3 7321 > /dev/null &
> sleep 2
> echo "device_del v1" | nc 127.0.0.1 1234
> sleep 1
> echo "device_add vhost-vsock-pci,id=v1,guest-cid=3" | nc 127.0.0.1 1234
> sleep 1
> done
>
> Do you think is enough or is better to have a test more accurate?
That's good when left running overnight so that thousands of hotplug
events are tested.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [RFC v2] vhost: introduce mdev based hardware vhost backend
From: Tiwei Bie @ 2019-07-03 9:13 UTC (permalink / raw)
To: mst, jasowang, alex.williamson, maxime.coquelin
Cc: linux-kernel, kvm, virtualization, netdev, dan.daly,
cunming.liang, zhihong.wang, tiwei.bie
Details about this can be found here:
https://lwn.net/Articles/750770/
What's new in this version
==========================
A new VFIO device type is introduced - vfio-vhost. This addressed
some comments from here: https://patchwork.ozlabs.org/cover/984763/
Below is the updated device interface:
Currently, there are two regions of this device: 1) CONFIG_REGION
(VFIO_VHOST_CONFIG_REGION_INDEX), which can be used to setup the
device; 2) NOTIFY_REGION (VFIO_VHOST_NOTIFY_REGION_INDEX), which
can be used to notify the device.
1. CONFIG_REGION
The region described by CONFIG_REGION is the main control interface.
Messages will be written to or read from this region.
The message type is determined by the `request` field in message
header. The message size is encoded in the message header too.
The message format looks like this:
struct vhost_vfio_op {
__u64 request;
__u32 flags;
/* Flag values: */
#define VHOST_VFIO_NEED_REPLY 0x1 /* Whether need reply */
__u32 size;
union {
__u64 u64;
struct vhost_vring_state state;
struct vhost_vring_addr addr;
} payload;
};
The existing vhost-kernel ioctl cmds are reused as the message
requests in above structure.
Each message will be written to or read from this region at offset 0:
int vhost_vfio_write(struct vhost_dev *dev, struct vhost_vfio_op *op)
{
int count = VHOST_VFIO_OP_HDR_SIZE + op->size;
struct vhost_vfio *vfio = dev->opaque;
int ret;
ret = pwrite64(vfio->device_fd, op, count, vfio->config_offset);
if (ret != count)
return -1;
return 0;
}
int vhost_vfio_read(struct vhost_dev *dev, struct vhost_vfio_op *op)
{
int count = VHOST_VFIO_OP_HDR_SIZE + op->size;
struct vhost_vfio *vfio = dev->opaque;
uint64_t request = op->request;
int ret;
ret = pread64(vfio->device_fd, op, count, vfio->config_offset);
if (ret != count || request != op->request)
return -1;
return 0;
}
It's quite straightforward to set things to the device. Just need to
write the message to device directly:
int vhost_vfio_set_features(struct vhost_dev *dev, uint64_t features)
{
struct vhost_vfio_op op;
op.request = VHOST_SET_FEATURES;
op.flags = 0;
op.size = sizeof(features);
op.payload.u64 = features;
return vhost_vfio_write(dev, &op);
}
To get things from the device, two steps are needed.
Take VHOST_GET_FEATURE as an example:
int vhost_vfio_get_features(struct vhost_dev *dev, uint64_t *features)
{
struct vhost_vfio_op op;
int ret;
op.request = VHOST_GET_FEATURES;
op.flags = VHOST_VFIO_NEED_REPLY;
op.size = 0;
/* Just need to write the header */
ret = vhost_vfio_write(dev, &op);
if (ret != 0)
goto out;
/* `op` wasn't changed during write */
op.flags = 0;
op.size = sizeof(*features);
ret = vhost_vfio_read(dev, &op);
if (ret != 0)
goto out;
*features = op.payload.u64;
out:
return ret;
}
2. NOTIFIY_REGION (mmap-able)
The region described by NOTIFY_REGION will be used to notify
the device.
Each queue will have a page for notification, and it can be mapped
to VM (if hardware also supports), and the virtio driver in the VM
will be able to notify the device directly.
The region described by NOTIFY_REGION is also write-able. If
the accelerator's notification register(s) cannot be mapped to
the VM, write() can also be used to notify the device. Something
like this:
void notify_relay(void *opaque)
{
......
offset = host_page_size * queue_idx;
ret = pwrite64(vfio->device_fd, &queue_idx, sizeof(queue_idx),
vfio->notify_offset + offset);
......
}
3. VFIO interrupt ioctl API
VFIO interrupt ioctl API is used to setup device interrupts.
IRQ-bypass can also be supported.
Currently, the data path interrupt can be configured via the
VFIO_VHOST_VQ_IRQ_INDEX with virtqueue's callfd.
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
drivers/vhost/Makefile | 2 +
drivers/vhost/vdpa.c | 770 +++++++++++++++++++++++++++++++++++++
include/linux/vdpa_mdev.h | 72 ++++
include/uapi/linux/vfio.h | 19 +
include/uapi/linux/vhost.h | 25 ++
5 files changed, 888 insertions(+)
create mode 100644 drivers/vhost/vdpa.c
create mode 100644 include/linux/vdpa_mdev.h
diff --git a/drivers/vhost/Makefile b/drivers/vhost/Makefile
index 6c6df24f770c..cabb71095940 100644
--- a/drivers/vhost/Makefile
+++ b/drivers/vhost/Makefile
@@ -10,4 +10,6 @@ vhost_vsock-y := vsock.o
obj-$(CONFIG_VHOST_RING) += vringh.o
+obj-$(CONFIG_VHOST_VFIO) += vdpa.o
+
obj-$(CONFIG_VHOST) += vhost.o
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
new file mode 100644
index 000000000000..5c9426e2a091
--- /dev/null
+++ b/drivers/vhost/vdpa.c
@@ -0,0 +1,770 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018-2019 Intel Corporation.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/vfio.h>
+#include <linux/vhost.h>
+#include <linux/mdev.h>
+#include <linux/vdpa_mdev.h>
+#include <asm/uaccess.h>
+
+#define VDPA_CONFIG_SIZE 0x1000000
+
+#define VDPA_VFIO_VHOST_OFFSET_SHIFT 40
+#define VDPA_VFIO_VHOST_OFFSET_MASK \
+ ((1ULL << VDPA_VFIO_VHOST_OFFSET_SHIFT) - 1)
+#define VDPA_VFIO_VHOST_OFFSET_TO_INDEX(offset) \
+ ((offset) >> VDPA_VFIO_VHOST_OFFSET_SHIFT)
+#define VDPA_VFIO_VHOST_INDEX_TO_OFFSET(index) \
+ ((u64)(index) << VDPA_VFIO_VHOST_OFFSET_SHIFT)
+#define VDPA_VFIO_VHOST_REGION_OFFSET(offset) \
+ ((offset) & VDPA_VFIO_VHOST_OFFSET_MASK)
+
+struct vdpa_dev *vdpa_alloc(struct mdev_device *mdev, void *private,
+ int max_vrings)
+{
+ struct vdpa_dev *vdpa;
+ size_t size;
+
+ size = sizeof(struct vdpa_dev) + max_vrings *
+ sizeof(struct vdpa_vring_info);
+
+ vdpa = kzalloc(size, GFP_KERNEL);
+ if (vdpa == NULL)
+ return NULL;
+
+ mutex_init(&vdpa->ops_lock);
+
+ vdpa->mdev = mdev;
+ vdpa->private = private;
+ vdpa->max_vrings = max_vrings;
+
+ return vdpa;
+}
+EXPORT_SYMBOL(vdpa_alloc);
+
+void vdpa_free(struct vdpa_dev *vdpa)
+{
+ struct mdev_device *mdev;
+
+ mdev = vdpa->mdev;
+
+ vdpa->ops->stop(vdpa);
+ mdev_set_drvdata(mdev, NULL);
+ mutex_destroy(&vdpa->ops_lock);
+ kfree(vdpa);
+}
+EXPORT_SYMBOL(vdpa_free);
+
+static ssize_t vdpa_handle_config_read(struct mdev_device *mdev,
+ char __user *buf, size_t count, loff_t *ppos)
+{
+ struct vdpa_dev *vdpa;
+ struct vhost_vfio_op *op = NULL;
+ loff_t pos = *ppos;
+ loff_t offset;
+ int ret;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ offset = VDPA_VFIO_VHOST_REGION_OFFSET(pos);
+ if (offset != 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!vdpa->pending_reply) {
+ ret = 0;
+ goto out;
+ }
+
+ vdpa->pending_reply = false;
+
+ op = kzalloc(VHOST_VFIO_OP_HDR_SIZE + VHOST_VFIO_OP_PAYLOAD_MAX_SIZE,
+ GFP_KERNEL);
+ if (op == NULL) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ op->request = vdpa->pending.request;
+
+ switch (op->request) {
+ case VHOST_GET_VRING_BASE:
+ op->payload.state = vdpa->pending.payload.state;
+ op->size = sizeof(op->payload.state);
+ break;
+ case VHOST_GET_FEATURES:
+ op->payload.u64 = vdpa->pending.payload.u64;
+ op->size = sizeof(op->payload.u64);
+ break;
+ default:
+ ret = -EINVAL;
+ goto out_free;
+ }
+
+ if (op->size + VHOST_VFIO_OP_HDR_SIZE != count) {
+ ret = -EINVAL;
+ goto out_free;
+ }
+
+ if (copy_to_user(buf, op, count)) {
+ ret = -EFAULT;
+ goto out_free;
+ }
+
+ ret = count;
+
+out_free:
+ kfree(op);
+out:
+ return ret;
+}
+
+ssize_t vdpa_read(struct mdev_device *mdev, char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ int done = 0;
+ unsigned int index;
+ loff_t pos = *ppos;
+ struct vdpa_dev *vdpa;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ mutex_lock(&vdpa->ops_lock);
+
+ index = VDPA_VFIO_VHOST_OFFSET_TO_INDEX(pos);
+
+ switch (index) {
+ case VFIO_VHOST_CONFIG_REGION_INDEX:
+ done = vdpa_handle_config_read(mdev, buf, count, ppos);
+ break;
+ }
+
+ if (done > 0)
+ *ppos += done;
+
+ mutex_unlock(&vdpa->ops_lock);
+
+ return done;
+}
+EXPORT_SYMBOL(vdpa_read);
+
+static int vhost_set_vring_addr(struct mdev_device *mdev,
+ struct vhost_vring_addr *addr)
+{
+ struct vdpa_dev *vdpa;
+ int qid = addr->index;
+ struct vdpa_vring_info *vring;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ if (qid >= vdpa->max_vrings)
+ return -EINVAL;
+
+ if (qid >= vdpa->nr_vring)
+ vdpa->nr_vring = qid + 1;
+
+ vring = &vdpa->vring_info[qid];
+
+ vring->desc_user_addr = addr->desc_user_addr;
+ vring->used_user_addr = addr->used_user_addr;
+ vring->avail_user_addr = addr->avail_user_addr;
+ vring->log_guest_addr = addr->log_guest_addr;
+
+ return 0;
+}
+
+static int vhost_set_vring_num(struct mdev_device *mdev,
+ struct vhost_vring_state *num)
+{
+ struct vdpa_dev *vdpa;
+ int qid = num->index;
+ struct vdpa_vring_info *vring;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ if (qid >= vdpa->max_vrings)
+ return -EINVAL;
+
+ vring = &vdpa->vring_info[qid];
+
+ vring->size = num->num;
+
+ return 0;
+}
+
+static int vhost_set_vring_base(struct mdev_device *mdev,
+ struct vhost_vring_state *base)
+{
+ struct vdpa_dev *vdpa;
+ int qid = base->index;
+ struct vdpa_vring_info *vring;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ if (qid >= vdpa->max_vrings)
+ return -EINVAL;
+
+ vring = &vdpa->vring_info[qid];
+
+ vring->base = base->num;
+
+ return 0;
+}
+
+static int vhost_get_vring_base(struct mdev_device *mdev,
+ struct vhost_vring_state *base)
+{
+ struct vdpa_dev *vdpa;
+ int qid = base->index;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ vdpa->pending_reply = true;
+ vdpa->pending.request = VHOST_GET_VRING_BASE;
+ vdpa->pending.payload.state.index = qid;
+ vdpa->pending.payload.state.num = vdpa->vring_info[qid].base;
+
+ return 0;
+}
+
+static int vhost_set_log_base(struct mdev_device *mdev, u64 *log_base)
+{
+ struct vdpa_dev *vdpa;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ vdpa->log_base = *log_base;
+ return 0;
+}
+
+static int vhost_set_features(struct mdev_device *mdev, u64 *features)
+{
+ struct vdpa_dev *vdpa;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ vdpa->features = *features;
+ vdpa->ops->set_features(vdpa);
+
+ return 0;
+}
+
+static int vhost_get_features(struct mdev_device *mdev, u64 *features)
+{
+ struct vdpa_dev *vdpa;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ vdpa->pending_reply = true;
+ vdpa->pending.request = VHOST_GET_FEATURES;
+ vdpa->pending.payload.u64 =
+ vdpa->ops->supported_features(vdpa);
+
+ return 0;
+}
+
+static int vhost_set_owner(struct mdev_device *mdev)
+{
+ // TODO
+ return 0;
+}
+
+static int vhost_reset_owner(struct mdev_device *mdev)
+{
+ // TODO
+ return 0;
+}
+
+static int vhost_set_state(struct mdev_device *mdev, u64 *state)
+{
+ struct vdpa_dev *vdpa;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ if (*state >= VHOST_DEVICE_S_MAX)
+ return -EINVAL;
+
+ if (vdpa->state == *state)
+ return 0;
+
+ vdpa->state = *state;
+
+ switch (vdpa->state) {
+ case VHOST_DEVICE_S_RUNNING:
+ vdpa->ops->start(vdpa);
+ break;
+ case VHOST_DEVICE_S_STOPPED:
+ vdpa->ops->stop(vdpa);
+ break;
+ }
+
+ return 0;
+}
+
+static ssize_t vdpa_handle_config_write(struct mdev_device *mdev,
+ const char __user *buf, size_t count, loff_t *ppos)
+{
+ struct vhost_vfio_op *op = NULL;
+ loff_t pos = *ppos;
+ loff_t offset;
+ int ret;
+
+ offset = VDPA_VFIO_VHOST_REGION_OFFSET(pos);
+ if (offset != 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (count < VHOST_VFIO_OP_HDR_SIZE) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ op = kzalloc(VHOST_VFIO_OP_HDR_SIZE + VHOST_VFIO_OP_PAYLOAD_MAX_SIZE,
+ GFP_KERNEL);
+ if (op == NULL) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ if (copy_from_user(op, buf, VHOST_VFIO_OP_HDR_SIZE)) {
+ ret = -EINVAL;
+ goto out_free;
+ }
+
+ if (op->size > VHOST_VFIO_OP_PAYLOAD_MAX_SIZE ||
+ op->size + VHOST_VFIO_OP_HDR_SIZE != count) {
+ ret = -EINVAL;
+ goto out_free;
+ }
+
+ if (copy_from_user(&op->payload, buf + VHOST_VFIO_OP_HDR_SIZE,
+ op->size)) {
+ ret = -EFAULT;
+ goto out_free;
+ }
+
+ switch (op->request) {
+ case VHOST_SET_LOG_BASE:
+ vhost_set_log_base(mdev, &op->payload.u64);
+ break;
+ case VHOST_SET_VRING_ADDR:
+ vhost_set_vring_addr(mdev, &op->payload.addr);
+ break;
+ case VHOST_SET_VRING_NUM:
+ vhost_set_vring_num(mdev, &op->payload.state);
+ break;
+ case VHOST_SET_VRING_BASE:
+ vhost_set_vring_base(mdev, &op->payload.state);
+ break;
+ case VHOST_GET_VRING_BASE:
+ vhost_get_vring_base(mdev, &op->payload.state);
+ break;
+ case VHOST_SET_FEATURES:
+ vhost_set_features(mdev, &op->payload.u64);
+ break;
+ case VHOST_GET_FEATURES:
+ vhost_get_features(mdev, &op->payload.u64);
+ break;
+ case VHOST_SET_OWNER:
+ vhost_set_owner(mdev);
+ break;
+ case VHOST_RESET_OWNER:
+ vhost_reset_owner(mdev);
+ break;
+ case VHOST_DEVICE_SET_STATE:
+ vhost_set_state(mdev, &op->payload.u64);
+ break;
+ default:
+ break;
+ }
+
+ ret = count;
+
+out_free:
+ kfree(op);
+out:
+ return ret;
+}
+
+static ssize_t vdpa_handle_notify_write(struct mdev_device *mdev,
+ const char __user *buf, size_t count, loff_t *ppos)
+{
+ struct vdpa_dev *vdpa;
+ int qid;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ if (count < sizeof(qid))
+ return -EINVAL;
+
+ if (copy_from_user(&qid, buf, sizeof(qid)))
+ return -EINVAL;
+
+ vdpa->ops->notify(vdpa, qid);
+
+ return count;
+}
+
+ssize_t vdpa_write(struct mdev_device *mdev, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ int done = 0;
+ unsigned int index;
+ loff_t pos = *ppos;
+ struct vdpa_dev *vdpa;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ mutex_lock(&vdpa->ops_lock);
+
+ index = VDPA_VFIO_VHOST_OFFSET_TO_INDEX(pos);
+
+ switch (index) {
+ case VFIO_VHOST_CONFIG_REGION_INDEX:
+ done = vdpa_handle_config_write(mdev, buf, count, ppos);
+ break;
+ case VFIO_VHOST_NOTIFY_REGION_INDEX:
+ done = vdpa_handle_notify_write(mdev, buf, count, ppos);
+ break;
+ }
+
+ if (done > 0)
+ *ppos += done;
+
+ mutex_unlock(&vdpa->ops_lock);
+
+ return done;
+}
+EXPORT_SYMBOL(vdpa_write);
+
+static int vdpa_get_region_info(struct mdev_device *mdev,
+ struct vfio_region_info *region_info,
+ u16 *cap_type_id, void **cap_type)
+{
+ struct vdpa_dev *vdpa;
+ u32 index, flags;
+ u64 size = 0;
+
+ if (!mdev)
+ return -EINVAL;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -EINVAL;
+
+ index = region_info->index;
+ if (index >= VFIO_VHOST_NUM_REGIONS)
+ return -EINVAL;
+
+ mutex_lock(&vdpa->ops_lock);
+
+ flags = VFIO_REGION_INFO_FLAG_READ | VFIO_REGION_INFO_FLAG_WRITE;
+
+ switch (index) {
+ case VFIO_VHOST_CONFIG_REGION_INDEX:
+ size = VDPA_CONFIG_SIZE;
+ break;
+ case VFIO_VHOST_NOTIFY_REGION_INDEX:
+ size = (u64)vdpa->max_vrings << PAGE_SHIFT;
+ flags |= VFIO_REGION_INFO_FLAG_MMAP;
+ break;
+ default:
+ size = 0;
+ break;
+ }
+
+ region_info->size = size;
+ region_info->offset = VDPA_VFIO_VHOST_INDEX_TO_OFFSET(index);
+ region_info->flags = flags;
+ mutex_unlock(&vdpa->ops_lock);
+ return 0;
+}
+
+static int vdpa_reset(struct mdev_device *mdev)
+{
+ struct vdpa_dev *vdpa;
+
+ if (!mdev)
+ return -EINVAL;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int vdpa_get_device_info(struct mdev_device *mdev,
+ struct vfio_device_info *dev_info)
+{
+ struct vdpa_dev *vdpa;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ dev_info->flags = VFIO_DEVICE_FLAGS_VHOST | VFIO_DEVICE_RESET;
+ dev_info->num_regions = VFIO_VHOST_NUM_REGIONS;
+ dev_info->num_irqs = VFIO_VHOST_NUM_IRQS;
+
+ return 0;
+}
+
+static int vdpa_get_irq_info(struct mdev_device *mdev,
+ struct vfio_irq_info *info)
+{
+ struct vdpa_dev *vdpa;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ if (info->index != VFIO_VHOST_VQ_IRQ_INDEX)
+ return -EINVAL;
+
+ info->flags = VFIO_IRQ_INFO_EVENTFD;
+ info->count = vdpa->max_vrings;
+
+ return 0;
+}
+
+static int vdpa_set_irqs(struct mdev_device *mdev, uint32_t flags,
+ unsigned int index, unsigned int start,
+ unsigned int count, void *data)
+{
+ struct vdpa_dev *vdpa;
+ int *fd = data, i;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -EINVAL;
+
+ if (index != VFIO_VHOST_VQ_IRQ_INDEX)
+ return -ENOTSUPP;
+
+ for (i = 0; i < count; i++)
+ vdpa->ops->set_eventfd(vdpa, start + i,
+ (flags & VFIO_IRQ_SET_DATA_EVENTFD) ? fd[i] : -1);
+
+ return 0;
+}
+
+long vdpa_ioctl(struct mdev_device *mdev, unsigned int cmd, unsigned long arg)
+{
+ int ret = 0;
+ unsigned long minsz;
+ struct vdpa_dev *vdpa;
+
+ if (!mdev)
+ return -EINVAL;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ switch (cmd) {
+ case VFIO_DEVICE_GET_INFO:
+ {
+ struct vfio_device_info info;
+
+ minsz = offsetofend(struct vfio_device_info, num_irqs);
+
+ if (copy_from_user(&info, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (info.argsz < minsz)
+ return -EINVAL;
+
+ ret = vdpa_get_device_info(mdev, &info);
+ if (ret)
+ return ret;
+
+ if (copy_to_user((void __user *)arg, &info, minsz))
+ return -EFAULT;
+
+ return 0;
+ }
+ case VFIO_DEVICE_GET_REGION_INFO:
+ {
+ struct vfio_region_info info;
+ u16 cap_type_id = 0;
+ void *cap_type = NULL;
+
+ minsz = offsetofend(struct vfio_region_info, offset);
+
+ if (copy_from_user(&info, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (info.argsz < minsz)
+ return -EINVAL;
+
+ ret = vdpa_get_region_info(mdev, &info, &cap_type_id,
+ &cap_type);
+ if (ret)
+ return ret;
+
+ if (copy_to_user((void __user *)arg, &info, minsz))
+ return -EFAULT;
+
+ return 0;
+ }
+ case VFIO_DEVICE_GET_IRQ_INFO:
+ {
+ struct vfio_irq_info info;
+
+ minsz = offsetofend(struct vfio_irq_info, count);
+
+ if (copy_from_user(&info, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (info.argsz < minsz || info.index >= vdpa->max_vrings)
+ return -EINVAL;
+
+ ret = vdpa_get_irq_info(mdev, &info);
+ if (ret)
+ return ret;
+
+ if (copy_to_user((void __user *)arg, &info, minsz))
+ return -EFAULT;
+
+ return 0;
+ }
+ case VFIO_DEVICE_SET_IRQS:
+ {
+ struct vfio_irq_set hdr;
+ size_t data_size = 0;
+ u8 *data = NULL;
+
+ minsz = offsetofend(struct vfio_irq_set, count);
+
+ if (copy_from_user(&hdr, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ ret = vfio_set_irqs_validate_and_prepare(&hdr, vdpa->max_vrings,
+ VFIO_VHOST_NUM_IRQS,
+ &data_size);
+ if (ret)
+ return ret;
+
+ if (data_size) {
+ data = memdup_user((void __user *)(arg + minsz),
+ data_size);
+ if (IS_ERR(data))
+ return PTR_ERR(data);
+ }
+
+ ret = vdpa_set_irqs(mdev, hdr.flags, hdr.index, hdr.start,
+ hdr.count, data);
+
+ kfree(data);
+ return ret;
+ }
+ case VFIO_DEVICE_RESET:
+ return vdpa_reset(mdev);
+ }
+ return -ENOTTY;
+}
+EXPORT_SYMBOL(vdpa_ioctl);
+
+static const struct vm_operations_struct vdpa_mm_ops = {
+#ifdef CONFIG_HAVE_IOREMAP_PROT
+ .access = generic_access_phys
+#endif
+};
+
+int vdpa_mmap(struct mdev_device *mdev, struct vm_area_struct *vma)
+{
+ struct vdpa_dev *vdpa;
+ unsigned int index;
+ loff_t pos;
+ loff_t offset;
+ int qid, ret;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ pos = vma->vm_pgoff << PAGE_SHIFT;
+
+ index = VDPA_VFIO_VHOST_OFFSET_TO_INDEX(pos);
+ offset = VDPA_VFIO_VHOST_REGION_OFFSET(pos);
+
+ qid = offset >> PAGE_SHIFT;
+
+ if (vma->vm_end < vma->vm_start)
+ return -EINVAL;
+ if ((vma->vm_flags & VM_SHARED) == 0)
+ return -EINVAL;
+ if (index != VFIO_VHOST_NOTIFY_REGION_INDEX)
+ return -EINVAL;
+ if (qid < 0 || qid >= vdpa->max_vrings)
+ return -EINVAL;
+
+ if (vma->vm_end - vma->vm_start > PAGE_SIZE)
+ return -EINVAL;
+
+ if (vdpa->ops->get_notify_addr == NULL)
+ return -ENOTSUPP;
+
+ mutex_lock(&vdpa->ops_lock);
+
+ vma->vm_ops = &vdpa_mm_ops;
+ vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+
+ vma->vm_pgoff = vdpa->ops->get_notify_addr(vdpa, qid) >> PAGE_SHIFT;
+
+ ret = remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
+ vma->vm_end - vma->vm_start, vma->vm_page_prot);
+
+ mutex_unlock(&vdpa->ops_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL(vdpa_mmap);
+
+int vdpa_open(struct mdev_device *mdev)
+{
+ return 0;
+}
+EXPORT_SYMBOL(vdpa_open);
+
+void vdpa_close(struct mdev_device *mdev)
+{
+}
+EXPORT_SYMBOL(vdpa_close);
+
+MODULE_VERSION("0.0.0");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Hardware vhost accelerator abstraction");
diff --git a/include/linux/vdpa_mdev.h b/include/linux/vdpa_mdev.h
new file mode 100644
index 000000000000..4bbdf7e2e712
--- /dev/null
+++ b/include/linux/vdpa_mdev.h
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018-2019 Intel Corporation.
+ */
+
+#ifndef VDPA_MDEV_H
+#define VDPA_MDEV_H
+
+struct mdev_device;
+struct vdpa_dev;
+
+/*
+ * XXX: Any comments about the vDPA API design for drivers
+ * would be appreciated!
+ */
+
+typedef int (*vdpa_start_device_t)(struct vdpa_dev *vdpa);
+typedef int (*vdpa_stop_device_t)(struct vdpa_dev *vdpa);
+typedef int (*vdpa_set_features_t)(struct vdpa_dev *vdpa);
+typedef int (*vdpa_set_eventfd_t)(struct vdpa_dev *vdpa, int queue_idx, int fd);
+typedef u64 (*vdpa_supported_features_t)(struct vdpa_dev *vdpa);
+typedef void (*vdpa_notify_device_t)(struct vdpa_dev *vdpa, int queue_idx);
+typedef u64 (*vdpa_get_notify_addr_t)(struct vdpa_dev *vdpa, int queue_idx);
+
+struct vdpa_device_ops {
+ vdpa_start_device_t start;
+ vdpa_stop_device_t stop;
+ vdpa_set_eventfd_t set_eventfd;
+ vdpa_supported_features_t supported_features;
+ vdpa_notify_device_t notify;
+ vdpa_get_notify_addr_t get_notify_addr;
+ vdpa_set_features_t set_features;
+};
+
+struct vdpa_vring_info {
+ u64 desc_user_addr;
+ u64 used_user_addr;
+ u64 avail_user_addr;
+ u64 log_guest_addr;
+ u16 size;
+ u16 base;
+};
+
+struct vdpa_dev {
+ struct mdev_device *mdev;
+ struct mutex ops_lock;
+ int nr_vring;
+ u64 features;
+ u64 state;
+ bool pending_reply;
+ struct vhost_vfio_op pending;
+ const struct vdpa_device_ops *ops;
+ void *private;
+ int max_vrings;
+ uint64_t log_base;
+ uint64_t log_size;
+ struct vdpa_vring_info vring_info[0];
+};
+
+struct vdpa_dev *vdpa_alloc(struct mdev_device *mdev, void *private,
+ int max_vrings);
+void vdpa_free(struct vdpa_dev *vdpa);
+ssize_t vdpa_read(struct mdev_device *mdev, char __user *buf,
+ size_t count, loff_t *ppos);
+ssize_t vdpa_write(struct mdev_device *mdev, const char __user *buf,
+ size_t count, loff_t *ppos);
+long vdpa_ioctl(struct mdev_device *mdev, unsigned int cmd, unsigned long arg);
+int vdpa_mmap(struct mdev_device *mdev, struct vm_area_struct *vma);
+int vdpa_open(struct mdev_device *mdev);
+void vdpa_close(struct mdev_device *mdev);
+
+#endif /* VDPA_MDEV_H */
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 8f10748dac79..6c5718ab7eeb 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -201,6 +201,7 @@ struct vfio_device_info {
#define VFIO_DEVICE_FLAGS_AMBA (1 << 3) /* vfio-amba device */
#define VFIO_DEVICE_FLAGS_CCW (1 << 4) /* vfio-ccw device */
#define VFIO_DEVICE_FLAGS_AP (1 << 5) /* vfio-ap device */
+#define VFIO_DEVICE_FLAGS_VHOST (1 << 6) /* vfio-vhost device */
__u32 num_regions; /* Max region index + 1 */
__u32 num_irqs; /* Max IRQ index + 1 */
};
@@ -217,6 +218,7 @@ struct vfio_device_info {
#define VFIO_DEVICE_API_AMBA_STRING "vfio-amba"
#define VFIO_DEVICE_API_CCW_STRING "vfio-ccw"
#define VFIO_DEVICE_API_AP_STRING "vfio-ap"
+#define VFIO_DEVICE_API_VHOST_STRING "vfio-vhost"
/**
* VFIO_DEVICE_GET_REGION_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 8,
@@ -573,6 +575,23 @@ enum {
VFIO_CCW_NUM_IRQS
};
+/*
+ * The vfio-vhost bus driver makes use of the following fixed region and
+ * IRQ index mapping. Unimplemented regions return a size of zero.
+ * Unimplemented IRQ types return a count of zero.
+ */
+
+enum {
+ VFIO_VHOST_CONFIG_REGION_INDEX,
+ VFIO_VHOST_NOTIFY_REGION_INDEX,
+ VFIO_VHOST_NUM_REGIONS
+};
+
+enum {
+ VFIO_VHOST_VQ_IRQ_INDEX,
+ VFIO_VHOST_NUM_IRQS
+};
+
/**
* VFIO_DEVICE_GET_PCI_HOT_RESET_INFO - _IORW(VFIO_TYPE, VFIO_BASE + 12,
* struct vfio_pci_hot_reset_info)
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index 40d028eed645..ad95b90c5c05 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -116,4 +116,29 @@
#define VHOST_VSOCK_SET_GUEST_CID _IOW(VHOST_VIRTIO, 0x60, __u64)
#define VHOST_VSOCK_SET_RUNNING _IOW(VHOST_VIRTIO, 0x61, int)
+/* VHOST_DEVICE specific defines */
+
+#define VHOST_DEVICE_SET_STATE _IOW(VHOST_VIRTIO, 0x70, __u64)
+
+#define VHOST_DEVICE_S_STOPPED 0
+#define VHOST_DEVICE_S_RUNNING 1
+#define VHOST_DEVICE_S_MAX 2
+
+struct vhost_vfio_op {
+ __u64 request;
+ __u32 flags;
+ /* Flag values: */
+#define VHOST_VFIO_NEED_REPLY 0x1 /* Whether need reply */
+ __u32 size;
+ union {
+ __u64 u64;
+ struct vhost_vring_state state;
+ struct vhost_vring_addr addr;
+ } payload;
+};
+
+#define VHOST_VFIO_OP_HDR_SIZE \
+ ((unsigned long)&((struct vhost_vfio_op *)NULL)->payload)
+#define VHOST_VFIO_OP_PAYLOAD_MAX_SIZE 1024 /* FIXME TBD */
+
#endif
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net] tcp: refine memory limit test in tcp_fragment()
From: Eric Dumazet @ 2019-07-03 9:19 UTC (permalink / raw)
To: Tony Lu, Eric Dumazet
Cc: David S . Miller, netdev, Eric Dumazet, Christoph Paasch,
oliver.yang, xlpang, dust.li
In-Reply-To: <20190703032718.GC55248@TonyMac-Alibaba>
On 7/2/19 8:27 PM, Tony Lu wrote:
> Hello Eric,
>
> We have applied that commit e358f4af19db ("tcp: tcp_fragment() should apply sane memory limits")
> as a hotpatch in production environment. We found that it will make
> tcp long connection reset during sending out packet when applying
> that commit.
>
> Our applications which in A/B test have suffered that
> and made them retransmit large data, and then caused retransmission
> storm and lower the performance and increase RT.
>
> Therefore we discontinued to apply this hotpatch in A/B test.
>
> After invesgation, we found this patch already fix this issue in
> stable. Before applying this patch, we have some questions:
>
Which stable version are you referring to exactly ?
> 1. This commit in stable hard coded a magic number 0x20000. I am
> wondering this value and if there any better solution.
0x20000 is two times 64KB, please read the changelog for the rationale.
> 2. Is there any known or unknown side effect? If any, we could test
> it in some suspicious scenarios before testing in prod env.
No known side effect.
Honestly, applications setting small SO_SNDBUF values can not expect good TCP performance anyway.
>
> Thanks.
>
> Cheers,
> Tony Lu
>
> On Fri, Jun 21, 2019 at 06:09:55AM -0700, Eric Dumazet wrote:
>> tcp_fragment() might be called for skbs in the write queue.
>>
>> Memory limits might have been exceeded because tcp_sendmsg() only
>> checks limits at full skb (64KB) boundaries.
>>
>> Therefore, we need to make sure tcp_fragment() wont punish applications
>> that might have setup very low SO_SNDBUF values.
>>
>> Fixes: f070ef2ac667 ("tcp: tcp_fragment() should apply sane memory limits")
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Reported-by: Christoph Paasch <cpaasch@apple.com>
>> ---
>> net/ipv4/tcp_output.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
>> index 00c01a01b547ec67c971dc25a74c9258563cf871..0ebc33d1c9e5099d163a234930e213ee35e9fbd1 100644
>> --- a/net/ipv4/tcp_output.c
>> +++ b/net/ipv4/tcp_output.c
>> @@ -1296,7 +1296,8 @@ int tcp_fragment(struct sock *sk, enum tcp_queue tcp_queue,
>> if (nsize < 0)
>> nsize = 0;
>>
>> - if (unlikely((sk->sk_wmem_queued >> 1) > sk->sk_sndbuf)) {
>> + if (unlikely((sk->sk_wmem_queued >> 1) > sk->sk_sndbuf &&
>> + tcp_queue != TCP_FRAG_IN_WRITE_QUEUE)) {
>> NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPWQUEUETOOBIG);
>> return -ENOMEM;
>> }
>> --
>> 2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply
* Re: [PATCH mlx5-next 4/5] net/mlx5: Introduce TLS TX offload hardware bits and structures
From: leon @ 2019-07-03 9:27 UTC (permalink / raw)
To: Saeed Mahameed
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
Eran Ben Elisha, Tariq Toukan
In-Reply-To: <20190703073909.14965-5-saeedm@mellanox.com>
On Wed, Jul 03, 2019 at 07:39:32AM +0000, Saeed Mahameed wrote:
> From: Eran Ben Elisha <eranbe@mellanox.com>
>
> Add TLS offload related IFC structs, layouts and enumerations.
>
> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
> include/linux/mlx5/device.h | 14 +++++
> include/linux/mlx5/mlx5_ifc.h | 104 ++++++++++++++++++++++++++++++++--
> 2 files changed, 114 insertions(+), 4 deletions(-)
<...>
> @@ -2725,7 +2739,8 @@ struct mlx5_ifc_traffic_counter_bits {
>
> struct mlx5_ifc_tisc_bits {
> u8 strict_lag_tx_port_affinity[0x1];
> - u8 reserved_at_1[0x3];
> + u8 tls_en[0x1];
> + u8 reserved_at_1[0x2];
It should be reserved_at_2.
Thanks
^ permalink raw reply
* Re: Shall we add some note info for tcp_min_snd_mss?
From: Eric Dumazet @ 2019-07-03 9:34 UTC (permalink / raw)
To: ZhangXiao, edumazet, davem; +Cc: netdev
In-Reply-To: <5D1C68C8.6020408@windriver.com>
On 7/3/19 1:35 AM, ZhangXiao wrote:
> Hi David & Eric,
>
> Commit 5f3e2bf0 (tcp: add tcp_min_snd_mss sysctl) add a new interface to
> adjust network. While if this variable been set too large, for example
> larger then (MTU - 40), the net link maybe damaged. So, how about adding
> some warning messages for the operator/administrator? In document, or in
> source code.
What kind of warning message do you
envision exactly ?
The sysctl is global, devices MTU can be quite arbitrary.
Like almost all sysctls, sysadmins are supposed to know what they are doing.
^ permalink raw reply
* Re: [PATCH v4 bpf-next 0/4] libbpf: add perf buffer abstraction and API
From: Daniel Borkmann @ 2019-07-03 9:36 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko, ast, kernel-team, bpf, netdev,
jakub.kicinski
In-Reply-To: <20190630065109.1794420-1-andriin@fb.com>
On 06/30/2019 08:51 AM, Andrii Nakryiko wrote:
> This patchset adds a high-level API for setting up and polling perf buffers
> associated with BPF_MAP_TYPE_PERF_EVENT_ARRAY map. Details of APIs are
> described in corresponding commit.
>
> Patch #1 adds a set of APIs to set up and work with perf buffer.
> Patch #2 enhances libbpf to support auto-setting PERF_EVENT_ARRAY map size.
> Patch #3 adds test.
> Patch #4 converts bpftool map event_pipe to new API.
>
> v3->v4:
> - fixed bpftool event_pipe cmd error handling (Jakub);
>
> v2->v3:
> - added perf_buffer__new_raw for more low-level control;
> - converted bpftool map event_pipe to new API (Daniel);
> - fixed bug with error handling in create_maps (Song);
>
> v1->v2:
> - add auto-sizing of PERF_EVENT_ARRAY maps;
>
> Andrii Nakryiko (4):
> libbpf: add perf buffer API
> libbpf: auto-set PERF_EVENT_ARRAY size to number of CPUs
> selftests/bpf: test perf buffer API
> tools/bpftool: switch map event_pipe to libbpf's perf_buffer
>
> tools/bpf/bpftool/map_perf_ring.c | 201 +++------
> tools/lib/bpf/libbpf.c | 397 +++++++++++++++++-
> tools/lib/bpf/libbpf.h | 49 +++
> tools/lib/bpf/libbpf.map | 4 +
> .../selftests/bpf/prog_tests/perf_buffer.c | 94 +++++
> .../selftests/bpf/progs/test_perf_buffer.c | 29 ++
> 6 files changed, 630 insertions(+), 144 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_buffer.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_perf_buffer.c
Hm, set looks good, but this does not apply cleanly. Please rebase against
bpf-next and resubmit. Please also update tools/lib/bpf/README.rst with regards
to the perf_buffer__ prefix. While at it, you could also address Jakub's comment.
Thanks,
Daniel
^ permalink raw reply
* [PATCH bpf-next v3] libbpf: add xsk_ring_prod__nb_free() function
From: Eelco Chaudron @ 2019-07-03 9:45 UTC (permalink / raw)
To: netdev
Cc: ast, daniel, kafai, songliubraving, yhs, andrii.nakryiko,
magnus.karlsson
When an AF_XDP application received X packets, it does not mean X
frames can be stuffed into the producer ring. To make it easier for
AF_XDP applications this API allows them to check how many frames can
be added into the ring.
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
v2 -> v3
- Removed cache by pass option
v1 -> v2
- Renamed xsk_ring_prod__free() to xsk_ring_prod__nb_free()
- Add caching so it will only touch global state when needed
tools/lib/bpf/xsk.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
index 82ea71a0f3ec..3411556e04d9 100644
--- a/tools/lib/bpf/xsk.h
+++ b/tools/lib/bpf/xsk.h
@@ -76,7 +76,7 @@ xsk_ring_cons__rx_desc(const struct xsk_ring_cons *rx, __u32 idx)
return &descs[idx & rx->mask];
}
-static inline __u32 xsk_prod_nb_free(struct xsk_ring_prod *r, __u32 nb)
+static inline __u32 xsk_prod__nb_free(struct xsk_ring_prod *r, __u32 nb)
{
__u32 free_entries = r->cached_cons - r->cached_prod;
@@ -110,7 +110,7 @@ static inline __u32 xsk_cons_nb_avail(struct xsk_ring_cons *r, __u32 nb)
static inline size_t xsk_ring_prod__reserve(struct xsk_ring_prod *prod,
size_t nb, __u32 *idx)
{
- if (xsk_prod_nb_free(prod, nb) < nb)
+ if (xsk_prod__nb_free(prod, nb) < nb)
return 0;
*idx = prod->cached_prod;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH bpf 1/3] bpf, x32: Fix bug with ALU64 {LSH,RSH,ARSH} BPF_X shift by 0
From: Daniel Borkmann @ 2019-07-03 9:49 UTC (permalink / raw)
To: Luke Nelson, linux-kernel
Cc: Luke Nelson, Xi Wang, Wang YanQing, David S. Miller,
Alexey Kuznetsov, Hideaki YOSHIFUJI, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H. Peter Anvin, x86, Alexei Starovoitov,
Martin KaFai Lau, Song Liu, Yonghong Song, Shuah Khan,
Jakub Kicinski, Jiong Wang, Björn Töpel, netdev, bpf,
linux-kselftest
In-Reply-To: <20190629055759.28365-1-luke.r.nels@gmail.com>
On 06/29/2019 07:57 AM, Luke Nelson wrote:
> The current x32 BPF JIT for shift operations is not correct when the
> shift amount in a register is 0. The expected behavior is a no-op, whereas
> the current implementation changes bits in the destination register.
>
> The following example demonstrates the bug. The expected result of this
> program is 1, but the current JITed code returns 2.
>
> r0 = 1
> r1 = 1
> r2 = 0
> r1 <<= r2
> if r1 == 1 goto end
> r0 = 2
> end:
> exit
>
> The bug is caused by an incorrect assumption by the JIT that a shift by
> 32 clear the register. On x32 however, shifts use the lower 5 bits of
> the source, making a shift by 32 equivalent to a shift by 0.
>
> This patch fixes the bug using double-precision shifts, which also
> simplifies the code.
>
> Fixes: 03f5781be2c7 ("bpf, x86_32: add eBPF JIT compiler for ia32")
> Co-developed-by: Xi Wang <xi.wang@gmail.com>
> Signed-off-by: Xi Wang <xi.wang@gmail.com>
> Signed-off-by: Luke Nelson <luke.r.nels@gmail.com>
Series applied, thanks!
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: fix precision tracking
From: Daniel Borkmann @ 2019-07-03 9:50 UTC (permalink / raw)
To: Alexei Starovoitov, davem; +Cc: netdev, bpf, kernel-team
In-Reply-To: <20190628162409.2513499-1-ast@kernel.org>
On 06/28/2019 06:24 PM, Alexei Starovoitov wrote:
> When equivalent state is found the current state needs to propagate precision marks.
> Otherwise the verifier will prune the search incorrectly.
>
> There is a price for correctness:
> before before broken fixed
> cnst spill precise precise
> bpf_lb-DLB_L3.o 1923 8128 1863 1898
> bpf_lb-DLB_L4.o 3077 6707 2468 2666
> bpf_lb-DUNKNOWN.o 1062 1062 544 544
> bpf_lxc-DDROP_ALL.o 166729 380712 22629 36823
> bpf_lxc-DUNKNOWN.o 174607 440652 28805 45325
> bpf_netdev.o 8407 31904 6801 7002
> bpf_overlay.o 5420 23569 4754 4858
> bpf_lxc_jit.o 39389 359445 50925 69631
> Overall precision tracking is still very effective.
>
> Fixes: b5dc0163d8fd ("bpf: precise scalar_value tracking")
> Reported-by: Lawrence Brakmo <brakmo@fb.com>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH v2 1/3] vsock/virtio: use RCU to avoid use-after-free on the_virtio_vsock
From: Jason Wang @ 2019-07-03 9:53 UTC (permalink / raw)
To: Stefano Garzarella, netdev
Cc: kvm, virtualization, Stefan Hajnoczi, Michael S. Tsirkin,
David S. Miller, linux-kernel
In-Reply-To: <20190628123659.139576-2-sgarzare@redhat.com>
On 2019/6/28 下午8:36, Stefano Garzarella wrote:
> Some callbacks used by the upper layers can run while we are in the
> .remove(). A potential use-after-free can happen, because we free
> the_virtio_vsock without knowing if the callbacks are over or not.
>
> To solve this issue we move the assignment of the_virtio_vsock at the
> end of .probe(), when we finished all the initialization, and at the
> beginning of .remove(), before to release resources.
> For the same reason, we do the same also for the vdev->priv.
>
> We use RCU to be sure that all callbacks that use the_virtio_vsock
> ended before freeing it. This is not required for callbacks that
> use vdev->priv, because after the vdev->config->del_vqs() we are sure
> that they are ended and will no longer be invoked.
>
> We also take the mutex during the .remove() to avoid that .probe() can
> run while we are resetting the device.
>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> net/vmw_vsock/virtio_transport.c | 67 +++++++++++++++++++++-----------
> 1 file changed, 44 insertions(+), 23 deletions(-)
>
> diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> index 9c287e3e393c..7ad510ec12e0 100644
> --- a/net/vmw_vsock/virtio_transport.c
> +++ b/net/vmw_vsock/virtio_transport.c
> @@ -65,19 +65,22 @@ struct virtio_vsock {
> u32 guest_cid;
> };
>
> -static struct virtio_vsock *virtio_vsock_get(void)
> -{
> - return the_virtio_vsock;
> -}
> -
> static u32 virtio_transport_get_local_cid(void)
> {
> - struct virtio_vsock *vsock = virtio_vsock_get();
> + struct virtio_vsock *vsock;
> + u32 ret;
>
> - if (!vsock)
> - return VMADDR_CID_ANY;
> + rcu_read_lock();
> + vsock = rcu_dereference(the_virtio_vsock);
> + if (!vsock) {
> + ret = VMADDR_CID_ANY;
> + goto out_rcu;
> + }
>
> - return vsock->guest_cid;
> + ret = vsock->guest_cid;
> +out_rcu:
> + rcu_read_unlock();
> + return ret;
> }
>
> static void virtio_transport_loopback_work(struct work_struct *work)
> @@ -197,14 +200,18 @@ virtio_transport_send_pkt(struct virtio_vsock_pkt *pkt)
> struct virtio_vsock *vsock;
> int len = pkt->len;
>
> - vsock = virtio_vsock_get();
> + rcu_read_lock();
> + vsock = rcu_dereference(the_virtio_vsock);
> if (!vsock) {
> virtio_transport_free_pkt(pkt);
> - return -ENODEV;
> + len = -ENODEV;
> + goto out_rcu;
> }
>
> - if (le64_to_cpu(pkt->hdr.dst_cid) == vsock->guest_cid)
> - return virtio_transport_send_pkt_loopback(vsock, pkt);
> + if (le64_to_cpu(pkt->hdr.dst_cid) == vsock->guest_cid) {
> + len = virtio_transport_send_pkt_loopback(vsock, pkt);
> + goto out_rcu;
> + }
>
> if (pkt->reply)
> atomic_inc(&vsock->queued_replies);
> @@ -214,6 +221,9 @@ virtio_transport_send_pkt(struct virtio_vsock_pkt *pkt)
> spin_unlock_bh(&vsock->send_pkt_list_lock);
>
> queue_work(virtio_vsock_workqueue, &vsock->send_pkt_work);
> +
> +out_rcu:
> + rcu_read_unlock();
> return len;
> }
>
> @@ -222,12 +232,14 @@ virtio_transport_cancel_pkt(struct vsock_sock *vsk)
> {
> struct virtio_vsock *vsock;
> struct virtio_vsock_pkt *pkt, *n;
> - int cnt = 0;
> + int cnt = 0, ret;
> LIST_HEAD(freeme);
>
> - vsock = virtio_vsock_get();
> + rcu_read_lock();
> + vsock = rcu_dereference(the_virtio_vsock);
> if (!vsock) {
> - return -ENODEV;
> + ret = -ENODEV;
> + goto out_rcu;
> }
>
> spin_lock_bh(&vsock->send_pkt_list_lock);
> @@ -255,7 +267,11 @@ virtio_transport_cancel_pkt(struct vsock_sock *vsk)
> queue_work(virtio_vsock_workqueue, &vsock->rx_work);
> }
>
> - return 0;
> + ret = 0;
> +
> +out_rcu:
> + rcu_read_unlock();
> + return ret;
> }
>
> static void virtio_vsock_rx_fill(struct virtio_vsock *vsock)
> @@ -590,8 +606,6 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
> vsock->rx_buf_max_nr = 0;
> atomic_set(&vsock->queued_replies, 0);
>
> - vdev->priv = vsock;
> - the_virtio_vsock = vsock;
> mutex_init(&vsock->tx_lock);
> mutex_init(&vsock->rx_lock);
> mutex_init(&vsock->event_lock);
> @@ -613,6 +627,9 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
> virtio_vsock_event_fill(vsock);
> mutex_unlock(&vsock->event_lock);
>
> + vdev->priv = vsock;
> + rcu_assign_pointer(the_virtio_vsock, vsock);
You probably need to use rcu_dereference_protected() to access
the_virtio_vsock in the function in order to survive from sparse.
> +
> mutex_unlock(&the_virtio_vsock_mutex);
> return 0;
>
> @@ -627,6 +644,12 @@ static void virtio_vsock_remove(struct virtio_device *vdev)
> struct virtio_vsock *vsock = vdev->priv;
> struct virtio_vsock_pkt *pkt;
>
> + mutex_lock(&the_virtio_vsock_mutex);
> +
> + vdev->priv = NULL;
> + rcu_assign_pointer(the_virtio_vsock, NULL);
This is still suspicious, can we access the_virtio_vsock through
vdev->priv? If yes, we may still get use-after-free since it was not
protected by RCU.
Another more interesting question, I believe we will do singleton for
virtio_vsock structure. Then what's the point of using vdev->priv to
access the_virtio_vsock? It looks to me we can it brings extra troubles
for doing synchronization.
Thanks
> + synchronize_rcu();
> +
> flush_work(&vsock->loopback_work);
> flush_work(&vsock->rx_work);
> flush_work(&vsock->tx_work);
> @@ -666,12 +689,10 @@ static void virtio_vsock_remove(struct virtio_device *vdev)
> }
> spin_unlock_bh(&vsock->loopback_list_lock);
>
> - mutex_lock(&the_virtio_vsock_mutex);
> - the_virtio_vsock = NULL;
> - mutex_unlock(&the_virtio_vsock_mutex);
> -
> vdev->config->del_vqs(vdev);
>
> + mutex_unlock(&the_virtio_vsock_mutex);
> +
> kfree(vsock);
> }
>
^ permalink raw reply
* Re: [PATCH net-next v6 05/15] ethtool: helper functions for netlink interface
From: Jiri Pirko @ 2019-07-03 10:04 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, David Miller, Jakub Kicinski, Andrew Lunn,
Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg,
linux-kernel
In-Reply-To: <20190702163437.GE20101@unicorn.suse.cz>
Tue, Jul 02, 2019 at 06:34:37PM CEST, mkubecek@suse.cz wrote:
>On Tue, Jul 02, 2019 at 03:05:15PM +0200, Jiri Pirko wrote:
>> Tue, Jul 02, 2019 at 01:50:04PM CEST, mkubecek@suse.cz wrote:
>> >
>> >+/* request header */
>> >+
>> >+/* use compact bitsets in reply */
>> >+#define ETHTOOL_RF_COMPACT (1 << 0)
>>
>> "COMPACT_BITSETS"?
>>
>> >+/* provide optional reply for SET or ACT requests */
>> >+#define ETHTOOL_RF_REPLY (1 << 1)
>>
>> "OPTIONAL_REPLY"?
>
>OK
>
>> >+ ret = nla_parse_nested(tb, ETHTOOL_A_HEADER_MAX, nest,
>> >+ policy ?: dflt_header_policy, extack);
>> >+ if (ret < 0)
>>
>> if (ret)
>>
>> Same remark goes to the rest of the code (also the rest of the patches),
>> in case called function cannot return positive values.
>
>The "if (ret < 0)" idiom for "on error do ..." is so ubiquitous through
>the whole kernel that I don't think it's worth it to carefully check
>which function can return a positive value and which cannot and risk
>that one day I overlook that some function. And yet another question is
>what exactly "cannot return" means: is it whenever the function does not
>return a positive value or only if it's explicitly documented not to?
>
>Looking at existing networking code, e.g. net/netfilter (except ipvs),
>net/sched or net/core/rtnetlink.c are using "if (ret < 0)" rather
>uniformly. And (as you objected to the check of genl_register_family()
>previous patch) even genetlink itself has
>
> err = genl_register_family(&genl_ctrl);
> if (err < 0)
> goto problem;
>
>in genl_init().
>
>>
>>
>> >+ return ret;
>> >+ devname_attr = tb[ETHTOOL_A_HEADER_DEV_NAME];
>> >+
>> >+ if (tb[ETHTOOL_A_HEADER_DEV_INDEX]) {
>> >+ u32 ifindex = nla_get_u32(tb[ETHTOOL_A_HEADER_DEV_INDEX]);
>> >+
>> >+ dev = dev_get_by_index(net, ifindex);
>> >+ if (!dev) {
>> >+ NL_SET_ERR_MSG_ATTR(extack,
>> >+ tb[ETHTOOL_A_HEADER_DEV_INDEX],
>> >+ "no device matches ifindex");
>> >+ return -ENODEV;
>> >+ }
>> >+ /* if both ifindex and ifname are passed, they must match */
>> >+ if (devname_attr &&
>> >+ strncmp(dev->name, nla_data(devname_attr), IFNAMSIZ)) {
>> >+ dev_put(dev);
>> >+ NL_SET_ERR_MSG_ATTR(extack, nest,
>> >+ "ifindex and name do not match");
>> >+ return -ENODEV;
>> >+ }
>> >+ } else if (devname_attr) {
>> >+ dev = dev_get_by_name(net, nla_data(devname_attr));
>> >+ if (!dev) {
>> >+ NL_SET_ERR_MSG_ATTR(extack, devname_attr,
>> >+ "no device matches name");
>> >+ return -ENODEV;
>> >+ }
>> >+ } else if (require_dev) {
>> >+ NL_SET_ERR_MSG_ATTR(extack, nest,
>> >+ "neither ifindex nor name specified");
>> >+ return -EINVAL;
>> >+ }
>> >+
>> >+ if (dev && !netif_device_present(dev)) {
>> >+ dev_put(dev);
>> >+ NL_SET_ERR_MSG(extack, "device not present");
>> >+ return -ENODEV;
>> >+ }
>> >+
>> >+ req_info->dev = dev;
>> >+ ethnl_update_u32(&req_info->req_mask, tb[ETHTOOL_A_HEADER_INFOMASK]);
>> >+ ethnl_update_u32(&req_info->global_flags, tb[ETHTOOL_A_HEADER_GFLAGS]);
>> >+ ethnl_update_u32(&req_info->req_flags, tb[ETHTOOL_A_HEADER_RFLAGS]);
>>
>> Just:
>> req_info->req_mask = nla_get_u32(tb[ETHTOOL_A_HEADER_INFOMASK];
>> ...
>>
>> Not sure what ethnl_update_u32() is good for, but it is not needed here.
>
>That would result in null pointer dereference if the attribute is
>missing. So you would need at least
>
> if (tb[ETHTOOL_A_HEADER_INFOMASK])
> req_info->req_mask = nla_get_u32(tb[ETHTOOL_A_HEADER_INFOMASK]);
> if (tb[ETHTOOL_A_HEADER_GFLAGS])
> req_info->global_flags =
> nla_get_u32(tb[ETHTOOL_A_HEADER_GFLAGS]);
> if (tb[ETHTOOL_A_HEADER_RFLAGS])
> req_info->req_flags = nla_get_u32(tb[ETHTOOL_A_HEADER_RFLAGS]);
Yeah, sure.
>
>I don't think it looks better.
Better than hiding something inside a helper in my opinion - helper that
is there for different reason moreover. Much easier to read the code
and follow.
>
>> >+
>> >+ return 0;
>> >+}
>> >+
>> >+/**
>> >+ * ethnl_fill_reply_header() - Put standard header into a reply message
>> >+ * @skb: skb with the message
>> >+ * @dev: network device to describe in header
>> >+ * @attrtype: attribute type to use for the nest
>> >+ *
>> >+ * Create a nested attribute with attributes describing given network device.
>> >+ * Clean up on error.
>>
>> Cleanup is obvious, no need to mention it in API docs.
>
>OK
>
>> >+ *
>> >+ * Return: 0 on success, error value (-EMSGSIZE only) on error
>> >+ */
>> >+int ethnl_fill_reply_header(struct sk_buff *skb, struct net_device *dev,
>> >+ u16 attrtype)
>> >+{
>> >+ struct nlattr *nest;
>> >+
>> >+ if (!dev)
>> >+ return 0;
>> >+ nest = nla_nest_start(skb, attrtype);
>> >+ if (!nest)
>> >+ return -EMSGSIZE;
>> >+
>> >+ if (nla_put_u32(skb, ETHTOOL_A_HEADER_DEV_INDEX, (u32)dev->ifindex) ||
>> >+ nla_put_string(skb, ETHTOOL_A_HEADER_DEV_NAME, dev->name))
>> >+ goto nla_put_failure;
>> >+ /* If more attributes are put into reply header, ethnl_header_size()
>> >+ * must be updated to account for them.
>> >+ */
>> >+
>> >+ nla_nest_end(skb, nest);
>> >+ return 0;
>> >+
>> >+nla_put_failure:
>> >+ nla_nest_cancel(skb, nest);
>> >+ return -EMSGSIZE;
>> >+}
>> >+
>> >+/**
>> >+ * ethnl_reply_init() - Create skb for a reply and fill device identification
>> >+ * @payload: payload length (without netlink and genetlink header)
>> >+ * @dev: device the reply is about (may be null)
>> >+ * @cmd: ETHTOOL_MSG_* message type for reply
>> >+ * @info: genetlink info of the received packet we respond to
>> >+ * @ehdrp: place to store payload pointer returned by genlmsg_new()
>> >+ *
>> >+ * Return: pointer to allocated skb on success, NULL on error
>> >+ */
>> >+struct sk_buff *ethnl_reply_init(size_t payload, struct net_device *dev, u8 cmd,
>> >+ u16 hdr_attrtype, struct genl_info *info,
>> >+ void **ehdrp)
>> >+{
>> >+ struct sk_buff *skb;
>> >+
>> >+ skb = genlmsg_new(payload, GFP_KERNEL);
>> >+ if (!skb)
>> >+ goto err;
>> >+ *ehdrp = genlmsg_put_reply(skb, info, ðtool_genl_family, 0, cmd);
>> >+ if (!*ehdrp)
>> >+ goto err_free;
>> >+
>> >+ if (dev) {
>> >+ int ret;
>> >+
>> >+ ret = ethnl_fill_reply_header(skb, dev, hdr_attrtype);
>> >+ if (ret < 0)
>> >+ goto err;
>> >+ }
>> >+ return skb;
>> >+
>> >+err_free:
>> >+ nlmsg_free(skb);
>> >+ if (info)
>> >+ GENL_SET_ERR_MSG(info, "failed to setup reply message");
>> >+err:
>>
>> Why also not fillup extack msg here?
>
>Right, err label should be right below the nlmsg_free(skb), thanks. And
>now I noticed another mistake: on ethnl_fill_reply_header() failure, we
>should go to err_free, not err.
>
>> >+static inline int ethnl_str_size(const char *s)
>>
>> If you really need this helper, put it into netlink code. There's nothing
>> ethtool-specific about this.
>
>OK, I'll look into it. I've been already thinking about some kind of
>NLA_SIZEOF() macro as about 1/3 of all uses of nla_total_size() follow
>the nla_total_size(sizeof(...)) pattern (and lot more should follow it
>but are written like e.g. nla_total_size(4) instead). This is another
>common pattern.
>
>> >+/* The ethnl_update_* helpers set value pointed to by @dst to the value of
>> >+ * netlink attribute @attr (if attr is not null). They return true if *dst
>> >+ * value was changed, false if not.
>> >+ */
>> >+static inline bool ethnl_update_u32(u32 *dst, struct nlattr *attr)
>>
>> I'm still not sure I'm convinced about these "update helpers" :)
>
>Just imagine what would e.g.
>
> if (ethnl_update_u32(&data.rx_pending, tb[ETHTOOL_A_RING_RX_PENDING]))
> mod = true;
> if (ethnl_update_u32(&data.rx_mini_pending,
> tb[ETHTOOL_A_RING_RX_MINI_PENDING]))
> mod = true;
> if (ethnl_update_u32(&data.rx_jumbo_pending,
> tb[ETHTOOL_A_RING_RX_JUMBO_PENDING]))
> mod = true;
> if (ethnl_update_u32(&data.tx_pending, tb[ETHTOOL_A_RING_TX_PENDING]))
> mod = true;
> if (!mod)
> return 0;
>
>look like without them. And coalescing parameters would be much worse
>(22 attributes / struct members).
No, I understand your motivation, don't get me wrong. I just wonder that
no other netlink implementation need such mechanism. Maybe I'm not
looking close enough. But if it does, should be rathe netlink helper.
Regarding the example code you have here. It is prefered to store
function result in a variable "if check" that variable. But in your,
code, couldn't this be done without ifs?
bool mod = false;
ethnl_update_u32(&mod, &data.rx_pending, tb[ETHTOOL_A_RING_RX_PENDING]))
ethnl_update_u32(&mod, &data.rx_mini_pending,
tb[ETHTOOL_A_RING_RX_MINI_PENDING]))
ethnl_update_u32(&mod, &data.rx_jumbo_pending,
tb[ETHTOOL_A_RING_RX_JUMBO_PENDING]))
ethnl_update_u32(&mod, &data.tx_pending, tb[ETHTOOL_A_RING_TX_PENDING]))
if (!mod)
return 0;
>
>> >+{
>> >+ u32 val;
>> >+
>> >+ if (!attr)
>> >+ return false;
>> >+ val = nla_get_u32(attr);
>> >+ if (*dst == val)
>> >+ return false;
>> >+
>> >+ *dst = val;
>> >+ return true;
>> >+}
>...
>> >+static inline bool ethnl_update_binary(u8 *dst, unsigned int len,
>>
>> void *dst
>
>OK.
>
>> >+/**
>> >+ * ethnl_is_privileged() - check if request has sufficient privileges
>> >+ * @skb: skb with client request
>> >+ *
>> >+ * Checks if client request has CAP_NET_ADMIN in its netns. Unlike the flags
>> >+ * in genl_ops, this allows finer access control, e.g. allowing or denying
>> >+ * the request based on its contents or witholding only part of the data
>> >+ * from unprivileged users.
>> >+ *
>> >+ * Return: true if request is privileged, false if not
>> >+ */
>> >+static inline bool ethnl_is_privileged(struct sk_buff *skb)
>>
>> I wonder why you need this helper. Genetlink uses
>> ops->flags & GENL_ADMIN_PERM for this.
>
>It's explained in the function description. Sometimes we need finer
>control than by request message type. An example is the WoL password:
>ETHTOOL_GWOL is privileged because of it but I believe there si no
>reason why unprivileged user couldn't see enabled WoL modes, we can
>simply omit the password for him. (Also, it allows to combine query for
>WoL settings with other unprivileged settings.)
Why can't we have rather:
ETHTOOL_WOL_GET for all
ETHTOOL_WOL_PASSWORD_GET with GENL_ADMIN_PERM
?
Better to stick with what we have in gennetlink rather then to bend the
implementation from the very beginning I think.
>
>> >+/**
>> >+ * ethnl_reply_header_size() - total size of reply header
>> >+ *
>> >+ * This is an upper estimate so that we do not need to hold RTNL lock longer
>> >+ * than necessary (to prevent rename between size estimate and composing the
>>
>> I guess this description is not relevant anymore. I don't see why to
>> hold rtnl mutex for this function...
>
>You don't need it for this function, it's the other way around: unless
>you hold RTNL lock for the whole time covering both checking needed
>message size and filling the message - and we don't - the device could
>be renamed in between. Thus if we returned size based on current device
>name, it might not be sufficient at the time the header is filled.
>That's why this function returns maximum possible size (which is
>actually a constant).
I suggest to avoid the description. It is misleading. Perhaps something
to have in a patch description but not here in code.
>
>Michal
>
>> >+ * message). Accounts only for device ifindex and name as those are the only
>> >+ * attributes ethnl_fill_reply_header() puts into the reply header.
>> >+ */
>> >+static inline unsigned int ethnl_reply_header_size(void)
>> >+{
>> >+ return nla_total_size(nla_total_size(sizeof(u32)) +
>> >+ nla_total_size(IFNAMSIZ));
>> >+}
^ permalink raw reply
* Re: [PATCH v2 0/3] vsock/virtio: several fixes in the .probe() and .remove()
From: Stefano Garzarella @ 2019-07-03 10:07 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: netdev, kvm, Michael S. Tsirkin, linux-kernel, virtualization,
Stefan Hajnoczi, David S. Miller
In-Reply-To: <20190703091453.GA11844@stefanha-x1.localdomain>
On Wed, Jul 03, 2019 at 10:14:53AM +0100, Stefan Hajnoczi wrote:
> On Mon, Jul 01, 2019 at 07:03:57PM +0200, Stefano Garzarella wrote:
> > On Mon, Jul 01, 2019 at 04:11:13PM +0100, Stefan Hajnoczi wrote:
> > > On Fri, Jun 28, 2019 at 02:36:56PM +0200, Stefano Garzarella wrote:
> > > > During the review of "[PATCH] vsock/virtio: Initialize core virtio vsock
> > > > before registering the driver", Stefan pointed out some possible issues
> > > > in the .probe() and .remove() callbacks of the virtio-vsock driver.
> > > >
> > > > This series tries to solve these issues:
> > > > - Patch 1 adds RCU critical sections to avoid use-after-free of
> > > > 'the_virtio_vsock' pointer.
> > > > - Patch 2 stops workers before to call vdev->config->reset(vdev) to
> > > > be sure that no one is accessing the device.
> > > > - Patch 3 moves the works flush at the end of the .remove() to avoid
> > > > use-after-free of 'vsock' object.
> > > >
> > > > v2:
> > > > - Patch 1: use RCU to protect 'the_virtio_vsock' pointer
> > > > - Patch 2: no changes
> > > > - Patch 3: flush works only at the end of .remove()
> > > > - Removed patch 4 because virtqueue_detach_unused_buf() returns all the buffers
> > > > allocated.
> > > >
> > > > v1: https://patchwork.kernel.org/cover/10964733/
> > >
> > > This looks good to me.
> >
> > Thanks for the review!
> >
> > >
> > > Did you run any stress tests? For example an SMP guest constantly
> > > connecting and sending packets together with a script that
> > > hotplug/unplugs vhost-vsock-pci from the host side.
> >
> > Yes, I started an SMP guest (-smp 4 -monitor tcp:127.0.0.1:1234,server,nowait)
> > and I run these scripts to stress the .probe()/.remove() path:
> >
> > - guest
> > while true; do
> > cat /dev/urandom | nc-vsock -l 4321 > /dev/null &
> > cat /dev/urandom | nc-vsock -l 5321 > /dev/null &
> > cat /dev/urandom | nc-vsock -l 6321 > /dev/null &
> > cat /dev/urandom | nc-vsock -l 7321 > /dev/null &
> > wait
> > done
> >
> > - host
> > while true; do
> > cat /dev/urandom | nc-vsock 3 4321 > /dev/null &
> > cat /dev/urandom | nc-vsock 3 5321 > /dev/null &
> > cat /dev/urandom | nc-vsock 3 6321 > /dev/null &
> > cat /dev/urandom | nc-vsock 3 7321 > /dev/null &
> > sleep 2
> > echo "device_del v1" | nc 127.0.0.1 1234
> > sleep 1
> > echo "device_add vhost-vsock-pci,id=v1,guest-cid=3" | nc 127.0.0.1 1234
> > sleep 1
> > done
> >
> > Do you think is enough or is better to have a test more accurate?
>
> That's good when left running overnight so that thousands of hotplug
> events are tested.
Honestly I run the test for ~30 mins (because without the patch the
crash happens in a few seconds), but of course, I'll run it this night :)
Thanks,
Stefano
^ 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