Re: [Qemu-devel] [PATCH v2 30/30] target-sparc: fix up niagara machine

[Artyom Tarasenko]

On Thu, Jan 26, 2017 at 8:35 AM, Markus Armbruster <address@hidden> wrote:
> niagara_init() does something naughty, which conflicts with Max's
> "[PATCH v6 0/9] block: Drop BDS.filename".  Details inline.
>
> Artyom Tarasenko <address@hidden> writes:
>
>> Remove the Niagara stub implementation from sun4u.c and add a machine,
>> compatible with Legion simulator from the OpenSPARC T1 project.
>>
>> The machine uses the firmware supplied with the OpenSPARC T1 project,
>> http://download.oracle.com/technetwork/systems/opensparc/OpenSPARCT1_Arch.1.5.tar.bz2
>> in the directory S10image/, and is able to boot the supplied Solaris 10 
>> image.
>>
>> Note that for compatibility with the naming conventions for SPARC machines
>> the new machine name is lowercase niagara.
>>
>> Signed-off-by: Artyom Tarasenko <address@hidden>
>> Reviewed-by: Richard Henderson <address@hidden>
>> ---
>>  MAINTAINERS                         |  13 +--
>>  default-configs/sparc64-softmmu.mak |   2 +
>>  hw/sparc64/Makefile.objs            |   1 +
>>  hw/sparc64/niagara.c                | 177 
>> ++++++++++++++++++++++++++++++++++++
>>  hw/sparc64/sun4u.c                  |  31 -------
>>  qemu-doc.texi                       |  14 ++-
>>  6 files changed, 199 insertions(+), 39 deletions(-)
>>  create mode 100644 hw/sparc64/niagara.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 54588e5..b5ebfab 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -725,6 +725,13 @@ S: Maintained
>>  F: hw/sparc64/sun4u.c
>>  F: pc-bios/openbios-sparc64
>>
>> +Sun4v
>> +M: Artyom Tarasenko <address@hidden>
>> +S: Maintained
>> +F: hw/sparc64/sun4v.c
>> +F: hw/timer/sun4v-rtc.c
>> +F: include/hw/timer/sun4v-rtc.h
>> +
>>  Leon3
>>  M: Fabien Chouteau <address@hidden>
>>  S: Maintained
>> @@ -1098,12 +1105,6 @@ F: hw/nvram/chrp_nvram.c
>>  F: include/hw/nvram/chrp_nvram.h
>>  F: tests/prom-env-test.c
>>
>> -sun4v RTC
>> -M: Artyom Tarasenko <address@hidden>
>> -S: Maintained
>> -F: hw/timer/sun4v-rtc.c
>> -F: include/hw/timer/sun4v-rtc.h
>> -
>>  Subsystems
>>  ----------
>>  Audio
>> diff --git a/default-configs/sparc64-softmmu.mak 
>> b/default-configs/sparc64-softmmu.mak
>> index c0cdd64..c581e61 100644
>> --- a/default-configs/sparc64-softmmu.mak
>> +++ b/default-configs/sparc64-softmmu.mak
>> @@ -13,3 +13,5 @@ CONFIG_IDE_CMD646=y
>>  CONFIG_PCI_APB=y
>>  CONFIG_MC146818RTC=y
>>  CONFIG_ISA_TESTDEV=y
>> +CONFIG_EMPTY_SLOT=y
>> +CONFIG_SUN4V_RTC=y
>> diff --git a/hw/sparc64/Makefile.objs b/hw/sparc64/Makefile.objs
>> index a96b1f8..cf9de21 100644
>> --- a/hw/sparc64/Makefile.objs
>> +++ b/hw/sparc64/Makefile.objs
>> @@ -1,2 +1,3 @@
>>  obj-y += sparc64.o
>>  obj-y += sun4u.o
>> +obj-y += niagara.o
>> \ No newline at end of file
>> diff --git a/hw/sparc64/niagara.c b/hw/sparc64/niagara.c
>> new file mode 100644
>> index 0000000..b55d4bb
>> --- /dev/null
>> +++ b/hw/sparc64/niagara.c
>> @@ -0,0 +1,177 @@
>> +/*
>> + * QEMU Sun4v/Niagara System Emulator
>> + *
>> + * Copyright (c) 2016 Artyom Tarasenko
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a 
>> copy
>> + * of this software and associated documentation files (the "Software"), to 
>> deal
>> + * in the Software without restriction, including without limitation the 
>> rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included 
>> in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
>> OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
>> FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qapi/error.h"
>> +#include "qemu-common.h"
>> +#include "cpu.h"
>> +#include "hw/hw.h"
>> +#include "hw/boards.h"
>> +#include "hw/char/serial.h"
>> +#include "hw/empty_slot.h"
>> +#include "hw/loader.h"
>> +#include "hw/sparc/sparc64.h"
>> +#include "hw/timer/sun4v-rtc.h"
>> +#include "exec/address-spaces.h"
>> +#include "sysemu/block-backend.h"
>> +
>> +
>> +typedef struct NiagaraBoardState {
>> +    MemoryRegion hv_ram;
>> +    MemoryRegion partition_ram;
>> +    MemoryRegion nvram;
>> +    MemoryRegion md_rom;
>> +    MemoryRegion hv_rom;
>> +    MemoryRegion vdisk_ram;
>> +    MemoryRegion prom;
>> +} NiagaraBoardState;
>> +
>> +#define NIAGARA_HV_RAM_BASE 0x100000ULL
>> +#define NIAGARA_HV_RAM_SIZE 0x3f00000ULL /* 63 MiB */
>> +
>> +#define NIAGARA_PARTITION_RAM_BASE 0x80000000ULL
>> +
>> +#define NIAGARA_UART_BASE   0x1f10000000ULL
>> +
>> +#define NIAGARA_NVRAM_BASE  0x1f11000000ULL
>> +#define NIAGARA_NVRAM_SIZE  0x2000
>> +
>> +#define NIAGARA_MD_ROM_BASE 0x1f12000000ULL
>> +#define NIAGARA_MD_ROM_SIZE 0x2000
>> +
>> +#define NIAGARA_HV_ROM_BASE 0x1f12080000ULL
>> +#define NIAGARA_HV_ROM_SIZE 0x2000
>> +
>> +#define NIAGARA_IOBBASE     0x9800000000ULL
>> +#define NIAGARA_IOBSIZE     0x0100000000ULL
>> +
>> +#define NIAGARA_VDISK_BASE  0x1f40000000ULL
>> +#define NIAGARA_RTC_BASE    0xfff0c1fff8ULL
>> +#define NIAGARA_UART_BASE   0x1f10000000ULL
>> +
>> +/* Firmware layout
>> + *
>> + * |------------------|
>> + * |   openboot.bin   |
>> + * |------------------| PROM_ADDR + OBP_OFFSET
>> + * |      q.bin       |
>> + * |------------------| PROM_ADDR + Q_OFFSET
>> + * |     reset.bin    |
>> + * |------------------| PROM_ADDR
>> + */
>> +#define NIAGARA_PROM_BASE   0xfff0000000ULL
>> +#define NIAGARA_Q_OFFSET    0x10000ULL
>> +#define NIAGARA_OBP_OFFSET  0x80000ULL
>> +#define PROM_SIZE_MAX       (4 * 1024 * 1024)
>> +
>> +/* Niagara hardware initialisation */
>> +static void niagara_init(MachineState *machine)
>> +{
>> +    NiagaraBoardState *s = g_new(NiagaraBoardState, 1);
>> +    DriveInfo *dinfo = drive_get_next(IF_PFLASH);
>
> @dinfo is the first -drive if=pflash (or its sugared form -pflash).
>
>> +    MemoryRegion *sysmem = get_system_memory();
>> +
>> +    /* init CPUs */
>> +    sparc64_cpu_devinit(machine->cpu_model, "Sun UltraSparc T1",
>> +                        NIAGARA_PROM_BASE);
>> +    /* set up devices */
>> +    memory_region_allocate_system_memory(&s->hv_ram, NULL, "sun4v-hv.ram",
>> +                                         NIAGARA_HV_RAM_SIZE);
>> +    memory_region_add_subregion(sysmem, NIAGARA_HV_RAM_BASE, &s->hv_ram);
>> +
>> +    memory_region_allocate_system_memory(&s->partition_ram, NULL,
>> +                                         "sun4v-partition.ram",
>> +                                         machine->ram_size);
>> +    memory_region_add_subregion(sysmem, NIAGARA_PARTITION_RAM_BASE,
>> +                                &s->partition_ram);
>> +
>> +    memory_region_allocate_system_memory(&s->nvram, NULL,
>> +                                         "sun4v.nvram", NIAGARA_NVRAM_SIZE);
>> +    memory_region_add_subregion(sysmem, NIAGARA_NVRAM_BASE, &s->nvram);
>> +    memory_region_allocate_system_memory(&s->md_rom, NULL,
>> +                                         "sun4v-md.rom", 
>> NIAGARA_MD_ROM_SIZE);
>> +    memory_region_add_subregion(sysmem, NIAGARA_MD_ROM_BASE, &s->md_rom);
>> +    memory_region_allocate_system_memory(&s->hv_rom, NULL,
>> +                                         "sun4v-hv.rom", 
>> NIAGARA_HV_ROM_SIZE);
>> +    memory_region_add_subregion(sysmem, NIAGARA_HV_ROM_BASE, &s->hv_rom);
>> +    memory_region_allocate_system_memory(&s->prom, NULL,
>> +                                         "sun4v.prom", PROM_SIZE_MAX);
>> +    memory_region_add_subregion(sysmem, NIAGARA_PROM_BASE, &s->prom);
>> +
>> +    rom_add_file_fixed("nvram1", NIAGARA_NVRAM_BASE, -1);
>> +    rom_add_file_fixed("1up-md.bin", NIAGARA_MD_ROM_BASE, -1);
>> +    rom_add_file_fixed("1up-hv.bin", NIAGARA_HV_ROM_BASE, -1);
>> +
>> +    rom_add_file_fixed("reset.bin", NIAGARA_PROM_BASE, -1);
>> +    rom_add_file_fixed("q.bin", NIAGARA_PROM_BASE + NIAGARA_Q_OFFSET, -1);
>> +    rom_add_file_fixed("openboot.bin", NIAGARA_PROM_BASE + 
>> NIAGARA_OBP_OFFSET,
>> +                       -1);
>> +
>> +    /* the virtual ramdisk is kind of initrd, but it resides
>> +       outside of the partition RAM */
>> +    if (dinfo) {
>> +        BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
>
> This is the block backend created by the first -drive if=pflash.
>
>> +        int size = blk_getlength(blk);
>> +        if (size > 0) {
>> +            memory_region_allocate_system_memory(&s->vdisk_ram, NULL,
>> +                                                 "sun4v_vdisk.ram", size);
>> +            memory_region_add_subregion(get_system_memory(),
>> +                                        NIAGARA_VDISK_BASE, &s->vdisk_ram);
>> +            dinfo->is_default = 1;
>> +            rom_add_file_fixed(blk_bs(blk)->filename, NIAGARA_VDISK_BASE, 
>> -1);
>
> The block backend isn't actually used.  It's merely a carrier for a
> filename.  Naughty.  Also conflicts with Max's work to kill ->filename.
>
> What you create for it isn't actually a pflash device.  Naughty.
>
> Despite rom_add_file_fixed(), it's RAM, not ROM:
>
>     (qemu) info mtree
>     address-space: memory
>       0000000000000000-ffffffffffffffff (prio 0, RW): system
>         0000000000100000-0000000003ffffff (prio 0, RW): sun4v-hv.ram
>         0000000080000000-0000000087ffffff (prio 0, RW): sun4v-partition.ram
>         0000001f10000000-0000001f10000007 (prio 0, RW): serial
>         0000001f11000000-0000001f11001fff (prio 0, RW): sun4v.nvram
>         0000001f12000000-0000001f12001fff (prio 0, RW): sun4v-md.rom
>         0000001f12080000-0000001f12081fff (prio 0, RW): sun4v-hv.rom
>         0000001f40000000-0000001f413fffff (prio 0, RW): sun4v_vdisk.ram
>         0000009800000000-00000098ffffffff (prio 0, RW): empty-slot
>         000000fff0000000-000000fff03fffff (prio 0, RW): sun4v.prom
>         000000fff0c1fff8-000000fff0c1ffff (prio 0, RW): sun4v-rtc
>
>     address-space: I/O
>       0000000000000000-000000000000ffff (prio 0, RW): io
>
>     address-space: cpu-memory
>       0000000000000000-ffffffffffffffff (prio 0, RW): system
>         0000000000100000-0000000003ffffff (prio 0, RW): sun4v-hv.ram
>         0000000080000000-0000000087ffffff (prio 0, RW): sun4v-partition.ram
>         0000001f10000000-0000001f10000007 (prio 0, RW): serial
>         0000001f11000000-0000001f11001fff (prio 0, RW): sun4v.nvram
>         0000001f12000000-0000001f12001fff (prio 0, RW): sun4v-md.rom
>         0000001f12080000-0000001f12081fff (prio 0, RW): sun4v-hv.rom
> --->    0000001f40000000-0000001f413fffff (prio 0, RW): sun4v_vdisk.ram
>         0000009800000000-00000098ffffffff (prio 0, RW): empty-slot
>         000000fff0000000-000000fff03fffff (prio 0, RW): sun4v.prom
>
> Note that sun4v-md.rom and sun4v-hv.rom aren't ROM, either.

Is rom_add_file_fixed supposed to mark the memory region read-only? Or
should it be created read-only before calling rom_add_file_fixed?

> The monkey-patching of dinfo->is_default is to silence the "Orphaned
> drive without device" warning.  Naughty.
>
> This use of -drive if=pflash is highly unorthodox.  To suggest better
> ways, we need to understand the role of "the virtual ramdisk".  Can you
> explain?  How does it work on a physical system?

AFAIK except for simulators it's only used on FPGA boards. It's what
the comment says: a kind of initrd, but it resides outside of the
partition RAM. So, it is a RAM region pre-filled with data in the way
not visible to the guest.

I would have used an -initrd option instead of -pflash, but it
required a -kernel option, and there is no sensible way to use the
-kernel with Solaris.
If QEMU had something like a nvram-disk - it would have worked here the best.

>> +        } else {
>> +            fprintf(stderr, "qemu: could not load ram disk '%s'\n",
>> +                    blk_bs(blk)->filename);
>> +            exit(1);
>> +        }
>> +    }
>> +    serial_mm_init(sysmem, NIAGARA_UART_BASE, 0, NULL, 115200,
>> +                   serial_hds[0], DEVICE_BIG_ENDIAN);
>> +
>> +    empty_slot_init(NIAGARA_IOBBASE, NIAGARA_IOBSIZE);
>> +    sun4v_rtc_init(NIAGARA_RTC_BASE);
>> +}
>> +
>> +static void niagara_class_init(ObjectClass *oc, void *data)
>> +{
>> +    MachineClass *mc = MACHINE_CLASS(oc);
>> +
>> +    mc->desc = "Sun4v platform, Niagara";
>> +    mc->init = niagara_init;
>> +    mc->max_cpus = 1; /* XXX for now */
>> +    mc->default_boot_order = "c";
>> +}
>> +
>> +static const TypeInfo niagara_type = {
>> +    .name = MACHINE_TYPE_NAME("niagara"),
>> +    .parent = TYPE_MACHINE,
>> +    .class_init = niagara_class_init,
>> +};
>> +
>> +static void niagara_register_types(void)
>> +{
>> +    type_register_static(&niagara_type);
>> +}
>> +
>> +type_init(niagara_register_types)
>> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
>> index 232d4a6..d1a6bca 100644
>> --- a/hw/sparc64/sun4u.c
>> +++ b/hw/sparc64/sun4u.c
>> @@ -542,7 +542,6 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>>  enum {
>>      sun4u_id = 0,
>>      sun4v_id = 64,
>> -    niagara_id,
>>  };
>>
>>  static const struct hwdef hwdefs[] = {
>> @@ -560,13 +559,6 @@ static const struct hwdef hwdefs[] = {
>>          .prom_addr = 0x1fff0000000ULL,
>>          .console_serial_base = 0,
>>      },
>> -    /* Sun4v generic Niagara machine */
>> -    {
>> -        .default_cpu_model = "Sun UltraSparc T1",
>> -        .machine_id = niagara_id,
>> -        .prom_addr = 0xfff0000000ULL,
>> -        .console_serial_base = 0xfff0c2c000ULL,
>> -    },
>>  };
>>
>>  /* Sun4u hardware initialisation */
>> @@ -581,12 +573,6 @@ static void sun4v_init(MachineState *machine)
>>      sun4uv_init(get_system_memory(), machine, &hwdefs[1]);
>>  }
>>
>> -/* Niagara hardware initialisation */
>> -static void niagara_init(MachineState *machine)
>> -{
>> -    sun4uv_init(get_system_memory(), machine, &hwdefs[2]);
>> -}
>> -
>>  static void sun4u_class_init(ObjectClass *oc, void *data)
>>  {
>>      MachineClass *mc = MACHINE_CLASS(oc);
>> @@ -620,22 +606,6 @@ static const TypeInfo sun4v_type = {
>>      .class_init = sun4v_class_init,
>>  };
>>
>> -static void niagara_class_init(ObjectClass *oc, void *data)
>> -{
>> -    MachineClass *mc = MACHINE_CLASS(oc);
>> -
>> -    mc->desc = "Sun4v platform, Niagara";
>> -    mc->init = niagara_init;
>> -    mc->max_cpus = 1; /* XXX for now */
>> -    mc->default_boot_order = "c";
>> -}
>> -
>> -static const TypeInfo niagara_type = {
>> -    .name = MACHINE_TYPE_NAME("Niagara"),
>> -    .parent = TYPE_MACHINE,
>> -    .class_init = niagara_class_init,
>> -};
>> -
>>  static void sun4u_register_types(void)
>>  {
>>      type_register_static(&ebus_info);
>> @@ -644,7 +614,6 @@ static void sun4u_register_types(void)
>>
>>      type_register_static(&sun4u_type);
>>      type_register_static(&sun4v_type);
>> -    type_register_static(&niagara_type);
>>  }
>>
>>  type_init(sun4u_register_types)
>> diff --git a/qemu-doc.texi b/qemu-doc.texi
>> index 02cb39d..0b2746f 100644
>> --- a/qemu-doc.texi
>> +++ b/qemu-doc.texi
>> @@ -2138,7 +2138,17 @@ Use the executable @file{qemu-system-sparc64} to 
>> simulate a Sun4u
>>  (UltraSPARC PC-like machine), Sun4v (T1 PC-like machine), or generic
>>  Niagara (T1) machine. The Sun4u emulator is mostly complete, being
>>  able to run Linux, NetBSD and OpenBSD in headless (-nographic) mode. The
>> -Sun4v and Niagara emulators are still a work in progress.
>> +Sun4v emulator is still a work in progress.
>> +
>> +The Niagara T1 emulator makes use of firmware and OS binaries supplied in 
>> the S10image/ directory
>> +of the OpenSPARC T1 project 
>> @url{http://download.oracle.com/technetwork/systems/opensparc/OpenSPARCT1_Arch.1.5.tar.bz2}
>> +and is able to boot the disk.s10hw2 Solaris image.
>> address@hidden
>> +qemu-system-sparc64 -M niagara -L /path-to/S10image/ \
>> +                    -nographic -m 256 \
>> +                    -drive if=pflash,readonly=on,file=/S10image/disk.s10hw2
>> address@hidden example
>> +
>>
>>  QEMU emulates the following peripherals:
>>
>> @@ -2173,7 +2183,7 @@ Set OpenBIOS variables in NVRAM, for example:
>>  qemu-system-sparc64 -prom-env 'auto-boot?=false'
>>  @end example
>>
>> address@hidden -M [sun4u|sun4v|Niagara]
>> address@hidden -M [sun4u|sun4v|niagara]
>>
>>  Set the emulated machine type. The default is sun4u.



-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu