This is a note to let you know that I've just added the patch titled
Revert "usbip: Implement a match function to fix usbip"
to my usb git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
in the usb-linus branch.
The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)
The patch will hopefully also be merged in Linus's tree for the
next -rc kernel release.
If you have any questions about this process, please let me know.
>From d6407613c1e2ef90213dee388aa16b6e1bd08cbc Mon Sep 17 00:00:00 2001
From: "M. Vefa Bicakci" <m.v.b(a)runbox.com>
Date: Tue, 22 Sep 2020 14:07:00 +0300
Subject: Revert "usbip: Implement a match function to fix usbip"
This commit reverts commit 7a2f2974f265 ("usbip: Implement a match
function to fix usbip").
In summary, commit d5643d2249b2 ("USB: Fix device driver race")
inadvertently broke usbip functionality, which I resolved in an incorrect
manner by introducing a match function to usbip, usbip_match(), that
unconditionally returns true.
However, the usbip_match function, as is, causes usbip to take over
virtual devices used by syzkaller for USB fuzzing, which is a regression
reported by Andrey Konovalov.
Furthermore, in conjunction with the fix of another bug, handled by another
patch titled "usbcore/driver: Fix specific driver selection" in this patch
set, the usbip_match function causes unexpected USB subsystem behaviour
when the usbip_host driver is loaded. The unexpected behaviour can be
qualified as follows:
- If commit 41160802ab8e ("USB: Simplify USB ID table match") is included
in the kernel, then all USB devices are bound to the usbip_host
driver, which appears to the user as if all USB devices were
disconnected.
- If the same commit (41160802ab8e) is not in the kernel (as is the case
with v5.8.10) then all USB devices are re-probed and re-bound to their
original device drivers, which appears to the user as a disconnection
and re-connection of USB devices.
Please note that this commit will make usbip non-operational again,
until yet another patch in this patch set is merged, titled
"usbcore/driver: Accommodate usbip".
Cc: <stable(a)vger.kernel.org> # 5.8: 41160802ab8e: USB: Simplify USB ID table match
Cc: <stable(a)vger.kernel.org> # 5.8
Cc: Bastien Nocera <hadess(a)hadess.net>
Cc: Valentina Manea <valentina.manea.m(a)gmail.com>
Cc: Shuah Khan <shuah(a)kernel.org>
Cc: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
Cc: Alan Stern <stern(a)rowland.harvard.edu>
Cc: <syzkaller(a)googlegroups.com>
Reported-by: Andrey Konovalov <andreyknvl(a)google.com>
Tested-by: Andrey Konovalov <andreyknvl(a)google.com>
Acked-by: Shuah Khan <skhan(a)linuxfoundation.org>
Signed-off-by: M. Vefa Bicakci <m.v.b(a)runbox.com>
Link: https://lore.kernel.org/r/20200922110703.720960-2-m.v.b@runbox.com
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
---
drivers/usb/usbip/stub_dev.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/usb/usbip/stub_dev.c b/drivers/usb/usbip/stub_dev.c
index 9d7d642022d1..2305d425e6c9 100644
--- a/drivers/usb/usbip/stub_dev.c
+++ b/drivers/usb/usbip/stub_dev.c
@@ -461,11 +461,6 @@ static void stub_disconnect(struct usb_device *udev)
return;
}
-static bool usbip_match(struct usb_device *udev)
-{
- return true;
-}
-
#ifdef CONFIG_PM
/* These functions need usb_port_suspend and usb_port_resume,
@@ -491,7 +486,6 @@ struct usb_device_driver stub_driver = {
.name = "usbip-host",
.probe = stub_probe,
.disconnect = stub_disconnect,
- .match = usbip_match,
#ifdef CONFIG_PM
.suspend = stub_suspend,
.resume = stub_resume,
--
2.28.0
Currently, we check we can send a pulse prior to disabling the
heartbeat to verify that we can change the heartbeat, but since we may
re-evaluate execution upon changing the heartbeat interval we need another
pulse afterwards to refresh execution.
Fixes: 9a40bddd47ca ("drm/i915/gt: Expose heartbeat interval via sysfs")
Signed-off-by: Chris Wilson <chris(a)chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen(a)linux.intel.com>
Cc: <stable(a)vger.kernel.org> # v5.7+
---
drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
index 8ffdf676c0a0..d09df370f7cd 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
@@ -192,10 +192,12 @@ int intel_engine_set_heartbeat(struct intel_engine_cs *engine,
WRITE_ONCE(engine->props.heartbeat_interval_ms, delay);
if (intel_engine_pm_get_if_awake(engine)) {
- if (delay)
+ if (delay) {
intel_engine_unpark_heartbeat(engine);
- else
+ } else {
intel_engine_park_heartbeat(engine);
+ intel_engine_pulse(engine); /* recheck execution */
+ }
intel_engine_pm_put(engine);
}
--
2.20.1
When running as Xen dom0 the kernel isn't responsible for selecting the
error handling mode, this should be handled by the hypervisor.
So disable setting FF mode when running as Xen pv guest. Not doing so
might result in boot splats like:
[ 7.509696] HEST: Enabling Firmware First mode for corrected errors.
[ 7.510382] mce: [Firmware Bug]: Ignoring request to disable invalid MCA bank 2.
[ 7.510383] mce: [Firmware Bug]: Ignoring request to disable invalid MCA bank 3.
[ 7.510384] mce: [Firmware Bug]: Ignoring request to disable invalid MCA bank 4.
[ 7.510384] mce: [Firmware Bug]: Ignoring request to disable invalid MCA bank 5.
[ 7.510385] mce: [Firmware Bug]: Ignoring request to disable invalid MCA bank 6.
[ 7.510386] mce: [Firmware Bug]: Ignoring request to disable invalid MCA bank 7.
[ 7.510386] mce: [Firmware Bug]: Ignoring request to disable invalid MCA bank 8.
Reason is that the HEST ACPI table contains the real number of MCA
banks, while the hypervisor is emulating only 2 banks for guests.
Cc: stable(a)vger.kernel.org
Signed-off-by: Juergen Gross <jgross(a)suse.com>
---
arch/x86/xen/enlighten_pv.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 22e741e0b10c..065c049d0f3c 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -1296,6 +1296,14 @@ asmlinkage __visible void __init xen_start_kernel(void)
xen_smp_init();
+#ifdef CONFIG_ACPI
+ /*
+ * Disable selecting "Firmware First mode" for correctable memory
+ * errors, as this is the duty of the hypervisor to decide.
+ */
+ acpi_disable_cmcff = 1;
+#endif
+
#ifdef CONFIG_ACPI_NUMA
/*
* The pages we from Xen are not related to machine pages, so
--
2.26.2
When using 128-bit interrupt-remapping table entry (IRTE) (a.k.a GA mode),
current driver disables interrupt remapping when it updates the IRTE
so that the upper and lower 64-bit values can be updated safely.
However, this creates a small window, where the interrupt could
arrive and result in IO_PAGE_FAULT (for interrupt) as shown below.
IOMMU Driver Device IRQ
============ ===========
irte.RemapEn=0
...
change IRTE IRQ from device ==> IO_PAGE_FAULT !!
...
irte.RemapEn=1
This scenario has been observed when changing irq affinity on a system
running I/O-intensive workload, in which the destination APIC ID
in the IRTE is updated.
Instead, use cmpxchg_double() to update the 128-bit IRTE at once without
disabling the interrupt remapping. However, this means several features,
which require GA (128-bit IRTE) support will also be affected if cmpxchg16b
is not supported (which is unprecedented for AMD processors w/ IOMMU).
Cc: stable(a)vger.kernel.org
Fixes: 880ac60e2538 ("iommu/amd: Introduce interrupt remapping ops structure")
Reported-by: Sean Osborne <sean.m.osborne(a)oracle.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit(a)amd.com>
Tested-by: Erik Rockstrom <erik.rockstrom(a)oracle.com>
Reviewed-by: Joao Martins <joao.m.martins(a)oracle.com>
Link: https://lore.kernel.org/r/20200903093822.52012-3-suravee.suthikulpanit@amd.…
Signed-off-by: Joerg Roedel <jroedel(a)suse.de>
---
Note: This patch is the back-port on top of the stable branch linux-5.4.y
for the upstream commit e52d58d54a32 ("iommu/amd: Use cmpxchg_double() when
updating 128-bit IRTE") since the original patch does not apply cleanly.
drivers/iommu/Kconfig | 2 +-
drivers/iommu/amd_iommu.c | 17 +++++++++++++----
drivers/iommu/amd_iommu_init.c | 21 +++++++++++++++++++--
3 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 390568afee9f..fc0160e8ed33 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -138,7 +138,7 @@ config AMD_IOMMU
select PCI_PASID
select IOMMU_API
select IOMMU_IOVA
- depends on X86_64 && PCI && ACPI
+ depends on X86_64 && PCI && ACPI && HAVE_CMPXCHG_DOUBLE
---help---
With this option you can enable support for AMD IOMMU hardware in
your system. An IOMMU is a hardware component which provides
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index fa91d856a43e..7b724f7b27a9 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3873,6 +3873,7 @@ static int alloc_irq_index(u16 devid, int count, bool align,
static int modify_irte_ga(u16 devid, int index, struct irte_ga *irte,
struct amd_ir_data *data)
{
+ bool ret;
struct irq_remap_table *table;
struct amd_iommu *iommu;
unsigned long flags;
@@ -3890,10 +3891,18 @@ static int modify_irte_ga(u16 devid, int index, struct irte_ga *irte,
entry = (struct irte_ga *)table->table;
entry = &entry[index];
- entry->lo.fields_remap.valid = 0;
- entry->hi.val = irte->hi.val;
- entry->lo.val = irte->lo.val;
- entry->lo.fields_remap.valid = 1;
+
+ ret = cmpxchg_double(&entry->lo.val, &entry->hi.val,
+ entry->lo.val, entry->hi.val,
+ irte->lo.val, irte->hi.val);
+ /*
+ * We use cmpxchg16 to atomically update the 128-bit IRTE,
+ * and it cannot be updated by the hardware or other processors
+ * behind us, so the return value of cmpxchg16 should be the
+ * same as the old value.
+ */
+ WARN_ON(!ret);
+
if (data)
data->ref = entry;
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 135ae5222cf3..31d7e2d4f304 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1522,7 +1522,14 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
iommu->mmio_phys_end = MMIO_REG_END_OFFSET;
else
iommu->mmio_phys_end = MMIO_CNTR_CONF_OFFSET;
- if (((h->efr_attr & (0x1 << IOMMU_FEAT_GASUP_SHIFT)) == 0))
+
+ /*
+ * Note: GA (128-bit IRTE) mode requires cmpxchg16b supports.
+ * GAM also requires GA mode. Therefore, we need to
+ * check cmpxchg16b support before enabling it.
+ */
+ if (!boot_cpu_has(X86_FEATURE_CX16) ||
+ ((h->efr_attr & (0x1 << IOMMU_FEAT_GASUP_SHIFT)) == 0))
amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_LEGACY;
break;
case 0x11:
@@ -1531,8 +1538,18 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
iommu->mmio_phys_end = MMIO_REG_END_OFFSET;
else
iommu->mmio_phys_end = MMIO_CNTR_CONF_OFFSET;
- if (((h->efr_reg & (0x1 << IOMMU_EFR_GASUP_SHIFT)) == 0))
+
+ /*
+ * Note: GA (128-bit IRTE) mode requires cmpxchg16b supports.
+ * XT, GAM also requires GA mode. Therefore, we need to
+ * check cmpxchg16b support before enabling them.
+ */
+ if (!boot_cpu_has(X86_FEATURE_CX16) ||
+ ((h->efr_reg & (0x1 << IOMMU_EFR_GASUP_SHIFT)) == 0)) {
amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_LEGACY;
+ break;
+ }
+
/*
* Note: Since iommu_update_intcapxt() leverages
* the IOMMU MMIO access to MSI capability block registers
--
2.17.1
Sasha,
This patch and the upstream commit 26e495f34107 ("iommu/amd: Restore IRTE.RemapEn bit after programming IRTE")
are related. Since the commit 26e495f34107 has been back-ported to linux-5.4.y branch of the stable tree,
this patch should also be add to linux-5.4.y as well.
Thanks,
Suravee
On 9/7/20 10:05 AM, Sasha Levin wrote:
> This is a note to let you know that I've just added the patch titled
>
> iommu/amd: Use cmpxchg_double() when updating 128-bit IRTE
>
> to the 5.8-stable tree which can be found at:
> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.kernel…
>
> The filename of the patch is:
> iommu-amd-use-cmpxchg_double-when-updating-128-bit-i.patch
> and it can be found in the queue-5.8 subdirectory.
>
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable(a)vger.kernel.org> know about it.
>
>
>
> commit 31640157d429339f8fffc14a645cb2c9739e0f17
> Author: Suravee Suthikulpanit <suravee.suthikulpanit(a)amd.com>
> Date: Thu Sep 3 09:38:22 2020 +0000
>
> iommu/amd: Use cmpxchg_double() when updating 128-bit IRTE
>
> [ Upstream commit e52d58d54a321d4fe9d0ecdabe4f8774449f0d6e ]
>
> When using 128-bit interrupt-remapping table entry (IRTE) (a.k.a GA mode),
> current driver disables interrupt remapping when it updates the IRTE
> so that the upper and lower 64-bit values can be updated safely.
>
> However, this creates a small window, where the interrupt could
> arrive and result in IO_PAGE_FAULT (for interrupt) as shown below.
>
> IOMMU Driver Device IRQ
> ============ ===========
> irte.RemapEn=0
> ...
> change IRTE IRQ from device ==> IO_PAGE_FAULT !!
> ...
> irte.RemapEn=1
>
> This scenario has been observed when changing irq affinity on a system
> running I/O-intensive workload, in which the destination APIC ID
> in the IRTE is updated.
>
> Instead, use cmpxchg_double() to update the 128-bit IRTE at once without
> disabling the interrupt remapping. However, this means several features,
> which require GA (128-bit IRTE) support will also be affected if cmpxchg16b
> is not supported (which is unprecedented for AMD processors w/ IOMMU).
>
> Fixes: 880ac60e2538 ("iommu/amd: Introduce interrupt remapping ops structure")
> Reported-by: Sean Osborne <sean.m.osborne(a)oracle.com>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit(a)amd.com>
> Tested-by: Erik Rockstrom <erik.rockstrom(a)oracle.com>
> Reviewed-by: Joao Martins <joao.m.martins(a)oracle.com>
> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kern…
> Signed-off-by: Joerg Roedel <jroedel(a)suse.de>
> Signed-off-by: Sasha Levin <sashal(a)kernel.org>
>
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index b0f308cb7f7c2..201b2718f0755 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -143,7 +143,7 @@ config AMD_IOMMU
> select IOMMU_API
> select IOMMU_IOVA
> select IOMMU_DMA
> - depends on X86_64 && PCI && ACPI
> + depends on X86_64 && PCI && ACPI && HAVE_CMPXCHG_DOUBLE
> help
> With this option you can enable support for AMD IOMMU hardware in
> your system. An IOMMU is a hardware component which provides
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 6ebd4825e3206..bf45f8e2c7edd 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -1518,7 +1518,14 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
> iommu->mmio_phys_end = MMIO_REG_END_OFFSET;
> else
> iommu->mmio_phys_end = MMIO_CNTR_CONF_OFFSET;
> - if (((h->efr_attr & (0x1 << IOMMU_FEAT_GASUP_SHIFT)) == 0))
> +
> + /*
> + * Note: GA (128-bit IRTE) mode requires cmpxchg16b supports.
> + * GAM also requires GA mode. Therefore, we need to
> + * check cmpxchg16b support before enabling it.
> + */
> + if (!boot_cpu_has(X86_FEATURE_CX16) ||
> + ((h->efr_attr & (0x1 << IOMMU_FEAT_GASUP_SHIFT)) == 0))
> amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_LEGACY;
> break;
> case 0x11:
> @@ -1527,8 +1534,18 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
> iommu->mmio_phys_end = MMIO_REG_END_OFFSET;
> else
> iommu->mmio_phys_end = MMIO_CNTR_CONF_OFFSET;
> - if (((h->efr_reg & (0x1 << IOMMU_EFR_GASUP_SHIFT)) == 0))
> +
> + /*
> + * Note: GA (128-bit IRTE) mode requires cmpxchg16b supports.
> + * XT, GAM also requires GA mode. Therefore, we need to
> + * check cmpxchg16b support before enabling them.
> + */
> + if (!boot_cpu_has(X86_FEATURE_CX16) ||
> + ((h->efr_reg & (0x1 << IOMMU_EFR_GASUP_SHIFT)) == 0)) {
> amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_LEGACY;
> + break;
> + }
> +
> /*
> * Note: Since iommu_update_intcapxt() leverages
> * the IOMMU MMIO access to MSI capability block registers
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index d7b037891fb7e..200ee948f6ec1 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -3283,6 +3283,7 @@ out:
> static int modify_irte_ga(u16 devid, int index, struct irte_ga *irte,
> struct amd_ir_data *data)
> {
> + bool ret;
> struct irq_remap_table *table;
> struct amd_iommu *iommu;
> unsigned long flags;
> @@ -3300,10 +3301,18 @@ static int modify_irte_ga(u16 devid, int index, struct irte_ga *irte,
>
> entry = (struct irte_ga *)table->table;
> entry = &entry[index];
> - entry->lo.fields_remap.valid = 0;
> - entry->hi.val = irte->hi.val;
> - entry->lo.val = irte->lo.val;
> - entry->lo.fields_remap.valid = 1;
> +
> + ret = cmpxchg_double(&entry->lo.val, &entry->hi.val,
> + entry->lo.val, entry->hi.val,
> + irte->lo.val, irte->hi.val);
> + /*
> + * We use cmpxchg16 to atomically update the 128-bit IRTE,
> + * and it cannot be updated by the hardware or other processors
> + * behind us, so the return value of cmpxchg16 should be the
> + * same as the old value.
> + */
> + WARN_ON(!ret);
> +
> if (data)
> data->ref = entry;
>
>