Re: [PATCH v6 13/14] iommu/arm-smmu-v3: Report events that belong to devices attached to vIOMMU

[Nicolin Chen <nicolinc@nvidia.com>]

On Tue, Feb 18, 2025 at 02:50:46PM -0400, Jason Gunthorpe wrote:
> On Tue, Feb 18, 2025 at 10:28:04AM -0800, Nicolin Chen wrote:
> > On Tue, Feb 18, 2025 at 01:18:21PM -0400, Jason Gunthorpe wrote:
> > > On Fri, Jan 24, 2025 at 04:30:42PM -0800, Nicolin Chen wrote:
> > > 
> > > > @@ -1831,31 +1831,30 @@ static int arm_smmu_handle_event(struct arm_smmu_device *smmu,
> > > >  		return -EOPNOTSUPP;
> > > >  	}
> > > 
> > > There is still the filter at the top:
> > > 
> > > 	switch (event->id) {
> > > 	case EVT_ID_TRANSLATION_FAULT:
> > > 	case EVT_ID_ADDR_SIZE_FAULT:
> > > 	case EVT_ID_ACCESS_FAULT:
> > > 	case EVT_ID_PERMISSION_FAULT:
> > > 		break;
> > > 	default:
> > > 		return -EOPNOTSUPP;
> > > 	}
> > > 
> > > Is that right here or should more event types be forwarded to the
> > > guest?
> > 
> > That doesn't seem to be right. Something like EVT_ID_BAD_CD_CONFIG
> > should be forwarded too. I will go through the list.
> 
> I think the above should decode into a 'faultable' path because they
> all decode to something with an IOVA
> 
> The rest should decode to things that include a SID and the SID decode
> should always be forwarded to the VM. Maybe there are small
> exclusions, but generally that is how I would see it..

I think we are safe to add these:

------------------------------------------------------------
diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
index fd2f13a63f27..be9746ecdc65 100644
--- a/include/uapi/linux/iommufd.h
+++ b/include/uapi/linux/iommufd.h
@@ -1067,7 +1067,16 @@ enum iommu_veventq_type {
  * struct iommu_vevent_arm_smmuv3 - ARM SMMUv3 Virtual Event
  *                                  (IOMMU_VEVENTQ_TYPE_ARM_SMMUV3)
  * @evt: 256-bit ARM SMMUv3 Event record, little-endian.
- *       (Refer to "7.3 Event records" in SMMUv3 HW Spec)
+ *       Reported event records: (Refer to "7.3 Event records" in SMMUv3 HW Spec)
+ *       - 0x02 C_BAD_STREAMID
+ *       - 0x04 C_BAD_STE
+ *       - 0x06 F_STREAM_DISABLED
+ *       - 0x08 C_BAD_SUBSTREAMID
+ *       - 0x0a C_BAD_STE
+ *       - 0x10 F_TRANSLATION
+ *       - 0x11 F_ADDR_SIZE
+ *       - 0x12 F_ACCESS
+ *       - 0x13 F_PERMISSION
  *
  * StreamID field reports a virtual device ID. To receive a virtual event for a
  * device, a vDEVICE must be allocated via IOMMU_VDEVICE_ALLOC.
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 0efda55ad6bd..f3aa9ce16058 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1827,7 +1827,15 @@ static int arm_smmu_handle_event(struct arm_smmu_device *smmu, u64 *evt,
        case EVT_ID_ADDR_SIZE_FAULT:
        case EVT_ID_ACCESS_FAULT:
        case EVT_ID_PERMISSION_FAULT:
+       case EVT_ID_BAD_CD_CONFIG:
+       case EVT_ID_BAD_STE_CONFIG:
+       case EVT_ID_BAD_STREAMID_CONFIG:
+       case EVT_ID_BAD_SUBSTREAMID_CONFIG:
+       case EVT_ID_STREAM_DISABLED_FAULT:
                break;
+       case EVT_ID_STE_FETCH_FAULT:
+       case EVT_ID_CD_FETCH_FAULT:
+               /* FIXME need to replace fetch_addr with IPA? */
        default:
                return -EOPNOTSUPP;
        }
------------------------------------------------------------

All of the supported events require vSID replacement. Those faults
with addresses are dealing with stage-1 IOVA or IPA, i.e. IOVA and
PA for a VM. So, they could be simply forwarded.

But F_CD_FETCH and F_STE_FETCH seem to be complicated here, as both
report PA in their FetchAddr fields, although the spec does mention
both might be injected to a guest VM:
 - "Note: This event might be injected into a guest VM, as though
    from a virtual SMMU, when a hypervisor receives a stage 2
    Translation-related fault indicating CD fetch as a cause (with
    CLASS == CD)."
 - "Note: This event might be injected into a guest VM, as though
    from a virtual SMMU, when a hypervisor detects invalid guest
    configuration that would cause a guest STE fetch from an illegal
    IPA."

For F_CD_FETCH, at least the CD table pointer in the nested STE is
an IPA, and all the entries in the CD table that can be 2-level are
IPAs as well. So, we need some kinda reverse translation from a PA
to IPA using its stage-2 mapping. I am not sure what's the best way
to do that...

For F_STE_FETCH, the host prepared the nested STE, so there is no
IPA involved. We would have to ask VMM to fill the field since an
STE IPA should be just a piece of entry given the vSID. One thing
that I am not sure is whether the FetchAddr is STE-size aligned or
not, though we can carry the offset in the FetchAddr field via the
vEVENT by masking away any upper bits...

I wonder if @Robin or @Will may also shed some light on these two
events.

Otherwise, perhaps not-supporting them in this series might be a
safer bet?

Thanks
Nicolin