* RE: [PATCH] HID: hyperv: Use in-place iterator API in the channel callback
From: Jiri Kosina @ 2019-09-04 14:23 UTC (permalink / raw)
To: Dexuan Cui
Cc: benjamin.tissoires@redhat.com, linux-input@vger.kernel.org,
linux-hyperv@vger.kernel.org, Stephen Hemminger, Sasha Levin,
sashal@kernel.org, Haiyang Zhang, KY Srinivasan, Michael Kelley,
gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org
In-Reply-To: <KU1P153MB016679060F4360071B751AF0BFB90@KU1P153MB0166.APCP153.PROD.OUTLOOK.COM>
On Tue, 3 Sep 2019, Dexuan Cui wrote:
> > Hi Jiri, Benjamin, can this patch go through Sasha's hyperv tree:
> > https://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git
> >
> > This is a purely Hyper-V specific change.
>
> Hi Jiri, Benjamin,
> Are you OK if this patch for the Hyper-V HID driver goes through the Hyper-V
> tree maintained by Sasha Levin? It's a purely Hyper-V change, and I have
> been using the patch for several months and there is no regression.
No problem with that. Feel free to add
Acked-by: Jiri Kosina <jkosina@suse.cz>
in that case.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* [PATCH AUTOSEL 4.19 006/167] Drivers: hv: kvp: Fix two "this statement may fall through" warnings
From: Sasha Levin @ 2019-09-03 16:22 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Dexuan Cui, K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger,
Stable, Greg Kroah-Hartman, Sasha Levin, linux-hyperv
In-Reply-To: <20190903162519.7136-1-sashal@kernel.org>
From: Dexuan Cui <decui@microsoft.com>
[ Upstream commit fc62c3b1977d62e6374fd6e28d371bb42dfa5c9d ]
We don't need to call process_ib_ipinfo() if message->kvp_hdr.operation is
KVP_OP_GET_IP_INFO in kvp_send_key(), because here we just need to pass on
the op code from the host to the userspace; when the userspace returns
the info requested by the host, we pass the info on to the host in
kvp_respond_to_host() -> process_ob_ipinfo(). BTW, the current buggy code
actually doesn't cause any harm, because only message->kvp_hdr.operation
is used by the userspace, in the case of KVP_OP_GET_IP_INFO.
The patch also adds a missing "break;" in kvp_send_key(). BTW, the current
buggy code actually doesn't cause any harm, because in the case of
KVP_OP_SET, the unexpected fall-through corrupts
message->body.kvp_set.data.key_size, but that is not really used: see
the definition of struct hv_kvp_exchg_msg_value.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Cc: K. Y. Srinivasan <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: <Stable@vger.kernel.org>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hv/hv_kvp.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index 5eed1e7da15c4..57715a0c81202 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -353,7 +353,6 @@ static void process_ib_ipinfo(void *in_msg, void *out_msg, int op)
out->body.kvp_ip_val.dhcp_enabled = in->kvp_ip_val.dhcp_enabled;
- default:
utf16s_to_utf8s((wchar_t *)in->kvp_ip_val.adapter_id,
MAX_ADAPTER_ID_SIZE,
UTF16_LITTLE_ENDIAN,
@@ -406,7 +405,7 @@ kvp_send_key(struct work_struct *dummy)
process_ib_ipinfo(in_msg, message, KVP_OP_SET_IP_INFO);
break;
case KVP_OP_GET_IP_INFO:
- process_ib_ipinfo(in_msg, message, KVP_OP_GET_IP_INFO);
+ /* We only need to pass on message->kvp_hdr.operation. */
break;
case KVP_OP_SET:
switch (in_msg->body.kvp_set.data.value_type) {
@@ -446,6 +445,9 @@ kvp_send_key(struct work_struct *dummy)
break;
}
+
+ break;
+
case KVP_OP_GET:
message->body.kvp_set.data.key_size =
utf16s_to_utf8s(
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 020/167] Drivers: hv: kvp: Fix the indentation of some "break" statements
From: Sasha Levin @ 2019-09-03 16:22 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Dexuan Cui, K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger,
Greg Kroah-Hartman, Sasha Levin, linux-hyperv
In-Reply-To: <20190903162519.7136-1-sashal@kernel.org>
From: Dexuan Cui <decui@microsoft.com>
[ Upstream commit d544c22d6951be3386ac59bb9a99c9bc566b3f09 ]
No functional change.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Cc: K. Y. Srinivasan <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hv/hv_kvp.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index 57715a0c81202..a7513a8a8e372 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -420,7 +420,7 @@ kvp_send_key(struct work_struct *dummy)
UTF16_LITTLE_ENDIAN,
message->body.kvp_set.data.value,
HV_KVP_EXCHANGE_MAX_VALUE_SIZE - 1) + 1;
- break;
+ break;
case REG_U32:
/*
@@ -456,7 +456,7 @@ kvp_send_key(struct work_struct *dummy)
UTF16_LITTLE_ENDIAN,
message->body.kvp_set.data.key,
HV_KVP_EXCHANGE_MAX_KEY_SIZE - 1) + 1;
- break;
+ break;
case KVP_OP_DELETE:
message->body.kvp_delete.key_size =
@@ -466,12 +466,12 @@ kvp_send_key(struct work_struct *dummy)
UTF16_LITTLE_ENDIAN,
message->body.kvp_delete.key,
HV_KVP_EXCHANGE_MAX_KEY_SIZE - 1) + 1;
- break;
+ break;
case KVP_OP_ENUMERATE:
message->body.kvp_enum_data.index =
in_msg->body.kvp_enum_data.index;
- break;
+ break;
}
kvp_transaction.state = HVUTIL_USERSPACE_REQ;
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 021/167] Drivers: hv: kvp: Fix the recent regression caused by incorrect clean-up
From: Sasha Levin @ 2019-09-03 16:22 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Dexuan Cui, K . Y . Srinivasan, Stephen Hemminger, Haiyang Zhang,
Stable, Greg Kroah-Hartman, Sasha Levin, linux-hyperv
In-Reply-To: <20190903162519.7136-1-sashal@kernel.org>
From: Dexuan Cui <decui@microsoft.com>
[ Upstream commit e670de54c813b5bc3672dd1c67871dc60e9206f4 ]
In kvp_send_key(), we do need call process_ib_ipinfo() if
message->kvp_hdr.operation is KVP_OP_GET_IP_INFO, because it turns out
the userland hv_kvp_daemon needs the info of operation, adapter_id and
addr_family. With the incorrect fc62c3b1977d, the host can't get the
VM's IP via KVP.
And, fc62c3b1977d added a "break;", but actually forgot to initialize
the key_size/value in the case of KVP_OP_SET, so the default key_size of
0 is passed to the kvp daemon, and the pool files
/var/lib/hyperv/.kvp_pool_* can't be updated.
This patch effectively rolls back the previous fc62c3b1977d, and
correctly fixes the "this statement may fall through" warnings.
This patch is tested on WS 2012 R2 and 2016.
Fixes: fc62c3b1977d ("Drivers: hv: kvp: Fix two "this statement may fall through" warnings")
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Cc: K. Y. Srinivasan <kys@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Cc: <Stable@vger.kernel.org>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hv/hv_kvp.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index a7513a8a8e372..d6106e1a0d4af 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -353,6 +353,9 @@ static void process_ib_ipinfo(void *in_msg, void *out_msg, int op)
out->body.kvp_ip_val.dhcp_enabled = in->kvp_ip_val.dhcp_enabled;
+ /* fallthrough */
+
+ case KVP_OP_GET_IP_INFO:
utf16s_to_utf8s((wchar_t *)in->kvp_ip_val.adapter_id,
MAX_ADAPTER_ID_SIZE,
UTF16_LITTLE_ENDIAN,
@@ -405,7 +408,11 @@ kvp_send_key(struct work_struct *dummy)
process_ib_ipinfo(in_msg, message, KVP_OP_SET_IP_INFO);
break;
case KVP_OP_GET_IP_INFO:
- /* We only need to pass on message->kvp_hdr.operation. */
+ /*
+ * We only need to pass on the info of operation, adapter_id
+ * and addr_family to the userland kvp daemon.
+ */
+ process_ib_ipinfo(in_msg, message, KVP_OP_GET_IP_INFO);
break;
case KVP_OP_SET:
switch (in_msg->body.kvp_set.data.value_type) {
@@ -446,9 +453,9 @@ kvp_send_key(struct work_struct *dummy)
}
- break;
-
- case KVP_OP_GET:
+ /*
+ * The key is always a string - utf16 encoding.
+ */
message->body.kvp_set.data.key_size =
utf16s_to_utf8s(
(wchar_t *)in_msg->body.kvp_set.data.key,
@@ -456,6 +463,17 @@ kvp_send_key(struct work_struct *dummy)
UTF16_LITTLE_ENDIAN,
message->body.kvp_set.data.key,
HV_KVP_EXCHANGE_MAX_KEY_SIZE - 1) + 1;
+
+ break;
+
+ case KVP_OP_GET:
+ message->body.kvp_get.data.key_size =
+ utf16s_to_utf8s(
+ (wchar_t *)in_msg->body.kvp_get.data.key,
+ in_msg->body.kvp_get.data.key_size,
+ UTF16_LITTLE_ENDIAN,
+ message->body.kvp_get.data.key,
+ HV_KVP_EXCHANGE_MAX_KEY_SIZE - 1) + 1;
break;
case KVP_OP_DELETE:
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] irqdomain: Add the missing assignment of domain->fwnode for named fwnode
From: Marc Zyngier @ 2019-09-03 8:22 UTC (permalink / raw)
To: Dexuan Cui, Thomas Gleixner
Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Lorenzo Pieralisi, Bjorn Helgaas, linux-kernel@vger.kernel.org,
linux-hyperv@vger.kernel.org, Michael Kelley,
Lili Deng (Wicresoft North America Ltd)
In-Reply-To: <PU1P153MB01694D9AF625AC335C600C5FBFBE0@PU1P153MB0169.APCP153.PROD.OUTLOOK.COM>
Hi Dexuan,
On 03/09/2019 00:14, Dexuan Cui wrote:
>
> Recently device pass-through stops working for Linux VM running on Hyper-V.
>
> git-bisect shows the regression is caused by the recent commit
> 467a3bb97432 ("PCI: hv: Allocate a named fwnode ..."), but the root cause
> is that the commit d59f6617eef0 forgets to set the domain->fwnode for
> IRQCHIP_FWNODE_NAMED*, and as a result:
>
> 1. The domain->fwnode remains to be NULL.
>
> 2. irq_find_matching_fwspec() returns NULL since "h->fwnode == fwnode" is
> false, and pci_set_bus_msi_domain() sets the Hyper-V PCI root bus's
> msi_domain to NULL.
>
> 3. When the device is added onto the root bus, the device's dev->msi_domain
> is set to NULL in pci_set_msi_domain().
>
> 4. When a device driver tries to enable MSI-X, pci_msi_setup_msi_irqs()
> calls arch_setup_msi_irqs(), which uses the native MSI chip (i.e.
> arch/x86/kernel/apic/msi.c: pci_msi_controller) to set up the irqs, but
> actually pci_msi_setup_msi_irqs() is supposed to call
> msi_domain_alloc_irqs() with the hbus->irq_domain, which is created in
> hv_pcie_init_irq_domain() and is associated with the Hyper-V chip
> hv_msi_irq_chip. Consequently, the irq line is not properly set up, and
> the device driver can not receive any interrupt.
>
> Fixes: d59f6617eef0 ("genirq: Allow fwnode to carry name information only")
> Fixes: 467a3bb97432 ("PCI: hv: Allocate a named fwnode instead of an address-based one")
> Reported-by: Lili Deng <v-lide@microsoft.com>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
>
> Note: the commit 467a3bb97432 ("PCI: hv: Allocate a named fwnode ...") has not
> gone in Linus's tree yet (the commit is in linux-next for a while), so the commit ID
> in the changelog can change when it goes in Linus's tree.
This branch is supposed to be stable, and I try to only apply fixes to
it. This normally ensures that commit IDs are the same once they land in
Linus' tree.
> This patch works in my test, but I'm not 100% sure this is the right fix.
>
> Looking forward to your comment!
>
> kernel/irq/irqdomain.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index e7bbab149750..132672b74e4b 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -149,6 +149,7 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, int size,
> switch (fwid->type) {
> case IRQCHIP_FWNODE_NAMED:
> case IRQCHIP_FWNODE_NAMED_ID:
> + domain->fwnode = fwnode;
> domain->name = kstrdup(fwid->name, GFP_KERNEL);
> if (!domain->name) {
> kfree(domain);
>
Looks absolutely correct to me, thanks for fixing it. I've applied it on
top of irqchip-next.
Thanks,
M.
--
Jazz is not dead, it just smells funny...
^ permalink raw reply
* RE: [PATCH] HID: hyperv: Use in-place iterator API in the channel callback
From: Dexuan Cui @ 2019-09-03 0:43 UTC (permalink / raw)
To: jikos@kernel.org, benjamin.tissoires@redhat.com,
linux-input@vger.kernel.org, linux-hyperv@vger.kernel.org,
Stephen Hemminger, Sasha Levin, sashal@kernel.org, Haiyang Zhang,
KY Srinivasan, Michael Kelley
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org
In-Reply-To: <1566269763-26817-1-git-send-email-decui@microsoft.com>
> -----Original Message-----
> From: Dexuan Cui <decui@microsoft.com>
> Sent: Monday, August 19, 2019 7:57 PM
> To: jikos@kernel.org; benjamin.tissoires@redhat.com;
> linux-input@vger.kernel.org; linux-hyperv@vger.kernel.org; Stephen
> Hemminger <sthemmin@microsoft.com>; Sasha Levin
> <Alexander.Levin@microsoft.com>; sashal@kernel.org; Haiyang Zhang
> <haiyangz@microsoft.com>; KY Srinivasan <kys@microsoft.com>; Michael
> Kelley <mikelley@microsoft.com>
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; Dexuan Cui
> <decui@microsoft.com>
> Subject: [PATCH] HID: hyperv: Use in-place iterator API in the channel callback
>
> 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>
> ---
>
> Hi Jiri, Benjamin, can this patch go through Sasha's hyperv tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git
>
> This is a purely Hyper-V specific change.
Hi Jiri, Benjamin,
Are you OK if this patch for the Hyper-V HID driver goes through the Hyper-V
tree maintained by Sasha Levin? It's a purely Hyper-V change, and I have
been using the patch for several months and there is no regression.
Thanks,
-- Dexuan
^ permalink raw reply
* RE: [PATCH v2] PCI: PM: Move to D0 before calling pci_legacy_resume_early()
From: Dexuan Cui @ 2019-09-03 0:34 UTC (permalink / raw)
To: lorenzo.pieralisi@arm.com, bhelgaas@google.com,
linux-pci@vger.kernel.org
Cc: Michael Kelley, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org,
driverdev-devel@linuxdriverproject.org, Sasha Levin,
Haiyang Zhang, KY Srinivasan, olaf@aepfle.de, apw@canonical.com,
jasowang@redhat.com, vkuznets, marcelo.cerri@canonical.com,
Stephen Hemminger, jackm@mellanox.com
In-Reply-To: <KU1P153MB016637CAEAD346F0AA8E3801BFAD0@KU1P153MB0166.APCP153.PROD.OUTLOOK.COM>
> From: Dexuan Cui <decui@microsoft.com>
> Sent: Tuesday, August 13, 2019 6:07 PM
> To: lorenzo.pieralisi@arm.com; bhelgaas@google.com;
> linux-pci@vger.kernel.org
> Cc: Michael Kelley <mikelley@microsoft.com>; linux-hyperv@vger.kernel.org;
> linux-kernel@vger.kernel.org; driverdev-devel@linuxdriverproject.org; Sasha
> Levin <Alexander.Levin@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; KY Srinivasan <kys@microsoft.com>;
> olaf@aepfle.de; apw@canonical.com; jasowang@redhat.com; vkuznets
> <vkuznets@redhat.com>; marcelo.cerri@canonical.com; Stephen Hemminger
> <sthemmin@microsoft.com>; jackm@mellanox.com; Dexuan Cui
> <decui@microsoft.com>
> Subject: [PATCH v2] PCI: PM: Move to D0 before calling
> pci_legacy_resume_early()
>
>
> In pci_legacy_suspend_late(), the device state is moved to PCI_UNKNOWN.
>
> In pci_pm_thaw_noirq(), the state is supposed to be moved back to PCI_D0,
> but the current code misses the pci_legacy_resume_early() path, so the
> state remains in PCI_UNKNOWN in that path. As a result, in the resume
> phase of hibernation, this causes an error for the Mellanox VF driver,
> which fails to enable MSI-X because pci_msi_supported() is false due
> to dev->current_state != PCI_D0:
>
> mlx4_core a6d1:00:02.0: Detected virtual function - running in slave mode
> mlx4_core a6d1:00:02.0: Sending reset
> mlx4_core a6d1:00:02.0: Sending vhcr0
> mlx4_core a6d1:00:02.0: HCA minimum page size:512
> mlx4_core a6d1:00:02.0: Timestamping is not supported in slave mode
> mlx4_core a6d1:00:02.0: INTx is not supported in multi-function mode,
> aborting
> PM: dpm_run_callback(): pci_pm_thaw+0x0/0xd7 returns -95
> PM: Device a6d1:00:02.0 failed to thaw: error -95
>
> To be more accurate, the "resume" phase means the "thaw" callbacks which
> run before the system enters hibernation: when the user runs the command
> "echo disk > /sys/power/state" for hibernation, first the kernel "freezes"
> all the devices and creates a hibernation image, then the kernel "thaws"
> the devices including the disk/NIC, writes the memory to the disk, and
> powers down. This patch fixes the error message for the Mellanox VF driver
> in this phase.
>
> When the system starts again, a fresh kernel starts to run, and when the
> kernel detects that a hibernation image was saved, the kernel "quiesces"
> the devices, and then "restores" the devices from the saved image. In this
> path:
> device_resume_noirq() -> ... ->
> pci_pm_restore_noirq() ->
> pci_pm_default_resume_early() ->
> pci_power_up() moves the device states back to PCI_D0. This path is
> not broken and doesn't need my patch.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
>
> changes in v2:
> Updated the changelog with more details.
>
> drivers/pci/pci-driver.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 36dbe960306b..27dfc68db9e7 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -1074,15 +1074,16 @@ static int pci_pm_thaw_noirq(struct device *dev)
> return error;
> }
>
> - if (pci_has_legacy_pm_support(pci_dev))
> - return pci_legacy_resume_early(dev);
> -
> /*
> * pci_restore_state() requires the device to be in D0 (because of MSI
> * restoration among other things), so force it into D0 in case the
> * driver's "freeze" callbacks put it into a low-power state directly.
> */
> pci_set_power_state(pci_dev, PCI_D0);
> +
> + if (pci_has_legacy_pm_support(pci_dev))
> + return pci_legacy_resume_early(dev);
> +
> pci_restore_state(pci_dev);
>
> if (drv && drv->pm && drv->pm->thaw_noirq)
> --
Hi, Lorenzo, Bjorn,
Can you please take a look at the v2 ?
Thanks,
-- Dexuan
^ permalink raw reply
* Re: [PATCH v4 4/5] HID: hv: Remove dependencies on PAGE_SIZE for ring buffer
From: Sasha Levin @ 2019-09-03 0:26 UTC (permalink / raw)
To: Jiri Kosina
Cc: Michael Kelley, m.maya.nakamura, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, benjamin.tissoires@redhat.com, x86@kernel.org,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <nycvar.YFH.7.76.1909021330230.27147@cbobk.fhfr.pm>
On Mon, Sep 02, 2019 at 01:30:44PM +0200, Jiri Kosina wrote:
>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>
Queued up for hyperv-next, thanks!
--
Thanks,
Sasha
^ permalink raw reply
* [PATCH v4 01/12] x86/hyper-v: Suspend/resume the hypercall page for hibernation
From: Dexuan Cui @ 2019-09-03 0:23 UTC (permalink / raw)
To: linux-hyperv@vger.kernel.org, gregkh@linuxfoundation.org,
Stephen Hemminger, Sasha Levin, sashal@kernel.org, Haiyang Zhang,
KY Srinivasan, Michael Kelley, tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org, Dexuan Cui
In-Reply-To: <1567470139-119355-1-git-send-email-decui@microsoft.com>
This is needed for hibernation, e.g. when we resume the old kernel, we need
to disable the "current" kernel's hypercall page and then resume the old
kernel's.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
---
arch/x86/hyperv/hv_init.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 0d25868..78e53d9 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -20,6 +20,7 @@
#include <linux/hyperv.h>
#include <linux/slab.h>
#include <linux/cpuhotplug.h>
+#include <linux/syscore_ops.h>
#include <clocksource/hyperv_timer.h>
void *hv_hypercall_pg;
@@ -223,6 +224,34 @@ static int __init hv_pci_init(void)
return 1;
}
+static int hv_suspend(void)
+{
+ union hv_x64_msr_hypercall_contents hypercall_msr;
+
+ /* Reset the hypercall page */
+ rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
+ hypercall_msr.enable = 0;
+ wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
+
+ return 0;
+}
+
+static void hv_resume(void)
+{
+ union hv_x64_msr_hypercall_contents hypercall_msr;
+
+ /* Re-enable the hypercall page */
+ rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
+ hypercall_msr.enable = 1;
+ hypercall_msr.guest_physical_address = vmalloc_to_pfn(hv_hypercall_pg);
+ wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
+}
+
+static struct syscore_ops hv_syscore_ops = {
+ .suspend = hv_suspend,
+ .resume = hv_resume,
+};
+
/*
* This function is to be invoked early in the boot sequence after the
* hypervisor has been detected.
@@ -303,6 +332,9 @@ void __init hyperv_init(void)
/* Register Hyper-V specific clocksource */
hv_init_clocksource();
+
+ register_syscore_ops(&hv_syscore_ops);
+
return;
remove_cpuhp_state:
@@ -322,6 +354,8 @@ void hyperv_cleanup(void)
{
union hv_x64_msr_hypercall_contents hypercall_msr;
+ unregister_syscore_ops(&hv_syscore_ops);
+
/* Reset our OS id */
wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
--
1.8.3.1
^ permalink raw reply related
* [PATCH v4 02/12] x86/hyper-v: Implement hv_is_hibernation_supported()
From: Dexuan Cui @ 2019-09-03 0:23 UTC (permalink / raw)
To: linux-hyperv@vger.kernel.org, gregkh@linuxfoundation.org,
Stephen Hemminger, Sasha Levin, sashal@kernel.org, Haiyang Zhang,
KY Srinivasan, Michael Kelley, tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org, Dexuan Cui
In-Reply-To: <1567470139-119355-1-git-send-email-decui@microsoft.com>
The API will be used by the hv_balloon and hv_vmbus drivers.
Balloon up/down and hot-add of memory must not be active if the user
wants the Linux VM to support hibernation, because they are incompatible
with hibernation according to Hyper-V team, e.g. upon suspend the
balloon VSP doesn't save any info about the 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 do that, since the VSP thinks it never stole the pages from the VM.
So, if the user wants Linux VM to support hibernation, Linux must forbid
balloon up/down and hot-add, and the only functionality of the balloon VSC
driver is reporting the VM's 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-add. 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 with the later changes to the
VSC driver, Linux VM still reports to the VSP that the VSC supports these
capabilities, but the VSC ignores the VSP's requests of balloon up/down
and hot add, and reports 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 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. However in practice
we decide to artificially use the presence of the virtual ACPI S4 state as
an indicator of the user's intent of using hibernation, because Linux VM
must find a way to know if the user wants to use the hibernation feature
or not.
By default, Hyper-V does not enable the virtual ACPI S4 state; on recent
Hyper-V hosts (e.g. RS5, 19H1), the administrator is able to enable the
state for a VM by WMI commands.
Once all the vmbus and VSC patches for the hibernation feature are
accepted, an extra patch will be submitted to forbid hibernation if the
virtual ACPI S4 state is absent, i.e. hv_is_hibernation_supported() is
false.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
arch/x86/hyperv/hv_init.c | 7 +++++++
include/asm-generic/mshyperv.h | 2 ++
2 files changed, 9 insertions(+)
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 78e53d9..6735e45 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -7,6 +7,7 @@
* Author : K. Y. Srinivasan <kys@microsoft.com>
*/
+#include <linux/acpi.h>
#include <linux/efi.h>
#include <linux/types.h>
#include <asm/apic.h>
@@ -453,3 +454,9 @@ bool hv_is_hyperv_initialized(void)
return hypercall_msr.enable;
}
EXPORT_SYMBOL_GPL(hv_is_hyperv_initialized);
+
+bool hv_is_hibernation_supported(void)
+{
+ return acpi_sleep_state_supported(ACPI_STATE_S4);
+}
+EXPORT_SYMBOL_GPL(hv_is_hibernation_supported);
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index 0becb7d..1cb4001 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -166,9 +166,11 @@ static inline int cpumask_to_vpset(struct hv_vpset *vpset,
void hyperv_report_panic(struct pt_regs *regs, long err);
void hyperv_report_panic_msg(phys_addr_t pa, size_t size);
bool hv_is_hyperv_initialized(void);
+bool hv_is_hibernation_supported(void);
void hyperv_cleanup(void);
#else /* CONFIG_HYPERV */
static inline bool hv_is_hyperv_initialized(void) { return false; }
+static inline bool hv_is_hibernation_supported(void) { return false; }
static inline void hyperv_cleanup(void) {}
#endif /* CONFIG_HYPERV */
--
1.8.3.1
^ permalink raw reply related
* [PATCH v4 03/12] clocksource/drivers: Suspend/resume Hyper-V clocksource for hibernation
From: Dexuan Cui @ 2019-09-03 0:23 UTC (permalink / raw)
To: linux-hyperv@vger.kernel.org, gregkh@linuxfoundation.org,
Stephen Hemminger, Sasha Levin, sashal@kernel.org, Haiyang Zhang,
KY Srinivasan, Michael Kelley, tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org, Dexuan Cui
In-Reply-To: <1567470139-119355-1-git-send-email-decui@microsoft.com>
This is needed for hibernation, e.g. when we resume the old kernel, we need
to disable the "current" kernel's TSC page and then resume the old kernel's.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
---
drivers/clocksource/hyperv_timer.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
index 17b96f9..8f3422c 100644
--- a/drivers/clocksource/hyperv_timer.c
+++ b/drivers/clocksource/hyperv_timer.c
@@ -238,12 +238,37 @@ static u64 read_hv_clock_tsc(struct clocksource *arg)
return read_hv_sched_clock_tsc();
}
+static void suspend_hv_clock_tsc(struct clocksource *arg)
+{
+ u64 tsc_msr;
+
+ /* Disable the TSC page */
+ hv_get_reference_tsc(tsc_msr);
+ tsc_msr &= ~BIT_ULL(0);
+ hv_set_reference_tsc(tsc_msr);
+}
+
+
+static void resume_hv_clock_tsc(struct clocksource *arg)
+{
+ phys_addr_t phys_addr = page_to_phys(vmalloc_to_page(tsc_pg));
+ u64 tsc_msr;
+
+ /* Re-enable the TSC page */
+ hv_get_reference_tsc(tsc_msr);
+ tsc_msr &= GENMASK_ULL(11, 0);
+ tsc_msr |= BIT_ULL(0) | (u64)phys_addr;
+ hv_set_reference_tsc(tsc_msr);
+}
+
static struct clocksource hyperv_cs_tsc = {
.name = "hyperv_clocksource_tsc_page",
.rating = 400,
.read = read_hv_clock_tsc,
.mask = CLOCKSOURCE_MASK(64),
.flags = CLOCK_SOURCE_IS_CONTINUOUS,
+ .suspend= suspend_hv_clock_tsc,
+ .resume = resume_hv_clock_tsc,
};
#endif
--
1.8.3.1
^ permalink raw reply related
* [PATCH v4 04/12] Drivers: hv: vmbus: Break out synic enable and disable operations
From: Dexuan Cui @ 2019-09-03 0:23 UTC (permalink / raw)
To: linux-hyperv@vger.kernel.org, gregkh@linuxfoundation.org,
Stephen Hemminger, Sasha Levin, sashal@kernel.org, Haiyang Zhang,
KY Srinivasan, Michael Kelley, tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org, Dexuan Cui
In-Reply-To: <1567470139-119355-1-git-send-email-decui@microsoft.com>
Break out synic enable and disable operations into separate
hv_synic_disable_regs() and hv_synic_enable_regs() functions for use by a
later patch to support hibernation.
There is no functional change except the unnecessary check
"if (sctrl.enable != 1) return -EFAULT;" which is removed, because when
we're in hv_synic_cleanup(), we're absolutely sure sctrl.enable must be 1.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
---
drivers/hv/hv.c | 66 ++++++++++++++++++++++++++---------------------
drivers/hv/hyperv_vmbus.h | 2 ++
2 files changed, 39 insertions(+), 29 deletions(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 6188fb7..fcc5279 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -154,7 +154,7 @@ void hv_synic_free(void)
* retrieve the initialized message and event pages. Otherwise, we create and
* initialize the message and event pages.
*/
-int hv_synic_init(unsigned int cpu)
+void hv_synic_enable_regs(unsigned int cpu)
{
struct hv_per_cpu_context *hv_cpu
= per_cpu_ptr(hv_context.cpu_context, cpu);
@@ -196,6 +196,11 @@ int hv_synic_init(unsigned int cpu)
sctrl.enable = 1;
hv_set_synic_state(sctrl.as_uint64);
+}
+
+int hv_synic_init(unsigned int cpu)
+{
+ hv_synic_enable_regs(cpu);
hv_stimer_init(cpu);
@@ -205,20 +210,45 @@ int hv_synic_init(unsigned int cpu)
/*
* hv_synic_cleanup - Cleanup routine for hv_synic_init().
*/
-int hv_synic_cleanup(unsigned int cpu)
+void hv_synic_disable_regs(unsigned int cpu)
{
union hv_synic_sint shared_sint;
union hv_synic_simp simp;
union hv_synic_siefp siefp;
union hv_synic_scontrol sctrl;
+
+ hv_get_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
+
+ shared_sint.masked = 1;
+
+ /* Need to correctly cleanup in the case of SMP!!! */
+ /* Disable the interrupt */
+ hv_set_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
+
+ hv_get_simp(simp.as_uint64);
+ simp.simp_enabled = 0;
+ simp.base_simp_gpa = 0;
+
+ hv_set_simp(simp.as_uint64);
+
+ hv_get_siefp(siefp.as_uint64);
+ siefp.siefp_enabled = 0;
+ siefp.base_siefp_gpa = 0;
+
+ hv_set_siefp(siefp.as_uint64);
+
+ /* Disable the global synic bit */
+ hv_get_synic_state(sctrl.as_uint64);
+ sctrl.enable = 0;
+ hv_set_synic_state(sctrl.as_uint64);
+}
+
+int hv_synic_cleanup(unsigned int cpu)
+{
struct vmbus_channel *channel, *sc;
bool channel_found = false;
unsigned long flags;
- hv_get_synic_state(sctrl.as_uint64);
- if (sctrl.enable != 1)
- 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
@@ -249,29 +279,7 @@ int hv_synic_cleanup(unsigned int cpu)
hv_stimer_cleanup(cpu);
- hv_get_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
-
- shared_sint.masked = 1;
-
- /* Need to correctly cleanup in the case of SMP!!! */
- /* Disable the interrupt */
- hv_set_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
-
- hv_get_simp(simp.as_uint64);
- simp.simp_enabled = 0;
- simp.base_simp_gpa = 0;
-
- hv_set_simp(simp.as_uint64);
-
- hv_get_siefp(siefp.as_uint64);
- siefp.siefp_enabled = 0;
- siefp.base_siefp_gpa = 0;
-
- hv_set_siefp(siefp.as_uint64);
-
- /* Disable the global synic bit */
- sctrl.enable = 0;
- hv_set_synic_state(sctrl.as_uint64);
+ hv_synic_disable_regs(cpu);
return 0;
}
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index fb16a62..9f7fb6d 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -169,8 +169,10 @@ extern int hv_post_message(union hv_connection_id connection_id,
extern void hv_synic_free(void);
+extern void hv_synic_enable_regs(unsigned int cpu);
extern int hv_synic_init(unsigned int cpu);
+extern void hv_synic_disable_regs(unsigned int cpu);
extern int hv_synic_cleanup(unsigned int cpu);
/* Interface */
--
1.8.3.1
^ permalink raw reply related
* [PATCH v4 06/12] Drivers: hv: vmbus: Add a helper function is_sub_channel()
From: Dexuan Cui @ 2019-09-03 0:23 UTC (permalink / raw)
To: linux-hyperv@vger.kernel.org, gregkh@linuxfoundation.org,
Stephen Hemminger, Sasha Levin, sashal@kernel.org, Haiyang Zhang,
KY Srinivasan, Michael Kelley, tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org, Dexuan Cui
In-Reply-To: <1567470139-119355-1-git-send-email-decui@microsoft.com>
The existing method of telling if a channel is sub-channel in
vmbus_process_offer() is cumbersome. This new simple helper function
is preferred in future.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
---
include/linux/hyperv.h | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 6256cc3..2d39248 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -245,7 +245,10 @@ struct vmbus_channel_offer {
} pipe;
} u;
/*
- * The sub_channel_index is defined in win8.
+ * The sub_channel_index is defined in Win8: a value of zero means a
+ * primary channel and a value of non-zero means a sub-channel.
+ *
+ * Before Win8, the field is reserved, meaning it's always zero.
*/
u16 sub_channel_index;
u16 reserved3;
@@ -934,6 +937,11 @@ static inline bool is_hvsock_channel(const struct vmbus_channel *c)
VMBUS_CHANNEL_TLNPI_PROVIDER_OFFER);
}
+static inline bool is_sub_channel(const struct vmbus_channel *c)
+{
+ return c->offermsg.offer.sub_channel_index != 0;
+}
+
static inline void set_channel_affinity_state(struct vmbus_channel *c,
enum hv_numa_policy policy)
{
--
1.8.3.1
^ permalink raw reply related
* [PATCH v4 09/12] Drivers: hv: vmbus: Suspend/resume the vmbus itself for hibernation
From: Dexuan Cui @ 2019-09-03 0:23 UTC (permalink / raw)
To: linux-hyperv@vger.kernel.org, gregkh@linuxfoundation.org,
Stephen Hemminger, Sasha Levin, sashal@kernel.org, Haiyang Zhang,
KY Srinivasan, Michael Kelley, tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org, Dexuan Cui
In-Reply-To: <1567470139-119355-1-git-send-email-decui@microsoft.com>
Before Linux enters hibernation, it sends the CHANNELMSG_UNLOAD message to
the host so all the offers are gone. After hibernation, Linux needs to
re-negotiate with the host using the same vmbus protocol version (which
was in use before hibernation), and ask the host to re-offer the vmbus
devices.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
---
drivers/hv/connection.c | 3 +--
drivers/hv/hyperv_vmbus.h | 2 ++
drivers/hv/vmbus_drv.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 62 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 09829e1..806319c 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -59,8 +59,7 @@ static __u32 vmbus_get_next_version(__u32 current_version)
}
}
-static int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo,
- __u32 version)
+int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
{
int ret = 0;
unsigned int cur_cpu;
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 9f7fb6d..613888e 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -272,6 +272,8 @@ struct vmbus_msginfo {
extern struct vmbus_connection vmbus_connection;
+int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version);
+
static inline void vmbus_send_interrupt(u32 relid)
{
sync_set_bit(relid, vmbus_connection.send_int_page);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index a30c70a..ce9974b 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -2089,6 +2089,51 @@ static int vmbus_acpi_add(struct acpi_device *device)
return ret_val;
}
+static int vmbus_bus_suspend(struct device *dev)
+{
+ vmbus_initiate_unload(false);
+
+ vmbus_connection.conn_state = DISCONNECTED;
+
+ return 0;
+}
+
+static int vmbus_bus_resume(struct device *dev)
+{
+ struct vmbus_channel_msginfo *msginfo;
+ size_t msgsize;
+ int ret;
+
+ /*
+ * We only use the 'vmbus_proto_version', which was in use before
+ * hibernation, to re-negotiate with the host.
+ */
+ if (vmbus_proto_version == VERSION_INVAL ||
+ vmbus_proto_version == 0) {
+ pr_err("Invalid proto version = 0x%x\n", vmbus_proto_version);
+ return -EINVAL;
+ }
+
+ msgsize = sizeof(*msginfo) +
+ sizeof(struct vmbus_channel_initiate_contact);
+
+ msginfo = kzalloc(msgsize, GFP_KERNEL);
+
+ if (msginfo == NULL)
+ return -ENOMEM;
+
+ ret = vmbus_negotiate_version(msginfo, vmbus_proto_version);
+
+ kfree(msginfo);
+
+ if (ret != 0)
+ return ret;
+
+ vmbus_request_offers();
+
+ return 0;
+}
+
static const struct acpi_device_id vmbus_acpi_device_ids[] = {
{"VMBUS", 0},
{"VMBus", 0},
@@ -2096,6 +2141,19 @@ static int vmbus_acpi_add(struct acpi_device *device)
};
MODULE_DEVICE_TABLE(acpi, vmbus_acpi_device_ids);
+/*
+ * Note: we must use SET_NOIRQ_SYSTEM_SLEEP_PM_OPS rather than
+ * SET_SYSTEM_SLEEP_PM_OPS, otherwise NIC SR-IOV can not work, because the
+ * "pci_dev_pm_ops" uses the "noirq" callbacks: in the resume path, the
+ * pci "noirq" restore callback runs before "non-noirq" callbacks (see
+ * resume_target_kernel() -> dpm_resume_start(), and hibernation_restore() ->
+ * dpm_resume_end()). This means vmbus_bus_resume() and the pci-hyperv's
+ * resume callback must also run via the "noirq" callbacks.
+ */
+static const struct dev_pm_ops vmbus_bus_pm = {
+ SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(vmbus_bus_suspend, vmbus_bus_resume)
+};
+
static struct acpi_driver vmbus_acpi_driver = {
.name = "vmbus",
.ids = vmbus_acpi_device_ids,
@@ -2103,6 +2161,7 @@ static int vmbus_acpi_add(struct acpi_device *device)
.add = vmbus_acpi_add,
.remove = vmbus_acpi_remove,
},
+ .drv.pm = &vmbus_bus_pm,
};
static void hv_kexec_handler(void)
--
1.8.3.1
^ permalink raw reply related
* [PATCH v4 12/12] Drivers: hv: vmbus: Resume after fixing up old primary channels
From: Dexuan Cui @ 2019-09-03 0:23 UTC (permalink / raw)
To: linux-hyperv@vger.kernel.org, gregkh@linuxfoundation.org,
Stephen Hemminger, Sasha Levin, sashal@kernel.org, Haiyang Zhang,
KY Srinivasan, Michael Kelley, tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org, Dexuan Cui
In-Reply-To: <1567470139-119355-1-git-send-email-decui@microsoft.com>
When the host re-offers the primary channels upon resume, the host only
guarantees the Instance GUID doesn't change, so vmbus_bus_suspend()
should invalidate channel->offermsg.child_relid and figure out the
number of primary channels that need to be fixed up upon resume.
Upon resume, vmbus_onoffer() finds the old channel structs, and maps
the new offers to the old channels, and fixes up the old structs,
and finally the resume callbacks of the VSC drivers will re-open
the channels.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
drivers/hv/channel_mgmt.c | 85 ++++++++++++++++++++++++++++++++++++-----------
drivers/hv/connection.c | 2 ++
drivers/hv/hyperv_vmbus.h | 14 ++++++++
drivers/hv/vmbus_drv.c | 17 ++++++++++
include/linux/hyperv.h | 3 ++
5 files changed, 101 insertions(+), 20 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 5518d03..8eb1675 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -407,7 +407,15 @@ void hv_process_channel_removal(struct vmbus_channel *channel)
cpumask_clear_cpu(channel->target_cpu,
&primary_channel->alloced_cpus_in_node);
- vmbus_release_relid(channel->offermsg.child_relid);
+ /*
+ * Upon suspend, an in-use hv_sock channel is marked as "rescinded" and
+ * the relid is invalidated; after hibernation, when the user-space app
+ * destroys the channel, the relid is INVALID_RELID, and in this case
+ * it's unnecessary and unsafe to release the old relid, since the same
+ * relid can refer to a completely different channel now.
+ */
+ if (channel->offermsg.child_relid != INVALID_RELID)
+ vmbus_release_relid(channel->offermsg.child_relid);
free_channel(channel);
}
@@ -851,6 +859,36 @@ void vmbus_initiate_unload(bool crash)
vmbus_wait_for_unload();
}
+static void check_ready_for_resume_event(void)
+{
+ /*
+ * If all the old primary channels have been fixed up, then it's safe
+ * to resume.
+ */
+ if (atomic_dec_and_test(&vmbus_connection.nr_chan_fixup_on_resume))
+ complete(&vmbus_connection.ready_for_resume_event);
+}
+
+static void vmbus_setup_channel_state(struct vmbus_channel *channel,
+ struct vmbus_channel_offer_channel *offer)
+{
+ /*
+ * Setup state for signalling the host.
+ */
+ channel->sig_event = VMBUS_EVENT_CONNECTION_ID;
+
+ if (vmbus_proto_version != VERSION_WS2008) {
+ channel->is_dedicated_interrupt =
+ (offer->is_dedicated_interrupt != 0);
+ channel->sig_event = offer->connection_id;
+ }
+
+ memcpy(&channel->offermsg, offer,
+ sizeof(struct vmbus_channel_offer_channel));
+ channel->monitor_grp = (u8)offer->monitorid / 32;
+ channel->monitor_bit = (u8)offer->monitorid % 32;
+}
+
/*
* find_primary_channel_by_offer - Get the channel object given the new offer.
* This is only used in the resume path of hibernation.
@@ -902,14 +940,29 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
atomic_dec(&vmbus_connection.offer_in_progress);
/*
- * We're resuming from hibernation: we expect the host to send
- * exactly the same offers that we had before the hibernation.
+ * We're resuming from hibernation: all the sub-channel and
+ * hv_sock channels we had before the hibernation should have
+ * been cleaned up, and now we must be seeing a re-offered
+ * primary channel that we had before the hibernation.
*/
+
+ WARN_ON(oldchannel->offermsg.child_relid != INVALID_RELID);
+ /* Fix up the relid. */
+ oldchannel->offermsg.child_relid = offer->child_relid;
+
offer_sz = sizeof(*offer);
- if (memcmp(offer, &oldchannel->offermsg, offer_sz) == 0)
+ if (memcmp(offer, &oldchannel->offermsg, offer_sz) == 0) {
+ check_ready_for_resume_event();
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,
@@ -917,6 +970,12 @@ 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);
+
+ /* Fix up the old channel. */
+ vmbus_setup_channel_state(oldchannel, offer);
+
+ check_ready_for_resume_event();
+
return;
}
@@ -929,21 +988,7 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
return;
}
- /*
- * Setup state for signalling the host.
- */
- newchannel->sig_event = VMBUS_EVENT_CONNECTION_ID;
-
- if (vmbus_proto_version != VERSION_WS2008) {
- newchannel->is_dedicated_interrupt =
- (offer->is_dedicated_interrupt != 0);
- newchannel->sig_event = offer->connection_id;
- }
-
- memcpy(&newchannel->offermsg, offer,
- sizeof(struct vmbus_channel_offer_channel));
- newchannel->monitor_grp = (u8)offer->monitorid / 32;
- newchannel->monitor_bit = (u8)offer->monitorid % 32;
+ vmbus_setup_channel_state(newchannel, offer);
vmbus_process_offer(newchannel);
}
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 99851ea..6e4c015 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -29,6 +29,8 @@ struct vmbus_connection vmbus_connection = {
.ready_for_suspend_event= COMPLETION_INITIALIZER(
vmbus_connection.ready_for_suspend_event),
+ .ready_for_resume_event = COMPLETION_INITIALIZER(
+ vmbus_connection.ready_for_resume_event),
};
EXPORT_SYMBOL_GPL(vmbus_connection);
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index eedbe59..ca9ef7c 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -270,6 +270,20 @@ struct vmbus_connection {
* drop to zero.
*/
struct completion ready_for_suspend_event;
+
+ /*
+ * The number of primary channels that should be "fixed up"
+ * upon resume: these channels are re-offered upon resume, and some
+ * fields of the channel offers (i.e. child_relid and connection_id)
+ * can change, so the old offermsg must be fixed up, before the resume
+ * callbacks of the VSC drivers start to further touch the channels.
+ */
+ atomic_t nr_chan_fixup_on_resume;
+ /*
+ * vmbus_bus_resume() waits for "nr_chan_fixup_on_resume" to
+ * drop to zero.
+ */
+ struct completion ready_for_resume_event;
};
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 32ec951..391f0b2 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -2164,9 +2164,17 @@ static int vmbus_bus_suspend(struct device *dev)
if (atomic_read(&vmbus_connection.nr_chan_close_on_suspend) > 0)
wait_for_completion(&vmbus_connection.ready_for_suspend_event);
+ WARN_ON(atomic_read(&vmbus_connection.nr_chan_fixup_on_resume) != 0);
+
mutex_lock(&vmbus_connection.channel_mutex);
list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
+ /*
+ * Invalidate the field. Upon resume, vmbus_onoffer() will fix
+ * up the field, and the other fields (if necessary).
+ */
+ channel->offermsg.child_relid = INVALID_RELID;
+
if (is_hvsock_channel(channel)) {
if (!channel->rescind) {
pr_err("hv_sock channel not rescinded!\n");
@@ -2181,6 +2189,8 @@ static int vmbus_bus_suspend(struct device *dev)
WARN_ON_ONCE(1);
}
spin_unlock_irqrestore(&channel->lock, flags);
+
+ atomic_inc(&vmbus_connection.nr_chan_fixup_on_resume);
}
mutex_unlock(&vmbus_connection.channel_mutex);
@@ -2189,6 +2199,9 @@ static int vmbus_bus_suspend(struct device *dev)
vmbus_connection.conn_state = DISCONNECTED;
+ /* Reset the event for the next resume. */
+ reinit_completion(&vmbus_connection.ready_for_resume_event);
+
return 0;
}
@@ -2223,8 +2236,12 @@ static int vmbus_bus_resume(struct device *dev)
if (ret != 0)
return ret;
+ WARN_ON(atomic_read(&vmbus_connection.nr_chan_fixup_on_resume) == 0);
+
vmbus_request_offers();
+ wait_for_completion(&vmbus_connection.ready_for_resume_event);
+
/* Reset the event for the next suspend. */
reinit_completion(&vmbus_connection.ready_for_suspend_event);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 8a60e77..a3aa9e9 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -426,6 +426,9 @@ enum vmbus_channel_message_type {
CHANNELMSG_COUNT
};
+/* Hyper-V supports about 2048 channels, and the RELIDs start with 1. */
+#define INVALID_RELID U32_MAX
+
struct vmbus_channel_message_header {
enum vmbus_channel_message_type msgtype;
u32 padding;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v4 05/12] Drivers: hv: vmbus: Suspend/resume the synic for hibernation
From: Dexuan Cui @ 2019-09-03 0:23 UTC (permalink / raw)
To: linux-hyperv@vger.kernel.org, gregkh@linuxfoundation.org,
Stephen Hemminger, Sasha Levin, sashal@kernel.org, Haiyang Zhang,
KY Srinivasan, Michael Kelley, tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org, Dexuan Cui
In-Reply-To: <1567470139-119355-1-git-send-email-decui@microsoft.com>
This is needed when we resume the old kernel from the "current" kernel.
Note: when hv_synic_suspend() and hv_synic_resume() run, all the
non-boot CPUs have been offlined, and interrupts are disabled on CPU0.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
---
drivers/hv/vmbus_drv.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index ebd35fc..2ef375c 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -30,6 +30,7 @@
#include <linux/kdebug.h>
#include <linux/efi.h>
#include <linux/random.h>
+#include <linux/syscore_ops.h>
#include <clocksource/hyperv_timer.h>
#include "hyperv_vmbus.h"
@@ -2086,6 +2087,47 @@ static void hv_crash_handler(struct pt_regs *regs)
hyperv_cleanup();
};
+static int hv_synic_suspend(void)
+{
+ /*
+ * When we reach here, all the non-boot CPUs have been offlined, and
+ * the stimers on them have been unbound in hv_synic_cleanup() ->
+ * hv_stimer_cleanup() -> clockevents_unbind_device().
+ *
+ * hv_synic_suspend() only runs on CPU0 with interrupts disabled. Here
+ * we do not unbind the stimer on CPU0 because: 1) it's unnecessary
+ * because the interrupts remain disabled between syscore_suspend()
+ * and syscore_resume(): see create_image() and resume_target_kernel();
+ * 2) the stimer on CPU0 is automatically disabled later by
+ * syscore_suspend() -> timekeeping_suspend() -> tick_suspend() -> ...
+ * -> clockevents_shutdown() -> ... -> hv_ce_shutdown(); 3) a warning
+ * would be triggered if we call clockevents_unbind_device(), which
+ * may sleep, in an interrupts-disabled context. So, we intentionally
+ * don't call hv_stimer_cleanup(0) here.
+ */
+
+ hv_synic_disable_regs(0);
+
+ return 0;
+}
+
+static void hv_synic_resume(void)
+{
+ hv_synic_enable_regs(0);
+
+ /*
+ * Note: we don't need to call hv_stimer_init(0), because the timer
+ * on CPU0 is not unbound in hv_synic_suspend(), and the timer is
+ * automatically re-enabled in timekeeping_resume().
+ */
+}
+
+/* The callbacks run only on CPU0, with irqs_disabled. */
+static struct syscore_ops hv_synic_syscore_ops = {
+ .suspend = hv_synic_suspend,
+ .resume = hv_synic_resume,
+};
+
static int __init hv_acpi_init(void)
{
int ret, t;
@@ -2116,6 +2158,8 @@ static int __init hv_acpi_init(void)
hv_setup_kexec_handler(hv_kexec_handler);
hv_setup_crash_handler(hv_crash_handler);
+ register_syscore_ops(&hv_synic_syscore_ops);
+
return 0;
cleanup:
@@ -2128,6 +2172,8 @@ static void __exit vmbus_exit(void)
{
int cpu;
+ unregister_syscore_ops(&hv_synic_syscore_ops);
+
hv_remove_kexec_handler();
hv_remove_crash_handler();
vmbus_connection.conn_state = DISCONNECTED;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v4 11/12] Drivers: hv: vmbus: Suspend after cleaning up hv_sock and sub channels
From: Dexuan Cui @ 2019-09-03 0:23 UTC (permalink / raw)
To: linux-hyperv@vger.kernel.org, gregkh@linuxfoundation.org,
Stephen Hemminger, Sasha Levin, sashal@kernel.org, Haiyang Zhang,
KY Srinivasan, Michael Kelley, tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org, Dexuan Cui
In-Reply-To: <1567470139-119355-1-git-send-email-decui@microsoft.com>
Before suspend, Linux must make sure all the hv_sock channels have been
properly cleaned up, because a hv_sock connection can not persist across
hibernation, and the user-space app must be properly notified of the
state change of the connection.
Before suspend, Linux also must make sure all the sub-channels have been
destroyed, i.e. the related channel structs of the sub-channels must be
properly removed, otherwise they would cause a conflict when the
sub-channels are recreated upon resume.
Add a counter to track such channels, and vmbus_bus_suspend() should wait
for the counter to drop to zero.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
drivers/hv/channel_mgmt.c | 26 ++++++++++++++++++++++++++
drivers/hv/connection.c | 3 +++
drivers/hv/hyperv_vmbus.h | 12 ++++++++++++
drivers/hv/vmbus_drv.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 44b92fa..5518d03 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -545,6 +545,10 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
mutex_lock(&vmbus_connection.channel_mutex);
+ /* Remember the channels that should be cleaned up upon suspend. */
+ if (is_hvsock_channel(newchannel) || is_sub_channel(newchannel))
+ atomic_inc(&vmbus_connection.nr_chan_close_on_suspend);
+
/*
* Now that we have acquired the channel_mutex,
* we can release the potentially racing rescind thread.
@@ -944,6 +948,16 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
vmbus_process_offer(newchannel);
}
+static void check_ready_for_suspend_event(void)
+{
+ /*
+ * If all the sub-channels or hv_sock channels have been cleaned up,
+ * then it's safe to suspend.
+ */
+ if (atomic_dec_and_test(&vmbus_connection.nr_chan_close_on_suspend))
+ complete(&vmbus_connection.ready_for_suspend_event);
+}
+
/*
* vmbus_onoffer_rescind - Rescind offer handler.
*
@@ -954,6 +968,7 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
struct vmbus_channel_rescind_offer *rescind;
struct vmbus_channel *channel;
struct device *dev;
+ bool clean_up_chan_for_suspend;
rescind = (struct vmbus_channel_rescind_offer *)hdr;
@@ -993,6 +1008,8 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
return;
}
+ clean_up_chan_for_suspend = is_hvsock_channel(channel) ||
+ is_sub_channel(channel);
/*
* Before setting channel->rescind in vmbus_rescind_cleanup(), we
* should make sure the channel callback is not running any more.
@@ -1018,6 +1035,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);
+
+ if (clean_up_chan_for_suspend)
+ check_ready_for_suspend_event();
+
return;
}
/*
@@ -1050,6 +1071,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();
}
void vmbus_hvsock_device_unregister(struct vmbus_channel *channel)
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 806319c..99851ea 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -26,6 +26,9 @@
struct vmbus_connection vmbus_connection = {
.conn_state = DISCONNECTED,
.next_gpadl_handle = ATOMIC_INIT(0xE1E10),
+
+ .ready_for_suspend_event= COMPLETION_INITIALIZER(
+ vmbus_connection.ready_for_suspend_event),
};
EXPORT_SYMBOL_GPL(vmbus_connection);
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 613888e..eedbe59 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -258,6 +258,18 @@ struct vmbus_connection {
struct workqueue_struct *work_queue;
struct workqueue_struct *handle_primary_chan_wq;
struct workqueue_struct *handle_sub_chan_wq;
+
+ /*
+ * The number of sub-channels and hv_sock channels that should be
+ * cleaned up upon suspend: sub-channels will be re-created upon
+ * resume, and hv_sock channels should not survive suspend.
+ */
+ atomic_t nr_chan_close_on_suspend;
+ /*
+ * vmbus_bus_suspend() waits for "nr_chan_close_on_suspend" to
+ * drop to zero.
+ */
+ struct completion ready_for_suspend_event;
};
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 45b976e..32ec951 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -2127,7 +2127,8 @@ static int vmbus_acpi_add(struct acpi_device *device)
static int vmbus_bus_suspend(struct device *dev)
{
- struct vmbus_channel *channel;
+ struct vmbus_channel *channel, *sc;
+ unsigned long flags;
while (atomic_read(&vmbus_connection.offer_in_progress) != 0) {
/*
@@ -2146,6 +2147,44 @@ static int vmbus_bus_suspend(struct device *dev)
}
mutex_unlock(&vmbus_connection.channel_mutex);
+ /*
+ * 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.
+ *
+ * Note: the counter nr_chan_close_on_suspend may never go above 0 if
+ * the VM has no sub-channel and hv_sock channel, e.g. a 1-vCPU VM.
+ */
+ if (atomic_read(&vmbus_connection.nr_chan_close_on_suspend) > 0)
+ wait_for_completion(&vmbus_connection.ready_for_suspend_event);
+
+ mutex_lock(&vmbus_connection.channel_mutex);
+
+ list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
+ if (is_hvsock_channel(channel)) {
+ if (!channel->rescind) {
+ pr_err("hv_sock channel not rescinded!\n");
+ WARN_ON_ONCE(1);
+ }
+ continue;
+ }
+
+ spin_lock_irqsave(&channel->lock, flags);
+ list_for_each_entry(sc, &channel->sc_list, sc_list) {
+ pr_err("Sub-channel not deleted!\n");
+ WARN_ON_ONCE(1);
+ }
+ spin_unlock_irqrestore(&channel->lock, flags);
+ }
+
+ mutex_unlock(&vmbus_connection.channel_mutex);
+
vmbus_initiate_unload(false);
vmbus_connection.conn_state = DISCONNECTED;
@@ -2186,6 +2225,9 @@ static int vmbus_bus_resume(struct device *dev)
vmbus_request_offers();
+ /* Reset the event for the next suspend. */
+ reinit_completion(&vmbus_connection.ready_for_suspend_event);
+
return 0;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH v4 10/12] Drivers: hv: vmbus: Clean up hv_sock channels by force upon suspend
From: Dexuan Cui @ 2019-09-03 0:23 UTC (permalink / raw)
To: linux-hyperv@vger.kernel.org, gregkh@linuxfoundation.org,
Stephen Hemminger, Sasha Levin, sashal@kernel.org, Haiyang Zhang,
KY Srinivasan, Michael Kelley, tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org, Dexuan Cui
In-Reply-To: <1567470139-119355-1-git-send-email-decui@microsoft.com>
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
hibernation is triggered: in this case, with this patch, the "rescind"
fields of the channels are set, and the apps will thoroughly destroy
the channels after hibernation.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
---
drivers/hv/vmbus_drv.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index ce9974b..45b976e 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -24,6 +24,7 @@
#include <linux/sched/task_stack.h>
#include <asm/mshyperv.h>
+#include <linux/delay.h>
#include <linux/notifier.h>
#include <linux/ptrace.h>
#include <linux/screen_info.h>
@@ -1069,6 +1070,41 @@ void vmbus_on_msg_dpc(unsigned long data)
vmbus_signal_eom(msg, message_type);
}
+/*
+ * Fake RESCIND_CHANNEL messages to clean up hv_sock channels by force for
+ * hibernation, because hv_sock connections can not persist across hibernation.
+ */
+static void vmbus_force_channel_rescinded(struct vmbus_channel *channel)
+{
+ struct onmessage_work_context *ctx;
+ struct vmbus_channel_rescind_offer *rescind;
+
+ WARN_ON(!is_hvsock_channel(channel));
+
+ /*
+ * sizeof(*ctx) is small and the allocation should really not fail,
+ * otherwise the state of the hv_sock connections ends up in limbo.
+ */
+ ctx = kzalloc(sizeof(*ctx), GFP_KERNEL | __GFP_NOFAIL);
+
+ /*
+ * So far, these are not really used by Linux. Just set them to the
+ * reasonable values conforming to the definitions of the fields.
+ */
+ ctx->msg.header.message_type = 1;
+ ctx->msg.header.payload_size = sizeof(*rescind);
+
+ /* These values are actually used by Linux. */
+ rescind = (struct vmbus_channel_rescind_offer *)ctx->msg.u.payload;
+ rescind->header.msgtype = CHANNELMSG_RESCIND_CHANNELOFFER;
+ rescind->child_relid = channel->offermsg.child_relid;
+
+ INIT_WORK(&ctx->work, vmbus_onmessage_work);
+
+ queue_work_on(vmbus_connection.connect_cpu,
+ vmbus_connection.work_queue,
+ &ctx->work);
+}
/*
* Direct callback for channels using other deferred processing
@@ -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 the completion of any channel
+ * offers that are currently in progress.
+ */
+ msleep(1);
+ }
+
+ mutex_lock(&vmbus_connection.channel_mutex);
+ list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
+ if (!is_hvsock_channel(channel))
+ continue;
+
+ vmbus_force_channel_rescinded(channel);
+ }
+ mutex_unlock(&vmbus_connection.channel_mutex);
+
vmbus_initiate_unload(false);
vmbus_connection.conn_state = DISCONNECTED;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v4 08/12] Drivers: hv: vmbus: Ignore the offers when resuming from hibernation
From: Dexuan Cui @ 2019-09-03 0:23 UTC (permalink / raw)
To: linux-hyperv@vger.kernel.org, gregkh@linuxfoundation.org,
Stephen Hemminger, Sasha Levin, sashal@kernel.org, Haiyang Zhang,
KY Srinivasan, Michael Kelley, tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org, Dexuan Cui
In-Reply-To: <1567470139-119355-1-git-send-email-decui@microsoft.com>
When the VM resumes, the host re-sends the offers. We should not add the
offers to the global vmbus_connection.chn_list again.
This patch assumes the RELIDs of the channels don't change across
hibernation. Actually this is not always true, especially in the case of
NIC SR-IOV the VF vmbus device's RELID sometimes can change. A later patch
will address this issue by mapping the new offers to the old channels and
fixing up the old channels, if necessary.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
drivers/hv/channel_mgmt.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 57 insertions(+), 1 deletion(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index addcef5..44b92fa 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -848,18 +848,74 @@ void vmbus_initiate_unload(bool crash)
}
/*
+ * find_primary_channel_by_offer - Get the channel object given the new offer.
+ * This is only used in the resume path of hibernation.
+ */
+static struct vmbus_channel *
+find_primary_channel_by_offer(const struct vmbus_channel_offer_channel *offer)
+{
+ struct vmbus_channel *channel = NULL, *iter;
+ const guid_t *inst1, *inst2;
+
+ /* Ignore sub-channel offers. */
+ if (offer->offer.sub_channel_index != 0)
+ return NULL;
+
+ mutex_lock(&vmbus_connection.channel_mutex);
+
+ list_for_each_entry(iter, &vmbus_connection.chn_list, listentry) {
+ inst1 = &iter->offermsg.offer.if_instance;
+ inst2 = &offer->offer.if_instance;
+
+ if (guid_equal(inst1, inst2)) {
+ channel = iter;
+ break;
+ }
+ }
+
+ mutex_unlock(&vmbus_connection.channel_mutex);
+
+ return channel;
+}
+
+/*
* vmbus_onoffer - Handler for channel offers from vmbus in parent partition.
*
*/
static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
{
struct vmbus_channel_offer_channel *offer;
- struct vmbus_channel *newchannel;
+ struct vmbus_channel *oldchannel, *newchannel;
+ size_t offer_sz;
offer = (struct vmbus_channel_offer_channel *)hdr;
trace_vmbus_onoffer(offer);
+ oldchannel = find_primary_channel_by_offer(offer);
+
+ if (oldchannel != NULL) {
+ atomic_dec(&vmbus_connection.offer_in_progress);
+
+ /*
+ * We're resuming from hibernation: we expect the host to send
+ * exactly the same offers that we had before the hibernation.
+ */
+ offer_sz = sizeof(*offer);
+ if (memcmp(offer, &oldchannel->offermsg, offer_sz) == 0)
+ return;
+
+ pr_debug("Mismatched offer from the host (relid=%d)\n",
+ offer->child_relid);
+
+ print_hex_dump_debug("Old vmbus offer: ", DUMP_PREFIX_OFFSET,
+ 16, 4, &oldchannel->offermsg, offer_sz,
+ false);
+ print_hex_dump_debug("New vmbus offer: ", DUMP_PREFIX_OFFSET,
+ 16, 4, offer, offer_sz, false);
+ return;
+ }
+
/* Allocate the channel object and save this offer. */
newchannel = alloc_channel();
if (!newchannel) {
--
1.8.3.1
^ permalink raw reply related
* [PATCH v4 07/12] Drivers: hv: vmbus: Implement suspend/resume for VSC drivers for hibernation
From: Dexuan Cui @ 2019-09-03 0:23 UTC (permalink / raw)
To: linux-hyperv@vger.kernel.org, gregkh@linuxfoundation.org,
Stephen Hemminger, Sasha Levin, sashal@kernel.org, Haiyang Zhang,
KY Srinivasan, Michael Kelley, tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org, Dexuan Cui
In-Reply-To: <1567470139-119355-1-git-send-email-decui@microsoft.com>
The high-level VSC drivers will implement device-specific callbacks.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
---
drivers/hv/vmbus_drv.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/hyperv.h | 3 +++
2 files changed, 49 insertions(+)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 2ef375c..a30c70a 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -911,6 +911,43 @@ static void vmbus_shutdown(struct device *child_device)
drv->shutdown(dev);
}
+/*
+ * vmbus_suspend - Suspend a vmbus device
+ */
+static int vmbus_suspend(struct device *child_device)
+{
+ struct hv_driver *drv;
+ struct hv_device *dev = device_to_hv_device(child_device);
+
+ /* The device may not be attached yet */
+ if (!child_device->driver)
+ return 0;
+
+ drv = drv_to_hv_drv(child_device->driver);
+ if (!drv->suspend)
+ return -EOPNOTSUPP;
+
+ return drv->suspend(dev);
+}
+
+/*
+ * vmbus_resume - Resume a vmbus device
+ */
+static int vmbus_resume(struct device *child_device)
+{
+ struct hv_driver *drv;
+ struct hv_device *dev = device_to_hv_device(child_device);
+
+ /* The device may not be attached yet */
+ if (!child_device->driver)
+ return 0;
+
+ drv = drv_to_hv_drv(child_device->driver);
+ if (!drv->resume)
+ return -EOPNOTSUPP;
+
+ return drv->resume(dev);
+}
/*
* vmbus_device_release - Final callback release of the vmbus child device
@@ -926,6 +963,14 @@ static void vmbus_device_release(struct device *device)
kfree(hv_dev);
}
+/*
+ * Note: we must use SET_NOIRQ_SYSTEM_SLEEP_PM_OPS rather than
+ * SET_SYSTEM_SLEEP_PM_OPS: see the comment before vmbus_bus_pm.
+ */
+static const struct dev_pm_ops vmbus_pm = {
+ SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(vmbus_suspend, vmbus_resume)
+};
+
/* The one and only one */
static struct bus_type hv_bus = {
.name = "vmbus",
@@ -936,6 +981,7 @@ static void vmbus_device_release(struct device *device)
.uevent = vmbus_uevent,
.dev_groups = vmbus_dev_groups,
.drv_groups = vmbus_drv_groups,
+ .pm = &vmbus_pm,
};
struct onmessage_work_context {
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 2d39248..8a60e77 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1157,6 +1157,9 @@ struct hv_driver {
int (*remove)(struct hv_device *);
void (*shutdown)(struct hv_device *);
+ int (*suspend)(struct hv_device *);
+ int (*resume)(struct hv_device *);
+
};
/* Base device object */
--
1.8.3.1
^ permalink raw reply related
* [PATCH v4 00/12] Enhance the hv_vmbus driver to support hibernation
From: Dexuan Cui @ 2019-09-03 0:23 UTC (permalink / raw)
To: linux-hyperv@vger.kernel.org, gregkh@linuxfoundation.org,
Stephen Hemminger, Sasha Levin, sashal@kernel.org, Haiyang Zhang,
KY Srinivasan, Michael Kelley, tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org, Dexuan Cui
Hi all,
The patchset is to enhance hv_vmbus to support hibernation when Linux VM
runs on Hyper-V. A second patchset to enhance the high-level VSC drivers
(hv_netvsc, hv_storvsc, etc.) for hibernation will be posted after this
patchset is acceped. If you want to test this hibernation feaure, all the
needed patches can be found on my github branch:
https://github.com/dcui/linux/commits/decui/hibernation/2019-0830/v4/v5.3-rc6-plus
This patchset is based on v5.3-rc6, and can also cleanly apply to -rc7.
Please review.
Hi tglx,
I hope all the 12 patchset, including the below 3 patches,
[PATCH v4 01/12] x86/hyper-v: Suspend/resume the hypercall page for hibernation
[PATCH v4 02/12] x86/hyper-v: Implement hv_is_hibernation_supported()
[PATCH v4 03/12] clocksource/drivers: Suspend/resume Hyper-V
can go through Sasha's hyperv/linux.git tree, since all the changes belong
to the hv stuff. However, if you think it's better for these 3 patches to go
through your tip.git tree, it also works for me.
Hi Michael,
I added your Reviewed-by's for patch 1, 3~7, 9~10, since you have reviewed
these patches. Please review the v4 of the others: 2, 8, 11 and 12.
Thanks,
Dexuan
Changes in v4:
Patch 2: Enhanced the changelog.
Patch 8: Moved find_primary_channel_by_offer() to channel_mgmt.c, where
it's used.
Patch 10: Improved the changelog (typo) and comment.
Patch 11: Removed WARN_ON_ONCE(), added a comment for the case
nr_chan_close_on_suspend == 0.
Patch 12: Improved the comment.
Changes in v3:
Patch 2: Add a new API hv_is_hibernation_supported().
Patch 6: Add a new helper function is_sub_channel().
Patch 8: Find the old channels via Instance GUID rather than the RELID.
Patch 10: Add code to clean up hv_sock channels by force
Patch 11: Add code to wait in the suspend path: all the hv_sock channels
and sub-channels should be cleaned up first before Linux sends
the VMBUS UNLOAD message.
Patch 12: Add code to fix up the old primary channels before further
resuming.
Changes in v2:
Patch 3: Improved the changelog and added a comment.
Patch 4: Remove the "hv_stimer_cleanup" in hv_synic_suspend(), because I
suppose https://lkml.org/lkml/2019/7/27/5 will be accepted. Also
improved changelog and the comment.
Patch 5: Fixed the third argument of print_hex_dump_debug(). Also improved
the changelog.
Patch 6: Improved the changelog and the comment. Added a check for the
'vmbus_proto_version' in vmbus_bus_resume().
Dexuan Cui (12):
x86/hyper-v: Suspend/resume the hypercall page for hibernation
x86/hyper-v: Implement hv_is_hibernation_supported()
clocksource/drivers: Suspend/resume Hyper-V clocksource for
hibernation
Drivers: hv: vmbus: Break out synic enable and disable operations
Drivers: hv: vmbus: Suspend/resume the synic for hibernation
Drivers: hv: vmbus: Add a helper function is_sub_channel()
Drivers: hv: vmbus: Implement suspend/resume for VSC drivers for
hibernation
Drivers: hv: vmbus: Ignore the offers when resuming from hibernation
Drivers: hv: vmbus: Suspend/resume the vmbus itself for hibernation
Drivers: hv: vmbus: Clean up hv_sock channels by force upon suspend
Drivers: hv: vmbus: Suspend after cleaning up hv_sock and sub channels
Drivers: hv: vmbus: Resume after fixing up old primary channels
arch/x86/hyperv/hv_init.c | 41 ++++++
drivers/clocksource/hyperv_timer.c | 25 ++++
drivers/hv/channel_mgmt.c | 161 +++++++++++++++++++---
drivers/hv/connection.c | 8 +-
drivers/hv/hv.c | 66 +++++----
drivers/hv/hyperv_vmbus.h | 30 +++++
drivers/hv/vmbus_drv.c | 265 +++++++++++++++++++++++++++++++++++++
include/asm-generic/mshyperv.h | 2 +
include/linux/hyperv.h | 16 ++-
9 files changed, 565 insertions(+), 49 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH] irqdomain: Add the missing assignment of domain->fwnode for named fwnode
From: Dexuan Cui @ 2019-09-02 23:14 UTC (permalink / raw)
To: Marc Zyngier, Thomas Gleixner
Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Lorenzo Pieralisi, Bjorn Helgaas, linux-kernel@vger.kernel.org,
linux-hyperv@vger.kernel.org, Michael Kelley,
Lili Deng (Wicresoft North America Ltd)
Recently device pass-through stops working for Linux VM running on Hyper-V.
git-bisect shows the regression is caused by the recent commit
467a3bb97432 ("PCI: hv: Allocate a named fwnode ..."), but the root cause
is that the commit d59f6617eef0 forgets to set the domain->fwnode for
IRQCHIP_FWNODE_NAMED*, and as a result:
1. The domain->fwnode remains to be NULL.
2. irq_find_matching_fwspec() returns NULL since "h->fwnode == fwnode" is
false, and pci_set_bus_msi_domain() sets the Hyper-V PCI root bus's
msi_domain to NULL.
3. When the device is added onto the root bus, the device's dev->msi_domain
is set to NULL in pci_set_msi_domain().
4. When a device driver tries to enable MSI-X, pci_msi_setup_msi_irqs()
calls arch_setup_msi_irqs(), which uses the native MSI chip (i.e.
arch/x86/kernel/apic/msi.c: pci_msi_controller) to set up the irqs, but
actually pci_msi_setup_msi_irqs() is supposed to call
msi_domain_alloc_irqs() with the hbus->irq_domain, which is created in
hv_pcie_init_irq_domain() and is associated with the Hyper-V chip
hv_msi_irq_chip. Consequently, the irq line is not properly set up, and
the device driver can not receive any interrupt.
Fixes: d59f6617eef0 ("genirq: Allow fwnode to carry name information only")
Fixes: 467a3bb97432 ("PCI: hv: Allocate a named fwnode instead of an address-based one")
Reported-by: Lili Deng <v-lide@microsoft.com>
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
Note: the commit 467a3bb97432 ("PCI: hv: Allocate a named fwnode ...") has not
gone in Linus's tree yet (the commit is in linux-next for a while), so the commit ID
in the changelog can change when it goes in Linus's tree.
This patch works in my test, but I'm not 100% sure this is the right fix.
Looking forward to your comment!
kernel/irq/irqdomain.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index e7bbab149750..132672b74e4b 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -149,6 +149,7 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, int size,
switch (fwid->type) {
case IRQCHIP_FWNODE_NAMED:
case IRQCHIP_FWNODE_NAMED_ID:
+ domain->fwnode = fwnode;
domain->name = kstrdup(fwid->name, GFP_KERNEL);
if (!domain->name) {
kfree(domain);
--
2.19.1
^ permalink raw reply related
* RE: [PATCH v4 1/2] drivers: hv: vmbus: Introduce latency testing
From: Michael Kelley @ 2019-09-02 18:31 UTC (permalink / raw)
To: brandonbonaby94, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org
Cc: brandonbonaby94, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <da1ab5c98ce902ec181a799d8d1f040e8cc39af6.1567047549.git.brandonbonaby94@gmail.com>
From: Branden Bonaby <brandonbonaby94@gmail.com> Sent: Wednesday, August 28, 2019 9:24 PM
>
> Introduce user specified latency in the packet reception path
> By exposing the test parameters as part of the debugfs channel
> attributes. We will control the testing state via these attributes.
>
> Signed-off-by: Branden Bonaby <brandonbonaby94@gmail.com>
> ---
> diff --git a/Documentation/ABI/testing/debugfs-hyperv
> b/Documentation/ABI/testing/debugfs-hyperv
> new file mode 100644
> index 000000000000..0903e6427a2d
> --- /dev/null
> +++ b/Documentation/ABI/testing/debugfs-hyperv
> @@ -0,0 +1,23 @@
> +What: /sys/kernel/debug/hyperv/<UUID>/fuzz_test_state
> +Date: August 2019
> +KernelVersion: 5.3
Given the way the timing works for adding code to the Linux kernel,
the earliest your new code will be included is 5.4. The merge window
for 5.4 will open in two weeks, so your code would need to be accepted
by then to be included in 5.4. Otherwise, it won't make it until the next
merge window, which would be for 5.5. I would suggest changing this
(and the others below) to 5.4 for now, but you might have to change to
5.5 if additional revisions are needed.
> +Contact: Branden Bonaby <brandonbonaby94@gmail.com>
> +Description: Fuzz testing status of a vmbus device, whether its in an ON
> + state or a OFF state
> +Users: Debugging tools
> +
> +What: /sys/kernel/debug/hyperv/<UUID>/delay/fuzz_test_buffer_interrupt_delay
> +Date: August 2019
> +KernelVersion: 5.3
> +Contact: Branden Bonaby <brandonbonaby94@gmail.com>
> +Description: Fuzz testing buffer interrupt delay value between 0 - 1000
> + microseconds (inclusive).
> +Users: Debugging tools
> +
> +What: /sys/kernel/debug/hyperv/<UUID>/delay/fuzz_test_message_delay
> +Date: August 2019
> +KernelVersion: 5.3
> +Contact: Branden Bonaby <brandonbonaby94@gmail.com>
> +Description: Fuzz testing message delay value between 0 - 1000 microseconds
> + (inclusive).
> +Users: Debugging tools
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b1bc9c87838b..6931a4eeaac0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7460,6 +7460,7 @@ F: include/uapi/linux/hyperv.h
> F: include/asm-generic/mshyperv.h
> F: tools/hv/
> F: Documentation/ABI/stable/sysfs-bus-vmbus
> +F: Documentation/ABI/testing/debugfs-hyperv
>
> HYPERBUS SUPPORT
> M: Vignesh Raghavendra <vigneshr@ti.com>
> 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
> diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
> index a1eec7177c2d..d754bd86ca47 100644
> --- a/drivers/hv/Makefile
> +++ b/drivers/hv/Makefile
> @@ -2,6 +2,7 @@
> obj-$(CONFIG_HYPERV) += hv_vmbus.o
> obj-$(CONFIG_HYPERV_UTILS) += hv_utils.o
> obj-$(CONFIG_HYPERV_BALLOON) += hv_balloon.o
> +obj-$(CONFIG_HYPERV_TESTING) += hv_debugfs.o
There's an additional complexity here that I failed to describe to
you in my previous comments. :-( The above line makes the
hv_debugfs code part of the main Linux OS image -- i.e., the
vmlinuz file that gets installed into /boot, such that if hv_debugfs
is built, it is always loaded into the memory of the running system.
That's OK, but we should consider the alternative, which is to
make the hv_debugfs code part of the hv_vmbus module that
is specified a bit further down in the same Makefile. A module
is installed into /lib/modules/<kernel version>/kernel, and
is only loaded into memory on the running system if something
actually references code in the module. This approach helps avoid
loading code into memory that isn't actually used.
Whether code is built as a separate module or is just built into
the main vmlinuz file is also controlled by the .config file setting.
For example, CONFIG_HYPERV=m says to treat hv_vmbus as a
separate module, while CONFIG_HYPERV=y says to build the
hv_vmbus module directly into the vmlinuz file even though the
Makefile constructs it as a module.
Making hv_debugfs part of the hv_vmbus module is generally better
than just building it into the main vmlinuz because it's best to include
only things that must always be present in the main vmlinuz file.
hv_debugfs doesn't have any reason it needs to always be present.
By comparison, hv_balloon is included in the main vmlinuz, which
might be due to it dealing with memory mgmt and needing to be
in the vmlinuz image, but I'm not sure.
You can make hv_debugfs part of the hv_vmbus module with this line
placed after the line specifying hv_vmbus_y, instead of the line you
currently have:
hv_vmbus-$(CONFIG_HYPERV_TESTING) += hv_debugfs.o
> +
> +static int hv_debugfs_delay_set(void *data, u64 val)
> +{
> + int ret = 0;
> +
> + if (val >= 1 && val <= 1000)
> + *(u32 *)data = val;
> + else if (val == 0)
> + *(u32 *)data = 0;
I think this "else if" clause can be folded into the first
"if" statement by just checking "val >= 0".
> + else
> + ret = -EINVAL;
> +
> + return ret;
> +}
> +
> +/* Remove dentry associated with released hv device */
> +void hv_debug_rm_dev_dir(struct hv_device *dev)
> +{
> + if (!IS_ERR(hv_debug_root))
> + debugfs_remove_recursive(dev->debug_dir);
> +}
This function is the first of five functions that are called by
code outside of hv_debugfs.c. You probably saw the separate
email from the kbuild test robot showing a build error on each
of these five functions. Here's why.
When CONFIG_HYPERV=m is set, and hv_vmbus is built as a
module, there are references to the five functions from the
module to your hv_debugfs that is built as core code in
vmlinuz. By default, Linux does not allow such core code to
be called by modules. Core code must add a statement like:
EXPORT_SYMBOL_GPL(hv_debug_rm_dev_dir);
to allow the module to call it. This gives the code writer
a very minimal amount of scope control. However, if you build
hv_debugfs as part of the module hv_vmbus, and the only
references to the five functions are within the module hv_vmbus,
then the EXPORT statements aren't needed because all
references are internal to the hv_vmbus module. But if
you envision a function like hv_debug_delay_test() being
called by other Hyper-V drivers that are outside the
hv_vmbus module, then you will need the EXPORT statement
at least for that function.
You probably have your .config file setup with
CONFIG_HYPERV=y. In that case, the hv_vmbus module is
built-in to the kernel, so you didn't get the errors reported
by the kbuild test robot. When testing new code, it's a
good practice to build with the HYPERV settings set to
'm' to make sure that any issues with module references
get flushed out and fixed.
Michael
^ permalink raw reply
* RE: [PATCH V2] x86/Hyper-V: Fix overflow issue in the fill_gva_list()
From: Michael Kelley @ 2019-09-02 14:40 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,
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: <20190902124143.119478-1-Tianyu.Lan@microsoft.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Monday, September 2, 2019 5:42 AM
>
> 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
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* 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
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