[libvirt] [PATCHv2 1/6] conf: add <irqchip mode> to <features>

Ján Tomko posted 6 patches 8 years, 9 months ago
[libvirt] [PATCHv2 1/6] conf: add <irqchip mode> to <features>
Posted by Ján Tomko 8 years, 9 months ago
Add a new <irqchip> element with a mode attribute.

Possible values are off, split or on.

https://bugzilla.redhat.com/show_bug.cgi?id=1427005
---
 docs/formatdomain.html.in                          | 10 +++++++
 docs/schemas/domaincommon.rng                      | 16 ++++++++++
 src/conf/domain_conf.c                             | 34 +++++++++++++++++++++-
 src/conf/domain_conf.h                             | 12 ++++++++
 .../qemuxml2argv-intel-iommu-irqchip.xml           | 29 ++++++++++++++++++
 .../qemuxml2xmlout-intel-iommu-irqchip.xml         |  1 +
 tests/qemuxml2xmltest.c                            |  1 +
 7 files changed, 102 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml
 create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index b1e38f0..abf089a 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1643,6 +1643,7 @@
   &lt;/kvm&gt;
   &lt;pvspinlock state='on'/&gt;
   &lt;gic version='2'/&gt;
+  &lt;irqchip mode='split'/&gt;
 
 &lt;/features&gt;
 ...</pre>
@@ -1804,6 +1805,15 @@
           for hypervisor to decide.
           <span class="since">Since 2.1.0</span>
       </dd>
+      <dt><code>irqchip</code></dt>
+      <dd>Tune the in-kernel irqchip. Possible values for the
+          <code>mode</code> attribute are:
+          <code>on</code>, <code>split</code> and <code>off</code>.
+          <code>split</code> is useful for using interrupt remapping
+          with the <a href="#elementsIommu">IOMMU device</a>.
+          The default is left for hypervisor to decide.
+          <span class="since">Since 3.3.0</span> (QEMU only)
+      </dd>
     </dl>
 
     <h3><a name="elementsTime">Time keeping</a></h3>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index edc225f..df3143e 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4511,6 +4511,9 @@
               </optional>
             </element>
           </optional>
+          <optional>
+            <ref name="irqchip"/>
+          </optional>
         </interleave>
       </element>
     </optional>
@@ -4686,6 +4689,19 @@
     </element>
   </define>
 
+  <define name="irqchip">
+    <element name="irqchip">
+      <attribute name="mode">
+        <choice>
+          <value>off</value>
+          <value>split</value>
+          <value>on</value>
+        </choice>
+      </attribute>
+      <empty/>
+    </element>
+  </define>
+
   <define name="address">
     <element name="address">
       <choice>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 705deb3..931320e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -140,7 +140,8 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST,
               "pmu",
               "vmport",
               "gic",
-              "smm")
+              "smm",
+              "irqchip")
 
 VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, VIR_DOMAIN_CAPABILITIES_POLICY_LAST,
               "default",
@@ -857,6 +858,12 @@ VIR_ENUM_IMPL(virDomainLoader,
               "rom",
               "pflash")
 
+VIR_ENUM_IMPL(virDomainIRQChip,
+              VIR_DOMAIN_IRQCHIP_LAST,
+              "off",
+              "split",
+              "on")
+
 /* Internal mapping: subset of block job types that can be present in
  * <mirror> XML (remaining types are not two-phase). */
 VIR_ENUM_DECL(virDomainBlockJob)
@@ -17442,6 +17449,24 @@ virDomainDefParseXML(xmlDocPtr xml,
             ctxt->node = node;
             break;
 
+        case VIR_DOMAIN_FEATURE_IRQCHIP:
+            node = ctxt->node;
+            ctxt->node = nodes[i];
+            tmp = virXPathString("string(./@mode)", ctxt);
+            if (tmp) {
+                int value = virDomainIRQChipTypeFromString(tmp);
+                if (value < 0) {
+                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                   _("Unknown irqchip mode: %s"),
+                                   tmp);
+                    goto error;
+                }
+                def->irqchip = value;
+                def->features[val] = VIR_TRISTATE_SWITCH_ON;
+            }
+            ctxt->node = node;
+            break;
+
         /* coverity[dead_error_begin] */
         case VIR_DOMAIN_FEATURE_LAST:
             break;
@@ -24520,6 +24545,13 @@ virDomainDefFormatInternal(virDomainDefPtr def,
                 }
                 break;
 
