Hi Christoffer, On 04/26/2018 12:06 PM, Christoffer Dall wrote: > 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? I introduced the suggested additional helper, vgic_dist_overlap, to check a region does not overlap with the distributor region and used it in vgic_v3_insert_redist_region. However in vgic_v3_check_base I do not need it as I already use vgic_v3_rdist_overlap() which does the job, ie. check the dist against all registered redists. Thanks Eric > > Thanks, > -Christoffer >
