The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* RE: [PATCH v2 09/15] Drivers: hv: mshv_vtl: Move hv_vtl_configure_reg_page() to x86
       [not found]     ` <567cf73b-6a48-4fc3-b312-9be6d71e2f22@linux.microsoft.com>
@ 2026-05-04 16:06       ` Michael Kelley
  2026-05-06  5:50         ` Naman Jain
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Kelley @ 2026-05-04 16:06 UTC (permalink / raw)
  To: Naman Jain, Michael Kelley, K . Y . Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Long Li, Catalin Marinas, Will Deacon,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	x86@kernel.org, H . Peter Anvin, Arnd Bergmann, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti
  Cc: Marc Zyngier, Timothy Hayes, Lorenzo Pieralisi, Sascha Bischoff,
	mrigendrachaubey, linux-hyperv@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	linux-riscv@lists.infradead.org, vdso@mailbox.org,
	ssengar@linux.microsoft.com

From: Naman Jain <namjain@linux.microsoft.com> Sent: Wednesday, April 29, 2026 2:58 AM
> 
> On 4/27/2026 11:10 AM, Michael Kelley wrote:
> > From: Naman Jain <namjain@linux.microsoft.com> Sent: Thursday, April 23, 2026 5:42 AM
> >>
> >> Move hv_vtl_configure_reg_page() from drivers/hv/mshv_vtl_main.c to
> >> arch/x86/hyperv/hv_vtl.c. The register page overlay is an x86-specific
> >> feature that uses HV_X64_REGISTER_REG_PAGE, so its configuration belongs
> >> in architecture-specific code.
> >>
> >> Move struct mshv_vtl_per_cpu and union hv_synic_overlay_page_msr to
> >> include/asm-generic/mshyperv.h so they are visible to both arch and
> >> driver code.
> >>
> >> Change the return type from void to bool so the caller can determine
> >> whether the register page was successfully configured and set
> >> mshv_has_reg_page accordingly.
> >>
> >> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> >> ---
> >>   arch/x86/hyperv/hv_vtl.c       | 32 ++++++++++++++++++++++
> >>   drivers/hv/mshv_vtl_main.c     | 49 +++-------------------------------
> >>   include/asm-generic/mshyperv.h | 17 ++++++++++++
> >>   3 files changed, 53 insertions(+), 45 deletions(-)
> >>
> 
> <snip>
> 
> >>   #if IS_ENABLED(CONFIG_HYPERV_VTL_MODE)
> >> +/* SYNIC_OVERLAY_PAGE_MSR - internal, identical to hv_synic_simp */
> >
> > This comment pre-dates your patch, but I don't understand the point
> > it is trying to make. The comment is factually true, but I don't know
> > why calling that out is relevant. The REG_PAGE MSR seems to be
> > conceptually separate and distinct from the SIMP MSR, so the fact
> > that the layouts are the same is just a coincidence. Or is there some
> > relationship between the two MSRs that I'm not aware of, and the
> > comment is trying (and failing?) to point out?
> 
> This was added as per suggestion from Nuno in my initial series for
> MSHV_VTL. If the reference in "identical to" is misleading, I should
> remove it.
> 
> https://lore.kernel.org/all/68143eb0-e6a7-4579-bedb-4c2ec5aaef6b@linux.microsoft.com/
> 
> Quoting:
> """
> it is a generic structure that
> appears to be used for several overlay page MSRs (SIMP, SIEF, etc).
> 
> But, the type doesn't appear in the hv*dk headers explicitly; it's just
> used internally by the hypervisor.
> 
> I think it should be renamed with a hv_ prefix to indicate it's part of
> the hypervisor ABI, and a brief comment with the provenance:
> 
> /* SYNIC_OVERLAY_PAGE_MSR - internal, identical to hv_synic_simp */
> union hv_synic_overlay_page_msr {
> 	/* <snip> */
> };

OK, so this union is not associated *only* with the REG_PAGE MSR
(though that MSR is the only current user). Instead, it is intended to
be a more generic description of MSRs that set up overlay pages. I
don't think I had previously noticed Nuno's comment on the topic.

Looking through hvgdk_mini.h and hvhdk.h, I see 6 definitions that
are exactly the same:

* union hv_reference_tsc_msr
* union hv_x64_msr_hypercall_contents
* union hv_vp_assist_msr_contents
* union hv_synic_simp
* union hv_synic_siefp
* union hv_synic_sirbp

There's an argument to be made for removing these 6 unique definitions
and using union hv_synic_overlay_page_msr instead (though "synic"
would need to be removed from the name).  I would not object to such
an approach. It's a small extra layer of conceptual indirection, but saves
some lines of code for duplicative definitions. The alternative is to drop
the idea of a generic overlay page MSR layout, and replace union
hv_synic_overlay_page_msr with a definition that is specific to the
REG_PAGE MSR, like the other six above. 

I could go either way. If we want to use a generic overlay page definition,
then that approach should be applied everywhere. With the current
state of your patch set, we're halfway in between -- the generic definition
is used one place, but duplicative specific MSR definitions are used other
places. That's probably the least desirable approach.

Michael

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH v2 07/15] arm64: hyperv: Add support for mshv_vtl_return_call
       [not found]     ` <516a4e65-3a4d-4e09-b445-28cb740b5653@linux.microsoft.com>
@ 2026-05-04 16:06       ` Michael Kelley
  2026-05-06  5:11         ` Naman Jain
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Kelley @ 2026-05-04 16:06 UTC (permalink / raw)
  To: Naman Jain, Michael Kelley, K . Y . Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Long Li, Catalin Marinas, Will Deacon,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	x86@kernel.org, H . Peter Anvin, Arnd Bergmann, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti
  Cc: Marc Zyngier, Timothy Hayes, Lorenzo Pieralisi, Sascha Bischoff,
	mrigendrachaubey, linux-hyperv@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	linux-riscv@lists.infradead.org, vdso@mailbox.org,
	ssengar@linux.microsoft.com

From: Naman Jain <namjain@linux.microsoft.com> Sent: Wednesday, April 29, 2026 2:57 AM
> 
> On 4/27/2026 11:08 AM, Michael Kelley wrote:
> > From: Naman Jain <namjain@linux.microsoft.com> Sent: Thursday, April 23, 2026 5:42 AM
> >>

[snip]

> >> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> >> index 08278547b84c..b4d80c9a673a 100644
> >> --- a/arch/x86/include/asm/mshyperv.h
> >> +++ b/arch/x86/include/asm/mshyperv.h
> >> @@ -286,7 +286,6 @@ struct mshv_vtl_cpu_context {
> >>   #ifdef CONFIG_HYPERV_VTL_MODE
> >>   void __init hv_vtl_init_platform(void);
> >>   int __init hv_vtl_early_init(void);
> >> -void mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0);
> >>   void mshv_vtl_return_call_init(u64 vtl_return_offset);
> >>   void mshv_vtl_return_hypercall(void);
> >>   void __mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0);
> >> @@ -294,7 +293,6 @@ int hv_vtl_get_set_reg(struct hv_register_assoc *regs, bool set, bool shared);
> >>   #else
> >>   static inline void __init hv_vtl_init_platform(void) {}
> >>   static inline int __init hv_vtl_early_init(void) { return 0; }
> >> -static inline void mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0) {}
> >>   static inline void mshv_vtl_return_call_init(u64 vtl_return_offset) {}
> >>   static inline void mshv_vtl_return_hypercall(void) {}
> >>   static inline void __mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0) {}
> >> diff --git a/drivers/hv/mshv_vtl.h b/drivers/hv/mshv_vtl.h
> >> index a6eea52f7aa2..103f07371f3f 100644
> >> --- a/drivers/hv/mshv_vtl.h
> >> +++ b/drivers/hv/mshv_vtl.h
> >> @@ -22,4 +22,7 @@ struct mshv_vtl_run {
> >>   	char vtl_ret_actions[MSHV_MAX_RUN_MSG_SIZE];
> >>   };
> >>
> >> +static_assert(sizeof(struct mshv_vtl_cpu_context) <= 1024,
> >> +	      "struct mshv_vtl_cpu_context exceeds reserved space in struct mshv_vtl_run");
> >> +
> >>   #endif /* _MSHV_VTL_H */
> >> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> >> index db183c8cfb95..8cdf2a9fbdfb 100644
> >> --- a/include/asm-generic/mshyperv.h
> >> +++ b/include/asm-generic/mshyperv.h
> >> @@ -396,8 +396,10 @@ static inline int hv_deposit_memory(u64 partition_id, u64 status)
> >>
> >>   #if IS_ENABLED(CONFIG_HYPERV_VTL_MODE)
> >>   u8 __init get_vtl(void);
> >> +void mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0);
> >>   #else
> >>   static inline u8 get_vtl(void) { return 0; }
> >> +static inline void mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0) {}
> >
> > Is this stub needed? Maybe I missed something, but it looks to me like none
> > of the code that calls this gets built unless CONFIG_HYPERV_VTL_MODE is set.
> > See further comments about stubs in Patch 8 of this series.
> >
> 
> Config dependencies would handle such cases, and this is not required. I
> saw similar stubs added in the code, so I thought this is a norm that
> should be followed, and not rely on config dependencies.
> I can remove it.
> 

Others might disagree with me, but I don't think it's the norm to add
stubs when they aren't truly needed. As you can see from some of my
other comments, I look for ways to eliminate stubs. Stubs are indicative
of a boundary between separately built components, and I generally
try to minimize the surface area of such boundaries. A large surface area
often means that the overall design could be improved by re-thinking
which code goes with which component.

Michael

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 07/15] arm64: hyperv: Add support for mshv_vtl_return_call
  2026-05-04 16:06       ` [PATCH v2 07/15] arm64: hyperv: Add support for mshv_vtl_return_call Michael Kelley
@ 2026-05-06  5:11         ` Naman Jain
  0 siblings, 0 replies; 10+ messages in thread
From: Naman Jain @ 2026-05-06  5:11 UTC (permalink / raw)
  To: Michael Kelley
  Cc: Marc Zyngier, Timothy Hayes, Lorenzo Pieralisi, Sascha Bischoff,
	mrigendrachaubey, linux-hyperv@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	linux-riscv@lists.infradead.org, vdso@mailbox.org,
	ssengar@linux.microsoft.com, K . Y . Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Long Li, Catalin Marinas, Will Deacon,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	x86@kernel.org, H . Peter Anvin, Arnd Bergmann, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti



On 5/4/2026 9:36 PM, Michael Kelley wrote:
> From: Naman Jain <namjain@linux.microsoft.com> Sent: Wednesday, April 29, 2026 2:57 AM
>>
>> On 4/27/2026 11:08 AM, Michael Kelley wrote:
>>> From: Naman Jain <namjain@linux.microsoft.com> Sent: Thursday, April 23, 2026 5:42 AM
>>>>
> 
> [snip]
> 
>>>> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
>>>> index 08278547b84c..b4d80c9a673a 100644
>>>> --- a/arch/x86/include/asm/mshyperv.h
>>>> +++ b/arch/x86/include/asm/mshyperv.h
>>>> @@ -286,7 +286,6 @@ struct mshv_vtl_cpu_context {
>>>>    #ifdef CONFIG_HYPERV_VTL_MODE
>>>>    void __init hv_vtl_init_platform(void);
>>>>    int __init hv_vtl_early_init(void);
>>>> -void mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0);
>>>>    void mshv_vtl_return_call_init(u64 vtl_return_offset);
>>>>    void mshv_vtl_return_hypercall(void);
>>>>    void __mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0);
>>>> @@ -294,7 +293,6 @@ int hv_vtl_get_set_reg(struct hv_register_assoc *regs, bool set, bool shared);
>>>>    #else
>>>>    static inline void __init hv_vtl_init_platform(void) {}
>>>>    static inline int __init hv_vtl_early_init(void) { return 0; }
>>>> -static inline void mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0) {}
>>>>    static inline void mshv_vtl_return_call_init(u64 vtl_return_offset) {}
>>>>    static inline void mshv_vtl_return_hypercall(void) {}
>>>>    static inline void __mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0) {}
>>>> diff --git a/drivers/hv/mshv_vtl.h b/drivers/hv/mshv_vtl.h
>>>> index a6eea52f7aa2..103f07371f3f 100644
>>>> --- a/drivers/hv/mshv_vtl.h
>>>> +++ b/drivers/hv/mshv_vtl.h
>>>> @@ -22,4 +22,7 @@ struct mshv_vtl_run {
>>>>    	char vtl_ret_actions[MSHV_MAX_RUN_MSG_SIZE];
>>>>    };
>>>>
>>>> +static_assert(sizeof(struct mshv_vtl_cpu_context) <= 1024,
>>>> +	      "struct mshv_vtl_cpu_context exceeds reserved space in struct mshv_vtl_run");
>>>> +
>>>>    #endif /* _MSHV_VTL_H */
>>>> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
>>>> index db183c8cfb95..8cdf2a9fbdfb 100644
>>>> --- a/include/asm-generic/mshyperv.h
>>>> +++ b/include/asm-generic/mshyperv.h
>>>> @@ -396,8 +396,10 @@ static inline int hv_deposit_memory(u64 partition_id, u64 status)
>>>>
>>>>    #if IS_ENABLED(CONFIG_HYPERV_VTL_MODE)
>>>>    u8 __init get_vtl(void);
>>>> +void mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0);
>>>>    #else
>>>>    static inline u8 get_vtl(void) { return 0; }
>>>> +static inline void mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0) {}
>>>
>>> Is this stub needed? Maybe I missed something, but it looks to me like none
>>> of the code that calls this gets built unless CONFIG_HYPERV_VTL_MODE is set.
>>> See further comments about stubs in Patch 8 of this series.
>>>
>>
>> Config dependencies would handle such cases, and this is not required. I
>> saw similar stubs added in the code, so I thought this is a norm that
>> should be followed, and not rely on config dependencies.
>> I can remove it.
>>
> 
> Others might disagree with me, but I don't think it's the norm to add
> stubs when they aren't truly needed. As you can see from some of my
> other comments, I look for ways to eliminate stubs. Stubs are indicative
> of a boundary between separately built components, and I generally
> try to minimize the surface area of such boundaries. A large surface area
> often means that the overall design could be improved by re-thinking
> which code goes with which component.
> 
> Michael

I agree. I'll remove these in next version.

Thanks,
Naman

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 09/15] Drivers: hv: mshv_vtl: Move hv_vtl_configure_reg_page() to x86
  2026-05-04 16:06       ` [PATCH v2 09/15] Drivers: hv: mshv_vtl: Move hv_vtl_configure_reg_page() to x86 Michael Kelley
@ 2026-05-06  5:50         ` Naman Jain
  2026-05-06 14:36           ` Michael Kelley
  0 siblings, 1 reply; 10+ messages in thread
From: Naman Jain @ 2026-05-06  5:50 UTC (permalink / raw)
  To: Michael Kelley
  Cc: Marc Zyngier, Timothy Hayes, Lorenzo Pieralisi, Sascha Bischoff,
	mrigendrachaubey, linux-hyperv@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	linux-riscv@lists.infradead.org, vdso@mailbox.org,
	ssengar@linux.microsoft.com, Michael Kelley, K . Y . Srinivasan,
	Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Catalin Marinas,
	Will Deacon, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86@kernel.org, H . Peter Anvin, Arnd Bergmann,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti



On 5/4/2026 9:36 PM, Michael Kelley wrote:
> From: Naman Jain <namjain@linux.microsoft.com> Sent: Wednesday, April 29, 2026 2:58 AM
>>
>> On 4/27/2026 11:10 AM, Michael Kelley wrote:
>>> From: Naman Jain <namjain@linux.microsoft.com> Sent: Thursday, April 23, 2026 5:42 AM
>>>>
>>>> Move hv_vtl_configure_reg_page() from drivers/hv/mshv_vtl_main.c to
>>>> arch/x86/hyperv/hv_vtl.c. The register page overlay is an x86-specific
>>>> feature that uses HV_X64_REGISTER_REG_PAGE, so its configuration belongs
>>>> in architecture-specific code.
>>>>
>>>> Move struct mshv_vtl_per_cpu and union hv_synic_overlay_page_msr to
>>>> include/asm-generic/mshyperv.h so they are visible to both arch and
>>>> driver code.
>>>>
>>>> Change the return type from void to bool so the caller can determine
>>>> whether the register page was successfully configured and set
>>>> mshv_has_reg_page accordingly.
>>>>
>>>> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
>>>> ---
>>>>    arch/x86/hyperv/hv_vtl.c       | 32 ++++++++++++++++++++++
>>>>    drivers/hv/mshv_vtl_main.c     | 49 +++-------------------------------
>>>>    include/asm-generic/mshyperv.h | 17 ++++++++++++
>>>>    3 files changed, 53 insertions(+), 45 deletions(-)
>>>>
>>
>> <snip>
>>
>>>>    #if IS_ENABLED(CONFIG_HYPERV_VTL_MODE)
>>>> +/* SYNIC_OVERLAY_PAGE_MSR - internal, identical to hv_synic_simp */
>>>
>>> This comment pre-dates your patch, but I don't understand the point
>>> it is trying to make. The comment is factually true, but I don't know
>>> why calling that out is relevant. The REG_PAGE MSR seems to be
>>> conceptually separate and distinct from the SIMP MSR, so the fact
>>> that the layouts are the same is just a coincidence. Or is there some
>>> relationship between the two MSRs that I'm not aware of, and the
>>> comment is trying (and failing?) to point out?
>>
>> This was added as per suggestion from Nuno in my initial series for
>> MSHV_VTL. If the reference in "identical to" is misleading, I should
>> remove it.
>>
>> https://lore.kernel.org/all/68143eb0-e6a7-4579-bedb-4c2ec5aaef6b@linux.microsoft.com/
>>
>> Quoting:
>> """
>> it is a generic structure that
>> appears to be used for several overlay page MSRs (SIMP, SIEF, etc).
>>
>> But, the type doesn't appear in the hv*dk headers explicitly; it's just
>> used internally by the hypervisor.
>>
>> I think it should be renamed with a hv_ prefix to indicate it's part of
>> the hypervisor ABI, and a brief comment with the provenance:
>>
>> /* SYNIC_OVERLAY_PAGE_MSR - internal, identical to hv_synic_simp */
>> union hv_synic_overlay_page_msr {
>> 	/* <snip> */
>> };
> 
> OK, so this union is not associated *only* with the REG_PAGE MSR
> (though that MSR is the only current user). Instead, it is intended to
> be a more generic description of MSRs that set up overlay pages. I
> don't think I had previously noticed Nuno's comment on the topic.
> 
> Looking through hvgdk_mini.h and hvhdk.h, I see 6 definitions that
> are exactly the same:
> 
> * union hv_reference_tsc_msr
> * union hv_x64_msr_hypercall_contents
> * union hv_vp_assist_msr_contents
> * union hv_synic_simp
> * union hv_synic_siefp
> * union hv_synic_sirbp
> 
> There's an argument to be made for removing these 6 unique definitions
> and using union hv_synic_overlay_page_msr instead (though "synic"
> would need to be removed from the name).  I would not object to such
> an approach. It's a small extra layer of conceptual indirection, but saves
> some lines of code for duplicative definitions. The alternative is to drop
> the idea of a generic overlay page MSR layout, and replace union
> hv_synic_overlay_page_msr with a definition that is specific to the
> REG_PAGE MSR, like the other six above.
> 

Hi Michael,

While having a generic definition looks good to have here, I can see two 
reasons for not going ahead with generic overlay page definition:
1. All of the above definitions are present in Hyper-V headers and 
generalizing them would deviate from the strategy of keeping the kernel 
headers in line with Hyper-V headers.
2. For any of these definitions, if the use-case requires using some of 
these reserved bits, then it would be a problem. I can actually see that 
happening in "hv_x64_msr_hypercall_contents" in the corresponding 
variant in the Hyper-V header.

> I could go either way. If we want to use a generic overlay page definition,
> then that approach should be applied everywhere. With the current
> state of your patch set, we're halfway in between -- the generic definition
> is used one place, but duplicative specific MSR definitions are used other
> places. That's probably the least desirable approach.
> 
> Michael


Now, coming back to the hv_synic_overlay_page_msr definition. While 
Nuno's comment hinted at it being "generic", the same is not documented 
in the name of this structure or its comments. So it should be safe to 
assume that it is specific to synic_overlay_page_msr usage. But since it 
is not part of Hyper-V header as such, we needed that comment:
"/* SYNIC_OVERLAY_PAGE_MSR - internal, identical to hv_synic_simp */"

Please let me know your thoughts on this.

Regards,
Naman

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 07/15] arm64: hyperv: Add support for mshv_vtl_return_call
       [not found]     ` <f4059f5d-a82b-40c2-942e-3e24cefab94f@linux.microsoft.com>
