Re: [Qemu-devel] [PATCH v2 0/3] Add new CD-ROM related qtests

[Hervé Poussineau]

Le 16/03/2018 à 06:39, Thomas Huth a écrit :

With one of my clean-up patches (see commit 1454509726719e0933c800), I
recently accidentially broke the "-cdrom" parameter (more precisely
"-drive if=scsi") on a couple of boards, since there was no error
detected during the "make check" regression testing. This is clearly an
indication that we are lacking tests in this area.
So this small patch series now introduces some tests for CD-ROM drives:
The first two patches introduce the possibility to check that booting
from CD-ROM drives still works fine for x86 and s390x, and the third
patch adds a test that certain machines can at least still be started
with the "-cdrom" parameter (i.e. that test would have catched the
mistake that I did with my SCSI cleanup patch).


Reviewed-by: Hervé Poussineau 



v2:
  - Use g_spawn_sync() instead of execlp() to run genisoimage
  - The "-cdrom" parameter test is now run on all architectures (with
machine "none" for the machines that are not explicitly checked)
  - Some rewordings and improved comments here and there

Thomas Huth (3):
   tests/boot-sector: Add magic bytes to s390x boot code header
   tests/cdrom-test: Test booting from CD-ROM ISO image file
   tests/cdrom-test: Test that -cdrom parameter is working

  tests/Makefile.include |   2 +
  tests/boot-sector.c|   9 +-
  tests/cdrom-test.c | 222 +
  3 files changed, 230 insertions(+), 3 deletions(-)
  create mode 100644 tests/cdrom-test.c

[Qemu-devel] [PATCH v2 3/3] tests/cdrom-test: Test that -cdrom parameter is working

[Thomas Huth]

Commit 1454509726719e0933c800 recently broke the "-cdrom" parameter
on a couple of boards without us noticing it immediately. Thus let's
add a test which checks that "-cdrom" can at least be used to start
QEMU with certain machine types.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
---
 tests/cdrom-test.c | 58 ++
 1 file changed, 58 insertions(+)

diff --git a/tests/cdrom-test.c b/tests/cdrom-test.c
index 5bbf322..7a1fce5 100644
--- a/tests/cdrom-test.c
+++ b/tests/cdrom-test.c
@@ -13,6 +13,7 @@
 #include "qemu/osdep.h"
 #include "libqtest.h"
 #include "boot-sector.h"
+#include "qapi/qmp/qdict.h"
 
 static char isoimage[] = "cdrom-boot-iso-XX";
 
@@ -89,6 +90,32 @@ cleanup:
 return ret;
 }
 
+/**
+ * Check that at least the -cdrom parameter is basically working, i.e. we can
+ * see the filename of the ISO image in the output of "info block" afterwards
+ */
+static void test_cdrom_param(gconstpointer data)
+{
+QTestState *qts;
+char *resp;
+
+qts = qtest_startf("-M %s -cdrom %s", (const char *)data, isoimage);
+resp = qtest_hmp(qts, "info block");
+g_assert(strstr(resp, isoimage) != 0);
+g_free(resp);
+qtest_quit(qts);
+}
+
+static void add_cdrom_param_tests(const char **machines)
+{
+while (*machines) {
+char *testname = g_strdup_printf("cdrom/param/%s", *machines);
+qtest_add_data_func(testname, *machines, test_cdrom_param);
+g_free(testname);
+machines++;
+}
+}
+
 static void test_cdboot(gconstpointer data)
 {
 QTestState *qts;
@@ -154,6 +181,37 @@ int main(int argc, char **argv)
 add_x86_tests();
 } else if (g_str_equal(arch, "s390x")) {
 add_s390x_tests();
+} else if (g_str_equal(arch, "ppc64")) {
+const char *ppcmachines[] = {
+"pseries", "mac99", "g3beige", "40p", "prep", NULL
+};
+add_cdrom_param_tests(ppcmachines);
+} else if (g_str_equal(arch, "sparc")) {
+const char *sparcmachines[] = {
+"LX", "SPARCClassic", "SPARCbook", "SS-10", "SS-20", "SS-4",
+"SS-5", "SS-600MP", "Voyager", "leon3_generic", NULL
+};
+add_cdrom_param_tests(sparcmachines);
+} else if (g_str_equal(arch, "sparc64")) {
+const char *sparc64machines[] = {
+"niagara", "sun4u", "sun4v", NULL
+};
+add_cdrom_param_tests(sparc64machines);
+} else if (!strncmp(arch, "mips64", 6)) {
+const char *mips64machines[] = {
+"magnum", "malta", "mips", "pica61", NULL
+};
+add_cdrom_param_tests(mips64machines);
+} else if (g_str_equal(arch, "arm") || g_str_equal(arch, "aarch64")) {
+const char *armmachines[] = {
+"realview-eb", "realview-eb-mpcore", "realview-pb-a8",
+"realview-pbx-a9", "versatileab", "versatilepb", "vexpress-a15",
+"vexpress-a9", "virt", NULL
+};
+add_cdrom_param_tests(armmachines);
+} else {
+const char *nonemachine[] = { "none", NULL };
+add_cdrom_param_tests(nonemachine);
 }
 
 ret = g_test_run();
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 1/3] tests/boot-sector: Add magic bytes to s390x boot code header

[Thomas Huth]

We're going to use the s390x boot code for testing CD-ROM booting.
But the ISO loader of the s390-ccw bios is a little bit more picky
than the network loader and expects some magic bytes in the header
of the file (see linux_s390_magic in pc-bios/s390-ccw/bootmap.c), so
we've got to add them in our boot code here, too.

Reviewed-by: Christian Borntraeger 
Signed-off-by: Thomas Huth 
---
 tests/boot-sector.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tests/boot-sector.c b/tests/boot-sector.c
index c373f0e..7824286 100644
--- a/tests/boot-sector.c
+++ b/tests/boot-sector.c
@@ -68,8 +68,11 @@ static uint8_t x86_boot_sector[512] = {
 };
 
 /* For s390x, use a mini "kernel" with the appropriate signature */
-static const uint8_t s390x_psw[] = {
-0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00
+static const uint8_t s390x_psw_and_magic[] = {
+0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00,  /* Program status word  */
+0x02, 0x00, 0x00, 0x18, 0x60, 0x00, 0x00, 0x50,  /* Magic:   */
+0x02, 0x00, 0x00, 0x68, 0x60, 0x00, 0x00, 0x50,  /* see linux_s390_magic */
+0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40   /* in the s390-ccw bios */
 };
 static const uint8_t s390x_code[] = {
 0xa7, 0xf4, 0x00, 0x0a,/* j 0x10010 */
@@ -110,7 +113,7 @@ int boot_sector_init(char *fname)
 } else if (g_str_equal(arch, "s390x")) {
 len = 0x1 + sizeof(s390x_code);
 boot_code = g_malloc0(len);
-memcpy(boot_code, s390x_psw, sizeof(s390x_psw));
+memcpy(boot_code, s390x_psw_and_magic, sizeof(s390x_psw_and_magic));
 memcpy(_code[0x1], s390x_code, sizeof(s390x_code));
 } else {
 g_assert_not_reached();
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 2/3] tests/cdrom-test: Test booting from CD-ROM ISO image file

[Thomas Huth]

We already have the code for a boot file in tests/boot-sector.c,
so if the genisoimage program is available, we can easily create
a bootable CD ISO image that we can use for testing whether our
CD-ROM emulation and the BIOS CD-ROM boot works correctly.

Signed-off-by: Thomas Huth 
---
 tests/Makefile.include |   2 +
 tests/cdrom-test.c | 164 +
 2 files changed, 166 insertions(+)
 create mode 100644 tests/cdrom-test.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index ef9b88c..37ca258 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -178,6 +178,7 @@ check-qtest-generic-y = tests/qmp-test$(EXESUF)
 gcov-files-generic-y = monitor.c qapi/qmp-dispatch.c
 check-qtest-generic-y += tests/device-introspect-test$(EXESUF)
 gcov-files-generic-y = qdev-monitor.c qmp.c
+check-qtest-generic-y += tests/cdrom-test$(EXESUF)
 
 gcov-files-ipack-y += hw/ipack/ipack.c
 check-qtest-ipack-y += tests/ipoctal232-test$(EXESUF)
@@ -829,6 +830,7 @@ tests/test-qapi-util$(EXESUF): tests/test-qapi-util.o 
$(test-util-obj-y)
 tests/numa-test$(EXESUF): tests/numa-test.o
 tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o tests/boot-sector.o 
tests/acpi-utils.o
 tests/sdhci-test$(EXESUF): tests/sdhci-test.o $(libqos-pc-obj-y)
+tests/cdrom-test$(EXESUF): tests/cdrom-test.o tests/boot-sector.o 
$(libqos-obj-y)
 
 tests/migration/stress$(EXESUF): tests/migration/stress.o
$(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< 
,"LINK","$(TARGET_DIR)$@")
diff --git a/tests/cdrom-test.c b/tests/cdrom-test.c
new file mode 100644
index 000..5bbf322
--- /dev/null
+++ b/tests/cdrom-test.c
@@ -0,0 +1,164 @@
+/*
+ * Various tests for emulated CD-ROM drives.
+ *
+ * Copyright (c) 2018 Red Hat Inc.
+ *
+ * Author:
+ *Thomas Huth 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2
+ * or later. See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "boot-sector.h"
+
+static char isoimage[] = "cdrom-boot-iso-XX";
+
+static int exec_genisoimg(const char **args)
+{
+gchar *out_err = NULL;
+gint exit_status = -1;
+bool success;
+
+success = g_spawn_sync(NULL, (gchar **)args, NULL,
+   G_SPAWN_SEARCH_PATH | G_SPAWN_STDOUT_TO_DEV_NULL,
+   NULL, NULL, NULL, _err, _status, NULL);
+if (!success) {
+return -ENOENT;
+}
+if (out_err) {
+fputs(out_err, stderr);
+g_free(out_err);
+}
+
+return exit_status;
+}
+
+static int prepare_image(const char *arch, char *isoimage)
+{
+char srcdir[] = "cdrom-test-dir-XX";
+char *codefile = NULL;
+int ifh, ret = -1;
+const char *args[] = {
+"genisoimage", "-quiet", "-l", "-no-emul-boot",
+"-b", NULL, "-o", isoimage, srcdir, NULL
+};
+
+ifh = mkstemp(isoimage);
+if (ifh < 0) {
+perror("Error creating temporary iso image file");
+return -1;
+}
+if (!mkdtemp(srcdir)) {
+perror("Error creating temporary directory");
+goto cleanup;
+}
+
+if (g_str_equal(arch, "i386") || g_str_equal(arch, "x86_64") ||
+g_str_equal(arch, "s390x")) {
+codefile = g_strdup_printf("%s/bootcode-XX", srcdir);
+ret = boot_sector_init(codefile);
+if (ret) {
+goto cleanup;
+}
+} else {
+/* Just create a dummy file */
+char txt[] = "empty disc";
+codefile = g_strdup_printf("%s/readme.txt", srcdir);
+if (!g_file_set_contents(codefile, txt, sizeof(txt) - 1, NULL)) {
+fprintf(stderr, "Failed to create '%s'\n", codefile);
+goto cleanup;
+}
+}
+
+args[5] = strchr(codefile, '/') + 1;
+ret = exec_genisoimg(args);
+if (ret) {
+fprintf(stderr, "genisoimage failed: %i\n", ret);
+}
+
+unlink(codefile);
+
+cleanup:
+g_free(codefile);
+rmdir(srcdir);
+close(ifh);
+
+return ret;
+}
+
+static void test_cdboot(gconstpointer data)
+{
+QTestState *qts;
+
+qts = qtest_startf("-accel kvm:tcg -no-shutdown %s%s", (const char *)data,
+   isoimage);
+boot_sector_test(qts);
+qtest_quit(qts);
+}
+
+static void add_x86_tests(void)
+{
+qtest_add_data_func("cdrom/boot/default", "-cdrom ", test_cdboot);
+qtest_add_data_func("cdrom/boot/virtio-scsi",
+"-device virtio-scsi -device scsi-cd,drive=cdr "
+"-blockdev file,node-name=cdr,filename=", test_cdboot);
+qtest_add_data_func("cdrom/boot/isapc", "-M isapc "
+"-drive if=ide,media=cdrom,file=", test_cdboot);
+qtest_add_data_func("cdrom/boot/am53c974",
+"-device am53c974 -device scsi-cd,drive=cd1 "
+"-drive if=none,id=cd1,format=raw,file=", test_cdboot);
+

[Qemu-devel] [PATCH v2 0/3] Add new CD-ROM related qtests

[Thomas Huth]

With one of my clean-up patches (see commit 1454509726719e0933c800), I
recently accidentially broke the "-cdrom" parameter (more precisely
"-drive if=scsi") on a couple of boards, since there was no error
detected during the "make check" regression testing. This is clearly an
indication that we are lacking tests in this area.
So this small patch series now introduces some tests for CD-ROM drives:
The first two patches introduce the possibility to check that booting
from CD-ROM drives still works fine for x86 and s390x, and the third
patch adds a test that certain machines can at least still be started
with the "-cdrom" parameter (i.e. that test would have catched the
mistake that I did with my SCSI cleanup patch).

v2:
 - Use g_spawn_sync() instead of execlp() to run genisoimage
 - The "-cdrom" parameter test is now run on all architectures (with
   machine "none" for the machines that are not explicitly checked)
 - Some rewordings and improved comments here and there

Thomas Huth (3):
  tests/boot-sector: Add magic bytes to s390x boot code header
  tests/cdrom-test: Test booting from CD-ROM ISO image file
  tests/cdrom-test: Test that -cdrom parameter is working

 tests/Makefile.include |   2 +
 tests/boot-sector.c|   9 +-
 tests/cdrom-test.c | 222 +
 3 files changed, 230 insertions(+), 3 deletions(-)
 create mode 100644 tests/cdrom-test.c

-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 3/3] vfio/pci: Add ioeventfd support

[Peter Xu]

On Thu, Mar 15, 2018 at 03:31:58PM -0600, Alex Williamson wrote:
> The ioeventfd here is actually irqfd handling of an ioeventfd such as
> supported in KVM.  A user is able to pre-program a device write to
> occur when the eventfd triggers.  This is yet another instance of
> eventfd-irqfd triggering between KVM and vfio.  The impetus for this
> is high frequency writes to pages which are virtualized in QEMU.
> Enabling this near-direct write path for selected registers within
> the virtualized page can improve performance and reduce overhead.
> Specifically this is initially targeted at NVIDIA graphics cards where
> the driver issues a write to an MMIO register within a virtualized
> region in order to allow the MSI interrupt to re-trigger.
> 
> Signed-off-by: Alex Williamson 

Reviewed-by: Peter Xu 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v4 14/29] libvhost-user+postcopy: Register new regions with the ufd

[Peter Xu]

On Thu, Mar 15, 2018 at 09:41:00AM +, Dr. David Alan Gilbert wrote:
> * Peter Xu (pet...@redhat.com) wrote:
> > On Mon, Mar 12, 2018 at 01:23:21PM +, Dr. David Alan Gilbert wrote:
> > > * Peter Xu (pet...@redhat.com) wrote:
> > > > On Thu, Mar 08, 2018 at 07:57:56PM +, Dr. David Alan Gilbert (git) 
> > > > wrote:
> > > > > From: "Dr. David Alan Gilbert" 
> > > > > 
> > > > > When new regions are sent to the client using SET_MEM_TABLE, register
> > > > > them with the userfaultfd.
> > > > > 
> > > > > Signed-off-by: Dr. David Alan Gilbert 
> > > > > ---
> > > > >  contrib/libvhost-user/libvhost-user.c | 34 
> > > > > ++
> > > > >  1 file changed, 34 insertions(+)
> > > > > 
> > > > > diff --git a/contrib/libvhost-user/libvhost-user.c 
> > > > > b/contrib/libvhost-user/libvhost-user.c
> > > > > index 4922b2c722..a18bc74a7c 100644
> > > > > --- a/contrib/libvhost-user/libvhost-user.c
> > > > > +++ b/contrib/libvhost-user/libvhost-user.c
> > > > > @@ -494,6 +494,40 @@ vu_set_mem_table_exec_postcopy(VuDev *dev, 
> > > > > VhostUserMsg *vmsg)
> > > > >  close(vmsg->fds[i]);
> > > > >  }
> > > > >  
> > > > > +/* TODO: Get address back to QEMU */
> > > > > +for (i = 0; i < dev->nregions; i++) {
> > > > > +VuDevRegion *dev_region = >regions[i];
> > > > > +#ifdef UFFDIO_REGISTER
> > > > > +/* We should already have an open ufd. Mark each memory
> > > > > + * range as ufd.
> > > > > + * Note: Do we need any madvises? Well it's not been accessed
> > > > > + * yet, still probably need no THP to be safe, discard to be 
> > > > > safe?
> > > > > + */
> > > > > +struct uffdio_register reg_struct;
> > > > > +reg_struct.range.start = (uintptr_t)dev_region->mmap_addr;
> > > > > +reg_struct.range.len = dev_region->size + 
> > > > > dev_region->mmap_offset;
> > > > 
> > > > Do we really care the page faults between offset zero to mmap_offset?
> > > 
> > > No, but if we saw them we'd think it meant something had gone wrong,
> > > so it's good to trap them.
> > 
> > I'm fine with that, especially since that's now only used in the test
> > codes.  However that's a still bit confusing to me, especially if
> > current QEMU won't really handle that page fault (and it seems that
> > should never happen).  Maybe at least a comment would help on
> > explaining why we need to explicitly extend the range to listen, just
> > like below code when we do the mapping, though with a different
> > reason.
> > 
> > > 
> > > > I'm thinking whether we should add that mmap_offset into range.start
> > > > instead of range.len.
> > > > 
> > > > Also, I see that in current vu_set_mem_table_exec():
> > > > 
> > > > /* We don't use offset argument of mmap() since the
> > > >  * mapped address has to be page aligned, and we use huge
> > > >  * pages.  */
> > > > mmap_addr = mmap(0, dev_region->size + dev_region->mmap_offset,
> > > >  PROT_READ | PROT_WRITE, MAP_SHARED,
> > > >  vmsg->fds[i], 0);
> > > > 
> > > > So adding the mmap_offset will help to make sure we'll use huge pages?
> > > > Could it?  Or say, how could we be sure that size+mmap_offset would be
> > > > page aligned?
> > > 
> > > If you look into the set_mem_table_exec (non-postcopy) you'll see that
> > > code and comment comes from the non-postcopy version; but it's something
> > > which as you say we could probably simplify now.
> > > 
> > > The problem used to be, before we did the merging as part of this series
> > > (0026 vhost Huge page align and merge), we could end up with mappings
> > > being passed from the qemu that were for small ranges of memory that
> > > weren't aligned to a huge page boundary and thus the mmap would fail.
> > > With the merging code that's no longer true, so it means we
> > > could simplify as you say;  although this way it's a smaller change from
> > > the existing code.
> > 
> > I was thinking what if the memory section was e.g. splitted as below:
> > 
> > - range A: [0x0,  0x10): non-RAM range, size 0x10
> > - range B: [0x10, 0x10): RAM range, size 0x1fffe0
> > - range C: [0x10, 0x20): non-RAM range, size 0x10
> > 
> > Ranges A+B+C will cover a 2M page while vhost-user master should only
> > send range B to the client. Then even size+mmap_offset (which is
> > 0x1fffe0+0x10=0x10) shouldn't be aligned with the 2M boundary.
> > If previous mmap() can fail, would this fail too?
> 
> I think, 'vhost: Huge page align and merge' will round (B) to page
> boundaries so that the range sent to the slave will be aligned.

Ah I see what you meant.  Yes that seems to work, and yes the old code
won't hurt too even with some useless extra pages mapped.

-- 
Peter Xu

Re: [Qemu-devel] Using new TCG Vector infrastructure in PowerPC

[Nikunj A Dadhania]

Richard Henderson  writes:

> On 03/07/2018 06:03 PM, Nikunj A Dadhania wrote:
>> Hi Richard,
>> 
>> I was working to get TCG vector support for PowerPC[1]. Started with
>> converting logical operations like vector AND/OR/XOR and compare
>> instructions. Found some inconsistency during my testing on x86 laptop
>> emulating PowerPC:
>
> Great.
>
> Well, the problem is that you cannot use TCG generic vectors and TCG global
> variables to access the same memory.

Interesting, wasn't aware of this.

> Thus your first step must be to remove all references to cpu_avrh and 
> cpu_avrl.
>  These can be replaced by translator helpers that perform an explicit
> tcg_gen_ld_i64 or tcg_gen_st_i64 to the proper memory locations.
>
> Only after that's done can you begin converting other references to use the
> host vectors.  Otherwise, the tcg optimizer will do Bad and Unpredictable
> Things, which may well have produced the incorrect results that you saw.
>
> I'll note that it's probably worth rearranging all of {fpr,vsr,avr} to the 
> more
> logical configuration presented by Power7 (?) such that it's one array of 64 x
> 128-bit registers.

I have following for taking care of making VSRs contiguous 128bits. This
has touched lot of code even out of tcg directory. So I currently have
32 AVRs (128bits) and 32 VSRs (128 bits).

@@ -1026,8 +1027,8 @@ struct CPUPPCState {
 
 /* Floating point execution context */
 float_status fp_status;
-/* floating point registers */
-float64 fpr[32];
+/* floating point registers multiplexed with vsr */
+
 /* floating point status and control register */
 target_ulong fpscr;
 
@@ -1078,8 +1079,8 @@ struct CPUPPCState {
 /* Altivec registers */
 ppc_avr_t avr[32];
 uint32_t vscr;
-/* VSX registers */
-uint64_t vsr[32];
+/* 32 (128bit)- VSX registers */
+ppc_avr_t vsr[32];
 /* SPE registers */
 uint64_t spe_acc;
 uint32_t spe_fscr;

Regards,
Nikunj

Re: [Qemu-devel] [PATCH v10 19/24] migration: Create multifd channels

[Peter Xu]

On Thu, Mar 15, 2018 at 01:57:54PM +0100, Juan Quintela wrote:
> Peter Xu  wrote:
> > On Wed, Mar 07, 2018 at 12:00:05PM +0100, Juan Quintela wrote:
> >> In both sides.  We still don't transmit anything through them.
> >
> > s/In/On/?
> >
> >> 
> >> Signed-off-by: Juan Quintela 
> >> ---
> >>  migration/ram.c | 52 ++--
> >>  1 file changed, 42 insertions(+), 10 deletions(-)
> >> 
> >> diff --git a/migration/ram.c b/migration/ram.c
> >> index b57d9fd667..7ef0c2b7e2 100644
> >> --- a/migration/ram.c
> >> +++ b/migration/ram.c
> >> @@ -399,6 +399,7 @@ struct MultiFDSendParams {
> >>  uint8_t id;
> >>  char *name;
> >>  QemuThread thread;
> >> +QIOChannel *c;
> >>  QemuSemaphore sem;
> >>  QemuMutex mutex;
> >>  bool running;
> >> @@ -455,6 +456,8 @@ int multifd_save_cleanup(Error **errp)
> >>  qemu_thread_join(>thread);
> >>  p->running = false;
> >>  }
> >> +socket_send_channel_destroy(p->c);
> >> +p->c = NULL;
> >>  qemu_mutex_destroy(>mutex);
> >>  qemu_sem_destroy(>sem);
> >>  g_free(p->name);
> >> @@ -514,6 +517,27 @@ static void *multifd_send_thread(void *opaque)
> >>  return NULL;
> >>  }
> >>  
> >> +static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque)
> >> +{
> >> +MultiFDSendParams *p = opaque;
> >> +QIOChannel *sioc = QIO_CHANNEL(qio_task_get_source(task));
> >> +Error *local_err = NULL;
> >> +
> >> +if (qio_task_propagate_error(task, _err)) {
> >> +if (multifd_save_cleanup(_err) != 0) {
> >
> > Do we need to call multifd_save_cleanup() explicitly here?
> 
> Is the easiest way of stoping all multifd threads, no?

Yeah, but again, I thought it would be called later too, since...

> 
> > Asked since I saw that it would also be called in migrate_fd_cleanup(),
> > and it seems that we should call migrate_fd_cleanup() soon too when
> > this happens?
> 
> We need to stop migraiton. thtat migrate_set_error() is only used for
> reporting in info migrate, it is not acted upon.
> 
> Yes, perhaps it should, but as it is, it is not.  So, I think it is
> right O:-)

... after Dave's 688a3dcba9 ("migration: Route errors down through
migration_channel_connect", 2018-02-06), all these channel errors
should finally be routed to migrate_fd_connect(), and in that we have:

void migrate_fd_connect(MigrationState *s, Error *error_in)
{
s->expected_downtime = s->parameters.downtime_limit;
s->cleanup_bh = qemu_bh_new(migrate_fd_cleanup, s);
if (error_in) {
migrate_fd_error(s, error_in);
migrate_fd_cleanup(s);
return;
}
...
}

Then, in migrate_fd_cleanup() we have multifd_save_cleanup().  That's
why I thought we can skip the cleanup here since after all we'll do it
in other places (and we can keep the cleanup code unified).

Thanks,

> 
> Later, Juan.
> 
> > Otherwise it looks fine to me.  Thanks,
> >
> >> +migrate_set_error(migrate_get_current(), local_err);
> >> +}
> >> +} else {
> >> +p->c = QIO_CHANNEL(sioc);
> >> +qio_channel_set_delay(p->c, false);
> >> +p->running = true;
> >> +qemu_thread_create(>thread, p->name, multifd_send_thread, p,
> >> +   QEMU_THREAD_JOINABLE);
> >> +
> >> +atomic_inc(_send_state->count);
> >> +}
> >> +}
> >
> > [...]

-- 
Peter Xu

[Qemu-devel] [PATCH v2] scripts/checkpatch.pl: Bug fix

[Su Hang]

checkpatch.pl stops complaining about following pattern:
"""
do {
//do somethins;
} while (conditions);
"""

One things need to be mentioned:
Becasue `if`, `while` and `for` check have been done in this
`if` block(Line: 2356), and this block contains following statement:
""" Line: 2379
$suppress_ifbraces{$ln + $offset} = 1;
"""
So the behind block may never run:
""" Line: 2415
if (!defined $suppress_ifbraces{$linenr - 1} &&
$line =~ /\b(if|while|for|else)\b/ &&
$line !~ /\#\s*if/ &&
$line !~ /\#\s*else/) {
"""
I'm not sure, please give me some advice.

(Sorry, I don't know this patch should base on which commit,
 so I generate this patch based on
 commit:fb8446d94ec7a3dc0c3a7e7da672406476f075ac,
 I choose this by `git log -2 scripts/checkpath.pl`.
 Sincerely say sorry, if I have misunderstand any meaning.)

Signed-off-by: Su Hang 
---
 scripts/checkpatch.pl | 21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a88af61ed4ee..d6f0747ba20a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2352,8 +2352,22 @@ sub process {
}
}

