September 2012 - SeaBIOS - mail.coreboot.org

[PATCH] Support for booting from QEMU emulated LSI Logic LSI53C1030, SAS1068, SAS1068e.
by Don Slutz Sept. 29, 2012

Sept. 29, 2012

Also known as Fusion MPT disk. This needs the patch (for the QEMU emulation): http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg01608.html Signed-off-by: Don Slutz <Don(a)CloudSwitch.com> --- Makefile | 2 +- src/Kconfig | 6 + src/block.c | 1 + src/blockcmd.c | 3 + src/disk.h | 1 + src/mpt-scsi.c | 340 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/mpt-scsi.h | 8 ++ src/post.c | 2 + 8 files changed, 362 insertions(+), 1 deletions(-) create mode 100644 src/mpt-scsi.c create mode 100644 src/mpt-scsi.h diff --git a/Makefile b/Makefile index 5486f88..d42d998 100644 --- a/Makefile +++ b/Makefile @@ -13,7 +13,7 @@ SRCBOTH=misc.c stacks.c pmm.c output.c util.c block.c floppy.c ata.c mouse.c \ pnpbios.c pirtable.c vgahooks.c ramdisk.c pcibios.c blockcmd.c \ usb.c usb-uhci.c usb-ohci.c usb-ehci.c usb-hid.c usb-msc.c \ virtio-ring.c virtio-pci.c virtio-blk.c virtio-scsi.c apm.c ahci.c \ - usb-uas.c lsi-scsi.c esp-scsi.c + usb-uas.c lsi-scsi.c esp-scsi.c mpt-scsi.c SRC16=$(SRCBOTH) system.c disk.c font.c SRC32FLAT=$(SRCBOTH) post.c shadow.c memmap.c coreboot.c boot.c \ acpi.c smm.c mptable.c smbios.c pciinit.c optionroms.c mtrr.c \ diff --git a/src/Kconfig b/src/Kconfig index b5dd63b..4243a05 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -131,6 +131,12 @@ menu "Hardware support" default y help Support boot from qemu-emulated lsi53c895a scsi storage. + config MPT_SCSI + depends on DRIVES && !COREBOOT + bool "lsi50c1030 scsi & lsi1068 sas controllers" + default y + help + Support boot from qemu-emulated lsi53c1030 scsi and lsi1068 sas storage. config FLOPPY depends on DRIVES bool "Floppy controller" diff --git a/src/block.c b/src/block.c index 243428e..4985bc1 100644 --- a/src/block.c +++ b/src/block.c @@ -336,6 +336,7 @@ process_op(struct disk_op_s *op) case DTYPE_VIRTIO_SCSI: case DTYPE_LSI_SCSI: case DTYPE_ESP_SCSI: + case DTYPE_MPT_SCSI: return process_scsi_op(op); default: op->count = 0; diff --git a/src/blockcmd.c b/src/blockcmd.c index 77c690f..233a501 100644 --- a/src/blockcmd.c +++ b/src/blockcmd.c @@ -17,6 +17,7 @@ #include "virtio-scsi.h" // virtio_scsi_cmd_data #include "lsi-scsi.h" // lsi_scsi_cmd_data #include "esp-scsi.h" // esp_scsi_cmd_data +#include "mpt-scsi.h" // esp_scsi_cmd_data #include "boot.h" // boot_add_hd // Route command to low-level handler. @@ -39,6 +40,8 @@ cdb_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) return lsi_scsi_cmd_data(op, cdbcmd, blocksize); case DTYPE_ESP_SCSI: return esp_scsi_cmd_data(op, cdbcmd, blocksize); + case DTYPE_MPT_SCSI: + return mpt_scsi_cmd_data(op, cdbcmd, blocksize); default: op->count = 0; return DISK_RET_EPARAM; diff --git a/src/disk.h b/src/disk.h index 3d07372..f8d37cf 100644 --- a/src/disk.h +++ b/src/disk.h @@ -235,6 +235,7 @@ struct drive_s { #define DTYPE_UAS 0x0b #define DTYPE_LSI_SCSI 0x0c #define DTYPE_ESP_SCSI 0x0d +#define DTYPE_MPT_SCSI 0x0e #define MAXDESCSIZE 80 diff --git a/src/mpt-scsi.c b/src/mpt-scsi.c new file mode 100644 index 0000000..da351e4 --- /dev/null +++ b/src/mpt-scsi.c @@ -0,0 +1,340 @@ +// (qemu-emulated) Fusion MPT boot support. +// +// Copyright (C) 2012 Red Hat Inc. +// +// Authors: +// Gerd Hoffmann <kraxel(a)redhat.com> +// +// based on virtio-scsi.c which is written by: +// Paolo Bonzini <pbonzini(a)redhat.com> +// +// This file may be distributed under the terms of the GNU LGPLv3 license. + +#include "util.h" // dprintf +#include "pci.h" // foreachpci +#include "config.h" // CONFIG_* +#include "biosvar.h" // GET_GLOBAL +#include "pci_ids.h" // PCI_DEVICE_ID_VIRTIO_BLK +#include "pci_regs.h" // PCI_VENDOR_ID +#include "boot.h" // bootprio_find_scsi_device +#include "blockcmd.h" // scsi_init_drive +#include "disk.h" + +#define MPT_REG_DOORBELL 0x00 +#define MPT_REG_WRITE_SEQ 0x04 +#define MPT_REG_HOST_DIAG 0x08 +#define MPT_REG_TEST 0x0c +#define MPT_REG_DIAG_DATA 0x10 +#define MPT_REG_DIAG_ADDR 0x14 +#define MPT_REG_ISTATUS 0x30 +#define MPT_REG_IMASK 0x34 +#define MPT_REG_REQ_Q 0x40 +#define MPT_REG_REP_Q 0x44 + +#define MPT_DOORBELL_MSG_RESET 0x40 +#define MPT_DOORBELL_HANDSHAKE 0x42 +#define MPT_DOORBELL_REG_FUNC(f, s) ((((f)&0xff)<<24)|(((s)&0xff)<<16)) + +#define MPT_IMASK_DOORBELL 0x01 +#define MPT_IMASK_REPLY 0x08 + +struct mpt_lun_s { + struct drive_s drive; + struct pci_device *pci; + u32 iobase; + u8 target; + u8 lun; +}; + +u8 reply_msg[128] VAR16VISIBLE; + +#define MPT_MESSAGE_HDR_FUNCTION_SCSI_IO_REQUEST (0x00) +#define MPT_MESSAGE_HDR_FUNCTION_IOC_INIT (0x02) + +static struct MptIOCInitRequest +{ + u8 WhoInit; /* Which system sent this init request. */ + u8 Reserved1; /* Reserved */ + u8 ChainOffset; /* Chain offset in the SG list. */ + u8 Function; /* Function to execute. */ + u8 Flags; /* Flags */ + u8 MaxDevices; /* Max devices the driver can handle. */ + u8 MaxBuses; /* Max buses the driver can handle. */ + u8 MessageFlags; /* Message flags. */ + u32 MessageContext; /* Message context ID. */ + u16 ReplyFrameSize; /* Reply frame size. */ + u16 Reserved2; /* Reserved */ + u32 HostMfaHighAddr; /* Upper 32bit of the message frames. */ + u32 SenseBufferHighAddr; /* Upper 32bit of the sense buffer. */ +} MptIOCInitRequest = { + 2, + 0, + 0, + MPT_MESSAGE_HDR_FUNCTION_IOC_INIT, + 0, + 8, + 1, + 0, + 0, + sizeof(reply_msg), + 0, + 0, + 0 +}; + +struct MptIOCInitReply { + u8 WhoInit; /* Which subsystem sent this init request. */ + u8 Reserved1; /* Reserved */ + u8 MessageLength; /* Message length */ + u8 Function; /* Function. */ + u8 Flags; /* Flags */ + u8 MaxDevices; /* Maximum number of devices the driver can handle. */ + u8 MaxBuses; /* Maximum number of busses the driver can handle. */ + u8 MessageFlags; /* Message flags. */ + u32 MessageContext; /* Message context ID */ + u16 Reserved2; /* Reserved */ + u16 IOCStatus; /* IO controller status. */ + u32 IOCLogInfo; /* IO controller log information. */ +}; + +#pragma pack(1) +typedef struct MptSCSIIORequest { + u8 TargetID; /* Target ID */ + u8 Bus; /* Bus number */ + u8 ChainOffset; /* Chain offset */ + u8 Function; /* Function number. */ + u8 CDBLength; /* CDB length. */ + u8 SenseBufferLength; /* Sense buffer length. */ + u8 Reserved; /* Reserved */ + u8 MessageFlags; /* Message flags. */ + u32 MessageContext; /* Message context ID. */ + u8 LUN[8]; /* LUN */ + u32 Control; /* Control values. */ + u8 CDB[16]; /* The CDB. */ + u32 DataLength; /* Data length. */ + u32 SenseBufferLowAddr; /* Sense buffer low 32bit address. */ +} MptSCSIIORequest_t; + +#define MPT_CONTEXT_MAGIC 0xaaaa5555 + +typedef struct MptSGEntrySimple32 { + unsigned u24Length:24; + unsigned fEndOfList:1; + unsigned f64BitAddress:1; + unsigned fBufferContainsData:1; + unsigned fLocalAddress:1; + unsigned u2ElementType:2; + unsigned fEndOfBuffer:1; + unsigned fLastElement:1; + u32 DataBufferAddressLow; +} MptSGEntrySimple32_t; +#pragma pack(0) + +static int +mpt_scsi_cmd(struct mpt_lun_s *llun, struct disk_op_s *op, + void *cdbcmd, u16 target, u16 lun, u16 blocksize) +{ + u32 iobase = GET_GLOBAL(llun->iobase); + int i; + u8 sense_buf[32]; + int retry = 0; + + struct scsi_req { + MptSCSIIORequest_t scsi_io; + MptSGEntrySimple32_t sge; + } req = { + { + target, + 0, + 0, + MPT_MESSAGE_HDR_FUNCTION_SCSI_IO_REQUEST, + 16, + 32, + 0, + 0, + MPT_CONTEXT_MAGIC, + {0, lun, 0, 0, 0, 0, 0, 0}, + 0, + {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, + (op->count * blocksize), + (u32)MAKE_FLATPTR(GET_SEG(SS), &sense_buf[0]) + }, + { + op->count * blocksize, + 1, + 0, + 0, + 0, + 1, + 1, + 1, + (u32)op->buf_fl + } + }; + + if (lun != 0) { + return DISK_RET_ENOTREADY; + } + + if (blocksize == 0) { + req.scsi_io.Control = 0; + } else if (cdb_is_read(cdbcmd, blocksize)) { + req.scsi_io.Control = 2 << 24; + } else { + req.scsi_io.Control = 1 << 24; + req.sge.fBufferContainsData = 1; + } + + u8 *cdb = (u8 *)cdbcmd; + for (i = 0; i < 16; i++) { + req.scsi_io.CDB[i] = cdb[i]; + } + +try_again: + outl((u32)MAKE_FLATPTR(GET_SEG(SS), &req), iobase + MPT_REG_REQ_Q); + + for (;;) { + u32 istatus = inl(iobase + MPT_REG_ISTATUS); + u32 resp; + if (istatus & MPT_IMASK_REPLY) { + do { + resp = inl(iobase + MPT_REG_REP_Q); + if (resp == MPT_CONTEXT_MAGIC) { + return DISK_RET_SUCCESS; + } else if ((resp << 1) == (u32)&reply_msg[0]) { + if (retry == 0) { + retry = 1; + goto try_again; + } + return DISK_RET_EBADTRACK; + } + } while (resp != 0xffffffff); + } + usleep(50); + } +} + +int +mpt_scsi_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) +{ + if (!CONFIG_MPT_SCSI) { + return DISK_RET_EBADTRACK; + } + + struct mpt_lun_s *llun = + container_of(op->drive_g, struct mpt_lun_s, drive); + + return mpt_scsi_cmd(llun, op, cdbcmd, + GET_GLOBAL(llun->target), GET_GLOBAL(llun->lun), + blocksize); +} + +static int +mpt_scsi_add_lun(struct pci_device *pci, u32 iobase, u8 target, u8 lun) +{ + struct mpt_lun_s *llun = malloc_fseg(sizeof(*llun)); + if (!llun) { + warn_noalloc(); + return -1; + } + memset(llun, 0, sizeof(*llun)); + llun->drive.type = DTYPE_MPT_SCSI; + llun->drive.cntl_id = pci->bdf; + llun->pci = pci; + llun->target = target; + llun->lun = lun; + llun->iobase = iobase; + + char *name = znprintf(16, "mpt %02x:%02x.%x %d:%d", + pci_bdf_to_bus(pci->bdf), pci_bdf_to_dev(pci->bdf), + pci_bdf_to_fn(pci->bdf), target, lun); + int prio = bootprio_find_scsi_device(pci, target, lun); + int ret = scsi_init_drive(&llun->drive, name, prio); + free(name); + if (ret) { + goto fail; + } + return 0; + +fail: + free(llun); + return -1; +} + +static void +mpt_scsi_scan_target(struct pci_device *pci, u32 iobase, u8 target) +{ + /* TODO: send REPORT LUNS. For now, only LUN 0 is recognized. */ + mpt_scsi_add_lun(pci, iobase, target, 0); +} + +static void +init_mpt_scsi(struct pci_device *pci, const char *dev_name) +{ + u32 *msg_out_p; + u16 *msg_in_p; + u16 bdf = pci->bdf; + u32 iobase = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0) + & PCI_BASE_ADDRESS_IO_MASK; + struct MptIOCInitReply MptIOCInitReply; + + dprintf(1, "found %s at %02x:%02x.%x, io @ %x\n", dev_name, + pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), + pci_bdf_to_fn(bdf), iobase); + + // reset + outl(MPT_DOORBELL_REG_FUNC(MPT_DOORBELL_MSG_RESET, 0), + iobase + MPT_REG_DOORBELL); + outl(MPT_IMASK_DOORBELL|MPT_IMASK_REPLY , iobase + MPT_REG_IMASK); + outl(0, iobase + MPT_REG_ISTATUS); + + outl(MPT_DOORBELL_REG_FUNC(MPT_DOORBELL_HANDSHAKE, + sizeof(MptIOCInitRequest)/sizeof(u32)), iobase + MPT_REG_DOORBELL); + msg_out_p = (u32 *)&MptIOCInitRequest; + int i; + for (i = 0; i < sizeof(MptIOCInitRequest)/sizeof(u32); i++) { + outl(*msg_out_p++, iobase + MPT_REG_DOORBELL); + } + + msg_in_p = (u16 *)&MptIOCInitReply; + for (i = 0; i < sizeof(MptIOCInitReply)/sizeof(u16); i++) + /* Lower 16 bits has the message */ + *msg_in_p++ = (u16)inl(iobase + MPT_REG_DOORBELL); + /* TODO: Should make sure reply message looks good */ + + outl(0, iobase + MPT_REG_ISTATUS); + + outl((u32)&reply_msg[0], iobase + MPT_REG_REP_Q); + + for (i = 0; i < 7; i++) { + mpt_scsi_scan_target(pci, iobase, i); + } + + return; +} + +void +mpt_scsi_setup(void) +{ + ASSERT32FLAT(); + if (!CONFIG_MPT_SCSI) { + return; + } + + dprintf(3, "init MPT\n"); + + struct pci_device *pci; + foreachpci(pci) { + if (pci->vendor == PCI_VENDOR_ID_LSI_LOGIC) { + if (pci->device == PCI_DEVICE_ID_LSI_53C1030) { + init_mpt_scsi(pci, "lsi53c1030"); + } + if (pci->device == PCI_DEVICE_ID_LSI_SAS1068) { + init_mpt_scsi(pci, "sas1068"); + } + if (pci->device == PCI_DEVICE_ID_LSI_SAS1068E) { + init_mpt_scsi(pci, "sas1068e"); + } + } + } +} diff --git a/src/mpt-scsi.h b/src/mpt-scsi.h new file mode 100644 index 0000000..938ac2b --- /dev/null +++ b/src/mpt-scsi.h @@ -0,0 +1,8 @@ +#ifndef __MPT_SCSI_H +#define __MPT_SCSI_H + +struct disk_op_s; +int mpt_scsi_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize); +void mpt_scsi_setup(void); + +#endif /* __MPT_SCSI_H */ diff --git a/src/post.c b/src/post.c index 924b311..d56b6a6 100644 --- a/src/post.c +++ b/src/post.c @@ -29,6 +29,7 @@ #include "virtio-scsi.h" // virtio_scsi_setup #include "lsi-scsi.h" // lsi_scsi_setup #include "esp-scsi.h" // esp_scsi_setup +#include "mpt-scsi.h" // mpt_scsi_setup /**************************************************************** @@ -198,6 +199,7 @@ init_hw(void) virtio_scsi_setup(); lsi_scsi_setup(); esp_scsi_setup(); + mpt_scsi_setup(); } // Begin the boot process by invoking an int0x19 in 16bit mode. -- 1.7.1

