[PATCH v2 0/3] Introduce CONFIG_MSHV_ROOT for root partition code

[PATCH v2 0/3] Introduce CONFIG_MSHV_ROOT for root partition code

[Nuno Das Neves]

Running in the root partition is a unique and specialized case that
requires additional code. CONFIG_MSHV_ROOT allows Hyper-V guest kernels
to exclude this code, which is important since significant additional code
specific to the root partition is expected to be added over time.

To do this, change hv_root_partition to be a function which is stubbed out
to return false if CONFIG_MSHV_ROOT=n, and don't compile hv_proc.c at all,
stubbing out those functions with inline versions.

Store the partition type (guest or root) in an enum hv_current_partition_type,
which can be extended beyond just guest and root partition.

While at it, introduce hv_result_to_errno() to convert Hyper-V status codes
to regular linux errors. This is useful because the caller of a hypercall
helper function (such as those in hv_proc.c) usually can't and doesn't
interpret the Hyper-V status, so it is better to convert it to an error code
and reduce the possibility of misinterpreting it. This also alows the stubbed
versions of the hv_proc.c functions to just return a linux error code.

Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
---
Changes in v2:
* Add patch to convert hypercall statuses to linux error codes [Easwar
  Hariharan]
* While at it, split the original patch into two logical pieces, one to change
  hv_root_partition into a function, and one to introduce MSHV_CONFIG_ROOT
* Improve the clarity of and add an error message to
  hv_identify_partition_type() [Easwar Hariharan] [Michael Kelley]
* Better explain *why* the patches are useful, in the commit messages [Michael
  Kelley]
* Add a Kconfig comment explaining why PAGE_SIZE_4KB is needed [Michael Kelley]
* Minor style and typo fixes

Nuno Das Neves (3):
  hyperv: Convert hypercall statuses to linux error codes
  hyperv: Change hv_root_partition into a function
  hyperv: Add CONFIG_MSHV_ROOT to gate root partition support

 arch/arm64/hyperv/mshyperv.c       |  2 +
 arch/x86/hyperv/hv_init.c          | 10 ++---
 arch/x86/kernel/cpu/mshyperv.c     | 24 +----------
 drivers/clocksource/hyperv_timer.c |  4 +-
 drivers/hv/Kconfig                 | 16 +++++++
 drivers/hv/Makefile                |  3 +-
 drivers/hv/hv.c                    | 10 ++---
 drivers/hv/hv_common.c             | 69 +++++++++++++++++++++++++++---
 drivers/hv/hv_proc.c               |  6 +--
 drivers/hv/vmbus_drv.c             |  2 +-
 drivers/iommu/hyperv-iommu.c       |  4 +-
 include/asm-generic/mshyperv.h     | 40 ++++++++++++++---
 12 files changed, 137 insertions(+), 53 deletions(-)

-- 
2.34.1

[PATCH v2 1/3] hyperv: Convert hypercall statuses to linux error codes

[Nuno Das Neves]

Return linux-friendly error codes from hypercall helper functions,
which allows them to be used more flexibly.

Introduce hv_result_to_errno() for this purpose, which also handles
the special value U64_MAX returned from hv_do_hypercall().

Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
---
 drivers/hv/hv_common.c         | 34 ++++++++++++++++++++++++++++++++++
 drivers/hv/hv_proc.c           |  6 +++---
 include/asm-generic/mshyperv.h |  1 +
 3 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index ee3083937b4f..2120aead98d9 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -683,3 +683,37 @@ u64 __weak hv_tdx_hypercall(u64 control, u64 param1, u64 param2)
 	return HV_STATUS_INVALID_PARAMETER;
 }
 EXPORT_SYMBOL_GPL(hv_tdx_hypercall);
+
+/* Convert a hypercall result into a linux-friendly error code. */
+int hv_result_to_errno(u64 status)
+{
+	/* hv_do_hypercall() may return U64_MAX, hypercalls aren't possible */
+	if (unlikely(status == U64_MAX))
+		return -EOPNOTSUPP;
+	/*
+	 * A failed hypercall is usually only recoverable (or loggable) near
+	 * the call site where the HV_STATUS_* code is known. So the errno
+	 * it gets converted to is not too useful further up the stack.
+	 * Provice a few mappings that could be useful, and revert to -EIO
+	 * as a fallback.
+	 */
+	switch (hv_result(status)) {
+	case HV_STATUS_SUCCESS:
+		return 0;
+	case HV_STATUS_INVALID_HYPERCALL_CODE:
+	case HV_STATUS_INVALID_HYPERCALL_INPUT:
+	case HV_STATUS_INVALID_PARAMETER:
+	case HV_STATUS_INVALID_PARTITION_ID:
+	case HV_STATUS_INVALID_VP_INDEX:
+	case HV_STATUS_INVALID_PORT_ID:
+	case HV_STATUS_INVALID_CONNECTION_ID:
+	case HV_STATUS_INVALID_LP_INDEX:
+	case HV_STATUS_INVALID_REGISTER_VALUE:
+		return -EINVAL;
+	case HV_STATUS_INSUFFICIENT_MEMORY:
+		return -ENOMEM;
+	default:
+		break;
+	}
+	return -EIO;
+}
diff --git a/drivers/hv/hv_proc.c b/drivers/hv/hv_proc.c
index 3e410489f480..72b09f1cfa3e 100644
--- a/drivers/hv/hv_proc.c
+++ b/drivers/hv/hv_proc.c
@@ -88,7 +88,7 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
 	local_irq_restore(flags);
 	if (!hv_result_success(status)) {
 		pr_err("Failed to deposit pages: %lld\n", status);
-		ret = hv_result(status);
+		ret = hv_result_to_errno(status);
 		goto err_free_allocations;
 	}
 
@@ -139,7 +139,7 @@ int hv_call_add_logical_proc(int node, u32 lp_index, u32 apic_id)
 			if (!hv_result_success(status)) {
 				pr_err("%s: cpu %u apic ID %u, %lld\n", __func__,
 				       lp_index, apic_id, status);
-				ret = hv_result(status);
+				ret = hv_result_to_errno(status);
 			}
 			break;
 		}
@@ -181,7 +181,7 @@ int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags)
 			if (!hv_result_success(status)) {
 				pr_err("%s: vcpu %u, lp %u, %lld\n", __func__,
 				       vp_index, flags, status);
-				ret = hv_result(status);
+				ret = hv_result_to_errno(status);
 			}
 			break;
 		}
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index 7adc10a4fa3e..3f115e2bcdaa 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -297,6 +297,7 @@ static inline int cpumask_to_vpset_skip(struct hv_vpset *vpset,
 	return __cpumask_to_vpset(vpset, cpus, func);
 }
 
+int hv_result_to_errno(u64 status);
 void hyperv_report_panic(struct pt_regs *regs, long err, bool in_die);
 bool hv_is_hyperv_initialized(void);
 bool hv_is_hibernation_supported(void);
-- 
2.34.1

Re: [PATCH v2 1/3] hyperv: Convert hypercall statuses to linux error codes

[Easwar Hariharan]

On 2/20/2025 10:33 AM, Nuno Das Neves wrote:
> Return linux-friendly error codes from hypercall helper functions,
> which allows them to be used more flexibly.
> 
> Introduce hv_result_to_errno() for this purpose, which also handles
> the special value U64_MAX returned from hv_do_hypercall().
> 
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> ---
>  drivers/hv/hv_common.c         | 34 ++++++++++++++++++++++++++++++++++
>  drivers/hv/hv_proc.c           |  6 +++---
>  include/asm-generic/mshyperv.h |  1 +
>  3 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index ee3083937b4f..2120aead98d9 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -683,3 +683,37 @@ u64 __weak hv_tdx_hypercall(u64 control, u64 param1, u64 param2)
>  	return HV_STATUS_INVALID_PARAMETER;
>  }
>  EXPORT_SYMBOL_GPL(hv_tdx_hypercall);
> +
> +/* Convert a hypercall result into a linux-friendly error code. */
> +int hv_result_to_errno(u64 status)
> +{
> +	/* hv_do_hypercall() may return U64_MAX, hypercalls aren't possible */
> +	if (unlikely(status == U64_MAX))
> +		return -EOPNOTSUPP;
> +	/*
> +	 * A failed hypercall is usually only recoverable (or loggable) near
> +	 * the call site where the HV_STATUS_* code is known. So the errno
> +	 * it gets converted to is not too useful further up the stack.
> +	 * Provice a few mappings that could be useful, and revert to -EIO

Typo: Provide

Otherwise, looks good to me.

Reviewed-by: Easwar Hariharan <eahariha@linux.microsoft.com>

Thanks,
Easwar (he/him)

Re: [PATCH v2 1/3] hyperv: Convert hypercall statuses to linux error codes

[Nuno Das Neves]

On 2/20/2025 10:49 AM, Easwar Hariharan wrote:
> On 2/20/2025 10:33 AM, Nuno Das Neves wrote:
>> Return linux-friendly error codes from hypercall helper functions,
>> which allows them to be used more flexibly.
>>
>> Introduce hv_result_to_errno() for this purpose, which also handles
>> the special value U64_MAX returned from hv_do_hypercall().
>>
>> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
>> ---
>>  drivers/hv/hv_common.c         | 34 ++++++++++++++++++++++++++++++++++
>>  drivers/hv/hv_proc.c           |  6 +++---
>>  include/asm-generic/mshyperv.h |  1 +
>>  3 files changed, 38 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
>> index ee3083937b4f..2120aead98d9 100644
>> --- a/drivers/hv/hv_common.c
>> +++ b/drivers/hv/hv_common.c
>> @@ -683,3 +683,37 @@ u64 __weak hv_tdx_hypercall(u64 control, u64 param1, u64 param2)
>>  	return HV_STATUS_INVALID_PARAMETER;
>>  }
>>  EXPORT_SYMBOL_GPL(hv_tdx_hypercall);
>> +
>> +/* Convert a hypercall result into a linux-friendly error code. */
>> +int hv_result_to_errno(u64 status)
>> +{
>> +	/* hv_do_hypercall() may return U64_MAX, hypercalls aren't possible */
>> +	if (unlikely(status == U64_MAX))
>> +		return -EOPNOTSUPP;
>> +	/*
>> +	 * A failed hypercall is usually only recoverable (or loggable) near
>> +	 * the call site where the HV_STATUS_* code is known. So the errno
>> +	 * it gets converted to is not too useful further up the stack.
>> +	 * Provice a few mappings that could be useful, and revert to -EIO
> 
> Typo: Provide
> 
Yep, looks like there will be a v3, I'll fix it there.
Thanks for the review!

> Otherwise, looks good to me.
> 
> Reviewed-by: Easwar Hariharan <eahariha@linux.microsoft.com>
> 
> Thanks,
> Easwar (he/him)

RE: [PATCH v2 1/3] hyperv: Convert hypercall statuses to linux error codes

[Michael Kelley]

From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Thursday, February 20, 2025 10:33 AM
> 
> Return linux-friendly error codes from hypercall helper functions,
> which allows them to be used more flexibly.
> 
> Introduce hv_result_to_errno() for this purpose, which also handles
> the special value U64_MAX returned from hv_do_hypercall().
> 
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> ---
>  drivers/hv/hv_common.c         | 34 ++++++++++++++++++++++++++++++++++
>  drivers/hv/hv_proc.c           |  6 +++---
>  include/asm-generic/mshyperv.h |  1 +
>  3 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index ee3083937b4f..2120aead98d9 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -683,3 +683,37 @@ u64 __weak hv_tdx_hypercall(u64 control, u64 param1, u64 param2)
>  	return HV_STATUS_INVALID_PARAMETER;
>  }
>  EXPORT_SYMBOL_GPL(hv_tdx_hypercall);
> +
> +/* Convert a hypercall result into a linux-friendly error code. */
> +int hv_result_to_errno(u64 status)
> +{
> +	/* hv_do_hypercall() may return U64_MAX, hypercalls aren't possible */
> +	if (unlikely(status == U64_MAX))
> +		return -EOPNOTSUPP;
> +	/*
> +	 * A failed hypercall is usually only recoverable (or loggable) near
> +	 * the call site where the HV_STATUS_* code is known. So the errno
> +	 * it gets converted to is not too useful further up the stack.
> +	 * Provice a few mappings that could be useful, and revert to -EIO
> +	 * as a fallback.
> +	 */
> +	switch (hv_result(status)) {
> +	case HV_STATUS_SUCCESS:
> +		return 0;
> +	case HV_STATUS_INVALID_HYPERCALL_CODE:
> +	case HV_STATUS_INVALID_HYPERCALL_INPUT:
> +	case HV_STATUS_INVALID_PARAMETER:
> +	case HV_STATUS_INVALID_PARTITION_ID:
> +	case HV_STATUS_INVALID_VP_INDEX:
> +	case HV_STATUS_INVALID_PORT_ID:
> +	case HV_STATUS_INVALID_CONNECTION_ID:
> +	case HV_STATUS_INVALID_LP_INDEX:
> +	case HV_STATUS_INVALID_REGISTER_VALUE:
> +		return -EINVAL;
> +	case HV_STATUS_INSUFFICIENT_MEMORY:
> +		return -ENOMEM;
> +	default:
> +		break;
> +	}
> +	return -EIO;
> +}
> diff --git a/drivers/hv/hv_proc.c b/drivers/hv/hv_proc.c
> index 3e410489f480..72b09f1cfa3e 100644
> --- a/drivers/hv/hv_proc.c
> +++ b/drivers/hv/hv_proc.c
> @@ -88,7 +88,7 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
>  	local_irq_restore(flags);
>  	if (!hv_result_success(status)) {
>  		pr_err("Failed to deposit pages: %lld\n", status);
> -		ret = hv_result(status);
> +		ret = hv_result_to_errno(status);
>  		goto err_free_allocations;
>  	}
> 
> @@ -139,7 +139,7 @@ int hv_call_add_logical_proc(int node, u32 lp_index, u32 apic_id)
>  			if (!hv_result_success(status)) {
>  				pr_err("%s: cpu %u apic ID %u, %lld\n", __func__,
>  				       lp_index, apic_id, status);
> -				ret = hv_result(status);
> +				ret = hv_result_to_errno(status);
>  			}
>  			break;
>  		}
> @@ -181,7 +181,7 @@ int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags)
>  			if (!hv_result_success(status)) {
>  				pr_err("%s: vcpu %u, lp %u, %lld\n", __func__,
>  				       vp_index, flags, status);
> -				ret = hv_result(status);
> +				ret = hv_result_to_errno(status);
>  			}
>  			break;
>  		}