@ 2026-05-06  7:52       ` Mark Rutland
  2026-05-08  4:26         ` Naman Jain
  2026-05-08  9:25       ` Marc Zyngier
  1 sibling, 1 reply; 10+ messages in thread
From: Mark Rutland @ 2026-05-06  7:52 UTC (permalink / raw)
  To: Naman Jain
  Cc: Marc Zyngier, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
	Dexuan Cui, Long Li, Catalin Marinas, Will Deacon,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H . Peter Anvin, Arnd Bergmann, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Alexandre Ghiti, Michael Kelley, Timothy Hayes,
	Lorenzo Pieralisi, Sascha Bischoff, mrigendrachaubey,
	linux-hyperv, linux-arm-kernel, linux-kernel, linux-arch,
	linux-riscv, vdso, ssengar

On Wed, Apr 29, 2026 at 03:26:11PM +0530, Naman Jain wrote:
> On 4/23/2026 7:26 PM, Mark Rutland wrote:
> > On Thu, Apr 23, 2026 at 12:41:57PM +0000, Naman Jain wrote:

[ non-SMMC hypercall code omitted for brevity ]

> > NAK to this.
> > 
> > * This is a non-SMCCC hypercall, which we have NAK'd in general in the
> >    past for various reasons that I am not going to rehash here.
> > 
> > * It's not clear how this is going to be extended with necessary
> >    architecture state in future (e.g. SVE, SME). This is not
> >    future-proof, and I don't believe this is maintainable.
> > 
> > * This breaks general requirements for reliable stacktracing by
> >    clobbering state (e.g. x29) that we depend upon being valid AT ALL
> >    TIMES outside of entry code.
> > 
> > * IMO, if this needs to be saved/restored, that should happen in
> >    whatever you are calling.
> > 
> > Mark.
> 
> Merging threads for addressing comments from Mark Rutland and Marc Zyngier
> on this patch.
> 
> Thanks for reviewing the changes. Please allow me to briefly explain the use
> case here and then address your comments.
> 
> Hyper-V's Virtual Trust Levels (VTLs) provide hardware-enforced isolation
> within a single VM, analogous to ARM TrustZone. The kernel runs in VTL2
> (higher privilege) as a "paravisor", a security monitor that handles
> intercepts for the primary OS in VTL0 (lower privilege). The VTL switch
> (mshv_vtl_return_call) is functionally equivalent to KVM's guest enter/exit.

It's worth noting that for KVM, the KVM hyp code is *tightly* coupled
with the host kernel (they are one single binary object), and the
calling convention between the two is an implementation detail that can
change at any time without any ABI concerns.

While I appreciate this might be trying to do the same thing from a
*functional* perspective, it's certainly different from a
maintainability perspective, and can't be treated in the same way.

> It saves VTL2 state, loads VTL0's GPRs other registers from a shared context
> structure, issues hvc #3 to let VTL0 run, and on return saves VTL0's updated
> state back.
> 
> Coming to the problems with the code, I have identified a few ways to
> address them.
> 
> I can put the assembly code in a separate .S file with
> SYM_FUNC_START/SYM_FUNC_END and marked as noinstr, to prevent ftrace/kprobes
> from instrumenting between the GPR load and the hvc, which could have
> corrupted VTL0 register state. This should solve x29 clobbering, stack
> tracing problems.

My point was that you must not clobber those registers.

Looking at the TLFS document you linked below, it says:

| Note: X29 (FP/frame pointer), X30 (LR/link register), and SP are private
| per-VTL

... so clobbering those doesn't seem to be necessary anyway. Clearly
having an arbitrary calling convention is confusing for everyone.

> I should use kernel_neon_begin()/kernel_neon_end() to save/restore the full
> extended FP state of the current task in VTL2. VTL0's Q0-Q31 can be
> loaded/saved separately via fpsimd_load_state()/fpsimd_save_state(). This
> way, the assembly touches none of the SIMD registers. This is SVE/SME-safe
> for VTL2's task state. VTL0 still only carries Q0-Q31 in the context struct,
> and extending to SVE, SME is a future context struct change, which will need
> Hyper-V arm64 ABI support.
> This way, VTL2's callee-saved regs (x19-x28, x29, x30) are explicitly saved
> to the stack frame at the top and restored at the bottom of assembly code.
> The C caller (in hv_vtl.c) is a clean function call.

That doesn't really address my concerns here.

I do not think that Linux should have to save/restore anything here;
that should be the job of the real hypervisor. The arbitrary separation
of PE state into private and shared (with shred state being directly
exposed to Linux) is a problem for maintainability and forward
compatibility.

Looking at the TLFS document you linked below, I see:

| Note: SVE state (Z0-Z31, P0-P15, FFR) and SME state are VTL-private.
| The lower 128-bit portion (Q registers) is shared, but the upper bits
| of Z registers may be corrupted on VTL transitions. Software should
| not rely on Z register contents being preserved across VTL switches.

... which is certainly going to be a pain to manage.

Note in particular "SME state" is not an architectural term. I don't
know which state in particular that is intended to cover (e.g. ZA, ZT0,
SVCR, all streaming mode state)?

There's no mention of SVCR, so I don't know how this is going to
interact with management of ZA state (ZA and ZT0, which are dependent
upon SVCR.ZA) or streaming mode (dependent upon SVCR.SM). That state has
been *incredibly* painful for us to manage generally. Regardless of the
SMCCC concerns, that needs to be specified better.

> Regarding Non-SMCCC "hvc #3" call, I have a limitation here owing to the ABI
> that is defined by the Hyper-V hypervisor. Fixing this requires a
> hypervisor-side change to support SMCCC-style dispatch for VTL return. Until
> then, hvc #3 is the only working interface. Moreover there would be backward
> compatibility issues with this new ABI interface, if at all it is added.

To be clear, that's Microsoft's problem, not the Linux kernel
community's problem. My NAK still stands.

Multiple years ago now, we made it clear that we would not accept a
non-SMCCC calling convention. Ignoring the substance of that feedback,
and inventing a new calling convention after that point is a
self-inflicted problem.

[...]

> Link to TLFS: https://learn.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/vsm#on-arm64-platforms-3

For shared state, aside fomr GPRs and FPSIMD/SVE/SME state, that says:

| * System Information Registers (read-only or non-security-critical):
|   * System identification and feature registers
|   * Cache and TLB type information

It's *implied* that some of those registers might be writable, but as
the specific set of registers is not described I cannot tell. Are there
any writable system registers which are shared?

I don't see how we can know which registers we might need to
save/restore without that being explicitly documented.

I also see:

| Note: SPE (Statistical Profiling Extension) state is shared across VTLs,
| except for PMBSR_EL1 which is VTL-private.

If "SPE state" includes PMBPTR or PMBLIMITR (which is the obvious
reading), this would be a security problem, as a lower-privileged VTL
could clobber those and cause SPE to write to arbitrary memory
immediately upon return to the higher-privileged VTL. Having PMBSR be
private on its own isn't sufficient to prevent that (e.g. since the
higher-privileged VTL could have its own active SPE profiling session).

I'm not keen on requiring hyper-v specific hooks in the SPE driver to
achieve that, and I'm also not keen on having hyper-v support code poke
SPE registers behind the SPE driver's back.

This does not give me confidence that any future PE state (e.g. things
like TRBE) will be managed in a safe way either.

Mark.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH v2 09/15] Drivers: hv: mshv_vtl: Move hv_vtl_configure_reg_page() to x86
  2026-05-06  5:50         ` Naman Jain
@ 2026-05-06 14:36           ` Michael Kelley
  2026-05-07  3:43             ` Naman Jain
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Kelley @ 2026-05-06 14:36 UTC (permalink / raw)
  To: Naman Jain, Michael Kelley
  Cc: Marc Zyngier, Timothy Hayes, Lorenzo Pieralisi, Sascha Bischoff,
	mrigendrachaubey, linux-hyperv@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	linux-riscv@lists.infradead.org, vdso@mailbox.org,
	ssengar@linux.microsoft.com, Michael Kelley, K . Y . Srinivasan,
	Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Catalin Marinas,
	Will Deacon, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86@kernel.org, H . Peter Anvin, Arnd Bergmann,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti

From: Naman Jain <namjain@linux.microsoft.com> Sent: Tuesday, May 5, 2026 10:50 PM
> 
> On 5/4/2026 9:36 PM, Michael Kelley wrote:
> > From: Naman Jain <namjain@linux.microsoft.com> Sent: Wednesday, April 29, 2026 2:58 AM
> >>
> >> On 4/27/2026 11:10 AM, Michael Kelley wrote:
> >>> From: Naman Jain <namjain@linux.microsoft.com> Sent: Thursday, April 23, 2026 5:42 AM
> >>>>
> >>>> Move hv_vtl_configure_reg_page() from drivers/hv/mshv_vtl_main.c to
> >>>> arch/x86/hyperv/hv_vtl.c. The register page overlay is an x86-specific
> >>>> feature that uses HV_X64_REGISTER_REG_PAGE, so its configuration belongs
> >>>> in architecture-specific code.
> >>>>
> >>>> Move struct mshv_vtl_per_cpu and union hv_synic_overlay_page_msr to
> >>>> include/asm-generic/mshyperv.h so they are visible to both arch and
> >>>> driver code.
> >>>>
> >>>> Change the return type from void to bool so the caller can determine
> >>>> whether the register page was successfully configured and set
> >>>> mshv_has_reg_page accordingly.
> >>>>
> >>>> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> >>>> ---
> >>>>    arch/x86/hyperv/hv_vtl.c       | 32 ++++++++++++++++++++++
> >>>>    drivers/hv/mshv_vtl_main.c     | 49 +++-------------------------------
> >>>>    include/asm-generic/mshyperv.h | 17 ++++++++++++
> >>>>    3 files changed, 53 insertions(+), 45 deletions(-)
> >>>>
> >>
> >> <snip>
> >>
> >>>>    #if IS_ENABLED(CONFIG_HYPERV_VTL_MODE)
> >>>> +/* SYNIC_OVERLAY_PAGE_MSR - internal, identical to hv_synic_simp */
> >>>
> >>> This comment pre-dates your patch, but I don't understand the point
> >>> it is trying to make. The comment is factually true, but I don't know
> >>> why calling that out is relevant. The REG_PAGE MSR seems to be
> >>> conceptually separate and distinct from the SIMP MSR, so the fact
> >>> that the layouts are the same is just a coincidence. Or is there some
> >>> relationship between the two MSRs that I'm not aware of, and the
> >>> comment is trying (and failing?) to point out?
> >>
> >> This was added as per suggestion from Nuno in my initial series for
> >> MSHV_VTL. If the reference in "identical to" is misleading, I should
> >> remove it.
> >>
> >> https://lore.kernel.org/all/68143eb0-e6a7-4579-bedb-4c2ec5aaef6b@linux.microsoft.com/
> >>
> >> Quoting:
> >> """
> >> it is a generic structure that
> >> appears to be used for several overlay page MSRs (SIMP, SIEF, etc).
> >>
> >> But, the type doesn't appear in the hv*dk headers explicitly; it's just
> >> used internally by the hypervisor.
> >>
> >> I think it should be renamed with a hv_ prefix to indicate it's part of
> >> the hypervisor ABI, and a brief comment with the provenance:
> >>
> >> /* SYNIC_OVERLAY_PAGE_MSR - internal, identical to hv_synic_simp */
> >> union hv_synic_overlay_page_msr {
> >> 	/* <snip> */
> >> };
> >
> > OK, so this union is not associated *only* with the REG_PAGE MSR
> > (though that MSR is the only current user). Instead, it is intended to
> > be a more generic description of MSRs that set up overlay pages. I
> > don't think I had previously noticed Nuno's comment on the topic.
> >
> > Looking through hvgdk_mini.h and hvhdk.h, I see 6 definitions that
> > are exactly the same:
> >
> > * union hv_reference_tsc_msr
> > * union hv_x64_msr_hypercall_contents
> > * union hv_vp_assist_msr_contents
> > * union hv_synic_simp
> > * union hv_synic_siefp
> > * union hv_synic_sirbp
> >
> > There's an argument to be made for removing these 6 unique definitions
> > and using union hv_synic_overlay_page_msr instead (though "synic"
> > would need to be removed from the name).  I would not object to such
> > an approach. It's a small extra layer of conceptual indirection, but saves
> > some lines of code for duplicative definitions. The alternative is to drop
> > the idea of a generic overlay page MSR layout, and replace union
> > hv_synic_overlay_page_msr with a definition that is specific to the
> > REG_PAGE MSR, like the other six above.
> >
> 
> Hi Michael,
> 
> While having a generic definition looks good to have here, I can see two
> reasons for not going ahead with generic overlay page definition:
> 1. All of the above definitions are present in Hyper-V headers and
> generalizing them would deviate from the strategy of keeping the kernel
> headers in line with Hyper-V headers.
> 2. For any of these definitions, if the use-case requires using some of
> these reserved bits, then it would be a problem. I can actually see that
> happening in "hv_x64_msr_hypercall_contents" in the corresponding
> variant in the Hyper-V header.

Your points are certainly valid, and I'm good with not going the
generic route.

> 
> > I could go either way. If we want to use a generic overlay page definition,
> > then that approach should be applied everywhere. With the current
> > state of your patch set, we're halfway in between -- the generic definition
> > is used one place, but duplicative specific MSR definitions are used other
> > places. That's probably the least desirable approach.
> >
> > Michael
> 
> 
> Now, coming back to the hv_synic_overlay_page_msr definition. While
> Nuno's comment hinted at it being "generic", the same is not documented
> in the name of this structure or its comments. So it should be safe to
> assume that it is specific to synic_overlay_page_msr usage. But since it
> is not part of Hyper-V header as such, we needed that comment:
> "/* SYNIC_OVERLAY_PAGE_MSR - internal, identical to hv_synic_simp */"
> 

An "overlay page" is a generic concept in the Hyper-V world, and it is used
in multiple places in the guest<->hypervisor interface. The old PDF version of
the Hyper-V TLFS describes overlay pages in the section 5.2.1 entitled "GPA
Overlay Pages". See [1]. Unfortunately, this material about overlay pages
doesn't seem to have been carried over to the web page version of the TLFS.

So in my thinking, the name "hv_synic_overlay_page_msr" is inherently
a generic definition that could be applied to multiple MSRs that are used to
specify overlay pages. Your patch is about a specific MSR,
HV_X64_REGISTER_REG_PAGE, which happens to be used to define an
overlay page. But if the decision is to *not* go the generic route, I
would expect to see something like "union hv_x64_reg_page_msr"
that is specific to the REG_PAGE MSR, and to have that type used in
hv_vtl_configure_reg_page(). The definition of hv_x64_reg_page_msr
would not have a comment referencing the SIMP or any other MSR
because it would be a standalone definition that is specific to
HV_X64_REGISTER_REG_PAGE. Then the pattern would be the same as
the other six cases that I listed above.

When not using the generic approach, hv_synic_overlay_page_msr
really has no purpose, and could go away.

Michael

[1] https://github.com/MicrosoftDocs/Virtualization-Documentation/raw/live/tlfs/Hypervisor%20Top%20Level%20Functional%20Specification%20v6.0b.pdf

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 09/15] Drivers: hv: mshv_vtl: Move hv_vtl_configure_reg_page() to x86
  2026-05-06 14:36           ` Michael Kelley
@ 2026-05-07  3:43             ` Naman Jain
  0 siblings, 0 replies; 10+ messages in thread
From: Naman Jain @ 2026-05-07  3:43 UTC (permalink / raw)
  To: Michael Kelley
  Cc: Marc Zyngier, Timothy Hayes, Lorenzo Pieralisi, Sascha Bischoff,
	mrigendrachaubey, linux-hyperv@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	linux-riscv@lists.infradead.org, vdso@mailbox.org,
	ssengar@linux.microsoft.com, K . Y . Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Long Li, Catalin Marinas, Will Deacon,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	x86@kernel.org, H . Peter Anvin, Arnd Bergmann, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti



On 5/6/2026 8:06 PM, Michael Kelley wrote:
> From: Naman Jain <namjain@linux.microsoft.com> Sent: Tuesday, May 5, 2026 10:50 PM
>>
>> On 5/4/2026 9:36 PM, Michael Kelley wrote:
>>> From: Naman Jain <namjain@linux.microsoft.com> Sent: Wednesday, April 29, 2026 2:58 AM
>>>>
>>>> On 4/27/2026 11:10 AM, Michael Kelley wrote:
>>>>> From: Naman Jain <namjain@linux.microsoft.com> Sent: Thursday, April 23, 2026 5:42 AM
>>>>>>
>>>>>> Move hv_vtl_configure_reg_page() from drivers/hv/mshv_vtl_main.c to
>>>>>> arch/x86/hyperv/hv_vtl.c. The register page overlay is an x86-specific
>>>>>> feature that uses HV_X64_REGISTER_REG_PAGE, so its configuration belongs
>>>>>> in architecture-specific code.
>>>>>>
>>>>>> Move struct mshv_vtl_per_cpu and union hv_synic_overlay_page_msr to
>>>>>> include/asm-generic/mshyperv.h so they are visible to both arch and
>>>>>> driver code.
>>>>>>
>>>>>> Change the return type from void to bool so the caller can determine
>>>>>> whether the register page was successfully configured and set
>>>>>> mshv_has_reg_page accordingly.
>>>>>>
>>>>>> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
>>>>>> ---
>>>>>>     arch/x86/hyperv/hv_vtl.c       | 32 ++++++++++++++++++++++
>>>>>>     drivers/hv/mshv_vtl_main.c     | 49 +++-------------------------------
>>>>>>     include/asm-generic/mshyperv.h | 17 ++++++++++++
>>>>>>     3 files changed, 53 insertions(+), 45 deletions(-)
>>>>>>
>>>>
>>>> <snip>
>>>>
>>>>>>     #if IS_ENABLED(CONFIG_HYPERV_VTL_MODE)
>>>>>> +/* SYNIC_OVERLAY_PAGE_MSR - internal, identical to hv_synic_simp */
>>>>>
>>>>> This comment pre-dates your patch, but I don't understand the point
>>>>> it is trying to make. The comment is factually true, but I don't know
>>>>> why calling that out is relevant. The REG_PAGE MSR seems to be
>>>>> conceptually separate and distinct from the SIMP MSR, so the fact
>>>>> that the layouts are the same is just a coincidence. Or is there some
>>>>> relationship between the two MSRs that I'm not aware of, and the
>>>>> comment is trying (and failing?) to point out?
>>>>
>>>> This was added as per suggestion from Nuno in my initial series for
>>>> MSHV_VTL. If the reference in "identical to" is misleading, I should
>>>> remove it.
>>>>
>>>> https://lore.kernel.org/all/68143eb0-e6a7-4579-bedb-4c2ec5aaef6b@linux.microsoft.com/
>>>>
>>>> Quoting:
>>>> """
>>>> it is a generic structure that
>>>> appears to be used for several overlay page MSRs (SIMP, SIEF, etc).
>>>>
>>>> But, the type doesn't appear in the hv*dk headers explicitly; it's just
>>>> used internally by the hypervisor.
>>>>
>>>> I think it should be renamed with a hv_ prefix to indicate it's part of
>>>> the hypervisor ABI, and a brief comment with the provenance:
>>>>
>>>> /* SYNIC_OVERLAY_PAGE_MSR - internal, identical to hv_synic_simp */
>>>> union hv_synic_overlay_page_msr {
>>>> 	/* <snip> */
>>>> };
>>>
>>> OK, so this union is not associated *only* with the REG_PAGE MSR
>>> (though that MSR is the only current user). Instead, it is intended to
>>> be a more generic description of MSRs that set up overlay pages. I
>>> don't think I had previously noticed Nuno's comment on the topic.
>>>
>>> Looking through hvgdk_mini.h and hvhdk.h, I see 6 definitions that
>>> are exactly the same:
>>>
>>> * union hv_reference_tsc_msr
>>> * union hv_x64_msr_hypercall_contents
>>> * union hv_vp_assist_msr_contents
>>> * union hv_synic_simp
>>> * union hv_synic_siefp
>>> * union hv_synic_sirbp
>>>
>>> There's an argument to be made for removing these 6 unique definitions
>>> and using union hv_synic_overlay_page_msr instead (though "synic"
>>> would need to be removed from the name).  I would not object to such
>>> an approach. It's a small extra layer of conceptual indirection, but saves
>>> some lines of code for duplicative definitions. The alternative is to drop
>>> the idea of a generic overlay page MSR layout, and replace union
>>> hv_synic_overlay_page_msr with a definition that is specific to the
>>> REG_PAGE MSR, like the other six above.
>>>
>>
>> Hi Michael,
>>
>> While having a generic definition looks good to have here, I can see two
>> reasons for not going ahead with generic overlay page definition:
>> 1. All of the above definitions are present in Hyper-V headers and
>> generalizing them would deviate from the strategy of keeping the kernel
>> headers in line with Hyper-V headers.
>> 2. For any of these definitions, if the use-case requires using some of
>> these reserved bits, then it would be a problem. I can actually see that
>> happening in "hv_x64_msr_hypercall_contents" in the corresponding
>> variant in the Hyper-V header.
> 
> Your points are certainly valid, and I'm good with not going the
> generic route.
> 
>>
>>> I could go either way. If we want to use a generic overlay page definition,
>>> then that approach should be applied everywhere. With the current
>>> state of your patch set, we're halfway in between -- the generic definition
>>> is used one place, but duplicative specific MSR definitions are used other
>>> places. That's probably the least desirable approach.
>>>
>>> Michael
>>
>>
>> Now, coming back to the hv_synic_overlay_page_msr definition. While
>> Nuno's comment hinted at it being "generic", the same is not documented
>> in the name of this structure or its comments. So it should be safe to
>> assume that it is specific to synic_overlay_page_msr usage. But since it
>> is not part of Hyper-V header as such, we needed that comment:
>> "/* SYNIC_OVERLAY_PAGE_MSR - internal, identical to hv_synic_simp */"
>>
> 
> An "overlay page" is a generic concept in the Hyper-V world, and it is used
> in multiple places in the guest<->hypervisor interface. The old PDF version of
> the Hyper-V TLFS describes overlay pages in the section 5.2.1 entitled "GPA
> Overlay Pages". See [1]. Unfortunately, this material about overlay pages
> doesn't seem to have been carried over to the web page version of the TLFS.
> 
> So in my thinking, the name "hv_synic_overlay_page_msr" is inherently
> a generic definition that could be applied to multiple MSRs that are used to
> specify overlay pages. Your patch is about a specific MSR,
> HV_X64_REGISTER_REG_PAGE, which happens to be used to define an
> overlay page. But if the decision is to *not* go the generic route, I
> would expect to see something like "union hv_x64_reg_page_msr"
> that is specific to the REG_PAGE MSR, and to have that type used in
> hv_vtl_configure_reg_page(). The definition of hv_x64_reg_page_msr
> would not have a comment referencing the SIMP or any other MSR
> because it would be a standalone definition that is specific to
> HV_X64_REGISTER_REG_PAGE. Then the pattern would be the same as
> the other six cases that I listed above.
> 
> When not using the generic approach, hv_synic_overlay_page_msr
> really has no purpose, and could go away.
> 
> Michael
> 
> [1] https://github.com/MicrosoftDocs/Virtualization-Documentation/raw/live/tlfs/Hypervisor%20Top%20Level%20Functional%20Specification%20v6.0b.pdf


