Re: [Qemu-devel] [PATCH v4 1/8] fw_cfg: max access size and region size are the same for MMIO data reg

2014-12-16 Thread Laszlo Ersek
On 12/16/14 21:41, Peter Maydell wrote:
> On 16 December 2014 at 19:00, Laszlo Ersek  wrote:
>> The root of this question is what each of
>>
>> enum device_endian {
>> DEVICE_NATIVE_ENDIAN,
>> DEVICE_BIG_ENDIAN,
>> DEVICE_LITTLE_ENDIAN,
>> };
>>
>> means.
> 
> An opening remark: endianness is a horribly confusing topic and
> support of more than one endianness is even worse. I may have made
> some inadvertent errors in this reply; if you think so please
> let me know and I'll have another stab at it.
> 
> That said: the device_endian options indicate what a device's
> internal idea of its endianness is. This is most relevant if
> a device accepts accesses at wider than byte width
> (for instance, if you can read a 32-bit value from
> address offset 0 and also an 8-bit value from offset 3 then
> how do those values line up? If you read a 32-bit value then
> which way round is it compared to what the device's io read/write
> function return?).
> 
> NATIVE_ENDIAN means "same order as the CPU's main data bus's
> natural representation". (Note that this is not necessarily
> the same as "the endianness the CPU currently has"; on ARM
> you can flip the CPU between LE and BE at runtime, which
> is basically inserting a byte-swizzling step between data
> accesses and the CPU's data bus, which is always LE for ARMv7+.)
> 
> Note that RAM accessible to a KVM guest is always NATIVE_ENDIAN
> (because the guest vcpu reads it directly with the h/w cpu).
> 
>> Consider the following call tree, which implements the splitting /
>> combining of an MMIO read:
>>
>> memory_region_dispatch_read() [memory.c]
>>   memory_region_dispatch_read1()
>> access_with_adjusted_size()
>>   memory_region_big_endian()
>>   for each byte in the wide access:
>> memory_region_read_accessor()
>>   fw_cfg_data_mem_read() [hw/nvram/fw_cfg.c]
>> fw_cfg_read()
>>   adjust_endianness()
>> memory_region_wrong_endianness()
>> bswapXX()
>>
>> The function access_with_adjusted_size() always iterates over the MMIO
>> address range in incrementing address order. However, it can calculate
>> the shift count for memory_region_read_accessor() in two ways.
>>
>> When memory_region_big_endian() returns "true", the shift count
>> decreases as the MMIO address increases.
>>
>> When memory_region_big_endian() returns "false", the shift count
>> increases as the MMIO address increases.
> 
> Yep, because this is how the device has told us that it thinks
> of accesses as being put together.
> 
> The column in your table "host value" is the 16 bit value read from
> the device, ie what we have decided (based on device_endian) that
> it would have returned us if it had supported a 16 bit read directly
> itself. BE devices compose 16 bit values with the high byte first,
> LE devices with the low byte first, and native-endian devices
> in the same order as guest-endianness.
> 
>> In memory_region_read_accessor(), the shift count is used for a logical
>> (ie. numeric) bitwise left shift (<<). That operator works with numeric
>> values and hides (ie. accounts for) host endianness.
>>
>> Let's consider
>> - an array of two bytes, [0xaa, 0xbb],
>> - a uint16_t access made from the guest,
>> - and all twelve possible cases.
>>
>> In the table below, the "host", "guest" and "device_endian" columns are
>> input variables. The columns to the right are calculated / derived
>> values. The arrows above the columns show the data dependencies.
>>
>> After memory_region_dispatch_read1() constructs the value that is
>> visible in the "host value" column, it is passed to adjust_endianness().
>> If memory_region_wrong_endianness() returns "true", then the host value
>> is byte-swapped. The result is then passed to the guest.
>>
>>   
>> +---+--+
>>   |  
>>  |  |
>> + 
>> --+-+
>>|  |
>> | | |
>>  |   |  |
>>   +--+-+ 
>>  |   |  |
>>   | | | || | 
>>  |   |  |
>>   |   +---+---+  ++  | | 
>>  ++---+  |  |
>>   |   | | |   | | |  ||  | | 
>>  |   ||   |  |  |
>>   |   | | |   | | v  |

[Qemu-devel] [Bug 1368815] Re: qemu-img convert intermittently corrupts output images

2014-12-16 Thread Alexei Sheplyakov
> Patch 0500-block-raw-posix-Try-both-FIEMAP-and-SEEK_HOLE.patch appears to be 
> part of a bigger re-write
> of the related code. and is ON TOP of the patches already applied in this bug.

Yep, sorry for not mentioning this. As far as I understand qemu-2.1 package 
contains this partially rewritten
code too (without any recent changes like disabling FIEMAP completely and 
rewriting the code using SEEK_HOLE).

> No doubt the rewirtten code is "better" but backporting it contains
more risk than the 2 simple fixes I already nominated.

Can we completely disable the FIEMAP code and pretend that all blocks are 
allocated? I'm afraid fsync'ing 100+ GB
files might be even slower than ignoring the sparseness.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1368815

Title:
  qemu-img convert intermittently corrupts output images

Status in Cinder:
  In Progress
Status in OpenStack Compute (Nova):
  In Progress
Status in QEMU:
  In Progress
Status in qemu package in Ubuntu:
  Fix Released
Status in qemu source package in Trusty:
  Fix Released
Status in qemu source package in Utopic:
  Fix Released
Status in qemu source package in Vivid:
  Fix Released

Bug description:
  ==
  Impact: occasional image corruption (any format on local filesystem)
  Test case: see the qemu-img command below
  Regression potential: this cherrypicks a patch from upstream to a 
not-insignificantly older qemu source tree.  While the cherrypick seems sane, 
it's possible that there are subtle interactions with the other delta.  I'd 
really like for a full qa-regression-test qemu testcase to be run against this 
package.
  ==

  -- Found in releases qemu-2.0.0, qemu-2.0.2, qemu-2.1.0. Tested on
  Ubuntu 14.04 using Ext4 filesystems.

  The command

    qemu-img convert -O raw inputimage.qcow2 outputimage.raw

  intermittently creates corrupted output images, when the input image
  is not yet fully synchronized to disk. While the issue has actually
  been discovered in operation of of OpenStack nova, it can be
  reproduced "easily" on command line using

    cat $SRC_PATH > $TMP_PATH && $QEMU_IMG_PATH convert -O raw $TMP_PATH
  $DST_PATH && cksum $DST_PATH

  on filesystems exposing this behavior. (The difficult part of this
  exercise is to prepare a filesystem to reliably trigger this race. On
  my test machine some filesystems are affected while other aren't, and
  unfortunately I haven't found the relevant difference between them,
  yet. Possible it's timing issues completely out of userspace control
  ...)

  The root cause, however, is the same as in

    http://lists.gnu.org/archive/html/coreutils/2011-04/msg00069.html

  and it can be solved the same way as suggested in

    http://lists.gnu.org/archive/html/coreutils/2011-04/msg00102.html

  In qemu, file block/raw-posix.c use the FIEMAP_FLAG_SYNC, i.e change

  f.fm.fm_flags = 0;

  to

  f.fm.fm_flags = FIEMAP_FLAG_SYNC;

  As discussed in the thread mentioned above, retrieving a page cache
  coherent map of file extents is possible only after fsync on that
  file.

  See also

    https://bugs.launchpad.net/nova/+bug/1350766

  In that bug report filed against nova, fsync had been suggested to be
  performed by the framework invoking qemu-img. However, as the choice
  of fiemap -- implying this otherwise unneeded fsync of a temporary
  file  -- is not made by the caller but by qemu-img, I agree with the
  nova bug reviewer's objection to put it into nova. The fsync should
  instead be triggered by qemu-img utilizing the FIEMAP_FLAG_SYNC,
  specifically intended for that purpose.

To manage notifications about this bug go to:
https://bugs.launchpad.net/cinder/+bug/1368815/+subscriptions



[Qemu-devel] [PATCH v1] vhost-user: fix not send all hugepage files to vhost-user

2014-12-16 Thread haifeng.lin
From: linhaifeng 

If we create VM with two or more numa nodes qemu will create two
or more hugepage files but qemu only send one hugepage file fd
to vhost-user when VM's memory size is 2G and with two numa nodes.

Signed-off-by: linhaifeng 
---
 hw/virtio/vhost-user.c  | 78 ++---
 hw/virtio/vhost.c   | 13 
 linux-headers/linux/vhost.h |  7 
 3 files changed, 73 insertions(+), 25 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index aefe0bb..439cbba 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -24,6 +24,10 @@
 #include 
 
 #define VHOST_MEMORY_MAX_NREGIONS8
