Re: [Xen-devel] [PATCH 0/7] xen/arm: CPU hotplug fixes

[Julien Grall]

Hi Mirela,

On 16/04/18 15:06, Mirela Simonovic wrote:

On Mon, Apr 16, 2018 at 1:33 PM, Julien Grall  wrote:

On 13/04/18 11:19, Mirela Simonovic wrote:

On Thu, Apr 12, 2018 at 10:43 AM, Julien Grall 

On 11/04/18 17:37, Mirela Simonovic wrote:

On Wed, Apr 11, 2018 at 6:02 PM, Julien Grall mailto:julien.gr...@arm.com>> wrote:

So this cover interrupt routed to a virtual CPU. However, this does not
handle interrupts used by Xen. How do you handle them?

For instance SMMUs IRQ might be routed to other interrupt than CPU #0.



Interrupts used by Xen should not wake-up the system and will be
disabled when we suspend the devices used by Xen.


Here you only speak about the suspend use case. While I understand your
ultimate goal is suspend/resume, this series is about CPU hotplug.



AFAIK, the only way and occasion to hotplug a CPU is using > 
disable/enable_nonboot_cpus() within the Xen suspend/resume procedure.


As I mention in a previous e-mail, suspend/resume is not the only way to 
hotplug a CPU. There are an interface (see XEN_SYSCTL_cpu_hotplug) to 
allow that from the userland.


But I agree that it is not implemented on Arm, so suspend/resume would 
be the only way to so far. To be clear, I am not asking you to implement 
XEN_SYSCTL_cpu_hotplug. Althougth it should be easy to do it and easy 
for testing CPU offline/online.



We are implementing CPU hotplug only to enable Xen suspend/resume.

>

This is how it is also done for x86 and we wanted to implement the
equivalent behavior for ARM.
If the cover-letter is misleading please let me know what would be
more appropriate title.


We have multiple places in Xen that could use cpu_up/cpu_down helpers. 
From the cover letter, it is quite unclear that you are only looking 
after suspend/resume.


The SYSCTL interface is one of the other use case. While most of the 
code can be shared, some of them may not make sense.


For instance, in the suspend/resume case routing the interrupt assigned 
to Xen to CPU#0 may not be necessary because all CPUs should come back 
later on. However, this should be necessary when offlining a vCPU with 
SYSCTL interface.


That's why I asked it and I think it should be clarified in the cover 
letter.


Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 0/7] xen/arm: CPU hotplug fixes

[Mirela Simonovic]

Hi Julien,

On Mon, Apr 16, 2018 at 1:33 PM, Julien Grall  wrote:
> Hi,
>
>
> On 13/04/18 11:19, Mirela Simonovic wrote:
>>
>> On Thu, Apr 12, 2018 at 10:43 AM, Julien Grall 
>> wrote:
>>>
>>>
>>>
>>> On 11/04/18 17:37, Mirela Simonovic wrote:


 Hi Julien,
>>>
>>>
>>>
>>> Hi,
>>>
>>> May I ask you to configure your mail client to use > for quoting and use
>>> plain text? Otherwise, this is going to be really difficult to follow the
>>> discussion after few round (see already below).
>>>
 On Wed, Apr 11, 2018 at 6:02 PM, Julien Grall >>> > wrote:

  Hi,

  On 11/04/18 16:58, Mirela Simonovic wrote:

  On 04/11/2018 05:07 PM, Julien Grall wrote:

  On 11/04/18 14:19, Mirela Simonovic wrote:

  Migrating interrupts when turning off a CPU already works.
  However, when a CPU is turned back on there is no interrupt
  migration back to the hotplugged CPU - all interrupts will
  remain routed to the CPU#0.
  Patch 7/7 fixes this


  What do you mean by all interrupts? Interrupts routed to guest will
  always follow the vCPU. So are you sure they are going to be
  migrated when that vCPU is paused/off?


 Just to make sure we're on the same page - this is about hotplugging
 physical CPUs. Hotplugging vCPUs using virtual PSCI CPU_OFF interface is
 already implemented and unrelated to this series.
>>>
>>>
>>>
>>> Yes, we are on the same page :). I was just wondering what happen to
>>> interrupt routed to that pCPU.
>>>

 Assuming that system has 2 pCPUs by 'all interrupts' I mean interrupts
 that were targeted to the pCPU#0 and pCPU#1 prior to doing any hotplug.

 For example, if a guest is pinned to pCPU#1 an interrupt of a device it
 owns will be targeted to pCPU#1.
 When pCPU#1 is turned off that interrupt will be migrated to pCPU#0.
 pCPU#0 finalizes the suspend and receives wake-up interrupts. However,
 when
 CPU#1 is turned back on that interrupt will remain targeted to the
 CPU#0,
 which I assumed is wrong.
 The scenario described here is also how I tested this.

  Can you give the path in Xen doing that?


 Sure, here is a backtrace (dumped on the CPU being turned off):
   0  0x2603dc arch_move_irqs(): vgic.c, line 309
   1  0x22ee58 sched_move_irqs()+20: schedule.c, line 303
   2  0x2318e8 cpu_disable_scheduler()+1000: schedule.c, line 586
   3  0x2318e8 cpu_disable_scheduler()+1000: schedule.c, line 586
   4  0x25aff8 __cpu_disable()+96: smpboot.c, line 386
   5  0x201608 take_cpu_down()+52: cpu.c, line 75
   6  0x23426c stopmachine_action()+188: stop_machine.c, line 159
   7  0x235858 do_tasklet_work()+176: tasklet.c, line 94
   8  0x235c80 do_tasklet()+104: tasklet.c, line 126
   9  0x24daec idle_loop()+144: domain.c, line 72
  10  0x25b1f8 start_secondary()+404: smpboot.c, line 368
>>>
>>>
>>>
>>>
>>> So this cover interrupt routed to a virtual CPU. However, this does not
>>> handle interrupts used by Xen. How do you handle them?
>>>
>>> For instance SMMUs IRQ might be routed to other interrupt than CPU #0.
>>
>>
>> Interrupts used by Xen should not wake-up the system and will be
>> disabled when we suspend the devices used by Xen.
>
> Here you only speak about the suspend use case. While I understand your
> ultimate goal is suspend/resume, this series is about CPU hotplug.
>

AFAIK, the only way and occasion to hotplug a CPU is using
disable/enable_nonboot_cpus() within the Xen suspend/resume procedure.
We are implementing CPU hotplug only to enable Xen suspend/resume.
This is how it is also done for x86 and we wanted to implement the
equivalent behavior for ARM.
If the cover-letter is misleading please let me know what would be
more appropriate title.

However, I absolutely agree that the interrupt routing before and
after the hotplug has to be the same.

> IHMO, the suspend/resume case is no more than a superset of CPU up/down. If
> you solve the problem for up/down, likely you are going to solve it for
> suspend/resume.
>
> So, what would happen to interrupts routed to the CPU going offline?
>
>> However, I need to double check that such interrupts get enabled on
>> the right CPU on resume. Could you please tell me which mechanism in
>> Xen is used to target such an interrupt to a secondary CPU only? Is
>> that even possible and why would that be used?
>
>
> SPIs will be routed to the CPU calling setup_irq. It may not always be
> CPU#0. For instance, this is the case context interrupt for the SMMU because
> they are setup when the device is assigned.
>
> I guess this decision is arguable. If you move all the interrupts to CPU#0
> it will potentially disrupt vCPU running on it. I am thinking in the case of
> SMMU fault th

Re: [Xen-devel] [PATCH 0/7] xen/arm: CPU hotplug fixes

[Julien Grall]

Hi,

On 13/04/18 11:19, Mirela Simonovic wrote:

On Thu, Apr 12, 2018 at 10:43 AM, Julien Grall  wrote:



On 11/04/18 17:37, Mirela Simonovic wrote:


Hi Julien,



Hi,

May I ask you to configure your mail client to use > for quoting and use
plain text? Otherwise, this is going to be really difficult to follow the
discussion after few round (see already below).


On Wed, Apr 11, 2018 at 6:02 PM, Julien Grall mailto:julien.gr...@arm.com>> wrote:

 Hi,

 On 11/04/18 16:58, Mirela Simonovic wrote:

 On 04/11/2018 05:07 PM, Julien Grall wrote:

 On 11/04/18 14:19, Mirela Simonovic wrote:

 Migrating interrupts when turning off a CPU already works.
 However, when a CPU is turned back on there is no interrupt
 migration back to the hotplugged CPU - all interrupts will
 remain routed to the CPU#0.
 Patch 7/7 fixes this


 What do you mean by all interrupts? Interrupts routed to guest will
 always follow the vCPU. So are you sure they are going to be
 migrated when that vCPU is paused/off?


Just to make sure we're on the same page - this is about hotplugging
physical CPUs. Hotplugging vCPUs using virtual PSCI CPU_OFF interface is
already implemented and unrelated to this series.



Yes, we are on the same page :). I was just wondering what happen to
interrupt routed to that pCPU.



Assuming that system has 2 pCPUs by 'all interrupts' I mean interrupts
that were targeted to the pCPU#0 and pCPU#1 prior to doing any hotplug.

For example, if a guest is pinned to pCPU#1 an interrupt of a device it
owns will be targeted to pCPU#1.
When pCPU#1 is turned off that interrupt will be migrated to pCPU#0.
pCPU#0 finalizes the suspend and receives wake-up interrupts. However, when
CPU#1 is turned back on that interrupt will remain targeted to the CPU#0,
which I assumed is wrong.
The scenario described here is also how I tested this.

 Can you give the path in Xen doing that?


Sure, here is a backtrace (dumped on the CPU being turned off):
  0  0x2603dc arch_move_irqs(): vgic.c, line 309
  1  0x22ee58 sched_move_irqs()+20: schedule.c, line 303
  2  0x2318e8 cpu_disable_scheduler()+1000: schedule.c, line 586
  3  0x2318e8 cpu_disable_scheduler()+1000: schedule.c, line 586
  4  0x25aff8 __cpu_disable()+96: smpboot.c, line 386
  5  0x201608 take_cpu_down()+52: cpu.c, line 75
  6  0x23426c stopmachine_action()+188: stop_machine.c, line 159
  7  0x235858 do_tasklet_work()+176: tasklet.c, line 94
  8  0x235c80 do_tasklet()+104: tasklet.c, line 126
  9  0x24daec idle_loop()+144: domain.c, line 72
 10  0x25b1f8 start_secondary()+404: smpboot.c, line 368




So this cover interrupt routed to a virtual CPU. However, this does not
handle interrupts used by Xen. How do you handle them?

For instance SMMUs IRQ might be routed to other interrupt than CPU #0.


Interrupts used by Xen should not wake-up the system and will be
disabled when we suspend the devices used by Xen.
Here you only speak about the suspend use case. While I understand your 
ultimate goal is suspend/resume, this series is about CPU hotplug.


IHMO, the suspend/resume case is no more than a superset of CPU up/down. 
If you solve the problem for up/down, likely you are going to solve it 
for suspend/resume.


So, what would happen to interrupts routed to the CPU going offline?


However, I need to double check that such interrupts get enabled on
the right CPU on resume. Could you please tell me which mechanism in
Xen is used to target such an interrupt to a secondary CPU only? Is
that even possible and why would that be used?


SPIs will be routed to the CPU calling setup_irq. It may not always be 
CPU#0. For instance, this is the case context interrupt for the SMMU 
because they are setup when the device is assigned.


I guess this decision is arguable. If you move all the interrupts to 
CPU#0 it will potentially disrupt vCPU running on it. I am thinking in 
the case of SMMU fault that could be triggered easily by another domain.


Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 0/7] xen/arm: CPU hotplug fixes

[Mirela Simonovic]

Hi Julien,

On Thu, Apr 12, 2018 at 10:43 AM, Julien Grall  wrote:
>
>
> On 11/04/18 17:37, Mirela Simonovic wrote:
>>
>> Hi Julien,
>
>
> Hi,
>
> May I ask you to configure your mail client to use > for quoting and use
> plain text? Otherwise, this is going to be really difficult to follow the
> discussion after few round (see already below).
>
>> On Wed, Apr 11, 2018 at 6:02 PM, Julien Grall > > wrote:
>>
>> Hi,
>>
>> On 11/04/18 16:58, Mirela Simonovic wrote:
>>
>> On 04/11/2018 05:07 PM, Julien Grall wrote:
>>
>> On 11/04/18 14:19, Mirela Simonovic wrote:
>>
>> Migrating interrupts when turning off a CPU already works.
>> However, when a CPU is turned back on there is no interrupt
>> migration back to the hotplugged CPU - all interrupts will
>> remain routed to the CPU#0.
>> Patch 7/7 fixes this
>>
>>
>> What do you mean by all interrupts? Interrupts routed to guest will
>> always follow the vCPU. So are you sure they are going to be
>> migrated when that vCPU is paused/off?
>>
>>
>> Just to make sure we're on the same page - this is about hotplugging
>> physical CPUs. Hotplugging vCPUs using virtual PSCI CPU_OFF interface is
>> already implemented and unrelated to this series.
>
>
> Yes, we are on the same page :). I was just wondering what happen to
> interrupt routed to that pCPU.
>
>>
>> Assuming that system has 2 pCPUs by 'all interrupts' I mean interrupts
>> that were targeted to the pCPU#0 and pCPU#1 prior to doing any hotplug.
>>
>> For example, if a guest is pinned to pCPU#1 an interrupt of a device it
>> owns will be targeted to pCPU#1.
>> When pCPU#1 is turned off that interrupt will be migrated to pCPU#0.
>> pCPU#0 finalizes the suspend and receives wake-up interrupts. However, when
>> CPU#1 is turned back on that interrupt will remain targeted to the CPU#0,
>> which I assumed is wrong.
>> The scenario described here is also how I tested this.
>>
>> Can you give the path in Xen doing that?
>>
>>
>> Sure, here is a backtrace (dumped on the CPU being turned off):
>>  0  0x2603dc arch_move_irqs(): vgic.c, line 309
>>  1  0x22ee58 sched_move_irqs()+20: schedule.c, line 303
>>  2  0x2318e8 cpu_disable_scheduler()+1000: schedule.c, line 586
>>  3  0x2318e8 cpu_disable_scheduler()+1000: schedule.c, line 586
>>  4  0x25aff8 __cpu_disable()+96: smpboot.c, line 386
>>  5  0x201608 take_cpu_down()+52: cpu.c, line 75
>>  6  0x23426c stopmachine_action()+188: stop_machine.c, line 159
>>  7  0x235858 do_tasklet_work()+176: tasklet.c, line 94
>>  8  0x235c80 do_tasklet()+104: tasklet.c, line 126
>>  9  0x24daec idle_loop()+144: domain.c, line 72
>> 10  0x25b1f8 start_secondary()+404: smpboot.c, line 368
>
>
>
> So this cover interrupt routed to a virtual CPU. However, this does not
> handle interrupts used by Xen. How do you handle them?
>
> For instance SMMUs IRQ might be routed to other interrupt than CPU #0.

Interrupts used by Xen should not wake-up the system and will be
disabled when we suspend the devices used by Xen.
However, I need to double check that such interrupts get enabled on
the right CPU on resume. Could you please tell me which mechanism in
Xen is used to target such an interrupt to a secondary CPU only? Is
that even possible and why would that be used?
Currently, I have no way to try out SMMU but I could try UART.

Thanks,
Mirela

>
> [...]
>
>>
>> If the former, it would be nice to get the code you used. If the
>> latter, then having a hack patch to test that code would be nice.
>> Ideally, you want to plug that in the SYSCTL interface for
>> out-of-box testing.
>>
>>
>> Ok, I have never used that but I'll try to figure it out. I may come up
>> with additional questions.
>
>
> You might want to have a look at the x86 version XEN_SYSCTL_cpu_hotplug in
> x86/systcl.c.
>
> Cheers,
>
> --
> Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 0/7] xen/arm: CPU hotplug fixes

[Julien Grall]

On 11/04/18 17:37, Mirela Simonovic wrote:

Hi Julien,


Hi,

May I ask you to configure your mail client to use > for quoting and use 
plain text? Otherwise, this is going to be really difficult to follow 
the discussion after few round (see already below).


On Wed, Apr 11, 2018 at 6:02 PM, Julien Grall > wrote:


Hi,

On 11/04/18 16:58, Mirela Simonovic wrote:

On 04/11/2018 05:07 PM, Julien Grall wrote:

On 11/04/18 14:19, Mirela Simonovic wrote:

Migrating interrupts when turning off a CPU already works.
However, when a CPU is turned back on there is no interrupt
migration back to the hotplugged CPU - all interrupts will
remain routed to the CPU#0.
Patch 7/7 fixes this


What do you mean by all interrupts? Interrupts routed to guest will
always follow the vCPU. So are you sure they are going to be
migrated when that vCPU is paused/off?


Just to make sure we're on the same page - this is about hotplugging 
physical CPUs. Hotplugging vCPUs using virtual PSCI CPU_OFF interface is 
already implemented and unrelated to this series.


Yes, we are on the same page :). I was just wondering what happen to 
interrupt routed to that pCPU.




Assuming that system has 2 pCPUs by 'all interrupts' I mean interrupts 
that were targeted to the pCPU#0 and pCPU#1 prior to doing any hotplug.


For example, if a guest is pinned to pCPU#1 an interrupt of a device it 
owns will be targeted to pCPU#1.
When pCPU#1 is turned off that interrupt will be migrated to pCPU#0. 
pCPU#0 finalizes the suspend and receives wake-up interrupts. However, 
when CPU#1 is turned back on that interrupt will remain targeted to the 
CPU#0, which I assumed is wrong.

The scenario described here is also how I tested this.

Can you give the path in Xen doing that?


Sure, here is a backtrace (dumped on the CPU being turned off):
     0  0x2603dc arch_move_irqs(): vgic.c, line 309
     1  0x22ee58 sched_move_irqs()+20: schedule.c, line 303
     2  0x2318e8 cpu_disable_scheduler()+1000: schedule.c, line 586
     3  0x2318e8 cpu_disable_scheduler()+1000: schedule.c, line 586
     4  0x25aff8 __cpu_disable()+96: smpboot.c, line 386
     5  0x201608 take_cpu_down()+52: cpu.c, line 75
     6  0x23426c stopmachine_action()+188: stop_machine.c, line 159
     7  0x235858 do_tasklet_work()+176: tasklet.c, line 94
     8  0x235c80 do_tasklet()+104: tasklet.c, line 126
     9  0x24daec idle_loop()+144: domain.c, line 72
    10  0x25b1f8 start_secondary()+404: smpboot.c, line 368



