[Xen-devel,1/4] xen/arm: Park CPUs with a MIDR different from the boot CPU.

From: Julien Grall <julien.grall@arm.com>

From: Julien Grall <julien.grall@arm.com>

Xen does not properly support big.LITTLE platform. All vCPUs of a guest
will always have the MIDR of the boot CPU (see arch_domain_create).
At best the guest may see unreliable performance (vCPU switching between
big and LITTLE), at worst the guest will become unreliable or insecure.

This is becoming more apparent with branch predictor hardening in Linux
because they target a specific kind of CPUs and may not work on other
CPUs.

For the time being, park any CPUs with a MDIR different from the boot
CPU. This will be revisited in the future once Xen gains understanding
of big.LITTLE.

[1] https://lists.xenproject.org/archives/html/xen-devel/2016-12/msg00826.html

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Oleksandr Tyshchenkko <oleksandr_tyshchenko@epam.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 docs/misc/xen-command-line.markdown | 10 ++++++++++
 xen/arch/arm/smpboot.c              | 26 ++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

Hi Stefano,

On 15/02/18 23:16, Stefano Stabellini wrote:
> On big.LITTLE systems not all cores have the same ACTLR. Instead of
> reading ACTLR and setting v->arch.actlr in vcpu_initialise, which is run
> always on pcpu 0, do it later on the same pcpu where the vcpu will run.

While the Hardware domain vCPUs will always be created on pCPU 0, this 
may not be the case for other domain as it is done via an hypercall.

> 
> This way, assuming that the vcpu has been created with the right pcpu
> affinity, the guest will be able to read the right ACTLR value, matching
> the one of the physical cpu.
> 
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
>   xen/arch/arm/domain.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index a010443..532e824 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -314,6 +314,8 @@ static void schedule_tail(struct vcpu *prev)
>   
>   static void continue_new_vcpu(struct vcpu *prev)
>   {
> +    current->arch.actlr = READ_SYSREG32(ACTLR_EL1);

This is not enough, you also need to move the call of 
processor_vcpu_initialise here. For instance on Cortex-A15, this will 
set/clear the SMP bit in ACTLR.

Also, I just noticed that processor_vcpu_initialize (in 
arch/arm/processor.c) assumes all the processor will be the same.  We 
probably want to turn the variable processor into a per-cpu value.

> +
>       schedule_tail(prev);
>   
>       if ( is_idle_vcpu(current) )
> @@ -540,8 +542,6 @@ int vcpu_initialise(struct vcpu *v)
>   
>       v->arch.vmpidr = MPIDR_SMP | vcpuid_to_vaffinity(v->vcpu_id);
>   
> -    v->arch.actlr = READ_SYSREG32(ACTLR_EL1);
> -
>       v->arch.hcr_el2 = get_default_hcr_flags();
>   
>       processor_vcpu_initialise(v);
> 

Cheers,

Hi Stefano,

On 15/02/18 23:16, Stefano Stabellini wrote:
> On big.LITTLE systems not all cores have the same midr. Instead of
> initializing the vpidr to the boot cpu midr, set it to the value of the
> midr of the pcpu where the vcpu will run.
> 
> This way, assuming that the vcpu has been created with the right pcpu
> affinity, the guest will be able to read the right vpidr value, matching
> the one of the physical cpu.
> 
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
>   xen/arch/arm/domain.c | 19 ++++++++++++++++---
>   1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 532e824..280125f 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -315,6 +315,22 @@ static void schedule_tail(struct vcpu *prev)
>   static void continue_new_vcpu(struct vcpu *prev)
>   {
>       current->arch.actlr = READ_SYSREG32(ACTLR_EL1);
> +    /*
> +     * Default the virtual ID to match the physical.
> +     *
> +     * In case the big.LITTLE systems, a guest should be created with
> +     * cpu affinity set so that all vcpus run on the same kind of pcpus.
> +     * Warn if it is not the case.

continue_new_vcpu is only called once at domain creation. So this looks 
pointless to check that here and probably in ctxt_switch_to.

But I don't want to see such check at every context switch. This is 
expensive and we should not impact all the platforms for the benefits of 
an unsafe configuration.

If you really want to do that, then it should only be done when the vCPU 
is migrating. That will reduce a lot the performance impact.

> +     */
> +    if ( !current->domain->arch.vpidr )
> +        current->domain->arch.vpidr = current_cpu_data.midr.bits;
> +    else if ( current->domain->arch.vpidr != current_cpu_data.midr.bits )
> +    {
> +        gdprintk(XENLOG_WARNING,
> +                 "WARNING: possible corruptions! d%uv%u is running on a pcpu with different midr (%x) from the others (%x)\n",

NIT: I would prefer if you use uppercase for pcpu and midr. This is 
easier to read.

Also, gdprintk already print the domain vCPU. So it is not necessary to 
have it again in the message.

> +                 current->domain->domain_id, current->vcpu_id,
> +                 current_cpu_data.midr.bits, current->domain->arch.vpidr);
> +    }
>   
>       schedule_tail(prev);
>   
> @@ -596,9 +612,6 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
>       if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
>           goto fail;
>   
> -    /* Default the virtual ID to match the physical */
> -    d->arch.vpidr = boot_cpu_data.midr.bits;
> -
>       clear_page(d->shared_info);
>       share_xen_page_with_guest(
>           virt_to_page(d->shared_info), d, XENSHARE_writable);
> 

Cheers,

Hi Stefano,

On 15/02/18 23:17, Stefano Stabellini wrote:
> Update the documentation of the hmp-unsafe option to explain how to use
> it safely, together with the right cpu affinity setting, on big.LITTLE
> systems.
> 
> Also update the warning message to point users to the docs.
> 
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> CC: jbeulich@suse.com
> CC: konrad.wilk@oracle.com
> CC: tim@xen.org
> CC: wei.liu2@citrix.com
> CC: andrew.cooper3@citrix.com
> CC: George.Dunlap@eu.citrix.com
> CC: ian.jackson@eu.citrix.com
> 
> ---
>   docs/misc/xen-command-line.markdown | 10 +++++++++-
>   xen/arch/arm/smpboot.c              |  9 +++++----
>   2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index 2184cb9..a1ebeea 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1007,7 +1007,15 @@ Control Xens use of the APEI Hardware Error Source Table, should one be found.
>   
>   Say yes at your own risk if you want to enable heterogenous computing
>   (such as big.LITTLE). This may result to an unstable and insecure
> -platform. When the option is disabled (default), CPUs that are not
> +platform, unless you manually specify the cpu affinity of all domains so
> +that all vcpus are scheduled on the same class of pcpus (big or LITTLE
> +but not both). vcpu migration between big cores and LITTLE cores is not
> +supported. Thus, if the first 4 pcpus are big and the last 4 are LITTLE,
> +all domains need to have either cpus = "0-3" or cpus = "4-7" in their VM
> +config. Moreover, dom0_vcpus_pin needs to be passed on the Xen command
> +line.

In your example here you suggest to have all the vCPUs of a guest to 
either on big or LITTLE cores. How about giving an example where the 
guest can have 2 LITTLE vCPUs and one big vCPU?

> +
> +When the hmp-unsafe option is disabled (default), CPUs that are not
>   identical to the boot CPU will be parked and not used by Xen.
>   
>   ### hpetbroadcast
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index 7ea4e41..20c1b4a 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -265,7 +265,7 @@ void __init smp_init_cpus(void)
>   
>       if ( opt_hmp_unsafe )
>           warning_add("WARNING: HMP COMPUTING HAS BEEN ENABLED.\n"
> -                    "It has implications on the security and stability of the system.\n");

I would still like to keep that line in the warning. Maybe with an 
"unless" after.

> +                    "Make sure to pass dom0_vcpus_pin, and specify the cpu affinity of all domains.\n");
>   }
>   
>   int __init
> @@ -306,13 +306,14 @@ void start_secondary(unsigned long boot_phys_offset,
>       /*
>        * Currently Xen assumes the platform has only one kind of CPUs.
>        * This assumption does not hold on big.LITTLE platform and may
> -     * result to instability and insecure platform. Better to park them
> -     * for now.
> +     * result to instability and insecure platform (unless cpu affinity
> +     * is manually specified for all domains). Better to park them for
> +     * now.
>        */
>       if ( !opt_hmp_unsafe &&
>            current_cpu_data.midr.bits != boot_cpu_data.midr.bits )
>       {
> -        printk(XENLOG_ERR "CPU%u MIDR (0x%x) does not match boot CPU MIDR (0x%x).\n",
> +        printk(XENLOG_ERR "CPU%u MIDR (0x%x) does not match boot CPU MIDR (0x%x), disable cpu. See hmp-unsafe.\n",

I am a bit reluctant to give the option in the message. It is a way for 
them to enable without looking at the documentation. Indeed it is quite 
obvious from the name to know hmp-unsafe is a boolean.

>                  smp_processor_id(), current_cpu_data.midr.bits,
>                  boot_cpu_data.midr.bits);
>           stop_cpu();
> 

Cheers,

On Fri, 16 Feb 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 15/02/18 23:16, Stefano Stabellini wrote:
> > On big.LITTLE systems not all cores have the same midr. Instead of
> > initializing the vpidr to the boot cpu midr, set it to the value of the
> > midr of the pcpu where the vcpu will run.
> > 
> > This way, assuming that the vcpu has been created with the right pcpu
> > affinity, the guest will be able to read the right vpidr value, matching
> > the one of the physical cpu.
> > 
> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > ---
> >   xen/arch/arm/domain.c | 19 ++++++++++++++++---
> >   1 file changed, 16 insertions(+), 3 deletions(-)
> > 
> > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> > index 532e824..280125f 100644
> > --- a/xen/arch/arm/domain.c
> > +++ b/xen/arch/arm/domain.c
> > @@ -315,6 +315,22 @@ static void schedule_tail(struct vcpu *prev)
> >   static void continue_new_vcpu(struct vcpu *prev)
> >   {
> >       current->arch.actlr = READ_SYSREG32(ACTLR_EL1);
> > +    /*
> > +     * Default the virtual ID to match the physical.
> > +     *
> > +     * In case the big.LITTLE systems, a guest should be created with
> > +     * cpu affinity set so that all vcpus run on the same kind of pcpus.
> > +     * Warn if it is not the case.
> 
> continue_new_vcpu is only called once at domain creation. So this looks
> pointless to check that here and probably in ctxt_switch_to.
> 
> But I don't want to see such check at every context switch. This is expensive
> and we should not impact all the platforms for the benefits of an unsafe
> configuration.
> 
> If you really want to do that, then it should only be done when the vCPU is
> migrating. That will reduce a lot the performance impact.

I don't want a check for every context switch either. I added it here
because continue_new_vcpu is only called once per vcpu at domain
creation -- it is a one time check. vcpus are supposed to be pinned (or
cpu affinity specified) anyway, so I thought I wouldn't add the check in
vcpu_migrate too. In any case, I am also happy to remove the check
completely, as we have already warned the user enough.


> > +     */
> > +    if ( !current->domain->arch.vpidr )
> > +        current->domain->arch.vpidr = current_cpu_data.midr.bits;
> > +    else if ( current->domain->arch.vpidr != current_cpu_data.midr.bits )
> > +    {
> > +        gdprintk(XENLOG_WARNING,
> > +                 "WARNING: possible corruptions! d%uv%u is running on a
> > pcpu with different midr (%x) from the others (%x)\n",
> 
> NIT: I would prefer if you use uppercase for pcpu and midr. This is easier to
> read.
> 
> Also, gdprintk already print the domain vCPU. So it is not necessary to have
> it again in the message.

I'll make the changes


> 
> > +                 current->domain->domain_id, current->vcpu_id,
> > +                 current_cpu_data.midr.bits, current->domain->arch.vpidr);
> > +    }
> >         schedule_tail(prev);
> >   @@ -596,9 +612,6 @@ int arch_domain_create(struct domain *d, unsigned int
> > domcr_flags,
> >       if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
> >           goto fail;
> >   -    /* Default the virtual ID to match the physical */
> > -    d->arch.vpidr = boot_cpu_data.midr.bits;
> > -
> >       clear_page(d->shared_info);
> >       share_xen_page_with_guest(
> >           virt_to_page(d->shared_info), d, XENSHARE_writable);
> > 
> 
> Cheers,
> 
> -- 
> Julien Grall
>

On 16/02/2018 20:31, Stefano Stabellini wrote:
> On Fri, 16 Feb 2018, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 15/02/18 23:16, Stefano Stabellini wrote:
>>> On big.LITTLE systems not all cores have the same midr. Instead of
>>> initializing the vpidr to the boot cpu midr, set it to the value of the
>>> midr of the pcpu where the vcpu will run.
>>>
>>> This way, assuming that the vcpu has been created with the right pcpu
>>> affinity, the guest will be able to read the right vpidr value, matching
>>> the one of the physical cpu.
>>>
>>> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
>>> ---
>>>    xen/arch/arm/domain.c | 19 ++++++++++++++++---
>>>    1 file changed, 16 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
>>> index 532e824..280125f 100644
>>> --- a/xen/arch/arm/domain.c
>>> +++ b/xen/arch/arm/domain.c
>>> @@ -315,6 +315,22 @@ static void schedule_tail(struct vcpu *prev)
>>>    static void continue_new_vcpu(struct vcpu *prev)
>>>    {
>>>        current->arch.actlr = READ_SYSREG32(ACTLR_EL1);
>>> +    /*
>>> +     * Default the virtual ID to match the physical.
>>> +     *
>>> +     * In case the big.LITTLE systems, a guest should be created with
>>> +     * cpu affinity set so that all vcpus run on the same kind of pcpus.
>>> +     * Warn if it is not the case.
>>
>> continue_new_vcpu is only called once at domain creation. So this looks
>> pointless to check that here and probably in ctxt_switch_to.
>>
>> But I don't want to see such check at every context switch. This is expensive
>> and we should not impact all the platforms for the benefits of an unsafe
>> configuration.
>>
>> If you really want to do that, then it should only be done when the vCPU is
>> migrating. That will reduce a lot the performance impact.
> 
> I don't want a check for every context switch either. I added it here
> because continue_new_vcpu is only called once per vcpu at domain
> creation -- it is a one time check. vcpus are supposed to be pinned (or
> cpu affinity specified) anyway, so I thought I wouldn't add the check in
> vcpu_migrate too. In any case, I am also happy to remove the check
> completely, as we have already warned the user enough.
If you agree that continue_new_vcpu is only called once per vCPU. Then I 
am not sure to understand the purpose of the check. What are you trying 
to warn the user with that?

Cheers,

On Fri, 16 Feb 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 15/02/18 23:17, Stefano Stabellini wrote:
> > Update the documentation of the hmp-unsafe option to explain how to use
> > it safely, together with the right cpu affinity setting, on big.LITTLE
> > systems.
> > 
> > Also update the warning message to point users to the docs.
> > 
> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > CC: jbeulich@suse.com
> > CC: konrad.wilk@oracle.com
> > CC: tim@xen.org
> > CC: wei.liu2@citrix.com
> > CC: andrew.cooper3@citrix.com
> > CC: George.Dunlap@eu.citrix.com
> > CC: ian.jackson@eu.citrix.com
> > 
> > ---
> >   docs/misc/xen-command-line.markdown | 10 +++++++++-
> >   xen/arch/arm/smpboot.c              |  9 +++++----
> >   2 files changed, 14 insertions(+), 5 deletions(-)
> > 
> > diff --git a/docs/misc/xen-command-line.markdown
> > b/docs/misc/xen-command-line.markdown
> > index 2184cb9..a1ebeea 100644
> > --- a/docs/misc/xen-command-line.markdown
> > +++ b/docs/misc/xen-command-line.markdown
> > @@ -1007,7 +1007,15 @@ Control Xens use of the APEI Hardware Error Source
> > Table, should one be found.
> >     Say yes at your own risk if you want to enable heterogenous computing
> >   (such as big.LITTLE). This may result to an unstable and insecure
> > -platform. When the option is disabled (default), CPUs that are not
> > +platform, unless you manually specify the cpu affinity of all domains so
> > +that all vcpus are scheduled on the same class of pcpus (big or LITTLE
> > +but not both). vcpu migration between big cores and LITTLE cores is not
> > +supported. Thus, if the first 4 pcpus are big and the last 4 are LITTLE,
> > +all domains need to have either cpus = "0-3" or cpus = "4-7" in their VM
> > +config. Moreover, dom0_vcpus_pin needs to be passed on the Xen command
> > +line.
> 
> In your example here you suggest to have all the vCPUs of a guest to either on
> big or LITTLE cores. How about giving an example where the guest can have 2
> LITTLE vCPUs and one big vCPU?

I would rather discourage it at the moment, given that it requires more
complex cpu affinity settings, or vcpu pinning. Also, I am afraid that
without matching corresponding topology information on the guest device
tree, guests might not work as expected in such a scenario.

What do you think?


> > +
> > +When the hmp-unsafe option is disabled (default), CPUs that are not
> >   identical to the boot CPU will be parked and not used by Xen.
> >     ### hpetbroadcast
> > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> > index 7ea4e41..20c1b4a 100644
> > --- a/xen/arch/arm/smpboot.c
> > +++ b/xen/arch/arm/smpboot.c
> > @@ -265,7 +265,7 @@ void __init smp_init_cpus(void)
> >         if ( opt_hmp_unsafe )
> >           warning_add("WARNING: HMP COMPUTING HAS BEEN ENABLED.\n"
> > -                    "It has implications on the security and stability of
> > the system.\n");
> 
> I would still like to keep that line in the warning. Maybe with an "unless"
> after.

OK, I'll do that


> > +                    "Make sure to pass dom0_vcpus_pin, and specify the cpu
> > affinity of all domains.\n");
> >   }
> >     int __init
> > @@ -306,13 +306,14 @@ void start_secondary(unsigned long boot_phys_offset,
> >       /*
> >        * Currently Xen assumes the platform has only one kind of CPUs.
> >        * This assumption does not hold on big.LITTLE platform and may
> > -     * result to instability and insecure platform. Better to park them
> > -     * for now.
> > +     * result to instability and insecure platform (unless cpu affinity
> > +     * is manually specified for all domains). Better to park them for
> > +     * now.
> >        */
> >       if ( !opt_hmp_unsafe &&
> >            current_cpu_data.midr.bits != boot_cpu_data.midr.bits )
> >       {
> > -        printk(XENLOG_ERR "CPU%u MIDR (0x%x) does not match boot CPU MIDR
> > (0x%x).\n",
> > +        printk(XENLOG_ERR "CPU%u MIDR (0x%x) does not match boot CPU MIDR
> > (0x%x), disable cpu. See hmp-unsafe.\n",
> 
> I am a bit reluctant to give the option in the message. It is a way for them
> to enable without looking at the documentation. Indeed it is quite obvious
> from the name to know hmp-unsafe is a boolean.

I see. What about:

  See docs/misc/xen-command-line.markdown:hmp-unsafe

Or:

  See hmp-unsafe under https://xenbits.xen.org/docs/unstable/misc/xen-command-line.html

?

> >                  smp_processor_id(), current_cpu_data.midr.bits,
> >                  boot_cpu_data.midr.bits);
> >           stop_cpu();
> > 
> 
> Cheers,
> 
> -- 
> Julien Grall
>

On Fri, 16 Feb 2018, Julien Grall wrote:
> On 16/02/2018 20:31, Stefano Stabellini wrote:
> > On Fri, 16 Feb 2018, Julien Grall wrote:
> > > Hi Stefano,
> > > 
> > > On 15/02/18 23:16, Stefano Stabellini wrote:
> > > > On big.LITTLE systems not all cores have the same midr. Instead of
> > > > initializing the vpidr to the boot cpu midr, set it to the value of the
> > > > midr of the pcpu where the vcpu will run.
> > > > 
> > > > This way, assuming that the vcpu has been created with the right pcpu
> > > > affinity, the guest will be able to read the right vpidr value, matching
> > > > the one of the physical cpu.
> > > > 
> > > > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > > > ---
> > > >    xen/arch/arm/domain.c | 19 ++++++++++++++++---
> > > >    1 file changed, 16 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> > > > index 532e824..280125f 100644
> > > > --- a/xen/arch/arm/domain.c
> > > > +++ b/xen/arch/arm/domain.c
> > > > @@ -315,6 +315,22 @@ static void schedule_tail(struct vcpu *prev)
> > > >    static void continue_new_vcpu(struct vcpu *prev)
> > > >    {
> > > >        current->arch.actlr = READ_SYSREG32(ACTLR_EL1);
> > > > +    /*
> > > > +     * Default the virtual ID to match the physical.
> > > > +     *
> > > > +     * In case the big.LITTLE systems, a guest should be created with
> > > > +     * cpu affinity set so that all vcpus run on the same kind of
> > > > pcpus.
> > > > +     * Warn if it is not the case.
> > > 
> > > continue_new_vcpu is only called once at domain creation. So this looks
> > > pointless to check that here and probably in ctxt_switch_to.
> > > 
> > > But I don't want to see such check at every context switch. This is
> > > expensive
> > > and we should not impact all the platforms for the benefits of an unsafe
> > > configuration.
> > > 
> > > If you really want to do that, then it should only be done when the vCPU
> > > is
> > > migrating. That will reduce a lot the performance impact.
> > 
> > I don't want a check for every context switch either. I added it here
> > because continue_new_vcpu is only called once per vcpu at domain
> > creation -- it is a one time check. vcpus are supposed to be pinned (or
> > cpu affinity specified) anyway, so I thought I wouldn't add the check in
> > vcpu_migrate too. In any case, I am also happy to remove the check
> > completely, as we have already warned the user enough.
> If you agree that continue_new_vcpu is only called once per vCPU. Then I am
> not sure to understand the purpose of the check. What are you trying to warn
> the user with that?

The intention was to warn the user if she made a mistake with vcpu
pinning and/or cpu affinity.

Even with this series and vcpu pinning, I assumed that only scenarios
with vcpus assigned to pcpus of the same kind are allowed (see my other
reply). Thus, I added this check to test once at boot that all vcpus
in a domain have the same actlr.

On 16/02/2018 20:50, Stefano Stabellini wrote:
> On Fri, 16 Feb 2018, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 15/02/18 23:17, Stefano Stabellini wrote:
>>> Update the documentation of the hmp-unsafe option to explain how to use
>>> it safely, together with the right cpu affinity setting, on big.LITTLE
>>> systems.
>>>
>>> Also update the warning message to point users to the docs.
>>>
>>> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
>>> CC: jbeulich@suse.com
>>> CC: konrad.wilk@oracle.com
>>> CC: tim@xen.org
>>> CC: wei.liu2@citrix.com
>>> CC: andrew.cooper3@citrix.com
>>> CC: George.Dunlap@eu.citrix.com
>>> CC: ian.jackson@eu.citrix.com
>>>
>>> ---
>>>    docs/misc/xen-command-line.markdown | 10 +++++++++-
>>>    xen/arch/arm/smpboot.c              |  9 +++++----
>>>    2 files changed, 14 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/docs/misc/xen-command-line.markdown
>>> b/docs/misc/xen-command-line.markdown
>>> index 2184cb9..a1ebeea 100644
>>> --- a/docs/misc/xen-command-line.markdown
>>> +++ b/docs/misc/xen-command-line.markdown
>>> @@ -1007,7 +1007,15 @@ Control Xens use of the APEI Hardware Error Source
>>> Table, should one be found.
>>>      Say yes at your own risk if you want to enable heterogenous computing
>>>    (such as big.LITTLE). This may result to an unstable and insecure
>>> -platform. When the option is disabled (default), CPUs that are not
>>> +platform, unless you manually specify the cpu affinity of all domains so
>>> +that all vcpus are scheduled on the same class of pcpus (big or LITTLE
>>> +but not both). vcpu migration between big cores and LITTLE cores is not
>>> +supported. Thus, if the first 4 pcpus are big and the last 4 are LITTLE,
>>> +all domains need to have either cpus = "0-3" or cpus = "4-7" in their VM
>>> +config. Moreover, dom0_vcpus_pin needs to be passed on the Xen command
>>> +line.
>>
>> In your example here you suggest to have all the vCPUs of a guest to either on
>> big or LITTLE cores. How about giving an example where the guest can have 2
>> LITTLE vCPUs and one big vCPU?
> 
> I would rather discourage it at the moment, given that it requires more
> complex cpu affinity settings, or vcpu pinning. Also, I am afraid that
> without matching corresponding topology information on the guest device
> tree, guests might not work as expected in such a scenario.
> 
> What do you think?

You already know my view on this. I would rather strongly discourage 
anyone pinning all vCPUs of a domain to big cores. We should avoid to 
provide shortcuts to use that could have potentially damageable impact 
on their platform without telling them.

> I see. What about:
> 
>    See docs/misc/xen-command-line.markdown:hmp-unsafe
> 
> Or:
> 
>    See hmp-unsafe under https://xenbits.xen.org/docs/unstable/misc/xen-command-line.html

I believe that people looking at big.LITTLE (and really want it) are 
smart enough to look at the docs or the code themselves. Given how 
fragile is your solution, I would rather avoid to help people doing bad 
thing.

Cheers,

Hi Stefano,

On 16/02/2018 20:59, Stefano Stabellini wrote:
> On Fri, 16 Feb 2018, Julien Grall wrote:
>> On 16/02/2018 20:31, Stefano Stabellini wrote:
>>> On Fri, 16 Feb 2018, Julien Grall wrote:
>>>> Hi Stefano,
>>>>
>>>> On 15/02/18 23:16, Stefano Stabellini wrote:
>>>>> On big.LITTLE systems not all cores have the same midr. Instead of
>>>>> initializing the vpidr to the boot cpu midr, set it to the value of the
>>>>> midr of the pcpu where the vcpu will run.
>>>>>
>>>>> This way, assuming that the vcpu has been created with the right pcpu
>>>>> affinity, the guest will be able to read the right vpidr value, matching
>>>>> the one of the physical cpu.
>>>>>
>>>>> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
>>>>> ---
>>>>>     xen/arch/arm/domain.c | 19 ++++++++++++++++---
>>>>>     1 file changed, 16 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
>>>>> index 532e824..280125f 100644
>>>>> --- a/xen/arch/arm/domain.c
>>>>> +++ b/xen/arch/arm/domain.c
>>>>> @@ -315,6 +315,22 @@ static void schedule_tail(struct vcpu *prev)
>>>>>     static void continue_new_vcpu(struct vcpu *prev)
>>>>>     {
>>>>>         current->arch.actlr = READ_SYSREG32(ACTLR_EL1);
>>>>> +    /*
>>>>> +     * Default the virtual ID to match the physical.
>>>>> +     *
>>>>> +     * In case the big.LITTLE systems, a guest should be created with
>>>>> +     * cpu affinity set so that all vcpus run on the same kind of
>>>>> pcpus.
>>>>> +     * Warn if it is not the case.
>>>>
>>>> continue_new_vcpu is only called once at domain creation. So this looks
>>>> pointless to check that here and probably in ctxt_switch_to.
>>>>
>>>> But I don't want to see such check at every context switch. This is
>>>> expensive
>>>> and we should not impact all the platforms for the benefits of an unsafe
>>>> configuration.
>>>>
>>>> If you really want to do that, then it should only be done when the vCPU
>>>> is
>>>> migrating. That will reduce a lot the performance impact.
>>>
>>> I don't want a check for every context switch either. I added it here
>>> because continue_new_vcpu is only called once per vcpu at domain
>>> creation -- it is a one time check. vcpus are supposed to be pinned (or
>>> cpu affinity specified) anyway, so I thought I wouldn't add the check in
>>> vcpu_migrate too. In any case, I am also happy to remove the check
>>> completely, as we have already warned the user enough.
>> If you agree that continue_new_vcpu is only called once per vCPU. Then I am
>> not sure to understand the purpose of the check. What are you trying to warn
>> the user with that?
> 
> The intention was to warn the user if she made a mistake with vcpu
> pinning and/or cpu affinity.

Oh that is current->domain->arch.vpidr and not vCPU. Sorry for that.

vpidr should be per vCPU. It is very dangerous to recommend the user to 
pin there all vCPUs of a domain to either only big or LITTLE. This is 
even worst than what we have today.

> 
> Even with this series and vcpu pinning, I assumed that only scenarios
> with vcpus assigned to pcpus of the same kind are allowed (see my other
> reply). Thus, I added this check to test once at boot that all vcpus
> in a domain have the same actlr.

That's plain wrong. You really can't assume that same actlr means same 
type of CPUs. Imagine they are RES0 on both.

Cheers,

On Fri, 16 Feb 2018, Julien Grall wrote:
> On 16/02/2018 20:50, Stefano Stabellini wrote:
> > On Fri, 16 Feb 2018, Julien Grall wrote:
> > > Hi Stefano,
> > > 
> > > On 15/02/18 23:17, Stefano Stabellini wrote:
> > > > Update the documentation of the hmp-unsafe option to explain how to use
> > > > it safely, together with the right cpu affinity setting, on big.LITTLE
> > > > systems.
> > > > 
> > > > Also update the warning message to point users to the docs.
> > > > 
> > > > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > > > CC: jbeulich@suse.com
> > > > CC: konrad.wilk@oracle.com
> > > > CC: tim@xen.org
> > > > CC: wei.liu2@citrix.com
> > > > CC: andrew.cooper3@citrix.com
> > > > CC: George.Dunlap@eu.citrix.com
> > > > CC: ian.jackson@eu.citrix.com
> > > > 
> > > > ---
> > > >    docs/misc/xen-command-line.markdown | 10 +++++++++-
> > > >    xen/arch/arm/smpboot.c              |  9 +++++----
> > > >    2 files changed, 14 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/docs/misc/xen-command-line.markdown
> > > > b/docs/misc/xen-command-line.markdown
> > > > index 2184cb9..a1ebeea 100644
> > > > --- a/docs/misc/xen-command-line.markdown
> > > > +++ b/docs/misc/xen-command-line.markdown
> > > > @@ -1007,7 +1007,15 @@ Control Xens use of the APEI Hardware Error
> > > > Source
> > > > Table, should one be found.
> > > >      Say yes at your own risk if you want to enable heterogenous
> > > > computing
> > > >    (such as big.LITTLE). This may result to an unstable and insecure
> > > > -platform. When the option is disabled (default), CPUs that are not
> > > > +platform, unless you manually specify the cpu affinity of all domains
> > > > so
> > > > +that all vcpus are scheduled on the same class of pcpus (big or LITTLE
> > > > +but not both). vcpu migration between big cores and LITTLE cores is not
> > > > +supported. Thus, if the first 4 pcpus are big and the last 4 are
> > > > LITTLE,
> > > > +all domains need to have either cpus = "0-3" or cpus = "4-7" in their
> > > > VM
> > > > +config. Moreover, dom0_vcpus_pin needs to be passed on the Xen command
> > > > +line.
> > > 
> > > In your example here you suggest to have all the vCPUs of a guest to
> > > either on
> > > big or LITTLE cores. How about giving an example where the guest can have
> > > 2
> > > LITTLE vCPUs and one big vCPU?
> > 
> > I would rather discourage it at the moment, given that it requires more
> > complex cpu affinity settings, or vcpu pinning. Also, I am afraid that
> > without matching corresponding topology information on the guest device
> > tree, guests might not work as expected in such a scenario.
> > 
> > What do you think?
> 
> You already know my view on this. I would rather strongly discourage anyone
> pinning all vCPUs of a domain to big cores. We should avoid to provide
> shortcuts to use that could have potentially damageable impact on their
> platform without telling them.

Do you have a link to a doc somewhere that provides more details about
this? We could add a link to it here to inform users. It would be
useful.


> > I see. What about:
> > 
> >    See docs/misc/xen-command-line.markdown:hmp-unsafe
> > 
> > Or:
> > 
> >    See hmp-unsafe under
> > https://xenbits.xen.org/docs/unstable/misc/xen-command-line.html
> 
> I believe that people looking at big.LITTLE (and really want it) are smart
> enough to look at the docs or the code themselves. Given how fragile is your
> solution, I would rather avoid to help people doing bad thing.

People might know very well the hardware, and have very detailed
information about big.LITTLE and their platform, but might not know that
much about Xen.

I would like to introduce a tie between the warning message and the
documentation provided. The tie doesn't have to be around hmp-unsafe:
the intention wasn't really to provide a shortcut to do more damage, but
rather to inform about the situation. In fact, I didn't mean to
recommend the usage of hmp-unsafe.

Maybe we could say:

  See big.LITTLE under https://xenbits.xen.org/docs/unstable/misc/xen-command-line.html

or create docs/mics/arm/big.LITTLE ?

On Fri, 16 Feb 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 16/02/2018 20:59, Stefano Stabellini wrote:
> > On Fri, 16 Feb 2018, Julien Grall wrote:
> > > On 16/02/2018 20:31, Stefano Stabellini wrote:
> > > > On Fri, 16 Feb 2018, Julien Grall wrote:
> > > > > Hi Stefano,
> > > > > 
> > > > > On 15/02/18 23:16, Stefano Stabellini wrote:
> > > > > > On big.LITTLE systems not all cores have the same midr. Instead of
> > > > > > initializing the vpidr to the boot cpu midr, set it to the value of
> > > > > > the
> > > > > > midr of the pcpu where the vcpu will run.
> > > > > > 
> > > > > > This way, assuming that the vcpu has been created with the right
> > > > > > pcpu
> > > > > > affinity, the guest will be able to read the right vpidr value,
> > > > > > matching
> > > > > > the one of the physical cpu.
> > > > > > 
> > > > > > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > > > > > ---
> > > > > >     xen/arch/arm/domain.c | 19 ++++++++++++++++---
> > > > > >     1 file changed, 16 insertions(+), 3 deletions(-)
> > > > > > 
> > > > > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> > > > > > index 532e824..280125f 100644
> > > > > > --- a/xen/arch/arm/domain.c
> > > > > > +++ b/xen/arch/arm/domain.c
> > > > > > @@ -315,6 +315,22 @@ static void schedule_tail(struct vcpu *prev)
> > > > > >     static void continue_new_vcpu(struct vcpu *prev)
> > > > > >     {
> > > > > >         current->arch.actlr = READ_SYSREG32(ACTLR_EL1);
> > > > > > +    /*
> > > > > > +     * Default the virtual ID to match the physical.
> > > > > > +     *
> > > > > > +     * In case the big.LITTLE systems, a guest should be created
> > > > > > with
> > > > > > +     * cpu affinity set so that all vcpus run on the same kind of
> > > > > > pcpus.
> > > > > > +     * Warn if it is not the case.
> > > > > 
> > > > > continue_new_vcpu is only called once at domain creation. So this
> > > > > looks
> > > > > pointless to check that here and probably in ctxt_switch_to.
> > > > > 
> > > > > But I don't want to see such check at every context switch. This is
> > > > > expensive
> > > > > and we should not impact all the platforms for the benefits of an
> > > > > unsafe
> > > > > configuration.
> > > > > 
> > > > > If you really want to do that, then it should only be done when the
> > > > > vCPU
> > > > > is
> > > > > migrating. That will reduce a lot the performance impact.
> > > > 
> > > > I don't want a check for every context switch either. I added it here
> > > > because continue_new_vcpu is only called once per vcpu at domain
> > > > creation -- it is a one time check. vcpus are supposed to be pinned (or
> > > > cpu affinity specified) anyway, so I thought I wouldn't add the check in
> > > > vcpu_migrate too. In any case, I am also happy to remove the check
> > > > completely, as we have already warned the user enough.
> > > If you agree that continue_new_vcpu is only called once per vCPU. Then I
> > > am
> > > not sure to understand the purpose of the check. What are you trying to
> > > warn
> > > the user with that?
> > 
> > The intention was to warn the user if she made a mistake with vcpu
> > pinning and/or cpu affinity.
> 
> Oh that is current->domain->arch.vpidr and not vCPU. Sorry for that.

Now your comments make sense! Yeah, I thought so too. I'll make the change.


> vpidr should be per vCPU. It is very dangerous to recommend the user to pin
> there all vCPUs of a domain to either only big or LITTLE. This is even worst
> than what we have today.

Let's continue this discussion in the other thread which is more
appropriately about documentation.


> > Even with this series and vcpu pinning, I assumed that only scenarios
> > with vcpus assigned to pcpus of the same kind are allowed (see my other
> > reply). Thus, I added this check to test once at boot that all vcpus
> > in a domain have the same actlr.
> 
> That's plain wrong. You really can't assume that same actlr means same type of
> CPUs. Imagine they are RES0 on both.

It would be a false negative, and wouldn't trigger the warning. A false
positive would be worse: different pcpus with the same midr. Is that
possible?

In any case, at this point I am convinced that it is best to remove the
warning.

Hi Stefano,

On 16/02/2018 21:34, Stefano Stabellini wrote:
> On Fri, 16 Feb 2018, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 16/02/2018 20:59, Stefano Stabellini wrote:
>>> On Fri, 16 Feb 2018, Julien Grall wrote:
>>>> On 16/02/2018 20:31, Stefano Stabellini wrote:
>>>>> On Fri, 16 Feb 2018, Julien Grall wrote:
>>>>>> Hi Stefano,
>>>>>>
>>>>>> On 15/02/18 23:16, Stefano Stabellini wrote:
>>>>>>> On big.LITTLE systems not all cores have the same midr. Instead of
>>>>>>> initializing the vpidr to the boot cpu midr, set it to the value of
>>>>>>> the
>>>>>>> midr of the pcpu where the vcpu will run.
>>>>>>>
>>>>>>> This way, assuming that the vcpu has been created with the right
>>>>>>> pcpu
>>>>>>> affinity, the guest will be able to read the right vpidr value,
>>>>>>> matching
>>>>>>> the one of the physical cpu.
>>>>>>>
>>>>>>> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
>>>>>>> ---
>>>>>>>      xen/arch/arm/domain.c | 19 ++++++++++++++++---
>>>>>>>      1 file changed, 16 insertions(+), 3 deletions(-)
>>>>>>>
>>>>>>> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
>>>>>>> index 532e824..280125f 100644
>>>>>>> --- a/xen/arch/arm/domain.c
>>>>>>> +++ b/xen/arch/arm/domain.c
>>>>>>> @@ -315,6 +315,22 @@ static void schedule_tail(struct vcpu *prev)
>>>>>>>      static void continue_new_vcpu(struct vcpu *prev)
>>>>>>>      {
>>>>>>>          current->arch.actlr = READ_SYSREG32(ACTLR_EL1);
>>>>>>> +    /*
>>>>>>> +     * Default the virtual ID to match the physical.
>>>>>>> +     *
>>>>>>> +     * In case the big.LITTLE systems, a guest should be created
>>>>>>> with
>>>>>>> +     * cpu affinity set so that all vcpus run on the same kind of
>>>>>>> pcpus.
>>>>>>> +     * Warn if it is not the case.
>>>>>>
>>>>>> continue_new_vcpu is only called once at domain creation. So this
>>>>>> looks
>>>>>> pointless to check that here and probably in ctxt_switch_to.
>>>>>>
>>>>>> But I don't want to see such check at every context switch. This is
>>>>>> expensive
>>>>>> and we should not impact all the platforms for the benefits of an
>>>>>> unsafe
>>>>>> configuration.
>>>>>>
>>>>>> If you really want to do that, then it should only be done when the
>>>>>> vCPU
>>>>>> is
>>>>>> migrating. That will reduce a lot the performance impact.
>>>>>
>>>>> I don't want a check for every context switch either. I added it here
>>>>> because continue_new_vcpu is only called once per vcpu at domain
>>>>> creation -- it is a one time check. vcpus are supposed to be pinned (or
>>>>> cpu affinity specified) anyway, so I thought I wouldn't add the check in
>>>>> vcpu_migrate too. In any case, I am also happy to remove the check
>>>>> completely, as we have already warned the user enough.
>>>> If you agree that continue_new_vcpu is only called once per vCPU. Then I
>>>> am
>>>> not sure to understand the purpose of the check. What are you trying to
>>>> warn
>>>> the user with that?
>>>
>>> The intention was to warn the user if she made a mistake with vcpu
>>> pinning and/or cpu affinity.
>>
>> Oh that is current->domain->arch.vpidr and not vCPU. Sorry for that.
> 
> Now your comments make sense! Yeah, I thought so too. I'll make the change.
> 
> 
>> vpidr should be per vCPU. It is very dangerous to recommend the user to pin
>> there all vCPUs of a domain to either only big or LITTLE. This is even worst
>> than what we have today.
> 
> Let's continue this discussion in the other thread which is more
> appropriately about documentation.
> 
> 
>>> Even with this series and vcpu pinning, I assumed that only scenarios
>>> with vcpus assigned to pcpus of the same kind are allowed (see my other
>>> reply). Thus, I added this check to test once at boot that all vcpus
>>> in a domain have the same actlr.
>>
>> That's plain wrong. You really can't assume that same actlr means same type of
>> CPUs. Imagine they are RES0 on both.
> 
> It would be a false negative, and wouldn't trigger the warning. A false
> positive would be worse: different pcpus with the same midr. Is that
> possible?
You mean having different value in ACTLR? I am not entirely sure, we do 
trap guest access on ACTLR_EL1. So they stay at the value initialized by 
the firmware or the hypervisor (we set SMP bit on Cortex-A15 cores).

> 
> In any case, at this point I am convinced that it is best to remove the
> warning.

A warning would be more meaningful when migrating. To double-check the 
user did configure the pinning correctly.

Cheers,

On 16/02/2018 21:15, Stefano Stabellini wrote:
> On Fri, 16 Feb 2018, Julien Grall wrote:
>> On 16/02/2018 20:50, Stefano Stabellini wrote:
>>> On Fri, 16 Feb 2018, Julien Grall wrote:
>>>> Hi Stefano,
>>>>
>>>> On 15/02/18 23:17, Stefano Stabellini wrote:
>>>>> Update the documentation of the hmp-unsafe option to explain how to use
>>>>> it safely, together with the right cpu affinity setting, on big.LITTLE
>>>>> systems.
>>>>>
>>>>> Also update the warning message to point users to the docs.
>>>>>
>>>>> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
>>>>> CC: jbeulich@suse.com
>>>>> CC: konrad.wilk@oracle.com
>>>>> CC: tim@xen.org
>>>>> CC: wei.liu2@citrix.com
>>>>> CC: andrew.cooper3@citrix.com
>>>>> CC: George.Dunlap@eu.citrix.com
>>>>> CC: ian.jackson@eu.citrix.com
>>>>>
>>>>> ---
>>>>>     docs/misc/xen-command-line.markdown | 10 +++++++++-
>>>>>     xen/arch/arm/smpboot.c              |  9 +++++----
>>>>>     2 files changed, 14 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/docs/misc/xen-command-line.markdown
>>>>> b/docs/misc/xen-command-line.markdown
>>>>> index 2184cb9..a1ebeea 100644
>>>>> --- a/docs/misc/xen-command-line.markdown
>>>>> +++ b/docs/misc/xen-command-line.markdown
>>>>> @@ -1007,7 +1007,15 @@ Control Xens use of the APEI Hardware Error
>>>>> Source
>>>>> Table, should one be found.
>>>>>       Say yes at your own risk if you want to enable heterogenous
>>>>> computing
>>>>>     (such as big.LITTLE). This may result to an unstable and insecure
>>>>> -platform. When the option is disabled (default), CPUs that are not
>>>>> +platform, unless you manually specify the cpu affinity of all domains
>>>>> so
>>>>> +that all vcpus are scheduled on the same class of pcpus (big or LITTLE
>>>>> +but not both). vcpu migration between big cores and LITTLE cores is not
>>>>> +supported. Thus, if the first 4 pcpus are big and the last 4 are
>>>>> LITTLE,
>>>>> +all domains need to have either cpus = "0-3" or cpus = "4-7" in their
>>>>> VM
>>>>> +config. Moreover, dom0_vcpus_pin needs to be passed on the Xen command
>>>>> +line.
>>>>
>>>> In your example here you suggest to have all the vCPUs of a guest to
>>>> either on
>>>> big or LITTLE cores. How about giving an example where the guest can have
>>>> 2
>>>> LITTLE vCPUs and one big vCPU?
>>>
>>> I would rather discourage it at the moment, given that it requires more
>>> complex cpu affinity settings, or vcpu pinning. Also, I am afraid that
>>> without matching corresponding topology information on the guest device
>>> tree, guests might not work as expected in such a scenario.
>>>
>>> What do you think?
>>
>> You already know my view on this. I would rather strongly discourage anyone
>> pinning all vCPUs of a domain to big cores. We should avoid to provide
>> shortcuts to use that could have potentially damageable impact on their
>> platform without telling them.
> 
> Do you have a link to a doc somewhere that provides more details about
> this? We could add a link to it here to inform users. It would be
> useful.

This is quite well described in 
https://xenbits.xen.org/docs/unstable/man/xl.cfg.5.html#CPU-Allocation 
see "cpus".

> 
> 
>>> I see. What about:
>>>
>>>     See docs/misc/xen-command-line.markdown:hmp-unsafe
>>>
>>> Or:
>>>
>>>     See hmp-unsafe under
>>> https://xenbits.xen.org/docs/unstable/misc/xen-command-line.html
>>
>> I believe that people looking at big.LITTLE (and really want it) are smart
>> enough to look at the docs or the code themselves. Given how fragile is your
>> solution, I would rather avoid to help people doing bad thing.
> 
> People might know very well the hardware, and have very detailed
> information about big.LITTLE and their platform, but might not know that
> much about Xen.

We also might have student who wants to try Xen on their platform and 
have no clue how it works. So we want to provide safe information here 
and avoid to induce pinning on big cores is always safe.

I think with the tools we have, it is not very difficult provide a quick 
tuto how to do big.LITTLE in Xen. Obviously this will not be as easy as 
the solution suggested by Dario in its design document.

The most important bits is libxl provide an extensive way to set CPUs 
affinity. OS such as Linux will be able to deal with big.LITTLE even 
without DT thought scheduling might not be that good... I have no idea 
how Android detects big.LITTLE and will leave that to person who knows more.

> 
> I would like to introduce a tie between the warning message and the
> documentation provided. The tie doesn't have to be around hmp-unsafe:
> the intention wasn't really to provide a shortcut to do more damage, but
> rather to inform about the situation. In fact, I didn't mean to
> recommend the usage of hmp-unsafe.

In that case we should make clear hmp-unsafe is not recommended. When I 
read the suggested paragraph for the command line option, it feels that 
it is safe to use it if you pin your domain on big cores only.

> 
> Maybe we could say:
> 
>    See big.LITTLE under https://xenbits.xen.org/docs/unstable/misc/xen-command-line.html
> 
> or create docs/mics/arm/big.LITTLE ?

A doc would make more sense over a long paragraph in the xen-command-line.

But if we decide to no recommend the usage of hmp-unsafe (as you 
suggested above), then we should avoid to tease the user with that. 
Anyone caring enough about big.LITTLE could find easily a documentation 
to enable it on Xen.

Cheers,

On Fri, 16 Feb 2018, Julien Grall wrote:
> On 16/02/2018 21:15, Stefano Stabellini wrote:
> > On Fri, 16 Feb 2018, Julien Grall wrote:
> > > On 16/02/2018 20:50, Stefano Stabellini wrote:
> > > > On Fri, 16 Feb 2018, Julien Grall wrote:
> > > > > Hi Stefano,
> > > > > 
> > > > > On 15/02/18 23:17, Stefano Stabellini wrote:
> > > > > > Update the documentation of the hmp-unsafe option to explain how to
> > > > > > use
> > > > > > it safely, together with the right cpu affinity setting, on
> > > > > > big.LITTLE
> > > > > > systems.
> > > > > > 
> > > > > > Also update the warning message to point users to the docs.
> > > > > > 
> > > > > > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > > > > > CC: jbeulich@suse.com
> > > > > > CC: konrad.wilk@oracle.com
> > > > > > CC: tim@xen.org
> > > > > > CC: wei.liu2@citrix.com
> > > > > > CC: andrew.cooper3@citrix.com
> > > > > > CC: George.Dunlap@eu.citrix.com
> > > > > > CC: ian.jackson@eu.citrix.com
> > > > > > 
> > > > > > ---
> > > > > >     docs/misc/xen-command-line.markdown | 10 +++++++++-
> > > > > >     xen/arch/arm/smpboot.c              |  9 +++++----
> > > > > >     2 files changed, 14 insertions(+), 5 deletions(-)
> > > > > > 
> > > > > > diff --git a/docs/misc/xen-command-line.markdown
> > > > > > b/docs/misc/xen-command-line.markdown
> > > > > > index 2184cb9..a1ebeea 100644
> > > > > > --- a/docs/misc/xen-command-line.markdown
> > > > > > +++ b/docs/misc/xen-command-line.markdown
> > > > > > @@ -1007,7 +1007,15 @@ Control Xens use of the APEI Hardware Error
> > > > > > Source
> > > > > > Table, should one be found.
> > > > > >       Say yes at your own risk if you want to enable heterogenous
> > > > > > computing
> > > > > >     (such as big.LITTLE). This may result to an unstable and
> > > > > > insecure
> > > > > > -platform. When the option is disabled (default), CPUs that are not
> > > > > > +platform, unless you manually specify the cpu affinity of all
> > > > > > domains
> > > > > > so
> > > > > > +that all vcpus are scheduled on the same class of pcpus (big or
> > > > > > LITTLE
> > > > > > +but not both). vcpu migration between big cores and LITTLE cores is
> > > > > > not
> > > > > > +supported. Thus, if the first 4 pcpus are big and the last 4 are
> > > > > > LITTLE,
> > > > > > +all domains need to have either cpus = "0-3" or cpus = "4-7" in
> > > > > > their
> > > > > > VM
> > > > > > +config. Moreover, dom0_vcpus_pin needs to be passed on the Xen
> > > > > > command
> > > > > > +line.
> > > > > 
> > > > > In your example here you suggest to have all the vCPUs of a guest to
> > > > > either on
> > > > > big or LITTLE cores. How about giving an example where the guest can
> > > > > have
> > > > > 2
> > > > > LITTLE vCPUs and one big vCPU?
> > > > 
> > > > I would rather discourage it at the moment, given that it requires more
> > > > complex cpu affinity settings, or vcpu pinning. Also, I am afraid that
> > > > without matching corresponding topology information on the guest device
> > > > tree, guests might not work as expected in such a scenario.
> > > > 
> > > > What do you think?
> > > 
> > > You already know my view on this. I would rather strongly discourage
> > > anyone
> > > pinning all vCPUs of a domain to big cores. We should avoid to provide
> > > shortcuts to use that could have potentially damageable impact on their
> > > platform without telling them.
> > 
> > Do you have a link to a doc somewhere that provides more details about
> > this? We could add a link to it here to inform users. It would be
> > useful.
> 
> This is quite well described in
> https://xenbits.xen.org/docs/unstable/man/xl.cfg.5.html#CPU-Allocation see
> "cpus".

OK, I'll add the link in a new big.LITTLE doc. Also, do you have any
documentation or link about big core being potentially damaging? It
would be good to provide information about that too in the big.LITTLE
doc.


> > > > I see. What about:
> > > > 
> > > >     See docs/misc/xen-command-line.markdown:hmp-unsafe
> > > > 
> > > > Or:
> > > > 
> > > >     See hmp-unsafe under
> > > > https://xenbits.xen.org/docs/unstable/misc/xen-command-line.html
> > > 
> > > I believe that people looking at big.LITTLE (and really want it) are smart
> > > enough to look at the docs or the code themselves. Given how fragile is
> > > your
> > > solution, I would rather avoid to help people doing bad thing.
> > 
> > People might know very well the hardware, and have very detailed
> > information about big.LITTLE and their platform, but might not know that
> > much about Xen.
> 
> We also might have student who wants to try Xen on their platform and have no
> clue how it works. So we want to provide safe information here and avoid to
> induce pinning on big cores is always safe.
> 
> I think with the tools we have, it is not very difficult provide a quick tuto
> how to do big.LITTLE in Xen. Obviously this will not be as easy as the
> solution suggested by Dario in its design document.
> 
> The most important bits is libxl provide an extensive way to set CPUs
> affinity. OS such as Linux will be able to deal with big.LITTLE even without
> DT thought scheduling might not be that good... I have no idea how Android
> detects big.LITTLE and will leave that to person who knows more.

Sure, I'll write a new doc.


> > I would like to introduce a tie between the warning message and the
> > documentation provided. The tie doesn't have to be around hmp-unsafe:
> > the intention wasn't really to provide a shortcut to do more damage, but
> > rather to inform about the situation. In fact, I didn't mean to
> > recommend the usage of hmp-unsafe.
> 
> In that case we should make clear hmp-unsafe is not recommended. When I read
> the suggested paragraph for the command line option, it feels that it is safe
> to use it if you pin your domain on big cores only.

I'll make it clearer that hmp-unsafe is not recommended. I'll completely
separate the big.LITTLE information doc from hmp-unsafe.


> > Maybe we could say:
> > 
> >    See big.LITTLE under
> > https://xenbits.xen.org/docs/unstable/misc/xen-command-line.html
> > 
> > or create docs/mics/arm/big.LITTLE ?
> 
> A doc would make more sense over a long paragraph in the xen-command-line.
> 
> But if we decide to no recommend the usage of hmp-unsafe (as you suggested
> above), then we should avoid to tease the user with that. Anyone caring enough
> about big.LITTLE could find easily a documentation to enable it on Xen.

On Fri, 16 Feb 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 15/02/18 23:16, Stefano Stabellini wrote:
> > On big.LITTLE systems not all cores have the same ACTLR. Instead of
> > reading ACTLR and setting v->arch.actlr in vcpu_initialise, which is run
> > always on pcpu 0, do it later on the same pcpu where the vcpu will run.
> 
> While the Hardware domain vCPUs will always be created on pCPU 0, this may not
> be the case for other domain as it is done via an hypercall.

I'll reword


> > 
> > This way, assuming that the vcpu has been created with the right pcpu
> > affinity, the guest will be able to read the right ACTLR value, matching
> > the one of the physical cpu.
> > 
> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > ---
> >   xen/arch/arm/domain.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> > index a010443..532e824 100644
> > --- a/xen/arch/arm/domain.c
> > +++ b/xen/arch/arm/domain.c
> > @@ -314,6 +314,8 @@ static void schedule_tail(struct vcpu *prev)
> >     static void continue_new_vcpu(struct vcpu *prev)
> >   {
> > +    current->arch.actlr = READ_SYSREG32(ACTLR_EL1);
> 
> This is not enough, you also need to move the call of
> processor_vcpu_initialise here. For instance on Cortex-A15, this will
> set/clear the SMP bit in ACTLR.

Right! I'll make the change.


> Also, I just noticed that processor_vcpu_initialize (in arch/arm/processor.c)
> assumes all the processor will be the same.  We probably want to turn the
> variable processor into a per-cpu value.

Good point, I'll add a patch for that


> > +
> >       schedule_tail(prev);
> >         if ( is_idle_vcpu(current) )
> > @@ -540,8 +542,6 @@ int vcpu_initialise(struct vcpu *v)
> >         v->arch.vmpidr = MPIDR_SMP | vcpuid_to_vaffinity(v->vcpu_id);
> >   -    v->arch.actlr = READ_SYSREG32(ACTLR_EL1);
> > -
> >       v->arch.hcr_el2 = get_default_hcr_flags();
> >         processor_vcpu_initialise(v);
> > 
> 
> Cheers,
> 
> -- 
> Julien Grall
>

Hi,

On 17/02/2018 00:31, Stefano Stabellini wrote:
> On Fri, 16 Feb 2018, Julien Grall wrote:
>> On 16/02/2018 21:15, Stefano Stabellini wrote:
>>> On Fri, 16 Feb 2018, Julien Grall wrote:
>>>> On 16/02/2018 20:50, Stefano Stabellini wrote:
>>>>> On Fri, 16 Feb 2018, Julien Grall wrote:
>>>>>> Hi Stefano,
>>>>>>
>>>>>> On 15/02/18 23:17, Stefano Stabellini wrote:
>>>>>>> Update the documentation of the hmp-unsafe option to explain how to
>>>>>>> use
>>>>>>> it safely, together with the right cpu affinity setting, on
>>>>>>> big.LITTLE
>>>>>>> systems.
>>>>>>>
>>>>>>> Also update the warning message to point users to the docs.
>>>>>>>
>>>>>>> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
>>>>>>> CC: jbeulich@suse.com
>>>>>>> CC: konrad.wilk@oracle.com
>>>>>>> CC: tim@xen.org
>>>>>>> CC: wei.liu2@citrix.com
>>>>>>> CC: andrew.cooper3@citrix.com
>>>>>>> CC: George.Dunlap@eu.citrix.com
>>>>>>> CC: ian.jackson@eu.citrix.com
>>>>>>>
>>>>>>> ---
>>>>>>>      docs/misc/xen-command-line.markdown | 10 +++++++++-
>>>>>>>      xen/arch/arm/smpboot.c              |  9 +++++----
>>>>>>>      2 files changed, 14 insertions(+), 5 deletions(-)
>>>>>>>
>>>>>>> diff --git a/docs/misc/xen-command-line.markdown
>>>>>>> b/docs/misc/xen-command-line.markdown
>>>>>>> index 2184cb9..a1ebeea 100644
>>>>>>> --- a/docs/misc/xen-command-line.markdown
>>>>>>> +++ b/docs/misc/xen-command-line.markdown
>>>>>>> @@ -1007,7 +1007,15 @@ Control Xens use of the APEI Hardware Error
>>>>>>> Source
>>>>>>> Table, should one be found.
>>>>>>>        Say yes at your own risk if you want to enable heterogenous
>>>>>>> computing
>>>>>>>      (such as big.LITTLE). This may result to an unstable and
>>>>>>> insecure
>>>>>>> -platform. When the option is disabled (default), CPUs that are not
>>>>>>> +platform, unless you manually specify the cpu affinity of all
>>>>>>> domains
>>>>>>> so
>>>>>>> +that all vcpus are scheduled on the same class of pcpus (big or
>>>>>>> LITTLE
>>>>>>> +but not both). vcpu migration between big cores and LITTLE cores is
>>>>>>> not
>>>>>>> +supported. Thus, if the first 4 pcpus are big and the last 4 are
>>>>>>> LITTLE,
>>>>>>> +all domains need to have either cpus = "0-3" or cpus = "4-7" in
>>>>>>> their
>>>>>>> VM
>>>>>>> +config. Moreover, dom0_vcpus_pin needs to be passed on the Xen
>>>>>>> command
>>>>>>> +line.
>>>>>>
>>>>>> In your example here you suggest to have all the vCPUs of a guest to
>>>>>> either on
>>>>>> big or LITTLE cores. How about giving an example where the guest can
>>>>>> have
>>>>>> 2
>>>>>> LITTLE vCPUs and one big vCPU?
>>>>>
>>>>> I would rather discourage it at the moment, given that it requires more
>>>>> complex cpu affinity settings, or vcpu pinning. Also, I am afraid that
>>>>> without matching corresponding topology information on the guest device
>>>>> tree, guests might not work as expected in such a scenario.
>>>>>
>>>>> What do you think?
>>>>
>>>> You already know my view on this. I would rather strongly discourage
>>>> anyone
>>>> pinning all vCPUs of a domain to big cores. We should avoid to provide
>>>> shortcuts to use that could have potentially damageable impact on their
>>>> platform without telling them.
>>>
>>> Do you have a link to a doc somewhere that provides more details about
>>> this? We could add a link to it here to inform users. It would be
>>> useful.
>>
>> This is quite well described in
>> https://xenbits.xen.org/docs/unstable/man/xl.cfg.5.html#CPU-Allocation see
>> "cpus".
> 
> OK, I'll add the link in a new big.LITTLE doc. Also, do you have any
> documentation or link about big core being potentially damaging? It
> would be good to provide information about that too in the big.LITTLE
> doc.

I don't have specific documentation to point on it but I would quite 
interesting to know what is your documentation regarding why always 
running on big is safe. I provided you quite a few insights why this may 
not safe on all platforms and we all remember those phones burning you 
when playing game or watching a video. So I don't feel Xen Project 
should encourage those setups by default.

I would recommend you to read the thread about big.LITTLE in Xen from 
2016: 
https://lists.xenproject.org/archives/html/xen-devel/2016-09/msg01802.html

A few interesting things from that conversation:

"big.LITTLE is a generic term to have 'power hungry and powerful core 
powerful' (big) with slower and battery-saving cores (LITTLE)."

"The use case of big.LITTLE is big cores are used for short period of 
burst and little core are used for the rest (e.g listening audio, 
fetching mail...). If you want to reduce latency when switch between big 
and little CPUs, you may want to put them within the same cluster."

Cheers,

On Sat, 17 Feb 2018, Julien Grall wrote:
> Hi,
> 
> On 17/02/2018 00:31, Stefano Stabellini wrote:
> > On Fri, 16 Feb 2018, Julien Grall wrote:
> > > On 16/02/2018 21:15, Stefano Stabellini wrote:
> > > > On Fri, 16 Feb 2018, Julien Grall wrote:
> > > > > On 16/02/2018 20:50, Stefano Stabellini wrote:
> > > > > > On Fri, 16 Feb 2018, Julien Grall wrote:
> > > > > > > Hi Stefano,
> > > > > > > 
> > > > > > > On 15/02/18 23:17, Stefano Stabellini wrote:
> > > > > > > > Update the documentation of the hmp-unsafe option to explain how
> > > > > > > > to
> > > > > > > > use
> > > > > > > > it safely, together with the right cpu affinity setting, on
> > > > > > > > big.LITTLE
> > > > > > > > systems.
> > > > > > > > 
> > > > > > > > Also update the warning message to point users to the docs.
> > > > > > > > 
> > > > > > > > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > > > > > > > CC: jbeulich@suse.com
> > > > > > > > CC: konrad.wilk@oracle.com
> > > > > > > > CC: tim@xen.org
> > > > > > > > CC: wei.liu2@citrix.com
> > > > > > > > CC: andrew.cooper3@citrix.com
> > > > > > > > CC: George.Dunlap@eu.citrix.com
> > > > > > > > CC: ian.jackson@eu.citrix.com
> > > > > > > > 
> > > > > > > > ---
> > > > > > > >      docs/misc/xen-command-line.markdown | 10 +++++++++-
> > > > > > > >      xen/arch/arm/smpboot.c              |  9 +++++----
> > > > > > > >      2 files changed, 14 insertions(+), 5 deletions(-)
> > > > > > > > 
> > > > > > > > diff --git a/docs/misc/xen-command-line.markdown
> > > > > > > > b/docs/misc/xen-command-line.markdown
> > > > > > > > index 2184cb9..a1ebeea 100644
> > > > > > > > --- a/docs/misc/xen-command-line.markdown
> > > > > > > > +++ b/docs/misc/xen-command-line.markdown
> > > > > > > > @@ -1007,7 +1007,15 @@ Control Xens use of the APEI Hardware
> > > > > > > > Error
> > > > > > > > Source
> > > > > > > > Table, should one be found.
> > > > > > > >        Say yes at your own risk if you want to enable
> > > > > > > > heterogenous
> > > > > > > > computing
> > > > > > > >      (such as big.LITTLE). This may result to an unstable and
> > > > > > > > insecure
> > > > > > > > -platform. When the option is disabled (default), CPUs that are
> > > > > > > > not
> > > > > > > > +platform, unless you manually specify the cpu affinity of all
> > > > > > > > domains
> > > > > > > > so
> > > > > > > > +that all vcpus are scheduled on the same class of pcpus (big or
> > > > > > > > LITTLE
> > > > > > > > +but not both). vcpu migration between big cores and LITTLE
> > > > > > > > cores is
> > > > > > > > not
> > > > > > > > +supported. Thus, if the first 4 pcpus are big and the last 4
> > > > > > > > are
> > > > > > > > LITTLE,
> > > > > > > > +all domains need to have either cpus = "0-3" or cpus = "4-7" in
> > > > > > > > their
> > > > > > > > VM
> > > > > > > > +config. Moreover, dom0_vcpus_pin needs to be passed on the Xen
> > > > > > > > command
> > > > > > > > +line.
> > > > > > > 
> > > > > > > In your example here you suggest to have all the vCPUs of a guest
> > > > > > > to
> > > > > > > either on
> > > > > > > big or LITTLE cores. How about giving an example where the guest
> > > > > > > can
> > > > > > > have
> > > > > > > 2
> > > > > > > LITTLE vCPUs and one big vCPU?
> > > > > > 
> > > > > > I would rather discourage it at the moment, given that it requires
> > > > > > more
> > > > > > complex cpu affinity settings, or vcpu pinning. Also, I am afraid
> > > > > > that
> > > > > > without matching corresponding topology information on the guest
> > > > > > device
> > > > > > tree, guests might not work as expected in such a scenario.
> > > > > > 
> > > > > > What do you think?
> > > > > 
> > > > > You already know my view on this. I would rather strongly discourage
> > > > > anyone
> > > > > pinning all vCPUs of a domain to big cores. We should avoid to provide
> > > > > shortcuts to use that could have potentially damageable impact on
> > > > > their
> > > > > platform without telling them.
> > > > 
> > > > Do you have a link to a doc somewhere that provides more details about
> > > > this? We could add a link to it here to inform users. It would be
> > > > useful.
> > > 
> > > This is quite well described in
> > > https://xenbits.xen.org/docs/unstable/man/xl.cfg.5.html#CPU-Allocation see
> > > "cpus".
> > 
> > OK, I'll add the link in a new big.LITTLE doc. Also, do you have any
> > documentation or link about big core being potentially damaging? It
> > would be good to provide information about that too in the big.LITTLE
> > doc.
> 
> I don't have specific documentation to point on it but I would quite
> interesting to know what is your documentation regarding why always running on
> big is safe. 

Hi Julien,

If I go on Amazon, I buy a big.LITTLE board, then it overheats and
breaks due to the big cores, I would call this a malfunction and return
the item expecting a refund.

Unless the hardware vendor states explicitly that the big cores cannot
be used all the time, then this use-case falls within the reasonable
usage of the platform. In fact, it is using a piece of the hardware the
way it was designed to be used. If the hardware itself is unstable, it
should be documented in the vendor's docs, and I would like to add a
link to it so that users are appropriately warned.


> I provided you quite a few insights why this may not safe on all
> platforms and we all remember those phones burning you when playing game or
> watching a video. So I don't feel Xen Project should encourage those setups by
> default.
> 
> I would recommend you to read the thread about big.LITTLE in Xen from 2016:
> https://lists.xenproject.org/archives/html/xen-devel/2016-09/msg01802.html
> 
> A few interesting things from that conversation:
> 
> "big.LITTLE is a generic term to have 'power hungry and powerful core
> powerful' (big) with slower and battery-saving cores (LITTLE)."
> 
> "The use case of big.LITTLE is big cores are used for short period of burst
> and little core are used for the rest (e.g listening audio, fetching mail...).
> If you want to reduce latency when switch between big and little CPUs, you may
> want to put them within the same cluster."

These two sentences are good, and I copy/paste them into the new doc,
but still they don't clarify the safety of the big cores usage.

On 19/02/2018 20:28, Stefano Stabellini wrote:
> On Sat, 17 Feb 2018, Julien Grall wrote:
>> Hi,
>>
>> On 17/02/2018 00:31, Stefano Stabellini wrote:
>>> On Fri, 16 Feb 2018, Julien Grall wrote:
>>>> On 16/02/2018 21:15, Stefano Stabellini wrote:
>>>>> On Fri, 16 Feb 2018, Julien Grall wrote:
>>>>>> On 16/02/2018 20:50, Stefano Stabellini wrote:
>>>>>>> On Fri, 16 Feb 2018, Julien Grall wrote:
>>>>>>>> Hi Stefano,
>>>>>>>>
>>>>>>>> On 15/02/18 23:17, Stefano Stabellini wrote:
>>>>>>>>> Update the documentation of the hmp-unsafe option to explain how
>>>>>>>>> to
>>>>>>>>> use
>>>>>>>>> it safely, together with the right cpu affinity setting, on
>>>>>>>>> big.LITTLE
>>>>>>>>> systems.
>>>>>>>>>
>>>>>>>>> Also update the warning message to point users to the docs.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
>>>>>>>>> CC: jbeulich@suse.com
>>>>>>>>> CC: konrad.wilk@oracle.com
>>>>>>>>> CC: tim@xen.org
>>>>>>>>> CC: wei.liu2@citrix.com
>>>>>>>>> CC: andrew.cooper3@citrix.com
>>>>>>>>> CC: George.Dunlap@eu.citrix.com
>>>>>>>>> CC: ian.jackson@eu.citrix.com
>>>>>>>>>
>>>>>>>>> ---
>>>>>>>>>       docs/misc/xen-command-line.markdown | 10 +++++++++-
>>>>>>>>>       xen/arch/arm/smpboot.c              |  9 +++++----
>>>>>>>>>       2 files changed, 14 insertions(+), 5 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/docs/misc/xen-command-line.markdown
>>>>>>>>> b/docs/misc/xen-command-line.markdown
>>>>>>>>> index 2184cb9..a1ebeea 100644
>>>>>>>>> --- a/docs/misc/xen-command-line.markdown
>>>>>>>>> +++ b/docs/misc/xen-command-line.markdown
>>>>>>>>> @@ -1007,7 +1007,15 @@ Control Xens use of the APEI Hardware
>>>>>>>>> Error
>>>>>>>>> Source
>>>>>>>>> Table, should one be found.
>>>>>>>>>         Say yes at your own risk if you want to enable
>>>>>>>>> heterogenous
>>>>>>>>> computing
>>>>>>>>>       (such as big.LITTLE). This may result to an unstable and
>>>>>>>>> insecure
>>>>>>>>> -platform. When the option is disabled (default), CPUs that are
>>>>>>>>> not
>>>>>>>>> +platform, unless you manually specify the cpu affinity of all
>>>>>>>>> domains
>>>>>>>>> so
>>>>>>>>> +that all vcpus are scheduled on the same class of pcpus (big or
>>>>>>>>> LITTLE
>>>>>>>>> +but not both). vcpu migration between big cores and LITTLE
>>>>>>>>> cores is
>>>>>>>>> not
>>>>>>>>> +supported. Thus, if the first 4 pcpus are big and the last 4
>>>>>>>>> are
>>>>>>>>> LITTLE,
>>>>>>>>> +all domains need to have either cpus = "0-3" or cpus = "4-7" in
>>>>>>>>> their
>>>>>>>>> VM
>>>>>>>>> +config. Moreover, dom0_vcpus_pin needs to be passed on the Xen
>>>>>>>>> command
>>>>>>>>> +line.
>>>>>>>>
>>>>>>>> In your example here you suggest to have all the vCPUs of a guest
>>>>>>>> to
>>>>>>>> either on
>>>>>>>> big or LITTLE cores. How about giving an example where the guest
>>>>>>>> can
>>>>>>>> have
>>>>>>>> 2
>>>>>>>> LITTLE vCPUs and one big vCPU?
>>>>>>>
>>>>>>> I would rather discourage it at the moment, given that it requires
>>>>>>> more
>>>>>>> complex cpu affinity settings, or vcpu pinning. Also, I am afraid
>>>>>>> that
>>>>>>> without matching corresponding topology information on the guest
>>>>>>> device
>>>>>>> tree, guests might not work as expected in such a scenario.
>>>>>>>
>>>>>>> What do you think?
>>>>>>
>>>>>> You already know my view on this. I would rather strongly discourage
>>>>>> anyone
>>>>>> pinning all vCPUs of a domain to big cores. We should avoid to provide
>>>>>> shortcuts to use that could have potentially damageable impact on
>>>>>> their
>>>>>> platform without telling them.
>>>>>
>>>>> Do you have a link to a doc somewhere that provides more details about
>>>>> this? We could add a link to it here to inform users. It would be
>>>>> useful.
>>>>
>>>> This is quite well described in
>>>> https://xenbits.xen.org/docs/unstable/man/xl.cfg.5.html#CPU-Allocation see
>>>> "cpus".
>>>
>>> OK, I'll add the link in a new big.LITTLE doc. Also, do you have any
>>> documentation or link about big core being potentially damaging? It
>>> would be good to provide information about that too in the big.LITTLE
>>> doc.
>>
>> I don't have specific documentation to point on it but I would quite
>> interesting to know what is your documentation regarding why always running on
>> big is safe.
> 
> Hi Julien,
> 
> If I go on Amazon, I buy a big.LITTLE board, then it overheats and
> breaks due to the big cores, I would call this a malfunction and return
> the item expecting a refund.

Why would they refund you? You run software that has not been proofed 
with their board. The vendor will nicely tell you to look for another 
software and will not give you the refund.

> 
> Unless the hardware vendor states explicitly that the big cores cannot
> be used all the time, then this use-case falls within the reasonable
> usage of the platform. In fact, it is using a piece of the hardware the
> way it was designed to be used. If the hardware itself is unstable, it
> should be documented in the vendor's docs, and I would like to add a
> link to it so that users are appropriately warned.
In normal circumstance, you have software controlling the overheat. But 
in case of Xen who is going to do that job? If it is the firmware and 
assuming it does not need to be taught, then this is likely going to 
work out-of-box with Xen. If it is the OS/Hypervisor, then you are going 
to get into trouble.

As you can see we already have different expectation on how the hardware 
should behave.

> 
> 
>> I provided you quite a few insights why this may not safe on all
>> platforms and we all remember those phones burning you when playing game or
>> watching a video. So I don't feel Xen Project should encourage those setups by
>> default.
>>
>> I would recommend you to read the thread about big.LITTLE in Xen from 2016:
>> https://lists.xenproject.org/archives/html/xen-devel/2016-09/msg01802.html
>>
>> A few interesting things from that conversation:
>>
>> "big.LITTLE is a generic term to have 'power hungry and powerful core
>> powerful' (big) with slower and battery-saving cores (LITTLE)."
>>
>> "The use case of big.LITTLE is big cores are used for short period of burst
>> and little core are used for the rest (e.g listening audio, fetching mail...).
>> If you want to reduce latency when switch between big and little CPUs, you may
>> want to put them within the same cluster."
> 
> These two sentences are good, and I copy/paste them into the new doc,
> but still they don't clarify the safety of the big cores usage.

You assume the software stack is correct. However, we clearly no that 
CPUFrequency/Power management on Xen Arm is not there... From that you 
can't even assume the basic functionality of the board will function 
properly when running Xen.

Cheers,

On Mon, 19 Feb 2018, Julien Grall wrote:
> On 19/02/2018 20:28, Stefano Stabellini wrote:
> > On Sat, 17 Feb 2018, Julien Grall wrote:
> > > Hi,
> > > 
> > > On 17/02/2018 00:31, Stefano Stabellini wrote:
> > > > On Fri, 16 Feb 2018, Julien Grall wrote:
> > > > > On 16/02/2018 21:15, Stefano Stabellini wrote:
> > > > > > On Fri, 16 Feb 2018, Julien Grall wrote:
> > > > > > > On 16/02/2018 20:50, Stefano Stabellini wrote:
> > > > > > > > On Fri, 16 Feb 2018, Julien Grall wrote:
> > > > > > > > > Hi Stefano,
> > > > > > > > > 
> > > > > > > > > On 15/02/18 23:17, Stefano Stabellini wrote:
> > > > > > > > > > Update the documentation of the hmp-unsafe option to explain
> > > > > > > > > > how
> > > > > > > > > > to
> > > > > > > > > > use
> > > > > > > > > > it safely, together with the right cpu affinity setting, on
> > > > > > > > > > big.LITTLE
> > > > > > > > > > systems.
> > > > > > > > > > 
> > > > > > > > > > Also update the warning message to point users to the docs.
> > > > > > > > > > 
> > > > > > > > > > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > > > > > > > > > CC: jbeulich@suse.com
> > > > > > > > > > CC: konrad.wilk@oracle.com
> > > > > > > > > > CC: tim@xen.org
> > > > > > > > > > CC: wei.liu2@citrix.com
> > > > > > > > > > CC: andrew.cooper3@citrix.com
> > > > > > > > > > CC: George.Dunlap@eu.citrix.com
> > > > > > > > > > CC: ian.jackson@eu.citrix.com
> > > > > > > > > > 
> > > > > > > > > > ---
> > > > > > > > > >       docs/misc/xen-command-line.markdown | 10 +++++++++-
> > > > > > > > > >       xen/arch/arm/smpboot.c              |  9 +++++----
> > > > > > > > > >       2 files changed, 14 insertions(+), 5 deletions(-)
> > > > > > > > > > 
> > > > > > > > > > diff --git a/docs/misc/xen-command-line.markdown
> > > > > > > > > > b/docs/misc/xen-command-line.markdown
> > > > > > > > > > index 2184cb9..a1ebeea 100644
> > > > > > > > > > --- a/docs/misc/xen-command-line.markdown
> > > > > > > > > > +++ b/docs/misc/xen-command-line.markdown
> > > > > > > > > > @@ -1007,7 +1007,15 @@ Control Xens use of the APEI Hardware
> > > > > > > > > > Error
> > > > > > > > > > Source
> > > > > > > > > > Table, should one be found.
> > > > > > > > > >         Say yes at your own risk if you want to enable
> > > > > > > > > > heterogenous
> > > > > > > > > > computing
> > > > > > > > > >       (such as big.LITTLE). This may result to an unstable
> > > > > > > > > > and
> > > > > > > > > > insecure
> > > > > > > > > > -platform. When the option is disabled (default), CPUs that
> > > > > > > > > > are
> > > > > > > > > > not
> > > > > > > > > > +platform, unless you manually specify the cpu affinity of
> > > > > > > > > > all
> > > > > > > > > > domains
> > > > > > > > > > so
> > > > > > > > > > +that all vcpus are scheduled on the same class of pcpus
> > > > > > > > > > (big or
> > > > > > > > > > LITTLE
> > > > > > > > > > +but not both). vcpu migration between big cores and LITTLE
> > > > > > > > > > cores is
> > > > > > > > > > not
> > > > > > > > > > +supported. Thus, if the first 4 pcpus are big and the last
> > > > > > > > > > 4
> > > > > > > > > > are
> > > > > > > > > > LITTLE,
> > > > > > > > > > +all domains need to have either cpus = "0-3" or cpus =
> > > > > > > > > > "4-7" in
> > > > > > > > > > their
> > > > > > > > > > VM
> > > > > > > > > > +config. Moreover, dom0_vcpus_pin needs to be passed on the
> > > > > > > > > > Xen
> > > > > > > > > > command
> > > > > > > > > > +line.
> > > > > > > > > 
> > > > > > > > > In your example here you suggest to have all the vCPUs of a
> > > > > > > > > guest
> > > > > > > > > to
> > > > > > > > > either on
> > > > > > > > > big or LITTLE cores. How about giving an example where the
> > > > > > > > > guest
> > > > > > > > > can
> > > > > > > > > have
> > > > > > > > > 2
> > > > > > > > > LITTLE vCPUs and one big vCPU?
> > > > > > > > 
> > > > > > > > I would rather discourage it at the moment, given that it
> > > > > > > > requires
> > > > > > > > more
> > > > > > > > complex cpu affinity settings, or vcpu pinning. Also, I am
> > > > > > > > afraid
> > > > > > > > that
> > > > > > > > without matching corresponding topology information on the guest
> > > > > > > > device
> > > > > > > > tree, guests might not work as expected in such a scenario.
> > > > > > > > 
> > > > > > > > What do you think?
> > > > > > > 
> > > > > > > You already know my view on this. I would rather strongly
> > > > > > > discourage
> > > > > > > anyone
> > > > > > > pinning all vCPUs of a domain to big cores. We should avoid to
> > > > > > > provide
> > > > > > > shortcuts to use that could have potentially damageable impact on
> > > > > > > their
> > > > > > > platform without telling them.
> > > > > > 
> > > > > > Do you have a link to a doc somewhere that provides more details
> > > > > > about
> > > > > > this? We could add a link to it here to inform users. It would be
> > > > > > useful.
> > > > > 
> > > > > This is quite well described in
> > > > > https://xenbits.xen.org/docs/unstable/man/xl.cfg.5.html#CPU-Allocation
> > > > > see
> > > > > "cpus".
> > > > 
> > > > OK, I'll add the link in a new big.LITTLE doc. Also, do you have any
> > > > documentation or link about big core being potentially damaging? It
> > > > would be good to provide information about that too in the big.LITTLE
> > > > doc.
> > > 
> > > I don't have specific documentation to point on it but I would quite
> > > interesting to know what is your documentation regarding why always
> > > running on
> > > big is safe.
> > 
> > Hi Julien,
> > 
> > If I go on Amazon, I buy a big.LITTLE board, then it overheats and
> > breaks due to the big cores, I would call this a malfunction and return
> > the item expecting a refund.
> 
> Why would they refund you? You run software that has not been proofed with
> their board. The vendor will nicely tell you to look for another software and
> will not give you the refund.

I guess it depends on the board. I don't think many dev boards (like
Pine64) require you to use a specific kernel version or hypevisor
version on them (I hope!).


> > Unless the hardware vendor states explicitly that the big cores cannot
> > be used all the time, then this use-case falls within the reasonable
> > usage of the platform. In fact, it is using a piece of the hardware the
> > way it was designed to be used. If the hardware itself is unstable, it
> > should be documented in the vendor's docs, and I would like to add a
> > link to it so that users are appropriately warned.
> In normal circumstance, you have software controlling the overheat. But in
> case of Xen who is going to do that job? If it is the firmware and assuming it
> does not need to be taught, then this is likely going to work out-of-box with
> Xen. If it is the OS/Hypervisor, then you are going to get into trouble.
> 
> As you can see we already have different expectation on how the hardware
> should behave.

I am starting to see your point. What if we add the following statement,
it should non-controversial and informative:

"Big cores are more powerful than LITTLE cores, but often use much more
power. Typically, they are recommended for burst activity, especially in
battery powered environments. Please check your vendor's big.LITTLE
and power management documentation."

As I was thinking about this, I realized that the same issue could occur
even with just the first patch
(https://marc.info/?l=xen-devel&m=151873668223723). If the first cpu
type is big, we would default to use only big cpus all the time, right?

On 19/02/2018 21:05, Stefano Stabellini wrote:
> On Mon, 19 Feb 2018, Julien Grall wrote:
>> On 19/02/2018 20:28, Stefano Stabellini wrote:
>>> On Sat, 17 Feb 2018, Julien Grall wrote:
>>>> Hi,
>>>>
>>>> On 17/02/2018 00:31, Stefano Stabellini wrote:
>>>>> On Fri, 16 Feb 2018, Julien Grall wrote:
>>>>>> On 16/02/2018 21:15, Stefano Stabellini wrote:
>>>>>>> On Fri, 16 Feb 2018, Julien Grall wrote:
>>>>>>>> On 16/02/2018 20:50, Stefano Stabellini wrote:
>>>>>>>>> On Fri, 16 Feb 2018, Julien Grall wrote:
>>>>>>>>>> Hi Stefano,
>>>>>>>>>>
>>>>>>>>>> On 15/02/18 23:17, Stefano Stabellini wrote:
>>>>>>>>>>> Update the documentation of the hmp-unsafe option to explain
>>>>>>>>>>> how
>>>>>>>>>>> to
>>>>>>>>>>> use
>>>>>>>>>>> it safely, together with the right cpu affinity setting, on
>>>>>>>>>>> big.LITTLE
>>>>>>>>>>> systems.
>>>>>>>>>>>
>>>>>>>>>>> Also update the warning message to point users to the docs.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
>>>>>>>>>>> CC: jbeulich@suse.com
>>>>>>>>>>> CC: konrad.wilk@oracle.com
>>>>>>>>>>> CC: tim@xen.org
>>>>>>>>>>> CC: wei.liu2@citrix.com
>>>>>>>>>>> CC: andrew.cooper3@citrix.com
>>>>>>>>>>> CC: George.Dunlap@eu.citrix.com
>>>>>>>>>>> CC: ian.jackson@eu.citrix.com
>>>>>>>>>>>
>>>>>>>>>>> ---
>>>>>>>>>>>        docs/misc/xen-command-line.markdown | 10 +++++++++-
>>>>>>>>>>>        xen/arch/arm/smpboot.c              |  9 +++++----
>>>>>>>>>>>        2 files changed, 14 insertions(+), 5 deletions(-)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/docs/misc/xen-command-line.markdown
>>>>>>>>>>> b/docs/misc/xen-command-line.markdown
>>>>>>>>>>> index 2184cb9..a1ebeea 100644
>>>>>>>>>>> --- a/docs/misc/xen-command-line.markdown
>>>>>>>>>>> +++ b/docs/misc/xen-command-line.markdown
>>>>>>>>>>> @@ -1007,7 +1007,15 @@ Control Xens use of the APEI Hardware
>>>>>>>>>>> Error
>>>>>>>>>>> Source
>>>>>>>>>>> Table, should one be found.
>>>>>>>>>>>          Say yes at your own risk if you want to enable
>>>>>>>>>>> heterogenous
>>>>>>>>>>> computing
>>>>>>>>>>>        (such as big.LITTLE). This may result to an unstable
>>>>>>>>>>> and
>>>>>>>>>>> insecure
>>>>>>>>>>> -platform. When the option is disabled (default), CPUs that
>>>>>>>>>>> are
>>>>>>>>>>> not
>>>>>>>>>>> +platform, unless you manually specify the cpu affinity of
>>>>>>>>>>> all
>>>>>>>>>>> domains
>>>>>>>>>>> so
>>>>>>>>>>> +that all vcpus are scheduled on the same class of pcpus
>>>>>>>>>>> (big or
>>>>>>>>>>> LITTLE
>>>>>>>>>>> +but not both). vcpu migration between big cores and LITTLE
>>>>>>>>>>> cores is
>>>>>>>>>>> not
>>>>>>>>>>> +supported. Thus, if the first 4 pcpus are big and the last
>>>>>>>>>>> 4
>>>>>>>>>>> are
>>>>>>>>>>> LITTLE,
>>>>>>>>>>> +all domains need to have either cpus = "0-3" or cpus =
>>>>>>>>>>> "4-7" in
>>>>>>>>>>> their
>>>>>>>>>>> VM
>>>>>>>>>>> +config. Moreover, dom0_vcpus_pin needs to be passed on the
>>>>>>>>>>> Xen
>>>>>>>>>>> command
>>>>>>>>>>> +line.
>>>>>>>>>>
>>>>>>>>>> In your example here you suggest to have all the vCPUs of a
>>>>>>>>>> guest
>>>>>>>>>> to
>>>>>>>>>> either on
>>>>>>>>>> big or LITTLE cores. How about giving an example where the
>>>>>>>>>> guest
>>>>>>>>>> can
>>>>>>>>>> have
>>>>>>>>>> 2
>>>>>>>>>> LITTLE vCPUs and one big vCPU?
>>>>>>>>>
>>>>>>>>> I would rather discourage it at the moment, given that it
>>>>>>>>> requires
>>>>>>>>> more
>>>>>>>>> complex cpu affinity settings, or vcpu pinning. Also, I am
>>>>>>>>> afraid
>>>>>>>>> that
>>>>>>>>> without matching corresponding topology information on the guest
>>>>>>>>> device
>>>>>>>>> tree, guests might not work as expected in such a scenario.
>>>>>>>>>
>>>>>>>>> What do you think?
>>>>>>>>
>>>>>>>> You already know my view on this. I would rather strongly
>>>>>>>> discourage
>>>>>>>> anyone
>>>>>>>> pinning all vCPUs of a domain to big cores. We should avoid to
>>>>>>>> provide
>>>>>>>> shortcuts to use that could have potentially damageable impact on
>>>>>>>> their
>>>>>>>> platform without telling them.
>>>>>>>
>>>>>>> Do you have a link to a doc somewhere that provides more details
>>>>>>> about
>>>>>>> this? We could add a link to it here to inform users. It would be
>>>>>>> useful.
>>>>>>
>>>>>> This is quite well described in
>>>>>> https://xenbits.xen.org/docs/unstable/man/xl.cfg.5.html#CPU-Allocation
>>>>>> see
>>>>>> "cpus".
>>>>>
>>>>> OK, I'll add the link in a new big.LITTLE doc. Also, do you have any
>>>>> documentation or link about big core being potentially damaging? It
>>>>> would be good to provide information about that too in the big.LITTLE
>>>>> doc.
>>>>
>>>> I don't have specific documentation to point on it but I would quite
>>>> interesting to know what is your documentation regarding why always
>>>> running on
>>>> big is safe.
>>>
>>> Hi Julien,
>>>
>>> If I go on Amazon, I buy a big.LITTLE board, then it overheats and
>>> breaks due to the big cores, I would call this a malfunction and return
>>> the item expecting a refund.
>>
>> Why would they refund you? You run software that has not been proofed with
>> their board. The vendor will nicely tell you to look for another software and
>> will not give you the refund.
> 
> I guess it depends on the board. I don't think many dev boards (like
> Pine64) require you to use a specific kernel version or hypevisor
> version on them (I hope!).

The problem I saw is some vendor decided to offload some firmware tasks 
to the Operating System. This means that Xen needs to handle those 
drivers in order to get full support of the board.

> 
> 
>>> Unless the hardware vendor states explicitly that the big cores cannot
>>> be used all the time, then this use-case falls within the reasonable
>>> usage of the platform. In fact, it is using a piece of the hardware the
>>> way it was designed to be used. If the hardware itself is unstable, it
>>> should be documented in the vendor's docs, and I would like to add a
>>> link to it so that users are appropriately warned.
>> In normal circumstance, you have software controlling the overheat. But in
>> case of Xen who is going to do that job? If it is the firmware and assuming it
>> does not need to be taught, then this is likely going to work out-of-box with
>> Xen. If it is the OS/Hypervisor, then you are going to get into trouble.
>>
>> As you can see we already have different expectation on how the hardware
>> should behave.

Hmmm I should have finished that paragraph. I meant that I would choose 
the more conservative way in the documentation because I would not 
assume we have the full stack working on Xen nowadays. If someone knows 
her platform is fine to always run on big cores, then it can still do it.

What I want to avoid is providing a way that we are not 100% sure will 
work on all the platforms.

> 
> I am starting to see your point. What if we add the following statement,
> it should non-controversial and informative:
> 
> "Big cores are more powerful than LITTLE cores, but often use much more
> power. Typically, they are recommended for burst activity, especially in
> battery powered environments. Please check your vendor's big.LITTLE
> and power management documentation."

Sounds good to me.

> 
> As I was thinking about this, I realized that the same issue could occur
> even with just the first patch
> (https://marc.info/?l=xen-devel&m=151873668223723). If the first cpu
> type is big, we would default to use only big cpus all the time, right?

Hmmm you are right. However we have no easy way to know whether you boot 
on big or little CPUs :/. Shall we update the warning?

Cheers,

On Mon, 19 Feb 2018, Julien Grall wrote:
> On 19/02/2018 21:05, Stefano Stabellini wrote:
> > On Mon, 19 Feb 2018, Julien Grall wrote:
> > > On 19/02/2018 20:28, Stefano Stabellini wrote:
> > > > On Sat, 17 Feb 2018, Julien Grall wrote:
> > > > > Hi,
> > > > > 
> > > > > On 17/02/2018 00:31, Stefano Stabellini wrote:
> > > > > > On Fri, 16 Feb 2018, Julien Grall wrote:
> > > > > > > On 16/02/2018 21:15, Stefano Stabellini wrote:
> > > > > > > > On Fri, 16 Feb 2018, Julien Grall wrote:
> > > > > > > > > On 16/02/2018 20:50, Stefano Stabellini wrote:
> > > > > > > > > > On Fri, 16 Feb 2018, Julien Grall wrote:
> > > > > > > > > > > Hi Stefano,
> > > > > > > > > > > 
> > > > > > > > > > > On 15/02/18 23:17, Stefano Stabellini wrote:
> > > > > > > > > > > > Update the documentation of the hmp-unsafe option to
> > > > > > > > > > > > explain
> > > > > > > > > > > > how
> > > > > > > > > > > > to
> > > > > > > > > > > > use
> > > > > > > > > > > > it safely, together with the right cpu affinity setting,
> > > > > > > > > > > > on
> > > > > > > > > > > > big.LITTLE
> > > > > > > > > > > > systems.
> > > > > > > > > > > > 
> > > > > > > > > > > > Also update the warning message to point users to the
> > > > > > > > > > > > docs.
> > > > > > > > > > > > 
> > > > > > > > > > > > Signed-off-by: Stefano Stabellini
> > > > > > > > > > > > <sstabellini@kernel.org>
> > > > > > > > > > > > CC: jbeulich@suse.com
> > > > > > > > > > > > CC: konrad.wilk@oracle.com
> > > > > > > > > > > > CC: tim@xen.org
> > > > > > > > > > > > CC: wei.liu2@citrix.com
> > > > > > > > > > > > CC: andrew.cooper3@citrix.com
> > > > > > > > > > > > CC: George.Dunlap@eu.citrix.com
> > > > > > > > > > > > CC: ian.jackson@eu.citrix.com
> > > > > > > > > > > > 
> > > > > > > > > > > > ---
> > > > > > > > > > > >        docs/misc/xen-command-line.markdown | 10
> > > > > > > > > > > > +++++++++-
> > > > > > > > > > > >        xen/arch/arm/smpboot.c              |  9
> > > > > > > > > > > > +++++----
> > > > > > > > > > > >        2 files changed, 14 insertions(+), 5 deletions(-)
> > > > > > > > > > > > 
> > > > > > > > > > > > diff --git a/docs/misc/xen-command-line.markdown
> > > > > > > > > > > > b/docs/misc/xen-command-line.markdown
> > > > > > > > > > > > index 2184cb9..a1ebeea 100644
> > > > > > > > > > > > --- a/docs/misc/xen-command-line.markdown
> > > > > > > > > > > > +++ b/docs/misc/xen-command-line.markdown
> > > > > > > > > > > > @@ -1007,7 +1007,15 @@ Control Xens use of the APEI
> > > > > > > > > > > > Hardware
> > > > > > > > > > > > Error
> > > > > > > > > > > > Source
> > > > > > > > > > > > Table, should one be found.
> > > > > > > > > > > >          Say yes at your own risk if you want to enable
> > > > > > > > > > > > heterogenous
> > > > > > > > > > > > computing
> > > > > > > > > > > >        (such as big.LITTLE). This may result to an
> > > > > > > > > > > > unstable
> > > > > > > > > > > > and
> > > > > > > > > > > > insecure
> > > > > > > > > > > > -platform. When the option is disabled (default), CPUs
> > > > > > > > > > > > that
> > > > > > > > > > > > are
> > > > > > > > > > > > not
> > > > > > > > > > > > +platform, unless you manually specify the cpu affinity
> > > > > > > > > > > > of
> > > > > > > > > > > > all
> > > > > > > > > > > > domains
> > > > > > > > > > > > so
> > > > > > > > > > > > +that all vcpus are scheduled on the same class of pcpus
> > > > > > > > > > > > (big or
> > > > > > > > > > > > LITTLE
> > > > > > > > > > > > +but not both). vcpu migration between big cores and
> > > > > > > > > > > > LITTLE
> > > > > > > > > > > > cores is
> > > > > > > > > > > > not
> > > > > > > > > > > > +supported. Thus, if the first 4 pcpus are big and the
> > > > > > > > > > > > last
> > > > > > > > > > > > 4
> > > > > > > > > > > > are
> > > > > > > > > > > > LITTLE,
> > > > > > > > > > > > +all domains need to have either cpus = "0-3" or cpus =
> > > > > > > > > > > > "4-7" in
> > > > > > > > > > > > their
> > > > > > > > > > > > VM
> > > > > > > > > > > > +config. Moreover, dom0_vcpus_pin needs to be passed on
> > > > > > > > > > > > the
> > > > > > > > > > > > Xen
> > > > > > > > > > > > command
> > > > > > > > > > > > +line.
> > > > > > > > > > > 
> > > > > > > > > > > In your example here you suggest to have all the vCPUs of
> > > > > > > > > > > a
> > > > > > > > > > > guest
> > > > > > > > > > > to
> > > > > > > > > > > either on
> > > > > > > > > > > big or LITTLE cores. How about giving an example where the
> > > > > > > > > > > guest
> > > > > > > > > > > can
> > > > > > > > > > > have
> > > > > > > > > > > 2
> > > > > > > > > > > LITTLE vCPUs and one big vCPU?
> > > > > > > > > > 
> > > > > > > > > > I would rather discourage it at the moment, given that it
> > > > > > > > > > requires
> > > > > > > > > > more
> > > > > > > > > > complex cpu affinity settings, or vcpu pinning. Also, I am
> > > > > > > > > > afraid
> > > > > > > > > > that
> > > > > > > > > > without matching corresponding topology information on the
> > > > > > > > > > guest
> > > > > > > > > > device
> > > > > > > > > > tree, guests might not work as expected in such a scenario.
> > > > > > > > > > 
> > > > > > > > > > What do you think?
> > > > > > > > > 
> > > > > > > > > You already know my view on this. I would rather strongly
> > > > > > > > > discourage
> > > > > > > > > anyone
> > > > > > > > > pinning all vCPUs of a domain to big cores. We should avoid to
> > > > > > > > > provide
> > > > > > > > > shortcuts to use that could have potentially damageable impact
> > > > > > > > > on
> > > > > > > > > their
> > > > > > > > > platform without telling them.
> > > > > > > > 
> > > > > > > > Do you have a link to a doc somewhere that provides more details
> > > > > > > > about
> > > > > > > > this? We could add a link to it here to inform users. It would
> > > > > > > > be
> > > > > > > > useful.
> > > > > > > 
> > > > > > > This is quite well described in
> > > > > > > https://xenbits.xen.org/docs/unstable/man/xl.cfg.5.html#CPU-Allocation
> > > > > > > see
> > > > > > > "cpus".
> > > > > > 
> > > > > > OK, I'll add the link in a new big.LITTLE doc. Also, do you have any
> > > > > > documentation or link about big core being potentially damaging? It
> > > > > > would be good to provide information about that too in the
> > > > > > big.LITTLE
> > > > > > doc.
> > > > > 
> > > > > I don't have specific documentation to point on it but I would quite
> > > > > interesting to know what is your documentation regarding why always
> > > > > running on
> > > > > big is safe.
> > > > 
> > > > Hi Julien,
> > > > 
> > > > If I go on Amazon, I buy a big.LITTLE board, then it overheats and
> > > > breaks due to the big cores, I would call this a malfunction and return
> > > > the item expecting a refund.
> > > 
> > > Why would they refund you? You run software that has not been proofed with
> > > their board. The vendor will nicely tell you to look for another software
> > > and
> > > will not give you the refund.
> > 
> > I guess it depends on the board. I don't think many dev boards (like
> > Pine64) require you to use a specific kernel version or hypevisor
> > version on them (I hope!).
> 
> The problem I saw is some vendor decided to offload some firmware tasks to the
> Operating System. This means that Xen needs to handle those drivers in order
> to get full support of the board.
> 
> > 
> > 
> > > > Unless the hardware vendor states explicitly that the big cores cannot
> > > > be used all the time, then this use-case falls within the reasonable
> > > > usage of the platform. In fact, it is using a piece of the hardware the
> > > > way it was designed to be used. If the hardware itself is unstable, it
> > > > should be documented in the vendor's docs, and I would like to add a
> > > > link to it so that users are appropriately warned.
> > > In normal circumstance, you have software controlling the overheat. But in
> > > case of Xen who is going to do that job? If it is the firmware and
> > > assuming it
> > > does not need to be taught, then this is likely going to work out-of-box
> > > with
> > > Xen. If it is the OS/Hypervisor, then you are going to get into trouble.
> > > 
> > > As you can see we already have different expectation on how the hardware
> > > should behave.
> 
> Hmmm I should have finished that paragraph. I meant that I would choose the
> more conservative way in the documentation because I would not assume we have
> the full stack working on Xen nowadays. If someone knows her platform is fine
> to always run on big cores, then it can still do it.
> 
> What I want to avoid is providing a way that we are not 100% sure will work on
> all the platforms.
> 
> > 
> > I am starting to see your point. What if we add the following statement,
> > it should non-controversial and informative:
> > 
> > "Big cores are more powerful than LITTLE cores, but often use much more
> > power. Typically, they are recommended for burst activity, especially in
> > battery powered environments. Please check your vendor's big.LITTLE
> > and power management documentation."
> 
> Sounds good to me.
> 
> > 
> > As I was thinking about this, I realized that the same issue could occur
> > even with just the first patch
> > (https://marc.info/?l=xen-devel&m=151873668223723). If the first cpu
> > type is big, we would default to use only big cpus all the time, right?
> 
> Hmmm you are right. However we have no easy way to know whether you boot on
> big or little CPUs :/. Shall we update the warning?

Yes, I'll point users to big.LITTLE.txt, where we have the space to
explain the problem properly.