[PATCH v3 18/19] KVM: arm64: ITS: Device table save/restore

[Auger Eric]

Hi,

On 24/03/2017 12:14, Andre Przywara wrote:
> Hi,
> 
> thanks for addressing the comments and for the fixes!
> 
> On 24/03/17 10:38, Auger Eric wrote:
>> Hi Andre,
>>
>> On 22/03/2017 15:39, Andre Przywara wrote:
>>> Hi,
>>>
>>> On 06/03/17 11:34, Eric Auger wrote:
>>>> This patch flushes the device table entries into guest RAM.
>>>> Both flat table and 2 stage tables are supported.  DeviceId
>>>> indexing is used.
>>>>
>>>> For each device listed in the device table, we also flush
>>>> the translation table using the vgic_its_flush/restore_itt
>>>> routines.
>>>>
>>>> On restore, devices are re-allocated and their itte are
>>>> re-built.
>>>
>>> Some minor things below.
>>> In general I had quite some trouble to understand what's going on here,
>>> though I convinced myself that this is correct. So could you add a bit
>>> more comments here? For instance to explain that we have to explicitly
>>> handle the L1 table on restore, but not on flush.
>>
>> On flush vgic_its_check_id does the 2 stage handling and computes the
>> entry address from the devid:
>>
>> 		if (!vgic_its_check_id(its, baser,
>> 				       dev->device_id, &eaddr))
>>
>> Then you simply flush the entry at that address
>>
>> On restore, you need to scan the level1 and level2 tables for valid entries.
> 
> That is exactly what I read from it, eventually - but only after staring
> at the code for quite a while. So can you put these explanations in some
> comments? An occasional reader would probably be puzzled to find the L1
> handling only in the restore, but not in the save path. It could be even
> shorter like: "Indirect table handling is covered by vgic_its_check_id()".
Yep sure I am going to comment that.

Thanks

Eric
> 
> Cheers,
> Andre.
>