Skip to contentSkip to conversation

x86/hyperv/Swiotlb: Add swiotlb_set_alloc_from_low_pages() switch function.

PATCH In review Tianyu Lan 2 participants 15 messages Feb 9, 2022 – Mar 1, 2022

Subsystems (4)
x86 Hyper-V/Azure core and drivers Microsoft MANA RDMA driver DMA mapping helpers
lore
Share & tools
lore

Tianyu Lan

author
#0 lore raw

v2 cover x86/hyperv/Swiotlb: Add swiotlb_set_alloc_from_low_pages() switch function.

From: Tianyu Lan <[email protected]>

Hyper-V Isolation VM may fail to allocate swiotlb bounce buffer due to there is no enough contiguous memory from 0 to 4G in some cases. Current swiotlb code allocates bounce buffer in the low end memory. This patchset adds a new function swiotlb_set_alloc_from_low_pages() to control swiotlb bounce buffer from low pages or no limitation. Devices in Hyper-V Isolation VM may use memory above 4G as DMA memory and switch swiotlb allocation in order to avoid no enough contiguous memory in low pages.

Tianyu Lan (2):
Swiotlb: Add swiotlb_alloc_from_low_pages switch x86/hyperv: Make swiotlb bounce buffer allocation not just from low
pages

 arch/x86/kernel/cpu/mshyperv.c |  1 +
 include/linux/swiotlb.h        |  1 +
 kernel/dma/swiotlb.c           | 18 ++++++++++++++++--
 3 files changed, 18 insertions(+), 2 deletions(-)
Signature · 1 line
2.25.1
author
#1 lore raw

PATCH v2 1/2 Swiotlb: Add swiotlb_alloc_from_low_pages switch

From: Tianyu Lan <[email protected]>

Hyper-V Isolation VM and AMD SEV VM uses swiotlb bounce buffer to share memory with hypervisor. Current swiotlb bounce buffer is only allocated from 0 to ARCH_LOW_ADDRESS_LIMIT which is default to 0xffffffffUL. Isolation VM and AMD SEV VM needs 1G bounce buffer at most. This will fail when there is not enough memory from 0 to 4G address space and devices also may use memory above 4G address space as DMA memory. Expose swiotlb_alloc_from_low_pages and platform mey set it to false when it's not necessary to limit bounce buffer from 0 to 4G memory.

Signed-off-by Tianyu Lan <[email protected]>
---
 include/linux/swiotlb.h |  1 +
 kernel/dma/swiotlb.c    | 18 ++++++++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)
include/linux/swiotlb.h +1 -0
index f6c3638255d5..2b4f92668bc7 100644
@@ -39,6 +39,7 @@ enum swiotlb_force {
 extern void swiotlb_init(int verbose);
 int swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose);
 unsigned long swiotlb_size_or_default(void);
+void swiotlb_set_alloc_from_low_pages(bool low);
 extern int swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs);
 extern int swiotlb_late_init_with_default_size(size_t default_size);
 extern void __init swiotlb_update_mem_attributes(void);
kernel/dma/swiotlb.c +16 -2
index f1e7ea160b43..62bf8b5cc3e4 100644
@@ -73,6 +73,8 @@ enum swiotlb_force swiotlb_force;
 
 struct io_tlb_mem io_tlb_default_mem;
 
+static bool swiotlb_alloc_from_low_pages = true;
+
 phys_addr_t swiotlb_unencrypted_base;
 
 /*

@@ -116,6 +118,11 @@ void swiotlb_set_max_segment(unsigned int val)
 		max_segment = rounddown(val, PAGE_SIZE);
 }
 
+void swiotlb_set_alloc_from_low_pages(bool low)
+{
+	swiotlb_alloc_from_low_pages = low;
+}
+
 unsigned long swiotlb_size_or_default(void)
 {
 	return default_nslabs << IO_TLB_SHIFT;

@@ -284,8 +291,15 @@ swiotlb_init(int verbose)
 	if (swiotlb_force == SWIOTLB_NO_FORCE)
 		return;
 
-	/* Get IO TLB memory from the low pages */
-	tlb = memblock_alloc_low(bytes, PAGE_SIZE);
+	/*
+	 * Get IO TLB memory from the low pages if swiotlb_alloc_from_low_pages
+	 * is set.
+	 */
+	if (swiotlb_alloc_from_low_pages)
+		tlb = memblock_alloc_low(bytes, PAGE_SIZE);
+	else
+		tlb = memblock_alloc(bytes, PAGE_SIZE);
+
 	if (!tlb)
 		goto fail;
 	if (swiotlb_init_with_tbl(tlb, default_nslabs, verbose))
Signature · 6 lines
2.25.1

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
#2 lore raw

Re: [PATCH V2 1/2] Swiotlb: Add swiotlb_alloc_from_low_pages switch

Adding a function to set the flag doesn't really change much. As Robin pointed out last time you should fine a way to just call swiotlb_init_with_tbl directly with the memory allocated the way you like it. Or given that we have quite a few of these trusted hypervisor schemes maybe add an argument to swiotlb_init that specifies how to allocate the memory.

author
#3 lore raw

Re: [PATCH V2 1/2] Swiotlb: Add swiotlb_alloc_from_low_pages switch

Quoted 6 lines · Christoph Hellwig

Adding a function to set the flag doesn't really change much. As Robin pointed out last time you should fine a way to just call swiotlb_init_with_tbl directly with the memory allocated the way you like it. Or given that we have quite a few of these trusted hypervisor schemes maybe add an argument to swiotlb_init that specifies how to allocate the memory.

Thanks for your suggestion. I will try the first approach first approach.

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

#4 lore raw

Re: [PATCH V2 1/2] Swiotlb: Add swiotlb_alloc_from_low_pages switch

Quoted 9 lines · Tianyu Lan
Quoted 6 lines · Christoph Hellwig

Adding a function to set the flag doesn't really change much. As Robin pointed out last time you should fine a way to just call swiotlb_init_with_tbl directly with the memory allocated the way you like it. Or given that we have quite a few of these trusted hypervisor schemes maybe add an argument to swiotlb_init that specifies how to allocate the memory.

Thanks for your suggestion. I will try the first approach first approach.

Take a look at the SWIOTLB_ANY flag in this WIP branch:

http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/swiotlb-init-cleanup

That being said I'm not sure that either this flag or the existing powerpc code iѕ actually the right thing to do. We still need the 4G limited buffer to support devices with addressing limitations. So I think we need an additional io_tlb_mem instance for the devices without addressing limitations instead.

author
#5 lore raw

Re: [PATCH V2 1/2] Swiotlb: Add swiotlb_alloc_from_low_pages switch

Quoted 13 lines · Christoph Hellwig
Quoted 9 lines · Tianyu Lan
Quoted 6 lines · Christoph Hellwig

Adding a function to set the flag doesn't really change much. As Robin pointed out last time you should fine a way to just call swiotlb_init_with_tbl directly with the memory allocated the way you like it. Or given that we have quite a few of these trusted hypervisor schemes maybe add an argument to swiotlb_init that specifies how to allocate the memory.

Thanks for your suggestion. I will try the first approach first approach.

Take a look at the SWIOTLB_ANY flag in this WIP branch:

Quoted 8 lines

http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/swiotlb-init-cleanup

That being said I'm not sure that either this flag or the existing powerpc code iѕ actually the right thing to do. We still need the 4G limited buffer to support devices with addressing limitations. So I think we need an additional io_tlb_mem instance for the devices without addressing limitations instead.

Hi Christoph:
Thanks for your patches. I tested these patches in Hyper-V trusted
VM and system can't boot up. I am debugging and will report back.

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

author
#6 lore raw

Re: [PATCH V2 1/2] Swiotlb: Add swiotlb_alloc_from_low_pages switch

Quoted 25 lines · Tianyu Lan
Quoted 14 lines · Christoph Hellwig
Quoted 9 lines · Tianyu Lan
Quoted 6 lines · Christoph Hellwig

