Re: [Qemu-devel] [PATCH] hw/arm/smmuv3: fix smmu emulation when guest smmu is in passthrough mode

[Auger Eric <eric.auger@redhat.com>]

Hi Jia,

On 06/07/2018 03:38 PM, Jia He wrote:
> There is an exception when I passes iommu.passthrough=1 to guest's
> kernel boot parameter(host QDF2400 kernel 4.17, guest kernel 4.14).
> The guest will be hang when booting up.
> 
> When guest smmu is in passthrough mode, entry.perm will not be assigned
> to flag in smmuv3_translate. It seems not be correct.
> 
> After this patch, I have tested in 4 cases and all passed.
> host smmu on/passthrough + guest smmu on/passthrough
> 
> Signed-off-by: jia.he@hxt-semitech.com
> ---
>  hw/arm/smmuv3.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 42dc521..5c46102 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -560,6 +560,12 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
>      }
>  
>      ret = smmuv3_decode_config(mr, &cfg, &event);
> +
> +    if (cfg.bypassed) {
> +        ret = 0;
> +        goto out;
> +    }
Thank you for spotting this bug. Yes you're correct: on bypass we
effectively need to set the IOMMUTLBEntry perm flags.

Reading the code again I think the error handling logic is really
confusing and if you don't mind, I suggest I submit a global fix. On
bypass we should rather have a bypass trace event issued instead of the
translated one. To me the aborted case is not properly handled either as
we are going to record an event whereas we shouldn't. In case of
abort/bypass translation structure decoding should rather return 0 I
think instead of returning errors. Please let me know if it suits you.

Thanks

Eric
> +
>      if (ret) {
>          goto out;
>      }
>