On Fri, Aug 22, 2025 at 17:53:28 +0800, BillXiang wrote:
> From: xiangwencheng <[email protected]>
>
> riscv-aia feature was introduced in qemu-8.2 to specify the
> KVM AIA mode. The "riscv-aia" parameter is passed along with
> -accel in QEMU command-line.
> 1) "riscv-aia=emul": IMSIC is emulated by hypervisor
> 2) "riscv-aia=hwaccel": use hardware guest IMSIC
> 3) "riscv-aia=auto": use the hardware guest IMSICs whenever available
> otherwise we fallback to software emulation.
>
> This patch add the corresponding feature named 'riscv-aia'.
>
> Signed-off-by: BillXiang <[email protected]>
> ---
> v1[1] -> v2:
> - Separate changes to NEWS.rst into a separate commit.
> - Remove changes to qemuxmlconfdata files that are used by x86.
> - Store riscv-aia mode as enum.
> - Remove VIR_DOMAIN_KVM_RISCV_AIA from virDomainDefFeaturesCheckABIStability
> beacuse it doesn't support migration currently.
>
> [1]
> https://lists.libvirt.org/archives/list/[email protected]/thread/XBDTP6C5T2RIVHGEI3X7WTDMHSXZMGKL
> ---
> docs/formatdomain.rst | 2 ++
> src/conf/domain_conf.c | 33 ++++++++++++++++++++++++++++++-
> src/conf/domain_conf.h | 10 ++++++++++
> src/conf/schemas/domaincommon.rng | 12 +++++++++++
> src/libvirt_private.syms | 2 ++
> src/qemu/qemu_command.c | 13 +++++++++---
> 6 files changed, 68 insertions(+), 4 deletions(-)
Fails to compile:
In file included from ../../../libvirt/src/util/virxml.h:31,
from ../../../libvirt/src/conf/cpu_conf.h:24,
from ../../../libvirt/src/conf/capabilities.h:26,
from ../../../libvirt/src/conf/domain_conf.h:31,
from ../../../libvirt/src/conf/checkpoint_conf.h:26,
from ../../../libvirt/src/conf/domain_conf.c:32:
../../../libvirt/src/conf/domain_conf.c:230:15: error: no previous prototype
for ‘virDomainKVMRiscvAIAModeTypeToString’ [-Werror=missing-prototypes]
230 | VIR_ENUM_IMPL(virDomainKVMRiscvAIAMode,
| ^~~~~~~~~~~~~~~~~~~~~~~~
../../../libvirt/src/util/virenum.h:35:17: note: in definition of macro
‘VIR_ENUM_IMPL’
35 | const char *name ## TypeToString(int type) { \
| ^~~~
../../../libvirt/src/conf/domain_conf.c:230:15: error: no previous prototype
for ‘virDomainKVMRiscvAIAModeTypeFromString’ [-Werror=missing-prototypes]
230 | VIR_ENUM_IMPL(virDomainKVMRiscvAIAMode,
| ^~~~~~~~~~~~~~~~~~~~~~~~
../../../libvirt/src/util/virenum.h:40:9: note: in definition of macro
‘VIR_ENUM_IMPL’
40 | int name ## TypeFromString(const char *type) { \
| ^~~~
../../../libvirt/src/conf/domain_conf.c: In function
‘virDomainDefFeaturesCheckABIStability’:
../../../libvirt/src/conf/domain_conf.c:21712:13: error: enumeration value
‘VIR_DOMAIN_KVM_RISCV_AIA’ not handled in switch [-Werror=switch]
21712 | switch ((virDomainKVM) i) {
| ^~~~~~
cc1: all warnings being treated as errors
ninja: build stopped: subcommand failed.
Looks like you're missing some VIR_ENUM_DECL()
Couple more points inline. I'll re-arrange some hunks for clarity.
>
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 9f7311b6d5..2ccbb1bfb9 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -2085,6 +2085,7 @@ Hypervisors may allow certain CPU / machine features to
> be toggled on/off.
> <poll-control state='on'/>
> <pv-ipi state='off'/>
> <dirty-ring state='on' size='4096'/>
> + <riscv-aia state='on' mode='auto'/>
> </kvm>
> <xen>
> <e820_host state='on'/>
> @@ -2206,6 +2207,7 @@ are:
> poll-control Decrease IO completion latency by introducing a grace
> period of busy waiting on, off
> :since:`6.10.0 (QEMU 4.2)`
> pv-ipi Paravirtualized send IPIs
> on, off
> :since:`7.10.0 (QEMU 3.1)`
> dirty-ring Enable dirty ring feature
> on, off; size - must be power of 2, range [1024,65536]
> :since:`8.0.0 (QEMU 6.1)`
> + riscv-aia Set riscv KVM AIA mode. Defaults to 'auto'.
> on, off; mode - optional string emul, hwaccel or auto
> :since:`11.7.0 (QEMU 8.2)`
> ==============
> ============================================================================
> ======================================================
> ============================
>
> ``xen``
Stating that default is auto ...
> @@ -17142,6 +17150,7 @@ virDomainFeaturesKVMDefParse(virDomainDef *def,
> {
> g_autoptr(GPtrArray) feats = virXMLNodeGetSubelementList(node, NULL);
> size_t i;
> + g_autofree char *ptval = NULL;
Unused variable. Compiler will catch this.
>
> if (!def->kvm_features)
> def->kvm_features = g_new0(virDomainFeatureKVM, 1);
The enum is defined as:
> +typedef enum {
> + VIR_DOMAIN_KVM_TISCV_AIA_MODE_EMUL = 0,
> + VIR_DOMAIN_KVM_TISCV_AIA_MODE_HWACCEL,
> + VIR_DOMAIN_KVM_TISCV_AIA_MODE_AUTO,
> +
> + VIR_DOMAIN_KVM_TISCV_AIA_MODE_LAST
> +} virDomainKVMRiscvAIAMode;
> +
> @@ -17183,6 +17192,15 @@ virDomainFeaturesKVMDefParse(virDomainDef *def,
> return -1;
> }
> }
> +
> + if (feature == VIR_DOMAIN_KVM_RISCV_AIA &&
> + value == VIR_TRISTATE_SWITCH_ON) {
> + if (virXMLPropEnum(feat, "mode",
> + virDomainKVMRiscvAIAModeTypeFromString,
> + VIR_XML_PROP_NONZERO,
Here you:
1) allow the "mode" property to be missing, in which case the value will
be kept '0' thus VIR_DOMAIN_KVM_TISCV_AIA_MODE_EMUL
2) you explicitly refuse VIR_DOMAIN_KVM_TISCV_AIA_MODE_EMUL case when
xml contains 'mode="emul"'
> + &def->kvm_features->kvm_riscv_aia_mode) < 0)
> + return -1;
> + }
> }
>
> def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON;
> @@ -28752,7 +28770,20 @@ virDomainDefFormatFeatures(virBuffer *buf,
> }
> }
> break;
> -
> + case VIR_DOMAIN_KVM_RISCV_AIA:
> + if (def->kvm_features->features[j] !=
> VIR_TRISTATE_SWITCH_ABSENT) {
> + virBufferAsprintf(&childBuf, "<%s state='%s'",
> + virDomainKVMTypeToString(j),
> +
> virTristateSwitchTypeToString(def->kvm_features->features[j]));
> + if (def->kvm_features->kvm_riscv_aia_mode >= 0 &&
Here you accept 0
> + def->kvm_features->kvm_riscv_aia_mode <
> VIR_DOMAIN_KVM_TISCV_AIA_MODE_LAST) {
This shouldn't be needed.
> + virBufferAsprintf(&childBuf, " mode='%s'/>\n",
> +
> virDomainKVMRiscvAIAModeTypeToString(def->kvm_features->kvm_riscv_aia_mode));
> + } else {
> + virBufferAddLit(&childBuf, "/>\n");
> + }
> + }
> + break;
> case VIR_DOMAIN_KVM_LAST:
> break;
> }
> typedef struct _virDomainFeatureTCG virDomainFeatureTCG;
> diff --git a/src/conf/schemas/domaincommon.rng
> b/src/conf/schemas/domaincommon.rng
> index e369fb6e81..3de5807b1e 100644
> --- a/src/conf/schemas/domaincommon.rng
> +++ b/src/conf/schemas/domaincommon.rng
> @@ -8192,6 +8192,18 @@
> </optional>
> </element>
> </optional>
> + <optional>
> + <element name="riscv-aia">
> + <ref name="featurestate"/>
> + <optional>
> + <attribute name="mode">
> + <data type="string">
> + <param name="pattern">(emul|hwaccel|auto)</param>
> + </data>
> + </attribute>
> + </optional>
> + </element>
> + </optional>
> </interleave>
> </element>
> </define>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index b846011f0f..f540bdbd5b 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -501,6 +501,8 @@ virDomainIOThreadIDDel;
> virDomainIOThreadIDFind;
> virDomainKeyWrapCipherNameTypeFromString;
> virDomainKeyWrapCipherNameTypeToString;
> +virDomainKVMRiscvAIAModeTypeFromString;
> +virDomainKVMRiscvAIAModeTypeToString;
> virDomainLaunchSecurityTypeFromString;
> virDomainLaunchSecurityTypeToString;
> virDomainLeaseDefFree;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index e8de386f30..9f34b5a37d 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6679,6 +6679,9 @@ qemuBuildCpuCommandLine(virCommand *cmd,
>
> case VIR_DOMAIN_KVM_DIRTY_RING:
> break;
> +
> + case VIR_DOMAIN_KVM_RISCV_AIA:
> + break;
>
> case VIR_DOMAIN_KVM_LAST:
> break;
> @@ -7314,9 +7317,13 @@ qemuBuildAccelCommandLine(virCommand *cmd,
> * not that either kvm or tcg can be specified by libvirt
> * so do not worry about the conflict of specifying both
> * */
> - if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON
> &&
> - def->kvm_features->features[VIR_DOMAIN_KVM_DIRTY_RING] ==
> VIR_TRISTATE_SWITCH_ON) {
> - virBufferAsprintf(&buf, ",dirty-ring-size=%d",
> def->kvm_features->dirty_ring_size);
> + if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON)
> {
> + if (def->kvm_features->features[VIR_DOMAIN_KVM_DIRTY_RING] ==
> VIR_TRISTATE_SWITCH_ON) {
> + virBufferAsprintf(&buf, ",dirty-ring-size=%d",
> def->kvm_features->dirty_ring_size);
> + }
> + if (def->kvm_features->features[VIR_DOMAIN_KVM_RISCV_AIA] ==
> VIR_TRISTATE_SWITCH_ON) {
> + virBufferAsprintf(&buf, ",riscv-aia=%s",
> virDomainKVMRiscvAIAModeTypeToString(def->kvm_features->kvm_riscv_aia_mode));
> + }
This still doesn't handle the situation where the setting is missing
from the XML correctly.
If qemu requires a mode, make it mandatory, do not declare deafults in
libvirt.
> }
> break;
>
> --
> 2.34.1
>
