Re: [PATCHv3 1/6] conf: add <irqchip mode> to <features>

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Apr 26, 2017 at 10:29:02AM +0200, Ján Tomko wrote:
> Add a new <irqchip> element with a mode attribute.
> 
> Possible values are off, split or on.

Hi, Ján,

Thanks you for cced me. One tiny comment on irqchip mode...

Here could user specify irqchip=off from libvirt side? IIUC that might
be dangerous since userspace APIC should only be for debugging purpose
(when kernel-irqchip=off, we'll be using userspace APIC, not kernel
one, and iirc Paolo mentioned known bugs in userspace APIC). So imho
we'd better not allow user to use "off", but only "on" and "split".

CCing Paolo here in case he has any comment on this.

Thanks,

> 
> 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 e31a271..4b92a0f 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 eb4b0f7..7772a91 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -4514,6 +4514,9 @@
>                </optional>
>              </element>
>            </optional>
> +          <optional>
> +            <ref name="irqchip"/>
> +          </optional>
>          </interleave>
>        </element>
>      </optional>
> @@ -4689,6 +4692,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 fe9b7c7..73fa268 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)
> @@ -17521,6 +17528,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;
> @@ -24601,6 +24626,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 3b6b174..61af012 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1673,6 +1673,7 @@ typedef enum {
>      VIR_DOMAIN_FEATURE_VMPORT,
>      VIR_DOMAIN_FEATURE_GIC,
>      VIR_DOMAIN_FEATURE_SMM,
> +    VIR_DOMAIN_FEATURE_IRQCHIP,
>  
>      VIR_DOMAIN_FEATURE_LAST
>  } virDomainFeature;
> @@ -1812,6 +1813,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;
> @@ -2261,6 +2272,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 2dccde7..40425f5 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -1122,6 +1122,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
> 

-- 
Peter Xu

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux