[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time


  • To: Luca Fancellu <Luca.Fancellu@xxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Fri, 11 Mar 2022 13:28:00 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=S5xjwBOe4g63+TsspZ5O0F0zu3Yh/XSsI5xQ0WkylhA=; b=h1bmc5gDG2ryZQzPJzhVWofGHJPZSjQALfnnzL+J2Nl0PwLmHTh0yruLJdkiXxQ69DNKWziEbde/WyuRgKmILM7eYZtLEkywm6XbfVhuKgKY8xi7CgksUoxAvY+MEqQJTI1M7jIWF10wIrpW97ygLgUYAQjvh589I3swrT5Vti2Sa3tKNgsqGxS/1hg88vAGNsHHFTttE77zb1zj7CPWRNxdhG8W84PX9gFYcEeB0JgOhk/iG3aTMjgtlTuZYiPoD2hwsxDHw7F/lElnr0JQLFBfTlYZuecwhcIvkLbQBawoqf6HWbW7amkOZ7Itgx45tWxF3TWXB9BVELziljzmMw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=QEVaqHMJtWmOsRObqNmUiXKezPs/edKfDbKw2EDuWjvmp3GeH0bnlnynVgid3+0WBKKx+KtD4sLyPnqdc8QOuR//siCT0ulXWhVD6TnrPyzHN+h5HjEeA4D4SS37y1fZyEO4UjJiumdO6wGMAInRgIkgF5zAJXkQqsfD7+TwfOpdjLfPIwtHLu9tE9LvLpgiOjaJmw33+3g/TZg0LHKtZGIHAWEQsg0UcMenz39lz8itsBp2dYUkRT7BedOVBSH3DXYWNpqxpGf/oWhpbYHM2n1Htu6D2RaAB0gove0VJJQm9wnlkzTvVU3E/czStGUhwP5+kQlPNdVJ4rQ/IPdS/g==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Wei Chen <Wei.Chen@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Bertrand Marquis <Bertrand.Marquis@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Dario Faggioli <dfaggioli@xxxxxxxx>, Juergen Gross <jgross@xxxxxxxx>
  • Delivery-date: Fri, 11 Mar 2022 12:28:18 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 11.03.2022 12:29, Luca Fancellu wrote:
>> On 11 Mar 2022, at 10:18, Juergen Gross <jgross@xxxxxxxx> wrote:
>> On 11.03.22 10:46, Jan Beulich wrote:
>>> On 11.03.2022 10:29, Juergen Gross wrote:
>>>> On 11.03.22 09:56, Luca Fancellu wrote:
>>>>>> On 11 Mar 2022, at 08:09, Juergen Gross <jgross@xxxxxxxx> wrote:
>>>>>> On 10.03.22 18:10, Luca Fancellu wrote:
>>>>>>> --- /dev/null
>>>>>>> +++ b/xen/common/boot_cpupools.c
>>>>>>> @@ -0,0 +1,212 @@
>>>>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>>>>> +/*
>>>>>>> + * xen/common/boot_cpupools.c
>>>>>>> + *
>>>>>>> + * Code to create cpupools at boot time for arm architecture.
>>>>>>
>>>>>> Please drop the arm reference here.
>>>>>>
>>>>>>> + *
>>>>>>> + * Copyright (C) 2022 Arm Ltd.
>>>>>>> + */
>>>>>>> +
>>>>>>> +#include <xen/sched.h>
>>>>>>> +
>>>>>>> +#define BTCPUPOOLS_DT_NODE_NO_REG     (-1)
>>>>>>> +#define BTCPUPOOLS_DT_NODE_NO_LOG_CPU (-2)
>>>>>>
>>>>>> Move those inside the #ifdef below, please
>>>>>>
>>>>>>> +
>>>>>>> +struct pool_map {
>>>>>>> +    int pool_id;
>>>>>>> +    int sched_id;
>>>>>>> +    struct cpupool *pool;
>>>>>>> +};
>>>>>>> +
>>>>>>> +static struct pool_map __initdata pool_cpu_map[NR_CPUS] =
>>>>>>> +    { [0 ... NR_CPUS-1] = {.pool_id = -1, .sched_id = -1, .pool = 
>>>>>>> NULL} };
>>>>>>> +static unsigned int __initdata next_pool_id;
>>>>>>> +
>>>>>>> +#ifdef CONFIG_ARM
>>>>>>
>>>>>> Shouldn't this be CONFIG_HAS_DEVICE_TREE?
>>>>>
>>>>> Yes, the only problem is that in get_logical_cpu_from_hw_id I use the arm 
>>>>> specific
>>>>> cpu_logical_map(…), so what do you think it’s the better way here?
>>>>> Do you think I should have everything under CONFIG_HAS_DEVICE_TREE
>>>>> and get_logical_cpu_from_hw_id under CONFIG_ARM like in this way below?
>>>>
>>>> Hmm, what is the hwid used for on Arm? I guess this could be similar
>>>> to the x86 acpi-id?
>>> Since there's going to be only one of DT or ACPI, if anything this could
>>> be the APIC ID and then ...
>>>> So I'd rather put get_logical_cpu_from_hw_id() into Arm specific code
>>>> and add a related x86 function to x86 code. Depending on the answer to
>>>> above question this could either be get_cpu_id(), or maybe an identity
>>>> function.
>>> ... a lookup loop over x86_cpu_to_apicid[] (I thought we had a function
>>> doing so, but right now I can't find one).
>>
>> It is the second half of get_cpu_id().
> 
> I was going to say, maybe I can do something like this:
> 
> #ifdef CONFIG_ARM
> #define hwid_from_logical_cpu_id(x) cpu_logical_map(x)
> #elif defined(CONFIG_X86)
> #define hwid_from_logical_cpu_id(x) x86_cpu_to_apicid(x)
> #else
> #define hwid_from_logical_cpu_id(x) (-1)
> #end
> 
> static int __init get_logical_cpu_from_hw_id(unsigned int hwid)
> {
>     unsigned int i;
> 
>     for ( i = 0; i < nr_cpu_ids; i++ )
>         if ( hwid_from_logical_cpu_id(i) == hwid )
>             return i;
> 
>     return -1;
> }
> 
> Do you think it is acceptable?

Why not, if even on Arm you have to use a loop. As Jürgen said, this
likely wants to move to some header file. Whether the names are
suitable for an arch abstraction I'm not sure, but I also have no
immediate alternative suggestion.

> I see the current get_cpu_id(…) from x86 code is starting from an acpi id to
> lookup the apicid and then it is looking for the logical cpu number.
> In the x86 context, eventually, the reg property of a cpu node would hold an
> Acpi id or an apicid? I would have say the last one but I’m not sure now.

Without ACPI it can't sensibly be an ACPI ID. The most logical thing
to expect would be an APIC ID, but then it's all up to whoever specifies
what DT is to supply.

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.