On 14.08.2013, at 11:19, Peter Maydell wrote:
> On 14 August 2013 10:02, Alexander Graf <agraf@xxxxxxx> wrote:
>>
>> On 13.08.2013, at 14:03, Peter Maydell wrote:
>>
>>> + /* No PSCI for TCG yet */
>>> +#ifdef CONFIG_KVM
>>
>> Do you need this #ifdef? Are the headers really not included when CONFIG_KVM is disabled?
>
> Yes, the KVM_PSCI_* constants are defined in the linux-headers/
> kernel include files, which are only pulled in when CONFIG_KVM is
> defined. (Has to be that way, because the kernel headers aren't
> guaranteed to be buildable on platforms other than Linux.)
What a shame. But then again once you implement a compatible interface in QEMU, the defines will be available regardless so you can get rid of the #ifdef again :).
>
>>> + if (kvm_enabled()) {
>>> + qemu_devtree_add_subnode(fdt, "/psci");
>>> + qemu_devtree_setprop_string(fdt, "/psci", "compatible", "arm,psci");
>>> + qemu_devtree_setprop_string(fdt, "/psci", "method", "hvc");
>>> + qemu_devtree_setprop_cell(fdt, "/psci", "cpu_suspend",
>>> + KVM_PSCI_FN_CPU_SUSPEND);
>>> + qemu_devtree_setprop_cell(fdt, "/psci", "cpu_off", KVM_PSCI_FN_CPU_OFF);
>>> + qemu_devtree_setprop_cell(fdt, "/psci", "cpu_on", KVM_PSCI_FN_CPU_ON);
>>> + qemu_devtree_setprop_cell(fdt, "/psci", "migrate", KVM_PSCI_FN_MIGRATE);
>>> + }
>>> +#endif
>
>>> + /*
>>> + * Only supported method of starting secondary CPUs is PSCI and
>>> + * PSCI is not yet supported with TCG, so limit smp_cpus to 1
>>
>> Sounds like a good point in time to implement a KVM compatible PSCI interface in TCG ;).
>
> It is certainly on our TODO list, but since it requires tackling the
> "SMC in a non-trustzone QEMU" issue I preferred not to tangle it up
> with this patchset.
I agree.
>
>>> + * if we're not using KVM.
>>> + */
>>> + if (!kvm_enabled() && smp_cpus > 1) {
>>> + error_report("mach-virt: must enable KVM to use multiple CPUs");
>>> + exit(1);
>>> + }
>>> +
>>> + if (args->ram_size > vbi->memmap[VIRT_MEM].size) {
>>> + error_report("mach-virt: cannot model more than 30GB RAM");
>>> + exit(1);
>>> + }
>>> +
>>> + create_fdt(vbi);
>>> + fdt_add_timer_nodes(vbi);
>>> +
>>> + for (n = 0; n < smp_cpus; n++) {
>>> + ARMCPU *cpu;
>>> + qemu_irq *irqp;
>>> +
>>> + cpu = cpu_arm_init(cpu_model);
>>> + irqp = arm_pic_init_cpu(cpu);
>>> + cpu_irq[n] = irqp[ARM_PIC_CPU_IRQ];
>>
>> I don't see a check for smp_cpus <= 4, but the array is size 4?
>
> This array will probably go away once this is rebased on my
> "get rid of arm_pic" patchset that I posted last week.
Can you just use smp_cpus as array length for the time being?
>
>>> + }
>>> + fdt_add_cpu_nodes(vbi);
>>> +
>>> + memory_region_init_ram(ram, NULL, "mach-virt.ram", args->ram_size);
>>> + vmstate_register_ram_global(ram);
>>> + memory_region_add_subregion(sysmem, vbi->memmap[VIRT_MEM].base, ram);
>>> +
>>> + dev = qdev_create(NULL, vbi->qdevname);
>>> + qdev_prop_set_uint32(dev, "num-cpu", smp_cpus);
>>> + qdev_init_nofail(dev);
>>> + busdev = SYS_BUS_DEVICE(dev);
>>> + sysbus_mmio_map(busdev, 0, vbi->memmap[VIRT_CPUPERIPHS].base);
>>> + fdt_add_gic_node(vbi);
>>> + for (n = 0; n < smp_cpus; n++) {
>>> + sysbus_connect_irq(busdev, n, cpu_irq[n]);
>>> + }
>>> +
>>> + for (n = 0; n < 64; n++) {
>>
>> Where does that 64 come from? I presume from the GIC? Any chance this could be put into a global define?
>
> Nope, it's just a decision by this board to have 64 external
> GIC interrupts. We could make it 96 or 128 if we chose (and
> set a property on the GIC device to match). You could argue
> that we shouldn't rely on knowing what the default value for
> the GIC's num-irqs property is though.
We shouldn't, no :).
>
>>> +static QEMUMachine machvirt_a15_machine = {
>>> + .name = "virt",
>>> + .desc = "ARM Virtual Machine",
>>> + .init = machvirt_init,
>>> + .max_cpus = 4,
>>
>> Ah, this is where smp_cpus gets limited to 4.
>>
>> Why is it limited to 4? And this really should be a define that you
>> reuse on the IRQ map above :).
>
> An A15 can only have 4 CPU cores maximum -- hardware limit.
> This will have to be adjusted when virt supports non-a15 CPUs,
> though.
Ah, ok. This is the 15 machine description, so I guess imposing a15 limits is ok.
Alex
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
