On Thu, Apr 26, 2018 at 10:29:35AM +0200, Auger Eric wrote: > Hi Christoffer, > On 04/24/2018 11:07 PM, Christoffer Dall wrote: > > On Fri, Apr 13, 2018 at 10:20:53AM +0200, Eric Auger wrote: > >> We introduce a new helper to check there is no overlap between > >> dist region (if set) and registered rdist regions. This both > >> handles the case of legacy single rdist region (implicitly sized > >> with the number of online vcpus) and the new case of multiple > >> explicitly sized rdist regions. > > > > I don't understand this change, really. Is this just a cleanup, or > > changing some functionality (why?). > > > > I think this could have come with the introduction of > > vgic_v3_rdist_overlap() before patch 6, and then patch 6 could have been > > simplified (hopefully) to just call this "check that nothing in the > > world ever collides withi itself" function. > I have merged this patch and vgic_v3_rd_region_size + > vgic_v3_rdist_overlap and put it before this patch. > > Also I reworked the commit message which was unclear I acknowledge. > > With respect to using the adapted vgic_v3_check_base() in > vgic_v3_insert_redist_region(), it is less obvious to me. > > In vgic_v3_insert_redist_region we do the checks *before* inserting the > new rdist region in the list of redist regions. While > vgic_v3_check_base() does the checks on already registered rdist and > dist regions. So I would be tempted to leave > vgic_v3_insert_redist_region() implementation as it is. > ok, but do see my suggestion there to factor out the check, which should make that function slightly easier to read. Then perhaps you can use that function from vgic_v3_check_base to check that each rdist doesn't overlap with the distributor? Thanks, -Christoffer
