* [PATCH kernel 0/3] Enable IOMMU support for pseries Secure VMs @ 2019-12-13 8:45 Alexey Kardashevskiy 2019-12-13 8:45 ` [PATCH kernel 1/3] powerpc/pseries/iommu: Use dma_iommu_ops for Secure VM Alexey Kardashevskiy ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Alexey Kardashevskiy @ 2019-12-13 8:45 UTC (permalink / raw) To: linuxppc-dev Cc: Alexey Kardashevskiy, Ram Pai, kvm-ppc, Paul Mackerras, Thiago Jung Bauermann, David Gibson This enables IOMMU for SVM. Instead of sharing a H_PUT_TCE_INDIRECT page, this uses H_PUT_TCE for mapping and H_STUFF_TCE for clearing TCEs which should bring acceptable performance at the boot time; otherwise things work with the same speed anyway. Please comment. Thanks. Alexey Kardashevskiy (2): powerpc/pseries: Allow not having ibm,hypertas-functions::hcall-multi-tce for DDW powerpc/pseries/iommu: Do not use H_PUT_TCE_INDIRECT in secure VM Ram Pai (1): powerpc/pseries/iommu: Use dma_iommu_ops for Secure VM. arch/powerpc/platforms/pseries/iommu.c | 59 +++++++++++++++----------- 1 file changed, 34 insertions(+), 25 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH kernel 1/3] powerpc/pseries/iommu: Use dma_iommu_ops for Secure VM. 2019-12-13 8:45 [PATCH kernel 0/3] Enable IOMMU support for pseries Secure VMs Alexey Kardashevskiy @ 2019-12-13 8:45 ` Alexey Kardashevskiy 2019-12-13 10:20 ` Michael Ellerman 2019-12-13 8:45 ` [PATCH kernel 2/3] powerpc/pseries: Allow not having ibm, hypertas-functions::hcall-multi-tce for DDW Alexey Kardashevskiy 2019-12-13 8:45 ` [PATCH kernel 3/3] powerpc/pseries/iommu: Do not use H_PUT_TCE_INDIRECT in secure VM Alexey Kardashevskiy 2 siblings, 1 reply; 7+ messages in thread From: Alexey Kardashevskiy @ 2019-12-13 8:45 UTC (permalink / raw) To: linuxppc-dev Cc: Alexey Kardashevskiy, Ram Pai, kvm-ppc, Thiago Jung Bauermann, David Gibson From: Ram Pai <linuxram@us.ibm.com> Commit edea902c1c1e ("powerpc/pseries/iommu: Don't use dma_iommu_ops on secure guests") disabled dma_iommu_ops path, for secure VMs. Disabling dma_iommu_ops path for secure VMs, helped enable dma_direct path. This enabled support for bounce-buffering through SWIOTLB. However it fails to operate when IOMMU is enabled, since I/O pages are not TCE mapped. Reenable dma_iommu_ops path for pseries Secure VMs. It handles all cases including, TCE mapping I/O pages, in the presence of a IOMMU. Signed-off-by: Ram Pai <linuxram@us.ibm.com> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- arch/powerpc/platforms/pseries/iommu.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c index 6ba081dd61c9..df7db33ca93b 100644 --- a/arch/powerpc/platforms/pseries/iommu.c +++ b/arch/powerpc/platforms/pseries/iommu.c @@ -36,7 +36,6 @@ #include <asm/udbg.h> #include <asm/mmzone.h> #include <asm/plpar_wrappers.h> -#include <asm/svm.h> #include "pseries.h" @@ -1320,15 +1319,7 @@ void iommu_init_early_pSeries(void) of_reconfig_notifier_register(&iommu_reconfig_nb); register_memory_notifier(&iommu_mem_nb); - /* - * Secure guest memory is inacessible to devices so regular DMA isn't - * possible. - * - * In that case keep devices' dma_map_ops as NULL so that the generic - * DMA code path will use SWIOTLB to bounce buffers for DMA. - */ - if (!is_secure_guest()) - set_pci_dma_ops(&dma_iommu_ops); + set_pci_dma_ops(&dma_iommu_ops); } static int __init disable_multitce(char *str) -- 2.17.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH kernel 1/3] powerpc/pseries/iommu: Use dma_iommu_ops for Secure VM. 2019-12-13 8:45 ` [PATCH kernel 1/3] powerpc/pseries/iommu: Use dma_iommu_ops for Secure VM Alexey Kardashevskiy @ 2019-12-13 10:20 ` Michael Ellerman 0 siblings, 0 replies; 7+ messages in thread From: Michael Ellerman @ 2019-12-13 10:20 UTC (permalink / raw) To: Alexey Kardashevskiy, linuxppc-dev Cc: Alexey Kardashevskiy, Ram Pai, Thiago Jung Bauermann, kvm-ppc, David Gibson Alexey Kardashevskiy <aik@ozlabs.ru> writes: > From: Ram Pai <linuxram@us.ibm.com> > > Commit edea902c1c1e ("powerpc/pseries/iommu: Don't use dma_iommu_ops on > secure guests") > disabled dma_iommu_ops path, for secure VMs. Disabling dma_iommu_ops > path for secure VMs, helped enable dma_direct path. This enabled > support for bounce-buffering through SWIOTLB. However it fails to > operate when IOMMU is enabled, since I/O pages are not TCE mapped. > > Reenable dma_iommu_ops path for pseries Secure VMs. It handles all > cases including, TCE mapping I/O pages, in the presence of a > IOMMU. > > Signed-off-by: Ram Pai <linuxram@us.ibm.com> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> This seems like it should have a Fixes tag? cheers > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c > index 6ba081dd61c9..df7db33ca93b 100644 > --- a/arch/powerpc/platforms/pseries/iommu.c > +++ b/arch/powerpc/platforms/pseries/iommu.c > @@ -36,7 +36,6 @@ > #include <asm/udbg.h> > #include <asm/mmzone.h> > #include <asm/plpar_wrappers.h> > -#include <asm/svm.h> > > #include "pseries.h" > > @@ -1320,15 +1319,7 @@ void iommu_init_early_pSeries(void) > of_reconfig_notifier_register(&iommu_reconfig_nb); > register_memory_notifier(&iommu_mem_nb); > > - /* > - * Secure guest memory is inacessible to devices so regular DMA isn't > - * possible. > - * > - * In that case keep devices' dma_map_ops as NULL so that the generic > - * DMA code path will use SWIOTLB to bounce buffers for DMA. > - */ > - if (!is_secure_guest()) > - set_pci_dma_ops(&dma_iommu_ops); > + set_pci_dma_ops(&dma_iommu_ops); > } > > static int __init disable_multitce(char *str) > -- > 2.17.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH kernel 2/3] powerpc/pseries: Allow not having ibm, hypertas-functions::hcall-multi-tce for DDW 2019-12-13 8:45 [PATCH kernel 0/3] Enable IOMMU support for pseries Secure VMs Alexey Kardashevskiy 2019-12-13 8:45 ` [PATCH kernel 1/3] powerpc/pseries/iommu: Use dma_iommu_ops for Secure VM Alexey Kardashevskiy @ 2019-12-13 8:45 ` Alexey Kardashevskiy 2019-12-13 20:32 ` [PATCH kernel 2/3] powerpc/pseries: Allow not having ibm,hypertas-functions::hcall-multi-tce " Ram Pai 2019-12-13 8:45 ` [PATCH kernel 3/3] powerpc/pseries/iommu: Do not use H_PUT_TCE_INDIRECT in secure VM Alexey Kardashevskiy 2 siblings, 1 reply; 7+ messages in thread From: Alexey Kardashevskiy @ 2019-12-13 8:45 UTC (permalink / raw) To: linuxppc-dev Cc: Alexey Kardashevskiy, Ram Pai, kvm-ppc, Thiago Jung Bauermann, David Gibson By default a pseries guest supports a H_PUT_TCE hypercall which maps a single IOMMU page in a DMA window. Additionally the hypervisor may support H_PUT_TCE_INDIRECT/H_STUFF_TCE which update multiple TCEs at once; this is advertised via the device tree /rtas/ibm,hypertas-functions property which Linux converts to FW_FEATURE_MULTITCE. FW_FEATURE_MULTITCE is checked when dma_iommu_ops is used; however the code managing the huge DMA window (DDW) ignores it and calls H_PUT_TCE_INDIRECT even if it is explicitly disabled via the "multitce=off" kernel command line parameter. This adds FW_FEATURE_MULTITCE checking to the DDW code path. This changes tce_build_pSeriesLP to take liobn and page size as the huge window does not have iommu_table descriptor which usually the place to store these numbers. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- The idea is then set FW_FEATURE_MULTITCE in init_svm() and have the guest use H_PUT_TCE without sharing a (temporary) page for H_PUT_TCE_INDIRECT. --- arch/powerpc/platforms/pseries/iommu.c | 44 ++++++++++++++++++-------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c index df7db33ca93b..f6e9b87c82fc 100644 --- a/arch/powerpc/platforms/pseries/iommu.c +++ b/arch/powerpc/platforms/pseries/iommu.c @@ -132,10 +132,10 @@ static unsigned long tce_get_pseries(struct iommu_table *tbl, long index) return be64_to_cpu(*tcep); } -static void tce_free_pSeriesLP(struct iommu_table*, long, long); +static void tce_free_pSeriesLP(unsigned long liobn, long, long); static void tce_freemulti_pSeriesLP(struct iommu_table*, long, long); -static int tce_build_pSeriesLP(struct iommu_table *tbl, long tcenum, +static int tce_build_pSeriesLP(unsigned long liobn, long tcenum, long tceshift, long npages, unsigned long uaddr, enum dma_data_direction direction, unsigned long attrs) @@ -146,25 +146,25 @@ static int tce_build_pSeriesLP(struct iommu_table *tbl, long tcenum, int ret = 0; long tcenum_start = tcenum, npages_start = npages; - rpn = __pa(uaddr) >> TCE_SHIFT; + rpn = __pa(uaddr) >> tceshift; proto_tce = TCE_PCI_READ; if (direction != DMA_TO_DEVICE) proto_tce |= TCE_PCI_WRITE; while (npages--) { - tce = proto_tce | (rpn & TCE_RPN_MASK) << TCE_RPN_SHIFT; - rc = plpar_tce_put((u64)tbl->it_index, (u64)tcenum << 12, tce); + tce = proto_tce | (rpn & TCE_RPN_MASK) << tceshift; + rc = plpar_tce_put((u64)liobn, (u64)tcenum << tceshift, tce); if (unlikely(rc == H_NOT_ENOUGH_RESOURCES)) { ret = (int)rc; - tce_free_pSeriesLP(tbl, tcenum_start, + tce_free_pSeriesLP(liobn, tcenum_start, (npages_start - (npages + 1))); break; } if (rc && printk_ratelimit()) { printk("tce_build_pSeriesLP: plpar_tce_put failed. rc=%lld\n", rc); - printk("\tindex = 0x%llx\n", (u64)tbl->it_index); + printk("\tindex = 0x%llx\n", (u64)liobn); printk("\ttcenum = 0x%llx\n", (u64)tcenum); printk("\ttce val = 0x%llx\n", tce ); dump_stack(); @@ -193,7 +193,8 @@ static int tce_buildmulti_pSeriesLP(struct iommu_table *tbl, long tcenum, unsigned long flags; if ((npages == 1) || !firmware_has_feature(FW_FEATURE_MULTITCE)) { - return tce_build_pSeriesLP(tbl, tcenum, npages, uaddr, + return tce_build_pSeriesLP(tbl->it_index, tcenum, + tbl->it_page_shift, npages, uaddr, direction, attrs); } @@ -209,8 +210,9 @@ static int tce_buildmulti_pSeriesLP(struct iommu_table *tbl, long tcenum, /* If allocation fails, fall back to the loop implementation */ if (!tcep) { local_irq_restore(flags); - return tce_build_pSeriesLP(tbl, tcenum, npages, uaddr, - direction, attrs); + return tce_build_pSeriesLP(tbl->it_index, tcenum, + tbl->it_page_shift, + npages, uaddr, direction, attrs); } __this_cpu_write(tce_page, tcep); } @@ -261,16 +263,16 @@ static int tce_buildmulti_pSeriesLP(struct iommu_table *tbl, long tcenum, return ret; } -static void tce_free_pSeriesLP(struct iommu_table *tbl, long tcenum, long npages) +static void tce_free_pSeriesLP(unsigned long liobn, long tcenum, long npages) { u64 rc; while (npages--) { - rc = plpar_tce_put((u64)tbl->it_index, (u64)tcenum << 12, 0); + rc = plpar_tce_put((u64)liobn, (u64)tcenum << 12, 0); if (rc && printk_ratelimit()) { printk("tce_free_pSeriesLP: plpar_tce_put failed. rc=%lld\n", rc); - printk("\tindex = 0x%llx\n", (u64)tbl->it_index); + printk("\tindex = 0x%llx\n", (u64)liobn); printk("\ttcenum = 0x%llx\n", (u64)tcenum); dump_stack(); } @@ -285,7 +287,7 @@ static void tce_freemulti_pSeriesLP(struct iommu_table *tbl, long tcenum, long n u64 rc; if (!firmware_has_feature(FW_FEATURE_MULTITCE)) - return tce_free_pSeriesLP(tbl, tcenum, npages); + return tce_free_pSeriesLP(tbl->it_index, tcenum, npages); rc = plpar_tce_stuff((u64)tbl->it_index, (u64)tcenum << 12, 0, npages); @@ -400,6 +402,20 @@ static int tce_setrange_multi_pSeriesLP(unsigned long start_pfn, u64 rc = 0; long l, limit; + if (!firmware_has_feature(FW_FEATURE_MULTITCE)) { + unsigned long tceshift = be32_to_cpu(maprange->tce_shift); + unsigned long dmastart = (start_pfn << PAGE_SHIFT) + + be64_to_cpu(maprange->dma_base); + unsigned long tcenum = dmastart >> tceshift; + unsigned long npages = num_pfn << PAGE_SHIFT >> + be32_to_cpu(maprange->tce_shift); + void *uaddr = __va(start_pfn << PAGE_SHIFT); + + return tce_build_pSeriesLP(be32_to_cpu(maprange->liobn), + tcenum, tceshift, npages, (unsigned long) uaddr, + DMA_BIDIRECTIONAL, 0); + } + local_irq_disable(); /* to protect tcep and the page behind it */ tcep = __this_cpu_read(tce_page); -- 2.17.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH kernel 2/3] powerpc/pseries: Allow not having ibm,hypertas-functions::hcall-multi-tce for DDW 2019-12-13 8:45 ` [PATCH kernel 2/3] powerpc/pseries: Allow not having ibm, hypertas-functions::hcall-multi-tce for DDW Alexey Kardashevskiy @ 2019-12-13 20:32 ` Ram Pai 0 siblings, 0 replies; 7+ messages in thread From: Ram Pai @ 2019-12-13 20:32 UTC (permalink / raw) To: Alexey Kardashevskiy Cc: linuxppc-dev, Thiago Jung Bauermann, kvm-ppc, David Gibson On Fri, Dec 13, 2019 at 07:45:36PM +1100, Alexey Kardashevskiy wrote: > By default a pseries guest supports a H_PUT_TCE hypercall which maps > a single IOMMU page in a DMA window. Additionally the hypervisor may > support H_PUT_TCE_INDIRECT/H_STUFF_TCE which update multiple TCEs at once; > this is advertised via the device tree /rtas/ibm,hypertas-functions > property which Linux converts to FW_FEATURE_MULTITCE. Thanks Alexey for the patches! > > FW_FEATURE_MULTITCE is checked when dma_iommu_ops is used; however > the code managing the huge DMA window (DDW) ignores it and calls > H_PUT_TCE_INDIRECT even if it is explicitly disabled via > the "multitce=off" kernel command line parameter. Also H_PUT_TCE_INDIRECT should not be called in secure VM, even when "multitce=on". right? Or does it get disabled somehow in the secure guest? > > This adds FW_FEATURE_MULTITCE checking to the DDW code path. > > This changes tce_build_pSeriesLP to take liobn and page size as > the huge window does not have iommu_table descriptor which usually > the place to store these numbers. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > > The idea is then set FW_FEATURE_MULTITCE in init_svm() and have the guest I think, you mean unset FW_FEATURE_MULTITCE. > use H_PUT_TCE without sharing a (temporary) page for H_PUT_TCE_INDIRECT. > --- > arch/powerpc/platforms/pseries/iommu.c | 44 ++++++++++++++++++-------- > 1 file changed, 30 insertions(+), 14 deletions(-) > > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c > index df7db33ca93b..f6e9b87c82fc 100644 > --- a/arch/powerpc/platforms/pseries/iommu.c > +++ b/arch/powerpc/platforms/pseries/iommu.c > @@ -132,10 +132,10 @@ static unsigned long tce_get_pseries(struct iommu_table *tbl, long index) > return be64_to_cpu(*tcep); > } > > -static void tce_free_pSeriesLP(struct iommu_table*, long, long); > +static void tce_free_pSeriesLP(unsigned long liobn, long, long); > static void tce_freemulti_pSeriesLP(struct iommu_table*, long, long); > > -static int tce_build_pSeriesLP(struct iommu_table *tbl, long tcenum, > +static int tce_build_pSeriesLP(unsigned long liobn, long tcenum, long tceshift, > long npages, unsigned long uaddr, > enum dma_data_direction direction, > unsigned long attrs) > @@ -146,25 +146,25 @@ static int tce_build_pSeriesLP(struct iommu_table *tbl, long tcenum, > int ret = 0; > long tcenum_start = tcenum, npages_start = npages; > > - rpn = __pa(uaddr) >> TCE_SHIFT; > + rpn = __pa(uaddr) >> tceshift; > proto_tce = TCE_PCI_READ; > if (direction != DMA_TO_DEVICE) > proto_tce |= TCE_PCI_WRITE; > > while (npages--) { > - tce = proto_tce | (rpn & TCE_RPN_MASK) << TCE_RPN_SHIFT; > - rc = plpar_tce_put((u64)tbl->it_index, (u64)tcenum << 12, tce); > + tce = proto_tce | (rpn & TCE_RPN_MASK) << tceshift; > + rc = plpar_tce_put((u64)liobn, (u64)tcenum << tceshift, tce); > > if (unlikely(rc == H_NOT_ENOUGH_RESOURCES)) { > ret = (int)rc; > - tce_free_pSeriesLP(tbl, tcenum_start, > + tce_free_pSeriesLP(liobn, tcenum_start, > (npages_start - (npages + 1))); > break; > } > > if (rc && printk_ratelimit()) { > printk("tce_build_pSeriesLP: plpar_tce_put failed. rc=%lld\n", rc); > - printk("\tindex = 0x%llx\n", (u64)tbl->it_index); > + printk("\tindex = 0x%llx\n", (u64)liobn); > printk("\ttcenum = 0x%llx\n", (u64)tcenum); > printk("\ttce val = 0x%llx\n", tce ); > dump_stack(); > @@ -193,7 +193,8 @@ static int tce_buildmulti_pSeriesLP(struct iommu_table *tbl, long tcenum, > unsigned long flags; > > if ((npages == 1) || !firmware_has_feature(FW_FEATURE_MULTITCE)) { > - return tce_build_pSeriesLP(tbl, tcenum, npages, uaddr, > + return tce_build_pSeriesLP(tbl->it_index, tcenum, > + tbl->it_page_shift, npages, uaddr, > direction, attrs); > } > > @@ -209,8 +210,9 @@ static int tce_buildmulti_pSeriesLP(struct iommu_table *tbl, long tcenum, > /* If allocation fails, fall back to the loop implementation */ > if (!tcep) { > local_irq_restore(flags); > - return tce_build_pSeriesLP(tbl, tcenum, npages, uaddr, > - direction, attrs); > + return tce_build_pSeriesLP(tbl->it_index, tcenum, > + tbl->it_page_shift, > + npages, uaddr, direction, attrs); > } > __this_cpu_write(tce_page, tcep); > } > @@ -261,16 +263,16 @@ static int tce_buildmulti_pSeriesLP(struct iommu_table *tbl, long tcenum, > return ret; > } > > -static void tce_free_pSeriesLP(struct iommu_table *tbl, long tcenum, long npages) > +static void tce_free_pSeriesLP(unsigned long liobn, long tcenum, long npages) > { > u64 rc; > > while (npages--) { > - rc = plpar_tce_put((u64)tbl->it_index, (u64)tcenum << 12, 0); > + rc = plpar_tce_put((u64)liobn, (u64)tcenum << 12, 0); > > if (rc && printk_ratelimit()) { > printk("tce_free_pSeriesLP: plpar_tce_put failed. rc=%lld\n", rc); > - printk("\tindex = 0x%llx\n", (u64)tbl->it_index); > + printk("\tindex = 0x%llx\n", (u64)liobn); > printk("\ttcenum = 0x%llx\n", (u64)tcenum); > dump_stack(); > } > @@ -285,7 +287,7 @@ static void tce_freemulti_pSeriesLP(struct iommu_table *tbl, long tcenum, long n > u64 rc; > > if (!firmware_has_feature(FW_FEATURE_MULTITCE)) > - return tce_free_pSeriesLP(tbl, tcenum, npages); > + return tce_free_pSeriesLP(tbl->it_index, tcenum, npages); > > rc = plpar_tce_stuff((u64)tbl->it_index, (u64)tcenum << 12, 0, npages); > > @@ -400,6 +402,20 @@ static int tce_setrange_multi_pSeriesLP(unsigned long start_pfn, > u64 rc = 0; > long l, limit; > > + if (!firmware_has_feature(FW_FEATURE_MULTITCE)) { this code should execute in secure guest. Which means we need a ' || is_secure_guest() ' check. > + unsigned long tceshift = be32_to_cpu(maprange->tce_shift); > + unsigned long dmastart = (start_pfn << PAGE_SHIFT) + > + be64_to_cpu(maprange->dma_base); > + unsigned long tcenum = dmastart >> tceshift; > + unsigned long npages = num_pfn << PAGE_SHIFT >> > + be32_to_cpu(maprange->tce_shift); > + void *uaddr = __va(start_pfn << PAGE_SHIFT); > + > + return tce_build_pSeriesLP(be32_to_cpu(maprange->liobn), > + tcenum, tceshift, npages, (unsigned long) uaddr, > + DMA_BIDIRECTIONAL, 0); > + } > + > local_irq_disable(); /* to protect tcep and the page behind it */ > tcep = __this_cpu_read(tce_page); RP ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH kernel 3/3] powerpc/pseries/iommu: Do not use H_PUT_TCE_INDIRECT in secure VM 2019-12-13 8:45 [PATCH kernel 0/3] Enable IOMMU support for pseries Secure VMs Alexey Kardashevskiy 2019-12-13 8:45 ` [PATCH kernel 1/3] powerpc/pseries/iommu: Use dma_iommu_ops for Secure VM Alexey Kardashevskiy 2019-12-13 8:45 ` [PATCH kernel 2/3] powerpc/pseries: Allow not having ibm, hypertas-functions::hcall-multi-tce for DDW Alexey Kardashevskiy @ 2019-12-13 8:45 ` Alexey Kardashevskiy 2019-12-13 10:20 ` Michael Ellerman 2 siblings, 1 reply; 7+ messages in thread From: Alexey Kardashevskiy @ 2019-12-13 8:45 UTC (permalink / raw) To: linuxppc-dev Cc: Alexey Kardashevskiy, Ram Pai, kvm-ppc, Thiago Jung Bauermann, David Gibson H_PUT_TCE_INDIRECT uses a 4K page with TCEs to allow handling up to 512 TCEs per hypercall. While it is a decent optimization, we rather share less of secure VM memory so let's avoid sharing. This only allows H_PUT_TCE_INDIRECT for normal (not secure) VMs. This keeps using H_STUFF_TCE as it does not require sharing. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- Possible alternatives are: 1. define FW_FEATURE_STUFFTCE (to allow H_STUFF_TCE) in addition to FW_FEATURE_MULTITCE (make it only enable H_PUT_TCE_INDIRECT) and enable only FW_FEATURE_STUFFTCE for SVM; pro = no SVM mention in iommu.c, con = a FW feature bit with very limited use 2. disable FW_FEATURE_MULTITCE and loose H_STUFF_TCE which adds a delay in booting process --- arch/powerpc/platforms/pseries/iommu.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c index f6e9b87c82fc..2334a67c7614 100644 --- a/arch/powerpc/platforms/pseries/iommu.c +++ b/arch/powerpc/platforms/pseries/iommu.c @@ -192,7 +192,8 @@ static int tce_buildmulti_pSeriesLP(struct iommu_table *tbl, long tcenum, int ret = 0; unsigned long flags; - if ((npages == 1) || !firmware_has_feature(FW_FEATURE_MULTITCE)) { + if ((npages == 1) || !firmware_has_feature(FW_FEATURE_MULTITCE) || + is_secure_guest()) { return tce_build_pSeriesLP(tbl->it_index, tcenum, tbl->it_page_shift, npages, uaddr, direction, attrs); @@ -402,7 +403,8 @@ static int tce_setrange_multi_pSeriesLP(unsigned long start_pfn, u64 rc = 0; long l, limit; - if (!firmware_has_feature(FW_FEATURE_MULTITCE)) { + if (!firmware_has_feature(FW_FEATURE_MULTITCE) || + is_secure_guest()) { unsigned long tceshift = be32_to_cpu(maprange->tce_shift); unsigned long dmastart = (start_pfn << PAGE_SHIFT) + be64_to_cpu(maprange->dma_base); -- 2.17.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH kernel 3/3] powerpc/pseries/iommu: Do not use H_PUT_TCE_INDIRECT in secure VM 2019-12-13 8:45 ` [PATCH kernel 3/3] powerpc/pseries/iommu: Do not use H_PUT_TCE_INDIRECT in secure VM Alexey Kardashevskiy @ 2019-12-13 10:20 ` Michael Ellerman 0 siblings, 0 replies; 7+ messages in thread From: Michael Ellerman @ 2019-12-13 10:20 UTC (permalink / raw) To: Alexey Kardashevskiy, linuxppc-dev Cc: Alexey Kardashevskiy, Ram Pai, Thiago Jung Bauermann, kvm-ppc, David Gibson Alexey Kardashevskiy <aik@ozlabs.ru> writes: > H_PUT_TCE_INDIRECT uses a 4K page with TCEs to allow handling up to 512 > TCEs per hypercall. While it is a decent optimization, we rather share > less of secure VM memory so let's avoid sharing. > > This only allows H_PUT_TCE_INDIRECT for normal (not secure) VMs. > > This keeps using H_STUFF_TCE as it does not require sharing. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > > Possible alternatives are: > > 1. define FW_FEATURE_STUFFTCE (to allow H_STUFF_TCE) in addition to > FW_FEATURE_MULTITCE (make it only enable H_PUT_TCE_INDIRECT) and enable > only FW_FEATURE_STUFFTCE for SVM; pro = no SVM mention in iommu.c, > con = a FW feature bit with very limited use Yes let's do that please. Actually let's rename FW_FEATURE_MULTITCE to FW_FEATURE_PUT_TCE_IND (?) to make it clear that's what it controls now. You should just be able to add two entries to hypertas_fw_features_table that both look for "hcall-multi-tce". And then the SVM code disables the PUT_TCE_IND feature at some point. cheers > > 2. disable FW_FEATURE_MULTITCE and loose H_STUFF_TCE which adds a delay > in booting process > --- > arch/powerpc/platforms/pseries/iommu.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c > index f6e9b87c82fc..2334a67c7614 100644 > --- a/arch/powerpc/platforms/pseries/iommu.c > +++ b/arch/powerpc/platforms/pseries/iommu.c > @@ -192,7 +192,8 @@ static int tce_buildmulti_pSeriesLP(struct iommu_table *tbl, long tcenum, > int ret = 0; > unsigned long flags; > > - if ((npages == 1) || !firmware_has_feature(FW_FEATURE_MULTITCE)) { > + if ((npages == 1) || !firmware_has_feature(FW_FEATURE_MULTITCE) || > + is_secure_guest()) { > return tce_build_pSeriesLP(tbl->it_index, tcenum, > tbl->it_page_shift, npages, uaddr, > direction, attrs); > @@ -402,7 +403,8 @@ static int tce_setrange_multi_pSeriesLP(unsigned long start_pfn, > u64 rc = 0; > long l, limit; > > - if (!firmware_has_feature(FW_FEATURE_MULTITCE)) { > + if (!firmware_has_feature(FW_FEATURE_MULTITCE) || > + is_secure_guest()) { > unsigned long tceshift = be32_to_cpu(maprange->tce_shift); > unsigned long dmastart = (start_pfn << PAGE_SHIFT) + > be64_to_cpu(maprange->dma_base); > -- > 2.17.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-12-13 20:42 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-13 8:45 [PATCH kernel 0/3] Enable IOMMU support for pseries Secure VMs Alexey Kardashevskiy 2019-12-13 8:45 ` [PATCH kernel 1/3] powerpc/pseries/iommu: Use dma_iommu_ops for Secure VM Alexey Kardashevskiy 2019-12-13 10:20 ` Michael Ellerman 2019-12-13 8:45 ` [PATCH kernel 2/3] powerpc/pseries: Allow not having ibm, hypertas-functions::hcall-multi-tce for DDW Alexey Kardashevskiy 2019-12-13 20:32 ` [PATCH kernel 2/3] powerpc/pseries: Allow not having ibm,hypertas-functions::hcall-multi-tce " Ram Pai 2019-12-13 8:45 ` [PATCH kernel 3/3] powerpc/pseries/iommu: Do not use H_PUT_TCE_INDIRECT in secure VM Alexey Kardashevskiy 2019-12-13 10:20 ` Michael Ellerman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).