[PATCH v4 00/10] xen/arm64: Suspend preconditions and CPU hotplug fixes

[PATCH v4 00/10] xen/arm64: Suspend preconditions and CPU hotplug fixes

[Mirela Simonovic]

This patch set contains fixes that are required as precondition for suspend to
RAM support, including the CPU hotplug which is required to suspend non-boot
CPUs.
The first two patches in this series:
1) xen/arm64: Added handling of the trapped access to OSLSR register
2) xen/arm: Ignore write to GICD_ISACTIVERn registers (vgic-v2)
are required to avoid Dom0 crashes when Dom0 performs its own suspend. This
patch set does not include the implementation of virtual PSCI system suspend
call that would allow guests to finalize their suspend procedures. This will
be submitted in the following series.

Remaining of the patches are related to enabling CPU hotplug for non-boot
CPUs is resume scenario. CPU hotplug of non-boot CPUs will be used for suspend
to RAM support for ARM. In suspend procedure, the hot-unplug of non-boot CPUs
will be triggered with disable_nonboot_cpus(), while the hotplug is triggered
with enable_nonboot_cpus(). Using these calls, the physical non-boot CPUs could
be powered down/up on suspend/resume, respectively, if the underlying firmware
allows so. Calls to enable/disable_nonboot_cpus() functions currently do not
exist in Xen ARM code. This will be added with the suspend to RAM support for
ARM.

When non-boot pCPUs are hot-unplugged their interrupts are migrated to the boot
pCPU. This series also includes a fix that would restore the interrupts affinity
once non-boot pCPUs are hotplugged. Here only SPIs used by guests are covered.
Migration of Xen internal SPIs is not covered. According to my understanding
Xen internal SPIs are routed to the boot CPU which initializes the respective
devices. Therefore, there is no need to migrate Xen internal SPIs.

The code is tested on Xilinx Zynq UltraScale+ MPSoC/ZCU102 board (includes
physical power down/up of non-boot CPUs). The testing requires additional
patches for issuing system suspend. These patches and instructions for testing
will be submitted later, when we get closer to the final version of the series.

---
Changes in v2:
-Rename cover-letter title and emphasize that 2 patches from this series are not
specific to CPU hotplug (my initial fault, splitting it now could be confusing)
-Fix cover-letter explanations
-Address all the issues and comments as discussed on mailing list for v1
-Add 3 patches to ensure that suspend/resume does not cause any memory leaks.
All the memory allocated when a CPU was hotplugged is now freed when the CPU is
hot-unplugged.
-Remove from the v1 series the patch which incorrectly dealt with an issue:
[PATCH 4/7] xen/arm: When CPU dies, free percpu area immediatelly
One solution to the issue addressed by the patch above is to add rcu_barrier()
prior to calling enable_nonboot_cpus() during the suspend. This is how it is
done in x86 suspend implementation. Until the discussion here
https://lists.xenproject.org/archives/html/xen-devel/2018-04/msg01199.html
doesn't conclude differently, I need to assume that adding rcu_barrier() prior
to calling enable_nonboot_cpus() as it is done for x86 is the right way to go.
Therefore, the fix to the issue will be part of the suspend to RAM series.

Changes in v3:
-Add acked-by where needed
-Fix CPU_OFF PSCI implementation (physical interface)
-Use notifiers to implement freeing memory and releasing interrupts on CPU
hotplug
-Use notifier to trigger setup of virtual paging for non-boot CPUs on CPU
hotplug
-Add enabling errata workarounds on CPU hotplug, also based on a notifier
-Remove patch:
[PATCH v2 10/10] xen/arm: Call check_local_cpu_errata for secondary CPU only on boot

Changes in v4:
-Add acked-by/reviewed-by where needed
-Cleanup: use smp_processor_id() instead of get_processor_id(), fixed
 indentation, add includes alphabetically, add newline before return, etc.
-Disable timers prior to releasing timer interrupts
-Initialize cpu_smpboot notifier at presmp_init rather than init phase
-In the last patch of the series errata notifier now returns an error

---
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
CC: George Dunlap <george.dunlap@eu.citrix.com>
CC: Dario Faggioli <dfaggioli@suse.com>
---

Mirela Simonovic (10):
  xen/arm64: Added handling of the trapped access to OSLSR register
  xen/arm: Ignore write to GICD_ISACTIVERn registers (vgic-v2)
  xen/arm: Implement CPU_OFF PSCI call (physical interface)
  xen/arm: Remove __initdata and __init to enable CPU hotplug
  xen/arm: Setup virtual paging for non-boot CPUs on hotplug/resume
  xen/common: Restore IRQ affinity when hotplugging a pCPU
  xen/arm: Release maintenance interrupt when CPU is hot-unplugged
  xen/arm: Disable timers and release their interrupts on CPU hot-unplug
  xen/arm: Free memory allocated for sibling/core maps on CPU hot-unplug
  xen/arm: Enable errata for secondary CPU on hotplug after the boot

 xen/arch/arm/arm64/smpboot.c     |  2 +-
 xen/arch/arm/arm64/vsysreg.c     |  3 ++-
 xen/arch/arm/cpuerrata.c         | 49 +++++++++++++++++++++++++++++++++++++
 xen/arch/arm/cpufeature.c        | 29 ++++++++++++++++++++++
 xen/arch/arm/gic.c               | 31 +++++++++++++++++++++++
 xen/arch/arm/irq.c               |  2 +-
 xen/arch/arm/p2m.c               | 53 ++++++++++++++++++++++++++++++++++++----
 xen/arch/arm/processor.c         |  2 +-
 xen/arch/arm/psci.c              | 13 ++++++++++
 xen/arch/arm/smpboot.c           | 42 +++++++++++++++++++++++++++++--
 xen/arch/arm/time.c              | 45 ++++++++++++++++++++++++++++++++++
 xen/arch/arm/vgic-v2.c           |  2 ++
 xen/common/schedule.c            |  4 +++
 xen/include/asm-arm/cpufeature.h |  1 +
 xen/include/asm-arm/procinfo.h   |  4 +--
 xen/include/asm-arm/psci.h       |  1 +
 16 files changed, 270 insertions(+), 13 deletions(-)

-- 
2.13.0


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

[PATCH v4 01/10] xen/arm64: Added handling of the trapped access to OSLSR register

[Mirela Simonovic]

Linux/dom0 accesses OSLSR register when saving CPU context during the
suspend procedure. Xen traps access to this register, but has no handling
for it. Consequently, Xen injects undef exception to linux, causing it to
crash. This patch adds handling of the trapped access to OSLSR as ro/raz.

Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Julien Grall <julien.grall@arm.com>

---
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
---
Changes in v2:
- Commit message fix (arm64 related change instead of arm)
- Add Stefano's reviewed-by

Changes in v3:
- Added Julien's acked-by
---
 xen/arch/arm/arm64/vsysreg.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
index c57ac12503..8f80e1735e 100644
--- a/xen/arch/arm/arm64/vsysreg.c
+++ b/xen/arch/arm/arm64/vsysreg.c
@@ -57,13 +57,14 @@ void do_sysreg(struct cpu_user_regs *regs,
      * ARMv8 (DDI 0487A.d): D1-1509 Table D1-58
      *
      * Unhandled:
-     *    OSLSR_EL1
      *    DBGPRCR_EL1
      */
     case HSR_SYSREG_OSLAR_EL1:
         return handle_wo_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
     case HSR_SYSREG_OSDLR_EL1:
         return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
+    case HSR_SYSREG_OSLSR_EL1:
+        return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 1);
 
     /*
      * MDCR_EL2.TDA
-- 
2.13.0


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

[PATCH v4 02/10] xen/arm: Ignore write to GICD_ISACTIVERn registers (vgic-v2)

[Mirela Simonovic]

Guests attempt to write into these registers on resume (for example Linux).
Without this patch a data abort exception will be raised to the guest.
This patch handles the write access by ignoring it, but only if the value
to be written is zero. This should be fine because reading these registers
is already handled as 'read as zero'.

Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>

---
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
---
Changes in v2:
- Write should be ignored only if the value to be written is zero
 (in v1 the write was ignored regardless of the value)

Changes in v3:
- Print warning only if the value to be written is not zero

Changes in v4:
- Added reviewed-by Julien
---
 xen/arch/arm/vgic-v2.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index 646d1f3d12..f6c11f1e41 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -485,6 +485,8 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
 
     case VRANGE32(GICD_ISACTIVER, GICD_ISACTIVERN):
         if ( dabt.size != DABT_WORD ) goto bad_width;
+        if ( r == 0 )
+            goto write_ignore_32;
         printk(XENLOG_G_ERR
                "%pv: vGICD: unhandled word write %#"PRIregister" to ISACTIVER%d\n",
                v, r, gicd_reg - GICD_ISACTIVER);
-- 
2.13.0


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

[PATCH v4 03/10] xen/arm: Implement CPU_OFF PSCI call (physical interface)

[Mirela Simonovic]

During the system suspend to RAM non-boot CPUs will be hotplugged.
This will be triggered via disable_nonboot_cpus() call. When
hotplugged the CPU will end up in an infinite wfi loop in stop_cpu().
This patch adds PSCI CPU_OFF call to the EL3 with the aim to get powered
down the calling CPU during the suspend. The CPU_OFF call will be made
only if the PSCI version is higher than v0.1 (Note that the CPU_OFF
function is mandatory since PSCI v0.2).
If PSCI CPU_OFF call to the EL3 succeeds it will not return. Otherwise,
when the PSCI CPU_OFF call returns we'll raise panic, because the
calling CPU couldn't be enabled afterwards (stays in WFI loop forever).
Note that if the PSCI version is higher than v0.1 the CPU_OFF will be
called regardless of the system state. This is done because scenarios
other than suspend may benefit from powering off the CPU.

Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>
Acked-by: Julien Grall <julien.grall@arm.com>

---
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
---
Changes in v2:
-Issue PSCI CPU_OFF only if the system is suspending
-If PSCI CPU_OFF call fails (unlikely to ever happen) raise panic
-Fixed commit message

Changes in v3:
-Check for PSCI version prior to calling CPU_OFF
-Don't check for system state - invoke CPU_OFF in all system states
-Don't check if returned error is not zero because it's always not
 zero if CPU_OFF SMC returns
-Fixed commit message

Changes in v4:
-Use smp_processor_id() instead of get_processor_id()
-Fixed indentation
-Added acked-by Julien
---
 xen/arch/arm/psci.c        | 13 +++++++++++++
 xen/arch/arm/smpboot.c     |  2 ++
 xen/include/asm-arm/psci.h |  1 +
 3 files changed, 16 insertions(+)

diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
index 94b616df9b..3cf5ecf0f3 100644
--- a/xen/arch/arm/psci.c
+++ b/xen/arch/arm/psci.c
@@ -46,6 +46,19 @@ int call_psci_cpu_on(int cpu)
     return call_smc(psci_cpu_on_nr, cpu_logical_map(cpu), __pa(init_secondary), 0);
 }
 
+void call_psci_cpu_off(void)
+{
+    if ( psci_ver > PSCI_VERSION(0, 1) )
+    {
+        int errno;
+
+        /* If successfull the PSCI cpu_off call doesn't return */
+        errno = call_smc(PSCI_0_2_FN32_CPU_OFF, 0, 0, 0);
+        panic("PSCI cpu off failed for CPU%d err=%d\n", smp_processor_id(),
+              errno);
+    }
+}
+
 void call_psci_system_off(void)
 {
     if ( psci_ver > PSCI_VERSION(0, 1) )
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index b2116f0d2d..8b1e274bf3 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -395,6 +395,8 @@ void stop_cpu(void)
     /* Make sure the write happens before we sleep forever */
     dsb(sy);
     isb();
+    call_psci_cpu_off();
+
     while ( 1 )
         wfi();
 }
diff --git a/xen/include/asm-arm/psci.h b/xen/include/asm-arm/psci.h
index 9ac820e94a..832f77afff 100644
--- a/xen/include/asm-arm/psci.h
+++ b/xen/include/asm-arm/psci.h
@@ -20,6 +20,7 @@ extern uint32_t psci_ver;
 
 int psci_init(void);
 int call_psci_cpu_on(int cpu);
+void call_psci_cpu_off(void);
 void call_psci_system_off(void);
 void call_psci_system_reset(void);
 
-- 
2.13.0


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

[PATCH v4 04/10] xen/arm: Remove __initdata and __init to enable CPU hotplug

[Mirela Simonovic]

CPU up flow is currently used during the initial boot to start secondary
CPUs. However, the same flow should be used for CPU hotplug, e.g. when
hotplugging secondary CPUs within the resume procedure (resume from the
suspend to RAM). Therefore, prefixes __initdata and __init had to be removed
from few data structures and functions that are used within the cpu up flow.

Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>
Acked-by: Julien Grall <julien.grall@arm.com>

---
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
---
Changes in v3:
- Added acked-by Julien
---
 xen/arch/arm/arm64/smpboot.c   | 2 +-
 xen/arch/arm/irq.c             | 2 +-
 xen/arch/arm/processor.c       | 2 +-
 xen/arch/arm/smpboot.c         | 4 ++--
 xen/include/asm-arm/procinfo.h | 4 ++--
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/arm64/smpboot.c b/xen/arch/arm/arm64/smpboot.c
index 4fd0ac68b7..694fbf67e6 100644
--- a/xen/arch/arm/arm64/smpboot.c
+++ b/xen/arch/arm/arm64/smpboot.c
@@ -104,7 +104,7 @@ int __init arch_cpu_init(int cpu, struct dt_device_node *dn)
         return smp_psci_init(cpu);
 }
 
-int __init arch_cpu_up(int cpu)
+int arch_cpu_up(int cpu)
 {
     if ( !smp_enable_ops[cpu].prepare_cpu )
         return -ENODEV;
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index aa4e832cae..098281f8ab 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -65,7 +65,7 @@ irq_desc_t *__irq_to_desc(int irq)
     return &irq_desc[irq-NR_LOCAL_IRQS];
 }
 
-int __init arch_init_one_irq_desc(struct irq_desc *desc)
+int arch_init_one_irq_desc(struct irq_desc *desc)
 {
     desc->arch.type = IRQ_TYPE_INVALID;
     return 0;
diff --git a/xen/arch/arm/processor.c b/xen/arch/arm/processor.c
index ce4385064a..acad8b31d6 100644
--- a/xen/arch/arm/processor.c
+++ b/xen/arch/arm/processor.c
@@ -20,7 +20,7 @@
 
 static DEFINE_PER_CPU(struct processor *, processor);
 
-void __init processor_setup(void)
+void processor_setup(void)
 {
     const struct proc_info_list *procinfo;
 
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index 8b1e274bf3..ad1f6b751b 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -52,8 +52,8 @@ nodemask_t __read_mostly node_online_map = { { [0] = 1UL } };
 static unsigned char __initdata cpu0_boot_stack[STACK_SIZE]
        __attribute__((__aligned__(STACK_SIZE)));
 
-/* Initial boot cpu data */
-struct init_info __initdata init_data =
+/* Boot cpu data */
+struct init_info init_data =
 {
     .stack = cpu0_boot_stack,
 };
diff --git a/xen/include/asm-arm/procinfo.h b/xen/include/asm-arm/procinfo.h
index 26306b35f8..02be56e348 100644
--- a/xen/include/asm-arm/procinfo.h
+++ b/xen/include/asm-arm/procinfo.h
@@ -35,9 +35,9 @@ struct proc_info_list {
     struct processor    *processor;
 };
 
-const __init struct proc_info_list *lookup_processor_type(void);
+const struct proc_info_list *lookup_processor_type(void);
 
-void __init processor_setup(void);
+void processor_setup(void);
 void processor_vcpu_initialise(struct vcpu *v);
 
 #endif
-- 
2.13.0


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

[PATCH v4 05/10] xen/arm: Setup virtual paging for non-boot CPUs on hotplug/resume

[Mirela Simonovic]

In existing code the virtual 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 using cpu starting notifier (notify_cpu_starting() call). The
notifier is registered in p2m.c using init call. This has to be done with
init call rather than presmp_init because the registered callback depends
on vtcr configuration value which is setup after the presmp init calls
are executed (do_presmp_initcalls() called from start_xen()). Init calls
are executed after initial virtual paging is set up for all CPUs on boot.
This ensures that no callback can fire until the vtcr value is calculated
by Xen and virtual paging is set up initially for all CPUs. Also, this way
the virtual paging 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, there is 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@aggios.com>

---
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
---
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)

Changes in v3:
-Fix commit message
-Remove setup_virt_paging_secondary() and use notifier to setup virtual
 paging for non-boot CPU on hotplug.
-In setup_virt_paging() use vtcr static variable instead of local val
-In setup_virt_paging_one() use vtcr static variable instead of provided
 argument

Changes in v4:
-Add includes alphabetically
-Add newline before return in cpu_virt_paging_init()
-Fix indentation in cpu_virt_paging_callback() definition
-Use local val in setup_virt_paging() for calculation, assign it to vtcr
 after the calculation is done
-Remove priority initialization in the notifier structure (priority
 doesn't matter here)

Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>
---
 xen/arch/arm/p2m.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 48 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index d43c3aa896..924226f63c 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -8,6 +8,8 @@
 #include <xen/iocap.h>
 #include <xen/mem_access.h>
 #include <xen/xmalloc.h>
+#include <xen/cpu.h>
+#include <xen/notifier.h>
 #include <public/vm_event.h>
 #include <asm/flushtlb.h>
 #include <asm/event.h>
@@ -1451,10 +1453,12 @@ err:
     return page;
 }
 
-static void __init setup_virt_paging_one(void *data)
+/* VTCR value to be configured by all CPUs. Set only once by the boot CPU */
+static uint64_t __read_mostly vtcr;
+
+static void setup_virt_paging_one(void *data)
 {
-    unsigned long val = (unsigned long)data;
-    WRITE_SYSREG32(val, VTCR_EL2);
+    WRITE_SYSREG32(vtcr, VTCR_EL2);
     isb();
 }
 
@@ -1538,10 +1542,49 @@ void __init setup_virt_paging(void)
 
     /* It is not allowed to concatenate a level zero root */
     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);
