Re: [PATCH v9 13/25] iommu/arm-smmu-v3-kvm: Add MMIO emulation

[Mostafa Saleh]

On Thu, Nov 05, 2026 at 03:04:52PM -0800, Nicolin Chen wrote:
> On Thu, Oct 15, 2026 at 08:08:44PM +0000, Mostafa Saleh wrote:
> > +static bool smmu_dabt_device(struct hyp_arm_smmu_v3_device *smmu,
> > +			     struct user_pt_regs *regs,
> > +			     u64 esr, u32 off)
> > +{
> > +	bool is_write = esr & ESR_ELx_WNR;
> > +	unsigned int len = BIT((esr & ESR_ELx_SAS) >> ESR_ELx_SAS_SHIFT);
> > +	int rd = (esr & ESR_ELx_SRT_MASK) >> ESR_ELx_SRT_SHIFT;
> > +	const u64 read_write = -1ULL;
> > +	const u64 no_access = 0;
> > +	u64 mask = no_access;
> > +	const u64 read_only = is_write ? no_access : read_write;
> > +	bool is_xzr = (rd == 31);
> > +	u64 val = is_xzr ? 0 : regs->regs[rd];
> > +
> > +	switch (off) {
> > +	case ARM_SMMU_IDR0:
> > +		if (len != sizeof(u32))
> > +			break;
> 
> Maybe add a small helper to list all 64-bit registers:
> 
> static unsigned int smmu_reg_width(u32 off)
> {
>         switch (off) {
>         case ARM_SMMU_CMDQ_BASE:
>         case ARM_SMMU_STRTAB_BASE:
>         case ARM_SMMU_EVTQ_BASE:
>         case ARM_SMMU_PRIQ_BASE:
>         case ARM_SMMU_EVTQ_IRQ_CFG0:
>         case ARM_SMMU_PRIQ_IRQ_CFG0:
>         case ARM_SMMU_GERROR_IRQ_CFG0:
>                 return sizeof(u64);
>         default:
>                 return sizeof(u32);
>         }
> }
> 
> Then it'd be tighter by testing the width once, prior to switch (off)?

Makes sense, I will try that.

> 
> > +	/* Allowed RO 32 bit registers. */
> > +	case ARM_SMMU_IIDR:
> > +	case ARM_SMMU_IDR5:
> > +	case ARM_SMMU_IDR3:
> > +	case ARM_SMMU_IDR1:
> > +	case ARM_SMMU_IRQ_CTRLACK:
> > +	case ARM_SMMU_GERROR:
> > +		if (len != sizeof(u32))
> > +			break;
> > +		mask = read_only;
> > +		break;
> > +	}
> > +
> > +	if (WARN_ON(!mask))
> > +		goto out_ret;
> 
> Your cover letter mentions "WARNs are fatal in the hypervisor".
> 
> Is this WARN_ON fatal?

Yes. This is reached if the host does something not emulated:
1) A bug (writing to a RO register, writing STRTAB if SMMU is
   enabled...)
2) New arch feature not supported.

In that case, I'd rather panic here and fix the driver instead of
ignoring it which will manifest in a harder to debug symptoms later.

Also, this driver only handles the Linux host kernel (which must
be built-in in this config) so we can assume how the emulated driver
works and what features it use.

Thanks,
Mostafa

> 
> Nicolin