From 77af8a2b95b79699de650965d5228772743efe84 Mon Sep 17 00:00:00 2001 From: Phil Dennis-Jordan Date: Wed, 15 Mar 2017 19:20:26 +1300 Subject: hw/i386: Use Rev3 FADT (ACPI 2.0) instead of Rev1 to improve guest OS support. This updates the FADT generated for x86/64 machine types from Revision 1 to 3. (Based on ACPI standard 2.0 instead of 1.0) The intention is to expose the reset register information to guest operating systems which require it, specifically OS X/macOS. Revision 1 FADTs do not contain the fields relating to the reset register. The new layout and contents remains backwards-compatible with operating systems which only support ACPI 1.0, as the existing fields are not modified by this change, as the 64-bit and 32-bit variants are allowed to co-exist according to the ACPI 2.0 standard. No regressions became apparent in tests with a range of Windows (XP-10) and Linux versions. The BIOS tables test suite's FADT checksum test has also been updated to reflect the new FADT layout and content. Signed-off-by: Phil Dennis-Jordan Message-Id: <1489558827-28971-2-git-send-email-phil@philjordan.eu> Signed-off-by: Paolo Bonzini --- hw/i386/acpi-build.c | 32 +++++++++++++++++-- include/hw/acpi/acpi-defs.h | 77 +++++++++++++++++++++------------------------ tests/acpi-utils.h | 10 ++++++ tests/bios-tables-test.c | 23 +++++++++++--- 4 files changed, 93 insertions(+), 49 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 2073108..7997f06 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -272,7 +272,7 @@ build_facs(GArray *table_data, BIOSLinker *linker) } /* Load chipset information in FADT */ -static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm) +static void fadt_setup(AcpiFadtDescriptorRev3 *fadt, AcpiPmInfo *pm) { fadt->model = 1; fadt->reserved1 = 0; @@ -304,6 +304,28 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm) fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL); } fadt->century = RTC_CENTURY; + + fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_RESET_REG_SUP); + fadt->reset_value = 0xf; + fadt->reset_register.space_id = AML_SYSTEM_IO; + fadt->reset_register.bit_width = 8; + fadt->reset_register.address = cpu_to_le64(ICH9_RST_CNT_IOPORT); + + fadt->xpm1a_event_block.space_id = AML_SYSTEM_IO; + fadt->xpm1a_event_block.bit_width = fadt->pm1_evt_len * 8; + fadt->xpm1a_event_block.address = cpu_to_le64(pm->io_base); + + fadt->xpm1a_control_block.space_id = AML_SYSTEM_IO; + fadt->xpm1a_control_block.bit_width = fadt->pm1_cnt_len * 8; + fadt->xpm1a_control_block.address = cpu_to_le64(pm->io_base + 0x4); + + fadt->xpm_timer_block.space_id = AML_SYSTEM_IO; + fadt->xpm_timer_block.bit_width = fadt->pm_tmr_len * 8; + fadt->xpm_timer_block.address = cpu_to_le64(pm->io_base + 0x8); + + fadt->xgpe0_block.space_id = AML_SYSTEM_IO; + fadt->xgpe0_block.bit_width = pm->gpe0_blk_len * 8; + fadt->xgpe0_block.address = cpu_to_le64(pm->gpe0_blk); } @@ -313,9 +335,10 @@ build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm, unsigned facs_tbl_offset, unsigned dsdt_tbl_offset, const char *oem_id, const char *oem_table_id) { - AcpiFadtDescriptorRev1 *fadt = acpi_data_push(table_data, sizeof(*fadt)); + AcpiFadtDescriptorRev3 *fadt = acpi_data_push(table_data, sizeof(*fadt)); unsigned fw_ctrl_offset = (char *)&fadt->firmware_ctrl - table_data->data; unsigned dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data; + unsigned xdsdt_entry_offset = (char *)&fadt->Xdsdt - table_data->data; /* FACS address to be filled by Guest linker */ bios_linker_loader_add_pointer(linker, @@ -327,9 +350,12 @@ build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm, bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt), ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset); + bios_linker_loader_add_pointer(linker, + ACPI_BUILD_TABLE_FILE, xdsdt_entry_offset, sizeof(fadt->Xdsdt), + ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset); build_header(linker, table_data, - (void *)fadt, "FACP", sizeof(*fadt), 1, oem_id, oem_table_id); + (void *)fadt, "FACP", sizeof(*fadt), 3, oem_id, oem_table_id); } void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h index 4cc3630..293ee45 100644 --- a/include/hw/acpi/acpi-defs.h +++ b/include/hw/acpi/acpi-defs.h @@ -131,17 +131,37 @@ typedef struct AcpiTableHeader AcpiTableHeader; uint8_t duty_width; /* Bit width of duty cycle field in p_cnt reg */ \ uint8_t day_alrm; /* Index to day-of-month alarm in RTC CMOS RAM */ \ uint8_t mon_alrm; /* Index to month-of-year alarm in RTC CMOS RAM */ \ - uint8_t century; /* Index to century in RTC CMOS RAM */ - -struct AcpiFadtDescriptorRev1 -{ - ACPI_FADT_COMMON_DEF - uint8_t reserved4; /* Reserved */ - uint8_t reserved4a; /* Reserved */ - uint8_t reserved4b; /* Reserved */ - uint32_t flags; -} QEMU_PACKED; -typedef struct AcpiFadtDescriptorRev1 AcpiFadtDescriptorRev1; + uint8_t century; /* Index to century in RTC CMOS RAM */ \ + /* IA-PC Boot Architecture Flags (see below for individual flags) */ \ + uint16_t boot_flags; \ + uint8_t reserved; /* Reserved, must be zero */ \ + /* Miscellaneous flag bits (see below for individual flags) */ \ + uint32_t flags; \ + /* 64-bit address of the Reset register */ \ + struct AcpiGenericAddress reset_register; \ + /* Value to write to the reset_register port to reset the system */ \ + uint8_t reset_value; \ + /* ARM-Specific Boot Flags (see below for individual flags) (ACPI 5.1) */ \ + uint16_t arm_boot_flags; \ + uint8_t minor_revision; /* FADT Minor Revision (ACPI 5.1) */ \ + uint64_t Xfacs; /* 64-bit physical address of FACS */ \ + uint64_t Xdsdt; /* 64-bit physical address of DSDT */ \ + /* 64-bit Extended Power Mgt 1a Event Reg Blk address */ \ + struct AcpiGenericAddress xpm1a_event_block; \ + /* 64-bit Extended Power Mgt 1b Event Reg Blk address */ \ + struct AcpiGenericAddress xpm1b_event_block; \ + /* 64-bit Extended Power Mgt 1a Control Reg Blk address */ \ + struct AcpiGenericAddress xpm1a_control_block; \ + /* 64-bit Extended Power Mgt 1b Control Reg Blk address */ \ + struct AcpiGenericAddress xpm1b_control_block; \ + /* 64-bit Extended Power Mgt 2 Control Reg Blk address */ \ + struct AcpiGenericAddress xpm2_control_block; \ + /* 64-bit Extended Power Mgt Timer Ctrl Reg Blk address */ \ + struct AcpiGenericAddress xpm_timer_block; \ + /* 64-bit Extended General Purpose Event 0 Reg Blk address */ \ + struct AcpiGenericAddress xgpe0_block; \ + /* 64-bit Extended General Purpose Event 1 Reg Blk address */ \ + struct AcpiGenericAddress xgpe1_block; \ struct AcpiGenericAddress { uint8_t space_id; /* Address space where struct or register exists */ @@ -151,38 +171,13 @@ struct AcpiGenericAddress { uint64_t address; /* 64-bit address of struct or register */ } QEMU_PACKED; +struct AcpiFadtDescriptorRev3 { + ACPI_FADT_COMMON_DEF +} QEMU_PACKED; +typedef struct AcpiFadtDescriptorRev3 AcpiFadtDescriptorRev3; + struct AcpiFadtDescriptorRev5_1 { ACPI_FADT_COMMON_DEF - /* IA-PC Boot Architecture Flags (see below for individual flags) */ - uint16_t boot_flags; - uint8_t reserved; /* Reserved, must be zero */ - /* Miscellaneous flag bits (see below for individual flags) */ - uint32_t flags; - /* 64-bit address of the Reset register */ - struct AcpiGenericAddress reset_register; - /* Value to write to the reset_register port to reset the system */ - uint8_t reset_value; - /* ARM-Specific Boot Flags (see below for individual flags) (ACPI 5.1) */ - uint16_t arm_boot_flags; - uint8_t minor_revision; /* FADT Minor Revision (ACPI 5.1) */ - uint64_t Xfacs; /* 64-bit physical address of FACS */ - uint64_t Xdsdt; /* 64-bit physical address of DSDT */ - /* 64-bit Extended Power Mgt 1a Event Reg Blk address */ - struct AcpiGenericAddress xpm1a_event_block; - /* 64-bit Extended Power Mgt 1b Event Reg Blk address */ - struct AcpiGenericAddress xpm1b_event_block; - /* 64-bit Extended Power Mgt 1a Control Reg Blk address */ - struct AcpiGenericAddress xpm1a_control_block; - /* 64-bit Extended Power Mgt 1b Control Reg Blk address */ - struct AcpiGenericAddress xpm1b_control_block; - /* 64-bit Extended Power Mgt 2 Control Reg Blk address */ - struct AcpiGenericAddress xpm2_control_block; - /* 64-bit Extended Power Mgt Timer Ctrl Reg Blk address */ - struct AcpiGenericAddress xpm_timer_block; - /* 64-bit Extended General Purpose Event 0 Reg Blk address */ - struct AcpiGenericAddress xgpe0_block; - /* 64-bit Extended General Purpose Event 1 Reg Blk address */ - struct AcpiGenericAddress xgpe1_block; /* 64-bit Sleep Control register (ACPI 5.0) */ struct AcpiGenericAddress sleep_control; /* 64-bit Sleep Status register (ACPI 5.0) */ diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h index 348e4d7..f8d8723 100644 --- a/tests/acpi-utils.h +++ b/tests/acpi-utils.h @@ -87,6 +87,16 @@ typedef struct { g_assert_cmpstr(ACPI_ASSERT_CMP_str, ==, expected); \ } while (0) +#define ACPI_READ_GENERIC_ADDRESS(field, addr) \ + do { \ + ACPI_READ_FIELD((field).space_id, addr); \ + ACPI_READ_FIELD((field).bit_width, addr); \ + ACPI_READ_FIELD((field).bit_offset, addr); \ + ACPI_READ_FIELD((field).access_width, addr); \ + ACPI_READ_FIELD((field).address, addr); \ + } while (0); + + uint8_t acpi_calc_checksum(const uint8_t *data, int len); uint32_t acpi_find_rsdp_address(void); void acpi_parse_rsdp_table(uint32_t addr, AcpiRsdpDescriptor *rsdp_table); diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c index 88dbf97..9c96a67 100644 --- a/tests/bios-tables-test.c +++ b/tests/bios-tables-test.c @@ -29,7 +29,7 @@ typedef struct { uint32_t rsdp_addr; AcpiRsdpDescriptor rsdp_table; AcpiRsdtDescriptorRev1 rsdt_table; - AcpiFadtDescriptorRev1 fadt_table; + AcpiFadtDescriptorRev3 fadt_table; AcpiFacsDescriptorRev1 facs_table; uint32_t *rsdt_tables_addr; int rsdt_tables_nr; @@ -126,7 +126,7 @@ static void test_acpi_rsdt_table(test_data *data) static void test_acpi_fadt_table(test_data *data) { - AcpiFadtDescriptorRev1 *fadt_table = &data->fadt_table; + AcpiFadtDescriptorRev3 *fadt_table = &data->fadt_table; uint32_t addr; /* FADT table comes first */ @@ -168,10 +168,23 @@ static void test_acpi_fadt_table(test_data *data) ACPI_READ_FIELD(fadt_table->day_alrm, addr); ACPI_READ_FIELD(fadt_table->mon_alrm, addr); ACPI_READ_FIELD(fadt_table->century, addr); - ACPI_READ_FIELD(fadt_table->reserved4, addr); - ACPI_READ_FIELD(fadt_table->reserved4a, addr); - ACPI_READ_FIELD(fadt_table->reserved4b, addr); + ACPI_READ_FIELD(fadt_table->boot_flags, addr); + ACPI_READ_FIELD(fadt_table->reserved, addr); ACPI_READ_FIELD(fadt_table->flags, addr); + ACPI_READ_GENERIC_ADDRESS(fadt_table->reset_register, addr); + ACPI_READ_FIELD(fadt_table->reset_value, addr); + ACPI_READ_FIELD(fadt_table->arm_boot_flags, addr); + ACPI_READ_FIELD(fadt_table->minor_revision, addr); + ACPI_READ_FIELD(fadt_table->Xfacs, addr); + ACPI_READ_FIELD(fadt_table->Xdsdt, addr); + ACPI_READ_GENERIC_ADDRESS(fadt_table->xpm1a_event_block, addr); + ACPI_READ_GENERIC_ADDRESS(fadt_table->xpm1b_event_block, addr); + ACPI_READ_GENERIC_ADDRESS(fadt_table->xpm1a_control_block, addr); + ACPI_READ_GENERIC_ADDRESS(fadt_table->xpm1b_control_block, addr); + ACPI_READ_GENERIC_ADDRESS(fadt_table->xpm2_control_block, addr); + ACPI_READ_GENERIC_ADDRESS(fadt_table->xpm_timer_block, addr); + ACPI_READ_GENERIC_ADDRESS(fadt_table->xgpe0_block, addr); + ACPI_READ_GENERIC_ADDRESS(fadt_table->xgpe1_block, addr); ACPI_ASSERT_CMP(fadt_table->signature, "FACP"); g_assert(!acpi_calc_checksum((uint8_t *)fadt_table, fadt_table->length)); -- cgit v1.1 From 6103451aeb749e92bf7d730429985189c6921c32 Mon Sep 17 00:00:00 2001 From: Phil Dennis-Jordan Date: Wed, 15 Mar 2017 19:20:27 +1300 Subject: hw/i386: Build-time assertion on pc/q35 reset register being identical. This adds a clarifying comment and build time assert to the FADT reset register field initialisation: the reset register is the same on both machine types. Signed-off-by: Phil Dennis-Jordan Message-Id: <1489558827-28971-3-git-send-email-phil@philjordan.eu> Signed-off-by: Paolo Bonzini --- hw/i386/acpi-build.c | 3 +++ hw/pci-host/piix.c | 6 ------ include/hw/i386/pc.h | 6 ++++++ 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 7997f06..1d8c645 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -310,6 +310,9 @@ static void fadt_setup(AcpiFadtDescriptorRev3 *fadt, AcpiPmInfo *pm) fadt->reset_register.space_id = AML_SYSTEM_IO; fadt->reset_register.bit_width = 8; fadt->reset_register.address = cpu_to_le64(ICH9_RST_CNT_IOPORT); + /* The above need not be conditional on machine type because the reset port + * happens to be the same on PIIX (pc) and ICH9 (q35). */ + QEMU_BUILD_BUG_ON(ICH9_RST_CNT_IOPORT != RCR_IOPORT); fadt->xpm1a_event_block.space_id = AML_SYSTEM_IO; fadt->xpm1a_event_block.bit_width = fadt->pm1_evt_len * 8; diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c index f9218aa..bf4221d 100644 --- a/hw/pci-host/piix.c +++ b/hw/pci-host/piix.c @@ -58,12 +58,6 @@ typedef struct I440FXState { #define XEN_PIIX_NUM_PIRQS 128ULL #define PIIX_PIRQC 0x60 -/* - * Reset Control Register: PCI-accessible ISA-Compatible Register at address - * 0xcf9, provided by the PCI/ISA bridge (PIIX3 PCI function 0, 8086:7000). - */ -#define RCR_IOPORT 0xcf9 - typedef struct PIIX3State { PCIDevice dev; diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index f278b3a..416aaa5 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -303,6 +303,12 @@ typedef struct PCII440FXState PCII440FXState; #define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "igd-passthrough-i440FX" +/* + * Reset Control Register: PCI-accessible ISA-Compatible Register at address + * 0xcf9, provided by the PCI/ISA bridge (PIIX3 PCI function 0, 8086:7000). + */ +#define RCR_IOPORT 0xcf9 + PCIBus *i440fx_init(const char *host_type, const char *pci_type, PCII440FXState **pi440fx_state, int *piix_devfn, ISABus **isa_bus, qemu_irq *pic, -- cgit v1.1 From b19456dd0ea4eb418ad093f092adbb882be13054 Mon Sep 17 00:00:00 2001 From: zhanghailiang Date: Wed, 19 Apr 2017 09:15:32 +0800 Subject: char: Fix removing wrong GSource that be found by fd_in_tag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We use fd_in_tag to find a GSource, fd_in_tag is return value of g_source_attach(GSource *source, GMainContext *context), the return value is unique only in the same context, so we may get the same values with different 'context' parameters. It is no problem to find the right fd_in_tag by using g_main_context_find_source_by_id(GMainContext *context, guint source_id) while there is only one default main context. But colo-compare tries to create/use its own context, and if we pass wrong 'context' parameter with right fd_in_tag, we will find a wrong GSource to handle. We tried to fix the related codes in commit b43decb015a6efeb9e3cdbdb80f6547ad7248a4c, but it didn't fix the bug completely, because we still have some codes didn't pass *right* context parameter for remove_fd_in_watch(). Let's fix it by record the GSource directly instead of fd_in_tag. Signed-off-by: zhanghailiang Reviewed-by: Marc-André Lureau Message-Id: <1492564532-91680-1-git-send-email-zhang.zhanghailiang@huawei.com> Signed-off-by: Paolo Bonzini --- chardev/char-fd.c | 8 ++++---- chardev/char-io.c | 23 ++++++++--------------- chardev/char-io.h | 4 ++-- chardev/char-pty.c | 6 +++--- chardev/char-socket.c | 8 ++++---- chardev/char-udp.c | 8 ++++---- chardev/char.c | 2 +- include/sysemu/char.h | 2 +- 8 files changed, 27 insertions(+), 34 deletions(-) diff --git a/chardev/char-fd.c b/chardev/char-fd.c index 548dd4c..0b182c5 100644 --- a/chardev/char-fd.c +++ b/chardev/char-fd.c @@ -58,7 +58,7 @@ static gboolean fd_chr_read(QIOChannel *chan, GIOCondition cond, void *opaque) ret = qio_channel_read( chan, (gchar *)buf, len, NULL); if (ret == 0) { - remove_fd_in_watch(chr, NULL); + remove_fd_in_watch(chr); qemu_chr_be_event(chr, CHR_EVENT_CLOSED); return FALSE; } @@ -89,9 +89,9 @@ static void fd_chr_update_read_handler(Chardev *chr, { FDChardev *s = FD_CHARDEV(chr); - remove_fd_in_watch(chr, NULL); + remove_fd_in_watch(chr); if (s->ioc_in) { - chr->fd_in_tag = io_add_watch_poll(chr, s->ioc_in, + chr->gsource = io_add_watch_poll(chr, s->ioc_in, fd_chr_read_poll, fd_chr_read, chr, context); @@ -103,7 +103,7 @@ static void char_fd_finalize(Object *obj) Chardev *chr = CHARDEV(obj); FDChardev *s = FD_CHARDEV(obj); - remove_fd_in_watch(chr, NULL); + remove_fd_in_watch(chr); if (s->ioc_in) { object_unref(OBJECT(s->ioc_in)); } diff --git a/chardev/char-io.c b/chardev/char-io.c index b4bb094..b5708ee 100644 --- a/chardev/char-io.c +++ b/chardev/char-io.c @@ -98,7 +98,7 @@ static GSourceFuncs io_watch_poll_funcs = { .finalize = io_watch_poll_finalize, }; -guint io_add_watch_poll(Chardev *chr, +GSource *io_add_watch_poll(Chardev *chr, QIOChannel *ioc, IOCanReadHandler *fd_can_read, QIOChannelFunc fd_read, @@ -106,7 +106,6 @@ guint io_add_watch_poll(Chardev *chr, GMainContext *context) { IOWatchPoll *iwp; - int tag; char *name; iwp = (IOWatchPoll *) g_source_new(&io_watch_poll_funcs, @@ -122,21 +121,15 @@ guint io_add_watch_poll(Chardev *chr, g_source_set_name((GSource *)iwp, name); g_free(name); - tag = g_source_attach(&iwp->parent, context); + g_source_attach(&iwp->parent, context); g_source_unref(&iwp->parent); - return tag; + return (GSource *)iwp; } -static void io_remove_watch_poll(guint tag, GMainContext *context) +static void io_remove_watch_poll(GSource *source) { - GSource *source; IOWatchPoll *iwp; - g_return_if_fail(tag > 0); - - source = g_main_context_find_source_by_id(context, tag); - g_return_if_fail(source != NULL); - iwp = io_watch_poll_from_source(source); if (iwp->src) { g_source_destroy(iwp->src); @@ -146,11 +139,11 @@ static void io_remove_watch_poll(guint tag, GMainContext *context) g_source_destroy(&iwp->parent); } -void remove_fd_in_watch(Chardev *chr, GMainContext *context) +void remove_fd_in_watch(Chardev *chr) { - if (chr->fd_in_tag) { - io_remove_watch_poll(chr->fd_in_tag, context); - chr->fd_in_tag = 0; + if (chr->gsource) { + io_remove_watch_poll(chr->gsource); + chr->gsource = NULL; } } diff --git a/chardev/char-io.h b/chardev/char-io.h index 842be56..55973a7 100644 --- a/chardev/char-io.h +++ b/chardev/char-io.h @@ -29,14 +29,14 @@ #include "sysemu/char.h" /* Can only be used for read */ -guint io_add_watch_poll(Chardev *chr, +GSource *io_add_watch_poll(Chardev *chr, QIOChannel *ioc, IOCanReadHandler *fd_can_read, QIOChannelFunc fd_read, gpointer user_data, GMainContext *context); -void remove_fd_in_watch(Chardev *chr, GMainContext *context); +void remove_fd_in_watch(Chardev *chr); int io_channel_send(QIOChannel *ioc, const void *buf, size_t len); diff --git a/chardev/char-pty.c b/chardev/char-pty.c index a6337be..581ab34 100644 --- a/chardev/char-pty.c +++ b/chardev/char-pty.c @@ -199,7 +199,7 @@ static void pty_chr_state(Chardev *chr, int connected) g_source_remove(s->open_tag); s->open_tag = 0; } - remove_fd_in_watch(chr, NULL); + remove_fd_in_watch(chr); s->connected = 0; /* (re-)connect poll interval for idle guests: once per second. * We check more frequently in case the guests sends data to @@ -215,8 +215,8 @@ static void pty_chr_state(Chardev *chr, int connected) s->connected = 1; s->open_tag = g_idle_add(qemu_chr_be_generic_open_func, chr); } - if (!chr->fd_in_tag) { - chr->fd_in_tag = io_add_watch_poll(chr, s->ioc, + if (!chr->gsource) { + chr->gsource = io_add_watch_poll(chr, s->ioc, pty_chr_read_poll, pty_chr_read, chr, NULL); diff --git a/chardev/char-socket.c b/chardev/char-socket.c index 36ab0d6..d8de051 100644 --- a/chardev/char-socket.c +++ b/chardev/char-socket.c @@ -327,7 +327,7 @@ static void tcp_chr_free_connection(Chardev *chr) } tcp_set_msgfds(chr, NULL, 0); - remove_fd_in_watch(chr, NULL); + remove_fd_in_watch(chr); object_unref(OBJECT(s->sioc)); s->sioc = NULL; object_unref(OBJECT(s->ioc)); @@ -484,7 +484,7 @@ static void tcp_chr_connect(void *opaque) s->connected = 1; if (s->ioc) { - chr->fd_in_tag = io_add_watch_poll(chr, s->ioc, + chr->gsource = io_add_watch_poll(chr, s->ioc, tcp_chr_read_poll, tcp_chr_read, chr, NULL); @@ -501,9 +501,9 @@ static void tcp_chr_update_read_handler(Chardev *chr, return; } - remove_fd_in_watch(chr, NULL); + remove_fd_in_watch(chr); if (s->ioc) { - chr->fd_in_tag = io_add_watch_poll(chr, s->ioc, + chr->gsource = io_add_watch_poll(chr, s->ioc, tcp_chr_read_poll, tcp_chr_read, chr, context); diff --git a/chardev/char-udp.c b/chardev/char-udp.c index 804bd22..12240e5 100644 --- a/chardev/char-udp.c +++ b/chardev/char-udp.c @@ -81,7 +81,7 @@ static gboolean udp_chr_read(QIOChannel *chan, GIOCondition cond, void *opaque) ret = qio_channel_read( s->ioc, (char *)s->buf, sizeof(s->buf), NULL); if (ret <= 0) { - remove_fd_in_watch(chr, NULL); + remove_fd_in_watch(chr); return FALSE; } s->bufcnt = ret; @@ -101,9 +101,9 @@ static void udp_chr_update_read_handler(Chardev *chr, { UdpChardev *s = UDP_CHARDEV(chr); - remove_fd_in_watch(chr, NULL); + remove_fd_in_watch(chr); if (s->ioc) { - chr->fd_in_tag = io_add_watch_poll(chr, s->ioc, + chr->gsource = io_add_watch_poll(chr, s->ioc, udp_chr_read_poll, udp_chr_read, chr, context); @@ -115,7 +115,7 @@ static void char_udp_finalize(Object *obj) Chardev *chr = CHARDEV(obj); UdpChardev *s = UDP_CHARDEV(obj); - remove_fd_in_watch(chr, NULL); + remove_fd_in_watch(chr); if (s->ioc) { object_unref(OBJECT(s->ioc)); } diff --git a/chardev/char.c b/chardev/char.c index 3df1163..54cd5f4 100644 --- a/chardev/char.c +++ b/chardev/char.c @@ -560,7 +560,7 @@ void qemu_chr_fe_set_handlers(CharBackend *b, cc = CHARDEV_GET_CLASS(s); if (!opaque && !fd_can_read && !fd_read && !fd_event) { fe_open = 0; - remove_fd_in_watch(s, context); + remove_fd_in_watch(s); } else { fe_open = 1; } diff --git a/include/sysemu/char.h b/include/sysemu/char.h index 450881d..84f5c23 100644 --- a/include/sysemu/char.h +++ b/include/sysemu/char.h @@ -93,7 +93,7 @@ struct Chardev { char *filename; int logfd; int be_open; - guint fd_in_tag; + GSource *gsource; DECLARE_BITMAP(features, QEMU_CHAR_FEATURE_LAST); QTAILQ_ENTRY(Chardev) next; }; -- cgit v1.1 From 00fcd100c3f47445f6a59d39e11601460880cfe4 Mon Sep 17 00:00:00 2001 From: Abdallah Bouassida Date: Wed, 28 Dec 2016 17:34:02 +0100 Subject: target/i386: Add GDB XML register description support This patch implements XML target description support for X86 and X86-64 architectures in the GDB stub, as the way with ARM and PowerPC: - gdb-xml/32bit-core.xml & gdb-xml/64bit-core.xml: Adding the XML target description files, these files are picked from GDB source code. - configure: Define gdb_xml_files for X86 targets. - target/i386/cpu.c: Define gdb_core_xml_file and gdb_arch_name to add XML awareness for this architecture, modify the gdb_num_core_regs to fit the registers number defined in each XML file. Signed-off-by: Abdallah Bouassida Message-Id: <2b3c8119-1602-28c7-eab4-296593877103@lauterbach.com> Signed-off-by: Paolo Bonzini --- configure | 2 ++ gdb-xml/i386-32bit-core.xml | 65 ++++++++++++++++++++++++++++++++++++++++ gdb-xml/i386-64bit-core.xml | 73 +++++++++++++++++++++++++++++++++++++++++++++ target/i386/cpu.c | 21 ++++++++++--- 4 files changed, 157 insertions(+), 4 deletions(-) create mode 100644 gdb-xml/i386-32bit-core.xml create mode 100644 gdb-xml/i386-64bit-core.xml diff --git a/configure b/configure index 48a9370..ff2c81f 100755 --- a/configure +++ b/configure @@ -6023,9 +6023,11 @@ TARGET_ABI_DIR="" case "$target_name" in i386) + gdb_xml_files="i386-32bit-core.xml" ;; x86_64) TARGET_BASE_ARCH=i386 + gdb_xml_files="i386-64bit-core.xml" ;; alpha) mttcg="yes" diff --git a/gdb-xml/i386-32bit-core.xml b/gdb-xml/i386-32bit-core.xml new file mode 100644 index 0000000..7aeeeca --- /dev/null +++ b/gdb-xml/i386-32bit-core.xml @@ -0,0 +1,65 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/gdb-xml/i386-64bit-core.xml b/gdb-xml/i386-64bit-core.xml new file mode 100644 index 0000000..5088d84 --- /dev/null +++ b/gdb-xml/i386-64bit-core.xml @@ -0,0 +1,73 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 13c0985..7e87031 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -2577,6 +2577,15 @@ out: return ret; } +static gchar *x86_gdb_arch_name(CPUState *cs) +{ +#ifdef TARGET_X86_64 + return g_strdup("i386:x86-64"); +#else + return g_strdup("i386"); +#endif +} + X86CPU *cpu_x86_init(const char *cpu_model) { return X86_CPU(cpu_generic_init(TYPE_X86_CPU, cpu_model)); @@ -4056,10 +4065,14 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data) cc->write_elf32_qemunote = x86_cpu_write_elf32_qemunote; cc->vmsd = &vmstate_x86_cpu; #endif - /* CPU_NB_REGS * 2 = general regs + xmm regs - * 25 = eip, eflags, 6 seg regs, st[0-7], fctrl,...,fop, mxcsr. - */ - cc->gdb_num_core_regs = CPU_NB_REGS * 2 + 25; + cc->gdb_arch_name = x86_gdb_arch_name; +#ifdef TARGET_X86_64 + cc->gdb_core_xml_file = "i386-64bit-core.xml"; + cc->gdb_num_core_regs = 40; +#else + cc->gdb_core_xml_file = "i386-32bit-core.xml"; + cc->gdb_num_core_regs = 32; +#endif #ifndef CONFIG_USER_ONLY cc->debug_excp_handler = breakpoint_handler; #endif -- cgit v1.1 From 49e00a18708e27c815828d9440d5c9300d19547c Mon Sep 17 00:00:00 2001 From: Andreas Grapentin Date: Tue, 14 Mar 2017 17:59:53 +0100 Subject: use _Static_assert in QEMU_BUILD_BUG_ON MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QEMU_BUILD_BUG_ON should use C11's _Static_assert, if the compiler supports it, to provide more readable messages on failure. We check for _Static_assert in configure, and set CONFIG_STATIC_ASSERT accordingly. QEMU_BUILD_BUG_ON invokes _Static_assert if CONFIG_STATIC_ASSERT is defined, and reverts to the old way otherwise. That way, systems without C11 conforming compiler will still have the old messages, as verified by intentionally breaking the configure check. the following example output was generated by inverting the condition in QEMU_BUILD_BUG_ON: without _Static_assert: > In file included from /qemu/include/qemu/osdep.h:36:0, > from /qemu/qga/commands.c:13: > /qemu/qga/commands.c: In function ‘qmp_guest_exec_status’: > /qemu/include/qemu/compiler.h:89:12: error: negative width in bit-field ‘’ > struct { \ > ^ > /qemu/include/qemu/compiler.h:96:38: note: in expansion of macro QEMU_BUILD_BUG_ON_STRUCT’ > #define QEMU_BUILD_BUG_ON(x) typedef QEMU_BUILD_BUG_ON_STRUCT(x) \ > ^~~~~~~~~~~~~~~~~~~~~~~~ > /qemu/include/qemu/atomic.h:146:5: note: in expansion of macro ‘QEMU_BUILD_BUG_ON’ > QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ > ^~~~~~~~~~~~~~~~~ > /qemu/include/qemu/atomic.h:417:5: note: in expansion of macro ‘atomic_load_acquire’ > atomic_load_acquire(ptr) > ^~~~~~~~~~~~~~~~~~~ > /qemu/qga/commands.c:160:21: note: in expansion of macro ‘atomic_mb_read’ > bool finished = atomic_mb_read(&gei->finished); > ^~~~~~~~~~~~~~ with _Static_assert: > In file included from /qemu/include/qemu/osdep.h:36:0, > from /qemu/qga/commands.c:13: > /qemu/qga/commands.c: In function ‘qmp_guest_exec_status’: > /qemu/include/qemu/compiler.h:94:30: error: static assertion failed: "not expecting: sizeof(*&gei->finished) > sizeof(void *)" > #define QEMU_BUILD_BUG_ON(x) _Static_assert((x), #x) > ^ > /qemu/include/qemu/atomic.h:146:5: note: in expansion of macro ‘QEMU_BUILD_BUG_ON’ > QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ > ^~~~~~~~~~~~~~~~~ > /qemu/include/qemu/atomic.h:417:5: note: in expansion of macro ‘atomic_load_acquire’ > atomic_load_acquire(ptr) > ^~~~~~~~~~~~~~~~~~~ > /qemu/qga/commands.c:160:21: note: in expansion of macro ‘atomic_mb_read’ > bool finished = atomic_mb_read(&gei->finished); > ^~~~~~~~~~~~~~ Signed-off-by: Andreas Grapentin Message-Id: <20170314165953.18506-1-andreas@grapentin.org> Signed-off-by: Paolo Bonzini --- configure | 18 ++++++++++++++++++ include/qemu/compiler.h | 4 +++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/configure b/configure index ff2c81f..7c020c0 100755 --- a/configure +++ b/configure @@ -4853,6 +4853,20 @@ EOF fi ########################################## +# check for _Static_assert() + +have_static_assert=no +cat > $TMPC << EOF +_Static_assert(1, "success"); +int main(void) { + return 0; +} +EOF +if compile_prog "" "" ; then + have_static_assert=yes +fi + +########################################## # End of CC checks # After here, no more $cc or $ld runs @@ -5848,6 +5862,10 @@ if test "$have_sysmacros" = "yes" ; then echo "CONFIG_SYSMACROS=y" >> $config_host_mak fi +if test "$have_static_assert" = "yes" ; then + echo "CONFIG_STATIC_ASSERT=y" >> $config_host_mak +fi + # Hold two types of flag: # CONFIG_THREAD_SETNAME_BYTHREAD - we've got a way of setting the name on # a thread we have a handle to diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h index 18e6100..340e5fd 100644 --- a/include/qemu/compiler.h +++ b/include/qemu/compiler.h @@ -82,7 +82,9 @@ int:(x) ? -1 : 1; \ } -#ifdef __COUNTER__ +#if defined(CONFIG_STATIC_ASSERT) +#define QEMU_BUILD_BUG_ON(x) _Static_assert(!(x), "not expecting: " #x) +#elif defined(__COUNTER__) #define QEMU_BUILD_BUG_ON(x) typedef QEMU_BUILD_BUG_ON_STRUCT(x) \ glue(qemu_build_bug_on__, __COUNTER__) __attribute__((unused)) #else -- cgit v1.1 From aab9e87e7a5d280bfbffaa4ea6e296c515c8dac2 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Thu, 27 Apr 2017 07:20:54 +0200 Subject: vl: deprecate the "-hdachs" option If the user needs to specify the disk geometry, the corresponding parameters of the "-device ide-hd" option should be used instead. "-hdachs" is considered as deprecated and might be removed soon. Signed-off-by: Thomas Huth Message-Id: <1493270454-1448-1-git-send-email-thuth@redhat.com> Signed-off-by: Paolo Bonzini --- qemu-options.hx | 4 ++-- vl.c | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index 787b9c3..f68829f 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -803,8 +803,8 @@ STEXI Force hard disk 0 physical geometry (1 <= @var{c} <= 16383, 1 <= @var{h} <= 16, 1 <= @var{s} <= 63) and optionally force the BIOS translation mode (@var{t}=none, lba or auto). Usually QEMU can guess -all those parameters. This option is useful for old MS-DOS disk -images. +all those parameters. This option is deprecated, please use +@code{-device ide-hd,cyls=c,heads=h,secs=s,...} instead. ETEXI DEF("fsdev", HAS_ARG, QEMU_OPTION_fsdev, diff --git a/vl.c b/vl.c index f46e070..42d4bce 100644 --- a/vl.c +++ b/vl.c @@ -3231,6 +3231,8 @@ int main(int argc, char **argv, char **envp) } } } + error_report("'-hdachs' is deprecated, please use '-device" + " ide-hd,cyls=c,heads=h,secs=s,...' instead"); break; case QEMU_OPTION_numa: opts = qemu_opts_parse_noisily(qemu_find_opts("numa"), -- cgit v1.1 From 24dfa9fa2f90a95ac33c7372de4f4f2c8a2c141f Mon Sep 17 00:00:00 2001 From: Prasad J Pandit Date: Mon, 24 Apr 2017 17:36:34 +0530 Subject: scsi: avoid an off-by-one error in megasas_mmio_write While reading magic sequence(MFI_SEQ) in megasas_mmio_write, an off-by-one error could occur as 's->adp_reset' index is not reset after reading the last sequence. Reported-by: YY Z Signed-off-by: Prasad J Pandit Message-Id: <20170424120634.12268-1-ppandit@redhat.com> Signed-off-by: Paolo Bonzini --- hw/scsi/megasas.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c index 84b8caf..804122a 100644 --- a/hw/scsi/megasas.c +++ b/hw/scsi/megasas.c @@ -2138,15 +2138,15 @@ static void megasas_mmio_write(void *opaque, hwaddr addr, case MFI_SEQ: trace_megasas_mmio_writel("MFI_SEQ", val); /* Magic sequence to start ADP reset */ - if (adp_reset_seq[s->adp_reset] == val) { - s->adp_reset++; + if (adp_reset_seq[s->adp_reset++] == val) { + if (s->adp_reset == 6) { + s->adp_reset = 0; + s->diag = MFI_DIAG_WRITE_ENABLE; + } } else { s->adp_reset = 0; s->diag = 0; } - if (s->adp_reset == 6) { - s->diag = MFI_DIAG_WRITE_ENABLE; - } break; case MFI_DIAG: trace_megasas_mmio_writel("MFI_DIAG", val); -- cgit v1.1 From c8c33fca88fc3ab8a5e4bb2e34d26a97e9ba8e5a Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 27 Apr 2017 10:39:14 +0200 Subject: sgabios: update for "fix wrong video attrs for int 10h,ah==13h" Update the submodule and rebuild the binary. Signed-off-by: Paolo Bonzini --- pc-bios/sgabios.bin | Bin 4096 -> 4096 bytes roms/sgabios | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/pc-bios/sgabios.bin b/pc-bios/sgabios.bin index c3da4c3..6308f2e 100644 Binary files a/pc-bios/sgabios.bin and b/pc-bios/sgabios.bin differ diff --git a/roms/sgabios b/roms/sgabios index 23d4749..cbaee52 160000 --- a/roms/sgabios +++ b/roms/sgabios @@ -1 +1 @@ -Subproject commit 23d474943dcd55d0550a3d20b3d30e9040a4f15b +Subproject commit cbaee52287e5f32373181cff50a00b6c4ac9015a -- cgit v1.1 From f68826989cd4d1217797251339579c57b3c0934e Mon Sep 17 00:00:00 2001 From: P J P Date: Tue, 25 Apr 2017 18:36:23 +0530 Subject: vmw_pvscsi: check message ring page count at initialisation A guest could set the message ring page count to zero, resulting in infinite loop. Add check to avoid it. Reported-by: YY Z Signed-off-by: P J P Message-Id: <20170425130623.3649-1-ppandit@redhat.com> Reviewed-by: Dmitry Fleytman Signed-off-by: Paolo Bonzini --- hw/scsi/vmw_pvscsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c index 7557546..4a106da 100644 --- a/hw/scsi/vmw_pvscsi.c +++ b/hw/scsi/vmw_pvscsi.c @@ -202,7 +202,7 @@ pvscsi_ring_init_msg(PVSCSIRingInfo *m, PVSCSICmdDescSetupMsgRing *ri) uint32_t len_log2; uint32_t ring_size; - if (ri->numPages > PVSCSI_SETUP_MSG_RING_MAX_NUM_PAGES) { + if (!ri->numPages || ri->numPages > PVSCSI_SETUP_MSG_RING_MAX_NUM_PAGES) { return -1; } ring_size = ri->numPages * PVSCSI_MAX_NUM_MSG_ENTRIES_PER_PAGE; -- cgit v1.1 From 31f5a726b59bda5580e2f9413867893501dd7d93 Mon Sep 17 00:00:00 2001 From: Jose Ricardo Ziviani Date: Mon, 24 Apr 2017 14:19:58 -0300 Subject: trace: add qemu mutex lock and unlock trace events These trace events were very useful to help me to understand and find a reordering issue in vfio, for example: qemu_mutex_lock locked mutex 0x10905ad8 vfio_region_write (0001:03:00.0:region1+0xc0, 0x2020c, 4) qemu_mutex_unlock unlocked mutex 0x10905ad8 qemu_mutex_lock locked mutex 0x10905ad8 vfio_region_write (0001:03:00.0:region1+0xc4, 0xa0000, 4) qemu_mutex_unlock unlocked mutex 0x10905ad8 that also helped me to see the desired result after the fix: qemu_mutex_lock locked mutex 0x10905ad8 vfio_region_write (0001:03:00.0:region1+0xc0, 0x2000c, 4) vfio_region_write (0001:03:00.0:region1+0xc4, 0xb0000, 4) qemu_mutex_unlock unlocked mutex 0x10905ad8 So it could be a good idea to have these traces implemented. It's worth mentioning that they should be surgically enabled during the debugging, otherwise it can flood the trace logs with lock/unlock messages. How to use it: trace-event qemu_mutex_lock on|off trace-event qemu_mutex_unlock on|off or trace-event qemu_mutex* on|off Signed-off-by: Jose Ricardo Ziviani Message-Id: <1493054398-26013-1-git-send-email-joserz@linux.vnet.ibm.com> Reviewed-by: Fam Zheng [Also handle trylock, cond_wait and win32; trace "unlocked" while still in the critical section, so that "unlocked" always comes before the next "locked" tracepoint. - Paolo] Signed-off-by: Paolo Bonzini --- util/qemu-thread-posix.c | 18 +++++++++++++++++- util/qemu-thread-win32.c | 11 ++++++++++- util/trace-events | 4 ++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c index 73e3a0e..eacd99e 100644 --- a/util/qemu-thread-posix.c +++ b/util/qemu-thread-posix.c @@ -14,6 +14,7 @@ #include "qemu/thread.h" #include "qemu/atomic.h" #include "qemu/notify.h" +#include "trace.h" static bool name_threads; @@ -60,17 +61,30 @@ void qemu_mutex_lock(QemuMutex *mutex) err = pthread_mutex_lock(&mutex->lock); if (err) error_exit(err, __func__); + + trace_qemu_mutex_locked(mutex); } int qemu_mutex_trylock(QemuMutex *mutex) { - return pthread_mutex_trylock(&mutex->lock); + int err; + + err = pthread_mutex_trylock(&mutex->lock); + if (err == 0) { + trace_qemu_mutex_locked(mutex); + return 0; + } + if (err != EBUSY) { + error_exit(err, __func__); + } + return -EBUSY; } void qemu_mutex_unlock(QemuMutex *mutex) { int err; + trace_qemu_mutex_unlocked(mutex); err = pthread_mutex_unlock(&mutex->lock); if (err) error_exit(err, __func__); @@ -130,7 +144,9 @@ void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex) { int err; + trace_qemu_mutex_unlocked(mutex); err = pthread_cond_wait(&cond->cond, &mutex->lock); + trace_qemu_mutex_locked(mutex); if (err) error_exit(err, __func__); } diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c index 59befd5..653f29f 100644 --- a/util/qemu-thread-win32.c +++ b/util/qemu-thread-win32.c @@ -19,6 +19,7 @@ #include "qemu-common.h" #include "qemu/thread.h" #include "qemu/notify.h" +#include "trace.h" #include static bool name_threads; @@ -55,6 +56,7 @@ void qemu_mutex_destroy(QemuMutex *mutex) void qemu_mutex_lock(QemuMutex *mutex) { AcquireSRWLockExclusive(&mutex->lock); + trace_qemu_mutex_locked(mutex); } int qemu_mutex_trylock(QemuMutex *mutex) @@ -62,11 +64,16 @@ int qemu_mutex_trylock(QemuMutex *mutex) int owned; owned = TryAcquireSRWLockExclusive(&mutex->lock); - return !owned; + if (owned) { + trace_qemu_mutex_locked(mutex); + return 0; + } + return -EBUSY; } void qemu_mutex_unlock(QemuMutex *mutex) { + trace_qemu_mutex_unlocked(mutex); ReleaseSRWLockExclusive(&mutex->lock); } @@ -118,7 +125,9 @@ void qemu_cond_broadcast(QemuCond *cond) void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex) { + trace_qemu_mutex_unlocked(mutex); SleepConditionVariableSRW(&cond->var, &mutex->lock, INFINITE, 0); + trace_qemu_mutex_locked(mutex); } void qemu_sem_init(QemuSemaphore *sem, int init) diff --git a/util/trace-events b/util/trace-events index b44ef4f..fa540c6 100644 --- a/util/trace-events +++ b/util/trace-events @@ -55,3 +55,7 @@ lockcnt_futex_wait_prepare(const void *lockcnt, int expected, int new) "lockcnt lockcnt_futex_wait(const void *lockcnt, int val) "lockcnt %p waiting on %d" lockcnt_futex_wait_resume(const void *lockcnt, int new) "lockcnt %p after wait: %d" lockcnt_futex_wake(const void *lockcnt) "lockcnt %p waking up one waiter" + +# util/qemu-thread-posix.c +qemu_mutex_locked(void *lock) "locked mutex %p" +qemu_mutex_unlocked(void *lock) "unlocked mutex %p" -- cgit v1.1 From 6e9389563e56607f72562bdb72db452fcd7e7f74 Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Thu, 27 Apr 2017 17:55:26 +0100 Subject: checkpatch: Disallow glib asserts in main code Glib commit a6a875068779 (from 2013) made many of the glib assert macros non-fatal if a flag is set. This causes two problems: a) Compilers moan that your code is unsafe even though you've put an assert in before the point of use. b) Someone evil could, in a library, call g_test_set_nonfatal_assertions() and cause our assertions in important places not to fail and potentially allow memory overruns. Ban most of the glib assertion functions (basically everything except g_assert and g_assert_not_reached) except in tests/ This makes checkpatch gives an error such as: ERROR: Use g_assert or g_assert_not_reached #77: FILE: vl.c:4725: + g_assert_cmpstr("Chocolate", >, "Cheese"); Signed-off-by: Dr. David Alan Gilbert Message-Id: <20170427165526.19836-1-dgilbert@redhat.com> Signed-off-by: Paolo Bonzini --- scripts/checkpatch.pl | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index f084542..73cee81 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2571,6 +2571,27 @@ sub process { if ($line =~ /\bbzero\(/) { ERROR("use memset() instead of bzero()\n" . $herecurr); } + my $non_exit_glib_asserts = qr{g_assert_cmpstr| + g_assert_cmpint| + g_assert_cmpuint| + g_assert_cmphex| + g_assert_cmpfloat| + g_assert_true| + g_assert_false| + g_assert_nonnull| + g_assert_null| + g_assert_no_error| + g_assert_error| + g_test_assert_expected_messages| + g_test_trap_assert_passed| + g_test_trap_assert_stdout| + g_test_trap_assert_stdout_unmatched| + g_test_trap_assert_stderr| + g_test_trap_assert_stderr_unmatched}x; + if ($realfile !~ /^tests\// && + $line =~ /\b(?:$non_exit_glib_asserts)\(/) { + ERROR("Use g_assert or g_assert_not_reached\n". $herecurr); + } } # If we have no input at all, then there is nothing to report on -- cgit v1.1 From 8a3c3d996ef7df6982d6f51bb79893036ab08c7f Mon Sep 17 00:00:00 2001 From: Yu Ning Date: Fri, 28 Apr 2017 15:27:23 +0800 Subject: hax: Fix memory mapping de-duplication logic hax_update_mapping() avoids unnecessary and potentially expensive calls to HAX_VM_IOCTL_SET_RAM by computing the net result (i.e. effective mapping changes) of each MemoryRegion transaction, with the help of a linked list of HAXMapping objects. However, when processing a new mapping that overlaps with an existing mapping in the list, it fails to handle the case where the start address of the new mapping is above that of the existing mapping in the guest physical address space. This happens when QEMU is launched with "-machine q35 -enable-hax", which involves the following MemoryRegion transaction for digging the VGA hole: region_del: 0x00000000->0x08000000 VA 05fa0000 ('pc.ram') region_add: 0x00000000->0x000a0000 VA 05fa0000 ('pc.ram') region_add: 0x000a0000->0x000c0000 VA 00000000 ('vga-lowmem') region_add: 0x000c0000->0x08000000 VA 06060000 ('pc.ram') where the third MemoryRegion is MMIO and is ignored. The current de-duplication logic handles the last MemoryRegion incorrectly and produces the following result: hax_mapping_dump_list updates: + 0x000c0000->0x08000000 VA 0x06060000 - 0x07fe0000->0x08000000 VA 0x0df80000 which is why VGA emulation does not work for Q35. With this patch, one can see VGA output as Q35 boots up. Note that Q35 support also requires a change to HAXM kernel module, which is not available in the current HAXM release (6.1.2). + Add a warning if the input MemoryRegion is a ROM device, which is not supported by HAXM kernel module at this time. Signed-off-by: Yu Ning Message-Id: <20170428072723.7036-1-yu.ning@linux.intel.com> Signed-off-by: Paolo Bonzini --- target/i386/hax-mem.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/target/i386/hax-mem.c b/target/i386/hax-mem.c index 2884040..af09034 100644 --- a/target/i386/hax-mem.c +++ b/target/i386/hax-mem.c @@ -106,10 +106,10 @@ static void hax_update_mapping(uint64_t start_pa, uint32_t size, uint64_t host_va, uint8_t flags) { uint64_t end_pa = start_pa + size; - uint32_t chunk_sz; HAXMapping *entry, *next; QTAILQ_FOREACH_SAFE(entry, &mappings, entry, next) { + uint32_t chunk_sz; if (start_pa >= entry->start_pa + entry->size) { continue; } @@ -121,7 +121,16 @@ static void hax_update_mapping(uint64_t start_pa, uint32_t size, start_pa += chunk_sz; host_va += chunk_sz; size -= chunk_sz; + } else if (start_pa > entry->start_pa) { + /* split the existing chunk at start_pa */ + chunk_sz = start_pa - entry->start_pa; + hax_insert_mapping_before(entry, entry->start_pa, chunk_sz, + entry->host_va, entry->flags); + entry->start_pa += chunk_sz; + entry->host_va += chunk_sz; + entry->size -= chunk_sz; } + /* now start_pa == entry->start_pa */ chunk_sz = MIN(size, entry->size); if (chunk_sz) { bool nop = hax_mapping_is_opposite(entry, host_va, flags); @@ -165,8 +174,14 @@ static void hax_process_section(MemoryRegionSection *section, uint8_t flags) unsigned int delta; uint64_t host_va; - /* We only care about RAM pages */ + /* We only care about RAM and ROM regions */ if (!memory_region_is_ram(mr)) { + if (memory_region_is_romd(mr)) { + /* HAXM kernel module does not support ROMD yet */ + fprintf(stderr, "%s: Warning: Ignoring ROMD region 0x%016" PRIx64 + "->0x%016" PRIx64 "\n", __func__, start_pa, + start_pa + size); + } return; } -- cgit v1.1 From 6796b4008bd195e6765a1144dc98a8673e69d169 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 3 May 2017 15:28:19 +0800 Subject: dump: Acquire BQL around vm_start() in dump thread This fixes an assertion failure in the following backtrace: __GI___assert_fail memory_region_transaction_commit memory_region_add_eventfd virtio_pci_ioeventfd_assign virtio_bus_set_host_notifier virtio_blk_data_plane_start virtio_bus_start_ioeventfd virtio_vmstate_change vm_state_notify vm_prepare_start vm_start dump_cleanup dump_process dump_thread start_thread clone vm_start need BQL, acquire it if doing cleaning up from main thread. Signed-off-by: Fam Zheng Message-Id: <20170503072819.14462-1-famz@redhat.com> Signed-off-by: Paolo Bonzini --- dump.c | 7 +++++++ include/sysemu/dump.h | 1 + 2 files changed, 8 insertions(+) diff --git a/dump.c b/dump.c index f7b80d8..d9090a2 100644 --- a/dump.c +++ b/dump.c @@ -77,7 +77,13 @@ static int dump_cleanup(DumpState *s) memory_mapping_list_free(&s->list); close(s->fd); if (s->resume) { + if (s->detached) { + qemu_mutex_lock_iothread(); + } vm_start(); + if (s->detached) { + qemu_mutex_unlock_iothread(); + } } return 0; @@ -1804,6 +1810,7 @@ void qmp_dump_guest_memory(bool paging, const char *file, if (detach_p) { /* detached dump */ + s->detached = true; qemu_thread_create(&s->dump_thread, "dump_thread", dump_thread, s, QEMU_THREAD_DETACHED); } else { diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h index ef931be..2672a15 100644 --- a/include/sysemu/dump.h +++ b/include/sysemu/dump.h @@ -157,6 +157,7 @@ typedef struct DumpState { uint32_t sh_info; bool have_section; bool resume; + bool detached; ssize_t note_size; hwaddr memory_offset; int fd; -- cgit v1.1 From bde4d9205ee9def98852ff6054cdef4efd74e1f8 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Thu, 4 May 2017 07:24:41 +0200 Subject: Fix the -accel parameter and the documentation for 'hax' Since 'hax' is a possible accelerator nowadays, too, the '-accel' option should support it and we should mention this accelerator in the documentation, too. Signed-off-by: Thomas Huth Message-Id: <1493875481-16388-1-git-send-email-thuth@redhat.com> Signed-off-by: Paolo Bonzini --- qemu-options.hx | 18 +++++++++--------- vl.c | 23 +++++++++-------------- 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index f68829f..70c0ded 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -31,7 +31,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ "-machine [type=]name[,prop[=value][,...]]\n" " selects emulated machine ('-machine help' for list)\n" " property accel=accel1[:accel2[:...]] selects accelerator\n" - " supported accelerators are kvm, xen, tcg (default: tcg)\n" + " supported accelerators are kvm, xen, hax or tcg (default: tcg)\n" " kernel_irqchip=on|off|split controls accelerated irqchip support (default=off)\n" " vmport=on|off|auto controls emulation of vmport (default: auto)\n" " kvm_shadow_mem=size of KVM shadow MMU in bytes\n" @@ -52,9 +52,9 @@ available machines. Supported machine properties are: @table @option @item accel=@var{accels1}[:@var{accels2}[:...]] This is used to enable an accelerator. Depending on the target architecture, -kvm, xen, or tcg can be available. By default, tcg is used. If there is more -than one accelerator specified, the next one is used if the previous one fails -to initialize. +kvm, xen, hax or tcg can be available. By default, tcg is used. If there is +more than one accelerator specified, the next one is used if the previous one +fails to initialize. @item kernel_irqchip=on|off Controls in-kernel irqchip support for the chosen accelerator when available. @item gfx_passthru=on|off @@ -97,15 +97,15 @@ ETEXI DEF("accel", HAS_ARG, QEMU_OPTION_accel, "-accel [accel=]accelerator[,thread=single|multi]\n" - " select accelerator ('-accel help for list')\n" - " thread=single|multi (enable multi-threaded TCG)", QEMU_ARCH_ALL) + " select accelerator (kvm, xen, hax or tcg; use 'help' for a list)\n" + " thread=single|multi (enable multi-threaded TCG)", QEMU_ARCH_ALL) STEXI @item -accel @var{name}[,prop=@var{value}[,...]] @findex -accel This is used to enable an accelerator. Depending on the target architecture, -kvm, xen, or tcg can be available. By default, tcg is used. If there is more -than one accelerator specified, the next one is used if the previous one fails -to initialize. +kvm, xen, hax or tcg can be available. By default, tcg is used. If there is +more than one accelerator specified, the next one is used if the previous one +fails to initialize. @table @option @item thread=single|multi Controls number of TCG threads. When the TCG is multi-threaded there will be one diff --git a/vl.c b/vl.c index 42d4bce..17b9a8a 100644 --- a/vl.c +++ b/vl.c @@ -3727,26 +3727,21 @@ int main(int argc, char **argv, char **envp) qdev_prop_register_global(&kvm_pit_lost_tick_policy); break; } - case QEMU_OPTION_accel: + case QEMU_OPTION_accel: { + QemuOpts *accel_opts; + accel_opts = qemu_opts_parse_noisily(qemu_find_opts("accel"), optarg, true); optarg = qemu_opt_get(accel_opts, "accel"); - - olist = qemu_find_opts("machine"); - if (strcmp("kvm", optarg) == 0) { - qemu_opts_parse_noisily(olist, "accel=kvm", false); - } else if (strcmp("xen", optarg) == 0) { - qemu_opts_parse_noisily(olist, "accel=xen", false); - } else if (strcmp("tcg", optarg) == 0) { - qemu_opts_parse_noisily(olist, "accel=tcg", false); - } else { - if (!is_help_option(optarg)) { - error_printf("Unknown accelerator: %s", optarg); - } - error_printf("Supported accelerators: kvm, xen, tcg\n"); + if (!optarg || is_help_option(optarg)) { + error_printf("Possible accelerators: kvm, xen, hax, tcg\n"); exit(1); } + accel_opts = qemu_opts_create(qemu_find_opts("machine"), NULL, + false, &error_abort); + qemu_opt_set(accel_opts, "accel", optarg, &error_abort); break; + } case QEMU_OPTION_usb: olist = qemu_find_opts("machine"); qemu_opts_parse_noisily(olist, "usb=on", false); -- cgit v1.1 From fdf6fab4df40cbd26a216c31379587310d1c4001 Mon Sep 17 00:00:00 2001 From: "Paul E. McKenney" Date: Mon, 2 Jun 2014 09:06:37 -0700 Subject: MAINTAINERS: Add "R:" tag for self-appointed reviewers Some people are not content with the amount of mail they get, and would like to be CCed on patches for areas they do not maintain. Let them satisfy their own appetite for qemu-devel messages. Seriously: the purpose here is a bit different from the Linux kernel. While Linux uses "R" to designate non-maintainers for reviewing patches in a given area, in QEMU I would also like to use "R" so that people can delegate sending pull requests while keeping some degree of oversight. Based on Linux commit eafbaac3093760d1fd3b2a5b9f016362dd68af36. Signed-off-by: Paul E. McKenney Reviewed-by: Thomas Huth Signed-off-by: Paolo Bonzini --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index a1d2b3a..46cf487 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12,6 +12,8 @@ consult qemu-devel and not any specific individual privately. Descriptions of section entries: M: Mail patches to: FullName + R: Designated reviewer: FullName + These reviewers should be CCed on patches. L: Mailing list that is relevant to this area W: Web-page with status/info Q: Patchwork web based patch tracking system site -- cgit v1.1 From 6668a2af21102ef472fcc9dbf816b2128909096d Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Mon, 2 Jun 2014 12:05:17 -0700 Subject: get_maintainer: Teach get_maintainer.pl about the new "R:" tag We can now designate reviewers in the MAINTAINERS file with the new "R:" tag, so this commit teaches get_maintainers.pl to add their email addresses. Cherry picked from Linux commit c1c3f2c906e35bcb6e4cdf5b8e077660fead14fe, with fixes to avoid \C as in QEMU commit ba10f729f1 ("get_maintainer.pl: \C is deprecated", 2015-09-25). Signed-off-by: Joe Perches Reviewed-by: Thomas Huth Signed-off-by: Paolo Bonzini --- scripts/get_maintainer.pl | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl index 8261bcb..c1146ca 100755 --- a/scripts/get_maintainer.pl +++ b/scripts/get_maintainer.pl @@ -21,6 +21,7 @@ my $lk_path = "./"; my $email = 1; my $email_usename = 1; my $email_maintainer = 1; +my $email_reviewer = 1; my $email_list = 1; my $email_subscriber_list = 0; my $email_git = 0; @@ -180,6 +181,7 @@ if (!GetOptions( 'remove-duplicates!' => \$email_remove_duplicates, 'mailmap!' => \$email_use_mailmap, 'm!' => \$email_maintainer, + 'r!' => \$email_reviewer, 'n!' => \$email_usename, 'l!' => \$email_list, 's!' => \$email_subscriber_list, @@ -238,7 +240,8 @@ if ($sections) { } if ($email && - ($email_maintainer + $email_list + $email_subscriber_list + + ($email_maintainer + $email_reviewer + + $email_list + $email_subscriber_list + $email_git + $email_git_blame) == 0) { die "$P: Please select at least 1 email option\n"; } @@ -718,6 +721,7 @@ MAINTAINER field selection options: --hg-since => hg history to use (default: $email_hg_since) --interactive => display a menu (mostly useful if used with the --git option) --m => include maintainer(s) if any + --r => include reviewer(s) if any --n => include name 'Full Name ' --l => include list(s) if any --s => include subscriber only list(s) if any @@ -1022,6 +1026,22 @@ sub add_categories { my $role = get_maintainer_role($i); push_email_addresses($pvalue, $role); } + } elsif ($ptype eq "R") { + my ($name, $address) = parse_email($pvalue); + if ($name eq "") { + if ($i > 0) { + my $tv = $typevalue[$i - 1]; + if ($tv =~ m/^(.):\s*(.*)/) { + if ($1 eq "P") { + $name = $2; + $pvalue = format_email($name, $address, $email_usename); + } + } + } + } + if ($email_reviewer) { + push_email_addresses($pvalue, 'reviewer'); + } } elsif ($ptype eq "T") { push(@scm, $pvalue); } elsif ($ptype eq "W") { -- cgit v1.1 From 7a6ae2cffc35b246269fde69f7a76191d7d5e9cd Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Wed, 3 May 2017 11:16:44 +0200 Subject: get_maintainer: it's '--pattern-depth', not '-pattern-depth' Though it appears that Perl's GetOptions will take either, the latter is not documented in the options listing. Cherry picked from Linux commit cc7ff0ef6eca3deeea4a424ca47a67c8450d5424. Signed-off-by: Brian Norris Reviewed-by: Thomas Huth Signed-off-by: Paolo Bonzini --- scripts/get_maintainer.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl index c1146ca..1f637ff 100755 --- a/scripts/get_maintainer.pl +++ b/scripts/get_maintainer.pl @@ -748,7 +748,7 @@ Other options: --help => show this help information Default options: - [--email --nogit --git-fallback --m --n --l --multiline -pattern-depth=0 + [--email --nogit --git-fallback --m --n --l --multiline --pattern-depth=0 --remove-duplicates --rolestats] Notes: -- cgit v1.1 From 9ff3a5e677378e9cd1a716943e48164d2ca279ca Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Fri, 6 Nov 2015 16:30:49 -0800 Subject: get_maintainer: --r (list reviewer) is on by default We don't consistenly document the default value next to the option listing, but we do have a list of defaults here, so let's keep it up to date. Cherry picked from Linux commit 4f07510df2e8c47fd65b8ffaaf6c5d334d59d598. Signed-off-by: Brian Norris Reviewed-by: Thomas Huth Signed-off-by: Paolo Bonzini --- scripts/get_maintainer.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl index 1f637ff..c7cb197 100755 --- a/scripts/get_maintainer.pl +++ b/scripts/get_maintainer.pl @@ -748,7 +748,7 @@ Other options: --help => show this help information Default options: - [--email --nogit --git-fallback --m --n --l --multiline --pattern-depth=0 + [--email --nogit --git-fallback --m --r --n --l --multiline --pattern-depth=0 --remove-duplicates --rolestats] Notes: -- cgit v1.1 From 622e42a71f6f6a928b1f2b9d95aff35b47d8b13c Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Fri, 6 Nov 2015 16:30:52 -0800 Subject: get_maintainer: add subsystem to reviewer output Reviewer output currently does not include the subsystem that matched. Add it. Miscellanea: o Add a get_subsystem_name routine to centralize this Cherry picked from Linux commit 2a7cb1dc82fc2a52e747b4c496c13f6575fb1790. Signed-off-by: Joe Perches Reviewed-by: Thomas Huth Signed-off-by: Paolo Bonzini --- scripts/get_maintainer.pl | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl index c7cb197..96e66a8 100755 --- a/scripts/get_maintainer.pl +++ b/scripts/get_maintainer.pl @@ -896,20 +896,29 @@ sub find_ending_index { return $index; } -sub get_maintainer_role { +sub get_subsystem_name { my ($index) = @_; - my $i; my $start = find_starting_index($index); - my $end = find_ending_index($index); - my $role = "unknown"; my $subsystem = $typevalue[$start]; if (length($subsystem) > 20) { $subsystem = substr($subsystem, 0, 17); $subsystem =~ s/\s*$//; $subsystem = $subsystem . "..."; } + return $subsystem; +} + +sub get_maintainer_role { + my ($index) = @_; + + my $i; + my $start = find_starting_index($index); + my $end = find_ending_index($index); + + my $role = "unknown"; + my $subsystem = get_subsystem_name($index); for ($i = $start + 1; $i < $end; $i++) { my $tv = $typevalue[$i]; @@ -943,16 +952,7 @@ sub get_maintainer_role { sub get_list_role { my ($index) = @_; - my $i; - my $start = find_starting_index($index); - my $end = find_ending_index($index); - - my $subsystem = $typevalue[$start]; - if (length($subsystem) > 20) { - $subsystem = substr($subsystem, 0, 17); - $subsystem =~ s/\s*$//; - $subsystem = $subsystem . "..."; - } + my $subsystem = get_subsystem_name($index); if ($subsystem eq "THE REST") { $subsystem = ""; @@ -1040,7 +1040,8 @@ sub add_categories { } } if ($email_reviewer) { - push_email_addresses($pvalue, 'reviewer'); + my $subsystem = get_subsystem_name($i); + push_email_addresses($pvalue, "reviewer:$subsystem"); } } elsif ($ptype eq "T") { push(@scm, $pvalue); -- cgit v1.1 From eae0f543341b311038fa2d40350688c646880082 Mon Sep 17 00:00:00 2001 From: Felipe Franciosi Date: Thu, 2 Mar 2017 10:25:50 -0800 Subject: libvhost-user: replace vasprintf() to fix build On gcc 3.4 and newer, simply using (void) in front of WUR functions is not sufficient to ignore the return value. That prevents a build when handling warnings as errors. libvhost-user had a usage of (void)vasprintf() which triggered such a condition. This fixes it by replacing this call with g_strdup_vprintf() which aborts on OOM. Signed-off-by: Felipe Franciosi Message-Id: <1488479153-21203-2-git-send-email-felipe@nutanix.com> Reviewed-by: Eric Blake --- contrib/libvhost-user/libvhost-user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c index af4faad..61e1657 100644 --- a/contrib/libvhost-user/libvhost-user.c +++ b/contrib/libvhost-user/libvhost-user.c @@ -81,7 +81,7 @@ vu_panic(VuDev *dev, const char *msg, ...) va_list ap; va_start(ap, msg); - (void)vasprintf(&buf, msg, ap); + buf = g_strdup_vprintf(msg, ap); va_end(ap); dev->broken = true; -- cgit v1.1 From 95615ce5a1beffff1a5dd3597d8cb6ba83f0010e Mon Sep 17 00:00:00 2001 From: Felipe Franciosi Date: Thu, 2 Mar 2017 10:25:51 -0800 Subject: vhost-scsi: create a vhost-scsi-common abstraction In order to introduce a new vhost-user-scsi host device type, it makes sense to abstract part of vhost-scsi into a common parent class. This commit does exactly that. Signed-off-by: Felipe Franciosi Message-Id: <1488479153-21203-3-git-send-email-felipe@nutanix.com> --- hw/scsi/Makefile.objs | 2 +- hw/scsi/vhost-scsi-common.c | 143 +++++++++++++++++++++++++ hw/scsi/vhost-scsi.c | 194 ++++++++++------------------------ include/hw/virtio/vhost-scsi-common.h | 48 +++++++++ include/hw/virtio/vhost-scsi.h | 11 +- include/hw/virtio/virtio-scsi.h | 2 + 6 files changed, 252 insertions(+), 148 deletions(-) create mode 100644 hw/scsi/vhost-scsi-common.c create mode 100644 include/hw/virtio/vhost-scsi-common.h diff --git a/hw/scsi/Makefile.objs b/hw/scsi/Makefile.objs index 5a2248b..54d8754 100644 --- a/hw/scsi/Makefile.objs +++ b/hw/scsi/Makefile.objs @@ -10,5 +10,5 @@ obj-$(CONFIG_PSERIES) += spapr_vscsi.o ifeq ($(CONFIG_VIRTIO),y) obj-y += virtio-scsi.o virtio-scsi-dataplane.o -obj-$(CONFIG_VHOST_SCSI) += vhost-scsi.o +obj-$(CONFIG_VHOST_SCSI) += vhost-scsi-common.o vhost-scsi.o endif diff --git a/hw/scsi/vhost-scsi-common.c b/hw/scsi/vhost-scsi-common.c new file mode 100644 index 0000000..e41c031 --- /dev/null +++ b/hw/scsi/vhost-scsi-common.c @@ -0,0 +1,143 @@ +/* + * vhost-scsi-common + * + * Copyright (c) 2016 Nutanix Inc. All rights reserved. + * + * Author: + * Felipe Franciosi + * + * This work is largely based on the "vhost-scsi" implementation by: + * Stefan Hajnoczi + * Nicholas Bellinger + * + * This work is licensed under the terms of the GNU LGPL, version 2 or later. + * See the COPYING.LIB file in the top-level directory. + * + */ + +#include "qemu/osdep.h" +#include +#include "qapi/error.h" +#include "qemu/error-report.h" +#include "migration/migration.h" +#include "hw/virtio/vhost.h" +#include "hw/virtio/vhost-scsi-common.h" +#include "hw/virtio/virtio-scsi.h" +#include "hw/virtio/virtio-bus.h" +#include "hw/virtio/virtio-access.h" +#include "hw/fw-path-provider.h" + +int vhost_scsi_common_start(VHostSCSICommon *vsc) +{ + int ret, i; + VirtIODevice *vdev = VIRTIO_DEVICE(vsc); + BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); + + if (!k->set_guest_notifiers) { + error_report("binding does not support guest notifiers"); + return -ENOSYS; + } + + ret = vhost_dev_enable_notifiers(&vsc->dev, vdev); + if (ret < 0) { + return ret; + } + + ret = k->set_guest_notifiers(qbus->parent, vsc->dev.nvqs, true); + if (ret < 0) { + error_report("Error binding guest notifier"); + goto err_host_notifiers; + } + + vsc->dev.acked_features = vdev->guest_features; + ret = vhost_dev_start(&vsc->dev, vdev); + if (ret < 0) { + error_report("Error start vhost dev"); + goto err_guest_notifiers; + } + + /* guest_notifier_mask/pending not used yet, so just unmask + * everything here. virtio-pci will do the right thing by + * enabling/disabling irqfd. + */ + for (i = 0; i < vsc->dev.nvqs; i++) { + vhost_virtqueue_mask(&vsc->dev, vdev, vsc->dev.vq_index + i, false); + } + + return ret; + +err_guest_notifiers: + k->set_guest_notifiers(qbus->parent, vsc->dev.nvqs, false); +err_host_notifiers: + vhost_dev_disable_notifiers(&vsc->dev, vdev); + return ret; +} + +void vhost_scsi_common_stop(VHostSCSICommon *vsc) +{ + VirtIODevice *vdev = VIRTIO_DEVICE(vsc); + BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); + int ret = 0; + + vhost_dev_stop(&vsc->dev, vdev); + + if (k->set_guest_notifiers) { + ret = k->set_guest_notifiers(qbus->parent, vsc->dev.nvqs, false); + if (ret < 0) { + error_report("vhost guest notifier cleanup failed: %d", ret); + } + } + assert(ret >= 0); + + vhost_dev_disable_notifiers(&vsc->dev, vdev); +} + +uint64_t vhost_scsi_common_get_features(VirtIODevice *vdev, uint64_t features, + Error **errp) +{ + VHostSCSICommon *vsc = VHOST_SCSI_COMMON(vdev); + + return vhost_get_features(&vsc->dev, vsc->feature_bits, features); +} + +void vhost_scsi_common_set_config(VirtIODevice *vdev, const uint8_t *config) +{ + VirtIOSCSIConfig *scsiconf = (VirtIOSCSIConfig *)config; + VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev); + + if ((uint32_t)virtio_ldl_p(vdev, &scsiconf->sense_size) != vs->sense_size || + (uint32_t)virtio_ldl_p(vdev, &scsiconf->cdb_size) != vs->cdb_size) { + error_report("vhost-scsi does not support changing the sense data and " + "CDB sizes"); + exit(1); + } +} + +/* + * Implementation of an interface to adjust firmware path + * for the bootindex property handling. + */ +char *vhost_scsi_common_get_fw_dev_path(FWPathProvider *p, BusState *bus, + DeviceState *dev) +{ + VHostSCSICommon *vsc = VHOST_SCSI_COMMON(dev); + /* format: /channel@channel/vhost-scsi@target,lun */ + return g_strdup_printf("/channel@%x/%s@%x,%x", vsc->channel, + qdev_fw_name(dev), vsc->target, vsc->lun); +} + +static const TypeInfo vhost_scsi_common_info = { + .name = TYPE_VHOST_SCSI_COMMON, + .parent = TYPE_VIRTIO_SCSI_COMMON, + .instance_size = sizeof(VHostSCSICommon), + .abstract = true, +}; + +static void virtio_register_types(void) +{ + type_register_static(&vhost_scsi_common_info); +} + +type_init(virtio_register_types) diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index f53bc17..8f53ac3 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -42,13 +42,14 @@ static const int kernel_feature_bits[] = { static int vhost_scsi_set_endpoint(VHostSCSI *s) { VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s); - const VhostOps *vhost_ops = s->dev.vhost_ops; + VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s); + const VhostOps *vhost_ops = vsc->dev.vhost_ops; struct vhost_scsi_target backend; int ret; memset(&backend, 0, sizeof(backend)); pstrcpy(backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->conf.wwpn); - ret = vhost_ops->vhost_scsi_set_endpoint(&s->dev, &backend); + ret = vhost_ops->vhost_scsi_set_endpoint(&vsc->dev, &backend); if (ret < 0) { return -errno; } @@ -58,130 +59,62 @@ static int vhost_scsi_set_endpoint(VHostSCSI *s) static void vhost_scsi_clear_endpoint(VHostSCSI *s) { VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s); + VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s); struct vhost_scsi_target backend; - const VhostOps *vhost_ops = s->dev.vhost_ops; + const VhostOps *vhost_ops = vsc->dev.vhost_ops; memset(&backend, 0, sizeof(backend)); pstrcpy(backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->conf.wwpn); - vhost_ops->vhost_scsi_clear_endpoint(&s->dev, &backend); + vhost_ops->vhost_scsi_clear_endpoint(&vsc->dev, &backend); } static int vhost_scsi_start(VHostSCSI *s) { - int ret, abi_version, i; - VirtIODevice *vdev = VIRTIO_DEVICE(s); - BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); - VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); - const VhostOps *vhost_ops = s->dev.vhost_ops; - - if (!k->set_guest_notifiers) { - error_report("binding does not support guest notifiers"); - return -ENOSYS; - } + int ret, abi_version; + VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s); + const VhostOps *vhost_ops = vsc->dev.vhost_ops; - ret = vhost_ops->vhost_scsi_get_abi_version(&s->dev, &abi_version); + ret = vhost_ops->vhost_scsi_get_abi_version(&vsc->dev, &abi_version); if (ret < 0) { return -errno; } if (abi_version > VHOST_SCSI_ABI_VERSION) { error_report("vhost-scsi: The running tcm_vhost kernel abi_version:" - " %d is greater than vhost_scsi userspace supports: %d, please" - " upgrade your version of QEMU", abi_version, + " %d is greater than vhost_scsi userspace supports: %d," + " please upgrade your version of QEMU", abi_version, VHOST_SCSI_ABI_VERSION); return -ENOSYS; } - ret = vhost_dev_enable_notifiers(&s->dev, vdev); + ret = vhost_scsi_common_start(vsc); if (ret < 0) { return ret; } - s->dev.acked_features = vdev->guest_features; - ret = vhost_dev_start(&s->dev, vdev); - if (ret < 0) { - error_report("Error start vhost dev"); - goto err_notifiers; - } - ret = vhost_scsi_set_endpoint(s); if (ret < 0) { - error_report("Error set vhost-scsi endpoint"); - goto err_vhost_stop; - } - - ret = k->set_guest_notifiers(qbus->parent, s->dev.nvqs, true); - if (ret < 0) { - error_report("Error binding guest notifier"); - goto err_endpoint; - } - - /* guest_notifier_mask/pending not used yet, so just unmask - * everything here. virtio-pci will do the right thing by - * enabling/disabling irqfd. - */ - for (i = 0; i < s->dev.nvqs; i++) { - vhost_virtqueue_mask(&s->dev, vdev, s->dev.vq_index + i, false); + error_report("Error setting vhost-scsi endpoint"); + vhost_scsi_common_stop(vsc); } return ret; - -err_endpoint: - vhost_scsi_clear_endpoint(s); -err_vhost_stop: - vhost_dev_stop(&s->dev, vdev); -err_notifiers: - vhost_dev_disable_notifiers(&s->dev, vdev); - return ret; } static void vhost_scsi_stop(VHostSCSI *s) { - VirtIODevice *vdev = VIRTIO_DEVICE(s); - BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); - VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); - int ret = 0; - - if (k->set_guest_notifiers) { - ret = k->set_guest_notifiers(qbus->parent, s->dev.nvqs, false); - if (ret < 0) { - error_report("vhost guest notifier cleanup failed: %d", ret); - } - } - assert(ret >= 0); + VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s); vhost_scsi_clear_endpoint(s); - vhost_dev_stop(&s->dev, vdev); - vhost_dev_disable_notifiers(&s->dev, vdev); -} - -static uint64_t vhost_scsi_get_features(VirtIODevice *vdev, - uint64_t features, - Error **errp) -{ - VHostSCSI *s = VHOST_SCSI(vdev); - - return vhost_get_features(&s->dev, kernel_feature_bits, features); -} - -static void vhost_scsi_set_config(VirtIODevice *vdev, - const uint8_t *config) -{ - VirtIOSCSIConfig *scsiconf = (VirtIOSCSIConfig *)config; - VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev); - - if ((uint32_t) virtio_ldl_p(vdev, &scsiconf->sense_size) != vs->sense_size || - (uint32_t) virtio_ldl_p(vdev, &scsiconf->cdb_size) != vs->cdb_size) { - error_report("vhost-scsi does not support changing the sense data and CDB sizes"); - exit(1); - } + vhost_scsi_common_stop(vsc); } static void vhost_scsi_set_status(VirtIODevice *vdev, uint8_t val) { - VHostSCSI *s = (VHostSCSI *)vdev; + VHostSCSI *s = VHOST_SCSI(vdev); + VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s); bool start = (val & VIRTIO_CONFIG_S_DRIVER_OK); - if (s->dev.started == start) { + if (vsc->dev.started == start) { return; } @@ -190,10 +123,7 @@ static void vhost_scsi_set_status(VirtIODevice *vdev, uint8_t val) ret = vhost_scsi_start(s); if (ret < 0) { - error_report("virtio-scsi: unable to start vhost: %s", - strerror(-ret)); - - /* There is no userspace virtio-scsi fallback so exit */ + error_report("unable to start vhost-scsi: %s", strerror(-ret)); exit(1); } } else { @@ -208,7 +138,7 @@ static void vhost_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq) static void vhost_scsi_realize(DeviceState *dev, Error **errp) { VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev); - VHostSCSI *s = VHOST_SCSI(dev); + VHostSCSICommon *vsc = VHOST_SCSI_COMMON(dev); Error *err = NULL; int vhostfd = -1; int ret; @@ -243,21 +173,21 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) goto close_fd; } - error_setg(&s->migration_blocker, + error_setg(&vsc->migration_blocker, "vhost-scsi does not support migration"); - migrate_add_blocker(s->migration_blocker, &err); + migrate_add_blocker(vsc->migration_blocker, &err); if (err) { error_propagate(errp, err); - error_free(s->migration_blocker); + error_free(vsc->migration_blocker); goto close_fd; } - s->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues; - s->dev.vqs = g_new(struct vhost_virtqueue, s->dev.nvqs); - s->dev.vq_index = 0; - s->dev.backend_features = 0; + vsc->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues; + vsc->dev.vqs = g_new(struct vhost_virtqueue, vsc->dev.nvqs); + vsc->dev.vq_index = 0; + vsc->dev.backend_features = 0; - ret = vhost_dev_init(&s->dev, (void *)(uintptr_t)vhostfd, + ret = vhost_dev_init(&vsc->dev, (void *)(uintptr_t)vhostfd, VHOST_BACKEND_TYPE_KERNEL, 0); if (ret < 0) { error_setg(errp, "vhost-scsi: vhost initialization failed: %s", @@ -266,16 +196,16 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) } /* At present, channel and lun both are 0 for bootable vhost-scsi disk */ - s->channel = 0; - s->lun = 0; + vsc->channel = 0; + vsc->lun = 0; /* Note: we can also get the minimum tpgt from kernel */ - s->target = vs->conf.boot_tpgt; + vsc->target = vs->conf.boot_tpgt; return; free_vqs: - migrate_del_blocker(s->migration_blocker); - g_free(s->dev.vqs); + migrate_del_blocker(vsc->migration_blocker); + g_free(vsc->dev.vqs); close_fd: close(vhostfd); return; @@ -284,42 +214,28 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) static void vhost_scsi_unrealize(DeviceState *dev, Error **errp) { VirtIODevice *vdev = VIRTIO_DEVICE(dev); - VHostSCSI *s = VHOST_SCSI(dev); + VHostSCSICommon *vsc = VHOST_SCSI_COMMON(dev); - migrate_del_blocker(s->migration_blocker); - error_free(s->migration_blocker); + migrate_del_blocker(vsc->migration_blocker); + error_free(vsc->migration_blocker); /* This will stop vhost backend. */ vhost_scsi_set_status(vdev, 0); - vhost_dev_cleanup(&s->dev); - g_free(s->dev.vqs); + vhost_dev_cleanup(&vsc->dev); + g_free(vsc->dev.vqs); virtio_scsi_common_unrealize(dev, errp); } -/* - * Implementation of an interface to adjust firmware path - * for the bootindex property handling. - */ -static char *vhost_scsi_get_fw_dev_path(FWPathProvider *p, BusState *bus, - DeviceState *dev) -{ - VHostSCSI *s = VHOST_SCSI(dev); - /* format: channel@channel/vhost-scsi@target,lun */ - return g_strdup_printf("/channel@%x/%s@%x,%x", s->channel, - qdev_fw_name(dev), s->target, s->lun); -} - static Property vhost_scsi_properties[] = { - DEFINE_PROP_STRING("vhostfd", VHostSCSI, parent_obj.conf.vhostfd), - DEFINE_PROP_STRING("wwpn", VHostSCSI, parent_obj.conf.wwpn), - DEFINE_PROP_UINT32("boot_tpgt", VHostSCSI, parent_obj.conf.boot_tpgt, 0), - DEFINE_PROP_UINT32("num_queues", VHostSCSI, parent_obj.conf.num_queues, 1), - DEFINE_PROP_UINT32("max_sectors", VHostSCSI, parent_obj.conf.max_sectors, - 0xFFFF), - DEFINE_PROP_UINT32("cmd_per_lun", VHostSCSI, parent_obj.conf.cmd_per_lun, - 128), + DEFINE_PROP_STRING("vhostfd", VirtIOSCSICommon, conf.vhostfd), + DEFINE_PROP_STRING("wwpn", VirtIOSCSICommon, conf.wwpn), + DEFINE_PROP_UINT32("boot_tpgt", VirtIOSCSICommon, conf.boot_tpgt, 0), + DEFINE_PROP_UINT32("num_queues", VirtIOSCSICommon, conf.num_queues, 1), + DEFINE_PROP_UINT32("max_sectors", VirtIOSCSICommon, conf.max_sectors, + 0xFFFF), + DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSICommon, conf.cmd_per_lun, 128), DEFINE_PROP_END_OF_LIST(), }; @@ -333,23 +249,25 @@ static void vhost_scsi_class_init(ObjectClass *klass, void *data) set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); vdc->realize = vhost_scsi_realize; vdc->unrealize = vhost_scsi_unrealize; - vdc->get_features = vhost_scsi_get_features; - vdc->set_config = vhost_scsi_set_config; + vdc->get_features = vhost_scsi_common_get_features; + vdc->set_config = vhost_scsi_common_set_config; vdc->set_status = vhost_scsi_set_status; - fwc->get_dev_path = vhost_scsi_get_fw_dev_path; + fwc->get_dev_path = vhost_scsi_common_get_fw_dev_path; } static void vhost_scsi_instance_init(Object *obj) { - VHostSCSI *dev = VHOST_SCSI(obj); + VHostSCSICommon *vsc = VHOST_SCSI_COMMON(obj); + + vsc->feature_bits = kernel_feature_bits; - device_add_bootindex_property(obj, &dev->bootindex, "bootindex", NULL, - DEVICE(dev), NULL); + device_add_bootindex_property(obj, &vsc->bootindex, "bootindex", NULL, + DEVICE(vsc), NULL); } static const TypeInfo vhost_scsi_info = { .name = TYPE_VHOST_SCSI, - .parent = TYPE_VIRTIO_SCSI_COMMON, + .parent = TYPE_VHOST_SCSI_COMMON, .instance_size = sizeof(VHostSCSI), .class_init = vhost_scsi_class_init, .instance_init = vhost_scsi_instance_init, diff --git a/include/hw/virtio/vhost-scsi-common.h b/include/hw/virtio/vhost-scsi-common.h new file mode 100644 index 0000000..4553be4 --- /dev/null +++ b/include/hw/virtio/vhost-scsi-common.h @@ -0,0 +1,48 @@ +/* + * vhost_scsi host device + * + * Copyright (c) 2016 Nutanix Inc. All rights reserved. + * + * Author: + * Felipe Franciosi + * + * This work is licensed under the terms of the GNU LGPL, version 2 or later. + * See the COPYING.LIB file in the top-level directory. + * + */ + +#ifndef VHOST_SCSI_COMMON_H +#define VHOST_SCSI_COMMON_H + +#include "qemu-common.h" +#include "hw/qdev.h" +#include "hw/virtio/virtio-scsi.h" +#include "hw/virtio/vhost.h" +#include "hw/fw-path-provider.h" + +#define TYPE_VHOST_SCSI_COMMON "vhost-scsi-common" +#define VHOST_SCSI_COMMON(obj) \ + OBJECT_CHECK(VHostSCSICommon, (obj), TYPE_VHOST_SCSI_COMMON) + +typedef struct VHostSCSICommon { + VirtIOSCSICommon parent_obj; + + Error *migration_blocker; + + struct vhost_dev dev; + const int *feature_bits; + int32_t bootindex; + int channel; + int target; + int lun; +} VHostSCSICommon; + +int vhost_scsi_common_start(VHostSCSICommon *vsc); +void vhost_scsi_common_stop(VHostSCSICommon *vsc); +char *vhost_scsi_common_get_fw_dev_path(FWPathProvider *p, BusState *bus, + DeviceState *dev); +void vhost_scsi_common_set_config(VirtIODevice *vdev, const uint8_t *config); +uint64_t vhost_scsi_common_get_features(VirtIODevice *vdev, uint64_t features, + Error **errp); + +#endif /* VHOST_SCSI_COMMON_H */ diff --git a/include/hw/virtio/vhost-scsi.h b/include/hw/virtio/vhost-scsi.h index 9fd63df..04658d1 100644 --- a/include/hw/virtio/vhost-scsi.h +++ b/include/hw/virtio/vhost-scsi.h @@ -18,6 +18,7 @@ #include "hw/qdev.h" #include "hw/virtio/virtio-scsi.h" #include "hw/virtio/vhost.h" +#include "hw/virtio/vhost-scsi-common.h" enum vhost_scsi_vq_list { VHOST_SCSI_VQ_CONTROL = 0, @@ -30,15 +31,7 @@ enum vhost_scsi_vq_list { OBJECT_CHECK(VHostSCSI, (obj), TYPE_VHOST_SCSI) typedef struct VHostSCSI { - VirtIOSCSICommon parent_obj; - - Error *migration_blocker; - - struct vhost_dev dev; - int32_t bootindex; - int channel; - int target; - int lun; + VHostSCSICommon parent_obj; } VHostSCSI; #endif diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h index 8c8453c..eac2013 100644 --- a/include/hw/virtio/virtio-scsi.h +++ b/include/hw/virtio/virtio-scsi.h @@ -49,8 +49,10 @@ struct VirtIOSCSIConf { uint32_t num_queues; uint32_t max_sectors; uint32_t cmd_per_lun; +#ifdef CONFIG_VHOST_SCSI char *vhostfd; char *wwpn; +#endif uint32_t boot_tpgt; IOThread *iothread; }; -- cgit v1.1