Thanks for suggesting this, I was not aware of it. I'll change it to 
hv_x64_reg_page_msr and remove the comment.

Regards,
Naman


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 07/15] arm64: hyperv: Add support for mshv_vtl_return_call
  2026-05-06  7:52       ` Mark Rutland
@ 2026-05-08  4:26         ` Naman Jain
  0 siblings, 0 replies; 10+ messages in thread
From: Naman Jain @ 2026-05-08  4:26 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Marc Zyngier, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
	Dexuan Cui, Long Li, Catalin Marinas, Will Deacon,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H . Peter Anvin, Arnd Bergmann, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Alexandre Ghiti, Michael Kelley, Timothy Hayes,
	Lorenzo Pieralisi, Sascha Bischoff, mrigendrachaubey,
	linux-hyperv, linux-arm-kernel, linux-kernel, linux-arch,
	linux-riscv, vdso, ssengar



On 5/6/2026 1:22 PM, Mark Rutland wrote:
> On Wed, Apr 29, 2026 at 03:26:11PM +0530, Naman Jain wrote:
>> On 4/23/2026 7:26 PM, Mark Rutland wrote:
>>> On Thu, Apr 23, 2026 at 12:41:57PM +0000, Naman Jain wrote:
> 
> [ non-SMMC hypercall code omitted for brevity ]
> 
>>> NAK to this.
>>>
>>> * This is a non-SMCCC hypercall, which we have NAK'd in general in the
>>>     past for various reasons that I am not going to rehash here.
>>>
>>> * It's not clear how this is going to be extended with necessary
>>>     architecture state in future (e.g. SVE, SME). This is not
>>>     future-proof, and I don't believe this is maintainable.
>>>
>>> * This breaks general requirements for reliable stacktracing by
>>>     clobbering state (e.g. x29) that we depend upon being valid AT ALL
>>>     TIMES outside of entry code.
>>>
>>> * IMO, if this needs to be saved/restored, that should happen in
>>>     whatever you are calling.
>>>
>>> Mark.
>>
>> Merging threads for addressing comments from Mark Rutland and Marc Zyngier
>> on this patch.
>>
>> Thanks for reviewing the changes. Please allow me to briefly explain the use
>> case here and then address your comments.
>>
>> Hyper-V's Virtual Trust Levels (VTLs) provide hardware-enforced isolation
>> within a single VM, analogous to ARM TrustZone. The kernel runs in VTL2
>> (higher privilege) as a "paravisor", a security monitor that handles
>> intercepts for the primary OS in VTL0 (lower privilege). The VTL switch
>> (mshv_vtl_return_call) is functionally equivalent to KVM's guest enter/exit.
> 
> It's worth noting that for KVM, the KVM hyp code is *tightly* coupled
> with the host kernel (they are one single binary object), and the
> calling convention between the two is an implementation detail that can
> change at any time without any ABI concerns.
> 
> While I appreciate this might be trying to do the same thing from a
> *functional* perspective, it's certainly different from a
> maintainability perspective, and can't be treated in the same way.
> 
>> It saves VTL2 state, loads VTL0's GPRs other registers from a shared context
>> structure, issues hvc #3 to let VTL0 run, and on return saves VTL0's updated
>> state back.
>>
>> Coming to the problems with the code, I have identified a few ways to
>> address them.
>>
>> I can put the assembly code in a separate .S file with
>> SYM_FUNC_START/SYM_FUNC_END and marked as noinstr, to prevent ftrace/kprobes
>> from instrumenting between the GPR load and the hvc, which could have
>> corrupted VTL0 register state. This should solve x29 clobbering, stack
>> tracing problems.
> 
> My point was that you must not clobber those registers.
> 
> Looking at the TLFS document you linked below, it says:
> 
> | Note: X29 (FP/frame pointer), X30 (LR/link register), and SP are private
> | per-VTL
> 
> ... so clobbering those doesn't seem to be necessary anyway. Clearly
> having an arbitrary calling convention is confusing for everyone.
> 
>> I should use kernel_neon_begin()/kernel_neon_end() to save/restore the full
>> extended FP state of the current task in VTL2. VTL0's Q0-Q31 can be
>> loaded/saved separately via fpsimd_load_state()/fpsimd_save_state(). This
>> way, the assembly touches none of the SIMD registers. This is SVE/SME-safe
>> for VTL2's task state. VTL0 still only carries Q0-Q31 in the context struct,
>> and extending to SVE, SME is a future context struct change, which will need
>> Hyper-V arm64 ABI support.
>> This way, VTL2's callee-saved regs (x19-x28, x29, x30) are explicitly saved
>> to the stack frame at the top and restored at the bottom of assembly code.
>> The C caller (in hv_vtl.c) is a clean function call.
> 
> That doesn't really address my concerns here.
> 
> I do not think that Linux should have to save/restore anything here;
> that should be the job of the real hypervisor. The arbitrary separation
> of PE state into private and shared (with shred state being directly
> exposed to Linux) is a problem for maintainability and forward
> compatibility.
> 
> Looking at the TLFS document you linked below, I see:
> 
> | Note: SVE state (Z0-Z31, P0-P15, FFR) and SME state are VTL-private.
> | The lower 128-bit portion (Q registers) is shared, but the upper bits
> | of Z registers may be corrupted on VTL transitions. Software should
> | not rely on Z register contents being preserved across VTL switches.
> 
> ... which is certainly going to be a pain to manage.
> 
> Note in particular "SME state" is not an architectural term. I don't
> know which state in particular that is intended to cover (e.g. ZA, ZT0,
> SVCR, all streaming mode state)?
> 
> There's no mention of SVCR, so I don't know how this is going to
> interact with management of ZA state (ZA and ZT0, which are dependent
> upon SVCR.ZA) or streaming mode (dependent upon SVCR.SM). That state has
> been *incredibly* painful for us to manage generally. Regardless of the
> SMCCC concerns, that needs to be specified better.
> 
>> Regarding Non-SMCCC "hvc #3" call, I have a limitation here owing to the ABI
>> that is defined by the Hyper-V hypervisor. Fixing this requires a
>> hypervisor-side change to support SMCCC-style dispatch for VTL return. Until
>> then, hvc #3 is the only working interface. Moreover there would be backward
>> compatibility issues with this new ABI interface, if at all it is added.
> 
> To be clear, that's Microsoft's problem, not the Linux kernel
> community's problem. My NAK still stands.
> 
> Multiple years ago now, we made it clear that we would not accept a
> non-SMCCC calling convention. Ignoring the substance of that feedback,
> and inventing a new calling convention after that point is a
> self-inflicted problem.
> 
> [...]
> 
>> Link to TLFS: https://learn.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/vsm#on-arm64-platforms-3
> 
> For shared state, aside fomr GPRs and FPSIMD/SVE/SME state, that says:
> 
> | * System Information Registers (read-only or non-security-critical):
> |   * System identification and feature registers
> |   * Cache and TLB type information
> 
> It's *implied* that some of those registers might be writable, but as
> the specific set of registers is not described I cannot tell. Are there
> any writable system registers which are shared?
> 
> I don't see how we can know which registers we might need to
> save/restore without that being explicitly documented.
> 
> I also see:
> 
> | Note: SPE (Statistical Profiling Extension) state is shared across VTLs,
> | except for PMBSR_EL1 which is VTL-private.
> 
> If "SPE state" includes PMBPTR or PMBLIMITR (which is the obvious
> reading), this would be a security problem, as a lower-privileged VTL
> could clobber those and cause SPE to write to arbitrary memory
> immediately upon return to the higher-privileged VTL. Having PMBSR be
> private on its own isn't sufficient to prevent that (e.g. since the
> higher-privileged VTL could have its own active SPE profiling session).
> 
> I'm not keen on requiring hyper-v specific hooks in the SPE driver to
> achieve that, and I'm also not keen on having hyper-v support code poke
> SPE registers behind the SPE driver's back.
> 
> This does not give me confidence that any future PE state (e.g. things
> like TRBE) will be managed in a safe way either.
> 
> Mark.


