* [PATCH 02/15] hyperv: Add a function to detect hv_device
2016-12-01 17:28 ` [PATCH 01/15] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() kys
@ 2016-12-01 17:28 ` kys
2016-12-01 20:33 ` Greg KH
2016-12-01 20:35 ` Greg KH
2016-12-01 17:28 ` [PATCH 03/15] hyperv: Fix spelling of HV_UNKOWN kys
` (12 subsequent siblings)
13 siblings, 2 replies; 33+ messages in thread
From: kys @ 2016-12-01 17:28 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: Haiyang Zhang, K. Y. Srinivasan
From: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/vmbus_drv.c | 6 ++++++
include/linux/hyperv.h | 2 ++
2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 0276d2e..1730ac0 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -692,6 +692,12 @@ struct onmessage_work_context {
struct hv_message msg;
};
+bool device_is_hyperv(struct device *dev)
+{
+ return dev->release == vmbus_device_release;
+}
+EXPORT_SYMBOL_GPL(device_is_hyperv);
+
static void vmbus_onmessage_work(struct work_struct *work)
{
struct onmessage_work_context *ctx;
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 2a52d9a..fdd541c 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -946,6 +946,8 @@ static inline void clear_low_latency_mode(struct vmbus_channel *c)
c->low_latency = false;
}
+bool device_is_hyperv(struct device *dev);
+
void vmbus_onmessage(void *context);
int vmbus_request_offers(void);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
2016-12-01 17:28 ` [PATCH 02/15] hyperv: Add a function to detect hv_device kys
@ 2016-12-01 20:33 ` Greg KH
2016-12-02 5:46 ` KY Srinivasan
2016-12-01 20:35 ` Greg KH
1 sibling, 1 reply; 33+ messages in thread
From: Greg KH @ 2016-12-01 20:33 UTC (permalink / raw)
To: kys
Cc: linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara, Haiyang Zhang
On Thu, Dec 01, 2016 at 09:28:39AM -0800, kys@exchange.microsoft.com wrote:
> From: Haiyang Zhang <haiyangz@microsoft.com>
>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
> drivers/hv/vmbus_drv.c | 6 ++++++
> include/linux/hyperv.h | 2 ++
> 2 files changed, 8 insertions(+), 0 deletions(-)
I can't, and you shouldn't, take patches with no changelog comments at
all.
sorry.
greg k-h
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH 02/15] hyperv: Add a function to detect hv_device
2016-12-01 20:33 ` Greg KH
@ 2016-12-02 5:46 ` KY Srinivasan
0 siblings, 0 replies; 33+ messages in thread
From: KY Srinivasan @ 2016-12-02 5:46 UTC (permalink / raw)
To: Greg KH
Cc: linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara, Haiyang Zhang
> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Thursday, December 1, 2016 12:34 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> jasowang@redhat.com; leann.ogasawara@canonical.com; Haiyang Zhang
> <haiyangz@microsoft.com>
> Subject: Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
>
> On Thu, Dec 01, 2016 at 09:28:39AM -0800, kys@exchange.microsoft.com
> wrote:
> > From: Haiyang Zhang <haiyangz@microsoft.com>
> >
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > ---
> > drivers/hv/vmbus_drv.c | 6 ++++++
> > include/linux/hyperv.h | 2 ++
> > 2 files changed, 8 insertions(+), 0 deletions(-)
>
> I can't, and you shouldn't, take patches with no changelog comments at
> all.
>
> sorry.
Sorry about this; this will be fixed.
K. Y
>
> greg k-h
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
2016-12-01 17:28 ` [PATCH 02/15] hyperv: Add a function to detect hv_device kys
2016-12-01 20:33 ` Greg KH
@ 2016-12-01 20:35 ` Greg KH
2016-12-01 20:38 ` Greg KH
2016-12-02 6:02 ` KY Srinivasan
1 sibling, 2 replies; 33+ messages in thread
From: Greg KH @ 2016-12-01 20:35 UTC (permalink / raw)
To: kys
Cc: linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara, Haiyang Zhang
On Thu, Dec 01, 2016 at 09:28:39AM -0800, kys@exchange.microsoft.com wrote:
> From: Haiyang Zhang <haiyangz@microsoft.com>
>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
> drivers/hv/vmbus_drv.c | 6 ++++++
> include/linux/hyperv.h | 2 ++
> 2 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 0276d2e..1730ac0 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -692,6 +692,12 @@ struct onmessage_work_context {
> struct hv_message msg;
> };
>
> +bool device_is_hyperv(struct device *dev)
> +{
> + return dev->release == vmbus_device_release;
> +}
> +EXPORT_SYMBOL_GPL(device_is_hyperv);
Wait, eek, no! That's NOT how you determine a device type, if you
really even ever need to do that.
Why are you needing this? You should always "just know" what type of
device a struct device * is, that's what we rely on in the driver model.
Otherwise things get messy very very quickly.
Sorry, I can't take this without a ton of justification, and even then,
you need to do this correctly (and no, I'm not going to tell you how to
do that as I don't like it being done...)
greg k-h
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
2016-12-01 20:35 ` Greg KH
@ 2016-12-01 20:38 ` Greg KH
2016-12-02 6:02 ` KY Srinivasan
1 sibling, 0 replies; 33+ messages in thread
From: Greg KH @ 2016-12-01 20:38 UTC (permalink / raw)
To: kys
Cc: linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara, Haiyang Zhang
On Thu, Dec 01, 2016 at 09:35:47PM +0100, Greg KH wrote:
> On Thu, Dec 01, 2016 at 09:28:39AM -0800, kys@exchange.microsoft.com wrote:
> > From: Haiyang Zhang <haiyangz@microsoft.com>
> >
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > ---
> > drivers/hv/vmbus_drv.c | 6 ++++++
> > include/linux/hyperv.h | 2 ++
> > 2 files changed, 8 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> > index 0276d2e..1730ac0 100644
> > --- a/drivers/hv/vmbus_drv.c
> > +++ b/drivers/hv/vmbus_drv.c
> > @@ -692,6 +692,12 @@ struct onmessage_work_context {
> > struct hv_message msg;
> > };
> >
> > +bool device_is_hyperv(struct device *dev)
> > +{
> > + return dev->release == vmbus_device_release;
> > +}
> > +EXPORT_SYMBOL_GPL(device_is_hyperv);
>
> Wait, eek, no! That's NOT how you determine a device type, if you
> really even ever need to do that.
>
> Why are you needing this? You should always "just know" what type of
> device a struct device * is, that's what we rely on in the driver model.
> Otherwise things get messy very very quickly.
>
> Sorry, I can't take this without a ton of justification, and even then,
> you need to do this correctly (and no, I'm not going to tell you how to
> do that as I don't like it being done...)
And, to make this even worse, you never use this function in this
series, making this something that no one even needs!!!
ugh.
I'm dropping this whole series, sorry, get it together, this was a
mess...
greg k-h
^ permalink raw reply [flat|nested] 33+ messages in thread* RE: [PATCH 02/15] hyperv: Add a function to detect hv_device
2016-12-01 20:35 ` Greg KH
2016-12-01 20:38 ` Greg KH
@ 2016-12-02 6:02 ` KY Srinivasan
2016-12-02 6:48 ` Greg KH
1 sibling, 1 reply; 33+ messages in thread
From: KY Srinivasan @ 2016-12-02 6:02 UTC (permalink / raw)
To: Greg KH
Cc: linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara, Haiyang Zhang
> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Thursday, December 1, 2016 12:36 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> jasowang@redhat.com; leann.ogasawara@canonical.com; Haiyang Zhang
> <haiyangz@microsoft.com>
> Subject: Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
>
> On Thu, Dec 01, 2016 at 09:28:39AM -0800, kys@exchange.microsoft.com
> wrote:
> > From: Haiyang Zhang <haiyangz@microsoft.com>
> >
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > ---
> > drivers/hv/vmbus_drv.c | 6 ++++++
> > include/linux/hyperv.h | 2 ++
> > 2 files changed, 8 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> > index 0276d2e..1730ac0 100644
> > --- a/drivers/hv/vmbus_drv.c
> > +++ b/drivers/hv/vmbus_drv.c
> > @@ -692,6 +692,12 @@ struct onmessage_work_context {
> > struct hv_message msg;
> > };
> >
> > +bool device_is_hyperv(struct device *dev)
> > +{
> > + return dev->release == vmbus_device_release;
> > +}
> > +EXPORT_SYMBOL_GPL(device_is_hyperv);
>
> Wait, eek, no! That's NOT how you determine a device type, if you
> really even ever need to do that.
>
> Why are you needing this? You should always "just know" what type of
> device a struct device * is, that's what we rely on in the driver model.
> Otherwise things get messy very very quickly.
>
> Sorry, I can't take this without a ton of justification, and even then,
> you need to do this correctly (and no, I'm not going to tell you how to
> do that as I don't like it being done...)
>
Greg,
To support SR-IOV, netvsc registers for all netdev events. For netdev events related to the
VF interface, we need to do some special processing. And so, we need to determine
if the device that is generating the netdev event is Hyper-V device or not
(passed through the PCI pass through driver). Is this justification sufficient.
As you have observed, currently there is no user of this API and that is because netvsc
will be the user. To avoid cross-tree dependency, we wanted to get this functionality in first
before submitting the netvsc patch.
Regards,
K. Y
> greg k-h
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
2016-12-02 6:02 ` KY Srinivasan
@ 2016-12-02 6:48 ` Greg KH
2016-12-02 7:14 ` KY Srinivasan
0 siblings, 1 reply; 33+ messages in thread
From: Greg KH @ 2016-12-02 6:48 UTC (permalink / raw)
To: KY Srinivasan
Cc: linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara, Haiyang Zhang
On Fri, Dec 02, 2016 at 06:02:29AM +0000, KY Srinivasan wrote:
>
>
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Thursday, December 1, 2016 12:36 PM
> > To: KY Srinivasan <kys@microsoft.com>
> > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> > jasowang@redhat.com; leann.ogasawara@canonical.com; Haiyang Zhang
> > <haiyangz@microsoft.com>
> > Subject: Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
Ugh, please fix your email client...
> >
> > On Thu, Dec 01, 2016 at 09:28:39AM -0800, kys@exchange.microsoft.com
> > wrote:
> > > From: Haiyang Zhang <haiyangz@microsoft.com>
> > >
> > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > ---
> > > drivers/hv/vmbus_drv.c | 6 ++++++
> > > include/linux/hyperv.h | 2 ++
> > > 2 files changed, 8 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> > > index 0276d2e..1730ac0 100644
> > > --- a/drivers/hv/vmbus_drv.c
> > > +++ b/drivers/hv/vmbus_drv.c
> > > @@ -692,6 +692,12 @@ struct onmessage_work_context {
> > > struct hv_message msg;
> > > };
> > >
> > > +bool device_is_hyperv(struct device *dev)
> > > +{
> > > + return dev->release == vmbus_device_release;
> > > +}
> > > +EXPORT_SYMBOL_GPL(device_is_hyperv);
> >
> > Wait, eek, no! That's NOT how you determine a device type, if you
> > really even ever need to do that.
> >
> > Why are you needing this? You should always "just know" what type of
> > device a struct device * is, that's what we rely on in the driver model.
> > Otherwise things get messy very very quickly.
> >
> > Sorry, I can't take this without a ton of justification, and even then,
> > you need to do this correctly (and no, I'm not going to tell you how to
> > do that as I don't like it being done...)
> >
> Greg,
>
> To support SR-IOV, netvsc registers for all netdev events. For netdev events related to the
> VF interface, we need to do some special processing. And so, we need to determine
> if the device that is generating the netdev event is Hyper-V device or not
> (passed through the PCI pass through driver). Is this justification sufficient.
> As you have observed, currently there is no user of this API and that is because netvsc
> will be the user. To avoid cross-tree dependency, we wanted to get this functionality in first
> before submitting the netvsc patch.
See, you do have some text for a changelog!
{grumble...}
No, I don't think this is a good justification, where are you going to
put this "check" into the networking stack? Your driver should only be
binding to devices of this "type" anyway, so by that logic, it already
"knows" that the device is of this type.
In other words, why do you need this and PCI or USB doesn't? Why is
hyperv "special"?
greg k-h
^ permalink raw reply [flat|nested] 33+ messages in thread* RE: [PATCH 02/15] hyperv: Add a function to detect hv_device
2016-12-02 6:48 ` Greg KH
@ 2016-12-02 7:14 ` KY Srinivasan
2016-12-02 7:36 ` Greg KH
0 siblings, 1 reply; 33+ messages in thread
From: KY Srinivasan @ 2016-12-02 7:14 UTC (permalink / raw)
To: Greg KH
Cc: linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara, Haiyang Zhang
> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Thursday, December 1, 2016 10:48 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> jasowang@redhat.com; leann.ogasawara@canonical.com; Haiyang Zhang
> <haiyangz@microsoft.com>
> Subject: Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
>
> On Fri, Dec 02, 2016 at 06:02:29AM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Thursday, December 1, 2016 12:36 PM
> > > To: KY Srinivasan <kys@microsoft.com>
> > > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > > olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> > > jasowang@redhat.com; leann.ogasawara@canonical.com; Haiyang Zhang
> > > <haiyangz@microsoft.com>
> > > Subject: Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
>
> Ugh, please fix your email client...
>
> > >
> > > On Thu, Dec 01, 2016 at 09:28:39AM -0800, kys@exchange.microsoft.com
> > > wrote:
> > > > From: Haiyang Zhang <haiyangz@microsoft.com>
> > > >
> > > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > > ---
> > > > drivers/hv/vmbus_drv.c | 6 ++++++
> > > > include/linux/hyperv.h | 2 ++
> > > > 2 files changed, 8 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> > > > index 0276d2e..1730ac0 100644
> > > > --- a/drivers/hv/vmbus_drv.c
> > > > +++ b/drivers/hv/vmbus_drv.c
> > > > @@ -692,6 +692,12 @@ struct onmessage_work_context {
> > > > struct hv_message msg;
> > > > };
> > > >
> > > > +bool device_is_hyperv(struct device *dev)
> > > > +{
> > > > + return dev->release == vmbus_device_release;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(device_is_hyperv);
> > >
> > > Wait, eek, no! That's NOT how you determine a device type, if you
> > > really even ever need to do that.
> > >
> > > Why are you needing this? You should always "just know" what type of
> > > device a struct device * is, that's what we rely on in the driver model.
> > > Otherwise things get messy very very quickly.
> > >
> > > Sorry, I can't take this without a ton of justification, and even then,
> > > you need to do this correctly (and no, I'm not going to tell you how to
> > > do that as I don't like it being done...)
> > >
> > Greg,
> >
> > To support SR-IOV, netvsc registers for all netdev events. For netdev
> events related to the
> > VF interface, we need to do some special processing. And so, we need to
> determine
> > if the device that is generating the netdev event is Hyper-V device or not
> > (passed through the PCI pass through driver). Is this justification sufficient.
> > As you have observed, currently there is no user of this API and that is
> because netvsc
> > will be the user. To avoid cross-tree dependency, we wanted to get this
> functionality in first
> > before submitting the netvsc patch.
>
> See, you do have some text for a changelog!
>
> {grumble...}
>
> No, I don't think this is a good justification, where are you going to
> put this "check" into the networking stack? Your driver should only be
> binding to devices of this "type" anyway, so by that logic, it already
> "knows" that the device is of this type.
>
> In other words, why do you need this and PCI or USB doesn't? Why is
> hyperv "special"?
On Hyper-V, each VF interface (SR-IOV interface)
is paired with an instance of the
synthetic interface that is managed by netvsc.
When the VF interface comes up, we
need to associate the VF instance with
the corresponding netvsc instance. To do this
without modifying the VF drivers, netvsc registers
for netdev events. In the netdev
event handler (in netvsc) currently I am
doing the association based on the MAC
address - this code is currently committed
upstream (see drivers/net/hyperv/netvsc_drv.c
netvsc_register_vf()). Going forward, we want to
base this association based on a sequence
number that the host publishes both for the
VF as well as the corresponding
synthetic (netvsc) instance.
You are right, netvsc already knows that the
devices it is managing belong to vmbus.
Since we are registering for netdev events,
we will get notified for devices that may not
be vmbus devices and that is where this new
API will be used. If the device is a vmbus device
we can extract the sequence number to implement the match.
Regards,
K. Y
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
2016-12-02 7:14 ` KY Srinivasan
@ 2016-12-02 7:36 ` Greg KH
2016-12-02 15:38 ` KY Srinivasan
0 siblings, 1 reply; 33+ messages in thread
From: Greg KH @ 2016-12-02 7:36 UTC (permalink / raw)
To: KY Srinivasan
Cc: linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara, Haiyang Zhang
On Fri, Dec 02, 2016 at 07:14:03AM +0000, KY Srinivasan wrote:
> > In other words, why do you need this and PCI or USB doesn't? Why is
> > hyperv "special"?
>
> On Hyper-V, each VF interface (SR-IOV interface)
> is paired with an instance of the
> synthetic interface that is managed by netvsc.
> When the VF interface comes up, we
> need to associate the VF instance with
> the corresponding netvsc instance. To do this
> without modifying the VF drivers, netvsc registers
> for netdev events.
Why not modify the VF drivers? You have the full source to them...
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH 02/15] hyperv: Add a function to detect hv_device
2016-12-02 7:36 ` Greg KH
@ 2016-12-02 15:38 ` KY Srinivasan
2016-12-02 16:02 ` Greg KH
0 siblings, 1 reply; 33+ messages in thread
From: KY Srinivasan @ 2016-12-02 15:38 UTC (permalink / raw)
To: Greg KH
Cc: linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara, Haiyang Zhang
> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Thursday, December 1, 2016 11:36 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> jasowang@redhat.com; leann.ogasawara@canonical.com; Haiyang Zhang
> <haiyangz@microsoft.com>
> Subject: Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
>
> On Fri, Dec 02, 2016 at 07:14:03AM +0000, KY Srinivasan wrote:
> > > In other words, why do you need this and PCI or USB doesn't? Why is
> > > hyperv "special"?
> >
> > On Hyper-V, each VF interface (SR-IOV interface)
> > is paired with an instance of the
> > synthetic interface that is managed by netvsc.
> > When the VF interface comes up, we
> > need to associate the VF instance with
> > the corresponding netvsc instance. To do this
> > without modifying the VF drivers, netvsc registers
> > for netdev events.
>
> Why not modify the VF drivers? You have the full source to them...
Greg,
This is even worse. On Linux, VF drivers are hypervisor agnostic
and I want to keep it that way.
Regards,
K. Y
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
2016-12-02 15:38 ` KY Srinivasan
@ 2016-12-02 16:02 ` Greg KH
2016-12-02 21:41 ` KY Srinivasan
0 siblings, 1 reply; 33+ messages in thread
From: Greg KH @ 2016-12-02 16:02 UTC (permalink / raw)
To: KY Srinivasan
Cc: olaf, jasowang, Haiyang Zhang, linux-kernel, apw, devel, leann.ogasawara
On Fri, Dec 02, 2016 at 03:38:51PM +0000, KY Srinivasan wrote:
>
>
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Thursday, December 1, 2016 11:36 PM
> > To: KY Srinivasan <kys@microsoft.com>
> > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> > jasowang@redhat.com; leann.ogasawara@canonical.com; Haiyang Zhang
> > <haiyangz@microsoft.com>
> > Subject: Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
> >
> > On Fri, Dec 02, 2016 at 07:14:03AM +0000, KY Srinivasan wrote:
> > > > In other words, why do you need this and PCI or USB doesn't? Why is
> > > > hyperv "special"?
> > >
> > > On Hyper-V, each VF interface (SR-IOV interface)
> > > is paired with an instance of the
> > > synthetic interface that is managed by netvsc.
> > > When the VF interface comes up, we
> > > need to associate the VF instance with
> > > the corresponding netvsc instance. To do this
> > > without modifying the VF drivers, netvsc registers
> > > for netdev events.
> >
> > Why not modify the VF drivers? You have the full source to them...
> Greg,
>
> This is even worse. On Linux, VF drivers are hypervisor agnostic
> and I want to keep it that way.
Ok, I really don't know what to suggest, other than this is probably not
the way to do this as no other bus has to. As I don't see the code that
actually uses this anywhere, it's really impossible to have this
conversation at all :(
greg k-h
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH 02/15] hyperv: Add a function to detect hv_device
2016-12-02 16:02 ` Greg KH
@ 2016-12-02 21:41 ` KY Srinivasan
0 siblings, 0 replies; 33+ messages in thread
From: KY Srinivasan @ 2016-12-02 21:41 UTC (permalink / raw)
To: Greg KH
Cc: olaf, jasowang, Haiyang Zhang, linux-kernel, apw, devel, leann.ogasawara
> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Friday, December 2, 2016 8:03 AM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: olaf@aepfle.de; jasowang@redhat.com; Haiyang Zhang
> <haiyangz@microsoft.com>; linux-kernel@vger.kernel.org;
> apw@canonical.com; devel@linuxdriverproject.org;
> leann.ogasawara@canonical.com
> Subject: Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
>
> On Fri, Dec 02, 2016 at 03:38:51PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Thursday, December 1, 2016 11:36 PM
> > > To: KY Srinivasan <kys@microsoft.com>
> > > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > > olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> > > jasowang@redhat.com; leann.ogasawara@canonical.com; Haiyang Zhang
> > > <haiyangz@microsoft.com>
> > > Subject: Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
> > >
> > > On Fri, Dec 02, 2016 at 07:14:03AM +0000, KY Srinivasan wrote:
> > > > > In other words, why do you need this and PCI or USB doesn't? Why is
> > > > > hyperv "special"?
> > > >
> > > > On Hyper-V, each VF interface (SR-IOV interface)
> > > > is paired with an instance of the
> > > > synthetic interface that is managed by netvsc.
> > > > When the VF interface comes up, we
> > > > need to associate the VF instance with
> > > > the corresponding netvsc instance. To do this
> > > > without modifying the VF drivers, netvsc registers
> > > > for netdev events.
> > >
> > > Why not modify the VF drivers? You have the full source to them...
> > Greg,
> >
> > This is even worse. On Linux, VF drivers are hypervisor agnostic
> > and I want to keep it that way.
>
> Ok, I really don't know what to suggest, other than this is probably not
> the way to do this as no other bus has to. As I don't see the code that
> actually uses this anywhere, it's really impossible to have this
> conversation at all :(
I agree it is difficult to discuss this without
having the code that uses this.
That said, there is currently code in
the tree that disambiguates the netdev
events that netvsc sees - look at the function
get_netvsc_bymac(). This function allows us to
associate the VF interface that maybe coming up with the
associated netvsc interface using MAC address. What I want to do
is to not use the MAC address but to use a serial number that the
host publishes. I could send the netvsc patches that use this if that
would help here. In any case once we have this functionality, we will be
submitting the patches that use this.
What I am trying to do here is to implement the equivalent of dev_is_pci()
for vmbus.
You also had concerns about how we were implementing this functionality.
We could certainly use the same mechanism used in dev_is_pci(). Perhaps
I could also use the same naming convention - dev_is_hv()?
Regards,
K. Y
>
> greg k-h
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 03/15] hyperv: Fix spelling of HV_UNKOWN
2016-12-01 17:28 ` [PATCH 01/15] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() kys
2016-12-01 17:28 ` [PATCH 02/15] hyperv: Add a function to detect hv_device kys
@ 2016-12-01 17:28 ` kys
2016-12-01 17:28 ` [PATCH 04/15] Drivers: hv: vmbus: Prevent sending data on a rescinded channel kys
` (11 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: kys @ 2016-12-01 17:28 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: Haiyang Zhang, K. Y. Srinivasan
From: Haiyang Zhang <haiyangz@microsoft.com>
Changed it to HV_UNKNOWN
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/channel_mgmt.c | 6 +++---
include/linux/hyperv.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index dc6b675..f9c5827 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -134,7 +134,7 @@
},
/* Unknown GUID */
- { .dev_type = HV_UNKOWN,
+ { .dev_type = HV_UNKNOWN,
.perf_device = false,
},
};
@@ -163,9 +163,9 @@ static u16 hv_get_dev_type(const struct vmbus_channel *channel)
u16 i;
if (is_hvsock_channel(channel) || is_unsupported_vmbus_devs(guid))
- return HV_UNKOWN;
+ return HV_UNKNOWN;
- for (i = HV_IDE; i < HV_UNKOWN; i++) {
+ for (i = HV_IDE; i < HV_UNKNOWN; i++) {
if (!uuid_le_cmp(*guid, vmbus_devs[i].guid))
return i;
}
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index fdd541c..5a2d2dd 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -696,7 +696,7 @@ enum vmbus_device_type {
HV_FCOPY,
HV_BACKUP,
HV_DM,
- HV_UNKOWN,
+ HV_UNKNOWN,
};
struct vmbus_device {
--
1.7.4.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 04/15] Drivers: hv: vmbus: Prevent sending data on a rescinded channel
2016-12-01 17:28 ` [PATCH 01/15] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() kys
2016-12-01 17:28 ` [PATCH 02/15] hyperv: Add a function to detect hv_device kys
2016-12-01 17:28 ` [PATCH 03/15] hyperv: Fix spelling of HV_UNKOWN kys
@ 2016-12-01 17:28 ` kys
2016-12-01 20:36 ` Greg KH
2016-12-05 10:01 ` Dan Carpenter
2016-12-01 17:28 ` [PATCH 05/15] Drivers: hv: vmbus: Enhance the rescind callback functionality kys
` (10 subsequent siblings)
13 siblings, 2 replies; 33+ messages in thread
From: kys @ 2016-12-01 17:28 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: K. Y. Srinivasan
From: K. Y. Srinivasan <kys@microsoft.com>
After the channel is rescinded, the host does not read from the rescinded channel.
Fail writes to a channel that has already been rescinded
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/ring_buffer.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index cd49cb1..72f649d 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -298,6 +298,9 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
unsigned long flags = 0;
struct hv_ring_buffer_info *outring_info = &channel->outbound;
+ if (channel->rescind)
+ return -ENODEV;
+
for (i = 0; i < kv_count; i++)
totalbytes_towrite += kv_list[i].iov_len;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 04/15] Drivers: hv: vmbus: Prevent sending data on a rescinded channel
2016-12-01 17:28 ` [PATCH 04/15] Drivers: hv: vmbus: Prevent sending data on a rescinded channel kys
@ 2016-12-01 20:36 ` Greg KH
2016-12-05 10:01 ` Dan Carpenter
1 sibling, 0 replies; 33+ messages in thread
From: Greg KH @ 2016-12-01 20:36 UTC (permalink / raw)
To: kys; +Cc: linux-kernel, devel, olaf, apw, vkuznets, jasowang, leann.ogasawara
On Thu, Dec 01, 2016 at 09:28:41AM -0800, kys@exchange.microsoft.com wrote:
> From: K. Y. Srinivasan <kys@microsoft.com>
>
> After the channel is rescinded, the host does not read from the rescinded channel.
> Fail writes to a channel that has already been rescinded
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
{sigh}
What's wrong with this picture...
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH 04/15] Drivers: hv: vmbus: Prevent sending data on a rescinded channel
2016-12-01 17:28 ` [PATCH 04/15] Drivers: hv: vmbus: Prevent sending data on a rescinded channel kys
2016-12-01 20:36 ` Greg KH
@ 2016-12-05 10:01 ` Dan Carpenter
2016-12-05 15:50 ` KY Srinivasan
1 sibling, 1 reply; 33+ messages in thread
From: Dan Carpenter @ 2016-12-05 10:01 UTC (permalink / raw)
To: kys
Cc: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
On Thu, Dec 01, 2016 at 09:28:41AM -0800, kys@exchange.microsoft.com wrote:
> From: K. Y. Srinivasan <kys@microsoft.com>
>
> After the channel is rescinded, the host does not read from the rescinded channel.
> Fail writes to a channel that has already been rescinded
What are the user visible effects of this bug? Presumably none right
since it doesn't need to be back ported.
But it's absolutely impossible to tell from the commit message.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH 04/15] Drivers: hv: vmbus: Prevent sending data on a rescinded channel
2016-12-05 10:01 ` Dan Carpenter
@ 2016-12-05 15:50 ` KY Srinivasan
0 siblings, 0 replies; 33+ messages in thread
From: KY Srinivasan @ 2016-12-05 15:50 UTC (permalink / raw)
To: Dan Carpenter
Cc: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Monday, December 5, 2016 2:01 AM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> vkuznets@redhat.com; jasowang@redhat.com;
> leann.ogasawara@canonical.com
> Subject: Re: [PATCH 04/15] Drivers: hv: vmbus: Prevent sending data on a
> rescinded channel
>
> On Thu, Dec 01, 2016 at 09:28:41AM -0800, kys@exchange.microsoft.com
> wrote:
> > From: K. Y. Srinivasan <kys@microsoft.com>
> >
> > After the channel is rescinded, the host does not read from the rescinded
> channel.
> > Fail writes to a channel that has already been rescinded
>
> What are the user visible effects of this bug? Presumably none right
> since it doesn't need to be back ported.
>
> But it's absolutely impossible to tell from the commit message.
Thanks Dan. I will update the commit log and resend.
K. Y
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 05/15] Drivers: hv: vmbus: Enhance the rescind callback functionality
2016-12-01 17:28 ` [PATCH 01/15] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() kys
` (2 preceding siblings ...)
2016-12-01 17:28 ` [PATCH 04/15] Drivers: hv: vmbus: Prevent sending data on a rescinded channel kys
@ 2016-12-01 17:28 ` kys
2016-12-01 20:36 ` Greg KH
2016-12-01 17:28 ` [PATCH 06/15] hv: acquire vmbus_connection.channel_mutex in vmbus_free_channels() kys
` (9 subsequent siblings)
13 siblings, 1 reply; 33+ messages in thread
From: kys @ 2016-12-01 17:28 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: K. Y. Srinivasan
From: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/channel_mgmt.c | 11 +++++++----
include/linux/hyperv.h | 7 ++++---
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index f9c5827..d83c1ac 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -825,8 +825,10 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
if (channel->device_obj) {
if (channel->chn_rescind_callback) {
- channel->chn_rescind_callback(channel);
- goto out;
+ channel->chn_rescind_callback(channel,
+ channel->rescind_arg);
+ if (is_hvsock_channel(channel))
+ goto out;
}
/*
* We will have to unregister this device from the
@@ -1215,9 +1217,10 @@ bool vmbus_are_subchannels_present(struct vmbus_channel *primary)
}
EXPORT_SYMBOL_GPL(vmbus_are_subchannels_present);
-void vmbus_set_chn_rescind_callback(struct vmbus_channel *channel,
- void (*chn_rescind_cb)(struct vmbus_channel *))
+void vmbus_set_chn_rescind_callback(struct vmbus_channel *channel, void *arg,
+ void (*chn_rescind_cb)(struct vmbus_channel *, void *))
{
channel->chn_rescind_callback = chn_rescind_cb;
+ channel->rescind_arg = arg;
}
EXPORT_SYMBOL_GPL(vmbus_set_chn_rescind_callback);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 5a2d2dd..5051eca 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -800,7 +800,8 @@ struct vmbus_channel {
* Channel rescind callback. Some channels (the hvsock ones), need to
* register a callback which is invoked in vmbus_onoffer_rescind().
*/
- void (*chn_rescind_callback)(struct vmbus_channel *channel);
+ void (*chn_rescind_callback)(struct vmbus_channel *channel, void *arg);
+ void *rescind_arg;
/*
* The spinlock to protect the structure. It is being used to protect
@@ -959,8 +960,8 @@ static inline void clear_low_latency_mode(struct vmbus_channel *c)
void vmbus_set_sc_create_callback(struct vmbus_channel *primary_channel,
void (*sc_cr_cb)(struct vmbus_channel *new_sc));
-void vmbus_set_chn_rescind_callback(struct vmbus_channel *channel,
- void (*chn_rescind_cb)(struct vmbus_channel *));
+void vmbus_set_chn_rescind_callback(struct vmbus_channel *channel, void *arg,
+ void (*chn_rescind_cb)(struct vmbus_channel *, void *));
/*
* Retrieve the (sub) channel on which to send an outgoing request.
--
1.7.4.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 05/15] Drivers: hv: vmbus: Enhance the rescind callback functionality
2016-12-01 17:28 ` [PATCH 05/15] Drivers: hv: vmbus: Enhance the rescind callback functionality kys
@ 2016-12-01 20:36 ` Greg KH
0 siblings, 0 replies; 33+ messages in thread
From: Greg KH @ 2016-12-01 20:36 UTC (permalink / raw)
To: kys; +Cc: linux-kernel, devel, olaf, apw, vkuznets, jasowang, leann.ogasawara
On Thu, Dec 01, 2016 at 09:28:42AM -0800, kys@exchange.microsoft.com wrote:
> From: K. Y. Srinivasan <kys@microsoft.com>
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
> drivers/hv/channel_mgmt.c | 11 +++++++----
> include/linux/hyperv.h | 7 ++++---
> 2 files changed, 11 insertions(+), 7 deletions(-)
no commit log == will not be committed
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 06/15] hv: acquire vmbus_connection.channel_mutex in vmbus_free_channels()
2016-12-01 17:28 ` [PATCH 01/15] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() kys
` (3 preceding siblings ...)
2016-12-01 17:28 ` [PATCH 05/15] Drivers: hv: vmbus: Enhance the rescind callback functionality kys
@ 2016-12-01 17:28 ` kys
2016-12-01 20:37 ` Greg KH
2016-12-01 17:28 ` [PATCH 07/15] hv: allocate synic pages for all present CPUs kys
` (8 subsequent siblings)
13 siblings, 1 reply; 33+ messages in thread
From: kys @ 2016-12-01 17:28 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: K. Y. Srinivasan
From: Vitaly Kuznetsov <vkuznets@redhat.com>
"kernel BUG at drivers/hv/channel_mgmt.c:350!" is observed when hv_vmbus
module is unloaded. BUG_ON() was introduced in commit 85d9aa705184
("Drivers: hv: vmbus: add an API vmbus_hvsock_device_unregister()") as
vmbus_free_channels() codepath was apparently forgotten.
Fixes: 85d9aa705184 ("Drivers: hv: vmbus: add an API vmbus_hvsock_device_unregister()")
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/channel_mgmt.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index d83c1ac..e68f06c 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -390,6 +390,7 @@ void vmbus_free_channels(void)
{
struct vmbus_channel *channel, *tmp;
+ mutex_lock(&vmbus_connection.channel_mutex);
list_for_each_entry_safe(channel, tmp, &vmbus_connection.chn_list,
listentry) {
/* hv_process_channel_removal() needs this */
@@ -397,6 +398,7 @@ void vmbus_free_channels(void)
vmbus_device_unregister(channel->device_obj);
}
+ mutex_unlock(&vmbus_connection.channel_mutex);
}
/*
--
1.7.4.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 06/15] hv: acquire vmbus_connection.channel_mutex in vmbus_free_channels()
2016-12-01 17:28 ` [PATCH 06/15] hv: acquire vmbus_connection.channel_mutex in vmbus_free_channels() kys
@ 2016-12-01 20:37 ` Greg KH
2016-12-02 6:11 ` KY Srinivasan
0 siblings, 1 reply; 33+ messages in thread
From: Greg KH @ 2016-12-01 20:37 UTC (permalink / raw)
To: kys; +Cc: linux-kernel, devel, olaf, apw, vkuznets, jasowang, leann.ogasawara
On Thu, Dec 01, 2016 at 09:28:43AM -0800, kys@exchange.microsoft.com wrote:
> From: Vitaly Kuznetsov <vkuznets@redhat.com>
>
> "kernel BUG at drivers/hv/channel_mgmt.c:350!" is observed when hv_vmbus
> module is unloaded. BUG_ON() was introduced in commit 85d9aa705184
> ("Drivers: hv: vmbus: add an API vmbus_hvsock_device_unregister()") as
> vmbus_free_channels() codepath was apparently forgotten.
>
> Fixes: 85d9aa705184 ("Drivers: hv: vmbus: add an API vmbus_hvsock_device_unregister()")
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
> drivers/hv/channel_mgmt.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
Shouldn't this go to stable kernels?
^ permalink raw reply [flat|nested] 33+ messages in thread* RE: [PATCH 06/15] hv: acquire vmbus_connection.channel_mutex in vmbus_free_channels()
2016-12-01 20:37 ` Greg KH
@ 2016-12-02 6:11 ` KY Srinivasan
0 siblings, 0 replies; 33+ messages in thread
From: KY Srinivasan @ 2016-12-02 6:11 UTC (permalink / raw)
To: Greg KH
Cc: linux-kernel, devel, olaf, apw, vkuznets, jasowang, leann.ogasawara
> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Thursday, December 1, 2016 12:37 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> jasowang@redhat.com; leann.ogasawara@canonical.com
> Subject: Re: [PATCH 06/15] hv: acquire vmbus_connection.channel_mutex in
> vmbus_free_channels()
>
> On Thu, Dec 01, 2016 at 09:28:43AM -0800, kys@exchange.microsoft.com
> wrote:
> > From: Vitaly Kuznetsov <vkuznets@redhat.com>
> >
> > "kernel BUG at drivers/hv/channel_mgmt.c:350!" is observed when
> hv_vmbus
> > module is unloaded. BUG_ON() was introduced in commit 85d9aa705184
> > ("Drivers: hv: vmbus: add an API vmbus_hvsock_device_unregister()") as
> > vmbus_free_channels() codepath was apparently forgotten.
> >
> > Fixes: 85d9aa705184 ("Drivers: hv: vmbus: add an API
> vmbus_hvsock_device_unregister()")
> > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > ---
> > drivers/hv/channel_mgmt.c | 2 ++
> > 1 files changed, 2 insertions(+), 0 deletions(-)
>
> Shouldn't this go to stable kernels?
Yes; I will resubmit.
Regards,
K. Y
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 07/15] hv: allocate synic pages for all present CPUs
2016-12-01 17:28 ` [PATCH 01/15] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() kys
` (4 preceding siblings ...)
2016-12-01 17:28 ` [PATCH 06/15] hv: acquire vmbus_connection.channel_mutex in vmbus_free_channels() kys
@ 2016-12-01 17:28 ` kys
2016-12-01 17:28 ` [PATCH 08/15] hv: init percpu_list in hv_synic_alloc() kys
` (7 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: kys @ 2016-12-01 17:28 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: K. Y. Srinivasan
From: Vitaly Kuznetsov <vkuznets@redhat.com>
It may happen that not all CPUs are online when we do hv_synic_alloc() and
in case more CPUs come online later we may try accessing these allocated
structures.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/hv.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 60dbd6c..e4bb498 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -411,7 +411,7 @@ int hv_synic_alloc(void)
goto err;
}
- for_each_online_cpu(cpu) {
+ for_each_present_cpu(cpu) {
hv_context.event_dpc[cpu] = kmalloc(size, GFP_ATOMIC);
if (hv_context.event_dpc[cpu] == NULL) {
pr_err("Unable to allocate event dpc\n");
@@ -482,7 +482,7 @@ void hv_synic_free(void)
int cpu;
kfree(hv_context.hv_numa_map);
- for_each_online_cpu(cpu)
+ for_each_present_cpu(cpu)
hv_synic_free_cpu(cpu);
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 08/15] hv: init percpu_list in hv_synic_alloc()
2016-12-01 17:28 ` [PATCH 01/15] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() kys
` (5 preceding siblings ...)
2016-12-01 17:28 ` [PATCH 07/15] hv: allocate synic pages for all present CPUs kys
@ 2016-12-01 17:28 ` kys
2016-12-01 17:28 ` [PATCH 09/15] hv: change clockevents unbind tactics kys
` (6 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: kys @ 2016-12-01 17:28 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: K. Y. Srinivasan
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Initializing hv_context.percpu_list in hv_synic_alloc() helps to prevent a
crash in percpu_channel_enq() when not all CPUs were online during
initialization and it naturally belongs there.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/hv.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index e4bb498..a2567a4 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -457,6 +457,8 @@ int hv_synic_alloc(void)
pr_err("Unable to allocate post msg page\n");
goto err;
}
+
+ INIT_LIST_HEAD(&hv_context.percpu_list[cpu]);
}
return 0;
@@ -552,8 +554,6 @@ void hv_synic_init(void *arg)
rdmsrl(HV_X64_MSR_VP_INDEX, vp_index);
hv_context.vp_index[cpu] = (u32)vp_index;
- INIT_LIST_HEAD(&hv_context.percpu_list[cpu]);
-
/*
* Register the per-cpu clockevent source.
*/
--
1.7.4.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 09/15] hv: change clockevents unbind tactics
2016-12-01 17:28 ` [PATCH 01/15] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() kys
` (6 preceding siblings ...)
2016-12-01 17:28 ` [PATCH 08/15] hv: init percpu_list in hv_synic_alloc() kys
@ 2016-12-01 17:28 ` kys
2016-12-01 17:28 ` [PATCH 10/15] hv: switch to cpuhp state machine for synic init/cleanup kys
` (5 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: kys @ 2016-12-01 17:28 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: K. Y. Srinivasan
From: Vitaly Kuznetsov <vkuznets@redhat.com>
To get prepared to CPU offlining support we need co change the way how we
unbind clockevent devices. As one CPU may go online/offline multiple times
we need to bind it in hv_synic_init() and unbind it in hv_synic_cleanup().
There is an additional corner case: when we unload the module completely we
need to switch to some other clockevent mechanism before stopping VMBus or
we will hang. We can't call hv_synic_cleanup() before unloading VMBus as
we won't be able to send UNLOAD request and get a response so
hv_synic_clockevents_cleanup() has to live. Luckily, we can always call
clockevents_unbind_device(), even if it wasn't bound before and there is
no issue if we call it twice.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/hv.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index a2567a4..c11393c 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -575,7 +575,7 @@ void hv_synic_clockevents_cleanup(void)
if (!(ms_hyperv.features & HV_X64_MSR_SYNTIMER_AVAILABLE))
return;
- for_each_online_cpu(cpu)
+ for_each_present_cpu(cpu)
clockevents_unbind_device(hv_context.clk_evt[cpu], cpu);
}
@@ -594,8 +594,10 @@ void hv_synic_cleanup(void *arg)
return;
/* Turn off clockevent device */
- if (ms_hyperv.features & HV_X64_MSR_SYNTIMER_AVAILABLE)
+ if (ms_hyperv.features & HV_X64_MSR_SYNTIMER_AVAILABLE) {
+ clockevents_unbind_device(hv_context.clk_evt[cpu], cpu);
hv_ce_shutdown(hv_context.clk_evt[cpu]);
+ }
rdmsrl(HV_X64_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 10/15] hv: switch to cpuhp state machine for synic init/cleanup
2016-12-01 17:28 ` [PATCH 01/15] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() kys
` (7 preceding siblings ...)
2016-12-01 17:28 ` [PATCH 09/15] hv: change clockevents unbind tactics kys
@ 2016-12-01 17:28 ` kys
2016-12-01 17:28 ` [PATCH 11/15] hv: make CPU offlining prevention fine-grained kys
` (4 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: kys @ 2016-12-01 17:28 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: K. Y. Srinivasan
From: Vitaly Kuznetsov <vkuznets@redhat.com>
To make it possible to online/offline CPUs switch to cpuhp infrastructure
for doing hv_synic_init()/hv_synic_cleanup().
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/hv.c | 15 +++++++--------
drivers/hv/hyperv_vmbus.h | 4 ++--
drivers/hv/vmbus_drv.c | 19 +++++++++++--------
3 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index c11393c..b3f6a1b 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -495,7 +495,7 @@ void hv_synic_free(void)
* retrieve the initialized message and event pages. Otherwise, we create and
* initialize the message and event pages.
*/
-void hv_synic_init(void *arg)
+int hv_synic_init(unsigned int cpu)
{
u64 version;
union hv_synic_simp simp;
@@ -504,10 +504,8 @@ void hv_synic_init(void *arg)
union hv_synic_scontrol sctrl;
u64 vp_index;
- int cpu = smp_processor_id();
-
if (!hv_context.hypercall_page)
- return;
+ return -EFAULT;
/* Check the version */
rdmsrl(HV_X64_MSR_SVERSION, version);
@@ -562,7 +560,7 @@ void hv_synic_init(void *arg)
HV_TIMER_FREQUENCY,
HV_MIN_DELTA_TICKS,
HV_MAX_MAX_DELTA_TICKS);
- return;
+ return 0;
}
/*
@@ -582,16 +580,15 @@ void hv_synic_clockevents_cleanup(void)
/*
* hv_synic_cleanup - Cleanup routine for hv_synic_init().
*/
-void hv_synic_cleanup(void *arg)
+int hv_synic_cleanup(unsigned int cpu)
{
union hv_synic_sint shared_sint;
union hv_synic_simp simp;
union hv_synic_siefp siefp;
union hv_synic_scontrol sctrl;
- int cpu = smp_processor_id();
if (!hv_context.synic_initialized)
- return;
+ return -EFAULT;
/* Turn off clockevent device */
if (ms_hyperv.features & HV_X64_MSR_SYNTIMER_AVAILABLE) {
@@ -623,4 +620,6 @@ void hv_synic_cleanup(void *arg)
rdmsrl(HV_X64_MSR_SCONTROL, sctrl.as_uint64);
sctrl.enable = 0;
wrmsrl(HV_X64_MSR_SCONTROL, sctrl.as_uint64);
+
+ return 0;
}
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 27982df..83beea7 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -505,9 +505,9 @@ extern int hv_post_message(union hv_connection_id connection_id,
extern void hv_synic_free(void);
-extern void hv_synic_init(void *irqarg);
+extern int hv_synic_init(unsigned int cpu);
-extern void hv_synic_cleanup(void *arg);
+extern int hv_synic_cleanup(unsigned int cpu);
extern void hv_synic_clockevents_cleanup(void);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 1730ac0..cafd273 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -49,6 +49,7 @@
static struct completion probe_event;
+static int hyperv_cpuhp_online;
static void hyperv_report_panic(struct pt_regs *regs)
{
@@ -850,7 +851,12 @@ static int vmbus_bus_init(void)
* Initialize the per-cpu interrupt state and
* connect to the host.
*/
- on_each_cpu(hv_synic_init, NULL, 1);
+ ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/hyperv:online",
+ hv_synic_init, hv_synic_cleanup);
+ if (ret < 0)
+ goto err_alloc;
+ hyperv_cpuhp_online = ret;
+
ret = vmbus_connect();
if (ret)
goto err_connect;
@@ -872,7 +878,7 @@ static int vmbus_bus_init(void)
return 0;
err_connect:
- on_each_cpu(hv_synic_cleanup, NULL, 1);
+ cpuhp_remove_state(hyperv_cpuhp_online);
err_alloc:
hv_synic_free();
hv_remove_vmbus_irq();
@@ -1326,12 +1332,9 @@ static int vmbus_acpi_add(struct acpi_device *device)
static void hv_kexec_handler(void)
{
- int cpu;
-
hv_synic_clockevents_cleanup();
vmbus_initiate_unload(false);
- for_each_online_cpu(cpu)
- smp_call_function_single(cpu, hv_synic_cleanup, NULL, 1);
+ cpuhp_remove_state(hyperv_cpuhp_online);
hv_cleanup(false);
};
@@ -1343,7 +1346,7 @@ static void hv_crash_handler(struct pt_regs *regs)
* doing the cleanup for current CPU only. This should be sufficient
* for kdump.
*/
- hv_synic_cleanup(NULL);
+ hv_synic_cleanup(smp_processor_id());
hv_cleanup(true);
};
@@ -1407,8 +1410,8 @@ static void __exit vmbus_exit(void)
hv_cleanup(false);
for_each_online_cpu(cpu) {
tasklet_kill(hv_context.event_dpc[cpu]);
- smp_call_function_single(cpu, hv_synic_cleanup, NULL, 1);
}
+ cpuhp_remove_state(hyperv_cpuhp_online);
hv_synic_free();
acpi_bus_unregister_driver(&vmbus_acpi_driver);
if (vmbus_proto_version > VERSION_WIN7)
--
1.7.4.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 11/15] hv: make CPU offlining prevention fine-grained
2016-12-01 17:28 ` [PATCH 01/15] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() kys
` (8 preceding siblings ...)
2016-12-01 17:28 ` [PATCH 10/15] hv: switch to cpuhp state machine for synic init/cleanup kys
@ 2016-12-01 17:28 ` kys
2016-12-01 17:28 ` [PATCH 12/15] hv: don't reset hv_context.tsc_page on crash kys
` (3 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: kys @ 2016-12-01 17:28 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: K. Y. Srinivasan
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Since commit e513229b4c38 ("Drivers: hv: vmbus: prevent cpu offlining on
newer hypervisors") cpu offlining was disabled. It is still true that we
can't offline CPUs which have VMBus channels bound to them but we may have
'free' CPUs (e.v. we booted with maxcpus= parameter and onlined CPUs after
VMBus was initialized), these CPUs may be disabled without issues.
In future, we may even allow closing CPUs which have only sub-channels
assinged to them by closing these sub-channels. All devices will continue
to work.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/hv.c | 31 +++++++++++++++++++++++++++++++
drivers/hv/vmbus_drv.c | 9 ++++-----
2 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index b3f6a1b..4ece040 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -586,10 +586,41 @@ int hv_synic_cleanup(unsigned int cpu)
union hv_synic_simp simp;
union hv_synic_siefp siefp;
union hv_synic_scontrol sctrl;
+ struct vmbus_channel *channel, *sc;
+ bool channel_found = false;
+ unsigned long flags;
if (!hv_context.synic_initialized)
return -EFAULT;
+ /*
+ * Search for channels which are bound to the CPU we're about to
+ * cleanup. In case we find one and vmbus is still connected we need to
+ * fail, this will effectively prevent CPU offlining. There is no way
+ * we can re-bind channels to different CPUs for now.
+ */
+ mutex_lock(&vmbus_connection.channel_mutex);
+ list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
+ if (channel->target_cpu == cpu) {
+ channel_found = true;
+ break;
+ }
+ spin_lock_irqsave(&channel->lock, flags);
+ list_for_each_entry(sc, &channel->sc_list, sc_list) {
+ if (sc->target_cpu == cpu) {
+ channel_found = true;
+ break;
+ }
+ }
+ spin_unlock_irqrestore(&channel->lock, flags);
+ if (channel_found)
+ break;
+ }
+ mutex_unlock(&vmbus_connection.channel_mutex);
+
+ if (channel_found && vmbus_connection.conn_state == CONNECTED)
+ return -EBUSY;
+
/* Turn off clockevent device */
if (ms_hyperv.features & HV_X64_MSR_SYNTIMER_AVAILABLE) {
clockevents_unbind_device(hv_context.clk_evt[cpu], cpu);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index cafd273..1e2a413 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -861,9 +861,6 @@ static int vmbus_bus_init(void)
if (ret)
goto err_connect;
- if (vmbus_proto_version > VERSION_WIN7)
- cpu_hotplug_disable();
-
/*
* Only register if the crash MSRs are available
*/
@@ -1334,6 +1331,9 @@ static void hv_kexec_handler(void)
{
hv_synic_clockevents_cleanup();
vmbus_initiate_unload(false);
+ vmbus_connection.conn_state = DISCONNECTED;
+ /* Make sure conn_state is set as hv_synic_cleanup checks for it */
+ mb();
cpuhp_remove_state(hyperv_cpuhp_online);
hv_cleanup(false);
};
@@ -1346,6 +1346,7 @@ static void hv_crash_handler(struct pt_regs *regs)
* doing the cleanup for current CPU only. This should be sufficient
* for kdump.
*/
+ vmbus_connection.conn_state = DISCONNECTED;
hv_synic_cleanup(smp_processor_id());
hv_cleanup(true);
};
@@ -1414,8 +1415,6 @@ static void __exit vmbus_exit(void)
cpuhp_remove_state(hyperv_cpuhp_online);
hv_synic_free();
acpi_bus_unregister_driver(&vmbus_acpi_driver);
- if (vmbus_proto_version > VERSION_WIN7)
- cpu_hotplug_enable();
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 12/15] hv: don't reset hv_context.tsc_page on crash
2016-12-01 17:28 ` [PATCH 01/15] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() kys
` (9 preceding siblings ...)
2016-12-01 17:28 ` [PATCH 11/15] hv: make CPU offlining prevention fine-grained kys
@ 2016-12-01 17:28 ` kys
2016-12-01 17:28 ` [PATCH 13/15] vmbus: add support for dynamic device id's kys
` (2 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: kys @ 2016-12-01 17:28 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: K. Y. Srinivasan
From: Vitaly Kuznetsov <vkuznets@redhat.com>
It may happen that secondary CPUs are still alive and resetting
hv_context.tsc_page will cause a consequent crash in read_hv_clock_tsc()
as we don't check for it being not NULL there. It is safe as we're not
freeing this page anyways.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/hv.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 4ece040..04a0fd8 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -309,9 +309,10 @@ void hv_cleanup(bool crash)
hypercall_msr.as_uint64 = 0;
wrmsrl(HV_X64_MSR_REFERENCE_TSC, hypercall_msr.as_uint64);
- if (!crash)
+ if (!crash) {
vfree(hv_context.tsc_page);
- hv_context.tsc_page = NULL;
+ hv_context.tsc_page = NULL;
+ }
}
#endif
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 13/15] vmbus: add support for dynamic device id's
2016-12-01 17:28 ` [PATCH 01/15] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() kys
` (10 preceding siblings ...)
2016-12-01 17:28 ` [PATCH 12/15] hv: don't reset hv_context.tsc_page on crash kys
@ 2016-12-01 17:28 ` kys
2016-12-01 17:28 ` [PATCH 14/15] uio-hv-generic: new userspace i/o driver for VMBus kys
2016-12-01 17:28 ` [PATCH 15/15] Tools: hv: kvp: configurable external scripts path kys
13 siblings, 0 replies; 33+ messages in thread
From: kys @ 2016-12-01 17:28 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: Stephen Hemminger, K. Y. Srinivasan
From: Stephen Hemminger <sthemmin@microsoft.com>
This patch adds sysfs interface to dynamically bind new UUID values
to existing VMBus device. This is useful for generic UIO driver to
act similar to uio_pci_generic.
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/vmbus_drv.c | 174 +++++++++++++++++++++++++++++++++++++++++++++--
include/linux/hyperv.h | 6 ++
2 files changed, 172 insertions(+), 8 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 1e2a413..fbf1422 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -45,6 +45,11 @@
#include <linux/random.h>
#include "hyperv_vmbus.h"
+struct vmbus_dynid {
+ struct list_head node;
+ struct hv_vmbus_device_id id;
+};
+
static struct acpi_device *hv_acpi_dev;
static struct completion probe_event;
@@ -501,7 +506,7 @@ static ssize_t device_show(struct device *dev,
static DEVICE_ATTR_RO(device);
/* Set up per device attributes in /sys/bus/vmbus/devices/<bus device> */
-static struct attribute *vmbus_attrs[] = {
+static struct attribute *vmbus_dev_attrs[] = {
&dev_attr_id.attr,
&dev_attr_state.attr,
&dev_attr_monitor_id.attr,
@@ -529,7 +534,7 @@ static ssize_t device_show(struct device *dev,
&dev_attr_device.attr,
NULL,
};
-ATTRIBUTE_GROUPS(vmbus);
+ATTRIBUTE_GROUPS(vmbus_dev);
/*
* vmbus_uevent - add uevent for our device
@@ -566,10 +571,29 @@ static inline bool is_null_guid(const uuid_le *guid)
* Return a matching hv_vmbus_device_id pointer.
* If there is no match, return NULL.
*/
-static const struct hv_vmbus_device_id *hv_vmbus_get_id(
- const struct hv_vmbus_device_id *id,
+static const struct hv_vmbus_device_id *hv_vmbus_get_id(struct hv_driver *drv,
const uuid_le *guid)
{
+ const struct hv_vmbus_device_id *id = NULL;
+ struct vmbus_dynid *dynid;
+
+ /* Look at the dynamic ids first, before the static ones */
+ spin_lock(&drv->dynids.lock);
+ list_for_each_entry(dynid, &drv->dynids.list, node) {
+ if (!uuid_le_cmp(dynid->id.guid, *guid)) {
+ id = &dynid->id;
+ break;
+ }
+ }
+ spin_unlock(&drv->dynids.lock);
+
+ if (id)
+ return id;
+
+ id = drv->id_table;
+ if (id == NULL)
+ return NULL; /* empty device table */
+
for (; !is_null_guid(&id->guid); id++)
if (!uuid_le_cmp(id->guid, *guid))
return id;
@@ -577,6 +601,134 @@ static inline bool is_null_guid(const uuid_le *guid)
return NULL;
}
+/* vmbus_add_dynid - add a new device ID to this driver and re-probe devices */
+static int vmbus_add_dynid(struct hv_driver *drv, uuid_le *guid)
+{
+ struct vmbus_dynid *dynid;
+
+ dynid = kzalloc(sizeof(*dynid), GFP_KERNEL);
+ if (!dynid)
+ return -ENOMEM;
+
+ dynid->id.guid = *guid;
+
+ spin_lock(&drv->dynids.lock);
+ list_add_tail(&dynid->node, &drv->dynids.list);
+ spin_unlock(&drv->dynids.lock);
+
+ return driver_attach(&drv->driver);
+}
+
+static void vmbus_free_dynids(struct hv_driver *drv)
+{
+ struct vmbus_dynid *dynid, *n;
+
+ spin_lock(&drv->dynids.lock);
+ list_for_each_entry_safe(dynid, n, &drv->dynids.list, node) {
+ list_del(&dynid->node);
+ kfree(dynid);
+ }
+ spin_unlock(&drv->dynids.lock);
+}
+
+/* Parse string of form: 1b4e28ba-2fa1-11d2-883f-b9a761bde3f */
+static int get_uuid_le(const char *str, uuid_le *uu)
+{
+ unsigned int b[16];
+ int i;
+
+ if (strlen(str) < 37)
+ return -1;
+
+ for (i = 0; i < 36; i++) {
+ switch (i) {
+ case 8: case 13: case 18: case 23:
+ if (str[i] != '-')
+ return -1;
+ break;
+ default:
+ if (!isxdigit(str[i]))
+ return -1;
+ }
+ }
+
+ /* unparse little endian output byte order */
+ if (sscanf(str,
+ "%2x%2x%2x%2x-%2x%2x-%2x%2x-%2x%2x-%2x%2x%2x%2x%2x%2x",
+ &b[3], &b[2], &b[1], &b[0],
+ &b[5], &b[4], &b[7], &b[6], &b[8], &b[9],
+ &b[10], &b[11], &b[12], &b[13], &b[14], &b[15]) != 16)
+ return -1;
+
+ for (i = 0; i < 16; i++)
+ uu->b[i] = b[i];
+ return 0;
+}
+
+/*
+ * store_new_id - sysfs frontend to vmbus_add_dynid()
+ *
+ * Allow GUIDs to be added to an existing driver via sysfs.
+ */
+static ssize_t new_id_store(struct device_driver *driver, const char *buf,
+ size_t count)
+{
+ struct hv_driver *drv = drv_to_hv_drv(driver);
+ uuid_le guid = NULL_UUID_LE;
+ ssize_t retval;
+
+ if (get_uuid_le(buf, &guid) != 0)
+ return -EINVAL;
+
+ if (hv_vmbus_get_id(drv, &guid))
+ return -EEXIST;
+
+ retval = vmbus_add_dynid(drv, &guid);
+ if (retval)
+ return retval;
+ return count;
+}
+static DRIVER_ATTR_WO(new_id);
+
+/*
+ * store_remove_id - remove a PCI device ID from this driver
+ *
+ * Removes a dynamic pci device ID to this driver.
+ */
+static ssize_t remove_id_store(struct device_driver *driver, const char *buf,
+ size_t count)
+{
+ struct hv_driver *drv = drv_to_hv_drv(driver);
+ struct vmbus_dynid *dynid, *n;
+ uuid_le guid = NULL_UUID_LE;
+ size_t retval = -ENODEV;
+
+ if (get_uuid_le(buf, &guid))
+ return -EINVAL;
+
+ spin_lock(&drv->dynids.lock);
+ list_for_each_entry_safe(dynid, n, &drv->dynids.list, node) {
+ struct hv_vmbus_device_id *id = &dynid->id;
+
+ if (!uuid_le_cmp(id->guid, guid)) {
+ list_del(&dynid->node);
+ kfree(dynid);
+ retval = count;
+ break;
+ }
+ }
+ spin_unlock(&drv->dynids.lock);
+
+ return retval;
+}
+static DRIVER_ATTR_WO(remove_id);
+
+static struct attribute *vmbus_drv_attrs[] = {
+ &driver_attr_new_id.attr,
+ &driver_attr_remove_id.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(vmbus_drv);
/*
@@ -591,7 +743,7 @@ static int vmbus_match(struct device *device, struct device_driver *driver)
if (is_hvsock_channel(hv_dev->channel))
return drv->hvsock;
- if (hv_vmbus_get_id(drv->id_table, &hv_dev->dev_type))
+ if (hv_vmbus_get_id(drv, &hv_dev->dev_type))
return 1;
return 0;
@@ -608,7 +760,7 @@ static int vmbus_probe(struct device *child_device)
struct hv_device *dev = device_to_hv_device(child_device);
const struct hv_vmbus_device_id *dev_id;
- dev_id = hv_vmbus_get_id(drv->id_table, &dev->dev_type);
+ dev_id = hv_vmbus_get_id(drv, &dev->dev_type);
if (drv->probe) {
ret = drv->probe(dev, dev_id);
if (ret != 0)
@@ -685,7 +837,8 @@ static void vmbus_device_release(struct device *device)
.remove = vmbus_remove,
.probe = vmbus_probe,
.uevent = vmbus_uevent,
- .dev_groups = vmbus_groups,
+ .dev_groups = vmbus_dev_groups,
+ .drv_groups = vmbus_drv_groups,
};
struct onmessage_work_context {
@@ -914,6 +1067,9 @@ int __vmbus_driver_register(struct hv_driver *hv_driver, struct module *owner, c
hv_driver->driver.mod_name = mod_name;
hv_driver->driver.bus = &hv_bus;
+ spin_lock_init(&hv_driver->dynids.lock);
+ INIT_LIST_HEAD(&hv_driver->dynids.list);
+
ret = driver_register(&hv_driver->driver);
return ret;
@@ -932,8 +1088,10 @@ void vmbus_driver_unregister(struct hv_driver *hv_driver)
{
pr_info("unregistering driver %s\n", hv_driver->name);
- if (!vmbus_exists())
+ if (!vmbus_exists()) {
driver_unregister(&hv_driver->driver);
+ vmbus_free_dynids(hv_driver);
+ }
}
EXPORT_SYMBOL_GPL(vmbus_driver_unregister);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 5051eca..f4c0788 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1122,6 +1122,12 @@ struct hv_driver {
struct device_driver driver;
+ /* dynamic device GUID's */
+ struct {
+ spinlock_t lock;
+ struct list_head list;
+ } dynids;
+
int (*probe)(struct hv_device *, const struct hv_vmbus_device_id *);
int (*remove)(struct hv_device *);
void (*shutdown)(struct hv_device *);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 14/15] uio-hv-generic: new userspace i/o driver for VMBus
2016-12-01 17:28 ` [PATCH 01/15] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() kys
` (11 preceding siblings ...)
2016-12-01 17:28 ` [PATCH 13/15] vmbus: add support for dynamic device id's kys
@ 2016-12-01 17:28 ` kys
2016-12-01 17:28 ` [PATCH 15/15] Tools: hv: kvp: configurable external scripts path kys
13 siblings, 0 replies; 33+ messages in thread
From: kys @ 2016-12-01 17:28 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: Stephen Hemminger, K. Y. Srinivasan
From: Stephen Hemminger <sthemmin@microsoft.com>
This is a new driver to enable userspace networking on VMBus.
It is based largely on the similar driver that already exists
for PCI, and earlier work done by Brocade to support DPDK.
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
MAINTAINERS | 1 +
drivers/hv/connection.c | 1 +
drivers/uio/Kconfig | 9 ++
drivers/uio/Makefile | 1 +
drivers/uio/uio_hv_generic.c | 218 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 230 insertions(+), 0 deletions(-)
create mode 100644 drivers/uio/uio_hv_generic.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 2f89c3b..6d6bea9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5851,6 +5851,7 @@ F: drivers/input/serio/hyperv-keyboard.c
F: drivers/pci/host/pci-hyperv.c
F: drivers/net/hyperv/
F: drivers/scsi/storvsc_drv.c
+F: drivers/uio/uio_hv_generic.c
F: drivers/video/fbdev/hyperv_fb.c
F: include/linux/hyperv.h
F: tools/hv/
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 840b6db..9b72ebc 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -39,6 +39,7 @@ struct vmbus_connection vmbus_connection = {
.conn_state = DISCONNECTED,
.next_gpadl_handle = ATOMIC_INIT(0xE1E10),
};
+EXPORT_SYMBOL_GPL(vmbus_connection);
/*
* Negotiated protocol version with the host.
diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
index 52c98ce..7e8dc78 100644
--- a/drivers/uio/Kconfig
+++ b/drivers/uio/Kconfig
@@ -155,4 +155,13 @@ config UIO_MF624
If you compile this as a module, it will be called uio_mf624.
+config UIO_HV_GENERIC
+ tristate "Generic driver for Hyper-V VMBus"
+ depends on HYPERV
+ help
+ Generic driver that you can bind, dynamically, to any
+ Hyper-V VMBus device. It is useful to provide direct access
+ to network and storage devices from userspace.
+
+ If you compile this as a module, it will be called uio_hv_generic.
endif
diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
index 8560dad..e9663bb 100644
--- a/drivers/uio/Makefile
+++ b/drivers/uio/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_UIO_NETX) += uio_netx.o
obj-$(CONFIG_UIO_PRUSS) += uio_pruss.o
obj-$(CONFIG_UIO_MF624) += uio_mf624.o
obj-$(CONFIG_UIO_FSL_ELBC_GPCM) += uio_fsl_elbc_gpcm.o
+obj-$(CONFIG_UIO_HV_GENERIC) += uio_hv_generic.o
diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
new file mode 100644
index 0000000..ad3ab58
--- /dev/null
+++ b/drivers/uio/uio_hv_generic.c
@@ -0,0 +1,218 @@
+/*
+ * uio_hv_generic - generic UIO driver for VMBus
+ *
+ * Copyright (c) 2013-2016 Brocade Communications Systems, Inc.
+ * Copyright (c) 2016, Microsoft Corporation.
+ *
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.
+ *
+ * Since the driver does not declare any device ids, you must allocate
+ * id and bind the device to the driver yourself. For example:
+ *
+ * # echo "f8615163-df3e-46c5-913f-f2d2f965ed0e" \
+ * > /sys/bus/vmbus/drivers/uio_hv_generic
+ * # echo -n vmbus-ed963694-e847-4b2a-85af-bc9cfc11d6f3 \
+ * > /sys/bus/vmbus/drivers/hv_netvsc/unbind
+ * # echo -n vmbus-ed963694-e847-4b2a-85af-bc9cfc11d6f3 \
+ * > /sys/bus/vmbus/drivers/uio_hv_generic/bind
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/uio_driver.h>
+#include <linux/netdevice.h>
+#include <linux/if_ether.h>
+#include <linux/skbuff.h>
+#include <linux/hyperv.h>
+#include <linux/vmalloc.h>
+#include <linux/slab.h>
+
+#include "../hv/hyperv_vmbus.h"
+
+#define DRIVER_VERSION "0.02.0"
+#define DRIVER_AUTHOR "Stephen Hemminger <sthemmin at microsoft.com>"
+#define DRIVER_DESC "Generic UIO driver for VMBus devices"
+
+/*
+ * List of resources to be mapped to user space
+ * can be extended up to MAX_UIO_MAPS(5) items
+ */
+enum hv_uio_map {
+ TXRX_RING_MAP = 0,
+ INT_PAGE_MAP,
+ MON_PAGE_MAP,
+};
+
+#define HV_RING_SIZE 512
+
+struct hv_uio_private_data {
+ struct uio_info info;
+ struct hv_device *device;
+};
+
+static int
+hv_uio_mmap(struct uio_info *info, struct vm_area_struct *vma)
+{
+ int mi;
+
+ if (vma->vm_pgoff >= MAX_UIO_MAPS)
+ return -EINVAL;
+
+ if (info->mem[vma->vm_pgoff].size == 0)
+ return -EINVAL;
+
+ mi = (int)vma->vm_pgoff;
+
+ return remap_pfn_range(vma, vma->vm_start,
+ virt_to_phys((void *)info->mem[mi].addr) >> PAGE_SHIFT,
+ vma->vm_end - vma->vm_start, vma->vm_page_prot);
+}
+
+/*
+ * This is the irqcontrol callback to be registered to uio_info.
+ * It can be used to disable/enable interrupt from user space processes.
+ *
+ * @param info
+ * pointer to uio_info.
+ * @param irq_state
+ * state value. 1 to enable interrupt, 0 to disable interrupt.
+ */
+static int
+hv_uio_irqcontrol(struct uio_info *info, s32 irq_state)
+{
+ struct hv_uio_private_data *pdata = info->priv;
+ struct hv_device *dev = pdata->device;
+
+ dev->channel->inbound.ring_buffer->interrupt_mask = !irq_state;
+ virt_mb();
+
+ return 0;
+}
+
+/*
+ * Callback from vmbus_event when something is in inbound ring.
+ */
+static void hv_uio_channel_cb(void *context)
+{
+ struct hv_uio_private_data *pdata = context;
+ struct hv_device *dev = pdata->device;
+
+ dev->channel->inbound.ring_buffer->interrupt_mask = 1;
+ virt_mb();
+
+ uio_event_notify(&pdata->info);
+}
+
+static int
+hv_uio_probe(struct hv_device *dev,
+ const struct hv_vmbus_device_id *dev_id)
+{
+ struct hv_uio_private_data *pdata;
+ int ret;
+
+ pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+
+ ret = vmbus_open(dev->channel, HV_RING_SIZE * PAGE_SIZE,
+ HV_RING_SIZE * PAGE_SIZE, NULL, 0,
+ hv_uio_channel_cb, pdata);
+ if (ret)
+ goto fail;
+
+ dev->channel->inbound.ring_buffer->interrupt_mask = 1;
+ dev->channel->batched_reading = false;
+
+ /* Fill general uio info */
+ pdata->info.name = "uio_hv_generic";
+ pdata->info.version = DRIVER_VERSION;
+ pdata->info.irqcontrol = hv_uio_irqcontrol;
+ pdata->info.mmap = hv_uio_mmap;
+ pdata->info.irq = UIO_IRQ_CUSTOM;
+
+ /* mem resources */
+ pdata->info.mem[TXRX_RING_MAP].name = "txrx_rings";
+ pdata->info.mem[TXRX_RING_MAP].addr
+ = (phys_addr_t)dev->channel->ringbuffer_pages;
+ pdata->info.mem[TXRX_RING_MAP].size
+ = dev->channel->ringbuffer_pagecount * PAGE_SIZE;
+ pdata->info.mem[TXRX_RING_MAP].memtype = UIO_MEM_LOGICAL;
+
+ pdata->info.mem[INT_PAGE_MAP].name = "int_page";
+ pdata->info.mem[INT_PAGE_MAP].addr =
+ (phys_addr_t)vmbus_connection.int_page;
+ pdata->info.mem[INT_PAGE_MAP].size = PAGE_SIZE;
+ pdata->info.mem[INT_PAGE_MAP].memtype = UIO_MEM_LOGICAL;
+
+ pdata->info.mem[MON_PAGE_MAP].name = "monitor_pages";
+ pdata->info.mem[MON_PAGE_MAP].addr =
+ (phys_addr_t)vmbus_connection.monitor_pages[1];
+ pdata->info.mem[MON_PAGE_MAP].size = PAGE_SIZE;
+ pdata->info.mem[MON_PAGE_MAP].memtype = UIO_MEM_LOGICAL;
+
+ pdata->info.priv = pdata;
+ pdata->device = dev;
+
+ ret = uio_register_device(&dev->device, &pdata->info);
+ if (ret) {
+ dev_err(&dev->device, "hv_uio register failed\n");
+ goto fail_close;
+ }
+
+ hv_set_drvdata(dev, pdata);
+
+ return 0;
+
+fail_close:
+ vmbus_close(dev->channel);
+fail:
+ kfree(pdata);
+
+ return ret;
+}
+
+static int
+hv_uio_remove(struct hv_device *dev)
+{
+ struct hv_uio_private_data *pdata = hv_get_drvdata(dev);
+
+ if (!pdata)
+ return 0;
+
+ uio_unregister_device(&pdata->info);
+ hv_set_drvdata(dev, NULL);
+ vmbus_close(dev->channel);
+ kfree(pdata);
+ return 0;
+}
+
+static struct hv_driver hv_uio_drv = {
+ .name = "uio_hv_generic",
+ .id_table = NULL, /* only dynamic id's */
+ .probe = hv_uio_probe,
+ .remove = hv_uio_remove,
+};
+
+static int __init
+hyperv_module_init(void)
+{
+ return vmbus_driver_register(&hv_uio_drv);
+}
+
+static void __exit
+hyperv_module_exit(void)
+{
+ vmbus_driver_unregister(&hv_uio_drv);
+}
+
+module_init(hyperv_module_init);
+module_exit(hyperv_module_exit);
+
+MODULE_VERSION(DRIVER_VERSION);
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR(DRIVER_AUTHOR);
+MODULE_DESCRIPTION(DRIVER_DESC);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 15/15] Tools: hv: kvp: configurable external scripts path
2016-12-01 17:28 ` [PATCH 01/15] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() kys
` (12 preceding siblings ...)
2016-12-01 17:28 ` [PATCH 14/15] uio-hv-generic: new userspace i/o driver for VMBus kys
@ 2016-12-01 17:28 ` kys
13 siblings, 0 replies; 33+ messages in thread
From: kys @ 2016-12-01 17:28 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara
Cc: Alex Fluter, K. Y. Srinivasan
From: Alex Fluter <afluter@yandex.com>
error when running hypervkvpd:
$ sudo ./hv_kvp_daemon -n
sh: hv_get_dns_info: command not found
sh: hv_get_dhcp_info: command not found
sh: hv_get_dns_info: command not found
sh: hv_get_dhcp_info: command not found
The external scripts are not installed in system path,
adding a configurable macro.
Signed-off-by: Alex Fluter <afluter@yandex.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
tools/hv/hv_kvp_daemon.c | 11 ++++++++---
1 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index d791dbf..f1758fc 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -93,6 +93,10 @@ enum {
#define KVP_CONFIG_LOC "/var/lib/hyperv"
+#ifndef KVP_SCRIPTS_PATH
+#define KVP_SCRIPTS_PATH "/usr/libexec/hypervkvpd/"
+#endif
+
#define MAX_FILE_NAME 100
#define ENTRIES_PER_BLOCK 50
@@ -818,7 +822,7 @@ static void kvp_get_ipconfig_info(char *if_name,
* .
*/
- sprintf(cmd, "%s", "hv_get_dns_info");
+ sprintf(cmd, KVP_SCRIPTS_PATH "%s", "hv_get_dns_info");
/*
* Execute the command to gather DNS info.
@@ -835,7 +839,7 @@ static void kvp_get_ipconfig_info(char *if_name,
* Enabled: DHCP enabled.
*/
- sprintf(cmd, "%s %s", "hv_get_dhcp_info", if_name);
+ sprintf(cmd, KVP_SCRIPTS_PATH "%s %s", "hv_get_dhcp_info", if_name);
file = popen(cmd, "r");
if (file == NULL)
@@ -1341,7 +1345,8 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
* invoke the external script to do its magic.
*/
- snprintf(cmd, sizeof(cmd), "%s %s", "hv_set_ifconfig", if_file);
+ snprintf(cmd, sizeof(cmd), KVP_SCRIPTS_PATH "%s %s",
+ "hv_set_ifconfig", if_file);
if (system(cmd)) {
syslog(LOG_ERR, "Failed to execute cmd '%s'; error: %d %s",
cmd, errno, strerror(errno));
--
1.7.4.1
^ permalink raw reply related [flat|nested] 33+ messages in thread