[libvirt] [PATCH 4/9] util: add RISC-V support

Lubomir Rintel posted 9 patches 6 years, 12 months ago
Only 8 patches received!
There is a newer version of this series
[libvirt] [PATCH 4/9] util: add RISC-V support
Posted by Lubomir Rintel 6 years, 12 months ago
What works, with images from [1]:

  virt-install \
    --import --name riscv64 \
    --arch riscv64 --machine virt \
    --memory 2048 \
    --rng /dev/urandom \
    --disk /var/lib/libvirt/images/stage4-disk.img,bus=virtio  \
    --boot kernel=/var/lib/libvirt/images/bbl,kernel_args="console=ttyS0 ro root=/dev/vda"

[1] https://fedorapeople.org/groups/risc-v/disk-images/

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 docs/schemas/basictypes.rng    |  2 ++
 src/qemu/qemu_command.c        | 15 ++++++++-------
 src/qemu/qemu_domain.c         | 18 +++++++++++++++---
 src/qemu/qemu_domain_address.c | 21 +++++++++++----------
 src/util/virarch.c             |  3 +++
 src/util/virarch.h             |  6 ++++++
 6 files changed, 45 insertions(+), 20 deletions(-)

diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index 1a18cd31b1..14a3670e5c 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -398,6 +398,8 @@
       <value>ppc64</value>
       <value>ppc64le</value>
       <value>ppcemb</value>
+      <value>riscv32</value>
+      <value>riscv64</value>
       <value>s390</value>
       <value>s390x</value>
       <value>sh4</value>
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f91d3ae5ac..12f90521d4 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8940,14 +8940,15 @@ static bool
 qemuChrIsPlatformDevice(const virDomainDef *def,
                         virDomainChrDefPtr chr)
 {
-    if (def->os.arch == VIR_ARCH_ARMV7L || def->os.arch == VIR_ARCH_AARCH64) {
-
-        /* pl011 (used on mach-virt) is a platform device */
-        if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
-            chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SYSTEM &&
-            chr->targetModel == VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_PL011) {
+    if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
+        chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SYSTEM) {
+        if ((def->os.arch == VIR_ARCH_ARMV7L || def->os.arch == VIR_ARCH_AARCH64) &&
+            chr->targetModel == VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_PL011)
+            return true;
+        if (ARCH_IS_RISCV(def->os.arch) &&
+            qemuDomainIsVirt(def) &&
+            chr->targetModel == VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_NS16550A)
             return true;
-        }
     }
 
     /* If we got all the way here and we're still stuck with the default
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index db73f45204..20d05da9c4 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2914,6 +2914,14 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
     case VIR_ARCH_PARISC:
     case VIR_ARCH_PARISC64:
     case VIR_ARCH_PPCLE:
+        break;
+
+    case VIR_ARCH_RISCV32:
+    case VIR_ARCH_RISCV64:
+        addDefaultUSB = false;
+        addDefaultMemballoon = false;
+        break;
+
     case VIR_ARCH_UNICORE32:
     case VIR_ARCH_XTENSA:
     case VIR_ARCH_XTENSAEB:
@@ -5138,7 +5146,7 @@ static const char *
 qemuDomainDefaultNetModel(const virDomainDef *def,
                           virQEMUCapsPtr qemuCaps)
 {
-    if (ARCH_IS_S390(def->os.arch))
+    if (ARCH_IS_S390(def->os.arch) || ARCH_IS_RISCV(def->os.arch))
         return "virtio";
 
     if (def->os.arch == VIR_ARCH_ARMV7L ||
@@ -5470,7 +5478,10 @@ qemuDomainChrDefPostParse(virDomainChrDefPtr chr,
             chr->targetModel = VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_SPAPR_VTY;
             break;
         case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SYSTEM:
-            chr->targetModel = VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_PL011;
+            if (ARCH_IS_RISCV(def->os.arch))
+                chr->targetModel = VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_NS16550A;
+            else
+                chr->targetModel = VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_PL011;
             break;
         case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLP:
             chr->targetModel = VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_SCLPCONSOLE;
@@ -8449,7 +8460,8 @@ qemuDomainMachineIsVirt(const char *machine,
                         const virArch arch)
 {
     if (arch != VIR_ARCH_ARMV7L &&
-        arch != VIR_ARCH_AARCH64)
+        arch != VIR_ARCH_AARCH64 &&
+        !ARCH_IS_RISCV(arch))
         return false;
 
     if (STRNEQ(machine, "virt") &&
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 87e1dc3bd2..5d9a073d25 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -480,13 +480,13 @@ static void
 qemuDomainAssignVirtioMMIOAddresses(virDomainDefPtr def,
                                     virQEMUCapsPtr qemuCaps)
 {
-    if (def->os.arch != VIR_ARCH_ARMV7L &&
-        def->os.arch != VIR_ARCH_AARCH64)
-        return;
-
-    if (!(STRPREFIX(def->os.machine, "vexpress-") ||
-          qemuDomainIsVirt(def)))
+    if (def->os.arch == VIR_ARCH_ARMV7L || def->os.arch == VIR_ARCH_AARCH64) {
+        if (!(STRPREFIX(def->os.machine, "vexpress-") ||
+              qemuDomainIsVirt(def)))
+            return;
+    } else if (!ARCH_IS_RISCV(def->os.arch)) {
         return;
+    }
 
     /* We use virtio-mmio by default on mach-virt guests only if they already
      * have at least one virtio-mmio device: in all other cases, we prefer
@@ -2163,11 +2163,12 @@ static bool
 qemuDomainSupportsPCI(virDomainDefPtr def,
                       virQEMUCapsPtr qemuCaps)
 {
-    if ((def->os.arch != VIR_ARCH_ARMV7L) && (def->os.arch != VIR_ARCH_AARCH64))
-        return true;
-
-    if (STREQ(def->os.machine, "versatilepb"))
+    if (def->os.arch == VIR_ARCH_ARMV7L) {
+        if (STREQ(def->os.machine, "versatilepb"))
+            return true;
+    } else if (!(def->os.arch != VIR_ARCH_AARCH64 || ARCH_IS_RISCV(def->os.arch))) {
         return true;
+    }
 
     if (qemuDomainIsVirt(def) &&
         virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_GPEX))
diff --git a/src/util/virarch.c b/src/util/virarch.c
index be48bcfb89..77b893b0ac 100644
--- a/src/util/virarch.c
+++ b/src/util/virarch.c
@@ -65,6 +65,9 @@ static const struct virArchData {
     { "ppc64le",      64, VIR_ARCH_LITTLE_ENDIAN },
     { "ppcemb",       32, VIR_ARCH_BIG_ENDIAN },
 
+    { "riscv32",      32, VIR_ARCH_LITTLE_ENDIAN },
+    { "riscv64",      64, VIR_ARCH_LITTLE_ENDIAN },
+
     { "s390",         32, VIR_ARCH_BIG_ENDIAN },
     { "s390x",        64, VIR_ARCH_BIG_ENDIAN },
     { "sh4",          32, VIR_ARCH_LITTLE_ENDIAN },
diff --git a/src/util/virarch.h b/src/util/virarch.h
index af5ff83528..806a23fade 100644
--- a/src/util/virarch.h
+++ b/src/util/virarch.h
@@ -55,6 +55,9 @@ typedef enum {
     VIR_ARCH_PPC64LE,      /* PowerPC     64 LE http://en.wikipedia.org/wiki/PowerPC */
     VIR_ARCH_PPCEMB,       /* PowerPC     32 BE http://en.wikipedia.org/wiki/PowerPC */
 
+    VIR_ARCH_RISCV32,      /* RISC-V     64 LE http://en.wikipedia.org/wiki/RISC-V */
+    VIR_ARCH_RISCV64,      /* RISC-V     32 BE http://en.wikipedia.org/wiki/RISC-V */
+
     VIR_ARCH_S390,         /* S390        32 BE http://en.wikipedia.org/wiki/S390 */
     VIR_ARCH_S390X,        /* S390        64 BE http://en.wikipedia.org/wiki/S390x */
     VIR_ARCH_SH4,          /* SuperH4     32 LE http://en.wikipedia.org/wiki/SuperH */
@@ -87,6 +90,9 @@ typedef enum {
                              (arch) == VIR_ARCH_ARMV7B ||\
                              (arch) == VIR_ARCH_AARCH64)
 
+# define ARCH_IS_RISCV(arch) ((arch) == VIR_ARCH_RISCV32 ||\
+                              (arch) == VIR_ARCH_RISCV64)
+
 # define ARCH_IS_S390(arch) ((arch) == VIR_ARCH_S390 ||\
                              (arch) == VIR_ARCH_S390X)
 
-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/9] util: add RISC-V support
Posted by Andrea Bolognani 6 years, 12 months ago
On Tue, 2018-05-15 at 12:53 +0200, Lubomir Rintel wrote:
> What works, with images from [1]:
> 
>   virt-install \
>     --import --name riscv64 \
>     --arch riscv64 --machine virt \
>     --memory 2048 \
>     --rng /dev/urandom \
>     --disk /var/lib/libvirt/images/stage4-disk.img,bus=virtio  \
>     --boot kernel=/var/lib/libvirt/images/bbl,kernel_args="console=ttyS0 ro root=/dev/vda"
> 
> [1] https://fedorapeople.org/groups/risc-v/disk-images/
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
>  docs/schemas/basictypes.rng    |  2 ++
>  src/qemu/qemu_command.c        | 15 ++++++++-------
>  src/qemu/qemu_domain.c         | 18 +++++++++++++++---
>  src/qemu/qemu_domain_address.c | 21 +++++++++++----------
>  src/util/virarch.c             |  3 +++
>  src/util/virarch.h             |  6 ++++++
>  6 files changed, 45 insertions(+), 20 deletions(-)

This patch breaks 'make check'. Please make sure that both 'make
check' and 'make syntax-check' pass after every single patch in
the series - it's a requirement for getting it merged.

I'll wait for a respin that addresses that before moving forward
with the review, but I'd like to make a few comments on the series
as a whole.

The way you've organized your changes could use some improvements:
for example, in this patch you're both teaching libvirt about the
RISC-V architecture and changing guest behavior, which is basically
two or more changes squashed into a single patch; moreover, I would
expect the former to happen fairly early in the series, certainly
before introducing a RISC-V specific serial device.

Squashing patches together is easier than disentangling unrelated
changes, so when in doubt always err on the side of small patches ;)

Overall, I would expect the series to be organized along the
lines of

  * preparatory work, such as creating a wrapper around
    qemuDomainAssignARMVirtioMMIOAddresses();

  * teaching libvirt about the RISC-V architecture;

  * teaching libvirt about the new serial console model (and
    potentially target type);

  * putting it all together;

  * updating the test suite.

I'm not saying you have to follow this to the letter, and more
specifically some changes to the test suite might need to happen
at the same time as you add RISC-V support, but overall it should
end up looking similar to that.

Looking forward to v2 :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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