Re: [PATCH v1 2/2] dma-mapping-common: add DMA attribute - DMA_ATTR_IOMMU_BYPASS

[Shamir Rabinovitch]

On Thu, Oct 29, 2015 at 10:35:25PM +, David Woodhouse wrote:
> > 
> > Could this be mitigated using pools?  I don't know if the net code
> > would play along easily.
> 
> For the receive side, it shouldn't be beyond the wit of man to
> introduce an API which allocates *and* DMA-maps a skb. Pass it to
> netif_rx() still mapped, with a destructor that just shoves it back in
> a pool for re-use.
> 
> Doing it for transmit might be a little more complex, but perhaps still
> possible.

Not sure this use case is possible for Infiniband where application hold
the data buffers and there is no way to force application to re use the 
buffer as suggested.

This is why I think there will be no easy way to bypass the DMA mapping cost
for all use cases unless we allow applications to request bypass/pass through
DMA mapping (implicitly or explicitly).

> 
> -- 
> dwmw2
> 


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kbuild-all] [PATCH 5/6] KVM: PPC: Book3S HV: Send IPI to host core to wake VCPU

[Fengguang Wu]

Hi Suresh,

Sorry for the noise!

Do you have a git tree that the robot can monitor and test?

In this case of one patchset depending on another, there is no chance
for the robot to do valid testing based on emailed patches.

Thanks,
Fengguang

On Fri, Oct 30, 2015 at 10:16:06AM -0500, Suresh E. Warrier wrote:
> This patch set depends upon a previous patch set that I had submitted to
> linux-ppc. The URL for that is:
> 
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2015-October/135794.html
> 
> -suresh
> 
> On 10/29/2015 11:52 PM, kbuild test robot wrote:
> > Hi Suresh,
> > 
> > [auto build test ERROR on kvm/linux-next -- if it's inappropriate base, 
> > please suggest rules for selecting the more suitable base]
> > 
> > url:
> > https://github.com/0day-ci/linux/commits/Suresh-Warrier/KVM-PPC-Book3S-HV-Optimize-wakeup-VCPU-from-H_IPI/20151030-081329
> > config: powerpc-defconfig (attached as .config)
> > reproduce:
> > wget 
> > https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
> >  -O ~/bin/make.cross
> > chmod +x ~/bin/make.cross
> > # save the attached .config to linux build tree
> > make.cross ARCH=powerpc 
> > 
> > All errors (new ones prefixed by >>):
> > 
> >arch/powerpc/kvm/book3s_hv_rm_xics.c: In function 'icp_rm_set_vcpu_irq':
> >>> arch/powerpc/kvm/book3s_hv_rm_xics.c:142:4: error: implicit declaration 
> >>> of function 'smp_muxed_ipi_rm_message_pass' 
> >>> [-Werror=implicit-function-declaration]
> >smp_muxed_ipi_rm_message_pass(hcpu,
> >^
> >arch/powerpc/kvm/book3s_hv_rm_xics.c:143:7: error: 
> > 'PPC_MSG_RM_HOST_ACTION' undeclared (first use in this function)
> >   PPC_MSG_RM_HOST_ACTION);
> >   ^
> >arch/powerpc/kvm/book3s_hv_rm_xics.c:143:7: note: each undeclared 
> > identifier is reported only once for each function it appears in
> >cc1: all warnings being treated as errors
> > 
> > vim +/smp_muxed_ipi_rm_message_pass +142 
> > arch/powerpc/kvm/book3s_hv_rm_xics.c
> > 
> >136  hcore = -1;
> >137  if (kvmppc_host_rm_ops_hv)
> >138  hcore = 
> > find_available_hostcore(XICS_RM_KICK_VCPU);
> >139  if (hcore != -1) {
> >140  hcpu = hcore << threads_shift;
> >141  
> > kvmppc_host_rm_ops_hv->rm_core[hcore].rm_data = vcpu;
> >  > 142  smp_muxed_ipi_rm_message_pass(hcpu,
> >143  
> > PPC_MSG_RM_HOST_ACTION);
> >144  } else {
> >145  this_icp->rm_action |= 
> > XICS_RM_KICK_VCPU;
> > 
> > ---
> > 0-DAY kernel test infrastructureOpen Source Technology 
> > Center
> > https://lists.01.org/pipermail/kbuild-all   Intel 
> > Corporation
> > 
> 
> ___
> kbuild-all mailing list
> kbuild-...@lists.01.org
> https://lists.01.org/mailman/listinfo/kbuild-all
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 6/6] arm-smmu: Allow to set iommu mapping for MSI

[Pranavkumar Sawargaonkar]

