On 09.08.2025 20:55, dm...@proton.me wrote:
> On Mon, Aug 04, 2025 at 12:11:03PM +0200, Jan Beulich wrote:
>> On 31.07.2025 21:21, dm...@proton.me wrote:
>>> --- a/xen/common/Kconfig
>>> +++ b/xen/common/Kconfig
>>> @@ -1,6 +1,8 @@
>>>
>>> menu "Common Features"
>>>
>>> +source "common/emul/Kconfig"
>>> +
>>> config COMPAT
>>
>> Why at the very top?
>
> I did not find a better place, since the settings are not sorted and to me it
> makes sense to list emulation capabilities first...
>
> Where would be the best location for that submenu?
> Close to another submenu `source "common/sched/Kconfig"`?
At least below there. Possibly yet further down.
>>> +int vuart_init(struct domain *d, struct vuart_params *params)
>>> +{
>>> + const struct vuart_ops *vdev;
>>> + int rc;
>>> +
>>> + if ( !domain_has_vuart(d) )
>>> + return 0;
>>> +
>>> + for_each_vuart(vdev)
>>> + {
>>> + rc = vdev->init(d, params);
>>> + if ( rc )
>>> + return rc;
>>> + }
>>> +
>>> + d->console.input_allowed = true;
>>
>> Unconditionally?
>
> Thanks.
> That should be a least under rc == 0.
You only ever make it there with rc == 0, though. (In fact that variable's
scope would better be just the loop body.)
>>> +/*
>>> + * Put character to the first suitable emulated UART's FIFO.
>>> + */
>>
>> What's "suitable"? Along the lines of the earlier remark, what if the domain
>> has vUART kind A configured, ...
>
> "suitable" is meant to be the first emulator with put_rx != NULL.
> I will update that.
Except that, as iirc Roger also pointed out, "first emulator with put_rx !=
NULL"
is a questionable condition.
>>> --- a/xen/common/keyhandler.c
>>> +++ b/xen/common/keyhandler.c
>>> @@ -22,6 +22,7 @@
>>> #include <xen/mm.h>
>>> #include <xen/watchdog.h>
>>> #include <xen/init.h>
>>> +#include <xen/vuart.h>
>>> #include <asm/div64.h>
>>>
>>> static unsigned char keypress_key;
>>> @@ -354,6 +355,8 @@ static void cf_check dump_domains(unsigned char key)
>>> v->periodic_period / 1000000);
>>> }
>>> }
>>> +
>>> + vuart_dump_state(d);
>>
>> How verbose is this going to get?
>
> Looks something like this:
> ```
> (XEN) [ 88.334893] 'q' pressed -> dumping domain info (now = 88334828303)
> [..]
> (XEN) [ 88.335673] Virtual ns16550 (COM2) I/O port 0x02f8 IRQ#3 owner d0
> (XEN) [ 88.335681] RX FIFO size 1024 in_prod 258 in_cons 258 used 0
> (XEN) [ 88.335689] TX FIFO size 2048 out_prod 15 out_cons 0 used 15
> (XEN) [ 88.335696] 00 RBR 02 THR 6f DLL 01 DLM 00
> (XEN) [ 88.335703] 01 IER 05
> (XEN) [ 88.335709] 02 FCR 81 IIR c1
> (XEN) [ 88.335715] 03 LCR 13
> (XEN) [ 88.335720] 04 MCR 0b
> (XEN) [ 88.335726] 05 LSR 60
> (XEN) [ 88.335731] 06 MSR b0
> (XEN) [ 88.335736] 07 SCR 00
>
> ```
Definitely too much (for my taste) to put under 'q'.
Jan
