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

[Shlomo Pongratz]

Thanks

Please see inline

Best regards,

S.P.

On Thursday, September 17, 2015, Peter Maydell <address@hidden> wrote:
> On 17 September 2015 at 18:38, Shlomo Pongratz <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.
> > This is because 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.
> >
> > Things left to do:
> >
> > Support SPI, note that this patch porpose is to enable running 64 cores using
> > the "virt" virtual machine.
> >
> > Add support for 128 cores. This requires the usage
> > of bitops which requires a major rewrite.
> >
> > Special thanks to Peter Crostwaite whose patch to th Linux (kernel) i.e.
> > Implement cpu_relax as yield solved the problem of the boot process getting
> > stuck for 24 cores and more.
>
> This still seems to have the same issues as were noted in previous
> rounds:
>  * you need to get rid of the limitation on number of cores. There
>    should be no hard limit imposed by the GIC emulation code: not
>    64, not 128, not anything.

 

The GICv3 spec limits the number of cores to 128. I assume you don't expect GICv2 to support more than 8 cores.

As I wrote since the largest "atomic" type is 64 bit supporting more than 64 cores requires major rewrite using bitops. This can be done but I think this can wait for future versions.

I wonder how can I remove the limit and not crash is someone tries an arbitrary large number.

How can I protect from wrong usage?

>  * patch 2 is over 2000 lines, which is far too big to review. It
>    needs to be split up. Aim for 200 lines per patch at maximum;
>    smaller is better.

 

I'm open to suggestions. This is actually a single file (with the necessary Makefile addition). Do you suggest that I'll break it for example to distributer part an redistributer part e.t.c.?

Please advice.

> Also, patch 4 looks like it's an older version of one of Pavel's;
> you probably want to rebase on top of Pavel's recent v14 series,
> which is nearly ready to go into master.

Patch #4 is based on Pavel's V14 5/5 patch but adds GICv3 to it. 

> thanks
> -- PMM