Re: [PATCH v8 12/25] iommu/arm-smmu-v3-kvm: Probe SMMU HW

On Tue, Sep 22, 2026 at 06:43:44PM -0700, Nicolin Chen wrote:

On Tue, Sep 22, 2026 at 01:12:45PM +0000, Mostafa Saleh wrote:

quoted

+++ b/drivers/iommu/arm/arm-smmu-v3/pkvm/arm-smmu-v3-hyp.h

@@ -11,6 +11,9 @@
  *
  * Other members are filled and used at runtime by the SMMU driver.
  * @base		Virtual address of SMMU registers
+ * @oas			PA size
+ * @pgsize_bitmap	Supported page sizes
+ * @sid_bits		Max number of SID bits supported
  */

Some of them belongs to one of the prior patches.

And, if we are going to list them all, could we do in kdoc style?

I will fix it.

quoted

+++ b/drivers/iommu/arm/arm-smmu-v3/pkvm/arm-smmu-v3.c

@@ -29,6 +29,47 @@ static void smmu_deinit_device(struct hyp_arm_smmu_v3_device *smmu)
 	smmu->base = NULL;
 }
 
+/*
+ * Mini-probe and validation for the hypervisor.
+ */
+static int smmu_probe(struct hyp_arm_smmu_v3_device *smmu)
+{
+	u32 reg;
+
+	if (!(smmu->features & ARM_SMMU_FEAT_COHERENCY))
+		return -EINVAL;

Does this need to check IDR0.COHACC? If not, an inline note that
explains why?

This is similar to the kernel driver logic, where FW will override
coherency and in case of mismatch the driver will just WARN and trust
the FW.
pKVM can't print a warning as WARN is fatal in EL2. So it just picks
what the FW says. I am not sure what systems rely on this, but I can
add more strict checks and fail probe in case of mismatch with pKVM.

quoted

+
+	reg = arm_smmu_idr0_probe(smmu);
+
+	if (!(FIELD_GET(IDR0_TTF, reg) & IDR0_TTF_AARCH64))
+		return -ENXIO;
+
+	if (!(smmu->features & (ARM_SMMU_FEAT_TT_LE | ARM_SMMU_FEAT_TT_BE)))
+		return -ENXIO;
+
+	reg = readl_relaxed(smmu->base + ARM_SMMU_IDR1);
+	if (reg & (IDR1_TABLES_PRESET | IDR1_QUEUES_PRESET | IDR1_REL))
+		return -EINVAL;
+
+	smmu->sid_bits = FIELD_GET(IDR1_SIDSIZE, reg);
+	/* Follows the kernel logic */
+	if (smmu->sid_bits <= STRTAB_SPLIT)
+		smmu->features &= ~ARM_SMMU_FEAT_2_LVL_STRTAB;

The IDR1 reg guard and sid_bits can be shared with the main driver.

Maybe add an arm_smmu_device_idr1_probe()?

I thought about that, but this will be only used for sid_bits, the
hypervisor struct doesn't have ssid_bits as it doesn't use it, it
only shadows the STE and not the CD as the CD is translated by stage-2.

Other fields also not used by the hypervisor (PRIQS, EVTQS...)

As sashiko pointed, I will need to add ARM_SMMU_FEAT_ATTR_TYPES_OVR
check in the hypervisor, so maybe arm_smmu_device_idr1_probe() can be added
to partially probe the register and return the value so it can be probed the
main driver.

quoted

+
+	arm_smmu_idr3_probe(smmu);
+
+	arm_smmu_idr5_probe(smmu);
+	if (!smmu->oas)
+		smmu->oas = 48;

The truncation can be shared with the main driver too?

The main driver will print an error, if arm_smmu_idr5_probe() fix it
silently we will lose that.

quoted

@@ -43,8 +84,14 @@ static int smmu_init_device(struct hyp_arm_smmu_v3_device *smmu)
 		return ret;
 
 	smmu->base = (void __iomem *)haddr;
+	ret = smmu_probe(smmu);
+	if (ret)
+		goto out_ret;
 
 	return 0;
+out_ret:
+	smmu_deinit_device(smmu);

Nit: out_deinit

Sure.

Thanks,
Mostafa

Nicolin