Re: [libvirt] [PATCH 7/9] qemu: format intremap= on intel-iommu command line

2017-03-28 Thread John Ferlan


On 03/23/2017 11:26 AM, Ján Tomko wrote:
> Add the intremap= option to QEMU command line, corresponding
> to the  attribute of the iommu device.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1427005
> ---
>  src/qemu/qemu_capabilities.c   |  8 
>  src/qemu/qemu_capabilities.h   |  1 +
>  src/qemu/qemu_command.c| 11 +
>  .../qemucapabilitiesdata/caps_2.4.0.x86_64.replies | 22 +++---
>  .../qemucapabilitiesdata/caps_2.5.0.x86_64.replies | 24 +++
>  .../qemucapabilitiesdata/caps_2.6.0.x86_64.replies | 24 +++
>  .../qemucapabilitiesdata/caps_2.7.0.x86_64.replies | 28 +
>  tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml   |  1 +
>  .../qemucapabilitiesdata/caps_2.8.0.x86_64.replies | 37 
>  tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml   |  1 +
>  .../qemucapabilitiesdata/caps_2.9.0.x86_64.replies | 49 
> ++
>  tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml   |  1 +
>  .../qemuxml2argv-intel-iommu-irqchip.args  |  2 +-
>  tests/qemuxml2argvtest.c   |  1 +
>  14 files changed, 166 insertions(+), 44 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 278badf..e9cc754 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -364,6 +364,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>  
>"query-cpu-definitions", /* 250 */
>"kernel-irqchip",
> +  "intel-iommu-intremap",
>  );
>  
>  
> @@ -1732,6 +1733,10 @@ static struct virQEMUCapsStringFlags 
> virQEMUCapsObjectPropsUSBNECXHCI[] = {
>  { "p3", QEMU_CAPS_NEC_USB_XHCI_PORTS },
>  };
>  
> +static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsIntelIOMMU[] = {
> +{ "intremap", QEMU_CAPS_INTEL_IOMMU_INTREMAP },
> +};
> +
>  /* see documentation for virQEMUCapsQMPSchemaGetByPath for the query format 
> */
>  static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = {
>  { "blockdev-add/arg-type/options/+gluster/debug-level", 
> QEMU_CAPS_GLUSTER_DEBUG_LEVEL},
> @@ -1839,6 +1844,9 @@ static struct virQEMUCapsObjectTypeProps 
> virQEMUCapsObjectProps[] = {
>  { "nec-usb-xhci", virQEMUCapsObjectPropsUSBNECXHCI,
>ARRAY_CARDINALITY(virQEMUCapsObjectPropsUSBNECXHCI),
>-1 },
> +{ "intel-iommu", virQEMUCapsObjectPropsIntelIOMMU,
> +  ARRAY_CARDINALITY(virQEMUCapsObjectPropsIntelIOMMU),
> +  QEMU_CAPS_DEVICE_INTEL_IOMMU},
>  };
>  
>  struct virQEMUCapsPropTypeObjects {
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index b9efab8..16db17f 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -400,6 +400,7 @@ typedef enum {
>  /* 250 */
>  QEMU_CAPS_QUERY_CPU_DEFINITIONS, /* qmp query-cpu-definitions */
>  QEMU_CAPS_MACHINE_KERNEL_IRQCHIP, /* -machine kernel_irqchip */
> +QEMU_CAPS_INTEL_IOMMU_INTREMAP, /* intel-iommu.intremap */
>  
>  QEMU_CAPS_LAST /* this must always be the last item */
>  } virQEMUCapsFlags;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index c1c7f1a..ddd889d 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6652,6 +6652,16 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd,
>  return -1;
>  }
>  virBufferAddLit(, "intel-iommu");
> +if (iommu->intremap) {

Since it's not a pointer, use VIR_TRISTATE_SWITCH_ABSENT

Also what if irqchip isn't at least at split, what happens?  IOW:
Shouldn't there a check for the feature being available before using?
What happens if this is "on", but the irqchip isn't set?

Thus something that's checking if the feature bit is set and that the
irqchip is at least split or on before adding.

John

> +if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_INTEL_IOMMU_INTREMAP)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("iommu: interrupt remapping is not 
> supported "
> + "with this QEMU binary"));
> +goto cleanup;
> +}
> +virBufferAsprintf(, ",intremap=%s",
> +  
> virTristateSwitchTypeToString(iommu->intremap));
> +}
>  case VIR_DOMAIN_IOMMU_MODEL_LAST:
>  break;
>  }
> @@ -6659,6 +6669,7 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd,
>  virCommandAddArgBuffer(cmd, );
>  
>  ret = 0;
> + cleanup:
>  virBufferFreeAndReset();
>  return ret;
>  }

[...]

> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args 
> b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args
> index ebf9c3e..d704ee4 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args
> @@ -16,4 +16,4 @@ QEMU_AUDIO_DRV=none \
>  

[libvirt] [PATCH 7/9] qemu: format intremap= on intel-iommu command line

