On 2022-09-07 14:47, Jason Gunthorpe wrote:
On Wed, Sep 07, 2022 at 02:41:54PM +0200, Joerg Roedel wrote:
On Mon, Aug 15, 2022 at 11:14:33AM -0700, Nicolin Chen wrote:
Provide a dedicated errno from the IOMMU driver during attach that the
reason attached failed is because of domain incompatability. EMEDIUMTYPE
is chosen because it is never used within the iommu subsystem today and
evokes a sense that the 'medium' aka the domain is incompatible.
I am not a fan of re-using EMEDIUMTYPE or any other special value. What
is needed here in EINVAL, but with a way to tell the caller which of the
function parameters is actually invalid.
Using errnos to indicate the nature of failure is a well established
unix practice, it is why we have hundreds of error codes and don't
just return -EINVAL for everything.
What don't you like about it?
Would you be happier if we wrote it like
#define IOMMU_EINCOMPATIBLE_DEVICE xx
Which tells "which of the function parameters is actually invalid" ?
FWIW, we're now very close to being able to validate dev->iommu against
where the domain came from in core code, and so short-circuit
->attach_dev entirely if they don't match. At that point -EINVAL at the
driver callback level could be assumed to refer to the domain argument,
while anything else could be taken as something going unexpectedly wrong
when the attach may otherwise have worked. I've forgotten if we actually
had a valid case anywhere for "this is my device but even if you retry
with a different domain it's still never going to work", but I think we
wouldn't actually need that anyway - it should be clear enough to a
caller that if attaching to an existing domain fails, then allocating a
fresh domain and attaching also fails, that's the point to give up.
Robin.
