Re: [PATCH v4 10/10] xen: add per-platform defaults for NR_CPUS

[Julien Grall <julien.grall@arm.com>]

Hi Stefano,

On 06/01/2018 09:53 PM, Stefano Stabellini wrote:
> On Fri, 1 Jun 2018, Julien Grall wrote:
>> Hi,
>> Sorry for the formatting. I am pretty sure you need to CC "THE REST" here.
>>
>> On Thu, 31 May 2018, 22:50 Stefano Stabellini, <sstabellini@kernel.org> wrote:
>>        Add specific per-platform defaults for NR_CPUS. Note that the order of
>>        the defaults matter: they need to go first, otherwise the generic
>>        defaults will be applied.
>>
>>        This is done so that Xen builds customized for a specific hardware
>>        platform can have the right NR_CPUS number.
>>
>>        Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
>>        CC: JBeulich@suse.com
>>        CC: andrew.cooper3@citrix.com
>>        ---
>>         xen/arch/Kconfig | 3 +++
>>         1 file changed, 3 insertions(+)
>>
>>        diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig
>>        index cf0acb7..d451eb8 100644
>>        --- a/xen/arch/Kconfig
>>        +++ b/xen/arch/Kconfig
>>        @@ -2,6 +2,9 @@
>>         config NR_CPUS
>>                int "Maximum number of physical CPUs"
>>                range 1 4095
>>        +       default "8" if ARM && RCAR3
>>        +       default "4" if ARM && QEMU
>>        +       default "4" if ARM && MPSOC
>>                default "256" if X86
>>                default "128" if ARM
>>                ---help---
>>
>>
>> But I am not sure how this will work as with this series you can select multiple platforms in on Kconfig. What will be the end
>> result?
> 
> The end result is the first default that applies. In this case, if
> RCAR3, QEMU and MPSOC are all selected, then NR_CPUS would be set to
> 8, which is actually what one would want if she is trying to build Xen
> for these three platforms, but it is not what one would want if she was
> trying to build a generic Xen config for distros.
> 
> This is not great.
> 
> The option list you suggested could help solve the NR_CPUS issue, see
> below.
> 
> 
>> Anyway, as I mention the way to go is a option list with only one possible choice.
> 
> I am OK with an option list, the issue that we cannot have an ALL option
> in the list that selects the other options as I wrote previously. For
> instance, the following is NOT allowed:
> 
> config ALL
> 	bool "All Platforms"
> 	select MPSOC

Well, what you describe is not my suggestion. I specifically mention two 
config per board (PLATFORM_FOO and FOO) to handle the recursive problem.

Also, as I said previously the purpose of ALL is not to select all the 
required drivers. But let the user select the drivers while still 
include all source in platforms/*.c.

The reason for that is if you impose to select all the board with the 
driver, there will be no way to disable them.

> 
> However, having CONFIG_ALL would solve the NR_CPUS problem because we
> could do:
> 
> config NR_CPUS
>      default "128" if ARM && ALL
> 	default "8" if ARM && RCAR3
> 	default "4" if ARM && QEMU
> 	default "4" if ARM && MPSOC
> 	default "128" if ARM
> 
> Which would give us exactly the default we want for NR_CPUS.
> 
> So I agree that we should turn platform support into an option list and
> we should also add an ALL option to solve the NR_CPUS problem.
> 
> If we do that, the remaining problem is how to implement ALL. I found a
> workaround for the kconfig issue described above. The following works
> correctly:
> 
> choice
> 	prompt "Platform Support"
> 	default ALL
> 	---help---
> 	Choose Xen support for different hardware platforms.
> 
>      config ALL
>      	bool "All Platforms"
>      	select MPSOC_PLATFORM
>      	select QEMU_PLATFORM
>      	select RCAR3_PLATFORM
>      	---help---
>      	Enable support for all platforms.
>      
>      config QEMU
>      	bool "QEMU aarch virt machine support"
>      	depends on ARM_64
>      	select QEMU_PLATFORM
>      	---help---
>      	Enable all the required drivers for QEMU aarch64 virt emulated
>      	machine.
>      
>      config RCAR3
>      	bool "Renesas RCar3 support"
>      	depends on ARM_64
>      	select RCAR3_PLATFORM
>      	---help---
>      	Enable all the required drivers for Renesas RCar3
>      
>      config MPSOC
>      	bool "Xilinx Ultrascale+ MPSoC support"
>      	depends on ARM_64
>      	select MPSOC_PLATFORM
>      	---help---
>      	Enable all the required drivers for Xilinx Ultrascale+ MPSoC
> endchoice
> 
> config QEMU_PLATFORM
> 	bool
> 	select GICV3
> 	select HAS_PL011
> 
> config RCAR3_PLATFORM
> 	bool
> 	select HAS_SCIF
> 
> config MPSOC_PLATFORM
> 	bool
> 	select HAS_CADENCE_UART
> 	select ARM_SMMU
> 
> 
> This looks like the best way forward and would solve all issues.

This is similar to what I suggested with a twist. With your solution it 
is not possible for a config selecting ALL to disable the unwanted 
drivers. Imagine someone that wants GICV2 but not GICV3.

What you want to do is:

choice
	prompt "Platform Support"
  	default ALL
  	---help---
  	Choose Xen support for different hardware platforms.

       config ALL
       bool "Custom"
       	select MPSOC_PLATFORM
       	select QEMU_PLATFORM
       	select RCAR3_PLATFORM
       	---help---
       	Support for any platform. The user can unselect/unselect drivers

      config QEMU
      	bool "QEMU aarch virt machine support"
       	depends on ARM_64
       	select QEMU_PLATFORM
	select GICV3
	select HAS_PL011
       	---help---
       	Enable all the required drivers for QEMU aarch64 virt emulated
       	machine.

       config RCAR3
      	bool "Renesas RCar3 support"
       	depends on ARM_64
       	select RCAR3_PLATFORM
	select HAS_SCIF
       	---help---
       	Enable all the required drivers for Renesas RCar3

      config MPSOC
       	bool "Xilinx Ultrascale+ MPSoC support"
       	depends on ARM_64
      	select MPSOC_PLATFORM
	select HAS_CADENCE_UART
	select ARM_SMMU
      	---help---
      	Enable all the required drivers for Xilinx Ultrascale+ MPSoC
endchoice

config QEMU_PLATFORM
  	bool

config RCAR3_PLATFORM
	bool
	select HAS_SCIF

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel