Konrad Rzeszutek Wilk
2013-Sep-27 21:17 UTC
[PATCH] hvm: Allow HVM guests to make console_io hypercall.
The console_io hypercall is provided for PV guests and for HVM guests it is done via the 0xe9 port. However the PV hypercall is more efficient as it takes a string rather than one character per write. Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- xen/arch/x86/hvm/hvm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index e0e0f5d..1f0d795 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -3299,6 +3299,7 @@ static hvm_hypercall_t *const hvm_hypercall64_table[NR_hypercalls] = { [ __HYPERVISOR_vcpu_op ] = (hvm_hypercall_t *)hvm_vcpu_op, [ __HYPERVISOR_physdev_op ] = (hvm_hypercall_t *)hvm_physdev_op, HYPERCALL(xen_version), + HYPERCALL(console_io), HYPERCALL(event_channel_op), HYPERCALL(sched_op), HYPERCALL(set_timer_op), @@ -3318,6 +3319,7 @@ static hvm_hypercall_t *const hvm_hypercall32_table[NR_hypercalls] = { [ __HYPERVISOR_vcpu_op ] = (hvm_hypercall_t *)hvm_vcpu_op_compat32, [ __HYPERVISOR_physdev_op ] = (hvm_hypercall_t *)hvm_physdev_op_compat32, COMPAT_CALL(xen_version), + HYPERCALL(console_io), HYPERCALL(event_channel_op), COMPAT_CALL(sched_op), COMPAT_CALL(set_timer_op), -- 1.8.3.1
Jan Beulich
2013-Sep-30 08:04 UTC
Re: [PATCH] hvm: Allow HVM guests to make console_io hypercall.
>>> On 27.09.13 at 23:17, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > The console_io hypercall is provided for PV guests and for HVM > guests it is done via the 0xe9 port. However the PV hypercall > is more efficient as it takes a string rather than one character > per write.While the change itself is fine, there are two questions arising from me looking at do_console_io() in this context: 1) While I can see why VERBOSE ought to control access to CONSOLEIO_write, I would think CONSOLEIO_read ought to be constrained to the control domain in any case. 2) xsm_console_io(), like quite a few other stubs in xsm/dummy.h, ignores its "d" parameter and uses current->domain instead. Is that really the right way (and if so, why are there other stubs that do honor their inputs)? Jan
Daniel De Graaf
2013-Sep-30 14:04 UTC
Re: [PATCH] hvm: Allow HVM guests to make console_io hypercall.
On 09/30/2013 04:04 AM, Jan Beulich wrote:>>>> On 27.09.13 at 23:17, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: >> The console_io hypercall is provided for PV guests and for HVM >> guests it is done via the 0xe9 port. However the PV hypercall >> is more efficient as it takes a string rather than one character >> per write. > > While the change itself is fine, there are two questions arising > from me looking at do_console_io() in this context: > > 1) While I can see why VERBOSE ought to control access to > CONSOLEIO_write, I would think CONSOLEIO_read ought to be > constrained to the control domain in any case.I agree on read. Do we still need the restriction on write with non-VERBOSE builds now that domains'' output is restricted to the guest debug loglevel? I recall this being discussed briefly when the output changes were being made, but I believe the decision was only that such a change belonged in a different patch.> 2) xsm_console_io(), like quite a few other stubs in xsm/dummy.h, > ignores its "d" parameter and uses current->domain instead. Is > that really the right way (and if so, why are there other stubs > that do honor their inputs)? > > JanNo, this is incorrect - it should be using the parameter, same as any of the other stubs that have a parameter that is always set to current->domain. -- Daniel De Graaf National Security Agency
Jan Beulich
2013-Sep-30 15:26 UTC
Re: [PATCH] hvm: Allow HVM guests to make console_io hypercall.
>>> On 30.09.13 at 16:04, Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote: > On 09/30/2013 04:04 AM, Jan Beulich wrote: >> 2) xsm_console_io(), like quite a few other stubs in xsm/dummy.h, >> ignores its "d" parameter and uses current->domain instead. Is >> that really the right way (and if so, why are there other stubs >> that do honor their inputs)? > > No, this is incorrect - it should be using the parameter, same as > any of the other stubs that have a parameter that is always set > to current->domain.Can we expect you to clean this up then? Jan