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.)
>> + 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.
>> + * 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.
>> + }
>> + 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.
>> +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.
-- PMM
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
