Re: [PATCH v5 10/14] iommu/amd: Introduce logic to enable/disable IOPF

[Jason Gunthorpe <jgg@ziepe.ca>]

On Tue, Feb 06, 2024 at 09:49:36PM +0530, Vasant Hegde wrote:

> > This dte change is in the wrong place. When the domain is first
> > attached we know if it requires PRI or not. At that moment the DTE
> > should be set properly, it should not be set wrong and then changed
> > later.
> 
> We want to enable PPR support only after setting up the handler.

That's backwards. It means error handling can't really be sane..

> > (and again the whole dte setting flow needs cleaning, I think you
> > should do that before trying to build more complex stuff on top)
> 
> Yeah. I want to fix few things in that path. But that's outside this series.

I was looking at it and there are many security bugs in here now that
iommufd can change the DTE at any time. The current design assumes DMA
will be stopped and ignores the spec guidance on how to do a safe DTE
update :(

I'm really not comfortable with adding more stuff here until the
security issue is solved. Especially if the more stuff is drifting
further from being correct. If you can keep the updates in set_dte
then maybe with some reluctance. But not like this with random touches
to the DTE all over the place.

I was looking at what it would take and got part way through. It is a
very similar problem to ARM and I drafted a similar solution.

Jason