On Mon, Jul 08, 2024 at 01:29:57PM -0300, Jason Gunthorpe wrote:
> On Wed, Jul 03, 2024 at 04:06:15PM -0700, Nicolin Chen wrote:
>
> > I learned that this hwpt->fault is exclusively for IOPF/PRI. And
> > Jason suggested me to add a different one for VIOMMU. Yet, after
> > taking a closer look, I found the fault object in this series is
> > seemingly quite generic at the uAPI level: its naming/structure,
> > and the way how it's allocated and passed to hwpt, despite being
> > highly correlated with IOPF in its fops code. So, I feel that we
> > might have a chance of reusing it for different fault types:
> >
> > +enum iommu_fault_type {
> > + IOMMU_FAULT_TYPE_HWPT_IOPF,
> > + IOMMU_FAULT_TYPE_VIOMMU_IRQ,
> > +};
> >
> > struct iommu_fault_alloc {
> > __u32 size;
> > __u32 flags;
> > + __u32 type; /* enum iommu_fault_type */
> > __u32 out_fault_id;
> > __u32 out_fault_fd;
> > };
>
> I think I would just add the type at the end of the struct and rely on
> our existing 0 is backwards compat mechanism. 0 means HWPT_IOPF. ie no
> need to do anything now.
Yea, I figured that it would work too, so let's add one in the
VIOMMU series (if we eventually decide to reuse the same ioctl).
> It would make some sense to call this a "report" object than "fault"
> if we are going to use it for different things. We could probably
> rename it without much trouble. There is also not a significant issue
> with having two alloc commands for FDs.
Ack.
> I'd also think VIOMMU_IRQ is probably not that right abstraction,
> likely it makes more sense to push driver-specific event messages sort
> of like IOPF and one of the messages can indicate a arm-smmu-v3 VCDMQ
> interrupt, other messages could indicate BAD_CD and similar sorts of
> events we might want to capture and forward.
Maybe something like this?
struct iommu_viommu_event_arm_smmuv3 {
u64 evt[4];
};
struct iommu_viommu_event_tegra241_cmdqv {
u64 vcmdq_err_map[2];
};
enum iommu_event_type {
IOMMM_HWPT_EVENT_TYPE_IOPF,
IOMMU_VIOMMU_EVENT_TYPE_SMMUv3,
IOMMU_VIOMMU_EVENT_TYPE_TEGRA241_CMDQV,
};
struct iommu_event_alloc {
__u32 size;
__u32 flags;
__u32 out_event_id;
__u32 out_event_fd;
__u32 type;
__u32 _reserved;
};
It can be "report" if you prefer.
Thanks
Nicolin
