diff mbox series

[v5,04/10] hyperv: Introduce hv_recommend_using_aeoi()

Message ID 1740611284-27506-5-git-send-email-nunodasneves@linux.microsoft.com (mailing list archive)
State New
Headers show
Series Introduce /dev/mshv root partition driver | expand

Commit Message

Nuno Das Neves Feb. 26, 2025, 11:07 p.m. UTC
Factor out the check for enabling auto eoi, to be reused in root
partition code.

Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
---
 drivers/hv/hv.c                | 12 +-----------
 include/asm-generic/mshyperv.h | 13 +++++++++++++
 2 files changed, 14 insertions(+), 11 deletions(-)

Comments

Stanislav Kinsburskii Feb. 26, 2025, 11:28 p.m. UTC | #1
On Wed, Feb 26, 2025 at 03:07:58PM -0800, Nuno Das Neves wrote:
> Factor out the check for enabling auto eoi, to be reused in root
> partition code.
> 

Reviewed-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
Roman Kisel Feb. 27, 2025, 6:04 p.m. UTC | #2
On 2/26/2025 3:07 PM, Nuno Das Neves wrote:
> Factor out the check for enabling auto eoi, to be reused in root
> partition code.
> 
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> ---

I think adding "No functional changes" would bring some benefit:
that's an additional invariant to check against when reviewing.
Easwar Hariharan Feb. 27, 2025, 11:03 p.m. UTC | #3
On 2/26/2025 3:07 PM, Nuno Das Neves wrote:
> Factor out the check for enabling auto eoi, to be reused in root
> partition code.
> 
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> ---
>  drivers/hv/hv.c                | 12 +-----------
>  include/asm-generic/mshyperv.h | 13 +++++++++++++
>  2 files changed, 14 insertions(+), 11 deletions(-)
> 

<snip>

> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index 258034dfd829..1f46d19a16aa 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -77,6 +77,19 @@ extern u64 hv_do_fast_hypercall16(u16 control, u64 input1, u64 input2);
>  bool hv_isolation_type_snp(void);
>  bool hv_isolation_type_tdx(void);
>  
> +/*
> + * On architectures where Hyper-V doesn't support AEOI (e.g., ARM64),
> + * it doesn't provide a recommendation flag and AEOI must be disabled.
> + */
> +static inline bool hv_recommend_using_aeoi(void)
> +{
> +#ifdef HV_DEPRECATING_AEOI_RECOMMENDED
> +	return !(ms_hyperv.hints & HV_DEPRECATING_AEOI_RECOMMENDED);
> +#else
> +	return false;
> +#endif
> +}
> +

I must be missing something very basic here, and if so, I apologize, and please enlighten me.

HV_DEPRECATING_AEOI_RECOMMENDED is defined as BIT(9) in include/hyperv/hvgdk_mini.h, and
asm-generic/mshyperv.h includes that via include/hyperv/hvhdk.h.

If this is the case, when would HV_DEPRECATING_AEOI_RECOMMENDED ever be not defined?
If it's always defined, do we need the #ifdef?

Thanks,
Easwar (he/him)
Nuno Das Neves Feb. 28, 2025, 12:21 a.m. UTC | #4
On 2/27/2025 10:04 AM, Roman Kisel wrote:
> 
> 
> On 2/26/2025 3:07 PM, Nuno Das Neves wrote:
>> Factor out the check for enabling auto eoi, to be reused in root
>> partition code.
>>
>> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
>> ---
> 
> I think adding "No functional changes" would bring some benefit:
> that's an additional invariant to check against when reviewing.
> 
Thanks, I can add it for next version :)
Nuno Das Neves Feb. 28, 2025, 12:33 a.m. UTC | #5
On 2/27/2025 3:03 PM, Easwar Hariharan wrote:
> On 2/26/2025 3:07 PM, Nuno Das Neves wrote:
>> Factor out the check for enabling auto eoi, to be reused in root
>> partition code.
>>
>> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
>> ---
>>  drivers/hv/hv.c                | 12 +-----------
>>  include/asm-generic/mshyperv.h | 13 +++++++++++++
>>  2 files changed, 14 insertions(+), 11 deletions(-)
>>
> 
> <snip>
> 
>> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
>> index 258034dfd829..1f46d19a16aa 100644
>> --- a/include/asm-generic/mshyperv.h
>> +++ b/include/asm-generic/mshyperv.h
>> @@ -77,6 +77,19 @@ extern u64 hv_do_fast_hypercall16(u16 control, u64 input1, u64 input2);
>>  bool hv_isolation_type_snp(void);
>>  bool hv_isolation_type_tdx(void);
>>  
>> +/*
>> + * On architectures where Hyper-V doesn't support AEOI (e.g., ARM64),
>> + * it doesn't provide a recommendation flag and AEOI must be disabled.
>> + */
>> +static inline bool hv_recommend_using_aeoi(void)
>> +{
>> +#ifdef HV_DEPRECATING_AEOI_RECOMMENDED
>> +	return !(ms_hyperv.hints & HV_DEPRECATING_AEOI_RECOMMENDED);
>> +#else
>> +	return false;
>> +#endif
>> +}
>> +
> 
> I must be missing something very basic here, and if so, I apologize, and please enlighten me.
> 
> HV_DEPRECATING_AEOI_RECOMMENDED is defined as BIT(9) in include/hyperv/hvgdk_mini.h, and
> asm-generic/mshyperv.h includes that via include/hyperv/hvhdk.h.
> 
> If this is the case, when would HV_DEPRECATING_AEOI_RECOMMENDED ever be not defined?
> If it's always defined, do we need the #ifdef?
> 
HV_DEPRECATING_AEOI_RECOMMENDED is only defined on x86 (it used to live in x86 hyperv-tlfs.h).
It lives inside a #if defined(CONFIG_X86) block in hvgdk_mini.h. It is a bit confusing since
it is surrounded by other x86-only definitions which are prefixed with HV_X64_.

