linux-kernel - RE: [PATCH 1/3] iommu: Add Visconti5 IOMMU driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BN9PR11MB52769A5FE5B8291F7D47D5AE8CD99@BN9PR11MB5276.namprd11.prod.outlook.com>
Date:   Thu, 26 May 2022 00:43:25 +0000
From:   "Tian, Kevin" <kevin.tian@...el.com>
To:     Jason Gunthorpe <jgg@...dia.com>,
        Baolu Lu <baolu.lu@...ux.intel.com>
CC:     "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "yuji2.ishikawa@...hiba.co.jp" <yuji2.ishikawa@...hiba.co.jp>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
        "Rob Herring" <robh+dt@...nel.org>,
        Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@...hiba.co.jp>,
        Will Deacon <will@...nel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: RE: [PATCH 1/3] iommu: Add Visconti5 IOMMU driver

> From: Jason Gunthorpe
> Sent: Thursday, May 26, 2022 2:27 AM
> 
> On Wed, May 25, 2022 at 02:26:37PM +0800, Baolu Lu wrote:
> > On 2022/5/25 09:31, Nobuhiro Iwamatsu wrote:
> > > +static const struct iommu_ops visconti_atu_ops = {
> > > +	.domain_alloc = visconti_atu_domain_alloc,
> > > +	.probe_device = visconti_atu_probe_device,
> > > +	.release_device = visconti_atu_release_device,
> > > +	.device_group = generic_device_group,
> > > +	.of_xlate = visconti_atu_of_xlate,
> > > +	.pgsize_bitmap = ATU_IOMMU_PGSIZE_BITMAP,
> > > +	.default_domain_ops = &(const struct iommu_domain_ops) {
> > > +		.attach_dev = visconti_atu_attach_device,
> > > +		.detach_dev = visconti_atu_detach_device,
> >
> > The detach_dev callback is about to be deprecated. The new drivers
> > should implement the default domain and blocking domain instead.
> 
> Yes please, new drivers need to use default_domains.
> 
> It is very strange that visconti_atu_detach_device() does nothing.  It
> is not required that a domain is fully unmapped before being
> destructed, I think detach should set ATU_AT_EN to 0.

Looks the atu is shared by all devices behind and can only serve one
I/O address space. The atu registers only keep information about
iova ranges without any device notation. That is probably the reason 
why both attach/detach() don't touch hardware.

iiuc then this suggests there should be only one iommu group per atu,
instead of using generic_device_group() to create one group per
device.

> 
> What behavior does the HW have when ATU_AT_ENTRY_EN == 0? If DMA is

I guess it's a blocking behavior since that register tracks which iova range
register is valid. 

> rejected then this driver should have a IOMMU_DOMAIN_BLOCKING and
> return that from ops->def_domain_type().

BLOCKING should not be used as a default domain type for DMA API
which needs either a DMA or IDENTITY type.

> 
> Attaching a the blocking domain should set ATU_AT_ENTRY_EN = 0

Agree

> 
> Also, if I surmise how this works properly, it is not following the
> iommu API to halt all DMA during map/unmap operations. Should at least
> document this and explain why it is OK..
> 
> I'm feeling like these "special" drivers need some kind of handshake
> with their only users because they don't work with things like VFIO..
> 
> Jason