From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v2 08/15] xen/arm: vgic-v3: Emulate correctly the re-distributor Date: Mon, 02 Feb 2015 16:33:37 +0000 Message-ID: <54CFA6E1.2010404@linaro.org> 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> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YIJwt-000761-2r for xen-devel@lists.xenproject.org; Mon, 02 Feb 2015 16:34:07 +0000 Received: by mail-wi0-f179.google.com with SMTP id l15so16287965wiw.0 for ; Mon, 02 Feb 2015 08:34:05 -0800 (PST) In-Reply-To: <1422892751.5838.20.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: xen-devel@lists.xenproject.org, Vijaya.Kumar@caviumnetworks.com, stefano.stabellini@citrix.com, tim@xen.org List-Id: xen-devel@lists.xenproject.org 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. AFAICT, we have a similar limitation for the memory region. Although I could sort them when we build DOM0. >> + >> + /* >> + * Note: We are assuming that d->vcpu[vcpu_id] is never NULL. If >> + * it's the case, the guest will receive a data abort and won't be >> + * able to boot. > > Is cpu hotplug a factor here? Do we support guests booting with offline > cpus yet? The "problem" is not because of CPU hotpluging. XEN_DOMCTL_max_vcpus doesn't allow the change of the number of vCPUs. AFAICT, this won't be supported (see comments within the code). But, the domctl may not set a vCPU if it's fail to initialize it. So in theory it would be possible to have d->vcpu[vcpu_id] == NULL. But in practice, the toolstack won't continue if one of the VCPU has not been allocated. I felt it was good to mention it for documentation purpose. > >> + /* >> + * Safety check mostly for DOM0. It's possible to have more vCPU >> + * than the number of physical CPU. Maybe we should deny this case? > > In general it's allowed, if a bit silly. Mainly for e.g. people working > on PV spinlock code who want to force contention... Unlikely for dom0 I > suppose. We don't control the DOM0 memory layout, so in practice we can't have more vCPUs than allowed by the hardware. This is because every re-distributors have it's own MMIO region. It's different for guests as we control the memory layout. Regards, -- Julien Grall