Re: [PATCH v2 0/5] add non-strict mode support for arm-smmu-v3

2018-06-11 Thread Jean-Philippe Brucker
Hi Zhen Lei,

On 10/06/18 12:07, Zhen Lei wrote:
> v1 -> v2:
> Use the lowest bit of the io_pgtable_ops.unmap's iova parameter to pass the 
> strict mode:
> 0, IOMMU_STRICT;
> 1, IOMMU_NON_STRICT;
> Treat 0 as IOMMU_STRICT, so that the unmap operation can compatible with
> other IOMMUs which still use strict mode. In other words, this patch series
> will not impact other IOMMU drivers. I tried add a new quirk 
> IO_PGTABLE_QUIRK_NON_STRICT
> in io_pgtable_cfg.quirks, but it can not pass the strict mode of the domain 
> from SMMUv3
> driver to io-pgtable module. 
> 
> Add a new member domain_non_strict in struct iommu_dma_cookie, this member 
> will only be
> initialized when the related domain and IOMMU driver support non-strict mode.

It's not obvious from the commit messages or comments what the
non-strict mode involves exactly. Could you add a description, and point
out the trade-off associated with it?

In this mode you don't send an invalidate commands when removing a leaf
entry, but instead send invalidate-all commands at regular interval.
This improves performance but introduces a vulnerability window, which
should be pointed out to users.

IOVA allocation isn't the only problem, I'm concerned about the page
allocator. If unmap() returns early, the TLB entry is still valid after
the kernel reallocate the page. The device can then perform a
use-after-free (instead of getting a translation fault), so a buggy
device will corrupt memory and an untrusted one will access arbitrary data.

Or is there a way in mm to ensure that the page isn't reallocated until
the invalidation succeeds? Could dma-iommu help with this? Having
support from the mm would also help consolidate ATS, mark a page stale
when an ATC invalidation times out. But last time I checked it seemed
quite difficult to implement, and ATS is inherently insecure so I didn't
bother.

At the very least I think it might be worth warning users in dmesg about
this pitfall (and add_taint?). Tell them that an IOMMU in this mode is
good for scatter-gather performance but lacks full isolation. The
"non-strict" name seems somewhat harmless, and people should know what
they're getting into before enabling this.

Thanks,
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces

2018-06-11 Thread Kenneth Lee
On Sat, May 26, 2018 at 10:24:45AM +0800, Kenneth Lee wrote:
> Date: Sat, 26 May 2018 10:24:45 +0800
> From: Kenneth Lee 
> To: Jonathan Cameron 
> Cc: Ilias Apalodimas , Jean-Philippe Brucker
>  , "xieyishe...@huawei.com"
>  , "k...@vger.kernel.org" ,
>  "linux-...@vger.kernel.org" ,
>  "xuza...@huawei.com" , Will Deacon
>  , "ok...@codeaurora.org" ,
>  "linux...@kvack.org" , "yi.l@intel.com"
>  , "ashok@intel.com" ,
>  "t...@semihalf.com" , "j...@8bytes.org" ,
>  "robdcl...@gmail.com" , "bhara...@xilinx.com"
>  , "linux-a...@vger.kernel.org"
>  , "liudongdo...@huawei.com"
>  , "rfr...@cavium.com" ,
>  "devicet...@vger.kernel.org" ,
>  "kevin.t...@intel.com" , Jacob Pan
>  , "alex.william...@redhat.com"
>  , "rgum...@xilinx.com" ,
>  "thunder.leiz...@huawei.com" ,
>  "linux-arm-ker...@lists.infradead.org"
>  , "shunyong.y...@hxt-semitech.com"
>  , "dw...@infradead.org"
>  , "liub...@huawei.com" ,
>  "jcro...@codeaurora.org" ,
>  "iommu@lists.linux-foundation.org" ,
>  Robin Murphy , "christian.koe...@amd.com"
>  , "nwatt...@codeaurora.org"
>  , "baolu...@linux.intel.com"
>  , liguo...@hisilicon.com
> Subject: Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces
> Message-ID: <20180526022445.GA6069@kllp05>
> 
> On Fri, May 25, 2018 at 09:39:59AM +0100, Jonathan Cameron wrote:
> > Date: Fri, 25 May 2018 09:39:59 +0100
> > From: Jonathan Cameron 
> > To: Ilias Apalodimas 
> > CC: Jean-Philippe Brucker ,
> >  "xieyishe...@huawei.com" , "k...@vger.kernel.org"
> >  , "linux-...@vger.kernel.org"
> >  , "xuza...@huawei.com" ,
> >  Will Deacon , "ok...@codeaurora.org"
> >  , "linux...@kvack.org" ,
> >  "yi.l@intel.com" , "ashok@intel.com"
> >  , "t...@semihalf.com" ,
> >  "j...@8bytes.org" , "robdcl...@gmail.com"
> >  , "bhara...@xilinx.com" ,
> >  "linux-a...@vger.kernel.org" ,
> >  "liudongdo...@huawei.com" , "rfr...@cavium.com"
> >  , "devicet...@vger.kernel.org"
> >  , "kevin.t...@intel.com"
> >  , Jacob Pan ,
> >  "alex.william...@redhat.com" ,
> >  "rgum...@xilinx.com" , "thunder.leiz...@huawei.com"
> >  , "linux-arm-ker...@lists.infradead.org"
> >  , "shunyong.y...@hxt-semitech.com"
> >  , "dw...@infradead.org"
> >  , "liub...@huawei.com" ,
> >  "jcro...@codeaurora.org" ,
> >  "iommu@lists.linux-foundation.org" ,
> >  Robin Murphy , "christian.koe...@amd.com"
> >  , "nwatt...@codeaurora.org"
> >  , "baolu...@linux.intel.com"
> >  , liguo...@hisilicon.com,
> >  kenneth-lee-2...@foxmail.com
> > Subject: Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces
> > Message-ID: <20180525093959.4...@huawei.com>
> > 
> > +CC Kenneth Lee
> > 
> > On Fri, 25 May 2018 09:33:11 +0300
> > Ilias Apalodimas  wrote:
> > 
> > > On Thu, May 24, 2018 at 04:04:39PM +0100, Jean-Philippe Brucker wrote:
> > > > On 24/05/18 12:50, Ilias Apalodimas wrote:  
> > > > >> Interesting, I hadn't thought about this use-case before. At first I
> > > > >> thought you were talking about mdev devices assigned to VMs, but I 
> > > > >> think
> > > > >> you're referring to mdevs assigned to userspace drivers instead? Out 
> > > > >> of
> > > > >> curiosity, is it only theoretical or does someone actually need 
> > > > >> this?  
> > > > > 
> > > > > There has been some non upstreamed efforts to have mdev and produce 
> > > > > userspace
> > > > > drivers. Huawei is using it on what they call "wrapdrive" for crypto 
> > > > > devices and
> > > > > we did a proof of concept for ethernet interfaces. At the time we 
> > > > > choose not to
> > > > > involve the IOMMU for the reason you mentioned, but having it there 
> > > > > would be
> > > > > good.  
> > > > 
> > > > I'm guessing there were good reasons to do it that way but I wonder, is
> > > > it not simpler to just have the kernel driver create a /dev/foo, with a
> > > > standard ioctl/mmap/poll interface? Here VFIO adds a layer of
> > > > indirection, and since the mediating driver has to implement these
> > > > operations already, what is gained?  
> > > The best reason i can come up with is "common code". You already have one 
> > > API
> > > doing that for you so we replicate it in a /dev file?
> > > The mdev approach still needs extentions to support what we tried to do 
> > > (i.e
> > > mdev bus might need yo have access on iommu_ops), but as far as i 
> > > undestand it's
> > > a possible case.
> 
> Hi, Jean, Please allow me to share my understanding here:
> https://zhuanlan.zhihu.com/p/35489035
> 
> The reason we do not use the /dev/foo scheme is that the devices to be
> shared are programmable accelerators. We cannot fix up the kernel driver for 
> them.
> > > > 
> > > > Thanks,
> > > > Jean  
> > 
> > 
> 
> -- 
>   -Kenneth Lee (Hisilicon)

I just found this mail was missed in the mailing list. I tried it once
again.

-- 
-Kenneth Lee (Hisilicon)


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces

2018-06-11 Thread Kenneth Lee
On Fri, May 25, 2018 at 09:39:59AM +0100, Jonathan Cameron wrote:
> Date: Fri, 25 May 2018 09:39:59 +0100
> From: Jonathan Cameron 
> To: Ilias Apalodimas 
> CC: Jean-Philippe Brucker ,
>  "xieyishe...@huawei.com" , "k...@vger.kernel.org"
>  , "linux-...@vger.kernel.org"
>  , "xuza...@huawei.com" ,
>  Will Deacon , "ok...@codeaurora.org"
>  , "linux...@kvack.org" ,
>  "yi.l@intel.com" , "ashok@intel.com"
>  , "t...@semihalf.com" ,
>  "j...@8bytes.org" , "robdcl...@gmail.com"
>  , "bhara...@xilinx.com" ,
>  "linux-a...@vger.kernel.org" ,
>  "liudongdo...@huawei.com" , "rfr...@cavium.com"
>  , "devicet...@vger.kernel.org"
>  , "kevin.t...@intel.com"
>  , Jacob Pan ,
>  "alex.william...@redhat.com" ,
>  "rgum...@xilinx.com" , "thunder.leiz...@huawei.com"
>  , "linux-arm-ker...@lists.infradead.org"
>  , "shunyong.y...@hxt-semitech.com"
>  , "dw...@infradead.org"
>  , "liub...@huawei.com" ,
>  "jcro...@codeaurora.org" ,
>  "iommu@lists.linux-foundation.org" ,
>  Robin Murphy , "christian.koe...@amd.com"
>  , "nwatt...@codeaurora.org"
>  , "baolu...@linux.intel.com"
>  , liguo...@hisilicon.com,
>  kenneth-lee-2...@foxmail.com
> Subject: Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces
> Message-ID: <20180525093959.4...@huawei.com>
> Organization: Huawei
> X-Mailer: Claws Mail 3.15.0 (GTK+ 2.24.31; x86_64-w64-mingw32)
> 
> +CC Kenneth Lee
> 
> On Fri, 25 May 2018 09:33:11 +0300
> Ilias Apalodimas  wrote:
> 
> > On Thu, May 24, 2018 at 04:04:39PM +0100, Jean-Philippe Brucker wrote:
> > > On 24/05/18 12:50, Ilias Apalodimas wrote:  
> > > >> Interesting, I hadn't thought about this use-case before. At first I
> > > >> thought you were talking about mdev devices assigned to VMs, but I 
> > > >> think
> > > >> you're referring to mdevs assigned to userspace drivers instead? Out of
> > > >> curiosity, is it only theoretical or does someone actually need this?  
> > > > 
> > > > There has been some non upstreamed efforts to have mdev and produce 
> > > > userspace
> > > > drivers. Huawei is using it on what they call "wrapdrive" for crypto 
> > > > devices and
> > > > we did a proof of concept for ethernet interfaces. At the time we 
> > > > choose not to
> > > > involve the IOMMU for the reason you mentioned, but having it there 
> > > > would be
> > > > good.  
> > > 
> > > I'm guessing there were good reasons to do it that way but I wonder, is
> > > it not simpler to just have the kernel driver create a /dev/foo, with a
> > > standard ioctl/mmap/poll interface? Here VFIO adds a layer of
> > > indirection, and since the mediating driver has to implement these
> > > operations already, what is gained?  
> > The best reason i can come up with is "common code". You already have one 
> > API
> > doing that for you so we replicate it in a /dev file?
> > The mdev approach still needs extentions to support what we tried to do (i.e
> > mdev bus might need yo have access on iommu_ops), but as far as i undestand 
> > it's
> > a possible case.

Hi, Jean, Please allow me to share my understanding here:
https://zhuanlan.zhihu.com/p/35489035

The reason we do not use the /dev/foo scheme is that the devices to be
shared are programmable accelerators. We cannot fix up the kernel driver for
them.

> > > 
> > > Thanks,
> > > Jean  
> 
> 

(p.s. I sent this mail on May 26 from my public email count. But it
seems the email server is blocked. I resent it from my company count until my
colleague told me just now. Sorry for inconvenience)

-- 
-Kenneth(Hisilicon)
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu