On 01.11.2021 16:20, Juergen Gross wrote:
> When running as pv-shim the hypercall is modified today in order to
> replace the functions for __HYPERVISOR_event_channel_op and
> __HYPERVISOR_grant_table_op hypercalls.
>
> Change this to call the related functions from the normal handlers
> instead when running as shim. The performance implications are not
> really relevant, as a normal production hypervisor will not be
> configured to support shim mode, so the related calls will be dropped
> due to optimization of the compiler.
>
> Note that for the CONFIG_PV_SHIM_EXCLUSIVE case there is a dummy
> wrapper do_grant_table_op() needed, as in this case grant_table.c
> isn't being built.
>
> Signed-off-by: Juergen Gross <jgr...@suse.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
albeit with a remark:
> @@ -1190,6 +1194,11 @@ long do_event_channel_op(int cmd,
> XEN_GUEST_HANDLE_PARAM(void) arg)
> {
> int rc;
>
> +#ifdef CONFIG_PV_SHIM
> + if ( pv_shim )
> + return pv_shim_event_channel_op(cmd, arg);
> +#endif
At the example of this, this is sub-optimal: In !PV_SHIM_EXCLUSIVE
this would imo better be unlikely(pv_shim). If it was just for the
positive forms, the annotation could actually be included in
pv_shim itself:
extern bool pv_shim;
#define pv_shim unlikely(pv_shim)
but this wouldn't necessarily play well with e.g. "if ( !pv_shim )".
As I would hope compilers don't mind constructs like "unlikely(1)",
I'd like to suggest considering to add likely() / unlikely() on
this and similar paths you add here.
Jan
