On 09.07.2025 02:44, dm...@proton.me wrote:
> On Wed, Jun 25, 2025 at 12:30:03AM +0000, dm...@proton.me wrote:
>> From: Denis Mukhin <dmuk...@ford.com>
>>
>> Introduce domain_console for grouping data structures used for integrating
>> domain's diagnostic console with Xen's console driver.
>>
>> Group all pbuf-related data structures under domain_console. Rename the moved
>> fields to plain .buf, .idx and .lock names, since all uses of the fields are
>> touched.
>>
>> Bump the domain console buffer size to the closest power of 2 (256) and
>> rename the symbol to DOMAIN_CONSOLE_BUF_SIZE.
>>
>> Finally, update the domain_console allocation and initialization code.
>>
>> No functional change.
>>
>> Signed-off-by: Denis Mukhin <dmuk...@ford.com>
>
> Can I ask for review?
I think I did give enough comments on earlier versions? I'm not entirely
comfortable actually ack-ing this change. One aspect is, that - despite
earlier comments - ...
>> --- a/xen/include/xen/sched.h
>> +++ b/xen/include/xen/sched.h
>> @@ -371,6 +371,25 @@ struct evtchn_port_ops;
>>
>> #define MAX_NR_IOREQ_SERVERS 8
>>
>> +/*
>> + * Arbitrary value.
>> + * The buffer is part of the dynamically-allocated data structure, using an
>> + * even multiple of a cache line size may help to optimize the allocation
>> + * overhead.
>> + */
>> +#define DOMAIN_CONSOLE_BUF_SIZE 256
... the comment here still isn't quite accurate imo, and hence potentially
misleading. The overall size of ...
>> +/* Domain console settings. */
>> +struct domain_console {
>> + /* Permission to take ownership of the physical console input. */
>> + bool input_allowed;
>> +
>> + /* hvm_print_line() and guest_console_write() logging. */
>> + spinlock_t lock;
>> + unsigned int idx;
>> + char buf[DOMAIN_CONSOLE_BUF_SIZE];
>> +};
... this struct isn't necessarily guaranteeing minimal overhead just
becasue buf[] size is a multiple of the system cacheline size.
Jan
