Re: [PATCH kernel v2 6/6] powerpc/powernv/ioda: Allocate indirect TCE levels on demand
On Thu, 21 Jun 2018 12:03:21 +1000
David Gibson wrote:
> On Sun, Jun 17, 2018 at 09:14:28PM +1000, Alexey Kardashevskiy wrote:
> > At the moment we allocate the entire TCE table, twice (hardware part and
> > userspace translation cache). This normally works as we normally have
> > contigous memory and the guest will map entire RAM for 64bit DMA.
> >
> > However if we have sparse RAM (one example is a memory device), then
> > we will allocate TCEs which will never be used as the guest only maps
> > actual memory for DMA. If it is a single level TCE table, there is nothing
> > we can really do but if it a multilevel table, we can skip allocating
> > TCEs we know we won't need.
> >
> > This adds ability to allocate only first level, saving memory.
> >
> > This changes iommu_table::free() to avoid allocating of an extra level;
> > iommu_table::set() will do this when needed.
> >
> > This adds @alloc parameter to iommu_table::exchange() to tell the callback
> > if it can allocate an extra level; the flag is set to "false" for
> > the realmode KVM handlers of H_PUT_TCE hcalls and the callback returns
> > H_TOO_HARD.
> >
> > This still requires the entire table to be counted in mm::locked_vm.
> >
> > To be conservative, this only does on-demand allocation when
> > the usespace cache table is requested which is the case of VFIO.
> >
> > The example math for a system replicating a powernv setup with NVLink2
> > in a guest:
> > 16GB RAM mapped at 0x0
> > 128GB GPU RAM window (16GB of actual RAM) mapped at 0x2440
> >
> > the table to cover that all with 64K pages takes:
> > (((0x2440 + 0x20) >> 16)*8)>>20 = 4556MB
> >
> > If we allocate only necessary TCE levels, we will only need:
> > (((0x4 + 0x4) >> 16)*8)>>20 = 4MB (plus some for indirect
> > levels).
> >
> > Signed-off-by: Alexey Kardashevskiy
> > ---
> > Changes:
> > v2:
> > * fixed bug in cleanup path which forced the entire table to be
> > allocated right before destroying
> > * added memory allocation error handling pnv_tce()
> > ---
> > arch/powerpc/include/asm/iommu.h | 7 ++-
> > arch/powerpc/platforms/powernv/pci.h | 6 ++-
> > arch/powerpc/kvm/book3s_64_vio_hv.c | 4 +-
> > arch/powerpc/platforms/powernv/pci-ioda-tce.c | 69
> > ---
> > arch/powerpc/platforms/powernv/pci-ioda.c | 8 ++--
> > drivers/vfio/vfio_iommu_spapr_tce.c | 2 +-
> > 6 files changed, 69 insertions(+), 27 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/iommu.h
> > b/arch/powerpc/include/asm/iommu.h
> > index 4bdcf22..daa3ee5 100644
> > --- a/arch/powerpc/include/asm/iommu.h
> > +++ b/arch/powerpc/include/asm/iommu.h
> > @@ -70,7 +70,7 @@ struct iommu_table_ops {
> > unsigned long *hpa,
> > enum dma_data_direction *direction);
> >
> > - __be64 *(*useraddrptr)(struct iommu_table *tbl, long index);
> > + __be64 *(*useraddrptr)(struct iommu_table *tbl, long index, bool alloc);
> > #endif
> > void (*clear)(struct iommu_table *tbl,
> > long index, long npages);
> > @@ -122,10 +122,13 @@ struct iommu_table {
> > __be64 *it_userspace; /* userspace view of the table */
> > struct iommu_table_ops *it_ops;
> > struct krefit_kref;
> > + int it_nid;
> > };
> >
> > +#define IOMMU_TABLE_USERSPACE_ENTRY_RM(tbl, entry) \
> > + ((tbl)->it_ops->useraddrptr((tbl), (entry), false))
> > #define IOMMU_TABLE_USERSPACE_ENTRY(tbl, entry) \
> > - ((tbl)->it_ops->useraddrptr((tbl), (entry)))
> > + ((tbl)->it_ops->useraddrptr((tbl), (entry), true))
> >
> > /* Pure 2^n version of get_order */
> > static inline __attribute_const__
> > diff --git a/arch/powerpc/platforms/powernv/pci.h
> > b/arch/powerpc/platforms/powernv/pci.h
> > index 5e02408..1fa5590 100644
> > --- a/arch/powerpc/platforms/powernv/pci.h
> > +++ b/arch/powerpc/platforms/powernv/pci.h
> > @@ -267,8 +267,10 @@ extern int pnv_tce_build(struct iommu_table *tbl, long
> > index, long npages,
> > unsigned long attrs);
> > extern void pnv_tce_free(struct iommu_table *tbl, long index, long npages);
> > extern int pnv_tce_xchg(struct iommu_table *tbl, long index,
> > - unsigned long *hpa, enum dma_data_direction *direction);
> > -extern __be64 *pnv_tce_useraddrptr(struct iommu_table *tbl, long index);
> > + unsigned long *hpa, enum dma_data_direction *direction,
> > + bool alloc);
> > +extern __be64 *pnv_tce_useraddrptr(struct iommu_table *tbl, long index,
> > + bool alloc);
> > extern unsigned long pnv_tce_get(struct iommu_table *tbl, long index);
> >
> > extern long pnv_pci_ioda2_table_alloc_pages(int nid, __u64 bus_offset,
> > diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c
> > b/arch/powerpc/kvm/book3s_64_vio_hv.c
> > index db0490c..05b4865 100644
> > --- a/arch/powerpc/kvm/book3s_64_vio_hv.c
> > +++
Re: [PATCH kernel v2 6/6] powerpc/powernv/ioda: Allocate indirect TCE levels on demand
On Sun, Jun 17, 2018 at 09:14:28PM +1000, Alexey Kardashevskiy wrote:
> At the moment we allocate the entire TCE table, twice (hardware part and
> userspace translation cache). This normally works as we normally have
> contigous memory and the guest will map entire RAM for 64bit DMA.
>
> However if we have sparse RAM (one example is a memory device), then
> we will allocate TCEs which will never be used as the guest only maps
> actual memory for DMA. If it is a single level TCE table, there is nothing
> we can really do but if it a multilevel table, we can skip allocating
> TCEs we know we won't need.
>
> This adds ability to allocate only first level, saving memory.
>
> This changes iommu_table::free() to avoid allocating of an extra level;
> iommu_table::set() will do this when needed.
>
> This adds @alloc parameter to iommu_table::exchange() to tell the callback
> if it can allocate an extra level; the flag is set to "false" for
> the realmode KVM handlers of H_PUT_TCE hcalls and the callback returns
> H_TOO_HARD.
>
> This still requires the entire table to be counted in mm::locked_vm.
>
> To be conservative, this only does on-demand allocation when
> the usespace cache table is requested which is the case of VFIO.
>
> The example math for a system replicating a powernv setup with NVLink2
> in a guest:
> 16GB RAM mapped at 0x0
> 128GB GPU RAM window (16GB of actual RAM) mapped at 0x2440
>
> the table to cover that all with 64K pages takes:
> (((0x2440 + 0x20) >> 16)*8)>>20 = 4556MB
>
> If we allocate only necessary TCE levels, we will only need:
> (((0x4 + 0x4) >> 16)*8)>>20 = 4MB (plus some for indirect
> levels).
>
> Signed-off-by: Alexey Kardashevskiy
> ---
> Changes:
> v2:
> * fixed bug in cleanup path which forced the entire table to be
> allocated right before destroying
> * added memory allocation error handling pnv_tce()
> ---
> arch/powerpc/include/asm/iommu.h | 7 ++-
> arch/powerpc/platforms/powernv/pci.h | 6 ++-
> arch/powerpc/kvm/book3s_64_vio_hv.c | 4 +-
> arch/powerpc/platforms/powernv/pci-ioda-tce.c | 69
> ---
> arch/powerpc/platforms/powernv/pci-ioda.c | 8 ++--
> drivers/vfio/vfio_iommu_spapr_tce.c | 2 +-
> 6 files changed, 69 insertions(+), 27 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/iommu.h
> b/arch/powerpc/include/asm/iommu.h
> index 4bdcf22..daa3ee5 100644
> --- a/arch/powerpc/include/asm/iommu.h
> +++ b/arch/powerpc/include/asm/iommu.h
> @@ -70,7 +70,7 @@ struct iommu_table_ops {
> unsigned long *hpa,
> enum dma_data_direction *direction);
>
> - __be64 *(*useraddrptr)(struct iommu_table *tbl, long index);
> + __be64 *(*useraddrptr)(struct iommu_table *tbl, long index, bool alloc);
> #endif
> void (*clear)(struct iommu_table *tbl,
> long index, long npages);
> @@ -122,10 +122,13 @@ struct iommu_table {
> __be64 *it_userspace; /* userspace view of the table */
> struct iommu_table_ops *it_ops;
> struct krefit_kref;
> + int it_nid;
> };
>
> +#define IOMMU_TABLE_USERSPACE_ENTRY_RM(tbl, entry) \
> + ((tbl)->it_ops->useraddrptr((tbl), (entry), false))
> #define IOMMU_TABLE_USERSPACE_ENTRY(tbl, entry) \
> - ((tbl)->it_ops->useraddrptr((tbl), (entry)))
> + ((tbl)->it_ops->useraddrptr((tbl), (entry), true))
>
> /* Pure 2^n version of get_order */
> static inline __attribute_const__
> diff --git a/arch/powerpc/platforms/powernv/pci.h
> b/arch/powerpc/platforms/powernv/pci.h
> index 5e02408..1fa5590 100644
> --- a/arch/powerpc/platforms/powernv/pci.h
> +++ b/arch/powerpc/platforms/powernv/pci.h
> @@ -267,8 +267,10 @@ extern int pnv_tce_build(struct iommu_table *tbl, long
> index, long npages,
> unsigned long attrs);
> extern void pnv_tce_free(struct iommu_table *tbl, long index, long npages);
> extern int pnv_tce_xchg(struct iommu_table *tbl, long index,
> - unsigned long *hpa, enum dma_data_direction *direction);
> -extern __be64 *pnv_tce_useraddrptr(struct iommu_table *tbl, long index);
> + unsigned long *hpa, enum dma_data_direction *direction,
> + bool alloc);
> +extern __be64 *pnv_tce_useraddrptr(struct iommu_table *tbl, long index,
> + bool alloc);
> extern unsigned long pnv_tce_get(struct iommu_table *tbl, long index);
>
> extern long pnv_pci_ioda2_table_alloc_pages(int nid, __u64 bus_offset,
> diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c
> b/arch/powerpc/kvm/book3s_64_vio_hv.c
> index db0490c..05b4865 100644
> --- a/arch/powerpc/kvm/book3s_64_vio_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_vio_hv.c
> @@ -200,7 +200,7 @@ static long kvmppc_rm_tce_iommu_mapped_dec(struct kvm
> *kvm,
> {
> struct mm_iommu_table_group_mem_t *mem = NULL;
> const unsigned long pgsize = 1ULL << tbl->it_page_shift;
> -
[PATCH kernel v2 6/6] powerpc/powernv/ioda: Allocate indirect TCE levels on demand
At the moment we allocate the entire TCE table, twice (hardware part and
userspace translation cache). This normally works as we normally have
contigous memory and the guest will map entire RAM for 64bit DMA.
However if we have sparse RAM (one example is a memory device), then
we will allocate TCEs which will never be used as the guest only maps
actual memory for DMA. If it is a single level TCE table, there is nothing
we can really do but if it a multilevel table, we can skip allocating
TCEs we know we won't need.
This adds ability to allocate only first level, saving memory.
This changes iommu_table::free() to avoid allocating of an extra level;
iommu_table::set() will do this when needed.
This adds @alloc parameter to iommu_table::exchange() to tell the callback
if it can allocate an extra level; the flag is set to "false" for
the realmode KVM handlers of H_PUT_TCE hcalls and the callback returns
H_TOO_HARD.
This still requires the entire table to be counted in mm::locked_vm.
To be conservative, this only does on-demand allocation when
the usespace cache table is requested which is the case of VFIO.
The example math for a system replicating a powernv setup with NVLink2
in a guest:
16GB RAM mapped at 0x0
128GB GPU RAM window (16GB of actual RAM) mapped at 0x2440
the table to cover that all with 64K pages takes:
(((0x2440 + 0x20) >> 16)*8)>>20 = 4556MB
If we allocate only necessary TCE levels, we will only need:
(((0x4 + 0x4) >> 16)*8)>>20 = 4MB (plus some for indirect
levels).
Signed-off-by: Alexey Kardashevskiy
---
Changes:
v2:
* fixed bug in cleanup path which forced the entire table to be
allocated right before destroying
* added memory allocation error handling pnv_tce()
---
arch/powerpc/include/asm/iommu.h | 7 ++-
arch/powerpc/platforms/powernv/pci.h | 6 ++-
arch/powerpc/kvm/book3s_64_vio_hv.c | 4 +-
arch/powerpc/platforms/powernv/pci-ioda-tce.c | 69 ---
arch/powerpc/platforms/powernv/pci-ioda.c | 8 ++--
drivers/vfio/vfio_iommu_spapr_tce.c | 2 +-
6 files changed, 69 insertions(+), 27 deletions(-)
diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index 4bdcf22..daa3ee5 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -70,7 +70,7 @@ struct iommu_table_ops {
unsigned long *hpa,
enum dma_data_direction *direction);
- __be64 *(*useraddrptr)(struct iommu_table *tbl, long index);
+ __be64 *(*useraddrptr)(struct iommu_table *tbl, long index, bool alloc);
#endif
void (*clear)(struct iommu_table *tbl,
long index, long npages);
@@ -122,10 +122,13 @@ struct iommu_table {
__be64 *it_userspace; /* userspace view of the table */
struct iommu_table_ops *it_ops;
struct krefit_kref;
+ int it_nid;
};
+#define IOMMU_TABLE_USERSPACE_ENTRY_RM(tbl, entry) \
+ ((tbl)->it_ops->useraddrptr((tbl), (entry), false))
#define IOMMU_TABLE_USERSPACE_ENTRY(tbl, entry) \
- ((tbl)->it_ops->useraddrptr((tbl), (entry)))
+ ((tbl)->it_ops->useraddrptr((tbl), (entry), true))
/* Pure 2^n version of get_order */
static inline __attribute_const__
diff --git a/arch/powerpc/platforms/powernv/pci.h
b/arch/powerpc/platforms/powernv/pci.h
index 5e02408..1fa5590 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -267,8 +267,10 @@ extern int pnv_tce_build(struct iommu_table *tbl, long
index, long npages,
unsigned long attrs);
extern void pnv_tce_free(struct iommu_table *tbl, long index, long npages);
extern int pnv_tce_xchg(struct iommu_table *tbl, long index,
- unsigned long *hpa, enum dma_data_direction *direction);
-extern __be64 *pnv_tce_useraddrptr(struct iommu_table *tbl, long index);
+ unsigned long *hpa, enum dma_data_direction *direction,
+ bool alloc);
+extern __be64 *pnv_tce_useraddrptr(struct iommu_table *tbl, long index,
+ bool alloc);
extern unsigned long pnv_tce_get(struct iommu_table *tbl, long index);
extern long pnv_pci_ioda2_table_alloc_pages(int nid, __u64 bus_offset,
diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c
b/arch/powerpc/kvm/book3s_64_vio_hv.c
index db0490c..05b4865 100644
--- a/arch/powerpc/kvm/book3s_64_vio_hv.c
+++ b/arch/powerpc/kvm/book3s_64_vio_hv.c
@@ -200,7 +200,7 @@ static long kvmppc_rm_tce_iommu_mapped_dec(struct kvm *kvm,
{
struct mm_iommu_table_group_mem_t *mem = NULL;
const unsigned long pgsize = 1ULL << tbl->it_page_shift;
- __be64 *pua = IOMMU_TABLE_USERSPACE_ENTRY(tbl, entry);
+ __be64 *pua = IOMMU_TABLE_USERSPACE_ENTRY_RM(tbl, entry);
if (!pua)
/* it_userspace allocation might be delayed */
@@ -264,7 +264,7 @@ static long