+    vtcr = val;
+    setup_virt_paging_one(NULL);
+    smp_call_function(setup_virt_paging_one, NULL, 1);
+}
+
+static int cpu_virt_paging_callback(struct notifier_block *nfb,
+                                    unsigned long action,
+                                    void *hcpu)
+{
+    switch ( action )
+    {
+    case CPU_STARTING:
+        ASSERT(system_state != SYS_STATE_boot);
+        setup_virt_paging_one(NULL);
+        break;
+    default:
+        break;
+    }
+
+    return NOTIFY_DONE;
 }
 
+static struct notifier_block cpu_virt_paging_nfb = {
+    .notifier_call = cpu_virt_paging_callback,
+};
+
+static int __init cpu_virt_paging_init(void)
+{
+    register_cpu_notifier(&cpu_virt_paging_nfb);
+
+    return 0;
+}
+/*
+ * Initialization of the notifier has to be done at init rather than presmp_init
+ * phase because: the registered notifier is used to setup virtual paging for
+ * non-boot CPUs after the initial virtual paging for all CPUs is already setup,
+ * i.e. when a non-boot CPU is hotplugged after the system has booted. In other
+ * words, the notifier should be registered after the virtual paging is
+ * initially setup (setup_virt_paging() is called from start_xen()). This is
+ * required because vtcr config value has to be set before a notifier can fire.
+ */
+__initcall(cpu_virt_paging_init);
+
 /*
  * Local variables:
  * mode: C
-- 
2.13.0


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

[PATCH v4 06/10] xen/common: Restore IRQ affinity when hotplugging a pCPU

[Mirela Simonovic]

Non-boot pCPUs are being hot-unplugged during the system suspend to
RAM and hotplugged during the resume. When non-boot pCPUs are
hot-unplugged the interrupts that were targeted to them are migrated
to the boot pCPU.
On suspend, each guest could have its own wake-up devices/interrupts
(passthrough) that could trigger the system resume. These interrupts
could be targeted to a non-boot pCPU, e.g. if the guest's vCPU is
pinned to a non-boot pCPU. Due to the hot-unplug of non-boot pCPUs
during the suspend such interrupts will be migrated from non-boot pCPUs
to the boot pCPU (this is fine). However, when non-boot pCPUs are
hotplugged on resume, these interrupts are not migrated back to non-boot
pCPUs, i.e. IRQ affinity is not restored on resume (this is wrong).
This patch adds the restoration of IRQ affinity when a pCPU is hotplugged.

Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>
Reviewed-by: Dario Faggioli <dfaggioli@suse.com>

---
CC: George Dunlap <george.dunlap@eu.citrix.com>
CC: Dario Faggioli <dfaggioli@suse.com>
---
Changes in v2:
-Instead of checking whether the affinity was broken check whether
 vcpu's processor has changed in order to trigger restoring of the
 IRQ affinity
-Fix commit message

Changes in v4:
-Added reviewed by Dario
---
 xen/common/schedule.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 049f93f7aa..ccf936db83 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -737,6 +737,7 @@ void restore_vcpu_affinity(struct domain *d)
     for_each_vcpu ( d, v )
     {
         spinlock_t *lock;
+        unsigned int old_cpu = v->processor;
 
         ASSERT(!vcpu_runnable(v));
 
@@ -769,6 +770,9 @@ void restore_vcpu_affinity(struct domain *d)
         lock = vcpu_schedule_lock_irq(v);
         v->processor = SCHED_OP(vcpu_scheduler(v), pick_cpu, v);
         spin_unlock_irq(lock);
+
+        if ( old_cpu != v->processor )
+            sched_move_irqs(v);
     }
 
     domain_update_node_affinity(d);
-- 
2.13.0


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

[PATCH v4 07/10] xen/arm: Release maintenance interrupt when CPU is hot-unplugged

[Mirela Simonovic]

When a CPU is hot-unplugged the maintenance interrupt has to be
released in order to free the memory that was allocated when the CPU
was hotplugged and interrupt requested. The interrupt was requested
using request_irq() which is called from start_secondary->
init_maintenance_interrupt. With this patch the interrupt will be
released when the CPU_DYING event is received by the callback which
is added in gic.c.

Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>
Acked-by: Julien Grall <julien.grall@arm.com>

---
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
---
Changes in v3:
-Add notifier in order to trigger releasing of the  maintenance
 interrupt when the CPU is dying.

Changes in v4:
-Add includes alphabetically
-Added newline before the return in cpu_gic_notifier_init()
-Fix indentation in cpu_gic_callback() definition
-Added acked-by Julien
---
 xen/arch/arm/gic.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 653a815127..5474030386 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -27,6 +27,8 @@
 #include <xen/list.h>
 #include <xen/device_tree.h>
 #include <xen/acpi.h>
+#include <xen/cpu.h>
+#include <xen/notifier.h>
 #include <asm/p2m.h>
 #include <asm/domain.h>
 #include <asm/platform.h>
@@ -462,6 +464,35 @@ int gic_iomem_deny_access(const struct domain *d)
     return gic_hw_ops->iomem_deny_access(d);
 }
 
+static int cpu_gic_callback(struct notifier_block *nfb,
+                            unsigned long action,
+                            void *hcpu)
+{
+    switch ( action )
+    {
+    case CPU_DYING:
+        /* This is reverting the work done in init_maintenance_interrupt */
+        release_irq(gic_hw_ops->info->maintenance_irq, NULL);
+        break;
+    default:
+        break;
+    }
+
+    return NOTIFY_DONE;
+}
+
+static struct notifier_block cpu_gic_nfb = {
+    .notifier_call = cpu_gic_callback,
+};
+
+static int __init cpu_gic_notifier_init(void)
+{
+    register_cpu_notifier(&cpu_gic_nfb);
+
+    return 0;
+}
+__initcall(cpu_gic_notifier_init);
+
 /*
  * Local variables:
  * mode: C
-- 
2.13.0


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

[PATCH v4 08/10] xen/arm: Disable timers and release their interrupts on CPU hot-unplug

[Mirela Simonovic]

When a CPU is hot-unplugged we need to disable timers and release
their interrupts in order to free the memory that was allocated when
interrupts were requested (using request_irq()). The request_irq()
is called for each timer interrupt when the CPU gets hotplugged
(start_secondary->init_timer_interrupt->request_irq).
With this patch timers will be disabled and interrupts will be
released when the newly added callback receives CPU_DYING event.

Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>

---
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
---
Changes in v3:
-Trigger releasing of timer interrupts using notifiers

Changes in v4:
-Fix commit message to include disabling of timers
-Disable timers prior to releasing interrupts
-Add new line before the return in cpu_time_notifier_init()
-Add includes alphabetically
-Fix indentation in cpu_time_callback() definition
---
 xen/arch/arm/time.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index c11fcfeadd..1635c8822d 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -29,6 +29,8 @@
 #include <xen/sched.h>
 #include <xen/event.h>
 #include <xen/acpi.h>
+#include <xen/cpu.h>
+#include <xen/notifier.h>
 #include <asm/system.h>
 #include <asm/time.h>
 #include <asm/vgic.h>
@@ -312,6 +314,21 @@ void init_timer_interrupt(void)
     check_timer_irq_cfg(timer_irq[TIMER_PHYS_NONSECURE_PPI], "NS-physical");
 }
 
+/*
+ * Revert actions done in init_timer_interrupt that are required to properly
+ * disable this CPU.
+ */
+static void deinit_timer_interrupt(void)
+{
+    WRITE_SYSREG32(0, CNTP_CTL_EL0);    /* Disable physical timer */
+    WRITE_SYSREG32(0, CNTHP_CTL_EL2);   /* Disable hypervisor's timer */
+    isb();
+
+    release_irq(timer_irq[TIMER_HYP_PPI], NULL);
+    release_irq(timer_irq[TIMER_VIRT_PPI], NULL);
+    release_irq(timer_irq[TIMER_PHYS_NONSECURE_PPI], NULL);
+}
+
 /* Wait a set number of microseconds */
 void udelay(unsigned long usecs)
 {
@@ -340,6 +357,34 @@ void domain_set_time_offset(struct domain *d, int64_t time_offset_seconds)
     /* XXX update guest visible wallclock time */
 }
 
+static int cpu_time_callback(struct notifier_block *nfb,
+                             unsigned long action,
+                             void *hcpu)
+{
+    switch ( action )
+    {
+    case CPU_DYING:
+        deinit_timer_interrupt();
+        break;
+    default:
+        break;
+    }
+
+    return NOTIFY_DONE;
+}
+
+static struct notifier_block cpu_time_nfb = {
+    .notifier_call = cpu_time_callback,
+};
+
+static int __init cpu_time_notifier_init(void)
+{
+    register_cpu_notifier(&cpu_time_nfb);
+
+    return 0;
+}
+__initcall(cpu_time_notifier_init);
+
 /*
  * Local variables:
  * mode: C
-- 
2.13.0


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

[PATCH v4 09/10] xen/arm: Free memory allocated for sibling/core maps on CPU hot-unplug

[Mirela Simonovic]

The memory allocated in setup_cpu_sibling_map() when a CPU is hotplugged
has to be freed when the CPU is hot-unplugged. This is done in
remove_cpu_sibling_map() and called when the CPU dies. The call to
remove_cpu_sibling_map() is made from a notifier callback when
CPU_DEAD event is received.

Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>

---
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
---
Changes in v3:
-Use notifier to trigger remove_cpu_sibling_map() when the CPU dies.

Changes in v4:
-Initialize cpu_smpboot notifier at presmp_init rather than init phase
 to cover the case where a secondary CPU dies beforehand the initcall
-Added newline before the return in cpu_smpboot_notifier_init()
-Fix indentation in cpu_smpboot_callback() definition
---
 xen/arch/arm/smpboot.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index ad1f6b751b..cf3a4ce659 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -89,6 +89,12 @@ static void setup_cpu_sibling_map(int cpu)
     cpumask_set_cpu(cpu, per_cpu(cpu_core_mask, cpu));
 }
 
+static void remove_cpu_sibling_map(int cpu)
+{
+    free_cpumask_var(per_cpu(cpu_sibling_mask, cpu));
+    free_cpumask_var(per_cpu(cpu_core_mask, cpu));
+}
+
 void __init
 smp_clear_cpu_maps (void)
 {
@@ -499,6 +505,36 @@ void __cpu_die(unsigned int cpu)
     smp_mb();
 }
 
+static int cpu_smpboot_callback(struct notifier_block *nfb,
+                                unsigned long action,
+                                void *hcpu)
+{
+    unsigned int cpu = (unsigned long)hcpu;
+
+    switch ( action )
+    {
+    case CPU_DEAD:
+        remove_cpu_sibling_map(cpu);
+        break;
+    default:
+        break;
+    }
+
+    return NOTIFY_DONE;
+}
+
+static struct notifier_block cpu_smpboot_nfb = {
+    .notifier_call = cpu_smpboot_callback,
+};
+
+static int __init cpu_smpboot_notifier_init(void)
+{
+    register_cpu_notifier(&cpu_smpboot_nfb);
+
+    return 0;
+}
+presmp_initcall(cpu_smpboot_notifier_init);
+
 /*
  * Local variables:
  * mode: C
-- 
2.13.0


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

[PATCH v4 10/10] xen/arm: Enable errata for secondary CPU on hotplug after the boot

[Mirela Simonovic]

On boot, enabling errata workarounds will be triggered by the boot CPU
from start_xen(). On CPU hotplug (non-boot scenario) this would not be
done. This patch adds the code required to enable errata workarounds for
a CPU being hotplugged after the system boots. This is triggered using
a notifier. If the CPU fails to enable workarounds the notifier will
return an error and Xen will hit the BUG_ON() in notify_cpu_starting().
To avoid the BUG_ON() in an error case either enabling notifiers should
be fixed to return void (not propagate error to notify_cpu_starting())
and the errata notifier will always return success for CPU_STARTING
event, or the notify_cpu_starting() and other common code should be
fixed to expect an error at CPU_STARTING phase.

Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>

---
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
---
Changes in v4:
-Add includes alphabetically
-Added newline before the return in cpu_errata_notifier_init()
-Enabling capabilities returns an error if enabling a capability fails
 (enable_nonboot_cpu_caps() returns int instead of void). When enabling
 any of the capability fails the error is remembered into a variable and
 the remaining capabilities are enabled. If enabling multiple capabilities
 fails the error returned by enable_nonboot_cpu_caps() represents the
 error code of the last failure.
-Callback enable_nonboot_cpu_caps() can return an error when CPU_STARTING
 fires. This is not right because of the assumption that starting a CPU
 cannot fail at this phase. Consequently, if an error happens it will
 cause Xen to hit the BUG_ON() in notify_cpu_starting(). In future,
 either this notifier/enabling capabilities should be fixed to always
 return success/void, or notify_cpu_starting() and other common code
 should be fixed to expect an error at CPU_STARTING phase.
-Fix commit message to reflect changes in v4
---
 xen/arch/arm/cpuerrata.c         | 49 ++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/cpufeature.c        | 29 ++++++++++++++++++++++++
 xen/include/asm-arm/cpufeature.h |  1 +
 3 files changed, 79 insertions(+)

diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index 1baa20654b..b829d226ef 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -1,3 +1,4 @@
+#include <xen/cpu.h>
 #include <xen/cpumask.h>
 #include <xen/mm.h>
 #include <xen/sizes.h>
@@ -5,6 +6,7 @@
 #include <xen/spinlock.h>
 #include <xen/vmap.h>
 #include <xen/warning.h>
+#include <xen/notifier.h>
 #include <asm/cpufeature.h>
 #include <asm/cpuerrata.h>
 #include <asm/psci.h>
@@ -349,6 +351,53 @@ void __init enable_errata_workarounds(void)
     enable_cpu_capabilities(arm_errata);
 }
 
+static int cpu_errata_callback(struct notifier_block *nfb,
+                               unsigned long action,
+                               void *hcpu)
+{
+    int rc = 0;
+
+    switch ( action )
+    {
+    case CPU_STARTING:
+        /*
+         * At CPU_STARTING phase no notifier shall return an error, because the
+         * system is designed with the assumption that starting a CPU cannot
+         * fail at this point. If an error happens here it will cause Xen to hit
+         * the BUG_ON() in notify_cpu_starting(). In future, either this
+         * notifier/enabling capabilities should be fixed to always return
+         * success/void or notify_cpu_starting() and other common code should be
+         * fixed to expect an error at CPU_STARTING phase.
+         */
+        ASSERT(system_state != SYS_STATE_boot);
+        rc = enable_nonboot_cpu_caps(arm_errata);
+        break;
+    default:
+        break;
+    }
+
+    return !rc ? NOTIFY_DONE : notifier_from_errno(rc);
+}
+
+static struct notifier_block cpu_errata_nfb = {
+    .notifier_call = cpu_errata_callback,
+};
+
+static int __init cpu_errata_notifier_init(void)
+{
+    register_cpu_notifier(&cpu_errata_nfb);
+
+    return 0;
+}
+/*
+ * Initialization has to be done at init rather than presmp_init phase because
+ * the callback should execute only after the secondary CPUs are initially
+ * booted (in hotplug scenarios when the system state is not boot). On boot,
+ * the enabling of errata workarounds will be triggered by the boot CPU from
+ * start_xen().
+ */
+__initcall(cpu_errata_notifier_init);
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
index 525b45e22f..3aaff4c0e6 100644
--- a/xen/arch/arm/cpufeature.c
+++ b/xen/arch/arm/cpufeature.c
@@ -69,6 +69,35 @@ void __init enable_cpu_capabilities(const struct arm_cpu_capabilities *caps)
 }
 
 /*
+ * Run through the enabled capabilities and enable() them on the calling CPU.
+ * If enabling of any capability fails the error is returned. After enabling a
+ * capability fails the error will be remembered into 'rc' and the remaining
+ * capabilities will be enabled. If enabling multiple capabilities fail the
+ * error returned by this function represents the error code of the last
+ * failure.
+ */
+int enable_nonboot_cpu_caps(const struct arm_cpu_capabilities *caps)
+{
+    int rc = 0;
+
+    for ( ; caps->matches; caps++ )
+    {
+        if ( !cpus_have_cap(caps->capability) )
+            continue;
+
+        if ( caps->enable )
+        {
+            int ret = caps->enable((void *)caps);
+
+            if ( ret )
+                rc = ret;
+        }
+    }
+
+    return rc;
+}
+
+/*
  * Local variables:
  * mode: C
  * c-file-style: "BSD"
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index e557a095af..c5d046218b 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -88,6 +88,7 @@ void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
                              const char *info);
 
 void enable_cpu_capabilities(const struct arm_cpu_capabilities *caps);
+int enable_nonboot_cpu_caps(const struct arm_cpu_capabilities *caps);
 
 #endif /* __ASSEMBLY__ */
 
-- 
2.13.0


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

Re: [PATCH v4 00/10] xen/arm64: Suspend preconditions and CPU hotplug fixes

[Mirela Simonovic]

[-- Attachment #1.1: Type: text/plain, Size: 6196 bytes --]

Hi,

Could you please provide feedback for the remaining patches?

Thanks,
Mirela

On Tue, May 15, 2018 at 1:44 PM, Mirela Simonovic <
mirela.simonovic@aggios.com> wrote:

> This patch set contains fixes that are required as precondition for
> suspend to
> RAM support, including the CPU hotplug which is required to suspend
> non-boot
> CPUs.
> The first two patches in this series:
> 1) xen/arm64: Added handling of the trapped access to OSLSR register
> 2) xen/arm: Ignore write to GICD_ISACTIVERn registers (vgic-v2)
> are required to avoid Dom0 crashes when Dom0 performs its own suspend. This
> patch set does not include the implementation of virtual PSCI system
> suspend
> call that would allow guests to finalize their suspend procedures. This
> will
> be submitted in the following series.
>
> Remaining of the patches are related to enabling CPU hotplug for non-boot
> CPUs is resume scenario. CPU hotplug of non-boot CPUs will be used for
> suspend
> to RAM support for ARM. In suspend procedure, the hot-unplug of non-boot
> CPUs
> will be triggered with disable_nonboot_cpus(), while the hotplug is
> triggered
> with enable_nonboot_cpus(). Using these calls, the physical non-boot CPUs
> could
> be powered down/up on suspend/resume, respectively, if the underlying
> firmware
> allows so. Calls to enable/disable_nonboot_cpus() functions currently do
> not
> exist in Xen ARM code. This will be added with the suspend to RAM support
> for
> ARM.
>
> When non-boot pCPUs are hot-unplugged their interrupts are migrated to the
> boot
> pCPU. This series also includes a fix that would restore the interrupts
> affinity
> once non-boot pCPUs are hotplugged. Here only SPIs used by guests are
> covered.
> Migration of Xen internal SPIs is not covered. According to my
> understanding
> Xen internal SPIs are routed to the boot CPU which initializes the
> respective
> devices. Therefore, there is no need to migrate Xen internal SPIs.
>
> The code is tested on Xilinx Zynq UltraScale+ MPSoC/ZCU102 board (includes
> physical power down/up of non-boot CPUs). The testing requires additional
> patches for issuing system suspend. These patches and instructions for
> testing
> will be submitted later, when we get closer to the final version of the
> series.
>
> ---
> Changes in v2:
> -Rename cover-letter title and emphasize that 2 patches from this series
> are not
> specific to CPU hotplug (my initial fault, splitting it now could be
> confusing)
> -Fix cover-letter explanations
> -Address all the issues and comments as discussed on mailing list for v1
> -Add 3 patches to ensure that suspend/resume does not cause any memory
> leaks.
> All the memory allocated when a CPU was hotplugged is now freed when the
> CPU is
> hot-unplugged.
> -Remove from the v1 series the patch which incorrectly dealt with an issue:
> [PATCH 4/7] xen/arm: When CPU dies, free percpu area immediatelly
> One solution to the issue addressed by the patch above is to add
> rcu_barrier()
> prior to calling enable_nonboot_cpus() during the suspend. This is how it
> is
> done in x86 suspend implementation. Until the discussion here
> https://lists.xenproject.org/archives/html/xen-devel/2018-04/msg01199.html
> doesn't conclude differently, I need to assume that adding rcu_barrier()
> prior
> to calling enable_nonboot_cpus() as it is done for x86 is the right way to
> go.
> Therefore, the fix to the issue will be part of the suspend to RAM series.
>
> Changes in v3:
> -Add acked-by where needed
> -Fix CPU_OFF PSCI implementation (physical interface)
> -Use notifiers to implement freeing memory and releasing interrupts on CPU
> hotplug
> -Use notifier to trigger setup of virtual paging for non-boot CPUs on CPU
> hotplug
> -Add enabling errata workarounds on CPU hotplug, also based on a notifier
> -Remove patch:
> [PATCH v2 10/10] xen/arm: Call check_local_cpu_errata for secondary CPU
> only on boot
>
> Changes in v4:
> -Add acked-by/reviewed-by where needed
> -Cleanup: use smp_processor_id() instead of get_processor_id(), fixed
>  indentation, add includes alphabetically, add newline before return, etc.
> -Disable timers prior to releasing timer interrupts
> -Initialize cpu_smpboot notifier at presmp_init rather than init phase
> -In the last patch of the series errata notifier now returns an error
>
> ---
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien.grall@arm.com>
> CC: George Dunlap <george.dunlap@eu.citrix.com>
> CC: Dario Faggioli <dfaggioli@suse.com>
> ---
>
> Mirela Simonovic (10):
>   xen/arm64: Added handling of the trapped access to OSLSR register
>   xen/arm: Ignore write to GICD_ISACTIVERn registers (vgic-v2)
>   xen/arm: Implement CPU_OFF PSCI call (physical interface)
>   xen/arm: Remove __initdata and __init to enable CPU hotplug
>   xen/arm: Setup virtual paging for non-boot CPUs on hotplug/resume
>   xen/common: Restore IRQ affinity when hotplugging a pCPU
>   xen/arm: Release maintenance interrupt when CPU is hot-unplugged
>   xen/arm: Disable timers and release their interrupts on CPU hot-unplug
>   xen/arm: Free memory allocated for sibling/core maps on CPU hot-unplug
>   xen/arm: Enable errata for secondary CPU on hotplug after the boot
>
>  xen/arch/arm/arm64/smpboot.c     |  2 +-
>  xen/arch/arm/arm64/vsysreg.c     |  3 ++-
>  xen/arch/arm/cpuerrata.c         | 49 ++++++++++++++++++++++++++++++
> +++++++
>  xen/arch/arm/cpufeature.c        | 29 ++++++++++++++++++++++
>  xen/arch/arm/gic.c               | 31 +++++++++++++++++++++++
>  xen/arch/arm/irq.c               |  2 +-
>  xen/arch/arm/p2m.c               | 53 ++++++++++++++++++++++++++++++
> ++++++----
>  xen/arch/arm/processor.c         |  2 +-
>  xen/arch/arm/psci.c              | 13 ++++++++++
>  xen/arch/arm/smpboot.c           | 42 +++++++++++++++++++++++++++++--
>  xen/arch/arm/time.c              | 45 ++++++++++++++++++++++++++++++++++
>  xen/arch/arm/vgic-v2.c           |  2 ++
>  xen/common/schedule.c            |  4 +++
>  xen/include/asm-arm/cpufeature.h |  1 +
>  xen/include/asm-arm/procinfo.h   |  4 +--
>  xen/include/asm-arm/psci.h       |  1 +
>  16 files changed, 270 insertions(+), 13 deletions(-)
>
> --
> 2.13.0
>
>

[-- Attachment #1.2: Type: text/html, Size: 7374 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: [PATCH v4 00/10] xen/arm64: Suspend preconditions and CPU hotplug fixes

[Julien Grall]

On 28/05/18 11:15, Mirela Simonovic wrote:
> Hi,
> 
> Could you please provide feedback for the remaining patches?

The series is in my queue of patches to review but I was focus on 
getting XSA-263 (Speculation Store Bypass Disabled) ready for Arm.

I will have a look at it now.

Cheers,

-- 
Julien Grall

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

Re: [PATCH v4 01/10] xen/arm64: Added handling of the trapped access to OSLSR register

[Julien Grall]

Hi,

On 15/05/18 12:44, Mirela Simonovic wrote:
> Linux/dom0 accesses OSLSR register when saving CPU context during the
> suspend procedure. Xen traps access to this register, but has no handling
> for it. Consequently, Xen injects undef exception to linux, causing it to
> crash. This patch adds handling of the trapped access to OSLSR as ro/raz.
> 
> Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> Acked-by: Julien Grall <julien.grall@arm.com>
> 
> ---
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien.grall@arm.com>
> ---
> Changes in v2:
> - Commit message fix (arm64 related change instead of arm)
> - Add Stefano's reviewed-by
> 
> Changes in v3:
> - Added Julien's acked-by
> ---
>   xen/arch/arm/arm64/vsysreg.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
> index c57ac12503..8f80e1735e 100644
> --- a/xen/arch/arm/arm64/vsysreg.c
> +++ b/xen/arch/arm/arm64/vsysreg.c
> @@ -57,13 +57,14 @@ void do_sysreg(struct cpu_user_regs *regs,
>        * ARMv8 (DDI 0487A.d): D1-1509 Table D1-58
>        *
>        * Unhandled:
> -     *    OSLSR_EL1
>        *    DBGPRCR_EL1
>        */
>       case HSR_SYSREG_OSLAR_EL1:
>           return handle_wo_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
>       case HSR_SYSREG_OSDLR_EL1:
>           return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
> +    case HSR_SYSREG_OSLSR_EL1:
> +        return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 1);

Looking at this patch again, OSLM (bit[3] and bit[1]) should be 10 as 
all other values are reserved. So you want to set bit 3 when the guest 
read it.

Cheers,

-- 
Julien Grall

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

Re: [PATCH v4 05/10] xen/arm: Setup virtual paging for non-boot CPUs on hotplug/resume

[Julien Grall]

Hi,

On 15/05/18 12:44, Mirela Simonovic wrote:
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index d43c3aa896..924226f63c 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -8,6 +8,8 @@
>   #include <xen/iocap.h>
>   #include <xen/mem_access.h>
>   #include <xen/xmalloc.h>
> +#include <xen/cpu.h>
> +#include <xen/notifier.h>
>   #include <public/vm_event.h>
>   #include <asm/flushtlb.h>
>   #include <asm/event.h>
> @@ -1451,10 +1453,12 @@ err:
>       return page;
>   }
>   
> -static void __init setup_virt_paging_one(void *data)
> +/* VTCR value to be configured by all CPUs. Set only once by the boot CPU */
> +static uint64_t __read_mostly vtcr;

This should be uint32_t as VTCR is a 32-bit register. The rest looks 
good to me.

Cheers,

-- 
Julien Grall

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

Re: [PATCH v4 08/10] xen/arm: Disable timers and release their interrupts on CPU hot-unplug

[Julien Grall]

Hi,

On 15/05/18 12:44, Mirela Simonovic wrote:
> When a CPU is hot-unplugged we need to disable timers and release
> their interrupts in order to free the memory that was allocated when
> interrupts were requested (using request_irq()). The request_irq()
> is called for each timer interrupt when the CPU gets hotplugged
> (start_secondary->init_timer_interrupt->request_irq).
> With this patch timers will be disabled and interrupts will be
> released when the newly added callback receives CPU_DYING event.
> 
> Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>

Acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

> 
> ---
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien.grall@arm.com>
> ---
> Changes in v3:
> -Trigger releasing of timer interrupts using notifiers
> 
> Changes in v4:
> -Fix commit message to include disabling of timers
> -Disable timers prior to releasing interrupts
> -Add new line before the return in cpu_time_notifier_init()
> -Add includes alphabetically
> -Fix indentation in cpu_time_callback() definition
> ---
>   xen/arch/arm/time.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 45 insertions(+)
> 
> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
> index c11fcfeadd..1635c8822d 100644
> --- a/xen/arch/arm/time.c
> +++ b/xen/arch/arm/time.c
> @@ -29,6 +29,8 @@
>   #include <xen/sched.h>
>   #include <xen/event.h>
>   #include <xen/acpi.h>
> +#include <xen/cpu.h>
> +#include <xen/notifier.h>
>   #include <asm/system.h>
>   #include <asm/time.h>
>   #include <asm/vgic.h>
> @@ -312,6 +314,21 @@ void init_timer_interrupt(void)
>       check_timer_irq_cfg(timer_irq[TIMER_PHYS_NONSECURE_PPI], "NS-physical");
>   }
>   
> +/*
> + * Revert actions done in init_timer_interrupt that are required to properly
> + * disable this CPU.
> + */
> +static void deinit_timer_interrupt(void)
> +{
> +    WRITE_SYSREG32(0, CNTP_CTL_EL0);    /* Disable physical timer */
> +    WRITE_SYSREG32(0, CNTHP_CTL_EL2);   /* Disable hypervisor's timer */
> +    isb();
> +
> +    release_irq(timer_irq[TIMER_HYP_PPI], NULL);
> +    release_irq(timer_irq[TIMER_VIRT_PPI], NULL);
> +    release_irq(timer_irq[TIMER_PHYS_NONSECURE_PPI], NULL);
> +}
> +
>   /* Wait a set number of microseconds */
>   void udelay(unsigned long usecs)
>   {
> @@ -340,6 +357,34 @@ void domain_set_time_offset(struct domain *d, int64_t time_offset_seconds)
>       /* XXX update guest visible wallclock time */
>   }
>   
> +static int cpu_time_callback(struct notifier_block *nfb,
> +                             unsigned long action,
> +                             void *hcpu)
> +{
> +    switch ( action )
> +    {
> +    case CPU_DYING:
> +        deinit_timer_interrupt();
> +        break;
> +    default:
> +        break;
> +    }
> +
> +    return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block cpu_time_nfb = {
> +    .notifier_call = cpu_time_callback,
> +};
> +
> +static int __init cpu_time_notifier_init(void)
> +{
> +    register_cpu_notifier(&cpu_time_nfb);
> +
> +    return 0;
> +}
> +__initcall(cpu_time_notifier_init);
> +
>   /*
>    * Local variables:
>    * mode: C
> 

-- 
Julien Grall

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

Re: [PATCH v4 09/10] xen/arm: Free memory allocated for sibling/core maps on CPU hot-unplug

[Julien Grall]

Hi Mirela,

On 15/05/18 12:44, Mirela Simonovic wrote:
> The memory allocated in setup_cpu_sibling_map() when a CPU is hotplugged
> has to be freed when the CPU is hot-unplugged. This is done in
> remove_cpu_sibling_map() and called when the CPU dies. The call to
> remove_cpu_sibling_map() is made from a notifier callback when
> CPU_DEAD event is received.
> 
> Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>

Acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

> 
> ---
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien.grall@arm.com>
> ---
> Changes in v3:
> -Use notifier to trigger remove_cpu_sibling_map() when the CPU dies.
> 
> Changes in v4:
> -Initialize cpu_smpboot notifier at presmp_init rather than init phase
>   to cover the case where a secondary CPU dies beforehand the initcall
> -Added newline before the return in cpu_smpboot_notifier_init()
> -Fix indentation in cpu_smpboot_callback() definition
> ---
>   xen/arch/arm/smpboot.c | 36 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)
> 
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index ad1f6b751b..cf3a4ce659 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -89,6 +89,12 @@ static void setup_cpu_sibling_map(int cpu)
>       cpumask_set_cpu(cpu, per_cpu(cpu_core_mask, cpu));
>   }
>   
> +static void remove_cpu_sibling_map(int cpu)
> +{
> +    free_cpumask_var(per_cpu(cpu_sibling_mask, cpu));
> +    free_cpumask_var(per_cpu(cpu_core_mask, cpu));
> +}
> +
>   void __init
>   smp_clear_cpu_maps (void)
>   {
> @@ -499,6 +505,36 @@ void __cpu_die(unsigned int cpu)
>       smp_mb();
>   }
>   
> +static int cpu_smpboot_callback(struct notifier_block *nfb,
> +                                unsigned long action,
> +                                void *hcpu)
> +{
> +    unsigned int cpu = (unsigned long)hcpu;
> +
> +    switch ( action )
> +    {
> +    case CPU_DEAD:
> +        remove_cpu_sibling_map(cpu);
> +        break;
> +    default:
> +        break;
> +    }
> +
> +    return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block cpu_smpboot_nfb = {
> +    .notifier_call = cpu_smpboot_callback,
> +};
> +
> +static int __init cpu_smpboot_notifier_init(void)
> +{
> +    register_cpu_notifier(&cpu_smpboot_nfb);
> +
> +    return 0;
> +}
> +presmp_initcall(cpu_smpboot_notifier_init);
> +
>   /*
>    * Local variables:
>    * mode: C
> 

-- 
Julien Grall

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

Re: [PATCH v4 10/10] xen/arm: Enable errata for secondary CPU on hotplug after the boot

[Julien Grall]

On 15/05/18 12:44, Mirela Simonovic wrote:
> On boot, enabling errata workarounds will be triggered by the boot CPU
> from start_xen(). On CPU hotplug (non-boot scenario) this would not be
> done. This patch adds the code required to enable errata workarounds for
> a CPU being hotplugged after the system boots. This is triggered using
> a notifier. If the CPU fails to enable workarounds the notifier will
> return an error and Xen will hit the BUG_ON() in notify_cpu_starting().
> To avoid the BUG_ON() in an error case either enabling notifiers should
> be fixed to return void (not propagate error to notify_cpu_starting())
> and the errata notifier will always return success for CPU_STARTING
> event, or the notify_cpu_starting() and other common code should be
> fixed to expect an error at CPU_STARTING phase.
> 
> Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>

Reviewed-by: Julien Grall <julien.grall@arm.com>

> 
> ---
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien.grall@arm.com>
> ---
> Changes in v4:
> -Add includes alphabetically
> -Added newline before the return in cpu_errata_notifier_init()
> -Enabling capabilities returns an error if enabling a capability fails
>   (enable_nonboot_cpu_caps() returns int instead of void). When enabling
>   any of the capability fails the error is remembered into a variable and
>   the remaining capabilities are enabled. If enabling multiple capabilities
>   fails the error returned by enable_nonboot_cpu_caps() represents the
>   error code of the last failure.
> -Callback enable_nonboot_cpu_caps() can return an error when CPU_STARTING
>   fires. This is not right because of the assumption that starting a CPU
>   cannot fail at this phase. Consequently, if an error happens it will
>   cause Xen to hit the BUG_ON() in notify_cpu_starting(). In future,
>   either this notifier/enabling capabilities should be fixed to always
>   return success/void, or notify_cpu_starting() and other common code
>   should be fixed to expect an error at CPU_STARTING phase.
> -Fix commit message to reflect changes in v4
> ---
>   xen/arch/arm/cpuerrata.c         | 49 ++++++++++++++++++++++++++++++++++++++++
>   xen/arch/arm/cpufeature.c        | 29 ++++++++++++++++++++++++
>   xen/include/asm-arm/cpufeature.h |  1 +
>   3 files changed, 79 insertions(+)
> 
> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> index 1baa20654b..b829d226ef 100644
> --- a/xen/arch/arm/cpuerrata.c
> +++ b/xen/arch/arm/cpuerrata.c
> @@ -1,3 +1,4 @@
> +#include <xen/cpu.h>
>   #include <xen/cpumask.h>
>   #include <xen/mm.h>
>   #include <xen/sizes.h>
> @@ -5,6 +6,7 @@
>   #include <xen/spinlock.h>
>   #include <xen/vmap.h>
>   #include <xen/warning.h>
> +#include <xen/notifier.h>
>   #include <asm/cpufeature.h>
>   #include <asm/cpuerrata.h>
>   #include <asm/psci.h>
> @@ -349,6 +351,53 @@ void __init enable_errata_workarounds(void)
>       enable_cpu_capabilities(arm_errata);
>   }
>   
> +static int cpu_errata_callback(struct notifier_block *nfb,
> +                               unsigned long action,
> +                               void *hcpu)
> +{
> +    int rc = 0;
> +
> +    switch ( action )
> +    {
> +    case CPU_STARTING:
> +        /*
> +         * At CPU_STARTING phase no notifier shall return an error, because the
> +         * system is designed with the assumption that starting a CPU cannot
> +         * fail at this point. If an error happens here it will cause Xen to hit
> +         * the BUG_ON() in notify_cpu_starting(). In future, either this
> +         * notifier/enabling capabilities should be fixed to always return
> +         * success/void or notify_cpu_starting() and other common code should be
> +         * fixed to expect an error at CPU_STARTING phase.
> +         */
> +        ASSERT(system_state != SYS_STATE_boot);
> +        rc = enable_nonboot_cpu_caps(arm_errata);
> +        break;
> +    default:
> +        break;
> +    }
> +
> +    return !rc ? NOTIFY_DONE : notifier_from_errno(rc);
> +}
> +
> +static struct notifier_block cpu_errata_nfb = {
> +    .notifier_call = cpu_errata_callback,
> +};
> +
> +static int __init cpu_errata_notifier_init(void)
> +{
> +    register_cpu_notifier(&cpu_errata_nfb);
> +
> +    return 0;
> +}
> +/*
> + * Initialization has to be done at init rather than presmp_init phase because
> + * the callback should execute only after the secondary CPUs are initially
> + * booted (in hotplug scenarios when the system state is not boot). On boot,
> + * the enabling of errata workarounds will be triggered by the boot CPU from
> + * start_xen().
> + */
> +__initcall(cpu_errata_notifier_init);
> +
>   /*
>    * Local variables:
>    * mode: C
> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
> index 525b45e22f..3aaff4c0e6 100644
> --- a/xen/arch/arm/cpufeature.c
> +++ b/xen/arch/arm/cpufeature.c
> @@ -69,6 +69,35 @@ void __init enable_cpu_capabilities(const struct arm_cpu_capabilities *caps)
>   }
>   
>   /*
> + * Run through the enabled capabilities and enable() them on the calling CPU.
> + * If enabling of any capability fails the error is returned. After enabling a
> + * capability fails the error will be remembered into 'rc' and the remaining
> + * capabilities will be enabled. If enabling multiple capabilities fail the
> + * error returned by this function represents the error code of the last
> + * failure.
> + */
> +int enable_nonboot_cpu_caps(const struct arm_cpu_capabilities *caps)
> +{
> +    int rc = 0;
> +
> +    for ( ; caps->matches; caps++ )
> +    {
> +        if ( !cpus_have_cap(caps->capability) )
> +            continue;
> +
> +        if ( caps->enable )
> +        {
> +            int ret = caps->enable((void *)caps);
> +
> +            if ( ret )
> +                rc = ret;
> +        }
> +    }
> +
> +    return rc;
> +}
> +
> +/*
>    * Local variables:
>    * mode: C
>    * c-file-style: "BSD"
> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> index e557a095af..c5d046218b 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -88,6 +88,7 @@ void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
>                                const char *info);
>   
>   void enable_cpu_capabilities(const struct arm_cpu_capabilities *caps);
> +int enable_nonboot_cpu_caps(const struct arm_cpu_capabilities *caps);
>   
>   #endif /* __ASSEMBLY__ */
>   
> 

-- 
Julien Grall

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

Re: [PATCH v4 01/10] xen/arm64: Added handling of the trapped access to OSLSR register

[Mirela Simonovic]

[-- Attachment #1.1: Type: text/plain, Size: 2277 bytes --]

Hi Julien,

Thanks for the feedback.

On Tue, May 29, 2018 at 3:19 PM, Julien Grall <julien.grall@arm.com> wrote:

> Hi,
>
>
> On 15/05/18 12:44, Mirela Simonovic wrote:
>
>> Linux/dom0 accesses OSLSR register when saving CPU context during the
>> suspend procedure. Xen traps access to this register, but has no handling
>> for it. Consequently, Xen injects undef exception to linux, causing it to
>> crash. This patch adds handling of the trapped access to OSLSR as ro/raz.
>>
>> Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>
>> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
>> Acked-by: Julien Grall <julien.grall@arm.com>
>>
>> ---
>> CC: Stefano Stabellini <sstabellini@kernel.org>
>> CC: Julien Grall <julien.grall@arm.com>
>> ---
>> Changes in v2:
>> - Commit message fix (arm64 related change instead of arm)
>> - Add Stefano's reviewed-by
>>
>> Changes in v3:
>> - Added Julien's acked-by
>> ---
>>   xen/arch/arm/arm64/vsysreg.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
>> index c57ac12503..8f80e1735e 100644
>> --- a/xen/arch/arm/arm64/vsysreg.c
>> +++ b/xen/arch/arm/arm64/vsysreg.c
>> @@ -57,13 +57,14 @@ void do_sysreg(struct cpu_user_regs *regs,
>>        * ARMv8 (DDI 0487A.d): D1-1509 Table D1-58
>>        *
>>        * Unhandled:
>> -     *    OSLSR_EL1
>>        *    DBGPRCR_EL1
>>        */
>>       case HSR_SYSREG_OSLAR_EL1:
>>           return handle_wo_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
>>       case HSR_SYSREG_OSDLR_EL1:
>>           return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
>> +    case HSR_SYSREG_OSLSR_EL1:
>> +        return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 1);
>>
>
> Looking at this patch again, OSLM (bit[3] and bit[1]) should be 10 as all
> other values are reserved. So you want to set bit 3 when the guest read it.
>
>
Could you please let me know is my proposal/understanding correct? Instead
of handle_ro_raz, I assume we need this:

        if ( psr_mode_is_user(regs) )
            return inject_undef_exception(regs, hsr);
        if ( hsr.sysreg.read )
            set_user_reg(regs, regidx, 0x8);
        break;

Thanks,
Mirela

Cheers,
>
> --
> Julien Grall
>

[-- Attachment #1.2: Type: text/html, Size: 4064 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: [PATCH v4 01/10] xen/arm64: Added handling of the trapped access to OSLSR register

[Mirela Simonovic]

[-- Attachment #1.1: Type: text/plain, Size: 2701 bytes --]

On Wed, May 30, 2018 at 11:48 AM, Mirela Simonovic <
mirela.simonovic@aggios.com> wrote:

> Hi Julien,
>
> Thanks for the feedback.
>
> On Tue, May 29, 2018 at 3:19 PM, Julien Grall <julien.grall@arm.com>
> wrote:
>
>> Hi,
>>
>>
>> On 15/05/18 12:44, Mirela Simonovic wrote:
>>
>>> Linux/dom0 accesses OSLSR register when saving CPU context during the
>>> suspend procedure. Xen traps access to this register, but has no handling
>>> for it. Consequently, Xen injects undef exception to linux, causing it to
>>> crash. This patch adds handling of the trapped access to OSLSR as ro/raz.
>>>
>>> Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>
>>> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
>>> Acked-by: Julien Grall <julien.grall@arm.com>
>>>
>>> ---
>>> CC: Stefano Stabellini <sstabellini@kernel.org>
>>> CC: Julien Grall <julien.grall@arm.com>
>>> ---
>>> Changes in v2:
>>> - Commit message fix (arm64 related change instead of arm)
>>> - Add Stefano's reviewed-by
>>>
>>> Changes in v3:
>>> - Added Julien's acked-by
>>> ---
>>>   xen/arch/arm/arm64/vsysreg.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
>>> index c57ac12503..8f80e1735e 100644
>>> --- a/xen/arch/arm/arm64/vsysreg.c
>>> +++ b/xen/arch/arm/arm64/vsysreg.c
>>> @@ -57,13 +57,14 @@ void do_sysreg(struct cpu_user_regs *regs,
>>>        * ARMv8 (DDI 0487A.d): D1-1509 Table D1-58
>>>        *
>>>        * Unhandled:
>>> -     *    OSLSR_EL1
>>>        *    DBGPRCR_EL1
>>>        */
>>>       case HSR_SYSREG_OSLAR_EL1:
>>>           return handle_wo_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
>>>       case HSR_SYSREG_OSDLR_EL1:
>>>           return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
>>> +    case HSR_SYSREG_OSLSR_EL1:
>>> +        return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 1);
>>>
>>
>> Looking at this patch again, OSLM (bit[3] and bit[1]) should be 10 as all
>> other values are reserved. So you want to set bit 3 when the guest read it.
>>
>>
> Could you please let me know is my proposal/understanding correct? Instead
> of handle_ro_raz, I assume we need this:
>
>         if ( psr_mode_is_user(regs) )
>             return inject_undef_exception(regs, hsr);
>         if ( hsr.sysreg.read )
>             set_user_reg(regs, regidx, 0x8);
>         break;
>
>

Please note - I'm not sure how should write to this register be treated. In
the original patch (handle_ro_raz implementation) undef exception will be
raised. I realized in what I proposed that wouldn't be the case, it's a
write-ignore.


> Thanks,
> Mirela
>
> Cheers,
>>
>> --
>> Julien Grall
>>
>
>

[-- Attachment #1.2: Type: text/html, Size: 5483 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: [PATCH v4 01/10] xen/arm64: Added handling of the trapped access to OSLSR register

[Julien Grall]

Hi Mirela,

On 30/05/18 10:53, Mirela Simonovic wrote:
> 
> 
> On Wed, May 30, 2018 at 11:48 AM, Mirela Simonovic 
> <mirela.simonovic@aggios.com <mailto:mirela.simonovic@aggios.com>> wrote:
> Please note - I'm not sure how should write to this register be treated. 
> In the original patch (handle_ro_raz implementation) undef exception 
> will be raised. I realized in what I proposed that wouldn't be the case, 
> it's a write-ignore.

I think you want to keep the inject_undef_exception here to stay 
consistent with the rest of RO register implementation. I would suggest 
to create a new helper handle_ro_read_val(...) that will take the value 
to return in parameter.

handle_ro_raz could then be implemented using the new helper.

Also, I would prefer if you use 1 << 3 as it is clearer that bit 3 is set.

Cheers,

-- 
Julien Grall

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