5 6

Re: [SeaBIOS] GeodeVGA global variable access
by Kevin O'Connor Sept. 29, 2012

Sept. 29, 2012

On Fri, Sep 28, 2012 at 05:26:45PM +0200, Christian Gmeiner wrote: > ping I forgot it was still pending - I just committed it. -Kevin

1 0

Re: [SeaBIOS] [RFC PATCH v3 08/19] pc: calculate dimm physical addresses and adjust memory map
by Blue Swirl Sept. 29, 2012

Sept. 29, 2012

On Mon, Sep 24, 2012 at 3:27 PM, Vasilis Liaskovitis <vasilis.liaskovitis(a)profitbricks.com> wrote: > On Sat, Sep 22, 2012 at 02:15:28PM +0000, Blue Swirl wrote: >> > + >> > +/* Function to configure memory offsets of hotpluggable dimms */ >> > + >> > +target_phys_addr_t pc_set_hp_memory_offset(uint64_t size) >> > +{ >> > + target_phys_addr_t ret; >> > + >> > + /* on first call, initialize ram_hp_offset */ >> > + if (!ram_hp_offset) { >> > + if (ram_size >= PCI_HOLE_START ) { >> > + ram_hp_offset = 0x100000000LL + (ram_size - PCI_HOLE_START); >> > + } else { >> > + ram_hp_offset = ram_size; >> > + } >> > + } >> > + >> > + if (ram_hp_offset >= 0x100000000LL) { >> > + ret = ram_hp_offset; >> > + above_4g_hp_mem_size += size; >> > + ram_hp_offset += size; >> > + } >> > + /* if dimm fits before pci hole, append it normally */ >> > + else if (ram_hp_offset + size <= PCI_HOLE_START) { >> >> } else if ... >> >> > + ret = ram_hp_offset; >> > + below_4g_hp_mem_size += size; >> > + ram_hp_offset += size; >> > + } >> > + /* otherwise place it above 4GB */ >> > + else { >> >> } else { >> >> > + ret = 0x100000000LL; >> > + above_4g_hp_mem_size += size; >> > + ram_hp_offset = 0x100000000LL + size; >> > + } >> > + >> > + return ret; >> > +} >> >> But the function and use of lots of global variables is ugly. The dimm >> devices should be just created in piix_pci.c (i440fx) directly with >> correct offsets and sizes, so all below_4g_mem_size etc. calculations >> should be moved there. That would implement the PMC part of i440fx. >> >> For ISA PC, probably the board should create the DIMMs since there may >> not be a memory controller. The >4G logic does not make sense there >> anyway. > > What about moving the implementation to pc_piix.c? > Initial RAM and pci windows are already calculated in pc_init1, and then passed > to i440fx_init. The memory bus could be attached to i440fx for pci-enabled pc > and to isabus-bridge for isa-pc (isa-pc not tested yet). I'd move the calculations also to i440fx, it (PMC) determines the memory configuration on real HW too. > > Something like the following: > > --- > hw/pc.h | 1 + > hw/pc_piix.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++------ > 2 files changed, 52 insertions(+), 6 deletions(-) > > diff --git a/hw/pc.h b/hw/pc.h > index e4db071..d6cc43b 100644 > --- a/hw/pc.h > +++ b/hw/pc.h > @@ -10,6 +10,7 @@ > #include "memory.h" > #include "ioapic.h" > > +#define PCI_HOLE_START 0xe0000000 > /* PC-style peripherals (also used by other machines). */ > > /* serial.c */ > diff --git a/hw/pc_piix.c b/hw/pc_piix.c > index 88ff041..17db95a 100644 > --- a/hw/pc_piix.c > +++ b/hw/pc_piix.c > @@ -43,6 +43,7 @@ > #include "xen.h" > #include "memory.h" > #include "exec-memory.h" > +#include "dimm.h" > #ifdef CONFIG_XEN > # include <xen/hvm/hvm_info_table.h> > #endif > @@ -52,6 +53,8 @@ > static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 }; > static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 }; > static const int ide_irq[MAX_IDE_BUS] = { 14, 15 }; > +static ram_addr_t below_4g_hp_mem_size = 0; > +static ram_addr_t above_4g_hp_mem_size = 0; > > static void kvm_piix3_setup_irq_routing(bool pci_enabled) > { > @@ -117,6 +120,41 @@ static void ioapic_init(GSIState *gsi_state) > } > } > > +static target_phys_addr_t pc_set_hp_memory_offset(uint64_t size) > +{ > + target_phys_addr_t ret; > + static ram_addr_t ram_hp_offset = 0; > + > + /* on first call, initialize ram_hp_offset */ > + if (!ram_hp_offset) { > + if (ram_size >= PCI_HOLE_START ) { > + ram_hp_offset = 0x100000000LL + (ram_size - PCI_HOLE_START); > + } else { > + ram_hp_offset = ram_size; > + } > + } > + > + if (ram_hp_offset >= 0x100000000LL) { > + ret = ram_hp_offset; > + above_4g_hp_mem_size += size; > + ram_hp_offset += size; > + } > + /* if dimm fits before pci hole, append it normally */ > + else if (ram_hp_offset + size <= PCI_HOLE_START) { > + ret = ram_hp_offset; > + below_4g_hp_mem_size += size; > + ram_hp_offset += size; > + } > + /* otherwise place it above 4GB */ > + else { > + ret = 0x100000000LL; > + above_4g_hp_mem_size += size; > + ram_hp_offset = 0x100000000LL + size; > + } > + > + return ret; > +} > + > /* PC hardware initialisation */ > static void pc_init1(MemoryRegion *system_memory, > MemoryRegion *system_io, > @@ -155,9 +193,9 @@ static void pc_init1(MemoryRegion *system_memory, > kvmclock_create(); > } > > - if (ram_size >= 0xe0000000 ) { > - above_4g_mem_size = ram_size - 0xe0000000; > - below_4g_mem_size = 0xe0000000; > + if (ram_size >= PCI_HOLE_START ) { > + above_4g_mem_size = ram_size - PCI_HOLE_START; > + below_4g_mem_size = PCI_HOLE_START; > } else { > above_4g_mem_size = 0; > below_4g_mem_size = ram_size; > @@ -172,6 +210,9 @@ static void pc_init1(MemoryRegion *system_memory, > rom_memory = system_memory; > } > > + /* adjust memory map for hotplug dimms */ > + dimm_calc_offsets(pc_set_hp_memory_offset); > + > /* allocate ram and load rom/bios */ > if (!xen_enabled()) { > fw_cfg = pc_memory_init(system_memory, > @@ -192,18 +233,22 @@ static void pc_init1(MemoryRegion *system_memory, > if (pci_enabled) { > pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi, > system_memory, system_io, ram_size, > - below_4g_mem_size, > - 0x100000000ULL - below_4g_mem_size, > - 0x100000000ULL + above_4g_mem_size, > + below_4g_mem_size + below_4g_hp_mem_size, > + 0x100000000ULL - below_4g_mem_size > + - below_4g_hp_mem_size, > + 0x100000000ULL + above_4g_mem_size > + + above_4g_hp_mem_size, > (sizeof(target_phys_addr_t) == 4 > ? 0 > : ((uint64_t)1 << 62)), > pci_memory, ram_memory); > + main_memory_bus_create(object_resolve_path("i440fx", NULL)); > } else { > pci_bus = NULL; > i440fx_state = NULL; > isa_bus = isa_bus_new(NULL, system_io); > no_hpet = 1; > + main_memory_bus_create(object_resolve_path("isabus-bridge", NULL)); > } > isa_bus_irqs(isa_bus, gsi); > > -- > 1.7.9 >

1 0

Re: [SeaBIOS] [RFC PATCH v3 06/19] Implement "-dimm" command line option
by Blue Swirl Sept. 29, 2012

Sept. 29, 2012

On Mon, Sep 24, 2012 at 10:42 AM, Vasilis Liaskovitis <vasilis.liaskovitis(a)profitbricks.com> wrote: > On Sat, Sep 22, 2012 at 01:46:57PM +0000, Blue Swirl wrote: >> On Fri, Sep 21, 2012 at 11:17 AM, Vasilis Liaskovitis >> <vasilis.liaskovitis(a)profitbricks.com> wrote: >> > Example: >> > "-dimm id=dimm0,size=512M,node=0,populated=off" >> >> There should not be a need to introduce a new top level option, >> instead you should just use -device, like >> -device dimm,base=0,id=dimm0,size=512M,node=0,populated=off >> >> That would also specify the start address. > > What is "base"? the start address? I think the start address should be calculated by the > chipset / board, not by the user. Yes. > > The "-dimm" option is supposed to specify the dimm/memory layout, and not create > any devices. > > If we don't want this new option, I have a question: > > A "-device/device_add" means we create a new qdev device at startup or as a > hotplug operation respectively. So, the semantics of > "-device dimm,id=dimm0,size=512M,node=0,populated=on" are clear to me. > > What does "-device dimm,populated=off" mean from a qdev perspective? There are 2 > alternatives: > > - The device is created on the dimmbus, but is not used/populated yet. Than the > activation/acpi-hotplug of the dimm may require a separate command (we used to have > "dimm_add" in versions < 3). "device_add" handling always hotplugs a new qdev > device, so this wouldn't fit this usecase, because the device already exists. In > this case, the actual "acpi hotplug" operation is decoupled from qdev device > creation. The bus exists but the devices do not, device_add would add DIMMs to the bus. This matches PCI bus created by the host bridge and PCI device hotplug. A more complex setup would be dimm bus, dimm slot devices and DIMM devices. The intermediate slot device would contain one DIMM device if plugged. > > - The dimmdevice is not created when "-device dimm,populated=off" (this would > require some ugly checking in normal -device argument handling). Only the dimm > layout is saved. The hotplug is triggered from a normal device_add later. So in > this case, the "acpi hotplug" happens at the same time as the qdev hotplug. > > Do you see a simpler alternative without introducing a new option? > > Using the "-dimm" option follows the second semantic and avoids changing the "-device" > semantics. Dimm layout description is decoupled from dimmdevice creation, and qdev > hotplug coincides with acpi hotplug. Maybe even the dimmbus device shouldn't exist by itself after all, or it should be pretty much invisible to users. On real HW, the memory controller or south bridge handles the memory. For i440fx, it's part of the same chipset. So I think we should just add qdev properties to i440fx to specify the sizes, nodes etc. Then i440fx should create the dimmbus device unconditionally using the properties. The default properties should create a sane configuration, otherwise -global i440fx.dimm_size=512M etc. could be used. Then the bus would be populated as before or with device_add. > > thanks, > > - Vasilis

1 0

Re: [SeaBIOS] GeodeVGA global variable access
by Christian Gmeiner Sept. 28, 2012

Sept. 28, 2012

ping --- Christian Gmeiner, MSc 2012/9/17 Christian Gmeiner <christian.gmeiner(a)gmail.com>: > everything works - feel free to commit :) > > Will rework my patches (VESA support and Flat Panel Support) and hope > to send them during this > week. Finally I will look into EDID. > --- > Christian Gmeiner, MSc > > > 2012/9/16 Christian Gmeiner <christian.gmeiner(a)gmail.com>: >> looks... fine... will comile it tomorrow in the office >> >> --- >> Christian Gmeiner, MSc >> >> >> 2012/9/16 Kevin O'Connor <kevin(a)koconnor.net>: >>> Hi Christian, >>> >>> I just noticed that a recent Geode patch introduced global variables >>> that are accessed without GET_GLOBAL/SET_VGA macros. This will cause >>> random memory corruption. Please see the patch below. >>> >>> -Kevin >>> >>> >>> From ef7f73fa23e42953932920d39f729df82605b2a8 Mon Sep 17 00:00:00 2001 >>> From: Kevin O'Connor <kevin(a)koconnor.net> >>> Date: Sun, 16 Sep 2012 13:24:30 -0400 >>> Subject: [PATCH] geodevga: Access global variables using GET_GLOBAL/SET_VGA >>> macros. >>> To: seabios(a)seabios.org >>> >>> The vgabios runs in 16bit mode so all global variables must be >>> accessed via macros. >>> >>> Signed-off-by: Kevin O'Connor <kevin(a)koconnor.net> >>> --- >>> vgasrc/geodevga.c | 76 ++++++++++++++++++++++++++++--------------------------- >>> 1 file changed, 39 insertions(+), 37 deletions(-) >>> >>> diff --git a/vgasrc/geodevga.c b/vgasrc/geodevga.c >>> index ed41be6..cf1382d 100644 >>> --- a/vgasrc/geodevga.c >>> +++ b/vgasrc/geodevga.c >>> @@ -86,55 +86,58 @@ static void geode_mem_mask(u32 addr, u32 off, u32 or) >>> ); >>> } >>> >>> -/**************************************************************** >>> - * Helper functions >>> - ****************************************************************/ >>> - >>> -struct geode { >>> - u32 fb; >>> - u32 dc; >>> - u32 vp; >>> -}; >>> -struct geode geode VAR16; >>> +static u32 GeodeFB VAR16; >>> +static u32 GeodeDC VAR16; >>> +static u32 GeodeVP VAR16; >>> >>> static u32 geode_dc_read(int reg) >>> { >>> - u32 val = geode_mem_read(geode.dc + reg); >>> - dprintf(4, "%s(0x%08x) = 0x%08x\n", __func__, geode.dc + reg, val); >>> + u32 val = geode_mem_read(GET_GLOBAL(GeodeDC) + reg); >>> + dprintf(4, "%s(0x%08x) = 0x%08x\n" >>> + , __func__, GET_GLOBAL(GeodeDC) + reg, val); >>> return val; >>> } >>> >>> static void geode_dc_write(int reg, u32 val) >>> { >>> - dprintf(4, "%s(0x%08x, 0x%08x)\n", __func__, geode.dc + reg, val); >>> - geode_mem_mask(geode.dc + reg, ~0, val); >>> + dprintf(4, "%s(0x%08x, 0x%08x)\n" >>> + , __func__, GET_GLOBAL(GeodeDC) + reg, val); >>> + geode_mem_mask(GET_GLOBAL(GeodeDC) + reg, ~0, val); >>> } >>> >>> static void geode_dc_mask(int reg, u32 off, u32 on) >>> { >>> - dprintf(4, "%s(0x%08x, 0x%08x, 0x%08x)\n", __func__, geode.dc + reg, off, on); >>> - geode_mem_mask(geode.dc + reg, off, on); >>> + dprintf(4, "%s(0x%08x, 0x%08x, 0x%08x)\n" >>> + , __func__, GET_GLOBAL(GeodeDC) + reg, off, on); >>> + geode_mem_mask(GET_GLOBAL(GeodeDC) + reg, off, on); >>> } >>> >>> static u32 geode_vp_read(int reg) >>> { >>> - u32 val = geode_mem_read(geode.vp + reg); >>> - dprintf(4, "%s(0x%08x) = 0x%08x\n", __func__, geode.vp + reg, val); >>> + u32 val = geode_mem_read(GET_GLOBAL(GeodeVP) + reg); >>> + dprintf(4, "%s(0x%08x) = 0x%08x\n" >>> + , __func__, GET_GLOBAL(GeodeVP) + reg, val); >>> return val; >>> } >>> >>> static void geode_vp_write(int reg, u32 val) >>> { >>> - dprintf(4, "%s(0x%08x, 0x%08x)\n", __func__, geode.vp + reg, val); >>> - geode_mem_mask(geode.vp + reg, ~0, val); >>> + dprintf(4, "%s(0x%08x, 0x%08x)\n" >>> + , __func__, GET_GLOBAL(GeodeVP) + reg, val); >>> + geode_mem_mask(GET_GLOBAL(GeodeVP) + reg, ~0, val); >>> } >>> >>> static void geode_vp_mask(int reg, u32 off, u32 on) >>> { >>> - dprintf(4, "%s(0x%08x, 0x%08x, 0x%08x)\n", __func__, geode.vp + reg, off, on); >>> - geode_mem_mask(geode.vp + reg, off, on); >>> + dprintf(4, "%s(0x%08x, 0x%08x, 0x%08x)\n" >>> + , __func__, GET_GLOBAL(GeodeVP) + reg, off, on); >>> + geode_mem_mask(GET_GLOBAL(GeodeVP) + reg, off, on); >>> } >>> >>> +/**************************************************************** >>> + * Helper functions >>> + ****************************************************************/ >>> + >>> static int legacyio_check(void) >>> { >>> int ret=0; >>> @@ -199,10 +202,9 @@ static void dc_setup(void) >>> geode_dc_write(DC_CURS_ST_OFFSET, 0x0); >>> >>> /* read fb-bar from pci, then point dc to the fb base */ >>> - u32 dc_fb = geode_dc_read(DC_GLIU0_MEM_OFFSET); >>> - if (geode.fb != dc_fb) { >>> - geode_dc_write(DC_GLIU0_MEM_OFFSET, geode.fb); >>> - } >>> + u32 fb = GET_GLOBAL(GeodeFB); >>> + if (geode_dc_read(DC_GLIU0_MEM_OFFSET) != fb) >>> + geode_dc_write(DC_GLIU0_MEM_OFFSET, fb); >>> >>> geode_dc_mask(DC_DISPLAY_CFG, ~DC_CFG_MSK, DC_GDEN|DC_TRUP); >>> geode_dc_write(DC_GENERAL_CFG, DC_VGAE); >>> @@ -210,10 +212,10 @@ static void dc_setup(void) >>> geode_dc_write(DC_UNLOCK, DC_LOCK_LOCK); >>> >>> u32 fb_size = framebuffer_size(); // in byte >>> - dprintf(1, "%d KB of video memory at 0x%08x\n", fb_size / 1024, geode.fb); >>> + dprintf(1, "%d KB of video memory at 0x%08x\n", fb_size / 1024, fb); >>> >>> /* update VBE variables */ >>> - SET_VGA(VBE_framebuffer, geode.fb); >>> + SET_VGA(VBE_framebuffer, fb); >>> SET_VGA(VBE_total_memory, fb_size / 1024 / 64); // number of 64K blocks >>> } >>> >>> @@ -335,16 +337,16 @@ int geodevga_init(void) >>> if (GET_GLOBAL(VgaBDF) < 0) >>> // Device should be at 00:01.1 >>> SET_VGA(VgaBDF, pci_to_bdf(0, 1, 1)); >>> - >>> + >>> // setup geode struct which is used for register access >>> - geode.fb = pci_config_readl(GET_GLOBAL(VgaBDF), PCI_BASE_ADDRESS_0); >>> - geode.dc = pci_config_readl(GET_GLOBAL(VgaBDF), PCI_BASE_ADDRESS_2); >>> - geode.vp = pci_config_readl(GET_GLOBAL(VgaBDF), PCI_BASE_ADDRESS_3); >>> - >>> - dprintf(1, "fb addr: 0x%08x\n", geode.fb); >>> - dprintf(1, "dc addr: 0x%08x\n", geode.dc); >>> - dprintf(1, "vp addr: 0x%08x\n", geode.vp); >>> - >>> + SET_VGA(GeodeFB, pci_config_readl(GET_GLOBAL(VgaBDF), PCI_BASE_ADDRESS_0)); >>> + SET_VGA(GeodeDC, pci_config_readl(GET_GLOBAL(VgaBDF), PCI_BASE_ADDRESS_2)); >>> + SET_VGA(GeodeVP, pci_config_readl(GET_GLOBAL(VgaBDF), PCI_BASE_ADDRESS_3)); >>> + >>> + dprintf(1, "fb addr: 0x%08x\n", GET_GLOBAL(GeodeFB)); >>> + dprintf(1, "dc addr: 0x%08x\n", GET_GLOBAL(GeodeDC)); >>> + dprintf(1, "vp addr: 0x%08x\n", GET_GLOBAL(GeodeVP)); >>> + >>> vp_setup(); >>> dc_setup(); >>> >>> -- >>> 1.7.11.4 >>>

1 0

Re: [SeaBIOS] [RFC PATCH v3 09/19] pc: Add dimm paravirt SRAT info
by Wen Congyang Sept. 27, 2012

Sept. 27, 2012

At 09/21/2012 07:17 PM, Vasilis Liaskovitis Wrote: > The numa_fw_cfg paravirt interface is extended to include SRAT information for > all hotplug-able dimms. There are 3 words for each hotplug-able memory slot, > denoting start address, size and node proximity. The new info is appended after > existing numa info, so that the fw_cfg layout does not break. This information > is used by Seabios to build hotplug memory device objects at runtime. > nb_numa_nodes is set to 1 by default (not 0), so that we always pass srat info > to SeaBIOS. You forgot to set nb_numa_nodes to 1... Thanks Wen Congyang > > v1->v2: > Dimm SRAT info (#dimms) is appended at end of existing numa fw_cfg in order not > to break existing layout > Documentation of the new fwcfg layout is included in docs/specs/fwcfg.txt > > Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis(a)profitbricks.com> > --- > docs/specs/fwcfg.txt | 28 ++++++++++++++++++++++++++++ > hw/pc.c | 14 ++++++++++++-- > 2 files changed, 40 insertions(+), 2 deletions(-) > create mode 100644 docs/specs/fwcfg.txt > > diff --git a/docs/specs/fwcfg.txt b/docs/specs/fwcfg.txt > new file mode 100644 > index 0000000..55f96d9 > --- /dev/null > +++ b/docs/specs/fwcfg.txt > @@ -0,0 +1,28 @@ > +QEMU<->BIOS Paravirt Documentation > +-------------------------------------- > + > +This document describes paravirt data structures passed from QEMU to BIOS. > + > +FW_CFG_NUMA paravirt info > +-------------------- > +The SRAT info passed from QEMU to BIOS has the following layout: > + > +----------------------------------------------------------------------------------------------- > +#nodes | cpu0_pxm | cpu1_pxm | ... | cpulast_pxm | node0_mem | node1_mem | ... | nodelast_mem > + > +----------------------------------------------------------------------------------------------- > +#dimms | dimm0_start | dimm0_sz | dimm0_pxm | ... | dimmlast_start | dimmlast_sz | dimmlast_pxm > + > +Entry 0 contains the number of numa nodes (nb_numa_nodes). > + > +Entries 1..max_cpus: The next max_cpus entries describe node proximity for each > +one of the vCPUs in the system. > + > +Entries max_cpus+1..max_cpus+nb_numa_nodes+1: The next nb_numa_nodes entries > +describe the memory size for each one of the NUMA nodes in the system. > + > +Entry max_cpus+nb_numa_nodes+1 contains the number of memory dimms (nb_hp_dimms) > + > +The last 3 * nb_hp_dimms entries are organized in triplets: Each triplet contains > +the physical address offset, size (in bytes), and node proximity for the > +respective dimm. > diff --git a/hw/pc.c b/hw/pc.c > index 2c9664d..f2604ae 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -598,6 +598,7 @@ static void *bochs_bios_init(void) > uint8_t *smbios_table; > size_t smbios_len; > uint64_t *numa_fw_cfg; > + uint64_t *hp_dimms_fw_cfg; > int i, j; > > register_ioport_write(0x400, 1, 2, bochs_bios_write, NULL); > @@ -632,8 +633,10 @@ static void *bochs_bios_init(void) > /* allocate memory for the NUMA channel: one (64bit) word for the number > * of nodes, one word for each VCPU->node and one word for each node to > * hold the amount of memory. > + * Finally one word for the number of hotplug memory slots and three words > + * for each hotplug memory slot (start address, size and node proximity). > */ > - numa_fw_cfg = g_malloc0((1 + max_cpus + nb_numa_nodes) * 8); > + numa_fw_cfg = g_malloc0((2 + max_cpus + nb_numa_nodes + 3 * nb_hp_dimms) * 8); > numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes); > for (i = 0; i < max_cpus; i++) { > for (j = 0; j < nb_numa_nodes; j++) { > @@ -646,8 +649,15 @@ static void *bochs_bios_init(void) > for (i = 0; i < nb_numa_nodes; i++) { > numa_fw_cfg[max_cpus + 1 + i] = cpu_to_le64(node_mem[i]); > } > + > + numa_fw_cfg[1 + max_cpus + nb_numa_nodes] = cpu_to_le64(nb_hp_dimms); > + > + hp_dimms_fw_cfg = numa_fw_cfg + 2 + max_cpus + nb_numa_nodes; > + if (nb_hp_dimms) > + setup_fwcfg_hp_dimms(hp_dimms_fw_cfg); > + > fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, (uint8_t *)numa_fw_cfg, > - (1 + max_cpus + nb_numa_nodes) * 8); > + (2 + max_cpus + nb_numa_nodes + 3 * nb_hp_dimms) * 8); > > return fw_cfg; > }

1 0

Re: [SeaBIOS] [RFC PATCH v3 08/19] pc: calculate dimm physical addresses and adjust memory map
by Vasilis Liaskovitis Sept. 24, 2012

Sept. 24, 2012

On Sat, Sep 22, 2012 at 02:15:28PM +0000, Blue Swirl wrote: > > + > > +/* Function to configure memory offsets of hotpluggable dimms */ > > + > > +target_phys_addr_t pc_set_hp_memory_offset(uint64_t size) > > +{ > > + target_phys_addr_t ret; > > + > > + /* on first call, initialize ram_hp_offset */ > > + if (!ram_hp_offset) { > > + if (ram_size >= PCI_HOLE_START ) { > > + ram_hp_offset = 0x100000000LL + (ram_size - PCI_HOLE_START); > > + } else { > > + ram_hp_offset = ram_size; > > + } > > + } > > + > > + if (ram_hp_offset >= 0x100000000LL) { > > + ret = ram_hp_offset; > > + above_4g_hp_mem_size += size; > > + ram_hp_offset += size; > > + } > > + /* if dimm fits before pci hole, append it normally */ > > + else if (ram_hp_offset + size <= PCI_HOLE_START) { > > } else if ... > > > + ret = ram_hp_offset; > > + below_4g_hp_mem_size += size; > > + ram_hp_offset += size; > > + } > > + /* otherwise place it above 4GB */ > > + else { > > } else { > > > + ret = 0x100000000LL; > > + above_4g_hp_mem_size += size; > > + ram_hp_offset = 0x100000000LL + size; > > + } > > + > > + return ret; > > +} > > But the function and use of lots of global variables is ugly. The dimm > devices should be just created in piix_pci.c (i440fx) directly with > correct offsets and sizes, so all below_4g_mem_size etc. calculations > should be moved there. That would implement the PMC part of i440fx. > > For ISA PC, probably the board should create the DIMMs since there may > not be a memory controller. The >4G logic does not make sense there > anyway. What about moving the implementation to pc_piix.c? Initial RAM and pci windows are already calculated in pc_init1, and then passed to i440fx_init. The memory bus could be attached to i440fx for pci-enabled pc and to isabus-bridge for isa-pc (isa-pc not tested yet). Something like the following: --- hw/pc.h | 1 + hw/pc_piix.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/hw/pc.h b/hw/pc.h index e4db071..d6cc43b 100644 --- a/hw/pc.h +++ b/hw/pc.h @@ -10,6 +10,7 @@ #include "memory.h" #include "ioapic.h" +#define PCI_HOLE_START 0xe0000000 /* PC-style peripherals (also used by other machines). */ /* serial.c */ diff --git a/hw/pc_piix.c b/hw/pc_piix.c index 88ff041..17db95a 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -43,6 +43,7 @@ #include "xen.h" #include "memory.h" #include "exec-memory.h" +#include "dimm.h" #ifdef CONFIG_XEN # include <xen/hvm/hvm_info_table.h> #endif @@ -52,6 +53,8 @@ static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 }; static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 }; static const int ide_irq[MAX_IDE_BUS] = { 14, 15 }; +static ram_addr_t below_4g_hp_mem_size = 0; +static ram_addr_t above_4g_hp_mem_size = 0; static void kvm_piix3_setup_irq_routing(bool pci_enabled) { @@ -117,6 +120,41 @@ static void ioapic_init(GSIState *gsi_state) } } +static target_phys_addr_t pc_set_hp_memory_offset(uint64_t size) +{ + target_phys_addr_t ret; + static ram_addr_t ram_hp_offset = 0; + + /* on first call, initialize ram_hp_offset */ + if (!ram_hp_offset) { + if (ram_size >= PCI_HOLE_START ) { + ram_hp_offset = 0x100000000LL + (ram_size - PCI_HOLE_START); + } else { + ram_hp_offset = ram_size; + } + } + + if (ram_hp_offset >= 0x100000000LL) { + ret = ram_hp_offset; + above_4g_hp_mem_size += size; + ram_hp_offset += size; + } + /* if dimm fits before pci hole, append it normally */ + else if (ram_hp_offset + size <= PCI_HOLE_START) { + ret = ram_hp_offset; + below_4g_hp_mem_size += size; + ram_hp_offset += size; + } + /* otherwise place it above 4GB */ + else { + ret = 0x100000000LL; + above_4g_hp_mem_size += size; + ram_hp_offset = 0x100000000LL + size; + } + + return ret; +} + /* PC hardware initialisation */ static void pc_init1(MemoryRegion *system_memory, MemoryRegion *system_io, @@ -155,9 +193,9 @@ static void pc_init1(MemoryRegion *system_memory, kvmclock_create(); } - if (ram_size >= 0xe0000000 ) { - above_4g_mem_size = ram_size - 0xe0000000; - below_4g_mem_size = 0xe0000000; + if (ram_size >= PCI_HOLE_START ) { + above_4g_mem_size = ram_size - PCI_HOLE_START; + below_4g_mem_size = PCI_HOLE_START; } else { above_4g_mem_size = 0; below_4g_mem_size = ram_size; @@ -172,6 +210,9 @@ static void pc_init1(MemoryRegion *system_memory, rom_memory = system_memory; } + /* adjust memory map for hotplug dimms */ + dimm_calc_offsets(pc_set_hp_memory_offset); + /* allocate ram and load rom/bios */ if (!xen_enabled()) { fw_cfg = pc_memory_init(system_memory, @@ -192,18 +233,22 @@ static void pc_init1(MemoryRegion *system_memory, if (pci_enabled) { pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi, system_memory, system_io, ram_size, - below_4g_mem_size, - 0x100000000ULL - below_4g_mem_size, - 0x100000000ULL + above_4g_mem_size, + below_4g_mem_size + below_4g_hp_mem_size, + 0x100000000ULL - below_4g_mem_size + - below_4g_hp_mem_size, + 0x100000000ULL + above_4g_mem_size + + above_4g_hp_mem_size, (sizeof(target_phys_addr_t) == 4 ? 0 : ((uint64_t)1 << 62)), pci_memory, ram_memory); + main_memory_bus_create(object_resolve_path("i440fx", NULL)); } else { pci_bus = NULL; i440fx_state = NULL; isa_bus = isa_bus_new(NULL, system_io); no_hpet = 1; + main_memory_bus_create(object_resolve_path("isabus-bridge", NULL)); } isa_bus_irqs(isa_bus, gsi); -- 1.7.9

1 0

Re: [SeaBIOS] [RFC PATCH v3 11/19] Implement qmp and hmp commands for notification lists
by Eric Blake Sept. 24, 2012

Sept. 24, 2012

On 09/21/2012 05:17 AM, Vasilis Liaskovitis wrote: > Guest can respond to ACPI hotplug events e.g. with _EJ or _OST method. > This patch implements a tail queue to store guest notifications for memory > hot-add and hot-remove requests. > > Guest responses for memory hotplug command on a per-dimm basis can be detected > with the new hmp command "info memhp" or the new qmp command "query-memhp" Naming doesn't match the QMP code. > Examples: > > (qemu) device_add dimm,id=ram0 > > These notification items should probably be part of migration state (not yet > implemented). In the case of libvirt driving migration, you already said in 10/19 that libvirt has to start the destination with the populated=on|off fields correct for each dimm according to the state it was in at the time the host started the update. Can the host hot unplug memory after migration has started? > + > +## > +# @MemHpInfo: > +# > +# Information about status of a memory hotplug command > +# > +# @dimm: the Dimm associated with the result > +# > +# @result: the result of the hotplug command > +# > +# Since: 1.3 > +# > +## > +{ 'type': 'MemHpInfo', > + 'data': {'dimm': 'str', 'request': 'str', 'result': 'str'} } Should 'result' be a bool (true for success, false for still pending) or an enum, instead of a free-form string? Likewise, isn't 'request' going to be exactly one of two values (plug or unplug)? -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

2 1

Re: [SeaBIOS] [RFC PATCH v3 20/19] alternative: Use paravirt interface for pci windows
by Wen Congyang Sept. 24, 2012

Sept. 24, 2012

At 09/21/2012 07:20 PM, Vasilis Liaskovitis Wrote: > Initialize the 32-bit and 64-bit pci starting offsets from values passed in by > the qemu paravirt interface QEMU_CFG_PCI_WINDOW. Qemu calculates the starting > offsets based on initial memory and hotplug-able dimms. This patch can't be applied if I apply the other patches for seabios. And I don't find this patch in your tree. Thanks Wen Congyang > > Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis(a)profitbricks.com> > --- > src/paravirt.c | 6 ++++++ > src/paravirt.h | 2 ++ > src/pciinit.c | 5 ++--- > 3 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/src/paravirt.c b/src/paravirt.c > index 2a98d53..390ef30 100644 > --- a/src/paravirt.c > +++ b/src/paravirt.c > @@ -346,3 +346,9 @@ void qemu_cfg_romfile_setup(void) > dprintf(3, "Found fw_cfg file: %s (size=%d)\n", file->name, file->size); > } > } > + > +void qemu_cfg_get_pci_offsets(u64 *pcimem_start, u64 *pcimem64_start) > +{ > + qemu_cfg_read_entry(pcimem_start, QEMU_CFG_PCI_WINDOW, sizeof(u64)); > + qemu_cfg_read((u8*)(pcimem64_start), sizeof(u64)); > +} > diff --git a/src/paravirt.h b/src/paravirt.h > index a284c41..b53ff88 100644 > --- a/src/paravirt.h > +++ b/src/paravirt.h > @@ -35,6 +35,7 @@ static inline int kvm_para_available(void) > #define QEMU_CFG_BOOT_MENU 0x0e > #define QEMU_CFG_MAX_CPUS 0x0f > #define QEMU_CFG_FILE_DIR 0x19 > +#define QEMU_CFG_PCI_WINDOW 0x1a > #define QEMU_CFG_ARCH_LOCAL 0x8000 > #define QEMU_CFG_ACPI_TABLES (QEMU_CFG_ARCH_LOCAL + 0) > #define QEMU_CFG_SMBIOS_ENTRIES (QEMU_CFG_ARCH_LOCAL + 1) > @@ -65,5 +66,6 @@ struct e820_reservation { > u32 qemu_cfg_e820_entries(void); > void* qemu_cfg_e820_load_next(void *addr); > void qemu_cfg_romfile_setup(void); > +void qemu_cfg_get_pci_offsets(u64 *pcimem_start, u64 *pcimem64_start); > > #endif > diff --git a/src/pciinit.c b/src/pciinit.c > index 68f302a..64468a0 100644 > --- a/src/pciinit.c > +++ b/src/pciinit.c > @@ -592,8 +592,7 @@ static void pci_region_map_entries(struct pci_bus *busses, struct pci_region *r) > > static void pci_bios_map_devices(struct pci_bus *busses) > { > - pcimem_start = RamSize; > - > + qemu_cfg_get_pci_offsets(&pcimem_start, &pcimem64_start); > if (pci_bios_init_root_regions(busses)) { > struct pci_region r64_mem, r64_pref; > r64_mem.list = NULL; > @@ -611,7 +610,7 @@ static void pci_bios_map_devices(struct pci_bus *busses) > u64 align_mem = pci_region_align(&r64_mem); > u64 align_pref = pci_region_align(&r64_pref); > > - r64_mem.base = ALIGN(0x100000000LL + RamSizeOver4G, align_mem); > + r64_mem.base = ALIGN(pcimem64_start, align_mem); > r64_pref.base = ALIGN(r64_mem.base + sum_mem, align_pref); > pcimem64_start = r64_mem.base; > pcimem64_end = r64_pref.base + sum_pref;

2 1

Re: [SeaBIOS] [RFC PATCH v3 06/19] Implement "-dimm" command line option
by Vasilis Liaskovitis Sept. 24, 2012

Sept. 24, 2012

On Sat, Sep 22, 2012 at 01:46:57PM +0000, Blue Swirl wrote: > On Fri, Sep 21, 2012 at 11:17 AM, Vasilis Liaskovitis > <vasilis.liaskovitis(a)profitbricks.com> wrote: > > Example: > > "-dimm id=dimm0,size=512M,node=0,populated=off" > > There should not be a need to introduce a new top level option, > instead you should just use -device, like > -device dimm,base=0,id=dimm0,size=512M,node=0,populated=off > > That would also specify the start address. What is "base"? the start address? I think the start address should be calculated by the chipset / board, not by the user. The "-dimm" option is supposed to specify the dimm/memory layout, and not create any devices. If we don't want this new option, I have a question: A "-device/device_add" means we create a new qdev device at startup or as a hotplug operation respectively. So, the semantics of "-device dimm,id=dimm0,size=512M,node=0,populated=on" are clear to me. What does "-device dimm,populated=off" mean from a qdev perspective? There are 2 alternatives: - The device is created on the dimmbus, but is not used/populated yet. Than the activation/acpi-hotplug of the dimm may require a separate command (we used to have "dimm_add" in versions < 3). "device_add" handling always hotplugs a new qdev device, so this wouldn't fit this usecase, because the device already exists. In this case, the actual "acpi hotplug" operation is decoupled from qdev device creation. - The dimmdevice is not created when "-device dimm,populated=off" (this would require some ugly checking in normal -device argument handling). Only the dimm layout is saved. The hotplug is triggered from a normal device_add later. So in this case, the "acpi hotplug" happens at the same time as the qdev hotplug. Do you see a simpler alternative without introducing a new option? Using the "-dimm" option follows the second semantic and avoids changing the "-device" semantics. Dimm layout description is decoupled from dimmdevice creation, and qdev hotplug coincides with acpi hotplug. thanks, - Vasilis

1 0