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. >