-# check for missing bracing round if etc
-   if ($line =~ /(^.*)\bif\b/ && $line !~ /\#\s*if/) {
+# check for missing bracing around if etc
+   if ($line =~ /(^.*)\b(?:if|while|for)\b/ &&
+   $line !~ /\#\s*if/) {
+   my $allowed = 0;
+
+   # Check the pre-context.
+   if ($line =~ /(\}.*?)$/) {
+   my $pre = $1;
+
+   if ($line !~ /else/) {
+   print "APW: ALLOWED: pre<$pre> 
line<$line>\n"
+   if $dbg_adv_apw;
+   $allowed = 1;
+   }
+   }
+
my ($level, $endln, @chunks) =
ctx_statement_full($linenr, $realcnt, 1);
 if ($dbg_adv_apw) {
@@ -2362,7 +2376,6 @@ sub process {
 if $#chunks >= 1;
 }
if ($#chunks >= 0 && $level == 0) {
-   my $allowed = 0;
my $seen = 0;
my $herectx = $here . "\n";
my $ln = $linenr - 1;
@@ -2406,7 +2419,7 @@ sub process {
 $allowed = 1;
}
}
-   if ($seen != ($#chunks + 1)) {
+   if ($seen != ($#chunks + 1) && !$allowed) {
ERROR("braces {} are necessary for all 
arms of this statement\n" . $herectx);
}
}
--
2.7.4

Re: [Qemu-devel] [PATCH] docker: fedora: test more components

[Fam Zheng]

On Thu, 03/15 14:47, Daniel P. Berrangé wrote:
> On Tue, Mar 13, 2018 at 01:05:52PM +0100, Paolo Bonzini wrote:
> > Install optional dependencies of QEMU to get better coverage.
> > 
> > Signed-off-by: Paolo Bonzini 
> > ---
> >  tests/docker/dockerfiles/fedora.docker | 13 ++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tests/docker/dockerfiles/fedora.docker 
> > b/tests/docker/dockerfiles/fedora.docker
> > index 994a35a..0d9da45 100644
> > --- a/tests/docker/dockerfiles/fedora.docker
> > +++ b/tests/docker/dockerfiles/fedora.docker
> > @@ -1,9 +1,16 @@
> >  FROM fedora:27
> >  ENV PACKAGES \
> >  ccache gettext git tar PyYAML sparse flex bison python3 bzip2 hostname 
> > \
> > -glib2-devel pixman-devel zlib-devel SDL-devel libfdt-devel \
> > -gcc gcc-c++ clang make perl which bc findutils libaio-devel \
> > -nettle-devel libasan libubsan \
> > +gcc gcc-c++ clang make perl which bc findutils glib2-devel \
> > +libaio-devel pixman-devel zlib-devel libfdt-devel libasan libubsan \
> > +bluez-libs-devel brlapi-devel bzip2-devel \
> > +device-mapper-multipath-devel glusterfs-api-devel gnutls-devel \
> > +gtk3-devel libattr-devel libcap-devel libcap-ng-devel libcurl-devel \
> > +libjpeg-devel libpng-devel librbd-devel libssh2-devel libusbx-devel \
> > +libxml2-devel lzo-devel ncurses-devel nettle-devel nss-devel \
> > +numactl-devel SDL2-devel snappy-devel spice-server-devel \
> > +systemtap-sdt-devel usbredir-devel virglrenderer-devel vte3-devel \
> > +xen-devel \
> 
> This still has some notable differences from the build deps we use in
> formal Fedora QEMU RPMs. I think we have missing:
> 
>alsa-lib-devel
>capstone-devel
>cyrus-sasl-devel
>glusterfs-devel
>libcacard-devel
>libepoxy-devel
>libiscsi-devel
>libnfs-devel
>librdmacm-devel
>libseccomp-devel
>libtasn1-devel
>libuuid-devel
>mesa-libgbm-devel
>perl-podlators
>pulseaudio-libs-devel
>systemd-devel
>texinfo
>vte291-devel

I wonder if we should simply do "dnf build-dep qemu" and only list additional
(mingw, for example) packages explicitly, like the debian dockerfiles.

Fam

> 
> 
> Some extra bits that are not needed:
> 
>   glib2-devel is not required as gtk3-devel pulls it in
>   nettle-devel is not required as gnutls-devel puls it in
>   
>   nss-devel shouldn't be used by anything in QEMU - we use gnutls
>   
>   vte3-devel is obsoleted by vte291-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 \
> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] i386: add KNM cpu model

[Boqun Feng]

On Wed, Mar 14, 2018 at 01:01:30PM +, Daniel P. Berrangé wrote:
> On Wed, Mar 14, 2018 at 03:29:59PM +0800, Boqun Feng wrote:
> > A new cpu model called "KNM" is added to model Knights Mill processors.
> 
> Why the obscure acryonym? Can't we just call it KnightsMill so it is
> obvious what it is to everyone, as we've done for all other Intel
> CPU model names in the past.
> 

Ah.. my bad, good point! I will rename to "KnightsMill" in the next
version.

> > Compared to "Skylake-Server" cpu model, the following features are
> > added:
> > 
> > avx512_4vnniw avx512_4fmaps avx512pf avx512er avx512_vpopcntdq
> > 
> > and the following features are removed:
> > 
> > pcid invpcid clflushopt avx512dq avx512bw axv512cd clwb smap rtm
> > mpx xsavec xgetbv1 hle
> 
> "pcid" was one of the features critical to mitigate performance
> downside of the Meltdown bug fix in guests. Will Knights Mill have
> a fix for Meltdown in hardware, to avoid the need for split table
> pages and thus avoid the perf hit in guests ?
> 

I got the confirmation the Knights Mill doesn't have PCID at all. For
the alternative fix for Meltdown in hardware, I will ask around and info
you if I get something.

Regards,
Boqun

> > 
> > Signed-off-by: Boqun Feng 
> > ---
> >  target/i386/cpu.c | 42 ++
> >  1 file changed, 42 insertions(+)
> > 
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 2c04645ceac9..215a9ee6026a 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -1795,6 +1795,48 @@ static X86CPUDefinition builtin_x86_defs[] = {
> >  .xlevel = 0x8008,
> >  .model_id = "Intel Xeon Processor (Skylake, IBRS)",
> >  },
> > +{
> > +.name = "KNM",
> > +.level = 0xd,
> > +.vendor = CPUID_VENDOR_INTEL,
> > +.family = 6,
> > +.model = 133,
> > +.stepping = 0,
> > +.features[FEAT_1_EDX] =
> > +CPUID_VME | CPUID_SS | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR |
> > +CPUID_MMX | CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | 
> > CPUID_CMOV |
> > +CPUID_MCA | CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC |
> > +CPUID_CX8 | CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC |
> > +CPUID_PSE | CPUID_DE | CPUID_FP87,
> > +.features[FEAT_1_ECX] =
> > +CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES |
> > +CPUID_EXT_POPCNT | CPUID_EXT_X2APIC | CPUID_EXT_SSE42 |
> > +CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 |
> > +CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3 |
> > +CPUID_EXT_TSC_DEADLINE_TIMER | CPUID_EXT_FMA | CPUID_EXT_MOVBE 
> > |
> > +CPUID_EXT_F16C | CPUID_EXT_RDRAND,
> > +.features[FEAT_8000_0001_EDX] =
> > +CPUID_EXT2_LM | CPUID_EXT2_PDPE1GB | CPUID_EXT2_RDTSCP |
> > +CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
> > +.features[FEAT_8000_0001_ECX] =
> > +CPUID_EXT3_ABM | CPUID_EXT3_LAHF_LM | CPUID_EXT3_3DNOWPREFETCH,
> > +.features[FEAT_7_0_EBX] =
> > +CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 | 
> > CPUID_7_0_EBX_AVX2 |
> > +CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS |
> > +CPUID_7_0_EBX_RDSEED | CPUID_7_0_EBX_ADX | 
> > CPUID_7_0_EBX_AVX512F |
> > +CPUID_7_0_EBX_AVX512CD | CPUID_7_0_EBX_AVX512PF |
> > +CPUID_7_0_EBX_AVX512ER,
> > +.features[FEAT_7_0_ECX] =
> > +CPUID_7_0_ECX_AVX512_VPOPCNTDQ,
> > +.features[FEAT_7_0_EDX] =
> > +CPUID_7_0_EDX_AVX512_4VNNIW | CPUID_7_0_EDX_AVX512_4FMAPS,
> > +.features[FEAT_XSAVE] =
> > +CPUID_XSAVE_XSAVEOPT,
> > +.features[FEAT_6_EAX] =
> > +CPUID_6_EAX_ARAT,
> > +.xlevel = 0x8008,
> > +.model_id = "Intel Xeon Phi Processor (Knights Mill)",
> > +},
> >  {
> >  .name = "Opteron_G1",
> >  .level = 5,
> > -- 
> > 2.16.1
> > 
> > 
> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2] docker: dump 'config.log' if ./configure fails

[Fam Zheng]

On Thu, 03/15 15:27, Philippe Mathieu-Daudé wrote:
> Suggested-by: Eric Blake 
> Signed-off-by: Philippe Mathieu-Daudé 
> Reviewed-by: Daniel P. Berrangé 
> ---
> v2:
> - avoid subshell using { ; } (Eric)
> - use test_fail() instead of prep_fail() (Fam)
> 
>  tests/docker/common.rc | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/docker/common.rc b/tests/docker/common.rc
> index 7951555e3f..046f8a5921 100755
> --- a/tests/docker/common.rc
> +++ b/tests/docker/common.rc
> @@ -30,7 +30,9 @@ build_qemu()
>   $@"
>  echo "Configure options:"
>  echo $config_opts
> -$QEMU_SRC/configure $config_opts && make $MAKEFLAGS
> +$QEMU_SRC/configure $config_opts || \
> +{ cat config.log && test_fail "Failed to run 'configure'"; }
> +make $MAKEFLAGS
>  }
>  
>  test_fail()
> -- 
> 2.16.2
> 

Queued, thanks!

Fam

[Qemu-devel] [PATCH v3 0/7] s390x: vfio-ap: guest dedicated crypto adapters

[Tony Krowiak]

This patch series is the QEMU counterpart to the KVM/kernel support for 
guest dedicated crypto adapters. The KVM/kernel model is built on the 
VFIO mediated device framework and provides the infrastructure for 
granting exclusive guest access to crypto devices installed on the linux 
host. This patch series introduces a new QEMU command line option, QEMU 
object model and CPU model features to exploit the KVM/kernel model.

See the detailed specifications for AP virtualization provided by this 
patch set in docs/vfio-ap.txt for a more complete discussion of the 
design introduced by this patch series.

v2 -> v3 Change log:
===
* The corresponding KVM/kernel patch series defined a new device attribute
  to the KVM_S390_VM_CRYPTO attribute group to set interpretive execution
  of AP instructions for the guest. Now, when the vfio-ap device is 
  realized, the KVM_SET_DEVICE_ATTR ioctl is invoked to enable 
  interpretation of AP instructions executed on the guest.

* When the CPU model feature indicating AP instructions are supported for
  the guest (i.e., -cpu ,ap=on), the expectation is that the AP bus 
  will be initialized on the guest. In v2, however; AP instructions were
  intercepted unless a vfio-ap device (i.e., -device vfio-ap,sysfsdev=$path)
  was defined for the guest. Since there were no handlers defined to 
  process intercepted AP instructions, an operation exception was injected
  into the guest causing the AP bus to fail initialization. The v3 version
  adds handlers for intercepted AP instructions that incorporate the 
  following logic:

  If the CPU model indicates AP instructions are installed 
 Set the status response code for the instruction to indicate that
 the APQN contained in the instruction is not valid. This is actually
 true because the AP devices are configured for the guest via the  
 sysfs attribute files of the vfio-ap device, so any APQN would not
 be valid.
   
  Else
 Return an error from the handler. This will cause an operation 
 exception to be injected into the guest in which case the AP bus
 would not initialize. This is the same behavior as exists prior to 
 the introduction of this patch series.

* Miscellaneous code formatting and other trivial changes.

Tony Krowiak (7):
  linux-headers: linux header updates for AP support
  s390x/ap: base Adjunct Processor (AP) object
  s390x/cpumodel: Set up CPU model for AP device support
  s390x/kvm: interface to interpret AP instructions
  s390x/vfio: ap: Introduce VFIO AP device
  s390x/kvm: handle AP instruction interception
  s390: doc: detailed specifications for AP virtualization

 default-configs/s390x-softmmu.mak |1 +
 docs/vfio-ap.txt  |  624 +
 hw/s390x/Makefile.objs|1 +
 hw/s390x/ap-device.c  |   38 +++
 hw/vfio/Makefile.objs |1 +
 hw/vfio/ap.c  |  229 ++
 include/hw/s390x/ap-device.h  |   44 +++
 include/hw/vfio/vfio-common.h |1 +
 linux-headers/asm-s390/kvm.h  |2 +
 linux-headers/linux/vfio.h|2 +
 target/s390x/cpu_features.c   |3 +
 target/s390x/cpu_features_def.h   |3 +
 target/s390x/cpu_models.c |2 +
 target/s390x/gen-features.c   |3 +
 target/s390x/kvm.c|   31 ++
 target/s390x/kvm_s390x.h  |2 +
 16 files changed, 987 insertions(+), 0 deletions(-)
 create mode 100644 docs/vfio-ap.txt
 create mode 100644 hw/s390x/ap-device.c
 create mode 100644 hw/vfio/ap.c
 create mode 100644 include/hw/s390x/ap-device.h

[Qemu-devel] [PATCH v3 7/7] s390: doc: detailed specifications for AP virtualization

[Tony Krowiak]

This patch provides documentation describing the AP architecture and
design concepts behind the virtualization of AP devices. It also
includes an example of how to configure AP devices for exclusive
use of KVM guests.

Signed-off-by: Tony Krowiak 
---
 docs/vfio-ap.txt |  624 ++
 1 files changed, 624 insertions(+), 0 deletions(-)
 create mode 100644 docs/vfio-ap.txt

diff --git a/docs/vfio-ap.txt b/docs/vfio-ap.txt
new file mode 100644
index 000..54e7523
--- /dev/null
+++ b/docs/vfio-ap.txt
@@ -0,0 +1,624 @@
+Adjunct Processor (AP) Device
+=
+
+Contents:
+=
+* Introduction
+* AP Architectural Overview
+* Start Interpretive Execution (SIE) Instruction
+* AP Matrix Configuration on Linux Host
+* AP Matrix Configuration for a Linux Guest
+* Starting a Linux Guest Configured with an AP Matrix
+* Example: Configure AP Matrices for Two Linux Guests
+
+Introduction:
+
+The IBM Adjunct Processor (AP) Cryptographic Facility is comprised
+of three AP instructions and from 1 to 256 PCIe cryptographic adapter cards.
+These AP devices provide cryptographic functions to all CPUs assigned to a
+linux system running in an IBM Z system LPAR.
+
+On s390x, AP adapter cards are exposed via the AP bus. This document
+describes how those cards may be made available to KVM guests using the
+VFIO mediated device framework.
+
+AP Architectural Overview:
+=
+In order understand the terminology used in the rest of this document, let's
+start with some definitions:
+
+* AP adapter
+
+  An AP adapter is an IBM Z adapter card that can perform cryptographic
+  functions. There can be from 0 to 256 adapters assigned to an LPAR. Adapters
+  assigned to the LPAR in which a linux host is running will be available to
+  the linux host. Each adapter is identified by a number from 0 to 255. When
+  installed, an AP adapter is accessed by AP instructions executed by any CPU.
+
+* AP domain
+
+  An adapter is partitioned into domains. Each domain can be thought of as
+  a set of hardware registers for processing AP instructions. An adapter can
+  hold up to 256 domains. Each domain is identified by a number from 0 to 255.
+  Domains can be further classified into two types:
+
+* Usage domains are domains that can be accessed directly to process AP
+  commands
+
+* Control domains are domains that are accessed indirectly by AP
+  commands sent to a usage domain to control or change the domain, for
+  example; to set a secure private key for the domain.
+
+* AP Queue
+
+  An AP queue is the means by which an AP command-request message is sent to an
+  AP usage domain inside a specific AP. An AP queue is identified by a tuple
+  comprised of an AP adapter ID (APID) and an AP queue index (APQI). The
+  APQI corresponds to a given usage domain number within the adapter. This 
tuple
+  forms an AP Queue Number (APQN) uniquely identifying an AP queue. AP
+  instructions include a field containing the APQN to identify the AP queue to
+  which the AP command-request message is to be sent for processing.
+
+* AP Instructions:
+
+  There are three AP instructions:
+
+  * NQAP: to enqueue an AP command-request message to a queue
+  * DQAP: to dequeue an AP command-reply message from a queue
+  * PQAP: to administer the queues
+
+Start Interpretive Execution (SIE) Instruction
+==
+A KVM guest is started by executing the Start Interpretive Execution (SIE)
+instruction. The SIE state description is a control block that contains the
+state information for a KVM guest and is supplied as input to the SIE
+instruction. The SIE state description contains a field that references
+a Crypto Control Block (CRYCB). The CRYCB contains three fields to identify the
+adapters, usage domains and control domains assigned to the KVM guest:
+
+* The AP Mask (APM) field is a bit mask that identifies the AP adapters 
assigned
+  to the KVM guest. Each bit in the mask, from most significant to least
+  significant bit, corresponds to an APID from 0-255. If a bit is set, the
+  corresponding adapter is valid for use by the KVM guest.
+
+* The AP Queue Mask (AQM) field is a bit mask identifying the AP queues 
assigned
+  to the KVM guest. Each bit in the mask, from most significant to least
+  significant bit, corresponds to an AP queue index (APQI) from 0-255. If a bit
+  is set, the corresponding queue is valid for use by the KVM guest.
+
+* The AP Domain Mask field is a bit mask that identifies the AP control domains
+  assigned to the KVM guest. The ADM bit mask controls which domains can be
+  changed by an AP command-request message sent to a usage domain from the
+  guest. Each bit in the mask, from least significant to most significant bit,
+  corresponds to a domain from 0-255. If a bit is set, the corresponding domain
+  can be modified by an AP 

[Qemu-devel] [PATCH v3 3/7] s390x/cpumodel: Set up CPU model for AP device support

[Tony Krowiak]

A new CPU model feature and two new CPU model facilities are
introduced to support AP devices for a KVM guest.

CPU model features:

1. The KVM_S390_VM_CPU_FEAT_AP CPU model feature indicates that
   AP facilities are installed. This feature will be enabled by
   the kernel only if the AP facilities are installed on the linux
   host. This feature must be turned on from userspace to access
   AP devices from the KVM guest. The QEMU command line to turn
   this feature looks something like this:

qemu-system-s390x ... -cpu xxx,ap=on

CPU model facilities:

1. The S390_FEAT_AP_QUERY_CONFIG_INFO feature indicates the AP Query
   Configuration Information (QCI) facility is installed. This feature
   will be enabled by the kernel only if the QCI is installed on
   the host.

2. The S390_FEAT_AP_FACILITY_TEST feature indicates that the AP
   Facility Test (APFT) facility is installed. This feature will
   be enabled by the kernel only if the APFT facility is installed
   on the host.

Signed-off-by: Tony Krowiak 
---
 target/s390x/cpu_features.c |3 +++
 target/s390x/cpu_features_def.h |3 +++
 target/s390x/cpu_models.c   |2 ++
 target/s390x/gen-features.c |3 +++
 target/s390x/kvm.c  |1 +
 5 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index a5619f2..1abe987 100644
--- a/target/s390x/cpu_features.c
+++ b/target/s390x/cpu_features.c
@@ -36,8 +36,10 @@ static const S390FeatDef s390_features[] = {
 FEAT_INIT("srs", S390_FEAT_TYPE_STFL, 9, "Sense-running-status facility"),
 FEAT_INIT("csske", S390_FEAT_TYPE_STFL, 10, "Conditional-SSKE facility"),
 FEAT_INIT("ctop", S390_FEAT_TYPE_STFL, 11, "Configuration-topology 
facility"),
+FEAT_INIT("qci", S390_FEAT_TYPE_STFL, 12, "Query AP Configuration 
facility"),
 FEAT_INIT("ipter", S390_FEAT_TYPE_STFL, 13, "IPTE-range facility"),
 FEAT_INIT("nonqks", S390_FEAT_TYPE_STFL, 14, "Nonquiescing key-setting 
facility"),
+FEAT_INIT("apft", S390_FEAT_TYPE_STFL, 15, "Adjunct Processor Facilities 
Test facility"),
 FEAT_INIT("etf2", S390_FEAT_TYPE_STFL, 16, "Extended-translation facility 
2"),
 FEAT_INIT("msa-base", S390_FEAT_TYPE_STFL, 17, "Message-security-assist 
facility (excluding subfunctions)"),
 FEAT_INIT("ldisp", S390_FEAT_TYPE_STFL, 18, "Long-displacement facility"),
@@ -125,6 +127,7 @@ static const S390FeatDef s390_features[] = {
 
 FEAT_INIT("dateh2", S390_FEAT_TYPE_MISC, 0, "DAT-enhancement facility 2"),
 FEAT_INIT("cmm", S390_FEAT_TYPE_MISC, 0, "Collaborative-memory-management 
facility"),
+FEAT_INIT("ap", S390_FEAT_TYPE_MISC, 0, "AP facilities installed"),
 
 FEAT_INIT("plo-cl", S390_FEAT_TYPE_PLO, 0, "PLO Compare and load (32 bit 
in general registers)"),
 FEAT_INIT("plo-clg", S390_FEAT_TYPE_PLO, 1, "PLO Compare and load (64 bit 
in parameter list)"),
diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
index 7c5915c..8998b65 100644
--- a/target/s390x/cpu_features_def.h
+++ b/target/s390x/cpu_features_def.h
@@ -27,8 +27,10 @@ typedef enum {
 S390_FEAT_SENSE_RUNNING_STATUS,
 S390_FEAT_CONDITIONAL_SSKE,
 S390_FEAT_CONFIGURATION_TOPOLOGY,
+S390_FEAT_AP_QUERY_CONFIG_INFO,
 S390_FEAT_IPTE_RANGE,
 S390_FEAT_NONQ_KEY_SETTING,
+S390_FEAT_AP_FACILITIES_TEST,
 S390_FEAT_EXTENDED_TRANSLATION_2,
 S390_FEAT_MSA,
 S390_FEAT_LONG_DISPLACEMENT,
@@ -118,6 +120,7 @@ typedef enum {
 /* Misc */
 S390_FEAT_DAT_ENH_2,
 S390_FEAT_CMM,
+S390_FEAT_AP,
 
 /* PLO */
 S390_FEAT_PLO_CL,
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index cf82589..7e2af09 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -769,6 +769,8 @@ static void check_consistency(const S390CPUModel *model)
 { S390_FEAT_PRNO_TRNG_QRTCR, S390_FEAT_MSA_EXT_5 },
 { S390_FEAT_PRNO_TRNG, S390_FEAT_MSA_EXT_5 },
 { S390_FEAT_SIE_KSS, S390_FEAT_SIE_F2 },
+{ S390_FEAT_AP_QUERY_CONFIG_INFO, S390_FEAT_AP },
+{ S390_FEAT_AP_FACILITIES_TEST, S390_FEAT_AP },
 };
 int i;
 
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 0cdbc15..0d5b0f7 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -447,6 +447,9 @@ static uint16_t full_GEN12_GA1[] = {
 S390_FEAT_ADAPTER_INT_SUPPRESSION,
 S390_FEAT_EDAT_2,
 S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
+S390_FEAT_AP_QUERY_CONFIG_INFO,
+S390_FEAT_AP_FACILITIES_TEST,
+S390_FEAT_AP,
 };
 
 static uint16_t full_GEN12_GA2[] = {
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index fbc887e..33e5ec3 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2178,6 +2178,7 @@ static int kvm_to_feat[][2] = {
 { KVM_S390_VM_CPU_FEAT_PFMFI, S390_FEAT_SIE_PFMFI},
 { KVM_S390_VM_CPU_FEAT_SIGPIF, S390_FEAT_SIE_SIGPIF},
 { 

[Qemu-devel] [PATCH v3 4/7] s390x/kvm: interface to interpret AP instructions

[Tony Krowiak]

The VFIO AP device exploits interpretive execution of AP
instructions (APIE). APIE is enabled by setting a device attribute
via the KVM_SET_DEVICE_ATTR ioctl.

Signed-off-by: Tony Krowiak 
---
 target/s390x/kvm.c   |   16 
 target/s390x/kvm_s390x.h |2 ++
 2 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 33e5ec3..2812e28 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -277,6 +277,22 @@ static void kvm_s390_init_dea_kw(void)
 }
 }
 
+int kvm_s390_set_interpret_ap(uint8_t enable)
+{
+struct kvm_device_attr attribute = {
+.group = KVM_S390_VM_CRYPTO,
+.attr  = KVM_S390_VM_CRYPTO_INTERPRET_AP,
+.addr = 1,
+};
+
+if (!kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO,
+   KVM_S390_VM_CRYPTO_INTERPRET_AP)) {
+return -EOPNOTSUPP;
+}
+
+return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, );
+}
+
 void kvm_s390_crypto_reset(void)
 {
 if (s390_has_feat(S390_FEAT_MSA_EXT_3)) {
diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h
index 34ee7e7..0d6c6e7 100644
--- a/target/s390x/kvm_s390x.h
+++ b/target/s390x/kvm_s390x.h
@@ -40,4 +40,6 @@ void kvm_s390_crypto_reset(void);
 void kvm_s390_restart_interrupt(S390CPU *cpu);
 void kvm_s390_stop_interrupt(S390CPU *cpu);
 
+int kvm_s390_set_interpret_ap(uint8_t enable);
+
 #endif /* KVM_S390X_H */
-- 
1.7.1

[Qemu-devel] [PATCH v3 6/7] s390x/kvm: handle AP instruction interception

[Tony Krowiak]

If the CPU model indicates that AP facility is installed on
the guest (i.e., -cpu ,ap=on), then the expectation is that
the AP bus running in the guest will initialize; however, if the
AP instructions are not being interpreted by the firmware, then
they will be intercepted and routed back to QEMU for handling.
If a handler is not defined to process the intercepted instruciton,
then an operation exception will be injected into the
guest, in which case the AP bus will not initialize.

There are two situations where AP instructions will not be
interpreted:

1. The guest is not configured with a vfio-ap device (i.e.,
   -device vfio-ap,sysfsdev=$path-to-mdev). The realize function for
   the vfio-ap device enables interpretive execution of AP
   instructions.

2. The guest is a second level guest but the first level guest has
   not enabled interpretive execution.

This patch introduces AP instruction handlers to ensure the AP bus
module initializes on the guest when the AP facility is installed
on the guest but AP instructions are not being interpreted. The logic
incorporated is:

* If the CPU model indicates AP instructions are
  installed

  * Set the status response code for the instruction to indicate that
the APQN contained in the instruction is not valid. This is
a valid response because there will be no devices configured for
the guest in any of the above scenarios.

* Else return an error from the handler. This will result in an
  operation being injected into the guest and the AP bus will not
  initialize on the guest. That is commensurate with how things work
  today.

Signed-off-by: Tony Krowiak 
---
 hw/vfio/ap.c |   45 ++
 include/hw/s390x/ap-device.h |6 +
 target/s390x/kvm.c   |   14 +
 3 files changed, 65 insertions(+), 0 deletions(-)

diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index b397bb1..88e744d 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -148,6 +148,51 @@ static void vfio_ap_unrealize(DeviceState *dev, Error 
**errp)
 kvm_s390_set_interpret_ap(0);
 }
 
+int ap_device_handle_nqap(S390CPU *cpu)
+{
+CPUS390XState *env = >env;
+
+if (s390_has_feat(S390_FEAT_AP)) {
+env->regs[1] = 0x1;
+
+return 0;
+}
+
+return -EOPNOTSUPP;
+}
+
+int ap_device_handle_dqap(S390CPU *cpu)
+{
+CPUS390XState *env = >env;
+
+if (s390_has_feat(S390_FEAT_AP)) {
+env->regs[1] = 0x1;
+
+return 0;
+}
+
+return -EOPNOTSUPP;
+}
+
+int ap_device_handle_pqap(S390CPU *cpu)
+{
+CPUS390XState *env = >env;
+int fc = 4 & (env->regs[0] >> 24);
+
+/*
+ * The Query Configuration Information (QCI) function (fc == 4) does not
+ * set a response code in reg 1, so check for that along with the
+ * AP feature.
+ */
+if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) {
+env->regs[1] = 0x1;
+
+return 0;
+}
+
+return -EOPNOTSUPP;
+}
+
 static Property vfio_ap_properties[] = {
 DEFINE_PROP_STRING("sysfsdev", VFIOAPDevice, vdev.sysfsdev),
 DEFINE_PROP_END_OF_LIST(),
diff --git a/include/hw/s390x/ap-device.h b/include/hw/s390x/ap-device.h
index 693df90..d45ae38 100644
--- a/include/hw/s390x/ap-device.h
+++ b/include/hw/s390x/ap-device.h
@@ -11,6 +11,8 @@
 #ifndef HW_S390X_AP_DEVICE_H
 #define HW_S390X_AP_DEVICE_H
 
+#include "cpu.h"
+
 #define AP_DEVICE_TYPE   "ap-device"
 
 typedef struct APDevice {
@@ -35,4 +37,8 @@ static inline APDevice *to_ap_dev(DeviceState *dev)
 #define AP_DEVICE_CLASS(klass) \
 OBJECT_CLASS_CHECK(APDeviceClass, (klass), AP_DEVICE_TYPE)
 
+int ap_device_handle_nqap(S390CPU *cpu);
+int ap_device_handle_dqap(S390CPU *cpu);
+int ap_device_handle_pqap(S390CPU *cpu);
+
 #endif /* HW_S390X_AP_DEVICE_H */
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 2812e28..a636394 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -50,6 +50,7 @@
 #include "exec/memattrs.h"
 #include "hw/s390x/s390-virtio-ccw.h"
 #include "hw/s390x/s390-virtio-hcall.h"
+#include "hw/s390x/ap-device.h"
 
 #ifndef DEBUG_KVM
 #define DEBUG_KVM  0
@@ -88,6 +89,9 @@
 #define PRIV_B2_CHSC0x5f
 #define PRIV_B2_SIGA0x74
 #define PRIV_B2_XSCH0x76
+#define PRIV_B2_NQAP0xad
+#define PRIV_B2_DQAP0xae
+#define PRIV_B2_PQAP0xaf
 
 #define PRIV_EB_SQBS0x8a
 #define PRIV_EB_PCISTB  0xd0
@@ -1245,6 +1249,16 @@ static int handle_b2(S390CPU *cpu, struct kvm_run *run, 
uint8_t ipa1)
 case PRIV_B2_SCLP_CALL:
 rc = kvm_sclp_service_call(cpu, run, ipbh0);
 break;
+case PRIV_B2_NQAP:
+rc = ap_device_handle_nqap(cpu);
+break;
+case PRIV_B2_DQAP:
+rc = ap_device_handle_dqap(cpu);
+break;
+case PRIV_B2_PQAP:
+rc = ap_device_handle_pqap(cpu);
+break;
+   

[Qemu-devel] [PATCH v3 5/7] s390x/vfio: ap: Introduce VFIO AP device

[Tony Krowiak]

Introduces a VFIO based AP device. The device is defined via
the QEMU command line by specifying:

-device vfio-ap,sysfsdev=

The mediated matrix device is created by the VFIO AP device
driver by writing a UUID to a sysfs attribute file (see
docs/vfio-ap.txt). The mediated matrix device will be named
after the UUID. Symbolic links to the $uuid are created in
many places, so the path to the mediated matrix device $uuid
can be specified in any of the following ways:

/sys/devices/vfio_ap/matrix/$uuid
/sys/devices/vfio_ap/matrix/mdev_supported_types/vfio_ap-passthrough/devices/$uuid
/sys/bus/mdev/devices/$uuid
/sys/bus/mdev/drivers/vfio_mdev/$uuid

When the vfio-ap device is realized, it acquires and opens the
VFIO iommu group to which the mediated matrix device is
bound. This causes a VFIO group notification event to be
signaled. The vfio_ap device driver's group notification
handler will get called at which time the device driver
will configure the the AP devices to which the guest will
be granted access.

Signed-off-by: Tony Krowiak 
---
 default-configs/s390x-softmmu.mak |1 +
 hw/vfio/Makefile.objs |1 +
 hw/vfio/ap.c  |  184 +
 include/hw/vfio/vfio-common.h |1 +
 4 files changed, 187 insertions(+), 0 deletions(-)
 create mode 100644 hw/vfio/ap.c

diff --git a/default-configs/s390x-softmmu.mak 
b/default-configs/s390x-softmmu.mak
index 2f4bfe7..0b784b6 100644
--- a/default-configs/s390x-softmmu.mak
+++ b/default-configs/s390x-softmmu.mak
@@ -9,3 +9,4 @@ CONFIG_S390_FLIC=y
 CONFIG_S390_FLIC_KVM=$(CONFIG_KVM)
 CONFIG_VFIO_CCW=$(CONFIG_LINUX)
 CONFIG_WDT_DIAG288=y
+CONFIG_VFIO_AP=$(CONFIG_LINUX)
diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
index c3ab909..7300860 100644
--- a/hw/vfio/Makefile.objs
+++ b/hw/vfio/Makefile.objs
@@ -6,4 +6,5 @@ obj-$(CONFIG_SOFTMMU) += platform.o
 obj-$(CONFIG_VFIO_XGMAC) += calxeda-xgmac.o
 obj-$(CONFIG_VFIO_AMD_XGBE) += amd-xgbe.o
 obj-$(CONFIG_SOFTMMU) += spapr.o
+obj-$(CONFIG_VFIO_AP) += ap.o
 endif
diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
new file mode 100644
index 000..b397bb1
--- /dev/null
+++ b/hw/vfio/ap.c
@@ -0,0 +1,184 @@
+/*
+ * VFIO based AP matrix device assignment
+ *
+ * Copyright 2018 IBM Corp.
+ * Author(s): Tony Krowiak 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+#include 
+#include 
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/sysbus.h"
+#include "hw/vfio/vfio.h"
+#include "hw/vfio/vfio-common.h"
+#include "hw/s390x/ap-device.h"
+#include "qemu/error-report.h"
+#include "qemu/queue.h"
+#include "cpu.h"
+#include "kvm_s390x.h"
+
+#define VFIO_AP_DEVICE_TYPE  "vfio-ap"
+
+typedef struct VFIOAPDevice {
+APDevice apdev;
+VFIODevice vdev;
+QTAILQ_ENTRY(VFIOAPDevice) sibling;
+} VFIOAPDevice;
+
+static void vfio_ap_compute_needs_reset(VFIODevice *vdev)
+{
+vdev->needs_reset = false;
+}
+
+/*
+ * We don't need vfio_hot_reset_multi and vfio_eoi operations for
+ * vfio-ap-matrix device now.
+ */
+struct VFIODeviceOps vfio_ap_ops = {
+.vfio_compute_needs_reset = vfio_ap_compute_needs_reset,
+};
+
+static QTAILQ_HEAD(, VFIOAPDevice) vfio_ap_devices =
+QTAILQ_HEAD_INITIALIZER(vfio_ap_devices);
+
+static void vfio_put_device(VFIOAPDevice *apdev)
+{
+g_free(apdev->vdev.name);
+vfio_put_base_device(>vdev);
+}
+
+static VFIOGroup *vfio_ap_get_group(VFIOAPDevice *vapdev, Error **errp)
+{
+char *tmp, group_path[PATH_MAX];
+ssize_t len;
+int groupid;
+
+tmp = g_strdup_printf("%s/iommu_group", vapdev->vdev.sysfsdev);
+len = readlink(tmp, group_path, sizeof(group_path));
+g_free(tmp);
+
+if (len <= 0 || len >= sizeof(group_path)) {
+error_setg(errp, "%s: no iommu_group found for %s",
+   VFIO_AP_DEVICE_TYPE, vapdev->vdev.sysfsdev);
+return NULL;
+}
+
+group_path[len] = 0;
+
+if (sscanf(basename(group_path), "%d", ) != 1) {
+error_setg(errp, "vfio: failed to read %s", group_path);
+return NULL;
+}
+
+return vfio_get_group(groupid, _space_memory, errp);
+}
+
+static void vfio_ap_realize(DeviceState *dev, Error **errp)
+{
+VFIODevice *vbasedev;
+VFIOGroup *vfio_group;
+APDevice *apdev = DO_UPCAST(APDevice, parent_obj, dev);
+VFIOAPDevice *vapdev = DO_UPCAST(VFIOAPDevice, apdev, apdev);
+char *mdevid;
+Error *local_err = NULL;
+int ret;
+
+if (!s390_has_feat(S390_FEAT_AP)) {
+error_setg(_err, "AP support not enabled");
+goto out_err;
+}
+
+ret = kvm_s390_set_interpret_ap(1);
+if (ret) {
+error_setg_errno(_err, errno,
+ "error setting interpretive execution of AP 
instructions");
+goto out_err;
+}
+
+vfio_group = 

[Qemu-devel] [PATCH v3 1/7] linux-headers: linux header updates for AP support

[Tony Krowiak]

Updates the linux header files in preparation for introduction
of the VFIO AP device:

* Added a feature ID to indicate AP facilities are installed

* Added a device attribute to the KVM_S390_VM_CRYPTO group to
  indicate whether AP instructions are to be interpreted

* Added VFIO device information for AP devices

Signed-off-by: Tony Krowiak 
---
 linux-headers/asm-s390/kvm.h |2 ++
 linux-headers/linux/vfio.h   |2 ++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
index 11def14..391b250 100644
--- a/linux-headers/asm-s390/kvm.h
+++ b/linux-headers/asm-s390/kvm.h
@@ -130,6 +130,7 @@ struct kvm_s390_vm_cpu_machine {
 #define KVM_S390_VM_CPU_FEAT_PFMFI 11
 #define KVM_S390_VM_CPU_FEAT_SIGPIF12
 #define KVM_S390_VM_CPU_FEAT_KSS   13
+#define KVM_S390_VM_CPU_FEAT_AP14
 struct kvm_s390_vm_cpu_feat {
__u64 feat[16];
 };
@@ -160,6 +161,7 @@ struct kvm_s390_vm_cpu_subfunc {
 #define KVM_S390_VM_CRYPTO_ENABLE_DEA_KW   1
 #define KVM_S390_VM_CRYPTO_DISABLE_AES_KW  2
 #define KVM_S390_VM_CRYPTO_DISABLE_DEA_KW  3
+#define KVM_S390_VM_CRYPTO_INTERPRET_AP4
 
 /* kvm attributes for migration mode */
 #define KVM_S390_VM_MIGRATION_STOP 0
diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
index 4312e96..91298dc 100644
--- a/linux-headers/linux/vfio.h
+++ b/linux-headers/linux/vfio.h
@@ -200,6 +200,7 @@ struct vfio_device_info {
 #define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2)/* vfio-platform device */
 #define VFIO_DEVICE_FLAGS_AMBA  (1 << 3)   /* vfio-amba device */
 #define VFIO_DEVICE_FLAGS_CCW  (1 << 4)/* vfio-ccw device */
+#define VFIO_DEVICE_FLAGS_AP (1 << 5)  /* vfio-ap device */
__u32   num_regions;/* Max region index + 1 */
__u32   num_irqs;   /* Max IRQ index + 1 */
 };
@@ -215,6 +216,7 @@ struct vfio_device_info {
 #define VFIO_DEVICE_API_PLATFORM_STRING"vfio-platform"
 #define VFIO_DEVICE_API_AMBA_STRING"vfio-amba"
 #define VFIO_DEVICE_API_CCW_STRING "vfio-ccw"
+#define VFIO_DEVICE_API_AP_STRING  "vfio-ap"
 
 /**
  * VFIO_DEVICE_GET_REGION_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 8,
-- 
1.7.1

[Qemu-devel] [PATCH v3 2/7] s390x/ap: base Adjunct Processor (AP) object

[Tony Krowiak]

This patch introduces the base object for an AP device.

Signed-off-by: Tony Krowiak 
---
 hw/s390x/Makefile.objs   |1 +
 hw/s390x/ap-device.c |   38 ++
 include/hw/s390x/ap-device.h |   38 ++
 3 files changed, 77 insertions(+), 0 deletions(-)
 create mode 100644 hw/s390x/ap-device.c
 create mode 100644 include/hw/s390x/ap-device.h

diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index dc704b5..3247a07 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -17,3 +17,4 @@ obj-y += s390-stattrib.o
 obj-$(CONFIG_KVM) += s390-skeys-kvm.o
 obj-$(CONFIG_KVM) += s390-stattrib-kvm.o
 obj-y += s390-ccw.o
+obj-y += ap-device.o
diff --git a/hw/s390x/ap-device.c b/hw/s390x/ap-device.c
new file mode 100644
index 000..448f6db
--- /dev/null
+++ b/hw/s390x/ap-device.c
@@ -0,0 +1,38 @@
+/*
+ * Adjunct Processor (AP) matrix device
+ *
+ * Copyright 2018 IBM Corp.
+ * Author(s): Tony Krowiak 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+#include "qemu/osdep.h"
+#include "qemu/module.h"
+#include "qapi/error.h"
+#include "hw/qdev.h"
+#include "hw/s390x/ap-device.h"
+
+static void ap_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+
+dc->desc = "AP device class";
+}
+
+static const TypeInfo ap_device_info = {
+.name = AP_DEVICE_TYPE,
+.parent = TYPE_DEVICE,
+.instance_size = sizeof(APDevice),
+.class_size = sizeof(APDeviceClass),
+.class_init = ap_class_init,
+.abstract = true,
+};
+
+static void ap_device_register(void)
+{
+type_register_static(_device_info);
+}
+
+type_init(ap_device_register)
diff --git a/include/hw/s390x/ap-device.h b/include/hw/s390x/ap-device.h
new file mode 100644
index 000..693df90
--- /dev/null
+++ b/include/hw/s390x/ap-device.h
@@ -0,0 +1,38 @@
+/*
+ * Adjunct Processor (AP) matrix device interfaces
+ *
+ * Copyright 2018 IBM Corp.
+ * Author(s): Tony Krowiak 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+#ifndef HW_S390X_AP_DEVICE_H
+#define HW_S390X_AP_DEVICE_H
+
+#define AP_DEVICE_TYPE   "ap-device"
+
+typedef struct APDevice {
+DeviceState parent_obj;
+} APDevice;
+
+typedef struct APDeviceClass {
+DeviceClass parent_class;
+} APDeviceClass;
+
+static inline APDevice *to_ap_dev(DeviceState *dev)
+{
+return container_of(dev, APDevice, parent_obj);
+}
+
+#define AP_DEVICE(obj) \
+OBJECT_CHECK(APDevice, (obj), AP_DEVICE_TYPE)
+
+#define AP_DEVICE_GET_CLASS(obj) \
+OBJECT_GET_CLASS(APDeviceClass, (obj), AP_DEVICE_TYPE)
+
+#define AP_DEVICE_CLASS(klass) \
+OBJECT_CLASS_CHECK(APDeviceClass, (klass), AP_DEVICE_TYPE)
+
+#endif /* HW_S390X_AP_DEVICE_H */
-- 
1.7.1

Re: [Qemu-devel] [PATCH 2/2] char: i.MX: Add support for "TX complete" interrupt

[Bill Paul]

Of all the gin joints in all the towns in all the world, Andrey Smirnov had to 
walk into mine at 12:11 on Thursday 15 March 2018 and say:

> Add support for "TX complete"/TXDC interrupt generate by real HW since
> it is needed to support guests other than Linux.
> 
> Based on the patch by Bill Paul as found here:
> https://bugs.launchpad.net/qemu/+bug/1753314
> 
> Cc: qemu-devel@nongnu.org
> Cc: qemu-...@nongnu.org
> Cc: Bill Paul 
> Cc: Peter Maydell 
> Signed-off-by: Andrey Smirnov 
> ---
> 
> Bill:
> 
> I only tested this with i.MX7/Linux guest combo and am hoping you can
> take this and previous patch and give it a try on your VxWorks setup.

I tried it and it seems to work as well as my original patch did. There is one 
thing I noticed, which is that when printing a lot of output to the console, 
sometimes the output will stall until you press a key to generate an input 
interrupt, and then it resumes. This happens with my original patch too. My 
suspicion is that this has to do with the lack of emulation of the FIFO 
feature of the i.MX6 UART, but I'm not certain. (It's still much better than 
not having it work at all, so I didn't really mind this. :) ) All I know for 
sure is that the stalls don't happen on real hardware.

FYI, I uploaded a sample VxWorks image that you can use for testing. You can 
find it here:

http://people.freebsd.org/~wpaul/qemu/sabrelite

The file vxWorks is the kernel, which is really all you need. This is an SMP 
image so you need to run QEMU with the -smp 4 option. The uVxWorks image and 
.dtb file are used on a real board with U-Boot and the bootm command. The 
qemu_imx.sh script contains the options I use for testing. I usually do:

% qemu_imx.sh vxWorks

You can download all the files at once by grabbing:

http://people.freebsd.org/~wpaul/qemu/sabrelite/vxworks_test.zip

At the -> prompt, you can type the following things (among others):

-> i -- show running task info
-> vxbIoctlShow -- show VxBus device tree
-> vxbDevShow -- show VxBus device tree in a different format
-> vxbDrvShow -- show VxBus drivers
-> vxbIntShow -- show information about interrupt bindings
-> vmContextShow -- show MMU mappings
-> memShow -- show heap usage
-> ifconfig -- show network interfaces
-> routec "show" -- show network routing table
-> netstat "-a" -- show network socket info

The image also includes network support. You can use ifconfig to set the enet0 
interface's addresss like this:

-> ifconfig "enet0 10.0.2.15 netmask 255.255.255.0 up"

You should then be able to telnet to the VxWorks image from the host machine 
by doing:

% telnet localhost 10023

To exit the telnet session, type logout:

-> logout

NOTE: this only works if you've patched the imx_fec module to fix the 
interrupt vector bug that I also reported.

If you run vxbIoctlShow, which generates a lot of output, you should be able 
to see the stall condition I'm talking about. If you don't, then maybe it has 
something to do with me running QEMU on FreeBSD.
 
> Peter:
> 
> Bill is the author of original code, so I am not sure how to handle
> Signed-off-by from him. I'd like to add it to this patch, but since
> his original submission to launchpad didn't have one it is currently
> omitted.

My original goal was to report the bug and provide as much info as I could on 
how to maybe fix it, and let somebody else make a proper fix/patch.

-Bill
 
>  include/hw/char/imx_serial.h |  3 +++
>  hw/char/imx_serial.c | 20 +---
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/char/imx_serial.h b/include/hw/char/imx_serial.h
> index baeec3183f..5b99cee7cf 100644
> --- a/include/hw/char/imx_serial.h
> +++ b/include/hw/char/imx_serial.h
> @@ -67,6 +67,8 @@
>  #define UCR2_RXEN   (1<<1)/* Receiver enable */
>  #define UCR2_SRST   (1<<0)/* Reset complete */
> 
> +#define UCR4_TCEN   BIT(3)/* TX complete interrupt enable */
> +
>  #define UTS1_TXEMPTY(1<<6)
>  #define UTS1_RXEMPTY(1<<5)
>  #define UTS1_TXFULL (1<<4)
> @@ -95,6 +97,7 @@ typedef struct IMXSerialState {
>  uint32_t ubmr;
>  uint32_t ubrc;
>  uint32_t ucr3;
> +uint32_t ucr4;
> 
>  qemu_irq irq;
>  CharBackend chr;
> diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c
> index d1e8586280..1e5540472b 100644
> --- a/hw/char/imx_serial.c
> +++ b/hw/char/imx_serial.c
> @@ -37,8 +37,8 @@
> 
>  static const VMStateDescription vmstate_imx_serial = {
>  .name = TYPE_IMX_SERIAL,
> -.version_id = 1,
> -.minimum_version_id = 1,
> +.version_id = 2,
> +.minimum_version_id = 2,
>  .fields = (VMStateField[]) {
>  VMSTATE_INT32(readbuff, IMXSerialState),
>  VMSTATE_UINT32(usr1, IMXSerialState),
> @@ -50,6 +50,7 @@ static const VMStateDescription vmstate_imx_serial = {
>  VMSTATE_UINT32(ubmr, IMXSerialState),
>  VMSTATE_UINT32(ubrc, 

Re: [Qemu-devel] [PATCH 2/2] char: i.MX: Add support for "TX complete" interrupt

[Bill Paul]

Of all the gin joints in all the towns in all the world, Bill Paul had to walk 
into mine at 13:45 on Thursday 15 March 2018 and say:

> Of all the gin joints in all the towns in all the world, Andrey Smirnov had
> to
> 
> walk into mine at 12:11 on Thursday 15 March 2018 and say:
> > Add support for "TX complete"/TXDC interrupt generate by real HW since
> > it is needed to support guests other than Linux.
> > 
> > Based on the patch by Bill Paul as found here:
> > https://bugs.launchpad.net/qemu/+bug/1753314
> > 
> > Cc: qemu-devel@nongnu.org
> > Cc: qemu-...@nongnu.org
> > Cc: Bill Paul 
> > Cc: Peter Maydell 
> > Signed-off-by: Andrey Smirnov 
> > ---
> > 
> > Bill:
> > 
> > I only tested this with i.MX7/Linux guest combo and am hoping you can
> > take this and previous patch and give it a try on your VxWorks setup.
> 
> I tried it and it seems to work as well as my original patch did. There is
> one thing I noticed, which is that when printing a lot of output to the
> console, sometimes the output will stall until you press a key to generate
> an input interrupt, and then it resumes. This happens with my original
> patch too. My suspicion is that this has to do with the lack of emulation
> of the FIFO feature of the i.MX6 UART, but I'm not certain. (It's still
> much better than not having it work at all, so I didn't really mind this.
> :) ) All I know for sure is that the stalls don't happen on real hardware.

In retrospect, I think the problem may not be with the UART emulation. I went 
back and created a uniprocessor VxWorks image instead of an SMP image, and it 
doesn't seem to have the same stall behavior.

I added a vxWorks_up image to the URL below so that you can try that too. It 
may well be that having more than a 1 byte FIFO would fix the problem with the 
SMP image too, but I don't want to force you to implement that. If you want to 
try to fix the issue, great, otherwise I am happy with the patch as it is, so 
I will provide this:

Signed-off-by: Bill Paul 

-Bill

> 
> FYI, I uploaded a sample VxWorks image that you can use for testing. You
> can find it here:
> 
> http://people.freebsd.org/~wpaul/qemu/sabrelite
> 
> The file vxWorks is the kernel, which is really all you need. This is an
> SMP image so you need to run QEMU with the -smp 4 option. The uVxWorks
> image and .dtb file are used on a real board with U-Boot and the bootm
> command. The qemu_imx.sh script contains the options I use for testing. I
> usually do:
> 
> % qemu_imx.sh vxWorks
> 
> You can download all the files at once by grabbing:
> 
> http://people.freebsd.org/~wpaul/qemu/sabrelite/vxworks_test.zip
> 
> At the -> prompt, you can type the following things (among others):
> 
> -> i -- show running task info
> -> vxbIoctlShow -- show VxBus device tree
> -> vxbDevShow -- show VxBus device tree in a different format
> -> vxbDrvShow -- show VxBus drivers
> -> vxbIntShow -- show information about interrupt bindings
> -> vmContextShow -- show MMU mappings
> -> memShow -- show heap usage
> -> ifconfig -- show network interfaces
> -> routec "show" -- show network routing table
> -> netstat "-a" -- show network socket info
> 
> The image also includes network support. You can use ifconfig to set the
> enet0 interface's addresss like this:
> 
> -> ifconfig "enet0 10.0.2.15 netmask 255.255.255.0 up"
> 
> You should then be able to telnet to the VxWorks image from the host
> machine by doing:
> 
> % telnet localhost 10023
> 
> To exit the telnet session, type logout:
> 
> -> logout
> 
> NOTE: this only works if you've patched the imx_fec module to fix the
> interrupt vector bug that I also reported.
> 
> If you run vxbIoctlShow, which generates a lot of output, you should be
> able to see the stall condition I'm talking about. If you don't, then
> maybe it has something to do with me running QEMU on FreeBSD.
> 
> > Peter:
> > 
> > Bill is the author of original code, so I am not sure how to handle
> > Signed-off-by from him. I'd like to add it to this patch, but since
> > his original submission to launchpad didn't have one it is currently
> > omitted.
> 
> My original goal was to report the bug and provide as much info as I could
> on how to maybe fix it, and let somebody else make a proper fix/patch.
> 
> -Bill
> 
> >  include/hw/char/imx_serial.h |  3 +++
> >  hw/char/imx_serial.c | 20 +---
> >  2 files changed, 20 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/hw/char/imx_serial.h b/include/hw/char/imx_serial.h
> > index baeec3183f..5b99cee7cf 100644
> > --- a/include/hw/char/imx_serial.h
> > +++ b/include/hw/char/imx_serial.h
> > @@ -67,6 +67,8 @@
> > 
> >  #define UCR2_RXEN   (1<<1)/* Receiver enable */
> >  #define UCR2_SRST   (1<<0)/* Reset complete */
> > 
> > +#define UCR4_TCEN   BIT(3)/* TX complete interrupt enable */
> > +
> > 
> >  #define UTS1_TXEMPTY(1<<6)

[Qemu-devel] [PATCH v2 1/3] vfio/pci: Pull BAR mapping setup from read-write path

[Alex Williamson]

This creates a common helper that we'll use for ioeventfd setup.

Reviewed-by: Peter Xu 
Reviewed-by: Eric Auger 
Signed-off-by: Alex Williamson 
---
 drivers/vfio/pci/vfio_pci_rdwr.c |   39 ++
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
index 357243d76f10..5f2b376dcebd 100644
--- a/drivers/vfio/pci/vfio_pci_rdwr.c
+++ b/drivers/vfio/pci/vfio_pci_rdwr.c
@@ -113,6 +113,30 @@ static ssize_t do_io_rw(void __iomem *io, char __user *buf,
return done;
 }
 
+static int vfio_pci_setup_barmap(struct vfio_pci_device *vdev, int bar)
+{
+   struct pci_dev *pdev = vdev->pdev;
+   int ret;
+   void __iomem *io;
+
+   if (vdev->barmap[bar])
+   return 0;
+
+   ret = pci_request_selected_regions(pdev, 1 << bar, "vfio");
+   if (ret)
+   return ret;
+
+   io = pci_iomap(pdev, bar, 0);
+   if (!io) {
+   pci_release_selected_regions(pdev, 1 << bar);
+   return -ENOMEM;
+   }
+
+   vdev->barmap[bar] = io;
+
+   return 0;
+}
+
 ssize_t vfio_pci_bar_rw(struct vfio_pci_device *vdev, char __user *buf,
size_t count, loff_t *ppos, bool iswrite)
 {
@@ -147,22 +171,13 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_device *vdev, 
char __user *buf,
if (!io)
return -ENOMEM;
x_end = end;
-   } else if (!vdev->barmap[bar]) {
-   int ret;
-
-   ret = pci_request_selected_regions(pdev, 1 << bar, "vfio");
+   } else {
+   int ret = vfio_pci_setup_barmap(vdev, bar);
if (ret)
return ret;
 
-   io = pci_iomap(pdev, bar, 0);
-   if (!io) {
-   pci_release_selected_regions(pdev, 1 << bar);
-   return -ENOMEM;
-   }
-
-   vdev->barmap[bar] = io;
-   } else
io = vdev->barmap[bar];
+   }
 
if (bar == vdev->msix_bar) {
x_start = vdev->msix_offset;

[Qemu-devel] [PATCH v2 0/3] vfio/pci: ioeventfd support

[Alex Williamson]

A vfio ioeventfd will perform the pre-specified device write on
triggering of an eventfd.  When coupled with KVM ioeventfds, this
feature allows a VM to trap a device page for virtualization, while
also registering targeted ioeventfds to maintain performance of high
frequency register writes within the trapped range.  Much like the
existing interrupt eventfd/irqfd coupling, such writes can be handled
entirely in the host kernel.

The new VFIO device ioctl may be supported by any vfio bus driver,
including mdev drivers, but the implementation here only enables
vfio-pci.  This is intended as an acceleration path, bus drivers
may choose which regions to support and userspace should always
intend to fall back to non-accelerated handling when unavailable.

v1->v2:
 * Peter & Eric Sign-offs on 1/3
 * mutex_destroy() in 3/3
 * Slight enhancement to uapi description
 * sparse clean - Sparse didn't like that we dropped the __iomem
   address space when calling iowriteXX, re-adding it via the opaque
   and data pointers of the virq was crude, and that was not a 32-bit
   friendly soluion anyway, so add the iomem address to our ioeventfd
   struct, pass that, and use a more simple, common handler.

RFC->v1:
 * An arbitrary limit is added for the number of ioeventfds supported
   per device.  The intention is to set this high enough to allow any
   reasonable use case, but limit malicious user behavior.
 * Split patches, including adding a patch for endian neutral io reads
   and writes.  This should be a nop for little-endian and avoid
   redundant swap on big-endian, and hopefully resolves Alexey's
   comments regarding the endian nature of this interface.
 * Rebase to v4.16-rc3

Thanks,
Alex

---

Alex Williamson (3):
  vfio/pci: Pull BAR mapping setup from read-write path
  vfio/pci: Use endian neutral helpers
  vfio/pci: Add ioeventfd support


 drivers/vfio/pci/vfio_pci.c |   35 +++
 drivers/vfio/pci/vfio_pci_private.h |   19 
 drivers/vfio/pci/vfio_pci_rdwr.c|  184 +++
 include/uapi/linux/vfio.h   |   27 +
 4 files changed, 245 insertions(+), 20 deletions(-)

[Qemu-devel] [PATCH v2 3/3] vfio/pci: Add ioeventfd support

[Alex Williamson]

The ioeventfd here is actually irqfd handling of an ioeventfd such as
supported in KVM.  A user is able to pre-program a device write to
occur when the eventfd triggers.  This is yet another instance of
eventfd-irqfd triggering between KVM and vfio.  The impetus for this
is high frequency writes to pages which are virtualized in QEMU.
Enabling this near-direct write path for selected registers within
the virtualized page can improve performance and reduce overhead.
Specifically this is initially targeted at NVIDIA graphics cards where
the driver issues a write to an MMIO register within a virtualized
region in order to allow the MSI interrupt to re-trigger.

Signed-off-by: Alex Williamson 
---
 drivers/vfio/pci/vfio_pci.c |   35 +++
 drivers/vfio/pci/vfio_pci_private.h |   19 ++
 drivers/vfio/pci/vfio_pci_rdwr.c|  111 +++
 include/uapi/linux/vfio.h   |   27 +
 4 files changed, 192 insertions(+)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index b0f759476900..c6822149b394 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -305,6 +305,7 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev)
 {
struct pci_dev *pdev = vdev->pdev;
struct vfio_pci_dummy_resource *dummy_res, *tmp;
+   struct vfio_pci_ioeventfd *ioeventfd, *ioeventfd_tmp;
int i, bar;
 
/* Stop the device from further DMA */
@@ -314,6 +315,15 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev)
VFIO_IRQ_SET_ACTION_TRIGGER,
vdev->irq_type, 0, 0, NULL);
 
+   /* Device closed, don't need mutex here */
+   list_for_each_entry_safe(ioeventfd, ioeventfd_tmp,
+>ioeventfds_list, next) {
+   vfio_virqfd_disable(>virqfd);
+   list_del(>next);
+   kfree(ioeventfd);
+   }
+   vdev->ioeventfds_nr = 0;
+
vdev->virq_disabled = false;
 
for (i = 0; i < vdev->num_regions; i++)
@@ -1012,6 +1022,28 @@ static long vfio_pci_ioctl(void *device_data,
 
kfree(groups);
return ret;
+   } else if (cmd == VFIO_DEVICE_IOEVENTFD) {
+   struct vfio_device_ioeventfd ioeventfd;
+   int count;
+
+   minsz = offsetofend(struct vfio_device_ioeventfd, fd);
+
+   if (copy_from_user(, (void __user *)arg, minsz))
+   return -EFAULT;
+
+   if (ioeventfd.argsz < minsz)
+   return -EINVAL;
+
+   if (ioeventfd.flags & ~VFIO_DEVICE_IOEVENTFD_SIZE_MASK)
+   return -EINVAL;
+
+   count = ioeventfd.flags & VFIO_DEVICE_IOEVENTFD_SIZE_MASK;
+
+   if (hweight8(count) != 1 || ioeventfd.fd < -1)
+   return -EINVAL;
+
+   return vfio_pci_ioeventfd(vdev, ioeventfd.offset,
+ ioeventfd.data, count, ioeventfd.fd);
}
 
return -ENOTTY;
@@ -1174,6 +1206,8 @@ static int vfio_pci_probe(struct pci_dev *pdev, const 
struct pci_device_id *id)
vdev->irq_type = VFIO_PCI_NUM_IRQS;
mutex_init(>igate);
spin_lock_init(>irqlock);
+   mutex_init(>ioeventfds_lock);
+   INIT_LIST_HEAD(>ioeventfds_list);
 
ret = vfio_add_group_dev(>dev, _pci_ops, vdev);
if (ret) {
@@ -1215,6 +1249,7 @@ static void vfio_pci_remove(struct pci_dev *pdev)
 
vfio_iommu_group_put(pdev->dev.iommu_group, >dev);
kfree(vdev->region);
+   mutex_destroy(>ioeventfds_lock);
kfree(vdev);
 
if (vfio_pci_is_vga(pdev)) {
diff --git a/drivers/vfio/pci/vfio_pci_private.h 
b/drivers/vfio/pci/vfio_pci_private.h
index f561ac1c78a0..cde3b5d3441a 100644
--- a/drivers/vfio/pci/vfio_pci_private.h
+++ b/drivers/vfio/pci/vfio_pci_private.h
@@ -29,6 +29,19 @@
 #define PCI_CAP_ID_INVALID 0xFF/* default raw access */
 #define PCI_CAP_ID_INVALID_VIRT0xFE/* default virt access 
*/
 
+/* Cap maximum number of ioeventfds per device (arbitrary) */
+#define VFIO_PCI_IOEVENTFD_MAX 1000
+
+struct vfio_pci_ioeventfd {
+   struct list_headnext;
+   struct virqfd   *virqfd;
+   void __iomem*addr;
+   uint64_tdata;
+   loff_t  pos;
+   int bar;
+   int count;
+};
+
 struct vfio_pci_irq_ctx {
struct eventfd_ctx  *trigger;
struct virqfd   *unmask;
@@ -92,9 +105,12 @@ struct vfio_pci_device {
boolnointx;
struct pci_saved_state  *pci_saved_state;
int refcnt;
+   int ioeventfds_nr;
struct eventfd_ctx  *err_trigger;
struct eventfd_ctx  *req_trigger;

[Qemu-devel] [PATCH v2 2/3] vfio/pci: Use endian neutral helpers

[Alex Williamson]

The iowriteXX/ioreadXX functions assume little endian hardware and
convert to little endian on a write and from little endian on a read.
We currently do our own explicit conversion to negate this.  Instead,
add some endian dependent defines to avoid all byte swaps.  There
should be no functional change other than big endian systems aren't
penalized with wasted swaps.

Signed-off-by: Alex Williamson 
---
 drivers/vfio/pci/vfio_pci_rdwr.c |   34 ++
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
index 5f2b376dcebd..925419e0f459 100644
--- a/drivers/vfio/pci/vfio_pci_rdwr.c
+++ b/drivers/vfio/pci/vfio_pci_rdwr.c
@@ -21,6 +21,24 @@
 
 #include "vfio_pci_private.h"
 
+#ifdef __LITTLE_ENDIAN
+#define vfio_ioread64  ioread64
+#define vfio_iowrite64 iowrite64
+#define vfio_ioread32  ioread32
+#define vfio_iowrite32 iowrite32
+#define vfio_ioread16  ioread16
+#define vfio_iowrite16 iowrite16
+#else
+#define vfio_ioread64  ioread64be
+#define vfio_iowrite64 iowrite64be
+#define vfio_ioread32  ioread32be
+#define vfio_iowrite32 iowrite32be
+#define vfio_ioread16  ioread16be
+#define vfio_iowrite16 iowrite16be
+#endif
+#define vfio_ioread8   ioread8
+#define vfio_iowrite8  iowrite8
+
 /*
  * Read or write from an __iomem region (MMIO or I/O port) with an excluded
  * range which is inaccessible.  The excluded range drops writes and fills
@@ -44,15 +62,15 @@ static ssize_t do_io_rw(void __iomem *io, char __user *buf,
fillable = 0;
 
if (fillable >= 4 && !(off % 4)) {
-   __le32 val;
+   u32 val;
 
if (iswrite) {
if (copy_from_user(, buf, 4))
return -EFAULT;
 
-   iowrite32(le32_to_cpu(val), io + off);
+   vfio_iowrite32(val, io + off);
} else {
-   val = cpu_to_le32(ioread32(io + off));
+   val = vfio_ioread32(io + off);
 
if (copy_to_user(buf, , 4))
return -EFAULT;
@@ -60,15 +78,15 @@ static ssize_t do_io_rw(void __iomem *io, char __user *buf,
 
filled = 4;
} else if (fillable >= 2 && !(off % 2)) {
-   __le16 val;
+   u16 val;
 
if (iswrite) {
if (copy_from_user(, buf, 2))
return -EFAULT;
 
-   iowrite16(le16_to_cpu(val), io + off);
+   vfio_iowrite16(val, io + off);
} else {
-   val = cpu_to_le16(ioread16(io + off));
+   val = vfio_ioread16(io + off);
 
if (copy_to_user(buf, , 2))
return -EFAULT;
@@ -82,9 +100,9 @@ static ssize_t do_io_rw(void __iomem *io, char __user *buf,
if (copy_from_user(, buf, 1))
return -EFAULT;
 
-   iowrite8(val, io + off);
+   vfio_iowrite8(val, io + off);
} else {
-   val = ioread8(io + off);
+   val = vfio_ioread8(io + off);
 
if (copy_to_user(buf, , 1))
return -EFAULT;

Re: [Qemu-devel] [RFC PATCH] vfio/pci: Add ioeventfd support

[Alex Williamson]

On Tue, 13 Mar 2018 13:38:00 +0100
Auger Eric  wrote:
> On 08/02/18 02:22, Alexey Kardashevskiy wrote:
> > On 08/02/18 01:12, Alex Williamson wrote:  
> >> On Wed, 7 Feb 2018 15:48:26 +1100
> >> Alexey Kardashevskiy  wrote:
> >>> On 07/02/18 15:25, Alex Williamson wrote:  
>  On Wed, 7 Feb 2018 15:09:22 +1100
>  Alexey Kardashevskiy  wrote:
> > On 07/02/18 11:08, Alex Williamson wrote:
> >> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> >> index e3301dbd27d4..07966a5f0832 100644
> >> --- a/include/uapi/linux/vfio.h
> >> +++ b/include/uapi/linux/vfio.h
> >> @@ -503,6 +503,30 @@ struct vfio_pci_hot_reset {
> >>  
> >>  #define VFIO_DEVICE_PCI_HOT_RESET _IO(VFIO_TYPE, VFIO_BASE + 13)
> >>  
> >> +/**
> >> + * VFIO_DEVICE_IOEVENTFD - _IOW(VFIO_TYPE, VFIO_BASE + 14,
> >> + *  struct vfio_device_ioeventfd)
> >> + *
> >> + * Perform a write to the device at the specified device fd offset, 
> >> with
> >> + * the specified data and width when the provided eventfd is 
> >> triggered.
> >> + *
> >> + * Return: 0 on success, -errno on failure.
> >> + */
> >> +struct vfio_device_ioeventfd {
> >> +  __u32   argsz;
> >> +  __u32   flags;
> >> +#define VFIO_DEVICE_IOEVENTFD_8   (1 << 0) /* 1-byte 
> >> write */
> >> +#define VFIO_DEVICE_IOEVENTFD_16  (1 << 1) /* 2-byte write */
> >> +#define VFIO_DEVICE_IOEVENTFD_32  (1 << 2) /* 4-byte write */
> >> +#define VFIO_DEVICE_IOEVENTFD_64  (1 << 3) /* 8-byte write */
> >> +#define VFIO_DEVICE_IOEVENTFD_SIZE_MASK   (0xf)
> >> +  __u64   offset; /* device fd offset of write */
> >> +  __u64   data;   /* data to be written */
> >> +  __s32   fd; /* -1 for de-assignment */
> >> +};
> >> +
> >> +#define VFIO_DEVICE_IOEVENTFD _IO(VFIO_TYPE, VFIO_BASE + 14)  
> >> 
> >
> >
> > Is this a first ioctl with endianness fixed to little-endian? I'd 
> > suggest
> > to comment on that as things like vfio_info_cap_header do use the host
> > endianness.
> 
>  Look at our current read and write interface, we call leXX_to_cpu
>  before calling iowriteXX there and I think a user would logically
>  expect to use the same data format here as they would there.
> >>>
> >>> If the data is "char data[8]" (i.e. bytestream), then it can be expected 
> >>> to
> >>> be device/bus endian (i.e. PCI == little endian), but if it is u64 - then 
> >>> I
> >>> am not so sure really, and this made me look around. It could be "__le64
> >>> data" too.
> >>>  
>  Also note
>  that iowriteXX does a cpu_to_leXX, so are we really defining the
>  interface as little-endian or are we just trying to make ourselves
>  endian neutral and counter that implicit conversion?  Thanks,
> >>>
> >>> Defining it LE is fine, I just find it a bit confusing when
> >>> vfio_info_cap_header is host endian but vfio_device_ioeventfd is not.  
> >>
> >> But I don't think we are defining the interface as little-endian.
> >> iowriteXX does a cpu_to_leXX byteswap.  Therefore in order to maintain
> >> endian neutrality, if the data does a cpu->le swap on the way out, I
> >> need to do a le->cpu swap on the way in, right?  Please defend the
> >> assertion that we're creating a little-endian interface.  Thanks,  
> > 
> > 
> > vfio_pci_ioctl() passes "endian-neutral" ioeventfd.data to
> > vfio_pci_ioeventfd() which immediately does the leXX_to_cpu() conversion
> > (and uses the result later on in iowriteXX(), which is not VFIO API) so I
> > read it as the ioctl really expects LE.
> > 
> > The QEMU part - vfio_nvidia_mirror_quirk MR - does not swap bytes but the
> > MR itself it declared DEVICE_LITTLE_ENDIAN which means
> > vfio_nvidia_quirk_mirror_write() receives byteswapped @data in the host
> > endian == bigendian on a big endian host. So the ioctl() handler will
> > receive a BE value, do byteswap #1 in leXX_to_cpu(), and then do byteswap
> > #2 in iowriteXX() so after all a BE will be written to a device. So I'd say
> > we rather do not need leXX_to_cpu() in vfio_pci_ioeventfd(). Correct me
> > where I am wrong. Thanks,  
> 
> It is not crystal clear to me what is the outcome of this discussion.
> Please can you clarify?
> 
> At the beginning I understood we had a chain of lexx_to_cpu and
> cpu_to_lexx (in iowritexx) so it was neutral. Now I am lost about what
> we want.

I've tried to address this with patch 2/3 in the newer series, adding
helpers such that the implicit endian-ness of the io{read,write}
functions is hidden and no extraneous swapping is done.  Therefore the
leXX_to_cpu() is gone, as Alexey wanted.  Unless there's a new
objection, this is what I intend to go with.  Thanks,

Alex

Re: [Qemu-devel] [PATCH 3/3] vfio/pci: Add ioeventfd support

[Alex Williamson]

On Wed, 7 Mar 2018 13:56:44 +0800
Peter Xu  wrote:

> On Wed, Feb 28, 2018 at 01:15:20PM -0700, Alex Williamson wrote:
> 
> [...]
> 
> > @@ -1174,6 +1206,8 @@ static int vfio_pci_probe(struct pci_dev *pdev, const 
> > struct pci_device_id *id)
> > vdev->irq_type = VFIO_PCI_NUM_IRQS;
> > mutex_init(>igate);
> > spin_lock_init(>irqlock);
> > +   mutex_init(>ioeventfds_lock);  
> 
> Do we better need to destroy the mutex in vfio_pci_remove?
> 
> I see that vfio_pci_device.igate is also without a destructor.  I'm
> not sure on both.

Yeah, mutex_destroy() is purely for debugging and I must have missed it
when implementing vfio.  I'll add it in the remove function and try to
cleanup the others in a separate patch, at some point.  Thanks,

Alex

Re: [Qemu-devel] [PATCH qemu] fdc: Exit if ISA controller does not support DMA

[John Snow]

On 03/05/2018 05:15 AM, Thomas Huth wrote:
> On 08.12.2017 22:29, John Snow wrote:
>>
>> On 11/21/2017 09:48 PM, Alexey Kardashevskiy wrote:
>>> On 07/11/17 11:58, John Snow wrote:


 On 10/26/2017 02:46 AM, Alexey Kardashevskiy wrote:
> A "powernv" machine type defines an ISA bus but it does not add any DMA
> controller to it so it is possible to hit assert(fdctrl->dma) by
> adding "-machine powernv -device isa-fdc".
>
> This replaces assert() with an error message.
>
> Signed-off-by: Alexey Kardashevskiy 
> ---
> [...]
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 67f78ac702..ed8b367572 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -2700,7 +2700,10 @@ static void isabus_fdc_realize(DeviceState *dev, 
> Error **errp)
>  fdctrl->dma_chann = isa->dma;
>  if (fdctrl->dma_chann != -1) {
>  fdctrl->dma = isa_get_dma(isa_bus_from_device(isadev), isa->dma);
> -assert(fdctrl->dma);
> +if (!fdctrl->dma) {
> +error_setg(errp, "ISA controller does not support DMA, 
> exiting");
> +return;
> +}
>  }
>  
>  qdev_set_legacy_instance_id(dev, isa->iobase, 2);
>

 I've been MIA for a little while, so I'm out of the loop -- but I am not
 sure this is entirely the right way to fix this problem. I think it is
 more the case that certain boards should not be able to ask for certain
 types of devices, and we should prohibit e.g. powernv from being able to
 ask for an ISA floppy disk controller.

 (It doesn't seem to have an ISA DMA controller by default, but I have no
 idea if that means it can't EVER have one...)

 Papering over this by making it a soft error when we fail to execute
 isa_get_dma and then assuming in retrospect it's because the machine
 type we're on cannot have an ISA DMA controller seems a little
 wrong-headed. It also leaves side-effects from isa_register_portio_list
 and isa_init_irq, so we can't just bail here -- it's only marginally
 better than the assert() it's doing.

 That said, I am not really sure what the right thing to do is ... I
 suspect the "right thing" is to express the dependency that isa-fdc
 requires an ISA DMA controller -- and maybe that check happens here when
 isa_get_dma fails and we have to unwind the realize function, but we
 need to do it gracefully.

 Give me a day to think about it, but I do want to make sure this is in
 the next release.
>>>
>>> The day has passed, any news? :)
>>
>> *cough* It turns out that understanding the intricacies of FDC and ISA
>> is nobody's favorite thing to do.
>>
>> OK, so ehabkost pointed me to this:
>>
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg496460.html
>>
>> Where we declare that DMA devices generally can't be created by the user
>> for the inverse of the reason we're seeing here: these devices need to
>> be created precisely once: not zero times, not twice, exactly once.
>>
>> So we made the ISA DMA devices themselves not user-creatable, so you are
>> indeed correct here that the absence of fdctrl->dma does more or less
>> mean that the current configuration "doesn't support DMA." ... but maybe
>> this won't always be true, and maybe some devices (TYPE_I82374?) are
>> user creatable, so let's make a "softer" error message:
>>
>> "No ISA DMA device present, can't create ISA FDC device."
>>
>> Then, on the other end, we need to unwind realize() gracefully, maybe we
>> can just shuffle up isa_get_dma() earlier so we don't have to unwind
>> anything if it comes back empty.
>>
>> Then I'll take the patch, because fixing this more properly I think will
>> take more time or effort than I have to spend on the FDC device.
> 
> The problem still persists ... was there ever a follow-up to this patch
> / discussion?
> 
>  Thomas
> 

No, I'll just stab at it during freeze. I can probably at least have it
fail gracefully, though what the "right" thing to do is still not
particularly clear to me as I don't really understand the platforms that
are failing.

Re: [Qemu-devel] [PATCH 3/3] vfio/pci: Add ioeventfd support

[Alex Williamson]

On Tue, 13 Mar 2018 14:12:34 +0100
Auger Eric  wrote:
> On 28/02/18 21:15, Alex Williamson wrote:
> > +long vfio_pci_ioeventfd(struct vfio_pci_device *vdev, loff_t offset,
> > +   uint64_t data, int count, int fd)
> > +{
> > +   struct pci_dev *pdev = vdev->pdev;
> > +   loff_t pos = offset & VFIO_PCI_OFFSET_MASK;
> > +   int ret, bar = VFIO_PCI_OFFSET_TO_INDEX(offset);
> > +   struct vfio_pci_ioeventfd *ioeventfd;
> > +   int (*handler)(void *addr, void *value);
> > +
> > +   /* Only support ioeventfds into BARs */
> > +   if (bar > VFIO_PCI_BAR5_REGION_INDEX)
> > +   return -EINVAL;
> > +
> > +   if (pos + count > pci_resource_len(pdev, bar))
> > +   return -EINVAL;
> > +
> > +   /* Disallow ioeventfds working around MSI-X table writes */
> > +   if (bar == vdev->msix_bar &&
> > +   !(pos + count <= vdev->msix_offset ||
> > + pos >= vdev->msix_offset + vdev->msix_size))
> > +   return -EINVAL;
> > +
> > +   switch (count) {
> > +   case 1:
> > +   handler = _pci_ioeventfd_handler8;
> > +   break;
> > +   case 2:
> > +   handler = _pci_ioeventfd_handler16;
> > +   break;
> > +   case 4:
> > +   handler = _pci_ioeventfd_handler32;
> > +   break;
> > +#ifdef iowrite64
> > +   case 8:
> > +   handler = _pci_ioeventfd_handler64;
> > +   break;  
> from a user point of view, it is straightforward this setup will be
> rejected? This is not documented in the uapi at the moment.

I added a mention in the uapi, do you see any need for more?
Essentially I consider this an entirely optional accelerator, bus
drivers are free to implement as much or little as they want.
Userspace can clearly make due without it, we've gone this long, and
it's easy to reject cases we don't want to support.  Thanks,

Alex

[Qemu-devel] [Bug 1753314] Re: UART in sabrelite machine simulation doesn't work with VxWorks 7

[Peter Maydell]

That's fine; Andrey Smirnov has taken your patch as a basis for a more 
cleaned-up set of changes:
http://lists.nongnu.org/archive/html/qemu-devel/2018-03/msg04608.html
http://lists.nongnu.org/archive/html/qemu-devel/2018-03/msg04609.html
What we would like from you is a Signed-off-by: line to say that you're happy 
for us to do that, please. (If you have a chance to test that it works for you 
that would also be great.)

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

Title:
  UART in sabrelite machine simulation doesn't work with VxWorks 7

Status in QEMU:
  New

Bug description:
  The imx_serial.c driver currently implements only partial support for
  the i.MX6 UART hardware. (I understand it's a work in progress and
  that's fine.) dIn particular, it does not implement support for the
  Transmit Complete Interrupt Enable bit in the UCR4 register. The
  VxWorks 7 i.MX6 serial driver depends on the behavior of this bit in
  actual hardware in order to send characters through the UART
  correctly. The result is that with the current machine model, VxWorks
  will boot and run in QEMU but it's unable to print any characters to
  the console serial port.

  I have produced a small patch for the imx_serial.c module to make it
  nominally functional with VxWorks 7. It works well enough to allow the
  boot banner to appear and for the user to interact with the target
  shell.

  I'm not submitting this as a patch to the development list as I'm not
  fully certain it complies with the hardware spec and doesn't break any
  other functionality. I would prefer if the maintainer (or someone)
  reviewed it for any issues/refinements first.

  I'm attaching the patch to this bug report. A copy can also be
  obtained from:

  http://people.freebsd.org/~wpaul/qemu/imx_serial.zip

  This patch was generated against QEMU 2.11.0 but also works with QEMU
  2.11.1.

  My host environment is FreeBSD/amd64 11.1-RELEASE with QEMU
  2.11.0/2.11.11 built from source.

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

Re: [Qemu-devel] ARM64 Interrupt handling on QEMU

[Peter Maydell]

On 15 March 2018 at 20:24, Brijen Raval  wrote:
> On Thu, Mar 15, 2018 at 2:59 AM Peter Maydell 
> wrote:
>> Exception 5 is IRQ. (These numbers are all internal to QEMU, and
>> don't have any architectural or guest-visible relevance. They're
>> the EXCP_* constants defined at the top of target/arm/cpu.h.)
>
>
> Yup I checked out the QEMU source and confirmed above. So is there any way
> to find out what is the IRQ for?

You would need to look at what the state of the interrupt controller
is. You can turn on all the tracepoints in the GIC with -d trace:gic*
(though to understand what it's saying you may need to have
some familiarity with the GIC spec and/or look at the code).

thanks
-- PMM

Re: [Qemu-devel] ARM64 Interrupt handling on QEMU

[Brijen Raval]

On Thu, Mar 15, 2018 at 2:59 AM Peter Maydell 
wrote:

> On 15 March 2018 at 03:07, Brijen Raval  wrote:
> > I am booting up a custom kernel on QEMU ARM64, upon completion of its
> > initial boot up it looks like it enters the arch_idle() state
> >
> > I enabled the -d int logging to understand what is going on, I see the
> > following repeated many times continuosly here after
> >
> > Taking exception 5 [IRQ]
> > ...from EL1 to EL1
> > ...with ESR 0x15/0x5600
> > ...with ELR 0x349c
> > ...to EL1 PC 0x8280 PSTATE 0x3c5
> >
> > Here's the dissassembly for the relevant piece of code:
> >
> >  3498 :
> >  arch_idle():
> >  ../../kernel/arch/arm64/arch.cpp:182
> >  3498:   d503207fwfi
> >  349c:   d65f03c0ret
> >
> > I am trying to understand what exceptions are occurring exactly when
> kernel
> > is idle (timer?). According to above ELR is pointing to arch_idle(), but
> I
> > believe "wfi" instruction would not be an IRQ but a sync abort which is
> > handle differently right?
>
> WFI is neither an IRQ nor an abort. It's just a hint to the CPU
> that it doesn't need to execute any more instructions until the
> next interrupt occurs. (It's a valid implementation for it to just
> be a NOP.) QEMU does implement WFI to be "don't execute more insns
> until the next interrupt", which is why you're seeing the ELR for
> the interrupt generally being the ret insn: if the guest is mostly
> idle and its idle loop includes a WFI then almost all the time an
> incoming interrupt will find that the CPU was in the WFI insn.
>
> Correctly written software will not ever issue a WFI unless it
> has some mechanism for being woken up from it, which is typically
> an outstanding interrupt, perhaps timer or an interrupt for
> completed disk or network operation. For an SMP kernel it could also be
> some other CPU sending this CPU an inter-processor-interrupt to
> wake us up). "idle" for an OS just means "nothing to do right now".
>

OK, that makes sense


>
> > Also from ESR, it looks like a SVC instruction but if I am not wrong for
> > IRQs ESRs are not updated (considered)
>
> QEMU's logging prints the ESR value for all exceptions, even those
> where architecturally the ESR is not updated. In those cases the
> printed value can be ignored.
>

Thought so, thanks for the clarification

>
> > One more thing, is there a way in QEMU I could find out what exception 5
> is
> > corresponding to?
>
> The logging tells you:
> > Taking exception 5 [IRQ]
>
> Exception 5 is IRQ. (These numbers are all internal to QEMU, and
> don't have any architectural or guest-visible relevance. They're
> the EXCP_* constants defined at the top of target/arm/cpu.h.)
>

Yup I checked out the QEMU source and confirmed above. So is there any way
to find out what is the IRQ for?


>
> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH 1/3] tests/boot-sector: Add magic bytes to s390x boot code header

[Michael S. Tsirkin]

On Thu, Mar 15, 2018 at 12:47:08PM +0100, Philippe Mathieu-Daudé wrote:
> On 03/15/2018 08:49 AM, Thomas Huth wrote:
> > We're going to use the s390x boot code for testing CD-ROM booting.
> > But the ISO loader of the s390-ccw bios is a little bit more picky
> > than the network loader and expects some magic bytes in the header
> > of the file (see linux_s390_magic in pc-bios/s390-ccw/bootmap.c), so
> > we've got to add them in our boot code here, too.
> > 
> > Signed-off-by: Thomas Huth 
> > ---
> >  tests/boot-sector.c | 9 ++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tests/boot-sector.c b/tests/boot-sector.c
> > index c373f0e..04721c9 100644
> > --- a/tests/boot-sector.c
> > +++ b/tests/boot-sector.c
> > @@ -68,8 +68,11 @@ static uint8_t x86_boot_sector[512] = {
> >  };
> >  
> >  /* For s390x, use a mini "kernel" with the appropriate signature */
> > -static const uint8_t s390x_psw[] = {
> > -0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00
> > +static const uint8_t s390x_psw_and_magic[] = {
> 
> Can you add a comment such "see linux_s390_magic in
> pc-bios/s390-ccw/bootmap.c"?

Or better yet, copy the code comment from there.

> > +0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00,
> > +0x02, 0x00, 0x00, 0x18, 0x60, 0x00, 0x00, 0x50,
> > +0x02, 0x00, 0x00, 0x68, 0x60, 0x00, 0x00, 0x50,
> > +0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40
> >  };
> >  static const uint8_t s390x_code[] = {
> >  0xa7, 0xf4, 0x00, 0x0a,/* j 0x10010 */
> > @@ -110,7 +113,7 @@ int boot_sector_init(char *fname)
> >  } else if (g_str_equal(arch, "s390x")) {
> >  len = 0x1 + sizeof(s390x_code);
> >  boot_code = g_malloc0(len);
> > -memcpy(boot_code, s390x_psw, sizeof(s390x_psw));
> > +memcpy(boot_code, s390x_psw_and_magic, 
> > sizeof(s390x_psw_and_magic));
> >  memcpy(_code[0x1], s390x_code, sizeof(s390x_code));
> >  } else {
> >  g_assert_not_reached();
> > 

[Qemu-devel] [PATCH] net/vde: print error on vde_open() failure

[Julia Suvorova via Qemu-devel]

Despite the fact that now when the initialization of vde fails, qemu
does not end silently, no informative error is printed. The patch
generates an error and pushes it through the calling function.

Related bug: https://bugs.launchpad.net/qemu/+bug/676029

Signed-off-by: Julia Suvorova 
---
 net/vde.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/vde.c b/net/vde.c
index e50e5d6..99189cc 100644
--- a/net/vde.c
+++ b/net/vde.c
@@ -30,6 +30,7 @@
 #include "qemu-common.h"
 #include "qemu/option.h"
 #include "qemu/main-loop.h"
+#include "qapi/error.h"
 
 typedef struct VDEState {
 NetClientState nc;
@@ -76,7 +77,7 @@ static NetClientInfo net_vde_info = {
 
 static int net_vde_init(NetClientState *peer, const char *model,
 const char *name, const char *sock,
-int port, const char *group, int mode)
+int port, const char *group, int mode, Error **errp)
 {
 NetClientState *nc;
 VDEState *s;
@@ -92,6 +93,7 @@ static int net_vde_init(NetClientState *peer, const char 
*model,
 
 vde = vde_open(init_sock, (char *)"QEMU", );
 if (!vde){
+error_setg_errno(errp, errno, "Could not open vde");
 return -1;
 }
 
@@ -112,7 +114,6 @@ static int net_vde_init(NetClientState *peer, const char 
*model,
 int net_init_vde(const Netdev *netdev, const char *name,
  NetClientState *peer, Error **errp)
 {
-/* FIXME error_setg(errp, ...) on failure */
 const NetdevVdeOptions *vde;
 
 assert(netdev->type == NET_CLIENT_DRIVER_VDE);
@@ -120,7 +121,7 @@ int net_init_vde(const Netdev *netdev, const char *name,
 
 /* missing optional values have been initialized to "all bits zero" */
 if (net_vde_init(peer, "vde", name, vde->sock, vde->port, vde->group,
- vde->has_mode ? vde->mode : 0700) == -1) {
+ vde->has_mode ? vde->mode : 0700, errp) == -1) {
 return -1;
 }
 
-- 
2.1.4

Re: [Qemu-devel] Using new TCG Vector infrastructure in PowerPC

[Richard Henderson]

On 03/07/2018 06:03 PM, Nikunj A Dadhania wrote:
> Hi Richard,
> 
> I was working to get TCG vector support for PowerPC[1]. Started with
> converting logical operations like vector AND/OR/XOR and compare
> instructions. Found some inconsistency during my testing on x86 laptop
> emulating PowerPC:

Great.

Well, the problem is that you cannot use TCG generic vectors and TCG global
variables to access the same memory.

Thus your first step must be to remove all references to cpu_avrh and cpu_avrl.
 These can be replaced by translator helpers that perform an explicit
tcg_gen_ld_i64 or tcg_gen_st_i64 to the proper memory locations.

Only after that's done can you begin converting other references to use the
host vectors.  Otherwise, the tcg optimizer will do Bad and Unpredictable
Things, which may well have produced the incorrect results that you saw.

I'll note that it's probably worth rearranging all of {fpr,vsr,avr} to the more
logical configuration presented by Power7 (?) such that it's one array of 64 x
128-bit registers.


r~

[Qemu-devel] [Bug 1753314] Re: UART in sabrelite machine simulation doesn't work with VxWorks 7

[Bill Paul]

As I said before:

"I'm not submitting this as a patch to the development list as I'm not
fully certain it complies with the hardware spec and doesn't break any
other functionality."

What I'm trying to say here is that while I may have been able to cobble
together a hack to make the UART nominally compatible with VxWorks, I do
not understand the hardware or QEMU well enough to really fix this the
right way. Even with my hack, every once in a while when printing lots
of data on the console, the output from the UART will stall unless I
press a key on the serial port, and I don't know why it does that. I did
try to investigate it a little but wasn't able to make much progress.
(My suspicion is that it has something to do with the fact that the
imx_serial module doesn't implement FIFO support, but even if that's
true I don't know how to fix it.)

Even so, I figured it was still worth it to attach my changes to the bug
report so that somebody who is better at this than me could use it as a
starting point, and so that anybody else who might want to experiment
with VxWorks using the QEMU sabrelite machine model wouldn't be totally
stuck like I was.

In short, the changes I made are good enough for my own needs (the
output stalls don't happen often enough to really bother me), but
they're not a fully debugged fix. That's why I filed a bug report
instead of just submitting a patch in the first place: I wanted somebody
sexier than me to create a fully debugged fix.

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

Title:
  UART in sabrelite machine simulation doesn't work with VxWorks 7

Status in QEMU:
  New

Bug description:
  The imx_serial.c driver currently implements only partial support for
  the i.MX6 UART hardware. (I understand it's a work in progress and
  that's fine.) dIn particular, it does not implement support for the
  Transmit Complete Interrupt Enable bit in the UCR4 register. The
  VxWorks 7 i.MX6 serial driver depends on the behavior of this bit in
  actual hardware in order to send characters through the UART
  correctly. The result is that with the current machine model, VxWorks
  will boot and run in QEMU but it's unable to print any characters to
  the console serial port.

  I have produced a small patch for the imx_serial.c module to make it
  nominally functional with VxWorks 7. It works well enough to allow the
  boot banner to appear and for the user to interact with the target
  shell.

  I'm not submitting this as a patch to the development list as I'm not
  fully certain it complies with the hardware spec and doesn't break any
  other functionality. I would prefer if the maintainer (or someone)
  reviewed it for any issues/refinements first.

  I'm attaching the patch to this bug report. A copy can also be
  obtained from:

  http://people.freebsd.org/~wpaul/qemu/imx_serial.zip

  This patch was generated against QEMU 2.11.0 but also works with QEMU
  2.11.1.

  My host environment is FreeBSD/amd64 11.1-RELEASE with QEMU
  2.11.0/2.11.11 built from source.

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

Re: [Qemu-devel] [PATCH v3 12/16] block/dirty-bitmap: Add bdrv_dirty_iter_next_area

[John Snow]

On 02/28/2018 01:05 PM, Max Reitz wrote:
> This new function allows to look for a consecutively dirty area in a
> dirty bitmap.
> 
> Signed-off-by: Max Reitz 
> ---
>  include/block/dirty-bitmap.h |  2 ++
>  block/dirty-bitmap.c | 55 
> 
>  2 files changed, 57 insertions(+)
> 
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index e3f4bbf51d..8c8f63e722 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -79,6 +79,8 @@ void bdrv_set_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,
>  void bdrv_reset_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,
>  int64_t offset, int64_t bytes);
>  int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter);
> +bool bdrv_dirty_iter_next_area(BdrvDirtyBitmapIter *iter, uint64_t 
> max_offset,
> +   uint64_t *offset, int *bytes);
>  void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t offset);
>  int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
>  int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index d8e999226e..5d6b8dba89 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -496,6 +496,61 @@ int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter)
>  return hbitmap_iter_next(>hbi, true);
>  }
>  
> +/**
> + * Return the next consecutively dirty area in the dirty bitmap
> + * belonging to the given iterator @iter.
> + *
> + * @max_offset: Maximum value that may be returned for
> + *  *offset + *bytes
> + * @offset: Will contain the start offset of the next dirty area
> + * @bytes:  Will contain the length of the next dirty area
> + *
> + * Returns: True if a dirty area could be found before max_offset
> + *  (which means that *offset and *bytes then contain valid
> + *  values), false otherwise.
> + *
> + * Note that @iter is never advanced if false is returned.  If an area
> + * is found (which means that true is returned), it will be advanced
> + * past that area.
> + */
> +bool bdrv_dirty_iter_next_area(BdrvDirtyBitmapIter *iter, uint64_t 
> max_offset,
> +   uint64_t *offset, int *bytes)
> +{
> +uint32_t granularity = bdrv_dirty_bitmap_granularity(iter->bitmap);
> +uint64_t gran_max_offset;
> +int64_t ret;
> +int size;
> +
> +if (max_offset == iter->bitmap->size) {
> +/* If max_offset points to the image end, round it up by the
> + * bitmap granularity */
> +gran_max_offset = ROUND_UP(max_offset, granularity);
> +} else {
> +gran_max_offset = max_offset;
> +}
> +
> +ret = hbitmap_iter_next(>hbi, false);
> +if (ret < 0 || ret + granularity > gran_max_offset) {
> +return false;
> +}
> +
> +*offset = ret;
> +size = 0;
> +
> +assert(granularity <= INT_MAX);
> +
> +do {
> +/* Advance iterator */
> +ret = hbitmap_iter_next(>hbi, true);
> +size += granularity;
> +} while (ret + granularity <= gran_max_offset &&
> + hbitmap_iter_next(>hbi, false) == ret + granularity &&
> + size <= INT_MAX - granularity);
> +
> +*bytes = MIN(size, max_offset - *offset);
> +return true;
> +}
> +
>  /* Called within bdrv_dirty_bitmap_lock..unlock */
>  void bdrv_set_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,
>int64_t offset, int64_t bytes)
> 

10, 11, 12:

Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH 1/5] multiboot: Reject kernels exceeding the address space

[Jack Schwartz]

On 03/15/18 10:18, Kevin Wolf wrote:

Am 15.03.2018 um 17:55 hat Jack Schwartz geschrieben:

On 03/15/18 08:54, Kevin Wolf wrote:

Am 15.03.2018 um 06:19 hat Jack Schwartz geschrieben:

Hi Kevin.

My comments are inline...

On 2018-03-14 10:32, Kevin Wolf wrote:

The code path with a manually set mh_load_addr in the Multiboot header
checks that load_end_addr <= load_addr, but the path where load_end_addr
is automatically detected if 0 is given in the header misses the
corresponding check.

1) The code checks that load_end_addr >= load_addr (before letting it
through).

2) load_end_addr is used only when it is read in as non-zero, so no check is
needed if zero.  (It gets debug-printed even when zero, but is used only to
calculate mb_load_size and only when non-zero.)

Oops, good point. I'll change the start of the commit message as follows:

  The code path with a manually set mh_load_end_addr in the Multiboot
  header checks that mh_load_end_addr >= mh_load_addr, but the path where
  mh_load_end_addr is 0 in the header and therefore automatically
  calculated from the file size misses the corresponding check.

Does this look better?

mb_load_size is calculated from the file size, not mh_load_end_addr, so
I think you mean mb_load_size rather than mh_load_end_addr.  Do you intend
to say:

   The code path where mh_load_end_addr is non-zero in the Multiboot
   header checks that mh_load_end_addr >= mh_load_addr and so
   mb_load_size is checked.  However, mb_load_size is not checked when
   calculated from the file size, when mh_load_end_addr is 0.

Ok, technically that's more accurate.
OK.  Note that I fixed a couple of missing spaces if you decide to use 
it...

Also, if this is what you intend to say, would the following code change be
more ofwhat you want:

Remove this:

     mb_load_size = kernel_file_size - mb_kernel_text_offset;
     }
-   if (mb_load_size > UINT32_MAX - mh_load_addr) {
-   error_report("kernel does not fit in address space");
-   exit(1);
-   }
     if (mh_bss_end_addr) {

and instead do this a few lines further down:

    mb_kernel_size = mh_bss_end_addr - mh_load_addr;
     } else {
     mb_kernel_size = mb_load_size;
     }

+   if (mb_kernel_size > UINT32_MAX - mh_load_addr) {
+   error_report("kernel does not fit in address space");
+   exit(1);
+   }

     mb_debug("multiboot: header_addr = %#x", mh_header_addr);
     mb_debug("multiboot: load_addr = %#x", mh_load_addr);

The reason would be to include the bss area in the calculation, when
mh_bss_end_addr is non-zero.

Ultimately, mb_load_size > mb_kernel_size is what kills us,

Right.

  so maybe we
could add an assertion for that.

But the reason why this condition could ever be true is the integer
overflow in this line:

 if (mh_bss_end_addr < (mh_load_addr + mb_load_size))

It's generally better to check for integer overflows before they happen
than trying to infer them from the result. In fact, your condition
wouldn't catch the error of test scenario 9:

 kernel_file_size= 0x2035
 mh_header_addr  = 0xf000
 mh_load_addr= 0xf000
 mh_load_end_addr= 0
 mh_bss_end_addr = 0xf001

 mb_kernel_text_offset   = i - (mh_header_addr - mh_load_addr)
 = 0

 mb_load_size= kernel_file_size - mb_kernel_text_offset
 = 0x2035
 > UINT32_MAX - mh_load_addr

 mb_kernel_size  = mh_bss_end_addr - mh_load_addr
 = 1
 < UINT32_MAX - mh_load_addr

OK, fair enough...

One other suggestion, would be to move what you added up by one line, 
into the "else" clause.  This moves the check to run only where it is 
needed.  This is cleaner as it does the check only where mb_load_size is 
based on the kernel_file_size.


Reason: it doesn't make sense to do it for the "if" clause as it will 
never be true anyway:

  if (mh_load_end_addr) {
   ...
   mb_load_size = mh_load_end_addr - mh_load_addr;
       if (mb_load_size > UINT32_MAX - mh_load_addr) {
                bomb();

    Thanks,
    Jack


Kevin

Re: [Qemu-devel] [PATCH 3/3] tests/cdrom-test: Test that -cdrom parameter is working

[Thomas Huth]

On 15.03.2018 12:42, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
> 
> On 03/15/2018 08:49 AM, Thomas Huth wrote:
>> Commit 1454509726719e0933c800 recently broke the "-cdrom" parameter
>> on a couple of boards without that we noticed it immediately. Thus
>> add a test which checks that "-cdrom" can at least be used to start
>> QEMU with certain machine types.
>>
>> Signed-off-by: Thomas Huth 
>> ---
>>  tests/Makefile.include |  7 +-
>>  tests/cdrom-test.c | 63 
>> +-
>>  2 files changed, 68 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index a104222..b744fea 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -321,8 +321,9 @@ check-qtest-microblaze-y = 
>> tests/boot-serial-test$(EXESUF)
>>  check-qtest-mips-y = tests/endianness-test$(EXESUF)
>>  
>>  check-qtest-mips64-y = tests/endianness-test$(EXESUF)
>> +check-qtest-mips64-y += tests/cdrom-test$(EXESUF)
>>  
>> -check-qtest-mips64el-y = tests/endianness-test$(EXESUF)
>> +check-qtest-mips64el-y = $(check-qtest-mips64-y)
>>  
>>  check-qtest-moxie-y = tests/boot-serial-test$(EXESUF)
>>  
>> @@ -356,6 +357,7 @@ check-qtest-ppc64-y += tests/display-vga-test$(EXESUF)
>>  check-qtest-ppc64-y += tests/numa-test$(EXESUF)
>>  check-qtest-ppc64-$(CONFIG_IVSHMEM) += tests/ivshmem-test$(EXESUF)
>>  check-qtest-ppc64-y += tests/cpu-plug-test$(EXESUF)
>> +check-qtest-ppc64-y += tests/cdrom-test$(EXESUF)
>>  
>>  check-qtest-sh4-y = tests/endianness-test$(EXESUF)
>>  
>> @@ -365,10 +367,12 @@ check-qtest-sparc-y = tests/prom-env-test$(EXESUF)
>>  check-qtest-sparc-y += tests/m48t59-test$(EXESUF)
>>  gcov-files-sparc-y = hw/timer/m48t59.c
>>  check-qtest-sparc-y += tests/boot-serial-test$(EXESUF)
>> +check-qtest-sparc-y += tests/cdrom-test$(EXESUF)
>>  
>>  check-qtest-sparc64-y = tests/endianness-test$(EXESUF)
>>  check-qtest-sparc64-y += tests/prom-env-test$(EXESUF)
>>  check-qtest-sparc64-y += tests/boot-serial-test$(EXESUF)
>> +check-qtest-sparc64-y += tests/cdrom-test$(EXESUF)
>>  
>>  check-qtest-arm-y = tests/tmp105-test$(EXESUF)
>>  check-qtest-arm-y += tests/ds1338-test$(EXESUF)
>> @@ -384,6 +388,7 @@ check-qtest-arm-y += tests/sdhci-test$(EXESUF)
>>  check-qtest-aarch64-y = tests/numa-test$(EXESUF)
>>  check-qtest-aarch64-y += tests/sdhci-test$(EXESUF)
>>  check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF)
>> +check-qtest-aarch64-y += tests/cdrom-test$(EXESUF)
> 
> Since you use g_str_equal(arch,), maybe this test can be added
> regardless the arch in check-qtest-generic.

Yes, sounds like a good idea. I think the test can even be run on other
architectures by using the default machine or "-M none" ... I'll give it
a try ...

>>  ret = g_test_run();
>>
> 
> Reviewed-by: Philippe Mathieu-Daudé 

 Thanks!
  Thomas

Re: [Qemu-devel] [RFC 0/3] target/m68k: fix TCGv array overflow

[Richard Henderson]

On 03/16/2018 03:19 AM, Laurent Vivier wrote:
> I try to fix this by introducing a new TCG function
> to try to free a TCGv if it is a temporary one and
> do nothing otherwise (patches 1 and 2)

I would prefer not to approach this in this way.

Better is to have translator helpers that allocate temps
that are freed at the end of the insn.

C.f. new_tmp_a64 in target/arm/translate-a64.c.


r~

Re: [Qemu-devel] [PATCH v2] dump-guest-memory: more descriptive lookup_type failure

[Laszlo Ersek]

On 03/14/18 16:38, Andrew Jones wrote:
> We've seen a few reports of
> 
>  (gdb) source /usr/share/qemu-kvm/dump-guest-memory.py
>  Traceback (most recent call last):
>File "/usr/share/qemu-kvm/dump-guest-memory.py", line 19, in 
>  UINTPTR_T = gdb.lookup_type("uintptr_t")
>  gdb.error: No type named uintptr_t.
> 
> This occurs when symbols haven't been loaded first, i.e. neither a
> QEMU binary was loaded nor a QEMU process was attached first. Let's
> better inform the user of how to fix the issue themselves in order
> to avoid more reports.
> 
> Acked-by: Janosch Frank 
> Signed-off-by: Andrew Jones 
> ---
> v2: Not quite so long a long line (< 90 only generates warnings)
> Pick up Janosch's ack
> 
>  scripts/dump-guest-memory.py | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
> index 51acfcd0c053..276eebf0c27e 100644
> --- a/scripts/dump-guest-memory.py
> +++ b/scripts/dump-guest-memory.py
> @@ -16,7 +16,12 @@ the COPYING file in the top-level directory.
>  import ctypes
>  import struct
>  
> -UINTPTR_T = gdb.lookup_type("uintptr_t")
> +try:
> +UINTPTR_T = gdb.lookup_type("uintptr_t")
> +except Exception as inst:
> +raise gdb.GdbError("Symbols must be loaded prior to sourcing 
> dump-guest-memory.\n"
> +   "Symbols may be loaded by 'attach'ing a QEMU process 
> id or by "
> +   "'load'ing a QEMU binary.")
>  
>  TARGET_PAGE_SIZE = 0x1000
>  TARGET_PAGE_MASK = 0xF000
> 

Ah, cool.

Reviewed-by: Laszlo Ersek 

Thanks!
Laszlo

Re: [Qemu-devel] [PATCH 1/3] tests/boot-sector: Add magic bytes to s390x boot code header

[Thomas Huth]

On 15.03.2018 12:47, Philippe Mathieu-Daudé wrote:
> On 03/15/2018 08:49 AM, Thomas Huth wrote:
>> We're going to use the s390x boot code for testing CD-ROM booting.
>> But the ISO loader of the s390-ccw bios is a little bit more picky
>> than the network loader and expects some magic bytes in the header
>> of the file (see linux_s390_magic in pc-bios/s390-ccw/bootmap.c), so
>> we've got to add them in our boot code here, too.
>>
>> Signed-off-by: Thomas Huth 
>> ---
>>  tests/boot-sector.c | 9 ++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/tests/boot-sector.c b/tests/boot-sector.c
>> index c373f0e..04721c9 100644
>> --- a/tests/boot-sector.c
>> +++ b/tests/boot-sector.c
>> @@ -68,8 +68,11 @@ static uint8_t x86_boot_sector[512] = {
>>  };
>>  
>>  /* For s390x, use a mini "kernel" with the appropriate signature */
>> -static const uint8_t s390x_psw[] = {
>> -0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00
>> +static const uint8_t s390x_psw_and_magic[] = {
> 
> Can you add a comment such "see linux_s390_magic in
> pc-bios/s390-ccw/bootmap.c"?

Sure, I'll add a comment.

 Thomas

Re: [Qemu-devel] [PATCH v2] RISC-V: Fix riscv_isa_string, use popcount to count bits

[Peter Maydell]

On 10 March 2018 at 21:25, Philippe Mathieu-Daudé  wrote:
> On 03/09/2018 10:01 PM, Michael Clark wrote:
>> Logic bug caused the string size calculation for the RISC-V
>> format ISA string to be small. This fix allows slack for rv128.
>>
>> Cc: Palmer Dabbelt 
>> Cc: Peter Maydell 
>> Cc: Eric Blake 
>> Signed-off-by: Michael Clark 
>> ---
>>  target/riscv/cpu.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 4851890..1456535 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -391,7 +391,7 @@ static const TypeInfo riscv_cpu_type_info = {
>>  char *riscv_isa_string(RISCVCPU *cpu)
>>  {
>>  int i;
>> -size_t maxlen = 5 + ctz32(cpu->env.misa);
>> +size_t maxlen = 8 + ctpop64(cpu->env.misa);
>
> Can you document the magic 5/8?
>
> This looks nice, but this seems to me too much optimization to save few
> bytes, using sizeof(riscv_exts) is overflow-free.
>
> Maybe this is enough and self-explanatory:
>
>const size_t maxlen = sizeof("rv128") + sizeof(riscv_exts);
>
>>  char *isa_string = g_new0(char, maxlen);
>>  snprintf(isa_string, maxlen, "rv%d", TARGET_LONG_BITS);
>
> Also, if you keep the snprintf() return value, you can (naming it 'n')
> simplify (also easier to review):
>
>>  for (i = 0; i < sizeof(riscv_exts); i++) {
>>
> if (cpu->env.misa & RV(riscv_exts[i])) {
> -   isa_string[strlen(isa_string)] = riscv_exts[i] - 'A' + 'a';
> +   isa_string[n++] = tolower(riscv_exts[i]);
> }
> }
>
> and simply use g_new() with:
>
> +   isa_string[n] = '\0';
>
> return isa_string;
> }

Hi -- any chance of a respin of this patch that addresses Philippe's
review comments so we can fix it before rc0? This is causing
my merge-build tests to fail about 50% of the time on OpenBSD
at the moment...

thanks
-- PMM

Re: [Qemu-devel] [PATCH] dump-guest-memory: more descriptive lookup_type failure

[Laszlo Ersek]

On 03/14/18 15:24, no-re...@patchew.org wrote:

> Checking PATCH 1/1: dump-guest-memory: more descriptive lookup_type failure...
> WARNING: line over 80 characters
> #34: FILE: scripts/dump-guest-memory.py:22:
> +raise gdb.GdbError("Symbols must be loaded prior to sourcing 
> dump-guest-memory.\n"
> 
> ERROR: line over 90 characters
> #35: FILE: scripts/dump-guest-memory.py:23:
> +   "Symbols may be loaded by first 'attach'ing a QEMU 
> process id or by 'load'ing a QEMU binary.")
> 
> total: 1 errors, 1 warnings, 12 lines checked

The current "width" of the script is currently:

$ wc -L scripts/dump-guest-memory.py
81 scripts/dump-guest-memory.py

I think it would be good to stick with 80 chars in this patch. (While
adhering to whatever Python idioms are considered necessary -- I think
Janosch and Marc-André can help with that.)

Once that's fixed, please add my:

Reviewed-by: Laszlo Ersek 

(And, I agree, this error message has been reported several times to me
as well.)

Thanks!
Laszlo

[Qemu-devel] [RFC 2/3] target/m68k: use tcg_temp_try_free()

[Laurent Vivier]

SRC_EA() and gen_extend() can return either a temporary
TCGv or memory allocated one. Try to free the temporary
one with tcg_temp_try_free().

Signed-off-by: Laurent Vivier 
---
 target/m68k/translate.c | 65 +
 1 file changed, 65 insertions(+)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index cef6f663ad..03aa701dde 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -1550,6 +1550,7 @@ DISAS_INSN(mulw)
 tcg_gen_ext16u_i32(tmp, reg);
 SRC_EA(env, src, OS_WORD, sign, NULL);
 tcg_gen_mul_i32(tmp, tmp, src);
+tcg_temp_try_free(src);
 tcg_gen_mov_i32(reg, tmp);
 gen_logic_cc(s, tmp, OS_LONG);
 tcg_temp_free(tmp);
@@ -1574,6 +1575,7 @@ DISAS_INSN(divw)
 } else {
 gen_helper_divuw(cpu_env, destr, src);
 }
+tcg_temp_try_free(src);
 tcg_temp_free(destr);
 
 set_cc_op(s, CC_OP_FLAGS);
@@ -1605,6 +1607,7 @@ DISAS_INSN(divl)
 } else {
 gen_helper_divull(cpu_env, num, reg, den);
 }
+tcg_temp_try_free(den);
 tcg_temp_free(reg);
 tcg_temp_free(num);
 set_cc_op(s, CC_OP_FLAGS);
@@ -1622,6 +1625,7 @@ DISAS_INSN(divl)
 } else {
 gen_helper_divul(cpu_env, num, reg, den);
 }
+tcg_temp_try_free(den);
 tcg_temp_free(reg);
 tcg_temp_free(num);
 
@@ -1762,9 +1766,11 @@ DISAS_INSN(abcd_reg)
 src = gen_extend(DREG(insn, 0), OS_BYTE, 0);
 dest = gen_extend(DREG(insn, 9), OS_BYTE, 0);
 bcd_add(dest, src);
+tcg_temp_try_free(src);
 gen_partset_reg(OS_BYTE, DREG(insn, 9), dest);
 
 bcd_flags(dest);
+tcg_temp_try_free(dest);
 }
 
 DISAS_INSN(abcd_mem)
@@ -1781,11 +1787,13 @@ DISAS_INSN(abcd_mem)
NULL_QREG, , EA_LOADU, IS_USER(s));
 
 bcd_add(dest, src);
+tcg_temp_try_free(src);
 
 gen_ea_mode(env, s, 4, REG(insn, 9), OS_BYTE, dest, ,
 EA_STORE, IS_USER(s));
 
 bcd_flags(dest);
+tcg_temp_try_free(dest);
 }
 
 DISAS_INSN(sbcd_reg)
@@ -1798,10 +1806,12 @@ DISAS_INSN(sbcd_reg)
 dest = gen_extend(DREG(insn, 9), OS_BYTE, 0);
 
 bcd_sub(dest, src);
+tcg_temp_try_free(src);
 
 gen_partset_reg(OS_BYTE, DREG(insn, 9), dest);
 
 bcd_flags(dest);
+tcg_temp_try_free(dest);
 }
 
 DISAS_INSN(sbcd_mem)
@@ -1818,11 +1828,13 @@ DISAS_INSN(sbcd_mem)
NULL_QREG, , EA_LOADU, IS_USER(s));
 
 bcd_sub(dest, src);
+tcg_temp_try_free(src);
 
 gen_ea_mode(env, s, 4, REG(insn, 9), OS_BYTE, dest, ,
 EA_STORE, IS_USER(s));
 
 bcd_flags(dest);
+tcg_temp_try_free(dest);
 }
 
 DISAS_INSN(nbcd)
@@ -1836,6 +1848,7 @@ DISAS_INSN(nbcd)
 
 dest = tcg_const_i32(0);
 bcd_sub(dest, src);
+tcg_temp_try_free(src);
 
 DEST_EA(env, insn, OS_BYTE, dest, );
 
@@ -1877,9 +1890,12 @@ DISAS_INSN(addsub)
 gen_update_cc_add(dest, src, opsize);
 if (insn & 0x100) {
 DEST_EA(env, insn, opsize, dest, );
+tcg_temp_try_free(tmp);
 } else {
 gen_partset_reg(opsize, DREG(insn, 9), dest);
+tcg_temp_try_free(src);
 }
+tcg_temp_try_free(reg);
 tcg_temp_free(dest);
 }
 
@@ -1935,6 +1951,7 @@ DISAS_INSN(bitop_reg)
 default: /* btst */
 break;
 }
+tcg_temp_try_free(src1);
 tcg_temp_free(tmp);
 if (op) {
 DEST_EA(env, insn, opsize, dest, );
@@ -2183,6 +2200,7 @@ DISAS_INSN(bitop_im)
 DEST_EA(env, insn, opsize, tmp, );
 tcg_temp_free(tmp);
 }
+tcg_temp_try_free(src1);
 }
 
 static TCGv gen_get_ccr(DisasContext *s)
@@ -2244,6 +2262,7 @@ static void gen_move_to_sr(CPUM68KState *env, 
DisasContext *s, uint16_t insn,
 TCGv src;
 SRC_EA(env, src, OS_WORD, 0, NULL);
 gen_set_sr(s, src, ccr_only);
+tcg_temp_try_free(src);
 }
 }
 
@@ -2306,6 +2325,7 @@ DISAS_INSN(arith_im)
 } else {
 DEST_EA(env, insn, opsize, dest, );
 gen_logic_cc(s, dest, opsize);
+tcg_temp_try_free(src1);
 }
 break;
 case 1: /* andi */
@@ -2315,6 +2335,7 @@ DISAS_INSN(arith_im)
 } else {
 DEST_EA(env, insn, opsize, dest, );
 gen_logic_cc(s, dest, opsize);
+tcg_temp_try_free(src1);
 }
 break;
 case 2: /* subi */
@@ -2323,6 +2344,7 @@ DISAS_INSN(arith_im)
 gen_update_cc_add(dest, im, opsize);
 set_cc_op(s, CC_OP_SUBB + opsize);
 DEST_EA(env, insn, opsize, dest, );
+tcg_temp_try_free(src1);
 break;
 case 3: /* addi */
 tcg_gen_add_i32(dest, src1, im);
@@ -2330,6 +2352,7 @@ DISAS_INSN(arith_im)
 tcg_gen_setcond_i32(TCG_COND_LTU, QREG_CC_X, dest, im);
 set_cc_op(s, CC_OP_ADDB + opsize);
 DEST_EA(env, insn, opsize, dest, );
+tcg_temp_try_free(src1);
 break;
 case 5: /* eori */
 tcg_gen_xor_i32(dest, src1, im);
@@ -2338,10 +2361,12 @@ 

[Qemu-devel] [RFC 3/3] m68k: Test if we overflow the temp variable array

[Laurent Vivier]

Since commit 15fa08f845 ("tcg: Dynamically allocate TCGOps")
we have no limit to fill the TCGOps cache and we can fill
the entire TCG variables array and overflow it.

To avoid that, we stop the translation when the array is close to
be full.

Signed-off-by: Laurent Vivier 
---
 target/m68k/translate.c | 2 +-
 tcg/tcg.h   | 6 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 03aa701dde..e235be46ba 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -6155,7 +6155,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock 
*tb)
 
 dc->insn_pc = dc->pc;
disas_m68k_insn(env, dc);
-} while (!dc->is_jmp && !tcg_op_buf_full() &&
+} while (!dc->is_jmp && !tcg_op_buf_full() && !tcg_temp_full(64) &&
  !cs->singlestep_enabled &&
  !singlestep &&
  (pc_offset) < (TARGET_PAGE_SIZE - 32) &&
diff --git a/tcg/tcg.h b/tcg/tcg.h
index e6d9dc0643..ccfe050e27 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -836,6 +836,12 @@ static inline bool tcg_op_buf_full(void)
 {
 return false;
 }
+/* Test if we overflow the temp variable array */
+
+static inline bool tcg_temp_full(int marging)
+{
+return tcg_ctx->nb_temps > TCG_MAX_TEMPS - marging;
+}
 
 /* pool based memory allocation */
 
-- 
2.14.3

Re: [Qemu-devel] [PULL 0/7] Machine queue, 2018-03-12

[Eduardo Habkost]

On Thu, Mar 15, 2018 at 07:31:09PM +0100, Philippe Mathieu-Daudé wrote:
> Hi Eduardo,
> 
> On 03/13/2018 07:37 PM, Eduardo Habkost wrote:
> > On Tue, Mar 13, 2018 at 06:29:10PM +, Peter Maydell wrote:
> >> On 12 March 2018 at 22:34, Eduardo Habkost  wrote:
> >>> The following changes since commit 
> >>> 6ceb1b51f05f9e1892d082960ed602dca7b6696e:
> >>>
> >>>   Merge remote-tracking branch 
> >>> 'remotes/kraxel/tags/audio-20180312-pull-request' into staging 
> >>> (2018-03-12 16:14:37 +)
> >>>
> >>> are available in the Git repository at:
> >>>
> >>>   git://github.com/ehabkost/qemu.git tags/machine-next-pull-request
> >>>
> >>> for you to fetch changes up to a82664f2f8b1ce2db90e644df01d7ca7845881a6:
> >>>
> >>>   cpu: drop unnecessary NULL check and cpu_common_class_by_name() 
> >>> (2018-03-12 19:33:32 -0300)
> >>>
> >>> 
> >>> Machine queue, 2018-03-12
> >>>
> >>
> >> Hi. It looks like this breaks bsd-user compilation:
> >>
> >> /root/qemu/bsd-user/main.c: In function 'main':
> >> /root/qemu/bsd-user/main.c:905:5: warning: implicit declaration of
> >> function 'create' [-Wimplicit-function-declaration]
> >>  cpu = create(cpu_type);
> >>  ^
> >> /root/qemu/bsd-user/main.c:905:5: warning: nested extern declaration
> >> of 'create' [-Wnested-externs]
> >> /root/qemu/bsd-user/main.c:905:9: warning: assignment makes pointer
> >> from integer without a cast [enabled by default]
> >>  cpu = create(cpu_type);
> >>  ^
> >>
> >> and then later it fails to link with
> >> /root/qemu/bsd-user/main.c:905: undefined reference to `create'
> > 
> > Oops, sorry.  I don't have a BSD host handy right now, are you able to 
> > confirm
> > if the new tag I pushed fixes it?
> 
> You might try:
> 
> $ make vm-test
> vm-test: Test QEMU in preconfigured virtual machines
> 
>   vm-build-ubuntu.i386- Build QEMU in ubuntu i386 VM
>   vm-build-freebsd- Build QEMU in FreeBSD VM
>   vm-build-netbsd - Build QEMU in NetBSD VM
>   vm-build-openbsd- Build QEMU in OpenBSD VM

This is awesome, thanks for the pointer!

-- 
Eduardo

[Qemu-devel] [RFC 1/3] tcg: introduce tcg_temp_try_free()

[Laurent Vivier]

m68k has some functions returning either
locally allocated TCGv or memory allocated
TCGv (registers). We want to free locally
allocated TCGv to avoid overflow in sequence like:

0xc00ae406:  movel %fp@(-132),%fp@(-268)
0xc00ae40c:  movel %fp@(-128),%fp@(-264)
0xc00ae412:  movel %fp@(-20),%fp@(-212)
0xc00ae418:  movel %fp@(-16),%fp@(-208)
0xc00ae41e:  movel %fp@(-60),%fp@(-220)
0xc00ae424:  movel %fp@(-56),%fp@(-216)
0xc00ae42a:  movel %fp@(-124),%fp@(-252)
0xc00ae430:  movel %fp@(-120),%fp@(-248)
0xc00ae436:  movel %fp@(-12),%fp@(-260)
0xc00ae43c:  movel %fp@(-8),%fp@(-256)
0xc00ae442:  movel %fp@(-52),%fp@(-276)
0xc00ae448:  movel %fp@(-48),%fp@(-272)
...

That can fill a lot of TCGv entries in a sequence,
especially since 15fa08f845 ("tcg: Dynamically allocate TCGOps")

Signed-off-by: Laurent Vivier 
---
 tcg/tcg-op.h |  2 ++
 tcg/tcg.c| 28 +---
 tcg/tcg.h|  3 +++
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
index 75bb55aeac..564e310426 100644
--- a/tcg/tcg-op.h
+++ b/tcg/tcg-op.h
@@ -811,6 +811,7 @@ void tcg_gen_lookup_and_goto_ptr(void);
 #define tcg_global_mem_new tcg_global_mem_new_i32
 #define tcg_temp_local_new() tcg_temp_local_new_i32()
 #define tcg_temp_free tcg_temp_free_i32
+#define tcg_temp_try_free tcg_temp_try_free_i32
 #define tcg_gen_qemu_ld_tl tcg_gen_qemu_ld_i32
 #define tcg_gen_qemu_st_tl tcg_gen_qemu_st_i32
 #else
@@ -819,6 +820,7 @@ void tcg_gen_lookup_and_goto_ptr(void);
 #define tcg_global_mem_new tcg_global_mem_new_i64
 #define tcg_temp_local_new() tcg_temp_local_new_i64()
 #define tcg_temp_free tcg_temp_free_i64
+#define tcg_temp_try_free tcg_temp_try_free_i64
 #define tcg_gen_qemu_ld_tl tcg_gen_qemu_ld_i64
 #define tcg_gen_qemu_st_tl tcg_gen_qemu_st_i64
 #endif
diff --git a/tcg/tcg.c b/tcg/tcg.c
index bb24526c93..9d02c07e7a 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -1072,11 +1072,15 @@ TCGv_vec tcg_temp_new_vec_matching(TCGv_vec match)
 return temp_tcgv_vec(t);
 }
 
-static void tcg_temp_free_internal(TCGTemp *ts)
+static void tcg_temp_free_internal(TCGTemp *ts, bool try)
 {
 TCGContext *s = tcg_ctx;
 int k, idx;
 
+if (try && ts->temp_allocated == 0) {
+return;
+}
+
 #if defined(CONFIG_DEBUG_TCG)
 s->temps_in_use--;
 if (s->temps_in_use < 0) {
@@ -1095,17 +1099,27 @@ static void tcg_temp_free_internal(TCGTemp *ts)
 
 void tcg_temp_free_i32(TCGv_i32 arg)
 {
-tcg_temp_free_internal(tcgv_i32_temp(arg));
+tcg_temp_free_internal(tcgv_i32_temp(arg), false);
 }
 
 void tcg_temp_free_i64(TCGv_i64 arg)
 {
-tcg_temp_free_internal(tcgv_i64_temp(arg));
+tcg_temp_free_internal(tcgv_i64_temp(arg), false);
+}
+
+void tcg_temp_try_free_i32(TCGv_i32 arg)
+{
+tcg_temp_free_internal(tcgv_i32_temp(arg), true);
+}
+
+void tcg_temp_try_free_i64(TCGv_i64 arg)
+{
+tcg_temp_free_internal(tcgv_i64_temp(arg), true);
 }
 
 void tcg_temp_free_vec(TCGv_vec arg)
 {
-tcg_temp_free_internal(tcgv_vec_temp(arg));
+tcg_temp_free_internal(tcgv_vec_temp(arg), false);
 }
 
 TCGv_i32 tcg_const_i32(int32_t val)
@@ -1572,8 +1586,8 @@ void tcg_gen_callN(void *func, TCGTemp *ret, int nargs, 
TCGTemp **args)
 for (i = real_args = 0; i < orig_nargs; ++i) {
 int is_64bit = orig_sizemask & (1 << (i+1)*2);
 if (is_64bit) {
-tcg_temp_free_internal(args[real_args++]);
-tcg_temp_free_internal(args[real_args++]);
+tcg_temp_free_internal(args[real_args++], false);
+tcg_temp_free_internal(args[real_args++], false);
 } else {
 real_args++;
 }
@@ -1590,7 +1604,7 @@ void tcg_gen_callN(void *func, TCGTemp *ret, int nargs, 
TCGTemp **args)
 for (i = 0; i < nargs; ++i) {
 int is_64bit = sizemask & (1 << (i+1)*2);
 if (!is_64bit) {
-tcg_temp_free_internal(args[i]);
+tcg_temp_free_internal(args[i], false);
 }
 }
 #endif /* TCG_TARGET_EXTEND_ARGS */
diff --git a/tcg/tcg.h b/tcg/tcg.h
index 9e2d909a4a..e6d9dc0643 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -890,6 +890,9 @@ void tcg_temp_free_i32(TCGv_i32 arg);
 void tcg_temp_free_i64(TCGv_i64 arg);
 void tcg_temp_free_vec(TCGv_vec arg);
 
+void tcg_temp_try_free_i32(TCGv_i32 arg);
+void tcg_temp_try_free_i64(TCGv_i64 arg);
+
 static inline TCGv_i32 tcg_global_mem_new_i32(TCGv_ptr reg, intptr_t offset,
   const char *name)
 {
-- 
2.14.3

[Qemu-devel] [RFC 0/3] target/m68k: fix TCGv array overflow

[Laurent Vivier]

Since commit 15fa08f845 ("tcg: Dynamically allocate TCGOps")
we have no limit to fill the TCGOps cache and we can fill
the entire TCG variables array and overflow it.

It seems to happen only with m68k, because m68k translator
doesn't free some TCGv at end of instruction translation
because the variable can be either temporary one or an
allocated one,

I try to fix this by introducing a new TCG function
to try to free a TCGv if it is a temporary one and
do nothing otherwise (patches 1 and 2)

The last patch is here to avoid the error and
stop the translation before the buffer overflows
(but I guess we should not need this with correctly
written translation functions...)

Laurent Vivier (3):
  tcg: introduce tcg_temp_try_free()
  target/m68k: use tcg_temp_try_free()
  m68k: Test if we overflow the temp variable array

 target/m68k/translate.c | 67 -
 tcg/tcg-op.h|  2 ++
 tcg/tcg.c   | 28 +++--
 tcg/tcg.h   |  9 +++
 4 files changed, 98 insertions(+), 8 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH 2/2] char: i.MX: Add support for "TX complete" interrupt

[Andrey Smirnov]

Add support for "TX complete"/TXDC interrupt generate by real HW since
it is needed to support guests other than Linux.

Based on the patch by Bill Paul as found here:
https://bugs.launchpad.net/qemu/+bug/1753314

Cc: qemu-devel@nongnu.org
Cc: qemu-...@nongnu.org
Cc: Bill Paul 
Cc: Peter Maydell 
Signed-off-by: Andrey Smirnov 
---

Bill:

I only tested this with i.MX7/Linux guest combo and am hoping you can
take this and previous patch and give it a try on your VxWorks setup.

Peter:

Bill is the author of original code, so I am not sure how to handle
Signed-off-by from him. I'd like to add it to this patch, but since
his original submission to launchpad didn't have one it is currently
omitted.

 include/hw/char/imx_serial.h |  3 +++
 hw/char/imx_serial.c | 20 +---
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/include/hw/char/imx_serial.h b/include/hw/char/imx_serial.h
index baeec3183f..5b99cee7cf 100644
--- a/include/hw/char/imx_serial.h
+++ b/include/hw/char/imx_serial.h
@@ -67,6 +67,8 @@
 #define UCR2_RXEN   (1<<1)/* Receiver enable */
 #define UCR2_SRST   (1<<0)/* Reset complete */
 
+#define UCR4_TCEN   BIT(3)/* TX complete interrupt enable */
+
 #define UTS1_TXEMPTY(1<<6)
 #define UTS1_RXEMPTY(1<<5)
 #define UTS1_TXFULL (1<<4)
@@ -95,6 +97,7 @@ typedef struct IMXSerialState {
 uint32_t ubmr;
 uint32_t ubrc;
 uint32_t ucr3;
+uint32_t ucr4;
 
 qemu_irq irq;
 CharBackend chr;
diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c
index d1e8586280..1e5540472b 100644
--- a/hw/char/imx_serial.c
+++ b/hw/char/imx_serial.c
@@ -37,8 +37,8 @@
 
 static const VMStateDescription vmstate_imx_serial = {
 .name = TYPE_IMX_SERIAL,
-.version_id = 1,
-.minimum_version_id = 1,
+.version_id = 2,
+.minimum_version_id = 2,
 .fields = (VMStateField[]) {
 VMSTATE_INT32(readbuff, IMXSerialState),
 VMSTATE_UINT32(usr1, IMXSerialState),
@@ -50,6 +50,7 @@ static const VMStateDescription vmstate_imx_serial = {
 VMSTATE_UINT32(ubmr, IMXSerialState),
 VMSTATE_UINT32(ubrc, IMXSerialState),
 VMSTATE_UINT32(ucr3, IMXSerialState),
+VMSTATE_UINT32(ucr4, IMXSerialState),
 VMSTATE_END_OF_LIST()
 },
 };
@@ -71,6 +72,11 @@ static void imx_update(IMXSerialState *s)
  * unfortunately.
  */
 mask = (s->ucr1 & UCR1_TXMPTYEN) ? USR2_TXFE : 0;
+/*
+ * TCEN and TXDC are both bit 3
+ */
+mask |= s->ucr4 & UCR4_TCEN;
+
 usr2 = s->usr2 & mask;
 
 qemu_set_irq(s->irq, usr1 || usr2);
@@ -163,6 +169,8 @@ static uint64_t imx_serial_read(void *opaque, hwaddr offset,
 return s->ucr3;
 
 case 0x23: /* UCR4 */
+return s->ucr4;
+
 case 0x29: /* BRM Incremental */
 return 0x0; /* TODO */
 
@@ -191,8 +199,10 @@ static void imx_serial_write(void *opaque, hwaddr offset,
  * qemu_chr_fe_write and background I/O callbacks */
 qemu_chr_fe_write_all(>chr, , 1);
 s->usr1 &= ~USR1_TRDY;
+s->usr2 &= ~USR2_TXDC;
 imx_update(s);
 s->usr1 |= USR1_TRDY;
+s->usr2 |= USR2_TXDC;
 imx_update(s);
 }
 break;
@@ -265,8 +275,12 @@ static void imx_serial_write(void *opaque, hwaddr offset,
 s->ucr3 = value & 0x;
 break;
 
-case 0x2d: /* UTS1 */
 case 0x23: /* UCR4 */
+s->ucr4 = value & 0x;
+imx_update(s);
+break;
+
+case 0x2d: /* UTS1 */
 qemu_log_mask(LOG_UNIMP, "[%s]%s: Unimplemented reg 0x%"
   HWADDR_PRIx "\n", TYPE_IMX_SERIAL, __func__, offset);
 /* TODO */
-- 
2.14.3

[Qemu-devel] [PATCH 1/2] char: i.MX: Simplify imx_update()

[Andrey Smirnov]

Code of imx_update() is slightly confusing since the "flags" variable
doesn't really corespond to anything in real hardware and server as a
kitchensink accumulating events normally reported via USR1 and USR2
registers.

Change the code to explicitly evaluate state of interrupts reported
via USR1 and USR2 against corresponding masking bits and use the to
detemine if IRQ line should be asserted or not.

NOTE: Check for UTS1_TXEMPTY being set has been dropped for two
reasons:

1. Emulation code implements a single character FIFO, so this flag
   will always be set since characters are trasmitted as a part of
   the code emulating "push" into the FIFO

2. imx_update() is really just a function doing ORing and maksing
   of reported events, so checking for UTS1_TXEMPTY should happen,
   if it's ever really needed should probably happen outside of
   it.

Cc: qemu-devel@nongnu.org
Cc: qemu-...@nongnu.org
Cc: Bill Paul 
Cc: Peter Maydell 
Signed-off-by: Andrey Smirnov 
---
 hw/char/imx_serial.c | 24 
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c
index 70405ccf8b..d1e8586280 100644
--- a/hw/char/imx_serial.c
+++ b/hw/char/imx_serial.c
@@ -56,16 +56,24 @@ static const VMStateDescription vmstate_imx_serial = {
 
 static void imx_update(IMXSerialState *s)
 {
-uint32_t flags;
+uint32_t usr1;
+uint32_t usr2;
+uint32_t mask;
 
-flags = (s->usr1 & s->ucr1) & (USR1_TRDY|USR1_RRDY);
-if (s->ucr1 & UCR1_TXMPTYEN) {
-flags |= (s->uts1 & UTS1_TXEMPTY);
-} else {
-flags &= ~USR1_TRDY;
-}
+/*
+ * Lucky for us TRDY and RRDY has the same offset in both USR1 and
+ * UCR1, so we can get away with something as simple as the
+ * following:
+ */
+usr1 = s->usr1 & s->ucr1 & (USR1_TRDY | USR1_RRDY);
+/*
+ * Bits that we want in USR2 are not as conveniently laid out,
+ * unfortunately.
+ */
+mask = (s->ucr1 & UCR1_TXMPTYEN) ? USR2_TXFE : 0;
+usr2 = s->usr2 & mask;
 
-qemu_set_irq(s->irq, !!flags);
+qemu_set_irq(s->irq, usr1 || usr2);
 }
 
 static void imx_serial_reset(IMXSerialState *s)
-- 
2.14.3

Re: [Qemu-devel] [PATCH v4 1/5] target/i386: Generalize some of the macro definitions

[Eduardo Habkost]

Hi,

Sorry for not reviewing the previous versions of this series
(making it miss soft freeze).


On Mon, Mar 12, 2018 at 05:00:45PM -0400, Babu Moger wrote:
> Generalize some of the macro definitions which are generic cache
> properties that are common between CPUID 4 and CPUID 0x801D
> in preparation for adding support for 0x801D.
> 
> Signed-off-by: Babu Moger 
> ---
>  target/i386/cpu.c | 52 ++--
>  1 file changed, 26 insertions(+), 26 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index b5e431e..42dd381 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -66,22 +66,22 @@
>  #define CPUID_2_L3_16MB_16WAY_64B 0x4d
>  
>  
> -/* CPUID Leaf 4 constants: */
> +/* Cache specific constants: */

We have too many CPUID leaves that describe caches, so I would
like to explicitly document on which CPUID leaves these macros
can be used.  I suggest:

  /* Macros for CPUID[4] and CPUID[0x801D] */


>  
>  /* EAX: */
> -#define CPUID_4_TYPE_DCACHE  1
> -#define CPUID_4_TYPE_ICACHE  2
> -#define CPUID_4_TYPE_UNIFIED 3
> +#define TYPE_DCACHE  1
> +#define TYPE_ICACHE  2
> +#define TYPE_UNIFIED 3

TYPE_* can be confused with QOM type names, I'd use something
else.  Maybe CACHE_TYPE_D, CACHE_TYPE_I, CACHE_TYPE_UNIFIED?


>  
> -#define CPUID_4_LEVEL(l)  ((l) << 5)
> +#define CACHE_LEVEL(l)  ((l) << 5)
>  
> -#define CPUID_4_SELF_INIT_LEVEL (1 << 8)
> -#define CPUID_4_FULLY_ASSOC (1 << 9)
> +#define CACHE_SELF_INIT_LEVEL (1 << 8)
> +#define CACHE_FULLY_ASSOC (1 << 9)
>  
>  /* EDX: */
> -#define CPUID_4_NO_INVD_SHARING (1 << 0)
> -#define CPUID_4_INCLUSIVE   (1 << 1)
> -#define CPUID_4_COMPLEX_IDX (1 << 2)
> +#define CACHE_NO_INVD_SHARING (1 << 0)
> +#define CACHE_INCLUSIVE   (1 << 1)
> +#define CACHE_COMPLEX_IDX (1 << 2)
>  
>  #define ASSOC_FULL 0xFF
>  
[...]

-- 
Eduardo

Re: [Qemu-devel] [PATCH v4 2/5] target/i386: Populate AMD Processor Cache Information

[Eduardo Habkost]

Hi,

Sorry for not reviewing the previous versions of this series (and
making it miss soft freeze).


On Mon, Mar 12, 2018 at 05:00:46PM -0400, Babu Moger wrote:
> From: Stanislav Lanci 
> 
> Add information for cpuid 0x801D leaf. Populate cache topology information
> for different cache types(Data Cache, Instruction Cache, L2 and L3) supported
> by 0x801D leaf. Please refer Processor Programming Reference (PPR) for AMD
> Family 17h Model for more details.
> 
> Signed-off-by: Stanislav Lanci 
> Signed-off-by: Babu Moger 
> ---
>  target/i386/cpu.c | 65 
> +++
>  target/i386/kvm.c | 29 ++---
>  2 files changed, 91 insertions(+), 3 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 42dd381..5fdbedd 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -118,6 +118,7 @@
>  #define L1I_LINE_SIZE 64
>  #define L1I_ASSOCIATIVITY  8
>  #define L1I_SETS  64
> +#define L1I_SETS_AMD 256
>  #define L1I_PARTITIONS 1
>  /* Size = LINE_SIZE*ASSOCIATIVITY*SETS*PARTITIONS = 32KiB */
>  #define L1I_DESCRIPTOR CPUID_2_L1I_32KB_8WAY_64B
> @@ -129,7 +130,9 @@
>  /* Level 2 unified cache: */
>  #define L2_LINE_SIZE  64
>  #define L2_ASSOCIATIVITY  16
> +#define L2_ASSOCIATIVITY_AMD   8
>  #define L2_SETS 4096
> +#define L2_SETS_AMD 1024
>  #define L2_PARTITIONS  1
>  /* Size = LINE_SIZE*ASSOCIATIVITY*SETS*PARTITIONS = 4MiB */
>  /*FIXME: CPUID leaf 2 descriptor is inconsistent with CPUID leaf 4 */
> @@ -146,6 +149,7 @@
>  #define L3_N_LINE_SIZE 64
>  #define L3_N_ASSOCIATIVITY 16
>  #define L3_N_SETS   16384
> +#define L3_N_SETS_AMD8192
>  #define L3_N_PARTITIONS 1
>  #define L3_N_DESCRIPTOR CPUID_2_L3_16MB_16WAY_64B
>  #define L3_N_LINES_PER_TAG  1

I wouldn't like to add even more inconsistencies between
different CPUID leaves.

If you really wish to have different defaults on AMD and Intel,
let's either hide Intel-specific CPUID leaves when using AMD
values, or make all of them agree (and choose the defaults based
on CPU model or vendor id).

-- 
Eduardo

Re: [Qemu-devel] [PULL 00/18] Linux user for 2.12 patches

[Peter Maydell]

On 13 March 2018 at 17:33, Laurent Vivier  wrote:
> The following changes since commit b39b61e410022f96ceb53d4381d25cba5126ac44:
>
>   memory: fix flatview_access_valid RCU read lock/unlock imbalance 
> (2018-03-09 15:55:20 +)
>
> are available in the Git repository at:
>
>   git://github.com/vivier/qemu.git tags/linux-user-for-2.12-pull-request
>
> for you to fetch changes up to 8c17d862b3cefed23a62c4e09d4b3f1f04a38631:
>
>   linux-user: init_guest_space: Add a comment about search strategy 
> (2018-03-13 15:04:01 +0100)
>
> 
>
> 
>
> Laurent Vivier (1):
>   linux-user: allows to use "--systemd ALL" with qemu-binfmt-conf.sh
>
> Luke Shumaker (9):
>   linux-user: Use #if to only call validate_guest_space for 32-bit ARM
> target
>   linux-user: Rename validate_guest_space => init_guest_commpage
>   linux-user: init_guest_space: Clean up if we can't initialize the
> commpage
>   linux-user: init_guest_space: Correctly handle guest_start in commpage
> initialization
>   linux-user: init_guest_space: Clarify page alignment logic
>   linux-user: init_guest_commpage: Add a comment about size check
>   linux-user: init_guest_space: Clean up control flow a bit
>   linux-user: init_guest_space: Don't try to align if we'll reject it
>   linux-user: init_guest_space: Add a comment about search strategy
>
> Max Filippov (5):
>   linux-user: fix mmap/munmap/mprotect/mremap/shmat
>   linux-user: fix assertion in shmdt
>   linux-user: fix target_mprotect/target_munmap error return values
>   linux-user: drop unused target_msync function
>   qemu-binfmt-conf.sh: add qemu-xtensa
>
> Peter Maydell (2):
>   linux-user: Drop unicore32 code
>   linux-user: Remove the unused "not implemented" signal handling stubs
>
> Shea Levy (1):
>   linux-user: Support f_flags in statfs when available.

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PULL 0/7] Machine queue, 2018-03-12

[Philippe Mathieu-Daudé]

Hi Eduardo,

On 03/13/2018 07:37 PM, Eduardo Habkost wrote:
> On Tue, Mar 13, 2018 at 06:29:10PM +, Peter Maydell wrote:
>> On 12 March 2018 at 22:34, Eduardo Habkost  wrote:
>>> The following changes since commit 6ceb1b51f05f9e1892d082960ed602dca7b6696e:
>>>
>>>   Merge remote-tracking branch 
>>> 'remotes/kraxel/tags/audio-20180312-pull-request' into staging (2018-03-12 
>>> 16:14:37 +)
>>>
>>> are available in the Git repository at:
>>>
>>>   git://github.com/ehabkost/qemu.git tags/machine-next-pull-request
>>>
>>> for you to fetch changes up to a82664f2f8b1ce2db90e644df01d7ca7845881a6:
>>>
>>>   cpu: drop unnecessary NULL check and cpu_common_class_by_name() 
>>> (2018-03-12 19:33:32 -0300)
>>>
>>> 
>>> Machine queue, 2018-03-12
>>>
>>
>> Hi. It looks like this breaks bsd-user compilation:
>>
>> /root/qemu/bsd-user/main.c: In function 'main':
>> /root/qemu/bsd-user/main.c:905:5: warning: implicit declaration of
>> function 'create' [-Wimplicit-function-declaration]
>>  cpu = create(cpu_type);
>>  ^
>> /root/qemu/bsd-user/main.c:905:5: warning: nested extern declaration
>> of 'create' [-Wnested-externs]
>> /root/qemu/bsd-user/main.c:905:9: warning: assignment makes pointer
>> from integer without a cast [enabled by default]
>>  cpu = create(cpu_type);
>>  ^
>>
>> and then later it fails to link with
>> /root/qemu/bsd-user/main.c:905: undefined reference to `create'
> 
> Oops, sorry.  I don't have a BSD host handy right now, are you able to confirm
> if the new tag I pushed fixes it?

You might try:

$ make vm-test
vm-test: Test QEMU in preconfigured virtual machines

  vm-build-ubuntu.i386- Build QEMU in ubuntu i386 VM
  vm-build-freebsd- Build QEMU in FreeBSD VM
  vm-build-netbsd - Build QEMU in NetBSD VM
  vm-build-openbsd- Build QEMU in OpenBSD VM

Regards,

Phil.

Re: [Qemu-devel] [PULL 0/7] Machine queue, 2018-03-12

[Eduardo Habkost]

On Tue, Mar 13, 2018 at 03:37:04PM -0300, Eduardo Habkost wrote:
> On Tue, Mar 13, 2018 at 06:29:10PM +, Peter Maydell wrote:
> > On 12 March 2018 at 22:34, Eduardo Habkost  wrote:
> > > The following changes since commit 
> > > 6ceb1b51f05f9e1892d082960ed602dca7b6696e:
> > >
> > >   Merge remote-tracking branch 
> > > 'remotes/kraxel/tags/audio-20180312-pull-request' into staging 
> > > (2018-03-12 16:14:37 +)
> > >
> > > are available in the Git repository at:
> > >
> > >   git://github.com/ehabkost/qemu.git tags/machine-next-pull-request
> > >
> > > for you to fetch changes up to a82664f2f8b1ce2db90e644df01d7ca7845881a6:
> > >
> > >   cpu: drop unnecessary NULL check and cpu_common_class_by_name() 
> > > (2018-03-12 19:33:32 -0300)
> > >
> > > 
> > > Machine queue, 2018-03-12
> > >
> > 
> > Hi. It looks like this breaks bsd-user compilation:
> > 
> > /root/qemu/bsd-user/main.c: In function 'main':
> > /root/qemu/bsd-user/main.c:905:5: warning: implicit declaration of
> > function 'create' [-Wimplicit-function-declaration]
> >  cpu = create(cpu_type);
> >  ^
> > /root/qemu/bsd-user/main.c:905:5: warning: nested extern declaration
> > of 'create' [-Wnested-externs]
> > /root/qemu/bsd-user/main.c:905:9: warning: assignment makes pointer
> > from integer without a cast [enabled by default]
> >  cpu = create(cpu_type);
> >  ^
> > 
> > and then later it fails to link with
> > /root/qemu/bsd-user/main.c:905: undefined reference to `create'
> 
> Oops, sorry.  I don't have a BSD host handy right now, are you able to confirm
> if the new tag I pushed fixes it?

I didn't get test results on BSD, but as the fix was obvious I sent a
new pull request today ("[PULL v2 0/7] Machine queue, 2018-03-15").


> 
> The following changes since commit 026aaf47c02b79036feb830206cfebb2a726510d:
> 
>   Merge remote-tracking branch 
> 'remotes/ehabkost/tags/python-next-pull-request' into staging (2018-03-13 
> 16:26:44 +)
> 
> are available in the Git repository at:
> 
>   git://github.com/ehabkost/qemu.git tags/machine-next-pull-request
> 
> for you to fetch changes up to e291aefcd38ca3d048e45f644e9321d02fc253cc:
> 
>   cpu: drop unnecessary NULL check and cpu_common_class_by_name() (2018-03-13 
> 15:33:26 -0300)
> 
> 
> machine queue, 2018-03-13
> 
> 
> 
> Igor Mammedov (6):
>   nios2: 10m50_devboard: replace cpu_model with cpu_type
>   tests: add machine 'none' with -cpu test
>   cpu: add CPU_RESOLVING_TYPE macro
>   Use cpu_create(type) instead of cpu_init(cpu_model)
>   cpu: get rid of unused cpu_init() defines
>   cpu: drop unnecessary NULL check and cpu_common_class_by_name()
> 
> Wang Xin (1):
>   pc: correct misspelled CPU model-id for pc 2.2
> 
> -- 
> Eduardo
> 

-- 
Eduardo

[Qemu-devel] [PULL v2 6/7] cpu: get rid of unused cpu_init() defines

[Eduardo Habkost]

From: Igor Mammedov 

cpu_init(cpu_model) were replaced by cpu_create(cpu_type) so
no users are left, remove it.

Signed-off-by: Igor Mammedov 
Acked-by: David Gibson  (ppc)
Reviewed-by: Eduardo Habkost 
Message-Id: <151827-274608-6-git-send-email-imamm...@redhat.com>
Signed-off-by: Eduardo Habkost 
---
 target/alpha/cpu.h  | 2 --
 target/arm/cpu.h| 2 --
 target/cris/cpu.h   | 2 --
 target/hppa/cpu.h   | 1 -
 target/i386/cpu.h   | 2 --
 target/lm32/cpu.h   | 2 --
 target/m68k/cpu.h   | 2 --
 target/microblaze/cpu.h | 1 -
 target/mips/cpu.h   | 2 --
 target/moxie/cpu.h  | 2 --
 target/nios2/cpu.h  | 1 -
 target/openrisc/cpu.h   | 2 --
 target/ppc/cpu.h| 2 --
 target/s390x/cpu.h  | 2 --
 target/sh4/cpu.h| 2 --
 target/sparc/cpu.h  | 4 
 target/tilegx/cpu.h | 1 -
 target/tricore/cpu.h| 2 --
 target/unicore32/cpu.h  | 2 --
 target/xtensa/cpu.h | 2 --
 20 files changed, 38 deletions(-)

diff --git a/target/alpha/cpu.h b/target/alpha/cpu.h
index 905855a2a1..7b50be785d 100644
--- a/target/alpha/cpu.h
+++ b/target/alpha/cpu.h
@@ -466,8 +466,6 @@ enum {
 
 void alpha_translate_init(void);
 
-#define cpu_init(cpu_model) cpu_generic_init(TYPE_ALPHA_CPU, cpu_model)
-
 #define ALPHA_CPU_TYPE_SUFFIX "-" TYPE_ALPHA_CPU
 #define ALPHA_CPU_TYPE_NAME(model) model ALPHA_CPU_TYPE_SUFFIX
 #define CPU_RESOLVING_TYPE TYPE_ALPHA_CPU
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index f4b4258655..19a0c03f9b 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2302,8 +2302,6 @@ static inline bool arm_excp_unmasked(CPUState *cs, 
unsigned int excp_idx,
 return unmasked || pstate_unmasked;
 }
 
-#define cpu_init(cpu_model) cpu_generic_init(TYPE_ARM_CPU, cpu_model)
-
 #define ARM_CPU_TYPE_SUFFIX "-" TYPE_ARM_CPU
 #define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX)
 #define CPU_RESOLVING_TYPE TYPE_ARM_CPU
diff --git a/target/cris/cpu.h b/target/cris/cpu.h
index cfb877c488..8bb1dbc989 100644
--- a/target/cris/cpu.h
+++ b/target/cris/cpu.h
@@ -267,8 +267,6 @@ enum {
 #define TARGET_PHYS_ADDR_SPACE_BITS 32
 #define TARGET_VIRT_ADDR_SPACE_BITS 32
 
-#define cpu_init(cpu_model) cpu_generic_init(TYPE_CRIS_CPU, cpu_model)
-
 #define CRIS_CPU_TYPE_SUFFIX "-" TYPE_CRIS_CPU
 #define CRIS_CPU_TYPE_NAME(name) (name CRIS_CPU_TYPE_SUFFIX)
 #define CPU_RESOLVING_TYPE TYPE_CRIS_CPU
diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index 6ea5f4c4e4..19dd12a93e 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -266,7 +266,6 @@ static inline int cpu_mmu_index(CPUHPPAState *env, bool 
ifetch)
 
 void hppa_translate_init(void);
 
-#define cpu_init(cpu_model) cpu_generic_init(TYPE_HPPA_CPU, cpu_model)
 #define CPU_RESOLVING_TYPE TYPE_HPPA_CPU
 
 void hppa_cpu_list(FILE *f, fprintf_function cpu_fprintf);
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index a0299289ae..4e95af2619 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1586,8 +1586,6 @@ uint64_t cpu_get_tsc(CPUX86State *env);
 
 #define PHYS_ADDR_MASK MAKE_64BIT_MASK(0, TCG_PHYS_ADDR_BITS)
 
-#define cpu_init(cpu_model) cpu_generic_init(TYPE_X86_CPU, cpu_model)
-
 #define X86_CPU_TYPE_SUFFIX "-" TYPE_X86_CPU
 #define X86_CPU_TYPE_NAME(name) (name X86_CPU_TYPE_SUFFIX)
 #define CPU_RESOLVING_TYPE TYPE_X86_CPU
diff --git a/target/lm32/cpu.h b/target/lm32/cpu.h
index 6f41955528..66157eefe9 100644
--- a/target/lm32/cpu.h
+++ b/target/lm32/cpu.h
@@ -255,8 +255,6 @@ void lm32_watchpoint_insert(CPULM32State *env, int index, 
target_ulong address,
 void lm32_watchpoint_remove(CPULM32State *env, int index);
 bool lm32_cpu_do_semihosting(CPUState *cs);
 
-#define cpu_init(cpu_model) cpu_generic_init(TYPE_LM32_CPU, cpu_model)
-
 #define LM32_CPU_TYPE_SUFFIX "-" TYPE_LM32_CPU
 #define LM32_CPU_TYPE_NAME(model) model LM32_CPU_TYPE_SUFFIX
 #define CPU_RESOLVING_TYPE TYPE_LM32_CPU
diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index f102e7216d..c63adf772f 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -527,8 +527,6 @@ enum {
 #define TARGET_PHYS_ADDR_SPACE_BITS 32
 #define TARGET_VIRT_ADDR_SPACE_BITS 32
 
-#define cpu_init(cpu_model) cpu_generic_init(TYPE_M68K_CPU, cpu_model)
-
 #define M68K_CPU_TYPE_SUFFIX "-" TYPE_M68K_CPU
 #define M68K_CPU_TYPE_NAME(model) model M68K_CPU_TYPE_SUFFIX
 #define CPU_RESOLVING_TYPE TYPE_M68K_CPU
diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index 55f63f7b24..5be71bc320 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -343,7 +343,6 @@ int cpu_mb_signal_handler(int host_signum, void *pinfo,
 #define TARGET_PHYS_ADDR_SPACE_BITS 32
 #define TARGET_VIRT_ADDR_SPACE_BITS 32
 
-#define cpu_init(cpu_model) cpu_generic_init(TYPE_MICROBLAZE_CPU, cpu_model)
 #define CPU_RESOLVING_TYPE TYPE_MICROBLAZE_CPU
 
 #define cpu_signal_handler cpu_mb_signal_handler
diff --git a/target/mips/cpu.h b/target/mips/cpu.h

[Qemu-devel] [PULL v2 7/7] cpu: drop unnecessary NULL check and cpu_common_class_by_name()

[Eduardo Habkost]

From: Igor Mammedov 

both do nothing as for the first all callers
   parse_cpu_model() and qmp_query_cpu_model_()
should provide non NULL value, so just abort if it's not so.

While at it drop cpu_common_class_by_name() which is not need
any more as every target has CPUClass::class_by_name callback
by now, though abort in case a new arch will forget to define one.

Signed-off-by: Igor Mammedov 
Message-Id: <1518013857-4372-1-git-send-email-imamm...@redhat.com>
Reviewed-by: Eduardo Habkost 
Signed-off-by: Eduardo Habkost 
---
 qom/cpu.c | 14 ++
 target/i386/cpu.c |  8 +---
 2 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/qom/cpu.c b/qom/cpu.c
index 60292dfde9..92599f3541 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -286,21 +286,12 @@ static bool cpu_common_has_work(CPUState *cs)
 
 ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
 {
-CPUClass *cc;
-
-if (!cpu_model) {
-return NULL;
-}
-cc = CPU_CLASS(object_class_by_name(typename));
+CPUClass *cc = CPU_CLASS(object_class_by_name(typename));
 
+assert(cpu_model && cc->class_by_name);
 return cc->class_by_name(cpu_model);
 }
 
-static ObjectClass *cpu_common_class_by_name(const char *cpu_model)
-{
-return NULL;
-}
-
 static void cpu_common_parse_features(const char *typename, char *features,
   Error **errp)
 {
@@ -418,7 +409,6 @@ static void cpu_class_init(ObjectClass *klass, void *data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 CPUClass *k = CPU_CLASS(klass);
 
-k->class_by_name = cpu_common_class_by_name;
 k->parse_features = cpu_common_parse_features;
 k->reset = cpu_common_reset;
 k->get_arch_id = cpu_common_get_arch_id;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ec1efd3a3c..4ae1631fc7 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -780,13 +780,7 @@ static char *x86_cpu_type_name(const char *model_name)
 static ObjectClass *x86_cpu_class_by_name(const char *cpu_model)
 {
 ObjectClass *oc;
-char *typename;
-
-if (cpu_model == NULL) {
-return NULL;
-}
-
-typename = x86_cpu_type_name(cpu_model);
+char *typename = x86_cpu_type_name(cpu_model);
 oc = object_class_by_name(typename);
 g_free(typename);
 return oc;
-- 
2.14.3

[Qemu-devel] [PULL v2 5/7] Use cpu_create(type) instead of cpu_init(cpu_model)

[Eduardo Habkost]

From: Igor Mammedov 

With all targets defining CPU_RESOLVING_TYPE, refactor
cpu_parse_cpu_model(type, cpu_model) to parse_cpu_model(cpu_model)
so that callers won't have to know internal resolving cpu
type. Place it in exec.c so it could be called from both
target independed vl.c and *-user/main.c.

That allows us to stop abusing cpu type from
  MachineClass::default_cpu_type
as resolver class in vl.c which were confusing part of
cpu_parse_cpu_model().

Also with new parse_cpu_model(), the last users of cpu_init()
in null-machine.c and bsd/linux-user targets could be switched
to cpu_create() API and cpu_init() API will be removed by
follow up patch.

With no longer users left remove MachineState::cpu_model field,
new code should use MachineState::cpu_type instead and
leave cpu_model parsing to generic code in vl.c.

Signed-off-by: Igor Mammedov 
Reviewed-by: Eduardo Habkost 
Message-Id: <151827-274608-5-git-send-email-imamm...@redhat.com>
[ehabkost: Fix bsd-user build error]
Signed-off-by: Eduardo Habkost 
---
 include/hw/boards.h|  1 -
 include/qom/cpu.h  | 16 ++--
 bsd-user/main.c|  4 +++-
 exec.c | 23 +++
 hw/core/null-machine.c |  6 +++---
 linux-user/main.c  |  8 ++--
 qom/cpu.c  | 47 ---
 vl.c   | 10 +++---
 8 files changed, 44 insertions(+), 71 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index efb0a9edfd..16b473a4f4 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -251,7 +251,6 @@ struct MachineState {
 char *kernel_filename;
 char *kernel_cmdline;
 char *initrd_filename;
-const char *cpu_model;
 const char *cpu_type;
 AccelState *accelerator;
 CPUArchIdList *possible_cpus;
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index dc6d4956a8..14e45c4282 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -662,8 +662,7 @@ ObjectClass *cpu_class_by_name(const char *typename, const 
char *cpu_model);
 CPUState *cpu_create(const char *typename);
 
 /**
- * cpu_parse_cpu_model:
- * @typename: The CPU base type or CPU type.
+ * parse_cpu_model:
  * @cpu_model: The model string including optional parameters.
  *
  * processes optional parameters and registers them as global properties
@@ -671,18 +670,7 @@ CPUState *cpu_create(const char *typename);
  * Returns: type of CPU to create or prints error and terminates process
  *  if an error occurred.
  */
-const char *cpu_parse_cpu_model(const char *typename, const char *cpu_model);
-
-/**
- * cpu_generic_init:
- * @typename: The CPU base type.
- * @cpu_model: The model string including optional parameters.
- *
- * Instantiates a CPU, processes optional parameters and realizes the CPU.
- *
- * Returns: A #CPUState or %NULL if an error occurred.
- */
-CPUState *cpu_generic_init(const char *typename, const char *cpu_model);
+const char *parse_cpu_model(const char *cpu_model);
 
 /**
  * cpu_has_work:
diff --git a/bsd-user/main.c b/bsd-user/main.c
index efef5ff8c5..984d0ff3bd 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -723,6 +723,7 @@ int main(int argc, char **argv)
 {
 const char *filename;
 const char *cpu_model;
+const char *cpu_type;
 const char *log_file = NULL;
 const char *log_mask = NULL;
 struct target_pt_regs regs1, *regs = 
@@ -900,7 +901,8 @@ int main(int argc, char **argv)
 tcg_exec_init(0);
 /* NOTE: we need to init the CPU at this stage to get
qemu_host_page_size */
-cpu = cpu_init(cpu_model);
+cpu_type = parse_cpu_model(cpu_model);
+cpu = cpu_create(cpu_type);
 env = cpu->env_ptr;
 #if defined(TARGET_SPARC) || defined(TARGET_PPC)
 cpu_reset(cpu);
diff --git a/exec.c b/exec.c
index a9181e6417..bc643fc50f 100644
--- a/exec.c
+++ b/exec.c
@@ -817,6 +817,29 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
 #endif
 }
 
+const char *parse_cpu_model(const char *cpu_model)
+{
+ObjectClass *oc;
+CPUClass *cc;
+gchar **model_pieces;
+const char *cpu_type;
+
+model_pieces = g_strsplit(cpu_model, ",", 2);
+
+oc = cpu_class_by_name(CPU_RESOLVING_TYPE, model_pieces[0]);
+if (oc == NULL) {
+error_report("unable to find CPU model '%s'", model_pieces[0]);
+g_strfreev(model_pieces);
+exit(EXIT_FAILURE);
+}
+
+cpu_type = object_class_get_name(oc);
+cc = CPU_CLASS(oc);
+cc->parse_features(cpu_type, model_pieces[1], _fatal);
+g_strfreev(model_pieces);
+return cpu_type;
+}
+
 #if defined(CONFIG_USER_ONLY)
 static void breakpoint_invalidate(CPUState *cpu, target_ulong pc)
 {
diff --git a/hw/core/null-machine.c b/hw/core/null-machine.c
index 864832db34..cde4d3eb57 100644
--- a/hw/core/null-machine.c
+++ b/hw/core/null-machine.c
@@ -24,9 +24,9 @@ static void machine_none_init(MachineState *mch)
 {
 

[Qemu-devel] [PULL v2 2/7] nios2: 10m50_devboard: replace cpu_model with cpu_type

[Eduardo Habkost]

From: Igor Mammedov 

use cpu_create() instead of being removed cpu_generic_init()

Signed-off-by: Igor Mammedov 
Reviewed-by: Eduardo Habkost 
Message-Id: <151827-274608-2-git-send-email-imamm...@redhat.com>
Signed-off-by: Eduardo Habkost 
---
 hw/nios2/10m50_devboard.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/nios2/10m50_devboard.c b/hw/nios2/10m50_devboard.c
index e4007f6d7f..42053b2ca9 100644
--- a/hw/nios2/10m50_devboard.c
+++ b/hw/nios2/10m50_devboard.c
@@ -75,7 +75,7 @@ static void nios2_10m50_ghrd_init(MachineState *machine)
 phys_ram_alias);
 
 /* Create CPU -- FIXME */
-cpu = NIOS2_CPU(cpu_generic_init(TYPE_NIOS2_CPU, "nios2"));
+cpu = NIOS2_CPU(cpu_create(TYPE_NIOS2_CPU));
 
 /* Register: CPU interrupt controller (PIC) */
 cpu_irq = nios2_cpu_pic_init(cpu);
-- 
2.14.3

[Qemu-devel] [PULL v2 4/7] cpu: add CPU_RESOLVING_TYPE macro

[Eduardo Habkost]

From: Igor Mammedov 

it will be used for providing to cpu name resolving class for
parsing cpu model for system and user emulation code.

Along with change add target to null-machine tests, so
that when switch to CPU_RESOLVING_TYPE happens,
it would ensure that null-machine usecase still works.

Signed-off-by: Igor Mammedov 
Reviewed-by: Laurent Vivier  (m68k)
Acked-by: David Gibson  (ppc)
Acked-by: Bastian Koppelmann  (tricore)
Message-Id: <151827-274608-4-git-send-email-imamm...@redhat.com>
Reviewed-by: Eduardo Habkost 
[ehabkost: Added macro to riscv too]
Signed-off-by: Eduardo Habkost 
---
 target/alpha/cpu.h  | 1 +
 target/arm/cpu.h| 1 +
 target/cris/cpu.h   | 1 +
 target/hppa/cpu.h   | 1 +
 target/i386/cpu.h   | 1 +
 target/lm32/cpu.h   | 1 +
 target/m68k/cpu.h   | 1 +
 target/microblaze/cpu.h | 1 +
 target/mips/cpu.h   | 1 +
 target/moxie/cpu.h  | 1 +
 target/nios2/cpu.h  | 1 +
 target/openrisc/cpu.h   | 1 +
 target/ppc/cpu.h| 1 +
 target/riscv/cpu.h  | 1 +
 target/s390x/cpu.h  | 1 +
 target/sh4/cpu.h| 1 +
 target/sparc/cpu.h  | 1 +
 target/tilegx/cpu.h | 1 +
 target/tricore/cpu.h| 1 +
 target/unicore32/cpu.h  | 1 +
 target/xtensa/cpu.h | 1 +
 21 files changed, 21 insertions(+)

diff --git a/target/alpha/cpu.h b/target/alpha/cpu.h
index a79fc2e780..905855a2a1 100644
--- a/target/alpha/cpu.h
+++ b/target/alpha/cpu.h
@@ -470,6 +470,7 @@ void alpha_translate_init(void);
 
 #define ALPHA_CPU_TYPE_SUFFIX "-" TYPE_ALPHA_CPU
 #define ALPHA_CPU_TYPE_NAME(model) model ALPHA_CPU_TYPE_SUFFIX
+#define CPU_RESOLVING_TYPE TYPE_ALPHA_CPU
 
 void alpha_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 /* you can call this signal handler from your SIGBUS and SIGSEGV
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 1e7e1f8a7e..f4b4258655 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2306,6 +2306,7 @@ static inline bool arm_excp_unmasked(CPUState *cs, 
unsigned int excp_idx,
 
 #define ARM_CPU_TYPE_SUFFIX "-" TYPE_ARM_CPU
 #define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX)
+#define CPU_RESOLVING_TYPE TYPE_ARM_CPU
 
 #define cpu_signal_handler cpu_arm_signal_handler
 #define cpu_list arm_cpu_list
diff --git a/target/cris/cpu.h b/target/cris/cpu.h
index 764b35cbae..cfb877c488 100644
--- a/target/cris/cpu.h
+++ b/target/cris/cpu.h
@@ -271,6 +271,7 @@ enum {
 
 #define CRIS_CPU_TYPE_SUFFIX "-" TYPE_CRIS_CPU
 #define CRIS_CPU_TYPE_NAME(name) (name CRIS_CPU_TYPE_SUFFIX)
+#define CPU_RESOLVING_TYPE TYPE_CRIS_CPU
 
 #define cpu_signal_handler cpu_cris_signal_handler
 
diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index c88d844938..6ea5f4c4e4 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -267,6 +267,7 @@ static inline int cpu_mmu_index(CPUHPPAState *env, bool 
ifetch)
 void hppa_translate_init(void);
 
 #define cpu_init(cpu_model) cpu_generic_init(TYPE_HPPA_CPU, cpu_model)
+#define CPU_RESOLVING_TYPE TYPE_HPPA_CPU
 
 void hppa_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 0c3f51445e..a0299289ae 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1590,6 +1590,7 @@ uint64_t cpu_get_tsc(CPUX86State *env);
 
 #define X86_CPU_TYPE_SUFFIX "-" TYPE_X86_CPU
 #define X86_CPU_TYPE_NAME(name) (name X86_CPU_TYPE_SUFFIX)
+#define CPU_RESOLVING_TYPE TYPE_X86_CPU
 
 #ifdef TARGET_X86_64
 #define TARGET_DEFAULT_CPU_TYPE X86_CPU_TYPE_NAME("qemu64")
diff --git a/target/lm32/cpu.h b/target/lm32/cpu.h
index ce0a2f24c4..6f41955528 100644
--- a/target/lm32/cpu.h
+++ b/target/lm32/cpu.h
@@ -259,6 +259,7 @@ bool lm32_cpu_do_semihosting(CPUState *cs);
 
 #define LM32_CPU_TYPE_SUFFIX "-" TYPE_LM32_CPU
 #define LM32_CPU_TYPE_NAME(model) model LM32_CPU_TYPE_SUFFIX
+#define CPU_RESOLVING_TYPE TYPE_LM32_CPU
 
 #define cpu_list lm32_cpu_list
 #define cpu_signal_handler cpu_lm32_signal_handler
diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index 2259bf22dc..f102e7216d 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -531,6 +531,7 @@ enum {
 
 #define M68K_CPU_TYPE_SUFFIX "-" TYPE_M68K_CPU
 #define M68K_CPU_TYPE_NAME(model) model M68K_CPU_TYPE_SUFFIX
+#define CPU_RESOLVING_TYPE TYPE_M68K_CPU
 
 #define cpu_signal_handler cpu_m68k_signal_handler
 #define cpu_list m68k_cpu_list
diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index 1fe21c8539..55f63f7b24 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -344,6 +344,7 @@ int cpu_mb_signal_handler(int host_signum, void *pinfo,
 #define TARGET_VIRT_ADDR_SPACE_BITS 32
 
 #define cpu_init(cpu_model) cpu_generic_init(TYPE_MICROBLAZE_CPU, cpu_model)
+#define CPU_RESOLVING_TYPE TYPE_MICROBLAZE_CPU
 
 #define cpu_signal_handler cpu_mb_signal_handler
 
diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 

[Qemu-devel] [PULL v2 3/7] tests: add machine 'none' with -cpu test

[Eduardo Habkost]

From: Igor Mammedov 

Check that "$QEMU -M none -cpu FOO" starts QEMU without error

Signed-off-by: Igor Mammedov 
Message-Id: <151827-274608-3-git-send-email-imamm...@redhat.com>
[ehabkost: include qmp/qdict.h instead of qmp/types.h]
Signed-off-by: Eduardo Habkost 
---
 tests/machine-none-test.c | 101 ++
 tests/Makefile.include|   2 +
 2 files changed, 103 insertions(+)
 create mode 100644 tests/machine-none-test.c

diff --git a/tests/machine-none-test.c b/tests/machine-none-test.c
new file mode 100644
index 00..596ad0153b
--- /dev/null
+++ b/tests/machine-none-test.c
@@ -0,0 +1,101 @@
+/*
+ * Machine 'none' tests.
+ *
+ * Copyright (c) 2018 Red Hat Inc.
+ *
+ * Authors:
+ *  Igor Mammedov ,
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+
+#include "qemu-common.h"
+#include "qemu/cutils.h"
+#include "libqtest.h"
+#include "qapi/qmp/qdict.h"
+
+
+struct arch2cpu {
+const char *arch;
+const char *cpu_model;
+};
+
+static struct arch2cpu cpus_map[] = {
+/* tested targets list */
+{ "arm", "cortex-a15" },
+{ "aarch64", "cortex-a57" },
+{ "x86_64", "qemu64,apic-id=0" },
+{ "i386", "qemu32,apic-id=0" },
+{ "alpha", "ev67" },
+{ "cris", "crisv32" },
+{ "lm32", "lm32-full" },
+{ "m68k", "m5206" },
+/* FIXME: { "microblaze", "any" }, doesn't work with -M none -cpu any */
+/* FIXME: { "microblazeel", "any" }, doesn't work with -M none -cpu any */
+{ "mips", "4Kc" },
+{ "mipsel", "4Kc" },
+{ "mips64", "20Kc" },
+{ "mips64el", "20Kc" },
+{ "moxie", "MoxieLite" },
+{ "nios2", "FIXME" },
+{ "or1k", "or1200" },
+{ "ppc", "604" },
+{ "ppc64", "power8e_v2.1" },
+{ "ppcemb", "440epb" },
+{ "s390x", "qemu" },
+{ "sh4", "sh7750r" },
+{ "sh4eb", "sh7751r" },
+{ "sparc", "LEON2" },
+{ "sparc64", "Fujitsu Sparc64" },
+{ "tricore", "tc1796" },
+{ "unicore32", "UniCore-II" },
+{ "xtensa", "dc233c" },
+{ "xtensaeb", "fsf" },
+{ "hppa", "hppa" },
+};
+
+static const char *get_cpu_model_by_arch(const char *arch)
+{
+int i;
+
+for (i = 0; i < ARRAY_SIZE(cpus_map); i++) {
+if (!strcmp(arch, cpus_map[i].arch)) {
+return cpus_map[i].cpu_model;
+}
+}
+return NULL;
+}
+
+static void test_machine_cpu_cli(void)
+{
+QDict *response;
+const char *arch = qtest_get_arch();
+const char *cpu_model = get_cpu_model_by_arch(arch);
+
+if (!cpu_model) {
+if (!(!strcmp(arch, "microblaze") || !strcmp(arch, "microblazeel"))) {
+fprintf(stderr, "WARNING: cpu name for target '%s' isn't defined,"
+" add it to cpus_map\n", arch);
+}
+return; /* TODO: die here to force all targets have a test */
+}
+global_qtest = qtest_startf("-machine none -cpu '%s'", cpu_model);
+
+response = qmp("{ 'execute': 'quit' }");
+g_assert(qdict_haskey(response, "return"));
+QDECREF(response);
+
+qtest_quit(global_qtest);
+}
+
+int main(int argc, char **argv)
+{
+g_test_init(, , NULL);
+
+qtest_add_func("machine/none/cpu_option", test_machine_cpu_cli);
+
+return g_test_run();
+}
diff --git a/tests/Makefile.include b/tests/Makefile.include
index ef9b88c369..b0e3785e0f 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -399,6 +399,7 @@ check-qtest-s390x-y += tests/virtio-console-test$(EXESUF)
 check-qtest-s390x-y += tests/virtio-serial-test$(EXESUF)
 check-qtest-s390x-y += tests/cpu-plug-test$(EXESUF)
 
+check-qtest-generic-y += tests/machine-none-test$(EXESUF)
 check-qtest-generic-y += tests/qom-test$(EXESUF)
 check-qtest-generic-y += tests/test-hmp$(EXESUF)
 
@@ -794,6 +795,7 @@ tests/display-vga-test$(EXESUF): tests/display-vga-test.o
 tests/ipoctal232-test$(EXESUF): tests/ipoctal232-test.o
 tests/qom-test$(EXESUF): tests/qom-test.o
 tests/test-hmp$(EXESUF): tests/test-hmp.o
+tests/machine-none-test$(EXESUF): tests/machine-none-test.o
 tests/drive_del-test$(EXESUF): tests/drive_del-test.o $(libqos-virtio-obj-y)
 tests/qdev-monitor-test$(EXESUF): tests/qdev-monitor-test.o $(libqos-pc-obj-y)
 tests/nvme-test$(EXESUF): tests/nvme-test.o
-- 
2.14.3

[Qemu-devel] [PULL v2 1/7] pc: correct misspelled CPU model-id for pc 2.2

[Eduardo Habkost]

From: Wang Xin 

Signed-off-by: Wang Xin 
Message-Id: <1517367668-25048-1-git-send-email-wangxinxin.w...@huawei.com>
Acked-by: Michael S. Tsirkin 
Signed-off-by: Eduardo Habkost 
---
 include/hw/i386/pc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index bb49165fe0..635c8b2a3b 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -617,7 +617,7 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 
 #define PC_COMPAT_2_2 \
 HW_COMPAT_2_2 \
-PC_CPU_MODEL_IDS("2.3.0") \
+PC_CPU_MODEL_IDS("2.2.0") \
 {\
 .driver = "kvm64" "-" TYPE_X86_CPU,\
 .property = "vme",\
-- 
2.14.3

[Qemu-devel] [PULL v2 0/7] Machine queue, 2018-03-15

[Eduardo Habkost]

Changes in v2 (v1 was 2018-03-12):
* Fix bsd-user build error

The following changes since commit 56e8698ffa8aba9f762f980bc21b5340b006f24b:

  Merge remote-tracking branch 
'remotes/stsquad/tags/pull-travis-speedup-130318-1' into staging (2018-03-15 
14:48:09 +)

are available in the Git repository at:

  git://github.com/ehabkost/qemu.git tags/machine-next-pull-request

for you to fetch changes up to 7cbb6fd8926b9b590c0725b9b7d0a11db6aefd08:

  cpu: drop unnecessary NULL check and cpu_common_class_by_name() (2018-03-15 
14:52:40 -0300)


Machine queue, 2018-03-15



Queue for Machine Core patches


Igor Mammedov (6):
  nios2: 10m50_devboard: replace cpu_model with cpu_type
  tests: add machine 'none' with -cpu test
  cpu: add CPU_RESOLVING_TYPE macro
  Use cpu_create(type) instead of cpu_init(cpu_model)
  cpu: get rid of unused cpu_init() defines
  cpu: drop unnecessary NULL check and cpu_common_class_by_name()

Wang Xin (1):
  pc: correct misspelled CPU model-id for pc 2.2

 include/hw/boards.h   |   1 -
 include/hw/i386/pc.h  |   2 +-
 include/qom/cpu.h |  16 +---
 target/alpha/cpu.h|   3 +-
 target/arm/cpu.h  |   3 +-
 target/cris/cpu.h |   3 +-
 target/hppa/cpu.h |   2 +-
 target/i386/cpu.h |   3 +-
 target/lm32/cpu.h |   3 +-
 target/m68k/cpu.h |   3 +-
 target/microblaze/cpu.h   |   2 +-
 target/mips/cpu.h |   3 +-
 target/moxie/cpu.h|   3 +-
 target/nios2/cpu.h|   2 +-
 target/openrisc/cpu.h |   3 +-
 target/ppc/cpu.h  |   3 +-
 target/riscv/cpu.h|   1 +
 target/s390x/cpu.h|   3 +-
 target/sh4/cpu.h  |   3 +-
 target/sparc/cpu.h|   5 +--
 target/tilegx/cpu.h   |   2 +-
 target/tricore/cpu.h  |   3 +-
 target/unicore32/cpu.h|   3 +-
 target/xtensa/cpu.h   |   3 +-
 bsd-user/main.c   |   4 +-
 exec.c|  23 +++
 hw/core/null-machine.c|   6 +--
 hw/nios2/10m50_devboard.c |   2 +-
 linux-user/main.c |   8 +++-
 qom/cpu.c |  61 +++-
 target/i386/cpu.c |   8 +---
 tests/machine-none-test.c | 101 ++
 vl.c  |  10 ++---
 tests/Makefile.include|   2 +
 34 files changed, 173 insertions(+), 130 deletions(-)
 create mode 100644 tests/machine-none-test.c

-- 
2.14.3

[Qemu-devel] [Bug 1749393] Re: sbrk() not working under qemu-user with a PIE-compiled binary?

[Peter Ogden]

Could we over-allocate the data segment by
QEMU_DATA_SIZE/getrlimit(RLIMIT_DATA)/128 MB depending on what's set -
similar to how the stack size is managed?

My current workaround for aarch64 on x86-64 is to mmap a dynamic main
executable in some far-away part of the address space but I'm not sure
how to find somewhere suitable on a 32-bit host/guest.

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

Title:
  sbrk() not working under qemu-user with a PIE-compiled binary?

Status in QEMU:
  New
Status in qemu package in Ubuntu:
  New

Bug description:
  In Debian unstable, we recently switched bash to be a PIE-compiled
  binary (for hardening). Unfortunately this resulted in bash being
  broken when run under qemu-user (for all target architectures, host
  being amd64 for me).

  $ sudo chroot /srv/chroots/sid-i386/ qemu-i386-static /bin/bash
  bash: xmalloc: .././shell.c:1709: cannot allocate 10 bytes (0 bytes allocated)

  bash has its own malloc implementation based on sbrk():
  https://git.savannah.gnu.org/cgit/bash.git/tree/lib/malloc/malloc.c

  When we disable this internal implementation and rely on glibc's
  malloc, then everything is fine. But it might be that glibc has a
  fallback when sbrk() is not working properly and it might hide the
  underlying problem in qemu-user.

  This issue has also been reported to the bash upstream author and he 
suggested that the issue might be in qemu-user so I'm opening a ticket here. 
Here's the discussion with the bash upstream author:
  https://lists.gnu.org/archive/html/bug-bash/2018-02/threads.html#00080

  You can find the problematic bash binary in that .deb file:
  
http://snapshot.debian.org/archive/debian/20180206T154716Z/pool/main/b/bash/bash_4.4.18-1_i386.deb

  The version of qemu I have been using is 2.11 (Debian package qemu-
  user-static version 1:2.11+dfsg-1) but I have had reports that the
  problem is reproducible with older versions (back to 2.8 at least).

  Here are the related Debian bug reports:
  https://bugs.debian.org/889869
  https://bugs.debian.org/865599

  It's worth noting that bash used to have this problem (when compiled as a PIE 
binary) even when run directly but then something got fixed in the kernel and 
now the problem only appears when run under qemu-user:
  https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1518483

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

Re: [Qemu-devel] [PULL v2 00/22] KVM patches for QEMU 2.12 soft freeze

[Peter Maydell]

On 13 March 2018 at 16:41, Paolo Bonzini  wrote:
> The following changes since commit fb5fff15881ba7a002924b967eb211c002897983:
>
>   Merge remote-tracking branch 
> 'remotes/kraxel/tags/vga-20180312-pull-request' into staging (2018-03-12 
> 18:35:37 +)
>
> are available in the git repository at:
>
>
>   git://github.com/bonzini/qemu.git tags/for-upstream-sev
>
> for you to fetch changes up to 9f750794985d7386f088da941c76b73880b2b6c4:
>
>   sev/i386: add sev_get_capabilities() (2018-03-13 12:04:04 +0100)
>
> 
> * Migrate MSR_SMI_COUNT (Liran)
> * Update kernel headers (Gerd, myself)
> * SEV support (Brijesh)
>
> I have not tested non-x86 compilation, but I reordered the SEV patches
> so that all non-x86-specific changes go first to catch any possible
> issues (which weren't there anyway :)).
>
> 

Applied, thanks. The issues were indeed not there :-)

-- PMM

Re: [Qemu-devel] [PULL 00/41] Block layer patches

[John Snow]

On 03/15/2018 12:56 PM, Kevin Wolf wrote:
> Am 15.03.2018 um 17:42 hat Peter Maydell geschrieben:
>> On 13 March 2018 at 16:17, Kevin Wolf  wrote:
>>> The following changes since commit 22ef7ba8e8ce7fef297549b3defcac333742b804:
>>>
>>>   Merge remote-tracking branch 'remotes/famz/tags/staging-pull-request' 
>>> into staging (2018-03-13 11:42:45 +)
>>>
>>> are available in the git repository at:
>>>
>>>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>>>
>>> for you to fetch changes up to be6c885842efded81a20f4ca24f0d4e123a80c00:
>>>
>>>   block/mirror: change the semantic of 'force' of block-job-cancel 
>>> (2018-03-13 16:54:47 +0100)
>>>
>>> 
>>> Block layer patches
>>>
>>> 
>>
>> I get a compile failure here on some hosts:
>>
>> /home/pm215/qemu/blockjob.c: In function ‘block_job_txn_apply.isra.8’:
>> /home/pm215/qemu/blockjob.c:524:5: error: ‘rc’ may be used
>> uninitialized in this function [-Werror=maybe-uninitialized]
>>  return rc;
>>  ^
>>
>> I guess the compiler can't always figure out whether there is
>> guaranteed to be at least one thing in the list ?
> 
> I think so.
> 
> John, I'll just modify your patch 'blockjobs: add prepare callback' to
> initialise rc = 0 in this function and send a v2 pull request.
> 
> Kevin
> 

Oh, interesting. I guess technically the list COULD be empty and that'd
be perfectly valid. I wonder why my compiler doesn't complain?

Anyway, initializing to zero seems like the correct behavior, thanks.

--js

[Qemu-devel] [PULL v2 0/3] tcg queued patches

[Richard Henderson]

Ho hum, I rushed v1 before leaving on holidays and failed to format
the cover letter properly, so Peter's scripts did not pick it up.

Patches 2 & 3 are real bug fixes and so still appropriate for the
softfreeze.  Patch 1 is trivial enough I didn't feel it needed to
be held back for 2.13 development.  Let me know if you disagree.


r~


The following changes since commit 56e8698ffa8aba9f762f980bc21b5340b006f24b:

  Merge remote-tracking branch 
'remotes/stsquad/tags/pull-travis-speedup-130318-1' into staging (2018-03-15 
14:48:09 +)

are available in the Git repository at:

  git://github.com/rth7680/qemu.git tags/pull-tcg-20180316

for you to fetch changes up to adb196cbd5cff26547bc32a208074f03f4c4a627:

  tcg: Add choose_vector_size (2018-03-16 00:55:04 +0800)


Queued TCG patches


Richard Henderson (3):
  tcg: Improve tcg_gen_muli_i32/i64
  tcg/i386: Support INDEX_op_dup2_vec for -m32
  tcg: Add choose_vector_size

 tcg/i386/tcg-target.inc.c |   9 +
 tcg/tcg-op-gvec.c | 438 +++---
 tcg/tcg-op.c  |  24 ++-
 3 files changed, 286 insertions(+), 185 deletions(-)

[Qemu-devel] [PULL v2 2/3] tcg/i386: Support INDEX_op_dup2_vec for -m32

[Richard Henderson]

Unknown why -m32 was passing with gcc but not clang; it should have
failed for both.  This would be used for tcg_gen_dup_i64_vec, and
visible with the right TB and an aarch64 guest.

Reported-by: Max Reitz 
Signed-off-by: Richard Henderson 
---
 tcg/i386/tcg-target.inc.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/tcg/i386/tcg-target.inc.c b/tcg/i386/tcg-target.inc.c
index fc05909d1d..d7e59e79c5 100644
--- a/tcg/i386/tcg-target.inc.c
+++ b/tcg/i386/tcg-target.inc.c
@@ -2696,6 +2696,12 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
 case INDEX_op_x86_packus_vec:
 insn = packus_insn[vece];
 goto gen_simd;
+#if TCG_TARGET_REG_BITS == 32
+case INDEX_op_dup2_vec:
+/* Constraints have already placed both 32-bit inputs in xmm regs.  */
+insn = OPC_PUNPCKLDQ;
+goto gen_simd;
+#endif
 gen_simd:
 tcg_debug_assert(insn != OPC_UD2);
 if (type == TCG_TYPE_V256) {
@@ -3045,6 +3051,9 @@ static const TCGTargetOpDef *tcg_target_op_def(TCGOpcode 
op)
 case INDEX_op_x86_vperm2i128_vec:
 case INDEX_op_x86_punpckl_vec:
 case INDEX_op_x86_punpckh_vec:
+#if TCG_TARGET_REG_BITS == 32
+case INDEX_op_dup2_vec:
+#endif
 return _x_x;
 case INDEX_op_dup_vec:
 case INDEX_op_shli_vec:
-- 
2.14.3

[Qemu-devel] [PULL v2 3/3] tcg: Add choose_vector_size

[Richard Henderson]

This unifies 5 copies of checks for supported vector size,
and in the process fixes a missing check in tcg_gen_gvec_2s.

This lead to an assertion failure for 64-bit vector multiply,
which is not available in the AVX instruction set.

Suggested-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 tcg/tcg-op-gvec.c | 438 --
 1 file changed, 259 insertions(+), 179 deletions(-)

diff --git a/tcg/tcg-op-gvec.c b/tcg/tcg-op-gvec.c
index bfe44bba81..22db1590d5 100644
--- a/tcg/tcg-op-gvec.c
+++ b/tcg/tcg-op-gvec.c
@@ -351,6 +351,42 @@ static void gen_dup_i64(unsigned vece, TCGv_i64 out, 
TCGv_i64 in)
 }
 }
 
+/* Select a supported vector type for implementing an operation on SIZE
+ * bytes.  If OP is 0, assume that the real operation to be performed is
+ * required by all backends.  Otherwise, make sure than OP can be performed
+ * on elements of size VECE in the selected type.  Do not select V64 if
+ * PREFER_I64 is true.  Return 0 if no vector type is selected.
+ */
+static TCGType choose_vector_type(TCGOpcode op, unsigned vece, uint32_t size,
+  bool prefer_i64)
+{
+if (TCG_TARGET_HAS_v256 && check_size_impl(size, 32)) {
+if (op == 0) {
+return TCG_TYPE_V256;
+}
+/* Recall that ARM SVE allows vector sizes that are not a
+ * power of 2, but always a multiple of 16.  The intent is
+ * that e.g. size == 80 would be expanded with 2x32 + 1x16.
+ * It is hard to imagine a case in which v256 is supported
+ * but v128 is not, but check anyway.
+ */
+if (tcg_can_emit_vec_op(op, TCG_TYPE_V256, vece)
+&& (size % 32 == 0
+|| tcg_can_emit_vec_op(op, TCG_TYPE_V128, vece))) {
+return TCG_TYPE_V256;
+}
+}
+if (TCG_TARGET_HAS_v128 && check_size_impl(size, 16)
+&& (op == 0 || tcg_can_emit_vec_op(op, TCG_TYPE_V128, vece))) {
+return TCG_TYPE_V128;
+}
+if (TCG_TARGET_HAS_v64 && !prefer_i64 && check_size_impl(size, 8)
+&& (op == 0 || tcg_can_emit_vec_op(op, TCG_TYPE_V64, vece))) {
+return TCG_TYPE_V64;
+}
+return 0;
+}
+
 /* Set OPRSZ bytes at DOFS to replications of IN_32, IN_64 or IN_C.
  * Only one of IN_32 or IN_64 may be set;
  * IN_C is used if IN_32 and IN_64 are unset.
@@ -376,19 +412,12 @@ static void do_dup(unsigned vece, uint32_t dofs, uint32_t 
oprsz,
 }
 }
 
-type = 0;
-if (TCG_TARGET_HAS_v256 && check_size_impl(oprsz, 32)) {
-type = TCG_TYPE_V256;
-} else if (TCG_TARGET_HAS_v128 && check_size_impl(oprsz, 16)) {
-type = TCG_TYPE_V128;
-} else if (TCG_TARGET_HAS_v64 && check_size_impl(oprsz, 8)
-   /* Prefer integer when 64-bit host and no variable dup.  */
-   && !(TCG_TARGET_REG_BITS == 64 && in_32 == NULL
-&& (in_64 == NULL || vece == MO_64))) {
-type = TCG_TYPE_V64;
-}
-
-/* Implement inline with a vector type, if possible.  */
+/* Implement inline with a vector type, if possible.
+ * Prefer integer when 64-bit host and no variable dup.
+ */
+type = choose_vector_type(0, vece, oprsz,
+  (TCG_TARGET_REG_BITS == 64 && in_32 == NULL
+   && (in_64 == NULL || vece == MO_64)));
 if (type != 0) {
 TCGv_vec t_vec = tcg_temp_new_vec(type);
 
@@ -414,21 +443,30 @@ static void do_dup(unsigned vece, uint32_t dofs, uint32_t 
oprsz,
 }
 
 i = 0;
-if (TCG_TARGET_HAS_v256) {
+switch (type) {
+case TCG_TYPE_V256:
+/* Recall that ARM SVE allows vector sizes that are not a
+ * power of 2, but always a multiple of 16.  The intent is
+ * that e.g. size == 80 would be expanded with 2x32 + 1x16.
+ */
 for (; i + 32 <= oprsz; i += 32) {
 tcg_gen_stl_vec(t_vec, cpu_env, dofs + i, TCG_TYPE_V256);
 }
-}
-if (TCG_TARGET_HAS_v128) {
+/* fallthru */
+case TCG_TYPE_V128:
 for (; i + 16 <= oprsz; i += 16) {
 tcg_gen_stl_vec(t_vec, cpu_env, dofs + i, TCG_TYPE_V128);
 }
-}
-if (TCG_TARGET_HAS_v64) {
+break;
+case TCG_TYPE_V64:
 for (; i < oprsz; i += 8) {
 tcg_gen_stl_vec(t_vec, cpu_env, dofs + i, TCG_TYPE_V64);
 }
+break;
+default:
+g_assert_not_reached();
 }
+
 tcg_temp_free_vec(t_vec);
 goto done;
 }
@@ -484,7 +522,7 @@ static void do_dup(unsigned vece, uint32_t dofs, uint32_t 
oprsz,
 }
 tcg_temp_free_i64(t_64);
 goto done;
-} 
+}
 }
 
 /* Otherwise implement out of line.  */
@@ -866,49 +904,55 @@ static void expand_4_vec(unsigned 

[Qemu-devel] [PULL v2 1/3] tcg: Improve tcg_gen_muli_i32/i64

[Richard Henderson]

Convert multiplication by power of two to left shift.

Reviewed-by: Emilio G. Cota 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 tcg/tcg-op.c | 24 ++--
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index 3467787323..34b96d68f3 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -277,9 +277,15 @@ void tcg_gen_setcondi_i32(TCGCond cond, TCGv_i32 ret,
 
 void tcg_gen_muli_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2)
 {
-TCGv_i32 t0 = tcg_const_i32(arg2);
-tcg_gen_mul_i32(ret, arg1, t0);
-tcg_temp_free_i32(t0);
+if (arg2 == 0) {
+tcg_gen_movi_i32(ret, 0);
+} else if (is_power_of_2(arg2)) {
+tcg_gen_shli_i32(ret, arg1, ctz32(arg2));
+} else {
+TCGv_i32 t0 = tcg_const_i32(arg2);
+tcg_gen_mul_i32(ret, arg1, t0);
+tcg_temp_free_i32(t0);
+}
 }
 
 void tcg_gen_div_i32(TCGv_i32 ret, TCGv_i32 arg1, TCGv_i32 arg2)
@@ -1430,9 +1436,15 @@ void tcg_gen_setcondi_i64(TCGCond cond, TCGv_i64 ret,
 
 void tcg_gen_muli_i64(TCGv_i64 ret, TCGv_i64 arg1, int64_t arg2)
 {
-TCGv_i64 t0 = tcg_const_i64(arg2);
-tcg_gen_mul_i64(ret, arg1, t0);
-tcg_temp_free_i64(t0);
+if (arg2 == 0) {
+tcg_gen_movi_i64(ret, 0);
+} else if (is_power_of_2(arg2)) {
+tcg_gen_shli_i64(ret, arg1, ctz64(arg2));
+} else {
+TCGv_i64 t0 = tcg_const_i64(arg2);
+tcg_gen_mul_i64(ret, arg1, t0);
+tcg_temp_free_i64(t0);
+}
 }
 
 void tcg_gen_div_i64(TCGv_i64 ret, TCGv_i64 arg1, TCGv_i64 arg2)
-- 
2.14.3

[Qemu-devel] [Bug 1619896] Re: Unsupported ancillary data: 0/8

[Peter Maydell]

** Tags added: linux-user

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

Title:
  Unsupported ancillary data: 0/8

Status in QEMU:
  New

Bug description:
  Hello,

  I have the following issue when launching the Teamspeak Server x86
  binary on an arm host.

  Host:
   Linux 4.6.2 (vanilla)
   Ubuntu 14.04.5 LTS
   HW: Cubietruck board, armv7l

  
  Used SW: Release archive qemu-2.7.0.tar.bz2 and git commit 
1dc33ed90bf1fe1c2014dffa0d9e863c520d953a
  Configure options:
../configure --target-list=i386-linux-user 
  I attached the output of the configure script as configure.log

  Testcase:

  1. Download and extract TeamSpeak 3 Server 3.0.13.3 (x86)
Souce: 
http://dl.4players.de/ts/releases/3.0.13.3/teamspeak3-server_linux_x86-3.0.13.3.tar.bz2

  2. Modifiy ts3server_minimal_runscript.sh for ease of use
- ./ts3server $@
+ /usr/local/bin/qemu-i386 ./ts3server $@

  3. Execute ./ts3server_minimal_runscript.sh

  Wait for 6 Minutes until teamspeak server started. QEMU saturates the
  cpu while Teamspeak is precomputing a puzzle. (Whatever that means)

  After that Teamspeak settles with the following output:
2016-09-03 10:50:59.82|INFO|Query |   |listening on 
0.0.0.0:10011, :::10011

  The Qemu process is now idling with ~2% cpu load. This is actually the
  first time for me that QEMU is able to successfully launch the
  Teamspeak server. Kudos!

  4. Connect client 1

  TS Clients can connect, but the following line is printed pretty often:
Unsupported ancillary data: 0/8

  The line seems to come from qemu (linux-user/syscall.c)

  
  5. Connect client 2
  When a second client is connected the audio transmission is successful for a 
few seconds, but the server drops the connection after that and refuses to take 
new connections.

  Please let me know, if you need more information. I'll gladly provide
  strace or valgrind logs.

  Best regards,
  Tobias

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

[Qemu-devel] [Bug 1680991] Re: raspi2: system timer device not implemented

[Peter Maydell]

** Summary changed:

- raspi2: system timer not device implemented
+ raspi2: system timer device not implemented

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

Title:
  raspi2: system timer device not implemented

Status in QEMU:
  Confirmed

Bug description:
  In a small hobby kernel for Raspberry Pi 2B, I am using the system
  timer to control wait durations.  This timer is located at 0x3f003000
  and the timer counts are located at 0x3f003004 (CLO) and 0x3f004008
  (CHI).  Reading these memory locations returns 0 for both.

  The basic code for this function is:
  
@
  @@ uint64_t ReadSysTimerCount() -- read the system time running count
  
@
  ReadSysTimerCount:
ldr r0,=ST_CLO  @ load the base address of the 
system timer
ldrdr0,r1,[r0]  @ Get the 64-bit timer "count" into 
r1:r0
mov pc,lr   @ return

  Tracing back the definition of ST_CLO in my code:
  #define ST_CLO  (ST_BASE+4) // Counter Lower 32 
bits
  #define ST_BASE (HW_BASE+0x3000)// System Timer base 
address
  #define HW_BASE (0x3f00)// this is the base 
address for all hardware I/O addresses

  I have tested a similar program that I know to work on real hardware
  with qemu-system-arm reading the same mmio register and have the same
  issue, so I'm pretty sure the issue is not with my code.

  My Host PC is a VM on vmWare esxi running FC25 (8 cores, 8GB RAM): 
  [adam@os-dev ~]$ uname -a
  Linux os-dev.jammin 4.10.8-200.fc25.x86_64 #1 SMP Fri Mar 31 13:20:22 UTC 
2017 x86_64 x86_64 x86_64 GNU/Linux

  I have confirmed this issue on QEMU 2.7.1 (fc25 Distro) and 2.9.0-rc3
  (git).

  adam@os-dev ~]$ qemu-system-arm --version
  QEMU emulator version 2.7.1(qemu-2.7.1-4.fc25), Copyright (c) 2003-2016 
Fabrice Bellard and the QEMU Project developers

  [adam@os-dev ~]$ 
./workspace/qemu/bin/debug/native/arm-softmmu/qemu-system-arm --version
  QEMU emulator version 2.8.93 (v2.9.0-rc3-15-g5daf9b3)
  Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

  A remote debugger for my kernel shows the following:
  (gdb) info reg
  r0 0x00
  r1 0x00
  r2 0x96   150
  r3 0x00
  r4 0xa000 40960
  r5 0x00
  r6 0x00
  r7 0x00
  r8 0x00
  r9 0xa000 40960
  r100x00
  r110x7fdc 32732
  r120x00
  sp 0x7fc8 0x7fc8
  lr 0x8194 33172
  pc 0x80a4 0x80a4
  cpsr   0x81d3 -2147483181
  (gdb) stepi
  0x80a8 in ?? ()
  (gdb) info reg
  r0 0x3f003004 1056976900
  r1 0x00
  r2 0x96   150
  r3 0x00
  r4 0xa000 40960
  r5 0x00
  r6 0x00
  r7 0x00
  r8 0x00
  r9 0xa000 40960
  r100x00
  r110x7fdc 32732
  r120x00
  sp 0x7fc8 0x7fc8
  lr 0x8194 33172
  pc 0x80a8 0x80a8
  cpsr   0x81d3 -2147483181
  (gdb) stepi
  0x80ac in ?? ()
  (gdb) info reg
  r0 0x00
  r1 0x00
  r2 0x96   150
  r3 0x00
  r4 0xa000 40960
  r5 0x00
  r6 0x00
  r7 0x00
  r8 0x00
  r9 0xa000 40960
  r100x00
  r110x7fdc 32732
  r120x00
  sp 0x7fc8 0x7fc8
  lr 0x8194 33172
  pc 0x80ac 0x80ac
  cpsr   0x81d3 -2147483181

  Notice r0 is loaded with the address for CLO and then cleared with 0
  when read.

  I am writing my code against the documented specifications in "BCM2835
  ARM Peripherals" (attached for convenience), section "12 System
  Timer".

  
  Please let me know if you need anything else from me.

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

[Qemu-devel] [PULL v2 00/44] Block layer patches

[Kevin Wolf]

The following changes since commit 56e8698ffa8aba9f762f980bc21b5340b006f24b:

  Merge remote-tracking branch 
'remotes/stsquad/tags/pull-travis-speedup-130318-1' into staging (2018-03-15 
14:48:09 +)

are available in the git repository at:

  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to d3137181e5e1e5e6b651b58484fc8f705a48ee36:

  iscsi: fix iSER compilation (2018-03-15 17:58:26 +0100)


Block layer patches


Fam Zheng (4):
  block: Fix flags in reopen queue
  iotests: Add regression test for commit base locking
  vvfat: Fix inherit_options flags
  block: Fix leak of ignore_children in error path

John Snow (21):
  blockjobs: fix set-speed kick
  blockjobs: model single jobs as transactions
  Blockjobs: documentation touchup
  blockjobs: add status enum
  blockjobs: add state transition table
  iotests: add pause_wait
  blockjobs: add block_job_verb permission table
  blockjobs: add ABORTING state
  blockjobs: add CONCLUDED state
  blockjobs: add NULL state
  blockjobs: add block_job_dismiss
  blockjobs: ensure abort is called for cancelled jobs
  blockjobs: add commit, abort, clean helpers
  blockjobs: add block_job_txn_apply function
  blockjobs: add prepare callback
  blockjobs: add waiting status
  blockjobs: add PENDING status and event
  blockjobs: add block-job-finalize
  blockjobs: Expose manual property
  iotests: test manual job dismissal
  tests/test-blockjob: test cancellations

Kevin Wolf (14):
  luks: Separate image file creation from formatting
  luks: Create block_crypto_co_create_generic()
  luks: Support .bdrv_co_create
  luks: Turn invalid assertion into check
  luks: Catch integer overflow for huge sizes
  qemu-iotests: Test luks QMP image creation
  parallels: Support .bdrv_co_create
  qemu-iotests: Enable write tests for parallels
  qcow: Support .bdrv_co_create
  qed: Support .bdrv_co_create
  vdi: Make comments consistent with other drivers
  vhdx: Support .bdrv_co_create
  vpc: Support .bdrv_co_create
  vpc: Require aligned size in .bdrv_co_create

Liang Li (1):
  block/mirror: change the semantic of 'force' of block-job-cancel

Max Reitz (3):
  vdi: Pull option parsing from vdi_co_create
  vdi: Move file creation to vdi_co_create_opts
  vdi: Implement .bdrv_co_create

Paolo Bonzini (1):
  iscsi: fix iSER compilation

 qapi/block-core.json  | 363 --
 include/block/blockjob.h  |  71 -
 include/block/blockjob_int.h  |  17 +-
 block.c   |  10 +-
 block/backup.c|   5 +-
 block/commit.c|   2 +-
 block/crypto.c| 150 -
 block/iscsi.c |   2 +-
 block/mirror.c|  12 +-
 block/parallels.c | 199 +--
 block/qcow.c  | 196 +++
 block/qed.c   | 204 
 block/stream.c|   2 +-
 block/vdi.c   | 147 +
 block/vhdx.c  | 216 +++--
 block/vpc.c   | 241 +---
 block/vvfat.c |   2 +-
 blockdev.c|  71 +++--
 blockjob.c| 358 +++--
 tests/test-bdrv-drain.c   |   5 +-
 tests/test-blockjob-txn.c |  27 ++--
 tests/test-blockjob.c | 233 ++-
 block/trace-events|   7 +
 hmp-commands.hx   |   3 +-
 tests/qemu-iotests/030|   6 +-
 tests/qemu-iotests/055|  17 +-
 tests/qemu-iotests/056| 187 ++
 tests/qemu-iotests/056.out|   4 +-
 tests/qemu-iotests/109.out|  24 +--
 tests/qemu-iotests/153|  12 ++
 tests/qemu-iotests/153.out|   5 +
 tests/qemu-iotests/181|   2 +-
 tests/qemu-iotests/209| 210 
 tests/qemu-iotests/209.out| 136 
 tests/qemu-iotests/check  |   1 -
 tests/qemu-iotests/common.rc  |   2 +-
 tests/qemu-iotests/group  |   1 +
 tests/qemu-iotests/iotests.py |  12 +-
 38 files changed, 2645 insertions(+), 517 deletions(-)
 create mode 100755 tests/qemu-iotests/209
 create mode 100644 tests/qemu-iotests/209.out

Re: [Qemu-devel] [PATCH 1/5] multiboot: Reject kernels exceeding the address space

[Kevin Wolf]

Am 15.03.2018 um 17:55 hat Jack Schwartz geschrieben:
> On 03/15/18 08:54, Kevin Wolf wrote:
> > Am 15.03.2018 um 06:19 hat Jack Schwartz geschrieben:
> > > Hi Kevin.
> > > 
> > > My comments are inline...
> > > 
> > > On 2018-03-14 10:32, Kevin Wolf wrote:
> > > > The code path with a manually set mh_load_addr in the Multiboot header
> > > > checks that load_end_addr <= load_addr, but the path where load_end_addr
> > > > is automatically detected if 0 is given in the header misses the
> > > > corresponding check.
> > > 1) The code checks that load_end_addr >= load_addr (before letting it
> > > through).
> > > 
> > > 2) load_end_addr is used only when it is read in as non-zero, so no check 
> > > is
> > > needed if zero.  (It gets debug-printed even when zero, but is used only 
> > > to
> > > calculate mb_load_size and only when non-zero.)
> > Oops, good point. I'll change the start of the commit message as follows:
> > 
> >  The code path with a manually set mh_load_end_addr in the Multiboot
> >  header checks that mh_load_end_addr >= mh_load_addr, but the path where
> >  mh_load_end_addr is 0 in the header and therefore automatically
> >  calculated from the file size misses the corresponding check.
> > 
> > Does this look better?
> 
> mb_load_size is calculated from the file size, not mh_load_end_addr, so
> I think you mean mb_load_size rather than mh_load_end_addr.  Do you intend
> to say:
> 
>   The code path where mh_load_end_addr is non-zero in the Multiboot
>   header checks that mh_load_end_addr >= mh_load_addr and so
>   mb_load_size ischecked.  However, mb_load_size is not checked when
>   calculated from thefile size, when mh_load_end_addr is 0.

Ok, technically that's more accurate.

> Also, if this is what you intend to say, would the following code change be
> more ofwhat you want:
> 
> Remove this:
> 
>     mb_load_size = kernel_file_size - mb_kernel_text_offset;
>     }
> -   if (mb_load_size > UINT32_MAX - mh_load_addr) {
> -   error_report("kernel does not fit in address space");
> -   exit(1);
> -   }
>     if (mh_bss_end_addr) {
> 
> and instead do this a few lines further down:
> 
>    mb_kernel_size = mh_bss_end_addr - mh_load_addr;
>     } else {
>     mb_kernel_size = mb_load_size;
>     }
> 
> +   if (mb_kernel_size > UINT32_MAX - mh_load_addr) {
> +   error_report("kernel does not fit in address space");
> +   exit(1);
> +   }
> 
>     mb_debug("multiboot: header_addr = %#x", mh_header_addr);
>     mb_debug("multiboot: load_addr = %#x", mh_load_addr);
> 
> The reason would be to include the bss area in the calculation, when
> mh_bss_end_addr is non-zero.

Ultimately, mb_load_size > mb_kernel_size is what kills us, so maybe we
could add an assertion for that.

But the reason why this condition could ever be true is the integer
overflow in this line:

if (mh_bss_end_addr < (mh_load_addr + mb_load_size))

It's generally better to check for integer overflows before they happen
than trying to infer them from the result. In fact, your condition
wouldn't catch the error of test scenario 9:

kernel_file_size= 0x2035
mh_header_addr  = 0xf000
mh_load_addr= 0xf000
mh_load_end_addr= 0
mh_bss_end_addr = 0xf001

mb_kernel_text_offset   = i - (mh_header_addr - mh_load_addr)
= 0

mb_load_size= kernel_file_size - mb_kernel_text_offset
= 0x2035
> UINT32_MAX - mh_load_addr

mb_kernel_size  = mh_bss_end_addr - mh_load_addr
= 1
< UINT32_MAX - mh_load_addr

Kevin

Re: [Qemu-devel] [PATCH 8/9] hw/arm/bcm2836: Hardcode correct CPU type

[Peter Maydell]

On 13 March 2018 at 16:55, Andrew Baumann  wrote:
>> From: Qemu-devel > bounces+andrew.baumann=microsoft@nongnu.org> On Behalf Of Peter
>> Maydell
>> Sent: Tuesday, 13 March 2018 08:35
>>
>> Now we have separate types for BCM2386 and BCM2387, we might as well
>> just hard-code the CPU type they use rather than having it passed
>> through as an object property. This then lets us put the initialization
>> of the CPU object in init rather than realize.

> What about the default_cpu_type field of MachineClass set in
> raspi[23]_machine_init? That seems irrelevant now...

Igor, can you help with this question? For a board that always
has one CPU type (because the real hardware only ever has
one SoC, and that SoC QOM object hard codes the CPU type)
does it still need to set the mc->default_cpu_type field in
its MachineClass, or does that become pointless ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2] linux-user: implement HWCAP bits on MIPS

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180315151348.6451-1-james.cowg...@mips.com
Subject: [Qemu-devel] [PATCH v2] linux-user: implement HWCAP bits on MIPS

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   6265e23b1c..56e8698ffa  master -> master
 t [tag update]patchew/20180315115144.801202-1-ebl...@redhat.com -> 
patchew/20180315115144.801202-1-ebl...@redhat.com
 * [new tag]   patchew/20180315151348.6451-1-james.cowg...@mips.com 
-> patchew/20180315151348.6451-1-james.cowg...@mips.com
Switched to a new branch 'test'
31ed3b34df linux-user: implement HWCAP bits on MIPS

=== OUTPUT BEGIN ===
Checking PATCH 1/1: linux-user: implement HWCAP bits on MIPS...
ERROR: braces {} are necessary for all arms of this statement
#35: FILE: linux-user/elfload.c:967:
+do { if (cpu->env.insn_flags & (flag)) { hwcaps |= hwcap; } } while (0)
[...]

total: 1 errors, 0 warnings, 30 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

Re: [Qemu-devel] [PULL 00/41] Block layer patches

[Kevin Wolf]

Am 15.03.2018 um 17:42 hat Peter Maydell geschrieben:
> On 13 March 2018 at 16:17, Kevin Wolf  wrote:
> > The following changes since commit 22ef7ba8e8ce7fef297549b3defcac333742b804:
> >
> >   Merge remote-tracking branch 'remotes/famz/tags/staging-pull-request' 
> > into staging (2018-03-13 11:42:45 +)
> >
> > are available in the git repository at:
> >
> >   git://repo.or.cz/qemu/kevin.git tags/for-upstream
> >
> > for you to fetch changes up to be6c885842efded81a20f4ca24f0d4e123a80c00:
> >
> >   block/mirror: change the semantic of 'force' of block-job-cancel 
> > (2018-03-13 16:54:47 +0100)
> >
> > 
> > Block layer patches
> >
> > 
> 
> I get a compile failure here on some hosts:
> 
> /home/pm215/qemu/blockjob.c: In function ‘block_job_txn_apply.isra.8’:
> /home/pm215/qemu/blockjob.c:524:5: error: ‘rc’ may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
>  return rc;
>  ^
> 
> I guess the compiler can't always figure out whether there is
> guaranteed to be at least one thing in the list ?

I think so.

John, I'll just modify your patch 'blockjobs: add prepare callback' to
initialise rc = 0 in this function and send a v2 pull request.

Kevin

Re: [Qemu-devel] [PATCH 1/5] multiboot: Reject kernels exceeding the address space

[Jack Schwartz]

On 03/15/18 08:54, Kevin Wolf wrote:

Am 15.03.2018 um 06:19 hat Jack Schwartz geschrieben:

Hi Kevin.

My comments are inline...

On 2018-03-14 10:32, Kevin Wolf wrote:

The code path with a manually set mh_load_addr in the Multiboot header
checks that load_end_addr <= load_addr, but the path where load_end_addr
is automatically detected if 0 is given in the header misses the
corresponding check.

1) The code checks that load_end_addr >= load_addr (before letting it
through).

2) load_end_addr is used only when it is read in as non-zero, so no check is
needed if zero.  (It gets debug-printed even when zero, but is used only to
calculate mb_load_size and only when non-zero.)

Oops, good point. I'll change the start of the commit message as follows:

 The code path with a manually set mh_load_end_addr in the Multiboot
 header checks that mh_load_end_addr >= mh_load_addr, but the path where
 mh_load_end_addr is 0 in the header and therefore automatically
 calculated from the file size misses the corresponding check.

Does this look better?


mb_load_size is calculated from the file size, not mh_load_end_addr, so
I think you mean mb_load_size rather than mh_load_end_addr.  Do you 
intend to say:


  The code path where mh_load_end_addr is non-zero in the Multiboot
  header checks that mh_load_end_addr >= mh_load_addr and so
  mb_load_size ischecked.  However, mb_load_size is not checked when
  calculated from thefile size, when mh_load_end_addr is 0.

Also, if this is what you intend to say, would the following code change 
be more ofwhat you want:


Remove this:

    mb_load_size = kernel_file_size - mb_kernel_text_offset;
    }
-   if (mb_load_size > UINT32_MAX - mh_load_addr) {
-   error_report("kernel does not fit in address space");
-   exit(1);
-   }
    if (mh_bss_end_addr) {

and instead do this a few lines further down:

   mb_kernel_size = mh_bss_end_addr - mh_load_addr;
    } else {
    mb_kernel_size = mb_load_size;
    }

+   if (mb_kernel_size > UINT32_MAX - mh_load_addr) {
+   error_report("kernel does not fit in address space");
+   exit(1);
+   }

    mb_debug("multiboot: header_addr = %#x", mh_header_addr);
    mb_debug("multiboot: load_addr = %#x", mh_load_addr);

The reason would be to include the bss area in the calculation, when
mh_bss_end_addr is non-zero.

    Thanks,
    Jack




If the kernel binary size is larger than can fit in
the address space after load_addr, we ended up with a kernel_size that
is smaller than load_size, which means that we read the file into a too
small buffer.

Add a check to reject kernel files with such Multiboot headers.

Code itself looks fine.

Modulo above comments:
     Reviewed-by: Jack Schwartz 

Thanks for your review of the series!

Kevin

Re: [Qemu-devel] -icount changes physical address assignments in QEMU 2.10/2.11

[Peter Maydell]

On 23 February 2018 at 00:39,   wrote:
> When porting our RTOS from QEMU 2.8 to 2.10/2.11, I ran into a problem
> where 16-bit writes to the "bochs dispi interface" were being reported
> differently depending on whether or not "-icount" was given to QEMU.
>
> For example, info mtree:
>   ...
>   11000500-11000515 (prio 0, i/o): bochs dispi interface
>
> A 16-bit write to 0x11000500 was delivered to pci_vga_bochs_write() as
> having
> address 0, when -icount was not specified, but as address 2 when
> -icount was specified.  Correspondingly writes to 0x11000502 were 2
> and 0 respectively.  Essentially the words were swapped depending on
> the presence of -icount.
>
> I suspect a similar problem for the AARCH64 GIC (generic interrupt
> controller), but other than observing the GIC changing from working to
> non-working depending on the absence/presence of -icount I haven't
> confirmed the underlying cause.
>
> 2.10 and 2.11 were built from source on Cygwin using mingw, 2.8 from a
> "native" MinGW.  The results are consistent for 2.10 and 2.11.  2.8
> does not have the -icount dependency.  The "broken" command line was:
>
> qemu-system-aarch64 -m 1077 -name "arm" -M virt,virtualization=on
>   -cpu cortex-a53 -icount align=off,shift=0,sleep=on
>   -vga std -device secondary-vga
>   -device
> virtio-net,netdev=vlan0,addr=2,disable-modern=false,mac=52:54:00:12:67:56
>   -kernel ...deosBoot.bin -initrd "deosBoot.qemu" -netdev
> tap,id=vlan0,ifname="DDCI-tap0"
>
> The "working" command line omitted -icount and its argument.
>
> FWIW, the error from the GIC with -icount was:
>
>   qemu: fatal: IO on conditional branch instruction
>
> Any pointers?

Can you try with this patch applied?
https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg06595.html
(that will go into git master within the next week or so but isn't
yet applied; it does fix at least some -icount related problems).

If that doesn't help, can you provide a demonstration test
binary that I can use to reproduce the bug?

thanks
-- PMM

Re: [Qemu-devel] [Qemu-block] [PATCH] iscsi: fix iSER compilation

[Kevin Wolf]

Am 15.03.2018 um 15:30 hat Paolo Bonzini geschrieben:
> This fails in Fedora 28.
> 
> Reported-by: Andreas Schwab 
> Signed-off-by: Paolo Bonzini 

Thanks, applied to the block branch.

Kevin

Re: [Qemu-devel] [PULL 00/41] Block layer patches

[Peter Maydell]

On 13 March 2018 at 16:17, Kevin Wolf  wrote:
> The following changes since commit 22ef7ba8e8ce7fef297549b3defcac333742b804:
>
>   Merge remote-tracking branch 'remotes/famz/tags/staging-pull-request' into 
> staging (2018-03-13 11:42:45 +)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to be6c885842efded81a20f4ca24f0d4e123a80c00:
>
>   block/mirror: change the semantic of 'force' of block-job-cancel 
> (2018-03-13 16:54:47 +0100)
>
> 
> Block layer patches
>
> 

I get a compile failure here on some hosts:

/home/pm215/qemu/blockjob.c: In function ‘block_job_txn_apply.isra.8’:
/home/pm215/qemu/blockjob.c:524:5: error: ‘rc’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
 return rc;
 ^

I guess the compiler can't always figure out whether there is
guaranteed to be at least one thing in the list ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] block: Fix leak of ignore_children in error path

[Kevin Wolf]

Am 15.03.2018 um 04:51 hat Fam Zheng geschrieben:
> Reported-by: Max Reitz 
> Signed-off-by: Fam Zheng 

Thanks, applied to the block branch.

Kevin

Re: [Qemu-devel] [PATCH] vvfat: Fix inherit_options flags

[Kevin Wolf]

Am 15.03.2018 um 04:45 hat Fam Zheng geschrieben:
> Overriding flags violates the precedence rules of
> bdrv_reopen_queue_child. Just like the read-only option, no-flush should
> be put into the options. The same is done in bdrv_temp_snapshot_options.
> 
> Reported-by: Stefan Hajnoczi  Signed-off-by: Fam Zheng 

Thanks, applied to the block branch.

Kevin

[Qemu-devel] [Bug 1749393] Re: sbrk() not working under qemu-user with a PIE-compiled binary?

[Peter Maydell]

There seem to be two parts to this. Firstly, with a big reserved-region,
which is the default for 32-bit-guest-on-64-bit-host, this code in
main.c:

if (reserved_va) {
mmap_next_start = reserved_va;
}

says to start trying for the next mmap address at the top of the
reserved section, which is typically right at the top of the guest's
address space. This means that for a PIE executable we'll try to load it
at a very high address, which then means there's no space above the data
section for the brk segment.

Secondly, for the no-reserved-region case (-R 0, or 64-on-64), we still
fail, but this time because we've chosen to mmap the dynamic interpreter
at an address just above the executable. Again, no space to expand the
data segment and brk fails.

Linux kernel commit a87938b2e246b81 message says something about there
being a guaranteed 128MB "gap" between data segment and stack on x86-64
which we're obviously not honourin; presumbably there's similar
requirements for other archs. (As an aside, is bash really happy with
only having perhaps 128MB of allocatable memory? Otherwise it really
ought to use mmap rather than brk for its allocator.)

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

Title:
  sbrk() not working under qemu-user with a PIE-compiled binary?

Status in QEMU:
  New
Status in qemu package in Ubuntu:
  New

Bug description:
  In Debian unstable, we recently switched bash to be a PIE-compiled
  binary (for hardening). Unfortunately this resulted in bash being
  broken when run under qemu-user (for all target architectures, host
  being amd64 for me).

  $ sudo chroot /srv/chroots/sid-i386/ qemu-i386-static /bin/bash
  bash: xmalloc: .././shell.c:1709: cannot allocate 10 bytes (0 bytes allocated)

  bash has its own malloc implementation based on sbrk():
  https://git.savannah.gnu.org/cgit/bash.git/tree/lib/malloc/malloc.c

  When we disable this internal implementation and rely on glibc's
  malloc, then everything is fine. But it might be that glibc has a
  fallback when sbrk() is not working properly and it might hide the
  underlying problem in qemu-user.

  This issue has also been reported to the bash upstream author and he 
suggested that the issue might be in qemu-user so I'm opening a ticket here. 
Here's the discussion with the bash upstream author:
  https://lists.gnu.org/archive/html/bug-bash/2018-02/threads.html#00080

  You can find the problematic bash binary in that .deb file:
  
http://snapshot.debian.org/archive/debian/20180206T154716Z/pool/main/b/bash/bash_4.4.18-1_i386.deb

  The version of qemu I have been using is 2.11 (Debian package qemu-
  user-static version 1:2.11+dfsg-1) but I have had reports that the
  problem is reproducible with older versions (back to 2.8 at least).

  Here are the related Debian bug reports:
  https://bugs.debian.org/889869
  https://bugs.debian.org/865599

  It's worth noting that bash used to have this problem (when compiled as a PIE 
binary) even when run directly but then something got fixed in the kernel and 
now the problem only appears when run under qemu-user:
  https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1518483

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

Re: [Qemu-devel] [PATCH 00/11] implement 680x0 FPU (part 5)

[Richard Henderson]

On 03/13/2018 04:27 AM, Laurent Vivier wrote:
> I have compared results of these instructions from a real m68040 and from
> QEMU, and they match (sincos differs [1] because in QEMU we compute it as
> sin and cos, and on m68040 sin and cos results differ also with
> sincos results. It looks like a rounding problem, perhaps a bug in m68040
> FPU? or in my test program?)
> 
> I know this will need more work, but for the moment I only would
> like to provide these new instructions to the maintream of QEMU.
> 
> [1] Example:
> 
> sincos -6.125952 -> (0.156586, 0.987664)
> 
> QEMU:
> 
>   sincos c001c407cd1182c2bb27 -> 3ffca05812beea449a4d
>  3ffefcd791d65887d19a
>   sin c001c407cd1182c2bb27-> 3ffca05812beea449a4d
>   cos c001c407cd1182c2bb27-> 3ffefcd791d65887d19a
> 
> m68040:
> 
>   sincos c001c407cd1182c2bb27 -> 3ffca05812beea449a4d
>  3ffefcd791d65887d199 <<--
>   sin c001c407cd1182c2bb27-> 3ffca05812beea449a4d
>   cos c001c407cd1182c2bb27-> 3ffefcd791d65887d19a

Wow, this does seem like a processor bug.

Anyway, I've looked through the patch set and nothing stands out,

Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [Bug 1756080] Re: QEMU does not provide non-Linux kernels with ATAGS structure on ARM targets

[Peter Maydell]

QEMU doesn't care whether the kernel you provide it is Linux or
something else. If you pass -kernel something that's not an ELF file,
we'll load and boot it using the Linux boot protocol (including ATAGS,
potentially). If you pass an ELF file or pass -bios a binary blob, we'll
just start it in the way the CPU usually resets, on the assumption it
can deal with whatever that is.

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

Title:
  QEMU does not provide non-Linux kernels with ATAGS structure on ARM
  targets

Status in QEMU:
  New

Bug description:
  This would be a useful feature. Many kernels, particularly hobbyist
  kernels, have support for ATAGS.

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

Re: [Qemu-devel] [PATCH 1/5] multiboot: Reject kernels exceeding the address space

[Kevin Wolf]

Am 15.03.2018 um 06:19 hat Jack Schwartz geschrieben:
> Hi Kevin.
> 
> My comments are inline...
> 
> On 2018-03-14 10:32, Kevin Wolf wrote:
> > The code path with a manually set mh_load_addr in the Multiboot header
> > checks that load_end_addr <= load_addr, but the path where load_end_addr
> > is automatically detected if 0 is given in the header misses the
> > corresponding check.
> 1) The code checks that load_end_addr >= load_addr (before letting it
> through).
> 
> 2) load_end_addr is used only when it is read in as non-zero, so no check is
> needed if zero.  (It gets debug-printed even when zero, but is used only to
> calculate mb_load_size and only when non-zero.)

Oops, good point. I'll change the start of the commit message as follows:

The code path with a manually set mh_load_end_addr in the Multiboot
header checks that mh_load_end_addr >= mh_load_addr, but the path where
mh_load_end_addr is 0 in the header and therefore automatically
calculated from the file size misses the corresponding check.

Does this look better?

> > If the kernel binary size is larger than can fit in
> > the address space after load_addr, we ended up with a kernel_size that
> > is smaller than load_size, which means that we read the file into a too
> > small buffer.
> > 
> > Add a check to reject kernel files with such Multiboot headers.
> Code itself looks fine.
> 
> Modulo above comments:
>     Reviewed-by: Jack Schwartz 

Thanks for your review of the series!

Kevin

[Qemu-devel] [Bug 1756080] [NEW] QEMU does not provide non-Linux kernels with ATAGS structure on ARM targets

[barry.of.smith--- via Qemu-devel]

Public bug reported:

This would be a useful feature. Many kernels, particularly hobbyist
kernels, have support for ATAGS.

** 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/1756080

Title:
  QEMU does not provide non-Linux kernels with ATAGS structure on ARM
  targets

Status in QEMU:
  New

Bug description:
  This would be a useful feature. Many kernels, particularly hobbyist
  kernels, have support for ATAGS.

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

[Qemu-devel] [Bug 1749393] Re: sbrk() not working under qemu-user with a PIE-compiled binary?

[Matthias Klose]

** Also affects: qemu (Ubuntu)
   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/1749393

Title:
  sbrk() not working under qemu-user with a PIE-compiled binary?

Status in QEMU:
  New
Status in qemu package in Ubuntu:
  New

Bug description:
  In Debian unstable, we recently switched bash to be a PIE-compiled
  binary (for hardening). Unfortunately this resulted in bash being
  broken when run under qemu-user (for all target architectures, host
  being amd64 for me).

  $ sudo chroot /srv/chroots/sid-i386/ qemu-i386-static /bin/bash
  bash: xmalloc: .././shell.c:1709: cannot allocate 10 bytes (0 bytes allocated)

  bash has its own malloc implementation based on sbrk():
  https://git.savannah.gnu.org/cgit/bash.git/tree/lib/malloc/malloc.c

  When we disable this internal implementation and rely on glibc's
  malloc, then everything is fine. But it might be that glibc has a
  fallback when sbrk() is not working properly and it might hide the
  underlying problem in qemu-user.

  This issue has also been reported to the bash upstream author and he 
suggested that the issue might be in qemu-user so I'm opening a ticket here. 
Here's the discussion with the bash upstream author:
  https://lists.gnu.org/archive/html/bug-bash/2018-02/threads.html#00080

  You can find the problematic bash binary in that .deb file:
  
http://snapshot.debian.org/archive/debian/20180206T154716Z/pool/main/b/bash/bash_4.4.18-1_i386.deb

  The version of qemu I have been using is 2.11 (Debian package qemu-
  user-static version 1:2.11+dfsg-1) but I have had reports that the
  problem is reproducible with older versions (back to 2.8 at least).

  Here are the related Debian bug reports:
  https://bugs.debian.org/889869
  https://bugs.debian.org/865599

  It's worth noting that bash used to have this problem (when compiled as a PIE 
binary) even when run directly but then something got fixed in the kernel and 
now the problem only appears when run under qemu-user:
  https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1518483

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

Re: [Qemu-devel] [PATCH v2] linux-user: implement HWCAP bits on MIPS

[Laurent Vivier]

Le 15/03/2018 à 16:13, James Cowgill a écrit :
> Add support for the two currently defined HWCAP bits on MIPS - R6 and
> MSA.
> 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1754372
> Signed-off-by: James Cowgill 
> ---
> v2 changes:
>  - Fix kernel hwcap.h path.
> 
>  linux-user/elfload.c | 24 
>  1 file changed, 24 insertions(+)
> 

Reviewed-by: Laurent Vivier 

[Qemu-devel] [Bug 1748434] Re: Possibly wrong GICv3 behavior when secure enabled

[Peter Maydell]

Patch which should fix this:
https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg04537.html


** Changed in: qemu
   Status: New => In Progress

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

Title:
  Possibly wrong GICv3 behavior when secure enabled

Status in QEMU:
  In Progress

Bug description:
  I an tried arm-aarch64 interrupt routing to EL3, by SCR_EL3.FIQ=1. First I am 
started QEMU with secure=on and GICv3 support.
  I programmed secure and non-secure timers and set-up appropriate 
interrupts.Secure timer to be GRP1_Secure and non-secure timer to be 
GRP1_NonSecure. ICC_PMR = 0xff. Then I switched CPU to EL1. 
  With that setup no interrupt was delivered to PE. GIC interface showed that 
non secure IRQ is pending. ICC_PMR read at EL1 returns 0 (shall return value 
((PMR_(el3) << 1) & 0xff) according to GIC specification.
  Than I tried to increase interrupt priority mask  - so I set ICC_PMR = 0x7f 
(at EL3). Then I read at EL1 ICC_PMR=0xfe - (is shall be 0). With this setup 
IRQ of secure timer was taken at EL3, non secure timer didn't rise IRQ (as it 
is masked by PMR). 
  I dig to qemu code and see wrong condition in file arm_gicv3_cpuif.c in 
function  icc_pmr_read(). This behavior is opposite of ARM specification.

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

[Qemu-devel] [PATCH v2] linux-user: implement HWCAP bits on MIPS

[James Cowgill]

Add support for the two currently defined HWCAP bits on MIPS - R6 and
MSA.

Buglink: https://bugs.launchpad.net/qemu/+bug/1754372
Signed-off-by: James Cowgill 
---
v2 changes:
 - Fix kernel hwcap.h path.

 linux-user/elfload.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 5fc130cc20..318c124712 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -950,6 +950,30 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, 
const CPUMIPSState *e
 #define USE_ELF_CORE_DUMP
 #define ELF_EXEC_PAGESIZE4096
 
+/* See arch/mips/include/uapi/asm/hwcap.h.  */
+enum {
+HWCAP_MIPS_R6   = (1 << 0),
+HWCAP_MIPS_MSA  = (1 << 1),
+};
+
+#define ELF_HWCAP get_elf_hwcap()
+
+static uint32_t get_elf_hwcap(void)
+{
+MIPSCPU *cpu = MIPS_CPU(thread_cpu);
+uint32_t hwcaps = 0;
+
+#define GET_FEATURE(flag, hwcap) \
+do { if (cpu->env.insn_flags & (flag)) { hwcaps |= hwcap; } } while (0)
+
+GET_FEATURE(ISA_MIPS32R6 | ISA_MIPS64R6, HWCAP_MIPS_R6);
+GET_FEATURE(ASE_MSA, HWCAP_MIPS_MSA);
+
+#undef GET_FEATURE
+
+return hwcaps;
+}
+
 #endif /* TARGET_MIPS */
 
 #ifdef TARGET_MICROBLAZE
-- 
2.16.2

Re: [Qemu-devel] [PULL 0/3] Reduce Travis timeouts

[Peter Maydell]

On 13 March 2018 at 16:46, Alex Bennée  wrote:
> The following changes since commit 22ef7ba8e8ce7fef297549b3defcac333742b804:
>
>   Merge remote-tracking branch 'remotes/famz/tags/staging-pull-request' into 
> staging (2018-03-13 11:42:45 +)
>
> are available in the Git repository at:
>
>   https://github.com/stsquad/qemu.git tags/pull-travis-speedup-130318-1
>
> for you to fetch changes up to 0b438fa627ca192e4be2c527bf188706d64bdd0d:
>
>   .travis.yml: add --disable-user with the rest of the disables (2018-03-13 
> 16:22:23 +)
>
> 
> Some updates to reduce timeouts in Travis
>
> 
> Alex Bennée (3):
>   .travis.yml: drop setting default log output
>   .travis.yml: split default config into system and user
>   .travis.yml: add --disable-user with the rest of the disables
>
>  .travis.yml | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v2 for-2.12] iotests: Avoid realpath, for CentOS 6

[Philippe Mathieu-Daudé]

On 03/15/2018 12:51 PM, Eric Blake wrote:
> CentOS 6 lacks a realpath binary on the base install, which makes
> all iotests runs fail since the 2.11 release:
> 
> 001 - output mismatch (see 001.out.bad)
> ./check: line 815: realpath: command not found
> diff: missing operand after `/home/dummy/qemu/tests/qemu-iotests/001.out'
> diff: Try `diff --help' for more information.
> 
> Many of the uses of 'realpath' in the check script were being
> used on the output of 'type -p' - but that is already an
> absolute file name.  While a canonical name can often be
> shorter (realpath gets rid of /../), it can also be longer (due
> to symlink expansion); and we really don't care if the name is
> canonical, merely that it was an executable file with an
> absolute path.  These were broken in commit cceaf1db.
> 
> The remaining use of realpath was to convert a possibly relative
> filename into an absolute one before calling diff to make it
> easier to copy-and-paste the filename for moving the .bad file
> into place as the new reference file even when running iotests
> out-of-tree (see commit 93e53fb6), but $PWD can achieve the same
> purpose.
> 
> Signed-off-by: Eric Blake 

TIL type -p :)

Reviewed-by: Philippe Mathieu-Daudé 

> 
> ---
> v2: Don't revert 93e53fb6, add commit id mentions in commit
> message, retitle
> ---
>  tests/qemu-iotests/check | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 469142cd586..ec8033350d1 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -92,7 +92,7 @@ set_prog_path()
>  {
>  p=`command -v $1 2> /dev/null`
>  if [ -n "$p" -a -x "$p" ]; then
> -realpath -- "$(type -p "$p")"
> +type -p "$p"
>  else
>  return 1
>  fi
> @@ -554,7 +554,7 @@ then
>  [ "$QEMU_PROG" = "" ] && _init_error "qemu not found"
>  fi
>  fi
> -export QEMU_PROG=$(realpath -- "$(type -p "$QEMU_PROG")")
> +export QEMU_PROG="$(type -p "$QEMU_PROG")"
> 
>  if [ -z "$QEMU_IMG_PROG" ]; then
>  if [ -x "$build_iotests/qemu-img" ]; then
> @@ -565,7 +565,7 @@ if [ -z "$QEMU_IMG_PROG" ]; then
>  _init_error "qemu-img not found"
>  fi
>  fi
> -export QEMU_IMG_PROG=$(realpath -- "$(type -p "$QEMU_IMG_PROG")")
> +export QEMU_IMG_PROG="$(type -p "$QEMU_IMG_PROG")"
> 
>  if [ -z "$QEMU_IO_PROG" ]; then
>  if [ -x "$build_iotests/qemu-io" ]; then
> @@ -576,7 +576,7 @@ if [ -z "$QEMU_IO_PROG" ]; then
>  _init_error "qemu-io not found"
>  fi
>  fi
> -export QEMU_IO_PROG=$(realpath -- "$(type -p "$QEMU_IO_PROG")")
> +export QEMU_IO_PROG="$(type -p "$QEMU_IO_PROG")"
> 
>  if [ -z $QEMU_NBD_PROG ]; then
>  if [ -x "$build_iotests/qemu-nbd" ]; then
> @@ -587,7 +587,7 @@ if [ -z $QEMU_NBD_PROG ]; then
>  _init_error "qemu-nbd not found"
>  fi
>  fi
> -export QEMU_NBD_PROG=$(realpath -- "$(type -p "$QEMU_NBD_PROG")")
> +export QEMU_NBD_PROG="$(type -p "$QEMU_NBD_PROG")"
> 
>  if [ -z "$QEMU_VXHS_PROG" ]; then
>export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
> @@ -811,7 +811,7 @@ do
>  else
>  echo " - output mismatch (see $seq.out.bad)"
>  mv $tmp.out $seq.out.bad
> -$diff -w "$reference" $(realpath $seq.out.bad)
> +$diff -w "$reference" "$PWD"/$seq.out.bad
>  err=true
>  fi
>  fi
> 

[Qemu-devel] [Bug 1602247] Re: SIGSEGV when reading ARM GIC registers through GDB stub

[Peter Maydell]

See also LP:1751674 -- similar crash trying to access the GICv2 per-cpu
regs from the QEMU monitor's pmemsave command.

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

Title:
  SIGSEGV when reading ARM GIC registers through GDB stub

Status in QEMU:
  Confirmed

Bug description:
  When trying to access ARM GIC CPU registers through a target GDB
  connected to QEMU, QEMU crashes with a SIGSEGV.

  Reproducible on last master revision (74e1b78 at the time of writing):

  $ ./configure --target-list=arm-softmmu --python=python2 --enable-debug
  $ make
  $ gdb --args ./arm-softmmu/qemu-system-arm -M vexpress-a15 -smp 2 -s -S

  
  Connect a gdb on the other side:
  $ arm-none-eabi-gdb
  (gdb) tar rem :1234
  Remote debugging using :1234
  0x in ?? ()
  (gdb) x /x 0x2c002000

  
  QEMU crashes as it tries to access current_cpu->cpu_index while current_cpu 
is NULL:

  Thread 1 "qemu-system-arm" received signal SIGSEGV, Segmentation fault.
  0x55a372cf in gic_get_current_cpu (s=0x56a34f10) at 
hw/intc/arm_gic.c:53
  53return current_cpu->cpu_index;
  (gdb) p current_cpu
  $1 = (CPUState *) 0x0

  Backtrace:
  (gdb) bt
  #0  0x55a372cf in gic_get_current_cpu (s=0x56a34f10) at 
hw/intc/arm_gic.c:53
  #1  0x55a3b0e3 in gic_thiscpu_read (opaque=0x56a34f10, addr=0, 
data=0x7fffa0a8, size=4, attrs=...) at hw/intc/arm_gic.c:1340
  #2  0x557ae2bb in memory_region_read_with_attrs_accessor 
(mr=0x56a37a70, addr=0, value=0x7fffa208, size=4, shift=0, 
mask=4294967295, attrs=...) at /home/sekoia/devel/src/qemu/memory.c:461
  #3  0x557ae7ac in access_with_adjusted_size (addr=0, 
value=0x7fffa208, size=4, access_size_min=1, access_size_max=4, 
access=0x557ae25f , 
mr=0x56a37a70, attrs=...)
  at /home/sekoia/devel/src/qemu/memory.c:591
  #4  0x557b0de7 in memory_region_dispatch_read1 (mr=0x56a37a70, 
addr=0, pval=0x7fffa208, size=4, attrs=...) at 
/home/sekoia/devel/src/qemu/memory.c:1187
  #5  0x557b0e9d in memory_region_dispatch_read (mr=0x56a37a70, 
addr=0, pval=0x7fffa208, size=4, attrs=...) at 
/home/sekoia/devel/src/qemu/memory.c:1212
  #6  0x5576775b in address_space_read_continue (as=0x569c70b0, 
addr=738205696, attrs=..., buf=0x7fffb440 "", len=4, addr1=0, l=4, 
mr=0x56a37a70) at /home/sekoia/devel/src/qemu/exec.c:2668
  #7  0x55767929 in address_space_read_full (as=0x569c70b0, 
addr=738205696, attrs=..., buf=0x7fffb440 "", len=4) at 
/home/sekoia/devel/src/qemu/exec.c:2725
  #8  0x557679eb in address_space_read (len=4, buf=0x7fffb440 "", 
attrs=..., addr=738205696, as=0x569c70b0) at 
/home/sekoia/devel/src/qemu/include/exec/memory.h:1476
  #9  address_space_rw (as=0x569c70b0, addr=738205696, attrs=..., 
buf=0x7fffb440 "", len=4, is_write=false) at 
/home/sekoia/devel/src/qemu/exec.c:2739
  #10 0x5576988f in cpu_memory_rw_debug (cpu=0x568a3d00, 
addr=738205696, buf=0x7fffb440 "", len=4, is_write=0) at 
/home/sekoia/devel/src/qemu/exec.c:3653
  #11 0x557a3db3 in target_memory_rw_debug (cpu=0x568a3d00, 
addr=738205696, buf=0x7fffb440 "", len=4, is_write=false) at 
/home/sekoia/devel/src/qemu/gdbstub.c:54
  #12 0x557a53f5 in gdb_handle_packet (s=0x5722c530, 
line_buf=0x5722c54c "m2c002000,4") at 
/home/sekoia/devel/src/qemu/gdbstub.c:968
  #13 0x557a6b84 in gdb_read_byte (s=0x5722c530, ch=52) at 
/home/sekoia/devel/src/qemu/gdbstub.c:1458
  #14 0x557a6ca4 in gdb_chr_receive (opaque=0x0, buf=0x7fffc590 
"$m2c002000,4#84c8ead:arm-neon.xml:7fd,802#4c;qRelocInsn+;fork-events+;vfork-events+;exec-events+;vContSupported+;QThreadEvents+;no-resumed+#df",
 size=15)
  at /home/sekoia/devel/src/qemu/gdbstub.c:1666
  #15 0x5591c562 in qemu_chr_be_write_impl (s=0x57226e20, 
buf=0x7fffc590 
"$m2c002000,4#84c8ead:arm-neon.xml:7fd,802#4c;qRelocInsn+;fork-events+;vfork-events+;exec-events+;vContSupported+;QThreadEvents+;no-resumed+#df",
 
  len=15) at qemu-char.c:387
  #16 0x5591c5c0 in qemu_chr_be_write (s=0x57226e20, 
buf=0x7fffc590 
"$m2c002000,4#84c8ead:arm-neon.xml:7fd,802#4c;qRelocInsn+;fork-events+;vfork-events+;exec-events+;vContSupported+;QThreadEvents+;no-resumed+#df",
 len=15)
  at qemu-char.c:399
  #17 0x559207f3 in tcp_chr_read (chan=0x56e52ff0, cond=G_IO_IN, 
opaque=0x57226e20) at qemu-char.c:2893
  #18 0x55c4a9b7 in qio_channel_fd_source_dispatch 
(source=0x57226ca0, callback=0x5592069d , 
user_data=0x57226e20) at io/channel-watch.c:84
  #19 0x7fffed977c8a in g_main_context_dispatch () from 
/usr/lib/libglib-2.0.so.0
  #20 0x55bbf711 in glib_pollfds_poll () at main-loop.c:213
  #21 0x55bbf7ec in os_host_main_loop_wait (timeout=29744918) at 
main-loop.c:258
  #22 

[Qemu-devel] [Bug 1746943] Re: qemu-aarch64-static: qemu: uncaught target signal 11 for ps/top cmd

[Peter Maydell]

Could you provide instructions to reproduce that don't require Fedora or
docker, please?

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

Title:
  qemu-aarch64-static: qemu: uncaught target signal 11 for ps/top cmd

Status in QEMU:
  New

Bug description:
  In a docker container created from an aarch64 image, injects 
qemu-aarch64-static (in /usr/bin)
run ps/top cmd  inside this container

reports "qemu: uncaught target signal 11 (Segmentation fault)"

  Tried qemu-aarch64-static from fedora 27 / ubuntu artful / debian
  unstable (i.e. qemu version 2.10 - 2.11)

  The aarch64 dock image is fedora 27(and with qemu-aarch64-static
  Fedora 27), hence I opened a related bug here
  https://bugzilla.redhat.com/show_bug.cgi?id=1541252

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

[Qemu-devel] [Bug 1751674] Re: qemu-system-arm segmentation fault trying to dump VM memory

[Peter Maydell]

LP:1602247 is the bug for the similar issue when using the gdb stub.

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

Title:
  qemu-system-arm segmentation fault trying to dump VM memory

Status in QEMU:
  New

Bug description:
  Qemu segfaults trying to generate a VM memory dump:

  $ QEMU_AUDIO_DRV=none qemu-git-src/arm-softmmu/qemu-system-arm -M vexpress-a9 
-smp 4 -m 1024 -machine secure=off,dump-guest-core=on -kernel 
linux-4.9.75/arch/arm/boot/zImage -append "root=/dev/mmcblk0 rw rootfstype=ext4 
mem=1024M net.ifnames=0 console=ttyAMA0" -dtb vexpress-v2p-ca9.dtb -sd 
armv7-hd.qcow2 -netdev tap,ifname=tap_armv7,script=no,downscript=no,id=net0 
-device virtio-net-device,mac=00:AA:AD:BB:FF:02,netdev=net0  -monitor stdio 
-serial vc  -loadvm SS0
  QEMU 2.11.50 monitor - type 'help' for more information
  (qemu) pmemsave 0 0x3FFF memory.dmp
  Segmentation fault (core dumped)

  $ git rev-parse HEAD
  b384cd95eb9c6f73ad84ed1bb0717a26e29cc78f

  It's the second time I try to submit this bug, I think last time it
  failed because the attached core dump size (400M compressed). Have a
  look if you can get that file, otherwise I will try to update this
  ticket once it's created:

  (Error ID: OOPS-65553b72bc14be693eb1e37814ff9267)

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