Hi Andrea :
On Wed, Jan 10, 2024 at 11:07:46AM +0800, Xianglai Li wrote:
From: xianglai li<lixianglai@xxxxxxxxxxx>
Please consider adjusting your git configuration so that the
authorship information matches your Signed-off-by and the email's
From header.
OK, I will fix it in the next version.
Add loongarch cpu support, Define new cpu type 'loongarch64'
and implement it's driver functions.
Signed-off-by: "Xianglai Li"<lixianglai@xxxxxxxxxxx>
---
src/cpu/cpu.c | 2 +
src/cpu/cpu_loongarch.c | 80 ++++++++++++++++++++++++++++++++++++
src/cpu/cpu_loongarch.h | 25 +++++++++++
src/cpu/meson.build | 1 +
src/qemu/qemu_capabilities.c | 2 +
src/qemu/qemu_domain.c | 4 ++
src/util/virarch.c | 2 +
src/util/virarch.h | 4 ++
8 files changed, 120 insertions(+)
create mode 100644 src/cpu/cpu_loongarch.c
create mode 100644 src/cpu/cpu_loongarch.h
diff --git a/src/cpu/cpu_loongarch.c b/src/cpu/cpu_loongarch.c
new file mode 100644
index 0000000000..48f9fef5ea
--- /dev/null
+++ b/src/cpu/cpu_loongarch.c
@@ -0,0 +1,80 @@
+/*
+ * cpu_loongarch.c: CPU driver for 64-bit LOONGARCH CPUs
+ *
+ * Copyright (C) 2024 Loongson Technology.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ *<http://www.gnu.org/licenses/>.
+ */
+
+#include <config.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <sys/time.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include "virlog.h"
+#include "viralloc.h"
+#include "cpu.h"
+#include "virstring.h"
+#include "cpu_map.h"
+#include "virbuffer.h"
Most of these includes are unnecessary. You only really need
<config.h>, "virlog.h" and "cpu.h".
OK, I will delete these unnecessary header files in the next version.
+#define VIR_FROM_THIS VIR_FROM_CPU
+
+VIR_LOG_INIT("cpu.cpu_loongarch");
+
+static const virArch archs[] = { VIR_ARCH_LOONGARCH64 };
+
+static virCPUCompareResult
+virCPULoongArchCompare(virCPUDef *host G_GNUC_UNUSED,
+ virCPUDef *cpu G_GNUC_UNUSED,
+ bool failIncompatible G_GNUC_UNUSED)
+{
+ return VIR_CPU_COMPARE_IDENTICAL;
+}
+
+static int
+virCPULoongArchUpdate(virCPUDef *guest,
+ const virCPUDef *host ATTRIBUTE_UNUSED,
G_GNUC_UNUSED
OK!
+ bool relative G_GNUC_UNUSED)
+{
+ /*
+ * - host-passthrough doesn't even get here
+ * - host-model is used for host CPU running in a compatibility mode and
+ * it needs to remain unchanged
+ * - custom doesn't support any optional features, there's nothing to
+ * update
+ */
This comment is lifted directly from the ppc64 CPU driver and it
feels out of place here, especially the part about "compatibility
mode", which is extremely ppc64 specific. I'd say just drop it.
OK!
+ if (guest->mode == VIR_CPU_MODE_CUSTOM)
+ guest->match = VIR_CPU_MATCH_EXACT;
This doesn't seem to be needed for a basic CPU driver. If you drop
it, this will end up looking exactly the way the RISC-V CPU driver
originally did, and that worked just fine until it was later
improved. So I'd say just drop it. We can add more features to the
CPU driver later.
OK!I will drop it in the next version.
diff --git a/src/util/virarch.h b/src/util/virarch.h
index 747f77c48e..c033e5c68d 100644
--- a/src/util/virarch.h
+++ b/src/util/virarch.h
@@ -36,6 +36,8 @@ typedef enum {
VIR_ARCH_ITANIUM, /* Itanium 64 LEhttps://en.wikipedia.org/wiki/Itanium */
VIR_ARCH_LM32, /* MilkyMist 32 BEhttps://en.wikipedia.org/wiki/Milkymist */
+ VIR_ARCH_LOONGARCH64, /* LoongArch 64 LE */
+
VIR_ARCH_M68K, /* m68k 32 BEhttps://en.wikipedia.org/wiki/Motorola_68000_family */
VIR_ARCH_MICROBLAZE, /* Microblaze 32 BEhttps://en.wikipedia.org/wiki/MicroBlaze */
VIR_ARCH_MICROBLAZEEL, /* Microblaze 32 LEhttps://en.wikipedia.org/wiki/MicroBlaze */
The entries here are grouped five by five, so as you add loongarch64
in the middle you're going to have to reorganize all the ones coming
after it. Same for the virarch.c part and preferredMachines in the
QEMU driver.
OK, I will rearrange all entries after loongarch64 in a 5-by-5 grouping
in the next version.
Same for the virarch.c part and preferredMachines in the QEMU driver.
There doesn't seem to be a Wikipedia entry for loongarch64 itself,
but perhaps it would be appropriate to link to
https://en.wikipedia.org/wiki/Loongson#LoongArch
instead. There are similar examples in the file already.
OK, I will modify the loongarch64 entry in the next patch release as
follows:
VIR_ARCH_LOONGARCH64, /* LoongArch 64 LEhttps://en.wikipedia.org/wiki/Loongson#LoongArch */
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 896aa8394f..0cea0b323a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4222,6 +4222,10 @@ qemuDomainDefAddDefaultDevices(virQEMUDriver *driver,
addPCIRoot = true;
break;
+ case VIR_ARCH_LOONGARCH64:
+ addPCIeRoot = true;
+ break;
+
case VIR_ARCH_ARMV7B:
case VIR_ARCH_CRIS:
case VIR_ARCH_ITANIUM:
In this commit you're just adding the architecture, and it's good
practice to touch the individual drivers as little as possible while
doing so.
So while you're forced to add a new case to the switch to keep things
building, leave out the actual logic for now and make it a no-action
entry, such as the existing ARMV7B, CRIS, etc.
In a later patch, when you actually implement loongarch64 support in
the QEMU driver, you can add this logic. When you do, please also set
addDefaultUSB = false;
addDefaultMemballoon = false;
The default behavior, which is to add these devices automatically for
all new domains, mainly exists to ensure backwards compatibility in
the context of x86, and we've moved away from it for architectures
that have been introduced more recently, such as aarch64 and RISC-V.
After I added the following logic to loongarch,
I found that creating a loongarch virtual machine no longer
creates a USB keyboard and mouse by default.
addDefaultUSB = false;
I tried to create the aarch64 virt virtual machine and found that it did not create a USB keyboard and mouse and
did not have spice graphics only serial port. In the case of aarch64,
I think it is reasonable to have no USB keyboard and mouse,
but loongarch64 can launch a graphical interface.
How do We control a virtual machine without a USB keyboard and mouse in the graphical interface?
I don't quite understand what's the point of adding it.
As above, with the following logic, the memory space in guest os will
not scale.
addDefaultMemballoon = false;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 83119e871a..f2339d6013 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2665,6 +2665,8 @@ static const char *preferredMachines[] =
NULL, /* VIR_ARCH_ITANIUM (doesn't exist in QEMU any more) */
"lm32-evr", /* VIR_ARCH_LM32 */
+ "virt", /* VIR_ARCH_LOONGARCH64 */
In this case too, it would be slightly nicer if you set this to NULL
as part of this patch and changed it to the actual value in the patch
where QEMU support is implemented.
OK!
Thanks,
Xianglai.
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
