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

[PATCH v2 2/4] common: reduce PV shim footprint


  • To: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Wed, 1 Mar 2023 15:14:41 +0100
  • 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=DyqpzDT7ddgLuTyJ8LCdvZqSXAQOB28REr3pHpSd2aU=; b=P4MuBvvpK1eha9CROFA/TquIX/O1KeEHJpEnBoPQNd1tJtVhsxUIM1+PJX127KHZ1udszJq54hrZiXndoFStSzSTuJr9QMtOaUJGV5vdA9mWXb6ITYIPij81B0x2tphpxZfMBaiA9CYCwrEdQXQWuGgAneCW61wh98kg2OTP7193CoXDZ/6flwZI7tIVIkc9Anum2aDt0rdJ00scOlM6JHjM1utBZ9BdCOn0xyAoU89fqk3NWKOgRXi++1aZtiUaBabtLUQftzdpAI500HSZSKJ9uT2a4vobixDqCP18TtLSt231+7d8ROw1bUepzoGzTNS3LOTEdnXMoC8o5uYYtg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=MT4HYdvsoxogJ/GowwguXLde2BnLiQ9eti0nXH8/Yd9sKqET5QB9Rnr0N5gidcYZ2D4fS5xhjn2ubOlLPKrUTV4C+6vs1NddCpiJxw+7cHImlMh54btHJzOrfj0Hj2owAi1cDv0BOrwKlLuObI99SssgHxGxbiD/Ej1xnp49zlTCme5w7iJuMzQmWKqVvco/SuIp+cRkcAceXcV5jlIOz6iibHAZUCWyXurh8zlmvD0xpzPIQamEnR9etHnv5rbM0RW14l31YNQjcVVBH4HY792zSsPMDCzA5gMkBB1D4xOf9n94MxLxYOB+f91xiX+BzITQCc+bkOEsh9aKbuHPtw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Daniel Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>, Dario Faggioli <dfaggioli@xxxxxxxx>
  • Delivery-date: Wed, 01 Mar 2023 14:15:00 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Having CONFIG_PV_SHIM conditionals in common code isn't really nice.
Utilize that we're no longer invoking hypercall handlers via indirect
calls through a table of function vectors. With the use of direct calls
from the macros defined by hypercall-defs.h, we can simply define
overriding macros for event channel and grant table ops handling. All
this requires is arrangement for careful double inclusion of
asm/hypercall.h out of xen/hypercall.h. Such double inclusion is
required because hypercall-defs.h expects certain definitions to be in
place, while the new handling (placed in pv/shim.h, which is now
included from asm/hypercall.h despite the apparent cyclic dependency)
requires prototypes from hypercall-defs.h to be available already.

Note that this makes it necessary to further constrain the stubbing of
pv_shim from common/sched/core.c, and allows removing the inclusion of
asm/guest.h there as well. Since this is actually part of the overall
goal, leverage the mechanism to also get rid of the similar construct in
xsm/flask/hooks.c, including xen/hypercall.h instead.

Note further that kind of as a side effect this fixes grant table
handling for 32-bit shim guests when GRANT_TABLE=y, as the non-stub
compat_grant_table_op() did not redirect to pv_shim_grant_table_op().

A downside of this is that now do_{event_channel,grant_table}_op() are
built in full again when PV_SHIM_EXCLUSIVE=y, despite all the code
actually being dead in that case.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
RFC: Sadly I had to restore the two "#define pv_shim false", for Arm to
     continue to build. Originally I was hoping to get rid of that
     #ifdef-ary altogether. Would it be acceptable to put a single,
     central #define in e.g. xen/sched.h or xen/hypercall.h?

--- a/xen/arch/x86/include/asm/hypercall.h
+++ b/xen/arch/x86/include/asm/hypercall.h
@@ -6,14 +6,23 @@
 #error "asm/hypercall.h should not be included directly - include 
xen/hypercall.h instead"
 #endif
 
-#ifndef __ASM_X86_HYPERCALL_H__
-#define __ASM_X86_HYPERCALL_H__
-
 #include <xen/types.h>
+#include <public/xen.h>
 #include <public/physdev.h>
-#include <public/event_channel.h>
 #include <public/arch-x86/xen-mca.h> /* for do_mca */
+
+#ifdef CONFIG_COMPAT
+#include <compat/arch-x86/xen.h>
+#include <compat/physdev.h>
+#include <compat/platform.h>
+#endif
+
+#if !defined(__ASM_X86_HYPERCALL_H__) && \
+    (!defined(CONFIG_PV_SHIM) || defined(hypercall_args_pv64))
+#define __ASM_X86_HYPERCALL_H__
+
 #include <asm/paging.h>
+#include <asm/pv/shim.h>
 
 #define __HYPERVISOR_paging_domctl_cont __HYPERVISOR_arch_1
 
