RE: [PATCH v2 3/4] xen/arm: Defer GICv2 CPU interface mapping until the first access

[Henry Wang <Henry.Wang@xxxxxxx>]

Hi Julien,

Thanks very much for your time and review :)

> -----Original Message-----
> From: Julien Grall <julien@xxxxxxx>
> Subject: Re: [PATCH v2 3/4] xen/arm: Defer GICv2 CPU interface mapping until
> the first access
> 
> Hi Henry,
> 
> On 30/01/2023 04:06, Henry Wang wrote:
> > Since the hardware domain (Dom0) has an unlimited size P2M pool, the
> > gicv2_map_hwdom_extra_mappings() is also not touched by this patch.
> 
> I didn't notice this in the previous version. The fact that dom0 has
> unlimited size P2M pool doesn't matter here (this could also change in
> the future). Even if the P2M pool was limited, then it would be fine
> because the extra mappings happen after domain_create(). So there is no
> need to map them on-demand as the code could be order the way we want.
> 
> So this paragraph needs to be reworded.

Sure, I've reworded this paragraph to below:
"Since gicv2_map_hwdom_extra_mappings() happens after domain_create(),
so there is no need to map the extra mappings on-demand, and therefore
keep the hwdom extra mappings as untouched."

> 
> > +    /*
> > +     * Map the GICv2 virtual CPU interface in the GIC CPU interface
> > +     * region of the guest on the first access of the MMIO region.
> > +     */
> > +    if ( d->arch.vgic.version == GIC_V2 &&
> > +         gfn_eq(gfn, gaddr_to_gfn(d->arch.vgic.cbase)) )
> 
> The CPU interface size is 8KB (bigger in some cases) but here you only
> check for the access to be in the first 4KB.

Yeah indeed, gfn might fall into the range between 4KB and 8KB, sorry
about that.

Considering that the CPU interface is continuous (I suppose), I have two
ways of rewriting the gfn check, we can do either:

gfn_eq(gfn, gaddr_to_gfn(d->arch.vgic.cbase)) ||
gfn_eq(gfn, gfn_add(gaddr_to_gfn(d->arch.vgic.cbase), 1))

or

gfn_to_gaddr(gfn) >= d->arch.vgic.cbase ||
gfn_to_gaddr(gfn) < d->arch.vgic.cbase + d->arch.vgic.csize

May I ask which one would you think is better? Thanks!

Kind regards,
Henry

> 
> Cheers,
> 
> --
> Julien Grall