Thanks for sharing this, I'll discuss it internally and come up with a plan.

Regards,
Naman

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 07/15] arm64: hyperv: Add support for mshv_vtl_return_call
       [not found]     ` <f4059f5d-a82b-40c2-942e-3e24cefab94f@linux.microsoft.com>
  2026-05-06  7:52       ` Mark Rutland
@ 2026-05-08  9:25       ` Marc Zyngier
  2026-05-08  9:56         ` Naman Jain
  1 sibling, 1 reply; 10+ messages in thread
From: Marc Zyngier @ 2026-05-08  9:25 UTC (permalink / raw)
  To: Naman Jain
  Cc: Mark Rutland, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
	Dexuan Cui, Long Li, Catalin Marinas, Will Deacon,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H . Peter Anvin, Arnd Bergmann, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Alexandre Ghiti, Michael Kelley, Timothy Hayes,
	Lorenzo Pieralisi, Sascha Bischoff, mrigendrachaubey,
	linux-hyperv, linux-arm-kernel, linux-kernel, linux-arch,
	linux-riscv, vdso, ssengar

On Wed, 29 Apr 2026 10:56:11 +0100,
Naman Jain <namjain@linux.microsoft.com> wrote:
> 

[...]

> Merging threads for addressing comments from Mark Rutland and Marc
> Zyngier on this patch.
> 
> Thanks for reviewing the changes. Please allow me to briefly explain
> the use case here and then address your comments.
> 
> Hyper-V's Virtual Trust Levels (VTLs) provide hardware-enforced
> isolation within a single VM, analogous to ARM TrustZone. The kernel
> runs in VTL2 (higher privilege) as a "paravisor", a security monitor
> that handles intercepts for the primary OS in VTL0 (lower
> privilege). The VTL switch (mshv_vtl_return_call) is functionally
> equivalent to KVM's guest enter/exit. It saves VTL2 state, loads
> VTL0's GPRs other registers from a shared context structure, issues
> hvc #3 to let VTL0 run, and on return saves VTL0's updated state back.

