* [PATCH v2 1/4] of: Return success from of_dma_set_restricted_buffer() when !OF_ADDRESS
2021-07-20 13:38 [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit() Will Deacon
@ 2021-07-20 13:38 ` Will Deacon
2021-07-20 13:38 ` [PATCH v2 2/4] swiotlb: Convert io_default_tlb_mem to static allocation Will Deacon
` (4 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Will Deacon @ 2021-07-20 13:38 UTC (permalink / raw)
To: iommu
Cc: linux-kernel, Will Deacon, Guenter Roeck, Claire Chang,
Christoph Hellwig, Robin Murphy, Konrad Rzeszutek Wilk,
Nathan Chancellor
When CONFIG_OF_ADDRESS=n, of_dma_set_restricted_buffer() returns -ENODEV
and breaks the boot for sparc[64] machines. Return 0 instead, since the
function is essentially a glorified NOP in this configuration.
Cc: Claire Chang <tientzu@chromium.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reported-by: Guenter Roeck <linux@roeck-us.net>
Suggested-by: Robin Murphy <robin.murphy@arm.com>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Claire Chang <tientzu@chromium.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Link: https://lore.kernel.org/r/20210702030807.GA2685166@roeck-us.net
Fixes: fec9b625095f ("of: Add plumbing for restricted DMA pool")
Signed-off-by: Will Deacon <will@kernel.org>
---
drivers/of/of_private.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 376462798f7e..f557bd22b0cf 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -173,7 +173,8 @@ static inline int of_dma_get_range(struct device_node *np,
static inline int of_dma_set_restricted_buffer(struct device *dev,
struct device_node *np)
{
- return -ENODEV;
+ /* Do nothing, successfully. */
+ return 0;
}
#endif
--
2.32.0.402.g57bb445576-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 2/4] swiotlb: Convert io_default_tlb_mem to static allocation
2021-07-20 13:38 [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit() Will Deacon
2021-07-20 13:38 ` [PATCH v2 1/4] of: Return success from of_dma_set_restricted_buffer() when !OF_ADDRESS Will Deacon
@ 2021-07-20 13:38 ` Will Deacon
2021-07-20 13:38 ` [PATCH v2 3/4] swiotlb: Emit diagnostic in swiotlb_exit() Will Deacon
` (3 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Will Deacon @ 2021-07-20 13:38 UTC (permalink / raw)
To: iommu
Cc: linux-kernel, Will Deacon, Guenter Roeck, Claire Chang,
Christoph Hellwig, Robin Murphy, Konrad Rzeszutek Wilk,
Nathan Chancellor
Since commit 69031f500865 ("swiotlb: Set dev->dma_io_tlb_mem to the
swiotlb pool used"), 'struct device' may hold a copy of the global
'io_default_tlb_mem' pointer if the device is using swiotlb for DMA. A
subsequent call to swiotlb_exit() will therefore leave dangling pointers
behind in these device structures, resulting in KASAN splats such as:
| BUG: KASAN: use-after-free in __iommu_dma_unmap_swiotlb+0x64/0xb0
| Read of size 8 at addr ffff8881d7830000 by task swapper/0/0
|
| CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.12.0-rc3-debug #1
| Hardware name: HP HP Desktop M01-F1xxx/87D6, BIOS F.12 12/17/2020
| Call Trace:
| <IRQ>
| dump_stack+0x9c/0xcf
| print_address_description.constprop.0+0x18/0x130
| kasan_report.cold+0x7f/0x111
| __iommu_dma_unmap_swiotlb+0x64/0xb0
| nvme_pci_complete_rq+0x73/0x130
| blk_complete_reqs+0x6f/0x80
| __do_softirq+0xfc/0x3be
Convert 'io_default_tlb_mem' to a static structure, so that the
per-device pointers remain valid after swiotlb_exit() has been invoked.
All users are updated to reference the static structure directly, using
the 'nslabs' field to determine whether swiotlb has been initialised.
The 'slots' array is still allocated dynamically and referenced via a
pointer rather than a flexible array member.
Cc: Claire Chang <tientzu@chromium.org>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Fixes: 69031f500865 ("swiotlb: Set dev->dma_io_tlb_mem to the swiotlb pool used")
Reported-by: Nathan Chancellor <nathan@kernel.org>
Tested-by: Nathan Chancellor <nathan@kernel.org>
Tested-by: Claire Chang <tientzu@chromium.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Will Deacon <will@kernel.org>
---
drivers/base/core.c | 2 +-
drivers/xen/swiotlb-xen.c | 4 +--
include/linux/swiotlb.h | 4 +--
kernel/dma/swiotlb.c | 66 +++++++++++++++++++++------------------
4 files changed, 41 insertions(+), 35 deletions(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index ea5b85354526..b49824001cfa 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2848,7 +2848,7 @@ void device_initialize(struct device *dev)
dev->dma_coherent = dma_default_coherent;
#endif
#ifdef CONFIG_SWIOTLB
- dev->dma_io_tlb_mem = io_tlb_default_mem;
+ dev->dma_io_tlb_mem = &io_tlb_default_mem;
#endif
}
EXPORT_SYMBOL_GPL(device_initialize);
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 785ec7e8be01..f06d9b4f1e0f 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -164,7 +164,7 @@ int __ref xen_swiotlb_init(void)
int rc = -ENOMEM;
char *start;
- if (io_tlb_default_mem != NULL) {
+ if (io_tlb_default_mem.nslabs) {
pr_warn("swiotlb buffer already initialized\n");
return -EEXIST;
}
@@ -547,7 +547,7 @@ xen_swiotlb_sync_sg_for_device(struct device *dev, struct scatterlist *sgl,
static int
xen_swiotlb_dma_supported(struct device *hwdev, u64 mask)
{
- return xen_phys_to_dma(hwdev, io_tlb_default_mem->end - 1) <= mask;
+ return xen_phys_to_dma(hwdev, io_tlb_default_mem.end - 1) <= mask;
}
const struct dma_map_ops xen_swiotlb_dma_ops = {
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 39284ff2a6cd..b0cb2a9973f4 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -103,9 +103,9 @@ struct io_tlb_mem {
phys_addr_t orig_addr;
size_t alloc_size;
unsigned int list;
- } slots[];
+ } *slots;
};
-extern struct io_tlb_mem *io_tlb_default_mem;
+extern struct io_tlb_mem io_tlb_default_mem;
static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
{
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index f1a9ae7fad8f..7948f274f9bb 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -70,7 +70,7 @@
enum swiotlb_force swiotlb_force;
-struct io_tlb_mem *io_tlb_default_mem;
+struct io_tlb_mem io_tlb_default_mem;
/*
* Max segment that we can provide which (if pages are contingous) will
@@ -101,7 +101,7 @@ early_param("swiotlb", setup_io_tlb_npages);
unsigned int swiotlb_max_segment(void)
{
- return io_tlb_default_mem ? max_segment : 0;
+ return io_tlb_default_mem.nslabs ? max_segment : 0;
}
EXPORT_SYMBOL_GPL(swiotlb_max_segment);
@@ -134,9 +134,9 @@ void __init swiotlb_adjust_size(unsigned long size)
void swiotlb_print_info(void)
{
- struct io_tlb_mem *mem = io_tlb_default_mem;
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
- if (!mem) {
+ if (!mem->nslabs) {
pr_warn("No low mem\n");
return;
}
@@ -163,11 +163,11 @@ static inline unsigned long nr_slots(u64 val)
*/
void __init swiotlb_update_mem_attributes(void)
{
- struct io_tlb_mem *mem = io_tlb_default_mem;
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
void *vaddr;
unsigned long bytes;
- if (!mem || mem->late_alloc)
+ if (!mem->nslabs || mem->late_alloc)
return;
vaddr = phys_to_virt(mem->start);
bytes = PAGE_ALIGN(mem->nslabs << IO_TLB_SHIFT);
@@ -201,25 +201,24 @@ static void swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t start,
int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)
{
- struct io_tlb_mem *mem;
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
size_t alloc_size;
if (swiotlb_force == SWIOTLB_NO_FORCE)
return 0;
/* protect against double initialization */
- if (WARN_ON_ONCE(io_tlb_default_mem))
+ if (WARN_ON_ONCE(mem->nslabs))
return -ENOMEM;
- alloc_size = PAGE_ALIGN(struct_size(mem, slots, nslabs));
- mem = memblock_alloc(alloc_size, PAGE_SIZE);
- if (!mem)
+ alloc_size = PAGE_ALIGN(array_size(sizeof(*mem->slots), nslabs));
+ mem->slots = memblock_alloc(alloc_size, PAGE_SIZE);
+ if (!mem->slots)
panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
__func__, alloc_size, PAGE_SIZE);
swiotlb_init_io_tlb_mem(mem, __pa(tlb), nslabs, false);
- io_tlb_default_mem = mem;
if (verbose)
swiotlb_print_info();
swiotlb_set_max_segment(mem->nslabs << IO_TLB_SHIFT);
@@ -304,26 +303,24 @@ swiotlb_late_init_with_default_size(size_t default_size)
int
swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
{
- struct io_tlb_mem *mem;
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
unsigned long bytes = nslabs << IO_TLB_SHIFT;
if (swiotlb_force == SWIOTLB_NO_FORCE)
return 0;
/* protect against double initialization */
- if (WARN_ON_ONCE(io_tlb_default_mem))
+ if (WARN_ON_ONCE(mem->nslabs))
return -ENOMEM;
- mem = (void *)__get_free_pages(GFP_KERNEL,
- get_order(struct_size(mem, slots, nslabs)));
- if (!mem)
+ mem->slots = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
+ get_order(array_size(sizeof(*mem->slots), nslabs)));
+ if (!mem->slots)
return -ENOMEM;
- memset(mem, 0, sizeof(*mem));
set_memory_decrypted((unsigned long)tlb, bytes >> PAGE_SHIFT);
swiotlb_init_io_tlb_mem(mem, virt_to_phys(tlb), nslabs, true);
- io_tlb_default_mem = mem;
swiotlb_print_info();
swiotlb_set_max_segment(mem->nslabs << IO_TLB_SHIFT);
return 0;
@@ -331,18 +328,18 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
void __init swiotlb_exit(void)
{
- struct io_tlb_mem *mem = io_tlb_default_mem;
size_t size;
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
- if (!mem)
+ if (!mem->nslabs)
return;
- size = struct_size(mem, slots, mem->nslabs);
+ size = array_size(sizeof(*mem->slots), mem->nslabs);
if (mem->late_alloc)
- free_pages((unsigned long)mem, get_order(size));
+ free_pages((unsigned long)mem->slots, get_order(size));
else
- memblock_free_late(__pa(mem), PAGE_ALIGN(size));
- io_tlb_default_mem = NULL;
+ memblock_free_late(__pa(mem->slots), PAGE_ALIGN(size));
+ memset(mem, 0, sizeof(*mem));
}
/*
@@ -696,7 +693,9 @@ size_t swiotlb_max_mapping_size(struct device *dev)
bool is_swiotlb_active(struct device *dev)
{
- return dev->dma_io_tlb_mem != NULL;
+ struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+
+ return mem && mem->nslabs;
}
EXPORT_SYMBOL_GPL(is_swiotlb_active);
@@ -711,10 +710,10 @@ static void swiotlb_create_debugfs_files(struct io_tlb_mem *mem)
static int __init swiotlb_create_default_debugfs(void)
{
- struct io_tlb_mem *mem = io_tlb_default_mem;
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
debugfs_dir = debugfs_create_dir("swiotlb", NULL);
- if (mem) {
+ if (mem->nslabs) {
mem->debugfs = debugfs_dir;
swiotlb_create_debugfs_files(mem);
}
@@ -783,10 +782,17 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
* to it.
*/
if (!mem) {
- mem = kzalloc(struct_size(mem, slots, nslabs), GFP_KERNEL);
+ mem = kzalloc(sizeof(*mem), GFP_KERNEL);
if (!mem)
return -ENOMEM;
+ mem->slots = kzalloc(array_size(sizeof(*mem->slots), nslabs),
+ GFP_KERNEL);
+ if (!mem->slots) {
+ kfree(mem);
+ return -ENOMEM;
+ }
+
set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
rmem->size >> PAGE_SHIFT);
swiotlb_init_io_tlb_mem(mem, rmem->base, nslabs, false);
@@ -806,7 +812,7 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
static void rmem_swiotlb_device_release(struct reserved_mem *rmem,
struct device *dev)
{
- dev->dma_io_tlb_mem = io_tlb_default_mem;
+ dev->dma_io_tlb_mem = &io_tlb_default_mem;
}
static const struct reserved_mem_ops rmem_swiotlb_ops = {
--
2.32.0.402.g57bb445576-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 3/4] swiotlb: Emit diagnostic in swiotlb_exit()
2021-07-20 13:38 [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit() Will Deacon
2021-07-20 13:38 ` [PATCH v2 1/4] of: Return success from of_dma_set_restricted_buffer() when !OF_ADDRESS Will Deacon
2021-07-20 13:38 ` [PATCH v2 2/4] swiotlb: Convert io_default_tlb_mem to static allocation Will Deacon
@ 2021-07-20 13:38 ` Will Deacon
2021-07-20 13:38 ` [PATCH v2 4/4] swiotlb: Free tbl memory " Will Deacon
` (2 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Will Deacon @ 2021-07-20 13:38 UTC (permalink / raw)
To: iommu
Cc: linux-kernel, Will Deacon, Guenter Roeck, Claire Chang,
Christoph Hellwig, Robin Murphy, Konrad Rzeszutek Wilk,
Nathan Chancellor
A recent debugging session would have been made a little bit easier if
we had noticed sooner that swiotlb_exit() was being called during boot.
Add a simple diagnostic message to swiotlb_exit() to complement the one
from swiotlb_print_info() during initialisation.
Cc: Claire Chang <tientzu@chromium.org>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Robin Murphy <robin.murphy@arm.com>
Link: https://lore.kernel.org/r/20210705190352.GA19461@willie-the-truck
Suggested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Tested-by: Nathan Chancellor <nathan@kernel.org>
Tested-by: Claire Chang <tientzu@chromium.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Will Deacon <will@kernel.org>
---
kernel/dma/swiotlb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 7948f274f9bb..b3c793ed9e64 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -334,6 +334,7 @@ void __init swiotlb_exit(void)
if (!mem->nslabs)
return;
+ pr_info("tearing down default memory pool\n");
size = array_size(sizeof(*mem->slots), mem->nslabs);
if (mem->late_alloc)
free_pages((unsigned long)mem->slots, get_order(size));
--
2.32.0.402.g57bb445576-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 4/4] swiotlb: Free tbl memory in swiotlb_exit()
2021-07-20 13:38 [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit() Will Deacon
` (2 preceding siblings ...)
2021-07-20 13:38 ` [PATCH v2 3/4] swiotlb: Emit diagnostic in swiotlb_exit() Will Deacon
@ 2021-07-20 13:38 ` Will Deacon
2021-07-31 18:26 ` Guenter Roeck
2021-07-22 19:22 ` [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit() Christian Borntraeger
2021-07-24 0:29 ` Konrad Rzeszutek Wilk
5 siblings, 1 reply; 18+ messages in thread
From: Will Deacon @ 2021-07-20 13:38 UTC (permalink / raw)
To: iommu
Cc: linux-kernel, Will Deacon, Guenter Roeck, Claire Chang,
Christoph Hellwig, Robin Murphy, Konrad Rzeszutek Wilk,
Nathan Chancellor
Although swiotlb_exit() frees the 'slots' metadata array referenced by
'io_tlb_default_mem', it leaves the underlying buffer pages allocated
despite no longer being usable.
Extend swiotlb_exit() to free the buffer pages as well as the slots
array.
Cc: Claire Chang <tientzu@chromium.org>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Tested-by: Nathan Chancellor <nathan@kernel.org>
Tested-by: Claire Chang <tientzu@chromium.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Will Deacon <will@kernel.org>
---
kernel/dma/swiotlb.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index b3c793ed9e64..87c40517e822 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -328,18 +328,27 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
void __init swiotlb_exit(void)
{
- size_t size;
struct io_tlb_mem *mem = &io_tlb_default_mem;
+ unsigned long tbl_vaddr;
+ size_t tbl_size, slots_size;
if (!mem->nslabs)
return;
pr_info("tearing down default memory pool\n");
- size = array_size(sizeof(*mem->slots), mem->nslabs);
- if (mem->late_alloc)
- free_pages((unsigned long)mem->slots, get_order(size));
- else
- memblock_free_late(__pa(mem->slots), PAGE_ALIGN(size));
+ tbl_vaddr = (unsigned long)phys_to_virt(mem->start);
+ tbl_size = PAGE_ALIGN(mem->end - mem->start);
+ slots_size = PAGE_ALIGN(array_size(sizeof(*mem->slots), mem->nslabs));
+
+ set_memory_encrypted(tbl_vaddr, tbl_size >> PAGE_SHIFT);
+ if (mem->late_alloc) {
+ free_pages(tbl_vaddr, get_order(tbl_size));
+ free_pages((unsigned long)mem->slots, get_order(slots_size));
+ } else {
+ memblock_free_late(mem->start, tbl_size);
+ memblock_free_late(__pa(mem->slots), slots_size);
+ }
+
memset(mem, 0, sizeof(*mem));
}
--
2.32.0.402.g57bb445576-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 4/4] swiotlb: Free tbl memory in swiotlb_exit()
2021-07-20 13:38 ` [PATCH v2 4/4] swiotlb: Free tbl memory " Will Deacon
@ 2021-07-31 18:26 ` Guenter Roeck
2021-08-01 2:29 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 18+ messages in thread
From: Guenter Roeck @ 2021-07-31 18:26 UTC (permalink / raw)
To: Will Deacon
Cc: iommu, linux-kernel, Claire Chang, Christoph Hellwig,
Robin Murphy, Konrad Rzeszutek Wilk, Nathan Chancellor
Hi,
On Tue, Jul 20, 2021 at 02:38:26PM +0100, Will Deacon wrote:
> Although swiotlb_exit() frees the 'slots' metadata array referenced by
> 'io_tlb_default_mem', it leaves the underlying buffer pages allocated
> despite no longer being usable.
>
> Extend swiotlb_exit() to free the buffer pages as well as the slots
> array.
>
This patch causes qemu pseries emulations to crash. Backtrace and bisect
log see below. Reverting it fixes the problem.
Guenter
---
Crash log:
...
[ 0.937801][ T1] software IO TLB: tearing down default memory pool
[ 0.938939][ T1] ------------[ cut here ]------------
[ 0.940331][ T1] kernel BUG at arch/powerpc/kernel/interrupt.c:98!
[ 0.940787][ T1] Oops: Exception in kernel mode, sig: 5 [#1]
[ 0.940883][ T1] BE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries
[ 0.940999][ T1] Modules linked in:
[ 0.941240][ T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc3-next-20210729 #1
[ 0.941523][ T1] NIP: c000000000031310 LR: c0000000000312f4 CTR: c00000000000c5f0
[ 0.941608][ T1] REGS: c000000008687ac0 TRAP: 0700 Not tainted (5.14.0-rc3-next-20210729)
[ 0.941795][ T1] MSR: 8000000000029032 <SF,EE,ME,IR,DR,RI> CR: 24002222 XER: 00000000
[ 0.942031][ T1] CFAR: c000000000300360 IRQMASK: 3
[ 0.942031][ T1] GPR00: c0000000000312f4 c000000008687d60 c0000000022ee300 0000000000000003
[ 0.942031][ T1] GPR04: 000000000000033f 0000000000000400 0000000000000000 000000003e5a0000
[ 0.942031][ T1] GPR08: 000000003e5a0000 0000000000000001 0000000000000000 0000000000000003
[ 0.942031][ T1] GPR12: ffffffffffffffff c000000002fb0000 c0000000000129c0 0000000000000000
[ 0.942031][ T1] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 0.942031][ T1] GPR20: 0000000000000000 0000000000000000 0000000000000000 c0000000021d8b00
[ 0.942031][ T1] GPR24: 0000000000000000 0000000000000000 ffffffffffffffff 0000000000000400
[ 0.942031][ T1] GPR28: 000000000000033f 000000000000f134 c00000000176c068 c000000008687e80
[ 0.942884][ T1] NIP [c000000000031310] .system_call_exception+0x70/0x2d0
[ 0.943399][ T1] LR [c0000000000312f4] .system_call_exception+0x54/0x2d0
[ 0.943594][ T1] Call Trace:
[ 0.943667][ T1] [c000000008687d60] [c0000000000312f4] .system_call_exception+0x54/0x2d0 (unreliable)
[ 0.943919][ T1] [c000000008687e10] [c00000000000c6e4] system_call_common+0xf4/0x258
[ 0.944066][ T1] --- interrupt: c00 at .ucall_norets+0x4/0x14
[ 0.944172][ T1] NIP: c000000000079ce0 LR: c0000000000fa274 CTR: 0000000000000000
[ 0.944245][ T1] REGS: c000000008687e80 TRAP: 0c00 Not tainted (5.14.0-rc3-next-20210729)
[ 0.944323][ T1] MSR: 8000000002009032 <SF,VEC,EE,ME,IR,DR,RI> CR: 24002222 XER: 00000000
[ 0.944463][ T1] IRQMASK: 0
[ 0.944463][ T1] GPR00: c00000000176c068 c000000008687a30 c0000000022ee300 000000000000f134
[ 0.944463][ T1] GPR04: 000000000000033f 0000000000000400 ffffffffffffffff 0000000000000000
[ 0.944463][ T1] GPR08: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 0.944463][ T1] GPR12: 0000000000000000 c000000002fb0000 c0000000000129c0 0000000000000000
[ 0.944463][ T1] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 0.944463][ T1] GPR20: 0000000000000000 0000000000000000 0000000000000000 c0000000021d8b00
[ 0.944463][ T1] GPR24: c0000000017c1088 0000000000000004 c00000000171c450 c000000002338e40
[ 0.944463][ T1] GPR28: c0000000017c10b0 c0000000033f0000 0000000000000400 c0000000033f0000
[ 0.945188][ T1] NIP [c000000000079ce0] .ucall_norets+0x4/0x14
[ 0.945285][ T1] LR [c0000000000fa274] .set_memory_encrypted+0x44/0x80
[ 0.945375][ T1] --- interrupt: c00
[ 0.945419][ T1] [c000000008687a30] [c00000000176c068] .swiotlb_exit+0xbc/0x180 (unreliable)
[ 0.945612][ T1] Instruction dump:
[ 0.945745][ T1] 7cbb2b78 7cda3378 7cf93b78 7d184378 482cefd1 60000000 e93f0108 692a0002
[ 0.945883][ T1] 794affe2 0b0a0000 69294000 792997e2 <0b090000> e93f0138 792907e0 0b090000
[ 0.946751][ T1] ---[ end trace 600e218cfc83c24b ]---
[ 0.955185][ T1]
[ 1.955805][ T1] BUG: sleeping function called from invalid context at ./include/linux/percpu-rwsem.h:49
[ 1.955918][ T1] in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: swapper/0
[ 1.956024][ T1] INFO: lockdep is turned off.
[ 1.956094][ T1] irq event stamp: 14792
[ 1.956142][ T1] hardirqs last enabled at (14791): [<c0000000001f8fe8>] .__up_console_sem+0xc8/0x110
[ 1.956269][ T1] hardirqs last disabled at (14792): [<c0000000000312f4>] .system_call_exception+0x54/0x2d0
[ 1.956384][ T1] softirqs last enabled at (10586): [<c000000000efd6d4>] .release_sock+0xb4/0x100
[ 1.956497][ T1] softirqs last disabled at (10584): [<c000000000efd654>] .release_sock+0x34/0x100
[ 1.956726][ T1] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G D 5.14.0-rc3-next-20210729 #1
[ 1.956846][ T1] Call Trace:
[ 1.956885][ T1] [c000000008687700] [c0000000009fe088] .dump_stack_lvl+0xa4/0x100 (unreliable)
[ 1.957011][ T1] [c000000008687790] [c000000000193060] .___might_sleep+0x2b0/0x2f0
[ 1.957130][ T1] [c000000008687820] [c00000000015fba0] .exit_signals+0x50/0x540
[ 1.957238][ T1] [c0000000086878e0] [c000000000148dbc] .do_exit+0xec/0xe20
[ 1.957335][ T1] [c0000000086879c0] [c000000000029bc4] .oops_end+0x144/0x210
[ 1.957441][ T1] [c000000008687a50] [c0000000000095d4] program_check_common_virt+0x2d4/0x320
[ 1.957561][ T1] --- interrupt: 700 at .system_call_exception+0x70/0x2d0
[ 1.957645][ T1] NIP: c000000000031310 LR: c0000000000312f4 CTR: c00000000000c5f0
[ 1.957715][ T1] REGS: c000000008687ac0 TRAP: 0700 Tainted: G D (5.14.0-rc3-next-20210729)
[ 1.957799][ T1] MSR: 8000000000029032 <SF,EE,ME,IR,DR,RI> CR: 24002222 XER: 00000000
[ 1.957919][ T1] CFAR: c000000000300360 IRQMASK: 3
[ 1.957919][ T1] GPR00: c0000000000312f4 c000000008687d60 c0000000022ee300 0000000000000003
[ 1.957919][ T1] GPR04: 000000000000033f 0000000000000400 0000000000000000 000000003e5a0000
[ 1.957919][ T1] GPR08: 000000003e5a0000 0000000000000001 0000000000000000 0000000000000003
[ 1.957919][ T1] GPR12: ffffffffffffffff c000000002fb0000 c0000000000129c0 0000000000000000
[ 1.957919][ T1] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 1.957919][ T1] GPR20: 0000000000000000 0000000000000000 0000000000000000 c0000000021d8b00
[ 1.957919][ T1] GPR24: 0000000000000000 0000000000000000 ffffffffffffffff 0000000000000400
[ 1.957919][ T1] GPR28: 000000000000033f 000000000000f134 c00000000176c068 c000000008687e80
[ 1.958633][ T1] NIP [c000000000031310] .system_call_exception+0x70/0x2d0
[ 1.958720][ T1] LR [c0000000000312f4] .system_call_exception+0x54/0x2d0
[ 1.958803][ T1] --- interrupt: 700
[ 1.958842][ T1] [c000000008687e10] [c00000000000c6e4] system_call_common+0xf4/0x258
[ 1.958961][ T1] --- interrupt: c00 at .ucall_norets+0x4/0x14
[ 1.959055][ T1] NIP: c000000000079ce0 LR: c0000000000fa274 CTR: 0000000000000000
[ 1.959127][ T1] REGS: c000000008687e80 TRAP: 0c00 Tainted: G D (5.14.0-rc3-next-20210729)
[ 1.959210][ T1] MSR: 8000000002009032 <SF,VEC,EE,ME,IR,DR,RI> CR: 24002222 XER: 00000000
[ 1.959337][ T1] IRQMASK: 0
[ 1.959337][ T1] GPR00: c00000000176c068 c000000008687a30 c0000000022ee300 000000000000f134
[ 1.959337][ T1] GPR04: 000000000000033f 0000000000000400 ffffffffffffffff 0000000000000000
[ 1.959337][ T1] GPR08: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 1.959337][ T1] GPR12: 0000000000000000 c000000002fb0000 c0000000000129c0 0000000000000000
[ 1.959337][ T1] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 1.959337][ T1] GPR20: 0000000000000000 0000000000000000 0000000000000000 c0000000021d8b00
[ 1.959337][ T1] GPR24: c0000000017c1088 0000000000000004 c00000000171c450 c000000002338e40
[ 1.959337][ T1] GPR28: c0000000017c10b0 c0000000033f0000 0000000000000400 c0000000033f0000
[ 1.960042][ T1] NIP [c000000000079ce0] .ucall_norets+0x4/0x14
[ 1.960137][ T1] LR [c0000000000fa274] .set_memory_encrypted+0x44/0x80
[ 1.960219][ T1] --- interrupt: c00
[ 1.960257][ T1] [c000000008687a30] [c00000000176c068] .swiotlb_exit+0xbc/0x180 (unreliable)
[ 1.960845][ T1] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000005
qemu-system-ppc64: OS terminated: OS panic: Attempted to kill init! exitcode=0x00000005
---
Bisect log:
# bad: [8d4b477da1a807199ca60e0829357ce7aa6758d5] Add linux-next specific files for 20210730
# good: [ff1176468d368232b684f75e82563369208bc371] Linux 5.14-rc3
git bisect start 'HEAD' 'v5.14-rc3'
# bad: [8f3eb1f5c702ef868d89799b03c21d122f2fe197] Merge remote-tracking branch 'bpf-next/for-next'
git bisect bad 8f3eb1f5c702ef868d89799b03c21d122f2fe197
# good: [02afbb8f68dc8b37c07e457d2f69d440781af23a] Merge remote-tracking branch 'cifsd/cifsd-for-next'
git bisect good 02afbb8f68dc8b37c07e457d2f69d440781af23a
# good: [3e12361b6d23f793580a50a6008633501c56ea1d] bcm63xx_enet: delete a redundant assignment
git bisect good 3e12361b6d23f793580a50a6008633501c56ea1d
# good: [78d788681492abe4980d5cc9b93b70df9f028880] Merge remote-tracking branch 'jc_docs/docs-next'
git bisect good 78d788681492abe4980d5cc9b93b70df9f028880
# good: [266234e7659f731cf471a1bdc4fd1ead4caa8303] Merge remote-tracking branch 'v4l-dvb-next/master'
git bisect good 266234e7659f731cf471a1bdc4fd1ead4caa8303
# bad: [be1841fe3ca0010139fe4fb44dd42dea7c0e3401] Merge remote-tracking branch 'swiotlb/linux-next'
git bisect bad be1841fe3ca0010139fe4fb44dd42dea7c0e3401
# good: [7b8798617c7975d10678d99dcc59d103e237b4cd] Merge remote-tracking branch 'ieee1394/for-next'
git bisect good 7b8798617c7975d10678d99dcc59d103e237b4cd
# good: [463e862ac63ef27fca423782536f6465abc3f180] swiotlb: Convert io_default_tlb_mem to static allocation
git bisect good 463e862ac63ef27fca423782536f6465abc3f180
# good: [2dc6b1158c28c3a5e86d162628810312f98d5e97] fs: dlm: introduce generic listen
git bisect good 2dc6b1158c28c3a5e86d162628810312f98d5e97
# good: [62699b3f0a62435fceb8debf295e90a5ea259e04] fs: dlm: move receive loop into receive handler
git bisect good 62699b3f0a62435fceb8debf295e90a5ea259e04
# bad: [ad6c00283163cb7ad52cdf97d2850547446f7d98] swiotlb: Free tbl memory in swiotlb_exit()
git bisect bad ad6c00283163cb7ad52cdf97d2850547446f7d98
# good: [1efd3fc0ccf52e1aa5f0bf5b0d82847180d20951] swiotlb: Emit diagnostic in swiotlb_exit()
git bisect good 1efd3fc0ccf52e1aa5f0bf5b0d82847180d20951
# first bad commit: [ad6c00283163cb7ad52cdf97d2850547446f7d98] swiotlb: Free tbl memory in swiotlb_exit()
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/4] swiotlb: Free tbl memory in swiotlb_exit()
2021-07-31 18:26 ` Guenter Roeck
@ 2021-08-01 2:29 ` Konrad Rzeszutek Wilk
2021-08-01 4:26 ` Guenter Roeck
0 siblings, 1 reply; 18+ messages in thread
From: Konrad Rzeszutek Wilk @ 2021-08-01 2:29 UTC (permalink / raw)
To: Guenter Roeck, will
Cc: Will Deacon, Konrad Rzeszutek Wilk, linux-kernel,
Nathan Chancellor, iommu, Claire Chang, Robin Murphy,
Christoph Hellwig
On Sat, Jul 31, 2021 at 11:26:11AM -0700, Guenter Roeck wrote:
> Hi,
>
> On Tue, Jul 20, 2021 at 02:38:26PM +0100, Will Deacon wrote:
> > Although swiotlb_exit() frees the 'slots' metadata array referenced by
> > 'io_tlb_default_mem', it leaves the underlying buffer pages allocated
> > despite no longer being usable.
> >
> > Extend swiotlb_exit() to free the buffer pages as well as the slots
> > array.
> >
>
> This patch causes qemu pseries emulations to crash. Backtrace and bisect
> log see below. Reverting it fixes the problem.
I am 99% sure it is fixed by this patch (which should be in linux-next
in 5 minutes):
From a449ffaf9181b5a2dc705d8a06b13e0068207fd4 Mon Sep 17 00:00:00 2001
From: Will Deacon <will@kernel.org>
Date: Fri, 30 Jul 2021 12:42:31 +0100
Subject: [PATCH] powerpc/svm: Don't issue ultracalls if !mem_encrypt_active()
Commit ad6c00283163 ("swiotlb: Free tbl memory in swiotlb_exit()")
introduced a set_memory_encrypted() call to swiotlb_exit() so that the
buffer pages are returned to an encrypted state prior to being freed.
Sachin reports that this leads to the following crash on a Power server:
[ 0.010799] software IO TLB: tearing down default memory pool
[ 0.010805] ------------[ cut here ]------------
[ 0.010808] kernel BUG at arch/powerpc/kernel/interrupt.c:98!
Nick spotted that this is because set_memory_encrypted() is issuing an
ultracall which doesn't exist for the processor, and should therefore
be gated by mem_encrypt_active() to mirror the x86 implementation.
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Claire Chang <tientzu@chromium.org>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Robin Murphy <robin.murphy@arm.com>
Fixes: ad6c00283163 ("swiotlb: Free tbl memory in swiotlb_exit()")
Suggested-by: Nicholas Piggin <npiggin@gmail.com>
Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Tested-by: Nathan Chancellor <nathan@kernel.org>
Link: https://lore.kernel.org/r/1905CD70-7656-42AE-99E2-A31FC3812EAC@linux.vnet.ibm.com/
Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Konrad Rzeszutek Wilk <konrad@kernel.org>
---
arch/powerpc/platforms/pseries/svm.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/svm.c b/arch/powerpc/platforms/pseries/svm.c
index 1d829e257996..87f001b4c4e4 100644
--- a/arch/powerpc/platforms/pseries/svm.c
+++ b/arch/powerpc/platforms/pseries/svm.c
@@ -63,6 +63,9 @@ void __init svm_swiotlb_init(void)
int set_memory_encrypted(unsigned long addr, int numpages)
{
+ if (!mem_encrypt_active())
+ return 0;
+
if (!PAGE_ALIGNED(addr))
return -EINVAL;
@@ -73,6 +76,9 @@ int set_memory_encrypted(unsigned long addr, int numpages)
int set_memory_decrypted(unsigned long addr, int numpages)
{
+ if (!mem_encrypt_active())
+ return 0;
+
if (!PAGE_ALIGNED(addr))
return -EINVAL;
--
2.31.1
>
> Guenter
>
> ---
> Crash log:
>
> ...
> [ 0.937801][ T1] software IO TLB: tearing down default memory pool
> [ 0.938939][ T1] ------------[ cut here ]------------
> [ 0.940331][ T1] kernel BUG at arch/powerpc/kernel/interrupt.c:98!
> [ 0.940787][ T1] Oops: Exception in kernel mode, sig: 5 [#1]
> [ 0.940883][ T1] BE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries
> [ 0.940999][ T1] Modules linked in:
> [ 0.941240][ T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc3-next-20210729 #1
> [ 0.941523][ T1] NIP: c000000000031310 LR: c0000000000312f4 CTR: c00000000000c5f0
> [ 0.941608][ T1] REGS: c000000008687ac0 TRAP: 0700 Not tainted (5.14.0-rc3-next-20210729)
> [ 0.941795][ T1] MSR: 8000000000029032 <SF,EE,ME,IR,DR,RI> CR: 24002222 XER: 00000000
> [ 0.942031][ T1] CFAR: c000000000300360 IRQMASK: 3
> [ 0.942031][ T1] GPR00: c0000000000312f4 c000000008687d60 c0000000022ee300 0000000000000003
> [ 0.942031][ T1] GPR04: 000000000000033f 0000000000000400 0000000000000000 000000003e5a0000
> [ 0.942031][ T1] GPR08: 000000003e5a0000 0000000000000001 0000000000000000 0000000000000003
> [ 0.942031][ T1] GPR12: ffffffffffffffff c000000002fb0000 c0000000000129c0 0000000000000000
> [ 0.942031][ T1] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> [ 0.942031][ T1] GPR20: 0000000000000000 0000000000000000 0000000000000000 c0000000021d8b00
> [ 0.942031][ T1] GPR24: 0000000000000000 0000000000000000 ffffffffffffffff 0000000000000400
> [ 0.942031][ T1] GPR28: 000000000000033f 000000000000f134 c00000000176c068 c000000008687e80
> [ 0.942884][ T1] NIP [c000000000031310] .system_call_exception+0x70/0x2d0
> [ 0.943399][ T1] LR [c0000000000312f4] .system_call_exception+0x54/0x2d0
> [ 0.943594][ T1] Call Trace:
> [ 0.943667][ T1] [c000000008687d60] [c0000000000312f4] .system_call_exception+0x54/0x2d0 (unreliable)
> [ 0.943919][ T1] [c000000008687e10] [c00000000000c6e4] system_call_common+0xf4/0x258
> [ 0.944066][ T1] --- interrupt: c00 at .ucall_norets+0x4/0x14
> [ 0.944172][ T1] NIP: c000000000079ce0 LR: c0000000000fa274 CTR: 0000000000000000
> [ 0.944245][ T1] REGS: c000000008687e80 TRAP: 0c00 Not tainted (5.14.0-rc3-next-20210729)
> [ 0.944323][ T1] MSR: 8000000002009032 <SF,VEC,EE,ME,IR,DR,RI> CR: 24002222 XER: 00000000
> [ 0.944463][ T1] IRQMASK: 0
> [ 0.944463][ T1] GPR00: c00000000176c068 c000000008687a30 c0000000022ee300 000000000000f134
> [ 0.944463][ T1] GPR04: 000000000000033f 0000000000000400 ffffffffffffffff 0000000000000000
> [ 0.944463][ T1] GPR08: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> [ 0.944463][ T1] GPR12: 0000000000000000 c000000002fb0000 c0000000000129c0 0000000000000000
> [ 0.944463][ T1] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> [ 0.944463][ T1] GPR20: 0000000000000000 0000000000000000 0000000000000000 c0000000021d8b00
> [ 0.944463][ T1] GPR24: c0000000017c1088 0000000000000004 c00000000171c450 c000000002338e40
> [ 0.944463][ T1] GPR28: c0000000017c10b0 c0000000033f0000 0000000000000400 c0000000033f0000
> [ 0.945188][ T1] NIP [c000000000079ce0] .ucall_norets+0x4/0x14
> [ 0.945285][ T1] LR [c0000000000fa274] .set_memory_encrypted+0x44/0x80
> [ 0.945375][ T1] --- interrupt: c00
> [ 0.945419][ T1] [c000000008687a30] [c00000000176c068] .swiotlb_exit+0xbc/0x180 (unreliable)
> [ 0.945612][ T1] Instruction dump:
> [ 0.945745][ T1] 7cbb2b78 7cda3378 7cf93b78 7d184378 482cefd1 60000000 e93f0108 692a0002
> [ 0.945883][ T1] 794affe2 0b0a0000 69294000 792997e2 <0b090000> e93f0138 792907e0 0b090000
> [ 0.946751][ T1] ---[ end trace 600e218cfc83c24b ]---
> [ 0.955185][ T1]
> [ 1.955805][ T1] BUG: sleeping function called from invalid context at ./include/linux/percpu-rwsem.h:49
> [ 1.955918][ T1] in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: swapper/0
> [ 1.956024][ T1] INFO: lockdep is turned off.
> [ 1.956094][ T1] irq event stamp: 14792
> [ 1.956142][ T1] hardirqs last enabled at (14791): [<c0000000001f8fe8>] .__up_console_sem+0xc8/0x110
> [ 1.956269][ T1] hardirqs last disabled at (14792): [<c0000000000312f4>] .system_call_exception+0x54/0x2d0
> [ 1.956384][ T1] softirqs last enabled at (10586): [<c000000000efd6d4>] .release_sock+0xb4/0x100
> [ 1.956497][ T1] softirqs last disabled at (10584): [<c000000000efd654>] .release_sock+0x34/0x100
> [ 1.956726][ T1] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G D 5.14.0-rc3-next-20210729 #1
> [ 1.956846][ T1] Call Trace:
> [ 1.956885][ T1] [c000000008687700] [c0000000009fe088] .dump_stack_lvl+0xa4/0x100 (unreliable)
> [ 1.957011][ T1] [c000000008687790] [c000000000193060] .___might_sleep+0x2b0/0x2f0
> [ 1.957130][ T1] [c000000008687820] [c00000000015fba0] .exit_signals+0x50/0x540
> [ 1.957238][ T1] [c0000000086878e0] [c000000000148dbc] .do_exit+0xec/0xe20
> [ 1.957335][ T1] [c0000000086879c0] [c000000000029bc4] .oops_end+0x144/0x210
> [ 1.957441][ T1] [c000000008687a50] [c0000000000095d4] program_check_common_virt+0x2d4/0x320
> [ 1.957561][ T1] --- interrupt: 700 at .system_call_exception+0x70/0x2d0
> [ 1.957645][ T1] NIP: c000000000031310 LR: c0000000000312f4 CTR: c00000000000c5f0
> [ 1.957715][ T1] REGS: c000000008687ac0 TRAP: 0700 Tainted: G D (5.14.0-rc3-next-20210729)
> [ 1.957799][ T1] MSR: 8000000000029032 <SF,EE,ME,IR,DR,RI> CR: 24002222 XER: 00000000
> [ 1.957919][ T1] CFAR: c000000000300360 IRQMASK: 3
> [ 1.957919][ T1] GPR00: c0000000000312f4 c000000008687d60 c0000000022ee300 0000000000000003
> [ 1.957919][ T1] GPR04: 000000000000033f 0000000000000400 0000000000000000 000000003e5a0000
> [ 1.957919][ T1] GPR08: 000000003e5a0000 0000000000000001 0000000000000000 0000000000000003
> [ 1.957919][ T1] GPR12: ffffffffffffffff c000000002fb0000 c0000000000129c0 0000000000000000
> [ 1.957919][ T1] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> [ 1.957919][ T1] GPR20: 0000000000000000 0000000000000000 0000000000000000 c0000000021d8b00
> [ 1.957919][ T1] GPR24: 0000000000000000 0000000000000000 ffffffffffffffff 0000000000000400
> [ 1.957919][ T1] GPR28: 000000000000033f 000000000000f134 c00000000176c068 c000000008687e80
> [ 1.958633][ T1] NIP [c000000000031310] .system_call_exception+0x70/0x2d0
> [ 1.958720][ T1] LR [c0000000000312f4] .system_call_exception+0x54/0x2d0
> [ 1.958803][ T1] --- interrupt: 700
> [ 1.958842][ T1] [c000000008687e10] [c00000000000c6e4] system_call_common+0xf4/0x258
> [ 1.958961][ T1] --- interrupt: c00 at .ucall_norets+0x4/0x14
> [ 1.959055][ T1] NIP: c000000000079ce0 LR: c0000000000fa274 CTR: 0000000000000000
> [ 1.959127][ T1] REGS: c000000008687e80 TRAP: 0c00 Tainted: G D (5.14.0-rc3-next-20210729)
> [ 1.959210][ T1] MSR: 8000000002009032 <SF,VEC,EE,ME,IR,DR,RI> CR: 24002222 XER: 00000000
> [ 1.959337][ T1] IRQMASK: 0
> [ 1.959337][ T1] GPR00: c00000000176c068 c000000008687a30 c0000000022ee300 000000000000f134
> [ 1.959337][ T1] GPR04: 000000000000033f 0000000000000400 ffffffffffffffff 0000000000000000
> [ 1.959337][ T1] GPR08: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> [ 1.959337][ T1] GPR12: 0000000000000000 c000000002fb0000 c0000000000129c0 0000000000000000
> [ 1.959337][ T1] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> [ 1.959337][ T1] GPR20: 0000000000000000 0000000000000000 0000000000000000 c0000000021d8b00
> [ 1.959337][ T1] GPR24: c0000000017c1088 0000000000000004 c00000000171c450 c000000002338e40
> [ 1.959337][ T1] GPR28: c0000000017c10b0 c0000000033f0000 0000000000000400 c0000000033f0000
> [ 1.960042][ T1] NIP [c000000000079ce0] .ucall_norets+0x4/0x14
> [ 1.960137][ T1] LR [c0000000000fa274] .set_memory_encrypted+0x44/0x80
> [ 1.960219][ T1] --- interrupt: c00
> [ 1.960257][ T1] [c000000008687a30] [c00000000176c068] .swiotlb_exit+0xbc/0x180 (unreliable)
> [ 1.960845][ T1] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000005
> qemu-system-ppc64: OS terminated: OS panic: Attempted to kill init! exitcode=0x00000005
>
> ---
> Bisect log:
>
> # bad: [8d4b477da1a807199ca60e0829357ce7aa6758d5] Add linux-next specific files for 20210730
> # good: [ff1176468d368232b684f75e82563369208bc371] Linux 5.14-rc3
> git bisect start 'HEAD' 'v5.14-rc3'
> # bad: [8f3eb1f5c702ef868d89799b03c21d122f2fe197] Merge remote-tracking branch 'bpf-next/for-next'
> git bisect bad 8f3eb1f5c702ef868d89799b03c21d122f2fe197
> # good: [02afbb8f68dc8b37c07e457d2f69d440781af23a] Merge remote-tracking branch 'cifsd/cifsd-for-next'
> git bisect good 02afbb8f68dc8b37c07e457d2f69d440781af23a
> # good: [3e12361b6d23f793580a50a6008633501c56ea1d] bcm63xx_enet: delete a redundant assignment
> git bisect good 3e12361b6d23f793580a50a6008633501c56ea1d
> # good: [78d788681492abe4980d5cc9b93b70df9f028880] Merge remote-tracking branch 'jc_docs/docs-next'
> git bisect good 78d788681492abe4980d5cc9b93b70df9f028880
> # good: [266234e7659f731cf471a1bdc4fd1ead4caa8303] Merge remote-tracking branch 'v4l-dvb-next/master'
> git bisect good 266234e7659f731cf471a1bdc4fd1ead4caa8303
> # bad: [be1841fe3ca0010139fe4fb44dd42dea7c0e3401] Merge remote-tracking branch 'swiotlb/linux-next'
> git bisect bad be1841fe3ca0010139fe4fb44dd42dea7c0e3401
> # good: [7b8798617c7975d10678d99dcc59d103e237b4cd] Merge remote-tracking branch 'ieee1394/for-next'
> git bisect good 7b8798617c7975d10678d99dcc59d103e237b4cd
> # good: [463e862ac63ef27fca423782536f6465abc3f180] swiotlb: Convert io_default_tlb_mem to static allocation
> git bisect good 463e862ac63ef27fca423782536f6465abc3f180
> # good: [2dc6b1158c28c3a5e86d162628810312f98d5e97] fs: dlm: introduce generic listen
> git bisect good 2dc6b1158c28c3a5e86d162628810312f98d5e97
> # good: [62699b3f0a62435fceb8debf295e90a5ea259e04] fs: dlm: move receive loop into receive handler
> git bisect good 62699b3f0a62435fceb8debf295e90a5ea259e04
> # bad: [ad6c00283163cb7ad52cdf97d2850547446f7d98] swiotlb: Free tbl memory in swiotlb_exit()
> git bisect bad ad6c00283163cb7ad52cdf97d2850547446f7d98
> # good: [1efd3fc0ccf52e1aa5f0bf5b0d82847180d20951] swiotlb: Emit diagnostic in swiotlb_exit()
> git bisect good 1efd3fc0ccf52e1aa5f0bf5b0d82847180d20951
> # first bad commit: [ad6c00283163cb7ad52cdf97d2850547446f7d98] swiotlb: Free tbl memory in swiotlb_exit()
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 4/4] swiotlb: Free tbl memory in swiotlb_exit()
2021-08-01 2:29 ` Konrad Rzeszutek Wilk
@ 2021-08-01 4:26 ` Guenter Roeck
0 siblings, 0 replies; 18+ messages in thread
From: Guenter Roeck @ 2021-08-01 4:26 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, will
Cc: Konrad Rzeszutek Wilk, linux-kernel, Nathan Chancellor, iommu,
Claire Chang, Robin Murphy, Christoph Hellwig
On 7/31/21 7:29 PM, Konrad Rzeszutek Wilk wrote:
> On Sat, Jul 31, 2021 at 11:26:11AM -0700, Guenter Roeck wrote:
>> Hi,
>>
>> On Tue, Jul 20, 2021 at 02:38:26PM +0100, Will Deacon wrote:
>>> Although swiotlb_exit() frees the 'slots' metadata array referenced by
>>> 'io_tlb_default_mem', it leaves the underlying buffer pages allocated
>>> despite no longer being usable.
>>>
>>> Extend swiotlb_exit() to free the buffer pages as well as the slots
>>> array.
>>>
>>
>> This patch causes qemu pseries emulations to crash. Backtrace and bisect
>> log see below. Reverting it fixes the problem.
>
> I am 99% sure it is fixed by this patch (which should be in linux-next
> in 5 minutes):
>
Yes, it does.
FWIW:
Tested-by: Guenter Roeck <linux@roeck-us.net>
Thanks!
Guenter
>
>>From a449ffaf9181b5a2dc705d8a06b13e0068207fd4 Mon Sep 17 00:00:00 2001
> From: Will Deacon <will@kernel.org>
> Date: Fri, 30 Jul 2021 12:42:31 +0100
> Subject: [PATCH] powerpc/svm: Don't issue ultracalls if !mem_encrypt_active()
>
> Commit ad6c00283163 ("swiotlb: Free tbl memory in swiotlb_exit()")
> introduced a set_memory_encrypted() call to swiotlb_exit() so that the
> buffer pages are returned to an encrypted state prior to being freed.
>
> Sachin reports that this leads to the following crash on a Power server:
>
> [ 0.010799] software IO TLB: tearing down default memory pool
> [ 0.010805] ------------[ cut here ]------------
> [ 0.010808] kernel BUG at arch/powerpc/kernel/interrupt.c:98!
>
> Nick spotted that this is because set_memory_encrypted() is issuing an
> ultracall which doesn't exist for the processor, and should therefore
> be gated by mem_encrypt_active() to mirror the x86 implementation.
>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Claire Chang <tientzu@chromium.org>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Fixes: ad6c00283163 ("swiotlb: Free tbl memory in swiotlb_exit()")
> Suggested-by: Nicholas Piggin <npiggin@gmail.com>
> Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> Tested-by: Nathan Chancellor <nathan@kernel.org>
> Link: https://lore.kernel.org/r/1905CD70-7656-42AE-99E2-A31FC3812EAC@linux.vnet.ibm.com/
> Signed-off-by: Will Deacon <will@kernel.org>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad@kernel.org>
> ---
> arch/powerpc/platforms/pseries/svm.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/powerpc/platforms/pseries/svm.c b/arch/powerpc/platforms/pseries/svm.c
> index 1d829e257996..87f001b4c4e4 100644
> --- a/arch/powerpc/platforms/pseries/svm.c
> +++ b/arch/powerpc/platforms/pseries/svm.c
> @@ -63,6 +63,9 @@ void __init svm_swiotlb_init(void)
>
> int set_memory_encrypted(unsigned long addr, int numpages)
> {
> + if (!mem_encrypt_active())
> + return 0;
> +
> if (!PAGE_ALIGNED(addr))
> return -EINVAL;
>
> @@ -73,6 +76,9 @@ int set_memory_encrypted(unsigned long addr, int numpages)
>
> int set_memory_decrypted(unsigned long addr, int numpages)
> {
> + if (!mem_encrypt_active())
> + return 0;
> +
> if (!PAGE_ALIGNED(addr))
> return -EINVAL;
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()
2021-07-20 13:38 [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit() Will Deacon
` (3 preceding siblings ...)
2021-07-20 13:38 ` [PATCH v2 4/4] swiotlb: Free tbl memory " Will Deacon
@ 2021-07-22 19:22 ` Christian Borntraeger
2021-07-23 1:12 ` Halil Pasic
2021-07-24 0:29 ` Konrad Rzeszutek Wilk
5 siblings, 1 reply; 18+ messages in thread
From: Christian Borntraeger @ 2021-07-22 19:22 UTC (permalink / raw)
To: Will Deacon, iommu
Cc: linux-kernel, Guenter Roeck, Claire Chang, Christoph Hellwig,
Robin Murphy, Konrad Rzeszutek Wilk, Nathan Chancellor,
Halil Pasic
On 20.07.21 15:38, Will Deacon wrote:
> Hi again, folks,
>
> This is version two of the patch series I posted yesterday:
>
> https://lore.kernel.org/r/20210719123054.6844-1-will@kernel.org
>
> The only changes since v1 are:
>
> * Squash patches 2 and 3, amending the commit message accordingly
> * Add Reviewed-by and Tested-by tags from Christoph and Claire (thanks!)
>
> I'd usually leave it a bit longer between postings, but since this fixes
> issues with patches in -next I thought I'd spin a new version immediately.
>
> Cheers,
FWIW, I just bisected virtio-errors with secure execution mode
qemu-system-s390x: virtio-serial-bus: Unexpected port id 4205794771 for device virtio-serial0.0
to
commit 903cd0f315fe426c6a64c54ed389de0becb663dc
Author: Claire Chang <tientzu@chromium.org>
Date: Thu Jun 24 23:55:20 2021 +0800
swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
Unfortunately this patch series does NOT fix this issue, so it seems that even more
things are broken.
Any idea what else might be broken?
Shall we rather revert these patches from next until we have things under control?
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()
2021-07-22 19:22 ` [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit() Christian Borntraeger
@ 2021-07-23 1:12 ` Halil Pasic
2021-07-23 5:51 ` Christian Borntraeger
2021-07-23 6:14 ` Christian Borntraeger
0 siblings, 2 replies; 18+ messages in thread
From: Halil Pasic @ 2021-07-23 1:12 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Will Deacon, iommu, linux-kernel, Guenter Roeck, Claire Chang,
Christoph Hellwig, Robin Murphy, Konrad Rzeszutek Wilk,
Nathan Chancellor, Halil Pasic
On Thu, 22 Jul 2021 21:22:58 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> On 20.07.21 15:38, Will Deacon wrote:
> > Hi again, folks,
> >
> > This is version two of the patch series I posted yesterday:
> >
> > https://lore.kernel.org/r/20210719123054.6844-1-will@kernel.org
> >
> > The only changes since v1 are:
> >
> > * Squash patches 2 and 3, amending the commit message accordingly
> > * Add Reviewed-by and Tested-by tags from Christoph and Claire (thanks!)
> >
> > I'd usually leave it a bit longer between postings, but since this fixes
> > issues with patches in -next I thought I'd spin a new version immediately.
> >
> > Cheers,
>
> FWIW, I just bisected virtio-errors with secure execution mode
> qemu-system-s390x: virtio-serial-bus: Unexpected port id 4205794771 for device virtio-serial0.0
>
> to
> commit 903cd0f315fe426c6a64c54ed389de0becb663dc
> Author: Claire Chang <tientzu@chromium.org>
> Date: Thu Jun 24 23:55:20 2021 +0800
>
> swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
>
> Unfortunately this patch series does NOT fix this issue, so it seems that even more
> things are broken.
>
> Any idea what else might be broken?
I've done some debugging, and I think I know what is going on. Since
that commit we need to set force_swiotlb before the swiotlb itself is
initialized. So the patch below should fix the problem.
--------------------8<-------------------------------------
From: Halil Pasic <pasic@linux.ibm.com>
Date: Fri, 23 Jul 2021 02:57:06 +0200
Subject: [PATCH 1/1] s390/pv: fix the forcing of the swiotlb
Since commit 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for
swiotlb data bouncing") if code sets swiotlb_force it needs to do so
before the swiotlb is initialised. Otherwise
io_tlb_default_mem->force_bounce will not get set to true, and devices
that use (the default) swiotlb will not bounce despite switolb_force
having the value of SWIOTLB_FORCE.
Let us restore swiotlb functionality for PV by fulfilling this new
requirement.
Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
---
arch/s390/mm/init.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 8ac710de1ab1..07bbee9b7320 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -186,9 +186,9 @@ static void pv_init(void)
return;
/* make sure bounce buffers are shared */
+ swiotlb_force = SWIOTLB_FORCE;
swiotlb_init(1);
swiotlb_update_mem_attributes();
- swiotlb_force = SWIOTLB_FORCE;
}
void __init mem_init(void)
--
2.29.2
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()
2021-07-23 1:12 ` Halil Pasic
@ 2021-07-23 5:51 ` Christian Borntraeger
2021-07-23 6:14 ` Christian Borntraeger
1 sibling, 0 replies; 18+ messages in thread
From: Christian Borntraeger @ 2021-07-23 5:51 UTC (permalink / raw)
To: Halil Pasic
Cc: Will Deacon, iommu, linux-kernel, Guenter Roeck, Claire Chang,
Christoph Hellwig, Robin Murphy, Konrad Rzeszutek Wilk,
Nathan Chancellor, linux-s390, Heiko Carstens, Vasily Gorbik
On 23.07.21 03:12, Halil Pasic wrote:
> On Thu, 22 Jul 2021 21:22:58 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>
>> On 20.07.21 15:38, Will Deacon wrote:
>>> Hi again, folks,
>>>
>>> This is version two of the patch series I posted yesterday:
>>>
>>> https://lore.kernel.org/r/20210719123054.6844-1-will@kernel.org
>>>
>>> The only changes since v1 are:
>>>
>>> * Squash patches 2 and 3, amending the commit message accordingly
>>> * Add Reviewed-by and Tested-by tags from Christoph and Claire (thanks!)
>>>
>>> I'd usually leave it a bit longer between postings, but since this fixes
>>> issues with patches in -next I thought I'd spin a new version immediately.
>>>
>>> Cheers,
>>
>> FWIW, I just bisected virtio-errors with secure execution mode
>> qemu-system-s390x: virtio-serial-bus: Unexpected port id 4205794771 for device virtio-serial0.0
>>
>> to
>> commit 903cd0f315fe426c6a64c54ed389de0becb663dc
>> Author: Claire Chang <tientzu@chromium.org>
>> Date: Thu Jun 24 23:55:20 2021 +0800
>>
>> swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
>>
>> Unfortunately this patch series does NOT fix this issue, so it seems that even more
>> things are broken.
>>
>> Any idea what else might be broken?
>
> I've done some debugging, and I think I know what is going on. Since
> that commit we need to set force_swiotlb before the swiotlb itself is
> initialized. So the patch below should fix the problem.
>
> --------------------8<-------------------------------------
>
> From: Halil Pasic <pasic@linux.ibm.com>
> Date: Fri, 23 Jul 2021 02:57:06 +0200
> Subject: [PATCH 1/1] s390/pv: fix the forcing of the swiotlb
>
> Since commit 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for
> swiotlb data bouncing") if code sets swiotlb_force it needs to do so
> before the swiotlb is initialised. Otherwise
> io_tlb_default_mem->force_bounce will not get set to true, and devices
> that use (the default) swiotlb will not bounce despite switolb_force
> having the value of SWIOTLB_FORCE.
>
> Let us restore swiotlb functionality for PV by fulfilling this new
> requirement.
>
I would add:
Fixes: 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing")
as this patch breaks things
and
Fixes: 64e1f0c531d1 ("s390/mm: force swiotlb for protected virtualization")
to make the s390 init code more robust in case people start backporting things.
> Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
I can confirm that this fixes the problem. This also makes sense codewise.
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Konrad, Heiko, Vasily, any preference which tree this goes? I think s390
would be easiest, but that requires that the patches in the swiotlb tree have
fixed commit IDs.
> ---
> arch/s390/mm/init.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
> index 8ac710de1ab1..07bbee9b7320 100644
> --- a/arch/s390/mm/init.c
> +++ b/arch/s390/mm/init.c
> @@ -186,9 +186,9 @@ static void pv_init(void)
> return;
>
> /* make sure bounce buffers are shared */
> + swiotlb_force = SWIOTLB_FORCE;
> swiotlb_init(1);
> swiotlb_update_mem_attributes();
> - swiotlb_force = SWIOTLB_FORCE;
> }
>
> void __init mem_init(void)
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()
2021-07-23 1:12 ` Halil Pasic
2021-07-23 5:51 ` Christian Borntraeger
@ 2021-07-23 6:14 ` Christian Borntraeger
2021-07-23 8:47 ` Halil Pasic
1 sibling, 1 reply; 18+ messages in thread
From: Christian Borntraeger @ 2021-07-23 6:14 UTC (permalink / raw)
To: Halil Pasic
Cc: Will Deacon, iommu, linux-kernel, Guenter Roeck, Claire Chang,
Christoph Hellwig, Robin Murphy, Konrad Rzeszutek Wilk,
Nathan Chancellor, linux-s390, Vasily Gorbik, Heiko Carstens
Resending with the correct email of Heiko....
On 23.07.21 03:12, Halil Pasic wrote:
> On Thu, 22 Jul 2021 21:22:58 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>
>> On 20.07.21 15:38, Will Deacon wrote:
>>> Hi again, folks,
>>>
>>> This is version two of the patch series I posted yesterday:
>>>
>>> https://lore.kernel.org/r/20210719123054.6844-1-will@kernel.org
>>>
>>> The only changes since v1 are:
>>>
>>> * Squash patches 2 and 3, amending the commit message accordingly
>>> * Add Reviewed-by and Tested-by tags from Christoph and Claire (thanks!)
>>>
>>> I'd usually leave it a bit longer between postings, but since this fixes
>>> issues with patches in -next I thought I'd spin a new version immediately.
>>>
>>> Cheers,
>>
>> FWIW, I just bisected virtio-errors with secure execution mode
>> qemu-system-s390x: virtio-serial-bus: Unexpected port id 4205794771 for device virtio-serial0.0
>>
>> to
>> commit 903cd0f315fe426c6a64c54ed389de0becb663dc
>> Author: Claire Chang <tientzu@chromium.org>
>> Date: Thu Jun 24 23:55:20 2021 +0800
>>
>> swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
>>
>> Unfortunately this patch series does NOT fix this issue, so it seems that even more
>> things are broken.
>>
>> Any idea what else might be broken?
>
> I've done some debugging, and I think I know what is going on. Since
> that commit we need to set force_swiotlb before the swiotlb itself is
> initialized. So the patch below should fix the problem.
>
> --------------------8<-------------------------------------
>
> From: Halil Pasic <pasic@linux.ibm.com>
> Date: Fri, 23 Jul 2021 02:57:06 +0200
> Subject: [PATCH 1/1] s390/pv: fix the forcing of the swiotlb
>
> Since commit 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for
> swiotlb data bouncing") if code sets swiotlb_force it needs to do so
> before the swiotlb is initialised. Otherwise
> io_tlb_default_mem->force_bounce will not get set to true, and devices
> that use (the default) swiotlb will not bounce despite switolb_force
> having the value of SWIOTLB_FORCE.
>
> Let us restore swiotlb functionality for PV by fulfilling this new
> requirement.
>
I would add:
Fixes: 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing")
as this patch breaks things
and
Fixes: 64e1f0c531d1 ("s390/mm: force swiotlb for protected virtualization")
to make the s390 init code more robust in case people start backporting things.
> Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
I can confirm that this fixes the problem. This also makes sense codewise.
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Konrad, Heiko, Vasily, any preference which tree this goes? I think s390
would be easiest, but that requires that the patches in the swiotlb tree have
fixed commit IDs.
> ---
> arch/s390/mm/init.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
> index 8ac710de1ab1..07bbee9b7320 100644
> --- a/arch/s390/mm/init.c
> +++ b/arch/s390/mm/init.c
> @@ -186,9 +186,9 @@ static void pv_init(void)
> return;
>
> /* make sure bounce buffers are shared */
> + swiotlb_force = SWIOTLB_FORCE;
> swiotlb_init(1);
> swiotlb_update_mem_attributes();
> - swiotlb_force = SWIOTLB_FORCE;
> }
>
> void __init mem_init(void)
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()
2021-07-23 6:14 ` Christian Borntraeger
@ 2021-07-23 8:47 ` Halil Pasic
2021-07-23 8:50 ` Christian Borntraeger
0 siblings, 1 reply; 18+ messages in thread
From: Halil Pasic @ 2021-07-23 8:47 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Will Deacon, iommu, linux-kernel, Guenter Roeck, Claire Chang,
Christoph Hellwig, Robin Murphy, Konrad Rzeszutek Wilk,
Nathan Chancellor, linux-s390, Vasily Gorbik, Heiko Carstens
On Fri, 23 Jul 2021 08:14:19 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> Resending with the correct email of Heiko....
>
> On 23.07.21 03:12, Halil Pasic wrote:
> > On Thu, 22 Jul 2021 21:22:58 +0200
> > Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> >
> >> On 20.07.21 15:38, Will Deacon wrote:
> >>> Hi again, folks,
> >>>
> >>> This is version two of the patch series I posted yesterday:
> >>>
> >>> https://lore.kernel.org/r/20210719123054.6844-1-will@kernel.org
> >>>
> >>> The only changes since v1 are:
> >>>
> >>> * Squash patches 2 and 3, amending the commit message accordingly
> >>> * Add Reviewed-by and Tested-by tags from Christoph and Claire (thanks!)
> >>>
> >>> I'd usually leave it a bit longer between postings, but since this fixes
> >>> issues with patches in -next I thought I'd spin a new version immediately.
> >>>
> >>> Cheers,
> >>
> >> FWIW, I just bisected virtio-errors with secure execution mode
> >> qemu-system-s390x: virtio-serial-bus: Unexpected port id 4205794771 for device virtio-serial0.0
> >>
> >> to
> >> commit 903cd0f315fe426c6a64c54ed389de0becb663dc
> >> Author: Claire Chang <tientzu@chromium.org>
> >> Date: Thu Jun 24 23:55:20 2021 +0800
> >>
> >> swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
> >>
> >> Unfortunately this patch series does NOT fix this issue, so it seems that even more
> >> things are broken.
> >>
> >> Any idea what else might be broken?
> >
> > I've done some debugging, and I think I know what is going on. Since
> > that commit we need to set force_swiotlb before the swiotlb itself is
> > initialized. So the patch below should fix the problem.
> >
> > --------------------8<-------------------------------------
> >
> > From: Halil Pasic <pasic@linux.ibm.com>
> > Date: Fri, 23 Jul 2021 02:57:06 +0200
> > Subject: [PATCH 1/1] s390/pv: fix the forcing of the swiotlb
> >
> > Since commit 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for
> > swiotlb data bouncing") if code sets swiotlb_force it needs to do so
> > before the swiotlb is initialised. Otherwise
> > io_tlb_default_mem->force_bounce will not get set to true, and devices
> > that use (the default) swiotlb will not bounce despite switolb_force
> > having the value of SWIOTLB_FORCE.
> >
> > Let us restore swiotlb functionality for PV by fulfilling this new
> > requirement.
> >
> I would add:
> Fixes: 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing")
> as this patch breaks things
> and
> Fixes: 64e1f0c531d1 ("s390/mm: force swiotlb for protected virtualization")
>
> to make the s390 init code more robust in case people start backporting things.
I agree. Do we want this backported to the stable releases that have
64e1f0c531d1 (i.e. do we need a cc stable) or should the fixes tag just
serve as metadata? My guess is, it's the former. In that sense should I
add the tags along with an explanation for the second fixes respin with
cc stable?
(BTW I don't think this formally qualifies for the stable backports, but
I hope we can make an exception...)
>
> > Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
>
> I can confirm that this fixes the problem. This also makes sense codewise.
>
> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Thanks!
Regards,
Halil
>
> Konrad, Heiko, Vasily, any preference which tree this goes? I think s390
> would be easiest, but that requires that the patches in the swiotlb tree have
> fixed commit IDs.
>
> > ---
> > arch/s390/mm/init.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
> > index 8ac710de1ab1..07bbee9b7320 100644
> > --- a/arch/s390/mm/init.c
> > +++ b/arch/s390/mm/init.c
> > @@ -186,9 +186,9 @@ static void pv_init(void)
> > return;
> >
> > /* make sure bounce buffers are shared */
> > + swiotlb_force = SWIOTLB_FORCE;
> > swiotlb_init(1);
> > swiotlb_update_mem_attributes();
> > - swiotlb_force = SWIOTLB_FORCE;
> > }
> >
> > void __init mem_init(void)
> >
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()
2021-07-23 8:47 ` Halil Pasic
@ 2021-07-23 8:50 ` Christian Borntraeger
2021-07-23 14:01 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 18+ messages in thread
From: Christian Borntraeger @ 2021-07-23 8:50 UTC (permalink / raw)
To: Halil Pasic
Cc: Will Deacon, iommu, linux-kernel, Guenter Roeck, Claire Chang,
Christoph Hellwig, Robin Murphy, Konrad Rzeszutek Wilk,
Nathan Chancellor, linux-s390, Vasily Gorbik, Heiko Carstens
On 23.07.21 10:47, Halil Pasic wrote:
> On Fri, 23 Jul 2021 08:14:19 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>
>> Resending with the correct email of Heiko....
>>
>> On 23.07.21 03:12, Halil Pasic wrote:
>>> On Thu, 22 Jul 2021 21:22:58 +0200
>>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>>>
>>>> On 20.07.21 15:38, Will Deacon wrote:
>>>>> Hi again, folks,
>>>>>
>>>>> This is version two of the patch series I posted yesterday:
>>>>>
>>>>> https://lore.kernel.org/r/20210719123054.6844-1-will@kernel.org
>>>>>
>>>>> The only changes since v1 are:
>>>>>
>>>>> * Squash patches 2 and 3, amending the commit message accordingly
>>>>> * Add Reviewed-by and Tested-by tags from Christoph and Claire (thanks!)
>>>>>
>>>>> I'd usually leave it a bit longer between postings, but since this fixes
>>>>> issues with patches in -next I thought I'd spin a new version immediately.
>>>>>
>>>>> Cheers,
>>>>
>>>> FWIW, I just bisected virtio-errors with secure execution mode
>>>> qemu-system-s390x: virtio-serial-bus: Unexpected port id 4205794771 for device virtio-serial0.0
>>>>
>>>> to
>>>> commit 903cd0f315fe426c6a64c54ed389de0becb663dc
>>>> Author: Claire Chang <tientzu@chromium.org>
>>>> Date: Thu Jun 24 23:55:20 2021 +0800
>>>>
>>>> swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
>>>>
>>>> Unfortunately this patch series does NOT fix this issue, so it seems that even more
>>>> things are broken.
>>>>
>>>> Any idea what else might be broken?
>>>
>>> I've done some debugging, and I think I know what is going on. Since
>>> that commit we need to set force_swiotlb before the swiotlb itself is
>>> initialized. So the patch below should fix the problem.
>>>
>>> --------------------8<-------------------------------------
>>>
>>> From: Halil Pasic <pasic@linux.ibm.com>
>>> Date: Fri, 23 Jul 2021 02:57:06 +0200
>>> Subject: [PATCH 1/1] s390/pv: fix the forcing of the swiotlb
>>>
>>> Since commit 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for
>>> swiotlb data bouncing") if code sets swiotlb_force it needs to do so
>>> before the swiotlb is initialised. Otherwise
>>> io_tlb_default_mem->force_bounce will not get set to true, and devices
>>> that use (the default) swiotlb will not bounce despite switolb_force
>>> having the value of SWIOTLB_FORCE.
>>>
>>> Let us restore swiotlb functionality for PV by fulfilling this new
>>> requirement.
>>>
>> I would add:
>> Fixes: 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing")
>> as this patch breaks things
>> and
>> Fixes: 64e1f0c531d1 ("s390/mm: force swiotlb for protected virtualization")
>>
>> to make the s390 init code more robust in case people start backporting things.
>
> I agree. Do we want this backported to the stable releases that have
> 64e1f0c531d1 (i.e. do we need a cc stable) or should the fixes tag just
> serve as metadata? My guess is, it's the former. In that sense should I
> add the tags along with an explanation for the second fixes respin with
> cc stable?
>
> (BTW I don't think this formally qualifies for the stable backports, but
> I hope we can make an exception...)
I think it makes sense for stable as it is cleaner to set the flags before
calling the init function. cc stable would be better and the right way
according to process, but the Fixes tag is mostly enough.
>
>>
>>> Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
>>
>> I can confirm that this fixes the problem. This also makes sense codewise.
>>
>> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
>
> Thanks!
>
> Regards,
> Halil
>>
>> Konrad, Heiko, Vasily, any preference which tree this goes? I think s390
>> would be easiest, but that requires that the patches in the swiotlb tree have
>> fixed commit IDs.
>>
>>> ---
>>> arch/s390/mm/init.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
>>> index 8ac710de1ab1..07bbee9b7320 100644
>>> --- a/arch/s390/mm/init.c
>>> +++ b/arch/s390/mm/init.c
>>> @@ -186,9 +186,9 @@ static void pv_init(void)
>>> return;
>>>
>>> /* make sure bounce buffers are shared */
>>> + swiotlb_force = SWIOTLB_FORCE;
>>> swiotlb_init(1);
>>> swiotlb_update_mem_attributes();
>>> - swiotlb_force = SWIOTLB_FORCE;
>>> }
>>>
>>> void __init mem_init(void)
>>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()
2021-07-23 8:50 ` Christian Borntraeger
@ 2021-07-23 14:01 ` Konrad Rzeszutek Wilk
2021-07-23 17:53 ` Christian Borntraeger
0 siblings, 1 reply; 18+ messages in thread
From: Konrad Rzeszutek Wilk @ 2021-07-23 14:01 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Halil Pasic, Will Deacon, iommu, linux-kernel, Guenter Roeck,
Claire Chang, Christoph Hellwig, Robin Murphy, Nathan Chancellor,
linux-s390, Vasily Gorbik, Heiko Carstens
On Fri, Jul 23, 2021 at 10:50:57AM +0200, Christian Borntraeger wrote:
>
>
> On 23.07.21 10:47, Halil Pasic wrote:
> > On Fri, 23 Jul 2021 08:14:19 +0200
> > Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> >
> > > Resending with the correct email of Heiko....
> > >
> > > On 23.07.21 03:12, Halil Pasic wrote:
> > > > On Thu, 22 Jul 2021 21:22:58 +0200
> > > > Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> > > > > On 20.07.21 15:38, Will Deacon wrote:
> > > > > > Hi again, folks,
> > > > > >
> > > > > > This is version two of the patch series I posted yesterday:
> > > > > >
> > > > > > https://lore.kernel.org/r/20210719123054.6844-1-will@kernel.org
> > > > > >
> > > > > > The only changes since v1 are:
> > > > > >
> > > > > > * Squash patches 2 and 3, amending the commit message accordingly
> > > > > > * Add Reviewed-by and Tested-by tags from Christoph and Claire (thanks!)
> > > > > >
> > > > > > I'd usually leave it a bit longer between postings, but since this fixes
> > > > > > issues with patches in -next I thought I'd spin a new version immediately.
> > > > > >
> > > > > > Cheers,
> > > > >
> > > > > FWIW, I just bisected virtio-errors with secure execution mode
> > > > > qemu-system-s390x: virtio-serial-bus: Unexpected port id 4205794771 for device virtio-serial0.0
> > > > >
> > > > > to
> > > > > commit 903cd0f315fe426c6a64c54ed389de0becb663dc
> > > > > Author: Claire Chang <tientzu@chromium.org>
> > > > > Date: Thu Jun 24 23:55:20 2021 +0800
> > > > >
> > > > > swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
> > > > >
> > > > > Unfortunately this patch series does NOT fix this issue, so it seems that even more
> > > > > things are broken.
> > > > >
> > > > > Any idea what else might be broken?
> > > >
> > > > I've done some debugging, and I think I know what is going on. Since
> > > > that commit we need to set force_swiotlb before the swiotlb itself is
> > > > initialized. So the patch below should fix the problem.
> > > >
> > > > --------------------8<-------------------------------------
> > > >
> > > > From: Halil Pasic <pasic@linux.ibm.com>
> > > > Date: Fri, 23 Jul 2021 02:57:06 +0200
> > > > Subject: [PATCH 1/1] s390/pv: fix the forcing of the swiotlb
> > > >
> > > > Since commit 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for
> > > > swiotlb data bouncing") if code sets swiotlb_force it needs to do so
> > > > before the swiotlb is initialised. Otherwise
> > > > io_tlb_default_mem->force_bounce will not get set to true, and devices
> > > > that use (the default) swiotlb will not bounce despite switolb_force
> > > > having the value of SWIOTLB_FORCE.
> > > >
> > > > Let us restore swiotlb functionality for PV by fulfilling this new
> > > > requirement.
> > > I would add:
> > > Fixes: 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing")
> > > as this patch breaks things
> > > and
> > > Fixes: 64e1f0c531d1 ("s390/mm: force swiotlb for protected virtualization")
> > >
> > > to make the s390 init code more robust in case people start backporting things.
> >
> > I agree. Do we want this backported to the stable releases that have
> > 64e1f0c531d1 (i.e. do we need a cc stable) or should the fixes tag just
> > serve as metadata? My guess is, it's the former. In that sense should I
> > add the tags along with an explanation for the second fixes respin with
> > cc stable?
> >
> > (BTW I don't think this formally qualifies for the stable backports, but
> > I hope we can make an exception...)
>
> I think it makes sense for stable as it is cleaner to set the flags before
> calling the init function. cc stable would be better and the right way
> according to process, but the Fixes tag is mostly enough.
But the reaso for fixing this is for code that is not yet in Linus's
tree?
I can just pick this patch up and add it in the pile I have for the next
merge window?
>
> >
> > >
> > > > Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
> > >
> > > I can confirm that this fixes the problem. This also makes sense codewise.
> > >
> > > Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > > Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
> >
> > Thanks!
> >
> > Regards,
> > Halil
> > >
> > > Konrad, Heiko, Vasily, any preference which tree this goes? I think s390
> > > would be easiest, but that requires that the patches in the swiotlb tree have
> > > fixed commit IDs.
> > >
> > > > ---
> > > > arch/s390/mm/init.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
> > > > index 8ac710de1ab1..07bbee9b7320 100644
> > > > --- a/arch/s390/mm/init.c
> > > > +++ b/arch/s390/mm/init.c
> > > > @@ -186,9 +186,9 @@ static void pv_init(void)
> > > > return;
> > > > /* make sure bounce buffers are shared */
> > > > + swiotlb_force = SWIOTLB_FORCE;
> > > > swiotlb_init(1);
> > > > swiotlb_update_mem_attributes();
> > > > - swiotlb_force = SWIOTLB_FORCE;
> > > > }
> > > > void __init mem_init(void)
> >
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()
2021-07-23 14:01 ` Konrad Rzeszutek Wilk
@ 2021-07-23 17:53 ` Christian Borntraeger
2021-07-23 22:18 ` Halil Pasic
0 siblings, 1 reply; 18+ messages in thread
From: Christian Borntraeger @ 2021-07-23 17:53 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Halil Pasic, Will Deacon, iommu, linux-kernel, Guenter Roeck,
Claire Chang, Christoph Hellwig, Robin Murphy, Nathan Chancellor,
linux-s390, Vasily Gorbik, Heiko Carstens
On 23.07.21 16:01, Konrad Rzeszutek Wilk wrote:
> On Fri, Jul 23, 2021 at 10:50:57AM +0200, Christian Borntraeger wrote:
>>
>>
>> On 23.07.21 10:47, Halil Pasic wrote:
>>> On Fri, 23 Jul 2021 08:14:19 +0200
>>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>>>
>>>> Resending with the correct email of Heiko....
>>>>
>>>> On 23.07.21 03:12, Halil Pasic wrote:
>>>>> On Thu, 22 Jul 2021 21:22:58 +0200
>>>>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>>>>>> On 20.07.21 15:38, Will Deacon wrote:
>>>>>>> Hi again, folks,
>>>>>>>
>>>>>>> This is version two of the patch series I posted yesterday:
>>>>>>>
>>>>>>> https://lore.kernel.org/r/20210719123054.6844-1-will@kernel.org
>>>>>>>
>>>>>>> The only changes since v1 are:
>>>>>>>
>>>>>>> * Squash patches 2 and 3, amending the commit message accordingly
>>>>>>> * Add Reviewed-by and Tested-by tags from Christoph and Claire (thanks!)
>>>>>>>
>>>>>>> I'd usually leave it a bit longer between postings, but since this fixes
>>>>>>> issues with patches in -next I thought I'd spin a new version immediately.
>>>>>>>
>>>>>>> Cheers,
>>>>>>
>>>>>> FWIW, I just bisected virtio-errors with secure execution mode
>>>>>> qemu-system-s390x: virtio-serial-bus: Unexpected port id 4205794771 for device virtio-serial0.0
>>>>>>
>>>>>> to
>>>>>> commit 903cd0f315fe426c6a64c54ed389de0becb663dc
>>>>>> Author: Claire Chang <tientzu@chromium.org>
>>>>>> Date: Thu Jun 24 23:55:20 2021 +0800
>>>>>>
>>>>>> swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
>>>>>>
>>>>>> Unfortunately this patch series does NOT fix this issue, so it seems that even more
>>>>>> things are broken.
>>>>>>
>>>>>> Any idea what else might be broken?
>>>>>
>>>>> I've done some debugging, and I think I know what is going on. Since
>>>>> that commit we need to set force_swiotlb before the swiotlb itself is
>>>>> initialized. So the patch below should fix the problem.
>>>>>
>>>>> --------------------8<-------------------------------------
>>>>>
>>>>> From: Halil Pasic <pasic@linux.ibm.com>
>>>>> Date: Fri, 23 Jul 2021 02:57:06 +0200
>>>>> Subject: [PATCH 1/1] s390/pv: fix the forcing of the swiotlb
>>>>>
>>>>> Since commit 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for
>>>>> swiotlb data bouncing") if code sets swiotlb_force it needs to do so
>>>>> before the swiotlb is initialised. Otherwise
>>>>> io_tlb_default_mem->force_bounce will not get set to true, and devices
>>>>> that use (the default) swiotlb will not bounce despite switolb_force
>>>>> having the value of SWIOTLB_FORCE.
>>>>>
>>>>> Let us restore swiotlb functionality for PV by fulfilling this new
>>>>> requirement.
>>>> I would add:
>>>> Fixes: 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing")
>>>> as this patch breaks things
>>>> and
>>>> Fixes: 64e1f0c531d1 ("s390/mm: force swiotlb for protected virtualization")
>>>>
>>>> to make the s390 init code more robust in case people start backporting things.
>>>
>>> I agree. Do we want this backported to the stable releases that have
>>> 64e1f0c531d1 (i.e. do we need a cc stable) or should the fixes tag just
>>> serve as metadata? My guess is, it's the former. In that sense should I
>>> add the tags along with an explanation for the second fixes respin with
>>> cc stable?
>>>
>>> (BTW I don't think this formally qualifies for the stable backports, but
>>> I hope we can make an exception...)
>>
>> I think it makes sense for stable as it is cleaner to set the flags before
>> calling the init function. cc stable would be better and the right way
>> according to process, but the Fixes tag is mostly enough.
>
> But the reaso for fixing this is for code that is not yet in Linus's
> tree?
>
> I can just pick this patch up and add it in the pile I have for the next
> merge window?
That would also work for me. I think Halil wanted to send out and v2.
In any case
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
so that you can take this via the swiotlb tree.
>>
>>>
>>>>
>>>>> Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
>>>>
>>>> I can confirm that this fixes the problem. This also makes sense codewise.
>>>>
>>>> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>>> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>>
>>> Thanks!
>>>
>>> Regards,
>>> Halil
>>>>
>>>> Konrad, Heiko, Vasily, any preference which tree this goes? I think s390
>>>> would be easiest, but that requires that the patches in the swiotlb tree have
>>>> fixed commit IDs.
>>>>
>>>>> ---
>>>>> arch/s390/mm/init.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
>>>>> index 8ac710de1ab1..07bbee9b7320 100644
>>>>> --- a/arch/s390/mm/init.c
>>>>> +++ b/arch/s390/mm/init.c
>>>>> @@ -186,9 +186,9 @@ static void pv_init(void)
>>>>> return;
>>>>> /* make sure bounce buffers are shared */
>>>>> + swiotlb_force = SWIOTLB_FORCE;
>>>>> swiotlb_init(1);
>>>>> swiotlb_update_mem_attributes();
>>>>> - swiotlb_force = SWIOTLB_FORCE;
>>>>> }
>>>>> void __init mem_init(void)
>>>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()
2021-07-23 17:53 ` Christian Borntraeger
@ 2021-07-23 22:18 ` Halil Pasic
0 siblings, 0 replies; 18+ messages in thread
From: Halil Pasic @ 2021-07-23 22:18 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Konrad Rzeszutek Wilk, Will Deacon, iommu, linux-kernel,
Guenter Roeck, Claire Chang, Christoph Hellwig, Robin Murphy,
Nathan Chancellor, linux-s390, Vasily Gorbik, Heiko Carstens
On Fri, 23 Jul 2021 19:53:58 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> On 23.07.21 16:01, Konrad Rzeszutek Wilk wrote:
> > On Fri, Jul 23, 2021 at 10:50:57AM +0200, Christian Borntraeger wrote:
> >>
> >>
> >> On 23.07.21 10:47, Halil Pasic wrote:
> >>> On Fri, 23 Jul 2021 08:14:19 +0200
> >>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> >>>
> >>>> Resending with the correct email of Heiko....
> >>>>
> >>>> On 23.07.21 03:12, Halil Pasic wrote:
> >>>>> On Thu, 22 Jul 2021 21:22:58 +0200
> >>>>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> >>>>>> On 20.07.21 15:38, Will Deacon wrote:
> >>>>>>> Hi again, folks,
> >>>>>>>
> >>>>>>> This is version two of the patch series I posted yesterday:
> >>>>>>>
> >>>>>>> https://lore.kernel.org/r/20210719123054.6844-1-will@kernel.org
> >>>>>>>
> >>>>>>> The only changes since v1 are:
> >>>>>>>
> >>>>>>> * Squash patches 2 and 3, amending the commit message accordingly
> >>>>>>> * Add Reviewed-by and Tested-by tags from Christoph and Claire (thanks!)
> >>>>>>>
> >>>>>>> I'd usually leave it a bit longer between postings, but since this fixes
> >>>>>>> issues with patches in -next I thought I'd spin a new version immediately.
> >>>>>>>
> >>>>>>> Cheers,
> >>>>>>
> >>>>>> FWIW, I just bisected virtio-errors with secure execution mode
> >>>>>> qemu-system-s390x: virtio-serial-bus: Unexpected port id 4205794771 for device virtio-serial0.0
> >>>>>>
> >>>>>> to
> >>>>>> commit 903cd0f315fe426c6a64c54ed389de0becb663dc
> >>>>>> Author: Claire Chang <tientzu@chromium.org>
> >>>>>> Date: Thu Jun 24 23:55:20 2021 +0800
> >>>>>>
> >>>>>> swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
> >>>>>>
> >>>>>> Unfortunately this patch series does NOT fix this issue, so it seems that even more
> >>>>>> things are broken.
> >>>>>>
> >>>>>> Any idea what else might be broken?
> >>>>>
> >>>>> I've done some debugging, and I think I know what is going on. Since
> >>>>> that commit we need to set force_swiotlb before the swiotlb itself is
> >>>>> initialized. So the patch below should fix the problem.
> >>>>>
> >>>>> --------------------8<-------------------------------------
> >>>>>
> >>>>> From: Halil Pasic <pasic@linux.ibm.com>
> >>>>> Date: Fri, 23 Jul 2021 02:57:06 +0200
> >>>>> Subject: [PATCH 1/1] s390/pv: fix the forcing of the swiotlb
> >>>>>
> >>>>> Since commit 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for
> >>>>> swiotlb data bouncing") if code sets swiotlb_force it needs to do so
> >>>>> before the swiotlb is initialised. Otherwise
> >>>>> io_tlb_default_mem->force_bounce will not get set to true, and devices
> >>>>> that use (the default) swiotlb will not bounce despite switolb_force
> >>>>> having the value of SWIOTLB_FORCE.
> >>>>>
> >>>>> Let us restore swiotlb functionality for PV by fulfilling this new
> >>>>> requirement.
> >>>> I would add:
> >>>> Fixes: 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing")
> >>>> as this patch breaks things
> >>>> and
> >>>> Fixes: 64e1f0c531d1 ("s390/mm: force swiotlb for protected virtualization")
> >>>>
> >>>> to make the s390 init code more robust in case people start backporting things.
> >>>
> >>> I agree. Do we want this backported to the stable releases that have
> >>> 64e1f0c531d1 (i.e. do we need a cc stable) or should the fixes tag just
> >>> serve as metadata? My guess is, it's the former. In that sense should I
> >>> add the tags along with an explanation for the second fixes respin with
> >>> cc stable?
> >>>
> >>> (BTW I don't think this formally qualifies for the stable backports, but
> >>> I hope we can make an exception...)
> >>
> >> I think it makes sense for stable as it is cleaner to set the flags before
> >> calling the init function. cc stable would be better and the right way
> >> according to process, but the Fixes tag is mostly enough.
> >
> > But the reaso for fixing this is for code that is not yet in Linus's
> > tree?
> >
> > I can just pick this patch up and add it in the pile I have for the next
> > merge window?
>
> That would also work for me. I think Halil wanted to send out and v2.
Sorry I didn't interpret your answer correctly. (I interpreted it
like the fixes tags are enough, and those can be added by the maintainer
that is going to merge the patch.) I will send out a v2 right away.
Regards,
Halil
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()
2021-07-20 13:38 [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit() Will Deacon
` (4 preceding siblings ...)
2021-07-22 19:22 ` [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit() Christian Borntraeger
@ 2021-07-24 0:29 ` Konrad Rzeszutek Wilk
5 siblings, 0 replies; 18+ messages in thread
From: Konrad Rzeszutek Wilk @ 2021-07-24 0:29 UTC (permalink / raw)
To: Will Deacon
Cc: iommu, linux-kernel, Guenter Roeck, Claire Chang,
Christoph Hellwig, Robin Murphy, Konrad Rzeszutek Wilk,
Nathan Chancellor
On Tue, Jul 20, 2021 at 02:38:22PM +0100, Will Deacon wrote:
> Hi again, folks,
>
> This is version two of the patch series I posted yesterday:
>
> https://lore.kernel.org/r/20210719123054.6844-1-will@kernel.org
>
> The only changes since v1 are:
>
> * Squash patches 2 and 3, amending the commit message accordingly
> * Add Reviewed-by and Tested-by tags from Christoph and Claire (thanks!)
>
> I'd usually leave it a bit longer between postings, but since this fixes
> issues with patches in -next I thought I'd spin a new version immediately.
Thank you!
I put them in devel/for-linus-5.15 and linux-next.
^ permalink raw reply [flat|nested] 18+ messages in thread