From 55ad781ca7fcaed19e04554b558727206d638a99 Mon Sep 17 00:00:00 2001 From: Wei Jiangang Date: Thu, 7 Apr 2016 10:46:24 +0800 Subject: [PATCH 01/12] use g_path_get_dirname instead of dirname Use g_path_get_basename to get the directory components of a file name, and free its return when no longer needed. Signed-off-by: Wei Jiangang Message-Id: <1459997185-15669-3-git-send-email-weijg.fnst@cn.fujitsu.com> Signed-off-by: Paolo Bonzini --- os-posix.c | 3 ++- util/oslib-posix.c | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/os-posix.c b/os-posix.c index 3755265582..c6ddb7d830 100644 --- a/os-posix.c +++ b/os-posix.c @@ -89,7 +89,7 @@ char *os_find_datadir(void) if (exec_dir == NULL) { return NULL; } - dir = dirname(exec_dir); + dir = g_path_get_dirname(exec_dir); max_len = strlen(dir) + MAX(strlen(SHARE_SUFFIX), strlen(BUILD_SUFFIX)) + 1; @@ -103,6 +103,7 @@ char *os_find_datadir(void) } } + g_free(dir); g_free(exec_dir); return res; } diff --git a/util/oslib-posix.c b/util/oslib-posix.c index d8e5dcfede..6d70d9a706 100644 --- a/util/oslib-posix.c +++ b/util/oslib-posix.c @@ -299,9 +299,11 @@ void qemu_init_exec_dir(const char *argv0) return; } } - dir = dirname(p); + dir = g_path_get_dirname(p); pstrcpy(exec_dir, sizeof(exec_dir), dir); + + g_free(dir); } char *qemu_get_exec_dir(void) From fd9a30483091eb2e297401dd1d860045188d779a Mon Sep 17 00:00:00 2001 From: Md Haris Iqbal Date: Tue, 5 Apr 2016 18:39:03 +0530 Subject: [PATCH 02/12] Changed malloc to g_malloc, free to g_free in bsd-user/qemu.h Signed-off-by: Md Haris Iqbal Message-Id: <1459861743-4514-1-git-send-email-haris.phnx@gmail.com> Reviewed-by: Sean Bruno Signed-off-by: Paolo Bonzini --- bsd-user/qemu.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h index 53163b8a47..6ccc544e7d 100644 --- a/bsd-user/qemu.h +++ b/bsd-user/qemu.h @@ -358,7 +358,7 @@ static inline void *lock_user(int type, abi_ulong guest_addr, long len, int copy #ifdef DEBUG_REMAP { void *addr; - addr = malloc(len); + addr = g_malloc(len); if (copy) memcpy(addr, g2h(guest_addr), len); else @@ -384,7 +384,7 @@ static inline void unlock_user(void *host_ptr, abi_ulong guest_addr, return; if (len > 0) memcpy(g2h(guest_addr), host_ptr, len); - free(host_ptr); + g_free(host_ptr); #endif } From 3cbeb52467a8b8f3a836c7783a7ebc6450c5ef30 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Wed, 6 Apr 2016 22:04:14 +0200 Subject: [PATCH 03/12] hw/i386: add device tree support With "-dtb" on command-line: - append the device tree blob to the kernel image; - pass the blob's pointer to the kernel through setup_data, as requested by upstream kernel commit da6b737b9ab7 ("x86: Add device tree support"). The device tree blob is passed as-is to the guest; none of its fields is modified nor updated. This is not an issue; the kernel commit above uses the device tree only as an extension to the traditional kernel configuration. To: "Michael S. Tsirkin" To: Paolo Bonzini To: Richard Henderson To: Eduardo Habkost Cc: qemu-devel@nongnu.org Cc: Sebastian Andrzej Siewior Signed-off-by: Antonio Borneo Message-Id: <1459973054-2777-1-git-send-email-borneo.antonio@gmail.com> Signed-off-by: Paolo Bonzini --- hw/i386/pc.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 1b8baa8fee..719884ff88 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -812,11 +812,26 @@ static long get_file_size(FILE *f) return size; } +/* setup_data types */ +#define SETUP_NONE 0 +#define SETUP_E820_EXT 1 +#define SETUP_DTB 2 +#define SETUP_PCI 3 +#define SETUP_EFI 4 + +struct setup_data { + uint64_t next; + uint32_t type; + uint32_t len; + uint8_t data[0]; +} __attribute__((packed)); + static void load_linux(PCMachineState *pcms, FWCfgState *fw_cfg) { uint16_t protocol; int setup_size, kernel_size, initrd_size = 0, cmdline_size; + int dtb_size, setup_data_offset; uint32_t initrd_max; uint8_t header[8192], *setup, *kernel, *initrd_data; hwaddr real_addr, prot_addr, cmdline_addr, initrd_addr = 0; @@ -824,8 +839,10 @@ static void load_linux(PCMachineState *pcms, char *vmode; MachineState *machine = MACHINE(pcms); PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms); + struct setup_data *setup_data; const char *kernel_filename = machine->kernel_filename; const char *initrd_filename = machine->initrd_filename; + const char *dtb_filename = machine->dtb; const char *kernel_cmdline = machine->kernel_cmdline; /* Align to 16 bytes as a paranoia measure */ @@ -988,6 +1005,35 @@ static void load_linux(PCMachineState *pcms, exit(1); } fclose(f); + + /* append dtb to kernel */ + if (dtb_filename) { + if (protocol < 0x209) { + fprintf(stderr, "qemu: Linux kernel too old to load a dtb\n"); + exit(1); + } + + dtb_size = get_image_size(dtb_filename); + if (dtb_size <= 0) { + fprintf(stderr, "qemu: error reading dtb %s: %s\n", + dtb_filename, strerror(errno)); + exit(1); + } + + setup_data_offset = QEMU_ALIGN_UP(kernel_size, 16); + kernel_size = setup_data_offset + sizeof(struct setup_data) + dtb_size; + kernel = g_realloc(kernel, kernel_size); + + stq_p(header+0x250, prot_addr + setup_data_offset); + + setup_data = (struct setup_data *)(kernel + setup_data_offset); + setup_data->next = 0; + setup_data->type = cpu_to_le32(SETUP_DTB); + setup_data->len = cpu_to_le32(dtb_size); + + load_image_size(dtb_filename, setup_data->data, dtb_size); + } + memcpy(setup, header, MIN(sizeof(header), setup_size)); fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, prot_addr); From 8bff06a0bbf257a2083223534c1607bf87d913e6 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 15 Jul 2016 18:27:40 +0200 Subject: [PATCH 04/12] compiler: never omit assertions if using a static analysis tool Assertions help both Coverity and the clang static analyzer avoid false positives, but on the other hand both are confused when the condition is compiled as (void)(x != FOO). Always expand assertion macros when using Coverity or clang, through a new QEMU_STATIC_ANALYSIS preprocessor symbol. This fixes a couple false positives in TCG. Signed-off-by: Paolo Bonzini --- include/qemu/compiler.h | 3 +++ tcg/tcg.h | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h index b64f899870..338d3a65b3 100644 --- a/include/qemu/compiler.h +++ b/include/qemu/compiler.h @@ -3,6 +3,9 @@ #ifndef COMPILER_H #define COMPILER_H +#if defined __clang_analyzer__ || defined __COVERITY__ +#define QEMU_STATIC_ANALYSIS 1 +#endif /*---------------------------------------------------------------------------- | The macro QEMU_GNUC_PREREQ tests for minimum version of the GNU C compiler. diff --git a/tcg/tcg.h b/tcg/tcg.h index 66ae0c76e6..6046dcdc89 100644 --- a/tcg/tcg.h +++ b/tcg/tcg.h @@ -191,7 +191,7 @@ typedef uint64_t tcg_insn_unit; #endif -#ifdef CONFIG_DEBUG_TCG +#if defined CONFIG_DEBUG_TCG || defined QEMU_STATIC_ANALYSIS # define tcg_debug_assert(X) do { assert(X); } while (0) #elif QEMU_GNUC_PREREQ(4, 5) # define tcg_debug_assert(X) \ From 8cc46787b5b58f01a11c919c7ff939ed009e27fc Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 15 Jul 2016 18:42:53 +0200 Subject: [PATCH 05/12] megasas: remove useless check for cmd->frame megasas_enqueue_frame always returns with non-NULL cmd->frame. Remove the "else" part as it is dead code. Signed-off-by: Paolo Bonzini --- hw/scsi/megasas.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c index 52a41239cf..e968302fdc 100644 --- a/hw/scsi/megasas.c +++ b/hw/scsi/megasas.c @@ -1981,11 +1981,7 @@ static void megasas_handle_frame(MegasasState *s, uint64_t frame_addr, break; } if (frame_status != MFI_STAT_INVALID_STATUS) { - if (cmd->frame) { - cmd->frame->header.cmd_status = frame_status; - } else { - megasas_frame_set_cmd_status(s, frame_addr, frame_status); - } + cmd->frame->header.cmd_status = frame_status; megasas_unmap_frame(s, cmd); megasas_complete_frame(s, cmd->context); } From d211bd6016a5d2d59911c6d3d343f114e9853366 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Fri, 15 Jul 2016 10:46:54 +0100 Subject: [PATCH 06/12] checkpatch: consider git extended headers valid patches Renames look like this with git-diff(1) when diff.renames = true is set: diff --git a/a b/b similarity index 100% rename from a rename to b This raises the "Does not appear to be a unified-diff format patch" error because checkpatch.pl only considers a diff valid if it contains at least one "@@" hunk. This patch accepts renames and copies too so that checkpatch.pl exits successfully when a diff only renames/copies files. The git diff extended header format is described on the git-diff(1) man page. Reported-by: Colin Lord Signed-off-by: Stefan Hajnoczi Message-Id: <1468576014-28788-1-git-send-email-stefanha@redhat.com> Signed-off-by: Paolo Bonzini --- scripts/checkpatch.pl | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index cf32c8f5fa..afa7f79f83 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1279,6 +1279,11 @@ sub process { } } +# Accept git diff extended headers as valid patches + if ($line =~ /^(?:rename|copy) (?:from|to) [\w\/\.\-]+\s*$/) { + $is_patch = 1; + } + #check the patch for a signoff: if ($line =~ /^\s*signed-off-by:/i) { # This is a signoff, if ugly, so do not double report. From 101420b886eec36990419bc9ed5b503622af8a0d Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Fri, 15 Jul 2016 12:03:50 +0200 Subject: [PATCH 07/12] exec: avoid realloc in phys_map_node_reserve this is the first step in reducing the brk heap fragmentation created by the map->nodes memory allocation. Since the introduction of RCU the freeing of the PhysPageMaps is delayed so that sometimes several hundred are allocated at the same time. Even worse the memory for map->nodes is allocated and shortly afterwards reallocated. Since the number of nodes it grows to in the end is the same for all PhysPageMaps remember this value and at least avoid the reallocation. The large number of simultaneous allocations (about 450 x 70kB in my configuration) has to be addressed later. Signed-off-by: Peter Lieven Message-Id: <1468577030-21097-1-git-send-email-pl@kamp.de> Signed-off-by: Paolo Bonzini --- exec.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/exec.c b/exec.c index 011babd584..60cf46a5b5 100644 --- a/exec.c +++ b/exec.c @@ -187,10 +187,12 @@ struct CPUAddressSpace { static void phys_map_node_reserve(PhysPageMap *map, unsigned nodes) { + static unsigned alloc_hint = 16; if (map->nodes_nb + nodes > map->nodes_nb_alloc) { - map->nodes_nb_alloc = MAX(map->nodes_nb_alloc * 2, 16); + map->nodes_nb_alloc = MAX(map->nodes_nb_alloc, alloc_hint); map->nodes_nb_alloc = MAX(map->nodes_nb_alloc, map->nodes_nb + nodes); map->nodes = g_renew(Node, map->nodes, map->nodes_nb_alloc); + alloc_hint = map->nodes_nb_alloc; } } From ca7d8e1c9ccb95257a9c9288617b270727ec3e79 Mon Sep 17 00:00:00 2001 From: Sergey Fedorov Date: Fri, 15 Jul 2016 22:31:23 +0300 Subject: [PATCH 08/12] cpu-exec: Move down some declarations in cpu_exec() This will fix a compiler warning with -Wclobbered: http://lists.nongnu.org/archive/html/qemu-devel/2016-07/msg03347.html Reported-by: Stefan Weil Signed-off-by: Sergey Fedorov Signed-off-by: Sergey Fedorov Message-Id: <20160715193123.28113-1-sergey.fedorov@linaro.org> Signed-off-by: Paolo Bonzini --- cpu-exec.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cpu-exec.c b/cpu-exec.c index b840e1d2dd..5d9710a1ea 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -608,17 +608,16 @@ int cpu_exec(CPUState *cpu) init_delay_params(&sc, cpu); for(;;) { - TranslationBlock *tb, *last_tb; - int tb_exit = 0; - /* prepare setjmp context for exception handling */ if (sigsetjmp(cpu->jmp_env, 0) == 0) { + TranslationBlock *tb, *last_tb = NULL; + int tb_exit = 0; + /* if an exception is pending, we execute it here */ if (cpu_handle_exception(cpu, &ret)) { break; } - last_tb = NULL; /* forget the last executed TB after exception */ cpu->tb_flushed = false; /* reset before first TB lookup */ for(;;) { cpu_handle_interrupt(cpu, &last_tb); From e5dfc5e8e715c572aea44ac4d96c43941d4741c7 Mon Sep 17 00:00:00 2001 From: Pranith Kumar Date: Fri, 15 Jul 2016 00:31:11 -0400 Subject: [PATCH 09/12] Move README to markdown Move the README file to markdown so that it makes the github page look prettier. I know that github repo is a mirror and not the official repo, but I think it doesn't hurt to have it in markdown format. Signed-off-by: Pranith Kumar Message-Id: <20160715043111.29007-1-bobby.prani@gmail.com> Signed-off-by: Paolo Bonzini --- README => README.md | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-) rename README => README.md (85%) diff --git a/README b/README.md similarity index 85% rename from README rename to README.md index f38193fc67..99da481fa1 100644 --- a/README +++ b/README.md @@ -1,5 +1,5 @@ - QEMU README - =========== +QEMU +--- QEMU is a generic and open source machine & userspace emulator and virtualizer. @@ -31,31 +31,31 @@ version 2. For full licensing details, consult the LICENSE file. Building -======== +--- QEMU is multi-platform software intended to be buildable on all modern Linux platforms, OS-X, Win32 (via the Mingw64 toolchain) and a variety of other UNIX targets. The simple steps to build QEMU are: - mkdir build - cd build - ../configure - make + mkdir build + cd build + ../configure + make Complete details of the process for building and configuring QEMU for all supported host platforms can be found in the qemu-tech.html file. Additional information can also be found online via the QEMU website: - http://qemu-project.org/Hosts/Linux - http://qemu-project.org/Hosts/W32 + http://qemu-project.org/Hosts/Linux + http://qemu-project.org/Hosts/W32 Submitting patches -================== +--- The QEMU source code is maintained under the GIT version control system. - git clone git://git.qemu-project.org/qemu.git + git clone git://git.qemu-project.org/qemu.git When submitting patches, the preferred approach is to use 'git format-patch' and/or 'git send-email' to format & send the mail to the @@ -66,18 +66,18 @@ guidelines set out in the HACKING and CODING_STYLE files. Additional information on submitting patches can be found online via the QEMU website - http://qemu-project.org/Contribute/SubmitAPatch - http://qemu-project.org/Contribute/TrivialPatches + http://qemu-project.org/Contribute/SubmitAPatch + http://qemu-project.org/Contribute/TrivialPatches Bug reporting -============= +--- The QEMU project uses Launchpad as its primary upstream bug tracker. Bugs found when running code built from QEMU git or upstream released sources should be reported via: - https://bugs.launchpad.net/qemu/ + https://bugs.launchpad.net/qemu/ If using QEMU via an operating system vendor pre-built binary package, it is preferable to report bugs to the vendor's own bug tracker first. If @@ -86,22 +86,21 @@ reported via launchpad. For additional information on bug reporting consult: - http://qemu-project.org/Contribute/ReportABug + http://qemu-project.org/Contribute/ReportABug Contact -======= +--- The QEMU community can be contacted in a number of ways, with the two main methods being email and IRC - - qemu-devel@nongnu.org - http://lists.nongnu.org/mailman/listinfo/qemu-devel - - #qemu on irc.oftc.net + - Mailing List: qemu-devel@nongnu.org + - Archives: http://lists.nongnu.org/mailman/listinfo/qemu-devel + - IRC: #qemu on irc.oftc.net Information on additional methods of contacting the community can be found online via the QEMU website: http://qemu-project.org/Contribute/StartHere --- End From eb36b953e0ebf4129b188a241fbc367062ac2e06 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Mon, 18 Jul 2016 10:52:19 +0200 Subject: [PATCH 10/12] block/iscsi: fix rounding in iscsi_allocationmap_set when setting clusters as alloacted the boundaries have to be expanded. As Paolo pointed out the calculation of the number of clusters is wrong: Suppose cluster_sectors is 2, sector_num = 1, nb_sectors = 6: In the "mark allocated" case, you want to set 0..8, i.e. cluster_num=0, nb_clusters=4. 0--.--2--.--4--.--6--.--8 <--|_________________|--> (<--> = expanded) Instead you are setting nb_clusters=3, so that 6..8 is not marked. 0--.--2--.--4--.--6--.--8 <--|______________|!!! (! = wrong) Cc: qemu-stable@nongnu.org Reported-by: Paolo Bonzini Signed-off-by: Peter Lieven Message-Id: <1468831940-15556-2-git-send-email-pl@kamp.de> Signed-off-by: Paolo Bonzini --- block/iscsi.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index cf1e9e7f66..22330e1fe0 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -432,12 +432,14 @@ static unsigned long *iscsi_allocationmap_init(IscsiLun *iscsilun) static void iscsi_allocationmap_set(IscsiLun *iscsilun, int64_t sector_num, int nb_sectors) { + int64_t cluster_num, nb_clusters; if (iscsilun->allocationmap == NULL) { return; } - bitmap_set(iscsilun->allocationmap, - sector_num / iscsilun->cluster_sectors, - DIV_ROUND_UP(nb_sectors, iscsilun->cluster_sectors)); + cluster_num = sector_num / iscsilun->cluster_sectors; + nb_clusters = DIV_ROUND_UP(sector_num + nb_sectors, + iscsilun->cluster_sectors) - cluster_num; + bitmap_set(iscsilun->allocationmap, cluster_num, nb_clusters); } static void iscsi_allocationmap_clear(IscsiLun *iscsilun, int64_t sector_num, From e1123a3b40a1a9a625a29c8ed4debb7e206ea690 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Mon, 18 Jul 2016 10:52:20 +0200 Subject: [PATCH 11/12] block/iscsi: allow caching of the allocation map until now the allocation map was used only as a hint if a cluster is allocated or not. If a block was not allocated (or Qemu had no info about the allocation status) a get_block_status call was issued to check the allocation status and possibly avoid a subsequent read of unallocated sectors. If a block known to be allocated the get_block_status call was omitted. In the other case a get_block_status call was issued before every read to avoid the necessity for a consistent allocation map. To avoid the potential overhead of calling get_block_status for each and every read request this took only place for the bigger requests. This patch enhances this mechanism to cache the allocation status and avoid calling get_block_status for blocks where the allocation status has been queried before. This allows for bypassing the read request even for smaller requests and additionally omits calling get_block_status for known to be unallocated blocks. Signed-off-by: Peter Lieven Message-Id: <1468831940-15556-3-git-send-email-pl@kamp.de> Signed-off-by: Paolo Bonzini --- block/iscsi.c | 250 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 192 insertions(+), 58 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index 22330e1fe0..129c3afa68 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -2,7 +2,7 @@ * QEMU Block driver for iSCSI images * * Copyright (c) 2010-2011 Ronnie Sahlberg - * Copyright (c) 2012-2015 Peter Lieven + * Copyright (c) 2012-2016 Peter Lieven * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -61,7 +61,23 @@ typedef struct IscsiLun { struct scsi_inquiry_logical_block_provisioning lbp; struct scsi_inquiry_block_limits bl; unsigned char *zeroblock; - unsigned long *allocationmap; + /* The allocmap tracks which clusters (pages) on the iSCSI target are + * allocated and which are not. In case a target returns zeros for + * unallocated pages (iscsilun->lprz) we can directly return zeros instead + * of reading zeros over the wire if a read request falls within an + * unallocated block. As there are 3 possible states we need 2 bitmaps to + * track. allocmap_valid keeps track if QEMU's information about a page is + * valid. allocmap tracks if a page is allocated or not. In case QEMU has no + * valid information about a page the corresponding allocmap entry should be + * switched to unallocated as well to force a new lookup of the allocation + * status as lookups are generally skipped if a page is suspect to be + * allocated. If a iSCSI target is opened with cache.direct = on the + * allocmap_valid does not exist turning all cached information invalid so + * that a fresh lookup is made for any page even if allocmap entry returns + * it's unallocated. */ + unsigned long *allocmap; + unsigned long *allocmap_valid; + long allocmap_size; int cluster_sectors; bool use_16_for_rw; bool write_protected; @@ -422,39 +438,135 @@ static bool is_sector_request_lun_aligned(int64_t sector_num, int nb_sectors, iscsilun); } -static unsigned long *iscsi_allocationmap_init(IscsiLun *iscsilun) +static void iscsi_allocmap_free(IscsiLun *iscsilun) { - return bitmap_try_new(DIV_ROUND_UP(sector_lun2qemu(iscsilun->num_blocks, - iscsilun), - iscsilun->cluster_sectors)); + g_free(iscsilun->allocmap); + g_free(iscsilun->allocmap_valid); + iscsilun->allocmap = NULL; + iscsilun->allocmap_valid = NULL; } -static void iscsi_allocationmap_set(IscsiLun *iscsilun, int64_t sector_num, - int nb_sectors) + +static int iscsi_allocmap_init(IscsiLun *iscsilun, int open_flags) { - int64_t cluster_num, nb_clusters; - if (iscsilun->allocationmap == NULL) { - return; + iscsi_allocmap_free(iscsilun); + + iscsilun->allocmap_size = + DIV_ROUND_UP(sector_lun2qemu(iscsilun->num_blocks, iscsilun), + iscsilun->cluster_sectors); + + iscsilun->allocmap = bitmap_try_new(iscsilun->allocmap_size); + if (!iscsilun->allocmap) { + return -ENOMEM; } - cluster_num = sector_num / iscsilun->cluster_sectors; - nb_clusters = DIV_ROUND_UP(sector_num + nb_sectors, - iscsilun->cluster_sectors) - cluster_num; - bitmap_set(iscsilun->allocationmap, cluster_num, nb_clusters); + + if (open_flags & BDRV_O_NOCACHE) { + /* in case that cache.direct = on all allocmap entries are + * treated as invalid to force a relookup of the block + * status on every read request */ + return 0; + } + + iscsilun->allocmap_valid = bitmap_try_new(iscsilun->allocmap_size); + if (!iscsilun->allocmap_valid) { + /* if we are under memory pressure free the allocmap as well */ + iscsi_allocmap_free(iscsilun); + return -ENOMEM; + } + + return 0; } -static void iscsi_allocationmap_clear(IscsiLun *iscsilun, int64_t sector_num, - int nb_sectors) +static void +iscsi_allocmap_update(IscsiLun *iscsilun, int64_t sector_num, + int nb_sectors, bool allocated, bool valid) { - int64_t cluster_num, nb_clusters; - if (iscsilun->allocationmap == NULL) { + int64_t cl_num_expanded, nb_cls_expanded, cl_num_shrunk, nb_cls_shrunk; + + if (iscsilun->allocmap == NULL) { return; } - cluster_num = DIV_ROUND_UP(sector_num, iscsilun->cluster_sectors); - nb_clusters = (sector_num + nb_sectors) / iscsilun->cluster_sectors - - cluster_num; - if (nb_clusters > 0) { - bitmap_clear(iscsilun->allocationmap, cluster_num, nb_clusters); + /* expand to entirely contain all affected clusters */ + cl_num_expanded = sector_num / iscsilun->cluster_sectors; + nb_cls_expanded = DIV_ROUND_UP(sector_num + nb_sectors, + iscsilun->cluster_sectors) - cl_num_expanded; + /* shrink to touch only completely contained clusters */ + cl_num_shrunk = DIV_ROUND_UP(sector_num, iscsilun->cluster_sectors); + nb_cls_shrunk = (sector_num + nb_sectors) / iscsilun->cluster_sectors + - cl_num_shrunk; + if (allocated) { + bitmap_set(iscsilun->allocmap, cl_num_expanded, nb_cls_expanded); + } else { + bitmap_clear(iscsilun->allocmap, cl_num_shrunk, nb_cls_shrunk); } + + if (iscsilun->allocmap_valid == NULL) { + return; + } + if (valid) { + bitmap_set(iscsilun->allocmap_valid, cl_num_shrunk, nb_cls_shrunk); + } else { + bitmap_clear(iscsilun->allocmap_valid, cl_num_expanded, + nb_cls_expanded); + } +} + +static void +iscsi_allocmap_set_allocated(IscsiLun *iscsilun, int64_t sector_num, + int nb_sectors) +{ + iscsi_allocmap_update(iscsilun, sector_num, nb_sectors, true, true); +} + +static void +iscsi_allocmap_set_unallocated(IscsiLun *iscsilun, int64_t sector_num, + int nb_sectors) +{ + /* Note: if cache.direct=on the fifth argument to iscsi_allocmap_update + * is ignored, so this will in effect be an iscsi_allocmap_set_invalid. + */ + iscsi_allocmap_update(iscsilun, sector_num, nb_sectors, false, true); +} + +static void iscsi_allocmap_set_invalid(IscsiLun *iscsilun, int64_t sector_num, + int nb_sectors) +{ + iscsi_allocmap_update(iscsilun, sector_num, nb_sectors, false, false); +} + +static void iscsi_allocmap_invalidate(IscsiLun *iscsilun) +{ + if (iscsilun->allocmap) { + bitmap_zero(iscsilun->allocmap, iscsilun->allocmap_size); + } + if (iscsilun->allocmap_valid) { + bitmap_zero(iscsilun->allocmap_valid, iscsilun->allocmap_size); + } +} + +static inline bool +iscsi_allocmap_is_allocated(IscsiLun *iscsilun, int64_t sector_num, + int nb_sectors) +{ + unsigned long size; + if (iscsilun->allocmap == NULL) { + return true; + } + size = DIV_ROUND_UP(sector_num + nb_sectors, iscsilun->cluster_sectors); + return !(find_next_bit(iscsilun->allocmap, size, + sector_num / iscsilun->cluster_sectors) == size); +} + +static inline bool iscsi_allocmap_is_valid(IscsiLun *iscsilun, + int64_t sector_num, int nb_sectors) +{ + unsigned long size; + if (iscsilun->allocmap_valid == NULL) { + return false; + } + size = DIV_ROUND_UP(sector_num + nb_sectors, iscsilun->cluster_sectors); + return (find_next_zero_bit(iscsilun->allocmap_valid, size, + sector_num / iscsilun->cluster_sectors) == size); } static int coroutine_fn @@ -517,26 +629,16 @@ retry: } if (iTask.status != SCSI_STATUS_GOOD) { + iscsi_allocmap_set_invalid(iscsilun, sector_num, nb_sectors); return iTask.err_code; } - iscsi_allocationmap_set(iscsilun, sector_num, nb_sectors); + iscsi_allocmap_set_allocated(iscsilun, sector_num, nb_sectors); return 0; } -static bool iscsi_allocationmap_is_allocated(IscsiLun *iscsilun, - int64_t sector_num, int nb_sectors) -{ - unsigned long size; - if (iscsilun->allocationmap == NULL) { - return true; - } - size = DIV_ROUND_UP(sector_num + nb_sectors, iscsilun->cluster_sectors); - return !(find_next_bit(iscsilun->allocationmap, size, - sector_num / iscsilun->cluster_sectors) == size); -} static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs, int64_t sector_num, @@ -621,9 +723,9 @@ retry: } if (ret & BDRV_BLOCK_ZERO) { - iscsi_allocationmap_clear(iscsilun, sector_num, *pnum); + iscsi_allocmap_set_unallocated(iscsilun, sector_num, *pnum); } else { - iscsi_allocationmap_set(iscsilun, sector_num, *pnum); + iscsi_allocmap_set_allocated(iscsilun, sector_num, *pnum); } if (*pnum > nb_sectors) { @@ -659,17 +761,32 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs, return -EINVAL; } - if (iscsilun->lbprz && nb_sectors >= ISCSI_CHECKALLOC_THRES && - !iscsi_allocationmap_is_allocated(iscsilun, sector_num, nb_sectors)) { - int64_t ret; + /* if cache.direct is off and we have a valid entry in our allocation map + * we can skip checking the block status and directly return zeroes if + * the request falls within an unallocated area */ + if (iscsi_allocmap_is_valid(iscsilun, sector_num, nb_sectors) && + !iscsi_allocmap_is_allocated(iscsilun, sector_num, nb_sectors)) { + qemu_iovec_memset(iov, 0, 0x00, iov->size); + return 0; + } + + if (nb_sectors >= ISCSI_CHECKALLOC_THRES && + !iscsi_allocmap_is_valid(iscsilun, sector_num, nb_sectors) && + !iscsi_allocmap_is_allocated(iscsilun, sector_num, nb_sectors)) { int pnum; BlockDriverState *file; - ret = iscsi_co_get_block_status(bs, sector_num, - BDRV_REQUEST_MAX_SECTORS, &pnum, &file); + /* check the block status from the beginning of the cluster + * containing the start sector */ + int64_t ret = iscsi_co_get_block_status(bs, + sector_num - sector_num % iscsilun->cluster_sectors, + BDRV_REQUEST_MAX_SECTORS, &pnum, &file); if (ret < 0) { return ret; } - if (ret & BDRV_BLOCK_ZERO && pnum >= nb_sectors) { + /* if the whole request falls into an unallocated area we can avoid + * to read and directly return zeroes instead */ + if (ret & BDRV_BLOCK_ZERO && + pnum >= nb_sectors + sector_num % iscsilun->cluster_sectors) { qemu_iovec_memset(iov, 0, 0x00, iov->size); return 0; } @@ -983,7 +1100,7 @@ retry: return iTask.err_code; } - iscsi_allocationmap_clear(iscsilun, sector_num, nb_sectors); + iscsi_allocmap_set_invalid(iscsilun, sector_num, nb_sectors); return 0; } @@ -1073,15 +1190,17 @@ retry: } if (iTask.status != SCSI_STATUS_GOOD) { + iscsi_allocmap_set_invalid(iscsilun, offset >> BDRV_SECTOR_BITS, + count >> BDRV_SECTOR_BITS); return iTask.err_code; } if (flags & BDRV_REQ_MAY_UNMAP) { - iscsi_allocationmap_clear(iscsilun, offset >> BDRV_SECTOR_BITS, - count >> BDRV_SECTOR_BITS); + iscsi_allocmap_set_invalid(iscsilun, offset >> BDRV_SECTOR_BITS, + count >> BDRV_SECTOR_BITS); } else { - iscsi_allocationmap_set(iscsilun, offset >> BDRV_SECTOR_BITS, - count >> BDRV_SECTOR_BITS); + iscsi_allocmap_set_allocated(iscsilun, offset >> BDRV_SECTOR_BITS, + count >> BDRV_SECTOR_BITS); } return 0; @@ -1654,10 +1773,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, iscsilun->cluster_sectors = (iscsilun->bl.opt_unmap_gran * iscsilun->block_size) >> BDRV_SECTOR_BITS; if (iscsilun->lbprz) { - iscsilun->allocationmap = iscsi_allocationmap_init(iscsilun); - if (iscsilun->allocationmap == NULL) { - ret = -ENOMEM; - } + ret = iscsi_allocmap_init(iscsilun, bs->open_flags); } } @@ -1694,7 +1810,7 @@ static void iscsi_close(BlockDriverState *bs) } iscsi_destroy_context(iscsi); g_free(iscsilun->zeroblock); - g_free(iscsilun->allocationmap); + iscsi_allocmap_free(iscsilun); memset(iscsilun, 0, sizeof(IscsiLun)); } @@ -1758,6 +1874,16 @@ static int iscsi_reopen_prepare(BDRVReopenState *state, return 0; } +static void iscsi_reopen_commit(BDRVReopenState *reopen_state) +{ + IscsiLun *iscsilun = reopen_state->bs->opaque; + + /* the cache.direct status might have changed */ + if (iscsilun->allocmap != NULL) { + iscsi_allocmap_init(iscsilun, reopen_state->flags); + } +} + static int iscsi_truncate(BlockDriverState *bs, int64_t offset) { IscsiLun *iscsilun = bs->opaque; @@ -1777,9 +1903,8 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t offset) return -EINVAL; } - if (iscsilun->allocationmap != NULL) { - g_free(iscsilun->allocationmap); - iscsilun->allocationmap = iscsi_allocationmap_init(iscsilun); + if (iscsilun->allocmap != NULL) { + iscsi_allocmap_init(iscsilun, bs->open_flags); } return 0; @@ -1839,6 +1964,13 @@ static int iscsi_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) return 0; } +static void iscsi_invalidate_cache(BlockDriverState *bs, + Error **errp) +{ + IscsiLun *iscsilun = bs->opaque; + iscsi_allocmap_invalidate(iscsilun); +} + static QemuOptsList iscsi_create_opts = { .name = "iscsi-create-opts", .head = QTAILQ_HEAD_INITIALIZER(iscsi_create_opts.head), @@ -1862,7 +1994,9 @@ static BlockDriver bdrv_iscsi = { .bdrv_close = iscsi_close, .bdrv_create = iscsi_create, .create_opts = &iscsi_create_opts, - .bdrv_reopen_prepare = iscsi_reopen_prepare, + .bdrv_reopen_prepare = iscsi_reopen_prepare, + .bdrv_reopen_commit = iscsi_reopen_commit, + .bdrv_invalidate_cache = iscsi_invalidate_cache, .bdrv_getlength = iscsi_getlength, .bdrv_get_info = iscsi_get_info, From da6d48e3348bbc266896cf8adf0c33f1eaf5b31f Mon Sep 17 00:00:00 2001 From: Sergey Fedorov Date: Fri, 15 Jul 2016 20:58:45 +0300 Subject: [PATCH 12/12] target-i386: Remove redundant HF_SOFTMMU_MASK MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 'HF_SOFTMMU_MASK' is only set when 'CONFIG_SOFTMMU' is defined. So there's no need in this flag: test 'CONFIG_SOFTMMU' instead. Suggested-by: Paolo Bonzini Signed-off-by: Sergey Fedorov Signed-off-by: Sergey Fedorov Reviewed-by: Alex Bennée Message-Id: <20160715175852.30749-6-sergey.fedorov@linaro.org> Signed-off-by: Paolo Bonzini --- target-i386/cpu.c | 3 --- target-i386/cpu.h | 3 --- target-i386/translate.c | 12 ++++-------- 3 files changed, 4 insertions(+), 14 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index fc209ee1cb..6e49e4ca82 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2725,9 +2725,6 @@ static void x86_cpu_reset(CPUState *s) /* init to reset state */ -#ifdef CONFIG_SOFTMMU - env->hflags |= HF_SOFTMMU_MASK; -#endif env->hflags2 |= HF2_GIF_MASK; cpu_x86_update_cr0(env, 0x60000010); diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 776efe630e..5b14a72baa 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -130,8 +130,6 @@ positions to ease oring with eflags. */ /* current cpl */ #define HF_CPL_SHIFT 0 -/* true if soft mmu is being used */ -#define HF_SOFTMMU_SHIFT 2 /* true if hardware interrupts must be disabled for next instruction */ #define HF_INHIBIT_IRQ_SHIFT 3 /* 16 or 32 segments */ @@ -161,7 +159,6 @@ #define HF_MPX_IU_SHIFT 26 /* BND registers in-use */ #define HF_CPL_MASK (3 << HF_CPL_SHIFT) -#define HF_SOFTMMU_MASK (1 << HF_SOFTMMU_SHIFT) #define HF_INHIBIT_IRQ_MASK (1 << HF_INHIBIT_IRQ_SHIFT) #define HF_CS32_MASK (1 << HF_CS32_SHIFT) #define HF_SS32_MASK (1 << HF_SS32_SHIFT) diff --git a/target-i386/translate.c b/target-i386/translate.c index 7dea18bd63..e81fce7bc2 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -8224,9 +8224,9 @@ void gen_intermediate_code(CPUX86State *env, TranslationBlock *tb) dc->popl_esp_hack = 0; /* select memory access functions */ dc->mem_index = 0; - if (flags & HF_SOFTMMU_MASK) { - dc->mem_index = cpu_mmu_index(env, false); - } +#ifdef CONFIG_SOFTMMU + dc->mem_index = cpu_mmu_index(env, false); +#endif dc->cpuid_features = env->features[FEAT_1_EDX]; dc->cpuid_ext_features = env->features[FEAT_1_ECX]; dc->cpuid_ext2_features = env->features[FEAT_8000_0001_EDX]; @@ -8239,11 +8239,7 @@ void gen_intermediate_code(CPUX86State *env, TranslationBlock *tb) #endif dc->flags = flags; dc->jmp_opt = !(dc->tf || cs->singlestep_enabled || - (flags & HF_INHIBIT_IRQ_MASK) -#ifndef CONFIG_SOFTMMU - || (flags & HF_SOFTMMU_MASK) -#endif - ); + (flags & HF_INHIBIT_IRQ_MASK)); /* Do not optimize repz jumps at all in icount mode, because rep movsS instructions are execured with different paths in !repz_opt and repz_opt modes. The first one was used