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
