| Message ID | 20241205-vuart-ns8250-v1-20-e9aa923127eb@ford.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce NS8250 UART emulator | expand |
On 2024-12-05 23:41, Denis Mukhin via B4 Relay wrote: > From: Denis Mukhin <dmukhin@ford.com> > > console_owner_domid() is introduced to obtain the "console owner" domain ID. > > The call is used in NS8250 emulator to identify the case when physical xen > console focus is owned by the domain w/ NS8250 emulator, in which case, > messages from guest OS are formatted w/o '(XEN)' prefix. > > Signed-off-by: Denis Mukhin <dmukhin@ford.com> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com> I expected this to be used immediately by patch 21, but it is not. You might want to re-order it directly before its first use is introduced. I haven't gotten far enough to know when that is. Regards, Jason
On 06.12.2024 05:41, Denis Mukhin via B4 Relay wrote: > From: Denis Mukhin <dmukhin@ford.com> > > console_owner_domid() is introduced to obtain the "console owner" domain ID. > > The call is used in NS8250 emulator to identify the case when physical xen > console focus is owned by the domain w/ NS8250 emulator, in which case, > messages from guest OS are formatted w/o '(XEN)' prefix. Such messages ought to be processed through guest_printk(), which wants a domain pointer, not a domid_t anyway. Plus isn't that going to be current->domain anyway at the callsite, eliminating the need for such a helper altogether? Jan
On 10.12.2024 23:11, Jason Andryuk wrote: > On 2024-12-05 23:41, Denis Mukhin via B4 Relay wrote: >> From: Denis Mukhin <dmukhin@ford.com> >> >> console_owner_domid() is introduced to obtain the "console owner" domain ID. >> >> The call is used in NS8250 emulator to identify the case when physical xen >> console focus is owned by the domain w/ NS8250 emulator, in which case, >> messages from guest OS are formatted w/o '(XEN)' prefix. >> >> Signed-off-by: Denis Mukhin <dmukhin@ford.com> > > Reviewed-by: Jason Andryuk <jason.andryuk@amd.com> > > I expected this to be used immediately by patch 21, but it is not. You > might want to re-order it directly before its first use is introduced. > I haven't gotten far enough to know when that is. Plus, no matter how far in the future it is, there'll be a window where the Misra rule of not having unreachable code in the project is violated. New functions now really need introducing when their first caller appears. Jan
On Thu, Dec 05, 2024 at 08:41:50PM -0800, Denis Mukhin via B4 Relay wrote: > From: Denis Mukhin <dmukhin@ford.com> > > console_owner_domid() is introduced to obtain the "console owner" domain ID. > > The call is used in NS8250 emulator to identify the case when physical xen > console focus is owned by the domain w/ NS8250 emulator, in which case, > messages from guest OS are formatted w/o '(XEN)' prefix. Nit: it would be better to not use abbreviations such as w/ or w/o in commit messages. > > Signed-off-by: Denis Mukhin <dmukhin@ford.com> > --- > xen/drivers/char/console.c | 5 +++++ > xen/include/xen/console.h | 1 + > 2 files changed, 6 insertions(+) > > diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c > index 52cf64dbf6fd18d599cb88835d03501a23b3e3c4..a8ab5c2bcb98e4cadf9ad2c9ad28d297977d0557 100644 > --- a/xen/drivers/char/console.c > +++ b/xen/drivers/char/console.c > @@ -498,6 +498,11 @@ struct domain *rcu_lock_domain_console_owner(void) > return rcu_lock_domain_console_by_id(console_owner); > } > > +domid_t console_owner_domid(void) > +{ > + return console_owner; > +} > + > static bool console_owner_possible(domid_t domid) > { > struct domain *d; > diff --git a/xen/include/xen/console.h b/xen/include/xen/console.h > index 57c482cfbf2da15b011e64841ea086e779f4588d..83be5794aff6630beaad46f910fcc0fc6d833808 100644 > --- a/xen/include/xen/console.h > +++ b/xen/include/xen/console.h > @@ -33,6 +33,7 @@ void console_end_log_everything(void); > > struct domain *rcu_lock_domain_console_owner(void); > int console_set_owner(domid_t); > +domid_t console_owner_domid(void); I would expect that either the caller already has a domain locked, or uses rcu_lock_domain_console_owner() to obtain the domain and then get the domid? (d->domain_id?) It's hard to tell why you need such way to get the console input target domid in such a way without seeing a caller to the function. Thanks, Roger.
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c index 52cf64dbf6fd18d599cb88835d03501a23b3e3c4..a8ab5c2bcb98e4cadf9ad2c9ad28d297977d0557 100644 --- a/xen/drivers/char/console.c +++ b/xen/drivers/char/console.c @@ -498,6 +498,11 @@ struct domain *rcu_lock_domain_console_owner(void) return rcu_lock_domain_console_by_id(console_owner); } +domid_t console_owner_domid(void) +{ + return console_owner; +} + static bool console_owner_possible(domid_t domid) { struct domain *d; diff --git a/xen/include/xen/console.h b/xen/include/xen/console.h index 57c482cfbf2da15b011e64841ea086e779f4588d..83be5794aff6630beaad46f910fcc0fc6d833808 100644 --- a/xen/include/xen/console.h +++ b/xen/include/xen/console.h @@ -33,6 +33,7 @@ void console_end_log_everything(void); struct domain *rcu_lock_domain_console_owner(void); int console_set_owner(domid_t); +domid_t console_owner_domid(void); /* * Steal output from the console. Returns +ve identifier, else -ve error.