From: Grygorii Strashko <grygorii_strashko@epam.com>
Factor out COMPAT HVM code under ifdefs in preparation for making HVM
COMPAT code optional.
Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
---
xen/arch/x86/hvm/hvm.c | 13 ++++++++++++-
xen/arch/x86/hvm/hypercall.c | 37 +++++++++++++++++++++++++++---------
2 files changed, 40 insertions(+), 10 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 2e47a71714fd..56c0059401d0 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -69,7 +69,9 @@
#include <public/version.h>
#include <public/vm_event.h>
+#ifdef CONFIG_COMPAT
#include <compat/hvm/hvm_op.h>
+#endif
bool __read_mostly hvm_enabled;
@@ -1255,6 +1257,7 @@ static int cf_check hvm_save_cpu_xsave_states(
return 0;
}
+#ifdef CONFIG_COMPAT
/*
* Structure layout conformity checks, documenting correctness of the cast in
* the invocation of validate_xstate() below.
@@ -1267,6 +1270,7 @@ CHECK_FIELD_(struct, xsave_hdr, xcomp_bv);
CHECK_FIELD_(struct, xsave_hdr, reserved);
#undef compat_xsave_hdr
#undef xen_xsave_hdr
+#endif /* CONFIG_COMPAT */
static int cf_check hvm_load_cpu_xsave_states(
struct domain *d, hvm_domain_context_t *h)
@@ -3991,7 +3995,7 @@ static void hvm_latch_shinfo_size(struct domain *d)
*/
if ( current->domain == d )
{
- d->arch.has_32bit_shinfo =
+ d->arch.has_32bit_shinfo = IS_ENABLED(CONFIG_COMPAT) &&
hvm_guest_x86_mode(current) != X86_MODE_64BIT;
/*
@@ -4965,6 +4969,7 @@ static int do_altp2m_op(
#endif /* CONFIG_ALTP2M */
}
+#ifdef CONFIG_COMPAT
DEFINE_XEN_GUEST_HANDLE(compat_hvm_altp2m_op_t);
/*
@@ -5064,6 +5069,12 @@ static int compat_altp2m_op(
return rc;
}
+#else
+static int compat_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
+{
+ return -EOPNOTSUPP;
+}
+#endif /* CONFIG_COMPAT */
static int hvmop_get_mem_type(
XEN_GUEST_HANDLE_PARAM(xen_hvm_get_mem_type_t) arg)
diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
index 549e25445e67..f8b2c90b7c41 100644
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -29,10 +29,12 @@ long hvm_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
return -ENOSYS;
}
- if ( !vcpu_is_hcall_compat(current) )
- rc = do_memory_op(cmd, arg);
- else
+#ifdef CONFIG_COMPAT
+ if ( vcpu_is_hcall_compat(current) )
rc = compat_memory_op(cmd, arg);
+ else
+#endif
+ rc = do_memory_op(cmd, arg);
return rc;
}
@@ -57,10 +59,12 @@ long hvm_grant_table_op(
return -ENOSYS;
}
- if ( !vcpu_is_hcall_compat(current) )
- return do_grant_table_op(cmd, uop, count);
- else
+#ifdef CONFIG_COMPAT
+ if ( vcpu_is_hcall_compat(current) )
return compat_grant_table_op(cmd, uop, count);
+ else
+#endif
+ return do_grant_table_op(cmd, uop, count);
}
#endif
@@ -96,10 +100,12 @@ long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
return -ENOSYS;
}
- if ( !vcpu_is_hcall_compat(curr) )
- return do_physdev_op(cmd, arg);
- else
+#ifdef CONFIG_COMPAT
+ if ( vcpu_is_hcall_compat(curr) )
return compat_physdev_op(cmd, arg);
+ else
+#endif
+ return do_physdev_op(cmd, arg);
}
int hvm_hypercall(struct cpu_user_regs *regs)
@@ -171,6 +177,7 @@ int hvm_hypercall(struct cpu_user_regs *regs)
HVM_DBG_LOG(DBG_LEVEL_HCALL, "hcall%lu(%x, %x, %x, %x, %x)", eax,
regs->ebx, regs->ecx, regs->edx, regs->esi, regs->edi);
+#ifdef CONFIG_COMPAT
curr->hcall_compat = true;
call_handlers_hvm32(eax, regs->eax, regs->ebx, regs->ecx, regs->edx,
regs->esi, regs->edi);
@@ -178,6 +185,9 @@ int hvm_hypercall(struct cpu_user_regs *regs)
if ( !curr->hcall_preempted && regs->eax != -ENOSYS )
clobber_regs(regs, eax, hvm, 32);
+#else
+ regs->eax = -EOPNOTSUPP;
+#endif
}
hvmemul_cache_restore(curr, token);
@@ -208,10 +218,19 @@ enum mc_disposition hvm_do_multicall_call(struct mc_state *state)
}
else
{
+#ifdef CONFIG_COMPAT
struct compat_multicall_entry *call = &state->compat_call;
call_handlers_hvm32(call->op, call->result, call->args[0], call->args[1],
call->args[2], call->args[3], call->args[4]);
+#else
+ /*
+ * code should never reach here in case !CONFIG_COMPAT as any
+ * 32-bit hypercall should bail out earlier from hvm_hypercall()
+ * with -EOPNOTSUPP
+ */
+ unreachable();
+#endif
}
return !hvm_get_cpl(curr) ? mc_continue : mc_preempt;
--
2.34.1On 11.11.2025 18:54, Grygorii Strashko wrote:
> @@ -3991,7 +3995,7 @@ static void hvm_latch_shinfo_size(struct domain *d)
> */
> if ( current->domain == d )
> {
> - d->arch.has_32bit_shinfo =
> + d->arch.has_32bit_shinfo = IS_ENABLED(CONFIG_COMPAT) &&
> hvm_guest_x86_mode(current) != X86_MODE_64BIT;
I think this could need commenting on if we really want to put it in this shape.
But why would we retain the has_32bit_shinfo field in the first place when
COMPAT=n?
> @@ -4965,6 +4969,7 @@ static int do_altp2m_op(
> #endif /* CONFIG_ALTP2M */
> }
>
> +#ifdef CONFIG_COMPAT
> DEFINE_XEN_GUEST_HANDLE(compat_hvm_altp2m_op_t);
>
> /*
> @@ -5064,6 +5069,12 @@ static int compat_altp2m_op(
>
> return rc;
> }
> +#else
> +static int compat_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
> +{
> + return -EOPNOTSUPP;
> +}
> +#endif /* CONFIG_COMPAT */
I'm not in favor of repeating the function "header". Imo such #ifdef-s better
go inside respective functions' bodies.
> --- a/xen/arch/x86/hvm/hypercall.c
> +++ b/xen/arch/x86/hvm/hypercall.c
> @@ -29,10 +29,12 @@ long hvm_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> return -ENOSYS;
> }
>
> - if ( !vcpu_is_hcall_compat(current) )
> - rc = do_memory_op(cmd, arg);
> - else
> +#ifdef CONFIG_COMPAT
> + if ( vcpu_is_hcall_compat(current) )
> rc = compat_memory_op(cmd, arg);
> + else
> +#endif
> + rc = do_memory_op(cmd, arg);
Why would this be needed when vcpu_is_hcall_compat() already honors CONFIG_COMPAT?
(Same question then applies elsewhere, of course.)
> @@ -171,6 +177,7 @@ int hvm_hypercall(struct cpu_user_regs *regs)
> HVM_DBG_LOG(DBG_LEVEL_HCALL, "hcall%lu(%x, %x, %x, %x, %x)", eax,
> regs->ebx, regs->ecx, regs->edx, regs->esi, regs->edi);
>
> +#ifdef CONFIG_COMPAT
> curr->hcall_compat = true;
> call_handlers_hvm32(eax, regs->eax, regs->ebx, regs->ecx, regs->edx,
> regs->esi, regs->edi);
> @@ -178,6 +185,9 @@ int hvm_hypercall(struct cpu_user_regs *regs)
>
> if ( !curr->hcall_preempted && regs->eax != -ENOSYS )
> clobber_regs(regs, eax, hvm, 32);
> +#else
> + regs->eax = -EOPNOTSUPP;
> +#endif
I'm generally against most attempts to use ENOSYS, but here it should be used:
The top-level hypercalls are (effectively) unimplemented in this mode.
> @@ -208,10 +218,19 @@ enum mc_disposition hvm_do_multicall_call(struct mc_state *state)
> }
> else
> {
> +#ifdef CONFIG_COMPAT
> struct compat_multicall_entry *call = &state->compat_call;
>
> call_handlers_hvm32(call->op, call->result, call->args[0], call->args[1],
> call->args[2], call->args[3], call->args[4]);
> +#else
> + /*
> + * code should never reach here in case !CONFIG_COMPAT as any
> + * 32-bit hypercall should bail out earlier from hvm_hypercall()
> + * with -EOPNOTSUPP
> + */
> + unreachable();
I.e. you rather mean ASSERT_UNREACHABLE()?
Jan
On 13.11.25 14:30, Jan Beulich wrote:
> On 11.11.2025 18:54, Grygorii Strashko wrote:
>> @@ -3991,7 +3995,7 @@ static void hvm_latch_shinfo_size(struct domain *d)
>> */
>> if ( current->domain == d )
>> {
>> - d->arch.has_32bit_shinfo =
>> + d->arch.has_32bit_shinfo = IS_ENABLED(CONFIG_COMPAT) &&
>> hvm_guest_x86_mode(current) != X86_MODE_64BIT;
>
> I think this could need commenting on if we really want to put it in this shape.
ok.
> But why would we retain the has_32bit_shinfo field in the first place when
> COMPAT=n?
Here I'm not sure. May be other can comment.
>
>> @@ -4965,6 +4969,7 @@ static int do_altp2m_op(
>> #endif /* CONFIG_ALTP2M */
>> }
>>
>> +#ifdef CONFIG_COMPAT
>> DEFINE_XEN_GUEST_HANDLE(compat_hvm_altp2m_op_t);
>>
>> /*
>> @@ -5064,6 +5069,12 @@ static int compat_altp2m_op(
>>
>> return rc;
>> }
>> +#else
>> +static int compat_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
>> +{
>> + return -EOPNOTSUPP;
>> +}
>> +#endif /* CONFIG_COMPAT */
>
> I'm not in favor of repeating the function "header". Imo such #ifdef-s better
> go inside respective functions' bodies.
ok.
>
>> --- a/xen/arch/x86/hvm/hypercall.c
>> +++ b/xen/arch/x86/hvm/hypercall.c
>> @@ -29,10 +29,12 @@ long hvm_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>> return -ENOSYS;
>> }
>>
>> - if ( !vcpu_is_hcall_compat(current) )
>> - rc = do_memory_op(cmd, arg);
>> - else
>> +#ifdef CONFIG_COMPAT
>> + if ( vcpu_is_hcall_compat(current) )
>> rc = compat_memory_op(cmd, arg);
>> + else
>> +#endif
>> + rc = do_memory_op(cmd, arg);
>
> Why would this be needed when vcpu_is_hcall_compat() already honors CONFIG_COMPAT?
> (Same question then applies elsewhere, of course.)
This I do not like by myself, but I was not able to find other options :(
hypercall-defs.h is autogenerated and it's the only one place where functions
declarations like do_x_op() are appeared or disappeared.
So build is failing without ifdefs as compiler can't find compat_memory_op()
declaration.
>
>> @@ -171,6 +177,7 @@ int hvm_hypercall(struct cpu_user_regs *regs)
>> HVM_DBG_LOG(DBG_LEVEL_HCALL, "hcall%lu(%x, %x, %x, %x, %x)", eax,
>> regs->ebx, regs->ecx, regs->edx, regs->esi, regs->edi);
>>
>> +#ifdef CONFIG_COMPAT
>> curr->hcall_compat = true;
>> call_handlers_hvm32(eax, regs->eax, regs->ebx, regs->ecx, regs->edx,
>> regs->esi, regs->edi);
>> @@ -178,6 +185,9 @@ int hvm_hypercall(struct cpu_user_regs *regs)
>>
>> if ( !curr->hcall_preempted && regs->eax != -ENOSYS )
>> clobber_regs(regs, eax, hvm, 32);
>> +#else
>> + regs->eax = -EOPNOTSUPP;
>> +#endif
>
> I'm generally against most attempts to use ENOSYS, but here it should be used:
> The top-level hypercalls are (effectively) unimplemented in this mode.
ok.
>
>> @@ -208,10 +218,19 @@ enum mc_disposition hvm_do_multicall_call(struct mc_state *state)
>> }
>> else
>> {
>> +#ifdef CONFIG_COMPAT
>> struct compat_multicall_entry *call = &state->compat_call;
>>
>> call_handlers_hvm32(call->op, call->result, call->args[0], call->args[1],
>> call->args[2], call->args[3], call->args[4]);
>> +#else
>> + /*
>> + * code should never reach here in case !CONFIG_COMPAT as any
>> + * 32-bit hypercall should bail out earlier from hvm_hypercall()
>> + * with -EOPNOTSUPP
>> + */
>> + unreachable();
>
> I.e. you rather mean ASSERT_UNREACHABLE()?
ok.
--
Best regards,
-grygorii
On 13.11.2025 14:18, Grygorii Strashko wrote: > On 13.11.25 14:30, Jan Beulich wrote: >> On 11.11.2025 18:54, Grygorii Strashko wrote: >>> --- a/xen/arch/x86/hvm/hypercall.c >>> +++ b/xen/arch/x86/hvm/hypercall.c >>> @@ -29,10 +29,12 @@ long hvm_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) >>> return -ENOSYS; >>> } >>> >>> - if ( !vcpu_is_hcall_compat(current) ) >>> - rc = do_memory_op(cmd, arg); >>> - else >>> +#ifdef CONFIG_COMPAT >>> + if ( vcpu_is_hcall_compat(current) ) >>> rc = compat_memory_op(cmd, arg); >>> + else >>> +#endif >>> + rc = do_memory_op(cmd, arg); >> >> Why would this be needed when vcpu_is_hcall_compat() already honors CONFIG_COMPAT? >> (Same question then applies elsewhere, of course.) > > This I do not like by myself, but I was not able to find other options :( > > hypercall-defs.h is autogenerated and it's the only one place where functions > declarations like do_x_op() are appeared or disappeared. > So build is failing without ifdefs as compiler can't find compat_memory_op() > declaration. Oh, I see; I hadn't thought of that aspect. I wonder if we wouldn't better take care of that in the machinery there. Cc-ing Jürgen, who did introduce this originally. Maybe he has concrete arguments against us doing so. Jan
On 13.11.25 14:23, Jan Beulich wrote: > On 13.11.2025 14:18, Grygorii Strashko wrote: >> On 13.11.25 14:30, Jan Beulich wrote: >>> On 11.11.2025 18:54, Grygorii Strashko wrote: >>>> --- a/xen/arch/x86/hvm/hypercall.c >>>> +++ b/xen/arch/x86/hvm/hypercall.c >>>> @@ -29,10 +29,12 @@ long hvm_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) >>>> return -ENOSYS; >>>> } >>>> >>>> - if ( !vcpu_is_hcall_compat(current) ) >>>> - rc = do_memory_op(cmd, arg); >>>> - else >>>> +#ifdef CONFIG_COMPAT >>>> + if ( vcpu_is_hcall_compat(current) ) >>>> rc = compat_memory_op(cmd, arg); >>>> + else >>>> +#endif >>>> + rc = do_memory_op(cmd, arg); >>> >>> Why would this be needed when vcpu_is_hcall_compat() already honors CONFIG_COMPAT? >>> (Same question then applies elsewhere, of course.) >> >> This I do not like by myself, but I was not able to find other options :( >> >> hypercall-defs.h is autogenerated and it's the only one place where functions >> declarations like do_x_op() are appeared or disappeared. >> So build is failing without ifdefs as compiler can't find compat_memory_op() >> declaration. > > Oh, I see; I hadn't thought of that aspect. I wonder if we wouldn't better take > care of that in the machinery there. Cc-ing Jürgen, who did introduce this > originally. Maybe he has concrete arguments against us doing so. No arguments against it. You probably will need a new Prefix defined (e.g. compat_always) and set it via #define PREFIX_compat_always compat unconditionally. Then it should be possible to have Prefix: compat_always memory_op(...) outside of #ifdefs and drop the memory_op() in the #ifdef CONFIG_COMPAT section. This should result in the compat_memory_op() prototype to be always available. Having no related function should be no problem due to DCO in case CONFIG_COMPAT isn't defined. Juergen
On 13.11.25 15:39, Jürgen Groß wrote: > On 13.11.25 14:23, Jan Beulich wrote: >> On 13.11.2025 14:18, Grygorii Strashko wrote: >>> On 13.11.25 14:30, Jan Beulich wrote: >>>> On 11.11.2025 18:54, Grygorii Strashko wrote: >>>>> --- a/xen/arch/x86/hvm/hypercall.c >>>>> +++ b/xen/arch/x86/hvm/hypercall.c >>>>> @@ -29,10 +29,12 @@ long hvm_memory_op(unsigned long cmd, >>>>> XEN_GUEST_HANDLE_PARAM(void) arg) >>>>> return -ENOSYS; >>>>> } >>>>> - if ( !vcpu_is_hcall_compat(current) ) >>>>> - rc = do_memory_op(cmd, arg); >>>>> - else >>>>> +#ifdef CONFIG_COMPAT >>>>> + if ( vcpu_is_hcall_compat(current) ) >>>>> rc = compat_memory_op(cmd, arg); >>>>> + else >>>>> +#endif >>>>> + rc = do_memory_op(cmd, arg); >>>> >>>> Why would this be needed when vcpu_is_hcall_compat() already honors >>>> CONFIG_COMPAT? >>>> (Same question then applies elsewhere, of course.) >>> >>> This I do not like by myself, but I was not able to find other options :( >>> >>> hypercall-defs.h is autogenerated and it's the only one place where functions >>> declarations like do_x_op() are appeared or disappeared. >>> So build is failing without ifdefs as compiler can't find compat_memory_op() >>> declaration. >> >> Oh, I see; I hadn't thought of that aspect. I wonder if we wouldn't better take >> care of that in the machinery there. Cc-ing Jürgen, who did introduce this >> originally. Maybe he has concrete arguments against us doing so. > > No arguments against it. > > You probably will need a new Prefix defined (e.g. compat_always) and set it via > > #define PREFIX_compat_always compat > > unconditionally. Then it should be possible to have > > Prefix: compat_always > memory_op(...) > > outside of #ifdefs and drop the memory_op() in the #ifdef CONFIG_COMPAT section. Oh, this might be wrong, as this will break the PV32 memory_op() hypercall. You need to keep the current memory_op() in the #ifdef CONFIG_COMPAT section and move the compat_always stuff into an #else part of the CONFIG_COMPAT. > > This should result in the compat_memory_op() prototype to be always available. > Having no related function should be no problem due to DCO in case CONFIG_COMPAT > isn't defined. Juergen
Hi Jürgen, On 13.11.25 16:46, Juergen Gross wrote: > On 13.11.25 15:39, Jürgen Groß wrote: >> On 13.11.25 14:23, Jan Beulich wrote: >>> On 13.11.2025 14:18, Grygorii Strashko wrote: >>>> On 13.11.25 14:30, Jan Beulich wrote: >>>>> On 11.11.2025 18:54, Grygorii Strashko wrote: >>>>>> --- a/xen/arch/x86/hvm/hypercall.c >>>>>> +++ b/xen/arch/x86/hvm/hypercall.c >>>>>> @@ -29,10 +29,12 @@ long hvm_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) >>>>>> return -ENOSYS; >>>>>> } >>>>>> - if ( !vcpu_is_hcall_compat(current) ) >>>>>> - rc = do_memory_op(cmd, arg); >>>>>> - else >>>>>> +#ifdef CONFIG_COMPAT >>>>>> + if ( vcpu_is_hcall_compat(current) ) >>>>>> rc = compat_memory_op(cmd, arg); >>>>>> + else >>>>>> +#endif >>>>>> + rc = do_memory_op(cmd, arg); >>>>> >>>>> Why would this be needed when vcpu_is_hcall_compat() already honors CONFIG_COMPAT? >>>>> (Same question then applies elsewhere, of course.) >>>> >>>> This I do not like by myself, but I was not able to find other options :( >>>> >>>> hypercall-defs.h is autogenerated and it's the only one place where functions >>>> declarations like do_x_op() are appeared or disappeared. >>>> So build is failing without ifdefs as compiler can't find compat_memory_op() >>>> declaration. >>> >>> Oh, I see; I hadn't thought of that aspect. I wonder if we wouldn't better take >>> care of that in the machinery there. Cc-ing Jürgen, who did introduce this >>> originally. Maybe he has concrete arguments against us doing so. >> >> No arguments against it. >> >> You probably will need a new Prefix defined (e.g. compat_always) and set it via >> >> #define PREFIX_compat_always compat >> >> unconditionally. Then it should be possible to have >> >> Prefix: compat_always >> memory_op(...) >> >> outside of #ifdefs and drop the memory_op() in the #ifdef CONFIG_COMPAT section. > > Oh, this might be wrong, as this will break the PV32 memory_op() hypercall. > > You need to keep the current memory_op() in the #ifdef CONFIG_COMPAT section > and move the compat_always stuff into an #else part of the CONFIG_COMPAT. > >> >> This should result in the compat_memory_op() prototype to be always available. >> Having no related function should be no problem due to DCO in case CONFIG_COMPAT >> isn't defined. Smth like this, right? diff --git a/xen/include/hypercall-defs.c b/xen/include/hypercall-defs.c index 338d7afe3048..e85943320bd2 100644 --- a/xen/include/hypercall-defs.c +++ b/xen/include/hypercall-defs.c @@ -80,6 +80,8 @@ rettype: compat int #define PREFIX_compat #endif +#define PREFIX_compat_always compat + #ifdef CONFIG_ARM #define PREFIX_dep dep #define PREFIX_do_arm do_arm @@ -156,6 +158,9 @@ platform_op(compat_platform_op_t *u_xenpf_op) #ifdef CONFIG_KEXEC kexec_op(unsigned int op, void *uarg) #endif +#else +prefix: PREFIX_compat_always +memory_op(unsigned int cmd, void *arg) #endif /* CONFIG_COMPAT */ -- Best regards, -grygorii
On 13.11.25 16:32, Grygorii Strashko wrote: > Hi Jürgen, > > On 13.11.25 16:46, Juergen Gross wrote: >> On 13.11.25 15:39, Jürgen Groß wrote: >>> On 13.11.25 14:23, Jan Beulich wrote: >>>> On 13.11.2025 14:18, Grygorii Strashko wrote: >>>>> On 13.11.25 14:30, Jan Beulich wrote: >>>>>> On 11.11.2025 18:54, Grygorii Strashko wrote: >>>>>>> --- a/xen/arch/x86/hvm/hypercall.c >>>>>>> +++ b/xen/arch/x86/hvm/hypercall.c >>>>>>> @@ -29,10 +29,12 @@ long hvm_memory_op(unsigned long cmd, >>>>>>> XEN_GUEST_HANDLE_PARAM(void) arg) >>>>>>> return -ENOSYS; >>>>>>> } >>>>>>> - if ( !vcpu_is_hcall_compat(current) ) >>>>>>> - rc = do_memory_op(cmd, arg); >>>>>>> - else >>>>>>> +#ifdef CONFIG_COMPAT >>>>>>> + if ( vcpu_is_hcall_compat(current) ) >>>>>>> rc = compat_memory_op(cmd, arg); >>>>>>> + else >>>>>>> +#endif >>>>>>> + rc = do_memory_op(cmd, arg); >>>>>> >>>>>> Why would this be needed when vcpu_is_hcall_compat() already honors >>>>>> CONFIG_COMPAT? >>>>>> (Same question then applies elsewhere, of course.) >>>>> >>>>> This I do not like by myself, but I was not able to find other options :( >>>>> >>>>> hypercall-defs.h is autogenerated and it's the only one place where functions >>>>> declarations like do_x_op() are appeared or disappeared. >>>>> So build is failing without ifdefs as compiler can't find compat_memory_op() >>>>> declaration. >>>> >>>> Oh, I see; I hadn't thought of that aspect. I wonder if we wouldn't better take >>>> care of that in the machinery there. Cc-ing Jürgen, who did introduce this >>>> originally. Maybe he has concrete arguments against us doing so. >>> >>> No arguments against it. >>> >>> You probably will need a new Prefix defined (e.g. compat_always) and set it via >>> >>> #define PREFIX_compat_always compat >>> >>> unconditionally. Then it should be possible to have >>> >>> Prefix: compat_always >>> memory_op(...) >>> >>> outside of #ifdefs and drop the memory_op() in the #ifdef CONFIG_COMPAT section. >> >> Oh, this might be wrong, as this will break the PV32 memory_op() hypercall. >> >> You need to keep the current memory_op() in the #ifdef CONFIG_COMPAT section >> and move the compat_always stuff into an #else part of the CONFIG_COMPAT. >> >>> >>> This should result in the compat_memory_op() prototype to be always available. >>> Having no related function should be no problem due to DCO in case CONFIG_COMPAT >>> isn't defined. > > Smth like this, right? > > diff --git a/xen/include/hypercall-defs.c b/xen/include/hypercall-defs.c > index 338d7afe3048..e85943320bd2 100644 > --- a/xen/include/hypercall-defs.c > +++ b/xen/include/hypercall-defs.c > @@ -80,6 +80,8 @@ rettype: compat int > #define PREFIX_compat > #endif > > +#define PREFIX_compat_always compat > + > #ifdef CONFIG_ARM > #define PREFIX_dep dep > #define PREFIX_do_arm do_arm > @@ -156,6 +158,9 @@ platform_op(compat_platform_op_t *u_xenpf_op) > #ifdef CONFIG_KEXEC > kexec_op(unsigned int op, void *uarg) > #endif > +#else > +prefix: PREFIX_compat_always This should be: +prefix: compat_always > +memory_op(unsigned int cmd, void *arg) > #endif /* CONFIG_COMPAT */ Juergen
© 2016 - 2025 Red Hat, Inc.