Re: [PATCH v4 08/19] hw/arm/smmuv3: Translate CD and TT using stage-2 table

[Jean-Philippe Brucker]

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