Re: [Xen-devel] [RFC PATCH 13/49] ARM: VGIC: Add hypervisor base address to vgic_v2_setup_hw()

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andre,

On 09/02/18 14:39, Andre Przywara wrote:
> The new VGIC will need to know the hypervisor base address at some
> point, which is private to the hardware facing part of the VGIC so far.
> Add a parameter to vgic_v2_setup_hw() to pass this address on, so a VGIC
> implementation can make use of it.
> The current VGIC ignores this new parameter.
>
> TODO: add proper value for GICv2 on GICv3 emulation!
>
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
> ---
>   xen/arch/arm/gic-v2.c      | 3 ++-
>   xen/arch/arm/gic-v3.c      | 3 ++-
>   xen/arch/arm/vgic-v2.c     | 3 ++-
>   xen/include/asm-arm/vgic.h | 3 ++-
>   4 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index 2b271ba322..7a18abecfa 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -1207,7 +1207,8 @@ static int __init gicv2_init(void)
>       if ( !gicv2.map_hbase )
>           panic("GICv2: Failed to ioremap for GIC Virtual interface\n");
>   
> -    vgic_v2_setup_hw(dbase, cbase, csize, vbase, aliased_offset);
> +    vgic_v2_setup_hw(dbase, cbase, csize, vbase, gicv2.map_hbase,
> +                     aliased_offset);
>   
>       /* Global settings: interrupt distributor */
>       spin_lock_init(&gicv2.lock);
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index ea14ab4028..08d4703687 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1238,7 +1238,8 @@ static void __init gicv3_init_v2(void)
>       printk("GICv3 compatible with GICv2 cbase %#"PRIpaddr" vbase 
> %#"PRIpaddr"\n",
>              cbase, vbase);
>   
> -    vgic_v2_setup_hw(dbase, cbase, csize, vbase, 0);
> +    /* TODO: provide the proper HBASE address! */
Well, on GICv3 the hypervisor interface will be configured using system 
registers. So I am not sure how your new interface is going to work with 
GICv3.
But IHMO, this is breaking the spirit of this interface. The goal is to 
tell "we can support a virtual GICv2". How the LRs (or anything touching 
the hardware) should be done using gic_hw_ops.
Cheers,

--
Julien Grall

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