| Message ID | 20200818221126.391073-1-bauerman@linux.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | eae9eec476d13fad9af6da1f44a054ee02b7b161 |
| Headers | show |
| Series | [v3] powerpc/pseries/svm: Allocate SWIOTLB buffer anywhere in memory | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (d4ecce4dcc8f8820286cf4e0859850c555e89854) |
| snowpatch_ozlabs/build-ppc64le | warning | Upstream build failed, couldn't test patch |
| snowpatch_ozlabs/build-ppc64be | warning | Upstream build failed, couldn't test patch |
| snowpatch_ozlabs/build-ppc64e | warning | Upstream build failed, couldn't test patch |
| snowpatch_ozlabs/build-pmac32 | warning | Upstream build failed, couldn't test patch |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 72 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
On Tue, Aug 18, 2020 at 07:11:26PM -0300, Thiago Jung Bauermann wrote: > POWER secure guests (i.e., guests which use the Protection Execution > Facility) need to use SWIOTLB to be able to do I/O with the hypervisor, but > they don't need the SWIOTLB memory to be in low addresses since the > hypervisor doesn't have any addressing limitation. > > This solves a SWIOTLB initialization problem we are seeing in secure guests > with 128 GB of RAM: they are configured with 4 GB of crashkernel reserved > memory, which leaves no space for SWIOTLB in low addresses. > > To do this, we use mostly the same code as swiotlb_init(), but allocate the > buffer using memblock_alloc() instead of memblock_alloc_low(). > > Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com> Looks fine to me (except for the pointlessly long comment lines, but I've been told that's the powerpc way).
Christoph Hellwig <hch@lst.de> writes: > On Tue, Aug 18, 2020 at 07:11:26PM -0300, Thiago Jung Bauermann wrote: >> POWER secure guests (i.e., guests which use the Protection Execution >> Facility) need to use SWIOTLB to be able to do I/O with the hypervisor, but >> they don't need the SWIOTLB memory to be in low addresses since the >> hypervisor doesn't have any addressing limitation. >> >> This solves a SWIOTLB initialization problem we are seeing in secure guests >> with 128 GB of RAM: they are configured with 4 GB of crashkernel reserved >> memory, which leaves no space for SWIOTLB in low addresses. >> >> To do this, we use mostly the same code as swiotlb_init(), but allocate the >> buffer using memblock_alloc() instead of memblock_alloc_low(). >> >> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com> > > Looks fine to me (except for the pointlessly long comment lines, but I've > been told that's the powerpc way). Thanks! Do I have your Reviewed-by?
On Tue, Aug 18, 2020 at 07:11:26PM -0300, Thiago Jung Bauermann wrote: > POWER secure guests (i.e., guests which use the Protection Execution > Facility) need to use SWIOTLB to be able to do I/O with the hypervisor, but > they don't need the SWIOTLB memory to be in low addresses since the > hypervisor doesn't have any addressing limitation. > > This solves a SWIOTLB initialization problem we are seeing in secure guests > with 128 GB of RAM: they are configured with 4 GB of crashkernel reserved > memory, which leaves no space for SWIOTLB in low addresses. > > To do this, we use mostly the same code as swiotlb_init(), but allocate the > buffer using memblock_alloc() instead of memblock_alloc_low(). > > Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > --- > arch/powerpc/include/asm/svm.h | 4 ++++ > arch/powerpc/mm/mem.c | 6 +++++- > arch/powerpc/platforms/pseries/svm.c | 26 ++++++++++++++++++++++++++ > 3 files changed, 35 insertions(+), 1 deletion(-) > > Changes from v2: > - Panic if unable to allocate buffer, as suggested by Christoph. > > Changes from v1: > - Open-code swiotlb_init() in arch-specific code, as suggested by > Christoph. > > diff --git a/arch/powerpc/include/asm/svm.h b/arch/powerpc/include/asm/svm.h > index 85580b30aba4..7546402d796a 100644 > --- a/arch/powerpc/include/asm/svm.h > +++ b/arch/powerpc/include/asm/svm.h > @@ -15,6 +15,8 @@ static inline bool is_secure_guest(void) > return mfmsr() & MSR_S; > } > > +void __init svm_swiotlb_init(void); > + > void dtl_cache_ctor(void *addr); > #define get_dtl_cache_ctor() (is_secure_guest() ? dtl_cache_ctor : NULL) > > @@ -25,6 +27,8 @@ static inline bool is_secure_guest(void) > return false; > } > > +static inline void svm_swiotlb_init(void) {} > + > #define get_dtl_cache_ctor() NULL > > #endif /* CONFIG_PPC_SVM */ > diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c > index c2c11eb8dcfc..0f21bcb16405 100644 > --- a/arch/powerpc/mm/mem.c > +++ b/arch/powerpc/mm/mem.c > @@ -50,6 +50,7 @@ > #include <asm/swiotlb.h> > #include <asm/rtas.h> > #include <asm/kasan.h> > +#include <asm/svm.h> > > #include <mm/mmu_decl.h> > > @@ -290,7 +291,10 @@ void __init mem_init(void) > * back to to-down. > */ > memblock_set_bottom_up(true); > - swiotlb_init(0); > + if (is_secure_guest()) > + svm_swiotlb_init(); > + else > + swiotlb_init(0); > #endif > > high_memory = (void *) __va(max_low_pfn * PAGE_SIZE); > diff --git a/arch/powerpc/platforms/pseries/svm.c b/arch/powerpc/platforms/pseries/svm.c > index 40c0637203d5..81085eb8f225 100644 > --- a/arch/powerpc/platforms/pseries/svm.c > +++ b/arch/powerpc/platforms/pseries/svm.c > @@ -7,6 +7,7 @@ > */ > > #include <linux/mm.h> > +#include <linux/memblock.h> > #include <asm/machdep.h> > #include <asm/svm.h> > #include <asm/swiotlb.h> > @@ -34,6 +35,31 @@ static int __init init_svm(void) > } > machine_early_initcall(pseries, init_svm); > > +/* > + * Initialize SWIOTLB. Essentially the same as swiotlb_init(), except that it > + * can allocate the buffer anywhere in memory. Since the hypervisor doesn't have > + * any addressing limitation, we don't need to allocate it in low addresses. > + */ > +void __init svm_swiotlb_init(void) > +{ > + unsigned char *vstart; > + unsigned long bytes, io_tlb_nslabs; > + > + io_tlb_nslabs = (swiotlb_size_or_default() >> IO_TLB_SHIFT); > + io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE); > + > + bytes = io_tlb_nslabs << IO_TLB_SHIFT; > + > + vstart = memblock_alloc(PAGE_ALIGN(bytes), PAGE_SIZE); > + if (vstart && !swiotlb_init_with_tbl(vstart, io_tlb_nslabs, false)) > + return; > + > + if (io_tlb_start) > + memblock_free_early(io_tlb_start, > + PAGE_ALIGN(io_tlb_nslabs << IO_TLB_SHIFT)); > + panic("SVM: Cannot allocate SWIOTLB buffer"); > +} > + > int set_memory_encrypted(unsigned long addr, int numpages) > { > if (!PAGE_ALIGNED(addr))
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> writes: > On Tue, Aug 18, 2020 at 07:11:26PM -0300, Thiago Jung Bauermann wrote: >> POWER secure guests (i.e., guests which use the Protection Execution >> Facility) need to use SWIOTLB to be able to do I/O with the hypervisor, but >> they don't need the SWIOTLB memory to be in low addresses since the >> hypervisor doesn't have any addressing limitation. >> >> This solves a SWIOTLB initialization problem we are seeing in secure guests >> with 128 GB of RAM: they are configured with 4 GB of crashkernel reserved >> memory, which leaves no space for SWIOTLB in low addresses. >> >> To do this, we use mostly the same code as swiotlb_init(), but allocate the >> buffer using memblock_alloc() instead of memblock_alloc_low(). >> >> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com> > > Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Thanks!
Christoph Hellwig <hch@lst.de> writes: > On Tue, Aug 18, 2020 at 07:11:26PM -0300, Thiago Jung Bauermann wrote: >> POWER secure guests (i.e., guests which use the Protection Execution >> Facility) need to use SWIOTLB to be able to do I/O with the hypervisor, but >> they don't need the SWIOTLB memory to be in low addresses since the >> hypervisor doesn't have any addressing limitation. >> >> This solves a SWIOTLB initialization problem we are seeing in secure guests >> with 128 GB of RAM: they are configured with 4 GB of crashkernel reserved >> memory, which leaves no space for SWIOTLB in low addresses. >> >> To do this, we use mostly the same code as swiotlb_init(), but allocate the >> buffer using memblock_alloc() instead of memblock_alloc_low(). >> >> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com> > > Looks fine to me (except for the pointlessly long comment lines, but I've > been told that's the powerpc way). They're 80 columns AFAICS? cheers
On Tue, 18 Aug 2020 19:11:26 -0300, Thiago Jung Bauermann wrote: > POWER secure guests (i.e., guests which use the Protection Execution > Facility) need to use SWIOTLB to be able to do I/O with the hypervisor, but > they don't need the SWIOTLB memory to be in low addresses since the > hypervisor doesn't have any addressing limitation. > > This solves a SWIOTLB initialization problem we are seeing in secure guests > with 128 GB of RAM: they are configured with 4 GB of crashkernel reserved > memory, which leaves no space for SWIOTLB in low addresses. > > [...] Applied to powerpc/next. [1/1] powerpc/pseries/svm: Allocate SWIOTLB buffer anywhere in memory https://git.kernel.org/powerpc/c/eae9eec476d13fad9af6da1f44a054ee02b7b161 cheers
diff --git a/arch/powerpc/include/asm/svm.h b/arch/powerpc/include/asm/svm.h index 85580b30aba4..7546402d796a 100644 --- a/arch/powerpc/include/asm/svm.h +++ b/arch/powerpc/include/asm/svm.h @@ -15,6 +15,8 @@ static inline bool is_secure_guest(void) return mfmsr() & MSR_S; } +void __init svm_swiotlb_init(void); + void dtl_cache_ctor(void *addr); #define get_dtl_cache_ctor() (is_secure_guest() ? dtl_cache_ctor : NULL) @@ -25,6 +27,8 @@ static inline bool is_secure_guest(void) return false; } +static inline void svm_swiotlb_init(void) {} + #define get_dtl_cache_ctor() NULL #endif /* CONFIG_PPC_SVM */ diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c index c2c11eb8dcfc..0f21bcb16405 100644 --- a/arch/powerpc/mm/mem.c +++ b/arch/powerpc/mm/mem.c @@ -50,6 +50,7 @@ #include <asm/swiotlb.h> #include <asm/rtas.h> #include <asm/kasan.h> +#include <asm/svm.h> #include <mm/mmu_decl.h> @@ -290,7 +291,10 @@ void __init mem_init(void) * back to to-down. */ memblock_set_bottom_up(true); - swiotlb_init(0); + if (is_secure_guest()) + svm_swiotlb_init(); + else + swiotlb_init(0); #endif high_memory = (void *) __va(max_low_pfn * PAGE_SIZE); diff --git a/arch/powerpc/platforms/pseries/svm.c b/arch/powerpc/platforms/pseries/svm.c index 40c0637203d5..81085eb8f225 100644 --- a/arch/powerpc/platforms/pseries/svm.c +++ b/arch/powerpc/platforms/pseries/svm.c @@ -7,6 +7,7 @@ */ #include <linux/mm.h> +#include <linux/memblock.h> #include <asm/machdep.h> #include <asm/svm.h> #include <asm/swiotlb.h> @@ -34,6 +35,31 @@ static int __init init_svm(void) } machine_early_initcall(pseries, init_svm); +/* + * Initialize SWIOTLB. Essentially the same as swiotlb_init(), except that it + * can allocate the buffer anywhere in memory. Since the hypervisor doesn't have + * any addressing limitation, we don't need to allocate it in low addresses. + */ +void __init svm_swiotlb_init(void) +{ + unsigned char *vstart; + unsigned long bytes, io_tlb_nslabs; + + io_tlb_nslabs = (swiotlb_size_or_default() >> IO_TLB_SHIFT); + io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE); + + bytes = io_tlb_nslabs << IO_TLB_SHIFT; + + vstart = memblock_alloc(PAGE_ALIGN(bytes), PAGE_SIZE); + if (vstart && !swiotlb_init_with_tbl(vstart, io_tlb_nslabs, false)) + return; + + if (io_tlb_start) + memblock_free_early(io_tlb_start, + PAGE_ALIGN(io_tlb_nslabs << IO_TLB_SHIFT)); + panic("SVM: Cannot allocate SWIOTLB buffer"); +} + int set_memory_encrypted(unsigned long addr, int numpages) { if (!PAGE_ALIGNED(addr))
POWER secure guests (i.e., guests which use the Protection Execution Facility) need to use SWIOTLB to be able to do I/O with the hypervisor, but they don't need the SWIOTLB memory to be in low addresses since the hypervisor doesn't have any addressing limitation. This solves a SWIOTLB initialization problem we are seeing in secure guests with 128 GB of RAM: they are configured with 4 GB of crashkernel reserved memory, which leaves no space for SWIOTLB in low addresses. To do this, we use mostly the same code as swiotlb_init(), but allocate the buffer using memblock_alloc() instead of memblock_alloc_low(). Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com> --- arch/powerpc/include/asm/svm.h | 4 ++++ arch/powerpc/mm/mem.c | 6 +++++- arch/powerpc/platforms/pseries/svm.c | 26 ++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) Changes from v2: - Panic if unable to allocate buffer, as suggested by Christoph. Changes from v1: - Open-code swiotlb_init() in arch-specific code, as suggested by Christoph.