So this cover interrupt routed to a virtual CPU. However, this does not 
handle interrupts used by Xen. How do you handle them?


For instance SMMUs IRQ might be routed to other interrupt than CPU #0.

[...]



If the former, it would be nice to get the code you used. If the
latter, then having a hack patch to test that code would be nice.
Ideally, you want to plug that in the SYSCTL interface for
out-of-box testing.


Ok, I have never used that but I'll try to figure it out. I may come up 
with additional questions.


You might want to have a look at the x86 version XEN_SYSCTL_cpu_hotplug 
in x86/systcl.c.


Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 0/7] xen/arm: CPU hotplug fixes

[Mirela Simonovic]

Hi Julien,

On Wed, Apr 11, 2018 at 6:02 PM, Julien Grall  wrote:

> Hi,
>
> On 11/04/18 16:58, Mirela Simonovic wrote:
>
>> On 04/11/2018 05:07 PM, Julien Grall wrote:
>>
>>> On 11/04/18 14:19, Mirela Simonovic wrote:
>>>
>> Migrating interrupts when turning off a CPU already works. However, when
>> a CPU is turned back on there is no interrupt migration back to the
>> hotplugged CPU - all interrupts will remain routed to the CPU#0.
>> Patch 7/7 fixes this
>>
>
> What do you mean by all interrupts? Interrupts routed to guest will always
> follow the vCPU. So are you sure they are going to be migrated when that
> vCPU is paused/off?
>
>
Just to make sure we're on the same page - this is about hotplugging
physical CPUs. Hotplugging vCPUs using virtual PSCI CPU_OFF interface is
already implemented and unrelated to this series.

Assuming that system has 2 pCPUs by 'all interrupts' I mean interrupts that
were targeted to the pCPU#0 and pCPU#1 prior to doing any hotplug.

For example, if a guest is pinned to pCPU#1 an interrupt of a device it
owns will be targeted to pCPU#1.
When pCPU#1 is turned off that interrupt will be migrated to pCPU#0. pCPU#0
finalizes the suspend and receives wake-up interrupts. However, when CPU#1
is turned back on that interrupt will remain targeted to the CPU#0, which I
assumed is wrong.
The scenario described here is also how I tested this.



> Can you give the path in Xen doing that?
>
>
Sure, here is a backtrace (dumped on the CPU being turned off):

0  0x2603dc arch_move_irqs(): vgic.c, line 309
1  0x22ee58 sched_move_irqs()+20: schedule.c, line 303
2  0x2318e8 cpu_disable_scheduler()+1000: schedule.c, line 586
3  0x2318e8 cpu_disable_scheduler()+1000: schedule.c, line 586
4  0x25aff8 __cpu_disable()+96: smpboot.c, line 386
5  0x201608 take_cpu_down()+52: cpu.c, line 75
6  0x23426c stopmachine_action()+188: stop_machine.c, line 159
7  0x235858 do_tasklet_work()+176: tasklet.c, line 94
8  0x235c80 do_tasklet()+104: tasklet.c, line 126
9  0x24daec idle_loop()+144: domain.c, line 72
   10  0x25b1f8 start_secondary()+404: smpboot.c, line 368



>
>>
 Calls to enable/disable_nonboot_cpus() functions currently don't exist
 in
 Xen ARM code. This will be added with the suspend to RAM support for
 ARM.

>>>
>>> What would be the way to test that series?
>>>
>>>
>> I tested/developed the series on Xilinx Zynq UltraScale+ MPSoC (ZCU102
>> board), but how would others test that - I don't know. We will have to ask
>> Xilinx for detailed answer to this question.
>>
>
> I am not sure to understand your answer. What I asked, is how did you did
> turned off the CPU? Did you hack Xen or do you rely on another series?
>
>
I rely on the suspend to RAM series that will be submitted after this one
is done. Not complete suspend to RAM support is needed in order to do the
testing.
Essentially, I have triggered suspend to RAM from Dom0 and the minimum in
Xen is to have the suspend support for secondary CPUs (calls to
disable/enable_nonboot_cpus).


> If the former, it would be nice to get the code you used. If the latter,
> then having a hack patch to test that code would be nice. Ideally, you want
> to plug that in the SYSCTL interface for out-of-box testing.
>
>
Ok, I have never used that but I'll try to figure it out. I may come up
with additional questions.

Thanks,
Mirela



> Cheers,
>
> --
> Julien Grall
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 0/7] xen/arm: CPU hotplug fixes

[Julien Grall]

Hi,

On 11/04/18 16:58, Mirela Simonovic wrote:

On 04/11/2018 05:07 PM, Julien Grall wrote:

On 11/04/18 14:19, Mirela Simonovic wrote:
Migrating interrupts when turning off a CPU already works. However, when 
a CPU is turned back on there is no interrupt migration back to the 
hotplugged CPU - all interrupts will remain routed to the CPU#0.

Patch 7/7 fixes this


What do you mean by all interrupts? Interrupts routed to guest will 
always follow the vCPU. So are you sure they are going to be migrated 
when that vCPU is paused/off?


Can you give the path in Xen doing that?





Calls to enable/disable_nonboot_cpus() functions currently don't 
exist in
Xen ARM code. This will be added with the suspend to RAM support for 
ARM.


What would be the way to test that series?



I tested/developed the series on Xilinx Zynq UltraScale+ MPSoC (ZCU102 
board), but how would others test that - I don't know. We will have to 
ask Xilinx for detailed answer to this question.


I am not sure to understand your answer. What I asked, is how did you 
did turned off the CPU? Did you hack Xen or do you rely on another series?


If the former, it would be nice to get the code you used. If the latter, 
then having a hack patch to test that code would be nice. Ideally, you 
want to plug that in the SYSCTL interface for out-of-box testing.


Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 0/7] xen/arm: CPU hotplug fixes

[Mirela Simonovic]

Hi Julien,


Thank you for the feedback.


On 04/11/2018 05:07 PM, Julien Grall wrote:

Hi Mirela,

Thank you for sending the series.

On 11/04/18 14:19, Mirela Simonovic wrote:
This patch set contains fixes required to enable CPU hotplug for 
secondary CPUs.
CPU hotplug of secondary CPUs will be used for suspend to RAM support 
for ARM.


With these patches calling disable_nonboot_cpus() from the boot CPU 
will cause
all secondary CPUs to be stopped. When a CPU is stopped it will issue 
the PSCI
CPU_OFF call to the EL3. The calling CPU will be powered down if the 
underlying
firmware allows so. If the CPU_OFF is not supported by the underlying 
firmware
the calling CPU will stay in infinite WFI loop. When secondary CPUs 
are stopped

all interrupts targeted to them are migrated to the boot CPU.
Calling enable_nonboot_cpus() from the boot CPU will cause all 
secondary CPUs
to be hotplugged. When a CPU is hotplugged the interrupts' affinity 
is restored.


I think this patch series is not complete. While it may work for you 
there are a few corner cases that will result to leak some memory 
everytime you turn off/on the CPU. This is the case of page-table, 
action associated to local IRQ I don't have a full list, but 
basically you need to ensured that anything that was initialized 
during the CPU boot are freed/reverted. You probably want to look at 
my answer you on your design document [1] for more details.




Thanks, I'll check.

I also don't see any code migrating Xen interrupt from the CPU that is 
going to be turned off to the other. Is it something you are going to 
plan for the future?




Migrating interrupts when turning off a CPU already works. However, when 
a CPU is turned back on there is no interrupt migration back to the 
hotplugged CPU - all interrupts will remain routed to the CPU#0.

Patch 7/7 fixes this.



Calls to enable/disable_nonboot_cpus() functions currently don't 
exist in
Xen ARM code. This will be added with the suspend to RAM support for 
ARM.


What would be the way to test that series?



I tested/developed the series on Xilinx Zynq UltraScale+ MPSoC (ZCU102 
board), but how would others test that - I don't know. We will have to 
ask Xilinx for detailed answer to this question.


Thanks,
Mirela



The code is tested on Xilinx Zynq UltraScale+ MPSoC/ZCU102 board 
(includes

physical power down/up of secondary CPUs).

Mirela Simonovic (7):
   xen/arm: Added handling of the trapped access to OSLSR register
   xen/arm/vgic-v2: Ignore write to GICD_ISACTIVERn registers
   xen/arm/psci: Implement CPU_OFF PSCI call (physical interface)
   xen/arm: When CPU dies, free percpu area immediatelly
   xen/arm: Remove __initdata and __init to enable CPU hotplug
   xen/arm: Setup virtual paging for secondary CPUs in non-boot scenario
   xen/arm: Restore IRQ affinity after hotplugging a CPU

  xen/arch/arm/arm64/smpboot.c   |  2 +-
  xen/arch/arm/arm64/vsysreg.c   |  3 ++-
  xen/arch/arm/irq.c |  2 +-
  xen/arch/arm/p2m.c | 10 --
  xen/arch/arm/percpu.c  |  2 +-
  xen/arch/arm/processor.c   |  2 +-
  xen/arch/arm/psci.c|  5 +
  xen/arch/arm/smpboot.c | 14 --
  xen/arch/arm/vgic-v2.c |  3 +--
  xen/common/schedule.c  |  4 
  xen/include/asm-arm/p2m.h  |  3 +++
  xen/include/asm-arm/procinfo.h |  4 ++--
  xen/include/asm-arm/psci.h |  1 +
  13 files changed, 42 insertions(+), 13 deletions(-)