Hi Bharat,

On 6 October 2015 at 15:56, Bhushan Bharat  wrote:
>
>
>> -Original Message-
>> From: Alex Williamson [mailto:alex.william...@redhat.com]
>> Sent: Tuesday, October 06, 2015 4:25 AM
>> To: Bhushan Bharat-R65777 
>> Cc: kvm...@lists.cs.columbia.edu; kvm@vger.kernel.org;
>> christoffer.d...@linaro.org; eric.au...@linaro.org; pranavku...@linaro.org;
>> marc.zyng...@arm.com; will.dea...@arm.com
>> Subject: Re: [RFC PATCH 6/6] arm-smmu: Allow to set iommu mapping for
>> MSI
>>
>> On Mon, 2015-10-05 at 08:33 +, Bhushan Bharat wrote:
>> >
>> >
>> > > -Original Message-
>> > > From: Alex Williamson [mailto:alex.william...@redhat.com]
>> > > Sent: Saturday, October 03, 2015 4:17 AM
>> > > To: Bhushan Bharat-R65777 
>> > > Cc: kvm...@lists.cs.columbia.edu; kvm@vger.kernel.org;
>> > > christoffer.d...@linaro.org; eric.au...@linaro.org;
>> > > pranavku...@linaro.org; marc.zyng...@arm.com; will.dea...@arm.com
>> > > Subject: Re: [RFC PATCH 6/6] arm-smmu: Allow to set iommu mapping
>> > > for MSI
>> > >
>> > > On Wed, 2015-09-30 at 20:26 +0530, Bharat Bhushan wrote:
>> > > > Finally ARM SMMU declare that iommu-mapping for MSI-pages are not
>> > > > set automatically and it should be set explicitly.
>> > > >
>> > > > Signed-off-by: Bharat Bhushan 
>> > > > ---
>> > > >  drivers/iommu/arm-smmu.c | 7 ++-
>> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
>> > > >
>> > > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> > > index
>> > > > a3956fb..9d37e72 100644
>> > > > --- a/drivers/iommu/arm-smmu.c
>> > > > +++ b/drivers/iommu/arm-smmu.c
>> > > > @@ -1401,13 +1401,18 @@ static int
>> arm_smmu_domain_get_attr(struct
>> > > iommu_domain *domain,
>> > > > enum iommu_attr attr, void *data)  
>> > > > {
>> > > > struct arm_smmu_domain *smmu_domain =
>> > > to_smmu_domain(domain);
>> > > > +   struct iommu_domain_msi_maps *msi_maps;
>> > > >
>> > > > switch (attr) {
>> > > > case DOMAIN_ATTR_NESTING:
>> > > > *(int *)data = (smmu_domain->stage ==
>> > > ARM_SMMU_DOMAIN_NESTED);
>> > > > return 0;
>> > > > case DOMAIN_ATTR_MSI_MAPPING:
>> > > > -   /* Dummy handling added */
>> > > > +   msi_maps = data;
>> > > > +
>> > > > +   msi_maps->automap = false;
>> > > > +   msi_maps->override_automap = true;
>> > > > +
>> > > > return 0;
>> > > > default:
>> > > > return -ENODEV;
>> > >
>> > > In previous discussions I understood one of the problems you were
>> > > trying to solve was having a limited number of MSI banks and while
>> > > you may be able to get isolated MSI banks for some number of users,
>> > > it wasn't unlimited and sharing may be required.  I don't see any of that
>> addressed in this series.
>> >
>> > That problem was on PowerPC. Infact there were two problems, one which
>> MSI bank to be used and second how to create iommu-mapping for device
>> assigned to userspace.
>> > First problem was PowerPC specific and that will be solved separately.
>> > For second problem, earlier I tried to added a couple of MSI specific 
>> > ioctls
>> and you suggested (IIUC) that we should have a generic reserved-iova type
>> of API and then we can map MSI bank using reserved-iova and this will not
>> require involvement of user-space.
>> >
>> > >
>> > > Also, the management of reserved IOVAs vs MSI addresses looks really
>> > > dubious to me.  How does your platform pick an MSI address and what
>> > > are we breaking by covertly changing it?  We seem to be masking over
>> > > at the VFIO level, where there should be lower level interfaces
>> > > doing the right thing when we configure MSI on the device.
>> >
>> > Yes, In my understanding the right solution should be:
>> >  1) VFIO driver should know what physical-msi-address will be used for
>> devices in an iommu-group.
>> > I did not find an generic API, on PowerPC I added some function in
>> ffrescale msi-driver and called from vfio-iommu-fsl-pamu.c (not yet
>> upstreamed).
>> >  2) VFIO driver should know what IOVA to be used for creating
>> > iommu-mapping (VFIO APIs patch of this patch series)
>> >  3) VFIO driver will create the iommu-mapping using (1) and (2)
>> >  4) VFIO driver should be able to tell the msi-driver that for a given 
>> > device it
>> should use different IOVA. So when composing the msi message (for the
>> devices is the given iommu-group) it should use that programmed iova as
>> MSI-address. This interface also needed to be developed.
>> >
>> > I was not sure of which approach we should take. The current approach in
>> the patch is simple to develop so I went ahead to take input but I agree this
>> does not look very good.
>> > What do you think, should drop this approach and work out 

