[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v2 05/10] xen/arm: Setup virtual paging for non-boot CPUs on hotplug/resume
On 04/24/2018 03:50 PM, Mirela Simonovic wrote:
Hi Julien,
Hi Mirela,
Thanks for the feedback.
On Mon, Apr 23, 2018 at 1:28 PM, Julien Grall <julien.grall@xxxxxxx> wrote:
Hi Mirela,
On 20/04/18 13:25, Mirela Simonovic wrote:
In existing code the paging for non-boot CPUs is setup only on boot. The
setup is triggered from start_xen() after all CPUs are brought online.
In other words, the initialization of VTCR_EL2 register is done out of the
cpu_up/start_secondary() control flow. However, the cpu_up flow is also
used
to hotplug non-boot CPUs on resume from suspend to RAM state, in which
case
the virtual paging will not be configured.
With this patch the setting of paging is triggered from start_secondary()
function if the current system state is not boot. This way, the paging
for secondary CPUs will be setup in non-boot scenarios as well, while the
setup in boot scenario remains unchanged.
It is assumed here that after the system completed the boot, CPUs that
execute start_secondary() were booted as well when the Xen itself was
booted. According to this assumption non-boot CPUs will always be
compliant
with the VTCR_EL2 value that was selected by Xen on boot.
Currently, these in no mechanism to trigger hotplugging of a CPU. This
will be added with the suspend to RAM support for ARM, where the hotplug
of non-boot CPUs will be triggered via enable_nonboot_cpus() call.
Signed-off-by: Mirela Simonovic <mirela.simonovic@xxxxxxxxxx>
---
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Julien Grall <julien.grall@xxxxxxx>
---
Changes in v2:
-Fix commit message
-Save configured VTCR_EL2 value into static variable that will be used
by non-boot CPUs on hotplug
-Add setup_virt_paging_secondary() and invoke it from start_secondary()
if that CPU has to setup virtual paging (if the system state is not
boot)
---
xen/arch/arm/p2m.c | 13 ++++++++++++-
xen/arch/arm/smpboot.c | 3 +++
xen/include/asm-arm/p2m.h | 3 +++
3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index d43c3aa896..9bb62c13cd 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1451,13 +1451,21 @@ err:
return page;
}
-static void __init setup_virt_paging_one(void *data)
+static void setup_virt_paging_one(void *data)
{
unsigned long val = (unsigned long)data;
WRITE_SYSREG32(val, VTCR_EL2);
isb();
}
+/* VTCR value to be configured by all CPUs. Set only once by the boot
CPU */
+static unsigned long __read_mostly vtcr_value;
VTCR is a register, so the type should be represented in term of bits (i.e
uint*_t).
I followed the type used in setup_virt_paging() and it's unsigned
long. However, the spec says the VTCR_EL2 is 32-bit register, meaning
that in the current implementation the type is not correct.
If I want the type to be correct in this patch Xen will not compile.
Are you suggesting to fix the type in existing implementation?
+
+void setup_virt_paging_secondary(void)
+{
+ setup_virt_paging_one((void *)vtcr_value);
That's fairly ugly. Is there any way to rework the interface? For instance,
because you have a static variable which contain the VTCR, you could just
use the variable in setup_virt_paging one.
If the argument provided to setup_virt_paging_one() is NULL within the
setup_virt_paging_one() I configure saved static vtcr_value? If that
is what you meant it was submitted in previous version of this patch
:)
Are you suggesting to revert the change to v1?
+}
+
void __init setup_virt_paging(void)
{
/* Setup Stage 2 address translation */
@@ -1540,6 +1548,9 @@ void __init setup_virt_paging(void)
BUG_ON( P2M_ROOT_LEVEL == 0 && P2M_ROOT_ORDER > 0 );
setup_virt_paging_one((void *)val);
smp_call_function(setup_virt_paging_one, (void *)val, 1);
+
+ /* Save configured value (to be used later for secondary CPUs
hotplug) */
+ vtcr_value = val;
}
/*
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index 38b665a6d2..abc642804f 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -358,6 +358,9 @@ void start_secondary(unsigned long boot_phys_offset,
local_irq_enable();
local_abort_enable();
+ if ( system_state != SYS_STATE_boot )
+ setup_virt_paging_secondary();
I think this code needs some documentation. So people understand why you
only call setup_virt_paging_secondary() after boot. But is there any reason
to not use a notifier (see notify_cpu_starting)? This would avoid yet
another export.
It works using notifiers, but I wouldn't say it's worth the overhead.
Implementation using notifiers requires 1 additional data structure
(notifier_block) and 2 functions to be implemented (an __init function
to register a notifier and another one for callback).
Moreover, such a callback would be called for each CPU event, which is
3 times when the CPU is hot-unplugged and nothing needs to be done.
I've already implemented notifier-based approach so I have no problem
submitting it. However, I'm not sure what you want to trade. Please
lets clarify.
I want to avoid tens of call in the code to every subsystem. This is
much cleaner to go through the notifiers.
The notifiers also have the advantage that it is much easier to see the
pairing between initialization and deinitialization (when existing) as
we will now have one place to look.
Cheers,
+
check_local_cpu_errata();
printk(XENLOG_DEBUG "CPU %u booted.\n", smp_processor_id());
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 8823707c17..85b66a1196 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -153,6 +153,9 @@ void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
/* Second stage paging setup, to be called on all CPUs */
void setup_virt_paging(void);
+/* To be called by secondary CPU on hotplug */
+void setup_virt_paging_secondary(void);
+
/* Init the datastructures for later use by the p2m code */
int p2m_init(struct domain *d);
Cheers,
--
Julien Grall
Thanks,
Mirela
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
