[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] hw/arm: Add SBSA machine type
From: |
Hongbo Zhang |
Subject: |
Re: [Qemu-devel] [PATCH] hw/arm: Add SBSA machine type |
Date: |
Thu, 28 Jun 2018 18:38:32 +0800 |
On 28 June 2018 at 17:31, Laszlo Ersek <address@hidden> wrote:
> On 06/27/18 12:13, Hongbo Zhang wrote:
>> This patch introduces a new Arm machine type 'SBSA' with features:
>> - Based on legacy 'virt' machine type.
>
> My understanding is that this new machine type serves a different use
> case than the "virt" machine type; i.e. it is not primarily meant to run
> on KVM and execute virtualization workloads. Instead, it is meant to
> provide an environment as faithful as possible to physical hardware, for
> supporting firmware and OS development for physical ARM64 machines.
>
> In other words, this machine type is not a "goal" for running production
> workloads (on KVM); instead it is a development and testbed "means",
> where the end-goal is writing OS and firmware code for physical machines
> that conform to the SBSA spec. Thus, the machine type is similar to e.g.
> the "vexpress" machine types, except the emulated hardware platform is
> "SBSA".
>
> Can you please confirm that?
>
Yes, yes, yes.
You describe much better than me, thanks.
> If that's the case, then please remove the word "legacy" from the commit
> message (multiple instances). This machine type does not obsolete or
> replace "virt", for "virt"'s stated use case; this machine type is for
> an independent use case.
>
Good suggestion.
I didn't want make 'virt' obsolete at all, English isn't my mother
language, this should be my misunderstanding of term 'legacy' some
how, I just meant SBSA based on 'virt' :)
> One consequence of this would be that performance isn't as important.
>
> Another consequence is that paravirt devices are less desirable.
>
> I have another question:
>
>> - Newly designed memory map.
>> - EL2 and EL3 are enabled by default.
>> - AHCI controller attached to system bus, and then CDROM and hard disc
>> can be added to it.
>> - EHCI controller attached to system bus, with USB mouse and key board
>> installed by default.
>> - E1000 ethernet card on PCIE bus.
>> - VGA display adaptor on PCIE bus.
>> - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>> - No virtio functions enabled, since this is to emulate real hardware.
>>
>> This is the prototype, more features can be added in futrue.
>>
>> Purpose of this is to have a standard QEMU platform for Arm firmware
>> developement etc. where the legacy machines cannot meets requirements.
>>
>> Arm Trusted Firmware and UEFI porting to this are done seperately.
>>
>> Signed-off-by: Hongbo Zhang <address@hidden>
>> ---
>> hw/arm/virt-acpi-build.c | 59 +++++++++++++-
>> hw/arm/virt.c | 196
>> ++++++++++++++++++++++++++++++++++++++++++++++-
>> include/hw/arm/virt.h | 3 +
>> 3 files changed, 254 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 6ea47e2..60af414 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -84,6 +84,52 @@ static void acpi_dsdt_add_uart(Aml *scope, const
>> MemMapEntry *uart_memmap,
>> aml_append(scope, dev);
>> }
>>
>> +static void acpi_dsdt_add_ahci(Aml *scope, const MemMapEntry *ahci_memmap,
>> + uint32_t ahci_irq)
>> +{
>> + Aml *dev = aml_device("AHC0");
>> + aml_append(dev, aml_name_decl("_HID", aml_string("LNRO001E")));
>> + aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>> + aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
>> +
>> + Aml *crs = aml_resource_template();
>> + aml_append(crs, aml_memory32_fixed(ahci_memmap->base,
>> + ahci_memmap->size, AML_READ_WRITE));
>> + aml_append(crs,
>> + aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
>> + AML_EXCLUSIVE, &ahci_irq, 1));
>> + aml_append(dev, aml_name_decl("_CRS", crs));
>> +
>> + Aml *pkg = aml_package(3);
>> + aml_append(pkg, aml_int(0x1));
>> + aml_append(pkg, aml_int(0x6));
>> + aml_append(pkg, aml_int(0x1));
>> +
>> + /* append the SATA class id */
>> + aml_append(dev, aml_name_decl("_CLS", pkg));
>> +
>> + aml_append(scope, dev);
>> +}
>> +
>> +static void acpi_dsdt_add_ehci(Aml *scope, const MemMapEntry *ehci_memmap,
>> + uint32_t ehci_irq)
>> +{
>> + Aml *dev = aml_device("EHC0");
>> + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0D20")));
>> + aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>> + aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
>> +
>> + Aml *crs = aml_resource_template();
>> + aml_append(crs, aml_memory32_fixed(ehci_memmap->base,
>> + ehci_memmap->size, AML_READ_WRITE));
>> + aml_append(crs,
>> + aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
>> + AML_EXCLUSIVE, &ehci_irq, 1));
>> + aml_append(dev, aml_name_decl("_CRS", crs));
>> +
>> + aml_append(scope, dev);
>> +}
>> +
>> static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry
>> *fw_cfg_memmap)
>> {
>> Aml *dev = aml_device("FWCF");
>> @@ -768,14 +814,23 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>> VirtMachineState *vms)
>> (irqmap[VIRT_UART] + ARM_SPI_BASE));
>> acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
>> acpi_dsdt_add_fw_cfg(scope, &memmap[VIRT_FW_CFG]);
>> - acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
>> - (irqmap[VIRT_MMIO] + ARM_SPI_BASE),
>> NUM_VIRTIO_TRANSPORTS);
>> + if (!vms->sbsa) {
>> + acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
>> + (irqmap[VIRT_MMIO] + ARM_SPI_BASE),
>> NUM_VIRTIO_TRANSPORTS);
>> + }
>> acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE),
>> vms->highmem, vms->highmem_ecam);
>> acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
>> (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
>> acpi_dsdt_add_power_button(scope);
>>
>> + if (vms->sbsa) {
>> + acpi_dsdt_add_ahci(scope, &memmap[VIRT_AHCI],
>> + (irqmap[VIRT_AHCI] + ARM_SPI_BASE));
>> + acpi_dsdt_add_ehci(scope, &memmap[VIRT_EHCI],
>> + (irqmap[VIRT_EHCI] + ARM_SPI_BASE));
>> + }
>> +
>> aml_append(dsdt, scope);
>>
>> /* copy AML table into ACPI tables blob and patch header there */
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 742f68a..491f23b 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -59,6 +59,10 @@
>> #include "qapi/visitor.h"
>> #include "standard-headers/linux/input.h"
>> #include "hw/arm/smmuv3.h"
>> +#include "hw/ide/internal.h"
>> +#include "hw/ide/ahci_internal.h"
>> +#include "hw/usb.h"
>> +#include "qemu/cutils.h"
>>
>> #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
>> static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
>> @@ -94,6 +98,8 @@
>>
>> #define PLATFORM_BUS_NUM_IRQS 64
>>
>> +#define SATA_NUM_PORTS 6
>> +
>> /* RAM limit in GB. Since VIRT_MEM starts at the 1GB mark, this means
>> * RAM can go up to the 256GB mark, leaving 256GB of the physical
>> * address space unallocated and free for future use between 256G and 512G.
>> @@ -168,6 +174,47 @@ static const int a15irqmap[] = {
>> [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
>> };
>>
>> +static const MemMapEntry sbsa_memmap[] = {
>> + /* Space up to 0x8000000 is reserved for a boot ROM */
>> + [VIRT_FLASH] = { 0, 0x08000000 },
>> + [VIRT_CPUPERIPHS] = { 0x08000000, 0x00020000 },
>> + /* GIC distributor and CPU interfaces sit inside the CPU peripheral
>> space */
>> + [VIRT_GIC_DIST] = { 0x08000000, 0x00010000 },
>> + [VIRT_GIC_CPU] = { 0x08010000, 0x00010000 },
>> + [VIRT_GIC_V2M] = { 0x08020000, 0x00001000 },
>> + /* The space in between here is reserved for GICv3 CPU/vCPU/HYP */
>> + [VIRT_GIC_ITS] = { 0x08080000, 0x00020000 },
>> + /* This redistributor space allows up to 2*64kB*123 CPUs */
>> + [VIRT_GIC_REDIST] = { 0x080A0000, 0x00F60000 },
>> + [VIRT_UART] = { 0x09000000, 0x00001000 },
>> + [VIRT_RTC] = { 0x09010000, 0x00001000 },
>> + [VIRT_FW_CFG] = { 0x09020000, 0x00000018 },
>
> It's hard for any device to be *more* paravirtual than fw_cfg is --
> given that (to my understanding) paravirtualization is an explicit
> non-goal for this machine type, what do you need fw_cfg for? Is it an
> oversight in the memory map, or is it an exception -- a limited paravirt
> info channel -- because you need *some* way for passing information from
> QEMU to a small subset of guest code?
>
Well, when design discussed, fw_cfg wasn't mentioned at all, so I
didn't pay much attention to it.
Will double check, I think this should be removed.
Thanks for review.
> Thanks
> Laszlo
>
>> + [VIRT_GPIO] = { 0x09030000, 0x00001000 },
>> + [VIRT_SECURE_UART] = { 0x09040000, 0x00001000 },
>> + [VIRT_AHCI] = { 0x09050000, 0x00010000 },
>> + [VIRT_EHCI] = { 0x09060000, 0x00010000 },
>> + [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 },
>> + [VIRT_SECURE_MEM] = { 0x0e000000, 0x01000000 },
>> + [VIRT_PCIE_MMIO] = { 0x10000000, 0x7fff0000 },
>> + [VIRT_PCIE_PIO] = { 0x8fff0000, 0x00010000 },
>> + [VIRT_PCIE_ECAM] = { 0x90000000, 0x10000000 },
>> + /* Second PCIe window, 508GB wide at the 4GB boundary */
>> + [VIRT_PCIE_MMIO_HIGH] = { 0x100000000ULL, 0x7F00000000ULL },
>> + [VIRT_MEM] = { 0x8000000000ULL, RAMLIMIT_BYTES },
>> +};
>> +
>> +static const int sbsa_irqmap[] = {
>> + [VIRT_UART] = 1,
>> + [VIRT_RTC] = 2,
>> + [VIRT_PCIE] = 3, /* ... to 6 */
>> + [VIRT_GPIO] = 7,
>> + [VIRT_SECURE_UART] = 8,
>> + [VIRT_AHCI] = 9,
>> + [VIRT_EHCI] = 10,
>> + [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
>> + [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
>> +};
>> +
>> static const char *valid_cpus[] = {
>> ARM_CPU_TYPE_NAME("cortex-a15"),
>> ARM_CPU_TYPE_NAME("cortex-a53"),
>> @@ -696,6 +743,72 @@ static void create_rtc(const VirtMachineState *vms,
>> qemu_irq *pic)
>> g_free(nodename);
>> }
>>
>> +static void create_ahci(const VirtMachineState *vms, qemu_irq *pic)
>> +{
>> + char *nodename;
>> + hwaddr base = vms->memmap[VIRT_AHCI].base;
>> + hwaddr size = vms->memmap[VIRT_AHCI].size;
>> + int irq = vms->irqmap[VIRT_AHCI];
>> + const char compat[] = "qemu,mach-virt-ahci\0generic-ahci";
>> + DeviceState *dev;
>> + DriveInfo *hd[SATA_NUM_PORTS];
>> + SysbusAHCIState *sysahci;
>> + AHCIState *ahci;
>> + int i;
>> +
>> + dev = qdev_create(NULL, "sysbus-ahci");
>> + qdev_prop_set_uint32(dev, "num-ports", SATA_NUM_PORTS);
>> + qdev_init_nofail(dev);
>> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
>> + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]);
>> +
>> + sysahci = SYSBUS_AHCI(dev);
>> + ahci = &sysahci->ahci;
>> + ide_drive_get(hd, ARRAY_SIZE(hd));
>> + for (i = 0; i < ahci->ports; i++) {
>> + if (hd[i] == NULL) {
>> + continue;
>> + }
>> + ide_create_drive(&ahci->dev[i].port, 0, hd[i]);
>> + }
>> +
>> + nodename = g_strdup_printf("/address@hidden" PRIx64, base);
>> + qemu_fdt_add_subnode(vms->fdt, nodename);
>> + qemu_fdt_setprop(vms->fdt, nodename, "compatible", compat,
>> sizeof(compat));
>> + qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
>> + 2, base, 2, size);
>> + qemu_fdt_setprop_cells(vms->fdt, nodename, "interrupts",
>> + GIC_FDT_IRQ_TYPE_SPI, irq,
>> + GIC_FDT_IRQ_FLAGS_LEVEL_HI);
>> + g_free(nodename);
>> +}
>> +
>> +static void create_ehci(const VirtMachineState *vms, qemu_irq *pic)
>> +{
>> + char *nodename;
>> + hwaddr base = vms->memmap[VIRT_EHCI].base;
>> + hwaddr size = vms->memmap[VIRT_EHCI].size;
>> + int irq = vms->irqmap[VIRT_EHCI];
>> + const char compat[] = "qemu,mach-virt-ehci\0generic-ehci";
>> + USBBus *usb_bus;
>> +
>> + sysbus_create_simple("exynos4210-ehci-usb", base, pic[irq]);
>> +
>> + usb_bus = usb_bus_find(-1);
>> + usb_create_simple(usb_bus, "usb-kbd");
>> + usb_create_simple(usb_bus, "usb-mouse");
>> +
>> + nodename = g_strdup_printf("/address@hidden" PRIx64, base);
>> + qemu_fdt_add_subnode(vms->fdt, nodename);
>> + qemu_fdt_setprop(vms->fdt, nodename, "compatible", compat,
>> sizeof(compat));
>> + qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
>> + 2, base, 2, size);
>> + qemu_fdt_setprop_cells(vms->fdt, nodename, "interrupts",
>> + GIC_FDT_IRQ_TYPE_SPI, irq,
>> + GIC_FDT_IRQ_FLAGS_LEVEL_HI);
>> + g_free(nodename);
>> +}
>> +
>> static DeviceState *gpio_key_dev;
>> static void virt_powerdown_req(Notifier *n, void *opaque)
>> {
>> @@ -1107,13 +1220,21 @@ static void create_pcie(VirtMachineState *vms,
>> qemu_irq *pic)
>> NICInfo *nd = &nd_table[i];
>>
>> if (!nd->model) {
>> - nd->model = g_strdup("virtio");
>> + if (vms->sbsa) {
>> + nd->model = g_strdup("e1000");
>> + } else {
>> + nd->model = g_strdup("virtio");
>> + }
>> }
>>
>> pci_nic_init_nofail(nd, pci->bus, nd->model, NULL);
>> }
>> }
>>
>> + if (vms->sbsa) {
>> + pci_create_simple(pci->bus, -1, "VGA");
>> + }
>> +
>> nodename = g_strdup_printf("/address@hidden" PRIx64, base);
>> qemu_fdt_add_subnode(vms->fdt, nodename);
>> qemu_fdt_setprop_string(vms->fdt, nodename,
>> @@ -1502,11 +1623,18 @@ static void machvirt_init(MachineState *machine)
>>
>> create_gpio(vms, pic);
>>
>> + if (vms->sbsa) {
>> + create_ahci(vms, pic);
>> + create_ehci(vms, pic);
>> + }
>> +
>> /* Create mmio transports, so the user can create virtio backends
>> * (which will be automatically plugged in to the transports). If
>> * no backend is created the transport will just sit harmlessly idle.
>> */
>> - create_virtio_devices(vms, pic);
>> + if (!vms->sbsa) {
>> + create_virtio_devices(vms, pic);
>> + }
>>
>> vms->fw_cfg = create_fw_cfg(vms, &address_space_memory);
>> rom_set_fw(vms->fw_cfg);
>> @@ -1818,6 +1946,7 @@ static void virt_3_0_instance_init(Object *obj)
>>
>> vms->memmap = a15memmap;
>> vms->irqmap = a15irqmap;
>> + vms->sbsa = false;
>> }
>>
>> static void virt_machine_3_0_options(MachineClass *mc)
>> @@ -1950,3 +2079,66 @@ static void virt_machine_2_6_options(MachineClass *mc)
>> vmc->no_pmu = true;
>> }
>> DEFINE_VIRT_MACHINE(2, 6)
>> +
>> +static void sbsa_instance_init(Object *obj)
>> +{
>> + VirtMachineState *vms = VIRT_MACHINE(obj);
>> +
>> + vms->secure = true;
>> + object_property_add_bool(obj, "secure", virt_get_secure,
>> + virt_set_secure, NULL);
>> + object_property_set_description(obj, "secure",
>> + "Set on/off to enable/disable the ARM "
>> + "Security Extensions (TrustZone)",
>> + NULL);
>> +
>> + vms->virt = true;
>> + object_property_add_bool(obj, "virtualization", virt_get_virt,
>> + virt_set_virt, NULL);
>> + object_property_set_description(obj, "virtualization",
>> + "Set on/off to enable/disable emulating
>> a "
>> + "guest CPU which implements the ARM "
>> + "Virtualization Extensions",
>> + NULL);
>> +
>> + vms->highmem = true;
>> +
>> + /* This can be changed to GICv3 when firmware is ready*/
>> + vms->gic_version = 2;
>> + object_property_add_str(obj, "gic-version", virt_get_gic_version,
>> + virt_set_gic_version, NULL);
>> + object_property_set_description(obj, "gic-version",
>> + "Set GIC version. "
>> + "Valid values are 2, 3, host, max",
>> NULL);
>> + vms->its = true;
>> +
>> + vms->memmap = sbsa_memmap;
>> + vms->irqmap = sbsa_irqmap;
>> + vms->sbsa = true;
>> +}
>> +
>> +static void sbsa_class_init(ObjectClass *oc, void *data)
>> +{
>> + MachineClass *mc = MACHINE_CLASS(oc);
>> +
>> + mc->max_cpus = 246;
>> + mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a57");
>> + mc->block_default_type = IF_IDE;
>> + mc->default_ram_size = 1 * G_BYTE;
>> + mc->default_cpus = 4;
>> + mc->desc = "QEMU 'SBSA' ARM Virtual Machine";
>> +}
>> +
>> +static const TypeInfo sbsa_info = {
>> + .name = MACHINE_TYPE_NAME("sbsa"),
>> + .parent = TYPE_VIRT_MACHINE,
>> + .instance_init = sbsa_instance_init,
>> + .class_init = sbsa_class_init,
>> +};
>> +
>> +static void sbsa_machine_init(void)
>> +{
>> + type_register_static(&sbsa_info);
>> +}
>> +
>> +type_init(sbsa_machine_init);
>> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
>> index 9a870cc..619473e 100644
>> --- a/include/hw/arm/virt.h
>> +++ b/include/hw/arm/virt.h
>> @@ -78,6 +78,8 @@ enum {
>> VIRT_GPIO,
>> VIRT_SECURE_UART,
>> VIRT_SECURE_MEM,
>> + VIRT_AHCI,
>> + VIRT_EHCI,
>> };
>>
>> typedef enum VirtIOMMUType {
>> @@ -124,6 +126,7 @@ typedef struct {
>> uint32_t msi_phandle;
>> uint32_t iommu_phandle;
>> int psci_conduit;
>> + bool sbsa;
>> } VirtMachineState;
>>
>> #define VIRT_ECAM_ID(high) (high ? VIRT_PCIE_ECAM_HIGH : VIRT_PCIE_ECAM)
>>
>
Re: [Qemu-devel] [PATCH] hw/arm: Add SBSA machine type, Daniel P . Berrangé, 2018/06/28
Re: [Qemu-devel] [PATCH] hw/arm: Add SBSA machine type, Igor Mammedov, 2018/06/27
Re: [Qemu-devel] [PATCH] hw/arm: Add SBSA machine type, Laszlo Ersek, 2018/06/28
- Re: [Qemu-devel] [PATCH] hw/arm: Add SBSA machine type,
Hongbo Zhang <=