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]

Date:   Wed, 25 May 2022 15:26:44 -0300
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Baolu Lu <baolu.lu@...ux.intel.com>
Cc:     Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@...hiba.co.jp>,
        Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>,
        Rob Herring <robh+dt@...nel.org>, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, iommu@...ts.linux-foundation.org,
        yuji2.ishikawa@...hiba.co.jp, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 1/3] iommu: Add Visconti5 IOMMU driver

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.

What behavior does the HW have when ATU_AT_ENTRY_EN == 0? If DMA is
rejected then this driver should have a IOMMU_DOMAIN_BLOCKING and
return that from ops->def_domain_type().

Attaching a the blocking domain should set ATU_AT_ENTRY_EN = 0

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