+/* FIXME: same as the max number of numa node?*/
+#define HUGEPAGE_MAX_FILES   8
+
+#define RAM_SHARED (1 << 1)
 
 typedef enum VhostUserRequest {
 VHOST_USER_NONE = 0,
@@ -41,14 +45,15 @@ typedef enum VhostUserRequest {
 VHOST_USER_SET_VRING_KICK = 12,
 VHOST_USER_SET_VRING_CALL = 13,
 VHOST_USER_SET_VRING_ERR = 14,
-VHOST_USER_MAX
+VHOST_USER_MMAP_HUGEPAGE_FILE = 15,
+VHOST_USER_UNMAP_HUGEPAGE_FILE = 16,
+VHOST_USER_MAX,
 } VhostUserRequest;
 
 typedef struct VhostUserMemoryRegion {
 uint64_t guest_phys_addr;
 uint64_t memory_size;
 uint64_t userspace_addr;
-uint64_t mmap_offset;
 } VhostUserMemoryRegion;
 
 typedef struct VhostUserMemory {
@@ -57,6 +62,16 @@ typedef struct VhostUserMemory {
 VhostUserMemoryRegion regions[VHOST_MEMORY_MAX_NREGIONS];
 } VhostUserMemory;
 
+typedef struct HugepageMemoryInfo {
+uint64_t base_addr;
+uint64_t size;
+}HugeMemInfo;
+
+typedef struct HugepageInfo {
+uint32_t num;
+HugeMemInfo files[HUGEPAGE_MAX_FILES];
+}HugepageInfo;
+
 typedef struct VhostUserMsg {
 VhostUserRequest request;
 
@@ -71,6 +86,7 @@ typedef struct VhostUserMsg {
 struct vhost_vring_state state;
 struct vhost_vring_addr addr;
 VhostUserMemory memory;
+HugepageInfo huge_info;
 };
 } QEMU_PACKED VhostUserMsg;
 
@@ -104,7 +120,9 @@ static unsigned long int 
ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
 VHOST_GET_VRING_BASE,   /* VHOST_USER_GET_VRING_BASE */
 VHOST_SET_VRING_KICK,   /* VHOST_USER_SET_VRING_KICK */
 VHOST_SET_VRING_CALL,   /* VHOST_USER_SET_VRING_CALL */
-VHOST_SET_VRING_ERR /* VHOST_USER_SET_VRING_ERR */
+VHOST_SET_VRING_ERR,/* VHOST_USER_SET_VRING_ERR */
+VHOST_MMAP_HUGEPAGE_FILE,  /* VHOST_USER_MMAP_HUGEPAGE_FILE */
+VHOST_UNMAP_HUGEPAGE_FILE, /* VHOST_USER_UNMAP_HUGEPAGE_FILE */
 };
 
 static VhostUserRequest vhost_user_request_translate(unsigned long int request)
@@ -190,6 +208,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned 
long int request,
 int fds[VHOST_MEMORY_MAX_NREGIONS];
 int i, fd;
 size_t fd_num = 0;
+RAMBlock *block;
 
 assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
 
@@ -213,37 +232,46 @@ static int vhost_user_call(struct vhost_dev *dev, 
unsigned long int request,
 case VHOST_RESET_OWNER:
 break;
 
-case VHOST_SET_MEM_TABLE:
-for (i = 0; i < dev->mem->nregions; ++i) {
-struct vhost_memory_region *reg = dev->mem->regions + i;
-ram_addr_t ram_addr;
+case VHOST_MMAP_HUGEPAGE_FILE:
+qemu_mutex_lock_ramlist();
 
-assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
-qemu_ram_addr_from_host((void *)(uintptr_t)reg->userspace_addr, 
&ram_addr);
-fd = qemu_get_ram_fd(ram_addr);
-if (fd > 0) {
-msg.memory.regions[fd_num].userspace_addr = 
reg->userspace_addr;
-msg.memory.regions[fd_num].memory_size  = reg->memory_size;
-msg.memory.regions[fd_num].guest_phys_addr = 
reg->guest_phys_addr;
-msg.memory.regions[fd_num].mmap_offset = reg->userspace_addr -
-(uintptr_t) qemu_get_ram_block_host_ptr(ram_addr);
-assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
-fds[fd_num++] = fd;
+/* Get hugepage file informations */
+QTAILQ_FOREACH(block, &ram_list.blocks, next) {
+if (block->flags & RAM_SHARED && block->fd > 0) {
+msg.huge_info.files[fd_num].size = block->length;
+msg.huge_info.files[fd_num].base_addr = block->host;
+fds[fd_num++] = block->fd;
 }
 }
+msg.huge_info.num = fd_num;
 
-msg.memory.nregions = fd_num;
+/* Calculate msg size */
+msg.size = sizeof(m.huge_info.num);
+msg.size += fd_num * sizeof(HugeMemInfo);
+
+qemu_mutex_unlock_ramlist();
+break;
 
-if (!fd_num) {
-error_report("Failed initializing vhost-user memory map\n"
-"consider using -object memory-backend-file share=on\n");
-return -1;
+case VHOST_UNMAP_HUGEPAGE_FILE:
+/* Tell vhost-user to u

Re: [Qemu-devel] [PATCH v7 0/3] machvirt dynamic sysbus device instantiation

2014-12-16 Thread Shannon Zhao
On 2014/12/16 18:42, Eric Auger wrote:
> This patch series enables machvirt to dynamically instantiate sysbus
> devices from command line (using -device option).
> 
> All those sysbus devices are plugged onto a platform bus. This latter
> device is instantiated in machvirt and takes care of the binding of
> children sysbus devices on a machine init done notifier. The device
> tree node generation for children dynamic sysbus device also happens
> on a subsequent notifier that must be executed after the above one.
> machvirt registers that notifier before the platform bus creation to
> make sure notifiers are executed in the right order: dt generation after
> actual QOM binding.
> 
> Very few sysbus devices are supposed to be instantiated that
> way. VFIO devices belong to them.
> 
> Node creation really is architecture specific. On ARM the dynamic
> sysbus device node creation is implemented in a new C module,
> hw/arm/sysbus-fdt.c and not in the machine file.
> 
> Machvirt transformations and sysbus-fdt are largely inspired from Alex work.
> 
> The patch series can be found at:
> http://git.linaro.org/people/eric.auger/qemu.git
> branch official_dynsysbus_v7
> 

Reviewed-by: Shannon Zhao 

Thanks,
Shannon




Re: [Qemu-devel] [PATCH v4 1/8] fw_cfg: max access size and region size are the same for MMIO data reg

2014-12-16 Thread Laszlo Ersek
On 12/16/14 21:40, Paolo Bonzini wrote:
> On 16/12/2014 21:06, Laszlo Ersek wrote:

>> You flipped the combined ops to LE in commit 6fdf98f2 (and, apparently,
>> I reviewed it). Shouldn't we do the same for the standalone selector?
> 
> No.  The standalone selector is used as MMIO, and the BE platforms
> expect the platform to be big-endian.  The combined ops are only used on
> ISA ports, where the firmware expects them to be little-endian (as
> mentioned in the commit message).
> 
> That said, the standalone selector is used by BE platforms only, so we
> know that the standalone selector is always DEVICE_BIG_ENDIAN.

This series exposes the standalone selector (as MMIO) to ARM guests as
well; and in "Documentation/devicetree/bindings/arm/fw-cfg.txt" for the
kernel I'm saying that the selector is little endian.

Therefore I think that the standalone selector is not only (going to be)
used by BE platforms (or I don't understand your above statement
correctly). But, the current (and to be preserved) NATIVE_ENDIAN setting
still matches what I say in
"Documentation/devicetree/bindings/arm/fw-cfg.txt", because, Peter said:

> NATIVE_ENDIAN means "same order as the CPU's main data bus's natural
> representation". (Note that this is not necessarily the same as "the
> endianness the CPU currently has"; on ARM you can flip the CPU between
> LE and BE at runtime, which is basically inserting a byte-swizzling
> step between data accesses and the CPU's data bus, which is always LE
> for ARMv7+.)

In other words, the standalone selector is NATIVE_ENDIAN, but in the
description of the *ARM* bindings, we can simply say that it's little
endian.

Is that right?

Thanks
Laszlo

> 
> So if you want, you can make the standalone selector and the standalone
> datum BE and swap them in the firmware.  If the suggestion doesn't make
> you jump up and down, I understand that. :)
> 
> Paolo
> 




[Qemu-devel] [PATCH V3 8/8] pc: acpi-build: simplify PCI bus tree generation

2014-12-16 Thread Igor Mammedov
it basicaly does the same as original approach,
* just without bus/notify tables tracking (less obscure)
  which is easier to follow.
* drops unnecessary loops and bitmaps,
  creating devices and notification method in the same loop.
* saves us ~100LOC

change in behavior:
* generate hotpluggable device entries for empty slots
  only if BUS itself is hotpluggable, otherwise do not
  create them.

Signed-off-by: Igor Mammedov 
---
v3:
  * use hotpluggable device object instead of not hotpluggable
for non present devices, and add it only when bus itself is
hotpluggable
---
 hw/i386/acpi-build.c | 265 +++
 1 file changed, 79 insertions(+), 186 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 707ac7e..da160c1 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -95,7 +95,6 @@ typedef struct AcpiPmInfo {
 typedef struct AcpiMiscInfo {
 bool has_hpet;
 bool has_tpm;
-DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX);
 const unsigned char *dsdt_code;
 unsigned dsdt_size;
 uint16_t pvpanic_port;
@@ -641,69 +640,37 @@ static void acpi_set_pci_info(void)
 }
 }
 
-static void build_pci_bus_state_init(AcpiBuildPciBusHotplugState *state,
- AcpiBuildPciBusHotplugState *parent,
- bool pcihp_bridge_en)
+static void build_append_pcihp_notify_entry(GArray *method, int slot)
 {
-state->parent = parent;
-state->device_table = build_alloc_array();
-state->notify_table = build_alloc_array();
-state->pcihp_bridge_en = pcihp_bridge_en;
-}
-
-static void build_pci_bus_state_cleanup(AcpiBuildPciBusHotplugState *state)
-{
-build_free_array(state->device_table);
-build_free_array(state->notify_table);
-}
+GArray *ifctx;
 
-static void *build_pci_bus_begin(PCIBus *bus, void *parent_state)
-{
-AcpiBuildPciBusHotplugState *parent = parent_state;
-AcpiBuildPciBusHotplugState *child = g_malloc(sizeof *child);
-
-build_pci_bus_state_init(child, parent, parent->pcihp_bridge_en);
+ifctx = build_alloc_array();
+build_append_byte(ifctx, 0x7B); /* AndOp */
+build_append_byte(ifctx, 0x68); /* Arg0Op */
+build_append_int(ifctx, 0x1U << slot);
+build_append_byte(ifctx, 0x00); /* NullName */
+build_append_byte(ifctx, 0x86); /* NotifyOp */
+build_append_namestring(ifctx, "S%.02X", PCI_DEVFN(slot, 0));
+build_append_byte(ifctx, 0x69); /* Arg1Op */
 
-return child;
+/* Pack it up */
+build_package(ifctx, 0xA0 /* IfOp */);
+build_append_array(method, ifctx);
+build_free_array(ifctx);
 }
 
-static void build_pci_bus_end(PCIBus *bus, void *bus_state)
+static void build_append_pci_bus_devices(GArray *parent_scope, PCIBus *bus,
+ bool pcihp_bridge_en)
 {
-AcpiBuildPciBusHotplugState *child = bus_state;
-AcpiBuildPciBusHotplugState *parent = child->parent;
 GArray *bus_table = build_alloc_array();
-DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX);
-DECLARE_BITMAP(slot_device_present, PCI_SLOT_MAX);
-DECLARE_BITMAP(slot_device_system, PCI_SLOT_MAX);
-DECLARE_BITMAP(slot_device_vga, PCI_SLOT_MAX);
-DECLARE_BITMAP(slot_device_qxl, PCI_SLOT_MAX);
-uint8_t op;
-int i;
+GArray *method = NULL;
 QObject *bsel;
-GArray *method;
-bool bus_hotplug_support = false;
-
-/*
- * Skip bridge subtree creation if bridge hotplug is disabled
- * to make acpi tables compatible with legacy machine types.
- */
-if (!child->pcihp_bridge_en && bus->parent_dev) {
-return;
-}
+PCIBus *sec;
+int i;
 
 if (bus->parent_dev) {
-op = 0x82; /* DeviceOp */
-build_append_namestring(bus_table, "S%.02X",
- bus->parent_dev->devfn);
-build_append_byte(bus_table, 0x08); /* NameOp */
-build_append_namestring(bus_table, "_SUN");
-build_append_value(bus_table, PCI_SLOT(bus->parent_dev->devfn), 1);
-build_append_byte(bus_table, 0x08); /* NameOp */
-build_append_namestring(bus_table, "_ADR");
-build_append_value(bus_table, (PCI_SLOT(bus->parent_dev->devfn) << 16) 
|
-   PCI_FUNC(bus->parent_dev->devfn), 4);
+build_append_namestring(bus_table, "S%.02X_", bus->parent_dev->devfn);
 } else {
-op = 0x10; /* ScopeOp */;
 build_append_namestring(bus_table, "PCI0");
 }
 
@@ -712,171 +679,103 @@ static void build_pci_bus_end(PCIBus *bus, void 
*bus_state)
 build_append_byte(bus_table, 0x08); /* NameOp */
 build_append_namestring(bus_table, "BSEL");
 build_append_int(bus_table, qint_get_int(qobject_to_qint(bsel)));
-memset(slot_hotplug_enable, 0xff, sizeof slot_hotplug_enable);
-} else {
-/* No bsel - no slots are hot-pluggable */
-memset(slot_hotplug_enable, 0x00, sizeof slot_hotplug_enable);
+ 

Re: [Qemu-devel] [PATCH v4 1/8] fw_cfg: max access size and region size are the same for MMIO data reg

2014-12-16 Thread Laszlo Ersek
On 12/16/14 22:47, Paolo Bonzini wrote:
> On 16/12/2014 21:17, Laszlo Ersek wrote:
 I can't imagine how that would happen; fw_cfg_data_mem_read() ignores
 both "addr" and "size", and fw_cfg_read() simply advances the
 "cur_offset" member.
>> Ah okay, I understand your point now; you're probably saying that
>> access_with_adjusted_size() traverses the offsets in the wrong order.
>> ... I don't see how; the only difference in the access() param list is
>> the shift count. (I don't know how it should work by design.)
> 
> I think I have figured it out.
> 
> Guest endianness affects where those bytes are placed, but not the order
> in which they are fetched; and the effects of guest endianness are
> always cleaned up by adjust_endianness, so ultimately they do not matter.
> 
> Think of how you would implement the uint64_t read in fw_cfg:
> 
> File bytes 12 34 56 78 9a bc de f0
> 
> fw_cfg_data_mem_read should read
> 
> size==4 BE host0x12345678
> size==4 LE host0x78563412
> size==8 BE host0x123456789abcdef0
> size==8 LE host0xf0debc9a78563412
> 
> So the implementation of fw_cfg_data_mem_read must depend on host
> endianness.  Instead, memory.c always fills in bytes in the same order,
> on the assumption that the reads are idempotent.

I see. Thanks!
Laszlo




Re: [Qemu-devel] [PATCH v3 04/10] vnc: switch to QemuOpts, allow multiple servers

2014-12-16 Thread Gonglei
On 2014/12/16 21:20, Gerd Hoffmann wrote:

> This patch switches vnc over to QemuOpts, and it (more or less
> as side effect) allows multiple vnc server instances.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  include/ui/console.h |   4 +-
>  qmp.c|  15 ++-
>  ui/vnc.c | 271 
> ---
>  vl.c |  42 +++-
>  4 files changed, 200 insertions(+), 132 deletions(-)
> 

Hi, Gerd
I'm afraid you forgot to update this patch following comments of version 2. :)

Regards,
-Gonglei

> diff --git a/include/ui/console.h b/include/ui/console.h
> index 5ff2e27..887ed91 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -328,12 +328,14 @@ void cocoa_display_init(DisplayState *ds, int 
> full_screen);
>  
>  /* vnc.c */
>  void vnc_display_init(const char *id);
> -void vnc_display_open(const char *id, const char *display, Error **errp);
> +void vnc_display_open(const char *id, Error **errp);
>  void vnc_display_add_client(const char *id, int csock, bool skipauth);
>  char *vnc_display_local_addr(const char *id);
>  #ifdef CONFIG_VNC
>  int vnc_display_password(const char *id, const char *password);
>  int vnc_display_pw_expire(const char *id, time_t expires);
> +QemuOpts *vnc_parse_func(const char *str);
> +int vnc_init_func(QemuOpts *opts, void *opaque);
>  #else
>  static inline int vnc_display_password(const char *id, const char *password)
>  {
> diff --git a/qmp.c b/qmp.c
> index 0b4f131..3fda973 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -368,7 +368,20 @@ void qmp_change_vnc_password(const char *password, Error 
> **errp)
>  
>  static void qmp_change_vnc_listen(const char *target, Error **errp)
>  {
> -vnc_display_open(NULL, target, errp);
> +QemuOptsList *olist = qemu_find_opts("vnc");
> +QemuOpts *opts;
> +
> +if (strstr(target, "id=")) {
> +error_setg(errp, "id not supported");
> +return;
> +}
> +
> +opts = qemu_opts_find(olist, "default");
> +if (opts) {
> +qemu_opts_del(opts);
> +}
> +opts = vnc_parse_func(target);
> +vnc_init_func(opts, NULL);
>  }
>  
>  static void qmp_change_vnc(const char *target, bool has_arg, const char *arg,
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 1b86365..cf8bed8 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -31,6 +31,7 @@
>  #include "qemu/sockets.h"
>  #include "qemu/timer.h"
>  #include "qemu/acl.h"
> +#include "qemu/config-file.h"
>  #include "qapi/qmp/types.h"
>  #include "qmp-commands.h"
>  #include "qemu/osdep.h"
> @@ -2969,7 +2970,12 @@ static const DisplayChangeListenerOps dcl_ops = {
>  
>  void vnc_display_init(const char *id)
>  {
> -VncDisplay *vs = g_malloc0(sizeof(*vs));
> +VncDisplay *vs;
> +
> +if (vnc_display_find(id) != NULL) {
> +return;
> +}
> +vs = g_malloc0(sizeof(*vs));
>  
>  vs->id = strdup(id);
>  QTAILQ_INSERT_TAIL(&vnc_displays, vs, next);
> @@ -3065,14 +3071,65 @@ char *vnc_display_local_addr(const char *id)
>  return vnc_socket_local_addr("%s:%s", vs->lsock);
>  }
>  
> -void vnc_display_open(const char *id, const char *display, Error **errp)
> +static QemuOptsList qemu_vnc_opts = {
> +.name = "vnc",
> +.head = QTAILQ_HEAD_INITIALIZER(qemu_vnc_opts.head),
> +.implied_opt_name = "vnc",
> +.desc = {
> +{
> +.name = "vnc",
> +.type = QEMU_OPT_STRING,
> +},{
> +.name = "websocket",
> +.type = QEMU_OPT_STRING,
> +},{
> +.name = "x509",
> +.type = QEMU_OPT_STRING,
> +},{
> +.name = "share",
> +.type = QEMU_OPT_STRING,
> +},{
> +.name = "password",
> +.type = QEMU_OPT_BOOL,
> +},{
> +.name = "reverse",
> +.type = QEMU_OPT_BOOL,
> +},{
> +.name = "lock-key-sync",
> +.type = QEMU_OPT_BOOL,
> +},{
> +.name = "sasl",
> +.type = QEMU_OPT_BOOL,
> +},{
> +.name = "tls",
> +.type = QEMU_OPT_BOOL,
> +},{
> +.name = "x509verify",
> +.type = QEMU_OPT_BOOL,
> +},{
> +.name = "acl",
> +.type = QEMU_OPT_BOOL,
> +},{
> +.name = "lossy",
> +.type = QEMU_OPT_BOOL,
> +},{
> +.name = "non-adaptive",
> +.type = QEMU_OPT_BOOL,
> +},
> +{ /* end of list */ }
> +},
> +};
> +
> +void vnc_display_open(const char *id, Error **errp)
>  {
>  VncDisplay *vs = vnc_display_find(id);
> -const char *options;
> +QemuOpts *opts = qemu_opts_find(&qemu_vnc_opts, id);
> +const char *display, *websocket, *share;
>  int password = 0;
>  int reverse = 0;
>  #ifdef CONFIG_VNC_TLS
>  int tls = 0, x509 = 0;
> +const char *path;
>  #endif
>  #ifdef CONFIG_VNC_SASL
>  int sasl = 0;
> @@ -3088,115 +3145,86 @@ void vnc_displa

Re: [Qemu-devel] [PULL 0/5] bootdevice patches

2014-12-16 Thread Gonglei
On 2014/12/16 22:01, Peter Maydell wrote:

> On 16 December 2014 at 09:22,   wrote:
>> From: root 
>>
>> This is my first pull request as a submaintainer. Those patches just
>> move boot order related code to bootdevice.c and add a Error **errp
>> argument for corresponding functions so that it can propagate error messages
>> to the caller. Please pull.
> 
> This also seems to cause 'make check' to fail:
> 
> TEST: tests/boot-order-test... (pid=16958)
>   /i386/boot-order/pc:
> Broken pipe
> FAIL
> GTester: last random seed: R02S5702c094d31af53e45fffabed844705a
> (pid=16973)
> FAIL: tests/boot-order-test
> 

Oops, That's because of a typo in patch 2,  'once' -> 'order'.
 'make check' should be executed before posting patches :(
I apologize for any inconvenience caused during this process,
and great thanks for your precise job.

Regards,
-Gonglei




Re: [Qemu-devel] [PULL 5/5] bootdevice: add Error **errp argument for QEMUBootSetHandler

2014-12-16 Thread Gonglei
On 2014/12/16 21:23, Peter Maydell wrote:

> On 16 December 2014 at 13:04, Gonglei  wrote:
>> On 2014/12/16 20:42, Peter Maydell wrote:
>>
>>> On 16 December 2014 at 09:22,   wrote:
 @@ -412,9 +411,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t 
 above_4g_mem_size,
  object_property_set_link(OBJECT(machine), OBJECT(s),
   "rtc_state", &error_abort);

 -if (set_boot_dev(s, boot_device)) {
 -exit(1);
 -}
 +set_boot_dev(s, boot_device, &error_abort);
>>>
>>> This turns a "print error message and exit" path into
>>> an abort(), which doesn't seem right (this can be triggered
>>> by bad user input arguments, yes?). error_abort should
>>> only be used in cases where you would assert() if there
>>> was an error (ie where it would be a QEMU bug if it
>>> happened).
>>>
>>
>> Yes, agree. How does use a incremental patch fix this, Peter?
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 99deba6..d7822b8 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -364,6 +364,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t 
>> above_4g_mem_size,
>>  FDriveType fd_type[2] = { FDRIVE_DRV_NONE, FDRIVE_DRV_NONE };
>>  static pc_cmos_init_late_arg arg;
>>  PCMachineState *pc_machine = PC_MACHINE(machine);
>> +Error *local_err = NULL;
>>
>>  /* various important CMOS locations needed by PC/Bochs bios */
>>
>> @@ -411,7 +412,10 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t 
>> above_4g_mem_size,
>>  object_property_set_link(OBJECT(machine), OBJECT(s),
>>   "rtc_state", &error_abort);
>>
>> -set_boot_dev(s, boot_device, &error_abort);
>> +set_boot_dev(s, boot_device, &local_err);
>> +if (local_err) {
>> +exit(1);
>> +}
> 
> That won't print the error message at all...
> 
Yes, I see. Thanks. I will send a new pull request :)


Regards,
-Gonglei





[Qemu-devel] [PULL 0/5] target-xtensa queue 2014-12-17

2014-12-16 Thread Max Filippov
Hi Peter,

please pull my current target-xtensa queue.
The following changes since commit d86fb03469e016af4e54f04efccbc20a8afa3e19:

  Merge remote-tracking branch 'remotes/spice/tags/pull-spice-20141216-1' into 
staging (2014-12-16 16:52:42 +)

are available in the git repository at:

  git://github.com/OSLL/qemu-xtensa.git tags/20141217-xtensa

for you to fetch changes up to 97e89ee914411384dcda771d38bf89f13726d71e:

  target-xtensa: don't generate dead code (2014-12-17 05:49:32 +0300)


Xtensa updates for 2.3:

- fix cross-page opcode handling;
- move window overflow exception generation decision to translation phase;
- don't generate dead code after privilege, window overflow or coprocessor
  exception;
- add monitor command 'info opcount' for dumping TCG opcode counters.


Max Filippov (5):
  tcg: add separate monitor command to dump opcode counters
  target-xtensa: fix translation for opcodes crossing page boundary
  target-xtensa: test cross-page opcode
  target-xtensa: record available window in TB flags
  target-xtensa: don't generate dead code

 include/exec/cpu-all.h  |   1 +
 monitor.c   |  12 +
 target-xtensa/cpu.h |  12 +
 target-xtensa/helper.h  |   2 +-
 target-xtensa/op_helper.c   |  29 +-
 target-xtensa/translate.c   | 688 +++-
 tcg/tcg.c   |  12 +-
 tcg/tcg.h   |   1 +
 tests/tcg/xtensa/test_mmu.S |  26 +-
 translate-all.c |   5 +
 10 files changed, 426 insertions(+), 362 deletions(-)

-- 
1.8.1.4



Re: [Qemu-devel] [PATCH v2 14/17] ahci: Migrate IDEStatus

2014-12-16 Thread John Snow



On 12/16/2014 08:36 PM, John Snow wrote:

Amazingly, we weren't doing this before.

Make sure we migrate the IDEState structure that belongs to
the AHCIDevice.IDEBus structure during migrations.

No version numbering changes because AHCI is not officially
migratable (and we can all see with good reason why) so we
do not impact any official builds by altering the stream and
leaving it at version 1.

This fixes the rerror=stop/werror=stop test case where we wish
to migrate a halted job. Previously, the error code would not
migrate, so even if the job completed successfully, AHCI would
report an error because it would still have the placeholder
error code from initialization time.

Signed-off-by: John Snow 
---
  hw/ide/ahci.c | 1 +
  hw/ide/internal.h | 3 +++
  2 files changed, 4 insertions(+)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index bc6d5ce..3f4fc77 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1321,6 +1321,7 @@ static const VMStateDescription vmstate_ahci_device = {
  .version_id = 1,
  .fields = (VMStateField[]) {
  VMSTATE_IDE_BUS(port, AHCIDevice),
+VMSTATE_IDE_DRIVE(port.ifs[0], AHCIDevice),
  VMSTATE_UINT32(port_state, AHCIDevice),
  VMSTATE_UINT32(finished, AHCIDevice),
  VMSTATE_UINT32(port_regs.lst_addr, AHCIDevice),
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 0beba43..c278dea 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -524,6 +524,9 @@ extern const VMStateDescription vmstate_ide_drive;
  #define VMSTATE_IDE_DRIVES(_field, _state) \
  VMSTATE_STRUCT_ARRAY(_field, _state, 2, 3, vmstate_ide_drive, IDEState)

+#define VMSTATE_IDE_DRIVE(_field, _state) \
+VMSTATE_STRUCT(_field, _state, 1, vmstate_ide_drive, IDEState)
+


Please check that this versioning is sane; I tested that it let me 
migrate AHCI successfully, but I don't know if this is semantically "right."


Please and thank you!
--js


  void ide_bus_reset(IDEBus *bus);
  int64_t ide_get_sector(IDEState *s);
  void ide_set_sector(IDEState *s, int64_t sector_num);





[Qemu-devel] [PATCH v2 16/17] ahci: Recompute cur_cmd on migrate post load

2014-12-16 Thread John Snow
When the AHCI HBA device is migrated, all of the information that
led to the request being created is stored in the AHCIDevice
structures, except for pointers into guest data where return
information needs to be stored.

The "cur_cmd" field is usually responsible for this.

To rebuild the cur_cmd pointer post-migration, we can utilize
the busy_slot index to figure out where the command header
we are still processing is.

This allows a machine in a halted state from rerror=stop or
werror=stop to be migrated and resume operations without issue.

Signed-off-by: John Snow 
---
 hw/ide/ahci.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index c153228..8078d3e 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1373,6 +1373,10 @@ static int ahci_state_post_load(void *opaque, int 
version_id)
  */
 if (ad->busy_slot == -1) {
 check_cmd(s, i);
+} else {
+/* We are in the middle of a command, and may need to access
+ * the command header in guest memory again. */
+ad->cur_cmd = &((AHCICmdHdr *)ad->lst)[ad->busy_slot];
 }
 }
 
-- 
1.9.3




[Qemu-devel] [PATCH v2 14/17] ahci: Migrate IDEStatus

2014-12-16 Thread John Snow
Amazingly, we weren't doing this before.

Make sure we migrate the IDEState structure that belongs to
the AHCIDevice.IDEBus structure during migrations.

No version numbering changes because AHCI is not officially
migratable (and we can all see with good reason why) so we
do not impact any official builds by altering the stream and
leaving it at version 1.

This fixes the rerror=stop/werror=stop test case where we wish
to migrate a halted job. Previously, the error code would not
migrate, so even if the job completed successfully, AHCI would
report an error because it would still have the placeholder
error code from initialization time.

Signed-off-by: John Snow 
---
 hw/ide/ahci.c | 1 +
 hw/ide/internal.h | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index bc6d5ce..3f4fc77 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1321,6 +1321,7 @@ static const VMStateDescription vmstate_ahci_device = {
 .version_id = 1,
 .fields = (VMStateField[]) {
 VMSTATE_IDE_BUS(port, AHCIDevice),
+VMSTATE_IDE_DRIVE(port.ifs[0], AHCIDevice),
 VMSTATE_UINT32(port_state, AHCIDevice),
 VMSTATE_UINT32(finished, AHCIDevice),
 VMSTATE_UINT32(port_regs.lst_addr, AHCIDevice),
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 0beba43..c278dea 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -524,6 +524,9 @@ extern const VMStateDescription vmstate_ide_drive;
 #define VMSTATE_IDE_DRIVES(_field, _state) \
 VMSTATE_STRUCT_ARRAY(_field, _state, 2, 3, vmstate_ide_drive, IDEState)
 
+#define VMSTATE_IDE_DRIVE(_field, _state) \
+VMSTATE_STRUCT(_field, _state, 1, vmstate_ide_drive, IDEState)
+
 void ide_bus_reset(IDEBus *bus);
 int64_t ide_get_sector(IDEState *s);
 void ide_set_sector(IDEState *s, int64_t sector_num);
-- 
1.9.3




[Qemu-devel] [PATCH v2 13/17] ide: support PIO restart for the ISA controller

2014-12-16 Thread John Snow
From: Paolo Bonzini 

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
---
 hw/ide/isa.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index b084162..5eb35c2 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -74,7 +74,8 @@ static void isa_ide_realizefn(DeviceState *dev, Error **errp)
 isa_init_irq(isadev, &s->irq, s->isairq);
 ide_init2(&s->bus, s->irq);
 vmstate_register(dev, 0, &vmstate_ide_isa, s);
-};
+ide_register_restart_cb(&s->bus);
+}
 
 ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int isairq,
 DriveInfo *hd0, DriveInfo *hd1)
-- 
1.9.3




[Qemu-devel] [PATCH v2 17/17] qtest/ide: Test flush / retry for ISA and PCI

2014-12-16 Thread John Snow
This patch adds tests for werror and rerror functionality
for the PCI and ISA ide buses.

Tests for the AHCI device are to be included at a later
date after requisite patches have been merged upstream
to support needed functionality by the tests.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
---
 tests/ide-test.c | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/tests/ide-test.c b/tests/ide-test.c
index 29f4039..b28a302 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -118,7 +118,6 @@ static void ide_test_start(const char *cmdline_fmt, ...)
 va_end(ap);
 
 qtest_start(cmdline);
-qtest_irq_intercept_in(global_qtest, "ioapic");
 guest_malloc = pc_alloc_init();
 
 g_free(cmdline);
@@ -388,6 +387,7 @@ static void test_bmdma_setup(void)
 "-drive file=%s,if=ide,serial=%s,cache=writeback,format=raw "
 "-global ide-hd.ver=%s",
 tmp_path, "testdisk", "version");
+qtest_irq_intercept_in(global_qtest, "ioapic");
 }
 
 static void test_bmdma_teardown(void)
@@ -516,7 +516,7 @@ static void prepare_blkdebug_script(const char *debug_fn, 
const char *event)
 g_assert(ret == 0);
 }
 
-static void test_retry_flush(void)
+static void test_retry_flush(const char *machine)
 {
 uint8_t data;
 const char *s;
@@ -580,6 +580,16 @@ static void test_flush_nodev(void)
 ide_test_quit();
 }
 
+static void test_pci_retry_flush(const char *machine)
+{
+test_retry_flush("pc");
+}
+
+static void test_isa_retry_flush(const char *machine)
+{
+test_retry_flush("isapc");
+}
+
 int main(int argc, char **argv)
 {
 const char *arch = qtest_get_arch();
@@ -617,9 +627,9 @@ int main(int argc, char **argv)
 qtest_add_func("/ide/bmdma/teardown", test_bmdma_teardown);
 
 qtest_add_func("/ide/flush", test_flush);
-qtest_add_func("/ide/flush_nodev", test_flush_nodev);
-
-qtest_add_func("/ide/retry/flush", test_retry_flush);
+qtest_add_func("/ide/flush/nodev", test_flush_nodev);
+qtest_add_func("/ide/flush/retry_pci", test_pci_retry_flush);
+qtest_add_func("/ide/flush/retry_isa", test_isa_retry_flush);
 
 ret = g_test_run();
 
-- 
1.9.3




[Qemu-devel] [PATCH v2 10/17] ide: migrate initial request state via IDEBus

2014-12-16 Thread John Snow
From: Paolo Bonzini 

This only breaks backwards migration compatibility if the bus is in
an error state.  It is in principle possible to avoid this by making
two subsections (one for version 1, and one for version 2, but with
the same name) with different "_needed" callbacks.  The v1 callback would
return true if error_status != 0 and the bus is PATA; the v2 callback
would return true if error_status != 0 and the bus is AHCI.

Forward migration keeps working.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
---
 hw/ide/core.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index d4659dc..0459ea1 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2633,10 +2633,13 @@ const VMStateDescription vmstate_ide_drive = {
 
 static const VMStateDescription vmstate_ide_error_status = {
 .name ="ide_bus/error",
-.version_id = 1,
+.version_id = 2,
 .minimum_version_id = 1,
 .fields = (VMStateField[]) {
 VMSTATE_INT32(error_status, IDEBus),
+VMSTATE_INT64_V(retry_sector_num, IDEBus, 2),
+VMSTATE_UINT32_V(retry_nsector, IDEBus, 2),
+VMSTATE_UINT8_V(retry_unit, IDEBus, 2),
 VMSTATE_END_OF_LIST()
 }
 };
-- 
1.9.3




[Qemu-devel] [PATCH v2 08/17] ide: replace set_unit callback with more IDEBus state

2014-12-16 Thread John Snow
From: Paolo Bonzini 

Start moving the initial state of the current request to IDEBus, so that
AHCI can use it.  The set_unit callback is not used anymore once this is
done.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
---
 hw/ide/ahci.c |  7 ---
 hw/ide/core.c |  6 --
 hw/ide/internal.h |  2 +-
 hw/ide/macio.c|  1 -
 hw/ide/pci.c  | 19 ++-
 hw/ide/pci.h  |  6 --
 6 files changed, 15 insertions(+), 26 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 3892025..bc6d5ce 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1226,12 +1226,6 @@ static int ahci_dma_rw_buf(IDEDMA *dma, int is_write)
 return 1;
 }
 
-static int ahci_dma_set_unit(IDEDMA *dma, int unit)
-{
-/* only a single unit per link */
-return 0;
-}
-
 static void ahci_cmd_done(IDEDMA *dma)
 {
 AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
@@ -1258,7 +1252,6 @@ static const IDEDMAOps ahci_dma_ops = {
 .prepare_buf = ahci_dma_prepare_buf,
 .commit_buf = ahci_commit_buf,
 .rw_buf = ahci_dma_rw_buf,
-.set_unit = ahci_dma_set_unit,
 .cmd_done = ahci_cmd_done,
 };
 
diff --git a/hw/ide/core.c b/hw/ide/core.c
index f3d98d7..fc5d227 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -646,6 +646,7 @@ static void dma_buf_commit(IDEState *s, uint32_t tx_bytes)
 void ide_set_inactive(IDEState *s, bool more)
 {
 s->bus->dma->aiocb = NULL;
+s->bus->retry_unit = -1;
 if (s->bus->dma->ops->set_inactive) {
 s->bus->dma->ops->set_inactive(s->bus->dma, more);
 }
@@ -666,7 +667,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int 
op)
 BlockErrorAction action = blk_get_error_action(s->blk, is_read, error);
 
 if (action == BLOCK_ERROR_ACTION_STOP) {
-s->bus->dma->ops->set_unit(s->bus->dma, s->unit);
+assert(s->bus->retry_unit == s->unit);
 s->bus->error_status = op;
 } else if (action == BLOCK_ERROR_ACTION_REPORT) {
 if (op & IDE_RETRY_DMA) {
@@ -799,6 +800,7 @@ static void ide_sector_start_dma(IDEState *s, enum 
ide_dma_cmd dma_cmd)
 
 void ide_start_dma(IDEState *s, BlockCompletionFunc *cb)
 {
+s->bus->retry_unit = s->unit;
 if (s->bus->dma->ops->start_dma) {
 s->bus->dma->ops->start_dma(s->bus->dma, s, cb);
 }
@@ -2327,11 +2329,11 @@ static const IDEDMAOps ide_dma_nop_ops = {
 .prepare_buf= ide_nop_int32,
 .restart_dma= ide_nop,
 .rw_buf = ide_nop_int,
-.set_unit   = ide_nop_int,
 };
 
 static void ide_restart_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
 {
+s->unit = s->bus->retry_unit;
 s->bus->dma->ops->restart_dma(s->bus->dma);
 s->io_buffer_index = 0;
 s->io_buffer_size = 0;
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 53ce932..07d82c4 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -434,7 +434,6 @@ struct IDEDMAOps {
 DMAInt32Func *prepare_buf;
 DMAu32Func *commit_buf;
 DMAIntFunc *rw_buf;
-DMAIntFunc *set_unit;
 DMAVoidFunc *restart_dma;
 DMAStopFunc *set_inactive;
 DMAVoidFunc *cmd_done;
@@ -463,6 +462,7 @@ struct IDEBus {
 qemu_irq irq;
 
 int error_status;
+uint8_t retry_unit;
 };
 
 #define TYPE_IDE_DEVICE "ide-device"
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index e7a5c99..a009674 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -572,7 +572,6 @@ static const IDEDMAOps dbdma_ops = {
 .start_dma  = ide_dbdma_start,
 .prepare_buf= ide_nop_int32,
 .rw_buf = ide_nop_int,
-.set_unit   = ide_nop_int,
 };
 
 static void macio_ide_realizefn(DeviceState *dev, Error **errp)
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index cd1bbb0..2b0e886 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -42,7 +42,6 @@ static void bmdma_start_dma(IDEDMA *dma, IDEState *s,
 {
 BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma);
 
-bm->unit = s->unit;
 bm->dma_cb = dma_cb;
 bm->cur_prd_last = 0;
 bm->cur_prd_addr = 0;
@@ -163,20 +162,11 @@ static int bmdma_rw_buf(IDEDMA *dma, int is_write)
 return 1;
 }
 
-static int bmdma_set_unit(IDEDMA *dma, int unit)
-{
-BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma);
-bm->unit = unit;
-
-return 0;
-}
-
 static void bmdma_set_inactive(IDEDMA *dma, bool more)
 {
 BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma);
 
 bm->dma_cb = NULL;
-bm->unit = -1;
 if (more) {
 bm->status |= BM_STATUS_DMAING;
 } else {
@@ -335,6 +325,7 @@ static void ide_bmdma_pre_save(void *opaque)
 BMDMAState *bm = opaque;
 uint8_t abused_bits = BM_MIGRATION_COMPAT_STATUS_BITS;
 
+bm->migration_retry_unit = bm->bus->retry_unit;
 bm->migration_compat_status =
 (bm->status & ~abused_bits) | (bm->bus->error_status & abused_bits);
 }
@@ -351,6 +342,9 @@ static int ide_bmdma_post_load(void *opaque, int version_id)
 bm->status = bm->migration_compat_status & ~abused_bits;
 bm->bus->error_status |= bm->migration_compat_statu

[Qemu-devel] [PATCH v2 11/17] ide: commonize io_buffer_index initialization

2014-12-16 Thread John Snow
From: Paolo Bonzini 

Resetting the io_buffer_index to 0 is commonized,
with the exception of the case within ide_atapi_cmd_reply,
where we need to reset this index to 0 prior to the
ide_atapi_cmd_reply_end call.

Note that not all calls to ide_atapi_cmd_reply_end
expect the index to be 0, so setting it there is
not appropriate.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
---
 hw/ide/atapi.c | 3 +--
 hw/ide/core.c  | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index c63b7e5..ef620e8 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -252,7 +252,6 @@ static void ide_atapi_cmd_reply(IDEState *s, int size, int 
max_size)
 s->packet_transfer_size = size;
 s->io_buffer_size = size;/* dma: send the reply data as one chunk */
 s->elementary_transfer_size = 0;
-s->io_buffer_index = 0;
 
 if (s->atapi_dma) {
 block_acct_start(blk_get_stats(s->blk), &s->acct, size,
@@ -261,6 +260,7 @@ static void ide_atapi_cmd_reply(IDEState *s, int size, int 
max_size)
 ide_start_dma(s, ide_atapi_cmd_read_dma_cb);
 } else {
 s->status = READY_STAT | SEEK_STAT;
+s->io_buffer_index = 0;
 ide_atapi_cmd_reply_end(s);
 }
 }
@@ -368,7 +368,6 @@ static void ide_atapi_cmd_read_dma(IDEState *s, int lba, 
int nb_sectors,
 {
 s->lba = lba;
 s->packet_transfer_size = nb_sectors * sector_size;
-s->io_buffer_index = 0;
 s->io_buffer_size = 0;
 s->cd_sector_size = sector_size;
 
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 0459ea1..b89cde0 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -780,7 +780,6 @@ eot:
 static void ide_sector_start_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
 {
 s->status = READY_STAT | SEEK_STAT | DRQ_STAT | BUSY_STAT;
-s->io_buffer_index = 0;
 s->io_buffer_size = 0;
 s->dma_cmd = dma_cmd;
 
@@ -802,6 +801,7 @@ static void ide_sector_start_dma(IDEState *s, enum 
ide_dma_cmd dma_cmd)
 
 void ide_start_dma(IDEState *s, BlockCompletionFunc *cb)
 {
+s->io_buffer_index = 0;
 s->bus->retry_unit = s->unit;
 s->bus->retry_sector_num = ide_get_sector(s);
 s->bus->retry_nsector = s->nsector;
@@ -2341,7 +2341,6 @@ static void ide_restart_dma(IDEState *s, enum ide_dma_cmd 
dma_cmd)
 ide_set_sector(s, s->bus->retry_sector_num);
 s->nsector = s->bus->retry_nsector;
 s->bus->dma->ops->restart_dma(s->bus->dma);
-s->io_buffer_index = 0;
 s->io_buffer_size = 0;
 s->dma_cmd = dma_cmd;
 ide_start_dma(s, ide_dma_cb);
-- 
1.9.3




[Qemu-devel] [PATCH v2 15/17] ahci: add support for restarting non-queued commands

2014-12-16 Thread John Snow
From: Paolo Bonzini 

This is easy, since start_dma already restarts processing from the
beginning of the PRDT.

Migration is also easy to cover; the comment about busy_slot is
wrong, busy_slot will only be set if there is an error.  In this
case we have nothing to do really.  The core IDE code will restart
the operation and command list processing will proceed after the
erroring command has been completed.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
---
 hw/ide/ahci.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 3f4fc77..c153228 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1160,6 +1160,11 @@ static void ahci_start_dma(IDEDMA *dma, IDEState *s,
 dma_cb(s, 0);
 }
 
+static void ahci_restart_dma(IDEDMA *dma)
+{
+/* Nothing to do, ahci_start_dma already resets s->io_buffer_offset.  */
+}
+
 /**
  * Called in DMA R/W chains to read the PRDT, utilizing ahci_populate_sglist.
  * Not currently invoked by PIO R/W chains,
@@ -1248,6 +1253,7 @@ static void ahci_irq_set(void *opaque, int n, int level)
 
 static const IDEDMAOps ahci_dma_ops = {
 .start_dma = ahci_start_dma,
+.restart_dma = ahci_restart_dma,
 .start_transfer = ahci_start_transfer,
 .prepare_buf = ahci_dma_prepare_buf,
 .commit_buf = ahci_commit_buf,
@@ -1282,6 +1288,7 @@ void ahci_init(AHCIState *s, DeviceState *qdev, 
AddressSpace *as, int ports)
 ad->port_no = i;
 ad->port.dma = &ad->dma;
 ad->port.dma->ops = &ahci_dma_ops;
+ide_register_restart_cb(&ad->port);
 }
 }
 
@@ -1360,16 +1367,13 @@ static int ahci_state_post_load(void *opaque, int 
version_id)
 map_page(s->as, &ad->res_fis,
  ((uint64_t)pr->fis_addr_hi << 32) | pr->fis_addr, 256);
 /*
- * All pending i/o should be flushed out on a migrate. However,
- * we might not have cleared the busy_slot since this is done
- * in a bh. Also, issue i/o against any slots that are pending.
+ * If an error was there, ad->busy_slot will not be -1.  Restarting
+ * the operation will resume execution of the command list, do
+ * nothing here.
  */
-if ((ad->busy_slot != -1) &&
-!(ad->port.ifs[0].status & (BUSY_STAT|DRQ_STAT))) {
-pr->cmd_issue &= ~(1 << ad->busy_slot);
-ad->busy_slot = -1;
+if (ad->busy_slot == -1) {
+check_cmd(s, i);
 }
-check_cmd(s, i);
 }
 
 return 0;
-- 
1.9.3




[Qemu-devel] [PATCH v2 06/17] ide: move restart callback to common code

2014-12-16 Thread John Snow
From: Paolo Bonzini 

With BMDMA specific excised from the restart functions,
create a HBA-agnostic restart callback to be shared
between the different HBAs.

Change the callback registered with the vmstate_change
handler to always point to ide_restart_cb instead of
relying on the IDEDMAOps.restart_cb() member.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
---
 hw/ide/core.c | 71 ++-
 hw/ide/internal.h |  2 ++
 hw/ide/pci.c  | 68 
 hw/ide/pci.h  |  1 -
 4 files changed, 72 insertions(+), 70 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index a836626..15b6a3f 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2314,6 +2314,10 @@ static int ide_nop_int(IDEDMA *dma, int x)
 return 0;
 }
 
+static void ide_nop(IDEDMA *dma)
+{
+}
+
 static int32_t ide_nop_int32(IDEDMA *dma, int x)
 {
 return 0;
@@ -2325,14 +2329,79 @@ static void ide_nop_restart(void *opaque, int x, 
RunState y)
 
 static const IDEDMAOps ide_dma_nop_ops = {
 .prepare_buf= ide_nop_int32,
+.restart_dma= ide_nop,
 .rw_buf = ide_nop_int,
 .set_unit   = ide_nop_int,
 .restart_cb = ide_nop_restart,
 };
 
+static void ide_restart_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
+{
+s->bus->dma->ops->restart_dma(s->bus->dma);
+s->io_buffer_index = 0;
+s->io_buffer_size = 0;
+s->dma_cmd = dma_cmd;
+ide_start_dma(s, ide_dma_cb);
+}
+
+static void ide_restart_bh(void *opaque)
+{
+IDEBus *bus = opaque;
+IDEState *s;
+bool is_read;
+int error_status;
+
+qemu_bh_delete(bus->bh);
+bus->bh = NULL;
+
+error_status = bus->error_status;
+if (bus->error_status == 0) {
+return;
+}
+
+s = idebus_active_if(bus);
+is_read = (bus->error_status & IDE_RETRY_READ) != 0;
+
+/* The error status must be cleared before resubmitting the request: The
+ * request may fail again, and this case can only be distinguished if the
+ * called function can set a new error status. */
+bus->error_status = 0;
+
+if (error_status & IDE_RETRY_DMA) {
+if (error_status & IDE_RETRY_TRIM) {
+ide_restart_dma(s, IDE_DMA_TRIM);
+} else {
+ide_restart_dma(s, is_read ? IDE_DMA_READ : IDE_DMA_WRITE);
+}
+} else if (error_status & IDE_RETRY_PIO) {
+if (is_read) {
+ide_sector_read(s);
+} else {
+ide_sector_write(s);
+}
+} else if (error_status & IDE_RETRY_FLUSH) {
+ide_flush_cache(s);
+}
+}
+
+static void ide_restart_cb(void *opaque, int running, RunState state)
+{
+IDEBus *bus = opaque;
+
+if (!running)
+return;
+
+if (!bus->bh) {
+bus->bh = qemu_bh_new(ide_restart_bh, bus);
+qemu_bh_schedule(bus->bh);
+}
+}
+
 void ide_register_restart_cb(IDEBus *bus)
 {
-qemu_add_vm_change_state_handler(bus->dma->ops->restart_cb, bus);
+if (bus->dma->ops->restart_dma) {
+qemu_add_vm_change_state_handler(ide_restart_cb, bus);
+}
 }
 
 static IDEDMA ide_dma_nop = {
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 73c5e63..7ea64d1 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -454,6 +454,8 @@ struct IDEBus {
 IDEDevice *master;
 IDEDevice *slave;
 IDEState ifs[2];
+QEMUBH *bh;
+
 int bus_id;
 int max_units;
 IDEDMA *dma;
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index e5caf11..cd1bbb0 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -194,73 +194,6 @@ static void bmdma_restart_dma(IDEDMA *dma)
 bm->cur_addr = bm->addr;
 }
 
-static void ide_restart_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
-{
-if (s->bus->dma->ops->restart_dma) {
-s->bus->dma->ops->restart_dma(s->bus->dma);
-}
-s->io_buffer_index = 0;
-s->io_buffer_size = 0;
-s->dma_cmd = dma_cmd;
-ide_start_dma(s, ide_dma_cb);
-}
-
-/* TODO This should be common IDE code */
-static void bmdma_restart_bh(void *opaque)
-{
-IDEBus *bus = opaque;
-BMDMAState *bm = DO_UPCAST(BMDMAState, dma, bus->dma);
-IDEState *s;
-bool is_read;
-int error_status;
-
-qemu_bh_delete(bm->bh);
-bm->bh = NULL;
-
-error_status = bus->error_status;
-if (bus->error_status == 0) {
-return;
-}
-
-s = idebus_active_if(bus);
-is_read = (bus->error_status & IDE_RETRY_READ) != 0;
-
-/* The error status must be cleared before resubmitting the request: The
- * request may fail again, and this case can only be distinguished if the
- * called function can set a new error status. */
-bus->error_status = 0;
-
-if (error_status & IDE_RETRY_DMA) {
-if (error_status & IDE_RETRY_TRIM) {
-ide_restart_dma(s, IDE_DMA_TRIM);
-} else {
-ide_restart_dma(s, is_read ? IDE_DMA_READ : IDE_DMA_WRITE);
-}
-} else if (error_status & IDE_RETRY_PIO) {
-if (is_read) {
- 

[Qemu-devel] [PATCH v2 12/17] ide: make more functions static

2014-12-16 Thread John Snow
From: Paolo Bonzini 

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
---
 hw/ide/core.c | 12 
 hw/ide/internal.h |  4 
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index b89cde0..66ab93d 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -561,6 +561,8 @@ static bool ide_sect_range_ok(IDEState *s,
 return true;
 }
 
+static void ide_sector_read(IDEState *s);
+
 static void ide_sector_read_cb(void *opaque, int ret)
 {
 IDEState *s = opaque;
@@ -595,7 +597,7 @@ static void ide_sector_read_cb(void *opaque, int ret)
 s->io_buffer_offset += 512 * n;
 }
 
-void ide_sector_read(IDEState *s)
+static void ide_sector_read(IDEState *s)
 {
 int64_t sector_num;
 int n;
@@ -682,7 +684,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int 
op)
 return action != BLOCK_ERROR_ACTION_IGNORE;
 }
 
-void ide_dma_cb(void *opaque, int ret)
+static void ide_dma_cb(void *opaque, int ret)
 {
 IDEState *s = opaque;
 int n;
@@ -810,6 +812,8 @@ void ide_start_dma(IDEState *s, BlockCompletionFunc *cb)
 }
 }
 
+static void ide_sector_write(IDEState *s);
+
 static void ide_sector_write_timer_cb(void *opaque)
 {
 IDEState *s = opaque;
@@ -869,7 +873,7 @@ static void ide_sector_write_cb(void *opaque, int ret)
 }
 }
 
-void ide_sector_write(IDEState *s)
+static void ide_sector_write(IDEState *s)
 {
 int64_t sector_num;
 int n;
@@ -923,7 +927,7 @@ static void ide_flush_cb(void *opaque, int ret)
 ide_set_irq(s->bus);
 }
 
-void ide_flush_cache(IDEState *s)
+static void ide_flush_cache(IDEState *s)
 {
 if (s->blk == NULL) {
 ide_flush_cb(s, 0);
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index da2230f..0beba43 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -554,10 +554,6 @@ void ide_init_ioport(IDEBus *bus, ISADevice *isa, int 
iobase, int iobase2);
 void ide_register_restart_cb(IDEBus *bus);
 
 void ide_exec_cmd(IDEBus *bus, uint32_t val);
-void ide_dma_cb(void *opaque, int ret);
-void ide_sector_write(IDEState *s);
-void ide_sector_read(IDEState *s);
-void ide_flush_cache(IDEState *s);
 
 void ide_transfer_start(IDEState *s, uint8_t *buf, int size,
 EndTransferFunc *end_transfer_func);
-- 
1.9.3




[Qemu-devel] [PATCH v2 03/17] ide: introduce ide_register_restart_cb

2014-12-16 Thread John Snow
From: Paolo Bonzini 

A helper is added that registers the IDEDMAOp .restart_cb()
via qemu_add_vm_change_state_handler instead of requiring
each HBA to register the callback themselves.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
---
 hw/ide/cmd646.c   | 3 +--
 hw/ide/core.c | 5 +
 hw/ide/internal.h | 1 +
 hw/ide/piix.c | 3 +--
 hw/ide/via.c  | 3 +--
 5 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index c8b0322..0374653 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -368,8 +368,7 @@ static int pci_cmd646_ide_initfn(PCIDevice *dev)
 
 bmdma_init(&d->bus[i], &d->bmdma[i], d);
 d->bmdma[i].bus = &d->bus[i];
-qemu_add_vm_change_state_handler(d->bus[i].dma->ops->restart_cb,
- &d->bmdma[i].dma);
+ide_register_restart_cb(&d->bus[i]);
 }
 
 vmstate_register(DEVICE(dev), 0, &vmstate_ide_pci, d);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index d4af5e2..179a001 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2330,6 +2330,11 @@ static const IDEDMAOps ide_dma_nop_ops = {
 .restart_cb = ide_nop_restart,
 };
 
+void ide_register_restart_cb(IDEBus *bus)
+{
+qemu_add_vm_change_state_handler(bus->dma->ops->restart_cb, bus->dma);
+}
+
 static IDEDMA ide_dma_nop = {
 .ops = &ide_dma_nop_ops,
 .aiocb = NULL,
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 59e24e2..73c5e63 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -548,6 +548,7 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, 
IDEDriveKind kind,
int chs_trans);
 void ide_init2(IDEBus *bus, qemu_irq irq);
 void ide_init_ioport(IDEBus *bus, ISADevice *isa, int iobase, int iobase2);
+void ide_register_restart_cb(IDEBus *bus);
 
 void ide_exec_cmd(IDEBus *bus, uint32_t val);
 void ide_dma_cb(void *opaque, int ret);
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index b0172fb..247dd4f 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -143,8 +143,7 @@ static void pci_piix_init_ports(PCIIDEState *d) {
 
 bmdma_init(&d->bus[i], &d->bmdma[i], d);
 d->bmdma[i].bus = &d->bus[i];
-qemu_add_vm_change_state_handler(d->bus[i].dma->ops->restart_cb,
- &d->bmdma[i].dma);
+ide_register_restart_cb(&d->bus[i]);
 }
 }
 
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 4d8089d..fb3d8a0 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -166,8 +166,7 @@ static void vt82c686b_init_ports(PCIIDEState *d) {
 
 bmdma_init(&d->bus[i], &d->bmdma[i], d);
 d->bmdma[i].bus = &d->bus[i];
-qemu_add_vm_change_state_handler(d->bus[i].dma->ops->restart_cb,
- &d->bmdma[i].dma);
+ide_register_restart_cb(&d->bus[i]);
 }
 }
 
-- 
1.9.3




[Qemu-devel] [PATCH v2 07/17] ide: remove restart_cb callback

2014-12-16 Thread John Snow
From: Paolo Bonzini 

With restarts now handled by ide_restart_cb and
the IDEDMAOps.restart_dma() member, remove the old
restart_cb callback.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
---
 hw/ide/ahci.c | 5 -
 hw/ide/core.c | 5 -
 hw/ide/internal.h | 1 -
 hw/ide/macio.c| 5 -
 4 files changed, 16 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 5651372..3892025 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1252,10 +1252,6 @@ static void ahci_irq_set(void *opaque, int n, int level)
 {
 }
 
-static void ahci_dma_restart_cb(void *opaque, int running, RunState state)
-{
-}
-
 static const IDEDMAOps ahci_dma_ops = {
 .start_dma = ahci_start_dma,
 .start_transfer = ahci_start_transfer,
@@ -1264,7 +1260,6 @@ static const IDEDMAOps ahci_dma_ops = {
 .rw_buf = ahci_dma_rw_buf,
 .set_unit = ahci_dma_set_unit,
 .cmd_done = ahci_cmd_done,
-.restart_cb = ahci_dma_restart_cb,
 };
 
 void ahci_init(AHCIState *s, DeviceState *qdev, AddressSpace *as, int ports)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 15b6a3f..f3d98d7 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2323,16 +2323,11 @@ static int32_t ide_nop_int32(IDEDMA *dma, int x)
 return 0;
 }
 
-static void ide_nop_restart(void *opaque, int x, RunState y)
-{
-}
-
 static const IDEDMAOps ide_dma_nop_ops = {
 .prepare_buf= ide_nop_int32,
 .restart_dma= ide_nop,
 .rw_buf = ide_nop_int,
 .set_unit   = ide_nop_int,
-.restart_cb = ide_nop_restart,
 };
 
 static void ide_restart_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 7ea64d1..53ce932 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -438,7 +438,6 @@ struct IDEDMAOps {
 DMAVoidFunc *restart_dma;
 DMAStopFunc *set_inactive;
 DMAVoidFunc *cmd_done;
-DMARestartFunc *restart_cb;
 DMAVoidFunc *reset;
 };
 
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index f6074f2..e7a5c99 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -558,10 +558,6 @@ static int32_t ide_nop_int32(IDEDMA *dma, int x)
 return 0;
 }
 
-static void ide_nop_restart(void *opaque, int x, RunState y)
-{
-}
-
 static void ide_dbdma_start(IDEDMA *dma, IDEState *s,
 BlockCompletionFunc *cb)
 {
@@ -577,7 +573,6 @@ static const IDEDMAOps dbdma_ops = {
 .prepare_buf= ide_nop_int32,
 .rw_buf = ide_nop_int,
 .set_unit   = ide_nop_int,
-.restart_cb = ide_nop_restart,
 };
 
 static void macio_ide_realizefn(DeviceState *dev, Error **errp)
-- 
1.9.3




[Qemu-devel] [PATCH v2 05/17] ide: pass IDEBus to the restart_cb

2014-12-16 Thread John Snow
From: Paolo Bonzini 

Pass the containing IDEBus to the restart_cb instead
of the more specific BMDMAState child.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
---
 hw/ide/core.c |  2 +-
 hw/ide/pci.c  | 10 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 179a001..a836626 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2332,7 +2332,7 @@ static const IDEDMAOps ide_dma_nop_ops = {
 
 void ide_register_restart_cb(IDEBus *bus)
 {
-qemu_add_vm_change_state_handler(bus->dma->ops->restart_cb, bus->dma);
+qemu_add_vm_change_state_handler(bus->dma->ops->restart_cb, bus);
 }
 
 static IDEDMA ide_dma_nop = {
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index eb480f6..e5caf11 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -208,8 +208,8 @@ static void ide_restart_dma(IDEState *s, enum ide_dma_cmd 
dma_cmd)
 /* TODO This should be common IDE code */
 static void bmdma_restart_bh(void *opaque)
 {
-BMDMAState *bm = opaque;
-IDEBus *bus = bm->bus;
+IDEBus *bus = opaque;
+BMDMAState *bm = DO_UPCAST(BMDMAState, dma, bus->dma);
 IDEState *s;
 bool is_read;
 int error_status;
@@ -249,14 +249,14 @@ static void bmdma_restart_bh(void *opaque)
 
 static void bmdma_restart_cb(void *opaque, int running, RunState state)
 {
-IDEDMA *dma = opaque;
-BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma);
+IDEBus *bus = opaque;
+BMDMAState *bm = DO_UPCAST(BMDMAState, dma, bus->dma);
 
 if (!running)
 return;
 
 if (!bm->bh) {
-bm->bh = qemu_bh_new(bmdma_restart_bh, &bm->dma);
+bm->bh = qemu_bh_new(bmdma_restart_bh, bus);
 qemu_bh_schedule(bm->bh);
 }
 }
-- 
1.9.3




[Qemu-devel] [PATCH v2 09/17] ide: place initial state of the current request to IDEBus

2014-12-16 Thread John Snow
From: Paolo Bonzini 

This moves more common restarting logic to the core IDE code.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
---
 hw/ide/core.c |  6 ++
 hw/ide/internal.h |  2 ++
 hw/ide/pci.c  | 15 ++-
 hw/ide/pci.h  |  5 ++---
 4 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index fc5d227..d4659dc 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -647,6 +647,8 @@ void ide_set_inactive(IDEState *s, bool more)
 {
 s->bus->dma->aiocb = NULL;
 s->bus->retry_unit = -1;
+s->bus->retry_sector_num = 0;
+s->bus->retry_nsector = 0;
 if (s->bus->dma->ops->set_inactive) {
 s->bus->dma->ops->set_inactive(s->bus->dma, more);
 }
@@ -801,6 +803,8 @@ static void ide_sector_start_dma(IDEState *s, enum 
ide_dma_cmd dma_cmd)
 void ide_start_dma(IDEState *s, BlockCompletionFunc *cb)
 {
 s->bus->retry_unit = s->unit;
+s->bus->retry_sector_num = ide_get_sector(s);
+s->bus->retry_nsector = s->nsector;
 if (s->bus->dma->ops->start_dma) {
 s->bus->dma->ops->start_dma(s->bus->dma, s, cb);
 }
@@ -2334,6 +2338,8 @@ static const IDEDMAOps ide_dma_nop_ops = {
 static void ide_restart_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
 {
 s->unit = s->bus->retry_unit;
+ide_set_sector(s, s->bus->retry_sector_num);
+s->nsector = s->bus->retry_nsector;
 s->bus->dma->ops->restart_dma(s->bus->dma);
 s->io_buffer_index = 0;
 s->io_buffer_size = 0;
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 07d82c4..da2230f 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -463,6 +463,8 @@ struct IDEBus {
 
 int error_status;
 uint8_t retry_unit;
+int64_t retry_sector_num;
+uint32_t retry_nsector;
 };
 
 #define TYPE_IDE_DEVICE "ide-device"
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 2b0e886..fab2abc 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -46,8 +46,6 @@ static void bmdma_start_dma(IDEDMA *dma, IDEState *s,
 bm->cur_prd_last = 0;
 bm->cur_prd_addr = 0;
 bm->cur_prd_len = 0;
-bm->sector_num = ide_get_sector(s);
-bm->nsector = s->nsector;
 
 if (bm->status & BM_STATUS_DMAING) {
 bm->dma_cb(bmdma_active_if(bm), 0);
@@ -177,10 +175,7 @@ static void bmdma_set_inactive(IDEDMA *dma, bool more)
 static void bmdma_restart_dma(IDEDMA *dma)
 {
 BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma);
-IDEState *s = bmdma_active_if(bm);
 
-ide_set_sector(s, bm->sector_num);
-s->nsector = bm->nsector;
 bm->cur_addr = bm->addr;
 }
 
@@ -207,8 +202,6 @@ static void bmdma_reset(IDEDMA *dma)
 bm->cur_prd_last = 0;
 bm->cur_prd_addr = 0;
 bm->cur_prd_len = 0;
-bm->sector_num = 0;
-bm->nsector = 0;
 }
 
 static void bmdma_irq(void *opaque, int n, int level)
@@ -326,6 +319,8 @@ static void ide_bmdma_pre_save(void *opaque)
 uint8_t abused_bits = BM_MIGRATION_COMPAT_STATUS_BITS;
 
 bm->migration_retry_unit = bm->bus->retry_unit;
+bm->migration_retry_sector_num = bm->bus->retry_sector_num;
+bm->migration_retry_nsector = bm->bus->retry_nsector;
 bm->migration_compat_status =
 (bm->status & ~abused_bits) | (bm->bus->error_status & abused_bits);
 }
@@ -343,6 +338,8 @@ static int ide_bmdma_post_load(void *opaque, int version_id)
 bm->bus->error_status |= bm->migration_compat_status & abused_bits;
 }
 if (bm->bus->error_status) {
+bm->bus->retry_sector_num = bm->migration_retry_sector_num;
+bm->bus->retry_nsector = bm->migration_retry_nsector;
 bm->bus->retry_unit = bm->migration_retry_unit;
 }
 
@@ -381,8 +378,8 @@ static const VMStateDescription vmstate_bmdma = {
 VMSTATE_UINT8(cmd, BMDMAState),
 VMSTATE_UINT8(migration_compat_status, BMDMAState),
 VMSTATE_UINT32(addr, BMDMAState),
-VMSTATE_INT64(sector_num, BMDMAState),
-VMSTATE_UINT32(nsector, BMDMAState),
+VMSTATE_INT64(migration_retry_sector_num, BMDMAState),
+VMSTATE_UINT32(migration_retry_nsector, BMDMAState),
 VMSTATE_UINT8(migration_retry_unit, BMDMAState),
 VMSTATE_END_OF_LIST()
 },
diff --git a/hw/ide/pci.h b/hw/ide/pci.h
index 222a163..0f2d4b9 100644
--- a/hw/ide/pci.h
+++ b/hw/ide/pci.h
@@ -22,10 +22,7 @@ typedef struct BMDMAState {
 uint32_t cur_prd_last;
 uint32_t cur_prd_addr;
 uint32_t cur_prd_len;
-uint8_t unit;
 BlockCompletionFunc *dma_cb;
-int64_t sector_num;
-uint32_t nsector;
 MemoryRegion addr_ioport;
 MemoryRegion extra_io;
 qemu_irq irq;
@@ -34,6 +31,8 @@ typedef struct BMDMAState {
  * Bit 3-6: bus->error_status */
 uint8_t migration_compat_status;
 uint8_t migration_retry_unit;
+int64_t migration_retry_sector_num;
+uint32_t migration_retry_nsector;
 
 struct PCIIDEState *pci_dev;
 } BMDMAState;
-- 
1.9.3




[Qemu-devel] [PATCH v2 04/17] ide: do not use BMDMA in restart callback

2014-12-16 Thread John Snow
From: Paolo Bonzini 

Whenever an error stops the VM, ide_handle_rw_error does
"s->bus->dma->unit = s->unit".  So we can just use
idebus_active_if.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
---
 hw/ide/pci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 71c7f2e..eb480f6 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -217,17 +217,17 @@ static void bmdma_restart_bh(void *opaque)
 qemu_bh_delete(bm->bh);
 bm->bh = NULL;
 
-if (bm->unit == (uint8_t) -1) {
+error_status = bus->error_status;
+if (bus->error_status == 0) {
 return;
 }
 
-s = bmdma_active_if(bm);
+s = idebus_active_if(bus);
 is_read = (bus->error_status & IDE_RETRY_READ) != 0;
 
 /* The error status must be cleared before resubmitting the request: The
  * request may fail again, and this case can only be distinguished if the
  * called function can set a new error status. */
-error_status = bus->error_status;
 bus->error_status = 0;
 
 if (error_status & IDE_RETRY_DMA) {
-- 
1.9.3




[Qemu-devel] [PATCH v2 02/17] ide: prepare to move restart to common code

2014-12-16 Thread John Snow
From: Paolo Bonzini 

This patch adds the restart_dma callback and adjusts
the ide_restart_dma function to utilize this callback
to call the BMDMA-specific restart code instead of statically
executing BMDMA-specific code.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
---
 hw/ide/internal.h |  1 +
 hw/ide/pci.c  | 12 +++-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 8a3eca4..59e24e2 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -435,6 +435,7 @@ struct IDEDMAOps {
 DMAu32Func *commit_buf;
 DMAIntFunc *rw_buf;
 DMAIntFunc *set_unit;
+DMAVoidFunc *restart_dma;
 DMAStopFunc *set_inactive;
 DMAVoidFunc *cmd_done;
 DMARestartFunc *restart_cb;
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 01f3cd2..71c7f2e 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -184,8 +184,9 @@ static void bmdma_set_inactive(IDEDMA *dma, bool more)
 }
 }
 
-static void bmdma_restart_dma(BMDMAState *bm)
+static void bmdma_restart_dma(IDEDMA *dma)
 {
+BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma);
 IDEState *s = bmdma_active_if(bm);
 
 ide_set_sector(s, bm->sector_num);
@@ -195,13 +196,13 @@ static void bmdma_restart_dma(BMDMAState *bm)
 
 static void ide_restart_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
 {
-BMDMAState *bm = DO_UPCAST(BMDMAState, dma, s->bus->dma);
-
-bmdma_restart_dma(bm);
+if (s->bus->dma->ops->restart_dma) {
+s->bus->dma->ops->restart_dma(s->bus->dma);
+}
 s->io_buffer_index = 0;
 s->io_buffer_size = 0;
 s->dma_cmd = dma_cmd;
-bmdma_start_dma(&bm->dma, s, ide_dma_cb);
+ide_start_dma(s, ide_dma_cb);
 }
 
 /* TODO This should be common IDE code */
@@ -521,6 +522,7 @@ static const struct IDEDMAOps bmdma_ops = {
 .prepare_buf = bmdma_prepare_buf,
 .rw_buf = bmdma_rw_buf,
 .set_unit = bmdma_set_unit,
+.restart_dma = bmdma_restart_dma,
 .set_inactive = bmdma_set_inactive,
 .restart_cb = bmdma_restart_cb,
 .reset = bmdma_reset,
-- 
1.9.3




[Qemu-devel] [PATCH v2 01/17] ide: start extracting ide_restart_dma out of bmdma_restart_dma

2014-12-16 Thread John Snow
From: Paolo Bonzini 

This patch begins refactoring the restart dma functions
out of bmdma to be shared with AHCI and other future
IDE HBA implementations.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
---
 hw/ide/pci.c | 30 +++---
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index bee5ad3..01f3cd2 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -184,18 +184,24 @@ static void bmdma_set_inactive(IDEDMA *dma, bool more)
 }
 }
 
-static void bmdma_restart_dma(BMDMAState *bm, enum ide_dma_cmd dma_cmd)
+static void bmdma_restart_dma(BMDMAState *bm)
 {
 IDEState *s = bmdma_active_if(bm);
 
 ide_set_sector(s, bm->sector_num);
-s->io_buffer_index = 0;
-s->io_buffer_size = 0;
 s->nsector = bm->nsector;
-s->dma_cmd = dma_cmd;
 bm->cur_addr = bm->addr;
-bm->dma_cb = ide_dma_cb;
-bmdma_start_dma(&bm->dma, s, bm->dma_cb);
+}
+
+static void ide_restart_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
+{
+BMDMAState *bm = DO_UPCAST(BMDMAState, dma, s->bus->dma);
+
+bmdma_restart_dma(bm);
+s->io_buffer_index = 0;
+s->io_buffer_size = 0;
+s->dma_cmd = dma_cmd;
+bmdma_start_dma(&bm->dma, s, ide_dma_cb);
 }
 
 /* TODO This should be common IDE code */
@@ -203,6 +209,7 @@ static void bmdma_restart_bh(void *opaque)
 {
 BMDMAState *bm = opaque;
 IDEBus *bus = bm->bus;
+IDEState *s;
 bool is_read;
 int error_status;
 
@@ -213,6 +220,7 @@ static void bmdma_restart_bh(void *opaque)
 return;
 }
 
+s = bmdma_active_if(bm);
 is_read = (bus->error_status & IDE_RETRY_READ) != 0;
 
 /* The error status must be cleared before resubmitting the request: The
@@ -223,18 +231,18 @@ static void bmdma_restart_bh(void *opaque)
 
 if (error_status & IDE_RETRY_DMA) {
 if (error_status & IDE_RETRY_TRIM) {
-bmdma_restart_dma(bm, IDE_DMA_TRIM);
+ide_restart_dma(s, IDE_DMA_TRIM);
 } else {
-bmdma_restart_dma(bm, is_read ? IDE_DMA_READ : IDE_DMA_WRITE);
+ide_restart_dma(s, is_read ? IDE_DMA_READ : IDE_DMA_WRITE);
 }
 } else if (error_status & IDE_RETRY_PIO) {
 if (is_read) {
-ide_sector_read(bmdma_active_if(bm));
+ide_sector_read(s);
 } else {
-ide_sector_write(bmdma_active_if(bm));
+ide_sector_write(s);
 }
 } else if (error_status & IDE_RETRY_FLUSH) {
-ide_flush_cache(bmdma_active_if(bm));
+ide_flush_cache(s);
 }
 }
 
-- 
1.9.3




[Qemu-devel] [PATCH v2 00/17] ide: rerror and werror support for IDE and AHCI

2014-12-16 Thread John Snow
This series was written mostly by Paolo Bonzini to do two things:

1. Unify the restart callbacks for ISA, AHCI and BMDMA
2. Ensure we can restart a command after migration

Many of the early patches only make much sense considering the
end-goal of eliminating BMDMA specific restart code to be shared
with ISA and AHCI codepaths.

Migration for halted commands is fixed for ISA, PCI and AHCI.
As a consequence, operations halted via rerror=stop or werror=stop
should be able to be successfully migrated and resumed when using
ISA, PCI, or AHCI.

This series includes tests for ISA and PCI/BMDMA, but does not
yet include tests for AHCI, which require some more qtest work
to be upstreamed first. Regardless, the AHCI tests have been
written and can be observed at:
https://github.com/jnsnow/qemu/commits/ahci-devel-latest

See "ahci: add migrate dma test" and "ahci-test: add flush migrate test"
for the WIP versions of the AHCI test that I used to exercise this
patchset.

John Snow (3):
  ahci: Migrate IDEStatus
  ahci: Recompute cur_cmd on migrate post load
  qtest/ide: Test flush / retry for ISA and PCI

Paolo Bonzini (14):
  ide: start extracting ide_restart_dma out of bmdma_restart_dma
  ide: prepare to move restart to common code
  ide: introduce ide_register_restart_cb
  ide: do not use BMDMA in restart callback
  ide: pass IDEBus to the restart_cb
  ide: move restart callback to common code
  ide: remove restart_cb callback
  ide: replace set_unit callback with more IDEBus state
  ide: place initial state of the current request to IDEBus
  ide: migrate initial request state via IDEBus
  ide: commonize io_buffer_index initialization
  ide: make more functions static
  ide: support PIO restart for the ISA controller
  ahci: add support for restarting non-queued commands

 hw/ide/ahci.c |  37 +-
 hw/ide/atapi.c|   3 +-
 hw/ide/cmd646.c   |   3 +-
 hw/ide/core.c | 109 +++---
 hw/ide/internal.h |  16 +---
 hw/ide/isa.c  |   3 +-
 hw/ide/macio.c|   6 ---
 hw/ide/pci.c  |  98 
 hw/ide/pci.h  |  12 +++---
 hw/ide/piix.c |   3 +-
 hw/ide/via.c  |   3 +-
 tests/ide-test.c  |  20 +++---
 12 files changed, 165 insertions(+), 148 deletions(-)

-- 
1.9.3




[Qemu-devel] alt-gr on Windows

2014-12-16 Thread Thebault, Remi

Hi list!

This is not the first post on this topic, but I haven't seen any 
solution about it.
I tested so far linux guest on windows host and the AltGr key is dead in 
the guest. (using git master branch)


On french keyboard, the keys to yield the bar "|" are alt-gr + 6.
when executing this combination on keyboard, Windows generates this:
- L-CTRL
- R-ALT
- 6

in Qemu (only digged gtk UI so far), pressing alt-gr + 6 generates the 
following trace

- L-CTRL
- L-ALT   <-- note left here
- 6

This comes from the Win32 call MapVirtualKey in gtk.c that maps to 
scancodes without left/right distinction.
Even when sending the right alt to the guest, the alt-gr key remains 
dead because of ctrl being virtually pressed. I found out however that 
if R-ALT + 6 is sent without the ctrl key, the guest finally recognize 
it and prints the bar, @, # and other [}{].


To make things easier, Windows delivers the ctrl code before the alt 
code, so catching it cleanly before delivery to the guest is probably tough.
I could however come to an easy and quick fix with sending the "ctrl up" 
signal to the guest before the "r-alt down" is sent.


My current code do not handle all corner cases (eg: turbo mode) and only 
fixes the gtk ui, but would such fix be accepted in the repo?

Would this break somehow the windows guest on windows host?

Thanks,
Rémi



[Qemu-devel] [Bug 1368815] Re: qemu-img convert intermittently corrupts output images

2014-12-16 Thread Tony Breeds
Patchg 0500-block-raw-posix-Try-both-FIEMAP-and-SEEK_HOLE.patch appears
to be part of a bigger re-write of the related code.   and is ON TOP of
the patches already applied in this bug.


No doubt the rewirtten code is "better" but backporting it contains more risk 
than the 2 simple fixes I already nominated.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1368815

Title:
  qemu-img convert intermittently corrupts output images

Status in Cinder:
  In Progress
Status in OpenStack Compute (Nova):
  In Progress
Status in QEMU:
  In Progress
Status in qemu package in Ubuntu:
  Fix Released
Status in qemu source package in Trusty:
  Fix Released
Status in qemu source package in Utopic:
  Fix Released
Status in qemu source package in Vivid:
  Fix Released

Bug description:
  ==
  Impact: occasional image corruption (any format on local filesystem)
  Test case: see the qemu-img command below
  Regression potential: this cherrypicks a patch from upstream to a 
not-insignificantly older qemu source tree.  While the cherrypick seems sane, 
it's possible that there are subtle interactions with the other delta.  I'd 
really like for a full qa-regression-test qemu testcase to be run against this 
package.
  ==

  -- Found in releases qemu-2.0.0, qemu-2.0.2, qemu-2.1.0. Tested on
  Ubuntu 14.04 using Ext4 filesystems.

  The command

    qemu-img convert -O raw inputimage.qcow2 outputimage.raw

  intermittently creates corrupted output images, when the input image
  is not yet fully synchronized to disk. While the issue has actually
  been discovered in operation of of OpenStack nova, it can be
  reproduced "easily" on command line using

    cat $SRC_PATH > $TMP_PATH && $QEMU_IMG_PATH convert -O raw $TMP_PATH
  $DST_PATH && cksum $DST_PATH

  on filesystems exposing this behavior. (The difficult part of this
  exercise is to prepare a filesystem to reliably trigger this race. On
  my test machine some filesystems are affected while other aren't, and
  unfortunately I haven't found the relevant difference between them,
  yet. Possible it's timing issues completely out of userspace control
  ...)

  The root cause, however, is the same as in

    http://lists.gnu.org/archive/html/coreutils/2011-04/msg00069.html

  and it can be solved the same way as suggested in

    http://lists.gnu.org/archive/html/coreutils/2011-04/msg00102.html

  In qemu, file block/raw-posix.c use the FIEMAP_FLAG_SYNC, i.e change

  f.fm.fm_flags = 0;

  to

  f.fm.fm_flags = FIEMAP_FLAG_SYNC;

  As discussed in the thread mentioned above, retrieving a page cache
  coherent map of file extents is possible only after fsync on that
  file.

  See also

    https://bugs.launchpad.net/nova/+bug/1350766

  In that bug report filed against nova, fsync had been suggested to be
  performed by the framework invoking qemu-img. However, as the choice
  of fiemap -- implying this otherwise unneeded fsync of a temporary
  file  -- is not made by the caller but by qemu-img, I agree with the
  nova bug reviewer's objection to put it into nova. The fsync should
  instead be triggered by qemu-img utilizing the FIEMAP_FLAG_SYNC,
  specifically intended for that purpose.

To manage notifications about this bug go to:
https://bugs.launchpad.net/cinder/+bug/1368815/+subscriptions



Re: [Qemu-devel] [PATCH v4 1/8] fw_cfg: max access size and region size are the same for MMIO data reg

2014-12-16 Thread Paolo Bonzini
On 16/12/2014 21:17, Laszlo Ersek wrote:
>> > I can't imagine how that would happen; fw_cfg_data_mem_read() ignores
>> > both "addr" and "size", and fw_cfg_read() simply advances the
>> > "cur_offset" member.
> Ah okay, I understand your point now; you're probably saying that
> access_with_adjusted_size() traverses the offsets in the wrong order.
> ... I don't see how; the only difference in the access() param list is
> the shift count. (I don't know how it should work by design.)

I think I have figured it out.

Guest endianness affects where those bytes are placed, but not the order
in which they are fetched; and the effects of guest endianness are
always cleaned up by adjust_endianness, so ultimately they do not matter.

Think of how you would implement the uint64_t read in fw_cfg:

File bytes 12 34 56 78 9a bc de f0

fw_cfg_data_mem_read should read

size==4 BE host0x12345678
size==4 LE host0x78563412
size==8 BE host0x123456789abcdef0
size==8 LE host0xf0debc9a78563412

So the implementation of fw_cfg_data_mem_read must depend on host
endianness.  Instead, memory.c always fills in bytes in the same order,
on the assumption that the reads are idempotent.

Paolo



Re: [Qemu-devel] [PATCH v4 1/8] fw_cfg: max access size and region size are the same for MMIO data reg

2014-12-16 Thread Peter Maydell
On 16 December 2014 at 20:40, Paolo Bonzini  wrote:
> Honestly neither can I.  But still the automatic splitting (which is
> even tested by tests/endianness-test.c :)) assumes idempotency of the
> components and it's not entirely surprising that it somehow/sometimes
> breaks if you don't respect that.

Oh, good point. Yeah, I don't think we want to make any guarantee
about the order in which the N byte accesses hit the device if
we're assembling an N-byte access. Implementing the device as
a proper wide-access-supporting device is definitely the way to go.

-- PMM



[Qemu-devel] [PATCH] hw/net/xen_nic.c: Set 'netdev->mac' to NULL after free it

2014-12-16 Thread Chen Gang
Since net_init() checks whether 'netdev->mac' is NULL, before alloc it;
net_release() also need set 'netdev->mac' to NULL after free it.

Signed-off-by: Chen Gang 
---
 hw/net/xen_nic.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index 8eaa77b..19ecfc4 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -428,6 +428,7 @@ static int net_free(struct XenDevice *xendev)
 netdev->nic = NULL;
 }
 g_free(netdev->mac);
+netdev->mac = NULL;
 return 0;
 }
 
-- 
1.9.3



[Qemu-devel] [PATCH] mips64-linux-user: Fix definition of struct sigaltstack

2014-12-16 Thread Ed Swierk
Without this fix, qemu segfaults when emulating the sigaltstack syscall,
because it incorrectly treats the ss_flags field as 64 bits rather than 32
bits.

Signed-off-by: Ed Swierk 
---
 linux-user/mips64/target_signal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/mips64/target_signal.h 
b/linux-user/mips64/target_signal.h
index 6e1dc8b..5fb6a2c 100644
--- a/linux-user/mips64/target_signal.h
+++ b/linux-user/mips64/target_signal.h
@@ -8,7 +8,7 @@
 typedef struct target_sigaltstack {
abi_long ss_sp;
abi_ulong ss_size;
-   abi_long ss_flags;
+   abi_int ss_flags;
 } target_stack_t;
 
 
-- 
1.9.1




[Qemu-devel] [PATCH] linux-user: Fix ioctl cmd type mismatch on 64-bit targets

2014-12-16 Thread Ed Swierk
linux-user passes the cmd argument of the ioctl syscall as a signed long,
but compares it to an unsigned int when iterating through the ioctl_entries
list.  When the cmd is a large value like 0x80047476 (TARGET_TIOCSWINSZ on
mips64) it gets sign-extended to 0x80047476, causing the comparison
to fail and resulting in lots of spurious "Unsupported ioctl" errors.
Changing the target_cmd field in the ioctl_entries list to a signed int
causes those values to be sign-extended as well during the comparison.

Signed-off-by: Ed Swierk 
---
 linux-user/syscall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index aaac6a2..d636c81 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3278,7 +3278,7 @@ typedef abi_long do_ioctl_fn(const IOCTLEntry *ie, 
uint8_t *buf_temp,
  int fd, abi_long cmd, abi_long arg);
 
 struct IOCTLEntry {
-unsigned int target_cmd;
+int target_cmd;
 unsigned int host_cmd;
 const char *name;
 int access;
-- 
1.9.1




[Qemu-devel] [PATCH] hw/net/xen_nic.c: Need free 'netdev->nic' in net_free() instead of net_disconnect()

2014-12-16 Thread Chen Gang
net_init() and net_free() are pairs, net_connect() and net_disconnect()
are pairs. net_init() creates 'netdev->nic', so also need free it in
net_free().

Signed-off-by: Chen Gang 
---
 hw/net/xen_nic.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index 7a57feb..8eaa77b 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -410,10 +410,6 @@ static void net_disconnect(struct XenDevice *xendev)
 xc_gnttab_munmap(netdev->xendev.gnttabdev, netdev->rxs, 1);
 netdev->rxs = NULL;
 }
-if (netdev->nic) {
-qemu_del_nic(netdev->nic);
-netdev->nic = NULL;
-}
 }
 
 static void net_event(struct XenDevice *xendev)
@@ -427,6 +423,10 @@ static int net_free(struct XenDevice *xendev)
 {
 struct XenNetDev *netdev = container_of(xendev, struct XenNetDev, xendev);
 
+if (netdev->nic) {
+qemu_del_nic(netdev->nic);
+netdev->nic = NULL;
+}
 g_free(netdev->mac);
 return 0;
 }
-- 
1.9.3



[Qemu-devel] [PATCH] hw/net/xen_nic.c: Free 'netdev->txs' when map 'netdev->rxs' fails

2014-12-16 Thread Chen Gang
When map 'netdev->rxs' fails, need free the original resource, or will
cause resource leak.

Signed-off-by: Chen Gang 
---
 hw/net/xen_nic.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index 63918ae..7a57feb 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -370,11 +370,16 @@ static int net_connect(struct XenDevice *xendev)
   netdev->xendev.dom,
   netdev->tx_ring_ref,
   PROT_READ | PROT_WRITE);
+if (!netdev->txs) {
+return -1;
+}
 netdev->rxs = xc_gnttab_map_grant_ref(netdev->xendev.gnttabdev,
   netdev->xendev.dom,
   netdev->rx_ring_ref,
   PROT_READ | PROT_WRITE);
-if (!netdev->txs || !netdev->rxs) {
+if (!netdev->rxs) {
+xc_gnttab_munmap(netdev->xendev.gnttabdev, netdev->txs, 1);
+netdev->txs = NULL;
 return -1;
 }
 BACK_RING_INIT(&netdev->tx_ring, netdev->txs, XC_PAGE_SIZE);
-- 
1.9.3



Re: [Qemu-devel] [PATCH v4 1/8] fw_cfg: max access size and region size are the same for MMIO data reg

2014-12-16 Thread Peter Maydell
On 16 December 2014 at 19:00, Laszlo Ersek  wrote:
> The root of this question is what each of
>
> enum device_endian {
> DEVICE_NATIVE_ENDIAN,
> DEVICE_BIG_ENDIAN,
> DEVICE_LITTLE_ENDIAN,
> };
>
> means.

An opening remark: endianness is a horribly confusing topic and
support of more than one endianness is even worse. I may have made
some inadvertent errors in this reply; if you think so please
let me know and I'll have another stab at it.

That said: the device_endian options indicate what a device's
internal idea of its endianness is. This is most relevant if
a device accepts accesses at wider than byte width
(for instance, if you can read a 32-bit value from
address offset 0 and also an 8-bit value from offset 3 then
how do those values line up? If you read a 32-bit value then
which way round is it compared to what the device's io read/write
function return?).

NATIVE_ENDIAN means "same order as the CPU's main data bus's
natural representation". (Note that this is not necessarily
the same as "the endianness the CPU currently has"; on ARM
you can flip the CPU between LE and BE at runtime, which
is basically inserting a byte-swizzling step between data
accesses and the CPU's data bus, which is always LE for ARMv7+.)

Note that RAM accessible to a KVM guest is always NATIVE_ENDIAN
(because the guest vcpu reads it directly with the h/w cpu).

> Consider the following call tree, which implements the splitting /
> combining of an MMIO read:
>
> memory_region_dispatch_read() [memory.c]
>   memory_region_dispatch_read1()
> access_with_adjusted_size()
>   memory_region_big_endian()
>   for each byte in the wide access:
> memory_region_read_accessor()
>   fw_cfg_data_mem_read() [hw/nvram/fw_cfg.c]
> fw_cfg_read()
>   adjust_endianness()
> memory_region_wrong_endianness()
> bswapXX()
>
> The function access_with_adjusted_size() always iterates over the MMIO
> address range in incrementing address order. However, it can calculate
> the shift count for memory_region_read_accessor() in two ways.
>
> When memory_region_big_endian() returns "true", the shift count
> decreases as the MMIO address increases.
>
> When memory_region_big_endian() returns "false", the shift count
> increases as the MMIO address increases.

Yep, because this is how the device has told us that it thinks
of accesses as being put together.

The column in your table "host value" is the 16 bit value read from
the device, ie what we have decided (based on device_endian) that
it would have returned us if it had supported a 16 bit read directly
itself. BE devices compose 16 bit values with the high byte first,
LE devices with the low byte first, and native-endian devices
in the same order as guest-endianness.

> In memory_region_read_accessor(), the shift count is used for a logical
> (ie. numeric) bitwise left shift (<<). That operator works with numeric
> values and hides (ie. accounts for) host endianness.
>
> Let's consider
> - an array of two bytes, [0xaa, 0xbb],
> - a uint16_t access made from the guest,
> - and all twelve possible cases.
>
> In the table below, the "host", "guest" and "device_endian" columns are
> input variables. The columns to the right are calculated / derived
> values. The arrows above the columns show the data dependencies.
>
> After memory_region_dispatch_read1() constructs the value that is
> visible in the "host value" column, it is passed to adjust_endianness().
> If memory_region_wrong_endianness() returns "true", then the host value
> is byte-swapped. The result is then passed to the guest.
>
>   
> +---+--+
>   |   
> |  |
> + 
> --+-+
>|  |
> | | | 
> |   |  |
>   +--+-+  
> |   |  |
>   | | | || |  
> |   |  |
>   |   +---+---+  ++  | |  
> ++---+  |  |
>   |   | | |   | | |  ||  | |  
> |   ||   |  |  |
>   |   | | |   | | v  |v  | v  
> |   v|   v  |  v
>  #  host  guest  device_endian  memory_region_big_endian()  host value  host 
> repr.

Re: [Qemu-devel] [PATCH v4 1/8] fw_cfg: max access size and region size are the same for MMIO data reg

2014-12-16 Thread Paolo Bonzini


On 16/12/2014 21:06, Laszlo Ersek wrote:
> On 12/16/14 20:49, Paolo Bonzini wrote:
>> fw_cfg_read (and
>> thus fw_cfg_data_mem_read) is not idempotent.  The split/compose stuff
>> accesses the bytes at offsets 8,9,10,11,12,13,14,15 and composes them
>> according to the endianness.
>>
>> In the case of fw_cfg it just retrieves 8 bytes, but in the case of host
>> big endian it reads them in the "wrong" order for some reason (sorry, I
>> haven't looked at this thoroughly).
> 
> I can't imagine how that would happen; fw_cfg_data_mem_read() ignores
> both "addr" and "size", and fw_cfg_read() simply advances the
> "cur_offset" member.

Honestly neither can I.  But still the automatic splitting (which is
even tested by tests/endianness-test.c :)) assumes idempotency of the
components and it's not entirely surprising that it somehow/sometimes
breaks if you don't respect that.

>> So the solution is:
>>
>> 1) make fw_cfg_data_mem_ops DEVICE_LITTLE_ENDIAN
>>
>> 2) make fw_cfg_data_mem_read and fw_cfg_data_mem_write call fw_cfg_read
>> and fw_cfg_write SIZE times and build up a value from the lowest byte up.
> 
> Nonetheless, that's a really nice idea! I got so stuck with the
> automatic splitting that I forgot about the possibility to act upon the
> "size" parameter in fw_cfg_data_mem_read(). Thanks!
> 
> ... Another thing that Andrew mentioned but I didn't cover in my other
> email -- what about fw_cfg_ctl_mem_ops? It's currently DEVICE_NATIVE_ENDIAN.
> 
> You flipped the combined ops to LE in commit 6fdf98f2 (and, apparently,
> I reviewed it). Shouldn't we do the same for the standalone selector?

No.  The standalone selector is used as MMIO, and the BE platforms
expect the platform to be big-endian.  The combined ops are only used on
ISA ports, where the firmware expects them to be little-endian (as
mentioned in the commit message).

That said, the standalone selector is used by BE platforms only, so we
know that the standalone selector is always DEVICE_BIG_ENDIAN.

So if you want, you can make the standalone selector and the standalone
datum BE and swap them in the firmware.  If the suggestion doesn't make
you jump up and down, I understand that. :)

Paolo



Re: [Qemu-devel] [PATCH v2 1/5] linux-aio: queue requests that cannot be submitted

2014-12-16 Thread Paolo Bonzini

> I could reproduce this very well on a random OS image that I had around.
>  This is raw over XFS over dm-crypt, and the image is about 75% sparse
> (8.2G used over 35G).  I only get 1-2%, but still it's visible.
> 
> However I can hardly reproduce it when using a partition directly:
> 
>  oldnew
> mean 9.9565 9.9636  (+0.07%)
> stddev   0.0405 0.0537
> min  9.871  9.867
> median   9.973  9.971
> max  10.01  10.053
> count20 20
> 
> I haven't tried removing layers (e.g. fully-allocated XFS image without
> dm-crypt).

Could not reproduce it with a fully-allocated XFS image, on the contrary
the patched QEMU is a bit faster:

old  new
mean14.83325 14.82660
stddev  0.016930 0.010328
min 14.819   14.818
max 14.854   14.883
median  14.8225  14.8255
count   20   20

Paolo




Re: [Qemu-devel] [PATCH v4 1/8] fw_cfg: max access size and region size are the same for MMIO data reg

2014-12-16 Thread Laszlo Ersek
On 12/16/14 21:06, Laszlo Ersek wrote:
> On 12/16/14 20:49, Paolo Bonzini wrote:
>>
>>
>> On 16/12/2014 20:00, Laszlo Ersek wrote:
>>> Yes.
>>>
>>> The root of this question is what each of
>>>
>>> enum device_endian {
>>> DEVICE_NATIVE_ENDIAN,
>>> DEVICE_BIG_ENDIAN,
>>> DEVICE_LITTLE_ENDIAN,
>>> };
>>
>> Actually, I think the root of the answer :) is that fw_cfg_read (and
>> thus fw_cfg_data_mem_read) is not idempotent.  The split/compose stuff
>> accesses the bytes at offsets 8,9,10,11,12,13,14,15 and composes them
>> according to the endianness.
>>
>> In the case of fw_cfg it just retrieves 8 bytes, but in the case of host
>> big endian it reads them in the "wrong" order for some reason (sorry, I
>> haven't looked at this thoroughly).
> 
> I can't imagine how that would happen; fw_cfg_data_mem_read() ignores
> both "addr" and "size", and fw_cfg_read() simply advances the
> "cur_offset" member.

Ah okay, I understand your point now; you're probably saying that
access_with_adjusted_size() traverses the offsets in the wrong order.
... I don't see how; the only difference in the access() param list is
the shift count. (I don't know how it should work by design.)

In any case fw_cfg should be able to handle the larger accesses itself.

Thanks
Laszlo



Re: [Qemu-devel] [PATCH v4 1/8] fw_cfg: max access size and region size are the same for MMIO data reg

2014-12-16 Thread Laszlo Ersek
On 12/16/14 20:49, Paolo Bonzini wrote:
> 
> 
> On 16/12/2014 20:00, Laszlo Ersek wrote:
>> Yes.
>>
>> The root of this question is what each of
>>
>> enum device_endian {
>> DEVICE_NATIVE_ENDIAN,
>> DEVICE_BIG_ENDIAN,
>> DEVICE_LITTLE_ENDIAN,
>> };
> 
> Actually, I think the root of the answer :) is that fw_cfg_read (and
> thus fw_cfg_data_mem_read) is not idempotent.  The split/compose stuff
> accesses the bytes at offsets 8,9,10,11,12,13,14,15 and composes them
> according to the endianness.
> 
> In the case of fw_cfg it just retrieves 8 bytes, but in the case of host
> big endian it reads them in the "wrong" order for some reason (sorry, I
> haven't looked at this thoroughly).

I can't imagine how that would happen; fw_cfg_data_mem_read() ignores
both "addr" and "size", and fw_cfg_read() simply advances the
"cur_offset" member.

> So the solution is:
> 
> 1) make fw_cfg_data_mem_ops DEVICE_LITTLE_ENDIAN
> 
> 2) make fw_cfg_data_mem_read and fw_cfg_data_mem_write call fw_cfg_read
> and fw_cfg_write SIZE times and build up a value from the lowest byte up.

Nonetheless, that's a really nice idea! I got so stuck with the
automatic splitting that I forgot about the possibility to act upon the
"size" parameter in fw_cfg_data_mem_read(). Thanks!

... Another thing that Andrew mentioned but I didn't cover in my other
email -- what about fw_cfg_ctl_mem_ops? It's currently DEVICE_NATIVE_ENDIAN.

You flipped the combined ops to LE in commit 6fdf98f2 (and, apparently,
I reviewed it). Shouldn't we do the same for the standalone selector?

Thanks
Laszlo




[Qemu-devel] [PULL 06/30] target-mips: Enable vectored interrupt support for the 74Kf CPU

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Enable vectored interrupt support for the 74Kf CPU, reflecting hardware.

Signed-off-by: Maciej W. Rozycki 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 target-mips/translate_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
index f0c1072..7f73aa2 100644
--- a/target-mips/translate_init.c
+++ b/target-mips/translate_init.c
@@ -334,7 +334,7 @@ static const mips_def_t mips_defs[] =
(1 << CP0C1_CA),
 .CP0_Config2 = MIPS_CONFIG2,
 .CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_DSP2P) | (1 << CP0C3_DSPP) |
-   (0 << CP0C3_VInt),
+   (1 << CP0C3_VInt),
 .CP0_LLAddr_rw_bitmask = 0,
 .CP0_LLAddr_shift = 4,
 .SYNCI_Step = 32,
-- 
2.1.0




[Qemu-devel] [PULL 05/30] target-mips: Add M14K and M14Kc MIPS32r2 microMIPS processors

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Add the M14K and M14Kc processors from MIPS Technologies that are the
original implementation of the microMIPS ISA.  They are dual instruction
set processors, implementing both the microMIPS and the standard MIPSr32
ISA.

These processors correspond to the M4K and 4KEc CPUs respectively,
except with support for the microMIPS instruction set added, support for
the MCU ASE added and two extra interrupt lines, making a total of 8
hardware interrupts plus 2 software interrupts.  The remaining parts of
the microarchitecture, in particular the pipeline, stayed unchanged.

The presence of the microMIPS ASE is is reflected in the configuration
added.  We currently have no support for the MCU ASE, including in
particular the ACLR, ASET and IRET instructions in either encoding, and
we have no support for the extra interrupt lines, including bits in
CP0.Status and CP0.Cause registers, so these features are not marked,
making our support diverge from real hardware.

Signed-off-by: Sandra Loosemore 
Signed-off-by: Maciej W. Rozycki 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 target-mips/translate_init.c | 41 +
 1 file changed, 41 insertions(+)

diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
index 607f1c8..f0c1072 100644
--- a/target-mips/translate_init.c
+++ b/target-mips/translate_init.c
@@ -348,6 +348,47 @@ static const mips_def_t mips_defs[] =
 .mmu_type = MMU_TYPE_R4000,
 },
 {
+.name = "M14K",
+.CP0_PRid = 0x00019b00,
+/* Config1 implemented, fixed mapping MMU,
+   no virtual icache, uncached coherency. */
+.CP0_Config0 = MIPS_CONFIG0 | (0x2 << CP0C0_KU) | (0x2 << CP0C0_K23) |
+   (0x1 << CP0C0_AR) | (MMU_TYPE_FMT << CP0C0_MT),
+.CP0_Config1 = MIPS_CONFIG1,
+.CP0_Config2 = MIPS_CONFIG2,
+.CP0_Config3 = MIPS_CONFIG3 | (0x2 << CP0C3_ISA) | (1 << CP0C3_VInt),
+.CP0_LLAddr_rw_bitmask = 0,
+.CP0_LLAddr_shift = 4,
+.SYNCI_Step = 32,
+.CCRes = 2,
+.CP0_Status_rw_bitmask = 0x1258FF17,
+.SEGBITS = 32,
+.PABITS = 32,
+.insn_flags = CPU_MIPS32R2 | ASE_MICROMIPS,
+.mmu_type = MMU_TYPE_FMT,
+},
+{
+.name = "M14Kc",
+/* This is the TLB-based MMU core.  */
+.CP0_PRid = 0x00019c00,
+.CP0_Config0 = MIPS_CONFIG0 | (0x1 << CP0C0_AR) |
+   (MMU_TYPE_R4000 << CP0C0_MT),
+.CP0_Config1 = MIPS_CONFIG1 | (15 << CP0C1_MMU) |
+   (0 << CP0C1_IS) | (3 << CP0C1_IL) | (1 << CP0C1_IA) |
+   (0 << CP0C1_DS) | (3 << CP0C1_DL) | (1 << CP0C1_DA),
+.CP0_Config2 = MIPS_CONFIG2,
+.CP0_Config3 = MIPS_CONFIG3 | (0x2 << CP0C3_ISA) | (0 << CP0C3_VInt),
+.CP0_LLAddr_rw_bitmask = 0,
+.CP0_LLAddr_shift = 4,
+.SYNCI_Step = 32,
+.CCRes = 2,
+.CP0_Status_rw_bitmask = 0x1278FF17,
+.SEGBITS = 32,
+.PABITS = 32,
+.insn_flags = CPU_MIPS32R2 | ASE_MICROMIPS,
+.mmu_type = MMU_TYPE_R4000,
+},
+{
 /* A generic CPU providing MIPS32 Release 5 features.
FIXME: Eventually this should be replaced by a real CPU model. */
 .name = "mips32r5-generic",
-- 
2.1.0




[Qemu-devel] [PULL 04/30] target-mips: Make CP0.Config4 and CP0.Config5 registers signed

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Make the data type used for the CP0.Config4 and CP0.Config5 registers
and their mask signed, for consistency with the remaining 32-bit CP0
registers, like CP0.Config0, etc.

Signed-off-by: Maciej W. Rozycki 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 target-mips/cpu.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index c01bbda..a08c2c8 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -446,8 +446,8 @@ struct CPUMIPSState {
 #define CP0C3_MT   2
 #define CP0C3_SM   1
 #define CP0C3_TL   0
-uint32_t CP0_Config4;
-uint32_t CP0_Config4_rw_bitmask;
+int32_t CP0_Config4;
+int32_t CP0_Config4_rw_bitmask;
 #define CP0C4_M31
 #define CP0C4_IE   29
 #define CP0C4_KScrExist 16
@@ -456,8 +456,8 @@ struct CPUMIPSState {
 #define CP0C4_FTLBWays 4
 #define CP0C4_FTLBSets 0
 #define CP0C4_MMUSizeExt 0
-uint32_t CP0_Config5;
-uint32_t CP0_Config5_rw_bitmask;
+int32_t CP0_Config5;
+int32_t CP0_Config5_rw_bitmask;
 #define CP0C5_M  31
 #define CP0C5_K  30
 #define CP0C5_CV 29
-- 
2.1.0




[Qemu-devel] [PULL 08/30] target-mips: Fix formatting in `mips_defs'

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Signed-off-by: Maciej W. Rozycki 
Signed-off-by: Leon Alrae 
---
 target-mips/translate_init.c | 40 +---
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
index 7f73aa2..1543f6c 100644
--- a/target-mips/translate_init.c
+++ b/target-mips/translate_init.c
@@ -645,10 +645,11 @@ static const mips_def_t mips_defs[] =
 {
 .name = "Loongson-2E",
 .CP0_PRid = 0x6302,
-/*64KB I-cache and d-cache. 4 way with 32 bit cache line size*/
-.CP0_Config0 = (0x1<<17) | (0x1<<16) | (0x1<<11) | (0x1<<8) | (0x1<<5) 
|
-   (0x1<<4) | (0x1<<1),
-/* Note: Config1 is only used internally, Loongson-2E has only 
Config0. */
+/* 64KB I-cache and d-cache. 4 way with 32 bit cache line size.  */
+.CP0_Config0 = (0x1<<17) | (0x1<<16) | (0x1<<11) | (0x1<<8) |
+   (0x1<<5) | (0x1<<4) | (0x1<<1),
+/* Note: Config1 is only used internally,
+   Loongson-2E has only Config0.  */
 .CP0_Config1 = (1 << CP0C1_FP) | (47 << CP0C1_MMU),
 .SYNCI_Step = 16,
 .CCRes = 2,
@@ -660,21 +661,22 @@ static const mips_def_t mips_defs[] =
 .mmu_type = MMU_TYPE_R4000,
 },
 {
-  .name = "Loongson-2F",
-  .CP0_PRid = 0x6303,
-  /*64KB I-cache and d-cache. 4 way with 32 bit cache line size*/
-  .CP0_Config0 = (0x1<<17) | (0x1<<16) | (0x1<<11) | (0x1<<8) | (0x1<<5) |
- (0x1<<4) | (0x1<<1),
-  /* Note: Config1 is only used internally, Loongson-2F has only Config0. 
*/
-  .CP0_Config1 = (1 << CP0C1_FP) | (47 << CP0C1_MMU),
-  .SYNCI_Step = 16,
-  .CCRes = 2,
-  .CP0_Status_rw_bitmask = 0xF5D0FF1F,   /*bit5:7 not writable*/
-  .CP1_fcr0 = (0x5 << FCR0_PRID) | (0x1 << FCR0_REV),
-  .SEGBITS = 40,
-  .PABITS = 40,
-  .insn_flags = CPU_LOONGSON2F,
-  .mmu_type = MMU_TYPE_R4000,
+.name = "Loongson-2F",
+.CP0_PRid = 0x6303,
+/* 64KB I-cache and d-cache. 4 way with 32 bit cache line size.  */
+.CP0_Config0 = (0x1<<17) | (0x1<<16) | (0x1<<11) | (0x1<<8) |
+   (0x1<<5) | (0x1<<4) | (0x1<<1),
+/* Note: Config1 is only used internally,
+   Loongson-2F has only Config0.  */
+.CP0_Config1 = (1 << CP0C1_FP) | (47 << CP0C1_MMU),
+.SYNCI_Step = 16,
+.CCRes = 2,
+.CP0_Status_rw_bitmask = 0xF5D0FF1F,   /* Bits 7:5 not writable.  */
+.CP1_fcr0 = (0x5 << FCR0_PRID) | (0x1 << FCR0_REV),
+.SEGBITS = 40,
+.PABITS = 40,
+.insn_flags = CPU_LOONGSON2F,
+.mmu_type = MMU_TYPE_R4000,
 },
 {
 /* A generic CPU providing MIPS64 ASE DSP 2 features.
-- 
2.1.0




[Qemu-devel] [PULL 07/30] target-mips: Fix formatting in `decode_extended_mips16_opc'

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Signed-off-by: Maciej W. Rozycki 
Signed-off-by: Leon Alrae 
---
 target-mips/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index f0b8e6f..643214a 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -11099,7 +11099,7 @@ static int decode_extended_mips16_opc (CPUMIPSState 
*env, DisasContext *ctx)
 break;
 #if defined(TARGET_MIPS64)
 case M16_OPC_LD:
-check_mips_64(ctx);
+check_mips_64(ctx);
 gen_ld(ctx, OPC_LD, ry, rx, offset);
 break;
 #endif
-- 
2.1.0




[Qemu-devel] [PULL 28/30] disas/mips: remove unused mips_msa_control_names_numeric[32]

2014-12-16 Thread Leon Alrae
Signed-off-by: Leon Alrae 
Reviewed-by: Peter Maydell 
---
 disas/mips.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/disas/mips.c b/disas/mips.c
index 2614c52..b94d5d9 100644
--- a/disas/mips.c
+++ b/disas/mips.c
@@ -3801,13 +3801,6 @@ static const char * const mips_hwr_names_mips3264r2[32] =
   "$24",  "$25",  "$26",  "$27",  "$28",  "$29",  "$30",  "$31"
 };
 
-static const char * const mips_msa_control_names_numeric[32] = {
-  "$0",   "$1",   "$2",   "$3",   "$4",   "$5",   "$6",   "$7",
-  "$8",   "$9",   "$10",  "$11",  "$12",  "$13",  "$14",  "$15",
-  "$16",  "$17",  "$18",  "$19",  "$20",  "$21",  "$22",  "$23",
-  "$24",  "$25",  "$26",  "$27",  "$28",  "$29",  "$30",  "$31"
-};
-
 static const char * const mips_msa_control_names_mips3264r2[32] = {
   "MSAIR", "MSACSR", "$2", "$3",  "$4",   "$5",   "$6",   "$7",
   "$8",   "$9",   "$10",  "$11",  "$12",  "$13",  "$14",  "$15",
-- 
2.1.0




[Qemu-devel] [PULL 29/30] disas/mips: disable unused mips16_to_32_reg_map[]

2014-12-16 Thread Leon Alrae
This array is used by print_mips16_insn_arg() which is guarded by #if 0.
Therefore doing the same with the array as it generates clang warnings.

Signed-off-by: Leon Alrae 
---
 disas/mips.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/disas/mips.c b/disas/mips.c
index b94d5d9..1afe0c5 100644
--- a/disas/mips.c
+++ b/disas/mips.c
@@ -3511,6 +3511,7 @@ struct mips_cp0sel_name
   const char * const name;
 };
 
+#if 0
 /* The mips16 registers.  */
 static const unsigned int mips16_to_32_reg_map[] =
 {
@@ -3518,7 +3519,7 @@ static const unsigned int mips16_to_32_reg_map[] =
 };
 
 #define mips16_reg_names(rn)   mips_gpr_names[mips16_to_32_reg_map[rn]]
-
+#endif
 
 static const char * const mips_gpr_names_numeric[32] =
 {
-- 
2.1.0




[Qemu-devel] [PULL 25/30] target-mips: Use local float status pointer across MSA macros

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Reduce line wrapping throughout MSA helper macros by using a local float
status pointer rather than referring to the float status through the
environment each time.  No functional change.

Signed-off-by: Maciej W. Rozycki 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 target-mips/msa_helper.c | 69 
 1 file changed, 34 insertions(+), 35 deletions(-)

diff --git a/target-mips/msa_helper.c b/target-mips/msa_helper.c
index b08f37f..6e07f6e 100644
--- a/target-mips/msa_helper.c
+++ b/target-mips/msa_helper.c
@@ -1782,15 +1782,14 @@ static inline int32 float64_to_q32(float64 a 
STATUS_PARAM)
 
 #define MSA_FLOAT_COND(DEST, OP, ARG1, ARG2, BITS, QUIET)   \
 do {\
+float_status *status = &env->active_tc.msa_fp_status;   \
 int c;  \
 int64_t cond;   \
-set_float_exception_flags(0, &env->active_tc.msa_fp_status);\
+set_float_exception_flags(0, status);   \
 if (!QUIET) {   \
-cond = float ## BITS ## _ ## OP(ARG1, ARG2, \
-  &env->active_tc.msa_fp_status);   \
+cond = float ## BITS ## _ ## OP(ARG1, ARG2, status);\
 } else {\
-cond = float ## BITS ## _ ## OP ## _quiet(ARG1, ARG2,   \
-  &env->active_tc.msa_fp_status);   \
+cond = float ## BITS ## _ ## OP ## _quiet(ARG1, ARG2, status);  \
 }   \
 DEST = cond ? M_MAX_UINT(BITS) : 0; \
 c = update_msacsr(env, CLEAR_IS_INEXACT, 0);\
@@ -2375,11 +2374,11 @@ void helper_msa_fsne_df(CPUMIPSState *env, uint32_t df, 
uint32_t wd,
 
 #define MSA_FLOAT_BINOP(DEST, OP, ARG1, ARG2, BITS) \
 do {\
+float_status *status = &env->active_tc.msa_fp_status;   \
 int c;  \
 \
-set_float_exception_flags(0, &env->active_tc.msa_fp_status);\
-DEST = float ## BITS ## _ ## OP(ARG1, ARG2, \
-&env->active_tc.msa_fp_status); \
+set_float_exception_flags(0, status);   \
+DEST = float ## BITS ## _ ## OP(ARG1, ARG2, status);\
 c = update_msacsr(env, 0, IS_DENORMAL(DEST, BITS)); \
 \
 if (get_enabled_exceptions(env, c)) {   \
@@ -2511,11 +2510,11 @@ void helper_msa_fdiv_df(CPUMIPSState *env, uint32_t df, 
uint32_t wd,
 
 #define MSA_FLOAT_MULADD(DEST, ARG1, ARG2, ARG3, NEGATE, BITS)  \
 do {\
+float_status *status = &env->active_tc.msa_fp_status;   \
 int c;  \
 \
-set_float_exception_flags(0, &env->active_tc.msa_fp_status);\
-DEST = float ## BITS ## _muladd(ARG2, ARG3, ARG1, NEGATE,   \
-&env->active_tc.msa_fp_status); \
+set_float_exception_flags(0, status);   \
+DEST = float ## BITS ## _muladd(ARG2, ARG3, ARG1, NEGATE, status);  \
 c = update_msacsr(env, 0, IS_DENORMAL(DEST, BITS)); \
 \
 if (get_enabled_exceptions(env, c)) {   \
@@ -2630,10 +2629,11 @@ void helper_msa_fexp2_df(CPUMIPSState *env, uint32_t 
df, uint32_t wd,
 
 #define MSA_FLOAT_UNOP(DEST, OP, ARG, BITS) \
 do {\
+float_status *status = &env->active_tc.msa_fp_status;   \
 int c;  \
 \
-set_float_exception_flags(0, &env->active_tc.msa_fp_status);\
-DEST = float ## BITS ## _ ## OP(ARG, &env->active_tc.msa_fp_status);\

[Qemu-devel] [PULL 27/30] target-mips: convert single case switch into if statement

2014-12-16 Thread Leon Alrae
Signed-off-by: Leon Alrae 
Reviewed-by: Peter Maydell 
---
 target-mips/translate.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index f65ed84..1205909 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -1882,10 +1882,8 @@ static inline void gen_r6_cmp_ ## fmt(DisasContext * 
ctx, int n,\
 {   \
 TCGv_i ## bits fp0 = tcg_temp_new_i ## bits();  \
 TCGv_i ## bits fp1 = tcg_temp_new_i ## bits();  \
-switch (ifmt) { \
-case FMT_D: \
+if (ifmt == FMT_D) {\
 check_cp1_registers(ctx, fs | ft | fd); \
-break;  \
 }   \
 gen_ldcmp_fpr ## bits(ctx, fp0, fs);\
 gen_ldcmp_fpr ## bits(ctx, fp1, ft);\
-- 
2.1.0




[Qemu-devel] [PULL 20/30] target-mips: Correct 32-bit address space wrapping

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Make sure the address space is unconditionally wrapped on 32-bit
processors, that is ones that do not implement at least the MIPS III
ISA.

Also make MIPS16 SAVE and RESTORE instructions use address calculation
rather than plain arithmetic operations for stack pointer manipulation
so that their semantics for stack accesses follows the architecture
specification.  That in particular applies to user software run on
64-bit processors with the CP0.Status.UX bit clear where the address
space is wrapped to 32 bits.

Signed-off-by: Maciej W. Rozycki 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 target-mips/cpu.h   |  8 +---
 target-mips/translate.c | 19 ++-
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index e59cb4c..f8cf143 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -838,10 +838,12 @@ static inline void compute_hflags(CPUMIPSState *env)
 env->hflags |= MIPS_HFLAG_64;
 }
 
-if (((env->hflags & MIPS_HFLAG_KSU) == MIPS_HFLAG_UM) &&
-!(env->CP0_Status & (1 << CP0St_UX))) {
+if (!(env->insn_flags & ISA_MIPS3)) {
 env->hflags |= MIPS_HFLAG_AWRAP;
-} else if (env->insn_flags & ISA_MIPS32R6) {
+} else if (((env->hflags & MIPS_HFLAG_KSU) == MIPS_HFLAG_UM) &&
+   !(env->CP0_Status & (1 << CP0St_UX))) {
+env->hflags |= MIPS_HFLAG_AWRAP;
+} else if (env->insn_flags & ISA_MIPS64R6) {
 /* Address wrapping for Supervisor and Kernel is specified in R6 */
 if env->hflags & MIPS_HFLAG_KSU) == MIPS_HFLAG_SM) &&
  !(env->CP0_Status & (1 << CP0St_SX))) ||
diff --git a/target-mips/translate.c b/target-mips/translate.c
index 2173ea5..9d90da0 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -10728,6 +10728,7 @@ static void gen_mips16_save (DisasContext *ctx,
 {
 TCGv t0 = tcg_temp_new();
 TCGv t1 = tcg_temp_new();
+TCGv t2 = tcg_temp_new();
 int args, astatic;
 
 switch (aregs) {
@@ -10786,7 +10787,8 @@ static void gen_mips16_save (DisasContext *ctx,
 gen_load_gpr(t0, 29);
 
 #define DECR_AND_STORE(reg) do { \
-tcg_gen_subi_tl(t0, t0, 4);  \
+tcg_gen_movi_tl(t2, -4); \
+gen_op_addr_add(ctx, t0, t0, t2);\
 gen_load_gpr(t1, reg);   \
 tcg_gen_qemu_st_tl(t1, t0, ctx->mem_idx, MO_TEUL); \
 } while (0)
@@ -10870,9 +10872,11 @@ static void gen_mips16_save (DisasContext *ctx,
 }
 #undef DECR_AND_STORE
 
-tcg_gen_subi_tl(cpu_gpr[29], cpu_gpr[29], framesize);
+tcg_gen_movi_tl(t2, -framesize);
+gen_op_addr_add(ctx, cpu_gpr[29], cpu_gpr[29], t2);
 tcg_temp_free(t0);
 tcg_temp_free(t1);
+tcg_temp_free(t2);
 }
 
 static void gen_mips16_restore (DisasContext *ctx,
@@ -10883,11 +10887,14 @@ static void gen_mips16_restore (DisasContext *ctx,
 int astatic;
 TCGv t0 = tcg_temp_new();
 TCGv t1 = tcg_temp_new();
+TCGv t2 = tcg_temp_new();
 
-tcg_gen_addi_tl(t0, cpu_gpr[29], framesize);
+tcg_gen_movi_tl(t2, framesize);
+gen_op_addr_add(ctx, t0, cpu_gpr[29], t2);
 
 #define DECR_AND_LOAD(reg) do {\
-tcg_gen_subi_tl(t0, t0, 4);\
+tcg_gen_movi_tl(t2, -4);   \
+gen_op_addr_add(ctx, t0, t0, t2);  \
 tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_TESL); \
 gen_store_gpr(t1, reg);\
 } while (0)
@@ -10971,9 +10978,11 @@ static void gen_mips16_restore (DisasContext *ctx,
 }
 #undef DECR_AND_LOAD
 
-tcg_gen_addi_tl(cpu_gpr[29], cpu_gpr[29], framesize);
+tcg_gen_movi_tl(t2, framesize);
+gen_op_addr_add(ctx, cpu_gpr[29], cpu_gpr[29], t2);
 tcg_temp_free(t0);
 tcg_temp_free(t1);
+tcg_temp_free(t2);
 }
 
 static void gen_addiupc (DisasContext *ctx, int rx, int imm,
-- 
2.1.0




[Qemu-devel] [PULL 24/30] target-mips: Add missing calls to synchronise SoftFloat status

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Add missing calls to synchronise the SoftFloat status with the CP1.FSCR:

+ for the rounding and flush-to-zero modes upon processor reset,

+ for the flush-to-zero mode on FSCR updates through the GDB stub.

Refactor code accordingly and remove the redundant RESTORE_ROUNDING_MODE
macro.

Signed-off-by: Thomas Schwinge 
Signed-off-by: Maciej W. Rozycki 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 target-mips/cpu.h   | 12 
 target-mips/gdbstub.c   |  8 +++-
 target-mips/op_helper.c | 12 
 target-mips/translate.c |  2 ++
 4 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index f8cf143..8875c97 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -777,6 +777,18 @@ target_ulong exception_resume_pc (CPUMIPSState *env);
 extern unsigned int ieee_rm[];
 int ieee_ex_to_mips(int xcpt);
 
+static inline void restore_rounding_mode(CPUMIPSState *env)
+{
+set_float_rounding_mode(ieee_rm[env->active_fpu.fcr31 & 3],
+&env->active_fpu.fp_status);
+}
+
+static inline void restore_flush_mode(CPUMIPSState *env)
+{
+set_flush_to_zero((env->active_fpu.fcr31 & (1 << 24)) != 0,
+  &env->active_fpu.fp_status);
+}
+
 static inline void cpu_get_tb_cpu_state(CPUMIPSState *env, target_ulong *pc,
 target_ulong *cs_base, int *flags)
 {
diff --git a/target-mips/gdbstub.c b/target-mips/gdbstub.c
index 2f2ffd2..9845d88 100644
--- a/target-mips/gdbstub.c
+++ b/target-mips/gdbstub.c
@@ -74,10 +74,6 @@ int mips_cpu_gdb_read_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 return 0;
 }
 
-#define RESTORE_ROUNDING_MODE \
-set_float_rounding_mode(ieee_rm[env->active_fpu.fcr31 & 3], \
-&env->active_fpu.fp_status)
-
 int mips_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
 {
 MIPSCPU *cpu = MIPS_CPU(cs);
@@ -95,7 +91,9 @@ int mips_cpu_gdb_write_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 case 70:
 env->active_fpu.fcr31 = tmp & 0xFF83;
 /* set rounding mode */
-RESTORE_ROUNDING_MODE;
+restore_rounding_mode(env);
+/* set flush-to-zero mode */
+restore_flush_mode(env);
 break;
 case 71:
 /* FIR is read-only.  Ignore writes.  */
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 7e632f6..d619ba4 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -2280,18 +2280,6 @@ unsigned int ieee_rm[] = {
 float_round_down
 };
 
-static inline void restore_rounding_mode(CPUMIPSState *env)
-{
-set_float_rounding_mode(ieee_rm[env->active_fpu.fcr31 & 3],
-&env->active_fpu.fp_status);
-}
-
-static inline void restore_flush_mode(CPUMIPSState *env)
-{
-set_flush_to_zero((env->active_fpu.fcr31 & (1 << 24)) != 0,
-  &env->active_fpu.fp_status);
-}
-
 target_ulong helper_cfc1(CPUMIPSState *env, uint32_t reg)
 {
 target_ulong arg1 = 0;
diff --git a/target-mips/translate.c b/target-mips/translate.c
index 9d90da0..571b7d7 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -19617,6 +19617,8 @@ void cpu_state_reset(CPUMIPSState *env)
 }
 
 compute_hflags(env);
+restore_rounding_mode(env);
+restore_flush_mode(env);
 cs->exception_index = EXCP_NONE;
 }
 
-- 
2.1.0




[Qemu-devel] [PULL 23/30] linux-user: Use the 5KEf processor for 64-bit emulation

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Replace the 20Kc original MIPS64 ISA processor used for 64-bit user
emulation with the 5KEf processor that implements the MIPS64r2 ISA,
complementing the choice of the 24Kf processor for 32-bit emulation.

Signed-off-by: Maciej W. Rozycki 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 linux-user/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 186ee4d..67b0231 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3905,7 +3905,7 @@ int main(int argc, char **argv, char **envp)
 #endif
 #elif defined(TARGET_MIPS)
 #if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64)
-cpu_model = "20Kc";
+cpu_model = "5KEf";
 #else
 cpu_model = "24Kf";
 #endif
-- 
2.1.0




[Qemu-devel] [PULL 22/30] target-mips: Also apply the CP0.Status mask to MTTC0

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Make CP0.Status writes made with the MTTC0 instruction respect this
register's mask just like all the other places.  Also preserve the
current values of masked out bits.

Signed-off-by: Maciej W. Rozycki 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 target-mips/op_helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 1267ef2..7e632f6 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -1413,9 +1413,10 @@ void helper_mtc0_status(CPUMIPSState *env, target_ulong 
arg1)
 void helper_mttc0_status(CPUMIPSState *env, target_ulong arg1)
 {
 int other_tc = env->CP0_VPEControl & (0xff << CP0VPECo_TargTC);
+uint32_t mask = env->CP0_Status_rw_bitmask & ~0xf118;
 CPUMIPSState *other = mips_cpu_map_tc(env, &other_tc);
 
-other->CP0_Status = arg1 & ~0xf118;
+other->CP0_Status = (other->CP0_Status & ~mask) | (arg1 & mask);
 sync_c0_status(env, other, other_tc);
 }
 
-- 
2.1.0




[Qemu-devel] [PULL 19/30] target-mips: Tighten ISA level checks

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Tighten ISA level checks down to MIPS II that many of our instructions
are missing.  Also make sure any 64-bit instruction enables are only
applied to 64-bit processors, that is ones that implement at least the
MIPS III ISA.

Signed-off-by: Maciej W. Rozycki 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 target-mips/cpu.h   |   7 ++--
 target-mips/helper.c|  15 +--
 target-mips/translate.c | 107 
 3 files changed, 114 insertions(+), 15 deletions(-)

diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index dd72d1e..e59cb4c 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -831,9 +831,10 @@ static inline void compute_hflags(CPUMIPSState *env)
 env->hflags |= (env->CP0_Status >> CP0St_KSU) & MIPS_HFLAG_KSU;
 }
 #if defined(TARGET_MIPS64)
-if (((env->hflags & MIPS_HFLAG_KSU) != MIPS_HFLAG_UM) ||
-(env->CP0_Status & (1 << CP0St_PX)) ||
-(env->CP0_Status & (1 << CP0St_UX))) {
+if ((env->insn_flags & ISA_MIPS3) &&
+(((env->hflags & MIPS_HFLAG_KSU) != MIPS_HFLAG_UM) ||
+ (env->CP0_Status & (1 << CP0St_PX)) ||
+ (env->CP0_Status & (1 << CP0St_UX {
 env->hflags |= MIPS_HFLAG_64;
 }
 
diff --git a/target-mips/helper.c b/target-mips/helper.c
index 3a93c20..c4b3658 100644
--- a/target-mips/helper.c
+++ b/target-mips/helper.c
@@ -527,7 +527,10 @@ void mips_cpu_do_interrupt(CPUState *cs)
 env->CP0_DEPC = exception_resume_pc(env);
 env->hflags &= ~MIPS_HFLAG_BMASK;
  enter_debug_mode:
-env->hflags |= MIPS_HFLAG_DM | MIPS_HFLAG_64 | MIPS_HFLAG_CP0;
+if (env->insn_flags & ISA_MIPS3) {
+env->hflags |= MIPS_HFLAG_64;
+}
+env->hflags |= MIPS_HFLAG_DM | MIPS_HFLAG_CP0;
 env->hflags &= ~(MIPS_HFLAG_KSU);
 /* EJTAG probe trap enable is not implemented... */
 if (!(env->CP0_Status & (1 << CP0St_EXL)))
@@ -548,7 +551,10 @@ void mips_cpu_do_interrupt(CPUState *cs)
 env->CP0_ErrorEPC = exception_resume_pc(env);
 env->hflags &= ~MIPS_HFLAG_BMASK;
 env->CP0_Status |= (1 << CP0St_ERL) | (1 << CP0St_BEV);
-env->hflags |= MIPS_HFLAG_64 | MIPS_HFLAG_CP0;
+if (env->insn_flags & ISA_MIPS3) {
+env->hflags |= MIPS_HFLAG_64;
+}
+env->hflags |= MIPS_HFLAG_CP0;
 env->hflags &= ~(MIPS_HFLAG_KSU);
 if (!(env->CP0_Status & (1 << CP0St_EXL)))
 env->CP0_Cause &= ~(1U << CP0Ca_BD);
@@ -726,7 +732,10 @@ void mips_cpu_do_interrupt(CPUState *cs)
 env->CP0_Cause &= ~(1U << CP0Ca_BD);
 }
 env->CP0_Status |= (1 << CP0St_EXL);
-env->hflags |= MIPS_HFLAG_64 | MIPS_HFLAG_CP0;
+if (env->insn_flags & ISA_MIPS3) {
+env->hflags |= MIPS_HFLAG_64;
+}
+env->hflags |= MIPS_HFLAG_CP0;
 env->hflags &= ~(MIPS_HFLAG_KSU);
 }
 env->hflags &= ~MIPS_HFLAG_BMASK;
diff --git a/target-mips/translate.c b/target-mips/translate.c
index d4fedfb..2173ea5 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -2398,7 +2398,14 @@ static void gen_cop1_ldst(DisasContext *ctx, uint32_t 
op, int rt,
 {
 if (ctx->CP0_Config1 & (1 << CP0C1_FP)) {
 check_cp1_enabled(ctx);
-gen_flt_ldst(ctx, op, rt, rs, imm);
+switch (op) {
+case OPC_LDC1:
+case OPC_SDC1:
+check_insn(ctx, ISA_MIPS2);
+/* Fallthrough */
+default:
+gen_flt_ldst(ctx, op, rt, rs, imm);
+}
 } else {
 generate_exception_err(ctx, EXCP_CpU, 1);
 }
@@ -10997,26 +11004,32 @@ static void decode_i64_mips16 (DisasContext *ctx,
 {
 switch (funct) {
 case I64_LDSP:
+check_insn(ctx, ISA_MIPS3);
 check_mips_64(ctx);
 offset = extended ? offset : offset << 3;
 gen_ld(ctx, OPC_LD, ry, 29, offset);
 break;
 case I64_SDSP:
+check_insn(ctx, ISA_MIPS3);
 check_mips_64(ctx);
 offset = extended ? offset : offset << 3;
 gen_st(ctx, OPC_SD, ry, 29, offset);
 break;
 case I64_SDRASP:
+check_insn(ctx, ISA_MIPS3);
 check_mips_64(ctx);
 offset = extended ? offset : (ctx->opcode & 0xff) << 3;
 gen_st(ctx, OPC_SD, 31, 29, offset);
 break;
 case I64_DADJSP:
+check_insn(ctx, ISA_MIPS3);
 check_mips_64(ctx);
 offset = extended ? offset : ((int8_t)ctx->opcode) << 3;
 gen_arith_imm(ctx, OPC_DADDIU, 29, 29, offset);
 break;
 case I64_LDPC:
+check_insn(ctx, ISA_MIPS3);
+check_mips_64(ctx);
 if (extended && (ctx->hflags & MIPS_HFLAG_BMASK)) {
 generate_exception(ctx, EXCP_RI);
 } else {
@@ -11025,16 +11038,19 @@ static void decode_i64_mips16 (DisasContext *ctx,
 }
 break;
 case I64_DADDIU5:
+check_insn(ctx

[Qemu-devel] [PULL 30/30] target-mips: remove excp_names[] from linux-user as it is unused

2014-12-16 Thread Leon Alrae
Signed-off-by: Leon Alrae 
Reviewed-by: Peter Maydell 
---
 target-mips/helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-mips/helper.c b/target-mips/helper.c
index c4b3658..7d26705 100644
--- a/target-mips/helper.c
+++ b/target-mips/helper.c
@@ -388,7 +388,6 @@ hwaddr cpu_mips_translate_address(CPUMIPSState *env, 
target_ulong address, int r
 return physical;
 }
 }
-#endif
 
 static const char * const excp_names[EXCP_LAST + 1] = {
 [EXCP_RESET] = "reset",
@@ -429,6 +428,7 @@ static const char * const excp_names[EXCP_LAST + 1] = {
 [EXCP_MSADIS] = "MSA disabled",
 [EXCP_MSAFPE] = "MSA floating point",
 };
+#endif
 
 target_ulong exception_resume_pc (CPUMIPSState *env)
 {
-- 
2.1.0




[Qemu-devel] [PULL 18/30] target-mips: Fix CP0.Config3.ISAOnExc write accesses

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Fix CP0.Config3.ISAOnExc write accesses on microMIPS processors.  This
bit is mandatory for any processor that implements the microMIPS
instruction set.  This bit is r/w for processors that implement both the
standard MIPS and the microMIPS instruction set.  This bit is r/o and
hardwired to 1 if only the microMIPS instruction set is implemented.

There is no other bit ever writable in CP0.Config3 so defining a
corresponding `CP0_Config3_rw_bitmask' member in `CPUMIPSState' is I
think an overkill.  Therefore make the ability to write the bit rely on
the presence of ASE_MICROMIPS set in the instruction flags.

The read-only case of the microMIPS instruction set being implemented
only can be added when we add support for such a configuration.  We do
not currently have such support, we have no instruction flag that would
control the presence of the standard MIPS instruction set nor any
associated code in instruction decoding.

This change is needed to boot a microMIPS Linux kernel successfully,
otherwise it hangs early on as interrupts are enabled and then the
exception handler invoked loops as its first instruction is interpreted
in the wrong execution mode and triggers another exception right away.
And then over and over again.

We already check the current setting of the CP0.Config3.ISAOnExc in
`set_hflags_for_handler' to set the ISA bit correctly on the exception
handler entry so it is the ability to set it that is missing only.

Signed-off-by: Maciej W. Rozycki 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 target-mips/helper.h| 1 +
 target-mips/op_helper.c | 8 
 target-mips/translate.c | 8 ++--
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/target-mips/helper.h b/target-mips/helper.h
index 9d02758..3bd0b02 100644
--- a/target-mips/helper.h
+++ b/target-mips/helper.h
@@ -137,6 +137,7 @@ DEF_HELPER_2(mtc0_ebase, void, env, tl)
 DEF_HELPER_2(mttc0_ebase, void, env, tl)
 DEF_HELPER_2(mtc0_config0, void, env, tl)
 DEF_HELPER_2(mtc0_config2, void, env, tl)
+DEF_HELPER_2(mtc0_config3, void, env, tl)
 DEF_HELPER_2(mtc0_config4, void, env, tl)
 DEF_HELPER_2(mtc0_config5, void, env, tl)
 DEF_HELPER_2(mtc0_lladdr, void, env, tl)
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 1ec2756..1267ef2 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -1503,6 +1503,14 @@ void helper_mtc0_config2(CPUMIPSState *env, target_ulong 
arg1)
 env->CP0_Config2 = (env->CP0_Config2 & 0x8FFF0FFF);
 }
 
+void helper_mtc0_config3(CPUMIPSState *env, target_ulong arg1)
+{
+if (env->insn_flags & ASE_MICROMIPS) {
+env->CP0_Config3 = (env->CP0_Config3 & ~(1 << CP0C3_ISA_ON_EXC)) |
+   (arg1 & (1 << CP0C3_ISA_ON_EXC));
+}
+}
+
 void helper_mtc0_config4(CPUMIPSState *env, target_ulong arg1)
 {
 env->CP0_Config4 = (env->CP0_Config4 & (~env->CP0_Config4_rw_bitmask)) |
diff --git a/target-mips/translate.c b/target-mips/translate.c
index 70da66f..d4fedfb 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -5846,8 +5846,10 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int 
reg, int sel)
 ctx->bstate = BS_STOP;
 break;
 case 3:
-/* ignored, read only */
+gen_helper_mtc0_config3(cpu_env, arg);
 rn = "Config3";
+/* Stop translation as we may have switched the execution mode */
+ctx->bstate = BS_STOP;
 break;
 case 4:
 gen_helper_mtc0_config4(cpu_env, arg);
@@ -7097,8 +7099,10 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int 
reg, int sel)
 ctx->bstate = BS_STOP;
 break;
 case 3:
-/* ignored */
+gen_helper_mtc0_config3(cpu_env, arg);
 rn = "Config3";
+/* Stop translation as we may have switched the execution mode */
+ctx->bstate = BS_STOP;
 break;
 case 4:
 /* currently ignored */
-- 
2.1.0




[Qemu-devel] [PULL 16/30] target-mips: Fix the 64-bit case for microMIPS MOVE16 and MOVEP

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Fix microMIPS MOVE16 and MOVEP instructions on 64-bit processors by
using register addition operations.

This copies the approach taken with MIPS16 MOVE instructions (I8_MOV32R
and I8_MOVR32 opcodes) and follows the observation that OPC_ADDU expands
to tcg_gen_mov_tl whenever `rt' is 0 and `rs' is not, therefore copying
`rs' to `rd' verbatim.  This is not the case with OPC_ADDIU where a
sign-extension from bit #31 is made, unless in the uninteresting case of
`rs' being 0, losing the upper 32 bits of the value copied for any
proper 64-bit values.

This also serves as an optimization as one op is produced in generated
code rather than two (again, unless `rs' is 0, where it doesn't change
anything).

Signed-off-by: Maciej W. Rozycki 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 target-mips/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index b5d5b39..1a275bf 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -13936,8 +13936,8 @@ static int decode_micromips_opc (CPUMIPSState *env, 
DisasContext *ctx)
 rs = rs_rt_enc[enc_rs];
 rt = rs_rt_enc[enc_rt];
 
-gen_arith_imm(ctx, OPC_ADDIU, rd, rs, 0);
-gen_arith_imm(ctx, OPC_ADDIU, re, rt, 0);
+gen_arith(ctx, OPC_ADDU, rd, rs, 0);
+gen_arith(ctx, OPC_ADDU, re, rt, 0);
 }
 break;
 case LBU16:
@@ -14018,7 +14018,7 @@ static int decode_micromips_opc (CPUMIPSState *env, 
DisasContext *ctx)
 int rd = uMIPS_RD5(ctx->opcode);
 int rs = uMIPS_RS5(ctx->opcode);
 
-gen_arith_imm(ctx, OPC_ADDIU, rd, rs, 0);
+gen_arith(ctx, OPC_ADDU, rd, rs, 0);
 }
 break;
 case ANDI16:
-- 
2.1.0




[Qemu-devel] [PULL 15/30] target-mips: Correct the writes to Status and Cause registers via gdbstub

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Make writes to CP0.Status and CP0.Cause have the same effect as
executing corresponding MTC0 instructions would in Kernel Mode.  Also
ignore writes in the user emulation mode.

Currently for requests from the GDB stub we write all the bits across
both registers, ignoring any read-only locations, and do not synchronise
the environment to evaluate side effects.  We also write these registers
in the user emulation mode even though a real kernel presents them as
read only.

Signed-off-by: Maciej W. Rozycki 
Signed-off-by: Leon Alrae 
---
 target-mips/cpu.h   | 89 +++
 target-mips/gdbstub.c   |  8 +++--
 target-mips/op_helper.c | 91 -
 3 files changed, 102 insertions(+), 86 deletions(-)

diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index a08c2c8..dd72d1e 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -904,4 +904,93 @@ static inline void compute_hflags(CPUMIPSState *env)
 }
 }
 
+#ifndef CONFIG_USER_ONLY
+/* Called for updates to CP0_Status.  */
+static inline void sync_c0_status(CPUMIPSState *env, CPUMIPSState *cpu, int tc)
+{
+int32_t tcstatus, *tcst;
+uint32_t v = cpu->CP0_Status;
+uint32_t cu, mx, asid, ksu;
+uint32_t mask = ((1 << CP0TCSt_TCU3)
+   | (1 << CP0TCSt_TCU2)
+   | (1 << CP0TCSt_TCU1)
+   | (1 << CP0TCSt_TCU0)
+   | (1 << CP0TCSt_TMX)
+   | (3 << CP0TCSt_TKSU)
+   | (0xff << CP0TCSt_TASID));
+
+cu = (v >> CP0St_CU0) & 0xf;
+mx = (v >> CP0St_MX) & 0x1;
+ksu = (v >> CP0St_KSU) & 0x3;
+asid = env->CP0_EntryHi & 0xff;
+
+tcstatus = cu << CP0TCSt_TCU0;
+tcstatus |= mx << CP0TCSt_TMX;
+tcstatus |= ksu << CP0TCSt_TKSU;
+tcstatus |= asid;
+
+if (tc == cpu->current_tc) {
+tcst = &cpu->active_tc.CP0_TCStatus;
+} else {
+tcst = &cpu->tcs[tc].CP0_TCStatus;
+}
+
+*tcst &= ~mask;
+*tcst |= tcstatus;
+compute_hflags(cpu);
+}
+
+static inline void cpu_mips_store_status(CPUMIPSState *env, target_ulong val)
+{
+uint32_t mask = env->CP0_Status_rw_bitmask;
+
+if (env->insn_flags & ISA_MIPS32R6) {
+bool has_supervisor = extract32(mask, CP0St_KSU, 2) == 0x3;
+
+if (has_supervisor && extract32(val, CP0St_KSU, 2) == 0x3) {
+mask &= ~(3 << CP0St_KSU);
+}
+mask &= ~(((1 << CP0St_SR) | (1 << CP0St_NMI)) & val);
+}
+
+env->CP0_Status = (env->CP0_Status & ~mask) | (val & mask);
+if (env->CP0_Config3 & (1 << CP0C3_MT)) {
+sync_c0_status(env, env, env->current_tc);
+} else {
+compute_hflags(env);
+}
+}
+
+static inline void cpu_mips_store_cause(CPUMIPSState *env, target_ulong val)
+{
+uint32_t mask = 0x00C00300;
+uint32_t old = env->CP0_Cause;
+int i;
+
+if (env->insn_flags & ISA_MIPS32R2) {
+mask |= 1 << CP0Ca_DC;
+}
+if (env->insn_flags & ISA_MIPS32R6) {
+mask &= ~((1 << CP0Ca_WP) & val);
+}
+
+env->CP0_Cause = (env->CP0_Cause & ~mask) | (val & mask);
+
+if ((old ^ env->CP0_Cause) & (1 << CP0Ca_DC)) {
+if (env->CP0_Cause & (1 << CP0Ca_DC)) {
+cpu_mips_stop_count(env);
+} else {
+cpu_mips_start_count(env);
+}
+}
+
+/* Set/reset software interrupts */
+for (i = 0 ; i < 2 ; i++) {
+if ((old ^ env->CP0_Cause) & (1 << (CP0Ca_IP + i))) {
+cpu_mips_soft_irq(env, i, env->CP0_Cause & (1 << (CP0Ca_IP + i)));
+}
+}
+}
+#endif
+
 #endif /* !defined (__MIPS_CPU_H__) */
diff --git a/target-mips/gdbstub.c b/target-mips/gdbstub.c
index e86df0e..964e6a7 100644
--- a/target-mips/gdbstub.c
+++ b/target-mips/gdbstub.c
@@ -112,7 +112,9 @@ int mips_cpu_gdb_write_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 }
 switch (n) {
 case 32:
-env->CP0_Status = tmp;
+#ifndef CONFIG_USER_ONLY
+cpu_mips_store_status(env, tmp);
+#endif
 break;
 case 33:
 env->active_tc.LO[0] = tmp;
@@ -124,7 +126,9 @@ int mips_cpu_gdb_write_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 env->CP0_BadVAddr = tmp;
 break;
 case 36:
-env->CP0_Cause = tmp;
+#ifndef CONFIG_USER_ONLY
+cpu_mips_store_cause(env, tmp);
+#endif
 break;
 case 37:
 env->active_tc.PC = tmp & ~(target_ulong)1;
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index a0cc729..1ec2756 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -625,40 +625,9 @@ static CPUMIPSState *mips_cpu_map_tc(CPUMIPSState *env, 
int *tc)
 
These helper call synchronizes the regs for a given cpu.  */
 
-/* Called for updates to CP0_Status.  */
-static void sync_c0_status(CPUMIPSState *env, CPUMIPSState *cpu, int tc)
-{
-int32_t tcstatus, *tcst;
-uint32_t v = cpu->CP0_Status;
-uint32_t cu, mx, asid, ksu

[Qemu-devel] [PULL 26/30] target-mips: Fix DisasContext's ulri member initialization

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Set DisasContext's ulri member to 0 or 1 as with other bool members.

Signed-off-by: Maciej W. Rozycki 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 target-mips/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 571b7d7..f65ed84 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -19116,7 +19116,7 @@ gen_intermediate_code_internal(MIPSCPU *cpu, 
TranslationBlock *tb,
 ctx.bp = (env->CP0_Config3 >> CP0C3_BP) & 1;
 /* Restore delay slot state from the tb context.  */
 ctx.hflags = (uint32_t)tb->flags; /* FIXME: maybe use 64 bits here? */
-ctx.ulri = env->CP0_Config3 & (1 << CP0C3_ULRI);
+ctx.ulri = (env->CP0_Config3 >> CP0C3_ULRI) & 1;
 restore_cpu_state(env, &ctx);
 #ifdef CONFIG_USER_ONLY
 ctx.mem_idx = MIPS_HFLAG_UM;
-- 
2.1.0




[Qemu-devel] [PULL 14/30] target-mips: Correct the handling of writes to CP0.Status for MIPSr6

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Correct these issues with the handling of CP0.Status for MIPSr6:

* only ignore the bit pattern of 0b11 on writes to CP0.Status.KSU, that
  is for processors that do implement Supervisor Mode, let the bit
  pattern be written to CP0.Status.UM:R0 freely (of course the value
  written to read-only CP0.Status.R0 will be discarded anyway); this is
  in accordance to the relevant architecture specification[1],

* check the newly written pattern rather than the current contents of
  CP0.Status for the KSU bits being 0b11,

* use meaningful macro names to refer to CP0.Status bits rather than
  magic numbers.

References:

[1] "MIPS Architecture For Programmers, Volume III: MIPS64 / microMIPS64
Privileged Resource Architecture", MIPS Technologies, Inc., Document
Number: MD00091, Revision 6.00, March 31, 2014, Table 9.45 "Status
Register Field Descriptions", pp. 210-211.

Signed-off-by: Maciej W. Rozycki 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 target-mips/op_helper.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index d25424f..a0cc729 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -1423,10 +1423,12 @@ void helper_mtc0_status(CPUMIPSState *env, target_ulong 
arg1)
 uint32_t mask = env->CP0_Status_rw_bitmask;
 
 if (env->insn_flags & ISA_MIPS32R6) {
-if (extract32(env->CP0_Status, CP0St_KSU, 2) == 0x3) {
+bool has_supervisor = extract32(mask, CP0St_KSU, 2) == 0x3;
+
+if (has_supervisor && extract32(arg1, CP0St_KSU, 2) == 0x3) {
 mask &= ~(3 << CP0St_KSU);
 }
-mask &= ~(0x0018 & arg1);
+mask &= ~(((1 << CP0St_SR) | (1 << CP0St_NMI)) & arg1);
 }
 
 val = arg1 & mask;
-- 
2.1.0




[Qemu-devel] [PULL 12/30] target-mips: Restore the order of helpers

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Restore the order of helpers that used to be: unary operations (generic,
then MIPS-specific), binary operations (generic, then MIPS-specific),
compare operations.  At one point FMA operations were inserted at a
random place in the file, disregarding the preexisting order, and later
on even more operations sprinkled across the file.  Revert the mess by
moving FMA operations to a new ternary class inserted after the binary
class and move the misplaced unary and binary operations to where they
belong.

Signed-off-by: Maciej W. Rozycki 
Signed-off-by: Leon Alrae 
---
 target-mips/op_helper.c | 319 
 1 file changed, 160 insertions(+), 159 deletions(-)

diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index f547801..d25424f 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -2935,110 +2935,6 @@ FLOAT_UNOP(abs)
 FLOAT_UNOP(chs)
 #undef FLOAT_UNOP
 
-#define FLOAT_FMADDSUB(name, bits, muladd_arg)  \
-uint ## bits ## _t helper_float_ ## name (CPUMIPSState *env,\
-  uint ## bits ## _t fs,\
-  uint ## bits ## _t ft,\
-  uint ## bits ## _t fd)\
-{   \
-uint ## bits ## _t fdret;   \
-\
-fdret = float ## bits ## _muladd(fs, ft, fd, muladd_arg,\
- &env->active_fpu.fp_status);   \
-update_fcr31(env, GETPC()); \
-return fdret;   \
-}
-
-FLOAT_FMADDSUB(maddf_s, 32, 0)
-FLOAT_FMADDSUB(maddf_d, 64, 0)
-FLOAT_FMADDSUB(msubf_s, 32, float_muladd_negate_product)
-FLOAT_FMADDSUB(msubf_d, 64, float_muladd_negate_product)
-#undef FLOAT_FMADDSUB
-
-#define FLOAT_MINMAX(name, bits, minmaxfunc)\
-uint ## bits ## _t helper_float_ ## name (CPUMIPSState *env,\
-  uint ## bits ## _t fs,\
-  uint ## bits ## _t ft)\
-{   \
-uint ## bits ## _t fdret;   \
-\
-fdret = float ## bits ## _ ## minmaxfunc(fs, ft,\
-   &env->active_fpu.fp_status); \
-update_fcr31(env, GETPC()); \
-return fdret;   \
-}
-
-FLOAT_MINMAX(max_s, 32, maxnum)
-FLOAT_MINMAX(max_d, 64, maxnum)
-FLOAT_MINMAX(maxa_s, 32, maxnummag)
-FLOAT_MINMAX(maxa_d, 64, maxnummag)
-
-FLOAT_MINMAX(min_s, 32, minnum)
-FLOAT_MINMAX(min_d, 64, minnum)
-FLOAT_MINMAX(mina_s, 32, minnummag)
-FLOAT_MINMAX(mina_d, 64, minnummag)
-#undef FLOAT_MINMAX
-
-#define FLOAT_RINT(name, bits)  \
-uint ## bits ## _t helper_float_ ## name (CPUMIPSState *env,\
-  uint ## bits ## _t fs)\
-{   \
-uint ## bits ## _t fdret;   \
-\
-fdret = float ## bits ## _round_to_int(fs, &env->active_fpu.fp_status); \
-update_fcr31(env, GETPC()); \
-return fdret;   \
-}
-
-FLOAT_RINT(rint_s, 32)
-FLOAT_RINT(rint_d, 64)
-#undef FLOAT_RINT
-
-#define FLOAT_CLASS_SIGNALING_NAN  0x001
-#define FLOAT_CLASS_QUIET_NAN  0x002
-#define FLOAT_CLASS_NEGATIVE_INFINITY  0x004
-#define FLOAT_CLASS_NEGATIVE_NORMAL0x008
-#define FLOAT_CLASS_NEGATIVE_SUBNORMAL 0x010
-#define FLOAT_CLASS_NEGATIVE_ZERO  0x020
-#define FLOAT_CLASS_POSITIVE_INFINITY  0x040
-#define FLOAT_CLASS_POSITIVE_NORMAL0x080
-#define FLOAT_CLASS_POSITIVE_SUBNORMAL 0x100
-#define FLOAT_CLASS_POSITIVE_ZERO  0x200
-
-#define FLOAT_CLASS(name, bits)  \
-uint ## bits ## _t helper_float_ ## name (uint ## bits ## _t arg)\
-{\
-if (float ## bits ## _is_signaling_nan(arg)) {   \
-return FLOAT_CLASS_SIGNALING_NAN;\
-} else if (float ## bits ## _is_quiet_nan(arg)) {\
-return FLOAT_CLASS_QUIET_NAN;\
-} else if (float ## bits ## _is_neg

[Qemu-devel] [PULL 13/30] target-mips: Correct MIPS16/microMIPS branch size calculation

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Correct MIPS16/microMIPS branch size calculation in PC adjustment
needed:

- to set the value of CP0.ErrorEPC at the entry to the reset exception,

- for the purpose of branch reexecution in the context of device I/O.

Follow the approach taken in `exception_resume_pc' for ordinary, Debug
and NMI exceptions.

MIPS16 and microMIPS branches can be 2 or 4 bytes in size and that has
to be reflected in calculation.  Original MIPS ISA branches, which is
where this code originates from, are always 4 bytes long, just as all
original MIPS ISA instructions.

Signed-off-by: Nathan Froyd 
Signed-off-by: Maciej W. Rozycki 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 target-mips/translate.c | 3 ++-
 translate-all.c | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index a5a5ca4..b5d5b39 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -19439,7 +19439,8 @@ void cpu_state_reset(CPUMIPSState *env)
 if (env->hflags & MIPS_HFLAG_BMASK) {
 /* If the exception was raised from a delay slot,
come back to the jump.  */
-env->CP0_ErrorEPC = env->active_tc.PC - 4;
+env->CP0_ErrorEPC = (env->active_tc.PC
+ - (env->hflags & MIPS_HFLAG_B16 ? 2 : 4));
 } else {
 env->CP0_ErrorEPC = env->active_tc.PC;
 }
diff --git a/translate-all.c b/translate-all.c
index cf05472..d930a5c 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1540,7 +1540,7 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)
branch.  */
 #if defined(TARGET_MIPS)
 if ((env->hflags & MIPS_HFLAG_BMASK) != 0 && n > 1) {
-env->active_tc.PC -= 4;
+env->active_tc.PC -= (env->hflags & MIPS_HFLAG_B16 ? 2 : 4);
 cpu->icount_decr.u16.low++;
 env->hflags &= ~MIPS_HFLAG_BMASK;
 }
-- 
2.1.0




[Qemu-devel] [PULL 21/30] target-mips: gdbstub: Clean up FPU register handling

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Rewrite the FPU register access parts of `mips_cpu_gdb_read_register'
and `mips_cpu_gdb_write_register' for consistency between each other.

Signed-off-by: Maciej W. Rozycki 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 target-mips/gdbstub.c | 38 +++---
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/target-mips/gdbstub.c b/target-mips/gdbstub.c
index 964e6a7..2f2ffd2 100644
--- a/target-mips/gdbstub.c
+++ b/target-mips/gdbstub.c
@@ -29,8 +29,13 @@ int mips_cpu_gdb_read_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 if (n < 32) {
 return gdb_get_regl(mem_buf, env->active_tc.gpr[n]);
 }
-if (env->CP0_Config1 & (1 << CP0C1_FP)) {
-if (n >= 38 && n < 70) {
+if (env->CP0_Config1 & (1 << CP0C1_FP) && n >= 38 && n < 72) {
+switch (n) {
+case 70:
+return gdb_get_regl(mem_buf, (int32_t)env->active_fpu.fcr31);
+case 71:
+return gdb_get_regl(mem_buf, (int32_t)env->active_fpu.fcr0);
+default:
 if (env->CP0_Status & (1 << CP0St_FR)) {
 return gdb_get_regl(mem_buf,
 env->active_fpu.fpr[n - 38].d);
@@ -39,12 +44,6 @@ int mips_cpu_gdb_read_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 env->active_fpu.fpr[n - 38].w[FP_ENDIAN_IDX]);
 }
 }
-switch (n) {
-case 70:
-return gdb_get_regl(mem_buf, (int32_t)env->active_fpu.fcr31);
-case 71:
-return gdb_get_regl(mem_buf, (int32_t)env->active_fpu.fcr0);
-}
 }
 switch (n) {
 case 32:
@@ -64,8 +63,10 @@ int mips_cpu_gdb_read_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 return gdb_get_regl(mem_buf, 0); /* fp */
 case 89:
 return gdb_get_regl(mem_buf, (int32_t)env->CP0_PRid);
-}
-if (n >= 73 && n <= 88) {
+default:
+if (n > 89) {
+return 0;
+}
 /* 16 embedded regs.  */
 return gdb_get_regl(mem_buf, 0);
 }
@@ -89,15 +90,7 @@ int mips_cpu_gdb_write_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 env->active_tc.gpr[n] = tmp;
 return sizeof(target_ulong);
 }
-if (env->CP0_Config1 & (1 << CP0C1_FP)
-&& n >= 38 && n < 72) {
-if (n < 70) {
-if (env->CP0_Status & (1 << CP0St_FR)) {
-env->active_fpu.fpr[n - 38].d = tmp;
-} else {
-env->active_fpu.fpr[n - 38].w[FP_ENDIAN_IDX] = tmp;
-}
-}
+if (env->CP0_Config1 & (1 << CP0C1_FP) && n >= 38 && n < 72) {
 switch (n) {
 case 70:
 env->active_fpu.fcr31 = tmp & 0xFF83;
@@ -107,6 +100,13 @@ int mips_cpu_gdb_write_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 case 71:
 /* FIR is read-only.  Ignore writes.  */
 break;
+default:
+if (env->CP0_Status & (1 << CP0St_FR)) {
+env->active_fpu.fpr[n - 38].d = tmp;
+} else {
+env->active_fpu.fpr[n - 38].w[FP_ENDIAN_IDX] = tmp;
+}
+break;
 }
 return sizeof(target_ulong);
 }
-- 
2.1.0




[Qemu-devel] [PULL 17/30] target-mips: Output CP0.Config2-5 in the register dump

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Include CP0.Config2 through CP0.Config5 registers in the register dump
produced with the `info registers' monitor command.  Align vertically
with the registers already output.

Signed-off-by: Maciej W. Rozycki 
Signed-off-by: Leon Alrae 
---
 target-mips/translate.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 1a275bf..70da66f 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -19264,6 +19264,10 @@ void mips_cpu_dump_state(CPUState *cs, FILE *f, 
fprintf_function cpu_fprintf,
 env->CP0_Status, env->CP0_Cause, env->CP0_EPC);
 cpu_fprintf(f, "Config0 0x%08x Config1 0x%08x LLAddr 0x" TARGET_FMT_lx 
"\n",
 env->CP0_Config0, env->CP0_Config1, env->lladdr);
+cpu_fprintf(f, "Config2 0x%08x Config3 0x%08x\n",
+env->CP0_Config2, env->CP0_Config3);
+cpu_fprintf(f, "Config4 0x%08x Config5 0x%08x\n",
+env->CP0_Config4, env->CP0_Config5);
 if (env->hflags & MIPS_HFLAG_FPU)
 fpu_dump_state(env, f, cpu_fprintf, flags);
 #if defined(TARGET_MIPS64) && defined(MIPS_DEBUG_SIGN_EXTENSIONS)
-- 
2.1.0




[Qemu-devel] [PULL 11/30] target-mips: Remove unused `FLOAT_OP' macro

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Remove the `FLOAT_OP' macro, unused since commit
b6d96beda3a6cbf20a2d04a609eff78adebd8859 [Use temporary registers for
the MIPS FPU emulation.].

Signed-off-by: Maciej W. Rozycki 
Signed-off-by: Leon Alrae 
---
 target-mips/op_helper.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 5a9f207..f547801 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -3140,8 +3140,6 @@ uint64_t helper_float_rsqrt1_ps(CPUMIPSState *env, 
uint64_t fdt0)
 return ((uint64_t)fsth2 << 32) | fst2;
 }
 
-#define FLOAT_OP(name, p) void helper_float_##name##_##p(CPUMIPSState *env)
-
 /* binary operations */
 #define FLOAT_BINOP(name)  \
 uint64_t helper_float_ ## name ## _d(CPUMIPSState *env,\
-- 
2.1.0




[Qemu-devel] [PULL 03/30] target-mips: Add 5KEc and 5KEf MIPS64r2 processors

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Add the 5KEc and 5KEf processors from MIPS Technologies that are the
original implementation of the MIPS64r2 ISA.

Silicon for these processors has never been taped out and no soft cores
were released even.  They do exist though, a CP0.PRId value has been
assigned and experimental RTLs produced at the time the MIPS64r2 ISA has
been finalized.  The settings introduced here faithfully reproduce that
hardware.

As far the implementation goes these processors are the same as the 5Kc
and the 5Kf CPUs respectively, except implementing the MIPS64r2 rather
than the original MIPS64 instruction set.  There must have been some
updates to the CP0 architecture as mandated by the ISA, such as the
addition of the EBase register, although I am not sure about the exact
details, no documentation has ever been produced for these processors.
The remaining parts of the microarchitecture, in particular the
pipeline, stayed unchanged.  Or to put it another way, the difference
between a 5K and a 5KE CPU corresponds to one between a 4K and a 4KE
CPU, except for the 64-bit rather than 32-bit ISA.

Signed-off-by: Maciej W. Rozycki 
Signed-off-by: Leon Alrae 
---
 target-mips/translate_init.c | 45 
 1 file changed, 45 insertions(+)

diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
index 148b394..607f1c8 100644
--- a/target-mips/translate_init.c
+++ b/target-mips/translate_init.c
@@ -520,6 +520,51 @@ static const mips_def_t mips_defs[] =
 .mmu_type = MMU_TYPE_R4000,
 },
 {
+.name = "5KEc",
+.CP0_PRid = 0x00018900,
+.CP0_Config0 = MIPS_CONFIG0 | (0x1 << CP0C0_AR) | (0x2 << CP0C0_AT) |
+   (MMU_TYPE_R4000 << CP0C0_MT),
+.CP0_Config1 = MIPS_CONFIG1 | (31 << CP0C1_MMU) |
+   (1 << CP0C1_IS) | (4 << CP0C1_IL) | (1 << CP0C1_IA) |
+   (1 << CP0C1_DS) | (4 << CP0C1_DL) | (1 << CP0C1_DA) |
+   (1 << CP0C1_PC) | (1 << CP0C1_WR) | (1 << CP0C1_EP),
+.CP0_Config2 = MIPS_CONFIG2,
+.CP0_Config3 = MIPS_CONFIG3,
+.CP0_LLAddr_rw_bitmask = 0,
+.CP0_LLAddr_shift = 4,
+.SYNCI_Step = 32,
+.CCRes = 2,
+.CP0_Status_rw_bitmask = 0x32F8,
+.SEGBITS = 42,
+.PABITS = 36,
+.insn_flags = CPU_MIPS64R2,
+.mmu_type = MMU_TYPE_R4000,
+},
+{
+.name = "5KEf",
+.CP0_PRid = 0x00018900,
+.CP0_Config0 = MIPS_CONFIG0 | (0x1 << CP0C0_AR) | (0x2 << CP0C0_AT) |
+   (MMU_TYPE_R4000 << CP0C0_MT),
+.CP0_Config1 = MIPS_CONFIG1 | (1 << CP0C1_FP) | (31 << CP0C1_MMU) |
+   (1 << CP0C1_IS) | (4 << CP0C1_IL) | (1 << CP0C1_IA) |
+   (1 << CP0C1_DS) | (4 << CP0C1_DL) | (1 << CP0C1_DA) |
+   (1 << CP0C1_PC) | (1 << CP0C1_WR) | (1 << CP0C1_EP),
+.CP0_Config2 = MIPS_CONFIG2,
+.CP0_Config3 = MIPS_CONFIG3,
+.CP0_LLAddr_rw_bitmask = 0,
+.CP0_LLAddr_shift = 4,
+.SYNCI_Step = 32,
+.CCRes = 2,
+.CP0_Status_rw_bitmask = 0x36F8,
+.CP1_fcr0 = (1 << FCR0_F64) | (1 << FCR0_L) | (1 << FCR0_W) |
+(1 << FCR0_D) | (1 << FCR0_S) |
+(0x89 << FCR0_PRID) | (0x0 << FCR0_REV),
+.SEGBITS = 42,
+.PABITS = 36,
+.insn_flags = CPU_MIPS64R2,
+.mmu_type = MMU_TYPE_R4000,
+},
+{
 /* A generic CPU supporting MIPS64 Release 6 ISA.
FIXME: Support IEEE 754-2008 FP and misaligned memory accesses.
   Eventually this should be replaced by a real CPU model. */
-- 
2.1.0




[Qemu-devel] [PULL 01/30] target-mips: Correct the handling of register #72 on writes

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Fix an off-by-one error in `mips_cpu_gdb_write_register' for register
matching how `mips_cpu_gdb_read_register' handles it.  This register
slot is a fake anyway, there's nothing in hardware that corresponds to
it.

Signed-off-by: Maciej W. Rozycki 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 target-mips/gdbstub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-mips/gdbstub.c b/target-mips/gdbstub.c
index f65fec2..7e3a604 100644
--- a/target-mips/gdbstub.c
+++ b/target-mips/gdbstub.c
@@ -90,7 +90,7 @@ int mips_cpu_gdb_write_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 return sizeof(target_ulong);
 }
 if (env->CP0_Config1 & (1 << CP0C1_FP)
-&& n >= 38 && n < 73) {
+&& n >= 38 && n < 72) {
 if (n < 70) {
 if (env->CP0_Status & (1 << CP0St_FR)) {
 env->active_fpu.fpr[n - 38].d = tmp;
-- 
2.1.0




[Qemu-devel] [PULL 10/30] target-mips: Make `helper_float_cvtw_s' consistent with the remaining helpers

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Move the call to `update_fcr31' in `helper_float_cvtw_s' after the
exception flag check, for consistency with the remaining helpers that do
it last too.

Signed-off-by: Maciej W. Rozycki 
Signed-off-by: Leon Alrae 
---
 target-mips/op_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 638c9f9..5a9f207 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -2659,11 +2659,11 @@ uint32_t helper_float_cvtw_s(CPUMIPSState *env, 
uint32_t fst0)
 uint32_t wt2;
 
 wt2 = float32_to_int32(fst0, &env->active_fpu.fp_status);
-update_fcr31(env, GETPC());
 if (get_float_exception_flags(&env->active_fpu.fp_status)
 & (float_flag_invalid | float_flag_overflow)) {
 wt2 = FP_TO_INT32_OVERFLOW;
 }
+update_fcr31(env, GETPC());
 return wt2;
 }
 
-- 
2.1.0




[Qemu-devel] [PULL 09/30] target-mips: Fix formatting in `decode_opc'

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

Signed-off-by: Maciej W. Rozycki 
Signed-off-by: Leon Alrae 
---
 target-mips/translate.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 643214a..a5a5ca4 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -18340,7 +18340,7 @@ static void gen_msa(CPUMIPSState *env, DisasContext 
*ctx)
 
 }
 
-static void decode_opc (CPUMIPSState *env, DisasContext *ctx)
+static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
 {
 int32_t offset;
 int rs, rt, rd, sa;
@@ -18506,7 +18506,8 @@ static void decode_opc (CPUMIPSState *env, DisasContext 
*ctx)
 save_cpu_state(ctx, 1);
 gen_helper_di(t0, cpu_env);
 gen_store_gpr(t0, rt);
-/* Stop translation as we may have switched the execution 
mode */
+/* Stop translation as we may have switched
+   the execution mode.  */
 ctx->bstate = BS_STOP;
 break;
 case OPC_EI:
@@ -18514,7 +18515,8 @@ static void decode_opc (CPUMIPSState *env, DisasContext 
*ctx)
 save_cpu_state(ctx, 1);
 gen_helper_ei(t0, cpu_env);
 gen_store_gpr(t0, rt);
-/* Stop translation as we may have switched the execution 
mode */
+/* Stop translation as we may have switched
+   the execution mode.  */
 ctx->bstate = BS_STOP;
 break;
 default:/* Invalid */
@@ -18780,8 +18782,9 @@ static void decode_opc (CPUMIPSState *env, DisasContext 
*ctx)
 gen_r6_cmp_d(ctx, ctx->opcode & 0x1f, rt, rd, sa);
 break;
 default:
-gen_farith(ctx, ctx->opcode & FOP(0x3f, 0x1f), rt, rd, sa,
-   (imm >> 8) & 0x7);
+gen_farith(ctx, ctx->opcode & FOP(0x3f, 0x1f),
+   rt, rd, sa, (imm >> 8) & 0x7);
+
 break;
 }
 } else {
-- 
2.1.0




[Qemu-devel] [PULL 02/30] target-mips: Make CP1.FIR read-only here too

2014-12-16 Thread Leon Alrae
From: "Maciej W. Rozycki" 

CP1.FIR is read-only in hardware so gdbstub must respect it.  We already
respect it for CTC1 instructions, so do it here too.

Signed-off-by: Maciej W. Rozycki 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 target-mips/gdbstub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-mips/gdbstub.c b/target-mips/gdbstub.c
index 7e3a604..e86df0e 100644
--- a/target-mips/gdbstub.c
+++ b/target-mips/gdbstub.c
@@ -105,7 +105,7 @@ int mips_cpu_gdb_write_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 RESTORE_ROUNDING_MODE;
 break;
 case 71:
-env->active_fpu.fcr0 = tmp;
+/* FIR is read-only.  Ignore writes.  */
 break;
 }
 return sizeof(target_ulong);
-- 
2.1.0




[Qemu-devel] [PULL 00/30] target-mips queue

2014-12-16 Thread Leon Alrae
Hi,

Here is first MIPS pull request for 2.3.

Thanks,
Leon

Cc: Peter Maydell 
Cc: Aurelien Jarno 

The following changes since commit dfa9c2a0f4d0a0c8b2c1449ecdbb1297427e1560:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging 
(2014-12-15 16:43:42 +)

are available in the git repository at:

  git://github.com/lalrae/qemu.git tags/mips-20141216

for you to fetch changes up to d4fa5354a246a1c6cb538a5d8ebcc21206d502fb:

  target-mips: remove excp_names[] from linux-user as it is unused (2014-12-16 
12:45:20 +)


MIPS patches 2014-12-16

Changes:
* number of bug fixes and minor improvements mainly in microMIPS, mips16
  and gdbstub
* make 5KEf default CPU in 64-bit linux-user
* cleanups


Leon Alrae (4):
  target-mips: convert single case switch into if statement
  disas/mips: remove unused mips_msa_control_names_numeric[32]
  disas/mips: disable unused mips16_to_32_reg_map[]
  target-mips: remove excp_names[] from linux-user as it is unused

Maciej W. Rozycki (26):
  target-mips: Correct the handling of register #72 on writes
  target-mips: Make CP1.FIR read-only here too
  target-mips: Add 5KEc and 5KEf MIPS64r2 processors
  target-mips: Make CP0.Config4 and CP0.Config5 registers signed
  target-mips: Add M14K and M14Kc MIPS32r2 microMIPS processors
  target-mips: Enable vectored interrupt support for the 74Kf CPU
  target-mips: Fix formatting in `decode_extended_mips16_opc'
  target-mips: Fix formatting in `mips_defs'
  target-mips: Fix formatting in `decode_opc'
  target-mips: Make `helper_float_cvtw_s' consistent with the remaining 
helpers
  target-mips: Remove unused `FLOAT_OP' macro
  target-mips: Restore the order of helpers
  target-mips: Correct MIPS16/microMIPS branch size calculation
  target-mips: Correct the handling of writes to CP0.Status for MIPSr6
  target-mips: Correct the writes to Status and Cause registers via gdbstub
  target-mips: Fix the 64-bit case for microMIPS MOVE16 and MOVEP
  target-mips: Output CP0.Config2-5 in the register dump
  target-mips: Fix CP0.Config3.ISAOnExc write accesses
  target-mips: Tighten ISA level checks
  target-mips: Correct 32-bit address space wrapping
  target-mips: gdbstub: Clean up FPU register handling
  target-mips: Also apply the CP0.Status mask to MTTC0
  linux-user: Use the 5KEf processor for 64-bit emulation
  target-mips: Add missing calls to synchronise SoftFloat status
  target-mips: Use local float status pointer across MSA macros
  target-mips: Fix DisasContext's ulri member initialization

 disas/mips.c |  10 +-
 linux-user/main.c|   2 +-
 target-mips/cpu.h| 124 -
 target-mips/gdbstub.c|  56 +++---
 target-mips/helper.c |  17 +-
 target-mips/helper.h |   1 +
 target-mips/msa_helper.c |  69 ---
 target-mips/op_helper.c  | 433 ++-
 target-mips/translate.c  | 170 ++---
 target-mips/translate_init.c | 128 +++--
 translate-all.c  |   2 +-
 11 files changed, 620 insertions(+), 392 deletions(-)



Re: [Qemu-devel] [PATCH v4 1/8] fw_cfg: max access size and region size are the same for MMIO data reg

2014-12-16 Thread Paolo Bonzini


On 16/12/2014 20:00, Laszlo Ersek wrote:
> Yes.
> 
> The root of this question is what each of
> 
> enum device_endian {
> DEVICE_NATIVE_ENDIAN,
> DEVICE_BIG_ENDIAN,
> DEVICE_LITTLE_ENDIAN,
> };

Actually, I think the root of the answer :) is that fw_cfg_read (and
thus fw_cfg_data_mem_read) is not idempotent.  The split/compose stuff
accesses the bytes at offsets 8,9,10,11,12,13,14,15 and composes them
according to the endianness.

In the case of fw_cfg it just retrieves 8 bytes, but in the case of host
big endian it reads them in the "wrong" order for some reason (sorry, I
haven't looked at this thoroughly).


So the solution is:

1) make fw_cfg_data_mem_ops DEVICE_LITTLE_ENDIAN

2) make fw_cfg_data_mem_read and fw_cfg_data_mem_write call fw_cfg_read
and fw_cfg_write SIZE times and build up a value from the lowest byte up.

Paolo



Re: [Qemu-devel] [PATCH v4 1/8] fw_cfg: max access size and region size are the same for MMIO data reg

2014-12-16 Thread Laszlo Ersek
On 12/16/14 14:48, Andrew Jones wrote:
> On Fri, Dec 12, 2014 at 04:58:45PM +0100, Laszlo Ersek wrote:
>> Make it clear that the maximum access size to the MMIO data register
>> determines the full size of the memory region.
>>
>> Currently the max access size is 1. Ensure that if a larger size were
>> used in "fw_cfg_data_mem_ops.valid.max_access_size", the memory
>> subsystem would split the access to byte-sized accesses internally,
>> in increasing address order.
>>
>> fw_cfg_data_mem_read() and fw_cfg_data_mem_write() don't care about
>> "address" or "size"; they just call the sequential fw_cfg_read() and
>> fw_cfg_write() functions, correspondingly. Therefore the automatic
>> splitting is just right. (The endianness of "fw_cfg_data_mem_ops" is
>> native.)
>
> This 'is native' caught my eye. Laszlo's
> Documentation/devicetree/bindings/arm/fw-cfg.txt patch states that the
> selector register is LE and
>
> "
> The data register allows accesses with 8, 16, 32 and 64-bit width.
> Accesses larger than a byte are interpreted as arrays, bundled
> together only for better performance. The bytes constituting such a
> word, in increasing address order, correspond to the bytes that would
> have been transferred by byte-wide accesses in chronological order.
> "
>
> I chatted with Laszlo to make sure the host-is-BE case was considered.
> It looks like there may be an issue there that Laszlo promised to look
> into. Laszlo had already noticed that the selector was defined as
> native in qemu as well, but should be LE. Now that we support > 1 byte
> reads and writes from the data port, then maybe we should look into
> changing that as well.

Yes.

The root of this question is what each of

enum device_endian {
DEVICE_NATIVE_ENDIAN,
DEVICE_BIG_ENDIAN,
DEVICE_LITTLE_ENDIAN,
};

means.

Consider the following call tree, which implements the splitting /
combining of an MMIO read:

memory_region_dispatch_read() [memory.c]
  memory_region_dispatch_read1()
access_with_adjusted_size()
  memory_region_big_endian()
  for each byte in the wide access:
memory_region_read_accessor()
  fw_cfg_data_mem_read() [hw/nvram/fw_cfg.c]
fw_cfg_read()
  adjust_endianness()
memory_region_wrong_endianness()
bswapXX()

The function access_with_adjusted_size() always iterates over the MMIO
address range in incrementing address order. However, it can calculate
the shift count for memory_region_read_accessor() in two ways.

When memory_region_big_endian() returns "true", the shift count
decreases as the MMIO address increases.

When memory_region_big_endian() returns "false", the shift count
increases as the MMIO address increases.

In memory_region_read_accessor(), the shift count is used for a logical
(ie. numeric) bitwise left shift (<<). That operator works with numeric
values and hides (ie. accounts for) host endianness.

Let's consider
- an array of two bytes, [0xaa, 0xbb],
- a uint16_t access made from the guest,
- and all twelve possible cases.

In the table below, the "host", "guest" and "device_endian" columns are
input variables. The columns to the right are calculated / derived
values. The arrows above the columns show the data dependencies.

After memory_region_dispatch_read1() constructs the value that is
visible in the "host value" column, it is passed to adjust_endianness().
If memory_region_wrong_endianness() returns "true", then the host value
is byte-swapped. The result is then passed to the guest.

  
+---+--+
  | 
  |  |
+ 
--+-+
   |  |
| | |   
  |   |  |
  +--+-+
  |   |  |
  | | | || |
  |   |  |
  |   +---+---+  ++  | |  
++---+  |  |
  |   | | |   | | |  ||  | |  | 
  ||   |  |  |
  |   | | |   | | v  |v  | v  | 
  v|   v  |  v
 #  host  guest  device_endian  memory_region_big_endian()  host value  host 
repr.memory_region_wrong_endianness()  guest repr.   guest value
--    -  -  --  --  
  --

Re: [Qemu-devel] [PATCH v4 3/8] fw_cfg: introduce the "data_memwidth" property

2014-12-16 Thread Laszlo Ersek
On 12/16/14 18:20, Alexander Graf wrote:
> On 12/16/14 18:10, Peter Maydell wrote:
>> On 16 December 2014 at 16:59, Laszlo Ersek  wrote:
>>> To elaborate on the above -- the fw_cfg device appears to be
>>> undestructible at the moment. It has no unrealize callback. If it were
>>> destructible, then the above leak would be the smallest of concerns --
>>> it doesn't unmap nor destroy the memory regions that implement the
>>> various registers.
>>>
>>> So, I think the above is not an actual leak, because the result of
>>> g_memdup() can never become unreferenced.
>> True, and we have a lot of device that are in this same
>> category of "can't ever be destroyed". However it is setting
>> up a minor beartrap for ourselves in future if we have
>> allocations which aren't tracked via a field in the device's
>> state structure, because the obvious future implementation of
>> destruction for a device is "just free/destroy everything
>> that is in the state struct".
>>
>> NB: I think what Alex had in mind with his option (2) was
>> just to have a "MemoryRegionOps ops;" field in the state
>> struct, and then use "s->ops = data_mem_ops;" rather than
>> the memdup. That retains the use of the static field for
>> the non-variable-width case, it's just that instead of
>> allocating off the heap for the var-width setup we use
>> an inline lump of memory in a struct we're already allocing.
>>
>> I don't think I care very much about this, but Alex's
>> suggestion 2 is slightly nicer I guess. Adding a whole
>> unrealize callback is definitely vastly overkill.
> 
> Yeah, it's exactly what I meant. Sorry for not being as clear. By moving
> the dynamically created struct into the device struct we're just making
> the whole allocation flow easier.
> 
> But if this is the only nitpick, there's no need for a respin just for
> that.

Okay, I understand it now. Thanks.

Laszlo




Re: [Qemu-devel] [PATCH v4 00/15] target-arm: Add CPU security extension enablement

2014-12-16 Thread Peter Maydell
On 15 December 2014 at 23:09, Greg Bellows  wrote:
> This patchset adds functionality for enabling the ARM CPU security extensions.
> At this time, the only machines supported are Versatile Express and the QEMU
> ARM virtual machines both with Cortex A9 & A15.
>
> The patchset establishes the default security state along with adding
> overriding controls of the state.  Booting with the "-kernel" QEMU command 
> line
> option will start by default in non-secure state with EL3 support disabled.
> Booting with the "-bios" QEMU command line option will default to
> secure state with EL3 features enabled.  An added "secure" machine property
> may be set to either 'on' or 'off' to override this default behavior.  For
> example, the below command line syntax would disable security extensions...
>
> aarch64-softmmu/qemu-system-aarch64
> -machine type=vexpress-a15,secure=off -kernel ...
>
> In order to add the machine specific 'secure' property, the vexpress machine
> object creation functionality needed to be updated.  The existing QEMU machine
> mechanism was replaced with proper type, class, and instance usage.

Thanks; applied all (including the dependencies) to target-arm.next.

-- PMM



Re: [Qemu-devel] [PATCH v2 1/5] linux-aio: queue requests that cannot be submitted

2014-12-16 Thread Paolo Bonzini


On 16/12/2014 14:10, Kevin Wolf wrote:
> Am 16.12.2014 um 12:28 hat Paolo Bonzini geschrieben:
>>
>>
>> On 16/12/2014 12:07, Kevin Wolf wrote:
>>> Am 11.12.2014 um 14:52 hat Paolo Bonzini geschrieben:
 Keep a queue of requests that were not submitted; pass them to
 the kernel when a completion is reported, unless the queue is
 plugged.

 The array of iocbs is rebuilt every time from scratch.  This
 avoids keeping the iocbs array and list synchronized.

 Signed-off-by: Paolo Bonzini 
>>>
>>> Just found out that in qemu-img bench, this patch seems to cost about
>>> 5-8% for me.
>>
>> What execution?  Queue depth=1?
> 
> My usual one:
> 
> $ ./qemu-img bench -t none -c 1000 -n /dev/loop0
> Sending 1000 requests, 4096 bytes each, 64 in parallel

I could reproduce this very well on a random OS image that I had around.
 This is raw over XFS over dm-crypt, and the image is about 75% sparse
(8.2G used over 35G).  I only get 1-2%, but still it's visible.

However I can hardly reproduce it when using a partition directly:

 oldnew
mean 9.9565 9.9636  (+0.07%)
stddev   0.0405 0.0537
min  9.871  9.867
median   9.973  9.971
max  10.01  10.053
count20 20

I haven't tried removing layers (e.g. fully-allocated XFS image without
dm-crypt).

Paolo



[Qemu-devel] Can we make monitor commands identify BDS / BB by name consistently? (was: Review of monitor commands identifying BDS / BB by name)

2014-12-16 Thread Markus Armbruster
Conscious design decision: Backend (BB) and node (BDS) names share a
common name space.

Enables a convenience feature: when a command needs a node, we accept
either kind of name, and a backend name is resolved to its root node.

Should not be confused with a command that can work either on a backend
or on a node.  There, a backend name resolves to the backend, not its
root node.  Can't point to an example offhand.

Let's concentrate on the "command needs a node" case.

As we saw in my review of monitor commands, we have two different
conventions there.

* Single name

  Within BlockdevOptions objects (used by blockdev-add), we use a single
  string member, with a name that explains its role.  Actually, the
  member is an anonymous union of string and BlockdevOptions.

  Example: a BlockdevOptionsGenericFormat object (used for format "raw"
  and others) has a member @file that may name a backend or a node.

  Example: a BlockdevOptionsQcow2 object (used for "qcow2"), has a
  member @file as above, and a member @backing that may again name a
  backend or a node.

* Pair of names

  Elsewhere, command argument objects have a pair of optional members,
  of which exactly one must be present.  One of them must name a
  backend, the other must name a node.  The former is commonly called
  @device, the latter @node-name.

  Example: block_passwd parameters @device and @node-name.

I'd very much like some consistency here.

As Kevin pointed out, you can't easily change BlockdevOptions to the
"pair of names" convention, because an anonymous union can have only one
object member, and that's taken by BlockdevOptions.  If you want us to
adopt the "pair of names" convention, you need to come up with a way to
use it with BlockdevOptions.

I want us to adopt the "single name" convention instead.  Therefore, I
need to come up with a way to use it with the command argument objects
that currently use "pair of names".  The problems there are
compatibility and discoverability.

Four ways come to mind:

1. Extend @node-name to accept backend names, deprecate @device

   @node-name becomes mandatory except in deprecated usage.
   Nevertheless, it remains optional in the schema, which is confusing.

   For discovery, you first have to try whether the command accepts
   parameter @node-name.  If no, you have a QEMU predating node names,
   and you need to use @device.  If yes, you need to try whether the
   command accepts a backend name as argument for @node-name.  Involves
   defining a backend.  Awkward.

2. Extend @device to accept node names, deprecate @node-name

   @device becomes mandatory except in deprecated usage.  Nevertheless,
   it remains optional in the schema, which is confusing.

   We're stuck with a bad parameter name: @device.

   For discovery, you need to try whether the command accepts a node
   name as argument for @device.  Involves defining a node.  Almost as
   awkward.

3. Add a new parameter, deprecate both old ones

   The new parameter is mandatory except in deprecated usage.
   Nevertheless, it's optional in the schema, which is confusing.

   Discovery needs to check which of the parameters the command accepts.
   Less awkward.

4. Add a new command, deprecate the old one

   Quick search for commands to deprecate: block_passwd, block_resize,
   blockdev-snapshot-sync.  Not really bad.

   Discovery needs to check query-commands for the new command.  Easy.

Any objections to #4?



Re: [Qemu-devel] [PATCH v4 3/8] fw_cfg: introduce the "data_memwidth" property

2014-12-16 Thread Alexander Graf

On 12/16/14 18:10, Peter Maydell wrote:

On 16 December 2014 at 16:59, Laszlo Ersek  wrote:

To elaborate on the above -- the fw_cfg device appears to be
undestructible at the moment. It has no unrealize callback. If it were
destructible, then the above leak would be the smallest of concerns --
it doesn't unmap nor destroy the memory regions that implement the
various registers.

So, I think the above is not an actual leak, because the result of
g_memdup() can never become unreferenced.

True, and we have a lot of device that are in this same
category of "can't ever be destroyed". However it is setting
up a minor beartrap for ourselves in future if we have
allocations which aren't tracked via a field in the device's
state structure, because the obvious future implementation of
destruction for a device is "just free/destroy everything
that is in the state struct".

NB: I think what Alex had in mind with his option (2) was
just to have a "MemoryRegionOps ops;" field in the state
struct, and then use "s->ops = data_mem_ops;" rather than
the memdup. That retains the use of the static field for
the non-variable-width case, it's just that instead of
allocating off the heap for the var-width setup we use
an inline lump of memory in a struct we're already allocing.

I don't think I care very much about this, but Alex's
suggestion 2 is slightly nicer I guess. Adding a whole
unrealize callback is definitely vastly overkill.


Yeah, it's exactly what I meant. Sorry for not being as clear. By moving 
the dynamically created struct into the device struct we're just making 
the whole allocation flow easier.


But if this is the only nitpick, there's no need for a respin just for that.


Alex




Re: [Qemu-devel] [PATCH v4 3/8] fw_cfg: introduce the "data_memwidth" property

2014-12-16 Thread Peter Maydell
On 16 December 2014 at 16:59, Laszlo Ersek  wrote:
> To elaborate on the above -- the fw_cfg device appears to be
> undestructible at the moment. It has no unrealize callback. If it were
> destructible, then the above leak would be the smallest of concerns --
> it doesn't unmap nor destroy the memory regions that implement the
> various registers.
>
> So, I think the above is not an actual leak, because the result of
> g_memdup() can never become unreferenced.

True, and we have a lot of device that are in this same
category of "can't ever be destroyed". However it is setting
up a minor beartrap for ourselves in future if we have
allocations which aren't tracked via a field in the device's
state structure, because the obvious future implementation of
destruction for a device is "just free/destroy everything
that is in the state struct".

NB: I think what Alex had in mind with his option (2) was
just to have a "MemoryRegionOps ops;" field in the state
struct, and then use "s->ops = data_mem_ops;" rather than
the memdup. That retains the use of the static field for
the non-variable-width case, it's just that instead of
allocating off the heap for the var-width setup we use
an inline lump of memory in a struct we're already allocing.

I don't think I care very much about this, but Alex's
suggestion 2 is slightly nicer I guess. Adding a whole
unrealize callback is definitely vastly overkill.

-- PMM



Re: [Qemu-devel] [PATCH v4 0/5] Geometry and blocksize detection for backing devices.

2014-12-16 Thread Markus Armbruster
Ekaterina Tumanova  writes:

> Updates:
> v2 -> v3:
> 1. Fix comments
> 2. Fix error codes to -ENOTSUP.
> 3. Reduce LOC in probe_logical_blocksize.
> 4. Adjust #ifdef - #else logic in couple of places.
> 5. Rebased.
>
> I hope that I addressed all the comments from the last round of review.
> If you think it's ok now, can you please give me your explicit Review-by.
> (I would really appreciate if you could give me some feedback this week,
> because I'm leaving for vacation next week)

Down to a commit message needing improvement, a few comment typos, and
one harmless buglet.  If you fix those, you can add my R-by.



Re: [Qemu-devel] [PATCH v4 3/5] block: Add driver methods to probe blocksizes and geometry

2014-12-16 Thread Markus Armbruster
Ekaterina Tumanova  writes:

> Introduce driver methods of defining disk blocksizes (physical and
> logical) and hard drive geometry.
> Methods are only implemented for "host_device". For "raw" devices
> driver calls child's method.
>
> For now geometry detection will only work for DASD devices. To check
> that a local check_for_dasd function was introduced. It calls BIODASDINFO2
> ioctl and returns its rc.
>
> Blocksizes detection function will probe sizes for DASD devices and
> set default for other devices.
>
> Signed-off-by: Ekaterina Tumanova 
> ---
>  block/raw-posix.c | 97 
> +++
>  block/raw_bsd.c   | 14 
>  2 files changed, 111 insertions(+)
>
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 38172ca..e1e7b29 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -56,6 +56,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #ifndef FS_NOCOW_FL
>  #define FS_NOCOW_FL 0x0080 /* Do not cow file */
>  #endif
> @@ -90,6 +91,10 @@
>  #include 
>  #endif
>  
> +#ifdef __s390__
> +#include 
> +#endif
> +
>  //#define DEBUG_FLOPPY
>  
>  //#define DEBUG_BLOCK
> @@ -238,6 +243,23 @@ static int probe_logical_blocksize(int fd, unsigned int 
> *sector_size)
>  #undef SECTOR_SIZE
>  }
>  
> +/**
> + * Get physical block size of @fd.
> + * On success, store it in @blk_size and return 0.
> + * On failure, return -errno.
> + */
> +static int probe_physical_blocksize(int fd, unsigned int *blk_size)
> +{
> +#ifdef BLKPBSZGET
> +if (ioctl(fd, BLKPBSZGET, blk_size) < 0) {
> +return -errno;
> +}
> +return 0;
> +#else
> +return -ENOTSUP;
> +#endif
> +}
> +
>  static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
>  {
>  BDRVRawState *s = bs->opaque;
> @@ -660,6 +682,79 @@ static void raw_refresh_limits(BlockDriverState *bs, 
> Error **errp)
>  bs->bl.opt_mem_alignment = s->buf_align;
>  }
>  
> +static int check_for_dasd(int fd)
> +{
> +#ifdef BIODASDINFO2
> +struct dasd_information2_t info = {0};
> +
> +return ioctl(fd, BIODASDINFO2, &info);
> +#else
> +return -ENOTSUP;
> +#endif
> +}

This function is confused about its return value: 0/-1 vs. 0/-errno.
Please return -1 instead of -ENOTSUP, or replace it by an is_dasd()
returning bool.

[...]



Re: [Qemu-devel] [PATCH v4 3/8] fw_cfg: introduce the "data_memwidth" property

2014-12-16 Thread Laszlo Ersek
On 12/16/14 13:42, Laszlo Ersek wrote:
> On 12/16/14 13:06, Alexander Graf wrote:
>>
>>
>> On 12.12.14 16:58, Laszlo Ersek wrote:
>>> The "data_memwidth" property is capable of changing the maximum valid
>>> access size to the MMIO data register, and (corresponding to the previous
>>> patch) resizes the memory region similarly, at device realization time.
>>>
>>> (Because "data_iomem" is configured and installed dynamically now, we must
>>> delay those steps to the realize callback.)
>>>
>>> The default value of "data_memwidth" is set so that we don't yet diverge
>>> from "fw_cfg_data_mem_ops".
>>>
>>> Most of the fw_cfg users will stick with the default, and for them we
>>> should continue using the statically allocated "fw_cfg_data_mem_ops". This
>>> is beneficial for debugging because gdb can resolve pointers referencing
>>> static objects to the names of those objects.
>>>
>>> Signed-off-by: Laszlo Ersek 
>>> ---
>>>
>>> Notes:
>>> v4:
>>> - reject I/O port combining if data register is wider than 1 byte
>>>   [Peter]
>>> 
>>> v3:
>>> - new in v3 [Drew Jones]
>>>
>>>  hw/nvram/fw_cfg.c | 24 ++--
>>>  1 file changed, 18 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
>>> index eb0ad83..0947136 100644
>>> --- a/hw/nvram/fw_cfg.c
>>> +++ b/hw/nvram/fw_cfg.c
>>> @@ -50,8 +50,9 @@ struct FWCfgState {
>>>  /*< public >*/
>>>  
>>>  MemoryRegion ctl_iomem, data_iomem, comb_iomem;
>>>  uint32_t ctl_iobase, data_iobase;
>>> +uint32_t data_memwidth;
>>>  FWCfgEntry entries[2][FW_CFG_MAX_ENTRY];
>>>  FWCfgFiles *files;
>>>  uint16_t cur_entry;
>>>  uint32_t cur_offset;
>>> @@ -569,8 +570,10 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t 
>>> data_port,
>>>  
>>>  dev = qdev_create(NULL, TYPE_FW_CFG);
>>>  qdev_prop_set_uint32(dev, "ctl_iobase", ctl_port);
>>>  qdev_prop_set_uint32(dev, "data_iobase", data_port);
>>> +qdev_prop_set_uint32(dev, "data_memwidth",
>>> + fw_cfg_data_mem_ops.valid.max_access_size);
>>>  d = SYS_BUS_DEVICE(dev);
>>>  
>>>  s = FW_CFG(dev);
>>>  
>>> @@ -607,12 +610,8 @@ static void fw_cfg_initfn(Object *obj)
>>>  
>>>  memory_region_init_io(&s->ctl_iomem, OBJECT(s), &fw_cfg_ctl_mem_ops, s,
>>>"fwcfg.ctl", FW_CFG_SIZE);
>>>  sysbus_init_mmio(sbd, &s->ctl_iomem);
>>> -memory_region_init_io(&s->data_iomem, OBJECT(s), &fw_cfg_data_mem_ops, 
>>> s,
>>> -  "fwcfg.data",
>>> -  fw_cfg_data_mem_ops.valid.max_access_size);
>>> -sysbus_init_mmio(sbd, &s->data_iomem);
>>>  /* In case ctl and data overlap: */
>>>  memory_region_init_io(&s->comb_iomem, OBJECT(s), &fw_cfg_comb_mem_ops, 
>>> s,
>>>"fwcfg", FW_CFG_SIZE);
>>>  }
>>> @@ -620,19 +619,31 @@ static void fw_cfg_initfn(Object *obj)
>>>  static void fw_cfg_realize(DeviceState *dev, Error **errp)
>>>  {
>>>  FWCfgState *s = FW_CFG(dev);
>>>  SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>> +const MemoryRegionOps *data_mem_ops = &fw_cfg_data_mem_ops;
>>>  uint32_t ctl_io_last;
>>>  uint32_t data_io_end;
>>>  
>>> +if (s->data_memwidth > data_mem_ops->valid.max_access_size) {
>>> +MemoryRegionOps *ops;
>>> +
>>> +ops = g_memdup(data_mem_ops, sizeof(*data_mem_ops));
>>
>> Hrm, this memory will leak if the device gets destroyed after realize,
>> right?
> 
> How do you destroy the fw_cfg device after it is successfully realized?
> I wouldn't introduce such a blatant leak out of oversight.
> 
>> I see 2 options around this:
>>
>>   1) Free it on destruction
> 
> Does that mean an unrealize callback?
> 
>>   2) Add the RegionOps as field into FWCfgState. Then it gets allocated
>> and free'd automatically
>>
>> Option 2 is easier (and more failure proof) but will waste a few bytes
>> of ram for data_memwidth=1 users. I don't think we need to bother about
>> the few bytes and rather go with safety :).
> 
> I wanted to keep the static ops object for the common user, because it
> is very convenient when debugging in gdb -- the address is automatically
> resolved to the name of the static object. I guess I can do (1) (if that
> means an unrealize callback).

To elaborate on the above -- the fw_cfg device appears to be
undestructible at the moment. It has no unrealize callback. If it were
destructible, then the above leak would be the smallest of concerns --
it doesn't unmap nor destroy the memory regions that implement the
various registers.

So, I think the above is not an actual leak, because the result of
g_memdup() can never become unreferenced.

Thanks,
Laszlo




Re: [Qemu-devel] [PATCH v4 2/5] raw-posix: Factor block size detection out of raw_probe_alignment()

2014-12-16 Thread Markus Armbruster
Ekaterina Tumanova  writes:

> Put it in new probe_logical_blocksize().
>
> Signed-off-by: Ekaterina Tumanova 
> ---
>  block/raw-posix.c | 41 -
>  1 file changed, 24 insertions(+), 17 deletions(-)
>
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index e51293a..38172ca 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -217,11 +217,31 @@ static int raw_normalize_devicepath(const char 
> **filename)
>  }
>  #endif
>  
> +/*
> + * Get logical block size via ioctl. On success return 0. Otherwise -errno.
> + */
> +static int probe_logical_blocksize(int fd, unsigned int *sector_size)
> +{
> +#if defined(BLKSSZGET)
> +#  define SECTOR_SIZE BLKSSZGET
> +#elif defined(DKIOCGETBLOCKSIZE)
> +#  define SECTOR_SIZE DKIOCGETBLOCKSIZE
> +#elif defined(DIOCGSECTORSIZE)
> +#  define SECTOR_SIZE DIOCGSECTORSIZE
> +#else
> +return -ENOTSUP
> +#endif
> +if (ioctl(fd, SECTOR_SIZE, sector_size) < 0) {
> +return -errno;
> +}
> +return 0;
> +#undef SECTOR_SIZE
> +}
> +
>  static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
>  {
>  BDRVRawState *s = bs->opaque;
>  char *buf;
> -unsigned int sector_size;
>  
>  /* For /dev/sg devices the alignment is not really used.
> With buffered I/O, we don't have any restrictions. */
> @@ -231,25 +251,12 @@ static void raw_probe_alignment(BlockDriverState *bs, 
> int fd, Error **errp)
>  return;
>  }
>  
> -/* Try a few ioctls to get the right size */
>  bs->request_alignment = 0;
>  s->buf_align = 0;
> -
> -#ifdef BLKSSZGET
> -if (ioctl(fd, BLKSSZGET, §or_size) >= 0) {
> -bs->request_alignment = sector_size;
> -}
> -#endif
> -#ifdef DKIOCGETBLOCKSIZE
> -if (ioctl(fd, DKIOCGETBLOCKSIZE, §or_size) >= 0) {
> -bs->request_alignment = sector_size;
> +/* Let's try to use the logical blocksize for the alignment. */
> +if (probe_logical_blocksize(fd, &bs->request_alignment) < 0) {
> +bs->request_alignment = 0;
>  }
> -#endif
> -#ifdef DIOCGSECTORSIZE
> -if (ioctl(fd, DIOCGSECTORSIZE, §or_size) >= 0) {
> -bs->request_alignment = sector_size;
> -}
> -#endif
>  #ifdef CONFIG_XFS
>  if (s->is_xfs) {
>  struct dioattr da;

New in v4: this isn't just factoring out code, you also change it!

Before: try BLKSSZGET (if defined), DKIOCGETBLOCKSIZE (if defined),
DIOCGSECTORSIZE (if defined) in order until.  The last one to succeed
wins.  If none succeeds, assume 0.

After: use BLKSSZGET if defined, else try DKIOCGETBLOCKSIZE if defined,
else try DIOCGSECTORSIZE if defined, else assume 0.

Your change may well be an improvement.  But your commit message should
explain *why* it's an improvement.

I guess I'd do a separate patch for that, but since the code patch seems
obvious enough as it is, you may just improve its commit message and
call it a day.



Re: [Qemu-devel] [PULL v2] Migration pull for 2.3

2014-12-16 Thread Peter Maydell
On 16 December 2014 at 12:26, Amit Shah  wrote:
> The following changes since commit 54600752a1dd67844c2cf3c467db562c39499838:
>
>   Merge remote-tracking branch 'remotes/rth/tags/x86-next-20141214' into 
> staging (2014-12-15 11:11:52 +)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/virt/qemu/amit/migration.git tags/for-2.3-2
>
> for you to fetch changes up to 44a1f94684eeaa6e312ea2d77ede266a81d31210:
>
>   MAINTAINERS: Update for migrated migration code (2014-12-16 17:47:36 +0530)
>
> 
> Migration pull for 2.3.  Mostly moving the code to the migration/
> directory, and updating MAINTAINERS.
>
> I've also folded my other MAINTAINERS update patches into this, as
> they're small by themselves.
>
> 

Applied, thanks.

-- PMM



Re: [Qemu-devel] [PATCH v4 1/5] block: add bdrv functions for geometry and blocksize

2014-12-16 Thread Markus Armbruster
Ekaterina Tumanova  writes:

> Add driver functions for geometry and blocksize detection
>
> Signed-off-by: Ekaterina Tumanova 
> Reviewed-by: Thomas Huth 
> ---
>  block.c   | 34 ++
>  include/block/block.h | 13 +
>  include/block/block_int.h | 15 +++
>  3 files changed, 62 insertions(+)
>
> diff --git a/block.c b/block.c
> index 4165d42..93409f5 100644
> --- a/block.c
> +++ b/block.c
> @@ -548,6 +548,40 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error 
> **errp)
>  }
>  }
>  
> +/**
> + * Get @bs's logical and physical block size, store them in @bsz.
> + * @bs must not be empty.
> + */
> +void bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
> +{
> +BlockDriver *drv = bs->drv;
> +
> +assert(drv != NULL);
> +if (drv->bdrv_probe_blocksizes &&
> +!drv->bdrv_probe_blocksizes(bs, bsz)) {
> +return;
> +}
> +bsz->log = BDRV_SECTOR_SIZE;
> +bsz->phys = BDRV_SECTOR_SIZE;
> +}
> +
> +/**
> + * Try to get @bs's geometry (cyls, heads, sectos)

s/sectos/sectors/

Since we're changing this comment anyway: I like my function contracts
to state restrictions prominently, and therefore prefer "@bs must not be
empty" on its own line, right here.

> + * On success, store them in @geo struct and return 0.
> + * On failure return -errno. @bs must not be empty.
> + */
> +int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
> +{
> +BlockDriver *drv = bs->drv;
> +
> +assert(drv != NULL);
> +if (drv->bdrv_probe_geometry) {
> +return drv->bdrv_probe_geometry(bs, geo);
> +}
> +
> +return -ENOTSUP;
> +}
> +
>  /*
>   * Create a uniquely-named empty temporary file.
>   * Return 0 upon success, otherwise a negative errno value.
> diff --git a/include/block/block.h b/include/block/block.h
> index 6e7275d..14ac3b1 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -60,6 +60,17 @@ typedef enum {
>  BDRV_REQ_MAY_UNMAP= 0x4,
>  } BdrvRequestFlags;
>  
> +typedef struct BlockSizes {
> +uint32_t phys;
> +uint32_t log;
> +} BlockSizes;
> +
> +typedef struct HDGeometry {
> +uint32_t heads;
> +uint32_t sectors;
> +uint32_t cylinders;
> +} HDGeometry;
> +
>  #define BDRV_O_RDWR0x0002
>  #define BDRV_O_SNAPSHOT0x0008 /* open the file read only and save writes 
> in a snapshot */
>  #define BDRV_O_TEMPORARY   0x0010 /* delete the file after use */
> @@ -539,6 +550,8 @@ AioContext *bdrv_get_aio_context(BlockDriverState *bs);
>   * the old #AioContext is not executing.
>   */
>  void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context);
> +void bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz);
> +int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo);
>  
>  void bdrv_io_plug(BlockDriverState *bs);
>  void bdrv_io_unplug(BlockDriverState *bs);
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 06a21dd..c2c5f0e 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -273,6 +273,21 @@ struct BlockDriver {
>  void (*bdrv_io_unplug)(BlockDriverState *bs);
>  void (*bdrv_flush_io_queue)(BlockDriverState *bs);
>  
> +/**
> + * Try to get @bs's logical and physical block size.
> + * On success, store them in @bsz and return zero.
> + * On failure, return negative errno.
> + */
> +int (*bdrv_probe_blocksizes)(BlockDriverState *bs, BlockSizes *bsz);
> +/**
> + * Try to get @bs's geometry (cyls, heads, sectos)

s/sectos/sectors/

> + * On success, store them in @geo and return 0.
> + * On failure return -errno.
> + * Only drivers that want to override guest geometry implement this
> + * callback; see hd_geometry_guess().
> + */
> +int (*bdrv_probe_geometry)(BlockDriverState *bs, HDGeometry *geo);
> +
>  QLIST_ENTRY(BlockDriver) list;
>  };



Re: [Qemu-devel] [RFC PATCH] target-arm: protect cpu_exclusive_*.

2014-12-16 Thread Peter Maydell
On 16 December 2014 at 09:13,   wrote:
> From: KONRAD Frederic 
>
> This adds a lock to avoid multiple exclusive access at the same time in case 
> of
> TCG multithread.

This feels to me like it's not really possible to review on
its own, since you can't see how it fits into the design of
the rest of the multithreading support.

The other approach here rather than having a pile of mutexes
in the target-* code would be to have TCG IR support for
"begin critical section"/"end critical section". Then you
could have the main loop ensure that no other CPU is running
at the same time as the critical-section code. (linux-user
already has an ad-hoc implementation of this for the
exclusives.)

-- PMM



Re: [Qemu-devel] [PATCH 07/16] parallels: change copyright information in the image header

2014-12-16 Thread Denis V. Lunev

On 15/12/14 14:06, Kevin Wolf wrote:

Am 15.12.2014 um 09:27 hat Denis V. Lunev geschrieben:

Signed-off-by: Denis V. Lunev 
Acked-by: Roman Kagan 
Reviewed-by: Jeff Cody 
CC: Kevin Wolf 
CC: Stefan Hajnoczi 
---
  block/parallels.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/block/parallels.c b/block/parallels.c
index fedb009..5fcede8 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -2,8 +2,10 @@
   * Block driver for Parallels disk image format
   *
   * Copyright (c) 2007 Alex Beregszaszi
+ * Copyright (c) 2014 Denis V. Lunev 
   *
- * This code is based on comparing different disk images created by Parallels.
+ * This code was originally based on comparing different disk images created
+ * by Parallels.

On what is it based now? (Link to a spec would be great, if it exists.)

I was wrong :)

Pls find disk descriptor XML XSD scheme under this link

https://github.com/CloudServer/parallels-sdk/blob/master/Sources/XmlModel/Schema/DiskDescriptor.xsd



Re: [Qemu-devel] [RFC PATCH 2/2] qga: implement qmp_guest_get_os_version for windows

2014-12-16 Thread Eric Blake
On 12/16/2014 04:48 AM, Yan Vugenfirer wrote:
>> +
>> +if (si.wProcessorArchitecture == PROCESSOR_ARCHITECTURE_AMD64 ||
>> +si.wProcessorArchitecture == PROCESSOR_ARCHITECTURE_IA64) {
> 
> If one of the motivations is to update drivers on the guest - those should be 
> treated as deferent architectures.
> Why not return string as well (x64, x86, IA64, ARM)? 

The architecture should already be known by the host (after all, the
flavor of qemu running the guest should tell you what architecture the
guest would report).

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [RFC PATCH 0/2] qga: add guest-get-os-version for windows

2014-12-16 Thread Eric Blake
On 12/16/2014 12:30 AM, zhanghailiang wrote:
> Hi,
> 
> This patch series add a new guest command 'guest-get-os-version'.
> It is now only available for windows guest.

Why not also supply it for Linux guests?  uname() is your friend; it
should be fairly easy to wire up.

> 
> It will return guest's OS version name and type, like bellow:
> '{"return":{"name":"Microsoft Windows Server 2012 R2","type":64}}'
> 
> Sometimes we need to know guest's OS version info.
> (Actually, we need this info when we update guest's applications and drivers 
> in our project.)

Have you looked into libguestfs' ability to get this information from an
(offline) guest image?  I'm not rejecting this command, but trying to
make sure that it is not duplicating something that can already be done.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [RFC PATCH 1/2] qga: Introduce guest-get-os-version command with stubs

2014-12-16 Thread Eric Blake
On 12/16/2014 12:30 AM, zhanghailiang wrote:
> Signed-off-by: zhanghailiang 
> ---

Might be nice to show an example (intended) usage of the new command in
the commit message.

> +++ b/qga/qapi-schema.json
> @@ -738,3 +738,29 @@
>  ##
>  { 'command': 'guest-get-fsinfo',
>'returns': ['GuestFilesystemInfo'] }
> +##

Blank line between commands.

> +# @GuestOSVersion:
> +#
> +# @name: system version.
> +#
> +# @:type: 64-bit or 32-bit.

s/@:/@/

'type' feels like the wrong name for word-width.  Maybe 'word-width' is
a better name?

> +#
> +# Since: 2.3
> +##
> +{ 'type': 'GuestOSVersion',
> +  'data': {'name': 'str', 'type': 'int'} }

'name' feels a bit vague; it looks like you are intending to use the
string as a free-form text field, where the guest can supply arbitrary
strings.  Maybe name it 'info' instead? Is it worth being any stricter,
such as having actual enums of known values?  On the other hand, tying
to known enums means we have to update qemu-ga every time a new guest
gains support for running the agent, while free-form string leaves us a
bit more flexible.  Or maybe you want both, as in:

{ 'enum': 'GuestOSFamily', 'data': ['Windows', 'Linux', 'other'] }
{ 'type': 'GuestOSVersion',
  'data': { 'info': 'str', 'family': 'GuestOSFamily',
'word-width': 'int' } }

> +
> +##
> +# @guest-get-os-version:
> +#
> +# Get the guest's operating system version and bit.

s/bit/word width/

> +#
> +# This is a read-only operation.
> +#
> +# Returns: version
> +#
> +# Since: 2.3
> +##
> +{ 'command': 'guest-get-os-version',
> +  'returns': 'GuestOSVersion' }

with my suggestions, a usage might be:
=> { "execute": "guest-get-os-version" }
<= { "return": { "family": "Windows",
 "info": "Microsoft Windows Server 2012 R2",
 "word-width": 64 } }

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH v2 7/8] memory: API to allocate resizeable RAM MR

2014-12-16 Thread Michael S. Tsirkin
Add API to allocate resizeable RAM MR.

This looks just like regular RAM generally, but
has a special property that only a portion of it
(used_length) is actually used, and migrated.

This used_length size can change across reboots.

Follow up patches will change used_length for such blocks at migration,
making it easier to extend devices using such RAM (notably ACPI,
but in the future thinkably other ROMs) without breaking migration
compatibility or wasting ROM (guest) memory.

Device is notified on resize, so it can adjust if necessary.

Note: nothing prevents making all RAM resizeable in this way.
However, reviewers felt that only enabling this selectively will
make some class of errors easier to detect.

Signed-off-by: Michael S. Tsirkin 
---
 include/exec/memory.h | 24 
 memory.c  | 17 +
 2 files changed, 41 insertions(+)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 0882221..0cd96b1 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -321,6 +321,30 @@ void memory_region_init_ram(MemoryRegion *mr,
 uint64_t size,
 Error **errp);
 
+/**
+ * memory_region_init_resizeable_ram:  Initialize memory region with resizeable
+ * RAM.  Accesses into the region will
+ * modify memory directly.  Only an initial
+ * portion of this RAM is actually used.
+ * The used size can change across reboots.
+ *
+ * @mr: the #MemoryRegion to be initialized.
+ * @owner: the object that tracks the region's reference count
+ * @name: the name of the region.
+ * @size: used size of the region.
+ * @max_size: max size of the region.
+ * @resized: callback to notify owner about used size change.
+ * @errp: pointer to Error*, to store an error if it happens.
+ */
+void memory_region_init_resizeable_ram(MemoryRegion *mr,
+   struct Object *owner,
+   const char *name,
+   uint64_t size,
+   uint64_t max_size,
+   void (*resized)(const char*,
+   uint64_t length,
+   void *host),
+   Error **errp);
 #ifdef __linux__
 /**
  * memory_region_init_ram_from_file:  Initialize RAM memory region with a
diff --git a/memory.c b/memory.c
index 618470b..c343bf3 100644
--- a/memory.c
+++ b/memory.c
@@ -1152,6 +1152,23 @@ void memory_region_init_ram(MemoryRegion *mr,
 mr->ram_addr = qemu_ram_alloc(size, mr, errp);
 }
 
+void memory_region_init_resizeable_ram(MemoryRegion *mr,
+   Object *owner,
+   const char *name,
+   uint64_t size,
+   uint64_t max_size,
+   void (*resized)(const char*,
+   uint64_t length,
+   void *host),
+   Error **errp)
+{
+memory_region_init(mr, owner, name, size);
+mr->ram = true;
+mr->terminates = true;
+mr->destructor = memory_region_destructor_ram;
+mr->ram_addr = qemu_ram_alloc_resizeable(size, max_size, resized, mr, 
errp);
+}
+
 #ifdef __linux__
 void memory_region_init_ram_from_file(MemoryRegion *mr,
   struct Object *owner,
-- 
MST




[Qemu-devel] [PATCH v2 8/8] acpi-build: make ROMs RAM blocks resizeable

2014-12-16 Thread Michael S. Tsirkin
Use resizeable ram API so we can painlessly extend ROMs in the
future.  Note: migration is not affected, as we are
not actually changing the used length for RAM, which
is the part that's migrated.

Use this in acpi: reserve x16 more RAM space.

Signed-off-by: Michael S. Tsirkin 
---
 hw/lm32/lm32_hwsetup.h |  3 ++-
 include/hw/loader.h|  4 ++--
 hw/core/loader.c   | 18 ++
 hw/i386/acpi-build.c   | 19 ++-
 4 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/hw/lm32/lm32_hwsetup.h b/hw/lm32/lm32_hwsetup.h
index 9fd5e69..838754d 100644
--- a/hw/lm32/lm32_hwsetup.h
+++ b/hw/lm32/lm32_hwsetup.h
@@ -73,7 +73,8 @@ static inline void hwsetup_free(HWSetup *hw)
 static inline void hwsetup_create_rom(HWSetup *hw,
 hwaddr base)
 {
-rom_add_blob("hwsetup", hw->data, TARGET_PAGE_SIZE, base, NULL, NULL, 
NULL);
+rom_add_blob("hwsetup", hw->data, TARGET_PAGE_SIZE,
+ TARGET_PAGE_SIZE, base, NULL, NULL, NULL);
 }
 
 static inline void hwsetup_add_u8(HWSetup *hw, uint8_t u)
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 6481639..1d76108 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -60,7 +60,7 @@ int rom_add_file(const char *file, const char *fw_dir,
  hwaddr addr, int32_t bootindex,
  bool option_rom);
 ram_addr_t rom_add_blob(const char *name, const void *blob, size_t len,
-   hwaddr addr, const char *fw_file_name,
+   size_t max_len, hwaddr addr, const char *fw_file_name,
FWCfgReadCallback fw_callback, void *callback_opaque);
 int rom_add_elf_program(const char *name, void *data, size_t datasize,
 size_t romsize, hwaddr addr);
@@ -74,7 +74,7 @@ void do_info_roms(Monitor *mon, const QDict *qdict);
 #define rom_add_file_fixed(_f, _a, _i)  \
 rom_add_file(_f, NULL, _a, _i, false)
 #define rom_add_blob_fixed(_f, _b, _l, _a)  \
-rom_add_blob(_f, _b, _l, _a, NULL, NULL, NULL)
+rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL)
 
 #define PC_ROM_MIN_VGA 0xc
 #define PC_ROM_MIN_OPTION  0xc8000
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 7527fd3..d3f8501 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -712,12 +712,22 @@ static void rom_insert(Rom *rom)
 QTAILQ_INSERT_TAIL(&roms, rom, next);
 }
 
+static void fw_cfg_resized(const char *id, uint64_t length, void *host)
+{
+if (fw_cfg) {
+fw_cfg_modify_file(fw_cfg, id + strlen("/rom@"), host, length);
+}
+}
+
 static void *rom_set_mr(Rom *rom, Object *owner, const char *name)
 {
 void *data;
 
 rom->mr = g_malloc(sizeof(*rom->mr));
-memory_region_init_ram(rom->mr, owner, name, rom->datasize, &error_abort);
+memory_region_init_resizeable_ram(rom->mr, owner, name,
+  rom->datasize, rom->romsize,
+  fw_cfg_resized,
+  &error_abort);
 memory_region_set_readonly(rom->mr, true);
 vmstate_register_ram_global(rom->mr);
 
@@ -812,7 +822,7 @@ err:
 }
 
 ram_addr_t rom_add_blob(const char *name, const void *blob, size_t len,
-   hwaddr addr, const char *fw_file_name,
+   size_t max_len, hwaddr addr, const char *fw_file_name,
FWCfgReadCallback fw_callback, void *callback_opaque)
 {
 Rom *rom;
@@ -821,7 +831,7 @@ ram_addr_t rom_add_blob(const char *name, const void *blob, 
size_t len,
 rom   = g_malloc0(sizeof(*rom));
 rom->name = g_strdup(name);
 rom->addr = addr;
-rom->romsize  = len;
+rom->romsize  = max_len ? max_len : len;
 rom->datasize = len;
 rom->data = g_malloc0(rom->datasize);
 memcpy(rom->data, blob, len);
@@ -841,7 +851,7 @@ ram_addr_t rom_add_blob(const char *name, const void *blob, 
size_t len,
 
 fw_cfg_add_file_callback(fw_cfg, fw_file_name,
  fw_callback, callback_opaque,
- data, rom->romsize);
+ data, rom->datasize);
 }
 return ret;
 }
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b37a397..dabb7a0 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -68,6 +68,9 @@
 
 #define ACPI_BUILD_TABLE_SIZE 0x2
 
+/* Reserve RAM space for tables: add another order of magnitude. */
+#define ACPI_BUILD_TABLE_MAX_SIZE 0x20
+
 typedef struct AcpiCpuInfo {
 DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
 } AcpiCpuInfo;
@@ -1712,6 +1715,11 @@ static void acpi_build_update(void *build_opaque, 
uint32_t offset)
 acpi_build(build_state->guest_info, &tables);
 
 assert(acpi_data_len(tables.table_data) == build_state->table_size);
+
+/* Make sure RAM size is correct - in case it got changed by migration */
+qemu_ram_resize(build_state->table_ram, build_state->table_size,
+   

[Qemu-devel] [PATCH v2 4/8] exec: split length -> used_length/max_length

2014-12-16 Thread Michael S. Tsirkin
This patch allows us to distinguish between two
length values for each block:
max_length - length of memory block that was allocated
used_length - length of block used by QEMU/guest

Currently, we set used_length - max_length, unconditionally.
Follow-up patches allow used_length <= max_length.

Signed-off-by: Michael S. Tsirkin 
---
 include/exec/cpu-all.h |  3 ++-
 arch_init.c| 19 +-
 exec.c | 52 +++---
 3 files changed, 40 insertions(+), 34 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 62f5581..6f2130e 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -303,7 +303,8 @@ typedef struct RAMBlock {
 struct MemoryRegion *mr;
 uint8_t *host;
 ram_addr_t offset;
-ram_addr_t length;
+ram_addr_t used_length;
+ram_addr_t max_length;
 uint32_t flags;
 char idstr[256];
 /* Reads can take either the iothread or the ramlist lock.
diff --git a/arch_init.c b/arch_init.c
index 7680d28..106f46e 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -522,7 +522,7 @@ static void migration_bitmap_sync(void)
 address_space_sync_dirty_bitmap(&address_space_memory);
 
 QTAILQ_FOREACH(block, &ram_list.blocks, next) {
-migration_bitmap_sync_range(block->mr->ram_addr, block->length);
+migration_bitmap_sync_range(block->mr->ram_addr, block->used_length);
 }
 trace_migration_bitmap_sync_end(migration_dirty_pages
 - num_dirty_pages_init);
@@ -668,7 +668,7 @@ static int ram_find_and_save_block(QEMUFile *f, bool 
last_stage)
 offset >= last_offset) {
 break;
 }
-if (offset >= block->length) {
+if (offset >= block->used_length) {
 offset = 0;
 block = QTAILQ_NEXT(block, next);
 if (!block) {
@@ -727,7 +727,7 @@ uint64_t ram_bytes_total(void)
 uint64_t total = 0;
 
 QTAILQ_FOREACH(block, &ram_list.blocks, next)
-total += block->length;
+total += block->used_length;
 
 return total;
 }
@@ -831,7 +831,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
 QTAILQ_FOREACH(block, &ram_list.blocks, next) {
 uint64_t block_pages;
 
-block_pages = block->length >> TARGET_PAGE_BITS;
+block_pages = block->used_length >> TARGET_PAGE_BITS;
 migration_dirty_pages += block_pages;
 }
 
@@ -844,7 +844,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
 QTAILQ_FOREACH(block, &ram_list.blocks, next) {
 qemu_put_byte(f, strlen(block->idstr));
 qemu_put_buffer(f, (uint8_t *)block->idstr, strlen(block->idstr));
-qemu_put_be64(f, block->length);
+qemu_put_be64(f, block->used_length);
 }
 
 qemu_mutex_unlock_ramlist();
@@ -1015,7 +1015,7 @@ static inline void *host_from_stream_offset(QEMUFile *f,
 uint8_t len;
 
 if (flags & RAM_SAVE_FLAG_CONTINUE) {
-if (!block || block->length <= offset) {
+if (!block || block->max_length <= offset) {
 error_report("Ack, bad migration stream!");
 return NULL;
 }
@@ -1028,7 +1028,8 @@ static inline void *host_from_stream_offset(QEMUFile *f,
 id[len] = 0;
 
 QTAILQ_FOREACH(block, &ram_list.blocks, next) {
-if (!strncmp(id, block->idstr, sizeof(id)) && block->length > offset) {
+if (!strncmp(id, block->idstr, sizeof(id)) &&
+block->max_length > offset) {
 return memory_region_get_ram_ptr(block->mr) + offset;
 }
 }
@@ -1085,10 +1086,10 @@ static int ram_load(QEMUFile *f, void *opaque, int 
version_id)
 
 QTAILQ_FOREACH(block, &ram_list.blocks, next) {
 if (!strncmp(id, block->idstr, sizeof(id))) {
-if (block->length != length) {
+if (block->used_length != length) {
 error_report("Length mismatch: %s: 0x" RAM_ADDR_FMT
  " in != 0x" RAM_ADDR_FMT, id, length,
- block->length);
+ block->used_length);
 ret =  -EINVAL;
 }
 break;
diff --git a/exec.c b/exec.c
index a89aa6c..b69216a 100644
--- a/exec.c
+++ b/exec.c
@@ -812,11 +812,11 @@ static RAMBlock *qemu_get_ram_block(ram_addr_t addr)
 
 /* The list is protected by the iothread lock here.  */
 block = ram_list.mru_block;
-if (block && addr - block->offset < block->length) {
+if (block && addr - block->offset < block->max_length) {
 goto found;
 }
 QTAILQ_FOREACH(block, &ram_list.blocks, next) {
-if (addr - block->offset < block->length) {
+if (addr - block->offset < block->max_length) {
 goto found;
 }
 }
@@ -1305,13 +1305,14 @@ static ram_addr_t ram_bloc

[Qemu-devel] [PATCH v2 5/8] exec: qemu_ram_alloc_resizeable, qemu_ram_resize

2014-12-16 Thread Michael S. Tsirkin
Add API to allocate "resizeable" RAM.
This looks just like regular RAM generally, but
has a special property that only a portion of it
(used_length) is actually used, and migrated.

This used_length size can change across reboots.

Follow up patches will change used_length for such blocks at migration,
making it easier to extend devices using such RAM (notably ACPI,
but in the future thinkably other ROMs) without breaking migration
compatibility or wasting ROM (guest) memory.

Device is notified on resize, so it can adjust if necessary.

qemu_ram_alloc_resizeable allocates this memory, qemu_ram_resize resizes
it.

Note: nothing prevents making all RAM resizeable in this way.
However, reviewers felt that only enabling this selectively will
make some class of errors easier to detect.

Signed-off-by: Michael S. Tsirkin 
---
 include/exec/cpu-all.h  |  9 --
 include/exec/ram_addr.h |  7 +
 exec.c  | 75 ++---
 3 files changed, 84 insertions(+), 7 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 6f2130e..7ced147 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -299,12 +299,15 @@ CPUArchState *cpu_copy(CPUArchState *env);
 
 /* memory API */
 
-typedef struct RAMBlock {
+typedef struct RAMBlock RAMBlock;
+
+struct RAMBlock {
 struct MemoryRegion *mr;
 uint8_t *host;
 ram_addr_t offset;
 ram_addr_t used_length;
 ram_addr_t max_length;
+void (*resized)(const char*, uint64_t length, void *host);
 uint32_t flags;
 char idstr[256];
 /* Reads can take either the iothread or the ramlist lock.
@@ -312,11 +315,11 @@ typedef struct RAMBlock {
  */
 QTAILQ_ENTRY(RAMBlock) next;
 int fd;
-} RAMBlock;
+};
 
 static inline void *ramblock_ptr(RAMBlock *block, ram_addr_t offset)
 {
-assert(offset < block->length);
+assert(offset < block->used_length);
 assert(block->host);
 return (char *)block->host + offset;
 }
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 254931c..ff558a4 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -28,12 +28,19 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, 
MemoryRegion *mr,
 ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
MemoryRegion *mr, Error **errp);
 ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error **errp);
+ram_addr_t qemu_ram_alloc_resizeable(ram_addr_t size, ram_addr_t max_size,
+ void (*resized)(const char*,
+ uint64_t length,
+ void *host),
+ MemoryRegion *mr, Error **errp);
 int qemu_get_ram_fd(ram_addr_t addr);
 void *qemu_get_ram_block_host_ptr(ram_addr_t addr);
 void *qemu_get_ram_ptr(ram_addr_t addr);
 void qemu_ram_free(ram_addr_t addr);
 void qemu_ram_free_from_ptr(ram_addr_t addr);
 
+int qemu_ram_resize(ram_addr_t base, ram_addr_t newsize, Error **errp);
+
 static inline bool cpu_physical_memory_get_dirty(ram_addr_t start,
  ram_addr_t length,
  unsigned client)
diff --git a/exec.c b/exec.c
index b69216a..2427319 100644
--- a/exec.c
+++ b/exec.c
@@ -75,6 +75,11 @@ static MemoryRegion io_mem_unassigned;
 /* RAM is mmap-ed with MAP_SHARED */
 #define RAM_SHARED (1 << 1)
 
+/* On-device RAM allocated with g_malloc: supports realloc,
+ * not accessible to vcpu on kvm.
+ */
+#define RAM_RESIZEABLE (1 << 2)
+
 #endif
 
 struct CPUTailQ cpus = QTAILQ_HEAD_INITIALIZER(cpus);
@@ -1186,7 +1191,7 @@ static ram_addr_t find_ram_offset(ram_addr_t size)
 QTAILQ_FOREACH(block, &ram_list.blocks, next) {
 ram_addr_t end, next = RAM_ADDR_MAX;
 
-end = block->offset + block->length;
+end = block->offset + block->max_length;
 
 QTAILQ_FOREACH(next_block, &ram_list.blocks, next) {
 if (next_block->offset >= end) {
@@ -1214,7 +1219,7 @@ ram_addr_t last_ram_offset(void)
 ram_addr_t last = 0;
 
 QTAILQ_FOREACH(block, &ram_list.blocks, next)
-last = MAX(last, block->offset + block->length);
+last = MAX(last, block->offset + block->max_length);
 
 return last;
 }
@@ -1296,6 +1301,42 @@ static int memory_try_enable_merging(void *addr, size_t 
len)
 return qemu_madvise(addr, len, QEMU_MADV_MERGEABLE);
 }
 
+int qemu_ram_resize(ram_addr_t base, ram_addr_t newsize, Error **errp)
+{
+RAMBlock *block = find_ram_block(base);
+
+assert(block);
+
+if (block->used_length == newsize) {
+return 0;
+}
+
+if (!(block->flags & RAM_RESIZEABLE)) {
+error_setg_errno(errp, EINVAL,
+ "Length mismatch: %s: 0x" RAM_ADDR_FMT
+ " in != 0x" RAM_ADDR_FMT, block->idstr,
+ news

[Qemu-devel] [PATCH v2 3/8] exec: cpu_physical_memory_set/clear_dirty_range

2014-12-16 Thread Michael S. Tsirkin
Make cpu_physical_memory_set/clear_dirty_range
behave symmetrically.

To clear range for a given client type only, add
cpu_physical_memory_clear_dirty_range_type.

Signed-off-by: Michael S. Tsirkin 
---
 include/exec/ram_addr.h | 15 ---
 exec.c  |  2 +-
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 18ec092..254931c 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -172,9 +172,9 @@ static inline void 
cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap,
 }
 #endif /* not _WIN32 */
 
-static inline void cpu_physical_memory_clear_dirty_range(ram_addr_t start,
- ram_addr_t length,
- unsigned client)
+static inline void cpu_physical_memory_clear_dirty_range_type(ram_addr_t start,
+  ram_addr_t 
length,
+  unsigned client)
 {
 unsigned long end, page;
 
@@ -184,11 +184,12 @@ static inline void 
cpu_physical_memory_clear_dirty_range(ram_addr_t start,
 bitmap_clear(ram_list.dirty_memory[client], page, end - page);
 }
 
-static inline void cpu_physical_memory_clear_dirty_range_nocode(ram_addr_t 
start,
-ram_addr_t 
length)
+static inline void cpu_physical_memory_clear_dirty_range(ram_addr_t start,
+ ram_addr_t length)
 {
-cpu_physical_memory_clear_dirty_range(start, length, 
DIRTY_MEMORY_MIGRATION);
-cpu_physical_memory_clear_dirty_range(start, length, DIRTY_MEMORY_VGA);
+cpu_physical_memory_clear_dirty_range_type(start, length, 
DIRTY_MEMORY_MIGRATION);
+cpu_physical_memory_clear_dirty_range_type(start, length, 
DIRTY_MEMORY_VGA);
+cpu_physical_memory_clear_dirty_range_type(start, length, 
DIRTY_MEMORY_CODE);
 }
 
 
diff --git a/exec.c b/exec.c
index 963481a..a89aa6c 100644
--- a/exec.c
+++ b/exec.c
@@ -850,7 +850,7 @@ void cpu_physical_memory_reset_dirty(ram_addr_t start, 
ram_addr_t length,
 {
 if (length == 0)
 return;
-cpu_physical_memory_clear_dirty_range(start, length, client);
+cpu_physical_memory_clear_dirty_range_type(start, length, client);
 
 if (tcg_enabled()) {
 tlb_reset_dirty_range_all(start, length);
-- 
MST




[Qemu-devel] [PATCH v2 6/8] arch_init: support resizing on incoming migration

2014-12-16 Thread Michael S. Tsirkin
If block used_length does not match, try to resize it.

Signed-off-by: Michael S. Tsirkin 
---
 arch_init.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 106f46e..cfedbf0 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1086,11 +1086,14 @@ static int ram_load(QEMUFile *f, void *opaque, int 
version_id)
 
 QTAILQ_FOREACH(block, &ram_list.blocks, next) {
 if (!strncmp(id, block->idstr, sizeof(id))) {
-if (block->used_length != length) {
-error_report("Length mismatch: %s: 0x" RAM_ADDR_FMT
- " in != 0x" RAM_ADDR_FMT, id, length,
- block->used_length);
-ret =  -EINVAL;
+if (length != block->used_length) {
+Error *local_err = NULL;
+
+ret = qemu_ram_resize(block->offset, length, 
&local_err);
+if (local_err) {
+error_report("%s", 
error_get_pretty(local_err));
+error_free(local_err);
+}
 }
 break;
 }
-- 
MST




  1   2   3   >