On 10/27/25 4:49 PM, Andrea Bolognani wrote:
On Thu, Sep 18, 2025 at 01:44:05PM +0800, BillXiang wrote:
+++ b/docs/formatdomain.rst
@@ -2206,6 +2207,12 @@ 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 - must be emul, hwaccel or auto          
:since:`11.8.0 (QEMU 8.2)`
+                  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 doesn't build:

   [377/617] Generating 'docs/formatdomain.html.p/formatdomain.html.in'
(wrapped by meson to capture output)
   FAILED: [code=1] docs/formatdomain.html.p/formatdomain.html.in
   /usr/bin/meson --internal exe --capture
docs/formatdomain.html.p/formatdomain.html.in -- /sbin/rst2html5
--exit-status=1 --stylesheet= --strict ../docs/formatdomain.rst
   --- stderr ---
   ../docs/formatdomain.rst:2224: (ERROR/3) Unexpected indentation.
   Exiting due to level-3 (ERROR) system message.

Please make sure that libvirt builds *and* the test suite passes
after each patch of your series.

This bit of documentation appears to have been lifted verbatim from

   
https://gitlab.com/qemu-project/qemu/-/commit/9634ef7eda5f5b57f03924351a213b776f6b8a23

Please rewrite it so that it's less redundant (no need to repeat the
name of the feature so many times) and only information that is
relevant to libvirt users is included (mentioning QEMU's --accel
option is not useful).

As a last note, this missed the 11.8.0 release and will not make it
into the upcoming 11.9.0 release either, so the :since: information
will need to be updated to reflect reality.

+++ b/src/conf/schemas/domaincommon.rng
@@ -8207,6 +8207,16 @@
              </optional>
            </element>
          </optional>
+        <optional>
+          <element name="riscv-aia">
+            <ref name="featurestate"/>
+            <attribute name="mode">
+              <data type="string">
+                <param name="pattern">(auto|emul|hwaccel)</param>
+              </data>
+            </attribute>

I believe this could be

   <attribute name="mode">
     <choice>
       <value>auto</value>
       <value>emul</value>
       <value>hwaccel</value>
     </choice>
   </attribute>

+++ b/src/qemu/qemu_command.c
@@ -6720,6 +6720,9 @@ qemuBuildCpuCommandLine(virCommand *cmd,

              case VIR_DOMAIN_KVM_DIRTY_RING:
                  break;
+
+            case VIR_DOMAIN_KVM_RISCV_AIA:
+                break;

There's some spurious whitespace in this hunk. Please get rid of it.

@@ -7355,9 +7358,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));
+            }

I'm not sure we need to have riscv-aia=auto on the command line.
Isn't that the default? In fact, I'm not convinced we need it in the
enum either.

Is there a situation in which the user can request the riscv-aia
feature to be on but QEMU will not be able to comply with that? Is
there a way to make the device *not* appear?


I'm also wondering how the riscv-aia KVM feature you're introducing
will interact with the existing aia feature. The latter is documented
somewhat extensively in

   https://gitlab.com/qemu-project/qemu/-/blob/master/docs/specs/riscv-aia.rst

but AFAICT there barely is any documentation at all for the former at
the QEMU level. Maybe Daniel (CC'd), who introduced the other
feature into libvirt, knows more.


Having a 'riscv-aia' KVM accel prop and an 'aia' board property is confusing
indeed ...

What we added in libvirt is the 'virt' board AIA support (aplic and 
aplic-imsic).
It interacts with the current accelerator capabilities (kvm, tcg) like the
QEMU doc above describes.

The 'riscv-aia' accelerator property attempts to configure the capabilities
of the in-kernel KVM AIA chip, if present. I'll just copy/paste the commit
msg that introduced it in QEMU:

------------
target/riscv: Create an KVM AIA irqchip


We create a vAIA chip by using the KVM_DEV_TYPE_RISCV_AIA and then set up
the chip with the KVM_DEV_RISCV_AIA_GRP_* APIs.
We also extend KVM accelerator 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.
------------

Note that these are properties implemented by the KVM module, not QEMU,
e.g. they depend on host KVM support.

They interact with the table in that doc in the IMSIC s-mode column,
in the "in-kernel" category:


accel  | accel properties   | IMSIC s-mode

kvm    |  none              |  in-kernel, riscv-aia = auto
kvm    |  riscv-aia=auto    |  in-kernel, riscv-aia = auto
kvm    |  riscv-aia=hwaccel |  in-kernel, use the IMSIC controller from the 
guest HW
kvm    |  riscv-aia=emul    |  in-kernel, KVM will emulate the IMSIC controller


As far as libvirt goes I wouldn't add 'auto' support. Not only it is already
the QEMU default value but it also means "give me either hwaccel or emul".
Might as well just leave the option alone then.


Hope this helps. I'll send a patch adding the riscv-aia info to the QEMU docs.



Thanks,

Daniel







Finally, I would expect more validation to be performed. For example,
that the QEMU binary supports the necessary option and that this
KVM-only feature is not being used for a TCG domain. Admittedly the
latter issue is pre-existing.


Reply via email to