In hv_call_add_logical_proc() and hv_call_create_vp(), local variable "ret" is
defined and initialized to HV_STATUS_SUCCESS.  Since "ret" now contains an
errno value instead of a hypercall status, it should be initialized to zero.  (Yes,
HV_STATUS_SUCCESS is defined as 0, but it's now the wrong abstraction.)

With that fixed,

Reviewed-by: Michael Kelley <mhklinux@outlook.com>

> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index 7adc10a4fa3e..3f115e2bcdaa 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -297,6 +297,7 @@ static inline int cpumask_to_vpset_skip(struct hv_vpset *vpset,
>  	return __cpumask_to_vpset(vpset, cpus, func);
>  }
> 
> +int hv_result_to_errno(u64 status);
>  void hyperv_report_panic(struct pt_regs *regs, long err, bool in_die);
>  bool hv_is_hyperv_initialized(void);
>  bool hv_is_hibernation_supported(void);
> --
> 2.34.1

Re: [PATCH v2 1/3] hyperv: Convert hypercall statuses to linux error codes

[Nuno Das Neves]

On 2/20/2025 11:03 AM, Michael Kelley wrote:
> From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Thursday, February 20, 2025 10:33 AM
>>
>> Return linux-friendly error codes from hypercall helper functions,
>> which allows them to be used more flexibly.
>>
>> Introduce hv_result_to_errno() for this purpose, which also handles
>> the special value U64_MAX returned from hv_do_hypercall().
>>
>> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
>> ---
>>  drivers/hv/hv_common.c         | 34 ++++++++++++++++++++++++++++++++++
>>  drivers/hv/hv_proc.c           |  6 +++---
>>  include/asm-generic/mshyperv.h |  1 +
>>  3 files changed, 38 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
>> index ee3083937b4f..2120aead98d9 100644
>> --- a/drivers/hv/hv_common.c
>> +++ b/drivers/hv/hv_common.c
>> @@ -683,3 +683,37 @@ u64 __weak hv_tdx_hypercall(u64 control, u64 param1, u64 param2)
>>  	return HV_STATUS_INVALID_PARAMETER;
>>  }
>>  EXPORT_SYMBOL_GPL(hv_tdx_hypercall);
>> +
>> +/* Convert a hypercall result into a linux-friendly error code. */
>> +int hv_result_to_errno(u64 status)
>> +{
>> +	/* hv_do_hypercall() may return U64_MAX, hypercalls aren't possible */
>> +	if (unlikely(status == U64_MAX))
>> +		return -EOPNOTSUPP;
>> +	/*
>> +	 * A failed hypercall is usually only recoverable (or loggable) near
>> +	 * the call site where the HV_STATUS_* code is known. So the errno
>> +	 * it gets converted to is not too useful further up the stack.
>> +	 * Provice a few mappings that could be useful, and revert to -EIO
>> +	 * as a fallback.
>> +	 */
>> +	switch (hv_result(status)) {
>> +	case HV_STATUS_SUCCESS:
>> +		return 0;
>> +	case HV_STATUS_INVALID_HYPERCALL_CODE:
>> +	case HV_STATUS_INVALID_HYPERCALL_INPUT:
>> +	case HV_STATUS_INVALID_PARAMETER:
>> +	case HV_STATUS_INVALID_PARTITION_ID:
>> +	case HV_STATUS_INVALID_VP_INDEX:
>> +	case HV_STATUS_INVALID_PORT_ID:
>> +	case HV_STATUS_INVALID_CONNECTION_ID:
>> +	case HV_STATUS_INVALID_LP_INDEX:
>> +	case HV_STATUS_INVALID_REGISTER_VALUE:
>> +		return -EINVAL;
>> +	case HV_STATUS_INSUFFICIENT_MEMORY:
>> +		return -ENOMEM;
>> +	default:
>> +		break;
>> +	}
>> +	return -EIO;
>> +}
>> diff --git a/drivers/hv/hv_proc.c b/drivers/hv/hv_proc.c
>> index 3e410489f480..72b09f1cfa3e 100644
>> --- a/drivers/hv/hv_proc.c
>> +++ b/drivers/hv/hv_proc.c
>> @@ -88,7 +88,7 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
>>  	local_irq_restore(flags);
>>  	if (!hv_result_success(status)) {
>>  		pr_err("Failed to deposit pages: %lld\n", status);
>> -		ret = hv_result(status);
>> +		ret = hv_result_to_errno(status);
>>  		goto err_free_allocations;
>>  	}
>>
>> @@ -139,7 +139,7 @@ int hv_call_add_logical_proc(int node, u32 lp_index, u32 apic_id)
>>  			if (!hv_result_success(status)) {
>>  				pr_err("%s: cpu %u apic ID %u, %lld\n", __func__,
>>  				       lp_index, apic_id, status);
>> -				ret = hv_result(status);
>> +				ret = hv_result_to_errno(status);
>>  			}
>>  			break;
>>  		}
>> @@ -181,7 +181,7 @@ int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags)
>>  			if (!hv_result_success(status)) {
>>  				pr_err("%s: vcpu %u, lp %u, %lld\n", __func__,
>>  				       vp_index, flags, status);
>> -				ret = hv_result(status);
>> +				ret = hv_result_to_errno(status);
>>  			}
>>  			break;
>>  		}
> 
> In hv_call_add_logical_proc() and hv_call_create_vp(), local variable "ret" is
> defined and initialized to HV_STATUS_SUCCESS.  Since "ret" now contains an
> errno value instead of a hypercall status, it should be initialized to zero.  (Yes,
> HV_STATUS_SUCCESS is defined as 0, but it's now the wrong abstraction.)
> 
Great point, I'll post a v3 to fix it.

> With that fixed,
> 
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> 
>> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
>> index 7adc10a4fa3e..3f115e2bcdaa 100644
>> --- a/include/asm-generic/mshyperv.h
>> +++ b/include/asm-generic/mshyperv.h
>> @@ -297,6 +297,7 @@ static inline int cpumask_to_vpset_skip(struct hv_vpset *vpset,
>>  	return __cpumask_to_vpset(vpset, cpus, func);
>>  }
>>
>> +int hv_result_to_errno(u64 status);
>>  void hyperv_report_panic(struct pt_regs *regs, long err, bool in_die);
>>  bool hv_is_hyperv_initialized(void);
>>  bool hv_is_hibernation_supported(void);
>> --
>> 2.34.1

[PATCH v2 2/3] hyperv: Change hv_root_partition into a function

[Nuno Das Neves]

Introduce hv_current_partition_type to store the partition type
as an enum.

Right now this is limited to guest or root partition, but there will
be other kinds in future and the enum is easily extensible.

Set up hv_current_partition_type early in Hyper-V initialization with
hv_identify_partition_type(). hv_root_partition() just queries this
value, and shouldn't be called before that.

Making this check into a function sets the stage for adding a config
option to gate the compilation of root partition code. In particular,
hv_root_partition() can be stubbed out always be false if root
partition support isn't desired.

Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
---
 arch/arm64/hyperv/mshyperv.c       |  2 ++
 arch/x86/hyperv/hv_init.c          | 10 ++++-----
 arch/x86/kernel/cpu/mshyperv.c     | 24 ++------------------
 drivers/clocksource/hyperv_timer.c |  4 ++--
 drivers/hv/hv.c                    | 10 ++++-----
 drivers/hv/hv_common.c             | 35 +++++++++++++++++++++++++-----
 drivers/hv/vmbus_drv.c             |  2 +-
 drivers/iommu/hyperv-iommu.c       |  4 ++--
 include/asm-generic/mshyperv.h     | 15 +++++++++++--
 9 files changed, 61 insertions(+), 45 deletions(-)

diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
index 29fcfd595f48..2265ea5ce5ad 100644
--- a/arch/arm64/hyperv/mshyperv.c
+++ b/arch/arm64/hyperv/mshyperv.c
@@ -61,6 +61,8 @@ static int __init hyperv_init(void)
 		ms_hyperv.features, ms_hyperv.priv_high, ms_hyperv.hints,
 		ms_hyperv.misc_features);
 
+	hv_identify_partition_type();
+
 	ret = hv_common_init();
 	if (ret)
 		return ret;
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 9be1446f5bd3..ddeb40930bc8 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -90,7 +90,7 @@ static int hv_cpu_init(unsigned int cpu)
 		return 0;
 
 	hvp = &hv_vp_assist_page[cpu];
