Taking struct cpu_user_regs as a full object is bogus, and while what was
probably meant was to take a struct cpu_user_regs pointer, that's still wrong.

This isn't a function; its an address stored in the VMCS that the CPU resumes
from on VMExit, meaning that it doesn't conform to a normal C API/ABI.

Annotate it with `nocall`, and move the declaration into vmcs.c next to its
sole user.

Fixes: 9c3118a82523 ("bitkeeper revision 1.1159.1.483 (41c0c417XYObowWqbfqU0cdLx30C9w)")
                 `-> "Initial Intel VMX support"
Reported-by: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Xenia Ragiadakou <burzalodowa@gmail.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
---
 xen/arch/x86/hvm/vmx/vmcs.c            | 2 ++
 xen/arch/x86/include/asm/hvm/vmx/vmx.h | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index e1c268789e7e..ed71ecfb6284 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1074,6 +1074,8 @@ static void pi_desc_init(struct vcpu *v)
     v->arch.hvm.vmx.pi_desc.ndst = APIC_INVALID_DEST;
 }
 
+void nocall vmx_asm_vmexit_handler(void);
+
 static int construct_vmcs(struct vcpu *v)
 {
     struct domain *d = v->domain;
diff --git a/xen/arch/x86/include/asm/hvm/vmx/vmx.h b/xen/arch/x86/include/asm/hvm/vmx/vmx.h
index 97d6b810ec55..f6308ed65601 100644
--- a/xen/arch/x86/include/asm/hvm/vmx/vmx.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmx.h
@@ -82,7 +82,6 @@ typedef enum {
 
 #define PI_xAPIC_NDST_MASK      0xFF00
 
-void vmx_asm_vmexit_handler(struct cpu_user_regs);
 void vmx_intr_assist(void);
 void noreturn cf_check vmx_do_resume(void);
 void cf_check vmx_vlapic_msr_changed(struct vcpu *v);
-- 
2.30.2

On 23.02.2023 13:07, Andrew Cooper wrote:
> Taking struct cpu_user_regs as a full object is bogus, and while what was
> probably meant was to take a struct cpu_user_regs pointer, that's still wrong.
> 
> This isn't a function; its an address stored in the VMCS that the CPU resumes
> from on VMExit, meaning that it doesn't conform to a normal C API/ABI.
> 
> Annotate it with `nocall`, and move the declaration into vmcs.c next to its
> sole user.
> 
> Fixes: 9c3118a82523 ("bitkeeper revision 1.1159.1.483 (41c0c417XYObowWqbfqU0cdLx30C9w)")
>                  `-> "Initial Intel VMX support"
> Reported-by: Jan Beulich <JBeulich@suse.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Since I had to look up what exactly "nocall" expands to, I'm wondering
whether it really is still the case that Clang doesn't support the error
attribute ...

Jan

On 23.02.2023 13:28, Jan Beulich wrote:
> On 23.02.2023 13:07, Andrew Cooper wrote:
>> Taking struct cpu_user_regs as a full object is bogus, and while what was
>> probably meant was to take a struct cpu_user_regs pointer, that's still wrong.
>>
>> This isn't a function; its an address stored in the VMCS that the CPU resumes
>> from on VMExit, meaning that it doesn't conform to a normal C API/ABI.
>>
>> Annotate it with `nocall`, and move the declaration into vmcs.c next to its
>> sole user.
>>
>> Fixes: 9c3118a82523 ("bitkeeper revision 1.1159.1.483 (41c0c417XYObowWqbfqU0cdLx30C9w)")
>>                  `-> "Initial Intel VMX support"
>> Reported-by: Jan Beulich <JBeulich@suse.com>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> Since I had to look up what exactly "nocall" expands to, I'm wondering
> whether it really is still the case that Clang doesn't support the error
> attribute ...

Argh, should have looked at your next patch ...

Jan