* [PATCH] x86/pv: Export pv_hypercall_table[] rather than working around it in several ways
@ 2018-01-24 12:16 Andrew Cooper
2018-01-24 12:52 ` Roger Pau Monné
2018-01-24 13:03 ` Jan Beulich
0 siblings, 2 replies; 3+ messages in thread
From: Andrew Cooper @ 2018-01-24 12:16 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Sergey Dyasli, Wei Liu, Jan Beulich, Roger Pau Monné
The functions in compat.c are thing wrappers around the main hypercalls,
massaging certain parameters. However, they second-guess the content of
pv_hypercall_table[], which is problematic for the shim case. Instead,
arrange for them to call via function pointer, which removes the need for
pv_get_hypercall_handler().
With pv_hypercall_table[] exported, there is no need for
pv_hypercall_table_replace(), so its single callsite gets modified to cope.
The backing code behind __va(__pa()) is substantial, and there is no need to
calculate it repeatedly (Xen's .rodata is also contiguous in the directmap).
While adjusting the declarations, guard content in arch/x86/pv with CONFIG_PV.
The net difference is:
add/remove: 0/2 grow/shrink: 4/1 up/down: 176/-321 (-145)
function old new delta
pv_shim_setup_dom 1130 1266 +136
do_sched_op_compat 176 192 +16
compat_physdev_op_compat 90 106 +16
do_physdev_op_compat 98 106 +8
do_event_channel_op_compat 145 123 -22
pv_get_hypercall_handler 28 - -28
pv_hypercall_table_replace 271 - -271
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Sergey Dyasli <sergey.dyasli@citrix.com>
---
xen/arch/x86/compat.c | 14 ++++++++++----
xen/arch/x86/pv/hypercall.c | 19 +------------------
xen/arch/x86/pv/shim.c | 21 +++++++++++++++------
xen/include/asm-x86/hypercall.h | 6 +++---
4 files changed, 29 insertions(+), 31 deletions(-)
diff --git a/xen/arch/x86/compat.c b/xen/arch/x86/compat.c
index 9d376a4..a40ec29 100644
--- a/xen/arch/x86/compat.c
+++ b/xen/arch/x86/compat.c
@@ -17,12 +17,14 @@ typedef long ret_t;
/* Legacy hypercall (as of 0x00030202). */
ret_t do_physdev_op_compat(XEN_GUEST_HANDLE(physdev_op_t) uop)
{
+ typeof(do_physdev_op) *fn =
+ (void *)pv_hypercall_table[__HYPERVISOR_physdev_op].native;
struct physdev_op op;
if ( unlikely(copy_from_guest(&op, uop, 1) != 0) )
return -EFAULT;
- return do_physdev_op(op.cmd, guest_handle_from_ptr(&uop.p->u, void));
+ return fn(op.cmd, guest_handle_from_ptr(&uop.p->u, void));
}
#ifndef COMPAT
@@ -30,11 +32,14 @@ ret_t do_physdev_op_compat(XEN_GUEST_HANDLE(physdev_op_t) uop)
/* Legacy hypercall (as of 0x00030101). */
long do_sched_op_compat(int cmd, unsigned long arg)
{
+ typeof(do_sched_op) *fn =
+ (void *)pv_hypercall_table[__HYPERVISOR_sched_op].native;
+
switch ( cmd )
{
case SCHEDOP_yield:
case SCHEDOP_block:
- return do_sched_op(cmd, guest_handle_from_ptr(NULL, void));
+ return fn(cmd, guest_handle_from_ptr(NULL, void));
case SCHEDOP_shutdown:
TRACE_3D(TRC_SCHED_SHUTDOWN,
@@ -52,6 +57,8 @@ long do_sched_op_compat(int cmd, unsigned long arg)
/* Legacy hypercall (as of 0x00030202). */
long do_event_channel_op_compat(XEN_GUEST_HANDLE_PARAM(evtchn_op_t) uop)
{
+ typeof(do_event_channel_op) *fn =
+ (void *)pv_hypercall_table[__HYPERVISOR_event_channel_op].native;
struct evtchn_op op;
if ( unlikely(copy_from_guest(&op, uop, 1) != 0) )
@@ -69,8 +76,7 @@ long do_event_channel_op_compat(XEN_GUEST_HANDLE_PARAM(evtchn_op_t) uop)
case EVTCHNOP_bind_ipi:
case EVTCHNOP_bind_vcpu:
case EVTCHNOP_unmask:
- return pv_get_hypercall_handler(__HYPERVISOR_event_channel_op, false)
- (op.cmd, (unsigned long)&uop.p->u, 0, 0, 0, 0);
+ return fn(op.cmd, guest_handle_from_ptr(&uop.p->u, void));
default:
return -ENOSYS;
diff --git a/xen/arch/x86/pv/hypercall.c b/xen/arch/x86/pv/hypercall.c
index 3b72d6a..bbc3011 100644
--- a/xen/arch/x86/pv/hypercall.c
+++ b/xen/arch/x86/pv/hypercall.c
@@ -32,7 +32,7 @@
#define do_arch_1 paging_domctl_continuation
-static const hypercall_table_t pv_hypercall_table[] = {
+const hypercall_table_t pv_hypercall_table[] = {
COMPAT_CALL(set_trap_table),
HYPERCALL(mmu_update),
COMPAT_CALL(set_gdt),
@@ -320,23 +320,6 @@ void hypercall_page_initialise_ring1_kernel(void *hypercall_page)
*(u16 *)(p+ 6) = (HYPERCALL_VECTOR << 8) | 0xcd; /* int $xx */
}
-void __init pv_hypercall_table_replace(unsigned int hypercall,
- hypercall_fn_t * native,
- hypercall_fn_t *compat)
-{
-#define HANDLER_POINTER(f) \
- ((unsigned long *)__va(__pa(&pv_hypercall_table[hypercall].f)))
- write_atomic(HANDLER_POINTER(native), (unsigned long)native);
- write_atomic(HANDLER_POINTER(compat), (unsigned long)compat);
-#undef HANDLER_POINTER
-}
-
-hypercall_fn_t *pv_get_hypercall_handler(unsigned int hypercall, bool compat)
-{
- return compat ? pv_hypercall_table[hypercall].compat
- : pv_hypercall_table[hypercall].native;
-}
-
/*
* Local variables:
* mode: C
diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
index d5383dc..9651952 100644
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -160,6 +160,7 @@ void __init pv_shim_setup_dom(struct domain *d, l4_pgentry_t *l4start,
unsigned long console_va, unsigned long vphysmap,
start_info_t *si)
{
+ hypercall_table_t *rw_pv_hypercall_table;
uint64_t param = 0;
long rc;
@@ -205,12 +206,20 @@ void __init pv_shim_setup_dom(struct domain *d, l4_pgentry_t *l4start,
mfn_x(console_mfn), vphysmap);
consoled_set_ring_addr(page);
}
- pv_hypercall_table_replace(__HYPERVISOR_event_channel_op,
- (hypercall_fn_t *)pv_shim_event_channel_op,
- (hypercall_fn_t *)pv_shim_event_channel_op);
- pv_hypercall_table_replace(__HYPERVISOR_grant_table_op,
- (hypercall_fn_t *)pv_shim_grant_table_op,
- (hypercall_fn_t *)pv_shim_grant_table_op);
+
+ /*
+ * Find a writeable mapping to pv_hypercall_table[] in the directmap
+ * insert some shim-specific hypercall handlers.
+ */
+ rw_pv_hypercall_table = __va(__pa(pv_hypercall_table));
+ rw_pv_hypercall_table[__HYPERVISOR_event_channel_op].native =
+ rw_pv_hypercall_table[__HYPERVISOR_event_channel_op].compat =
+ (hypercall_fn_t *)pv_shim_event_channel_op;
+
+ rw_pv_hypercall_table[__HYPERVISOR_grant_table_op].native =
+ rw_pv_hypercall_table[__HYPERVISOR_grant_table_op].compat =
+ (hypercall_fn_t *)pv_shim_grant_table_op;
+
guest = d;
/*
diff --git a/xen/include/asm-x86/hypercall.h b/xen/include/asm-x86/hypercall.h
index b9f3ecf..1cc2e37 100644
--- a/xen/include/asm-x86/hypercall.h
+++ b/xen/include/asm-x86/hypercall.h
@@ -25,12 +25,12 @@ typedef struct {
extern const hypercall_args_t hypercall_args_table[NR_hypercalls];
+#ifdef CONFIG_PV
+extern const hypercall_table_t pv_hypercall_table[];
void pv_hypercall(struct cpu_user_regs *regs);
void hypercall_page_initialise_ring3_kernel(void *hypercall_page);
void hypercall_page_initialise_ring1_kernel(void *hypercall_page);
-void pv_hypercall_table_replace(unsigned int hypercall, hypercall_fn_t * native,
- hypercall_fn_t *compat);
-hypercall_fn_t *pv_get_hypercall_handler(unsigned int hypercall, bool compat);
+#endif
/*
* Both do_mmuext_op() and do_mmu_update():
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] x86/pv: Export pv_hypercall_table[] rather than working around it in several ways
2018-01-24 12:16 [PATCH] x86/pv: Export pv_hypercall_table[] rather than working around it in several ways Andrew Cooper
@ 2018-01-24 12:52 ` Roger Pau Monné
2018-01-24 13:03 ` Jan Beulich
1 sibling, 0 replies; 3+ messages in thread
From: Roger Pau Monné @ 2018-01-24 12:52 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Sergey Dyasli, Wei Liu, Jan Beulich, Xen-devel
On Wed, Jan 24, 2018 at 12:16:09PM +0000, Andrew Cooper wrote:
> The functions in compat.c are thing wrappers around the main hypercalls,
> massaging certain parameters. However, they second-guess the content of
> pv_hypercall_table[], which is problematic for the shim case. Instead,
> arrange for them to call via function pointer, which removes the need for
> pv_get_hypercall_handler().
>
> With pv_hypercall_table[] exported, there is no need for
> pv_hypercall_table_replace(), so its single callsite gets modified to cope.
> The backing code behind __va(__pa()) is substantial, and there is no need to
> calculate it repeatedly (Xen's .rodata is also contiguous in the directmap).
>
> While adjusting the declarations, guard content in arch/x86/pv with CONFIG_PV.
>
> The net difference is:
> add/remove: 0/2 grow/shrink: 4/1 up/down: 176/-321 (-145)
> function old new delta
> pv_shim_setup_dom 1130 1266 +136
> do_sched_op_compat 176 192 +16
> compat_physdev_op_compat 90 106 +16
> do_physdev_op_compat 98 106 +8
> do_event_channel_op_compat 145 123 -22
> pv_get_hypercall_handler 28 - -28
> pv_hypercall_table_replace 271 - -271
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
LGTM:
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
> index d5383dc..9651952 100644
> --- a/xen/arch/x86/pv/shim.c
> +++ b/xen/arch/x86/pv/shim.c
> @@ -160,6 +160,7 @@ void __init pv_shim_setup_dom(struct domain *d, l4_pgentry_t *l4start,
> unsigned long console_va, unsigned long vphysmap,
> start_info_t *si)
> {
> + hypercall_table_t *rw_pv_hypercall_table;
> uint64_t param = 0;
> long rc;
>
> @@ -205,12 +206,20 @@ void __init pv_shim_setup_dom(struct domain *d, l4_pgentry_t *l4start,
> mfn_x(console_mfn), vphysmap);
> consoled_set_ring_addr(page);
> }
> - pv_hypercall_table_replace(__HYPERVISOR_event_channel_op,
> - (hypercall_fn_t *)pv_shim_event_channel_op,
> - (hypercall_fn_t *)pv_shim_event_channel_op);
> - pv_hypercall_table_replace(__HYPERVISOR_grant_table_op,
> - (hypercall_fn_t *)pv_shim_grant_table_op,
> - (hypercall_fn_t *)pv_shim_grant_table_op);
> +
> + /*
> + * Find a writeable mapping to pv_hypercall_table[] in the directmap
> + * insert some shim-specific hypercall handlers.
^ and
Maybe add something like:
"The hypercall table lives in .rodata, and so it's contiguous in
physical address space, which makes it also contiguous in VA space in
the direct map"
Or better worded.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] x86/pv: Export pv_hypercall_table[] rather than working around it in several ways
2018-01-24 12:16 [PATCH] x86/pv: Export pv_hypercall_table[] rather than working around it in several ways Andrew Cooper
2018-01-24 12:52 ` Roger Pau Monné
@ 2018-01-24 13:03 ` Jan Beulich
1 sibling, 0 replies; 3+ messages in thread
From: Jan Beulich @ 2018-01-24 13:03 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Sergey Dyasli, Xen-devel, Wei Liu, Roger Pau Monné
>>> On 24.01.18 at 13:16, <andrew.cooper3@citrix.com> wrote:
> The functions in compat.c are thing wrappers around the main hypercalls,
> massaging certain parameters. However, they second-guess the content of
> pv_hypercall_table[], which is problematic for the shim case. Instead,
> arrange for them to call via function pointer, which removes the need for
> pv_get_hypercall_handler().
>
> With pv_hypercall_table[] exported, there is no need for
> pv_hypercall_table_replace(), so its single callsite gets modified to cope.
> The backing code behind __va(__pa()) is substantial, and there is no need to
> calculate it repeatedly (Xen's .rodata is also contiguous in the directmap).
>
> While adjusting the declarations, guard content in arch/x86/pv with
> CONFIG_PV.
>
> The net difference is:
> add/remove: 0/2 grow/shrink: 4/1 up/down: 176/-321 (-145)
> function old new delta
> pv_shim_setup_dom 1130 1266 +136
> do_sched_op_compat 176 192 +16
> compat_physdev_op_compat 90 106 +16
> do_physdev_op_compat 98 106 +8
> do_event_channel_op_compat 145 123 -22
> pv_get_hypercall_handler 28 - -28
> pv_hypercall_table_replace 271 - -271
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-01-24 13:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-24 12:16 [PATCH] x86/pv: Export pv_hypercall_table[] rather than working around it in several ways Andrew Cooper
2018-01-24 12:52 ` Roger Pau Monné
2018-01-24 13:03 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.