virtio/serial: initial CONSOLE support #12760

yf13 · 2024-07-24T08:28:31Z

Summary

This adds initial support to allow virtio serial device being used as console on QEMU rv-virt device. It includes the following changes:

virtio/serial: add VIRTIO_SERIAL_CONSOLE Kconfig and support.
qemu-rv_start: guard u16550 uses with HAVE_16550_CONSOLE.
rv-virt_appinit: probe virtio devices upon BOARD_EARLY_INIITIALIZE

Impacts

None

Testing

Local checks on rv-virt
CI checks

drivers/virtio/Kconfig

acassis · 2024-07-24T10:02:22Z

@yf13 please include Documentation/

yf13 · 2024-07-24T12:23:31Z

@yf13 please include Documentation/

@acassis, yes will do that after a concrete config is available, so far I want take this chance to review the basic changes and make sure I use virtio serial driver correctly.

yf13 · 2024-07-24T13:25:15Z

@acassis, it seems that I've found the way to use virtio-serial as normal console now, so it is no longer output only. I still need time to clean up the conf and then update the rv-virt document later.

acassis · 2024-07-24T14:57:10Z

@acassis, it seems that I've found the way to use virtio-serial as normal console now, so it is no longer output only. I still need time to clean up the conf and then update the rv-virt document later.

@yf13 are you using busy await reading (like the LWL console) ? We need to find a way to avoid it, because it eats too much processing time.

acassis · 2024-07-24T19:25:24Z

@yf13 suggestion: instead nsh_aux why don't you use virt_nsh ?

arch/risc-v/src/qemu-rv/qemu_rv_start.c

drivers/virtio/virtio-serial.c

yf13 · 2024-07-25T01:07:28Z

@acassis config renamed to virt_nsh

But it seems imx9-sdimage.img has been leaked from imx93-evk:bootloader? I saw .gitignore already contains a line for that file, not sure how to resolve the CI issue.

yf13 · 2024-07-25T02:15:18Z

@anchao, thanks for your comments!

I am wondering that serial devices like uart_16550, uart_pl011, serial_rtt, cdcacm are quite standard, thus there is no need to treat them as arch specific devices? I feel virtio-serial is also quite standard, thus we can treat it more like uart_16550?

anchao · 2024-07-25T03:07:45Z

@anchao, thanks for your comments!

I am wondering that serial devices like uart_16550, uart_pl011, serial_rtt, cdcacm are quite standard, thus there is no need to treat them as arch specific devices? I feel virtio-serial is also quite standard, thus we can treat it more like uart_16550?

@yf13 Your changes are correct, I missed that the tty device is also registered in this driver, please ignore my comments

drivers/virtio/Kconfig

drivers/virtio/virtio-serial.c

yf13 · 2024-07-25T04:18:44Z

@anchao, thanks for all the in-line comments! I've updated them,

CV-Bowen · 2024-07-25T11:41:49Z

@yf13 #12764 Hi, how about support custom the virtio serial device name, and register the console based on the device name (/dev/console).

This adds DRIVERS_VIRTIO_SERIAL_CONSOLE config and related logic to virtio serial so that it can be used as console device. Note that due to its dependency on OS services, this console is available late so it is not proper for debugging too early booting issues. Signed-off-by: Yanfeng Liu <[email protected]>

This guards the uses for u16550 serial initialization, just in case that no u16550 is configured. Signed-off-by: Yanfeng Liu <[email protected]>

This allows virtio devices to be probed upon board_early_init, thus making virito-serial ready earlier for console use. Signed-off-by: Yanfeng Liu <[email protected]>

xiaoxiang781216 · 2024-07-25T14:03:15Z

drivers/virtio/virtio-serial.c

+#ifdef CONFIG_DRIVERS_VIRTIO_SERIAL_CONSOLE
+  if (g_virtio_console == NULL)
+    {
+      DEBUGVERIFY(uart_register("/dev/console", &priv->udev));


it's better to drop this change and utilize this general approach: #12764

xiaoxiang781216

let's reuse #12764

anchao · 2024-07-25T14:48:37Z

These two commits are completely different features, why drop this PR reuse #12764? #12764 was committed as early as "9 Jan 2024", why not submit it earlier ? We need to respect the work of every independent developer, not deny it.

acassis · 2024-07-25T14:54:23Z

These two commits are completely different features, why drop this PR reuse #12764? #12764 was committed as early as "9 Jan 2024", why not submit it earlier ? We need to respect the work of every independent developer, not deny it.

You are 100% right! "Release early, release often!"

I think we need to respect individual contributor and respect the order the PRs are created.

This one here has priority, please apply this here and ask your developer to adapt his PR later

acassis · 2024-07-25T18:23:06Z

@yf13 please take a look:

====================================================================================
Configuration/Tool: rv-virt/ksmp64
2024-07-25 16:00:35
------------------------------------------------------------------------------------
  Cleaning...
HEAD detached at pull/12760/merge
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   boards/risc-v/qemu-rv/rv-virt/configs/virt_nsh/defconfig

no changes added to commit (use "git add" and/or "git commit -a")
  Configuring...
  Building NuttX...
/github/workspace/sources/apps /github/workspace/sources/nuttx
##[debug]Dropping file value '/home/runner/work/nuttx/nuttx/fpu.c'. Path does not exist
Warning: fpu.c:55:8: warning: #warning "FPU test not built; Only available in the flat build (CONFIG_BUILD_FLAT)" [-Wcpp]
   55 | #      warning "FPU test not built; Only available in the flat build (CONFIG_BUILD_FLAT)"
      |        ^~~~~~~
/github/workspace/sources/nuttx
  Normalize rv-virt/ksmp64

acassis · 2024-07-25T18:23:35Z

##[debug]Dropping file value '/home/runner/work/nuttx/nuttx/fpu.c'. Path does not exist

This adds `rv-virt:virt_nsh` config w/ docs for using virtio serial as NuttX console. Signed-off-by: Yanfeng Liu <[email protected]>

yf13 · 2024-07-26T00:10:12Z

@acassis, thanks for reminding. It looks fpu.c warning is fine. It was my bad forgetting tools/refresh.sh the virt_nsh conf after taking @anchao's suggestion.

yf13 force-pushed the virtio branch 2 times, most recently from 23b6632 to 6fde32a Compare July 24, 2024 09:33

acassis reviewed Jul 24, 2024

View reviewed changes

drivers/virtio/Kconfig Outdated Show resolved Hide resolved

yf13 force-pushed the virtio branch from 6fde32a to 0701fee Compare July 24, 2024 10:32

yf13 force-pushed the virtio branch from 0701fee to ec3b96c Compare July 24, 2024 13:35

yf13 force-pushed the virtio branch from ec3b96c to 6d70681 Compare July 24, 2024 18:23

yf13 force-pushed the virtio branch from 1cd9cf5 to f70ee0c Compare July 25, 2024 00:42

anchao reviewed Jul 25, 2024

View reviewed changes

arch/risc-v/src/qemu-rv/qemu_rv_start.c Outdated Show resolved Hide resolved

drivers/virtio/virtio-serial.c Outdated Show resolved Hide resolved

drivers/virtio/virtio-serial.c Outdated Show resolved Hide resolved

anchao reviewed Jul 25, 2024

View reviewed changes

yf13 force-pushed the virtio branch 2 times, most recently from 5924193 to 6dbe79a Compare July 25, 2024 04:17

anchao requested a review from xiaoxiang781216 July 25, 2024 05:14

yf13 force-pushed the virtio branch from 6dbe79a to 1ef61b3 Compare July 25, 2024 11:15

acassis approved these changes Jul 25, 2024

View reviewed changes

yf13 added 3 commits July 25, 2024 21:31

riscv/qemu: guard u16550 uses

74d5f4a

This guards the uses for u16550 serial initialization, just in case that no u16550 is configured. Signed-off-by: Yanfeng Liu <[email protected]>

riscv/virt: probe virtio earlier

a49da62

This allows virtio devices to be probed upon board_early_init, thus making virito-serial ready earlier for console use. Signed-off-by: Yanfeng Liu <[email protected]>

yf13 force-pushed the virtio branch from 1ef61b3 to 2caf398 Compare July 25, 2024 13:32

xiaoxiang781216 reviewed Jul 25, 2024

View reviewed changes

xiaoxiang781216 requested changes Jul 25, 2024

View reviewed changes

anchao approved these changes Jul 25, 2024

View reviewed changes

acassis mentioned this pull request Jul 25, 2024

virtio-serial: support custom the virtio serial device name #12764

Closed

acassis requested a review from xiaoxiang781216 July 25, 2024 16:04

riscv/virt: add virt_nsh with docs

667ef8f

This adds `rv-virt:virt_nsh` config w/ docs for using virtio serial as NuttX console. Signed-off-by: Yanfeng Liu <[email protected]>

yf13 force-pushed the virtio branch from c490b01 to 667ef8f Compare July 26, 2024 00:08

xiaoxiang781216 approved these changes Jul 26, 2024

View reviewed changes

xiaoxiang781216 merged commit 4038abd into apache:master Jul 26, 2024
26 checks passed

yf13 deleted the virtio branch August 1, 2024 11:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

virtio/serial: initial CONSOLE support #12760

virtio/serial: initial CONSOLE support #12760

yf13 commented Jul 24, 2024

acassis commented Jul 24, 2024

yf13 commented Jul 24, 2024

yf13 commented Jul 24, 2024 •

edited

Loading

acassis commented Jul 24, 2024

acassis commented Jul 24, 2024

yf13 commented Jul 25, 2024 •

edited

Loading

yf13 commented Jul 25, 2024

anchao commented Jul 25, 2024

yf13 commented Jul 25, 2024

CV-Bowen commented Jul 25, 2024

xiaoxiang781216 Jul 25, 2024

xiaoxiang781216 left a comment

anchao commented Jul 25, 2024

acassis commented Jul 25, 2024

acassis commented Jul 25, 2024

acassis commented Jul 25, 2024

yf13 commented Jul 26, 2024

virtio/serial: initial CONSOLE support #12760

virtio/serial: initial CONSOLE support #12760

Conversation

yf13 commented Jul 24, 2024

Summary

Impacts

Testing

acassis commented Jul 24, 2024

yf13 commented Jul 24, 2024

yf13 commented Jul 24, 2024 • edited Loading

acassis commented Jul 24, 2024

acassis commented Jul 24, 2024

yf13 commented Jul 25, 2024 • edited Loading

yf13 commented Jul 25, 2024

anchao commented Jul 25, 2024

yf13 commented Jul 25, 2024

CV-Bowen commented Jul 25, 2024

xiaoxiang781216 Jul 25, 2024

Choose a reason for hiding this comment

xiaoxiang781216 left a comment

Choose a reason for hiding this comment

anchao commented Jul 25, 2024

acassis commented Jul 25, 2024

acassis commented Jul 25, 2024

acassis commented Jul 25, 2024

yf13 commented Jul 26, 2024

yf13 commented Jul 24, 2024 •

edited

Loading

yf13 commented Jul 25, 2024 •

edited

Loading