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: jia...@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.
>
Ok with me
Cheers，
Jia
