Re: [PATCH] ARM/vgic: Clean up vgic_v2_setup_hw()

["Orzel, Michal" <michal.orzel@xxxxxxx>]

On 17-Aug-26 22:33, Andrew Cooper wrote:
> vgic_v2_setup_hw()'s callers are __init, so it should be too.  vgic_v2_hw is
> written once during init and unmodified thereafter, so make it
> __ro_after_init.  Reposition 'bool enabled' to fit in the tail padding,
> removing 8 bytes from the structure.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
The patch is good but exactly the same cleanup should be done for new vGICv2's
`gic_v2_hw_data` + `vgic_v2_setup_hw()` and vGICv3's `vgic_v3_hw` +
`vgic_v3_setup_hw()`. I can do the follow-up in which case for this patch:
Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
or you can bundle everything in one patch. Let me know.

~Michal
> ---
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien@xxxxxxx>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> CC: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> CC: Michal Orzel <michal.orzel@xxxxxxx>
> 
> Found when looking at the code while reviewing something else.  Only compile
> tested.
> ---
>  xen/arch/arm/vgic-v2.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
> index 642407fd5b05..3446d521de2d 100644
> --- a/xen/arch/arm/vgic-v2.c
> +++ b/xen/arch/arm/vgic-v2.c
> @@ -25,7 +25,6 @@
>  #include <asm/vreg.h>
>  
>  static struct {
> -    bool enabled;
>      /* Distributor interface address */
>      paddr_t dbase;
>      /* CPU interface address & size */
> @@ -36,10 +35,12 @@ static struct {
>  
>      /* Offset to add to get an 8kB contiguous region if GIC is aliased */
>      uint32_t aliased_offset;
> -} vgic_v2_hw;
> +    bool enabled;
> +} vgic_v2_hw __ro_after_init;
>  
> -void vgic_v2_setup_hw(paddr_t dbase, paddr_t cbase, paddr_t csize,
> -                      paddr_t vbase, uint32_t aliased_offset)
> +void __init vgic_v2_setup_hw(
> +    paddr_t dbase, paddr_t cbase, paddr_t csize, paddr_t vbase,
> +    uint32_t aliased_offset)
>  {
>      vgic_v2_hw.enabled = true;
>      vgic_v2_hw.dbase = dbase;
> 
> base-commit: ebc00c30c65023bd1498ae20dc511c4e39f6c0f7