[PATCH] hvm: Allow HVM guests to make console_io hypercall.

[PATCH] hvm: Allow HVM guests to make console_io hypercall.

[Konrad Rzeszutek Wilk]

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

Re: [PATCH] hvm: Allow HVM guests to make console_io hypercall.

[Jan Beulich]

>>> 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

Re: [PATCH] hvm: Allow HVM guests to make console_io hypercall.

[Daniel De Graaf]

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)?
>
> Jan

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.

-- 
Daniel De Graaf
National Security Agency

Re: [PATCH] hvm: Allow HVM guests to make console_io hypercall.

[Jan Beulich]

>>> 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