[Qemu-devel] ARM CPU affinities

[Qemu-devel] ARM CPU affinities

[Peter Crosthwaite]

Hi Peter,

I am looking at this:

static void arm_cpu_initfn(Object *obj)
{
    ...
    Aff1 = cs->cpu_index / ARM_CPUS_PER_CLUSTER;
    Aff0 = cs->cpu_index % ARM_CPUS_PER_CLUSTER;
    cpu->mp_affinity = (Aff1 << ARM_AFF1_SHIFT) | Aff0;


Should we push this up to the machine model? I am trying to fix a
machine where Aff1 of the one and only cluster is non-zero. The kernel
SMP support barfs when Aff1 is mismatched to whats in the DTB (CPU reg
property).

I think this modulo 8 starting from 0:0 policy might be specific to mach-virt?

Regards,
Peter

Re: [Qemu-devel] ARM CPU affinities

[Peter Maydell]

On 27 September 2015 at 22:28, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> Hi Peter,
>
> I am looking at this:
>
> static void arm_cpu_initfn(Object *obj)
> {
>     ...
>     Aff1 = cs->cpu_index / ARM_CPUS_PER_CLUSTER;
>     Aff0 = cs->cpu_index % ARM_CPUS_PER_CLUSTER;
>     cpu->mp_affinity = (Aff1 << ARM_AFF1_SHIFT) | Aff0;
>
>
> Should we push this up to the machine model? I am trying to fix a
> machine where Aff1 of the one and only cluster is non-zero. The kernel
> SMP support barfs when Aff1 is mismatched to whats in the DTB (CPU reg
> property).
>
> I think this modulo 8 starting from 0:0 policy might be specific to mach-virt?

Yeah, it should be a CPU property if you need it to be something
different. I think we just left it as a hardcoded thing until
somebody needed it to be different.

NB that as the comment says KVM currently imposes its own numbering
anyway -- if you care about that you need to get the kernel to
support having userspace tell it about affinity numbering.

thanks
-- PMM

Re: [Qemu-devel] ARM CPU affinities

[Pavel Fedin]

 Hello!

> NB that as the comment says KVM currently imposes its own numbering
> anyway -- if you care about that you need to get the kernel to
> support having userspace tell it about affinity numbering.

 Yes. You cannot set MPIDR values for the KVM, because upon reset they are reverted to something which KVM thinks must be. IIRC it is 16 CPUs per cluster, starting from 0:0. So, currently you can do whatever you want only for TCG.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [Qemu-devel] ARM CPU affinities

[Andrew Jones]

On Mon, Sep 28, 2015 at 10:13:15AM +0100, Peter Maydell wrote:
> On 27 September 2015 at 22:28, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
> > Hi Peter,
> >
> > I am looking at this:
> >
> > static void arm_cpu_initfn(Object *obj)
> > {
> >     ...
> >     Aff1 = cs->cpu_index / ARM_CPUS_PER_CLUSTER;
> >     Aff0 = cs->cpu_index % ARM_CPUS_PER_CLUSTER;
> >     cpu->mp_affinity = (Aff1 << ARM_AFF1_SHIFT) | Aff0;
> >
> >
> > Should we push this up to the machine model? I am trying to fix a
> > machine where Aff1 of the one and only cluster is non-zero. The kernel
> > SMP support barfs when Aff1 is mismatched to whats in the DTB (CPU reg
> > property).
> >
> > I think this modulo 8 starting from 0:0 policy might be specific to mach-virt?
> 
> Yeah, it should be a CPU property if you need it to be something
> different. I think we just left it as a hardcoded thing until
> somebody needed it to be different.
> 
> NB that as the comment says KVM currently imposes its own numbering
> anyway -- if you care about that you need to get the kernel to
> support having userspace tell it about affinity numbering.

I've been thinking about picking this work up. Let me confirm first that
Pavel hasn't already, because, iirc, he was the last to look into it.
Pavel?

Thanks,
drew

Re: [Qemu-devel] ARM CPU affinities

[Pavel Fedin]

 Hello!

> I've been thinking about picking this work up. Let me confirm first that
> Pavel hasn't already, because, iirc, he was the last to look into it.
> Pavel?

 I am here. Yes, i did not do anything else with this. The problem is in the kernel; every time vCPU
is reset, it resets its MPIDR value to something that it wants it to be. IIRC the hardcoded
assignment is 16 CPUs per cluster starting from 0:0.

 I have some strange problem with delivering mails, my emails do not seem to make it to the list.
Please reply me privately if you got this.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [Qemu-devel] ARM CPU affinities

[Andrew Jones]

On Mon, Sep 28, 2015 at 06:16:26PM +0300, Pavel Fedin wrote:
>  Hello!
> 
> > I've been thinking about picking this work up. Let me confirm first that
> > Pavel hasn't already, because, iirc, he was the last to look into it.
> > Pavel?
> 
>  I am here. Yes, i did not do anything else with this. The problem is in the kernel; every time vCPU
> is reset, it resets its MPIDR value to something that it wants it to be. IIRC the hardcoded
> assignment is 16 CPUs per cluster starting from 0:0.

I think we'll need to cache mpidr in the vcpu data in order to properly
reset it.

> 
>  I have some strange problem with delivering mails, my emails do not seem to make it to the list.
> Please reply me privately if you got this.

Got your mail, replying not only to you, but also the list though, as I'll
use the opportunity to confirm that I'll go ahead and pick up this work.

Thanks,
drew

Re: [Qemu-devel] ARM CPU affinities

[Pavel Fedin]

 Hello!

> I think we'll need to cache mpidr in the vcpu data in order to properly
> reset it.

 We already have it in our data. I took a quick look at that, but kernel needs patching. IIRC reset
for secondary vCPUs happens inside kernel's PSCI code, before primary tells it to start up. You
cannot separate these two events.
 In a short: you can use KVM_SET_ONE_REG for the MPIDR, and the value will go into kernel, but as
soon as you call KVM_VCPU_RUN for the secondary, it gets reset.

> Got your mail, replying not only to you, but also the list though,

 And my emails do not go to the list indeed... Don't understand why.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [Qemu-devel] ARM CPU affinities

[Peter Crosthwaite]

On Mon, Sep 28, 2015 at 2:13 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 27 September 2015 at 22:28, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> Hi Peter,
>>
>> I am looking at this:
>>
>> static void arm_cpu_initfn(Object *obj)
>> {
>>     ...
>>     Aff1 = cs->cpu_index / ARM_CPUS_PER_CLUSTER;
>>     Aff0 = cs->cpu_index % ARM_CPUS_PER_CLUSTER;
>>     cpu->mp_affinity = (Aff1 << ARM_AFF1_SHIFT) | Aff0;
>>
>>
>> Should we push this up to the machine model? I am trying to fix a
>> machine where Aff1 of the one and only cluster is non-zero. The kernel
>> SMP support barfs when Aff1 is mismatched to whats in the DTB (CPU reg
>> property).
>>
>> I think this modulo 8 starting from 0:0 policy might be specific to mach-virt?
>
> Yeah, it should be a CPU property if you need it to be something
> different. I think we just left it as a hardcoded thing until
> somebody needed it to be different.
>
> NB that as the comment says KVM currently imposes its own numbering
> anyway -- if you care about that you need to get the kernel to
> support having userspace tell it about affinity numbering.
>

Not worrying about KVM yet, but I think pushing this up to virt
shouldn’t interfere with that work anyway.

Regards,
Peter

> thanks
> -- PMM

Re: [Qemu-devel] ARM CPU affinities

[Pavel Fedin]

 Hello!

> Not worrying about KVM yet, but I think pushing this up to virt
> shouldn’t interfere with that work anyway.

 Actually, IMHO this code is quite universal. My initial idea was to introduce a global variable instead of ARM_CPUS_PER_CLUSTER definition. By setting this variable before creating CPU objects you could make whatever layout you want. But, yes, it always starts from 0. Perhaps you could add another global variable, like 'offset', in order to cover this.

 But, PM didn't want global variable. Okay, you can add a property and override this default during machine model creation. And this has nothing to do with KVM. KVM code is executed after default MPIDR is assigned and overrides IDs with whatever it gets from the kernel. All you can do with current kernels is to patch device tree with new IDs.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [Qemu-devel] ARM CPU affinities

[Andrew Jones]

On Mon, Sep 28, 2015 at 06:36:29PM +0300, Pavel Fedin wrote:
>  Hello!
> 
> > I think we'll need to cache mpidr in the vcpu data in order to properly
> > reset it.
> 
>  We already have it in our data. I took a quick look at that, but kernel needs patching. IIRC reset

Right. I was referring to the kernel's vcpu data (struct kvm_vcpu).

> for secondary vCPUs happens inside kernel's PSCI code, before primary tells it to start up. You
> cannot separate these two events.
>  In a short: you can use KVM_SET_ONE_REG for the MPIDR, and the value will go into kernel, but as
> soon as you call KVM_VCPU_RUN for the secondary, it gets reset.

My current thinking is that we'll set MPIDR with KVM_SET_ONE_REG, which
will also set the cached value in struct kvm_vcpu. Reset will then use
that value.

Thanks,
drew

Re: [Qemu-devel] ARM CPU affinities

[Peter Maydell]

On 29 September 2015 at 07:15, Andrew Jones <drjones@redhat.com> wrote:
> My current thinking is that we'll set MPIDR with KVM_SET_ONE_REG, which
> will also set the cached value in struct kvm_vcpu. Reset will then use
> that value.

Yes, supporting SET_ONE_REG is definitely the way to go for
the userspace API for this.

Watch out for the kernel's GIC code which I think currently
makes assumptions about guest vcpu MPIDR values...

thanks
-- PMM