* Re: [PATCH net,stable] cdc_mbim: apply "NDP to end" quirk to all Huawei devices
From: David Miller @ 2016-04-15 1:04 UTC (permalink / raw)
To: bjorn-yOkvZcmFvRU
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
andreas.fett-opNxpl+3fjRBDgjK7y7TUQ,
burdandrei-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1460470272-9289-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
From: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
Date: Tue, 12 Apr 2016 16:11:12 +0200
> We now have a positive report of another Huawei device needing
> this quirk: The ME906s-158 (12d1:15c1). This is an m.2 form
> factor modem with no obvious relationship to the E3372 (12d1:157d)
> we already have a quirk entry for. This is reason enough to
> believe the quirk might be necessary for any number of current
> and future Huawei devices.
>
> Applying the quirk to all Huawei devices, since it is crucial
> to any device affected by the firmware bug, while the impact
> on non-affected devices is negligible.
>
> The quirk can if necessary be disabled per-device by writing
> N to /sys/class/net/<iface>/cdc_ncm/ndp_to_end
>
> Reported-by: Andreas Fett <andreas.fett-opNxpl+3fjRBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
Applied and queued up for -stable, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH nf] netfilter: ipv6: Orphan skbs in nf_ct_frag6_gather()
From: Joe Stringer @ 2016-04-15 0:35 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Florian Westphal, David Laight, netfilter-devel@vger.kernel.org,
diproiettod@vmware.com, netdev@vger.kernel.org
In-Reply-To: <20160414103533.GA3211@salvia>
On 14 April 2016 at 03:35, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Thu, Apr 14, 2016 at 10:40:15AM +0200, Florian Westphal wrote:
>> David Laight <David.Laight@ACULAB.COM> wrote:
>> > From: Joe Stringer
>> > > Sent: 13 April 2016 19:10
>> > > This is the IPv6 equivalent of commit 8282f27449bf ("inet: frag: Always
>> > > orphan skbs inside ip_defrag()").
>> > >
>> > > Prior to commit 029f7f3b8701 ("netfilter: ipv6: nf_defrag: avoid/free
>> > > clone operations"), ipv6 fragments sent to nf_ct_frag6_gather() would be
>> > > cloned (implicitly orphaning) prior to queueing for reassembly. As such,
>> > > when the IPv6 message is eventually reassembled, the skb->sk for all
>> > > fragments would be NULL. After that commit was introduced, rather than
>> > > cloning, the original skbs were queued directly without orphaning. The
>> > > end result is that all frags except for the first and last may have a
>> > > socket attached.
>> >
>> > I'd have thought that the queued fragments would still want to be
>> > resource-counted against the socket (I think that is what skb->sk is for).
>>
>> No, ipv4/ipv6 reasm has its own accouting.
>>
>> > Although I can't imagine why IPv6 reassembly is happening on skb
>> > associated with a socket.
>>
>> Right, thats a much more interesting question -- both ipv4 and
>> ipv6 orphan skbs before NF_HOOK prerouting trip.
>>
>> (That being said, I don't mind the patch, I'm just be curious how this
>> can happen).
>
> If this change is specific to get this working in ovs and its
> conntrack support, then I don't think this belong to core
> infrastructure. This should be fixed in ovs instead.
I admit I've only been able to reproduce it with OVS. My main reason
for proposing the fix this way was just because this is what the IPv4
code does, so I figured IPv6 should be consistent with that.
^ permalink raw reply
* RE: [PATCH net-next 2/2] intel: ixgbevf: Support Windows hosts (Hyper-V)
From: KY Srinivasan @ 2016-04-15 0:26 UTC (permalink / raw)
To: Alexander Duyck
Cc: olaf@aepfle.de, netdev, jasowang@redhat.com,
linux-kernel@vger.kernel.org, jackm@mellanox.com,
yevgenyp@mellanox.com, Ronciak, John,
intel-wired-lan@lists.osuosl.org, eli@mellanox.com,
apw@canonical.com, devel@linuxdriverproject.org, Rustad, Mark D,
David Miller
In-Reply-To: <CAKgT0UckBes1tNXrNjA0R4uCdXouG9qQrf+VaLJMd8AhgYQieQ@mail.gmail.com>
> -----Original Message-----
> From: Alexander Duyck [mailto:alexander.duyck@gmail.com]
> Sent: Thursday, April 14, 2016 5:19 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: Rustad, Mark D <mark.d.rustad@intel.com>; David Miller
> <davem@davemloft.net>; netdev <netdev@vger.kernel.org>; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; jasowang@redhat.com; eli@mellanox.com;
> jackm@mellanox.com; yevgenyp@mellanox.com; Ronciak, John
> <john.ronciak@intel.com>; intel-wired-lan@linuxonhyperv.com
> Subject: Re: [PATCH net-next 2/2] intel: ixgbevf: Support Windows hosts
> (Hyper-V)
>
> On Thu, Apr 14, 2016 at 5:11 PM, KY Srinivasan <kys@microsoft.com> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Rustad, Mark D [mailto:mark.d.rustad@intel.com]
> >> Sent: Thursday, April 14, 2016 5:07 PM
> >> To: KY Srinivasan <kys@microsoft.com>
> >> Cc: David Miller <davem@davemloft.net>; netdev
> >> <netdev@vger.kernel.org>; linux-kernel@vger.kernel.org;
> >> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> >> jasowang@redhat.com; eli@mellanox.com; jackm@mellanox.com;
> >> yevgenyp@mellanox.com; Ronciak, John <john.ronciak@intel.com>;
> intel-
> >> wired-lan@linuxonhyperv.com
> >> Subject: Re: [PATCH net-next 2/2] intel: ixgbevf: Support Windows hosts
> >> (Hyper-V)
> >>
> >> KY Srinivasan <kys@microsoft.com> wrote:
> >>
> >> >
> >> >
> >> >> -----Original Message-----
> >> >> From: Rustad, Mark D [mailto:mark.d.rustad@intel.com]
> >> >> Sent: Thursday, April 14, 2016 4:01 PM
> >> >> To: KY Srinivasan <kys@microsoft.com>
> >> >> Cc: David Miller <davem@davemloft.net>; netdev
> >> >> <netdev@vger.kernel.org>; linux-kernel@vger.kernel.org;
> >> >> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> >> >> jasowang@redhat.com; eli@mellanox.com; jackm@mellanox.com;
> >> >> yevgenyp@mellanox.com; Ronciak, John <john.ronciak@intel.com>;
> >> intel-
> >> >> wired-lan@linuxonhyperv.com
> >> >> Subject: Re: [PATCH net-next 2/2] intel: ixgbevf: Support Windows
> hosts
> >> >> (Hyper-V)
> >> >>
> >> >> Some comments below:
> >> >
> >> > Mark,
> >> >
> >> > Thank you for the comments. I will address them and repost the
> patches.
> >> >
> >> > Regards,
> >> >
> >> > K. Y
> >>
> >> Please look closely at Alex's comments. I think they are much more
> >> important.
> >
> > I am looking at Alex's comments as I am writing this.
> >
>
> On additional thought that just occurred to me after looking over the
> other patches you submitted for the hv_netvsc is that you might just
> stub out the multicast, unicast, and vfta configuration calls for the
> hyperV interface since all that stuff should be handled by the other
> link in the bond anyway. Then you should be able to mostly contain
> all the changes other than the device IDs to the vf.c file which is
> really how this kind of change should work anyway.
I will do that.
>
> Also I was wondering. Since HyperV is using a proprietary device ID
> anyway do you really need the calls like the one below?:
> + if (x86_hyper == &x86_hyper_ms_hyperv) {
>
> If we can just identify HyperV via the device Id then we could drop
> the x86 arch specific bits and instead just build for all cases.
Yes; I was planning to get rid of the x86 dependency. I will fix this.
K. Y
>
> - Alex
^ permalink raw reply
* Re: [PATCH net-next 2/2] intel: ixgbevf: Support Windows hosts (Hyper-V)
From: Alexander Duyck @ 2016-04-15 0:19 UTC (permalink / raw)
To: KY Srinivasan
Cc: olaf@aepfle.de, intel-wired-lan@linuxonhyperv.com, netdev,
jasowang@redhat.com, linux-kernel@vger.kernel.org,
jackm@mellanox.com, yevgenyp@mellanox.com, Ronciak, John,
eli@mellanox.com, apw@canonical.com, devel@linuxdriverproject.org,
Rustad, Mark D, David Miller
In-Reply-To: <SN2PR03MB19196E84DF934294DE7BB739A0680@SN2PR03MB1919.namprd03.prod.outlook.com>
On Thu, Apr 14, 2016 at 5:11 PM, KY Srinivasan <kys@microsoft.com> wrote:
>
>
>> -----Original Message-----
>> From: Rustad, Mark D [mailto:mark.d.rustad@intel.com]
>> Sent: Thursday, April 14, 2016 5:07 PM
>> To: KY Srinivasan <kys@microsoft.com>
>> Cc: David Miller <davem@davemloft.net>; netdev
>> <netdev@vger.kernel.org>; linux-kernel@vger.kernel.org;
>> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
>> jasowang@redhat.com; eli@mellanox.com; jackm@mellanox.com;
>> yevgenyp@mellanox.com; Ronciak, John <john.ronciak@intel.com>; intel-
>> wired-lan@linuxonhyperv.com
>> Subject: Re: [PATCH net-next 2/2] intel: ixgbevf: Support Windows hosts
>> (Hyper-V)
>>
>> KY Srinivasan <kys@microsoft.com> wrote:
>>
>> >
>> >
>> >> -----Original Message-----
>> >> From: Rustad, Mark D [mailto:mark.d.rustad@intel.com]
>> >> Sent: Thursday, April 14, 2016 4:01 PM
>> >> To: KY Srinivasan <kys@microsoft.com>
>> >> Cc: David Miller <davem@davemloft.net>; netdev
>> >> <netdev@vger.kernel.org>; linux-kernel@vger.kernel.org;
>> >> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
>> >> jasowang@redhat.com; eli@mellanox.com; jackm@mellanox.com;
>> >> yevgenyp@mellanox.com; Ronciak, John <john.ronciak@intel.com>;
>> intel-
>> >> wired-lan@linuxonhyperv.com
>> >> Subject: Re: [PATCH net-next 2/2] intel: ixgbevf: Support Windows hosts
>> >> (Hyper-V)
>> >>
>> >> Some comments below:
>> >
>> > Mark,
>> >
>> > Thank you for the comments. I will address them and repost the patches.
>> >
>> > Regards,
>> >
>> > K. Y
>>
>> Please look closely at Alex's comments. I think they are much more
>> important.
>
> I am looking at Alex's comments as I am writing this.
>
On additional thought that just occurred to me after looking over the
other patches you submitted for the hv_netvsc is that you might just
stub out the multicast, unicast, and vfta configuration calls for the
hyperV interface since all that stuff should be handled by the other
link in the bond anyway. Then you should be able to mostly contain
all the changes other than the device IDs to the vf.c file which is
really how this kind of change should work anyway.
Also I was wondering. Since HyperV is using a proprietary device ID
anyway do you really need the calls like the one below?:
+ if (x86_hyper == &x86_hyper_ms_hyperv) {
If we can just identify HyperV via the device Id then we could drop
the x86 arch specific bits and instead just build for all cases.
- Alex
^ permalink raw reply
* Re: [PATCH net-next 2/2] intel: ixgbevf: Support Windows hosts (Hyper-V)
From: Rustad, Mark D @ 2016-04-15 0:19 UTC (permalink / raw)
To: KY Srinivasan
Cc: olaf@aepfle.de, netdev, jasowang@redhat.com,
linux-kernel@vger.kernel.org, jackm@mellanox.com,
yevgenyp@mellanox.com, Ronciak, John, intel-wired-lan,
eli@mellanox.com, apw@canonical.com, devel@linuxdriverproject.org,
David Miller
In-Reply-To: <SN2PR03MB19196E84DF934294DE7BB739A0680@SN2PR03MB1919.namprd03.prod.outlook.com>
[-- Attachment #1.1: Type: text/plain, Size: 1863 bytes --]
KY Srinivasan <kys@microsoft.com> wrote:
>
>
>> -----Original Message-----
>> From: Rustad, Mark D [mailto:mark.d.rustad@intel.com]
>> Sent: Thursday, April 14, 2016 5:07 PM
>> To: KY Srinivasan <kys@microsoft.com>
>> Cc: David Miller <davem@davemloft.net>; netdev
>> <netdev@vger.kernel.org>; linux-kernel@vger.kernel.org;
>> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
>> jasowang@redhat.com; eli@mellanox.com; jackm@mellanox.com;
>> yevgenyp@mellanox.com; Ronciak, John <john.ronciak@intel.com>; intel-
>> wired-lan@linuxonhyperv.com
>> Subject: Re: [PATCH net-next 2/2] intel: ixgbevf: Support Windows hosts
>> (Hyper-V)
>>
>> KY Srinivasan <kys@microsoft.com> wrote:
>>
>>>> -----Original Message-----
>>>> From: Rustad, Mark D [mailto:mark.d.rustad@intel.com]
>>>> Sent: Thursday, April 14, 2016 4:01 PM
>>>> To: KY Srinivasan <kys@microsoft.com>
>>>> Cc: David Miller <davem@davemloft.net>; netdev
>>>> <netdev@vger.kernel.org>; linux-kernel@vger.kernel.org;
>>>> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
>>>> jasowang@redhat.com; eli@mellanox.com; jackm@mellanox.com;
>>>> yevgenyp@mellanox.com; Ronciak, John <john.ronciak@intel.com>;
>> intel-
>>>> wired-lan@linuxonhyperv.com
>>>> Subject: Re: [PATCH net-next 2/2] intel: ixgbevf: Support Windows hosts
>>>> (Hyper-V)
>>>>
>>>> Some comments below:
>>>
>>> Mark,
>>>
>>> Thank you for the comments. I will address them and repost the patches.
>>>
>>> Regards,
>>>
>>> K. Y
>>
>> Please look closely at Alex's comments. I think they are much more
>> important.
>
> I am looking at Alex's comments as I am writing this.
>
> K. Y
>> --
>> Mark Rustad, Networking Division, Intel Corporation
Can you please stop putting that crazy intel-wired-lan@linuxonhyperv.com in
your messages? It doesn't exist.
--
Mark Rustad, Networking Division, Intel Corporation
[-- Attachment #1.2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 841 bytes --]
[-- Attachment #2: Type: text/plain, Size: 169 bytes --]
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply
* Re: [PATCH nf] netfilter: ipv6: Orphan skbs in nf_ct_frag6_gather()
From: Joe Stringer @ 2016-04-15 0:14 UTC (permalink / raw)
To: Florian Westphal
Cc: David Laight, netfilter-devel@vger.kernel.org,
diproiettod@vmware.com, netdev@vger.kernel.org
In-Reply-To: <20160414084015.GA3192@breakpoint.cc>
On 14 April 2016 at 01:40, Florian Westphal <fw@strlen.de> wrote:
> David Laight <David.Laight@ACULAB.COM> wrote:
>> From: Joe Stringer
>> > Sent: 13 April 2016 19:10
>> > This is the IPv6 equivalent of commit 8282f27449bf ("inet: frag: Always
>> > orphan skbs inside ip_defrag()").
>> >
>> > Prior to commit 029f7f3b8701 ("netfilter: ipv6: nf_defrag: avoid/free
>> > clone operations"), ipv6 fragments sent to nf_ct_frag6_gather() would be
>> > cloned (implicitly orphaning) prior to queueing for reassembly. As such,
>> > when the IPv6 message is eventually reassembled, the skb->sk for all
>> > fragments would be NULL. After that commit was introduced, rather than
>> > cloning, the original skbs were queued directly without orphaning. The
>> > end result is that all frags except for the first and last may have a
>> > socket attached.
>>
>> I'd have thought that the queued fragments would still want to be
>> resource-counted against the socket (I think that is what skb->sk is for).
>
> No, ipv4/ipv6 reasm has its own accouting.
>
>> Although I can't imagine why IPv6 reassembly is happening on skb
>> associated with a socket.
>
> Right, thats a much more interesting question -- both ipv4 and
> ipv6 orphan skbs before NF_HOOK prerouting trip.
>
> (That being said, I don't mind the patch, I'm just be curious how this
> can happen).
The topology is quite simple, there is a veth pair connected between a
namespace and OVS. The OVS bridge has an internal port. The
bridge is configured with flows to send packets through conntrack
(causing packet reassembly + refragmentation on output), and then
forward packets between the host and the veth. The internal port and
the veth inside the netns have IP addresses configured in the same
subnet.
In the test case, we send a large ICMPv6 ping request from the
namespace to the internal port. The namespace will fragment the IP
message into fragments and send through the veth. OVS processes these,
sends to conntrack (reassembly), then decides to output to the
internal port (refragmenting). The host stack finally receives the
fragments and processes the ICMP request. On response, the host sends
several fragments to OVS. OVS reassembles these and sends them to
conntrack, then decides to forward to the veth. When forwarding to the
veth, the skb frag queue is in this state with many skbs (all except
first and last) having skb->sk populated, and we hit the
BUG_ON(skb->sk) in ip6_fragment() just prior to transmitting to the veth.
In regards to your question about prerouting, does the response even
hit the input path on the host? An ICMP response is generated, and it
needs to be directed out to the device (output path), then when the
internal device receives it, it starts OVS processing.
^ permalink raw reply
* RE: [PATCH net-next 2/2] intel: ixgbevf: Support Windows hosts (Hyper-V)
From: KY Srinivasan @ 2016-04-15 0:11 UTC (permalink / raw)
To: Rustad, Mark D
Cc: olaf@aepfle.de, intel-wired-lan@linuxonhyperv.com, netdev,
jasowang@redhat.com, linux-kernel@vger.kernel.org,
jackm@mellanox.com, yevgenyp@mellanox.com, Ronciak, John,
eli@mellanox.com, apw@canonical.com, devel@linuxdriverproject.org,
David Miller
In-Reply-To: <DA0F5562-FC38-4037-B018-42116DD142E8@intel.com>
> -----Original Message-----
> From: Rustad, Mark D [mailto:mark.d.rustad@intel.com]
> Sent: Thursday, April 14, 2016 5:07 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: David Miller <davem@davemloft.net>; netdev
> <netdev@vger.kernel.org>; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com; eli@mellanox.com; jackm@mellanox.com;
> yevgenyp@mellanox.com; Ronciak, John <john.ronciak@intel.com>; intel-
> wired-lan@linuxonhyperv.com
> Subject: Re: [PATCH net-next 2/2] intel: ixgbevf: Support Windows hosts
> (Hyper-V)
>
> KY Srinivasan <kys@microsoft.com> wrote:
>
> >
> >
> >> -----Original Message-----
> >> From: Rustad, Mark D [mailto:mark.d.rustad@intel.com]
> >> Sent: Thursday, April 14, 2016 4:01 PM
> >> To: KY Srinivasan <kys@microsoft.com>
> >> Cc: David Miller <davem@davemloft.net>; netdev
> >> <netdev@vger.kernel.org>; linux-kernel@vger.kernel.org;
> >> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> >> jasowang@redhat.com; eli@mellanox.com; jackm@mellanox.com;
> >> yevgenyp@mellanox.com; Ronciak, John <john.ronciak@intel.com>;
> intel-
> >> wired-lan@linuxonhyperv.com
> >> Subject: Re: [PATCH net-next 2/2] intel: ixgbevf: Support Windows hosts
> >> (Hyper-V)
> >>
> >> Some comments below:
> >
> > Mark,
> >
> > Thank you for the comments. I will address them and repost the patches.
> >
> > Regards,
> >
> > K. Y
>
> Please look closely at Alex's comments. I think they are much more
> important.
I am looking at Alex's comments as I am writing this.
K. Y
>
> --
> Mark Rustad, Networking Division, Intel Corporation
^ permalink raw reply
* Re: [PATCH net-next 2/2] intel: ixgbevf: Support Windows hosts (Hyper-V)
From: Rustad, Mark D @ 2016-04-15 0:06 UTC (permalink / raw)
To: KY Srinivasan
Cc: olaf@aepfle.de, intel-wired-lan@linuxonhyperv.com, netdev,
jasowang@redhat.com, linux-kernel@vger.kernel.org,
jackm@mellanox.com, yevgenyp@mellanox.com, Ronciak, John,
eli@mellanox.com, apw@canonical.com, devel@linuxdriverproject.org,
David Miller
In-Reply-To: <SN2PR03MB1919133D2E9B5DDBC2AED643A0680@SN2PR03MB1919.namprd03.prod.outlook.com>
[-- Attachment #1.1: Type: text/plain, Size: 917 bytes --]
KY Srinivasan <kys@microsoft.com> wrote:
>
>
>> -----Original Message-----
>> From: Rustad, Mark D [mailto:mark.d.rustad@intel.com]
>> Sent: Thursday, April 14, 2016 4:01 PM
>> To: KY Srinivasan <kys@microsoft.com>
>> Cc: David Miller <davem@davemloft.net>; netdev
>> <netdev@vger.kernel.org>; linux-kernel@vger.kernel.org;
>> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
>> jasowang@redhat.com; eli@mellanox.com; jackm@mellanox.com;
>> yevgenyp@mellanox.com; Ronciak, John <john.ronciak@intel.com>; intel-
>> wired-lan@linuxonhyperv.com
>> Subject: Re: [PATCH net-next 2/2] intel: ixgbevf: Support Windows hosts
>> (Hyper-V)
>>
>> Some comments below:
>
> Mark,
>
> Thank you for the comments. I will address them and repost the patches.
>
> Regards,
>
> K. Y
Please look closely at Alex's comments. I think they are much more important.
--
Mark Rustad, Networking Division, Intel Corporation
[-- Attachment #1.2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 841 bytes --]
[-- Attachment #2: Type: text/plain, Size: 169 bytes --]
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply
* RE: [PATCH net-next 2/2] intel: ixgbevf: Support Windows hosts (Hyper-V)
From: KY Srinivasan @ 2016-04-15 0:00 UTC (permalink / raw)
To: Rustad, Mark D
Cc: olaf@aepfle.de, intel-wired-lan@linuxonhyperv.com, netdev,
jasowang@redhat.com, linux-kernel@vger.kernel.org,
jackm@mellanox.com, yevgenyp@mellanox.com, Ronciak, John,
eli@mellanox.com, apw@canonical.com, devel@linuxdriverproject.org,
David Miller
In-Reply-To: <5F69AC74-16B9-4EAD-8B4B-21D2EB6B3653@intel.com>
> -----Original Message-----
> From: Rustad, Mark D [mailto:mark.d.rustad@intel.com]
> Sent: Thursday, April 14, 2016 4:01 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: David Miller <davem@davemloft.net>; netdev
> <netdev@vger.kernel.org>; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com; eli@mellanox.com; jackm@mellanox.com;
> yevgenyp@mellanox.com; Ronciak, John <john.ronciak@intel.com>; intel-
> wired-lan@linuxonhyperv.com
> Subject: Re: [PATCH net-next 2/2] intel: ixgbevf: Support Windows hosts
> (Hyper-V)
>
> Some comments below:
Mark,
Thank you for the comments. I will address them and repost the patches.
Regards,
K. Y
^ permalink raw reply
* [PATCH net-next 2/2] intel: ixgbevf: Support Windows hosts (Hyper-V)
From: K. Y. Srinivasan @ 2016-04-14 23:55 UTC (permalink / raw)
To: davem, netdev, linux-kernel, devel, olaf, apw, jasowang, eli,
jackm, yevgenyp, john.ronciak, intel-wired-lan
In-Reply-To: <1460678142-30353-1-git-send-email-kys@microsoft.com>
On Hyper-V, the VF/PF communication is a via software mediated path
as opposed to the hardware mailbox. Make the necessary
adjustments to support Hyper-V.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 11 ++
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 56 ++++++---
drivers/net/ethernet/intel/ixgbevf/mbx.c | 12 ++
drivers/net/ethernet/intel/ixgbevf/vf.c | 138 +++++++++++++++++++++
drivers/net/ethernet/intel/ixgbevf/vf.h | 2 +
5 files changed, 201 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index 5ac60ee..f8d2a0b 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -460,9 +460,13 @@ enum ixbgevf_state_t {
enum ixgbevf_boards {
board_82599_vf,
+ board_82599_vf_hv,
board_X540_vf,
+ board_X540_vf_hv,
board_X550_vf,
+ board_X550_vf_hv,
board_X550EM_x_vf,
+ board_X550EM_x_vf_hv,
};
enum ixgbevf_xcast_modes {
@@ -477,6 +481,13 @@ extern const struct ixgbevf_info ixgbevf_X550_vf_info;
extern const struct ixgbevf_info ixgbevf_X550EM_x_vf_info;
extern const struct ixgbe_mbx_operations ixgbevf_mbx_ops;
+
+extern const struct ixgbevf_info ixgbevf_82599_vf_hv_info;
+extern const struct ixgbevf_info ixgbevf_X540_vf_hv_info;
+extern const struct ixgbevf_info ixgbevf_X550_vf_hv_info;
+extern const struct ixgbevf_info ixgbevf_X550EM_x_vf_hv_info;
+extern const struct ixgbe_mbx_operations ixgbevf_hv_mbx_ops;
+
/* needed by ethtool.c */
extern const char ixgbevf_driver_name[];
extern const char ixgbevf_driver_version[];
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 007cbe0..4a0ffac 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -49,6 +49,7 @@
#include <linux/if.h>
#include <linux/if_vlan.h>
#include <linux/prefetch.h>
+#include <asm/mshyperv.h>
#include "ixgbevf.h"
@@ -62,10 +63,14 @@ static char ixgbevf_copyright[] =
"Copyright (c) 2009 - 2015 Intel Corporation.";
static const struct ixgbevf_info *ixgbevf_info_tbl[] = {
- [board_82599_vf] = &ixgbevf_82599_vf_info,
- [board_X540_vf] = &ixgbevf_X540_vf_info,
- [board_X550_vf] = &ixgbevf_X550_vf_info,
- [board_X550EM_x_vf] = &ixgbevf_X550EM_x_vf_info,
+ [board_82599_vf] = &ixgbevf_82599_vf_info,
+ [board_82599_vf_hv] = &ixgbevf_82599_vf_hv_info,
+ [board_X540_vf] = &ixgbevf_X540_vf_info,
+ [board_X540_vf_hv] = &ixgbevf_X540_vf_hv_info,
+ [board_X550_vf] = &ixgbevf_X550_vf_info,
+ [board_X550_vf_hv] = &ixgbevf_X550_vf_hv_info,
+ [board_X550EM_x_vf] = &ixgbevf_X550EM_x_vf_info,
+ [board_X550EM_x_vf_hv] = &ixgbevf_X550EM_x_vf_hv_info,
};
/* ixgbevf_pci_tbl - PCI Device ID Table
@@ -78,9 +83,13 @@ static const struct ixgbevf_info *ixgbevf_info_tbl[] = {
*/
static const struct pci_device_id ixgbevf_pci_tbl[] = {
{PCI_VDEVICE(INTEL, IXGBE_DEV_ID_82599_VF), board_82599_vf },
+ {PCI_VDEVICE(INTEL, IXGBE_DEV_ID_82599_VF_HV), board_82599_vf_hv },
{PCI_VDEVICE(INTEL, IXGBE_DEV_ID_X540_VF), board_X540_vf },
+ {PCI_VDEVICE(INTEL, IXGBE_DEV_ID_X540_VF_HV), board_X540_vf_hv },
{PCI_VDEVICE(INTEL, IXGBE_DEV_ID_X550_VF), board_X550_vf },
+ {PCI_VDEVICE(INTEL, IXGBE_DEV_ID_X550_VF_HV), board_X550_vf_hv },
{PCI_VDEVICE(INTEL, IXGBE_DEV_ID_X550EM_X_VF), board_X550EM_x_vf },
+ {PCI_VDEVICE(INTEL, IXGBE_DEV_ID_X550EM_X_VF_HV), board_X550EM_x_vf_hv},
/* required last entry */
{0, }
};
@@ -1809,12 +1818,13 @@ static int ixgbevf_vlan_rx_add_vid(struct net_device *netdev,
{
struct ixgbevf_adapter *adapter = netdev_priv(netdev);
struct ixgbe_hw *hw = &adapter->hw;
- int err;
+ int err = 0;
spin_lock_bh(&adapter->mbx_lock);
/* add VID to filter table */
- err = hw->mac.ops.set_vfta(hw, vid, 0, true);
+ if (hw->mac.ops.set_vfta)
+ err = hw->mac.ops.set_vfta(hw, vid, 0, true);
spin_unlock_bh(&adapter->mbx_lock);
@@ -1835,12 +1845,13 @@ static int ixgbevf_vlan_rx_kill_vid(struct net_device *netdev,
{
struct ixgbevf_adapter *adapter = netdev_priv(netdev);
struct ixgbe_hw *hw = &adapter->hw;
- int err;
+ int err = 0;
spin_lock_bh(&adapter->mbx_lock);
/* remove VID from filter table */
- err = hw->mac.ops.set_vfta(hw, vid, 0, false);
+ if (hw->mac.ops.set_vfta)
+ err = hw->mac.ops.set_vfta(hw, vid, 0, false);
spin_unlock_bh(&adapter->mbx_lock);
@@ -1873,14 +1884,16 @@ static int ixgbevf_write_uc_addr_list(struct net_device *netdev)
struct netdev_hw_addr *ha;
netdev_for_each_uc_addr(ha, netdev) {
- hw->mac.ops.set_uc_addr(hw, ++count, ha->addr);
+ if (hw->mac.ops.set_uc_addr)
+ hw->mac.ops.set_uc_addr(hw, ++count, ha->addr);
udelay(200);
}
} else {
/* If the list is empty then send message to PF driver to
* clear all MAC VLANs on this VF.
*/
- hw->mac.ops.set_uc_addr(hw, 0, NULL);
+ if (hw->mac.ops.set_uc_addr)
+ hw->mac.ops.set_uc_addr(hw, 0, NULL);
}
return count;
@@ -1908,10 +1921,13 @@ static void ixgbevf_set_rx_mode(struct net_device *netdev)
spin_lock_bh(&adapter->mbx_lock);
- hw->mac.ops.update_xcast_mode(hw, netdev, xcast_mode);
+ if (hw->mac.ops.update_mc_addr_list)
+ if (hw->mac.ops.update_xcast_mode)
+ hw->mac.ops.update_xcast_mode(hw, netdev, xcast_mode);
/* reprogram multicast list */
- hw->mac.ops.update_mc_addr_list(hw, netdev);
+ if (hw->mac.ops.update_mc_addr_list)
+ hw->mac.ops.update_mc_addr_list(hw, netdev);
ixgbevf_write_uc_addr_list(netdev);
@@ -2074,10 +2090,13 @@ static void ixgbevf_up_complete(struct ixgbevf_adapter *adapter)
spin_lock_bh(&adapter->mbx_lock);
- if (is_valid_ether_addr(hw->mac.addr))
- hw->mac.ops.set_rar(hw, 0, hw->mac.addr, 0);
- else
- hw->mac.ops.set_rar(hw, 0, hw->mac.perm_addr, 0);
+ if (is_valid_ether_addr(hw->mac.addr)) {
+ if (hw->mac.ops.set_rar)
+ hw->mac.ops.set_rar(hw, 0, hw->mac.addr, 0);
+ } else {
+ if (hw->mac.ops.set_rar)
+ hw->mac.ops.set_rar(hw, 0, hw->mac.perm_addr, 0);
+ }
spin_unlock_bh(&adapter->mbx_lock);
@@ -3672,14 +3691,15 @@ static int ixgbevf_set_mac(struct net_device *netdev, void *p)
struct ixgbevf_adapter *adapter = netdev_priv(netdev);
struct ixgbe_hw *hw = &adapter->hw;
struct sockaddr *addr = p;
- int err;
+ int err = 0;
if (!is_valid_ether_addr(addr->sa_data))
return -EADDRNOTAVAIL;
spin_lock_bh(&adapter->mbx_lock);
- err = hw->mac.ops.set_rar(hw, 0, addr->sa_data, 0);
+ if (hw->mac.ops.set_rar)
+ err = hw->mac.ops.set_rar(hw, 0, addr->sa_data, 0);
spin_unlock_bh(&adapter->mbx_lock);
diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.c b/drivers/net/ethernet/intel/ixgbevf/mbx.c
index dc68fea..298a0da 100644
--- a/drivers/net/ethernet/intel/ixgbevf/mbx.c
+++ b/drivers/net/ethernet/intel/ixgbevf/mbx.c
@@ -346,3 +346,15 @@ const struct ixgbe_mbx_operations ixgbevf_mbx_ops = {
.check_for_rst = ixgbevf_check_for_rst_vf,
};
+/**
+ * Mailbox operations when running on Hyper-V.
+ * On Hyper-V, PF/VF communiction is not through the
+ * hardware mailbox; this communication is through
+ * a software mediated path.
+ * Most mail box operations are noop while running on
+ * Hyper-V.
+ */
+const struct ixgbe_mbx_operations ixgbevf_hv_mbx_ops = {
+ .init_params = ixgbevf_init_mbx_params_vf,
+ .check_for_rst = ixgbevf_check_for_rst_vf,
+};
diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c
index 4d613a4..92397fd 100644
--- a/drivers/net/ethernet/intel/ixgbevf/vf.c
+++ b/drivers/net/ethernet/intel/ixgbevf/vf.c
@@ -27,6 +27,13 @@
#include "vf.h"
#include "ixgbevf.h"
+/*
+ * On Hyper-V, to reset, we need to read from this offset
+ * from the PCI config space. This is the mechanism used on
+ * Hyper-V to support PF/VF communication.
+ */
+#define IXGBE_HV_RESET_OFFSET 0x201
+
/**
* ixgbevf_start_hw_vf - Prepare hardware for Tx/Rx
* @hw: pointer to hardware structure
@@ -126,6 +133,23 @@ static s32 ixgbevf_reset_hw_vf(struct ixgbe_hw *hw)
}
/**
+ * Hyper-V variant; the VF/PF communication is through the PCI
+ * config space.
+ */
+static s32 ixgbevf_hv_reset_hw_vf(struct ixgbe_hw *hw)
+{
+ int i;
+ struct ixgbevf_adapter *adapter = hw->back;
+
+ for (i = 0; i < 6; i++)
+ pci_read_config_byte(adapter->pdev,
+ (i + IXGBE_HV_RESET_OFFSET),
+ &hw->mac.perm_addr[i]);
+
+ return 0;
+}
+
+/**
* ixgbevf_stop_hw_vf - Generic stop Tx/Rx units
* @hw: pointer to hardware structure
*
@@ -656,6 +680,68 @@ out:
}
/**
+ * Hyper-V variant; there is no mailbox communication.
+ */
+static s32 ixgbevf_hv_check_mac_link_vf(struct ixgbe_hw *hw,
+ ixgbe_link_speed *speed,
+ bool *link_up,
+ bool autoneg_wait_to_complete)
+{
+ struct ixgbe_mbx_info *mbx = &hw->mbx;
+ struct ixgbe_mac_info *mac = &hw->mac;
+ s32 ret_val = 0;
+ u32 links_reg;
+
+ /* If we were hit with a reset drop the link */
+ if (!mbx->ops.check_for_rst(hw) || !mbx->timeout)
+ mac->get_link_status = true;
+
+ if (!mac->get_link_status)
+ goto out;
+
+ /* if link status is down no point in checking to see if pf is up */
+ links_reg = IXGBE_READ_REG(hw, IXGBE_VFLINKS);
+ if (!(links_reg & IXGBE_LINKS_UP))
+ goto out;
+
+ /* for SFP+ modules and DA cables on 82599 it can take up to 500usecs
+ * before the link status is correct
+ */
+ if (mac->type == ixgbe_mac_82599_vf) {
+ int i;
+
+ for (i = 0; i < 5; i++) {
+ udelay(100);
+ links_reg = IXGBE_READ_REG(hw, IXGBE_VFLINKS);
+
+ if (!(links_reg & IXGBE_LINKS_UP))
+ goto out;
+ }
+ }
+
+ switch (links_reg & IXGBE_LINKS_SPEED_82599) {
+ case IXGBE_LINKS_SPEED_10G_82599:
+ *speed = IXGBE_LINK_SPEED_10GB_FULL;
+ break;
+ case IXGBE_LINKS_SPEED_1G_82599:
+ *speed = IXGBE_LINK_SPEED_1GB_FULL;
+ break;
+ case IXGBE_LINKS_SPEED_100_82599:
+ *speed = IXGBE_LINK_SPEED_100_FULL;
+ break;
+ }
+
+ /* if we passed all the tests above then the link is up and we no
+ * longer need to check for link
+ */
+ mac->get_link_status = false;
+
+out:
+ *link_up = !mac->get_link_status;
+ return ret_val;
+}
+
+/**
* ixgbevf_rlpml_set_vf - Set the maximum receive packet length
* @hw: pointer to the HW structure
* @max_size: value to assign to max frame size
@@ -663,6 +749,19 @@ out:
void ixgbevf_rlpml_set_vf(struct ixgbe_hw *hw, u16 max_size)
{
u32 msgbuf[2];
+ u32 reg;
+
+ if (x86_hyper == &x86_hyper_ms_hyperv) {
+ /*
+ * If we are on Hyper-V, we implement
+ * this functionality differently.
+ */
+ reg = IXGBE_READ_REG(hw, IXGBE_VFRXDCTL(0));
+ /* CRC == 4 */
+ reg |= ((max_size + 4) | IXGBE_RXDCTL_RLPML_EN);
+ IXGBE_WRITE_REG(hw, IXGBE_VFRXDCTL(0), reg);
+ return;
+ }
msgbuf[0] = IXGBE_VF_SET_LPE;
msgbuf[1] = max_size;
@@ -679,6 +778,16 @@ int ixgbevf_negotiate_api_version(struct ixgbe_hw *hw, int api)
int err;
u32 msg[3];
+ if (x86_hyper == &x86_hyper_ms_hyperv) {
+ /*
+ * Hyper-V only supports api version ixgbe_mbox_api_10
+ */
+ if (api != ixgbe_mbox_api_10)
+ return IXGBE_ERR_INVALID_ARGUMENT;
+
+ return 0;
+ }
+
/* Negotiate the mailbox API version */
msg[0] = IXGBE_VF_API_NEGOTIATE;
msg[1] = api;
@@ -776,22 +885,51 @@ static const struct ixgbe_mac_operations ixgbevf_mac_ops = {
.set_vfta = ixgbevf_set_vfta_vf,
};
+static const struct ixgbe_mac_operations ixgbevf_hv_mac_ops = {
+ .init_hw = ixgbevf_init_hw_vf,
+ .reset_hw = ixgbevf_hv_reset_hw_vf,
+ .start_hw = ixgbevf_start_hw_vf,
+ .get_mac_addr = ixgbevf_get_mac_addr_vf,
+ .stop_adapter = ixgbevf_stop_hw_vf,
+ .setup_link = ixgbevf_setup_mac_link_vf,
+ .check_link = ixgbevf_hv_check_mac_link_vf,
+};
const struct ixgbevf_info ixgbevf_82599_vf_info = {
.mac = ixgbe_mac_82599_vf,
.mac_ops = &ixgbevf_mac_ops,
};
+const struct ixgbevf_info ixgbevf_82599_vf_hv_info = {
+ .mac = ixgbe_mac_82599_vf,
+ .mac_ops = &ixgbevf_hv_mac_ops,
+};
+
const struct ixgbevf_info ixgbevf_X540_vf_info = {
.mac = ixgbe_mac_X540_vf,
.mac_ops = &ixgbevf_mac_ops,
};
+const struct ixgbevf_info ixgbevf_X540_vf_hv_info = {
+ .mac = ixgbe_mac_X540_vf,
+ .mac_ops = &ixgbevf_hv_mac_ops,
+};
+
const struct ixgbevf_info ixgbevf_X550_vf_info = {
.mac = ixgbe_mac_X550_vf,
.mac_ops = &ixgbevf_mac_ops,
};
+const struct ixgbevf_info ixgbevf_X550_vf_hv_info = {
+ .mac = ixgbe_mac_X550_vf,
+ .mac_ops = &ixgbevf_hv_mac_ops,
+};
+
const struct ixgbevf_info ixgbevf_X550EM_x_vf_info = {
.mac = ixgbe_mac_X550EM_x_vf,
.mac_ops = &ixgbevf_mac_ops,
};
+
+const struct ixgbevf_info ixgbevf_X550EM_x_vf_hv_info = {
+ .mac = ixgbe_mac_X550EM_x_vf,
+ .mac_ops = &ixgbevf_hv_mac_ops,
+};
diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.h b/drivers/net/ethernet/intel/ixgbevf/vf.h
index ef9f773..7242a97 100644
--- a/drivers/net/ethernet/intel/ixgbevf/vf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/vf.h
@@ -32,7 +32,9 @@
#include <linux/interrupt.h>
#include <linux/if_ether.h>
#include <linux/netdevice.h>
+#include <asm/hypervisor.h>
+#include <asm/hypervisor.h>
#include "defines.h"
#include "regs.h"
#include "mbx.h"
--
1.7.4.1
^ permalink raw reply related
* [PATCH net-next 1/2] ethernet: intel: Add the device ID's presented while running on Hyper-V
From: K. Y. Srinivasan @ 2016-04-14 23:55 UTC (permalink / raw)
To: davem, netdev, linux-kernel, devel, olaf, apw, jasowang, eli,
jackm, yevgenyp, john.ronciak, intel-wired-lan
In-Reply-To: <1460678121-30308-1-git-send-email-kys@microsoft.com>
Intel SR-IOV cards present different ID when running on Hyper-V.
Add the device IDs presented while running on Hyper-V.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/net/ethernet/intel/ixgbevf/defines.h | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/defines.h b/drivers/net/ethernet/intel/ixgbevf/defines.h
index 5843458..1306a0d 100644
--- a/drivers/net/ethernet/intel/ixgbevf/defines.h
+++ b/drivers/net/ethernet/intel/ixgbevf/defines.h
@@ -33,6 +33,11 @@
#define IXGBE_DEV_ID_X550_VF 0x1565
#define IXGBE_DEV_ID_X550EM_X_VF 0x15A8
+#define IXGBE_DEV_ID_82599_VF_HV 0x152E
+#define IXGBE_DEV_ID_X540_VF_HV 0x1530
+#define IXGBE_DEV_ID_X550_VF_HV 0x1564
+#define IXGBE_DEV_ID_X550EM_X_VF_HV 0x15A9
+
#define IXGBE_VF_IRQ_CLEAR_MASK 7
#define IXGBE_VF_MAX_TX_QUEUES 8
#define IXGBE_VF_MAX_RX_QUEUES 8
--
1.7.4.1
^ permalink raw reply related
* [PATCH net-next 0/2] ethernet: intel: Support Hyper-V hosts
From: K. Y. Srinivasan @ 2016-04-14 23:55 UTC (permalink / raw)
To: davem, netdev, linux-kernel, devel, olaf, apw, jasowang, eli,
jackm, yevgenyp, john.ronciak, intel-wired-lan
Make adjustments to the Intel 10G VF driver to support
running on Hyper-V hosts.
K. Y. Srinivasan (2):
ethernet: intel: Add the device ID's presented while running on
Hyper-V
intel: ixgbevf: Support Windows hosts (Hyper-V)
drivers/net/ethernet/intel/ixgbevf/defines.h | 5 +
drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 11 ++
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 56 ++++++---
drivers/net/ethernet/intel/ixgbevf/mbx.c | 12 ++
drivers/net/ethernet/intel/ixgbevf/vf.c | 138 +++++++++++++++++++++
drivers/net/ethernet/intel/ixgbevf/vf.h | 2 +
6 files changed, 206 insertions(+), 18 deletions(-)
--
1.7.4.1
^ permalink raw reply
* Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver
From: Timur Tabi @ 2016-04-14 23:34 UTC (permalink / raw)
To: Vikram Sethi, Florian Fainelli, netdev, linux-kernel, devicetree,
linux-arm-msm, sdharia, Shanker Donthineni, Greg Kroah-Hartman,
cov, gavidov, Rob Herring, andrew, bjorn.andersson,
Mark Langsdorf, Jon Masters, Andy Gross, David S. Miller
In-Reply-To: <571012E6.6050303@codeaurora.org>
Vikram Sethi wrote:
>>> >> retval = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>>> >> if (retval) {
>>> >> dev_err(&pdev->dev, "failed to set DMA mask err %d\n", retval);
>>> >> goto err_res;
>>> >> }
> How can you set the mask to 64 bits when the EMAC IP on FSM9900 and QDF2432 can only do 32 bit DMA?
> The mask in that API is a bit mask describing which bits of an address your device supports.
Vikram, Shanker, and I discussed this offline, and came to a consensus.
The FSM9900 is a 32-bit platform, so the kernel will never create a DMA
address above 4GB. Even if the driver sets the mask to 64 bits, it will
technically work. However, the mask should be set to 32 because all
address buses are 32 bits.
The QDF2432 is different. Although it's an ARM64 platform, we have the
unfortunate situation that only 32 bits of that address is wired to the
rest of the chip. So even though the Emac can handle 64-bit bus
addresses, if it actually attempts to DMA above 4GB, the address will
get truncated and corrupt memory. The mask needs to be set to 32.
There may or may not be other ARM64 chips from us that won't have this
problem in the future, so these hypothetical chips would have a mask of 64.
So I think the solution is to create a device tree (and ACPI) property
that holds the mask.
dma-mask = <0 0xffffffff>;
or
dma-mask = <0xffffffff 0xffffffff>;
The driver will then do this:
u64 dma_mask;
device_property_read_u64(&pdev->dev, "dma-mask", &dma_mask);
dma_coerce_mask_and_coherent(&pdev->dev, dma_mask);
What I'm not sure yet is whether I should call
dma_coerce_mask_and_coherent() or dma_set_coherent_mask().
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, a Linux Foundation collaborative project.
^ permalink raw reply
* [PATCH net-next 1/1] hv_netvsc: Implement support for VF drivers on Hyper-V
From: K. Y. Srinivasan @ 2016-04-14 23:31 UTC (permalink / raw)
To: davem, netdev, linux-kernel, devel, olaf, apw, jasowang, eli,
jackm, yevgenyp, john.ronciak, intel-wired-lan
Support VF drivers on Hyper-V. On Hyper-V, each VF instance presented to
the guest has an associated synthetic interface that shares the MAC address
with the VF instance. Typically these are bonded together to support
live migration. By default, the host delivers all the incoming packets
on the synthetic interface. Once the VF is up, we need to explicitly switch
the data path on the host to divert traffic onto the VF interface. Even after
switching the data path, broadcast and multicast packets are always delivered
on the synthetic interface and these will have to be injected back onto the
VF interface (if VF is up).
This patch implements the necessary support in netvsc to support Linux
VF drivers.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/net/hyperv/hyperv_net.h | 14 ++
drivers/net/hyperv/netvsc.c | 29 ++++
drivers/net/hyperv/netvsc_drv.c | 312 +++++++++++++++++++++++++++++++++---
drivers/net/hyperv/rndis_filter.c | 6 +
4 files changed, 335 insertions(+), 26 deletions(-)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 8b3bd8e..6700a4d 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -202,6 +202,8 @@ int rndis_filter_receive(struct hv_device *dev,
int rndis_filter_set_packet_filter(struct rndis_device *dev, u32 new_filter);
int rndis_filter_set_device_mac(struct hv_device *hdev, char *mac);
+void netvsc_switch_datapath(struct netvsc_device *nv_dev, bool vf);
+
#define NVSP_INVALID_PROTOCOL_VERSION ((u32)0xFFFFFFFF)
#define NVSP_PROTOCOL_VERSION_1 2
@@ -641,6 +643,12 @@ struct netvsc_reconfig {
u32 event;
};
+struct garp_wrk {
+ struct work_struct dwrk;
+ struct net_device *netdev;
+ struct netvsc_device *netvsc_dev;
+};
+
/* The context of the netvsc device */
struct net_device_context {
/* point back to our device context */
@@ -656,6 +664,7 @@ struct net_device_context {
struct work_struct work;
u32 msg_enable; /* debug level */
+ struct garp_wrk gwrk;
struct netvsc_stats __percpu *tx_stats;
struct netvsc_stats __percpu *rx_stats;
@@ -730,6 +739,11 @@ struct netvsc_device {
u32 vf_alloc;
/* Serial number of the VF to team with */
u32 vf_serial;
+ atomic_t open_cnt;
+ /* State to manage the associated VF interface. */
+ bool vf_inject;
+ struct net_device *vf_netdev;
+ atomic_t vf_use_cnt;
};
/* NdisInitialize message */
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index ec313fc..eddce3c 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -33,6 +33,30 @@
#include "hyperv_net.h"
+/*
+ * Switch the data path from the synthetic interface to the VF
+ * interface.
+ */
+void netvsc_switch_datapath(struct netvsc_device *nv_dev, bool vf)
+{
+ struct nvsp_message *init_pkt = &nv_dev->channel_init_pkt;
+ struct hv_device *dev = nv_dev->dev;
+
+ memset(init_pkt, 0, sizeof(struct nvsp_message));
+ init_pkt->hdr.msg_type = NVSP_MSG4_TYPE_SWITCH_DATA_PATH;
+ if (vf)
+ init_pkt->msg.v4_msg.active_dp.active_datapath =
+ NVSP_DATAPATH_VF;
+ else
+ init_pkt->msg.v4_msg.active_dp.active_datapath =
+ NVSP_DATAPATH_SYNTHETIC;
+
+ vmbus_sendpacket(dev->channel, init_pkt,
+ sizeof(struct nvsp_message),
+ (unsigned long)init_pkt,
+ VM_PKT_DATA_INBAND, 0);
+}
+
static struct netvsc_device *alloc_net_device(struct hv_device *device)
{
@@ -52,11 +76,16 @@ static struct netvsc_device *alloc_net_device(struct hv_device *device)
init_waitqueue_head(&net_device->wait_drain);
net_device->start_remove = false;
net_device->destroy = false;
+ atomic_set(&net_device->open_cnt, 0);
+ atomic_set(&net_device->vf_use_cnt, 0);
net_device->dev = device;
net_device->ndev = ndev;
net_device->max_pkt = RNDIS_MAX_PKT_DEFAULT;
net_device->pkt_align = RNDIS_PKT_ALIGN_DEFAULT;
+ net_device->vf_netdev = NULL;
+ net_device->vf_inject = false;
+
hv_set_drvdata(device, net_device);
return net_device;
}
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index b8121eb..bfdb568a 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -610,42 +610,24 @@ void netvsc_linkstatus_callback(struct hv_device *device_obj,
schedule_delayed_work(&ndev_ctx->dwork, 0);
}
-/*
- * netvsc_recv_callback - Callback when we receive a packet from the
- * "wire" on the specified device.
- */
-int netvsc_recv_callback(struct hv_device *device_obj,
+
+static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
struct hv_netvsc_packet *packet,
- void **data,
struct ndis_tcp_ip_checksum_info *csum_info,
- struct vmbus_channel *channel,
- u16 vlan_tci)
+ void *data, u16 vlan_tci)
{
- struct net_device *net;
- struct net_device_context *net_device_ctx;
struct sk_buff *skb;
- struct netvsc_stats *rx_stats;
- net = ((struct netvsc_device *)hv_get_drvdata(device_obj))->ndev;
- if (!net || net->reg_state != NETREG_REGISTERED) {
- return NVSP_STAT_FAIL;
- }
- net_device_ctx = netdev_priv(net);
- rx_stats = this_cpu_ptr(net_device_ctx->rx_stats);
-
- /* Allocate a skb - TODO direct I/O to pages? */
skb = netdev_alloc_skb_ip_align(net, packet->total_data_buflen);
- if (unlikely(!skb)) {
- ++net->stats.rx_dropped;
- return NVSP_STAT_FAIL;
- }
+ if (!skb)
+ return skb;
/*
* Copy to skb. This copy is needed here since the memory pointed by
* hv_netvsc_packet cannot be deallocated
*/
- memcpy(skb_put(skb, packet->total_data_buflen), *data,
- packet->total_data_buflen);
+ memcpy(skb_put(skb, packet->total_data_buflen), data,
+ packet->total_data_buflen);
skb->protocol = eth_type_trans(skb, net);
if (csum_info) {
@@ -663,6 +645,75 @@ int netvsc_recv_callback(struct hv_device *device_obj,
__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
vlan_tci);
+ return skb;
+}
+
+/*
+ * netvsc_recv_callback - Callback when we receive a packet from the
+ * "wire" on the specified device.
+ */
+int netvsc_recv_callback(struct hv_device *device_obj,
+ struct hv_netvsc_packet *packet,
+ void **data,
+ struct ndis_tcp_ip_checksum_info *csum_info,
+ struct vmbus_channel *channel,
+ u16 vlan_tci)
+{
+ struct net_device *net;
+ struct net_device_context *net_device_ctx;
+ struct sk_buff *skb;
+ struct sk_buff *vf_skb;
+ struct netvsc_stats *rx_stats;
+ struct netvsc_device *netvsc_dev = hv_get_drvdata(device_obj);
+ u32 bytes_recvd = packet->total_data_buflen;
+ int ret = 0;
+
+ net = netvsc_dev->ndev;
+ if (!net || net->reg_state != NETREG_REGISTERED)
+ return NVSP_STAT_FAIL;
+
+ if (READ_ONCE(netvsc_dev->vf_inject)) {
+ atomic_inc(&netvsc_dev->vf_use_cnt);
+ if (!READ_ONCE(netvsc_dev->vf_inject)) {
+ /*
+ * We raced; just move on.
+ */
+ atomic_dec(&netvsc_dev->vf_use_cnt);
+ goto vf_injection_done;
+ }
+
+ /*
+ * Inject this packet into the VF inerface.
+ * On Hyper-V, multicast and brodcast packets
+ * are only delivered on the synthetic interface
+ * (after subjecting these to policy filters on
+ * the host). Deliver these via the VF interface
+ * in the guest.
+ */
+ vf_skb = netvsc_alloc_recv_skb(netvsc_dev->vf_netdev, packet,
+ csum_info, *data, vlan_tci);
+ if (vf_skb != NULL) {
+ ++netvsc_dev->vf_netdev->stats.rx_packets;
+ netvsc_dev->vf_netdev->stats.rx_bytes += bytes_recvd;
+ netif_receive_skb(vf_skb);
+ } else {
+ ++net->stats.rx_dropped;
+ ret = NVSP_STAT_FAIL;
+ }
+ atomic_dec(&netvsc_dev->vf_use_cnt);
+ return ret;
+ }
+
+vf_injection_done:
+ net_device_ctx = netdev_priv(net);
+ rx_stats = this_cpu_ptr(net_device_ctx->rx_stats);
+
+ /* Allocate a skb - TODO direct I/O to pages? */
+ skb = netvsc_alloc_recv_skb(net, packet, csum_info, *data, vlan_tci);
+ if (unlikely(!skb)) {
+ ++net->stats.rx_dropped;
+ return NVSP_STAT_FAIL;
+ }
skb_record_rx_queue(skb, channel->
offermsg.offer.sub_channel_index);
@@ -1102,6 +1153,175 @@ static void netvsc_free_netdev(struct net_device *netdev)
free_netdev(netdev);
}
+static void netvsc_notify_peers(struct work_struct *wrk)
+{
+ struct garp_wrk *gwrk;
+
+ gwrk = container_of(wrk, struct garp_wrk, dwrk);
+
+ netdev_notify_peers(gwrk->netdev);
+
+ atomic_dec(&gwrk->netvsc_dev->vf_use_cnt);
+}
+
+static struct netvsc_device *get_netvsc_device(char *mac)
+{
+ struct net_device *dev;
+ struct net_device_context *netvsc_ctx = NULL;
+ int rtnl_locked;
+
+ rtnl_locked = rtnl_trylock();
+
+ for_each_netdev(&init_net, dev) {
+ if (memcmp(dev->dev_addr, mac, ETH_ALEN) == 0) {
+ if (dev->netdev_ops != &device_ops)
+ continue;
+ netvsc_ctx = netdev_priv(dev);
+ break;
+ }
+ }
+ if (rtnl_locked)
+ rtnl_unlock();
+
+ if (netvsc_ctx == NULL)
+ return NULL;
+
+ return hv_get_drvdata(netvsc_ctx->device_ctx);
+}
+
+static int netvsc_register_vf(struct net_device *vf_netdev)
+{
+ struct netvsc_device *netvsc_dev;
+ const struct ethtool_ops *eth_ops = vf_netdev->ethtool_ops;
+
+ if (eth_ops == NULL || eth_ops == ðtool_ops)
+ return NOTIFY_DONE;
+
+ /*
+ * We will use the MAC address to locate the synthetic interface to
+ * associate with the VF interface. If we don't find a matching
+ * synthetic interface, move on.
+ */
+ netvsc_dev = get_netvsc_device(vf_netdev->dev_addr);
+ if (netvsc_dev == NULL)
+ return NOTIFY_DONE;
+
+ netdev_info(netvsc_dev->ndev, "VF registering: %s\n", vf_netdev->name);
+ /*
+ * Take a reference on the module.
+ */
+ try_module_get(THIS_MODULE);
+ netvsc_dev->vf_netdev = vf_netdev;
+ return NOTIFY_OK;
+}
+
+
+static int netvsc_vf_up(struct net_device *vf_netdev)
+{
+ struct netvsc_device *netvsc_dev;
+ const struct ethtool_ops *eth_ops = vf_netdev->ethtool_ops;
+ struct net_device_context *net_device_ctx;
+
+ if (eth_ops == ðtool_ops)
+ return NOTIFY_DONE;
+
+ netvsc_dev = get_netvsc_device(vf_netdev->dev_addr);
+
+ if ((netvsc_dev == NULL) || (netvsc_dev->vf_netdev == NULL))
+ return NOTIFY_DONE;
+
+ netdev_info(netvsc_dev->ndev, "VF up: %s\n", vf_netdev->name);
+ net_device_ctx = netdev_priv(netvsc_dev->ndev);
+ netvsc_dev->vf_inject = true;
+
+ /*
+ * Open the device before switching data path.
+ */
+ rndis_filter_open(net_device_ctx->device_ctx);
+
+ /*
+ * notify the host to switch the data path.
+ */
+ netvsc_switch_datapath(netvsc_dev, true);
+ netdev_info(netvsc_dev->ndev, "Data path switched to VF: %s\n",
+ vf_netdev->name);
+
+ netif_carrier_off(netvsc_dev->ndev);
+
+ /*
+ * Now notify peers. We are scheduling work to
+ * notify peers; take a reference to prevent
+ * the VF interface from vanishing.
+ */
+ atomic_inc(&netvsc_dev->vf_use_cnt);
+ net_device_ctx->gwrk.netdev = vf_netdev;
+ net_device_ctx->gwrk.netvsc_dev = netvsc_dev;
+ schedule_work(&net_device_ctx->gwrk.dwrk);
+
+ return NOTIFY_OK;
+}
+
+
+static int netvsc_vf_down(struct net_device *vf_netdev)
+{
+ struct netvsc_device *netvsc_dev;
+ struct net_device_context *net_device_ctx;
+ const struct ethtool_ops *eth_ops = vf_netdev->ethtool_ops;
+
+ if (eth_ops == ðtool_ops)
+ return NOTIFY_DONE;
+
+ netvsc_dev = get_netvsc_device(vf_netdev->dev_addr);
+
+ if ((netvsc_dev == NULL) || (netvsc_dev->vf_netdev == NULL))
+ return NOTIFY_DONE;
+
+ netdev_info(netvsc_dev->ndev, "VF down: %s\n", vf_netdev->name);
+ net_device_ctx = netdev_priv(netvsc_dev->ndev);
+ netvsc_dev->vf_inject = false;
+ /*
+ * Wait for currently active users to
+ * drain out.
+ */
+
+ while (atomic_read(&netvsc_dev->vf_use_cnt) != 0)
+ udelay(50);
+ netvsc_switch_datapath(netvsc_dev, false);
+ netdev_info(netvsc_dev->ndev, "Data path switched from VF: %s\n",
+ vf_netdev->name);
+ rndis_filter_close(net_device_ctx->device_ctx);
+ netif_carrier_on(netvsc_dev->ndev);
+ /*
+ * Notify peers.
+ */
+ atomic_inc(&netvsc_dev->vf_use_cnt);
+ net_device_ctx->gwrk.netdev = netvsc_dev->ndev;
+ net_device_ctx->gwrk.netvsc_dev = netvsc_dev;
+ schedule_work(&net_device_ctx->gwrk.dwrk);
+
+ return NOTIFY_OK;
+}
+
+
+static int netvsc_unregister_vf(struct net_device *vf_netdev)
+{
+ struct netvsc_device *netvsc_dev;
+ const struct ethtool_ops *eth_ops = vf_netdev->ethtool_ops;
+
+ if (eth_ops == ðtool_ops)
+ return NOTIFY_DONE;
+
+ netvsc_dev = get_netvsc_device(vf_netdev->dev_addr);
+ if (netvsc_dev == NULL)
+ return NOTIFY_DONE;
+ netdev_info(netvsc_dev->ndev, "VF unregistering: %s\n",
+ vf_netdev->name);
+
+ netvsc_dev->vf_netdev = NULL;
+ module_put(THIS_MODULE);
+ return NOTIFY_OK;
+}
+
static int netvsc_probe(struct hv_device *dev,
const struct hv_vmbus_device_id *dev_id)
{
@@ -1140,6 +1360,7 @@ static int netvsc_probe(struct hv_device *dev,
hv_set_drvdata(dev, net);
INIT_DELAYED_WORK(&net_device_ctx->dwork, netvsc_link_change);
INIT_WORK(&net_device_ctx->work, do_set_multicast);
+ INIT_WORK(&net_device_ctx->gwrk.dwrk, netvsc_notify_peers);
spin_lock_init(&net_device_ctx->lock);
INIT_LIST_HEAD(&net_device_ctx->reconfig_events);
@@ -1235,19 +1456,58 @@ static struct hv_driver netvsc_drv = {
.remove = netvsc_remove,
};
+
+/*
+ * On Hyper-V, every VF interface is matched with a corresponding
+ * synthetic interface. The synthetic interface is presented first
+ * to the guest. When the corresponding VF instance is registered,
+ * we will take care of switching the data path.
+ */
+static int netvsc_netdev_event(struct notifier_block *this,
+ unsigned long event, void *ptr)
+{
+ struct net_device *event_dev = netdev_notifier_info_to_dev(ptr);
+
+ switch (event) {
+ case NETDEV_REGISTER:
+ return netvsc_register_vf(event_dev);
+ case NETDEV_UNREGISTER:
+ return netvsc_unregister_vf(event_dev);
+ case NETDEV_UP:
+ return netvsc_vf_up(event_dev);
+ case NETDEV_DOWN:
+ return netvsc_vf_down(event_dev);
+ default:
+ return NOTIFY_DONE;
+ }
+}
+
+static struct notifier_block netvsc_netdev_notifier = {
+ .notifier_call = netvsc_netdev_event,
+};
+
static void __exit netvsc_drv_exit(void)
{
+ unregister_netdevice_notifier(&netvsc_netdev_notifier);
vmbus_driver_unregister(&netvsc_drv);
}
static int __init netvsc_drv_init(void)
{
+ int ret;
+
if (ring_size < RING_SIZE_MIN) {
ring_size = RING_SIZE_MIN;
pr_info("Increased ring_size to %d (min allowed)\n",
ring_size);
}
- return vmbus_driver_register(&netvsc_drv);
+ ret = vmbus_driver_register(&netvsc_drv);
+
+ if (ret)
+ return ret;
+
+ register_netdevice_notifier(&netvsc_netdev_notifier);
+ return 0;
}
MODULE_LICENSE("GPL");
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index c4e1e04..a59cdeb 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -1229,6 +1229,9 @@ int rndis_filter_open(struct hv_device *dev)
if (!net_device)
return -EINVAL;
+ if (atomic_inc_return(&net_device->open_cnt) != 1)
+ return 0;
+
return rndis_filter_open_device(net_device->extension);
}
@@ -1239,5 +1242,8 @@ int rndis_filter_close(struct hv_device *dev)
if (!nvdev)
return -EINVAL;
+ if (atomic_dec_return(&nvdev->open_cnt) != 0)
+ return 0;
+
return rndis_filter_close_device(nvdev->extension);
}
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH net-next 2/2] intel: ixgbevf: Support Windows hosts (Hyper-V)
From: Alexander Duyck @ 2016-04-14 23:18 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: olaf, Netdev, Jason Wang, linux-kernel@vger.kernel.org, jackm,
yevgenyp, John Ronciak, intel-wired-lan, eli, Robo Bot, devel,
David Miller
In-Reply-To: <1460678142-30353-2-git-send-email-kys@microsoft.com>
On Thu, Apr 14, 2016 at 4:55 PM, K. Y. Srinivasan <kys@microsoft.com> wrote:
> On Hyper-V, the VF/PF communication is a via software mediated path
> as opposed to the hardware mailbox. Make the necessary
> adjustments to support Hyper-V.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 11 ++
> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 56 ++++++---
> drivers/net/ethernet/intel/ixgbevf/mbx.c | 12 ++
> drivers/net/ethernet/intel/ixgbevf/vf.c | 138 +++++++++++++++++++++
> drivers/net/ethernet/intel/ixgbevf/vf.h | 2 +
> 5 files changed, 201 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> index 5ac60ee..f8d2a0b 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> @@ -460,9 +460,13 @@ enum ixbgevf_state_t {
>
> enum ixgbevf_boards {
> board_82599_vf,
> + board_82599_vf_hv,
> board_X540_vf,
> + board_X540_vf_hv,
> board_X550_vf,
> + board_X550_vf_hv,
> board_X550EM_x_vf,
> + board_X550EM_x_vf_hv,
> };
>
> enum ixgbevf_xcast_modes {
> @@ -477,6 +481,13 @@ extern const struct ixgbevf_info ixgbevf_X550_vf_info;
> extern const struct ixgbevf_info ixgbevf_X550EM_x_vf_info;
> extern const struct ixgbe_mbx_operations ixgbevf_mbx_ops;
>
> +
> +extern const struct ixgbevf_info ixgbevf_82599_vf_hv_info;
> +extern const struct ixgbevf_info ixgbevf_X540_vf_hv_info;
> +extern const struct ixgbevf_info ixgbevf_X550_vf_hv_info;
> +extern const struct ixgbevf_info ixgbevf_X550EM_x_vf_hv_info;
> +extern const struct ixgbe_mbx_operations ixgbevf_hv_mbx_ops;
> +
> /* needed by ethtool.c */
> extern const char ixgbevf_driver_name[];
> extern const char ixgbevf_driver_version[];
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> index 007cbe0..4a0ffac 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> @@ -49,6 +49,7 @@
> #include <linux/if.h>
> #include <linux/if_vlan.h>
> #include <linux/prefetch.h>
> +#include <asm/mshyperv.h>
>
> #include "ixgbevf.h"
>
> @@ -62,10 +63,14 @@ static char ixgbevf_copyright[] =
> "Copyright (c) 2009 - 2015 Intel Corporation.";
>
> static const struct ixgbevf_info *ixgbevf_info_tbl[] = {
> - [board_82599_vf] = &ixgbevf_82599_vf_info,
> - [board_X540_vf] = &ixgbevf_X540_vf_info,
> - [board_X550_vf] = &ixgbevf_X550_vf_info,
> - [board_X550EM_x_vf] = &ixgbevf_X550EM_x_vf_info,
> + [board_82599_vf] = &ixgbevf_82599_vf_info,
> + [board_82599_vf_hv] = &ixgbevf_82599_vf_hv_info,
> + [board_X540_vf] = &ixgbevf_X540_vf_info,
> + [board_X540_vf_hv] = &ixgbevf_X540_vf_hv_info,
> + [board_X550_vf] = &ixgbevf_X550_vf_info,
> + [board_X550_vf_hv] = &ixgbevf_X550_vf_hv_info,
> + [board_X550EM_x_vf] = &ixgbevf_X550EM_x_vf_info,
> + [board_X550EM_x_vf_hv] = &ixgbevf_X550EM_x_vf_hv_info,
> };
>
> /* ixgbevf_pci_tbl - PCI Device ID Table
> @@ -78,9 +83,13 @@ static const struct ixgbevf_info *ixgbevf_info_tbl[] = {
> */
> static const struct pci_device_id ixgbevf_pci_tbl[] = {
> {PCI_VDEVICE(INTEL, IXGBE_DEV_ID_82599_VF), board_82599_vf },
> + {PCI_VDEVICE(INTEL, IXGBE_DEV_ID_82599_VF_HV), board_82599_vf_hv },
> {PCI_VDEVICE(INTEL, IXGBE_DEV_ID_X540_VF), board_X540_vf },
> + {PCI_VDEVICE(INTEL, IXGBE_DEV_ID_X540_VF_HV), board_X540_vf_hv },
> {PCI_VDEVICE(INTEL, IXGBE_DEV_ID_X550_VF), board_X550_vf },
> + {PCI_VDEVICE(INTEL, IXGBE_DEV_ID_X550_VF_HV), board_X550_vf_hv },
> {PCI_VDEVICE(INTEL, IXGBE_DEV_ID_X550EM_X_VF), board_X550EM_x_vf },
> + {PCI_VDEVICE(INTEL, IXGBE_DEV_ID_X550EM_X_VF_HV), board_X550EM_x_vf_hv},
> /* required last entry */
> {0, }
> };
> @@ -1809,12 +1818,13 @@ static int ixgbevf_vlan_rx_add_vid(struct net_device *netdev,
> {
> struct ixgbevf_adapter *adapter = netdev_priv(netdev);
> struct ixgbe_hw *hw = &adapter->hw;
> - int err;
> + int err = 0;
>
> spin_lock_bh(&adapter->mbx_lock);
>
> /* add VID to filter table */
> - err = hw->mac.ops.set_vfta(hw, vid, 0, true);
> + if (hw->mac.ops.set_vfta)
> + err = hw->mac.ops.set_vfta(hw, vid, 0, true);
>
> spin_unlock_bh(&adapter->mbx_lock);
>
> @@ -1835,12 +1845,13 @@ static int ixgbevf_vlan_rx_kill_vid(struct net_device *netdev,
> {
> struct ixgbevf_adapter *adapter = netdev_priv(netdev);
> struct ixgbe_hw *hw = &adapter->hw;
> - int err;
> + int err = 0;
>
> spin_lock_bh(&adapter->mbx_lock);
>
> /* remove VID from filter table */
> - err = hw->mac.ops.set_vfta(hw, vid, 0, false);
> + if (hw->mac.ops.set_vfta)
> + err = hw->mac.ops.set_vfta(hw, vid, 0, false);
>
> spin_unlock_bh(&adapter->mbx_lock);
>
> @@ -1873,14 +1884,16 @@ static int ixgbevf_write_uc_addr_list(struct net_device *netdev)
> struct netdev_hw_addr *ha;
>
> netdev_for_each_uc_addr(ha, netdev) {
> - hw->mac.ops.set_uc_addr(hw, ++count, ha->addr);
> + if (hw->mac.ops.set_uc_addr)
> + hw->mac.ops.set_uc_addr(hw, ++count, ha->addr);
> udelay(200);
> }
> } else {
> /* If the list is empty then send message to PF driver to
> * clear all MAC VLANs on this VF.
> */
> - hw->mac.ops.set_uc_addr(hw, 0, NULL);
> + if (hw->mac.ops.set_uc_addr)
> + hw->mac.ops.set_uc_addr(hw, 0, NULL);
> }
>
> return count;
So if I am understanding this correctly it looks like you cannot read
or write any addresses for this device. Would I be correct in
assuming that you get to have the one unicast address that is provided
by the PF and that is it?
If so we may want to look at possibly returning some sort of error on
these calls so that we can configure the device to indicate that we
cannot support any of these filters.
> @@ -1908,10 +1921,13 @@ static void ixgbevf_set_rx_mode(struct net_device *netdev)
>
> spin_lock_bh(&adapter->mbx_lock);
>
> - hw->mac.ops.update_xcast_mode(hw, netdev, xcast_mode);
> + if (hw->mac.ops.update_mc_addr_list)
> + if (hw->mac.ops.update_xcast_mode)
> + hw->mac.ops.update_xcast_mode(hw, netdev, xcast_mode);
>
> /* reprogram multicast list */
> - hw->mac.ops.update_mc_addr_list(hw, netdev);
> + if (hw->mac.ops.update_mc_addr_list)
> + hw->mac.ops.update_mc_addr_list(hw, netdev);
>
> ixgbevf_write_uc_addr_list(netdev);
>
> @@ -2074,10 +2090,13 @@ static void ixgbevf_up_complete(struct ixgbevf_adapter *adapter)
>
> spin_lock_bh(&adapter->mbx_lock);
>
> - if (is_valid_ether_addr(hw->mac.addr))
> - hw->mac.ops.set_rar(hw, 0, hw->mac.addr, 0);
> - else
> - hw->mac.ops.set_rar(hw, 0, hw->mac.perm_addr, 0);
> + if (is_valid_ether_addr(hw->mac.addr)) {
> + if (hw->mac.ops.set_rar)
> + hw->mac.ops.set_rar(hw, 0, hw->mac.addr, 0);
> + } else {
> + if (hw->mac.ops.set_rar)
> + hw->mac.ops.set_rar(hw, 0, hw->mac.perm_addr, 0);
> + }
>
> spin_unlock_bh(&adapter->mbx_lock);
>
Same here. We shouldn't let the user set a MAC address that we cannot
support. We should be returning an error.
> @@ -3672,14 +3691,15 @@ static int ixgbevf_set_mac(struct net_device *netdev, void *p)
> struct ixgbevf_adapter *adapter = netdev_priv(netdev);
> struct ixgbe_hw *hw = &adapter->hw;
> struct sockaddr *addr = p;
> - int err;
> + int err = 0;
>
> if (!is_valid_ether_addr(addr->sa_data))
> return -EADDRNOTAVAIL;
>
> spin_lock_bh(&adapter->mbx_lock);
>
> - err = hw->mac.ops.set_rar(hw, 0, addr->sa_data, 0);
> + if (hw->mac.ops.set_rar)
> + err = hw->mac.ops.set_rar(hw, 0, addr->sa_data, 0);
>
> spin_unlock_bh(&adapter->mbx_lock);
>
Specifically here. If hw->mac.ops.set_rar is NULL we cannot allow any
MAC address change so we should probably return "-EADDRNOTAVAIL".
> diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.c b/drivers/net/ethernet/intel/ixgbevf/mbx.c
> index dc68fea..298a0da 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/mbx.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/mbx.c
> @@ -346,3 +346,15 @@ const struct ixgbe_mbx_operations ixgbevf_mbx_ops = {
> .check_for_rst = ixgbevf_check_for_rst_vf,
> };
>
> +/**
> + * Mailbox operations when running on Hyper-V.
> + * On Hyper-V, PF/VF communiction is not through the
> + * hardware mailbox; this communication is through
> + * a software mediated path.
> + * Most mail box operations are noop while running on
> + * Hyper-V.
> + */
> +const struct ixgbe_mbx_operations ixgbevf_hv_mbx_ops = {
> + .init_params = ixgbevf_init_mbx_params_vf,
> + .check_for_rst = ixgbevf_check_for_rst_vf,
> +};
I'd say if you aren't going to use a mailbox then don't initialize it.
The code was based off of the same code that runs the ixgbe driver.
It should be able to function without a mailbox provided the necessary
calls are updated in the ixgbe_mac_operations that are used by the
hyperv VF.
> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c
> index 4d613a4..92397fd 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/vf.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.c
> @@ -27,6 +27,13 @@
> #include "vf.h"
> #include "ixgbevf.h"
>
> +/*
> + * On Hyper-V, to reset, we need to read from this offset
> + * from the PCI config space. This is the mechanism used on
> + * Hyper-V to support PF/VF communication.
> + */
> +#define IXGBE_HV_RESET_OFFSET 0x201
> +
> /**
> * ixgbevf_start_hw_vf - Prepare hardware for Tx/Rx
> * @hw: pointer to hardware structure
> @@ -126,6 +133,23 @@ static s32 ixgbevf_reset_hw_vf(struct ixgbe_hw *hw)
> }
>
> /**
> + * Hyper-V variant; the VF/PF communication is through the PCI
> + * config space.
> + */
> +static s32 ixgbevf_hv_reset_hw_vf(struct ixgbe_hw *hw)
> +{
> + int i;
> + struct ixgbevf_adapter *adapter = hw->back;
> +
> + for (i = 0; i < 6; i++)
> + pci_read_config_byte(adapter->pdev,
> + (i + IXGBE_HV_RESET_OFFSET),
> + &hw->mac.perm_addr[i]);
> +
> + return 0;
> +}
> +
This bit can be problematic. What about the case where PCI_MMCONFIG
is not defined. In such a case it will kill this function without
reporting an error other than maybe a MAC address that is all 0s or
all FF's.
You might want to add some sort of check here with some message if
such a situation occurs just so it can be easier to debug.
> +/**
> * ixgbevf_stop_hw_vf - Generic stop Tx/Rx units
> * @hw: pointer to hardware structure
> *
> @@ -656,6 +680,68 @@ out:
> }
>
> /**
> + * Hyper-V variant; there is no mailbox communication.
> + */
> +static s32 ixgbevf_hv_check_mac_link_vf(struct ixgbe_hw *hw,
> + ixgbe_link_speed *speed,
> + bool *link_up,
> + bool autoneg_wait_to_complete)
> +{
> + struct ixgbe_mbx_info *mbx = &hw->mbx;
> + struct ixgbe_mac_info *mac = &hw->mac;
> + s32 ret_val = 0;
> + u32 links_reg;
> +
> + /* If we were hit with a reset drop the link */
> + if (!mbx->ops.check_for_rst(hw) || !mbx->timeout)
> + mac->get_link_status = true;
> +
> + if (!mac->get_link_status)
> + goto out;
> +
> + /* if link status is down no point in checking to see if pf is up */
> + links_reg = IXGBE_READ_REG(hw, IXGBE_VFLINKS);
> + if (!(links_reg & IXGBE_LINKS_UP))
> + goto out;
> +
> + /* for SFP+ modules and DA cables on 82599 it can take up to 500usecs
> + * before the link status is correct
> + */
> + if (mac->type == ixgbe_mac_82599_vf) {
> + int i;
> +
> + for (i = 0; i < 5; i++) {
> + udelay(100);
> + links_reg = IXGBE_READ_REG(hw, IXGBE_VFLINKS);
> +
> + if (!(links_reg & IXGBE_LINKS_UP))
> + goto out;
> + }
> + }
> +
> + switch (links_reg & IXGBE_LINKS_SPEED_82599) {
> + case IXGBE_LINKS_SPEED_10G_82599:
> + *speed = IXGBE_LINK_SPEED_10GB_FULL;
> + break;
> + case IXGBE_LINKS_SPEED_1G_82599:
> + *speed = IXGBE_LINK_SPEED_1GB_FULL;
> + break;
> + case IXGBE_LINKS_SPEED_100_82599:
> + *speed = IXGBE_LINK_SPEED_100_FULL;
> + break;
> + }
> +
> + /* if we passed all the tests above then the link is up and we no
> + * longer need to check for link
> + */
> + mac->get_link_status = false;
> +
> +out:
> + *link_up = !mac->get_link_status;
> + return ret_val;
> +}
> +
How does this handle the PF resetting? Seems like you are going to be
generating Tx hangs in such a case.
> +/**
> * ixgbevf_rlpml_set_vf - Set the maximum receive packet length
> * @hw: pointer to the HW structure
> * @max_size: value to assign to max frame size
> @@ -663,6 +749,19 @@ out:
> void ixgbevf_rlpml_set_vf(struct ixgbe_hw *hw, u16 max_size)
> {
> u32 msgbuf[2];
> + u32 reg;
> +
> + if (x86_hyper == &x86_hyper_ms_hyperv) {
> + /*
> + * If we are on Hyper-V, we implement
> + * this functionality differently.
> + */
> + reg = IXGBE_READ_REG(hw, IXGBE_VFRXDCTL(0));
> + /* CRC == 4 */
> + reg |= ((max_size + 4) | IXGBE_RXDCTL_RLPML_EN);
> + IXGBE_WRITE_REG(hw, IXGBE_VFRXDCTL(0), reg);
> + return;
> + }
>
> msgbuf[0] = IXGBE_VF_SET_LPE;
> msgbuf[1] = max_size;
You would be better off just implementing a hyperv version of this
function and avoiding the mailbox call entirely.
> @@ -679,6 +778,16 @@ int ixgbevf_negotiate_api_version(struct ixgbe_hw *hw, int api)
> int err;
> u32 msg[3];
>
> + if (x86_hyper == &x86_hyper_ms_hyperv) {
> + /*
> + * Hyper-V only supports api version ixgbe_mbox_api_10
> + */
> + if (api != ixgbe_mbox_api_10)
> + return IXGBE_ERR_INVALID_ARGUMENT;
> +
> + return 0;
> + }
> +
> /* Negotiate the mailbox API version */
> msg[0] = IXGBE_VF_API_NEGOTIATE;
> msg[1] = api;
Same here. Just implement a hyperv version of this function instead
of splicing into the existing call. Also you will need to wrap this
code up so that we can build on all architectures, not just x86.
> @@ -776,22 +885,51 @@ static const struct ixgbe_mac_operations ixgbevf_mac_ops = {
> .set_vfta = ixgbevf_set_vfta_vf,
> };
>
> +static const struct ixgbe_mac_operations ixgbevf_hv_mac_ops = {
> + .init_hw = ixgbevf_init_hw_vf,
> + .reset_hw = ixgbevf_hv_reset_hw_vf,
> + .start_hw = ixgbevf_start_hw_vf,
> + .get_mac_addr = ixgbevf_get_mac_addr_vf,
> + .stop_adapter = ixgbevf_stop_hw_vf,
> + .setup_link = ixgbevf_setup_mac_link_vf,
> + .check_link = ixgbevf_hv_check_mac_link_vf,
> +};
You might want to consider implementing a set_rar call that will
return an error if you try to change the address to anything that is
not the permanent addr.
> const struct ixgbevf_info ixgbevf_82599_vf_info = {
> .mac = ixgbe_mac_82599_vf,
> .mac_ops = &ixgbevf_mac_ops,
> };
>
> +const struct ixgbevf_info ixgbevf_82599_vf_hv_info = {
> + .mac = ixgbe_mac_82599_vf,
> + .mac_ops = &ixgbevf_hv_mac_ops,
> +};
> +
> const struct ixgbevf_info ixgbevf_X540_vf_info = {
> .mac = ixgbe_mac_X540_vf,
> .mac_ops = &ixgbevf_mac_ops,
> };
>
> +const struct ixgbevf_info ixgbevf_X540_vf_hv_info = {
> + .mac = ixgbe_mac_X540_vf,
> + .mac_ops = &ixgbevf_hv_mac_ops,
> +};
> +
> const struct ixgbevf_info ixgbevf_X550_vf_info = {
> .mac = ixgbe_mac_X550_vf,
> .mac_ops = &ixgbevf_mac_ops,
> };
>
> +const struct ixgbevf_info ixgbevf_X550_vf_hv_info = {
> + .mac = ixgbe_mac_X550_vf,
> + .mac_ops = &ixgbevf_hv_mac_ops,
> +};
> +
> const struct ixgbevf_info ixgbevf_X550EM_x_vf_info = {
> .mac = ixgbe_mac_X550EM_x_vf,
> .mac_ops = &ixgbevf_mac_ops,
> };
> +
> +const struct ixgbevf_info ixgbevf_X550EM_x_vf_hv_info = {
> + .mac = ixgbe_mac_X550EM_x_vf,
> + .mac_ops = &ixgbevf_hv_mac_ops,
> +};
> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.h b/drivers/net/ethernet/intel/ixgbevf/vf.h
> index ef9f773..7242a97 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/vf.h
> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.h
> @@ -32,7 +32,9 @@
> #include <linux/interrupt.h>
> #include <linux/if_ether.h>
> #include <linux/netdevice.h>
> +#include <asm/hypervisor.h>
>
> +#include <asm/hypervisor.h>
I don't think you need to include this twice. Also this is a arch
specific header file. You might want to move this to vf.c since that
is where you are using the code it provides. Then you can probably
wrap it in an X86 build check so that you don't break the build on
other architectures.
> #include "defines.h"
> #include "regs.h"
> #include "mbx.h"
> --
> 1.7.4.1
>
^ permalink raw reply
* Re: [PATCH iproute2] ip: neigh: Fix leftover attributes message during flush
From: David Ahern @ 2016-04-14 23:04 UTC (permalink / raw)
To: Jeff Harris, netdev
In-Reply-To: <1460657703-8222-1-git-send-email-jefftharris@gmail.com>
On 4/14/16 12:15 PM, Jeff Harris wrote:
> Use the same rtnl_dump_request_n call as the show. The rtnl_wilddump_request
> assumes the type uses an ifinfomsg which is not the case for the neighbor
> table.
>
> Signed-off-by: Jeff Harris <jefftharris@gmail.com>
> ---
Acked-by: David Ahern <dsa@cumulusnetworks.com>
Fixes the "netlink: 12 bytes leftover after parsing attributes in
process `ip'." message.
^ permalink raw reply
* Re: [PATCH net-next 2/2] intel: ixgbevf: Support Windows hosts (Hyper-V)
From: Rustad, Mark D @ 2016-04-14 23:01 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: olaf@aepfle.de, intel-wired-lan@linuxonhyperv.com, netdev,
jasowang@redhat.com, linux-kernel@vger.kernel.org,
jackm@mellanox.com, yevgenyp@mellanox.com, Ronciak, John,
eli@mellanox.com, apw@canonical.com, devel@linuxdriverproject.org,
David Miller
In-Reply-To: <1460678142-30353-2-git-send-email-kys@microsoft.com>
[-- Attachment #1.1: Type: text/plain, Size: 6949 bytes --]
Some comments below:
K. Y. Srinivasan <kys@microsoft.com> wrote:
> On Hyper-V, the VF/PF communication is a via software mediated path
> as opposed to the hardware mailbox. Make the necessary
> adjustments to support Hyper-V.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 11 ++
> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 56 ++++++---
> drivers/net/ethernet/intel/ixgbevf/mbx.c | 12 ++
> drivers/net/ethernet/intel/ixgbevf/vf.c | 138 +++++++++++++++++++++
> drivers/net/ethernet/intel/ixgbevf/vf.h | 2 +
> 5 files changed, 201 insertions(+), 18 deletions(-)
<snip>
> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c
> b/drivers/net/ethernet/intel/ixgbevf/vf.c
> index 4d613a4..92397fd 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/vf.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.c
<snip>
> @@ -126,6 +133,23 @@ static s32 ixgbevf_reset_hw_vf(struct ixgbe_hw *hw)
> }
>
> /**
> + * Hyper-V variant; the VF/PF communication is through the PCI
> + * config space.
> + */
> +static s32 ixgbevf_hv_reset_hw_vf(struct ixgbe_hw *hw)
> +{
> + int i;
> + struct ixgbevf_adapter *adapter = hw->back;
The two lines above should be swapped so that the longer one is first.
> +
> + for (i = 0; i < 6; i++)
> + pci_read_config_byte(adapter->pdev,
> + (i + IXGBE_HV_RESET_OFFSET),
> + &hw->mac.perm_addr[i]);
> +
> + return 0;
> +}
> +
> +/**
> * ixgbevf_stop_hw_vf - Generic stop Tx/Rx units
> * @hw: pointer to hardware structure
> *
> @@ -656,6 +680,68 @@ out:
> }
>
> /**
> + * Hyper-V variant; there is no mailbox communication.
> + */
> +static s32 ixgbevf_hv_check_mac_link_vf(struct ixgbe_hw *hw,
> + ixgbe_link_speed *speed,
> + bool *link_up,
> + bool autoneg_wait_to_complete)
> +{
> + struct ixgbe_mbx_info *mbx = &hw->mbx;
> + struct ixgbe_mac_info *mac = &hw->mac;
> + s32 ret_val = 0;
ret_val here is redundant as this is the only assignment to it.
Please delete it and just return 0 at the end.
> + u32 links_reg;
> +
> + /* If we were hit with a reset drop the link */
> + if (!mbx->ops.check_for_rst(hw) || !mbx->timeout)
> + mac->get_link_status = true;
> +
> + if (!mac->get_link_status)
> + goto out;
> +
> + /* if link status is down no point in checking to see if pf is up */
> + links_reg = IXGBE_READ_REG(hw, IXGBE_VFLINKS);
> + if (!(links_reg & IXGBE_LINKS_UP))
> + goto out;
> +
> + /* for SFP+ modules and DA cables on 82599 it can take up to 500usecs
> + * before the link status is correct
> + */
> + if (mac->type == ixgbe_mac_82599_vf) {
> + int i;
> +
> + for (i = 0; i < 5; i++) {
> + udelay(100);
> + links_reg = IXGBE_READ_REG(hw, IXGBE_VFLINKS);
> +
> + if (!(links_reg & IXGBE_LINKS_UP))
> + goto out;
> + }
> + }
> +
> + switch (links_reg & IXGBE_LINKS_SPEED_82599) {
> + case IXGBE_LINKS_SPEED_10G_82599:
> + *speed = IXGBE_LINK_SPEED_10GB_FULL;
> + break;
> + case IXGBE_LINKS_SPEED_1G_82599:
> + *speed = IXGBE_LINK_SPEED_1GB_FULL;
> + break;
> + case IXGBE_LINKS_SPEED_100_82599:
> + *speed = IXGBE_LINK_SPEED_100_FULL;
> + break;
> + }
> +
> + /* if we passed all the tests above then the link is up and we no
> + * longer need to check for link
> + */
> + mac->get_link_status = false;
> +
> +out:
> + *link_up = !mac->get_link_status;
> + return ret_val;
Just return 0 above.
> +}
> +
> +/**
> * ixgbevf_rlpml_set_vf - Set the maximum receive packet length
> * @hw: pointer to the HW structure
> * @max_size: value to assign to max frame size
<snip>
> @@ -663,6 +749,19 @@ out:
> void ixgbevf_rlpml_set_vf(struct ixgbe_hw *hw, u16 max_size)
> {
> u32 msgbuf[2];
> + u32 reg;
> +
> + if (x86_hyper == &x86_hyper_ms_hyperv) {
> + /*
> + * If we are on Hyper-V, we implement
Please format the comment above netdev-style, /* If we are...
as you did elsewhere.
> + * this functionality differently.
> + */
> + reg = IXGBE_READ_REG(hw, IXGBE_VFRXDCTL(0));
> + /* CRC == 4 */
> + reg |= ((max_size + 4) | IXGBE_RXDCTL_RLPML_EN);
> + IXGBE_WRITE_REG(hw, IXGBE_VFRXDCTL(0), reg);
> + return;
> + }
>
> msgbuf[0] = IXGBE_VF_SET_LPE;
> msgbuf[1] = max_size;
> @@ -679,6 +778,16 @@ int ixgbevf_negotiate_api_version(struct ixgbe_hw
> *hw, int api)
> int err;
> u32 msg[3];
>
> + if (x86_hyper == &x86_hyper_ms_hyperv) {
> + /*
> + * Hyper-V only supports api version ixgbe_mbox_api_10
Again, please use netdev-style comment above.
> + */
> + if (api != ixgbe_mbox_api_10)
> + return IXGBE_ERR_INVALID_ARGUMENT;
> +
> + return 0;
> + }
> +
> /* Negotiate the mailbox API version */
> msg[0] = IXGBE_VF_API_NEGOTIATE;
> msg[1] = api;
> @@ -776,22 +885,51 @@ static const struct ixgbe_mac_operations
> ixgbevf_mac_ops = {
> .set_vfta = ixgbevf_set_vfta_vf,
> };
>
> +static const struct ixgbe_mac_operations ixgbevf_hv_mac_ops = {
> + .init_hw = ixgbevf_init_hw_vf,
> + .reset_hw = ixgbevf_hv_reset_hw_vf,
> + .start_hw = ixgbevf_start_hw_vf,
> + .get_mac_addr = ixgbevf_get_mac_addr_vf,
> + .stop_adapter = ixgbevf_stop_hw_vf,
> + .setup_link = ixgbevf_setup_mac_link_vf,
> + .check_link = ixgbevf_hv_check_mac_link_vf,
> +};
Please add a blank line between these two structures as you have
done elsewhere.
> const struct ixgbevf_info ixgbevf_82599_vf_info = {
> .mac = ixgbe_mac_82599_vf,
> .mac_ops = &ixgbevf_mac_ops,
> };
>
> +const struct ixgbevf_info ixgbevf_82599_vf_hv_info = {
> + .mac = ixgbe_mac_82599_vf,
> + .mac_ops = &ixgbevf_hv_mac_ops,
> +};
> +
> const struct ixgbevf_info ixgbevf_X540_vf_info = {
> .mac = ixgbe_mac_X540_vf,
> .mac_ops = &ixgbevf_mac_ops,
> };
>
> +const struct ixgbevf_info ixgbevf_X540_vf_hv_info = {
> + .mac = ixgbe_mac_X540_vf,
> + .mac_ops = &ixgbevf_hv_mac_ops,
> +};
> +
> const struct ixgbevf_info ixgbevf_X550_vf_info = {
> .mac = ixgbe_mac_X550_vf,
> .mac_ops = &ixgbevf_mac_ops,
> };
>
> +const struct ixgbevf_info ixgbevf_X550_vf_hv_info = {
> + .mac = ixgbe_mac_X550_vf,
> + .mac_ops = &ixgbevf_hv_mac_ops,
> +};
> +
> const struct ixgbevf_info ixgbevf_X550EM_x_vf_info = {
> .mac = ixgbe_mac_X550EM_x_vf,
> .mac_ops = &ixgbevf_mac_ops,
> };
> +
> +const struct ixgbevf_info ixgbevf_X550EM_x_vf_hv_info = {
> + .mac = ixgbe_mac_X550EM_x_vf,
> + .mac_ops = &ixgbevf_hv_mac_ops,
> +};
> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.h
> b/drivers/net/ethernet/intel/ixgbevf/vf.h
> index ef9f773..7242a97 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/vf.h
> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.h
> @@ -32,7 +32,9 @@
> #include <linux/interrupt.h>
> #include <linux/if_ether.h>
> #include <linux/netdevice.h>
> +#include <asm/hypervisor.h>
>
> +#include <asm/hypervisor.h>
Surely you didn't mean to include the same file twice!
> #include "defines.h"
> #include "regs.h"
> #include "mbx.h"
--
Mark Rustad, Networking Division, Intel Corporation
[-- Attachment #1.2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 841 bytes --]
[-- Attachment #2: Type: text/plain, Size: 169 bytes --]
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply
* Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed
From: Paul Moore @ 2016-04-14 22:53 UTC (permalink / raw)
To: Paolo Abeni, Casey Schaufler
Cc: Florian Westphal, linux-security-module, David S. Miller,
James Morris, Andreas Gruenbacher, Stephen Smalley, netdev,
selinux
In-Reply-To: <1460451162.5965.16.camel@redhat.com>
On Tue, Apr 12, 2016 at 4:52 AM, Paolo Abeni <pabeni@redhat.com> wrote:
> Will be ok if we post a v2 version of this series, removing the hooks
> de-registration bits, but preserving the selinux nf-hooks and
> socket_sock_rcv_skb() on-demand/delayed registration ? Will that fit
> with the post-init read only memory usage that you are planning ?
The work Florian and and I were talking about would be limited just to
the netfilter hooks, the LSM hooks, e.g. socket_sock_rcv_skb() and
friends, would remain as they are today. What what we discussing was
defaulting to not registering the netfilter hooks until it became
necessary due to a labeled networking configuration or the
always_check_network policy capability; the registration of the
netfilter hooks would be permanent, you could not unregister the hooks
at that point, you would need to reboot. Does that make sense?
As far as Casey's concerns, I don't think the work we are talking
about for the v2 patchset would have any effect on the socket/sock
security blobs as you really can't manage those adequately from the
netfilter hooks; you most likely will reference them and perhaps even
update the data within, but not allocate or free the blobs. Besides,
even in some weird case you were alloc/free'ing security blobs in the
netfilter hooks, we can deal with that on a per-LSM basis if/when the
full fledged stacking patches are merged; everything we are talking
about is a hidden implementation detail so changing it in the future
shouldn't be a problem.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* [PATCH 2/3] phy: add generic function to support ksetting support
From: Philippe Reynes @ 2016-04-14 22:35 UTC (permalink / raw)
To: davem, decot, f.fainelli, fugang.duan
Cc: linux-kernel, netdev, Philippe Reynes
In-Reply-To: <1460673301-10599-1-git-send-email-tremyfr@gmail.com>
The old ethtool api (get_setting and set_setting) has
generic phy functions phy_ethtool_sset and phy_ethtool_gset.
To supprt the new ethtool api (get_link_ksettings and
set_link_ksettings), we add generic phy function
phy_ethtool_ksettings_get and phy_ethtool_ksettings_set.
Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
---
drivers/net/phy/phy.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/phy.h | 4 ++
2 files changed, 85 insertions(+), 0 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 5590b9c..6f221c8 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -362,6 +362,60 @@ int phy_ethtool_sset(struct phy_device *phydev, struct ethtool_cmd *cmd)
}
EXPORT_SYMBOL(phy_ethtool_sset);
+int phy_ethtool_ksettings_set(struct phy_device *phydev,
+ const struct ethtool_link_ksettings *cmd)
+{
+ u8 autoneg = cmd->base.autoneg;
+ u8 duplex = cmd->base.duplex;
+ u32 speed = cmd->base.speed;
+ u32 advertising;
+
+ if (cmd->base.phy_address != phydev->mdio.addr)
+ return -EINVAL;
+
+ ethtool_convert_link_mode_to_legacy_u32(&advertising,
+ cmd->link_modes.advertising);
+
+ /* We make sure that we don't pass unsupported values in to the PHY */
+ advertising &= phydev->supported;
+
+ /* Verify the settings we care about. */
+ if (autoneg != AUTONEG_ENABLE && autoneg != AUTONEG_DISABLE)
+ return -EINVAL;
+
+ if (autoneg == AUTONEG_ENABLE && advertising == 0)
+ return -EINVAL;
+
+ if (autoneg == AUTONEG_DISABLE &&
+ ((speed != SPEED_1000 &&
+ speed != SPEED_100 &&
+ speed != SPEED_10) ||
+ (duplex != DUPLEX_HALF &&
+ duplex != DUPLEX_FULL)))
+ return -EINVAL;
+
+ phydev->autoneg = autoneg;
+
+ phydev->speed = speed;
+
+ phydev->advertising = advertising;
+
+ if (autoneg == AUTONEG_ENABLE)
+ phydev->advertising |= ADVERTISED_Autoneg;
+ else
+ phydev->advertising &= ~ADVERTISED_Autoneg;
+
+ phydev->duplex = duplex;
+
+ phydev->mdix = cmd->base.eth_tp_mdix_ctrl;
+
+ /* Restart the PHY */
+ phy_start_aneg(phydev);
+
+ return 0;
+}
+EXPORT_SYMBOL(phy_ethtool_ksettings_set);
+
int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd)
{
cmd->supported = phydev->supported;
@@ -385,6 +439,33 @@ int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd)
}
EXPORT_SYMBOL(phy_ethtool_gset);
+int phy_ethtool_ksettings_get(struct phy_device *phydev,
+ struct ethtool_link_ksettings *cmd)
+{
+ ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.supported,
+ phydev->supported);
+
+ ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.advertising,
+ phydev->advertising);
+
+ ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.lp_advertising,
+ phydev->lp_advertising);
+
+ cmd->base.speed = phydev->speed;
+ cmd->base.duplex = phydev->duplex;
+ if (phydev->interface == PHY_INTERFACE_MODE_MOCA)
+ cmd->base.port = PORT_BNC;
+ else
+ cmd->base.port = PORT_MII;
+
+ cmd->base.phy_address = phydev->mdio.addr;
+ cmd->base.autoneg = phydev->autoneg;
+ cmd->base.eth_tp_mdix_ctrl = phydev->mdix;
+
+ return 0;
+}
+EXPORT_SYMBOL(phy_ethtool_ksettings_get);
+
/**
* phy_mii_ioctl - generic PHY MII ioctl interface
* @phydev: the phy_device struct
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 2abd791..be3f83b 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -805,6 +805,10 @@ void phy_start_machine(struct phy_device *phydev);
void phy_stop_machine(struct phy_device *phydev);
int phy_ethtool_sset(struct phy_device *phydev, struct ethtool_cmd *cmd);
int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd);
+int phy_ethtool_ksettings_get(struct phy_device *phydev,
+ struct ethtool_link_ksettings *cmd);
+int phy_ethtool_ksettings_set(struct phy_device *phydev,
+ const struct ethtool_link_ksettings *cmd);
int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd);
int phy_start_interrupts(struct phy_device *phydev);
void phy_print_status(struct phy_device *phydev);
--
1.7.4.4
^ permalink raw reply related
* [PATCH 1/3] net: ethtool: export conversion function between u32 and link mode
From: Philippe Reynes @ 2016-04-14 22:34 UTC (permalink / raw)
To: davem, decot, f.fainelli, fugang.duan
Cc: linux-kernel, netdev, Philippe Reynes
In-Reply-To: <1460673301-10599-1-git-send-email-tremyfr@gmail.com>
The function convert_legacy_u32_to_link_mode and
convert_link_mode_to_legacy_u32 may be used outside
of ethtool.c. We rename them to ethtool_convert_...
and export them, so we could use them in others
drivers and modules.
Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
---
include/linux/ethtool.h | 7 +++++++
net/core/ethtool.c | 21 ++++++++++++---------
2 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index e2b7bf2..9ded8c6 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -150,6 +150,13 @@ extern int
__ethtool_get_link_ksettings(struct net_device *dev,
struct ethtool_link_ksettings *link_ksettings);
+void ethtool_convert_legacy_u32_to_link_mode(unsigned long *dst,
+ u32 legacy_u32);
+
+/* return false if src had higher bits set. lower bits always updated. */
+bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32,
+ const unsigned long *src);
+
/**
* struct ethtool_ops - optional netdev operations
* @get_settings: DEPRECATED, use %get_link_ksettings/%set_link_ksettings
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index f426c5a..81e3082 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -387,15 +387,17 @@ static int __ethtool_set_flags(struct net_device *dev, u32 data)
return 0;
}
-static void convert_legacy_u32_to_link_mode(unsigned long *dst, u32 legacy_u32)
+void ethtool_convert_legacy_u32_to_link_mode(unsigned long *dst,
+ u32 legacy_u32)
{
bitmap_zero(dst, __ETHTOOL_LINK_MODE_MASK_NBITS);
dst[0] = legacy_u32;
}
+EXPORT_SYMBOL(ethtool_convert_legacy_u32_to_link_mode);
/* return false if src had higher bits set. lower bits always updated. */
-static bool convert_link_mode_to_legacy_u32(u32 *legacy_u32,
- const unsigned long *src)
+bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32,
+ const unsigned long *src)
{
bool retval = true;
@@ -415,6 +417,7 @@ static bool convert_link_mode_to_legacy_u32(u32 *legacy_u32,
*legacy_u32 = src[0];
return retval;
}
+EXPORT_SYMBOL(ethtool_convert_link_mode_to_legacy_u32);
/* return false if legacy contained non-0 deprecated fields
* transceiver/maxtxpkt/maxrxpkt. rest of ksettings always updated
@@ -437,13 +440,13 @@ convert_legacy_settings_to_link_ksettings(
legacy_settings->maxrxpkt)
retval = false;
- convert_legacy_u32_to_link_mode(
+ ethtool_convert_legacy_u32_to_link_mode(
link_ksettings->link_modes.supported,
legacy_settings->supported);
- convert_legacy_u32_to_link_mode(
+ ethtool_convert_legacy_u32_to_link_mode(
link_ksettings->link_modes.advertising,
legacy_settings->advertising);
- convert_legacy_u32_to_link_mode(
+ ethtool_convert_legacy_u32_to_link_mode(
link_ksettings->link_modes.lp_advertising,
legacy_settings->lp_advertising);
link_ksettings->base.speed
@@ -482,13 +485,13 @@ convert_link_ksettings_to_legacy_settings(
* __u32 maxrxpkt;
*/
- retval &= convert_link_mode_to_legacy_u32(
+ retval &= ethtool_convert_link_mode_to_legacy_u32(
&legacy_settings->supported,
link_ksettings->link_modes.supported);
- retval &= convert_link_mode_to_legacy_u32(
+ retval &= ethtool_convert_link_mode_to_legacy_u32(
&legacy_settings->advertising,
link_ksettings->link_modes.advertising);
- retval &= convert_link_mode_to_legacy_u32(
+ retval &= ethtool_convert_link_mode_to_legacy_u32(
&legacy_settings->lp_advertising,
link_ksettings->link_modes.lp_advertising);
ethtool_cmd_speed_set(legacy_settings, link_ksettings->base.speed);
--
1.7.4.4
^ permalink raw reply related
* [PATCH 3/3] fec: move to new ethtool api {get|set}_link_ksettings
From: Philippe Reynes @ 2016-04-14 22:35 UTC (permalink / raw)
To: davem, decot, f.fainelli, fugang.duan
Cc: linux-kernel, netdev, Philippe Reynes
In-Reply-To: <1460673301-10599-1-git-send-email-tremyfr@gmail.com>
The ethtool api {get|set}_settings is deprecated.
We move the fec driver to new api {get|set}_link_ksettings.
Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
---
drivers/net/ethernet/freescale/fec_main.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 08243c2..bfa10c3 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2058,8 +2058,8 @@ static void fec_enet_mii_remove(struct fec_enet_private *fep)
}
}
-static int fec_enet_get_settings(struct net_device *ndev,
- struct ethtool_cmd *cmd)
+static int fec_enet_get_link_ksettings(struct net_device *ndev,
+ struct ethtool_link_ksettings *cmd)
{
struct fec_enet_private *fep = netdev_priv(ndev);
struct phy_device *phydev = fep->phy_dev;
@@ -2067,11 +2067,11 @@ static int fec_enet_get_settings(struct net_device *ndev,
if (!phydev)
return -ENODEV;
- return phy_ethtool_gset(phydev, cmd);
+ return phy_ethtool_ksettings_get(phydev, cmd);
}
-static int fec_enet_set_settings(struct net_device *ndev,
- struct ethtool_cmd *cmd)
+static int fec_enet_set_link_ksettings(struct net_device *ndev,
+ const struct ethtool_link_ksettings *cmd)
{
struct fec_enet_private *fep = netdev_priv(ndev);
struct phy_device *phydev = fep->phy_dev;
@@ -2079,7 +2079,7 @@ static int fec_enet_set_settings(struct net_device *ndev,
if (!phydev)
return -ENODEV;
- return phy_ethtool_sset(phydev, cmd);
+ return phy_ethtool_ksettings_set(phydev, cmd);
}
static void fec_enet_get_drvinfo(struct net_device *ndev,
@@ -2562,8 +2562,6 @@ fec_enet_set_wol(struct net_device *ndev, struct ethtool_wolinfo *wol)
}
static const struct ethtool_ops fec_enet_ethtool_ops = {
- .get_settings = fec_enet_get_settings,
- .set_settings = fec_enet_set_settings,
.get_drvinfo = fec_enet_get_drvinfo,
.get_regs_len = fec_enet_get_regs_len,
.get_regs = fec_enet_get_regs,
@@ -2583,6 +2581,8 @@ static const struct ethtool_ops fec_enet_ethtool_ops = {
.set_tunable = fec_enet_set_tunable,
.get_wol = fec_enet_get_wol,
.set_wol = fec_enet_set_wol,
+ .get_link_ksettings = fec_enet_get_link_ksettings,
+ .set_link_ksettings = fec_enet_set_link_ksettings,
};
static int fec_enet_ioctl(struct net_device *ndev, struct ifreq *rq, int cmd)
--
1.7.4.4
^ permalink raw reply related
* [PATCH 0/3] fec: ethtool: move to new api {get|set}_link_ksettings
From: Philippe Reynes @ 2016-04-14 22:34 UTC (permalink / raw)
To: davem, decot, f.fainelli, fugang.duan
Cc: linux-kernel, netdev, Philippe Reynes
Ethtool has a new api {get|set}_link_ksettings that deprecate
the old api {get|set}_settings. We update the fec driver to use
this new ethtool api.
For this first version, I've converted old u32 value in phy structure
to link_modes structure. Another way would be to replace u32 in
phy structure to use DECLARE_LINK_MODE_MASK for advertising, ....
Philippe Reynes (3):
net: ethtool: export conversion function between u32 and link mode
phy: add generic function to support ksetting support
fec: move to new ethtool api {get|set}_link_ksettings
drivers/net/ethernet/freescale/fec_main.c | 16 +++---
drivers/net/phy/phy.c | 81 +++++++++++++++++++++++++++++
include/linux/ethtool.h | 7 +++
include/linux/phy.h | 4 ++
net/core/ethtool.c | 21 ++++---
5 files changed, 112 insertions(+), 17 deletions(-)
--
1.7.4.4
^ permalink raw reply
* Re: [PATCH net-next 2/2] intel: ixgbevf: Support Windows hosts (Hyper-V)
From: kbuild test robot @ 2016-04-14 22:34 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: kbuild-all, davem, netdev, linux-kernel, devel, olaf, apw,
jasowang, eli, jackm, yevgenyp, john.ronciak, intel-wired-lan
In-Reply-To: <1460678142-30353-2-git-send-email-kys@microsoft.com>
[-- Attachment #1: Type: text/plain, Size: 2086 bytes --]
Hi Srinivasan,
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/K-Y-Srinivasan/ethernet-intel-Add-the-device-ID-s-presented-while-running-on-Hyper-V/20160415-061821
config: sparc64-allyesconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64
All errors (new ones prefixed by >>):
drivers/net/ethernet/intel/ixgbevf/vf.c: In function 'ixgbevf_rlpml_set_vf':
>> drivers/net/ethernet/intel/ixgbevf/vf.c:754:6: error: 'x86_hyper' undeclared (first use in this function)
if (x86_hyper == &x86_hyper_ms_hyperv) {
^
drivers/net/ethernet/intel/ixgbevf/vf.c:754:6: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/net/ethernet/intel/ixgbevf/vf.c:754:20: error: 'x86_hyper_ms_hyperv' undeclared (first use in this function)
if (x86_hyper == &x86_hyper_ms_hyperv) {
^
drivers/net/ethernet/intel/ixgbevf/vf.c: In function 'ixgbevf_negotiate_api_version':
drivers/net/ethernet/intel/ixgbevf/vf.c:781:6: error: 'x86_hyper' undeclared (first use in this function)
if (x86_hyper == &x86_hyper_ms_hyperv) {
^
drivers/net/ethernet/intel/ixgbevf/vf.c:781:20: error: 'x86_hyper_ms_hyperv' undeclared (first use in this function)
if (x86_hyper == &x86_hyper_ms_hyperv) {
^
vim +/x86_hyper +754 drivers/net/ethernet/intel/ixgbevf/vf.c
748 **/
749 void ixgbevf_rlpml_set_vf(struct ixgbe_hw *hw, u16 max_size)
750 {
751 u32 msgbuf[2];
752 u32 reg;
753
> 754 if (x86_hyper == &x86_hyper_ms_hyperv) {
755 /*
756 * If we are on Hyper-V, we implement
757 * this functionality differently.
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 45930 bytes --]
^ permalink raw reply
* Re: [PATCH] dsa: mv88e6xxx: Kill the REG_READ and REG_WRITE macros
From: Vivien Didelot @ 2016-04-14 22:16 UTC (permalink / raw)
To: Andrew Lunn, David Miller; +Cc: Florian Fainelli, netdev, Andrew Lunn
In-Reply-To: <1460670432-23801-1-git-send-email-andrew@lunn.ch>
Andrew Lunn <andrew@lunn.ch> writes:
> These macros hide a ds variable and a return statement on error, which
> can lead to locking issues. Kill them off.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>
> As requested by Vivien, this patch has been split out of the series.
>
> v2: Use the existing ret variable
> Hold the lock for the whole function, unlock on exit.
> Removed duplicate error test
> Fixed indentation.
> ---
> drivers/net/dsa/mv88e6123.c | 13 ++-
> drivers/net/dsa/mv88e6131.c | 41 ++++----
> drivers/net/dsa/mv88e6171.c | 16 +--
> drivers/net/dsa/mv88e6352.c | 15 +--
> drivers/net/dsa/mv88e6xxx.c | 241 +++++++++++++++++++++++++++++++-------------
> drivers/net/dsa/mv88e6xxx.h | 21 ----
> 6 files changed, 224 insertions(+), 123 deletions(-)
>
> diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
> index c34283d929c4..140e44e50e8a 100644
> --- a/drivers/net/dsa/mv88e6123.c
> +++ b/drivers/net/dsa/mv88e6123.c
> @@ -52,7 +52,9 @@ static int mv88e6123_setup_global(struct dsa_switch *ds)
> * external PHYs to poll), don't discard packets with
> * excessive collisions, and mask all interrupt sources.
> */
> - REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL, 0x0000);
> + ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL, 0x0000);
> + if (ret)
> + return ret;
>
> /* Configure the upstream port, and configure the upstream
> * port as the port to which ingress and egress monitor frames
> @@ -61,14 +63,15 @@ static int mv88e6123_setup_global(struct dsa_switch *ds)
> reg = upstream_port << GLOBAL_MONITOR_CONTROL_INGRESS_SHIFT |
> upstream_port << GLOBAL_MONITOR_CONTROL_EGRESS_SHIFT |
> upstream_port << GLOBAL_MONITOR_CONTROL_ARP_SHIFT;
> - REG_WRITE(REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
> + ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
> + if (ret)
> + return ret;
>
> /* Disable remote management for now, and set the switch's
> * DSA device number.
> */
> - REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL_2, ds->index & 0x1f);
> -
> - return 0;
> + return mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL_2,
> + ds->index & 0x1f);
> }
>
> static int mv88e6123_setup(struct dsa_switch *ds)
> diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
> index f5d75fce1e96..34d297b65040 100644
> --- a/drivers/net/dsa/mv88e6131.c
> +++ b/drivers/net/dsa/mv88e6131.c
> @@ -49,11 +49,16 @@ static int mv88e6131_setup_global(struct dsa_switch *ds)
> * to arbitrate between packet queues, set the maximum frame
> * size to 1632, and mask all interrupt sources.
> */
> - REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL,
> - GLOBAL_CONTROL_PPU_ENABLE | GLOBAL_CONTROL_MAX_FRAME_1632);
> + ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL,
> + GLOBAL_CONTROL_PPU_ENABLE |
> + GLOBAL_CONTROL_MAX_FRAME_1632);
> + if (ret)
> + return ret;
>
> /* Set the VLAN ethertype to 0x8100. */
> - REG_WRITE(REG_GLOBAL, GLOBAL_CORE_TAG_TYPE, 0x8100);
> + ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CORE_TAG_TYPE, 0x8100);
> + if (ret)
> + return ret;
>
> /* Disable ARP mirroring, and configure the upstream port as
> * the port to which ingress and egress monitor frames are to
> @@ -62,31 +67,33 @@ static int mv88e6131_setup_global(struct dsa_switch *ds)
> reg = upstream_port << GLOBAL_MONITOR_CONTROL_INGRESS_SHIFT |
> upstream_port << GLOBAL_MONITOR_CONTROL_EGRESS_SHIFT |
> GLOBAL_MONITOR_CONTROL_ARP_DISABLED;
> - REG_WRITE(REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
> + ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
> + if (ret)
> + return ret;
>
> /* Disable cascade port functionality unless this device
> * is used in a cascade configuration, and set the switch's
> * DSA device number.
> */
> if (ds->dst->pd->nr_chips > 1)
> - REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL_2,
> - GLOBAL_CONTROL_2_MULTIPLE_CASCADE |
> - (ds->index & 0x1f));
> + ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL_2,
> + GLOBAL_CONTROL_2_MULTIPLE_CASCADE |
> + (ds->index & 0x1f));
> else
> - REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL_2,
> - GLOBAL_CONTROL_2_NO_CASCADE |
> - (ds->index & 0x1f));
> + ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL_2,
> + GLOBAL_CONTROL_2_NO_CASCADE |
> + (ds->index & 0x1f));
> + if (ret)
> + return ret;
>
> /* Force the priority of IGMP/MLD snoop frames and ARP frames
> * to the highest setting.
> */
> - REG_WRITE(REG_GLOBAL2, GLOBAL2_PRIO_OVERRIDE,
> - GLOBAL2_PRIO_OVERRIDE_FORCE_SNOOP |
> - 7 << GLOBAL2_PRIO_OVERRIDE_SNOOP_SHIFT |
> - GLOBAL2_PRIO_OVERRIDE_FORCE_ARP |
> - 7 << GLOBAL2_PRIO_OVERRIDE_ARP_SHIFT);
> -
> - return 0;
> + return mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_PRIO_OVERRIDE,
> + GLOBAL2_PRIO_OVERRIDE_FORCE_SNOOP |
> + 7 << GLOBAL2_PRIO_OVERRIDE_SNOOP_SHIFT |
> + GLOBAL2_PRIO_OVERRIDE_FORCE_ARP |
> + 7 << GLOBAL2_PRIO_OVERRIDE_ARP_SHIFT);
> }
>
> static int mv88e6131_setup(struct dsa_switch *ds)
> diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
> index f5622506cdfa..b7af2b78f8ee 100644
> --- a/drivers/net/dsa/mv88e6171.c
> +++ b/drivers/net/dsa/mv88e6171.c
> @@ -46,8 +46,11 @@ static int mv88e6171_setup_global(struct dsa_switch *ds)
> /* Discard packets with excessive collisions, mask all
> * interrupt sources, enable PPU.
> */
> - REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL,
> - GLOBAL_CONTROL_PPU_ENABLE | GLOBAL_CONTROL_DISCARD_EXCESS);
> + ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL,
> + GLOBAL_CONTROL_PPU_ENABLE |
> + GLOBAL_CONTROL_DISCARD_EXCESS);
> + if (ret)
> + return ret;
>
> /* Configure the upstream port, and configure the upstream
> * port as the port to which ingress and egress monitor frames
> @@ -57,14 +60,15 @@ static int mv88e6171_setup_global(struct dsa_switch *ds)
> upstream_port << GLOBAL_MONITOR_CONTROL_EGRESS_SHIFT |
> upstream_port << GLOBAL_MONITOR_CONTROL_ARP_SHIFT |
> upstream_port << GLOBAL_MONITOR_CONTROL_MIRROR_SHIFT;
> - REG_WRITE(REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
> + ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
> + if (ret)
> + return ret;
>
> /* Disable remote management for now, and set the switch's
> * DSA device number.
> */
> - REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL_2, ds->index & 0x1f);
> -
> - return 0;
> + return mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL_2,
> + ds->index & 0x1f);
> }
>
> static int mv88e6171_setup(struct dsa_switch *ds)
> diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
> index e54ee27db129..e8cb03fad21a 100644
> --- a/drivers/net/dsa/mv88e6352.c
> +++ b/drivers/net/dsa/mv88e6352.c
> @@ -59,8 +59,11 @@ static int mv88e6352_setup_global(struct dsa_switch *ds)
> /* Discard packets with excessive collisions,
> * mask all interrupt sources, enable PPU (bit 14, undocumented).
> */
> - REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL,
> - GLOBAL_CONTROL_PPU_ENABLE | GLOBAL_CONTROL_DISCARD_EXCESS);
> + ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL,
> + GLOBAL_CONTROL_PPU_ENABLE |
> + GLOBAL_CONTROL_DISCARD_EXCESS);
> + if (ret)
> + return ret;
>
> /* Configure the upstream port, and configure the upstream
> * port as the port to which ingress and egress monitor frames
> @@ -69,14 +72,14 @@ static int mv88e6352_setup_global(struct dsa_switch *ds)
> reg = upstream_port << GLOBAL_MONITOR_CONTROL_INGRESS_SHIFT |
> upstream_port << GLOBAL_MONITOR_CONTROL_EGRESS_SHIFT |
> upstream_port << GLOBAL_MONITOR_CONTROL_ARP_SHIFT;
> - REG_WRITE(REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
> + ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
> + if (ret)
> + return ret;
>
> /* Disable remote management for now, and set the switch's
> * DSA device number.
> */
> - REG_WRITE(REG_GLOBAL, 0x1c, ds->index & 0x1f);
> -
> - return 0;
> + return mv88e6xxx_reg_write(ds, REG_GLOBAL, 0x1c, ds->index & 0x1f);
> }
>
> static int mv88e6352_setup(struct dsa_switch *ds)
> diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
> index 9985a0cf31f1..b018f20829fb 100644
> --- a/drivers/net/dsa/mv88e6xxx.c
> +++ b/drivers/net/dsa/mv88e6xxx.c
> @@ -180,28 +180,44 @@ int mv88e6xxx_reg_write(struct dsa_switch *ds, int addr, int reg, u16 val)
>
> int mv88e6xxx_set_addr_direct(struct dsa_switch *ds, u8 *addr)
> {
> - REG_WRITE(REG_GLOBAL, GLOBAL_MAC_01, (addr[0] << 8) | addr[1]);
> - REG_WRITE(REG_GLOBAL, GLOBAL_MAC_23, (addr[2] << 8) | addr[3]);
> - REG_WRITE(REG_GLOBAL, GLOBAL_MAC_45, (addr[4] << 8) | addr[5]);
> + int err;
>
> - return 0;
> + err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MAC_01,
> + (addr[0] << 8) | addr[1]);
> + if (err)
> + return err;
> +
> + err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MAC_23,
> + (addr[2] << 8) | addr[3]);
> + if (err)
> + return err;
> +
> + return mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MAC_45,
> + (addr[4] << 8) | addr[5]);
> }
>
> int mv88e6xxx_set_addr_indirect(struct dsa_switch *ds, u8 *addr)
> {
> - int i;
> int ret;
> + int i;
Unnecessary.
>
> for (i = 0; i < 6; i++) {
> int j;
>
> /* Write the MAC address byte. */
> - REG_WRITE(REG_GLOBAL2, GLOBAL2_SWITCH_MAC,
> - GLOBAL2_SWITCH_MAC_BUSY | (i << 8) | addr[i]);
> + ret = mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_SWITCH_MAC,
> + GLOBAL2_SWITCH_MAC_BUSY |
> + (i << 8) | addr[i]);
> + if (ret)
> + return ret;
>
> /* Wait for the write to complete. */
> for (j = 0; j < 16; j++) {
> - ret = REG_READ(REG_GLOBAL2, GLOBAL2_SWITCH_MAC);
> + ret = mv88e6xxx_reg_read(ds, REG_GLOBAL2,
> + GLOBAL2_SWITCH_MAC);
> + if (ret < 0)
> + return ret;
> +
> if ((ret & GLOBAL2_SWITCH_MAC_BUSY) == 0)
> break;
> }
> @@ -233,13 +249,21 @@ static int mv88e6xxx_ppu_disable(struct dsa_switch *ds)
> int ret;
> unsigned long timeout;
>
> - ret = REG_READ(REG_GLOBAL, GLOBAL_CONTROL);
> - REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL,
> - ret & ~GLOBAL_CONTROL_PPU_ENABLE);
> + ret = mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_CONTROL);
> + if (ret < 0)
> + return ret;
> +
> + ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL,
> + ret & ~GLOBAL_CONTROL_PPU_ENABLE);
> + if (ret)
> + return ret;
>
> timeout = jiffies + 1 * HZ;
> while (time_before(jiffies, timeout)) {
> - ret = REG_READ(REG_GLOBAL, GLOBAL_STATUS);
> + ret = mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_STATUS);
> + if (ret < 0)
> + return ret;
> +
> usleep_range(1000, 2000);
> if ((ret & GLOBAL_STATUS_PPU_MASK) !=
> GLOBAL_STATUS_PPU_POLLING)
> @@ -251,15 +275,24 @@ static int mv88e6xxx_ppu_disable(struct dsa_switch *ds)
>
> static int mv88e6xxx_ppu_enable(struct dsa_switch *ds)
> {
> - int ret;
> + int ret, err;
> unsigned long timeout;
>
> - ret = REG_READ(REG_GLOBAL, GLOBAL_CONTROL);
> - REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL, ret | GLOBAL_CONTROL_PPU_ENABLE);
> + ret = mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_CONTROL);
> + if (ret < 0)
> + return ret;
> +
> + err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL,
> + ret | GLOBAL_CONTROL_PPU_ENABLE);
> + if (err)
> + return err;
You could've kept ret here.
>
> timeout = jiffies + 1 * HZ;
> while (time_before(jiffies, timeout)) {
> - ret = REG_READ(REG_GLOBAL, GLOBAL_STATUS);
> + ret = mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_STATUS);
> + if (ret < 0)
> + return ret;
> +
> usleep_range(1000, 2000);
> if ((ret & GLOBAL_STATUS_PPU_MASK) ==
> GLOBAL_STATUS_PPU_POLLING)
> @@ -2667,7 +2700,9 @@ int mv88e6xxx_setup_common(struct dsa_switch *ds)
> ps->ds = ds;
> mutex_init(&ps->smi_mutex);
>
> - ps->id = REG_READ(REG_PORT(0), PORT_SWITCH_ID) & 0xfff0;
> + ps->id = mv88e6xxx_reg_read(ds, REG_PORT(0), PORT_SWITCH_ID) & 0xfff0;
> + if (ps->id < 0)
> + return ps->id;
>
> INIT_WORK(&ps->bridge_work, mv88e6xxx_bridge_work);
>
> @@ -2677,42 +2712,67 @@ int mv88e6xxx_setup_common(struct dsa_switch *ds)
> int mv88e6xxx_setup_global(struct dsa_switch *ds)
> {
> struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> - int ret;
> + int err;
> int i;
>
> + mutex_lock(&ps->smi_mutex);
> /* Set the default address aging time to 5 minutes, and
> * enable address learn messages to be sent to all message
> * ports.
> */
> - REG_WRITE(REG_GLOBAL, GLOBAL_ATU_CONTROL,
> - 0x0140 | GLOBAL_ATU_CONTROL_LEARN2ALL);
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_ATU_CONTROL,
> + 0x0140 | GLOBAL_ATU_CONTROL_LEARN2ALL);
> + if (err)
> + goto unlock;
>
> /* Configure the IP ToS mapping registers. */
> - REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_0, 0x0000);
> - REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_1, 0x0000);
> - REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_2, 0x5555);
> - REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_3, 0x5555);
> - REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_4, 0xaaaa);
> - REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_5, 0xaaaa);
> - REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_6, 0xffff);
> - REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_7, 0xffff);
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_0, 0x0000);
> + if (err)
> + goto unlock;
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_1, 0x0000);
> + if (err)
> + goto unlock;
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_2, 0x5555);
> + if (err)
> + goto unlock;
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_3, 0x5555);
> + if (err)
> + goto unlock;
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_4, 0xaaaa);
> + if (err)
> + goto unlock;
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_5, 0xaaaa);
> + if (err)
> + goto unlock;
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_6, 0xffff);
> + if (err)
> + goto unlock;
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_7, 0xffff);
> + if (err)
> + goto unlock;
>
> /* Configure the IEEE 802.1p priority mapping register. */
> - REG_WRITE(REG_GLOBAL, GLOBAL_IEEE_PRI, 0xfa41);
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IEEE_PRI, 0xfa41);
> + if (err)
> + goto unlock;
>
> /* Send all frames with destination addresses matching
> * 01:80:c2:00:00:0x to the CPU port.
> */
> - REG_WRITE(REG_GLOBAL2, GLOBAL2_MGMT_EN_0X, 0xffff);
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_MGMT_EN_0X, 0xffff);
> + if (err)
> + goto unlock;
>
> /* Ignore removed tag data on doubly tagged packets, disable
> * flow control messages, force flow control priority to the
> * highest, and send all special multicast frames to the CPU
> * port at the highest priority.
> */
> - REG_WRITE(REG_GLOBAL2, GLOBAL2_SWITCH_MGMT,
> - 0x7 | GLOBAL2_SWITCH_MGMT_RSVD2CPU | 0x70 |
> - GLOBAL2_SWITCH_MGMT_FORCE_FLOW_CTRL_PRI);
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_SWITCH_MGMT,
> + 0x7 | GLOBAL2_SWITCH_MGMT_RSVD2CPU | 0x70 |
> + GLOBAL2_SWITCH_MGMT_FORCE_FLOW_CTRL_PRI);
> + if (err)
> + goto unlock;
>
> /* Program the DSA routing table. */
> for (i = 0; i < 32; i++) {
> @@ -2722,23 +2782,35 @@ int mv88e6xxx_setup_global(struct dsa_switch *ds)
> i != ds->index && i < ds->dst->pd->nr_chips)
> nexthop = ds->pd->rtable[i] & 0x1f;
>
> - REG_WRITE(REG_GLOBAL2, GLOBAL2_DEVICE_MAPPING,
> - GLOBAL2_DEVICE_MAPPING_UPDATE |
> - (i << GLOBAL2_DEVICE_MAPPING_TARGET_SHIFT) |
> - nexthop);
> + err = _mv88e6xxx_reg_write(
> + ds, REG_GLOBAL2,
> + GLOBAL2_DEVICE_MAPPING,
> + GLOBAL2_DEVICE_MAPPING_UPDATE |
> + (i << GLOBAL2_DEVICE_MAPPING_TARGET_SHIFT) | nexthop);
> + if (err)
> + goto unlock;
> }
>
> /* Clear all trunk masks. */
> - for (i = 0; i < 8; i++)
> - REG_WRITE(REG_GLOBAL2, GLOBAL2_TRUNK_MASK,
> - 0x8000 | (i << GLOBAL2_TRUNK_MASK_NUM_SHIFT) |
> - ((1 << ps->num_ports) - 1));
> + for (i = 0; i < 8; i++) {
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_TRUNK_MASK,
> + 0x8000 |
> + (i << GLOBAL2_TRUNK_MASK_NUM_SHIFT) |
> + ((1 << ps->num_ports) - 1));
> + if (err)
> + goto unlock;
> + }
>
> /* Clear all trunk mappings. */
> - for (i = 0; i < 16; i++)
> - REG_WRITE(REG_GLOBAL2, GLOBAL2_TRUNK_MAPPING,
> - GLOBAL2_TRUNK_MAPPING_UPDATE |
> - (i << GLOBAL2_TRUNK_MAPPING_ID_SHIFT));
> + for (i = 0; i < 16; i++) {
> + err = _mv88e6xxx_reg_write(
> + ds, REG_GLOBAL2,
> + GLOBAL2_TRUNK_MAPPING,
> + GLOBAL2_TRUNK_MAPPING_UPDATE |
> + (i << GLOBAL2_TRUNK_MAPPING_ID_SHIFT));
> + if (err)
> + goto unlock;
> + }
>
> if (mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds) ||
> mv88e6xxx_6165_family(ds) || mv88e6xxx_6097_family(ds) ||
> @@ -2746,17 +2818,27 @@ int mv88e6xxx_setup_global(struct dsa_switch *ds)
> /* Send all frames with destination addresses matching
> * 01:80:c2:00:00:2x to the CPU port.
> */
> - REG_WRITE(REG_GLOBAL2, GLOBAL2_MGMT_EN_2X, 0xffff);
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL2,
> + GLOBAL2_MGMT_EN_2X, 0xffff);
> + if (err)
> + goto unlock;
>
> /* Initialise cross-chip port VLAN table to reset
> * defaults.
> */
> - REG_WRITE(REG_GLOBAL2, GLOBAL2_PVT_ADDR, 0x9000);
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL2,
> + GLOBAL2_PVT_ADDR, 0x9000);
> + if (err)
> + goto unlock;
>
> /* Clear the priority override table. */
> - for (i = 0; i < 16; i++)
> - REG_WRITE(REG_GLOBAL2, GLOBAL2_PRIO_OVERRIDE,
> - 0x8000 | (i << 8));
> + for (i = 0; i < 16; i++) {
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL2,
> + GLOBAL2_PRIO_OVERRIDE,
> + 0x8000 | (i << 8));
> + if (err)
> + goto unlock;
> + }
> }
>
> if (mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds) ||
> @@ -2767,31 +2849,37 @@ int mv88e6xxx_setup_global(struct dsa_switch *ds)
> * ingress rate limit registers to their initial
> * state.
> */
> - for (i = 0; i < ps->num_ports; i++)
> - REG_WRITE(REG_GLOBAL2, GLOBAL2_INGRESS_OP,
> - 0x9000 | (i << 8));
> + for (i = 0; i < ps->num_ports; i++) {
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL2,
> + GLOBAL2_INGRESS_OP,
> + 0x9000 | (i << 8));
> + if (err)
> + goto unlock;
> + }
> }
>
> /* Clear the statistics counters for all ports */
> - REG_WRITE(REG_GLOBAL, GLOBAL_STATS_OP, GLOBAL_STATS_OP_FLUSH_ALL);
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_STATS_OP,
> + GLOBAL_STATS_OP_FLUSH_ALL);
> + if (err)
> + goto unlock;
>
> /* Wait for the flush to complete. */
> - mutex_lock(&ps->smi_mutex);
> - ret = _mv88e6xxx_stats_wait(ds);
> - if (ret < 0)
> + err = _mv88e6xxx_stats_wait(ds);
> + if (err < 0)
> goto unlock;
>
> /* Clear all ATU entries */
> - ret = _mv88e6xxx_atu_flush(ds, 0, true);
> - if (ret < 0)
> + err = _mv88e6xxx_atu_flush(ds, 0, true);
> + if (err < 0)
> goto unlock;
>
> /* Clear all the VTU and STU entries */
> - ret = _mv88e6xxx_vtu_stu_flush(ds);
> + err = _mv88e6xxx_vtu_stu_flush(ds);
> unlock:
> mutex_unlock(&ps->smi_mutex);
>
> - return ret;
> + return err;
> }
>
> int mv88e6xxx_switch_reset(struct dsa_switch *ds, bool ppu_active)
> @@ -2803,10 +2891,18 @@ int mv88e6xxx_switch_reset(struct dsa_switch *ds, bool ppu_active)
> int ret;
> int i;
>
> + mutex_lock(&ps->smi_mutex);
> +
> /* Set all ports to the disabled state. */
> for (i = 0; i < ps->num_ports; i++) {
> - ret = REG_READ(REG_PORT(i), PORT_CONTROL);
> - REG_WRITE(REG_PORT(i), PORT_CONTROL, ret & 0xfffc);
> + ret = _mv88e6xxx_reg_read(ds, REG_PORT(i), PORT_CONTROL);
> + if (ret < 0)
> + goto unlock;
> +
> + ret = _mv88e6xxx_reg_write(ds, REG_PORT(i), PORT_CONTROL,
> + ret & 0xfffc);
> + if (ret)
> + goto unlock;
> }
>
> /* Wait for transmit queues to drain. */
> @@ -2825,22 +2921,31 @@ int mv88e6xxx_switch_reset(struct dsa_switch *ds, bool ppu_active)
> * through global registers 0x18 and 0x19.
> */
> if (ppu_active)
> - REG_WRITE(REG_GLOBAL, 0x04, 0xc000);
> + ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, 0x04, 0xc000);
> else
> - REG_WRITE(REG_GLOBAL, 0x04, 0xc400);
> + ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, 0x04, 0xc400);
> + if (ret)
> + goto unlock;
>
> /* Wait up to one second for reset to complete. */
> timeout = jiffies + 1 * HZ;
> while (time_before(jiffies, timeout)) {
> - ret = REG_READ(REG_GLOBAL, 0x00);
> + ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, 0x00);
> + if (ret < 0)
> + goto unlock;
> +
> if ((ret & is_reset) == is_reset)
> break;
> usleep_range(1000, 2000);
> }
> if (time_after(jiffies, timeout))
> - return -ETIMEDOUT;
> + ret = -ETIMEDOUT;
> + else
> + ret = 0;
> +unlock:
> + mutex_unlock(&ps->smi_mutex);
>
> - return 0;
> + return ret;
> }
>
> int mv88e6xxx_phy_page_read(struct dsa_switch *ds, int port, int page, int reg)
> diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
> index 5d27decc85cb..0debb9f3cf0a 100644
> --- a/drivers/net/dsa/mv88e6xxx.h
> +++ b/drivers/net/dsa/mv88e6xxx.h
> @@ -542,25 +542,4 @@ extern struct dsa_switch_driver mv88e6123_switch_driver;
> extern struct dsa_switch_driver mv88e6352_switch_driver;
> extern struct dsa_switch_driver mv88e6171_switch_driver;
>
> -#define REG_READ(addr, reg) \
> - ({ \
> - int __ret; \
> - \
> - __ret = mv88e6xxx_reg_read(ds, addr, reg); \
> - if (__ret < 0) \
> - return __ret; \
> - __ret; \
> - })
> -
> -#define REG_WRITE(addr, reg, val) \
> - ({ \
> - int __ret; \
> - \
> - __ret = mv88e6xxx_reg_write(ds, addr, reg, val); \
> - if (__ret < 0) \
> - return __ret; \
> - })
> -
> -
> -
> #endif
> --
> 2.7.0
But the idea is here, we need these macro killed.
Tested-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Thank you,
Vivien
^ permalink raw reply
* Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver
From: Vikram Sethi @ 2016-04-14 22:00 UTC (permalink / raw)
To: Florian Fainelli, Timur Tabi, netdev, linux-kernel, devicetree,
linux-arm-msm, sdharia, Shanker Donthineni, Greg Kroah-Hartman,
cov, gavidov, Rob Herring, andrew, bjorn.andersson,
Mark Langsdorf, Jon Masters, Andy Gross, David S. Miller
In-Reply-To: <57100962.40404@gmail.com>
A couple of clarifications on the SGMII internal PHY and the DMA capability of the EMAC inline.
On 04/14/2016 04:19 PM, Florian Fainelli wrote:
> On 14/04/16 13:19, Timur Tabi wrote:
>> Florian Fainelli wrote:
>>> On 13/04/16 10:59, Timur Tabi wrote:
>>>> From: Gilad Avidov <gavidov@codeaurora.org>
>>>>
>>>> Add supports for ethernet controller HW on Qualcomm Technologies,
>>>> Inc. SoC.
>>>> This driver supports the following features:
>>>> 1) Checksum offload.
>>>> 2) Runtime power management support.
>>>> 3) Interrupt coalescing support.
>>>> 4) SGMII phy.
>>>> 5) SGMII direct connection without external phy.
>>>
>>>
>>> [snip]
>>>
>>>> +- qcom,no-external-phy : Indicates there is no external PHY
>>>> connected to
>>>> + EMAC. Include this only if the EMAC is directly
>>>> + connected to the peer end without EPHY.
>>> How is the internal PHY accessed, is it responding on the MDIO bus at a
>>> particular address?
>> There is a set of memory-mapped registers. It's not connected via MDIO
>> at all. It's mapped via the "sgmii" addresses in the device tree (see
>> function emac_sgmii_config).
>>
>>> If so, standard MDIO scanning/probing works, and you
>>> can have your PHY driver flag this device has internal. Worst case, you
>>> can do what BCMGENET does, and have a special "phy-mode" value set to
>>> "internal" when this knowledge needs to exist prior to MDIO bus scanning
>>> (e.g: to power on the PHY).
>> So the internal phy is not a real phy. It's not capable of driving an
>> RJ45 port (there's no analog part). It's an SGMII-like device that is
>> hard-wired to the EMAC itself.
There *is* an analog part to the internal SGMII PHY. Please check the SGMII specification. The only non-standard part is that it's not on MDIO.
> OK, that explains things a bit, thanks, this is quite a bit of important
> detail actually.
>
>> In theory, the internal PHY is optional. You could design an SOC that
>> has just the EMAC connected via normal MDIO to an external phy. I
>> really wish our hardware designers has done that. But unfortunately,
>> there are no SOCs like that, and so we have to treat the internal phy as
>> an extension of the EMAC.
>>
>> My preference would be to get rid of the "qcom,no-external-phy" property
>> and have an external phy be required, at least until Qualcomm creates an
>> SOC without the internal phy (which may never happen, for all I know).
>>
> Can we just say that, an absence of PHY specified in the Device Tree (no
> phy-handle property and PHY not a child node of the MDIO bus), means
> that there is no external PHY?
>
> [snip]
>
>
[snip]
>>>> + dev_set_drvdata(&pdev->dev, netdev);
>>>> + SET_NETDEV_DEV(netdev, &pdev->dev);
>>>> +
>>>> + adpt = netdev_priv(netdev);
>>>> + adpt->netdev = netdev;
>>>> + phy = &adpt->phy;
>>>> + adpt->msg_enable = netif_msg_init(debug, EMAC_MSG_DEFAULT);
>>>> +
>>>> + dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
>>> Really, is not that supposed to run on ARM64 servers?
>> Well, this version of the driver isn't, which is why it supports DT and
>> not ACPI. I'm planning on adding that support in a later patch.
>> However, I'll add support for 64-bit masks in the next version of this
>> patch.
>>
>> Would this be okay:
>>
>> retval = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>> if (retval) {
>> dev_err(&pdev->dev, "failed to set DMA mask err %d\n", retval);
>> goto err_res;
>> }
How can you set the mask to 64 bits when the EMAC IP on FSM9900 and QDF2432 can only do 32 bit DMA?
The mask in that API is a bit mask describing which bits of an address your device supports.
>> I've seen code like this in other drivers:
>>
>> ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
>> if (ret) {
>> ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
>> if (ret) {
>> dev_err(dev, "failed to set dma mask\n");
>> return ret;
>> }
>> }
>>
>> and I've never understood why it's necessary to fall back to 32-bits if
>> 64 bits fails. Isn't 64 bits a superset of 32 bits? The driver is
>> saying that the hardware supports all of DDR. How could fail, and how
>> could 32-bit succeed if 64-bits fails?
> I believe there could be cases where the HW is capable of addressing
> more physical memory than the CPU itself (usually unlikely, but it
> could), there could be cases where the HW is behind an IOMMMU which only
> has a window into the DDR, and that could prevent a higher DMA_BIT_MASK
> from being successfully configured.
--
Vikram Sethi
Qualcomm Technologies Inc, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ 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