Hi Mostafa,
On Tue, Jul 09, 2024 at 07:12:59AM +0000, Mostafa Saleh wrote:
> > In this case I think we're reporting InputAddr as the CD address, but it
> > should be the IOVA
>
> As Eric mentioned this would require some rework to propagate the iova,
> but what I am more worried about is the readability in that case, may be we
> can just fixup the event after smmuv3_get_config() in case of errors,
> something like:
>
> /*
> * smmuv3_get_config() Only return translation faults in case of
> * nested translation, otherwise it can only return C_BAD_CD,
> * C_BAD_STE, C_BAD_STREAMID or F_STE_FETCH.
> * But in case of translation fault, we need to fixup the
> * InputAddr to be the IOVA of the translation as the decode
> * functions don't know about it.
> */
> static void smmuv3_config_fixup_event(SMMUEventInfo *event, hwaddr iova)
> {
> switch (event->type) {
> case SMMU_EVT_F_WALK_EABT:
> case SMMU_EVT_F_TRANSLATION:
> case SMMU_EVT_F_ADDR_SIZE:
> case SMMU_EVT_F_ACCESS:
> case SMMU_EVT_F_PERMISSION:
> event->u.f_walk_eabt.addr = iova;
> break;
> default:
> break;
> }
> }
>
> What do you think?
Yes, I think that's also what I came up with. Maybe it would be simpler to
unconditionally do the fixup at the end of smmuv3_translate() and remove
.addr write from smmuv3_do_translate()?
A separate union field "f_common" rather than f_walk_eabt may be clearer.
Thanks,
Jean
