* Re: [PATCH] scsi: storvsc: Use kzfree() in storvsc_suspend()
From: Denis Efremov @ 2020-06-04 21:43 UTC (permalink / raw)
To: Dexuan-Linux Cui, Dexuan Cui, Michael Kelley
Cc: James E.J. Bottomley, Martin K. Petersen, linux-hyperv,
Linux SCSI List, Linux Kernel Mailing List
In-Reply-To: <CAA42JLat6Ern5_mztmoBX9-ONtmz=gZE3YUphY+njTa+A=efVw@mail.gmail.com>
>
> Hi Denis,
> When I added the function storvsc_suspend() several months ago, somehow I forgot
> to remove the unnecessary memset(). Sorry!
>
> The buffer is recreated in storvsc_resume() -> storvsc_connect_to_vsp() ->
> storvsc_channel_init() -> stor_device->stor_chns = kcalloc(...), so I believe
> the memset() can be safely removed.
I'm not sure that I understand your description. As for me, memset with 0 before
memory freeing is required only for sensitive information and it's completely
unrelated to memory zeroing during allocation with kzalloc/kcalloc.
If it's not a sensitive information then memset could be safely removed.
> Can you please make a v2 patch for it and Cc my corporate email "decui" (in To)?
Yes, of course. Could I add "Suggested-by"?
Thanks,
Denis
^ permalink raw reply
* Re: [PATCH] scsi: storvsc: Use kzfree() in storvsc_suspend()
From: Dexuan-Linux Cui @ 2020-06-04 17:05 UTC (permalink / raw)
To: Denis Efremov, Dexuan Cui, Michael Kelley
Cc: James E.J. Bottomley, Martin K. Petersen, linux-hyperv,
Linux SCSI List, Linux Kernel Mailing List
In-Reply-To: <20200604130406.108940-1-efremov@linux.com>
On Thu, Jun 4, 2020 at 6:06 AM Denis Efremov <efremov@linux.com> wrote:
>
> Use kzfree() instead of memset() with 0 followed by kfree().
>
> Signed-off-by: Denis Efremov <efremov@linux.com>
> ---
> drivers/scsi/storvsc_drv.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 072ed8728657..e5a19cd8a450 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -2035,10 +2035,7 @@ static int storvsc_suspend(struct hv_device *hv_dev)
>
> vmbus_close(hv_dev->channel);
>
> - memset(stor_device->stor_chns, 0,
> - num_possible_cpus() * sizeof(void *));
> -
> - kfree(stor_device->stor_chns);
> + kzfree(stor_device->stor_chns);
> stor_device->stor_chns = NULL;
>
> cpumask_clear(&stor_device->alloced_cpus);
> --
> 2.26.2
Hi Denis,
When I added the function storvsc_suspend() several months ago, somehow I forgot
to remove the unnecessary memset(). Sorry!
The buffer is recreated in storvsc_resume() -> storvsc_connect_to_vsp() ->
storvsc_channel_init() -> stor_device->stor_chns = kcalloc(...), so I believe
the memset() can be safely removed. Can you please make a v2 patch for it
and Cc my corporate email "decui" (in To)?
Thanks,
Dexuan
^ permalink raw reply
* [PATCH] scsi: storvsc: Use kzfree() in storvsc_suspend()
From: Denis Efremov @ 2020-06-04 13:04 UTC (permalink / raw)
To: James E.J. Bottomley
Cc: Denis Efremov, Martin K. Petersen, linux-hyperv, linux-scsi,
linux-kernel
Use kzfree() instead of memset() with 0 followed by kfree().
Signed-off-by: Denis Efremov <efremov@linux.com>
---
drivers/scsi/storvsc_drv.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 072ed8728657..e5a19cd8a450 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -2035,10 +2035,7 @@ static int storvsc_suspend(struct hv_device *hv_dev)
vmbus_close(hv_dev->channel);
- memset(stor_device->stor_chns, 0,
- num_possible_cpus() * sizeof(void *));
-
- kfree(stor_device->stor_chns);
+ kzfree(stor_device->stor_chns);
stor_device->stor_chns = NULL;
cpumask_clear(&stor_device->alloced_cpus);
--
2.26.2
^ permalink raw reply related
* Re: [GIT PULL] Hyper-V commits for 5.8
From: pr-tracker-bot @ 2020-06-03 22:30 UTC (permalink / raw)
To: Wei Liu
Cc: Linus Torvalds, Wei Liu, kys, sthemmin, haiyangz, Michael Kelley,
Linux on Hyper-V List, Linux Kernel List
In-Reply-To: <20200602174752.u67tmpxojt5dv655@liuwe-devbox-debian-v2.j3c5onc20sse1dnehy4noqpfcg.zx.internal.cloudapp.net>
The pull request you sent on Tue, 2 Jun 2020 17:47:52 +0000:
> ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git tags/hyperv-next-signed
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/6b2591c21273ebf65c13dae5d260ce88f0f197dd
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* RE: [RFC PATCH 1/2] Drivers: hv: vmbus: Re-balance channel interrupts across CPUs at CPU hotplug
From: Michael Kelley @ 2020-06-02 23:47 UTC (permalink / raw)
To: Andrea Parri (Microsoft), linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Nuno Das Neves
In-Reply-To: <20200526223218.184057-2-parri.andrea@gmail.com>
From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Tuesday, May 26, 2020 3:32 PM
>
> CPU hot removals and additions present an opportunity for (re-)balancing
> the channel interrupts across the available CPUs. Current code does not
> balance the interrupts at CPU hotplug; furthermore/consequently, the hot
> removal path currently fails (to remove the specified CPU) whenever some
> interrupt is bound to the CPU to be removed and the VMBus is connected.
>
> Address such issues by implementing vmbus_balance_vp_indexes_at_cpuhp():
> invoke this primitive to balance the channel interrupts across available
> CPUs at CPU hotplug operations. In the hot removal path, such primitive
> will (try to) move/balance interrupts out of the to-be-removed CPU so as
> to meet the user request to hot remove the CPU.
>
> The balancing algorithm distributes the channel interrupts evenly across
> the available CPUs and NUMA nodes; to do so, it introduces and maintains
> per-device and per-connection channel statistics/counts to keep track of
> the (current) assignments of the channels to the CPUs/nodes. By design,
> only "performance"-critical channels/devices are "balanced".
>
> The proposed algorithm relies on the (recently introduced) capability to
> reassign a channel interrupt to a CPU (cf., the CHANNELMSG_MODIFYCHANNEL
> message type). As such, the new balancing process is effective starting
> with VMBus version 4.1 (no changes in semantics or behavior are intended
> for VMBus versions lower than 4.1).
>
> Suggested-by: Nuno Das Neves <nuno.das@microsoft.com>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
> drivers/hv/channel.c | 38 +++++++
> drivers/hv/channel_mgmt.c | 219 ++++++++++++++++++++++++++++++++++++++
> drivers/hv/connection.c | 23 ++--
> drivers/hv/hv.c | 62 ++++++-----
> drivers/hv/hyperv_vmbus.h | 72 +++++++++++++
> drivers/hv/vmbus_drv.c | 45 +++-----
> include/linux/hyperv.h | 22 +++-
> kernel/cpu.c | 1 +
> 8 files changed, 416 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index 90070b337c10d..2974aa9dc956c 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -18,6 +18,7 @@
> #include <linux/uio.h>
> #include <linux/interrupt.h>
> #include <asm/page.h>
> +#include <asm/mshyperv.h>
>
> #include "hyperv_vmbus.h"
>
> @@ -317,6 +318,43 @@ int vmbus_send_modifychannel(u32 child_relid, u32 target_vp)
> }
> EXPORT_SYMBOL_GPL(vmbus_send_modifychannel);
>
> +bool vmbus_modifychannel(struct vmbus_channel *channel,
> + u32 origin_cpu, u32 target_cpu)
> +{
> + if (vmbus_send_modifychannel(channel->offermsg.child_relid,
> + hv_cpu_number_to_vp_number(target_cpu)))
> + return false;
> +
> + /*
> + * Warning. At this point, there is *no* guarantee that the host will
> + * have successfully processed the vmbus_send_modifychannel() request.
> + * See the header comment of vmbus_send_modifychannel() for more info.
> + *
> + * Lags in the processing of the above vmbus_send_modifychannel() can
> + * result in missed interrupts if the "old" target CPU is taken offline
> + * before Hyper-V starts sending interrupts to the "new" target CPU.
> + * But apart from this offlining scenario, the code tolerates such
> + * lags. It will function correctly even if a channel interrupt comes
> + * in on a CPU that is different from the channel target_cpu value.
> + */
> +
> + channel->target_cpu = target_cpu;
> + channel->target_vp = hv_cpu_number_to_vp_number(target_cpu);
> + channel->numa_node = cpu_to_node(target_cpu);
> +
> + /* See init_vp_index(). */
> + if (hv_is_perf_channel(channel))
> + hv_update_alloced_cpus(origin_cpu, target_cpu);
> +
> + /* Currently set only for storvsc channels. */
> + if (channel->change_target_cpu_callback) {
> + (*channel->change_target_cpu_callback)(channel,
> + origin_cpu, target_cpu);
> + }
> +
> + return true;
> +}
> +
> /*
> * create_gpadl_header - Creates a gpadl for the specified buffer
> */
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 417a95e5094dd..c158f86787940 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -497,10 +497,14 @@ static void vmbus_add_channel_work(struct work_struct *work)
> /*
> * Start the process of binding the primary channel to the driver
> */
> +
> + /* See vmbus_balance_vp_indexes_at_cpuhp(). */
> + mutex_lock(&vmbus_connection.channel_mutex);
> newchannel->device_obj = vmbus_device_create(
> &newchannel->offermsg.offer.if_type,
> &newchannel->offermsg.offer.if_instance,
> newchannel);
> + mutex_unlock(&vmbus_connection.channel_mutex);
> if (!newchannel->device_obj)
> goto err_deq_chan;
>
> @@ -515,6 +519,8 @@ static void vmbus_add_channel_work(struct work_struct *work)
> if (ret != 0) {
> pr_err("unable to add child device object (relid %d)\n",
> newchannel->offermsg.child_relid);
> + if (hv_is_perf_channel(newchannel))
> + free_chn_counts(&newchannel->device_obj->chn_cnt);
You could drop the "if" condition and just always call free_chn_counts() since
it will do the right thing for non-perf channels where the memory was never
allocated.
> kfree(newchannel->device_obj);
> goto err_deq_chan;
> }
> @@ -667,6 +673,219 @@ static void vmbus_process_offer(struct vmbus_channel
> *newchannel)
> queue_work(wq, &newchannel->add_channel_work);
> }
>
> +static void filter_on_cpus(cpumask_var_t cpu_msk,
> + struct vmbus_channel_counts *chn_cnt)
> +{
> + unsigned int min_cnt = MAX_CHANNEL_RELIDS;
> + unsigned int cpu;
> +
> + for_each_cpu(cpu, cpu_msk) {
> + unsigned int cnt = chn_cnt->cpu[cpu];
> +
> + if (cnt < min_cnt)
> + min_cnt = cnt;
> + }
> +
> + for_each_cpu(cpu, cpu_msk) {
> + if (chn_cnt->cpu[cpu] > min_cnt)
> + cpumask_clear_cpu(cpu, cpu_msk);
> + }
> +}
> +
> +static void filter_on_nodes(cpumask_var_t cpu_msk,
> + struct vmbus_channel_counts *chn_cnt)
> +{
> + unsigned int min_cnt = MAX_CHANNEL_RELIDS;
> + unsigned int cpu;
> +
> + for_each_cpu(cpu, cpu_msk) {
> + unsigned int node = cpu_to_node(cpu);
> + unsigned int cnt = chn_cnt->node[node];
> +
> + if (cnt < min_cnt)
> + min_cnt = cnt;
> +
> + cpu = cpumask_last(cpumask_of_node(node));
> + }
> +
> + for_each_cpu(cpu, cpu_msk) {
> + unsigned int node = cpu_to_node(cpu);
> + const struct cpumask *msk_node = cpumask_of_node(node);
> +
> + if (chn_cnt->node[node] > min_cnt)
> + cpumask_andnot(cpu_msk, cpu_msk, msk_node);
> +
> + cpu = cpumask_last(msk_node);
> + }
> +}
> +
> +static inline void __filter_vp_index(struct vmbus_channel_counts *chn_cnt,
> + cpumask_var_t cpu_msk)
> +{
> + filter_on_cpus(cpu_msk, chn_cnt);
> + filter_on_nodes(cpu_msk, chn_cnt);
> +}
> +
> +static void filter_vp_index(struct hv_device *hv_dev, cpumask_var_t cpu_msk)
> +{
> + /*
> + * The selection of the target CPUs is performed in two domains,
> + * the device domain and the connection domain. At each domain,
> + * in turn, invalid CPUs are filtered out at two levels, the CPU
I would drop the word "invalid". You are filtering out CPUs that meet the
criteria in the sentence starting after the colon below.
> + * level and the NUMA-node level: CPUs corresponding to channel
> + * counts *greater* than the minimum channel count for the given
> + * level/domain are cleared in the mask of candidate target CPUs.
> + */
> +
> + __filter_vp_index(&hv_dev->chn_cnt, cpu_msk);
> + __filter_vp_index(&vmbus_connection.chn_cnt, cpu_msk);
> +}
> +
> +static void balance_vp_index(struct vmbus_channel *chn,
> + struct hv_device *hv_dev, cpumask_var_t cpu_msk)
> +{
> + u32 cur_cpu = chn->target_cpu, tgt_cpu = cur_cpu;
> +
> + if (chn->state != CHANNEL_OPENED_STATE) {
> + /*
> + * The channel may never have been opened or it may be in
> + * a closed/closing state; if so, do not touch the target
> + * CPU of this channel.
> + */
> + goto update_chn_cnts;
> + }
> +
> + /*
> + * The channel was open, and it will remain open until we release
> + * channel_mutex, cf. the use of channel_mutex and channel->state
> + * in vmbus_disconnect_ring() -> vmbus_close_internal().
> + */
> +
> + if (!hv_is_perf_channel(chn)) {
> + /*
> + * Only used by the CPU hot removal path, remark that
> + * the connect CPU can not go offline.
To be super explicit in the comment: If the channel is not a
performance channel, then it does not participate in the balancing,
and we move it back to interrupting the VMBUS_CONNECT_CPU for
lack of a better choice. Because non-perf channels are initially set to
interrupt the VMBUS_CONNECT_CPU, the only way a non-perf channel
could be found in this state (i.e., set to a CPU other than
VMBUS_CONNECT_CPU) is a manual change through the sysfs interface.
> + */
> + tgt_cpu = VMBUS_CONNECT_CPU;
> + goto modify_vp_index;
> + }
> +
> + /*
> + * Retrieve the new "candidate" target CPUs for this channel
> + * /device; see the inline comments in filter_vp_index() for
> + * a high-level description of this algorithm.
> + */
> + filter_vp_index(hv_dev, cpu_msk);
> +
> + /*
> + * (Try to) preserve the channel's CPU and NUMA node affinities
> + * respectively:
> + */
> +
> + if (cpumask_test_cpu(cur_cpu, cpu_msk))
> + goto update_chn_cnts;
> +
> + /*
> + * If we reach here, a modification of the channel's target CPU
> + * is "needed".
> + */
> +
> + tgt_cpu = cpumask_first_and(cpumask_of_node(cpu_to_node(cur_cpu)),
> + cpu_msk);
> + if (tgt_cpu < nr_cpu_ids)
> + goto modify_vp_index;
> +
> + /*
> + * It was not possible to preserve this channel's CPU and NUMA
> + * node affinities; pick the "first" candidate target CPU (the
> + * CPU mask can not be empty).
> + */
> + tgt_cpu = cpumask_first(cpu_msk);
> +
> +modify_vp_index:
> + if (!vmbus_modifychannel(chn, cur_cpu, tgt_cpu))
> + tgt_cpu = cur_cpu;
> +
> +update_chn_cnts:
> + /* Do not account for non-"perf" channels in chn_cnt. */
> + if (!hv_is_perf_channel(chn))
> + return;
> +
> + inc_chn_counts(&hv_dev->chn_cnt, tgt_cpu);
> + inc_chn_counts(&vmbus_connection.chn_cnt, tgt_cpu);
> +}
> +
> +void vmbus_balance_vp_indexes_at_cpuhp(unsigned int cpu, bool add)
> +{
> + struct vmbus_channel *chn, *sc;
> + cpumask_var_t cpu_msk;
> +
> + lockdep_assert_cpus_held();
> + lockdep_assert_held(&vmbus_connection.channel_mutex);
> +
> + WARN_ON(!cpumask_test_cpu(cpu, cpu_online_mask));
> +
> + /* See the header comment for vmbus_send_modifychannel(). */
> + if (vmbus_proto_version < VERSION_WIN10_V4_1)
> + return;
> +
> + if (!alloc_cpumask_var(&cpu_msk, GFP_KERNEL))
> + return;
> +
> + reset_chn_counts(&vmbus_connection.chn_cnt);
> +
> + list_for_each_entry(chn, &vmbus_connection.chn_list, listentry) {
> + struct hv_device *dev = chn->device_obj;
> +
> + /*
> + * The device may not have been allocated/assigned to
> + * the primary channel yet; if so, do not balance the
> + * channels associated to this device. If dev != NULL,
> + * the synchronization on channel_mutex ensures that
> + * the device's chn_cnt has been (properly) allocated
> + * *and* initialized, cf. vmbus_add_channel_work().
> + */
> + if (dev == NULL)
> + continue;
> +
> + /*
> + * By design, non-"perf" channels do not take part in
> + * the balancing process.
> + *
> + * The user may have assigned some non-"perf" channel
> + * to this CPU. To satisfy the user's request to hot
> + * remove the CPU, we will re-assign such channels to
> + * the connect CPU; cf. balance_vp_index().
> + */
> + if (!hv_is_perf_channel(chn)) {
> + if (add)
> + continue;
> + /*
> + * Assume that the non-"perf" channel has no
> + * sub-channels.
> + */
The "if" statement below could use a bit further explanation to help
the reader. :-) If this non-perf channel is assigned to some CPU other
than the one we are hot-removing, then we execute the "continue"
statement and leave its target CPU unchanged. But if it is assigned
to the CPU we are hot removing, then we need to move the channel
to some other CPU.
I'm also not clear on how the above comment about having no
sub-channels is relevant. Maybe a bit more explanation would
help.
> + if (chn->target_cpu != cpu)
> + continue;
> + } else {
> + reset_chn_counts(&dev->chn_cnt);
> + }
> +
> + cpumask_copy(cpu_msk, cpu_online_mask);
> + if (!add)
> + cpumask_clear_cpu(cpu, cpu_msk);
> + balance_vp_index(chn, dev, cpu_msk);
> +
> + list_for_each_entry(sc, &chn->sc_list, sc_list) {
> + cpumask_copy(cpu_msk, cpu_online_mask);
> + if (!add)
> + cpumask_clear_cpu(cpu, cpu_msk);
> + balance_vp_index(sc, dev, cpu_msk);
> + }
> + }
> +
> + free_cpumask_var(cpu_msk);
> +}
> +
> /*
> * We use this state to statically distribute the channel interrupt load.
> */
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index 11170d9a2e1a5..7ec562fac8e58 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -183,6 +183,18 @@ int vmbus_connect(void)
> spin_lock_init(&vmbus_connection.channelmsg_lock);
>
> INIT_LIST_HEAD(&vmbus_connection.chn_list);
> +
> + vmbus_connection.channels = kcalloc(MAX_CHANNEL_RELIDS,
> + sizeof(struct vmbus_channel *),
> + GFP_KERNEL);
> + if (vmbus_connection.channels == NULL) {
> + ret = -ENOMEM;
> + goto cleanup;
> + }
> +
> + if (alloc_chn_counts(&vmbus_connection.chn_cnt))
> + goto cleanup;
> +
> mutex_init(&vmbus_connection.channel_mutex);
>
> /*
> @@ -248,14 +260,6 @@ int vmbus_connect(void)
> pr_info("Vmbus version:%d.%d\n",
> version >> 16, version & 0xFFFF);
>
> - vmbus_connection.channels = kcalloc(MAX_CHANNEL_RELIDS,
> - sizeof(struct vmbus_channel *),
> - GFP_KERNEL);
> - if (vmbus_connection.channels == NULL) {
> - ret = -ENOMEM;
> - goto cleanup;
> - }
> -
> kfree(msginfo);
> return 0;
>
> @@ -295,6 +299,9 @@ void vmbus_disconnect(void)
> hv_free_hyperv_page((unsigned long)vmbus_connection.monitor_pages[1]);
> vmbus_connection.monitor_pages[0] = NULL;
> vmbus_connection.monitor_pages[1] = NULL;
> +
> + free_chn_counts(&vmbus_connection.chn_cnt);
> + kfree(vmbus_connection.channels);
> }
>
> /*
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 188b42b07f07b..e1cade3c3c967 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -198,6 +198,20 @@ void hv_synic_enable_regs(unsigned int cpu)
>
> int hv_synic_init(unsigned int cpu)
> {
> + /*
> + * The CPU has been hot added: try to re-balance the channels
> + * across the online CPUs (including "this" CPU), provided that
> + * the VMBus is connected; in part., avoid the re-balancing at
> + * the very first CPU initialization.
> + *
> + * See also inline comments in hv_synic_cleanup().
> + */
> + if (vmbus_connection.conn_state == CONNECTED) {
> + mutex_lock(&vmbus_connection.channel_mutex);
> + vmbus_balance_vp_indexes_at_cpuhp(cpu, true);
Does the target CPU have its bit in cpu_online_mask set at the time this
is executed? reset_chn_counts() does a for_each_online_cpu() loop, and
we want to make sure the count for this CPU gets reset to zero.
> + mutex_unlock(&vmbus_connection.channel_mutex);
> + }
> +
> hv_synic_enable_regs(cpu);
>
> hv_stimer_legacy_init(cpu, VMBUS_MESSAGE_SINT);
> @@ -243,10 +257,6 @@ void hv_synic_disable_regs(unsigned int cpu)
>
> int hv_synic_cleanup(unsigned int cpu)
> {
> - struct vmbus_channel *channel, *sc;
> - bool channel_found = false;
> - unsigned long flags;
> -
> /*
> * Hyper-V does not provide a way to change the connect CPU once
> * it is set; we must prevent the connect CPU from going offline.
> @@ -254,35 +264,37 @@ int hv_synic_cleanup(unsigned int cpu)
> if (cpu == VMBUS_CONNECT_CPU)
> return -EBUSY;
>
> + mutex_lock(&vmbus_connection.channel_mutex);
> + /*
> + * The CPU is being hot removed: re-balance the channels across
> + * the online CPUs but excluding "this" CPU.
> + *
> + * Note. We do not roll back on failure; IOW, we may end up with
> + * situations where the (re-)balancing process failed but some of
> + * the channels have been re-assigned to different CPUs.
> + *
> + * Also, the balancing process makes no attempt to re-assign "non-
> + * open" channels, that is, it only applies to channels which are
> + * found in CHANNEL_OPENED_STATE.
> + */
> + vmbus_balance_vp_indexes_at_cpuhp(cpu, false);
> +
> /*
> * Search for channels which are bound to the CPU we're about to
> * cleanup. In case we find one and vmbus is still connected, we
> * fail; this will effectively prevent CPU offlining.
> - *
> - * TODO: Re-bind the channels to different CPUs.
> */
> - mutex_lock(&vmbus_connection.channel_mutex);
> - list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
> - if (channel->target_cpu == cpu) {
> - channel_found = true;
> - break;
> - }
> - spin_lock_irqsave(&channel->lock, flags);
> - list_for_each_entry(sc, &channel->sc_list, sc_list) {
> - if (sc->target_cpu == cpu) {
> - channel_found = true;
> - break;
> - }
> - }
> - spin_unlock_irqrestore(&channel->lock, flags);
> - if (channel_found)
> - break;
> + if (hv_is_busy_cpu(cpu) && vmbus_connection.conn_state == CONNECTED) {
> + /*
> + * The CPU was busy, and we are going to prevent it from
> + * going offline; balance the channels accordingly.
> + */
> + vmbus_balance_vp_indexes_at_cpuhp(cpu, true);
> + mutex_unlock(&vmbus_connection.channel_mutex);
> + return -EBUSY;
> }
> mutex_unlock(&vmbus_connection.channel_mutex);
>
> - if (channel_found && vmbus_connection.conn_state == CONNECTED)
> - return -EBUSY;
> -
> hv_stimer_legacy_cleanup(cpu);
>
> hv_synic_disable_regs(cpu);
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index 40e2b9f91163c..b6d194caf69ed 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -18,6 +18,7 @@
> #include <linux/atomic.h>
> #include <linux/hyperv.h>
> #include <linux/interrupt.h>
> +#include <linux/slab.h>
>
> #include "hv_trace.h"
>
> @@ -250,6 +251,19 @@ struct vmbus_connection {
> /* Array of channels */
> struct vmbus_channel **channels;
>
> + /*
> + * Channel counts used by the channel-interrupts balancing scheme:
> + *
> + * - chn_cnt.cpu[cpu], the number of channel interrupts assigned
> + * to CPU #cpu;
> + *
> + * - chn_cnt.node[node], the number of channel interrupts assigned
> + * to NUMA node #node.
> + *
> + * The counts refer to "open" *and "performance" channels only.
> + */
> + struct vmbus_channel_counts chn_cnt;
> +
> /*
> * An offer message is handled first on the work_queue, and then
> * is further handled on handle_primary_chan_wq or
> @@ -344,6 +358,8 @@ struct vmbus_channel *relid2channel(u32 relid);
>
> void vmbus_free_channels(void);
>
> +void vmbus_balance_vp_indexes_at_cpuhp(unsigned int cpu, bool add);
> +
> /* Connection interface */
>
> int vmbus_connect(void);
> @@ -443,6 +459,62 @@ static inline void hv_update_alloced_cpus(unsigned int old_cpu,
> hv_clear_alloced_cpu(old_cpu);
> }
>
> +static inline bool hv_is_busy_cpu(unsigned int cpu)
> +{
> + struct vmbus_channel *channel, *sc;
> +
> + lockdep_assert_held(&vmbus_connection.channel_mutex);
> + /*
> + * List additions/deletions as well as updates of the target CPUs are
> + * protected by channel_mutex.
> + */
> + list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
> + if (channel->target_cpu == cpu)
> + return true;
> + list_for_each_entry(sc, &channel->sc_list, sc_list) {
> + if (sc->target_cpu == cpu)
> + return true;
> + }
> + }
> + return false;
> +}
> +
> +static inline int alloc_chn_counts(struct vmbus_channel_counts *chn_cnt)
> +{
> + chn_cnt->cpu = kcalloc(nr_cpu_ids, sizeof(unsigned int), GFP_KERNEL);
> + if (chn_cnt->cpu == NULL)
> + return -ENOMEM;
> +
> + chn_cnt->node = kcalloc(nr_node_ids, sizeof(unsigned int), GFP_KERNEL);
> + if (chn_cnt->node == NULL)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> +static inline void inc_chn_counts(struct vmbus_channel_counts *chn_cnt,
> + unsigned int cpu)
> +{
> + chn_cnt->cpu[cpu] += 1;
> + chn_cnt->node[cpu_to_node(cpu)] += 1;
> +}
> +
> +static inline void reset_chn_counts(struct vmbus_channel_counts *chn_cnt)
> +{
> + unsigned int i;
> +
> + for_each_online_cpu(i)
> + chn_cnt->cpu[i] = 0;
> + for_each_online_node(i)
> + chn_cnt->node[i] = 0;
> +}
> +
> +static inline void free_chn_counts(struct vmbus_channel_counts *chn_cnt)
> +{
> + kfree(chn_cnt->cpu);
> + kfree(chn_cnt->node);
> +}
> +
> #ifdef CONFIG_HYPERV_TESTING
>
> int hv_debug_add_dev_dir(struct hv_device *dev);
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 47747755d2e1d..a40b930fb86a5 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -980,6 +980,9 @@ static void vmbus_device_release(struct device *device)
> mutex_lock(&vmbus_connection.channel_mutex);
> hv_process_channel_removal(channel);
> mutex_unlock(&vmbus_connection.channel_mutex);
> +
> + if (hv_is_perf_channel(channel))
> + free_chn_counts(&hv_dev->chn_cnt);
Again, can drop the 'if' statement.
> kfree(hv_dev);
> }
>
> @@ -1745,38 +1748,8 @@ static ssize_t target_cpu_store(struct vmbus_channel *channel,
> if (target_cpu == origin_cpu)
> goto cpu_store_unlock;
>
> - if (vmbus_send_modifychannel(channel->offermsg.child_relid,
> - hv_cpu_number_to_vp_number(target_cpu))) {
> + if (!vmbus_modifychannel(channel, origin_cpu, target_cpu))
> ret = -EIO;
> - goto cpu_store_unlock;
> - }
> -
> - /*
> - * Warning. At this point, there is *no* guarantee that the host will
> - * have successfully processed the vmbus_send_modifychannel() request.
> - * See the header comment of vmbus_send_modifychannel() for more info.
> - *
> - * Lags in the processing of the above vmbus_send_modifychannel() can
> - * result in missed interrupts if the "old" target CPU is taken offline
> - * before Hyper-V starts sending interrupts to the "new" target CPU.
> - * But apart from this offlining scenario, the code tolerates such
> - * lags. It will function correctly even if a channel interrupt comes
> - * in on a CPU that is different from the channel target_cpu value.
> - */
> -
> - channel->target_cpu = target_cpu;
> - channel->target_vp = hv_cpu_number_to_vp_number(target_cpu);
> - channel->numa_node = cpu_to_node(target_cpu);
> -
> - /* See init_vp_index(). */
> - if (hv_is_perf_channel(channel))
> - hv_update_alloced_cpus(origin_cpu, target_cpu);
> -
> - /* Currently set only for storvsc channels. */
> - if (channel->change_target_cpu_callback) {
> - (*channel->change_target_cpu_callback)(channel,
> - origin_cpu, target_cpu);
> - }
>
> cpu_store_unlock:
> mutex_unlock(&vmbus_connection.channel_mutex);
> @@ -1972,6 +1945,15 @@ struct hv_device *vmbus_device_create(const guid_t *type,
> guid_copy(&child_device_obj->dev_instance, instance);
> child_device_obj->vendor_id = 0x1414; /* MSFT vendor ID */
>
> + if (!hv_is_perf_channel(channel))
> + return child_device_obj;
> +
> + if (alloc_chn_counts(&child_device_obj->chn_cnt)) {
> + free_chn_counts(&child_device_obj->chn_cnt);
> + kfree(child_device_obj);
> + return NULL;
> + }
> +
> return child_device_obj;
> }
>
> @@ -2616,7 +2598,6 @@ static void __exit vmbus_exit(void)
> hv_debug_rm_all_dir();
>
> vmbus_free_channels();
> - kfree(vmbus_connection.channels);
>
> if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
> kmsg_dump_unregister(&hv_kmsg_dumper);
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 40df3103e890b..0e9f695ea8f87 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1175,6 +1175,12 @@ struct hv_driver {
>
> };
>
> +/* For channel-interrupts balancing. */
> +struct vmbus_channel_counts {
> + unsigned int *cpu;
> + unsigned int *node;
> +};
> +
> /* Base device object */
> struct hv_device {
> /* the device type id of this device */
> @@ -1191,9 +1197,21 @@ struct hv_device {
> struct vmbus_channel *channel;
> struct kset *channels_kset;
>
> + /*
> + * Channel counts used by the channel-interrupts balancing scheme:
> + *
> + * - chn_cnt.cpu[cpu], the number of channel interrupts of this
> + * device assigned to CPU #cpu;
> + *
> + * - chn_cnt.node[node], the number of channel interrupts of this
> + * device assigned to NUMA node #node.
> + *
> + * The counts refer to "open" *and "performance" channels only.
> + */
> + struct vmbus_channel_counts chn_cnt;
> +
> /* place holder to keep track of the dir for hv device in debugfs */
> struct dentry *debug_dir;
> -
> };
>
>
> @@ -1523,6 +1541,8 @@ extern __u32 vmbus_proto_version;
> int vmbus_send_tl_connect_request(const guid_t *shv_guest_servie_id,
> const guid_t *shv_host_servie_id);
> int vmbus_send_modifychannel(u32 child_relid, u32 target_vp);
> +bool vmbus_modifychannel(struct vmbus_channel *channel,
> + u32 origin_cpu, u32 target_cpu);
> void vmbus_set_event(struct vmbus_channel *channel);
>
> /* Get the start of the ring buffer. */
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 2371292f30b03..6853b1d3e3ce0 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -328,6 +328,7 @@ void lockdep_assert_cpus_held(void)
>
> percpu_rwsem_assert_held(&cpu_hotplug_lock);
> }
> +EXPORT_SYMBOL_GPL(lockdep_assert_cpus_held);
>
> static void lockdep_acquire_cpus_lock(void)
> {
> --
> 2.25.1
^ permalink raw reply
* [GIT PULL] Hyper-V commits for 5.8
From: Wei Liu @ 2020-06-02 17:47 UTC (permalink / raw)
To: Linus Torvalds
Cc: Wei Liu, kys, sthemmin, haiyangz, Michael Kelley,
Linux on Hyper-V List, Linux Kernel List
Hi Linus
Please pull the signed tag for Hyper-V commits for 5.8.
There is a conflict in arch/x86/include/asm/hyperv-tlfs.h with a patch
in KVM tree. Michael Kelley and Jon Doron touched that file separately.
Michael's patches are going through the Hyper-V tree while Jon's patches
are going through KVM tree.
A fix for the conflict can be found at:
https://lore.kernel.org/lkml/20200602171802.560d07bc@canb.auug.org.au/
For the same reason, you will see a build failure after merging Hyper-V
and KVM tree. That's because a constant was renamed from
HV_X64_DEBUGGING to HV_DEBUGGING. A patch to fix the build can be found
at:
https://lore.kernel.org/lkml/20200602173556.17ad06a1@canb.auug.org.au/
The following changes since commit ae83d0b416db002fe95601e7f97f64b59514d936:
Linux 5.7-rc2 (2020-04-19 14:35:30 -0700)
are available in the Git repository at:
ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git tags/hyperv-next-signed
for you to fetch changes up to afaa33da08abd10be8978781d7c99a9e67d2bbff:
Drivers: hv: vmbus: Resolve more races involving init_vp_index() (2020-05-23 09:07:00 +0000)
----------------------------------------------------------------
- A series from Andrea to support channel reassignment
- A series from Vitaly to clean up Vmbus message handling
- A series from Michael to clean up and augment hyperv-tlfs.h
- Patches from Andy to clean up GUID usage in Hyper-V code
- A few other misc patches
----------------------------------------------------------------
Andrea Parri (Microsoft) (13):
Drivers: hv: vmbus: Always handle the VMBus messages on CPU0
Drivers: hv: vmbus: Don't bind the offer&rescind works to a specific CPU
Drivers: hv: vmbus: Replace the per-CPU channel lists with a global array of channels
hv_netvsc: Disable NAPI before closing the VMBus channel
hv_utils: Always execute the fcopy and vss callbacks in a tasklet
Drivers: hv: vmbus: Use a spin lock for synchronizing channel scheduling vs. channel removal
PCI: hv: Prepare hv_compose_msi_msg() for the VMBus-channel-interrupt-to-vCPU reassignment functionality
Drivers: hv: vmbus: Remove the unused HV_LOCALIZED channel affinity logic
Drivers: hv: vmbus: Synchronize init_vp_index() vs. CPU hotplug
Drivers: hv: vmbus: Introduce the CHANNELMSG_MODIFYCHANNEL message type
scsi: storvsc: Re-init stor_chns when a channel interrupt is re-assigned
Drivers: hv: vmbus: Resolve race between init_vp_index() and CPU hotplug
Drivers: hv: vmbus: Resolve more races involving init_vp_index()
Andy Shevchenko (4):
hyper-v: Use UUID API for exporting the GUID (part 2)
hyper-v: Supply GUID pointer to printf() like functions
hyper-v: Replace open-coded variant of %*phN specifier
hyper-v: Switch to use UUID types directly
Colin Ian King (1):
drivers: hv: remove redundant assignment to pointer primary_channel
Gustavo A. R. Silva (1):
vmbus: Replace zero-length array with flexible-array
Michael Kelley (4):
KVM: x86: hyperv: Remove duplicate definitions of Reference TSC Page
x86/hyperv: Remove HV_PROCESSOR_POWER_STATE #defines
x86/hyperv: Split hyperv-tlfs.h into arch dependent and independent files
asm-generic/hyperv: Add definitions for Get/SetVpRegister hypercalls
Vitaly Kuznetsov (5):
Drivers: hv: copy from message page only what's needed
Drivers: hv: allocate the exact needed memory for messages
Drivers: hv: avoid passing opaque pointer to vmbus_onmessage()
Drivers: hv: make sure that 'struct vmbus_channel_message_header' compiles correctly
Drivers: hv: check VMBus messages lengths
Wei Liu (1):
Driver: hv: vmbus: drop a no long applicable comment
MAINTAINERS | 1 +
arch/x86/include/asm/hyperv-tlfs.h | 472 +++-------------------------------
arch/x86/include/asm/kvm_host.h | 2 +-
arch/x86/kvm/hyperv.c | 4 +-
drivers/hv/channel.c | 58 +++--
drivers/hv/channel_mgmt.c | 439 +++++++++++++++++---------------
drivers/hv/connection.c | 58 +----
drivers/hv/hv.c | 16 +-
drivers/hv/hv_fcopy.c | 2 +-
drivers/hv/hv_snapshot.c | 2 +-
drivers/hv/hv_trace.h | 25 +-
drivers/hv/hyperv_vmbus.h | 81 ++++--
drivers/hv/vmbus_drv.c | 314 +++++++++++++++++------
drivers/net/hyperv/netvsc.c | 7 +-
drivers/pci/controller/pci-hyperv.c | 44 ++--
drivers/scsi/storvsc_drv.c | 96 ++++++-
include/asm-generic/hyperv-tlfs.h | 493 ++++++++++++++++++++++++++++++++++++
include/linux/hyperv.h | 68 +++--
include/linux/mod_devicetable.h | 2 +-
19 files changed, 1309 insertions(+), 875 deletions(-)
create mode 100644 include/asm-generic/hyperv-tlfs.h
^ permalink raw reply
* [PATCH v3] x86/apic/flat64: Add back the early_param("apic", parse_apic)
From: Dexuan Cui @ 2020-05-31 16:48 UTC (permalink / raw)
To: tglx, mingo, rdunlap, bp, hpa, x86, peterz, allison,
alexios.zavras, gregkh, decui, namit, mikelley, longli
Cc: linux-kernel, linux-hyperv
parse_apic() allows the user to try a different APIC driver than the
default one that's automatically chosen. It works for X86-32, but
doesn't work for X86-64 because it was removed in 2009 for X86-64 by
commit 7b38725318f4 ("x86: remove subarchitecture support code"),
whose changelog doesn't explicitly describe the removal for X86-64.
The patch adds back the functionality for X86-64. The intent is mainly
to work around an APIC emulation bug in Hyper-V in the case of kdump:
currently Hyper-V does not honor the disabled state of the local APICs,
so all the IOAPIC-based interrupts may not be delivered to the correct
virtual CPU, if the logical-mode APIC driver is used (the kdump
kernel usually uses the logical-mode APIC driver, since typically
only 1 CPU is active). Luckily the kdump issue can be worked around by
forcing the kdump kernel to use physical mode, before the fix to Hyper-V
becomes widely available.
The current algorithm of choosing an APIC driver is:
1. The global pointer "struct apic *apic" has a default value, i.e
"apic_default" on X86-32, and "apic_flat" on X86-64.
2. If the early_param "apic=" is specified, parse_apic() is called and
the pointer "apic" is changed if a matching APIC driver is found.
3. default_acpi_madt_oem_check() calls the acpi_madt_oem_check() method
of all APIC drivers, which may override the "apic" pointer.
4. default_setup_apic_routing() may override the "apic" pointer, e.g.
by calling the probe() method of all APIC drivers. Note: refer to the
order of the APIC drivers specified in arch/x86/kernel/apic/Makefile.
The patch is safe because if the apic= early param is not specified,
the current algorithm of choosing an APIC driver is unchanged; when the
param is specified (e.g. on X86-64, "apic=physical flat"), the kernel
still tries to find a "more suitable" APIC driver in the above step 3 and
4: e.g. if the BIOS/firmware requires that apic_x2apic_phys should be used,
the above step 4 will override the APIC driver to apic_x2apic_phys, even
if an early_param "apic=physical flat" is specified.
On Hyper-V, when a Linux VM has <= 8 virtual CPUs, if we use
"apic=physical flat", sending IPIs to multiple vCPUs is still fast because
Linux VM uses the para-virtualized IPI hypercalls: see hv_apic_init().
The patch adds the __init tag for flat_acpi_madt_oem_check() and
physflat_acpi_madt_oem_check() to avoid a warning seen with "make W=1":
flat_acpi_madt_oem_check() accesses cmdline_apic, which has a __initdata
tag.
Fixes: 7b38725318f4 ("x86: remove subarchitecture support code")
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
Changes in v2:
Updated Documentation/admin-guide/kernel-parameters.txt. [Randy Dunlap]
Changed apic_set_verbosity().
Enhanced the changelog.
Changes in v3:
Added the __init tag for flat_acpi_madt_oem_check() and
physflat_acpi_madt_oem_check() to avoid a warning seen with "make W=1".
(Thanks to kbuild test robot <lkp@intel.com>).
Updated the changelog for the __init tag.
.../admin-guide/kernel-parameters.txt | 11 +++++--
arch/x86/kernel/apic/apic.c | 11 +++----
arch/x86/kernel/apic/apic_flat_64.c | 31 +++++++++++++++++--
3 files changed, 40 insertions(+), 13 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 7bc83f3d9bdf..c4503fff9348 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -341,10 +341,15 @@
Format: { quiet (default) | verbose | debug }
Change the amount of debugging information output
when initialising the APIC and IO-APIC components.
- For X86-32, this can also be used to specify an APIC
- driver name.
+ This can also be used to specify an APIC driver name.
Format: apic=driver_name
- Examples: apic=bigsmp
+ Examples:
+ On X86-32: apic=bigsmp
+ On X86-64: "apic=physical flat"
+ Note: the available driver names depend on the
+ architecture and the kernel config; the setting may
+ be overridden by the acpi_madt_oem_check() and probe()
+ methods of other APIC drivers.
apic_extnmi= [APIC,X86] External NMI delivery setting
Format: { bsp (default) | all | none }
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index e53dda210cd7..6f7d75b6358b 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2855,13 +2855,10 @@ static int __init apic_set_verbosity(char *arg)
apic_verbosity = APIC_DEBUG;
else if (strcmp("verbose", arg) == 0)
apic_verbosity = APIC_VERBOSE;
-#ifdef CONFIG_X86_64
- else {
- pr_warn("APIC Verbosity level %s not recognised"
- " use apic=verbose or apic=debug\n", arg);
- return -EINVAL;
- }
-#endif
+
+ /* Ignore unrecognized verbosity level setting. */
+
+ pr_info("APIC Verbosity level is %d\n", apic_verbosity);
return 0;
}
diff --git a/arch/x86/kernel/apic/apic_flat_64.c b/arch/x86/kernel/apic/apic_flat_64.c
index 7862b152a052..da8f3640453f 100644
--- a/arch/x86/kernel/apic/apic_flat_64.c
+++ b/arch/x86/kernel/apic/apic_flat_64.c
@@ -23,9 +23,34 @@ static struct apic apic_flat;
struct apic *apic __ro_after_init = &apic_flat;
EXPORT_SYMBOL_GPL(apic);
-static int flat_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
+static int cmdline_apic __initdata;
+static int __init parse_apic(char *arg)
{
- return 1;
+ struct apic **drv;
+
+ if (!arg)
+ return -EINVAL;
+
+ for (drv = __apicdrivers; drv < __apicdrivers_end; drv++) {
+ if (!strcmp((*drv)->name, arg)) {
+ apic = *drv;
+ cmdline_apic = 1;
+ return 0;
+ }
+ }
+
+ /* Parsed again by __setup for debug/verbose */
+ return 0;
+}
+early_param("apic", parse_apic);
+
+
+static int __init flat_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
+{
+ if (!cmdline_apic)
+ return 1;
+
+ return apic == &apic_flat;
}
/*
@@ -157,7 +182,7 @@ static struct apic apic_flat __ro_after_init = {
* We cannot use logical delivery in this case because the mask
* overflows, so use physical mode.
*/
-static int physflat_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
+static int __init physflat_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
{
#ifdef CONFIG_ACPI
/*
--
2.19.1
^ permalink raw reply related
* RE: [PATCH] x86/apic/flat64: Add back the early_param("apic", parse_apic)
From: Dexuan Cui @ 2020-05-30 22:40 UTC (permalink / raw)
To: Randy Dunlap, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
hpa@zytor.com, x86@kernel.org, peterz@infradead.org,
allison@lohutok.net, alexios.zavras@intel.com,
gregkh@linuxfoundation.org, namit@vmware.com, Michael Kelley,
Long Li
Cc: linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org
In-Reply-To: <928eb231-242c-d2b1-d40b-a2892c55b415@infradead.org>
> From: Randy Dunlap <rdunlap@infradead.org>
> Sent: Friday, May 29, 2020 6:33 AM
> Hi,
> Looks like you will also need to update
> Documentation/admin-guide/kernel-parameters.txt, where it says:
>
> For X86-32, this can also be used to specify an APIC
> driver name.
> --
> ~Randy
Hi Randy,
Thanks for you reminder! I just posted a v2 for this with you Cc'd.
Thanks,
-- Dexuan
^ permalink raw reply
* [PATCH v2] x86/apic/flat64: Add back the early_param("apic", parse_apic)
From: Dexuan Cui @ 2020-05-30 22:38 UTC (permalink / raw)
To: tglx, mingo, rdunlap, bp, hpa, x86, peterz, allison,
alexios.zavras, gregkh, decui, namit, mikelley, longli
Cc: linux-kernel, linux-hyperv
parse_apic() allows the user to try a different APIC driver than the
default one that's automatically chosen. It works for X86-32, but
doesn't work for X86-64 because it was removed in 2009 for X86-64 by
commit 7b38725318f4 ("x86: remove subarchitecture support code"),
whose changelog doesn't explicitly describe the removal for X86-64.
The patch adds back the functionality for X86-64. The intent is mainly
to work around an APIC emulation bug in Hyper-V in the case of kdump:
currently Hyper-V does not honor the disabled state of the local APICs,
so all the IOAPIC-based interrupts may not be delivered to the correct
virtual CPU, if the logical-mode APIC driver is used (the kdump
kernel usually uses the logical-mode APIC driver, since typically
only 1 CPU is active). Luckily the kdump issue can be worked around by
forcing the kdump kernel to use physical mode, before the fix to Hyper-V
becomes widely available.
The current algorithm of choosing an APIC driver is:
1. The global pointer "struct apic *apic" has a default value, i.e
"apic_default" on X86-32, and "apic_flat" on X86-64.
2. If the early_param "apic=" is specified, parse_apic() is called and
the pointer "apic" is changed if a matching APIC driver is found.
3. default_acpi_madt_oem_check() calls the acpi_madt_oem_check() method
of all APIC drivers, which may override the "apic" pointer.
4. default_setup_apic_routing() may override the "apic" pointer, e.g.
by calling the probe() method of all APIC drivers. Note: refer to the
order of the APIC drivers specified in arch/x86/kernel/apic/Makefile.
The patch is safe because if the apic= early param is not specified,
the current algorithm of choosing an APIC driver is unchanged; when the
param is specified (e.g. on X86-64, "apic=physical flat"), the kernel
still tries to find a "more suitable" APIC driver in the above step 3 and
4: e.g. if the BIOS/firmware requires that apic_x2apic_phys should be used,
the above step 4 will override the APIC driver to apic_x2apic_phys, even
if an early_param "apic=physical flat" is specified.
On Hyper-V, when a Linux VM has <= 8 virtual CPUs, if we use
"apic=physical flat", sending IPIs to multiple vCPUs is still fast because
Linux VM uses the para-virtualized IPI hypercalls: see hv_apic_init().
Fixes: 7b38725318f4 ("x86: remove subarchitecture support code")
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
Changes in v2:
Updated Documentation/admin-guide/kernel-parameters.txt. [Randy Dunlap]
Changed apic_set_verbosity().
Enhanced the changelog.
.../admin-guide/kernel-parameters.txt | 11 +++++---
arch/x86/kernel/apic/apic.c | 11 +++-----
arch/x86/kernel/apic/apic_flat_64.c | 27 ++++++++++++++++++-
3 files changed, 38 insertions(+), 11 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 7bc83f3d9bdf..c4503fff9348 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -341,10 +341,15 @@
Format: { quiet (default) | verbose | debug }
Change the amount of debugging information output
when initialising the APIC and IO-APIC components.
- For X86-32, this can also be used to specify an APIC
- driver name.
+ This can also be used to specify an APIC driver name.
Format: apic=driver_name
- Examples: apic=bigsmp
+ Examples:
+ On X86-32: apic=bigsmp
+ On X86-64: "apic=physical flat"
+ Note: the available driver names depend on the
+ architecture and the kernel config; the setting may
+ be overridden by the acpi_madt_oem_check() and probe()
+ methods of other APIC drivers.
apic_extnmi= [APIC,X86] External NMI delivery setting
Format: { bsp (default) | all | none }
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 81b9c63dae1b..ee2363b3c59e 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2854,13 +2854,10 @@ static int __init apic_set_verbosity(char *arg)
apic_verbosity = APIC_DEBUG;
else if (strcmp("verbose", arg) == 0)
apic_verbosity = APIC_VERBOSE;
-#ifdef CONFIG_X86_64
- else {
- pr_warn("APIC Verbosity level %s not recognised"
- " use apic=verbose or apic=debug\n", arg);
- return -EINVAL;
- }
-#endif
+
+ /* Ignore unrecognized verbosity level setting. */
+
+ pr_info("APIC Verbosity level is %d\n", apic_verbosity);
return 0;
}
diff --git a/arch/x86/kernel/apic/apic_flat_64.c b/arch/x86/kernel/apic/apic_flat_64.c
index 7862b152a052..efbec63bb01f 100644
--- a/arch/x86/kernel/apic/apic_flat_64.c
+++ b/arch/x86/kernel/apic/apic_flat_64.c
@@ -23,9 +23,34 @@ static struct apic apic_flat;
struct apic *apic __ro_after_init = &apic_flat;
EXPORT_SYMBOL_GPL(apic);
+static int cmdline_apic __initdata;
+static int __init parse_apic(char *arg)
+{
+ struct apic **drv;
+
+ if (!arg)
+ return -EINVAL;
+
+ for (drv = __apicdrivers; drv < __apicdrivers_end; drv++) {
+ if (!strcmp((*drv)->name, arg)) {
+ apic = *drv;
+ cmdline_apic = 1;
+ return 0;
+ }
+ }
+
+ /* Parsed again by __setup for debug/verbose */
+ return 0;
+}
+early_param("apic", parse_apic);
+
+
static int flat_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
{
- return 1;
+ if (!cmdline_apic)
+ return 1;
+
+ return apic == &apic_flat;
}
/*
--
2.19.1
^ permalink raw reply related
* Re: [PATCH v12 0/6] x86/kvm/hyper-v: add support for synthetic
From: Paolo Bonzini @ 2020-05-29 15:15 UTC (permalink / raw)
To: Jon Doron, kvm, linux-hyperv; +Cc: vkuznets, rvkagan
In-Reply-To: <20200529134543.1127440-1-arilou@gmail.com>
On 29/05/20 15:45, Jon Doron wrote:
> Add support for the synthetic debugger interface of hyper-v, the synthetic
> debugger has 2 modes.
> 1. Use a set of MSRs to send/recv information (undocumented so it's not
> going to the hyperv-tlfs.h)
> 2. Use hypercalls
>
> The first mode is based the following MSRs:
> 1. Control/Status MSRs which either asks for a send/recv .
> 2. Send/Recv MSRs each holds GPA where the send/recv buffers are.
> 3. Pending MSR, holds a GPA to a PAGE that simply has a boolean that
> indicates if there is data pending to issue a recv VMEXIT.
>
> The first mode implementation is to simply exit to user-space when
> either the control MSR or the pending MSR are being set.
> Then it's up-to userspace to implement the rest of the logic of sending/recving.
>
> In the second mode instead of using MSRs KNet will simply issue
> Hypercalls with the information to send/recv, in this mode the data
> being transferred is UDP encapsulated, unlike in the previous mode in
> which you get just the data to send.
>
> The new hypercalls will exit to userspace which will be incharge of
> re-encapsulating if needed the UDP packets to be sent.
>
> There is an issue though in which KDNet does not respect the hypercall
> page and simply issues vmcall/vmmcall instructions depending on the cpu
> type expecting them to be handled as it a real hypercall was issued.
>
> It's important to note that part of this feature has been subject to be
> removed in future versions of Windows, which is why some of the
> defintions will not be present the the TLFS but in the kvm hyperv header
> instead.
>
> v12:
> - Rebased on latest origin/master
> - Make the KVM_CAP_HYPERV_SYNDBG always enabled, in previous version
> userspace was required to explicitly enable the syndbg capability just
> like with the EVMCS feature.
I removed the capability altogether; the CPUID interface was added
exactly to avoid a proliferation of capabilities.
Otherwise it's great; queued, thanks.
Paolo
^ permalink raw reply
* [PATCH v12 6/6] KVM: selftests: update hyperv_cpuid with SynDBG tests
From: Jon Doron @ 2020-05-29 13:45 UTC (permalink / raw)
To: kvm, linux-hyperv; +Cc: vkuznets, pbonzini, rvkagan, Jon Doron
In-Reply-To: <20200529134543.1127440-1-arilou@gmail.com>
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Update tests to reflect new CPUID capabilities with SYNDBG.
Check that we get the right number of entries and that
0x40000000.EAX always returns the correct max leaf.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Jon Doron <arilou@gmail.com>
---
.../selftests/kvm/x86_64/hyperv_cpuid.c | 103 ++++++++++--------
1 file changed, 56 insertions(+), 47 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
index 83323f3d7ca0..4a7967cca281 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
@@ -26,18 +26,18 @@ static void guest_code(void)
{
}
-static int smt_possible(void)
+static bool smt_possible(void)
{
char buf[16];
FILE *f;
- bool res = 1;
+ bool res = true;
f = fopen("/sys/devices/system/cpu/smt/control", "r");
if (f) {
if (fread(buf, sizeof(*buf), sizeof(buf), f) > 0) {
if (!strncmp(buf, "forceoff", 8) ||
!strncmp(buf, "notsupported", 12))
- res = 0;
+ res = false;
}
fclose(f);
}
@@ -46,29 +46,31 @@ static int smt_possible(void)
}
static void test_hv_cpuid(struct kvm_cpuid2 *hv_cpuid_entries,
- int evmcs_enabled)
+ bool evmcs_enabled)
{
int i;
+ int nent = 9;
+ u32 test_val;
- if (!evmcs_enabled)
- TEST_ASSERT(hv_cpuid_entries->nent == 6,
- "KVM_GET_SUPPORTED_HV_CPUID should return 6 entries"
- " when Enlightened VMCS is disabled (returned %d)",
- hv_cpuid_entries->nent);
- else
- TEST_ASSERT(hv_cpuid_entries->nent == 7,
- "KVM_GET_SUPPORTED_HV_CPUID should return 7 entries"
- " when Enlightened VMCS is enabled (returned %d)",
- hv_cpuid_entries->nent);
+ if (evmcs_enabled)
+ nent += 1; /* 0x4000000A */
+
+ TEST_ASSERT(hv_cpuid_entries->nent == nent,
+ "KVM_GET_SUPPORTED_HV_CPUID should return %d entries"
+ " with evmcs=%d (returned %d)",
+ nent, evmcs_enabled, hv_cpuid_entries->nent);
for (i = 0; i < hv_cpuid_entries->nent; i++) {
struct kvm_cpuid_entry2 *entry = &hv_cpuid_entries->entries[i];
TEST_ASSERT((entry->function >= 0x40000000) &&
- (entry->function <= 0x4000000A),
+ (entry->function <= 0x40000082),
"function %x is our of supported range",
entry->function);
+ TEST_ASSERT(evmcs_enabled || (entry->function != 0x4000000A),
+ "0x4000000A leaf should not be reported");
+
TEST_ASSERT(entry->index == 0,
".index field should be zero");
@@ -78,12 +80,23 @@ static void test_hv_cpuid(struct kvm_cpuid2 *hv_cpuid_entries,
TEST_ASSERT(!entry->padding[0] && !entry->padding[1] &&
!entry->padding[2], "padding should be zero");
- if (entry->function == 0x40000004) {
- int nononarchcs = !!(entry->eax & (1UL << 18));
+ switch (entry->function) {
+ case 0x40000000:
+ test_val = 0x40000082;
- TEST_ASSERT(nononarchcs == !smt_possible(),
+ TEST_ASSERT(entry->eax == test_val,
+ "Wrong max leaf report in 0x40000000.EAX: %x"
+ " (evmcs=%d)",
+ entry->eax, evmcs_enabled
+ );
+ break;
+ case 0x40000004:
+ test_val = entry->eax & (1UL << 18);
+
+ TEST_ASSERT(!!test_val == !smt_possible(),
"NoNonArchitecturalCoreSharing bit"
" doesn't reflect SMT setting");
+ break;
}
/*
@@ -133,8 +146,9 @@ struct kvm_cpuid2 *kvm_get_supported_hv_cpuid(struct kvm_vm *vm)
int main(int argc, char *argv[])
{
struct kvm_vm *vm;
- int rv;
+ int rv, stage;
struct kvm_cpuid2 *hv_cpuid_entries;
+ bool evmcs_enabled;
/* Tell stdout not to buffer its content */
setbuf(stdout, NULL);
@@ -145,36 +159,31 @@ int main(int argc, char *argv[])
exit(KSFT_SKIP);
}
- /* Create VM */
- vm = vm_create_default(VCPU_ID, 0, guest_code);
-
- test_hv_cpuid_e2big(vm);
-
- hv_cpuid_entries = kvm_get_supported_hv_cpuid(vm);
- if (!hv_cpuid_entries)
- return 1;
-
- test_hv_cpuid(hv_cpuid_entries, 0);
-
- free(hv_cpuid_entries);
+ for (stage = 0; stage < 3; stage++) {
+ evmcs_enabled = false;
+
+ vm = vm_create_default(VCPU_ID, 0, guest_code);
+ switch (stage) {
+ case 0:
+ test_hv_cpuid_e2big(vm);
+ continue;
+ case 1:
+ break;
+ case 2:
+ if (!kvm_check_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS)) {
+ print_skip("Enlightened VMCS is unsupported");
+ continue;
+ }
+ vcpu_enable_evmcs(vm, VCPU_ID);
+ evmcs_enabled = true;
+ break;
+ }
- if (!kvm_check_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS)) {
- print_skip("Enlightened VMCS is unsupported");
- goto vm_free;
+ hv_cpuid_entries = kvm_get_supported_hv_cpuid(vm);
+ test_hv_cpuid(hv_cpuid_entries, evmcs_enabled);
+ free(hv_cpuid_entries);
+ kvm_vm_free(vm);
}
- vcpu_enable_evmcs(vm, VCPU_ID);
-
- hv_cpuid_entries = kvm_get_supported_hv_cpuid(vm);
- if (!hv_cpuid_entries)
- return 1;
-
- test_hv_cpuid(hv_cpuid_entries, 1);
-
- free(hv_cpuid_entries);
-
-vm_free:
- kvm_vm_free(vm);
-
return 0;
}
--
2.24.1
^ permalink raw reply related
* [PATCH v12 4/6] x86/kvm/hyper-v: enable hypercalls regardless of hypercall page
From: Jon Doron @ 2020-05-29 13:45 UTC (permalink / raw)
To: kvm, linux-hyperv; +Cc: vkuznets, pbonzini, rvkagan, Jon Doron
In-Reply-To: <20200529134543.1127440-1-arilou@gmail.com>
Microsoft's kdvm.dll dbgtransport module does not respect the hypercall
page and simply identifies the CPU being used (AMD/Intel) and according
to it simply makes hypercalls with the relevant instruction
(vmmcall/vmcall respectively).
The relevant function in kdvm is KdHvConnectHypervisor which first checks
if the hypercall page has been enabled via HV_X64_MSR_HYPERCALL_ENABLE,
and in case it was not it simply sets the HV_X64_MSR_GUEST_OS_ID to
0x1000101010001 which means:
build_number = 0x0001
service_version = 0x01
minor_version = 0x01
major_version = 0x01
os_id = 0x00 (Undefined)
vendor_id = 1 (Microsoft)
os_type = 0 (A value of 0 indicates a proprietary, closed source OS)
and starts issuing the hypercall without setting the hypercall page.
To resolve this issue simply enable hypercalls also if the guest_os_id
is not 0.
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Jon Doron <arilou@gmail.com>
---
arch/x86/kvm/hyperv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index dc7dd48621ca..3730c38f4f71 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1657,7 +1657,7 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *current_vcpu, u64 ingpa, u64 outgpa,
bool kvm_hv_hypercall_enabled(struct kvm *kvm)
{
- return READ_ONCE(kvm->arch.hyperv.hv_hypercall) & HV_X64_MSR_HYPERCALL_ENABLE;
+ return READ_ONCE(kvm->arch.hyperv.hv_guest_os_id) != 0;
}
static void kvm_hv_hypercall_set_result(struct kvm_vcpu *vcpu, u64 result)
--
2.24.1
^ permalink raw reply related
* [PATCH v12 5/6] x86/kvm/hyper-v: Add support for synthetic debugger via hypercalls
From: Jon Doron @ 2020-05-29 13:45 UTC (permalink / raw)
To: kvm, linux-hyperv; +Cc: vkuznets, pbonzini, rvkagan, Jon Doron
In-Reply-To: <20200529134543.1127440-1-arilou@gmail.com>
There is another mode for the synthetic debugger which uses hypercalls
to send/recv network data instead of the MSR interface.
This interface is much slower and less recommended since you might get
a lot of VMExits while KDVM polling for new packets to recv, rather
than simply checking the pending page to see if there is data avialble
and then request.
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Jon Doron <arilou@gmail.com>
---
arch/x86/kvm/hyperv.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 3730c38f4f71..640fd98583b1 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1836,6 +1836,34 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
}
ret = kvm_hv_send_ipi(vcpu, ingpa, outgpa, true, false);
break;
+ case HVCALL_POST_DEBUG_DATA:
+ case HVCALL_RETRIEVE_DEBUG_DATA:
+ if (unlikely(fast)) {
+ ret = HV_STATUS_INVALID_PARAMETER;
+ break;
+ }
+ fallthrough;
+ case HVCALL_RESET_DEBUG_SESSION: {
+ struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
+
+ if (!kvm_hv_is_syndbg_enabled(vcpu)) {
+ ret = HV_STATUS_INVALID_HYPERCALL_CODE;
+ break;
+ }
+
+ if (!(syndbg->options & HV_X64_SYNDBG_OPTION_USE_HCALLS)) {
+ ret = HV_STATUS_OPERATION_DENIED;
+ break;
+ }
+ vcpu->run->exit_reason = KVM_EXIT_HYPERV;
+ vcpu->run->hyperv.type = KVM_EXIT_HYPERV_HCALL;
+ vcpu->run->hyperv.u.hcall.input = param;
+ vcpu->run->hyperv.u.hcall.params[0] = ingpa;
+ vcpu->run->hyperv.u.hcall.params[1] = outgpa;
+ vcpu->arch.complete_userspace_io =
+ kvm_hv_hypercall_complete_userspace;
+ return 0;
+ }
default:
ret = HV_STATUS_INVALID_HYPERCALL_CODE;
break;
--
2.24.1
^ permalink raw reply related
* [PATCH v12 3/6] x86/kvm/hyper-v: Add support for synthetic debugger capability
From: Jon Doron @ 2020-05-29 13:45 UTC (permalink / raw)
To: kvm, linux-hyperv; +Cc: vkuznets, pbonzini, rvkagan, Jon Doron
In-Reply-To: <20200529134543.1127440-1-arilou@gmail.com>
Add support for Hyper-V synthetic debugger (syndbg) interface.
The syndbg interface is using MSRs to emulate a way to send/recv packets
data.
The debug transport dll (kdvm/kdnet) will identify if Hyper-V is enabled
and if it supports the synthetic debugger interface it will attempt to
use it, instead of trying to initialize a network adapter.
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Jon Doron <arilou@gmail.com>
---
Documentation/virt/kvm/api.rst | 16 ++++
arch/x86/include/asm/kvm_host.h | 13 +++
arch/x86/kvm/hyperv.c | 158 +++++++++++++++++++++++++++++++-
arch/x86/kvm/hyperv.h | 5 +
arch/x86/kvm/trace.h | 51 +++++++++++
arch/x86/kvm/x86.c | 9 ++
include/uapi/linux/kvm.h | 11 +++
7 files changed, 260 insertions(+), 3 deletions(-)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 750d005a75bc..52ba12758f7c 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -5066,6 +5066,7 @@ EOI was received.
struct kvm_hyperv_exit {
#define KVM_EXIT_HYPERV_SYNIC 1
#define KVM_EXIT_HYPERV_HCALL 2
+ #define KVM_EXIT_HYPERV_SYNDBG 3
__u32 type;
__u32 pad1;
union {
@@ -5081,6 +5082,15 @@ EOI was received.
__u64 result;
__u64 params[2];
} hcall;
+ struct {
+ __u32 msr;
+ __u32 pad2;
+ __u64 control;
+ __u64 status;
+ __u64 send_page;
+ __u64 recv_page;
+ __u64 pending_page;
+ } syndbg;
} u;
};
/* KVM_EXIT_HYPERV */
@@ -5097,6 +5107,12 @@ Hyper-V SynIC state change. Notification is used to remap SynIC
event/message pages and to enable/disable SynIC messages/events processing
in userspace.
+ - KVM_EXIT_HYPERV_SYNDBG -- synchronously notify user-space about
+
+Hyper-V Synthetic debugger state change. Notification is used to either update
+the pending_page location or to send a control command (send the buffer located
+in send_page or recv a buffer to recv_page).
+
::
/* KVM_EXIT_ARM_NISV */
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 0a6b35353fc7..cf13a1b0c680 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -855,6 +855,18 @@ struct kvm_apic_map {
struct kvm_lapic *phys_map[];
};
+/* Hyper-V synthetic debugger (SynDbg)*/
+struct kvm_hv_syndbg {
+ struct {
+ u64 control;
+ u64 status;
+ u64 send_page;
+ u64 recv_page;
+ u64 pending_page;
+ } control;
+ u64 options;
+};
+
/* Hyper-V emulation context */
struct kvm_hv {
struct mutex hv_lock;
@@ -878,6 +890,7 @@ struct kvm_hv {
atomic_t num_mismatched_vp_indexes;
struct hv_partition_assist_pg *hv_pa_pg;
+ struct kvm_hv_syndbg hv_syndbg;
};
enum kvm_irqchip_mode {
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 54d4b98b49e1..dc7dd48621ca 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -21,6 +21,7 @@
#include "x86.h"
#include "lapic.h"
#include "ioapic.h"
+#include "cpuid.h"
#include "hyperv.h"
#include <linux/cpu.h>
@@ -266,6 +267,123 @@ static int synic_set_msr(struct kvm_vcpu_hv_synic *synic,
return ret;
}
+static bool kvm_hv_is_syndbg_enabled(struct kvm_vcpu *vcpu)
+{
+ struct kvm_cpuid_entry2 *entry;
+
+ entry = kvm_find_cpuid_entry(vcpu,
+ HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES,
+ 0);
+ if (!entry)
+ return false;
+
+ return entry->eax & HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING;
+}
+
+static int kvm_hv_syndbg_complete_userspace(struct kvm_vcpu *vcpu)
+{
+ struct kvm *kvm = vcpu->kvm;
+ struct kvm_hv *hv = &kvm->arch.hyperv;
+
+ if (vcpu->run->hyperv.u.syndbg.msr == HV_X64_MSR_SYNDBG_CONTROL)
+ hv->hv_syndbg.control.status =
+ vcpu->run->hyperv.u.syndbg.status;
+ return 1;
+}
+
+static void syndbg_exit(struct kvm_vcpu *vcpu, u32 msr)
+{
+ struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
+ struct kvm_vcpu_hv *hv_vcpu = &vcpu->arch.hyperv;
+
+ hv_vcpu->exit.type = KVM_EXIT_HYPERV_SYNDBG;
+ hv_vcpu->exit.u.syndbg.msr = msr;
+ hv_vcpu->exit.u.syndbg.control = syndbg->control.control;
+ hv_vcpu->exit.u.syndbg.send_page = syndbg->control.send_page;
+ hv_vcpu->exit.u.syndbg.recv_page = syndbg->control.recv_page;
+ hv_vcpu->exit.u.syndbg.pending_page = syndbg->control.pending_page;
+ vcpu->arch.complete_userspace_io =
+ kvm_hv_syndbg_complete_userspace;
+
+ kvm_make_request(KVM_REQ_HV_EXIT, vcpu);
+}
+
+static int syndbg_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host)
+{
+ struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
+
+ if (!kvm_hv_is_syndbg_enabled(vcpu) && !host)
+ return 1;
+
+ trace_kvm_hv_syndbg_set_msr(vcpu->vcpu_id,
+ vcpu_to_hv_vcpu(vcpu)->vp_index, msr, data);
+ switch (msr) {
+ case HV_X64_MSR_SYNDBG_CONTROL:
+ syndbg->control.control = data;
+ if (!host)
+ syndbg_exit(vcpu, msr);
+ break;
+ case HV_X64_MSR_SYNDBG_STATUS:
+ syndbg->control.status = data;
+ break;
+ case HV_X64_MSR_SYNDBG_SEND_BUFFER:
+ syndbg->control.send_page = data;
+ break;
+ case HV_X64_MSR_SYNDBG_RECV_BUFFER:
+ syndbg->control.recv_page = data;
+ break;
+ case HV_X64_MSR_SYNDBG_PENDING_BUFFER:
+ syndbg->control.pending_page = data;
+ if (!host)
+ syndbg_exit(vcpu, msr);
+ break;
+ case HV_X64_MSR_SYNDBG_OPTIONS:
+ syndbg->options = data;
+ break;
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static int syndbg_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host)
+{
+ struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
+
+ if (!kvm_hv_is_syndbg_enabled(vcpu) && !host)
+ return 1;
+
+ switch (msr) {
+ case HV_X64_MSR_SYNDBG_CONTROL:
+ *pdata = syndbg->control.control;
+ break;
+ case HV_X64_MSR_SYNDBG_STATUS:
+ *pdata = syndbg->control.status;
+ break;
+ case HV_X64_MSR_SYNDBG_SEND_BUFFER:
+ *pdata = syndbg->control.send_page;
+ break;
+ case HV_X64_MSR_SYNDBG_RECV_BUFFER:
+ *pdata = syndbg->control.recv_page;
+ break;
+ case HV_X64_MSR_SYNDBG_PENDING_BUFFER:
+ *pdata = syndbg->control.pending_page;
+ break;
+ case HV_X64_MSR_SYNDBG_OPTIONS:
+ *pdata = syndbg->options;
+ break;
+ default:
+ break;
+ }
+
+ trace_kvm_hv_syndbg_get_msr(vcpu->vcpu_id,
+ vcpu_to_hv_vcpu(vcpu)->vp_index, msr,
+ *pdata);
+
+ return 0;
+}
+
static int synic_get_msr(struct kvm_vcpu_hv_synic *synic, u32 msr, u64 *pdata,
bool host)
{
@@ -800,6 +918,8 @@ static bool kvm_hv_msr_partition_wide(u32 msr)
case HV_X64_MSR_REENLIGHTENMENT_CONTROL:
case HV_X64_MSR_TSC_EMULATION_CONTROL:
case HV_X64_MSR_TSC_EMULATION_STATUS:
+ case HV_X64_MSR_SYNDBG_OPTIONS:
+ case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
r = true;
break;
}
@@ -1061,6 +1181,9 @@ static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data,
if (!host)
return 1;
break;
+ case HV_X64_MSR_SYNDBG_OPTIONS:
+ case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
+ return syndbg_set_msr(vcpu, msr, data, host);
default:
vcpu_unimpl(vcpu, "Hyper-V unhandled wrmsr: 0x%x data 0x%llx\n",
msr, data);
@@ -1190,7 +1313,8 @@ static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host)
return 0;
}
-static int kvm_hv_get_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
+static int kvm_hv_get_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata,
+ bool host)
{
u64 data = 0;
struct kvm *kvm = vcpu->kvm;
@@ -1227,6 +1351,9 @@ static int kvm_hv_get_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
case HV_X64_MSR_TSC_EMULATION_STATUS:
data = hv->hv_tsc_emulation_status;
break;
+ case HV_X64_MSR_SYNDBG_OPTIONS:
+ case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
+ return syndbg_get_msr(vcpu, msr, pdata, host);
default:
vcpu_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr);
return 1;
@@ -1316,7 +1443,7 @@ int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host)
int r;
mutex_lock(&vcpu->kvm->arch.hyperv.hv_lock);
- r = kvm_hv_get_msr_pw(vcpu, msr, pdata);
+ r = kvm_hv_get_msr_pw(vcpu, msr, pdata, host);
mutex_unlock(&vcpu->kvm->arch.hyperv.hv_lock);
return r;
} else
@@ -1796,6 +1923,9 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
{ .function = HYPERV_CPUID_FEATURES },
{ .function = HYPERV_CPUID_ENLIGHTMENT_INFO },
{ .function = HYPERV_CPUID_IMPLEMENT_LIMITS },
+ { .function = HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS },
+ { .function = HYPERV_CPUID_SYNDBG_INTERFACE },
+ { .function = HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES },
{ .function = HYPERV_CPUID_NESTED_FEATURES },
};
int i, nent = ARRAY_SIZE(cpuid_entries);
@@ -1821,7 +1951,7 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
case HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS:
memcpy(signature, "Linux KVM Hv", 12);
- ent->eax = HYPERV_CPUID_NESTED_FEATURES;
+ ent->eax = HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES;
ent->ebx = signature[0];
ent->ecx = signature[1];
ent->edx = signature[2];
@@ -1860,6 +1990,10 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
ent->edx |= HV_FEATURE_FREQUENCY_MSRS_AVAILABLE;
ent->edx |= HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE;
+ ent->ebx |= HV_X64_DEBUGGING;
+ ent->edx |= HV_X64_GUEST_DEBUGGING_AVAILABLE;
+ ent->edx |= HV_FEATURE_DEBUG_MSRS_AVAILABLE;
+
/*
* Direct Synthetic timers only make sense with in-kernel
* LAPIC
@@ -1903,6 +2037,24 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
break;
+ case HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS:
+ memcpy(signature, "Linux KVM Hv", 12);
+
+ ent->eax = 0;
+ ent->ebx = signature[0];
+ ent->ecx = signature[1];
+ ent->edx = signature[2];
+ break;
+
+ case HYPERV_CPUID_SYNDBG_INTERFACE:
+ memcpy(signature, "VS#1\0\0\0\0\0\0\0\0", 12);
+ ent->eax = signature[0];
+ break;
+
+ case HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES:
+ ent->eax |= HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING;
+ break;
+
default:
break;
}
diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
index 7f50ff0bad07..e68c6c2e9649 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -73,6 +73,11 @@ static inline struct kvm_vcpu *synic_to_vcpu(struct kvm_vcpu_hv_synic *synic)
return hv_vcpu_to_vcpu(container_of(synic, struct kvm_vcpu_hv, synic));
}
+static inline struct kvm_hv_syndbg *vcpu_to_hv_syndbg(struct kvm_vcpu *vcpu)
+{
+ return &vcpu->kvm->arch.hyperv.hv_syndbg;
+}
+
int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host);
int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host);
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index 249062f24b94..df95c45ec3bb 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -1539,6 +1539,57 @@ TRACE_EVENT(kvm_nested_vmenter_failed,
__print_symbolic(__entry->err, VMX_VMENTER_INSTRUCTION_ERRORS))
);
+/*
+ * Tracepoint for syndbg_set_msr.
+ */
+TRACE_EVENT(kvm_hv_syndbg_set_msr,
+ TP_PROTO(int vcpu_id, u32 vp_index, u32 msr, u64 data),
+ TP_ARGS(vcpu_id, vp_index, msr, data),
+
+ TP_STRUCT__entry(
+ __field(int, vcpu_id)
+ __field(u32, vp_index)
+ __field(u32, msr)
+ __field(u64, data)
+ ),
+
+ TP_fast_assign(
+ __entry->vcpu_id = vcpu_id;
+ __entry->vp_index = vp_index;
+ __entry->msr = msr;
+ __entry->data = data;
+ ),
+
+ TP_printk("vcpu_id %d vp_index %u msr 0x%x data 0x%llx",
+ __entry->vcpu_id, __entry->vp_index, __entry->msr,
+ __entry->data)
+);
+
+/*
+ * Tracepoint for syndbg_get_msr.
+ */
+TRACE_EVENT(kvm_hv_syndbg_get_msr,
+ TP_PROTO(int vcpu_id, u32 vp_index, u32 msr, u64 data),
+ TP_ARGS(vcpu_id, vp_index, msr, data),
+
+ TP_STRUCT__entry(
+ __field(int, vcpu_id)
+ __field(u32, vp_index)
+ __field(u32, msr)
+ __field(u64, data)
+ ),
+
+ TP_fast_assign(
+ __entry->vcpu_id = vcpu_id;
+ __entry->vp_index = vp_index;
+ __entry->msr = msr;
+ __entry->data = data;
+ ),
+
+ TP_printk("vcpu_id %d vp_index %u msr 0x%x data 0x%llx",
+ __entry->vcpu_id, __entry->vp_index, __entry->msr,
+ __entry->data)
+);
#endif /* _TRACE_KVM_H */
#undef TRACE_INCLUDE_PATH
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c17e6eb9ad43..26f89660a6f6 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1233,6 +1233,10 @@ static const u32 emulated_msrs_all[] = {
HV_X64_MSR_VP_ASSIST_PAGE,
HV_X64_MSR_REENLIGHTENMENT_CONTROL, HV_X64_MSR_TSC_EMULATION_CONTROL,
HV_X64_MSR_TSC_EMULATION_STATUS,
+ HV_X64_MSR_SYNDBG_OPTIONS,
+ HV_X64_MSR_SYNDBG_CONTROL, HV_X64_MSR_SYNDBG_STATUS,
+ HV_X64_MSR_SYNDBG_SEND_BUFFER, HV_X64_MSR_SYNDBG_RECV_BUFFER,
+ HV_X64_MSR_SYNDBG_PENDING_BUFFER,
MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
MSR_KVM_PV_EOI_EN,
@@ -2932,6 +2936,8 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
*/
break;
case HV_X64_MSR_GUEST_OS_ID ... HV_X64_MSR_SINT15:
+ case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
+ case HV_X64_MSR_SYNDBG_OPTIONS:
case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
case HV_X64_MSR_CRASH_CTL:
case HV_X64_MSR_STIMER0_CONFIG ... HV_X64_MSR_STIMER3_COUNT:
@@ -3187,6 +3193,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
msr_info->data = 0x20000000;
break;
case HV_X64_MSR_GUEST_OS_ID ... HV_X64_MSR_SINT15:
+ case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
+ case HV_X64_MSR_SYNDBG_OPTIONS:
case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
case HV_X64_MSR_CRASH_CTL:
case HV_X64_MSR_STIMER0_CONFIG ... HV_X64_MSR_STIMER3_COUNT:
@@ -3358,6 +3366,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
case KVM_CAP_HYPERV_TLBFLUSH:
case KVM_CAP_HYPERV_SEND_IPI:
case KVM_CAP_HYPERV_CPUID:
+ case KVM_CAP_HYPERV_SYNDBG:
case KVM_CAP_PCI_SEGMENT:
case KVM_CAP_DEBUGREGS:
case KVM_CAP_X86_ROBUST_SINGLESTEP:
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 9cdc5356f542..ec1b2c7b449e 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -188,6 +188,7 @@ struct kvm_s390_cmma_log {
struct kvm_hyperv_exit {
#define KVM_EXIT_HYPERV_SYNIC 1
#define KVM_EXIT_HYPERV_HCALL 2
+#define KVM_EXIT_HYPERV_SYNDBG 3
__u32 type;
__u32 pad1;
union {
@@ -203,6 +204,15 @@ struct kvm_hyperv_exit {
__u64 result;
__u64 params[2];
} hcall;
+ struct {
+ __u32 msr;
+ __u32 pad2;
+ __u64 control;
+ __u64 status;
+ __u64 send_page;
+ __u64 recv_page;
+ __u64 pending_page;
+ } syndbg;
} u;
};
@@ -1019,6 +1029,7 @@ struct kvm_ppc_resize_hpt {
#define KVM_CAP_S390_VCPU_RESETS 179
#define KVM_CAP_S390_PROTECTED 180
#define KVM_CAP_PPC_SECURE_GUEST 181
+#define KVM_CAP_HYPERV_SYNDBG 182
#ifdef KVM_CAP_IRQ_ROUTING
--
2.24.1
^ permalink raw reply related
* [PATCH v12 2/6] x86/hyper-v: Add synthetic debugger definitions
From: Jon Doron @ 2020-05-29 13:45 UTC (permalink / raw)
To: kvm, linux-hyperv; +Cc: vkuznets, pbonzini, rvkagan, Jon Doron, Michael Kelley
In-Reply-To: <20200529134543.1127440-1-arilou@gmail.com>
Hyper-V synthetic debugger has two modes, one that uses MSRs and
the other that use Hypercalls.
Add all the required definitions to both types of synthetic debugger
interface.
Some of the required new CPUIDs and MSRs are not documented in the TLFS
so they are in hyperv.h instead.
The reason they are not documented is because they are subjected to be
removed in future versions of Windows.
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Signed-off-by: Jon Doron <arilou@gmail.com>
---
arch/x86/include/asm/hyperv-tlfs.h | 6 ++++++
arch/x86/kvm/hyperv.h | 27 +++++++++++++++++++++++++++
2 files changed, 33 insertions(+)
diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index 29336574d0bc..53ef6b7bd380 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -131,6 +131,8 @@
#define HV_FEATURE_FREQUENCY_MSRS_AVAILABLE BIT(8)
/* Crash MSR available */
#define HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE BIT(10)
+/* Support for debug MSRs available */
+#define HV_FEATURE_DEBUG_MSRS_AVAILABLE BIT(11)
/* stimer Direct Mode is available */
#define HV_STIMER_DIRECT_MODE_AVAILABLE BIT(19)
@@ -376,6 +378,9 @@ struct hv_tsc_emulation_status {
#define HVCALL_SEND_IPI_EX 0x0015
#define HVCALL_POST_MESSAGE 0x005c
#define HVCALL_SIGNAL_EVENT 0x005d
+#define HVCALL_POST_DEBUG_DATA 0x0069
+#define HVCALL_RETRIEVE_DEBUG_DATA 0x006a
+#define HVCALL_RESET_DEBUG_SESSION 0x006b
#define HVCALL_RETARGET_INTERRUPT 0x007e
#define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE 0x00af
#define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_LIST 0x00b0
@@ -422,6 +427,7 @@ enum HV_GENERIC_SET_FORMAT {
#define HV_STATUS_INVALID_HYPERCALL_INPUT 3
#define HV_STATUS_INVALID_ALIGNMENT 4
#define HV_STATUS_INVALID_PARAMETER 5
+#define HV_STATUS_OPERATION_DENIED 8
#define HV_STATUS_INSUFFICIENT_MEMORY 11
#define HV_STATUS_INVALID_PORT_ID 17
#define HV_STATUS_INVALID_CONNECTION_ID 18
diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
index 757cb578101c..7f50ff0bad07 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -23,6 +23,33 @@
#include <linux/kvm_host.h>
+/*
+ * The #defines related to the synthetic debugger are required by KDNet, but
+ * they are not documented in the Hyper-V TLFS because the synthetic debugger
+ * functionality has been deprecated and is subject to removal in future
+ * versions of Windows.
+ */
+#define HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS 0x40000080
+#define HYPERV_CPUID_SYNDBG_INTERFACE 0x40000081
+#define HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES 0x40000082
+
+/*
+ * Hyper-V synthetic debugger platform capabilities
+ * These are HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES.EAX bits.
+ */
+#define HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING BIT(1)
+
+/* Hyper-V Synthetic debug options MSR */
+#define HV_X64_MSR_SYNDBG_CONTROL 0x400000F1
+#define HV_X64_MSR_SYNDBG_STATUS 0x400000F2
+#define HV_X64_MSR_SYNDBG_SEND_BUFFER 0x400000F3
+#define HV_X64_MSR_SYNDBG_RECV_BUFFER 0x400000F4
+#define HV_X64_MSR_SYNDBG_PENDING_BUFFER 0x400000F5
+#define HV_X64_MSR_SYNDBG_OPTIONS 0x400000FF
+
+/* Hyper-V HV_X64_MSR_SYNDBG_OPTIONS bits */
+#define HV_X64_SYNDBG_OPTION_USE_HCALLS BIT(2)
+
static inline struct kvm_vcpu_hv *vcpu_to_hv_vcpu(struct kvm_vcpu *vcpu)
{
return &vcpu->arch.hyperv;
--
2.24.1
^ permalink raw reply related
* [PATCH v12 1/6] x86/kvm/hyper-v: Explicitly align hcall param for kvm_hyperv_exit
From: Jon Doron @ 2020-05-29 13:45 UTC (permalink / raw)
To: kvm, linux-hyperv; +Cc: vkuznets, pbonzini, rvkagan, Jon Doron
In-Reply-To: <20200529134543.1127440-1-arilou@gmail.com>
The problem the patch is trying to address is the fact that 'struct
kvm_hyperv_exit' has different layout on when compiling in 32 and 64 bit
modes.
In 64-bit mode the default alignment boundary is 64 bits thus
forcing extra gaps after 'type' and 'msr' but in 32-bit mode the
boundary is at 32 bits thus no extra gaps.
This is an issue as even when the kernel is 64 bit, the userspace using
the interface can be both 32 and 64 bit but the same 32 bit userspace has
to work with 32 bit kernel.
The issue is fixed by forcing the 64 bit layout, this leads to ABI
change for 32 bit builds and while we are obviously breaking '32 bit
userspace with 32 bit kernel' case, we're fixing the '32 bit userspace
with 64 bit kernel' one.
As the interface has no (known) users and 32 bit KVM is rather baroque
nowadays, this seems like a reasonable decision.
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Jon Doron <arilou@gmail.com>
---
Documentation/virt/kvm/api.rst | 2 ++
include/uapi/linux/kvm.h | 2 ++
2 files changed, 4 insertions(+)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index efbbe570aa9b..750d005a75bc 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -5067,9 +5067,11 @@ EOI was received.
#define KVM_EXIT_HYPERV_SYNIC 1
#define KVM_EXIT_HYPERV_HCALL 2
__u32 type;
+ __u32 pad1;
union {
struct {
__u32 msr;
+ __u32 pad2;
__u64 control;
__u64 evt_page;
__u64 msg_page;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 428c7dde6b4b..9cdc5356f542 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -189,9 +189,11 @@ struct kvm_hyperv_exit {
#define KVM_EXIT_HYPERV_SYNIC 1
#define KVM_EXIT_HYPERV_HCALL 2
__u32 type;
+ __u32 pad1;
union {
struct {
__u32 msr;
+ __u32 pad2;
__u64 control;
__u64 evt_page;
__u64 msg_page;
--
2.24.1
^ permalink raw reply related
* [PATCH v12 0/6] x86/kvm/hyper-v: add support for synthetic
From: Jon Doron @ 2020-05-29 13:45 UTC (permalink / raw)
To: kvm, linux-hyperv; +Cc: vkuznets, pbonzini, rvkagan, Jon Doron
Add support for the synthetic debugger interface of hyper-v, the synthetic
debugger has 2 modes.
1. Use a set of MSRs to send/recv information (undocumented so it's not
going to the hyperv-tlfs.h)
2. Use hypercalls
The first mode is based the following MSRs:
1. Control/Status MSRs which either asks for a send/recv .
2. Send/Recv MSRs each holds GPA where the send/recv buffers are.
3. Pending MSR, holds a GPA to a PAGE that simply has a boolean that
indicates if there is data pending to issue a recv VMEXIT.
The first mode implementation is to simply exit to user-space when
either the control MSR or the pending MSR are being set.
Then it's up-to userspace to implement the rest of the logic of sending/recving.
In the second mode instead of using MSRs KNet will simply issue
Hypercalls with the information to send/recv, in this mode the data
being transferred is UDP encapsulated, unlike in the previous mode in
which you get just the data to send.
The new hypercalls will exit to userspace which will be incharge of
re-encapsulating if needed the UDP packets to be sent.
There is an issue though in which KDNet does not respect the hypercall
page and simply issues vmcall/vmmcall instructions depending on the cpu
type expecting them to be handled as it a real hypercall was issued.
It's important to note that part of this feature has been subject to be
removed in future versions of Windows, which is why some of the
defintions will not be present the the TLFS but in the kvm hyperv header
instead.
v12:
- Rebased on latest origin/master
- Make the KVM_CAP_HYPERV_SYNDBG always enabled, in previous version
userspace was required to explicitly enable the syndbg capability just
like with the EVMCS feature.
Jon Doron (5):
x86/kvm/hyper-v: Explicitly align hcall param for kvm_hyperv_exit
x86/hyper-v: Add synthetic debugger definitions
x86/kvm/hyper-v: Add support for synthetic debugger capability
x86/kvm/hyper-v: enable hypercalls regardless of hypercall page
x86/kvm/hyper-v: Add support for synthetic debugger via hypercalls
Vitaly Kuznetsov (1):
KVM: selftests: update hyperv_cpuid with SynDBG tests
Documentation/virt/kvm/api.rst | 18 ++
arch/x86/include/asm/hyperv-tlfs.h | 6 +
arch/x86/include/asm/kvm_host.h | 13 ++
arch/x86/kvm/hyperv.c | 180 +++++++++++++++++-
arch/x86/kvm/hyperv.h | 32 ++++
arch/x86/kvm/trace.h | 51 +++++
arch/x86/kvm/x86.c | 9 +
include/uapi/linux/kvm.h | 13 ++
.../selftests/kvm/x86_64/hyperv_cpuid.c | 103 +++++-----
9 files changed, 374 insertions(+), 51 deletions(-)
--
2.24.1
^ permalink raw reply
* Re: [PATCH] x86/apic/flat64: Add back the early_param("apic", parse_apic)
From: Randy Dunlap @ 2020-05-29 13:33 UTC (permalink / raw)
To: Dexuan Cui, tglx, mingo, bp, hpa, x86, peterz, allison,
alexios.zavras, gregkh, namit, mikelley, longli
Cc: linux-kernel, linux-hyperv
In-Reply-To: <20200529063729.22047-1-decui@microsoft.com>
On 5/28/20 11:37 PM, Dexuan Cui wrote:
> parse_apic() allows the user to try a different apic driver than the
> default one that's automatically chosen. It works for x86_32, but
> doesn't work for x86_64 becauase it was removed in 2009 for x86_64 by:
> commit 7b38725318f4 ("x86: remove subarchitecture support code"),
> whose changelog doesn't explicitly describe the removal for x86_64.
>
> The patch adds back the functionality for x86_64. The intent is mainly
> to work around an APIC emulation bug in Hyper-V in the case of kdump:
> currently Hyper-V does not honor the disabled state of the local APICs,
> and all the IOAPIC-based interrupts may not be delivered to the correct
> virtual CPU, if the logical-mode APIC driver is used (the kdump
> kernel usually uses the logical-mode APIC driver, since typically
> only 1 CPU is active). Luckily the kdump issue can be worked around by
> forcing the kdump kernel to use physical mode, before the fix to Hyper-V
> becomes widely available.
>
> IMHO the patch is safe because the current default algorithm to choose
> the apic driver is unchanged; the patch makes a difference only when
> the user specifies the apic= kernel parameter, e.g. "apic=physical flat".
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
> arch/x86/kernel/apic/apic_flat_64.c | 27 ++++++++++++++++++++++++++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
Hi,
Looks like you will also need to update
Documentation/admin-guide/kernel-parameters.txt, where it says:
For X86-32, this can also be used to specify an APIC
driver name.
> diff --git a/arch/x86/kernel/apic/apic_flat_64.c b/arch/x86/kernel/apic/apic_flat_64.c
> index 7862b152a052..efbec63bb01f 100644
> --- a/arch/x86/kernel/apic/apic_flat_64.c
> +++ b/arch/x86/kernel/apic/apic_flat_64.c
> @@ -23,9 +23,34 @@ static struct apic apic_flat;
> struct apic *apic __ro_after_init = &apic_flat;
> EXPORT_SYMBOL_GPL(apic);
>
> +static int cmdline_apic __initdata;
> +static int __init parse_apic(char *arg)
> +{
> + struct apic **drv;
> +
> + if (!arg)
> + return -EINVAL;
> +
> + for (drv = __apicdrivers; drv < __apicdrivers_end; drv++) {
> + if (!strcmp((*drv)->name, arg)) {
> + apic = *drv;
> + cmdline_apic = 1;
> + return 0;
> + }
> + }
> +
> + /* Parsed again by __setup for debug/verbose */
> + return 0;
> +}
> +early_param("apic", parse_apic);
> +
> +
> static int flat_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
> {
> - return 1;
> + if (!cmdline_apic)
> + return 1;
> +
> + return apic == &apic_flat;
> }
>
> /*
>
--
~Randy
^ permalink raw reply
* Re: [PATCH v11 4/7] x86/kvm/hyper-v: Add support for synthetic debugger capability
From: Paolo Bonzini @ 2020-05-29 12:17 UTC (permalink / raw)
To: Vitaly Kuznetsov; +Cc: Jon Doron, kvm, linux-hyperv
In-Reply-To: <87ftbjhrk0.fsf@vitty.brq.redhat.com>
On 29/05/20 14:08, Vitaly Kuznetsov wrote:
>> On 24/04/20 13:37, Jon Doron wrote:
>>> +static int syndbg_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host)
>>> +{
>>> + struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
>>> +
>>> + if (!syndbg->active && !host)
>>> + return 1;
>>> +
>> One small thing: is the ENABLE_CAP and active field needed? Can you
>> just check if the guest has the syndbg CPUID bits set?
>>
> Yes, we can probably get away with a static capability (so userspace
> knows that the interface is supported and CPUID bit can be set) and
> check guest_cpuid_has() here but we don't have Hyper-V feature leaves
> exposed as X86_FEATURE_* (yet). It is probably possible to implement an
> interim solution by open coding the check with kvm_find_cpuid_entry() or
> something like that.
Yes, that would be fine if you just abstract it in its own function.
Paolo
^ permalink raw reply
* Re: [PATCH v11 4/7] x86/kvm/hyper-v: Add support for synthetic debugger capability
From: Vitaly Kuznetsov @ 2020-05-29 12:08 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Jon Doron, kvm, linux-hyperv
In-Reply-To: <12df4348-3dc1-a4cc-aa41-4492cd42dcc8@redhat.com>
Paolo Bonzini <pbonzini@redhat.com> writes:
> On 24/04/20 13:37, Jon Doron wrote:
>> +static int syndbg_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host)
>> +{
>> + struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
>> +
>> + if (!syndbg->active && !host)
>> + return 1;
>> +
>
> One small thing: is the ENABLE_CAP and active field needed? Can you
> just check if the guest has the syndbg CPUID bits set?
>
Yes, we can probably get away with a static capability (so userspace
knows that the interface is supported and CPUID bit can be set) and
check guest_cpuid_has() here but we don't have Hyper-V feature leaves
exposed as X86_FEATURE_* (yet). It is probably possible to implement an
interim solution by open coding the check with kvm_find_cpuid_entry() or
something like that.
--
Vitaly
^ permalink raw reply
* Re: [PATCH v11 2/7] x86/kvm/hyper-v: Simplify addition for custom cpuid leafs
From: Paolo Bonzini @ 2020-05-29 11:13 UTC (permalink / raw)
To: Jon Doron, Roman Kagan, kvm, linux-hyperv, vkuznets
In-Reply-To: <20200513124915.GM2862@jondnuc>
On 13/05/20 14:49, Jon Doron wrote:
>>
>
> To be honest from my understanding of the TLFS it states:
> "The maximum input value for hypervisor CPUID information."
>
> So we should not expose stuff we wont "answer" to, I agree you can
> always issue CPUID to any leaf and you will get zeroes but if we try to
> follow TLFS it sounds like this needs to be capped here.
I think Roman is right in the reading, but it's also nicer to have a
capped EAX because you can just look at EAX and guess what features are
there (it's simpler than looking for zeroes).
Paolo
^ permalink raw reply
* Re: [PATCH v11 5/7] x86/kvm/hyper-v: enable hypercalls without hypercall page with syndbg
From: Paolo Bonzini @ 2020-05-29 10:48 UTC (permalink / raw)
To: Jon Doron, kvm, linux-hyperv; +Cc: vkuznets
In-Reply-To: <20200424113746.3473563-6-arilou@gmail.com>
On 24/04/20 13:37, Jon Doron wrote:
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 435516595090..524b5466a515 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -1650,7 +1650,10 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *current_vcpu, u64 ingpa, u64 outgpa,
>
> bool kvm_hv_hypercall_enabled(struct kvm *kvm)
> {
> - return READ_ONCE(kvm->arch.hyperv.hv_hypercall) & HV_X64_MSR_HYPERCALL_ENABLE;
> + struct kvm_hv *hv = &kvm->arch.hyperv;
> +
> + return READ_ONCE(hv->hv_hypercall) & HV_X64_MSR_HYPERCALL_ENABLE ||
> + (hv->hv_syndbg.active && READ_ONCE(hv->hv_guest_os_id) != 0);
> }
Here too we could just shrug and allow hypercalls if the guest OS is not
NULL.
Paolo
^ permalink raw reply
* Re: [PATCH v11 4/7] x86/kvm/hyper-v: Add support for synthetic debugger capability
From: Paolo Bonzini @ 2020-05-29 10:46 UTC (permalink / raw)
To: Jon Doron, kvm, linux-hyperv; +Cc: vkuznets
In-Reply-To: <20200424113746.3473563-5-arilou@gmail.com>
On 24/04/20 13:37, Jon Doron wrote:
> +static int syndbg_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host)
> +{
> + struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
> +
> + if (!syndbg->active && !host)
> + return 1;
> +
One small thing: is the ENABLE_CAP and active field needed? Can you
just check if the guest has the syndbg CPUID bits set?
Thanks,
Paolo
^ permalink raw reply
* [PATCH] x86/apic/flat64: Add back the early_param("apic", parse_apic)
From: Dexuan Cui @ 2020-05-29 6:37 UTC (permalink / raw)
To: tglx, mingo, bp, hpa, x86, peterz, allison, alexios.zavras,
gregkh, decui, namit, mikelley, longli
Cc: linux-kernel, linux-hyperv
parse_apic() allows the user to try a different apic driver than the
default one that's automatically chosen. It works for x86_32, but
doesn't work for x86_64 becauase it was removed in 2009 for x86_64 by:
commit 7b38725318f4 ("x86: remove subarchitecture support code"),
whose changelog doesn't explicitly describe the removal for x86_64.
The patch adds back the functionality for x86_64. The intent is mainly
to work around an APIC emulation bug in Hyper-V in the case of kdump:
currently Hyper-V does not honor the disabled state of the local APICs,
and all the IOAPIC-based interrupts may not be delivered to the correct
virtual CPU, if the logical-mode APIC driver is used (the kdump
kernel usually uses the logical-mode APIC driver, since typically
only 1 CPU is active). Luckily the kdump issue can be worked around by
forcing the kdump kernel to use physical mode, before the fix to Hyper-V
becomes widely available.
IMHO the patch is safe because the current default algorithm to choose
the apic driver is unchanged; the patch makes a difference only when
the user specifies the apic= kernel parameter, e.g. "apic=physical flat".
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
arch/x86/kernel/apic/apic_flat_64.c | 27 ++++++++++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/apic/apic_flat_64.c b/arch/x86/kernel/apic/apic_flat_64.c
index 7862b152a052..efbec63bb01f 100644
--- a/arch/x86/kernel/apic/apic_flat_64.c
+++ b/arch/x86/kernel/apic/apic_flat_64.c
@@ -23,9 +23,34 @@ static struct apic apic_flat;
struct apic *apic __ro_after_init = &apic_flat;
EXPORT_SYMBOL_GPL(apic);
+static int cmdline_apic __initdata;
+static int __init parse_apic(char *arg)
+{
+ struct apic **drv;
+
+ if (!arg)
+ return -EINVAL;
+
+ for (drv = __apicdrivers; drv < __apicdrivers_end; drv++) {
+ if (!strcmp((*drv)->name, arg)) {
+ apic = *drv;
+ cmdline_apic = 1;
+ return 0;
+ }
+ }
+
+ /* Parsed again by __setup for debug/verbose */
+ return 0;
+}
+early_param("apic", parse_apic);
+
+
static int flat_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
{
- return 1;
+ if (!cmdline_apic)
+ return 1;
+
+ return apic == &apic_flat;
}
/*
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] PCI: hv: Use struct_size() helper
From: Lorenzo Pieralisi @ 2020-05-28 14:41 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Rob Herring, Bjorn Helgaas, linux-hyperv, linux-pci, linux-kernel,
Gustavo A. R. Silva, Kees Cook
In-Reply-To: <20200525164319.GA13596@embeddedor>
On Mon, May 25, 2020 at 11:43:19AM -0500, Gustavo A. R. Silva wrote:
> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:
>
> struct hv_dr_state {
> ...
> struct hv_pcidev_description func[];
> };
>
> struct pci_bus_relations {
> ...
> struct pci_function_description func[];
> } __packed;
>
> Make use of the struct_size() helper instead of an open-coded version
> in order to avoid any potential type mistakes.
>
> So, replace the following forms:
>
> offsetof(struct hv_dr_state, func) +
> (sizeof(struct hv_pcidev_description) *
> (relations->device_count))
>
> offsetof(struct pci_bus_relations, func) +
> (sizeof(struct pci_function_description) *
> (bus_rel->device_count))
>
> with:
>
> struct_size(dr, func, relations->device_count)
>
> and
>
> struct_size(bus_rel, func, bus_rel->device_count)
>
> respectively.
>
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
> drivers/pci/controller/pci-hyperv.c | 22 ++++++++--------------
> 1 file changed, 8 insertions(+), 14 deletions(-)
Applied to pci/hv, thanks.
Lorenzo
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 892f3a742117a..bf40ff09c99d6 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -2213,10 +2213,8 @@ static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
> 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);
> -
> + dr = kzalloc(struct_size(dr, func, relations->device_count),
> + GFP_NOWAIT);
> if (!dr)
> return;
>
> @@ -2250,10 +2248,8 @@ static void hv_pci_devices_present2(struct hv_pcibus_device *hbus,
> 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);
> -
> + dr = kzalloc(struct_size(dr, func, relations->device_count),
> + GFP_NOWAIT);
> if (!dr)
> return;
>
> @@ -2447,9 +2443,8 @@ static void hv_pci_onchannelcallback(void *context)
>
> bus_rel = (struct pci_bus_relations *)buffer;
> if (bytes_recvd <
> - offsetof(struct pci_bus_relations, func) +
> - (sizeof(struct pci_function_description) *
> - (bus_rel->device_count))) {
> + struct_size(bus_rel, func,
> + bus_rel->device_count)) {
> dev_err(&hbus->hdev->device,
> "bus relations too small\n");
> break;
> @@ -2462,9 +2457,8 @@ static void hv_pci_onchannelcallback(void *context)
>
> 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))) {
> + struct_size(bus_rel2, func,
> + bus_rel2->device_count)) {
> dev_err(&hbus->hdev->device,
> "bus relations v2 too small\n");
> break;
> --
> 2.26.2
>
^ 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