This patch contains a few general comments in the source code of the
arch subsystem.

Signed-off-by: Claudio Scordino <[email protected]>
---
 hypervisor/arch/arm-common/control.c     |  6 ++++++
 hypervisor/arch/arm/include/asm/percpu.h |  3 +++
 hypervisor/arch/x86/apic.c               | 12 ++++++++++++
 hypervisor/arch/x86/control.c            |  6 ++++++
 hypervisor/arch/x86/entry.S              |  7 +++++++
 hypervisor/arch/x86/vmx.c                |  6 ++++++
 hypervisor/include/jailhouse/control.h   |  4 ++--
 7 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/hypervisor/arch/arm-common/control.c 
b/hypervisor/arch/arm-common/control.c
index 9c40e53..4cc1541 100644
--- a/hypervisor/arch/arm-common/control.c
+++ b/hypervisor/arch/arm-common/control.c
@@ -57,6 +57,12 @@ void arch_suspend_cpu(unsigned int cpu_id)
        spin_unlock(&target_data->control_lock);
 
        if (!target_suspended) {
+               /*
+                * Send a maintenance signal (SGI_EVENT) to the target CPU.
+                * Then, wait for the target CPU to enter the suspended state.
+                * The target CPU, in turn, will leave the guest and handle the
+                * request in the event loop.
+                */
                arm_cpu_kick(cpu_id);
 
                while (!target_data->cpu_suspended)
diff --git a/hypervisor/arch/arm/include/asm/percpu.h 
b/hypervisor/arch/arm/include/asm/percpu.h
index 77e0414..70f0118 100644
--- a/hypervisor/arch/arm/include/asm/percpu.h
+++ b/hypervisor/arch/arm/include/asm/percpu.h
@@ -30,7 +30,10 @@
        (BITS_PER_LONG - __builtin_clzl(sizeof(struct per_cpu) - 1))
 
 struct per_cpu {
+       /** Stack used while in hypervisor mode. */
        u8 stack[PAGE_SIZE];
+
+       /** Linux stack pointer, used for handover to the hypervisor. */
        unsigned long linux_sp;
        unsigned long linux_ret;
        unsigned long linux_flags;
diff --git a/hypervisor/arch/x86/apic.c b/hypervisor/arch/x86/apic.c
index 84b92ea..b3558fe 100644
--- a/hypervisor/arch/x86/apic.c
+++ b/hypervisor/arch/x86/apic.c
@@ -23,7 +23,17 @@
 
 #define XAPIC_REG(x2apic_reg)          ((x2apic_reg) << 4)
 
+ /**
+  * Modern x86 processors are equipped with a local APIC that handles delivery
+  * of external interrupts. The APIC can work in two modes:
+  * - xAPIC: programmed via memory mapped I/O (MMIO)
+  * - x2APIC: programmed throughs model-specific registers (MSRs)
+  */
 bool using_x2apic;
+
+/**
+ * Mapping from a physical APIC ID to the logical CPU ID as used by Jailhouse.
+ */
 u8 apic_to_cpu_id[] = { [0 ... APIC_MAX_PHYS_ID] = CPU_ID_INVALID };
 
 /* Initialized for x2APIC, adjusted for xAPIC during init */
@@ -163,12 +173,14 @@ int apic_init(void)
        unsigned long apicbase = read_msr(MSR_IA32_APICBASE);
 
        if (apicbase & APIC_BASE_EXTD) {
+               /* x2APIC mode */
                apic_ops.read = read_x2apic;
                apic_ops.read_id = read_x2apic_id;
                apic_ops.write = write_x2apic;
                apic_ops.send_ipi = send_x2apic_ipi;
                using_x2apic = true;
        } else if (apicbase & APIC_BASE_EN) {
+               /* xAPIC mode */
                xapic_page = paging_map_device(XAPIC_BASE, PAGE_SIZE);
                if (!xapic_page)
                        return -ENOMEM;
diff --git a/hypervisor/arch/x86/control.c b/hypervisor/arch/x86/control.c
index 46bf2cb..6f1d568 100644
--- a/hypervisor/arch/x86/control.c
+++ b/hypervisor/arch/x86/control.c
@@ -161,6 +161,12 @@ void arch_suspend_cpu(unsigned int cpu_id)
        spin_unlock(&target_data->control_lock);
 
        if (!target_suspended) {
+               /*
+                * Send a maintenance signal (NMI) to the target CPU.
+                * Then, wait for the target CPU to enter the suspended state.
+                * The target CPU, in turn, will leave the guest and handle the
+                * request in the event loop.
+                */
                apic_send_nmi_ipi(target_data);
 
                while (!target_data->cpu_suspended)
diff --git a/hypervisor/arch/x86/entry.S b/hypervisor/arch/x86/entry.S
index 986bfee..e97a6fa 100644
--- a/hypervisor/arch/x86/entry.S
+++ b/hypervisor/arch/x86/entry.S
@@ -25,17 +25,24 @@ arch_entry:
        push %r14
        push %r15
 
+       /* Locate the per_cpu region for the given CPU ID*/
        mov %rdi,%rsi
        shl $PERCPU_SIZE_SHIFT_ASM,%rsi
        lea __page_pool(%rip),%rax
        add %rax,%rsi
 
+       /* Save the Linux stack pointer inside the per_cpu region */
        mov %rsp,PERCPU_LINUX_SP(%rsi)
 
+       /* Set the Jailhouse stack pointer */
        lea PERCPU_STACK_END-8(%rsi),%rsp
 
        push %rsi
 
+       /*
+        * Call the architecture-independent entry(cpuid, struct per_cpu*)
+        * function.
+        */
        call entry
 
        pop %rsi
diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index f832612..7336aa7 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -78,6 +78,8 @@ static u8 __attribute__((aligned(PAGE_SIZE))) 
msr_bitmap[][0x2000/8] = {
                [      0/8 ... 0x1fff/8 ] = 0,
        },
 };
+
+/* Special access page to trap guest's attempts of accessing APIC in xAPIC 
mode */
 static u8 __attribute__((aligned(PAGE_SIZE))) apic_access_page[PAGE_SIZE];
 static struct paging ept_paging[EPT_PAGE_DIR_LEVELS];
 static u32 secondary_exec_addon;
@@ -339,6 +341,8 @@ int vcpu_vendor_cell_init(struct cell *cell)
        cell->arch.vmx.ept_structs.root_table =
                (page_table_t)cell->arch.root_table_page;
 
+       /* Map the special APIC access page into the guest's physical address
+        * space at the default address (XAPIC_BASE) */
        err = paging_create(&cell->arch.vmx.ept_structs,
                            paging_hvirt2phys(apic_access_page),
                            PAGE_SIZE, XAPIC_BASE,
@@ -511,6 +515,8 @@ static bool vmcs_setup(struct per_cpu *cpu_data)
 
        ok &= vmcs_write64(HOST_RSP, (unsigned long)cpu_data->stack +
                           sizeof(cpu_data->stack));
+
+       /* Set function executed when trapping to the hypervisor */
        ok &= vmcs_write64(HOST_RIP, (unsigned long)vmx_vmexit);
 
        ok &= vmx_set_guest_cr(CR0_IDX, cpu_data->linux_cr0);
diff --git a/hypervisor/include/jailhouse/control.h 
b/hypervisor/include/jailhouse/control.h
index bfc5cc9..29ed9ac 100644
--- a/hypervisor/include/jailhouse/control.h
+++ b/hypervisor/include/jailhouse/control.h
@@ -134,8 +134,8 @@ void panic_park(void);
  * @param cpu_id       ID of the target CPU.
  *
  * Suspension means that the target CPU is no longer executing cell code or
- * arbitrary hypervisor code. It may actively wait in the hypervisor context,
- * so the suspension time should be kept short.
+ * arbitrary hypervisor code. It may actively busy-wait in the hypervisor
+ * context, so the suspension time should be kept short.
  *
  * The function waits for the target CPU to enter suspended state.
  *
-- 
2.7.4

-- 
You received this message because you are subscribed to the Google Groups 
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to