Hi Drew,
On 06/14/2018 03:39 PM, Andrew Jones wrote:
> On Wed, Jun 13, 2018 at 10:48:38AM +0200, Eric Auger wrote:
>> Let's check if KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION is supported.
>> If not, we check the number of redist region is equal to 1 and use the
>> legacy KVM_VGIC_V3_ADDR_TYPE_REDIST attribute. Otherwise we use
>> the new attribute and allow to register multiple regions to the
>> KVM device.
>>
>> Signed-off-by: Eric Auger <eric.au...@redhat.com>
>> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
>>
>> ---
>>
>> v2 -> v3:
>> - In kvm_arm_gicv3_realize rename val into add_ormask local variable and
>> add a comment
>> - start the redist region registration from s->nb_redist_regions - 1
>> downwards
>> ---
>> hw/intc/arm_gicv3_kvm.c | 33 ++++++++++++++++++++++++++++++---
>> 1 file changed, 30 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
>> index ed7b719..52e6e70 100644
>> --- a/hw/intc/arm_gicv3_kvm.c
>> +++ b/hw/intc/arm_gicv3_kvm.c
>> @@ -753,6 +753,7 @@ static void kvm_arm_gicv3_realize(DeviceState *dev,
>> Error **errp)
>> {
>> GICv3State *s = KVM_ARM_GICV3(dev);
>> KVMARMGICv3Class *kgc = KVM_ARM_GICV3_GET_CLASS(s);
>> + bool multiple_redist_region_allowed;
>> Error *local_err = NULL;
>> int i;
>>
>> @@ -789,6 +790,18 @@ static void kvm_arm_gicv3_realize(DeviceState *dev,
>> Error **errp)
>> return;
>> }
>>
>> + multiple_redist_region_allowed =
>> + kvm_device_check_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
>> + KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION);
>> +
>> + if (!multiple_redist_region_allowed && s->nb_redist_regions > 1) {
>> + error_setg(errp, "Multiple VGICv3 redistributor regions are not "
>> + "supported by this host kernel");
>> + error_append_hint(errp, "A maximum of %d VCPUs can be used",
>> + s->redist_region_count[0]);
>> + return;
>> + }
>> +
>> kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_NR_IRQS,
>> 0, &s->num_irq, true, &error_abort);
>>
>> @@ -798,9 +811,23 @@ static void kvm_arm_gicv3_realize(DeviceState *dev,
>> Error **errp)
>>
>> kvm_arm_register_device(&s->iomem_dist, -1, KVM_DEV_ARM_VGIC_GRP_ADDR,
>> KVM_VGIC_V3_ADDR_TYPE_DIST, s->dev_fd, 0);
>> - kvm_arm_register_device(&s->iomem_redist[0], -1,
>> - KVM_DEV_ARM_VGIC_GRP_ADDR,
>> - KVM_VGIC_V3_ADDR_TYPE_REDIST, s->dev_fd, 0);
>> +
>> + if (!multiple_redist_region_allowed) {
>> + kvm_arm_register_device(&s->iomem_redist[0], -1,
>> + KVM_DEV_ARM_VGIC_GRP_ADDR,
>> + KVM_VGIC_V3_ADDR_TYPE_REDIST, s->dev_fd, 0);
>> + } else {
>> + for (i = s->nb_redist_regions - 1; i >= 0; i--) {
>
> So we register in reverse-index order? Looking at your update to Linux doc
> Documentation/virtual/kvm/devices/arm-vgic-v3.txt, shouldn't they be
> registered in index-order? The doc says "Redistributor regions must be
> registered in the incremental index order, starting from index 0."
kvm_arm_register_device adds the device in a QSLIST using
QSLIST_INSERT_HEAD. Then the list is read by kvm_arm_machine_init_done
from the HEAD. So we need to register in reversed order.
Thanks
Eric
>
> Thanks,
> drew
>
>> + /* Address mask made of the rdist region index and count */
>> + uint64_t addr_ormask =
>> + i | ((uint64_t)s->redist_region_count[i] << 52);
>> +
>> + kvm_arm_register_device(&s->iomem_redist[i], -1,
>> + KVM_DEV_ARM_VGIC_GRP_ADDR,
>> + KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION,
>> + s->dev_fd, addr_ormask);
>> + }
>> + }
>>
>> if (kvm_has_gsi_routing()) {
>> /* set up irq routing */
>> --
>> 2.5.5
>>
>>
