On Fri, Jan 08, 2021 at 04:24:01PM +0100, Jan Beulich wrote: > On 08.01.2021 16:11, Roger Pau Monné wrote: > > On Fri, Jan 08, 2021 at 04:01:52PM +0100, Jan Beulich wrote: > >> On 08.01.2021 15:41, Roger Pau Monne wrote: > >>> --- a/xen/arch/x86/hypercall.c > >>> +++ b/xen/arch/x86/hypercall.c > >>> @@ -47,7 +47,7 @@ const hypercall_args_t > >>> hypercall_args_table[NR_hypercalls] = > >>> ARGS(xen_version, 2), > >>> ARGS(console_io, 3), > >>> ARGS(physdev_op_compat, 1), > >>> -#ifdef CONFIG_GRANT_TABLE > >>> +#if defined(CONFIG_GRANT_TABLE) || defined(CONFIG_PV_SHIM) > >>> ARGS(grant_table_op, 3), > >>> #endif > >>> ARGS(vm_assist, 2), > >> > >> This is correct when a shim-enabled build runs as shim, but > >> not when it runs as normal hypervisor. Just like the hypercall > >> handler gets patched into the hypercall table (in > >> pv_shim_setup_dom()), the argument count will also want > >> updating there, I think. > > > > Having the argument count set when the hypercall handler is NULL is > > fine, as Xen won't get into processing hypercall_args_table if the > > handler is NULL. > > Oh, good point. > > > While it's true that can be fixed at run time in the > > same way that we patch the handler it seems more cumbersome, that's > > why I went for this IMO simpler fix. > > Agreed: > Reviewed-by: Jan Beulich <jbeul...@suse.com>
Thanks. In fact I'm not sure it's helpful to have any build time conditionals in hypercall_args_table, but let's leave that for another fix. Roger.