Thanks
Nuno

> Thanks,
> Easwar (he/him)
Easwar Hariharan Feb. 28, 2025, 12:49 a.m. UTC | #6
On 2/27/2025 4:33 PM, Nuno Das Neves wrote:
> On 2/27/2025 3:03 PM, Easwar Hariharan wrote:
>> On 2/26/2025 3:07 PM, Nuno Das Neves wrote:
>>> Factor out the check for enabling auto eoi, to be reused in root
>>> partition code.
>>>
>>> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
>>> ---
>>>  drivers/hv/hv.c                | 12 +-----------
>>>  include/asm-generic/mshyperv.h | 13 +++++++++++++
>>>  2 files changed, 14 insertions(+), 11 deletions(-)
>>>
>>
>> <snip>
>>
>>> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
>>> index 258034dfd829..1f46d19a16aa 100644
>>> --- a/include/asm-generic/mshyperv.h
>>> +++ b/include/asm-generic/mshyperv.h
>>> @@ -77,6 +77,19 @@ extern u64 hv_do_fast_hypercall16(u16 control, u64 input1, u64 input2);
>>>  bool hv_isolation_type_snp(void);
>>>  bool hv_isolation_type_tdx(void);
>>>  
>>> +/*
>>> + * On architectures where Hyper-V doesn't support AEOI (e.g., ARM64),
>>> + * it doesn't provide a recommendation flag and AEOI must be disabled.
>>> + */
>>> +static inline bool hv_recommend_using_aeoi(void)
>>> +{
>>> +#ifdef HV_DEPRECATING_AEOI_RECOMMENDED
>>> +	return !(ms_hyperv.hints & HV_DEPRECATING_AEOI_RECOMMENDED);
>>> +#else
>>> +	return false;
>>> +#endif
>>> +}
>>> +
>>
>> I must be missing something very basic here, and if so, I apologize, and please enlighten me.
>>
>> HV_DEPRECATING_AEOI_RECOMMENDED is defined as BIT(9) in include/hyperv/hvgdk_mini.h, and
>> asm-generic/mshyperv.h includes that via include/hyperv/hvhdk.h.
>>
>> If this is the case, when would HV_DEPRECATING_AEOI_RECOMMENDED ever be not defined?
>> If it's always defined, do we need the #ifdef?
>>
> HV_DEPRECATING_AEOI_RECOMMENDED is only defined on x86 (it used to live in x86 hyperv-tlfs.h).
> It lives inside a #if defined(CONFIG_X86) block in hvgdk_mini.h. It is a bit confusing since
> it is surrounded by other x86-only definitions which are prefixed with HV_X64_.
> 

Ah, thank you. I knew it must be something glaringly obvious in hindsight. With that resolved,

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

Patch

diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index a38f84548bc2..308c8f279df8 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -313,17 +313,7 @@  void hv_synic_enable_regs(unsigned int cpu)
 
 	shared_sint.vector = vmbus_interrupt;
 	shared_sint.masked = false;
-
-	/*
-	 * On architectures where Hyper-V doesn't support AEOI (e.g., ARM64),
-	 * it doesn't provide a recommendation flag and AEOI must be disabled.
-	 */
-#ifdef HV_DEPRECATING_AEOI_RECOMMENDED
-	shared_sint.auto_eoi =
-			!(ms_hyperv.hints & HV_DEPRECATING_AEOI_RECOMMENDED);
-#else
-	shared_sint.auto_eoi = 0;
-#endif
+	shared_sint.auto_eoi = hv_recommend_using_aeoi();
 	hv_set_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
 
 	/* Enable the global synic bit */
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index 258034dfd829..1f46d19a16aa 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -77,6 +77,19 @@  extern u64 hv_do_fast_hypercall16(u16 control, u64 input1, u64 input2);
 bool hv_isolation_type_snp(void);
 bool hv_isolation_type_tdx(void);
 
+/*
+ * On architectures where Hyper-V doesn't support AEOI (e.g., ARM64),
+ * it doesn't provide a recommendation flag and AEOI must be disabled.
+ */
+static inline bool hv_recommend_using_aeoi(void)
+{
+#ifdef HV_DEPRECATING_AEOI_RECOMMENDED
+	return !(ms_hyperv.hints & HV_DEPRECATING_AEOI_RECOMMENDED);
+#else
+	return false;
+#endif
+}
+
 static inline struct hv_proximity_domain_info hv_numa_node_to_pxm_info(int node)
 {
 	struct hv_proximity_domain_info pxm_info = {};