Re: [Xen-devel] [RFC PATCH 44/49] ARM: new VGIC: vgic-init: register VGIC

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andre,

On 09/02/18 14:39, Andre Przywara wrote:
> This patch implements the function which is called by Xen when it wants
> to register the virtual GIC.
>
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
> ---
>   xen/arch/arm/vgic/vgic-init.c | 62 +++++++++++++++++++++++++++++++++++++++++++
>   xen/arch/arm/vgic/vgic.h      |  3 +++
>   2 files changed, 65 insertions(+)
>   create mode 100644 xen/arch/arm/vgic/vgic-init.c
>
> diff --git a/xen/arch/arm/vgic/vgic-init.c b/xen/arch/arm/vgic/vgic-init.c
> new file mode 100644
> index 0000000000..b5f1183a50
> --- /dev/null
> +++ b/xen/arch/arm/vgic/vgic-init.c
> @@ -0,0 +1,62 @@
> +/*
> + * Copyright (C) 2015, 2016 ARM Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <xen/sched.h>
> +#include <asm/arm_vgic.h>
> +
> +#include "vgic.h"
> +
> +/* CREATION */
> +
> +/**
> + * domain_vgic_register: create a virtual GIC
> + * @d: domain pointer
> + * @mmio_count: pointer to add number of required MMIO regions
> + *
> + * was: kvm_vgic_create
> + */
> +int domain_vgic_register(struct domain *d, int *mmio_count)
mmio_count should be set to the number of I/O region you will register.

> +{
> +    switch ( d->arch.vgic.version )
> +    {
> +#ifdef CONFIG_HAS_GICV3
> +    case GIC_V3:
> +        d->arch.max_vcpus = VGIC_V3_MAX_CPUS;
> +        break;
> +#endif
> +    case GIC_V2:
> +        d->arch.max_vcpus = VGIC_V2_MAX_CPUS;
> +        break;
> +    }
> +
> +    if ( d->max_vcpus > d->arch.max_vcpus )
> +        return -E2BIG;
> +
> +    d->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
> +    d->arch.vgic.vgic_cpu_base = VGIC_ADDR_UNDEF;
> +    d->arch.vgic.vgic_redist_base = VGIC_ADDR_UNDEF;
Is there any reason to store an address rather than a frame? The latter 
would add a be more safety.
> +
> +    return 0;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/vgic/vgic.h b/xen/arch/arm/vgic/vgic.h
> index b104f8e964..205ce10ffa 100644
> --- a/xen/arch/arm/vgic/vgic.h
> +++ b/xen/arch/arm/vgic/vgic.h
> @@ -20,6 +20,9 @@
>   #define PRODUCT_ID_KVM      0x4b    /* ASCII code K */
>   #define IMPLEMENTER_ARM     0x43b
>   
> +#define VGIC_ADDR_UNDEF     (-1)
Please use INVALID_PADDR here.

> +#define IS_VGIC_ADDR_UNDEF(_x)  ((_x) == VGIC_ADDR_UNDEF)
> +
>   #define VGIC_PRI_BITS       5
>   
>   #define vgic_irq_is_sgi(intid) ((intid) < VGIC_NR_SGIS)
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel