Messages in this thread | | | From | <> | Subject | RE: [PATCH 1/3] iommu: Add Visconti5 IOMMU driver | Date | Mon, 20 Jun 2022 05:49:13 +0000 |
| |
Hi,
Thanks for your review.
> -----Original Message----- > From: Jason Gunthorpe <jgg@nvidia.com> > Sent: Thursday, May 26, 2022 3:27 AM > To: Baolu Lu <baolu.lu@linux.intel.com> > Cc: iwamatsu nobuhiro(岩松 信洋 □SWC◯ACT) > <nobuhiro1.iwamatsu@toshiba.co.jp>; Joerg Roedel <joro@8bytes.org>; Will > Deacon <will@kernel.org>; Rob Herring <robh+dt@kernel.org>; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; > iommu@lists.linux-foundation.org; ishikawa yuji(石川 悠司 ○RDC□AIT > C○EA開) <yuji2.ishikawa@toshiba.co.jp>; > linux-arm-kernel@lists.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.
I see, I rethink implementation.
> > 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().
If ATU_AT_ENTRY_EN is 0, nothing happens. It does not work with IOMMU, it works with the memory space set in device tree. Also, I investigate about IOMMU_DOMAIN_BLOCKING.
> > 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 see, I will check DMA during map and unmap operations.
> > 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..
Since the devices that utilize this IOMMU function are fixed, I do not think that a special handshake is required. Could you you tell me where you thought you needed a handshake?
Best regards, Nobuhiro
> > Jason
| |