Re: [RFC PATCH v3 6/9] hw/arm/virt-acpi-build: Use possible cpus in generation of MADT

[Andrew Jones]

On Tue, May 18, 2021 at 07:47:24PM +0800, wangyanan (Y) wrote:
> > > Is there any better alternative way about this?
> > Move the
> > 
> >    if (arm_feature(&armcpu->env, ARM_FEATURE_PMU)) {
> >        gicc->performance_interrupt = cpu_to_le32(PPI(VIRTUAL_PMU_IRQ));
> >    }
> > 
> > into the if (possible_cpus->cpus[i].cpu != NULL) block?
> We can. But this will only ensure that we initialize
> gicc->performance_interrupt
> for enabled GICC entries but not the disabled ones.

I'd write a comment and leave that problem for the hot plug support to
solve.

> > > Can we also update the arch_id at the same time when we change 
> > > mp_affinity?
> > The proper fix is to send patches to KVM enabling userspace to control
> > MPIDR. Otherwise we can't be sure we don't have inconsistencies in QEMU,
> > since some user of possible_cpus could have made decisions or copied IDs
> > prior to KVM vcpu init time. Now, all that said, I think
> > virt_cpu_mp_affinity() should be generating the same ID as KVM does, so
> > maybe it doesn't matter in practice right now, but we're living with the
> > risk that KVM could change. For now, maybe we should just sanity check
> > that the KVM values match the possible_cpus values and emit warnings if
> > they don't?
> I think it may not so reasonable to emit warnings if they don't match, on
> the contrary we should ensure they will match even when KVM changes.
> 
> Now virt_cpu_mp_affinity() is only called by virt_possible_cpu_arch_ids() to
> initialize possible_cpus, so an idea is that we can move the stuff of
> resetting
> "cpu->mp_affinity" from kvm_arch_init_vcpu() to virt_cpu_mp_affinity() to
> initialize arch_id. So that we can ensure mp_affinity only comes from
> arch_id
> and won't change later. Can it work?

No. virt_cpu_mp_affinity can run way before cpu realize, but
kvm_arch_init_vcpu runs at cpu realize time.

> 
> BTW, I plan to pack patch 4-6 into a separate patchset and repost it until
> we have a mature solution for the probelm, that's also Salil's suggestion.
> Is it appropriate?

Well, you still need to set the enabled flag in the GICC table and you'll
still need to come up with a solution for user input of cpus < maxcpus,
but if your solution to the latter is good, then whatever you like.

Thanks,
drew