Re: [PATCH 4/4] m68k: add Virtual M68k Machine

[Laurent Vivier]

Le 13/10/2020 à 19:56, Philippe Mathieu-Daudé a écrit :
> On 10/13/20 5:51 PM, Laurent Vivier wrote:
>> The machine is based on Goldfish interfaces defined by Google
>> for Android simulator. It uses Goldfish-rtc (timer and RTC),
>> Goldfish-pic (PIC) and Goldfish-tty (for serial port and early tty).
>>
>> The machine is created with 128 virtio-mmio bus, and they can
>> be used to use serial console, GPU, disk, NIC, HID, ...
>>
>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>> ---
>>   default-configs/devices/m68k-softmmu.mak      |   1 +
>>   .../standard-headers/asm-m68k/bootinfo-virt.h |  17 +
>>   hw/m68k/virt.c                                | 296 ++++++++++++++++++
>>   MAINTAINERS                                   |   9 +
>>   hw/m68k/Kconfig                               |   8 +
>>   hw/m68k/meson.build                           |   1 +
>>   6 files changed, 332 insertions(+)
>>   create mode 100644 include/standard-headers/asm-m68k/bootinfo-virt.h
>>   create mode 100644 hw/m68k/virt.c
...
>> +/*
>> + * The GLUE (General Logic Unit) is an Apple custom integrated
>> circuit chip
>> + * that performs a variety of functions (RAM management, clock
>> generation, ...).
>> + * The GLUE chip receives interrupt requests from various devices,
>> + * assign priority to each, and asserts one or more interrupt line to
>> the
>> + * CPU.
> 
> Does your virt machine really requires a GLUE? Or only another
> cascaded PIC?

I agree code needs cleanup here.

We need something to set the ÌRQ level (priority) and vector (see
m68k_set_irq_level()).

>> + */
>> +
>> +typedef struct {
>> +    M68kCPU *cpu;
>> +    uint8_t ipr;
>> +} GLUEState;
>> +
>> +static void GLUE_set_irq(void *opaque, int irq, int level)
>> +{
>> +    GLUEState *s = opaque;
>> +    int i;
>> +
>> +    if (level) {
>> +        s->ipr |= 1 << irq;
>> +    } else {
>> +        s->ipr &= ~(1 << irq);
>> +    }
>> +
>> +    for (i = 7; i >= 0; i--) {
>> +        if ((s->ipr >> i) & 1) {
>> +            m68k_set_irq_level(s->cpu, i + 1, i + 25);
>> +            return;
>> +        }
>> +    }
>> +    m68k_set_irq_level(s->cpu, 0, 0);
>> +}
>> +
>> +static void main_cpu_reset(void *opaque)
>> +{
>> +    M68kCPU *cpu = opaque;
>> +    CPUState *cs = CPU(cpu);
>> +
>> +    cpu_reset(cs);
>> +    cpu->env.aregs[7] = ldl_phys(cs->as, 0);
>> +    cpu->env.pc = ldl_phys(cs->as, 4);
>> +}
>> +
>> +static void virt_init(MachineState *machine)
>> +{
>> +    M68kCPU *cpu = NULL;
>> +    int32_t kernel_size;
>> +    uint64_t elf_entry;
>> +    ram_addr_t initrd_base;
>> +    int32_t initrd_size;
>> +    ram_addr_t ram_size = machine->ram_size;
>> +    const char *kernel_filename = machine->kernel_filename;
>> +    const char *initrd_filename = machine->initrd_filename;
>> +    const char *kernel_cmdline = machine->kernel_cmdline;
>> +    hwaddr parameters_base;
>> +    DeviceState *dev;
>> +    DeviceState *pic_dev[7];
>> +    GLUEState *irq;
>> +    qemu_irq *cpu_pic;
>> +    SysBusDevice *sysbus;
>> +    hwaddr io_base;
>> +    int i;
>> +
>> +
>> +    if (ram_size > 3399672 * KiB) {
>> +        /*
>> +         * The physical memory can be up to 4 GiB - 16 MiB, but linux
>> +         * kernel crashes after this limit (~ 3.2 GiB)
>> +         */
>> +        error_report("Too much memory for this machine: %" PRId64 "
>> KiB, "
>> +                     "maximum 3399672 KiB", ram_size / KiB);
>> +        exit(1);
>> +    }
>> +
>> +    /* init CPUs */
>> +    cpu = M68K_CPU(cpu_create(machine->cpu_type));
> 
> Due to BOOTINFO1(..., BI_CPUTYPE, CPU_68040) below, don't you
> need to check machine->cpu_type == M68K_CPU_TYPE_NAME("m68040")?

Yes, you're right.

68030MMU is not implemented. So we can't use other CPU than 68040.

> 
>> +    qemu_register_reset(main_cpu_reset, cpu);
>> +
>> +    /* RAM */
>> +    memory_region_add_subregion(get_system_memory(), 0, machine->ram);
>> +
>> +    /* IRQ Glue */
>> +
>> +    irq = g_new0(GLUEState, 1);
>> +    irq->cpu = cpu;
>> +    cpu_pic = qemu_allocate_irqs(GLUE_set_irq, irq, 8);
>> +
>> +    /*
>> +     * 6 goldfish-pic
>> +     *
>> +     * map: 0xff000000 - 0xff006fff = 28 KiB
>> +     * IRQ: #1 (lower priority) -> #6 (higher priority)
>> +     *
>> +     */
>> +    io_base = VIRT_GF_PIC_MMIO_BASE;
>> +    for (i = 0; i < 6; i++) {
>> +        pic_dev[i] = qdev_new(TYPE_GOLDFISH_PIC);
>> +        sysbus = SYS_BUS_DEVICE(pic_dev[i]);
>> +        sysbus_realize_and_unref(sysbus, &error_fatal);
>> +
>> +        sysbus_mmio_map(sysbus, 0, io_base);
>> +        sysbus_connect_irq(sysbus, 0, cpu_pic[i]);
>> +
>> +        io_base += 0x1000;
>> +    }
>> +
>> +    /* goldfish-rtc */
>> +    dev = qdev_new(TYPE_GOLDFISH_RTC);
>> +    sysbus = SYS_BUS_DEVICE(dev);
>> +    sysbus_realize_and_unref(sysbus, &error_fatal);
>> +    sysbus_mmio_map(sysbus, 0, VIRT_GF_RTC_MMIO_BASE);
>> +    sysbus_connect_irq(sysbus, 0, PIC_GPIO(VIRT_GF_RTC_IRQ_BASE));
>> +
>> +    /* goldfish-tty */
>> +    dev = qdev_new(TYPE_GOLDFISH_TTY);
>> +    sysbus = SYS_BUS_DEVICE(dev);
>> +    qdev_prop_set_chr(dev, "chardev", serial_hd(0));
>> +    sysbus_realize_and_unref(sysbus, &error_fatal);
>> +    sysbus_mmio_map(sysbus, 0, VIRT_GF_TTY_MMIO_BASE);
>> +    sysbus_connect_irq(sysbus, 0, PIC_GPIO(VIRT_GF_TTY_IRQ_BASE));
>> +
>> +    /* virtio-mmio */
>> +    io_base = VIRT_VIRTIO_MMIO_BASE;
>> +    for (i = 0; i < 128; i++) {
>> +        dev = qdev_new(TYPE_VIRTIO_MMIO);
>> +        qdev_prop_set_bit(dev, "force-legacy", false);
>> +        sysbus = SYS_BUS_DEVICE(dev);
>> +        sysbus_realize_and_unref(sysbus, &error_fatal);
>> +        sysbus_connect_irq(sysbus, 0, PIC_GPIO(VIRT_VIRTIO_IRQ_BASE +
>> i));
>> +        sysbus_mmio_map(sysbus, 0, io_base);
>> +        io_base += 0x200;
>> +    }
>> +
>> +    if (kernel_filename) {
>> +        CPUState *cs = CPU(cpu);
>> +        uint64_t high;
>> +
>> +        kernel_size = load_elf(kernel_filename, NULL, NULL, NULL,
>> +                               &elf_entry, NULL, &high, NULL, 1,
>> +                               EM_68K, 0, 0);
>> +        if (kernel_size < 0) {
>> +            error_report("could not load kernel '%s'", kernel_filename);
>> +            exit(1);
>> +        }
>> +        stl_phys(cs->as, 4, elf_entry); /* reset initial PC */
>> +        parameters_base = (high + 1) & ~1;
>> +
>> +        BOOTINFO1(cs->as, parameters_base, BI_MACHTYPE, MACH_VIRT);
>> +        BOOTINFO1(cs->as, parameters_base, BI_FPUTYPE, FPU_68040);
>> +        BOOTINFO1(cs->as, parameters_base, BI_MMUTYPE, MMU_68040);
>> +        BOOTINFO1(cs->as, parameters_base, BI_CPUTYPE, CPU_68040);
> 
> (see machine->cpu_type question earlier).

see answer above.

>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index d0962a22e1b4..b08cf4251246 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1101,6 +1101,15 @@ F: include/hw/nubus/*
>>   F: include/hw/display/macfb.h
>>   F: include/hw/block/swim.h
>>   +virt
> 
> Maybe "m68k virt".

We have "virt" only for ARM, RISCV, Xtensa, ...
the m68k is implied by the qemu-system-m68k

> 
>> +M: Laurent Vivier <laurent@vivier.eu>
>> +S: Maintained
>> +F: hw/m68k/virt.c
>> +F: hw/char/goldfish_tty.c
>> +F: hw/intc/goldfish_pic.c
>> +F: include/hw/char/goldfish_tty.h
>> +F: include/hw/intc/goldfish_pic.h
>> +

Thanks,
Laurent