Re: [Qemu-devel] virtio-net: configurable TX queue size

[Wang, Wei W]

On 05/06/2017 04:37 AM, Michael S. Tsirkin wrote:
> On Fri, May 05, 2017 at 10:27:13AM +0800, Jason Wang wrote:
> >
> >
> > On 2017年05月04日 18:58, Wang, Wei W wrote:
> > > Hi,
> > >
> > > I want to re-open the discussion left long time ago:
> > > https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg06194.html
> > > , and discuss the possibility of changing the hardcoded (256) TX
> > > queue size to be configurable between 256 and 1024.
> >
> > Yes, I think we probably need this.
> >
> > >
> > > The reason to propose this request is that a severe issue of packet
> > > drops in TX direction was observed with the existing hardcoded 256
> > > queue size, which causes performance issues for packet drop
> > > sensitive guest applications that cannot use indirect descriptor
> > > tables. The issue goes away with 1K queue size.
> >
> > Do we need even more, what if we find 1K is even not sufficient in the
> > future? Modern nics has size up to ~8192.
> >
> > >
> > > The concern mentioned in the previous discussion (please check the
> > > link
> > > above) is that the number of chained descriptors would exceed
> > > UIO_MAXIOV (1024) supported by the Linux.
> >
> > We could try to address this limitation but probably need a new
> > feature bit to allow more than UIO_MAXIOV sgs.
> 
> I'd say we should split the queue size and the sg size.
> 

I think we can just split the iov size in the virtio-net backend,
that is, split the large iov[] into multiple iov[1024] to send to writev.

Best,
Wei

[Qemu-devel] [Bug 1689003] [NEW] USB passthrough should not fail if SET CONFIGURATION fails

[Manfred Härtel]

Public bug reported:

QEMU's USB passthrough was not working for my new smartphone.

While analyzing the problem, I found out that a SET CONFIGURATION
Request was NACKed by the USB device (probably because a SET
CONFIGURATION request was already sent from the host to the device).

So I wrote a simple program to fake a successful call to
libusb_set_configuration and did an LD_PRELOAD on this program before
starting qemu, and it worked.

Looking at QEMU's code in host-libusb.c, I can see that QEMU does not
try to claim the interface if its call to libusb_set_configuration
fails.

I think QEMU should try to claim the device anyway even if
libusb_set_configuration fails.

I did my tests against QEMU 2.6.2, but as I can see from the source
code, this problem should happen on all versions.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  USB passthrough should not fail if SET CONFIGURATION fails

Status in QEMU:
  New

Bug description:
  QEMU's USB passthrough was not working for my new smartphone.

  While analyzing the problem, I found out that a SET CONFIGURATION
  Request was NACKed by the USB device (probably because a SET
  CONFIGURATION request was already sent from the host to the device).

  So I wrote a simple program to fake a successful call to
  libusb_set_configuration and did an LD_PRELOAD on this program before
  starting qemu, and it worked.

  Looking at QEMU's code in host-libusb.c, I can see that QEMU does not
  try to claim the interface if its call to libusb_set_configuration
  fails.

  I think QEMU should try to claim the device anyway even if
  libusb_set_configuration fails.

  I did my tests against QEMU 2.6.2, but as I can see from the source
  code, this problem should happen on all versions.

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

[Qemu-devel] [Bug 795866] Re: pci passthrough doesn´t work

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  pci passthrough doesn´t work

Status in QEMU:
  Expired

Bug description:
  Hi all,

  I have some problems passing through a pci device to kvm guest.
  First I have to say that I´m using the latest kvm-kernel und qemu-kvm from 
git-tree (Date 11.06.2011).

  I want´t to passthrough this device to guest:

  lspci-output:

  02:00.0 Multimedia video controller: Micronas Semiconductor Holding AG
  Device 0720 (rev 01)

  So at first I have bind the driver to psi-stub:

  modprobe -r kvm-intel
  modprobe -r kvm
  echo "18c3 0720" > /sys/bus/pci/drivers/pci-stub/new_id
  echo :02:00.0  > /sys/bus/pci/devices/:02:00.0/driver/unbind
  echo :02:00.0  > /sys/bus/pci/drivers/pci-stub/bind
  modprobe kvm
  modprobe kvm-intel

  Then I have assigned device to guest:
  -device pci-assign,host=02:00.0

  When I start the guest. The device succesfully get´s an msi-IRQ on
  host-system:

  cat /proc/interrupt output:

   32:  0  0  0  0   PCI-MSI-edge
  kvm_assigned_msi_device

  
  On guest device is visibel:

  lspci output:
  00:04.0 Multimedia video controller: Micronas Semiconductor Holding AG Device 
0720 (rev 01)

  
  Sometimes the device (on guest) get´s an IRQ between 10-16:

  00:05.0 Multimedia video controller: Micronas Semiconductor Holding AG Device 
0720 (rev 01)
  Subsystem: Micronas Semiconductor Holding AG Device dd00
  Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx+
  Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- 
SERR-  Link[LNKA] -> GSI 11 (level, 
high) -> IRQ 11
  [   69.977909] ngene: Found Linux4Media cineS2 DVB-S2 Twin Tuner (v5)
  [   69.978962] ngene :00:05.0: setting latency timer to 64
  [   69.979118] ngene: Device version 1
  [   69.979129] ngene :00:05.0: firmware: requesting ngene_18.fw
  [   69.980884] ngene: Loading firmware file ngene_18.fw.
  [   71.981052] ngene: Command timeout cmd=01 prev=00
  [   71.981205] host_to_ngene (c000): 01 00 00 00 00 00 00 00
  [   71.981457] ngene_to_host (c100): 00 00 00 00 00 00 00 00
  [   71.981704] dev->hosttongene (ec902000): 01 00 00 00 00 00 00 00
  [   71.981963] dev->ngenetohost (ec902100): 00 00 00 00 00 00 00 00
  [   73.985111] ngene: Command timeout cmd=02 prev=00
  [   73.985415] host_to_ngene (c000): 02 04 00 d0 00 04 00 00
  [   73.985684] ngene_to_host (c100): 00 00 00 00 00 00 00 00
  [   73.985931] dev->hosttongene (ec902000): 02 04 00 d0 00 04 00 00
  [   73.986191] dev->ngenetohost (ec902100): 00 00 00 00 00 00 00 00
  [   73.986568] ngene :00:05.0: PCI INT A disabled
  [   73.986584] ngene: probe of :00:05.0 failed with error -1

  
  Sometimes the device (on guest) gets an msi-irq f. e. IRQ 29.
  Then kernel-modul (ngene) can succesfully load the driver and all works fine.

  
  Short to say:

  HOST  GUEST   STATUS
  MSI-IRQ   MSI-IRQ ALL FINE
  MSI-IRQ IOAPIC-IRQDOESN´t WORK

  with modinfo I had a look at the kernel-modul if there is way to force
  msi, but without success.

  But I think IRQ between (10-16) should also work because when I load the 
kernel-modul on host with IRQ (10-16)
  it works. (Device only get´s an MSI-IRQ If I start the vm to passthrough)

  Do anyone know where can be the problem?

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

Re: [Qemu-devel] [PATCH v10 3/6] virtio-balloon: VIRTIO_BALLOON_F_PAGE_CHUNKS

[Wang, Wei W]

On 05/06/2017 06:29 AM, Michael S. Tsirkin wrote:
> On Thu, May 04, 2017 at 04:50:12PM +0800, Wei Wang wrote:
> > Add a new feature, VIRTIO_BALLOON_F_PAGE_CHUNKS, which enables the
> > transfer of the ballooned (i.e. inflated/deflated) pages in chunks to
> > the host.
> >
> > The implementation of the previous virtio-balloon is not very
> > efficient, because the ballooned pages are transferred to the host one
> > by one. Here is the breakdown of the time in percentage spent on each
> > step of the balloon inflating process (inflating 7GB of an 8GB idle
> > guest).
> >
> > 1) allocating pages (6.5%)
> > 2) sending PFNs to host (68.3%)
> > 3) address translation (6.1%)
> > 4) madvise (19%)
> >
> > It takes about 4126ms for the inflating process to complete.
> > The above profiling shows that the bottlenecks are stage 2) and stage
> > 4).
> >
> > This patch optimizes step 2) by transferring pages to the host in
> > chunks. A chunk consists of guest physically continuous pages.
> > When the pages are packed into a chunk, they are converted into
> > balloon page size (4KB) pages. A chunk is offered to the host via a
> > base PFN (i.e. the start PFN of those physically continuous
> > pages) and the size (i.e. the total number of the 4KB balloon size
> > pages). A chunk is formatted as below:
> > 
> > | Base (52 bit)| Rsvd (12 bit) |
> > 
> > 
> > | Size (52 bit)| Rsvd (12 bit) |
> > 
> >
> > By doing so, step 4) can also be optimized by doing address
> > translation and madvise() in chunks rather than page by page.
> >
> > With this new feature, the above ballooning process takes ~590ms
> > resulting in an improvement of ~85%.
> >
> > TODO: optimize stage 1) by allocating/freeing a chunk of pages instead
> > of a single page each time.
> >
> > Signed-off-by: Wei Wang 
> > Signed-off-by: Liang Li 
> > Suggested-by: Michael S. Tsirkin 
> 
> 
> This is much cleaner, thanks. It might be even better to have wrappers that 
> put
> array and its size in a struct and manage that struct, but I won't require 
> this for
> submission.

OK, thanks. Would this be your suggestion:

struct virtio_balloon_page_struct { 
unsigned int page_bmap_num;
unsigned long *page_bmap[VIRTIO_BALLOON_PAGE_BMAP_MAX_NUM];
}

Best,
Wei

Re: [Qemu-devel] [PATCH v9 5/5] virtio-balloon: VIRTIO_BALLOON_F_MISC_VQ

[Wang, Wei W]

On 05/06/2017 06:21 AM, Michael S. Tsirkin wrote:
> On Thu, Apr 27, 2017 at 02:33:44PM +0800, Wei Wang wrote:
> > On 04/14/2017 01:08 AM, Michael S. Tsirkin wrote:
> > > On Thu, Apr 13, 2017 at 05:35:08PM +0800, Wei Wang wrote:
> > > > Add a new vq, miscq, to handle miscellaneous requests between the
> > > > device and the driver.
> > > >
> > > > This patch implemnts the
> VIRTIO_BALLOON_MISCQ_INQUIRE_UNUSED_PAGES
> > > implements
> > >
> > > > request sent from the device.
> > > Commands are sent from host and handled on guest.
> > > In fact how is this so different from stats?
> > > How about reusing the stats vq then? You can use one buffer for
> > > stats and one buffer for commands.
> > >
> >
> > The meaning of the two vqs is a little different. statq is used for
> > reporting statistics, while miscq is intended to be used to handle
> > miscellaneous requests from the guest or host
> 
> misc just means "anything goes". If you want it to mean "commands" name it so.

Ok, will change it.

> > (I think it can
> > also be used the other way around in the future when other new
> > features are added which need the guest to send requests and the host
> > to provide responses).
> >
> > I would prefer to have them separate, because:
> > If we plan to combine them, we need to put the previous statq related
> > implementation under miscq with a new command (I think we can't
> > combine them without using commands to distinguish the two features).
> 
> Right.

> > In this way, an old driver won't work with a new QEMU or a new driver
> > won't work with an old QEMU. Would this be considered as an issue
> > here?
> 
> Compatibility is and should always be handled using feature flags.  There's a
> feature flag for this, isn't it?

The negotiation of the existing feature flag, VIRTIO_BALLOON_F_STATS_VQ
only indicates the support of the old statq implementation. To move the statq
implementation under cmdq, I think we would need a new feature flag for the
new statq implementation:
#define VIRTIO_BALLOON_F_CMDQ_STATS  5

What do you think?

Best,
Wei

Re: [Qemu-devel] [virtio-dev] Re: [PATCH v9 2/5] virtio-balloon: VIRTIO_BALLOON_F_BALLOON_CHUNKS

[Wang, Wei W]

On 05/06/2017 06:26 AM, Michael S. Tsirkin wrote:
> On Thu, Apr 27, 2017 at 02:31:49PM +0800, Wei Wang wrote:
> > On 04/27/2017 07:20 AM, Michael S. Tsirkin wrote:
> > > On Wed, Apr 26, 2017 at 11:03:34AM +, Wang, Wei W wrote:
> > > > Hi Michael, could you please give some feedback?
> > > I'm sorry, I'm not sure feedback on what you are requesting.
> > Oh, just some trivial things (e.g. use a field in the header,
> > hdr->chunks to indicate the number of chunks in the payload) that
> > wasn't confirmed.
> >
> > I will prepare the new version with fixing the agreed issues, and we
> > can continue to discuss those parts if you still find them improper.
> >
> >
> > >
> > > The interface looks reasonable now, even though there's a way to
> > > make it even simpler if we can limit chunk size to 2G (in fact 4G -
> > > 1). Do you think we can live with this limitation?
> > Yes, I think we can. So, is it good to change to use the previous
> > 64-bit chunk format (52-bit base + 12-bit size)?
> 
> This isn't what I meant. virtio ring has descriptors with a 64 bit address 
> and 32 bit
> size.
> 
> If size < 4g is not a significant limitation, why not just use that to pass
> address/size in a standard s/g list, possibly using INDIRECT?

OK, I see your point, thanks. Post the two options here for an analysis:
Option1 (what we have now):
struct virtio_balloon_page_chunk {
__le64 chunk_num;
struct virtio_balloon_page_chunk_entry entry[];
};
Option2:
struct virtio_balloon_page_chunk {
__le64 chunk_num;
struct scatterlist entry[];
};

I don't have an issue to change it to Option2, but I would prefer Option1,
because I think there is no be obvious difference between the two options,
while Option1 appears to have little advantages here:
1) "struct virtio_balloon_page_chunk_entry" has smaller size than
"struct scatterlist", so the same size of allocated page chunk buffer
can hold more entry[] using Option1;
2) INDIRECT needs on demand kmalloc();
3) no 4G size limit;

What do you think?

Best,
Wei

[Qemu-devel] [PATCH v13 09/12] iotests: Add test 179 to cover write zeroes with unmap

[Eric Blake]

No tests were covering write zeroes with unmap.  Additionally,
I needed to prove that my previous patches for correct status
reporting and write zeroes optimizations actually had an impact.

The test works for cluster_size between 8k and 2M (for smaller
sizes, it fails because our allocation patterns are not contiguous
with small clusters - in part, the largest consecutive allocation
we tend to get is often bounded by the size covered by one L2
table).

Note that testing for zero clusters is tricky: 'qemu-io map'
reports whether data comes from the current layer of the image
(useful for sniffing out which regions of the file have
QCOW_OFLAG_ZERO) - but doesn't show which clusters have mappings;
while 'qemu-img map' sees "zero":true for both unallocated and
zero clusters for any qcow2 with no backing layer (so less useful
at detecting true zero clusters), but reliably shows mappings.
So we have to rely on both queries side-by-side at each point of
the test.

Signed-off-by: Eric Blake 
Reviewed-by: Max Reitz 

---
v13: drop final failing test of blkdebug
v12: probe the map in more places, to make test easier to follow
v11: reserved for blkdebug half of v10
v10: drop any changes to v2 files, rewrite test to work with updates
earlier in the series, add a blkdebug probe
v9: new patch
---
 tests/qemu-iotests/179 | 130 +
 tests/qemu-iotests/179.out | 156 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 287 insertions(+)
 create mode 100755 tests/qemu-iotests/179
 create mode 100644 tests/qemu-iotests/179.out

diff --git a/tests/qemu-iotests/179 b/tests/qemu-iotests/179
new file mode 100755
index 000..7bc8db8
--- /dev/null
+++ b/tests/qemu-iotests/179
@@ -0,0 +1,130 @@
+#!/bin/bash
+#
+# Test case for write zeroes with unmap
+#
+# Copyright (C) 2017 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=ebl...@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+status=1   # failure is the default!
+
+_cleanup()
+{
+   _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+# v2 images can't mark clusters as zero
+_unsupported_imgopts compat=0.10
+
+echo
+echo '=== Testing write zeroes with unmap ==='
+echo
+
+TEST_IMG="$TEST_IMG.base" _make_test_img 64M
+_make_test_img -b "$TEST_IMG.base"
+
+# Offsets chosen at or near 2M boundaries so test works at all cluster sizes
+# 8k and larger (smaller clusters fail due to non-contiguous allocations)
+
+# Aligned writes to unallocated cluster should not allocate mapping, but must
+# mark cluster as zero, whether or not unmap was requested
+$QEMU_IO -c "write -z -u 2M 2M" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -z 6M 2M" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "map" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IMG map --output=json "$TEST_IMG.base" | _filter_qemu_img_map
+
+# Unaligned writes need not allocate mapping if the cluster already reads
+# as zero, but must mark cluster as zero, whether or not unmap was requested
+$QEMU_IO -c "write -z -u 10485761 2097150" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -z 14680065 2097150" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "map" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IMG map --output=json "$TEST_IMG.base" | _filter_qemu_img_map
+
+# Requesting unmap of normal data must deallocate; omitting unmap should
+# preserve the mapping
+$QEMU_IO -c "write 18M 14M" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -z -u 20M 2M" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -z 24M 6M" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "map" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IMG map --output=json "$TEST_IMG.base" | _filter_qemu_img_map
+
+# Likewise when writing on already-mapped zero data
+$QEMU_IO -c "write -z -u 26M 2M" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -z 28M 2M" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "map" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IMG map --output=json "$TEST_IMG.base" | _filter_qemu_img_map
+
+# Writing on unmapped zeroes does not allocate
+$QEMU_IO -c "write -z 32M 8M" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c 

[Qemu-devel] [PATCH v13 10/12] qcow2: Optimize write zero of unaligned tail cluster

[Eric Blake]

We've already improved discards to operate efficiently on the tail
of an unaligned qcow2 image; it's time to make a similar improvement
to write zeroes.  The special case is only valid at the tail
cluster of a file, where we must recognize that any sectors beyond
the image end would implicitly read as zero, and therefore should
not penalize our logic for widening a partial cluster into writing
the whole cluster as zero.

However, note that for now, the special case of end-of-file is only
recognized if there is no backing file, or if the backing file has
the same length; that's because when the backing file is shorter
than the active layer, we don't have code in place to recognize
that reads of a sector unallocated at the top and beyond the backing
end-of-file are implicitly zero.  It's not much of a real loss,
because most people don't use images that aren't cluster-aligned,
or where the active layer is a different size than the backing
layer (especially where the difference falls within a single cluster).

Update test 154 to cover the new scenarios, using two images of
intentionally differing length.

While at it, fix the test to gracefully skip when run as
./check -qcow2 -o compat=0.10 154
since the older format lacks zero clusters already required earlier
in the test.

Signed-off-by: Eric Blake 
Reviewed-by: Max Reitz 

---
v13: fix typo that left pattern verification failure in test
v12: fix testsuite problems, document shortcoming of differing
v11: reserved for blkdebug half of v10
size of backing file

v10: rebase to better reporting of preallocated zero clusters
v9: new patch
---
 block/qcow2.c  |   7 ++
 tests/qemu-iotests/154 | 160 -
 tests/qemu-iotests/154.out | 128 
 3 files changed, 293 insertions(+), 2 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index dded5a0..3478bb6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2451,6 +2451,10 @@ static bool is_zero_sectors(BlockDriverState *bs, 
int64_t start,
 BlockDriverState *file;
 int64_t res;

+if (start + count > bs->total_sectors) {
+count = bs->total_sectors - start;
+}
+
 if (!count) {
 return true;
 }
@@ -2469,6 +2473,9 @@ static coroutine_fn int 
qcow2_co_pwrite_zeroes(BlockDriverState *bs,
 uint32_t tail = (offset + count) % s->cluster_size;

 trace_qcow2_pwrite_zeroes_start_req(qemu_coroutine_self(), offset, count);
+if (offset + count == bs->total_sectors * BDRV_SECTOR_SIZE) {
+tail = 0;
+}

 if (head || tail) {
 int64_t cl_start = (offset - head) >> BDRV_SECTOR_BITS;
diff --git a/tests/qemu-iotests/154 b/tests/qemu-iotests/154
index 7ca7219..dd8a426 100755
--- a/tests/qemu-iotests/154
+++ b/tests/qemu-iotests/154
@@ -2,7 +2,7 @@
 #
 # qcow2 specific bdrv_pwrite_zeroes tests with backing files (complements 034)
 #
-# Copyright (C) 2016 Red Hat, Inc.
+# Copyright (C) 2016-2017 Red Hat, Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -42,7 +42,10 @@ _supported_proto file
 _supported_os Linux

 CLUSTER_SIZE=4k
-size=128M
+size=$((128 * 1024 * 1024))
+
+# This test requires zero clusters, added in v3 images
+_unsupported_imgopts compat=0.10

 echo
 echo == backing file contains zeros ==
@@ -299,6 +302,159 @@ $QEMU_IO -c "read -P 0 75k 1k" "$TEST_IMG" | 
_filter_qemu_io

 $QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map

+echo
+echo == unaligned image tail cluster, no allocation needed ==
+
+# With no backing file, write to all or part of unallocated partial cluster
+# will mark the cluster as zero, but does not allocate.
+# Re-create the image each time to get back to unallocated clusters.
+
+# Write at the front: sector-wise, the request is: 128m... | 00 -- -- --
+_make_test_img $((size + 2048))
+$QEMU_IO -c "write -z $size 512" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "alloc $size 2048" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
+
+# Write at the back: sector-wise, the request is: 128m... | -- -- -- 00
+_make_test_img $((size + 2048))
+$QEMU_IO -c "write -z $((size + 1536)) 512" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "alloc $size 2048" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
+
+# Write at middle: sector-wise, the request is: 128m... | -- 00 00 --
+_make_test_img $((size + 2048))
+$QEMU_IO -c "write -z $((size + 512)) 1024" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "alloc $size 2048" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
+
+# Write entire cluster: sector-wise, the request is: 128m... | 00 00 00 00
+_make_test_img $((size + 2048))
+$QEMU_IO -c "write -z $size 2048" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "alloc $size 2048" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG map --outp

[Qemu-devel] [PATCH v13 11/12] qcow2: Assert that cluster operations are aligned

[Eric Blake]

We already audited (in commit 0c1bd469) that qcow2_discard_clusters()
is only passed cluster-aligned start values; but we can further
tighten the assertion that the only unaligned end value is at EOF.

Recent commits have taken advantage of an unaligned tail cluster,
for both discard and write zeroes.

Signed-off-by: Eric Blake 
Reviewed-by: Max Reitz 

---
v13: no change
v12: no change
v11: reserved for blkdebug half of v10
v10: rebase to context
v9: rebase to master, by asserting that only tail cluster is unaligned
v7, v8: only earlier half of series submitted for 2.9
v6: avoid assertion on non-cluster-aligned image, use s->cluster_sectors
to avoid a shift, drop R-b
v5: no change
v4: new patch
---
 block/qcow2-cluster.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 6c59708..1b9592d 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1559,11 +1559,10 @@ int qcow2_discard_clusters(BlockDriverState *bs, 
uint64_t offset,

 end_offset = offset + (nb_sectors << BDRV_SECTOR_BITS);

-/* The caller must cluster-align start; round end down except at EOF */
+/* Caller must pass aligned values, except at image end */
 assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
-if (end_offset != bs->total_sectors * BDRV_SECTOR_SIZE) {
-end_offset = start_of_cluster(s, end_offset);
-}
+assert(QEMU_IS_ALIGNED(end_offset, s->cluster_size) ||
+   end_offset == bs->total_sectors << BDRV_SECTOR_BITS);

 nb_clusters = size_to_clusters(s, end_offset - offset);

@@ -1646,9 +1645,17 @@ int qcow2_zero_clusters(BlockDriverState *bs, uint64_t 
offset, int nb_sectors,
 int flags)
 {
 BDRVQcow2State *s = bs->opaque;
+uint64_t end_offset;
 uint64_t nb_clusters;
 int ret;

+end_offset = offset + (nb_sectors << BDRV_SECTOR_BITS);
+
+/* Caller must pass aligned values, except at image end */
+assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
+assert(QEMU_IS_ALIGNED(end_offset, s->cluster_size) ||
+   end_offset == bs->total_sectors << BDRV_SECTOR_BITS);
+
 /* The zero flag is only supported by version 3 and newer */
 if (s->qcow_version < 3) {
 return -ENOTSUP;
-- 
2.9.3

[Qemu-devel] [PATCH v13 05/12] qcow2: Name typedef for cluster type

[Eric Blake]

Although it doesn't add all that much type safety (this is C, after
all), it does add a bit of legibility to use the name QCow2ClusterType
instead of a plain int.

In particular, qcow2_get_cluster_offset() has an overloaded return
type; a QCow2ClusterType on success, and -errno on failure; keeping
the cluster type in a separate variable makes it slightly easier for
the next patch to make further computations based on the type.

Suggested-by: Max Reitz 
Signed-off-by: Eric Blake 

---
v13: new patch
---
 block/qcow2.h  |  6 +++---
 block/qcow2-cluster.c  | 17 +
 block/qcow2-refcount.c |  2 +-
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index 8731f24..c148bbc 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -349,12 +349,12 @@ typedef struct QCowL2Meta
 QLIST_ENTRY(QCowL2Meta) next_in_flight;
 } QCowL2Meta;

-enum {
+typedef enum QCow2ClusterType {
 QCOW2_CLUSTER_UNALLOCATED,
 QCOW2_CLUSTER_NORMAL,
 QCOW2_CLUSTER_COMPRESSED,
 QCOW2_CLUSTER_ZERO
-};
+} QCow2ClusterType;

 typedef enum QCow2MetadataOverlap {
 QCOW2_OL_MAIN_HEADER_BITNR= 0,
@@ -443,7 +443,7 @@ static inline uint64_t 
qcow2_max_refcount_clusters(BDRVQcow2State *s)
 return QCOW_MAX_REFTABLE_SIZE >> s->cluster_bits;
 }

-static inline int qcow2_get_cluster_type(uint64_t l2_entry)
+static inline QCow2ClusterType qcow2_get_cluster_type(uint64_t l2_entry)
 {
 if (l2_entry & QCOW_OFLAG_COMPRESSED) {
 return QCOW2_CLUSTER_COMPRESSED;
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index f3bfce6..ed78a30 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -340,7 +340,7 @@ static int count_contiguous_clusters(int nb_clusters, int 
cluster_size,
  */
 static int count_contiguous_clusters_unallocated(int nb_clusters,
  uint64_t *l2_table,
- int wanted_type)
+ QCow2ClusterType wanted_type)
 {
 int i;

@@ -500,6 +500,7 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t 
offset,
 int l1_bits, c;
 unsigned int offset_in_cluster;
 uint64_t bytes_available, bytes_needed, nb_clusters;
+QCow2ClusterType type;
 int ret;

 offset_in_cluster = offset_into_cluster(s, offset);
@@ -522,13 +523,13 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, 
uint64_t offset,

 l1_index = offset >> l1_bits;
 if (l1_index >= s->l1_size) {
-ret = QCOW2_CLUSTER_UNALLOCATED;
+type = QCOW2_CLUSTER_UNALLOCATED;
 goto out;
 }

 l2_offset = s->l1_table[l1_index] & L1E_OFFSET_MASK;
 if (!l2_offset) {
-ret = QCOW2_CLUSTER_UNALLOCATED;
+type = QCOW2_CLUSTER_UNALLOCATED;
 goto out;
 }

@@ -557,8 +558,8 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t 
offset,
  * true */
 assert(nb_clusters <= INT_MAX);

-ret = qcow2_get_cluster_type(*cluster_offset);
-switch (ret) {
+type = qcow2_get_cluster_type(*cluster_offset);
+switch (type) {
 case QCOW2_CLUSTER_COMPRESSED:
 /* Compressed clusters can only be processed one by one */
 c = 1;
@@ -633,7 +634,7 @@ out:
 assert(bytes_available - offset_in_cluster <= UINT_MAX);
 *bytes = bytes_available - offset_in_cluster;

-return ret;
+return type;

 fail:
 qcow2_cache_put(bs, s->l2_table_cache, (void **)&l2_table);
@@ -891,7 +892,7 @@ static int count_cow_clusters(BDRVQcow2State *s, int 
nb_clusters,

 for (i = 0; i < nb_clusters; i++) {
 uint64_t l2_entry = be64_to_cpu(l2_table[l2_index + i]);
-int cluster_type = qcow2_get_cluster_type(l2_entry);
+QCow2ClusterType cluster_type = qcow2_get_cluster_type(l2_entry);

 switch(cluster_type) {
 case QCOW2_CLUSTER_NORMAL:
@@ -1757,7 +1758,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState 
*bs, uint64_t *l1_table,
 for (j = 0; j < s->l2_size; j++) {
 uint64_t l2_entry = be64_to_cpu(l2_table[j]);
 int64_t offset = l2_entry & L2E_OFFSET_MASK;
-int cluster_type = qcow2_get_cluster_type(l2_entry);
+QCow2ClusterType cluster_type = qcow2_get_cluster_type(l2_entry);
 bool preallocated = offset != 0;

 if (cluster_type != QCOW2_CLUSTER_ZERO) {
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 908dbe5..e639b34 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1640,7 +1640,7 @@ static int check_oflag_copied(BlockDriverState *bs, 
BdrvCheckResult *res,
 for (j = 0; j < s->l2_size; j++) {
 uint64_t l2_entry = be64_to_cpu(l2_table[j]);
 uint64_t data_offset = l2_entry & L2E_OFFSET_MASK;
-int cluster_type = qcow2_get_cluster_type(l2_entry);
+QCow2ClusterType cluster_type = qcow2_get_cluster_type(l2_entry);

 if ((cluster_

[Qemu-devel] [PATCH v13 04/12] qcow2: Correctly report status of preallocated zero clusters

[Eric Blake]

We were throwing away the preallocation information associated with
zero clusters.  But we should be matching the well-defined semantics
in bdrv_get_block_status(), where (BDRV_BLOCK_ZERO |
BDRV_BLOCK_OFFSET_VALID) informs the user which offset is reserved,
while still reminding the user that reading from that offset is
likely to read garbage.

count_contiguous_clusters_by_type() is now used only for unallocated
cluster runs, hence it gets renamed and tightened.

Making this change lets us see which portions of an image are zero
but preallocated, when using qemu-img map --output=json.  The
--output=human side intentionally ignores all zero clusters, whether
or not they are preallocated.

The fact that there is no change to qemu-iotests './check -qcow2'
merely means that we aren't yet testing this aspect of qemu-img;
a later patch will add a test.

Signed-off-by: Eric Blake 
Reviewed-by: Max Reitz 

---
v13: no change
v12: rename helper function
v11: reserved for blkdebug half of v10
v10: new patch
---
 block/qcow2-cluster.c | 45 +++--
 1 file changed, 35 insertions(+), 10 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 335a505..f3bfce6 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -334,16 +334,23 @@ static int count_contiguous_clusters(int nb_clusters, int 
cluster_size,
return i;
 }

-static int count_contiguous_clusters_by_type(int nb_clusters,
- uint64_t *l2_table,
- int wanted_type)
+/*
+ * Checks how many consecutive unallocated clusters in a given L2
+ * table have the same cluster type.
+ */
+static int count_contiguous_clusters_unallocated(int nb_clusters,
+ uint64_t *l2_table,
+ int wanted_type)
 {
 int i;

+assert(wanted_type == QCOW2_CLUSTER_ZERO ||
+   wanted_type == QCOW2_CLUSTER_UNALLOCATED);
 for (i = 0; i < nb_clusters; i++) {
-int type = qcow2_get_cluster_type(be64_to_cpu(l2_table[i]));
+uint64_t entry = be64_to_cpu(l2_table[i]);
+int type = qcow2_get_cluster_type(entry);

-if (type != wanted_type) {
+if (type != wanted_type || entry & L2E_OFFSET_MASK) {
 break;
 }
 }
@@ -565,14 +572,32 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, 
uint64_t offset,
 ret = -EIO;
 goto fail;
 }
-c = count_contiguous_clusters_by_type(nb_clusters, &l2_table[l2_index],
-  QCOW2_CLUSTER_ZERO);
-*cluster_offset = 0;
+/* Distinguish between pure zero clusters and pre-allocated ones */
+if (*cluster_offset & L2E_OFFSET_MASK) {
+c = count_contiguous_clusters(nb_clusters, s->cluster_size,
+  &l2_table[l2_index], 
QCOW_OFLAG_ZERO);
+*cluster_offset &= L2E_OFFSET_MASK;
+if (offset_into_cluster(s, *cluster_offset)) {
+qcow2_signal_corruption(bs, true, -1, -1,
+"Preallocated zero cluster offset %#"
+PRIx64 " unaligned (L2 offset: %#"
+PRIx64 ", L2 index: %#x)",
+*cluster_offset, l2_offset, l2_index);
+ret = -EIO;
+goto fail;
+}
+} else {
+c = count_contiguous_clusters_unallocated(nb_clusters,
+  &l2_table[l2_index],
+  QCOW2_CLUSTER_ZERO);
+*cluster_offset = 0;
+}
 break;
 case QCOW2_CLUSTER_UNALLOCATED:
 /* how many empty clusters ? */
-c = count_contiguous_clusters_by_type(nb_clusters, &l2_table[l2_index],
-  QCOW2_CLUSTER_UNALLOCATED);
+c = count_contiguous_clusters_unallocated(nb_clusters,
+  &l2_table[l2_index],
+  QCOW2_CLUSTER_UNALLOCATED);
 *cluster_offset = 0;
 break;
 case QCOW2_CLUSTER_NORMAL:
-- 
2.9.3

[Qemu-devel] [PATCH v13 08/12] iotests: Improve _filter_qemu_img_map

[Eric Blake]

Although _filter_qemu_img_map documents that it scrubs offsets, it
was only doing so for human mode.  Of the existing tests using the
filter (97, 122, 150, 154, 176), two of them are affected, but it
does not hurt the validity of the tests to not require particular
mappings (another test, 66, uses offsets but intentionally does not
pass through _filter_qemu_img_map, because it checks that offsets
are unchanged before and after an operation).

Another justification for this patch is that it will allow a future
patch to utilize 'qemu-img map --output=json' to check the status of
preallocated zero clusters without regards to the mapping (since
the qcow2 mapping can be very sensitive to the chosen cluster size,
when preallocation is not in use).

Signed-off-by: Eric Blake 
Reviewed-by: Max Reitz 

---
v13: kill TAB damage
v12: new patch
---
 tests/qemu-iotests/common.filter |  4 +++-
 tests/qemu-iotests/122.out   | 16 
 tests/qemu-iotests/154.out   | 30 +++---
 3 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index f58548d..2f595b2 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -152,10 +152,12 @@ _filter_img_info()
 -e "/log_size: [0-9]\\+/d"
 }

-# filter out offsets and file names from qemu-img map
+# filter out offsets and file names from qemu-img map; good for both
+# human and json output
 _filter_qemu_img_map()
 {
 sed -e 's/\([0-9a-fx]* *[0-9a-fx]* *\)[0-9a-fx]* */\1/g' \
+-e 's/"offset": [0-9]\+/"offset": OFFSET/g' \
 -e 's/Mapped to *//' | _filter_testdir | _filter_imgfmt
 }

diff --git a/tests/qemu-iotests/122.out b/tests/qemu-iotests/122.out
index 9317d80..47d8656 100644
--- a/tests/qemu-iotests/122.out
+++ b/tests/qemu-iotests/122.out
@@ -112,7 +112,7 @@ read 3145728/3145728 bytes at offset 0
 3 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 63963136/63963136 bytes at offset 3145728
 61 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true, 
"offset": 327680}]
+[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true, 
"offset": OFFSET}]

 convert -c -S 0:
 read 3145728/3145728 bytes at offset 0
@@ -134,7 +134,7 @@ read 30408704/30408704 bytes at offset 3145728
 29 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 33554432/33554432 bytes at offset 33554432
 32 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true, 
"offset": 327680}]
+[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true, 
"offset": OFFSET}]

 convert -c -S 0 with source backing file:
 read 3145728/3145728 bytes at offset 0
@@ -152,7 +152,7 @@ read 30408704/30408704 bytes at offset 3145728
 29 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 33554432/33554432 bytes at offset 33554432
 32 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true, 
"offset": 327680}]
+[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true, 
"offset": OFFSET}]

 convert -c -S 0 -B ...
 read 3145728/3145728 bytes at offset 0
@@ -176,11 +176,11 @@ wrote 1024/1024 bytes at offset 17408
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

 convert -S 4k
-[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true, 
"offset": 8192},
+[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true, 
"offset": OFFSET},
 { "start": 1024, "length": 7168, "depth": 0, "zero": true, "data": false},
-{ "start": 8192, "length": 1024, "depth": 0, "zero": false, "data": true, 
"offset": 9216},
+{ "start": 8192, "length": 1024, "depth": 0, "zero": false, "data": true, 
"offset": OFFSET},
 { "start": 9216, "length": 8192, "depth": 0, "zero": true, "data": false},
-{ "start": 17408, "length": 1024, "depth": 0, "zero": false, "data": true, 
"offset": 10240},
+{ "start": 17408, "length": 1024, "depth": 0, "zero": false, "data": true, 
"offset": OFFSET},
 { "start": 18432, "length": 67090432, "depth": 0, "zero": true, "data": false}]

 convert -c -S 4k
@@ -192,9 +192,9 @@ convert -c -S 4k
 { "start": 18432, "length": 67090432, "depth": 0, "zero": true, "data": false}]

 convert -S 8k
-[{ "start": 0, "length": 9216, "depth": 0, "zero": false, "data": true, 
"offset": 8192},
+[{ "start": 0, "length": 9216, "depth": 0, "zero": false, "data": true, 
"offset": OFFSET},
 { "start": 9216, "length": 8192, "depth": 0, "zero": true, "data": false},
-{ "start": 17408, "length": 1024, "depth": 0, "zero": false, "data": true, 
"offset": 17408},
+{ "start": 17408, "length": 1024, "depth": 0, "zero": false, "data": true, 
"offset": OFFSET},
 { "start": 18432, "length": 67090432, "depth": 0, "zero": true, "data": false}]

 convert -c -S 8k
diff --git a/tests/qemu-i

[Qemu-devel] [PATCH v13 02/12] qcow2: Use consistent switch indentation

[Eric Blake]

Fix a couple of inconsistent indentations, before an upcoming
patch further tweaks the switch statements.
(best viewed with 'git diff -b').

Signed-off-by: Eric Blake 
Reviewed-by: Max Reitz 

---
v13: split off non-indentation changes
v12: new patch
---
 block/qcow2-cluster.c  | 32 +--
 block/qcow2-refcount.c | 84 +-
 2 files changed, 58 insertions(+), 58 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 31077d8..335a505 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1504,25 +1504,25 @@ static int discard_single_l2(BlockDriverState *bs, 
uint64_t offset,
  * but rather fall through to the backing file.
  */
 switch (qcow2_get_cluster_type(old_l2_entry)) {
-case QCOW2_CLUSTER_UNALLOCATED:
-if (full_discard || !bs->backing) {
-continue;
-}
-break;
+case QCOW2_CLUSTER_UNALLOCATED:
+if (full_discard || !bs->backing) {
+continue;
+}
+break;

-case QCOW2_CLUSTER_ZERO:
-/* Preallocated zero clusters should be discarded in any case 
*/
-if (!full_discard && (old_l2_entry & L2E_OFFSET_MASK) == 0) {
-continue;
-}
-break;
+case QCOW2_CLUSTER_ZERO:
+/* Preallocated zero clusters should be discarded in any case */
+if (!full_discard && (old_l2_entry & L2E_OFFSET_MASK) == 0) {
+continue;
+}
+break;

-case QCOW2_CLUSTER_NORMAL:
-case QCOW2_CLUSTER_COMPRESSED:
-break;
+case QCOW2_CLUSTER_NORMAL:
+case QCOW2_CLUSTER_COMPRESSED:
+break;

-default:
-abort();
+default:
+abort();
 }

 /* First remove L2 entries */
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index db0af2c..908dbe5 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1128,61 +1128,61 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
 offset = entry & L2E_OFFSET_MASK;

 switch (qcow2_get_cluster_type(entry)) {
-case QCOW2_CLUSTER_COMPRESSED:
-nb_csectors = ((entry >> s->csize_shift) &
-   s->csize_mask) + 1;
-if (addend != 0) {
-ret = update_refcount(bs,
+case QCOW2_CLUSTER_COMPRESSED:
+nb_csectors = ((entry >> s->csize_shift) &
+   s->csize_mask) + 1;
+if (addend != 0) {
+ret = update_refcount(bs,
 (entry & s->cluster_offset_mask) & ~511,
 nb_csectors * 512, abs(addend), addend < 0,
 QCOW2_DISCARD_SNAPSHOT);
-if (ret < 0) {
-goto fail;
-}
-}
-/* compressed clusters are never modified */
-refcount = 2;
-break;
-
-case QCOW2_CLUSTER_NORMAL:
-case QCOW2_CLUSTER_ZERO:
-if (offset_into_cluster(s, offset)) {
-qcow2_signal_corruption(bs, true, -1, -1, "Data "
-"cluster offset %#" PRIx64
-" unaligned (L2 offset: %#"
-PRIx64 ", L2 index: %#x)",
-offset, l2_offset, j);
-ret = -EIO;
+if (ret < 0) {
 goto fail;
 }
+}
+/* compressed clusters are never modified */
+refcount = 2;
+break;

-cluster_index = offset >> s->cluster_bits;
-if (!cluster_index) {
-/* unallocated */
-refcount = 0;
-break;
-}
-if (addend != 0) {
-ret = qcow2_update_cluster_refcount(bs,
+case QCOW2_CLUSTER_NORMAL:
+case QCOW2_CLUSTER_ZERO:
+if (offset_into_cluster(s, offset)) {
+qcow2_signal_corruption(bs, true, -1, -1, "Data "
+"cluster offset %#" PRIx64
+" unaligned (L2 offset: %#"
+   

[Qemu-devel] [PATCH v13 12/12] qcow2: Discard/zero clusters by byte count

[Eric Blake]

Passing a byte offset, but sector count, when we ultimately
want to operate on cluster granularity, is madness.  Clean up
the external interfaces to take both offset and count as bytes,
while still keeping the assertion added previously that the
caller must align the values to a cluster.  Then rename things
to make sure backports don't get confused by changed units:
instead of qcow2_discard_clusters() and qcow2_zero_clusters(),
we now have qcow2_cluster_discard() and qcow2_cluster_zeroize().

The internal functions still operate on clusters at a time, and
return an int for number of cleared clusters; but on an image
with 2M clusters, a single L2 table holds 256k entries that each
represent a 2M cluster, totalling well over INT_MAX bytes if we
ever had a request for that many bytes at once.  All our callers
currently limit themselves to 32-bit bytes (and therefore fewer
clusters), but by making this function 64-bit clean, we have one
less place to clean up if we later improve the block layer to
support 64-bit bytes through all operations (with the block layer
auto-fragmenting on behalf of more-limited drivers), rather than
the current state where some interfaces are artificially limited
to INT_MAX at a time.

Signed-off-by: Eric Blake 
Reviewed-by: Max Reitz 

---
v13: no change
v12: minor tweaks suggested by Max
v11: reserved for blkdebug half of v10
v10: squash in fixup accounting for unaligned file end
v9: rebase to earlier changes, drop R-b
v7, v8: only earlier half of series submitted for 2.9
v6: rebase due to context
v5: s/count/byte/ to make the units obvious, and rework the math
to ensure no 32-bit integer overflow on large clusters
v4: improve function names, split assertion additions into earlier patch
[no v3 or v2]
v1: https://lists.gnu.org/archive/html/qemu-devel/2016-12/msg00339.html
---
 block/qcow2.h  |  9 +
 block/qcow2-cluster.c  | 42 ++
 block/qcow2-snapshot.c |  7 +++
 block/qcow2.c  | 22 +-
 4 files changed, 39 insertions(+), 41 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index 810ceb8..1801dc3 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -551,10 +551,11 @@ uint64_t 
qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
  int compressed_size);

 int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
-int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
-int nb_sectors, enum qcow2_discard_type type, bool full_discard);
-int qcow2_zero_clusters(BlockDriverState *bs, uint64_t offset, int nb_sectors,
-int flags);
+int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
+  uint64_t bytes, enum qcow2_discard_type type,
+  bool full_discard);
+int qcow2_cluster_zeroize(BlockDriverState *bs, uint64_t offset,
+  uint64_t bytes, int flags);

 int qcow2_expand_zero_clusters(BlockDriverState *bs,
BlockDriverAmendStatusCB *status_cb,
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 1b9592d..89d23e5 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1549,34 +1549,36 @@ static int discard_single_l2(BlockDriverState *bs, 
uint64_t offset,
 return nb_clusters;
 }

-int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
-int nb_sectors, enum qcow2_discard_type type, bool full_discard)
+int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
+  uint64_t bytes, enum qcow2_discard_type type,
+  bool full_discard)
 {
 BDRVQcow2State *s = bs->opaque;
-uint64_t end_offset;
+uint64_t end_offset = offset + bytes;
 uint64_t nb_clusters;
+int64_t cleared;
 int ret;

-end_offset = offset + (nb_sectors << BDRV_SECTOR_BITS);
-
 /* Caller must pass aligned values, except at image end */
 assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
 assert(QEMU_IS_ALIGNED(end_offset, s->cluster_size) ||
end_offset == bs->total_sectors << BDRV_SECTOR_BITS);

-nb_clusters = size_to_clusters(s, end_offset - offset);
+nb_clusters = size_to_clusters(s, bytes);

 s->cache_discards = true;

 /* Each L2 table is handled by its own loop iteration */
 while (nb_clusters > 0) {
-ret = discard_single_l2(bs, offset, nb_clusters, type, full_discard);
-if (ret < 0) {
+cleared = discard_single_l2(bs, offset, nb_clusters, type,
+full_discard);
+if (cleared < 0) {
+ret = cleared;
 goto fail;
 }

-nb_clusters -= ret;
-offset += (ret * s->cluster_size);
+nb_clusters -= cleared;
+offset += (cleared * s->cluster_size);
 }

 ret = 0;
@@ -1641,16 +1643,15 @@ static int zero_single_l2(BlockDriverState *bs, 
uint64_t offset,
  

[Qemu-devel] [PATCH v13 00/12] qcow2 zero-cluster tweaks [was add blkdebug tests]

[Eric Blake]

I've collected several improvements for qcow2 zero-cluster handling.

Available as a tag at:
git fetch git://repo.or.cz/qemu/ericb.git nbd-blkdebug-v13

Marked as v13 for "hysterical raisins", since it it the half of
v10 [1] that was not resubmitted as v11 [2].

Depends on Max's block tree:
https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg00641.html
and on Max's qcow2 cleanups:
https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg00689.html

[1] https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg05227.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg05896.html
[3] https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg00722.html

Changes since last posting (v12 [3]):
- 2 more new patches: split indentation from other changes (patch 1),
and add a typedef (patch 5)
- address Max's findings

001/12:[down] 'qcow2: Nicer variable names in qcow2_update_snapshot_refcount()'
002/12:[0040] [FC] 'qcow2: Use consistent switch indentation'
003/12:[] [--] 'block: Update comments on BDRV_BLOCK_* meanings'
004/12:[] [--] 'qcow2: Correctly report status of preallocated zero 
clusters'
005/12:[down] 'qcow2: Name typedef for cluster type'
006/12:[0032] [FC] 'qcow2: Make distinction between zero cluster types obvious'
007/12:[0002] [FC] 'qcow2: Optimize zero_single_l2() to minimize L2 churn'
008/12:[0002] [FC] 'iotests: Improve _filter_qemu_img_map'
009/12:[0010] [FC] 'iotests: Add test 179 to cover write zeroes with unmap'
010/12:[0003] [FC] 'qcow2: Optimize write zero of unaligned tail cluster'
011/12:[] [--] 'qcow2: Assert that cluster operations are aligned'
012/12:[] [--] 'qcow2: Discard/zero clusters by byte count'

Eric Blake (12):
  qcow2: Nicer variable names in qcow2_update_snapshot_refcount()
  qcow2: Use consistent switch indentation
  block: Update comments on BDRV_BLOCK_* meanings
  qcow2: Correctly report status of preallocated zero clusters
  qcow2: Name typedef for cluster type
  qcow2: Make distinction between zero cluster types obvious
  qcow2: Optimize zero_single_l2() to minimize L2 churn
  iotests: Improve _filter_qemu_img_map
  iotests: Add test 179 to cover write zeroes with unmap
  qcow2: Optimize write zero of unaligned tail cluster
  qcow2: Assert that cluster operations are aligned
  qcow2: Discard/zero clusters by byte count

 block/qcow2.h|  23 +++--
 include/block/block.h|  35 
 include/block/block_int.h|   7 ++
 block/qcow2-cluster.c| 181 ++-
 block/qcow2-refcount.c   | 144 +++
 block/qcow2-snapshot.c   |   7 +-
 block/qcow2.c|  38 
 tests/qemu-iotests/common.filter |   4 +-
 tests/qemu-iotests/060.out   |   6 +-
 tests/qemu-iotests/122.out   |  16 ++--
 tests/qemu-iotests/154   | 160 +-
 tests/qemu-iotests/154.out   | 158 ++
 tests/qemu-iotests/179   | 130 
 tests/qemu-iotests/179.out   | 156 +
 tests/qemu-iotests/group |   1 +
 15 files changed, 840 insertions(+), 226 deletions(-)
 create mode 100755 tests/qemu-iotests/179
 create mode 100644 tests/qemu-iotests/179.out

-- 
2.9.3

[Qemu-devel] [PATCH v13 06/12] qcow2: Make distinction between zero cluster types obvious

[Eric Blake]

Treat plain zero clusters differently from allocated ones, so that
we can simplify the logic of checking whether an offset is present.
Do this by splitting QCOW2_CLUSTER_ZERO into two new enums,
QCOW2_CLUSTER_ZERO_PLAIN and QCOW2_CLUSTER_ZERO_ALLOC.

I tried to arrange the enum so that we could use
'ret <= QCOW2_CLUSTER_ZERO_PLAIN' for all unallocated types, and
'ret >= QCOW2_CLUSTER_ZERO_ALLOC' for allocated types, although
I didn't actually end up taking advantage of the layout.

In many cases, this leads to simpler code, by properly combining
cases (sometimes, both zero types pair together, other times,
plain zero is more like unallocated while allocated zero is more
like normal).

Signed-off-by: Eric Blake 

---
v13: s/!preallocated/cluster_type == QCOW2_CLUSTER_ZERO_PLAIN/, fix
error message text (including iotest fallout), rebase to earlier cleanups
v12: new patch
---
 block/qcow2.h  |  8 +++--
 block/qcow2-cluster.c  | 81 ++
 block/qcow2-refcount.c | 44 +++--
 block/qcow2.c  |  9 --
 tests/qemu-iotests/060.out |  6 ++--
 5 files changed, 64 insertions(+), 84 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index c148bbc..810ceb8 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -351,9 +351,10 @@ typedef struct QCowL2Meta

 typedef enum QCow2ClusterType {
 QCOW2_CLUSTER_UNALLOCATED,
+QCOW2_CLUSTER_ZERO_PLAIN,
+QCOW2_CLUSTER_ZERO_ALLOC,
 QCOW2_CLUSTER_NORMAL,
 QCOW2_CLUSTER_COMPRESSED,
-QCOW2_CLUSTER_ZERO
 } QCow2ClusterType;

 typedef enum QCow2MetadataOverlap {
@@ -448,7 +449,10 @@ static inline QCow2ClusterType 
qcow2_get_cluster_type(uint64_t l2_entry)
 if (l2_entry & QCOW_OFLAG_COMPRESSED) {
 return QCOW2_CLUSTER_COMPRESSED;
 } else if (l2_entry & QCOW_OFLAG_ZERO) {
-return QCOW2_CLUSTER_ZERO;
+if (l2_entry & L2E_OFFSET_MASK) {
+return QCOW2_CLUSTER_ZERO_ALLOC;
+}
+return QCOW2_CLUSTER_ZERO_PLAIN;
 } else if (!(l2_entry & L2E_OFFSET_MASK)) {
 return QCOW2_CLUSTER_UNALLOCATED;
 } else {
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index ed78a30..a4a3229 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -321,8 +321,7 @@ static int count_contiguous_clusters(int nb_clusters, int 
cluster_size,
 /* must be allocated */
 first_cluster_type = qcow2_get_cluster_type(first_entry);
 assert(first_cluster_type == QCOW2_CLUSTER_NORMAL ||
-   (first_cluster_type == QCOW2_CLUSTER_ZERO &&
-(first_entry & L2E_OFFSET_MASK) != 0));
+   first_cluster_type == QCOW2_CLUSTER_ZERO_ALLOC);

 for (i = 0; i < nb_clusters; i++) {
 uint64_t l2_entry = be64_to_cpu(l2_table[i]) & mask;
@@ -344,13 +343,13 @@ static int count_contiguous_clusters_unallocated(int 
nb_clusters,
 {
 int i;

-assert(wanted_type == QCOW2_CLUSTER_ZERO ||
+assert(wanted_type == QCOW2_CLUSTER_ZERO_PLAIN ||
wanted_type == QCOW2_CLUSTER_UNALLOCATED);
 for (i = 0; i < nb_clusters; i++) {
 uint64_t entry = be64_to_cpu(l2_table[i]);
-int type = qcow2_get_cluster_type(entry);
+QCow2ClusterType type = qcow2_get_cluster_type(entry);

-if (type != wanted_type || entry & L2E_OFFSET_MASK) {
+if (type != wanted_type) {
 break;
 }
 }
@@ -559,55 +558,36 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, 
uint64_t offset,
 assert(nb_clusters <= INT_MAX);

 type = qcow2_get_cluster_type(*cluster_offset);
+if (s->qcow_version < 3 && (type == QCOW2_CLUSTER_ZERO_PLAIN ||
+type == QCOW2_CLUSTER_ZERO_ALLOC)) {
+qcow2_signal_corruption(bs, true, -1, -1, "Zero cluster entry found"
+" in pre-v3 image (L2 offset: %#" PRIx64
+", L2 index: %#x)", l2_offset, l2_index);
+ret = -EIO;
+goto fail;
+}
 switch (type) {
 case QCOW2_CLUSTER_COMPRESSED:
 /* Compressed clusters can only be processed one by one */
 c = 1;
 *cluster_offset &= L2E_COMPRESSED_OFFSET_SIZE_MASK;
 break;
-case QCOW2_CLUSTER_ZERO:
-if (s->qcow_version < 3) {
-qcow2_signal_corruption(bs, true, -1, -1, "Zero cluster entry 
found"
-" in pre-v3 image (L2 offset: %#" PRIx64
-", L2 index: %#x)", l2_offset, l2_index);
-ret = -EIO;
-goto fail;
-}
-/* Distinguish between pure zero clusters and pre-allocated ones */
-if (*cluster_offset & L2E_OFFSET_MASK) {
-c = count_contiguous_clusters(nb_clusters, s->cluster_size,
-  &l2_table[l2_index], 
QCOW_OFLAG_ZERO);
-*cluster_offset &= L2E_OFFSET_MASK;
-if (offset_into_cluster(s, *cluster_offset)) {
-

[Qemu-devel] [PATCH v13 07/12] qcow2: Optimize zero_single_l2() to minimize L2 churn

[Eric Blake]

Similar to discard_single_l2(), we should try to avoid dirtying
the L2 cache when the cluster we are changing already has the
right characteristics.

Note that by the time we get to zero_single_l2(), BDRV_REQ_MAY_UNMAP
is a requirement to unallocate a cluster (this is because the block
layer clears that flag if discard.* flags during open requested that
we never punch holes - see the conversation around commit 170f4b2e,
https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg07306.html).
Therefore, this patch can only reuse a zero cluster as-is if either
unmapping is not requested, or if the zero cluster was not associated
with an allocation.

Technically, there are some cases where an unallocated cluster
already reads as all zeroes (namely, when there is no backing file
[easy: check bs->backing], or when the backing file also reads as
zeroes [harder: we can't check bdrv_get_block_status since we are
already holding the lock]), where the guest would not immediately see
a difference if we left that cluster unallocated.  But if the user
did not request unmapping, leaving an unallocated cluster is wrong;
and even if the user DID request unmapping, keeping a cluster
unallocated risks a subtle semantic change of guest-visible contents
if a backing file is later added, and it is not worth auditing
whether all internal uses such as mirror properly avoid an unmap
request.  Thus, this patch is intentionally limited to just clusters
that are already marked as zero.

Signed-off-by: Eric Blake 
Reviewed-by: Max Reitz 

---
v13: use enum name
v12: store cluster type in temporary
v11: reserved for blkdebug half of v10
v10: new patch, replacing earlier attempt to use unallocated clusters,
and ditching any optimization of v2 files
---
 block/qcow2-cluster.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index a4a3229..6c59708 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1601,6 +1601,7 @@ static int zero_single_l2(BlockDriverState *bs, uint64_t 
offset,
 int l2_index;
 int ret;
 int i;
+bool unmap = !!(flags & BDRV_REQ_MAY_UNMAP);

 ret = get_cluster_table(bs, offset, &l2_table, &l2_index);
 if (ret < 0) {
@@ -1613,12 +1614,22 @@ static int zero_single_l2(BlockDriverState *bs, 
uint64_t offset,

 for (i = 0; i < nb_clusters; i++) {
 uint64_t old_offset;
+QCow2ClusterType cluster_type;

 old_offset = be64_to_cpu(l2_table[l2_index + i]);

-/* Update L2 entries */
+/*
+ * Minimize L2 changes if the cluster already reads back as
+ * zeroes with correct allocation.
+ */
+cluster_type = qcow2_get_cluster_type(old_offset);
+if (cluster_type == QCOW2_CLUSTER_ZERO_PLAIN ||
+(cluster_type == QCOW2_CLUSTER_ZERO_ALLOC && !unmap)) {
+continue;
+}
+
 qcow2_cache_entry_mark_dirty(bs, s->l2_table_cache, l2_table);
-if (old_offset & QCOW_OFLAG_COMPRESSED || flags & BDRV_REQ_MAY_UNMAP) {
+if (cluster_type == QCOW2_CLUSTER_COMPRESSED || unmap) {
 l2_table[l2_index + i] = cpu_to_be64(QCOW_OFLAG_ZERO);
 qcow2_free_any_clusters(bs, old_offset, 1, QCOW2_DISCARD_REQUEST);
 } else {
-- 
2.9.3

[Qemu-devel] [PATCH v13 03/12] block: Update comments on BDRV_BLOCK_* meanings

[Eric Blake]

We had some conflicting documentation: a nice 8-way table that
described all possible combinations of DATA, ZERO, and
OFFSET_VALID, contrasted with text that implied that OFFSET_VALID
always meant raw data could be read directly.  Furthermore, the
text refers a lot to bs->file, even though the interface was
updated back in 67a0fd2a to let the driver pass back a specific
BDS (not necessarily bs->file).  As the 8-way table is the
intended semantics, simplify the rest of the text to get rid of
the confusion.

ALLOCATED is always set by the block layer for convenience (drivers
do not have to worry about it).  RAW is used only internally, but
by more than the raw driver.  Document these additional items on
the driver callback.

Suggested-by: Max Reitz 
Signed-off-by: Eric Blake 
Reviewed-by: Max Reitz 

---
v13: commit message tweaks
v12: even more wording tweaks
v11: reserved for blkdebug half of v10
v10: new patch
---
 include/block/block.h | 35 +++
 include/block/block_int.h |  7 +++
 2 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index 877fbb0..14fba5e 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -121,29 +121,32 @@ typedef struct HDGeometry {
 #define BDRV_REQUEST_MAX_BYTES (BDRV_REQUEST_MAX_SECTORS << BDRV_SECTOR_BITS)

 /*
- * Allocation status flags
- * BDRV_BLOCK_DATA: data is read from a file returned by bdrv_get_block_status.
- * BDRV_BLOCK_ZERO: sectors read as zero
- * BDRV_BLOCK_OFFSET_VALID: sector stored as raw data in a file returned by
- *  bdrv_get_block_status.
+ * Allocation status flags for bdrv_get_block_status() and friends.
+ *
+ * Public flags:
+ * BDRV_BLOCK_DATA: allocation for data at offset is tied to this layer
+ * BDRV_BLOCK_ZERO: offset reads as zero
+ * BDRV_BLOCK_OFFSET_VALID: an associated offset exists for accessing raw data
  * BDRV_BLOCK_ALLOCATED: the content of the block is determined by this
- *   layer (as opposed to the backing file)
- * BDRV_BLOCK_RAW: used internally to indicate that the request
- * was answered by the raw driver and that one
- * should look in bs->file directly.
+ *   layer (short for DATA || ZERO), set by block layer
  *
- * If BDRV_BLOCK_OFFSET_VALID is set, bits 9-62 represent the offset in
- * bs->file where sector data can be read from as raw data.
+ * Internal flag:
+ * BDRV_BLOCK_RAW: used internally to indicate that the request was
+ * answered by a passthrough driver such as raw and that the
+ * block layer should recompute the answer from bs->file.
  *
- * DATA == 0 && ZERO == 0 means that data is read from backing_hd if present.
+ * If BDRV_BLOCK_OFFSET_VALID is set, bits 9-62 (BDRV_BLOCK_OFFSET_MASK)
+ * represent the offset in the returned BDS that is allocated for the
+ * corresponding raw data; however, whether that offset actually contains
+ * data also depends on BDRV_BLOCK_DATA and BDRV_BLOCK_ZERO, as follows:
  *
  * DATA ZERO OFFSET_VALID
- *  ttt   sectors read as zero, bs->file is zero at offset
- *  tft   sectors read as valid from bs->file at offset
- *  ftt   sectors preallocated, read as zero, bs->file not
+ *  ttt   sectors read as zero, returned file is zero at offset
+ *  tft   sectors read as valid from file at offset
+ *  ftt   sectors preallocated, read as zero, returned file not
  *necessarily zero at offset
  *  fft   sectors preallocated but read from backing_hd,
- *bs->file contains garbage at offset
+ *returned file contains garbage at offset
  *  ttf   sectors preallocated, read as zero, unknown offset
  *  tff   sectors read from unknown file or offset
  *  ftf   not allocated or unknown offset, read as zero
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 1b4d08e..771fa97 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -165,6 +165,13 @@ struct BlockDriver {
 int64_t offset, int count, BdrvRequestFlags flags);
 int coroutine_fn (*bdrv_co_pdiscard)(BlockDriverState *bs,
 int64_t offset, int count);
+
+/*
+ * Building block for bdrv_block_status[_above]. The driver should
+ * answer only according to the current layer, and should not
+ * set BDRV_BLOCK_ALLOCATED, but may set BDRV_BLOCK_RAW.  See block.h
+ * for the meaning of _DATA, _ZERO, and _OFFSET_VALID.
+ */
 int64_t coroutine_fn (*bdrv_co_get_block_status)(BlockDriverState *bs,
 int64_t sector_num, int nb_sectors, int *pnum,
 BlockDriverState **file);
-- 
2.9.3

[Qemu-devel] [PATCH v13 01/12] qcow2: Nicer variable names in qcow2_update_snapshot_refcount()

[Eric Blake]

In order to keep checkpatch happy when the next patch changes
indentation, we first have to shorten some long lines.  The easiest
approach is to use a new variable in place of
'offset & L2E_OFFSET_MASK', except that 'offset' is the best name
for that variable.  Change '[old_]offset' to '[old_]entry' to
make room.

While touching things, also fix checkpatch warnings about unusual
'for' statements.

Suggested by Max Reitz 
Signed-off-by: Eric Blake 

---
v13: new patch
---
 block/qcow2-refcount.c | 42 ++
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 4efca7e..db0af2c 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1059,9 +1059,9 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
 int64_t l1_table_offset, int l1_size, int addend)
 {
 BDRVQcow2State *s = bs->opaque;
-uint64_t *l1_table, *l2_table, l2_offset, offset, l1_size2, refcount;
+uint64_t *l1_table, *l2_table, l2_offset, entry, l1_size2, refcount;
 bool l1_allocated = false;
-int64_t old_offset, old_l2_offset;
+int64_t old_entry, old_l2_offset;
 int i, j, l1_modified = 0, nb_csectors;
 int ret;

@@ -1089,15 +1089,16 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
 goto fail;
 }

-for(i = 0;i < l1_size; i++)
+for (i = 0; i < l1_size; i++) {
 be64_to_cpus(&l1_table[i]);
+}
 } else {
 assert(l1_size == s->l1_size);
 l1_table = s->l1_table;
 l1_allocated = false;
 }

-for(i = 0; i < l1_size; i++) {
+for (i = 0; i < l1_size; i++) {
 l2_offset = l1_table[i];
 if (l2_offset) {
 old_l2_offset = l2_offset;
@@ -1117,20 +1118,22 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
 goto fail;
 }

-for(j = 0; j < s->l2_size; j++) {
+for (j = 0; j < s->l2_size; j++) {
 uint64_t cluster_index;
+uint64_t offset;

-offset = be64_to_cpu(l2_table[j]);
-old_offset = offset;
-offset &= ~QCOW_OFLAG_COPIED;
+entry = be64_to_cpu(l2_table[j]);
+old_entry = entry;
+entry &= ~QCOW_OFLAG_COPIED;
+offset = entry & L2E_OFFSET_MASK;

-switch (qcow2_get_cluster_type(offset)) {
+switch (qcow2_get_cluster_type(entry)) {
 case QCOW2_CLUSTER_COMPRESSED:
-nb_csectors = ((offset >> s->csize_shift) &
+nb_csectors = ((entry >> s->csize_shift) &
s->csize_mask) + 1;
 if (addend != 0) {
 ret = update_refcount(bs,
-(offset & s->cluster_offset_mask) & ~511,
+(entry & s->cluster_offset_mask) & ~511,
 nb_csectors * 512, abs(addend), addend < 0,
 QCOW2_DISCARD_SNAPSHOT);
 if (ret < 0) {
@@ -1143,18 +1146,17 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,

 case QCOW2_CLUSTER_NORMAL:
 case QCOW2_CLUSTER_ZERO:
-if (offset_into_cluster(s, offset & L2E_OFFSET_MASK)) {
+if (offset_into_cluster(s, offset)) {
 qcow2_signal_corruption(bs, true, -1, -1, "Data "
-"cluster offset %#llx "
-"unaligned (L2 offset: %#"
+"cluster offset %#" PRIx64
+" unaligned (L2 offset: %#"
 PRIx64 ", L2 index: %#x)",
-offset & L2E_OFFSET_MASK,
-l2_offset, j);
+offset, l2_offset, j);
 ret = -EIO;
 goto fail;
 }

-cluster_index = (offset & L2E_OFFSET_MASK) >> 
s->cluster_bits;
+cluster_index = offset >> s->cluster_bits;
 if (!cluster_index) {
 /* unallocated */
 refcount = 0;
@@ -1184,14 +1186,14 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
 }

 if (refcount == 1) {
-offset |= QCOW_OFLAG_COPIED;
+entry |= QCOW_OFLAG_COPIED;
 }
-if (offset != old_offset) {
+if (entry != old_entry) {
 if (addend

[Qemu-devel] slirp: question about using SMB on windows host with linux guest

[FONNEMANN Mark]

Hello-

I am trying to shares files between Windows host and Linux guest using slirp’s 
built-in SMB server.
I start QEMU 2.9.0 using the following:

C:\Program Files\qemu>qemu-system-i386.exe -readconfig 
\Users\mfonnemann\qemu.cfg -net nic -net user,smb=\Users\mfonnemann\smb

I then try to mount the SMB share first with 10.0.2.4:

root@qemu:~# mount -t cifs //10.0.2.4/Users/mfonnemann/smb tmp -o 
user=mfonnemann
Password:
 CIFS VFS: Error connecting to IPv4 socket. Aborting 
operation
 CIFS VFS: cifs_mount failed w/return code = -113
mount error 113 = No route to host
mount: mount.cifs failed with status -1

And then with 10.0.2.2:

root@qemu:~# mount -t cifs //10.0.2.2/Users/mfonnemann/smb tmp -o 
user=mfonnemann
Password:
 CIFS VFS: Unknown RFC 1002 frame

 CIFS VFS: Send error in SessSetup = -13
 CIFS VFS: cifs_mount failed w/return code = -13
mount error 13 = Permission denied
mount: mount.cifs failed with status -1

Regarding RFC 1002 frame, I see the following when using “dmesg”:

CIFS VFS: Unknown RFC 1002 frame
 Received Data: : dump of 36 bytes of data at 0xf71400c0

 008d 57770064 6e006900 6f006400 . . . . d . w W . i . 
n . d . o
 73007700 31002000 20003000 72005000 . w . s .   . 1 . 0 .  
 . P . r
 20006f00 . o .

Any help would be greatly appreciated!!
Thank you very much.

Best Regards / Mit freundlichen Grüßen / 敬具,
Mark.

Re: [Qemu-devel] [PATCH v12 04/10] qcow2: Make distinction between zero cluster types obvious

[Eric Blake]

On 05/05/2017 03:51 PM, Max Reitz wrote:
> On 04.05.2017 05:07, Eric Blake wrote:
>> Treat plain zero clusters differently from allocated ones, so that
>> we can simplify the logic of checking whether an offset is present.
>> Do this by splitting QCOW2_CLUSTER_ZERO into two new enums,
>> QCOW2_CLUSTER_ZERO_PLAIN and QCOW2_CLUSTER_ZERO_ALLOC.
>>
>> I tried to arrange the enum so that we could use
>> 'ret <= QCOW2_CLUSTER_ZERO_PLAIN' for all unallocated types, and
>> 'ret >= QCOW2_CLUSTER_ZERO_ALLOC' for allocated types, although
>> I didn't actually end up taking advantage of the layout.
>>
>> In many cases, this leads to simpler code, by properly combining
>> cases (sometimes, both zero types pair together, other times,
>> plain zero is more like unallocated while allocated zero is more
>> like normal).
>>
>> Signed-off-by: Eric Blake 
>>

>> @@ -558,52 +557,32 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, 
>> uint64_t offset,
>>  assert(nb_clusters <= INT_MAX);
>>
>>  ret = qcow2_get_cluster_type(*cluster_offset);
>> +if (s->qcow_version < 3 && (ret == QCOW2_CLUSTER_ZERO_PLAIN ||
>> +ret == QCOW2_CLUSTER_ZERO_ALLOC)) {
>> +qcow2_signal_corruption(bs, true, -1, -1, "Zero cluster entry found"
>> +" in pre-v3 image (L2 offset: %#" PRIx64
>> +", L2 index: %#x)", l2_offset, l2_index);
>> +ret = -EIO;
>> +goto fail;
>> +}
...
>> +case QCOW2_CLUSTER_ZERO_PLAIN:
>>  case QCOW2_CLUSTER_UNALLOCATED:
>>  /* how many empty clusters ? */
>>  c = count_contiguous_clusters_unallocated(nb_clusters,
>> -  &l2_table[l2_index],
>> -  
>> QCOW2_CLUSTER_UNALLOCATED);
>> +  &l2_table[l2_index], ret);
> 
> Nit pick: Using ret here is a bit weird (because it's such a meaningless
> name). It would be good if we had a separate cluster_type variable.

qcow2_get_cluster_offset() returns the cluster type on success, and
-errno on failure.  So 'ret' actually makes some sense: it really is the
value we are about to return.  But it may also work to have a separate
variable up front, then assign ret = cluster_type at the end; I'll play
with it and see which one looks better.

> 
>>  *cluster_offset = 0;
>>  break;
>> +case QCOW2_CLUSTER_ZERO_ALLOC:
>>  case QCOW2_CLUSTER_NORMAL:
>>  /* how many allocated clusters ? */
>>  c = count_contiguous_clusters(nb_clusters, s->cluster_size,
>> -&l2_table[l2_index], QCOW_OFLAG_ZERO);
>> +  &l2_table[l2_index], QCOW_OFLAG_ZERO);
>>  *cluster_offset &= L2E_OFFSET_MASK;
>>  if (offset_into_cluster(s, *cluster_offset)) {
>>  qcow2_signal_corruption(bs, true, -1, -1, "Data cluster offset 
>> %#"
> 
> Well, preallocated zero clusters are not exactly data clusters... Not
> that any user cared, but s/Data cluster/Cluster allocation/ would be
> more correct.

Good idea.

> 
> By the way, allow me to state just how much I love this hunk: Very much.
> Looks great! It gets a place on my list of favorite hunks of this year
> at least.
> 
> [...]
> 
>> @@ -1760,7 +1740,8 @@ static int expand_zero_clusters_in_l1(BlockDriverState 
>> *bs, uint64_t *l1_table,
>>  int cluster_type = qcow2_get_cluster_type(l2_entry);>   
>>bool preallocated = offset != 0;
> 
> I could get behind removing this variable and replacing all
> "if (!preallocated)" instances by
> "if (cluster_type == QCOW2_CLUSTER_ZERO_PLAIN)". Up to you, though.

Makes sense.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 1/3] hw/block: Introduce rotational qdev property

[Eric Blake]

On 05/06/2017 07:43 AM, Aurelien Jarno wrote:
> The Linux kernel uses different I/O scheduler depending if the block
> device is a rotational device or not. Also it uses rotational devices
> to add entropy to the random pool.
> 
> This patch add a rotational qdev property so that the block device can
> be configured as a rotational device or a non-rotational device. Default
> to true to not change the default behavior.
> 
> Signed-off-by: Aurelien Jarno 
> ---
>  blockdev.c   | 4 
>  include/hw/block/block.h | 4 +++-
>  2 files changed, 7 insertions(+), 1 deletion(-)

Please remember to send a 0/3 cover letter when sending a series (you
can use 'git config format.coverletter auto' to make it easier).

The NBD server has the ability to expose to a client whether a given
exported volume is rotational or not; so there may be some additional
integration you should perform to make nbd-server properly export this
new property.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 2/5] docker: Add bzip2 and hostname to fedora image

[Philippe Mathieu-Daudé]

On 05/05/2017 12:23 AM, Fam Zheng wrote:

It is used by qemu-iotests.

Signed-off-by: Fam Zheng 


Reviewed-by: Philippe Mathieu-Daudé 


---
 tests/docker/dockerfiles/fedora.docker | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/docker/dockerfiles/fedora.docker 
b/tests/docker/dockerfiles/fedora.docker
index c4f80ad..39f6b58 100644
--- a/tests/docker/dockerfiles/fedora.docker
+++ b/tests/docker/dockerfiles/fedora.docker
@@ -1,6 +1,6 @@
 FROM fedora:latest
 ENV PACKAGES \
-ccache git tar PyYAML sparse flex bison python2 \
+ccache git tar PyYAML sparse flex bison python2 bzip2 hostname \
 glib2-devel pixman-devel zlib-devel SDL-devel libfdt-devel \
 gcc gcc-c++ clang make perl which bc findutils \
 mingw32-pixman mingw32-glib2 mingw32-gmp mingw32-SDL mingw32-pkg-config \

Re: [Qemu-devel] [PATCH 3/5] docker: Use unconfined security profile

[Philippe Mathieu-Daudé]

Hi Fam, Alex, Paolo,

On 05/05/2017 12:23 AM, Fam Zheng wrote:

Some by default blocked syscalls are required to run tests for example
userfaultfd.

Signed-off-by: Fam Zheng 
---
 tests/docker/Makefile.include | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 0ed8c3d..09d157c 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -127,6 +127,7 @@ docker-run: docker-qemu-src
$(call quiet-command,   \
$(SRC_PATH)/tests/docker/docker.py run  \
$(if $(NOUSER),,-u $(shell id -u)) -t   \
+   --security-opt seccomp=unconfined   \


I think this should be an option in the matrix, and eventually run tests 
using userfaultfd() apart.


$(if $(UNCONFINED),,--security-opt seccomp=unconfined)

I'm having the same problem with getcontext() using x32 ABI.


$(if $V,,--rm)  \
$(if $(DEBUG),-i,--net=none)\
-e TARGET_LIST=$(TARGET_LIST)   \



Regards,

Phil.

Re: [Qemu-devel] [PATCH 4/5] docker: Add libaio to fedora image

[Philippe Mathieu-Daudé]

On 05/05/2017 12:23 AM, Fam Zheng wrote:

Signed-off-by: Fam Zheng 


Reviewed-by: Philippe Mathieu-Daudé 


---
 tests/docker/dockerfiles/fedora.docker | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/docker/dockerfiles/fedora.docker 
b/tests/docker/dockerfiles/fedora.docker
index 39f6b58..4eaa8ed 100644
--- a/tests/docker/dockerfiles/fedora.docker
+++ b/tests/docker/dockerfiles/fedora.docker
@@ -2,7 +2,7 @@ FROM fedora:latest
 ENV PACKAGES \
 ccache git tar PyYAML sparse flex bison python2 bzip2 hostname \
 glib2-devel pixman-devel zlib-devel SDL-devel libfdt-devel \
-gcc gcc-c++ clang make perl which bc findutils \
+gcc gcc-c++ clang make perl which bc findutils libaio-devel \
 mingw32-pixman mingw32-glib2 mingw32-gmp mingw32-SDL mingw32-pkg-config \
 mingw32-gtk2 mingw32-gtk3 mingw32-gnutls mingw32-nettle mingw32-libtasn1 \
 mingw32-libjpeg-turbo mingw32-libpng mingw32-curl mingw32-libssh2 \

Re: [Qemu-devel] [PATCH 1/5] docker: Run tests with current user

[Philippe Mathieu-Daudé]

On 05/05/2017 12:23 AM, Fam Zheng wrote:

We've used --add-current-user to create a user in the image, use it to
run tests, because root has too much priviledge, and can surprise test
cases.

Signed-off-by: Fam Zheng 


Reviewed-by: Philippe Mathieu-Daudé 


---
 tests/docker/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 03eda37..0ed8c3d 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -126,7 +126,7 @@ docker-run: docker-qemu-src
"  COPYING $(EXECUTABLE) to $(IMAGE)"))
$(call quiet-command,   \
$(SRC_PATH)/tests/docker/docker.py run  \
-   -t  \
+   $(if $(NOUSER),,-u $(shell id -u)) -t   \
$(if $V,,--rm)  \
$(if $(DEBUG),-i,--net=none)\
-e TARGET_LIST=$(TARGET_LIST)   \

Re: [Qemu-devel] [PATCH v2 10/14] target/sh4: optimize gen_write_sr using extract op

[Philippe Mathieu-Daudé]

Lovely!

What about a Coccinelle meta-script to catch similar (ab)uses?

Phil.

On 05/06/2017 08:14 AM, Aurelien Jarno wrote:

This doesn't change the generated code on x86, but optimizes it on most
RISC architectures and makes the code simpler to read.

Signed-off-by: Aurelien Jarno 


Reviewed-by: Philippe Mathieu-Daudé 


---
 target/sh4/translate.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index b4e5606098..8c766eed2a 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -204,12 +204,9 @@ static void gen_write_sr(TCGv src)
 {
 tcg_gen_andi_i32(cpu_sr, src,
  ~((1u << SR_Q) | (1u << SR_M) | (1u << SR_T)));
-tcg_gen_shri_i32(cpu_sr_q, src, SR_Q);
-tcg_gen_andi_i32(cpu_sr_q, cpu_sr_q, 1);
-tcg_gen_shri_i32(cpu_sr_m, src, SR_M);
-tcg_gen_andi_i32(cpu_sr_m, cpu_sr_m, 1);
-tcg_gen_shri_i32(cpu_sr_t, src, SR_T);
-tcg_gen_andi_i32(cpu_sr_t, cpu_sr_t, 1);
+tcg_gen_extract_i32(cpu_sr_q, src, SR_Q, 1);
+tcg_gen_extract_i32(cpu_sr_m, src, SR_M, 1);
+tcg_gen_extract_i32(cpu_sr_t, src, SR_T, 1);
 }

 static inline void gen_save_cpu_state(DisasContext *ctx, bool save_pc)

Re: [Qemu-devel] [PATCH v2 06/14] target/sh4: fix BS_EXCP exit

[Philippe Mathieu-Daudé]

On 05/06/2017 08:14 AM, Aurelien Jarno wrote:

In case of exception, there is no need to call tcg_gen_exit_tb as the
exception helper won't return.

Also fix a few cases where BS_BRANCH is called instead of BS_EXCP.

Signed-off-by: Aurelien Jarno 


Reviewed-by: Philippe Mathieu-Daudé 


---
 target/sh4/translate.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 04bc18bf7c..f608e314b6 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -339,7 +339,7 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
 if (ctx->envflags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) { \
 tcg_gen_movi_i32(cpu_pc, ctx->pc);   \
 gen_helper_raise_slot_illegal_instruction(cpu_env);  \
-ctx->bstate = BS_BRANCH; \
+ctx->bstate = BS_EXCP;   \
 return;  \
 }

@@ -351,7 +351,7 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
 } else { \
 gen_helper_raise_illegal_instruction(cpu_env);   \
 }\
-ctx->bstate = BS_BRANCH; \
+ctx->bstate = BS_EXCP;   \
 return;  \
 }

@@ -363,7 +363,7 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
 } else { \
 gen_helper_raise_fpu_disable(cpu_env);   \
 }\
-ctx->bstate = BS_BRANCH; \
+ctx->bstate = BS_EXCP;   \
 return;  \
 }

@@ -1289,7 +1289,7 @@ static void _decode_opc(DisasContext * ctx)
imm = tcg_const_i32(B7_0);
 gen_helper_trapa(cpu_env, imm);
tcg_temp_free(imm);
-   ctx->bstate = BS_BRANCH;
+ctx->bstate = BS_EXCP;
}
return;
 case 0xc800:   /* tst #imm,R0 */
@@ -1798,7 +1798,7 @@ static void _decode_opc(DisasContext * ctx)
 } else {
 gen_helper_raise_illegal_instruction(cpu_env);
 }
-ctx->bstate = BS_BRANCH;
+ctx->bstate = BS_EXCP;
 }

 static void decode_opc(DisasContext * ctx)
@@ -1867,7 +1867,7 @@ void gen_intermediate_code(CPUSH4State * env, struct 
TranslationBlock *tb)
 /* We have hit a breakpoint - make sure PC is up-to-date */
 tcg_gen_movi_i32(cpu_pc, ctx.pc);
 gen_helper_debug(cpu_env);
-ctx.bstate = BS_BRANCH;
+ctx.bstate = BS_EXCP;
 /* The address covered by the breakpoint must be included in
[tb->pc, tb->pc + tb->size) in order to for it to be
properly cleared -- thus we increment the PC here so that
@@ -1911,9 +1911,7 @@ void gen_intermediate_code(CPUSH4State * env, struct 
TranslationBlock *tb)
 gen_goto_tb(&ctx, 0, ctx.pc);
 break;
 case BS_EXCP:
-/* gen_op_interrupt_restart(); */
-tcg_gen_exit_tb(0);
-break;
+/* fall through */
 case BS_BRANCH:
 default:
 break;

Re: [Qemu-devel] [PATCH v2 03/14] target/sh4: do not include DELAY_SLOT_TRUE in the TB state

[Philippe Mathieu-Daudé]

On 05/06/2017 08:14 AM, Aurelien Jarno wrote:

DELAY_SLOT_TRUE is used as a dynamic condition for the branch after the
delay slot instruction. It is not used in code generation, so there is
no need to including in the TB state.

Signed-off-by: Aurelien Jarno 


Reviewed-by: Philippe Mathieu-Daudé 


---
 target/sh4/cpu.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/sh4/cpu.h b/target/sh4/cpu.h
index 9445cc779f..da8d15f1b9 100644
--- a/target/sh4/cpu.h
+++ b/target/sh4/cpu.h
@@ -382,8 +382,7 @@ static inline void cpu_get_tb_cpu_state(CPUSH4State *env, 
target_ulong *pc,
 {
 *pc = env->pc;
 *cs_base = 0;
-*flags = (env->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL
-| DELAY_SLOT_TRUE))/* Bits  0- 2 */
+*flags = (env->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) /* Bits 0-1 
*/
 | (env->fpscr & (FPSCR_FR | FPSCR_SZ | FPSCR_PR))  /* Bits 19-21 */
 | (env->sr & ((1u << SR_MD) | (1u << SR_RB)))  /* Bits 29-30 */
 | (env->sr & (1u << SR_FD))/* Bit 15 */

Re: [Qemu-devel] [PATCH v2 01/14] target/sh4: split ctx->flags into ctx->tbflags and ctx->envflags

[Philippe Mathieu-Daudé]

On 05/06/2017 08:14 AM, Aurelien Jarno wrote:

There is a confusion (and not only in the SH4 target) between tb->flags,
env->flags and ctx->flags. To avoid it, split ctx->flags into
ctx->tbflags and ctx->envflags. ctx->tbflags stays unchanged during the
whole TB translation, while ctx->envflags evolves and is kept in sync
with env->flags using TCG instructions. ctx->envflags now only contains
the part that of env->flags that is contained in the TB state, i.e. the
DELAY_SLOT* flags.

Signed-off-by: Aurelien Jarno 


Reviewed-by: Philippe Mathieu-Daudé 


---
 target/sh4/translate.c | 161 +
 1 file changed, 82 insertions(+), 79 deletions(-)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index c89a14733f..6b526a02f2 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -37,7 +37,8 @@ typedef struct DisasContext {
 struct TranslationBlock *tb;
 target_ulong pc;
 uint16_t opcode;
-uint32_t flags;
+uint32_t tbflags;/* should stay unmodified during the TB translation */
+uint32_t envflags;   /* should stay in sync with env->flags using TCG ops 
*/
 int bstate;
 int memidx;
 uint32_t delayed_pc;
@@ -49,7 +50,7 @@ typedef struct DisasContext {
 #if defined(CONFIG_USER_ONLY)
 #define IS_USER(ctx) 1
 #else
-#define IS_USER(ctx) (!(ctx->flags & (1u << SR_MD)))
+#define IS_USER(ctx) (!(ctx->tbflags & (1u << SR_MD)))
 #endif

 enum {
@@ -317,51 +318,50 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
 #define B11_8 ((ctx->opcode >> 8) & 0xf)
 #define B15_12 ((ctx->opcode >> 12) & 0xf)

-#define REG(x) ((x) < 8 && (ctx->flags & (1u << SR_MD))\
-&& (ctx->flags & (1u << SR_RB))\
+#define REG(x) ((x) < 8 && (ctx->tbflags & (1u << SR_MD))\
+&& (ctx->tbflags & (1u << SR_RB))\
 ? (cpu_gregs[x + 16]) : (cpu_gregs[x]))

-#define ALTREG(x) ((x) < 8 && (!(ctx->flags & (1u << SR_MD))\
-   || !(ctx->flags & (1u << SR_RB)))\
+#define ALTREG(x) ((x) < 8 && (!(ctx->tbflags & (1u << SR_MD))\
+   || !(ctx->tbflags & (1u << SR_RB)))\
? (cpu_gregs[x + 16]) : (cpu_gregs[x]))

-#define FREG(x) (ctx->flags & FPSCR_FR ? (x) ^ 0x10 : (x))
+#define FREG(x) (ctx->tbflags & FPSCR_FR ? (x) ^ 0x10 : (x))
 #define XHACK(x) x) & 1 ) << 4) | ((x) & 0xe))
-#define XREG(x) (ctx->flags & FPSCR_FR ? XHACK(x) ^ 0x10 : XHACK(x))
+#define XREG(x) (ctx->tbflags & FPSCR_FR ? XHACK(x) ^ 0x10 : XHACK(x))
 #define DREG(x) FREG(x) /* Assumes lsb of (x) is always 0 */

 #define CHECK_NOT_DELAY_SLOT \
-  if (ctx->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) \
-  {   \
-  tcg_gen_movi_i32(cpu_pc, ctx->pc);  \
-  gen_helper_raise_slot_illegal_instruction(cpu_env); \
-  ctx->bstate = BS_BRANCH;\
-  return; \
-  }
-
-#define CHECK_PRIVILEGED\
-  if (IS_USER(ctx)) {   \
-  tcg_gen_movi_i32(cpu_pc, ctx->pc);\
-  if (ctx->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) { \
-  gen_helper_raise_slot_illegal_instruction(cpu_env);   \
-  } else {  \
-  gen_helper_raise_illegal_instruction(cpu_env);\
-  } \
-  ctx->bstate = BS_BRANCH;  \
-  return;   \
-  }
-
-#define CHECK_FPU_ENABLED   \
-  if (ctx->flags & (1u << SR_FD)) { \
-  tcg_gen_movi_i32(cpu_pc, ctx->pc);\
-  if (ctx->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) { \
-  gen_helper_raise_slot_fpu_disable(cpu_env);   \
-  } else {  \
-  gen_helper_raise_fpu_disable(cpu_env);\
-  } \
-  ctx->bstate = BS_BRANCH;  \
-  return;   \
-  }
+if (ctx->envflags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) { \
+tcg_gen_movi_i32(cpu_pc, ctx->pc);   \
+gen_helper_raise_slot_illegal_instruction(cpu_env);  \
+ctx->bstate = BS_BRANCH; \
+return;  \
+}
+
+#define CHECK_PRIVILEGED \
+if (IS_USER(ctx)) {  \
+tcg_gen_movi_i32(cpu_pc, ctx->pc);   \
+if (

Re: [Qemu-devel] [Qemu-arm] [PATCH 1/3] hw/arm/exynos: Move RAM initialization next boards

[Krzysztof Kozlowski]

On Sat, May 06, 2017 at 01:05:19PM -0300, Philippe Mathieu-Daudé wrote:
> Hi Krzysztof,
> 
> Please modify RAM -> DRAM in subject/description so there is no confusion
> about also moving the IRAM (which is SoC specific).
> 
> With this:
> Reviewed-by: Philippe Mathieu-Daudé 

Sure, thanks for review.

Best regards,
Krzysztof

Re: [Qemu-devel] [PATCH 3/3] ide: export rotational qdev property

[Philippe Mathieu-Daudé]

On 05/06/2017 09:43 AM, Aurelien Jarno wrote:

Export the rotational qdev property in the IDENTIFY request.

Signed-off-by: Aurelien Jarno 


Reviewed-by: Philippe Mathieu-Daudé 


---
 hw/ide/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 0b48b64d3a..1aa76b0d90 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -191,6 +191,9 @@ static void ide_identify(IDEState *s)
 if (dev && dev->conf.discard_granularity) {
 put_le16(p + 169, 1); /* TRIM support */
 }
+if (dev && !dev->conf.rotational) {
+put_le16(p + 217, 1); /* non-rotating device */
+}

 ide_identify_size(s);
 s->identify_set = 1;

Re: [Qemu-devel] [PATCH 1/3] hw/block: Introduce rotational qdev property

[Philippe Mathieu-Daudé]

On 05/06/2017 09:43 AM, Aurelien Jarno wrote:

The Linux kernel uses different I/O scheduler depending if the block
device is a rotational device or not. Also it uses rotational devices
to add entropy to the random pool.

This patch add a rotational qdev property so that the block device can
be configured as a rotational device or a non-rotational device. Default
to true to not change the default behavior.

Signed-off-by: Aurelien Jarno 


Reviewed-by: Philippe Mathieu-Daudé 


---
 blockdev.c   | 4 
 include/hw/block/block.h | 4 +++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index 4d8cdedd54..c914420641 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -4037,6 +4037,10 @@ QemuOptsList qemu_common_drive_opts = {
 .name = BDRV_OPT_READ_ONLY,
 .type = QEMU_OPT_BOOL,
 .help = "open drive file as read-only",
+},{
+.name = "rotational",
+.type = QEMU_OPT_BOOL,
+.help = "rotational drive (off, on)",
 },

 THROTTLE_OPTS,
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index f3f6e8ef02..304a579eca 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -27,6 +27,7 @@ typedef struct BlockConf {
 uint32_t cyls, heads, secs;
 OnOffAuto wce;
 bool share_rw;
+bool rotational;
 BlockdevOnError rerror;
 BlockdevOnError werror;
 } BlockConf;
@@ -56,7 +57,8 @@ static inline unsigned int get_physical_block_exp(BlockConf 
*conf)
_conf.discard_granularity, -1), \
 DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, \
 ON_OFF_AUTO_AUTO), \
-DEFINE_PROP_BOOL("share-rw", _state, _conf.share_rw, false)
+DEFINE_PROP_BOOL("share-rw", _state, _conf.share_rw, false), \
+DEFINE_PROP_BOOL("rotational", _state, _conf.rotational, true)

 #define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf)  \
 DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0),  \

Re: [Qemu-devel] [PATCH 2/3] scsi-disk: export rotational qdev property

[Philippe Mathieu-Daudé]

On 05/06/2017 09:43 AM, Aurelien Jarno wrote:

Export the rotational qdev property to the block device characteristics
VPD page.

Signed-off-by: Aurelien Jarno 


Reviewed-by: Philippe Mathieu-Daudé 


---
 hw/scsi/scsi-disk.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index a53f058621..21b7e21a23 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -597,6 +597,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, 
uint8_t *outbuf)
 outbuf[buflen++] = 0x83; // device identification
 if (s->qdev.type == TYPE_DISK) {
 outbuf[buflen++] = 0xb0; // block limits
+outbuf[buflen++] = 0xb1; /* block device characteristics */
 outbuf[buflen++] = 0xb2; // thin provisioning
 }
 break;
@@ -739,6 +740,19 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, 
uint8_t *outbuf)
 outbuf[43] = max_io_sectors & 0xff;
 break;
 }
+case 0xb1: /* block device characteristics */
+{
+buflen = 0x40;
+memset(outbuf + 4, 0, buflen - 4);
+
+/* medium rotation rate: 0 = not reported, 1 = non-rotating */
+outbuf[4] = 0;
+outbuf[5] = s->qdev.conf.rotational ? 0 : 1;
+
+/* nominal form factor */
+outbuf[7] = 0; /* not reported */
+break;
+}
 case 0xb2: /* thin provisioning */
 {
 buflen = 8;

Re: [Qemu-devel] [PATCH 3/3] hw/arm/exynos: QOM-ify the SoC

[Philippe Mathieu-Daudé]

On 05/06/2017 12:24 PM, Krzysztof Kozlowski wrote:

Convert the Exynos4210 SoC code into a QOM model which is a preferred
approach instead of directly initializing SoC-related devices from the
board file.

Signed-off-by: Krzysztof Kozlowski 


Reviewed-by: Philippe Mathieu-Daudé 


---
 hw/arm/exynos4210.c | 18 +++---
 hw/arm/exynos4_boards.c |  9 ++---
 include/hw/arm/exynos4210.h |  8 ++--
 3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 27a7bf28a5a9..034fc8be9d76 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -160,9 +160,10 @@ static uint64_t exynos4210_calc_affinity(int cpu)
 return mp_affinity;
 }

-Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
+static void exynos4210_init(Object *obj)
 {
-Exynos4210State *s = g_new(Exynos4210State, 1);
+MemoryRegion *system_mem = get_system_memory();
+Exynos4210State *s = EXYNOS4210(obj);
 qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];
 SysBusDevice *busdev;
 ObjectClass *cpu_oc;
@@ -402,6 +403,17 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem)

 sysbus_create_simple(TYPE_EXYNOS4210_EHCI, EXYNOS4210_EHCI_BASE_ADDR,
 s->irq_table[exynos4210_get_irq(28, 3)]);
+}
+
+static const TypeInfo exynos4210_type_info = {
+.name = TYPE_EXYNOS4210,
+.parent = TYPE_DEVICE,
+.instance_size = sizeof(Exynos4210State),
+.instance_init = exynos4210_init,
+};

-return s;
+static void exynos4210_register_types(void)
+{
+type_register_static(&exynos4210_type_info);
 }
+type_init(exynos4210_register_types)
diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
index 6240b26839cd..5e7c6b562ae2 100644
--- a/hw/arm/exynos4_boards.c
+++ b/hw/arm/exynos4_boards.c
@@ -58,7 +58,7 @@ typedef enum Exynos4BoardType {
 } Exynos4BoardType;

 typedef struct Exynos4BoardState {
-Exynos4210State *soc;
+Exynos4210State soc;
 MemoryRegion dram0_mem;
 MemoryRegion dram1_mem;
 } Exynos4BoardState;
@@ -162,7 +162,10 @@ exynos4_boards_init_common(MachineState *machine,
 exynos4_boards_init_ram(s, get_system_memory(),
 exynos4_board_ram_size[board_type]);

-s->soc = exynos4210_init(get_system_memory());
+object_initialize(&s->soc, sizeof(s->soc), TYPE_EXYNOS4210);
+object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
+  &error_abort);
+object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);

 return s;
 }
@@ -180,7 +183,7 @@ static void smdkc210_init(MachineState *machine)
   EXYNOS4_BOARD_SMDKC210);

 lan9215_init(SMDK_LAN9118_BASE_ADDR,
-qemu_irq_invert(s->soc->irq_table[exynos4210_get_irq(37, 1)]));
+qemu_irq_invert(s->soc.irq_table[exynos4210_get_irq(37, 1)]));
 arm_load_kernel(ARM_CPU(first_cpu), &exynos4_board_binfo);
 }

diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index 098a69ec73d3..116eae62756b 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -29,6 +29,10 @@
 #include "exec/memory.h"
 #include "target/arm/cpu-qom.h"

+#define TYPE_EXYNOS4210 "exynos4210"
+#define EXYNOS4210(obj) \
+OBJECT_CHECK(Exynos4210State, (obj), TYPE_EXYNOS4210)
+
 #define EXYNOS4210_NCPUS2

 #define EXYNOS4210_DRAM0_BASE_ADDR  0x4000
@@ -85,6 +89,8 @@ typedef struct Exynos4210Irq {
 } Exynos4210Irq;

 typedef struct Exynos4210State {
+DeviceState parent_obj;
+
 ARMCPU *cpu[EXYNOS4210_NCPUS];
 Exynos4210Irq irqs;
 qemu_irq *irq_table;
@@ -101,8 +107,6 @@ typedef struct Exynos4210State {
 void exynos4210_write_secondary(ARMCPU *cpu,
 const struct arm_boot_info *info);

-Exynos4210State *exynos4210_init(MemoryRegion *system_mem);
-
 /* Initialize exynos4210 IRQ subsystem stub */
 qemu_irq *exynos4210_init_irq(Exynos4210Irq *env);

Re: [Qemu-devel] [PATCH 2/3] hw/arm/exynos: Declare local variables in some order

[Philippe Mathieu-Daudé]

Aesthetic.

Reviewed-by: Philippe Mathieu-Daudé 

On 05/06/2017 12:24 PM, Krzysztof Kozlowski wrote:

Bring some more readability by declaring local function variables, first
initialized ones and then the rest (with reversed-christmas-tree order).

Signed-off-by: Krzysztof Kozlowski 
---
 hw/arm/exynos4210.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 0da877f8db0a..27a7bf28a5a9 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -162,12 +162,12 @@ static uint64_t exynos4210_calc_affinity(int cpu)

 Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
 {
-int i, n;
 Exynos4210State *s = g_new(Exynos4210State, 1);
 qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];
-DeviceState *dev;
 SysBusDevice *busdev;
 ObjectClass *cpu_oc;
+DeviceState *dev;
+int i, n;

 cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, "cortex-a9");
 assert(cpu_oc);

Re: [Qemu-devel] [Qemu-arm] [PATCH 1/3] hw/arm/exynos: Move RAM initialization next boards

[Philippe Mathieu-Daudé]

Hi Krzysztof,

Please modify RAM -> DRAM in subject/description so there is no 
confusion about also moving the IRAM (which is SoC specific).


With this:
Reviewed-by: Philippe Mathieu-Daudé 

On 05/06/2017 12:24 PM, Krzysztof Kozlowski wrote:

Before QOM-ifying the Exynos4 SoC model, move the RAM initialization
from exynos4210.c to exynos4_boards.c because RAM is board specific,
not SoC.

Signed-off-by: Krzysztof Kozlowski 
---
 hw/arm/exynos4210.c | 20 +-
 hw/arm/exynos4_boards.c | 50 ++---
 include/hw/arm/exynos4210.h |  5 +
 3 files changed, 45 insertions(+), 30 deletions(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 960f27e45a36..0da877f8db0a 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -160,13 +160,11 @@ static uint64_t exynos4210_calc_affinity(int cpu)
 return mp_affinity;
 }

-Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
-unsigned long ram_size)
+Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
 {
 int i, n;
 Exynos4210State *s = g_new(Exynos4210State, 1);
 qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];
-unsigned long mem_size;
 DeviceState *dev;
 SysBusDevice *busdev;
 ObjectClass *cpu_oc;
@@ -299,22 +297,6 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
 memory_region_add_subregion(system_mem, EXYNOS4210_IRAM_BASE_ADDR,
 &s->iram_mem);

-/* DRAM */
-mem_size = ram_size;
-if (mem_size > EXYNOS4210_DRAM_MAX_SIZE) {
-memory_region_init_ram(&s->dram1_mem, NULL, "exynos4210.dram1",
-mem_size - EXYNOS4210_DRAM_MAX_SIZE, &error_fatal);
-vmstate_register_ram_global(&s->dram1_mem);
-memory_region_add_subregion(system_mem, EXYNOS4210_DRAM1_BASE_ADDR,
-&s->dram1_mem);
-mem_size = EXYNOS4210_DRAM_MAX_SIZE;
-}
-memory_region_init_ram(&s->dram0_mem, NULL, "exynos4210.dram0", mem_size,
-   &error_fatal);
-vmstate_register_ram_global(&s->dram0_mem);
-memory_region_add_subregion(system_mem, EXYNOS4210_DRAM0_BASE_ADDR,
-&s->dram0_mem);
-
/* PMU.
 * The only reason of existence at the moment is that secondary CPU boot
 * loader uses PMU INFORM5 register as a holding pen.
diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
index 4853c318023c..6240b26839cd 100644
--- a/hw/arm/exynos4_boards.c
+++ b/hw/arm/exynos4_boards.c
@@ -22,6 +22,7 @@
  */

 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "qemu-common.h"
 #include "cpu.h"
@@ -56,6 +57,12 @@ typedef enum Exynos4BoardType {
 EXYNOS4_NUM_OF_BOARDS
 } Exynos4BoardType;

+typedef struct Exynos4BoardState {
+Exynos4210State *soc;
+MemoryRegion dram0_mem;
+MemoryRegion dram1_mem;
+} Exynos4BoardState;
+
 static int exynos4_board_id[EXYNOS4_NUM_OF_BOARDS] = {
 [EXYNOS4_BOARD_NURI] = 0xD33,
 [EXYNOS4_BOARD_SMDKC210] = 0xB16,
@@ -96,9 +103,34 @@ static void lan9215_init(uint32_t base, qemu_irq irq)
 }
 }

-static Exynos4210State *exynos4_boards_init_common(MachineState *machine,
-   Exynos4BoardType board_type)
+static void exynos4_boards_init_ram(Exynos4BoardState *s,
+MemoryRegion *system_mem,
+unsigned long ram_size)
+{
+unsigned long mem_size = ram_size;
+
+if (mem_size > EXYNOS4210_DRAM_MAX_SIZE) {
+memory_region_init_ram(&s->dram1_mem, NULL, "exynos4210.dram1",
+   mem_size - EXYNOS4210_DRAM_MAX_SIZE,
+   &error_fatal);
+vmstate_register_ram_global(&s->dram1_mem);
+memory_region_add_subregion(system_mem, EXYNOS4210_DRAM1_BASE_ADDR,
+&s->dram1_mem);
+mem_size = EXYNOS4210_DRAM_MAX_SIZE;
+}
+
+memory_region_init_ram(&s->dram0_mem, NULL, "exynos4210.dram0", mem_size,
+   &error_fatal);
+vmstate_register_ram_global(&s->dram0_mem);
+memory_region_add_subregion(system_mem, EXYNOS4210_DRAM0_BASE_ADDR,
+&s->dram0_mem);
+}
+
+static Exynos4BoardState *
+exynos4_boards_init_common(MachineState *machine,
+   Exynos4BoardType board_type)
 {
+Exynos4BoardState *s = g_new(Exynos4BoardState, 1);
 MachineClass *mc = MACHINE_GET_CLASS(machine);

 if (smp_cpus != EXYNOS4210_NCPUS && !qtest_enabled()) {
@@ -127,8 +159,12 @@ static Exynos4210State 
*exynos4_boards_init_common(MachineState *machine,
 machine->kernel_cmdline,
 machine->initrd_filename);

-return exynos4210_init(get_system_memory(),
-exynos4_board_ram_size[board_type]);
+exynos4_boards_init_ram(s, get_system_memory(),
+exynos4_board_ram_si

[Qemu-devel] [PATCH 1/3] hw/arm/exynos: Move RAM initialization next boards

[Krzysztof Kozlowski]

Before QOM-ifying the Exynos4 SoC model, move the RAM initialization
from exynos4210.c to exynos4_boards.c because RAM is board specific,
not SoC.

Signed-off-by: Krzysztof Kozlowski 
---
 hw/arm/exynos4210.c | 20 +-
 hw/arm/exynos4_boards.c | 50 ++---
 include/hw/arm/exynos4210.h |  5 +
 3 files changed, 45 insertions(+), 30 deletions(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 960f27e45a36..0da877f8db0a 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -160,13 +160,11 @@ static uint64_t exynos4210_calc_affinity(int cpu)
 return mp_affinity;
 }
 
-Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
-unsigned long ram_size)
+Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
 {
 int i, n;
 Exynos4210State *s = g_new(Exynos4210State, 1);
 qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];
-unsigned long mem_size;
 DeviceState *dev;
 SysBusDevice *busdev;
 ObjectClass *cpu_oc;
@@ -299,22 +297,6 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
 memory_region_add_subregion(system_mem, EXYNOS4210_IRAM_BASE_ADDR,
 &s->iram_mem);
 
-/* DRAM */
-mem_size = ram_size;
-if (mem_size > EXYNOS4210_DRAM_MAX_SIZE) {
-memory_region_init_ram(&s->dram1_mem, NULL, "exynos4210.dram1",
-mem_size - EXYNOS4210_DRAM_MAX_SIZE, &error_fatal);
-vmstate_register_ram_global(&s->dram1_mem);
-memory_region_add_subregion(system_mem, EXYNOS4210_DRAM1_BASE_ADDR,
-&s->dram1_mem);
-mem_size = EXYNOS4210_DRAM_MAX_SIZE;
-}
-memory_region_init_ram(&s->dram0_mem, NULL, "exynos4210.dram0", mem_size,
-   &error_fatal);
-vmstate_register_ram_global(&s->dram0_mem);
-memory_region_add_subregion(system_mem, EXYNOS4210_DRAM0_BASE_ADDR,
-&s->dram0_mem);
-
/* PMU.
 * The only reason of existence at the moment is that secondary CPU boot
 * loader uses PMU INFORM5 register as a holding pen.
diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
index 4853c318023c..6240b26839cd 100644
--- a/hw/arm/exynos4_boards.c
+++ b/hw/arm/exynos4_boards.c
@@ -22,6 +22,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "qemu-common.h"
 #include "cpu.h"
@@ -56,6 +57,12 @@ typedef enum Exynos4BoardType {
 EXYNOS4_NUM_OF_BOARDS
 } Exynos4BoardType;
 
+typedef struct Exynos4BoardState {
+Exynos4210State *soc;
+MemoryRegion dram0_mem;
+MemoryRegion dram1_mem;
+} Exynos4BoardState;
+
 static int exynos4_board_id[EXYNOS4_NUM_OF_BOARDS] = {
 [EXYNOS4_BOARD_NURI] = 0xD33,
 [EXYNOS4_BOARD_SMDKC210] = 0xB16,
@@ -96,9 +103,34 @@ static void lan9215_init(uint32_t base, qemu_irq irq)
 }
 }
 
-static Exynos4210State *exynos4_boards_init_common(MachineState *machine,
-   Exynos4BoardType board_type)
+static void exynos4_boards_init_ram(Exynos4BoardState *s,
+MemoryRegion *system_mem,
+unsigned long ram_size)
+{
+unsigned long mem_size = ram_size;
+
+if (mem_size > EXYNOS4210_DRAM_MAX_SIZE) {
+memory_region_init_ram(&s->dram1_mem, NULL, "exynos4210.dram1",
+   mem_size - EXYNOS4210_DRAM_MAX_SIZE,
+   &error_fatal);
+vmstate_register_ram_global(&s->dram1_mem);
+memory_region_add_subregion(system_mem, EXYNOS4210_DRAM1_BASE_ADDR,
+&s->dram1_mem);
+mem_size = EXYNOS4210_DRAM_MAX_SIZE;
+}
+
+memory_region_init_ram(&s->dram0_mem, NULL, "exynos4210.dram0", mem_size,
+   &error_fatal);
+vmstate_register_ram_global(&s->dram0_mem);
+memory_region_add_subregion(system_mem, EXYNOS4210_DRAM0_BASE_ADDR,
+&s->dram0_mem);
+}
+
+static Exynos4BoardState *
+exynos4_boards_init_common(MachineState *machine,
+   Exynos4BoardType board_type)
 {
+Exynos4BoardState *s = g_new(Exynos4BoardState, 1);
 MachineClass *mc = MACHINE_GET_CLASS(machine);
 
 if (smp_cpus != EXYNOS4210_NCPUS && !qtest_enabled()) {
@@ -127,8 +159,12 @@ static Exynos4210State 
*exynos4_boards_init_common(MachineState *machine,
 machine->kernel_cmdline,
 machine->initrd_filename);
 
-return exynos4210_init(get_system_memory(),
-exynos4_board_ram_size[board_type]);
+exynos4_boards_init_ram(s, get_system_memory(),
+exynos4_board_ram_size[board_type]);
+
+s->soc = exynos4210_init(get_system_memory());
+
+return s;
 }
 
 static void nuri_init(MachineState *machine)
@@ -140,11 +176,11 @@ static void nuri_init(MachineState *machine)
 
 static void smdkc210_init(Mach

[Qemu-devel] [PATCH 3/3] hw/arm/exynos: QOM-ify the SoC

[Krzysztof Kozlowski]

Convert the Exynos4210 SoC code into a QOM model which is a preferred
approach instead of directly initializing SoC-related devices from the
board file.

Signed-off-by: Krzysztof Kozlowski 
---
 hw/arm/exynos4210.c | 18 +++---
 hw/arm/exynos4_boards.c |  9 ++---
 include/hw/arm/exynos4210.h |  8 ++--
 3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 27a7bf28a5a9..034fc8be9d76 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -160,9 +160,10 @@ static uint64_t exynos4210_calc_affinity(int cpu)
 return mp_affinity;
 }
 
-Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
+static void exynos4210_init(Object *obj)
 {
-Exynos4210State *s = g_new(Exynos4210State, 1);
+MemoryRegion *system_mem = get_system_memory();
+Exynos4210State *s = EXYNOS4210(obj);
 qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];
 SysBusDevice *busdev;
 ObjectClass *cpu_oc;
@@ -402,6 +403,17 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
 
 sysbus_create_simple(TYPE_EXYNOS4210_EHCI, EXYNOS4210_EHCI_BASE_ADDR,
 s->irq_table[exynos4210_get_irq(28, 3)]);
+}
+
+static const TypeInfo exynos4210_type_info = {
+.name = TYPE_EXYNOS4210,
+.parent = TYPE_DEVICE,
+.instance_size = sizeof(Exynos4210State),
+.instance_init = exynos4210_init,
+};
 
-return s;
+static void exynos4210_register_types(void)
+{
+type_register_static(&exynos4210_type_info);
 }
+type_init(exynos4210_register_types)
diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
index 6240b26839cd..5e7c6b562ae2 100644
--- a/hw/arm/exynos4_boards.c
+++ b/hw/arm/exynos4_boards.c
@@ -58,7 +58,7 @@ typedef enum Exynos4BoardType {
 } Exynos4BoardType;
 
 typedef struct Exynos4BoardState {
-Exynos4210State *soc;
+Exynos4210State soc;
 MemoryRegion dram0_mem;
 MemoryRegion dram1_mem;
 } Exynos4BoardState;
@@ -162,7 +162,10 @@ exynos4_boards_init_common(MachineState *machine,
 exynos4_boards_init_ram(s, get_system_memory(),
 exynos4_board_ram_size[board_type]);
 
-s->soc = exynos4210_init(get_system_memory());
+object_initialize(&s->soc, sizeof(s->soc), TYPE_EXYNOS4210);
+object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
+  &error_abort);
+object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
 
 return s;
 }
@@ -180,7 +183,7 @@ static void smdkc210_init(MachineState *machine)
   EXYNOS4_BOARD_SMDKC210);
 
 lan9215_init(SMDK_LAN9118_BASE_ADDR,
-qemu_irq_invert(s->soc->irq_table[exynos4210_get_irq(37, 1)]));
+qemu_irq_invert(s->soc.irq_table[exynos4210_get_irq(37, 1)]));
 arm_load_kernel(ARM_CPU(first_cpu), &exynos4_board_binfo);
 }
 
diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index 098a69ec73d3..116eae62756b 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -29,6 +29,10 @@
 #include "exec/memory.h"
 #include "target/arm/cpu-qom.h"
 
+#define TYPE_EXYNOS4210 "exynos4210"
+#define EXYNOS4210(obj) \
+OBJECT_CHECK(Exynos4210State, (obj), TYPE_EXYNOS4210)
+
 #define EXYNOS4210_NCPUS2
 
 #define EXYNOS4210_DRAM0_BASE_ADDR  0x4000
@@ -85,6 +89,8 @@ typedef struct Exynos4210Irq {
 } Exynos4210Irq;
 
 typedef struct Exynos4210State {
+DeviceState parent_obj;
+
 ARMCPU *cpu[EXYNOS4210_NCPUS];
 Exynos4210Irq irqs;
 qemu_irq *irq_table;
@@ -101,8 +107,6 @@ typedef struct Exynos4210State {
 void exynos4210_write_secondary(ARMCPU *cpu,
 const struct arm_boot_info *info);
 
-Exynos4210State *exynos4210_init(MemoryRegion *system_mem);
-
 /* Initialize exynos4210 IRQ subsystem stub */
 qemu_irq *exynos4210_init_irq(Exynos4210Irq *env);
 
-- 
2.9.3

[Qemu-devel] [PATCH 2/3] hw/arm/exynos: Declare local variables in some order

[Krzysztof Kozlowski]

Bring some more readability by declaring local function variables, first
initialized ones and then the rest (with reversed-christmas-tree order).

Signed-off-by: Krzysztof Kozlowski 
---
 hw/arm/exynos4210.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 0da877f8db0a..27a7bf28a5a9 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -162,12 +162,12 @@ static uint64_t exynos4210_calc_affinity(int cpu)
 
 Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
 {
-int i, n;
 Exynos4210State *s = g_new(Exynos4210State, 1);
 qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];
-DeviceState *dev;
 SysBusDevice *busdev;
 ObjectClass *cpu_oc;
+DeviceState *dev;
+int i, n;
 
 cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, "cortex-a9");
 assert(cpu_oc);
-- 
2.9.3

[Qemu-devel] [PATCH 0/3] hw/arm/exynos: QOM-ify the SoC

[Krzysztof Kozlowski]

Hi,

Convert the Exynos4210 SoC driver into QOM model.

No external dependencies, rebased on v2.9.0-363-g0de9191deb14.

Best regards,
Krzysztof

Krzysztof Kozlowski (3):
  hw/arm/exynos: Move RAM initialization next boards
  hw/arm/exynos: Declare local variables in some order
  hw/arm/exynos: QOM-ify the SoC

 hw/arm/exynos4210.c | 40 +++---
 hw/arm/exynos4_boards.c | 53 +++--
 include/hw/arm/exynos4210.h | 11 +-
 3 files changed, 69 insertions(+), 35 deletions(-)

-- 
2.9.3

Re: [Qemu-devel] dns server not working in QEMU using usermode networking (SLIRP)

[FONNEMANN Mark]

>So, in your /etc/nsswitch.conf, use
>hosts: files dns

This resolved the problem. I am now able to do DNS queries using nslookup.
Thanks for all your help!

Mark.

Re: [Qemu-devel] [PATCH v4 1/1] slirp: add SOCKS5 support

[Laurent Vivier]

Le 06/05/2017 à 01:27, Samuel Thibault a écrit :
> Hello,
> 

Hi,

> Laurent Vivier, on sam. 06 mai 2017 00:48:33 +0200, wrote:
>> @@ -617,6 +622,10 @@ void slirp_pollfds_poll(GArray *pollfds, int 
>> select_error)
>>   * Check sockets for reading
>>   */
>>  else if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) {
>> +if (so->so_state & SS_ISFCONNECTING) {
>> +socks5_recv(so->s, &so->so_proxy_state);
>> +continue;
>> +}
> 
> Again, I don't see how this can work with both socks5 case and
> non-socks5 case.  Don't we need to somehow check for the type of socket
> before calling socks5_recv?

The check is done previously by:

@@ -442,6 +443,10 @@ void slirp_pollfds_fill(GArray *pollfds, uint32_t
*timeout)
 .fd = so->s,
 .events = G_IO_OUT | G_IO_ERR,
 };
+if (so->so_proxy_state &&
+so->so_proxy_state != SOCKS5_STATE_ERROR) {
+pfd.events |= G_IO_IN;
+}

We can enter in socks5_recv() only if so->so_proxy_state is in a valid
state because G_IO_IN triggers that.

> 
>> @@ -645,11 +654,19 @@ void slirp_pollfds_poll(GArray *pollfds, int 
>> select_error)
>>  /*
>>   * Check for non-blocking, still-connecting sockets
>>   */
>> -if (so->so_state & SS_ISFCONNECTING) {
>> -/* Connected */
>> -so->so_state &= ~SS_ISFCONNECTING;
>>  
>> -ret = send(so->s, (const void *) &ret, 0, 0);
>> +if (so->so_state & SS_ISFCONNECTING) {
>> +ret = socks5_send(so->s, slirp->proxy_user,
> 
> Ditto.

if so_proxy_state is 0 the function does nothing and all remains as
without the function call (it's the case "ret > 0)"

> 
>> diff --git a/slirp/socks5.c b/slirp/socks5.c
>> new file mode 100644
>> index 000..2bba045
>> --- /dev/null
>> +++ b/slirp/socks5.c
>> @@ -0,0 +1,371 @@
> 
> In v2 of the patch, this was said to have "some parts from nmap/ncat
> GPLv2".  Is that really not true any more?  If any part of the file is
> not original, it *has* to wear proper copyright notices, otherwise it's
> copyright infrigement.

No, I've re-written entirely this part from RFC.

for ncat.h license is 281 lines long (with clarifications and
extensions) for 60 lines copied...

> Also, see the bot build error report:  doesn't exist on
> windows,
> 
> #include 
> #include 
> #include 
> 
> should be used instead.

In fact, the include is not needed at all.

Thanks,
Laurent

[Qemu-devel] qemu_savevm_state_begin

[ali saeedi]

Hi
what is the difference between functions "qemu_savevm_state_begin" and
"qemu_savevm_state_header" in migration_thread?
thanks a lot

[Qemu-devel] [PATCH 2/3] scsi-disk: export rotational qdev property

[Aurelien Jarno]

Export the rotational qdev property to the block device characteristics
VPD page.

Signed-off-by: Aurelien Jarno 
---
 hw/scsi/scsi-disk.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index a53f058621..21b7e21a23 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -597,6 +597,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, 
uint8_t *outbuf)
 outbuf[buflen++] = 0x83; // device identification
 if (s->qdev.type == TYPE_DISK) {
 outbuf[buflen++] = 0xb0; // block limits
+outbuf[buflen++] = 0xb1; /* block device characteristics */
 outbuf[buflen++] = 0xb2; // thin provisioning
 }
 break;
@@ -739,6 +740,19 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, 
uint8_t *outbuf)
 outbuf[43] = max_io_sectors & 0xff;
 break;
 }
+case 0xb1: /* block device characteristics */
+{
+buflen = 0x40;
+memset(outbuf + 4, 0, buflen - 4);
+
+/* medium rotation rate: 0 = not reported, 1 = non-rotating */
+outbuf[4] = 0;
+outbuf[5] = s->qdev.conf.rotational ? 0 : 1;
+
+/* nominal form factor */
+outbuf[7] = 0; /* not reported */
+break;
+}
 case 0xb2: /* thin provisioning */
 {
 buflen = 8;
-- 
2.11.0

[Qemu-devel] [PATCH 1/3] hw/block: Introduce rotational qdev property

[Aurelien Jarno]

The Linux kernel uses different I/O scheduler depending if the block
device is a rotational device or not. Also it uses rotational devices
to add entropy to the random pool.

This patch add a rotational qdev property so that the block device can
be configured as a rotational device or a non-rotational device. Default
to true to not change the default behavior.

Signed-off-by: Aurelien Jarno 
---
 blockdev.c   | 4 
 include/hw/block/block.h | 4 +++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index 4d8cdedd54..c914420641 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -4037,6 +4037,10 @@ QemuOptsList qemu_common_drive_opts = {
 .name = BDRV_OPT_READ_ONLY,
 .type = QEMU_OPT_BOOL,
 .help = "open drive file as read-only",
+},{
+.name = "rotational",
+.type = QEMU_OPT_BOOL,
+.help = "rotational drive (off, on)",
 },
 
 THROTTLE_OPTS,
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index f3f6e8ef02..304a579eca 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -27,6 +27,7 @@ typedef struct BlockConf {
 uint32_t cyls, heads, secs;
 OnOffAuto wce;
 bool share_rw;
+bool rotational;
 BlockdevOnError rerror;
 BlockdevOnError werror;
 } BlockConf;
@@ -56,7 +57,8 @@ static inline unsigned int get_physical_block_exp(BlockConf 
*conf)
_conf.discard_granularity, -1), \
 DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, \
 ON_OFF_AUTO_AUTO), \
-DEFINE_PROP_BOOL("share-rw", _state, _conf.share_rw, false)
+DEFINE_PROP_BOOL("share-rw", _state, _conf.share_rw, false), \
+DEFINE_PROP_BOOL("rotational", _state, _conf.rotational, true)
 
 #define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf)  \
 DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0),  \
-- 
2.11.0

[Qemu-devel] [PATCH 3/3] ide: export rotational qdev property

[Aurelien Jarno]

Export the rotational qdev property in the IDENTIFY request.

Signed-off-by: Aurelien Jarno 
---
 hw/ide/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 0b48b64d3a..1aa76b0d90 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -191,6 +191,9 @@ static void ide_identify(IDEState *s)
 if (dev && dev->conf.discard_granularity) {
 put_le16(p + 169, 1); /* TRIM support */
 }
+if (dev && !dev->conf.rotational) {
+put_le16(p + 217, 1); /* non-rotating device */
+}
 
 ide_identify_size(s);
 s->identify_set = 1;
-- 
2.11.0

Re: [Qemu-devel] [PATCH v4 6/8] tpm-backend: Move realloc_buffer() implementation to tpm-tis model

[Marc-André Lureau]

On Thu, May 4, 2017 at 3:29 PM Amarnath Valluri 
wrote:

> buffer reallocation is very unlikely to be backend specific. Hence move
> inside
> the tis.
>
> Signed-off-by: Amarnath Valluri 
>

Reviewed-by: Marc-André Lureau 

(Please resend the whole series, so we can easily test and grab it thanks
to patchew)

---
>  backends/tpm.c   |  9 -
>  hw/tpm/tpm_passthrough.c | 12 
>  hw/tpm/tpm_tis.c | 14 --
>  include/sysemu/tpm_backend.h | 12 
>  4 files changed, 12 insertions(+), 35 deletions(-)
>
> diff --git a/backends/tpm.c b/backends/tpm.c
> index 4ae6129..3519570 100644
> --- a/backends/tpm.c
> +++ b/backends/tpm.c
> @@ -71,15 +71,6 @@ bool tpm_backend_had_startup_error(TPMBackend *s)
>  return k->ops->had_startup_error(s);
>  }
>
> -size_t tpm_backend_realloc_buffer(TPMBackend *s, TPMSizedBuffer *sb)
> -{
> -TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> -
> -assert(k->ops->realloc_buffer);
> -
> -return k->ops->realloc_buffer(sb);
> -}
> -
>  void tpm_backend_deliver_request(TPMBackend *s)
>  {
>  g_thread_pool_push(s->thread_pool,
> (gpointer)TPM_BACKEND_CMD_PROCESS_CMD,
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index c7706e4..4ac47ed 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -258,17 +258,6 @@ static bool
> tpm_passthrough_get_startup_error(TPMBackend *tb)
>  return tpm_pt->had_startup_error;
>  }
>
> -static size_t tpm_passthrough_realloc_buffer(TPMSizedBuffer *sb)
> -{
> -size_t wanted_size = 4096; /* Linux tpm.c buffer size */
> -
> -if (sb->size != wanted_size) {
> -sb->buffer = g_realloc(sb->buffer, wanted_size);
> -sb->size = wanted_size;
> -}
> -return sb->size;
> -}
> -
>  static void tpm_passthrough_cancel_cmd(TPMBackend *tb)
>  {
>  TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
> @@ -475,7 +464,6 @@ static const TPMDriverOps tpm_passthrough_driver = {
>  .opts = tpm_passthrough_cmdline_opts,
>  .desc = "Passthrough TPM backend driver",
>  .create   = tpm_passthrough_create,
> -.realloc_buffer   = tpm_passthrough_realloc_buffer,
>  .reset= tpm_passthrough_reset,
>  .had_startup_error= tpm_passthrough_get_startup_error,
>  .cancel_cmd   = tpm_passthrough_cancel_cmd,
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index a6440fe..d5118e7 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -963,6 +963,16 @@ static int tpm_tis_do_startup_tpm(TPMState *s)
>  return tpm_backend_startup_tpm(s->be_driver);
>  }
>
> +static void tpm_tis_realloc_buffer(TPMSizedBuffer *sb)
> +{
> +size_t wanted_size = 4096; /* Linux tpm.c buffer size */
> +
> +if (sb->size != wanted_size) {
> +sb->buffer = g_realloc(sb->buffer, wanted_size);
> +sb->size = wanted_size;
> +}
> +}
> +
>  /*
>   * Get the TPMVersion of the backend device being used
>   */
> @@ -1010,9 +1020,9 @@ static void tpm_tis_reset(DeviceState *dev)
>  tis->loc[c].state = TPM_TIS_STATE_IDLE;
>
>  tis->loc[c].w_offset = 0;
> -tpm_backend_realloc_buffer(s->be_driver, &tis->loc[c].w_buffer);
> +tpm_tis_realloc_buffer(&tis->loc[c].w_buffer);
>  tis->loc[c].r_offset = 0;
> -tpm_backend_realloc_buffer(s->be_driver, &tis->loc[c].r_buffer);
> +tpm_tis_realloc_buffer(&tis->loc[c].r_buffer);
>  }
>
>  tpm_tis_do_startup_tpm(s);
> diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
> index 6e5bb8d..bebfba3 100644
> --- a/include/sysemu/tpm_backend.h
> +++ b/include/sysemu/tpm_backend.h
> @@ -85,8 +85,6 @@ struct TPMDriverOps {
>  /* returns true if nothing will ever answer TPM requests */
>  bool (*had_startup_error)(TPMBackend *t);
>
> -size_t (*realloc_buffer)(TPMSizedBuffer *sb);
> -
>  void (*reset)(TPMBackend *t);
>
>  void (*cancel_cmd)(TPMBackend *t);
> @@ -132,16 +130,6 @@ int tpm_backend_startup_tpm(TPMBackend *s);
>  bool tpm_backend_had_startup_error(TPMBackend *s);
>
>  /**
> - * tpm_backend_realloc_buffer:
> - * @s: the backend
> - * @sb: the TPMSizedBuffer to re-allocated to the size suitable for the
> - *  backend.
> - *
> - * This function returns the size of the allocated buffer
> - */
> -size_t tpm_backend_realloc_buffer(TPMBackend *s, TPMSizedBuffer *sb);
> -
> -/**
>   * tpm_backend_deliver_request:
>   * @s: the backend to send the request to
>   *
> --
> 2.7.4
>
>
> --
Marc-André Lureau

Re: [Qemu-devel] [PATCH v6 15/25] tcg/s390: Implement goto_ptr

[Aurelien Jarno]

On 2017-05-02 12:22, Richard Henderson wrote:
> Signed-off-by: Richard Henderson 
> ---
>  tcg/s390/tcg-target.h |  2 +-
>  tcg/s390/tcg-target.inc.c | 24 +---
>  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/tcg/s390/tcg-target.h b/tcg/s390/tcg-target.h
> index 6b7bcfb..957f0c0 100644
> --- a/tcg/s390/tcg-target.h
> +++ b/tcg/s390/tcg-target.h
> @@ -92,7 +92,7 @@ extern uint64_t s390_facilities;
>  #define TCG_TARGET_HAS_mulsh_i32  0
>  #define TCG_TARGET_HAS_extrl_i64_i32  0
>  #define TCG_TARGET_HAS_extrh_i64_i32  0
> -#define TCG_TARGET_HAS_goto_ptr   0
> +#define TCG_TARGET_HAS_goto_ptr   1
>  
>  #define TCG_TARGET_HAS_div2_i64   1
>  #define TCG_TARGET_HAS_rot_i641
> diff --git a/tcg/s390/tcg-target.inc.c b/tcg/s390/tcg-target.inc.c
> index a679280..5d7083e 100644
> --- a/tcg/s390/tcg-target.inc.c
> +++ b/tcg/s390/tcg-target.inc.c
> @@ -1741,9 +1741,14 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode 
> opc,
>  
>  switch (opc) {
>  case INDEX_op_exit_tb:
> -/* return value */
> -tcg_out_movi(s, TCG_TYPE_PTR, TCG_REG_R2, args[0]);
> -tgen_gotoi(s, S390_CC_ALWAYS, tb_ret_addr);
> +/* Reuse the zeroing that exists for goto_ptr.  */
> +a0 = args[0];
> +if (a0 == 0) {
> +tgen_gotoi(s, S390_CC_ALWAYS, s->code_gen_epilogue);
> +} else {
> +tcg_out_movi(s, TCG_TYPE_PTR, TCG_REG_R2, a0);
> +tgen_gotoi(s, S390_CC_ALWAYS, tb_ret_addr);
> +}
>  break;
>  
>  case INDEX_op_goto_tb:
> @@ -1767,6 +1772,10 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode 
> opc,
>  s->tb_jmp_reset_offset[args[0]] = tcg_current_code_size(s);
>  break;
>  
> +case INDEX_op_goto_ptr:
> +tcg_out_insn(s, RR, BCR, S390_CC_ALWAYS, args[0]);
> +break;
> +
>  OP_32_64(ld8u):
>  /* ??? LLC (RXY format) is only present with the extended-immediate
> facility, whereas LLGC is always present.  */
> @@ -2241,6 +2250,7 @@ static const TCGTargetOpDef s390_op_defs[] = {
>  { INDEX_op_exit_tb, { } },
>  { INDEX_op_goto_tb, { } },
>  { INDEX_op_br, { } },
> +{ INDEX_op_goto_ptr, { "r" } },
>  
>  { INDEX_op_ld8u_i32, { "r", "r" } },
>  { INDEX_op_ld8s_i32, { "r", "r" } },
> @@ -2439,6 +2449,14 @@ static void tcg_target_qemu_prologue(TCGContext *s)
>  /* br %r3 (go to TB) */
>  tcg_out_insn(s, RR, BCR, S390_CC_ALWAYS, tcg_target_call_iarg_regs[1]);
>  
> +/*
> + * Return path for goto_ptr. Set return value to 0, a-la exit_tb,
> + * and fall through to the rest of the epilogue.
> + */
> +s->code_gen_epilogue = s->code_ptr;
> +tcg_out_movi(s, TCG_TYPE_PTR, TCG_REG_R2, 0);
> +
> +/* TB epilogue */
>  tb_ret_addr = s->code_ptr;
>  
>  /* lmg %r6,%r15,fs+48(%r15) (restore registers) */

Reviewed-by: Aurelien Jarno 
Tested-by: Aurelien Jarno 

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH v6 18/25] target/s390: Use tcg_gen_lookup_and_goto_ptr

[Aurelien Jarno]

On 2017-05-02 12:22, Richard Henderson wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/s390x/translate.c | 17 -
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 01c6217..f7c2123 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -608,11 +608,16 @@ static void gen_op_calc_cc(DisasContext *s)
>  set_cc_static(s);
>  }
>  
> -static int use_goto_tb(DisasContext *s, uint64_t dest)
> +static bool use_exit_tb(DisasContext *s)
>  {
> -if (unlikely(s->singlestep_enabled) ||
> -(s->tb->cflags & CF_LAST_IO) ||
> -(s->tb->flags & FLAG_MASK_PER)) {
> +return (s->singlestep_enabled ||
> +(s->tb->cflags & CF_LAST_IO) ||
> +(s->tb->flags & FLAG_MASK_PER));
> +}
> +
> +static bool use_goto_tb(DisasContext *s, uint64_t dest)
> +{
> +if (unlikely(use_exit_tb(s))) {
>  return false;
>  }
>  #ifndef CONFIG_USER_ONLY
> @@ -5426,8 +5431,10 @@ void gen_intermediate_code(CPUS390XState *env, struct 
> TranslationBlock *tb)
>  /* Exit the TB, either by raising a debug exception or by return.  */
>  if (do_debug) {
>  gen_exception(EXCP_DEBUG);
> -} else {
> +} else if (use_exit_tb(&dc)) {
>  tcg_gen_exit_tb(0);
> +} else {
> +tcg_gen_lookup_and_goto_ptr(psw_addr);
>  }
>  break;
>  default:

Reviewed-by: Aurelien Jarno 
Tested-by: Aurelien Jarno 


-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH v6 13/25] tcg/aarch64: Implement goto_ptr

[Aurelien Jarno]

On 2017-05-02 12:22, Richard Henderson wrote:
> Measurements:
> 
>   SPECint06 (test set), x86_64-linux-user. Host: APM 
> 64-bit ARMv8 (Atlas/A57) @ 2.4 GHz
> 
>  1.45x 
> +-+-+-+
>|  *   
>|
>|  +++ *   *   
>  +goto-ptr |
>   1.4x 
> +-+...**...*+-+
>| *+++**   *   
>  +++   |
>  1.35x 
> +-+...*...**...*...*+-+
>| *   **   *   
> *+++*  |
>| *   **   *   
> *   *  |
>   1.3x 
> +-+...*...**...*...*...*+-+
>| *   **   *   
> *   *  |
>| *   **   *   
> *   ** |
>  1.25x 
> +-+...*...*...**...*...*...**...*...*...+-+
>| *   *   *   **   *   
> *   **+++*   *   * |
>   1.2x 
> +-+...*...*...*...**...*...*...**...*...*...*...+-+
>| *   *   *   **   *   
> *   **   *   *   * |
>| *   *   *   **   *   
> *   **   *   *   *   * |
>  1.15x 
> +-+...*...*...*...**...*...*...**...*...*...*...*...*...+-+
>| *   *   *   **   *   
> *   *+++ *   *   *   *   *   * |
>| *   *   *   **   *   
> *   *   **   *   *   *   *   * |
>   1.1x 
> +-+...*...*...*...**...*...*...*...*...*...*...**...*...*...*...*...*...+-+
>| *   *   *   **   *   *   *   *   *   
> *   *   *   **   *   *   *   *   * |
>  1.05x 
> +-+...*...*...*...**...*...*...*...*...*...*...*...*...**...*...*...*...*...*...+-+
>| *   *   *   *   **   *   *   *   *   *   
> *   *   *   **   *   *   *   *   * |
>| *   *   *   *   *   **   *   *   *   *   *   *   *   
> *   *   *   **   *   *   *   *   * |
> 1x 
> +-+---*---*---**---*---*---*---*---*---**---*---*---+-+
>   astar   bzip2 gccgobmk h264ref   hmmlibquantum mcf 
> omnetpperlbenchsjenxalancbmk   hmean
>   png: http://imgur.com/en9HE8L
> 
> Tested-by: Emilio G. Cota 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/aarch64/tcg-target.h |  2 +-
>  tcg/aarch64/tcg-target.inc.c | 22 --
>  2 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/tcg/aarch64/tcg-target.h b/tcg/aarch64/tcg-target.h
> index b82eac4..55a46ac 100644
> --- a/tcg/aarch64/tcg-target.h
> +++ b/tcg/aarch64/tcg-target.h
> @@ -77,7 +77,7 @@ typedef enum {
>  #define TCG_TARGET_HAS_mulsh_i320
>  #define TCG_TARGET_HAS_extrl_i64_i320
>  #define TCG_TARGET_HAS_extrh_i64_i320
> -#define TCG_TARGET_HAS_goto_ptr 0
> +#define TCG_TARGET_HAS_goto_ptr 1
>  
>  #define TCG_TARGET_HAS_div_i64  1
>  #define TCG_TARGET_HAS_rem_i64  1
> diff --git a/tcg/aarch64/tcg-target.inc.c b/tcg/aarch64/tcg-target.inc.c
> index 290de6d..5f18545 100644
> --- a/tcg/aarch64/tcg-target.inc.c
> +++ b/tcg/aarch64/tcg-target.inc.c
> @@ -1357,8 +1357,13 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
>  
>  switch (opc) {
>  case INDEX_op_exit_tb:
> -tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_X0, a0);
> -tcg_out_goto(s, tb_ret_addr);
> +/* Reuse the zeroing that exists for goto_ptr.  */
> +if (a0 == 0) {
> +tcg_out_goto(s, s->code_gen_epilogue);
> +} else {
> +tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_X0, a0);
> +tcg_out_goto(s, tb_ret_addr);
> +}
>  break;
> 

Re: [Qemu-devel] [PATCH] mips: set CP0 Debug DExcCode for SDBBP instruction

[Aurelien Jarno]

On 2017-05-02 15:03, Pavel Dovgalyuk wrote:
> From: Pavel Dovgalyuk 
> 
> This patch fixes setting DExcCode field of CP0 Debug register
> when SDBBP instruction is executed. According to EJTAG specification,
> this field must be set to the value 9 (Bp).
> 
> Signed-off-by: Pavel Dovgalyuk 
> ---
>  target/mips/helper.c |2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target/mips/helper.c b/target/mips/helper.c
> index e359ca3b44..166f0d1243 100644
> --- a/target/mips/helper.c
> +++ b/target/mips/helper.c
> @@ -627,6 +627,8 @@ void mips_cpu_do_interrupt(CPUState *cs)
>  goto set_DEPC;
>  case EXCP_DBp:
>  env->CP0_Debug |= 1 << CP0DB_DBp;
> +/* Setup DExcCode - SDBBP instruction */
> +env->CP0_Debug = (env->CP0_Debug & ~(0x1fULL << CP0DB_DEC)) | 9 << 
> CP0DB_DEC;
>  goto set_DEPC;
>  case EXCP_DDBS:
>  env->CP0_Debug |= 1 << CP0DB_DDBS;
> 

Reviewed-by: Aurelien Jarno 

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

[Qemu-devel] vmstate_save_state

[ali saeedi]

what does "vmstate_save_state(f, &vmstate_configuration, &savevm_state, 0)"
function call do in "qemu_savevm_state_header" function in savevm.c at line
968 ?
why has "savevm_state" been passed to this function?
thanks a lot

[Qemu-devel] [PATCH v2 01/14] target/sh4: split ctx->flags into ctx->tbflags and ctx->envflags

[Aurelien Jarno]

There is a confusion (and not only in the SH4 target) between tb->flags,
env->flags and ctx->flags. To avoid it, split ctx->flags into
ctx->tbflags and ctx->envflags. ctx->tbflags stays unchanged during the
whole TB translation, while ctx->envflags evolves and is kept in sync
with env->flags using TCG instructions. ctx->envflags now only contains
the part that of env->flags that is contained in the TB state, i.e. the
DELAY_SLOT* flags.

Signed-off-by: Aurelien Jarno 
---
 target/sh4/translate.c | 161 +
 1 file changed, 82 insertions(+), 79 deletions(-)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index c89a14733f..6b526a02f2 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -37,7 +37,8 @@ typedef struct DisasContext {
 struct TranslationBlock *tb;
 target_ulong pc;
 uint16_t opcode;
-uint32_t flags;
+uint32_t tbflags;/* should stay unmodified during the TB translation */
+uint32_t envflags;   /* should stay in sync with env->flags using TCG ops 
*/
 int bstate;
 int memidx;
 uint32_t delayed_pc;
@@ -49,7 +50,7 @@ typedef struct DisasContext {
 #if defined(CONFIG_USER_ONLY)
 #define IS_USER(ctx) 1
 #else
-#define IS_USER(ctx) (!(ctx->flags & (1u << SR_MD)))
+#define IS_USER(ctx) (!(ctx->tbflags & (1u << SR_MD)))
 #endif
 
 enum {
@@ -317,51 +318,50 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
 #define B11_8 ((ctx->opcode >> 8) & 0xf)
 #define B15_12 ((ctx->opcode >> 12) & 0xf)
 
-#define REG(x) ((x) < 8 && (ctx->flags & (1u << SR_MD))\
-&& (ctx->flags & (1u << SR_RB))\
+#define REG(x) ((x) < 8 && (ctx->tbflags & (1u << SR_MD))\
+&& (ctx->tbflags & (1u << SR_RB))\
 ? (cpu_gregs[x + 16]) : (cpu_gregs[x]))
 
-#define ALTREG(x) ((x) < 8 && (!(ctx->flags & (1u << SR_MD))\
-   || !(ctx->flags & (1u << SR_RB)))\
+#define ALTREG(x) ((x) < 8 && (!(ctx->tbflags & (1u << SR_MD))\
+   || !(ctx->tbflags & (1u << SR_RB)))\
? (cpu_gregs[x + 16]) : (cpu_gregs[x]))
 
-#define FREG(x) (ctx->flags & FPSCR_FR ? (x) ^ 0x10 : (x))
+#define FREG(x) (ctx->tbflags & FPSCR_FR ? (x) ^ 0x10 : (x))
 #define XHACK(x) x) & 1 ) << 4) | ((x) & 0xe))
-#define XREG(x) (ctx->flags & FPSCR_FR ? XHACK(x) ^ 0x10 : XHACK(x))
+#define XREG(x) (ctx->tbflags & FPSCR_FR ? XHACK(x) ^ 0x10 : XHACK(x))
 #define DREG(x) FREG(x) /* Assumes lsb of (x) is always 0 */
 
 #define CHECK_NOT_DELAY_SLOT \
-  if (ctx->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) \
-  {   \
-  tcg_gen_movi_i32(cpu_pc, ctx->pc);  \
-  gen_helper_raise_slot_illegal_instruction(cpu_env); \
-  ctx->bstate = BS_BRANCH;\
-  return; \
-  }
-
-#define CHECK_PRIVILEGED\
-  if (IS_USER(ctx)) {   \
-  tcg_gen_movi_i32(cpu_pc, ctx->pc);\
-  if (ctx->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) { \
-  gen_helper_raise_slot_illegal_instruction(cpu_env);   \
-  } else {  \
-  gen_helper_raise_illegal_instruction(cpu_env);\
-  } \
-  ctx->bstate = BS_BRANCH;  \
-  return;   \
-  }
-
-#define CHECK_FPU_ENABLED   \
-  if (ctx->flags & (1u << SR_FD)) { \
-  tcg_gen_movi_i32(cpu_pc, ctx->pc);\
-  if (ctx->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) { \
-  gen_helper_raise_slot_fpu_disable(cpu_env);   \
-  } else {  \
-  gen_helper_raise_fpu_disable(cpu_env);\
-  } \
-  ctx->bstate = BS_BRANCH;  \
-  return;   \
-  }
+if (ctx->envflags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) { \
+tcg_gen_movi_i32(cpu_pc, ctx->pc);   \
+gen_helper_raise_slot_illegal_instruction(cpu_env);  \
+ctx->bstate = BS_BRANCH; \
+return;  \
+}
+
+#define CHECK_PRIVILEGED \
+if (IS_USER(ctx)) {  \
+tcg_gen_movi_i32(cpu_pc, ctx->pc);   \
+if (ctx->envflags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) { \
+gen_helper_r

[Qemu-devel] [PATCH v2 14/14] target/sh4: trap unaligned accesses

[Aurelien Jarno]

SH4 requires that memory accesses are naturally aligned, except for the
SH4-A movua.l instructions which can do unaligned loads.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Aurelien Jarno 
---
 target/sh4/cpu.c   |  1 +
 target/sh4/cpu.h   |  4 
 target/sh4/op_helper.c | 19 +++
 target/sh4/translate.c |  6 --
 4 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c
index 9a481c35dc..9da7e1ed38 100644
--- a/target/sh4/cpu.c
+++ b/target/sh4/cpu.c
@@ -301,6 +301,7 @@ static void superh_cpu_class_init(ObjectClass *oc, void 
*data)
 #ifdef CONFIG_USER_ONLY
 cc->handle_mmu_fault = superh_cpu_handle_mmu_fault;
 #else
+cc->do_unaligned_access = superh_cpu_do_unaligned_access;
 cc->get_phys_page_debug = superh_cpu_get_phys_page_debug;
 #endif
 cc->disas_set_info = superh_cpu_disas_set_info;
diff --git a/target/sh4/cpu.h b/target/sh4/cpu.h
index faab3012f9..6c07c6b24b 100644
--- a/target/sh4/cpu.h
+++ b/target/sh4/cpu.h
@@ -24,6 +24,7 @@
 #include "cpu-qom.h"
 
 #define TARGET_LONG_BITS 32
+#define ALIGNED_ONLY
 
 /* CPU Subtypes */
 #define SH_CPU_SH7750  (1 << 0)
@@ -215,6 +216,9 @@ void superh_cpu_dump_state(CPUState *cpu, FILE *f,
 hwaddr superh_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 int superh_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
 int superh_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
+void superh_cpu_do_unaligned_access(CPUState *cpu, vaddr addr,
+MMUAccessType access_type,
+int mmu_idx, uintptr_t retaddr);
 
 void sh4_translate_init(void);
 SuperHCPU *cpu_sh4_init(const char *cpu_model);
diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
index 684d3f3758..4abd05667c 100644
--- a/target/sh4/op_helper.c
+++ b/target/sh4/op_helper.c
@@ -24,6 +24,25 @@
 
 #ifndef CONFIG_USER_ONLY
 
+void superh_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
+MMUAccessType access_type,
+int mmu_idx, uintptr_t retaddr)
+{
+if (retaddr) {
+cpu_restore_state(cs, retaddr);
+}
+switch (access_type) {
+case MMU_INST_FETCH:
+case MMU_DATA_LOAD:
+cs->exception_index = 0x0e0;
+break;
+case MMU_DATA_STORE:
+cs->exception_index = 0x100;
+break;
+}
+cpu_loop_exit(cs);
+}
+
 void tlb_fill(CPUState *cs, target_ulong addr, MMUAccessType access_type,
   int mmu_idx, uintptr_t retaddr)
 {
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 29ea506c1d..e4e602021e 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -1505,14 +1505,16 @@ static void _decode_opc(DisasContext * ctx)
 case 0x40a9:/* movua.l @Rm,R0 */
 /* Load non-boundary-aligned data */
 if (ctx->features & SH_FEATURE_SH4A) {
-tcg_gen_qemu_ld_i32(REG(0), REG(B11_8), ctx->memidx, MO_TEUL);
+tcg_gen_qemu_ld_i32(REG(0), REG(B11_8), ctx->memidx,
+MO_TEUL | MO_UNALN);
 return;
 }
 break;
 case 0x40e9:/* movua.l @Rm+,R0 */
 /* Load non-boundary-aligned data */
 if (ctx->features & SH_FEATURE_SH4A) {
-tcg_gen_qemu_ld_i32(REG(0), REG(B11_8), ctx->memidx, MO_TEUL);
+tcg_gen_qemu_ld_i32(REG(0), REG(B11_8), ctx->memidx,
+MO_TEUL | MO_UNALN);
 tcg_gen_addi_i32(REG(B11_8), REG(B11_8), 4);
 return;
 }
-- 
2.11.0

[Qemu-devel] [PATCH v2 07/14] target/sh4: only save flags state at the end of the TB

[Aurelien Jarno]

There is no need to save flags when entering and exiting the delay slot.
They can be saved only when reaching the end of the TB. If the TB is
interrupted before by an exception, they will be restored using
restore_state_to_opc.

Signed-off-by: Aurelien Jarno 
---
 target/sh4/translate.c | 69 --
 1 file changed, 33 insertions(+), 36 deletions(-)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index f608e314b6..8cee7d333f 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -212,6 +212,20 @@ static void gen_write_sr(TCGv src)
 tcg_gen_andi_i32(cpu_sr_t, cpu_sr_t, 1);
 }
 
+static inline void gen_save_cpu_state(DisasContext *ctx, bool save_pc)
+{
+if (save_pc) {
+tcg_gen_movi_i32(cpu_pc, ctx->pc);
+}
+if (ctx->delayed_pc != (uint32_t) -1) {
+tcg_gen_movi_i32(cpu_delayed_pc, ctx->delayed_pc);
+}
+if ((ctx->tbflags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL))
+!= ctx->envflags) {
+tcg_gen_movi_i32(cpu_flags, ctx->envflags);
+}
+}
+
 static inline bool use_goto_tb(DisasContext *ctx, target_ulong dest)
 {
 if (unlikely(ctx->singlestep_enabled)) {
@@ -246,6 +260,7 @@ static void gen_jump(DisasContext * ctx)
/* Target is not statically known, it comes necessarily from a
   delayed jump as immediate jump are conditinal jumps */
tcg_gen_mov_i32(cpu_pc, cpu_delayed_pc);
+tcg_gen_discard_i32(cpu_delayed_pc);
if (ctx->singlestep_enabled)
 gen_helper_debug(cpu_env);
tcg_gen_exit_tb(0);
@@ -254,21 +269,12 @@ static void gen_jump(DisasContext * ctx)
 }
 }
 
-static inline void gen_branch_slot(uint32_t delayed_pc, int t)
-{
-tcg_gen_movi_i32(cpu_delayed_pc, delayed_pc);
-if (t) {
-tcg_gen_mov_i32(cpu_delayed_cond, cpu_sr_t);
-} else {
-tcg_gen_xori_i32(cpu_delayed_cond, cpu_sr_t, 1);
-}
-}
-
 /* Immediate conditional jump (bt or bf) */
 static void gen_conditional_jump(DisasContext * ctx,
 target_ulong ift, target_ulong ifnott)
 {
 TCGLabel *l1 = gen_new_label();
+gen_save_cpu_state(ctx, false);
 tcg_gen_brcondi_i32(TCG_COND_NE, cpu_sr_t, 0, l1);
 gen_goto_tb(ctx, 0, ifnott);
 gen_set_label(l1);
@@ -291,11 +297,6 @@ static void gen_delayed_conditional_jump(DisasContext * 
ctx)
 gen_jump(ctx);
 }
 
-static inline void gen_store_flags(uint32_t flags)
-{
-tcg_gen_movi_i32(cpu_flags, flags);
-}
-
 static inline void gen_load_fpr64(TCGv_i64 t, int reg)
 {
 tcg_gen_concat_i32_i64(t, cpu_fregs[reg + 1], cpu_fregs[reg]);
@@ -337,7 +338,7 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
 
 #define CHECK_NOT_DELAY_SLOT \
 if (ctx->envflags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) { \
-tcg_gen_movi_i32(cpu_pc, ctx->pc);   \
+gen_save_cpu_state(ctx, true);   \
 gen_helper_raise_slot_illegal_instruction(cpu_env);  \
 ctx->bstate = BS_EXCP;   \
 return;  \
@@ -345,7 +346,7 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
 
 #define CHECK_PRIVILEGED \
 if (IS_USER(ctx)) {  \
-tcg_gen_movi_i32(cpu_pc, ctx->pc);   \
+gen_save_cpu_state(ctx, true);   \
 if (ctx->envflags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) { \
 gen_helper_raise_slot_illegal_instruction(cpu_env);  \
 } else { \
@@ -357,7 +358,7 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
 
 #define CHECK_FPU_ENABLED\
 if (ctx->tbflags & (1u << SR_FD)) {  \
-tcg_gen_movi_i32(cpu_pc, ctx->pc);   \
+gen_save_cpu_state(ctx, true);   \
 if (ctx->envflags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) { \
 gen_helper_raise_slot_fpu_disable(cpu_env);  \
 } else { \
@@ -501,14 +502,12 @@ static void _decode_opc(DisasContext * ctx)
 case 0xa000:   /* bra disp */
CHECK_NOT_DELAY_SLOT
ctx->delayed_pc = ctx->pc + 4 + B11_0s * 2;
-   tcg_gen_movi_i32(cpu_delayed_pc, ctx->delayed_pc);
 ctx->envflags |= DELAY_SLOT;
return;
 case 0xb000:   /* bsr disp */
CHECK_NOT_DELAY_SLOT
tcg_gen_movi_i32(cpu_pr, ctx->pc + 4);
ctx->delayed_pc = ctx->pc + 4 + B11_0s * 2;
-   tcg_gen_movi_i32(cpu_delayed_pc, ctx->delayed_pc);
 ctx->envflags |= DELAY_SLOT;
return;
 }
@@ -1165,7 +1164,8 @@ static void _decode_opc(DisasCon

[Qemu-devel] [PATCH v2 08/14] target/sh4: fold ctx->bstate = BS_BRANCH into gen_conditional_jump

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno 
---
 target/sh4/translate.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 8cee7d333f..a4c7a0895b 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -279,6 +279,7 @@ static void gen_conditional_jump(DisasContext * ctx,
 gen_goto_tb(ctx, 0, ifnott);
 gen_set_label(l1);
 gen_goto_tb(ctx, 1, ift);
+ctx->bstate = BS_BRANCH;
 }
 
 /* Delayed conditional jump (bt or bf) */
@@ -1158,9 +1159,7 @@ static void _decode_opc(DisasContext * ctx)
return;
 case 0x8b00:   /* bf label */
CHECK_NOT_DELAY_SLOT
-   gen_conditional_jump(ctx, ctx->pc + 2,
-ctx->pc + 4 + B7_0s * 2);
-   ctx->bstate = BS_BRANCH;
+gen_conditional_jump(ctx, ctx->pc + 2, ctx->pc + 4 + B7_0s * 2);
return;
 case 0x8f00:   /* bf/s label */
CHECK_NOT_DELAY_SLOT
@@ -1170,9 +1169,7 @@ static void _decode_opc(DisasContext * ctx)
return;
 case 0x8900:   /* bt label */
CHECK_NOT_DELAY_SLOT
-   gen_conditional_jump(ctx, ctx->pc + 4 + B7_0s * 2,
-ctx->pc + 2);
-   ctx->bstate = BS_BRANCH;
+gen_conditional_jump(ctx, ctx->pc + 4 + B7_0s * 2, ctx->pc + 2);
return;
 case 0x8d00:   /* bt/s label */
CHECK_NOT_DELAY_SLOT
-- 
2.11.0

[Qemu-devel] [PATCH v2 10/14] target/sh4: optimize gen_write_sr using extract op

[Aurelien Jarno]

This doesn't change the generated code on x86, but optimizes it on most
RISC architectures and makes the code simpler to read.

Signed-off-by: Aurelien Jarno 
---
 target/sh4/translate.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index b4e5606098..8c766eed2a 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -204,12 +204,9 @@ static void gen_write_sr(TCGv src)
 {
 tcg_gen_andi_i32(cpu_sr, src,
  ~((1u << SR_Q) | (1u << SR_M) | (1u << SR_T)));
-tcg_gen_shri_i32(cpu_sr_q, src, SR_Q);
-tcg_gen_andi_i32(cpu_sr_q, cpu_sr_q, 1);
-tcg_gen_shri_i32(cpu_sr_m, src, SR_M);
-tcg_gen_andi_i32(cpu_sr_m, cpu_sr_m, 1);
-tcg_gen_shri_i32(cpu_sr_t, src, SR_T);
-tcg_gen_andi_i32(cpu_sr_t, cpu_sr_t, 1);
+tcg_gen_extract_i32(cpu_sr_q, src, SR_Q, 1);
+tcg_gen_extract_i32(cpu_sr_m, src, SR_M, 1);
+tcg_gen_extract_i32(cpu_sr_t, src, SR_T, 1);
 }
 
 static inline void gen_save_cpu_state(DisasContext *ctx, bool save_pc)
-- 
2.11.0

[Qemu-devel] [PATCH v2 13/14] target/sh4: movua.l is an SH4-A only instruction

[Aurelien Jarno]

At the same time change the comment describing the instruction the same
way than other instruction, so that the code is easier to read and search.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Aurelien Jarno 
---
 target/sh4/translate.c | 26 +++---
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index d86fd29264..29ea506c1d 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -1502,17 +1502,21 @@ static void _decode_opc(DisasContext * ctx)
 }
 ctx->has_movcal = 1;
return;
-case 0x40a9:
-   /* MOVUA.L @Rm,R0 (Rm) -> R0
-  Load non-boundary-aligned data */
-tcg_gen_qemu_ld_i32(REG(0), REG(B11_8), ctx->memidx, MO_TEUL);
-   return;
-case 0x40e9:
-   /* MOVUA.L @Rm+,R0   (Rm) -> R0, Rm + 4 -> Rm
-  Load non-boundary-aligned data */
-tcg_gen_qemu_ld_i32(REG(0), REG(B11_8), ctx->memidx, MO_TEUL);
-   tcg_gen_addi_i32(REG(B11_8), REG(B11_8), 4);
-   return;
+case 0x40a9:/* movua.l @Rm,R0 */
+/* Load non-boundary-aligned data */
+if (ctx->features & SH_FEATURE_SH4A) {
+tcg_gen_qemu_ld_i32(REG(0), REG(B11_8), ctx->memidx, MO_TEUL);
+return;
+}
+break;
+case 0x40e9:/* movua.l @Rm+,R0 */
+/* Load non-boundary-aligned data */
+if (ctx->features & SH_FEATURE_SH4A) {
+tcg_gen_qemu_ld_i32(REG(0), REG(B11_8), ctx->memidx, MO_TEUL);
+tcg_gen_addi_i32(REG(B11_8), REG(B11_8), 4);
+return;
+}
+break;
 case 0x0029:   /* movt Rn */
 tcg_gen_mov_i32(REG(B11_8), cpu_sr_t);
return;
-- 
2.11.0

[Qemu-devel] [PATCH v2 09/14] target/sh4: optimize gen_store_fpr64

[Aurelien Jarno]

Isuing extrh and avoiding intermediate temps.

Signed-off-by: Aurelien Jarno 
---
 target/sh4/translate.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index a4c7a0895b..b4e5606098 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -305,13 +305,8 @@ static inline void gen_load_fpr64(TCGv_i64 t, int reg)
 
 static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
 {
-TCGv_i32 tmp = tcg_temp_new_i32();
-tcg_gen_extrl_i64_i32(tmp, t);
-tcg_gen_mov_i32(cpu_fregs[reg + 1], tmp);
-tcg_gen_shri_i64(t, t, 32);
-tcg_gen_extrl_i64_i32(tmp, t);
-tcg_gen_mov_i32(cpu_fregs[reg], tmp);
-tcg_temp_free_i32(tmp);
+tcg_gen_extrl_i64_i32(cpu_fregs[reg + 1], t);
+tcg_gen_extrh_i64_i32(cpu_fregs[reg], t);
 }
 
 #define B3_0 (ctx->opcode & 0xf)
-- 
2.11.0

[Qemu-devel] [PATCH v2 02/14] target/sh4: get rid of DELAY_SLOT_CLEARME

[Aurelien Jarno]

Now that ctx->flags has been split, it becomes clear that
DELAY_SLOT_CLEARME has not impact on the code generation: in both case
ctx->envflags is cleared, either by clearing all the flags, or by
setting it to 0. This is left-over from pre-TCG era.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Aurelien Jarno 
---
 target/sh4/cpu.h   |  3 +--
 target/sh4/helper.c|  2 --
 target/sh4/translate.c | 17 +
 3 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/target/sh4/cpu.h b/target/sh4/cpu.h
index cad8989f7e..9445cc779f 100644
--- a/target/sh4/cpu.h
+++ b/target/sh4/cpu.h
@@ -93,7 +93,6 @@
 #define DELAY_SLOT (1 << 0)
 #define DELAY_SLOT_CONDITIONAL (1 << 1)
 #define DELAY_SLOT_TRUE(1 << 2)
-#define DELAY_SLOT_CLEARME (1 << 3)
 /* The dynamic value of the DELAY_SLOT_TRUE flag determines whether the jump
  * after the delay slot should be taken or not. It is calculated from SR_T.
  *
@@ -384,7 +383,7 @@ static inline void cpu_get_tb_cpu_state(CPUSH4State *env, 
target_ulong *pc,
 *pc = env->pc;
 *cs_base = 0;
 *flags = (env->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL
-| DELAY_SLOT_TRUE | DELAY_SLOT_CLEARME))   /* Bits  0- 3 */
+| DELAY_SLOT_TRUE))/* Bits  0- 2 */
 | (env->fpscr & (FPSCR_FR | FPSCR_SZ | FPSCR_PR))  /* Bits 19-21 */
 | (env->sr & ((1u << SR_MD) | (1u << SR_RB)))  /* Bits 29-30 */
 | (env->sr & (1u << SR_FD))/* Bit 15 */
diff --git a/target/sh4/helper.c b/target/sh4/helper.c
index 036c5ca56c..71bb49a670 100644
--- a/target/sh4/helper.c
+++ b/target/sh4/helper.c
@@ -170,8 +170,6 @@ void superh_cpu_do_interrupt(CPUState *cs)
/* Clear flags for exception/interrupt routine. */
env->flags &= ~(DELAY_SLOT | DELAY_SLOT_CONDITIONAL | DELAY_SLOT_TRUE);
 }
-if (env->flags & DELAY_SLOT_CLEARME)
-env->flags = 0;
 
 if (do_exp) {
 env->expevt = cs->exception_index;
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 6b526a02f2..d84a7a2e6e 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -1804,15 +1804,9 @@ static void decode_opc(DisasContext * ctx)
 _decode_opc(ctx);
 
 if (old_flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) {
-if (ctx->envflags & DELAY_SLOT_CLEARME) {
-gen_store_flags(0);
-} else {
-   /* go out of the delay slot */
-uint32_t new_flags = ctx->envflags;
-   new_flags &= ~(DELAY_SLOT | DELAY_SLOT_CONDITIONAL);
-   gen_store_flags(new_flags);
-}
-ctx->envflags = 0;
+/* go out of the delay slot */
+ctx->envflags &= ~(DELAY_SLOT | DELAY_SLOT_CONDITIONAL);
+gen_store_flags(ctx->envflags);
 ctx->bstate = BS_BRANCH;
 if (old_flags & DELAY_SLOT_CONDITIONAL) {
gen_delayed_conditional_jump(ctx);
@@ -1840,8 +1834,7 @@ void gen_intermediate_code(CPUSH4State * env, struct 
TranslationBlock *tb)
 pc_start = tb->pc;
 ctx.pc = pc_start;
 ctx.tbflags = (uint32_t)tb->flags;
-ctx.envflags = tb->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL |
-DELAY_SLOT_CLEARME);
+ctx.envflags = tb->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL);
 ctx.bstate = BS_NONE;
 ctx.memidx = (ctx.tbflags & (1u << SR_MD)) == 0 ? 1 : 0;
 /* We don't know if the delayed pc came from a dynamic or static branch,
@@ -1908,7 +1901,7 @@ void gen_intermediate_code(CPUSH4State * env, struct 
TranslationBlock *tb)
 /* fall through */
 case BS_NONE:
 if (ctx.envflags) {
-gen_store_flags(ctx.envflags | DELAY_SLOT_CLEARME);
+gen_store_flags(ctx.envflags);
}
 gen_goto_tb(&ctx, 0, ctx.pc);
 break;
-- 
2.11.0

[Qemu-devel] [PATCH v2 03/14] target/sh4: do not include DELAY_SLOT_TRUE in the TB state

[Aurelien Jarno]

DELAY_SLOT_TRUE is used as a dynamic condition for the branch after the
delay slot instruction. It is not used in code generation, so there is
no need to including in the TB state.

Signed-off-by: Aurelien Jarno 
---
 target/sh4/cpu.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/sh4/cpu.h b/target/sh4/cpu.h
index 9445cc779f..da8d15f1b9 100644
--- a/target/sh4/cpu.h
+++ b/target/sh4/cpu.h
@@ -382,8 +382,7 @@ static inline void cpu_get_tb_cpu_state(CPUSH4State *env, 
target_ulong *pc,
 {
 *pc = env->pc;
 *cs_base = 0;
-*flags = (env->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL
-| DELAY_SLOT_TRUE))/* Bits  0- 2 */
+*flags = (env->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) /* Bits 0-1 
*/
 | (env->fpscr & (FPSCR_FR | FPSCR_SZ | FPSCR_PR))  /* Bits 19-21 */
 | (env->sr & ((1u << SR_MD) | (1u << SR_RB)))  /* Bits 29-30 */
 | (env->sr & (1u << SR_FD))/* Bit 15 */
-- 
2.11.0

[Qemu-devel] [PATCH v2 06/14] target/sh4: fix BS_EXCP exit

[Aurelien Jarno]

In case of exception, there is no need to call tcg_gen_exit_tb as the
exception helper won't return.

Also fix a few cases where BS_BRANCH is called instead of BS_EXCP.

Signed-off-by: Aurelien Jarno 
---
 target/sh4/translate.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 04bc18bf7c..f608e314b6 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -339,7 +339,7 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
 if (ctx->envflags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) { \
 tcg_gen_movi_i32(cpu_pc, ctx->pc);   \
 gen_helper_raise_slot_illegal_instruction(cpu_env);  \
-ctx->bstate = BS_BRANCH; \
+ctx->bstate = BS_EXCP;   \
 return;  \
 }
 
@@ -351,7 +351,7 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
 } else { \
 gen_helper_raise_illegal_instruction(cpu_env);   \
 }\
-ctx->bstate = BS_BRANCH; \
+ctx->bstate = BS_EXCP;   \
 return;  \
 }
 
@@ -363,7 +363,7 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
 } else { \
 gen_helper_raise_fpu_disable(cpu_env);   \
 }\
-ctx->bstate = BS_BRANCH; \
+ctx->bstate = BS_EXCP;   \
 return;  \
 }
 
@@ -1289,7 +1289,7 @@ static void _decode_opc(DisasContext * ctx)
imm = tcg_const_i32(B7_0);
 gen_helper_trapa(cpu_env, imm);
tcg_temp_free(imm);
-   ctx->bstate = BS_BRANCH;
+ctx->bstate = BS_EXCP;
}
return;
 case 0xc800:   /* tst #imm,R0 */
@@ -1798,7 +1798,7 @@ static void _decode_opc(DisasContext * ctx)
 } else {
 gen_helper_raise_illegal_instruction(cpu_env);
 }
-ctx->bstate = BS_BRANCH;
+ctx->bstate = BS_EXCP;
 }
 
 static void decode_opc(DisasContext * ctx)
@@ -1867,7 +1867,7 @@ void gen_intermediate_code(CPUSH4State * env, struct 
TranslationBlock *tb)
 /* We have hit a breakpoint - make sure PC is up-to-date */
 tcg_gen_movi_i32(cpu_pc, ctx.pc);
 gen_helper_debug(cpu_env);
-ctx.bstate = BS_BRANCH;
+ctx.bstate = BS_EXCP;
 /* The address covered by the breakpoint must be included in
[tb->pc, tb->pc + tb->size) in order to for it to be
properly cleared -- thus we increment the PC here so that
@@ -1911,9 +1911,7 @@ void gen_intermediate_code(CPUSH4State * env, struct 
TranslationBlock *tb)
 gen_goto_tb(&ctx, 0, ctx.pc);
 break;
 case BS_EXCP:
-/* gen_op_interrupt_restart(); */
-tcg_gen_exit_tb(0);
-break;
+/* fall through */
 case BS_BRANCH:
 default:
 break;
-- 
2.11.0

[Qemu-devel] [PATCH v2 04/14] target/sh4: move DELAY_SLOT_TRUE flag into a separate global

[Aurelien Jarno]

Instead of using one bit of the env flags to store the condition of the
next delay slot, use a separate global. It simplifies reading and
writing the flags variable and also removes some confusion between
ctx->envflags and env->flags.

Note that the global is first transfered to a temp in order to be
able to discard the global before the brcond.

Signed-off-by: Aurelien Jarno 
---
 target/sh4/cpu.h   | 10 ++
 target/sh4/helper.c|  2 +-
 target/sh4/translate.c | 22 +-
 3 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/target/sh4/cpu.h b/target/sh4/cpu.h
index da8d15f1b9..faab3012f9 100644
--- a/target/sh4/cpu.h
+++ b/target/sh4/cpu.h
@@ -92,13 +92,6 @@
 
 #define DELAY_SLOT (1 << 0)
 #define DELAY_SLOT_CONDITIONAL (1 << 1)
-#define DELAY_SLOT_TRUE(1 << 2)
-/* The dynamic value of the DELAY_SLOT_TRUE flag determines whether the jump
- * after the delay slot should be taken or not. It is calculated from SR_T.
- *
- * It is unclear if it is permitted to modify the SR_T flag in a delay slot.
- * The use of DELAY_SLOT_TRUE flag makes us accept such SR_T modification.
- */
 
 typedef struct tlb_t {
 uint32_t vpn;  /* virtual page number */
@@ -148,7 +141,8 @@ typedef struct CPUSH4State {
 uint32_t sgr;  /* saved global register 15 */
 uint32_t dbr;  /* debug base register */
 uint32_t pc;   /* program counter */
-uint32_t delayed_pc;   /* target of delayed jump */
+uint32_t delayed_pc;/* target of delayed branch */
+uint32_t delayed_cond;  /* condition of delayed branch */
 uint32_t mach; /* multiply and accumulate high */
 uint32_t macl; /* multiply and accumulate low */
 uint32_t pr;   /* procedure register */
diff --git a/target/sh4/helper.c b/target/sh4/helper.c
index 71bb49a670..8f8ce81401 100644
--- a/target/sh4/helper.c
+++ b/target/sh4/helper.c
@@ -168,7 +168,7 @@ void superh_cpu_do_interrupt(CPUState *cs)
 /* Branch instruction should be executed again before delay slot. */
env->spc -= 2;
/* Clear flags for exception/interrupt routine. */
-   env->flags &= ~(DELAY_SLOT | DELAY_SLOT_CONDITIONAL | DELAY_SLOT_TRUE);
+env->flags &= ~(DELAY_SLOT | DELAY_SLOT_CONDITIONAL);
 }
 
 if (do_exp) {
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index d84a7a2e6e..2e29936ad8 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -72,7 +72,7 @@ static TCGv cpu_pr, cpu_fpscr, cpu_fpul, cpu_ldst;
 static TCGv cpu_fregs[32];
 
 /* internal register indexes */
-static TCGv cpu_flags, cpu_delayed_pc;
+static TCGv cpu_flags, cpu_delayed_pc, cpu_delayed_cond;
 
 #include "exec/gen-icount.h"
 
@@ -147,6 +147,10 @@ void sh4_translate_init(void)
 cpu_delayed_pc = tcg_global_mem_new_i32(cpu_env,
offsetof(CPUSH4State, delayed_pc),
"_delayed_pc_");
+cpu_delayed_cond = tcg_global_mem_new_i32(cpu_env,
+  offsetof(CPUSH4State,
+   delayed_cond),
+  "_delayed_cond_");
 cpu_ldst = tcg_global_mem_new_i32(cpu_env,
  offsetof(CPUSH4State, ldst), "_ldst_");
 
@@ -252,11 +256,12 @@ static void gen_jump(DisasContext * ctx)
 
 static inline void gen_branch_slot(uint32_t delayed_pc, int t)
 {
-TCGLabel *label = gen_new_label();
 tcg_gen_movi_i32(cpu_delayed_pc, delayed_pc);
-tcg_gen_brcondi_i32(t ? TCG_COND_EQ : TCG_COND_NE, cpu_sr_t, 0, label);
-tcg_gen_ori_i32(cpu_flags, cpu_flags, DELAY_SLOT_TRUE);
-gen_set_label(label);
+if (t) {
+tcg_gen_mov_i32(cpu_delayed_cond, cpu_sr_t);
+} else {
+tcg_gen_xori_i32(cpu_delayed_cond, cpu_sr_t, 1);
+}
 }
 
 /* Immediate conditional jump (bt or bf) */
@@ -278,18 +283,17 @@ static void gen_delayed_conditional_jump(DisasContext * 
ctx)
 
 l1 = gen_new_label();
 ds = tcg_temp_new();
-tcg_gen_andi_i32(ds, cpu_flags, DELAY_SLOT_TRUE);
+tcg_gen_mov_i32(ds, cpu_delayed_cond);
+tcg_gen_discard_i32(cpu_delayed_cond);
 tcg_gen_brcondi_i32(TCG_COND_NE, ds, 0, l1);
 gen_goto_tb(ctx, 1, ctx->pc + 2);
 gen_set_label(l1);
-tcg_gen_andi_i32(cpu_flags, cpu_flags, ~DELAY_SLOT_TRUE);
 gen_jump(ctx);
 }
 
 static inline void gen_store_flags(uint32_t flags)
 {
-tcg_gen_andi_i32(cpu_flags, cpu_flags, DELAY_SLOT_TRUE);
-tcg_gen_ori_i32(cpu_flags, cpu_flags, flags);
+tcg_gen_movi_i32(cpu_flags, flags);
 }
 
 static inline void gen_load_fpr64(TCGv_i64 t, int reg)
-- 
2.11.0

[Qemu-devel] [PATCH v2 12/14] target/sh4: implement tas.b using atomic helper

[Aurelien Jarno]

We only emulate UP SH4, however as the tas.b instruction is used in the GNU
libc, this improve linux-user emulation.

Signed-off-by: Aurelien Jarno 
---
 target/sh4/translate.c | 19 +++
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 9fe2e2d4d9..d86fd29264 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -1635,19 +1635,14 @@ static void _decode_opc(DisasContext * ctx)
tcg_gen_shri_i32(REG(B11_8), REG(B11_8), 16);
return;
 case 0x401b:   /* tas.b @Rn */
-   {
-   TCGv addr, val;
-   addr = tcg_temp_local_new();
-   tcg_gen_mov_i32(addr, REG(B11_8));
-   val = tcg_temp_local_new();
-tcg_gen_qemu_ld_i32(val, addr, ctx->memidx, MO_UB);
+{
+TCGv val = tcg_const_i32(0x80);
+tcg_gen_atomic_fetch_or_i32(val, REG(B11_8), val,
+ctx->memidx, MO_UB);
 tcg_gen_setcondi_i32(TCG_COND_EQ, cpu_sr_t, val, 0);
-   tcg_gen_ori_i32(val, val, 0x80);
-tcg_gen_qemu_st_i32(val, addr, ctx->memidx, MO_UB);
-   tcg_temp_free(val);
-   tcg_temp_free(addr);
-   }
-   return;
+tcg_temp_free(val);
+}
+return;
 case 0xf00d: /* fsts FPUL,FRn - FPSCR: Nothing */
CHECK_FPU_ENABLED
tcg_gen_mov_i32(cpu_fregs[FREG(B11_8)], cpu_fpul);
-- 
2.11.0

[Qemu-devel] [PATCH v2 05/14] target/sh4: fix BS_STOP exit

[Aurelien Jarno]

When stopping the translation because the state has changed, goto_tb
should not be used as it might link TB with different flags.

Signed-off-by: Aurelien Jarno 
---
 target/sh4/translate.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 2e29936ad8..04bc18bf7c 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -1901,8 +1901,9 @@ void gen_intermediate_code(CPUSH4State * env, struct 
TranslationBlock *tb)
 } else {
switch (ctx.bstate) {
 case BS_STOP:
-/* gen_op_interrupt_restart(); */
-/* fall through */
+tcg_gen_movi_i32(cpu_pc, ctx.pc);
+tcg_gen_exit_tb(0);
+break;
 case BS_NONE:
 if (ctx.envflags) {
 gen_store_flags(ctx.envflags);
-- 
2.11.0

[Qemu-devel] [PATCH v2 11/14] target/sh4: generate fences for SH4

[Aurelien Jarno]

synco is a SH4-A only instruction.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Aurelien Jarno 
---
 target/sh4/translate.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 8c766eed2a..9fe2e2d4d9 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -1570,10 +1570,11 @@ static void _decode_opc(DisasContext * ctx)
else
break;
 case 0x00ab:   /* synco */
-   if (ctx->features & SH_FEATURE_SH4A)
-   return;
-   else
-   break;
+if (ctx->features & SH_FEATURE_SH4A) {
+tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC);
+return;
+}
+break;
 case 0x4024:   /* rotcl Rn */
{
TCGv tmp = tcg_temp_new();
-- 
2.11.0

[Qemu-devel] [PATCH v2 00/14] target/sh4: misc fixes, cleanup and optimizations

[Aurelien Jarno]

This patch series try to improve the SH4 target by using the (more or
less) recently introduced TCG features. It also fixes some issues spot
when writting the patches (linking of TB with different flags, SH4-A
specific instructions allowed on SH4) and correctly trap unaligned
accesses.

v2:
- Add some comments in the struct DisasContext declaration, as suggested
  by Philippe Mathieu-Daudé
- Add Reviewed-by entries

Aurelien Jarno (14):
  target/sh4: split ctx->flags into ctx->tbflags and ctx->envflags
  target/sh4: get rid of DELAY_SLOT_CLEARME
  target/sh4: do not include DELAY_SLOT_TRUE in the TB state
  target/sh4: move DELAY_SLOT_TRUE flag into a separate global
  target/sh4: fix BS_STOP exit
  target/sh4: fix BS_EXCP exit
  target/sh4: only save flags state at the end of the TB
  target/sh4: fold ctx->bstate = BS_BRANCH into gen_conditional_jump
  target/sh4: optimize gen_store_fpr64
  target/sh4: optimize gen_write_sr using extract op
  target/sh4: generate fences for SH4
  target/sh4: implement tas.b using atomic helper
  target/sh4: movua.l is an SH4-A only instruction
  target/sh4: trap unaligned accesses

 target/sh4/cpu.c   |   1 +
 target/sh4/cpu.h   |  18 ++-
 target/sh4/helper.c|   4 +-
 target/sh4/op_helper.c |  19 +++
 target/sh4/translate.c | 323 -
 5 files changed, 183 insertions(+), 182 deletions(-)

-- 
2.11.0

[Qemu-devel] [PULL 0/1] tcg-mips queue

[Aurelien Jarno]

The following changes since commit 12a95f320a36ef66f724a49bb05e4fb553ac5dbe:

  Merge remote-tracking branch 'kwolf/tags/for-upstream' into staging 
(2017-05-04 13:44:32 +0100)

are available in the git repository at:

  git://git.aurel32.net/qemu.git tags/pull-tcg-mips-20170506

for you to fetch changes up to 2f5a5f5774d95baacf86c03aa8a77a2d0390f2b2:

  tcg/mips: fix field extraction opcode (2017-05-06 12:48:53 +0200)


Fix MIPS R2 hosts support



Aurelien Jarno (1):
  tcg/mips: fix field extraction opcode

 tcg/mips/tcg-target.inc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.11.0

[Qemu-devel] [PULL 1/1] tcg/mips: fix field extraction opcode

[Aurelien Jarno]

The "msb" argument should correspond to (len - 1).

Signed-off-by: Aurelien Jarno 
---
 tcg/mips/tcg-target.inc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tcg/mips/tcg-target.inc.c b/tcg/mips/tcg-target.inc.c
index 01ac7b2c81..2a7e1c7f5b 100644
--- a/tcg/mips/tcg-target.inc.c
+++ b/tcg/mips/tcg-target.inc.c
@@ -2093,11 +2093,11 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode 
opc,
  args[3] + args[4] - 1, args[3]);
 break;
 case INDEX_op_extract_i32:
-tcg_out_opc_bf(s, OPC_EXT, a0, a1, a2 + args[3] - 1, a2);
+tcg_out_opc_bf(s, OPC_EXT, a0, a1, args[3] - 1, a2);
 break;
 case INDEX_op_extract_i64:
 tcg_out_opc_bf64(s, OPC_DEXT, OPC_DEXTM, OPC_DEXTU, a0, a1,
- a2 + args[3] - 1, a2);
+ args[3] - 1, a2);
 break;
 
 case INDEX_op_brcond_i32:
-- 
2.11.0

Re: [Qemu-devel] dns server not working in QEMU using usermode networking (SLIRP)

[Samuel Thibault]

FONNEMANN Mark, on sam. 06 mai 2017 04:18:52 +, wrote:
> >That's so weird there's no packet log. Does your guest perhaps have the 
> >strace tool? Or perhaps you >can manage to copy it over from another VM, 
> >since it only depends on libc.
> 
> See attached file.

Ok, so for some reason it doesn't even try to emit a packet...

> open("/etc/resolv.conf", O_RDONLY)  = 3
> read(3, "nameserver 10.0.2.3\n\n", 4096) = 21

It does get the proper nameserver.

> connect(3, {sa_family=AF_FILE, path="/var/run/nscd/socket"}, 110) = -1 ENOENT 
> (No such file or directory)

It tries to connect to nscd but that's not running.

> open("/etc/nsswitch.conf", O_RDONLY)= 3
> read(3, "hosts:  files\n", 4096)= 18
> read(3, "", 4096)   = 0

And I guess that's the culprit: in nsswitch.conf you only have
"hosts: files", so

> open("/lib/libnss_files.so.2", O_RDONLY) = 3
> open("/etc/hosts", O_RDONLY)= 3
> read(3, "127.0.0.1\tlocalhost\n10.0.2.15\tqe"..., 4096) = 35

It only uses /etc/hosts.


So, in your /etc/nsswitch.conf, use

hosts: files dns

Samuel

Re: [Qemu-devel] [Qemu-devel RFC v3 4/5] msf2: Add Smartfusion2 SoC.

[sundeep subbaraya]

Hi Alistair,

On Sat, May 6, 2017 at 5:23 AM, Alistair Francis  wrote:
> On Fri, May 5, 2017 at 9:14 AM, sundeep subbaraya
>  wrote:
>> Hi Alistair,
>>
>> On Fri, May 5, 2017 at 3:51 AM, Alistair Francis  
>> wrote:
>>> On Fri, Apr 28, 2017 at 9:51 AM, Subbaraya Sundeep
>>>  wrote:
 Smartfusion2 SoC has hardened Microcontroller subsystem
 and flash based FPGA fabric. This patch adds support for
 Microcontroller subsystem in the SoC.

 Signed-off-by: Subbaraya Sundeep 
 ---
  default-configs/arm-softmmu.mak |   1 +
  hw/arm/Makefile.objs|   2 +-
  hw/arm/msf2_soc.c   | 194 
 
  include/hw/arm/msf2_soc.h   |  62 +
  4 files changed, 258 insertions(+), 1 deletion(-)
  create mode 100644 hw/arm/msf2_soc.c
  create mode 100644 include/hw/arm/msf2_soc.h

 diff --git a/default-configs/arm-softmmu.mak 
 b/default-configs/arm-softmmu.mak
 index 78d7af0..7062512 100644
 --- a/default-configs/arm-softmmu.mak
 +++ b/default-configs/arm-softmmu.mak
 @@ -122,3 +122,4 @@ CONFIG_ACPI=y
  CONFIG_SMBIOS=y
  CONFIG_ASPEED_SOC=y
  CONFIG_GPIO_KEY=y
 +CONFIG_MSF2=y
 diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
 index 4c5c4ee..cce2759 100644
 --- a/hw/arm/Makefile.objs
 +++ b/hw/arm/Makefile.objs
 @@ -1,7 +1,7 @@
  obj-y += boot.o collie.o exynos4_boards.o gumstix.o highbank.o
  obj-$(CONFIG_DIGIC) += digic_boards.o
  obj-y += integratorcp.o mainstone.o musicpal.o nseries.o
 -obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
 +obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o msf2_soc.o
  obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o
  obj-$(CONFIG_ACPI) += virt-acpi-build.o
  obj-y += netduino2.o
 diff --git a/hw/arm/msf2_soc.c b/hw/arm/msf2_soc.c
 new file mode 100644
 index 000..a470872
 --- /dev/null
 +++ b/hw/arm/msf2_soc.c
 @@ -0,0 +1,194 @@
 +/*
 + * SmartFusion2 SoC emulation.
 + *
 + * Copyright (c) 2017 Subbaraya Sundeep 
 + *
 + * Permission is hereby granted, free of charge, to any person obtaining 
 a copy
 + * of this software and associated documentation files (the "Software"), 
 to deal
 + * in the Software without restriction, including without limitation the 
 rights
 + * to use, copy, modify, merge, publish, distribute, sublicense, and/or 
 sell
 + * copies of the Software, and to permit persons to whom the Software is
 + * furnished to do so, subject to the following conditions:
 + *
 + * The above copyright notice and this permission notice shall be 
 included in
 + * all copies or substantial portions of the Software.
 + *
 + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
 EXPRESS OR
 + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
 MERCHANTABILITY,
 + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
 + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
 OTHER
 + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
 ARISING FROM,
 + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS 
 IN
 + * THE SOFTWARE.
 + */
 +
 +#include "qemu/osdep.h"
 +#include "qapi/error.h"
 +#include "qemu-common.h"
 +#include "hw/arm/arm.h"
 +#include "exec/address-spaces.h"
 +#include "hw/char/serial.h"
 +#include "hw/boards.h"
 +#include "sysemu/block-backend.h"
 +#include "hw/arm/msf2_soc.h"
 +
 +#define MSF2_TIMER_BASE   0x40004000
 +#define MSF2_SYSREG_BASE  0x40038000
 +
 +#define MSF2_TIMER_IRQ0   14
 +#define MSF2_TIMER_IRQ1   15
 +
 +static const uint32_t spi_addr[MSF2_NUM_SPIS] = { 0x40001000 , 0x40011000 
 };
>>>
>>> Hey Sundeep,
>>>
>>> From your other patch it sounds like this should just be a single SPI
>>> device with to busses, so this will have to be re-worked.
>>>
>> I may be entirely wrong please correct me. I assumed this SoC and Board file
>> are analogous to dtsi and dts in linux. So SoC file will instantiate
>> all the controllers
>> like SPI0 and SPI1 present in SoC whereas board file will attach slaves 
>> present
>> on board (like EEPROM, SPI flashes etc.,).  Hence in board file I
>> attached flash to
>> SPI0. I was expecting one bus for each controller eg:
>> SPI0:
>> bus: spi0
>>  device: flash (Chip select 0)
>>  device: other device (Chip select 1)
>> SPI1:
>> bus: spi1
>>  device: other device (Chip select 0) (if attached in board file)
>
> That's pretty much each. The SoC should create everything that is on
> the SoC. So the SoC should create the SPI controller (which will
> create the SPI buses) but not connec

Re: [Qemu-devel] [RFC PATCH v1 0/5] Enable virtio-scsi boot from /dev/sgX

[Paolo Bonzini]

On 05/05/2017 18:12, Eric Farman wrote:
> 
> 
> On 05/05/2017 11:13 AM, Paolo Bonzini wrote:
>>
>>
>> On 05/05/2017 17:03, Eric Farman wrote:
>>> We get a value of x3f when sending that to a scsi-disk from bios
>>> code.  That's fully emulated though, in scsi_disk_emulate_inquiry.  And
>>> that's the scenario that already works.
>>>
>>> While there is indeed code in hw/scsi/scsi-generic.c to wire that in,
>>> that only happens after the I/O goes to the device itself.  The Block
>>> Limits page isn't supported [1] and thus it gets rejected with "invalid
>>> field in cdb".  We never get to that fixup code you reference, since the
>>> returned len is zero.
>>>
>>> Should I be refactoring this code to always patch in that block limit
>>> regardless of a response from the host/device?  (That is, when page xb0
>>> isn't supported by the hw.)
>>
>> What is the BLKSECTGET value you get?
> 
> x14 bytes when using /dev/sg0 (xa00 sectors when using /dev/sda).
> 
>>  Is there a sensible default value
>> that you can use when page 0xb0 isn't supported by the hardware?
> 
> I was setting max_sectors to x800 with good success, which was the
> power-of-2 floor that BLKSECTGET gave us.  That kept us within the
> limits of the host biovec code.  But it's a long way from the
> virtio-scsi value of x when max_sectors isn't specified, so don't
> know what side effects that may cause.

It's just slower, but 0x800 is already a megabyte worth of data so it's
not going to be that much slower.

Paolo

Re: [Qemu-devel] [PATCH v5 0/5] Memory leak fixes

[Paolo Bonzini]

On 04/05/2017 00:38, Marc-André Lureau wrote:
> Hi,
> 
> A new series of leaks spotted by ASAN. Mostly after introducing of the
> test-hmp. Would it be useful having a configure --enable-asan, and
> enabled by default with --enable-debug?

Yes, --enable-asan would be a nice idea, and we can also add a build
target for Docker tests.

Paolo

Re: [Qemu-devel] [RFC PATCH v1 8/9] util/qemu-thread-*: add qemu_lock, locked and unlock trace events

[Paolo Bonzini]

On 05/05/2017 17:59, Alex Bennée wrote:
> I'll re-spin my trace analysis script and the lock trace point once that
> pull request is merged. I would be nice if we could associate locks with
> names though as the QemuMutex * is basically just an anonymous handle.
> Would it be overly extravagant to add a const char * to QemuMutex to can
> be init'ed with a human readable name?

Sure, why not.  But please do make it a constant (as you suggest) so
that qemu_mutex_destroy does not need to g_free.

Paolo

[Qemu-devel] MigrationParams

[ali saeedi]

Hi
what are 'blk' and 'shared' boolean variables in MigrationParams structure
in migration.h?
thanks a lot