From mboxrd@z Thu Jan  1 00:00:00 1970
From: Julien Grall <julien.grall@linaro.org>
Subject: Re: [RFC PATCH v1 01/10] xen/arm: make secondary gic
 init as notifier call
Date: Wed, 26 Mar 2014 14:41:20 +0000
Message-ID: <5332E710.1060105@linaro.org>
References: <1395238631-2024-1-git-send-email-vijay.kilari@gmail.com>
	<1395238631-2024-2-git-send-email-vijay.kilari@gmail.com>
	<532AE392.1080107@linaro.org>
	<CALicx6spfuZw7fXfkVFRjrbh2ikwxr85hJ6HgdrvZ=mcTGG2aQ@mail.gmail.com>
	<532EF1DC.6000203@linaro.org>
	<CALicx6uiTw=hyQdZdEbQPssnPa40b8RPQct2VNGfXT6_crex8w@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <CALicx6uiTw=hyQdZdEbQPssnPa40b8RPQct2VNGfXT6_crex8w@mail.gmail.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Vijay Kilari <vijay.kilari@gmail.com>
Cc: Ian Campbell <Ian.Campbell@citrix.com>, Stefano Stabellini <stefano.stabellini@eu.citrix.com>, Prasun Kapoor <Prasun.Kapoor@caviumnetworks.com>, Vijaya Kumar K <vijaya.kumar@caviumnetworks.com>, xen-devel@lists.xen.org, George Dunlap <george.dunlap@eu.citrix.com>, Stefano Stabellini <stefano.stabellini@citrix.com>
List-Id: xen-devel@lists.xenproject.org

Hello Vijay,

(Adding George for the scheduler part)

On 03/26/2014 11:27 AM, Vijay Kilari wrote:
> Hi Julien,
> 
>>>>> @@ -283,7 +283,7 @@ void __cpuinit start_secondary(unsigned long
>>>>> boot_phys_offset,
>>>>>
>>>>>       mmu_init_secondary_cpu();
>>>>>
>>>>> -    gic_init_secondary_cpu();
>>>>> +    notify_cpu_starting(cpuid);
>>>>
>>>>
>>>> Can you explain why it's safe to move notify_cpu_starting earlier?
>>>>
>>>     When gic registers a notifier with action as CPU_STARTING, I am
>>> getting a panic
>>>     from gic driver because notify_cpu_starting() is called after most of
>>> the GIC
>>>     initialization calls as below from start_secondary() are called.
>>> Especially the issue is coming
>>>     with init_maintenanc_interrupt(). So I have moved this notifier
>>> before other GIC initialization
>>>     calls and since I move notifier only before GIC initialization
>>> calls it not be a problem.
>>
>>
>> It doesn't explain why it's safe... CPU_STARTING is also used in some place
>> to initialize internal data structure. Are you sure that theses callbacks
>> can be called earlier?
>>
> 
> The below callback is the only callback that is using CPU_STARTING,
> IMO it is only initializing pcpu data.
> 
> static int cpu_credit2_callback(
>     struct notifier_block *nfb, unsigned long action, void *hcpu)
> {
>     unsigned int cpu = (unsigned long)hcpu;
>     int rc = 0;
> 
>     switch ( action )
>     {
>     case CPU_STARTING:
>         csched_cpu_starting(cpu);
>         break;
>     default:
>         break;
>     }
> 
>     return !rc ? NOTIFY_DONE : notifier_from_errno(rc);
> }
> 
> With this patch, notifier is only called just before GIC initialization.
> So should not be a problem. Let me know your opinion? I am
> not very familiar with this piece of code.

I think, the sched credit2 initialization code is relying on the sibling
map (see cpu_to_socket), which is basically a no-op for now on ARM.

You may have to also move setup_cpu_sibling_map(cpuid) earlier.

I don't know enough the scheduler to say it's safe to move earlier.
George any opinion?

The second issue I can see is, a developer wants to add a CPU_STARTING
callback for his shiny driver which will request a PPI. In this case we
will fail because the IRQ desc is not correctly initialized
(init_secondary_IRQ is called after notify_cpu_starting).

Regards,

-- 
Julien Grall