Messages in this thread Patch in this message | | | Date | Mon, 23 Nov 2020 18:06:47 +0100 | From | Borislav Petkov <> | Subject | Re: [PATCH v6] swiotlb: Adjust SWIOTBL bounce buffer size for SEV guests. |
| |
On Thu, Nov 19, 2020 at 09:42:05PM +0000, Ashish Kalra wrote: > From: Ashish Kalra <ashish.kalra@amd.com> > > For SEV, all DMA to and from guest has to use shared (un-encrypted) pages. > SEV uses SWIOTLB to make this happen without requiring changes to device > drivers. However, depending on workload being run, the default 64MB of > SWIOTLB might not be enough and SWIOTLB may run out of buffers to use > for DMA, resulting in I/O errors and/or performance degradation for > high I/O workloads. > > Increase the default size of SWIOTLB for SEV guests using a minimum > value of 128MB and a maximum value of 512MB, determining on amount > of provisioned guest memory.
That sentence needs massaging.
> Using late_initcall() interface to invoke swiotlb_adjust() does not > work as the size adjustment needs to be done before mem_encrypt_init() > and reserve_crashkernel() which use the allocated SWIOTLB buffer size, > hence calling it explicitly from setup_arch().
"hence call it ... "
> > The SWIOTLB default size adjustment is added as an architecture specific
"... is added... " needs to be "Add ..."
> interface/callback to allow architectures such as those supporting memory > encryption to adjust/expand SWIOTLB size for their use. > > v5 fixed build errors and warnings as > Reported-by: kbuild test robot <lkp@intel.com> > > Signed-off-by: Ashish Kalra <ashish.kalra@amd.com> > --- > arch/x86/kernel/setup.c | 2 ++ > arch/x86/mm/mem_encrypt.c | 32 ++++++++++++++++++++++++++++++++ > include/linux/swiotlb.h | 6 ++++++ > kernel/dma/swiotlb.c | 24 ++++++++++++++++++++++++ > 4 files changed, 64 insertions(+) > > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c > index 3511736fbc74..b073d58dd4a3 100644 > --- a/arch/x86/kernel/setup.c > +++ b/arch/x86/kernel/setup.c > @@ -1166,6 +1166,8 @@ void __init setup_arch(char **cmdline_p) > if (boot_cpu_has(X86_FEATURE_GBPAGES)) > hugetlb_cma_reserve(PUD_SHIFT - PAGE_SHIFT); > > + swiotlb_adjust(); > + > /* > * Reserve memory for crash kernel after SRAT is parsed so that it > * won't consume hotpluggable memory. > diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c > index 3f248f0d0e07..c79a0d761db5 100644 > --- a/arch/x86/mm/mem_encrypt.c > +++ b/arch/x86/mm/mem_encrypt.c > @@ -490,6 +490,38 @@ static void print_mem_encrypt_feature_info(void) > } > > /* Architecture __weak replacement functions */ > +unsigned long __init arch_swiotlb_adjust(unsigned long iotlb_default_size) > +{ > + unsigned long size = 0;
unsigned long size = iotlb_default_size;
> + > + /* > + * For SEV, all DMA has to occur via shared/unencrypted pages. > + * SEV uses SWOTLB to make this happen without changing device > + * drivers. However, depending on the workload being run, the > + * default 64MB of SWIOTLB may not be enough & SWIOTLB may ^
Use words pls, not "&".
> + * run out of buffers for DMA, resulting in I/O errors and/or > + * performance degradation especially with high I/O workloads. > + * Increase the default size of SWIOTLB for SEV guests using > + * a minimum value of 128MB and a maximum value of 512MB, > + * depending on amount of provisioned guest memory. > + */ > + if (sev_active()) { > + phys_addr_t total_mem = memblock_phys_mem_size(); > + > + if (total_mem <= SZ_1G) > + size = max(iotlb_default_size, (unsigned long) SZ_128M); > + else if (total_mem <= SZ_4G) > + size = max(iotlb_default_size, (unsigned long) SZ_256M); > + else > + size = max(iotlb_default_size, (unsigned long) SZ_512M); > + > + pr_info("SWIOTLB bounce buffer size adjusted to %luMB for SEV platform",
just "... for SEV" - no need for "platform". ...
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c > index c19379fabd20..3be9a19ea0a5 100644 > --- a/kernel/dma/swiotlb.c > +++ b/kernel/dma/swiotlb.c > @@ -163,6 +163,30 @@ unsigned long swiotlb_size_or_default(void) > return size ? size : (IO_TLB_DEFAULT_SIZE); > } > > +unsigned long __init __weak arch_swiotlb_adjust(unsigned long size) > +{ > + return 0;
That, of course, needs to return size, not 0.
> +} > + > +void __init swiotlb_adjust(void) > +{ > + unsigned long size; > + > + /* > + * If swiotlb parameter has not been specified, give a chance to > + * architectures such as those supporting memory encryption to > + * adjust/expand SWIOTLB size for their use. > + */
And when you preset the function-local argument "size" with the size coming in as the size argument of arch_swiotlb_adjust()...
> + if (!io_tlb_nslabs) { > + size = arch_swiotlb_adjust(IO_TLB_DEFAULT_SIZE); > + if (size) {
... you don't have to do if (size) here either but simply use size to compute io_tlb_nslabs, I'd say. Thx.
-- Regards/Gruss, Boris.
https://people.kernel.org/tglx/notes-about-netiquette
| |