Re: [PATCH-for-10.1 v2 03/13] hw/arm/virt: Remove VirtMachineClass::disallow_affinity_adjustment

[Andrew Jones]

On Thu, Jan 16, 2025 at 03:59:34PM +0100, Philippe Mathieu-Daudé wrote:
> The VirtMachineClass::disallow_affinity_adjustment
> field was only used by virt-2.6 machine, which got
> removed. Remove it and simplify virt_cpu_mp_affinity().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/hw/arm/virt.h |  1 -
>  hw/arm/virt.c         | 30 +++++++++++++++---------------
>  2 files changed, 15 insertions(+), 16 deletions(-)
> 
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 27c5bb585cb..5d3b25509ff 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -117,7 +117,6 @@ typedef enum VirtGICType {
>  
>  struct VirtMachineClass {
>      MachineClass parent;
> -    bool disallow_affinity_adjustment;
>      bool no_its;
>      bool no_tcg_its;
>      bool claim_edge_triggered_timers;
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 0080577e1a9..53f4a96e517 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1759,24 +1759,24 @@ void virt_machine_done(Notifier *notifier, void *data)
>  
>  static uint64_t virt_cpu_mp_affinity(VirtMachineState *vms, int idx)
>  {
> -    uint8_t clustersz = ARM_DEFAULT_CPUS_PER_CLUSTER;
> +    uint8_t clustersz;
>      VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
>  
> -    if (!vmc->disallow_affinity_adjustment) {
> -        /* Adjust MPIDR like 64-bit KVM hosts, which incorporate the
> -         * GIC's target-list limitations. 32-bit KVM hosts currently
> -         * always create clusters of 4 CPUs, but that is expected to
> -         * change when they gain support for gicv3. When KVM is enabled
> -         * it will override the changes we make here, therefore our
> -         * purposes are to make TCG consistent (with 64-bit KVM hosts)
> -         * and to improve SGI efficiency.
> -         */
> -        if (vms->gic_version == VIRT_GIC_VERSION_2) {
> -            clustersz = GIC_TARGETLIST_BITS;
> -        } else {
> -            clustersz = GICV3_TARGETLIST_BITS;
> -        }
> +    /*
> +     * Adjust MPIDR like 64-bit KVM hosts, which incorporate the
> +     * GIC's target-list limitations. 32-bit KVM hosts currently
> +     * always create clusters of 4 CPUs, but that is expected to
> +     * change when they gain support for gicv3. When KVM is enabled
> +     * it will override the changes we make here, therefore our
> +     * purposes are to make TCG consistent (with 64-bit KVM hosts)
> +     * and to improve SGI efficiency.

32-bit KVM hosts are dead a long time, so we could definitely trim
this comment to at least remove those sentences. We'd also have
to double check arm64 KVM to see if the comment is still accurate,
since the comment is so old. Or, we could just delete the comment.

Thanks,
drew

> +     */
> +    if (vms->gic_version == VIRT_GIC_VERSION_2) {
> +        clustersz = GIC_TARGETLIST_BITS;
> +    } else {
> +        clustersz = GICV3_TARGETLIST_BITS;
>      }
> +
>      return arm_build_mp_affinity(idx, clustersz);
>  }
>  
> -- 
> 2.47.1
>