[PATCH] qemu: support riscv-aia feature for RISC-V KVM
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 <xiangwencheng@lanxincomputing.com> --- NEWS.rst | 10 ++++++++ docs/formatdomain.rst | 2 ++ src/conf/domain_conf.c | 29 ++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/conf/schemas/domaincommon.rng | 12 +++++++++ src/qemu/qemu_command.c | 13 +++++++--- tests/qemuxmlconfdata/kvm-features-off.xml | 1 + tests/qemuxmlconfdata/kvm-features.xml | 1 + 8 files changed, 67 insertions(+), 3 deletions(-) diff --git a/NEWS.rst b/NEWS.rst index 435760e797..7e91f81d64 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -17,6 +17,16 @@ v11.7.0 (unreleased) * **New features** + * Support riscv-aia feature for RISC-V KVM + + 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. + * **Improvements** * **Bug fixes** 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`` diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7766e302ec..8ed65d1794 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -224,6 +224,7 @@ VIR_ENUM_IMPL(virDomainKVM, "poll-control", "pv-ipi", "dirty-ring", + "riscv-aia", ); VIR_ENUM_IMPL(virDomainXen, @@ -17183,6 +17184,20 @@ virDomainFeaturesKVMDefParse(virDomainDef *def, return -1; } } + + if (feature == VIR_DOMAIN_KVM_RISCV_AIA && + value == VIR_TRISTATE_SWITCH_ON) { + def->kvm_features->riscv_aia_mode = virXMLPropString(feat, "mode"); + + if (strcmp(def->kvm_features->riscv_aia_mode, "emul") && + strcmp(def->kvm_features->riscv_aia_mode, "hwaccel") && + strcmp(def->kvm_features->riscv_aia_mode, "auto")) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("riscv aia must be emul, hwaccel or auto")); + + return -1; + } + } } def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON; @@ -21697,6 +21712,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src, case VIR_DOMAIN_KVM_POLLCONTROL: case VIR_DOMAIN_KVM_PVIPI: case VIR_DOMAIN_KVM_DIRTY_RING: + case VIR_DOMAIN_KVM_RISCV_AIA: if (src->kvm_features->features[i] != dst->kvm_features->features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of KVM feature '%1$s' differs: source: '%2$s', destination: '%3$s'"), @@ -28752,6 +28768,19 @@ 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->riscv_aia_mode != NULL) { + virBufferAsprintf(&childBuf, " mode='%s'/>\n", + def->kvm_features->riscv_aia_mode); + } else { + virBufferAddLit(&childBuf, "/>\n"); + } + } + break; case VIR_DOMAIN_KVM_LAST: break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index eca820892e..0d749c0f3c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2270,6 +2270,7 @@ typedef enum { VIR_DOMAIN_KVM_POLLCONTROL, VIR_DOMAIN_KVM_PVIPI, VIR_DOMAIN_KVM_DIRTY_RING, + VIR_DOMAIN_KVM_RISCV_AIA, VIR_DOMAIN_KVM_LAST } virDomainKVM; @@ -2476,6 +2477,7 @@ struct _virDomainFeatureKVM { int features[VIR_DOMAIN_KVM_LAST]; unsigned int dirty_ring_size; /* size of dirty ring for each vCPU, no units */ + char *riscv_aia_mode; }; 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/qemu/qemu_command.c b/src/qemu/qemu_command.c index e8de386f30..6db283ef29 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", def->kvm_features->riscv_aia_mode); + } } break; diff --git a/tests/qemuxmlconfdata/kvm-features-off.xml b/tests/qemuxmlconfdata/kvm-features-off.xml index 3cd4ff18f2..2340a93ce5 100644 --- a/tests/qemuxmlconfdata/kvm-features-off.xml +++ b/tests/qemuxmlconfdata/kvm-features-off.xml @@ -16,6 +16,7 @@ <poll-control state='off'/> <pv-ipi state='off'/> <dirty-ring state='off'/> + <riscv-aia state='off'/> </kvm> </features> <cpu mode='host-passthrough' check='none' migratable='off'/> diff --git a/tests/qemuxmlconfdata/kvm-features.xml b/tests/qemuxmlconfdata/kvm-features.xml index 78091064b1..c06b0aa58b 100644 --- a/tests/qemuxmlconfdata/kvm-features.xml +++ b/tests/qemuxmlconfdata/kvm-features.xml @@ -16,6 +16,7 @@ <poll-control state='on'/> <pv-ipi state='on'/> <dirty-ring state='on' size='4096'/> + <riscv-aia state='on' mode='auto'/> </kvm> </features> <cpu mode='host-passthrough' check='none' migratable='off'/> -- 2.46.2.windows.1
On Fri, Aug 15, 2025 at 19:44:15 +0800, BillXiang wrote:
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 <xiangwencheng@lanxincomputing.com> ---
Note that in this review I'll note some common mistakes from this patch. I don't know what this feature is about so I'll not comment on that
NEWS.rst | 10 ++++++++
We mandate that changes to NEWS.rst are always separated into a separate commit with no other changes. This is to avoid conflicts both when applying the code (which is the case with this patch which can't be applied due to a conflict in the news) and also for backports.
docs/formatdomain.rst | 2 ++ src/conf/domain_conf.c | 29 ++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/conf/schemas/domaincommon.rng | 12 +++++++++ src/qemu/qemu_command.c | 13 +++++++--- tests/qemuxmlconfdata/kvm-features-off.xml | 1 + tests/qemuxmlconfdata/kvm-features.xml | 1 +
You've modified test input files but apparently didn't run the test suite, because it now fails: Summary of Failures: 302/312 libvirt:syntax-check / trailing_blank FAIL 0.50s exit status 2 311/312 libvirt:bin / qemuxmlconftest FAIL 4.11s exit status 1 See https://libvirt.org/advanced-tests.html namely VIR_TEST_REGENERATE_OUTPUT to avoid needing to do it manually.
8 files changed, 67 insertions(+), 3 deletions(-)
diff --git a/NEWS.rst b/NEWS.rst index 435760e797..7e91f81d64 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -17,6 +17,16 @@ v11.7.0 (unreleased)
* **New features**
+ * Support riscv-aia feature for RISC-V KVM + + 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. + * **Improvements**
* **Bug fixes** 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`` diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7766e302ec..8ed65d1794 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -224,6 +224,7 @@ VIR_ENUM_IMPL(virDomainKVM, "poll-control", "pv-ipi", "dirty-ring", + "riscv-aia", );
VIR_ENUM_IMPL(virDomainXen, @@ -17183,6 +17184,20 @@ virDomainFeaturesKVMDefParse(virDomainDef *def, return -1; } } + + if (feature == VIR_DOMAIN_KVM_RISCV_AIA && + value == VIR_TRISTATE_SWITCH_ON) { + def->kvm_features->riscv_aia_mode = virXMLPropString(feat, "mode"); + + if (strcmp(def->kvm_features->riscv_aia_mode, "emul") && + strcmp(def->kvm_features->riscv_aia_mode, "hwaccel") && + strcmp(def->kvm_features->riscv_aia_mode, "auto")) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("riscv aia must be emul, hwaccel or auto"));
For constructs like this I'd suggest to declare an enum and store it as an enum value rather than a string. In addition we don't allow use of strcmp and requre use of STREQ/STRNEQ Also the string copied into 'riscv_aia_mode' is not freed, thus leaked. This iwll be avoided by using the enum.
+ + return -1; + } + } }
def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON; @@ -21697,6 +21712,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src, case VIR_DOMAIN_KVM_POLLCONTROL: case VIR_DOMAIN_KVM_PVIPI: case VIR_DOMAIN_KVM_DIRTY_RING: + case VIR_DOMAIN_KVM_RISCV_AIA: if (src->kvm_features->features[i] != dst->kvm_features->features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of KVM feature '%1$s' differs: source: '%2$s', destination: '%3$s'"),
Is the 'mode' guest visible? I'd expect that it's checked here too if it is guest visible.
@@ -28752,6 +28768,19 @@ 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->riscv_aia_mode != NULL) { + virBufferAsprintf(&childBuf, " mode='%s'/>\n", + def->kvm_features->riscv_aia_mode); + } else { + virBufferAddLit(&childBuf, "/>\n"); + } + } + break;
case VIR_DOMAIN_KVM_LAST: break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index eca820892e..0d749c0f3c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2270,6 +2270,7 @@ typedef enum { VIR_DOMAIN_KVM_POLLCONTROL, VIR_DOMAIN_KVM_PVIPI, VIR_DOMAIN_KVM_DIRTY_RING, + VIR_DOMAIN_KVM_RISCV_AIA,
VIR_DOMAIN_KVM_LAST } virDomainKVM; @@ -2476,6 +2477,7 @@ struct _virDomainFeatureKVM { int features[VIR_DOMAIN_KVM_LAST];
unsigned int dirty_ring_size; /* size of dirty ring for each vCPU, no units */ + char *riscv_aia_mode; };
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/qemu/qemu_command.c b/src/qemu/qemu_command.c index e8de386f30..6db283ef29 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", def->kvm_features->riscv_aia_mode);
The mode is declared as optional, here you use it unconditionally.
+ } } break;
diff --git a/tests/qemuxmlconfdata/kvm-features-off.xml b/tests/qemuxmlconfdata/kvm-features-off.xml index 3cd4ff18f2..2340a93ce5 100644 --- a/tests/qemuxmlconfdata/kvm-features-off.xml +++ b/tests/qemuxmlconfdata/kvm-features-off.xml @@ -16,6 +16,7 @@ <poll-control state='off'/> <pv-ipi state='off'/> <dirty-ring state='off'/> + <riscv-aia state='off'/> </kvm> </features> <cpu mode='host-passthrough' check='none' migratable='off'/> diff --git a/tests/qemuxmlconfdata/kvm-features.xml b/tests/qemuxmlconfdata/kvm-features.xml index 78091064b1..c06b0aa58b 100644 --- a/tests/qemuxmlconfdata/kvm-features.xml +++ b/tests/qemuxmlconfdata/kvm-features.xml @@ -16,6 +16,7 @@ <poll-control state='on'/> <pv-ipi state='on'/> <dirty-ring state='on' size='4096'/> + <riscv-aia state='on' mode='auto'/> </kvm> </features> <cpu mode='host-passthrough' check='none' migratable='off'/> -- 2.46.2.windows.1
From: "Peter Krempa"<pkrempa@redhat.com> Date: Wed, Aug 20, 2025, 16:08 Subject: Re: [PATCH] qemu: support riscv-aia feature for RISC-V KVM To: "BillXiang"<xiangwencheng@lanxincomputing.com> Cc: <devel@lists.libvirt.org> On Fri, Aug 15, 2025 at 19:44:15 +0800, BillXiang wrote:
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 <xiangwencheng@lanxincomputing.com> --- Note that in this review I'll note some common mistakes from this patch. I don't know what this feature is about so I'll not comment on that
Hi Peter, Thank you for your review. I've a few points I'd like to clarify, could you please provide a bit more detail?
NEWS.rst | 10 ++++++++ We mandate that changes to NEWS.rst are always separated into a separate commit with no other changes. This is to avoid conflicts both when applying the code (which is the case with this patch which can't be applied due to a conflict in the news) and also for backports.
I'll separate it into a separate patch.
docs/formatdomain.rst | 2 ++ src/conf/domain_conf.c | 29 ++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/conf/schemas/domaincommon.rng | 12 +++++++++ src/qemu/qemu_command.c | 13 +++++++--- tests/qemuxmlconfdata/kvm-features-off.xml | 1 + tests/qemuxmlconfdata/kvm-features.xml | 1 + You've modified test input files but apparently didn't run the test suite, because it now fails: Summary of Failures: 302/312 libvirt:syntax-check / trailing_blank FAIL 0.50s exit status 2 311/312 libvirt:bin / qemuxmlconftest FAIL 4.11s exit status 1 See https://libvirt.org/advanced-tests.html namely VIR_TEST_REGENERATE_OUTPUT to avoid needing to do it manually.
I've run the test suite but get lots of TIMEOUT. I'll try again.
8 files changed, 67 insertions(+), 3 deletions(-) diff --git a/NEWS.rst b/NEWS.rst index 435760e797..7e91f81d64 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -17,6 +17,16 @@ v11.7.0 (unreleased) * **New features** + * Support riscv-aia feature for RISC-V KVM + + 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. + * **Improvements** * **Bug fixes** 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`` diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7766e302ec..8ed65d1794 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -224,6 +224,7 @@ VIR_ENUM_IMPL(virDomainKVM, "poll-control", "pv-ipi", "dirty-ring", + "riscv-aia", ); VIR_ENUM_IMPL(virDomainXen, @@ -17183,6 +17184,20 @@ virDomainFeaturesKVMDefParse(virDomainDef *def, return -1; } } + + if (feature == VIR_DOMAIN_KVM_RISCV_AIA && + value == VIR_TRISTATE_SWITCH_ON) { + def->kvm_features->riscv_aia_mode = virXMLPropString(feat, "mode"); + + if (strcmp(def->kvm_features->riscv_aia_mode, "emul") && + strcmp(def->kvm_features->riscv_aia_mode, "hwaccel") && + strcmp(def->kvm_features->riscv_aia_mode, "auto")) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("riscv aia must be emul, hwaccel or auto")); For constructs like this I'd suggest to declare an enum and store it as an enum value rather than a string. In addition we don't allow use of strcmp and requre use of STREQ/STRNEQ Also the string copied into 'riscv_aia_mode' is not freed, thus leaked. This iwll be avoided by using the enum.
I'll change it to enum.
+ + return -1; + } + } } def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON; @@ -21697,6 +21712,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src, case VIR_DOMAIN_KVM_POLLCONTROL: case VIR_DOMAIN_KVM_PVIPI: case VIR_DOMAIN_KVM_DIRTY_RING: + case VIR_DOMAIN_KVM_RISCV_AIA: if (src->kvm_features->features[i] != dst->kvm_features->features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of KVM feature '%1$s' differs: source: '%2$s', destination: '%3$s'"), Is the 'mode' guest visible? I'd expect that it's checked here too if it is guest visible.
The check here is for VM migration. But AFAIK,the RISC-V 'riscv_aia' feature requires the 'aia' feature being set to 'aplic-imsic', which currently lacks support for migration or snapshotting.
@@ -28752,6 +28768,19 @@ 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->riscv_aia_mode != NULL) { + virBufferAsprintf(&childBuf, " mode='%s'/>\n", + def->kvm_features->riscv_aia_mode); + } else { + virBufferAddLit(&childBuf, "/>\n"); + } + } + break; case VIR_DOMAIN_KVM_LAST: break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index eca820892e..0d749c0f3c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2270,6 +2270,7 @@ typedef enum { VIR_DOMAIN_KVM_POLLCONTROL, VIR_DOMAIN_KVM_PVIPI, VIR_DOMAIN_KVM_DIRTY_RING, + VIR_DOMAIN_KVM_RISCV_AIA, VIR_DOMAIN_KVM_LAST } virDomainKVM; @@ -2476,6 +2477,7 @@ struct _virDomainFeatureKVM { int features[VIR_DOMAIN_KVM_LAST]; unsigned int dirty_ring_size; /* size of dirty ring for each vCPU, no units */ + char *riscv_aia_mode; }; 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/qemu/qemu_command.c b/src/qemu/qemu_command.c index e8de386f30..6db283ef29 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", def->kvm_features->riscv_aia_mode); The mode is declared as optional, here you use it unconditionally.
I've mirrored the implementation of VIR_DOMAIN_KVM_DIRTY_RING and checked VIR_DOMAIN_KVM_RISCV_AIA with VIR_TRISTATE_SWITCH_ON. Could you let me know what additional condition I should introduce?
+ } } break; diff --git a/tests/qemuxmlconfdata/kvm-features-off.xml b/tests/qemuxmlconfdata/kvm-features-off.xml index 3cd4ff18f2..2340a93ce5 100644 --- a/tests/qemuxmlconfdata/kvm-features-off.xml +++ b/tests/qemuxmlconfdata/kvm-features-off.xml @@ -16,6 +16,7 @@ <poll-control state='off'/> <pv-ipi state='off'/> <dirty-ring state='off'/> + <riscv-aia state='off'/> </kvm> </features> <cpu mode='host-passthrough' check='none' migratable='off'/> diff --git a/tests/qemuxmlconfdata/kvm-features.xml b/tests/qemuxmlconfdata/kvm-features.xml index 78091064b1..c06b0aa58b 100644 --- a/tests/qemuxmlconfdata/kvm-features.xml +++ b/tests/qemuxmlconfdata/kvm-features.xml @@ -16,6 +16,7 @@ <poll-control state='on'/> <pv-ipi state='on'/> <dirty-ring state='on' size='4096'/> + <riscv-aia state='on' mode='auto'/> </kvm> </features> <cpu mode='host-passthrough' check='none' migratable='off'/> -- 2.46.2.windows.1
On Thu, Aug 21, 2025 at 18:36:44 +0800, Bill Xiang wrote:
From: "Peter Krempa"<pkrempa@redhat.com> Date: Wed, Aug 20, 2025, 16:08 Subject: Re: [PATCH] qemu: support riscv-aia feature for RISC-V KVM To: "BillXiang"<xiangwencheng@lanxincomputing.com> Cc: <devel@lists.libvirt.org> On Fri, Aug 15, 2025 at 19:44:15 +0800, BillXiang wrote:
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 <xiangwencheng@lanxincomputing.com> --- Note that in this review I'll note some common mistakes from this patch. I don't know what this feature is about so I'll not comment on that
Hi Peter, Thank you for your review. I've a few points I'd like to clarify, could you please provide a bit more detail?
NEWS.rst | 10 ++++++++ We mandate that changes to NEWS.rst are always separated into a separate commit with no other changes. This is to avoid conflicts both when applying the code (which is the case with this patch which can't be applied due to a conflict in the news) and also for backports.
I'll separate it into a separate patch.
docs/formatdomain.rst | 2 ++ src/conf/domain_conf.c | 29 ++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/conf/schemas/domaincommon.rng | 12 +++++++++ src/qemu/qemu_command.c | 13 +++++++--- tests/qemuxmlconfdata/kvm-features-off.xml | 1 + tests/qemuxmlconfdata/kvm-features.xml | 1 + You've modified test input files but apparently didn't run the test suite, because it now fails: Summary of Failures: 302/312 libvirt:syntax-check / trailing_blank FAIL 0.50s exit status 2 311/312 libvirt:bin / qemuxmlconftest FAIL 4.11s exit status 1 See https://libvirt.org/advanced-tests.html namely VIR_TEST_REGENERATE_OUTPUT to avoid needing to do it manually.
I've run the test suite but get lots of TIMEOUT. I'll try again.
Weird. The testsuite works okay for me, also it works okay in the upstream CI. Let us know if you encounter any problems, but don't forget to mention the exact steps and setup needed to reproduce the issue (same way as when reporting a bug).
8 files changed, 67 insertions(+), 3 deletions(-)
[...]
@@ -21697,6 +21712,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src, case VIR_DOMAIN_KVM_POLLCONTROL: case VIR_DOMAIN_KVM_PVIPI: case VIR_DOMAIN_KVM_DIRTY_RING: + case VIR_DOMAIN_KVM_RISCV_AIA: if (src->kvm_features->features[i] != dst->kvm_features->features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of KVM feature '%1$s' differs: source: '%2$s', destination: '%3$s'"), Is the 'mode' guest visible? I'd expect that it's checked here too if it is guest visible.
The check here is for VM migration. But AFAIK,the RISC-V 'riscv_aia' feature requires the 'aia' feature being set to 'aplic-imsic', which currently lacks support for migration or snapshotting.
It doesn't really matter that it's currently unsupported. Support can be added later in qemu. If it is a guest visible feature it must be handled in the ABI stability check. If the setting doesn't influence anything guest visible then it is okay. I noted this here because the feature has an additional setting and you didn't handle it here. If it is not guest visible you'll need to node in a comment stating that this particular config is not guest visible for anyone coming after you wanting to understand why it works this way.
@@ -28752,6 +28768,19 @@ 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->riscv_aia_mode != NULL) { + virBufferAsprintf(&childBuf, " mode='%s'/>\n", + def->kvm_features->riscv_aia_mode); + } else { + virBufferAddLit(&childBuf, "/>\n"); + } + } + break; case VIR_DOMAIN_KVM_LAST: break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index eca820892e..0d749c0f3c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2270,6 +2270,7 @@ typedef enum { VIR_DOMAIN_KVM_POLLCONTROL, VIR_DOMAIN_KVM_PVIPI, VIR_DOMAIN_KVM_DIRTY_RING, + VIR_DOMAIN_KVM_RISCV_AIA, VIR_DOMAIN_KVM_LAST } virDomainKVM; @@ -2476,6 +2477,7 @@ struct _virDomainFeatureKVM { int features[VIR_DOMAIN_KVM_LAST]; unsigned int dirty_ring_size; /* size of dirty ring for each vCPU, no units */ + char *riscv_aia_mode; }; 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/qemu/qemu_command.c b/src/qemu/qemu_command.c index e8de386f30..6db283ef29 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", def->kvm_features->riscv_aia_mode); The mode is declared as optional, here you use it unconditionally.
I've mirrored the implementation of VIR_DOMAIN_KVM_DIRTY_RING and checked VIR_DOMAIN_KVM_RISCV_AIA with VIR_TRISTATE_SWITCH_ON. Could you let me know what additional condition I should introduce?
I was pointing out the discrepancy between the docs + RNG schema, which declared it as optional and the code in the parser, which didn't handle the possibilty that it's missing (passed it to strcmp, which would crash) and the formatter, which didn't handle it being NULL and would pass NULL to the formatter (this wouldn't crash, but would create invalid config). As noted I'm not really interested in the feature so I don't know if mirroring existing code makes sense. I merely pointed out that the implementation as documented was wrong.
On 2025/8/25 15:58, Peter Krempa wrote:
On Thu, Aug 21, 2025 at 18:36:44 +0800, Bill Xiang wrote:
From: "Peter Krempa"<pkrempa@redhat.com> Date: Wed, Aug 20, 2025, 16:08 Subject: Re: [PATCH] qemu: support riscv-aia feature for RISC-V KVM To: "BillXiang"<xiangwencheng@lanxincomputing.com> Cc: <devel@lists.libvirt.org> On Fri, Aug 15, 2025 at 19:44:15 +0800, BillXiang wrote:
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 <xiangwencheng@lanxincomputing.com> ---
Note that in this review I'll note some common mistakes from this patch. I don't know what this feature is about so I'll not comment on that
Hi Peter, Thank you for your review. I've a few points I'd like to clarify, could you please provide a bit more detail?
NEWS.rst | 10 ++++++++
We mandate that changes to NEWS.rst are always separated into a separate commit with no other changes. This is to avoid conflicts both when applying the code (which is the case with this patch which can't be applied due to a conflict in the news) and also for backports.
I'll separate it into a separate patch.
docs/formatdomain.rst | 2 ++ src/conf/domain_conf.c | 29 ++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/conf/schemas/domaincommon.rng | 12 +++++++++ src/qemu/qemu_command.c | 13 +++++++--- tests/qemuxmlconfdata/kvm-features-off.xml | 1 + tests/qemuxmlconfdata/kvm-features.xml | 1 +
You've modified test input files but apparently didn't run the test suite, because it now fails:
Summary of Failures:
302/312 libvirt:syntax-check / trailing_blank FAIL 0.50s exit status 2 311/312 libvirt:bin / qemuxmlconftest FAIL 4.11s exit status 1
See https://libvirt.org/advanced-tests.html namely VIR_TEST_REGENERATE_OUTPUT to avoid needing to do it manually.
I've run the test suite but get lots of TIMEOUT. I'll try again.
Weird. The testsuite works okay for me, also it works okay in the upstream CI.
Let us know if you encounter any problems, but don't forget to mention the exact steps and setup needed to reproduce the issue (same way as when reporting a bug).
8 files changed, 67 insertions(+), 3 deletions(-)
[...]
@@ -21697,6 +21712,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src, case VIR_DOMAIN_KVM_POLLCONTROL: case VIR_DOMAIN_KVM_PVIPI: case VIR_DOMAIN_KVM_DIRTY_RING: + case VIR_DOMAIN_KVM_RISCV_AIA: if (src->kvm_features->features[i] != dst->kvm_features->features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of KVM feature '%1$s' differs: source: '%2$s', destination: '%3$s'"),
Is the 'mode' guest visible? I'd expect that it's checked here too if it is guest visible.
The check here is for VM migration. But AFAIK,the RISC-V 'riscv_aia' feature requires the 'aia' feature being set to 'aplic-imsic', which currently lacks support for migration or snapshotting.
It doesn't really matter that it's currently unsupported. Support can be added later in qemu. If it is a guest visible feature it must be handled in the ABI stability check. If the setting doesn't influence anything guest visible then it is okay.
I noted this here because the feature has an additional setting and you didn't handle it here. If it is not guest visible you'll need to node in a comment stating that this particular config is not guest visible for anyone coming after you wanting to understand why it works this way.
Perhaps I’ve misinterpreted 'guest visible'. The mode is visible to QEMU itself as described in the commit, not to the guest kernel.
@@ -28752,6 +28768,19 @@ 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->riscv_aia_mode != NULL) { + virBufferAsprintf(&childBuf, " mode='%s'/>\n", + def->kvm_features->riscv_aia_mode); + } else { + virBufferAddLit(&childBuf, "/>\n"); + } + } + break;
case VIR_DOMAIN_KVM_LAST: break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index eca820892e..0d749c0f3c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2270,6 +2270,7 @@ typedef enum { VIR_DOMAIN_KVM_POLLCONTROL, VIR_DOMAIN_KVM_PVIPI, VIR_DOMAIN_KVM_DIRTY_RING, + VIR_DOMAIN_KVM_RISCV_AIA,
VIR_DOMAIN_KVM_LAST } virDomainKVM; @@ -2476,6 +2477,7 @@ struct _virDomainFeatureKVM { int features[VIR_DOMAIN_KVM_LAST];
unsigned int dirty_ring_size; /* size of dirty ring for each vCPU, no units */ + char *riscv_aia_mode; };
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/qemu/qemu_command.c b/src/qemu/qemu_command.c index e8de386f30..6db283ef29 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", def->kvm_features->riscv_aia_mode);
The mode is declared as optional, here you use it unconditionally.
I've mirrored the implementation of VIR_DOMAIN_KVM_DIRTY_RING and checked VIR_DOMAIN_KVM_RISCV_AIA with VIR_TRISTATE_SWITCH_ON. Could you let me know what additional condition I should introduce?
I was pointing out the discrepancy between the docs + RNG schema, which declared it as optional and the code in the parser, which didn't handle the possibilty that it's missing (passed it to strcmp, which would crash) and the formatter, which didn't handle it being NULL and would pass NULL to the formatter (this wouldn't crash, but would create invalid config).
As noted I'm not really interested in the feature so I don't know if mirroring existing code makes sense. I merely pointed out that the implementation as documented was wrong.
participants (3)
-
Bill Xiang -
BillXiang
-
Peter Krempa