Re: [Qemu-devel] [PATCH RFC] Implement GIC-500 from GICv3 family for arm64

[Claudio Fontana]

On 10.03.2015 10:50, Shannon Zhao wrote:
> On 2015/3/10 17:34, Shlomo Pongratz wrote:
>>
>> On 10 آذار, 2015 ص 03:18, Shannon Zhao wrote:
>>> On 2015/3/9 22:41, address@hidden wrote:
>>>> From: Shlomo Pongratz <address@hidden>
>>>>
>>>> This patch is a first step toward 128 cores support for arm64.
>>>>
>>>> At first only 64 cores are supported for two reasons:
>>>> First the largest integer type has the size of 64 bits and modifying
>>>> essential data structures in order to support 128 cores will require
>>>> the usage of bitops.
>>>> Second currently the Linux (kernel) can be configured to support
>>>> up to 64 cores thus there is no urgency with 128 cores support.
>>>>
>>>> Things left to do:
>>>>
>>>> Currently the booting Linux may got stuck. The probability of getting stuck
>>>> increases with the number of cores. I'll appreciate core review.
>>>>
>>>> There is a need to support flexible clusters size. The GIC-500 can support
>>>> up to 128 cores, up to 32 clusters and up to 8 cores is a cluster.
>>>> So for example, if one wishes to have 16 cores, the options are:
>>>> 2 clusters of 8 cores each, 4 clusters with 4 cores each
>>>> Currently only the first option is supported.
>>>> There is an issue of passing clock affinity to via the dtb. In the dtb
>>>>
>>>> interrupt section there are only 24 bit left to affinity since the
>>>> variable is a 32 bit entity and 8 bits are reserved for flags.
>>>> See Documentation/devicetree/bindings/arm/arch_timer.txt.
>>>> Note that this issue is not seems to be critical as when checking
>>>> /proc/irq/3/smp_affinity with 32 cores all 32 bits are one.
>>>>
>>>> The last issue is to add support for 128 cores. This requires the usage
>>>> of bitops and currently can be tested up to 64 cores.
>>>>
>>>> Signed-off-by: Shlomo Pongratz <address@hidden>
>>>> ---
>>>>   hw/arm/Makefile.objs               |    2 +-
>>>>   hw/arm/virtv2.c                    |  774 +++++++++++++++++
>>> Hi,
>>>
>>> I think here you want to introduce GICv3 in this patch. So is this 
>>> necessary to
>>> add a new virtv2 machine? And the codes of this machine mostly are same 
>>> with virt.
>>>
>>> Maybe we can add a parameter such as -GICv3 for machine virt to choose 
>>> GICv3 for it
>>> and choose GICv2 without this parameter. Then we can reuse more codes.
>>>
>> Hi Shannon,
>>
>> Using a parameter and configuring the virtual machine makes the core
>> unreadable.
>> There are to many if then...else statements.
> 
> Sorry, I don't think so. As we have implement GICv3 in qemu using a parameter 
> way,
> just about 10 if then...else statements are needed. The repeat codes are huge
> compared with those statements.
> 

As Shannon mentions it should be possible to at least try to do it without that 
amount of code duplication.
Should every change to the virt platform be reflected in two files every time 
anything minor has to be changed?
I don't think so. Lets try to put it in virt and see how the result looks like..

Btw, as Peter mentioned this patch needs to be split.

Ciao,

Claudio