@@ -33,10 +42,6 @@ void pv_ring3_init_hypercall_page(void *
 
 #ifdef CONFIG_COMPAT
 
-#include <compat/arch-x86/xen.h>
-#include <compat/physdev.h>
-#include <compat/platform.h>
-
 extern int
 compat_common_vcpu_op(
     int cmd, struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg);
--- a/xen/arch/x86/include/asm/pv/shim.h
+++ b/xen/arch/x86/include/asm/pv/shim.h
@@ -49,6 +49,22 @@ const struct platform_bad_page *pv_shim_
 typeof(do_event_channel_op) pv_shim_event_channel_op;
 typeof(do_grant_table_op) pv_shim_grant_table_op;
 
+#ifdef CONFIG_PV_SHIM_EXCLUSIVE
+#define REVECTOR(pfx, op, args...) pv_shim_ ## op(args)
+#else
+#define REVECTOR(pfx, op, args...) ({ \
+    likely(!pv_shim) \
+    ? pfx ## _ ## op(args) \
+    : pv_shim_ ## op(args); \
+})
+#endif
+
+#define do_event_channel_op(args...) REVECTOR(do, event_channel_op, args)
+#define do_grant_table_op(args...) REVECTOR(do, grant_table_op, args)
+#ifdef CONFIG_COMPAT
+#define compat_grant_table_op(args...) REVECTOR(compat, grant_table_op, args)
+#endif
+
 #else
 
 static inline void pv_shim_setup_dom(struct domain *d, l4_pgentry_t *l4start,
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -822,9 +822,9 @@ long pv_shim_grant_table_op(unsigned int
     return rc;
 }
 
-#ifndef CONFIG_GRANT_TABLE
+#if !defined(CONFIG_GRANT_TABLE) && !defined(CONFIG_PV_SHIM_EXCLUSIVE)
 /* Thin wrapper(s) needed. */
-long do_grant_table_op(
+long (do_grant_table_op)(
     unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) uop, unsigned int count)
 {
     if ( !pv_shim )
@@ -834,7 +834,7 @@ long do_grant_table_op(
 }
 
 #ifdef CONFIG_PV32
-int compat_grant_table_op(
+int (compat_grant_table_op)(
     unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) uop, unsigned int count)
 {
     if ( !pv_shim )
--- a/xen/common/compat/grant_table.c
+++ b/xen/common/compat/grant_table.c
@@ -56,7 +56,7 @@ CHECK_gnttab_swap_grant_ref;
 CHECK_gnttab_cache_flush;
 #undef xen_gnttab_cache_flush
 
-int compat_grant_table_op(
+int (compat_grant_table_op)(
     unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) cmp_uop, unsigned int count)
 {
     int rc = 0;
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -32,10 +32,6 @@
 #include <public/event_channel.h>
 #include <xsm/xsm.h>
 
-#ifdef CONFIG_PV_SHIM
-#include <asm/guest.h>
-#endif
-
 #define ERROR_EXIT(_errno)                                          \
     do {                                                            \
         gdprintk(XENLOG_WARNING,                                    \
@@ -1222,15 +1218,10 @@ static int evtchn_set_priority(const str
     return ret;
 }
 
-long do_event_channel_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
+long (do_event_channel_op)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     int rc;
 
-#ifdef CONFIG_PV_SHIM
-    if ( unlikely(pv_shim) )
-        return pv_shim_event_channel_op(cmd, arg);
-#endif
-
     switch ( cmd )
     {
     case EVTCHNOP_alloc_unbound: {
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -45,10 +45,6 @@
 #include <asm/flushtlb.h>
 #include <asm/guest_atomics.h>
 
-#ifdef CONFIG_PV_SHIM
-#include <asm/guest.h>
-#endif
-
 /* Per-domain grant information. */
 struct grant_table {
     /*
@@ -3563,17 +3559,12 @@ gnttab_cache_flush(XEN_GUEST_HANDLE_PARA
     return 0;
 }
 
-long do_grant_table_op(
+long (do_grant_table_op)(
     unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) uop, unsigned int count)
 {
     long rc;
     unsigned int opaque_in = cmd & GNTTABOP_ARG_MASK, opaque_out = 0;
 
-#ifdef CONFIG_PV_SHIM
-    if ( unlikely(pv_shim) )
-        return pv_shim_grant_table_op(cmd, uop, count);
-#endif
-
     if ( (int)count < 0 )
         return -EINVAL;
 
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -40,9 +40,7 @@
 
 #include "private.h"
 
-#ifdef CONFIG_XEN_GUEST
-#include <asm/guest.h>
-#else
+#ifndef CONFIG_X86
 #define pv_shim false
 #endif
 
--- a/xen/include/xen/hypercall.h
+++ b/xen/include/xen/hypercall.h
@@ -24,6 +24,9 @@
 /* Needs to be after asm/hypercall.h. */
 #include <xen/hypercall-defs.h>
 
+/* Include a 2nd time, for x86'es PV shim. */
+#include <asm/hypercall.h>
+
 extern long
 arch_do_domctl(
     struct xen_domctl *domctl, struct domain *d,
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -19,6 +19,7 @@
 #include <xen/cpumask.h>
 #include <xen/errno.h>
 #include <xen/guest_access.h>
+#include <xen/hypercall.h>
 #include <xen/xenoprof.h>
 #include <xen/iommu.h>
 #ifdef CONFIG_HAS_PCI_MSI
@@ -38,9 +39,7 @@
 #include <conditional.h>
 #include "private.h"
 
-#ifdef CONFIG_X86
-#include <asm/pv/shim.h>
-#else
+#ifndef CONFIG_X86
 #define pv_shim false
 #endif
 




 


Rackspace

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