[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v6] xen/console: introduce domain_console struct
On Thu, Jul 10, 2025 at 01:16:24PM +0200, Jan Beulich wrote: > On 10.07.2025 03:35, dmkhn@xxxxxxxxx wrote: > > @@ -877,6 +873,16 @@ struct domain *domain_create(domid_t domid, > > > > /* All error paths can depend on the above setup. */ > > > > + BUILD_BUG_ON(DOMAIN_CONSOLE_BUF_SIZE <= 0); > > While the "equals 0" case can in principle happen, the "less than" part > is dead code (and hence this needs checking differently): The type of > DOMAIN_CONSOLE_BUF_SIZE is an unsigned one, so wrapping through 0 will > yield huge positive values. > > > + err = -ENOMEM; > > + d->console = xzalloc_bytes(DOMAIN_CONSOLE_SIZE); > > As previously indicated, new code ought to use the xv*alloc family of > functions, which deliberately doesn't include any ..._bytes() forms. > Note how instead there is xvzalloc_flex_struct() for situations like > the one here. Looks like xvzalloc_flex_struct() is not used anywhere in the code base... > > > @@ -371,6 +373,26 @@ struct evtchn_port_ops; > > > > #define MAX_NR_IOREQ_SERVERS 8 > > > > +/* > > + * Domain console settings is the dynamically-allocated data structure. > > + * Using an even multiple of a cache line size may help to optimize the > > + * allocation overhead. > > + */ > > +#define DOMAIN_CONSOLE_SIZE ROUNDUP(256, SMP_CACHE_BYTES) > > +#define DOMAIN_CONSOLE_BUF_SIZE (DOMAIN_CONSOLE_SIZE - \ > > + sizeof(struct domain_console)) > > But you're aware that there's allocation overhead, which consumes part of > a cacheline? I simply don't understand why this struct is so different > from others that such cleverness needs building in. Yet if it's relevant, > it really needs doing correctly. > > > +/* 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. */ > > + unsigned int idx; > > + spinlock_t lock; > > + char buf[0]; > > Iirc recent gcc warns about the use of this historic gcc extension, since > there has been the better form using just [] for quite a long time. > > Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |