7:07 a.m.
gfx_passthru xl.cfg option enables GPU specific quirks required for working
Intel GPU passthru. Qemu (used for device model by xen) will refuse to start
a VM when an IGD is passed, but this option was not set in Xen.
Signed-off-by: Artur Puzio <contact(a)puzio.waw.pl>
---
docs/formatdomain.html.in | 7 +++++++
docs/schemas/domaincommon.rng | 5 +++++
src/conf/domain_conf.c | 4 ++++
src/conf/domain_conf.h | 1 +
src/libxl/libxl_conf.c | 13 +++++++++++++
5 files changed, 30 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 91d6f6c0d3..5307844a23 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2064,6 +2064,7 @@
<xen>
<e820_host state='on'/>
<passthrough state='on' mode='share_pt'/>
+ <gfx_passthru state='on'/>
</xen>
<pvspinlock state='on'/>
<gic version='2'/>
@@ -2270,6 +2271,12 @@
<td>on, off; mode - optional string sync_pt or share_pt</td>
<td><span class="since">6.3.0</span></td>
</tr>
+ <tr>
+ <td>gfx_passthru</td>
+ <td>Enable Intel GPU specific quirks. Required and allowed only when
passing an IGD.</td>
+ <td>on, off</td>
+ <td><span class="since">6.3.0</span></td>
+ </tr>
</table>
</dd>
<dt><code>pmu</code></dt>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 9d60b090f3..7d8ea879a1 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -6409,6 +6409,11 @@
</optional>
</element>
</optional>
+ <optional>
+ <element name="gfx_passthru">
+ <ref name="featurestate"/>
+ </element>
+ </optional>
</interleave>
</element>
</define>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8a87586936..75f72ff64c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -214,6 +214,7 @@ VIR_ENUM_IMPL(virDomainXen,
VIR_DOMAIN_XEN_LAST,
"e820_host",
"passthrough",
+ "gfx_passthru"
);
VIR_ENUM_IMPL(virDomainXenPassthroughMode,
@@ -19649,6 +19650,7 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
switch ((virDomainXen) feature) {
case VIR_DOMAIN_XEN_E820_HOST:
+ case VIR_DOMAIN_XEN_GFX_PASSTHRU:
break;
case VIR_DOMAIN_XEN_PASSTHROUGH:
@@ -23579,6 +23581,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src,
}
switch ((virDomainXen) i) {
case VIR_DOMAIN_XEN_E820_HOST:
+ case VIR_DOMAIN_XEN_GFX_PASSTHRU:
break;
case VIR_DOMAIN_XEN_PASSTHROUGH:
@@ -29235,6 +29238,7 @@ virDomainDefFormatFeatures(virBufferPtr buf,
switch ((virDomainXen) j) {
case VIR_DOMAIN_XEN_E820_HOST:
+ case VIR_DOMAIN_XEN_GFX_PASSTHRU:
virBufferAddLit(&childBuf, "/>\n");
break;
case VIR_DOMAIN_XEN_PASSTHROUGH:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 4afd8f04bc..f28f0741ac 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1867,6 +1867,7 @@ typedef enum {
typedef enum {
VIR_DOMAIN_XEN_E820_HOST = 0,
VIR_DOMAIN_XEN_PASSTHROUGH,
+ VIR_DOMAIN_XEN_GFX_PASSTHRU,
VIR_DOMAIN_XEN_LAST
} virDomainXen;
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 458dfc2399..a5605f6200 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -679,6 +679,19 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
return -1;
}
#endif
+ if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) {
+ switch ((virTristateSwitch)
def->xen_features[VIR_DOMAIN_XEN_GFX_PASSTHRU]) {
+ case VIR_TRISTATE_SWITCH_ON:
+ libxl_defbool_set(&b_info->u.hvm.gfx_passthru, true);
+ break;
+ case VIR_TRISTATE_SWITCH_OFF:
+ libxl_defbool_set(&b_info->u.hvm.gfx_passthru, false);
+ break;
+ case VIR_TRISTATE_SWITCH_ABSENT:
+ case VIR_TRISTATE_SWITCH_LAST:
+ break;
+ }
+ }
} else if (pvh) {
b_info->cmdline = g_strdup(def->os.cmdline);
b_info->kernel = g_strdup(def->os.kernel);
--
2.26.2
11:41 a.m.
On 4/30/20 6:07 AM, Artur Puzio wrote:
gfx_passthru xl.cfg option enables GPU specific quirks required for
working
Intel GPU passthru. Qemu (used for device model by xen) will refuse to start
a VM when an IGD is passed, but this option was not set in Xen.
Do we really need to expose this setting to the user? I'm not really sure what
to think about it after reading the xl.cfg(5) man page. It starts with
gfx_passthru=BOOLEAN|"STRING"
The setting can be a boolean or a string - nice. And it really seems specific to
Intel graphics cards. The man page claims that a value of 1 "Enables graphics
device PCI passthrough and autodetects the type of device which is being used.".
It also says "Note that some graphics cards (AMD/ATI cards, for example) do not
necessarily require the gfx_passthru option".
Can't libxl just enable this itself if there's a PCI passthrough device and
it's
detected as type igd?
Regards,
Jim
Signed-off-by: Artur Puzio <contact(a)puzio.waw.pl>
---
docs/formatdomain.html.in | 7 +++++++
docs/schemas/domaincommon.rng | 5 +++++
src/conf/domain_conf.c | 4 ++++
src/conf/domain_conf.h | 1 +
src/libxl/libxl_conf.c | 13 +++++++++++++
5 files changed, 30 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 91d6f6c0d3..5307844a23 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2064,6 +2064,7 @@
<xen>
<e820_host state='on'/>
<passthrough state='on' mode='share_pt'/>
+ <gfx_passthru state='on'/>
</xen>
<pvspinlock state='on'/>
<gic version='2'/>
@@ -2270,6 +2271,12 @@
<td>on, off; mode - optional string sync_pt or share_pt</td>
<td><span class="since">6.3.0</span></td>
</tr>
+ <tr>
+ <td>gfx_passthru</td>
+ <td>Enable Intel GPU specific quirks. Required and allowed only when
passing an IGD.</td>
+ <td>on, off</td>
+ <td><span class="since">6.3.0</span></td>
+ </tr>
</table>
</dd>
<dt><code>pmu</code></dt>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 9d60b090f3..7d8ea879a1 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -6409,6 +6409,11 @@
</optional>
</element>
</optional>
+ <optional>
+ <element name="gfx_passthru">
+ <ref name="featurestate"/>
+ </element>
+ </optional>
</interleave>
</element>
</define>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8a87586936..75f72ff64c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -214,6 +214,7 @@ VIR_ENUM_IMPL(virDomainXen,
VIR_DOMAIN_XEN_LAST,
"e820_host",
"passthrough",
+ "gfx_passthru"
);
VIR_ENUM_IMPL(virDomainXenPassthroughMode,
@@ -19649,6 +19650,7 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
switch ((virDomainXen) feature) {
case VIR_DOMAIN_XEN_E820_HOST:
+ case VIR_DOMAIN_XEN_GFX_PASSTHRU:
break;
case VIR_DOMAIN_XEN_PASSTHROUGH:
@@ -23579,6 +23581,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src,
}
switch ((virDomainXen) i) {
case VIR_DOMAIN_XEN_E820_HOST:
+ case VIR_DOMAIN_XEN_GFX_PASSTHRU:
break;
case VIR_DOMAIN_XEN_PASSTHROUGH:
@@ -29235,6 +29238,7 @@ virDomainDefFormatFeatures(virBufferPtr buf,
switch ((virDomainXen) j) {
case VIR_DOMAIN_XEN_E820_HOST:
+ case VIR_DOMAIN_XEN_GFX_PASSTHRU:
virBufferAddLit(&childBuf, "/>\n");
break;
case VIR_DOMAIN_XEN_PASSTHROUGH:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 4afd8f04bc..f28f0741ac 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1867,6 +1867,7 @@ typedef enum {
typedef enum {
VIR_DOMAIN_XEN_E820_HOST = 0,
VIR_DOMAIN_XEN_PASSTHROUGH,
+ VIR_DOMAIN_XEN_GFX_PASSTHRU,
VIR_DOMAIN_XEN_LAST
} virDomainXen;
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 458dfc2399..a5605f6200 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -679,6 +679,19 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
return -1;
}
#endif
+ if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) {
+ switch ((virTristateSwitch)
def->xen_features[VIR_DOMAIN_XEN_GFX_PASSTHRU]) {
+ case VIR_TRISTATE_SWITCH_ON:
+ libxl_defbool_set(&b_info->u.hvm.gfx_passthru, true);
+ break;
+ case VIR_TRISTATE_SWITCH_OFF:
+ libxl_defbool_set(&b_info->u.hvm.gfx_passthru, false);
+ break;
+ case VIR_TRISTATE_SWITCH_ABSENT:
+ case VIR_TRISTATE_SWITCH_LAST:
+ break;
+ }
+ }
} else if (pvh) {
b_info->cmdline = g_strdup(def->os.cmdline);
b_info->kernel = g_strdup(def->os.kernel);
8:47 a.m.
On 08.05.2020 18:41, Jim Fehlig wrote:
On 4/30/20 6:07 AM, Artur Puzio wrote:
> gfx_passthru xl.cfg option enables GPU specific quirks required for
> working
> Intel GPU passthru. Qemu (used for device model by xen) will refuse
> to start
> a VM when an IGD is passed, but this option was not set in Xen.
Do we really need to expose this setting to the user? I'm not really
sure what to think about it after reading the xl.cfg(5) man page. It
starts with
gfx_passthru=BOOLEAN|"STRING"
The setting can be a boolean or a string - nice. And it really seems
specific to Intel graphics cards. The man page claims that a value of
1 "Enables graphics device PCI passthrough and autodetects the type of
device which is being used.". It also says "Note that some graphics
cards (AMD/ATI cards, for example) do not necessarily require the
gfx_passthru option".
Can't libxl just enable this itself if there's a PCI passthrough
device and it's detected as type igd?
Regards,
Jim
Hi, sorry for slowish response.
Setting gfx_passthru to 1 as in this patch enables the autodetection
routines (it's equivalent to gfx_passthru=1 from xl.cfg). Currently
libxl handles only IGD. If there is no IGD specified at creation and
gfx_passthru is 1, autodetection routines will report an error
preventing domain creation. Relevant libxl code:
libxl_dm.c:1746
if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
enum libxl_gfx_passthru_kind gfx_passthru_kind =
libxl__detect_gfx_passthru_kind(gc,
guest_config);
switch (gfx_passthru_kind) {
case LIBXL_GFX_PASSTHRU_KIND_IGD:
machinearg = GCSPRINTF("%s,igd-passthru=on", machinearg);
break;
case LIBXL_GFX_PASSTHRU_KIND_DEFAULT:
LOGD(ERROR, guest_domid, "unable to detect required
gfx_passthru_kind");
return ERROR_FAIL;
default:
LOGD(ERROR, guest_domid, "invalid value for
gfx_passthru_kind");
return ERROR_INVAL;
}
}
libxl contains a function for checking if IGD is present. After some
testing I will send another version of this patch that will enable
gfx_passthru if there is a IGD attached.
Artur
>
> Signed-off-by: Artur Puzio <contact(a)puzio.waw.pl>
> ---
> docs/formatdomain.html.in | 7 +++++++
> docs/schemas/domaincommon.rng | 5 +++++
> src/conf/domain_conf.c | 4 ++++
> src/conf/domain_conf.h | 1 +
> src/libxl/libxl_conf.c | 13 +++++++++++++
> 5 files changed, 30 insertions(+)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 91d6f6c0d3..5307844a23 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -2064,6 +2064,7 @@
> <xen>
> <e820_host state='on'/>
> <passthrough state='on' mode='share_pt'/>
> + <gfx_passthru state='on'/>
> </xen>
> <pvspinlock state='on'/>
> <gic version='2'/>
> @@ -2270,6 +2271,12 @@
> <td>on, off; mode - optional string sync_pt or share_pt</td>
> <td><span
class="since">6.3.0</span></td>
> </tr>
> + <tr>
> + <td>gfx_passthru</td>
> + <td>Enable Intel GPU specific quirks. Required and allowed
> only when passing an IGD.</td>
> + <td>on, off</td>
> + <td><span
class="since">6.3.0</span></td>
> + </tr>
> </table>
> </dd>
> <dt><code>pmu</code></dt>
> diff --git a/docs/schemas/domaincommon.rng
> b/docs/schemas/domaincommon.rng
> index 9d60b090f3..7d8ea879a1 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -6409,6 +6409,11 @@
> </optional>
> </element>
> </optional>
> + <optional>
> + <element name="gfx_passthru">
> + <ref name="featurestate"/>
> + </element>
> + </optional>
> </interleave>
> </element>
> </define>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 8a87586936..75f72ff64c 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -214,6 +214,7 @@ VIR_ENUM_IMPL(virDomainXen,
> VIR_DOMAIN_XEN_LAST,
> "e820_host",
> "passthrough",
> + "gfx_passthru"
> );
> VIR_ENUM_IMPL(virDomainXenPassthroughMode,
> @@ -19649,6 +19650,7 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
> switch ((virDomainXen) feature) {
> case VIR_DOMAIN_XEN_E820_HOST:
> + case VIR_DOMAIN_XEN_GFX_PASSTHRU:
> break;
> case VIR_DOMAIN_XEN_PASSTHROUGH:
> @@ -23579,6 +23581,7 @@
> virDomainDefFeaturesCheckABIStability(virDomainDefPtr src,
> }
> switch ((virDomainXen) i) {
> case VIR_DOMAIN_XEN_E820_HOST:
> + case VIR_DOMAIN_XEN_GFX_PASSTHRU:
> break;
> case VIR_DOMAIN_XEN_PASSTHROUGH:
> @@ -29235,6 +29238,7 @@ virDomainDefFormatFeatures(virBufferPtr buf,
> switch ((virDomainXen) j) {
> case VIR_DOMAIN_XEN_E820_HOST:
> + case VIR_DOMAIN_XEN_GFX_PASSTHRU:
> virBufferAddLit(&childBuf, "/>\n");
> break;
> case VIR_DOMAIN_XEN_PASSTHROUGH:
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 4afd8f04bc..f28f0741ac 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1867,6 +1867,7 @@ typedef enum {
> typedef enum {
> VIR_DOMAIN_XEN_E820_HOST = 0,
> VIR_DOMAIN_XEN_PASSTHROUGH,
> + VIR_DOMAIN_XEN_GFX_PASSTHRU,
> VIR_DOMAIN_XEN_LAST
> } virDomainXen;
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 458dfc2399..a5605f6200 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -679,6 +679,19 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
> return -1;
> }
> #endif
> + if (def->features[VIR_DOMAIN_FEATURE_XEN] ==
> VIR_TRISTATE_SWITCH_ON) {
> + switch ((virTristateSwitch)
> def->xen_features[VIR_DOMAIN_XEN_GFX_PASSTHRU]) {
> + case VIR_TRISTATE_SWITCH_ON:
> + libxl_defbool_set(&b_info->u.hvm.gfx_passthru,
> true);
> + break;
> + case VIR_TRISTATE_SWITCH_OFF:
> + libxl_defbool_set(&b_info->u.hvm.gfx_passthru,
> false);
> + break;
> + case VIR_TRISTATE_SWITCH_ABSENT:
> + case VIR_TRISTATE_SWITCH_LAST:
> + break;
> + }
> + }
> } else if (pvh) {
> b_info->cmdline = g_strdup(def->os.cmdline);
> b_info->kernel = g_strdup(def->os.kernel);
>
6:56 a.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
On 12.05.2020 15:47, Artur Puzio wrote:
On 08.05.2020 18:41, Jim Fehlig wrote:
> On 4/30/20 6:07 AM, Artur Puzio wrote:
>> gfx_passthru xl.cfg option enables GPU specific quirks required for
>> working
>> Intel GPU passthru. Qemu (used for device model by xen) will refuse
>> to start
>> a VM when an IGD is passed, but this option was not set in Xen.
>
> Do we really need to expose this setting to the user? I'm not really
> sure what to think about it after reading the xl.cfg(5) man page. It
> starts with
>
> gfx_passthru=BOOLEAN|"STRING"
>
> The setting can be a boolean or a string - nice. And it really seems
> specific to Intel graphics cards. The man page claims that a value of
> 1 "Enables graphics device PCI passthrough and autodetects the type of
> device which is being used.". It also says "Note that some graphics
> cards (AMD/ATI cards, for example) do not necessarily require the
> gfx_passthru option".
>
> Can't libxl just enable this itself if there's a PCI passthrough
> device and it's detected as type igd?
>
> Regards,
> Jim
Hi, sorry for slowish response.
Setting gfx_passthru to 1 as in this patch enables the autodetection
routines (it's equivalent to gfx_passthru=1 from xl.cfg). Currently
libxl handles only IGD. If there is no IGD specified at creation and
gfx_passthru is 1, autodetection routines will report an error
preventing domain creation. Relevant libxl code:
libxl_dm.c:1746
if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
enum libxl_gfx_passthru_kind gfx_passthru_kind =
libxl__detect_gfx_passthru_kind(gc,
guest_config);
switch (gfx_passthru_kind) {
case LIBXL_GFX_PASSTHRU_KIND_IGD:
machinearg = GCSPRINTF("%s,igd-passthru=on", machinearg);
break;
case LIBXL_GFX_PASSTHRU_KIND_DEFAULT:
LOGD(ERROR, guest_domid, "unable to detect required
gfx_passthru_kind");
return ERROR_FAIL;
default:
LOGD(ERROR, guest_domid, "invalid value for
gfx_passthru_kind");
return ERROR_INVAL;
}
}
libxl contains a function for checking if IGD is present. After some
testing I will send another version of this patch that will enable
gfx_passthru if there is a IGD attached.
Artur
I have checked possible ways for detection and reusing existing libxl
functions is not possible. Most of relevant functions (including
libxl__detect_gfx_passthru_kind & libxl__is_igd_vga_passthru) use
libxl__gc (libxl garbage collector). AFAIK no libvirt code uses
libxl__gc. We probably shouldn't start doing that(?) Also
libxl__detect_gfx_passthru_kind is static.
So the only way would be to reimplement detection if IGD is attached.
Even if it would work the same as libxl detection, it would probably
stop in the future... I think just exposing the option to the user is
much, much easier and future proof.
Artur
>
>>
>> Signed-off-by: Artur Puzio <contact(a)puzio.waw.pl>
>> ---
>> docs/formatdomain.html.in | 7 +++++++
>> docs/schemas/domaincommon.rng | 5 +++++
>> src/conf/domain_conf.c | 4 ++++
>> src/conf/domain_conf.h | 1 +
>> src/libxl/libxl_conf.c | 13 +++++++++++++
>> 5 files changed, 30 insertions(+)
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index 91d6f6c0d3..5307844a23 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -2064,6 +2064,7 @@
>> <xen>
>> <e820_host state='on'/>
>> <passthrough state='on' mode='share_pt'/>
>> + <gfx_passthru state='on'/>
>> </xen>
>> <pvspinlock state='on'/>
>> <gic version='2'/>
>> @@ -2270,6 +2271,12 @@
>> <td>on, off; mode - optional string sync_pt or
share_pt</td>
>> <td><span
class="since">6.3.0</span></td>
>> </tr>
>> + <tr>
>> + <td>gfx_passthru</td>
>> + <td>Enable Intel GPU specific quirks. Required and allowed
>> only when passing an IGD.</td>
>> + <td>on, off</td>
>> + <td><span
class="since">6.3.0</span></td>
>> + </tr>
>> </table>
>> </dd>
>> <dt><code>pmu</code></dt>
>> diff --git a/docs/schemas/domaincommon.rng
>> b/docs/schemas/domaincommon.rng
>> index 9d60b090f3..7d8ea879a1 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -6409,6 +6409,11 @@
>> </optional>
>> </element>
>> </optional>
>> + <optional>
>> + <element name="gfx_passthru">
>> + <ref name="featurestate"/>
>> + </element>
>> + </optional>
>> </interleave>
>> </element>
>> </define>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 8a87586936..75f72ff64c 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -214,6 +214,7 @@ VIR_ENUM_IMPL(virDomainXen,
>> VIR_DOMAIN_XEN_LAST,
>> "e820_host",
>> "passthrough",
>> + "gfx_passthru"
>> );
>> VIR_ENUM_IMPL(virDomainXenPassthroughMode,
>> @@ -19649,6 +19650,7 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
>> switch ((virDomainXen) feature) {
>> case VIR_DOMAIN_XEN_E820_HOST:
>> + case VIR_DOMAIN_XEN_GFX_PASSTHRU:
>> break;
>> case VIR_DOMAIN_XEN_PASSTHROUGH:
>> @@ -23579,6 +23581,7 @@
>> virDomainDefFeaturesCheckABIStability(virDomainDefPtr src,
>> }
>> switch ((virDomainXen) i) {
>> case VIR_DOMAIN_XEN_E820_HOST:
>> + case VIR_DOMAIN_XEN_GFX_PASSTHRU:
>> break;
>> case VIR_DOMAIN_XEN_PASSTHROUGH:
>> @@ -29235,6 +29238,7 @@ virDomainDefFormatFeatures(virBufferPtr buf,
>> switch ((virDomainXen) j) {
>> case VIR_DOMAIN_XEN_E820_HOST:
>> + case VIR_DOMAIN_XEN_GFX_PASSTHRU:
>> virBufferAddLit(&childBuf, "/>\n");
>> break;
>> case VIR_DOMAIN_XEN_PASSTHROUGH:
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index 4afd8f04bc..f28f0741ac 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -1867,6 +1867,7 @@ typedef enum {
>> typedef enum {
>> VIR_DOMAIN_XEN_E820_HOST = 0,
>> VIR_DOMAIN_XEN_PASSTHROUGH,
>> + VIR_DOMAIN_XEN_GFX_PASSTHRU,
>> VIR_DOMAIN_XEN_LAST
>> } virDomainXen;
>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
>> index 458dfc2399..a5605f6200 100644
>> --- a/src/libxl/libxl_conf.c
>> +++ b/src/libxl/libxl_conf.c
>> @@ -679,6 +679,19 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
>> return -1;
>> }
>> #endif
>> + if (def->features[VIR_DOMAIN_FEATURE_XEN] ==
>> VIR_TRISTATE_SWITCH_ON) {
>> + switch ((virTristateSwitch)
>> def->xen_features[VIR_DOMAIN_XEN_GFX_PASSTHRU]) {
>> + case VIR_TRISTATE_SWITCH_ON:
>> + libxl_defbool_set(&b_info->u.hvm.gfx_passthru,
>> true);
>> + break;
>> + case VIR_TRISTATE_SWITCH_OFF:
>> + libxl_defbool_set(&b_info->u.hvm.gfx_passthru,
>> false);
>> + break;
>> + case VIR_TRISTATE_SWITCH_ABSENT:
>> + case VIR_TRISTATE_SWITCH_LAST:
>> + break;
>> + }
>> + }
>> } else if (pvh) {
>> b_info->cmdline = g_strdup(def->os.cmdline);
>> b_info->kernel = g_strdup(def->os.kernel);
>>
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEEO6ZYTXOQH2Fi5VSz2HdiIEz/5y8FAl674H8ACgkQ2HdiIEz/
5y/JBg//cxAc7VQ8iiY+Z+dNTGQPbU+uXmc1stToZQqxvVmJwI9KyMNT2uxPgpV4
jjL8B8RQ7IVlx9q11x9bXW4gwJ8FM63F1JsIzmBeLLyaFD+6KLnqpfZE0lCa5BYb
vZ8N+uSKda8yyCsssR5NJ47I6IwlDpTSK3dcMWmCxM389fbyHeKhCUi0pGCGpNEe
PTlPapqwdMzIOdQNEfJewZ/EtDOr/uw2aAZaywyiuDNtGVN+9eEwiuGUUm0tM9Za
k66F5c6gFqpwXYpYVl3jKC3O8oadkcOncYIy3CZ/2I3V6odRUAARsFmEm+zg9Yvl
hSyG7j0WBAUbH+uot7tmDyfnhskje4YFSOKbycILcOFPgtWkvVy3COPzxUz4MOSj
8hMd+9nBnEQXK5ZYkcX1KeDR0REcV7b5C9r7Rm3fJkZybjF62+G/aaoD0GP7sQtZ
4na5taIed/k/aHhu9x/T1Q1QsQOZ/B4NcStG6hh5vO5GkrUPG3fJBAykwFw/Gv/H
2+pG3Z2iOSSP4xtES+jCnQ1bajDX/Wo5xmHpi6FUTGpeR7ObXxL38UrLrrs/pB6y
gaYDvOo+itY3hIKnHb9EmRfU/4Gm43xsBe/xp36LRSZRF3Q27SBzy061oQHI9ZYP
h4zvHLu8FN/0XBP+kq+fomkmEWVjgYpH0SVGvb/k8ealHCdZKW0=
=az2+
-----END PGP SIGNATURE----- 
8:41 p.m.
On Wed, May 13, 2020 at 01:56:52PM +0200, Artur Puzio wrote:
On 12.05.2020 15:47, Artur Puzio wrote:
> On 08.05.2020 18:41, Jim Fehlig wrote:
>> On 4/30/20 6:07 AM, Artur Puzio wrote:
>>> gfx_passthru xl.cfg option enables GPU specific quirks required for
>>> working
>>> Intel GPU passthru. Qemu (used for device model by xen) will refuse
>>> to start
>>> a VM when an IGD is passed, but this option was not set in Xen.
>>
>> Do we really need to expose this setting to the user? I'm not really
>> sure what to think about it after reading the xl.cfg(5) man page. It
>> starts with
>>
>>Â Â gfx_passthru=BOOLEAN|"STRING"
>>
>> The setting can be a boolean or a string - nice. And it really seems
>> specific to Intel graphics cards. The man page claims that a value of
>> 1 "Enables graphics device PCI passthrough and autodetects the type of
>> device which is being used.". It also says "Note that some graphics
>> cards (AMD/ATI cards, for example) do not necessarily require the
>> gfx_passthru option".
>>
>> Can't libxl just enable this itself if there's a PCI passthrough
>> device and it's detected as type igd?
I have checked possible ways for detection and reusing existing libxl
functions is not possible. Most of relevant functions (including
libxl__detect_gfx_passthru_kind & libxl__is_igd_vga_passthru) use
libxl__gc (libxl garbage collector). AFAIK no libvirt code uses
libxl__gc. We probably shouldn't start doing that(?) Also
libxl__detect_gfx_passthru_kind is static.
So the only way would be to reimplement detection if IGD is attached.
Even if it would work the same as libxl detection, it would probably
stop in the future... I think just exposing the option to the user is
much, much easier and future proof.
Existing strategies:
1. libxl__is_igd_vga_passthru checks for Intel device (0x8086 vendor) with
VGA class (0x030000). This should be easy to duplicate using libvirt
API, like virPCIDeviceReadID(dev, "vendor") and same for "class".
2. qemu uses a different approach and checks for device being
0000:00:02.0. Again, simple to duplicate into libvirt.
As we can see above, there are already two approaches, and it isn't
exactly clear which should be used. In fact, I think if those two
strategies won't match at some point, the domain may fail to start,
because qemu refuses to start with IGD attached (according to its
definition) but without the option enabled.
Looking at the libxl code, the only thing that option is used for, is to
add it to the qemu command line (Artur, have I missed anything?). In
that case, I think ideal solution would be to patch it at the qemu
level, to enable the option automatically. I believe this is exactly
the case for KVM case (vfio-pci). But I think that's non-trivial
approach in Xen path (if it would be easy, I think we wouldn't have this
option in the first place). For example the option makes qemu choose a
different PCI bridge configuration, which is I think done before
checking if IGD is attached. So, while there maybe a technical reason
for having this option in qmeu (instead of auto-detection), I don't
really see why user needs to provide it manually.
Since libxl already has a function to detect IGD, then adding
autodetection there would be a better idea. I don't know what libxl
maintainers would prefer, but I guess either of "not failing if
gfx_passthru=1 but IGD isn't detected" or adding "gfx_passthru=auto".
BTW Here is the discussion when the option was added to qemu:
https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg02756.html
It may be quite helpful to understand what is it about, but also why
(for example I see mentions of some specific Windows drivers).
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
7:50 a.m.
On 14.05.2020 03:41, Marek Marczykowski-Górecki wrote:
On Wed, May 13, 2020 at 01:56:52PM +0200, Artur Puzio wrote:
>
> On 12.05.2020 15:47, Artur Puzio wrote:
>> On 08.05.2020 18:41, Jim Fehlig wrote:
>>> On 4/30/20 6:07 AM, Artur Puzio wrote:
>>>> gfx_passthru xl.cfg option enables GPU specific quirks required for
>>>> working
>>>> Intel GPU passthru. Qemu (used for device model by xen) will refuse
>>>> to start
>>>> a VM when an IGD is passed, but this option was not set in Xen.
>>> Do we really need to expose this setting to the user? I'm not really
>>> sure what to think about it after reading the xl.cfg(5) man page. It
>>> starts with
>>>
>>> Â Â gfx_passthru=BOOLEAN|"STRING"
>>>
>>> The setting can be a boolean or a string - nice. And it really seems
>>> specific to Intel graphics cards. The man page claims that a value of
>>> 1 "Enables graphics device PCI passthrough and autodetects the type of
>>> device which is being used.". It also says "Note that some
graphics
>>> cards (AMD/ATI cards, for example) do not necessarily require the
>>> gfx_passthru option".
>>>
>>> Can't libxl just enable this itself if there's a PCI passthrough
>>> device and it's detected as type igd?
> I have checked possible ways for detection and reusing existing libxl
> functions is not possible. Most of relevant functions (including
> libxl__detect_gfx_passthru_kind & libxl__is_igd_vga_passthru) use
> libxl__gc (libxl garbage collector). AFAIK no libvirt code uses
> libxl__gc. We probably shouldn't start doing that(?) Also
> libxl__detect_gfx_passthru_kind is static.
>
> So the only way would be to reimplement detection if IGD is attached.
> Even if it would work the same as libxl detection, it would probably
> stop in the future... I think just exposing the option to the user is
> much, much easier and future proof.
Existing strategies:
1. libxl__is_igd_vga_passthru checks for Intel device (0x8086 vendor) with
VGA class (0x030000). This should be easy to duplicate using libvirt
API, like virPCIDeviceReadID(dev, "vendor") and same for "class".
2. qemu uses a different approach and checks for device being
0000:00:02.0. Again, simple to duplicate into libvirt.
As we can see above, there are already two approaches, and it isn't
exactly clear which should be used. In fact, I think if those two
strategies won't match at some point, the domain may fail to start,
because qemu refuses to start with IGD attached (according to its
definition) but without the option enabled.
Looking at the libxl code, the only thing that option is used for, is to
add it to the qemu command line (Artur, have I missed anything?).
It's also used in libxl_pci.c:2449 in func:
libxl__grant_vga_iomem_permission. This check is not really necessary as
libxl__grant_vga_iomem_permission already grants VGA IO memory
permissions only if it finds a VGA class device.
In that case, I think ideal solution would be to patch it at the
qemu
level, to enable the option automatically. I believe this is exactly
the case for KVM case (vfio-pci). But I think that's non-trivial
approach in Xen path (if it would be easy, I think we wouldn't have this
option in the first place). For example the option makes qemu choose a
different PCI bridge configuration, which is I think done before
checking if IGD is attached. So, while there maybe a technical reason
for having this option in qmeu (instead of auto-detection), I don't
really see why user needs to provide it manually.
Since libxl already has a function to detect IGD, then adding
autodetection there would be a better idea. I don't know what libxl
maintainers would prefer, but I guess either of "not failing if
gfx_passthru=1 but IGD isn't detected" or adding "gfx_passthru=auto".
I will go ahead and contact libxl maintainers if they would agree to
some autodetection solution. Maybe in the mean time we could expose this
option to the user in case he want's to disable/enable the autodetection
explicitly? When Xen will have some autodetection we could change the
default.
BTW Here is the discussion when the option was added to qemu:
https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg02756.html
It may be quite helpful to understand what is it about, but also why
(for example I see mentions of some specific Windows drivers).
1685
days inactive
1699
days old
5 comments
3 participants
participants (3)
-
Artur Puzio -
Jim Fehlig -
Marek Marczykowski-Górecki