* [PATCH 2/2] PCI: hv: Add support for protocol 1.3 and support PCI_BUS_RELATIONS2
From: longli @ 2019-11-23 1:57 UTC (permalink / raw)
To: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Lorenzo Pieralisi, Andrew Murray, Bjorn Helgaas, linux-hyperv,
linux-pci, linux-kernel
Cc: Long Li
In-Reply-To: <1574474229-44840-1-git-send-email-longli@linuxonhyperv.com>
From: Long Li <longli@microsoft.com>
Starting with Hyper-V PCI protocol version 1.3, the host VSP can send
PCI_BUS_RELATIONS2 and pass the vNUMA node information for devices on the bus.
The vNUMA node tells which guest NUMA node this device is on based on guest
VM configuration topology and physical device inforamtion.
The patch adds code to negotiate v1.3 and process PCI_BUS_RELATIONS2.
Signed-off-by: Long Li <longli@microsoft.com>
---
drivers/pci/controller/pci-hyperv.c | 107 ++++++++++++++++++++++++++++
1 file changed, 107 insertions(+)
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index f2e028cfa7cd..488235563c7d 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -63,6 +63,7 @@
enum pci_protocol_version_t {
PCI_PROTOCOL_VERSION_1_1 = PCI_MAKE_VERSION(1, 1), /* Win10 */
PCI_PROTOCOL_VERSION_1_2 = PCI_MAKE_VERSION(1, 2), /* RS1 */
+ PCI_PROTOCOL_VERSION_1_3 = PCI_MAKE_VERSION(1, 3), /* VB */
};
#define CPU_AFFINITY_ALL -1ULL
@@ -72,6 +73,7 @@ enum pci_protocol_version_t {
* first.
*/
static enum pci_protocol_version_t pci_protocol_versions[] = {
+ PCI_PROTOCOL_VERSION_1_3,
PCI_PROTOCOL_VERSION_1_2,
PCI_PROTOCOL_VERSION_1_1,
};
@@ -124,6 +126,7 @@ enum pci_message_type {
PCI_RESOURCES_ASSIGNED2 = PCI_MESSAGE_BASE + 0x16,
PCI_CREATE_INTERRUPT_MESSAGE2 = PCI_MESSAGE_BASE + 0x17,
PCI_DELETE_INTERRUPT_MESSAGE2 = PCI_MESSAGE_BASE + 0x18, /* unused */
+ PCI_BUS_RELATIONS2 = PCI_MESSAGE_BASE + 0x19,
PCI_MESSAGE_MAXIMUM
};
@@ -169,6 +172,26 @@ struct pci_function_description {
u32 ser; /* serial number */
} __packed;
+enum pci_device_description_flags {
+ HV_PCI_DEVICE_FLAG_NONE = 0x0,
+ HV_PCI_DEVICE_FLAG_NUMA_AFFINITY = 0x1,
+};
+
+struct pci_function_description2 {
+ u16 v_id; /* vendor ID */
+ u16 d_id; /* device ID */
+ u8 rev;
+ u8 prog_intf;
+ u8 subclass;
+ u8 base_class;
+ u32 subsystem_id;
+ union win_slot_encoding win_slot;
+ u32 ser; /* serial number */
+ u32 flags;
+ u16 virtual_numa_node;
+ u16 reserved;
+} __packed;
+
/**
* struct hv_msi_desc
* @vector: IDT entry
@@ -304,6 +327,12 @@ struct pci_bus_relations {
struct pci_function_description func[0];
} __packed;
+struct pci_bus_relations2 {
+ struct pci_incoming_message incoming;
+ u32 device_count;
+ struct pci_function_description2 func[0];
+} __packed;
+
struct pci_q_res_req_response {
struct vmpacket_descriptor hdr;
s32 status; /* negative values are failures */
@@ -1417,6 +1446,7 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
break;
case PCI_PROTOCOL_VERSION_1_2:
+ case PCI_PROTOCOL_VERSION_1_3:
size = hv_compose_msi_req_v2(&ctxt.int_pkts.v2,
dest,
hpdev->desc.win_slot.slot,
@@ -1798,6 +1828,25 @@ static void hv_pci_remove_slots(struct hv_pcibus_device *hbus)
}
}
+/*
+ * Set NUMA node for the devices on the bus
+ */
+static void pci_assign_numa_node(struct hv_pcibus_device *hbus)
+{
+ struct pci_dev *dev;
+ struct pci_bus *bus = hbus->pci_bus;
+ struct hv_pci_dev *hv_dev;
+
+ list_for_each_entry(dev, &bus->devices, bus_list) {
+ hv_dev = get_pcichild_wslot(hbus, devfn_to_wslot(dev->devfn));
+ if (!hv_dev)
+ continue;
+
+ if (hv_dev->desc.flags & HV_PCI_DEVICE_FLAG_NUMA_AFFINITY)
+ set_dev_node(&dev->dev, hv_dev->desc.virtual_numa_node);
+ }
+}
+
/**
* create_root_hv_pci_bus() - Expose a new root PCI bus
* @hbus: Root PCI bus, as understood by this driver
@@ -1820,6 +1869,7 @@ static int create_root_hv_pci_bus(struct hv_pcibus_device *hbus)
pci_lock_rescan_remove();
pci_scan_child_bus(hbus->pci_bus);
+ pci_assign_numa_node(hbus);
pci_bus_assign_resources(hbus->pci_bus);
hv_pci_assign_slots(hbus);
pci_bus_add_devices(hbus->pci_bus);
@@ -2088,6 +2138,7 @@ static void pci_devices_present_work(struct work_struct *work)
*/
pci_lock_rescan_remove();
pci_scan_child_bus(hbus->pci_bus);
+ pci_assign_numa_node(hbus);
hv_pci_assign_slots(hbus);
pci_unlock_rescan_remove();
break;
@@ -2183,6 +2234,46 @@ static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
kfree(dr);
}
+/**
+ * hv_pci_devices_present2() - Handles list of new children
+ * @hbus: Root PCI bus, as understood by this driver
+ * @relations2: Packet from host listing children
+ *
+ * This function is the v2 version of hv_pci_devices_present()
+ */
+static void hv_pci_devices_present2(struct hv_pcibus_device *hbus,
+ struct pci_bus_relations2 *relations)
+{
+ struct hv_dr_state *dr;
+ int i;
+
+ dr = kzalloc(offsetof(struct hv_dr_state, func) +
+ (sizeof(struct hv_pcidev_description) *
+ (relations->device_count)), GFP_NOWAIT);
+
+ if (!dr)
+ return;
+
+ dr->device_count = relations->device_count;
+ for (i = 0; i < dr->device_count; i++) {
+ dr->func[i].v_id = relations->func[i].v_id;
+ dr->func[i].d_id = relations->func[i].d_id;
+ dr->func[i].rev = relations->func[i].rev;
+ dr->func[i].prog_intf = relations->func[i].prog_intf;
+ dr->func[i].subclass = relations->func[i].subclass;
+ dr->func[i].base_class = relations->func[i].base_class;
+ dr->func[i].subsystem_id = relations->func[i].subsystem_id;
+ dr->func[i].win_slot = relations->func[i].win_slot;
+ dr->func[i].ser = relations->func[i].ser;
+ dr->func[i].flags = relations->func[i].flags;
+ dr->func[i].virtual_numa_node =
+ relations->func[i].virtual_numa_node;
+ }
+
+ if (hv_pci_start_relations_work(hbus, dr))
+ kfree(dr);
+}
+
/**
* hv_eject_device_work() - Asynchronously handles ejection
* @work: Work struct embedded in internal device struct
@@ -2288,6 +2379,7 @@ static void hv_pci_onchannelcallback(void *context)
struct pci_response *response;
struct pci_incoming_message *new_message;
struct pci_bus_relations *bus_rel;
+ struct pci_bus_relations2 *bus_rel2;
struct pci_dev_inval_block *inval;
struct pci_dev_incoming *dev_message;
struct hv_pci_dev *hpdev;
@@ -2355,6 +2447,21 @@ static void hv_pci_onchannelcallback(void *context)
hv_pci_devices_present(hbus, bus_rel);
break;
+ case PCI_BUS_RELATIONS2:
+
+ bus_rel2 = (struct pci_bus_relations2 *)buffer;
+ if (bytes_recvd <
+ offsetof(struct pci_bus_relations2, func) +
+ (sizeof(struct pci_function_description2) *
+ (bus_rel2->device_count))) {
+ dev_err(&hbus->hdev->device,
+ "bus relations v2 too small\n");
+ break;
+ }
+
+ hv_pci_devices_present2(hbus, bus_rel2);
+ break;
+
case PCI_EJECT:
dev_message = (struct pci_dev_incoming *)buffer;
--
2.17.1
^ permalink raw reply related
* [PATCH net-next] hv_netvsc: make recording RSS hash depend on feature flag
From: Haiyang Zhang @ 2019-11-23 23:50 UTC (permalink / raw)
To: sashal, linux-hyperv, netdev
Cc: haiyangz, kys, sthemmin, olaf, vkuznets, davem, linux-kernel
From: Stephen Hemminger <sthemmin@microsoft.com>
The recording of RSS hash should be controlled by NETIF_F_RXHASH.
Fixes: 1fac7ca4e63b ("hv_netvsc: record hardware hash in skb")
Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/net/hyperv/hyperv_net.h | 3 ++-
drivers/net/hyperv/netvsc_drv.c | 2 +-
drivers/net/hyperv/rndis_filter.c | 1 +
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 4209d1cf57f6..0be5ce90dc7c 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -822,7 +822,8 @@ 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_SG)
+ NETIF_F_TSO6 | NETIF_F_LRO | \
+ NETIF_F_SG | NETIF_F_RXHASH)
#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 5fa5c49e481b..868e22e286ca 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -803,7 +803,7 @@ static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
skb->ip_summed = CHECKSUM_UNNECESSARY;
}
- if (hash_info)
+ if (hash_info && (net->features & NETIF_F_RXHASH))
skb_set_hash(skb, *hash_info, PKT_HASH_TYPE_L4);
if (vlan) {
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index c06178380ac8..206b4e77eaf0 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -1214,6 +1214,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;
+ net->hw_features |= NETIF_F_RXHASH;
if ((hwcaps.csum.ip4_txcsum & NDIS_TXCSUM_ALL_TCP4) == NDIS_TXCSUM_ALL_TCP4) {
/* Can checksum TCP */
--
2.20.1
^ permalink raw reply related
* RE: [PATCH net-next] hv_netvsc: make recording RSS hash depend on feature flag
From: Michael Kelley @ 2019-11-24 0:16 UTC (permalink / raw)
To: Haiyang Zhang, 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: <1574553017-87877-1-git-send-email-haiyangz@microsoft.com>
From: Haiyang Zhang <haiyangz@microsoft.com> Sent: Saturday, November 23, 2019 3:50 PM
>
> From: Stephen Hemminger <sthemmin@microsoft.com>
>
> The recording of RSS hash should be controlled by NETIF_F_RXHASH.
>
> Fixes: 1fac7ca4e63b ("hv_netvsc: record hardware hash in skb")
> Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* Re: [PATCH net-next] hv_netvsc: make recording RSS hash depend on feature flag
From: Jakub Kicinski @ 2019-11-24 2:44 UTC (permalink / raw)
To: Haiyang Zhang
Cc: sashal, linux-hyperv, netdev, kys, sthemmin, olaf, vkuznets,
davem, linux-kernel
In-Reply-To: <1574553017-87877-1-git-send-email-haiyangz@microsoft.com>
On Sat, 23 Nov 2019 15:50:17 -0800, Haiyang Zhang wrote:
> From: Stephen Hemminger <sthemmin@microsoft.com>
>
> The recording of RSS hash should be controlled by NETIF_F_RXHASH.
>
> Fixes: 1fac7ca4e63b ("hv_netvsc: record hardware hash in skb")
> Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Applied, thank you!
^ permalink raw reply
* RE: [PATCH v2] Input: hyperv-keyboard: Add the support of hibernation
From: Michael Kelley @ 2019-11-24 20:31 UTC (permalink / raw)
To: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, dmitry.torokhov@gmail.com,
linux-hyperv@vger.kernel.org, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, Sasha Levin
In-Reply-To: <1574234068-48688-1-git-send-email-decui@microsoft.com>
From: Dexuan Cui <decui@microsoft.com> Sent: Tuesday, November 19, 2019 11:14 PM
>
> During the suspend process and resume process, if there is any keyboard
> event, there is a small chance the suspend and the resume process can be
> aborted because of hv_kbd_on_receive() -> pm_wakeup_hard_event().
>
> This behavior can be avoided by disabling the Hyper-V keyboard device as
> a wakeup source:
>
> echo disabled > /sys/bus/vmbus/drivers/hyperv_keyboard/XXX/power/wakeup
> (XXX is the device's GUID).
I'd like to see a more descriptive commit message, along the lines of:
Add suspend() and resume() functions so the Hyper-V virtual keyboard
can participate in VM hibernation.
Note that the keyboard is a "wakeup" device that could abort an in-progress
hibernation if there is keyboard event. No attempt is made to suppress this
behavior. If desired, a sysadmin can disable the keyboard as a wakeup device
using standard mechanisms such as:
echo disabled > /sys/bus/vmbus/drivers/hyperv_keyboard/XXX/power/wakeup
(where XXX is the device's GUID)
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
>
> This patch is basically a pure Hyper-V specific change. I suggest it should
> go through the Hyper-V tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git/log/?h=hyperv-next
>
> Changes in v2:
> Removed the "struct notifier_block pm_nb;" after the discussion
> with Dmitry Torokhov:
>
> https://lore.kernel.org/lkml/PU1P153MB016914A7C827CA35D7FEB66ABF8B0@PU1P153MB0169.APCP153.PROD.OUTLOOK.COM/T/#m8948c711301220a36a1a413eead74cd2fb6dcac1>
>
> drivers/input/serio/hyperv-keyboard.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
Notwithstanding the suggestion on the commit message,
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [PATCH v2 4/4] PCI: hv: kmemleak: Track the page allocations for struct hv_pcibus_device
From: Michael Kelley @ 2019-11-24 21:52 UTC (permalink / raw)
To: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, lorenzo.pieralisi@arm.com, bhelgaas@google.com,
linux-hyperv@vger.kernel.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, Sasha Levin
In-Reply-To: <1574234218-49195-5-git-send-email-decui@microsoft.com>
From: Dexuan Cui <decui@microsoft.com> Sent: Tuesday, November 19, 2019 11:17 PM
>
> The page allocated for struct hv_pcibus_device contains pointers to other
> slab allocations in new_pcichild_device(). Since kmemleak does not track
> and scan page allocations, the slab objects will be reported as leaks
> (false positives). Use the kmemleak APIs to allow tracking of such pages.
>
> Reported-by: Lili Deng <v-lide@microsoft.com>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
>
> This is actually v1. I use "v2" in the Subject just to be consistent with
> the other patches in the patchset.
>
> Without the patch, we can see the below warning in dmesg, if kmemleak is
> enabled:
>
> kmemleak: 1 new suspected memory leaks (see /sys/kernel/debug/kmemleak)
>
> and "cat /sys/kernel/debug/kmemleak" shows:
>
> unreferenced object 0xffff9217d1f2bec0 (size 192):
> comm "kworker/u256:7", pid 100821, jiffies 4501481057 (age 61409.997s)
> hex dump (first 32 bytes):
> a8 60 b1 63 17 92 ff ff a8 60 b1 63 17 92 ff ff .`.c.....`.c....
> 02 00 00 00 00 00 00 00 80 92 cd 61 17 92 ff ff ...........a....
> backtrace:
> [<0000000006f7ae93>] pci_devices_present_work+0x326/0x5e0 [pci_hyperv]
> [<00000000278eb88a>] process_one_work+0x15f/0x360
> [<00000000c59501e6>] worker_thread+0x49/0x3c0
> [<000000000a0a7a94>] kthread+0xf8/0x130
> [<000000007075c2e7>] ret_from_fork+0x35/0x40
>
> drivers/pci/controller/pci-hyperv.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index d7e05d436b5e..cc73f49c9e03 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -46,6 +46,7 @@
> #include <asm/irqdomain.h>
> #include <asm/apic.h>
> #include <linux/irq.h>
> +#include <linux/kmemleak.h>
> #include <linux/msi.h>
> #include <linux/hyperv.h>
> #include <linux/refcount.h>
> @@ -2907,6 +2908,16 @@ static int hv_pci_probe(struct hv_device *hdev,
> hbus = (struct hv_pcibus_device *)get_zeroed_page(GFP_KERNEL);
> if (!hbus)
> return -ENOMEM;
> +
> + /*
> + * kmemleak doesn't track page allocations as they are not commonly
> + * used for kernel data structures, but here hbus->children indeed
> + * contains pointers to hv_pci_dev structs, which are dynamically
> + * created in new_pcichild_device(). Allow kmemleak to scan the page
> + * so we don't get a false warning of memory leak.
> + */
> + kmemleak_alloc(hbus, PAGE_SIZE, 1, GFP_KERNEL);
> +
As noted in the existing comments, the hbus data structure must not cross
a page boundary, so that a portion of it can be used as a hypercall argument.
Historically kzalloc() didn't provide any alignment guarantee, so
get_zeroed_page() is used. But a recent commit (59bb47985c1d) changes
the behavior of kmalloc()/kzalloc() so that alignment *is* guaranteed for
power of 2 allocation sizes. With this change, the better fix is to use
kzalloc() instead of get_zeroed_page(). Note that the allocation size should
be HV_HYP_PAGE_SIZE instead of PAGE_SIZE, as the hbus structure must not
cross a page boundary based on Hyper-V's concept of a page, which may be
different from the guest page size on some architectures.
Michael
> hbus->state = hv_pcibus_init;
>
> /*
> @@ -3133,6 +3144,8 @@ static int hv_pci_remove(struct hv_device *hdev)
>
> hv_put_dom_num(hbus->sysdata.domain);
>
> + /* Remove kmemleak object previously allocated in hv_pci_probe() */
> + kmemleak_free(hbus);
> free_page((unsigned long)hbus);
> return ret;
> }
> --
> 2.19.1
^ permalink raw reply
* RE: [PATCH v2 3/4] PCI: hv: Change pci_protocol_version to per-hbus
From: Michael Kelley @ 2019-11-24 22:00 UTC (permalink / raw)
To: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, lorenzo.pieralisi@arm.com, bhelgaas@google.com,
linux-hyperv@vger.kernel.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, Sasha Levin
In-Reply-To: <1574234218-49195-4-git-send-email-decui@microsoft.com>
From: Dexuan Cui <decui@microsoft.com> Sent: Tuesday, November 19, 2019 11:17 PM
>
> A VM can have multiple Hyper-V hbus. It's incorrect to set the global
> variable 'pci_protocol_version' when *every* hbus is initialized in
> hv_pci_protocol_negotiation(). This is not an issue in practice since
> every hbus should have the same value of hbus->protocol_version, but
> we should make the variable per-hbus, so in case we have busses
> with different protocol_versions, the driver can still work correctly.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
>
> changes in v2: Improved the changelog by making it clear that this patch
> fixes a potential bug if we have busses with different protocol_versions.
>
> drivers/pci/controller/pci-hyperv.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [PATCH v2 2/4] PCI: hv: Add the support of hibernation
From: Michael Kelley @ 2019-11-24 22:19 UTC (permalink / raw)
To: Lorenzo Pieralisi, Dexuan Cui
Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, bhelgaas@google.com,
linux-hyperv@vger.kernel.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, Sasha Levin
In-Reply-To: <20191121114419.GA4318@e121166-lin.cambridge.arm.com>
From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Sent: Thursday, November 21, 2019 3:44 AM
>
> On Thu, Nov 21, 2019 at 12:50:17AM +0000, Dexuan Cui wrote:
> > > From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > Sent: Wednesday, November 20, 2019 9:20 AM
> > >
> > > On Tue, Nov 19, 2019 at 11:16:56PM -0800, Dexuan Cui wrote:
> > > > Implement the suspend/resume callbacks.
> > > >
> > > > We must make sure there is no pending work items before we call
> > > > vmbus_close().
> > >
> > > Where ? Why ? Imagine a developer reading this log to try to understand
> > > why you made this change, do you really think this commit log is
> > > informative in its current form ?
> > >
> > > I am not asking a book but this is a significant feature please make
> > > an effort to explain it (I can update the log for you but please
> > > write one and I shall do it).
> > >
> > > Lorenzo
> >
> > Sorry for being sloppy on this patch's changelog! Can you please use the
> > below? I can also post v3 with the new changelog if that's better.
>
> As you wish but more importantly get hyper-V maintainers to ACK these
> changes since time is running out for v5.5.
>
> Lorenzo
>
> > PCI: hv: Add the support of hibernation
> >
> > hv_pci_suspend() runs in a process context as a callback in dpm_suspend().
> > When it starts to run, the channel callback hv_pci_onchannelcallback(),
> > which runs in a tasklet context, can be still running concurrently and
> > scheduling new work items onto hbus->wq in hv_pci_devices_present() and
> > hv_pci_eject_device(), and the work item handlers can access the vmbus
> > channel, which can be being closed by hv_pci_suspend(), e.g. the work item
> > handler pci_devices_present_work() -> new_pcichild_device() writes to
> > the vmbus channel.
> >
> > To eliminate the race, hv_pci_suspend() disables the channel callback
> > tasklet, sets hbus->state to hv_pcibus_removing, and re-enables the tasklet.
> >
> > This way, when hv_pci_suspend() proceeds, it knows that no new work item
> > can be scheduled, and then it flushes hbus->wq and safely closes the vmbus
> > channel.
> >
> > Thanks,
> > -- Dexuan
FWIW, I'd like to see the above level of detail also as comments in the code
Itself so that whoever next looks at the code sees the explanation directly
without having to review the commit logs.
Also, the commit message doesn't say what the commit actually does and
why. I'd suggest the commit message along these lines:
Add suspend() and resume() functions so that Hyper-V virtual PCI devices are
handled properly when the VM hibernates and resumes from hibernation.
Note that the suspend() function must make sure there are no pending work
items before calling vmbus_close(), since it runs in a process context as a
callback in dpm_suspend(). When it starts to run, the channel callback
hv_pci_onchannelcallback(), which runs in a tasklet context, can be still running
concurrently and scheduling new work items onto hbus->wq in
hv_pci_devices_present() and hv_pci_eject_device(), and the work item
handlers can access the vmbus channel, which can be being closed by
hv_pci_suspend(), e.g. the work item handler pci_devices_present_work() ->
new_pcichild_device() writes to the vmbus channel.
To eliminate the race, hv_pci_suspend() disables the channel callback
tasklet, sets hbus->state to hv_pcibus_removing, and re-enables the tasklet.
This way, when hv_pci_suspend() proceeds, it knows that no new work item
can be scheduled, and then it flushes hbus->wq and safely closes the vmbus
channel.
Michael
^ permalink raw reply
* RE: [PATCH v2 1/4] PCI: hv: Reorganize the code in preparation of hibernation
From: Michael Kelley @ 2019-11-24 22:21 UTC (permalink / raw)
To: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, lorenzo.pieralisi@arm.com, bhelgaas@google.com,
linux-hyperv@vger.kernel.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, Sasha Levin
In-Reply-To: <1574234218-49195-2-git-send-email-decui@microsoft.com>
From: Dexuan Cui <decui@microsoft.com> Sent: Tuesday, November 19, 2019 11:17 PM
>
> There is no functional change. This is just preparatory to a later
> patch which adds the hibernation support for the pci-hyperv driver.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
>
> No change in v2.
>
> drivers/pci/controller/pci-hyperv.c | 43 +++++++++++++++++++----------
> 1 file changed, 28 insertions(+), 15 deletions(-)
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* [PATCH v3] Input: hyperv-keyboard: Add the support of hibernation
From: Dexuan Cui @ 2019-11-25 5:33 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, dmitry.torokhov, linux-hyperv,
linux-input, linux-kernel, mikelley, Alexander.Levin
Cc: Dexuan Cui
Add suspend() and resume() functions so the Hyper-V virtual keyboard
can participate in VM hibernation.
Note that the keyboard is a "wakeup" device that could abort an in-progress
hibernation if there is keyboard event. No attempt is made to suppress this
behavior. If desired, a sysadmin can disable the keyboard as a wakeup device
using standard mechanisms such as:
echo disabled > /sys/bus/vmbus/drivers/hyperv_keyboard/XXX/power/wakeup
(where XXX is the device's GUID)
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
This patch is basically a pure Hyper-V specific change. I suggest it should
go through the Hyper-V tree:
https://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git/log/?h=hyperv-next
Changes in v2:
I removed the "struct notifier_block pm_nb;" after the discussion
with Dmitry Torokhov:
https://lore.kernel.org/lkml/PU1P153MB016914A7C827CA35D7FEB66ABF8B0@PU1P153MB0169.APCP153.PROD.OUTLOOK.COM/T/#m8948c711301220a36a1a413eead74cd2fb6dcac1
Changes in v3:
Made the commit message more descriptive by using the version from Michael.
Added Michael's Reviewed-by.
drivers/input/serio/hyperv-keyboard.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/input/serio/hyperv-keyboard.c b/drivers/input/serio/hyperv-keyboard.c
index e486a8a74c40..df4e9f6f4529 100644
--- a/drivers/input/serio/hyperv-keyboard.c
+++ b/drivers/input/serio/hyperv-keyboard.c
@@ -259,6 +259,8 @@ static int hv_kbd_connect_to_vsp(struct hv_device *hv_dev)
u32 proto_status;
int error;
+ reinit_completion(&kbd_dev->wait_event);
+
request = &kbd_dev->protocol_req;
memset(request, 0, sizeof(struct synth_kbd_protocol_request));
request->header.type = __cpu_to_le32(SYNTH_KBD_PROTOCOL_REQUEST);
@@ -380,6 +382,29 @@ static int hv_kbd_remove(struct hv_device *hv_dev)
return 0;
}
+static int hv_kbd_suspend(struct hv_device *hv_dev)
+{
+ vmbus_close(hv_dev->channel);
+
+ return 0;
+}
+
+static int hv_kbd_resume(struct hv_device *hv_dev)
+{
+ int ret;
+
+ ret = vmbus_open(hv_dev->channel,
+ KBD_VSC_SEND_RING_BUFFER_SIZE,
+ KBD_VSC_RECV_RING_BUFFER_SIZE,
+ NULL, 0,
+ hv_kbd_on_channel_callback,
+ hv_dev);
+ if (ret == 0)
+ ret = hv_kbd_connect_to_vsp(hv_dev);
+
+ return ret;
+}
+
static const struct hv_vmbus_device_id id_table[] = {
/* Keyboard guid */
{ HV_KBD_GUID, },
@@ -393,6 +418,8 @@ static struct hv_driver hv_kbd_drv = {
.id_table = id_table,
.probe = hv_kbd_probe,
.remove = hv_kbd_remove,
+ .suspend = hv_kbd_suspend,
+ .resume = hv_kbd_resume,
.driver = {
.probe_type = PROBE_PREFER_ASYNCHRONOUS,
},
--
2.19.1
^ permalink raw reply related
* [PATCH v3 0/4] Enhance pci-hyperv to support hibernation, and 2 misc fixes
From: Dexuan Cui @ 2019-11-25 5:33 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, lorenzo.pieralisi, bhelgaas,
linux-hyperv, linux-pci, linux-kernel, mikelley, Alexander.Levin
Cc: Dexuan Cui
I suggest the patchset goes through the pci.git tree.
Patch #1: no functional change.
Patch #2 enhances the pci-hyperv driver to support hibernation.
Patch #3 is unrelated to hibernation.
Patch #4 is unrelated to hibernation.
Changes in v3:
Patch #1: Added Michael Kelley's Signed-off-by.
Patch #2: Used a better commit log message from Michael Kelley.
Patch #3: Added Michael Kelley's Signed-off-by.
Patch #4: Used kzalloc() rather than get_zeroed_page()/kmemleak_alloc()/
kmemleak_free(), and added the necessary comments.
Michael, can you please review #2 and #4 again?
Dexuan Cui (4):
PCI: hv: Reorganize the code in preparation of hibernation
PCI: hv: Add the support of hibernation
PCI: hv: Change pci_protocol_version to per-hbus
PCI: hv: Avoid a kmemleak false positive caused by the hbus buffer
drivers/pci/controller/pci-hyperv.c | 208 ++++++++++++++++++++++++----
1 file changed, 179 insertions(+), 29 deletions(-)
--
2.19.1
^ permalink raw reply
* [PATCH v3 1/4] PCI: hv: Reorganize the code in preparation of hibernation
From: Dexuan Cui @ 2019-11-25 5:33 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, lorenzo.pieralisi, bhelgaas,
linux-hyperv, linux-pci, linux-kernel, mikelley, Alexander.Levin
Cc: Dexuan Cui
In-Reply-To: <1574660034-98780-1-git-send-email-decui@microsoft.com>
There is no functional change. This is just preparatory to a later
patch which adds the hibernation support for the pci-hyperv driver.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
---
drivers/pci/controller/pci-hyperv.c | 43 +++++++++++++++++++----------
1 file changed, 28 insertions(+), 15 deletions(-)
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index f1f300218fab..65f18f840ce9 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -2379,7 +2379,9 @@ static void hv_pci_onchannelcallback(void *context)
* failing if the host doesn't support the necessary protocol
* level.
*/
-static int hv_pci_protocol_negotiation(struct hv_device *hdev)
+static int hv_pci_protocol_negotiation(struct hv_device *hdev,
+ enum pci_protocol_version_t version[],
+ int num_version)
{
struct pci_version_request *version_req;
struct hv_pci_compl comp_pkt;
@@ -2403,8 +2405,8 @@ static int hv_pci_protocol_negotiation(struct hv_device *hdev)
version_req = (struct pci_version_request *)&pkt->message;
version_req->message_type.type = PCI_QUERY_PROTOCOL_VERSION;
- for (i = 0; i < ARRAY_SIZE(pci_protocol_versions); i++) {
- version_req->protocol_version = pci_protocol_versions[i];
+ for (i = 0; i < num_version; i++) {
+ version_req->protocol_version = version[i];
ret = vmbus_sendpacket(hdev->channel, version_req,
sizeof(struct pci_version_request),
(unsigned long)pkt, VM_PKT_DATA_INBAND,
@@ -2420,7 +2422,7 @@ static int hv_pci_protocol_negotiation(struct hv_device *hdev)
}
if (comp_pkt.completion_status >= 0) {
- pci_protocol_version = pci_protocol_versions[i];
+ pci_protocol_version = version[i];
dev_info(&hdev->device,
"PCI VMBus probing: Using version %#x\n",
pci_protocol_version);
@@ -2930,7 +2932,8 @@ static int hv_pci_probe(struct hv_device *hdev,
hv_set_drvdata(hdev, hbus);
- ret = hv_pci_protocol_negotiation(hdev);
+ ret = hv_pci_protocol_negotiation(hdev, pci_protocol_versions,
+ ARRAY_SIZE(pci_protocol_versions));
if (ret)
goto close;
@@ -3011,7 +3014,7 @@ static int hv_pci_probe(struct hv_device *hdev,
return ret;
}
-static void hv_pci_bus_exit(struct hv_device *hdev)
+static int hv_pci_bus_exit(struct hv_device *hdev, bool hibernating)
{
struct hv_pcibus_device *hbus = hv_get_drvdata(hdev);
struct {
@@ -3027,16 +3030,20 @@ static void hv_pci_bus_exit(struct hv_device *hdev)
* access the per-channel ringbuffer any longer.
*/
if (hdev->channel->rescind)
- return;
+ return 0;
- /* Delete any children which might still exist. */
- memset(&relations, 0, sizeof(relations));
- hv_pci_devices_present(hbus, &relations);
+ if (!hibernating) {
+ /* Delete any children which might still exist. */
+ memset(&relations, 0, sizeof(relations));
+ hv_pci_devices_present(hbus, &relations);
+ }
ret = hv_send_resources_released(hdev);
- if (ret)
+ if (ret) {
dev_err(&hdev->device,
"Couldn't send resources released packet(s)\n");
+ return ret;
+ }
memset(&pkt.teardown_packet, 0, sizeof(pkt.teardown_packet));
init_completion(&comp_pkt.host_event);
@@ -3049,8 +3056,13 @@ static void hv_pci_bus_exit(struct hv_device *hdev)
(unsigned long)&pkt.teardown_packet,
VM_PKT_DATA_INBAND,
VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
- if (!ret)
- wait_for_completion_timeout(&comp_pkt.host_event, 10 * HZ);
+ if (ret)
+ return ret;
+
+ if (wait_for_completion_timeout(&comp_pkt.host_event, 10 * HZ) == 0)
+ return -ETIMEDOUT;
+
+ return 0;
}
/**
@@ -3062,6 +3074,7 @@ static void hv_pci_bus_exit(struct hv_device *hdev)
static int hv_pci_remove(struct hv_device *hdev)
{
struct hv_pcibus_device *hbus;
+ int ret;
hbus = hv_get_drvdata(hdev);
if (hbus->state == hv_pcibus_installed) {
@@ -3074,7 +3087,7 @@ static int hv_pci_remove(struct hv_device *hdev)
hbus->state = hv_pcibus_removed;
}
- hv_pci_bus_exit(hdev);
+ ret = hv_pci_bus_exit(hdev, false);
vmbus_close(hdev->channel);
@@ -3091,7 +3104,7 @@ static int hv_pci_remove(struct hv_device *hdev)
hv_put_dom_num(hbus->sysdata.domain);
free_page((unsigned long)hbus);
- return 0;
+ return ret;
}
static const struct hv_vmbus_device_id hv_pci_id_table[] = {
--
2.19.1
^ permalink raw reply related
* [PATCH v3 4/4] PCI: hv: Avoid a kmemleak false positive caused by the hbus buffer
From: Dexuan Cui @ 2019-11-25 5:33 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, lorenzo.pieralisi, bhelgaas,
linux-hyperv, linux-pci, linux-kernel, mikelley, Alexander.Levin
Cc: Dexuan Cui
In-Reply-To: <1574660034-98780-1-git-send-email-decui@microsoft.com>
With the recent 59bb47985c1d ("mm, sl[aou]b: guarantee natural
alignment for kmalloc(power-of-two)"), kzalloc() is able to allocate
a 4KB buffer that is guaranteed to be 4KB-aligned. Here the size and
alignment of hbus is important because hbus's field
retarget_msi_interrupt_params must not cross a 4KB page boundary.
Here we prefer kzalloc to get_zeroed_page(), because a buffer
allocated by the latter is not tracked and scanned by kmemleak, and
hence kmemleak reports the pointer contained in the hbus buffer
(i.e. the hpdev struct, which is created in new_pcichild_device() and
is tracked by hbus->children) as memory leak (false positive).
If the kernel doesn't have 59bb47985c1d, get_zeroed_page() *must* be
used to allocate the hbus buffer and we can avoid the kmemleak false
positive by using kmemleak_alloc() and kmemleak_free() to ask
kmemleak to track and scan the hbus buffer.
Reported-by: Lili Deng <v-lide@microsoft.com>
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
drivers/pci/controller/pci-hyperv.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 910fa016d095..be99862166d0 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -2902,9 +2902,27 @@ static int hv_pci_probe(struct hv_device *hdev,
* hv_pcibus_device contains the hypercall arguments for retargeting in
* hv_irq_unmask(). Those must not cross a page boundary.
*/
- BUILD_BUG_ON(sizeof(*hbus) > PAGE_SIZE);
+ BUILD_BUG_ON(sizeof(*hbus) > HV_HYP_PAGE_SIZE);
- hbus = (struct hv_pcibus_device *)get_zeroed_page(GFP_KERNEL);
+ /*
+ * With the recent 59bb47985c1d ("mm, sl[aou]b: guarantee natural
+ * alignment for kmalloc(power-of-two)"), kzalloc() is able to allocate
+ * a 4KB buffer that is guaranteed to be 4KB-aligned. Here the size and
+ * alignment of hbus is important because hbus's field
+ * retarget_msi_interrupt_params must not cross a 4KB page boundary.
+ *
+ * Here we prefer kzalloc to get_zeroed_page(), because a buffer
+ * allocated by the latter is not tracked and scanned by kmemleak, and
+ * hence kmemleak reports the pointer contained in the hbus buffer
+ * (i.e. the hpdev struct, which is created in new_pcichild_device() and
+ * is tracked by hbus->children) as memory leak (false positive).
+ *
+ * If the kernel doesn't have 59bb47985c1d, get_zeroed_page() *must* be
+ * used to allocate the hbus buffer and we can avoid the kmemleak false
+ * positive by using kmemleak_alloc() and kmemleak_free() to ask
+ * kmemleak to track and scan the hbus buffer.
+ */
+ hbus = (struct hv_pcibus_device *)kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
if (!hbus)
return -ENOMEM;
hbus->state = hv_pcibus_init;
@@ -3133,7 +3151,7 @@ static int hv_pci_remove(struct hv_device *hdev)
hv_put_dom_num(hbus->sysdata.domain);
- free_page((unsigned long)hbus);
+ kfree(hbus);
return ret;
}
--
2.19.1
^ permalink raw reply related
* [PATCH v3 3/4] PCI: hv: Change pci_protocol_version to per-hbus
From: Dexuan Cui @ 2019-11-25 5:33 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, lorenzo.pieralisi, bhelgaas,
linux-hyperv, linux-pci, linux-kernel, mikelley, Alexander.Levin
Cc: Dexuan Cui
In-Reply-To: <1574660034-98780-1-git-send-email-decui@microsoft.com>
A VM can have multiple Hyper-V hbus. It's incorrect to set the global
variable 'pci_protocol_version' when *every* hbus is initialized in
hv_pci_protocol_negotiation(). This is not an issue in practice since
every hbus should have the same value of hbus->protocol_version, but
we should make the variable per-hbus, so in case we have busses
with different protocol_versions, the driver can still work correctly.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
---
drivers/pci/controller/pci-hyperv.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 3c4996b073ca..910fa016d095 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -76,11 +76,6 @@ static enum pci_protocol_version_t pci_protocol_versions[] = {
PCI_PROTOCOL_VERSION_1_1,
};
-/*
- * Protocol version negotiated by hv_pci_protocol_negotiation().
- */
-static enum pci_protocol_version_t pci_protocol_version;
-
#define PCI_CONFIG_MMIO_LENGTH 0x2000
#define CFG_PAGE_OFFSET 0x1000
#define CFG_PAGE_SIZE (PCI_CONFIG_MMIO_LENGTH - CFG_PAGE_OFFSET)
@@ -462,6 +457,8 @@ enum hv_pcibus_state {
struct hv_pcibus_device {
struct pci_sysdata sysdata;
+ /* Protocol version negotiated with the host */
+ enum pci_protocol_version_t protocol_version;
enum hv_pcibus_state state;
refcount_t remove_lock;
struct hv_device *hdev;
@@ -1225,7 +1222,7 @@ static void hv_irq_unmask(struct irq_data *data)
* negative effect (yet?).
*/
- if (pci_protocol_version >= PCI_PROTOCOL_VERSION_1_2) {
+ if (hbus->protocol_version >= PCI_PROTOCOL_VERSION_1_2) {
/*
* PCI_PROTOCOL_VERSION_1_2 supports the VP_SET version of the
* HVCALL_RETARGET_INTERRUPT hypercall, which also coincides
@@ -1395,7 +1392,7 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
ctxt.pci_pkt.completion_func = hv_pci_compose_compl;
ctxt.pci_pkt.compl_ctxt = ∁
- switch (pci_protocol_version) {
+ switch (hbus->protocol_version) {
case PCI_PROTOCOL_VERSION_1_1:
size = hv_compose_msi_req_v1(&ctxt.int_pkts.v1,
dest,
@@ -2415,6 +2412,7 @@ static int hv_pci_protocol_negotiation(struct hv_device *hdev,
enum pci_protocol_version_t version[],
int num_version)
{
+ struct hv_pcibus_device *hbus = hv_get_drvdata(hdev);
struct pci_version_request *version_req;
struct hv_pci_compl comp_pkt;
struct pci_packet *pkt;
@@ -2454,10 +2452,10 @@ static int hv_pci_protocol_negotiation(struct hv_device *hdev,
}
if (comp_pkt.completion_status >= 0) {
- pci_protocol_version = version[i];
+ hbus->protocol_version = version[i];
dev_info(&hdev->device,
"PCI VMBus probing: Using version %#x\n",
- pci_protocol_version);
+ hbus->protocol_version);
goto exit;
}
@@ -2741,7 +2739,7 @@ static int hv_send_resources_allocated(struct hv_device *hdev)
u32 wslot;
int ret;
- size_res = (pci_protocol_version < PCI_PROTOCOL_VERSION_1_2)
+ size_res = (hbus->protocol_version < PCI_PROTOCOL_VERSION_1_2)
? sizeof(*res_assigned) : sizeof(*res_assigned2);
pkt = kmalloc(sizeof(*pkt) + size_res, GFP_KERNEL);
@@ -2760,7 +2758,7 @@ static int hv_send_resources_allocated(struct hv_device *hdev)
pkt->completion_func = hv_pci_generic_compl;
pkt->compl_ctxt = &comp_pkt;
- if (pci_protocol_version < PCI_PROTOCOL_VERSION_1_2) {
+ if (hbus->protocol_version < PCI_PROTOCOL_VERSION_1_2) {
res_assigned =
(struct pci_resources_assigned *)&pkt->message;
res_assigned->message_type.type =
@@ -3200,7 +3198,7 @@ static int hv_pci_resume(struct hv_device *hdev)
return ret;
/* Only use the version that was in use before hibernation. */
- version[0] = pci_protocol_version;
+ version[0] = hbus->protocol_version;
ret = hv_pci_protocol_negotiation(hdev, version, 1);
if (ret)
goto out;
--
2.19.1
^ permalink raw reply related
* [PATCH v3 2/4] PCI: hv: Add the support of hibernation
From: Dexuan Cui @ 2019-11-25 5:33 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, lorenzo.pieralisi, bhelgaas,
linux-hyperv, linux-pci, linux-kernel, mikelley, Alexander.Levin
Cc: Dexuan Cui
In-Reply-To: <1574660034-98780-1-git-send-email-decui@microsoft.com>
Add suspend() and resume() functions so that Hyper-V virtual PCI devices are
handled properly when the VM hibernates and resumes from hibernation.
Note that the suspend() function must make sure there are no pending work
items before calling vmbus_close(), since it runs in a process context as a
callback in dpm_suspend(). When it starts to run, the channel callback
hv_pci_onchannelcallback(), which runs in a tasklet context, can be still running
concurrently and scheduling new work items onto hbus->wq in
hv_pci_devices_present() and hv_pci_eject_device(), and the work item
handlers can access the vmbus channel, which can be being closed by
hv_pci_suspend(), e.g. the work item handler pci_devices_present_work() ->
new_pcichild_device() writes to the vmbus channel.
To eliminate the race, hv_pci_suspend() disables the channel callback
tasklet, sets hbus->state to hv_pcibus_removing, and re-enables the tasklet.
This way, when hv_pci_suspend() proceeds, it knows that no new work item
can be scheduled, and then it flushes hbus->wq and safely closes the vmbus
channel.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
drivers/pci/controller/pci-hyperv.c | 125 +++++++++++++++++++++++++++-
1 file changed, 123 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 65f18f840ce9..3c4996b073ca 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -455,6 +455,7 @@ enum hv_pcibus_state {
hv_pcibus_init = 0,
hv_pcibus_probed,
hv_pcibus_installed,
+ hv_pcibus_removing,
hv_pcibus_removed,
hv_pcibus_maximum
};
@@ -1681,6 +1682,23 @@ static void prepopulate_bars(struct hv_pcibus_device *hbus)
spin_lock_irqsave(&hbus->device_list_lock, flags);
+ /*
+ * Clear the memory enable bit, in case it's already set. This occurs
+ * in the suspend path of hibernation, where the device is suspended,
+ * resumed and suspended again: see hibernation_snapshot() and
+ * hibernation_platform_enter().
+ *
+ * If the memory enable bit is already set, Hyper-V sliently ignores
+ * the below BAR updates, and the related PCI device driver can not
+ * work, because reading from the device register(s) always returns
+ * 0xFFFFFFFF.
+ */
+ list_for_each_entry(hpdev, &hbus->children, list_entry) {
+ _hv_pcifront_read_config(hpdev, PCI_COMMAND, 2, &command);
+ command &= ~PCI_COMMAND_MEMORY;
+ _hv_pcifront_write_config(hpdev, PCI_COMMAND, 2, command);
+ }
+
/* Pick addresses for the BARs. */
do {
list_for_each_entry(hpdev, &hbus->children, list_entry) {
@@ -2107,6 +2125,12 @@ static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
unsigned long flags;
bool pending_dr;
+ if (hbus->state == hv_pcibus_removing) {
+ dev_info(&hbus->hdev->device,
+ "PCI VMBus BUS_RELATIONS: ignored\n");
+ return;
+ }
+
dr_wrk = kzalloc(sizeof(*dr_wrk), GFP_NOWAIT);
if (!dr_wrk)
return;
@@ -2223,11 +2247,19 @@ static void hv_eject_device_work(struct work_struct *work)
*/
static void hv_pci_eject_device(struct hv_pci_dev *hpdev)
{
+ struct hv_pcibus_device *hbus = hpdev->hbus;
+ struct hv_device *hdev = hbus->hdev;
+
+ if (hbus->state == hv_pcibus_removing) {
+ dev_info(&hdev->device, "PCI VMBus EJECT: ignored\n");
+ return;
+ }
+
hpdev->state = hv_pcichild_ejecting;
get_pcichild(hpdev);
INIT_WORK(&hpdev->wrk, hv_eject_device_work);
- get_hvpcibus(hpdev->hbus);
- queue_work(hpdev->hbus->wq, &hpdev->wrk);
+ get_hvpcibus(hbus);
+ queue_work(hbus->wq, &hpdev->wrk);
}
/**
@@ -3107,6 +3139,93 @@ static int hv_pci_remove(struct hv_device *hdev)
return ret;
}
+static int hv_pci_suspend(struct hv_device *hdev)
+{
+ struct hv_pcibus_device *hbus = hv_get_drvdata(hdev);
+ enum hv_pcibus_state old_state;
+ int ret;
+
+ /*
+ * hv_pci_suspend() must make sure there are no pending work items
+ * before calling vmbus_close(), since it runs in a process context
+ * as a callback in dpm_suspend(). When it starts to run, the channel
+ * callback hv_pci_onchannelcallback(), which runs in a tasklet
+ * context, can be still running concurrently and scheduling new work
+ * items onto hbus->wq in hv_pci_devices_present() and
+ * hv_pci_eject_device(), and the work item handlers can access the
+ * vmbus channel, which can be being closed by hv_pci_suspend(), e.g.
+ * the work item handler pci_devices_present_work() ->
+ * new_pcichild_device() writes to the vmbus channel.
+ *
+ * To eliminate the race, hv_pci_suspend() disables the channel
+ * callback tasklet, sets hbus->state to hv_pcibus_removing, and
+ * re-enables the tasklet. This way, when hv_pci_suspend() proceeds,
+ * it knows that no new work item can be scheduled, and then it flushes
+ * hbus->wq and safely closes the vmbus channel.
+ */
+ tasklet_disable(&hdev->channel->callback_event);
+
+ /* Change the hbus state to prevent new work items. */
+ old_state = hbus->state;
+ if (hbus->state == hv_pcibus_installed)
+ hbus->state = hv_pcibus_removing;
+
+ tasklet_enable(&hdev->channel->callback_event);
+
+ if (old_state != hv_pcibus_installed)
+ return -EINVAL;
+
+ flush_workqueue(hbus->wq);
+
+ ret = hv_pci_bus_exit(hdev, true);
+ if (ret)
+ return ret;
+
+ vmbus_close(hdev->channel);
+
+ return 0;
+}
+
+static int hv_pci_resume(struct hv_device *hdev)
+{
+ struct hv_pcibus_device *hbus = hv_get_drvdata(hdev);
+ enum pci_protocol_version_t version[1];
+ int ret;
+
+ hbus->state = hv_pcibus_init;
+
+ ret = vmbus_open(hdev->channel, pci_ring_size, pci_ring_size, NULL, 0,
+ hv_pci_onchannelcallback, hbus);
+ if (ret)
+ return ret;
+
+ /* Only use the version that was in use before hibernation. */
+ version[0] = pci_protocol_version;
+ ret = hv_pci_protocol_negotiation(hdev, version, 1);
+ if (ret)
+ goto out;
+
+ ret = hv_pci_query_relations(hdev);
+ if (ret)
+ goto out;
+
+ ret = hv_pci_enter_d0(hdev);
+ if (ret)
+ goto out;
+
+ ret = hv_send_resources_allocated(hdev);
+ if (ret)
+ goto out;
+
+ prepopulate_bars(hbus);
+
+ hbus->state = hv_pcibus_installed;
+ return 0;
+out:
+ vmbus_close(hdev->channel);
+ return ret;
+}
+
static const struct hv_vmbus_device_id hv_pci_id_table[] = {
/* PCI Pass-through Class ID */
/* 44C4F61D-4444-4400-9D52-802E27EDE19F */
@@ -3121,6 +3240,8 @@ static struct hv_driver hv_pci_drv = {
.id_table = hv_pci_id_table,
.probe = hv_pci_probe,
.remove = hv_pci_remove,
+ .suspend = hv_pci_suspend,
+ .resume = hv_pci_resume,
};
static void __exit exit_hv_pci_drv(void)
--
2.19.1
^ permalink raw reply related
* Re: [PATCH v2] video: hyperv: hyperv_fb: Use physical memory for fb on HyperV Gen 1 VMs.
From: kbuild test robot @ 2019-11-25 7:04 UTC (permalink / raw)
To: Wei Hu
Cc: kbuild-all, b.zolnierkie, kys, haiyangz, sthemmin, sashal, hch,
m.szyprowski, mchehab+samsung, sam, gregkh, alexandre.belloni,
info, arnd, dri-devel, linux-fbdev, linux-kernel, linux-hyperv,
decui, mikelley, Wei Hu
In-Reply-To: <20191122082408.3210-1-weh@microsoft.com>
[-- Attachment #1: Type: text/plain, Size: 3741 bytes --]
Hi Wei,
I love your patch! Perhaps something to improve:
[auto build test WARNING on next-20191122]
[cannot apply to linus/master v5.4-rc8 v5.4-rc7 v5.4-rc6 v5.4]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Wei-Hu/video-hyperv-hyperv_fb-Use-physical-memory-for-fb-on-HyperV-Gen-1-VMs/20191124-163533
base: b9d3d01405061bb42358fe53f824e894a1922ced
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-14) 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 warnings (new ones prefixed by >>):
In file included from include/linux/printk.h:7:0,
from include/linux/kernel.h:15,
from include/linux/list.h:9,
from include/linux/module.h:12,
from drivers/video/fbdev/hyperv_fb.c:48:
drivers/video/fbdev/hyperv_fb.c: In function 'hvfb_get_phymem':
include/linux/kern_levels.h:5:18: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 3 has type 'phys_addr_t {aka unsigned int}' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:14:19: note: in expansion of macro 'KERN_SOH'
#define KERN_INFO KERN_SOH "6" /* informational */
^~~~~~~~
include/linux/printk.h:311:9: note: in expansion of macro 'KERN_INFO'
printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~
>> drivers/video/fbdev/hyperv_fb.c:1003:2: note: in expansion of macro 'pr_info'
pr_info("Allocated %d pages starts at physical addr 0x%llx\n",
^~~~~~~
vim +/pr_info +1003 drivers/video/fbdev/hyperv_fb.c
955
956 /*
957 * Allocate enough contiguous physical memory.
958 * Return physical address if succeeded or -1 if failed.
959 */
960 static phys_addr_t hvfb_get_phymem(struct hv_device *hdev,
961 unsigned int request_size)
962 {
963 struct page *page = NULL;
964 dma_addr_t dma_handle;
965 void *vmem;
966 unsigned int request_pages;
967 phys_addr_t paddr = 0;
968 unsigned int order = get_order(request_size);
969
970 if (request_size == 0)
971 return -1;
972
973 /* Try to call alloc_pages if the size is less than 2^MAX_ORDER */
974 if (order < MAX_ORDER) {
975 page = alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
976 if (!page)
977 return -1;
978
979 paddr = (page_to_pfn(page) << PAGE_SHIFT);
980 request_pages = (1 << order);
981 goto get_phymem1;
982 }
983
984 /* Allocate from CMA */
985 if (hdev == NULL)
986 return -1;
987
988 hdev->device.coherent_dma_mask = DMA_BIT_MASK(64);
989
990 request_pages = (round_up(request_size, PAGE_SIZE) >> PAGE_SHIFT);
991
992 vmem = dma_alloc_coherent(&hdev->device,
993 request_pages * PAGE_SIZE,
994 &dma_handle,
995 GFP_KERNEL | __GFP_NOWARN);
996
997 if (!vmem)
998 return -1;
999
1000 paddr = virt_to_phys(vmem);
1001
1002 get_phymem1:
> 1003 pr_info("Allocated %d pages starts at physical addr 0x%llx\n",
1004 request_pages, paddr);
1005
1006 return paddr;
1007 }
1008
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 71011 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/4] PCI: hv: Add the support of hibernation
From: Lorenzo Pieralisi @ 2019-11-25 10:44 UTC (permalink / raw)
To: Michael Kelley
Cc: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, bhelgaas@google.com,
linux-hyperv@vger.kernel.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, Sasha Levin
In-Reply-To: <CY4PR21MB06290283219FC78C45688DC4D74B0@CY4PR21MB0629.namprd21.prod.outlook.com>
On Sun, Nov 24, 2019 at 10:19:46PM +0000, Michael Kelley wrote:
> From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Sent: Thursday, November 21, 2019 3:44 AM
> >
> > On Thu, Nov 21, 2019 at 12:50:17AM +0000, Dexuan Cui wrote:
> > > > From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > > Sent: Wednesday, November 20, 2019 9:20 AM
> > > >
> > > > On Tue, Nov 19, 2019 at 11:16:56PM -0800, Dexuan Cui wrote:
> > > > > Implement the suspend/resume callbacks.
> > > > >
> > > > > We must make sure there is no pending work items before we call
> > > > > vmbus_close().
> > > >
> > > > Where ? Why ? Imagine a developer reading this log to try to understand
> > > > why you made this change, do you really think this commit log is
> > > > informative in its current form ?
> > > >
> > > > I am not asking a book but this is a significant feature please make
> > > > an effort to explain it (I can update the log for you but please
> > > > write one and I shall do it).
> > > >
> > > > Lorenzo
> > >
> > > Sorry for being sloppy on this patch's changelog! Can you please use the
> > > below? I can also post v3 with the new changelog if that's better.
> >
> > As you wish but more importantly get hyper-V maintainers to ACK these
> > changes since time is running out for v5.5.
> >
> > Lorenzo
> >
> > > PCI: hv: Add the support of hibernation
> > >
> > > hv_pci_suspend() runs in a process context as a callback in dpm_suspend().
> > > When it starts to run, the channel callback hv_pci_onchannelcallback(),
> > > which runs in a tasklet context, can be still running concurrently and
> > > scheduling new work items onto hbus->wq in hv_pci_devices_present() and
> > > hv_pci_eject_device(), and the work item handlers can access the vmbus
> > > channel, which can be being closed by hv_pci_suspend(), e.g. the work item
> > > handler pci_devices_present_work() -> new_pcichild_device() writes to
> > > the vmbus channel.
> > >
> > > To eliminate the race, hv_pci_suspend() disables the channel callback
> > > tasklet, sets hbus->state to hv_pcibus_removing, and re-enables the tasklet.
> > >
> > > This way, when hv_pci_suspend() proceeds, it knows that no new work item
> > > can be scheduled, and then it flushes hbus->wq and safely closes the vmbus
> > > channel.
> > >
> > > Thanks,
> > > -- Dexuan
>
> FWIW, I'd like to see the above level of detail also as comments in the code
> Itself so that whoever next looks at the code sees the explanation directly
> without having to review the commit logs.
>
> Also, the commit message doesn't say what the commit actually does and
> why. I'd suggest the commit message along these lines:
>
> Add suspend() and resume() functions so that Hyper-V virtual PCI devices are
> handled properly when the VM hibernates and resumes from hibernation.
>
> Note that the suspend() function must make sure there are no pending work
> items before calling vmbus_close(), since it runs in a process context as a
> callback in dpm_suspend(). When it starts to run, the channel callback
> hv_pci_onchannelcallback(), which runs in a tasklet context, can be still running
> concurrently and scheduling new work items onto hbus->wq in
> hv_pci_devices_present() and hv_pci_eject_device(), and the work item
> handlers can access the vmbus channel, which can be being closed by
> hv_pci_suspend(), e.g. the work item handler pci_devices_present_work() ->
> new_pcichild_device() writes to the vmbus channel.
>
> To eliminate the race, hv_pci_suspend() disables the channel callback
> tasklet, sets hbus->state to hv_pcibus_removing, and re-enables the tasklet.
> This way, when hv_pci_suspend() proceeds, it knows that no new work item
> can be scheduled, and then it flushes hbus->wq and safely closes the vmbus
> channel.
This is much better, thank you, if you are happy with the patches
please add your tags so that I can pull the series asap, hopefully
we can merge it in v5.5.
Thanks,
Lorenzo
^ permalink raw reply
* Re: [PATCH v2] video: hyperv: hyperv_fb: Use physical memory for fb on HyperV Gen 1 VMs.
From: kbuild test robot @ 2019-11-25 13:07 UTC (permalink / raw)
To: Wei Hu
Cc: kbuild-all, b.zolnierkie, kys, haiyangz, sthemmin, sashal, hch,
m.szyprowski, mchehab+samsung, sam, gregkh, alexandre.belloni,
info, arnd, dri-devel, linux-fbdev, linux-kernel, linux-hyperv,
decui, mikelley, Wei Hu
In-Reply-To: <20191122082408.3210-1-weh@microsoft.com>
[-- Attachment #1: Type: text/plain, Size: 10810 bytes --]
Hi Wei,
I love your patch! Yet something to improve:
[auto build test ERROR on next-20191122]
[cannot apply to linus/master v5.4-rc8 v5.4-rc7 v5.4-rc6 v5.4]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Wei-Hu/video-hyperv-hyperv_fb-Use-physical-memory-for-fb-on-HyperV-Gen-1-VMs/20191124-163533
base: b9d3d01405061bb42358fe53f824e894a1922ced
config: i386-randconfig-b003-20191125 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-14) 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 >>):
ld: kernel/dma/contiguous.o: in function `dma_alloc_from_contiguous':
>> kernel/dma/contiguous.c:199: undefined reference to `cma_alloc'
>> ld: kernel/dma/contiguous.c:199: undefined reference to `cma_alloc'
ld: kernel/dma/contiguous.o: in function `dma_release_from_contiguous':
>> kernel/dma/contiguous.c:215: undefined reference to `cma_release'
>> ld: kernel/dma/contiguous.c:215: undefined reference to `cma_release'
ld: kernel/dma/contiguous.o: in function `dma_alloc_contiguous':
kernel/dma/contiguous.c:248: undefined reference to `cma_alloc'
ld: kernel/dma/contiguous.o: in function `dma_free_contiguous':
kernel/dma/contiguous.c:267: undefined reference to `cma_release'
ld: kernel/dma/contiguous.c:267: undefined reference to `cma_release'
ld: kernel/dma/contiguous.o: in function `dma_contiguous_reserve_area':
>> kernel/dma/contiguous.c:169: undefined reference to `cma_declare_contiguous'
>> ld: kernel/dma/contiguous.c:175: undefined reference to `cma_get_size'
>> ld: kernel/dma/contiguous.c:175: undefined reference to `cma_get_base'
vim +199 kernel/dma/contiguous.c
c64be2bb1c6eb4 drivers/base/dma-contiguous.c Marek Szyprowski 2011-12-29 145
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 146 /**
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 147 * dma_contiguous_reserve_area() - reserve custom contiguous area
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 148 * @size: Size of the reserved area (in bytes),
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 149 * @base: Base address of the reserved area optional, use 0 for any
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 150 * @limit: End address of the reserved memory (optional, 0 for any).
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 151 * @res_cma: Pointer to store the created cma region.
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 152 * @fixed: hint about where to place the reserved area
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 153 *
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 154 * This function reserves memory from early allocator. It should be
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 155 * called by arch specific code once the early allocator (memblock or bootmem)
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 156 * has been activated and all other subsystems have already allocated/reserved
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 157 * memory. This function allows to create custom reserved areas for specific
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 158 * devices.
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 159 *
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 160 * If @fixed is true, reserve contiguous area at exactly @base. If false,
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 161 * reserve in range from @base to @limit.
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 162 */
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 163 int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 164 phys_addr_t limit, struct cma **res_cma,
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 165 bool fixed)
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 166 {
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 167 int ret;
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 168
f318dd083c8128 drivers/base/dma-contiguous.c Laura Abbott 2017-04-18 @169 ret = cma_declare_contiguous(base, size, limit, 0, 0, fixed,
f318dd083c8128 drivers/base/dma-contiguous.c Laura Abbott 2017-04-18 170 "reserved", res_cma);
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 171 if (ret)
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 172 return ret;
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 173
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 174 /* Architecture specific contiguous memory fixup. */
a254129e8686bf drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 @175 dma_contiguous_early_fixup(cma_get_base(*res_cma),
a254129e8686bf drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 176 cma_get_size(*res_cma));
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 177
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 178 return 0;
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 179 }
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 180
c64be2bb1c6eb4 drivers/base/dma-contiguous.c Marek Szyprowski 2011-12-29 181 /**
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 182 * dma_alloc_from_contiguous() - allocate pages from contiguous area
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 183 * @dev: Pointer to device for which the allocation is performed.
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 184 * @count: Requested number of pages.
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 185 * @align: Requested alignment of pages (in PAGE_SIZE order).
d834c5ab83febf kernel/dma/contiguous.c Marek Szyprowski 2018-08-17 186 * @no_warn: Avoid printing message about failed allocation.
c64be2bb1c6eb4 drivers/base/dma-contiguous.c Marek Szyprowski 2011-12-29 187 *
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 188 * This function allocates memory buffer for specified device. It uses
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 189 * device specific contiguous memory area if available or the default
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 190 * global one. Requires architecture specific dev_get_cma_area() helper
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 191 * function.
c64be2bb1c6eb4 drivers/base/dma-contiguous.c Marek Szyprowski 2011-12-29 192 */
67a2e213e7e937 drivers/base/dma-contiguous.c Rohit Vaswani 2015-10-22 193 struct page *dma_alloc_from_contiguous(struct device *dev, size_t count,
d834c5ab83febf kernel/dma/contiguous.c Marek Szyprowski 2018-08-17 194 unsigned int align, bool no_warn)
c64be2bb1c6eb4 drivers/base/dma-contiguous.c Marek Szyprowski 2011-12-29 195 {
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 196 if (align > CONFIG_CMA_ALIGNMENT)
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 197 align = CONFIG_CMA_ALIGNMENT;
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 198
d834c5ab83febf kernel/dma/contiguous.c Marek Szyprowski 2018-08-17 @199 return cma_alloc(dev_get_cma_area(dev), count, align, no_warn);
c64be2bb1c6eb4 drivers/base/dma-contiguous.c Marek Szyprowski 2011-12-29 200 }
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 201
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 202 /**
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 203 * dma_release_from_contiguous() - release allocated pages
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 204 * @dev: Pointer to device for which the pages were allocated.
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 205 * @pages: Allocated pages.
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 206 * @count: Number of allocated pages.
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 207 *
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 208 * This function releases memory allocated by dma_alloc_from_contiguous().
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 209 * It returns false when provided pages do not belong to contiguous area and
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 210 * true otherwise.
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 211 */
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 212 bool dma_release_from_contiguous(struct device *dev, struct page *pages,
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 213 int count)
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 214 {
a254129e8686bf drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 @215 return cma_release(dev_get_cma_area(dev), pages, count);
3162bbd7e65b9c drivers/base/dma-contiguous.c Joonsoo Kim 2014-08-06 216 }
de9e14eebf33a6 drivers/base/dma-contiguous.c Marek Szyprowski 2014-10-13 217
:::::: The code at line 199 was first introduced by commit
:::::: d834c5ab83febf9624ad3b16c3c348aa1e02014c kernel/dma: remove unsupported gfp_mask parameter from dma_alloc_from_contiguous()
:::::: TO: Marek Szyprowski <m.szyprowski@samsung.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35695 bytes --]
^ permalink raw reply
* RE: [PATCH v3 2/4] PCI: hv: Add the support of hibernation
From: Michael Kelley @ 2019-11-25 22:49 UTC (permalink / raw)
To: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, lorenzo.pieralisi@arm.com, bhelgaas@google.com,
linux-hyperv@vger.kernel.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, Sasha Levin
In-Reply-To: <1574660034-98780-3-git-send-email-decui@microsoft.com>
From: Dexuan Cui <decui@microsoft.com> Sent: Sunday, November 24, 2019 9:34 PM
>
> Add suspend() and resume() functions so that Hyper-V virtual PCI devices are
> handled properly when the VM hibernates and resumes from hibernation.
>
> Note that the suspend() function must make sure there are no pending work
> items before calling vmbus_close(), since it runs in a process context as a
> callback in dpm_suspend(). When it starts to run, the channel callback
> hv_pci_onchannelcallback(), which runs in a tasklet context, can be still running
> concurrently and scheduling new work items onto hbus->wq in
> hv_pci_devices_present() and hv_pci_eject_device(), and the work item
> handlers can access the vmbus channel, which can be being closed by
> hv_pci_suspend(), e.g. the work item handler pci_devices_present_work() ->
> new_pcichild_device() writes to the vmbus channel.
>
> To eliminate the race, hv_pci_suspend() disables the channel callback
> tasklet, sets hbus->state to hv_pcibus_removing, and re-enables the tasklet.
> This way, when hv_pci_suspend() proceeds, it knows that no new work item
> can be scheduled, and then it flushes hbus->wq and safely closes the vmbus
> channel.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [PATCH v3 4/4] PCI: hv: Avoid a kmemleak false positive caused by the hbus buffer
From: Michael Kelley @ 2019-11-25 22:52 UTC (permalink / raw)
To: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, lorenzo.pieralisi@arm.com, bhelgaas@google.com,
linux-hyperv@vger.kernel.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, Sasha Levin
In-Reply-To: <1574660034-98780-5-git-send-email-decui@microsoft.com>
From: Dexuan Cui <decui@microsoft.com> Sent: Sunday, November 24, 2019 9:34 PM
>
> With the recent 59bb47985c1d ("mm, sl[aou]b: guarantee natural
> alignment for kmalloc(power-of-two)"), kzalloc() is able to allocate
> a 4KB buffer that is guaranteed to be 4KB-aligned. Here the size and
> alignment of hbus is important because hbus's field
> retarget_msi_interrupt_params must not cross a 4KB page boundary.
>
> Here we prefer kzalloc to get_zeroed_page(), because a buffer
> allocated by the latter is not tracked and scanned by kmemleak, and
> hence kmemleak reports the pointer contained in the hbus buffer
> (i.e. the hpdev struct, which is created in new_pcichild_device() and
> is tracked by hbus->children) as memory leak (false positive).
>
> If the kernel doesn't have 59bb47985c1d, get_zeroed_page() *must* be
> used to allocate the hbus buffer and we can avoid the kmemleak false
> positive by using kmemleak_alloc() and kmemleak_free() to ask
> kmemleak to track and scan the hbus buffer.
>
> Reported-by: Lili Deng <v-lide@microsoft.com>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
> drivers/pci/controller/pci-hyperv.c | 24 +++++++++++++++++++++---
> 1 file changed, 21 insertions(+), 3 deletions(-)
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* [PATCH] clocksource: hyperv: Reserve PAGE_SIZE space for tsc page
From: Boqun Feng @ 2019-11-26 2:17 UTC (permalink / raw)
To: linux-hyperv
Cc: Michael Kelley, Vincenzo Frascino, Boqun Feng, K. Y. Srinivasan,
Haiyang Zhang, Stephen Hemminger, Sasha Levin, Daniel Lezcano,
Thomas Gleixner, linux-kernel
Currently, the reserved size for a tsc page is 4K, which is enough for
communicating with hypervisor. However, in the case where we want to
export the tsc page to userspace (e.g. for vDSO to read the
clocksource), the tsc page should be at least PAGE_SIZE, otherwise, when
PAGE_SIZE is larger than 4K, extra kernel data will be mapped into
userspace, which means leaking kernel information.
Therefore reserve PAGE_SIZE space for tsc_pg as a preparation for the
vDSO support of ARM64 in the future. Also, while at it, replace all
reference to tsc_pg with hv_get_tsc_page() since it should be the only
interface to access tsc page.
Signed-off-by: Boqun Feng (Microsoft) <boqun.feng@gmail.com>
Cc: linux-hyperv@vger.kernel.org
---
drivers/clocksource/hyperv_timer.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
index 2317d4e3daaf..bcac936fa62b 100644
--- a/drivers/clocksource/hyperv_timer.c
+++ b/drivers/clocksource/hyperv_timer.c
@@ -213,17 +213,20 @@ EXPORT_SYMBOL_GPL(hv_stimer_global_cleanup);
struct clocksource *hyperv_cs;
EXPORT_SYMBOL_GPL(hyperv_cs);
-static struct ms_hyperv_tsc_page tsc_pg __aligned(PAGE_SIZE);
+static union {
+ struct ms_hyperv_tsc_page page;
+ u8 reserved[PAGE_SIZE];
+} tsc_pg __aligned(PAGE_SIZE);
struct ms_hyperv_tsc_page *hv_get_tsc_page(void)
{
- return &tsc_pg;
+ return &tsc_pg.page;
}
EXPORT_SYMBOL_GPL(hv_get_tsc_page);
static u64 notrace read_hv_clock_tsc(struct clocksource *arg)
{
- u64 current_tick = hv_read_tsc_page(&tsc_pg);
+ u64 current_tick = hv_read_tsc_page(hv_get_tsc_page());
if (current_tick == U64_MAX)
hv_get_time_ref_count(current_tick);
@@ -278,7 +281,7 @@ static bool __init hv_init_tsc_clocksource(void)
return false;
hyperv_cs = &hyperv_cs_tsc;
- phys_addr = virt_to_phys(&tsc_pg);
+ phys_addr = virt_to_phys(hv_get_tsc_page());
/*
* The Hyper-V TLFS specifies to preserve the value of reserved
--
2.24.0
^ permalink raw reply related
* Re: [PATCH v3 0/4] Enhance pci-hyperv to support hibernation, and 2 misc fixes
From: Lorenzo Pieralisi @ 2019-11-26 10:46 UTC (permalink / raw)
To: Dexuan Cui
Cc: kys, haiyangz, sthemmin, sashal, bhelgaas, linux-hyperv,
linux-pci, linux-kernel, mikelley, Alexander.Levin
In-Reply-To: <1574660034-98780-1-git-send-email-decui@microsoft.com>
On Sun, Nov 24, 2019 at 09:33:50PM -0800, Dexuan Cui wrote:
> I suggest the patchset goes through the pci.git tree.
>
> Patch #1: no functional change.
> Patch #2 enhances the pci-hyperv driver to support hibernation.
> Patch #3 is unrelated to hibernation.
> Patch #4 is unrelated to hibernation.
>
> Changes in v3:
> Patch #1: Added Michael Kelley's Signed-off-by.
> Patch #2: Used a better commit log message from Michael Kelley.
> Patch #3: Added Michael Kelley's Signed-off-by.
> Patch #4: Used kzalloc() rather than get_zeroed_page()/kmemleak_alloc()/
> kmemleak_free(), and added the necessary comments.
>
> Michael, can you please review #2 and #4 again?
>
> Dexuan Cui (4):
> PCI: hv: Reorganize the code in preparation of hibernation
> PCI: hv: Add the support of hibernation
> PCI: hv: Change pci_protocol_version to per-hbus
> PCI: hv: Avoid a kmemleak false positive caused by the hbus buffer
>
> drivers/pci/controller/pci-hyperv.c | 208 ++++++++++++++++++++++++----
> 1 file changed, 179 insertions(+), 29 deletions(-)
Applied to pci/hv, should be able to hit v5.5, thanks.
Lorenzo
^ permalink raw reply
* RE: [PATCH v2] video: hyperv: hyperv_fb: Use physical memory for fb on HyperV Gen 1 VMs.
From: Michael Kelley @ 2019-11-27 18:14 UTC (permalink / raw)
To: Wei Hu, b.zolnierkie@samsung.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, sashal@kernel.org, hch@lst.de,
m.szyprowski@samsung.com, mchehab+samsung@kernel.org,
sam@ravnborg.org, gregkh@linuxfoundation.org,
alexandre.belloni@bootlin.com, info@metux.net, arnd@arndb.de,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
Dexuan Cui
In-Reply-To: <20191122082408.3210-1-weh@microsoft.com>
From: Wei Hu <weh@microsoft.com> Sent: Friday, November 22, 2019 12:24 AM
>
> On Hyper-V, Generation 1 VMs can directly use VM's physical memory for
> their framebuffers. This can improve the efficiency of framebuffer and
> overall performence for VM. The physical memory assigned to framebuffer
> must be contiguous. We use CMA allocator to get contiguouse physicial
> memory when the framebuffer size is greater than 4MB. For size under
> 4MB, we use alloc_pages to achieve this.
>
> To enable framebuffer memory allocation from CMA, supply a kernel
> parameter to give enough space to CMA allocator at boot time. For
> example:
> cma=130m
> This gives 130MB memory to CAM allocator that can be allocated to
> framebuffer. If this fails, we fall back to the old way of using
> mmio for framebuffer.
>
> Signed-off-by: Wei Hu <weh@microsoft.com>
> ---
> v2: Incorporated review comments form hch@lst.de, Michael Kelley and
> Dexuan Cui
> - Use dma_alloc_coherent to allocate large contiguous memory
> - Use phys_addr_t for physical addresses
> - Corrected a few spelling errors and minor cleanups
> - Also tested on 32 bit Ubuntu guest
>
> drivers/video/fbdev/Kconfig | 1 +
> drivers/video/fbdev/hyperv_fb.c | 196 +++++++++++++++++++++++++-------
> 2 files changed, 158 insertions(+), 39 deletions(-)
>
[snip]
> +/*
> + * Allocate enough contiguous physical memory.
> + * Return physical address if succeeded or -1 if failed.
> + */
> +static phys_addr_t hvfb_get_phymem(struct hv_device *hdev,
> + unsigned int request_size)
> +{
> + struct page *page = NULL;
> + dma_addr_t dma_handle;
> + void *vmem;
> + unsigned int request_pages;
> + phys_addr_t paddr = 0;
> + unsigned int order = get_order(request_size);
> +
> + if (request_size == 0)
> + return -1;
> +
> + /* Try to call alloc_pages if the size is less than 2^MAX_ORDER */
> + if (order < MAX_ORDER) {
> + page = alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
> + if (!page)
> + return -1;
> +
> + paddr = (page_to_pfn(page) << PAGE_SHIFT);
> + request_pages = (1 << order);
> + goto get_phymem1;
> + }
Could you use an 'else' clause here and eliminate the above
'goto' statement? I know that makes the below code be indented
one level deeper, but that doesn't seem particularly problematic
here. The reason we have 'else' clauses is to avoid 'goto's and
labels. :-)
> +
> + /* Allocate from CMA */
> + if (hdev == NULL)
> + return -1;
The above test seems unnecessary. A lot of things would have
broken before getting to this function if hdev was NULL.
> +
> + hdev->device.coherent_dma_mask = DMA_BIT_MASK(64);
> +
> + request_pages = (round_up(request_size, PAGE_SIZE) >> PAGE_SHIFT);
> +
> + vmem = dma_alloc_coherent(&hdev->device,
> + request_pages * PAGE_SIZE,
> + &dma_handle,
> + GFP_KERNEL | __GFP_NOWARN);
> +
> + if (!vmem)
> + return -1;
> +
> + paddr = virt_to_phys(vmem);
> +
> +get_phymem1:
> + pr_info("Allocated %d pages starts at physical addr 0x%llx\n",
> + request_pages, paddr);
I wonder if we want to show the physical address here. The Linux kernel
definitely does not show kernel virtual addresses due to security
concerns, and I'm wondering if the same applies to physical addresses.
What's the benefit to showing the physical address?
And in the message "starts" should be "starting".
> +
> + return paddr;
> +}
> +
> +/* Release contiguous physical memory */
> +static void hvfb_release_phymem(struct hv_device *hdev,
> + phys_addr_t paddr, unsigned int size)
> +{
> + unsigned int order = get_order(size);
> +
> + if (order < MAX_ORDER)
> + __free_pages(pfn_to_page(paddr >> PAGE_SHIFT), order);
> + else
> + dma_free_coherent(&hdev->device,
> + round_up(size, PAGE_SIZE),
> + phys_to_virt(paddr),
> + paddr);
> +}
> +
>
> /* Get framebuffer memory from Hyper-V video pci space */
> static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info)
> @@ -947,8 +1028,57 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info
> *info)
> void __iomem *fb_virt;
> int gen2vm = efi_enabled(EFI_BOOT);
> resource_size_t pot_start, pot_end;
> + phys_addr_t paddr;
> int ret;
>
> + if (!gen2vm) {
> + pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT,
> + PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
> + if (!pdev) {
> + pr_err("Unable to find PCI Hyper-V video\n");
> + return -ENODEV;
> + }
> + }
> +
> + info->apertures = alloc_apertures(1);
> + if (!info->apertures)
> + goto err1;
There's a small memory leak here. The apertures are never freed in any
of the error cases in this function, or in hvfb_putmem(). This is not a bug you
introduced -- the original code had the same leak.
> +
> + if (gen2vm) {
> + info->apertures->ranges[0].base = screen_info.lfb_base;
> + info->apertures->ranges[0].size = screen_info.lfb_size;
> + } else {
> + info->apertures->ranges[0].base = pci_resource_start(pdev, 0);
> + info->apertures->ranges[0].size = pci_resource_len(pdev, 0);
> + }
> +
> + /*
> + * For Gen 1 VM, we can directly use the contiguous memory
> + * from VM. If we succeed, deferred IO happens directly
> + * on this allocated framebuffer memory, avoiding extra
> + * memory copy.
> + */
> + if (!gen2vm) {
> + paddr = hvfb_get_phymem(hdev, screen_fb_size);
> + if (paddr != (phys_addr_t) -1) {
> + par->mmio_pp = paddr;
> + par->mmio_vp = par->dio_vp = __va(paddr);
> +
> + info->fix.smem_start = paddr;
> + info->fix.smem_len = screen_fb_size;
> + info->screen_base = par->mmio_vp;
> + info->screen_size = screen_fb_size;
> +
> + par->need_docopy = false;
> + goto getmem1;
Maybe change the 'getmem1' label to 'done' or something similarly
indicative having successfully completed everything that needs to be
done?
> + }
> + pr_info("Unable to allocate enough contiguous physical memory on Gen 1
> VM. Use MMIO instead.\n");
I'd suggest changing the message to say "Using MMIO instead". This is just an
informative message indicating what the driver is doing. "Use MMIO instead"
sounds like a directive to the user to do something different, like change his
kernel configuration, and that's not the intent.
> + }
In the above code, there are three, almost consecutive, tests of the "gen2vm"
variable. It seems like the apertures could be allocated first, and then the three
tests combined into one test. Then you have one range of code for Gen 1 and
another range for Gen 2 and fewer lines 'if' statements, 'else' statements, and
curly braces.
> +
> + /*
> + * Cannot use the contiguous physical memory.
> + * Allocate mmio space for framebuffer.
> + */
> dio_fb_size =
> screen_width * screen_height * screen_depth / 8;
>
> @@ -956,13 +1086,6 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info
> *info)
> pot_start = 0;
> pot_end = -1;
> } else {
> - pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT,
> - PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
> - if (!pdev) {
> - pr_err("Unable to find PCI Hyper-V video\n");
> - return -ENODEV;
> - }
> -
> if (!(pci_resource_flags(pdev, 0) & IORESOURCE_MEM) ||
> pci_resource_len(pdev, 0) < screen_fb_size) {
> pr_err("Resource not available or (0x%lx < 0x%lx)\n",
> @@ -991,20 +1114,6 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info
> *info)
> if (par->dio_vp == NULL)
> goto err3;
>
> - info->apertures = alloc_apertures(1);
> - if (!info->apertures)
> - goto err4;
> -
> - if (gen2vm) {
> - info->apertures->ranges[0].base = screen_info.lfb_base;
> - info->apertures->ranges[0].size = screen_info.lfb_size;
> - remove_conflicting_framebuffers(info->apertures,
> - KBUILD_MODNAME, false);
> - } else {
> - info->apertures->ranges[0].base = pci_resource_start(pdev, 0);
> - info->apertures->ranges[0].size = pci_resource_len(pdev, 0);
> - }
> -
> /* Physical address of FB device */
> par->mmio_pp = par->mem->start;
> /* Virtual address of FB device */
> @@ -1015,13 +1124,15 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info
> *info)
> info->screen_base = par->dio_vp;
> info->screen_size = dio_fb_size;
>
> +getmem1:
> + remove_conflicting_framebuffers(info->apertures,
> + KBUILD_MODNAME, false);
With your change, remove_conflicting_framebuffers() is called for both
Gen 1 and Gen 2 VMs. In the old code, it was called only for Gen 2 VMs.
Is this change intentional? If so, why? I haven't delved into the details
of what remove_conflicting_framebuffers() does, so my question is
more of a double-check rather than my definitely thinking something
is wrong.
> +
> if (!gen2vm)
> pci_dev_put(pdev);
>
> return 0;
>
> -err4:
> - vfree(par->dio_vp);
> err3:
> iounmap(fb_virt);
> err2:
> @@ -1035,13 +1146,19 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info
> *info)
> }
>
> /* Release the framebuffer */
> -static void hvfb_putmem(struct fb_info *info)
> +static void hvfb_putmem(struct hv_device *hdev, struct fb_info *info)
> {
> struct hvfb_par *par = info->par;
>
> - vfree(par->dio_vp);
> - iounmap(info->screen_base);
> - vmbus_free_mmio(par->mem->start, screen_fb_size);
> + if (par->need_docopy) {
> + vfree(par->dio_vp);
> + iounmap(info->screen_base);
> + vmbus_free_mmio(par->mem->start, screen_fb_size);
> + } else {
> + hvfb_release_phymem(hdev, info->fix.smem_start,
> + screen_fb_size);
> + }
> +
> par->mem = NULL;
There's a small memory leak in the above statement. The data
structure pointed to by "mem" is not freed. The same problem
occurs in hvfb_getmem() in the "err2" path. This bug existed in
the old code as well, so it was not introduced by your changes.
Michael
^ permalink raw reply
* [GIT PULL] Hyper-V commits for v5.5
From: Sasha Levin @ 2019-11-28 15:25 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-hyperv, kys, sthemmin, linux-kernel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
The following changes since commit a99d8080aaf358d5d23581244e5da23b35e340b9:
Linux 5.4-rc6 (2019-11-03 14:07:26 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git tags/hyperv-next-signed
for you to fetch changes up to 7a1323b5dfe44a9013a2cc56ef2973034a00bf88:
Drivers: hv: vmbus: Fix crash handler reset of Hyper-V synic (2019-11-21 20:10:46 -0500)
- ----------------------------------------------------------------
- - Support for new VMBus protocols (Andrea Parri).
- - Hibernation support (Dexuan Cui).
- - Latency testing framework (Branden Bonaby).
- - Decoupling Hyper-V page size from guest page size (Himadri Pandya).
- ----------------------------------------------------------------
Andrea Parri (3):
Drivers: hv: vmbus: Introduce table of VMBus protocol versions
Drivers: hv: vmbus: Enable VMBus protocol versions 4.1, 5.1 and 5.2
Drivers: hv: vmbus: Add module parameter to cap the VMBus version
Boqun Feng (1):
drivers: iommu: hyperv: Make HYPERV_IOMMU only available on x86
Branden Bonaby (2):
drivers: hv: vmbus: Introduce latency testing
tools: hv: add vmbus testing tool
Davidlohr Bueso (1):
drivers/hv: Replace binary semaphore with mutex
Dexuan Cui (7):
scsi: storvsc: Add the support of hibernation
video: hyperv_fb: Add the support of hibernation
hv_sock: Add the support of hibernation
hv_netvsc: Add the support of hibernation
x86/hyperv: Implement hv_is_hibernation_supported()
hv_balloon: Add the support of hibernation
HID: hyperv: Add the support of hibernation
Himadri Pandya (5):
Drivers: hv: Specify receive buffer size using Hyper-V page size
Drivers: hv: util: Specify ring buffer size using Hyper-V page size
x86: hv: Add function to allocate zeroed page for Hyper-V
Drivers: hv: vmbus: Remove dependencies on guest page size
Drivers: hv: balloon: Remove dependencies on guest page size
Michael Kelley (1):
Drivers: hv: vmbus: Fix crash handler reset of Hyper-V synic
Wei Hu (2):
video: hyperv: hyperv_fb: Obtain screen resolution from Hyper-V host
video: hyperv: hyperv_fb: Support deferred IO for Hyper-V frame buffer driver
Documentation/ABI/testing/debugfs-hyperv | 23 ++
MAINTAINERS | 1 +
arch/x86/hyperv/hv_init.c | 15 ++
arch/x86/include/asm/mshyperv.h | 1 +
drivers/hid/hid-hyperv.c | 34 +++
drivers/hv/Makefile | 1 +
drivers/hv/connection.c | 87 ++++---
drivers/hv/hv_balloon.c | 112 ++++++--
drivers/hv/hv_debugfs.c | 178 +++++++++++++
drivers/hv/hv_fcopy.c | 3 +-
drivers/hv/hv_kvp.c | 3 +-
drivers/hv/hv_snapshot.c | 3 +-
drivers/hv/hv_util.c | 13 +-
drivers/hv/hyperv_vmbus.h | 31 +++
drivers/hv/ring_buffer.c | 2 +
drivers/hv/vmbus_drv.c | 27 +-
drivers/iommu/Kconfig | 2 +-
drivers/net/hyperv/hyperv_net.h | 3 +
drivers/net/hyperv/netvsc_drv.c | 57 ++++
drivers/scsi/storvsc_drv.c | 41 +++
drivers/video/fbdev/Kconfig | 1 +
drivers/video/fbdev/hyperv_fb.c | 428 ++++++++++++++++++++++++++++---
include/asm-generic/mshyperv.h | 2 +
include/linux/hyperv.h | 31 ++-
lib/Kconfig.debug | 7 +
net/vmw_vsock/hyperv_transport.c | 20 ++
tools/hv/vmbus_testing | 376 +++++++++++++++++++++++++++
27 files changed, 1386 insertions(+), 116 deletions(-)
create mode 100644 Documentation/ABI/testing/debugfs-hyperv
create mode 100644 drivers/hv/hv_debugfs.c
create mode 100755 tools/hv/vmbus_testing
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCgAdFiEE4n5dijQDou9mhzu83qZv95d3LNwFAl3f5okACgkQ3qZv95d3
LNwLEQ//biCeM3j2rks/f2pEaY1CM2dGDHpi8wRwgnN1RNvWpc/M6JFGM/+g6a3j
U0FDmCU2mTdomM09zInq7RYW1ahHMGLO8shzvuP7LZxUdElzaaGXBVmOjFbL4NRJ
9vpDjhdpBmfR1Zy6ekLszp8NH9ZHR0FGmXt99Ljtjg8nyxBbSgBohVosbOkN55NQ
x7kLa9dPB0tm7mMCLCm00PK7+U5e520D0sML1YfMpf7FJROz+locdILI6IBRdN56
9xSSJV/hVmcGxr3tG3Aa7wQc30Hp64T2BjF9wv7yJMU98bJJE0lNP18FM/AbHi9P
YGSL9jFM26a8BFxgsBou+kLZCotk4A+MAV5jLU83g9/8tfLSBk2zVlPb1Bk6V8in
6KBP4GaBCTOa2OoL6QD/8mny+weByCaB4silgzjThBQ/BMjcQu9TpLp19GXSAZk3
dJhZkK/qYgqr4u7GOgnsVtrk2cU66YIT5+FBWt4wWhVCdvOa5BeSvYEks+eb+qFl
RUM0iI7y1BhLORztoMttLS+wrBh/Thm+UtEg2/ipI0R2JuKRsYLcbw+QATW2l02W
8mDeM94Z9KcHIMUy1MO5X+VeAfSgTzT0zBoeC5IeC5VAZNu02aoVKfLNM30vD7rY
IkdiUoLBNmKNrfbw7o+py820bFLzgYDKeDckKe2cMDXGrnykpQA=
=6xUH
-----END PGP SIGNATURE-----
^ permalink raw reply
* [RFC PATCH 0/3] vsock: support network namespace
From: Stefano Garzarella @ 2019-11-28 17:15 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, linux-hyperv, virtualization, kvm,
Michael S. Tsirkin, Stefano Garzarella, David S. Miller,
Dexuan Cui, Jason Wang, Stefan Hajnoczi, Jorgen Hansen
Hi,
now that we have multi-transport upstream, I started to take a look to
support network namespace (netns) in vsock.
As we partially discussed in the multi-transport proposal [1], it could
be nice to support network namespace in vsock to reach the following
goals:
- isolate host applications from guest applications using the same ports
with CID_ANY
- assign the same CID of VMs running in different network namespaces
- partition VMs between VMMs or at finer granularity
This preliminary implementation provides the following behavior:
- packets received from the host (received by G2H transports) are
assigned to the default netns (init_net)
- packets received from the guest (received by H2G - vhost-vsock) are
assigned to the netns of the process that opens /dev/vhost-vsock
(usually the VMM, qemu in my tests, opens the /dev/vhost-vsock)
- for vmci I need some suggestions, because I don't know how to do
and test the same in the vmci driver, for now vmci uses the
init_net
- loopback packets are exchanged only in the same netns
Questions:
1. Should we make configurable the netns (now it is init_net) where
packets from the host should be delivered?
2. Should we provide an ioctl in vhost-vsock to configure the netns
to use? (instead of using the netns of the process that opens
/dev/vhost-vsock)
3. Should we provide a way to disable the netns support in vsock?
4. Jorgen: Do you think can be useful support it in vmci host
driver?
I tested the series in this way:
l0_host$ qemu-system-x86_64 -m 4G -M accel=kvm -smp 4 \
-drive file=/tmp/vsockvm0.img,if=virtio --nographic \
-device vhost-vsock-pci,guest-cid=3
l1_vm$ ip netns add ns1
l1_vm$ ip netns add ns2
# same CID on different netns
l1_vm$ ip netns exec ns1 qemu-system-x86_64 -m 1G -M accel=kvm -smp 2 \
-drive file=/tmp/vsockvm1.img,if=virtio --nographic \
-device vhost-vsock-pci,guest-cid=4
l1_vm$ ip netns exec ns2 qemu-system-x86_64 -m 1G -M accel=kvm -smp 2 \
-drive file=/tmp/vsockvm2.img,if=virtio --nographic \
-device vhost-vsock-pci,guest-cid=4
# all iperf3 listen on CID_ANY and port 5201, but in different netns
l1_vm$ ./iperf3 --vsock -s # connection from l0 or guests started
# on default netns (init_net)
l1_vm$ ip netns exec ns1 ./iperf3 --vsock -s
l1_vm$ ip netns exec ns1 ./iperf3 --vsock -s
l0_host$ ./iperf3 --vsock -c 3
l2_vm1$ ./iperf3 --vsock -c 2
l2_vm2$ ./iperf3 --vsock -c 2
This series is on top of the vsock-loopback series (not yet merged),
and it is available in the Git repository at:
git://github.com/stefano-garzarella/linux.git vsock-netns
Any comments are really appreciated!
Thanks,
Stefano
[1] https://www.spinics.net/lists/netdev/msg575792.html
Stefano Garzarella (3):
vsock: add network namespace support
vsock/virtio_transport_common: handle netns of received packets
vhost/vsock: use netns of process that opens the vhost-vsock device
drivers/vhost/vsock.c | 29 ++++++++++++++++-------
include/linux/virtio_vsock.h | 2 ++
include/net/af_vsock.h | 6 +++--
net/vmw_vsock/af_vsock.c | 31 ++++++++++++++++++-------
net/vmw_vsock/hyperv_transport.c | 5 ++--
net/vmw_vsock/virtio_transport.c | 2 ++
net/vmw_vsock/virtio_transport_common.c | 12 ++++++++--
net/vmw_vsock/vmci_transport.c | 5 ++--
8 files changed, 67 insertions(+), 25 deletions(-)
--
2.23.0
^ 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