-	if (hv_root_partition) {
+	if (hv_root_partition()) {
 		/*
 		 * For root partition we get the hypervisor provided VP assist
 		 * page, instead of allocating a new page.
@@ -242,7 +242,7 @@ static int hv_cpu_die(unsigned int cpu)
 
 	if (hv_vp_assist_page && hv_vp_assist_page[cpu]) {
 		union hv_vp_assist_msr_contents msr = { 0 };
-		if (hv_root_partition) {
+		if (hv_root_partition()) {
 			/*
 			 * For root partition the VP assist page is mapped to
 			 * hypervisor provided page, and thus we unmap the
@@ -317,7 +317,7 @@ static int hv_suspend(void)
 	union hv_x64_msr_hypercall_contents hypercall_msr;
 	int ret;
 
-	if (hv_root_partition)
+	if (hv_root_partition())
 		return -EPERM;
 
 	/*
@@ -518,7 +518,7 @@ void __init hyperv_init(void)
 	rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
 	hypercall_msr.enable = 1;
 
-	if (hv_root_partition) {
+	if (hv_root_partition()) {
 		struct page *pg;
 		void *src;
 
@@ -592,7 +592,7 @@ void __init hyperv_init(void)
 	 * If we're running as root, we want to create our own PCI MSI domain.
 	 * We can't set this in hv_pci_init because that would be too late.
 	 */
-	if (hv_root_partition)
+	if (hv_root_partition())
 		x86_init.irqs.create_pci_msi_domain = hv_create_pci_msi_domain;
 #endif
 
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index f285757618fc..4f01f424ea5b 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -33,8 +33,6 @@
 #include <asm/numa.h>
 #include <asm/svm.h>
 
-/* Is Linux running as the root partition? */
-bool hv_root_partition;
 /* Is Linux running on nested Microsoft Hypervisor */
 bool hv_nested;
 struct ms_hyperv_info ms_hyperv;
@@ -451,25 +449,7 @@ static void __init ms_hyperv_init_platform(void)
 	pr_debug("Hyper-V: max %u virtual processors, %u logical processors\n",
 		 ms_hyperv.max_vp_index, ms_hyperv.max_lp_index);
 
-	/*
-	 * Check CPU management privilege.
-	 *
-	 * To mirror what Windows does we should extract CPU management
-	 * features and use the ReservedIdentityBit to detect if Linux is the
-	 * root partition. But that requires negotiating CPU management
-	 * interface (a process to be finalized). For now, use the privilege
-	 * flag as the indicator for running as root.
-	 *
-	 * Hyper-V should never specify running as root and as a Confidential
-	 * VM. But to protect against a compromised/malicious Hyper-V trying
-	 * to exploit root behavior to expose Confidential VM memory, ignore
-	 * the root partition setting if also a Confidential VM.
-	 */
-	if ((ms_hyperv.priv_high & HV_CPU_MANAGEMENT) &&
-	    !(ms_hyperv.priv_high & HV_ISOLATION)) {
-		hv_root_partition = true;
-		pr_info("Hyper-V: running as root partition\n");
-	}
+	hv_identify_partition_type();
 
 	if (ms_hyperv.hints & HV_X64_HYPERV_NESTED) {
 		hv_nested = true;
@@ -618,7 +598,7 @@ static void __init ms_hyperv_init_platform(void)
 
 # ifdef CONFIG_SMP
 	smp_ops.smp_prepare_boot_cpu = hv_smp_prepare_boot_cpu;
-	if (hv_root_partition ||
+	if (hv_root_partition() ||
 	    (!ms_hyperv.paravisor_present && hv_isolation_type_snp()))
 		smp_ops.smp_prepare_cpus = hv_smp_prepare_cpus;
 # endif
diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
index f00019b078a7..09549451dd51 100644
--- a/drivers/clocksource/hyperv_timer.c
+++ b/drivers/clocksource/hyperv_timer.c
@@ -582,7 +582,7 @@ static void __init hv_init_tsc_clocksource(void)
 	 * mapped.
 	 */
 	tsc_msr.as_uint64 = hv_get_msr(HV_MSR_REFERENCE_TSC);
-	if (hv_root_partition)
+	if (hv_root_partition())
 		tsc_pfn = tsc_msr.pfn;
 	else
 		tsc_pfn = HVPFN_DOWN(virt_to_phys(tsc_page));
@@ -627,7 +627,7 @@ void __init hv_remap_tsc_clocksource(void)
 	if (!(ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE))
 		return;
 
-	if (!hv_root_partition) {
+	if (!hv_root_partition()) {
 		WARN(1, "%s: attempt to remap TSC page in guest partition\n",
 		     __func__);
 		return;
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index fab0690b5c41..a38f84548bc2 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -144,7 +144,7 @@ int hv_synic_alloc(void)
 		 * Synic message and event pages are allocated by paravisor.
 		 * Skip these pages allocation here.
 		 */
-		if (!ms_hyperv.paravisor_present && !hv_root_partition) {
+		if (!ms_hyperv.paravisor_present && !hv_root_partition()) {
 			hv_cpu->synic_message_page =
 				(void *)get_zeroed_page(GFP_ATOMIC);
 			if (!hv_cpu->synic_message_page) {
@@ -272,7 +272,7 @@ void hv_synic_enable_regs(unsigned int cpu)
 	simp.as_uint64 = hv_get_msr(HV_MSR_SIMP);
 	simp.simp_enabled = 1;
 
-	if (ms_hyperv.paravisor_present || hv_root_partition) {
+	if (ms_hyperv.paravisor_present || hv_root_partition()) {
 		/* Mask out vTOM bit. ioremap_cache() maps decrypted */
 		u64 base = (simp.base_simp_gpa << HV_HYP_PAGE_SHIFT) &
 				~ms_hyperv.shared_gpa_boundary;
@@ -291,7 +291,7 @@ void hv_synic_enable_regs(unsigned int cpu)
 	siefp.as_uint64 = hv_get_msr(HV_MSR_SIEFP);
 	siefp.siefp_enabled = 1;
 
-	if (ms_hyperv.paravisor_present || hv_root_partition) {
+	if (ms_hyperv.paravisor_present || hv_root_partition()) {
 		/* Mask out vTOM bit. ioremap_cache() maps decrypted */
 		u64 base = (siefp.base_siefp_gpa << HV_HYP_PAGE_SHIFT) &
 				~ms_hyperv.shared_gpa_boundary;
@@ -367,7 +367,7 @@ void hv_synic_disable_regs(unsigned int cpu)
 	 * addresses.
 	 */
 	simp.simp_enabled = 0;
-	if (ms_hyperv.paravisor_present || hv_root_partition) {
+	if (ms_hyperv.paravisor_present || hv_root_partition()) {
 		iounmap(hv_cpu->synic_message_page);
 		hv_cpu->synic_message_page = NULL;
 	} else {
@@ -379,7 +379,7 @@ void hv_synic_disable_regs(unsigned int cpu)
 	siefp.as_uint64 = hv_get_msr(HV_MSR_SIEFP);
 	siefp.siefp_enabled = 0;
 
-	if (ms_hyperv.paravisor_present || hv_root_partition) {
+	if (ms_hyperv.paravisor_present || hv_root_partition()) {
 		iounmap(hv_cpu->synic_event_page);
 		hv_cpu->synic_event_page = NULL;
 	} else {
diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index 2120aead98d9..c5c5dc92ff21 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -34,8 +34,11 @@
 u64 hv_current_partition_id = HV_PARTITION_ID_SELF;
 EXPORT_SYMBOL_GPL(hv_current_partition_id);
 
+enum hv_partition_type hv_current_partition_type;
+EXPORT_SYMBOL_GPL(hv_current_partition_type);
+
 /*
- * hv_root_partition, ms_hyperv and hv_nested are defined here with other
+ * ms_hyperv and hv_nested are defined here with other
  * Hyper-V specific globals so they are shared across all architectures and are
  * built only when CONFIG_HYPERV is defined.  But on x86,
  * ms_hyperv_init_platform() is built even when CONFIG_HYPERV is not
@@ -43,9 +46,6 @@ EXPORT_SYMBOL_GPL(hv_current_partition_id);
  * here, allowing for an overriding definition in the module containing
  * ms_hyperv_init_platform().
  */
-bool __weak hv_root_partition;
-EXPORT_SYMBOL_GPL(hv_root_partition);
-
 bool __weak hv_nested;
 EXPORT_SYMBOL_GPL(hv_nested);
 
@@ -283,7 +283,7 @@ static void hv_kmsg_dump_register(void)
 
 static inline bool hv_output_page_exists(void)
 {
-	return hv_root_partition || IS_ENABLED(CONFIG_HYPERV_VTL_MODE);
+	return hv_root_partition() || IS_ENABLED(CONFIG_HYPERV_VTL_MODE);
 }
 
 void __init hv_get_partition_id(void)
@@ -594,7 +594,7 @@ EXPORT_SYMBOL_GPL(hv_setup_dma_ops);
 
 bool hv_is_hibernation_supported(void)
 {
-	return !hv_root_partition && acpi_sleep_state_supported(ACPI_STATE_S4);
+	return !hv_root_partition() && acpi_sleep_state_supported(ACPI_STATE_S4);
 }
 EXPORT_SYMBOL_GPL(hv_is_hibernation_supported);
 
@@ -717,3 +717,26 @@ int hv_result_to_errno(u64 status)
 	}
 	return -EIO;
 }
+
+void hv_identify_partition_type(void)
+{
+	/* Assume guest role */
+	hv_current_partition_type = HV_PARTITION_TYPE_GUEST;
+	/*
+	 * Check partition creation and cpu management privileges
+	 *
+	 * Hyper-V should never specify running as root and as a Confidential
+	 * VM. But to protect against a compromised/malicious Hyper-V trying
+	 * to exploit root behavior to expose Confidential VM memory, ignore
+	 * the root partition setting if also a Confidential VM.
+	 */
+	if ((ms_hyperv.priv_high & HV_CREATE_PARTITIONS) &&
+	    (ms_hyperv.priv_high & HV_CPU_MANAGEMENT) &&
+	    !(ms_hyperv.priv_high & HV_ISOLATION)) {
+		pr_info("Hyper-V: running as root partition\n");
+		if (IS_ENABLED(CONFIG_MSHV_ROOT))
+			hv_current_partition_type = HV_PARTITION_TYPE_ROOT;
+		else
+			pr_crit("Hyper-V: CONFIG_MSHV_ROOT not enabled!\n");
+	}
+}
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 75eb1390b45c..22afebfc28ff 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -2656,7 +2656,7 @@ static int __init hv_acpi_init(void)
 	if (!hv_is_hyperv_initialized())
 		return -ENODEV;
 
-	if (hv_root_partition && !hv_nested)
+	if (hv_root_partition() && !hv_nested)
 		return 0;
 
 	/*
diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
index 2a86aa5d54c6..53e4b37716af 100644
--- a/drivers/iommu/hyperv-iommu.c
+++ b/drivers/iommu/hyperv-iommu.c
@@ -130,7 +130,7 @@ static int __init hyperv_prepare_irq_remapping(void)
 	    x86_init.hyper.msi_ext_dest_id())
 		return -ENODEV;
 
-	if (hv_root_partition) {
+	if (hv_root_partition()) {
 		name = "HYPERV-ROOT-IR";
 		ops = &hyperv_root_ir_domain_ops;
 	} else {
@@ -151,7 +151,7 @@ static int __init hyperv_prepare_irq_remapping(void)
 		return -ENOMEM;
 	}
 
-	if (hv_root_partition)
+	if (hv_root_partition())
 		return 0; /* The rest is only relevant to guests */
 
 	/*
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index 3f115e2bcdaa..d2b1a8fc074c 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -28,6 +28,11 @@
 
 #define VTPM_BASE_ADDRESS 0xfed40000
 
+enum hv_partition_type {
+	HV_PARTITION_TYPE_GUEST,
+	HV_PARTITION_TYPE_ROOT,
+};
+
 struct ms_hyperv_info {
 	u32 features;
 	u32 priv_high;
@@ -59,6 +64,7 @@ struct ms_hyperv_info {
 extern struct ms_hyperv_info ms_hyperv;
 extern bool hv_nested;
 extern u64 hv_current_partition_id;
+extern enum hv_partition_type hv_current_partition_type;
 
 extern void * __percpu *hyperv_pcpu_input_arg;
 extern void * __percpu *hyperv_pcpu_output_arg;
@@ -190,8 +196,6 @@ void hv_remove_crash_handler(void);
 extern int vmbus_interrupt;
 extern int vmbus_irq;
 
-extern bool hv_root_partition;
-
 #if IS_ENABLED(CONFIG_HYPERV)
 /*
  * Hypervisor's notion of virtual processor ID is different from
@@ -213,6 +217,7 @@ void __init hv_common_free(void);
 void __init ms_hyperv_late_init(void);
 int hv_common_cpu_init(unsigned int cpu);
 int hv_common_cpu_die(unsigned int cpu);
+void hv_identify_partition_type(void);
 
 void *hv_alloc_hyperv_page(void);
 void *hv_alloc_hyperv_zeroed_page(void);
@@ -310,6 +315,7 @@ void hyperv_cleanup(void);
 bool hv_query_ext_cap(u64 cap_query);
 void hv_setup_dma_ops(struct device *dev, bool coherent);
 #else /* CONFIG_HYPERV */
+static inline void hv_identify_partition_type(void) {}
 static inline bool hv_is_hyperv_initialized(void) { return false; }
 static inline bool hv_is_hibernation_supported(void) { return false; }
 static inline void hyperv_cleanup(void) {}
@@ -321,4 +327,9 @@ static inline enum hv_isolation_type hv_get_isolation_type(void)
 }
 #endif /* CONFIG_HYPERV */
 
+static inline bool hv_root_partition(void)
+{
+	return hv_current_partition_type == HV_PARTITION_TYPE_ROOT;
+}
+
 #endif
-- 
2.34.1

Re: [PATCH v2 2/3] hyperv: Change hv_root_partition into a function

[Easwar Hariharan]

On 2/20/2025 10:33 AM, Nuno Das Neves wrote:
> Introduce hv_current_partition_type to store the partition type
> as an enum.
> 
> Right now this is limited to guest or root partition, but there will
> be other kinds in future and the enum is easily extensible.
> 
> Set up hv_current_partition_type early in Hyper-V initialization with
> hv_identify_partition_type(). hv_root_partition() just queries this
> value, and shouldn't be called before that.
> 
> Making this check into a function sets the stage for adding a config
> option to gate the compilation of root partition code. In particular,
> hv_root_partition() can be stubbed out always be false if root
> partition support isn't desired.
> 
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> ---
>  arch/arm64/hyperv/mshyperv.c       |  2 ++
>  arch/x86/hyperv/hv_init.c          | 10 ++++-----
>  arch/x86/kernel/cpu/mshyperv.c     | 24 ++------------------
>  drivers/clocksource/hyperv_timer.c |  4 ++--
>  drivers/hv/hv.c                    | 10 ++++-----
>  drivers/hv/hv_common.c             | 35 +++++++++++++++++++++++++-----
>  drivers/hv/vmbus_drv.c             |  2 +-
>  drivers/iommu/hyperv-iommu.c       |  4 ++--
>  include/asm-generic/mshyperv.h     | 15 +++++++++++--
>  9 files changed, 61 insertions(+), 45 deletions(-)

Looks good to me.

Reviewed-by: Easwar Hariharan <eahariha@linux.microsoft.com>

RE: [PATCH v2 2/3] hyperv: Change hv_root_partition into a function

[Michael Kelley]

From: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> 
> Introduce hv_current_partition_type to store the partition type
> as an enum.
> 
> Right now this is limited to guest or root partition, but there will
> be other kinds in future and the enum is easily extensible.
> 
> Set up hv_current_partition_type early in Hyper-V initialization with
> hv_identify_partition_type(). hv_root_partition() just queries this
> value, and shouldn't be called before that.
> 
> Making this check into a function sets the stage for adding a config
> option to gate the compilation of root partition code. In particular,
> hv_root_partition() can be stubbed out always be false if root
> partition support isn't desired.
> 
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>

Reviewed-by: Michael Kelley <mhklinux@outlook.com>

> ---
>  arch/arm64/hyperv/mshyperv.c       |  2 ++
>  arch/x86/hyperv/hv_init.c          | 10 ++++-----
>  arch/x86/kernel/cpu/mshyperv.c     | 24 ++------------------
>  drivers/clocksource/hyperv_timer.c |  4 ++--
>  drivers/hv/hv.c                    | 10 ++++-----
>  drivers/hv/hv_common.c             | 35 +++++++++++++++++++++++++-----
>  drivers/hv/vmbus_drv.c             |  2 +-
>  drivers/iommu/hyperv-iommu.c       |  4 ++--
>  include/asm-generic/mshyperv.h     | 15 +++++++++++--
>  9 files changed, 61 insertions(+), 45 deletions(-)
> 

[snip]

RE: [PATCH v2 2/3] hyperv: Change hv_root_partition into a function

[Michael Kelley]

From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Thursday, February 20, 2025 10:33 AM
> 
> Introduce hv_current_partition_type to store the partition type
> as an enum.
> 
> Right now this is limited to guest or root partition, but there will
> be other kinds in future and the enum is easily extensible.
> 
> Set up hv_current_partition_type early in Hyper-V initialization with
> hv_identify_partition_type(). hv_root_partition() just queries this
> value, and shouldn't be called before that.
> 
> Making this check into a function sets the stage for adding a config
> option to gate the compilation of root partition code. In particular,
> hv_root_partition() can be stubbed out always be false if root
> partition support isn't desired.
> 
>

[snip]
 
> +void hv_identify_partition_type(void)
> +{
> +	/* Assume guest role */
> +	hv_current_partition_type = HV_PARTITION_TYPE_GUEST;
> +	/*
> +	 * Check partition creation and cpu management privileges
> +	 *
> +	 * Hyper-V should never specify running as root and as a Confidential
> +	 * VM. But to protect against a compromised/malicious Hyper-V trying
> +	 * to exploit root behavior to expose Confidential VM memory, ignore
> +	 * the root partition setting if also a Confidential VM.
> +	 */
> +	if ((ms_hyperv.priv_high & HV_CREATE_PARTITIONS) &&
> +	    (ms_hyperv.priv_high & HV_CPU_MANAGEMENT) &&
> +	    !(ms_hyperv.priv_high & HV_ISOLATION)) {
> +		pr_info("Hyper-V: running as root partition\n");
> +		if (IS_ENABLED(CONFIG_MSHV_ROOT))

I'll have to rescind the "Reviewed-by:" that I just gave. There's a patch
sequencing problem in that CONFIG_MSHV_ROOT doesn't exist yet.
It's added in Patch 3 of the series.  Because it doesn't exist, the
IS_ENABLED() will always return 'false', which isn't fatal in the sense
of causing a compile error.  But the code won't run in the root partition
because hv_current_partition_type isn't set.

Michael

> +			hv_current_partition_type = HV_PARTITION_TYPE_ROOT;
> +		else
> +			pr_crit("Hyper-V: CONFIG_MSHV_ROOT not enabled!\n");
> +	}
> +}

Re: [PATCH v2 2/3] hyperv: Change hv_root_partition into a function

[Nuno Das Neves]

On 2/20/2025 11:17 AM, Michael Kelley wrote:
> From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Thursday, February 20, 2025 10:33 AM
>>
>> Introduce hv_current_partition_type to store the partition type
>> as an enum.
>>
>> Right now this is limited to guest or root partition, but there will
>> be other kinds in future and the enum is easily extensible.
>>
>> Set up hv_current_partition_type early in Hyper-V initialization with
>> hv_identify_partition_type(). hv_root_partition() just queries this
>> value, and shouldn't be called before that.
>>
>> Making this check into a function sets the stage for adding a config
>> option to gate the compilation of root partition code. In particular,
>> hv_root_partition() can be stubbed out always be false if root
>> partition support isn't desired.
>>
>>
> 
> [snip]
>  
>> +void hv_identify_partition_type(void)
>> +{
>> +	/* Assume guest role */
>> +	hv_current_partition_type = HV_PARTITION_TYPE_GUEST;
>> +	/*
>> +	 * Check partition creation and cpu management privileges
>> +	 *
>> +	 * Hyper-V should never specify running as root and as a Confidential
>> +	 * VM. But to protect against a compromised/malicious Hyper-V trying
>> +	 * to exploit root behavior to expose Confidential VM memory, ignore
>> +	 * the root partition setting if also a Confidential VM.
>> +	 */
>> +	if ((ms_hyperv.priv_high & HV_CREATE_PARTITIONS) &&
>> +	    (ms_hyperv.priv_high & HV_CPU_MANAGEMENT) &&
>> +	    !(ms_hyperv.priv_high & HV_ISOLATION)) {
>> +		pr_info("Hyper-V: running as root partition\n");
>> +		if (IS_ENABLED(CONFIG_MSHV_ROOT))
> 
> I'll have to rescind the "Reviewed-by:" that I just gave. There's a patch
> sequencing problem in that CONFIG_MSHV_ROOT doesn't exist yet.
> It's added in Patch 3 of the series.  Because it doesn't exist, the
> IS_ENABLED() will always return 'false', which isn't fatal in the sense
> of causing a compile error.  But the code won't run in the root partition
> because hv_current_partition_type isn't set.
> 

Oops! Thanks for catching that, I'll just move the check from this patch
to patch 3.

> Michael
> 
>> +			hv_current_partition_type = HV_PARTITION_TYPE_ROOT;
>> +		else
>> +			pr_crit("Hyper-V: CONFIG_MSHV_ROOT not enabled!\n");
>> +	}
>> +}

Re: [PATCH v2 2/3] hyperv: Change hv_root_partition into a function

[MUKESH RATHOR]

On 2/20/25 10:33, Nuno Das Neves wrote:
 > Introduce hv_current_partition_type to store the partition type
 > as an enum.
 >
 > Right now this is limited to guest or root partition, but there will
 > be other kinds in future and the enum is easily extensible.
 >
 > Set up hv_current_partition_type early in Hyper-V initialization with
 > hv_identify_partition_type(). hv_root_partition() just queries this
 > value, and shouldn't be called before that.
 >
 > Making this check into a function sets the stage for adding a config
 > option to gate the compilation of root partition code. In particular,
 > hv_root_partition() can be stubbed out always be false if root
 > partition support isn't desired.
 >
 > Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
 > ---
 >   arch/arm64/hyperv/mshyperv.c       |  2 ++
 >   arch/x86/hyperv/hv_init.c          | 10 ++++-----
 >   arch/x86/kernel/cpu/mshyperv.c     | 24 ++------------------
 >   drivers/clocksource/hyperv_timer.c |  4 ++--
 >   drivers/hv/hv.c                    | 10 ++++-----
 >   drivers/hv/hv_common.c             | 35 +++++++++++++++++++++++++-----
 >   drivers/hv/vmbus_drv.c             |  2 +-
 >   drivers/iommu/hyperv-iommu.c       |  4 ++--
 >   include/asm-generic/mshyperv.h     | 15 +++++++++++--
 >   9 files changed, 61 insertions(+), 45 deletions(-)
 >
 > diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
 > index 29fcfd595f48..2265ea5ce5ad 100644
 > --- a/arch/arm64/hyperv/mshyperv.c
 > +++ b/arch/arm64/hyperv/mshyperv.c
 > @@ -61,6 +61,8 @@ static int __init hyperv_init(void)
 >   		ms_hyperv.features, ms_hyperv.priv_high, ms_hyperv.hints,
 >   		ms_hyperv.misc_features);
 >
 > +	hv_identify_partition_type();
 > +
 >   	ret = hv_common_init();
 >   	if (ret)
 >   		return ret;
 > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
 > index 9be1446f5bd3..ddeb40930bc8 100644
 > --- a/arch/x86/hyperv/hv_init.c
 > +++ b/arch/x86/hyperv/hv_init.c
 > @@ -90,7 +90,7 @@ static int hv_cpu_init(unsigned int cpu)
 >   		return 0;
 >
 >   	hvp = &hv_vp_assist_page[cpu];
 > -	if (hv_root_partition) {
 > +	if (hv_root_partition()) {
 >   		/*
 >   		 * For root partition we get the hypervisor provided VP assist
 >   		 * page, instead of allocating a new page.
 > @@ -242,7 +242,7 @@ static int hv_cpu_die(unsigned int cpu)
 >
 >   	if (hv_vp_assist_page && hv_vp_assist_page[cpu]) {
 >   		union hv_vp_assist_msr_contents msr = { 0 };
 > -		if (hv_root_partition) {
 > +		if (hv_root_partition()) {
 >   			/*
 >   			 * For root partition the VP assist page is mapped to
 >   			 * hypervisor provided page, and thus we unmap the
 > @@ -317,7 +317,7 @@ static int hv_suspend(void)
 >   	union hv_x64_msr_hypercall_contents hypercall_msr;
 >   	int ret;
 >
 > -	if (hv_root_partition)
 > +	if (hv_root_partition())
 >   		return -EPERM;
 >
 >   	/*
 > @@ -518,7 +518,7 @@ void __init hyperv_init(void)
 >   	rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
 >   	hypercall_msr.enable = 1;
 >
 > -	if (hv_root_partition) {
 > +	if (hv_root_partition()) {
 >   		struct page *pg;
 >   		void *src;
 >
 > @@ -592,7 +592,7 @@ void __init hyperv_init(void)
 >   	 * If we're running as root, we want to create our own PCI MSI domain.
 >   	 * We can't set this in hv_pci_init because that would be too late.
 >   	 */
 > -	if (hv_root_partition)
 > +	if (hv_root_partition())
 >   		x86_init.irqs.create_pci_msi_domain = hv_create_pci_msi_domain;
 >   #endif
 >
 > diff --git a/arch/x86/kernel/cpu/mshyperv.c 
b/arch/x86/kernel/cpu/mshyperv.c
 > index f285757618fc..4f01f424ea5b 100644
 > --- a/arch/x86/kernel/cpu/mshyperv.c
 > +++ b/arch/x86/kernel/cpu/mshyperv.c
 > @@ -33,8 +33,6 @@
 >   #include <asm/numa.h>
 >   #include <asm/svm.h>
 >
 > -/* Is Linux running as the root partition? */
 > -bool hv_root_partition;
 >   /* Is Linux running on nested Microsoft Hypervisor */
 >   bool hv_nested;
 >   struct ms_hyperv_info ms_hyperv;
 > @@ -451,25 +449,7 @@ static void __init ms_hyperv_init_platform(void)
 >   	pr_debug("Hyper-V: max %u virtual processors, %u logical 
processors\n",
 >   		 ms_hyperv.max_vp_index, ms_hyperv.max_lp_index);
 >
 > -	/*
 > -	 * Check CPU management privilege.
 > -	 *
 > -	 * To mirror what Windows does we should extract CPU management
 > -	 * features and use the ReservedIdentityBit to detect if Linux is the
 > -	 * root partition. But that requires negotiating CPU management
 > -	 * interface (a process to be finalized). For now, use the privilege
 > -	 * flag as the indicator for running as root.
 > -	 *
 > -	 * Hyper-V should never specify running as root and as a Confidential
 > -	 * VM. But to protect against a compromised/malicious Hyper-V trying
 > -	 * to exploit root behavior to expose Confidential VM memory, ignore
 > -	 * the root partition setting if also a Confidential VM.
 > -	 */
 > -	if ((ms_hyperv.priv_high & HV_CPU_MANAGEMENT) &&
 > -	    !(ms_hyperv.priv_high & HV_ISOLATION)) {
 > -		hv_root_partition = true;
 > -		pr_info("Hyper-V: running as root partition\n");
 > -	}
 > +	hv_identify_partition_type();
 >
 >   	if (ms_hyperv.hints & HV_X64_HYPERV_NESTED) {
 >   		hv_nested = true;
 > @@ -618,7 +598,7 @@ static void __init ms_hyperv_init_platform(void)
 >
 >   # ifdef CONFIG_SMP
 >   	smp_ops.smp_prepare_boot_cpu = hv_smp_prepare_boot_cpu;
 > -	if (hv_root_partition ||
 > +	if (hv_root_partition() ||
 >   	    (!ms_hyperv.paravisor_present && hv_isolation_type_snp()))
 >   		smp_ops.smp_prepare_cpus = hv_smp_prepare_cpus;
 >   # endif
 > diff --git a/drivers/clocksource/hyperv_timer.c 
b/drivers/clocksource/hyperv_timer.c
 > index f00019b078a7..09549451dd51 100644
 > --- a/drivers/clocksource/hyperv_timer.c
 > +++ b/drivers/clocksource/hyperv_timer.c
 > @@ -582,7 +582,7 @@ static void __init hv_init_tsc_clocksource(void)
 >   	 * mapped.
 >   	 */
 >   	tsc_msr.as_uint64 = hv_get_msr(HV_MSR_REFERENCE_TSC);
 > -	if (hv_root_partition)
 > +	if (hv_root_partition())
 >   		tsc_pfn = tsc_msr.pfn;
 >   	else
 >   		tsc_pfn = HVPFN_DOWN(virt_to_phys(tsc_page));
 > @@ -627,7 +627,7 @@ void __init hv_remap_tsc_clocksource(void)
 >   	if (!(ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE))
 >   		return;
 >
 > -	if (!hv_root_partition) {
 > +	if (!hv_root_partition()) {
 >   		WARN(1, "%s: attempt to remap TSC page in guest partition\n",
 >   		     __func__);
 >   		return;
 > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
 > index fab0690b5c41..a38f84548bc2 100644
 > --- a/drivers/hv/hv.c
 > +++ b/drivers/hv/hv.c
 > @@ -144,7 +144,7 @@ int hv_synic_alloc(void)
 >   		 * Synic message and event pages are allocated by paravisor.
 >   		 * Skip these pages allocation here.
 >   		 */
 > -		if (!ms_hyperv.paravisor_present && !hv_root_partition) {
 > +		if (!ms_hyperv.paravisor_present && !hv_root_partition()) {
 >   			hv_cpu->synic_message_page =
 >   				(void *)get_zeroed_page(GFP_ATOMIC);
 >   			if (!hv_cpu->synic_message_page) {
 > @@ -272,7 +272,7 @@ void hv_synic_enable_regs(unsigned int cpu)
 >   	simp.as_uint64 = hv_get_msr(HV_MSR_SIMP);
 >   	simp.simp_enabled = 1;
 >
 > -	if (ms_hyperv.paravisor_present || hv_root_partition) {
 > +	if (ms_hyperv.paravisor_present || hv_root_partition()) {
 >   		/* Mask out vTOM bit. ioremap_cache() maps decrypted */
 >   		u64 base = (simp.base_simp_gpa << HV_HYP_PAGE_SHIFT) &
 >   				~ms_hyperv.shared_gpa_boundary;
 > @@ -291,7 +291,7 @@ void hv_synic_enable_regs(unsigned int cpu)
 >   	siefp.as_uint64 = hv_get_msr(HV_MSR_SIEFP);
 >   	siefp.siefp_enabled = 1;
 >
 > -	if (ms_hyperv.paravisor_present || hv_root_partition) {
 > +	if (ms_hyperv.paravisor_present || hv_root_partition()) {
 >   		/* Mask out vTOM bit. ioremap_cache() maps decrypted */
 >   		u64 base = (siefp.base_siefp_gpa << HV_HYP_PAGE_SHIFT) &
 >   				~ms_hyperv.shared_gpa_boundary;
 > @@ -367,7 +367,7 @@ void hv_synic_disable_regs(unsigned int cpu)
 >   	 * addresses.
 >   	 */
 >   	simp.simp_enabled = 0;
 > -	if (ms_hyperv.paravisor_present || hv_root_partition) {
 > +	if (ms_hyperv.paravisor_present || hv_root_partition()) {
 >   		iounmap(hv_cpu->synic_message_page);
 >   		hv_cpu->synic_message_page = NULL;
 >   	} else {
 > @@ -379,7 +379,7 @@ void hv_synic_disable_regs(unsigned int cpu)
 >   	siefp.as_uint64 = hv_get_msr(HV_MSR_SIEFP);
 >   	siefp.siefp_enabled = 0;
 >
 > -	if (ms_hyperv.paravisor_present || hv_root_partition) {
 > +	if (ms_hyperv.paravisor_present || hv_root_partition()) {
 >   		iounmap(hv_cpu->synic_event_page);
 >   		hv_cpu->synic_event_page = NULL;
 >   	} else {
 > diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
 > index 2120aead98d9..c5c5dc92ff21 100644
 > --- a/drivers/hv/hv_common.c
 > +++ b/drivers/hv/hv_common.c
 > @@ -34,8 +34,11 @@
 >   u64 hv_current_partition_id = HV_PARTITION_ID_SELF;
 >   EXPORT_SYMBOL_GPL(hv_current_partition_id);
 >
 > +enum hv_partition_type hv_current_partition_type;
 > +EXPORT_SYMBOL_GPL(hv_current_partition_type);
 > +

nit: if possible and not too late, can we please use more Unix
style naming, eg, hv_curr_ptid and hv_curr_pt_type rather than this
long windows style names that causes unnecessary line wraps/splits.

Thanks,
-Mukesh

Re: [PATCH v2 2/3] hyperv: Change hv_root_partition into a function

[Easwar Hariharan]

On 2/20/2025 1:59 PM, MUKESH RATHOR wrote:
> 
> 
> On 2/20/25 10:33, Nuno Das Neves wrote:
>  > Introduce hv_current_partition_type to store the partition type
>  > as an enum.
>  >
>  > Right now this is limited to guest or root partition, but there will
>  > be other kinds in future and the enum is easily extensible.
>  >
>  > Set up hv_current_partition_type early in Hyper-V initialization with
>  > hv_identify_partition_type(). hv_root_partition() just queries this
>  > value, and shouldn't be called before that.
>  >
>  > Making this check into a function sets the stage for adding a config
>  > option to gate the compilation of root partition code. In particular,
>  > hv_root_partition() can be stubbed out always be false if root
>  > partition support isn't desired.
>  >
>  > Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
>  > ---
>  >   arch/arm64/hyperv/mshyperv.c       |  2 ++
>  >   arch/x86/hyperv/hv_init.c          | 10 ++++-----
>  >   arch/x86/kernel/cpu/mshyperv.c     | 24 ++------------------
>  >   drivers/clocksource/hyperv_timer.c |  4 ++--
>  >   drivers/hv/hv.c                    | 10 ++++-----
>  >   drivers/hv/hv_common.c             | 35 +++++++++++++++++++++++++-----
>  >   drivers/hv/vmbus_drv.c             |  2 +-
>  >   drivers/iommu/hyperv-iommu.c       |  4 ++--
>  >   include/asm-generic/mshyperv.h     | 15 +++++++++++--
>  >   9 files changed, 61 insertions(+), 45 deletions(-)
>  >

<snip>

>  > @@ -34,8 +34,11 @@
>  >   u64 hv_current_partition_id = HV_PARTITION_ID_SELF;
>  >   EXPORT_SYMBOL_GPL(hv_current_partition_id);
>  >
>  > +enum hv_partition_type hv_current_partition_type;
>  > +EXPORT_SYMBOL_GPL(hv_current_partition_type);
>  > +
> 
> nit: if possible and not too late, can we please use more Unix
> style naming, eg, hv_curr_ptid and hv_curr_pt_type rather than this
> long windows style names that causes unnecessary line wraps/splits.
> 
> Thanks,
> -Mukesh
> 

Per https://docs.kernel.org/process/coding-style.html#naming

GLOBAL variables (to be used only if you really need them) need to have descriptive names,
as do global functions. If you have a function that counts the number of active users,
you should call that count_active_users() or similar, you should not call it cntusr().

Thanks,
Easwar (he/him)

Re: [PATCH v2 2/3] hyperv: Change hv_root_partition into a function

[MUKESH RATHOR]

On 2/20/25 14:56, Easwar Hariharan wrote:
 > On 2/20/2025 1:59 PM, MUKESH RATHOR wrote:
 >>
 >>
 >> On 2/20/25 10:33, Nuno Das Neves wrote:
 >>   > Introduce hv_current_partition_type to store the partition type
 >>   > as an enum.
 >>   >
 >>   > Right now this is limited to guest or root partition, but there will
 >>   > be other kinds in future and the enum is easily extensible.
 >>   >
 >>   > Set up hv_current_partition_type early in Hyper-V initialization
with
 >>   > hv_identify_partition_type(). hv_root_partition() just queries this
 >>   > value, and shouldn't be called before that.
 >>   >
 >>   > Making this check into a function sets the stage for adding a config
 >>   > option to gate the compilation of root partition code. In
particular,
 >>   > hv_root_partition() can be stubbed out always be false if root
 >>   > partition support isn't desired.
 >>   >
 >>   > Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
 >>   > ---
 >>   >   arch/arm64/hyperv/mshyperv.c       |  2 ++
 >>   >   arch/x86/hyperv/hv_init.c          | 10 ++++-----
 >>   >   arch/x86/kernel/cpu/mshyperv.c     | 24 ++------------------
 >>   >   drivers/clocksource/hyperv_timer.c |  4 ++--
 >>   >   drivers/hv/hv.c                    | 10 ++++-----
 >>   >   drivers/hv/hv_common.c             | 35
+++++++++++++++++++++++++-----
 >>   >   drivers/hv/vmbus_drv.c             |  2 +-
 >>   >   drivers/iommu/hyperv-iommu.c       |  4 ++--
 >>   >   include/asm-generic/mshyperv.h     | 15 +++++++++++--
 >>   >   9 files changed, 61 insertions(+), 45 deletions(-)
 >>   >
 >
 > <snip>
 >
 >>   > @@ -34,8 +34,11 @@
 >>   >   u64 hv_current_partition_id = HV_PARTITION_ID_SELF;
 >>   >   EXPORT_SYMBOL_GPL(hv_current_partition_id);
 >>   >
 >>   > +enum hv_partition_type hv_current_partition_type;
 >>   > +EXPORT_SYMBOL_GPL(hv_current_partition_type);
 >>   > +
 >>
 >> nit: if possible and not too late, can we please use more Unix
 >> style naming, eg, hv_curr_ptid and hv_curr_pt_type rather than this
 >> long windows style names that causes unnecessary line wraps/splits.
 >>
 >> Thanks,
 >> -Mukesh
 >>
 >
 > Per
https://docs.kernel.org/process/coding-style.html#naming
 >
 > GLOBAL variables (to be used only if you really need them) need to
have descriptive names,
 > as do global functions. If you have a function that counts the number
of active users,
 > you should call that count_active_users() or similar, you should not
call it cntusr().

Thant's hardly a fair comparison. Suggestion was NOT hvptid.

Thanks,
-Mukesh

Re: [PATCH v2 2/3] hyperv: Change hv_root_partition into a function

[Nuno Das Neves]

On 2/20/2025 2:59 PM, MUKESH RATHOR wrote:
> 
> 
> On 2/20/25 14:56, Easwar Hariharan wrote:
>  > On 2/20/2025 1:59 PM, MUKESH RATHOR wrote:
>  >>
>  >>
>  >> On 2/20/25 10:33, Nuno Das Neves wrote:
>  >>   > Introduce hv_current_partition_type to store the partition type
>  >>   > as an enum.
>  >>   >
>  >>   > Right now this is limited to guest or root partition, but there will
>  >>   > be other kinds in future and the enum is easily extensible.
>  >>   >
>  >>   > Set up hv_current_partition_type early in Hyper-V initialization
> with
>  >>   > hv_identify_partition_type(). hv_root_partition() just queries this
>  >>   > value, and shouldn't be called before that.
>  >>   >
>  >>   > Making this check into a function sets the stage for adding a config
>  >>   > option to gate the compilation of root partition code. In
> particular,
>  >>   > hv_root_partition() can be stubbed out always be false if root
>  >>   > partition support isn't desired.
>  >>   >
>  >>   > Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
>  >>   > ---
>  >>   >   arch/arm64/hyperv/mshyperv.c       |  2 ++
>  >>   >   arch/x86/hyperv/hv_init.c          | 10 ++++-----
>  >>   >   arch/x86/kernel/cpu/mshyperv.c     | 24 ++------------------
>  >>   >   drivers/clocksource/hyperv_timer.c |  4 ++--
>  >>   >   drivers/hv/hv.c                    | 10 ++++-----
>  >>   >   drivers/hv/hv_common.c             | 35
> +++++++++++++++++++++++++-----
>  >>   >   drivers/hv/vmbus_drv.c             |  2 +-
>  >>   >   drivers/iommu/hyperv-iommu.c       |  4 ++--
>  >>   >   include/asm-generic/mshyperv.h     | 15 +++++++++++--
>  >>   >   9 files changed, 61 insertions(+), 45 deletions(-)
>  >>   >
>  >
>  > <snip>
>  >
>  >>   > @@ -34,8 +34,11 @@
>  >>   >   u64 hv_current_partition_id = HV_PARTITION_ID_SELF;
>  >>   >   EXPORT_SYMBOL_GPL(hv_current_partition_id);
>  >>   >
>  >>   > +enum hv_partition_type hv_current_partition_type;
>  >>   > +EXPORT_SYMBOL_GPL(hv_current_partition_type);
>  >>   > +
>  >>
>  >> nit: if possible and not too late, can we please use more Unix
>  >> style naming, eg, hv_curr_ptid and hv_curr_pt_type rather than this
>  >> long windows style names that causes unnecessary line wraps/splits.
>  >>
>  >> Thanks,
>  >> -Mukesh
>  >>
>  >
>  > Per
> https://docs.kernel.org/process/coding-style.html#naming
>  >
>  > GLOBAL variables (to be used only if you really need them) need to
> have descriptive names,
>  > as do global functions. If you have a function that counts the number
> of active users,
>  > you should call that count_active_users() or similar, you should not
> call it cntusr().
> 
> Thant's hardly a fair comparison. Suggestion was NOT hvptid.
> 
I'm in favor of shortening the names when the abbreviation is common and
therefore still perfectly clear to anyone reading it - e.g. "curr" is
a perfectly acceptable abbreviation of "current", in my view.

I think abbreviating "partition" to "pt" is probably not a good fit for
global variables. Anyone seeing a variable with the word "partition"
(and hv_ prefix) can go look up what a Hyper-V partition is if they don't
know, but "pt" would be completely impenetrable without reading through a
fair amount of the code that uses it to figure out what it refers to.

I think even slightly longer abbreviations like "part", "ptn", "prt", or
"prtn" are not good enough unfortunately... the word "partition" just
doesn't lend itself to abbreviation in an obvious way.

So, for this patch I'm fine with changing it to "hv_curr_partition_type"
which saves a few characters.

Feel free to post a followup for "hv_curr_partition_id" if you like.

Note - For the driver code which isn't as exposed to the rest of the
kernel, I think we can continue to use "pt" or similar to keep the names
shorter.

Thanks
Nuno

Re: [PATCH v2 2/3] hyperv: Change hv_root_partition into a function

[Easwar Hariharan]

On 2/21/2025 10:10 AM, Nuno Das Neves wrote:
> On 2/20/2025 2:59 PM, MUKESH RATHOR wrote:
>>
>>
>> On 2/20/25 14:56, Easwar Hariharan wrote:
>>  > On 2/20/2025 1:59 PM, MUKESH RATHOR wrote:
>>  >>
>>  >>
>>  >> On 2/20/25 10:33, Nuno Das Neves wrote:
>>  >>   > Introduce hv_current_partition_type to store the partition type
>>  >>   > as an enum.
>>  >>   >
>>  >>   > Right now this is limited to guest or root partition, but there will
>>  >>   > be other kinds in future and the enum is easily extensible.
>>  >>   >
>>  >>   > Set up hv_current_partition_type early in Hyper-V initialization
>> with
>>  >>   > hv_identify_partition_type(). hv_root_partition() just queries this
>>  >>   > value, and shouldn't be called before that.
>>  >>   >
>>  >>   > Making this check into a function sets the stage for adding a config
>>  >>   > option to gate the compilation of root partition code. In
>> particular,
>>  >>   > hv_root_partition() can be stubbed out always be false if root
>>  >>   > partition support isn't desired.
>>  >>   >
>>  >>   > Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
>>  >>   > ---
>>  >>   >   arch/arm64/hyperv/mshyperv.c       |  2 ++
>>  >>   >   arch/x86/hyperv/hv_init.c          | 10 ++++-----
>>  >>   >   arch/x86/kernel/cpu/mshyperv.c     | 24 ++------------------
>>  >>   >   drivers/clocksource/hyperv_timer.c |  4 ++--
>>  >>   >   drivers/hv/hv.c                    | 10 ++++-----
>>  >>   >   drivers/hv/hv_common.c             | 35
>> +++++++++++++++++++++++++-----
>>  >>   >   drivers/hv/vmbus_drv.c             |  2 +-
>>  >>   >   drivers/iommu/hyperv-iommu.c       |  4 ++--
>>  >>   >   include/asm-generic/mshyperv.h     | 15 +++++++++++--
>>  >>   >   9 files changed, 61 insertions(+), 45 deletions(-)
>>  >>   >
>>  >
>>  > <snip>
>>  >
>>  >>   > @@ -34,8 +34,11 @@
>>  >>   >   u64 hv_current_partition_id = HV_PARTITION_ID_SELF;
>>  >>   >   EXPORT_SYMBOL_GPL(hv_current_partition_id);
>>  >>   >
>>  >>   > +enum hv_partition_type hv_current_partition_type;
>>  >>   > +EXPORT_SYMBOL_GPL(hv_current_partition_type);
>>  >>   > +
>>  >>
>>  >> nit: if possible and not too late, can we please use more Unix
>>  >> style naming, eg, hv_curr_ptid and hv_curr_pt_type rather than this
>>  >> long windows style names that causes unnecessary line wraps/splits.
>>  >>
>>  >> Thanks,
>>  >> -Mukesh
>>  >>
>>  >
>>  > Per
>> https://docs.kernel.org/process/coding-style.html#naming
>>  >
>>  > GLOBAL variables (to be used only if you really need them) need to
>> have descriptive names,
>>  > as do global functions. If you have a function that counts the number
>> of active users,
>>  > you should call that count_active_users() or similar, you should not
>> call it cntusr().
>>
>> Thant's hardly a fair comparison. Suggestion was NOT hvptid.
>>
> I'm in favor of shortening the names when the abbreviation is common and
> therefore still perfectly clear to anyone reading it - e.g. "curr" is
> a perfectly acceptable abbreviation of "current", in my view.
> 
> I think abbreviating "partition" to "pt" is probably not a good fit for
> global variables. Anyone seeing a variable with the word "partition"
> (and hv_ prefix) can go look up what a Hyper-V partition is if they don't
> know, but "pt" would be completely impenetrable without reading through a
> fair amount of the code that uses it to figure out what it refers to.
> 
> I think even slightly longer abbreviations like "part", "ptn", "prt", or
> "prtn" are not good enough unfortunately... the word "partition" just
> doesn't lend itself to abbreviation in an obvious way.
> 
> So, for this patch I'm fine with changing it to "hv_curr_partition_type"
> which saves a few characters.
> 

FWIW, I generally prioritize readability of code and your proposal neatly
splits the difference. Also, it's your patch, so you get to make the decision. :)

Thanks,
Easwar (he/him)

Re: [PATCH v2 2/3] hyperv: Change hv_root_partition into a function

[MUKESH RATHOR]

On 2/21/25 10:10, Nuno Das Neves wrote:
 > On 2/20/2025 2:59 PM, MUKESH RATHOR wrote:
 >>
 >>
 >> On 2/20/25 14:56, Easwar Hariharan wrote:
 >>   > On 2/20/2025 1:59 PM, MUKESH RATHOR wrote:
 >>   >>
 >>   >>
 >>   >> On 2/20/25 10:33, Nuno Das Neves wrote:
 >>   >>   > Introduce hv_current_partition_type to store the partition type
 >>   >>   > as an enum.
 >>   >>   >
 >>   >>   > option to gate the compilation of root partition code. In
 >> particular,

<snip>

 >>   > <snip>
 >>   >
 >>   >>   > @@ -34,8 +34,11 @@
 >>   >>   >   u64 hv_current_partition_id = HV_PARTITION_ID_SELF;
 >>   >>   >   EXPORT_SYMBOL_GPL(hv_current_partition_id);
 >>   >>   >
 >>   >>   > +enum hv_partition_type hv_current_partition_type;
 >>   >>   > +EXPORT_SYMBOL_GPL(hv_current_partition_type);
 >>   >>   > +
 >>   >>
 >>   >> nit: if possible and not too late, can we please use more Unix
 >>   >> style naming, eg, hv_curr_ptid and hv_curr_pt_type rather than this
 >>   >> long windows style names that causes unnecessary line wraps/splits.
 >>   >>
 >>   >> Thanks,
 >>   >> -Mukesh
 >>   >>
 >>   >
 >>   > Per
 >>
https://docs.kernel.org/process/coding-style.html#naming
 >>   >
 >>   > GLOBAL variables (to be used only if you really need them) need to
 >> have descriptive names,
 >>   > as do global functions. If you have a function that counts the
number
 >> of active users,
 >>   > you should call that count_active_users() or similar, you should not
 >> call it cntusr().
 >>
 >> Thant's hardly a fair comparison. Suggestion was NOT hvptid.
 >>
 > I'm in favor of shortening the names when the abbreviation is common and
 > therefore still perfectly clear to anyone reading it - e.g. "curr" is
 > a perfectly acceptable abbreviation of "current", in my view.
 >
 > I think abbreviating "partition" to "pt" is probably not a good fit for
 > global variables. Anyone seeing a variable with the word "partition"
 > (and hv_ prefix) can go look up what a Hyper-V partition is if they don't
 > know, but "pt" would be completely impenetrable without reading through a
 > fair amount of the code that uses it to figure out what it refers to.
 >
 > I think even slightly longer abbreviations like "part", "ptn", "prt", or
 > "prtn" are not good enough unfortunately... the word "partition" just
 > doesn't lend itself to abbreviation in an obvious way.

That is fine, IMO. We look at the code often enough, and some names like
that get well established. For example, pfn or current for current task.
Moreover, the prefix hv_* narrows it down to hyperv related. Other
suggestions: hv_curr_partn_type and hv_curr_partn_id if you don't
like part, ptn etc. I'll let you pick whatever, hv_curr_partition_id is
def better.  In the end, concise yet unique enough that anyone can
easily find all references via grep/cscope/etc without RSI :).

 > So, for this patch I'm fine with changing it to "hv_curr_partition_type"
 > which saves a few characters.
 >
 > Feel free to post a followup for "hv_curr_partition_id" if you like.

 > Note - For the driver code which isn't as exposed to the rest of the
 > kernel, I think we can continue to use "pt" or similar to keep the names
 > shorter.

That's a good idea.

Thanks,
-Mukesh

[PATCH v2 3/3] hyperv: Add CONFIG_MSHV_ROOT to gate root partition support

[Nuno Das Neves]

CONFIG_MSHV_ROOT allows kernels built to run as a normal Hyper-V guest
to exclude the root partition code, which is expected to grow
significantly over time.

This option is a tristate so future driver code can be built as a
(m)odule, allowing faster development iteration cycles.

If CONFIG_MSHV_ROOT is disabled, don't compile hv_proc.c, and stub
hv_root_partition() to return false unconditionally. This allows the
compiler to optimize away root partition code blocks since they will
be disabled at compile time.

Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
---
 drivers/hv/Kconfig             | 16 ++++++++++++++++
 drivers/hv/Makefile            |  3 ++-
 include/asm-generic/mshyperv.h | 24 ++++++++++++++++++++----
 3 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
index 862c47b191af..794e88e9dc6b 100644
--- a/drivers/hv/Kconfig
+++ b/drivers/hv/Kconfig
@@ -55,4 +55,20 @@ config HYPERV_BALLOON
 	help
 	  Select this option to enable Hyper-V Balloon driver.
 
+config MSHV_ROOT
+	tristate "Microsoft Hyper-V root partition support"
+	depends on HYPERV
+	depends on !HYPERV_VTL_MODE
+	# The hypervisor interface operates on 4k pages. Enforcing it here
+	# simplifies many assumptions in the root partition code.
+	# e.g. When withdrawing memory, the hypervisor gives back 4k pages in
+	# no particular order, making it impossible to reassemble larger pages
+	depends on PAGE_SIZE_4KB
+	default n
+	help
+	  Select this option to enable support for booting and running as root
+	  partition on Microsoft Hyper-V.
+
+	  If unsure, say N.
+
 endmenu
diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
index 9afcabb3fbd2..2b8dc954b350 100644
--- a/drivers/hv/Makefile
+++ b/drivers/hv/Makefile
@@ -13,4 +13,5 @@ hv_vmbus-$(CONFIG_HYPERV_TESTING)	+= hv_debugfs.o
 hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o hv_utils_transport.o
 
 # Code that must be built-in
-obj-$(subst m,y,$(CONFIG_HYPERV)) += hv_common.o hv_proc.o
+obj-$(subst m,y,$(CONFIG_HYPERV)) += hv_common.o
+obj-$(subst m,y,$(CONFIG_MSHV_ROOT)) += hv_proc.o
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index d2b1a8fc074c..b29357cff2f7 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -223,10 +223,6 @@ void *hv_alloc_hyperv_page(void);
 void *hv_alloc_hyperv_zeroed_page(void);
 void hv_free_hyperv_page(void *addr);
 
-int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages);
-int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id);
-int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags);
-
 /**
  * hv_cpu_number_to_vp_number() - Map CPU to VP.
  * @cpu_number: CPU number in Linux terms
@@ -327,9 +323,29 @@ static inline enum hv_isolation_type hv_get_isolation_type(void)
 }
 #endif /* CONFIG_HYPERV */
 
+#if IS_ENABLED(CONFIG_MSHV_ROOT)
 static inline bool hv_root_partition(void)
 {
 	return hv_current_partition_type == HV_PARTITION_TYPE_ROOT;
 }
+int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages);
+int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id);
+int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags);
+
+#else /* CONFIG_MSHV_ROOT */
+static inline bool hv_root_partition(void) { return false; }
+static inline int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
+{
+	return -EOPNOTSUPP;
+}
+static inline int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id)
+{
+	return -EOPNOTSUPP;
+}
+static inline int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags)
+{
+	return -EOPNOTSUPP;
+}
+#endif /* CONFIG_MSHV_ROOT */
 
 #endif
-- 
2.34.1

Re: [PATCH v2 3/3] hyperv: Add CONFIG_MSHV_ROOT to gate root partition support

[Easwar Hariharan]

On 2/20/2025 10:33 AM, Nuno Das Neves wrote:
> CONFIG_MSHV_ROOT allows kernels built to run as a normal Hyper-V guest
> to exclude the root partition code, which is expected to grow
> significantly over time.
> 
> This option is a tristate so future driver code can be built as a
> (m)odule, allowing faster development iteration cycles.
> 
> If CONFIG_MSHV_ROOT is disabled, don't compile hv_proc.c, and stub
> hv_root_partition() to return false unconditionally. This allows the
> compiler to optimize away root partition code blocks since they will
> be disabled at compile time.
> 
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> ---
>  drivers/hv/Kconfig             | 16 ++++++++++++++++
>  drivers/hv/Makefile            |  3 ++-
>  include/asm-generic/mshyperv.h | 24 ++++++++++++++++++++----
>  3 files changed, 38 insertions(+), 5 deletions(-)

Looks good to me.

Reviewed-by: Easwar Hariharan <eahariha@linux.microsoft.com>

RE: [PATCH v2 3/3] hyperv: Add CONFIG_MSHV_ROOT to gate root partition support

[Michael Kelley]

From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Thursday, February 20, 2025 10:33 AM
> 
> CONFIG_MSHV_ROOT allows kernels built to run as a normal Hyper-V guest
> to exclude the root partition code, which is expected to grow
> significantly over time.
> 
> This option is a tristate so future driver code can be built as a
> (m)odule, allowing faster development iteration cycles.
> 
> If CONFIG_MSHV_ROOT is disabled, don't compile hv_proc.c, and stub
> hv_root_partition() to return false unconditionally. This allows the
> compiler to optimize away root partition code blocks since they will
> be disabled at compile time.
> 
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> ---
>  drivers/hv/Kconfig             | 16 ++++++++++++++++
>  drivers/hv/Makefile            |  3 ++-
>  include/asm-generic/mshyperv.h | 24 ++++++++++++++++++++----
>  3 files changed, 38 insertions(+), 5 deletions(-)
> 

This code LGTM, but I'll wait on my Reviewed-by: until the dependency
with Patch 2 of the series is sorted out.

Michael Kelley


> diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
> index 862c47b191af..794e88e9dc6b 100644
> --- a/drivers/hv/Kconfig
> +++ b/drivers/hv/Kconfig
> @@ -55,4 +55,20 @@ config HYPERV_BALLOON
>  	help
>  	  Select this option to enable Hyper-V Balloon driver.
> 
> +config MSHV_ROOT
> +	tristate "Microsoft Hyper-V root partition support"
> +	depends on HYPERV
> +	depends on !HYPERV_VTL_MODE
> +	# The hypervisor interface operates on 4k pages. Enforcing it here
> +	# simplifies many assumptions in the root partition code.
> +	# e.g. When withdrawing memory, the hypervisor gives back 4k pages in
> +	# no particular order, making it impossible to reassemble larger pages
> +	depends on PAGE_SIZE_4KB
> +	default n
> +	help
> +	  Select this option to enable support for booting and running as root
> +	  partition on Microsoft Hyper-V.
> +
> +	  If unsure, say N.
> +
>  endmenu
> diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
> index 9afcabb3fbd2..2b8dc954b350 100644
> --- a/drivers/hv/Makefile
> +++ b/drivers/hv/Makefile
> @@ -13,4 +13,5 @@ hv_vmbus-$(CONFIG_HYPERV_TESTING)	+= hv_debugfs.o
>  hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o hv_utils_transport.o
> 
>  # Code that must be built-in
> -obj-$(subst m,y,$(CONFIG_HYPERV)) += hv_common.o hv_proc.o
> +obj-$(subst m,y,$(CONFIG_HYPERV)) += hv_common.o
> +obj-$(subst m,y,$(CONFIG_MSHV_ROOT)) += hv_proc.o
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index d2b1a8fc074c..b29357cff2f7 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -223,10 +223,6 @@ void *hv_alloc_hyperv_page(void);
>  void *hv_alloc_hyperv_zeroed_page(void);
>  void hv_free_hyperv_page(void *addr);
> 
> -int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages);
> -int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id);
> -int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags);
> -
>  /**
>   * hv_cpu_number_to_vp_number() - Map CPU to VP.
>   * @cpu_number: CPU number in Linux terms
> @@ -327,9 +323,29 @@ static inline enum hv_isolation_type
> hv_get_isolation_type(void)
>  }
>  #endif /* CONFIG_HYPERV */
> 
> +#if IS_ENABLED(CONFIG_MSHV_ROOT)
>  static inline bool hv_root_partition(void)
>  {
>  	return hv_current_partition_type == HV_PARTITION_TYPE_ROOT;
>  }
> +int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages);
> +int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id);
> +int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags);
> +
> +#else /* CONFIG_MSHV_ROOT */
> +static inline bool hv_root_partition(void) { return false; }
> +static inline int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
> +{
> +	return -EOPNOTSUPP;
> +}
> +static inline int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id)
> +{
> +	return -EOPNOTSUPP;
> +}
> +static inline int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags)
> +{
> +	return -EOPNOTSUPP;
> +}
> +#endif /* CONFIG_MSHV_ROOT */
> 
>  #endif
> --
> 2.34.1

[PATCH v2 0/3] Introduce CONFIG_MSHV_ROOT for root partition code

[Nuno Das Neves]

Running in the root partition is a unique and specialized case that
requires additional code. CONFIG_MSHV_ROOT allows Hyper-V guest kernels
to exclude this code, which is important since significant additional code
specific to the root partition is expected to be added over time.

To do this, change hv_root_partition to be a function which is stubbed out
to return false if CONFIG_MSHV_ROOT=n, and don't compile hv_proc.c at all,
stubbing out those functions with inline versions.

Store the partition type (guest or root) in an enum hv_curr_partition_type,
which can be extended beyond just guest and root partition.

While at it, introduce hv_result_to_errno() to convert Hyper-V status codes
to regular linux errors. This is useful because the caller of a hypercall
helper function (such as those in hv_proc.c) usually can't and doesn't
interpret the Hyper-V status, so it is better to convert it to an error code
and reduce the possibility of misinterpreting it. This also alows the stubbed
versions of the hv_proc.c functions to just return a linux error code.

Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
---
Changes in v3:
* Fix a typo [Easwar Hariharan]
* Fix ret being initialized to HV_STATUS_SUCCESS in some hypercall helpers
  [Michael Kelley]
* Fix CONFIG_MSHV_ROOT being used in patch 2, before it is defined [Michael
  Kelley]
* Shorten hv_current_partition_type to hv_curr_partition_type [Mukesh Rathor]

Changes in v2:
* Add patch to convert hypercall statuses to linux error codes [Easwar
  Hariharan]
* While at it, split the original patch into two logical pieces, one to change
  hv_root_partition into a function, and one to introduce MSHV_CONFIG_ROOT
* Improve the clarity of and add an error message to
  hv_identify_partition_type() [Easwar Hariharan] [Michael Kelley]
* Better explain *why* the patches are useful, in the commit messages [Michael
  Kelley]
* Add a Kconfig comment explaining why PAGE_SIZE_4KB is needed [Michael Kelley]
* Minor style and typo fixes

Nuno Das Neves (3):
  hyperv: Convert hypercall statuses to linux error codes
  hyperv: Change hv_root_partition into a function
  hyperv: Add CONFIG_MSHV_ROOT to gate root partition support

 arch/arm64/hyperv/mshyperv.c       |  2 +
 arch/x86/hyperv/hv_init.c          | 10 ++---
 arch/x86/kernel/cpu/mshyperv.c     | 24 +----------
 drivers/clocksource/hyperv_timer.c |  4 +-
 drivers/hv/Kconfig                 | 16 +++++++
 drivers/hv/Makefile                |  3 +-
 drivers/hv/hv.c                    | 10 ++---
 drivers/hv/hv_common.c             | 69 +++++++++++++++++++++++++++---
 drivers/hv/hv_proc.c               | 10 ++---
 drivers/hv/vmbus_drv.c             |  2 +-
 drivers/iommu/hyperv-iommu.c       |  4 +-
 include/asm-generic/mshyperv.h     | 40 ++++++++++++++---
 12 files changed, 139 insertions(+), 55 deletions(-)

-- 
2.34.1

Re: [PATCH v2 0/3] Introduce CONFIG_MSHV_ROOT for root partition code

[Nuno Das Neves]

The subject line says v2 but this is actually v3!

Nuno

On 2/21/2025 11:56 AM, Nuno Das Neves wrote:
> Running in the root partition is a unique and specialized case that
> requires additional code. CONFIG_MSHV_ROOT allows Hyper-V guest kernels
> to exclude this code, which is important since significant additional code
> specific to the root partition is expected to be added over time.
> 
> To do this, change hv_root_partition to be a function which is stubbed out
> to return false if CONFIG_MSHV_ROOT=n, and don't compile hv_proc.c at all,
> stubbing out those functions with inline versions.
> 
> Store the partition type (guest or root) in an enum hv_curr_partition_type,
> which can be extended beyond just guest and root partition.
> 
> While at it, introduce hv_result_to_errno() to convert Hyper-V status codes
> to regular linux errors. This is useful because the caller of a hypercall
> helper function (such as those in hv_proc.c) usually can't and doesn't
> interpret the Hyper-V status, so it is better to convert it to an error code
> and reduce the possibility of misinterpreting it. This also alows the stubbed
> versions of the hv_proc.c functions to just return a linux error code.
> 
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> ---
> Changes in v3:
> * Fix a typo [Easwar Hariharan]
> * Fix ret being initialized to HV_STATUS_SUCCESS in some hypercall helpers
>   [Michael Kelley]
> * Fix CONFIG_MSHV_ROOT being used in patch 2, before it is defined [Michael
>   Kelley]
> * Shorten hv_current_partition_type to hv_curr_partition_type [Mukesh Rathor]
> 
> Changes in v2:
> * Add patch to convert hypercall statuses to linux error codes [Easwar
>   Hariharan]
> * While at it, split the original patch into two logical pieces, one to change
>   hv_root_partition into a function, and one to introduce MSHV_CONFIG_ROOT
> * Improve the clarity of and add an error message to
>   hv_identify_partition_type() [Easwar Hariharan] [Michael Kelley]
> * Better explain *why* the patches are useful, in the commit messages [Michael
>   Kelley]
> * Add a Kconfig comment explaining why PAGE_SIZE_4KB is needed [Michael Kelley]
> * Minor style and typo fixes
> 
> Nuno Das Neves (3):
>   hyperv: Convert hypercall statuses to linux error codes
>   hyperv: Change hv_root_partition into a function
>   hyperv: Add CONFIG_MSHV_ROOT to gate root partition support
> 
>  arch/arm64/hyperv/mshyperv.c       |  2 +
>  arch/x86/hyperv/hv_init.c          | 10 ++---
>  arch/x86/kernel/cpu/mshyperv.c     | 24 +----------
>  drivers/clocksource/hyperv_timer.c |  4 +-
>  drivers/hv/Kconfig                 | 16 +++++++
>  drivers/hv/Makefile                |  3 +-
>  drivers/hv/hv.c                    | 10 ++---
>  drivers/hv/hv_common.c             | 69 +++++++++++++++++++++++++++---
>  drivers/hv/hv_proc.c               | 10 ++---
>  drivers/hv/vmbus_drv.c             |  2 +-
>  drivers/iommu/hyperv-iommu.c       |  4 +-
>  include/asm-generic/mshyperv.h     | 40 ++++++++++++++---
>  12 files changed, 139 insertions(+), 55 deletions(-)
> 

Re: [PATCH v2 0/3] Introduce CONFIG_MSHV_ROOT for root partition code

[Easwar Hariharan]

On 2/21/2025 12:01 PM, Nuno Das Neves wrote:
> The subject line says v2 but this is actually v3!
> 
> Nuno

For the series,

Reviewed-by: Easwar Hariharan <eahariha@linux.microsoft.com>

Re: [PATCH v2 0/3] Introduce CONFIG_MSHV_ROOT for root partition code

[Wei Liu]

On Fri, Feb 21, 2025 at 11:56:32AM -0800, Nuno Das Neves wrote:
> Running in the root partition is a unique and specialized case that
> requires additional code. CONFIG_MSHV_ROOT allows Hyper-V guest kernels
> to exclude this code, which is important since significant additional code
> specific to the root partition is expected to be added over time.
> 
> To do this, change hv_root_partition to be a function which is stubbed out
> to return false if CONFIG_MSHV_ROOT=n, and don't compile hv_proc.c at all,
> stubbing out those functions with inline versions.
> 
> Store the partition type (guest or root) in an enum hv_curr_partition_type,
> which can be extended beyond just guest and root partition.
> 
> While at it, introduce hv_result_to_errno() to convert Hyper-V status codes
> to regular linux errors. This is useful because the caller of a hypercall
> helper function (such as those in hv_proc.c) usually can't and doesn't
> interpret the Hyper-V status, so it is better to convert it to an error code
> and reduce the possibility of misinterpreting it. This also alows the stubbed
> versions of the hv_proc.c functions to just return a linux error code.
> 
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>

No need to sign this off. :-)

> 
> Nuno Das Neves (3):
>   hyperv: Convert hypercall statuses to linux error codes
>   hyperv: Change hv_root_partition into a function
>   hyperv: Add CONFIG_MSHV_ROOT to gate root partition support
> 

Applied to hyperv-next. Thanks.