Cheers,

[1] See [RFC v2] xen/arm: Suspend to RAM Support in Xen for ARM




___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 0/7] xen/arm: CPU hotplug fixes

[Julien Grall]

Hi,

I forgot to mention that the title of most of the patch gives the 
impression you fixes hotplug for both Arm32 and Arm64. After a deeper 
look, it is only arm64. Please make clear over commit message and cover 
letter.


Cheers,

On 11/04/18 14:19, Mirela Simonovic wrote:

This patch set contains fixes required to enable CPU hotplug for secondary CPUs.
CPU hotplug of secondary CPUs will be used for suspend to RAM support for ARM.

With these patches calling disable_nonboot_cpus() from the boot CPU will cause
all secondary CPUs to be stopped. When a CPU is stopped it will issue the PSCI
CPU_OFF call to the EL3. The calling CPU will be powered down if the underlying
firmware allows so. If the CPU_OFF is not supported by the underlying firmware
the calling CPU will stay in infinite WFI loop. When secondary CPUs are stopped
all interrupts targeted to them are migrated to the boot CPU.
Calling enable_nonboot_cpus() from the boot CPU will cause all secondary CPUs
to be hotplugged. When a CPU is hotplugged the interrupts' affinity is restored.

Calls to enable/disable_nonboot_cpus() functions currently don't exist in
Xen ARM code. This will be added with the suspend to RAM support for ARM.

The code is tested on Xilinx Zynq UltraScale+ MPSoC/ZCU102 board (includes
physical power down/up of secondary CPUs).

Mirela Simonovic (7):
   xen/arm: Added handling of the trapped access to OSLSR register
   xen/arm/vgic-v2: Ignore write to GICD_ISACTIVERn registers
   xen/arm/psci: Implement CPU_OFF PSCI call (physical interface)
   xen/arm: When CPU dies, free percpu area immediatelly
   xen/arm: Remove __initdata and __init to enable CPU hotplug
   xen/arm: Setup virtual paging for secondary CPUs in non-boot scenario
   xen/arm: Restore IRQ affinity after hotplugging a CPU

  xen/arch/arm/arm64/smpboot.c   |  2 +-
  xen/arch/arm/arm64/vsysreg.c   |  3 ++-
  xen/arch/arm/irq.c |  2 +-
  xen/arch/arm/p2m.c | 10 --
  xen/arch/arm/percpu.c  |  2 +-
  xen/arch/arm/processor.c   |  2 +-
  xen/arch/arm/psci.c|  5 +
  xen/arch/arm/smpboot.c | 14 --
  xen/arch/arm/vgic-v2.c |  3 +--
  xen/common/schedule.c  |  4 
  xen/include/asm-arm/p2m.h  |  3 +++
  xen/include/asm-arm/procinfo.h |  4 ++--
  xen/include/asm-arm/psci.h |  1 +
  13 files changed, 42 insertions(+), 13 deletions(-)



--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 0/7] xen/arm: CPU hotplug fixes

[Julien Grall]

Hi Mirela,

Thank you for sending the series.

On 11/04/18 14:19, Mirela Simonovic wrote:

This patch set contains fixes required to enable CPU hotplug for secondary CPUs.
CPU hotplug of secondary CPUs will be used for suspend to RAM support for ARM.

With these patches calling disable_nonboot_cpus() from the boot CPU will cause
all secondary CPUs to be stopped. When a CPU is stopped it will issue the PSCI
CPU_OFF call to the EL3. The calling CPU will be powered down if the underlying
firmware allows so. If the CPU_OFF is not supported by the underlying firmware
the calling CPU will stay in infinite WFI loop. When secondary CPUs are stopped
all interrupts targeted to them are migrated to the boot CPU.
Calling enable_nonboot_cpus() from the boot CPU will cause all secondary CPUs
to be hotplugged. When a CPU is hotplugged the interrupts' affinity is restored.


I think this patch series is not complete. While it may work for you 
there are a few corner cases that will result to leak some memory 
everytime you turn off/on the CPU. This is the case of page-table, 
action associated to local IRQ I don't have a full list, but 
basically you need to ensured that anything that was initialized during 
the CPU boot are freed/reverted. You probably want to look at my answer 
you on your design document [1] for more details.


I also don't see any code migrating Xen interrupt from the CPU that is 
going to be turned off to the other. Is it something you are going to 
plan for the future?




Calls to enable/disable_nonboot_cpus() functions currently don't exist in
Xen ARM code. This will be added with the suspend to RAM support for ARM.


What would be the way to test that series?



The code is tested on Xilinx Zynq UltraScale+ MPSoC/ZCU102 board (includes
physical power down/up of secondary CPUs).

Mirela Simonovic (7):
   xen/arm: Added handling of the trapped access to OSLSR register
   xen/arm/vgic-v2: Ignore write to GICD_ISACTIVERn registers
   xen/arm/psci: Implement CPU_OFF PSCI call (physical interface)
   xen/arm: When CPU dies, free percpu area immediatelly
   xen/arm: Remove __initdata and __init to enable CPU hotplug
   xen/arm: Setup virtual paging for secondary CPUs in non-boot scenario
   xen/arm: Restore IRQ affinity after hotplugging a CPU

  xen/arch/arm/arm64/smpboot.c   |  2 +-
  xen/arch/arm/arm64/vsysreg.c   |  3 ++-
  xen/arch/arm/irq.c |  2 +-
  xen/arch/arm/p2m.c | 10 --
  xen/arch/arm/percpu.c  |  2 +-
  xen/arch/arm/processor.c   |  2 +-
  xen/arch/arm/psci.c|  5 +
  xen/arch/arm/smpboot.c | 14 --
  xen/arch/arm/vgic-v2.c |  3 +--
  xen/common/schedule.c  |  4 
  xen/include/asm-arm/p2m.h  |  3 +++
  xen/include/asm-arm/procinfo.h |  4 ++--
  xen/include/asm-arm/psci.h |  1 +
  13 files changed, 42 insertions(+), 13 deletions(-)



Cheers,

[1] See [RFC v2] xen/arm: Suspend to RAM Support in Xen for ARM

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 0/7] xen/arm: CPU hotplug fixes

[Mirela Simonovic]

This patch set contains fixes required to enable CPU hotplug for secondary CPUs.
CPU hotplug of secondary CPUs will be used for suspend to RAM support for ARM.

With these patches calling disable_nonboot_cpus() from the boot CPU will cause
all secondary CPUs to be stopped. When a CPU is stopped it will issue the PSCI
CPU_OFF call to the EL3. The calling CPU will be powered down if the underlying
firmware allows so. If the CPU_OFF is not supported by the underlying firmware
the calling CPU will stay in infinite WFI loop. When secondary CPUs are stopped
all interrupts targeted to them are migrated to the boot CPU.
Calling enable_nonboot_cpus() from the boot CPU will cause all secondary CPUs
to be hotplugged. When a CPU is hotplugged the interrupts' affinity is restored.

Calls to enable/disable_nonboot_cpus() functions currently don't exist in
Xen ARM code. This will be added with the suspend to RAM support for ARM.

The code is tested on Xilinx Zynq UltraScale+ MPSoC/ZCU102 board (includes
physical power down/up of secondary CPUs).

Mirela Simonovic (7):
  xen/arm: Added handling of the trapped access to OSLSR register
  xen/arm/vgic-v2: Ignore write to GICD_ISACTIVERn registers
  xen/arm/psci: Implement CPU_OFF PSCI call (physical interface)
  xen/arm: When CPU dies, free percpu area immediatelly
  xen/arm: Remove __initdata and __init to enable CPU hotplug
  xen/arm: Setup virtual paging for secondary CPUs in non-boot scenario
  xen/arm: Restore IRQ affinity after hotplugging a CPU

 xen/arch/arm/arm64/smpboot.c   |  2 +-
 xen/arch/arm/arm64/vsysreg.c   |  3 ++-
 xen/arch/arm/irq.c |  2 +-
 xen/arch/arm/p2m.c | 10 --
 xen/arch/arm/percpu.c  |  2 +-
 xen/arch/arm/processor.c   |  2 +-
 xen/arch/arm/psci.c|  5 +
 xen/arch/arm/smpboot.c | 14 --
 xen/arch/arm/vgic-v2.c |  3 +--
 xen/common/schedule.c  |  4 
 xen/include/asm-arm/p2m.h  |  3 +++
 xen/include/asm-arm/procinfo.h |  4 ++--
 xen/include/asm-arm/psci.h |  1 +
 13 files changed, 42 insertions(+), 13 deletions(-)

-- 
2.13.0


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel