Re: [XEN v2 04/12] xen/Arm: vGICv3: Adapt emulation of GICR_TYPER for AArch32

[Julien Grall <julien@xxxxxxx>]

Hi Ayan,

On 07/11/2022 11:33, Ayan Kumar Halder wrote:
> On 06/11/2022 18:04, Julien Grall wrote:
> > Hi Ayan,
> Hi Julien,
>
> I need a clarification.
>
> > In the title you are using AArch32 but below you are using...
> >
> > On 31/10/2022 15:13, Ayan Kumar Halder wrote:
> > > v->arch.vmpidr is assigned to uint64_t variable. This is to enable 
> > > left shifts
> > > for Aarch32 so that one can extract affinity bits.
> > ... Aarch32. The naming also seem to be inconsistent across your 
> > series. AFAIU, it should be AArch32. So please look at all your 
> > commits and make sure you use the same everywhere.
> Ack
> > > This is then assigned to 'typer' so that the affinity bits form the 
> > > upper 32 bits.
> > > Refer Arm IHI 0069H ID020922,
> > > The upper 32 bits of GICR_TYPER represent the affinity
> > > whereas the lower 32 bits represent the other bits (eg processor
> > > number, etc).
> > >
> > > Signed-off-by: Ayan Kumar Halder <ayankuma@xxxxxxx>
> > > ---
> > >
> > > Changes from :-
> > > 1. v1 - Assigned v->arch.vmpidr to "uint64_t vmpdir". Then, we can use
> > > MPIDR_AFFINITY_LEVEL macros to extract the affinity value.
> > >
> > >   xen/arch/arm/vgic-v3.c | 10 ++++++----
> > >   1 file changed, 6 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> > > index 3f4509dcd3..e5e6f2c573 100644
> > > --- a/xen/arch/arm/vgic-v3.c
> > > +++ b/xen/arch/arm/vgic-v3.c
> > > @@ -191,13 +191,15 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct 
> > > vcpu *v, mmio_info_t *info,
> > >       case VREG64(GICR_TYPER):
> > >       {
> > >           uint64_t typer, aff;
> > > +        uint64_t vmpidr = v->arch.vmpidr;
> > >             if ( !vgic_reg64_check_access(dabt) ) goto bad_width;
> > > -        aff = (MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 3) << 56 |
> > > -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 2) << 48 |
> > > -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 1) << 40 |
> > > -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 0) << 32);
> > > +        aff = (MPIDR_AFFINITY_LEVEL(vmpidr, 3) << 56 |
> > Shouldn't we #ifdef this level for 32-bit? Or maybe check if the 
> > domain is 64-bit so we are using consistently regardless of the 
> > hypervisor bitness.
> We have typecasted "v->arch.vmpidr" (which is 32bit for AArch32 and 
> 64bit for AArch64)  to vmpidr (uint64_t).
> So, we don't need to have any #ifdef for AArch32 or AArch64.
This is not related to the typecast. This is more that fact that 
affinity level 3 doesn't exist for 32-bit guest. For instance vpsci.c 
will protect level 3 with an #ifdef.
Cheers,

--
Julien Grall