Re: [Qemu-arm] [RFC 8/8] hw/arm/virt: Allow up to 512 vcpus along with KVM VGICv3

[Peter Maydell]

On 27 March 2018 at 15:15, Eric Auger <address@hidden> wrote:
> With KVM acceleration and if KVM VGICV3 supports to set multiple
> redistributor regions, we now allow up to 512 vcpus.
>
> Signed-off-by: Eric Auger <address@hidden>
> ---
>  hw/arm/virt.c         | 17 ++++++++++++++++-
>  include/hw/arm/virt.h |  1 +
>  2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 8258f6f..cdb1a75 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -149,8 +149,10 @@ static const MemMapEntry a15memmap[] = {
>      [VIRT_PCIE_PIO] =           { 0x3eff0000, 0x00010000 },
>      [VIRT_PCIE_ECAM] =          { 0x3f000000, 0x01000000 },
>      [VIRT_MEM] =                { 0x40000000, RAMLIMIT_BYTES },
> +    /* Allows 512 - 123 additional vcpus (each 2x64kB) */
> +    [VIRT_GIC_REDIST2] =        { 0x4000000000ULL, 0x30A0000LL },

Maybe we should make the 2nd redist region go up to a slightly
rounder address rather than making it exactly big enough to get
us up to 512 CPUs? (Why 512, by the way?)

>      /* Second PCIe window, 512GB wide at the 512GB boundary */
> -    [VIRT_PCIE_MMIO_HIGH] =   { 0x8000000000ULL, 0x8000000000ULL },
> +    [VIRT_PCIE_MMIO_HIGH] =     { 0x8000000000ULL, 0x8000000000ULL },
>  };
>
>  static const int a15irqmap[] = {
> @@ -553,6 +555,11 @@ static void create_gic(VirtMachineState *vms, qemu_irq 
> *pic)
>          agcc->register_redist_region((GICv3State *)gicdev,
>                                   vms->memmap[VIRT_GIC_REDIST].base,
>                                   vms->memmap[VIRT_GIC_REDIST].size >> 17);
> +        if (vms->smp_cpus > 123) {
> +            agcc->register_redist_region((GICv3State *)gicdev,
> +                     vms->memmap[VIRT_GIC_REDIST2].base,
> +                                 vms->memmap[VIRT_GIC_REDIST2].size >> 17);
> +        }
>      } else {
>          sysbus_mmio_map(gicbusdev, 1, vms->memmap[VIRT_GIC_CPU].base);
>      }
> @@ -1284,6 +1291,14 @@ static void machvirt_init(MachineState *machine)
>       */
>      if (vms->gic_version == 3) {
>          virt_max_cpus = vms->memmap[VIRT_GIC_REDIST].size / 0x20000;
> +        if (kvm_max_vcpus(kvm_state) > 255) {
> +            /*
> +             * VGICv3 KVM device capability to set multiple redistributor
> +             * was introduced at the same time KVM_MAX_VCPUS was bumped
> +             * from 255 to 512
> +             */
> +            virt_max_cpus += vms->memmap[VIRT_GIC_REDIST2].size / 0x20000;

I think it would be better to explicitly check "do we have
support for split redistributors" rather than looking at
KVM_MAX_VCPUS. It's not impossible that a distro kernel
might have chosen to backport support for one but not the
other.

> +        }
>      } else {
>          virt_max_cpus = GIC_NCPU;
>      }
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index d168291..801a4ad 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -60,6 +60,7 @@ enum {
>      VIRT_GIC_V2M,
>      VIRT_GIC_ITS,
>      VIRT_GIC_REDIST,
> +    VIRT_GIC_REDIST2,
>      VIRT_UART,
>      VIRT_MMIO,
>      VIRT_RTC,
> --
> 2.5.5

thanks
-- PMM