1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 0/9] RISC-V Guest Support
  4. View patch

[libvirt] [PATCH 1/9] conf: add an option to specify a NS16550A serial port

Lubomir Rintel posted 9 patches 6 years, 12 months ago
Only 8 patches received!
There is a newer version of this series
[libvirt] [PATCH 1/9] conf: add an option to specify a NS16550A serial port
Posted by Lubomir Rintel 6 years, 12 months ago 
QEMU attaches this to a riscv/virt board. It's perhaps the same thing as
used on x86/pc, somewhat unfortunately called model="isa-serial".

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 docs/schemas/domaincommon.rng | 1 +
 src/conf/domain_conf.c        | 1 +
 src/conf/domain_conf.h        | 1 +
 src/qemu/qemu_command.c       | 2 ++
 src/qemu/qemu_domain.c        | 5 ++++-
 5 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index ff539607cc..47d4ee28ff 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3631,6 +3631,7 @@
           <value>pci-serial</value>
           <value>spapr-vty</value>
           <value>pl011</value>
+          <value>ns16550a</value>
           <value>sclpconsole</value>
           <value>sclplmconsole</value>
         </choice>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d38a775f21..119a074a0a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -486,6 +486,7 @@ VIR_ENUM_IMPL(virDomainChrSerialTargetModel,
               "pci-serial",
               "spapr-vty",
               "pl011",
+              "ns16550a",
               "sclpconsole",
               "sclplmconsole",
 );
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 07d04fb2f9..c00293a07b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1125,6 +1125,7 @@ typedef enum {
     VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_PCI_SERIAL,
     VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_SPAPR_VTY,
     VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_PL011,
+    VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_NS16550A,
     VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_SCLPCONSOLE,
     VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_SCLPLMCONSOLE,
 
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0727361f19..f91d3ae5ac 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8909,6 +8909,7 @@ qemuChrSerialTargetModelToCaps(virDomainChrSerialTargetModel targetModel)
         return QEMU_CAPS_DEVICE_SCLPLMCONSOLE;
     case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_PL011:
         return QEMU_CAPS_DEVICE_PL011;
+    case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_NS16550A:
     case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_NONE:
     case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_LAST:
         break;
@@ -10200,6 +10201,7 @@ qemuBuildSerialChrDeviceStr(char **deviceStr,
         break;
 
     case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_PL011:
+    case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_NS16550A:
     case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_NONE:
     case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_LAST:
         /* Except from _LAST, which is just a guard value and will never
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8d0ab9788f..97633bfbb9 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3724,6 +3724,7 @@ qemuDomainChrSerialTargetModelToTargetType(int targetModel)
     case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_SPAPR_VTY:
         return VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR_VIO;
     case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_PL011:
+    case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_NS16550A:
         return VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SYSTEM;
     case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_SCLPCONSOLE:
     case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_SCLPLMCONSOLE:
@@ -3787,6 +3788,7 @@ qemuDomainChrTargetDefValidate(const virDomainChrDef *chr)
         case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_PCI_SERIAL:
         case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_SPAPR_VTY:
         case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_PL011:
+        case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_NS16550A:
         case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_SCLPCONSOLE:
         case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_SCLPLMCONSOLE:
 
@@ -3847,7 +3849,8 @@ qemuDomainChrDefValidate(const virDomainChrDef *dev,
 
         if (!qemuDomainIsVirt(def) &&
             (dev->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SYSTEM ||
-             dev->targetModel == VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_PL011)) {
+             dev->targetModel == VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_PL011 ||
+             dev->targetModel == VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_NS16550A)) {
             isCompatible = false;
         }
 
-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/9] conf: add an option to specify a NS16550A serial port
Posted by Andrea Bolognani 6 years, 12 months ago 
On Tue, 2018-05-15 at 12:53 +0200, Lubomir Rintel wrote:
> QEMU attaches this to a riscv/virt board. It's perhaps the same thing as
> used on x86/pc, somewhat unfortunately called model="isa-serial".
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
>  docs/schemas/domaincommon.rng | 1 +
>  src/conf/domain_conf.c        | 1 +
>  src/conf/domain_conf.h        | 1 +
>  src/qemu/qemu_command.c       | 2 ++
>  src/qemu/qemu_domain.c        | 5 ++++-
>  5 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index ff539607cc..47d4ee28ff 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3631,6 +3631,7 @@
>            <value>pci-serial</value>
>            <value>spapr-vty</value>
>            <value>pl011</value>
> +          <value>ns16550a</value>
>            <value>sclpconsole</value>
>            <value>sclplmconsole</value>
>          </choice>

Usually, when adding new items to an existing list, we append them
rather than inserting them somewhere in the middle.

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index d38a775f21..119a074a0a 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -486,6 +486,7 @@ VIR_ENUM_IMPL(virDomainChrSerialTargetModel,
>                "pci-serial",
>                "spapr-vty",
>                "pl011",
> +              "ns16550a",
>                "sclpconsole",
>                "sclplmconsole",
>  );

Same here.

> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 07d04fb2f9..c00293a07b 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1125,6 +1125,7 @@ typedef enum {
>      VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_PCI_SERIAL,
>      VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_SPAPR_VTY,
>      VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_PL011,
> +    VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_NS16550A,
>      VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_SCLPCONSOLE,
>      VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_SCLPLMCONSOLE,

And of course here.

[...]
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 8d0ab9788f..97633bfbb9 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3724,6 +3724,7 @@ qemuDomainChrSerialTargetModelToTargetType(int targetModel)
>      case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_SPAPR_VTY:
>          return VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR_VIO;
>      case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_PL011:
> +    case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_NS16550A:
>          return VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SYSTEM;
>      case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_SCLPCONSOLE:
>      case VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_SCLPLMCONSOLE:

I'm not sure whether we want to reuse TARGET_TYPE_SYSTEM here, or
whether we need to introduce a new target type specific to RISC-V.

Last time we added target types, we asked QEMU developers for input
about the naming. I think it's a good idea to do that again.

> @@ -3847,7 +3849,8 @@ qemuDomainChrDefValidate(const virDomainChrDef *dev,
>  
>          if (!qemuDomainIsVirt(def) &&
>              (dev->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SYSTEM ||
> -             dev->targetModel == VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_PL011)) {
> +             dev->targetModel == VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_PL011 ||
> +             dev->targetModel == VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_NS16550A)) {
>              isCompatible = false;
>          }

This is wrong, as it would allow users to add ns16550a serial
consoles to aarch64 virt guests, which is clearly an incorrect
configuration. More on this later.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.