| Message ID | 20241205-vuart-ns8250-v1-34-e9aa923127eb@ford.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce NS8250 UART emulator | expand |
On 2024-12-05 23:42, Denis Mukhin via B4 Relay wrote: > From: Denis Mukhin <dmukhin@ford.com> > > Enable console focus for domains w/ virtual NS8250. > > Code change allows to capture the output from the guest OS now and send it to > the physical console device. > > Signed-off-by: Denis Mukhin <dmukhin@ford.com> > --- > xen/drivers/char/console.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c > index a26daee9c4c4b1134d0ae3d105ffdb656340b6df..798dfdf3412a2feef35e72946d6c59bee59a9251 100644 > --- a/xen/drivers/char/console.c > +++ b/xen/drivers/char/console.c > @@ -41,6 +41,9 @@ > #ifdef CONFIG_SBSA_VUART_CONSOLE > #include <asm/vpl011.h> > #endif > +#if defined(CONFIG_HAS_VUART_NS8250) > +#include <asm/hvm/vuart_ns8250.h> > +#endif > > /* console: comma-separated list of console outputs. */ > static char __initdata opt_console[30] = OPT_CONSOLE_STR; > @@ -627,6 +630,8 @@ static void handle_keypress_in_domain(struct domain *d, char c) > { > #if defined(CONFIG_SBSA_VUART_CONSOLE) > rc = vpl011_rx_char_xen(d, c); > +#elif defined(CONFIG_HAS_VUART_NS8250) > + rc = vuart_putchar(&d->arch.hvm.vuart, c); > #endif I think it would be nicer to just use a single name and avoid ifdef-ery. vuart_putchar() is generic and matches domain_has_vuart(), so that seems good. You can then have a default stub that returns -ENODEV for when an implementation is not built. (This goes along with Jan's suggestion of a common, default domain_has_vuart().) Something like: #ifndef vuart_putchar static inline int vuart_putchar(struct domain *d, char c) { return -ENODEV; } #define vuart_putchar vuart_putchar #endif and ARM can do: #define vuart_putchar vpl011_rx_char_xen x86 would need to change its arguments, but that should be straight forward. What do you think? Regards, Jason
On 10.12.2024 23:46, Jason Andryuk wrote: > On 2024-12-05 23:42, Denis Mukhin via B4 Relay wrote: >> From: Denis Mukhin <dmukhin@ford.com> >> >> Enable console focus for domains w/ virtual NS8250. >> >> Code change allows to capture the output from the guest OS now and send it to >> the physical console device. >> >> Signed-off-by: Denis Mukhin <dmukhin@ford.com> >> --- >> xen/drivers/char/console.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c >> index a26daee9c4c4b1134d0ae3d105ffdb656340b6df..798dfdf3412a2feef35e72946d6c59bee59a9251 100644 >> --- a/xen/drivers/char/console.c >> +++ b/xen/drivers/char/console.c >> @@ -41,6 +41,9 @@ >> #ifdef CONFIG_SBSA_VUART_CONSOLE >> #include <asm/vpl011.h> >> #endif >> +#if defined(CONFIG_HAS_VUART_NS8250) >> +#include <asm/hvm/vuart_ns8250.h> >> +#endif >> >> /* console: comma-separated list of console outputs. */ >> static char __initdata opt_console[30] = OPT_CONSOLE_STR; >> @@ -627,6 +630,8 @@ static void handle_keypress_in_domain(struct domain *d, char c) >> { >> #if defined(CONFIG_SBSA_VUART_CONSOLE) >> rc = vpl011_rx_char_xen(d, c); >> +#elif defined(CONFIG_HAS_VUART_NS8250) >> + rc = vuart_putchar(&d->arch.hvm.vuart, c); >> #endif > > I think it would be nicer to just use a single name and avoid ifdef-ery. > vuart_putchar() is generic and matches domain_has_vuart(), so that > seems good. > > You can then have a default stub that returns -ENODEV for when an > implementation is not built. (This goes along with Jan's suggestion of > a common, default domain_has_vuart().) Something like: > > #ifndef vuart_putchar > static inline int vuart_putchar(struct domain *d, char c) { > return -ENODEV; > } > #define vuart_putchar vuart_putchar > #endif > > and ARM can do: > #define vuart_putchar vpl011_rx_char_xen +1 Jan
On 10.12.2024 23:46, Jason Andryuk wrote: > On 2024-12-05 23:42, Denis Mukhin via B4 Relay wrote: >> From: Denis Mukhin <dmukhin@ford.com> >> >> Enable console focus for domains w/ virtual NS8250. >> >> Code change allows to capture the output from the guest OS now and send it to >> the physical console device. >> >> Signed-off-by: Denis Mukhin <dmukhin@ford.com> >> --- >> xen/drivers/char/console.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c >> index a26daee9c4c4b1134d0ae3d105ffdb656340b6df..798dfdf3412a2feef35e72946d6c59bee59a9251 100644 >> --- a/xen/drivers/char/console.c >> +++ b/xen/drivers/char/console.c >> @@ -41,6 +41,9 @@ >> #ifdef CONFIG_SBSA_VUART_CONSOLE >> #include <asm/vpl011.h> >> #endif >> +#if defined(CONFIG_HAS_VUART_NS8250) >> +#include <asm/hvm/vuart_ns8250.h> >> +#endif >> >> /* console: comma-separated list of console outputs. */ >> static char __initdata opt_console[30] = OPT_CONSOLE_STR; >> @@ -627,6 +630,8 @@ static void handle_keypress_in_domain(struct domain *d, char c) >> { >> #if defined(CONFIG_SBSA_VUART_CONSOLE) >> rc = vpl011_rx_char_xen(d, c); >> +#elif defined(CONFIG_HAS_VUART_NS8250) >> + rc = vuart_putchar(&d->arch.hvm.vuart, c); >> #endif > > I think it would be nicer to just use a single name and avoid ifdef-ery. > vuart_putchar() is generic and matches domain_has_vuart(), so that > seems good. > > You can then have a default stub that returns -ENODEV for when an > implementation is not built. (This goes along with Jan's suggestion of > a common, default domain_has_vuart().) Something like: > > #ifndef vuart_putchar > static inline int vuart_putchar(struct domain *d, char c) { > return -ENODEV; > } > #define vuart_putchar vuart_putchar > #endif > > and ARM can do: > #define vuart_putchar vpl011_rx_char_xen > > x86 would need to change its arguments, but that should be straight forward. Actually, I don't even see a need for the stub: { #ifdef vuart_putchar rc = vuart_putchar(d, c); #endif } This way behavior won't change from what there is now, when vuart_putchar() isn't defined. Jan
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c index a26daee9c4c4b1134d0ae3d105ffdb656340b6df..798dfdf3412a2feef35e72946d6c59bee59a9251 100644 --- a/xen/drivers/char/console.c +++ b/xen/drivers/char/console.c @@ -41,6 +41,9 @@ #ifdef CONFIG_SBSA_VUART_CONSOLE #include <asm/vpl011.h> #endif +#if defined(CONFIG_HAS_VUART_NS8250) +#include <asm/hvm/vuart_ns8250.h> +#endif /* console: comma-separated list of console outputs. */ static char __initdata opt_console[30] = OPT_CONSOLE_STR; @@ -627,6 +630,8 @@ static void handle_keypress_in_domain(struct domain *d, char c) { #if defined(CONFIG_SBSA_VUART_CONSOLE) rc = vpl011_rx_char_xen(d, c); +#elif defined(CONFIG_HAS_VUART_NS8250) + rc = vuart_putchar(&d->arch.hvm.vuart, c); #endif } /*