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

[Jia He]

Hi Eric

On 6/8/2018 1:06 AM, Auger Eric Wrote:
> 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: address@hidden
>> ---
>>  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.
> 
Ok with me

Cheers，
Jia