+            case VIR_DOMAIN_FEATURE_IRQCHIP:
+                if (def->features[i] == VIR_TRISTATE_SWITCH_ON) {
+                    virBufferAsprintf(buf, "<irqchip mode='%s'/>\n",
+                                      virDomainIRQChipTypeToString(def->irqchip));
+                }
+                break;
+
             /* coverity[dead_error_begin] */
             case VIR_DOMAIN_FEATURE_LAST:
                 break;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 7da554f..97c4418 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1671,6 +1671,7 @@ typedef enum {
     VIR_DOMAIN_FEATURE_VMPORT,
     VIR_DOMAIN_FEATURE_GIC,
     VIR_DOMAIN_FEATURE_SMM,
+    VIR_DOMAIN_FEATURE_IRQCHIP,
 
     VIR_DOMAIN_FEATURE_LAST
 } virDomainFeature;
@@ -1810,6 +1811,16 @@ struct _virDomainLoaderDef {
 
 void virDomainLoaderDefFree(virDomainLoaderDefPtr loader);
 
+typedef enum {
+    VIR_DOMAIN_IRQCHIP_OFF = 0,
+    VIR_DOMAIN_IRQCHIP_SPLIT,
+    VIR_DOMAIN_IRQCHIP_ON,
+
+    VIR_DOMAIN_IRQCHIP_LAST
+} virDomainIRQChip;
+
+VIR_ENUM_DECL(virDomainIRQChip);
+
 /* Operating system configuration data & machine / arch */
 typedef struct _virDomainOSDef virDomainOSDef;
 typedef virDomainOSDef *virDomainOSDefPtr;
@@ -2259,6 +2270,7 @@ struct _virDomainDef {
     unsigned int hyperv_spinlocks;
     virGICVersion gic_version;
     char *hyperv_vendor_id;
+    virDomainIRQChip irqchip;
 
     /* These options are of type virTristateSwitch: ON = keep, OFF = drop */
     int caps_features[VIR_DOMAIN_CAPS_FEATURE_LAST];
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml
new file mode 100644
index 0000000..cc895af
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml
@@ -0,0 +1,29 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219100</memory>
+  <currentMemory unit='KiB'>219100</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64' machine='q35'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <irqchip mode='split'/>
+  </features>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <controller type='pci' index='0' model='pcie-root'/>
+    <controller type='sata' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/>
+    </controller>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='none'/>
+    <iommu model='intel'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml
new file mode 120000
index 0000000..58a0199
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml
@@ -0,0 +1 @@
+../qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml
\ No newline at end of file
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index e4b510f..c7d4788 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1121,6 +1121,7 @@ mymain(void)
     DO_TEST("intel-iommu-machine",
             QEMU_CAPS_MACHINE_OPT,
             QEMU_CAPS_MACHINE_IOMMU);
+    DO_TEST("intel-iommu-irqchip", NONE);
 
     DO_TEST("cpu-check-none", NONE);
     DO_TEST("cpu-check-partial", NONE);
-- 
2.10.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 1/6] conf: add <irqchip mode> to <features>
Posted by John Ferlan 8 years, 9 months ago

On 04/20/2017 08:19 AM, Ján Tomko wrote:
> Add a new <irqchip> element with a mode attribute.
> 
> Possible values are off, split or on.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1427005
> ---
>  docs/formatdomain.html.in                          | 10 +++++++
>  docs/schemas/domaincommon.rng                      | 16 ++++++++++
>  src/conf/domain_conf.c                             | 34 +++++++++++++++++++++-
>  src/conf/domain_conf.h                             | 12 ++++++++
>  .../qemuxml2argv-intel-iommu-irqchip.xml           | 29 ++++++++++++++++++
>  .../qemuxml2xmlout-intel-iommu-irqchip.xml         |  1 +
>  tests/qemuxml2xmltest.c                            |  1 +
>  7 files changed, 102 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml
>  create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index b1e38f0..abf089a 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1643,6 +1643,7 @@
>    &lt;/kvm&gt;
>    &lt;pvspinlock state='on'/&gt;
>    &lt;gic version='2'/&gt;
> +  &lt;irqchip mode='split'/&gt;
>  
>  &lt;/features&gt;
>  ...</pre>
> @@ -1804,6 +1805,15 @@
>            for hypervisor to decide.
>            <span class="since">Since 2.1.0</span>
>        </dd>
> +      <dt><code>irqchip</code></dt>
> +      <dd>Tune the in-kernel irqchip. Possible values for the
> +          <code>mode</code> attribute are:
> +          <code>on</code>, <code>split</code> and <code>off</code>.
> +          <code>split</code> is useful for using interrupt remapping
> +          with the <a href="#elementsIommu">IOMMU device</a>.

Something that isn't implemented until the subsequent patch, but I'm not
against describing this feature a bit more here...

I think most importantly what setting this feature will "do" would be
useful.  How does someone know they need this? And secondarily what
would it be required for? What does "on" really do?  IOW: What the
difference between split and on.

> +          The default is left for hypervisor to decide.
> +          <span class="since">Since 3.3.0</span> (QEMU only)
> +      </dd>
>      </dl>
>  
>      <h3><a name="elementsTime">Time keeping</a></h3>

...

> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml
> new file mode 100644
> index 0000000..cc895af
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml
> @@ -0,0 +1,29 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219100</memory>
> +  <currentMemory unit='KiB'>219100</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='x86_64' machine='q35'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <features>
> +    <irqchip mode='split'/>
> +  </features>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu-system-x86_64</emulator>
> +    <controller type='pci' index='0' model='pcie-root'/>
> +    <controller type='sata' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/>
> +    </controller>
> +    <input type='mouse' bus='ps2'/>
> +    <input type='keyboard' bus='ps2'/>
> +    <memballoon model='none'/>
> +    <iommu model='intel'/>
> +  </devices>
> +</domain>

Could a few devices be added so that future patches will actually
generate a command line that would should the code conforms to the
requirement that the "-device intel-iommu" appears in the command list
before the "rest of the devices".

John

> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml
> new file mode 120000
> index 0000000..58a0199
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml
> @@ -0,0 +1 @@
> +../qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml
> \ No newline at end of file
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index e4b510f..c7d4788 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -1121,6 +1121,7 @@ mymain(void)
>      DO_TEST("intel-iommu-machine",
>              QEMU_CAPS_MACHINE_OPT,
>              QEMU_CAPS_MACHINE_IOMMU);
> +    DO_TEST("intel-iommu-irqchip", NONE);
>  
>      DO_TEST("cpu-check-none", NONE);
>      DO_TEST("cpu-check-partial", NONE);
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 1/6] conf: add <irqchip mode> to <features>
Posted by Ján Tomko 8 years, 9 months ago
On Mon, Apr 24, 2017 at 05:40:07PM -0400, John Ferlan wrote:
>
>
>On 04/20/2017 08:19 AM, Ján Tomko wrote:
>> Add a new <irqchip> element with a mode attribute.
>>
>> Possible values are off, split or on.
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=1427005
>> ---
>>  docs/formatdomain.html.in                          | 10 +++++++
>>  docs/schemas/domaincommon.rng                      | 16 ++++++++++
>>  src/conf/domain_conf.c                             | 34 +++++++++++++++++++++-
>>  src/conf/domain_conf.h                             | 12 ++++++++
>>  .../qemuxml2argv-intel-iommu-irqchip.xml           | 29 ++++++++++++++++++
>>  .../qemuxml2xmlout-intel-iommu-irqchip.xml         |  1 +
>>  tests/qemuxml2xmltest.c                            |  1 +
>>  7 files changed, 102 insertions(+), 1 deletion(-)
>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml
>>  create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index b1e38f0..abf089a 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -1643,6 +1643,7 @@
>>    &lt;/kvm&gt;
>>    &lt;pvspinlock state='on'/&gt;
>>    &lt;gic version='2'/&gt;
>> +  &lt;irqchip mode='split'/&gt;
>>
>>  &lt;/features&gt;
>>  ...</pre>
>> @@ -1804,6 +1805,15 @@
>>            for hypervisor to decide.
>>            <span class="since">Since 2.1.0</span>
>>        </dd>
>> +      <dt><code>irqchip</code></dt>
>> +      <dd>Tune the in-kernel irqchip. Possible values for the
>> +          <code>mode</code> attribute are:
>> +          <code>on</code>, <code>split</code> and <code>off</code>.
>> +          <code>split</code> is useful for using interrupt remapping
>> +          with the <a href="#elementsIommu">IOMMU device</a>.
>
>Something that isn't implemented until the subsequent patch, but I'm not
>against describing this feature a bit more here...
>

What would you say?

>I think most importantly what setting this feature will "do" would be
>useful.  How does someone know they need this?

It is needed if they want interrupt remapping for assigned devices.
They can find out from a guide like:
http://wiki.qemu.org/Features/VT-d#References
or the linked BZ, or from the error message QEMU reports when
they try to use interrupt remapping from libvirt without setting
this to "split".

> And secondarily what
>would it be required for? What does "on" really do?  IOW: What the
>difference between split and on.
>

IIUC options other than "split" aren't that useful.

Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list