* Re: [PATCH] x86/Hyper-V: Fix reference of pv_ops with CONFIG_PARAVIRT=N
From: Wei Liu @ 2019-09-02 14:27 UTC (permalink / raw)
To: lantianyu1986
Cc: kys, haiyangz, sthemmin, sashal, tglx, mingo, bp, hpa, x86,
michael.h.kelley, Tianyu Lan, linux-hyperv, linux-kernel, Wei Liu
In-Reply-To: <20190828080747.204419-1-Tianyu.Lan@microsoft.com>
On Wed, Aug 28, 2019 at 04:07:47PM +0800, lantianyu1986@gmail.com wrote:
> From: Tianyu Lan <Tianyu.Lan@microsoft.com>
>
> hv_setup_sched_clock() references pv_ops and this should
> be under CONFIG_PARAVIRT=Y. Fix it.
>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
Reviewed-by: Wei Liu <wei.liu@kernel.org>
^ permalink raw reply
* Re: [PATCH] x86/Hyper-V: Fix overflow issue in the fill_gva_list()
From: Tianyu Lan @ 2019-09-02 12:46 UTC (permalink / raw)
To: Michael Kelley
Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, hpa@zytor.com, x86@kernel.org,
gregkh@linuxfoundation.org, alex.williamson@redhat.com,
cohuck@redhat.com, Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, kvm@vger.kernel.org
In-Reply-To: <DM5PR21MB0137B7C2AAD0FC65CB3E1306D7BD0@DM5PR21MB0137.namprd21.prod.outlook.com>
On Sat, Aug 31, 2019 at 1:41 AM Michael Kelley <mikelley@microsoft.com> wrote:
>
> From: lantianyu1986@gmail.com Sent: Thursday, August 29, 2019 11:16 PM
> >
> > From: Tianyu Lan <Tianyu.Lan@microsoft.com>
> >
> > fill_gva_list() populates gva list and adds offset
> > HV_TLB_FLUSH_UNIT(0x1000000) to variable "cur"
> > in the each loop. When diff between "end" and "cur" is
> > less than HV_TLB_FLUSH_UNIT, the gva entry should
> > be the last one and the loop should be end.
> >
> > If cur is equal or greater than 0xFF000000 on 32-bit
> > mode, "cur" will be overflow after adding HV_TLB_FLUSH_UNIT.
> > Its value will be wrapped and less than "end". fill_gva_list()
> > falls into an infinite loop and fill gva list out of
> > border finally.
> >
> > Set "cur" to be "end" to make loop end when diff is
> > less than HV_TLB_FLUSH_UNIT and add HV_TLB_FLUSH_UNIT to
> > "cur" when diff is equal or greater than HV_TLB_FLUSH_UNIT.
> > Fix the overflow issue.
>
> Let me suggest simplifying the commit message a bit. It
> doesn't need to describe every line of the code change. I think
> it should also make clear that the same problem could occur on
> 64-bit systems with the right "start" address. My suggestion:
>
> When the 'start' parameter is >= 0xFF000000 on 32-bit
> systems, or >= 0xFFFFFFFF'FF000000 on 64-bit systems,
> fill_gva_list gets into an infinite loop. With such inputs,
> 'cur' overflows after adding HV_TLB_FLUSH_UNIT and always
> compares as less than end. Memory is filled with guest virtual
> addresses until the system crashes
>
> Fix this by never incrementing 'cur' to be larger than 'end'.
>
> >
> > Reported-by: Jong Hyun Park <park.jonghyun@yonsei.ac.kr>
> > Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> > Fixes: 2ffd9e33ce4a ("x86/hyper-v: Use hypercall for remote
> > TLB flush")
>
> The "Fixes:" line needs to not wrap. It's exempt from the
> "wrap at 75 columns" rule in order to simplify parsing scripts.
>
> The code itself looks good.
Hi Michael:
Thanks for suggestion. Update commit log in V2.
--
Best regards
Tianyu Lan
^ permalink raw reply
* [PATCH V2] x86/Hyper-V: Fix overflow issue in the fill_gva_list()
From: lantianyu1986 @ 2019-09-02 12:41 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, tglx, mingo, bp, hpa, x86,
alex.williamson, cohuck, michael.h.kelley
Cc: Tianyu Lan, linux-hyperv, linux-kernel, kvm
From: Tianyu Lan <Tianyu.Lan@microsoft.com>
When the 'start' parameter is >= 0xFF000000 on 32-bit
systems, or >= 0xFFFFFFFF'FF000000 on 64-bit systems,
fill_gva_list gets into an infinite loop. With such inputs,
'cur' overflows after adding HV_TLB_FLUSH_UNIT and always
compares as less than end. Memory is filled with guest virtual
addresses until the system crashes
Fix this by never incrementing 'cur' to be larger than 'end'.
Reported-by: Jong Hyun Park <park.jonghyun@yonsei.ac.kr>
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
Fixes: 2ffd9e33ce4a ("x86/hyper-v: Use hypercall for remote TLB flush")
---
Change since v1:
- Simply the commit message
arch/x86/hyperv/mmu.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/x86/hyperv/mmu.c b/arch/x86/hyperv/mmu.c
index e65d7fe6489f..5208ba49c89a 100644
--- a/arch/x86/hyperv/mmu.c
+++ b/arch/x86/hyperv/mmu.c
@@ -37,12 +37,14 @@ static inline int fill_gva_list(u64 gva_list[], int offset,
* Lower 12 bits encode the number of additional
* pages to flush (in addition to the 'cur' page).
*/
- if (diff >= HV_TLB_FLUSH_UNIT)
+ if (diff >= HV_TLB_FLUSH_UNIT) {
gva_list[gva_n] |= ~PAGE_MASK;
- else if (diff)
+ cur += HV_TLB_FLUSH_UNIT;
+ } else if (diff) {
gva_list[gva_n] |= (diff - 1) >> PAGE_SHIFT;
+ cur = end;
+ }
- cur += HV_TLB_FLUSH_UNIT;
gva_n++;
} while (cur < end);
--
2.14.5
^ permalink raw reply related
* RE: [PATCH v4 4/5] HID: hv: Remove dependencies on PAGE_SIZE for ring buffer
From: Jiri Kosina @ 2019-09-02 11:30 UTC (permalink / raw)
To: Michael Kelley
Cc: m.maya.nakamura, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, benjamin.tissoires@redhat.com, x86@kernel.org,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <DM5PR21MB013708BF1876B7282C049B76D7BC0@DM5PR21MB0137.namprd21.prod.outlook.com>
On Sat, 31 Aug 2019, Michael Kelley wrote:
> From: Maya Nakamura <m.maya.nakamura@gmail.com> Sent: Friday, July 12, 2019 1:28 AM
> >
> > Define the ring buffer size as a constant expression because it should
> > not depend on the guest page size.
> >
> > Signed-off-by: Maya Nakamura <m.maya.nakamura@gmail.com>
> > Reviewed-by: Michael Kelley <mikelley@microsoft.com>
>
> Jiri and Benjamin -- OK if this small patch for the Hyper-V HID driver
> goes through the Hyper-V tree maintained by Sasha Levin? It's a purely
> Hyper-V change so the ring buffer size isn't bigger when running
> on ARM64 where the page size might be 16K or 64K.
Yeah; FWIW feel free to add
Acked-by: Jiri Kosina <jkosina@suse.cz>
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* RE: [PATCH v3 12/12] Drivers: hv: vmbus: Resume after fixing up old primary channels
From: Dexuan Cui @ 2019-08-31 5:08 UTC (permalink / raw)
To: Michael Kelley, linux-hyperv@vger.kernel.org,
gregkh@linuxfoundation.org, Stephen Hemminger, Sasha Levin,
sashal@kernel.org, Haiyang Zhang, KY Srinivasan,
tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org
In-Reply-To: <PU1P153MB0169E3DD602FB575C346186DBFBC0@PU1P153MB0169.APCP153.PROD.OUTLOOK.COM>
> From: Dexuan Cui
> Sent: Friday, August 30, 2019 9:37 PM
> > Is the intent to proceed and use the new offer?
> Yes, since this is not an error.
>
> I'll add a comment before the "Mismatched offer from the host" for this.
Hi Michael,
I'm going to make the below change in v4.
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -956,7 +956,13 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
return;
}
- pr_debug("Mismatched offer from the host (relid=%d)\n",
+ /*
+ * This is not an error, since the host can also change the
+ * other field(s) of the offer, e.g. on WS RS5 (Build 17763),
+ * the offer->connection_id of the Mellanox VF vmbus device
+ * can change when the host reoffers the device upon resume.
+ */
+ pr_debug("vmbus offer changed: relid=%d\n",
offer->child_relid);
print_hex_dump_debug("Old vmbus offer: ", DUMP_PREFIX_OFFSET,
@@ -965,6 +971,7 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
print_hex_dump_debug("New vmbus offer: ", DUMP_PREFIX_OFFSET,
16, 4, offer, offer_sz, false);
+ /* Fix up the old channel. */
vmbus_setup_channel_state(oldchannel, offer);
check_ready_for_resume_event();
Thanks,
-- Dexuan
^ permalink raw reply
* RE: [PATCH v3 12/12] Drivers: hv: vmbus: Resume after fixing up old primary channels
From: Dexuan Cui @ 2019-08-31 4:37 UTC (permalink / raw)
To: Michael Kelley, linux-hyperv@vger.kernel.org,
gregkh@linuxfoundation.org, Stephen Hemminger, Sasha Levin,
sashal@kernel.org, Haiyang Zhang, KY Srinivasan,
tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org
In-Reply-To: <DM5PR21MB01370691E881D59773B9EF60D7A40@DM5PR21MB0137.namprd21.prod.outlook.com>
> From: Michael Kelley <mikelley@microsoft.com>
> Sent: Friday, August 23, 2019 1:25 PM
>
> From: Dexuan Cui Sent: Monday, August 19, 2019 6:52 PM
> > @@ -890,6 +937,11 @@ static void vmbus_onoffer(struct
> > vmbus_channel_message_header *hdr)
> > false);
> > print_hex_dump_debug("New vmbus offer: ",
> DUMP_PREFIX_OFFSET,
> > 16, 4, offer, offer_sz, false);
> > +
> > + vmbus_setup_channel_state(oldchannel, offer);
> > +
> > + check_ready_for_resume_event();
>
> This is the error case where the new offer didn't match some aspect of
> the old offer.
Actually, this is not an error: besides the RELID, the host can also change
the offer->connection_id when it re-offers a device to the guest: so far,
I only see this host behavior for the VF vmbus device, and in this case, the
first vmbus_setup_channel_state() in vmbus_onoffer() is used to do the
fix-up:
channel->sig_event = offer->connection_id;
and later channel->sig_event is used in vmbus_set_event().
Despite the host behavior, it looks the VF vmbus device still works fine,
so (IMO) this is not an error. I'll write a separate email to report this to
Hyper-V team.
> Is the intent to proceed and use the new offer?
Yes, since this is not an error.
I'll add a comment before the "Mismatched offer from the host" for this.
BTW, the 3 debug lines here output nothing, unless we enable the output
by
cd /sys/kernel/debug/dynamic_debug/
echo 'file drivers/hv/channel_mgmt.c +p' > control
.
> I can see that check_ready_for_resume_event() has to be called in
> the error case, otherwise the resume operation will hang forever, but
> I'm not sure about setting up the channel state and then proceeding as
> if all is good.
> > +
> > return;
Thanks,
-- Dexuan
^ permalink raw reply
* RE: [PATCH v3 11/12] Drivers: hv: vmbus: Suspend after cleaning up hv_sock and sub channels
From: Dexuan Cui @ 2019-08-31 2:54 UTC (permalink / raw)
To: Michael Kelley, linux-hyperv@vger.kernel.org,
gregkh@linuxfoundation.org, Stephen Hemminger, Sasha Levin,
sashal@kernel.org, Haiyang Zhang, KY Srinivasan,
tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org
In-Reply-To: <DM5PR21MB01379C31FC628F4666413804D7A40@DM5PR21MB0137.namprd21.prod.outlook.com>
> From: Michael Kelley <mikelley@microsoft.com>
> Sent: Friday, August 23, 2019 1:17 PM
>
> From: Dexuan Cui Sent: Monday, August 19, 2019 6:52 PM
> >
> > diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> > @@ -499,6 +499,8 @@ static void vmbus_add_channel_work(struct
> work_struct *work)
> > return;
> >
> > err_deq_chan:
> > + WARN_ON_ONCE(1);
> > +
>
> Why this change? I was thinking maybe it's a debug statement that got
> left in.
This is indeed a debug statement. :-)
I was not so sure if the error handling is absolutely correct there.
I think I can remove this WARN_ON_ONCE() since almost all of the possible
error paths have a pr_err(). We can make an extra patch to fix any bug in
the existing error handling code. BTW, it should be pretty unlikely to reach
the "err_deq_chan label" in practice.
> > @@ -1021,6 +1044,11 @@ static void vmbus_onoffer_rescind(struct
> > vmbus_channel_message_header *hdr)
> > }
> > mutex_unlock(&vmbus_connection.channel_mutex);
> > }
> > +
> > + /* The "channel" may have been freed. Do not access it any longer. */
> > +
> > + if (clean_up_chan_for_suspend)
> > + check_ready_for_suspend_event();
> > }
>
> Having to add the above lines twice is a bit clumsy, but the problem is
> the overall structure of the vmbus_onoffer_rescind. The early return in
> the case of a rescind_callback function is a bit weird, but I guess it makes
> sense since from what I can tell, only uio and hv_sock have rescind callback
> functions. Some minor restructuring might be warranted, but I don't feel
> strongly about it.
I agree. We should restructure vmbus_onoffer_rescind(), but I think we can
do it in a separate patch. Here in this patch I'm focused on the minimal
required change for hibernation.
> > + /*
> > + * Wait until all the sub-channels and hv_sock channels have been
> > + * cleaned up. Sub-channels should be destroyed upon suspend,
> otherwise
> > + * they would conflict with the new sub-channels that will be created
> > + * in the resume path. hv_sock channels should also be destroyed, but
> > + * a hv_sock channel of an established hv_sock connection can not be
> > + * really destroyed since it may still be referenced by the userspace
> > + * application, so we just force the hv_sock channel to be rescinded
> > + * by vmbus_force_channel_rescinded(), and the userspace application
> > + * will thoroughly destroy the channel after hibernation.
> > + */
> > + if (atomic_read(&vmbus_connection.nr_chan_close_on_suspend) > 0)
>
> At first glance, the above line seemed useless to me. You could just do the
> wait_for_completion() unconditionally. But is the intent to handle the case
> where the VM never had any sub-channels or hv_sock channels, and so
> nr_chan_close_on_suspend never went above 0?
Yes.
> If so, a comment might be helpful.
> > +wait_for_completion(&vmbus_connection.ready_for_suspend_event);
Ok. I'll add a commnt for this in v4.
Thanks,
-- Dexuan
^ permalink raw reply
* RE: [PATCH v3 10/12] Drivers: hv: vmbus: Clean up hv_sock channels by force upon suspend
From: Dexuan Cui @ 2019-08-31 2:00 UTC (permalink / raw)
To: Michael Kelley, linux-hyperv@vger.kernel.org,
gregkh@linuxfoundation.org, Stephen Hemminger, Sasha Levin,
sashal@kernel.org, Haiyang Zhang, KY Srinivasan,
tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org
In-Reply-To: <DM5PR21MB013722B88011C7D587BB6182D7A40@DM5PR21MB0137.namprd21.prod.outlook.com>
> From: Michael Kelley <mikelley@microsoft.com>
> Sent: Friday, August 23, 2019 1:02 PM
>
> From: Dexuan Cui Sent: Monday, August 19, 2019 6:52 PM
> >
> > Fake RESCIND_CHANNEL messages to clean up hv_sock channels by force for
> > hibernation. There is no better method to clean up the channels since
> > some of the channels may still be referenced by the userspace apps when
> > hiberantin is triggered: in this case, the "rescind" fields of the
>
> s/hiberantin/hibernation/
Thanks! Will fix this in v4.
> > @@ -2091,6 +2127,25 @@ static int vmbus_acpi_add(struct acpi_device
> *device)
> >
> > static int vmbus_bus_suspend(struct device *dev)
> > {
> > + struct vmbus_channel *channel;
> > +
> > + while (atomic_read(&vmbus_connection.offer_in_progress) != 0) {
> > + /*
> > + * We wait here until any channel offer is currently
> > + * being processed.
> > + */
>
> The wording of the comment is a bit off. Maybe
>
> /*
> * We wait here until the completion of any channel
> * offers that are currently in progress.
> */
Will use the better version this in v4.
Thanks,
-- Dexuan
^ permalink raw reply
* RE: [PATCH v3 08/12] Drivers: hv: vmbus: Ignore the offers when resuming from hibernation
From: Dexuan Cui @ 2019-08-31 0:23 UTC (permalink / raw)
To: Michael Kelley, linux-hyperv@vger.kernel.org,
gregkh@linuxfoundation.org, Stephen Hemminger, Sasha Levin,
sashal@kernel.org, Haiyang Zhang, KY Srinivasan,
tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org
In-Reply-To: <DM5PR21MB01375F5A46E46079DA611622D7A40@DM5PR21MB0137.namprd21.prod.outlook.com>
> From: Michael Kelley <mikelley@microsoft.com>
> Sent: Friday, August 23, 2019 12:57 PM
>
> From: Dexuan Cui <decui@microsoft.com> Sent: August 19, 2019 6:52 PM
> >
> > diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> > @@ -337,6 +337,33 @@ struct vmbus_channel *relid2channel(u32 relid)
> > }
> >
> > /*
> > + * find_primary_channel_by_offer - Get the channel object given the new
> offer.
> > + * This is only used in the resume path of hibernation.
> > + */
> > +struct vmbus_channel *
> > +find_primary_channel_by_offer(const struct vmbus_channel_offer_channel
> *offer)
> > +{
> > + struct vmbus_channel *channel;
> > + const guid_t *inst1, *inst2;
> > +
> > + WARN_ON(!mutex_is_locked(&vmbus_connection.channel_mutex));
> > +
> > + /* Ignore sub-channel offers. */
> > + if (offer->offer.sub_channel_index != 0)
> > + return NULL;
> > +
> > + list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
> > + inst1 = &channel->offermsg.offer.if_instance;
> > + inst2 = &offer->offer.if_instance;
> > +
> > + if (guid_equal(inst1, inst2))
> > + return channel;
> > + }
> > +
> > + return NULL;
> > +}
>
> Any particular reason this new function is in connection.c instead of
> putting it in channel_mgmt.c where it is called?
There is a similar function relid2channel(), which is in connection.c.
Since the new function is only used in channel_mgmt.c. I'll move it to
channel_mgmt.c in v4.
Thanks,
-- Dexuan
^ permalink raw reply
* RE: [PATCH v3 02/12] x86/hyper-v: Implement hv_is_hibernation_supported()
From: Dexuan Cui @ 2019-08-30 23:37 UTC (permalink / raw)
To: Michael Kelley, linux-hyperv@vger.kernel.org,
gregkh@linuxfoundation.org, Stephen Hemminger, Sasha Levin,
sashal@kernel.org, Haiyang Zhang, KY Srinivasan,
tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org
In-Reply-To: <DM5PR21MB0137A7EC5E51EAF8A0667359D7A40@DM5PR21MB0137.namprd21.prod.outlook.com>
> From: Michael Kelley <mikelley@microsoft.com>
> Sent: Friday, August 23, 2019 12:50 PM
>
> From: Dexuan Cui <decui@microsoft.com> Sent: August 19, 2019 6:52 PM
> >
> > When a Linux VM runs on Hyper-V and hibernates, it must disable the
> > memory hot-add/remove and balloon up/down capabilities in the hv_balloon
> > driver.
>
> I'm unclear on the above statement. I think the requirement is that
> ballooning must not be active when hibernation is initiated. Is hibernation
> blocked in that case? If not, what happens?
Ballooning (and hot-addition of memory) must not be active if the user
wants the Linux VM to support hibernation, not just when hibernation is
initiated. This is because ballooning and hot-addition of memory are
incompatible with hibernation according to Hyper-V team, e.g. upon
hibernation the balloon VSP doesn't save any info about ballooned-out
pages (if any), so after Linux resumes, Linux balloon VSC expects that the
VSP will return the pages if Linux is under memory pressure, but the VSP
will never return the pages, since the VSP thinks it never stole the
pages from the VM.
So, if the user wants Linux VM to support hibernation, Linux must completely
forbid ballooning and hot-addition of memory, and hence the only
functionality of balloon VSC is reporting the memory pressure to the host.
Ideally, when Linux detects that the user wants it to support hibernation,
the balloon VSC should tell the VSP that it does not support ballooning and
hot-addition; however, the current version of the VSP requires
the VSC should support these capabilities, otherwise the capability negotiation
fails and the VSC can not load at all, so in my changes to the VSC driver, I
still report to the VSP that Linux supports the capabilities, but the VSC
ignores the host's requests of balloon up/down and hot add, and returns an
error to the VSP, when applicable.
BTW, in the future the balloon VSP driver will allow the VSC to not support
the capabilities of balloon up/down and hot add.
The remaining problem is: how can Linux know the user wants Linux VM
to support hibernation?
The ACPI S4 state is not a must for hibernation to work, because Linux is
able to hibernate as long as the system can shut down.
My decision is that: we artificially use the presence of the virtual
ACPI S4 state as the indicator of the user's intent of using hibernation.
BTW, I believe the Windows team made the same decision.
Once all the vmbus and VSC patches are accepted, I'll submit a patch to
forbid hibernation if the virtual ACPI S4 state is absent, e.g.
hv_is_hibernation_supported() is false.
> > By default, Hyper-V does not enable the virtual ACPI S4 state for a VM;
> > on recent Hyper-V hosts, the administrator is able to enable the virtual
> > ACPI S4 state for a VM, so we hope to use the presence of the virtual ACPI
>
> "we hope" sounds very indefinite. :-( Does ACPI S4 have to be enabled for
> hibernation to be initiated? Goes back to my first question ....
Technically, we don't have to enable ACPI S4, but in practice, as I mentioned,
I'll submit a patch to forbid hibernation if ACPI S4 is absent.
>
> > S4 state as a hint for hv_balloon to disable the aforementioned
> > capabilities.
> >
> > The new API will be used by hv_balloon.
I'll add my above explanation into the changelog in v4.
Thanks,
-- Dexuan
^ permalink raw reply
* RE: [PATCH net-next, 2/2] hv_netvsc: Sync offloading features to VF NIC
From: Haiyang Zhang @ 2019-08-30 23:12 UTC (permalink / raw)
To: Jakub Kicinski
Cc: sashal@kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, KY Srinivasan, Stephen Hemminger,
olaf@aepfle.de, vkuznets, davem@davemloft.net,
linux-kernel@vger.kernel.org, Mark Bloch
In-Reply-To: <20190830160451.43a61cf9@cakuba.netronome.com>
> -----Original Message-----
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Sent: Friday, August 30, 2019 4:05 PM
> To: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: sashal@kernel.org; linux-hyperv@vger.kernel.org;
> netdev@vger.kernel.org; KY Srinivasan <kys@microsoft.com>; Stephen
> Hemminger <sthemmin@microsoft.com>; olaf@aepfle.de; vkuznets
> <vkuznets@redhat.com>; davem@davemloft.net; linux-
> kernel@vger.kernel.org; Mark Bloch <markb@mellanox.com>
> Subject: Re: [PATCH net-next, 2/2] hv_netvsc: Sync offloading features to VF
> NIC
>
> On Fri, 30 Aug 2019 03:45:38 +0000, Haiyang Zhang wrote:
> > VF NIC may go down then come up during host servicing events. This
> > causes the VF NIC offloading feature settings to roll back to the
> > defaults. This patch can synchronize features from synthetic NIC to
> > the VF NIC during ndo_set_features (ethtool -K), and
> > netvsc_register_vf when VF comes back after host events.
> >
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Cc: Mark Bloch <markb@mellanox.com>
>
> If we want to make this change in behaviour we should change net_failover
> at the same time.
I will check net_failover. Thanks.
^ permalink raw reply
* Re: [PATCH net-next, 1/2] hv_netvsc: Allow scatter-gather feature to be tunable
From: Jakub Kicinski @ 2019-08-30 23:05 UTC (permalink / raw)
To: Haiyang Zhang
Cc: sashal@kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, KY Srinivasan, Stephen Hemminger,
olaf@aepfle.de, vkuznets, davem@davemloft.net,
linux-kernel@vger.kernel.org
In-Reply-To: <1567136656-49288-2-git-send-email-haiyangz@microsoft.com>
On Fri, 30 Aug 2019 03:45:24 +0000, Haiyang Zhang wrote:
> In a previous patch, the NETIF_F_SG was missing after the code changes.
> That caused the SG feature to be "fixed". This patch includes it into
> hw_features, so it is tunable again.
>
> Fixes: 23312a3be999 ("netvsc: negotiate checksum and segmentation parameters")
^
Looks like a tab sneaked in there.
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
^ permalink raw reply
* Re: [PATCH net-next, 2/2] hv_netvsc: Sync offloading features to VF NIC
From: Jakub Kicinski @ 2019-08-30 23:04 UTC (permalink / raw)
To: Haiyang Zhang
Cc: sashal@kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, KY Srinivasan, Stephen Hemminger,
olaf@aepfle.de, vkuznets, davem@davemloft.net,
linux-kernel@vger.kernel.org, Mark Bloch
In-Reply-To: <1567136656-49288-3-git-send-email-haiyangz@microsoft.com>
On Fri, 30 Aug 2019 03:45:38 +0000, Haiyang Zhang wrote:
> VF NIC may go down then come up during host servicing events. This
> causes the VF NIC offloading feature settings to roll back to the
> defaults. This patch can synchronize features from synthetic NIC to
> the VF NIC during ndo_set_features (ethtool -K),
> and netvsc_register_vf when VF comes back after host events.
>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: Mark Bloch <markb@mellanox.com>
If we want to make this change in behaviour we should change
net_failover at the same time.
^ permalink raw reply
* RE: [PATCH] x86/Hyper-V: Fix overflow issue in the fill_gva_list()
From: Michael Kelley @ 2019-08-30 17:41 UTC (permalink / raw)
To: lantianyu1986@gmail.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, sashal@kernel.org, tglx@linutronix.de,
mingo@redhat.com, bp@alien8.de, hpa@zytor.com, x86@kernel.org,
gregkh@linuxfoundation.org, alex.williamson@redhat.com,
cohuck@redhat.com
Cc: Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, kvm@vger.kernel.org
In-Reply-To: <20190830061540.211072-1-Tianyu.Lan@microsoft.com>
From: lantianyu1986@gmail.com Sent: Thursday, August 29, 2019 11:16 PM
>
> From: Tianyu Lan <Tianyu.Lan@microsoft.com>
>
> fill_gva_list() populates gva list and adds offset
> HV_TLB_FLUSH_UNIT(0x1000000) to variable "cur"
> in the each loop. When diff between "end" and "cur" is
> less than HV_TLB_FLUSH_UNIT, the gva entry should
> be the last one and the loop should be end.
>
> If cur is equal or greater than 0xFF000000 on 32-bit
> mode, "cur" will be overflow after adding HV_TLB_FLUSH_UNIT.
> Its value will be wrapped and less than "end". fill_gva_list()
> falls into an infinite loop and fill gva list out of
> border finally.
>
> Set "cur" to be "end" to make loop end when diff is
> less than HV_TLB_FLUSH_UNIT and add HV_TLB_FLUSH_UNIT to
> "cur" when diff is equal or greater than HV_TLB_FLUSH_UNIT.
> Fix the overflow issue.
Let me suggest simplifying the commit message a bit. It
doesn't need to describe every line of the code change. I think
it should also make clear that the same problem could occur on
64-bit systems with the right "start" address. My suggestion:
When the 'start' parameter is >= 0xFF000000 on 32-bit
systems, or >= 0xFFFFFFFF'FF000000 on 64-bit systems,
fill_gva_list gets into an infinite loop. With such inputs,
'cur' overflows after adding HV_TLB_FLUSH_UNIT and always
compares as less than end. Memory is filled with guest virtual
addresses until the system crashes
Fix this by never incrementing 'cur' to be larger than 'end'.
>
> Reported-by: Jong Hyun Park <park.jonghyun@yonsei.ac.kr>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> Fixes: 2ffd9e33ce4a ("x86/hyper-v: Use hypercall for remote
> TLB flush")
The "Fixes:" line needs to not wrap. It's exempt from the
"wrap at 75 columns" rule in order to simplify parsing scripts.
The code itself looks good.
Michael
^ permalink raw reply
* [PATCH] x86/Hyper-V: Fix overflow issue in the fill_gva_list()
From: lantianyu1986 @ 2019-08-30 6:15 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, tglx, mingo, bp, hpa, x86,
gregkh, alex.williamson, cohuck, michael.h.kelley
Cc: Tianyu Lan, linux-hyperv, linux-kernel, kvm
From: Tianyu Lan <Tianyu.Lan@microsoft.com>
fill_gva_list() populates gva list and adds offset
HV_TLB_FLUSH_UNIT(0x1000000) to variable "cur"
in the each loop. When diff between "end" and "cur" is
less than HV_TLB_FLUSH_UNIT, the gva entry should
be the last one and the loop should be end.
If cur is equal or greater than 0xFF000000 on 32-bit
mode, "cur" will be overflow after adding HV_TLB_FLUSH_UNIT.
Its value will be wrapped and less than "end". fill_gva_list()
falls into an infinite loop and fill gva list out of
border finally.
Set "cur" to be "end" to make loop end when diff is
less than HV_TLB_FLUSH_UNIT and add HV_TLB_FLUSH_UNIT to
"cur" when diff is equal or greater than HV_TLB_FLUSH_UNIT.
Fix the overflow issue.
Reported-by: Jong Hyun Park <park.jonghyun@yonsei.ac.kr>
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
Fixes: 2ffd9e33ce4a ("x86/hyper-v: Use hypercall for remote
TLB flush")
---
arch/x86/hyperv/mmu.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/x86/hyperv/mmu.c b/arch/x86/hyperv/mmu.c
index e65d7fe6489f..5208ba49c89a 100644
--- a/arch/x86/hyperv/mmu.c
+++ b/arch/x86/hyperv/mmu.c
@@ -37,12 +37,14 @@ static inline int fill_gva_list(u64 gva_list[], int offset,
* Lower 12 bits encode the number of additional
* pages to flush (in addition to the 'cur' page).
*/
- if (diff >= HV_TLB_FLUSH_UNIT)
+ if (diff >= HV_TLB_FLUSH_UNIT) {
gva_list[gva_n] |= ~PAGE_MASK;
- else if (diff)
+ cur += HV_TLB_FLUSH_UNIT;
+ } else if (diff) {
gva_list[gva_n] |= (diff - 1) >> PAGE_SHIFT;
+ cur = end;
+ }
- cur += HV_TLB_FLUSH_UNIT;
gva_n++;
} while (cur < end);
--
2.14.5
^ permalink raw reply related
* [PATCH net-next, 2/2] hv_netvsc: Sync offloading features to VF NIC
From: Haiyang Zhang @ 2019-08-30 3:45 UTC (permalink / raw)
To: sashal@kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org
Cc: Haiyang Zhang, KY Srinivasan, Stephen Hemminger, olaf@aepfle.de,
vkuznets, davem@davemloft.net, linux-kernel@vger.kernel.org,
Mark Bloch
In-Reply-To: <1567136656-49288-1-git-send-email-haiyangz@microsoft.com>
VF NIC may go down then come up during host servicing events. This
causes the VF NIC offloading feature settings to roll back to the
defaults. This patch can synchronize features from synthetic NIC to
the VF NIC during ndo_set_features (ethtool -K),
and netvsc_register_vf when VF comes back after host events.
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Mark Bloch <markb@mellanox.com>
---
drivers/net/hyperv/netvsc_drv.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 1f1192e..39dddcd 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1785,13 +1785,15 @@ static int netvsc_set_features(struct net_device *ndev,
netdev_features_t change = features ^ ndev->features;
struct net_device_context *ndevctx = netdev_priv(ndev);
struct netvsc_device *nvdev = rtnl_dereference(ndevctx->nvdev);
+ struct net_device *vf_netdev = rtnl_dereference(ndevctx->vf_netdev);
struct ndis_offload_params offloads;
+ int ret = 0;
if (!nvdev || nvdev->destroy)
return -ENODEV;
if (!(change & NETIF_F_LRO))
- return 0;
+ goto syncvf;
memset(&offloads, 0, sizeof(struct ndis_offload_params));
@@ -1803,7 +1805,19 @@ static int netvsc_set_features(struct net_device *ndev,
offloads.rsc_ip_v6 = NDIS_OFFLOAD_PARAMETERS_RSC_DISABLED;
}
- return rndis_filter_set_offload_params(ndev, nvdev, &offloads);
+ ret = rndis_filter_set_offload_params(ndev, nvdev, &offloads);
+
+ if (ret)
+ features ^= NETIF_F_LRO;
+
+syncvf:
+ if (!vf_netdev)
+ return ret;
+
+ vf_netdev->wanted_features = features;
+ netdev_update_features(vf_netdev);
+
+ return ret;
}
static u32 netvsc_get_msglevel(struct net_device *ndev)
@@ -2181,6 +2195,10 @@ static int netvsc_register_vf(struct net_device *vf_netdev)
dev_hold(vf_netdev);
rcu_assign_pointer(net_device_ctx->vf_netdev, vf_netdev);
+
+ vf_netdev->wanted_features = ndev->features;
+ netdev_update_features(vf_netdev);
+
return NOTIFY_OK;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next, 1/2] hv_netvsc: Allow scatter-gather feature to be tunable
From: Haiyang Zhang @ 2019-08-30 3:45 UTC (permalink / raw)
To: sashal@kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org
Cc: Haiyang Zhang, KY Srinivasan, Stephen Hemminger, olaf@aepfle.de,
vkuznets, davem@davemloft.net, linux-kernel@vger.kernel.org
In-Reply-To: <1567136656-49288-1-git-send-email-haiyangz@microsoft.com>
In a previous patch, the NETIF_F_SG was missing after the code changes.
That caused the SG feature to be "fixed". This patch includes it into
hw_features, so it is tunable again.
Fixes: 23312a3be999 ("netvsc: negotiate checksum and segmentation parameters")
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/net/hyperv/hyperv_net.h | 2 +-
drivers/net/hyperv/netvsc_drv.c | 4 ++--
drivers/net/hyperv/rndis_filter.c | 1 +
3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index ecc9af0..670ef68 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -822,7 +822,7 @@ struct nvsp_message {
#define NETVSC_SUPPORTED_HW_FEATURES (NETIF_F_RXCSUM | NETIF_F_IP_CSUM | \
NETIF_F_TSO | NETIF_F_IPV6_CSUM | \
- NETIF_F_TSO6 | NETIF_F_LRO)
+ NETIF_F_TSO6 | NETIF_F_LRO | NETIF_F_SG)
#define VRSS_SEND_TAB_SIZE 16 /* must be power of 2 */
#define VRSS_CHANNEL_MAX 64
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 0a6cd2f..1f1192e 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -2313,8 +2313,8 @@ static int netvsc_probe(struct hv_device *dev,
/* hw_features computed in rndis_netdev_set_hwcaps() */
net->features = net->hw_features |
- NETIF_F_HIGHDMA | NETIF_F_SG |
- NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX;
+ NETIF_F_HIGHDMA | NETIF_F_HW_VLAN_CTAG_TX |
+ NETIF_F_HW_VLAN_CTAG_RX;
net->vlan_features = net->features;
netdev_lockdep_set_classes(net);
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index 317dbe9..abaf815 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -1207,6 +1207,7 @@ static int rndis_netdev_set_hwcaps(struct rndis_device *rndis_device,
/* Compute tx offload settings based on hw capabilities */
net->hw_features |= NETIF_F_RXCSUM;
+ net->hw_features |= NETIF_F_SG;
if ((hwcaps.csum.ip4_txcsum & NDIS_TXCSUM_ALL_TCP4) == NDIS_TXCSUM_ALL_TCP4) {
/* Can checksum TCP */
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next, 0/2] Enable sg as tunable, sync offload settings to VF NIC
From: Haiyang Zhang @ 2019-08-30 3:45 UTC (permalink / raw)
To: sashal@kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org
Cc: Haiyang Zhang, KY Srinivasan, Stephen Hemminger, olaf@aepfle.de,
vkuznets, davem@davemloft.net, linux-kernel@vger.kernel.org
This patch set fixes an issue in SG tuning, and sync
offload settings from synthetic NIC to VF NIC.
Haiyang Zhang (2):
hv_netvsc: hv_netvsc: Allow scatter-gather feature to be tunable
hv_netvsc: Sync offloading features to VF NIC
drivers/net/hyperv/hyperv_net.h | 2 +-
drivers/net/hyperv/netvsc_drv.c | 26 ++++++++++++++++++++++----
drivers/net/hyperv/rndis_filter.c | 1 +
3 files changed, 24 insertions(+), 5 deletions(-)
--
1.8.3.1
^ permalink raw reply
* Re: [PATCH v4 1/2] drivers: hv: vmbus: Introduce latency testing
From: kbuild test robot @ 2019-08-30 3:23 UTC (permalink / raw)
To: Branden Bonaby
Cc: kbuild-all, kys, haiyangz, sthemmin, sashal, Branden Bonaby,
linux-hyperv, linux-kernel
In-Reply-To: <da1ab5c98ce902ec181a799d8d1f040e8cc39af6.1567047549.git.brandonbonaby94@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1179 bytes --]
Hi Branden,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc6 next-20190829]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Branden-Bonaby/drivers-hv-vmbus-Introduce-latency-testing/20190830-032123
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-11) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> ERROR: "hv_debug_add_dev_dir" [drivers/hv/hv_vmbus.ko] undefined!
>> ERROR: "hv_debug_init" [drivers/hv/hv_vmbus.ko] undefined!
>> ERROR: "hv_debug_delay_test" [drivers/hv/hv_vmbus.ko] undefined!
>> ERROR: "hv_debug_rm_all_dir" [drivers/hv/hv_vmbus.ko] undefined!
>> ERROR: "hv_debug_rm_dev_dir" [drivers/hv/hv_vmbus.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 69479 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/3] drivers: hv: vmbus: Introduce latency testing
From: Stephen Hemminger @ 2019-08-29 21:57 UTC (permalink / raw)
To: Branden Bonaby
Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, sashal,
linux-hyperv
In-Reply-To: <ebca54bf70d2af53de419c1b7ac8db5b77b888cb.1566340843.git.brandonbonaby94@gmail.com>
On Tue, 20 Aug 2019 16:39:02 -0700
"Branden Bonaby" <brandonbonaby94@gmail.com> wrote:
> diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
> index 9a59957922d4..d97437ba0626 100644
> --- a/drivers/hv/Kconfig
> +++ b/drivers/hv/Kconfig
> @@ -29,4 +29,11 @@ config HYPERV_BALLOON
> help
> Select this option to enable Hyper-V Balloon driver.
>
> +config HYPERV_TESTING
> + bool "Hyper-V testing"
> + default n
> + depends on HYPERV && DEBUG_FS
> + help
> + Select this option to enable Hyper-V vmbus testing.
> +
> endmenu
Maybe this should go under the Kernel hacking
section in lib/Kconfig.debug?
^ permalink raw reply
* [PATCH AUTOSEL 5.2 54/76] Tools: hv: kvp: eliminate 'may be used uninitialized' warning
From: Sasha Levin @ 2019-08-29 18:12 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Vitaly Kuznetsov, Sasha Levin, linux-hyperv
In-Reply-To: <20190829181311.7562-1-sashal@kernel.org>
From: Vitaly Kuznetsov <vkuznets@redhat.com>
[ Upstream commit 89eb4d8d25722a0a0194cf7fa47ba602e32a6da7 ]
When building hv_kvp_daemon GCC-8.3 complains:
hv_kvp_daemon.c: In function ‘kvp_get_ip_info.constprop’:
hv_kvp_daemon.c:812:30: warning: ‘ip_buffer’ may be used uninitialized in this function [-Wmaybe-uninitialized]
struct hv_kvp_ipaddr_value *ip_buffer;
this seems to be a false positive: we only use ip_buffer when
op == KVP_OP_GET_IP_INFO and it is only unset when op == KVP_OP_ENUMERATE.
Silence the warning by initializing ip_buffer to NULL.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
tools/hv/hv_kvp_daemon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index d7e06fe0270ee..6d809abaf338a 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -809,7 +809,7 @@ kvp_get_ip_info(int family, char *if_name, int op,
int sn_offset = 0;
int error = 0;
char *buffer;
- struct hv_kvp_ipaddr_value *ip_buffer;
+ struct hv_kvp_ipaddr_value *ip_buffer = NULL;
char cidr_mask[5]; /* /xyz */
int weight;
int i;
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 33/45] Tools: hv: kvp: eliminate 'may be used uninitialized' warning
From: Sasha Levin @ 2019-08-29 18:15 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Vitaly Kuznetsov, Sasha Levin, linux-hyperv
In-Reply-To: <20190829181547.8280-1-sashal@kernel.org>
From: Vitaly Kuznetsov <vkuznets@redhat.com>
[ Upstream commit 89eb4d8d25722a0a0194cf7fa47ba602e32a6da7 ]
When building hv_kvp_daemon GCC-8.3 complains:
hv_kvp_daemon.c: In function ‘kvp_get_ip_info.constprop’:
hv_kvp_daemon.c:812:30: warning: ‘ip_buffer’ may be used uninitialized in this function [-Wmaybe-uninitialized]
struct hv_kvp_ipaddr_value *ip_buffer;
this seems to be a false positive: we only use ip_buffer when
op == KVP_OP_GET_IP_INFO and it is only unset when op == KVP_OP_ENUMERATE.
Silence the warning by initializing ip_buffer to NULL.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
tools/hv/hv_kvp_daemon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index d7e06fe0270ee..6d809abaf338a 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -809,7 +809,7 @@ kvp_get_ip_info(int family, char *if_name, int op,
int sn_offset = 0;
int error = 0;
char *buffer;
- struct hv_kvp_ipaddr_value *ip_buffer;
+ struct hv_kvp_ipaddr_value *ip_buffer = NULL;
char cidr_mask[5]; /* /xyz */
int weight;
int i;
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 32/45] Input: hyperv-keyboard: Use in-place iterator API in the channel callback
From: Sasha Levin @ 2019-08-29 18:15 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Dexuan Cui, Dmitry Torokhov, Sasha Levin, linux-hyperv,
linux-input
In-Reply-To: <20190829181547.8280-1-sashal@kernel.org>
From: Dexuan Cui <decui@microsoft.com>
[ Upstream commit d09bc83640d524b8467a660db7b1d15e6562a1de ]
Simplify the ring buffer handling with the in-place API.
Also avoid the dynamic allocation and the memory leak in the channel
callback function.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/input/serio/hyperv-keyboard.c | 35 +++++----------------------
1 file changed, 6 insertions(+), 29 deletions(-)
diff --git a/drivers/input/serio/hyperv-keyboard.c b/drivers/input/serio/hyperv-keyboard.c
index a8b9be3e28db7..7d0a5ccf57751 100644
--- a/drivers/input/serio/hyperv-keyboard.c
+++ b/drivers/input/serio/hyperv-keyboard.c
@@ -245,40 +245,17 @@ static void hv_kbd_handle_received_packet(struct hv_device *hv_dev,
static void hv_kbd_on_channel_callback(void *context)
{
+ struct vmpacket_descriptor *desc;
struct hv_device *hv_dev = context;
- void *buffer;
- int bufferlen = 0x100; /* Start with sensible size */
u32 bytes_recvd;
u64 req_id;
- int error;
- buffer = kmalloc(bufferlen, GFP_ATOMIC);
- if (!buffer)
- return;
-
- while (1) {
- error = vmbus_recvpacket_raw(hv_dev->channel, buffer, bufferlen,
- &bytes_recvd, &req_id);
- switch (error) {
- case 0:
- if (bytes_recvd == 0) {
- kfree(buffer);
- return;
- }
-
- hv_kbd_handle_received_packet(hv_dev, buffer,
- bytes_recvd, req_id);
- break;
+ foreach_vmbus_pkt(desc, hv_dev->channel) {
+ bytes_recvd = desc->len8 * 8;
+ req_id = desc->trans_id;
- case -ENOBUFS:
- kfree(buffer);
- /* Handle large packet */
- bufferlen = bytes_recvd;
- buffer = kmalloc(bytes_recvd, GFP_ATOMIC);
- if (!buffer)
- return;
- break;
- }
+ hv_kbd_handle_received_packet(hv_dev, desc, bytes_recvd,
+ req_id);
}
}
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.14 02/27] hv_netvsc: Fix a warning of suspicious RCU usage
From: Sasha Levin @ 2019-08-29 18:16 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Dexuan Cui, David S . Miller, Sasha Levin, linux-hyperv, netdev
In-Reply-To: <20190829181655.8741-1-sashal@kernel.org>
From: Dexuan Cui <decui@microsoft.com>
[ Upstream commit 6d0d779dca73cd5acb649c54f81401f93098b298 ]
This fixes a warning of "suspicious rcu_dereference_check() usage"
when nload runs.
Fixes: 776e726bfb34 ("netvsc: fix RCU warning in get_stats")
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/hyperv/netvsc_drv.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index eb92720dd1c4a..33c1f6548fb79 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1170,12 +1170,15 @@ static void netvsc_get_stats64(struct net_device *net,
struct rtnl_link_stats64 *t)
{
struct net_device_context *ndev_ctx = netdev_priv(net);
- struct netvsc_device *nvdev = rcu_dereference_rtnl(ndev_ctx->nvdev);
+ struct netvsc_device *nvdev;
struct netvsc_vf_pcpu_stats vf_tot;
int i;
+ rcu_read_lock();
+
+ nvdev = rcu_dereference(ndev_ctx->nvdev);
if (!nvdev)
- return;
+ goto out;
netdev_stats_to_stats64(t, &net->stats);
@@ -1214,6 +1217,8 @@ static void netvsc_get_stats64(struct net_device *net,
t->rx_packets += packets;
t->multicast += multicast;
}
+out:
+ rcu_read_unlock();
}
static int netvsc_set_mac_addr(struct net_device *ndev, void *p)
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.14 22/27] Tools: hv: kvp: eliminate 'may be used uninitialized' warning
From: Sasha Levin @ 2019-08-29 18:16 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Vitaly Kuznetsov, Sasha Levin, linux-hyperv
In-Reply-To: <20190829181655.8741-1-sashal@kernel.org>
From: Vitaly Kuznetsov <vkuznets@redhat.com>
[ Upstream commit 89eb4d8d25722a0a0194cf7fa47ba602e32a6da7 ]
When building hv_kvp_daemon GCC-8.3 complains:
hv_kvp_daemon.c: In function ‘kvp_get_ip_info.constprop’:
hv_kvp_daemon.c:812:30: warning: ‘ip_buffer’ may be used uninitialized in this function [-Wmaybe-uninitialized]
struct hv_kvp_ipaddr_value *ip_buffer;
this seems to be a false positive: we only use ip_buffer when
op == KVP_OP_GET_IP_INFO and it is only unset when op == KVP_OP_ENUMERATE.
Silence the warning by initializing ip_buffer to NULL.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
tools/hv/hv_kvp_daemon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 62c9a503ae052..bb245d4afc0cf 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -867,7 +867,7 @@ kvp_get_ip_info(int family, char *if_name, int op,
int sn_offset = 0;
int error = 0;
char *buffer;
- struct hv_kvp_ipaddr_value *ip_buffer;
+ struct hv_kvp_ipaddr_value *ip_buffer = NULL;
char cidr_mask[5]; /* /xyz */
int weight;
int i;
--
2.20.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox