From: Ram Pai <linuxram@us.ibm.com> To: Alexey Kardashevskiy <aik@ozlabs.ru> Cc: mpe@ellerman.id.au, linuxppc-dev@lists.ozlabs.org, benh@kernel.crashing.org, david@gibson.dropbear.id.au, paulus@ozlabs.org, mdroth@linux.vnet.ibm.com, hch@lst.de, andmike@us.ibm.com, sukadev@linux.vnet.ibm.com, mst@redhat.com, ram.n.pai@gmail.com, cai@lca.pw, tglx@linutronix.de, bauerman@linux.ibm.com, linux-kernel@vger.kernel.org, leonardo@linux.ibm.com Subject: RE: [PATCH v5 1/2] powerpc/pseries/iommu: Share the per-cpu TCE page with the hypervisor. Date: Wed, 11 Dec 2019 20:11:15 -0800 [thread overview] Message-ID: <20191212041115.GC5702@oc0525413822.ibm.com> (raw) In-Reply-To: <90f6019b-d756-7f33-21b0-bb49c1c842da@ozlabs.ru> On Wed, Dec 11, 2019 at 07:15:44PM +1100, Alexey Kardashevskiy wrote: > > > On 11/12/2019 02:35, Ram Pai wrote: > > On Tue, Dec 10, 2019 at 04:32:10PM +1100, Alexey Kardashevskiy wrote: > >> ..snip.. > >> As discussed in slack, by default we do not need to clear the entire TCE > >> table and we only have to map swiotlb buffer using the small window. It > >> is a guest kernel change only. Thanks, > > > > Can you tell me what code you are talking about here. Where is the TCE > > table getting cleared? What code needs to be changed to not clear it? > > > pci_dma_bus_setup_pSeriesLP() > iommu_init_table() > iommu_table_clear() > for () tbl->it_ops->get() > > We do not really need to clear it there, we only need it for VFIO with > IOMMU SPAPR TCE v1 which reuses these tables but there are > iommu_take_ownership/iommu_release_ownership to clear these tables. I'll > send a patch for this. Did some experiments. It spent the first 9s in tce_free_pSeriesLP() clearing the tce entries. And the second 13s in tce_setrange_multi_pSeriesLP_walk(). BTW: the code in tce_setrange_multi_pSeriesLP_walk() is modified to use DIRECT_TCE. So it looks like the amount of time spent in tce_setrange_multi_pSeriesLP_walk() is a function of the size of the memory that is mapped in the ddw. > ..snip.. > > > But before I close, you have not told me clearly, what is the problem > > with; 'share the page, make the H_PUT_INDIRECT_TCE hcall, unshare the page'. > > Between share and unshare you have a (tiny) window of opportunity to > attack the guest. No, I do not know how exactly. > > For example, the hypervisor does a lot of PHB+PCI hotplug-unplug with > 64bit devices - each time this will create a huge window which will > share/unshare the same page. No, I do not know how exactly how this can > be exploited either, we cannot rely of what you or myself know today. My > point is that we should not be sharing pages at all unless we really > really have to, and this does not seem to be the case. > > But since this seems to an acceptable compromise anyway, > > Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > Thanks! RP
WARNING: multiple messages have this Message-ID (diff)
From: Ram Pai <linuxram@us.ibm.com> To: Alexey Kardashevskiy <aik@ozlabs.ru> Cc: andmike@us.ibm.com, mst@redhat.com, mdroth@linux.vnet.ibm.com, leonardo@linux.ibm.com, linux-kernel@vger.kernel.org, ram.n.pai@gmail.com, cai@lca.pw, tglx@linutronix.de, sukadev@linux.vnet.ibm.com, linuxppc-dev@lists.ozlabs.org, hch@lst.de, bauerman@linux.ibm.com, david@gibson.dropbear.id.au Subject: RE: [PATCH v5 1/2] powerpc/pseries/iommu: Share the per-cpu TCE page with the hypervisor. Date: Wed, 11 Dec 2019 20:11:15 -0800 [thread overview] Message-ID: <20191212041115.GC5702@oc0525413822.ibm.com> (raw) In-Reply-To: <90f6019b-d756-7f33-21b0-bb49c1c842da@ozlabs.ru> On Wed, Dec 11, 2019 at 07:15:44PM +1100, Alexey Kardashevskiy wrote: > > > On 11/12/2019 02:35, Ram Pai wrote: > > On Tue, Dec 10, 2019 at 04:32:10PM +1100, Alexey Kardashevskiy wrote: > >> ..snip.. > >> As discussed in slack, by default we do not need to clear the entire TCE > >> table and we only have to map swiotlb buffer using the small window. It > >> is a guest kernel change only. Thanks, > > > > Can you tell me what code you are talking about here. Where is the TCE > > table getting cleared? What code needs to be changed to not clear it? > > > pci_dma_bus_setup_pSeriesLP() > iommu_init_table() > iommu_table_clear() > for () tbl->it_ops->get() > > We do not really need to clear it there, we only need it for VFIO with > IOMMU SPAPR TCE v1 which reuses these tables but there are > iommu_take_ownership/iommu_release_ownership to clear these tables. I'll > send a patch for this. Did some experiments. It spent the first 9s in tce_free_pSeriesLP() clearing the tce entries. And the second 13s in tce_setrange_multi_pSeriesLP_walk(). BTW: the code in tce_setrange_multi_pSeriesLP_walk() is modified to use DIRECT_TCE. So it looks like the amount of time spent in tce_setrange_multi_pSeriesLP_walk() is a function of the size of the memory that is mapped in the ddw. > ..snip.. > > > But before I close, you have not told me clearly, what is the problem > > with; 'share the page, make the H_PUT_INDIRECT_TCE hcall, unshare the page'. > > Between share and unshare you have a (tiny) window of opportunity to > attack the guest. No, I do not know how exactly. > > For example, the hypervisor does a lot of PHB+PCI hotplug-unplug with > 64bit devices - each time this will create a huge window which will > share/unshare the same page. No, I do not know how exactly how this can > be exploited either, we cannot rely of what you or myself know today. My > point is that we should not be sharing pages at all unless we really > really have to, and this does not seem to be the case. > > But since this seems to an acceptable compromise anyway, > > Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > Thanks! RP
next prev parent reply other threads:[~2019-12-12 4:11 UTC|newest] Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-07 1:12 [PATCH v5 0/2] Enable IOMMU support for pseries Secure VMs Ram Pai 2019-12-07 1:12 ` Ram Pai 2019-12-07 1:12 ` [PATCH v5 1/2] powerpc/pseries/iommu: Share the per-cpu TCE page with the hypervisor Ram Pai 2019-12-07 1:12 ` Ram Pai 2019-12-07 1:12 ` [PATCH v5 2/2] powerpc/pseries/iommu: Use dma_iommu_ops for Secure VM Ram Pai 2019-12-07 1:12 ` Ram Pai 2019-12-10 3:08 ` Alexey Kardashevskiy 2019-12-10 3:08 ` Alexey Kardashevskiy 2019-12-10 22:09 ` Thiago Jung Bauermann 2019-12-10 22:09 ` Thiago Jung Bauermann 2019-12-11 1:43 ` Michael Roth 2019-12-11 1:43 ` Michael Roth 2019-12-11 8:36 ` Alexey Kardashevskiy 2019-12-11 8:36 ` Alexey Kardashevskiy 2019-12-11 18:07 ` Michael Roth 2019-12-11 18:07 ` Michael Roth 2019-12-11 18:20 ` Christoph Hellwig 2019-12-11 18:20 ` Christoph Hellwig 2019-12-12 6:45 ` Ram Pai 2019-12-12 6:45 ` Ram Pai 2019-12-13 0:19 ` Michael Roth 2019-12-13 0:19 ` Michael Roth 2019-12-10 3:07 ` [PATCH v5 1/2] powerpc/pseries/iommu: Share the per-cpu TCE page with the hypervisor Alexey Kardashevskiy 2019-12-10 3:07 ` Alexey Kardashevskiy 2019-12-10 5:12 ` Ram Pai 2019-12-10 5:12 ` Ram Pai 2019-12-10 5:32 ` Alexey Kardashevskiy 2019-12-10 5:32 ` Alexey Kardashevskiy 2019-12-10 15:35 ` Ram Pai 2019-12-10 15:35 ` Ram Pai 2019-12-11 8:15 ` Alexey Kardashevskiy 2019-12-11 8:15 ` Alexey Kardashevskiy 2019-12-11 20:31 ` Michael Roth 2019-12-11 20:31 ` Michael Roth 2019-12-11 22:47 ` Alexey Kardashevskiy 2019-12-11 22:47 ` Alexey Kardashevskiy 2019-12-12 2:39 ` Alexey Kardashevskiy 2019-12-12 2:39 ` Alexey Kardashevskiy 2019-12-13 0:22 ` Michael Roth 2019-12-13 0:22 ` Michael Roth 2019-12-12 4:11 ` Ram Pai [this message] 2019-12-12 4:11 ` Ram Pai
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20191212041115.GC5702@oc0525413822.ibm.com \ --to=linuxram@us.ibm.com \ --cc=aik@ozlabs.ru \ --cc=andmike@us.ibm.com \ --cc=bauerman@linux.ibm.com \ --cc=benh@kernel.crashing.org \ --cc=cai@lca.pw \ --cc=david@gibson.dropbear.id.au \ --cc=hch@lst.de \ --cc=leonardo@linux.ibm.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=mdroth@linux.vnet.ibm.com \ --cc=mpe@ellerman.id.au \ --cc=mst@redhat.com \ --cc=paulus@ozlabs.org \ --cc=ram.n.pai@gmail.com \ --cc=sukadev@linux.vnet.ibm.com \ --cc=tglx@linutronix.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.