[PATCH 01/31] x86: Remove local cpu_data variable from vcpu_handle_hypercall

[Jan Kiszka]

Only used once.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
hypervisor/arch/x86/vcpu.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
index 12e35bd..cf24cf4 100644
--- a/hypervisor/arch/x86/vcpu.c
+++ b/hypervisor/arch/x86/vcpu.c
@@ -138,7 +138,6 @@ void vcpu_handle_hypercall(struct registers *guest_regs,
{
bool long_mode = !!(x_state->efer & EFER_LMA);
unsigned long arg_mask = long_mode ? (u64)-1 : (u32)-1;
- struct per_cpu *cpu_data = this_cpu_data();
unsigned long code = guest_regs->rax;

vcpu_skip_emulated_instruction(X86_INST_LEN_HYPERCALL);
@@ -153,7 +152,7 @@ void vcpu_handle_hypercall(struct registers *guest_regs,
guest_regs->rsi & arg_mask);
if (guest_regs->rax == -ENOSYS)
printk("CPU %d: Unknown hypercall %d, RIP: %p\n",
- cpu_data->cpu_id, code,
+ this_cpu_id(), code,
x_state->rip - X86_INST_LEN_HYPERCALL);

if (code == JAILHOUSE_HC_DISABLE && guest_regs->rax == 0)
--
2.1.4

[Jan Kiszka]

No use case in sight.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/apic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hypervisor/arch/x86/apic.c b/hypervisor/arch/x86/apic.c
index 67a4453..f173673 100644
--- a/hypervisor/arch/x86/apic.c
+++ b/hypervisor/arch/x86/apic.c
@@ -327,7 +327,7 @@ void apic_clear(void)
apic_ops.write(APIC_REG_TPR, 0);
}

-static bool apic_valid_ipi_mode(struct per_cpu *cpu_data, u32 lo_val)
+static bool apic_valid_ipi_mode(u32 lo_val)
{
switch (lo_val & APIC_ICR_DLVR_MASK) {
case APIC_ICR_DLVR_INIT:
@@ -416,7 +416,7 @@ bool apic_handle_icr_write(struct per_cpu *cpu_data, u32 lo_val, u32 hi_val)
unsigned int target_cpu_id;
unsigned long dest;

- if (!apic_valid_ipi_mode(cpu_data, lo_val))
+ if (!apic_valid_ipi_mode(lo_val))
return false;

if ((lo_val & APIC_ICR_SH_MASK) == APIC_ICR_SH_SELF) {
--
2.1.4

[Jan Kiszka]

The function only works against the current CPU, thus should avoid to
take the misleading parameter. The necessary reference can be obtained
inline.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/apic.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/hypervisor/arch/x86/apic.c b/hypervisor/arch/x86/apic.c
index f173673..5528064 100644
--- a/hypervisor/arch/x86/apic.c
+++ b/hypervisor/arch/x86/apic.c
@@ -354,13 +354,13 @@ static bool apic_valid_ipi_mode(u32 lo_val)
return true;
}

-static void apic_send_ipi(struct per_cpu *cpu_data, unsigned int target_cpu_id,
- u32 orig_icr_hi, u32 icr_lo)
+static void apic_send_ipi(unsigned int target_cpu_id, u32 orig_icr_hi,
+ u32 icr_lo)
{
- if (!cell_owns_cpu(cpu_data->cell, target_cpu_id)) {
+ if (!cell_owns_cpu(this_cell(), target_cpu_id)) {
printk("WARNING: CPU %d specified IPI destination outside "
"cell boundaries, ICR.hi=%x\n",
- cpu_data->cpu_id, orig_icr_hi);
+ this_cpu_id(), orig_icr_hi);
return;
}

@@ -381,8 +381,7 @@ static void apic_send_ipi(struct per_cpu *cpu_data, unsigned int target_cpu_id,
}
}

-static void apic_send_logical_dest_ipi(struct per_cpu *cpu_data,
- unsigned long dest, u32 lo_val,
+static void apic_send_logical_dest_ipi(unsigned long dest, u32 lo_val,
u32 hi_val)
{
unsigned int target_cpu_id = CPU_ID_INVALID;
@@ -401,13 +400,13 @@ static void apic_send_logical_dest_ipi(struct per_cpu *cpu_data,
(cluster_id << X2APIC_CLUSTER_ID_SHIFT);
if (apic_id <= APIC_MAX_PHYS_ID)
target_cpu_id = apic_to_cpu_id[apic_id];
- apic_send_ipi(cpu_data, target_cpu_id, hi_val, lo_val);
+ apic_send_ipi(target_cpu_id, hi_val, lo_val);
}
} else
while (dest != 0) {
target_cpu_id = ffsl(dest);
dest &= ~(1UL << target_cpu_id);
- apic_send_ipi(cpu_data, target_cpu_id, hi_val, lo_val);
+ apic_send_ipi(target_cpu_id, hi_val, lo_val);
}
}

@@ -433,12 +432,12 @@ bool apic_handle_icr_write(struct per_cpu *cpu_data, u32 lo_val, u32 hi_val)

if (lo_val & APIC_ICR_DEST_LOGICAL) {
lo_val &= ~APIC_ICR_DEST_LOGICAL;
- apic_send_logical_dest_ipi(cpu_data, dest, lo_val, hi_val);
+ apic_send_logical_dest_ipi(dest, lo_val, hi_val);
} else {
target_cpu_id = CPU_ID_INVALID;
if (dest <= APIC_MAX_PHYS_ID)
target_cpu_id = apic_to_cpu_id[dest];
- apic_send_ipi(cpu_data, target_cpu_id, hi_val, lo_val);
+ apic_send_ipi(target_cpu_id, hi_val, lo_val);
}
return true;
}
--
2.1.4

[Jan Kiszka]

The function only works against the current CPU, thus should avoid to
take the misleading parameter. The necessary reference can be obtained
inline.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/apic.c | 4 ++--
hypervisor/arch/x86/control.c | 4 ++--
hypervisor/arch/x86/include/asm/apic.h | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hypervisor/arch/x86/apic.c b/hypervisor/arch/x86/apic.c
index 61e2216..67a4453 100644
--- a/hypervisor/arch/x86/apic.c
+++ b/hypervisor/arch/x86/apic.c
@@ -289,7 +289,7 @@ static void apic_mask_lvt(unsigned int reg)
apic_ops.write(reg, val | APIC_LVT_MASKED);
}

-void apic_clear(struct per_cpu *cpu_data)
+void apic_clear(void)
{
unsigned int maxlvt = (apic_ops.read(APIC_REG_LVR) >> 16) & 0xff;
unsigned int xlc = (apic_ext_features() >> 16) & 0xff;
@@ -318,7 +318,7 @@ void apic_clear(struct per_cpu *cpu_data)
/* Consume pending interrupts to clear IRR.
* Need to reset TPR to ensure interrupt delivery. */
apic_ops.write(APIC_REG_TPR, 0);
- cpu_data->num_clear_apic_irqs = 0;
+ this_cpu_data()->num_clear_apic_irqs = 0;
enable_irq();
cpu_relax();
disable_irq();
diff --git a/hypervisor/arch/x86/control.c b/hypervisor/arch/x86/control.c
index 90b2ea8..8e2f66d 100644
--- a/hypervisor/arch/x86/control.c
+++ b/hypervisor/arch/x86/control.c
@@ -237,7 +237,7 @@ int x86_handle_events(struct per_cpu *cpu_data)
cpu_relax();

if (cpu_data->shutdown_cpu) {
- apic_clear(cpu_data);
+ apic_clear();
vcpu_exit(cpu_data);
asm volatile("1: hlt; jmp 1b");
}
@@ -267,7 +267,7 @@ int x86_handle_events(struct per_cpu *cpu_data)
if (cpu_data->wait_for_sipi)
vcpu_park(cpu_data);
else if (sipi_vector >= 0)
- apic_clear(cpu_data);
+ apic_clear();

return sipi_vector;
}
diff --git a/hypervisor/arch/x86/include/asm/apic.h b/hypervisor/arch/x86/include/asm/apic.h
index 89856cd..f6c7df1 100644
--- a/hypervisor/arch/x86/include/asm/apic.h
+++ b/hypervisor/arch/x86/include/asm/apic.h
@@ -153,7 +153,7 @@ extern u8 apic_to_cpu_id[];
int apic_init(void);
int apic_cpu_init(struct per_cpu *cpu_data);

-void apic_clear(struct per_cpu *cpu_data);
+void apic_clear(void);

void apic_send_nmi_ipi(struct per_cpu *target_data);
bool apic_filter_irq_dest(struct cell *cell, struct apic_irq_message *irq_msg);
--
2.1.4

[Jan Kiszka]

This is part two of flushing out next. It's about dropping some per-cpu
function parameters and local variables. Moreover, it refactors struct
registers to a union and allows to obtain that one directly from struct
per_cpu.

To recall: The philosophy behind removing per-cpu parameters from our
internal (inter-module) interfaces is not only to avoid having to pass
them down to deep call stacks but primarily to document that functions
work on the current CPU only, thus the parameter cannot be freely
chosen.

Jan

Jan Kiszka (31):
x86: Remove local cpu_data variable from vcpu_handle_hypercall
x86: Remove cpu_data parameter from apic_clear
x86: Remove unused parameter from apic_valid_ipi_mode
x86: Remove cpu_data parameter from apic_send_[logical_dest_]ipi
x86: Update apic_handle_icr_write signature
x86: Remove cpu_data parameter from vcpu_park
x86: Obtain execution state inside vcpu_handle_hypercall
x86: Retrieve vcpu_io_intercept from vcpu_handle_io_access
x86: Rename parameter of vcpu_vendor_get_io_intercept
x86: Rename vcpu_handle_pt_violation to vcpu_handle_mmio_access
x86: Retrieve vcpu_mmio_intercept from vcpu_handle_mmio_access
x86: Factor out vcpu_handle_xsetbv
x86: Allow index-based guest register access without type casts
x86: Reorder stack layout in svm_vmexit
x86: Enable direct access to per-cpu guest registers
x86: Remove guest registers parameter from vcpu_handle_exit
x86: Remove guest registers parameter from vcpu_reset
x86: Remove guest registers parameter from vcpu_deactivate_vmm
x86: Remove guest registers parameter from vcpu_handle_hypercall
x86: Remove guest registers parameter from vcpu_handle_xsetbv
x86: Remove parameters from x2apic_handle_read/write
x86: Remove guest registers and cpu_data parameters from
apic_mmio_access
x86: Remove guest registers parameter from i8042_access_handler
x86: Rework RAX register accessors of PCI layer
x86: Remove guest registers and cell parameters from
x86_pci_config_handler
x86: Remove unused guest registers parameter from
vcpu_handle_io_access
x86: Remove guest registers parameter from vcpu_handle_mmio_access
x86: Remove guest registers parameter from vcpu_handle_msr_read/write
x86: Remove parameters from vmx_handle_cr
x86: Factor out vmx_handle_cpuid
x86: Drop local guest_regs variable from VMX version of
vcpu_handle_exit

hypervisor/arch/x86/apic.c | 49 ++++++-----
hypervisor/arch/x86/control.c | 8 +-
hypervisor/arch/x86/i8042.c | 4 +-
hypervisor/arch/x86/include/asm/apic.h | 12 +--
hypervisor/arch/x86/include/asm/i8042.h | 5 +-
hypervisor/arch/x86/include/asm/pci.h | 4 +-
hypervisor/arch/x86/include/asm/percpu.h | 14 +++-
hypervisor/arch/x86/include/asm/processor.h | 41 +++++-----
hypervisor/arch/x86/include/asm/vcpu.h | 31 ++++---
hypervisor/arch/x86/pci.c | 57 ++++++-------
hypervisor/arch/x86/svm-vmexit.S | 13 ++-
hypervisor/arch/x86/svm.c | 93 ++++++++-------------
hypervisor/arch/x86/vcpu.c | 123 +++++++++++++++++-----------
hypervisor/arch/x86/vmx-vmexit.S | 3 +-
hypervisor/arch/x86/vmx.c | 103 ++++++++++-------------
15 files changed, 270 insertions(+), 290 deletions(-)

--
2.1.4

[Jan Kiszka]

apic_handle_icr_write became locally used only by 3da0bd91d9, and we
also no longer use the cpu_data parameter.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/apic.c | 6 +++---
hypervisor/arch/x86/include/asm/apic.h | 2 --
2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/hypervisor/arch/x86/apic.c b/hypervisor/arch/x86/apic.c
index 5528064..ac06fb3 100644
--- a/hypervisor/arch/x86/apic.c
+++ b/hypervisor/arch/x86/apic.c
@@ -410,7 +410,7 @@ static void apic_send_logical_dest_ipi(unsigned long dest, u32 lo_val,
}
}

-bool apic_handle_icr_write(struct per_cpu *cpu_data, u32 lo_val, u32 hi_val)
+static bool apic_handle_icr_write(u32 lo_val, u32 hi_val)
{
unsigned int target_cpu_id;
unsigned long dest;
@@ -490,7 +490,7 @@ unsigned int apic_mmio_access(struct registers *guest_regs,
return 0;

if (reg == APIC_REG_ICR) {
- if (!apic_handle_icr_write(cpu_data, val,
+ if (!apic_handle_icr_write(val,
apic_ops.read(APIC_REG_ICR_HI)))
return 0;
} else if (reg == APIC_REG_LDR &&
@@ -530,7 +530,7 @@ bool x2apic_handle_write(struct registers *guest_regs,
/* TODO: emulate */
printk("Unhandled x2APIC self IPI write\n");
else if (reg == APIC_REG_ICR)
- return apic_handle_icr_write(cpu_data, val, guest_regs->rdx);
+ return apic_handle_icr_write(val, guest_regs->rdx);
else if (reg >= APIC_REG_LVTCMCI && reg <= APIC_REG_LVTERR &&
apic_invalid_lvt_delivery_mode(reg, val))
return false;
diff --git a/hypervisor/arch/x86/include/asm/apic.h b/hypervisor/arch/x86/include/asm/apic.h
index f6c7df1..e8eea68 100644
--- a/hypervisor/arch/x86/include/asm/apic.h
+++ b/hypervisor/arch/x86/include/asm/apic.h
@@ -161,8 +161,6 @@ void apic_send_irq(struct apic_irq_message irq_msg);

void apic_irq_handler(void);

-bool apic_handle_icr_write(struct per_cpu *cpu_data, u32 lo_val, u32 hi_val);
-
unsigned int apic_mmio_access(struct registers *guest_regs,
struct per_cpu *cpu_data, unsigned long rip,
const struct guest_paging_structures *pg_structs,
--
2.1.4

[Jan Kiszka]

The function only works against the current CPU, thus should avoid to

take the misleading parameter. The implementations can obtain the
reference inline as needed.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/control.c | 4 ++--
hypervisor/arch/x86/include/asm/vcpu.h | 2 +-
hypervisor/arch/x86/svm.c | 8 +++-----
hypervisor/arch/x86/vmx.c | 2 +-
4 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/hypervisor/arch/x86/control.c b/hypervisor/arch/x86/control.c
index 8e2f66d..a9e2172 100644
--- a/hypervisor/arch/x86/control.c
+++ b/hypervisor/arch/x86/control.c
@@ -265,7 +265,7 @@ int x86_handle_events(struct per_cpu *cpu_data)
/* wait_for_sipi is only modified on this CPU, so checking outside of
* control_lock is fine */
if (cpu_data->wait_for_sipi)
- vcpu_park(cpu_data);
+ vcpu_park();

else if (sipi_vector >= 0)

apic_clear();

@@ -304,5 +304,5 @@ void arch_panic_park(void)
x86_enter_wait_for_sipi(cpu_data);
spin_unlock(&cpu_data->control_lock);

- vcpu_park(cpu_data);
+ vcpu_park();
}
diff --git a/hypervisor/arch/x86/include/asm/vcpu.h b/hypervisor/arch/x86/include/asm/vcpu.h
index e8f5b47..1ec14e2 100644
--- a/hypervisor/arch/x86/include/asm/vcpu.h
+++ b/hypervisor/arch/x86/include/asm/vcpu.h
@@ -72,7 +72,7 @@ vcpu_deactivate_vmm(struct registers *guest_regs);

void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data);

-void vcpu_park(struct per_cpu *cpu_data);
+void vcpu_park(void);

void vcpu_nmi_handler(void);

diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c
index f65d61c..88cf47f 100644
--- a/hypervisor/arch/x86/svm.c
+++ b/hypervisor/arch/x86/svm.c
@@ -1027,13 +1027,11 @@ void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data)
panic_park();
}

-void vcpu_park(struct per_cpu *cpu_data)
+void vcpu_park(void)
{
- struct vmcb *vmcb = &cpu_data->vmcb;
-
- svm_vcpu_reset(cpu_data, APIC_BSP_PSEUDO_SIPI);
+ svm_vcpu_reset(this_cpu_data(), APIC_BSP_PSEUDO_SIPI);
/* No need to clear VMCB Clean bit: vcpu_reset() already does this */
- vmcb->n_cr3 = paging_hvirt2phys(parked_mode_npt);
+ this_cpu_data()->vmcb.n_cr3 = paging_hvirt2phys(parked_mode_npt);

vcpu_tlb_flush();
}
diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index 3a2456f..931856c 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -861,7 +861,7 @@ void vcpu_nmi_handler(void)
vmcs_write32(PIN_BASED_VM_EXEC_CONTROL, pin_based_ctrl);
}

-void vcpu_park(struct per_cpu *cpu_data)
+void vcpu_park(void)
{
vmx_vcpu_reset(0);
vmcs_write32(GUEST_ACTIVITY_STATE, GUEST_ACTIVITY_HLT);
--
2.1.4

[Jan Kiszka]

This simplifies the invocation of the handler and aligns it with
vcpu_handle_pt_violation.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/include/asm/vcpu.h | 3 +--
hypervisor/arch/x86/svm.c | 4 +---
hypervisor/arch/x86/vcpu.c | 19 ++++++++++++-------
hypervisor/arch/x86/vmx.c | 4 +---
4 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/hypervisor/arch/x86/include/asm/vcpu.h b/hypervisor/arch/x86/include/asm/vcpu.h
index 1ec14e2..6f1ae92 100644
--- a/hypervisor/arch/x86/include/asm/vcpu.h
+++ b/hypervisor/arch/x86/include/asm/vcpu.h
@@ -100,8 +100,7 @@ void vcpu_vendor_get_cell_io_bitmap(struct cell *cell,

void vcpu_vendor_get_execution_state(struct vcpu_execution_state *x_state);

-void vcpu_handle_hypercall(struct registers *guest_regs,
- struct vcpu_execution_state *x_state);
+void vcpu_handle_hypercall(struct registers *guest_regs);

bool vcpu_handle_io_access(struct registers *guest_regs,
struct vcpu_io_intercept *io);
diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c
index 88cf47f..25376cb 100644
--- a/hypervisor/arch/x86/svm.c
+++ b/hypervisor/arch/x86/svm.c
@@ -920,7 +920,6 @@ static void svm_get_vcpu_io_intercept(struct per_cpu *cpu_data,

void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data)

{

struct vmcb *vmcb = &cpu_data->vmcb;

- struct vcpu_execution_state x_state;
struct vcpu_pf_intercept pf;
struct vcpu_io_intercept io;
bool res = false;
@@ -958,8 +957,7 @@ void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data)
/* FIXME: We are not intercepting CPUID now */
return;
case VMEXIT_VMMCALL:
- vcpu_vendor_get_execution_state(&x_state);
- vcpu_handle_hypercall(guest_regs, &x_state);
+ vcpu_handle_hypercall(guest_regs);
return;
case VMEXIT_CR0_SEL_WRITE:
cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_CR]++;
diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
index cf24cf4..5911fe1 100644
--- a/hypervisor/arch/x86/vcpu.c
+++ b/hypervisor/arch/x86/vcpu.c
@@ -133,17 +133,22 @@ void vcpu_cell_exit(struct cell *cell)
vcpu_vendor_cell_exit(cell);
}

-void vcpu_handle_hypercall(struct registers *guest_regs,
- struct vcpu_execution_state *x_state)
+void vcpu_handle_hypercall(struct registers *guest_regs)
{
- bool long_mode = !!(x_state->efer & EFER_LMA);
- unsigned long arg_mask = long_mode ? (u64)-1 : (u32)-1;

unsigned long code = guest_regs->rax;

+ struct vcpu_execution_state x_state;
+ unsigned long arg_mask;
+ bool long_mode;

vcpu_skip_emulated_instruction(X86_INST_LEN_HYPERCALL);

- if ((!long_mode && (x_state->rflags & X86_RFLAGS_VM)) ||
- (x_state->cs & 3) != 0) {
+ vcpu_vendor_get_execution_state(&x_state);
+
+ long_mode = !!(x_state.efer & EFER_LMA);
+ arg_mask = long_mode ? (u64)-1 : (u32)-1;
+
+ if ((!long_mode && (x_state.rflags & X86_RFLAGS_VM)) ||
+ (x_state.cs & 3) != 0) {
guest_regs->rax = -EPERM;
return;
}
@@ -153,7 +158,7 @@ void vcpu_handle_hypercall(struct registers *guest_regs,

if (guest_regs->rax == -ENOSYS)
printk("CPU %d: Unknown hypercall %d, RIP: %p\n",

this_cpu_id(), code,
- x_state->rip - X86_INST_LEN_HYPERCALL);
+ x_state.rip - X86_INST_LEN_HYPERCALL);

if (code == JAILHOUSE_HC_DISABLE && guest_regs->rax == 0)

vcpu_deactivate_vmm(guest_regs);
diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index 931856c..90b2144 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -1041,7 +1041,6 @@ static void vmx_get_vcpu_pf_intercept(struct vcpu_pf_intercept *out)

void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data)

{
u32 reason = vmcs_read32(VM_EXIT_REASON);
- struct vcpu_execution_state x_state;
struct vcpu_io_intercept io;
struct vcpu_pf_intercept pf;
int sipi_vector;
@@ -1074,8 +1073,7 @@ void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data)
(u32 *)&guest_regs->rcx, (u32 *)&guest_regs->rdx);
return;
case EXIT_REASON_VMCALL:
- vcpu_vendor_get_execution_state(&x_state);
- vcpu_handle_hypercall(guest_regs, &x_state);
+ vcpu_handle_hypercall(guest_regs);
return;
case EXIT_REASON_CR_ACCESS:
cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_CR]++;
--
2.1.4

[Jan Kiszka]

Convert the vendor-specific functions into vcpu_vendor_get_io_intercept
and invoke that one from vcpu_handle_io_access. That offloads this
burden from the callers of vcpu_handle_io_access and takes us further
towards consistent vendor callbacks for such purposes.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/include/asm/vcpu.h | 10 +++++-----
hypervisor/arch/x86/svm.c | 9 +++------
hypervisor/arch/x86/vcpu.c | 20 +++++++++++---------
hypervisor/arch/x86/vmx.c | 6 ++----
4 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/hypervisor/arch/x86/include/asm/vcpu.h b/hypervisor/arch/x86/include/asm/vcpu.h
index 6f1ae92..9edddc8 100644
--- a/hypervisor/arch/x86/include/asm/vcpu.h
+++ b/hypervisor/arch/x86/include/asm/vcpu.h
@@ -99,16 +99,16 @@ void vcpu_vendor_get_cell_io_bitmap(struct cell *cell,
struct vcpu_io_bitmap *out);

void vcpu_vendor_get_execution_state(struct vcpu_execution_state *x_state);
-
-void vcpu_handle_hypercall(struct registers *guest_regs);
-
-bool vcpu_handle_io_access(struct registers *guest_regs,
- struct vcpu_io_intercept *io);
+void vcpu_vendor_get_io_intercept(struct vcpu_io_intercept *io);

bool vcpu_get_guest_paging_structs(struct guest_paging_structures *pg_structs);

void vcpu_vendor_set_guest_pat(unsigned long val);

+void vcpu_handle_hypercall(struct registers *guest_regs);
+
+bool vcpu_handle_io_access(struct registers *guest_regs);
+
bool vcpu_handle_pt_violation(struct registers *guest_regs,
struct vcpu_pf_intercept *pf);

diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c
index 25376cb..05a1189 100644
--- a/hypervisor/arch/x86/svm.c
+++ b/hypervisor/arch/x86/svm.c
@@ -903,10 +903,9 @@ static void svm_get_vcpu_pf_intercept(struct per_cpu *cpu_data,
out->is_write = !!(vmcb->exitinfo1 & 0x2);
}

-static void svm_get_vcpu_io_intercept(struct per_cpu *cpu_data,
- struct vcpu_io_intercept *out)
+void vcpu_vendor_get_io_intercept(struct vcpu_io_intercept *out)
{
- struct vmcb *vmcb = &cpu_data->vmcb;
+ struct vmcb *vmcb = &this_cpu_data()->vmcb;
u64 exitinfo = vmcb->exitinfo1;

/* parse exit info for I/O instructions (see APM, 15.10.2 ) */
@@ -921,7 +920,6 @@ void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data)

{
struct vmcb *vmcb = &cpu_data->vmcb;

struct vcpu_pf_intercept pf;
- struct vcpu_io_intercept io;
bool res = false;
int sipi_vector;

@@ -1011,8 +1009,7 @@ void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data)
break;
case VMEXIT_IOIO:
cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_PIO]++;
- svm_get_vcpu_io_intercept(cpu_data, &io);
- if (vcpu_handle_io_access(guest_regs, &io))
+ if (vcpu_handle_io_access(guest_regs))
return;
break;
/* TODO: Handle VMEXIT_AVIC_NOACCEL and VMEXIT_AVIC_INCOMPLETE_IPI */
diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
index 5911fe1..82f5bbb 100644
--- a/hypervisor/arch/x86/vcpu.c
+++ b/hypervisor/arch/x86/vcpu.c
@@ -164,29 +164,31 @@ void vcpu_handle_hypercall(struct registers *guest_regs)
vcpu_deactivate_vmm(guest_regs);
}

-bool vcpu_handle_io_access(struct registers *guest_regs,
- struct vcpu_io_intercept *io)
+bool vcpu_handle_io_access(struct registers *guest_regs)
{
+ struct vcpu_io_intercept io;
int result = 0;

+ vcpu_vendor_get_io_intercept(&io);
+
/* string and REP-prefixed instructions are not supported */
- if (io->rep_or_str)
+ if (io.rep_or_str)
goto invalid_access;

- result = x86_pci_config_handler(guest_regs, this_cell(), io->port,
- io->in, io->size);
+ result = x86_pci_config_handler(guest_regs, this_cell(), io.port,
+ io.in, io.size);
if (result == 0)
- result = i8042_access_handler(guest_regs, io->port,
- io->in, io->size);
+ result = i8042_access_handler(guest_regs, io.port, io.in,
+ io.size);

if (result == 1) {
- vcpu_skip_emulated_instruction(io->inst_len);
+ vcpu_skip_emulated_instruction(io.inst_len);
return true;
}

invalid_access:
panic_printk("FATAL: Invalid PIO %s, port: %x size: %d\n",
- io->in ? "read" : "write", io->port, io->size);
+ io.in ? "read" : "write", io.port, io.size);
return false;
}

diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index 90b2144..a5b2980 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -1016,7 +1016,7 @@ static void dump_guest_regs(struct registers *guest_regs)
panic_printk("EFER: %p\n", vmcs_read64(GUEST_IA32_EFER));
}

-static void vmx_get_vcpu_io_intercept(struct vcpu_io_intercept *out)
+void vcpu_vendor_get_io_intercept(struct vcpu_io_intercept *out)
{
u64 exitq = vmcs_read64(EXIT_QUALIFICATION);

@@ -1041,7 +1041,6 @@ static void vmx_get_vcpu_pf_intercept(struct vcpu_pf_intercept *out)
void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data)
{
u32 reason = vmcs_read32(VM_EXIT_REASON);

- struct vcpu_io_intercept io;

struct vcpu_pf_intercept pf;
int sipi_vector;

@@ -1118,8 +1117,7 @@ void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data)
break;
case EXIT_REASON_IO_INSTRUCTION:
cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_PIO]++;
- vmx_get_vcpu_io_intercept(&io);
- if (vcpu_handle_io_access(guest_regs, &io))
+ if (vcpu_handle_io_access(guest_regs))
return;
break;
case EXIT_REASON_EPT_VIOLATION:
--
2.1.4

[Jan Kiszka]

This aligns the internal names with those used externally. No functional
changes.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/svm.c | 12 ++++++------
hypervisor/arch/x86/vmx.c | 12 ++++++------
2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c
index 05a1189..f0ac6ab 100644
--- a/hypervisor/arch/x86/svm.c
+++ b/hypervisor/arch/x86/svm.c
@@ -903,17 +903,17 @@ static void svm_get_vcpu_pf_intercept(struct per_cpu *cpu_data,

out->is_write = !!(vmcb->exitinfo1 & 0x2);
}

-void vcpu_vendor_get_io_intercept(struct vcpu_io_intercept *out)
+void vcpu_vendor_get_io_intercept(struct vcpu_io_intercept *io)
{

struct vmcb *vmcb = &this_cpu_data()->vmcb;
u64 exitinfo = vmcb->exitinfo1;

/* parse exit info for I/O instructions (see APM, 15.10.2 ) */

- out->port = (exitinfo >> 16) & 0xFFFF;
- out->size = (exitinfo >> 4) & 0x7;
- out->in = !!(exitinfo & 0x1);
- out->inst_len = vmcb->exitinfo2 - vmcb->rip;
- out->rep_or_str = !!(exitinfo & 0x0c);
+ io->port = (exitinfo >> 16) & 0xFFFF;
+ io->size = (exitinfo >> 4) & 0x7;
+ io->in = !!(exitinfo & 0x1);
+ io->inst_len = vmcb->exitinfo2 - vmcb->rip;
+ io->rep_or_str = !!(exitinfo & 0x0c);

}

void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data)

diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index a5b2980..f0f1af4 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -1016,16 +1016,16 @@ static void dump_guest_regs(struct registers *guest_regs)

panic_printk("EFER: %p\n", vmcs_read64(GUEST_IA32_EFER));
}

-void vcpu_vendor_get_io_intercept(struct vcpu_io_intercept *out)
+void vcpu_vendor_get_io_intercept(struct vcpu_io_intercept *io)
{
u64 exitq = vmcs_read64(EXIT_QUALIFICATION);

/* parse exit qualification for I/O instructions (see SDM, 27.2.1 ) */
- out->port = (exitq >> 16) & 0xFFFF;
- out->size = (exitq & 0x3) + 1;
- out->in = !!((exitq & 0x8) >> 3);
- out->inst_len = vmcs_read64(VM_EXIT_INSTRUCTION_LEN);
- out->rep_or_str = !!(exitq & 0x30);
+ io->port = (exitq >> 16) & 0xFFFF;
+ io->size = (exitq & 0x3) + 1;
+ io->in = !!((exitq & 0x8) >> 3);
+ io->inst_len = vmcs_read64(VM_EXIT_INSTRUCTION_LEN);
+ io->rep_or_str = !!(exitq & 0x30);

}

static void vmx_get_vcpu_pf_intercept(struct vcpu_pf_intercept *out)

--
2.1.4

[Jan Kiszka]

This describes more accurately what the handler does and aligns us with
vcpu_handle_io_access.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/include/asm/vcpu.h | 5 ++---
hypervisor/arch/x86/svm.c | 2 +-
hypervisor/arch/x86/vcpu.c | 4 ++--
hypervisor/arch/x86/vmx.c | 2 +-
4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/hypervisor/arch/x86/include/asm/vcpu.h b/hypervisor/arch/x86/include/asm/vcpu.h
index 9edddc8..f126534 100644
--- a/hypervisor/arch/x86/include/asm/vcpu.h
+++ b/hypervisor/arch/x86/include/asm/vcpu.h
@@ -108,9 +108,8 @@ void vcpu_vendor_set_guest_pat(unsigned long val);
void vcpu_handle_hypercall(struct registers *guest_regs);

bool vcpu_handle_io_access(struct registers *guest_regs);
-
-bool vcpu_handle_pt_violation(struct registers *guest_regs,
- struct vcpu_pf_intercept *pf);
+bool vcpu_handle_mmio_access(struct registers *guest_regs,
+ struct vcpu_pf_intercept *pf);

bool vcpu_handle_msr_read(struct registers *guest_regs);
bool vcpu_handle_msr_write(struct registers *guest_regs);
diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c
index f0ac6ab..a22a741 100644
--- a/hypervisor/arch/x86/svm.c
+++ b/hypervisor/arch/x86/svm.c
@@ -983,7 +983,7 @@ void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data)
/* General MMIO (IOAPIC, PCI etc) */
cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_MMIO]++;
svm_get_vcpu_pf_intercept(cpu_data, &pf);
- if (vcpu_handle_pt_violation(guest_regs, &pf))
+ if (vcpu_handle_mmio_access(guest_regs, &pf))
return;
}

diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
index 82f5bbb..58eff48 100644
--- a/hypervisor/arch/x86/vcpu.c
+++ b/hypervisor/arch/x86/vcpu.c
@@ -192,8 +192,8 @@ invalid_access:
return false;
}

-bool vcpu_handle_pt_violation(struct registers *guest_regs,
- struct vcpu_pf_intercept *pf)
+bool vcpu_handle_mmio_access(struct registers *guest_regs,
+ struct vcpu_pf_intercept *pf)
{

struct per_cpu *cpu_data = this_cpu_data();

struct guest_paging_structures pg_structs;
diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index f0f1af4..7ce9937 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -1123,7 +1123,7 @@ void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data)
case EXIT_REASON_EPT_VIOLATION:
cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_MMIO]++;
vmx_get_vcpu_pf_intercept(&pf);
- if (vcpu_handle_pt_violation(guest_regs, &pf))
+ if (vcpu_handle_mmio_access(guest_regs, &pf))
return;
break;
default:
--
2.1.4

[Jan Kiszka]

Analogously to vcpu_handle_io_access, define the vendor callback
vcpu_vendor_get_mmio_intercept and call it from vcpu_handle_mmio_access
instead of passing it to the handler. For consistency reasons, rename
vcpu_pf_intercept to vcpu_mmio_intercept.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/include/asm/vcpu.h | 6 +++---
hypervisor/arch/x86/svm.c | 21 +++++++++------------
hypervisor/arch/x86/vcpu.c | 25 +++++++++++++------------
hypervisor/arch/x86/vmx.c | 10 ++++------
4 files changed, 29 insertions(+), 33 deletions(-)

diff --git a/hypervisor/arch/x86/include/asm/vcpu.h b/hypervisor/arch/x86/include/asm/vcpu.h
index f126534..b21da00 100644
--- a/hypervisor/arch/x86/include/asm/vcpu.h
+++ b/hypervisor/arch/x86/include/asm/vcpu.h
@@ -46,7 +46,7 @@ struct vcpu_io_intercept {
bool rep_or_str;
};

-struct vcpu_pf_intercept {
+struct vcpu_mmio_intercept {
u64 phys_addr;
bool is_write;
};
@@ -100,6 +100,7 @@ void vcpu_vendor_get_cell_io_bitmap(struct cell *cell,

void vcpu_vendor_get_execution_state(struct vcpu_execution_state *x_state);
void vcpu_vendor_get_io_intercept(struct vcpu_io_intercept *io);
+void vcpu_vendor_get_mmio_intercept(struct vcpu_mmio_intercept *mmio);

bool vcpu_get_guest_paging_structs(struct guest_paging_structures *pg_structs);

@@ -108,8 +109,7 @@ void vcpu_vendor_set_guest_pat(unsigned long val);

void vcpu_handle_hypercall(struct registers *guest_regs);

bool vcpu_handle_io_access(struct registers *guest_regs);

-bool vcpu_handle_mmio_access(struct registers *guest_regs,
- struct vcpu_pf_intercept *pf);
+bool vcpu_handle_mmio_access(struct registers *guest_regs);

bool vcpu_handle_msr_read(struct registers *guest_regs);
bool vcpu_handle_msr_write(struct registers *guest_regs);
diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c

index a22a741..4187461 100644
--- a/hypervisor/arch/x86/svm.c
+++ b/hypervisor/arch/x86/svm.c
@@ -894,15 +894,6 @@ static void dump_guest_regs(struct registers *guest_regs, struct vmcb *vmcb)
panic_printk("EFER: %p\n", vmcb->efer);
}

-static void svm_get_vcpu_pf_intercept(struct per_cpu *cpu_data,
- struct vcpu_pf_intercept *out)
-{

- struct vmcb *vmcb = &cpu_data->vmcb;

-
- out->phys_addr = vmcb->exitinfo2;
- out->is_write = !!(vmcb->exitinfo1 & 0x2);
-}
-

void vcpu_vendor_get_io_intercept(struct vcpu_io_intercept *io)
{
struct vmcb *vmcb = &this_cpu_data()->vmcb;

@@ -916,10 +907,17 @@ void vcpu_vendor_get_io_intercept(struct vcpu_io_intercept *io)

io->rep_or_str = !!(exitinfo & 0x0c);
}

+void vcpu_vendor_get_mmio_intercept(struct vcpu_mmio_intercept *mmio)
+{
+ struct vmcb *vmcb = &this_cpu_data()->vmcb;
+
+ mmio->phys_addr = vmcb->exitinfo2;
+ mmio->is_write = !!(vmcb->exitinfo1 & 0x2);
+}
+

void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data)

{

struct vmcb *vmcb = &cpu_data->vmcb;

- struct vcpu_pf_intercept pf;

bool res = false;
int sipi_vector;

@@ -982,8 +980,7 @@ void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data)
} else {

/* General MMIO (IOAPIC, PCI etc) */
cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_MMIO]++;

- svm_get_vcpu_pf_intercept(cpu_data, &pf);
- if (vcpu_handle_mmio_access(guest_regs, &pf))
+ if (vcpu_handle_mmio_access(guest_regs))

return;
}

diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c

index 58eff48..d809f40 100644
--- a/hypervisor/arch/x86/vcpu.c
+++ b/hypervisor/arch/x86/vcpu.c
@@ -192,40 +192,41 @@ invalid_access:
return false;
}

-bool vcpu_handle_mmio_access(struct registers *guest_regs,
- struct vcpu_pf_intercept *pf)
+bool vcpu_handle_mmio_access(struct registers *guest_regs)

{
struct per_cpu *cpu_data = this_cpu_data();
struct guest_paging_structures pg_structs;

struct vcpu_execution_state x_state;
+ struct vcpu_mmio_intercept mmio;
struct mmio_instruction inst;
int result = 0;
u32 val;

vcpu_vendor_get_execution_state(&x_state);
+ vcpu_vendor_get_mmio_intercept(&mmio);

if (!vcpu_get_guest_paging_structs(&pg_structs))
goto invalid_access;

- inst = x86_mmio_parse(x_state.rip, &pg_structs, pf->is_write);
+ inst = x86_mmio_parse(x_state.rip, &pg_structs, mmio.is_write);
if (!inst.inst_len || inst.access_size != 4)
goto invalid_access;

- if (pf->is_write)
+ if (mmio.is_write)
val = ((unsigned long *)guest_regs)[inst.reg_num];

- result = ioapic_access_handler(cpu_data->cell, pf->is_write,
- pf->phys_addr, &val);
+ result = ioapic_access_handler(cpu_data->cell, mmio.is_write,
+ mmio.phys_addr, &val);
if (result == 0)
- result = pci_mmio_access_handler(cpu_data->cell, pf->is_write,
- pf->phys_addr, &val);
+ result = pci_mmio_access_handler(cpu_data->cell, mmio.is_write,
+ mmio.phys_addr, &val);

if (result == 0)
- result = iommu_mmio_access_handler(pf->is_write,
- pf->phys_addr, &val);
+ result = iommu_mmio_access_handler(mmio.is_write,
+ mmio.phys_addr, &val);

if (result == 1) {
- if (!pf->is_write)
+ if (!mmio.is_write)
((unsigned long *)guest_regs)[inst.reg_num] = val;
vcpu_skip_emulated_instruction(inst.inst_len);
return true;
@@ -235,7 +236,7 @@ invalid_access:
/* report only unhandled access failures */
if (result == 0)
panic_printk("FATAL: Invalid MMIO/RAM %s, addr: %p\n",
- pf->is_write ? "write" : "read", pf->phys_addr);
+ mmio.is_write ? "write" : "read", mmio.phys_addr);
return false;
}

diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index 7ce9937..015e62b 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -1028,20 +1028,19 @@ void vcpu_vendor_get_io_intercept(struct vcpu_io_intercept *io)

io->rep_or_str = !!(exitq & 0x30);
}

-static void vmx_get_vcpu_pf_intercept(struct vcpu_pf_intercept *out)
+void vcpu_vendor_get_mmio_intercept(struct vcpu_mmio_intercept *mmio)
{
u64 exitq = vmcs_read64(EXIT_QUALIFICATION);

- out->phys_addr = vmcs_read64(GUEST_PHYSICAL_ADDRESS);
+ mmio->phys_addr = vmcs_read64(GUEST_PHYSICAL_ADDRESS);
/* We don't enable dirty/accessed bit updated in EPTP,
* so only read of write flags can be set, not both. */
- out->is_write = !!(exitq & 0x2);
+ mmio->is_write = !!(exitq & 0x2);

}

void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data)

{
u32 reason = vmcs_read32(VM_EXIT_REASON);
- struct vcpu_pf_intercept pf;
int sipi_vector;

cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_TOTAL]++;
@@ -1122,8 +1121,7 @@ void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data)
break;

case EXIT_REASON_EPT_VIOLATION:
cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_MMIO]++;

- vmx_get_vcpu_pf_intercept(&pf);
- if (vcpu_handle_mmio_access(guest_regs, &pf))
+ if (vcpu_handle_mmio_access(guest_regs))

[Jan Kiszka]

The handling logic is fully shared between AMD and Intel CPUs.
Consolidate it.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/include/asm/vcpu.h | 2 ++
hypervisor/arch/x86/svm.c | 14 +-------------
hypervisor/arch/x86/vcpu.c | 20 ++++++++++++++++++++
hypervisor/arch/x86/vmx.c | 15 +--------------
4 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/hypervisor/arch/x86/include/asm/vcpu.h b/hypervisor/arch/x86/include/asm/vcpu.h
index b21da00..6a5f5d3 100644
--- a/hypervisor/arch/x86/include/asm/vcpu.h
+++ b/hypervisor/arch/x86/include/asm/vcpu.h
@@ -114,6 +114,8 @@ bool vcpu_handle_mmio_access(struct registers *guest_regs);

bool vcpu_handle_msr_read(struct registers *guest_regs);
bool vcpu_handle_msr_write(struct registers *guest_regs);

+bool vcpu_handle_xsetbv(struct registers *guest_regs);
+
void vcpu_reset(struct registers *guest_regs);

#endif
diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c
index 4187461..8e5152d 100644
--- a/hypervisor/arch/x86/svm.c
+++ b/hypervisor/arch/x86/svm.c
@@ -989,20 +989,8 @@ void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data)
vmcb->exitinfo1 & 0xf);
break;
case VMEXIT_XSETBV:
- cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_XSETBV]++;
- if ((guest_regs->rax & X86_XCR0_FP) &&
- (guest_regs->rax & ~cpuid_eax(0x0d)) == 0 &&
- guest_regs->rcx == 0 && guest_regs->rdx == 0) {
- vcpu_skip_emulated_instruction(X86_INST_LEN_XSETBV);
- asm volatile(
- "xsetbv"
- : /* no output */
- : "a" (guest_regs->rax), "c" (0), "d" (0));
+ if (vcpu_handle_xsetbv(guest_regs))
return;
- }
- panic_printk("FATAL: Invalid xsetbv parameters: "
- "xcr[%d] = %x:%x\n", guest_regs->rcx,
- guest_regs->rdx, guest_regs->rax);

break;
case VMEXIT_IOIO:
cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_PIO]++;

diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
index d809f40..c9893e8 100644
--- a/hypervisor/arch/x86/vcpu.c
+++ b/hypervisor/arch/x86/vcpu.c
@@ -311,6 +311,26 @@ bool vcpu_handle_msr_write(struct registers *guest_regs)
return true;
}

+bool vcpu_handle_xsetbv(struct registers *guest_regs)
+{
+ this_cpu_data()->stats[JAILHOUSE_CPU_STAT_VMEXITS_XSETBV]++;
+
+ if (cpuid_ecx(1) & X86_FEATURE_XSAVE &&
+ guest_regs->rax & X86_XCR0_FP &&
+ (guest_regs->rax & ~cpuid_eax(0x0d)) == 0 &&
+ guest_regs->rcx == 0 && guest_regs->rdx == 0) {
+ vcpu_skip_emulated_instruction(X86_INST_LEN_XSETBV);
+ asm volatile(
+ "xsetbv"
+ : /* no output */
+ : "a" (guest_regs->rax), "c" (0), "d" (0));
+ return true;
+ }
+ panic_printk("FATAL: Invalid xsetbv parameters: xcr[%d] = %08x:%08x\n",
+ guest_regs->rcx, guest_regs->rdx, guest_regs->rax);
+ return false;
+}
+
void vcpu_reset(struct registers *guest_regs)

{
struct per_cpu *cpu_data = this_cpu_data();

diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index 015e62b..cc8582b 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -1098,21 +1098,8 @@ void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data)
return;
break;
case EXIT_REASON_XSETBV:
- cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_XSETBV]++;
- if (cpuid_ecx(1) & X86_FEATURE_XSAVE &&
- guest_regs->rax & X86_XCR0_FP &&
- (guest_regs->rax & ~cpuid_eax(0x0d)) == 0 &&
- guest_regs->rcx == 0 && guest_regs->rdx == 0) {
- vcpu_skip_emulated_instruction(X86_INST_LEN_XSETBV);
- asm volatile(
- "xsetbv"
- : /* no output */
- : "a" (guest_regs->rax), "c" (0), "d" (0));
+ if (vcpu_handle_xsetbv(guest_regs))
return;
- }
- panic_printk("FATAL: Invalid xsetbv parameters: "
- "xcr[%d] = %08x:%08x\n", guest_regs->rcx,
- guest_regs->rdx, guest_regs->rax);

break;
case EXIT_REASON_IO_INSTRUCTION:
cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_PIO]++;

--
2.1.4

[Jan Kiszka]

Convert struct registers into a union and provide a by_index array for
index-based access. This is used by various handlers that parse guest
instructions and so far use a blunt type case on the structure.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/apic.c | 11 ++++----
hypervisor/arch/x86/i8042.c | 2 +-
hypervisor/arch/x86/include/asm/apic.h | 6 ++---
hypervisor/arch/x86/include/asm/i8042.h | 2 +-
hypervisor/arch/x86/include/asm/pci.h | 2 +-
hypervisor/arch/x86/include/asm/processor.h | 41 ++++++++++++++++-------------
hypervisor/arch/x86/include/asm/vcpu.h | 18 ++++++-------
hypervisor/arch/x86/pci.c | 10 +++----
hypervisor/arch/x86/svm.c | 14 +++++-----
hypervisor/arch/x86/vcpu.c | 18 ++++++-------
hypervisor/arch/x86/vmx.c | 12 ++++-----
11 files changed, 69 insertions(+), 67 deletions(-)

diff --git a/hypervisor/arch/x86/apic.c b/hypervisor/arch/x86/apic.c
index ac06fb3..a755f95 100644
--- a/hypervisor/arch/x86/apic.c
+++ b/hypervisor/arch/x86/apic.c
@@ -468,7 +468,7 @@ static bool apic_invalid_lvt_delivery_mode(unsigned int reg, u32 val)
return true;
}

-unsigned int apic_mmio_access(struct registers *guest_regs,
+unsigned int apic_mmio_access(union registers *guest_regs,

struct per_cpu *cpu_data, unsigned long rip,
const struct guest_paging_structures *pg_structs,

unsigned int reg, bool is_write)
@@ -485,7 +485,7 @@ unsigned int apic_mmio_access(struct registers *guest_regs,
return 0;
}
if (is_write) {
- val = ((unsigned long *)guest_regs)[inst.reg_num];
+ val = guest_regs->by_index[inst.reg_num];
if (apic_accessing_reserved_bits(reg, val))
return 0;

@@ -512,13 +512,12 @@ unsigned int apic_mmio_access(struct registers *guest_regs,
apic_ops.write(reg, val);
} else {
val = apic_ops.read(reg);
- ((unsigned long *)guest_regs)[inst.reg_num] = val;
+ guest_regs->by_index[inst.reg_num] = val;
}
return inst.inst_len;
}

-bool x2apic_handle_write(struct registers *guest_regs,
- struct per_cpu *cpu_data)
+bool x2apic_handle_write(union registers *guest_regs, struct per_cpu *cpu_data)
{
u32 reg = guest_regs->rcx - MSR_X2APIC_BASE;
u32 val = guest_regs->rax;
@@ -540,7 +539,7 @@ bool x2apic_handle_write(struct registers *guest_regs,
}

/* must only be called for readable registers */
-void x2apic_handle_read(struct registers *guest_regs)
+void x2apic_handle_read(union registers *guest_regs)
{
u32 reg = guest_regs->rcx - MSR_X2APIC_BASE;

diff --git a/hypervisor/arch/x86/i8042.c b/hypervisor/arch/x86/i8042.c
index cabed53..d6b3356 100644
--- a/hypervisor/arch/x86/i8042.c
+++ b/hypervisor/arch/x86/i8042.c
@@ -18,7 +18,7 @@

#include <jailhouse/cell-config.h>

-int i8042_access_handler(struct registers *guest_regs, u16 port, bool dir_in,
+int i8042_access_handler(union registers *guest_regs, u16 port, bool dir_in,
unsigned int size)
{
const struct jailhouse_cell_desc *config = this_cell()->config;
diff --git a/hypervisor/arch/x86/include/asm/apic.h b/hypervisor/arch/x86/include/asm/apic.h
index e8eea68..28779ca 100644
--- a/hypervisor/arch/x86/include/asm/apic.h
+++ b/hypervisor/arch/x86/include/asm/apic.h
@@ -161,14 +161,14 @@ void apic_send_irq(struct apic_irq_message irq_msg);

void apic_irq_handler(void);

-unsigned int apic_mmio_access(struct registers *guest_regs,
+unsigned int apic_mmio_access(union registers *guest_regs,

struct per_cpu *cpu_data, unsigned long rip,
const struct guest_paging_structures *pg_structs,

unsigned int reg, bool is_write);

-bool x2apic_handle_write(struct registers *guest_regs,
+bool x2apic_handle_write(union registers *guest_regs,
struct per_cpu *cpu_data);
-void x2apic_handle_read(struct registers *guest_regs);
+void x2apic_handle_read(union registers *guest_regs);

u32 x2apic_filter_logical_dest(struct cell *cell, u32 destination);

diff --git a/hypervisor/arch/x86/include/asm/i8042.h b/hypervisor/arch/x86/include/asm/i8042.h
index 9fb5e27..9d25def 100644
--- a/hypervisor/arch/x86/include/asm/i8042.h
+++ b/hypervisor/arch/x86/include/asm/i8042.h
@@ -19,7 +19,7 @@
# define I8042_CMD_WRITE_CTRL_PORT 0xd1
# define I8042_CMD_PULSE_CTRL_PORT 0xf0

-int i8042_access_handler(struct registers *guest_regs, u16 port, bool dir_in,
+int i8042_access_handler(union registers *guest_regs, u16 port, bool dir_in,
unsigned int size);

#endif /* !_JAILHOUSE_ASM_I8042_H */
diff --git a/hypervisor/arch/x86/include/asm/pci.h b/hypervisor/arch/x86/include/asm/pci.h
index b9aeca1..37ee686 100644
--- a/hypervisor/arch/x86/include/asm/pci.h
+++ b/hypervisor/arch/x86/include/asm/pci.h
@@ -32,7 +32,7 @@
* @{
*/

-int x86_pci_config_handler(struct registers *guest_regs, struct cell *cell,
+int x86_pci_config_handler(union registers *guest_regs, struct cell *cell,
u16 port, bool dir_in, unsigned int size);

/** @} */
diff --git a/hypervisor/arch/x86/include/asm/processor.h b/hypervisor/arch/x86/include/asm/processor.h
index 22ca32b..1dd6b7d 100644
--- a/hypervisor/arch/x86/include/asm/processor.h
+++ b/hypervisor/arch/x86/include/asm/processor.h
@@ -138,23 +138,26 @@
* @{
*/

-struct registers {
- unsigned long r15;
- unsigned long r14;
- unsigned long r13;
- unsigned long r12;
- unsigned long r11;
- unsigned long r10;
- unsigned long r9;
- unsigned long r8;
- unsigned long rdi;
- unsigned long rsi;
- unsigned long rbp;
- unsigned long unused;
- unsigned long rbx;
- unsigned long rdx;
- unsigned long rcx;
- unsigned long rax;
+union registers {
+ struct {
+ unsigned long r15;
+ unsigned long r14;
+ unsigned long r13;
+ unsigned long r12;
+ unsigned long r11;
+ unsigned long r10;
+ unsigned long r9;
+ unsigned long r8;
+ unsigned long rdi;
+ unsigned long rsi;
+ unsigned long rbp;
+ unsigned long unused;
+ unsigned long rbx;
+ unsigned long rdx;
+ unsigned long rcx;
+ unsigned long rax;
+ };
+ unsigned long by_index[16];
};

struct desc_table_reg {
@@ -275,13 +278,13 @@ static inline void write_msr(unsigned int msr, unsigned long val)
: "memory");
}

-static inline void set_rdmsr_value(struct registers *regs, unsigned long val)
+static inline void set_rdmsr_value(union registers *regs, unsigned long val)
{
regs->rax = (u32)val;
regs->rdx = val >> 32;
}

-static inline unsigned long get_wrmsr_value(struct registers *regs)
+static inline unsigned long get_wrmsr_value(union registers *regs)
{
return (u32)regs->rax | (regs->rdx << 32);
}
diff --git a/hypervisor/arch/x86/include/asm/vcpu.h b/hypervisor/arch/x86/include/asm/vcpu.h
index 6a5f5d3..8dd048f 100644
--- a/hypervisor/arch/x86/include/asm/vcpu.h
+++ b/hypervisor/arch/x86/include/asm/vcpu.h
@@ -68,9 +68,9 @@ void vcpu_exit(struct per_cpu *cpu_data);

void __attribute__((noreturn)) vcpu_activate_vmm(struct per_cpu *cpu_data);
void __attribute__((noreturn))
-vcpu_deactivate_vmm(struct registers *guest_regs);
+vcpu_deactivate_vmm(union registers *guest_regs);

-void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data);
+void vcpu_handle_exit(union registers *guest_regs, struct per_cpu *cpu_data);

void vcpu_park(void);

@@ -106,16 +106,16 @@ bool vcpu_get_guest_paging_structs(struct guest_paging_structures *pg_structs);

void vcpu_vendor_set_guest_pat(unsigned long val);

-void vcpu_handle_hypercall(struct registers *guest_regs);
+void vcpu_handle_hypercall(union registers *guest_regs);

-bool vcpu_handle_io_access(struct registers *guest_regs);
-bool vcpu_handle_mmio_access(struct registers *guest_regs);
+bool vcpu_handle_io_access(union registers *guest_regs);
+bool vcpu_handle_mmio_access(union registers *guest_regs);

-bool vcpu_handle_msr_read(struct registers *guest_regs);
-bool vcpu_handle_msr_write(struct registers *guest_regs);
+bool vcpu_handle_msr_read(union registers *guest_regs);
+bool vcpu_handle_msr_write(union registers *guest_regs);

-bool vcpu_handle_xsetbv(struct registers *guest_regs);
+bool vcpu_handle_xsetbv(union registers *guest_regs);

-void vcpu_reset(struct registers *guest_regs);
+void vcpu_reset(union registers *guest_regs);

#endif
diff --git a/hypervisor/arch/x86/pci.c b/hypervisor/arch/x86/pci.c
index 7bbf96c..af48c49 100644
--- a/hypervisor/arch/x86/pci.c
+++ b/hypervisor/arch/x86/pci.c
@@ -71,7 +71,7 @@ void arch_pci_write_config(u16 bdf, u16 address, u32 value, unsigned int size)
*
* @private
*/
-static void set_rax_reg(struct registers *guest_regs,
+static void set_rax_reg(union registers *guest_regs,
u32 value_new, u8 size)
{
u64 value_old = guest_regs->rax;
@@ -91,7 +91,7 @@ static void set_rax_reg(struct registers *guest_regs,
*
* @private
*/
-static u32 get_rax_reg(struct registers *guest_regs, u8 size)
+static u32 get_rax_reg(union registers *guest_regs, u8 size)
{
return guest_regs->rax & BYTE_MASK(size);
}
@@ -108,7 +108,7 @@ static u32 get_rax_reg(struct registers *guest_regs, u8 size)
* @private
*/
static int
-data_port_in_handler(struct registers *guest_regs, struct pci_device *device,
+data_port_in_handler(union registers *guest_regs, struct pci_device *device,
u16 address, unsigned int size)
{
u32 reg_data;
@@ -135,7 +135,7 @@ data_port_in_handler(struct registers *guest_regs, struct pci_device *device,
* @private
*/
static int
-data_port_out_handler(struct registers *guest_regs, struct pci_device *device,
+data_port_out_handler(union registers *guest_regs, struct pci_device *device,
u16 address, unsigned int size)
{
u32 reg_data = get_rax_reg(guest_regs, size);
@@ -160,7 +160,7 @@ data_port_out_handler(struct registers *guest_regs, struct pci_device *device,
*
* @return 1 if handled successfully, 0 if unhandled, -1 on access error.
*/
-int x86_pci_config_handler(struct registers *guest_regs, struct cell *cell,
+int x86_pci_config_handler(union registers *guest_regs, struct cell *cell,
u16 port, bool dir_in, unsigned int size)
{
struct pci_device *device;
diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c
index 8e5152d..ef7773a 100644
--- a/hypervisor/arch/x86/svm.c
+++ b/hypervisor/arch/x86/svm.c
@@ -469,7 +469,7 @@ void __attribute__((noreturn)) vcpu_activate_vmm(struct per_cpu *cpu_data)
}

void __attribute__((noreturn))
-vcpu_deactivate_vmm(struct registers *guest_regs)
+vcpu_deactivate_vmm(union registers *guest_regs)

{
struct per_cpu *cpu_data = this_cpu_data();

struct vmcb *vmcb = &cpu_data->vmcb;

@@ -778,7 +778,7 @@ out:
* result in no more than VMEXIT_INVALID. Maybe we can get along without it
* altogether?
*/
-static bool svm_handle_cr(struct registers *guest_regs,
+static bool svm_handle_cr(union registers *guest_regs,
struct per_cpu *cpu_data)
{

struct vmcb *vmcb = &cpu_data->vmcb;

@@ -804,7 +804,7 @@ static bool svm_handle_cr(struct registers *guest_regs,
if (reg == 4)
val = vmcb->rsp;
else
- val = ((unsigned long *)guest_regs)[15 - reg];
+ val = guest_regs->by_index[15 - reg];

vcpu_skip_emulated_instruction(X86_INST_LEN_MOV_TO_CR);
/* Flush TLB on PG/WP/CD/NW change: See APMv2, Sect. 15.16 */
@@ -821,7 +821,7 @@ out:
return ok;
}

-static bool svm_handle_msr_write(struct registers *guest_regs,
+static bool svm_handle_msr_write(union registers *guest_regs,
struct per_cpu *cpu_data)
{

struct vmcb *vmcb = &cpu_data->vmcb;

@@ -846,7 +846,7 @@ static bool svm_handle_msr_write(struct registers *guest_regs,
* TODO: This handles unaccelerated (non-AVIC) access. AVIC should
* be treated separately in svm_handle_avic_access().
*/
-static bool svm_handle_apic_access(struct registers *guest_regs,
+static bool svm_handle_apic_access(union registers *guest_regs,
struct per_cpu *cpu_data)
{

struct vmcb *vmcb = &cpu_data->vmcb;

@@ -878,7 +878,7 @@ out_err:
return false;
}

-static void dump_guest_regs(struct registers *guest_regs, struct vmcb *vmcb)
+static void dump_guest_regs(union registers *guest_regs, struct vmcb *vmcb)
{
panic_printk("RIP: %p RSP: %p FLAGS: %x\n", vmcb->rip,
vmcb->rsp, vmcb->rflags);
@@ -915,7 +915,7 @@ void vcpu_vendor_get_mmio_intercept(struct vcpu_mmio_intercept *mmio)

mmio->is_write = !!(vmcb->exitinfo1 & 0x2);
}

-void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data)
+void vcpu_handle_exit(union registers *guest_regs, struct per_cpu *cpu_data)
{

struct vmcb *vmcb = &cpu_data->vmcb;

bool res = false;
diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
index c9893e8..49324b0 100644
--- a/hypervisor/arch/x86/vcpu.c
+++ b/hypervisor/arch/x86/vcpu.c
@@ -133,7 +133,7 @@ void vcpu_cell_exit(struct cell *cell)
vcpu_vendor_cell_exit(cell);
}

-void vcpu_handle_hypercall(struct registers *guest_regs)
+void vcpu_handle_hypercall(union registers *guest_regs)
{

unsigned long code = guest_regs->rax;

struct vcpu_execution_state x_state;
@@ -164,7 +164,7 @@ void vcpu_handle_hypercall(struct registers *guest_regs)
vcpu_deactivate_vmm(guest_regs);
}

-bool vcpu_handle_io_access(struct registers *guest_regs)
+bool vcpu_handle_io_access(union registers *guest_regs)
{

struct vcpu_io_intercept io;
int result = 0;

@@ -192,7 +192,7 @@ invalid_access:
return false;
}

-bool vcpu_handle_mmio_access(struct registers *guest_regs)
+bool vcpu_handle_mmio_access(union registers *guest_regs)

{
struct per_cpu *cpu_data = this_cpu_data();

struct guest_paging_structures pg_structs;
@@ -213,7 +213,7 @@ bool vcpu_handle_mmio_access(struct registers *guest_regs)
goto invalid_access;

if (mmio.is_write)
- val = ((unsigned long *)guest_regs)[inst.reg_num];
+ val = guest_regs->by_index[inst.reg_num];

result = ioapic_access_handler(cpu_data->cell, mmio.is_write,

mmio.phys_addr, &val);
@@ -227,7 +227,7 @@ bool vcpu_handle_mmio_access(struct registers *guest_regs)

if (result == 1) {
if (!mmio.is_write)
- ((unsigned long *)guest_regs)[inst.reg_num] = val;
+ guest_regs->by_index[inst.reg_num] = val;
vcpu_skip_emulated_instruction(inst.inst_len);
return true;
}
@@ -240,7 +240,7 @@ invalid_access:
return false;
}

-bool vcpu_handle_msr_read(struct registers *guest_regs)
+bool vcpu_handle_msr_read(union registers *guest_regs)

{
struct per_cpu *cpu_data = this_cpu_data();

@@ -264,7 +264,7 @@ bool vcpu_handle_msr_read(struct registers *guest_regs)
return true;
}

-bool vcpu_handle_msr_write(struct registers *guest_regs)
+bool vcpu_handle_msr_write(union registers *guest_regs)

{
struct per_cpu *cpu_data = this_cpu_data();

unsigned int bit_pos, pa;
@@ -311,7 +311,7 @@ bool vcpu_handle_msr_write(struct registers *guest_regs)
return true;
}

-bool vcpu_handle_xsetbv(struct registers *guest_regs)
+bool vcpu_handle_xsetbv(union registers *guest_regs)
{
this_cpu_data()->stats[JAILHOUSE_CPU_STAT_VMEXITS_XSETBV]++;

@@ -331,7 +331,7 @@ bool vcpu_handle_xsetbv(struct registers *guest_regs)
return false;
}

-void vcpu_reset(struct registers *guest_regs)
+void vcpu_reset(union registers *guest_regs)

{
struct per_cpu *cpu_data = this_cpu_data();

diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c

index cc8582b..b8e8c19 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -698,7 +698,7 @@ void __attribute__((noreturn)) vcpu_activate_vmm(struct per_cpu *cpu_data)
}

void __attribute__((noreturn))
-vcpu_deactivate_vmm(struct registers *guest_regs)
+vcpu_deactivate_vmm(union registers *guest_regs)
{
unsigned long *stack = (unsigned long *)vmcs_read64(GUEST_RSP);
unsigned long linux_ip = vmcs_read64(GUEST_RIP);
@@ -893,7 +893,7 @@ static void update_efer(void)
vmcs_read32(VM_ENTRY_CONTROLS) | VM_ENTRY_IA32E_MODE);
}

-static bool vmx_handle_cr(struct registers *guest_regs,
+static bool vmx_handle_cr(union registers *guest_regs,
struct per_cpu *cpu_data)
{
u64 exit_qualification = vmcs_read64(EXIT_QUALIFICATION);
@@ -907,7 +907,7 @@ static bool vmx_handle_cr(struct registers *guest_regs,
if (reg == 4)
val = vmcs_read64(GUEST_RSP);
else
- val = ((unsigned long *)guest_regs)[15 - reg];
+ val = guest_regs->by_index[15 - reg];

if (cr == 0 || cr == 4) {
vcpu_skip_emulated_instruction(X86_INST_LEN_MOV_TO_CR);
@@ -949,7 +949,7 @@ void vcpu_vendor_set_guest_pat(unsigned long val)
vmcs_write64(GUEST_IA32_PAT, val);
}

-static bool vmx_handle_apic_access(struct registers *guest_regs,
+static bool vmx_handle_apic_access(union registers *guest_regs,
struct per_cpu *cpu_data)
{
struct guest_paging_structures pg_structs;
@@ -998,7 +998,7 @@ static void dump_vm_exit_details(u32 reason)
vmcs_read64(GUEST_LINEAR_ADDRESS));
}

-static void dump_guest_regs(struct registers *guest_regs)
+static void dump_guest_regs(union registers *guest_regs)
{
panic_printk("RIP: %p RSP: %p FLAGS: %x\n", vmcs_read64(GUEST_RIP),
vmcs_read64(GUEST_RSP), vmcs_read64(GUEST_RFLAGS));
@@ -1038,7 +1038,7 @@ void vcpu_vendor_get_mmio_intercept(struct vcpu_mmio_intercept *mmio)

mmio->is_write = !!(exitq & 0x2);
}

-void vcpu_handle_exit(struct registers *guest_regs, struct per_cpu *cpu_data)
+void vcpu_handle_exit(union registers *guest_regs, struct per_cpu *cpu_data)
{
u32 reason = vmcs_read32(VM_EXIT_REASON);
int sipi_vector;
--
2.1.4

[Jan Kiszka]

Push the guest registers first so that they end up at the same location
on the stack as on Intel. This will allow to address them generically
via the per_cpu structure.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/svm-vmexit.S | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/hypervisor/arch/x86/svm-vmexit.S b/hypervisor/arch/x86/svm-vmexit.S
index fa16549..c20193e 100644
--- a/hypervisor/arch/x86/svm-vmexit.S
+++ b/hypervisor/arch/x86/svm-vmexit.S
@@ -18,9 +18,7 @@
.globl svm_vmexit
svm_vmexit:
/* XXX: GIF is always cleared here */
- push %rax
-
- push -PERCPU_STACK_END+1*8+PERCPU_VMCB_RAX(%rsp)
+ push -PERCPU_STACK_END+PERCPU_VMCB_RAX(%rsp)
push %rcx
push %rdx
push %rbx
@@ -38,8 +36,10 @@ svm_vmexit:
push %r15

mov %rsp,%rdi
- lea -PERCPU_STACK_END+1*8+16*8(%rsp),%rsi
+ lea -PERCPU_STACK_END+16*8(%rsp),%rsi
+ push %rax
call vcpu_handle_exit
+ pop %rax

pop %r15
pop %r14
@@ -56,9 +56,7 @@ svm_vmexit:
pop %rbx
pop %rdx
pop %rcx
- pop -PERCPU_STACK_END+1*8+PERCPU_VMCB_RAX(%rsp)
-
- pop %rax
+ pop -PERCPU_STACK_END+PERCPU_VMCB_RAX(%rsp)

vmload %rax
vmrun %rax
--
2.1.4

[Jan Kiszka]

Now that the guest registers are saved at the same location on the
per-cpu stack for both Intel and AMD, we can enable direct access via
the per-cpu data structure. This will allow to drop the guest registers
parameter from most functions.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/include/asm/percpu.h | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/hypervisor/arch/x86/include/asm/percpu.h b/hypervisor/arch/x86/include/asm/percpu.h
index 05e2900..cb4309c 100644
--- a/hypervisor/arch/x86/include/asm/percpu.h
+++ b/hypervisor/arch/x86/include/asm/percpu.h
@@ -23,6 +23,8 @@

#define NUM_ENTRY_REGS 6

+#define STACK_SIZE PAGE_SIZE
+
#ifndef __ASSEMBLY__

#include <asm/cell.h>
@@ -40,8 +42,16 @@

/** Per-CPU states. */
struct per_cpu {
- /** Stack used while in hypervisor mode. */
- u8 stack[PAGE_SIZE];
+ union {
+ /** Stack used while in hypervisor mode. */
+ u8 stack[STACK_SIZE];
+ struct {
+ u8 __fill[STACK_SIZE - sizeof(union registers)];
+ /** Guest registers saved on stack during VM exit. */
+ union registers guest_regs;
+ };
+ };
+
/** Linux stack pointer, used for handover to hypervisor. */
unsigned long linux_sp;

--
2.1.4

[Jan Kiszka]

We can retrieve them from the per-cpu data structure now.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/include/asm/vcpu.h | 2 +-
hypervisor/arch/x86/svm-vmexit.S | 3 +--
hypervisor/arch/x86/svm.c | 3 ++-
hypervisor/arch/x86/vmx-vmexit.S | 3 +--
hypervisor/arch/x86/vmx.c | 3 ++-
5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hypervisor/arch/x86/include/asm/vcpu.h b/hypervisor/arch/x86/include/asm/vcpu.h
index 8dd048f..cce0f5f 100644
--- a/hypervisor/arch/x86/include/asm/vcpu.h
+++ b/hypervisor/arch/x86/include/asm/vcpu.h
@@ -70,7 +70,7 @@ void __attribute__((noreturn)) vcpu_activate_vmm(struct per_cpu *cpu_data);
void __attribute__((noreturn))
vcpu_deactivate_vmm(union registers *guest_regs);

-void vcpu_handle_exit(union registers *guest_regs, struct per_cpu *cpu_data);
+void vcpu_handle_exit(struct per_cpu *cpu_data);

void vcpu_park(void);

diff --git a/hypervisor/arch/x86/svm-vmexit.S b/hypervisor/arch/x86/svm-vmexit.S
index c20193e..f374b37 100644
--- a/hypervisor/arch/x86/svm-vmexit.S
+++ b/hypervisor/arch/x86/svm-vmexit.S
@@ -35,8 +35,7 @@ svm_vmexit:
push %r14
push %r15

- mov %rsp,%rdi
- lea -PERCPU_STACK_END+16*8(%rsp),%rsi
+ lea -PERCPU_STACK_END+16*8(%rsp),%rdi
push %rax
call vcpu_handle_exit
pop %rax
diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c
index ef7773a..9c09c0e 100644
--- a/hypervisor/arch/x86/svm.c
+++ b/hypervisor/arch/x86/svm.c
@@ -915,8 +915,9 @@ void vcpu_vendor_get_mmio_intercept(struct vcpu_mmio_intercept *mmio)

mmio->is_write = !!(vmcb->exitinfo1 & 0x2);
}

-void vcpu_handle_exit(union registers *guest_regs, struct per_cpu *cpu_data)
+void vcpu_handle_exit(struct per_cpu *cpu_data)
{
+ union registers *guest_regs = &cpu_data->guest_regs;

struct vmcb *vmcb = &cpu_data->vmcb;
bool res = false;

int sipi_vector;
diff --git a/hypervisor/arch/x86/vmx-vmexit.S b/hypervisor/arch/x86/vmx-vmexit.S
index a82fd27..19402ef 100644
--- a/hypervisor/arch/x86/vmx-vmexit.S
+++ b/hypervisor/arch/x86/vmx-vmexit.S
@@ -32,8 +32,7 @@ vmx_vmexit:
push %r14
push %r15

- mov %rsp,%rdi
- lea -PERCPU_STACK_END+16*8(%rsp),%rsi
+ lea -PERCPU_STACK_END+16*8(%rsp),%rdi
call vcpu_handle_exit

pop %r15
diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index b8e8c19..1bbc64a 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -1038,8 +1038,9 @@ void vcpu_vendor_get_mmio_intercept(struct vcpu_mmio_intercept *mmio)

mmio->is_write = !!(exitq & 0x2);
}

-void vcpu_handle_exit(union registers *guest_regs, struct per_cpu *cpu_data)
+void vcpu_handle_exit(struct per_cpu *cpu_data)
{
+ union registers *guest_regs = &cpu_data->guest_regs;

[Jan Kiszka]

The function only works against the current CPU, thus should avoid to

take the misleading parameter. The necessary reference can be obtained

from the per-cpu data structure now.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
hypervisor/arch/x86/include/asm/vcpu.h | 2 +-

hypervisor/arch/x86/svm.c | 2 +-
hypervisor/arch/x86/vcpu.c | 4 ++--
hypervisor/arch/x86/vmx.c | 2 +-

4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hypervisor/arch/x86/include/asm/vcpu.h b/hypervisor/arch/x86/include/asm/vcpu.h
index cce0f5f..290fb27 100644
--- a/hypervisor/arch/x86/include/asm/vcpu.h
+++ b/hypervisor/arch/x86/include/asm/vcpu.h
@@ -116,6 +116,6 @@ bool vcpu_handle_msr_write(union registers *guest_regs);

bool vcpu_handle_xsetbv(union registers *guest_regs);

-void vcpu_reset(union registers *guest_regs);
+void vcpu_reset(void);

#endif
diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c
index 9c09c0e..03407b9 100644
--- a/hypervisor/arch/x86/svm.c
+++ b/hypervisor/arch/x86/svm.c
@@ -946,7 +946,7 @@ void vcpu_handle_exit(struct per_cpu *cpu_data)
printk("CPU %d received SIPI, vector %x\n",
cpu_data->cpu_id, sipi_vector);
svm_vcpu_reset(cpu_data, sipi_vector);
- vcpu_reset(guest_regs);
+ vcpu_reset();
}
iommu_check_pending_faults(cpu_data);
return;
diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
index 49324b0..2ddab36 100644
--- a/hypervisor/arch/x86/vcpu.c
+++ b/hypervisor/arch/x86/vcpu.c
@@ -331,11 +331,11 @@ bool vcpu_handle_xsetbv(union registers *guest_regs)
return false;
}

-void vcpu_reset(union registers *guest_regs)
+void vcpu_reset(void)

{
struct per_cpu *cpu_data = this_cpu_data();

- memset(guest_regs, 0, sizeof(*guest_regs));
+ memset(&cpu_data->guest_regs, 0, sizeof(cpu_data->guest_regs));
cpu_data->pat = PAT_RESET_VALUE;
cpu_data->mtrr_def_type = 0;
vcpu_vendor_set_guest_pat(0);
diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index 1bbc64a..41618d1 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -1058,7 +1058,7 @@ void vcpu_handle_exit(struct per_cpu *cpu_data)
printk("CPU %d received SIPI, vector %x\n",
cpu_data->cpu_id, sipi_vector);
vmx_vcpu_reset(sipi_vector);
- vcpu_reset(guest_regs);
+ vcpu_reset();
}
iommu_check_pending_faults(cpu_data);
return;
--
2.1.4

[Jan Kiszka]

The function only works against the current CPU, thus should avoid to
take the misleading parameter. The necessary reference can be obtained
from the per-cpu data structure now.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/include/asm/vcpu.h | 3 +--
hypervisor/arch/x86/svm.c | 5 ++---
hypervisor/arch/x86/vcpu.c | 2 +-
hypervisor/arch/x86/vmx.c | 5 ++---
4 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/hypervisor/arch/x86/include/asm/vcpu.h b/hypervisor/arch/x86/include/asm/vcpu.h
index 290fb27..4cd0eaa 100644
--- a/hypervisor/arch/x86/include/asm/vcpu.h
+++ b/hypervisor/arch/x86/include/asm/vcpu.h
@@ -67,8 +67,7 @@ int vcpu_init(struct per_cpu *cpu_data);
void vcpu_exit(struct per_cpu *cpu_data);

void __attribute__((noreturn)) vcpu_activate_vmm(struct per_cpu *cpu_data);

-void __attribute__((noreturn))
-vcpu_deactivate_vmm(union registers *guest_regs);
+void __attribute__((noreturn)) vcpu_deactivate_vmm(void);

void vcpu_handle_exit(struct per_cpu *cpu_data);

diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c
index 03407b9..fa88c4b 100644
--- a/hypervisor/arch/x86/svm.c
+++ b/hypervisor/arch/x86/svm.c
@@ -468,8 +468,7 @@ void __attribute__((noreturn)) vcpu_activate_vmm(struct per_cpu *cpu_data)
__builtin_unreachable();
}

-void __attribute__((noreturn))
-vcpu_deactivate_vmm(union registers *guest_regs)
+void __attribute__((noreturn)) vcpu_deactivate_vmm(void)

{
struct per_cpu *cpu_data = this_cpu_data();

struct vmcb *vmcb = &cpu_data->vmcb;

@@ -539,7 +538,7 @@ vcpu_deactivate_vmm(union registers *guest_regs)
"mov %%rax,%%rsp\n\t"
"xor %%rax,%%rax\n\t"
"ret"
- : : "a" (stack), "b" (guest_regs));
+ : : "a" (stack), "b" (&cpu_data->guest_regs));
__builtin_unreachable();
}

diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
index 2ddab36..4c204e5 100644
--- a/hypervisor/arch/x86/vcpu.c
+++ b/hypervisor/arch/x86/vcpu.c
@@ -161,7 +161,7 @@ void vcpu_handle_hypercall(union registers *guest_regs)

x_state.rip - X86_INST_LEN_HYPERCALL);

if (code == JAILHOUSE_HC_DISABLE && guest_regs->rax == 0)

- vcpu_deactivate_vmm(guest_regs);
+ vcpu_deactivate_vmm();
}

bool vcpu_handle_io_access(union registers *guest_regs)
diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index 41618d1..5116e73 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -697,8 +697,7 @@ void __attribute__((noreturn)) vcpu_activate_vmm(struct per_cpu *cpu_data)
panic_stop();
}

-void __attribute__((noreturn))
-vcpu_deactivate_vmm(union registers *guest_regs)
+void __attribute__((noreturn)) vcpu_deactivate_vmm(void)

{
unsigned long *stack = (unsigned long *)vmcs_read64(GUEST_RSP);
unsigned long linux_ip = vmcs_read64(GUEST_RIP);

@@ -755,7 +754,7 @@ vcpu_deactivate_vmm(union registers *guest_regs)
"mov %%rax,%%rsp\n\t"
"xor %%rax,%%rax\n\t"
"ret"
- : : "a" (stack), "b" (guest_regs));
+ : : "a" (stack), "b" (&cpu_data->guest_regs));
__builtin_unreachable();
}

--
2.1.4

[Jan Kiszka]

The function only works against the current CPU, thus should avoid to
take the misleading parameter. The necessary reference can be obtained
from the per-cpu data structure now.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/include/asm/vcpu.h | 2 +-
hypervisor/arch/x86/svm.c | 2 +-

hypervisor/arch/x86/vcpu.c | 3 ++-
hypervisor/arch/x86/vmx.c | 2 +-
4 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/hypervisor/arch/x86/include/asm/vcpu.h b/hypervisor/arch/x86/include/asm/vcpu.h
index 4cd0eaa..a827f6e 100644
--- a/hypervisor/arch/x86/include/asm/vcpu.h
+++ b/hypervisor/arch/x86/include/asm/vcpu.h
@@ -105,7 +105,7 @@ bool vcpu_get_guest_paging_structs(struct guest_paging_structures *pg_structs);

void vcpu_vendor_set_guest_pat(unsigned long val);

-void vcpu_handle_hypercall(union registers *guest_regs);
+void vcpu_handle_hypercall(void);

bool vcpu_handle_io_access(union registers *guest_regs);
bool vcpu_handle_mmio_access(union registers *guest_regs);
diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c
index fa88c4b..7022639 100644
--- a/hypervisor/arch/x86/svm.c
+++ b/hypervisor/arch/x86/svm.c
@@ -953,7 +953,7 @@ void vcpu_handle_exit(struct per_cpu *cpu_data)

/* FIXME: We are not intercepting CPUID now */
return;
case VMEXIT_VMMCALL:

- vcpu_handle_hypercall(guest_regs);
+ vcpu_handle_hypercall();

return;
case VMEXIT_CR0_SEL_WRITE:
cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_CR]++;

diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
index 4c204e5..dd3e1cf 100644
--- a/hypervisor/arch/x86/vcpu.c
+++ b/hypervisor/arch/x86/vcpu.c
@@ -133,8 +133,9 @@ void vcpu_cell_exit(struct cell *cell)
vcpu_vendor_cell_exit(cell);
}

-void vcpu_handle_hypercall(union registers *guest_regs)
+void vcpu_handle_hypercall(void)
{
+ union registers *guest_regs = &this_cpu_data()->guest_regs;

unsigned long code = guest_regs->rax;
struct vcpu_execution_state x_state;

unsigned long arg_mask;
diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index 5116e73..eba68ac 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -1071,7 +1071,7 @@ void vcpu_handle_exit(struct per_cpu *cpu_data)

(u32 *)&guest_regs->rcx, (u32 *)&guest_regs->rdx);
return;
case EXIT_REASON_VMCALL:

- vcpu_handle_hypercall(guest_regs);
+ vcpu_handle_hypercall();

[Jan Kiszka]

The function only works against the current CPU, thus should avoid to
take the misleading parameter. The necessary reference can be obtained
from the per-cpu data structure now.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
hypervisor/arch/x86/include/asm/vcpu.h | 2 +-
hypervisor/arch/x86/svm.c | 2 +-

hypervisor/arch/x86/vcpu.c | 4 +++-
hypervisor/arch/x86/vmx.c | 2 +-
4 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/hypervisor/arch/x86/include/asm/vcpu.h b/hypervisor/arch/x86/include/asm/vcpu.h
index a827f6e..031b80e 100644
--- a/hypervisor/arch/x86/include/asm/vcpu.h
+++ b/hypervisor/arch/x86/include/asm/vcpu.h
@@ -113,7 +113,7 @@ bool vcpu_handle_mmio_access(union registers *guest_regs);
bool vcpu_handle_msr_read(union registers *guest_regs);
bool vcpu_handle_msr_write(union registers *guest_regs);

-bool vcpu_handle_xsetbv(union registers *guest_regs);
+bool vcpu_handle_xsetbv(void);

void vcpu_reset(void);

diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c
index 7022639..098be1e 100644
--- a/hypervisor/arch/x86/svm.c
+++ b/hypervisor/arch/x86/svm.c
@@ -989,7 +989,7 @@ void vcpu_handle_exit(struct per_cpu *cpu_data)

vmcb->exitinfo1 & 0xf);
break;
case VMEXIT_XSETBV:

- if (vcpu_handle_xsetbv(guest_regs))
+ if (vcpu_handle_xsetbv())
return;
break;
case VMEXIT_IOIO:
diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
index dd3e1cf..df40741 100644
--- a/hypervisor/arch/x86/vcpu.c
+++ b/hypervisor/arch/x86/vcpu.c
@@ -312,8 +312,10 @@ bool vcpu_handle_msr_write(union registers *guest_regs)
return true;
}

-bool vcpu_handle_xsetbv(union registers *guest_regs)
+bool vcpu_handle_xsetbv(void)

{
+ union registers *guest_regs = &this_cpu_data()->guest_regs;

+
this_cpu_data()->stats[JAILHOUSE_CPU_STAT_VMEXITS_XSETBV]++;

if (cpuid_ecx(1) & X86_FEATURE_XSAVE &&

diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index eba68ac..0eb58da 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -1098,7 +1098,7 @@ void vcpu_handle_exit(struct per_cpu *cpu_data)
return;
break;
case EXIT_REASON_XSETBV:
- if (vcpu_handle_xsetbv(guest_regs))
+ if (vcpu_handle_xsetbv())
return;
break;
case EXIT_REASON_IO_INSTRUCTION:
--
2.1.4

[Jan Kiszka]

The function only works against the current CPU, thus should avoid to

take the misleading parameters. We can retrieve the per-cpu data
structure and the guest registers in the function now.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/apic.c | 6 ++++--
hypervisor/arch/x86/include/asm/apic.h | 5 ++---
hypervisor/arch/x86/vcpu.c | 4 ++--
3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/hypervisor/arch/x86/apic.c b/hypervisor/arch/x86/apic.c
index a755f95..393ec9c 100644
--- a/hypervisor/arch/x86/apic.c
+++ b/hypervisor/arch/x86/apic.c
@@ -517,8 +517,9 @@ unsigned int apic_mmio_access(union registers *guest_regs,
return inst.inst_len;
}

-bool x2apic_handle_write(union registers *guest_regs, struct per_cpu *cpu_data)
+bool x2apic_handle_write(void)

{
+ union registers *guest_regs = &this_cpu_data()->guest_regs;

u32 reg = guest_regs->rcx - MSR_X2APIC_BASE;
u32 val = guest_regs->rax;

@@ -539,8 +540,9 @@ bool x2apic_handle_write(union registers *guest_regs, struct per_cpu *cpu_data)

}

/* must only be called for readable registers */

-void x2apic_handle_read(union registers *guest_regs)
+void x2apic_handle_read(void)

{
+ union registers *guest_regs = &this_cpu_data()->guest_regs;

u32 reg = guest_regs->rcx - MSR_X2APIC_BASE;

if (reg == APIC_REG_ID)
diff --git a/hypervisor/arch/x86/include/asm/apic.h b/hypervisor/arch/x86/include/asm/apic.h
index 28779ca..20a4d9c 100644
--- a/hypervisor/arch/x86/include/asm/apic.h
+++ b/hypervisor/arch/x86/include/asm/apic.h
@@ -166,9 +166,8 @@ unsigned int apic_mmio_access(union registers *guest_regs,

const struct guest_paging_structures *pg_structs,
unsigned int reg, bool is_write);

-bool x2apic_handle_write(union registers *guest_regs,
- struct per_cpu *cpu_data);
-void x2apic_handle_read(union registers *guest_regs);
+bool x2apic_handle_write(void);
+void x2apic_handle_read(void);

u32 x2apic_filter_logical_dest(struct cell *cell, u32 destination);

diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
index df40741..fb0d366 100644
--- a/hypervisor/arch/x86/vcpu.c
+++ b/hypervisor/arch/x86/vcpu.c
@@ -247,7 +247,7 @@ bool vcpu_handle_msr_read(union registers *guest_regs)

switch (guest_regs->rcx) {
case MSR_X2APIC_BASE ... MSR_X2APIC_END:
- x2apic_handle_read(guest_regs);
+ x2apic_handle_read();
break;
case MSR_IA32_PAT:
set_rdmsr_value(guest_regs, cpu_data->pat);
@@ -273,7 +273,7 @@ bool vcpu_handle_msr_write(union registers *guest_regs)

switch (guest_regs->rcx) {
case MSR_X2APIC_BASE ... MSR_X2APIC_END:
- if (!x2apic_handle_write(guest_regs, cpu_data))
+ if (!x2apic_handle_write())
return false;
break;
case MSR_IA32_PAT:
--
2.1.4

[Jan Kiszka]

The function only works on the current CPU, thus should avoid to take
misleading parameters. The necessary references can be obtained inline.

With the parameters no longer needed, the callers
svm/vmx_handle_apic_access can drop some of them as well.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/apic.c | 9 ++++-----
hypervisor/arch/x86/include/asm/apic.h | 3 +--
hypervisor/arch/x86/svm.c | 9 ++++-----
hypervisor/arch/x86/vmx.c | 8 +++-----
4 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/hypervisor/arch/x86/apic.c b/hypervisor/arch/x86/apic.c
index 393ec9c..f514943 100644
--- a/hypervisor/arch/x86/apic.c
+++ b/hypervisor/arch/x86/apic.c
@@ -468,8 +468,7 @@ static bool apic_invalid_lvt_delivery_mode(unsigned int reg, u32 val)
return true;
}

-unsigned int apic_mmio_access(union registers *guest_regs,
- struct per_cpu *cpu_data, unsigned long rip,
+unsigned int apic_mmio_access(unsigned long rip,

const struct guest_paging_structures *pg_structs,
unsigned int reg, bool is_write)

{
@@ -485,7 +484,7 @@ unsigned int apic_mmio_access(union registers *guest_regs,
return 0;
}
if (is_write) {
- val = guest_regs->by_index[inst.reg_num];
+ val = this_cpu_data()->guest_regs.by_index[inst.reg_num];

if (apic_accessing_reserved_bits(reg, val))
return 0;

@@ -494,7 +493,7 @@ unsigned int apic_mmio_access(union registers *guest_regs,

apic_ops.read(APIC_REG_ICR_HI)))
return 0;
} else if (reg == APIC_REG_LDR &&

- val != 1UL << (cpu_data->cpu_id + XAPIC_DEST_SHIFT)) {
+ val != 1UL << (this_cpu_id() + XAPIC_DEST_SHIFT)) {
panic_printk("FATAL: Unsupported change to LDR: %x\n",
val);
return 0;
@@ -512,7 +511,7 @@ unsigned int apic_mmio_access(union registers *guest_regs,

apic_ops.write(reg, val);
} else {
val = apic_ops.read(reg);

- guest_regs->by_index[inst.reg_num] = val;
+ this_cpu_data()->guest_regs.by_index[inst.reg_num] = val;
}
return inst.inst_len;
}
diff --git a/hypervisor/arch/x86/include/asm/apic.h b/hypervisor/arch/x86/include/asm/apic.h
index 20a4d9c..9c7ca12 100644
--- a/hypervisor/arch/x86/include/asm/apic.h
+++ b/hypervisor/arch/x86/include/asm/apic.h
@@ -161,8 +161,7 @@ void apic_send_irq(struct apic_irq_message irq_msg);

void apic_irq_handler(void);

-unsigned int apic_mmio_access(union registers *guest_regs,
- struct per_cpu *cpu_data, unsigned long rip,
+unsigned int apic_mmio_access(unsigned long rip,

const struct guest_paging_structures *pg_structs,
unsigned int reg, bool is_write);

diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c
index 098be1e..87db3b3 100644
--- a/hypervisor/arch/x86/svm.c
+++ b/hypervisor/arch/x86/svm.c
@@ -845,8 +845,7 @@ static bool svm_handle_msr_write(union registers *guest_regs,

* TODO: This handles unaccelerated (non-AVIC) access. AVIC should
* be treated separately in svm_handle_avic_access().
*/

-static bool svm_handle_apic_access(union registers *guest_regs,
- struct per_cpu *cpu_data)
+static bool svm_handle_apic_access(struct per_cpu *cpu_data)
{

struct vmcb *vmcb = &cpu_data->vmcb;

struct guest_paging_structures pg_structs;
@@ -863,8 +862,8 @@ static bool svm_handle_apic_access(union registers *guest_regs,
if (!vcpu_get_guest_paging_structs(&pg_structs))
goto out_err;

- inst_len = apic_mmio_access(guest_regs, cpu_data, vmcb->rip,
- &pg_structs, offset >> 4, is_write);
+ inst_len = apic_mmio_access(vmcb->rip, &pg_structs, offset >> 4,
+ is_write);
if (!inst_len)
goto out_err;

@@ -975,7 +974,7 @@ void vcpu_handle_exit(struct per_cpu *cpu_data)
vmcb->exitinfo2 < XAPIC_BASE + PAGE_SIZE) {
/* APIC access in non-AVIC mode */
cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_XAPIC]++;
- if (svm_handle_apic_access(guest_regs, cpu_data))
+ if (svm_handle_apic_access(cpu_data))
return;

} else {
/* General MMIO (IOAPIC, PCI etc) */

diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index 0eb58da..b3aa2ca 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -948,8 +948,7 @@ void vcpu_vendor_set_guest_pat(unsigned long val)
vmcs_write64(GUEST_IA32_PAT, val);
}

-static bool vmx_handle_apic_access(union registers *guest_regs,
- struct per_cpu *cpu_data)
+static bool vmx_handle_apic_access(void)
{
struct guest_paging_structures pg_structs;
unsigned int inst_len, offset;
@@ -969,8 +968,7 @@ static bool vmx_handle_apic_access(union registers *guest_regs,
if (!vcpu_get_guest_paging_structs(&pg_structs))
break;

- inst_len = apic_mmio_access(guest_regs, cpu_data,
- vmcs_read64(GUEST_RIP),
+ inst_len = apic_mmio_access(vmcs_read64(GUEST_RIP),
&pg_structs, offset >> 4,
is_write);
if (!inst_len)
@@ -1094,7 +1092,7 @@ void vcpu_handle_exit(struct per_cpu *cpu_data)
break;
case EXIT_REASON_APIC_ACCESS:
cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_XAPIC]++;
- if (vmx_handle_apic_access(guest_regs, cpu_data))
+ if (vmx_handle_apic_access())
return;
break;
case EXIT_REASON_XSETBV:
--
2.1.4

[Jan Kiszka]

The function only works against the current CPU, thus should avoid to
take the misleading parameter. The necessary reference can be obtained
from the per-cpu data structure now.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/i8042.c | 4 ++--
hypervisor/arch/x86/include/asm/i8042.h | 5 ++---
hypervisor/arch/x86/vcpu.c | 3 +--
3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/hypervisor/arch/x86/i8042.c b/hypervisor/arch/x86/i8042.c
index d6b3356..7f35606 100644
--- a/hypervisor/arch/x86/i8042.c
+++ b/hypervisor/arch/x86/i8042.c
@@ -18,9 +18,9 @@

#include <jailhouse/cell-config.h>

-int i8042_access_handler(union registers *guest_regs, u16 port, bool dir_in,
- unsigned int size)
+int i8042_access_handler(u16 port, bool dir_in, unsigned int size)

{
+ union registers *guest_regs = &this_cpu_data()->guest_regs;

const struct jailhouse_cell_desc *config = this_cell()->config;

const u8 *pio_bitmap = jailhouse_cell_pio_bitmap(config);
u8 val;
diff --git a/hypervisor/arch/x86/include/asm/i8042.h b/hypervisor/arch/x86/include/asm/i8042.h
index 9d25def..6bd7469 100644
--- a/hypervisor/arch/x86/include/asm/i8042.h
+++ b/hypervisor/arch/x86/include/asm/i8042.h
@@ -13,13 +13,12 @@
#ifndef _JAILHOUSE_ASM_I8042_H
#define _JAILHOUSE_ASM_I8042_H

-#include <asm/processor.h>
+#include <jailhouse/types.h>

#define I8042_CMD_REG 0x64

# define I8042_CMD_WRITE_CTRL_PORT 0xd1
# define I8042_CMD_PULSE_CTRL_PORT 0xf0

-int i8042_access_handler(union registers *guest_regs, u16 port, bool dir_in,
- unsigned int size);
+int i8042_access_handler(u16 port, bool dir_in, unsigned int size);

#endif /* !_JAILHOUSE_ASM_I8042_H */
diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
index fb0d366..6da5b14 100644
--- a/hypervisor/arch/x86/vcpu.c
+++ b/hypervisor/arch/x86/vcpu.c
@@ -179,8 +179,7 @@ bool vcpu_handle_io_access(union registers *guest_regs)

result = x86_pci_config_handler(guest_regs, this_cell(), io.port,

io.in, io.size);
if (result == 0)
- result = i8042_access_handler(guest_regs, io.port, io.in,
- io.size);
+ result = i8042_access_handler(io.port, io.in, io.size);

if (result == 1) {
vcpu_skip_emulated_instruction(io.inst_len);
--
2.1.4

[Jan Kiszka]

The function only works against the current CPU, thus should avoid to

take the misleading parameters. Guest registers are no long er required,
and the cell reference can be obtained inline.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/include/asm/pci.h | 4 +---
hypervisor/arch/x86/pci.c | 7 +++----
hypervisor/arch/x86/vcpu.c | 3 +--
3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/hypervisor/arch/x86/include/asm/pci.h b/hypervisor/arch/x86/include/asm/pci.h
index 37ee686..0458e4d 100644
--- a/hypervisor/arch/x86/include/asm/pci.h
+++ b/hypervisor/arch/x86/include/asm/pci.h
@@ -15,7 +15,6 @@
#define _JAILHOUSE_ASM_PCI_H

#include <jailhouse/types.h>
-#include <asm/percpu.h>

/* --- PCI configuration ports --- */
#define PCI_REG_ADDR_PORT 0xcf8
@@ -32,8 +31,7 @@
* @{
*/

-int x86_pci_config_handler(union registers *guest_regs, struct cell *cell,
- u16 port, bool dir_in, unsigned int size);
+int x86_pci_config_handler(u16 port, bool dir_in, unsigned int size);

/** @} */
#endif /* !_JAILHOUSE_ASM_PCI_H */
diff --git a/hypervisor/arch/x86/pci.c b/hypervisor/arch/x86/pci.c
index 65688b4..a6d0ba1 100644
--- a/hypervisor/arch/x86/pci.c
+++ b/hypervisor/arch/x86/pci.c
@@ -20,6 +20,7 @@
#include <asm/io.h>
#include <asm/iommu.h>
#include <asm/pci.h>
+#include <asm/processor.h>

/** Protects the root bridge's PIO interface to the PCI config space. */
static DEFINE_SPINLOCK(pci_lock);
@@ -145,17 +146,15 @@ static int data_port_out_handler(struct pci_device *device, u16 address,

/**
* Handler for accesses to PCI config space.
- * @param guest_regs Guest register set.
- * @param cell Issuing cell.
* @param port I/O port number of this access.
* @param dir_in True for input, false for output.
* @param size Size of access in bytes (1, 2 or 4 bytes).

*
* @return 1 if handled successfully, 0 if unhandled, -1 on access error.
*/

-int x86_pci_config_handler(union registers *guest_regs, struct cell *cell,
- u16 port, bool dir_in, unsigned int size)
+int x86_pci_config_handler(u16 port, bool dir_in, unsigned int size)
{
+ struct cell *cell = this_cell();
struct pci_device *device;
u32 addr_port_val;
u16 bdf, address;
diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
index 6da5b14..4325e8f 100644
--- a/hypervisor/arch/x86/vcpu.c
+++ b/hypervisor/arch/x86/vcpu.c
@@ -176,8 +176,7 @@ bool vcpu_handle_io_access(union registers *guest_regs)
if (io.rep_or_str)
goto invalid_access;

- result = x86_pci_config_handler(guest_regs, this_cell(), io.port,
- io.in, io.size);
+ result = x86_pci_config_handler(io.port, io.in, io.size);
if (result == 0)

result = i8042_access_handler(io.port, io.in, io.size);

--
2.1.4

[Jan Kiszka]

Stop requiring that the guest registers are passed down to the
accessors. Access handlers always work over the issuing CPU, thus can
obtain the register state themselves. Rename the accessors to make it
clear that they work against guest registers.

This allows to drop the guest_regs parameters from
data_port_in/out_handler.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/pci.c | 50 +++++++++++++++++++----------------------------
1 file changed, 20 insertions(+), 30 deletions(-)

diff --git a/hypervisor/arch/x86/pci.c b/hypervisor/arch/x86/pci.c
index af48c49..65688b4 100644
--- a/hypervisor/arch/x86/pci.c
+++ b/hypervisor/arch/x86/pci.c
@@ -64,41 +64,37 @@ void arch_pci_write_config(u16 bdf, u16 address, u32 value, unsigned int size)
}

/**
- * Set value of RAX in guest register set.

- * @param guest_regs Guest register set.

- * @param value_new New value to be written.
+ * Set value of RAX in current CPU's guest register set.
+ * @param value Value to be written.
* @param size Access size (1, 2 or 4 bytes).
*
* @private
*/
-static void set_rax_reg(union registers *guest_regs,
- u32 value_new, u8 size)
+static void set_guest_rax_reg(u32 value, u8 size)
{
- u64 value_old = guest_regs->rax;
- /* 32-bit access is special, since it clears all the upper
- * part of RAX. Another types of access leave it intact */

+ union registers *guest_regs = &this_cpu_data()->guest_regs;

+ /* 32-bit access is special since it clears all the upper part of RAX.
+ * Any other types of access leave it intact. */
u64 mask = (size == 4 ? BYTE_MASK(8) : BYTE_MASK(size));

- guest_regs->rax = (value_old & ~mask) | (value_new & mask);
+ guest_regs->rax = (guest_regs->rax & ~mask) | (value & mask);
}

/**
- * Get value of RAX from guest register set.
- * @param guest_regs Guest register set.
+ * Get value of RAX from current CPU's guest register set.
* @param size Access size (1, 2 or 4 bytes).
*
* @return Register value.
*
* @private
*/
-static u32 get_rax_reg(union registers *guest_regs, u8 size)
+static u32 get_guest_rax_reg(u8 size)
{
- return guest_regs->rax & BYTE_MASK(size);
+ return this_cpu_data()->guest_regs.rax & BYTE_MASK(size);
}

/**
* Handler for IN accesses to data port.

- * @param guest_regs Guest register set.

* @param device Structure describing PCI device.
* @param address Config space access address.
* @param size Access size (1, 2 or 4 bytes).
@@ -107,9 +103,8 @@ static u32 get_rax_reg(union registers *guest_regs, u8 size)
*
* @private
*/
-static int
-data_port_in_handler(union registers *guest_regs, struct pci_device *device,
- u16 address, unsigned int size)
+static int data_port_in_handler(struct pci_device *device, u16 address,
+ unsigned int size)
{
u32 reg_data;

@@ -118,14 +113,13 @@ data_port_in_handler(union registers *guest_regs, struct pci_device *device,
reg_data = arch_pci_read_config(device->info->bdf, address,
size);

- set_rax_reg(guest_regs, reg_data, size);
+ set_guest_rax_reg(reg_data, size);

return 1;
}

/**
* Handler for OUT accesses to data port.

- * @param guest_regs Guest register set.

* @param device Structure describing PCI device.
* @param address Config space access address.
* @param size Access size (1, 2 or 4 bytes).
@@ -134,11 +128,10 @@ data_port_in_handler(union registers *guest_regs, struct pci_device *device,
*
* @private
*/
-static int
-data_port_out_handler(union registers *guest_regs, struct pci_device *device,
- u16 address, unsigned int size)
+static int data_port_out_handler(struct pci_device *device, u16 address,
+ unsigned int size)
{
- u32 reg_data = get_rax_reg(guest_regs, size);
+ u32 reg_data = get_guest_rax_reg(size);
enum pci_access access;

access = pci_cfg_write_moderate(device, address, size, reg_data);
@@ -174,10 +167,9 @@ int x86_pci_config_handler(union registers *guest_regs, struct cell *cell,
goto invalid_access;

if (dir_in)
- set_rax_reg(guest_regs, cell->pci_addr_port_val, size);
+ set_guest_rax_reg(cell->pci_addr_port_val, size);
else
- cell->pci_addr_port_val =
- get_rax_reg(guest_regs, size);
+ cell->pci_addr_port_val = get_guest_rax_reg(size);
result = 1;
} else if (port >= PCI_REG_DATA_PORT &&
port < (PCI_REG_DATA_PORT + 4)) {
@@ -200,11 +192,9 @@ int x86_pci_config_handler(union registers *guest_regs, struct cell *cell,
port - PCI_REG_DATA_PORT;

if (dir_in)
- result = data_port_in_handler(guest_regs, device,
- address, size);
+ result = data_port_in_handler(device, address, size);
else
- result = data_port_out_handler(guest_regs, device,
- address, size);
+ result = data_port_out_handler(device, address, size);
if (result < 0)
goto invalid_access;
}
--
2.1.4

[Jan Kiszka]

All filter functions obtain the reference themselves now.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/include/asm/vcpu.h | 2 +-
hypervisor/arch/x86/svm.c | 2 +-

hypervisor/arch/x86/vcpu.c | 2 +-
hypervisor/arch/x86/vmx.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hypervisor/arch/x86/include/asm/vcpu.h b/hypervisor/arch/x86/include/asm/vcpu.h
index 031b80e..88b2e6e 100644
--- a/hypervisor/arch/x86/include/asm/vcpu.h
+++ b/hypervisor/arch/x86/include/asm/vcpu.h
@@ -107,7 +107,7 @@ void vcpu_vendor_set_guest_pat(unsigned long val);

void vcpu_handle_hypercall(void);

-bool vcpu_handle_io_access(union registers *guest_regs);
+bool vcpu_handle_io_access(void);

bool vcpu_handle_mmio_access(union registers *guest_regs);

bool vcpu_handle_msr_read(union registers *guest_regs);

diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c
index 87db3b3..0b80dab 100644
--- a/hypervisor/arch/x86/svm.c
+++ b/hypervisor/arch/x86/svm.c
@@ -993,7 +993,7 @@ void vcpu_handle_exit(struct per_cpu *cpu_data)
break;

case VMEXIT_IOIO:
cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_PIO]++;

- if (vcpu_handle_io_access(guest_regs))
+ if (vcpu_handle_io_access())

return;
break;
/* TODO: Handle VMEXIT_AVIC_NOACCEL and VMEXIT_AVIC_INCOMPLETE_IPI */

diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
index 4325e8f..6ddf42a 100644
--- a/hypervisor/arch/x86/vcpu.c
+++ b/hypervisor/arch/x86/vcpu.c
@@ -165,7 +165,7 @@ void vcpu_handle_hypercall(void)
vcpu_deactivate_vmm();
}

-bool vcpu_handle_io_access(union registers *guest_regs)
+bool vcpu_handle_io_access(void)

{
struct vcpu_io_intercept io;
int result = 0;

diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index b3aa2ca..06b1329 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -1101,7 +1101,7 @@ void vcpu_handle_exit(struct per_cpu *cpu_data)
break;

case EXIT_REASON_IO_INSTRUCTION:
cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_PIO]++;

- if (vcpu_handle_io_access(guest_regs))
+ if (vcpu_handle_io_access())

[Jan Kiszka]

The function only works against the current CPU, thus should avoid to

take the misleading parameter. The necessary reference can be obtained

from the per-cpu data structure now.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
hypervisor/arch/x86/include/asm/vcpu.h | 2 +-
hypervisor/arch/x86/svm.c | 2 +-

hypervisor/arch/x86/vcpu.c | 6 +++---
hypervisor/arch/x86/vmx.c | 2 +-
4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hypervisor/arch/x86/include/asm/vcpu.h b/hypervisor/arch/x86/include/asm/vcpu.h
index 88b2e6e..f43e6ef 100644
--- a/hypervisor/arch/x86/include/asm/vcpu.h
+++ b/hypervisor/arch/x86/include/asm/vcpu.h
@@ -108,7 +108,7 @@ void vcpu_vendor_set_guest_pat(unsigned long val);
void vcpu_handle_hypercall(void);

bool vcpu_handle_io_access(void);
-bool vcpu_handle_mmio_access(union registers *guest_regs);
+bool vcpu_handle_mmio_access(void);

bool vcpu_handle_msr_read(union registers *guest_regs);
bool vcpu_handle_msr_write(union registers *guest_regs);
diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c
index 0b80dab..69ccb63 100644
--- a/hypervisor/arch/x86/svm.c
+++ b/hypervisor/arch/x86/svm.c
@@ -979,7 +979,7 @@ void vcpu_handle_exit(struct per_cpu *cpu_data)

} else {
/* General MMIO (IOAPIC, PCI etc) */

cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_MMIO]++;
- if (vcpu_handle_mmio_access(guest_regs))
+ if (vcpu_handle_mmio_access())
return;
}

diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
index 6ddf42a..12cf5cb 100644
--- a/hypervisor/arch/x86/vcpu.c
+++ b/hypervisor/arch/x86/vcpu.c
@@ -191,7 +191,7 @@ invalid_access:
return false;
}

-bool vcpu_handle_mmio_access(union registers *guest_regs)
+bool vcpu_handle_mmio_access(void)

{
struct per_cpu *cpu_data = this_cpu_data();

struct guest_paging_structures pg_structs;
@@ -212,7 +212,7 @@ bool vcpu_handle_mmio_access(union registers *guest_regs)
goto invalid_access;

if (mmio.is_write)

- val = guest_regs->by_index[inst.reg_num];

+ val = cpu_data->guest_regs.by_index[inst.reg_num];

result = ioapic_access_handler(cpu_data->cell, mmio.is_write,
mmio.phys_addr, &val);

@@ -226,7 +226,7 @@ bool vcpu_handle_mmio_access(union registers *guest_regs)

if (result == 1) {
if (!mmio.is_write)

- guest_regs->by_index[inst.reg_num] = val;

+ cpu_data->guest_regs.by_index[inst.reg_num] = val;
vcpu_skip_emulated_instruction(inst.inst_len);
return true;
}
diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index 06b1329..b17ad2f 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -1106,7 +1106,7 @@ void vcpu_handle_exit(struct per_cpu *cpu_data)
break;

case EXIT_REASON_EPT_VIOLATION:
cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_MMIO]++;

- if (vcpu_handle_mmio_access(guest_regs))
+ if (vcpu_handle_mmio_access())

[Jan Kiszka]

The function only works against the current CPU, thus should avoid to
take the misleading parameter. The necessary reference can be obtained
from the per-cpu data structure now.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/include/asm/vcpu.h | 4 ++--
hypervisor/arch/x86/svm.c | 4 ++--
hypervisor/arch/x86/vcpu.c | 21 +++++++++++----------
hypervisor/arch/x86/vmx.c | 4 ++--
4 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/hypervisor/arch/x86/include/asm/vcpu.h b/hypervisor/arch/x86/include/asm/vcpu.h
index f43e6ef..a42d8af 100644
--- a/hypervisor/arch/x86/include/asm/vcpu.h
+++ b/hypervisor/arch/x86/include/asm/vcpu.h
@@ -110,8 +110,8 @@ void vcpu_handle_hypercall(void);
bool vcpu_handle_io_access(void);
bool vcpu_handle_mmio_access(void);

-bool vcpu_handle_msr_read(union registers *guest_regs);
-bool vcpu_handle_msr_write(union registers *guest_regs);
+bool vcpu_handle_msr_read(void);
+bool vcpu_handle_msr_write(void);

bool vcpu_handle_xsetbv(void);

diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c
index 69ccb63..0299cc1 100644
--- a/hypervisor/arch/x86/svm.c
+++ b/hypervisor/arch/x86/svm.c
@@ -838,7 +838,7 @@ static bool svm_handle_msr_write(union registers *guest_regs,
return true;
}

- return vcpu_handle_msr_write(guest_regs);
+ return vcpu_handle_msr_write();
}

/*
@@ -962,7 +962,7 @@ void vcpu_handle_exit(struct per_cpu *cpu_data)
case VMEXIT_MSR:
cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR]++;
if (!vmcb->exitinfo1)
- res = vcpu_handle_msr_read(guest_regs);
+ res = vcpu_handle_msr_read();
else
res = svm_handle_msr_write(guest_regs, cpu_data);
if (res)
diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
index 12cf5cb..0bb6bb7 100644
--- a/hypervisor/arch/x86/vcpu.c
+++ b/hypervisor/arch/x86/vcpu.c
@@ -239,23 +239,24 @@ invalid_access:
return false;
}

-bool vcpu_handle_msr_read(union registers *guest_regs)
+bool vcpu_handle_msr_read(void)

{
struct per_cpu *cpu_data = this_cpu_data();

- switch (guest_regs->rcx) {
+ switch (cpu_data->guest_regs.rcx) {
case MSR_X2APIC_BASE ... MSR_X2APIC_END:
x2apic_handle_read();
break;
case MSR_IA32_PAT:
- set_rdmsr_value(guest_regs, cpu_data->pat);
+ set_rdmsr_value(&cpu_data->guest_regs, cpu_data->pat);
break;
case MSR_IA32_MTRR_DEF_TYPE:
- set_rdmsr_value(guest_regs, cpu_data->mtrr_def_type);
+ set_rdmsr_value(&cpu_data->guest_regs,
+ cpu_data->mtrr_def_type);
break;
default:
panic_printk("FATAL: Unhandled MSR read: %x\n",
- guest_regs->rcx);
+ cpu_data->guest_regs.rcx);
return false;
}

@@ -263,19 +264,19 @@ bool vcpu_handle_msr_read(union registers *guest_regs)
return true;
}

-bool vcpu_handle_msr_write(union registers *guest_regs)
+bool vcpu_handle_msr_write(void)

{
struct per_cpu *cpu_data = this_cpu_data();

unsigned int bit_pos, pa;
unsigned long val;

- switch (guest_regs->rcx) {
+ switch (cpu_data->guest_regs.rcx) {
case MSR_X2APIC_BASE ... MSR_X2APIC_END:

if (!x2apic_handle_write())
return false;
break;
case MSR_IA32_PAT:

- val = get_wrmsr_value(guest_regs);
+ val = get_wrmsr_value(&cpu_data->guest_regs);
for (bit_pos = 0; bit_pos < 64; bit_pos += 8) {
pa = (val >> bit_pos) & 0xff;
/* filter out reserved memory types */
@@ -295,14 +296,14 @@ bool vcpu_handle_msr_write(union registers *guest_regs)
* setting the guest PAT to 0. When enabled, guest PAT +
* host-controlled MTRRs define the guest's memory types.
*/
- val = get_wrmsr_value(guest_regs);
+ val = get_wrmsr_value(&cpu_data->guest_regs);
cpu_data->mtrr_def_type = val;
vcpu_vendor_set_guest_pat(val & MTRR_ENABLE ?
cpu_data->pat : 0);
break;
default:
panic_printk("FATAL: Unhandled MSR write: %x\n",
- guest_regs->rcx);
+ cpu_data->guest_regs.rcx);
return false;
}

diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index b17ad2f..2d2c036 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -1078,7 +1078,7 @@ void vcpu_handle_exit(struct per_cpu *cpu_data)
break;
case EXIT_REASON_MSR_READ:
cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR]++;
- if (vcpu_handle_msr_read(guest_regs))
+ if (vcpu_handle_msr_read())
return;
break;
case EXIT_REASON_MSR_WRITE:
@@ -1087,7 +1087,7 @@ void vcpu_handle_exit(struct per_cpu *cpu_data)
/* ignore writes */
vcpu_skip_emulated_instruction(X86_INST_LEN_WRMSR);
return;
- } else if (vcpu_handle_msr_write(guest_regs))
+ } else if (vcpu_handle_msr_write())
return;
break;
case EXIT_REASON_APIC_ACCESS:
--
2.1.4

[Jan Kiszka]

Guest registers can be retrieved inline.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/vmx.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index 2d2c036..a6d507b 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -892,8 +892,7 @@ static void update_efer(void)
vmcs_read32(VM_ENTRY_CONTROLS) | VM_ENTRY_IA32E_MODE);
}

-static bool vmx_handle_cr(union registers *guest_regs,
- struct per_cpu *cpu_data)
+static bool vmx_handle_cr(void)
{
u64 exit_qualification = vmcs_read64(EXIT_QUALIFICATION);
unsigned long cr, reg, val;
@@ -906,7 +905,7 @@ static bool vmx_handle_cr(union registers *guest_regs,

if (reg == 4)
val = vmcs_read64(GUEST_RSP);
else

- val = guest_regs->by_index[15 - reg];
+ val = this_cpu_data()->guest_regs.by_index[15 - reg];

if (cr == 0 || cr == 4) {
vcpu_skip_emulated_instruction(X86_INST_LEN_MOV_TO_CR);

@@ -1073,7 +1072,7 @@ void vcpu_handle_exit(struct per_cpu *cpu_data)

return;
case EXIT_REASON_CR_ACCESS:
cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_CR]++;

- if (vmx_handle_cr(guest_regs, cpu_data))
+ if (vmx_handle_cr())
return;
break;
case EXIT_REASON_MSR_READ:
--
2.1.4

[Jan Kiszka]

Shortens vcpu_handle_exit and improves readability.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/vmx.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index a6d507b..c84b93d 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -879,6 +879,20 @@ void vcpu_skip_emulated_instruction(unsigned int inst_len)
vmcs_write64(GUEST_RIP, vmcs_read64(GUEST_RIP) + inst_len);
}

+static void vmx_handle_cpuid(union registers *guest_regs)
+{
+ /* clear upper 32 bits of the involved registers */
+ guest_regs->rax &= 0xffffffff;
+ guest_regs->rbx &= 0xffffffff;
+ guest_regs->rcx &= 0xffffffff;
+ guest_regs->rdx &= 0xffffffff;
+
+ cpuid((u32 *)&guest_regs->rax, (u32 *)&guest_regs->rbx,
+ (u32 *)&guest_regs->rcx, (u32 *)&guest_regs->rdx);
+
+ vcpu_skip_emulated_instruction(X86_INST_LEN_CPUID);
+}
+
static void update_efer(void)
{
unsigned long efer = vmcs_read64(GUEST_IA32_EFER);
@@ -1059,13 +1073,7 @@ void vcpu_handle_exit(struct per_cpu *cpu_data)
iommu_check_pending_faults(cpu_data);
return;
case EXIT_REASON_CPUID:
- vcpu_skip_emulated_instruction(X86_INST_LEN_CPUID);
- guest_regs->rax &= 0xffffffff;
- guest_regs->rbx &= 0xffffffff;
- guest_regs->rcx &= 0xffffffff;
- guest_regs->rdx &= 0xffffffff;
- cpuid((u32 *)&guest_regs->rax, (u32 *)&guest_regs->rbx,
- (u32 *)&guest_regs->rcx, (u32 *)&guest_regs->rdx);
+ vmx_handle_cpuid(&cpu_data->guest_regs);
return;
case EXIT_REASON_VMCALL:
vcpu_handle_hypercall();
--
2.1.4

[Jan Kiszka]

No need to cache it. It can be derived from cpu_data now.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/x86/vmx.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index c84b93d..0cb34bd 100644
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -1050,7 +1050,6 @@ void vcpu_vendor_get_mmio_intercept(struct vcpu_mmio_intercept *mmio)

void vcpu_handle_exit(struct per_cpu *cpu_data)
{
- union registers *guest_regs = &cpu_data->guest_regs;

u32 reason = vmcs_read32(VM_EXIT_REASON);
int sipi_vector;

@@ -1090,7 +1089,7 @@ void vcpu_handle_exit(struct per_cpu *cpu_data)
break;
case EXIT_REASON_MSR_WRITE:
cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_MSR]++;
- if (guest_regs->rcx == MSR_IA32_PERF_GLOBAL_CTRL) {
+ if (cpu_data->guest_regs.rcx == MSR_IA32_PERF_GLOBAL_CTRL) {

/* ignore writes */
vcpu_skip_emulated_instruction(X86_INST_LEN_WRMSR);
return;

@@ -1124,7 +1123,7 @@ void vcpu_handle_exit(struct per_cpu *cpu_data)
dump_vm_exit_details(reason);
break;
}
- dump_guest_regs(guest_regs);
+ dump_guest_regs(&cpu_data->guest_regs);
panic_park();
}

--
2.1.4

[Valentine Sinitsyn]

Hi Jan,

Always wondered, why do we need this handler? Is it a VMX requirement to
trap CPUID? I mean, it just forwards what CPUID returns to the guest -
we don't mask any bits etc.

Valentine

[Jan Kiszka]

VMX traps unconditionally, unfortunately. And it's also unlikely to
change as almost all hypervisors like to tweak the reported values in
one way or the other.

Jan

--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

[Valentine Sinitsyn]

On 14.04.2015 00:08, Jan Kiszka wrote:
> VMX traps unconditionally, unfortunately. And it's also unlikely to
> change as almost all hypervisors like to tweak the reported values in
> one way or the other.

I see. IIRC I'm not intercepting CPUID in SVM code. If we'll want to
manifest ourselves in 0x8000-something, this will need to change.

Valentine

[Jan Kiszka]

Right. Not yet in sight, though.

[Valentine Sinitsyn]

Hi Jan,

On 13.04.2015 09:57, Jan Kiszka wrote:

> This is part two of flushing out next. It's about dropping some per-cpu
> function parameters and local variables. Moreover, it refactors struct
> registers to a union and allows to obtain that one directly from struct
> per_cpu.
>
> To recall: The philosophy behind removing per-cpu parameters from our
> internal (inter-module) interfaces is not only to avoid having to pass
> them down to deep call stacks but primarily to document that functions
> work on the current CPU only, thus the parameter cannot be freely
> chosen.
Looks good, but please consider having some macro for
this_cpu_data()->guest_regs.by_index[x] as it could make troubles with
our 80-character limit. I feel a shortcut like LOCAL_REG(x) (the first
name that came up to my mind) will be more readable.

Valentine

[Jan Kiszka]

On 2015-04-14 20:47, Valentine Sinitsyn wrote:
> Hi Jan,
>
> On 13.04.2015 09:57, Jan Kiszka wrote:
>> This is part two of flushing out next. It's about dropping some per-cpu
>> function parameters and local variables. Moreover, it refactors struct
>> registers to a union and allows to obtain that one directly from struct
>> per_cpu.
>>
>> To recall: The philosophy behind removing per-cpu parameters from our
>> internal (inter-module) interfaces is not only to avoid having to pass
>> them down to deep call stacks but primarily to document that functions
>> work on the current CPU only, thus the parameter cannot be freely
>> chosen.
> Looks good, but please consider having some macro for

Thanks for the review!

> this_cpu_data()->guest_regs.by_index[x] as it could make troubles with
> our 80-character limit. I feel a shortcut like LOCAL_REG(x) (the first
> name that came up to my mind) will be more readable.

In such cases I tend to define guest_regs as a local var and use that as
abbreviation. I'm not a big fan of too intransparent macros, but I was
considering to introduce this_guest_regs(). However, there is no cheap
way for doing this because DEFINE_PER_CPU_ACCESSOR only works on basic
types.