On Mon, Jul 14, 2025 at 04:59:40PM +0100, Shameer Kolothum wrote:
> From: Nicolin Chen <nicol...@nvidia.com>
>
> Not all fields in the SMMU IDR registers are meaningful for userspace.
> Only the following fields can be used:
>
> - IDR0: ST_LEVEL, TERM_MODEL, STALL_MODEL, TTENDIAN, CD2L, ASID16, TTF
> - IDR1: SIDSIZE, SSIDSIZE
> - IDR3: BBML, RIL
> - IDR5: VAX, GRAN64K, GRAN16K, GRAN4K
But half of these fields are not validated in the patch :-/
My vSMMU didn't work until I added entries like SIDSIZE, SSIDSIZE,
TERM_MODEL, STALL_MODEL, and RIL.
I think IDR5.OAS should be also added in the list. Maybe we should
update the kernel uAPI meanwhile.
> + val = FIELD_EX32(s_accel->info.idr[5], IDR5, GRAN4K);
> + if (val < FIELD_EX32(s->idr[5], IDR5, GRAN4K)) {
> + s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN4K, val);
> + }
> + val = FIELD_EX32(s_accel->info.idr[5], IDR5, GRAN16K);
> + if (val < FIELD_EX32(s->idr[5], IDR5, GRAN16K)) {
> + s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN16K, val);
> + }
> + val = FIELD_EX32(s_accel->info.idr[5], IDR5, GRAN64K);
> + if (val < FIELD_EX32(s->idr[5], IDR5, GRAN64K)) {
> + s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN64K, val);
Unless there is some conflicts between the QEMU emulation and the
SMMU HW, I think we should probably just override these fields to
the HW values, instead of running comparisons. The justification
could be that these fields are unlikely going to be controlled by
the QEMU but supported directly by the real HW.
For example, if HW supports SSIDSIZE=5, there seems to be no good
reason to limit it to SSIDSIZE=4? Even if the default SSIDSIZE in
the smmuv3_init_regs() is 4.
> @@ -1903,6 +1904,9 @@ static void smmu_reset_exit(Object *obj, ResetType type)
> }
>
> smmuv3_init_regs(s);
> + if (sys->accel) {
> + smmuv3_accel_init_regs(s);
> + }
I feel that we should likely do an if-else instead, i.e.
if (sys->accel) {
smmuv3_accel_init_regs(s);
} else {
smmuv3_init_regs(s);
}
The smmuv3_init_regs() enables certain bits that really should be
set by the returned IDRs from hw_info in smmuv3_accel_init_regs().
Doing an overriding call can potentially give us some trouble in
the future if there are new bits being introduced and enabled in
smmuv3_init_regs() but missed in smmuv3_accel_init_regs().
So, it can be simpler in the long run if smmuv3_accel_init_regs()
initializes in its own way, IMHO.
Thanks
Nicolin