No, this is fundamentally different. KVM is purely architectural,
doesn't try to "sanitise" anything, and context switches *all* of the
guest state. No ifs, no buts, no "reserved registers".

[...]

> Regarding Non-SMCCC "hvc #3" call, I have a limitation here owing to
> the ABI that is defined by the Hyper-V hypervisor. Fixing this
> requires a hypervisor-side change to support SMCCC-style dispatch for
> VTL return. Until then, hvc #3 is the only working interface. Moreover
> there would be backward compatibility issues with this new ABI
> interface, if at all it is added.

Left hand, please talk to right hand. This is not the first time we
push back on this, and we already had this annoying discussion back
when arm64 as a Hyper-V guest was first proposed (6, 7 years ago?).

What has changed since? Absolutely nothing.

If the Hyper-V folks decide to ignore the standard and go their own
way, that's fine. They only get to keep the pieces.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 07/15] arm64: hyperv: Add support for mshv_vtl_return_call
  2026-05-08  9:25       ` Marc Zyngier
@ 2026-05-08  9:56         ` Naman Jain
  0 siblings, 0 replies; 10+ messages in thread
From: Naman Jain @ 2026-05-08  9:56 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Mark Rutland, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
	Dexuan Cui, Long Li, Catalin Marinas, Will Deacon,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H . Peter Anvin, Arnd Bergmann, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Alexandre Ghiti, Michael Kelley, Timothy Hayes,
	Lorenzo Pieralisi, Sascha Bischoff, mrigendrachaubey,
	linux-hyperv, linux-arm-kernel, linux-kernel, linux-arch,
	linux-riscv, vdso, ssengar



On 5/8/2026 2:55 PM, Marc Zyngier wrote:
> On Wed, 29 Apr 2026 10:56:11 +0100,
> Naman Jain <namjain@linux.microsoft.com> wrote:
>>
> 
> [...]
> 
>> Merging threads for addressing comments from Mark Rutland and Marc
>> Zyngier on this patch.
>>
>> Thanks for reviewing the changes. Please allow me to briefly explain
>> the use case here and then address your comments.
>>
>> Hyper-V's Virtual Trust Levels (VTLs) provide hardware-enforced
>> isolation within a single VM, analogous to ARM TrustZone. The kernel
>> runs in VTL2 (higher privilege) as a "paravisor", a security monitor
>> that handles intercepts for the primary OS in VTL0 (lower
>> privilege). The VTL switch (mshv_vtl_return_call) is functionally
>> equivalent to KVM's guest enter/exit. It saves VTL2 state, loads
>> VTL0's GPRs other registers from a shared context structure, issues
>> hvc #3 to let VTL0 run, and on return saves VTL0's updated state back.
> 
> No, this is fundamentally different. KVM is purely architectural,
> doesn't try to "sanitise" anything, and context switches *all* of the
> guest state. No ifs, no buts, no "reserved registers".
> 
> [...]

Acked.

> 
>> Regarding Non-SMCCC "hvc #3" call, I have a limitation here owing to
>> the ABI that is defined by the Hyper-V hypervisor. Fixing this
>> requires a hypervisor-side change to support SMCCC-style dispatch for
>> VTL return. Until then, hvc #3 is the only working interface. Moreover
>> there would be backward compatibility issues with this new ABI
>> interface, if at all it is added.
> 
> Left hand, please talk to right hand. This is not the first time we
> push back on this, and we already had this annoying discussion back
> when arm64 as a Hyper-V guest was first proposed (6, 7 years ago?).
> 
> What has changed since? Absolutely nothing.
> 
> If the Hyper-V folks decide to ignore the standard and go their own
> way, that's fine. They only get to keep the pieces.
> 
> Thanks,
> 
> 	M.
> 

Thanks for the feedback. I understand your and Mark’s concerns with this 
approach now, and I’ve initiated internal discussions with the team to 
explore potential solutions.

Regards,
Naman

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-05-08  9:56 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260423124206.2410879-1-namjain@linux.microsoft.com>
     [not found] ` <20260423124206.2410879-10-namjain@linux.microsoft.com>
     [not found]   ` <SN6PR02MB4157467FDBC0203C67A67042D4362@SN6PR02MB4157.namprd02.prod.outlook.com>
     [not found]     ` <567cf73b-6a48-4fc3-b312-9be6d71e2f22@linux.microsoft.com>
2026-05-04 16:06       ` [PATCH v2 09/15] Drivers: hv: mshv_vtl: Move hv_vtl_configure_reg_page() to x86 Michael Kelley
2026-05-06  5:50         ` Naman Jain
2026-05-06 14:36           ` Michael Kelley
2026-05-07  3:43             ` Naman Jain
     [not found] ` <20260423124206.2410879-8-namjain@linux.microsoft.com>
     [not found]   ` <SN6PR02MB4157C147A1B915F9B45D3B74D4362@SN6PR02MB4157.namprd02.prod.outlook.com>
     [not found]     ` <516a4e65-3a4d-4e09-b445-28cb740b5653@linux.microsoft.com>
2026-05-04 16:06       ` [PATCH v2 07/15] arm64: hyperv: Add support for mshv_vtl_return_call Michael Kelley
2026-05-06  5:11         ` Naman Jain
     [not found]   ` <aeolHwXHFH4AnX_n@J2N7QTR9R3.cambridge.arm.com>
     [not found]     ` <f4059f5d-a82b-40c2-942e-3e24cefab94f@linux.microsoft.com>
2026-05-06  7:52       ` Mark Rutland
2026-05-08  4:26         ` Naman Jain
2026-05-08  9:25       ` Marc Zyngier
2026-05-08  9:56         ` Naman Jain

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox