From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v2 08/15] xen/arm: vgic-v3: Emulate correctly the re-distributor Date: Mon, 2 Feb 2015 17:38:37 +0000 Message-ID: <1422898717.9323.5.camel@citrix.com> References: <1422555950-31821-1-git-send-email-julien.grall@linaro.org> <1422555950-31821-9-git-send-email-julien.grall@linaro.org> <1422892751.5838.20.camel@citrix.com> <54CFA6E1.2010404@linaro.org> <1422895674.5838.31.camel@citrix.com> <54CFAE55.6030209@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YIKxQ-0007vW-Dk for xen-devel@lists.xenproject.org; Mon, 02 Feb 2015 17:38:44 +0000 In-Reply-To: <54CFAE55.6030209@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall Cc: xen-devel@lists.xenproject.org, Vijaya.Kumar@caviumnetworks.com, stefano.stabellini@citrix.com, tim@xen.org List-Id: xen-devel@lists.xenproject.org On Mon, 2015-02-02 at 17:05 +0000, Julien Grall wrote: > On 02/02/15 16:47, Ian Campbell wrote: > > On Mon, 2015-02-02 at 16:33 +0000, Julien Grall wrote: > >> Hi Ian, > >> > >> On 02/02/15 15:59, Ian Campbell wrote: > >>> On Thu, 2015-01-29 at 18:25 +0000, Julien Grall wrote: > >>>> There is a one-to-one mapping between each re-distributors and processors. > >>>> Each re-distributors can be accessed by any processor at any time. For > >>>> instance during the initialization of the GIC, the drivers will browse > >>>> the re-distributor to find the one associated to the current processor > >>>> (via GICR_TYPER). So each re-distributor has its own MMIO region. > >>>> > >>>> The current implementation of the vGICv3 emulation assumes that the > >>>> re-distributor region is banked. Therefore, the processor won't be able > >>>> to access other re-distributor. While this is working fine for Linux, a > >>>> processor will only access GICR_TYPER to find the associated re-distributor, > >>>> we should have a correct implementation for the other operating system. > >>> > >>> You could state something stronger here, which is that it is wrong and > >>> should be fixed as a matter of principal. The fact that we happen to get > >>> away with it for some versions of Linux is an aside (although worth > >>> mentioning) > >> > >> I will rework the paragraph. > >> > >>>> > >>>> All emulated registers of the re-distributors take a vCPU in parameter > >>>> and necessary lock. Therefore concurrent access is already properly handled. > >>>> > >>>> The missing bit is retrieving the right vCPU following the region accessed. > >>>> Retrieving the right vCPU could be slow, so it has been divided in 2 paths: > >>>> - fast path: The current vCPU is accessing its own re-distributor > >>>> - slow path: The current vCPU is accessing an other re-distributor > >>> > >>> "another". > >>> > >>>> > >>>> As the processor needs to initialize itself, the former case is very > >>>> common. To handle the access quickly, the base address of the > >>>> re-distributor is computed and stored per-vCPU during the vCPU initialization. > >>>> > >>>> The latter is less common and more complicate to handle. The re-distributors > >>>> can be spread accross multiple regions in the memory. > >>> > >>> "across" > >>> > >>>> + /* > >>>> + * Find the region where the re-distributor lives. For this purpose, > >>>> + * we look one region ahead as only MMIO range for redistributors > >>>> + * traps here. > >>>> + * Note: We assume that the region are ordered. > >>> > >>> You could also check base+size vs gpa to avoid this limitation. > >> > >> IHMO, this limitation is harmless. This would happen for DOM0 if the > >> device tree doesn't sort the region. > > > > If it can happen then it isn't harmless, and it's easy to avoid so why > > not do so. > > The code is looking one region a-head to avoid check in the case there > is only one big re-distributors region. > > >> AFAICT, we have a similar limitation for the memory region. Although I > >> could sort them when we build DOM0. > > > > I'm not sure we do these days, but in any case two wrongs don't make a > > right. > > See consider_modules. We implicitly assume memory ordering. > > Anyway, if we do that, we should also check the overlapping between > regions, the size of the region is valid (i.e aligned to 64K and > correctly sized...),... I don't see how all that follows, certainly not at run time. Those are the sorts of things which should be checked at initialisation time and cause us to complain loudly. > The main drawbacks here would be DOM0 access the wrong vCPU or receive a > data abort. It's not too bad compare to "slowing down" the lookup. > If you really want to support non-ordered access, I could do at Domain > initialization. If we are at liberty to sort the list at init time then that would be fine. We probably ought to do the same with the memory regions > > I'd not be all that surprised to see systems with more rdist region > > space than they have physical processors e.g. sku's with different > > numbers of cores, but otherwise the same platform. > > So the current check would be enough? Even though, having more vCPUs > than physical CPUs for DOM0 sounds silly. > I expect so, yes.