Re: [RFC PATCH v1 04/12] Arm: GICv3: Emulate GICR_TYPER on AArch32

[Xenia Ragiadakou <burzalodowa@xxxxxxxxx>]

On 10/21/22 18:31, Ayan Kumar Halder wrote:
Hi Ayan

> 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).
> MPIDR_AFFINITY_LEVEL() returns a 32 bit number on aarch32. Thus, this
> is appended to return GICR_TYPER register.
>
> Signed-off-by: Ayan Kumar Halder <ayankuma@xxxxxxx>
> ---
>   xen/arch/arm/vgic-v3.c | 14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index c31140eb20..d86b41a39f 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -190,14 +190,18 @@ 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 typer;
> +        uint32_t aff;
>   
>           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(v->arch.vmpidr, 3) << 24 |
> +               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 2) << 16 |
> +               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 1) << 8 |
> +               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 0));
>           typer = aff;
> +
> +        typer = typer << 32;
> +
>           /* We use the VCPU ID as the redistributor ID in bits[23:8] */
>           typer |= v->vcpu_id << GICR_TYPER_PROC_NUM_SHIFT;
>   
I don't see an issue I just want to propose alternatives that I think 
would reduce the changes, hopefully without breaking it.
So, other ways would be either to assign v->arch.vmpidr to a new 
variable uint64_t vmpidr and operate on this (without changing the 
shifts), or to leave the type of aff uint64_t, adjust the shifts and do 
typer = aff << 32.
--
Xenia