[PATCH 0/5] hyper-v: Don't assume cpu_possible_mask is dense

[PATCH 0/5] hyper-v: Don't assume cpu_possible_mask is dense

[mhkelley58]

From: Michael Kelley <mhklinux@outlook.com>

Code specific to Hyper-V guests currently assumes the cpu_possible_mask
is "dense" -- i.e., all bit positions 0 thru (nr_cpu_ids - 1) are set,
with no "holes". Therefore, num_possible_cpus() is assumed to be equal
to nr_cpu_ids.

Per a separate discussion[1], this assumption is not valid in the
general case. For example, the function setup_nr_cpu_ids() in
kernel/smp.c is coded to assume cpu_possible_mask may be sparse,
and other patches have been made in the past to correctly handle
the sparseness. See bc75e99983df1efd ("rcu: Correctly handle sparse
possible cpu") as noted by Mark Rutland.

The general case notwithstanding, the configurations that Hyper-V
provides to guest VMs on x86 and ARM64 hardware, in combination
with the algorithms currently used by architecture specific code
to assign Linux CPU numbers, *does* always produce a dense
cpu_possible_mask. So the invalid assumption is not currently
causing failures. But in the interest of correctness, and robustness
against future changes in the code that populates cpu_possible_mask,
update the Hyper-V code to no longer assume denseness.

The typical code pattern with the invalid assumption is as follows:

	array = kcalloc(num_possible_cpus(), sizeof(<some struct>),
			GFP_KERNEL);
	....
	index into "array" with smp_processor_id()

In such as case, the array might be indexed by a value beyond the size
of the array. The correct approach is to allocate the array with size
"nr_cpu_ids". While this will probably leave unused any array entries
corresponding to holes in cpu_possible_mask, the holes are assumed to
be minimal and hence the amount of memory wasted by unused entries is
minimal.

Removing the assumption in Hyper-V code is done in several patches
because they touch different kernel subsystems:

Patch 1: Hyper-V x86 initialization of hv_vp_assist_page (there's no
	 hv_vp_assist_page on ARM64)
Patch 2: Hyper-V common init of hv_vp_index
Patch 3: Hyper-V IOMMU driver
Patch 4: storvsc driver
Patch 5: netvsc driver

I tested the changes by hacking the construction of cpu_possible_mask
to include a hole on x86. With a configuration set to demonstrate the
problem, a Hyper-V guest kernel eventually crashes due to memory
corruption. After the patches in this series, the crash does not occur.

[1] https://lore.kernel.org/lkml/SN6PR02MB4157210CC36B2593F8572E5ED4692@SN6PR02MB4157.namprd02.prod.outlook.com/

Michael Kelley (5):
  x86/hyperv: Don't assume cpu_possible_mask is dense
  Drivers: hv: Don't assume cpu_possible_mask is dense
  iommu/hyper-v: Don't assume cpu_possible_mask is dense
  scsi: storvsc: Don't assume cpu_possible_mask is dense
  hv_netvsc: Don't assume cpu_possible_mask is dense

 arch/x86/hyperv/hv_init.c       |  2 +-
 drivers/hv/hv_common.c          |  4 ++--
 drivers/iommu/hyperv-iommu.c    |  4 ++--
 drivers/net/hyperv/netvsc_drv.c |  2 +-
 drivers/scsi/storvsc_drv.c      | 13 ++++++-------
 5 files changed, 12 insertions(+), 13 deletions(-)

-- 
2.25.1

[PATCH 1/5] x86/hyperv: Don't assume cpu_possible_mask is dense

[mhkelley58]

From: Michael Kelley <mhklinux@outlook.com>

Current code allocates the hv_vp_assist_page array with size
num_possible_cpus(). This code assumes cpu_possible_mask is dense,
which is not true in the general case per [1]. If cpu_possible_mask
is sparse, the array might be indexed by a value beyond the size of
the array.

However, the configurations that Hyper-V provides to guest VMs on x86
hardware, in combination with how x86 code assigns Linux CPU numbers,
*does* always produce a dense cpu_possible_mask. So the dense assumption
is not currently causing failures. But for robustness against future
changes in how cpu_possible_mask is populated, update the code to no
longer assume dense.

The correct approach is to allocate the array with size "nr_cpu_ids".
While this leaves unused array entries corresponding to holes in
cpu_possible_mask, the holes are assumed to be minimal and hence the
amount of memory wasted by unused entries is minimal.

[1] https://lore.kernel.org/lkml/SN6PR02MB4157210CC36B2593F8572E5ED4692@SN6PR02MB4157.namprd02.prod.outlook.com/

Signed-off-by: Michael Kelley <mhklinux@outlook.com>
---
 arch/x86/hyperv/hv_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 95eada2994e1..2cec4dfec165 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -473,7 +473,7 @@ void __init hyperv_init(void)
 	if (hv_isolation_type_tdx())
 		hv_vp_assist_page = NULL;
 	else
-		hv_vp_assist_page = kcalloc(num_possible_cpus(),
+		hv_vp_assist_page = kcalloc(nr_cpu_ids,
 					    sizeof(*hv_vp_assist_page),
 					    GFP_KERNEL);
 	if (!hv_vp_assist_page) {
-- 
2.25.1

[PATCH 2/5] Drivers: hv: Don't assume cpu_possible_mask is dense

[mhkelley58]

From: Michael Kelley <mhklinux@outlook.com>

Current code allocates the hv_vp_index array with size
num_possible_cpus(). This code assumes cpu_possible_mask is dense,
which is not true in the general case per [1]. If cpu_possible_mask
is sparse, the array might be indexed by a value beyond the size of
the array.

However, the configurations that Hyper-V provides to guest VMs on x86
and ARM64 hardware, in combination with how architecture specific code
assigns Linux CPU numbers, *does* always produce a dense cpu_possible_mask.
So the dense assumption is not currently causing failures. But for
robustness against future changes in how cpu_possible_mask is populated,
update the code to no longer assume dense.

The correct approach is to allocate and initialize the array using size
"nr_cpu_ids". While this leaves unused array entries corresponding to
holes in cpu_possible_mask, the holes are assumed to be minimal and hence
the amount of memory wasted by unused entries is minimal.

Using nr_cpu_ids also reduces initialization time, in that the loop to
initialize the array currently rescans cpu_possible_mask on each
iteration. This is n-squared in the number of CPUs, which could be
significant for large CPU counts.

[1] https://lore.kernel.org/lkml/SN6PR02MB4157210CC36B2593F8572E5ED4692@SN6PR02MB4157.namprd02.prod.outlook.com/

Signed-off-by: Michael Kelley <mhklinux@outlook.com>
---
 drivers/hv/hv_common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index d50caf0d723d..8c44938cb084 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -345,14 +345,14 @@ int __init hv_common_init(void)
 		BUG_ON(!hyperv_pcpu_output_arg);
 	}
 
-	hv_vp_index = kmalloc_array(num_possible_cpus(), sizeof(*hv_vp_index),
+	hv_vp_index = kmalloc_array(nr_cpu_ids, sizeof(*hv_vp_index),
 				    GFP_KERNEL);
 	if (!hv_vp_index) {
 		hv_common_free();
 		return -ENOMEM;
 	}
 
-	for (i = 0; i < num_possible_cpus(); i++)
+	for (i = 0; i < nr_cpu_ids; i++)
 		hv_vp_index[i] = VP_INVAL;
 
 	return 0;
-- 
2.25.1

[PATCH 3/5] iommu/hyper-v: Don't assume cpu_possible_mask is dense

[mhkelley58]

From: Michael Kelley <mhklinux@outlook.com>

Current code gets the APIC IDs for CPUs numbered 255 and lower.
This code assumes cpu_possible_mask is dense, which is not true in
the general case per [1]. If cpu_possible_mask contains holes,
num_possible_cpus() is less than nr_cpu_ids, so some CPUs might get
skipped. Furthermore, getting the APIC ID of a CPU that isn't in
cpu_possible_mask is invalid.

However, the configurations that Hyper-V provides to guest VMs on x86
hardware, in combination with how x86 code assigns Linux CPU numbers,
*does* always produce a dense cpu_possible_mask. So the dense assumption
is not currently causing failures. But for robustness against future
changes in how cpu_possible_mask is populated, update the code to no
longer assume dense.

The correct approach is to determine the range to scan based on
nr_cpu_ids, and skip any CPUs that are not in the cpu_possible_mask.

[1] https://lore.kernel.org/lkml/SN6PR02MB4157210CC36B2593F8572E5ED4692@SN6PR02MB4157.namprd02.prod.outlook.com/

Signed-off-by: Michael Kelley <mhklinux@outlook.com>
---
 drivers/iommu/hyperv-iommu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
index 8a5c17b97310..2a86aa5d54c6 100644
--- a/drivers/iommu/hyperv-iommu.c
+++ b/drivers/iommu/hyperv-iommu.c
@@ -164,8 +164,8 @@ static int __init hyperv_prepare_irq_remapping(void)
 	 * max cpu affinity for IOAPIC irqs. Scan cpu 0-255 and set cpu
 	 * into ioapic_max_cpumask if its APIC ID is less than 256.
 	 */
-	for (i = min_t(unsigned int, num_possible_cpus() - 1, 255); i >= 0; i--)
-		if (cpu_physical_id(i) < 256)
+	for (i = min_t(unsigned int, nr_cpu_ids - 1, 255); i >= 0; i--)
+		if (cpu_possible(i) && cpu_physical_id(i) < 256)
 			cpumask_set_cpu(i, &ioapic_max_cpumask);
 
 	return 0;
-- 
2.25.1

[PATCH 4/5] scsi: storvsc: Don't assume cpu_possible_mask is dense

[mhkelley58]

From: Michael Kelley <mhklinux@outlook.com>

Current code allocates the stor_chns array with size num_possible_cpus().
This code assumes cpu_possible_mask is dense, which is not true in
the general case per [1]. If cpu_possible_mask is sparse, the array
might be indexed by a value beyond the size of the array.

However, the configurations that Hyper-V provides to guest VMs on x86
and ARM64 hardware, in combination with how architecture specific code
assigns Linux CPU numbers, *does* always produce a dense cpu_possible_mask.
So the dense assumption is not currently causing failures. But for
robustness against future changes in how cpu_possible_mask is populated,
update the code to no longer assume dense.

The correct approach is to allocate and initialize the array using size
"nr_cpu_ids". While this leaves unused array entries corresponding to
holes in cpu_possible_mask, the holes are assumed to be minimal and hence
the amount of memory wasted by unused entries is minimal.

[1] https://lore.kernel.org/lkml/SN6PR02MB4157210CC36B2593F8572E5ED4692@SN6PR02MB4157.namprd02.prod.outlook.com/

Signed-off-by: Michael Kelley <mhklinux@outlook.com>
---
 drivers/scsi/storvsc_drv.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 11b3fc3b24c9..f2beb6b23284 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -917,14 +917,13 @@ static int storvsc_channel_init(struct hv_device *device, bool is_fc)
 
 	/*
 	 * Allocate state to manage the sub-channels.
-	 * We allocate an array based on the numbers of possible CPUs
-	 * (Hyper-V does not support cpu online/offline).
-	 * This Array will be sparseley populated with unique
-	 * channels - primary + sub-channels.
-	 * We will however populate all the slots to evenly distribute
-	 * the load.
+	 * We allocate an array based on the number of CPU ids. This array
+	 * is initially sparsely populated for the CPUs assigned to channels:
+	 * primary + sub-channels. As I/Os are initiated by different CPUs,
+	 * the slots for all online CPUs are populated to evenly distribute
+	 * the load across all channels.
 	 */
-	stor_device->stor_chns = kcalloc(num_possible_cpus(), sizeof(void *),
+	stor_device->stor_chns = kcalloc(nr_cpu_ids, sizeof(void *),
 					 GFP_KERNEL);
 	if (stor_device->stor_chns == NULL)
 		return -ENOMEM;
-- 
2.25.1

[PATCH net-next 5/5] hv_netvsc: Don't assume cpu_possible_mask is dense

[mhkelley58]

From: Michael Kelley <mhklinux@outlook.com>

Current code allocates the pcpu_sum array with size num_possible_cpus().
This code assumes the cpu_possible_mask is dense, which is not true in
the general case per [1]. If cpu_possible_mask is sparse, the array
might be indexed by a value beyond the size of the array.

However, the configurations that Hyper-V provides to guest VMs on x86
and ARM64 hardware, in combination with how architecture specific code
assigns Linux CPU numbers, *does* always produce a dense cpu_possible_mask.
So the dense assumption is not currently causing failures. But for
robustness against future changes in how cpu_possible_mask is populated,
update the code to no longer assume dense.

The correct approach is to allocate and initialize the array using size
"nr_cpu_ids". While this leaves unused array entries corresponding to
holes in cpu_possible_mask, the holes are assumed to be minimal and hence
the amount of memory wasted by unused entries is minimal.

[1] https://lore.kernel.org/lkml/SN6PR02MB4157210CC36B2593F8572E5ED4692@SN6PR02MB4157.namprd02.prod.outlook.com/

Signed-off-by: Michael Kelley <mhklinux@outlook.com>
---
 drivers/net/hyperv/netvsc_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 153b97f8ec0d..f8e2dd6d271d 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1557,7 +1557,7 @@ static void netvsc_get_ethtool_stats(struct net_device *dev,
 		data[i++] = xdp_tx;
 	}
 
-	pcpu_sum = kvmalloc_array(num_possible_cpus(),
+	pcpu_sum = kvmalloc_array(nr_cpu_ids,
 				  sizeof(struct netvsc_ethtool_pcpu_stats),
 				  GFP_KERNEL);
 	if (!pcpu_sum)
-- 
2.25.1

Re: [PATCH 0/5] hyper-v: Don't assume cpu_possible_mask is dense

[Peter Zijlstra]

On Wed, Oct 02, 2024 at 08:53:28PM -0700, mhkelley58@gmail.com wrote:
> 
> Michael Kelley (5):
>   x86/hyperv: Don't assume cpu_possible_mask is dense
>   Drivers: hv: Don't assume cpu_possible_mask is dense
>   iommu/hyper-v: Don't assume cpu_possible_mask is dense
>   scsi: storvsc: Don't assume cpu_possible_mask is dense
>   hv_netvsc: Don't assume cpu_possible_mask is dense

Thanks, these look sane.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Re: [PATCH 0/5] hyper-v: Don't assume cpu_possible_mask is dense

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed,  2 Oct 2024 20:53:28 -0700 you wrote:
> From: Michael Kelley <mhklinux@outlook.com>
> 
> Code specific to Hyper-V guests currently assumes the cpu_possible_mask
> is "dense" -- i.e., all bit positions 0 thru (nr_cpu_ids - 1) are set,
> with no "holes". Therefore, num_possible_cpus() is assumed to be equal
> to nr_cpu_ids.
> 
> [...]

Here is the summary with links:
  - [1/5] x86/hyperv: Don't assume cpu_possible_mask is dense
    (no matching commit)
  - [2/5] Drivers: hv: Don't assume cpu_possible_mask is dense
    (no matching commit)
  - [3/5] iommu/hyper-v: Don't assume cpu_possible_mask is dense
    (no matching commit)
  - [4/5] scsi: storvsc: Don't assume cpu_possible_mask is dense
    (no matching commit)
  - [net-next,5/5] hv_netvsc: Don't assume cpu_possible_mask is dense
    https://git.kernel.org/netdev/net-next/c/c86ab60b92d1

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH 0/5] hyper-v: Don't assume cpu_possible_mask is dense

[Jakub Kicinski]

On Fri, 04 Oct 2024 23:20:40 +0000 patchwork-bot+netdevbpf@kernel.org
wrote:
>   - [net-next,5/5] hv_netvsc: Don't assume cpu_possible_mask is dense
>     https://git.kernel.org/netdev/net-next/c/c86ab60b92d1

On reflection I probably should have asked you for a fixes tag and
applied it to net :S  Oh well.

RE: [PATCH 0/5] hyper-v: Don't assume cpu_possible_mask is dense

[Michael Kelley]

From: Jakub Kicinski <kuba@kernel.org> Sent: Friday, October 4, 2024 4:26 PM
> 
> On Fri, 04 Oct 2024 23:20:40 +0000 patchwork-bot+netdevbpf@kernel.org
> wrote:
> >   - [net-next,5/5] hv_netvsc: Don't assume cpu_possible_mask is dense
> >
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=c86ab60b92d1
> 
> On reflection I probably should have asked you for a fixes tag and
> applied it to net :S  Oh well.

I thought about including a Fixes: tag, but decided that since current and
past builds don't exhibit a failure due to this issue, it's not necessary to
backport. The patches are more for robustness against future changes.
I've heard differing views on whether changes that aren't fixing an
actual failure should be backported ....

Also, patchwork bot email says that the *series* was applied to net-next.
But appears that's not the case.  Only the network-related patch from the
series was applied.  Still need the SCSI maintainer to pick up the SCSI patch,
and the Hyper-V maintainer to pick up the others.

Thanks,

Michael

RE: [PATCH 4/5] scsi: storvsc: Don't assume cpu_possible_mask is dense

[Michael Kelley]

From: mhkelley58@gmail.com <mhkelley58@gmail.com> Sent: Wednesday, October 2, 2024 8:54 PM
> 
> Current code allocates the stor_chns array with size num_possible_cpus().
> This code assumes cpu_possible_mask is dense, which is not true in
> the general case per [1]. If cpu_possible_mask is sparse, the array
> might be indexed by a value beyond the size of the array.
> 
> However, the configurations that Hyper-V provides to guest VMs on x86
> and ARM64 hardware, in combination with how architecture specific code
> assigns Linux CPU numbers, *does* always produce a dense cpu_possible_mask.
> So the dense assumption is not currently causing failures. But for
> robustness against future changes in how cpu_possible_mask is populated,
> update the code to no longer assume dense.
> 
> The correct approach is to allocate and initialize the array using size
> "nr_cpu_ids". While this leaves unused array entries corresponding to
> holes in cpu_possible_mask, the holes are assumed to be minimal and hence
> the amount of memory wasted by unused entries is minimal.
> 
> [1] https://lore.kernel.org/lkml/SN6PR02MB4157210CC36B2593F8572E5ED4692@SN6PR02MB4157.namprd02.prod.outlook.com/
> 
> Signed-off-by: Michael Kelley <mhklinux@outlook.com>

Martin or James --

This entire series was Acked-by: Peter Zijlstra [1]. Patch 5 of the
series was picked up by the net-next tree a few weeks back and is
in 6.13-rc1. Do you need anything else to pick up this single patch
in the appropriate scsi tree?

I'll separately pursue getting Patches 1 thru 3 of the series
picked up by the Hyper-V tree. There's no interdependency
between the patches in the series, so they can each go
separately.

Michael

[1] https://lore.kernel.org/linux-hyperv/20241004100742.GO18071@noisy.programming.kicks-ass.net/

> ---
>  drivers/scsi/storvsc_drv.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 11b3fc3b24c9..f2beb6b23284 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -917,14 +917,13 @@ static int storvsc_channel_init(struct hv_device *device, bool
> is_fc)
> 
>  	/*
>  	 * Allocate state to manage the sub-channels.
> -	 * We allocate an array based on the numbers of possible CPUs
> -	 * (Hyper-V does not support cpu online/offline).
> -	 * This Array will be sparseley populated with unique
> -	 * channels - primary + sub-channels.
> -	 * We will however populate all the slots to evenly distribute
> -	 * the load.
> +	 * We allocate an array based on the number of CPU ids. This array
> +	 * is initially sparsely populated for the CPUs assigned to channels:
> +	 * primary + sub-channels. As I/Os are initiated by different CPUs,
> +	 * the slots for all online CPUs are populated to evenly distribute
> +	 * the load across all channels.
>  	 */
> -	stor_device->stor_chns = kcalloc(num_possible_cpus(), sizeof(void *),
> +	stor_device->stor_chns = kcalloc(nr_cpu_ids, sizeof(void *),
>  					 GFP_KERNEL);
>  	if (stor_device->stor_chns == NULL)
>  		return -ENOMEM;
> --
> 2.25.1
> 

Re: [PATCH 4/5] scsi: storvsc: Don't assume cpu_possible_mask is dense

[Martin K. Petersen]

Michael,

>> Current code allocates the stor_chns array with size
>> num_possible_cpus(). This code assumes cpu_possible_mask is dense,
>> which is not true in the general case per [1]. If cpu_possible_mask
>> is sparse, the array might be indexed by a value beyond the size of
>> the array.

Applied to 6.14/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

RE: [PATCH 0/5] hyper-v: Don't assume cpu_possible_mask is dense

[Michael Kelley]

From: mhkelley58@gmail.com <mhkelley58@gmail.com> Sent: Wednesday, October 2, 2024 8:53 PM
> 
> Code specific to Hyper-V guests currently assumes the cpu_possible_mask
> is "dense" -- i.e., all bit positions 0 thru (nr_cpu_ids - 1) are set,
> with no "holes". Therefore, num_possible_cpus() is assumed to be equal
> to nr_cpu_ids.
> 
> Per a separate discussion[1], this assumption is not valid in the
> general case. For example, the function setup_nr_cpu_ids() in
> kernel/smp.c is coded to assume cpu_possible_mask may be sparse,
> and other patches have been made in the past to correctly handle
> the sparseness. See bc75e99983df1efd ("rcu: Correctly handle sparse
> possible cpu") as noted by Mark Rutland.
> 
> The general case notwithstanding, the configurations that Hyper-V
> provides to guest VMs on x86 and ARM64 hardware, in combination
> with the algorithms currently used by architecture specific code
> to assign Linux CPU numbers, *does* always produce a dense
> cpu_possible_mask. So the invalid assumption is not currently
> causing failures. But in the interest of correctness, and robustness
> against future changes in the code that populates cpu_possible_mask,
> update the Hyper-V code to no longer assume denseness.
> 
> The typical code pattern with the invalid assumption is as follows:
> 
> 	array = kcalloc(num_possible_cpus(), sizeof(<some struct>),
> 			GFP_KERNEL);
> 	....
> 	index into "array" with smp_processor_id()
> 
> In such as case, the array might be indexed by a value beyond the size
> of the array. The correct approach is to allocate the array with size
> "nr_cpu_ids". While this will probably leave unused any array entries
> corresponding to holes in cpu_possible_mask, the holes are assumed to
> be minimal and hence the amount of memory wasted by unused entries is
> minimal.
> 
> Removing the assumption in Hyper-V code is done in several patches
> because they touch different kernel subsystems:
> 
> Patch 1: Hyper-V x86 initialization of hv_vp_assist_page (there's no
> 	 hv_vp_assist_page on ARM64)
> Patch 2: Hyper-V common init of hv_vp_index
> Patch 3: Hyper-V IOMMU driver
> Patch 4: storvsc driver
> Patch 5: netvsc driver

Wei --

Could you pick up Patches 1, 2, and 3 in this series for the hyperv-next
tree? Peter Zijlstra acked the full series [2], and Patches 4 and 5 have
already been picked by the SCSI and net maintainers respectively [3][4].

Let me know if you have any concerns.

Thanks,

Michael

[2] https://lore.kernel.org/linux-hyperv/20241004100742.GO18071@noisy.programming.kicks-ass.net/
[3] https://lore.kernel.org/linux-hyperv/yq15xnsjlc1.fsf@ca-mkp.ca.oracle.com/
[4] https://lore.kernel.org/linux-hyperv/172808404024.2772330.2975585273609596688.git-patchwork-notify@kernel.org/

> 
> I tested the changes by hacking the construction of cpu_possible_mask
> to include a hole on x86. With a configuration set to demonstrate the
> problem, a Hyper-V guest kernel eventually crashes due to memory
> corruption. After the patches in this series, the crash does not occur.
> 
> [1] https://lore.kernel.org/lkml/SN6PR02MB4157210CC36B2593F8572E5ED4692@SN6PR02MB4157.namprd02.prod.outlook.com/
> 
> Michael Kelley (5):
>   x86/hyperv: Don't assume cpu_possible_mask is dense
>   Drivers: hv: Don't assume cpu_possible_mask is dense
>   iommu/hyper-v: Don't assume cpu_possible_mask is dense
>   scsi: storvsc: Don't assume cpu_possible_mask is dense
>   hv_netvsc: Don't assume cpu_possible_mask is dense
> 
>  arch/x86/hyperv/hv_init.c       |  2 +-
>  drivers/hv/hv_common.c          |  4 ++--
>  drivers/iommu/hyperv-iommu.c    |  4 ++--
>  drivers/net/hyperv/netvsc_drv.c |  2 +-
>  drivers/scsi/storvsc_drv.c      | 13 ++++++-------
>  5 files changed, 12 insertions(+), 13 deletions(-)
> 
> --
> 2.25.1
> 

Re: [PATCH 0/5] hyper-v: Don't assume cpu_possible_mask is dense

[Wei Liu]

On Tue, Dec 10, 2024 at 07:58:34PM +0000, Michael Kelley wrote:
> From: mhkelley58@gmail.com <mhkelley58@gmail.com> Sent: Wednesday, October 2, 2024 8:53 PM
> > 
> > Code specific to Hyper-V guests currently assumes the cpu_possible_mask
> > is "dense" -- i.e., all bit positions 0 thru (nr_cpu_ids - 1) are set,
> > with no "holes". Therefore, num_possible_cpus() is assumed to be equal
> > to nr_cpu_ids.
> > 
> > Per a separate discussion[1], this assumption is not valid in the
> > general case. For example, the function setup_nr_cpu_ids() in
> > kernel/smp.c is coded to assume cpu_possible_mask may be sparse,
> > and other patches have been made in the past to correctly handle
> > the sparseness. See bc75e99983df1efd ("rcu: Correctly handle sparse
> > possible cpu") as noted by Mark Rutland.
> > 
> > The general case notwithstanding, the configurations that Hyper-V
> > provides to guest VMs on x86 and ARM64 hardware, in combination
> > with the algorithms currently used by architecture specific code
> > to assign Linux CPU numbers, *does* always produce a dense
> > cpu_possible_mask. So the invalid assumption is not currently
> > causing failures. But in the interest of correctness, and robustness
> > against future changes in the code that populates cpu_possible_mask,
> > update the Hyper-V code to no longer assume denseness.
> > 
> > The typical code pattern with the invalid assumption is as follows:
> > 
> > 	array = kcalloc(num_possible_cpus(), sizeof(<some struct>),
> > 			GFP_KERNEL);
> > 	....
> > 	index into "array" with smp_processor_id()
> > 
> > In such as case, the array might be indexed by a value beyond the size
> > of the array. The correct approach is to allocate the array with size
> > "nr_cpu_ids". While this will probably leave unused any array entries
> > corresponding to holes in cpu_possible_mask, the holes are assumed to
> > be minimal and hence the amount of memory wasted by unused entries is
> > minimal.
> > 
> > Removing the assumption in Hyper-V code is done in several patches
> > because they touch different kernel subsystems:
> > 
> > Patch 1: Hyper-V x86 initialization of hv_vp_assist_page (there's no
> > 	 hv_vp_assist_page on ARM64)
> > Patch 2: Hyper-V common init of hv_vp_index
> > Patch 3: Hyper-V IOMMU driver
> > Patch 4: storvsc driver
> > Patch 5: netvsc driver
> 
> Wei --
> 
> Could you pick up Patches 1, 2, and 3 in this series for the hyperv-next
> tree? Peter Zijlstra acked the full series [2], and Patches 4 and 5 have
> already been picked by the SCSI and net maintainers respectively [3][4].
> 
> Let me know if you have any concerns.

Michael, I will take a look later after I finish dealing with the
hyperv-fixes branch.

Thanks,
Wei.

> 
> Thanks,
> 
> Michael
> 
> [2] https://lore.kernel.org/linux-hyperv/20241004100742.GO18071@noisy.programming.kicks-ass.net/
> [3] https://lore.kernel.org/linux-hyperv/yq15xnsjlc1.fsf@ca-mkp.ca.oracle.com/
> [4] https://lore.kernel.org/linux-hyperv/172808404024.2772330.2975585273609596688.git-patchwork-notify@kernel.org/
> 
> > 
> > I tested the changes by hacking the construction of cpu_possible_mask
> > to include a hole on x86. With a configuration set to demonstrate the
> > problem, a Hyper-V guest kernel eventually crashes due to memory
> > corruption. After the patches in this series, the crash does not occur.
> > 
> > [1] https://lore.kernel.org/lkml/SN6PR02MB4157210CC36B2593F8572E5ED4692@SN6PR02MB4157.namprd02.prod.outlook.com/
> > 
> > Michael Kelley (5):
> >   x86/hyperv: Don't assume cpu_possible_mask is dense
> >   Drivers: hv: Don't assume cpu_possible_mask is dense
> >   iommu/hyper-v: Don't assume cpu_possible_mask is dense
> >   scsi: storvsc: Don't assume cpu_possible_mask is dense
> >   hv_netvsc: Don't assume cpu_possible_mask is dense
> > 
> >  arch/x86/hyperv/hv_init.c       |  2 +-
> >  drivers/hv/hv_common.c          |  4 ++--
> >  drivers/iommu/hyperv-iommu.c    |  4 ++--
> >  drivers/net/hyperv/netvsc_drv.c |  2 +-
> >  drivers/scsi/storvsc_drv.c      | 13 ++++++-------
> >  5 files changed, 12 insertions(+), 13 deletions(-)
> > 
> > --
> > 2.25.1
> > 

Re: [PATCH 0/5] hyper-v: Don't assume cpu_possible_mask is dense

[Wei Liu]

On Wed, Dec 11, 2024 at 12:14:50AM +0000, Wei Liu wrote:
> On Tue, Dec 10, 2024 at 07:58:34PM +0000, Michael Kelley wrote:
> > From: mhkelley58@gmail.com <mhkelley58@gmail.com> Sent: Wednesday, October 2, 2024 8:53 PM
> > > 
> > > Code specific to Hyper-V guests currently assumes the cpu_possible_mask
> > > is "dense" -- i.e., all bit positions 0 thru (nr_cpu_ids - 1) are set,
> > > with no "holes". Therefore, num_possible_cpus() is assumed to be equal
> > > to nr_cpu_ids.
> > > 
> > > Per a separate discussion[1], this assumption is not valid in the
> > > general case. For example, the function setup_nr_cpu_ids() in
> > > kernel/smp.c is coded to assume cpu_possible_mask may be sparse,
> > > and other patches have been made in the past to correctly handle
> > > the sparseness. See bc75e99983df1efd ("rcu: Correctly handle sparse
> > > possible cpu") as noted by Mark Rutland.
> > > 
> > > The general case notwithstanding, the configurations that Hyper-V
> > > provides to guest VMs on x86 and ARM64 hardware, in combination
> > > with the algorithms currently used by architecture specific code
> > > to assign Linux CPU numbers, *does* always produce a dense
> > > cpu_possible_mask. So the invalid assumption is not currently
> > > causing failures. But in the interest of correctness, and robustness
> > > against future changes in the code that populates cpu_possible_mask,
> > > update the Hyper-V code to no longer assume denseness.
> > > 
> > > The typical code pattern with the invalid assumption is as follows:
> > > 
> > > 	array = kcalloc(num_possible_cpus(), sizeof(<some struct>),
> > > 			GFP_KERNEL);
> > > 	....
> > > 	index into "array" with smp_processor_id()
> > > 
> > > In such as case, the array might be indexed by a value beyond the size
> > > of the array. The correct approach is to allocate the array with size
> > > "nr_cpu_ids". While this will probably leave unused any array entries
> > > corresponding to holes in cpu_possible_mask, the holes are assumed to
> > > be minimal and hence the amount of memory wasted by unused entries is
> > > minimal.
> > > 
> > > Removing the assumption in Hyper-V code is done in several patches
> > > because they touch different kernel subsystems:
> > > 
> > > Patch 1: Hyper-V x86 initialization of hv_vp_assist_page (there's no
> > > 	 hv_vp_assist_page on ARM64)
> > > Patch 2: Hyper-V common init of hv_vp_index
> > > Patch 3: Hyper-V IOMMU driver
> > > Patch 4: storvsc driver
> > > Patch 5: netvsc driver
> > 
> > Wei --
> > 
> > Could you pick up Patches 1, 2, and 3 in this series for the hyperv-next
> > tree? Peter Zijlstra acked the full series [2], and Patches 4 and 5 have
> > already been picked by the SCSI and net maintainers respectively [3][4].
> > 
> > Let me know if you have any concerns.
> 
> Michael, I will take a look later after I finish dealing with the
> hyperv-fixes branch.

Patch 1 to 3 have been applied to the hyperv-next branch.

Wei.

Re: (subset) [PATCH 0/5] hyper-v: Don't assume cpu_possible_mask is dense

[Martin K. Petersen]

On Wed, 02 Oct 2024 20:53:28 -0700, mhkelley58@gmail.com wrote:

> Code specific to Hyper-V guests currently assumes the cpu_possible_mask
> is "dense" -- i.e., all bit positions 0 thru (nr_cpu_ids - 1) are set,
> with no "holes". Therefore, num_possible_cpus() is assumed to be equal
> to nr_cpu_ids.
> 
> Per a separate discussion[1], this assumption is not valid in the
> general case. For example, the function setup_nr_cpu_ids() in
> kernel/smp.c is coded to assume cpu_possible_mask may be sparse,
> and other patches have been made in the past to correctly handle
> the sparseness. See bc75e99983df1efd ("rcu: Correctly handle sparse
> possible cpu") as noted by Mark Rutland.
> 
> [...]

Applied to 6.14/scsi-queue, thanks!

[4/5] scsi: storvsc: Don't assume cpu_possible_mask is dense
      https://git.kernel.org/mkp/scsi/c/6cb7063feb2e

-- 
Martin K. Petersen	Oracle Linux Engineering