2017-03-23 Thread Ján Tomko
Add the intremap= option to QEMU command line, corresponding
to the  attribute of the iommu device.

https://bugzilla.redhat.com/show_bug.cgi?id=1427005
---
 src/qemu/qemu_capabilities.c   |  8 
 src/qemu/qemu_capabilities.h   |  1 +
 src/qemu/qemu_command.c| 11 +
 .../qemucapabilitiesdata/caps_2.4.0.x86_64.replies | 22 +++---
 .../qemucapabilitiesdata/caps_2.5.0.x86_64.replies | 24 +++
 .../qemucapabilitiesdata/caps_2.6.0.x86_64.replies | 24 +++
 .../qemucapabilitiesdata/caps_2.7.0.x86_64.replies | 28 +
 tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml   |  1 +
 .../qemucapabilitiesdata/caps_2.8.0.x86_64.replies | 37 
 tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml   |  1 +
 .../qemucapabilitiesdata/caps_2.9.0.x86_64.replies | 49 ++
 tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml   |  1 +
 .../qemuxml2argv-intel-iommu-irqchip.args  |  2 +-
 tests/qemuxml2argvtest.c   |  1 +
 14 files changed, 166 insertions(+), 44 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 278badf..e9cc754 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -364,6 +364,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
 
   "query-cpu-definitions", /* 250 */
   "kernel-irqchip",
+  "intel-iommu-intremap",
 );
 
 
@@ -1732,6 +1733,10 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsObjectPropsUSBNECXHCI[] = {
 { "p3", QEMU_CAPS_NEC_USB_XHCI_PORTS },
 };
 
+static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsIntelIOMMU[] = {
+{ "intremap", QEMU_CAPS_INTEL_IOMMU_INTREMAP },
+};
+
 /* see documentation for virQEMUCapsQMPSchemaGetByPath for the query format */
 static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = {
 { "blockdev-add/arg-type/options/+gluster/debug-level", 
QEMU_CAPS_GLUSTER_DEBUG_LEVEL},
@@ -1839,6 +1844,9 @@ static struct virQEMUCapsObjectTypeProps 
virQEMUCapsObjectProps[] = {
 { "nec-usb-xhci", virQEMUCapsObjectPropsUSBNECXHCI,
   ARRAY_CARDINALITY(virQEMUCapsObjectPropsUSBNECXHCI),
   -1 },
+{ "intel-iommu", virQEMUCapsObjectPropsIntelIOMMU,
+  ARRAY_CARDINALITY(virQEMUCapsObjectPropsIntelIOMMU),
+  QEMU_CAPS_DEVICE_INTEL_IOMMU},
 };
 
 struct virQEMUCapsPropTypeObjects {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index b9efab8..16db17f 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -400,6 +400,7 @@ typedef enum {
 /* 250 */
 QEMU_CAPS_QUERY_CPU_DEFINITIONS, /* qmp query-cpu-definitions */
 QEMU_CAPS_MACHINE_KERNEL_IRQCHIP, /* -machine kernel_irqchip */
+QEMU_CAPS_INTEL_IOMMU_INTREMAP, /* intel-iommu.intremap */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c1c7f1a..ddd889d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6652,6 +6652,16 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd,
 return -1;
 }
 virBufferAddLit(, "intel-iommu");
+if (iommu->intremap) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_INTEL_IOMMU_INTREMAP)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("iommu: interrupt remapping is not supported "
+ "with this QEMU binary"));
+goto cleanup;
+}
+virBufferAsprintf(, ",intremap=%s",
+  virTristateSwitchTypeToString(iommu->intremap));
+}
 case VIR_DOMAIN_IOMMU_MODEL_LAST:
 break;
 }
@@ -6659,6 +6669,7 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd,
 virCommandAddArgBuffer(cmd, );
 
 ret = 0;
+ cleanup:
 virBufferFreeAndReset();
 return ret;
 }
diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies 
b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies
index 6822181..9f256c4 100644
--- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies
+++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies
@@ -3123,6 +3123,16 @@
 {
   "return": [
 {
+  "name": "version",
+  "type": "uint32"
+}
+  ],
+  "id": "libvirt-41"
+}
+
+{
+  "return": [
+{
   "name": "pc-i440fx-2.4",
   "is-default": true,
   "cpu-max": 255,
@@ -3246,7 +3256,7 @@
   "cpu-max": 255
 }
   ],
-  "id": "libvirt-41"
+  "id": "libvirt-42"
 }
 
 {
@@ -3336,21 +3346,21 @@
   "name": "qemu64"
 }
   ],
-  "id": "libvirt-42"
+  "id": "libvirt-43"
 }
 
 {
   "return": [
 "tpm-tis"
   ],
-  "id": "libvirt-43"
+  "id": "libvirt-44"
 }
 
 {
   "return": [
 "passthrough"
   ],
-  "id": "libvirt-44"
+  "id": "libvirt-45"
 }
 
 {
@@ -4358,7 +4368,7 @@
   "option": "drive"
 }
   ],
-  "id":