* [Qemu-devel] [PATCH 0/7] target-arm: Support AArch64 KVM
@ 2013-11-28 13:33 Peter Maydell
2013-11-28 13:33 ` [Qemu-devel] [PATCH 1/7] target-arm/kvm: Split 32 bit only code into its own file Peter Maydell
` (7 more replies)
0 siblings, 8 replies; 38+ messages in thread
From: Peter Maydell @ 2013-11-28 13:33 UTC (permalink / raw)
To: qemu-devel; +Cc: agraf, afaerber, kvmarm, Mian M. Hamayun, patches
This patchset adds support for basic AArch64 KVM VM control. It sits
on top of the mach-virt + cpu-host patchset I sent out last week.
The core of these patches is the work done by Mian M. Hamayun; I've
just taken that, refactored it a bit to sit on top of the
mach-virt+cpu-host patchset instead af defining an 'a57' cpu, and
made some minor bugfixes as part of the code review I did in the
process.
(Mian: my apologies for not looking at your last patch series sooner.
This actually ended up in my generating extra work for myself since
if I'd been a bit quicker about that we could have dealt with more of
this in code review rather than my fixing things up. I'll try to do
better next time around.)
This patch series supports:
* 64 bit KVM VM control
* SMP and UP
* PSCI boot of secondary CPUs
It doesn't support:
* migration
* reset (partly because there's no way to reset a mach-virt system yet)
* anything except "-cpu host"
* debugging the VM via qemu gdbstub
* running 32 bit VMs on a 64 bit system
[Mian's patchset includes support for that but I have left it out
for the moment because it needs more thought about UI and so on]
You can find this patchset plus the mach-virt/cpu-host one at
git://git.linaro.org/people/pmaydell/qemu-arm.git mach-virt-64
https://git.linaro.org/gitweb?p=people/pmaydell/qemu-arm.git;a=shortlog;h=refs/heads/mach-virt-64
thanks
-- PMM
Mian M. Hamayun (2):
target-arm: Add minimal KVM AArch64 support
hw/arm/boot: Add boot support for AArch64 processor
Peter Maydell (5):
target-arm/kvm: Split 32 bit only code into its own file
target-arm: Clean up handling of AArch64 PSTATE
configure: Enable KVM for aarch64 host/target combination
hw/arm/boot: Allow easier swapping in of different loader code
default-configs: Add config for aarch64-softmmu
configure | 2 +-
default-configs/aarch64-softmmu.mak | 9 +
hw/arm/boot.c | 190 +++++++++----
linux-user/signal.c | 6 +-
target-arm/Makefile.objs | 2 +
target-arm/cpu.c | 6 +
target-arm/cpu.h | 68 ++++-
target-arm/gdbstub64.c | 4 +-
target-arm/kvm.c | 495 +--------------------------------
target-arm/kvm32.c | 515 +++++++++++++++++++++++++++++++++++
target-arm/kvm64.c | 204 ++++++++++++++
target-arm/translate-a64.c | 12 +-
12 files changed, 952 insertions(+), 561 deletions(-)
create mode 100644 default-configs/aarch64-softmmu.mak
create mode 100644 target-arm/kvm32.c
create mode 100644 target-arm/kvm64.c
--
1.7.9.5
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 1/7] target-arm/kvm: Split 32 bit only code into its own file
2013-11-28 13:33 [Qemu-devel] [PATCH 0/7] target-arm: Support AArch64 KVM Peter Maydell
@ 2013-11-28 13:33 ` Peter Maydell
2013-12-16 23:39 ` Christoffer Dall
2013-11-28 13:33 ` [Qemu-devel] [PATCH 2/7] target-arm: Clean up handling of AArch64 PSTATE Peter Maydell
` (6 subsequent siblings)
7 siblings, 1 reply; 38+ messages in thread
From: Peter Maydell @ 2013-11-28 13:33 UTC (permalink / raw)
To: qemu-devel; +Cc: agraf, afaerber, kvmarm, Mian M. Hamayun, patches
Split ARM KVM support code which is 32 bit specific out into its
own file, which we only compile on 32 bit hosts. This will give
us a place to add the 64 bit support code without adding lots of
ifdefs to kvm.c.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target-arm/Makefile.objs | 1 +
target-arm/kvm.c | 491 -------------------------------------------
target-arm/kvm32.c | 515 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 516 insertions(+), 491 deletions(-)
create mode 100644 target-arm/kvm32.c
diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs
index 356fbfc..d1db77c 100644
--- a/target-arm/Makefile.objs
+++ b/target-arm/Makefile.objs
@@ -6,3 +6,4 @@ obj-y += translate.o op_helper.o helper.o cpu.o
obj-y += neon_helper.o iwmmxt_helper.o
obj-y += gdbstub.o
obj-$(TARGET_AARCH64) += cpu64.o translate-a64.o gdbstub64.o
+obj-$(call land,$(CONFIG_KVM),$(call lnot,$(TARGET_AARCH64))) += kvm32.o
diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index f865dac..5cdb3b9 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -100,120 +100,6 @@ void kvm_arm_destroy_scratch_host_vcpu(int *fdarray)
}
}
-static inline void set_feature(uint64_t *features, int feature)
-{
- *features |= 1ULL << feature;
-}
-
-bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
-{
- /* Identify the feature bits corresponding to the host CPU, and
- * fill out the ARMHostCPUClass fields accordingly. To do this
- * we have to create a scratch VM, create a single CPU inside it,
- * and then query that CPU for the relevant ID registers.
- */
- int i, ret, fdarray[3];
- uint32_t midr, id_pfr0, id_isar0, mvfr1;
- uint64_t features = 0;
- /* Old kernels may not know about the PREFERRED_TARGET ioctl: however
- * we know these will only support creating one kind of guest CPU,
- * which is its preferred CPU type.
- */
- static const uint32_t cpus_to_try[] = {
- QEMU_KVM_ARM_TARGET_CORTEX_A15,
- QEMU_KVM_ARM_TARGET_NONE
- };
- struct kvm_vcpu_init init;
- struct kvm_one_reg idregs[] = {
- {
- .id = KVM_REG_ARM | KVM_REG_SIZE_U32
- | ENCODE_CP_REG(15, 0, 0, 0, 0, 0),
- .addr = (uintptr_t)&midr,
- },
- {
- .id = KVM_REG_ARM | KVM_REG_SIZE_U32
- | ENCODE_CP_REG(15, 0, 0, 1, 0, 0),
- .addr = (uintptr_t)&id_pfr0,
- },
- {
- .id = KVM_REG_ARM | KVM_REG_SIZE_U32
- | ENCODE_CP_REG(15, 0, 0, 2, 0, 0),
- .addr = (uintptr_t)&id_isar0,
- },
- {
- .id = KVM_REG_ARM | KVM_REG_SIZE_U32
- | KVM_REG_ARM_VFP | KVM_REG_ARM_VFP_MVFR1,
- .addr = (uintptr_t)&mvfr1,
- },
- };
-
- if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) {
- return false;
- }
-
- ahcc->target = init.target;
-
- /* This is not strictly blessed by the device tree binding docs yet,
- * but in practice the kernel does not care about this string so
- * there is no point maintaining an KVM_ARM_TARGET_* -> string table.
- */
- ahcc->dtb_compatible = "arm,arm-v7";
-
- for (i = 0; i < ARRAY_SIZE(idregs); i++) {
- ret = ioctl(fdarray[2], KVM_GET_ONE_REG, &idregs[i]);
- if (ret) {
- break;
- }
- }
-
- kvm_arm_destroy_scratch_host_vcpu(fdarray);
-
- if (ret) {
- return false;
- }
-
- /* Now we've retrieved all the register information we can
- * set the feature bits based on the ID register fields.
- * We can assume any KVM supporting CPU is at least a v7
- * with VFPv3, LPAE and the generic timers; this in turn implies
- * most of the other feature bits, but a few must be tested.
- */
- set_feature(&features, ARM_FEATURE_V7);
- set_feature(&features, ARM_FEATURE_VFP3);
- set_feature(&features, ARM_FEATURE_LPAE);
- set_feature(&features, ARM_FEATURE_GENERIC_TIMER);
-
- switch (extract32(id_isar0, 24, 4)) {
- case 1:
- set_feature(&features, ARM_FEATURE_THUMB_DIV);
- break;
- case 2:
- set_feature(&features, ARM_FEATURE_ARM_DIV);
- set_feature(&features, ARM_FEATURE_THUMB_DIV);
- break;
- default:
- break;
- }
-
- if (extract32(id_pfr0, 12, 4) == 1) {
- set_feature(&features, ARM_FEATURE_THUMB2EE);
- }
- if (extract32(mvfr1, 20, 4) == 1) {
- set_feature(&features, ARM_FEATURE_VFP_FP16);
- }
- if (extract32(mvfr1, 12, 4) == 1) {
- set_feature(&features, ARM_FEATURE_NEON);
- }
- if (extract32(mvfr1, 28, 4) == 1) {
- /* FMAC support implies VFPv4 */
- set_feature(&features, ARM_FEATURE_VFP4);
- }
-
- ahcc->features = features;
-
- return true;
-}
-
static void kvm_arm_host_cpu_class_init(ObjectClass *oc, void *data)
{
ARMHostCPUClass *ahcc = ARM_HOST_CPU_CLASS(oc);
@@ -265,144 +151,6 @@ unsigned long kvm_arch_vcpu_id(CPUState *cpu)
return cpu->cpu_index;
}
-static bool reg_syncs_via_tuple_list(uint64_t regidx)
-{
- /* Return true if the regidx is a register we should synchronize
- * via the cpreg_tuples array (ie is not a core reg we sync by
- * hand in kvm_arch_get/put_registers())
- */
- switch (regidx & KVM_REG_ARM_COPROC_MASK) {
- case KVM_REG_ARM_CORE:
- case KVM_REG_ARM_VFP:
- return false;
- default:
- return true;
- }
-}
-
-static int compare_u64(const void *a, const void *b)
-{
- if (*(uint64_t *)a > *(uint64_t *)b) {
- return 1;
- }
- if (*(uint64_t *)a < *(uint64_t *)b) {
- return -1;
- }
- return 0;
-}
-
-int kvm_arch_init_vcpu(CPUState *cs)
-{
- struct kvm_vcpu_init init;
- int i, ret, arraylen;
- uint64_t v;
- struct kvm_one_reg r;
- struct kvm_reg_list rl;
- struct kvm_reg_list *rlp;
- ARMCPU *cpu = ARM_CPU(cs);
-
- if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE) {
- fprintf(stderr, "KVM is not supported for this guest CPU type\n");
- return -EINVAL;
- }
-
- init.target = cpu->kvm_target;
- memset(init.features, 0, sizeof(init.features));
- if (cpu->start_powered_off) {
- init.features[0] = 1 << KVM_ARM_VCPU_POWER_OFF;
- }
- ret = kvm_vcpu_ioctl(cs, KVM_ARM_VCPU_INIT, &init);
- if (ret) {
- return ret;
- }
- /* Query the kernel to make sure it supports 32 VFP
- * registers: QEMU's "cortex-a15" CPU is always a
- * VFP-D32 core. The simplest way to do this is just
- * to attempt to read register d31.
- */
- r.id = KVM_REG_ARM | KVM_REG_SIZE_U64 | KVM_REG_ARM_VFP | 31;
- r.addr = (uintptr_t)(&v);
- ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
- if (ret == -ENOENT) {
- return -EINVAL;
- }
-
- /* Populate the cpreg list based on the kernel's idea
- * of what registers exist (and throw away the TCG-created list).
- */
- rl.n = 0;
- ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, &rl);
- if (ret != -E2BIG) {
- return ret;
- }
- rlp = g_malloc(sizeof(struct kvm_reg_list) + rl.n * sizeof(uint64_t));
- rlp->n = rl.n;
- ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, rlp);
- if (ret) {
- goto out;
- }
- /* Sort the list we get back from the kernel, since cpreg_tuples
- * must be in strictly ascending order.
- */
- qsort(&rlp->reg, rlp->n, sizeof(rlp->reg[0]), compare_u64);
-
- for (i = 0, arraylen = 0; i < rlp->n; i++) {
- if (!reg_syncs_via_tuple_list(rlp->reg[i])) {
- continue;
- }
- switch (rlp->reg[i] & KVM_REG_SIZE_MASK) {
- case KVM_REG_SIZE_U32:
- case KVM_REG_SIZE_U64:
- break;
- default:
- fprintf(stderr, "Can't handle size of register in kernel list\n");
- ret = -EINVAL;
- goto out;
- }
-
- arraylen++;
- }
-
- cpu->cpreg_indexes = g_renew(uint64_t, cpu->cpreg_indexes, arraylen);
- cpu->cpreg_values = g_renew(uint64_t, cpu->cpreg_values, arraylen);
- cpu->cpreg_vmstate_indexes = g_renew(uint64_t, cpu->cpreg_vmstate_indexes,
- arraylen);
- cpu->cpreg_vmstate_values = g_renew(uint64_t, cpu->cpreg_vmstate_values,
- arraylen);
- cpu->cpreg_array_len = arraylen;
- cpu->cpreg_vmstate_array_len = arraylen;
-
- for (i = 0, arraylen = 0; i < rlp->n; i++) {
- uint64_t regidx = rlp->reg[i];
- if (!reg_syncs_via_tuple_list(regidx)) {
- continue;
- }
- cpu->cpreg_indexes[arraylen] = regidx;
- arraylen++;
- }
- assert(cpu->cpreg_array_len == arraylen);
-
- if (!write_kvmstate_to_list(cpu)) {
- /* Shouldn't happen unless kernel is inconsistent about
- * what registers exist.
- */
- fprintf(stderr, "Initial read of kernel register state failed\n");
- ret = -EINVAL;
- goto out;
- }
-
- /* Save a copy of the initial register values so that we can
- * feed it back to the kernel on VCPU reset.
- */
- cpu->cpreg_reset_values = g_memdup(cpu->cpreg_values,
- cpu->cpreg_array_len *
- sizeof(cpu->cpreg_values[0]));
-
-out:
- g_free(rlp);
- return ret;
-}
-
/* We track all the KVM devices which need their memory addresses
* passing to the kernel in a list of these structures.
* When board init is complete we run through the list and
@@ -563,232 +311,6 @@ bool write_list_to_kvmstate(ARMCPU *cpu)
return ok;
}
-typedef struct Reg {
- uint64_t id;
- int offset;
-} Reg;
-
-#define COREREG(KERNELNAME, QEMUFIELD) \
- { \
- KVM_REG_ARM | KVM_REG_SIZE_U32 | \
- KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(KERNELNAME), \
- offsetof(CPUARMState, QEMUFIELD) \
- }
-
-#define VFPSYSREG(R) \
- { \
- KVM_REG_ARM | KVM_REG_SIZE_U32 | KVM_REG_ARM_VFP | \
- KVM_REG_ARM_VFP_##R, \
- offsetof(CPUARMState, vfp.xregs[ARM_VFP_##R]) \
- }
-
-static const Reg regs[] = {
- /* R0_usr .. R14_usr */
- COREREG(usr_regs.uregs[0], regs[0]),
- COREREG(usr_regs.uregs[1], regs[1]),
- COREREG(usr_regs.uregs[2], regs[2]),
- COREREG(usr_regs.uregs[3], regs[3]),
- COREREG(usr_regs.uregs[4], regs[4]),
- COREREG(usr_regs.uregs[5], regs[5]),
- COREREG(usr_regs.uregs[6], regs[6]),
- COREREG(usr_regs.uregs[7], regs[7]),
- COREREG(usr_regs.uregs[8], usr_regs[0]),
- COREREG(usr_regs.uregs[9], usr_regs[1]),
- COREREG(usr_regs.uregs[10], usr_regs[2]),
- COREREG(usr_regs.uregs[11], usr_regs[3]),
- COREREG(usr_regs.uregs[12], usr_regs[4]),
- COREREG(usr_regs.uregs[13], banked_r13[0]),
- COREREG(usr_regs.uregs[14], banked_r14[0]),
- /* R13, R14, SPSR for SVC, ABT, UND, IRQ banks */
- COREREG(svc_regs[0], banked_r13[1]),
- COREREG(svc_regs[1], banked_r14[1]),
- COREREG(svc_regs[2], banked_spsr[1]),
- COREREG(abt_regs[0], banked_r13[2]),
- COREREG(abt_regs[1], banked_r14[2]),
- COREREG(abt_regs[2], banked_spsr[2]),
- COREREG(und_regs[0], banked_r13[3]),
- COREREG(und_regs[1], banked_r14[3]),
- COREREG(und_regs[2], banked_spsr[3]),
- COREREG(irq_regs[0], banked_r13[4]),
- COREREG(irq_regs[1], banked_r14[4]),
- COREREG(irq_regs[2], banked_spsr[4]),
- /* R8_fiq .. R14_fiq and SPSR_fiq */
- COREREG(fiq_regs[0], fiq_regs[0]),
- COREREG(fiq_regs[1], fiq_regs[1]),
- COREREG(fiq_regs[2], fiq_regs[2]),
- COREREG(fiq_regs[3], fiq_regs[3]),
- COREREG(fiq_regs[4], fiq_regs[4]),
- COREREG(fiq_regs[5], banked_r13[5]),
- COREREG(fiq_regs[6], banked_r14[5]),
- COREREG(fiq_regs[7], banked_spsr[5]),
- /* R15 */
- COREREG(usr_regs.uregs[15], regs[15]),
- /* VFP system registers */
- VFPSYSREG(FPSID),
- VFPSYSREG(MVFR1),
- VFPSYSREG(MVFR0),
- VFPSYSREG(FPEXC),
- VFPSYSREG(FPINST),
- VFPSYSREG(FPINST2),
-};
-
-int kvm_arch_put_registers(CPUState *cs, int level)
-{
- ARMCPU *cpu = ARM_CPU(cs);
- CPUARMState *env = &cpu->env;
- struct kvm_one_reg r;
- int mode, bn;
- int ret, i;
- uint32_t cpsr, fpscr;
-
- /* Make sure the banked regs are properly set */
- mode = env->uncached_cpsr & CPSR_M;
- bn = bank_number(mode);
- if (mode == ARM_CPU_MODE_FIQ) {
- memcpy(env->fiq_regs, env->regs + 8, 5 * sizeof(uint32_t));
- } else {
- memcpy(env->usr_regs, env->regs + 8, 5 * sizeof(uint32_t));
- }
- env->banked_r13[bn] = env->regs[13];
- env->banked_r14[bn] = env->regs[14];
- env->banked_spsr[bn] = env->spsr;
-
- /* Now we can safely copy stuff down to the kernel */
- for (i = 0; i < ARRAY_SIZE(regs); i++) {
- r.id = regs[i].id;
- r.addr = (uintptr_t)(env) + regs[i].offset;
- ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
- if (ret) {
- return ret;
- }
- }
-
- /* Special cases which aren't a single CPUARMState field */
- cpsr = cpsr_read(env);
- r.id = KVM_REG_ARM | KVM_REG_SIZE_U32 |
- KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(usr_regs.ARM_cpsr);
- r.addr = (uintptr_t)(&cpsr);
- ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
- if (ret) {
- return ret;
- }
-
- /* VFP registers */
- r.id = KVM_REG_ARM | KVM_REG_SIZE_U64 | KVM_REG_ARM_VFP;
- for (i = 0; i < 32; i++) {
- r.addr = (uintptr_t)(&env->vfp.regs[i]);
- ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
- if (ret) {
- return ret;
- }
- r.id++;
- }
-
- r.id = KVM_REG_ARM | KVM_REG_SIZE_U32 | KVM_REG_ARM_VFP |
- KVM_REG_ARM_VFP_FPSCR;
- fpscr = vfp_get_fpscr(env);
- r.addr = (uintptr_t)&fpscr;
- ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
- if (ret) {
- return ret;
- }
-
- /* Note that we do not call write_cpustate_to_list()
- * here, so we are only writing the tuple list back to
- * KVM. This is safe because nothing can change the
- * CPUARMState cp15 fields (in particular gdb accesses cannot)
- * and so there are no changes to sync. In fact syncing would
- * be wrong at this point: for a constant register where TCG and
- * KVM disagree about its value, the preceding write_list_to_cpustate()
- * would not have had any effect on the CPUARMState value (since the
- * register is read-only), and a write_cpustate_to_list() here would
- * then try to write the TCG value back into KVM -- this would either
- * fail or incorrectly change the value the guest sees.
- *
- * If we ever want to allow the user to modify cp15 registers via
- * the gdb stub, we would need to be more clever here (for instance
- * tracking the set of registers kvm_arch_get_registers() successfully
- * managed to update the CPUARMState with, and only allowing those
- * to be written back up into the kernel).
- */
- if (!write_list_to_kvmstate(cpu)) {
- return EINVAL;
- }
-
- return ret;
-}
-
-int kvm_arch_get_registers(CPUState *cs)
-{
- ARMCPU *cpu = ARM_CPU(cs);
- CPUARMState *env = &cpu->env;
- struct kvm_one_reg r;
- int mode, bn;
- int ret, i;
- uint32_t cpsr, fpscr;
-
- for (i = 0; i < ARRAY_SIZE(regs); i++) {
- r.id = regs[i].id;
- r.addr = (uintptr_t)(env) + regs[i].offset;
- ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
- if (ret) {
- return ret;
- }
- }
-
- /* Special cases which aren't a single CPUARMState field */
- r.id = KVM_REG_ARM | KVM_REG_SIZE_U32 |
- KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(usr_regs.ARM_cpsr);
- r.addr = (uintptr_t)(&cpsr);
- ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
- if (ret) {
- return ret;
- }
- cpsr_write(env, cpsr, 0xffffffff);
-
- /* Make sure the current mode regs are properly set */
- mode = env->uncached_cpsr & CPSR_M;
- bn = bank_number(mode);
- if (mode == ARM_CPU_MODE_FIQ) {
- memcpy(env->regs + 8, env->fiq_regs, 5 * sizeof(uint32_t));
- } else {
- memcpy(env->regs + 8, env->usr_regs, 5 * sizeof(uint32_t));
- }
- env->regs[13] = env->banked_r13[bn];
- env->regs[14] = env->banked_r14[bn];
- env->spsr = env->banked_spsr[bn];
-
- /* VFP registers */
- r.id = KVM_REG_ARM | KVM_REG_SIZE_U64 | KVM_REG_ARM_VFP;
- for (i = 0; i < 32; i++) {
- r.addr = (uintptr_t)(&env->vfp.regs[i]);
- ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
- if (ret) {
- return ret;
- }
- r.id++;
- }
-
- r.id = KVM_REG_ARM | KVM_REG_SIZE_U32 | KVM_REG_ARM_VFP |
- KVM_REG_ARM_VFP_FPSCR;
- r.addr = (uintptr_t)&fpscr;
- ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
- if (ret) {
- return ret;
- }
- vfp_set_fpscr(env, fpscr);
-
- if (!write_kvmstate_to_list(cpu)) {
- return EINVAL;
- }
- /* Note that it's OK to have registers which aren't in CPUState,
- * so we can ignore a failure return here.
- */
- write_list_to_cpustate(cpu);
-
- return 0;
-}
-
void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run)
{
}
@@ -802,19 +324,6 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
return 0;
}
-void kvm_arch_reset_vcpu(CPUState *cs)
-{
- /* Feed the kernel back its initial register state */
- ARMCPU *cpu = ARM_CPU(cs);
-
- memmove(cpu->cpreg_values, cpu->cpreg_reset_values,
- cpu->cpreg_array_len * sizeof(cpu->cpreg_values[0]));
-
- if (!write_list_to_kvmstate(cpu)) {
- abort();
- }
-}
-
bool kvm_arch_stop_on_emulation_error(CPUState *cs)
{
return true;
diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c
new file mode 100644
index 0000000..a4fde07
--- /dev/null
+++ b/target-arm/kvm32.c
@@ -0,0 +1,515 @@
+/*
+ * ARM implementation of KVM hooks, 32 bit specific code.
+ *
+ * Copyright Christoffer Dall 2009-2010
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <sys/ioctl.h>
+#include <sys/mman.h>
+
+#include <linux/kvm.h>
+
+#include "qemu-common.h"
+#include "qemu/timer.h"
+#include "sysemu/sysemu.h"
+#include "sysemu/kvm.h"
+#include "kvm_arm.h"
+#include "cpu.h"
+#include "hw/arm/arm.h"
+
+static inline void set_feature(uint64_t *features, int feature)
+{
+ *features |= 1ULL << feature;
+}
+
+bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
+{
+ /* Identify the feature bits corresponding to the host CPU, and
+ * fill out the ARMHostCPUClass fields accordingly. To do this
+ * we have to create a scratch VM, create a single CPU inside it,
+ * and then query that CPU for the relevant ID registers.
+ */
+ int i, ret, fdarray[3];
+ uint32_t midr, id_pfr0, id_isar0, mvfr1;
+ uint64_t features = 0;
+ /* Old kernels may not know about the PREFERRED_TARGET ioctl: however
+ * we know these will only support creating one kind of guest CPU,
+ * which is its preferred CPU type.
+ */
+ static const uint32_t cpus_to_try[] = {
+ QEMU_KVM_ARM_TARGET_CORTEX_A15,
+ QEMU_KVM_ARM_TARGET_NONE
+ };
+ struct kvm_vcpu_init init;
+ struct kvm_one_reg idregs[] = {
+ {
+ .id = KVM_REG_ARM | KVM_REG_SIZE_U32
+ | ENCODE_CP_REG(15, 0, 0, 0, 0, 0),
+ .addr = (uintptr_t)&midr,
+ },
+ {
+ .id = KVM_REG_ARM | KVM_REG_SIZE_U32
+ | ENCODE_CP_REG(15, 0, 0, 1, 0, 0),
+ .addr = (uintptr_t)&id_pfr0,
+ },
+ {
+ .id = KVM_REG_ARM | KVM_REG_SIZE_U32
+ | ENCODE_CP_REG(15, 0, 0, 2, 0, 0),
+ .addr = (uintptr_t)&id_isar0,
+ },
+ {
+ .id = KVM_REG_ARM | KVM_REG_SIZE_U32
+ | KVM_REG_ARM_VFP | KVM_REG_ARM_VFP_MVFR1,
+ .addr = (uintptr_t)&mvfr1,
+ },
+ };
+
+ if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) {
+ return false;
+ }
+
+ ahcc->target = init.target;
+
+ /* This is not strictly blessed by the device tree binding docs yet,
+ * but in practice the kernel does not care about this string so
+ * there is no point maintaining an KVM_ARM_TARGET_* -> string table.
+ */
+ ahcc->dtb_compatible = "arm,arm-v7";
+
+ for (i = 0; i < ARRAY_SIZE(idregs); i++) {
+ ret = ioctl(fdarray[2], KVM_GET_ONE_REG, &idregs[i]);
+ if (ret) {
+ break;
+ }
+ }
+
+ kvm_arm_destroy_scratch_host_vcpu(fdarray);
+
+ if (ret) {
+ return false;
+ }
+
+ /* Now we've retrieved all the register information we can
+ * set the feature bits based on the ID register fields.
+ * We can assume any KVM supporting CPU is at least a v7
+ * with VFPv3, LPAE and the generic timers; this in turn implies
+ * most of the other feature bits, but a few must be tested.
+ */
+ set_feature(&features, ARM_FEATURE_V7);
+ set_feature(&features, ARM_FEATURE_VFP3);
+ set_feature(&features, ARM_FEATURE_LPAE);
+ set_feature(&features, ARM_FEATURE_GENERIC_TIMER);
+
+ switch (extract32(id_isar0, 24, 4)) {
+ case 1:
+ set_feature(&features, ARM_FEATURE_THUMB_DIV);
+ break;
+ case 2:
+ set_feature(&features, ARM_FEATURE_ARM_DIV);
+ set_feature(&features, ARM_FEATURE_THUMB_DIV);
+ break;
+ default:
+ break;
+ }
+
+ if (extract32(id_pfr0, 12, 4) == 1) {
+ set_feature(&features, ARM_FEATURE_THUMB2EE);
+ }
+ if (extract32(mvfr1, 20, 4) == 1) {
+ set_feature(&features, ARM_FEATURE_VFP_FP16);
+ }
+ if (extract32(mvfr1, 12, 4) == 1) {
+ set_feature(&features, ARM_FEATURE_NEON);
+ }
+ if (extract32(mvfr1, 28, 4) == 1) {
+ /* FMAC support implies VFPv4 */
+ set_feature(&features, ARM_FEATURE_VFP4);
+ }
+
+ ahcc->features = features;
+
+ return true;
+}
+
+static bool reg_syncs_via_tuple_list(uint64_t regidx)
+{
+ /* Return true if the regidx is a register we should synchronize
+ * via the cpreg_tuples array (ie is not a core reg we sync by
+ * hand in kvm_arch_get/put_registers())
+ */
+ switch (regidx & KVM_REG_ARM_COPROC_MASK) {
+ case KVM_REG_ARM_CORE:
+ case KVM_REG_ARM_VFP:
+ return false;
+ default:
+ return true;
+ }
+}
+
+static int compare_u64(const void *a, const void *b)
+{
+ if (*(uint64_t *)a > *(uint64_t *)b) {
+ return 1;
+ }
+ if (*(uint64_t *)a < *(uint64_t *)b) {
+ return -1;
+ }
+ return 0;
+}
+
+int kvm_arch_init_vcpu(CPUState *cs)
+{
+ struct kvm_vcpu_init init;
+ int i, ret, arraylen;
+ uint64_t v;
+ struct kvm_one_reg r;
+ struct kvm_reg_list rl;
+ struct kvm_reg_list *rlp;
+ ARMCPU *cpu = ARM_CPU(cs);
+
+ if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE) {
+ fprintf(stderr, "KVM is not supported for this guest CPU type\n");
+ return -EINVAL;
+ }
+
+ init.target = cpu->kvm_target;
+ memset(init.features, 0, sizeof(init.features));
+ if (cpu->start_powered_off) {
+ init.features[0] = 1 << KVM_ARM_VCPU_POWER_OFF;
+ }
+ ret = kvm_vcpu_ioctl(cs, KVM_ARM_VCPU_INIT, &init);
+ if (ret) {
+ return ret;
+ }
+ /* Query the kernel to make sure it supports 32 VFP
+ * registers: QEMU's "cortex-a15" CPU is always a
+ * VFP-D32 core. The simplest way to do this is just
+ * to attempt to read register d31.
+ */
+ r.id = KVM_REG_ARM | KVM_REG_SIZE_U64 | KVM_REG_ARM_VFP | 31;
+ r.addr = (uintptr_t)(&v);
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
+ if (ret == -ENOENT) {
+ return -EINVAL;
+ }
+
+ /* Populate the cpreg list based on the kernel's idea
+ * of what registers exist (and throw away the TCG-created list).
+ */
+ rl.n = 0;
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, &rl);
+ if (ret != -E2BIG) {
+ return ret;
+ }
+ rlp = g_malloc(sizeof(struct kvm_reg_list) + rl.n * sizeof(uint64_t));
+ rlp->n = rl.n;
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, rlp);
+ if (ret) {
+ goto out;
+ }
+ /* Sort the list we get back from the kernel, since cpreg_tuples
+ * must be in strictly ascending order.
+ */
+ qsort(&rlp->reg, rlp->n, sizeof(rlp->reg[0]), compare_u64);
+
+ for (i = 0, arraylen = 0; i < rlp->n; i++) {
+ if (!reg_syncs_via_tuple_list(rlp->reg[i])) {
+ continue;
+ }
+ switch (rlp->reg[i] & KVM_REG_SIZE_MASK) {
+ case KVM_REG_SIZE_U32:
+ case KVM_REG_SIZE_U64:
+ break;
+ default:
+ fprintf(stderr, "Can't handle size of register in kernel list\n");
+ ret = -EINVAL;
+ goto out;
+ }
+
+ arraylen++;
+ }
+
+ cpu->cpreg_indexes = g_renew(uint64_t, cpu->cpreg_indexes, arraylen);
+ cpu->cpreg_values = g_renew(uint64_t, cpu->cpreg_values, arraylen);
+ cpu->cpreg_vmstate_indexes = g_renew(uint64_t, cpu->cpreg_vmstate_indexes,
+ arraylen);
+ cpu->cpreg_vmstate_values = g_renew(uint64_t, cpu->cpreg_vmstate_values,
+ arraylen);
+ cpu->cpreg_array_len = arraylen;
+ cpu->cpreg_vmstate_array_len = arraylen;
+
+ for (i = 0, arraylen = 0; i < rlp->n; i++) {
+ uint64_t regidx = rlp->reg[i];
+ if (!reg_syncs_via_tuple_list(regidx)) {
+ continue;
+ }
+ cpu->cpreg_indexes[arraylen] = regidx;
+ arraylen++;
+ }
+ assert(cpu->cpreg_array_len == arraylen);
+
+ if (!write_kvmstate_to_list(cpu)) {
+ /* Shouldn't happen unless kernel is inconsistent about
+ * what registers exist.
+ */
+ fprintf(stderr, "Initial read of kernel register state failed\n");
+ ret = -EINVAL;
+ goto out;
+ }
+
+ /* Save a copy of the initial register values so that we can
+ * feed it back to the kernel on VCPU reset.
+ */
+ cpu->cpreg_reset_values = g_memdup(cpu->cpreg_values,
+ cpu->cpreg_array_len *
+ sizeof(cpu->cpreg_values[0]));
+
+out:
+ g_free(rlp);
+ return ret;
+}
+
+typedef struct Reg {
+ uint64_t id;
+ int offset;
+} Reg;
+
+#define COREREG(KERNELNAME, QEMUFIELD) \
+ { \
+ KVM_REG_ARM | KVM_REG_SIZE_U32 | \
+ KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(KERNELNAME), \
+ offsetof(CPUARMState, QEMUFIELD) \
+ }
+
+#define VFPSYSREG(R) \
+ { \
+ KVM_REG_ARM | KVM_REG_SIZE_U32 | KVM_REG_ARM_VFP | \
+ KVM_REG_ARM_VFP_##R, \
+ offsetof(CPUARMState, vfp.xregs[ARM_VFP_##R]) \
+ }
+
+static const Reg regs[] = {
+ /* R0_usr .. R14_usr */
+ COREREG(usr_regs.uregs[0], regs[0]),
+ COREREG(usr_regs.uregs[1], regs[1]),
+ COREREG(usr_regs.uregs[2], regs[2]),
+ COREREG(usr_regs.uregs[3], regs[3]),
+ COREREG(usr_regs.uregs[4], regs[4]),
+ COREREG(usr_regs.uregs[5], regs[5]),
+ COREREG(usr_regs.uregs[6], regs[6]),
+ COREREG(usr_regs.uregs[7], regs[7]),
+ COREREG(usr_regs.uregs[8], usr_regs[0]),
+ COREREG(usr_regs.uregs[9], usr_regs[1]),
+ COREREG(usr_regs.uregs[10], usr_regs[2]),
+ COREREG(usr_regs.uregs[11], usr_regs[3]),
+ COREREG(usr_regs.uregs[12], usr_regs[4]),
+ COREREG(usr_regs.uregs[13], banked_r13[0]),
+ COREREG(usr_regs.uregs[14], banked_r14[0]),
+ /* R13, R14, SPSR for SVC, ABT, UND, IRQ banks */
+ COREREG(svc_regs[0], banked_r13[1]),
+ COREREG(svc_regs[1], banked_r14[1]),
+ COREREG(svc_regs[2], banked_spsr[1]),
+ COREREG(abt_regs[0], banked_r13[2]),
+ COREREG(abt_regs[1], banked_r14[2]),
+ COREREG(abt_regs[2], banked_spsr[2]),
+ COREREG(und_regs[0], banked_r13[3]),
+ COREREG(und_regs[1], banked_r14[3]),
+ COREREG(und_regs[2], banked_spsr[3]),
+ COREREG(irq_regs[0], banked_r13[4]),
+ COREREG(irq_regs[1], banked_r14[4]),
+ COREREG(irq_regs[2], banked_spsr[4]),
+ /* R8_fiq .. R14_fiq and SPSR_fiq */
+ COREREG(fiq_regs[0], fiq_regs[0]),
+ COREREG(fiq_regs[1], fiq_regs[1]),
+ COREREG(fiq_regs[2], fiq_regs[2]),
+ COREREG(fiq_regs[3], fiq_regs[3]),
+ COREREG(fiq_regs[4], fiq_regs[4]),
+ COREREG(fiq_regs[5], banked_r13[5]),
+ COREREG(fiq_regs[6], banked_r14[5]),
+ COREREG(fiq_regs[7], banked_spsr[5]),
+ /* R15 */
+ COREREG(usr_regs.uregs[15], regs[15]),
+ /* VFP system registers */
+ VFPSYSREG(FPSID),
+ VFPSYSREG(MVFR1),
+ VFPSYSREG(MVFR0),
+ VFPSYSREG(FPEXC),
+ VFPSYSREG(FPINST),
+ VFPSYSREG(FPINST2),
+};
+
+int kvm_arch_put_registers(CPUState *cs, int level)
+{
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
+ struct kvm_one_reg r;
+ int mode, bn;
+ int ret, i;
+ uint32_t cpsr, fpscr;
+
+ /* Make sure the banked regs are properly set */
+ mode = env->uncached_cpsr & CPSR_M;
+ bn = bank_number(mode);
+ if (mode == ARM_CPU_MODE_FIQ) {
+ memcpy(env->fiq_regs, env->regs + 8, 5 * sizeof(uint32_t));
+ } else {
+ memcpy(env->usr_regs, env->regs + 8, 5 * sizeof(uint32_t));
+ }
+ env->banked_r13[bn] = env->regs[13];
+ env->banked_r14[bn] = env->regs[14];
+ env->banked_spsr[bn] = env->spsr;
+
+ /* Now we can safely copy stuff down to the kernel */
+ for (i = 0; i < ARRAY_SIZE(regs); i++) {
+ r.id = regs[i].id;
+ r.addr = (uintptr_t)(env) + regs[i].offset;
+ ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
+ if (ret) {
+ return ret;
+ }
+ }
+
+ /* Special cases which aren't a single CPUARMState field */
+ cpsr = cpsr_read(env);
+ r.id = KVM_REG_ARM | KVM_REG_SIZE_U32 |
+ KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(usr_regs.ARM_cpsr);
+ r.addr = (uintptr_t)(&cpsr);
+ ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
+ if (ret) {
+ return ret;
+ }
+
+ /* VFP registers */
+ r.id = KVM_REG_ARM | KVM_REG_SIZE_U64 | KVM_REG_ARM_VFP;
+ for (i = 0; i < 32; i++) {
+ r.addr = (uintptr_t)(&env->vfp.regs[i]);
+ ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
+ if (ret) {
+ return ret;
+ }
+ r.id++;
+ }
+
+ r.id = KVM_REG_ARM | KVM_REG_SIZE_U32 | KVM_REG_ARM_VFP |
+ KVM_REG_ARM_VFP_FPSCR;
+ fpscr = vfp_get_fpscr(env);
+ r.addr = (uintptr_t)&fpscr;
+ ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
+ if (ret) {
+ return ret;
+ }
+
+ /* Note that we do not call write_cpustate_to_list()
+ * here, so we are only writing the tuple list back to
+ * KVM. This is safe because nothing can change the
+ * CPUARMState cp15 fields (in particular gdb accesses cannot)
+ * and so there are no changes to sync. In fact syncing would
+ * be wrong at this point: for a constant register where TCG and
+ * KVM disagree about its value, the preceding write_list_to_cpustate()
+ * would not have had any effect on the CPUARMState value (since the
+ * register is read-only), and a write_cpustate_to_list() here would
+ * then try to write the TCG value back into KVM -- this would either
+ * fail or incorrectly change the value the guest sees.
+ *
+ * If we ever want to allow the user to modify cp15 registers via
+ * the gdb stub, we would need to be more clever here (for instance
+ * tracking the set of registers kvm_arch_get_registers() successfully
+ * managed to update the CPUARMState with, and only allowing those
+ * to be written back up into the kernel).
+ */
+ if (!write_list_to_kvmstate(cpu)) {
+ return EINVAL;
+ }
+
+ return ret;
+}
+
+int kvm_arch_get_registers(CPUState *cs)
+{
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
+ struct kvm_one_reg r;
+ int mode, bn;
+ int ret, i;
+ uint32_t cpsr, fpscr;
+
+ for (i = 0; i < ARRAY_SIZE(regs); i++) {
+ r.id = regs[i].id;
+ r.addr = (uintptr_t)(env) + regs[i].offset;
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
+ if (ret) {
+ return ret;
+ }
+ }
+
+ /* Special cases which aren't a single CPUARMState field */
+ r.id = KVM_REG_ARM | KVM_REG_SIZE_U32 |
+ KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(usr_regs.ARM_cpsr);
+ r.addr = (uintptr_t)(&cpsr);
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
+ if (ret) {
+ return ret;
+ }
+ cpsr_write(env, cpsr, 0xffffffff);
+
+ /* Make sure the current mode regs are properly set */
+ mode = env->uncached_cpsr & CPSR_M;
+ bn = bank_number(mode);
+ if (mode == ARM_CPU_MODE_FIQ) {
+ memcpy(env->regs + 8, env->fiq_regs, 5 * sizeof(uint32_t));
+ } else {
+ memcpy(env->regs + 8, env->usr_regs, 5 * sizeof(uint32_t));
+ }
+ env->regs[13] = env->banked_r13[bn];
+ env->regs[14] = env->banked_r14[bn];
+ env->spsr = env->banked_spsr[bn];
+
+ /* VFP registers */
+ r.id = KVM_REG_ARM | KVM_REG_SIZE_U64 | KVM_REG_ARM_VFP;
+ for (i = 0; i < 32; i++) {
+ r.addr = (uintptr_t)(&env->vfp.regs[i]);
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
+ if (ret) {
+ return ret;
+ }
+ r.id++;
+ }
+
+ r.id = KVM_REG_ARM | KVM_REG_SIZE_U32 | KVM_REG_ARM_VFP |
+ KVM_REG_ARM_VFP_FPSCR;
+ r.addr = (uintptr_t)&fpscr;
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
+ if (ret) {
+ return ret;
+ }
+ vfp_set_fpscr(env, fpscr);
+
+ if (!write_kvmstate_to_list(cpu)) {
+ return EINVAL;
+ }
+ /* Note that it's OK to have registers which aren't in CPUState,
+ * so we can ignore a failure return here.
+ */
+ write_list_to_cpustate(cpu);
+
+ return 0;
+}
+
+void kvm_arch_reset_vcpu(CPUState *cs)
+{
+ /* Feed the kernel back its initial register state */
+ ARMCPU *cpu = ARM_CPU(cs);
+
+ memmove(cpu->cpreg_values, cpu->cpreg_reset_values,
+ cpu->cpreg_array_len * sizeof(cpu->cpreg_values[0]));
+
+ if (!write_list_to_kvmstate(cpu)) {
+ abort();
+ }
+}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 2/7] target-arm: Clean up handling of AArch64 PSTATE
2013-11-28 13:33 [Qemu-devel] [PATCH 0/7] target-arm: Support AArch64 KVM Peter Maydell
2013-11-28 13:33 ` [Qemu-devel] [PATCH 1/7] target-arm/kvm: Split 32 bit only code into its own file Peter Maydell
@ 2013-11-28 13:33 ` Peter Maydell
2013-12-16 23:39 ` Christoffer Dall
2013-11-28 13:33 ` [Qemu-devel] [PATCH 3/7] target-arm: Add minimal KVM AArch64 support Peter Maydell
` (5 subsequent siblings)
7 siblings, 1 reply; 38+ messages in thread
From: Peter Maydell @ 2013-11-28 13:33 UTC (permalink / raw)
To: qemu-devel; +Cc: agraf, afaerber, kvmarm, Mian M. Hamayun, patches
The env->pstate field is a little odd since it doesn't strictly
speaking represent an architectural register. However it's convenient
for QEMU to use it to hold the various PSTATE architectural bits
in the same format the architecture specifies for SPSR registers
(since this is the same format the kernel uses for signal handlers
and the KVM register). Add some structure to how we deal with it:
* document what env->pstate is
* add some #defines for various bits in it
* add helpers for reading/writing it taking account of caching
of NZCV, and use them where appropriate
* reset it on startup
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
linux-user/signal.c | 6 ++--
target-arm/cpu.c | 6 ++++
target-arm/cpu.h | 68 +++++++++++++++++++++++++++++++++++++-------
target-arm/gdbstub64.c | 4 +--
target-arm/translate-a64.c | 12 ++++----
5 files changed, 76 insertions(+), 20 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 7751c47..4e7148a 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -1171,7 +1171,7 @@ static int target_setup_sigframe(struct target_rt_sigframe *sf,
}
__put_user(env->xregs[31], &sf->uc.tuc_mcontext.sp);
__put_user(env->pc, &sf->uc.tuc_mcontext.pc);
- __put_user(env->pstate, &sf->uc.tuc_mcontext.pstate);
+ __put_user(pstate_read(env), &sf->uc.tuc_mcontext.pstate);
__put_user(/*current->thread.fault_address*/ 0,
&sf->uc.tuc_mcontext.fault_address);
@@ -1210,6 +1210,7 @@ static int target_restore_sigframe(CPUARMState *env,
struct target_aux_context *aux =
(struct target_aux_context *)sf->uc.tuc_mcontext.__reserved;
uint32_t magic, size;
+ uint64_t pstate;
target_to_host_sigset(&set, &sf->uc.tuc_sigmask);
sigprocmask(SIG_SETMASK, &set, NULL);
@@ -1220,7 +1221,8 @@ static int target_restore_sigframe(CPUARMState *env,
__get_user(env->xregs[31], &sf->uc.tuc_mcontext.sp);
__get_user(env->pc, &sf->uc.tuc_mcontext.pc);
- __get_user(env->pstate, &sf->uc.tuc_mcontext.pstate);
+ __get_user(pstate, &sf->uc.tuc_mcontext.pstate);
+ pstate_write(env, pstate);
__get_user(magic, &aux->fpsimd.head.magic);
__get_user(size, &aux->fpsimd.head.size);
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 0635e78..42057ad 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -88,6 +88,12 @@ static void arm_cpu_reset(CPUState *s)
if (arm_feature(env, ARM_FEATURE_AARCH64)) {
/* 64 bit CPUs always start in 64 bit mode */
env->aarch64 = 1;
+#if defined(CONFIG_USER_ONLY)
+ env->pstate = PSTATE_MODE_EL0t;
+#else
+ env->pstate = PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F
+ | PSTATE_MODE_EL1h;
+#endif
}
#if defined(CONFIG_USER_ONLY)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index c3f007f..ff7aac5 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -113,8 +113,13 @@ typedef struct CPUARMState {
/* Regs for A64 mode. */
uint64_t xregs[32];
uint64_t pc;
- /* TODO: pstate doesn't correspond to an architectural register;
- * it would be better modelled as the underlying fields.
+ /* PSTATE isn't an architectural register for ARMv8. However, it is
+ * convenient for us to assemble the underlying state into a 32 bit format
+ * identical to the architectural format used for the SPSR. (This is also
+ * what the Linux kernel's 'pstate' field in signal handlers and KVM's
+ * 'pstate' register are.) NZCV are kept in their split fields; aarch64
+ * is an inverted split version of PSTATE.nRW (aka M[4]); other bits are
+ * stored here when in AArch64.
*/
uint32_t pstate;
uint32_t aarch64; /* 1 if CPU is in aarch64 state; inverse of PSTATE.nRW */
@@ -309,15 +314,6 @@ static inline bool is_a64(CPUARMState *env)
return env->aarch64;
}
-#define PSTATE_N_SHIFT 3
-#define PSTATE_N (1 << PSTATE_N_SHIFT)
-#define PSTATE_Z_SHIFT 2
-#define PSTATE_Z (1 << PSTATE_Z_SHIFT)
-#define PSTATE_C_SHIFT 1
-#define PSTATE_C (1 << PSTATE_C_SHIFT)
-#define PSTATE_V_SHIFT 0
-#define PSTATE_V (1 << PSTATE_V_SHIFT)
-
/* you can call this signal handler from your SIGBUS and SIGSEGV
signal handlers to inform the virtual CPU of exceptions. non zero
is returned if the signal was handled by the virtual CPU. */
@@ -352,6 +348,56 @@ int cpu_arm_handle_mmu_fault (CPUARMState *env, target_ulong address, int rw,
/* Execution state bits. MRS read as zero, MSR writes ignored. */
#define CPSR_EXEC (CPSR_T | CPSR_IT | CPSR_J)
+/* Bit definitions for ARMv8 SPSR (PSTATE) format.
+ * Only these are valid when in AArch64 mode; in
+ * AArch32 mode SPSRs are basically CPSR-format.
+ */
+#define PSTATE_M (0xFU)
+#define PSTATE_nRW (1U << 4)
+#define PSTATE_F (1U << 6)
+#define PSTATE_I (1U << 7)
+#define PSTATE_A (1U << 8)
+#define PSTATE_D (1U << 9)
+#define PSTATE_IL (1U << 20)
+#define PSTATE_SS (1U << 21)
+#define PSTATE_V (1U << 28)
+#define PSTATE_C (1U << 29)
+#define PSTATE_Z (1U << 30)
+#define PSTATE_N (1U << 31)
+#define PSTATE_NZCV (PSTATE_N | PSTATE_Z | PSTATE_C | PSTATE_V)
+#define CACHED_PSTATE_BITS (PSTATE_NZCV)
+/* Mode values for AArch64 */
+#define PSTATE_MODE_EL3h 13
+#define PSTATE_MODE_EL3t 12
+#define PSTATE_MODE_EL2h 9
+#define PSTATE_MODE_EL2t 8
+#define PSTATE_MODE_EL1h 5
+#define PSTATE_MODE_EL1t 4
+#define PSTATE_MODE_EL0t 0
+
+/* Return the current PSTATE value. For the moment we don't support 32<->64 bit
+ * interprocessing, so we don't attempt to sync with the cpsr state used by
+ * the 32 bit decoder.
+ */
+static inline uint32_t pstate_read(CPUARMState *env)
+{
+ int ZF;
+
+ ZF = (env->ZF == 0);
+ return (env->NF & 0x80000000) | (ZF << 30)
+ | (env->CF << 29) | ((env->VF & 0x80000000) >> 3)
+ | env->pstate;
+}
+
+static inline void pstate_write(CPUARMState *env, uint32_t val)
+{
+ env->ZF = (~val) & PSTATE_Z;
+ env->NF = val;
+ env->CF = (val >> 29) & 1;
+ env->VF = (val << 3) & 0x80000000;
+ env->pstate = val & ~CACHED_PSTATE_BITS;
+}
+
/* Return the current CPSR value. */
uint32_t cpsr_read(CPUARMState *env);
/* Set the CPSR. Note that some bits of mask must be all-set or all-clear. */
diff --git a/target-arm/gdbstub64.c b/target-arm/gdbstub64.c
index 7cb6a7c..e8a8295 100644
--- a/target-arm/gdbstub64.c
+++ b/target-arm/gdbstub64.c
@@ -37,7 +37,7 @@ int aarch64_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
return gdb_get_reg64(mem_buf, env->pc);
break;
case 33:
- return gdb_get_reg32(mem_buf, env->pstate);
+ return gdb_get_reg32(mem_buf, pstate_read(env));
}
/* Unknown register. */
return 0;
@@ -65,7 +65,7 @@ int aarch64_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
return 8;
case 33:
/* CPSR */
- env->pstate = tmp;
+ pstate_write(env, tmp);
return 4;
}
/* Unknown register. */
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index f120088..932b601 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -67,6 +67,7 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
{
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
+ uint32_t psr = pstate_read(env);
int i;
cpu_fprintf(f, "PC=%016"PRIx64" SP=%016"PRIx64"\n",
@@ -79,11 +80,12 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
cpu_fprintf(f, " ");
}
}
- cpu_fprintf(f, "PSTATE=%c%c%c%c\n",
- env->pstate & PSTATE_N ? 'n' : '.',
- env->pstate & PSTATE_Z ? 'z' : '.',
- env->pstate & PSTATE_C ? 'c' : '.',
- env->pstate & PSTATE_V ? 'v' : '.');
+ cpu_fprintf(f, "PSTATE=%08x (flags %c%c%c%c)\n",
+ psr,
+ psr & PSTATE_N ? 'N' : '-',
+ psr & PSTATE_Z ? 'Z' : '-',
+ psr & PSTATE_C ? 'C' : '-',
+ psr & PSTATE_V ? 'V' : '-');
cpu_fprintf(f, "\n");
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 3/7] target-arm: Add minimal KVM AArch64 support
2013-11-28 13:33 [Qemu-devel] [PATCH 0/7] target-arm: Support AArch64 KVM Peter Maydell
2013-11-28 13:33 ` [Qemu-devel] [PATCH 1/7] target-arm/kvm: Split 32 bit only code into its own file Peter Maydell
2013-11-28 13:33 ` [Qemu-devel] [PATCH 2/7] target-arm: Clean up handling of AArch64 PSTATE Peter Maydell
@ 2013-11-28 13:33 ` Peter Maydell
2013-12-16 23:39 ` Christoffer Dall
2013-11-28 13:33 ` [Qemu-devel] [PATCH 4/7] configure: Enable KVM for aarch64 host/target combination Peter Maydell
` (4 subsequent siblings)
7 siblings, 1 reply; 38+ messages in thread
From: Peter Maydell @ 2013-11-28 13:33 UTC (permalink / raw)
To: qemu-devel; +Cc: agraf, afaerber, kvmarm, Mian M. Hamayun, patches
From: "Mian M. Hamayun" <m.hamayun@virtualopensystems.com>
Add the bare minimum set of functions needed for control of an
AArch64 KVM vcpu:
* CPU initialization
* minimal get/put register functions which only handle the
basic state of the CPU
Signed-off-by: Mian M. Hamayun <m.hamayun@virtualopensystems.com>
[PMM: significantly overhauled; most notably:
* code lives in kvm64.c rather than using #ifdefs
* support '-cpu host' rather than implicitly using whatever the
host's CPU is regardless of what the user requests
* fix bug attempting to get/set nonexistent X[31]
* fix bug writing 64 bit kernel pstate into uint32_t env field
]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target-arm/Makefile.objs | 1 +
target-arm/kvm.c | 4 +
target-arm/kvm64.c | 204 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 209 insertions(+)
create mode 100644 target-arm/kvm64.c
diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs
index d1db77c..5493a4c 100644
--- a/target-arm/Makefile.objs
+++ b/target-arm/Makefile.objs
@@ -7,3 +7,4 @@ obj-y += neon_helper.o iwmmxt_helper.o
obj-y += gdbstub.o
obj-$(TARGET_AARCH64) += cpu64.o translate-a64.o gdbstub64.o
obj-$(call land,$(CONFIG_KVM),$(call lnot,$(TARGET_AARCH64))) += kvm32.o
+obj-$(call land,$(CONFIG_KVM),$(TARGET_AARCH64)) += kvm64.o
diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index 5cdb3b9..1d2688d 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -128,7 +128,11 @@ static void kvm_arm_host_cpu_initfn(Object *obj)
static const TypeInfo host_arm_cpu_type_info = {
.name = TYPE_ARM_HOST_CPU,
+#ifdef TARGET_AARCH64
+ .parent = TYPE_AARCH64_CPU,
+#else
.parent = TYPE_ARM_CPU,
+#endif
.instance_init = kvm_arm_host_cpu_initfn,
.class_init = kvm_arm_host_cpu_class_init,
.class_size = sizeof(ARMHostCPUClass),
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
new file mode 100644
index 0000000..599db5d
--- /dev/null
+++ b/target-arm/kvm64.c
@@ -0,0 +1,204 @@
+/*
+ * ARM implementation of KVM hooks, 64 bit specific code
+ *
+ * Copyright Mian-M. Hamayun 2013, Virtual Open Systems
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <sys/ioctl.h>
+#include <sys/mman.h>
+
+#include <linux/kvm.h>
+
+#include "qemu-common.h"
+#include "qemu/timer.h"
+#include "sysemu/sysemu.h"
+#include "sysemu/kvm.h"
+#include "kvm_arm.h"
+#include "cpu.h"
+#include "hw/arm/arm.h"
+
+static inline void set_feature(uint64_t *features, int feature)
+{
+ *features |= 1ULL << feature;
+}
+
+bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
+{
+ /* Identify the feature bits corresponding to the host CPU, and
+ * fill out the ARMHostCPUClass fields accordingly. To do this
+ * we have to create a scratch VM, create a single CPU inside it,
+ * and then query that CPU for the relevant ID registers.
+ * For AArch64 we currently don't care about ID registers at
+ * all; we just want to know the CPU type.
+ */
+ int fdarray[3];
+ uint64_t features = 0;
+ /* Old kernels may not know about the PREFERRED_TARGET ioctl: however
+ * we know these will only support creating one kind of guest CPU,
+ * which is its preferred CPU type. Fortunately these old kernels
+ * support only a very limited number of CPUs.
+ */
+ static const uint32_t cpus_to_try[] = {
+ KVM_ARM_TARGET_AEM_V8,
+ KVM_ARM_TARGET_FOUNDATION_V8,
+ KVM_ARM_TARGET_CORTEX_A57,
+ QEMU_KVM_ARM_TARGET_NONE
+ };
+ struct kvm_vcpu_init init;
+
+ if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) {
+ return false;
+ }
+
+ ahcc->target = init.target;
+ ahcc->dtb_compatible = "arm,arm-v7";
+
+ kvm_arm_destroy_scratch_host_vcpu(fdarray);
+
+ /* We can assume any KVM supporting CPU is at least a v8
+ * with VFPv4+Neon; this in turn implies most of the other
+ * feature bits.
+ */
+ set_feature(&features, ARM_FEATURE_V8);
+ set_feature(&features, ARM_FEATURE_VFP4);
+ set_feature(&features, ARM_FEATURE_NEON);
+ set_feature(&features, ARM_FEATURE_AARCH64);
+
+ ahcc->features = features;
+
+ return true;
+}
+
+int kvm_arch_init_vcpu(CPUState *cs)
+{
+ ARMCPU *cpu = ARM_CPU(cs);
+ struct kvm_vcpu_init init;
+ int ret;
+
+ if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE ||
+ !arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
+ fprintf(stderr, "KVM is not supported for this guest CPU type\n");
+ return -EINVAL;
+ }
+
+ init.target = cpu->kvm_target;
+ memset(init.features, 0, sizeof(init.features));
+ if (cpu->start_powered_off) {
+ init.features[0] = 1 << KVM_ARM_VCPU_POWER_OFF;
+ }
+ ret = kvm_vcpu_ioctl(cs, KVM_ARM_VCPU_INIT, &init);
+
+ /* TODO : support for save/restore/reset of system regs via tuple list */
+
+ return ret;
+}
+
+#define AARCH64_CORE_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \
+ KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
+
+int kvm_arch_put_registers(CPUState *cs, int level)
+{
+ struct kvm_one_reg reg;
+ uint64_t val;
+ int i;
+ int ret;
+
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
+
+ for (i = 0; i < 31; i++) {
+ reg.id = AARCH64_CORE_REG(regs.regs[i]);
+ reg.addr = (uintptr_t) &env->xregs[i];
+ ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+ }
+
+ reg.id = AARCH64_CORE_REG(regs.sp);
+ reg.addr = (uintptr_t) &env->xregs[31];
+ ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+
+ /* Note that KVM thinks pstate is 64 bit but we use a uint32_t */
+ val = pstate_read(env);
+ reg.id = AARCH64_CORE_REG(regs.pstate);
+ reg.addr = (uintptr_t) &val;
+ ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+
+ reg.id = AARCH64_CORE_REG(regs.pc);
+ reg.addr = (uintptr_t) &env->pc;
+ ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+
+ /* TODO:
+ * SP_EL1
+ * ELR_EL1
+ * SPSR[]
+ * FP state
+ * system registers
+ */
+ return ret;
+}
+
+int kvm_arch_get_registers(CPUState *cs)
+{
+ struct kvm_one_reg reg;
+ uint64_t val;
+ int i;
+ int ret;
+
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
+
+ for (i = 0; i < 31; i++) {
+ reg.id = AARCH64_CORE_REG(regs.regs[i]);
+ reg.addr = (uintptr_t) &env->xregs[i];
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+ }
+
+ reg.id = AARCH64_CORE_REG(regs.sp);
+ reg.addr = (uintptr_t) &env->xregs[31];
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+
+ reg.id = AARCH64_CORE_REG(regs.pstate);
+ reg.addr = (uintptr_t) &val;
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+ pstate_write(env, val);
+
+ reg.id = AARCH64_CORE_REG(regs.pc);
+ reg.addr = (uintptr_t) &env->pc;
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+
+ /* TODO: other registers */
+ return ret;
+}
+
+void kvm_arch_reset_vcpu(CPUState *cs)
+{
+}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 4/7] configure: Enable KVM for aarch64 host/target combination
2013-11-28 13:33 [Qemu-devel] [PATCH 0/7] target-arm: Support AArch64 KVM Peter Maydell
` (2 preceding siblings ...)
2013-11-28 13:33 ` [Qemu-devel] [PATCH 3/7] target-arm: Add minimal KVM AArch64 support Peter Maydell
@ 2013-11-28 13:33 ` Peter Maydell
2013-12-16 23:40 ` Christoffer Dall
2013-11-28 13:33 ` [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code Peter Maydell
` (3 subsequent siblings)
7 siblings, 1 reply; 38+ messages in thread
From: Peter Maydell @ 2013-11-28 13:33 UTC (permalink / raw)
To: qemu-devel; +Cc: agraf, afaerber, kvmarm, Mian M. Hamayun, patches
Enable KVM if the host and target CPU are both aarch64. Note
that host aarch64 + target arm is not valid for KVM acceleration:
the 64 bit kernel does not support the ioctl interface for
32 bit CPUs. 32 bit VMs on 64 bit hosts need to be created
using the 64 bit ioctl interface; when QEMU supports this it
will be on the arch64-softmmu target with a -cpu parameter for
a 32 bit CPU, which is still an aarch64/aarch64 combination
as far as configure is concerned.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
configure | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure b/configure
index 508f6a5..3317013 100755
--- a/configure
+++ b/configure
@@ -4513,7 +4513,7 @@ case "$target_name" in
*)
esac
case "$target_name" in
- arm|i386|x86_64|ppcemb|ppc|ppc64|s390x)
+ aarch64|arm|i386|x86_64|ppcemb|ppc|ppc64|s390x)
# Make sure the target and host cpus are compatible
if test "$kvm" = "yes" -a "$target_softmmu" = "yes" -a \
\( "$target_name" = "$cpu" -o \
--
1.7.9.5
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code
2013-11-28 13:33 [Qemu-devel] [PATCH 0/7] target-arm: Support AArch64 KVM Peter Maydell
` (3 preceding siblings ...)
2013-11-28 13:33 ` [Qemu-devel] [PATCH 4/7] configure: Enable KVM for aarch64 host/target combination Peter Maydell
@ 2013-11-28 13:33 ` Peter Maydell
2013-12-13 3:19 ` Peter Crosthwaite
2013-12-16 23:40 ` Christoffer Dall
2013-11-28 13:33 ` [Qemu-devel] [PATCH 6/7] hw/arm/boot: Add boot support for AArch64 processor Peter Maydell
` (2 subsequent siblings)
7 siblings, 2 replies; 38+ messages in thread
From: Peter Maydell @ 2013-11-28 13:33 UTC (permalink / raw)
To: qemu-devel; +Cc: agraf, afaerber, kvmarm, Mian M. Hamayun, patches
For AArch64 we will obviously require a different set of
primary and secondary boot loader code fragments. However currently
we hardcode the offsets into the loader code where we must write
the entrypoint and other data into arm_load_kernel(). This makes it
hard to substitute a different loader fragment, so switch to a more
flexible scheme where instead of a raw array of instructions we use
an array of (instruction, fixup-type) pairs that indicate which
words need special action or data written into them.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/boot.c | 152 ++++++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 107 insertions(+), 45 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 55d552f..77d29a8 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -20,15 +20,33 @@
#define KERNEL_ARGS_ADDR 0x100
#define KERNEL_LOAD_ADDR 0x00010000
+typedef enum {
+ FIXUP_NONE = 0, /* do nothing */
+ FIXUP_TERMINATOR, /* end of insns */
+ FIXUP_BOARDID, /* overwrite with board ID number */
+ FIXUP_ARGPTR, /* overwrite with pointer to kernel args */
+ FIXUP_ENTRYPOINT, /* overwrite with kernel entry point */
+ FIXUP_GIC_CPU_IF, /* overwrite with GIC CPU interface address */
+ FIXUP_BOOTREG, /* overwrite with boot register address */
+ FIXUP_DSB, /* overwrite with correct DSB insn for cpu */
+ FIXUP_MAX,
+} FixupType;
+
+typedef struct ARMInsnFixup {
+ uint32_t insn;
+ FixupType fixup;
+} ARMInsnFixup;
+
/* The worlds second smallest bootloader. Set r0-r2, then jump to kernel. */
-static uint32_t bootloader[] = {
- 0xe3a00000, /* mov r0, #0 */
- 0xe59f1004, /* ldr r1, [pc, #4] */
- 0xe59f2004, /* ldr r2, [pc, #4] */
- 0xe59ff004, /* ldr pc, [pc, #4] */
- 0, /* Board ID */
- 0, /* Address of kernel args. Set by integratorcp_init. */
- 0 /* Kernel entry point. Set by integratorcp_init. */
+static const ARMInsnFixup bootloader[] = {
+ { 0xe3a00000 }, /* mov r0, #0 */
+ { 0xe59f1004 }, /* ldr r1, [pc, #4] */
+ { 0xe59f2004 }, /* ldr r2, [pc, #4] */
+ { 0xe59ff004 }, /* ldr pc, [pc, #4] */
+ { 0, FIXUP_BOARDID },
+ { 0, FIXUP_ARGPTR },
+ { 0, FIXUP_ENTRYPOINT },
+ { 0, FIXUP_TERMINATOR }
};
/* Handling for secondary CPU boot in a multicore system.
@@ -48,39 +66,83 @@ static uint32_t bootloader[] = {
#define DSB_INSN 0xf57ff04f
#define CP15_DSB_INSN 0xee070f9a /* mcr cp15, 0, r0, c7, c10, 4 */
-static uint32_t smpboot[] = {
- 0xe59f2028, /* ldr r2, gic_cpu_if */
- 0xe59f0028, /* ldr r0, startaddr */
- 0xe3a01001, /* mov r1, #1 */
- 0xe5821000, /* str r1, [r2] - set GICC_CTLR.Enable */
- 0xe3a010ff, /* mov r1, #0xff */
- 0xe5821004, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
- DSB_INSN, /* dsb */
- 0xe320f003, /* wfi */
- 0xe5901000, /* ldr r1, [r0] */
- 0xe1110001, /* tst r1, r1 */
- 0x0afffffb, /* beq <wfi> */
- 0xe12fff11, /* bx r1 */
- 0, /* gic_cpu_if: base address of GIC CPU interface */
- 0 /* bootreg: Boot register address is held here */
+static const ARMInsnFixup smpboot[] = {
+ { 0xe59f2028 }, /* ldr r2, gic_cpu_if */
+ { 0xe59f0028 }, /* ldr r0, startaddr */
+ { 0xe3a01001 }, /* mov r1, #1 */
+ { 0xe5821000 }, /* str r1, [r2] - set GICC_CTLR.Enable */
+ { 0xe3a010ff }, /* mov r1, #0xff */
+ { 0xe5821004 }, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
+ { 0, FIXUP_DSB }, /* dsb */
+ { 0xe320f003 }, /* wfi */
+ { 0xe5901000 }, /* ldr r1, [r0] */
+ { 0xe1110001 }, /* tst r1, r1 */
+ { 0x0afffffb }, /* beq <wfi> */
+ { 0xe12fff11 }, /* bx r1 */
+ { 0, FIXUP_GIC_CPU_IF },
+ { 0, FIXUP_BOOTREG },
+ { 0, FIXUP_TERMINATOR }
};
+static void write_bootloader(const char *name, hwaddr addr,
+ const ARMInsnFixup *insns, uint32_t *fixupcontext)
+{
+ /* Fix up the specified bootloader fragment and write it into
+ * guest memory using rom_add_blob_fixed(). fixupcontext is
+ * an array giving the values to write in for the fixup types
+ * which write a value into the code array.
+ */
+ int i, len;
+ uint32_t *code;
+
+ len = 0;
+ while (insns[len].fixup != FIXUP_TERMINATOR) {
+ len++;
+ }
+
+ code = g_new0(uint32_t, len);
+
+ for (i = 0; i < len; i++) {
+ uint32_t insn = insns[i].insn;
+ FixupType fixup = insns[i].fixup;
+
+ switch (fixup) {
+ case FIXUP_NONE:
+ break;
+ case FIXUP_BOARDID:
+ case FIXUP_ARGPTR:
+ case FIXUP_ENTRYPOINT:
+ case FIXUP_GIC_CPU_IF:
+ case FIXUP_BOOTREG:
+ case FIXUP_DSB:
+ insn = fixupcontext[fixup];
+ break;
+ default:
+ abort();
+ }
+ code[i] = tswap32(insn);
+ }
+
+ rom_add_blob_fixed(name, code, len * sizeof(uint32_t), addr);
+
+ g_free(code);
+}
+
static void default_write_secondary(ARMCPU *cpu,
const struct arm_boot_info *info)
{
- int n;
- smpboot[ARRAY_SIZE(smpboot) - 1] = info->smp_bootreg_addr;
- smpboot[ARRAY_SIZE(smpboot) - 2] = info->gic_cpu_if_addr;
- for (n = 0; n < ARRAY_SIZE(smpboot); n++) {
- /* Replace DSB with the pre-v7 DSB if necessary. */
- if (!arm_feature(&cpu->env, ARM_FEATURE_V7) &&
- smpboot[n] == DSB_INSN) {
- smpboot[n] = CP15_DSB_INSN;
- }
- smpboot[n] = tswap32(smpboot[n]);
+ uint32_t fixupcontext[FIXUP_MAX];
+
+ fixupcontext[FIXUP_GIC_CPU_IF] = info->gic_cpu_if_addr;
+ fixupcontext[FIXUP_BOOTREG] = info->smp_bootreg_addr;
+ if (arm_feature(&cpu->env, ARM_FEATURE_V7)) {
+ fixupcontext[FIXUP_DSB] = DSB_INSN;
+ } else {
+ fixupcontext[FIXUP_DSB] = CP15_DSB_INSN;
}
- rom_add_blob_fixed("smpboot", smpboot, sizeof(smpboot),
- info->smp_loader_start);
+
+ write_bootloader("smpboot", info->smp_loader_start,
+ smpboot, fixupcontext);
}
static void default_reset_secondary(ARMCPU *cpu,
@@ -354,7 +416,6 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
CPUState *cs = CPU(cpu);
int kernel_size;
int initrd_size;
- int n;
int is_linux = 0;
uint64_t elf_entry;
hwaddr entry;
@@ -420,6 +481,8 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
}
info->entry = entry;
if (is_linux) {
+ uint32_t fixupcontext[FIXUP_MAX];
+
if (info->initrd_filename) {
initrd_size = load_ramdisk(info->initrd_filename,
info->initrd_start,
@@ -441,7 +504,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
}
info->initrd_size = initrd_size;
- bootloader[4] = info->board_id;
+ fixupcontext[FIXUP_BOARDID] = info->board_id;
/* for device tree boot, we pass the DTB directly in r2. Otherwise
* we point to the kernel args.
@@ -456,9 +519,9 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
if (load_dtb(dtb_start, info)) {
exit(1);
}
- bootloader[5] = dtb_start;
+ fixupcontext[FIXUP_ARGPTR] = dtb_start;
} else {
- bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
+ fixupcontext[FIXUP_ARGPTR] = info->loader_start + KERNEL_ARGS_ADDR;
if (info->ram_size >= (1ULL << 32)) {
fprintf(stderr, "qemu: RAM size must be less than 4GB to boot"
" Linux kernel using ATAGS (try passing a device tree"
@@ -466,12 +529,11 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
exit(1);
}
}
- bootloader[6] = entry;
- for (n = 0; n < sizeof(bootloader) / 4; n++) {
- bootloader[n] = tswap32(bootloader[n]);
- }
- rom_add_blob_fixed("bootloader", bootloader, sizeof(bootloader),
- info->loader_start);
+ fixupcontext[FIXUP_ENTRYPOINT] = entry;
+
+ write_bootloader("bootloader", info->loader_start,
+ bootloader, fixupcontext);
+
if (info->nb_cpus > 1) {
info->write_secondary_boot(cpu, info);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 6/7] hw/arm/boot: Add boot support for AArch64 processor
2013-11-28 13:33 [Qemu-devel] [PATCH 0/7] target-arm: Support AArch64 KVM Peter Maydell
` (4 preceding siblings ...)
2013-11-28 13:33 ` [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code Peter Maydell
@ 2013-11-28 13:33 ` Peter Maydell
2013-12-16 23:40 ` Christoffer Dall
2013-11-28 13:33 ` [Qemu-devel] [PATCH 7/7] default-configs: Add config for aarch64-softmmu Peter Maydell
2013-12-05 15:23 ` [Qemu-devel] [PATCH 0/7] target-arm: Support AArch64 KVM Peter Maydell
7 siblings, 1 reply; 38+ messages in thread
From: Peter Maydell @ 2013-11-28 13:33 UTC (permalink / raw)
To: qemu-devel; +Cc: agraf, afaerber, kvmarm, Mian M. Hamayun, patches
From: "Mian M. Hamayun" <m.hamayun@virtualopensystems.com>
This commit adds support for booting a single AArch64 CPU by setting
appropriate registers. The bootloader includes placehoders for Board-ID
that are used to implement uniform indexing across different bootloaders.
Signed-off-by: Mian M. Hamayun <m.hamayun@virtualopensystems.com>
[PMM:
* updated to use ARMInsnFixup style bootloader fragments
* dropped virt.c additions
* use runtime checks for "is this an AArch64 core" rather than ifdefs
* drop some unnecessary setting of registers in reset hook
]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/boot.c | 40 +++++++++++++++++++++++++++++++++++-----
1 file changed, 35 insertions(+), 5 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 77d29a8..b6b04b7 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -19,6 +19,8 @@
#define KERNEL_ARGS_ADDR 0x100
#define KERNEL_LOAD_ADDR 0x00010000
+/* The AArch64 kernel boot protocol specifies a different preferred address */
+#define KERNEL64_LOAD_ADDR 0x00080000
typedef enum {
FIXUP_NONE = 0, /* do nothing */
@@ -37,6 +39,20 @@ typedef struct ARMInsnFixup {
FixupType fixup;
} ARMInsnFixup;
+static const ARMInsnFixup bootloader_aarch64[] = {
+ { 0x580000c0 }, /* ldr x0, 18 ; Load the lower 32-bits of DTB */
+ { 0xaa1f03e1 }, /* mov x1, xzr */
+ { 0xaa1f03e2 }, /* mov x2, xzr */
+ { 0xaa1f03e3 }, /* mov x3, xzr */
+ { 0x58000084 }, /* ldr x4, 20 ; Load the lower 32-bits of kernel entry */
+ { 0xd61f0080 }, /* br x4 ; Jump to the kernel entry point */
+ { 0, FIXUP_ARGPTR }, /* .word @DTB Lower 32-bits */
+ { 0 }, /* .word @DTB Higher 32-bits */
+ { 0, FIXUP_ENTRYPOINT }, /* .word @Kernel Entry Lower 32-bits */
+ { 0 }, /* .word @Kernel Entry Higher 32-bits */
+ { 0, FIXUP_TERMINATOR }
+};
+
/* The worlds second smallest bootloader. Set r0-r2, then jump to kernel. */
static const ARMInsnFixup bootloader[] = {
{ 0xe3a00000 }, /* mov r0, #0 */
@@ -396,7 +412,12 @@ static void do_cpu_reset(void *opaque)
env->thumb = info->entry & 1;
} else {
if (CPU(cpu) == first_cpu) {
- env->regs[15] = info->loader_start;
+ if (env->aarch64) {
+ env->pc = info->loader_start;
+ } else {
+ env->regs[15] = info->loader_start;
+ }
+
if (!info->dtb_filename) {
if (old_param) {
set_kernel_args_old(info);
@@ -418,8 +439,9 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
int initrd_size;
int is_linux = 0;
uint64_t elf_entry;
- hwaddr entry;
+ hwaddr entry, kernel_load_offset;
int big_endian;
+ static const ARMInsnFixup *primary_loader;
/* Load the kernel. */
if (!info->kernel_filename) {
@@ -429,6 +451,14 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
return;
}
+ if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
+ primary_loader = bootloader_aarch64;
+ kernel_load_offset = KERNEL64_LOAD_ADDR;
+ } else {
+ primary_loader = bootloader;
+ kernel_load_offset = KERNEL_LOAD_ADDR;
+ }
+
info->dtb_filename = qemu_opt_get(qemu_get_machine_opts(), "dtb");
if (!info->secondary_cpu_reset_hook) {
@@ -469,9 +499,9 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
&is_linux);
}
if (kernel_size < 0) {
- entry = info->loader_start + KERNEL_LOAD_ADDR;
+ entry = info->loader_start + kernel_load_offset;
kernel_size = load_image_targphys(info->kernel_filename, entry,
- info->ram_size - KERNEL_LOAD_ADDR);
+ info->ram_size - kernel_load_offset);
is_linux = 1;
}
if (kernel_size < 0) {
@@ -532,7 +562,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
fixupcontext[FIXUP_ENTRYPOINT] = entry;
write_bootloader("bootloader", info->loader_start,
- bootloader, fixupcontext);
+ primary_loader, fixupcontext);
if (info->nb_cpus > 1) {
info->write_secondary_boot(cpu, info);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 7/7] default-configs: Add config for aarch64-softmmu
2013-11-28 13:33 [Qemu-devel] [PATCH 0/7] target-arm: Support AArch64 KVM Peter Maydell
` (5 preceding siblings ...)
2013-11-28 13:33 ` [Qemu-devel] [PATCH 6/7] hw/arm/boot: Add boot support for AArch64 processor Peter Maydell
@ 2013-11-28 13:33 ` Peter Maydell
2013-12-16 23:40 ` Christoffer Dall
2013-12-05 15:23 ` [Qemu-devel] [PATCH 0/7] target-arm: Support AArch64 KVM Peter Maydell
7 siblings, 1 reply; 38+ messages in thread
From: Peter Maydell @ 2013-11-28 13:33 UTC (permalink / raw)
To: qemu-devel; +Cc: agraf, afaerber, kvmarm, Mian M. Hamayun, patches
Add a config for aarch64-softmmu; this enables building of this target.
The resulting executable doesn't know about any 64 bit CPUs, but all
the 32 bit CPUs and board models work.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
default-configs/aarch64-softmmu.mak | 9 +++++++++
1 file changed, 9 insertions(+)
create mode 100644 default-configs/aarch64-softmmu.mak
diff --git a/default-configs/aarch64-softmmu.mak b/default-configs/aarch64-softmmu.mak
new file mode 100644
index 0000000..76a45cc
--- /dev/null
+++ b/default-configs/aarch64-softmmu.mak
@@ -0,0 +1,9 @@
+# Default configuration for aarch64-softmmu
+
+include pci.mak
+include usb.mak
+
+# We support all the 32 bit boards so need all their config
+include arm-softmmu.mak
+
+# Currently no 64-bit specific config requirements
--
1.7.9.5
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] target-arm: Support AArch64 KVM
2013-11-28 13:33 [Qemu-devel] [PATCH 0/7] target-arm: Support AArch64 KVM Peter Maydell
` (6 preceding siblings ...)
2013-11-28 13:33 ` [Qemu-devel] [PATCH 7/7] default-configs: Add config for aarch64-softmmu Peter Maydell
@ 2013-12-05 15:23 ` Peter Maydell
2013-12-12 16:41 ` Peter Maydell
7 siblings, 1 reply; 38+ messages in thread
From: Peter Maydell @ 2013-12-05 15:23 UTC (permalink / raw)
To: QEMU Developers; +Cc: Christoffer Dall, kvmarm, Mian M. Hamayun, Patch Tracking
Slightly over-eager ping for code review and/or testing, since the A64
patches are going to sit on top of this and they're starting to pile up :-)
(Also noticed I forgot to cc Mian; apologies.)
thanks
-- PMM
On 28 November 2013 13:33, Peter Maydell <peter.maydell@linaro.org> wrote:
> This patchset adds support for basic AArch64 KVM VM control. It sits
> on top of the mach-virt + cpu-host patchset I sent out last week.
> The core of these patches is the work done by Mian M. Hamayun; I've
> just taken that, refactored it a bit to sit on top of the
> mach-virt+cpu-host patchset instead af defining an 'a57' cpu, and
> made some minor bugfixes as part of the code review I did in the
> process.
>
> (Mian: my apologies for not looking at your last patch series sooner.
> This actually ended up in my generating extra work for myself since
> if I'd been a bit quicker about that we could have dealt with more of
> this in code review rather than my fixing things up. I'll try to do
> better next time around.)
>
> This patch series supports:
> * 64 bit KVM VM control
> * SMP and UP
> * PSCI boot of secondary CPUs
> It doesn't support:
> * migration
> * reset (partly because there's no way to reset a mach-virt system yet)
> * anything except "-cpu host"
> * debugging the VM via qemu gdbstub
> * running 32 bit VMs on a 64 bit system
> [Mian's patchset includes support for that but I have left it out
> for the moment because it needs more thought about UI and so on]
>
> You can find this patchset plus the mach-virt/cpu-host one at
> git://git.linaro.org/people/pmaydell/qemu-arm.git mach-virt-64
> https://git.linaro.org/gitweb?p=people/pmaydell/qemu-arm.git;a=shortlog;h=refs/heads/mach-virt-64
>
> thanks
> -- PMM
>
> Mian M. Hamayun (2):
> target-arm: Add minimal KVM AArch64 support
> hw/arm/boot: Add boot support for AArch64 processor
>
> Peter Maydell (5):
> target-arm/kvm: Split 32 bit only code into its own file
> target-arm: Clean up handling of AArch64 PSTATE
> configure: Enable KVM for aarch64 host/target combination
> hw/arm/boot: Allow easier swapping in of different loader code
> default-configs: Add config for aarch64-softmmu
>
> configure | 2 +-
> default-configs/aarch64-softmmu.mak | 9 +
> hw/arm/boot.c | 190 +++++++++----
> linux-user/signal.c | 6 +-
> target-arm/Makefile.objs | 2 +
> target-arm/cpu.c | 6 +
> target-arm/cpu.h | 68 ++++-
> target-arm/gdbstub64.c | 4 +-
> target-arm/kvm.c | 495 +--------------------------------
> target-arm/kvm32.c | 515 +++++++++++++++++++++++++++++++++++
> target-arm/kvm64.c | 204 ++++++++++++++
> target-arm/translate-a64.c | 12 +-
> 12 files changed, 952 insertions(+), 561 deletions(-)
> create mode 100644 default-configs/aarch64-softmmu.mak
> create mode 100644 target-arm/kvm32.c
> create mode 100644 target-arm/kvm64.c
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] target-arm: Support AArch64 KVM
2013-12-05 15:23 ` [Qemu-devel] [PATCH 0/7] target-arm: Support AArch64 KVM Peter Maydell
@ 2013-12-12 16:41 ` Peter Maydell
0 siblings, 0 replies; 38+ messages in thread
From: Peter Maydell @ 2013-12-12 16:41 UTC (permalink / raw)
To: QEMU Developers; +Cc: Christoffer Dall, kvmarm, Mian M. Hamayun, Patch Tracking
Last call for review/testing/comments on this patchset:
I'm planning to do a target-arm pullreq early next week
which will include this patchset.
thanks
-- PMM
On 5 December 2013 15:23, Peter Maydell <peter.maydell@linaro.org> wrote:
> Slightly over-eager ping for code review and/or testing, since the A64
> patches are going to sit on top of this and they're starting to pile up :-)
> (Also noticed I forgot to cc Mian; apologies.)
>
> thanks
> -- PMM
>
> On 28 November 2013 13:33, Peter Maydell <peter.maydell@linaro.org> wrote:
>> This patchset adds support for basic AArch64 KVM VM control. It sits
>> on top of the mach-virt + cpu-host patchset I sent out last week.
>> The core of these patches is the work done by Mian M. Hamayun; I've
>> just taken that, refactored it a bit to sit on top of the
>> mach-virt+cpu-host patchset instead af defining an 'a57' cpu, and
>> made some minor bugfixes as part of the code review I did in the
>> process.
>>
>> (Mian: my apologies for not looking at your last patch series sooner.
>> This actually ended up in my generating extra work for myself since
>> if I'd been a bit quicker about that we could have dealt with more of
>> this in code review rather than my fixing things up. I'll try to do
>> better next time around.)
>>
>> This patch series supports:
>> * 64 bit KVM VM control
>> * SMP and UP
>> * PSCI boot of secondary CPUs
>> It doesn't support:
>> * migration
>> * reset (partly because there's no way to reset a mach-virt system yet)
>> * anything except "-cpu host"
>> * debugging the VM via qemu gdbstub
>> * running 32 bit VMs on a 64 bit system
>> [Mian's patchset includes support for that but I have left it out
>> for the moment because it needs more thought about UI and so on]
>>
>> You can find this patchset plus the mach-virt/cpu-host one at
>> git://git.linaro.org/people/pmaydell/qemu-arm.git mach-virt-64
>> https://git.linaro.org/gitweb?p=people/pmaydell/qemu-arm.git;a=shortlog;h=refs/heads/mach-virt-64
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code
2013-11-28 13:33 ` [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code Peter Maydell
@ 2013-12-13 3:19 ` Peter Crosthwaite
2013-12-13 10:05 ` Peter Maydell
2013-12-16 23:40 ` Christoffer Dall
1 sibling, 1 reply; 38+ messages in thread
From: Peter Crosthwaite @ 2013-12-13 3:19 UTC (permalink / raw)
To: Peter Maydell
Cc: Alexander Graf, Mian M. Hamayun, patches,
qemu-devel@nongnu.org Developers, Andreas Färber, kvmarm
On Thu, Nov 28, 2013 at 11:33 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> For AArch64 we will obviously require a different set of
> primary and secondary boot loader code fragments. However currently
> we hardcode the offsets into the loader code where we must write
> the entrypoint and other data into arm_load_kernel(). This makes it
> hard to substitute a different loader fragment, so switch to a more
> flexible scheme where instead of a raw array of instructions we use
> an array of (instruction, fixup-type) pairs that indicate which
> words need special action or data written into them.
>
Why do we need blobs at all? Cant we just fix arm/boot to directly
setup the CPU state to the desired? Rather than complex blobs that
execute ARM instructions just manipulate the regs directly. The
booloader already directly accesses r15 for setting the boot blob
entry point so I don't see whats wrong with setting board-id and dtb
directly either. See the microblaze bootloader for an example.
Regards,
Peter
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/arm/boot.c | 152 ++++++++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 107 insertions(+), 45 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 55d552f..77d29a8 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -20,15 +20,33 @@
> #define KERNEL_ARGS_ADDR 0x100
> #define KERNEL_LOAD_ADDR 0x00010000
>
> +typedef enum {
> + FIXUP_NONE = 0, /* do nothing */
> + FIXUP_TERMINATOR, /* end of insns */
> + FIXUP_BOARDID, /* overwrite with board ID number */
> + FIXUP_ARGPTR, /* overwrite with pointer to kernel args */
> + FIXUP_ENTRYPOINT, /* overwrite with kernel entry point */
> + FIXUP_GIC_CPU_IF, /* overwrite with GIC CPU interface address */
> + FIXUP_BOOTREG, /* overwrite with boot register address */
> + FIXUP_DSB, /* overwrite with correct DSB insn for cpu */
> + FIXUP_MAX,
> +} FixupType;
> +
> +typedef struct ARMInsnFixup {
> + uint32_t insn;
> + FixupType fixup;
> +} ARMInsnFixup;
> +
> /* The worlds second smallest bootloader. Set r0-r2, then jump to kernel. */
> -static uint32_t bootloader[] = {
> - 0xe3a00000, /* mov r0, #0 */
> - 0xe59f1004, /* ldr r1, [pc, #4] */
> - 0xe59f2004, /* ldr r2, [pc, #4] */
> - 0xe59ff004, /* ldr pc, [pc, #4] */
> - 0, /* Board ID */
> - 0, /* Address of kernel args. Set by integratorcp_init. */
> - 0 /* Kernel entry point. Set by integratorcp_init. */
> +static const ARMInsnFixup bootloader[] = {
> + { 0xe3a00000 }, /* mov r0, #0 */
> + { 0xe59f1004 }, /* ldr r1, [pc, #4] */
> + { 0xe59f2004 }, /* ldr r2, [pc, #4] */
> + { 0xe59ff004 }, /* ldr pc, [pc, #4] */
> + { 0, FIXUP_BOARDID },
> + { 0, FIXUP_ARGPTR },
> + { 0, FIXUP_ENTRYPOINT },
> + { 0, FIXUP_TERMINATOR }
> };
>
> /* Handling for secondary CPU boot in a multicore system.
> @@ -48,39 +66,83 @@ static uint32_t bootloader[] = {
> #define DSB_INSN 0xf57ff04f
> #define CP15_DSB_INSN 0xee070f9a /* mcr cp15, 0, r0, c7, c10, 4 */
>
> -static uint32_t smpboot[] = {
> - 0xe59f2028, /* ldr r2, gic_cpu_if */
> - 0xe59f0028, /* ldr r0, startaddr */
> - 0xe3a01001, /* mov r1, #1 */
> - 0xe5821000, /* str r1, [r2] - set GICC_CTLR.Enable */
> - 0xe3a010ff, /* mov r1, #0xff */
> - 0xe5821004, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
> - DSB_INSN, /* dsb */
> - 0xe320f003, /* wfi */
> - 0xe5901000, /* ldr r1, [r0] */
> - 0xe1110001, /* tst r1, r1 */
> - 0x0afffffb, /* beq <wfi> */
> - 0xe12fff11, /* bx r1 */
> - 0, /* gic_cpu_if: base address of GIC CPU interface */
> - 0 /* bootreg: Boot register address is held here */
> +static const ARMInsnFixup smpboot[] = {
> + { 0xe59f2028 }, /* ldr r2, gic_cpu_if */
> + { 0xe59f0028 }, /* ldr r0, startaddr */
> + { 0xe3a01001 }, /* mov r1, #1 */
> + { 0xe5821000 }, /* str r1, [r2] - set GICC_CTLR.Enable */
> + { 0xe3a010ff }, /* mov r1, #0xff */
> + { 0xe5821004 }, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
> + { 0, FIXUP_DSB }, /* dsb */
> + { 0xe320f003 }, /* wfi */
> + { 0xe5901000 }, /* ldr r1, [r0] */
> + { 0xe1110001 }, /* tst r1, r1 */
> + { 0x0afffffb }, /* beq <wfi> */
> + { 0xe12fff11 }, /* bx r1 */
> + { 0, FIXUP_GIC_CPU_IF },
> + { 0, FIXUP_BOOTREG },
> + { 0, FIXUP_TERMINATOR }
> };
>
> +static void write_bootloader(const char *name, hwaddr addr,
> + const ARMInsnFixup *insns, uint32_t *fixupcontext)
> +{
> + /* Fix up the specified bootloader fragment and write it into
> + * guest memory using rom_add_blob_fixed(). fixupcontext is
> + * an array giving the values to write in for the fixup types
> + * which write a value into the code array.
> + */
> + int i, len;
> + uint32_t *code;
> +
> + len = 0;
> + while (insns[len].fixup != FIXUP_TERMINATOR) {
> + len++;
> + }
> +
> + code = g_new0(uint32_t, len);
> +
> + for (i = 0; i < len; i++) {
> + uint32_t insn = insns[i].insn;
> + FixupType fixup = insns[i].fixup;
> +
> + switch (fixup) {
> + case FIXUP_NONE:
> + break;
> + case FIXUP_BOARDID:
> + case FIXUP_ARGPTR:
> + case FIXUP_ENTRYPOINT:
> + case FIXUP_GIC_CPU_IF:
> + case FIXUP_BOOTREG:
> + case FIXUP_DSB:
> + insn = fixupcontext[fixup];
> + break;
> + default:
> + abort();
> + }
> + code[i] = tswap32(insn);
> + }
> +
> + rom_add_blob_fixed(name, code, len * sizeof(uint32_t), addr);
> +
> + g_free(code);
> +}
> +
> static void default_write_secondary(ARMCPU *cpu,
> const struct arm_boot_info *info)
> {
> - int n;
> - smpboot[ARRAY_SIZE(smpboot) - 1] = info->smp_bootreg_addr;
> - smpboot[ARRAY_SIZE(smpboot) - 2] = info->gic_cpu_if_addr;
> - for (n = 0; n < ARRAY_SIZE(smpboot); n++) {
> - /* Replace DSB with the pre-v7 DSB if necessary. */
> - if (!arm_feature(&cpu->env, ARM_FEATURE_V7) &&
> - smpboot[n] == DSB_INSN) {
> - smpboot[n] = CP15_DSB_INSN;
> - }
> - smpboot[n] = tswap32(smpboot[n]);
> + uint32_t fixupcontext[FIXUP_MAX];
> +
> + fixupcontext[FIXUP_GIC_CPU_IF] = info->gic_cpu_if_addr;
> + fixupcontext[FIXUP_BOOTREG] = info->smp_bootreg_addr;
> + if (arm_feature(&cpu->env, ARM_FEATURE_V7)) {
> + fixupcontext[FIXUP_DSB] = DSB_INSN;
> + } else {
> + fixupcontext[FIXUP_DSB] = CP15_DSB_INSN;
> }
> - rom_add_blob_fixed("smpboot", smpboot, sizeof(smpboot),
> - info->smp_loader_start);
> +
> + write_bootloader("smpboot", info->smp_loader_start,
> + smpboot, fixupcontext);
> }
>
> static void default_reset_secondary(ARMCPU *cpu,
> @@ -354,7 +416,6 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
> CPUState *cs = CPU(cpu);
> int kernel_size;
> int initrd_size;
> - int n;
> int is_linux = 0;
> uint64_t elf_entry;
> hwaddr entry;
> @@ -420,6 +481,8 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
> }
> info->entry = entry;
> if (is_linux) {
> + uint32_t fixupcontext[FIXUP_MAX];
> +
> if (info->initrd_filename) {
> initrd_size = load_ramdisk(info->initrd_filename,
> info->initrd_start,
> @@ -441,7 +504,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
> }
> info->initrd_size = initrd_size;
>
> - bootloader[4] = info->board_id;
> + fixupcontext[FIXUP_BOARDID] = info->board_id;
>
> /* for device tree boot, we pass the DTB directly in r2. Otherwise
> * we point to the kernel args.
> @@ -456,9 +519,9 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
> if (load_dtb(dtb_start, info)) {
> exit(1);
> }
> - bootloader[5] = dtb_start;
> + fixupcontext[FIXUP_ARGPTR] = dtb_start;
> } else {
> - bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
> + fixupcontext[FIXUP_ARGPTR] = info->loader_start + KERNEL_ARGS_ADDR;
> if (info->ram_size >= (1ULL << 32)) {
> fprintf(stderr, "qemu: RAM size must be less than 4GB to boot"
> " Linux kernel using ATAGS (try passing a device tree"
> @@ -466,12 +529,11 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
> exit(1);
> }
> }
> - bootloader[6] = entry;
> - for (n = 0; n < sizeof(bootloader) / 4; n++) {
> - bootloader[n] = tswap32(bootloader[n]);
> - }
> - rom_add_blob_fixed("bootloader", bootloader, sizeof(bootloader),
> - info->loader_start);
> + fixupcontext[FIXUP_ENTRYPOINT] = entry;
> +
> + write_bootloader("bootloader", info->loader_start,
> + bootloader, fixupcontext);
> +
> if (info->nb_cpus > 1) {
> info->write_secondary_boot(cpu, info);
> }
> --
> 1.7.9.5
>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code
2013-12-13 3:19 ` Peter Crosthwaite
@ 2013-12-13 10:05 ` Peter Maydell
2013-12-17 0:52 ` Peter Crosthwaite
0 siblings, 1 reply; 38+ messages in thread
From: Peter Maydell @ 2013-12-13 10:05 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Alexander Graf, Mian M. Hamayun, Patch Tracking,
qemu-devel@nongnu.org Developers, Andreas Färber, kvmarm
On 13 December 2013 03:19, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> Why do we need blobs at all? Cant we just fix arm/boot to directly
> setup the CPU state to the desired? Rather than complex blobs that
> execute ARM instructions just manipulate the regs directly.
We could in theory do this for the primary bootloader, but
the secondary CPU blob has to have a loop in it so we
can sit around waiting for the guest code running in the
primary to tell us it's time to go:
>> +static const ARMInsnFixup smpboot[] = {
>> + { 0xe59f2028 }, /* ldr r2, gic_cpu_if */
>> + { 0xe59f0028 }, /* ldr r0, startaddr */
>> + { 0xe3a01001 }, /* mov r1, #1 */
>> + { 0xe5821000 }, /* str r1, [r2] - set GICC_CTLR.Enable */
>> + { 0xe3a010ff }, /* mov r1, #0xff */
>> + { 0xe5821004 }, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
>> + { 0, FIXUP_DSB }, /* dsb */
>> + { 0xe320f003 }, /* wfi */
>> + { 0xe5901000 }, /* ldr r1, [r0] */
>> + { 0xe1110001 }, /* tst r1, r1 */
>> + { 0x0afffffb }, /* beq <wfi> */
>> + { 0xe12fff11 }, /* bx r1 */
>> + { 0, FIXUP_GIC_CPU_IF },
>> + { 0, FIXUP_BOOTREG },
>> + { 0, FIXUP_TERMINATOR }
>> };
We're also writing to devices here, and it's cleaner to do that
by running a guest code instruction than by somehow having
the boot code ferret around inside the device's implementation
pre-start, I think.
thanks
-- PMM
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 1/7] target-arm/kvm: Split 32 bit only code into its own file
2013-11-28 13:33 ` [Qemu-devel] [PATCH 1/7] target-arm/kvm: Split 32 bit only code into its own file Peter Maydell
@ 2013-12-16 23:39 ` Christoffer Dall
0 siblings, 0 replies; 38+ messages in thread
From: Christoffer Dall @ 2013-12-16 23:39 UTC (permalink / raw)
To: Peter Maydell; +Cc: patches, qemu-devel, kvmarm
On Thu, Nov 28, 2013 at 01:33:16PM +0000, Peter Maydell wrote:
> Split ARM KVM support code which is 32 bit specific out into its
> own file, which we only compile on 32 bit hosts. This will give
> us a place to add the 64 bit support code without adding lots of
> ifdefs to kvm.c.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Looks nice:
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
> target-arm/Makefile.objs | 1 +
> target-arm/kvm.c | 491 -------------------------------------------
> target-arm/kvm32.c | 515 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 516 insertions(+), 491 deletions(-)
> create mode 100644 target-arm/kvm32.c
>
> diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs
> index 356fbfc..d1db77c 100644
> --- a/target-arm/Makefile.objs
> +++ b/target-arm/Makefile.objs
> @@ -6,3 +6,4 @@ obj-y += translate.o op_helper.o helper.o cpu.o
> obj-y += neon_helper.o iwmmxt_helper.o
> obj-y += gdbstub.o
> obj-$(TARGET_AARCH64) += cpu64.o translate-a64.o gdbstub64.o
> +obj-$(call land,$(CONFIG_KVM),$(call lnot,$(TARGET_AARCH64))) += kvm32.o
> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
> index f865dac..5cdb3b9 100644
> --- a/target-arm/kvm.c
> +++ b/target-arm/kvm.c
> @@ -100,120 +100,6 @@ void kvm_arm_destroy_scratch_host_vcpu(int *fdarray)
> }
> }
>
> -static inline void set_feature(uint64_t *features, int feature)
> -{
> - *features |= 1ULL << feature;
> -}
> -
> -bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
> -{
> - /* Identify the feature bits corresponding to the host CPU, and
> - * fill out the ARMHostCPUClass fields accordingly. To do this
> - * we have to create a scratch VM, create a single CPU inside it,
> - * and then query that CPU for the relevant ID registers.
> - */
> - int i, ret, fdarray[3];
> - uint32_t midr, id_pfr0, id_isar0, mvfr1;
> - uint64_t features = 0;
> - /* Old kernels may not know about the PREFERRED_TARGET ioctl: however
> - * we know these will only support creating one kind of guest CPU,
> - * which is its preferred CPU type.
> - */
> - static const uint32_t cpus_to_try[] = {
> - QEMU_KVM_ARM_TARGET_CORTEX_A15,
> - QEMU_KVM_ARM_TARGET_NONE
> - };
> - struct kvm_vcpu_init init;
> - struct kvm_one_reg idregs[] = {
> - {
> - .id = KVM_REG_ARM | KVM_REG_SIZE_U32
> - | ENCODE_CP_REG(15, 0, 0, 0, 0, 0),
> - .addr = (uintptr_t)&midr,
> - },
> - {
> - .id = KVM_REG_ARM | KVM_REG_SIZE_U32
> - | ENCODE_CP_REG(15, 0, 0, 1, 0, 0),
> - .addr = (uintptr_t)&id_pfr0,
> - },
> - {
> - .id = KVM_REG_ARM | KVM_REG_SIZE_U32
> - | ENCODE_CP_REG(15, 0, 0, 2, 0, 0),
> - .addr = (uintptr_t)&id_isar0,
> - },
> - {
> - .id = KVM_REG_ARM | KVM_REG_SIZE_U32
> - | KVM_REG_ARM_VFP | KVM_REG_ARM_VFP_MVFR1,
> - .addr = (uintptr_t)&mvfr1,
> - },
> - };
> -
> - if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) {
> - return false;
> - }
> -
> - ahcc->target = init.target;
> -
> - /* This is not strictly blessed by the device tree binding docs yet,
> - * but in practice the kernel does not care about this string so
> - * there is no point maintaining an KVM_ARM_TARGET_* -> string table.
> - */
> - ahcc->dtb_compatible = "arm,arm-v7";
> -
> - for (i = 0; i < ARRAY_SIZE(idregs); i++) {
> - ret = ioctl(fdarray[2], KVM_GET_ONE_REG, &idregs[i]);
> - if (ret) {
> - break;
> - }
> - }
> -
> - kvm_arm_destroy_scratch_host_vcpu(fdarray);
> -
> - if (ret) {
> - return false;
> - }
> -
> - /* Now we've retrieved all the register information we can
> - * set the feature bits based on the ID register fields.
> - * We can assume any KVM supporting CPU is at least a v7
> - * with VFPv3, LPAE and the generic timers; this in turn implies
> - * most of the other feature bits, but a few must be tested.
> - */
> - set_feature(&features, ARM_FEATURE_V7);
> - set_feature(&features, ARM_FEATURE_VFP3);
> - set_feature(&features, ARM_FEATURE_LPAE);
> - set_feature(&features, ARM_FEATURE_GENERIC_TIMER);
> -
> - switch (extract32(id_isar0, 24, 4)) {
> - case 1:
> - set_feature(&features, ARM_FEATURE_THUMB_DIV);
> - break;
> - case 2:
> - set_feature(&features, ARM_FEATURE_ARM_DIV);
> - set_feature(&features, ARM_FEATURE_THUMB_DIV);
> - break;
> - default:
> - break;
> - }
> -
> - if (extract32(id_pfr0, 12, 4) == 1) {
> - set_feature(&features, ARM_FEATURE_THUMB2EE);
> - }
> - if (extract32(mvfr1, 20, 4) == 1) {
> - set_feature(&features, ARM_FEATURE_VFP_FP16);
> - }
> - if (extract32(mvfr1, 12, 4) == 1) {
> - set_feature(&features, ARM_FEATURE_NEON);
> - }
> - if (extract32(mvfr1, 28, 4) == 1) {
> - /* FMAC support implies VFPv4 */
> - set_feature(&features, ARM_FEATURE_VFP4);
> - }
> -
> - ahcc->features = features;
> -
> - return true;
> -}
> -
> static void kvm_arm_host_cpu_class_init(ObjectClass *oc, void *data)
> {
> ARMHostCPUClass *ahcc = ARM_HOST_CPU_CLASS(oc);
> @@ -265,144 +151,6 @@ unsigned long kvm_arch_vcpu_id(CPUState *cpu)
> return cpu->cpu_index;
> }
>
> -static bool reg_syncs_via_tuple_list(uint64_t regidx)
> -{
> - /* Return true if the regidx is a register we should synchronize
> - * via the cpreg_tuples array (ie is not a core reg we sync by
> - * hand in kvm_arch_get/put_registers())
> - */
> - switch (regidx & KVM_REG_ARM_COPROC_MASK) {
> - case KVM_REG_ARM_CORE:
> - case KVM_REG_ARM_VFP:
> - return false;
> - default:
> - return true;
> - }
> -}
> -
> -static int compare_u64(const void *a, const void *b)
> -{
> - if (*(uint64_t *)a > *(uint64_t *)b) {
> - return 1;
> - }
> - if (*(uint64_t *)a < *(uint64_t *)b) {
> - return -1;
> - }
> - return 0;
> -}
> -
> -int kvm_arch_init_vcpu(CPUState *cs)
> -{
> - struct kvm_vcpu_init init;
> - int i, ret, arraylen;
> - uint64_t v;
> - struct kvm_one_reg r;
> - struct kvm_reg_list rl;
> - struct kvm_reg_list *rlp;
> - ARMCPU *cpu = ARM_CPU(cs);
> -
> - if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE) {
> - fprintf(stderr, "KVM is not supported for this guest CPU type\n");
> - return -EINVAL;
> - }
> -
> - init.target = cpu->kvm_target;
> - memset(init.features, 0, sizeof(init.features));
> - if (cpu->start_powered_off) {
> - init.features[0] = 1 << KVM_ARM_VCPU_POWER_OFF;
> - }
> - ret = kvm_vcpu_ioctl(cs, KVM_ARM_VCPU_INIT, &init);
> - if (ret) {
> - return ret;
> - }
> - /* Query the kernel to make sure it supports 32 VFP
> - * registers: QEMU's "cortex-a15" CPU is always a
> - * VFP-D32 core. The simplest way to do this is just
> - * to attempt to read register d31.
> - */
> - r.id = KVM_REG_ARM | KVM_REG_SIZE_U64 | KVM_REG_ARM_VFP | 31;
> - r.addr = (uintptr_t)(&v);
> - ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
> - if (ret == -ENOENT) {
> - return -EINVAL;
> - }
> -
> - /* Populate the cpreg list based on the kernel's idea
> - * of what registers exist (and throw away the TCG-created list).
> - */
> - rl.n = 0;
> - ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, &rl);
> - if (ret != -E2BIG) {
> - return ret;
> - }
> - rlp = g_malloc(sizeof(struct kvm_reg_list) + rl.n * sizeof(uint64_t));
> - rlp->n = rl.n;
> - ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, rlp);
> - if (ret) {
> - goto out;
> - }
> - /* Sort the list we get back from the kernel, since cpreg_tuples
> - * must be in strictly ascending order.
> - */
> - qsort(&rlp->reg, rlp->n, sizeof(rlp->reg[0]), compare_u64);
> -
> - for (i = 0, arraylen = 0; i < rlp->n; i++) {
> - if (!reg_syncs_via_tuple_list(rlp->reg[i])) {
> - continue;
> - }
> - switch (rlp->reg[i] & KVM_REG_SIZE_MASK) {
> - case KVM_REG_SIZE_U32:
> - case KVM_REG_SIZE_U64:
> - break;
> - default:
> - fprintf(stderr, "Can't handle size of register in kernel list\n");
> - ret = -EINVAL;
> - goto out;
> - }
> -
> - arraylen++;
> - }
> -
> - cpu->cpreg_indexes = g_renew(uint64_t, cpu->cpreg_indexes, arraylen);
> - cpu->cpreg_values = g_renew(uint64_t, cpu->cpreg_values, arraylen);
> - cpu->cpreg_vmstate_indexes = g_renew(uint64_t, cpu->cpreg_vmstate_indexes,
> - arraylen);
> - cpu->cpreg_vmstate_values = g_renew(uint64_t, cpu->cpreg_vmstate_values,
> - arraylen);
> - cpu->cpreg_array_len = arraylen;
> - cpu->cpreg_vmstate_array_len = arraylen;
> -
> - for (i = 0, arraylen = 0; i < rlp->n; i++) {
> - uint64_t regidx = rlp->reg[i];
> - if (!reg_syncs_via_tuple_list(regidx)) {
> - continue;
> - }
> - cpu->cpreg_indexes[arraylen] = regidx;
> - arraylen++;
> - }
> - assert(cpu->cpreg_array_len == arraylen);
> -
> - if (!write_kvmstate_to_list(cpu)) {
> - /* Shouldn't happen unless kernel is inconsistent about
> - * what registers exist.
> - */
> - fprintf(stderr, "Initial read of kernel register state failed\n");
> - ret = -EINVAL;
> - goto out;
> - }
> -
> - /* Save a copy of the initial register values so that we can
> - * feed it back to the kernel on VCPU reset.
> - */
> - cpu->cpreg_reset_values = g_memdup(cpu->cpreg_values,
> - cpu->cpreg_array_len *
> - sizeof(cpu->cpreg_values[0]));
> -
> -out:
> - g_free(rlp);
> - return ret;
> -}
> -
> /* We track all the KVM devices which need their memory addresses
> * passing to the kernel in a list of these structures.
> * When board init is complete we run through the list and
> @@ -563,232 +311,6 @@ bool write_list_to_kvmstate(ARMCPU *cpu)
> return ok;
> }
>
> -typedef struct Reg {
> - uint64_t id;
> - int offset;
> -} Reg;
> -
> -#define COREREG(KERNELNAME, QEMUFIELD) \
> - { \
> - KVM_REG_ARM | KVM_REG_SIZE_U32 | \
> - KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(KERNELNAME), \
> - offsetof(CPUARMState, QEMUFIELD) \
> - }
> -
> -#define VFPSYSREG(R) \
> - { \
> - KVM_REG_ARM | KVM_REG_SIZE_U32 | KVM_REG_ARM_VFP | \
> - KVM_REG_ARM_VFP_##R, \
> - offsetof(CPUARMState, vfp.xregs[ARM_VFP_##R]) \
> - }
> -
> -static const Reg regs[] = {
> - /* R0_usr .. R14_usr */
> - COREREG(usr_regs.uregs[0], regs[0]),
> - COREREG(usr_regs.uregs[1], regs[1]),
> - COREREG(usr_regs.uregs[2], regs[2]),
> - COREREG(usr_regs.uregs[3], regs[3]),
> - COREREG(usr_regs.uregs[4], regs[4]),
> - COREREG(usr_regs.uregs[5], regs[5]),
> - COREREG(usr_regs.uregs[6], regs[6]),
> - COREREG(usr_regs.uregs[7], regs[7]),
> - COREREG(usr_regs.uregs[8], usr_regs[0]),
> - COREREG(usr_regs.uregs[9], usr_regs[1]),
> - COREREG(usr_regs.uregs[10], usr_regs[2]),
> - COREREG(usr_regs.uregs[11], usr_regs[3]),
> - COREREG(usr_regs.uregs[12], usr_regs[4]),
> - COREREG(usr_regs.uregs[13], banked_r13[0]),
> - COREREG(usr_regs.uregs[14], banked_r14[0]),
> - /* R13, R14, SPSR for SVC, ABT, UND, IRQ banks */
> - COREREG(svc_regs[0], banked_r13[1]),
> - COREREG(svc_regs[1], banked_r14[1]),
> - COREREG(svc_regs[2], banked_spsr[1]),
> - COREREG(abt_regs[0], banked_r13[2]),
> - COREREG(abt_regs[1], banked_r14[2]),
> - COREREG(abt_regs[2], banked_spsr[2]),
> - COREREG(und_regs[0], banked_r13[3]),
> - COREREG(und_regs[1], banked_r14[3]),
> - COREREG(und_regs[2], banked_spsr[3]),
> - COREREG(irq_regs[0], banked_r13[4]),
> - COREREG(irq_regs[1], banked_r14[4]),
> - COREREG(irq_regs[2], banked_spsr[4]),
> - /* R8_fiq .. R14_fiq and SPSR_fiq */
> - COREREG(fiq_regs[0], fiq_regs[0]),
> - COREREG(fiq_regs[1], fiq_regs[1]),
> - COREREG(fiq_regs[2], fiq_regs[2]),
> - COREREG(fiq_regs[3], fiq_regs[3]),
> - COREREG(fiq_regs[4], fiq_regs[4]),
> - COREREG(fiq_regs[5], banked_r13[5]),
> - COREREG(fiq_regs[6], banked_r14[5]),
> - COREREG(fiq_regs[7], banked_spsr[5]),
> - /* R15 */
> - COREREG(usr_regs.uregs[15], regs[15]),
> - /* VFP system registers */
> - VFPSYSREG(FPSID),
> - VFPSYSREG(MVFR1),
> - VFPSYSREG(MVFR0),
> - VFPSYSREG(FPEXC),
> - VFPSYSREG(FPINST),
> - VFPSYSREG(FPINST2),
> -};
> -
> -int kvm_arch_put_registers(CPUState *cs, int level)
> -{
> - ARMCPU *cpu = ARM_CPU(cs);
> - CPUARMState *env = &cpu->env;
> - struct kvm_one_reg r;
> - int mode, bn;
> - int ret, i;
> - uint32_t cpsr, fpscr;
> -
> - /* Make sure the banked regs are properly set */
> - mode = env->uncached_cpsr & CPSR_M;
> - bn = bank_number(mode);
> - if (mode == ARM_CPU_MODE_FIQ) {
> - memcpy(env->fiq_regs, env->regs + 8, 5 * sizeof(uint32_t));
> - } else {
> - memcpy(env->usr_regs, env->regs + 8, 5 * sizeof(uint32_t));
> - }
> - env->banked_r13[bn] = env->regs[13];
> - env->banked_r14[bn] = env->regs[14];
> - env->banked_spsr[bn] = env->spsr;
> -
> - /* Now we can safely copy stuff down to the kernel */
> - for (i = 0; i < ARRAY_SIZE(regs); i++) {
> - r.id = regs[i].id;
> - r.addr = (uintptr_t)(env) + regs[i].offset;
> - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
> - if (ret) {
> - return ret;
> - }
> - }
> -
> - /* Special cases which aren't a single CPUARMState field */
> - cpsr = cpsr_read(env);
> - r.id = KVM_REG_ARM | KVM_REG_SIZE_U32 |
> - KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(usr_regs.ARM_cpsr);
> - r.addr = (uintptr_t)(&cpsr);
> - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
> - if (ret) {
> - return ret;
> - }
> -
> - /* VFP registers */
> - r.id = KVM_REG_ARM | KVM_REG_SIZE_U64 | KVM_REG_ARM_VFP;
> - for (i = 0; i < 32; i++) {
> - r.addr = (uintptr_t)(&env->vfp.regs[i]);
> - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
> - if (ret) {
> - return ret;
> - }
> - r.id++;
> - }
> -
> - r.id = KVM_REG_ARM | KVM_REG_SIZE_U32 | KVM_REG_ARM_VFP |
> - KVM_REG_ARM_VFP_FPSCR;
> - fpscr = vfp_get_fpscr(env);
> - r.addr = (uintptr_t)&fpscr;
> - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
> - if (ret) {
> - return ret;
> - }
> -
> - /* Note that we do not call write_cpustate_to_list()
> - * here, so we are only writing the tuple list back to
> - * KVM. This is safe because nothing can change the
> - * CPUARMState cp15 fields (in particular gdb accesses cannot)
> - * and so there are no changes to sync. In fact syncing would
> - * be wrong at this point: for a constant register where TCG and
> - * KVM disagree about its value, the preceding write_list_to_cpustate()
> - * would not have had any effect on the CPUARMState value (since the
> - * register is read-only), and a write_cpustate_to_list() here would
> - * then try to write the TCG value back into KVM -- this would either
> - * fail or incorrectly change the value the guest sees.
> - *
> - * If we ever want to allow the user to modify cp15 registers via
> - * the gdb stub, we would need to be more clever here (for instance
> - * tracking the set of registers kvm_arch_get_registers() successfully
> - * managed to update the CPUARMState with, and only allowing those
> - * to be written back up into the kernel).
> - */
> - if (!write_list_to_kvmstate(cpu)) {
> - return EINVAL;
> - }
> -
> - return ret;
> -}
> -
> -int kvm_arch_get_registers(CPUState *cs)
> -{
> - ARMCPU *cpu = ARM_CPU(cs);
> - CPUARMState *env = &cpu->env;
> - struct kvm_one_reg r;
> - int mode, bn;
> - int ret, i;
> - uint32_t cpsr, fpscr;
> -
> - for (i = 0; i < ARRAY_SIZE(regs); i++) {
> - r.id = regs[i].id;
> - r.addr = (uintptr_t)(env) + regs[i].offset;
> - ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
> - if (ret) {
> - return ret;
> - }
> - }
> -
> - /* Special cases which aren't a single CPUARMState field */
> - r.id = KVM_REG_ARM | KVM_REG_SIZE_U32 |
> - KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(usr_regs.ARM_cpsr);
> - r.addr = (uintptr_t)(&cpsr);
> - ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
> - if (ret) {
> - return ret;
> - }
> - cpsr_write(env, cpsr, 0xffffffff);
> -
> - /* Make sure the current mode regs are properly set */
> - mode = env->uncached_cpsr & CPSR_M;
> - bn = bank_number(mode);
> - if (mode == ARM_CPU_MODE_FIQ) {
> - memcpy(env->regs + 8, env->fiq_regs, 5 * sizeof(uint32_t));
> - } else {
> - memcpy(env->regs + 8, env->usr_regs, 5 * sizeof(uint32_t));
> - }
> - env->regs[13] = env->banked_r13[bn];
> - env->regs[14] = env->banked_r14[bn];
> - env->spsr = env->banked_spsr[bn];
> -
> - /* VFP registers */
> - r.id = KVM_REG_ARM | KVM_REG_SIZE_U64 | KVM_REG_ARM_VFP;
> - for (i = 0; i < 32; i++) {
> - r.addr = (uintptr_t)(&env->vfp.regs[i]);
> - ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
> - if (ret) {
> - return ret;
> - }
> - r.id++;
> - }
> -
> - r.id = KVM_REG_ARM | KVM_REG_SIZE_U32 | KVM_REG_ARM_VFP |
> - KVM_REG_ARM_VFP_FPSCR;
> - r.addr = (uintptr_t)&fpscr;
> - ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
> - if (ret) {
> - return ret;
> - }
> - vfp_set_fpscr(env, fpscr);
> -
> - if (!write_kvmstate_to_list(cpu)) {
> - return EINVAL;
> - }
> - /* Note that it's OK to have registers which aren't in CPUState,
> - * so we can ignore a failure return here.
> - */
> - write_list_to_cpustate(cpu);
> -
> - return 0;
> -}
> -
> void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run)
> {
> }
> @@ -802,19 +324,6 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
> return 0;
> }
>
> -void kvm_arch_reset_vcpu(CPUState *cs)
> -{
> - /* Feed the kernel back its initial register state */
> - ARMCPU *cpu = ARM_CPU(cs);
> -
> - memmove(cpu->cpreg_values, cpu->cpreg_reset_values,
> - cpu->cpreg_array_len * sizeof(cpu->cpreg_values[0]));
> -
> - if (!write_list_to_kvmstate(cpu)) {
> - abort();
> - }
> -}
> -
> bool kvm_arch_stop_on_emulation_error(CPUState *cs)
> {
> return true;
> diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c
> new file mode 100644
> index 0000000..a4fde07
> --- /dev/null
> +++ b/target-arm/kvm32.c
> @@ -0,0 +1,515 @@
> +/*
> + * ARM implementation of KVM hooks, 32 bit specific code.
> + *
> + * Copyright Christoffer Dall 2009-2010
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include <stdio.h>
> +#include <sys/types.h>
> +#include <sys/ioctl.h>
> +#include <sys/mman.h>
> +
> +#include <linux/kvm.h>
> +
> +#include "qemu-common.h"
> +#include "qemu/timer.h"
> +#include "sysemu/sysemu.h"
> +#include "sysemu/kvm.h"
> +#include "kvm_arm.h"
> +#include "cpu.h"
> +#include "hw/arm/arm.h"
> +
> +static inline void set_feature(uint64_t *features, int feature)
> +{
> + *features |= 1ULL << feature;
> +}
> +
> +bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
> +{
> + /* Identify the feature bits corresponding to the host CPU, and
> + * fill out the ARMHostCPUClass fields accordingly. To do this
> + * we have to create a scratch VM, create a single CPU inside it,
> + * and then query that CPU for the relevant ID registers.
> + */
> + int i, ret, fdarray[3];
> + uint32_t midr, id_pfr0, id_isar0, mvfr1;
> + uint64_t features = 0;
> + /* Old kernels may not know about the PREFERRED_TARGET ioctl: however
> + * we know these will only support creating one kind of guest CPU,
> + * which is its preferred CPU type.
> + */
> + static const uint32_t cpus_to_try[] = {
> + QEMU_KVM_ARM_TARGET_CORTEX_A15,
> + QEMU_KVM_ARM_TARGET_NONE
> + };
> + struct kvm_vcpu_init init;
> + struct kvm_one_reg idregs[] = {
> + {
> + .id = KVM_REG_ARM | KVM_REG_SIZE_U32
> + | ENCODE_CP_REG(15, 0, 0, 0, 0, 0),
> + .addr = (uintptr_t)&midr,
> + },
> + {
> + .id = KVM_REG_ARM | KVM_REG_SIZE_U32
> + | ENCODE_CP_REG(15, 0, 0, 1, 0, 0),
> + .addr = (uintptr_t)&id_pfr0,
> + },
> + {
> + .id = KVM_REG_ARM | KVM_REG_SIZE_U32
> + | ENCODE_CP_REG(15, 0, 0, 2, 0, 0),
> + .addr = (uintptr_t)&id_isar0,
> + },
> + {
> + .id = KVM_REG_ARM | KVM_REG_SIZE_U32
> + | KVM_REG_ARM_VFP | KVM_REG_ARM_VFP_MVFR1,
> + .addr = (uintptr_t)&mvfr1,
> + },
> + };
> +
> + if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) {
> + return false;
> + }
> +
> + ahcc->target = init.target;
> +
> + /* This is not strictly blessed by the device tree binding docs yet,
> + * but in practice the kernel does not care about this string so
> + * there is no point maintaining an KVM_ARM_TARGET_* -> string table.
> + */
> + ahcc->dtb_compatible = "arm,arm-v7";
> +
> + for (i = 0; i < ARRAY_SIZE(idregs); i++) {
> + ret = ioctl(fdarray[2], KVM_GET_ONE_REG, &idregs[i]);
> + if (ret) {
> + break;
> + }
> + }
> +
> + kvm_arm_destroy_scratch_host_vcpu(fdarray);
> +
> + if (ret) {
> + return false;
> + }
> +
> + /* Now we've retrieved all the register information we can
> + * set the feature bits based on the ID register fields.
> + * We can assume any KVM supporting CPU is at least a v7
> + * with VFPv3, LPAE and the generic timers; this in turn implies
> + * most of the other feature bits, but a few must be tested.
> + */
> + set_feature(&features, ARM_FEATURE_V7);
> + set_feature(&features, ARM_FEATURE_VFP3);
> + set_feature(&features, ARM_FEATURE_LPAE);
> + set_feature(&features, ARM_FEATURE_GENERIC_TIMER);
> +
> + switch (extract32(id_isar0, 24, 4)) {
> + case 1:
> + set_feature(&features, ARM_FEATURE_THUMB_DIV);
> + break;
> + case 2:
> + set_feature(&features, ARM_FEATURE_ARM_DIV);
> + set_feature(&features, ARM_FEATURE_THUMB_DIV);
> + break;
> + default:
> + break;
> + }
> +
> + if (extract32(id_pfr0, 12, 4) == 1) {
> + set_feature(&features, ARM_FEATURE_THUMB2EE);
> + }
> + if (extract32(mvfr1, 20, 4) == 1) {
> + set_feature(&features, ARM_FEATURE_VFP_FP16);
> + }
> + if (extract32(mvfr1, 12, 4) == 1) {
> + set_feature(&features, ARM_FEATURE_NEON);
> + }
> + if (extract32(mvfr1, 28, 4) == 1) {
> + /* FMAC support implies VFPv4 */
> + set_feature(&features, ARM_FEATURE_VFP4);
> + }
> +
> + ahcc->features = features;
> +
> + return true;
> +}
> +
> +static bool reg_syncs_via_tuple_list(uint64_t regidx)
> +{
> + /* Return true if the regidx is a register we should synchronize
> + * via the cpreg_tuples array (ie is not a core reg we sync by
> + * hand in kvm_arch_get/put_registers())
> + */
> + switch (regidx & KVM_REG_ARM_COPROC_MASK) {
> + case KVM_REG_ARM_CORE:
> + case KVM_REG_ARM_VFP:
> + return false;
> + default:
> + return true;
> + }
> +}
> +
> +static int compare_u64(const void *a, const void *b)
> +{
> + if (*(uint64_t *)a > *(uint64_t *)b) {
> + return 1;
> + }
> + if (*(uint64_t *)a < *(uint64_t *)b) {
> + return -1;
> + }
> + return 0;
> +}
> +
> +int kvm_arch_init_vcpu(CPUState *cs)
> +{
> + struct kvm_vcpu_init init;
> + int i, ret, arraylen;
> + uint64_t v;
> + struct kvm_one_reg r;
> + struct kvm_reg_list rl;
> + struct kvm_reg_list *rlp;
> + ARMCPU *cpu = ARM_CPU(cs);
> +
> + if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE) {
> + fprintf(stderr, "KVM is not supported for this guest CPU type\n");
> + return -EINVAL;
> + }
> +
> + init.target = cpu->kvm_target;
> + memset(init.features, 0, sizeof(init.features));
> + if (cpu->start_powered_off) {
> + init.features[0] = 1 << KVM_ARM_VCPU_POWER_OFF;
> + }
> + ret = kvm_vcpu_ioctl(cs, KVM_ARM_VCPU_INIT, &init);
> + if (ret) {
> + return ret;
> + }
> + /* Query the kernel to make sure it supports 32 VFP
> + * registers: QEMU's "cortex-a15" CPU is always a
> + * VFP-D32 core. The simplest way to do this is just
> + * to attempt to read register d31.
> + */
> + r.id = KVM_REG_ARM | KVM_REG_SIZE_U64 | KVM_REG_ARM_VFP | 31;
> + r.addr = (uintptr_t)(&v);
> + ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
> + if (ret == -ENOENT) {
> + return -EINVAL;
> + }
> +
> + /* Populate the cpreg list based on the kernel's idea
> + * of what registers exist (and throw away the TCG-created list).
> + */
> + rl.n = 0;
> + ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, &rl);
> + if (ret != -E2BIG) {
> + return ret;
> + }
> + rlp = g_malloc(sizeof(struct kvm_reg_list) + rl.n * sizeof(uint64_t));
> + rlp->n = rl.n;
> + ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, rlp);
> + if (ret) {
> + goto out;
> + }
> + /* Sort the list we get back from the kernel, since cpreg_tuples
> + * must be in strictly ascending order.
> + */
> + qsort(&rlp->reg, rlp->n, sizeof(rlp->reg[0]), compare_u64);
> +
> + for (i = 0, arraylen = 0; i < rlp->n; i++) {
> + if (!reg_syncs_via_tuple_list(rlp->reg[i])) {
> + continue;
> + }
> + switch (rlp->reg[i] & KVM_REG_SIZE_MASK) {
> + case KVM_REG_SIZE_U32:
> + case KVM_REG_SIZE_U64:
> + break;
> + default:
> + fprintf(stderr, "Can't handle size of register in kernel list\n");
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + arraylen++;
> + }
> +
> + cpu->cpreg_indexes = g_renew(uint64_t, cpu->cpreg_indexes, arraylen);
> + cpu->cpreg_values = g_renew(uint64_t, cpu->cpreg_values, arraylen);
> + cpu->cpreg_vmstate_indexes = g_renew(uint64_t, cpu->cpreg_vmstate_indexes,
> + arraylen);
> + cpu->cpreg_vmstate_values = g_renew(uint64_t, cpu->cpreg_vmstate_values,
> + arraylen);
> + cpu->cpreg_array_len = arraylen;
> + cpu->cpreg_vmstate_array_len = arraylen;
> +
> + for (i = 0, arraylen = 0; i < rlp->n; i++) {
> + uint64_t regidx = rlp->reg[i];
> + if (!reg_syncs_via_tuple_list(regidx)) {
> + continue;
> + }
> + cpu->cpreg_indexes[arraylen] = regidx;
> + arraylen++;
> + }
> + assert(cpu->cpreg_array_len == arraylen);
> +
> + if (!write_kvmstate_to_list(cpu)) {
> + /* Shouldn't happen unless kernel is inconsistent about
> + * what registers exist.
> + */
> + fprintf(stderr, "Initial read of kernel register state failed\n");
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + /* Save a copy of the initial register values so that we can
> + * feed it back to the kernel on VCPU reset.
> + */
> + cpu->cpreg_reset_values = g_memdup(cpu->cpreg_values,
> + cpu->cpreg_array_len *
> + sizeof(cpu->cpreg_values[0]));
> +
> +out:
> + g_free(rlp);
> + return ret;
> +}
> +
> +typedef struct Reg {
> + uint64_t id;
> + int offset;
> +} Reg;
> +
> +#define COREREG(KERNELNAME, QEMUFIELD) \
> + { \
> + KVM_REG_ARM | KVM_REG_SIZE_U32 | \
> + KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(KERNELNAME), \
> + offsetof(CPUARMState, QEMUFIELD) \
> + }
> +
> +#define VFPSYSREG(R) \
> + { \
> + KVM_REG_ARM | KVM_REG_SIZE_U32 | KVM_REG_ARM_VFP | \
> + KVM_REG_ARM_VFP_##R, \
> + offsetof(CPUARMState, vfp.xregs[ARM_VFP_##R]) \
> + }
> +
> +static const Reg regs[] = {
> + /* R0_usr .. R14_usr */
> + COREREG(usr_regs.uregs[0], regs[0]),
> + COREREG(usr_regs.uregs[1], regs[1]),
> + COREREG(usr_regs.uregs[2], regs[2]),
> + COREREG(usr_regs.uregs[3], regs[3]),
> + COREREG(usr_regs.uregs[4], regs[4]),
> + COREREG(usr_regs.uregs[5], regs[5]),
> + COREREG(usr_regs.uregs[6], regs[6]),
> + COREREG(usr_regs.uregs[7], regs[7]),
> + COREREG(usr_regs.uregs[8], usr_regs[0]),
> + COREREG(usr_regs.uregs[9], usr_regs[1]),
> + COREREG(usr_regs.uregs[10], usr_regs[2]),
> + COREREG(usr_regs.uregs[11], usr_regs[3]),
> + COREREG(usr_regs.uregs[12], usr_regs[4]),
> + COREREG(usr_regs.uregs[13], banked_r13[0]),
> + COREREG(usr_regs.uregs[14], banked_r14[0]),
> + /* R13, R14, SPSR for SVC, ABT, UND, IRQ banks */
> + COREREG(svc_regs[0], banked_r13[1]),
> + COREREG(svc_regs[1], banked_r14[1]),
> + COREREG(svc_regs[2], banked_spsr[1]),
> + COREREG(abt_regs[0], banked_r13[2]),
> + COREREG(abt_regs[1], banked_r14[2]),
> + COREREG(abt_regs[2], banked_spsr[2]),
> + COREREG(und_regs[0], banked_r13[3]),
> + COREREG(und_regs[1], banked_r14[3]),
> + COREREG(und_regs[2], banked_spsr[3]),
> + COREREG(irq_regs[0], banked_r13[4]),
> + COREREG(irq_regs[1], banked_r14[4]),
> + COREREG(irq_regs[2], banked_spsr[4]),
> + /* R8_fiq .. R14_fiq and SPSR_fiq */
> + COREREG(fiq_regs[0], fiq_regs[0]),
> + COREREG(fiq_regs[1], fiq_regs[1]),
> + COREREG(fiq_regs[2], fiq_regs[2]),
> + COREREG(fiq_regs[3], fiq_regs[3]),
> + COREREG(fiq_regs[4], fiq_regs[4]),
> + COREREG(fiq_regs[5], banked_r13[5]),
> + COREREG(fiq_regs[6], banked_r14[5]),
> + COREREG(fiq_regs[7], banked_spsr[5]),
> + /* R15 */
> + COREREG(usr_regs.uregs[15], regs[15]),
> + /* VFP system registers */
> + VFPSYSREG(FPSID),
> + VFPSYSREG(MVFR1),
> + VFPSYSREG(MVFR0),
> + VFPSYSREG(FPEXC),
> + VFPSYSREG(FPINST),
> + VFPSYSREG(FPINST2),
> +};
> +
> +int kvm_arch_put_registers(CPUState *cs, int level)
> +{
> + ARMCPU *cpu = ARM_CPU(cs);
> + CPUARMState *env = &cpu->env;
> + struct kvm_one_reg r;
> + int mode, bn;
> + int ret, i;
> + uint32_t cpsr, fpscr;
> +
> + /* Make sure the banked regs are properly set */
> + mode = env->uncached_cpsr & CPSR_M;
> + bn = bank_number(mode);
> + if (mode == ARM_CPU_MODE_FIQ) {
> + memcpy(env->fiq_regs, env->regs + 8, 5 * sizeof(uint32_t));
> + } else {
> + memcpy(env->usr_regs, env->regs + 8, 5 * sizeof(uint32_t));
> + }
> + env->banked_r13[bn] = env->regs[13];
> + env->banked_r14[bn] = env->regs[14];
> + env->banked_spsr[bn] = env->spsr;
> +
> + /* Now we can safely copy stuff down to the kernel */
> + for (i = 0; i < ARRAY_SIZE(regs); i++) {
> + r.id = regs[i].id;
> + r.addr = (uintptr_t)(env) + regs[i].offset;
> + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
> + if (ret) {
> + return ret;
> + }
> + }
> +
> + /* Special cases which aren't a single CPUARMState field */
> + cpsr = cpsr_read(env);
> + r.id = KVM_REG_ARM | KVM_REG_SIZE_U32 |
> + KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(usr_regs.ARM_cpsr);
> + r.addr = (uintptr_t)(&cpsr);
> + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
> + if (ret) {
> + return ret;
> + }
> +
> + /* VFP registers */
> + r.id = KVM_REG_ARM | KVM_REG_SIZE_U64 | KVM_REG_ARM_VFP;
> + for (i = 0; i < 32; i++) {
> + r.addr = (uintptr_t)(&env->vfp.regs[i]);
> + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
> + if (ret) {
> + return ret;
> + }
> + r.id++;
> + }
> +
> + r.id = KVM_REG_ARM | KVM_REG_SIZE_U32 | KVM_REG_ARM_VFP |
> + KVM_REG_ARM_VFP_FPSCR;
> + fpscr = vfp_get_fpscr(env);
> + r.addr = (uintptr_t)&fpscr;
> + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
> + if (ret) {
> + return ret;
> + }
> +
> + /* Note that we do not call write_cpustate_to_list()
> + * here, so we are only writing the tuple list back to
> + * KVM. This is safe because nothing can change the
> + * CPUARMState cp15 fields (in particular gdb accesses cannot)
> + * and so there are no changes to sync. In fact syncing would
> + * be wrong at this point: for a constant register where TCG and
> + * KVM disagree about its value, the preceding write_list_to_cpustate()
> + * would not have had any effect on the CPUARMState value (since the
> + * register is read-only), and a write_cpustate_to_list() here would
> + * then try to write the TCG value back into KVM -- this would either
> + * fail or incorrectly change the value the guest sees.
> + *
> + * If we ever want to allow the user to modify cp15 registers via
> + * the gdb stub, we would need to be more clever here (for instance
> + * tracking the set of registers kvm_arch_get_registers() successfully
> + * managed to update the CPUARMState with, and only allowing those
> + * to be written back up into the kernel).
> + */
> + if (!write_list_to_kvmstate(cpu)) {
> + return EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +int kvm_arch_get_registers(CPUState *cs)
> +{
> + ARMCPU *cpu = ARM_CPU(cs);
> + CPUARMState *env = &cpu->env;
> + struct kvm_one_reg r;
> + int mode, bn;
> + int ret, i;
> + uint32_t cpsr, fpscr;
> +
> + for (i = 0; i < ARRAY_SIZE(regs); i++) {
> + r.id = regs[i].id;
> + r.addr = (uintptr_t)(env) + regs[i].offset;
> + ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
> + if (ret) {
> + return ret;
> + }
> + }
> +
> + /* Special cases which aren't a single CPUARMState field */
> + r.id = KVM_REG_ARM | KVM_REG_SIZE_U32 |
> + KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(usr_regs.ARM_cpsr);
> + r.addr = (uintptr_t)(&cpsr);
> + ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
> + if (ret) {
> + return ret;
> + }
> + cpsr_write(env, cpsr, 0xffffffff);
> +
> + /* Make sure the current mode regs are properly set */
> + mode = env->uncached_cpsr & CPSR_M;
> + bn = bank_number(mode);
> + if (mode == ARM_CPU_MODE_FIQ) {
> + memcpy(env->regs + 8, env->fiq_regs, 5 * sizeof(uint32_t));
> + } else {
> + memcpy(env->regs + 8, env->usr_regs, 5 * sizeof(uint32_t));
> + }
> + env->regs[13] = env->banked_r13[bn];
> + env->regs[14] = env->banked_r14[bn];
> + env->spsr = env->banked_spsr[bn];
> +
> + /* VFP registers */
> + r.id = KVM_REG_ARM | KVM_REG_SIZE_U64 | KVM_REG_ARM_VFP;
> + for (i = 0; i < 32; i++) {
> + r.addr = (uintptr_t)(&env->vfp.regs[i]);
> + ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
> + if (ret) {
> + return ret;
> + }
> + r.id++;
> + }
> +
> + r.id = KVM_REG_ARM | KVM_REG_SIZE_U32 | KVM_REG_ARM_VFP |
> + KVM_REG_ARM_VFP_FPSCR;
> + r.addr = (uintptr_t)&fpscr;
> + ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
> + if (ret) {
> + return ret;
> + }
> + vfp_set_fpscr(env, fpscr);
> +
> + if (!write_kvmstate_to_list(cpu)) {
> + return EINVAL;
> + }
> + /* Note that it's OK to have registers which aren't in CPUState,
> + * so we can ignore a failure return here.
> + */
> + write_list_to_cpustate(cpu);
> +
> + return 0;
> +}
> +
> +void kvm_arch_reset_vcpu(CPUState *cs)
> +{
> + /* Feed the kernel back its initial register state */
> + ARMCPU *cpu = ARM_CPU(cs);
> +
> + memmove(cpu->cpreg_values, cpu->cpreg_reset_values,
> + cpu->cpreg_array_len * sizeof(cpu->cpreg_values[0]));
> +
> + if (!write_list_to_kvmstate(cpu)) {
> + abort();
> + }
> +}
> --
> 1.7.9.5
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
--
Christoffer
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 2/7] target-arm: Clean up handling of AArch64 PSTATE
2013-11-28 13:33 ` [Qemu-devel] [PATCH 2/7] target-arm: Clean up handling of AArch64 PSTATE Peter Maydell
@ 2013-12-16 23:39 ` Christoffer Dall
2013-12-17 0:15 ` Peter Maydell
0 siblings, 1 reply; 38+ messages in thread
From: Christoffer Dall @ 2013-12-16 23:39 UTC (permalink / raw)
To: Peter Maydell; +Cc: Patch Tracking, QEMU Developers, kvmarm
On Thu, Nov 28, 2013 at 01:33:17PM +0000, Peter Maydell wrote:
> The env->pstate field is a little odd since it doesn't strictly
> speaking represent an architectural register. However it's convenient
> for QEMU to use it to hold the various PSTATE architectural bits
> in the same format the architecture specifies for SPSR registers
> (since this is the same format the kernel uses for signal handlers
> and the KVM register). Add some structure to how we deal with it:
> * document what env->pstate is
> * add some #defines for various bits in it
> * add helpers for reading/writing it taking account of caching
> of NZCV, and use them where appropriate
> * reset it on startup
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> linux-user/signal.c | 6 ++--
> target-arm/cpu.c | 6 ++++
> target-arm/cpu.h | 68 +++++++++++++++++++++++++++++++++++++-------
> target-arm/gdbstub64.c | 4 +--
> target-arm/translate-a64.c | 12 ++++----
> 5 files changed, 76 insertions(+), 20 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 7751c47..4e7148a 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -1171,7 +1171,7 @@ static int target_setup_sigframe(struct target_rt_sigframe *sf,
> }
> __put_user(env->xregs[31], &sf->uc.tuc_mcontext.sp);
> __put_user(env->pc, &sf->uc.tuc_mcontext.pc);
> - __put_user(env->pstate, &sf->uc.tuc_mcontext.pstate);
> + __put_user(pstate_read(env), &sf->uc.tuc_mcontext.pstate);
>
> __put_user(/*current->thread.fault_address*/ 0,
> &sf->uc.tuc_mcontext.fault_address);
> @@ -1210,6 +1210,7 @@ static int target_restore_sigframe(CPUARMState *env,
> struct target_aux_context *aux =
> (struct target_aux_context *)sf->uc.tuc_mcontext.__reserved;
> uint32_t magic, size;
> + uint64_t pstate;
>
> target_to_host_sigset(&set, &sf->uc.tuc_sigmask);
> sigprocmask(SIG_SETMASK, &set, NULL);
> @@ -1220,7 +1221,8 @@ static int target_restore_sigframe(CPUARMState *env,
>
> __get_user(env->xregs[31], &sf->uc.tuc_mcontext.sp);
> __get_user(env->pc, &sf->uc.tuc_mcontext.pc);
> - __get_user(env->pstate, &sf->uc.tuc_mcontext.pstate);
> + __get_user(pstate, &sf->uc.tuc_mcontext.pstate);
> + pstate_write(env, pstate);
>
> __get_user(magic, &aux->fpsimd.head.magic);
> __get_user(size, &aux->fpsimd.head.size);
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 0635e78..42057ad 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -88,6 +88,12 @@ static void arm_cpu_reset(CPUState *s)
> if (arm_feature(env, ARM_FEATURE_AARCH64)) {
> /* 64 bit CPUs always start in 64 bit mode */
> env->aarch64 = 1;
> +#if defined(CONFIG_USER_ONLY)
> + env->pstate = PSTATE_MODE_EL0t;
> +#else
> + env->pstate = PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F
> + | PSTATE_MODE_EL1h;
> +#endif
> }
>
> #if defined(CONFIG_USER_ONLY)
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index c3f007f..ff7aac5 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -113,8 +113,13 @@ typedef struct CPUARMState {
> /* Regs for A64 mode. */
> uint64_t xregs[32];
> uint64_t pc;
> - /* TODO: pstate doesn't correspond to an architectural register;
> - * it would be better modelled as the underlying fields.
> + /* PSTATE isn't an architectural register for ARMv8. However, it is
> + * convenient for us to assemble the underlying state into a 32 bit format
> + * identical to the architectural format used for the SPSR. (This is also
> + * what the Linux kernel's 'pstate' field in signal handlers and KVM's
> + * 'pstate' register are.) NZCV are kept in their split fields; aarch64
> + * is an inverted split version of PSTATE.nRW (aka M[4]); other bits are
> + * stored here when in AArch64.
I really don't understand what you are trying to say beginning with
aarch64 is an inverted split version... Which other bits are stored
where in AArch64?
Also what is the rationale behind keeping NZCV in their split fields?
> */
> uint32_t pstate;
> uint32_t aarch64; /* 1 if CPU is in aarch64 state; inverse of PSTATE.nRW */
> @@ -309,15 +314,6 @@ static inline bool is_a64(CPUARMState *env)
> return env->aarch64;
> }
>
> -#define PSTATE_N_SHIFT 3
> -#define PSTATE_N (1 << PSTATE_N_SHIFT)
> -#define PSTATE_Z_SHIFT 2
> -#define PSTATE_Z (1 << PSTATE_Z_SHIFT)
> -#define PSTATE_C_SHIFT 1
> -#define PSTATE_C (1 << PSTATE_C_SHIFT)
> -#define PSTATE_V_SHIFT 0
> -#define PSTATE_V (1 << PSTATE_V_SHIFT)
> -
> /* you can call this signal handler from your SIGBUS and SIGSEGV
> signal handlers to inform the virtual CPU of exceptions. non zero
> is returned if the signal was handled by the virtual CPU. */
> @@ -352,6 +348,56 @@ int cpu_arm_handle_mmu_fault (CPUARMState *env, target_ulong address, int rw,
> /* Execution state bits. MRS read as zero, MSR writes ignored. */
> #define CPSR_EXEC (CPSR_T | CPSR_IT | CPSR_J)
>
> +/* Bit definitions for ARMv8 SPSR (PSTATE) format.
> + * Only these are valid when in AArch64 mode; in
> + * AArch32 mode SPSRs are basically CPSR-format.
> + */
> +#define PSTATE_M (0xFU)
> +#define PSTATE_nRW (1U << 4)
> +#define PSTATE_F (1U << 6)
> +#define PSTATE_I (1U << 7)
> +#define PSTATE_A (1U << 8)
> +#define PSTATE_D (1U << 9)
> +#define PSTATE_IL (1U << 20)
> +#define PSTATE_SS (1U << 21)
> +#define PSTATE_V (1U << 28)
> +#define PSTATE_C (1U << 29)
> +#define PSTATE_Z (1U << 30)
> +#define PSTATE_N (1U << 31)
> +#define PSTATE_NZCV (PSTATE_N | PSTATE_Z | PSTATE_C | PSTATE_V)
> +#define CACHED_PSTATE_BITS (PSTATE_NZCV)
> +/* Mode values for AArch64 */
> +#define PSTATE_MODE_EL3h 13
> +#define PSTATE_MODE_EL3t 12
> +#define PSTATE_MODE_EL2h 9
> +#define PSTATE_MODE_EL2t 8
> +#define PSTATE_MODE_EL1h 5
> +#define PSTATE_MODE_EL1t 4
> +#define PSTATE_MODE_EL0t 0
> +
> +/* Return the current PSTATE value. For the moment we don't support 32<->64 bit
> + * interprocessing, so we don't attempt to sync with the cpsr state used by
> + * the 32 bit decoder.
> + */
> +static inline uint32_t pstate_read(CPUARMState *env)
> +{
> + int ZF;
> +
> + ZF = (env->ZF == 0);
So the comment on the ZF field means "if th ZF field is zero, then the
pstate.Z field is set, meaning the result of an op was zero". Crystal
clear.
> + return (env->NF & 0x80000000) | (ZF << 30)
> + | (env->CF << 29) | ((env->VF & 0x80000000) >> 3)
> + | env->pstate;
> +}
> +
> +static inline void pstate_write(CPUARMState *env, uint32_t val)
> +{
> + env->ZF = (~val) & PSTATE_Z;
> + env->NF = val;
> + env->CF = (val >> 29) & 1;
> + env->VF = (val << 3) & 0x80000000;
> + env->pstate = val & ~CACHED_PSTATE_BITS;
> +}
> +
> /* Return the current CPSR value. */
> uint32_t cpsr_read(CPUARMState *env);
> /* Set the CPSR. Note that some bits of mask must be all-set or all-clear. */
> diff --git a/target-arm/gdbstub64.c b/target-arm/gdbstub64.c
> index 7cb6a7c..e8a8295 100644
> --- a/target-arm/gdbstub64.c
> +++ b/target-arm/gdbstub64.c
> @@ -37,7 +37,7 @@ int aarch64_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
> return gdb_get_reg64(mem_buf, env->pc);
> break;
> case 33:
> - return gdb_get_reg32(mem_buf, env->pstate);
> + return gdb_get_reg32(mem_buf, pstate_read(env));
> }
> /* Unknown register. */
> return 0;
> @@ -65,7 +65,7 @@ int aarch64_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
> return 8;
> case 33:
> /* CPSR */
> - env->pstate = tmp;
> + pstate_write(env, tmp);
> return 4;
> }
> /* Unknown register. */
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index f120088..932b601 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -67,6 +67,7 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
> {
> ARMCPU *cpu = ARM_CPU(cs);
> CPUARMState *env = &cpu->env;
> + uint32_t psr = pstate_read(env);
> int i;
>
> cpu_fprintf(f, "PC=%016"PRIx64" SP=%016"PRIx64"\n",
> @@ -79,11 +80,12 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
> cpu_fprintf(f, " ");
> }
> }
> - cpu_fprintf(f, "PSTATE=%c%c%c%c\n",
> - env->pstate & PSTATE_N ? 'n' : '.',
> - env->pstate & PSTATE_Z ? 'z' : '.',
> - env->pstate & PSTATE_C ? 'c' : '.',
> - env->pstate & PSTATE_V ? 'v' : '.');
> + cpu_fprintf(f, "PSTATE=%08x (flags %c%c%c%c)\n",
> + psr,
> + psr & PSTATE_N ? 'N' : '-',
> + psr & PSTATE_Z ? 'Z' : '-',
> + psr & PSTATE_C ? 'C' : '-',
> + psr & PSTATE_V ? 'V' : '-');
> cpu_fprintf(f, "\n");
> }
>
> --
> 1.7.9.5
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
Otherwise it looks good to me:
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 3/7] target-arm: Add minimal KVM AArch64 support
2013-11-28 13:33 ` [Qemu-devel] [PATCH 3/7] target-arm: Add minimal KVM AArch64 support Peter Maydell
@ 2013-12-16 23:39 ` Christoffer Dall
2013-12-17 0:21 ` Peter Maydell
0 siblings, 1 reply; 38+ messages in thread
From: Christoffer Dall @ 2013-12-16 23:39 UTC (permalink / raw)
To: Peter Maydell; +Cc: patches, qemu-devel, kvmarm
On Thu, Nov 28, 2013 at 01:33:18PM +0000, Peter Maydell wrote:
> From: "Mian M. Hamayun" <m.hamayun@virtualopensystems.com>
>
> Add the bare minimum set of functions needed for control of an
> AArch64 KVM vcpu:
> * CPU initialization
> * minimal get/put register functions which only handle the
> basic state of the CPU
>
> Signed-off-by: Mian M. Hamayun <m.hamayun@virtualopensystems.com>
> [PMM: significantly overhauled; most notably:
> * code lives in kvm64.c rather than using #ifdefs
> * support '-cpu host' rather than implicitly using whatever the
> host's CPU is regardless of what the user requests
> * fix bug attempting to get/set nonexistent X[31]
> * fix bug writing 64 bit kernel pstate into uint32_t env field
> ]
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Besides the minor nits/questions below:
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
> target-arm/Makefile.objs | 1 +
> target-arm/kvm.c | 4 +
> target-arm/kvm64.c | 204 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 209 insertions(+)
> create mode 100644 target-arm/kvm64.c
>
> diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs
> index d1db77c..5493a4c 100644
> --- a/target-arm/Makefile.objs
> +++ b/target-arm/Makefile.objs
> @@ -7,3 +7,4 @@ obj-y += neon_helper.o iwmmxt_helper.o
> obj-y += gdbstub.o
> obj-$(TARGET_AARCH64) += cpu64.o translate-a64.o gdbstub64.o
> obj-$(call land,$(CONFIG_KVM),$(call lnot,$(TARGET_AARCH64))) += kvm32.o
> +obj-$(call land,$(CONFIG_KVM),$(TARGET_AARCH64)) += kvm64.o
> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
> index 5cdb3b9..1d2688d 100644
> --- a/target-arm/kvm.c
> +++ b/target-arm/kvm.c
> @@ -128,7 +128,11 @@ static void kvm_arm_host_cpu_initfn(Object *obj)
>
> static const TypeInfo host_arm_cpu_type_info = {
> .name = TYPE_ARM_HOST_CPU,
> +#ifdef TARGET_AARCH64
> + .parent = TYPE_AARCH64_CPU,
> +#else
> .parent = TYPE_ARM_CPU,
> +#endif
> .instance_init = kvm_arm_host_cpu_initfn,
> .class_init = kvm_arm_host_cpu_class_init,
> .class_size = sizeof(ARMHostCPUClass),
> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
> new file mode 100644
> index 0000000..599db5d
> --- /dev/null
> +++ b/target-arm/kvm64.c
> @@ -0,0 +1,204 @@
> +/*
> + * ARM implementation of KVM hooks, 64 bit specific code
> + *
> + * Copyright Mian-M. Hamayun 2013, Virtual Open Systems
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include <stdio.h>
> +#include <sys/types.h>
> +#include <sys/ioctl.h>
> +#include <sys/mman.h>
> +
> +#include <linux/kvm.h>
> +
> +#include "qemu-common.h"
> +#include "qemu/timer.h"
> +#include "sysemu/sysemu.h"
> +#include "sysemu/kvm.h"
> +#include "kvm_arm.h"
> +#include "cpu.h"
> +#include "hw/arm/arm.h"
> +
> +static inline void set_feature(uint64_t *features, int feature)
> +{
> + *features |= 1ULL << feature;
> +}
> +
> +bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
> +{
> + /* Identify the feature bits corresponding to the host CPU, and
> + * fill out the ARMHostCPUClass fields accordingly. To do this
> + * we have to create a scratch VM, create a single CPU inside it,
> + * and then query that CPU for the relevant ID registers.
> + * For AArch64 we currently don't care about ID registers at
> + * all; we just want to know the CPU type.
> + */
> + int fdarray[3];
> + uint64_t features = 0;
> + /* Old kernels may not know about the PREFERRED_TARGET ioctl: however
> + * we know these will only support creating one kind of guest CPU,
> + * which is its preferred CPU type. Fortunately these old kernels
> + * support only a very limited number of CPUs.
> + */
> + static const uint32_t cpus_to_try[] = {
> + KVM_ARM_TARGET_AEM_V8,
> + KVM_ARM_TARGET_FOUNDATION_V8,
> + KVM_ARM_TARGET_CORTEX_A57,
> + QEMU_KVM_ARM_TARGET_NONE
> + };
> + struct kvm_vcpu_init init;
> +
> + if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) {
> + return false;
> + }
> +
> + ahcc->target = init.target;
> + ahcc->dtb_compatible = "arm,arm-v7";
arm,arm-v8 ?
> +
> + kvm_arm_destroy_scratch_host_vcpu(fdarray);
> +
> + /* We can assume any KVM supporting CPU is at least a v8
> + * with VFPv4+Neon; this in turn implies most of the other
> + * feature bits.
not sure I understand the bit about implying other feature bits, the
only other thing we're setting here is AARCH64 and the features bits are
enum values?
> + */
> + set_feature(&features, ARM_FEATURE_V8);
> + set_feature(&features, ARM_FEATURE_VFP4);
> + set_feature(&features, ARM_FEATURE_NEON);
> + set_feature(&features, ARM_FEATURE_AARCH64);
> +
> + ahcc->features = features;
> +
> + return true;
> +}
> +
> +int kvm_arch_init_vcpu(CPUState *cs)
> +{
> + ARMCPU *cpu = ARM_CPU(cs);
> + struct kvm_vcpu_init init;
> + int ret;
> +
> + if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE ||
> + !arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
> + fprintf(stderr, "KVM is not supported for this guest CPU type\n");
> + return -EINVAL;
> + }
> +
> + init.target = cpu->kvm_target;
> + memset(init.features, 0, sizeof(init.features));
> + if (cpu->start_powered_off) {
> + init.features[0] = 1 << KVM_ARM_VCPU_POWER_OFF;
> + }
> + ret = kvm_vcpu_ioctl(cs, KVM_ARM_VCPU_INIT, &init);
> +
> + /* TODO : support for save/restore/reset of system regs via tuple list */
> +
> + return ret;
> +}
> +
> +#define AARCH64_CORE_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \
> + KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
> +
> +int kvm_arch_put_registers(CPUState *cs, int level)
> +{
> + struct kvm_one_reg reg;
> + uint64_t val;
> + int i;
> + int ret;
> +
> + ARMCPU *cpu = ARM_CPU(cs);
> + CPUARMState *env = &cpu->env;
> +
> + for (i = 0; i < 31; i++) {
> + reg.id = AARCH64_CORE_REG(regs.regs[i]);
> + reg.addr = (uintptr_t) &env->xregs[i];
> + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
> + if (ret) {
> + return ret;
> + }
> + }
> +
> + reg.id = AARCH64_CORE_REG(regs.sp);
> + reg.addr = (uintptr_t) &env->xregs[31];
> + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
> + if (ret) {
> + return ret;
> + }
> +
> + /* Note that KVM thinks pstate is 64 bit but we use a uint32_t */
> + val = pstate_read(env);
> + reg.id = AARCH64_CORE_REG(regs.pstate);
> + reg.addr = (uintptr_t) &val;
> + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
> + if (ret) {
> + return ret;
> + }
> +
> + reg.id = AARCH64_CORE_REG(regs.pc);
> + reg.addr = (uintptr_t) &env->pc;
> + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
> + if (ret) {
> + return ret;
> + }
> +
> + /* TODO:
> + * SP_EL1
> + * ELR_EL1
> + * SPSR[]
> + * FP state
> + * system registers
> + */
> + return ret;
> +}
> +
> +int kvm_arch_get_registers(CPUState *cs)
> +{
> + struct kvm_one_reg reg;
> + uint64_t val;
> + int i;
> + int ret;
> +
> + ARMCPU *cpu = ARM_CPU(cs);
> + CPUARMState *env = &cpu->env;
> +
> + for (i = 0; i < 31; i++) {
> + reg.id = AARCH64_CORE_REG(regs.regs[i]);
> + reg.addr = (uintptr_t) &env->xregs[i];
> + ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
> + if (ret) {
> + return ret;
> + }
> + }
> +
> + reg.id = AARCH64_CORE_REG(regs.sp);
> + reg.addr = (uintptr_t) &env->xregs[31];
> + ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
> + if (ret) {
> + return ret;
> + }
> +
> + reg.id = AARCH64_CORE_REG(regs.pstate);
> + reg.addr = (uintptr_t) &val;
> + ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
> + if (ret) {
> + return ret;
> + }
> + pstate_write(env, val);
> +
> + reg.id = AARCH64_CORE_REG(regs.pc);
> + reg.addr = (uintptr_t) &env->pc;
> + ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
> + if (ret) {
> + return ret;
> + }
> +
> + /* TODO: other registers */
> + return ret;
> +}
> +
> +void kvm_arch_reset_vcpu(CPUState *cs)
> +{
> +}
> --
> 1.7.9.5
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 4/7] configure: Enable KVM for aarch64 host/target combination
2013-11-28 13:33 ` [Qemu-devel] [PATCH 4/7] configure: Enable KVM for aarch64 host/target combination Peter Maydell
@ 2013-12-16 23:40 ` Christoffer Dall
0 siblings, 0 replies; 38+ messages in thread
From: Christoffer Dall @ 2013-12-16 23:40 UTC (permalink / raw)
To: Peter Maydell; +Cc: patches, qemu-devel, kvmarm
On Thu, Nov 28, 2013 at 01:33:19PM +0000, Peter Maydell wrote:
> Enable KVM if the host and target CPU are both aarch64. Note
> that host aarch64 + target arm is not valid for KVM acceleration:
> the 64 bit kernel does not support the ioctl interface for
> 32 bit CPUs. 32 bit VMs on 64 bit hosts need to be created
> using the 64 bit ioctl interface; when QEMU supports this it
> will be on the arch64-softmmu target with a -cpu parameter for
> a 32 bit CPU, which is still an aarch64/aarch64 combination
> as far as configure is concerned.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
> configure | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index 508f6a5..3317013 100755
> --- a/configure
> +++ b/configure
> @@ -4513,7 +4513,7 @@ case "$target_name" in
> *)
> esac
> case "$target_name" in
> - arm|i386|x86_64|ppcemb|ppc|ppc64|s390x)
> + aarch64|arm|i386|x86_64|ppcemb|ppc|ppc64|s390x)
> # Make sure the target and host cpus are compatible
> if test "$kvm" = "yes" -a "$target_softmmu" = "yes" -a \
> \( "$target_name" = "$cpu" -o \
> --
> 1.7.9.5
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code
2013-11-28 13:33 ` [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code Peter Maydell
2013-12-13 3:19 ` Peter Crosthwaite
@ 2013-12-16 23:40 ` Christoffer Dall
2013-12-17 0:23 ` Peter Maydell
1 sibling, 1 reply; 38+ messages in thread
From: Christoffer Dall @ 2013-12-16 23:40 UTC (permalink / raw)
To: Peter Maydell
Cc: agraf, Mian M. Hamayun, patches, qemu-devel, afaerber, kvmarm
On Thu, Nov 28, 2013 at 01:33:20PM +0000, Peter Maydell wrote:
> For AArch64 we will obviously require a different set of
> primary and secondary boot loader code fragments. However currently
> we hardcode the offsets into the loader code where we must write
> the entrypoint and other data into arm_load_kernel(). This makes it
> hard to substitute a different loader fragment, so switch to a more
> flexible scheme where instead of a raw array of instructions we use
> an array of (instruction, fixup-type) pairs that indicate which
> words need special action or data written into them.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Minor thing below, otherwise it looks quite nice:
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
> hw/arm/boot.c | 152 ++++++++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 107 insertions(+), 45 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 55d552f..77d29a8 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -20,15 +20,33 @@
> #define KERNEL_ARGS_ADDR 0x100
> #define KERNEL_LOAD_ADDR 0x00010000
>
> +typedef enum {
> + FIXUP_NONE = 0, /* do nothing */
> + FIXUP_TERMINATOR, /* end of insns */
> + FIXUP_BOARDID, /* overwrite with board ID number */
> + FIXUP_ARGPTR, /* overwrite with pointer to kernel args */
> + FIXUP_ENTRYPOINT, /* overwrite with kernel entry point */
> + FIXUP_GIC_CPU_IF, /* overwrite with GIC CPU interface address */
> + FIXUP_BOOTREG, /* overwrite with boot register address */
> + FIXUP_DSB, /* overwrite with correct DSB insn for cpu */
> + FIXUP_MAX,
> +} FixupType;
> +
> +typedef struct ARMInsnFixup {
> + uint32_t insn;
> + FixupType fixup;
> +} ARMInsnFixup;
> +
> /* The worlds second smallest bootloader. Set r0-r2, then jump to kernel. */
> -static uint32_t bootloader[] = {
> - 0xe3a00000, /* mov r0, #0 */
> - 0xe59f1004, /* ldr r1, [pc, #4] */
> - 0xe59f2004, /* ldr r2, [pc, #4] */
> - 0xe59ff004, /* ldr pc, [pc, #4] */
> - 0, /* Board ID */
> - 0, /* Address of kernel args. Set by integratorcp_init. */
> - 0 /* Kernel entry point. Set by integratorcp_init. */
> +static const ARMInsnFixup bootloader[] = {
> + { 0xe3a00000 }, /* mov r0, #0 */
> + { 0xe59f1004 }, /* ldr r1, [pc, #4] */
> + { 0xe59f2004 }, /* ldr r2, [pc, #4] */
> + { 0xe59ff004 }, /* ldr pc, [pc, #4] */
> + { 0, FIXUP_BOARDID },
> + { 0, FIXUP_ARGPTR },
> + { 0, FIXUP_ENTRYPOINT },
> + { 0, FIXUP_TERMINATOR }
> };
>
> /* Handling for secondary CPU boot in a multicore system.
> @@ -48,39 +66,83 @@ static uint32_t bootloader[] = {
> #define DSB_INSN 0xf57ff04f
> #define CP15_DSB_INSN 0xee070f9a /* mcr cp15, 0, r0, c7, c10, 4 */
>
> -static uint32_t smpboot[] = {
> - 0xe59f2028, /* ldr r2, gic_cpu_if */
> - 0xe59f0028, /* ldr r0, startaddr */
> - 0xe3a01001, /* mov r1, #1 */
> - 0xe5821000, /* str r1, [r2] - set GICC_CTLR.Enable */
> - 0xe3a010ff, /* mov r1, #0xff */
> - 0xe5821004, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
> - DSB_INSN, /* dsb */
> - 0xe320f003, /* wfi */
> - 0xe5901000, /* ldr r1, [r0] */
> - 0xe1110001, /* tst r1, r1 */
> - 0x0afffffb, /* beq <wfi> */
> - 0xe12fff11, /* bx r1 */
> - 0, /* gic_cpu_if: base address of GIC CPU interface */
> - 0 /* bootreg: Boot register address is held here */
> +static const ARMInsnFixup smpboot[] = {
> + { 0xe59f2028 }, /* ldr r2, gic_cpu_if */
> + { 0xe59f0028 }, /* ldr r0, startaddr */
> + { 0xe3a01001 }, /* mov r1, #1 */
> + { 0xe5821000 }, /* str r1, [r2] - set GICC_CTLR.Enable */
> + { 0xe3a010ff }, /* mov r1, #0xff */
> + { 0xe5821004 }, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
> + { 0, FIXUP_DSB }, /* dsb */
> + { 0xe320f003 }, /* wfi */
> + { 0xe5901000 }, /* ldr r1, [r0] */
> + { 0xe1110001 }, /* tst r1, r1 */
> + { 0x0afffffb }, /* beq <wfi> */
> + { 0xe12fff11 }, /* bx r1 */
> + { 0, FIXUP_GIC_CPU_IF },
> + { 0, FIXUP_BOOTREG },
couldn't we add the gic_cpu_if and startaddr "labels" as comments to the
two lines above? (alternatively also rename the reference to startaddr
to bootret in the second instruction comment).
> + { 0, FIXUP_TERMINATOR }
> };
>
> +static void write_bootloader(const char *name, hwaddr addr,
> + const ARMInsnFixup *insns, uint32_t *fixupcontext)
> +{
> + /* Fix up the specified bootloader fragment and write it into
> + * guest memory using rom_add_blob_fixed(). fixupcontext is
> + * an array giving the values to write in for the fixup types
> + * which write a value into the code array.
> + */
> + int i, len;
> + uint32_t *code;
> +
> + len = 0;
> + while (insns[len].fixup != FIXUP_TERMINATOR) {
> + len++;
> + }
> +
> + code = g_new0(uint32_t, len);
> +
> + for (i = 0; i < len; i++) {
> + uint32_t insn = insns[i].insn;
> + FixupType fixup = insns[i].fixup;
> +
> + switch (fixup) {
> + case FIXUP_NONE:
> + break;
> + case FIXUP_BOARDID:
> + case FIXUP_ARGPTR:
> + case FIXUP_ENTRYPOINT:
> + case FIXUP_GIC_CPU_IF:
> + case FIXUP_BOOTREG:
> + case FIXUP_DSB:
> + insn = fixupcontext[fixup];
> + break;
> + default:
> + abort();
> + }
> + code[i] = tswap32(insn);
> + }
> +
> + rom_add_blob_fixed(name, code, len * sizeof(uint32_t), addr);
> +
> + g_free(code);
> +}
> +
> static void default_write_secondary(ARMCPU *cpu,
> const struct arm_boot_info *info)
> {
> - int n;
> - smpboot[ARRAY_SIZE(smpboot) - 1] = info->smp_bootreg_addr;
> - smpboot[ARRAY_SIZE(smpboot) - 2] = info->gic_cpu_if_addr;
> - for (n = 0; n < ARRAY_SIZE(smpboot); n++) {
> - /* Replace DSB with the pre-v7 DSB if necessary. */
> - if (!arm_feature(&cpu->env, ARM_FEATURE_V7) &&
> - smpboot[n] == DSB_INSN) {
> - smpboot[n] = CP15_DSB_INSN;
> - }
> - smpboot[n] = tswap32(smpboot[n]);
> + uint32_t fixupcontext[FIXUP_MAX];
> +
> + fixupcontext[FIXUP_GIC_CPU_IF] = info->gic_cpu_if_addr;
> + fixupcontext[FIXUP_BOOTREG] = info->smp_bootreg_addr;
> + if (arm_feature(&cpu->env, ARM_FEATURE_V7)) {
> + fixupcontext[FIXUP_DSB] = DSB_INSN;
> + } else {
> + fixupcontext[FIXUP_DSB] = CP15_DSB_INSN;
> }
> - rom_add_blob_fixed("smpboot", smpboot, sizeof(smpboot),
> - info->smp_loader_start);
> +
> + write_bootloader("smpboot", info->smp_loader_start,
> + smpboot, fixupcontext);
> }
>
> static void default_reset_secondary(ARMCPU *cpu,
> @@ -354,7 +416,6 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
> CPUState *cs = CPU(cpu);
> int kernel_size;
> int initrd_size;
> - int n;
> int is_linux = 0;
> uint64_t elf_entry;
> hwaddr entry;
> @@ -420,6 +481,8 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
> }
> info->entry = entry;
> if (is_linux) {
> + uint32_t fixupcontext[FIXUP_MAX];
> +
> if (info->initrd_filename) {
> initrd_size = load_ramdisk(info->initrd_filename,
> info->initrd_start,
> @@ -441,7 +504,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
> }
> info->initrd_size = initrd_size;
>
> - bootloader[4] = info->board_id;
> + fixupcontext[FIXUP_BOARDID] = info->board_id;
>
> /* for device tree boot, we pass the DTB directly in r2. Otherwise
> * we point to the kernel args.
> @@ -456,9 +519,9 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
> if (load_dtb(dtb_start, info)) {
> exit(1);
> }
> - bootloader[5] = dtb_start;
> + fixupcontext[FIXUP_ARGPTR] = dtb_start;
> } else {
> - bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
> + fixupcontext[FIXUP_ARGPTR] = info->loader_start + KERNEL_ARGS_ADDR;
> if (info->ram_size >= (1ULL << 32)) {
> fprintf(stderr, "qemu: RAM size must be less than 4GB to boot"
> " Linux kernel using ATAGS (try passing a device tree"
> @@ -466,12 +529,11 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
> exit(1);
> }
> }
> - bootloader[6] = entry;
> - for (n = 0; n < sizeof(bootloader) / 4; n++) {
> - bootloader[n] = tswap32(bootloader[n]);
> - }
> - rom_add_blob_fixed("bootloader", bootloader, sizeof(bootloader),
> - info->loader_start);
> + fixupcontext[FIXUP_ENTRYPOINT] = entry;
> +
> + write_bootloader("bootloader", info->loader_start,
> + bootloader, fixupcontext);
> +
> if (info->nb_cpus > 1) {
> info->write_secondary_boot(cpu, info);
> }
> --
> 1.7.9.5
>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 6/7] hw/arm/boot: Add boot support for AArch64 processor
2013-11-28 13:33 ` [Qemu-devel] [PATCH 6/7] hw/arm/boot: Add boot support for AArch64 processor Peter Maydell
@ 2013-12-16 23:40 ` Christoffer Dall
2013-12-17 0:25 ` Peter Maydell
0 siblings, 1 reply; 38+ messages in thread
From: Christoffer Dall @ 2013-12-16 23:40 UTC (permalink / raw)
To: Peter Maydell; +Cc: patches, qemu-devel, kvmarm
On Thu, Nov 28, 2013 at 01:33:21PM +0000, Peter Maydell wrote:
> From: "Mian M. Hamayun" <m.hamayun@virtualopensystems.com>
>
> This commit adds support for booting a single AArch64 CPU by setting
> appropriate registers. The bootloader includes placehoders for Board-ID
> that are used to implement uniform indexing across different bootloaders.
>
> Signed-off-by: Mian M. Hamayun <m.hamayun@virtualopensystems.com>
> [PMM:
> * updated to use ARMInsnFixup style bootloader fragments
> * dropped virt.c additions
> * use runtime checks for "is this an AArch64 core" rather than ifdefs
> * drop some unnecessary setting of registers in reset hook
> ]
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/arm/boot.c | 40 +++++++++++++++++++++++++++++++++++-----
> 1 file changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 77d29a8..b6b04b7 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -19,6 +19,8 @@
>
> #define KERNEL_ARGS_ADDR 0x100
> #define KERNEL_LOAD_ADDR 0x00010000
> +/* The AArch64 kernel boot protocol specifies a different preferred address */
> +#define KERNEL64_LOAD_ADDR 0x00080000
I assume you referring to Documentation/arm/Booting and
Documentation/arm64/booting.txt here? Perhaps it would be nicer to
refer to that and state how we reach the address for the two archs
instead of having the aarch64 specific note here, e.g. "The kernel
recommends booting at offset 0x80000 from system RAM" or something like
that...
>
> typedef enum {
> FIXUP_NONE = 0, /* do nothing */
> @@ -37,6 +39,20 @@ typedef struct ARMInsnFixup {
> FixupType fixup;
> } ARMInsnFixup;
>
> +static const ARMInsnFixup bootloader_aarch64[] = {
> + { 0x580000c0 }, /* ldr x0, 18 ; Load the lower 32-bits of DTB */
so by 18 you mean the label 0x18 assuming the instruction above has the
label 0 or something like that? Is this an accepted/familiar notation
or shoudl we do something like the arm32 bootloaders and "define a
label in the comments"...?
> + { 0xaa1f03e1 }, /* mov x1, xzr */
> + { 0xaa1f03e2 }, /* mov x2, xzr */
> + { 0xaa1f03e3 }, /* mov x3, xzr */
> + { 0x58000084 }, /* ldr x4, 20 ; Load the lower 32-bits of kernel entry */
same as above
> + { 0xd61f0080 }, /* br x4 ; Jump to the kernel entry point */
> + { 0, FIXUP_ARGPTR }, /* .word @DTB Lower 32-bits */
> + { 0 }, /* .word @DTB Higher 32-bits */
> + { 0, FIXUP_ENTRYPOINT }, /* .word @Kernel Entry Lower 32-bits */
> + { 0 }, /* .word @Kernel Entry Higher 32-bits */
> + { 0, FIXUP_TERMINATOR }
> +};
> +
> /* The worlds second smallest bootloader. Set r0-r2, then jump to kernel. */
> static const ARMInsnFixup bootloader[] = {
> { 0xe3a00000 }, /* mov r0, #0 */
> @@ -396,7 +412,12 @@ static void do_cpu_reset(void *opaque)
> env->thumb = info->entry & 1;
> } else {
> if (CPU(cpu) == first_cpu) {
> - env->regs[15] = info->loader_start;
> + if (env->aarch64) {
> + env->pc = info->loader_start;
> + } else {
> + env->regs[15] = info->loader_start;
> + }
> +
> if (!info->dtb_filename) {
> if (old_param) {
> set_kernel_args_old(info);
> @@ -418,8 +439,9 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
> int initrd_size;
> int is_linux = 0;
> uint64_t elf_entry;
> - hwaddr entry;
> + hwaddr entry, kernel_load_offset;
> int big_endian;
> + static const ARMInsnFixup *primary_loader;
>
> /* Load the kernel. */
> if (!info->kernel_filename) {
> @@ -429,6 +451,14 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
> return;
> }
>
> + if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
> + primary_loader = bootloader_aarch64;
> + kernel_load_offset = KERNEL64_LOAD_ADDR;
> + } else {
> + primary_loader = bootloader;
> + kernel_load_offset = KERNEL_LOAD_ADDR;
> + }
> +
> info->dtb_filename = qemu_opt_get(qemu_get_machine_opts(), "dtb");
>
> if (!info->secondary_cpu_reset_hook) {
> @@ -469,9 +499,9 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
> &is_linux);
> }
> if (kernel_size < 0) {
> - entry = info->loader_start + KERNEL_LOAD_ADDR;
> + entry = info->loader_start + kernel_load_offset;
> kernel_size = load_image_targphys(info->kernel_filename, entry,
> - info->ram_size - KERNEL_LOAD_ADDR);
> + info->ram_size - kernel_load_offset);
> is_linux = 1;
> }
> if (kernel_size < 0) {
> @@ -532,7 +562,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
> fixupcontext[FIXUP_ENTRYPOINT] = entry;
>
> write_bootloader("bootloader", info->loader_start,
> - bootloader, fixupcontext);
> + primary_loader, fixupcontext);
>
> if (info->nb_cpus > 1) {
> info->write_secondary_boot(cpu, info);
> --
> 1.7.9.5
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
Otherwise looks good to me:
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 7/7] default-configs: Add config for aarch64-softmmu
2013-11-28 13:33 ` [Qemu-devel] [PATCH 7/7] default-configs: Add config for aarch64-softmmu Peter Maydell
@ 2013-12-16 23:40 ` Christoffer Dall
2013-12-17 0:27 ` Peter Maydell
2013-12-17 13:33 ` Christopher Covington
0 siblings, 2 replies; 38+ messages in thread
From: Christoffer Dall @ 2013-12-16 23:40 UTC (permalink / raw)
To: Peter Maydell; +Cc: patches, qemu-devel, kvmarm
On Thu, Nov 28, 2013 at 01:33:22PM +0000, Peter Maydell wrote:
> Add a config for aarch64-softmmu; this enables building of this target.
> The resulting executable doesn't know about any 64 bit CPUs, but all
> the 32 bit CPUs and board models work.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> default-configs/aarch64-softmmu.mak | 9 +++++++++
> 1 file changed, 9 insertions(+)
> create mode 100644 default-configs/aarch64-softmmu.mak
>
> diff --git a/default-configs/aarch64-softmmu.mak b/default-configs/aarch64-softmmu.mak
> new file mode 100644
> index 0000000..76a45cc
> --- /dev/null
> +++ b/default-configs/aarch64-softmmu.mak
> @@ -0,0 +1,9 @@
> +# Default configuration for aarch64-softmmu
> +
> +include pci.mak
> +include usb.mak
pci and usb? pci is required for virtio I presume, and USB? for 32-bit
boards?
> +
> +# We support all the 32 bit boards so need all their config
> +include arm-softmmu.mak
> +
> +# Currently no 64-bit specific config requirements
> --
> 1.7.9.5
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 2/7] target-arm: Clean up handling of AArch64 PSTATE
2013-12-16 23:39 ` Christoffer Dall
@ 2013-12-17 0:15 ` Peter Maydell
2013-12-17 4:45 ` Christoffer Dall
0 siblings, 1 reply; 38+ messages in thread
From: Peter Maydell @ 2013-12-17 0:15 UTC (permalink / raw)
To: Christoffer Dall; +Cc: Patch Tracking, QEMU Developers, kvmarm
On 16 December 2013 23:39, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> On Thu, Nov 28, 2013 at 01:33:17PM +0000, Peter Maydell wrote
>> --- a/target-arm/cpu.h
>> +++ b/target-arm/cpu.h
>> @@ -113,8 +113,13 @@ typedef struct CPUARMState {
>> /* Regs for A64 mode. */
>> uint64_t xregs[32];
>> uint64_t pc;
>> - /* TODO: pstate doesn't correspond to an architectural register;
>> - * it would be better modelled as the underlying fields.
>> + /* PSTATE isn't an architectural register for ARMv8. However, it is
>> + * convenient for us to assemble the underlying state into a 32 bit format
>> + * identical to the architectural format used for the SPSR. (This is also
>> + * what the Linux kernel's 'pstate' field in signal handlers and KVM's
>> + * 'pstate' register are.) NZCV are kept in their split fields; aarch64
>> + * is an inverted split version of PSTATE.nRW (aka M[4]); other bits are
>> + * stored here when in AArch64.
>
> I really don't understand what you are trying to say beginning with
> aarch64 is an inverted split version...
When PSTATE.nRW == 1, aarch64 == 0; when PSTATE.nRW == 0,
aarch64 == 1. That is, aarch64 is the nRW bit inverted. Some descriptions
of the SPSR format call the nRW bit the 4th bit of the mode field, M[4].
> Which other bits are stored
> where in AArch64?
All the bits not listed specifically in the first half of the sentence, ie
everything except nzcv and nRW, are stored here, ie in the
"uint32_t pstate" field this comment is documenting.
> Also what is the rationale behind keeping NZCV in their split fields?
TCG generated code is faster: there are some neat sequences for
generating the correct flags results from arithmetic and logic ops
which you can use (and which are the rationale for the rather odd
definitions of the cpu_NF/ZF/CF/VF). aarch64 we keep separate
partly for historical reasons and partly because there's a whole
load of places in the C code that want to test it.
>> */
>> uint32_t pstate;
>> uint32_t aarch64; /* 1 if CPU is in aarch64 state; inverse of PSTATE.nRW */
>> +/* Return the current PSTATE value. For the moment we don't support 32<->64 bit
>> + * interprocessing, so we don't attempt to sync with the cpsr state used by
>> + * the 32 bit decoder.
>> + */
>> +static inline uint32_t pstate_read(CPUARMState *env)
>> +{
>> + int ZF;
>> +
>> + ZF = (env->ZF == 0);
>
> So the comment on the ZF field means "if th ZF field is zero, then the
> pstate.Z field is set, meaning the result of an op was zero". Crystal
> clear.
Yes. If you're generating TCG ops this means you can calculate
the correct value of cpu_ZF by just copying the result into cpu_ZF
if it's a 32 bit op. 64 bit code is not quite as nice but it's not
terrible.
-- PMM
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 3/7] target-arm: Add minimal KVM AArch64 support
2013-12-16 23:39 ` Christoffer Dall
@ 2013-12-17 0:21 ` Peter Maydell
2013-12-17 4:46 ` Christoffer Dall
0 siblings, 1 reply; 38+ messages in thread
From: Peter Maydell @ 2013-12-17 0:21 UTC (permalink / raw)
To: Christoffer Dall; +Cc: Patch Tracking, QEMU Developers, kvmarm
On 16 December 2013 23:39, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> On Thu, Nov 28, 2013 at 01:33:18PM +0000, Peter Maydell wrote:
>> + ahcc->target = init.target;
>> + ahcc->dtb_compatible = "arm,arm-v7";
>
> arm,arm-v8 ?
Oops, yes, cut-n-pasto.
>
>> +
>> + kvm_arm_destroy_scratch_host_vcpu(fdarray);
>> +
>> + /* We can assume any KVM supporting CPU is at least a v8
>> + * with VFPv4+Neon; this in turn implies most of the other
>> + * feature bits.
>
> not sure I understand the bit about implying other feature bits, the
> only other thing we're setting here is AARCH64 and the features bits are
> enum values?
target-arm/cpu.c:cpu_realize_fn() has a large set of if statements
like
if (arm_feature(env, ARM_FEATURE_V8)) {
set_feature(env, ARM_FEATURE_V7);
set_feature(env, ARM_FEATURE_ARM_DIV);
set_feature(env, ARM_FEATURE_LPAE);
}
because architecturally some features or arch versions imply
that you have others (eg above v8 means we always know
we have LPAE and division)...
>> + */
>> + set_feature(&features, ARM_FEATURE_V8);
>> + set_feature(&features, ARM_FEATURE_VFP4);
>> + set_feature(&features, ARM_FEATURE_NEON);
>> + set_feature(&features, ARM_FEATURE_AARCH64);
...and because presence of the 'v8', 'vfp4', 'neon' features implies
(as enforced via those if statements) presence of just about every
other feature it means we don't need to have specific
tests for "do the CPU's feature registers say we support
division?" like the v7 KVM code does, because we know
that it's all implied automatically.
thanks
-- PMM
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code
2013-12-16 23:40 ` Christoffer Dall
@ 2013-12-17 0:23 ` Peter Maydell
0 siblings, 0 replies; 38+ messages in thread
From: Peter Maydell @ 2013-12-17 0:23 UTC (permalink / raw)
To: Christoffer Dall
Cc: Alexander Graf, Mian M. Hamayun, Patch Tracking, QEMU Developers,
Andreas Färber, kvmarm
On 16 December 2013 23:40, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> On Thu, Nov 28, 2013 at 01:33:20PM +0000, Peter Maydell wrote:
>> For AArch64 we will obviously require a different set of
>> primary and secondary boot loader code fragments. However currently
>> we hardcode the offsets into the loader code where we must write
>> the entrypoint and other data into arm_load_kernel(). This makes it
>> hard to substitute a different loader fragment, so switch to a more
>> flexible scheme where instead of a raw array of instructions we use
>> an array of (instruction, fixup-type) pairs that indicate which
>> words need special action or data written into them.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
> Minor thing below, otherwise it looks quite nice:
>
> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
>
>> ---
>> hw/arm/boot.c | 152 ++++++++++++++++++++++++++++++++++++++++-----------------
>> 1 file changed, 107 insertions(+), 45 deletions(-)
>>
>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>> index 55d552f..77d29a8 100644
>> --- a/hw/arm/boot.c
>> +++ b/hw/arm/boot.c
>> @@ -20,15 +20,33 @@
>> #define KERNEL_ARGS_ADDR 0x100
>> #define KERNEL_LOAD_ADDR 0x00010000
>>
>> +typedef enum {
>> + FIXUP_NONE = 0, /* do nothing */
>> + FIXUP_TERMINATOR, /* end of insns */
>> + FIXUP_BOARDID, /* overwrite with board ID number */
>> + FIXUP_ARGPTR, /* overwrite with pointer to kernel args */
>> + FIXUP_ENTRYPOINT, /* overwrite with kernel entry point */
>> + FIXUP_GIC_CPU_IF, /* overwrite with GIC CPU interface address */
>> + FIXUP_BOOTREG, /* overwrite with boot register address */
>> + FIXUP_DSB, /* overwrite with correct DSB insn for cpu */
>> + FIXUP_MAX,
>> +} FixupType;
>> +
>> +typedef struct ARMInsnFixup {
>> + uint32_t insn;
>> + FixupType fixup;
>> +} ARMInsnFixup;
>> +
>> /* The worlds second smallest bootloader. Set r0-r2, then jump to kernel. */
>> -static uint32_t bootloader[] = {
>> - 0xe3a00000, /* mov r0, #0 */
>> - 0xe59f1004, /* ldr r1, [pc, #4] */
>> - 0xe59f2004, /* ldr r2, [pc, #4] */
>> - 0xe59ff004, /* ldr pc, [pc, #4] */
>> - 0, /* Board ID */
>> - 0, /* Address of kernel args. Set by integratorcp_init. */
>> - 0 /* Kernel entry point. Set by integratorcp_init. */
>> +static const ARMInsnFixup bootloader[] = {
>> + { 0xe3a00000 }, /* mov r0, #0 */
>> + { 0xe59f1004 }, /* ldr r1, [pc, #4] */
>> + { 0xe59f2004 }, /* ldr r2, [pc, #4] */
>> + { 0xe59ff004 }, /* ldr pc, [pc, #4] */
>> + { 0, FIXUP_BOARDID },
>> + { 0, FIXUP_ARGPTR },
>> + { 0, FIXUP_ENTRYPOINT },
>> + { 0, FIXUP_TERMINATOR }
>> };
>>
>> /* Handling for secondary CPU boot in a multicore system.
>> @@ -48,39 +66,83 @@ static uint32_t bootloader[] = {
>> #define DSB_INSN 0xf57ff04f
>> #define CP15_DSB_INSN 0xee070f9a /* mcr cp15, 0, r0, c7, c10, 4 */
>>
>> -static uint32_t smpboot[] = {
>> - 0xe59f2028, /* ldr r2, gic_cpu_if */
>> - 0xe59f0028, /* ldr r0, startaddr */
>> - 0xe3a01001, /* mov r1, #1 */
>> - 0xe5821000, /* str r1, [r2] - set GICC_CTLR.Enable */
>> - 0xe3a010ff, /* mov r1, #0xff */
>> - 0xe5821004, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
>> - DSB_INSN, /* dsb */
>> - 0xe320f003, /* wfi */
>> - 0xe5901000, /* ldr r1, [r0] */
>> - 0xe1110001, /* tst r1, r1 */
>> - 0x0afffffb, /* beq <wfi> */
>> - 0xe12fff11, /* bx r1 */
>> - 0, /* gic_cpu_if: base address of GIC CPU interface */
>> - 0 /* bootreg: Boot register address is held here */
>> +static const ARMInsnFixup smpboot[] = {
>> + { 0xe59f2028 }, /* ldr r2, gic_cpu_if */
>> + { 0xe59f0028 }, /* ldr r0, startaddr */
>> + { 0xe3a01001 }, /* mov r1, #1 */
>> + { 0xe5821000 }, /* str r1, [r2] - set GICC_CTLR.Enable */
>> + { 0xe3a010ff }, /* mov r1, #0xff */
>> + { 0xe5821004 }, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
>> + { 0, FIXUP_DSB }, /* dsb */
>> + { 0xe320f003 }, /* wfi */
>> + { 0xe5901000 }, /* ldr r1, [r0] */
>> + { 0xe1110001 }, /* tst r1, r1 */
>> + { 0x0afffffb }, /* beq <wfi> */
>> + { 0xe12fff11 }, /* bx r1 */
>> + { 0, FIXUP_GIC_CPU_IF },
>> + { 0, FIXUP_BOOTREG },
>
> couldn't we add the gic_cpu_if and startaddr "labels" as comments to the
> two lines above? (alternatively also rename the reference to startaddr
> to bootret in the second instruction comment).
Yeah, I figured there wasn't any need to comment since the FIXUP
constant name made the meaning obvious but I forgot about the
reference to the labels in the code comments. I'll change the
startaddr reference to bootreg.
thanks
-- PMM
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 6/7] hw/arm/boot: Add boot support for AArch64 processor
2013-12-16 23:40 ` Christoffer Dall
@ 2013-12-17 0:25 ` Peter Maydell
2013-12-17 4:50 ` Christoffer Dall
0 siblings, 1 reply; 38+ messages in thread
From: Peter Maydell @ 2013-12-17 0:25 UTC (permalink / raw)
To: Christoffer Dall; +Cc: Patch Tracking, QEMU Developers, kvmarm
On 16 December 2013 23:40, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> On Thu, Nov 28, 2013 at 01:33:21PM +0000, Peter Maydell wrote:
>> From: "Mian M. Hamayun" <m.hamayun@virtualopensystems.com>
>>
>> This commit adds support for booting a single AArch64 CPU by setting
>> appropriate registers. The bootloader includes placehoders for Board-ID
>> that are used to implement uniform indexing across different bootloaders.
>>
>> Signed-off-by: Mian M. Hamayun <m.hamayun@virtualopensystems.com>
>> [PMM:
>> * updated to use ARMInsnFixup style bootloader fragments
>> * dropped virt.c additions
>> * use runtime checks for "is this an AArch64 core" rather than ifdefs
>> * drop some unnecessary setting of registers in reset hook
>> ]
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> hw/arm/boot.c | 40 +++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 35 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>> index 77d29a8..b6b04b7 100644
>> --- a/hw/arm/boot.c
>> +++ b/hw/arm/boot.c
>> @@ -19,6 +19,8 @@
>>
>> #define KERNEL_ARGS_ADDR 0x100
>> #define KERNEL_LOAD_ADDR 0x00010000
>> +/* The AArch64 kernel boot protocol specifies a different preferred address */
>> +#define KERNEL64_LOAD_ADDR 0x00080000
>
> I assume you referring to Documentation/arm/Booting and
> Documentation/arm64/booting.txt here? Perhaps it would be nicer to
> refer to that and state how we reach the address for the two archs
> instead of having the aarch64 specific note here, e.g. "The kernel
> recommends booting at offset 0x80000 from system RAM" or something like
> that...
Yeah, we could put the references to the document names in.
I don't see the point repeating the 0x80000 figure in the comment though.
>>
>> typedef enum {
>> FIXUP_NONE = 0, /* do nothing */
>> @@ -37,6 +39,20 @@ typedef struct ARMInsnFixup {
>> FixupType fixup;
>> } ARMInsnFixup;
>>
>> +static const ARMInsnFixup bootloader_aarch64[] = {
>> + { 0x580000c0 }, /* ldr x0, 18 ; Load the lower 32-bits of DTB */
>
> so by 18 you mean the label 0x18 assuming the instruction above has the
> label 0 or something like that? Is this an accepted/familiar notation
> or shoudl we do something like the arm32 bootloaders and "define a
> label in the comments"...?
referring down to a label or something would be better. This is probably
just a copy of output from a disassembler of a binary blob with assumed
offset zero.
thanks
-- PMM
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 7/7] default-configs: Add config for aarch64-softmmu
2013-12-16 23:40 ` Christoffer Dall
@ 2013-12-17 0:27 ` Peter Maydell
2013-12-17 13:33 ` Christopher Covington
1 sibling, 0 replies; 38+ messages in thread
From: Peter Maydell @ 2013-12-17 0:27 UTC (permalink / raw)
To: Christoffer Dall; +Cc: Patch Tracking, QEMU Developers, kvmarm
On 16 December 2013 23:40, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> On Thu, Nov 28, 2013 at 01:33:22PM +0000, Peter Maydell wrote:
>> Add a config for aarch64-softmmu; this enables building of this target.
>> The resulting executable doesn't know about any 64 bit CPUs, but all
>> the 32 bit CPUs and board models work.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> default-configs/aarch64-softmmu.mak | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>> create mode 100644 default-configs/aarch64-softmmu.mak
>>
>> diff --git a/default-configs/aarch64-softmmu.mak b/default-configs/aarch64-softmmu.mak
>> new file mode 100644
>> index 0000000..76a45cc
>> --- /dev/null
>> +++ b/default-configs/aarch64-softmmu.mak
>> @@ -0,0 +1,9 @@
>> +# Default configuration for aarch64-softmmu
>> +
>> +include pci.mak
>> +include usb.mak
>
> pci and usb? pci is required for virtio I presume, and USB? for 32-bit
> boards?
This is a leftover -- we should just include arm-softmmu.mak,
which in turn includes pci.mak and usb.mak for us.
In both cases the requirement is for the 32 bit boards
(lots of things use USB, and versatilepb uses PCI).
thanks
-- PMM
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code
2013-12-13 10:05 ` Peter Maydell
@ 2013-12-17 0:52 ` Peter Crosthwaite
2013-12-17 0:58 ` Peter Maydell
0 siblings, 1 reply; 38+ messages in thread
From: Peter Crosthwaite @ 2013-12-17 0:52 UTC (permalink / raw)
To: Peter Maydell
Cc: Alexander Graf, Mian M. Hamayun, Patch Tracking,
qemu-devel@nongnu.org Developers, kvmarm, Andreas Färber
On Fri, Dec 13, 2013 at 8:05 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 13 December 2013 03:19, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> Why do we need blobs at all? Cant we just fix arm/boot to directly
>> setup the CPU state to the desired? Rather than complex blobs that
>> execute ARM instructions just manipulate the regs directly.
>
> We could in theory do this for the primary bootloader, but
> the secondary CPU blob has to have a loop in it so we
> can sit around waiting for the guest code running in the
> primary to tell us it's time to go:
>
>>> +static const ARMInsnFixup smpboot[] = {
>>> + { 0xe59f2028 }, /* ldr r2, gic_cpu_if */
>>> + { 0xe59f0028 }, /* ldr r0, startaddr */
>>> + { 0xe3a01001 }, /* mov r1, #1 */
>>> + { 0xe5821000 }, /* str r1, [r2] - set GICC_CTLR.Enable */
>>> + { 0xe3a010ff }, /* mov r1, #0xff */
>>> + { 0xe5821004 }, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
>>> + { 0, FIXUP_DSB }, /* dsb */
>>> + { 0xe320f003 }, /* wfi */
>>> + { 0xe5901000 }, /* ldr r1, [r0] */
>>> + { 0xe1110001 }, /* tst r1, r1 */
>>> + { 0x0afffffb }, /* beq <wfi> */
>>> + { 0xe12fff11 }, /* bx r1 */
>>> + { 0, FIXUP_GIC_CPU_IF },
Reading up on Christopher's Kernel documentation link:
Documentation/arm64/booting.txt
Documentation/arm/Booting
I can't see any reference to GIC, where are these GICisms coming from?
>>> + { 0, FIXUP_BOOTREG },
>>> + { 0, FIXUP_TERMINATOR }
>>> };
>
> We're also writing to devices here, and it's cleaner to do that
> by running a guest code instruction than by somehow having
> the boot code ferret around inside the device's implementation
> pre-start, I think.
dma_memory_write(&address_space_memory, ...
Its a level above ferreting, and a level below the machine code blob.
Regards,
Peter
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code
2013-12-17 0:52 ` Peter Crosthwaite
@ 2013-12-17 0:58 ` Peter Maydell
2013-12-17 1:24 ` Peter Crosthwaite
0 siblings, 1 reply; 38+ messages in thread
From: Peter Maydell @ 2013-12-17 0:58 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Alexander Graf, Mian M. Hamayun, Patch Tracking,
qemu-devel@nongnu.org Developers, kvmarm, Andreas Färber
On 17 December 2013 00:52, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Fri, Dec 13, 2013 at 8:05 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 13 December 2013 03:19, Peter Crosthwaite
>> <peter.crosthwaite@xilinx.com> wrote:
>>> Why do we need blobs at all? Cant we just fix arm/boot to directly
>>> setup the CPU state to the desired? Rather than complex blobs that
>>> execute ARM instructions just manipulate the regs directly.
>>
>> We could in theory do this for the primary bootloader, but
>> the secondary CPU blob has to have a loop in it so we
>> can sit around waiting for the guest code running in the
>> primary to tell us it's time to go:
>>
>>>> +static const ARMInsnFixup smpboot[] = {
>>>> + { 0xe59f2028 }, /* ldr r2, gic_cpu_if */
>>>> + { 0xe59f0028 }, /* ldr r0, startaddr */
>>>> + { 0xe3a01001 }, /* mov r1, #1 */
>>>> + { 0xe5821000 }, /* str r1, [r2] - set GICC_CTLR.Enable */
>>>> + { 0xe3a010ff }, /* mov r1, #0xff */
>>>> + { 0xe5821004 }, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
>>>> + { 0, FIXUP_DSB }, /* dsb */
>>>> + { 0xe320f003 }, /* wfi */
>>>> + { 0xe5901000 }, /* ldr r1, [r0] */
>>>> + { 0xe1110001 }, /* tst r1, r1 */
>>>> + { 0x0afffffb }, /* beq <wfi> */
>>>> + { 0xe12fff11 }, /* bx r1 */
>>>> + { 0, FIXUP_GIC_CPU_IF },
>
>
> Reading up on Christopher's Kernel documentation link:
>
> Documentation/arm64/booting.txt
> Documentation/arm/Booting
>
> I can't see any reference to GIC, where are these GICisms coming from?
v7 secondary CPU boot protocol is platform specific,
though most boards seem to do something vaguely
vexpress like. The kernel expects that we've set the
GIC up so that the primary CPU can do an IPI to get
the secondary out of the holding pen loop (that's the
"wfi / ldr /tst / beq" sequence, which loops waiting
for a CPU interrupt).
>>>> + { 0, FIXUP_BOOTREG },
>>>> + { 0, FIXUP_TERMINATOR }
>>>> };
>>
>> We're also writing to devices here, and it's cleaner to do that
>> by running a guest code instruction than by somehow having
>> the boot code ferret around inside the device's implementation
>> pre-start, I think.
>
> dma_memory_write(&address_space_memory, ...
>
> Its a level above ferreting, and a level below the machine code blob.
This doesn't work in the reset hook because you can't guarantee
that this reset hook gets run after the device resets itself rather
than before...
thanks
-- PMM
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code
2013-12-17 0:58 ` Peter Maydell
@ 2013-12-17 1:24 ` Peter Crosthwaite
2013-12-17 4:56 ` Christoffer Dall
2013-12-17 10:31 ` Peter Maydell
0 siblings, 2 replies; 38+ messages in thread
From: Peter Crosthwaite @ 2013-12-17 1:24 UTC (permalink / raw)
To: Peter Maydell, Soren Brinkmann, Nathan Rossi
Cc: Alexander Graf, Mian M. Hamayun, Patch Tracking,
qemu-devel@nongnu.org Developers, kvmarm, Andreas Färber
On Tue, Dec 17, 2013 at 10:58 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 17 December 2013 00:52, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> On Fri, Dec 13, 2013 at 8:05 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 13 December 2013 03:19, Peter Crosthwaite
>>> <peter.crosthwaite@xilinx.com> wrote:
>>>> Why do we need blobs at all? Cant we just fix arm/boot to directly
>>>> setup the CPU state to the desired? Rather than complex blobs that
>>>> execute ARM instructions just manipulate the regs directly.
>>>
>>> We could in theory do this for the primary bootloader, but
>>> the secondary CPU blob has to have a loop in it so we
>>> can sit around waiting for the guest code running in the
>>> primary to tell us it's time to go:
>>>
>>>>> +static const ARMInsnFixup smpboot[] = {
>>>>> + { 0xe59f2028 }, /* ldr r2, gic_cpu_if */
>>>>> + { 0xe59f0028 }, /* ldr r0, startaddr */
>>>>> + { 0xe3a01001 }, /* mov r1, #1 */
>>>>> + { 0xe5821000 }, /* str r1, [r2] - set GICC_CTLR.Enable */
>>>>> + { 0xe3a010ff }, /* mov r1, #0xff */
>>>>> + { 0xe5821004 }, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
>>>>> + { 0, FIXUP_DSB }, /* dsb */
>>>>> + { 0xe320f003 }, /* wfi */
>>>>> + { 0xe5901000 }, /* ldr r1, [r0] */
>>>>> + { 0xe1110001 }, /* tst r1, r1 */
>>>>> + { 0x0afffffb }, /* beq <wfi> */
>>>>> + { 0xe12fff11 }, /* bx r1 */
>>>>> + { 0, FIXUP_GIC_CPU_IF },
>>
>>
>> Reading up on Christopher's Kernel documentation link:
>>
>> Documentation/arm64/booting.txt
>> Documentation/arm/Booting
>>
>> I can't see any reference to GIC, where are these GICisms coming from?
>
> v7 secondary CPU boot protocol is platform specific,
> though most boards seem to do something vaguely
> vexpress like.
So Zynq is very different. It just rewrites the vector table and
resets the secondarys from peripherals rst controller.
The kernel expects that we've set the
> GIC up so that the primary CPU can do an IPI to get
> the secondary out of the holding pen loop (that's the
> "wfi / ldr /tst / beq" sequence, which loops waiting
> for a CPU interrupt).
>
It seems a bit board-specific and an obstacle to ultimately sanitizing
the embedded bootloaders across the architectures (I hope to one day
boot MB and ARM with one BL once I get the arch-isms out of the boot
flow).
I have another problem while we are on the bootstrap discussion - In
Zynq we have some Linux specific bootstrap issues out in device land.
Our clock driver expects the bootloader to setup some state:
https://github.com/Xilinx/meta-xilinx/blob/master/recipes-devtools/qemu/files/HACK_zynq_slcr_Bring_SLCR_out_of_reset_in_kernel_state.patch
This seems similar to the Vexpress GIC requirement - peripherals
needing linux specific setup. Can we solve both problems here with a
bit or framework allowing devs to have an alternate Linux-specific
reset state?
Then we can ditch on the machine code too :)
Regards,
Peter
>>>>> + { 0, FIXUP_BOOTREG },
>>>>> + { 0, FIXUP_TERMINATOR }
>>>>> };
>>>
>>> We're also writing to devices here, and it's cleaner to do that
>>> by running a guest code instruction than by somehow having
>>> the boot code ferret around inside the device's implementation
>>> pre-start, I think.
>>
>> dma_memory_write(&address_space_memory, ...
>>
>> Its a level above ferreting, and a level below the machine code blob.
>
> This doesn't work in the reset hook because you can't guarantee
> that this reset hook gets run after the device resets itself rather
> than before...
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 2/7] target-arm: Clean up handling of AArch64 PSTATE
2013-12-17 0:15 ` Peter Maydell
@ 2013-12-17 4:45 ` Christoffer Dall
2013-12-17 11:42 ` Peter Maydell
0 siblings, 1 reply; 38+ messages in thread
From: Christoffer Dall @ 2013-12-17 4:45 UTC (permalink / raw)
To: Peter Maydell; +Cc: Patch Tracking, QEMU Developers, kvmarm
On Tue, Dec 17, 2013 at 12:15:54AM +0000, Peter Maydell wrote:
> On 16 December 2013 23:39, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> > On Thu, Nov 28, 2013 at 01:33:17PM +0000, Peter Maydell wrote
> >> --- a/target-arm/cpu.h
> >> +++ b/target-arm/cpu.h
> >> @@ -113,8 +113,13 @@ typedef struct CPUARMState {
> >> /* Regs for A64 mode. */
> >> uint64_t xregs[32];
> >> uint64_t pc;
> >> - /* TODO: pstate doesn't correspond to an architectural register;
> >> - * it would be better modelled as the underlying fields.
> >> + /* PSTATE isn't an architectural register for ARMv8. However, it is
> >> + * convenient for us to assemble the underlying state into a 32 bit format
> >> + * identical to the architectural format used for the SPSR. (This is also
> >> + * what the Linux kernel's 'pstate' field in signal handlers and KVM's
> >> + * 'pstate' register are.) NZCV are kept in their split fields; aarch64
> >> + * is an inverted split version of PSTATE.nRW (aka M[4]); other bits are
> >> + * stored here when in AArch64.
> >
> > I really don't understand what you are trying to say beginning with
> > aarch64 is an inverted split version...
>
> When PSTATE.nRW == 1, aarch64 == 0; when PSTATE.nRW == 0,
> aarch64 == 1. That is, aarch64 is the nRW bit inverted. Some descriptions
> of the SPSR format call the nRW bit the 4th bit of the mode field, M[4].
>
ah, got it, thanks.
> > Which other bits are stored
> > where in AArch64?
>
> All the bits not listed specifically in the first half of the sentence, ie
> everything except nzcv and nRW, are stored here, ie in the
> "uint32_t pstate" field this comment is documenting.
ok, it makes sense with the above.
I think this could be written slightly more clearly for the uninitiated,
but maybe I'm just not qemu-savy enough.
>
> > Also what is the rationale behind keeping NZCV in their split fields?
>
> TCG generated code is faster: there are some neat sequences for
> generating the correct flags results from arithmetic and logic ops
> which you can use (and which are the rationale for the rather odd
> definitions of the cpu_NF/ZF/CF/VF). aarch64 we keep separate
> partly for historical reasons and partly because there's a whole
> load of places in the C code that want to test it.
>
ok, I sort of figured when I read the pstate_read/write functions, but
thanks for the clarification.
> >> */
> >> uint32_t pstate;
> >> uint32_t aarch64; /* 1 if CPU is in aarch64 state; inverse of PSTATE.nRW */
> >> +/* Return the current PSTATE value. For the moment we don't support 32<->64 bit
> >> + * interprocessing, so we don't attempt to sync with the cpsr state used by
> >> + * the 32 bit decoder.
> >> + */
> >> +static inline uint32_t pstate_read(CPUARMState *env)
> >> +{
> >> + int ZF;
> >> +
> >> + ZF = (env->ZF == 0);
> >
> > So the comment on the ZF field means "if th ZF field is zero, then the
> > pstate.Z field is set, meaning the result of an op was zero". Crystal
> > clear.
>
> Yes. If you're generating TCG ops this means you can calculate
> the correct value of cpu_ZF by just copying the result into cpu_ZF
> if it's a 32 bit op. 64 bit code is not quite as nice but it's not
> terrible.
>
ok, I think the comment on the ZF field cna be misread in a number of
ways, but it has been around forever, so it was good enough for others I
guess.
Thanks for explaining.
-Christoffer
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 3/7] target-arm: Add minimal KVM AArch64 support
2013-12-17 0:21 ` Peter Maydell
@ 2013-12-17 4:46 ` Christoffer Dall
0 siblings, 0 replies; 38+ messages in thread
From: Christoffer Dall @ 2013-12-17 4:46 UTC (permalink / raw)
To: Peter Maydell; +Cc: Patch Tracking, QEMU Developers, kvmarm
On Tue, Dec 17, 2013 at 12:21:27AM +0000, Peter Maydell wrote:
> On 16 December 2013 23:39, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> > On Thu, Nov 28, 2013 at 01:33:18PM +0000, Peter Maydell wrote:
> >> + ahcc->target = init.target;
> >> + ahcc->dtb_compatible = "arm,arm-v7";
> >
> > arm,arm-v8 ?
>
> Oops, yes, cut-n-pasto.
>
>
> >
> >> +
> >> + kvm_arm_destroy_scratch_host_vcpu(fdarray);
> >> +
> >> + /* We can assume any KVM supporting CPU is at least a v8
> >> + * with VFPv4+Neon; this in turn implies most of the other
> >> + * feature bits.
> >
> > not sure I understand the bit about implying other feature bits, the
> > only other thing we're setting here is AARCH64 and the features bits are
> > enum values?
>
> target-arm/cpu.c:cpu_realize_fn() has a large set of if statements
> like
>
> if (arm_feature(env, ARM_FEATURE_V8)) {
> set_feature(env, ARM_FEATURE_V7);
> set_feature(env, ARM_FEATURE_ARM_DIV);
> set_feature(env, ARM_FEATURE_LPAE);
> }
>
> because architecturally some features or arch versions imply
> that you have others (eg above v8 means we always know
> we have LPAE and division)...
>
> >> + */
> >> + set_feature(&features, ARM_FEATURE_V8);
> >> + set_feature(&features, ARM_FEATURE_VFP4);
> >> + set_feature(&features, ARM_FEATURE_NEON);
> >> + set_feature(&features, ARM_FEATURE_AARCH64);
>
> ...and because presence of the 'v8', 'vfp4', 'neon' features implies
> (as enforced via those if statements) presence of just about every
> other feature it means we don't need to have specific
> tests for "do the CPU's feature registers say we support
> division?" like the v7 KVM code does, because we know
> that it's all implied automatically.
>
Got it, I was looking for something like that, but somehow missed the
realize function. Thanks!
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 6/7] hw/arm/boot: Add boot support for AArch64 processor
2013-12-17 0:25 ` Peter Maydell
@ 2013-12-17 4:50 ` Christoffer Dall
0 siblings, 0 replies; 38+ messages in thread
From: Christoffer Dall @ 2013-12-17 4:50 UTC (permalink / raw)
To: Peter Maydell; +Cc: Patch Tracking, QEMU Developers, kvmarm
On Tue, Dec 17, 2013 at 12:25:43AM +0000, Peter Maydell wrote:
> On 16 December 2013 23:40, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> > On Thu, Nov 28, 2013 at 01:33:21PM +0000, Peter Maydell wrote:
> >> From: "Mian M. Hamayun" <m.hamayun@virtualopensystems.com>
> >>
> >> This commit adds support for booting a single AArch64 CPU by setting
> >> appropriate registers. The bootloader includes placehoders for Board-ID
> >> that are used to implement uniform indexing across different bootloaders.
> >>
> >> Signed-off-by: Mian M. Hamayun <m.hamayun@virtualopensystems.com>
> >> [PMM:
> >> * updated to use ARMInsnFixup style bootloader fragments
> >> * dropped virt.c additions
> >> * use runtime checks for "is this an AArch64 core" rather than ifdefs
> >> * drop some unnecessary setting of registers in reset hook
> >> ]
> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >> ---
> >> hw/arm/boot.c | 40 +++++++++++++++++++++++++++++++++++-----
> >> 1 file changed, 35 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> >> index 77d29a8..b6b04b7 100644
> >> --- a/hw/arm/boot.c
> >> +++ b/hw/arm/boot.c
> >> @@ -19,6 +19,8 @@
> >>
> >> #define KERNEL_ARGS_ADDR 0x100
> >> #define KERNEL_LOAD_ADDR 0x00010000
> >> +/* The AArch64 kernel boot protocol specifies a different preferred address */
> >> +#define KERNEL64_LOAD_ADDR 0x00080000
> >
> > I assume you referring to Documentation/arm/Booting and
> > Documentation/arm64/booting.txt here? Perhaps it would be nicer to
> > refer to that and state how we reach the address for the two archs
> > instead of having the aarch64 specific note here, e.g. "The kernel
> > recommends booting at offset 0x80000 from system RAM" or something like
> > that...
>
> Yeah, we could put the references to the document names in.
> I don't see the point repeating the 0x80000 figure in the comment though.
>
I didn't mean it that literal, just that now it's sort of weird that
there's no comment for 32-bit, but something for the aarch64, which sort
of suggests that any braindead monkey of course knows the 32-bit
details:)
But it's also something that can be fixed later if it creates a
rebasing headache for you at this point.
> >>
> >> typedef enum {
> >> FIXUP_NONE = 0, /* do nothing */
> >> @@ -37,6 +39,20 @@ typedef struct ARMInsnFixup {
> >> FixupType fixup;
> >> } ARMInsnFixup;
> >>
> >> +static const ARMInsnFixup bootloader_aarch64[] = {
> >> + { 0x580000c0 }, /* ldr x0, 18 ; Load the lower 32-bits of DTB */
> >
> > so by 18 you mean the label 0x18 assuming the instruction above has the
> > label 0 or something like that? Is this an accepted/familiar notation
> > or shoudl we do something like the arm32 bootloaders and "define a
> > label in the comments"...?
>
> referring down to a label or something would be better. This is probably
> just a copy of output from a disassembler of a binary blob with assumed
> offset zero.
>
Yeah, I actually dumped your binary values and did a raw disassembly to
check that you weren't lying in the comments, and my disassembler gave
me the hex versions so I sort of figured, but that could be made easier
for the readers who don't want to do that or possess an intimate
knowledge of the aarch64 opcodes in their head.
-Christoffer
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code
2013-12-17 1:24 ` Peter Crosthwaite
@ 2013-12-17 4:56 ` Christoffer Dall
2013-12-17 10:31 ` Peter Maydell
1 sibling, 0 replies; 38+ messages in thread
From: Christoffer Dall @ 2013-12-17 4:56 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Peter Maydell, Patch Tracking, qemu-devel@nongnu.org Developers,
Nathan Rossi, kvmarm, Soren Brinkmann
On Tue, Dec 17, 2013 at 11:24:45AM +1000, Peter Crosthwaite wrote:
> On Tue, Dec 17, 2013 at 10:58 AM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
> > On 17 December 2013 00:52, Peter Crosthwaite
> > <peter.crosthwaite@xilinx.com> wrote:
> >> On Fri, Dec 13, 2013 at 8:05 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> >>> On 13 December 2013 03:19, Peter Crosthwaite
> >>> <peter.crosthwaite@xilinx.com> wrote:
> >>>> Why do we need blobs at all? Cant we just fix arm/boot to directly
> >>>> setup the CPU state to the desired? Rather than complex blobs that
> >>>> execute ARM instructions just manipulate the regs directly.
> >>>
> >>> We could in theory do this for the primary bootloader, but
> >>> the secondary CPU blob has to have a loop in it so we
> >>> can sit around waiting for the guest code running in the
> >>> primary to tell us it's time to go:
> >>>
> >>>>> +static const ARMInsnFixup smpboot[] = {
> >>>>> + { 0xe59f2028 }, /* ldr r2, gic_cpu_if */
> >>>>> + { 0xe59f0028 }, /* ldr r0, startaddr */
> >>>>> + { 0xe3a01001 }, /* mov r1, #1 */
> >>>>> + { 0xe5821000 }, /* str r1, [r2] - set GICC_CTLR.Enable */
> >>>>> + { 0xe3a010ff }, /* mov r1, #0xff */
> >>>>> + { 0xe5821004 }, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
> >>>>> + { 0, FIXUP_DSB }, /* dsb */
> >>>>> + { 0xe320f003 }, /* wfi */
> >>>>> + { 0xe5901000 }, /* ldr r1, [r0] */
> >>>>> + { 0xe1110001 }, /* tst r1, r1 */
> >>>>> + { 0x0afffffb }, /* beq <wfi> */
> >>>>> + { 0xe12fff11 }, /* bx r1 */
> >>>>> + { 0, FIXUP_GIC_CPU_IF },
> >>
> >>
> >> Reading up on Christopher's Kernel documentation link:
> >>
> >> Documentation/arm64/booting.txt
> >> Documentation/arm/Booting
> >>
> >> I can't see any reference to GIC, where are these GICisms coming from?
> >
> > v7 secondary CPU boot protocol is platform specific,
> > though most boards seem to do something vaguely
> > vexpress like.
>
> So Zynq is very different. It just rewrites the vector table and
> resets the secondarys from peripherals rst controller.
>
> > The kernel expects that we've set the
> > GIC up so that the primary CPU can do an IPI to get
> > the secondary out of the holding pen loop (that's the
> > "wfi / ldr /tst / beq" sequence, which loops waiting
> > for a CPU interrupt).
> >
>
> It seems a bit board-specific and an obstacle to ultimately sanitizing
> the embedded bootloaders across the architectures (I hope to one day
> boot MB and ARM with one BL once I get the arch-isms out of the boot
> flow).
ARM is already making a good effort at this with PSCI. However,
supporting this in TCG requires some secure state / hyp mode emulation
that we don't have currently, afaik.
>
> I have another problem while we are on the bootstrap discussion - In
> Zynq we have some Linux specific bootstrap issues out in device land.
> Our clock driver expects the bootloader to setup some state:
>
> https://github.com/Xilinx/meta-xilinx/blob/master/recipes-devtools/qemu/files/HACK_zynq_slcr_Bring_SLCR_out_of_reset_in_kernel_state.patch
>
> This seems similar to the Vexpress GIC requirement - peripherals
> needing linux specific setup. Can we solve both problems here with a
> bit or framework allowing devs to have an alternate Linux-specific
> reset state?
>
> Then we can ditch on the machine code too :)
>
> Regards,
> Peter
>
> >>>>> + { 0, FIXUP_BOOTREG },
> >>>>> + { 0, FIXUP_TERMINATOR }
> >>>>> };
> >>>
> >>> We're also writing to devices here, and it's cleaner to do that
> >>> by running a guest code instruction than by somehow having
> >>> the boot code ferret around inside the device's implementation
> >>> pre-start, I think.
> >>
> >> dma_memory_write(&address_space_memory, ...
> >>
> >> Its a level above ferreting, and a level below the machine code blob.
> >
> > This doesn't work in the reset hook because you can't guarantee
> > that this reset hook gets run after the device resets itself rather
> > than before...
> >
> > thanks
> > -- PMM
> >
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
--
Christoffer
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code
2013-12-17 1:24 ` Peter Crosthwaite
2013-12-17 4:56 ` Christoffer Dall
@ 2013-12-17 10:31 ` Peter Maydell
2013-12-17 11:36 ` Peter Crosthwaite
1 sibling, 1 reply; 38+ messages in thread
From: Peter Maydell @ 2013-12-17 10:31 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Mian M. Hamayun, Patch Tracking,
qemu-devel@nongnu.org Developers, Alexander Graf,
Andreas Färber, Nathan Rossi, kvmarm, Soren Brinkmann
On 17 December 2013 01:24, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Tue, Dec 17, 2013 at 10:58 AM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
>> On 17 December 2013 00:52, Peter Crosthwaite
>> <peter.crosthwaite@xilinx.com> wrote:
>>> On Fri, Dec 13, 2013 at 8:05 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>>> On 13 December 2013 03:19, Peter Crosthwaite
>>>> <peter.crosthwaite@xilinx.com> wrote:
>>>>> Why do we need blobs at all? Cant we just fix arm/boot to directly
>>>>> setup the CPU state to the desired? Rather than complex blobs that
>>>>> execute ARM instructions just manipulate the regs directly.
>>>>
>>>> We could in theory do this for the primary bootloader, but
>>>> the secondary CPU blob has to have a loop in it so we
>>>> can sit around waiting for the guest code running in the
>>>> primary to tell us it's time to go:
>>>>
>>>>>> +static const ARMInsnFixup smpboot[] = {
>>>>>> + { 0xe59f2028 }, /* ldr r2, gic_cpu_if */
>>>>>> + { 0xe59f0028 }, /* ldr r0, startaddr */
>>>>>> + { 0xe3a01001 }, /* mov r1, #1 */
>>>>>> + { 0xe5821000 }, /* str r1, [r2] - set GICC_CTLR.Enable */
>>>>>> + { 0xe3a010ff }, /* mov r1, #0xff */
>>>>>> + { 0xe5821004 }, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
>>>>>> + { 0, FIXUP_DSB }, /* dsb */
>>>>>> + { 0xe320f003 }, /* wfi */
>>>>>> + { 0xe5901000 }, /* ldr r1, [r0] */
>>>>>> + { 0xe1110001 }, /* tst r1, r1 */
>>>>>> + { 0x0afffffb }, /* beq <wfi> */
>>>>>> + { 0xe12fff11 }, /* bx r1 */
>>>>>> + { 0, FIXUP_GIC_CPU_IF },
>>>
>>>
>>> Reading up on Christopher's Kernel documentation link:
>>>
>>> Documentation/arm64/booting.txt
>>> Documentation/arm/Booting
>>>
>>> I can't see any reference to GIC, where are these GICisms coming from?
>>
>> v7 secondary CPU boot protocol is platform specific,
>> though most boards seem to do something vaguely
>> vexpress like.
>
> So Zynq is very different. It just rewrites the vector table and
> resets the secondarys from peripherals rst controller.
Yeah. This is why boot.c supports letting the board provide its
own secondary boot code string (used by exynos and highbank).
> The kernel expects that we've set the
>> GIC up so that the primary CPU can do an IPI to get
>> the secondary out of the holding pen loop (that's the
>> "wfi / ldr /tst / beq" sequence, which loops waiting
>> for a CPU interrupt).
>>
>
> It seems a bit board-specific and an obstacle to ultimately sanitizing
> the embedded bootloaders across the architectures (I hope to one day
> boot MB and ARM with one BL once I get the arch-isms out of the boot
> flow).
>
> I have another problem while we are on the bootstrap discussion - In
> Zynq we have some Linux specific bootstrap issues out in device land.
> Our clock driver expects the bootloader to setup some state:
>
> https://github.com/Xilinx/meta-xilinx/blob/master/recipes-devtools/qemu/files/HACK_zynq_slcr_Bring_SLCR_out_of_reset_in_kernel_state.patch
>
> This seems similar to the Vexpress GIC requirement - peripherals
> needing linux specific setup. Can we solve both problems here with a
> bit or framework allowing devs to have an alternate Linux-specific
> reset state?
>
> Then we can ditch on the machine code too :)
I'm not convinced about this at all -- this would be letting the
"I know about booting Linux" code spread out from boot.c
where it currently is into every device. That sounds like a bad idea.
-- PMM
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code
2013-12-17 10:31 ` Peter Maydell
@ 2013-12-17 11:36 ` Peter Crosthwaite
2013-12-17 11:47 ` Peter Maydell
0 siblings, 1 reply; 38+ messages in thread
From: Peter Crosthwaite @ 2013-12-17 11:36 UTC (permalink / raw)
To: Peter Maydell
Cc: Mian M. Hamayun, Patch Tracking, Alexander Graf,
qemu-devel@nongnu.org Developers, kvmarm, Nathan Rossi,
Andreas Färber, Soren Brinkmann
On Tue, Dec 17, 2013 at 8:31 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 17 December 2013 01:24, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> On Tue, Dec 17, 2013 at 10:58 AM, Peter Maydell
>> <peter.maydell@linaro.org> wrote:
>>> On 17 December 2013 00:52, Peter Crosthwaite
>>> <peter.crosthwaite@xilinx.com> wrote:
>>>> On Fri, Dec 13, 2013 at 8:05 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>>>> On 13 December 2013 03:19, Peter Crosthwaite
>>>>> <peter.crosthwaite@xilinx.com> wrote:
>>>>>> Why do we need blobs at all? Cant we just fix arm/boot to directly
>>>>>> setup the CPU state to the desired? Rather than complex blobs that
>>>>>> execute ARM instructions just manipulate the regs directly.
>>>>>
>>>>> We could in theory do this for the primary bootloader, but
>>>>> the secondary CPU blob has to have a loop in it so we
>>>>> can sit around waiting for the guest code running in the
>>>>> primary to tell us it's time to go:
>>>>>
>>>>>>> +static const ARMInsnFixup smpboot[] = {
>>>>>>> + { 0xe59f2028 }, /* ldr r2, gic_cpu_if */
>>>>>>> + { 0xe59f0028 }, /* ldr r0, startaddr */
>>>>>>> + { 0xe3a01001 }, /* mov r1, #1 */
>>>>>>> + { 0xe5821000 }, /* str r1, [r2] - set GICC_CTLR.Enable */
>>>>>>> + { 0xe3a010ff }, /* mov r1, #0xff */
>>>>>>> + { 0xe5821004 }, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
>>>>>>> + { 0, FIXUP_DSB }, /* dsb */
>>>>>>> + { 0xe320f003 }, /* wfi */
>>>>>>> + { 0xe5901000 }, /* ldr r1, [r0] */
>>>>>>> + { 0xe1110001 }, /* tst r1, r1 */
>>>>>>> + { 0x0afffffb }, /* beq <wfi> */
>>>>>>> + { 0xe12fff11 }, /* bx r1 */
>>>>>>> + { 0, FIXUP_GIC_CPU_IF },
>>>>
>>>>
>>>> Reading up on Christopher's Kernel documentation link:
>>>>
>>>> Documentation/arm64/booting.txt
>>>> Documentation/arm/Booting
>>>>
>>>> I can't see any reference to GIC, where are these GICisms coming from?
>>>
>>> v7 secondary CPU boot protocol is platform specific,
>>> though most boards seem to do something vaguely
>>> vexpress like.
>>
>> So Zynq is very different. It just rewrites the vector table and
>> resets the secondarys from peripherals rst controller.
>
> Yeah. This is why boot.c supports letting the board provide its
> own secondary boot code string (used by exynos and highbank).
>
>> The kernel expects that we've set the
>>> GIC up so that the primary CPU can do an IPI to get
>>> the secondary out of the holding pen loop (that's the
>>> "wfi / ldr /tst / beq" sequence, which loops waiting
>>> for a CPU interrupt).
>>>
>>
>> It seems a bit board-specific and an obstacle to ultimately sanitizing
>> the embedded bootloaders across the architectures (I hope to one day
>> boot MB and ARM with one BL once I get the arch-isms out of the boot
>> flow).
>>
>> I have another problem while we are on the bootstrap discussion - In
>> Zynq we have some Linux specific bootstrap issues out in device land.
>> Our clock driver expects the bootloader to setup some state:
>>
>> https://github.com/Xilinx/meta-xilinx/blob/master/recipes-devtools/qemu/files/HACK_zynq_slcr_Bring_SLCR_out_of_reset_in_kernel_state.patch
>>
>> This seems similar to the Vexpress GIC requirement - peripherals
>> needing linux specific setup. Can we solve both problems here with a
>> bit or framework allowing devs to have an alternate Linux-specific
>> reset state?
>>
>> Then we can ditch on the machine code too :)
>
> I'm not convinced about this at all -- this would be letting the
> "I know about booting Linux" code spread out from boot.c
> where it currently is into every device. That sounds like a bad idea. -
>
So the "I know about boot linux" code already has to span multiple
architectures it's already reasonably 'spread out' just looking at the
zoo of Linux bootloaders. Its also a contributor to the much disliked
inconsistencies in boot flow between archs.
If we can outsource arch/board specific bringup to device land it
would allow consolidation of all the different Linux bootloaders. Or
at least across embedded - E.g. Microblaze, ARM, some of the board
level PPC bootloaders O.R. and the newly posted Moxie machine.
ARM_CPU / MICROBLAZE_CPU would be responsible for setting dtb reg ptr
/ kernel args, program entry point.
ARM_GIC would set itself up.
ZYNQ_SLCR would set itself up.
QOM interfaces can make this purely additive and unobtrusive. E.G.
interface TYPE_LINUX_RESET implements a reset hook that runs iff the
system is booting Linux.
Anyways,
we are not going to solve this today and at the end of the day, the
patch does make this much more self documenting and flexible for ARM,
so:
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Regards,
Peter
> -- PMM
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 2/7] target-arm: Clean up handling of AArch64 PSTATE
2013-12-17 4:45 ` Christoffer Dall
@ 2013-12-17 11:42 ` Peter Maydell
2013-12-17 18:44 ` Christoffer Dall
0 siblings, 1 reply; 38+ messages in thread
From: Peter Maydell @ 2013-12-17 11:42 UTC (permalink / raw)
To: Christoffer Dall; +Cc: Patch Tracking, QEMU Developers, kvmarm
On 17 December 2013 04:45, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> I think this could be written slightly more clearly for the uninitiated,
> but maybe I'm just not qemu-savy enough.
It was a bit compressed; I've reworded it to:
/* PSTATE isn't an architectural register for ARMv8. However, it is
* convenient for us to assemble the underlying state into a 32 bit format
* identical to the architectural format used for the SPSR. (This is also
* what the Linux kernel's 'pstate' field in signal handlers and KVM's
* 'pstate' register are.) Of the PSTATE bits:
* NZCV are kept in the split out env->CF/VF/NF/ZF, (which have the same
* semantics as for AArch32, as described in the comments on each field)
* nRW (also known as M[4]) is kept, inverted, in env->aarch64
* all other bits are stored in their correct places in env->pstate
*/
thanks
-- PMM
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code
2013-12-17 11:36 ` Peter Crosthwaite
@ 2013-12-17 11:47 ` Peter Maydell
2013-12-17 12:02 ` Peter Crosthwaite
0 siblings, 1 reply; 38+ messages in thread
From: Peter Maydell @ 2013-12-17 11:47 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Mian M. Hamayun, Patch Tracking, Alexander Graf,
qemu-devel@nongnu.org Developers, kvmarm, Nathan Rossi,
Andreas Färber, Soren Brinkmann
On 17 December 2013 11:36, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Tue, Dec 17, 2013 at 8:31 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> I'm not convinced about this at all -- this would be letting the
>> "I know about booting Linux" code spread out from boot.c
>> where it currently is into every device. That sounds like a bad idea. -
>>
>
> So the "I know about boot linux" code already has to span multiple
> architectures it's already reasonably 'spread out' just looking at the
> zoo of Linux bootloaders. Its also a contributor to the much disliked
> inconsistencies in boot flow between archs.
>
> If we can outsource arch/board specific bringup to device land it
> would allow consolidation of all the different Linux bootloaders. Or
> at least across embedded - E.g. Microblaze, ARM, some of the board
> level PPC bootloaders O.R. and the newly posted Moxie machine.
>
> ARM_CPU / MICROBLAZE_CPU would be responsible for setting dtb reg ptr
> / kernel args, program entry point.
> ARM_GIC would set itself up.
...but how? What the GIC state needs to be depends on the board.
It's times like this I see the appeal of pushing the whole thing off onto
a custom "firmware blob" which does the work :-)
> we are not going to solve this today and at the end of the day, the
> patch does make this much more self documenting and flexible for ARM,
> so:
>
> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Thanks.
-- PMM
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code
2013-12-17 11:47 ` Peter Maydell
@ 2013-12-17 12:02 ` Peter Crosthwaite
0 siblings, 0 replies; 38+ messages in thread
From: Peter Crosthwaite @ 2013-12-17 12:02 UTC (permalink / raw)
To: Peter Maydell
Cc: Mian M. Hamayun, Patch Tracking,
qemu-devel@nongnu.org Developers, Alexander Graf,
Andreas Färber, Nathan Rossi, kvmarm, Soren Brinkmann
On Tue, Dec 17, 2013 at 9:47 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 17 December 2013 11:36, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> On Tue, Dec 17, 2013 at 8:31 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> I'm not convinced about this at all -- this would be letting the
>>> "I know about booting Linux" code spread out from boot.c
>>> where it currently is into every device. That sounds like a bad idea. -
>>>
>>
>> So the "I know about boot linux" code already has to span multiple
>> architectures it's already reasonably 'spread out' just looking at the
>> zoo of Linux bootloaders. Its also a contributor to the much disliked
>> inconsistencies in boot flow between archs.
>>
>> If we can outsource arch/board specific bringup to device land it
>> would allow consolidation of all the different Linux bootloaders. Or
>> at least across embedded - E.g. Microblaze, ARM, some of the board
>> level PPC bootloaders O.R. and the newly posted Moxie machine.
>>
>> ARM_CPU / MICROBLAZE_CPU would be responsible for setting dtb reg ptr
>> / kernel args, program entry point.
>> ARM_GIC would set itself up.
>
> ...but how? What the GIC state needs to be depends on the board.
>
In such cases, boards could trivially define a lightweight QOM child class
(of the device in question) that implements the linux_reset() hook their
specific way:
TypeInfo vexpress_gic_info {
.parent = TYPE_ARM_GIC,
.interfaces = {
{ TYPE_LINUX_RESET },
{}
},
.class_init = vexpress_gic_class_init,
}
Regards,
Peter
> It's times like this I see the appeal of pushing the whole thing off onto
> a custom "firmware blob" which does the work :-)
>
>> we are not going to solve this today and at the end of the day, the
>> patch does make this much more self documenting and flexible for ARM,
>> so:
>>
>> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> Thanks.
>
> -- PMM
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 7/7] default-configs: Add config for aarch64-softmmu
2013-12-16 23:40 ` Christoffer Dall
2013-12-17 0:27 ` Peter Maydell
@ 2013-12-17 13:33 ` Christopher Covington
1 sibling, 0 replies; 38+ messages in thread
From: Christopher Covington @ 2013-12-17 13:33 UTC (permalink / raw)
To: Christoffer Dall; +Cc: Peter Maydell, kvmarm, qemu-devel, patches
On 12/16/2013 06:40 PM, Christoffer Dall wrote:
> On Thu, Nov 28, 2013 at 01:33:22PM +0000, Peter Maydell wrote:
>> Add a config for aarch64-softmmu; this enables building of this target.
>> The resulting executable doesn't know about any 64 bit CPUs, but all
>> the 32 bit CPUs and board models work.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> default-configs/aarch64-softmmu.mak | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>> create mode 100644 default-configs/aarch64-softmmu.mak
>>
>> diff --git a/default-configs/aarch64-softmmu.mak b/default-configs/aarch64-softmmu.mak
>> new file mode 100644
>> index 0000000..76a45cc
>> --- /dev/null
>> +++ b/default-configs/aarch64-softmmu.mak
>> @@ -0,0 +1,9 @@
>> +# Default configuration for aarch64-softmmu
>> +
>> +include pci.mak
>> +include usb.mak
>
> pci and usb? pci is required for virtio I presume, and USB? for 32-bit
> boards?
PCI is not required for VirtIO since the introduction of VirtIO-MMIO.
Christopher
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by the Linux Foundation.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 2/7] target-arm: Clean up handling of AArch64 PSTATE
2013-12-17 11:42 ` Peter Maydell
@ 2013-12-17 18:44 ` Christoffer Dall
0 siblings, 0 replies; 38+ messages in thread
From: Christoffer Dall @ 2013-12-17 18:44 UTC (permalink / raw)
To: Peter Maydell; +Cc: Patch Tracking, QEMU Developers, kvmarm
On Tue, Dec 17, 2013 at 11:42:42AM +0000, Peter Maydell wrote:
> On 17 December 2013 04:45, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> > I think this could be written slightly more clearly for the uninitiated,
> > but maybe I'm just not qemu-savy enough.
>
> It was a bit compressed; I've reworded it to:
> /* PSTATE isn't an architectural register for ARMv8. However, it is
> * convenient for us to assemble the underlying state into a 32 bit format
> * identical to the architectural format used for the SPSR. (This is also
> * what the Linux kernel's 'pstate' field in signal handlers and KVM's
> * 'pstate' register are.) Of the PSTATE bits:
> * NZCV are kept in the split out env->CF/VF/NF/ZF, (which have the same
> * semantics as for AArch32, as described in the comments on each field)
> * nRW (also known as M[4]) is kept, inverted, in env->aarch64
> * all other bits are stored in their correct places in env->pstate
> */
>
Much clearer, thanks!
--
Christoffer
^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2013-12-17 18:45 UTC | newest]
Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-28 13:33 [Qemu-devel] [PATCH 0/7] target-arm: Support AArch64 KVM Peter Maydell
2013-11-28 13:33 ` [Qemu-devel] [PATCH 1/7] target-arm/kvm: Split 32 bit only code into its own file Peter Maydell
2013-12-16 23:39 ` Christoffer Dall
2013-11-28 13:33 ` [Qemu-devel] [PATCH 2/7] target-arm: Clean up handling of AArch64 PSTATE Peter Maydell
2013-12-16 23:39 ` Christoffer Dall
2013-12-17 0:15 ` Peter Maydell
2013-12-17 4:45 ` Christoffer Dall
2013-12-17 11:42 ` Peter Maydell
2013-12-17 18:44 ` Christoffer Dall
2013-11-28 13:33 ` [Qemu-devel] [PATCH 3/7] target-arm: Add minimal KVM AArch64 support Peter Maydell
2013-12-16 23:39 ` Christoffer Dall
2013-12-17 0:21 ` Peter Maydell
2013-12-17 4:46 ` Christoffer Dall
2013-11-28 13:33 ` [Qemu-devel] [PATCH 4/7] configure: Enable KVM for aarch64 host/target combination Peter Maydell
2013-12-16 23:40 ` Christoffer Dall
2013-11-28 13:33 ` [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code Peter Maydell
2013-12-13 3:19 ` Peter Crosthwaite
2013-12-13 10:05 ` Peter Maydell
2013-12-17 0:52 ` Peter Crosthwaite
2013-12-17 0:58 ` Peter Maydell
2013-12-17 1:24 ` Peter Crosthwaite
2013-12-17 4:56 ` Christoffer Dall
2013-12-17 10:31 ` Peter Maydell
2013-12-17 11:36 ` Peter Crosthwaite
2013-12-17 11:47 ` Peter Maydell
2013-12-17 12:02 ` Peter Crosthwaite
2013-12-16 23:40 ` Christoffer Dall
2013-12-17 0:23 ` Peter Maydell
2013-11-28 13:33 ` [Qemu-devel] [PATCH 6/7] hw/arm/boot: Add boot support for AArch64 processor Peter Maydell
2013-12-16 23:40 ` Christoffer Dall
2013-12-17 0:25 ` Peter Maydell
2013-12-17 4:50 ` Christoffer Dall
2013-11-28 13:33 ` [Qemu-devel] [PATCH 7/7] default-configs: Add config for aarch64-softmmu Peter Maydell
2013-12-16 23:40 ` Christoffer Dall
2013-12-17 0:27 ` Peter Maydell
2013-12-17 13:33 ` Christopher Covington
2013-12-05 15:23 ` [Qemu-devel] [PATCH 0/7] target-arm: Support AArch64 KVM Peter Maydell
2013-12-12 16:41 ` Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.