From: xianglai li <lixianglai@loongson.cn>
Add loongarch cpu support, Define new cpu type 'loongarch64'
and implement it's driver functions.
Signed-off-by: "Xianglai Li" <lixianglai@loongson.cn>
---
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.c b/src/cpu/cpu.c
index bc43aa4e93..1e7c879ca5 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -27,6 +27,7 @@
#include "cpu_ppc64.h"
#include "cpu_s390.h"
#include "cpu_arm.h"
+#include "cpu_loongarch.h"
#include "cpu_riscv64.h"
#include "capabilities.h"
@@ -41,6 +42,7 @@ static struct cpuArchDriver *drivers[] = {
&cpuDriverS390,
&cpuDriverArm,
&cpuDriverRiscv64,
+ &cpuDriverLoongArch,
};
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"
+
+#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,
+ 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
+ */
+
+ if (guest->mode == VIR_CPU_MODE_CUSTOM)
+ guest->match = VIR_CPU_MATCH_EXACT;
+
+ return 0;
+}
+
+struct cpuArchDriver cpuDriverLoongArch = {
+ .name = "LoongArch",
+ .arch = archs,
+ .narch = G_N_ELEMENTS(archs),
+ .compare = virCPULoongArchCompare,
+ .decode = NULL,
+ .encode = NULL,
+ .dataFree = NULL,
+ .baseline = NULL,
+ .update = virCPULoongArchUpdate,
+ .getModels = NULL,
+};
diff --git a/src/cpu/cpu_loongarch.h b/src/cpu/cpu_loongarch.h
new file mode 100644
index 0000000000..4bc1c0cd8f
--- /dev/null
+++ b/src/cpu/cpu_loongarch.h
@@ -0,0 +1,25 @@
+/*
+ * cpu_loongarch.h: 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/>.
+ */
+
+#pragma once
+
+#include "cpu.h"
+
+extern struct cpuArchDriver cpuDriverLoongArch;
diff --git a/src/cpu/meson.build b/src/cpu/meson.build
index 55396903b9..141230e380 100644
--- a/src/cpu/meson.build
+++ b/src/cpu/meson.build
@@ -1,6 +1,7 @@
cpu_sources = [
'cpu.c',
'cpu_arm.c',
+ 'cpu_loongarch.c',
'cpu_map.c',
'cpu_ppc64.c',
'cpu_riscv64.c',
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 */
+
"mcf5208evb", /* VIR_ARCH_M68K */
"petalogix-s3adsp1800", /* VIR_ARCH_MICROBLAZE */
"petalogix-s3adsp1800", /* VIR_ARCH_MICROBLAZEEL */
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:
diff --git a/src/util/virarch.c b/src/util/virarch.c
index 01e520de73..74b7ec6f1b 100644
--- a/src/util/virarch.c
+++ b/src/util/virarch.c
@@ -51,6 +51,8 @@ static const struct virArchData {
{ "ia64", 64, VIR_ARCH_LITTLE_ENDIAN },
{ "lm32", 32, VIR_ARCH_BIG_ENDIAN },
+ { "loongarch64", 64, VIR_ARCH_LITTLE_ENDIAN },
+
{ "m68k", 32, VIR_ARCH_BIG_ENDIAN },
{ "microblaze", 32, VIR_ARCH_BIG_ENDIAN },
{ "microblazeel", 32, VIR_ARCH_LITTLE_ENDIAN},
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 LE https://en.wikipedia.org/wiki/Itanium */
VIR_ARCH_LM32, /* MilkyMist 32 BE https://en.wikipedia.org/wiki/Milkymist */
+ VIR_ARCH_LOONGARCH64, /* LoongArch 64 LE */
+
VIR_ARCH_M68K, /* m68k 32 BE https://en.wikipedia.org/wiki/Motorola_68000_family */
VIR_ARCH_MICROBLAZE, /* Microblaze 32 BE https://en.wikipedia.org/wiki/MicroBlaze */
VIR_ARCH_MICROBLAZEEL, /* Microblaze 32 LE https://en.wikipedia.org/wiki/MicroBlaze */
@@ -106,6 +108,8 @@ typedef enum {
#define ARCH_IS_SH4(arch) ((arch) == VIR_ARCH_SH4 ||\
(arch) == VIR_ARCH_SH4EB)
+#define ARCH_IS_LOONGARCH(arch) ((arch) == VIR_ARCH_LOONGARCH64)
+
typedef enum {
VIR_ARCH_LITTLE_ENDIAN,
VIR_ARCH_BIG_ENDIAN,
--
2.39.1
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
On Wed, Jan 10, 2024 at 11:07:46AM +0800, Xianglai Li wrote:
> From: xianglai li <lixianglai@loongson.cn>
Please consider adjusting your git configuration so that the
authorship information matches your Signed-off-by and the email's
From header.
> Add loongarch cpu support, Define new cpu type 'loongarch64'
> and implement it's driver functions.
>
> Signed-off-by: "Xianglai Li" <lixianglai@loongson.cn>
> ---
> 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".
> +#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
> + 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.
> + 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.
> 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 LE https://en.wikipedia.org/wiki/Itanium */
> VIR_ARCH_LM32, /* MilkyMist 32 BE https://en.wikipedia.org/wiki/Milkymist */
>
> + VIR_ARCH_LOONGARCH64, /* LoongArch 64 LE */
> +
> VIR_ARCH_M68K, /* m68k 32 BE https://en.wikipedia.org/wiki/Motorola_68000_family */
> VIR_ARCH_MICROBLAZE, /* Microblaze 32 BE https://en.wikipedia.org/wiki/MicroBlaze */
> VIR_ARCH_MICROBLAZEEL, /* Microblaze 32 LE https://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.
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.
> 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.
> 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.
--
Andrea Bolognani / Red Hat / Virtualization
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Hi Andrea :
> On Wed, Jan 10, 2024 at 11:07:46AM +0800, Xianglai Li wrote:
>> From: xianglai li<lixianglai@loongson.cn>
> 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@loongson.cn>
>> ---
>> 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@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
On Tue, Jan 30, 2024 at 05:00:43PM +0800, lixianglai wrote:
> > > +static int
> > > +virCPULoongArchUpdate(virCPUDef *guest,
> > > + const virCPUDef *host ATTRIBUTE_UNUSED,
> >
> > G_GNUC_UNUSED
>
> OK!
Please feel free to *not* explicitly acknowledge every single review
comment separately. Unless you disagree with the comment, or want to
clarify something, it is usually assumed that it has been heard and
will be acted upon in the next revision.
This also allows you to liberally drop big chunks of the original
message on reply, just like I've done here, and keep messages short
and focused :)
> > 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.
aarch64 and riscv64 VMs can also run a GUI just fine. That doesn't
mean that a USB controller should be present in every single VM.
> 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.
If you want a USB controller, you can just add one :)
Back when the libvirt project was started, only the x86 architecture
was really considered and I'm not even sure QEMU provided a way to
fine-tune the virtual hardware configuration to the point where
creating a VM with no USB controller was possible. So every VM would
get one.
Later on, the ability to opt out was implemented via the use of
<controller type='usb' model='none'/>
While that works, it is a bit clunky, and from the semantics point of
view it just feels wrong that you would need to *add* some XML
element to get libvirt to *remove* a device.
With that in mind, when introducing support for aarch64 and riscv64
the choice was made to flip the default around and build very minimal
VMs by default.
Note that virt-manager will still default to adding a bunch of
devices, including the USB controller, for new VMs, even for those
architectures. But at the libvirt level we intentionally keep things
lean.
To that end, I have just pushed the following patch:
commit d583ff601f9f1906b46b5dd38236cdf1aec821ef
Author: Andrea Bolognani <abologna@redhat.com>
Date: Tue Jan 16 19:14:56 2024 +0100
qemu: Default to no USB and no memballoon for new architectures
The current defaults, that can be altered on a per-architecture
basis, are derived from the historical x86 behavior.
Every time support for a new architecture is added to libvirt,
care must be taken to override these default: if that doesn't
happen, guests will end up with additional hardware, which is
something that's generally undesirable.
Turn things around, and require architectures to explicitly
ask for the devices to be created by default instead. The
behavior for existing architectures is preserved.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
which makes this intention more explicit.
loongarch64 should follow the example set by aarch64 and riscv64.
> As above, with the following logic, the memory space in guest os will not
> scale.
>
> addDefaultMemballoon = false;
Same as above: using the memballoon will still be possible, it's just
that it will be an opt-in feature instead of an opt-out one.
--
Andrea Bolognani / Red Hat / Virtualization
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
© 2016 - 2024 Red Hat, Inc.