[Qemu-devel] [PATCH v2 0/7] AARCH64 support on machvirt machine model using KVM

[Qemu-devel] [PATCH v2 0/7] AARCH64 support on machvirt machine model using KVM

[Mian M. Hamayun]

From: "Mian M. Hamayun" <m.hamayun@virtualopensystems.com>

This is the v2 of patch series that implements KVM support in QEMU for the ARMv8
Cortex A57 CPU. It depends on the previously submitted AArch64 preparation patch
series v5 and machvirt patches, and uses the already available KVM in-kernel GIC
support. Current implementation for 64-bit guests only, and 32-bit guest support
will be introduced in near future.

As a reference, KVM Tool and the AArch64 bootwrapper were used, as well as
public documentation from ARM. The following work has been tested with SMP
capabilities, under ARMv8 Fast and Foundation Models (Open Embedded userspace
with an emulated MMC).

The v1 of this patch series related to AArch64 CPU model for Versatile Express
was sponsored by Huawei, and developed in collaboration between Huawei
Technologies Duesseldorf GmbH - European Research Center Munich (ERC) and
Virtual Open Systems.

A working tree of this implementation is available on the "kvm-aarch64-v2"
branch of the following github repository.

https://github.com/virtualopensystems/qemu/tree/kvm-aarch64-v2

Summary of Changes:

Changes v1 -> v2
 * Based on AArch64 Preparation Patchset V5 and machvirt patches.
 * Implemented for Machvirt Machine Model.
 * Architecture-specific CPU initialization code improved. Removed hardcoding
   from register set/get loops and introduced CPU target type array to find
   appropriate ARMv8 CPU type supported by KVM.
 * Disable the PSCI method in case of AArch64 and use the spin-table method
   instead for booting secondary CPUs.
 * 32-bit guest support still missing

v1
 * Based on AArch64 Preparation Patchset V4
 * Implemented for Versatile Express Machine Model
 * Support for SMP using bootcode injection
 * No 32-bit guest support

Alexander Spyridakis (1):
  AARCH64: Add SMP support for aarch64 processors

Mian M. Hamayun (6):
  AARCH64: Add A57 CPU to default AArch64 configuration and enable KVM
  Add the additional parent parameter to memory region init calls
  AARCH64: Add aarch64 CPU initialization, get and put registers
    support
  AARCH64: Add boot support for aarch64 processor
  AARCH64: Disable the non-aarch64 specific reset code
  AARCH64: Use the spin-table method for booting secondary processors
    in machvirt

 configure                           |    2 +-
 default-configs/aarch64-softmmu.mak |    3 +-
 hw/arm/boot.c                       |   62 +++++++++++++++--
 hw/arm/virt.c                       |   16 ++++-
 hw/cpu/a57mpcore.c                  |    2 +-
 linux-headers/linux/kvm.h           |    1 +
 target-arm/kvm.c                    |  127 +++++++++++++++++++++++++++++++++++
 7 files changed, 201 insertions(+), 12 deletions(-)

-- 
1.7.9.5

[Qemu-devel] [PATCH v2 1/7] AARCH64: Add A57 CPU to default AArch64 configuration and enable KVM

[Mian M. Hamayun]

From: "Mian M. Hamayun" <m.hamayun@virtualopensystems.com>

Introduce the A57 cpu to the default AArch64 configuration and enable KVM for
64-bit guests only.

Signed-off-by: Mian M. Hamayun <m.hamayun@virtualopensystems.com>
---
 configure                           |    2 +-
 default-configs/aarch64-softmmu.mak |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 2f83771..021c1dd 100755
--- a/configure
+++ b/configure
@@ -4293,7 +4293,7 @@ case "$target_name" in
   *)
 esac
 case "$target_name" in
-  arm|i386|x86_64|ppcemb|ppc|ppc64|s390x)
+  arm|aarch64|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 \
diff --git a/default-configs/aarch64-softmmu.mak b/default-configs/aarch64-softmmu.mak
index 27cbe3d..00c8bdd 100644
--- a/default-configs/aarch64-softmmu.mak
+++ b/default-configs/aarch64-softmmu.mak
@@ -1,4 +1,4 @@
-# Default configuration for arm-softmmu
+# Default configuration for aarch64-softmmu
 
 include pci.mak
 include usb.mak
@@ -37,6 +37,7 @@ CONFIG_USB_MUSB=y
 CONFIG_ARM9MPCORE=y
 CONFIG_ARM11MPCORE=y
 CONFIG_ARM15MPCORE=y
+CONFIG_ARM57MPCORE=y
 
 CONFIG_ARM_GIC=y
 CONFIG_ARM_GIC_KVM=$(CONFIG_KVM)
-- 
1.7.9.5

[Qemu-devel] [PATCH v2 2/7] Add the additional parent parameter to memory region init calls

[Mian M. Hamayun]

