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()". Cheers, Andre.
