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

[Thomas Huth]

On 16/01/2025 16.16, Andrew Jones wrote:
> 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.

I agree that this comment needs an update, but it could also be done in a 
separate patch later, so from my side for this patch here:

Reviewed-by: Thomas Huth <thuth@redhat.com>