Adding a function to set the flag doesn't really change much.  As Robin pointed out last time you should fine a way to just call swiotlb_init_with_tbl directly with the memory allocated the way you like it.  Or given that we have quite a few of these trusted hypervisor schemes maybe add an argument to swiotlb_init that specifies how to allocate the memory.

Thanks for your suggestion. I will try the first approach first approach.

Take a look at the SWIOTLB_ANY flag in this WIP branch:

Quoted 9 lines

http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/swiotlb-init-cleanup

That being said I'm not sure that either this flag or the existing powerpc code iѕ actually the right thing to do.  We still need the 4G limited buffer to support devices with addressing limitations.  So I think we need an additional io_tlb_mem instance for the devices without addressing limitations instead.

Quoted 3 lines

Hi Christoph:
     Thanks for your patches. I tested these patches in Hyper-V trusted
VM and system can't boot up. I am debugging and will report back.

Sorry. The boot failure is not related with these patches and the issue has been fixed in the latest upstream code.

There is a performance bottleneck due to io tlb mem's spin lock during
performance test. All devices'io queues uses same io tlb mem entry
and the spin lock of io tlb mem introduce overheads. There is a fix patch from Andi Kleen in the github. Could you have a look?

https://github.com/intel/tdx/commit/4529b5784c141782c72ec9bd9a92df2b68cb7d45

Thanks.

#7 lore raw

Re: [PATCH V2 1/2] Swiotlb: Add swiotlb_alloc_from_low_pages switch

Quoted 9 lines · Tianyu Lan

Sorry. The boot failure is not related with these patches and the issue has been fixed in the latest upstream code.

There is a performance bottleneck due to io tlb mem's spin lock during performance test. All devices'io queues uses same io tlb mem entry and the spin lock of io tlb mem introduce overheads. There is a fix patch from Andi Kleen in the github. Could you have a look?

https://github.com/intel/tdx/commit/4529b5784c141782c72ec9bd9a92df2b68cb7d45

Please post these things to the list.

But I suspect the right answer for the "secure" hypervisor case is to use the per-device swiotlb regions that we've recently added.

author
#8 lore raw

Re: [PATCH V2 1/2] Swiotlb: Add swiotlb_alloc_from_low_pages switch

Quoted 9 lines · Christoph Hellwig
Quoted 8 lines · Tianyu Lan

Sorry. The boot failure is not related with these patches and the issue has been fixed in the latest upstream code.

There is a performance bottleneck due to io tlb mem's spin lock during performance test. All devices'io queues uses same io tlb mem entry and the spin lock of io tlb mem introduce overheads. There is a fix patch from Andi Kleen in the github. Could you have a look?

Quoted 6 lines
Quoted 1 line

https://github.com/intel/tdx/commit/4529b5784c141782c72ec9bd9a92df2b68cb7d45

Please post these things to the list.

But I suspect the right answer for the "secure" hypervisor case is to use the per-device swiotlb regions that we've recently added.

Thanks for your comment. That means we need to expose an swiotlb_device_init() interface to allocate bounce buffer and initialize io tlb mem entry. DMA API Current rmem_swiotlb_device_init() only works
for platform with device tree. The new API should be called in the bus
driver or new DMA API. Could you check whether this is a right way before we start the work.

Thanks.

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

#9 lore raw

Re: [PATCH V2 1/2] Swiotlb: Add swiotlb_alloc_from_low_pages switch

Quoted 6 lines · Tianyu Lan

Thanks for your comment. That means we need to expose an swiotlb_device_init() interface to allocate bounce buffer and initialize io tlb mem entry. DMA API Current rmem_swiotlb_device_init() only works for platform with device tree. The new API should be called in the bus driver or new DMA API. Could you check whether this is a right way before we start the work.

Do these VMs use ACPI? We'd probably really want some kind of higher level configuration and not have the drivers request it themselves.

author
#10 lore raw

Re: [PATCH V2 1/2] Swiotlb: Add swiotlb_alloc_from_low_pages switch

Quoted 10 lines · Christoph Hellwig
Quoted 6 lines · Tianyu Lan

Thanks for your comment. That means we need to expose an swiotlb_device_init() interface to allocate bounce buffer and initialize io tlb mem entry. DMA API Current rmem_swiotlb_device_init() only works for platform with device tree. The new API should be called in the bus driver or new DMA API. Could you check whether this is a right way before we start the work.

Do these VMs use ACPI? We'd probably really want some kind of higher level configuration and not have the drivers request it themselves.

Yes, Hyper-V isolation VM uses ACPI. Devices are enumerated via vmbus host and there is no child device information in ACPI table. The host driver seems to be the right place to call new API.
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

author
#11 lore raw

Re: [PATCH V2 1/2] Swiotlb: Add swiotlb_alloc_from_low_pages switch

Quoted 11 lines · Tianyu Lan
Quoted 8 lines · Christoph Hellwig
Quoted 6 lines · Tianyu Lan

Thanks for your comment. That means we need to expose an swiotlb_device_init() interface to allocate bounce buffer and initialize io tlb mem entry. DMA API Current  rmem_swiotlb_device_init() only works for platform with device tree. The new API should be called in the bus driver or new DMA API. Could you check whether this is a right way before we start the work.

Quoted 4 lines
Quoted 2 lines

Do these VMs use ACPI?  We'd probably really want some kind of higher level configuration and not have the drivers request it themselves.

Yes, Hyper-V isolation VM uses ACPI. Devices are enumerated via vmbus host and there is no child device information in ACPI table. The host driver seems to be the right place to call new API.

Hi Christoph:
One more perspective is that one device may have multiple queues
and each queues should have independent swiotlb bounce buffer to avoid spin lock overhead. The number of queues is only available in the device driver. This means new API needs to be called in the device driver according to queue number.

#12 lore raw

Re: [PATCH V2 1/2] Swiotlb: Add swiotlb_alloc_from_low_pages switch

Quoted 5 lines · Tianyu Lan

One more perspective is that one device may have multiple queues and
each queues should have independent swiotlb bounce buffer to avoid spin lock overhead. The number of queues is only available in the device driver. This means new API needs to be called in the device driver according to queue number.

Well, given how hell bent people are on bounce buffering we might need some scalability work there anyway.

author
#13 lore raw

Re: [PATCH V2 1/2] Swiotlb: Add swiotlb_alloc_from_low_pages switch

Quoted 9 lines · Christoph Hellwig
Quoted 5 lines · Tianyu Lan

One more perspective is that one device may have multiple queues and
each queues should have independent swiotlb bounce buffer to avoid spin lock overhead. The number of queues is only available in the device driver. This means new API needs to be called in the device driver according to queue number.

Well, given how hell bent people are on bounce buffering we might need some scalability work there anyway.

According to my test on the local machine with two VMs, Linux guest without swiotlb bounce buffer or with the fix patch from Andi Kleen can achieve about 40G/s throughput but it's just 24-25G/s with current swiotlb code. Otherwise, the spinlock contention also consumes more cpu usage.

author
#14 lore raw

PATCH v2 2/2 x86/hyperv: Make swiotlb bounce buffer allocation not just from low pages

In reply to Tianyu Lan

From: Tianyu Lan <[email protected]>

In Hyper-V Isolation VM, swiotlb bnounce buffer size maybe 1G at most and there maybe no enough memory from 0 to 4G according to memory layout. Devices in Isolation VM can use memory above 4G as DMA memory and call swiotlb_alloc_from_low_pages() to allocate swiotlb bounce buffer not limit from 0 to 4G.

Signed-off-by Tianyu Lan <[email protected]>
---
 arch/x86/kernel/cpu/mshyperv.c | 1 +
 1 file changed, 1 insertion(+)
arch/x86/kernel/cpu/mshyperv.c +1 -0
index 5a99f993e639..50ba4622c650 100644
@@ -343,6 +343,7 @@ static void __init ms_hyperv_init_platform(void)
 		 * use swiotlb bounce buffer for dma transaction.
 		 */
 		swiotlb_force = SWIOTLB_FORCE;
+		swiotlb_set_alloc_from_low_pages(false);
 #endif
 	}
Signature · 1 line
2.25.1