[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 08/12] x86/pv-shim: don't modify hypercall table


  • To: Juergen Gross <jgross@xxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Fri, 15 Oct 2021 15:57:32 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=9JWv/VNFc7LRjtRkF+Z3aoimnueAbl2VwHj/UtZSMVI=; b=RTma+yq0OZuDXI4bMNZ3WuZJRH/ZY5wv0ita8XGEGye+Z27vYF9efiQgAUkWZqC7cWZQnmzNLmcgtfHlf/2Y9eJ8C2d+8mdu1U+D/CxCqm94tb0lL69KsAtO3cGgwevrbaG+AU7nq3LJlS8AxBw1m5/hpxRsrIcchZy6rz2TOLDIF86Lm15l3xXXJ9WwsW4pmoJfSeEu5G4fID3BBUMxlvQylBriU/FSdgb2QbqbfiWLNOv43D+AA2uoA41nC9FCHbp+bcDxSLctBBAP/vZnKtfOecxSD51apWKrcbOy8v0yLvK3gV+YSw1haBGlTwNPF1g0h08ZFrxEElqHQcOfRg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=KFj8BU/k7IJOYMumema3iUbweeQfyvL14RVcoBFMVGAaA1HdcGuivQQwPA8ycxEZWuGeJWx8F8fzcV9EJKGTA63jeiLq5WXjboBCX1MRWcv8G81Ja+i6Sw9wXElw0n9VUSXG/iQSt1BqjKkGJwUnkDNfjOoGFdOVqJX5tJJUtKj1WJzqGoXa1dbHEYVVME2bhcbnDV5nYuTd/AdqIQIPly0uZhBDLAxdFZdvBPQhhcaIkuG0TndKOHLd+dl3qotLdPz47opmX9uL6tIKaNSnbpNQrqhERU+7puTT+yVmGoarHpwwiZBH8GBYxPidJdnoaEVUwsfsdeObOjAMXGRZtQ==
  • Authentication-results: lists.xenproject.org; dkim=none (message not signed) header.d=none;lists.xenproject.org; dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Fri, 15 Oct 2021 13:57:43 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 15.10.2021 14:51, 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.

While you say CONFIG_PV_SHIM_EXCLUSIVE here, ...

> @@ -845,6 +822,23 @@ static long pv_shim_grant_table_op(unsigned int cmd,
>      return rc;
>  }
>  
> +#ifndef CONFIG_GRANT_TABLE

... you don't actually enforce this here. I also don't see why it would
be needed in the "exclusive" case only. A binary usable both ways would
still need these, wouldn't it?

> +/* Thin wrapper(s) needed. */
> +long do_grant_table_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) uop,
> +                       unsigned int count)
> +{
> +    return pv_shim_grant_table_op(cmd, uop, count);
> +}
> +
> +#ifdef CONFIG_PV32
> +int compat_grant_table_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) uop,
> +                          unsigned int count)
> +{
> +    return pv_shim_grant_table_op(cmd, uop, count);
> +}
> +#endif
> +#endif

Don't you also need to adjust the respective #ifdef in
pv_hypercall_table[]? Otherwise I don't see how, at this point of the
series, the functions would actually get hooked up. In a bi-modal
binary further guarding will then be needed inside the wrappers, I
think. (While the table is going to go away, that guarding is going
to be needed even at the end of this series, I believe.)

Talking of wrappers - do you need actual code to be emitted for this?
Can't you simply put in place an alias each, which is permitted now
that pv_shim_grant_table_op() isn't static anymore? (Albeit that's
partly moot if guarding code gets added to the functions - in that
case only compat_grant_table_op() could become an alias of
do_grant_table_op().)

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.