From: "Mian M. Hamayun" <m.hamayun@virtualopensystems.com>

The memory region init calls require an additional parent parameter, so
introduce a null parent parameter to make it happy.

Signed-off-by: Mian M. Hamayun <m.hamayun@virtualopensystems.com>
---
 hw/arm/virt.c      |    2 +-
 hw/cpu/a57mpcore.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 97712d7..8a2bdc7 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -407,7 +407,7 @@ static void machvirt_init(QEMUMachineInitArgs *args)
     }
     fdt_add_cpu_nodes(virt_fdt, mi, smp_cpus);
 
-    memory_region_init_ram(ram, "mach-virt.ram", ram_size);
+    memory_region_init_ram(ram, NULL, "mach-virt.ram", ram_size);
     vmstate_register_ram_global(ram);
     memory_region_add_subregion(sysmem, mi->mem_base, ram);
 
diff --git a/hw/cpu/a57mpcore.c b/hw/cpu/a57mpcore.c
index 2923a2a..1ab6dc0 100644
--- a/hw/cpu/a57mpcore.c
+++ b/hw/cpu/a57mpcore.c
@@ -70,7 +70,7 @@ static int a57mp_priv_init(SysBusDevice *dev)
      *  0x5000-0x5fff -- GIC virtual interface control (not modelled)
      *  0x6000-0x7fff -- GIC virtual CPU interface (not modelled)
      */
-    memory_region_init(&s->container, "a57mp-priv-container", 0x8000);
+    memory_region_init(&s->container, NULL, "a57mp-priv-container", 0x8000);
     memory_region_add_subregion(&s->container, 0x1000,
                                 sysbus_mmio_get_region(busdev, 0));
     memory_region_add_subregion(&s->container, 0x2000,
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH v2 2/7] Add the additional parent parameter to memory region init calls

[Andreas Färber]

Am 23.07.2013 11:33, schrieb Mian M. Hamayun:
> From: "Mian M. Hamayun" <m.hamayun@virtualopensystems.com>
> 
> The memory region init calls require an additional parent parameter, so
> introduce a null parent parameter to make it happy.
> 
> Signed-off-by: Mian M. Hamayun <m.hamayun@virtualopensystems.com>

This is not OK for something labelled "PATCH". Patch series need to be
bisectable, not fixing up earlier patch series that have not been
applied yet.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v2 2/7] Add the additional parent parameter to memory region init calls

[Peter Maydell]

On 23 July 2013 10:43, Andreas Färber <afaerber@suse.de> wrote:
> Am 23.07.2013 11:33, schrieb Mian M. Hamayun:
>> From: "Mian M. Hamayun" <m.hamayun@virtualopensystems.com>
>>
>> The memory region init calls require an additional parent parameter, so
>> introduce a null parent parameter to make it happy.
>>
>> Signed-off-by: Mian M. Hamayun <m.hamayun@virtualopensystems.com>
>
> This is not OK for something labelled "PATCH". Patch series need to be
> bisectable, not fixing up earlier patch series that have not been
> applied yet.

I have a rebased version of John's mach-virt patch which includes
these fixes; I haven't sent it out yet because I've still been
pondering whether the "create device tree nodes for everything"
code can be made less ugly...

-- PMM

Re: [Qemu-devel] [PATCH v2 2/7] Add the additional parent parameter to memory region init calls

[Andreas Färber]

Am 23.07.2013 12:00, schrieb Peter Maydell:
> On 23 July 2013 10:43, Andreas Färber <afaerber@suse.de> wrote:
>> Am 23.07.2013 11:33, schrieb Mian M. Hamayun:
>>> From: "Mian M. Hamayun" <m.hamayun@virtualopensystems.com>
>>>
>>> The memory region init calls require an additional parent parameter, so
>>> introduce a null parent parameter to make it happy.
>>>
>>> Signed-off-by: Mian M. Hamayun <m.hamayun@virtualopensystems.com>
>>
>> This is not OK for something labelled "PATCH". Patch series need to be
>> bisectable, not fixing up earlier patch series that have not been
>> applied yet.
> 
> I have a rebased version of John's mach-virt patch which includes
> these fixes; I haven't sent it out yet because I've still been
> pondering whether the "create device tree nodes for everything"
> code can be made less ugly...

I'd also appreciate if you would update cpu/a57core.c wrt the container
MemoryRegion and QOM realize (still a SysBus initfn here) - that was the
intent of my a15mpcore patches I cc'ed all aarch64 people on.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH v2 3/7] AARCH64: Add aarch64 CPU initialization, get and put registers support

[Mian M. Hamayun]

From: "Mian M. Hamayun" <m.hamayun@virtualopensystems.com>

The cpu init function tries to initialize with all possible cpu types, as
KVM does not provide a means to detect the real cpu type and simply refuses
to initialize on cpu type mis-match. By using the loop based init function,
we avoid the need to modify code if the underlying platform is different,
such as Fast Models instead of Foundation Models.

