Re: [PATCH RFC] xen/console: buffer and show origin of guest PV writes

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

On 08/13/2013 01:18 PM, Andrew Cooper wrote:
> On 13/08/13 17:34, Daniel De Graaf wrote:
>> Guests other than domain 0 using the console output have previously been
>> controlled by the VERBOSE define, but with no designation of which
>> guest's output was on the console. This patch converts the HVM output
>> buffering to be used by all domains, line buffering their output and
>> prefixing it with the domain ID. This is especially useful for debugging
>> stub domains during early boot.
>>
>> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>>
>> ---
>>
>> A few questions for discussion (the reason this is an RFC):
>>
>> 1. HVM guests' output is currently limited to printable characters; do
>> we want to implement the same restriction on PV guests?
>
> No.  If a guest is doing something such as listing HP ACPI tables
> (something dom0 would very reasonably do on HP hardware), restricting to
> printable characters will leave strange omissions.

Dom0 would not be included in this restriction, in order to allow other
control characters (using console=hvc0 with Fedora includes color codes
and other control characters that would turn into a mess with such
stripping enabled). I have not observed a domU trying to do this since
they will switch to the shared-page PV console prior to this output.

> HVM guests should be relaxed, with PVH on the way.

HVM guests can still use the PV output - they just need to use the console
write hypercall instead of the HVM I/O port. I would think that PVH guests
would default to using the hypercall as it is more efficient (it takes a
string rather than one character per write).

Actually, checking... the console_io hypercall would need to be added to
the hvm_hypercall{32,64}_table for an HVM guest to be able to use it; they
currently must use the I/O port. I didn't check the PVH patches.

>>
>> 2. The prefix on the serial console for PV output is "(XEN) d5: ", while
>> HVM output is still "(XEN) HVM5: "; should these be made consistent?
>
> I tend to find it useful to distinguish between HVM and PV at a glance,
> but would agree that something more consistent would be better.

Agreed.

[...]
>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
>> index ae6a3b8..5a5d7ba 100644
>> --- a/xen/include/xen/sched.h
>> +++ b/xen/include/xen/sched.h
>> @@ -341,6 +341,12 @@ struct domain
>>       /* Control-plane tools handle for this domain. */
>>       xen_domain_handle_t handle;
>>
>> +    /* hvm_print_line() and guest_console_write() logging. */
>> +#define DOMAIN_PBUF_SIZE 80
>> +    char       *pbuf;
>> +    int         pbuf_idx;
>
> It might have been wrong before, but as it is changing, can we please
> use the correct type, unsigned, for an index.
>
> ~Andrew

Seems like a good idea.

>> +    spinlock_t  pbuf_lock;
>> +
>>       /* OProfile support. */
>>       struct xenoprof *xenoprof;
>>       int32_t time_offset_seconds;

-- 
Daniel De Graaf
National Security Agency