Re: [PATCH v2 2/5] iommu/amd: Introduce rw_semaphore for Device Table Entry (DTE)

[Jason Gunthorpe]

On Thu, Sep 05, 2024 at 01:20:42PM +0700, Suthikulpanit, Suravee wrote:

> > There is no such thing as 'clear' in the iommu domain API. The DTE is
> > either PAGING, BLOCKED or IDENTITY, and any write to it should be
> > writing an entire DTE for that target.
> > 
> > I guess clear is actually trying to set the DTE to BLOCKING?
> 
> Yes, it's called from blocked_domain->attach_dev() and when doing
> amd_iommu_detach_device().

Could give it a better name then too
 
> > Also no need to get the lock here because you don't touch 128 bit
> > quanta [1] which holds the IRQ stuff that is racey. This is already
> > locked by the iommu core group lock.
> 
> Actually, we Need to preserve DTE[96:106] because certain fields are
> programmed using value in IVRS table from early init phase. So, to avoid
> try_cmpxchg128 failure, I need to spin_lock on dte_lock across the
> get_dte256() and update_dte256()

But who is concurrently writing those bits? Isn't it just set at boot
time once and sort of stored in the DTE, never changing?

There are two locking regimes here, all the normal DTE touches from
iomm ops are locked by the group mutex and they are non-concurrent
already.

Jason