Get and Put Registers deal with the basic state of Aarch64 CPUs for the moment.

Signed-off-by: Mian M. Hamayun <m.hamayun@virtualopensystems.com>

Conflicts:

	target-arm/kvm.c

Conflicts:

	target-arm/cpu.c
---
 linux-headers/linux/kvm.h |    1 +
 target-arm/kvm.c          |  125 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 126 insertions(+)

diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index c614070..4df5292 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -783,6 +783,7 @@ struct kvm_dirty_tlb {
 #define KVM_REG_IA64		0x3000000000000000ULL
 #define KVM_REG_ARM		0x4000000000000000ULL
 #define KVM_REG_S390		0x5000000000000000ULL
+#define KVM_REG_ARM64          0x6000000000000000ULL
 
 #define KVM_REG_SIZE_SHIFT	52
 #define KVM_REG_SIZE_MASK	0x00f0000000000000ULL
diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index b92e00d..c96b871 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -32,6 +32,11 @@
 #error mismatch between cpu.h and KVM header definitions
 #endif
 
+#ifdef TARGET_AARCH64
+#define AARCH64_CORE_REG(x)   (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \
+                 KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
+#endif
+
 const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
     KVM_CAP_LAST_INFO
 };
@@ -50,6 +55,33 @@ unsigned long kvm_arch_vcpu_id(CPUState *cpu)
     return cpu->cpu_index;
 }
 
+#ifdef TARGET_AARCH64
+static uint32_t kvm_arm_targets[KVM_ARM_NUM_TARGETS] = {
+    KVM_ARM_TARGET_AEM_V8,
+    KVM_ARM_TARGET_FOUNDATION_V8,
+    KVM_ARM_TARGET_CORTEX_A57
+};
+
+int kvm_arch_init_vcpu(CPUState *cs)
+{
+    struct kvm_vcpu_init init;
+    int ret, i;
+
+    memset(init.features, 0, sizeof(init.features));
+    /* Find an appropriate target CPU type.
+     * KVM does not provide means to detect the host CPU type on aarch64,
+     * and simply refuses to initialize, if the CPU type mis-matches;
+     * so we try each possible CPU type on aarch64 before giving up! */
+    for (i = 0; i < KVM_ARM_NUM_TARGETS; ++i) {
+        init.target = kvm_arm_targets[i];
+        ret = kvm_vcpu_ioctl(cs, KVM_ARM_VCPU_INIT, &init);
+        if (!ret)
+            break;
+    }
+
+    return ret;
+}
+#else
 static bool reg_syncs_via_tuple_list(uint64_t regidx)
 {
     /* Return true if the regidx is a register we should synchronize
@@ -173,6 +205,7 @@ out:
     g_free(rlp);
     return ret;
 }
+#endif
 
 /* We track all the KVM devices which need their memory addresses
  * passing to the kernel in a list of these structures.
@@ -339,6 +372,7 @@ typedef struct Reg {
     int offset;
 } Reg;
 
+#ifndef TARGET_AARCH64
 #define COREREG(KERNELNAME, QEMUFIELD)                       \
     {                                                        \
         KVM_REG_ARM | KVM_REG_SIZE_U32 |                     \
@@ -402,7 +436,52 @@ static const Reg regs[] = {
     VFPSYSREG(FPINST),
     VFPSYSREG(FPINST2),
 };
+#endif
 
+#ifdef TARGET_AARCH64
+int kvm_arch_put_registers(CPUState *cs, int level)
+{
+    struct kvm_one_reg reg;
+    int i;
+    int ret;
+
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+
+    for (i = 0; i < ARRAY_SIZE(env->xregs); 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, &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, &reg);
+    if (ret) {
+        return ret;
+    }
+
+    reg.id = AARCH64_CORE_REG(regs.pstate);
+    reg.addr = (uintptr_t) &env->pstate;
+    ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &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, &reg);
+    if (ret) {
+        return ret;
+    }
+
+    /* TODO: Set Rest of Registers */
+    return ret;
+}
+#else
 int kvm_arch_put_registers(CPUState *cs, int level)
 {
     ARMCPU *cpu = ARM_CPU(cs);
@@ -488,7 +567,52 @@ int kvm_arch_put_registers(CPUState *cs, int level)
 
     return ret;
 }
+#endif
 
+#ifdef TARGET_AARCH64
+int kvm_arch_get_registers(CPUState *cs)
+{
+    struct kvm_one_reg reg;
+    int i;
+    int ret;
+
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+
+    for (i = 0; i < ARRAY_SIZE(env->xregs); 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, &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, &reg);
+    if (ret) {
+        return ret;
+    }
+
+    reg.id = AARCH64_CORE_REG(regs.pstate);
+    reg.addr = (uintptr_t) &env->pstate;
+    ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
+    if (ret) {
+        return ret;
+    }
+
+    reg.id = AARCH64_CORE_REG(regs.pc);
+    reg.addr = (uintptr_t) &env->pc;
+    ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
+    if (ret) {
+        return ret;
+    }
+
+    /* TODO: Set Rest of Registers */
+    return ret;
+}
+#else
 int kvm_arch_get_registers(CPUState *cs)
 {
     ARMCPU *cpu = ARM_CPU(cs);
@@ -559,6 +683,7 @@ int kvm_arch_get_registers(CPUState *cs)
 
     return 0;
 }
+#endif
 
 void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run)
 {
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH v2 3/7] AARCH64: Add aarch64 CPU initialization, get and put registers support

[Peter Maydell]

On 23 July 2013 10:33, Mian M. Hamayun <m.hamayun@virtualopensystems.com> wrote:
> From: "Mian M. Hamayun" <m.hamayun@virtualopensystems.com>
>
> The cpu init function tries to initialize with all possible cpu types, as
> KVM does not provide a means to detect the real cpu type and simply refuses
> to initialize on cpu type mis-match. By using the loop based init function,
> we avoid the need to modify code if the underlying platform is different,
> such as Fast Models instead of Foundation Models.
>
> Get and Put Registers deal with the basic state of Aarch64 CPUs for the moment.
>
> Signed-off-by: Mian M. Hamayun <m.hamayun@virtualopensystems.com>
>
> Conflicts:
>
>         target-arm/kvm.c
>
> Conflicts:
>
>         target-arm/cpu.c

This sort of Conflicts note shouldn't be in a commit message.

> ---
>  linux-headers/linux/kvm.h |    1 +
>  target-arm/kvm.c          |  125 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 126 insertions(+)
>
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index c614070..4df5292 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -783,6 +783,7 @@ struct kvm_dirty_tlb {
>  #define KVM_REG_IA64           0x3000000000000000ULL
>  #define KVM_REG_ARM            0x4000000000000000ULL
>  #define KVM_REG_S390           0x5000000000000000ULL
> +#define KVM_REG_ARM64          0x6000000000000000ULL
>
>  #define KVM_REG_SIZE_SHIFT     52
>  #define KVM_REG_SIZE_MASK      0x00f0000000000000ULL

Updates to the linux-headers/ files need to:
 * be a separate patch
 * be the result of running scripts/update-linux-headers.sh
   on an upstream mainline kernel or kvm-next kernel tree
 * include the kernel tree/commit hash in the commit message

> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
> index b92e00d..c96b871 100644
> --- a/target-arm/kvm.c
> +++ b/target-arm/kvm.c
> @@ -32,6 +32,11 @@
>  #error mismatch between cpu.h and KVM header definitions
>  #endif
>
> +#ifdef TARGET_AARCH64
> +#define AARCH64_CORE_REG(x)   (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \
> +                 KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
> +#endif
> +
>  const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>      KVM_CAP_LAST_INFO
>  };
> @@ -50,6 +55,33 @@ unsigned long kvm_arch_vcpu_id(CPUState *cpu)
>      return cpu->cpu_index;
>  }
>
> +#ifdef TARGET_AARCH64

This set of #ifdefs is pretty messy. I suggest splitting kvm.c
into three:
  target-arm/kvm.c   -- anything common to both 32 and 64 bit
  target-arm/kvm32.c -- non-TARGET_AARCH64 functions
  target-arm/kvm64.c -- TARGET_AARCH64 functions

and have target-arm/Makefile.objs build only one of kvm32.c
or kvm64.c depending on whether TARGET_AARCH64 is set.

> +    /* Find an appropriate target CPU type.
> +     * KVM does not provide means to detect the host CPU type on aarch64,
> +     * and simply refuses to initialize, if the CPU type mis-matches;
> +     * so we try each possible CPU type on aarch64 before giving up! */
> +    for (i = 0; i < KVM_ARM_NUM_TARGETS; ++i) {
> +        init.target = kvm_arm_targets[i];
> +        ret = kvm_vcpu_ioctl(cs, KVM_ARM_VCPU_INIT, &init);
> +        if (!ret)
> +            break;
> +    }

We definitely don't want to do this -- see my notes on
'-cpu host' in another email thread.  (We should make sure
we coordinate who of you or Linaro is going to do the
-cpu host support, incidentally.)

-- PMM

Re: [Qemu-devel] [PATCH v2 3/7] AARCH64: Add aarch64 CPU initialization, get and put registers support

[Mian M. Hamayun]

On 09/08/2013 15:24, Peter Maydell wrote:
> On 23 July 2013 10:33, Mian M. Hamayun <m.hamayun@virtualopensystems.com> wrote:
>> From: "Mian M. Hamayun" <m.hamayun@virtualopensystems.com>
>>
>> The cpu init function tries to initialize with all possible cpu types, as
>> KVM does not provide a means to detect the real cpu type and simply refuses
>> to initialize on cpu type mis-match. By using the loop based init function,
>> we avoid the need to modify code if the underlying platform is different,
>> such as Fast Models instead of Foundation Models.
>>
>> Get and Put Registers deal with the basic state of Aarch64 CPUs for the moment.
>>
>> Signed-off-by: Mian M. Hamayun <m.hamayun@virtualopensystems.com>
>>
>> Conflicts:
>>
>>          target-arm/kvm.c
>>
>> Conflicts:
>>
>>          target-arm/cpu.c
> This sort of Conflicts note shouldn't be in a commit message.
Agreed, will remove it in the next revision.
>
>> ---
>>   linux-headers/linux/kvm.h |    1 +
>>   target-arm/kvm.c          |  125 +++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 126 insertions(+)
>>
>> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
>> index c614070..4df5292 100644
>> --- a/linux-headers/linux/kvm.h
>> +++ b/linux-headers/linux/kvm.h
>> @@ -783,6 +783,7 @@ struct kvm_dirty_tlb {
>>   #define KVM_REG_IA64           0x3000000000000000ULL
>>   #define KVM_REG_ARM            0x4000000000000000ULL
>>   #define KVM_REG_S390           0x5000000000000000ULL
>> +#define KVM_REG_ARM64          0x6000000000000000ULL
>>
>>   #define KVM_REG_SIZE_SHIFT     52
>>   #define KVM_REG_SIZE_MASK      0x00f0000000000000ULL
> Updates to the linux-headers/ files need to:
>   * be a separate patch
>   * be the result of running scripts/update-linux-headers.sh
>     on an upstream mainline kernel or kvm-next kernel tree
>   * include the kernel tree/commit hash in the commit message
Agreed, will do it like this.
>
>> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
>> index b92e00d..c96b871 100644
>> --- a/target-arm/kvm.c
>> +++ b/target-arm/kvm.c
>> @@ -32,6 +32,11 @@
>>   #error mismatch between cpu.h and KVM header definitions
>>   #endif
>>
>> +#ifdef TARGET_AARCH64
>> +#define AARCH64_CORE_REG(x)   (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \
>> +                 KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
>> +#endif
>> +
>>   const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>>       KVM_CAP_LAST_INFO
>>   };
>> @@ -50,6 +55,33 @@ unsigned long kvm_arch_vcpu_id(CPUState *cpu)
>>       return cpu->cpu_index;
>>   }
>>
>> +#ifdef TARGET_AARCH64
> This set of #ifdefs is pretty messy. I suggest splitting kvm.c
> into three:
>    target-arm/kvm.c   -- anything common to both 32 and 64 bit
>    target-arm/kvm32.c -- non-TARGET_AARCH64 functions
>    target-arm/kvm64.c -- TARGET_AARCH64 functions
>
> and have target-arm/Makefile.objs build only one of kvm32.c
> or kvm64.c depending on whether TARGET_AARCH64 is set.
My initial intuition was to separate 32 and 64 bit versions, but as
many functions are common, so I decided to use #ifdefs instead.
btw, I have a similar situation in hw/arm/boot.c as well.
>
>> +    /* Find an appropriate target CPU type.
>> +     * KVM does not provide means to detect the host CPU type on aarch64,
>> +     * and simply refuses to initialize, if the CPU type mis-matches;
>> +     * so we try each possible CPU type on aarch64 before giving up! */
>> +    for (i = 0; i < KVM_ARM_NUM_TARGETS; ++i) {
>> +        init.target = kvm_arm_targets[i];
>> +        ret = kvm_vcpu_ioctl(cs, KVM_ARM_VCPU_INIT, &init);
>> +        if (!ret)
>> +            break;
>> +    }
> We definitely don't want to do this -- see my notes on
> '-cpu host' in another email thread.  (We should make sure
> we coordinate who of you or Linaro is going to do the
> -cpu host support, incidentally.)
I tend to agree with this proposition as well. But the point that I don't
understand is how something is acceptable in kvmtool and not in the
qemu. If this implementation lets us use the 64-bit architecture in the
current state of affairs, then why not use it. We can always replace
this particular part, when the -cpu host support becomes available, right ?

Hamayun

Re: [Qemu-devel] [PATCH v2 3/7] AARCH64: Add aarch64 CPU initialization, get and put registers support

[Peter Maydell]

On 9 August 2013 20:03, Mian M. Hamayun
<m.hamayun@virtualopensystems.com> wrote:
> On 09/08/2013 15:24, Peter Maydell wrote:
>> This set of #ifdefs is pretty messy. I suggest splitting kvm.c
>> into three:
>>    target-arm/kvm.c   -- anything common to both 32 and 64 bit
>>    target-arm/kvm32.c -- non-TARGET_AARCH64 functions
>>    target-arm/kvm64.c -- TARGET_AARCH64 functions
>>
>> and have target-arm/Makefile.objs build only one of kvm32.c
>> or kvm64.c depending on whether TARGET_AARCH64 is set.
>
> My initial intuition was to separate 32 and 64 bit versions, but as
> many functions are common, so I decided to use #ifdefs instead.

That's why I propose having a common-functions file.
Big "disable whole set of functions" ifdefs make it
harder to read a source file.

> btw, I have a similar situation in hw/arm/boot.c as well.

Yeah, I'm not entirely happy with the boot code either, but I didn't
have any concrete suggestions for improvement there.

>> We definitely don't want to do this -- see my notes on
>> '-cpu host' in another email thread.  (We should make sure
>> we coordinate who of you or Linaro is going to do the
>> -cpu host support, incidentally.)
>
> I tend to agree with this proposition as well. But the point that I don't
> understand is how something is acceptable in kvmtool and not in the
> qemu.

Different projects, different rules. In particular, kvmtool aims
to be a good medium for quick development and testing of new
features, whereas QEMU is trying to be the production-ready
backwards-compatible stable solution. That means that it's
often going to be possible to put code into kvmtool as an
interim "this works and we'll revisit it later" approach but
not really be able to do the same in QEMU.

> If this implementation lets us use the 64-bit architecture in the
> current state of affairs, then why not use it. We can always replace
> this particular part, when the -cpu host support becomes available,
> right ?

Also, the way that '-cpu host' support becomes available
is that we insist that it gets written in order for 64-bit
support to land. It's not really all that complicated.

-- PMM

[Qemu-devel] [PATCH v2 4/7] AARCH64: Add boot support for aarch64 processor

[Mian M. Hamayun]

From: "Mian M. Hamayun" <m.hamayun@virtualopensystems.com>

This version supports booting of a single Aarch64 CPU by setting appropriate
registers. The bootloader includes placehoders for Board-ID that are used to
implementing uniform indexing across different bootloaders. The same macro
names are used with different values when compiling for different processors.

Signed-off-by: Mian M. Hamayun <m.hamayun@virtualopensystems.com>

Conflicts:

	hw/arm/boot.c
---
 hw/arm/boot.c |   44 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 7cca2b3..b9b0beb 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -18,7 +18,33 @@
 #include "qemu/config-file.h"
 
 #define KERNEL_ARGS_ADDR 0x100
-#define KERNEL_LOAD_ADDR 0x00010000
+
+#ifdef TARGET_AARCH64
+#define KERNEL_LOAD_ADDR        0x00080000
+#define KERNEL_ARGS_INDEX       6
+#define KERNEL_ENTRY_INDEX      8
+#define KERNEL_BOARDID_INDEX    10
+
+static uint32_t bootloader[] = {
+    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 */
+    0x00000000, 	/* .word @DTB Lower 32-bits */
+    0x00000000, 	/* .word @DTB Higher 32-bits */
+    0x00000000, 	/* .word @Kernel Entry Lower 32-bits */
+    0x00000000,  	/* .word @Kernel Entry Higher 32-bits */
+    0x00000000, 	/* .word @Board ID Lower 32-bits -- Placeholder */
+    0x00000000  	/* .word @Board ID Higher 32-bits -- Placeholder */
+};
+
+#else
+#define KERNEL_LOAD_ADDR        0x00010000
+#define KERNEL_BOARDID_INDEX    4
+#define KERNEL_ARGS_INDEX       5
+#define KERNEL_ENTRY_INDEX      6
 
 /* The worlds second smallest bootloader.  Set r0-r2, then jump to kernel.  */
 static uint32_t bootloader[] = {
@@ -30,6 +56,7 @@ static uint32_t bootloader[] = {
   0, /* Address of kernel args.  Set by integratorcp_init.  */
   0  /* Kernel entry point.  Set by integratorcp_init.  */
 };
+#endif
 
 /* Handling for secondary CPU boot in a multicore system.
  * Unlike the uniprocessor/primary CPU boot, this is platform
@@ -341,8 +368,15 @@ static void do_cpu_reset(void *opaque)
             env->regs[15] = info->entry & 0xfffffffe;
             env->thumb = info->entry & 1;
         } else {
+#ifdef TARGET_AARCH64
+            env->pstate = PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT | PSR_MODE_EL1h;
+#endif
             if (env == first_cpu) {
+#ifdef TARGET_AARCH64
+                env->pc = info->loader_start;
+#else
                 env->regs[15] = info->loader_start;
+#endif
                 if (!info->dtb_filename) {
                     if (old_param) {
                         set_kernel_args_old(info);
@@ -447,7 +481,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
         }
         info->initrd_size = initrd_size;
 
-        bootloader[4] = info->board_id;
+        bootloader[KERNEL_BOARDID_INDEX] = info->board_id;
 
         /* for device tree boot, we pass the DTB directly in r2. Otherwise
          * we point to the kernel args.
@@ -462,9 +496,9 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
             if (load_dtb(dtb_start, info)) {
                 exit(1);
             }
-            bootloader[5] = dtb_start;
+            bootloader[KERNEL_ARGS_INDEX] = dtb_start;
         } else {
-            bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
+            bootloader[KERNEL_ARGS_INDEX] = 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"
@@ -472,7 +506,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
                 exit(1);
             }
         }
-        bootloader[6] = entry;
+        bootloader[KERNEL_ENTRY_INDEX] = entry;
         for (n = 0; n < sizeof(bootloader) / 4; n++) {
             bootloader[n] = tswap32(bootloader[n]);
         }
-- 
1.7.9.5

[Qemu-devel] [PATCH v2 5/7] AARCH64: Disable the non-aarch64 specific reset code

[Mian M. Hamayun]

From: "Mian M. Hamayun" <m.hamayun@virtualopensystems.com>

This commit disables the co-processor registers reset code for KVM, when
compiling for AArch64 cpus.

Signed-off-by: Mian M. Hamayun <m.hamayun@virtualopensystems.com>
---
 target-arm/kvm.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index c96b871..5909d75 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -700,6 +700,7 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
 
 void kvm_arch_reset_vcpu(CPUState *cs)
 {
+#ifndef TARGET_AARCH64
     /* Feed the kernel back its initial register state */
     ARMCPU *cpu = ARM_CPU(cs);
 
@@ -709,6 +710,7 @@ void kvm_arch_reset_vcpu(CPUState *cs)
     if (!write_list_to_kvmstate(cpu)) {
         abort();
     }
+#endif
 }
 
 bool kvm_arch_stop_on_emulation_error(CPUState *cs)
-- 
1.7.9.5

[Qemu-devel] [PATCH v2 6/7] AARCH64: Add SMP support for aarch64 processors

[Mian M. Hamayun]

From: Alexander Spyridakis <a.spyridakis@virtualopensystems.com>

AArch64 uses a cpu-release-addr memory location (defined in the dts) as
a way to inform secondary CPUs where to jump to and enter their holding
pen. Inject a very simple bootloader that polls this memory location,
until the primary CPU sets it to the right address.

Signed-off-by: Alexander Spyridakis <a.spyridakis@virtualopensystems.com>
---
 hw/arm/boot.c |   20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index b9b0beb..efbd984 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -17,6 +17,8 @@
 #include "sysemu/device_tree.h"
 #include "qemu/config-file.h"
 
+#define DSB_INSN 0xf57ff04f
+#define CP15_DSB_INSN 0xee070f9a /* mcr cp15, 0, r0, c7, c10, 4 */
 #define KERNEL_ARGS_ADDR 0x100
 
 #ifdef TARGET_AARCH64
@@ -40,6 +42,16 @@ static uint32_t bootloader[] = {
     0x00000000  	/* .word @Board ID Higher 32-bits -- Placeholder */
 };
 
+static uint32_t smpboot[] = {
+    0x180000c5, /* ldr w5, =mbox_value - mbox value for secondary CPUs */
+    0xf94000a4, /* 1: ldr      x4, [x5] - Read address to jump to */
+    0xb4ffffe4, /* cbz x4, 1b - Check if mbox value is zero, if yes retry */
+    0xd61f0080, /* br  x4 - Branch to given address */
+    0x0,        /* padding word */
+    0x0,        /* gic_cpu_if_addr */
+    0x8000fff8  /* mbox_value: default mbox value (aka cpu_release_addr) */
+};
+
 #else
 #define KERNEL_LOAD_ADDR        0x00010000
 #define KERNEL_BOARDID_INDEX    4
@@ -56,7 +68,6 @@ static uint32_t bootloader[] = {
   0, /* Address of kernel args.  Set by integratorcp_init.  */
   0  /* Kernel entry point.  Set by integratorcp_init.  */
 };
-#endif
 
 /* Handling for secondary CPU boot in a multicore system.
  * Unlike the uniprocessor/primary CPU boot, this is platform
@@ -72,8 +83,6 @@ static uint32_t bootloader[] = {
  * for an interprocessor interrupt and polling a configurable
  * location for the kernel secondary CPU entry point.
  */
-#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 */
@@ -91,6 +100,7 @@ static uint32_t smpboot[] = {
   0,          /* gic_cpu_if: base address of GIC CPU interface */
   0           /* bootreg: Boot register address is held here */
 };
+#endif
 
 static void default_write_secondary(ARMCPU *cpu,
                                     const struct arm_boot_info *info)
@@ -115,8 +125,12 @@ static void default_reset_secondary(ARMCPU *cpu,
 {
     CPUARMState *env = &cpu->env;
 
+#ifdef TARGET_AARCH64
+    env->pc = info->smp_loader_start;
+#else
     stl_phys_notdirty(info->smp_bootreg_addr, 0);
     env->regs[15] = info->smp_loader_start;
+#endif
 }
 
 #define WRITE_WORD(p, value) do { \
-- 
1.7.9.5

[Qemu-devel] [PATCH v2 7/7] AARCH64: Use the spin-table method for booting secondary processors in machvirt

[Mian M. Hamayun]

From: "Mian M. Hamayun" <m.hamayun@virtualopensystems.com>

As the SMP bootloader uses a spin-table to wait for the cpu_release_addr, we
disable the PSCI method for AArch64 in machvirt and use spin-table instead.

The CPU_RELEASE_OFFSET is introduced in machvirt and is to calculate the
cpu_release_addr by addition of this value to the memory base address, and this
value is updated in the smp bootloader using the smp_bootreg_addr board info
parameter.

Tested-by: Alexander Spyridakis <a.spyridakis@virtualopensystems.com>
Signed-off-by: Mian M. Hamayun <m.hamayun@virtualopensystems.com>
---
 hw/arm/virt.c |   14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 8a2bdc7..44ab59d 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -52,6 +52,7 @@
 #define IO_LEN 0x000f0000
 
 #if defined(TARGET_AARCH64)
+#define CPU_RELEASE_OFFSET 0x0000fff8
 #define DEFAULT_CPU_MODEL "cortex-a57"
 #elif defined(TARGET_ARM)
 #define DEFAULT_CPU_MODEL "cortex-a15"
@@ -170,7 +171,7 @@ static void *initial_fdt(struct machine_info *mi)
     qemu_devtree_setprop_cell(fdt, "/soc", "#interrupt-cells", 0x1);
 
     /* No PSCI for TCG yet */
-#ifdef CONFIG_KVM
+#if defined (CONFIG_KVM) && !defined(TARGET_AARCH64)
     if (kvm_enabled()) {
         qemu_devtree_add_subnode(fdt, "/psci");
         qemu_devtree_setprop_string(fdt, "/psci", "compatible", "arm,psci");
@@ -234,7 +235,14 @@ static void fdt_add_cpu_nodes(void *fdt, struct machine_info *mi, int smp_cpus)
             mi->cpu_compatible);
 
         if (smp_cpus > 1) {
+#if defined(TARGET_AARCH64)
+            qemu_devtree_setprop_string(fdt, cpu_name, "enable-method",
+                "spin-table");
+            qemu_devtree_setprop_u64(fdt, cpu_name, "cpu-release-addr",
+                (mi->mem_base + CPU_RELEASE_OFFSET));
+#else
             qemu_devtree_setprop_string(fdt, cpu_name, "enable-method", "psci");
+#endif
         }
 
         qemu_devtree_setprop_cell(fdt, cpu_name, "reg", cpu);
@@ -437,6 +445,10 @@ static void machvirt_init(QEMUMachineInitArgs *args)
     machvirt_binfo.nb_cpus = smp_cpus;
     machvirt_binfo.board_id = -1;
     machvirt_binfo.loader_start = mi->mem_base;
+    machvirt_binfo.smp_loader_start = mi->mem_base + 0x1000;
+#if defined(TARGET_AARCH64)
+    machvirt_binfo.smp_bootreg_addr = mi->mem_base + CPU_RELEASE_OFFSET;
+#endif
     machvirt_binfo.get_dtb = machvirt_dtb;
     arm_load_kernel(arm_env_get_cpu(first_cpu), &machvirt_binfo);
 }
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH v2 7/7] AARCH64: Use the spin-table method for booting secondary processors in machvirt

[Peter Maydell]

On 23 July 2013 10:33, Mian M. Hamayun <m.hamayun@virtualopensystems.com> wrote:
> From: "Mian M. Hamayun" <m.hamayun@virtualopensystems.com>
>
> As the SMP bootloader uses a spin-table to wait for the cpu_release_addr, we
> disable the PSCI method for AArch64 in machvirt and use spin-table instead.

Marc Z says we should be using PSCI for secondary CPU boot for aarch64,
same as for aarch32.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 7/7] AARCH64: Use the spin-table method for booting secondary processors in machvirt

[Mian M. Hamayun]

On 09/08/2013 16:34, Peter Maydell wrote:
> On 23 July 2013 10:33, Mian M. Hamayun <m.hamayun@virtualopensystems.com> wrote:
>> From: "Mian M. Hamayun" <m.hamayun@virtualopensystems.com>
>>
>> As the SMP bootloader uses a spin-table to wait for the cpu_release_addr, we
>> disable the PSCI method for AArch64 in machvirt and use spin-table instead.
> Marc Z says we should be using PSCI for secondary CPU boot for aarch64,
> same as for aarch32.
>
OK, we will look into this as well.
Thanks!