Re: [Xen-devel] [PATCH] VT-d: don't panic/warn on iommu=no-igfx
Tian, Kevin: > > From: Rusty Bird [mailto:rustyb...@openmailbox.org] > > +if ( !iommu_igfx ) > > +return; > > A message might be also helpful here so user can confirm its > boot option takes effect... Done, see v2 patch. Thanks for the feedback! Rusty signature.asc Description: PGP signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] VT-d: don't panic/warn on iommu=no-igfx
When operating on an Intel graphics device, iommu_enable_translation()
panicked (force_iommu==1) or warned (force_iommu==0) about the BIOS if
is_igd_vt_enabled_quirk() returned 0. That's good if the actual BIOS
problem has been detected. But since commit 1463411, returning 0 could
also happen if the user simply passed "iommu=no-igfx", in which case
bailing out with an info message (instead of a panic/warning) would be
more appropriate.
The panic broke the combination "iommu=force,no-igfx", and also the case
where "iommu=no-igfx" is passed but force_iommu=1 is set automatically
by x2apic_bsp_setup().
Move the iommu_igfx check from is_igd_vt_enabled_quirk() into its only
caller iommu_enable_translation(), and tweak the logic.
Signed-off-by: Rusty Bird <rustyb...@openmailbox.org>
---
Notes:
Changed since v1: print info message when iommu_igfx==0
Best viewed with "git show --ignore-space-change --function-context"
xen/drivers/passthrough/vtd/iommu.c | 22 --
xen/drivers/passthrough/vtd/quirks.c | 3 ---
2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/xen/drivers/passthrough/vtd/iommu.c
b/xen/drivers/passthrough/vtd/iommu.c
index 19328f6..daaed0a 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -747,14 +747,24 @@ static void iommu_enable_translation(struct
acpi_drhd_unit *drhd)
unsigned long flags;
struct iommu *iommu = drhd->iommu;
-if ( is_igd_drhd(drhd) && !is_igd_vt_enabled_quirk() )
+if ( is_igd_drhd(drhd) )
{
-if ( force_iommu )
-panic("BIOS did not enable IGD for VT properly, crash Xen for
security purpose");
+if ( !iommu_igfx )
+{
+printk(XENLOG_INFO VTDPREFIX
+ "Passed iommu=no-igfx option. Disabling IGD VT-d
engine.\n");
+return;
+}
-printk(XENLOG_WARNING VTDPREFIX
- "BIOS did not enable IGD for VT properly. Disabling IGD VT-d
engine.\n");
-return;
+if ( !is_igd_vt_enabled_quirk() )
+{
+if ( force_iommu )
+panic("BIOS did not enable IGD for VT properly, crash Xen for
security purpose");
+
+printk(XENLOG_WARNING VTDPREFIX
+ "BIOS did not enable IGD for VT properly. Disabling IGD
VT-d engine.\n");
+return;
+}
}
/* apply platform specific errata workarounds */
diff --git a/xen/drivers/passthrough/vtd/quirks.c
b/xen/drivers/passthrough/vtd/quirks.c
index 91f96ac..5bbbd96 100644
--- a/xen/drivers/passthrough/vtd/quirks.c
+++ b/xen/drivers/passthrough/vtd/quirks.c
@@ -70,9 +70,6 @@ int is_igd_vt_enabled_quirk(void)
{
u16 ggc;
-if ( !iommu_igfx )
-return 0;
-
if ( !IS_ILK(ioh_id) )
return 1;
--
2.9.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] VT-d: don't panic/warn on iommu=no-igfx
When operating on an Intel graphics device, iommu_enable_translation()
panicked (force_iommu==1) or warned (force_iommu==0) about the BIOS if
is_igd_vt_enabled_quirk() returned 0. That's good if the actual BIOS
problem has been detected. But since commit 1463411, returning 0 could
also happen if the user simply passed "iommu=no-igfx", in which case
bailing out _without_ the panic/warning would be more appropriate.
The panic broke the combination "iommu=force,no-igfx", and also the case
where "iommu=no-igfx" is passed but force_iommu=1 is set automatically
by x2apic_bsp_setup().
Move the iommu_igfx check from is_igd_vt_enabled_quirk() into its only
caller iommu_enable_translation(), and tweak the logic.
Signed-off-by: Rusty Bird <rustyb...@openmailbox.org>
---
Notes:
Best viewed with "git show --ignore-space-change --function-context"
xen/drivers/passthrough/vtd/iommu.c | 18 --
xen/drivers/passthrough/vtd/quirks.c | 3 ---
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/xen/drivers/passthrough/vtd/iommu.c
b/xen/drivers/passthrough/vtd/iommu.c
index 19328f6..2849ea1 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -747,14 +747,20 @@ static void iommu_enable_translation(struct
acpi_drhd_unit *drhd)
unsigned long flags;
struct iommu *iommu = drhd->iommu;
-if ( is_igd_drhd(drhd) && !is_igd_vt_enabled_quirk() )
+if ( is_igd_drhd(drhd) )
{
-if ( force_iommu )
-panic("BIOS did not enable IGD for VT properly, crash Xen for
security purpose");
+if ( !iommu_igfx )
+return;
-printk(XENLOG_WARNING VTDPREFIX
- "BIOS did not enable IGD for VT properly. Disabling IGD VT-d
engine.\n");
-return;
+if ( !is_igd_vt_enabled_quirk() )
+{
+if ( force_iommu )
+panic("BIOS did not enable IGD for VT properly, crash Xen for
security purpose");
+
+printk(XENLOG_WARNING VTDPREFIX
+ "BIOS did not enable IGD for VT properly. Disabling IGD
VT-d engine.\n");
+return;
+}
}
/* apply platform specific errata workarounds */
diff --git a/xen/drivers/passthrough/vtd/quirks.c
b/xen/drivers/passthrough/vtd/quirks.c
index 91f96ac..5bbbd96 100644
--- a/xen/drivers/passthrough/vtd/quirks.c
+++ b/xen/drivers/passthrough/vtd/quirks.c
@@ -70,9 +70,6 @@ int is_igd_vt_enabled_quirk(void)
{
u16 ggc;
-if ( !iommu_igfx )
-return 0;
-
if ( !IS_ILK(ioh_id) )
return 1;
--
2.9.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] systemd: remove hard-coded pid file in xendriverdomain service
Wei Liu:
> On Tue, Aug 02, 2016 at 02:41:58PM +0100, Ian Jackson wrote:
>> Wei Liu writes ("[PATCH] systemd: remove hard-coded pid file in
>> xendriverdomain service"):
>>> Per the discussion in [0], the hard-coded pid file can be removed
>>> completely. Systemd has no trouble figuring out the pid of devd all by
>>> itself.
>>>
>>> [0]: https://lists.xen.org/archives/html/xen-devel/2016-07/msg01393.html
>>
>> I'm not qualified to have much of an opinion this but I'm happy to see
>> it go in based on what I assume is a test report from Rusty Bird ?
>>
>
> No, it's not a test report.
>
> Rusty Bird was the submitter of that xl devd unit file. I assumed he had
> experience of setting up xl devd without a pid file under systemd and
> did what he asked for.
Yes, I tried it without a PID file and did not notice any issues.
Rusty
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 0/7] Remove hard-coded /var/run in tools
Hello Wei, For systemd/xendriverdomain.service.in, the hardcoded PID file could be removed altogether -- systemd has no trouble figuring out the PID with only one process. But I wasn't sure if maybe something outside of the xen tree uses it? (Sorry that you're not getting my CCs: "554-SMTP.EU.CITRIX.COM 554 You are being rejected because your senderbase score is below our accepted policy. If you have any questions about your senderbase score, please goto http://www.senderbase.org; -- they don't like mail.openmailbox.org for some reason) Rusty signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] tools/hotplug: Add native systemd xendriverdomain.service
Wei Liu: >> diff --git a/tools/configure.ac b/tools/configure.ac >> index 8704927..e08fa8e 100644 >> --- a/tools/configure.ac >> +++ b/tools/configure.ac >> @@ -437,6 +437,7 @@ AS_IF([test "x$systemd" = "xy"], [ >> hotplug/Linux/systemd/xenconsoled.service >> hotplug/Linux/systemd/xendomains.service >> hotplug/Linux/systemd/xenstored.service >> +hotplug/Linux/systemd/xendriverdomain.service > > I failed to mention that I would like to sort this list alphabetically, > i.e. the new addition should be moved before xenstored.service. I can > make the adjustment while committing if you don't object. Of course, thank you! Rusty signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] tools/hotplug: Add native systemd xendriverdomain.service
Wei Liu: > While I understand it might be necessary to have a dedicated service > file for xendriverdomain, I can't seem to be able to figure out the > rationale from the commit message. > > After thinking a bit harder, may I suggest commit message like this (and > please correct me if I'm wrong): > > We need to have a dedicated service for xendriverdoamin in driver > domain. This patch creates a service file for it. This service is only > relevant to DomU. > > The service file uses ConditionVirtualization=xen becuase that evaluates > to false in Dom0 while true in DomU, so that we only starts this service > in DomU. Thanks for the feedback! I've sent a v2 patch with a less cryptic commit message that incorporates this information. Rusty signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] tools/hotplug: Add native systemd xendriverdomain.service
A dedicated Xen driver domain init service starts "xl devd" in domU. But currently, it is only supplied in the form of a SysV init script, which systemd users run through a backward compatiblity wrapper automatically generated by systemd-sysv-generator. This patch adds a (naturally more lightweight) native systemd unit to be used instead. The xendriverdomain service is only relevant to domU, but should not run in dom0. Therefore, the systemd unit uses "ConditionVirtualization=xen", which evaluates to true in domU and (since systemd version 214, released on 2014-06-11) to false in dom0. Users or distributors who need to be compatible with even older systemd versions, but still want to prevent "xl devd" startup in dom0, could add the following line in [Service]: ExecStartPre=/bin/sh -c "! grep -q control_d /proc/xen/capabilities" (Please rerun autogen.sh after applying this patch) Signed-off-by: Rusty Bird <rustyb...@openmailbox.org> Cc: Ian Jackson <ian.jack...@eu.citrix.com> Cc: Wei Liu <wei.l...@citrix.com> --- Changed since v1: * more detailed commit message tools/configure.ac | 1 + tools/hotplug/Linux/systemd/Makefile | 1 + tools/hotplug/Linux/systemd/xendriverdomain.service.in | 14 ++ 3 files changed, 16 insertions(+) create mode 100644 tools/hotplug/Linux/systemd/xendriverdomain.service.in diff --git a/tools/configure.ac b/tools/configure.ac index 8704927..e08fa8e 100644 --- a/tools/configure.ac +++ b/tools/configure.ac @@ -437,6 +437,7 @@ AS_IF([test "x$systemd" = "xy"], [ hotplug/Linux/systemd/xenconsoled.service hotplug/Linux/systemd/xendomains.service hotplug/Linux/systemd/xenstored.service +hotplug/Linux/systemd/xendriverdomain.service hotplug/Linux/systemd/xenstored.socket hotplug/Linux/systemd/xenstored_ro.socket ]) diff --git a/tools/hotplug/Linux/systemd/Makefile b/tools/hotplug/Linux/systemd/Makefile index 83e3b32..558e459 100644 --- a/tools/hotplug/Linux/systemd/Makefile +++ b/tools/hotplug/Linux/systemd/Makefile @@ -15,6 +15,7 @@ XEN_SYSTEMD_SERVICE += xen-qemu-dom0-disk-backend.service XEN_SYSTEMD_SERVICE += xendomains.service XEN_SYSTEMD_SERVICE += xen-watchdog.service XEN_SYSTEMD_SERVICE += xen-init-dom0.service +XEN_SYSTEMD_SERVICE += xendriverdomain.service ALL_XEN_SYSTEMD = $(XEN_SYSTEMD_MODULES) \ $(XEN_SYSTEMD_MOUNT)\ diff --git a/tools/hotplug/Linux/systemd/xendriverdomain.service.in b/tools/hotplug/Linux/systemd/xendriverdomain.service.in new file mode 100644 index 000..c0cd454 --- /dev/null +++ b/tools/hotplug/Linux/systemd/xendriverdomain.service.in @@ -0,0 +1,14 @@ +[Unit] +Description=Xen driver domain device daemon +DefaultDependencies=no +Requires=proc-xen.mount +After=proc-xen.mount +ConditionVirtualization=xen + +[Service] +Type=forking +ExecStart=@sbindir@/xl devd --pidfile=/var/run/xldevd.pid +PIDFile=/var/run/xldevd.pid + +[Install] +WantedBy=multi-user.target -- 2.5.5 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] tools/hotplug: Add native systemd xendriverdomain.service
Uses ConditionVirtualization=xen, which evaluates to false in dom0 since systemd 214 (released 2014-06-11). An alternative would be this line: ExecStartPre=/bin/sh -c "! grep -q control_d /proc/xen/capabilities" (Please rerun autogen.sh) Signed-off-by: Rusty Bird <rustyb...@openmailbox.org> Cc: Ian Jackson <ian.jack...@eu.citrix.com> Cc: Wei Liu <wei.l...@citrix.com> --- tools/configure.ac | 1 + tools/hotplug/Linux/systemd/Makefile | 1 + tools/hotplug/Linux/systemd/xendriverdomain.service.in | 14 ++ 3 files changed, 16 insertions(+) create mode 100644 tools/hotplug/Linux/systemd/xendriverdomain.service.in diff --git a/tools/configure.ac b/tools/configure.ac index 8704927..e08fa8e 100644 --- a/tools/configure.ac +++ b/tools/configure.ac @@ -437,6 +437,7 @@ AS_IF([test "x$systemd" = "xy"], [ hotplug/Linux/systemd/xenconsoled.service hotplug/Linux/systemd/xendomains.service hotplug/Linux/systemd/xenstored.service +hotplug/Linux/systemd/xendriverdomain.service hotplug/Linux/systemd/xenstored.socket hotplug/Linux/systemd/xenstored_ro.socket ]) diff --git a/tools/hotplug/Linux/systemd/Makefile b/tools/hotplug/Linux/systemd/Makefile index 83e3b32..558e459 100644 --- a/tools/hotplug/Linux/systemd/Makefile +++ b/tools/hotplug/Linux/systemd/Makefile @@ -15,6 +15,7 @@ XEN_SYSTEMD_SERVICE += xen-qemu-dom0-disk-backend.service XEN_SYSTEMD_SERVICE += xendomains.service XEN_SYSTEMD_SERVICE += xen-watchdog.service XEN_SYSTEMD_SERVICE += xen-init-dom0.service +XEN_SYSTEMD_SERVICE += xendriverdomain.service ALL_XEN_SYSTEMD = $(XEN_SYSTEMD_MODULES) \ $(XEN_SYSTEMD_MOUNT)\ diff --git a/tools/hotplug/Linux/systemd/xendriverdomain.service.in b/tools/hotplug/Linux/systemd/xendriverdomain.service.in new file mode 100644 index 000..c0cd454 --- /dev/null +++ b/tools/hotplug/Linux/systemd/xendriverdomain.service.in @@ -0,0 +1,14 @@ +[Unit] +Description=Xen driver domain device daemon +DefaultDependencies=no +Requires=proc-xen.mount +After=proc-xen.mount +ConditionVirtualization=xen + +[Service] +Type=forking +ExecStart=@sbindir@/xl devd --pidfile=/var/run/xldevd.pid +PIDFile=/var/run/xldevd.pid + +[Install] +WantedBy=multi-user.target -- 2.5.5 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
