Re: [PATCH v6 2/6] iommufd: Add IOMMU_HWPT_INVALIDATE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2023/12/1 15:10, Tian, Kevin wrote:
From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
Sent: Friday, December 1, 2023 3:05 PM

On 2023/12/1 13:19, Tian, Kevin wrote:
From: Nicolin Chen <nicolinc@xxxxxxxxxx>
Sent: Friday, December 1, 2023 12:50 PM

On Fri, Dec 01, 2023 at 11:51:26AM +0800, Yi Liu wrote:
On 2023/11/29 08:57, Jason Gunthorpe wrote:
On Tue, Nov 28, 2023 at 04:51:21PM -0800, Nicolin Chen wrote:
I also thought about making this out_driver_error_code per HW.
Yet, an error can be either per array or per entry/quest. The
array-related error should be reported in the array structure
that is a core uAPI, v.s. the per-HW entry structure. Though
we could still report an array error in the entry structure
at the first entry (or indexed by "array->entry_num")?


why would there be an array error? array is just a software
entity containing actual HW invalidation cmds. If there is
any error with the array itself it should be reported via
ioctl errno.

User array reading is a software operation, but kernel array
reading is a hardware operation that can raise an error when
the memory location to the array is incorrect or so.

Well, we shouldn't get into a situation like that.. By the time the HW
got the address it should be valid.

With that being said, I think errno (-EIO) could do the job,
as you suggested too.

Do we have any idea what HW failures can be generated by the
commands
this will execture? IIRC I don't remember seeing any smmu specific
codes related to invalid invalidation? Everything is a valid input?

Can vt-d fail single commands? What about AMD?

Intel VT-d side, after each invalidation request, there is a wait
descriptor which either provide an interrupt or an address for the
hw to notify software the request before the wait descriptor has been
completed. While, if there is error happened on the invalidation request,
a flag (IQE, ICE, ITE) would be set in the Fault Status Register, and some
detailed information would be recorded in the Invalidation Queue Error
Record Register. So an invalidation request may be failed with some
error
reported. If no error, will return completion via the wait descriptor. Is
this what you mean by "fail a single command"?

I see the current VT-d series marking those as "REVISIT". How
will it report an error to the user space from those register?

Are they global status registers so that it might be difficult
to direct the error to the nested domain for an event fd?


They are global registers but invalidation queue is also the global
resource. intel-iommu driver polls the status register after queueing
new invalidation descriptors. The submission is serialized.

If the error is related to a descriptor itself (e.g. format issue) then
the head register points to the problematic descriptor so software
can direct it to the related domain.

If the error is related to device tlb invalidation (e.g. timeout) there
is no way to associate the error with a specific descriptor by current
spec. But intel-iommu driver batches descriptors per domain so
we can still direct the error to the nested domain.

But I don't see the need of doing it via eventfd.

The poll semantics in intel-iommu driver is essentially a sync model.
vt-d spec does allow software to optionally enable notification upon
those errors but it's not used so far.

With that I still prefer to having driver-specific error code defined
in the entry. If ARM is an event-driven model then we can define
that field at least in vtd specific data structure.

btw given vtd doesn't use native format in uAPI it doesn't make
sense to forward descriptor formatting errors back to userspace.
Those, if happen, are driver's own problem. intel-iommu driver
should verify the uAPI structure and return -EINVAL or proper
errno to userspace purely in software.

With that Yi please just define error codes for device tlb related
errors for vtd.

hmmm, this sounds like customized error code. is it? So far, VT-d

yes. there is no need to replicate hardware registers/bits if most
of them are irrelevant to userspace.

spec has two errors (ICE and ITE). ITE is valuable to let userspace
know. For ICE, looks like no much value. Intel iommu driver should
be responsible to submit a valid device-tlb invalidation to device.

it's an invalid completion message from the device which could be
caused by various reasons (not exactly due to the invalidation
request by iommu driver). so it still makes sense to forward.

ok. so we may need to define a field to forward the detailed info to
user as well. This data is error-code specific. @Nic, are we aligned
that the error_code field and error data reporting should be moved
to the driver-specific part since it is different between vendors?

--
Regards,
Yi Liu




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux