[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YzGuc7jVSvE2g91T@nvidia.com>
Date: Mon, 26 Sep 2022 10:51:47 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Niklas Schnelle <schnelle@...ux.ibm.com>
Cc: Matthew Rosato <mjrosato@...ux.ibm.com>,
Pierre Morel <pmorel@...ux.ibm.com>, iommu@...ts.linux.dev,
linux-s390@...r.kernel.org, borntraeger@...ux.ibm.com,
hca@...ux.ibm.com, gor@...ux.ibm.com,
gerald.schaefer@...ux.ibm.com, agordeev@...ux.ibm.com,
svens@...ux.ibm.com, joro@...tes.org, will@...nel.org,
robin.murphy@....com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/3] iommu/s390: Fix duplicate domain attachments
On Mon, Sep 26, 2022 at 03:29:49PM +0200, Niklas Schnelle wrote:
> I did miss a problem in my initial answer. While zpci_register_ioat()
> is indeed the actual "attach" operation, it assumes that at that point
> no DMA address translations are registered. In that state DMA is
> blocked of course. With that zpci_register_ioat() needs to come after
> the zpci_unregister_ioat() that is part of __s390_iommu_detach_device()
> and zpci_dma_exit_device(). If we do call those though we fundamentally
> need to restore the previous domain / DMA API state on any subsequent
> failure. If we don't restore we would leave the device detached from
> any domain with DMA blocked. I wonder if this could be an acceptable
> failure state though? It's safe as no DMA is possible and we could get
> out of it with a successful attach.
Is this because of that FW call it does?
It seems like an FW API misdesign to not allow an unfailable change of
translation, if the FW forces an unregister first then there are
always error cases you can't correctly recover from.
IMHO if the FW fails an attach you are just busted, there is no reason
to think it would suddenly accept attaching the old domain just
because it has a different physical address, right?
So make it simple, leave it DMA blocked and throw a WARN_ON..
Talk to your FW people about fixing the API?
Jason
Powered by blists - more mailing lists