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

[Mirela Simonovic <mirela.simonovic@xxxxxxxxxx>]

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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel