* Re: [PATCH v5 2/8] arm64: hyperv: Add hypercall and register access functions
From: Boqun Feng @ 2019-11-04 4:36 UTC (permalink / raw)
To: Michael Kelley
Cc: will@kernel.org, catalin.marinas@arm.com, mark.rutland@arm.com,
maz@kernel.org, linux-arm-kernel@lists.infradead.org,
gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
linux-hyperv@vger.kernel.org, devel@linuxdriverproject.org,
olaf@aepfle.de, apw@canonical.com, vkuznets, jasowang@redhat.com,
marcelo.cerri@canonical.com, KY Srinivasan, Sunil Muthuswamy
In-Reply-To: <1570129355-16005-3-git-send-email-mikelley@microsoft.com>
Hi Michael,
On Thu, Oct 03, 2019 at 07:03:19PM +0000, Michael Kelley wrote:
> Add ARM64-specific code to make Hyper-V hypercalls and to
> access virtual processor synthetic registers via hypercalls.
> Hypercalls use a Hyper-V specific calling sequence with a non-zero
> immediate value per Section 2.9 of the SMC Calling Convention
> spec.
>
> This code is architecture dependent and is mostly driven by
> architecture independent code in the VMbus driver and the
> Hyper-V timer clocksource driver.
>
> This code is built only when CONFIG_HYPERV is enabled.
>
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> ---
> MAINTAINERS | 1 +
> arch/arm64/Kbuild | 1 +
> arch/arm64/hyperv/Makefile | 2 +
> arch/arm64/hyperv/hv_hvc.S | 44 +++++++++++++++
> arch/arm64/hyperv/hv_init.c | 133 ++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 181 insertions(+)
> create mode 100644 arch/arm64/hyperv/Makefile
> create mode 100644 arch/arm64/hyperv/hv_hvc.S
> create mode 100644 arch/arm64/hyperv/hv_init.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d464067..84f76f9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7566,6 +7566,7 @@ F: arch/x86/kernel/cpu/mshyperv.c
> F: arch/x86/hyperv
> F: arch/arm64/include/asm/hyperv-tlfs.h
> F: arch/arm64/include/asm/mshyperv.h
> +F: arch/arm64/hyperv
> F: drivers/clocksource/hyperv_timer.c
> F: drivers/hid/hid-hyperv.c
> F: drivers/hv/
> diff --git a/arch/arm64/Kbuild b/arch/arm64/Kbuild
> index d646582..2469421 100644
> --- a/arch/arm64/Kbuild
> +++ b/arch/arm64/Kbuild
> @@ -3,4 +3,5 @@ obj-y += kernel/ mm/
> obj-$(CONFIG_NET) += net/
> obj-$(CONFIG_KVM) += kvm/
> obj-$(CONFIG_XEN) += xen/
> +obj-$(CONFIG_HYPERV) += hyperv/
I did a kernel built with CONFIG_HYPERV=m today, and found out this line
should be (similar to x86):
+obj-$(subst m,y,$(CONFIG_HYPERV)) += hyperv/
, otherwise, when CONFIG_HYPERV=m, files in arch/arm64/hyperv/ will be
compiled as obj-m, and symbols defined in those files cannot be
used by kernel builtin, e.g. hyperv_timer (since CONFIG_HYPERV_TIMER=y
in this case).
A compile/link error I hit today is:
| /home/boqun/linux-arm64/drivers/clocksource/hyperv_timer.c:98: undefined reference to `hv_set_vpreg'
| aarch64-linux-gnu-ld: /home/boqun/linux-arm64/drivers/clocksource/hyperv_timer.c:98: undefined reference to `hv_set_vpreg'
[...]
Besides, another problem I hit when compiled with CONFIG_HYPERV=m is:
| ERROR: "screen_info" [drivers/hv/hv_vmbus.ko] undefined!
, which can be fixed by the following change.
Regards,
Boqun
---------------->8
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index d0cf596db82c..8ff557ae5cc6 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -55,6 +55,7 @@ static __init pteval_t create_mapping_protection(efi_memory_desc_t *md)
/* we will fill this structure from the stub, so don't put it in .bss */
struct screen_info screen_info __section(.data);
+EXPORT_SYMBOL(screen_info);
int __init efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md)
{
^ permalink raw reply
* Re: [PATCH net-next 12/14] vsock/vmci: register vmci_transport only when VMCI guest/host are active
From: Stefano Garzarella @ 2019-11-04 10:10 UTC (permalink / raw)
To: Jorgen Hansen
Cc: Michael S. Tsirkin, kvm, Greg Kroah-Hartman, Jason Wang,
David S. Miller, Dexuan Cui, Haiyang Zhang, Sasha Levin,
linux-kernel, Arnd Bergmann, Stefan Hajnoczi, linux-hyperv,
K. Y. Srinivasan, Stephen Hemminger, virtualization, netdev
In-Reply-To: <20191023095554.11340-13-sgarzare@redhat.com>
Hi Jorgen,
I'm preparing the v2, but first, if you have time, I'd like to have
a comment from you on this patch that modifies a bit vmci.
Thank you very much,
Stefano
On Wed, Oct 23, 2019 at 11:55:52AM +0200, Stefano Garzarella wrote:
> To allow other transports to be loaded with vmci_transport,
> we register the vmci_transport as G2H or H2G only when a VMCI guest
> or host is active.
>
> To do that, this patch adds a callback registered in the vmci driver
> that will be called when a new host or guest become active.
> This callback will register the vmci_transport in the VSOCK core.
> If the transport is already registered, we ignore the error coming
> from vsock_core_register().
>
> Cc: Jorgen Hansen <jhansen@vmware.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> drivers/misc/vmw_vmci/vmci_driver.c | 50 +++++++++++++++++++++++++++++
> drivers/misc/vmw_vmci/vmci_driver.h | 2 ++
> drivers/misc/vmw_vmci/vmci_guest.c | 2 ++
> drivers/misc/vmw_vmci/vmci_host.c | 7 ++++
> include/linux/vmw_vmci_api.h | 2 ++
> net/vmw_vsock/vmci_transport.c | 29 +++++++++++------
> 6 files changed, 82 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/misc/vmw_vmci/vmci_driver.c b/drivers/misc/vmw_vmci/vmci_driver.c
> index 819e35995d32..195afbd7edc1 100644
> --- a/drivers/misc/vmw_vmci/vmci_driver.c
> +++ b/drivers/misc/vmw_vmci/vmci_driver.c
> @@ -28,6 +28,9 @@ MODULE_PARM_DESC(disable_guest,
> static bool vmci_guest_personality_initialized;
> static bool vmci_host_personality_initialized;
>
> +static DEFINE_MUTEX(vmci_vsock_mutex); /* protects vmci_vsock_transport_cb */
> +static vmci_vsock_cb vmci_vsock_transport_cb;
> +
> /*
> * vmci_get_context_id() - Gets the current context ID.
> *
> @@ -45,6 +48,53 @@ u32 vmci_get_context_id(void)
> }
> EXPORT_SYMBOL_GPL(vmci_get_context_id);
>
> +/*
> + * vmci_register_vsock_callback() - Register the VSOCK vmci_transport callback.
> + *
> + * The callback will be called every time a new host or guest become active,
> + * or if they are already active when this function is called.
> + * To unregister the callback, call this function with NULL parameter.
> + *
> + * Returns 0 on success. -EBUSY if a callback is already registered.
> + */
> +int vmci_register_vsock_callback(vmci_vsock_cb callback)
> +{
> + int err = 0;
> +
> + mutex_lock(&vmci_vsock_mutex);
> +
> + if (vmci_vsock_transport_cb && callback) {
> + err = -EBUSY;
> + goto out;
> + }
> +
> + vmci_vsock_transport_cb = callback;
> +
> + if (!vmci_vsock_transport_cb)
> + goto out;
> +
> + if (vmci_guest_code_active())
> + vmci_vsock_transport_cb(false);
> +
> + if (vmci_host_users() > 0)
> + vmci_vsock_transport_cb(true);
> +
> +out:
> + mutex_unlock(&vmci_vsock_mutex);
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(vmci_register_vsock_callback);
> +
> +void vmci_call_vsock_callback(bool is_host)
> +{
> + mutex_lock(&vmci_vsock_mutex);
> +
> + if (vmci_vsock_transport_cb)
> + vmci_vsock_transport_cb(is_host);
> +
> + mutex_unlock(&vmci_vsock_mutex);
> +}
> +
> static int __init vmci_drv_init(void)
> {
> int vmci_err;
> diff --git a/drivers/misc/vmw_vmci/vmci_driver.h b/drivers/misc/vmw_vmci/vmci_driver.h
> index aab81b67670c..990682480bf6 100644
> --- a/drivers/misc/vmw_vmci/vmci_driver.h
> +++ b/drivers/misc/vmw_vmci/vmci_driver.h
> @@ -36,10 +36,12 @@ extern struct pci_dev *vmci_pdev;
>
> u32 vmci_get_context_id(void);
> int vmci_send_datagram(struct vmci_datagram *dg);
> +void vmci_call_vsock_callback(bool is_host);
>
> int vmci_host_init(void);
> void vmci_host_exit(void);
> bool vmci_host_code_active(void);
> +int vmci_host_users(void);
>
> int vmci_guest_init(void);
> void vmci_guest_exit(void);
> diff --git a/drivers/misc/vmw_vmci/vmci_guest.c b/drivers/misc/vmw_vmci/vmci_guest.c
> index 7a84a48c75da..cc8eeb361fcd 100644
> --- a/drivers/misc/vmw_vmci/vmci_guest.c
> +++ b/drivers/misc/vmw_vmci/vmci_guest.c
> @@ -637,6 +637,8 @@ static int vmci_guest_probe_device(struct pci_dev *pdev,
> vmci_dev->iobase + VMCI_CONTROL_ADDR);
>
> pci_set_drvdata(pdev, vmci_dev);
> +
> + vmci_call_vsock_callback(false);
> return 0;
>
> err_free_irq:
> diff --git a/drivers/misc/vmw_vmci/vmci_host.c b/drivers/misc/vmw_vmci/vmci_host.c
> index 833e2bd248a5..ff3c396146ff 100644
> --- a/drivers/misc/vmw_vmci/vmci_host.c
> +++ b/drivers/misc/vmw_vmci/vmci_host.c
> @@ -108,6 +108,11 @@ bool vmci_host_code_active(void)
> atomic_read(&vmci_host_active_users) > 0);
> }
>
> +int vmci_host_users(void)
> +{
> + return atomic_read(&vmci_host_active_users);
> +}
> +
> /*
> * Called on open of /dev/vmci.
> */
> @@ -338,6 +343,8 @@ static int vmci_host_do_init_context(struct vmci_host_dev *vmci_host_dev,
> vmci_host_dev->ct_type = VMCIOBJ_CONTEXT;
> atomic_inc(&vmci_host_active_users);
>
> + vmci_call_vsock_callback(true);
> +
> retval = 0;
>
> out:
> diff --git a/include/linux/vmw_vmci_api.h b/include/linux/vmw_vmci_api.h
> index acd9fafe4fc6..f28907345c80 100644
> --- a/include/linux/vmw_vmci_api.h
> +++ b/include/linux/vmw_vmci_api.h
> @@ -19,6 +19,7 @@
> struct msghdr;
> typedef void (vmci_device_shutdown_fn) (void *device_registration,
> void *user_data);
> +typedef void (*vmci_vsock_cb) (bool is_host);
>
> int vmci_datagram_create_handle(u32 resource_id, u32 flags,
> vmci_datagram_recv_cb recv_cb,
> @@ -37,6 +38,7 @@ int vmci_doorbell_destroy(struct vmci_handle handle);
> int vmci_doorbell_notify(struct vmci_handle handle, u32 priv_flags);
> u32 vmci_get_context_id(void);
> bool vmci_is_context_owner(u32 context_id, kuid_t uid);
> +int vmci_register_vsock_callback(vmci_vsock_cb callback);
>
> int vmci_event_subscribe(u32 event,
> vmci_event_cb callback, void *callback_data,
> diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
> index 2eb3f16d53e7..04437f822d82 100644
> --- a/net/vmw_vsock/vmci_transport.c
> +++ b/net/vmw_vsock/vmci_transport.c
> @@ -2053,19 +2053,22 @@ static bool vmci_check_transport(struct vsock_sock *vsk)
> return vsk->transport == &vmci_transport;
> }
>
> -static int __init vmci_transport_init(void)
> +void vmci_vsock_transport_cb(bool is_host)
> {
> - int features = VSOCK_TRANSPORT_F_DGRAM | VSOCK_TRANSPORT_F_H2G;
> - int cid;
> - int err;
> + int features;
>
> - cid = vmci_get_context_id();
> + if (is_host)
> + features = VSOCK_TRANSPORT_F_H2G;
> + else
> + features = VSOCK_TRANSPORT_F_G2H;
>
> - if (cid == VMCI_INVALID_ID)
> - return -EINVAL;
> + vsock_core_register(&vmci_transport, features);
> +}
>
> - if (cid != VMCI_HOST_CONTEXT_ID)
> - features |= VSOCK_TRANSPORT_F_G2H;
> +static int __init vmci_transport_init(void)
> +{
> + int features = VSOCK_TRANSPORT_F_DGRAM;
> + int err;
>
> /* Create the datagram handle that we will use to send and receive all
> * VSocket control messages for this context.
> @@ -2079,7 +2082,6 @@ static int __init vmci_transport_init(void)
> pr_err("Unable to create datagram handle. (%d)\n", err);
> return vmci_transport_error_to_vsock_error(err);
> }
> -
> err = vmci_event_subscribe(VMCI_EVENT_QP_RESUMED,
> vmci_transport_qp_resumed_cb,
> NULL, &vmci_transport_qp_resumed_sub_id);
> @@ -2094,8 +2096,14 @@ static int __init vmci_transport_init(void)
> if (err < 0)
> goto err_unsubscribe;
>
> + err = vmci_register_vsock_callback(vmci_vsock_transport_cb);
> + if (err < 0)
> + goto err_unregister;
> +
> return 0;
>
> +err_unregister:
> + vsock_core_unregister(&vmci_transport);
> err_unsubscribe:
> vmci_event_unsubscribe(vmci_transport_qp_resumed_sub_id);
> err_destroy_stream_handle:
> @@ -2121,6 +2129,7 @@ static void __exit vmci_transport_exit(void)
> vmci_transport_qp_resumed_sub_id = VMCI_INVALID_ID;
> }
>
> + vmci_register_vsock_callback(NULL);
> vsock_core_unregister(&vmci_transport);
> }
> module_exit(vmci_transport_exit);
> --
> 2.21.0
>
--
^ permalink raw reply
* RE: [PATCH net-next, 2/4] hv_netvsc: Fix error handling in netvsc_attach()
From: Haiyang Zhang @ 2019-11-04 15:08 UTC (permalink / raw)
To: Markus Elfring, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org
Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org,
David S. Miller, KY Srinivasan, Olaf Hering, Sasha Levin,
Stephen Hemminger, vkuznets
In-Reply-To: <cdf7b308-940a-ff9c-07ae-f42b94687e24@web.de>
> -----Original Message-----
> From: Markus Elfring <Markus.Elfring@web.de>
> Sent: Friday, November 1, 2019 4:43 PM
> To: Haiyang Zhang <haiyangz@microsoft.com>; linux-
> hyperv@vger.kernel.org; netdev@vger.kernel.org
> Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org; David S.
> Miller <davem@davemloft.net>; KY Srinivasan <kys@microsoft.com>; Olaf
> Hering <olaf@aepfle.de>; Sasha Levin <sashal@kernel.org>; Stephen
> Hemminger <sthemmin@microsoft.com>; vkuznets <vkuznets@redhat.com>
> Subject: Re: [PATCH net-next, 2/4] hv_netvsc: Fix error handling in
> netvsc_attach()
>
> > If rndis_filter_open() fails, we need to remove the rndis device
> > created in earlier steps, before returning an error code. Otherwise,
> > the retry of
> > netvsc_attach() from its callers will fail and hang.
>
> How do you think about to choose a more “imperative mood” for your
> change description?
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.k
> ernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%
> 2Ftree%2FDocumentation%2Fprocess%2Fsubmitting-
> patches.rst%3Fid%3D0dbe6cb8f7e05bc9611602ef45980a6c57b245a3%23n151
> &data=02%7C01%7Chaiyangz%40microsoft.com%7C162aa016f45e4293c
> abb08d75f0c0fee%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637
> 082377796295159&sdata=ytjxGYTPI2D4BoNbslKPvBbsfGUEM7hXj1YAiG
> hn8Ik%3D&reserved=0
Agreed. Thanks.
>
>
> …
> > +++ b/drivers/net/hyperv/netvsc_drv.c
> > @@ -982,7 +982,7 @@ static int netvsc_attach(struct net_device *ndev,
> > if (netif_running(ndev)) {
> > ret = rndis_filter_open(nvdev);
> > if (ret)
> > - return ret;
> > + goto err;
> >
> > rdev = nvdev->extension;
> > if (!rdev->link_state)
> …
>
> I would prefer to specify the completed exception handling (addition of two
> function calls) by a compound statement in the shown if branch directly.
>
> If you would insist to use a goto statement, I find an other label more
> appropriate according to the Linux coding style.
I will have more patches that make multiple entry points of error clean up
steps -- so I used goto instead of putting the functions in each if-branch.
I will name the labels more meaningfully.
Thanks,
- Haiyang
^ permalink raw reply
* RE: [PATCH] Input: hyperv-keyboard: Add the support of hibernation
From: Dexuan Cui @ 2019-11-05 5:18 UTC (permalink / raw)
To: dmitry.torokhov@gmail.com
Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, linux-hyperv@vger.kernel.org,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
Michael Kelley
In-Reply-To: <20191003174530.GB22365@dtor-ws>
> From: dmitry.torokhov@gmail.com <dmitry.torokhov@gmail.com>
> Sent: Thursday, October 3, 2019 10:46 AM
> To: Dexuan Cui <decui@microsoft.com>
> On Thu, Oct 03, 2019 at 06:44:04AM +0000, Dexuan Cui wrote:
> > ...
> > I think I understood now: it looks the vmbus driver should implement
> > a prepare() or freeze(), which calls the hyperv_keyboard driver's
> > prepare() or freeze(), which can set the flag or disable the keyboard
> > event handling. This way we don't need the notifier.
>
> Right. I think in practice the current suspend implementation can work
> as freeze() for the HV keyboard, because in suspend you shut off vmbus
> channel, so there should not be wakeup signals anymore. What you do not
> want is to have the current resume to be used in place of thaw(), as
> there you re-enable the vmbus channel and resume sending wakeup requests
> as you are writing out the hibernation image to storage.
>
> I think if vmbus allowed HV keyboard driver to supply empty thaw() and
> poweroff() implementations, while using suspend() as freeze() and
> resume() as restore(), it would solve the issue for you.
>
> Dmitry
Hi Dmitry,
Sorry for the late reply! I finally came back on this patch. :-)
After I dug more into the issues, this is my understanding now:
As I checked the code in drivers/ , it doesn't look commom for a driver to
distinguish between thaw() and restore(). Typically a driver uses the macro
SET_SYSTEM_SLEEP_PM_OPS() to define the dev_pm_ops, and the macro uses the
same function resume_fn as thaw() and restore().
BTW, the macro already uses the same function suspend_fn as suspend() and
freeze(), and uses the same function resume_fn as resume() and restore(). And,
it looks unusual for a driver to provide an empty thaw(), if any. If I follow your
suggestions, I'll have to fix the vmbus driver first (i.e. drivers/hv/vmbus_drv.c:
vmbus_pm()) by manually assigning a new function vmbus_thaw() to the
thaw() dev_pm_op, and vmbus_thaw() should call the Hyper-V keyboard
driver's empty hv_kbd_thaw(), meaning I have to add a .thaw function
pointer to the struct hv_driver. IMHO all these changes look too big just for
the rare corner cases of the unexpected wake-up issues.
More important, even if we make the suggested changes, we actually only fix
the unexpected wakeup caused by PMSG_THAW , and there are still some corner
cases of failures (please see below).
Before any of the dev_pm_op is called, the global counter 'pm_abort_suspend'
can be already non-zero, meaning pm_wakeup_pending() is true, so
try_to_freeze_tasks() returns -EBUSY, i.e. hibernate() -> freeze_processes()
or hibernate() -> hibernation_snapshot() -> freeze_kernel_threads() fails.
When the VM boots up and tries to resume from the saved file from
disk, before the fresh new kernel's Hyper-V keyboard device is PMSG_QUIESCE'ed,
the global counter 'pm_abort_suspend' can be already non-zero (I can cause this
scenario by holding the Enter key when the kernel starts), so
pm_wakeup_pending() is true, and the below freeze_processes() or
device_suspend() can return -EBUSY and the resume process fails.
software_resume() ->
freeze_processes()
pm_wakeup_clear(true) -> Note: this resets the counter to 0.
try_to_freeze_tasks ->
pm_wakeup_pending
load_image_and_restore() ->
hibernation_restore() ->
dpm_suspend_start() ->
dpm_suspend() ->
device_suspend() ->
__device_suspend() ->
pm_wakeup_pending()
IMO on a Linux physical machine these issues should happen as well. I think
we can fix them separately. For this patch, I suggest we keep it simple like
the below:
[PATCH v2] Input: hyperv-keyboard: Add the support of hibernation
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).
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
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,
},
--
I plan to post this as v2.
Looking forward to your comments!
Thanks,
-- Dexuan
^ permalink raw reply related
* RE: [PATCH] Input: hyperv-keyboard: Add the support of hibernation
From: Dexuan Cui @ 2019-11-05 5:43 UTC (permalink / raw)
To: dmitry.torokhov@gmail.com
Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, linux-hyperv@vger.kernel.org,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
Michael Kelley
In-Reply-To: <20191003174530.GB22365@dtor-ws>
> From: dmitry.torokhov@gmail.com <dmitry.torokhov@gmail.com>
> Sent: Thursday, October 3, 2019 10:46 AM
> ...
> Right. I think in practice the current suspend implementation can work
> as freeze() for the HV keyboard, because in suspend you shut off vmbus
> channel, so there should not be wakeup signals anymore. What you do not
> want is to have the current resume to be used in place of thaw(), as
> there you re-enable the vmbus channel and resume sending wakeup requests
> as you are writing out the hibernation image to storage.
>
> I think if vmbus allowed HV keyboard driver to supply empty thaw() and
> poweroff() implementations, while using suspend() as freeze() and
> resume() as restore(), it would solve the issue for you.
>
> Dmitry
BTW, IMO thaw() should not be empty, because drivers/base/power/main.c:
pm_op() uses ops->thaw for both PM_EVENT_THAW and PM_EVENT_RECOVER.
PMSG_RECOVER is the same as PM_EVENT_RECOVER.
If some step in hibernation_snapshot() -> create_image() fails, we call
dpm_resume_start(PMSG_RECOVER) at the end of create_image(), and call
dpm_resume() in hibernation_snapshot(). An empty thaw() can not bring the
device back to normal.
Thanks,
-- Dexuan
^ permalink raw reply
* Re: [PATCH net-next 0/2] netvsc: RSS related patches
From: David Miller @ 2019-11-05 22:02 UTC (permalink / raw)
To: stephen; +Cc: haiyangz, kys, sashal, netdev, linux-hyperv
In-Reply-To: <20191101234238.23921-1-stephen@networkplumber.org>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Fri, 1 Nov 2019 16:42:36 -0700
> Address a couple of issues related to recording RSS hash
> value in skb. These were found by reviewing RSS support.
Series applied.
^ permalink raw reply
* Re: [PATCH net-next 2/2] hv_netvsc: record hardware hash in skb
From: Eric Dumazet @ 2019-11-05 23:13 UTC (permalink / raw)
To: Stephen Hemminger, davem, haiyangz, kys, sashal
Cc: netdev, linux-hyperv, Stephen Hemminger
In-Reply-To: <20191101234238.23921-3-stephen@networkplumber.org>
On 11/1/19 4:42 PM, Stephen Hemminger wrote:
> From: Stephen Hemminger <sthemmin@microsoft.com>
>
> Since RSS hash is available from the host, record it in
> the skb.
>
Note that drivers are supposed to do this only if (dev->features & NETIF_F_RXHASH) is true...
^ permalink raw reply
* Re: [PATCH v5 2/8] arm64: hyperv: Add hypercall and register access functions
From: Marc Zyngier @ 2019-11-06 10:19 UTC (permalink / raw)
To: Michael Kelley
Cc: will, catalin.marinas, mark.rutland, linux-arm-kernel, gregkh,
linux-kernel, linux-hyperv, devel, olaf, apw, vkuznets, jasowang,
marcelo.cerri, KY Srinivasan, Sunil Muthuswamy, boqun.feng
In-Reply-To: <1570129355-16005-3-git-send-email-mikelley@microsoft.com>
On 2019-10-03 20:12, Michael Kelley wrote:
> Add ARM64-specific code to make Hyper-V hypercalls and to
> access virtual processor synthetic registers via hypercalls.
> Hypercalls use a Hyper-V specific calling sequence with a non-zero
> immediate value per Section 2.9 of the SMC Calling Convention
> spec.
I find this "following the spec by actively sidestepping it" counter
productive. You (or rather the Hyper-V people) are reinventing the
wheel (of the slightly square variety) instead of using the standard
that the whole of the ARM ecosystem seems happy to take advantage
of.
I wonder what is the rational for this. If something doesn't quite
work for Hyper-V, I think we'd all like to know.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* RE:elektronicka faktura 71081403108
From: Ladislav Adamec @ 2019-11-07 7:34 UTC (permalink / raw)
To: linux-hyperv
[-- Attachment #1: Type: text/plain, Size: 75 bytes --]
__
Dobre odpoledne,
Vase fakturacniv priloze,
S pozdravem.
[-- Attachment #2: faktura_0003589.docx --]
[-- Type: application/octet-stream, Size: 46287 bytes --]
^ permalink raw reply
* RE: [PATCH v5 2/8] arm64: hyperv: Add hypercall and register access functions
From: Marc Zyngier @ 2019-11-07 9:10 UTC (permalink / raw)
To: Michael Kelley
Cc: will, catalin.marinas, mark.rutland, linux-arm-kernel, gregkh,
linux-kernel, linux-hyperv, devel, olaf, apw, vkuznets, jasowang,
marcelo.cerri, KY Srinivasan, Sunil Muthuswamy, boqun.feng
In-Reply-To: <DM5PR21MB013730D09CB8BA7658DE57F7D7790@DM5PR21MB0137.namprd21.prod.outlook.com>
On 2019-11-06 19:08, Michael Kelley wrote:
> From: Marc Zyngier <maz@kernel.org> Sent: Wednesday, November 6,
> 2019 2:20 AM
>>
>> On 2019-10-03 20:12, Michael Kelley wrote:
>> > Add ARM64-specific code to make Hyper-V hypercalls and to
>> > access virtual processor synthetic registers via hypercalls.
>> > Hypercalls use a Hyper-V specific calling sequence with a non-zero
>> > immediate value per Section 2.9 of the SMC Calling Convention
>> > spec.
>>
>> I find this "following the spec by actively sidestepping it" counter
>> productive. You (or rather the Hyper-V people) are reinventing the
>> wheel (of the slightly square variety) instead of using the standard
>> that the whole of the ARM ecosystem seems happy to take advantage
>> of.
>>
>> I wonder what is the rational for this. If something doesn't quite
>> work for Hyper-V, I think we'd all like to know.
>>
>
> I'll go another round internally with the Hyper-V people on this
> topic and impress upon them the desire of the Linux community to
> have Hyper-V adopt the true spirit of the spec. But I know they are
> fairly set in their approach at this point, regardless of the
> technical
> merits or lack thereof. Hyper-V is shipping and in use as a
> commercial
> product on ARM64 hardware, which makes it harder to change. I
> hope we can find a way to avoid a complete impasse ....
Hyper-V shipping with their own calling convention is fine by me. Linux
having to implement multiple calling conventions because the Hyper-V
folks refuse (for undisclosed reason) to adopt the standard isn't fine
at
all.
HV can perfectly retain its interface for Windows or other things, but
please *at least* implement the standard interface on which all
existing
operating systems rely.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* Re: [PATCH v3] x86/hyper-v: micro-optimize send_ipi_one case
From: Vitaly Kuznetsov @ 2019-11-07 13:26 UTC (permalink / raw)
To: Sasha Levin
Cc: linux-hyperv, linux-kernel, x86, K. Y. Srinivasan, Haiyang Zhang,
Stephen Hemminger, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
H. Peter Anvin, Roman Kagan, Michael Kelley, Joe Perches
In-Reply-To: <20191027151938.7296-1-vkuznets@redhat.com>
Vitaly Kuznetsov <vkuznets@redhat.com> writes:
> When sending an IPI to a single CPU there is no need to deal with cpumasks.
> With 2 CPU guest on WS2019 I'm seeing a minor (like 3%, 8043 -> 7761 CPU
> cycles) improvement with smp_call_function_single() loop benchmark. The
> optimization, however, is tiny and straitforward. Also, send_ipi_one() is
> important for PV spinlock kick.
>
> I was also wondering if it would make sense to switch to using regular
> APIC IPI send for CPU > 64 case but no, it is twice as expesive (12650 CPU
> cycles for __send_ipi_mask_ex() call, 26000 for orig_apic.send_IPI(cpu,
> vector)).
>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> Changes since v2:
> - Check VP number instead of CPU number against >= 64 [Michael]
> - Check for VP_INVAL
Hi Sasha,
do you have plans to pick this up for hyperv-next or should we ask x86
folks to?
Thanks!
--
Vitaly
^ permalink raw reply
* Re: [PATCH v1] tools/hv: async name resolution in kvp_daemon
From: Vitaly Kuznetsov @ 2019-11-07 13:39 UTC (permalink / raw)
To: Olaf Hering, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Sasha Levin, open list:Hyper-V CORE AND DRIVERS, open list
Cc: Olaf Hering
In-Reply-To: <20191024144943.26199-1-olaf@aepfle.de>
Olaf Hering <olaf@aepfle.de> writes:
> The hostname is resolved just once since commit 58125210ab3b ("Tools:
> hv: cache FQDN in kvp_daemon to avoid timeouts") to make sure the VM
> responds within the timeout limits to requests from the host.
>
> If for some reason getaddrinfo fails, the string returned by the
> "FullyQualifiedDomainName" request contains some error string, which is
> then used by tools on the host side.
>
> Adjust the code to resolve the current hostname in a separate thread.
> This thread loops until getaddrinfo returns success. During this time
> all "FullyQualifiedDomainName" requests will be answered with an empty
> string.
>
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
> tools/hv/Makefile | 2 ++
> tools/hv/hv_kvp_daemon.c | 69 ++++++++++++++++++++++++++++++++----------------
> 2 files changed, 48 insertions(+), 23 deletions(-)
>
> diff --git a/tools/hv/Makefile b/tools/hv/Makefile
> index b57143d9459c..3b5481015a84 100644
> --- a/tools/hv/Makefile
> +++ b/tools/hv/Makefile
> @@ -22,6 +22,8 @@ ALL_PROGRAMS := $(patsubst %,$(OUTPUT)%,$(ALL_TARGETS))
>
> ALL_SCRIPTS := hv_get_dhcp_info.sh hv_get_dns_info.sh hv_set_ifconfig.sh
>
> +$(OUTPUT)hv_kvp_daemon: LDFLAGS += -lpthread
> +
> all: $(ALL_PROGRAMS)
>
> export srctree OUTPUT CC LD CFLAGS
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index e9ef4ca6a655..22cf1c4dbf5c 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -41,6 +41,7 @@
> #include <net/if.h>
> #include <limits.h>
> #include <getopt.h>
> +#include <pthread.h>
>
> /*
> * KVP protocol: The user mode component first registers with the
> @@ -85,7 +86,7 @@ static char *processor_arch;
> static char *os_build;
> static char *os_version;
> static char *lic_version = "Unknown version";
> -static char full_domain_name[HV_KVP_EXCHANGE_MAX_VALUE_SIZE];
> +static char *full_domain_name;
> static struct utsname uts_buf;
>
> /*
> @@ -1327,27 +1328,53 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
> return error;
> }
>
> -
> -static void
> -kvp_get_domain_name(char *buffer, int length)
> +/*
> + * Async retrival of Fully Qualified Domain Name because getaddrinfo takes an
> + * unpredictable amount of time to finish.
> + */
> +static void *kvp_getaddrinfo(void *p)
> {
> - struct addrinfo hints, *info ;
> - int error = 0;
> + char *tmp, **str_ptr = (char **)p;
> + char hostname[HOST_NAME_MAX + 1];
> + struct addrinfo *info, hints = {
> + .ai_family = AF_INET, /* Get only ipv4 addrinfo. */
> + .ai_socktype = SOCK_STREAM,
> + .ai_flags = AI_CANONNAME,
> + };
> + int ret;
> +
> + if (gethostname(hostname, sizeof(hostname) - 1) < 0)
> + goto out;
> +
> + do {
> + ret = getaddrinfo(hostname, NULL, &hints, &info);
> + if (ret)
> + sleep(1);
Is it only EAI_AGAIN or do you see any other return values which justify
the retry? I'm afraid that in case of a e.g. non-existing hostname we'll
be infinitely looping with EAI_FAIL.
> + } while (ret);
> +
> + ret = asprintf(&tmp, "%s", info->ai_canonname);
> + freeaddrinfo(info);
> + if (ret <= 0)
> + goto out;
> +
> + if (ret > HV_KVP_EXCHANGE_MAX_VALUE_SIZE)
> + tmp[HV_KVP_EXCHANGE_MAX_VALUE_SIZE - 1] = '\0';
> + *str_ptr = tmp;
>
> - gethostname(buffer, length);
> - memset(&hints, 0, sizeof(hints));
> - hints.ai_family = AF_INET; /*Get only ipv4 addrinfo. */
> - hints.ai_socktype = SOCK_STREAM;
> - hints.ai_flags = AI_CANONNAME;
> +out:
> + pthread_exit(NULL);
> +}
> +
> +static void kvp_obtain_domain_name(char **str_ptr)
> +{
> + pthread_t t;
>
> - error = getaddrinfo(buffer, NULL, &hints, &info);
> - if (error != 0) {
> - snprintf(buffer, length, "getaddrinfo failed: 0x%x %s",
> - error, gai_strerror(error));
> + if (pthread_create(&t, NULL, kvp_getaddrinfo, str_ptr)) {
> + syslog(LOG_ERR, "pthread_create failed; error: %d %s",
> + errno, strerror(errno));
> return;
> }
> - snprintf(buffer, length, "%s", info->ai_canonname);
> - freeaddrinfo(info);
> + pthread_detach(t);
I think this should be complemented with pthread_cancel/pthread_join
before exiting main().
> }
>
> void print_usage(char *argv[])
> @@ -1412,11 +1439,7 @@ int main(int argc, char *argv[])
> * Retrieve OS release information.
> */
> kvp_get_os_info();
> - /*
> - * Cache Fully Qualified Domain Name because getaddrinfo takes an
> - * unpredictable amount of time to finish.
> - */
> - kvp_get_domain_name(full_domain_name, sizeof(full_domain_name));
> + kvp_obtain_domain_name(&full_domain_name);
>
> if (kvp_file_init()) {
> syslog(LOG_ERR, "Failed to initialize the pools");
> @@ -1571,7 +1594,7 @@ int main(int argc, char *argv[])
>
> switch (hv_msg->body.kvp_enum_data.index) {
> case FullyQualifiedDomainName:
> - strcpy(key_value, full_domain_name);
> + strcpy(key_value, full_domain_name ? : "");
> strcpy(key_name, "FullyQualifiedDomainName");
> break;
> case IntegrationServicesVersion:
--
Vitaly
^ permalink raw reply
* Re: [PATCH v1] tools/hv: async name resolution in kvp_daemon
From: Olaf Hering @ 2019-11-07 13:48 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
open list:Hyper-V CORE AND DRIVERS, open list
In-Reply-To: <874kzfbybk.fsf@vitty.brq.redhat.com>
[-- Attachment #1: Type: text/plain, Size: 946 bytes --]
Am Thu, 07 Nov 2019 14:39:11 +0100
schrieb Vitaly Kuznetsov <vkuznets@redhat.com>:
> Olaf Hering <olaf@aepfle.de> writes:
> Is it only EAI_AGAIN or do you see any other return values which justify
> the retry? I'm afraid that in case of a e.g. non-existing hostname we'll
> be infinitely looping with EAI_FAIL.
I currently do not have a setup that reproduces the failure.
I think if this thread loops forever, so be it.
The report I have shows "getaddrinfo failed: 0xfffffffe Name or service not known" on the host side.
And that went away within the VM once "networking was fixed", whatever this means.
But hv_kvp_daemon would report the error string forever.
> > + pthread_detach(t);
> I think this should be complemented with pthread_cancel/pthread_join
> before exiting main().
If the thread is detached, it is exactly that: detached. Why do you think the main thread should wait for the detached thread?
Olaf
[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v1] tools/hv: async name resolution in kvp_daemon
From: Vitaly Kuznetsov @ 2019-11-07 14:15 UTC (permalink / raw)
To: Olaf Hering
Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
open list:Hyper-V CORE AND DRIVERS, open list
In-Reply-To: <20191107144850.37587edb.olaf@aepfle.de>
Olaf Hering <olaf@aepfle.de> writes:
> Am Thu, 07 Nov 2019 14:39:11 +0100
> schrieb Vitaly Kuznetsov <vkuznets@redhat.com>:
>
>> Olaf Hering <olaf@aepfle.de> writes:
>
>> Is it only EAI_AGAIN or do you see any other return values which justify
>> the retry? I'm afraid that in case of a e.g. non-existing hostname we'll
>> be infinitely looping with EAI_FAIL.
>
> I currently do not have a setup that reproduces the failure.
> I think if this thread loops forever, so be it.
>
> The report I have shows "getaddrinfo failed: 0xfffffffe Name or service not known" on the host side.
> And that went away within the VM once "networking was fixed", whatever this means.
> But hv_kvp_daemon would report the error string forever.
Looping forever with a permanent error is pretty unusual...
>
>> > + pthread_detach(t);
>> I think this should be complemented with pthread_cancel/pthread_join
>> before exiting main().
>
> If the thread is detached, it is exactly that: detached. Why do you think the main thread should wait for the detached thread?
Ah, my bad: you actually can't join a detached thread, scratch my
comment.
--
Vitaly
^ permalink raw reply
* Re: [PATCH v5 3/8] arm64: hyperv: Add memory alloc/free functions for Hyper-V size pages
From: Marc Zyngier @ 2019-11-07 14:19 UTC (permalink / raw)
To: Michael Kelley
Cc: will, catalin.marinas, mark.rutland, linux-arm-kernel, gregkh,
linux-kernel, linux-hyperv, devel, olaf, apw, vkuznets, jasowang,
marcelo.cerri, KY Srinivasan, Sunil Muthuswamy, boqun.feng
In-Reply-To: <1570129355-16005-4-git-send-email-mikelley@microsoft.com>
On 2019-10-03 20:12, Michael Kelley wrote:
> Add ARM64-specific code to allocate memory with HV_HYP_PAGE_SIZE
> size and alignment. These are for use when pages need to be shared
> with Hyper-V. Separate functions are needed as the page size used
> by Hyper-V may not be the same as the guest page size. Free
> operations are rarely done, so no attempt is made to combine
> freed pages into larger chunks.
>
> This code is built only when CONFIG_HYPERV is enabled.
>
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> ---
> arch/arm64/hyperv/hv_init.c | 68
> ++++++++++++++++++++++++++++++++++++++++++
> include/asm-generic/mshyperv.h | 5 ++++
> 2 files changed, 73 insertions(+)
>
> diff --git a/arch/arm64/hyperv/hv_init.c
> b/arch/arm64/hyperv/hv_init.c
> index 6808bc8..9c294f6 100644
> --- a/arch/arm64/hyperv/hv_init.c
> +++ b/arch/arm64/hyperv/hv_init.c
> @@ -15,10 +15,78 @@
> #include <linux/export.h>
> #include <linux/mm.h>
> #include <linux/hyperv.h>
> +#include <linux/spinlock.h>
> +#include <linux/list.h>
> +#include <linux/string.h>
> #include <asm-generic/bug.h>
> #include <asm/hyperv-tlfs.h>
> #include <asm/mshyperv.h>
>
> +
> +/*
> + * Functions for allocating and freeing memory with size and
> + * alignment HV_HYP_PAGE_SIZE. These functions are needed because
> + * the guest page size may not be the same as the Hyper-V page
> + * size. And while kalloc() could allocate the memory, it does not
> + * guarantee the required alignment. So a separate small memory
> + * allocator is needed. The free function is rarely used, so it
> + * does not try to combine freed pages into larger chunks.
Is this still needed now that kmalloc has alignment guarantees
(see 59bb47985c1d)?
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* Re: [PATCH v1] tools/hv: async name resolution in kvp_daemon
From: Olaf Hering @ 2019-11-07 14:20 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
open list:Hyper-V CORE AND DRIVERS, open list
In-Reply-To: <87zhh7ai26.fsf@vitty.brq.redhat.com>
[-- Attachment #1: Type: text/plain, Size: 314 bytes --]
Am Thu, 07 Nov 2019 15:15:45 +0100
schrieb Vitaly Kuznetsov <vkuznets@redhat.com>:
> Looping forever with a permanent error is pretty unusual...
That might be true, but how would you detect a permanent error?
Just because the DNS server is gone for one hour does not mean it will be gone forever.
Olaf
[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v1] tools/hv: async name resolution in kvp_daemon
From: Vitaly Kuznetsov @ 2019-11-07 14:44 UTC (permalink / raw)
To: Olaf Hering
Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
open list:Hyper-V CORE AND DRIVERS, open list
In-Reply-To: <20191107152059.6cae8f30.olaf@aepfle.de>
Olaf Hering <olaf@aepfle.de> writes:
> Am Thu, 07 Nov 2019 15:15:45 +0100
> schrieb Vitaly Kuznetsov <vkuznets@redhat.com>:
>
>> Looping forever with a permanent error is pretty unusual...
>
> That might be true, but how would you detect a permanent error?
> Just because the DNS server is gone for one hour does not mean it will be gone forever.
'man 3 getaddrinfo' lists the following:
EAI_ADDRFAMILY
EAI_AGAIN
EAI_BADFLAGS
EAI_FAIL
EAI_FAMILY
EAI_MEMORY
EAI_NODATA
EAI_NONAME
EAI_SERVICE
EAI_SOCKTYPE
EAI_SYSTEM
I *think* what you're aiming at is EAI_AGAIN and EAI_FAIL, the rest
should probably terminate the resolver thread (e.g. AF_INET is
unsupported or something like that).
--
Vitaly
^ permalink raw reply
* Re: [PATCH v3] x86/hyper-v: micro-optimize send_ipi_one case
From: Thomas Gleixner @ 2019-11-07 21:21 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: Sasha Levin, linux-hyperv, linux-kernel, x86, K. Y. Srinivasan,
Haiyang Zhang, Stephen Hemminger, Ingo Molnar, Borislav Petkov,
H. Peter Anvin, Roman Kagan, Michael Kelley, Joe Perches
In-Reply-To: <877e4bbyw2.fsf@vitty.brq.redhat.com>
On Thu, 7 Nov 2019, Vitaly Kuznetsov wrote:
> Vitaly Kuznetsov <vkuznets@redhat.com> writes:
>
> > When sending an IPI to a single CPU there is no need to deal with cpumasks.
> > With 2 CPU guest on WS2019 I'm seeing a minor (like 3%, 8043 -> 7761 CPU
> > cycles) improvement with smp_call_function_single() loop benchmark. The
> > optimization, however, is tiny and straitforward. Also, send_ipi_one() is
> > important for PV spinlock kick.
> >
> > I was also wondering if it would make sense to switch to using regular
> > APIC IPI send for CPU > 64 case but no, it is twice as expesive (12650 CPU
> > cycles for __send_ipi_mask_ex() call, 26000 for orig_apic.send_IPI(cpu,
> > vector)).
> >
> > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> > ---
> > Changes since v2:
> > - Check VP number instead of CPU number against >= 64 [Michael]
> > - Check for VP_INVAL
>
> Hi Sasha,
>
> do you have plans to pick this up for hyperv-next or should we ask x86
> folks to?
I'm picking up the constant TSC one anyway, so I can just throw that in as
well.
Thanks,
tglx
^ permalink raw reply
* [PATCH AUTOSEL 4.14 077/103] Drivers: hv: vmbus: Fix synic per-cpu context initialization
From: Sasha Levin @ 2019-11-08 11:42 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Michael Kelley, Dan Carpenter, K . Y . Srinivasan,
Greg Kroah-Hartman, Sasha Levin, linux-hyperv
In-Reply-To: <20191108114310.14363-1-sashal@kernel.org>
From: Michael Kelley <mikelley@microsoft.com>
[ Upstream commit f25a7ece08bdb1f2b3c4bbeae942682fc3a99dde ]
If hv_synic_alloc() errors out, the state of the per-cpu context
for some CPUs is unknown since the zero'ing is done as each
CPU is iterated over. In such case, hv_synic_cleanup() may try to
free memory based on uninitialized values. Fix this by zero'ing
the per-cpu context for all CPUs before doing any memory
allocations that might fail.
Signed-off-by: Michael Kelley <mikelley@microsoft.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.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.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index fe041f22521da..23f312b4c6aa2 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -148,6 +148,17 @@ static void hv_init_clockevent_device(struct clock_event_device *dev, int cpu)
int hv_synic_alloc(void)
{
int cpu;
+ struct hv_per_cpu_context *hv_cpu;
+
+ /*
+ * First, zero all per-cpu memory areas so hv_synic_free() can
+ * detect what memory has been allocated and cleanup properly
+ * after any failures.
+ */
+ for_each_present_cpu(cpu) {
+ hv_cpu = per_cpu_ptr(hv_context.cpu_context, cpu);
+ memset(hv_cpu, 0, sizeof(*hv_cpu));
+ }
hv_context.hv_numa_map = kzalloc(sizeof(struct cpumask) * nr_node_ids,
GFP_ATOMIC);
@@ -157,10 +168,8 @@ int hv_synic_alloc(void)
}
for_each_present_cpu(cpu) {
- struct hv_per_cpu_context *hv_cpu
- = per_cpu_ptr(hv_context.cpu_context, cpu);
+ hv_cpu = per_cpu_ptr(hv_context.cpu_context, cpu);
- memset(hv_cpu, 0, sizeof(*hv_cpu));
tasklet_init(&hv_cpu->msg_dpc,
vmbus_on_msg_dpc, (unsigned long) hv_cpu);
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 156/205] Drivers: hv: vmbus: Fix synic per-cpu context initialization
From: Sasha Levin @ 2019-11-08 11:37 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Michael Kelley, Dan Carpenter, K . Y . Srinivasan,
Greg Kroah-Hartman, Sasha Levin, linux-hyperv
In-Reply-To: <20191108113752.12502-1-sashal@kernel.org>
From: Michael Kelley <mikelley@microsoft.com>
[ Upstream commit f25a7ece08bdb1f2b3c4bbeae942682fc3a99dde ]
If hv_synic_alloc() errors out, the state of the per-cpu context
for some CPUs is unknown since the zero'ing is done as each
CPU is iterated over. In such case, hv_synic_cleanup() may try to
free memory based on uninitialized values. Fix this by zero'ing
the per-cpu context for all CPUs before doing any memory
allocations that might fail.
Signed-off-by: Michael Kelley <mikelley@microsoft.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.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.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 8e923e70e5945..12bc9fa211117 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -189,6 +189,17 @@ static void hv_init_clockevent_device(struct clock_event_device *dev, int cpu)
int hv_synic_alloc(void)
{
int cpu;
+ struct hv_per_cpu_context *hv_cpu;
+
+ /*
+ * First, zero all per-cpu memory areas so hv_synic_free() can
+ * detect what memory has been allocated and cleanup properly
+ * after any failures.
+ */
+ for_each_present_cpu(cpu) {
+ hv_cpu = per_cpu_ptr(hv_context.cpu_context, cpu);
+ memset(hv_cpu, 0, sizeof(*hv_cpu));
+ }
hv_context.hv_numa_map = kcalloc(nr_node_ids, sizeof(struct cpumask),
GFP_KERNEL);
@@ -198,10 +209,8 @@ int hv_synic_alloc(void)
}
for_each_present_cpu(cpu) {
- struct hv_per_cpu_context *hv_cpu
- = per_cpu_ptr(hv_context.cpu_context, cpu);
+ hv_cpu = per_cpu_ptr(hv_context.cpu_context, cpu);
- memset(hv_cpu, 0, sizeof(*hv_cpu));
tasklet_init(&hv_cpu->msg_dpc,
vmbus_on_msg_dpc, (unsigned long) hv_cpu);
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 156/191] x86/hyperv: Suppress "PCI: Fatal: No config space access function found"
From: Sasha Levin @ 2019-11-10 2:39 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Dexuan Cui, Thomas Gleixner, Michael Kelley, H. Peter Anvin,
KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
devel@linuxdriverproject.org, Olaf Aepfle, Andy Whitcroft,
Jason Wang, Vitaly Kuznetsov, Marcelo Cerri, Josh Poulson,
Sasha Levin, linux-hyperv
In-Reply-To: <20191110024013.29782-1-sashal@kernel.org>
From: Dexuan Cui <decui@microsoft.com>
[ Upstream commit 2f285f46240d67060061d153786740d4df53cd78 ]
A Generation-2 Linux VM on Hyper-V doesn't have the legacy PCI bus, and
users always see the scary warning, which is actually harmless.
Suppress it.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: KY Srinivasan <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: "devel@linuxdriverproject.org" <devel@linuxdriverproject.org>
Cc: Olaf Aepfle <olaf@aepfle.de>
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Marcelo Cerri <marcelo.cerri@canonical.com>
Cc: Josh Poulson <jopoulso@microsoft.com>
Link: https://lkml.kernel.org/r/ <KU1P153MB0166D977DC930996C4BF538ABF1D0@KU1P153MB0166.APCP153.PROD.OUTLOOK.COM
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/x86/hyperv/hv_init.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 3fb8551552862..8a9cff1f129dc 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -17,6 +17,7 @@
*
*/
+#include <linux/efi.h>
#include <linux/types.h>
#include <asm/apic.h>
#include <asm/desc.h>
@@ -257,6 +258,22 @@ static int hv_cpu_die(unsigned int cpu)
return 0;
}
+static int __init hv_pci_init(void)
+{
+ int gen2vm = efi_enabled(EFI_BOOT);
+
+ /*
+ * For Generation-2 VM, we exit from pci_arch_init() by returning 0.
+ * The purpose is to suppress the harmless warning:
+ * "PCI: Fatal: No config space access function found"
+ */
+ if (gen2vm)
+ return 0;
+
+ /* For Generation-1 VM, we'll proceed in pci_arch_init(). */
+ return 1;
+}
+
/*
* This function is to be invoked early in the boot sequence after the
* hypervisor has been detected.
@@ -333,6 +350,8 @@ void __init hyperv_init(void)
hv_apic_init();
+ x86_init.pci.arch_init = hv_pci_init;
+
/*
* Register Hyper-V specific clocksource.
*/
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.14 085/109] x86/hyperv: Suppress "PCI: Fatal: No config space access function found"
From: Sasha Levin @ 2019-11-10 2:45 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Dexuan Cui, Thomas Gleixner, Michael Kelley, H. Peter Anvin,
KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
devel@linuxdriverproject.org, Olaf Aepfle, Andy Whitcroft,
Jason Wang, Vitaly Kuznetsov, Marcelo Cerri, Josh Poulson,
Sasha Levin, linux-hyperv
In-Reply-To: <20191110024541.31567-1-sashal@kernel.org>
From: Dexuan Cui <decui@microsoft.com>
[ Upstream commit 2f285f46240d67060061d153786740d4df53cd78 ]
A Generation-2 Linux VM on Hyper-V doesn't have the legacy PCI bus, and
users always see the scary warning, which is actually harmless.
Suppress it.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: KY Srinivasan <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: "devel@linuxdriverproject.org" <devel@linuxdriverproject.org>
Cc: Olaf Aepfle <olaf@aepfle.de>
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Marcelo Cerri <marcelo.cerri@canonical.com>
Cc: Josh Poulson <jopoulso@microsoft.com>
Link: https://lkml.kernel.org/r/ <KU1P153MB0166D977DC930996C4BF538ABF1D0@KU1P153MB0166.APCP153.PROD.OUTLOOK.COM
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/x86/hyperv/hv_init.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 2e9d58cc371e6..2653b7b25d176 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -17,6 +17,7 @@
*
*/
+#include <linux/efi.h>
#include <linux/types.h>
#include <asm/hypervisor.h>
#include <asm/hyperv.h>
@@ -101,6 +102,22 @@ static int hv_cpu_init(unsigned int cpu)
return 0;
}
+static int __init hv_pci_init(void)
+{
+ int gen2vm = efi_enabled(EFI_BOOT);
+
+ /*
+ * For Generation-2 VM, we exit from pci_arch_init() by returning 0.
+ * The purpose is to suppress the harmless warning:
+ * "PCI: Fatal: No config space access function found"
+ */
+ if (gen2vm)
+ return 0;
+
+ /* For Generation-1 VM, we'll proceed in pci_arch_init(). */
+ return 1;
+}
+
/*
* This function is to be invoked early in the boot sequence after the
* hypervisor has been detected.
@@ -154,6 +171,8 @@ void hyperv_init(void)
hyper_alloc_mmu();
+ x86_init.pci.arch_init = hv_pci_init;
+
/*
* Register Hyper-V specific clocksource.
*/
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 123/191] vmbus: keep pointer to ring buffer page
From: Sasha Levin @ 2019-11-10 2:39 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Stephen Hemminger, Stephen Hemminger, Greg Kroah-Hartman,
Sasha Levin, linux-hyperv
In-Reply-To: <20191110024013.29782-1-sashal@kernel.org>
From: Stephen Hemminger <stephen@networkplumber.org>
[ Upstream commit 52a42c2a90226dc61c99bbd0cb096deeb52c334b ]
Avoid going from struct page to virt address (and back) by just
keeping pointer to the allocated pages instead of virt address.
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hv/channel.c | 20 +++++++++-----------
drivers/uio/uio_hv_generic.c | 5 +++--
include/linux/hyperv.h | 2 +-
3 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index fdb0f832fadef..5e515533e9cdb 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -91,11 +91,14 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
unsigned long flags;
int ret, err = 0;
struct page *page;
+ unsigned int order;
if (send_ringbuffer_size % PAGE_SIZE ||
recv_ringbuffer_size % PAGE_SIZE)
return -EINVAL;
+ order = get_order(send_ringbuffer_size + recv_ringbuffer_size);
+
spin_lock_irqsave(&newchannel->lock, flags);
if (newchannel->state == CHANNEL_OPEN_STATE) {
newchannel->state = CHANNEL_OPENING_STATE;
@@ -110,21 +113,17 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
/* Allocate the ring buffer */
page = alloc_pages_node(cpu_to_node(newchannel->target_cpu),
- GFP_KERNEL|__GFP_ZERO,
- get_order(send_ringbuffer_size +
- recv_ringbuffer_size));
+ GFP_KERNEL|__GFP_ZERO, order);
if (!page)
- page = alloc_pages(GFP_KERNEL|__GFP_ZERO,
- get_order(send_ringbuffer_size +
- recv_ringbuffer_size));
+ page = alloc_pages(GFP_KERNEL|__GFP_ZERO, order);
if (!page) {
err = -ENOMEM;
goto error_set_chnstate;
}
- newchannel->ringbuffer_pages = page_address(page);
+ newchannel->ringbuffer_page = page;
newchannel->ringbuffer_pagecount = (send_ringbuffer_size +
recv_ringbuffer_size) >> PAGE_SHIFT;
@@ -239,8 +238,7 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
error_free_pages:
hv_ringbuffer_cleanup(&newchannel->outbound);
hv_ringbuffer_cleanup(&newchannel->inbound);
- __free_pages(page,
- get_order(send_ringbuffer_size + recv_ringbuffer_size));
+ __free_pages(page, order);
error_set_chnstate:
newchannel->state = CHANNEL_OPEN_STATE;
return err;
@@ -666,8 +664,8 @@ static int vmbus_close_internal(struct vmbus_channel *channel)
hv_ringbuffer_cleanup(&channel->outbound);
hv_ringbuffer_cleanup(&channel->inbound);
- free_pages((unsigned long)channel->ringbuffer_pages,
- get_order(channel->ringbuffer_pagecount * PAGE_SIZE));
+ __free_pages(channel->ringbuffer_page,
+ get_order(channel->ringbuffer_pagecount << PAGE_SHIFT));
out:
return ret;
diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
index e401be8321ab5..170fa1f8f00e0 100644
--- a/drivers/uio/uio_hv_generic.c
+++ b/drivers/uio/uio_hv_generic.c
@@ -131,11 +131,12 @@ static int hv_uio_ring_mmap(struct file *filp, struct kobject *kobj,
= container_of(kobj, struct vmbus_channel, kobj);
struct hv_device *dev = channel->primary_channel->device_obj;
u16 q_idx = channel->offermsg.offer.sub_channel_index;
+ void *ring_buffer = page_address(channel->ringbuffer_page);
dev_dbg(&dev->device, "mmap channel %u pages %#lx at %#lx\n",
q_idx, vma_pages(vma), vma->vm_pgoff);
- return vm_iomap_memory(vma, virt_to_phys(channel->ringbuffer_pages),
+ return vm_iomap_memory(vma, virt_to_phys(ring_buffer),
channel->ringbuffer_pagecount << PAGE_SHIFT);
}
@@ -224,7 +225,7 @@ hv_uio_probe(struct hv_device *dev,
/* mem resources */
pdata->info.mem[TXRX_RING_MAP].name = "txrx_rings";
pdata->info.mem[TXRX_RING_MAP].addr
- = (uintptr_t)dev->channel->ringbuffer_pages;
+ = (uintptr_t)page_address(dev->channel->ringbuffer_page);
pdata->info.mem[TXRX_RING_MAP].size
= dev->channel->ringbuffer_pagecount << PAGE_SHIFT;
pdata->info.mem[TXRX_RING_MAP].memtype = UIO_MEM_LOGICAL;
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index bbde887ed3931..c43e694fef7dd 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -739,7 +739,7 @@ struct vmbus_channel {
u32 ringbuffer_gpadlhandle;
/* Allocated memory for ring buffer */
- void *ringbuffer_pages;
+ struct page *ringbuffer_page;
u32 ringbuffer_pagecount;
struct hv_ring_buffer_info outbound; /* send to parent */
struct hv_ring_buffer_info inbound; /* receive from parent */
--
2.20.1
^ permalink raw reply related
* [PATCH] VFIO/VMBUS: Add VFIO VMBUS driver support
From: lantianyu1986 @ 2019-11-11 8:45 UTC (permalink / raw)
To: alex.williamson, cohuck, kys, haiyangz, sthemmin, sashal,
mchehab+samsung, davem, gregkh, robh, Jonathan.Cameron, paulmck,
michael.h.kelley
Cc: Tianyu Lan, linux-kernel, kvm, linux-hyperv, vkuznets
From: Tianyu Lan <Tianyu.Lan@microsoft.com>
This patch is to add VFIO VMBUS driver support in order to expose
VMBUS devices to user space drivers(Reference Hyper-V UIO driver).
DPDK now has netvsc PMD driver support and it may get VMBUS resources
via VFIO interface with new driver support.
So far, Hyper-V doesn't provide virtual IOMMU support and so this
driver needs to be used with VFIO noiommu mode.
Current netvsc PMD driver in DPDK doesn't have IRQ mode support.
This driver version skips IRQ control part and adds later when
there is a real request.
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
MAINTAINERS | 1 +
drivers/vfio/Kconfig | 1 +
drivers/vfio/Makefile | 1 +
drivers/vfio/vmbus/Kconfig | 9 +
drivers/vfio/vmbus/vfio_vmbus.c | 407 ++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/vfio.h | 12 ++
6 files changed, 431 insertions(+)
create mode 100644 drivers/vfio/vmbus/Kconfig
create mode 100644 drivers/vfio/vmbus/vfio_vmbus.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 55199ef7fa74..6f61fb351a5d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7574,6 +7574,7 @@ F: drivers/scsi/storvsc_drv.c
F: drivers/uio/uio_hv_generic.c
F: drivers/video/fbdev/hyperv_fb.c
F: drivers/iommu/hyperv-iommu.c
+F: drivers/vfio/vmbus/
F: net/vmw_vsock/hyperv_transport.c
F: include/clocksource/hyperv_timer.h
F: include/linux/hyperv.h
diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index fd17db9b432f..f4e075fcedbe 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -47,4 +47,5 @@ menuconfig VFIO_NOIOMMU
source "drivers/vfio/pci/Kconfig"
source "drivers/vfio/platform/Kconfig"
source "drivers/vfio/mdev/Kconfig"
+source "drivers/vfio/vmbus/Kconfig"
source "virt/lib/Kconfig"
diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
index de67c4725cce..acef916cba7f 100644
--- a/drivers/vfio/Makefile
+++ b/drivers/vfio/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_VFIO_SPAPR_EEH) += vfio_spapr_eeh.o
obj-$(CONFIG_VFIO_PCI) += pci/
obj-$(CONFIG_VFIO_PLATFORM) += platform/
obj-$(CONFIG_VFIO_MDEV) += mdev/
+obj-$(CONFIG_VFIO_VMBUS) += vmbus/
diff --git a/drivers/vfio/vmbus/Kconfig b/drivers/vfio/vmbus/Kconfig
new file mode 100644
index 000000000000..27a85daae55a
--- /dev/null
+++ b/drivers/vfio/vmbus/Kconfig
@@ -0,0 +1,9 @@
+config VFIO_VMBUS
+ tristate "VFIO support for vmbus devices"
+ depends on VFIO && HYPERV
+ help
+ Support for the VMBUS VFIO bus driver. Expose VMBUS
+ resources to user space drivers(e.g, DPDK and SPDK).
+
+ If you don't know what to do here, say N.
+
diff --git a/drivers/vfio/vmbus/vfio_vmbus.c b/drivers/vfio/vmbus/vfio_vmbus.c
new file mode 100644
index 000000000000..25d9cafa2c0a
--- /dev/null
+++ b/drivers/vfio/vmbus/vfio_vmbus.c
@@ -0,0 +1,407 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * VFIO VMBUS driver.
+ *
+ * Copyright (C) 2019, Microsoft, Inc.
+ *
+ * Author : Lan Tianyu <Tianyu.Lan@microsoft.com>
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/vfio.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/hyperv.h>
+
+#include "../../hv/hyperv_vmbus.h"
+
+
+#define DRIVER_VERSION "0.0.1"
+#define DRIVER_AUTHOR "Tianyu Lan <Tianyu.Lan@microsoft.com>"
+#define DRIVER_DESC "VFIO driver for VMBus devices"
+
+#define HV_RING_SIZE (512 * HV_HYP_PAGE_SIZE) /* 2M Ring size */
+#define SEND_BUFFER_SIZE (16 * 1024 * 1024)
+#define RECV_BUFFER_SIZE (31 * 1024 * 1024)
+
+struct vmbus_mem {
+ phys_addr_t addr;
+ resource_size_t size;
+};
+
+struct vfio_vmbus_device {
+ struct hv_device *hdev;
+ atomic_t refcnt;
+ struct vmbus_mem mem[VFIO_VMBUS_MAX_MAP_NUM];
+
+ void *recv_buf;
+ void *send_buf;
+};
+
+static void vfio_vmbus_channel_cb(void *context)
+{
+ struct vmbus_channel *chan = context;
+
+ /* Disable interrupts on channel */
+ chan->inbound.ring_buffer->interrupt_mask = 1;
+
+ /* Issue a full memory barrier after masking interrupt */
+ virt_mb();
+}
+
+static int vfio_vmbus_ring_mmap(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr,
+ struct vm_area_struct *vma)
+{
+ struct vmbus_channel *channel
+ = container_of(kobj, struct vmbus_channel, kobj);
+ void *ring_buffer = page_address(channel->ringbuffer_page);
+
+ if (channel->state != CHANNEL_OPENED_STATE)
+ return -ENODEV;
+
+ return vm_iomap_memory(vma, virt_to_phys(ring_buffer),
+ channel->ringbuffer_pagecount << PAGE_SHIFT);
+}
+
+static const struct bin_attribute ring_buffer_bin_attr = {
+ .attr = {
+ .name = "ring",
+ .mode = 0600,
+ },
+ .size = 2 * HV_RING_SIZE,
+ .mmap = vfio_vmbus_ring_mmap,
+};
+
+static void
+vfio_vmbus_new_channel(struct vmbus_channel *new_sc)
+{
+ struct hv_device *hv_dev = new_sc->primary_channel->device_obj;
+ struct device *device = &hv_dev->device;
+ int ret;
+
+ /* Create host communication ring */
+ ret = vmbus_open(new_sc, HV_RING_SIZE, HV_RING_SIZE, NULL, 0,
+ vfio_vmbus_channel_cb, new_sc);
+ if (ret) {
+ dev_err(device, "vmbus_open subchannel failed: %d\n", ret);
+ return;
+ }
+
+ /* Disable interrupts on sub channel */
+ new_sc->inbound.ring_buffer->interrupt_mask = 1;
+ set_channel_read_mode(new_sc, HV_CALL_ISR);
+
+ ret = sysfs_create_bin_file(&new_sc->kobj, &ring_buffer_bin_attr);
+ if (ret)
+ dev_notice(&hv_dev->device,
+ "sysfs create ring bin file failed; %d\n", ret);
+}
+
+static int vfio_vmbus_open(void *device_data)
+{
+ struct vfio_vmbus_device *vdev = device_data;
+ struct hv_device *dev = vdev->hdev;
+ int ret;
+
+ vmbus_set_sc_create_callback(dev->channel, vfio_vmbus_new_channel);
+
+ ret = vmbus_connect_ring(dev->channel,
+ vfio_vmbus_channel_cb, dev->channel);
+ if (ret == 0)
+ dev->channel->inbound.ring_buffer->interrupt_mask = 1;
+
+ return 0;
+}
+
+static long vfio_vmbus_ioctl(void *device_data, unsigned int cmd,
+ unsigned long arg)
+{
+ struct vfio_vmbus_device *vdev = device_data;
+ unsigned long minsz;
+
+ if (cmd == VFIO_DEVICE_GET_INFO) {
+ struct vfio_device_info info;
+
+ minsz = offsetofend(struct vfio_device_info, num_irqs);
+
+ if (copy_from_user(&info, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (info.argsz < minsz)
+ return -EINVAL;
+
+ info.flags = VFIO_DEVICE_FLAGS_VMBUS;
+ info.num_regions = VFIO_VMBUS_MAX_MAP_NUM;
+
+ return copy_to_user((void __user *)arg, &info, minsz) ?
+ -EFAULT : 0;
+ } else if (cmd == VFIO_DEVICE_GET_REGION_INFO) {
+ struct vfio_region_info info;
+
+ minsz = offsetofend(struct vfio_region_info, offset);
+
+ if (copy_from_user(&info, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (info.argsz < minsz)
+ return -EINVAL;
+
+ if (info.index >= VFIO_VMBUS_MAX_MAP_NUM)
+ return -EINVAL;
+
+ info.offset = vdev->mem[info.index].addr;
+ info.size = vdev->mem[info.index].size;
+ info.flags = VFIO_REGION_INFO_FLAG_READ
+ | VFIO_REGION_INFO_FLAG_WRITE
+ | VFIO_REGION_INFO_FLAG_MMAP;
+
+ return copy_to_user((void __user *)arg, &info, minsz) ?
+ -EFAULT : 0;
+ }
+
+ return -ENOTTY;
+}
+
+static void vfio_vmbus_release(void *device_data)
+{
+ struct vfio_vmbus_device *vdev = device_data;
+
+ vmbus_disconnect_ring(vdev->hdev->channel);
+}
+
+static vm_fault_t vfio_vma_fault(struct vm_fault *vmf)
+{
+ struct vfio_vmbus_device *vdev = vmf->vma->vm_private_data;
+ struct vm_area_struct *vma = vmf->vma;
+ struct page *page;
+ unsigned long offset;
+ void *addr;
+ int index;
+
+ index = vma->vm_pgoff;
+
+ /*
+ * Page fault should only happen on mmap regiones
+ * and bypass GPADL indexes here.
+ */
+ if (index >= VFIO_VMBUS_MAX_MAP_NUM - 2)
+ return VM_FAULT_SIGBUS;
+
+ offset = (vmf->pgoff - index) << PAGE_SHIFT;
+ addr = (void *)(vdev->mem[index].addr + offset);
+
+ if (index == VFIO_VMBUS_SEND_BUF_MAP ||
+ index == VFIO_VMBUS_RECV_BUF_MAP)
+ page = vmalloc_to_page(addr);
+ else
+ page = virt_to_page(addr);
+
+ get_page(page);
+ vmf->page = page;
+
+ return 0;
+}
+
+static const struct vm_operations_struct vfio_logical_vm_ops = {
+ .fault = vfio_vma_fault,
+};
+
+static const struct vm_operations_struct vfio_physical_vm_ops = {
+#ifdef CONFIG_HAVE_IOREMAP_PROT
+ .access = generic_access_phys,
+#endif
+};
+
+static int vfio_vmbus_mmap(void *device_data, struct vm_area_struct *vma)
+{
+ struct vfio_vmbus_device *vdev = device_data;
+ int index;
+
+ if (vma->vm_end < vma->vm_start)
+ return -EINVAL;
+
+ /*
+ * Mmap should only happen on map regions
+ * and bypass GPADL index here.
+ */
+ if (vma->vm_pgoff >= VFIO_VMBUS_MAX_MAP_NUM - 2)
+ return -EINVAL;
+
+ index = vma->vm_pgoff;
+ vma->vm_private_data = vdev;
+
+ if (index == VFIO_VMBUS_TXRX_RING_MAP) {
+ u64 addr;
+
+ addr = vdev->mem[VFIO_VMBUS_TXRX_RING_MAP].addr;
+ vma->vm_ops = &vfio_physical_vm_ops;
+ return remap_pfn_range(vma,
+ vma->vm_start,
+ addr >> PAGE_SHIFT,
+ vma->vm_end - vma->vm_start,
+ vma->vm_page_prot);
+ } else {
+ vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
+ vma->vm_ops = &vfio_logical_vm_ops;
+ return 0;
+ }
+}
+
+static const struct vfio_device_ops vfio_vmbus_ops = {
+ .name = "vfio-vmbus",
+ .open = vfio_vmbus_open,
+ .release = vfio_vmbus_release,
+ .mmap = vfio_vmbus_mmap,
+ .ioctl = vfio_vmbus_ioctl,
+};
+
+static int vfio_vmbus_probe(struct hv_device *dev,
+ const struct hv_vmbus_device_id *dev_id)
+{
+ struct vmbus_channel *channel = dev->channel;
+ struct vfio_vmbus_device *vdev;
+ struct iommu_group *group;
+ u32 gpadl;
+ void *ring_buffer;
+ int ret;
+
+ group = vfio_iommu_group_get(&dev->device);
+ if (!group)
+ return -EINVAL;
+
+ vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
+ if (!vdev) {
+ vfio_iommu_group_put(group, &dev->device);
+ return -ENOMEM;
+ }
+
+ ret = vfio_add_group_dev(&dev->device, &vfio_vmbus_ops, vdev);
+ if (ret)
+ goto free_vdev;
+
+ vdev->hdev = dev;
+ ret = vmbus_alloc_ring(channel, HV_RING_SIZE, HV_RING_SIZE);
+ if (ret)
+ goto del_group_dev;
+
+ set_channel_read_mode(channel, HV_CALL_ISR);
+
+ ring_buffer = page_address(channel->ringbuffer_page);
+ vdev->mem[VFIO_VMBUS_TXRX_RING_MAP].addr
+ = virt_to_phys(ring_buffer);
+ vdev->mem[VFIO_VMBUS_TXRX_RING_MAP].size
+ = channel->ringbuffer_pagecount << PAGE_SHIFT;
+
+ vdev->mem[VFIO_VMBUS_INT_PAGE_MAP].addr
+ = (phys_addr_t)vmbus_connection.int_page;
+ vdev->mem[VFIO_VMBUS_INT_PAGE_MAP].size = PAGE_SIZE;
+
+ vdev->mem[VFIO_VMBUS_MON_PAGE_MAP].addr
+ = (phys_addr_t)vmbus_connection.monitor_pages[1];
+ vdev->mem[VFIO_VMBUS_MON_PAGE_MAP].size = PAGE_SIZE;
+
+ vdev->recv_buf = vzalloc(RECV_BUFFER_SIZE);
+ if (vdev->recv_buf == NULL) {
+ ret = -ENOMEM;
+ goto free_ring;
+ }
+
+ ret = vmbus_establish_gpadl(channel, vdev->recv_buf,
+ RECV_BUFFER_SIZE, &gpadl);
+ if (ret)
+ goto free_recv_buf;
+
+ vdev->mem[VFIO_VMBUS_RECV_BUF_MAP].addr
+ = (phys_addr_t)vdev->recv_buf;
+ vdev->mem[VFIO_VMBUS_RECV_BUF_MAP].size = RECV_BUFFER_SIZE;
+
+ /* Expose Recv GPADL via region info. */
+ vdev->mem[VFIO_VMBUS_RECV_GPADL].addr = gpadl;
+
+ vdev->send_buf = vzalloc(SEND_BUFFER_SIZE);
+ if (vdev->send_buf == NULL) {
+ ret = -ENOMEM;
+ goto free_recv_gpadl;
+ }
+
+ ret = vmbus_establish_gpadl(channel, vdev->send_buf,
+ SEND_BUFFER_SIZE, &gpadl);
+ if (ret)
+ goto free_send_buf;
+
+ vdev->mem[VFIO_VMBUS_SEND_BUF_MAP].addr
+ = (phys_addr_t)vdev->send_buf;
+ vdev->mem[VFIO_VMBUS_SEND_BUF_MAP].size = SEND_BUFFER_SIZE;
+
+ /* Expose Send GPADL via region info. */
+ vdev->mem[VFIO_VMBUS_SEND_GPADL].addr = gpadl;
+
+ ret = sysfs_create_bin_file(&channel->kobj, &ring_buffer_bin_attr);
+ if (ret)
+ dev_notice(&dev->device,
+ "sysfs create ring bin file failed; %d\n", ret);
+
+ return 0;
+
+ free_send_buf:
+ vfree(vdev->send_buf);
+ free_recv_gpadl:
+ vmbus_teardown_gpadl(channel, vdev->mem[VFIO_VMBUS_RECV_GPADL].addr);
+ free_recv_buf:
+ vfree(vdev->recv_buf);
+ free_ring:
+ vmbus_free_ring(channel);
+ del_group_dev:
+ vfio_del_group_dev(&dev->device);
+ free_vdev:
+ vfio_iommu_group_put(group, &dev->device);
+ kfree(vdev);
+ return -EINVAL;
+}
+
+static int vfio_vmbus_remove(struct hv_device *hdev)
+{
+ struct vfio_vmbus_device *vdev = vfio_del_group_dev(&hdev->device);
+ struct vmbus_channel *channel = hdev->channel;
+
+ if (!vdev)
+ return 0;
+
+ sysfs_remove_bin_file(&channel->kobj, &ring_buffer_bin_attr);
+
+ vmbus_teardown_gpadl(channel, vdev->mem[VFIO_VMBUS_SEND_GPADL].addr);
+ vmbus_teardown_gpadl(channel, vdev->mem[VFIO_VMBUS_RECV_GPADL].addr);
+ vfree(vdev->recv_buf);
+ vfree(vdev->send_buf);
+ vmbus_free_ring(channel);
+ vfio_iommu_group_put(hdev->device.iommu_group, &hdev->device);
+ kfree(vdev);
+
+ return 0;
+}
+
+static struct hv_driver hv_vfio_drv = {
+ .name = "hv_vfio",
+ .id_table = NULL,
+ .probe = vfio_vmbus_probe,
+ .remove = vfio_vmbus_remove,
+};
+
+static int __init vfio_vmbus_init(void)
+{
+ return vmbus_driver_register(&hv_vfio_drv);
+}
+
+static void __exit vfio_vmbus_exit(void)
+{
+ vmbus_driver_unregister(&hv_vfio_drv);
+}
+
+module_init(vfio_vmbus_init);
+module_exit(vfio_vmbus_exit);
+
+MODULE_VERSION(DRIVER_VERSION);
+MODULE_LICENSE("GPL v2");
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 9e843a147ead..611baf7a30e4 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -201,6 +201,7 @@ struct vfio_device_info {
#define VFIO_DEVICE_FLAGS_AMBA (1 << 3) /* vfio-amba device */
#define VFIO_DEVICE_FLAGS_CCW (1 << 4) /* vfio-ccw device */
#define VFIO_DEVICE_FLAGS_AP (1 << 5) /* vfio-ap device */
+#define VFIO_DEVICE_FLAGS_VMBUS (1 << 6) /* vfio-vmbus device */
__u32 num_regions; /* Max region index + 1 */
__u32 num_irqs; /* Max IRQ index + 1 */
};
@@ -564,6 +565,17 @@ enum {
VFIO_PCI_NUM_IRQS
};
+enum {
+ VFIO_VMBUS_TXRX_RING_MAP = 0,
+ VFIO_VMBUS_INT_PAGE_MAP,
+ VFIO_VMBUS_MON_PAGE_MAP,
+ VFIO_VMBUS_RECV_BUF_MAP,
+ VFIO_VMBUS_SEND_BUF_MAP,
+ VFIO_VMBUS_RECV_GPADL,
+ VFIO_VMBUS_SEND_GPADL,
+ VFIO_VMBUS_MAX_MAP_NUM,
+};
+
/*
* The vfio-ccw bus driver makes use of the following fixed region and
* IRQ index mapping. Unimplemented regions return a size of zero.
--
2.14.5
^ permalink raw reply related
* Re: [PATCH] VFIO/VMBUS: Add VFIO VMBUS driver support
From: Greg KH @ 2019-11-11 9:49 UTC (permalink / raw)
To: lantianyu1986
Cc: alex.williamson, cohuck, kys, haiyangz, sthemmin, sashal,
mchehab+samsung, davem, robh, Jonathan.Cameron, paulmck,
michael.h.kelley, Tianyu Lan, linux-kernel, kvm, linux-hyperv,
vkuznets
In-Reply-To: <20191111084507.9286-1-Tianyu.Lan@microsoft.com>
On Mon, Nov 11, 2019 at 04:45:07PM +0800, lantianyu1986@gmail.com wrote:
> +#define DRIVER_VERSION "0.0.1"
Never a need for DRIVER_VERSION as your driver just becomes part of the
main kernel tree, so please drop this. We keep trying to delete these
types of numbers and they keep coming back...
> +static void
> +vfio_vmbus_new_channel(struct vmbus_channel *new_sc)
> +{
> + struct hv_device *hv_dev = new_sc->primary_channel->device_obj;
> + struct device *device = &hv_dev->device;
> + int ret;
> +
> + /* Create host communication ring */
> + ret = vmbus_open(new_sc, HV_RING_SIZE, HV_RING_SIZE, NULL, 0,
> + vfio_vmbus_channel_cb, new_sc);
> + if (ret) {
> + dev_err(device, "vmbus_open subchannel failed: %d\n", ret);
> + return;
> + }
> +
> + /* Disable interrupts on sub channel */
> + new_sc->inbound.ring_buffer->interrupt_mask = 1;
> + set_channel_read_mode(new_sc, HV_CALL_ISR);
> +
> + ret = sysfs_create_bin_file(&new_sc->kobj, &ring_buffer_bin_attr);
No documentation on this new sysfs file?
And by creating it here, userspace is not notified of it, so tools will
not see it :(
> + if (ret)
> + dev_notice(&hv_dev->device,
> + "sysfs create ring bin file failed; %d\n", ret);
Doesn't the call spit out an error if something happens?
> + ret = sysfs_create_bin_file(&channel->kobj, &ring_buffer_bin_attr);
> + if (ret)
> + dev_notice(&dev->device,
> + "sysfs create ring bin file failed; %d\n", ret);
> +
Again, don't create sysfs files on your own, the bus code should be
doing this for you automatically and in a way that is race-free.
thanks,
greg k-h
^ 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