Re: [Xen-devel] [PATCH 2/7] ARM: vGICv3: drop GUEST_GICV3_RDIST_REGIONS symbol

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Andre,

On 24/01/18 16:58, Andre Przywara wrote:
> On 24/01/18 16:13, Julien Grall wrote:
> > Hi Andre,
> >
> > On 24/01/18 14:35, Andre Przywara wrote:
> > > Architecturally there is only one GICv3 redistributor region.
> > > Drop the symbol which suggested that was a delibarate choice for Xen
> > > guests, instead hard code the "1" in the appropriate places, along with
> > > a comment to explain the reasons.
> > >
> > > Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
> > > ---
> > >    xen/arch/arm/vgic-v3.c        | 17 ++++++++++++-----
> > >    xen/include/public/arch-arm.h |  1 -
> > >    2 files changed, 12 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> > > index af16dfd005..7d3ea171b4 100644
> > > --- a/xen/arch/arm/vgic-v3.c
> > > +++ b/xen/arch/arm/vgic-v3.c
> > > @@ -1640,8 +1640,18 @@ static int vgic_v3_vcpu_init(struct vcpu *v)
> > >      static inline unsigned int vgic_v3_rdist_count(struct domain *d)
> > >    {
> > > -    return is_hardware_domain(d) ? vgic_v3_hw.nr_rdist_regions :
> > > -               GUEST_GICV3_RDIST_REGIONS;
> > > +    /*
> > > +     * Architecturally there is only one GICv3 redistributor region.
> > > +     * The GICv3 DT binding provisions for multiple regions, since
> > > there are
> > > +     * platforms out there which break this architectural assumption.
> > > +     * ACPI does not support this workaround at all.
> > This is not true. The ACPI spec supports multiple regions of
> > redistributors. What ACPI does not support is a different stride.
> Ah, that's true. Sorry for that, I was a bit too enthusiastic here ;-)
> Will change the comment, definitely.
> However I would still be interested in dropping the
> GUEST_GICV3_RDIST_REGIONS symbol, possibly replacing it with
> DEFAULT_GICV3_RDIST_REGIONS, avoiding the misleading GUEST_ prefix.
Why GUEST_ is misleading?

Bear in mind that the toolstack is in charge of the memory map. Not the 
hypervisor.
Today, the memory map is static and so rather than implementing a bunch 
of hypercalls the hypervisor is directly using the value.
Cheers,

--
Julien Grall

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