diff mbox

[15/15] ppc: Add aCube Sam460ex board

Message ID ab632ef70b02efb2257d9e2f08b45b097ebacb3a.1503249785.git.balaton@eik.bme.hu
State New
Headers show

Commit Message

BALATON Zoltan Aug. 20, 2017, 5:23 p.m. UTC
Add emulation of aCube Sam460ex board based on AMCC 460EX embedded SoC.
This is not a full implementation yet with a lot of components still
missing but enough to start a Linux kernel and the U-Boot firmware.

Signed-off-by: François Revol <revol@free.fr>
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 default-configs/ppcemb-softmmu.mak |   3 +
 hw/ppc/Makefile.objs               |   2 +
 hw/ppc/sam460ex.c                  | 611 +++++++++++++++++++++++++++++++++++++
 3 files changed, 616 insertions(+)
 create mode 100644 hw/ppc/sam460ex.c

Comments

Philippe Mathieu-Daudé Aug. 20, 2017, 10:10 p.m. UTC | #1
Hi Zoltan,

On 08/20/2017 02:23 PM, BALATON Zoltan wrote:
> Add emulation of aCube Sam460ex board based on AMCC 460EX embedded SoC.
> This is not a full implementation yet with a lot of components still
> missing but enough to start a Linux kernel and the U-Boot firmware.
> 
> Signed-off-by: François Revol <revol@free.fr>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   default-configs/ppcemb-softmmu.mak |   3 +
>   hw/ppc/Makefile.objs               |   2 +
>   hw/ppc/sam460ex.c                  | 611 +++++++++++++++++++++++++++++++++++++
>   3 files changed, 616 insertions(+)
>   create mode 100644 hw/ppc/sam460ex.c
> 
> diff --git a/default-configs/ppcemb-softmmu.mak b/default-configs/ppcemb-softmmu.mak
> index 635923a..90b42f0 100644
> --- a/default-configs/ppcemb-softmmu.mak
> +++ b/default-configs/ppcemb-softmmu.mak
> @@ -17,3 +17,6 @@ CONFIG_XILINX=y
>   CONFIG_XILINX_ETHLITE=y
>   CONFIG_LIBDECNUMBER=y
>   CONFIG_SM501=y
> +CONFIG_USB_EHCI_SYSBUS=y
> +CONFIG_IDE_SII3112=y
> +CONFIG_SAM460EX=y
> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> index fc39fe4..0aaea5b 100644
> --- a/hw/ppc/Makefile.objs
> +++ b/hw/ppc/Makefile.objs
> @@ -27,3 +27,5 @@ obj-$(CONFIG_E500) += e500.o mpc8544ds.o e500plat.o
>   obj-$(CONFIG_E500) += mpc8544_guts.o ppce500_spin.o
>   # PowerPC 440 Xilinx ML507 reference board.
>   obj-$(CONFIG_XILINX) += virtex_ml507.o
> +# ACube Sam460ex board.
> +obj-$(CONFIG_SAM460EX) += ppc440_uc.o sam460ex.o
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> new file mode 100644
> index 0000000..7cf4f6f
> --- /dev/null
> +++ b/hw/ppc/sam460ex.c
> @@ -0,0 +1,611 @@
> +/*
> + * QEMU aCube Sam460ex board emulation
> + *
> + * Copyright (c) 2012 François Revol
> + * Copyright (c) 2016-2017 BALATON Zoltan
> + *
> + * This file is derived from hw/ppc440_bamboo.c,
> + * the copyright for that material belongs to the original owners.
> + *
> + * This work is licensed under the GNU GPL license version 2 or later.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "qemu/cutils.h"
> +#include "qapi/error.h"
> +#include "hw/hw.h"
> +#include "sysemu/blockdev.h"
> +#include "hw/boards.h"
> +#include "sysemu/kvm.h"
> +#include "kvm_ppc.h"
> +#include "sysemu/device_tree.h"
> +#include "sysemu/block-backend.h"
> +#include "hw/loader.h"
> +#include "elf.h"
> +#include "exec/address-spaces.h"
> +#include "exec/memory.h"
> +#include "hw/ppc/ppc440.h"
> +#include "hw/ppc/ppc405.h"
> +#include "hw/block/flash.h"
> +#include "sysemu/sysemu.h"
> +#include "hw/sysbus.h"
> +#include "hw/char/serial.h"
> +#include "hw/i2c/ppc4xx_i2c.h"
> +#include "hw/i2c/smbus.h"
> +#include "hw/usb/hcd-ehci.h"
> +
> +#define BINARY_DEVICE_TREE_FILE "sam460ex.dtb"
> +#define UBOOT_FILENAME "u-boot-sam460-20100605.bin"
> +/* to extract the official U-Boot bin from the updater: */
> +/* dd bs=1 skip=$(($(stat -c '%s' updater/updater-460) - 0x80000)) \
> +     if=updater/updater-460 of=u-boot-sam460-20100605.bin */
> +
> +/* from Sam460 U-Boot include/configs/Sam460ex.h */
> +#define FLASH_BASE             0xfff00000
> +#define FLASH_BASE_H           0x4
> +#define FLASH_SIZE             (1 << 20)
> +#define UBOOT_LOAD_BASE        0xfff80000
> +#define UBOOT_SIZE             0x00080000
> +#define UBOOT_ENTRY            0xfffffffc
> +
> +/* from U-Boot */
> +#define EPAPR_MAGIC           (0x45504150)
> +#define KERNEL_ADDR           0x1000000
> +#define FDT_ADDR              0x1800000
> +#define RAMDISK_ADDR          0x1900000
> +
> +/* Sam460ex IRQ MAP:
> +   IRQ0  = ETH_INT
> +   IRQ1  = FPGA_INT
> +   IRQ2  = PCI_INT (PCIA, PCIB, PCIC, PCIB)
> +   IRQ3  = FPGA_INT2
> +   IRQ11 = RTC_INT
> +   IRQ12 = SM502_INT
> +*/
> +
> +#define SDRAM_NR_BANKS 4
> +
> +/* FIXME: See u-boot.git 8ac41e */
> +static const unsigned int ppc460ex_sdram_bank_sizes[] = {
> +    1024 << 20, 512 << 20, 256 << 20, 128 << 20, 64 << 20, 32 << 20, 0
> +};
> +
> +struct boot_info {
> +    uint32_t dt_base;
> +    uint32_t dt_size;
> +    uint32_t entry;
> +};
> +
> +/*****************************************************************************/
> +/* SPD eeprom content from mips_malta.c */
> +
> +struct _eeprom24c0x_t {
> +  uint8_t tick;
> +  uint8_t address;
> +  uint8_t command;
> +  uint8_t ack;
> +  uint8_t scl;
> +  uint8_t sda;
> +  uint8_t data;
> +  uint8_t contents[256];
> +};
> +
> +typedef struct _eeprom24c0x_t eeprom24c0x_t;
> +
> +static eeprom24c0x_t spd_eeprom = {
> +    .contents = {
> +        /* 00000000: */ 0x80, 0x08, 0xFF, 0x0D, 0x0A, 0xFF, 0x40, 0x00,
> +        /* 00000008: */ 0x04, 0x75, 0x54, 0x00, 0x82, 0x08, 0x00, 0x01,
> +        /* 00000010: */ 0x8F, 0x04, 0x02, 0x01, 0x01, 0x00, 0x00, 0x00,
> +        /* 00000018: */ 0x00, 0x00, 0x00, 0x14, 0x0F, 0x14, 0x2D, 0xFF,
> +        /* 00000020: */ 0x15, 0x08, 0x15, 0x08, 0x00, 0x00, 0x00, 0x00,
> +        /* 00000028: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +        /* 00000030: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +        /* 00000038: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x12, 0xD0,
> +        /* 00000040: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +        /* 00000048: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +        /* 00000050: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +        /* 00000058: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +        /* 00000060: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +        /* 00000068: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +        /* 00000070: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +        /* 00000078: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x64, 0xF4,
> +    },
> +};
> +
> +static void generate_eeprom_spd(uint8_t *eeprom, ram_addr_t ram_size)
> +{
> +    enum { SDR = 0x4, DDR1 = 0x7, DDR2 = 0x8 } type;
> +    uint8_t *spd = spd_eeprom.contents;
> +    uint8_t nbanks = 0;
> +    uint16_t density = 0;
> +    int i;
> +
> +    /* work in terms of MB */
> +    ram_size >>= 20;
> +
> +    while ((ram_size >= 4) && (nbanks <= 2)) {
> +        int sz_log2 = MIN(31 - clz32(ram_size), 14);
> +        nbanks++;
> +        density |= 1 << (sz_log2 - 2);
> +        ram_size -= 1 << sz_log2;
> +    }
> +
> +    /* split to 2 banks if possible */
> +    if ((nbanks == 1) && (density > 1)) {
> +        nbanks++;
> +        density >>= 1;
> +    }
> +
> +    if (density & 0xff00) {
> +        density = (density & 0xe0) | ((density >> 8) & 0x1f);
> +        type = DDR2;
> +    } else if (!(density & 0x1f)) {
> +        type = DDR2;
> +    } else {
> +        type = SDR;
> +    }
> +
> +    if (ram_size) {
> +        fprintf(stderr, "Warning: SPD cannot represent final %dMB"
> +                " of SDRAM\n", (int)ram_size);
> +    }
> +
> +    /* fill in SPD memory information */
> +    spd[2] = type;
> +    spd[5] = nbanks;
> +    spd[31] = density;
> +#ifdef DEBUG_SDRAM
> +    printf("SPD: nbanks %d density %d\n", nbanks, density);
> +#endif
> +    /* XXX: this is totally random */
> +    spd[9] = 0x10; /* CAS tcyc */
> +    spd[18] = 0x20; /* CAS bit */
> +    spd[23] = 0x10; /* CAS tcyc */
> +    spd[25] = 0x10; /* CAS tcyc */
> +
> +    /* checksum */
> +    spd[63] = 0;
> +    for (i = 0; i < 63; i++) {
> +        spd[63] += spd[i];
> +    }
> +
> +    /* copy for SMBUS */
> +    memcpy(eeprom, spd, sizeof(spd_eeprom.contents));
> +}
> +
> +static void generate_eeprom_serial(uint8_t *eeprom)
> +{
> +    int i, pos = 0;
> +    uint8_t mac[6] = { 0x00 };
> +    uint8_t sn[5] = { 0x01, 0x23, 0x45, 0x67, 0x89 };
> +
> +    /* version */
> +    eeprom[pos++] = 0x01;
> +
> +    /* count */
> +    eeprom[pos++] = 0x02;
> +
> +    /* MAC address */
> +    eeprom[pos++] = 0x01; /* MAC */
> +    eeprom[pos++] = 0x06; /* length */
> +    memcpy(&eeprom[pos], mac, sizeof(mac));
> +    pos += sizeof(mac);
> +
> +    /* serial number */
> +    eeprom[pos++] = 0x02; /* serial */
> +    eeprom[pos++] = 0x05; /* length */
> +    memcpy(&eeprom[pos], sn, sizeof(sn));
> +    pos += sizeof(sn);
> +
> +    /* checksum */
> +    eeprom[pos] = 0;
> +    for (i = 0; i < pos; i++) {
> +        eeprom[pos] += eeprom[i];
> +    }
> +}
> +
> +/*****************************************************************************/
> +
> +static int sam460ex_load_uboot(void)
> +{
> +    DriveInfo *dinfo;
> +    BlockBackend *blk = NULL;
> +    hwaddr base = FLASH_BASE | ((hwaddr)FLASH_BASE_H << 32);
> +    long bios_size = FLASH_SIZE;
> +    int fl_sectors;
> +
> +    dinfo = drive_get(IF_PFLASH, 0, 0);
> +    if (dinfo) {
> +        blk = blk_by_legacy_dinfo(dinfo);
> +        bios_size = blk_getlength(blk);
> +    }
> +    fl_sectors = (bios_size + 65535) >> 16;
> +
> +    if (!pflash_cfi01_register(base, NULL, "sam460ex.flash", bios_size,
> +                               blk, (64 * 1024), fl_sectors,
> +                               1, 0x89, 0x18, 0x0000, 0x0, 1)) {
> +        fprintf(stderr, "qemu: Error registering flash memory.\n");
> +        /* XXX: return an error instead? */
> +        exit(1);
> +    }
> +
> +    if (!blk) {
> +        /*fprintf(stderr, "No flash image given with the 'pflash' parameter,"
> +                " using default u-boot image\n");*/
> +        base = UBOOT_LOAD_BASE | ((hwaddr)FLASH_BASE_H << 32);
> +        rom_add_file_fixed(UBOOT_FILENAME, base, -1);
> +    }
> +
> +    return 0;
> +}
> +
> +static int sam460ex_load_device_tree(hwaddr addr,

This function looks generic enough to go in hw/ppc/loader.c eventually.

> +                                     uint32_t ramsize,
> +                                     hwaddr initrd_base,
> +                                     hwaddr initrd_size,
> +                                     const char *kernel_cmdline)
> +{
> +    int ret = -1;
> +    uint32_t mem_reg_property[] = { 0, 0, cpu_to_be32(ramsize) };
> +    char *filename;
> +    int fdt_size;
> +    void *fdt;
> +    uint32_t tb_freq = 400000000;
> +    uint32_t clock_freq = 400000000;
> +
> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE);
> +    if (!filename) {
> +        goto out;
> +    }
> +    fdt = load_device_tree(filename, &fdt_size);
> +    g_free(filename);
> +    if (fdt == NULL) {
> +        goto out;
> +    }
> +
> +    /* Manipulate device tree in memory. */
> +
> +    ret = qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
> +                               sizeof(mem_reg_property));
> +    if (ret < 0) {
> +        fprintf(stderr, "couldn't set /memory/reg\n");
> +    }
> +
> +    /* default FDT doesn't have a /chosen node... */
> +    qemu_fdt_add_subnode(fdt, "/chosen");
> +
> +    ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
> +                                    initrd_base);
> +    if (ret < 0) {
> +        fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
> +    }
> +
> +    ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
> +                                    (initrd_base + initrd_size));
> +    if (ret < 0) {
> +        fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
> +    }
> +
> +    ret = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
> +                                      kernel_cmdline);
> +    if (ret < 0) {
> +        fprintf(stderr, "couldn't set /chosen/bootargs\n");
> +    }
> +
> +    /* Copy data from the host device tree into the guest. Since the guest can
> +     * directly access the timebase without host involvement, we must expose
> +     * the correct frequencies. */
> +    if (kvm_enabled()) {
> +        tb_freq = kvmppc_get_tbfreq();
> +        clock_freq = kvmppc_get_clockfreq();
> +    }
> +
> +    qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "clock-frequency",
> +                              clock_freq);
> +    qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "timebase-frequency",
> +                              tb_freq);
> +
> +    rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
> +    g_free(fdt);
> +    ret = fdt_size;
> +
> +out:
> +
> +    return ret;
> +}
> +
> +/* Create reset TLB entries for BookE, mapping only the flash memory.  */
> +static void mmubooke_create_initial_mapping_uboot(CPUPPCState *env)
> +{
> +    ppcemb_tlb_t *tlb = &env->tlb.tlbe[0];
> +
> +    /* on reset the flash is mapped by a shadow TLB,
> +     * but since we don't implement them we need to use
> +     * the same values U-Boot will use to avoid a fault.
> +     */
> +    tlb->attr = 0;
> +    tlb->prot = PAGE_VALID | ((PAGE_READ | PAGE_WRITE | PAGE_EXEC) << 4);
> +    tlb->size = 0x10000000; /* up to 0xffffffff  */
> +    tlb->EPN = 0xf0000000 & TARGET_PAGE_MASK;
> +    tlb->RPN = (0xf0000000 & TARGET_PAGE_MASK) | 0x4;
> +    tlb->PID = 0;
> +}
> +
> +/* Create reset TLB entries for BookE, spanning the 32bit addr space.  */
> +static void mmubooke_create_initial_mapping(CPUPPCState *env,
> +                                     target_ulong va,
> +                                     hwaddr pa)
> +{
> +    ppcemb_tlb_t *tlb = &env->tlb.tlbe[0];
> +
> +    tlb->attr = 0;
> +    tlb->prot = PAGE_VALID | ((PAGE_READ | PAGE_WRITE | PAGE_EXEC) << 4);
> +    tlb->size = 1 << 31; /* up to 0x80000000  */
> +    tlb->EPN = va & TARGET_PAGE_MASK;
> +    tlb->RPN = pa & TARGET_PAGE_MASK;
> +    tlb->PID = 0;
> +}
> +
> +static void main_cpu_reset(void *opaque)
> +{
> +    PowerPCCPU *cpu = opaque;
> +    CPUPPCState *env = &cpu->env;
> +    struct boot_info *bi = env->load_info;
> +
> +    cpu_reset(CPU(cpu));
> +
> +    /* either we have a kernel to boot or we jump to U-Boot */
> +    if (bi->entry != UBOOT_ENTRY) {
> +        env->gpr[1] = (16 << 20) - 8;
> +        env->gpr[3] = FDT_ADDR;
> +
> +        fprintf(stderr, "cpu reset: kernel entry %x\n", bi->entry);
> +        env->nip = bi->entry;
> +
> +        /* Create a mapping for the kernel.  */
> +        mmubooke_create_initial_mapping(env, 0, 0);
> +        env->gpr[6] = tswap32(EPAPR_MAGIC);
> +        env->gpr[7] = (16 << 20) - 8; /*bi->ima_size;*/
> +
> +    } else {
> +        env->nip = UBOOT_ENTRY;
> +        mmubooke_create_initial_mapping_uboot(env);
> +        fprintf(stderr, "cpu reset: U-Boot entry\n");
> +    }
> +}
> +
> +static void sam460ex_init(MachineState *machine)
> +{
> +    MemoryRegion *address_space_mem = get_system_memory();
> +    MemoryRegion *isa = g_new(MemoryRegion, 1);
> +    MemoryRegion *ram_memories = g_new(MemoryRegion, SDRAM_NR_BANKS);
> +    hwaddr ram_bases[SDRAM_NR_BANKS];
> +    hwaddr ram_sizes[SDRAM_NR_BANKS];
> +    MemoryRegion *l2cache_ram = g_new(MemoryRegion, 1);
> +    qemu_irq *irqs, *uic[4];
> +    PCIBus *pci_bus;
> +    PowerPCCPU *cpu;
> +    CPUPPCState *env;
> +    PPC4xxI2CState *i2c[2];
> +    hwaddr entry = UBOOT_ENTRY;
> +    hwaddr loadaddr = 0;
> +    target_long initrd_size = 0;
> +    DeviceState *dev;
> +    SysBusDevice *sbdev;
> +    int success;
> +    int i;
> +    struct boot_info *boot_info;
> +    const size_t smbus_eeprom_size = 8 * 256;
> +    uint8_t *smbus_eeprom_buf = g_malloc0(smbus_eeprom_size);
> +
> +    /* Setup CPU. */
> +    if (machine->cpu_model == NULL) {
> +        machine->cpu_model = "460EX";
> +    }
> +    cpu = cpu_ppc_init(machine->cpu_model);
> +    if (cpu == NULL) {
> +        fprintf(stderr, "Unable to initialize CPU!\n");
> +        exit(1);
> +    }
> +    env = &cpu->env;
> +
> +    qemu_register_reset(main_cpu_reset, cpu);
> +    boot_info = g_malloc0(sizeof(*boot_info));
> +    env->load_info = boot_info;
> +
> +    ppc_booke_timers_init(cpu, 50000000, 0);
> +    ppc_dcr_init(env, NULL, NULL);
> +
> +    /* PLB arbitrer */
> +    ppc4xx_plb_init(env);
> +
> +    /* interrupt controllers */
> +    irqs = g_malloc0(sizeof(*irqs) * PPCUIC_OUTPUT_NB);
> +    irqs[PPCUIC_OUTPUT_INT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT];
> +    irqs[PPCUIC_OUTPUT_CINT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT];
> +    uic[0] = ppcuic_init(env, irqs, 0x0C0, 0, 1);
> +    uic[1] = ppcuic_init(env, &uic[0][30], 0x0D0, 0, 1);
> +    uic[2] = ppcuic_init(env, &uic[0][10], 0x0E0, 0, 1);
> +    uic[3] = ppcuic_init(env, &uic[0][16], 0x0F0, 0, 1);
> +
> +    /* SDRAM controller */
> +    memset(ram_bases, 0, sizeof(ram_bases));
> +    memset(ram_sizes, 0, sizeof(ram_sizes));
> +    /* put all RAM on first bank because board has one slot
> +     * and firmware only checks that */
> +    machine->ram_size = ppc4xx_sdram_adjust(machine->ram_size,
> +                                   1/*SDRAM_NR_BANKS*/,
> +                                   ram_memories,
> +                                   ram_bases, ram_sizes,
> +                                   ppc460ex_sdram_bank_sizes);
> +#ifdef DEBUG_SDRAM
> +    printf("RAMSIZE %dMB\n", (int)(machine->ram_size / M_BYTE));
> +#endif
> +
> +    /* XXX does 460EX have ECC interrupts? */
> +    ppc440_sdram_init(env, SDRAM_NR_BANKS, ram_memories,
> +                      ram_bases, ram_sizes, 1);
> +
> +    /* generate SPD EEPROM data */
> +    for (i = 0; i < SDRAM_NR_BANKS; i++) {
> +#ifdef DEBUG_SDRAM
> +        printf("bank %d: %" HWADDR_PRIx "\n", i, ram_sizes[i]);
> +#endif
> +        generate_eeprom_spd(&smbus_eeprom_buf[i * 256], ram_sizes[i]);
> +    }
> +    generate_eeprom_serial(&smbus_eeprom_buf[4 * 256]);
> +    generate_eeprom_serial(&smbus_eeprom_buf[6 * 256]);
> +
> +    /* IIC controllers */
> +    dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600700, uic[0][2]);
> +    i2c[0] = PPC4xx_I2C(dev);
> +    object_property_set_bool(OBJECT(dev), true, "realized", NULL);
> +    smbus_eeprom_init(i2c[0]->bus, 8, smbus_eeprom_buf, smbus_eeprom_size);
> +    g_free(smbus_eeprom_buf);
> +
> +    dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600800, uic[0][3]);
> +    i2c[1] = PPC4xx_I2C(dev);
> +
> +    /* External bus controller */
> +    ppc405_ebc_init(env);
> +
> +    /* CPR */
> +    ppc4xx_cpr_init(env);
> +
> +    /* PLB to AHB bridge */
> +    ppc4xx_ahb_init(env);
> +
> +    /* System DCRs */
> +    ppc4xx_sdr_init(env);
> +
> +    /* MAL */
> +    ppc4xx_mal_init(env, 4, 16, &uic[2][3]);
> +
> +    /* 256K of L2 cache as memory */
> +    ppc4xx_l2sram_init(env);
> +    /* FIXME: remove this after fixing l2sram mapping in ppc440_uc.c? */
> +    memory_region_init_ram(l2cache_ram, NULL, "ppc440.l2cache_ram", 256 << 10,
> +                           &error_abort);
> +    memory_region_add_subregion(address_space_mem, 0x400000000LL, l2cache_ram);
> +
> +    /* USB */
> +    sysbus_create_simple(TYPE_PPC4xx_EHCI, 0x4bffd0400, uic[2][29]);
> +    dev = qdev_create(NULL, "sysbus-ohci");
> +    qdev_prop_set_string(dev, "masterbus", "usb-bus.0");
> +    qdev_prop_set_uint32(dev, "num-ports", 6);
> +    qdev_init_nofail(dev);
> +    sbdev = SYS_BUS_DEVICE(dev);
> +    sysbus_mmio_map(sbdev, 0, 0x4bffd0000);
> +    sysbus_connect_irq(sbdev, 0, uic[2][30]);
> +    usb_create_simple(usb_bus_find(-1), "usb-kbd");
> +    usb_create_simple(usb_bus_find(-1), "usb-mouse");
> +
> +    /* PCI bus */
> +    ppc460ex_pcie_init(env);
> +    /*XXX: FIXME: is this correct? */
> +    dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec00000,
> +                                uic[1][0], uic[1][20], uic[1][21], uic[1][22],
> +                                NULL);
> +    pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
> +    if (!pci_bus) {
> +        fprintf(stderr, "couldn't create PCI controller!\n");
> +        exit(1);
> +    }
> +    memory_region_init_alias(isa, NULL, "isa_mmio", get_system_io(),
> +                             0, 0x10000);
> +    memory_region_add_subregion(get_system_memory(), 0xc08000000, isa);
> +
> +    /* PCI devices */
> +    pci_create_simple(pci_bus, PCI_DEVFN(6, 0), "sm501");
> +    /* SoC has a single SATA port but we don't emulate that yet
> +     * However, firmware and usual clients have driver for SiI311x
> +     * so add one for convenience by default */
> +    pci_create_simple(pci_bus, -1, "sii3112");
> +
> +    /* SoC has 4 UARTs
> +     * but board has only one wired and two are present in fdt */
> +    if (serial_hds[0] != NULL) {
> +        serial_mm_init(address_space_mem, 0x4ef600300, 0, uic[1][1],
> +                       PPC_SERIAL_MM_BAUDBASE, serial_hds[0],
> +                       DEVICE_BIG_ENDIAN);
> +    }
> +    if (serial_hds[1] != NULL) {
> +        serial_mm_init(address_space_mem, 0x4ef600400, 0, uic[0][1],
> +                       PPC_SERIAL_MM_BAUDBASE, serial_hds[1],
> +                       DEVICE_BIG_ENDIAN);
> +    }
> +
> +    /* Load U-Boot image. */
> +    if (!machine->kernel_filename) {
> +        success = sam460ex_load_uboot();
> +        if (success < 0) {
> +            fprintf(stderr, "qemu: could not load firmware\n");
> +            exit(1);
> +        }
> +    }
> +
> +    /* Load kernel. */
> +    if (machine->kernel_filename) {
> +        success = load_uimage(machine->kernel_filename, &entry, &loadaddr, NULL,
> +            NULL, NULL);
> +        fprintf(stderr, "load_uimage: %d\n", success);
> +        if (success < 0) {
> +            uint64_t elf_entry, elf_lowaddr;
> +
> +            success = load_elf(machine->kernel_filename, NULL, NULL, &elf_entry,
> +                               &elf_lowaddr, NULL, 1, PPC_ELF_MACHINE, 0, 0);
> +            fprintf(stderr, "load_elf: %d\n", success);
> +            entry = elf_entry;
> +            loadaddr = elf_lowaddr;
> +        }
> +        /* XXX try again as binary */
> +        if (success < 0) {
> +            fprintf(stderr, "qemu: could not load kernel '%s'\n",
> +                    machine->kernel_filename);
> +            exit(1);
> +        }
> +    }
> +
> +    /* Load initrd. */
> +    if (machine->initrd_filename) {
> +        initrd_size = load_image_targphys(machine->initrd_filename,
> +                                          RAMDISK_ADDR,
> +                                          machine->ram_size - RAMDISK_ADDR);
> +        fprintf(stderr, "load_image: %d\n", initrd_size);
> +        if (initrd_size < 0) {
> +            fprintf(stderr, "qemu: could not load ram disk '%s' at %x\n",
> +                    machine->initrd_filename, RAMDISK_ADDR);
> +            exit(1);
> +        }
> +    }
> +
> +    /* If we're loading a kernel directly, we must load the device tree too. */
> +    if (machine->kernel_filename) {
> +        int dt_size;
> +
> +        dt_size = sam460ex_load_device_tree(FDT_ADDR, machine->ram_size,
> +                                    RAMDISK_ADDR, initrd_size,
> +                                    machine->kernel_cmdline);
> +        if (dt_size < 0) {
> +            fprintf(stderr, "couldn't load device tree\n");
> +            exit(1);
> +        }
> +
> +        boot_info->dt_base = FDT_ADDR;
> +        boot_info->dt_size = dt_size;
> +    }
> +
> +    boot_info->entry = entry;
> +}
> +
> +static void sam460ex_machine_init(MachineClass *mc)
> +{
> +    mc->desc = "aCube Sam460ex";
> +    mc->init = sam460ex_init;
> +    mc->default_ram_size = 512 * M_BYTE;
> +}
> +
> +DEFINE_MACHINE("sam460ex", sam460ex_machine_init)
>
David Gibson Aug. 23, 2017, 4:16 a.m. UTC | #2
On Sun, Aug 20, 2017 at 07:23:05PM +0200, BALATON Zoltan wrote:
> Add emulation of aCube Sam460ex board based on AMCC 460EX embedded SoC.
> This is not a full implementation yet with a lot of components still
> missing but enough to start a Linux kernel and the U-Boot firmware.
> 
> Signed-off-by: François Revol <revol@free.fr>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>

As usual, only fairly superficial review here.

> ---
>  default-configs/ppcemb-softmmu.mak |   3 +
>  hw/ppc/Makefile.objs               |   2 +
>  hw/ppc/sam460ex.c                  | 611 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 616 insertions(+)
>  create mode 100644 hw/ppc/sam460ex.c
> 
> diff --git a/default-configs/ppcemb-softmmu.mak b/default-configs/ppcemb-softmmu.mak
> index 635923a..90b42f0 100644
> --- a/default-configs/ppcemb-softmmu.mak
> +++ b/default-configs/ppcemb-softmmu.mak
> @@ -17,3 +17,6 @@ CONFIG_XILINX=y
>  CONFIG_XILINX_ETHLITE=y
>  CONFIG_LIBDECNUMBER=y
>  CONFIG_SM501=y
> +CONFIG_USB_EHCI_SYSBUS=y
> +CONFIG_IDE_SII3112=y
> +CONFIG_SAM460EX=y
> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> index fc39fe4..0aaea5b 100644
> --- a/hw/ppc/Makefile.objs
> +++ b/hw/ppc/Makefile.objs
> @@ -27,3 +27,5 @@ obj-$(CONFIG_E500) += e500.o mpc8544ds.o e500plat.o
>  obj-$(CONFIG_E500) += mpc8544_guts.o ppce500_spin.o
>  # PowerPC 440 Xilinx ML507 reference board.
>  obj-$(CONFIG_XILINX) += virtex_ml507.o
> +# ACube Sam460ex board.
> +obj-$(CONFIG_SAM460EX) += ppc440_uc.o sam460ex.o
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> new file mode 100644
> index 0000000..7cf4f6f
> --- /dev/null
> +++ b/hw/ppc/sam460ex.c
> @@ -0,0 +1,611 @@
> +/*
> + * QEMU aCube Sam460ex board emulation
> + *
> + * Copyright (c) 2012 François Revol
> + * Copyright (c) 2016-2017 BALATON Zoltan
> + *
> + * This file is derived from hw/ppc440_bamboo.c,
> + * the copyright for that material belongs to the original owners.
> + *
> + * This work is licensed under the GNU GPL license version 2 or later.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "qemu/cutils.h"
> +#include "qapi/error.h"
> +#include "hw/hw.h"
> +#include "sysemu/blockdev.h"
> +#include "hw/boards.h"
> +#include "sysemu/kvm.h"
> +#include "kvm_ppc.h"
> +#include "sysemu/device_tree.h"
> +#include "sysemu/block-backend.h"
> +#include "hw/loader.h"
> +#include "elf.h"
> +#include "exec/address-spaces.h"
> +#include "exec/memory.h"
> +#include "hw/ppc/ppc440.h"
> +#include "hw/ppc/ppc405.h"
> +#include "hw/block/flash.h"
> +#include "sysemu/sysemu.h"
> +#include "hw/sysbus.h"
> +#include "hw/char/serial.h"
> +#include "hw/i2c/ppc4xx_i2c.h"
> +#include "hw/i2c/smbus.h"
> +#include "hw/usb/hcd-ehci.h"
> +
> +#define BINARY_DEVICE_TREE_FILE "sam460ex.dtb"
> +#define UBOOT_FILENAME "u-boot-sam460-20100605.bin"
> +/* to extract the official U-Boot bin from the updater: */
> +/* dd bs=1 skip=$(($(stat -c '%s' updater/updater-460) - 0x80000)) \
> +     if=updater/updater-460 of=u-boot-sam460-20100605.bin */
> +
> +/* from Sam460 U-Boot include/configs/Sam460ex.h */
> +#define FLASH_BASE             0xfff00000
> +#define FLASH_BASE_H           0x4
> +#define FLASH_SIZE             (1 << 20)
> +#define UBOOT_LOAD_BASE        0xfff80000
> +#define UBOOT_SIZE             0x00080000
> +#define UBOOT_ENTRY            0xfffffffc
> +
> +/* from U-Boot */
> +#define EPAPR_MAGIC           (0x45504150)
> +#define KERNEL_ADDR           0x1000000
> +#define FDT_ADDR              0x1800000
> +#define RAMDISK_ADDR          0x1900000
> +
> +/* Sam460ex IRQ MAP:
> +   IRQ0  = ETH_INT
> +   IRQ1  = FPGA_INT
> +   IRQ2  = PCI_INT (PCIA, PCIB, PCIC, PCIB)
> +   IRQ3  = FPGA_INT2
> +   IRQ11 = RTC_INT
> +   IRQ12 = SM502_INT
> +*/
> +
> +#define SDRAM_NR_BANKS 4
> +
> +/* FIXME: See u-boot.git 8ac41e */
> +static const unsigned int ppc460ex_sdram_bank_sizes[] = {
> +    1024 << 20, 512 << 20, 256 << 20, 128 << 20, 64 << 20, 32 << 20, 0
> +};
> +
> +struct boot_info {
> +    uint32_t dt_base;
> +    uint32_t dt_size;
> +    uint32_t entry;
> +};
> +
> +/*****************************************************************************/
> +/* SPD eeprom content from mips_malta.c */

What's the connection with mips_malta?

> +struct _eeprom24c0x_t {
> +  uint8_t tick;
> +  uint8_t address;
> +  uint8_t command;
> +  uint8_t ack;
> +  uint8_t scl;
> +  uint8_t sda;
> +  uint8_t data;
> +  uint8_t contents[256];
> +};
> +
> +typedef struct _eeprom24c0x_t eeprom24c0x_t;
> +
> +static eeprom24c0x_t spd_eeprom = {
> +    .contents = {
> +        /* 00000000: */ 0x80, 0x08, 0xFF, 0x0D, 0x0A, 0xFF, 0x40, 0x00,
> +        /* 00000008: */ 0x04, 0x75, 0x54, 0x00, 0x82, 0x08, 0x00, 0x01,
> +        /* 00000010: */ 0x8F, 0x04, 0x02, 0x01, 0x01, 0x00, 0x00, 0x00,
> +        /* 00000018: */ 0x00, 0x00, 0x00, 0x14, 0x0F, 0x14, 0x2D, 0xFF,
> +        /* 00000020: */ 0x15, 0x08, 0x15, 0x08, 0x00, 0x00, 0x00, 0x00,
> +        /* 00000028: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +        /* 00000030: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +        /* 00000038: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x12, 0xD0,
> +        /* 00000040: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +        /* 00000048: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +        /* 00000050: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +        /* 00000058: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +        /* 00000060: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +        /* 00000068: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +        /* 00000070: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +        /* 00000078: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x64, 0xF4,
> +    },
> +};
> +
> +static void generate_eeprom_spd(uint8_t *eeprom, ram_addr_t ram_size)
> +{
> +    enum { SDR = 0x4, DDR1 = 0x7, DDR2 = 0x8 } type;
> +    uint8_t *spd = spd_eeprom.contents;
> +    uint8_t nbanks = 0;
> +    uint16_t density = 0;
> +    int i;
> +
> +    /* work in terms of MB */
> +    ram_size >>= 20;
> +
> +    while ((ram_size >= 4) && (nbanks <= 2)) {
> +        int sz_log2 = MIN(31 - clz32(ram_size), 14);
> +        nbanks++;
> +        density |= 1 << (sz_log2 - 2);
> +        ram_size -= 1 << sz_log2;
> +    }
> +
> +    /* split to 2 banks if possible */
> +    if ((nbanks == 1) && (density > 1)) {
> +        nbanks++;
> +        density >>= 1;
> +    }
> +
> +    if (density & 0xff00) {
> +        density = (density & 0xe0) | ((density >> 8) & 0x1f);
> +        type = DDR2;
> +    } else if (!(density & 0x1f)) {
> +        type = DDR2;
> +    } else {
> +        type = SDR;
> +    }
> +
> +    if (ram_size) {
> +        fprintf(stderr, "Warning: SPD cannot represent final %dMB"
> +                " of SDRAM\n", (int)ram_size);
> +    }
> +
> +    /* fill in SPD memory information */
> +    spd[2] = type;
> +    spd[5] = nbanks;
> +    spd[31] = density;
> +#ifdef DEBUG_SDRAM
> +    printf("SPD: nbanks %d density %d\n", nbanks, density);
> +#endif
> +    /* XXX: this is totally random */
> +    spd[9] = 0x10; /* CAS tcyc */
> +    spd[18] = 0x20; /* CAS bit */
> +    spd[23] = 0x10; /* CAS tcyc */
> +    spd[25] = 0x10; /* CAS tcyc */
> +
> +    /* checksum */
> +    spd[63] = 0;
> +    for (i = 0; i < 63; i++) {
> +        spd[63] += spd[i];
> +    }
> +
> +    /* copy for SMBUS */
> +    memcpy(eeprom, spd, sizeof(spd_eeprom.contents));
> +}
> +
> +static void generate_eeprom_serial(uint8_t *eeprom)
> +{
> +    int i, pos = 0;
> +    uint8_t mac[6] = { 0x00 };
> +    uint8_t sn[5] = { 0x01, 0x23, 0x45, 0x67, 0x89 };
> +
> +    /* version */
> +    eeprom[pos++] = 0x01;
> +
> +    /* count */
> +    eeprom[pos++] = 0x02;
> +
> +    /* MAC address */
> +    eeprom[pos++] = 0x01; /* MAC */
> +    eeprom[pos++] = 0x06; /* length */
> +    memcpy(&eeprom[pos], mac, sizeof(mac));
> +    pos += sizeof(mac);
> +
> +    /* serial number */
> +    eeprom[pos++] = 0x02; /* serial */
> +    eeprom[pos++] = 0x05; /* length */
> +    memcpy(&eeprom[pos], sn, sizeof(sn));
> +    pos += sizeof(sn);
> +
> +    /* checksum */
> +    eeprom[pos] = 0;
> +    for (i = 0; i < pos; i++) {
> +        eeprom[pos] += eeprom[i];
> +    }
> +}
> +
> +/*****************************************************************************/
> +
> +static int sam460ex_load_uboot(void)
> +{
> +    DriveInfo *dinfo;
> +    BlockBackend *blk = NULL;
> +    hwaddr base = FLASH_BASE | ((hwaddr)FLASH_BASE_H << 32);
> +    long bios_size = FLASH_SIZE;
> +    int fl_sectors;
> +
> +    dinfo = drive_get(IF_PFLASH, 0, 0);
> +    if (dinfo) {
> +        blk = blk_by_legacy_dinfo(dinfo);
> +        bios_size = blk_getlength(blk);
> +    }
> +    fl_sectors = (bios_size + 65535) >> 16;
> +
> +    if (!pflash_cfi01_register(base, NULL, "sam460ex.flash", bios_size,
> +                               blk, (64 * 1024), fl_sectors,
> +                               1, 0x89, 0x18, 0x0000, 0x0, 1)) {
> +        fprintf(stderr, "qemu: Error registering flash memory.\n");

Use error_report() instead, please.

> +        /* XXX: return an error instead? */
> +        exit(1);
> +    }
> +
> +    if (!blk) {
> +        /*fprintf(stderr, "No flash image given with the 'pflash' parameter,"
> +                " using default u-boot image\n");*/
> +        base = UBOOT_LOAD_BASE | ((hwaddr)FLASH_BASE_H << 32);
> +        rom_add_file_fixed(UBOOT_FILENAME, base, -1);
> +    }
> +
> +    return 0;
> +}
> +
> +static int sam460ex_load_device_tree(hwaddr addr,
> +                                     uint32_t ramsize,
> +                                     hwaddr initrd_base,
> +                                     hwaddr initrd_size,
> +                                     const char *kernel_cmdline)
> +{
> +    int ret = -1;
> +    uint32_t mem_reg_property[] = { 0, 0, cpu_to_be32(ramsize) };
> +    char *filename;
> +    int fdt_size;
> +    void *fdt;
> +    uint32_t tb_freq = 400000000;
> +    uint32_t clock_freq = 400000000;
> +
> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE);
> +    if (!filename) {
> +        goto out;
> +    }
> +    fdt = load_device_tree(filename, &fdt_size);
> +    g_free(filename);
> +    if (fdt == NULL) {
> +        goto out;
> +    }
> +
> +    /* Manipulate device tree in memory. */
> +
> +    ret = qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
> +                               sizeof(mem_reg_property));
> +    if (ret < 0) {
> +        fprintf(stderr, "couldn't set /memory/reg\n");

And in all these places.

> +    }
> +
> +    /* default FDT doesn't have a /chosen node... */
> +    qemu_fdt_add_subnode(fdt, "/chosen");
> +
> +    ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
> +                                    initrd_base);
> +    if (ret < 0) {
> +        fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
> +    }
> +
> +    ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
> +                                    (initrd_base + initrd_size));
> +    if (ret < 0) {
> +        fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
> +    }
> +
> +    ret = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
> +                                      kernel_cmdline);
> +    if (ret < 0) {
> +        fprintf(stderr, "couldn't set /chosen/bootargs\n");
> +    }
> +
> +    /* Copy data from the host device tree into the guest. Since the guest can
> +     * directly access the timebase without host involvement, we must expose
> +     * the correct frequencies. */
> +    if (kvm_enabled()) {
> +        tb_freq = kvmppc_get_tbfreq();
> +        clock_freq = kvmppc_get_clockfreq();
> +    }
> +
> +    qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "clock-frequency",
> +                              clock_freq);
> +    qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "timebase-frequency",
> +                              tb_freq);
> +
> +    rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
> +    g_free(fdt);
> +    ret = fdt_size;
> +
> +out:
> +
> +    return ret;
> +}
> +
> +/* Create reset TLB entries for BookE, mapping only the flash memory.  */
> +static void mmubooke_create_initial_mapping_uboot(CPUPPCState *env)
> +{
> +    ppcemb_tlb_t *tlb = &env->tlb.tlbe[0];
> +
> +    /* on reset the flash is mapped by a shadow TLB,
> +     * but since we don't implement them we need to use
> +     * the same values U-Boot will use to avoid a fault.
> +     */

Usually the reset state of the MMU is handled in the cpu code rather
than the board code.  Is there a specific reason you need it in the
board code here?

> +    tlb->attr = 0;
> +    tlb->prot = PAGE_VALID | ((PAGE_READ | PAGE_WRITE | PAGE_EXEC) << 4);
> +    tlb->size = 0x10000000; /* up to 0xffffffff  */
> +    tlb->EPN = 0xf0000000 & TARGET_PAGE_MASK;
> +    tlb->RPN = (0xf0000000 & TARGET_PAGE_MASK) | 0x4;
> +    tlb->PID = 0;
> +}
> +
> +/* Create reset TLB entries for BookE, spanning the 32bit addr space.  */
> +static void mmubooke_create_initial_mapping(CPUPPCState *env,
> +                                     target_ulong va,
> +                                     hwaddr pa)
> +{
> +    ppcemb_tlb_t *tlb = &env->tlb.tlbe[0];
> +
> +    tlb->attr = 0;
> +    tlb->prot = PAGE_VALID | ((PAGE_READ | PAGE_WRITE | PAGE_EXEC) << 4);
> +    tlb->size = 1 << 31; /* up to 0x80000000  */
> +    tlb->EPN = va & TARGET_PAGE_MASK;
> +    tlb->RPN = pa & TARGET_PAGE_MASK;
> +    tlb->PID = 0;
> +}
> +
> +static void main_cpu_reset(void *opaque)
> +{
> +    PowerPCCPU *cpu = opaque;
> +    CPUPPCState *env = &cpu->env;
> +    struct boot_info *bi = env->load_info;
> +
> +    cpu_reset(CPU(cpu));
> +
> +    /* either we have a kernel to boot or we jump to U-Boot */
> +    if (bi->entry != UBOOT_ENTRY) {
> +        env->gpr[1] = (16 << 20) - 8;
> +        env->gpr[3] = FDT_ADDR;
> +
> +        fprintf(stderr, "cpu reset: kernel entry %x\n", bi->entry);

These should be tracepoints rather than bare fprintf().

> +        env->nip = bi->entry;
> +
> +        /* Create a mapping for the kernel.  */
> +        mmubooke_create_initial_mapping(env, 0, 0);
> +        env->gpr[6] = tswap32(EPAPR_MAGIC);

I'm pretty sure the tswap can't be right here.  env->gpr is in host
native order and I'd expect the constant to be as well.  

> +        env->gpr[7] = (16 << 20) - 8; /*bi->ima_size;*/

So, entering the kernel directly can be useful, particularly during
early development.  However, having both firmware and non-firmware
entry paths can lead to confusing bugs if there's a subtle difference
between the initial (to the kernel) states between the two paths.  For
that reason, the usual preferred way to implement -kernel is to still
run the usual firmware, but use some way of telling it to boot
immediately into the supplied kernel.

I won't object to merging it this way - just a wanrning that this may
bite you in the future if you're not careful.

> +
> +    } else {
> +        env->nip = UBOOT_ENTRY;
> +        mmubooke_create_initial_mapping_uboot(env);
> +        fprintf(stderr, "cpu reset: U-Boot entry\n");

Tracepoint.

> +    }
> +}
> +
> +static void sam460ex_init(MachineState *machine)
> +{
> +    MemoryRegion *address_space_mem = get_system_memory();
> +    MemoryRegion *isa = g_new(MemoryRegion, 1);
> +    MemoryRegion *ram_memories = g_new(MemoryRegion, SDRAM_NR_BANKS);
> +    hwaddr ram_bases[SDRAM_NR_BANKS];
> +    hwaddr ram_sizes[SDRAM_NR_BANKS];
> +    MemoryRegion *l2cache_ram = g_new(MemoryRegion, 1);
> +    qemu_irq *irqs, *uic[4];
> +    PCIBus *pci_bus;
> +    PowerPCCPU *cpu;
> +    CPUPPCState *env;
> +    PPC4xxI2CState *i2c[2];
> +    hwaddr entry = UBOOT_ENTRY;
> +    hwaddr loadaddr = 0;
> +    target_long initrd_size = 0;
> +    DeviceState *dev;
> +    SysBusDevice *sbdev;
> +    int success;
> +    int i;
> +    struct boot_info *boot_info;
> +    const size_t smbus_eeprom_size = 8 * 256;
> +    uint8_t *smbus_eeprom_buf = g_malloc0(smbus_eeprom_size);
> +
> +    /* Setup CPU. */
> +    if (machine->cpu_model == NULL) {
> +        machine->cpu_model = "460EX";
> +    }
> +    cpu = cpu_ppc_init(machine->cpu_model);
> +    if (cpu == NULL) {
> +        fprintf(stderr, "Unable to initialize CPU!\n");
> +        exit(1);
> +    }
> +    env = &cpu->env;
> +
> +    qemu_register_reset(main_cpu_reset, cpu);
> +    boot_info = g_malloc0(sizeof(*boot_info));
> +    env->load_info = boot_info;
> +
> +    ppc_booke_timers_init(cpu, 50000000, 0);
> +    ppc_dcr_init(env, NULL, NULL);
> +
> +    /* PLB arbitrer */
> +    ppc4xx_plb_init(env);
> +
> +    /* interrupt controllers */
> +    irqs = g_malloc0(sizeof(*irqs) * PPCUIC_OUTPUT_NB);
> +    irqs[PPCUIC_OUTPUT_INT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT];
> +    irqs[PPCUIC_OUTPUT_CINT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT];
> +    uic[0] = ppcuic_init(env, irqs, 0x0C0, 0, 1);
> +    uic[1] = ppcuic_init(env, &uic[0][30], 0x0D0, 0, 1);
> +    uic[2] = ppcuic_init(env, &uic[0][10], 0x0E0, 0, 1);
> +    uic[3] = ppcuic_init(env, &uic[0][16], 0x0F0, 0, 1);
> +
> +    /* SDRAM controller */
> +    memset(ram_bases, 0, sizeof(ram_bases));
> +    memset(ram_sizes, 0, sizeof(ram_sizes));
> +    /* put all RAM on first bank because board has one slot
> +     * and firmware only checks that */
> +    machine->ram_size = ppc4xx_sdram_adjust(machine->ram_size,
> +                                   1/*SDRAM_NR_BANKS*/,
> +                                   ram_memories,
> +                                   ram_bases, ram_sizes,
> +                                   ppc460ex_sdram_bank_sizes);
> +#ifdef DEBUG_SDRAM
> +    printf("RAMSIZE %dMB\n", (int)(machine->ram_size / M_BYTE));
> +#endif
> +
> +    /* XXX does 460EX have ECC interrupts? */
> +    ppc440_sdram_init(env, SDRAM_NR_BANKS, ram_memories,
> +                      ram_bases, ram_sizes, 1);
> +
> +    /* generate SPD EEPROM data */
> +    for (i = 0; i < SDRAM_NR_BANKS; i++) {
> +#ifdef DEBUG_SDRAM
> +        printf("bank %d: %" HWADDR_PRIx "\n", i, ram_sizes[i]);
> +#endif

Tracepoint.

> +        generate_eeprom_spd(&smbus_eeprom_buf[i * 256], ram_sizes[i]);
> +    }
> +    generate_eeprom_serial(&smbus_eeprom_buf[4 * 256]);
> +    generate_eeprom_serial(&smbus_eeprom_buf[6 * 256]);
> +
> +    /* IIC controllers */
> +    dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600700, uic[0][2]);
> +    i2c[0] = PPC4xx_I2C(dev);
> +    object_property_set_bool(OBJECT(dev), true, "realized", NULL);
> +    smbus_eeprom_init(i2c[0]->bus, 8, smbus_eeprom_buf, smbus_eeprom_size);
> +    g_free(smbus_eeprom_buf);
> +
> +    dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600800, uic[0][3]);
> +    i2c[1] = PPC4xx_I2C(dev);
> +
> +    /* External bus controller */
> +    ppc405_ebc_init(env);
> +
> +    /* CPR */
> +    ppc4xx_cpr_init(env);
> +
> +    /* PLB to AHB bridge */
> +    ppc4xx_ahb_init(env);
> +
> +    /* System DCRs */
> +    ppc4xx_sdr_init(env);
> +
> +    /* MAL */
> +    ppc4xx_mal_init(env, 4, 16, &uic[2][3]);
> +
> +    /* 256K of L2 cache as memory */
> +    ppc4xx_l2sram_init(env);

Seems like a lot of this peripheral setup should be handled by a SoC
helper function, since it's not really board specific (I'm guessing).
I'm ok with that being a later clean up though.

> +    /* FIXME: remove this after fixing l2sram mapping in ppc440_uc.c? */
> +    memory_region_init_ram(l2cache_ram, NULL, "ppc440.l2cache_ram", 256 << 10,
> +                           &error_abort);
> +    memory_region_add_subregion(address_space_mem, 0x400000000LL, l2cache_ram);
> +
> +    /* USB */
> +    sysbus_create_simple(TYPE_PPC4xx_EHCI, 0x4bffd0400, uic[2][29]);
> +    dev = qdev_create(NULL, "sysbus-ohci");
> +    qdev_prop_set_string(dev, "masterbus", "usb-bus.0");
> +    qdev_prop_set_uint32(dev, "num-ports", 6);
> +    qdev_init_nofail(dev);
> +    sbdev = SYS_BUS_DEVICE(dev);
> +    sysbus_mmio_map(sbdev, 0, 0x4bffd0000);
> +    sysbus_connect_irq(sbdev, 0, uic[2][30]);
> +    usb_create_simple(usb_bus_find(-1), "usb-kbd");
> +    usb_create_simple(usb_bus_find(-1), "usb-mouse");
> +
> +    /* PCI bus */
> +    ppc460ex_pcie_init(env);
> +    /*XXX: FIXME: is this correct? */
> +    dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec00000,
> +                                uic[1][0], uic[1][20], uic[1][21], uic[1][22],
> +                                NULL);
> +    pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
> +    if (!pci_bus) {
> +        fprintf(stderr, "couldn't create PCI controller!\n");
> +        exit(1);
> +    }
> +    memory_region_init_alias(isa, NULL, "isa_mmio", get_system_io(),
> +                             0, 0x10000);
> +    memory_region_add_subregion(get_system_memory(), 0xc08000000, isa);

Does it really make sense to just embed the ISA IO space here, rather
than actually instanting a PCI<->ISA bridge?

> +    /* PCI devices */
> +    pci_create_simple(pci_bus, PCI_DEVFN(6, 0), "sm501");
> +    /* SoC has a single SATA port but we don't emulate that yet
> +     * However, firmware and usual clients have driver for SiI311x
> +     * so add one for convenience by default */
> +    pci_create_simple(pci_bus, -1, "sii3112");

You should probably not create this when started with -nodefaults.

> +    /* SoC has 4 UARTs
> +     * but board has only one wired and two are present in fdt */
> +    if (serial_hds[0] != NULL) {
> +        serial_mm_init(address_space_mem, 0x4ef600300, 0, uic[1][1],
> +                       PPC_SERIAL_MM_BAUDBASE, serial_hds[0],
> +                       DEVICE_BIG_ENDIAN);
> +    }
> +    if (serial_hds[1] != NULL) {
> +        serial_mm_init(address_space_mem, 0x4ef600400, 0, uic[0][1],
> +                       PPC_SERIAL_MM_BAUDBASE, serial_hds[1],
> +                       DEVICE_BIG_ENDIAN);
> +    }
> +
> +    /* Load U-Boot image. */
> +    if (!machine->kernel_filename) {
> +        success = sam460ex_load_uboot();
> +        if (success < 0) {
> +            fprintf(stderr, "qemu: could not load firmware\n");
> +            exit(1);
> +        }
> +    }
> +
> +    /* Load kernel. */
> +    if (machine->kernel_filename) {
> +        success = load_uimage(machine->kernel_filename, &entry, &loadaddr, NULL,
> +            NULL, NULL);
> +        fprintf(stderr, "load_uimage: %d\n", success);

tracepoint

> +        if (success < 0) {
> +            uint64_t elf_entry, elf_lowaddr;
> +
> +            success = load_elf(machine->kernel_filename, NULL, NULL, &elf_entry,
> +                               &elf_lowaddr, NULL, 1, PPC_ELF_MACHINE, 0, 0);
> +            fprintf(stderr, "load_elf: %d\n", success);

tracepoint

> +            entry = elf_entry;
> +            loadaddr = elf_lowaddr;
> +        }
> +        /* XXX try again as binary */
> +        if (success < 0) {
> +            fprintf(stderr, "qemu: could not load kernel '%s'\n",
> +                    machine->kernel_filename);

error_report().

> +            exit(1);
> +        }
> +    }
> +
> +    /* Load initrd. */
> +    if (machine->initrd_filename) {
> +        initrd_size = load_image_targphys(machine->initrd_filename,
> +                                          RAMDISK_ADDR,
> +                                          machine->ram_size - RAMDISK_ADDR);
> +        fprintf(stderr, "load_image: %d\n", initrd_size);
> +        if (initrd_size < 0) {
> +            fprintf(stderr, "qemu: could not load ram disk '%s' at %x\n",
> +                    machine->initrd_filename, RAMDISK_ADDR);
> +            exit(1);
> +        }
> +    }
> +
> +    /* If we're loading a kernel directly, we must load the device tree too. */
> +    if (machine->kernel_filename) {
> +        int dt_size;
> +
> +        dt_size = sam460ex_load_device_tree(FDT_ADDR, machine->ram_size,
> +                                    RAMDISK_ADDR, initrd_size,
> +                                    machine->kernel_cmdline);
> +        if (dt_size < 0) {
> +            fprintf(stderr, "couldn't load device tree\n");
> +            exit(1);
> +        }
> +
> +        boot_info->dt_base = FDT_ADDR;
> +        boot_info->dt_size = dt_size;
> +    }
> +
> +    boot_info->entry = entry;
> +}
> +
> +static void sam460ex_machine_init(MachineClass *mc)
> +{
> +    mc->desc = "aCube Sam460ex";
> +    mc->init = sam460ex_init;
> +    mc->default_ram_size = 512 * M_BYTE;
> +}
> +
> +DEFINE_MACHINE("sam460ex", sam460ex_machine_init)
BALATON Zoltan Aug. 23, 2017, 11:12 a.m. UTC | #3
On Wed, 23 Aug 2017, David Gibson wrote:
> On Sun, Aug 20, 2017 at 07:23:05PM +0200, BALATON Zoltan wrote:
>> Add emulation of aCube Sam460ex board based on AMCC 460EX embedded SoC.
>> This is not a full implementation yet with a lot of components still
>> missing but enough to start a Linux kernel and the U-Boot firmware.
>>
>> Signed-off-by: François Revol <revol@free.fr>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>
> As usual, only fairly superficial review here.
>
>> ---
>>  default-configs/ppcemb-softmmu.mak |   3 +
>>  hw/ppc/Makefile.objs               |   2 +
>>  hw/ppc/sam460ex.c                  | 611 +++++++++++++++++++++++++++++++++++++
>>  3 files changed, 616 insertions(+)
>>  create mode 100644 hw/ppc/sam460ex.c
>>
>> diff --git a/default-configs/ppcemb-softmmu.mak b/default-configs/ppcemb-softmmu.mak
>> index 635923a..90b42f0 100644
>> --- a/default-configs/ppcemb-softmmu.mak
>> +++ b/default-configs/ppcemb-softmmu.mak
[...]
>> +/*****************************************************************************/
>> +/* SPD eeprom content from mips_malta.c */
>
> What's the connection with mips_malta?

The board's firmware wants to see SPD EEPROMs of the connected memory 
while initialising the memory controller. This is why we need to implement 
SDRAM controller, I2C and SPD EEPROMs. MIPS malta board had already SPD 
EEPROM implementation so this is based on that. The comment just indicates 
where this code comes from.

>> +struct _eeprom24c0x_t {
>> +  uint8_t tick;
>> +  uint8_t address;
>> +  uint8_t command;
>> +  uint8_t ack;
>> +  uint8_t scl;
>> +  uint8_t sda;
>> +  uint8_t data;
>> +  uint8_t contents[256];
>> +};
>> +
>> +typedef struct _eeprom24c0x_t eeprom24c0x_t;
>> +
>> +static eeprom24c0x_t spd_eeprom = {
>> +    .contents = {
>> +        /* 00000000: */ 0x80, 0x08, 0xFF, 0x0D, 0x0A, 0xFF, 0x40, 0x00,
>> +        /* 00000008: */ 0x04, 0x75, 0x54, 0x00, 0x82, 0x08, 0x00, 0x01,
>> +        /* 00000010: */ 0x8F, 0x04, 0x02, 0x01, 0x01, 0x00, 0x00, 0x00,
>> +        /* 00000018: */ 0x00, 0x00, 0x00, 0x14, 0x0F, 0x14, 0x2D, 0xFF,
>> +        /* 00000020: */ 0x15, 0x08, 0x15, 0x08, 0x00, 0x00, 0x00, 0x00,
>> +        /* 00000028: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> +        /* 00000030: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> +        /* 00000038: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x12, 0xD0,
>> +        /* 00000040: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> +        /* 00000048: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> +        /* 00000050: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> +        /* 00000058: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> +        /* 00000060: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> +        /* 00000068: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> +        /* 00000070: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> +        /* 00000078: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x64, 0xF4,
>> +    },
>> +};
>> +
>> +static void generate_eeprom_spd(uint8_t *eeprom, ram_addr_t ram_size)
>> +{
>> +    enum { SDR = 0x4, DDR1 = 0x7, DDR2 = 0x8 } type;
>> +    uint8_t *spd = spd_eeprom.contents;
>> +    uint8_t nbanks = 0;
>> +    uint16_t density = 0;
>> +    int i;
>> +
>> +    /* work in terms of MB */
>> +    ram_size >>= 20;
>> +
>> +    while ((ram_size >= 4) && (nbanks <= 2)) {
>> +        int sz_log2 = MIN(31 - clz32(ram_size), 14);
>> +        nbanks++;
>> +        density |= 1 << (sz_log2 - 2);
>> +        ram_size -= 1 << sz_log2;
>> +    }
>> +
>> +    /* split to 2 banks if possible */
>> +    if ((nbanks == 1) && (density > 1)) {
>> +        nbanks++;
>> +        density >>= 1;
>> +    }
>> +
>> +    if (density & 0xff00) {
>> +        density = (density & 0xe0) | ((density >> 8) & 0x1f);
>> +        type = DDR2;
>> +    } else if (!(density & 0x1f)) {
>> +        type = DDR2;
>> +    } else {
>> +        type = SDR;
>> +    }
>> +
>> +    if (ram_size) {
>> +        fprintf(stderr, "Warning: SPD cannot represent final %dMB"
>> +                " of SDRAM\n", (int)ram_size);
>> +    }
>> +
>> +    /* fill in SPD memory information */
>> +    spd[2] = type;
>> +    spd[5] = nbanks;
>> +    spd[31] = density;
>> +#ifdef DEBUG_SDRAM
>> +    printf("SPD: nbanks %d density %d\n", nbanks, density);
>> +#endif
>> +    /* XXX: this is totally random */
>> +    spd[9] = 0x10; /* CAS tcyc */
>> +    spd[18] = 0x20; /* CAS bit */
>> +    spd[23] = 0x10; /* CAS tcyc */
>> +    spd[25] = 0x10; /* CAS tcyc */
>> +
>> +    /* checksum */
>> +    spd[63] = 0;
>> +    for (i = 0; i < 63; i++) {
>> +        spd[63] += spd[i];
>> +    }
>> +
>> +    /* copy for SMBUS */
>> +    memcpy(eeprom, spd, sizeof(spd_eeprom.contents));
>> +}
>> +
>> +static void generate_eeprom_serial(uint8_t *eeprom)
>> +{
>> +    int i, pos = 0;
>> +    uint8_t mac[6] = { 0x00 };
>> +    uint8_t sn[5] = { 0x01, 0x23, 0x45, 0x67, 0x89 };
>> +
>> +    /* version */
>> +    eeprom[pos++] = 0x01;
>> +
>> +    /* count */
>> +    eeprom[pos++] = 0x02;
>> +
>> +    /* MAC address */
>> +    eeprom[pos++] = 0x01; /* MAC */
>> +    eeprom[pos++] = 0x06; /* length */
>> +    memcpy(&eeprom[pos], mac, sizeof(mac));
>> +    pos += sizeof(mac);
>> +
>> +    /* serial number */
>> +    eeprom[pos++] = 0x02; /* serial */
>> +    eeprom[pos++] = 0x05; /* length */
>> +    memcpy(&eeprom[pos], sn, sizeof(sn));
>> +    pos += sizeof(sn);
>> +
>> +    /* checksum */
>> +    eeprom[pos] = 0;
>> +    for (i = 0; i < pos; i++) {
>> +        eeprom[pos] += eeprom[i];
>> +    }
>> +}
>> +
>> +/*****************************************************************************/
>> +
>> +static int sam460ex_load_uboot(void)
>> +{
>> +    DriveInfo *dinfo;
>> +    BlockBackend *blk = NULL;
>> +    hwaddr base = FLASH_BASE | ((hwaddr)FLASH_BASE_H << 32);
>> +    long bios_size = FLASH_SIZE;
>> +    int fl_sectors;
>> +
>> +    dinfo = drive_get(IF_PFLASH, 0, 0);
>> +    if (dinfo) {
>> +        blk = blk_by_legacy_dinfo(dinfo);
>> +        bios_size = blk_getlength(blk);
>> +    }
>> +    fl_sectors = (bios_size + 65535) >> 16;
>> +
>> +    if (!pflash_cfi01_register(base, NULL, "sam460ex.flash", bios_size,
>> +                               blk, (64 * 1024), fl_sectors,
>> +                               1, 0x89, 0x18, 0x0000, 0x0, 1)) {
>> +        fprintf(stderr, "qemu: Error registering flash memory.\n");
>
> Use error_report() instead, please.
>
>> +        /* XXX: return an error instead? */
>> +        exit(1);
>> +    }
>> +
>> +    if (!blk) {
>> +        /*fprintf(stderr, "No flash image given with the 'pflash' parameter,"
>> +                " using default u-boot image\n");*/
>> +        base = UBOOT_LOAD_BASE | ((hwaddr)FLASH_BASE_H << 32);
>> +        rom_add_file_fixed(UBOOT_FILENAME, base, -1);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int sam460ex_load_device_tree(hwaddr addr,
>> +                                     uint32_t ramsize,
>> +                                     hwaddr initrd_base,
>> +                                     hwaddr initrd_size,
>> +                                     const char *kernel_cmdline)
>> +{
>> +    int ret = -1;
>> +    uint32_t mem_reg_property[] = { 0, 0, cpu_to_be32(ramsize) };
>> +    char *filename;
>> +    int fdt_size;
>> +    void *fdt;
>> +    uint32_t tb_freq = 400000000;
>> +    uint32_t clock_freq = 400000000;
>> +
>> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE);
>> +    if (!filename) {
>> +        goto out;
>> +    }
>> +    fdt = load_device_tree(filename, &fdt_size);
>> +    g_free(filename);
>> +    if (fdt == NULL) {
>> +        goto out;
>> +    }
>> +
>> +    /* Manipulate device tree in memory. */
>> +
>> +    ret = qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
>> +                               sizeof(mem_reg_property));
>> +    if (ret < 0) {
>> +        fprintf(stderr, "couldn't set /memory/reg\n");
>
> And in all these places.
>
>> +    }
>> +
>> +    /* default FDT doesn't have a /chosen node... */
>> +    qemu_fdt_add_subnode(fdt, "/chosen");
>> +
>> +    ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
>> +                                    initrd_base);
>> +    if (ret < 0) {
>> +        fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
>> +    }
>> +
>> +    ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
>> +                                    (initrd_base + initrd_size));
>> +    if (ret < 0) {
>> +        fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
>> +    }
>> +
>> +    ret = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
>> +                                      kernel_cmdline);
>> +    if (ret < 0) {
>> +        fprintf(stderr, "couldn't set /chosen/bootargs\n");
>> +    }
>> +
>> +    /* Copy data from the host device tree into the guest. Since the guest can
>> +     * directly access the timebase without host involvement, we must expose
>> +     * the correct frequencies. */
>> +    if (kvm_enabled()) {
>> +        tb_freq = kvmppc_get_tbfreq();
>> +        clock_freq = kvmppc_get_clockfreq();
>> +    }
>> +
>> +    qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "clock-frequency",
>> +                              clock_freq);
>> +    qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "timebase-frequency",
>> +                              tb_freq);
>> +
>> +    rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
>> +    g_free(fdt);
>> +    ret = fdt_size;
>> +
>> +out:
>> +
>> +    return ret;
>> +}
>> +
>> +/* Create reset TLB entries for BookE, mapping only the flash memory.  */
>> +static void mmubooke_create_initial_mapping_uboot(CPUPPCState *env)
>> +{
>> +    ppcemb_tlb_t *tlb = &env->tlb.tlbe[0];
>> +
>> +    /* on reset the flash is mapped by a shadow TLB,
>> +     * but since we don't implement them we need to use
>> +     * the same values U-Boot will use to avoid a fault.
>> +     */
>
> Usually the reset state of the MMU is handled in the cpu code rather
> than the board code.  Is there a specific reason you need it in the
> board code here?

I'm not sure, probably lack of a better place. The ppc440_bamboo board 
this is based on has it the same way in the board code. Maybe this could 
be cleaned up when someone wants to QOMify the SoC models sometimes.

>> +    tlb->attr = 0;
>> +    tlb->prot = PAGE_VALID | ((PAGE_READ | PAGE_WRITE | PAGE_EXEC) << 4);
>> +    tlb->size = 0x10000000; /* up to 0xffffffff  */
>> +    tlb->EPN = 0xf0000000 & TARGET_PAGE_MASK;
>> +    tlb->RPN = (0xf0000000 & TARGET_PAGE_MASK) | 0x4;
>> +    tlb->PID = 0;
>> +}
>> +
>> +/* Create reset TLB entries for BookE, spanning the 32bit addr space.  */
>> +static void mmubooke_create_initial_mapping(CPUPPCState *env,
>> +                                     target_ulong va,
>> +                                     hwaddr pa)
>> +{
>> +    ppcemb_tlb_t *tlb = &env->tlb.tlbe[0];
>> +
>> +    tlb->attr = 0;
>> +    tlb->prot = PAGE_VALID | ((PAGE_READ | PAGE_WRITE | PAGE_EXEC) << 4);
>> +    tlb->size = 1 << 31; /* up to 0x80000000  */
>> +    tlb->EPN = va & TARGET_PAGE_MASK;
>> +    tlb->RPN = pa & TARGET_PAGE_MASK;
>> +    tlb->PID = 0;
>> +}
>> +
>> +static void main_cpu_reset(void *opaque)
>> +{
>> +    PowerPCCPU *cpu = opaque;
>> +    CPUPPCState *env = &cpu->env;
>> +    struct boot_info *bi = env->load_info;
>> +
>> +    cpu_reset(CPU(cpu));
>> +
>> +    /* either we have a kernel to boot or we jump to U-Boot */
>> +    if (bi->entry != UBOOT_ENTRY) {
>> +        env->gpr[1] = (16 << 20) - 8;
>> +        env->gpr[3] = FDT_ADDR;
>> +
>> +        fprintf(stderr, "cpu reset: kernel entry %x\n", bi->entry);
>
> These should be tracepoints rather than bare fprintf().
>
>> +        env->nip = bi->entry;
>> +
>> +        /* Create a mapping for the kernel.  */
>> +        mmubooke_create_initial_mapping(env, 0, 0);
>> +        env->gpr[6] = tswap32(EPAPR_MAGIC);
>
> I'm pretty sure the tswap can't be right here.  env->gpr is in host
> native order and I'd expect the constant to be as well.

I know nothing about this, maybe Francois remembers why it's there. But 
booting linux with -kernel works so it's probably either correct or does 
not matter.

>> +        env->gpr[7] = (16 << 20) - 8; /*bi->ima_size;*/
>
> So, entering the kernel directly can be useful, particularly during
> early development.  However, having both firmware and non-firmware
> entry paths can lead to confusing bugs if there's a subtle difference
> between the initial (to the kernel) states between the two paths.  For
> that reason, the usual preferred way to implement -kernel is to still
> run the usual firmware, but use some way of telling it to boot
> immediately into the supplied kernel.
>
> I won't object to merging it this way - just a wanrning that this may
> bite you in the future if you're not careful.

Warning taken, at this point until firmware cannot reliably boot things 
having another way to test is useful to have. In the future when booting 
via firmware works well we can figure out what to do with this.

>> +
>> +    } else {
>> +        env->nip = UBOOT_ENTRY;
>> +        mmubooke_create_initial_mapping_uboot(env);
>> +        fprintf(stderr, "cpu reset: U-Boot entry\n");
>
> Tracepoint.
>
>> +    }
>> +}
>> +
>> +static void sam460ex_init(MachineState *machine)
>> +{
>> +    MemoryRegion *address_space_mem = get_system_memory();
>> +    MemoryRegion *isa = g_new(MemoryRegion, 1);
>> +    MemoryRegion *ram_memories = g_new(MemoryRegion, SDRAM_NR_BANKS);
>> +    hwaddr ram_bases[SDRAM_NR_BANKS];
>> +    hwaddr ram_sizes[SDRAM_NR_BANKS];
>> +    MemoryRegion *l2cache_ram = g_new(MemoryRegion, 1);
>> +    qemu_irq *irqs, *uic[4];
>> +    PCIBus *pci_bus;
>> +    PowerPCCPU *cpu;
>> +    CPUPPCState *env;
>> +    PPC4xxI2CState *i2c[2];
>> +    hwaddr entry = UBOOT_ENTRY;
>> +    hwaddr loadaddr = 0;
>> +    target_long initrd_size = 0;
>> +    DeviceState *dev;
>> +    SysBusDevice *sbdev;
>> +    int success;
>> +    int i;
>> +    struct boot_info *boot_info;
>> +    const size_t smbus_eeprom_size = 8 * 256;
>> +    uint8_t *smbus_eeprom_buf = g_malloc0(smbus_eeprom_size);
>> +
>> +    /* Setup CPU. */
>> +    if (machine->cpu_model == NULL) {
>> +        machine->cpu_model = "460EX";
>> +    }
>> +    cpu = cpu_ppc_init(machine->cpu_model);
>> +    if (cpu == NULL) {
>> +        fprintf(stderr, "Unable to initialize CPU!\n");
>> +        exit(1);
>> +    }
>> +    env = &cpu->env;
>> +
>> +    qemu_register_reset(main_cpu_reset, cpu);
>> +    boot_info = g_malloc0(sizeof(*boot_info));
>> +    env->load_info = boot_info;
>> +
>> +    ppc_booke_timers_init(cpu, 50000000, 0);
>> +    ppc_dcr_init(env, NULL, NULL);
>> +
>> +    /* PLB arbitrer */
>> +    ppc4xx_plb_init(env);
>> +
>> +    /* interrupt controllers */
>> +    irqs = g_malloc0(sizeof(*irqs) * PPCUIC_OUTPUT_NB);
>> +    irqs[PPCUIC_OUTPUT_INT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT];
>> +    irqs[PPCUIC_OUTPUT_CINT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT];
>> +    uic[0] = ppcuic_init(env, irqs, 0x0C0, 0, 1);
>> +    uic[1] = ppcuic_init(env, &uic[0][30], 0x0D0, 0, 1);
>> +    uic[2] = ppcuic_init(env, &uic[0][10], 0x0E0, 0, 1);
>> +    uic[3] = ppcuic_init(env, &uic[0][16], 0x0F0, 0, 1);
>> +
>> +    /* SDRAM controller */
>> +    memset(ram_bases, 0, sizeof(ram_bases));
>> +    memset(ram_sizes, 0, sizeof(ram_sizes));
>> +    /* put all RAM on first bank because board has one slot
>> +     * and firmware only checks that */
>> +    machine->ram_size = ppc4xx_sdram_adjust(machine->ram_size,
>> +                                   1/*SDRAM_NR_BANKS*/,
>> +                                   ram_memories,
>> +                                   ram_bases, ram_sizes,
>> +                                   ppc460ex_sdram_bank_sizes);
>> +#ifdef DEBUG_SDRAM
>> +    printf("RAMSIZE %dMB\n", (int)(machine->ram_size / M_BYTE));
>> +#endif
>> +
>> +    /* XXX does 460EX have ECC interrupts? */
>> +    ppc440_sdram_init(env, SDRAM_NR_BANKS, ram_memories,
>> +                      ram_bases, ram_sizes, 1);
>> +
>> +    /* generate SPD EEPROM data */
>> +    for (i = 0; i < SDRAM_NR_BANKS; i++) {
>> +#ifdef DEBUG_SDRAM
>> +        printf("bank %d: %" HWADDR_PRIx "\n", i, ram_sizes[i]);
>> +#endif
>
> Tracepoint.
>
>> +        generate_eeprom_spd(&smbus_eeprom_buf[i * 256], ram_sizes[i]);
>> +    }
>> +    generate_eeprom_serial(&smbus_eeprom_buf[4 * 256]);
>> +    generate_eeprom_serial(&smbus_eeprom_buf[6 * 256]);
>> +
>> +    /* IIC controllers */
>> +    dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600700, uic[0][2]);
>> +    i2c[0] = PPC4xx_I2C(dev);
>> +    object_property_set_bool(OBJECT(dev), true, "realized", NULL);
>> +    smbus_eeprom_init(i2c[0]->bus, 8, smbus_eeprom_buf, smbus_eeprom_size);
>> +    g_free(smbus_eeprom_buf);
>> +
>> +    dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600800, uic[0][3]);
>> +    i2c[1] = PPC4xx_I2C(dev);
>> +
>> +    /* External bus controller */
>> +    ppc405_ebc_init(env);
>> +
>> +    /* CPR */
>> +    ppc4xx_cpr_init(env);
>> +
>> +    /* PLB to AHB bridge */
>> +    ppc4xx_ahb_init(env);
>> +
>> +    /* System DCRs */
>> +    ppc4xx_sdr_init(env);
>> +
>> +    /* MAL */
>> +    ppc4xx_mal_init(env, 4, 16, &uic[2][3]);
>> +
>> +    /* 256K of L2 cache as memory */
>> +    ppc4xx_l2sram_init(env);
>
> Seems like a lot of this peripheral setup should be handled by a SoC
> helper function, since it's not really board specific (I'm guessing).
> I'm ok with that being a later clean up though.

I know, the ppc405 has an init function and I considered moving some of 
this to a similar function but I've left it here for now for simplicity 
until it's decided what's the best way to clean this up.

>> +    /* FIXME: remove this after fixing l2sram mapping in ppc440_uc.c? */
>> +    memory_region_init_ram(l2cache_ram, NULL, "ppc440.l2cache_ram", 256 << 10,
>> +                           &error_abort);
>> +    memory_region_add_subregion(address_space_mem, 0x400000000LL, l2cache_ram);
>> +
>> +    /* USB */
>> +    sysbus_create_simple(TYPE_PPC4xx_EHCI, 0x4bffd0400, uic[2][29]);
>> +    dev = qdev_create(NULL, "sysbus-ohci");
>> +    qdev_prop_set_string(dev, "masterbus", "usb-bus.0");
>> +    qdev_prop_set_uint32(dev, "num-ports", 6);
>> +    qdev_init_nofail(dev);
>> +    sbdev = SYS_BUS_DEVICE(dev);
>> +    sysbus_mmio_map(sbdev, 0, 0x4bffd0000);
>> +    sysbus_connect_irq(sbdev, 0, uic[2][30]);
>> +    usb_create_simple(usb_bus_find(-1), "usb-kbd");
>> +    usb_create_simple(usb_bus_find(-1), "usb-mouse");
>> +
>> +    /* PCI bus */
>> +    ppc460ex_pcie_init(env);
>> +    /*XXX: FIXME: is this correct? */
>> +    dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec00000,
>> +                                uic[1][0], uic[1][20], uic[1][21], uic[1][22],
>> +                                NULL);
>> +    pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
>> +    if (!pci_bus) {
>> +        fprintf(stderr, "couldn't create PCI controller!\n");
>> +        exit(1);
>> +    }
>> +    memory_region_init_alias(isa, NULL, "isa_mmio", get_system_io(),
>> +                             0, 0x10000);
>> +    memory_region_add_subregion(get_system_memory(), 0xc08000000, isa);
>
> Does it really make sense to just embed the ISA IO space here, rather
> than actually instanting a PCI<->ISA bridge?

I'm not sure if this is correct but I don't know how it's handled on real 
hardware. The board does not have ISA and I don't think it has a bridge 
but the IO space appears at this location according to the datasheet (In 
System Memory Address Map it's listed as PCI I/O 0xc08000000-0xc0800ffff) 
and clients expect PCI card's io registers to be accessible here. If 
anyone knows how it's done on real hardware and if there's a better way to 
model this please let me know.

>> +    /* PCI devices */
>> +    pci_create_simple(pci_bus, PCI_DEVFN(6, 0), "sm501");
>> +    /* SoC has a single SATA port but we don't emulate that yet
>> +     * However, firmware and usual clients have driver for SiI311x
>> +     * so add one for convenience by default */
>> +    pci_create_simple(pci_bus, -1, "sii3112");
>
> You should probably not create this when started with -nodefaults.

We don't emulate the on-board SATA port of the SoC and without this 
there's no other way to connect disks (maybe over USB, but firmware has a 
bug which prevents that too even on real hardware AFAIK, I've backported a 
fix which made booting from USB work but that broke keyboard) so while 
this is an external card it's pretty much unusable without this so it's 
added by default.

>> +    /* SoC has 4 UARTs
>> +     * but board has only one wired and two are present in fdt */
>> +    if (serial_hds[0] != NULL) {
>> +        serial_mm_init(address_space_mem, 0x4ef600300, 0, uic[1][1],
>> +                       PPC_SERIAL_MM_BAUDBASE, serial_hds[0],
>> +                       DEVICE_BIG_ENDIAN);
>> +    }
>> +    if (serial_hds[1] != NULL) {
>> +        serial_mm_init(address_space_mem, 0x4ef600400, 0, uic[0][1],
>> +                       PPC_SERIAL_MM_BAUDBASE, serial_hds[1],
>> +                       DEVICE_BIG_ENDIAN);
>> +    }
>> +
>> +    /* Load U-Boot image. */
>> +    if (!machine->kernel_filename) {
>> +        success = sam460ex_load_uboot();
>> +        if (success < 0) {
>> +            fprintf(stderr, "qemu: could not load firmware\n");
>> +            exit(1);
>> +        }
>> +    }
>> +
>> +    /* Load kernel. */
>> +    if (machine->kernel_filename) {
>> +        success = load_uimage(machine->kernel_filename, &entry, &loadaddr, NULL,
>> +            NULL, NULL);
>> +        fprintf(stderr, "load_uimage: %d\n", success);
>
> tracepoint
>
>> +        if (success < 0) {
>> +            uint64_t elf_entry, elf_lowaddr;
>> +
>> +            success = load_elf(machine->kernel_filename, NULL, NULL, &elf_entry,
>> +                               &elf_lowaddr, NULL, 1, PPC_ELF_MACHINE, 0, 0);
>> +            fprintf(stderr, "load_elf: %d\n", success);
>
> tracepoint
>
>> +            entry = elf_entry;
>> +            loadaddr = elf_lowaddr;
>> +        }
>> +        /* XXX try again as binary */
>> +        if (success < 0) {
>> +            fprintf(stderr, "qemu: could not load kernel '%s'\n",
>> +                    machine->kernel_filename);
>
> error_report().
>
>> +            exit(1);
>> +        }
>> +    }
>> +
>> +    /* Load initrd. */
>> +    if (machine->initrd_filename) {
>> +        initrd_size = load_image_targphys(machine->initrd_filename,
>> +                                          RAMDISK_ADDR,
>> +                                          machine->ram_size - RAMDISK_ADDR);
>> +        fprintf(stderr, "load_image: %d\n", initrd_size);
>> +        if (initrd_size < 0) {
>> +            fprintf(stderr, "qemu: could not load ram disk '%s' at %x\n",
>> +                    machine->initrd_filename, RAMDISK_ADDR);
>> +            exit(1);
>> +        }
>> +    }
>> +
>> +    /* If we're loading a kernel directly, we must load the device tree too. */
>> +    if (machine->kernel_filename) {
>> +        int dt_size;
>> +
>> +        dt_size = sam460ex_load_device_tree(FDT_ADDR, machine->ram_size,
>> +                                    RAMDISK_ADDR, initrd_size,
>> +                                    machine->kernel_cmdline);
>> +        if (dt_size < 0) {
>> +            fprintf(stderr, "couldn't load device tree\n");
>> +            exit(1);
>> +        }
>> +
>> +        boot_info->dt_base = FDT_ADDR;
>> +        boot_info->dt_size = dt_size;
>> +    }
>> +
>> +    boot_info->entry = entry;
>> +}
>> +
>> +static void sam460ex_machine_init(MachineClass *mc)
>> +{
>> +    mc->desc = "aCube Sam460ex";
>> +    mc->init = sam460ex_init;
>> +    mc->default_ram_size = 512 * M_BYTE;
>> +}
>> +
>> +DEFINE_MACHINE("sam460ex", sam460ex_machine_init)
>
>
François Revol Aug. 23, 2017, 11:43 a.m. UTC | #4
Le 23/08/2017 à 13:12, BALATON Zoltan a écrit :
>> What's the connection with mips_malta?
> 
> The board's firmware wants to see SPD EEPROMs of the connected memory
> while initialising the memory controller. This is why we need to
> implement SDRAM controller, I2C and SPD EEPROMs. MIPS malta board had
> already SPD EEPROM implementation so this is based on that. The comment
> just indicates where this code comes from.

Indeed, and I copy-pasted from elsewhere for this.

>>> +        fprintf(stderr, "qemu: Error registering flash memory.\n");
>>
>> Use error_report() instead, please.

I guess this didn't exist back when I started writing it...

>>> +/* Create reset TLB entries for BookE, mapping only the flash
>>> memory.  */
>>> +static void mmubooke_create_initial_mapping_uboot(CPUPPCState *env)
>>> +{
>>> +    ppcemb_tlb_t *tlb = &env->tlb.tlbe[0];
>>> +
>>> +    /* on reset the flash is mapped by a shadow TLB,
>>> +     * but since we don't implement them we need to use
>>> +     * the same values U-Boot will use to avoid a fault.
>>> +     */
>>
>> Usually the reset state of the MMU is handled in the cpu code rather
>> than the board code.  Is there a specific reason you need it in the
>> board code here?
> 
> I'm not sure, probably lack of a better place. The ppc440_bamboo board
> this is based on has it the same way in the board code. Maybe this could
> be cleaned up when someone wants to QOMify the SoC models sometimes.

Thing is, the code allows both booting with U-Boot and with a kernel
directly, and the MMU mapping differ in those cases.

Maybe the CPU reset should use the U-Boot setup and the kernel boot
would just overwrite it?

>>> +        env->nip = bi->entry;
>>> +
>>> +        /* Create a mapping for the kernel.  */
>>> +        mmubooke_create_initial_mapping(env, 0, 0);
>>> +        env->gpr[6] = tswap32(EPAPR_MAGIC);
>>
>> I'm pretty sure the tswap can't be right here.  env->gpr is in host
>> native order and I'd expect the constant to be as well.
> 
> I know nothing about this, maybe Francois remembers why it's there. But
> booting linux with -kernel works so it's probably either correct or does
> not matter.

Absolutely no idea. It seems to be there from the first commit in my own
history here.

I don't recall testing booting linux at all though.
Linux does check the magic, so it'd be weird if it booted:

https://github.com/torvalds/linux/blob/master/arch/powerpc/boot/epapr.c

But maybe it got added later than the version you tested?

At least my current Haiku port ignores the magic for now.

>>> +        env->gpr[7] = (16 << 20) - 8; /*bi->ima_size;*/
>>
>> So, entering the kernel directly can be useful, particularly during
>> early development.  However, having both firmware and non-firmware
>> entry paths can lead to confusing bugs if there's a subtle difference
>> between the initial (to the kernel) states between the two paths.  For
>> that reason, the usual preferred way to implement -kernel is to still
>> run the usual firmware, but use some way of telling it to boot
>> immediately into the supplied kernel.
>>
>> I won't object to merging it this way - just a wanrning that this may
>> bite you in the future if you're not careful.
> 
> Warning taken, at this point until firmware cannot reliably boot things
> having another way to test is useful to have. In the future when booting
> via firmware works well we can figure out what to do with this.

Possibly we could dig the U-Boot environment...

>>> +    memory_region_init_alias(isa, NULL, "isa_mmio", get_system_io(),
>>> +                             0, 0x10000);
>>> +    memory_region_add_subregion(get_system_memory(), 0xc08000000, isa);
>>
>> Does it really make sense to just embed the ISA IO space here, rather
>> than actually instanting a PCI<->ISA bridge?
> 
> I'm not sure if this is correct but I don't know how it's handled on
> real hardware. The board does not have ISA and I don't think it has a
> bridge but the IO space appears at this location according to the
> datasheet (In System Memory Address Map it's listed as PCI I/O
> 0xc08000000-0xc0800ffff) and clients expect PCI card's io registers to
> be accessible here. If anyone knows how it's done on real hardware and
> if there's a better way to model this please let me know.

Indeed it's the PCI I/O space, maybe it's just copy-paste error.

As for how to declare it, keep in mind this code is years old and I
fixed it several times when compilation broke, without really reading
the documentation (actually, do we have proper documentation for the
internal API?), and it kept changing over the years.

> 
>>> +    /* PCI devices */
>>> +    pci_create_simple(pci_bus, PCI_DEVFN(6, 0), "sm501");
>>> +    /* SoC has a single SATA port but we don't emulate that yet
>>> +     * However, firmware and usual clients have driver for SiI311x
>>> +     * so add one for convenience by default */
>>> +    pci_create_simple(pci_bus, -1, "sii3112");
>>
>> You should probably not create this when started with -nodefaults.
> 
> We don't emulate the on-board SATA port of the SoC and without this
> there's no other way to connect disks (maybe over USB, but firmware has
> a bug which prevents that too even on real hardware AFAIK, I've
> backported a fix which made booting from USB work but that broke
> keyboard) so while this is an external card it's pretty much unusable
> without this so it's added by default.

Maybe add a /*TODO*/ then?

François.
BALATON Zoltan Aug. 23, 2017, 12:47 p.m. UTC | #5
On Wed, 23 Aug 2017, François Revol wrote:
> Le 23/08/2017 à 13:12, BALATON Zoltan a écrit :
>>> What's the connection with mips_malta?
>>
>> The board's firmware wants to see SPD EEPROMs of the connected memory
>> while initialising the memory controller. This is why we need to
>> implement SDRAM controller, I2C and SPD EEPROMs. MIPS malta board had
>> already SPD EEPROM implementation so this is based on that. The comment
>> just indicates where this code comes from.
>
> Indeed, and I copy-pasted from elsewhere for this.
>
>>>> +        fprintf(stderr, "qemu: Error registering flash memory.\n");
>>>
>>> Use error_report() instead, please.
>
> I guess this didn't exist back when I started writing it...

No problem, I can take care of these.

>>>> +/* Create reset TLB entries for BookE, mapping only the flash
>>>> memory.  */
>>>> +static void mmubooke_create_initial_mapping_uboot(CPUPPCState *env)
>>>> +{
>>>> +    ppcemb_tlb_t *tlb = &env->tlb.tlbe[0];
>>>> +
>>>> +    /* on reset the flash is mapped by a shadow TLB,
>>>> +     * but since we don't implement them we need to use
>>>> +     * the same values U-Boot will use to avoid a fault.
>>>> +     */
>>>
>>> Usually the reset state of the MMU is handled in the cpu code rather
>>> than the board code.  Is there a specific reason you need it in the
>>> board code here?
>>
>> I'm not sure, probably lack of a better place. The ppc440_bamboo board
>> this is based on has it the same way in the board code. Maybe this could
>> be cleaned up when someone wants to QOMify the SoC models sometimes.
>
> Thing is, the code allows both booting with U-Boot and with a kernel
> directly, and the MMU mapping differ in those cases.
>
> Maybe the CPU reset should use the U-Boot setup and the kernel boot
> would just overwrite it?
>
>>>> +        env->nip = bi->entry;
>>>> +
>>>> +        /* Create a mapping for the kernel.  */
>>>> +        mmubooke_create_initial_mapping(env, 0, 0);
>>>> +        env->gpr[6] = tswap32(EPAPR_MAGIC);
>>>
>>> I'm pretty sure the tswap can't be right here.  env->gpr is in host
>>> native order and I'd expect the constant to be as well.
>>
>> I know nothing about this, maybe Francois remembers why it's there. But
>> booting linux with -kernel works so it's probably either correct or does
>> not matter.
>
> Absolutely no idea. It seems to be there from the first commit in my own
> history here.
>
> I don't recall testing booting linux at all though.
> Linux does check the magic, so it'd be weird if it booted:
>
> https://github.com/torvalds/linux/blob/master/arch/powerpc/boot/epapr.c

Is this code used on Sam460 at all? Is U-Boot ePAPR compliant firmware? 
Isn't that only needed on OpenFirmware?

> But maybe it got added later than the version you tested?

I've tried some of these: 
http://www.supertuxkart-amiga.de/amiga/sam.html#downloads
which also have kernel 4.5 so that's fairly recent. These kernels are 
"u-boot legacy uImage" so maybe they don't need ePAPR magic? Are there 
some docs on what the kernel expects on this board or it has to be dug out 
from U-Boot?

> At least my current Haiku port ignores the magic for now.
>
>>>> +        env->gpr[7] = (16 << 20) - 8; /*bi->ima_size;*/
>>>
>>> So, entering the kernel directly can be useful, particularly during
>>> early development.  However, having both firmware and non-firmware
>>> entry paths can lead to confusing bugs if there's a subtle difference
>>> between the initial (to the kernel) states between the two paths.  For
>>> that reason, the usual preferred way to implement -kernel is to still
>>> run the usual firmware, but use some way of telling it to boot
>>> immediately into the supplied kernel.
>>>
>>> I won't object to merging it this way - just a wanrning that this may
>>> bite you in the future if you're not careful.
>>
>> Warning taken, at this point until firmware cannot reliably boot things
>> having another way to test is useful to have. In the future when booting
>> via firmware works well we can figure out what to do with this.
>
> Possibly we could dig the U-Boot environment...
>
>>>> +    memory_region_init_alias(isa, NULL, "isa_mmio", get_system_io(),
>>>> +                             0, 0x10000);
>>>> +    memory_region_add_subregion(get_system_memory(), 0xc08000000, isa);
>>>
>>> Does it really make sense to just embed the ISA IO space here, rather
>>> than actually instanting a PCI<->ISA bridge?
>>
>> I'm not sure if this is correct but I don't know how it's handled on
>> real hardware. The board does not have ISA and I don't think it has a
>> bridge but the IO space appears at this location according to the
>> datasheet (In System Memory Address Map it's listed as PCI I/O
>> 0xc08000000-0xc0800ffff) and clients expect PCI card's io registers to
>> be accessible here. If anyone knows how it's done on real hardware and
>> if there's a better way to model this please let me know.
>
> Indeed it's the PCI I/O space, maybe it's just copy-paste error.
>
> As for how to declare it, keep in mind this code is years old and I
> fixed it several times when compilation broke, without really reading
> the documentation (actually, do we have proper documentation for the
> internal API?), and it kept changing over the years.

And I've also changed it while cleaning up and getting it to work with 
some clients. This particular mapping was at the wrong place in your first 
commits then got removed but I found it's needed but at a different 
address so I've added it again with the fixed address.

>>>> +    /* PCI devices */
>>>> +    pci_create_simple(pci_bus, PCI_DEVFN(6, 0), "sm501");
>>>> +    /* SoC has a single SATA port but we don't emulate that yet
>>>> +     * However, firmware and usual clients have driver for SiI311x
>>>> +     * so add one for convenience by default */
>>>> +    pci_create_simple(pci_bus, -1, "sii3112");
>>>
>>> You should probably not create this when started with -nodefaults.
>>
>> We don't emulate the on-board SATA port of the SoC and without this
>> there's no other way to connect disks (maybe over USB, but firmware has
>> a bug which prevents that too even on real hardware AFAIK, I've
>> backported a fix which made booting from USB work but that broke
>> keyboard) so while this is an external card it's pretty much unusable
>> without this so it's added by default.
>
> Maybe add a /*TODO*/ then?

I don't indent to implement that. Seems to be too complex and unnecessary 
when we have sii3112 (unless you want to test drivers for it on QEMU). I'd 
rather see an implementation of the network interfaces that seems to be 
more useful to spend time on than another SATA controller. So I chose to 
use the sii3112 by default.
luigi burdo Aug. 23, 2017, 1:33 p.m. UTC | #6
Hi Zoltan,

>So I chose to

>use the sii3112 by default.
right chooice because internal sata on sam460 had issue and wont boot os4,morphos... only the cdrom was booting if i remember  good from internal sata 0.
personally when i had a Sam460ex i was using a sii3114 or internal micro sd for boot... the sii was perfect compatible with sam uboot.

Thanks
Luigi
David Gibson Aug. 24, 2017, 2:44 a.m. UTC | #7
On Wed, Aug 23, 2017 at 01:12:06PM +0200, BALATON Zoltan wrote:
> On Wed, 23 Aug 2017, David Gibson wrote:
> > On Sun, Aug 20, 2017 at 07:23:05PM +0200, BALATON Zoltan wrote:
> > > Add emulation of aCube Sam460ex board based on AMCC 460EX embedded SoC.
> > > This is not a full implementation yet with a lot of components still
> > > missing but enough to start a Linux kernel and the U-Boot firmware.
> > > 
> > > Signed-off-by: François Revol <revol@free.fr>
> > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> > 
> > As usual, only fairly superficial review here.
> > 
> > > ---
> > >  default-configs/ppcemb-softmmu.mak |   3 +
> > >  hw/ppc/Makefile.objs               |   2 +
> > >  hw/ppc/sam460ex.c                  | 611 +++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 616 insertions(+)
> > >  create mode 100644 hw/ppc/sam460ex.c
> > > 
> > > diff --git a/default-configs/ppcemb-softmmu.mak b/default-configs/ppcemb-softmmu.mak
> > > index 635923a..90b42f0 100644
> > > --- a/default-configs/ppcemb-softmmu.mak
> > > +++ b/default-configs/ppcemb-softmmu.mak
> [...]
> > > +/*****************************************************************************/
> > > +/* SPD eeprom content from mips_malta.c */
> > 
> > What's the connection with mips_malta?
> 
> The board's firmware wants to see SPD EEPROMs of the connected memory while
> initialising the memory controller. This is why we need to implement SDRAM
> controller, I2C and SPD EEPROMs. MIPS malta board had already SPD EEPROM
> implementation so this is based on that. The comment just indicates where
> this code comes from.

Ok.

[snip]
> > > +        env->nip = bi->entry;
> > > +
> > > +        /* Create a mapping for the kernel.  */
> > > +        mmubooke_create_initial_mapping(env, 0, 0);
> > > +        env->gpr[6] = tswap32(EPAPR_MAGIC);
> > 
> > I'm pretty sure the tswap can't be right here.  env->gpr is in host
> > native order and I'd expect the constant to be as well.
> 
> I know nothing about this, maybe Francois remembers why it's there. But
> booting linux with -kernel works so it's probably either correct or does not
> matter.

Have you attempted it on both BE and LE hosts though?

> 
> > > +        env->gpr[7] = (16 << 20) - 8; /*bi->ima_size;*/
> > 
> > So, entering the kernel directly can be useful, particularly during
> > early development.  However, having both firmware and non-firmware
> > entry paths can lead to confusing bugs if there's a subtle difference
> > between the initial (to the kernel) states between the two paths.  For
> > that reason, the usual preferred way to implement -kernel is to still
> > run the usual firmware, but use some way of telling it to boot
> > immediately into the supplied kernel.
> > 
> > I won't object to merging it this way - just a wanrning that this may
> > bite you in the future if you're not careful.
> 
> Warning taken, at this point until firmware cannot reliably boot things
> having another way to test is useful to have. In the future when booting via
> firmware works well we can figure out what to do with this.

Fair enough.

[snip]
> > > +        generate_eeprom_spd(&smbus_eeprom_buf[i * 256], ram_sizes[i]);
> > > +    }
> > > +    generate_eeprom_serial(&smbus_eeprom_buf[4 * 256]);
> > > +    generate_eeprom_serial(&smbus_eeprom_buf[6 * 256]);
> > > +
> > > +    /* IIC controllers */
> > > +    dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600700, uic[0][2]);
> > > +    i2c[0] = PPC4xx_I2C(dev);
> > > +    object_property_set_bool(OBJECT(dev), true, "realized", NULL);
> > > +    smbus_eeprom_init(i2c[0]->bus, 8, smbus_eeprom_buf, smbus_eeprom_size);
> > > +    g_free(smbus_eeprom_buf);
> > > +
> > > +    dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600800, uic[0][3]);
> > > +    i2c[1] = PPC4xx_I2C(dev);
> > > +
> > > +    /* External bus controller */
> > > +    ppc405_ebc_init(env);
> > > +
> > > +    /* CPR */
> > > +    ppc4xx_cpr_init(env);
> > > +
> > > +    /* PLB to AHB bridge */
> > > +    ppc4xx_ahb_init(env);
> > > +
> > > +    /* System DCRs */
> > > +    ppc4xx_sdr_init(env);
> > > +
> > > +    /* MAL */
> > > +    ppc4xx_mal_init(env, 4, 16, &uic[2][3]);
> > > +
> > > +    /* 256K of L2 cache as memory */
> > > +    ppc4xx_l2sram_init(env);
> > 
> > Seems like a lot of this peripheral setup should be handled by a SoC
> > helper function, since it's not really board specific (I'm guessing).
> > I'm ok with that being a later clean up though.
> 
> I know, the ppc405 has an init function and I considered moving some of this
> to a similar function but I've left it here for now for simplicity until
> it's decided what's the best way to clean this up.

Ok, seems reasonable.

> > > +    /* FIXME: remove this after fixing l2sram mapping in ppc440_uc.c? */
> > > +    memory_region_init_ram(l2cache_ram, NULL, "ppc440.l2cache_ram", 256 << 10,
> > > +                           &error_abort);
> > > +    memory_region_add_subregion(address_space_mem, 0x400000000LL, l2cache_ram);
> > > +
> > > +    /* USB */
> > > +    sysbus_create_simple(TYPE_PPC4xx_EHCI, 0x4bffd0400, uic[2][29]);
> > > +    dev = qdev_create(NULL, "sysbus-ohci");
> > > +    qdev_prop_set_string(dev, "masterbus", "usb-bus.0");
> > > +    qdev_prop_set_uint32(dev, "num-ports", 6);
> > > +    qdev_init_nofail(dev);
> > > +    sbdev = SYS_BUS_DEVICE(dev);
> > > +    sysbus_mmio_map(sbdev, 0, 0x4bffd0000);
> > > +    sysbus_connect_irq(sbdev, 0, uic[2][30]);
> > > +    usb_create_simple(usb_bus_find(-1), "usb-kbd");
> > > +    usb_create_simple(usb_bus_find(-1), "usb-mouse");
> > > +
> > > +    /* PCI bus */
> > > +    ppc460ex_pcie_init(env);
> > > +    /*XXX: FIXME: is this correct? */
> > > +    dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec00000,
> > > +                                uic[1][0], uic[1][20], uic[1][21], uic[1][22],
> > > +                                NULL);
> > > +    pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
> > > +    if (!pci_bus) {
> > > +        fprintf(stderr, "couldn't create PCI controller!\n");
> > > +        exit(1);
> > > +    }
> > > +    memory_region_init_alias(isa, NULL, "isa_mmio", get_system_io(),
> > > +                             0, 0x10000);
> > > +    memory_region_add_subregion(get_system_memory(), 0xc08000000, isa);
> > 
> > Does it really make sense to just embed the ISA IO space here, rather
> > than actually instanting a PCI<->ISA bridge?
> 
> I'm not sure if this is correct but I don't know how it's handled on real
> hardware. The board does not have ISA and I don't think it has a bridge but
> the IO space appears at this location according to the datasheet (In System
> Memory Address Map it's listed as PCI I/O 0xc08000000-0xc0800ffff) and
> clients expect PCI card's io registers to be accessible here. If anyone
> knows how it's done on real hardware and if there's a better way to model
> this please let me know.

Ah, ok.  I think the confusion here is that you can have PCI I/O space
without ISA or a system IO space.  In fact that's pretty standard on
things without a CPU level IO space (which means just about everything
except x86).

But in that case I'd expect the PCI host bridge to map its IO memory
regions directly into address_space_memory rather than involving the
global address_space_io (what get_system_io()) returns.  The only
reason I can see that you'd need to involve get_system_io() is if you
have devices registering themselves directly into the global IO space,
which should only happen for legacy ISA devices, which it sounds like
you don't have.

Possibly this is an error in the PCI bridge implementation that your
code here is essentially a workaround for, though.

> 
> > > +    /* PCI devices */
> > > +    pci_create_simple(pci_bus, PCI_DEVFN(6, 0), "sm501");
> > > +    /* SoC has a single SATA port but we don't emulate that yet
> > > +     * However, firmware and usual clients have driver for SiI311x
> > > +     * so add one for convenience by default */
> > > +    pci_create_simple(pci_bus, -1, "sii3112");
> > 
> > You should probably not create this when started with -nodefaults.
> 
> We don't emulate the on-board SATA port of the SoC and without this there's
> no other way to connect disks (maybe over USB, but firmware has a bug which
> prevents that too even on real hardware AFAIK, I've backported a fix which
> made booting from USB work but that broke keyboard) so while this is an
> external card it's pretty much unusable without this so it's added by
> default.

Yes, but you're not just adding it by default, you're adding it
*always*.  -nodefaults should turn that off (and the user will need to
manually instantiate it - or another disk controller, I guess).
David Gibson Aug. 24, 2017, 2:51 a.m. UTC | #8
On Wed, Aug 23, 2017 at 01:43:56PM +0200, François Revol wrote:
> Le 23/08/2017 à 13:12, BALATON Zoltan a écrit :
> >> What's the connection with mips_malta?
> > 
> > The board's firmware wants to see SPD EEPROMs of the connected memory
> > while initialising the memory controller. This is why we need to
> > implement SDRAM controller, I2C and SPD EEPROMs. MIPS malta board had
> > already SPD EEPROM implementation so this is based on that. The comment
> > just indicates where this code comes from.
> 
> Indeed, and I copy-pasted from elsewhere for this.
> 
> >>> +        fprintf(stderr, "qemu: Error registering flash memory.\n");
> >>
> >> Use error_report() instead, please.
> 
> I guess this didn't exist back when I started writing it...

Probably not.

> >>> +/* Create reset TLB entries for BookE, mapping only the flash
> >>> memory.  */
> >>> +static void mmubooke_create_initial_mapping_uboot(CPUPPCState *env)
> >>> +{
> >>> +    ppcemb_tlb_t *tlb = &env->tlb.tlbe[0];
> >>> +
> >>> +    /* on reset the flash is mapped by a shadow TLB,
> >>> +     * but since we don't implement them we need to use
> >>> +     * the same values U-Boot will use to avoid a fault.
> >>> +     */
> >>
> >> Usually the reset state of the MMU is handled in the cpu code rather
> >> than the board code.  Is there a specific reason you need it in the
> >> board code here?
> > 
> > I'm not sure, probably lack of a better place. The ppc440_bamboo board
> > this is based on has it the same way in the board code. Maybe this could
> > be cleaned up when someone wants to QOMify the SoC models sometimes.
> 
> Thing is, the code allows both booting with U-Boot and with a kernel
> directly, and the MMU mapping differ in those cases.
> 
> Maybe the CPU reset should use the U-Boot setup and the kernel boot
> would just overwrite it?

Yes.  Basically the CPU reset should do what real hardware does - and
I'd expect u-boot to be written to work with that.  The kernel, on the
other hand will expect whatever mappings come out of u-boot (or
another bootloader).  Long term, I think you want to always use the
hardware setup and actually run the guest firmware to set up the
mappings the kernel expects.  For early development where it's useful
to be able to boot into a kernel without firmware, faking the right
mappings in the board code is perfectly reasonable.
David Gibson Aug. 24, 2017, 2:54 a.m. UTC | #9
On Wed, Aug 23, 2017 at 02:47:42PM +0200, BALATON Zoltan wrote:
> On Wed, 23 Aug 2017, François Revol wrote:
> > Le 23/08/2017 à 13:12, BALATON Zoltan a écrit :
> > > > What's the connection with mips_malta?
> > > 
> > > The board's firmware wants to see SPD EEPROMs of the connected memory
> > > while initialising the memory controller. This is why we need to
> > > implement SDRAM controller, I2C and SPD EEPROMs. MIPS malta board had
> > > already SPD EEPROM implementation so this is based on that. The comment
> > > just indicates where this code comes from.
> > 
> > Indeed, and I copy-pasted from elsewhere for this.
> > 
> > > > > +        fprintf(stderr, "qemu: Error registering flash memory.\n");
> > > > 
> > > > Use error_report() instead, please.
> > 
> > I guess this didn't exist back when I started writing it...
> 
> No problem, I can take care of these.
> 
> > > > > +/* Create reset TLB entries for BookE, mapping only the flash
> > > > > memory.  */
> > > > > +static void mmubooke_create_initial_mapping_uboot(CPUPPCState *env)
> > > > > +{
> > > > > +    ppcemb_tlb_t *tlb = &env->tlb.tlbe[0];
> > > > > +
> > > > > +    /* on reset the flash is mapped by a shadow TLB,
> > > > > +     * but since we don't implement them we need to use
> > > > > +     * the same values U-Boot will use to avoid a fault.
> > > > > +     */
> > > > 
> > > > Usually the reset state of the MMU is handled in the cpu code rather
> > > > than the board code.  Is there a specific reason you need it in the
> > > > board code here?
> > > 
> > > I'm not sure, probably lack of a better place. The ppc440_bamboo board
> > > this is based on has it the same way in the board code. Maybe this could
> > > be cleaned up when someone wants to QOMify the SoC models sometimes.
> > 
> > Thing is, the code allows both booting with U-Boot and with a kernel
> > directly, and the MMU mapping differ in those cases.
> > 
> > Maybe the CPU reset should use the U-Boot setup and the kernel boot
> > would just overwrite it?
> > 
> > > > > +        env->nip = bi->entry;
> > > > > +
> > > > > +        /* Create a mapping for the kernel.  */
> > > > > +        mmubooke_create_initial_mapping(env, 0, 0);
> > > > > +        env->gpr[6] = tswap32(EPAPR_MAGIC);
> > > > 
> > > > I'm pretty sure the tswap can't be right here.  env->gpr is in host
> > > > native order and I'd expect the constant to be as well.
> > > 
> > > I know nothing about this, maybe Francois remembers why it's there. But
> > > booting linux with -kernel works so it's probably either correct or does
> > > not matter.
> > 
> > Absolutely no idea. It seems to be there from the first commit in my own
> > history here.
> > 
> > I don't recall testing booting linux at all though.
> > Linux does check the magic, so it'd be weird if it booted:
> > 
> > https://github.com/torvalds/linux/blob/master/arch/powerpc/boot/epapr.c
> 
> Is this code used on Sam460 at all? Is U-Boot ePAPR compliant
> firmware?

It can be, depends on the build, I think.

> Isn't that only needed on OpenFirmware?

No, not at all.  True OF will generally *not* be ePAPR compliant -
ePAPR was explicitly built as a much simpler interface that doesn't
require an OF implementation.  It uses some concepts from OF,
specifically the contents of the device tree, but other than that it's
not related.

> 
> > But maybe it got added later than the version you tested?
> 
> I've tried some of these:
> http://www.supertuxkart-amiga.de/amiga/sam.html#downloads
> which also have kernel 4.5 so that's fairly recent. These kernels are
> "u-boot legacy uImage" so maybe they don't need ePAPR magic? Are there some
> docs on what the kernel expects on this board or it has to be dug out from
> U-Boot?

If it's a legacy uImage I suspect it's not using ePAPR.
BALATON Zoltan Aug. 24, 2017, 9:37 p.m. UTC | #10
On Thu, 24 Aug 2017, David Gibson wrote:
> On Wed, Aug 23, 2017 at 01:12:06PM +0200, BALATON Zoltan wrote:
>> On Wed, 23 Aug 2017, David Gibson wrote:
>>> On Sun, Aug 20, 2017 at 07:23:05PM +0200, BALATON Zoltan wrote:
>>>> Add emulation of aCube Sam460ex board based on AMCC 460EX embedded SoC.
>>>> This is not a full implementation yet with a lot of components still
>>>> missing but enough to start a Linux kernel and the U-Boot firmware.
>>>>
>>>> Signed-off-by: François Revol <revol@free.fr>
>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>
>>> As usual, only fairly superficial review here.
>>>
>>>> ---
>>>>  default-configs/ppcemb-softmmu.mak |   3 +
>>>>  hw/ppc/Makefile.objs               |   2 +
>>>>  hw/ppc/sam460ex.c                  | 611 +++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 616 insertions(+)
>>>>  create mode 100644 hw/ppc/sam460ex.c
>>>>
>>>> diff --git a/default-configs/ppcemb-softmmu.mak b/default-configs/ppcemb-softmmu.mak
>>>> index 635923a..90b42f0 100644
>>>> --- a/default-configs/ppcemb-softmmu.mak
>>>> +++ b/default-configs/ppcemb-softmmu.mak
>> [...]
>>>> +/*****************************************************************************/
>>>> +/* SPD eeprom content from mips_malta.c */
>>>
>>> What's the connection with mips_malta?
>>
>> The board's firmware wants to see SPD EEPROMs of the connected memory while
>> initialising the memory controller. This is why we need to implement SDRAM
>> controller, I2C and SPD EEPROMs. MIPS malta board had already SPD EEPROM
>> implementation so this is based on that. The comment just indicates where
>> this code comes from.
>
> Ok.
>
> [snip]
>>>> +        env->nip = bi->entry;
>>>> +
>>>> +        /* Create a mapping for the kernel.  */
>>>> +        mmubooke_create_initial_mapping(env, 0, 0);
>>>> +        env->gpr[6] = tswap32(EPAPR_MAGIC);
>>>
>>> I'm pretty sure the tswap can't be right here.  env->gpr is in host
>>> native order and I'd expect the constant to be as well.
>>
>> I know nothing about this, maybe Francois remembers why it's there. But
>> booting linux with -kernel works so it's probably either correct or does not
>> matter.
>
> Have you attempted it on both BE and LE hosts though?

No, I don't have a BE host, only tried on LE. I'm guessing this may come 
from hw/ppc/virtex_ml507.c where EPAPR_MAGIC is also swapped. The only 
other place this magic number appears is in e500 where it's not swapped 
though so I don't really know what should be correct here. In u-boot 
sources (arch/powerpc/lib/bootm.c) it seems to use this magic if 
CONFIG_OF_LIBFDT is defined which seems to be set for this board so that 
means we likely need this (but maybe not for the legacy uImages I've 
tried). Since CPU is big endian and constant is defined on the host this 
probably should be cpu_to_be32(EPAPR_MAGIC). Does that sound better?

[...]
>>>> +    /* FIXME: remove this after fixing l2sram mapping in ppc440_uc.c? */
>>>> +    memory_region_init_ram(l2cache_ram, NULL, "ppc440.l2cache_ram", 256 << 10,
>>>> +                           &error_abort);
>>>> +    memory_region_add_subregion(address_space_mem, 0x400000000LL, l2cache_ram);
>>>> +
>>>> +    /* USB */
>>>> +    sysbus_create_simple(TYPE_PPC4xx_EHCI, 0x4bffd0400, uic[2][29]);
>>>> +    dev = qdev_create(NULL, "sysbus-ohci");
>>>> +    qdev_prop_set_string(dev, "masterbus", "usb-bus.0");
>>>> +    qdev_prop_set_uint32(dev, "num-ports", 6);
>>>> +    qdev_init_nofail(dev);
>>>> +    sbdev = SYS_BUS_DEVICE(dev);
>>>> +    sysbus_mmio_map(sbdev, 0, 0x4bffd0000);
>>>> +    sysbus_connect_irq(sbdev, 0, uic[2][30]);
>>>> +    usb_create_simple(usb_bus_find(-1), "usb-kbd");
>>>> +    usb_create_simple(usb_bus_find(-1), "usb-mouse");
>>>> +
>>>> +    /* PCI bus */
>>>> +    ppc460ex_pcie_init(env);
>>>> +    /*XXX: FIXME: is this correct? */
>>>> +    dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec00000,
>>>> +                                uic[1][0], uic[1][20], uic[1][21], uic[1][22],
>>>> +                                NULL);
>>>> +    pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
>>>> +    if (!pci_bus) {
>>>> +        fprintf(stderr, "couldn't create PCI controller!\n");
>>>> +        exit(1);
>>>> +    }
>>>> +    memory_region_init_alias(isa, NULL, "isa_mmio", get_system_io(),
>>>> +                             0, 0x10000);
>>>> +    memory_region_add_subregion(get_system_memory(), 0xc08000000, isa);
>>>
>>> Does it really make sense to just embed the ISA IO space here, rather
>>> than actually instanting a PCI<->ISA bridge?
>>
>> I'm not sure if this is correct but I don't know how it's handled on real
>> hardware. The board does not have ISA and I don't think it has a bridge but
>> the IO space appears at this location according to the datasheet (In System
>> Memory Address Map it's listed as PCI I/O 0xc08000000-0xc0800ffff) and
>> clients expect PCI card's io registers to be accessible here. If anyone
>> knows how it's done on real hardware and if there's a better way to model
>> this please let me know.
>
> Ah, ok.  I think the confusion here is that you can have PCI I/O space
> without ISA or a system IO space.  In fact that's pretty standard on
> things without a CPU level IO space (which means just about everything
> except x86).
>
> But in that case I'd expect the PCI host bridge to map its IO memory
> regions directly into address_space_memory rather than involving the
> global address_space_io (what get_system_io()) returns.  The only
> reason I can see that you'd need to involve get_system_io() is if you
> have devices registering themselves directly into the global IO space,
> which should only happen for legacy ISA devices, which it sounds like
> you don't have.
>
> Possibly this is an error in the PCI bridge implementation that your
> code here is essentially a workaround for, though.

So in my understanding, there's a system_io space created automatically 
which appears in the memory tree anyway but would otherwise be unused and 
this was just reused here for the pci io space so it does not need another 
memory region for this. If it's not acceptable this way (although 
ppc440_bamboo.c and ppc4xx_pci.c also does it the exact same way) an 
alternative may be to change it to add another mem region for io to 
ppc440_pcix.c (although it already has iomem for pci.reg so this might be 
more confusing than using system_io here) but I think pcix device can't 
map this to address_space_memory itself because this device could appear 
in different SoCs where the memory areas might be at different addresses 
so this new region should then be registered as a sysbus mmio space and be 
mapped from the board code with sysbus_mmio_map()? I find that much more 
confusing than how it's done now which is also more consistent with 
existing code modelling similar devices.

>>>> +    /* PCI devices */
>>>> +    pci_create_simple(pci_bus, PCI_DEVFN(6, 0), "sm501");
>>>> +    /* SoC has a single SATA port but we don't emulate that yet
>>>> +     * However, firmware and usual clients have driver for SiI311x
>>>> +     * so add one for convenience by default */
>>>> +    pci_create_simple(pci_bus, -1, "sii3112");
>>>
>>> You should probably not create this when started with -nodefaults.
>>
>> We don't emulate the on-board SATA port of the SoC and without this there's
>> no other way to connect disks (maybe over USB, but firmware has a bug which
>> prevents that too even on real hardware AFAIK, I've backported a fix which
>> made booting from USB work but that broke keyboard) so while this is an
>> external card it's pretty much unusable without this so it's added by
>> default.
>
> Yes, but you're not just adding it by default, you're adding it
> *always*.  -nodefaults should turn that off (and the user will need to
> manually instantiate it - or another disk controller, I guess).

OK I got it now. I still don't see when -nodefaults could be useful to 
cripple the board and make it easier to create non-working configurations 
but I can easily add this conditional around this line and hope users stay 
away from this switch and won't complain when it does not boot when they 
use it. (Well, it does not really boot most of the time without this 
switch either so I don't think it matters much at the moment :-) )
BALATON Zoltan Aug. 24, 2017, 9:43 p.m. UTC | #11
On Thu, 24 Aug 2017, David Gibson wrote:
> On Wed, Aug 23, 2017 at 01:43:56PM +0200, François Revol wrote:
>> Le 23/08/2017 à 13:12, BALATON Zoltan a écrit :
>>>> What's the connection with mips_malta?
>>>
>>> The board's firmware wants to see SPD EEPROMs of the connected memory
>>> while initialising the memory controller. This is why we need to
>>> implement SDRAM controller, I2C and SPD EEPROMs. MIPS malta board had
>>> already SPD EEPROM implementation so this is based on that. The comment
>>> just indicates where this code comes from.
>>
>> Indeed, and I copy-pasted from elsewhere for this.
>>
>>>>> +        fprintf(stderr, "qemu: Error registering flash memory.\n");
>>>>
>>>> Use error_report() instead, please.
>>
>> I guess this didn't exist back when I started writing it...
>
> Probably not.
>
>>>>> +/* Create reset TLB entries for BookE, mapping only the flash
>>>>> memory.  */
>>>>> +static void mmubooke_create_initial_mapping_uboot(CPUPPCState *env)
>>>>> +{
>>>>> +    ppcemb_tlb_t *tlb = &env->tlb.tlbe[0];
>>>>> +
>>>>> +    /* on reset the flash is mapped by a shadow TLB,
>>>>> +     * but since we don't implement them we need to use
>>>>> +     * the same values U-Boot will use to avoid a fault.
>>>>> +     */
>>>>
>>>> Usually the reset state of the MMU is handled in the cpu code rather
>>>> than the board code.  Is there a specific reason you need it in the
>>>> board code here?
>>>
>>> I'm not sure, probably lack of a better place. The ppc440_bamboo board
>>> this is based on has it the same way in the board code. Maybe this could
>>> be cleaned up when someone wants to QOMify the SoC models sometimes.
>>
>> Thing is, the code allows both booting with U-Boot and with a kernel
>> directly, and the MMU mapping differ in those cases.
>>
>> Maybe the CPU reset should use the U-Boot setup and the kernel boot
>> would just overwrite it?
>
> Yes.  Basically the CPU reset should do what real hardware does - and
> I'd expect u-boot to be written to work with that.  The kernel, on the
> other hand will expect whatever mappings come out of u-boot (or
> another bootloader).  Long term, I think you want to always use the
> hardware setup and actually run the guest firmware to set up the
> mappings the kernel expects.

Maybe we should emulate the nvram for that and then we could patch it to 
have it boot the kernel with the parameters specified in the command line? 
This would also be needed to avoid the warning from u-boot about bad 
checksum and always going with the built in defaults and would also allow 
users to store settings like on real hardware but it's not something that 
looks high priority to me at the moment.

>  For early development where it's useful
> to be able to boot into a kernel without firmware, faking the right
> mappings in the board code is perfectly reasonable.

So for now I'd just go with this, because there's much more to clean up 
and improve before this becomes and issue I think.
David Gibson Aug. 24, 2017, 11:55 p.m. UTC | #12
On Thu, Aug 24, 2017 at 11:43:18PM +0200, BALATON Zoltan wrote:
> On Thu, 24 Aug 2017, David Gibson wrote:
> > On Wed, Aug 23, 2017 at 01:43:56PM +0200, François Revol wrote:
> > > Le 23/08/2017 à 13:12, BALATON Zoltan a écrit :
> > > > > What's the connection with mips_malta?
> > > > 
> > > > The board's firmware wants to see SPD EEPROMs of the connected memory
> > > > while initialising the memory controller. This is why we need to
> > > > implement SDRAM controller, I2C and SPD EEPROMs. MIPS malta board had
> > > > already SPD EEPROM implementation so this is based on that. The comment
> > > > just indicates where this code comes from.
> > > 
> > > Indeed, and I copy-pasted from elsewhere for this.
> > > 
> > > > > > +        fprintf(stderr, "qemu: Error registering flash memory.\n");
> > > > > 
> > > > > Use error_report() instead, please.
> > > 
> > > I guess this didn't exist back when I started writing it...
> > 
> > Probably not.
> > 
> > > > > > +/* Create reset TLB entries for BookE, mapping only the flash
> > > > > > memory.  */
> > > > > > +static void mmubooke_create_initial_mapping_uboot(CPUPPCState *env)
> > > > > > +{
> > > > > > +    ppcemb_tlb_t *tlb = &env->tlb.tlbe[0];
> > > > > > +
> > > > > > +    /* on reset the flash is mapped by a shadow TLB,
> > > > > > +     * but since we don't implement them we need to use
> > > > > > +     * the same values U-Boot will use to avoid a fault.
> > > > > > +     */
> > > > > 
> > > > > Usually the reset state of the MMU is handled in the cpu code rather
> > > > > than the board code.  Is there a specific reason you need it in the
> > > > > board code here?
> > > > 
> > > > I'm not sure, probably lack of a better place. The ppc440_bamboo board
> > > > this is based on has it the same way in the board code. Maybe this could
> > > > be cleaned up when someone wants to QOMify the SoC models sometimes.
> > > 
> > > Thing is, the code allows both booting with U-Boot and with a kernel
> > > directly, and the MMU mapping differ in those cases.
> > > 
> > > Maybe the CPU reset should use the U-Boot setup and the kernel boot
> > > would just overwrite it?
> > 
> > Yes.  Basically the CPU reset should do what real hardware does - and
> > I'd expect u-boot to be written to work with that.  The kernel, on the
> > other hand will expect whatever mappings come out of u-boot (or
> > another bootloader).  Long term, I think you want to always use the
> > hardware setup and actually run the guest firmware to set up the
> > mappings the kernel expects.
> 
> Maybe we should emulate the nvram for that and then we could patch it to
> have it boot the kernel with the parameters specified in the command line?
> This would also be needed to avoid the warning from u-boot about bad
> checksum and always going with the built in defaults and would also allow
> users to store settings like on real hardware but it's not something that
> looks high priority to me at the moment.

That all sounds reasonable.

> >  For early development where it's useful
> > to be able to boot into a kernel without firmware, faking the right
> > mappings in the board code is perfectly reasonable.
> 
> So for now I'd just go with this, because there's much more to clean up and
> improve before this becomes and issue I think.

Yup, makes sense.
David Gibson Aug. 25, 2017, 12:15 a.m. UTC | #13
On Thu, Aug 24, 2017 at 11:37:09PM +0200, BALATON Zoltan wrote:
> On Thu, 24 Aug 2017, David Gibson wrote:
> > On Wed, Aug 23, 2017 at 01:12:06PM +0200, BALATON Zoltan wrote:
> > > On Wed, 23 Aug 2017, David Gibson wrote:
> > > > On Sun, Aug 20, 2017 at 07:23:05PM +0200, BALATON Zoltan wrote:
> > > > > Add emulation of aCube Sam460ex board based on AMCC 460EX embedded SoC.
> > > > > This is not a full implementation yet with a lot of components still
> > > > > missing but enough to start a Linux kernel and the U-Boot firmware.
> > > > > 
> > > > > Signed-off-by: François Revol <revol@free.fr>
> > > > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> > > > 
> > > > As usual, only fairly superficial review here.
> > > > 
> > > > > ---
> > > > >  default-configs/ppcemb-softmmu.mak |   3 +
> > > > >  hw/ppc/Makefile.objs               |   2 +
> > > > >  hw/ppc/sam460ex.c                  | 611 +++++++++++++++++++++++++++++++++++++
> > > > >  3 files changed, 616 insertions(+)
> > > > >  create mode 100644 hw/ppc/sam460ex.c
> > > > > 
> > > > > diff --git a/default-configs/ppcemb-softmmu.mak b/default-configs/ppcemb-softmmu.mak
> > > > > index 635923a..90b42f0 100644
> > > > > --- a/default-configs/ppcemb-softmmu.mak
> > > > > +++ b/default-configs/ppcemb-softmmu.mak
> > > [...]
> > > > > +/*****************************************************************************/
> > > > > +/* SPD eeprom content from mips_malta.c */
> > > > 
> > > > What's the connection with mips_malta?
> > > 
> > > The board's firmware wants to see SPD EEPROMs of the connected memory while
> > > initialising the memory controller. This is why we need to implement SDRAM
> > > controller, I2C and SPD EEPROMs. MIPS malta board had already SPD EEPROM
> > > implementation so this is based on that. The comment just indicates where
> > > this code comes from.
> > 
> > Ok.
> > 
> > [snip]
> > > > > +        env->nip = bi->entry;
> > > > > +
> > > > > +        /* Create a mapping for the kernel.  */
> > > > > +        mmubooke_create_initial_mapping(env, 0, 0);
> > > > > +        env->gpr[6] = tswap32(EPAPR_MAGIC);
> > > > 
> > > > I'm pretty sure the tswap can't be right here.  env->gpr is in host
> > > > native order and I'd expect the constant to be as well.
> > > 
> > > I know nothing about this, maybe Francois remembers why it's there. But
> > > booting linux with -kernel works so it's probably either correct or does not
> > > matter.
> > 
> > Have you attempted it on both BE and LE hosts though?
> 
> No, I don't have a BE host, only tried on LE. I'm guessing this may come
> from hw/ppc/virtex_ml507.c where EPAPR_MAGIC is also swapped.

Sounds like virtex is wrong too - bet they haven't tried on a BE host either.

> The only other
> place this magic number appears is in e500 where it's not swapped though so
> I don't really know what should be correct here. In u-boot sources
> (arch/powerpc/lib/bootm.c) it seems to use this magic if CONFIG_OF_LIBFDT is
> defined which seems to be set for this board so that means we likely need
> this (but maybe not for the legacy uImages I've tried). Since CPU is big
> endian and constant is defined on the host this probably should be
> cpu_to_be32(EPAPR_MAGIC). Does that sound better?

No.  It's going into a register, not memory, and the registers are in
host-native order, since they're being manipulated as whole values,
not byte arrays (the appropriate swaps for the guest endianness happen
when emulating load and store instructions).  Remember an integer is
just an integer - it's only when you encode it into a byte array that
it acquires an endianness.

Although it sounded from another part of this thread as if this might
be a non-epapr boot anyway, making the whole thing moot.

> [...]
> > > > > +    /* FIXME: remove this after fixing l2sram mapping in ppc440_uc.c? */
> > > > > +    memory_region_init_ram(l2cache_ram, NULL, "ppc440.l2cache_ram", 256 << 10,
> > > > > +                           &error_abort);
> > > > > +    memory_region_add_subregion(address_space_mem, 0x400000000LL, l2cache_ram);
> > > > > +
> > > > > +    /* USB */
> > > > > +    sysbus_create_simple(TYPE_PPC4xx_EHCI, 0x4bffd0400, uic[2][29]);
> > > > > +    dev = qdev_create(NULL, "sysbus-ohci");
> > > > > +    qdev_prop_set_string(dev, "masterbus", "usb-bus.0");
> > > > > +    qdev_prop_set_uint32(dev, "num-ports", 6);
> > > > > +    qdev_init_nofail(dev);
> > > > > +    sbdev = SYS_BUS_DEVICE(dev);
> > > > > +    sysbus_mmio_map(sbdev, 0, 0x4bffd0000);
> > > > > +    sysbus_connect_irq(sbdev, 0, uic[2][30]);
> > > > > +    usb_create_simple(usb_bus_find(-1), "usb-kbd");
> > > > > +    usb_create_simple(usb_bus_find(-1), "usb-mouse");
> > > > > +
> > > > > +    /* PCI bus */
> > > > > +    ppc460ex_pcie_init(env);
> > > > > +    /*XXX: FIXME: is this correct? */
> > > > > +    dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec00000,
> > > > > +                                uic[1][0], uic[1][20], uic[1][21], uic[1][22],
> > > > > +                                NULL);
> > > > > +    pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
> > > > > +    if (!pci_bus) {
> > > > > +        fprintf(stderr, "couldn't create PCI controller!\n");
> > > > > +        exit(1);
> > > > > +    }
> > > > > +    memory_region_init_alias(isa, NULL, "isa_mmio", get_system_io(),
> > > > > +                             0, 0x10000);
> > > > > +    memory_region_add_subregion(get_system_memory(), 0xc08000000, isa);
> > > > 
> > > > Does it really make sense to just embed the ISA IO space here, rather
> > > > than actually instanting a PCI<->ISA bridge?
> > > 
> > > I'm not sure if this is correct but I don't know how it's handled on real
> > > hardware. The board does not have ISA and I don't think it has a bridge but
> > > the IO space appears at this location according to the datasheet (In System
> > > Memory Address Map it's listed as PCI I/O 0xc08000000-0xc0800ffff) and
> > > clients expect PCI card's io registers to be accessible here. If anyone
> > > knows how it's done on real hardware and if there's a better way to model
> > > this please let me know.
> > 
> > Ah, ok.  I think the confusion here is that you can have PCI I/O space
> > without ISA or a system IO space.  In fact that's pretty standard on
> > things without a CPU level IO space (which means just about everything
> > except x86).
> > 
> > But in that case I'd expect the PCI host bridge to map its IO memory
> > regions directly into address_space_memory rather than involving the
> > global address_space_io (what get_system_io()) returns.  The only
> > reason I can see that you'd need to involve get_system_io() is if you
> > have devices registering themselves directly into the global IO space,
> > which should only happen for legacy ISA devices, which it sounds like
> > you don't have.
> > 
> > Possibly this is an error in the PCI bridge implementation that your
> > code here is essentially a workaround for, though.
> 
> So in my understanding, there's a system_io space created automatically
> which appears in the memory tree anyway but would otherwise be unused and
> this was just reused here for the pci io space so it does not need another
> memory region for this. If it's not acceptable this way (although
> ppc440_bamboo.c and ppc4xx_pci.c also does it the exact same way) an
> alternative may be to change it to add another mem region for io to
> ppc440_pcix.c (although it already has iomem for pci.reg so this might be
> more confusing than using system_io here) but I think pcix device can't map
> this to address_space_memory itself because this device could appear in
> different SoCs where the memory areas might be at different addresses so
> this new region should then be registered as a sysbus mmio space and be
> mapped from the board code with sysbus_mmio_map()? I find that much more
> confusing than how it's done now which is also more consistent with existing
> code modelling similar devices.

Ugh, I see.  I hadn't realized bamboo etc. were doing this.  It's not
a good idea - most obviously it breaks down if you have multiple
independent PCI host bridges, which have independent IO spaces.

I'm a bit torn whether to let this in as is because precedent and hope
to get them all fixed at some future point, or to insist that you
change it to stop the bad idea spreading.

The right way is, indeed, to create a new memory region for the
bridge's IO space.  A SysBusDevice can have multiple MMIO regions, so
you can use one for the bridge's memory space, another for its IO
space (and still more for config space and/or bridge internal
registers if you need them).

> > > > > +    /* PCI devices */
> > > > > +    pci_create_simple(pci_bus, PCI_DEVFN(6, 0), "sm501");
> > > > > +    /* SoC has a single SATA port but we don't emulate that yet
> > > > > +     * However, firmware and usual clients have driver for SiI311x
> > > > > +     * so add one for convenience by default */
> > > > > +    pci_create_simple(pci_bus, -1, "sii3112");
> > > > 
> > > > You should probably not create this when started with -nodefaults.
> > > 
> > > We don't emulate the on-board SATA port of the SoC and without this there's
> > > no other way to connect disks (maybe over USB, but firmware has a bug which
> > > prevents that too even on real hardware AFAIK, I've backported a fix which
> > > made booting from USB work but that broke keyboard) so while this is an
> > > external card it's pretty much unusable without this so it's added by
> > > default.
> > 
> > Yes, but you're not just adding it by default, you're adding it
> > *always*.  -nodefaults should turn that off (and the user will need to
> > manually instantiate it - or another disk controller, I guess).
> 
> OK I got it now. I still don't see when -nodefaults could be useful to
> cripple the board and make it easier to create non-working configurations
> but I can easily add this conditional around this line and hope users stay
> away from this switch and won't complain when it does not boot when they use
> it. (Well, it does not really boot most of the time without this switch
> either so I don't think it matters much at the moment :-) )

The convention exists because for non-trivial cases the next layer up
- whether it's a person or a management layer like livirt - often
needs more explicit and complete control of the device in the system.
Even if that's at the cost of having to explicitly add essential
devices.  It's still pretty imperfect, there's still a distinction
between "default" and "always present" devices which can be far from
obvious.

Nonetheless, I think the sil device lies firmly enough on the
"shouldn't be always present" side of the fuzzy line to exclude it
with -nodefaults.

Note that this can also be useful for experiments - e.g. just looking
at firmware behaviour with no disk at all, adding some entirely
different block interface with a specialized kernel to handle it.  Or
even booting into a kernel running purely from a ramfs with no disk.
diff mbox

Patch

diff --git a/default-configs/ppcemb-softmmu.mak b/default-configs/ppcemb-softmmu.mak
index 635923a..90b42f0 100644
--- a/default-configs/ppcemb-softmmu.mak
+++ b/default-configs/ppcemb-softmmu.mak
@@ -17,3 +17,6 @@  CONFIG_XILINX=y
 CONFIG_XILINX_ETHLITE=y
 CONFIG_LIBDECNUMBER=y
 CONFIG_SM501=y
+CONFIG_USB_EHCI_SYSBUS=y
+CONFIG_IDE_SII3112=y
+CONFIG_SAM460EX=y
diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
index fc39fe4..0aaea5b 100644
--- a/hw/ppc/Makefile.objs
+++ b/hw/ppc/Makefile.objs
@@ -27,3 +27,5 @@  obj-$(CONFIG_E500) += e500.o mpc8544ds.o e500plat.o
 obj-$(CONFIG_E500) += mpc8544_guts.o ppce500_spin.o
 # PowerPC 440 Xilinx ML507 reference board.
 obj-$(CONFIG_XILINX) += virtex_ml507.o
+# ACube Sam460ex board.
+obj-$(CONFIG_SAM460EX) += ppc440_uc.o sam460ex.o
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
new file mode 100644
index 0000000..7cf4f6f
--- /dev/null
+++ b/hw/ppc/sam460ex.c
@@ -0,0 +1,611 @@ 
+/*
+ * QEMU aCube Sam460ex board emulation
+ *
+ * Copyright (c) 2012 François Revol
+ * Copyright (c) 2016-2017 BALATON Zoltan
+ *
+ * This file is derived from hw/ppc440_bamboo.c,
+ * the copyright for that material belongs to the original owners.
+ *
+ * This work is licensed under the GNU GPL license version 2 or later.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "qemu/cutils.h"
+#include "qapi/error.h"
+#include "hw/hw.h"
+#include "sysemu/blockdev.h"
+#include "hw/boards.h"
+#include "sysemu/kvm.h"
+#include "kvm_ppc.h"
+#include "sysemu/device_tree.h"
+#include "sysemu/block-backend.h"
+#include "hw/loader.h"
+#include "elf.h"
+#include "exec/address-spaces.h"
+#include "exec/memory.h"
+#include "hw/ppc/ppc440.h"
+#include "hw/ppc/ppc405.h"
+#include "hw/block/flash.h"
+#include "sysemu/sysemu.h"
+#include "hw/sysbus.h"
+#include "hw/char/serial.h"
+#include "hw/i2c/ppc4xx_i2c.h"
+#include "hw/i2c/smbus.h"
+#include "hw/usb/hcd-ehci.h"
+
+#define BINARY_DEVICE_TREE_FILE "sam460ex.dtb"
+#define UBOOT_FILENAME "u-boot-sam460-20100605.bin"
+/* to extract the official U-Boot bin from the updater: */
+/* dd bs=1 skip=$(($(stat -c '%s' updater/updater-460) - 0x80000)) \
+     if=updater/updater-460 of=u-boot-sam460-20100605.bin */
+
+/* from Sam460 U-Boot include/configs/Sam460ex.h */
+#define FLASH_BASE             0xfff00000
+#define FLASH_BASE_H           0x4
+#define FLASH_SIZE             (1 << 20)
+#define UBOOT_LOAD_BASE        0xfff80000
+#define UBOOT_SIZE             0x00080000
+#define UBOOT_ENTRY            0xfffffffc
+
+/* from U-Boot */
+#define EPAPR_MAGIC           (0x45504150)
+#define KERNEL_ADDR           0x1000000
+#define FDT_ADDR              0x1800000
+#define RAMDISK_ADDR          0x1900000
+
+/* Sam460ex IRQ MAP:
+   IRQ0  = ETH_INT
+   IRQ1  = FPGA_INT
+   IRQ2  = PCI_INT (PCIA, PCIB, PCIC, PCIB)
+   IRQ3  = FPGA_INT2
+   IRQ11 = RTC_INT
+   IRQ12 = SM502_INT
+*/
+
+#define SDRAM_NR_BANKS 4
+
+/* FIXME: See u-boot.git 8ac41e */
+static const unsigned int ppc460ex_sdram_bank_sizes[] = {
+    1024 << 20, 512 << 20, 256 << 20, 128 << 20, 64 << 20, 32 << 20, 0
+};
+
+struct boot_info {
+    uint32_t dt_base;
+    uint32_t dt_size;
+    uint32_t entry;
+};
+
+/*****************************************************************************/
+/* SPD eeprom content from mips_malta.c */
+
+struct _eeprom24c0x_t {
+  uint8_t tick;
+  uint8_t address;
+  uint8_t command;
+  uint8_t ack;
+  uint8_t scl;
+  uint8_t sda;
+  uint8_t data;
+  uint8_t contents[256];
+};
+
+typedef struct _eeprom24c0x_t eeprom24c0x_t;
+
+static eeprom24c0x_t spd_eeprom = {
+    .contents = {
+        /* 00000000: */ 0x80, 0x08, 0xFF, 0x0D, 0x0A, 0xFF, 0x40, 0x00,
+        /* 00000008: */ 0x04, 0x75, 0x54, 0x00, 0x82, 0x08, 0x00, 0x01,
+        /* 00000010: */ 0x8F, 0x04, 0x02, 0x01, 0x01, 0x00, 0x00, 0x00,
+        /* 00000018: */ 0x00, 0x00, 0x00, 0x14, 0x0F, 0x14, 0x2D, 0xFF,
+        /* 00000020: */ 0x15, 0x08, 0x15, 0x08, 0x00, 0x00, 0x00, 0x00,
+        /* 00000028: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        /* 00000030: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        /* 00000038: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x12, 0xD0,
+        /* 00000040: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        /* 00000048: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        /* 00000050: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        /* 00000058: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        /* 00000060: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        /* 00000068: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        /* 00000070: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        /* 00000078: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x64, 0xF4,
+    },
+};
+
+static void generate_eeprom_spd(uint8_t *eeprom, ram_addr_t ram_size)
+{
+    enum { SDR = 0x4, DDR1 = 0x7, DDR2 = 0x8 } type;
+    uint8_t *spd = spd_eeprom.contents;
+    uint8_t nbanks = 0;
+    uint16_t density = 0;
+    int i;
+
+    /* work in terms of MB */
+    ram_size >>= 20;
+
+    while ((ram_size >= 4) && (nbanks <= 2)) {
+        int sz_log2 = MIN(31 - clz32(ram_size), 14);
+        nbanks++;
+        density |= 1 << (sz_log2 - 2);
+        ram_size -= 1 << sz_log2;
+    }
+
+    /* split to 2 banks if possible */
+    if ((nbanks == 1) && (density > 1)) {
+        nbanks++;
+        density >>= 1;
+    }
+
+    if (density & 0xff00) {
+        density = (density & 0xe0) | ((density >> 8) & 0x1f);
+        type = DDR2;
+    } else if (!(density & 0x1f)) {
+        type = DDR2;
+    } else {
+        type = SDR;
+    }
+
+    if (ram_size) {
+        fprintf(stderr, "Warning: SPD cannot represent final %dMB"
+                " of SDRAM\n", (int)ram_size);
+    }
+
+    /* fill in SPD memory information */
+    spd[2] = type;
+    spd[5] = nbanks;
+    spd[31] = density;
+#ifdef DEBUG_SDRAM
+    printf("SPD: nbanks %d density %d\n", nbanks, density);
+#endif
+    /* XXX: this is totally random */
+    spd[9] = 0x10; /* CAS tcyc */
+    spd[18] = 0x20; /* CAS bit */
+    spd[23] = 0x10; /* CAS tcyc */
+    spd[25] = 0x10; /* CAS tcyc */
+
+    /* checksum */
+    spd[63] = 0;
+    for (i = 0; i < 63; i++) {
+        spd[63] += spd[i];
+    }
+
+    /* copy for SMBUS */
+    memcpy(eeprom, spd, sizeof(spd_eeprom.contents));
+}
+
+static void generate_eeprom_serial(uint8_t *eeprom)
+{
+    int i, pos = 0;
+    uint8_t mac[6] = { 0x00 };
+    uint8_t sn[5] = { 0x01, 0x23, 0x45, 0x67, 0x89 };
+
+    /* version */
+    eeprom[pos++] = 0x01;
+
+    /* count */
+    eeprom[pos++] = 0x02;
+
+    /* MAC address */
+    eeprom[pos++] = 0x01; /* MAC */
+    eeprom[pos++] = 0x06; /* length */
+    memcpy(&eeprom[pos], mac, sizeof(mac));
+    pos += sizeof(mac);
+
+    /* serial number */
+    eeprom[pos++] = 0x02; /* serial */
+    eeprom[pos++] = 0x05; /* length */
+    memcpy(&eeprom[pos], sn, sizeof(sn));
+    pos += sizeof(sn);
+
+    /* checksum */
+    eeprom[pos] = 0;
+    for (i = 0; i < pos; i++) {
+        eeprom[pos] += eeprom[i];
+    }
+}
+
+/*****************************************************************************/
+
+static int sam460ex_load_uboot(void)
+{
+    DriveInfo *dinfo;
+    BlockBackend *blk = NULL;
+    hwaddr base = FLASH_BASE | ((hwaddr)FLASH_BASE_H << 32);
+    long bios_size = FLASH_SIZE;
+    int fl_sectors;
+
+    dinfo = drive_get(IF_PFLASH, 0, 0);
+    if (dinfo) {
+        blk = blk_by_legacy_dinfo(dinfo);
+        bios_size = blk_getlength(blk);
+    }
+    fl_sectors = (bios_size + 65535) >> 16;
+
+    if (!pflash_cfi01_register(base, NULL, "sam460ex.flash", bios_size,
+                               blk, (64 * 1024), fl_sectors,
+                               1, 0x89, 0x18, 0x0000, 0x0, 1)) {
+        fprintf(stderr, "qemu: Error registering flash memory.\n");
+        /* XXX: return an error instead? */
+        exit(1);
+    }
+
+    if (!blk) {
+        /*fprintf(stderr, "No flash image given with the 'pflash' parameter,"
+                " using default u-boot image\n");*/
+        base = UBOOT_LOAD_BASE | ((hwaddr)FLASH_BASE_H << 32);
+        rom_add_file_fixed(UBOOT_FILENAME, base, -1);
+    }
+
+    return 0;
+}
+
+static int sam460ex_load_device_tree(hwaddr addr,
+                                     uint32_t ramsize,
+                                     hwaddr initrd_base,
+                                     hwaddr initrd_size,
+                                     const char *kernel_cmdline)
+{
+    int ret = -1;
+    uint32_t mem_reg_property[] = { 0, 0, cpu_to_be32(ramsize) };
+    char *filename;
+    int fdt_size;
+    void *fdt;
+    uint32_t tb_freq = 400000000;
+    uint32_t clock_freq = 400000000;
+
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE);
+    if (!filename) {
+        goto out;
+    }
+    fdt = load_device_tree(filename, &fdt_size);
+    g_free(filename);
+    if (fdt == NULL) {
+        goto out;
+    }
+
+    /* Manipulate device tree in memory. */
+
+    ret = qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
+                               sizeof(mem_reg_property));
+    if (ret < 0) {
+        fprintf(stderr, "couldn't set /memory/reg\n");
+    }
+
+    /* default FDT doesn't have a /chosen node... */
+    qemu_fdt_add_subnode(fdt, "/chosen");
+
+    ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
+                                    initrd_base);
+    if (ret < 0) {
+        fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
+    }
+
+    ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
+                                    (initrd_base + initrd_size));
+    if (ret < 0) {
+        fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
+    }
+
+    ret = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
+                                      kernel_cmdline);
+    if (ret < 0) {
+        fprintf(stderr, "couldn't set /chosen/bootargs\n");
+    }
+
+    /* Copy data from the host device tree into the guest. Since the guest can
+     * directly access the timebase without host involvement, we must expose
+     * the correct frequencies. */
+    if (kvm_enabled()) {
+        tb_freq = kvmppc_get_tbfreq();
+        clock_freq = kvmppc_get_clockfreq();
+    }
+
+    qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "clock-frequency",
+                              clock_freq);
+    qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "timebase-frequency",
+                              tb_freq);
+
+    rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
+    g_free(fdt);
+    ret = fdt_size;
+
+out:
+
+    return ret;
+}
+
+/* Create reset TLB entries for BookE, mapping only the flash memory.  */
+static void mmubooke_create_initial_mapping_uboot(CPUPPCState *env)
+{
+    ppcemb_tlb_t *tlb = &env->tlb.tlbe[0];
+
+    /* on reset the flash is mapped by a shadow TLB,
+     * but since we don't implement them we need to use
+     * the same values U-Boot will use to avoid a fault.
+     */
+    tlb->attr = 0;
+    tlb->prot = PAGE_VALID | ((PAGE_READ | PAGE_WRITE | PAGE_EXEC) << 4);
+    tlb->size = 0x10000000; /* up to 0xffffffff  */
+    tlb->EPN = 0xf0000000 & TARGET_PAGE_MASK;
+    tlb->RPN = (0xf0000000 & TARGET_PAGE_MASK) | 0x4;
+    tlb->PID = 0;
+}
+
+/* Create reset TLB entries for BookE, spanning the 32bit addr space.  */
+static void mmubooke_create_initial_mapping(CPUPPCState *env,
+                                     target_ulong va,
+                                     hwaddr pa)
+{
+    ppcemb_tlb_t *tlb = &env->tlb.tlbe[0];
+
+    tlb->attr = 0;
+    tlb->prot = PAGE_VALID | ((PAGE_READ | PAGE_WRITE | PAGE_EXEC) << 4);
+    tlb->size = 1 << 31; /* up to 0x80000000  */
+    tlb->EPN = va & TARGET_PAGE_MASK;
+    tlb->RPN = pa & TARGET_PAGE_MASK;
+    tlb->PID = 0;
+}
+
+static void main_cpu_reset(void *opaque)
+{
+    PowerPCCPU *cpu = opaque;
+    CPUPPCState *env = &cpu->env;
+    struct boot_info *bi = env->load_info;
+
+    cpu_reset(CPU(cpu));
+
+    /* either we have a kernel to boot or we jump to U-Boot */
+    if (bi->entry != UBOOT_ENTRY) {
+        env->gpr[1] = (16 << 20) - 8;
+        env->gpr[3] = FDT_ADDR;
+
+        fprintf(stderr, "cpu reset: kernel entry %x\n", bi->entry);
+        env->nip = bi->entry;
+
+        /* Create a mapping for the kernel.  */
+        mmubooke_create_initial_mapping(env, 0, 0);
+        env->gpr[6] = tswap32(EPAPR_MAGIC);
+        env->gpr[7] = (16 << 20) - 8; /*bi->ima_size;*/
+
+    } else {
+        env->nip = UBOOT_ENTRY;
+        mmubooke_create_initial_mapping_uboot(env);
+        fprintf(stderr, "cpu reset: U-Boot entry\n");
+    }
+}
+
+static void sam460ex_init(MachineState *machine)
+{
+    MemoryRegion *address_space_mem = get_system_memory();
+    MemoryRegion *isa = g_new(MemoryRegion, 1);
+    MemoryRegion *ram_memories = g_new(MemoryRegion, SDRAM_NR_BANKS);
+    hwaddr ram_bases[SDRAM_NR_BANKS];
+    hwaddr ram_sizes[SDRAM_NR_BANKS];
+    MemoryRegion *l2cache_ram = g_new(MemoryRegion, 1);
+    qemu_irq *irqs, *uic[4];
+    PCIBus *pci_bus;
+    PowerPCCPU *cpu;
+    CPUPPCState *env;
+    PPC4xxI2CState *i2c[2];
+    hwaddr entry = UBOOT_ENTRY;
+    hwaddr loadaddr = 0;
+    target_long initrd_size = 0;
+    DeviceState *dev;
+    SysBusDevice *sbdev;
+    int success;
+    int i;
+    struct boot_info *boot_info;
+    const size_t smbus_eeprom_size = 8 * 256;
+    uint8_t *smbus_eeprom_buf = g_malloc0(smbus_eeprom_size);
+
+    /* Setup CPU. */
+    if (machine->cpu_model == NULL) {
+        machine->cpu_model = "460EX";
+    }
+    cpu = cpu_ppc_init(machine->cpu_model);
+    if (cpu == NULL) {
+        fprintf(stderr, "Unable to initialize CPU!\n");
+        exit(1);
+    }
+    env = &cpu->env;
+
+    qemu_register_reset(main_cpu_reset, cpu);
+    boot_info = g_malloc0(sizeof(*boot_info));
+    env->load_info = boot_info;
+
+    ppc_booke_timers_init(cpu, 50000000, 0);
+    ppc_dcr_init(env, NULL, NULL);
+
+    /* PLB arbitrer */
+    ppc4xx_plb_init(env);
+
+    /* interrupt controllers */
+    irqs = g_malloc0(sizeof(*irqs) * PPCUIC_OUTPUT_NB);
+    irqs[PPCUIC_OUTPUT_INT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT];
+    irqs[PPCUIC_OUTPUT_CINT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT];
+    uic[0] = ppcuic_init(env, irqs, 0x0C0, 0, 1);
+    uic[1] = ppcuic_init(env, &uic[0][30], 0x0D0, 0, 1);
+    uic[2] = ppcuic_init(env, &uic[0][10], 0x0E0, 0, 1);
+    uic[3] = ppcuic_init(env, &uic[0][16], 0x0F0, 0, 1);
+
+    /* SDRAM controller */
+    memset(ram_bases, 0, sizeof(ram_bases));
+    memset(ram_sizes, 0, sizeof(ram_sizes));
+    /* put all RAM on first bank because board has one slot
+     * and firmware only checks that */
+    machine->ram_size = ppc4xx_sdram_adjust(machine->ram_size,
+                                   1/*SDRAM_NR_BANKS*/,
+                                   ram_memories,
+                                   ram_bases, ram_sizes,
+                                   ppc460ex_sdram_bank_sizes);
+#ifdef DEBUG_SDRAM
+    printf("RAMSIZE %dMB\n", (int)(machine->ram_size / M_BYTE));
+#endif
+
+    /* XXX does 460EX have ECC interrupts? */
+    ppc440_sdram_init(env, SDRAM_NR_BANKS, ram_memories,
+                      ram_bases, ram_sizes, 1);
+
+    /* generate SPD EEPROM data */
+    for (i = 0; i < SDRAM_NR_BANKS; i++) {
+#ifdef DEBUG_SDRAM
+        printf("bank %d: %" HWADDR_PRIx "\n", i, ram_sizes[i]);
+#endif
+        generate_eeprom_spd(&smbus_eeprom_buf[i * 256], ram_sizes[i]);
+    }
+    generate_eeprom_serial(&smbus_eeprom_buf[4 * 256]);
+    generate_eeprom_serial(&smbus_eeprom_buf[6 * 256]);
+
+    /* IIC controllers */
+    dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600700, uic[0][2]);
+    i2c[0] = PPC4xx_I2C(dev);
+    object_property_set_bool(OBJECT(dev), true, "realized", NULL);
+    smbus_eeprom_init(i2c[0]->bus, 8, smbus_eeprom_buf, smbus_eeprom_size);
+    g_free(smbus_eeprom_buf);
+
+    dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600800, uic[0][3]);
+    i2c[1] = PPC4xx_I2C(dev);
+
+    /* External bus controller */
+    ppc405_ebc_init(env);
+
+    /* CPR */
+    ppc4xx_cpr_init(env);
+
+    /* PLB to AHB bridge */
+    ppc4xx_ahb_init(env);
+
+    /* System DCRs */
+    ppc4xx_sdr_init(env);
+
+    /* MAL */
+    ppc4xx_mal_init(env, 4, 16, &uic[2][3]);
+
+    /* 256K of L2 cache as memory */
+    ppc4xx_l2sram_init(env);
+    /* FIXME: remove this after fixing l2sram mapping in ppc440_uc.c? */
+    memory_region_init_ram(l2cache_ram, NULL, "ppc440.l2cache_ram", 256 << 10,
+                           &error_abort);
+    memory_region_add_subregion(address_space_mem, 0x400000000LL, l2cache_ram);
+
+    /* USB */
+    sysbus_create_simple(TYPE_PPC4xx_EHCI, 0x4bffd0400, uic[2][29]);
+    dev = qdev_create(NULL, "sysbus-ohci");
+    qdev_prop_set_string(dev, "masterbus", "usb-bus.0");
+    qdev_prop_set_uint32(dev, "num-ports", 6);
+    qdev_init_nofail(dev);
+    sbdev = SYS_BUS_DEVICE(dev);
+    sysbus_mmio_map(sbdev, 0, 0x4bffd0000);
+    sysbus_connect_irq(sbdev, 0, uic[2][30]);
+    usb_create_simple(usb_bus_find(-1), "usb-kbd");
+    usb_create_simple(usb_bus_find(-1), "usb-mouse");
+
+    /* PCI bus */
+    ppc460ex_pcie_init(env);
+    /*XXX: FIXME: is this correct? */
+    dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec00000,
+                                uic[1][0], uic[1][20], uic[1][21], uic[1][22],
+                                NULL);
+    pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
+    if (!pci_bus) {
+        fprintf(stderr, "couldn't create PCI controller!\n");
+        exit(1);
+    }
+    memory_region_init_alias(isa, NULL, "isa_mmio", get_system_io(),
+                             0, 0x10000);
+    memory_region_add_subregion(get_system_memory(), 0xc08000000, isa);
+
+    /* PCI devices */
+    pci_create_simple(pci_bus, PCI_DEVFN(6, 0), "sm501");
+    /* SoC has a single SATA port but we don't emulate that yet
+     * However, firmware and usual clients have driver for SiI311x
+     * so add one for convenience by default */
+    pci_create_simple(pci_bus, -1, "sii3112");
+
+    /* SoC has 4 UARTs
+     * but board has only one wired and two are present in fdt */
+    if (serial_hds[0] != NULL) {
+        serial_mm_init(address_space_mem, 0x4ef600300, 0, uic[1][1],
+                       PPC_SERIAL_MM_BAUDBASE, serial_hds[0],
+                       DEVICE_BIG_ENDIAN);
+    }
+    if (serial_hds[1] != NULL) {
+        serial_mm_init(address_space_mem, 0x4ef600400, 0, uic[0][1],
+                       PPC_SERIAL_MM_BAUDBASE, serial_hds[1],
+                       DEVICE_BIG_ENDIAN);
+    }
+
+    /* Load U-Boot image. */
+    if (!machine->kernel_filename) {
+        success = sam460ex_load_uboot();
+        if (success < 0) {
+            fprintf(stderr, "qemu: could not load firmware\n");
+            exit(1);
+        }
+    }
+
+    /* Load kernel. */
+    if (machine->kernel_filename) {
+        success = load_uimage(machine->kernel_filename, &entry, &loadaddr, NULL,
+            NULL, NULL);
+        fprintf(stderr, "load_uimage: %d\n", success);
+        if (success < 0) {
+            uint64_t elf_entry, elf_lowaddr;
+
+            success = load_elf(machine->kernel_filename, NULL, NULL, &elf_entry,
+                               &elf_lowaddr, NULL, 1, PPC_ELF_MACHINE, 0, 0);
+            fprintf(stderr, "load_elf: %d\n", success);
+            entry = elf_entry;
+            loadaddr = elf_lowaddr;
+        }
+        /* XXX try again as binary */
+        if (success < 0) {
+            fprintf(stderr, "qemu: could not load kernel '%s'\n",
+                    machine->kernel_filename);
+            exit(1);
+        }
+    }
+
+    /* Load initrd. */
+    if (machine->initrd_filename) {
+        initrd_size = load_image_targphys(machine->initrd_filename,
+                                          RAMDISK_ADDR,
+                                          machine->ram_size - RAMDISK_ADDR);
+        fprintf(stderr, "load_image: %d\n", initrd_size);
+        if (initrd_size < 0) {
+            fprintf(stderr, "qemu: could not load ram disk '%s' at %x\n",
+                    machine->initrd_filename, RAMDISK_ADDR);
+            exit(1);
+        }
+    }
+
+    /* If we're loading a kernel directly, we must load the device tree too. */
+    if (machine->kernel_filename) {
+        int dt_size;
+
+        dt_size = sam460ex_load_device_tree(FDT_ADDR, machine->ram_size,
+                                    RAMDISK_ADDR, initrd_size,
+                                    machine->kernel_cmdline);
+        if (dt_size < 0) {
+            fprintf(stderr, "couldn't load device tree\n");
+            exit(1);
+        }
+
+        boot_info->dt_base = FDT_ADDR;
+        boot_info->dt_size = dt_size;
+    }
+
+    boot_info->entry = entry;
+}
+
+static void sam460ex_machine_init(MachineClass *mc)
+{
+    mc->desc = "aCube Sam460ex";
+    mc->init = sam460ex_init;
+    mc->default_ram_size = 512 * M_BYTE;
+}
+
+DEFINE_MACHINE("sam460ex", sam460ex_machine_init)