Re: [Qemu-arm] [Qemu-devel] [PATCH 3/9] hw/intc/arm_gicv3: Introduce redist-region-count array property

[Andrew Jones]

On Thu, Jun 14, 2018 at 03:55:56PM +0200, Auger Eric wrote:
> Hi Drew,
> 
> On 06/14/2018 03:32 PM, Andrew Jones wrote:
> > On Wed, Jun 13, 2018 at 10:48:37AM +0200, Eric Auger wrote:
> >> To prepare for multiple redistributor regions, we introduce
> >> an array of uint32_t properties that stores the redistributor
> >> count of each redistributor region.
> >>
> >> Non accelerated VGICv3 only supports a single redistributor region.
> >> The capacity of all redist regions is checked against the number of
> >> vcpus.
> >>
> >> Machvirt is updated to set those properties, ie. a single
> >> redistributor region with count set to the number of vcpus
> >> capped by 123.
> >>
> >> Signed-off-by: Eric Auger <address@hidden>
> >>
> >> ---
> >> v2 -> v3:
> >> - add missing return in arm_gic_realize
> >> - in gicv3_init_irqs_and_mmio, compute/check rdist_capacity first
> >> - rdist region 0 size set to MIN(smp_cpus, redist0_capacity)
> >> - add GICV3_REDIST_SIZE
> >> ---
> >>  hw/arm/virt.c                      | 11 ++++++++++-
> >>  hw/intc/arm_gicv3.c                | 12 +++++++++++-
> >>  hw/intc/arm_gicv3_common.c         | 38 
> >> +++++++++++++++++++++++++++++++++-----
> >>  hw/intc/arm_gicv3_kvm.c            |  9 +++++++--
> >>  include/hw/intc/arm_gicv3_common.h |  8 ++++++--
> >>  5 files changed, 67 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> >> index f0a4fa0..2885d18 100644
> >> --- a/hw/arm/virt.c
> >> +++ b/hw/arm/virt.c
> >> @@ -522,6 +522,15 @@ static void create_gic(VirtMachineState *vms, 
> >> qemu_irq *pic)
> >>      if (!kvm_irqchip_in_kernel()) {
> >>          qdev_prop_set_bit(gicdev, "has-security-extensions", vms->secure);
> >>      }
> >> +
> >> +    if (type == 3) {
> >> +        uint32_t redist0_capacity =
> >> +                    vms->memmap[VIRT_GIC_REDIST].size / GICV3_REDIST_SIZE;
> >> +        uint32_t redist0_count = MIN(smp_cpus, redist0_capacity);
> >> +
> >> +        qdev_prop_set_uint32(gicdev, "len-redist-region-count", 1);
> > 
> > I don't see where this property is defined or used.
> 
> This is due to the fact we have an array property. nb_redist_regions is
> the storage of this value. len-redist-region-count is the name of the
> field (size of the array) from the user point of view.
> 
> DEFINE_PROP_ARRAY("redist-region-count", GICv3State, nb_redist_regions,
>                       redist_region_count, qdev_prop_uint32, uint32_t),
> 
> see len-db-voltage property in hw/arm/vexpress.c and
> hw/misc/arm_sysctl.c for a similar usage.

Thanks. I see '#define PROP_ARRAY_LEN_PREFIX "len-"' now too. That's just
the kind of magic that makes us all <blank> QEMU. Fill in <blank> as you
wish. I know what I'd fill it in with...

drew