Re: [PATCH v1 2/2] dma-mapping-common: add DMA attribute - DMA_ATTR_IOMMU_BYPASS

[Benjamin Herrenschmidt]

On Sun, 2015-11-01 at 09:45 +0200, Shamir Rabinovitch wrote:
> Not sure this use case is possible for Infiniband where application hold
> the data buffers and there is no way to force application to re use the 
> buffer as suggested.
> 
> This is why I think there will be no easy way to bypass the DMA mapping cost
> for all use cases unless we allow applications to request bypass/pass through
> DMA mapping (implicitly or explicitly).

But but but ...

What I don't understand is how that brings you any safety.

Basically, either your bridge has a bypass window, or it doesn't. (Or
it has one and it's enabled or not, same thing).

If you request the bypass on a per-mapping basis, you basically have to
keep the window always enabled, unless you do some nasty refcounting of
how many people have a bypass mapping in flight, but that doesn't seem
useful.

Thus you have already lost all protection from the device, since your
entire memory is accessible via the bypass mapping.

Which means what is the point of then having non-bypass mappings for
other things ? Just to make things slower ?

I really don't see what this whole "bypass on a per-mapping basis" buys
you.

Note that we implicitly already do that on powerpc, but not for those
reasons, we do it based on the DMA mask, so that if your coherent mask
is 32-bit but your dma mask is 64-bit (which is not an uncommon
combination), we service the "coherent" requests (basically the long
lifed dma allocs) from the remapped region and the "stream" requests
(ie, map_page/map_sg) from the bypass.

Cheers,
Ben.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 2/2] dma-mapping-common: add DMA attribute - DMA_ATTR_IOMMU_BYPASS

[Shamir Rabinovitch]

On Mon, Nov 02, 2015 at 08:10:49AM +1100, Benjamin Herrenschmidt wrote:
> But but but ...
> 
> What I don't understand is how that brings you any safety.

Limited safety maybe? If some device DMA mappings are via IOMMU 
and this fall to some address range that is far from the bypass / 
pass through range then small drifts in address might be still figured 
if we do not bypass / pass through the IOMMU - right?

Device can sure use the bypass address and just reach the memory w/o 
IOMMU protection. See some comments about that below.

> 
> Basically, either your bridge has a bypass window, or it doesn't. (Or
> it has one and it's enabled or not, same thing).

Agree.

> 
> If you request the bypass on a per-mapping basis, you basically have to
> keep the window always enabled, unless you do some nasty refcounting of
> how many people have a bypass mapping in flight, but that doesn't seem
> useful.
> 
> Thus you have already lost all protection from the device, since your
> entire memory is accessible via the bypass mapping.

Correct, see my above comment. 

> 
> Which means what is the point of then having non-bypass mappings for
> other things ? Just to make things slower ?
> 
> I really don't see what this whole "bypass on a per-mapping basis" buys
> you.
> 
> Note that we implicitly already do that on powerpc, but not for those
> reasons, we do it based on the DMA mask, so that if your coherent mask
> is 32-bit but your dma mask is 64-bit (which is not an uncommon
> combination), we service the "coherent" requests (basically the long
> lifed dma allocs) from the remapped region and the "stream" requests
> (ie, map_page/map_sg) from the bypass.



To summary -

1. The whole point of the IOMMU pass through was to get bigger address space
and faster map/unmap operations for performance critical hardware
2. SPARC IOMMU in particular has the ability to DVMA which adress all the 
protection concerns raised above. Not sure what will be the performance
impact though. This still need a lot of work before we could test this.
3. On x86 we use IOMMU in pass through mode so all the above concerns are valid

The question are -

1. Does partial use of IOMMU while the pass through window is enabled add some
protection?
2. Do we rather the x86 way of doing this which is enable / disable IOMMU 
translations at kernel level?

I think that I can live with option (2) till I have DVMA if there is strong
disagree on the need for per allocation IOMMU bypass.


> 
> Cheers,
> Ben.
> 
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html