[Qemu-devel] [PATCH RFC] hw/pc: set q35 as the default x86 machine

2018-06-03 Thread Marcel Apfelbaum
Moving to QEMU 3.0 seems like a good opportunity for such a change.

I440FX is really old and does not support modern features like IOMMU.
Q35's SATA emulation is faster than pc's IDE, native PCI express hotplug
is cleaner than ACPI based one and so on...

Also the libvirt guys added very good support for the Q35 machine (thanks!).

Management software should always specify the machine type and for the
current setups, adding '-machine pc' to the command line is not such a
big deal.

In time the pc machine will fade out and we will probably stop adding
new versions at some point.

Signed-off-by: Marcel Apfelbaum 
---
 hw/i386/pc_piix.c | 2 --
 hw/i386/pc_q35.c  | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index b4c5b03274..16dd65198f 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -429,7 +429,6 @@ static void pc_i440fx_3_0_machine_options(MachineClass *m)
 {
 pc_i440fx_machine_options(m);
 m->alias = "pc";
-m->is_default = 1;
 }
 
 DEFINE_I440FX_MACHINE(v3_0, "pc-i440fx-3.0", NULL,
@@ -438,7 +437,6 @@ DEFINE_I440FX_MACHINE(v3_0, "pc-i440fx-3.0", NULL,
 static void pc_i440fx_2_12_machine_options(MachineClass *m)
 {
 pc_i440fx_3_0_machine_options(m);
-m->is_default = 0;
 m->alias = NULL;
 SET_MACHINE_COMPAT(m, PC_COMPAT_2_12);
 }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 83d6d75efa..b33c235d49 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -312,6 +312,7 @@ static void pc_q35_3_0_machine_options(MachineClass *m)
 {
 pc_q35_machine_options(m);
 m->alias = "q35";
+m->is_default = 1;
 }
 
 DEFINE_Q35_MACHINE(v3_0, "pc-q35-3.0", NULL,
@@ -321,6 +322,7 @@ static void pc_q35_2_12_machine_options(MachineClass *m)
 {
 pc_q35_3_0_machine_options(m);
 m->alias = NULL;
+m->is_default = 0;
 SET_MACHINE_COMPAT(m, PC_COMPAT_2_12);
 }
 
-- 
2.13.6




[Qemu-devel] [Bug 1774853] [NEW] claims temp file is used by another process

2018-06-03 Thread tsiros
Public bug reported:

QEMU emulator version 2.12.50 (v2.12.0-12378-g99a34dc4d2-dirty)

"c:\Program Files\qemu\qemu-system-x86_64.exe" -net none -parallel none -bios 
OVMF.fd -L . -hda fat:rw:image
vvfat image chs 1024,16,63
c:\Program Files\qemu\qemu-system-x86_64.exe: -hda fat:rw:image: Could not open 
'C:\Users\tsiros\AppData\Local\Temp\qem5B92.tmp': The process cannot access the 
file because it is being used by another process.

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

Title:
  claims temp file is used by another process

Status in QEMU:
  New

Bug description:
  QEMU emulator version 2.12.50 (v2.12.0-12378-g99a34dc4d2-dirty)

  "c:\Program Files\qemu\qemu-system-x86_64.exe" -net none -parallel none -bios 
OVMF.fd -L . -hda fat:rw:image
  vvfat image chs 1024,16,63
  c:\Program Files\qemu\qemu-system-x86_64.exe: -hda fat:rw:image: Could not 
open 'C:\Users\tsiros\AppData\Local\Temp\qem5B92.tmp': The process cannot 
access the file because it is being used by another process.

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



[Qemu-devel] [Bug 1774853] Re: claims temp file is used by another process

2018-06-03 Thread tsiros
someone please delete this

** Changed in: qemu
   Status: New => Invalid

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

Title:
  claims temp file is used by another process

Status in QEMU:
  Invalid

Bug description:
  QEMU emulator version 2.12.50 (v2.12.0-12378-g99a34dc4d2-dirty)

  "c:\Program Files\qemu\qemu-system-x86_64.exe" -net none -parallel none -bios 
OVMF.fd -L . -hda fat:rw:image
  vvfat image chs 1024,16,63
  c:\Program Files\qemu\qemu-system-x86_64.exe: -hda fat:rw:image: Could not 
open 'C:\Users\tsiros\AppData\Local\Temp\qem5B92.tmp': The process cannot 
access the file because it is being used by another process.

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



Re: [Qemu-devel] Mistake on man page

2018-06-03 Thread kvaps
Ok, thanks for the clarification.

On Fri, Jun 1, 2018 at 7:17 PM Daniel P. Berrangé 
wrote:

> On Fri, Jun 01, 2018 at 04:47:55PM +0200, kvaps wrote:
> > Hi, here is mistake in man page for qemu-nbd:
> >
> > --- a/qemu-nbd.texi
> > +++ b/qemu-nbd.texi
> > @@ -1,2 +1,2 @@
> > --- a/qemu-nbd.texi
> > -Don't exit on the last connection
> > +++ b/qemu-nbd.texi
> > +Don't exit on the lost connection
>
> It isn't a mistake actually. qemu-nbd can allow multiple concurrent
> client connections, and will exit once the last client closes.
>
> 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 2/2] hw/arm/armv7m: Remove unused armv7m_init() function

2018-06-03 Thread Peter Maydell
On 3 June 2018 at 01:48, Joel Stanley  wrote:
> Hi Peter,
>
> On 2 June 2018 at 00:13, Peter Maydell  wrote:
>> Remove the now-unused armv7m_init() function. This was a legacy from
>> before we properly QOMified ARMv7M, and it has some flaws:
>>
>>  * it combines work that needs to be done by an SoC object (creating
>>and initializing the TYPE_ARMV7M object) with work that needs to
>>be done by the board model (setting the system up to load the ELF
>>file specified with -kernel)
>>  * TYPE_ARMV7M creation failure is fatal, but an SoC object wants to
>>arrange to propagate the failure outward
>>  * it uses allocate-and-create via qdev_create() whereas the current
>>preferred style for SoC objects is to do creation in-place
>>
>> Board and SoC models can instead do the two jobs this function
>> was doing themselves, in the right places and with whatever their
>> preferred style/error handling is.

>> -DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int 
>> num_irq,
>> - const char *kernel_filename, const char *cpu_type)
>> -{
>> -DeviceState *armv7m;
>> -
>> -armv7m = qdev_create(NULL, TYPE_ARMV7M);
>> -qdev_prop_set_uint32(armv7m, "num-irq", num_irq);
>> -qdev_prop_set_string(armv7m, "cpu-type", cpu_type);
>> -object_property_set_link(OBJECT(armv7m), OBJECT(get_system_memory()),
>> - "memory", &error_abort);
>
> It looks like the snippet above is going to be a cut/paste for all v7m 
> machines.

If you look at pretty much all the v7m machines except stellaris,
they don't use this code fragment. (Instead you have an
object_initialize()/qdev_set_parent_bus() in the SoC container's
init function, calls to set properties and realize in the container's
realize, and a call to armv7m_load_kernel() in the board code.)

I agree that what you might call the "modern style" of writing
SoC containers like that has a lot of boilerplate, but that's
not specific to the armv7m object, and if we want to reduce
boilerplate we should look at how we can do it consistently
across devices.

thanks
-- PMM



Re: [Qemu-devel] Cortex M0 emulation tasks

2018-06-03 Thread Liviu Ionescu
On 2 June 2018 at 17:15:15, Stefan Hajnoczi (stefa...@gmail.com) wrote:

> > I have put together a basic Cortex M0 ARMv6-M CPU that can serve
> as
> the basis for this work. Please see the RFC patches that I've sent
> separately.

you can also take a look at the Cortex-M implementation in the GNU MCU
Eclipse QEMU.

https://gnu-mcu-eclipse.github.io/qemu/

it is a bit old now; a refresh is planned, but no date is set yet.


regards,

Liviu



Re: [Qemu-devel] [PATCH v4 04/12] migration: avoid concurrent invoke channel_close by different threads

2018-06-03 Thread 858585 jemmy
On Thu, May 31, 2018 at 6:52 PM, Dr. David Alan Gilbert
 wrote:
> * 858585 jemmy (jemmy858...@gmail.com) wrote:
>> On Wed, May 30, 2018 at 10:45 PM, Dr. David Alan Gilbert
>>  wrote:
>> > * Lidong Chen (jemmy858...@gmail.com) wrote:
>> >> From: Lidong Chen 
>> >>
>> >> The channel_close maybe invoked by different threads. For example, source
>> >> qemu invokes qemu_fclose in main thread, migration thread and return path
>> >> thread. Destination qemu invokes qemu_fclose in main thread, listen thread
>> >> and COLO incoming thread.
>> >>
>> >> Add a mutex in QEMUFile struct to avoid concurrent invoke channel_close.
>> >>
>> >> Signed-off-by: Lidong Chen 
>> >> ---
>> >>  migration/qemu-file.c | 5 +
>> >>  1 file changed, 5 insertions(+)
>> >>
>> >> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
>> >> index 977b9ae..87d0f05 100644
>> >> --- a/migration/qemu-file.c
>> >> +++ b/migration/qemu-file.c
>> >> @@ -52,6 +52,7 @@ struct QEMUFile {
>> >>  unsigned int iovcnt;
>> >>
>> >>  int last_error;
>> >> +QemuMutex lock;
>> >
>> > That could do with a comment saying what you're protecting
>> >
>> >>  };
>> >>
>> >>  /*
>> >> @@ -96,6 +97,7 @@ QEMUFile *qemu_fopen_ops(void *opaque, const 
>> >> QEMUFileOps *ops)
>> >>
>> >>  f = g_new0(QEMUFile, 1);
>> >>
>> >> +qemu_mutex_init(&f->lock);
>> >>  f->opaque = opaque;
>> >>  f->ops = ops;
>> >>  return f;
>> >> @@ -328,7 +330,9 @@ int qemu_fclose(QEMUFile *f)
>> >>  ret = qemu_file_get_error(f);
>> >>
>> >>  if (f->ops->close) {
>> >> +qemu_mutex_lock(&f->lock);
>> >>  int ret2 = f->ops->close(f->opaque);
>> >> +qemu_mutex_unlock(&f->lock);
>> >
>> > OK, and at least for the RDMA code, if it calls
>> > close a 2nd time, rioc->rdma is checked so it wont actually free stuff a
>> > 2nd time.
>> >
>> >>  if (ret >= 0) {
>> >>  ret = ret2;
>> >>  }
>> >> @@ -339,6 +343,7 @@ int qemu_fclose(QEMUFile *f)
>> >>  if (f->last_error) {
>> >>  ret = f->last_error;
>> >>  }
>> >> +qemu_mutex_destroy(&f->lock);
>> >>  g_free(f);
>> >
>> > Hmm but that's not safe; if two things really do call qemu_fclose()
>> > on the same structure they race here and can end up destroying the lock
>> > twice, or doing f->lock  after the 1st one has already g_free(f).
>>
>> >
>> >
>> > So lets go back a step.
>> > I think:
>> >   a) There should always be a separate QEMUFile* for
>> >  to_src_file and from_src_file - I don't see where you open
>> >  the 2nd one; I don't see your implementation of
>> >  f->ops->get_return_path.
>>
>> yes, current qemu version use a separate QEMUFile* for to_src_file and
>> from_src_file.
>> and the two QEMUFile point to one QIOChannelRDMA.
>>
>> the f->ops->get_return_path is implemented by channel_output_ops or
>> channel_input_ops.
>
> Ah OK, yes that makes sense.
>
>> >   b) I *think* that while the different threads might all call
>> >  fclose(), I think there should only ever be one qemu_fclose
>> >  call for each direction on the QEMUFile.
>> >
>> > But now we have two problems:
>> >   If (a) is true then f->lock  is separate on each one so
>> >doesn't really protect if the two directions are closed
>> >at once. (Assuming (b) is true)
>>
>> yes, you are right.  so I should add a QemuMutex in QIOChannel structure, not
>> QEMUFile structure. and qemu_mutex_destroy the QemuMutex in
>> qio_channel_finalize.
>
> OK, that sounds better.
>
> Dave
>

Hi Dave:
Another way is protect channel_close in migration part, like
QemuMutex rp_mutex.
As Daniel mentioned, QIOChannel impls are only intended to a single thread.
https://www.mail-archive.com/qemu-devel@nongnu.org/msg530100.html

which way is better? Does QIOChannel have the plan to support multi thread?
Not only channel_close need lock between different threads,
writev_buffer write also
need.

thanks.


>> Thank you.
>>
>> >
>> >   If (a) is false and we actually share a single QEMUFile then
>> >  that race at the end happens.
>> >
>> > Dave
>> >
>> >
>> >>  trace_qemu_file_fclose();
>> >>  return ret;
>> >> --
>> >> 1.8.3.1
>> >>
>> > --
>> > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [PATCH v4 04/12] migration: avoid concurrent invoke channel_close by different threads

2018-06-03 Thread 858585 jemmy
On Sun, Jun 3, 2018 at 9:50 PM, 858585 jemmy  wrote:
> On Thu, May 31, 2018 at 6:52 PM, Dr. David Alan Gilbert
>  wrote:
>> * 858585 jemmy (jemmy858...@gmail.com) wrote:
>>> On Wed, May 30, 2018 at 10:45 PM, Dr. David Alan Gilbert
>>>  wrote:
>>> > * Lidong Chen (jemmy858...@gmail.com) wrote:
>>> >> From: Lidong Chen 
>>> >>
>>> >> The channel_close maybe invoked by different threads. For example, source
>>> >> qemu invokes qemu_fclose in main thread, migration thread and return path
>>> >> thread. Destination qemu invokes qemu_fclose in main thread, listen 
>>> >> thread
>>> >> and COLO incoming thread.
>>> >>
>>> >> Add a mutex in QEMUFile struct to avoid concurrent invoke channel_close.
>>> >>
>>> >> Signed-off-by: Lidong Chen 
>>> >> ---
>>> >>  migration/qemu-file.c | 5 +
>>> >>  1 file changed, 5 insertions(+)
>>> >>
>>> >> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
>>> >> index 977b9ae..87d0f05 100644
>>> >> --- a/migration/qemu-file.c
>>> >> +++ b/migration/qemu-file.c
>>> >> @@ -52,6 +52,7 @@ struct QEMUFile {
>>> >>  unsigned int iovcnt;
>>> >>
>>> >>  int last_error;
>>> >> +QemuMutex lock;
>>> >
>>> > That could do with a comment saying what you're protecting
>>> >
>>> >>  };
>>> >>
>>> >>  /*
>>> >> @@ -96,6 +97,7 @@ QEMUFile *qemu_fopen_ops(void *opaque, const 
>>> >> QEMUFileOps *ops)
>>> >>
>>> >>  f = g_new0(QEMUFile, 1);
>>> >>
>>> >> +qemu_mutex_init(&f->lock);
>>> >>  f->opaque = opaque;
>>> >>  f->ops = ops;
>>> >>  return f;
>>> >> @@ -328,7 +330,9 @@ int qemu_fclose(QEMUFile *f)
>>> >>  ret = qemu_file_get_error(f);
>>> >>
>>> >>  if (f->ops->close) {
>>> >> +qemu_mutex_lock(&f->lock);
>>> >>  int ret2 = f->ops->close(f->opaque);
>>> >> +qemu_mutex_unlock(&f->lock);
>>> >
>>> > OK, and at least for the RDMA code, if it calls
>>> > close a 2nd time, rioc->rdma is checked so it wont actually free stuff a
>>> > 2nd time.
>>> >
>>> >>  if (ret >= 0) {
>>> >>  ret = ret2;
>>> >>  }
>>> >> @@ -339,6 +343,7 @@ int qemu_fclose(QEMUFile *f)
>>> >>  if (f->last_error) {
>>> >>  ret = f->last_error;
>>> >>  }
>>> >> +qemu_mutex_destroy(&f->lock);
>>> >>  g_free(f);
>>> >
>>> > Hmm but that's not safe; if two things really do call qemu_fclose()
>>> > on the same structure they race here and can end up destroying the lock
>>> > twice, or doing f->lock  after the 1st one has already g_free(f).
>>>
>>> >
>>> >
>>> > So lets go back a step.
>>> > I think:
>>> >   a) There should always be a separate QEMUFile* for
>>> >  to_src_file and from_src_file - I don't see where you open
>>> >  the 2nd one; I don't see your implementation of
>>> >  f->ops->get_return_path.
>>>
>>> yes, current qemu version use a separate QEMUFile* for to_src_file and
>>> from_src_file.
>>> and the two QEMUFile point to one QIOChannelRDMA.
>>>
>>> the f->ops->get_return_path is implemented by channel_output_ops or
>>> channel_input_ops.
>>
>> Ah OK, yes that makes sense.
>>
>>> >   b) I *think* that while the different threads might all call
>>> >  fclose(), I think there should only ever be one qemu_fclose
>>> >  call for each direction on the QEMUFile.
>>> >
>>> > But now we have two problems:
>>> >   If (a) is true then f->lock  is separate on each one so
>>> >doesn't really protect if the two directions are closed
>>> >at once. (Assuming (b) is true)
>>>
>>> yes, you are right.  so I should add a QemuMutex in QIOChannel structure, 
>>> not
>>> QEMUFile structure. and qemu_mutex_destroy the QemuMutex in
>>> qio_channel_finalize.
>>
>> OK, that sounds better.
>>
>> Dave
>>
>
> Hi Dave:
> Another way is protect channel_close in migration part, like
> QemuMutex rp_mutex.
> As Daniel mentioned, QIOChannel impls are only intended to a single 
> thread.
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg530100.html
>
> which way is better? Does QIOChannel have the plan to support multi 
> thread?
> Not only channel_close need lock between different threads,
> writev_buffer write also
> need.
>
> thanks.
>
>

I find qemu not call qemu_mutex_destroy to release rp_mutex in
migration_instance_finalize:(
although qemu_mutex_destroy is not necceesary, but it is a good practice to do.
it's better we fixed it.

>>> Thank you.
>>>
>>> >
>>> >   If (a) is false and we actually share a single QEMUFile then
>>> >  that race at the end happens.
>>> >
>>> > Dave
>>> >
>>> >
>>> >>  trace_qemu_file_fclose();
>>> >>  return ret;
>>> >> --
>>> >> 1.8.3.1
>>> >>
>>> > --
>>> > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
>> --
>> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [PATCH v4 11/12] migration: poll the cm event while wait RDMA work request completion

2018-06-03 Thread Aviad Yehezkel

+Gal

Gal, please comment with our findings.

Thanks!


On 5/31/2018 10:36 AM, 858585 jemmy wrote:

On Thu, May 31, 2018 at 1:33 AM, Dr. David Alan Gilbert
 wrote:

* Lidong Chen (jemmy858...@gmail.com) wrote:

If the peer qemu is crashed, the qemu_rdma_wait_comp_channel function
maybe loop forever. so we should also poll the cm event fd, and when
receive any cm event, we consider some error happened.

Signed-off-by: Lidong Chen 

I don't understand enough about the way the infiniband fd's work to
fully review this; so I'd appreciate if some one who does could
comment/add their review.

Hi Avaid:
 we need your help. I also not find any document about the cq
channel event fd and
cm channel event f.
 Should we set the events to G_IO_IN | G_IO_HUP | G_IO_ERR? or
G_IO_IN is enough?
 pfds[0].events = G_IO_IN | G_IO_HUP | G_IO_ERR;
 pfds[1].events = G_IO_IN | G_IO_HUP | G_IO_ERR;
 Thanks.


---
  migration/rdma.c | 35 ---
  1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 1b9e261..d611a06 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -1489,6 +1489,9 @@ static uint64_t qemu_rdma_poll(RDMAContext *rdma, 
uint64_t *wr_id_out,
   */
  static int qemu_rdma_wait_comp_channel(RDMAContext *rdma)
  {
+struct rdma_cm_event *cm_event;
+int ret = -1;
+
  /*
   * Coroutine doesn't start until migration_fd_process_incoming()
   * so don't yield unless we know we're running inside of a coroutine.
@@ -1504,25 +1507,35 @@ static int qemu_rdma_wait_comp_channel(RDMAContext 
*rdma)
   * But we need to be able to handle 'cancel' or an error
   * without hanging forever.
   */
-while (!rdma->error_state  && !rdma->received_error) {
-GPollFD pfds[1];
+while (!rdma->error_state && !rdma->received_error) {
+GPollFD pfds[2];
  pfds[0].fd = rdma->comp_channel->fd;
  pfds[0].events = G_IO_IN | G_IO_HUP | G_IO_ERR;
+pfds[0].revents = 0;
+
+pfds[1].fd = rdma->channel->fd;
+pfds[1].events = G_IO_IN | G_IO_HUP | G_IO_ERR;
+pfds[1].revents = 0;
+
  /* 0.1s timeout, should be fine for a 'cancel' */
-switch (qemu_poll_ns(pfds, 1, 100 * 1000 * 1000)) {
-case 1: /* fd active */
-return 0;
+qemu_poll_ns(pfds, 2, 100 * 1000 * 1000);

Shouldn't we still check the return value of this; if it's negative
something has gone wrong.

I will fix this.
Thanks.


Dave


-case 0: /* Timeout, go around again */
-break;
+if (pfds[1].revents) {
+ret = rdma_get_cm_event(rdma->channel, &cm_event);
+if (!ret) {
+rdma_ack_cm_event(cm_event);
+}
+error_report("receive cm event while wait comp channel,"
+ "cm event is %d", cm_event->event);

-default: /* Error of some type -
-  * I don't trust errno from qemu_poll_ns
- */
-error_report("%s: poll failed", __func__);
+/* consider any rdma communication event as an error */
  return -EPIPE;
  }

+if (pfds[0].revents) {
+return 0;
+}
+
  if (migrate_get_current()->state == MIGRATION_STATUS_CANCELLING) {
  /* Bail out and let the cancellation happen */
  return -EPIPE;
--
1.8.3.1


--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK





Re: [Qemu-devel] Virtio-net drivers immune to Nethammer?

2018-06-03 Thread Michael S. Tsirkin
On Sat, Jun 02, 2018 at 03:08:54AM +, procmem wrote:
> 
> 
> Michael S. Tsirkin:
> > On Fri, Jun 01, 2018 at 01:15:44PM +, procmem wrote:
> >>
> >>
> >> Stefan Hajnoczi:
> >>> On Mon, May 21, 2018 at 11:24:43PM +, procmem wrote:
>  Hi I'm a privacy distro maintainer investigating the implications of the
>  newly published nethammer attack [0] on KVM guests particularly the
>  virtio-net drivers. The summary of the paper is that rowhammer can be
>  remotely triggered by feeding susceptible* network driver crafted
>  traffic. This attack can do all kinds of nasty things such as modifying
>  SSL certs on the victim system.
> 
>  * Susceptible drivers are those relying on Intel CAT, uncached memory or
>  the clflush instruction.
> 
>  My question is, do virtio-net drivers do any of these things?
> >>> I have CCed Michael Tsirkin and Jason Wang, the virtio maintainers.
> >>>
>  ***
> 
>  [0] https://arxiv.org/abs/1805.04956
> 
> 
> 
> >>
> >> Thanks :) I thought my message was forgotten
> > 
> > 
> > I don't think virtio is using either of these.
> > 
> > Linux does support CAT AFAIK but it has nothing to do with virtio.
> > 
> 
> Thanks for confirming. This is good news indeed. I am considering
> posting about this to kernel-hardening so it's on the sec team's radar
> when considering upstream network drivers. What do you think?

It's up to you but the usefulness of reposting like that will be limited IMHO,
unless you have something specific to add.

I think everyone saw the nethammer paper by now, and kernel hardening
team doesn't review network driver patches.

-- 
MST



Re: [Qemu-devel] [PATCH 1/8] virtio: feature bit, data structure for packed ring

2018-06-03 Thread Wei Xu
On Tue, Apr 10, 2018 at 03:05:24PM +0800, Jason Wang wrote:
> 
> 
> On 2018年04月04日 20:53, w...@redhat.com wrote:
> >From: Wei Xu 
> >
> >Only minimum definitions from the spec are included
> >for prototype.
> >
> >Signed-off-by: Wei Xu 
> >---
> >  hw/virtio/virtio.c | 47 
> > +++---
> >  include/hw/virtio/virtio.h | 12 ++-
> >  include/standard-headers/linux/virtio_config.h |  2 ++
> >  3 files changed, 56 insertions(+), 5 deletions(-)
> >
> >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> >index 006d3d1..9a6bfe7 100644
> >--- a/hw/virtio/virtio.c
> >+++ b/hw/virtio/virtio.c
> >@@ -39,6 +39,14 @@ typedef struct VRingDesc
> >  uint16_t next;
> >  } VRingDesc;
> >+typedef struct VRingDescPacked
> >+{
> >+uint64_t addr;
> >+uint32_t len;
> >+uint16_t id;
> >+uint16_t flags;
> >+} VRingDescPacked;
> >+
> >  typedef struct VRingAvail
> >  {
> >  uint16_t flags;
> >@@ -61,9 +69,18 @@ typedef struct VRingUsed
> >  typedef struct VRingMemoryRegionCaches {
> >  struct rcu_head rcu;
> >-MemoryRegionCache desc;
> >-MemoryRegionCache avail;
> >-MemoryRegionCache used;
> >+union {
> >+struct {
> >+MemoryRegionCache desc;
> >+MemoryRegionCache avail;
> >+MemoryRegionCache used;
> >+};
> >+struct {
> >+MemoryRegionCache desc_packed;
> >+MemoryRegionCache driver;
> >+MemoryRegionCache device;
> >+};
> >+};
> 
> I think we can reuse exist memory region caches? Especially consider
> device/driver area could be treated as a renaming of avail/used area.
> 
> E.g desc for desc_packed, avail for driver area and used for device area.

Yes, I will take it.

> 
> >  } VRingMemoryRegionCaches;
> >  typedef struct VRing
> >@@ -77,10 +94,31 @@ typedef struct VRing
> >  VRingMemoryRegionCaches *caches;
> >  } VRing;
> >+typedef struct VRingPackedDescEvent {
> >+uint16_t desc_event_off:15,
> >+ desc_event_wrap:1;
> >+uint16_t desc_event_flags:2;
> >+} VRingPackedDescEvent ;
> >+
> >+typedef struct VRingPacked
> >+{
> >+unsigned int num;
> >+unsigned int num_default;
> >+unsigned int align;
> >+hwaddr desc;
> >+hwaddr driver;
> >+hwaddr device;
> >+VRingMemoryRegionCaches *caches;
> >+} VRingPacked;
> 
> Same here, can we reuse VRing here?

Yes.

> 
> >+
> >  struct VirtQueue
> >  {
> >-VRing vring;
> >+union {
> >+struct VRing vring;
> >+struct VRingPacked packed;
> >+};
> >+uint8_t wrap_counter:1;
> >  /* Next head to pop */
> >  uint16_t last_avail_idx;
> >@@ -1220,6 +1258,7 @@ void virtio_reset(void *opaque)
> >  vdev->vq[i].vring.num = vdev->vq[i].vring.num_default;
> >  vdev->vq[i].inuse = 0;
> >  virtio_virtqueue_reset_region_cache(&vdev->vq[i]);
> >+vdev->vq[i].wrap_counter = 1;
> >  }
> >  }
> >diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> >index 098bdaa..563e88e 100644
> >--- a/include/hw/virtio/virtio.h
> >+++ b/include/hw/virtio/virtio.h
> >@@ -46,6 +46,14 @@ typedef struct VirtQueueElement
> >  unsigned int index;
> >  unsigned int out_num;
> >  unsigned int in_num;
> >+
> >+/* Number of descriptors used by packed ring */
> 
> Do you mean the number of chained descriptors?

These has been removed.

> 
> >+uint16_t count;
> >+uint8_t wrap_counter:1;
> 
> What's the use of this bit? If you refer to my v1 vhost code, I used to have
> this, but it won't work for OOO completion e.g when zerocopy is disabled.
> I've dropped it now.
> 
> This is tricky and can only work when device complete descriptors in order.

Same here.

> 
> >+/* FIXME: Length of every used buffer for a descriptor,
> >+   move to dynamical allocating due to out/in sgs numbers */
> >+uint32_t len[VIRTQUEUE_MAX_SIZE];
> 
> Can you explain more about this?

Also here.

> 
> >+
> >  hwaddr *in_addr;
> >  hwaddr *out_addr;
> >  struct iovec *in_sg;
> >@@ -262,7 +270,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
> >  DEFINE_PROP_BIT64("any_layout", _state, _field, \
> >VIRTIO_F_ANY_LAYOUT, true), \
> >  DEFINE_PROP_BIT64("iommu_platform", _state, _field, \
> >-  VIRTIO_F_IOMMU_PLATFORM, false)
> >+  VIRTIO_F_IOMMU_PLATFORM, false), \
> >+DEFINE_PROP_BIT64("ring_packed", _state, _field, \
> >+  VIRTIO_F_RING_PACKED, true)
> 
> Remember to disable this for old machine types in next version.

Sure, will do.

> 
> Thanks
> 
> >  hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
> >  hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
> >diff --git a/include/standard-headers/linux/virtio_config.h 
> >b/include/standard-headers/linux/virtio_config.h
> >index b777069..6ee5529 100644
> >--- a/include/standard-headers/linux/virti

[Qemu-devel] [Bug 1754542] Re: colo: vm crash with segmentation fault

2018-06-03 Thread Zhang Chen
Hi Suiheng,

This bug have been fixed in my latest patch.
Please retest it.
https://www.mail-archive.com/qemu-devel@nongnu.org/msg538383.html

github:
https://github.com/zhangckid/qemu/tree/qemu-colo-18jun1

Thanks
Zhang Chen

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

Title:
  colo:  vm crash with segmentation fault

Status in QEMU:
  New

Bug description:
  I use Arch Linux x86_64
  Zhang Chen's(https://github.com/zhangckid/qemu/tree/qemu-colo-18mar10)
  Following document 'COLO-FT.txt',
  I test colo feature on my hosts

  I run this command
  Primary:
  sudo /usr/local/bin/qemu-system-x86_64 -enable-kvm -m 2048 -smp 2 -qmp stdio 
-name primary \
  -device piix3-usb-uhci \
  -device usb-tablet -netdev tap,id=hn0,vhost=off \
  -device virtio-net-pci,id=net-pci0,netdev=hn0 \
  -drive 
if=virtio,id=primary-disk0,driver=quorum,read-pattern=fifo,vote-threshold=1,\
  children.0.file.filename=/var/lib/libvirt/images/1.raw,\
  children.0.driver=raw -S

  Secondary:
  sudo /usr/local/bin/qemu-system-x86_64 -enable-kvm -m 2048 -smp 2 -qmp stdio 
-name secondary \
  -device piix3-usb-uhci \
  -device usb-tablet -netdev tap,id=hn0,vhost=off \
  -device virtio-net-pci,id=net-pci0,netdev=hn0 \
  -drive 
if=none,id=secondary-disk0,file.filename=/var/lib/libvirt/images/2.raw,driver=raw,node-name=node0
 \
  -drive if=virtio,id=active-disk0,driver=replication,mode=secondary,\
  file.driver=qcow2,top-id=active-disk0,\
  file.file.filename=/mnt/ramfs/active_disk.img,\
  file.backing.driver=qcow2,\
  file.backing.file.filename=/mnt/ramfs/hidden_disk.img,\
  file.backing.backing=secondary-disk0 \
  -incoming tcp:0:

  Secondary:
  {'execute':'qmp_capabilities'}
  { 'execute': 'nbd-server-start',
'arguments': {'addr': {'type': 'inet', 'data': {'host': '192.168.0.34', 
'port': '8889'} } }
  }
  {'execute': 'nbd-server-add', 'arguments': {'device': 'secondary-disk0', 
'writable': true } }

  Primary:
  {'execute':'qmp_capabilities'}
  { 'execute': 'human-monitor-command',
'arguments': {'command-line': 'drive_add -n buddy 
driver=replication,mode=primary,file.driver=nbd,file.host=192.168.0.34,file.port=8889,file.export=secondary-disk0,node-name=nbd_client0'}}
  { 'execute':'x-blockdev-change', 'arguments':{'parent': 'primary-disk0', 
'node': 'nbd_client0' } }
  { 'execute': 'migrate-set-capabilities',
'arguments': {'capabilities': [ {'capability': 'x-colo', 'state': true 
} ] } }
  { 'execute': 'migrate', 'arguments': {'uri': 'tcp:192.168.0.34:' } }
  And two VM with cash
  Primary:
  {"timestamp": {"seconds": 1520763655, "microseconds": 511415}, "event": 
"RESUME"}
  [1]329 segmentation fault  sudo /usr/local/bin/qemu-system-x86_64 -boot c 
-enable-kvm -m 2048 -smp 2 -qm

  Secondary:
  {"timestamp": {"seconds": 1520763655, "microseconds": 510907}, "event": 
"RESUME"}
  [1]367 segmentation fault  sudo /usr/local/bin/qemu-system-x86_64 -boot c 
-enable-kvm -m 2048 -smp 2 -qm

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



Re: [Qemu-devel] [PATCH 3/8] virtio: add empty check for packed ring

2018-06-03 Thread Wei Xu
On Tue, Apr 10, 2018 at 03:23:03PM +0800, Jason Wang wrote:
> 
> 
> On 2018年04月04日 20:53, w...@redhat.com wrote:
> >From: Wei Xu 
> >
> >helper for ring empty check.
> 
> And descriptor read.

OK.

> 
> >
> >Signed-off-by: Wei Xu 
> >---
> >  hw/virtio/virtio.c | 62 
> > +++---
> >  1 file changed, 59 insertions(+), 3 deletions(-)
> >
> >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> >index 73a35a4..478df3d 100644
> >--- a/hw/virtio/virtio.c
> >+++ b/hw/virtio/virtio.c
> >@@ -24,6 +24,9 @@
> >  #include "hw/virtio/virtio-access.h"
> >  #include "sysemu/dma.h"
> >+#define AVAIL_DESC_PACKED(b) ((b) << 7)
> >+#define USED_DESC_PACKED(b)  ((b) << 15)
> 
> Can we pass value other than 1 to this macro?

Yes, '0' is also provided for some clear/reset cases.

> 
> >+
> >  /*
> >   * The alignment to use between consumer and producer parts of vring.
> >   * x86 pagesize again. This is the default, used by transports like PCI
> >@@ -446,10 +449,27 @@ int virtio_queue_ready(VirtQueue *vq)
> >  return vq->vring.avail != 0;
> >  }
> >+static void vring_desc_read_packed(VirtIODevice *vdev, VRingDescPacked 
> >*desc,
> >+MemoryRegionCache *cache, int i)
> >+{
> >+address_space_read_cached(cache, i * sizeof(VRingDescPacked),
> >+  desc, sizeof(VRingDescPacked));
> >+virtio_tswap64s(vdev, &desc->addr);
> >+virtio_tswap32s(vdev, &desc->len);
> >+virtio_tswap16s(vdev, &desc->id);
> >+virtio_tswap16s(vdev, &desc->flags);
> >+}
> >+
> >+static inline bool is_desc_avail(struct VRingDescPacked* desc)
> >+{
> >+return (!!(desc->flags & AVAIL_DESC_PACKED(1)) !=
> >+!!(desc->flags & USED_DESC_PACKED(1)));
> 
> Don't we need to care about endian here?

Usually we don't since endian has been converted during reading,
will double check it.

> 
> >+}
> >+
> >  /* Fetch avail_idx from VQ memory only when we really need to know if
> >   * guest has added some buffers.
> >   * Called within rcu_read_lock().  */
> >-static int virtio_queue_empty_rcu(VirtQueue *vq)
> >+static int virtio_queue_empty_split_rcu(VirtQueue *vq)
> >  {
> >  if (unlikely(!vq->vring.avail)) {
> >  return 1;
> >@@ -462,7 +482,7 @@ static int virtio_queue_empty_rcu(VirtQueue *vq)
> >  return vring_avail_idx(vq) == vq->last_avail_idx;
> >  }
> >-int virtio_queue_empty(VirtQueue *vq)
> >+static int virtio_queue_empty_split(VirtQueue *vq)
> >  {
> >  bool empty;
> >@@ -480,6 +500,42 @@ int virtio_queue_empty(VirtQueue *vq)
> >  return empty;
> >  }
> >+static int virtio_queue_empty_packed_rcu(VirtQueue *vq)
> >+{
> >+struct VRingDescPacked desc;
> >+VRingMemoryRegionCaches *cache;
> >+
> >+if (unlikely(!vq->packed.desc)) {
> >+return 1;
> >+}
> >+
> >+cache = vring_get_region_caches(vq);
> >+vring_desc_read_packed(vq->vdev, &desc, &cache->desc_packed, 
> >vq->last_avail_idx);
> >+
> >+/* Make sure we see the updated flag */
> >+smp_mb();
> 
> What we need here is to make sure flag is read before all other fields,
> looks like this barrier can't.

Isn't flag updated yet if it has been read?

> 
> >+return !is_desc_avail(&desc);
> >+}
> >+
> >+static int virtio_queue_empty_packed(VirtQueue *vq)
> >+{
> >+bool empty;
> >+
> >+rcu_read_lock();
> >+empty = virtio_queue_empty_packed_rcu(vq);
> >+rcu_read_unlock();
> >+return empty;
> >+}
> >+
> >+int virtio_queue_empty(VirtQueue *vq)
> >+{
> >+if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
> >+return virtio_queue_empty_packed(vq);
> >+} else {
> >+return virtio_queue_empty_split(vq);
> >+}
> >+}
> >+
> >  static void virtqueue_unmap_sg(VirtQueue *vq, const VirtQueueElement *elem,
> > unsigned int len)
> >  {
> >@@ -951,7 +1007,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
> >  return NULL;
> >  }
> >  rcu_read_lock();
> >-if (virtio_queue_empty_rcu(vq)) {
> >+if (virtio_queue_empty_split_rcu(vq)) {
> 
> I think you'd better have a switch inside virtio_queue_empty_rcu() like
> virtio_queue_empty() here.

OK.

> 
> Thanks
> 
> >  goto done;
> >  }
> >  /* Needed after virtio_queue_empty(), see comment in
> 



[Qemu-devel] [RFC v2] qapi: command category to stimulate high-level machine devices

2018-06-03 Thread Steffen Görtz
Changes since v1:
  - Fix coding style issues

Add a new category "stimulate" to host commands that
act upon high-level devices associated with machines/boards.

This is patch is part of an effort to add support for BBC:microbit
educational computer to QEMU.

The microbit board, as many other microcontroller boards,
contains typical trivial (digital) input and output options such as buttons and 
LEDs,
but also more sophiscated possibilities such as analog inputs and
complex dedicated sensor ICs that are connected to the microbit machine
by means of inter-ic bus (i2c).
These devices interact with peripheral devices of the microcontroller
in use, for example with the GPIO peripheral of the used NRF51.

One aim of our efforts is to create a user interface that models the
look and feel of the physical microbit board and lets the user interact
with its inputs and provide feedback on its outsputs.

For this it is necessary to transmit user inputs such as the pressing of a
button to the machine.
In return, when the state of an output is changed on the machine, this
change needs to be reflected in the user interface.

At the moment, there are only a few QMP commands that provide user input to the
machine, eg. send-keys, input-button. These commands require very
high-level concepts, which are not really suitable for applications that
microcontrollers are typically used in in.

This RFC is meant to start a discussion on how to provide stimuli to
low-complex inputs and outputs typically found in embedded
microcontroller applications. To the best of my knowledge, there are
currently no examples of how to provide such stimuli to either SoC
device peripheral or machine level concepts like pushbuttons and LEDs.

To my understanding, the following concepts/apis are needed:
- QMP commands to send stimuli to machine level concepts like
pushbuttons
- Machine level devices that receive these stimuli and act as an adapter
to manipulate the associated SoC peripheral devices
- For outputs, machine level devices are needed that observe changes in
SoC peripherals and emit QMP events that clients can receive

This patch adds a new qmp command "buttons-set-state" to update the
push-down state of arbitrarily identified buttons (string identifier).

The handler of this command is mocked to set the machines SoC gpio/irq
lines associated with these buttons on the machine by means of object
property aliases. This is just a mock until a proper concept/api is
agreed upon.

As i recently joined the QEMU community as part of GSoC, i am still a
greenhorn to the codebase and i am really excited to discuss potential
concepts and APIs to realize the described features.

Steffen

Based-on: <20180503090532.3113-1-j...@jms.id.au>
Signed-off-by: Steffen Goertz 
---
 Makefile  |  8 +++
 Makefile.objs |  4 
 Makefile.target   |  1 +
 hw/arm/microbit.c |  7 ++
 qapi/qapi-schema.json |  1 +
 qapi/stimulate.json   | 24 
 stimulate.c   | 52 +++
 7 files changed, 97 insertions(+)
 create mode 100644 qapi/stimulate.json
 create mode 100644 stimulate.c

diff --git a/Makefile b/Makefile
index d71dd5bea4..d9319efa68 100644
--- a/Makefile
+++ b/Makefile
@@ -108,6 +108,7 @@ GENERATED_FILES += qapi/qapi-types-tpm.h 
qapi/qapi-types-tpm.c
 GENERATED_FILES += qapi/qapi-types-trace.h qapi/qapi-types-trace.c
 GENERATED_FILES += qapi/qapi-types-transaction.h qapi/qapi-types-transaction.c
 GENERATED_FILES += qapi/qapi-types-ui.h qapi/qapi-types-ui.c
+GENERATED_FILES += qapi/qapi-types-stimulate.h qapi/qapi-types-stimulate.c
 GENERATED_FILES += qapi/qapi-builtin-visit.h qapi/qapi-builtin-visit.c
 GENERATED_FILES += qapi/qapi-visit.h qapi/qapi-visit.c
 GENERATED_FILES += qapi/qapi-visit-block-core.h qapi/qapi-visit-block-core.c
@@ -126,6 +127,7 @@ GENERATED_FILES += qapi/qapi-visit-tpm.h 
qapi/qapi-visit-tpm.c
 GENERATED_FILES += qapi/qapi-visit-trace.h qapi/qapi-visit-trace.c
 GENERATED_FILES += qapi/qapi-visit-transaction.h qapi/qapi-visit-transaction.c
 GENERATED_FILES += qapi/qapi-visit-ui.h qapi/qapi-visit-ui.c
+GENERATED_FILES += qapi/qapi-visit-stimulate.h qapi/qapi-visit-stimulate.c
 GENERATED_FILES += qapi/qapi-commands.h qapi/qapi-commands.c
 GENERATED_FILES += qapi/qapi-commands-block-core.h 
qapi/qapi-commands-block-core.c
 GENERATED_FILES += qapi/qapi-commands-block.h qapi/qapi-commands-block.c
@@ -143,6 +145,7 @@ GENERATED_FILES += qapi/qapi-commands-tpm.h 
qapi/qapi-commands-tpm.c
 GENERATED_FILES += qapi/qapi-commands-trace.h qapi/qapi-commands-trace.c
 GENERATED_FILES += qapi/qapi-commands-transaction.h 
qapi/qapi-commands-transaction.c
 GENERATED_FILES += qapi/qapi-commands-ui.h qapi/qapi-commands-ui.c
+GENERATED_FILES += qapi/qapi-commands-stimulate.h 
qapi/qapi-commands-stimulate.c
 GENERATED_FILES += qapi/qapi-events.h qapi/qapi-events.c
 GENERATED_FILES += qapi/qapi-events-block-core.h qapi/qapi-events-block-core.c
 GENERATED_FILES += qapi/qap

Re: [Qemu-devel] [RFC v2] qapi: command category to stimulate high-level machine devices

2018-06-03 Thread no-reply
Hi,

This series failed docker-mingw@fedora build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 20180603192242.17675-1-cont...@steffen-goertz.de
Subject: [Qemu-devel] [RFC v2] qapi: command category to stimulate high-level 
machine devices

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
148ce700a2 qapi: command category to stimulate high-level machine devices

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-nuhs55kf/src/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
  BUILD   fedora
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-nuhs55kf/src'
  GEN 
/var/tmp/patchew-tester-tmp-nuhs55kf/src/docker-src.2018-06-03-15.27.49.30327/qemu.tar
Cloning into 
'/var/tmp/patchew-tester-tmp-nuhs55kf/src/docker-src.2018-06-03-15.27.49.30327/qemu.tar.vroot'...
done.
Checking out files:  49% (3040/6116)   
Checking out files:  50% (3058/6116)   
Checking out files:  51% (3120/6116)   
Checking out files:  52% (3181/6116)   
Checking out files:  53% (3242/6116)   
Checking out files:  54% (3303/6116)   
Checking out files:  55% (3364/6116)   
Checking out files:  56% (3425/6116)   
Checking out files:  57% (3487/6116)   
Checking out files:  58% (3548/6116)   
Checking out files:  59% (3609/6116)   
Checking out files:  60% (3670/6116)   
Checking out files:  61% (3731/6116)   
Checking out files:  62% (3792/6116)   
Checking out files:  63% (3854/6116)   
Checking out files:  64% (3915/6116)   
Checking out files:  65% (3976/6116)   
Checking out files:  66% (4037/6116)   
Checking out files:  67% (4098/6116)   
Checking out files:  68% (4159/6116)   
Checking out files:  69% (4221/6116)   
Checking out files:  70% (4282/6116)   
Checking out files:  71% (4343/6116)   
Checking out files:  72% (4404/6116)   
Checking out files:  73% (4465/6116)   
Checking out files:  74% (4526/6116)   
Checking out files:  75% (4587/6116)   
Checking out files:  76% (4649/6116)   
Checking out files:  77% (4710/6116)   
Checking out files:  78% (4771/6116)   
Checking out files:  79% (4832/6116)   
Checking out files:  80% (4893/6116)   
Checking out files:  81% (4954/6116)   
Checking out files:  82% (5016/6116)   
Checking out files:  83% (5077/6116)   
Checking out files:  84% (5138/6116)   
Checking out files:  85% (5199/6116)   
Checking out files:  86% (5260/6116)   
Checking out files:  87% (5321/6116)   
Checking out files:  88% (5383/6116)   
Checking out files:  89% (5444/6116)   
Checking out files:  90% (5505/6116)   
Checking out files:  91% (5566/6116)   
Checking out files:  92% (5627/6116)   
Checking out files:  93% (5688/6116)   
Checking out files:  94% (5750/6116)   
Checking out files:  95% (5811/6116)   
Checking out files:  96% (5872/6116)   
Checking out files:  97% (5933/6116)   
Checking out files:  98% (5994/6116)   
Checking out files:  98% (6036/6116)   
Checking out files:  99% (6055/6116)   
Checking out files: 100% (6116/6116)   
Checking out files: 100% (6116/6116), done.
Your branch is up-to-date with 'origin/test'.
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 
'/var/tmp/patchew-tester-tmp-nuhs55kf/src/docker-src.2018-06-03-15.27.49.30327/qemu.tar.vroot/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered 
for path 'ui/keycodemapdb'
Cloning into 
'/var/tmp/patchew-tester-tmp-nuhs55kf/src/docker-src.2018-06-03-15.27.49.30327/qemu.tar.vroot/ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out 
'6b3d716e2b6472eb7189d3220552280ef3d832ce'
  COPYRUNNER
RUN test-mingw in qemu:fedora 
Packages installed:
PyYAML-3.12-5.fc27.x86_64
SDL2-devel-2.0.7-2.fc27.x86_64
bc-1.07.1-3.fc27.x86_64
bison-3.0.4-8.fc27.x86_64
bluez-libs-devel-5.48-3.fc27.x86_64
brlapi-devel-0.6.6-8.fc27.x86_64
bzip2-1.0.6-24.fc27.x86_64
bzip2-devel-1.0.6-24.fc27.x86_64
ccache-3.3.6-1.fc27.x86_64
clang-5.0.1-5.fc27.x86_64
device-mapper-multipath-devel-0.7.1-9.git847cc43.fc27.x86_64
findutils-4.6.0-16.fc27.x86_64
flex-2.6.1-5.fc27.x86_64
gcc-7.3.1-5.fc27.x86_64
gcc-c++-7.3.1-5.fc27.x86_64
gettext-0.19.8.1-12.fc27.x86_64
git-2.14.3-3.fc27.x86_64
glib2-devel-2.54.3-2.fc27.x86_64
glusterfs-api-devel-3.12.7-1.fc27.x86_64
gnutls-devel-3.5.18-2.fc27.x86_64
gtk3-devel-3.22.26-2.fc27.x86_64
hostname-3.18-4.fc27.x86_64
libaio-devel-0.3.110-9.fc27.x86_64
libasan-7.3.1-5.fc27.x86_64
libattr-devel-2.4.47-21.fc27.x86_64
libcap-devel-2.25-7.fc27.x86_64
libcap-ng-devel-0.7.8-5.fc27.x86_64
libcurl-d

Re: [Qemu-devel] [RFC v2] qapi: command category to stimulate high-level machine devices

2018-06-03 Thread no-reply
Hi,

This series failed build test on s390x host. Please find the details below.

Type: series
Message-id: 20180603192242.17675-1-cont...@steffen-goertz.de
Subject: [Qemu-devel] [RFC v2] qapi: command category to stimulate high-level 
machine devices

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
set -e
echo "=== ENV ==="
env
echo "=== PACKAGES ==="
rpm -qa
echo "=== TEST BEGIN ==="
CC=$HOME/bin/cc
INSTALL=$PWD/install
BUILD=$PWD/build
echo -n "Using CC: "
realpath $CC
mkdir -p $BUILD $INSTALL
SRC=$PWD
cd $BUILD
$SRC/configure --cc=$CC --prefix=$INSTALL
make -j4
# XXX: we need reliable clean up
# make check -j4 V=1
make install
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]   
patchew/20180603192242.17675-1-cont...@steffen-goertz.de -> 
patchew/20180603192242.17675-1-cont...@steffen-goertz.de
Switched to a new branch 'test'
148ce700a2 qapi: command category to stimulate high-level machine devices

=== OUTPUT BEGIN ===
=== ENV ===
LANG=en_US.UTF-8
XDG_SESSION_ID=214891
USER=fam
PWD=/var/tmp/patchew-tester-tmp-un92ne0p/src
HOME=/home/fam
SHELL=/bin/sh
SHLVL=2
PATCHEW=/home/fam/patchew/patchew-cli -s http://patchew.org --nodebug
LOGNAME=fam
DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1012/bus
XDG_RUNTIME_DIR=/run/user/1012
PATH=/usr/bin:/bin
_=/usr/bin/env
=== PACKAGES ===
gpg-pubkey-873529b8-54e386ff
glibc-debuginfo-common-2.24-10.fc25.s390x
fedora-release-26-1.noarch
dejavu-sans-mono-fonts-2.35-4.fc26.noarch
xemacs-filesystem-21.5.34-22.20170124hgf412e9f093d4.fc26.noarch
bash-4.4.12-7.fc26.s390x
libSM-1.2.2-5.fc26.s390x
libmpc-1.0.2-6.fc26.s390x
libaio-0.3.110-7.fc26.s390x
libverto-0.2.6-7.fc26.s390x
perl-Scalar-List-Utils-1.48-1.fc26.s390x
iptables-libs-1.6.1-2.fc26.s390x
tcl-8.6.6-2.fc26.s390x
libxshmfence-1.2-4.fc26.s390x
expect-5.45-23.fc26.s390x
perl-Thread-Queue-3.12-1.fc26.noarch
perl-encoding-2.19-6.fc26.s390x
keyutils-1.5.10-1.fc26.s390x
gmp-devel-6.1.2-4.fc26.s390x
enchant-1.6.0-16.fc26.s390x
python-gobject-base-3.24.1-1.fc26.s390x
python3-enchant-1.6.10-1.fc26.noarch
python-lockfile-0.11.0-6.fc26.noarch
python2-pyparsing-2.1.10-3.fc26.noarch
python2-lxml-4.1.1-1.fc26.s390x
librados2-10.2.7-2.fc26.s390x
trousers-lib-0.3.13-7.fc26.s390x
libdatrie-0.2.9-4.fc26.s390x
libsoup-2.58.2-1.fc26.s390x
passwd-0.79-9.fc26.s390x
bind99-libs-9.9.10-3.P3.fc26.s390x
python3-rpm-4.13.0.2-1.fc26.s390x
systemd-233-7.fc26.s390x
virglrenderer-0.6.0-1.20170210git76b3da97b.fc26.s390x
s390utils-ziomon-1.36.1-3.fc26.s390x
s390utils-osasnmpd-1.36.1-3.fc26.s390x
libXrandr-1.5.1-2.fc26.s390x
libglvnd-glx-1.0.0-1.fc26.s390x
texlive-ifxetex-svn19685.0.5-33.fc26.2.noarch
texlive-psnfss-svn33946.9.2a-33.fc26.2.noarch
texlive-dvipdfmx-def-svn40328-33.fc26.2.noarch
texlive-natbib-svn20668.8.31b-33.fc26.2.noarch
texlive-xdvi-bin-svn40750-33.20160520.fc26.2.s390x
texlive-cm-svn32865.0-33.fc26.2.noarch
texlive-beton-svn15878.0-33.fc26.2.noarch
texlive-fpl-svn15878.1.002-33.fc26.2.noarch
texlive-mflogo-svn38628-33.fc26.2.noarch
texlive-texlive-docindex-svn41430-33.fc26.2.noarch
texlive-luaotfload-bin-svn34647.0-33.20160520.fc26.2.noarch
texlive-koma-script-svn41508-33.fc26.2.noarch
texlive-pst-tree-svn24142.1.12-33.fc26.2.noarch
texlive-breqn-svn38099.0.98d-33.fc26.2.noarch
texlive-xetex-svn41438-33.fc26.2.noarch
gstreamer1-plugins-bad-free-1.12.3-1.fc26.s390x
xorg-x11-font-utils-7.5-33.fc26.s390x
ghostscript-fonts-5.50-36.fc26.noarch
libXext-devel-1.3.3-5.fc26.s390x
libusbx-devel-1.0.21-2.fc26.s390x
libglvnd-devel-1.0.0-1.fc26.s390x
emacs-25.3-3.fc26.s390x
alsa-lib-devel-1.1.4.1-1.fc26.s390x
kbd-2.0.4-2.fc26.s390x
dconf-0.26.0-2.fc26.s390x
mc-4.8.19-5.fc26.s390x
doxygen-1.8.13-9.fc26.s390x
dpkg-1.18.24-1.fc26.s390x
libtdb-1.3.13-1.fc26.s390x
python2-pynacl-1.1.1-1.fc26.s390x
perl-Filter-1.58-1.fc26.s390x
python2-pip-9.0.1-11.fc26.noarch
dnf-2.7.5-2.fc26.noarch
bind-license-9.11.2-1.P1.fc26.noarch
libtasn1-4.13-1.fc26.s390x
cpp-7.3.1-2.fc26.s390x
pkgconf-1.3.12-2.fc26.s390x
python2-fedora-0.10.0-1.fc26.noarch
cmake-filesystem-3.10.1-11.fc26.s390x
python3-requests-kerberos-0.12.0-1.fc26.noarch
libmicrohttpd-0.9.59-1.fc26.s390x
GeoIP-GeoLite-data-2018.01-1.fc26.noarch
python2-libs-2.7.14-7.fc26.s390x
libidn2-2.0.4-3.fc26.s390x
p11-kit-devel-0.23.10-1.fc26.s390x
perl-Errno-1.25-396.fc26.s390x
libdrm-2.4.90-2.fc26.s390x
sssd-common-1.16.1-1.fc26.s390x
boost-random-1.63.0-11.fc26.s390x
urw-fonts-2.4-24.fc26.noarch
ccache-3.3.6-1.fc26.s390x
glibc-debuginfo-2.24-10.fc25.s390x
dejavu-fonts-common-2.35-4.fc26.noarch
bind99-license-9.9.10-3.P3.fc26.noarch
ncurses-libs-6.0-8.20170212.fc26.s390x
libpng-1.6.28-2.fc26.s390x
libICE-1.0.9-9.fc26.s390x
perl-Text-ParseWords-3.30-366.fc26.noarch
libtool-ltdl-2.4.6-17.fc26.s390x
libselinux-utils-2.6-7.fc26.s390x
userspace-rcu-0.9.3-2.fc26.s390x
perl-Class-Inspector-1.31-3.f

[Qemu-devel] [RFC v3] qapi: command category to stimulate high-level machine devices

2018-06-03 Thread Steffen Görtz
Changes in v2:
  - Remove stray 1

Changes in v1:
  - Fix coding style issues

Add a new category "stimulate" to host commands that
act upon high-level devices associated with machines/boards.

This is patch is part of an effort to add support for BBC:microbit
educational computer to QEMU.

The microbit board, as many other microcontroller boards,
contains typical trivial (digital) input and output options such as buttons and 
LEDs,
but also more sophiscated possibilities such as analog inputs and
complex dedicated sensor ICs that are connected to the microbit machine
by means of inter-ic bus (i2c).
These devices interact with peripheral devices of the microcontroller
in use, for example with the GPIO peripheral of the used NRF51.

One aim of our efforts is to create a user interface that models the
look and feel of the physical microbit board and lets the user interact
with its inputs and provide feedback on its outsputs.

For this it is necessary to transmit user inputs such as the pressing of a
button to the machine.
In return, when the state of an output is changed on the machine, this
change needs to be reflected in the user interface.

At the moment, there are only a few QMP commands that provide user input to the
machine, eg. send-keys, input-button. These commands require very
high-level concepts, which are not really suitable for applications that
microcontrollers are typically used in in.

This RFC is meant to start a discussion on how to provide stimuli to
low-complex inputs and outputs typically found in embedded
microcontroller applications. To the best of my knowledge, there are
currently no examples of how to provide such stimuli to either SoC
device peripheral or machine level concepts like pushbuttons and LEDs.

To my understanding, the following concepts/apis are needed:
- QMP commands to send stimuli to machine level concepts like
pushbuttons
- Machine level devices that receive these stimuli and act as an adapter
to manipulate the associated SoC peripheral devices
- For outputs, machine level devices are needed that observe changes in
SoC peripherals and emit QMP events that clients can receive

This patch adds a new qmp command "buttons-set-state" to update the
push-down state of arbitrarily identified buttons (string identifier).

The handler of this command is mocked to set the machines SoC gpio/irq
lines associated with these buttons on the machine by means of object
property aliases. This is just a mock until a proper concept/api is
agreed upon.

As i recently joined the QEMU community as part of GSoC, i am still a
greenhorn to the codebase and i am really excited to discuss potential
concepts and APIs to realize the described features.

Steffen

Based-on: <20180503090532.3113-1-j...@jms.id.au>
Signed-off-by: Steffen Goertz 
---
 Makefile  |  8 +++
 Makefile.objs |  4 
 Makefile.target   |  1 +
 hw/arm/microbit.c |  7 ++
 qapi/qapi-schema.json |  1 +
 qapi/stimulate.json   | 22 ++
 stimulate.c   | 52 +++
 7 files changed, 95 insertions(+)
 create mode 100644 qapi/stimulate.json
 create mode 100644 stimulate.c

diff --git a/Makefile b/Makefile
index d71dd5bea4..d9319efa68 100644
--- a/Makefile
+++ b/Makefile
@@ -108,6 +108,7 @@ GENERATED_FILES += qapi/qapi-types-tpm.h 
qapi/qapi-types-tpm.c
 GENERATED_FILES += qapi/qapi-types-trace.h qapi/qapi-types-trace.c
 GENERATED_FILES += qapi/qapi-types-transaction.h qapi/qapi-types-transaction.c
 GENERATED_FILES += qapi/qapi-types-ui.h qapi/qapi-types-ui.c
+GENERATED_FILES += qapi/qapi-types-stimulate.h qapi/qapi-types-stimulate.c
 GENERATED_FILES += qapi/qapi-builtin-visit.h qapi/qapi-builtin-visit.c
 GENERATED_FILES += qapi/qapi-visit.h qapi/qapi-visit.c
 GENERATED_FILES += qapi/qapi-visit-block-core.h qapi/qapi-visit-block-core.c
@@ -126,6 +127,7 @@ GENERATED_FILES += qapi/qapi-visit-tpm.h 
qapi/qapi-visit-tpm.c
 GENERATED_FILES += qapi/qapi-visit-trace.h qapi/qapi-visit-trace.c
 GENERATED_FILES += qapi/qapi-visit-transaction.h qapi/qapi-visit-transaction.c
 GENERATED_FILES += qapi/qapi-visit-ui.h qapi/qapi-visit-ui.c
+GENERATED_FILES += qapi/qapi-visit-stimulate.h qapi/qapi-visit-stimulate.c
 GENERATED_FILES += qapi/qapi-commands.h qapi/qapi-commands.c
 GENERATED_FILES += qapi/qapi-commands-block-core.h 
qapi/qapi-commands-block-core.c
 GENERATED_FILES += qapi/qapi-commands-block.h qapi/qapi-commands-block.c
@@ -143,6 +145,7 @@ GENERATED_FILES += qapi/qapi-commands-tpm.h 
qapi/qapi-commands-tpm.c
 GENERATED_FILES += qapi/qapi-commands-trace.h qapi/qapi-commands-trace.c
 GENERATED_FILES += qapi/qapi-commands-transaction.h 
qapi/qapi-commands-transaction.c
 GENERATED_FILES += qapi/qapi-commands-ui.h qapi/qapi-commands-ui.c
+GENERATED_FILES += qapi/qapi-commands-stimulate.h 
qapi/qapi-commands-stimulate.c
 GENERATED_FILES += qapi/qapi-events.h qapi/qapi-events.c
 GENERATED_FILES += qapi/qapi-events-block-core.h qapi/qapi-events-block-core.

Re: [Qemu-devel] [RFC v3] qapi: command category to stimulate high-level machine devices

2018-06-03 Thread no-reply
Hi,

This series failed docker-mingw@fedora build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 20180603203412.11033-1-cont...@steffen-goertz.de
Subject: [Qemu-devel] [RFC v3] qapi: command category to stimulate high-level 
machine devices

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
423a4be2c3 qapi: command category to stimulate high-level machine devices

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-_5yx3xb5/src/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
  BUILD   fedora
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-_5yx3xb5/src'
  GEN 
/var/tmp/patchew-tester-tmp-_5yx3xb5/src/docker-src.2018-06-03-16.39.13.7960/qemu.tar
Cloning into 
'/var/tmp/patchew-tester-tmp-_5yx3xb5/src/docker-src.2018-06-03-16.39.13.7960/qemu.tar.vroot'...
done.
Your branch is up-to-date with 'origin/test'.
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 
'/var/tmp/patchew-tester-tmp-_5yx3xb5/src/docker-src.2018-06-03-16.39.13.7960/qemu.tar.vroot/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered 
for path 'ui/keycodemapdb'
Cloning into 
'/var/tmp/patchew-tester-tmp-_5yx3xb5/src/docker-src.2018-06-03-16.39.13.7960/qemu.tar.vroot/ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out 
'6b3d716e2b6472eb7189d3220552280ef3d832ce'
  COPYRUNNER
RUN test-mingw in qemu:fedora 
Packages installed:
PyYAML-3.12-5.fc27.x86_64
SDL2-devel-2.0.7-2.fc27.x86_64
bc-1.07.1-3.fc27.x86_64
bison-3.0.4-8.fc27.x86_64
bluez-libs-devel-5.48-3.fc27.x86_64
brlapi-devel-0.6.6-8.fc27.x86_64
bzip2-1.0.6-24.fc27.x86_64
bzip2-devel-1.0.6-24.fc27.x86_64
ccache-3.3.6-1.fc27.x86_64
clang-5.0.1-5.fc27.x86_64
device-mapper-multipath-devel-0.7.1-9.git847cc43.fc27.x86_64
findutils-4.6.0-16.fc27.x86_64
flex-2.6.1-5.fc27.x86_64
gcc-7.3.1-5.fc27.x86_64
gcc-c++-7.3.1-5.fc27.x86_64
gettext-0.19.8.1-12.fc27.x86_64
git-2.14.3-3.fc27.x86_64
glib2-devel-2.54.3-2.fc27.x86_64
glusterfs-api-devel-3.12.7-1.fc27.x86_64
gnutls-devel-3.5.18-2.fc27.x86_64
gtk3-devel-3.22.26-2.fc27.x86_64
hostname-3.18-4.fc27.x86_64
libaio-devel-0.3.110-9.fc27.x86_64
libasan-7.3.1-5.fc27.x86_64
libattr-devel-2.4.47-21.fc27.x86_64
libcap-devel-2.25-7.fc27.x86_64
libcap-ng-devel-0.7.8-5.fc27.x86_64
libcurl-devel-7.55.1-10.fc27.x86_64
libfdt-devel-1.4.6-1.fc27.x86_64
libpng-devel-1.6.31-1.fc27.x86_64
librbd-devel-12.2.4-1.fc27.x86_64
libssh2-devel-1.8.0-5.fc27.x86_64
libubsan-7.3.1-5.fc27.x86_64
libusbx-devel-1.0.21-4.fc27.x86_64
libxml2-devel-2.9.7-1.fc27.x86_64
llvm-5.0.1-6.fc27.x86_64
lzo-devel-2.08-11.fc27.x86_64
make-4.2.1-4.fc27.x86_64
mingw32-SDL-1.2.15-9.fc27.noarch
mingw32-bzip2-1.0.6-9.fc27.noarch
mingw32-curl-7.54.1-2.fc27.noarch
mingw32-glib2-2.54.1-1.fc27.noarch
mingw32-gmp-6.1.2-2.fc27.noarch
mingw32-gnutls-3.5.13-2.fc27.noarch
mingw32-gtk2-2.24.31-4.fc27.noarch
mingw32-gtk3-3.22.16-1.fc27.noarch
mingw32-libjpeg-turbo-1.5.1-3.fc27.noarch
mingw32-libpng-1.6.29-2.fc27.noarch
mingw32-libssh2-1.8.0-3.fc27.noarch
mingw32-libtasn1-4.13-1.fc27.noarch
mingw32-nettle-3.3-3.fc27.noarch
mingw32-pixman-0.34.0-3.fc27.noarch
mingw32-pkg-config-0.28-9.fc27.x86_64
mingw64-SDL-1.2.15-9.fc27.noarch
mingw64-bzip2-1.0.6-9.fc27.noarch
mingw64-curl-7.54.1-2.fc27.noarch
mingw64-glib2-2.54.1-1.fc27.noarch
mingw64-gmp-6.1.2-2.fc27.noarch
mingw64-gnutls-3.5.13-2.fc27.noarch
mingw64-gtk2-2.24.31-4.fc27.noarch
mingw64-gtk3-3.22.16-1.fc27.noarch
mingw64-libjpeg-turbo-1.5.1-3.fc27.noarch
mingw64-libpng-1.6.29-2.fc27.noarch
mingw64-libssh2-1.8.0-3.fc27.noarch
mingw64-libtasn1-4.13-1.fc27.noarch
mingw64-nettle-3.3-3.fc27.noarch
mingw64-pixman-0.34.0-3.fc27.noarch
mingw64-pkg-config-0.28-9.fc27.x86_64
ncurses-devel-6.0-13.20170722.fc27.x86_64
nettle-devel-3.4-1.fc27.x86_64
nss-devel-3.36.0-1.0.fc27.x86_64
numactl-devel-2.0.11-5.fc27.x86_64
package libjpeg-devel is not installed
perl-5.26.1-403.fc27.x86_64
pixman-devel-0.34.0-4.fc27.x86_64
python3-3.6.2-13.fc27.x86_64
snappy-devel-1.1.4-5.fc27.x86_64
sparse-0.5.1-2.fc27.x86_64
spice-server-devel-0.14.0-1.fc27.x86_64
systemtap-sdt-devel-3.2-3.fc27.x86_64
tar-1.29-7.fc27.x86_64
usbredir-devel-0.7.1-5.fc27.x86_64
virglrenderer-devel-0.6.0-3.20170210git76b3da97b.fc27.x86_64
vte3-devel-0.36.5-5.fc27.x86_64
which-2.21-4.fc27.x86_64
xen-devel-4.9.1-5.fc27.x86_64
zlib-devel-1.2.11-4.fc27.x86_64

Environment variables:
TARGET_LIST=
PACKAGES=ccache gettext git tar PyYAML sparse flex bison python3 bzip2 hostname 
 

Re: [Qemu-devel] [PATCH for 2.10 26/35] linux-user: use is_error() to avoid warnings and make the code clearer

2018-06-03 Thread Laurent Vivier
Le 29/05/2018 à 17:19, Laurent Vivier a écrit :
> Le 29/05/2018 à 16:25, Philippe Mathieu-Daudé a écrit :
>> Hi Laurent,
> 
> Hi Philippe,
> 
>> On 07/24/2017 04:16 PM, Laurent Vivier wrote:
>>> Le 24/07/2017 à 20:27, Philippe Mathieu-Daudé a écrit :
 linux-user/flatload.c:740:9: warning: Loss of sign in implicit conversion
 if (res > (unsigned long)-4096)
 ^~~

 Reported-by: Clang Static Analyzer
 Signed-off-by: Philippe Mathieu-Daudé 
>>>
>>> Reviewed-by: Laurent Vivier 
>>>
 ---
  linux-user/flatload.c | 15 +--
  1 file changed, 9 insertions(+), 6 deletions(-)

 diff --git a/linux-user/flatload.c b/linux-user/flatload.c
 index a35a560904..10c529910f 100644
 --- a/linux-user/flatload.c
 +++ b/linux-user/flatload.c
 @@ -224,8 +224,9 @@ static int decompress_exec(
ret = bprm->file->f_op->read(bprm->file, buf, LBUFSIZE, &fpos);
if (ret <= 0)
break;
 -  if (ret >= (unsigned long) -4096)
 +if (is_error(ret)) {
break;
 +}
len -= ret;
  
strm.next_in = buf;
 @@ -283,8 +284,7 @@ calc_reloc(abi_ulong r, struct lib_info *p, int curid, 
 int internalp)
  "in same module (%d != %d)\n",
  (unsigned) r, curid, id);
  goto failed;
 -} else if ( ! p[id].loaded &&
 -load_flat_shared_library(id, p) > (unsigned long) 
 -4096) {
 +} else if (!p[id].loaded && is_error(load_flat_shared_library(id, 
 p))) {
  fprintf(stderr, "BINFMT_FLAT: failed to load library %d\n", 
 id);
  goto failed;
  }
 @@ -523,9 +523,10 @@ static int load_flat_file(struct linux_binprm * bprm,
  fpos = 0;
  result = bprm->file->f_op->read(bprm->file,
  (char *) textpos, text_len, &fpos);
 -if (result < (unsigned long) -4096)
 +if (!is_error(result)) {
  result = decompress_exec(bprm, text_len, (char *) 
 datapos,
   data_len + (relocs * 
 sizeof(unsigned long)), 0);
 +}
  }
  else
  #endif
 @@ -693,8 +694,9 @@ static int load_flat_shared_library(int id, struct 
 lib_info *libs)
  
res = prepare_binprm(&bprm);
  
 -  if (res <= (unsigned long)-4096)
 +if (!is_error(res)) {
res = load_flat_file(&bprm, libs, id, NULL);
 +}
if (bprm.file) {
allow_write_access(bprm.file);
fput(bprm.file);
 @@ -737,8 +739,9 @@ int load_flt_binary(struct linux_binprm *bprm, struct 
 image_info *info)
  
  
  res = load_flat_file(bprm, libinfo, 0, &stack_len);
 -if (res > (unsigned long)-4096)
 +if (is_error(res)) {
  return res;
 +}
  
  /* Update data segment pointers for all libraries */
  for (i=0; i>>>
>>
>> Can you take this via your linux-user tree?
>>
> 
> Applied, thanks.

Unapplied, it needs a rebase:

qemu/linux-user/flatload.c: In function 'load_flt_binary':
qemu/linux-user/flatload.c:742:9: error: implicit declaration of
function 'is_error'; did you mean 'g_error'?
[-Werror=implicit-function-declaration]
 if (is_error(res)) {
 ^~~~
 g_error
qemu/linux-user/flatload.c:742:9: error: nested extern declaration of
'is_error' [-Werror=nested-externs]

Thanks,
Laurent





[Qemu-devel] [PATCH 0/4] Misc sam460ex improvements

2018-06-03 Thread BALATON Zoltan
Some more improvements to sam460ex emulation:
- Implemented RTC (which also needed fixing I2C to work)
- Fix a problem in sm501 which led to AmigaOS slow down

BALATON Zoltan (4):
  ppc4xx_i2c: Rewrite PPC4xx I2C emulation
  hw/timer: Add basic M41T80 emulation
  sam460ex: Add RTC device
  sm501: Do not clear read only bits when writing register

 MAINTAINERS |   1 +
 default-configs/ppc-softmmu.mak |   2 +
 hw/display/sm501.c  |   6 +-
 hw/i2c/ppc4xx_i2c.c | 366 +---
 hw/ppc/sam460ex.c   |   1 +
 hw/timer/Makefile.objs  |   1 +
 hw/timer/m41t80.c   | 117 +
 include/hw/i2c/ppc4xx_i2c.h |  19 +--
 8 files changed, 321 insertions(+), 192 deletions(-)
 create mode 100644 hw/timer/m41t80.c

-- 
2.7.6




[Qemu-devel] [PATCH 3/4] sam460ex: Add RTC device

2018-06-03 Thread BALATON Zoltan
The Sam460ex has an M41T80 serial RTC chip on I2C bus 0 at address 0x68.

Signed-off-by: BALATON Zoltan 
---
 hw/ppc/sam460ex.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index a48e6e6..3848885 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -458,6 +458,7 @@ static void sam460ex_init(MachineState *machine)
 object_property_set_bool(OBJECT(dev), true, "realized", NULL);
 smbus_eeprom_init(i2c[0]->bus, 8, smbus_eeprom_buf, smbus_eeprom_size);
 g_free(smbus_eeprom_buf);
+i2c_create_slave(i2c[0]->bus, "m41t80", 0x68);
 
 dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600800, uic[0][3]);
 i2c[1] = PPC4xx_I2C(dev);
-- 
2.7.6




[Qemu-devel] [PATCH 1/4] ppc4xx_i2c: Rewrite PPC4xx I2C emulation

2018-06-03 Thread BALATON Zoltan
I2C emulation currently is just enough for U-Boot to access SPD
EEPROMs but features that guests use to access I2C devices are not
correctly emulated. Rewrite to implement missing features to make it
work with all clients.

Signed-off-by: BALATON Zoltan 
---
Maybe this could be split up into more patches but because the
previous implementation was wrong only allowing U-Boot to pass and no
clients could access I2C devices before this rewrite it probably does
not worth to try to make it a lot of small changes instead of dropping
the previous hack and rewrite following features of real hardware more
closely. (It turns out that each client driver accesses I2C in a
different way so we need to implement almost all features of the
hardware to please everyone.)

default-configs/ppc-softmmu.mak |   1 +
 hw/i2c/ppc4xx_i2c.c | 366 +---
 include/hw/i2c/ppc4xx_i2c.h |  19 +--
 3 files changed, 197 insertions(+), 189 deletions(-)

diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index 4d7be45..7d0dc2f 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -26,6 +26,7 @@ CONFIG_USB_EHCI_SYSBUS=y
 CONFIG_SM501=y
 CONFIG_IDE_SII3112=y
 CONFIG_I2C=y
+CONFIG_BITBANG_I2C=y
 
 # For Macs
 CONFIG_MAC=y
diff --git a/hw/i2c/ppc4xx_i2c.c b/hw/i2c/ppc4xx_i2c.c
index ab64d19..638bb01 100644
--- a/hw/i2c/ppc4xx_i2c.c
+++ b/hw/i2c/ppc4xx_i2c.c
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 2007 Jocelyn Mayer
  * Copyright (c) 2012 François Revol
- * Copyright (c) 2016 BALATON Zoltan
+ * Copyright (c) 2016-2018 BALATON Zoltan
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to 
deal
@@ -30,281 +30,300 @@
 #include "cpu.h"
 #include "hw/hw.h"
 #include "hw/i2c/ppc4xx_i2c.h"
+#include "bitbang_i2c.h"
 
-#define PPC4xx_I2C_MEM_SIZE 0x12
+#define PPC4xx_I2C_MEM_SIZE 18
 
 #define IIC_CNTL_PT (1 << 0)
 #define IIC_CNTL_READ   (1 << 1)
 #define IIC_CNTL_CHT(1 << 2)
 #define IIC_CNTL_RPST   (1 << 3)
+#define IIC_CNTL_AMD(1 << 6)
+#define IIC_CNTL_HMT(1 << 7)
+
+#define IIC_MDCNTL_EINT (1 << 2)
+#define IIC_MDCNTL_ESM  (1 << 3)
+#define IIC_MDCNTL_FMDB (1 << 6)
 
 #define IIC_STS_PT  (1 << 0)
+#define IIC_STS_IRQA(1 << 1)
 #define IIC_STS_ERR (1 << 2)
+#define IIC_STS_MDBF(1 << 4)
 #define IIC_STS_MDBS(1 << 5)
 
 #define IIC_EXTSTS_XFRA (1 << 0)
 
+#define IIC_INTRMSK_EIMTC   (1 << 0)
+#define IIC_INTRMSK_EITA(1 << 1)
+#define IIC_INTRMSK_EIIC(1 << 2)
+#define IIC_INTRMSK_EIHE(1 << 3)
+
 #define IIC_XTCNTLSS_SRST   (1 << 0)
 
+#define IIC_DIRECTCNTL_SDAC (1 << 3)
+#define IIC_DIRECTCNTL_SCLC (1 << 2)
+#define IIC_DIRECTCNTL_MSDA (1 << 1)
+#define IIC_DIRECTCNTL_MSCL (1 << 0)
+
+typedef struct {
+bitbang_i2c_interface *bitbang;
+int mdidx;
+uint8_t mdata[4];
+uint8_t lmadr;
+uint8_t hmadr;
+uint8_t cntl;
+uint8_t mdcntl;
+uint8_t sts;
+uint8_t extsts;
+uint8_t lsadr;
+uint8_t hsadr;
+uint8_t clkdiv;
+uint8_t intrmsk;
+uint8_t xfrcnt;
+uint8_t xtcntlss;
+uint8_t directcntl;
+} PPC4xxI2CRegs;
+
 static void ppc4xx_i2c_reset(DeviceState *s)
 {
-PPC4xxI2CState *i2c = PPC4xx_I2C(s);
+PPC4xxI2CRegs *i2c = PPC4xx_I2C(s)->regs;
 
-/* FIXME: Should also reset bus?
- *if (s->address != ADDR_RESET) {
- *i2c_end_transfer(s->bus);
- *}
- */
-
-i2c->mdata = 0;
-i2c->lmadr = 0;
-i2c->hmadr = 0;
+i2c->mdidx = -1;
+memset(i2c->mdata, 0, ARRAY_SIZE(i2c->mdata));
+/* [hl][ms]addr are not affected by reset */
 i2c->cntl = 0;
 i2c->mdcntl = 0;
 i2c->sts = 0;
-i2c->extsts = 0x8f;
-i2c->sdata = 0;
-i2c->lsadr = 0;
-i2c->hsadr = 0;
+i2c->extsts = (1 << 6);
 i2c->clkdiv = 0;
 i2c->intrmsk = 0;
 i2c->xfrcnt = 0;
 i2c->xtcntlss = 0;
-i2c->directcntl = 0x0f;
-i2c->intr = 0;
-}
-
-static inline bool ppc4xx_i2c_is_master(PPC4xxI2CState *i2c)
-{
-return true;
+i2c->directcntl = 0xf;
 }
 
 static uint64_t ppc4xx_i2c_readb(void *opaque, hwaddr addr, unsigned int size)
 {
-PPC4xxI2CState *i2c = PPC4xx_I2C(opaque);
+PPC4xxI2CState *s = PPC4xx_I2C(opaque);
+PPC4xxI2CRegs *i2c = s->regs;
 uint64_t ret;
+int i;
 
 switch (addr) {
-case 0x00:
-ret = i2c->mdata;
-if (ppc4xx_i2c_is_master(i2c)) {
+case 0:
+if (i2c->mdidx < 0) {
 ret = 0xff;
-
-if (!(i2c->sts & IIC_STS_MDBS)) {
-qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Trying to read "
-  "without starting transfer\n",
-  TYPE_PPC4xx_I2C, __func__);
-} else {
-int pending = (i2c->cntl >> 4) & 3;
-
-/* get the next byte */
-int byte

[Qemu-devel] [PATCH 4/4] sm501: Do not clear read only bits when writing register

2018-06-03 Thread BALATON Zoltan
When writing a register that has read only bits besides reserved bits
we have to avoid changing read only bits that may have non zero
default values. While at it, fix a reserved bit mask and a white space
error.

Signed-off-by: BALATON Zoltan 
---
 hw/display/sm501.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index f4bb33c..51b2bb8 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -837,10 +837,10 @@ static void sm501_system_config_write(void *opaque, 
hwaddr addr,
 
 switch (addr) {
 case SM501_SYSTEM_CONTROL:
-s->system_control = value & 0xE300B8F7;
+s->system_control |= value & 0xEF00B8F7;
 break;
 case SM501_MISC_CONTROL:
-s->misc_control = value & 0xFF7FFF20;
+s->misc_control |= value & 0xFF7FFF10;
 break;
 case SM501_GPIO31_0_CONTROL:
 s->gpio_31_0_control = value;
@@ -854,7 +854,7 @@ static void sm501_system_config_write(void *opaque, hwaddr 
addr,
 s->dram_control |=  value & 0x7FC3;
 break;
 case SM501_ARBTRTN_CONTROL:
-s->arbitration_control =  value & 0x3777;
+s->arbitration_control = value & 0x3777;
 break;
 case SM501_IRQ_MASK:
 s->irq_mask = value;
-- 
2.7.6




[Qemu-devel] [PATCH 2/4] hw/timer: Add basic M41T80 emulation

2018-06-03 Thread BALATON Zoltan
Basic emulation of the M41T80 serial (I2C) RTC chip. Only getting time
of day is implemented. Setting time and RTC alarm are not supported.

Signed-off-by: BALATON Zoltan 
---
 MAINTAINERS |   1 +
 default-configs/ppc-softmmu.mak |   1 +
 hw/timer/Makefile.objs  |   1 +
 hw/timer/m41t80.c   | 117 
 4 files changed, 120 insertions(+)
 create mode 100644 hw/timer/m41t80.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 41cd373..9e13bc1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -826,6 +826,7 @@ M: BALATON Zoltan 
 L: qemu-...@nongnu.org
 S: Maintained
 F: hw/ide/sii3112.c
+F: hw/timer/m41t80.c
 
 SH4 Machines
 
diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index 7d0dc2f..9fbaadc 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -27,6 +27,7 @@ CONFIG_SM501=y
 CONFIG_IDE_SII3112=y
 CONFIG_I2C=y
 CONFIG_BITBANG_I2C=y
+CONFIG_M41T80=y
 
 # For Macs
 CONFIG_MAC=y
diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index 8b27a4b..e16b2b9 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -6,6 +6,7 @@ common-obj-$(CONFIG_CADENCE) += cadence_ttc.o
 common-obj-$(CONFIG_DS1338) += ds1338.o
 common-obj-$(CONFIG_HPET) += hpet.o
 common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
+common-obj-$(CONFIG_M41T80) += m41t80.o
 common-obj-$(CONFIG_M48T59) += m48t59.o
 ifeq ($(CONFIG_ISA_BUS),y)
 common-obj-$(CONFIG_M48T59) += m48t59-isa.o
diff --git a/hw/timer/m41t80.c b/hw/timer/m41t80.c
new file mode 100644
index 000..9dbdb1b
--- /dev/null
+++ b/hw/timer/m41t80.c
@@ -0,0 +1,117 @@
+/*
+ * M41T80 serial rtc emulation
+ *
+ * Copyright (c) 2018 BALATON Zoltan
+ *
+ * This work is licensed under the GNU GPL license version 2 or later.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/timer.h"
+#include "qemu/bcd.h"
+#include "hw/i2c/i2c.h"
+
+#define TYPE_M41T80 "m41t80"
+#define M41T80(obj) OBJECT_CHECK(M41t80State, (obj), TYPE_M41T80)
+
+typedef struct M41t80State {
+I2CSlave parent_obj;
+int8_t addr;
+} M41t80State;
+
+static void m41t80_realize(DeviceState *dev, Error **errp)
+{
+M41t80State *s = M41T80(dev);
+
+s->addr = -1;
+}
+
+static int m41t80_send(I2CSlave *i2c, uint8_t data)
+{
+M41t80State *s = M41T80(i2c);
+
+if (s->addr < 0) {
+s->addr = data;
+} else {
+s->addr++;
+}
+return 0;
+}
+
+static int m41t80_recv(I2CSlave *i2c)
+{
+M41t80State *s = M41T80(i2c);
+struct tm now;
+qemu_timeval tv;
+
+if (s->addr < 0) {
+s->addr = 0;
+}
+if (s->addr >= 1 && s->addr <= 7) {
+qemu_get_timedate(&now, -1);
+}
+switch (s->addr++) {
+case 0:
+qemu_gettimeofday(&tv);
+return to_bcd(tv.tv_usec / 1);
+case 1:
+return to_bcd(now.tm_sec);
+case 2:
+return to_bcd(now.tm_min);
+case 3:
+return to_bcd(now.tm_hour);
+case 4:
+return to_bcd(now.tm_wday);
+case 5:
+return to_bcd(now.tm_mday);
+case 6:
+return to_bcd(now.tm_mon + 1);
+case 7:
+return to_bcd(now.tm_year % 100);
+case 8 ... 19:
+qemu_log_mask(LOG_UNIMP, "\n%s: unimplemented register: %d\n",
+  __func__, s->addr - 1);
+return 0;
+default:
+qemu_log_mask(LOG_GUEST_ERROR, "\n%s: invalid register: %d",
+  __func__, s->addr - 1);
+return 0;
+}
+}
+
+static int m41t80_event(I2CSlave *i2c, enum i2c_event event)
+{
+M41t80State *s = M41T80(i2c);
+
+if (event == I2C_START_SEND) {
+s->addr = -1;
+}
+return 0;
+}
+
+static void m41t80_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass);
+
+dc->realize = m41t80_realize;
+sc->send = m41t80_send;
+sc->recv = m41t80_recv;
+sc->event = m41t80_event;
+}
+
+static const TypeInfo m41t80_info = {
+.name  = TYPE_M41T80,
+.parent= TYPE_I2C_SLAVE,
+.instance_size = sizeof(M41t80State),
+.class_init= m41t80_class_init,
+};
+
+static void m41t80_register_types(void)
+{
+type_register_static(&m41t80_info);
+}
+
+type_init(m41t80_register_types)
-- 
2.7.6




Re: [Qemu-devel] [PATCH 6/6] linux-user: Use *at functions to implement interp_prefix

2018-06-03 Thread Laurent Vivier
On 01/06/2018 00:49, Richard Henderson wrote:
> If the interp_prefix is a complete chroot, it may have a *lot* of files.
> Setting up the cache for this is quite expensive.
> 
> For the most part, we can use the *at versions of various syscalls to
> attempt the operation in the prefix.  For the few cases that remain,
> attempt the operation in the prefix via concatenation and then retry
> if that fails.
> 

I like the idea, but it breaks real chroot.

You can test it with:

wget
https://github.com/vivier/linux-user-test-scrips/raw/master/create_chroot.sh

then

sudo sh ./create_chroot.sh /path/to/static/qemu-s390x stretch

It fails with:

Reading package lists... Done
E: Method /usr/lib/apt/methods/http did not start correctly
E: Failed to fetch http://ftp.us.debian.org/debian/dists/stretch/InRelease
E: Some index files failed to download. They have been ignored, or old
ones used instead.
stretch s390x FAILED


Thanks,
Laurent



Re: [Qemu-devel] [PATCH 4/4] uninorth: remove token register from uninorth device

2018-06-03 Thread David Gibson
On Sun, May 06, 2018 at 03:20:05PM +0100, Mark Cave-Ayland wrote:
> >From observation of various OS sources it can be seen that the token register
> introduced in 4e46dcdbd3 "PPC: Newworld: Add uninorth token register" is not
> required, since the only register currently implemented is the uninorth 
> hardware
> version which is read-only.
> 
> Remove the token register implementation and instead return the uninorth
> version corresponding to the hardware.
> 
> Signed-off-by: Mark Cave-Ayland 

Applied, thanks.

> ---
>  hw/pci-host/uninorth.c | 11 +--
>  include/hw/pci-host/uninorth.h |  4 +++-
>  2 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
> index a658f9230a..abebfaf755 100644
> --- a/hw/pci-host/uninorth.c
> +++ b/hw/pci-host/uninorth.c
> @@ -524,19 +524,18 @@ static void unin_write(void *opaque, hwaddr addr, 
> uint64_t value,
> unsigned size)
>  {
>  trace_unin_write(addr, value);
> -if (addr == 0x0) {
> -*(int *)opaque = value;
> -}
>  }
>  
>  static uint64_t unin_read(void *opaque, hwaddr addr, unsigned size)
>  {
>  uint32_t value;
>  
> -value = 0;
>  switch (addr) {
>  case 0:
> -value = *(int *)opaque;
> +value = UNINORTH_VERSION_10A;
> +break;
> +default:
> +value = 0;
>  }
>  
>  trace_unin_read(addr, value);
> @@ -559,7 +558,7 @@ static void unin_init(Object *obj)
>  UNINState *s = UNI_NORTH(obj);
>  SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>  
> -memory_region_init_io(&s->mem, obj, &unin_ops, &s->token, "unin", 
> 0x1000);
> +memory_region_init_io(&s->mem, obj, &unin_ops, s, "unin", 0x1000);
>  
>  sysbus_init_mmio(sbd, &s->mem);
>  }
> diff --git a/include/hw/pci-host/uninorth.h b/include/hw/pci-host/uninorth.h
> index f6654bad9b..2a1cf9f284 100644
> --- a/include/hw/pci-host/uninorth.h
> +++ b/include/hw/pci-host/uninorth.h
> @@ -29,6 +29,9 @@
>  
>  #include "hw/ppc/openpic.h"
>  
> +/* UniNorth version */
> +#define UNINORTH_VERSION_10A0x7
> +
>  #define TYPE_UNI_NORTH_PCI_HOST_BRIDGE "uni-north-pci-pcihost"
>  #define TYPE_UNI_NORTH_AGP_HOST_BRIDGE "uni-north-agp-pcihost"
>  #define TYPE_UNI_NORTH_INTERNAL_PCI_HOST_BRIDGE 
> "uni-north-internal-pci-pcihost"
> @@ -57,7 +60,6 @@ typedef struct UNINState {
>  SysBusDevice parent_obj;
>  
>  MemoryRegion mem;
> -int token[1];
>  } UNINState;
>  
>  #define TYPE_UNI_NORTH "uni-north"

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH 1/4] uninorth: add impl min_access_size and max_access_size to unin_ops

2018-06-03 Thread David Gibson
On Sun, May 06, 2018 at 03:20:02PM +0100, Mark Cave-Ayland wrote:
> >From testing all my local images the uninorth registers are only ever
> read or written with 32-bit accesses.

If that's the case, shouldn't you be setting the range of valid
accesses, rather than the range of implemented accesses?

> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/pci-host/uninorth.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
> index ba76b84dbc..a658f9230a 100644
> --- a/hw/pci-host/uninorth.c
> +++ b/hw/pci-host/uninorth.c
> @@ -548,6 +548,10 @@ static const MemoryRegionOps unin_ops = {
>  .read = unin_read,
>  .write = unin_write,
>  .endianness = DEVICE_BIG_ENDIAN,
> +.impl = {
> +.min_access_size = 4,
> +.max_access_size = 4,
> +},
>  };
>  
>  static void unin_init(Object *obj)

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH v3] target/ppc: Allow PIR read in privileged mode

2018-06-03 Thread David Gibson
On Mon, May 07, 2018 at 01:52:42PM -0300, luporl wrote:
> According to PowerISA, the PIR register should be readable in privileged
> mode also, not only in hypervisor privileged mode.
> 
> PowerISA 3.0 - 4.3.3 Processor Identification Register
> 
> "Read access to the PIR is privileged; write access is not
> provided."

Yes... but a little further down it says "The PIR is a hypervisor
resource".  Looking at the older 2.07 ISA, it says that
guest-supervisor mode reads to the PIR should be redirected to the
GPIR register, which this change won't accomplish.

So, I'm not sure what to make of this.

> 
> Cc: David Gibson 
> Cc: Alexander Graf 
> Cc: qemu-...@nongnu.org
> Signed-off-by: Leandro Lupori 
> Reviewed-by: Jose Ricardo Ziviani 
> Reviewed-by: Greg Kurz 
> ---
> Changes in v2:
> - added my Signed-off-by, maintainers CC and Jose's Reviewed-by tags
> 
> Changes in v3:
> - added subsystem name, version tag and summary of changes
> - added the section of PowerISA that describes PIR access privileges
> 
>  target/ppc/translate_init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> index a72be6d121..7b56e3ffb9 100644
> --- a/target/ppc/translate_init.c
> +++ b/target/ppc/translate_init.c
> @@ -7816,7 +7816,7 @@ static void gen_spr_book3s_ids(CPUPPCState *env)
>  /* Processor identification */
>  spr_register_hv(env, SPR_PIR, "PIR",
>   SPR_NOACCESS, SPR_NOACCESS,
> - SPR_NOACCESS, SPR_NOACCESS,
> + &spr_read_generic, SPR_NOACCESS,
>   &spr_read_generic, NULL,
>   0x);
>  spr_register_hv(env, SPR_HID0, "HID0",

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH 2/4] macio: add trace-events to timer device

2018-06-03 Thread David Gibson
On Sun, May 06, 2018 at 03:20:03PM +0100, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland 

Applied, thanks.

> ---
>  hw/misc/macio/macio.c  | 3 +++
>  hw/misc/macio/trace-events | 4 
>  2 files changed, 7 insertions(+)
> 
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index 79621eb879..f9a40eea81 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -32,6 +32,7 @@
>  #include "hw/char/escc.h"
>  #include "hw/misc/macio/macio.h"
>  #include "hw/intc/heathrow_pic.h"
> +#include "trace.h"
>  
>  /* Note: this code is strongly inspirated from the corresponding code
>   * in PearPC */
> @@ -246,6 +247,7 @@ static void macio_oldworld_init(Object *obj)
>  static void timer_write(void *opaque, hwaddr addr, uint64_t value,
> unsigned size)
>  {
> +trace_macio_timer_write(addr, size, value);
>  }
>  
>  static uint64_t timer_read(void *opaque, hwaddr addr, unsigned size)
> @@ -266,6 +268,7 @@ static uint64_t timer_read(void *opaque, hwaddr addr, 
> unsigned size)
>  break;
>  }
>  
> +trace_macio_timer_read(addr, size, value);
>  return value;
>  }
>  
> diff --git a/hw/misc/macio/trace-events b/hw/misc/macio/trace-events
> index 24c0a36824..d499d78c99 100644
> --- a/hw/misc/macio/trace-events
> +++ b/hw/misc/macio/trace-events
> @@ -9,3 +9,7 @@ cuda_packet_receive(int len) "length %d"
>  cuda_packet_receive_data(int i, const uint8_t data) "[%d] 0x%02x"
>  cuda_packet_send(int len) "length %d"
>  cuda_packet_send_data(int i, const uint8_t data) "[%d] 0x%02x"
> +
> +# hw/misc/macio/macio.c
> +macio_timer_write(uint64_t addr, unsigned len, uint64_t val) "write addr 
> 0x%"PRIx64 " len %d val 0x%"PRIx64
> +macio_timer_read(uint64_t addr, unsigned len, uint32_t val) "read addr 
> 0x%"PRIx64 " len %d val 0x%"PRIx32

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH] ppc: e500: use g_strdup_printf() instead of snprintf()

2018-06-03 Thread David Gibson
On Tue, May 08, 2018 at 12:01:59PM +0100, Peter Maydell wrote:
> On 8 May 2018 at 10:34, Greg Kurz  wrote:
> > On Mon, 7 May 2018 12:53:45 -0500
> > Eric Blake  wrote:
> >
> >> On 05/07/2018 04:02 AM, Greg Kurz wrote:
> >> > qemu-system-ppc fails to build with GCC 8.0.1:
> >> >
> >> > /home/hsp/src/qemu-master/hw/ppc/e500.c: In function 
> >> > ‘ppce500_load_device_tree’:
> >> > /home/hsp/src/qemu-master/hw/ppc/e500.c:442:37: error: ‘/pic@’
> >> > directive output may be truncated writing 5 bytes into a region of
> >> > size between 1 and 128 [-Werror=format-truncation=]
> >> >   snprintf(mpic, sizeof(mpic), "%s/pic@%llx", soc, 
> >> > MPC8544_MPIC_REGS_OFFSET);
> >> >   ^
> >>
> >> >
> >> > Fix this by converting e500 to use g_strdup_printf()+g_free() instead
> >> > of snprintf(). This is done globally, even for call sites that don't
> >> > break build, since this is the preferred practice in QEMU.
> >> >
> >> > Reported-by: Howard Spoelstra 
> >> > Signed-off-by: Greg Kurz 
> >> > ---
> >> >   hw/ppc/e500.c |   39 +++
> >> >   1 file changed, 23 insertions(+), 16 deletions(-)
> >> >
> >>
> >> Reviewed-by: Eric Blake 
> >>
> >
> > Hi Peter,
> >
> > David said the next pull request for ppc would happen in a month. This
> > patch fixes an annoying build break with recent GCC, and it already
> > got two positive reviews, is it possible to have it merged upstream ?
> 
> Sure; applied to master.

Thanks!

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH 4/8] virtio: add detach element for packed ring(1.1)

2018-06-03 Thread Wei Xu
On Tue, Apr 10, 2018 at 03:32:53PM +0800, Jason Wang wrote:
> 
> 
> On 2018年04月04日 20:54, w...@redhat.com wrote:
> >From: Wei Xu 
> >
> >helper for packed ring
> 
> It's odd and hard to review if you put detach patch first. I think this
> patch needs to be reordered after the implementation of pop/map.

This patch is not necessary after sync to tiwei's v5, so we can skip it.

Wei

> 
> Thanks
> 
> >Signed-off-by: Wei Xu 
> >---
> >  hw/virtio/virtio.c | 21 +++--
> >  1 file changed, 19 insertions(+), 2 deletions(-)
> >
> >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> >index 478df3d..fdee40f 100644
> >--- a/hw/virtio/virtio.c
> >+++ b/hw/virtio/virtio.c
> >@@ -561,6 +561,20 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const 
> >VirtQueueElement *elem,
> >   elem->out_sg[i].iov_len);
> >  }
> >+static void virtqueue_detach_element_split(VirtQueue *vq,
> >+const VirtQueueElement *elem, unsigned int len)
> >+{
> >+vq->inuse--;
> >+virtqueue_unmap_sg(vq, elem, len);
> >+}
> >+
> >+static void virtqueue_detach_element_packed(VirtQueue *vq,
> >+const VirtQueueElement *elem, unsigned int len)
> >+{
> >+vq->inuse -= elem->count;
> >+virtqueue_unmap_sg(vq, elem, len);
> >+}
> >+
> >  /* virtqueue_detach_element:
> >   * @vq: The #VirtQueue
> >   * @elem: The #VirtQueueElement
> >@@ -573,8 +587,11 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const 
> >VirtQueueElement *elem,
> >  void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem,
> >unsigned int len)
> >  {
> >-vq->inuse--;
> >-virtqueue_unmap_sg(vq, elem, len);
> >+if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
> >+virtqueue_detach_element_packed(vq, elem, len);
> >+} else {
> >+virtqueue_detach_element_split(vq, elem, len);
> >+}
> >  }
> >  /* virtqueue_unpop:
> 



Re: [Qemu-devel] [PATCH RFC] hw/pc: set q35 as the default x86 machine

2018-06-03 Thread Michael S. Tsirkin
On Sun, Jun 03, 2018 at 12:27:49PM +0300, Marcel Apfelbaum wrote:
> Moving to QEMU 3.0 seems like a good opportunity for such a change.
> 
> I440FX is really old and does not support modern features like IOMMU.
> Q35's SATA emulation is faster than pc's IDE, native PCI express hotplug
> is cleaner than ACPI based one and so on...
> 
> Also the libvirt guys added very good support for the Q35 machine (thanks!).
> 
> Management software should always specify the machine type and for the
> current setups, adding '-machine pc' to the command line is not such a
> big deal.
> 
> In time the pc machine will fade out and we will probably stop adding
> new versions at some point.
> 
> Signed-off-by: Marcel Apfelbaum 

For command line users, I think changing the default isn't nice.

Yes it's easy to add -machine pc but there's no documentation
that tells you to do so. Add to that shortcuts like -cdrom
stop working, hotplug needs extra bridges to work, and one
can see that while management tool users benefit from q35,
command line users will suffer.

Can't we add a tag for management without changing the command line
default? How about "management-default"? "recommended"? "latest"?

-- 
MST



Re: [Qemu-devel] [PATCH 4/8] virtio: add detach element for packed ring(1.1)

2018-06-03 Thread Michael S. Tsirkin
On Mon, Jun 04, 2018 at 09:34:35AM +0800, Wei Xu wrote:
> On Tue, Apr 10, 2018 at 03:32:53PM +0800, Jason Wang wrote:
> > 
> > 
> > On 2018年04月04日 20:54, w...@redhat.com wrote:
> > >From: Wei Xu 
> > >
> > >helper for packed ring
> > 
> > It's odd and hard to review if you put detach patch first. I think this
> > patch needs to be reordered after the implementation of pop/map.
> 
> This patch is not necessary after sync to tiwei's v5, so we can skip it.
> 
> Wei

I suspect we will need to bring detach back eventually but yes,
it can wait.

> > 
> > Thanks
> > 
> > >Signed-off-by: Wei Xu 
> > >---
> > >  hw/virtio/virtio.c | 21 +++--
> > >  1 file changed, 19 insertions(+), 2 deletions(-)
> > >
> > >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > >index 478df3d..fdee40f 100644
> > >--- a/hw/virtio/virtio.c
> > >+++ b/hw/virtio/virtio.c
> > >@@ -561,6 +561,20 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const 
> > >VirtQueueElement *elem,
> > >   elem->out_sg[i].iov_len);
> > >  }
> > >+static void virtqueue_detach_element_split(VirtQueue *vq,
> > >+const VirtQueueElement *elem, unsigned int 
> > >len)
> > >+{
> > >+vq->inuse--;
> > >+virtqueue_unmap_sg(vq, elem, len);
> > >+}
> > >+
> > >+static void virtqueue_detach_element_packed(VirtQueue *vq,
> > >+const VirtQueueElement *elem, unsigned int 
> > >len)
> > >+{
> > >+vq->inuse -= elem->count;
> > >+virtqueue_unmap_sg(vq, elem, len);
> > >+}
> > >+
> > >  /* virtqueue_detach_element:
> > >   * @vq: The #VirtQueue
> > >   * @elem: The #VirtQueueElement
> > >@@ -573,8 +587,11 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const 
> > >VirtQueueElement *elem,
> > >  void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement 
> > > *elem,
> > >unsigned int len)
> > >  {
> > >-vq->inuse--;
> > >-virtqueue_unmap_sg(vq, elem, len);
> > >+if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
> > >+virtqueue_detach_element_packed(vq, elem, len);
> > >+} else {
> > >+virtqueue_detach_element_split(vq, elem, len);
> > >+}
> > >  }
> > >  /* virtqueue_unpop:
> > 



Re: [Qemu-devel] [PATCH v7 3/5] migration: API to clear bits of guest free pages from the dirty bitmap

2018-06-03 Thread Peter Xu
On Fri, Jun 01, 2018 at 08:32:27PM +0800, Wei Wang wrote:
> On 06/01/2018 06:06 PM, Peter Xu wrote:
> > On Fri, Jun 01, 2018 at 03:36:01PM +0800, Wei Wang wrote:
> > > On 06/01/2018 12:00 PM, Peter Xu wrote:
> > > > On Tue, Apr 24, 2018 at 02:13:46PM +0800, Wei Wang wrote:
> > > > >/*
> > > > > + * This function clears bits of the free pages reported by the 
> > > > > caller from the
> > > > > + * migration dirty bitmap. @addr is the host address corresponding 
> > > > > to the
> > > > > + * start of the continuous guest free pages, and @len is the total 
> > > > > bytes of
> > > > > + * those pages.
> > > > > + */
> > > > > +void qemu_guest_free_page_hint(void *addr, size_t len)
> > > > > +{
> > > > > +RAMBlock *block;
> > > > > +ram_addr_t offset;
> > > > > +size_t used_len, start, npages;
> > > > Do we need to check here on whether a migration is in progress?  Since
> > > > if not I'm not sure whether this hint still makes any sense any more,
> > > > and more importantly it seems to me that block->bmap below at [1] is
> > > > only valid during a migration.  So I'm not sure whether QEMU will
> > > > crash if this function is called without a running migration.
> > > OK. How about just adding comments above to have users noted that this
> > > function should be used during migration?
> > > 
> > > If we want to do a sanity check here, I think it would be easier to just
> > > check !block->bmap here.
> > I think the faster way might be that we check against the migration
> > state.
> > 
> 
> Sounds good. We can do a sanity check:
> 
> MigrationState *s = migrate_get_current();
> if (!migration_is_setup_or_active(s->state))
> return;

Yes.

> 
> 
> 
> > > 
> > > > > +
> > > > > +for (; len > 0; len -= used_len) {
> > > > > +block = qemu_ram_block_from_host(addr, false, &offset);
> > > > > +if (unlikely(!block)) {
> > > > > +return;
> > > > We should never reach here, should we?  Assuming the callers of this
> > > > function should always pass in a correct host address. If we are very
> > > > sure that the host addr should be valid, could we just assert?
> > > Probably not the case, because of the corner case that the memory would be
> > > hot unplugged after the free page is reported to QEMU.
> > Question: Do we allow to do hot plug/unplug for memory during
> > migration?
> 
> I think so. From the code, I don't find where it forbids memory hotplug
> during migration.

I don't play with that much; do we need to do "device_add" after all?

  (qemu) object_add 
memory-backend-file,id=mem1,size=1G,mem-path=/mnt/hugepages-1GB
  (qemu) device_add pc-dimm,id=dimm1,memdev=mem1

If so, we may not allow that since in qdev_device_add() we don't allow
that:

if (!migration_is_idle()) {
error_setg(errp, "device_add not allowed while migrating");
return NULL;
}

> 
> > > 
> > > 
> > > > > +}
> > > > > +
> > > > > +/*
> > > > > + * This handles the case that the RAMBlock is resized after 
> > > > > the free
> > > > > + * page hint is reported.
> > > > > + */
> > > > > +if (unlikely(offset > block->used_length)) {
> > > > > +return;
> > > > > +}
> > > > > +
> > > > > +if (len <= block->used_length - offset) {
> > > > > +used_len = len;
> > > > > +} else {
> > > > > +used_len = block->used_length - offset;
> > > > > +addr += used_len;
> > > > > +}
> > > > > +
> > > > > +start = offset >> TARGET_PAGE_BITS;
> > > > > +npages = used_len >> TARGET_PAGE_BITS;
> > > > > +
> > > > > +qemu_mutex_lock(&ram_state->bitmap_mutex);
> > > > So now I think I understand the lock can still be meaningful since
> > > > this function now can be called outside the migration thread (e.g., in
> > > > vcpu thread).  But still it would be nice to mention it somewhere on
> > (Actually after read the next patch I think it's in iothread, so I'd
> >   better reply with all the series read over next time :)
> 
> That's fine actually :) Whether it is called by an iothread or a vcpu thread
> doesn't affect our discussion here.
> 
> I think we could just focus on the interfaces here and the usage in live
> migration. I can explain more when needed.

Ok.  Thanks!

-- 
Peter Xu



[Qemu-devel] [PATCH qemu v4] memory/hmp: Print owners/parents in "info mtree"

2018-06-03 Thread Alexey Kardashevskiy
This adds owners/parents (which are the same, just occasionally
owner==NULL) printing for memory regions; a new '-o' flag
enabled new output.

Signed-off-by: Alexey Kardashevskiy 
---

The only orphan object I know is printed now as:
  - (prio 0, i/o): mem-container-smram 
owner:{obj type=kvm-accel}


Unrelated observation:
  QEMU in x86_64 constantly hits breakpoints in unassigned_mem_accepts
  and unassigned_mem_read (when set), in early boot stage before SeaBIOS
  even shows up, the address is 0xfed40f00 and that is not anything
  assigned. "info mtree" shows that it is surrounded by:

fec0-fec00fff (prio 0, i/o): kvm-ioapic
fed0-fed003ff (prio 0, i/o): hpet
fee0-feef (prio 4096, i/o): kvm-apic-msi

  Is it something to worry about?

---
Changes:
v4:
* prints object typename if canonical path could not be resolved

v3:
* removed QOM's "id" property as there are no objects left which would
have this property and own an MR

v2:
* cleanups
---
 include/exec/memory.h |  2 +-
 memory.c  | 72 ---
 monitor.c |  4 ++-
 hmp-commands-info.hx  |  7 ++---
 4 files changed, 70 insertions(+), 15 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 67ea7fe..95340b8 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1627,7 +1627,7 @@ void memory_global_dirty_log_start(void);
 void memory_global_dirty_log_stop(void);
 
 void mtree_info(fprintf_function mon_printf, void *f, bool flatview,
-bool dispatch_tree);
+bool dispatch_tree, bool owner);
 
 /**
  * memory_region_request_mmio_ptr: request a pointer to an mmio
diff --git a/memory.c b/memory.c
index 10fa2dd..bf0a9f2 100644
--- a/memory.c
+++ b/memory.c
@@ -2832,10 +2832,49 @@ typedef QTAILQ_HEAD(mrqueue, MemoryRegionList) 
MemoryRegionListHead;
int128_sub((size), int128_one())) : 0)
 #define MTREE_INDENT "  "
 
+static void mtree_expand_owner(fprintf_function mon_printf, void *f,
+   const char *label, Object *obj)
+{
+DeviceState *dev = (DeviceState *) object_dynamic_cast(obj, TYPE_DEVICE);
+
+mon_printf(f, " %s:{%s", label, dev ? "dev" : "obj");
+if (dev && dev->id) {
+mon_printf(f, " id=%s", dev->id);
+} else {
+gchar *canonical_path = object_get_canonical_path(obj);
+if (canonical_path) {
+mon_printf(f, " path=%s", canonical_path);
+g_free(canonical_path);
+} else {
+mon_printf(f, " type=%s", object_get_typename(obj));
+}
+}
+mon_printf(f, "}");
+}
+
+static void mtree_print_mr_owner(fprintf_function mon_printf, void *f,
+ const MemoryRegion *mr)
+{
+Object *owner = mr->owner;
+Object *parent = memory_region_owner((MemoryRegion *)mr);
+
+if (!owner && !parent) {
+mon_printf(f, " orphan");
+return;
+}
+if (owner) {
+mtree_expand_owner(mon_printf, f, "owner", owner);
+}
+if (parent && parent != owner) {
+mtree_expand_owner(mon_printf, f, "parent", parent);
+}
+}
+
 static void mtree_print_mr(fprintf_function mon_printf, void *f,
const MemoryRegion *mr, unsigned int level,
hwaddr base,
-   MemoryRegionListHead *alias_print_queue)
+   MemoryRegionListHead *alias_print_queue,
+   bool owner)
 {
 MemoryRegionList *new_ml, *ml, *next_ml;
 MemoryRegionListHead submr_print_queue;
@@ -2881,7 +2920,7 @@ static void mtree_print_mr(fprintf_function mon_printf, 
void *f,
 }
 mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx
" (prio %d, %s): alias %s @%s " TARGET_FMT_plx
-   "-" TARGET_FMT_plx "%s\n",
+   "-" TARGET_FMT_plx "%s",
cur_start, cur_end,
mr->priority,
memory_region_type((MemoryRegion *)mr),
@@ -2890,15 +2929,22 @@ static void mtree_print_mr(fprintf_function mon_printf, 
void *f,
mr->alias_offset,
mr->alias_offset + MR_SIZE(mr->size),
mr->enabled ? "" : " [disabled]");
+if (owner) {
+mtree_print_mr_owner(mon_printf, f, mr);
+}
 } else {
 mon_printf(f,
-   TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): %s%s\n",
+   TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): %s%s",
cur_start, cur_end,
mr->priority,
memory_region_type((MemoryRegion *)mr),
memory_region_name(mr),
mr->enabled ? "" : " [disabled]");
+if (owner) {
+mtree_print_mr_owner(mon_printf, f, 

Re: [Qemu-devel] [PATCH v4] migration: discard non-migratable RAMBlocks

2018-06-03 Thread Juan Quintela
Cédric Le Goater  wrote:
> On the POWER9 processor, the XIVE interrupt controller can control
> interrupt sources using MMIO to trigger events, to EOI or to turn off
> the sources. Priority management and interrupt acknowledgment is also
> controlled by MMIO in the presenter sub-engine.
>
> These MMIO regions are exposed to guests in QEMU with a set of 'ram
> device' memory mappings, similarly to VFIO, and the VMAs are populated
> dynamically with the appropriate pages using a fault handler.
>
> But, these regions are an issue for migration. We need to discard the
> associated RAMBlocks from the RAM state on the source VM and let the
> destination VM rebuild the memory mappings on the new host in the
> post_load() operation just before resuming the system.
>
> To achieve this goal, the following introduces a new RAMBlock flag
> RAM_MIGRATABLE which is updated in the vmstate_register_ram() and
> vmstate_unregister_ram() routines. This flag is then used by the
> migration to identify RAMBlocks to discard on the source. Some checks
> are also performed on the destination to make sure nothing invalid was
> sent.
>
> This change impacts the boston, malta and jazz mips boards for which
> migration compatibility is broken.
>
> Signed-off-by: Cédric Le Goater 

Reviewed-by: Juan Quintela 
queued



Re: [Qemu-devel] [PATCH v2 for 2.13] migration: Don't activate block devices if using -S

2018-06-03 Thread Juan Quintela
"Dr. David Alan Gilbert (git)"  wrote:
> From: "Dr. David Alan Gilbert" 
>
> Activating the block devices causes the locks to be taken on
> the backing file.  If we're running with -S and the destination libvirt
> hasn't started the destination with 'cont', it's expecting the locks are
> still untaken.
>
> Don't activate the block devices if we're not going to autostart the VM;
> 'cont' already will do that anyway.   This change is tied to the new
> migration capability 'late-block-activate' that defaults to off, keeping
> the old behaviour by default.
>
> bz: https://bugzilla.redhat.com/show_bug.cgi?id=1560854
> Signed-off-by: Dr. David Alan Gilbert 

Reviewed-by: Juan Quintela 



[Qemu-devel] [PULL 1/5] migration: introduce decompress-error-check

2018-06-03 Thread Juan Quintela
From: Xiao Guangrong 

QEMU 3.0 enables strict check for compression & decompression to
make the migration more robust, that depends on the source to fix
the internal design which triggers the unexpected error conditions

To make it work for migrating old version QEMU to 2.13 QEMU, we
introduce this parameter to disable the error check on the
destination which is the default behavior of the machine type
which is older than 2.13, alternately, the strict check can be
enabled explicitly as followings:
  -M pc-q35-2.11 -global migration.decompress-error-check=true

Signed-off-by: Xiao Guangrong 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 hw/arm/virt.c | 4 
 hw/i386/pc_piix.c | 1 +
 hw/i386/pc_q35.c  | 1 +
 include/hw/compat.h   | 7 ++-
 migration/migration.c | 4 
 migration/migration.h | 7 +++
 migration/ram.c   | 2 +-
 7 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index a3a28e20e8..b2a67a4c00 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1693,6 +1693,9 @@ static void machvirt_machine_init(void)
 }
 type_init(machvirt_machine_init);
 
+#define VIRT_COMPAT_2_12 \
+HW_COMPAT_2_12
+
 static void virt_2_12_instance_init(Object *obj)
 {
 VirtMachineState *vms = VIRT_MACHINE(obj);
@@ -1763,6 +1766,7 @@ static void virt_2_12_instance_init(Object *obj)
 
 static void virt_machine_2_12_options(MachineClass *mc)
 {
+SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_12);
 }
 DEFINE_VIRT_MACHINE_AS_LATEST(2, 12)
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index b4c5b03274..3d81136065 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -430,6 +430,7 @@ static void pc_i440fx_3_0_machine_options(MachineClass *m)
 pc_i440fx_machine_options(m);
 m->alias = "pc";
 m->is_default = 1;
+SET_MACHINE_COMPAT(m, PC_COMPAT_2_12);
 }
 
 DEFINE_I440FX_MACHINE(v3_0, "pc-i440fx-3.0", NULL,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 83d6d75efa..b60cbb9266 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -312,6 +312,7 @@ static void pc_q35_3_0_machine_options(MachineClass *m)
 {
 pc_q35_machine_options(m);
 m->alias = "q35";
+SET_MACHINE_COMPAT(m, PC_COMPAT_2_12);
 }
 
 DEFINE_Q35_MACHINE(v3_0, "pc-q35-3.0", NULL,
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 4681c2719a..563908b874 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -1,7 +1,12 @@
 #ifndef HW_COMPAT_H
 #define HW_COMPAT_H
 
-#define HW_COMPAT_2_12
+#define HW_COMPAT_2_12 \
+{\
+.driver   = "migration",\
+.property = "decompress-error-check",\
+.value= "off",\
+},
 
 #define HW_COMPAT_2_11 \
 {\
diff --git a/migration/migration.c b/migration/migration.c
index 05aec2c905..a5384865ff 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2971,6 +2971,8 @@ void migration_global_dump(Monitor *mon)
ms->send_configuration ? "on" : "off");
 monitor_printf(mon, "send-section-footer: %s\n",
ms->send_section_footer ? "on" : "off");
+monitor_printf(mon, "decompress-error-check: %s\n",
+   ms->decompress_error_check ? "on" : "off");
 }
 
 #define DEFINE_PROP_MIG_CAP(name, x) \
@@ -2984,6 +2986,8 @@ static Property migration_properties[] = {
  send_configuration, true),
 DEFINE_PROP_BOOL("send-section-footer", MigrationState,
  send_section_footer, true),
+DEFINE_PROP_BOOL("decompress-error-check", MigrationState,
+  decompress_error_check, true),
 
 /* Migration parameters */
 DEFINE_PROP_UINT8("x-compress-level", MigrationState,
diff --git a/migration/migration.h b/migration/migration.h
index 8f0c82159b..5af57d616c 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -212,6 +212,13 @@ struct MigrationState
 /* Needed by postcopy-pause state */
 QemuSemaphore postcopy_pause_sem;
 QemuSemaphore postcopy_pause_rp_sem;
+/*
+ * Whether we abort the migration if decompression errors are
+ * detected at the destination. It is left at false for qemu
+ * older than 3.0, since only newer qemu sends streams that
+ * do not trigger spurious decompression errors.
+ */
+bool decompress_error_check;
 };
 
 void migrate_set_state(int *state, int old_state, int new_state);
diff --git a/migration/ram.c b/migration/ram.c
index c53e8369a3..090187ca04 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2881,7 +2881,7 @@ static void *do_data_decompress(void *opaque)
 
 ret = qemu_uncompress_data(¶m->stream, des, pagesize,
param->compbuf, len);
-if (ret < 0) {
+if (ret < 0 && migrate_get_current()->decompress_error_check) {
 error_report("decompress data failed");
 qemu_file_set_error(decomp_file, ret);
 }
-- 
2.17.0




[Qemu-devel] [PULL 0/5] Migration pull request

2018-06-03 Thread Juan Quintela
The following changes since commit 392fba9f583223786f844dce9b2e7f9a0ce0147a:

  Merge remote-tracking branch 
'remotes/stsquad/tags/pull-travis-updates-010618-1' into staging (2018-06-01 
17:32:30 +0100)

are available in the Git repository at:

  git://github.com/juanquintela/qemu.git tags/migration/20180604

for you to fetch changes up to c5e76115ccb4979cec795a8ae38becd07c2fde9f:

  migration: not wait RDMA_CM_EVENT_DISCONNECTED event after rdma_disconnect 
(2018-06-04 05:46:15 +0200)


migration/next for 20180604

- RDMA fixes from (lidong)
- Fix docempress-error-check (Xiao)
- make -S and block work better (dave)
- don't migrate "bad" ramblocks (Cédric)

Please apply, Juan.


Cédric Le Goater (1):
  migration: discard non-migratable RAMBlocks

Dr. David Alan Gilbert (1):
  migration: Don't activate block devices if using -S

Lidong Chen (2):
  migration: remove unnecessary variables len in QIOChannelRDMA
  migration: not wait RDMA_CM_EVENT_DISCONNECTED event after rdma_disconnect

Xiao Guangrong (1):
  migration: introduce decompress-error-check

 exec.c| 38 +
 hw/arm/virt.c |  4 
 hw/i386/pc_piix.c |  1 +
 hw/i386/pc_q35.c  |  1 +
 include/exec/cpu-common.h |  4 
 include/hw/compat.h   |  7 ++-
 migration/migration.c | 38 ++---
 migration/migration.h |  7 +++
 migration/postcopy-ram.c  | 12 ++--
 migration/ram.c   | 48 ++-
 migration/rdma.c  | 27 +-
 migration/savevm.c|  2 ++
 migration/trace-events|  1 -
 qapi/migration.json   |  6 +-
 14 files changed, 149 insertions(+), 47 deletions(-)



[Qemu-devel] [PULL 3/5] migration: Don't activate block devices if using -S

2018-06-03 Thread Juan Quintela
From: "Dr. David Alan Gilbert" 

Activating the block devices causes the locks to be taken on
the backing file.  If we're running with -S and the destination libvirt
hasn't started the destination with 'cont', it's expecting the locks are
still untaken.

Don't activate the block devices if we're not going to autostart the VM;
'cont' already will do that anyway.   This change is tied to the new
migration capability 'late-block-activate' that defaults to off, keeping
the old behaviour by default.

bz: https://bugzilla.redhat.com/show_bug.cgi?id=1560854
Signed-off-by: Dr. David Alan Gilbert 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/migration.c | 34 +++---
 qapi/migration.json   |  6 +-
 2 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index a5384865ff..1e99ec9b7e 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -202,6 +202,16 @@ static void migrate_generate_event(int new_state)
 }
 }
 
+static bool migrate_late_block_activate(void)
+{
+MigrationState *s;
+
+s = migrate_get_current();
+
+return s->enabled_capabilities[
+MIGRATION_CAPABILITY_LATE_BLOCK_ACTIVATE];
+}
+
 /*
  * Called on -incoming with a defer: uri.
  * The migration can be started later after any parameters have been
@@ -311,13 +321,23 @@ static void process_incoming_migration_bh(void *opaque)
 Error *local_err = NULL;
 MigrationIncomingState *mis = opaque;
 
-/* Make sure all file formats flush their mutable metadata.
- * If we get an error here, just don't restart the VM yet. */
-bdrv_invalidate_cache_all(&local_err);
-if (local_err) {
-error_report_err(local_err);
-local_err = NULL;
-autostart = false;
+/* If capability late_block_activate is set:
+ * Only fire up the block code now if we're going to restart the
+ * VM, else 'cont' will do it.
+ * This causes file locking to happen; so we don't want it to happen
+ * unless we really are starting the VM.
+ */
+if (!migrate_late_block_activate() ||
+ (autostart && (!global_state_received() ||
+global_state_get_runstate() == RUN_STATE_RUNNING))) {
+/* Make sure all file formats flush their mutable metadata.
+ * If we get an error here, just don't restart the VM yet. */
+bdrv_invalidate_cache_all(&local_err);
+if (local_err) {
+error_report_err(local_err);
+local_err = NULL;
+autostart = false;
+}
 }
 
 /*
diff --git a/qapi/migration.json b/qapi/migration.json
index dc9cc85545..f7e10ee90f 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -376,13 +376,17 @@
 # @postcopy-blocktime: Calculate downtime for postcopy live migration
 # (since 3.0)
 #
+# @late-block-activate: If enabled, the destination will not activate block
+#   devices (and thus take locks) immediately at the end of migration.
+#   (since 3.0)
+#
 # Since: 1.2
 ##
 { 'enum': 'MigrationCapability',
   'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram',
'block', 'return-path', 'pause-before-switchover', 'x-multifd',
-   'dirty-bitmaps', 'postcopy-blocktime' ] }
+   'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate' ] }
 
 ##
 # @MigrationCapabilityStatus:
-- 
2.17.0




[Qemu-devel] [PULL 2/5] migration: discard non-migratable RAMBlocks

2018-06-03 Thread Juan Quintela
From: Cédric Le Goater 

On the POWER9 processor, the XIVE interrupt controller can control
interrupt sources using MMIO to trigger events, to EOI or to turn off
the sources. Priority management and interrupt acknowledgment is also
controlled by MMIO in the presenter sub-engine.

These MMIO regions are exposed to guests in QEMU with a set of 'ram
device' memory mappings, similarly to VFIO, and the VMAs are populated
dynamically with the appropriate pages using a fault handler.

But, these regions are an issue for migration. We need to discard the
associated RAMBlocks from the RAM state on the source VM and let the
destination VM rebuild the memory mappings on the new host in the
post_load() operation just before resuming the system.

To achieve this goal, the following introduces a new RAMBlock flag
RAM_MIGRATABLE which is updated in the vmstate_register_ram() and
vmstate_unregister_ram() routines. This flag is then used by the
migration to identify RAMBlocks to discard on the source. Some checks
are also performed on the destination to make sure nothing invalid was
sent.

This change impacts the boston, malta and jazz mips boards for which
migration compatibility is broken.

Signed-off-by: Cédric Le Goater 
Reviewed-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Juan Quintela 
---
 exec.c| 38 
 include/exec/cpu-common.h |  4 
 migration/postcopy-ram.c  | 12 +-
 migration/ram.c   | 46 +--
 migration/savevm.c|  2 ++
 5 files changed, 84 insertions(+), 18 deletions(-)

diff --git a/exec.c b/exec.c
index c30f905598..af90e914cf 100644
--- a/exec.c
+++ b/exec.c
@@ -104,6 +104,9 @@ static MemoryRegion io_mem_unassigned;
  * (Set during postcopy)
  */
 #define RAM_UF_ZEROPAGE (1 << 3)
+
+/* RAM can be migrated */
+#define RAM_MIGRATABLE (1 << 4)
 #endif
 
 #ifdef TARGET_PAGE_BITS_VARY
@@ -1838,6 +1841,21 @@ void qemu_ram_set_uf_zeroable(RAMBlock *rb)
 rb->flags |= RAM_UF_ZEROPAGE;
 }
 
+bool qemu_ram_is_migratable(RAMBlock *rb)
+{
+return rb->flags & RAM_MIGRATABLE;
+}
+
+void qemu_ram_set_migratable(RAMBlock *rb)
+{
+rb->flags |= RAM_MIGRATABLE;
+}
+
+void qemu_ram_unset_migratable(RAMBlock *rb)
+{
+rb->flags &= ~RAM_MIGRATABLE;
+}
+
 /* Called with iothread lock held.  */
 void qemu_ram_set_idstr(RAMBlock *new_block, const char *name, DeviceState 
*dev)
 {
@@ -3893,6 +3911,26 @@ int qemu_ram_foreach_block(RAMBlockIterFunc func, void 
*opaque)
 return ret;
 }
 
+int qemu_ram_foreach_migratable_block(RAMBlockIterFunc func, void *opaque)
+{
+RAMBlock *block;
+int ret = 0;
+
+rcu_read_lock();
+RAMBLOCK_FOREACH(block) {
+if (!qemu_ram_is_migratable(block)) {
+continue;
+}
+ret = func(block->idstr, block->host, block->offset,
+   block->used_length, opaque);
+if (ret) {
+break;
+}
+}
+rcu_read_unlock();
+return ret;
+}
+
 /*
  * Unmap pages of memory from start to start+length such that
  * they a) read as 0, b) Trigger whatever fault mechanism
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 24d335f95d..0b58e262f3 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -75,6 +75,9 @@ const char *qemu_ram_get_idstr(RAMBlock *rb);
 bool qemu_ram_is_shared(RAMBlock *rb);
 bool qemu_ram_is_uf_zeroable(RAMBlock *rb);
 void qemu_ram_set_uf_zeroable(RAMBlock *rb);
+bool qemu_ram_is_migratable(RAMBlock *rb);
+void qemu_ram_set_migratable(RAMBlock *rb);
+void qemu_ram_unset_migratable(RAMBlock *rb);
 
 size_t qemu_ram_pagesize(RAMBlock *block);
 size_t qemu_ram_pagesize_largest(void);
@@ -119,6 +122,7 @@ typedef int (RAMBlockIterFunc)(const char *block_name, void 
*host_addr,
 ram_addr_t offset, ram_addr_t length, void *opaque);
 
 int qemu_ram_foreach_block(RAMBlockIterFunc func, void *opaque);
+int qemu_ram_foreach_migratable_block(RAMBlockIterFunc func, void *opaque);
 int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length);
 
 #endif
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 658b750a8e..48e51556a7 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -374,7 +374,7 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState 
*mis)
 }
 
 /* We don't support postcopy with shared RAM yet */
-if (qemu_ram_foreach_block(test_ramblock_postcopiable, NULL)) {
+if (qemu_ram_foreach_migratable_block(test_ramblock_postcopiable, NULL)) {
 goto out;
 }
 
@@ -502,7 +502,7 @@ static int cleanup_range(const char *block_name, void 
*host_addr,
  */
 int postcopy_ram_incoming_init(MigrationIncomingState *mis, size_t ram_pages)
 {
-if (qemu_ram_foreach_block(init_range, NULL)) {
+if (qemu_ram_foreach_migratable_block(init_range, NULL)) {
 return -1;
 }
 
@@ -524,7 +524,7 @@ int postcopy_ram_incoming_cleanup(MigrationInco

[Qemu-devel] [PULL 4/5] migration: remove unnecessary variables len in QIOChannelRDMA

2018-06-03 Thread Juan Quintela
From: Lidong Chen 

Because qio_channel_rdma_writev and qio_channel_rdma_readv maybe invoked
by different threads concurrently, this patch removes unnecessary variables
len in QIOChannelRDMA and use local variable instead.

Signed-off-by: Lidong Chen 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 

Signed-off-by: Lidong Chen 
---
 migration/rdma.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 7d233b0820..60779221b1 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -400,7 +400,6 @@ struct QIOChannelRDMA {
 QIOChannel parent;
 RDMAContext *rdma;
 QEMUFile *file;
-size_t len;
 bool blocking; /* XXX we don't actually honour this yet */
 };
 
@@ -2608,6 +2607,7 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc,
 int ret;
 ssize_t done = 0;
 size_t i;
+size_t len = 0;
 
 CHECK_ERROR_STATE();
 
@@ -2627,10 +2627,10 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc,
 while (remaining) {
 RDMAControlHeader head;
 
-rioc->len = MIN(remaining, RDMA_SEND_INCREMENT);
-remaining -= rioc->len;
+len = MIN(remaining, RDMA_SEND_INCREMENT);
+remaining -= len;
 
-head.len = rioc->len;
+head.len = len;
 head.type = RDMA_CONTROL_QEMU_FILE;
 
 ret = qemu_rdma_exchange_send(rdma, &head, data, NULL, NULL, NULL);
@@ -2640,8 +2640,8 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc,
 return ret;
 }
 
-data += rioc->len;
-done += rioc->len;
+data += len;
+done += len;
 }
 }
 
@@ -2736,8 +2736,7 @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc,
 }
 }
 }
-rioc->len = done;
-return rioc->len;
+return done;
 }
 
 /*
-- 
2.17.0




[Qemu-devel] [PULL 5/5] migration: not wait RDMA_CM_EVENT_DISCONNECTED event after rdma_disconnect

2018-06-03 Thread Juan Quintela
From: Lidong Chen 

When cancel migration during RDMA precopy, the source qemu main thread hangs 
sometime.

The backtrace is:
(gdb) bt
#0  0x7f249eabd43d in write () from /lib64/libpthread.so.0
#1  0x7f24a1ce98e4 in rdma_get_cm_event (channel=0x4675d10, 
event=0x7ffe2f643dd0) at src/cma.c:2189
#2  0x007b6166 in qemu_rdma_cleanup (rdma=0x6784000) at 
migration/rdma.c:2296
#3  0x007b7cae in qio_channel_rdma_close (ioc=0x3bfcc30, errp=0x0) 
at migration/rdma.c:2999
#4  0x008db60e in qio_channel_close (ioc=0x3bfcc30, errp=0x0) at 
io/channel.c:273
#5  0x007a8765 in channel_close (opaque=0x3bfcc30) at 
migration/qemu-file-channel.c:98
#6  0x007a71f9 in qemu_fclose (f=0x527c000) at 
migration/qemu-file.c:334
#7  0x00795b96 in migrate_fd_cleanup (opaque=0x3b46280) at 
migration/migration.c:1162
#8  0x0093a71b in aio_bh_call (bh=0x3db7a20) at util/async.c:90
#9  0x0093a7b2 in aio_bh_poll (ctx=0x3b121c0) at util/async.c:118
#10 0x0093f2ad in aio_dispatch (ctx=0x3b121c0) at 
util/aio-posix.c:436
#11 0x0093ab41 in aio_ctx_dispatch (source=0x3b121c0, callback=0x0, 
user_data=0x0)
at util/async.c:261
#12 0x7f249f73c7aa in g_main_context_dispatch () from 
/lib64/libglib-2.0.so.0
#13 0x0093dc5e in glib_pollfds_poll () at util/main-loop.c:215
#14 0x0093dd4e in os_host_main_loop_wait (timeout=2800) at 
util/main-loop.c:263
#15 0x0093de05 in main_loop_wait (nonblocking=0) at 
util/main-loop.c:522
#16 0x005bc6a5 in main_loop () at vl.c:1944
#17 0x005c39b5 in main (argc=56, argv=0x7ffe2f6443f8, 
envp=0x3ad0030) at vl.c:4752

It does not get the RDMA_CM_EVENT_DISCONNECTED event after rdma_disconnect 
sometime.

According to IB Spec once active side send DREQ message, it should wait for 
DREP message
and only once it arrived it should trigger a DISCONNECT event. DREP message can 
be dropped
due to network issues.
For that case the spec defines a DREP_timeout state in the CM state machine, if 
the DREP is
dropped we should get a timeout and a TIMEWAIT_EXIT event will be trigger.
Unfortunately the current kernel CM implementation doesn't include the 
DREP_timeout state
and in above scenario we will not get DISCONNECT or TIMEWAIT_EXIT events.

So it should not invoke rdma_get_cm_event which may hang forever, and the event 
channel
is also destroyed in qemu_rdma_cleanup.

Signed-off-by: Lidong Chen 
Reviewed-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Juan Quintela 
---
 migration/rdma.c   | 12 ++--
 migration/trace-events |  1 -
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 60779221b1..05aee3d591 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -2267,8 +2267,7 @@ static int qemu_rdma_write(QEMUFile *f, RDMAContext *rdma,
 
 static void qemu_rdma_cleanup(RDMAContext *rdma)
 {
-struct rdma_cm_event *cm_event;
-int ret, idx;
+int idx;
 
 if (rdma->cm_id && rdma->connected) {
 if ((rdma->error_state ||
@@ -2282,14 +2281,7 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
 qemu_rdma_post_send_control(rdma, NULL, &head);
 }
 
-ret = rdma_disconnect(rdma->cm_id);
-if (!ret) {
-trace_qemu_rdma_cleanup_waiting_for_disconnect();
-ret = rdma_get_cm_event(rdma->channel, &cm_event);
-if (!ret) {
-rdma_ack_cm_event(cm_event);
-}
-}
+rdma_disconnect(rdma->cm_id);
 trace_qemu_rdma_cleanup_disconnect();
 rdma->connected = false;
 }
diff --git a/migration/trace-events b/migration/trace-events
index 3c798ddd11..4a768eaaeb 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -146,7 +146,6 @@ qemu_rdma_accept_pin_state(bool pin) "%d"
 qemu_rdma_accept_pin_verbsc(void *verbs) "Verbs context after listen: %p"
 qemu_rdma_block_for_wrid_miss(const char *wcompstr, int wcomp, const char 
*gcompstr, uint64_t req) "A Wanted wrid %s (%d) but got %s (%" PRIu64 ")"
 qemu_rdma_cleanup_disconnect(void) ""
-qemu_rdma_cleanup_waiting_for_disconnect(void) ""
 qemu_rdma_close(void) ""
 qemu_rdma_connect_pin_all_requested(void) ""
 qemu_rdma_connect_pin_all_outcome(bool pin) "%d"
-- 
2.17.0




Re: [Qemu-devel] [PULL 0/5] Migration pull request

2018-06-03 Thread no-reply
Hi,

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

Type: series
Message-id: 20180604042156.13812-1-quint...@redhat.com
Subject: [Qemu-devel] [PULL 0/5] Migration pull request

=== 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
 t [tag update]patchew/20180514065700.22202-1-...@kaod.org -> 
patchew/20180514065700.22202-1-...@kaod.org
 * [new tag]   patchew/20180604042156.13812-1-quint...@redhat.com 
-> patchew/20180604042156.13812-1-quint...@redhat.com
Switched to a new branch 'test'
99ac0fb20d migration: not wait RDMA_CM_EVENT_DISCONNECTED event after 
rdma_disconnect
7c902253bb migration: remove unnecessary variables len in QIOChannelRDMA
3c1203cb38 migration: Don't activate block devices if using -S
2ecc458546 migration: discard non-migratable RAMBlocks
718b388d4e migration: introduce decompress-error-check

=== OUTPUT BEGIN ===
Checking PATCH 1/5: migration: introduce decompress-error-check...
Checking PATCH 2/5: migration: discard non-migratable RAMBlocks...
ERROR: Macros with multiple statements should be enclosed in a do - while loop
#191: FILE: migration/ram.c:161:
+#define RAMBLOCK_FOREACH_MIGRATABLE(block) \
+RAMBLOCK_FOREACH(block)\
+if (!qemu_ram_is_migratable(block)) {} else

ERROR: trailing statements should be on next line
#193: FILE: migration/ram.c:163:
+if (!qemu_ram_is_migratable(block)) {} else

total: 2 errors, 0 warnings, 273 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.

Checking PATCH 3/5: migration: Don't activate block devices if using -S...
Checking PATCH 4/5: migration: remove unnecessary variables len in 
QIOChannelRDMA...
Checking PATCH 5/5: migration: not wait RDMA_CM_EVENT_DISCONNECTED event after 
rdma_disconnect...
=== OUTPUT END ===

Test command exited with code: 1


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

[Qemu-devel] [Bug 1426092] Re: virtio-balloon-device locks up Guest

2018-06-03 Thread Launchpad Bug Tracker
[Expired for QEMU because there has been no activity for 60 days.]

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

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

Title:
  virtio-balloon-device locks up Guest

Status in QEMU:
  Expired

Bug description:
  Setting arbitrary balloon target values locks up the guest in some cases 
crashes it, if the target memory is < used +~5% free.  
  Found while testing aggressive memory over-commit, scenarios.
  You get messages like:

  [  155.827448] [] (show_stack) from [] 
(dump_stack+0x6c/0x88)
  [  155.837076] [] (dump_stack) from [] 
(warn_alloc_failed+0xe0/0x120)
  [  155.847075] [] (warn_alloc_failed) from [] 
(__alloc_pages_nodemask+0x600/0x91c)
  [  155.859039] [] (__alloc_pages_nodemask) from [] 
(balloon_page_enqueue+0x20/0xbc)
  [  155.870556] [] (balloon_page_enqueue) from [] 
(balloon+0x140/0x2cc)
  [  155.881377] [] (balloon) from [] (kthread+0xd8/0xf4)

  page dumped bacause: nonzero _count
  BUG: BAad page state in process Xorg pfn:396e5

  Test Environment:
  x86_64
  Ubuntu 13.10, Guest Linux Kernel 3.19, qemu 2.2.0 with following patches 
applied - balloon OOM enhancement
  commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5
  Author: Raushaniya Maksudova 

  Boot guest with '-balloon virtio', -qmp  -hmp access
  1. sudo info balloon | socat - tcp4-connect:127.0.0.1:
 - or over qmp interface
  {"execute":"qom-get", "arguments": { "path": 
"/machine/peripheral-anon/device[1]","property": "guest-stats" }}
  {"return": {"stats": {"stat-swap-out": 0, "stat-free-memory": 513454080, 
"stat-minor-faults": 1261, "stat-major-faults": 0, "stat-total-memory": 
526073856, "stat-swap-in": 0}, "last-update": 11109}}

  2. Take memory down check free memory using (1)
  3. Issue "sudo echo balloon XXX | socat - tcp4-connect:127.0.0.1:" - XXX 
is value in threshold mentioned above
 and you get Guest lockup

  ARM - samething except '-device virtio-balloon-device' used

  Libvirt - virsh setmem causes same issue.

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



[Qemu-devel] [Bug 1025244] Re: qcow2 image increasing disk size above the virtual limit

2018-06-03 Thread Launchpad Bug Tracker
[Expired for QEMU because there has been no activity for 60 days.]

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

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

Title:
  qcow2 image increasing disk size above the virtual limit

Status in QEMU:
  Expired
Status in qemu-kvm package in Ubuntu:
  Expired

Bug description:
  Using qemu/kvm, qcow2 images, ext4 file systems on both guest and host
   Host and Guest: Ubuntu server 12.04 64bit
  To create an image I did this:

  qemu-img create -f qcow2 -o preallocation=metadata ubuntu-pdc-vda.img 
10737418240 (not sure about the exact bytes, but around this)
  ls -l ubuntu-pdc-vda.img
  fallocate -l theSizeInBytesFromAbove ubuntu-pdc-vda.img

  The problem is that the image is growing progressively and has
  obviously no limit, although I gave it one. The root filesystem's
  image is the same case:

  qemu-img info ubuntu-pdc-vda.img
   image: ubuntu-pdc-vda.img
   file format: qcow2
   virtual size: 10G (10737418240 bytes)
   disk size: 14G
   cluster_size: 65536

  and for confirmation:
   du -sh ubuntu-pdc-vda.img
   15G ubuntu-pdc-vda.img

  I made a test and saw that when I delete something from the guest, the real 
size of the image is not decreasing (I read it is normal). OK, but when I write 
something again, it doesn't use the freed space, but instead grows the image. 
So for example:
   1. The initial physical size of the image is 1GB.
   2. I copy 1GB of data in the guest. It's physical size becomes 2GB.
   3. I delete this data (1GB). The physical size of the image remains 2GB.
   4. I copy another 1GB of data to the guest.
   5. The physical size of the image becomes 3GB.
   6. And so on with no limit. It doesn't care if the virtual size is less.

  Is this normal - the real/physical size of the image to be larger than
  the virtual limit???

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



[Qemu-devel] [Bug 1025244] Re: qcow2 image increasing disk size above the virtual limit

2018-06-03 Thread Launchpad Bug Tracker
[Expired for qemu-kvm (Ubuntu) because there has been no activity for 60
days.]

** Changed in: qemu-kvm (Ubuntu)
   Status: Incomplete => Expired

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

Title:
  qcow2 image increasing disk size above the virtual limit

Status in QEMU:
  Expired
Status in qemu-kvm package in Ubuntu:
  Expired

Bug description:
  Using qemu/kvm, qcow2 images, ext4 file systems on both guest and host
   Host and Guest: Ubuntu server 12.04 64bit
  To create an image I did this:

  qemu-img create -f qcow2 -o preallocation=metadata ubuntu-pdc-vda.img 
10737418240 (not sure about the exact bytes, but around this)
  ls -l ubuntu-pdc-vda.img
  fallocate -l theSizeInBytesFromAbove ubuntu-pdc-vda.img

  The problem is that the image is growing progressively and has
  obviously no limit, although I gave it one. The root filesystem's
  image is the same case:

  qemu-img info ubuntu-pdc-vda.img
   image: ubuntu-pdc-vda.img
   file format: qcow2
   virtual size: 10G (10737418240 bytes)
   disk size: 14G
   cluster_size: 65536

  and for confirmation:
   du -sh ubuntu-pdc-vda.img
   15G ubuntu-pdc-vda.img

  I made a test and saw that when I delete something from the guest, the real 
size of the image is not decreasing (I read it is normal). OK, but when I write 
something again, it doesn't use the freed space, but instead grows the image. 
So for example:
   1. The initial physical size of the image is 1GB.
   2. I copy 1GB of data in the guest. It's physical size becomes 2GB.
   3. I delete this data (1GB). The physical size of the image remains 2GB.
   4. I copy another 1GB of data to the guest.
   5. The physical size of the image becomes 3GB.
   6. And so on with no limit. It doesn't care if the virtual size is less.

  Is this normal - the real/physical size of the image to be larger than
  the virtual limit???

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



Re: [Qemu-devel] [RFC v2 2/4] tests: iotests: don't compare SHUTDOWN event

2018-06-03 Thread Peter Xu
On Thu, May 31, 2018 at 09:42:23AM -0500, Eric Blake wrote:
> On 05/31/2018 12:16 AM, Peter Xu wrote:
> > This event is not really necessary.  After OOB series it might affect
> > the timing of the script so this event may or may not be there comparing
> > to the old *.out results.  Let's just filter it out.
> 
> This is worrying. Are you stating that the SHUTDOWN event can occur in a
> different order than it used to, or is it even worse that the SHUTDOWN event
> disappears altogether?  If enabling OOB makes the SHUTDOWN event sometimes
> disappear, that's a regression that we should fix.  If it just makes things
> occur in a different order, we need an explanation why that is okay.

The event might conditionally disappear in two of the 100+ qcow2
tests.  And when it happens, it's not disappearing in all the
testcases in the test but only some.  For example, 087 might
conditionally fail with this:

087 8s ... - output mismatch (see 087.out.bad)
--- /home/peterx/git/qemu/tests/qemu-iotests/087.out2018-06-01 
18:44:22.378982462 +0800
+++ /home/peterx/git/qemu/bin/tests/qemu-iotests/087.out.bad2018-06-01 
18:53:44.267840928 +0800
@@ -8,7 +8,6 @@
 {"return": {}}
 {"error": {"class": "GenericError", "desc": "'node-name' must be specified for 
the root node"}}
 {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"SHUTDOWN", "data": {"guest": false}}


 === Duplicate ID ===
@@ -53,7 +52,6 @@
 {"return": {}}
 {"return": {}}
 {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"SHUTDOWN", "data": {"guest": false}}


 === Missing driver ===

Firstly, it does not fail every time I run "./check -qcow2 087", but
it might fail like 1 out of 5.  Then, it's not failing all the
testcases in 087.  For above example, it's failing "Missing ID and
node-name" and "Encrypted image LUKS", and it can change too.

> 
> > 
> > Since some of the scripts are using qmp-pretty, we need some trick in
> > the filtering script to make sure sed works for multiple lines to
> > explicitly mask out this event.
> > 
> > CC: John Snow 
> > CC: Max Reitz 
> > CC: Kevin Wolf 
> > Signed-off-by: Peter Xu 
> > ---
> 
> > +++ b/tests/qemu-iotests/067.out
> > @@ -70,6 +70,7 @@ Testing: -drive 
> > file=TEST_DIR/t.IMGFMT,format=IMGFMT,if=none,id=disk -device vir
> >   }
> >   }
> > +
> >   === -drive/device_add and device_del ===
> 
> Why is this blank line being added (multiple times in this file)?  Is there
> something about the new filter that isn't quite stripping all the newlines
> when encountering the SHUTDOWN event in pretty form?
> 
> Aha - it's because test 067 is already doing a very similar filtering of
> events.  Can we reduce the code duplication by promoting _filter_qmp_events
> from there into common.filter (as a separate patch)?

Yeah, can do.

> 
> > +++ b/tests/qemu-iotests/common.filter
> > @@ -88,7 +88,10 @@ _filter_qmp()
> >   sed -e 's#\("\(micro\)\?seconds": \)[0-9]\+#\1 TIMESTAMP#g' \
> >   -e 's#^{"QMP":.*}$#QMP_VERSION#' \
> >   -e '/^"QMP": {\s*$/, /^}\s*$/ c\' \
> > --e 'QMP_VERSION'
> > +-e 'QMP_VERSION' | \
> > +tr '\n' '\r' | \
> > +sed -e 
> > 's/{\s*"timestamp":\s*{\s*"seconds":\s*TIMESTAMP,\s*"microseconds":\s*TIMESTAMP\s*},\s*"event":\s*"SHUTDOWN",\s*"data":\s*{\s*"guest":\s*false\s*}\s*}\s//'
> >  | \
> 
> Really long line. This should do the same:
> 
> sed -e 's/\r{\(\r[^}]\|[^\r]\)*SHUTDOWN\(\r[^}]\|[^\r]\)*\r}//'
> 
> where the \(\r[^}]\|[^\r]\)* subpattern picks up all line breaks that do not
> end the current top-level {}, as well as any non-line breaks.
> 
> In fact, if you like my suggestion about promoting the filter from 67 into
> common.filter, we have two use cases: filter a single pretty-printed filter,
> and filter ANY pretty-printed filter.  Maybe we do that as follows:
> 
> # $1 is a regex of event names to filter; leave empty to filter all
> _filter_qmp_events()
> {
> fluff='\(\r[^}]\|[^\r]\)*'
> tr \\n \\r | \
> sed -e "s/$fluff"'"event": "'"$1$fluff\\r}//" \
>   | tr \\r \\n
> }
> 
> at which point '_filter_qmp_events SHUTDOWN' works in this patch, and
> '_filter_qmp_events' works for 067.  [Untested, but hopefully that gives you
> some ideas to play with]

Regex magic.  Thanks for the lesson. :)

-- 
Peter Xu



Re: [Qemu-devel] [PATCH V8 01/17] filter-rewriter: fix memory leak for connection in connection_track_table

2018-06-03 Thread Jason Wang




On 2018年06月03日 13:05, Zhang Chen wrote:

After a net connection is closed, we didn't clear its releated resources
in connection_track_table, which will lead to memory leak.

Let't track the state of net connection, if it is closed, its related
resources will be cleared up.

Signed-off-by: zhanghailiang 
Signed-off-by: Zhang Chen 
---
  net/colo.h|  4 +++
  net/filter-rewriter.c | 69 ++-
  2 files changed, 66 insertions(+), 7 deletions(-)

diff --git a/net/colo.h b/net/colo.h
index da6c36dcf7..cd118510c5 100644
--- a/net/colo.h
+++ b/net/colo.h
@@ -18,6 +18,7 @@
  #include "slirp/slirp.h"
  #include "qemu/jhash.h"
  #include "qemu/timer.h"
+#include "slirp/tcp.h"
  
  #define HASHTABLE_MAX_SIZE 16384
  
@@ -86,6 +87,9 @@ typedef struct Connection {

   * run once in independent tcp connection
   */
  int syn_flag;
+
+int tcp_state; /* TCP FSM state */
+tcp_seq fin_ack_seq; /* the seq of 'fin=1,ack=1' */


So the question is, the state machine is not complete. I suspect there 
will be corner cases that will be left because of lacking sufficient 
states. LAST_ACK happens only for passive close. How about active close?


So I think we need either maintain a full state machine or not instead 
of a partial one. We don't want endless bugs.


Thanks


  } Connection;
  
  uint32_t connection_key_hash(const void *opaque);

diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
index 62dad2d773..0909a9a8af 100644
--- a/net/filter-rewriter.c
+++ b/net/filter-rewriter.c
@@ -59,9 +59,9 @@ static int is_tcp_packet(Packet *pkt)
  }
  
  /* handle tcp packet from primary guest */

-static int handle_primary_tcp_pkt(NetFilterState *nf,
+static int handle_primary_tcp_pkt(RewriterState *rf,
Connection *conn,
-  Packet *pkt)
+  Packet *pkt, ConnectionKey *key)
  {
  struct tcphdr *tcp_pkt;
  
@@ -99,15 +99,44 @@ static int handle_primary_tcp_pkt(NetFilterState *nf,

  net_checksum_calculate((uint8_t *)pkt->data + pkt->vnet_hdr_len,
 pkt->size - pkt->vnet_hdr_len);
  }
+/*
+ * Case 1:
+ * The *server* side of this connect is VM, *client* tries to close
+ * the connection.
+ *
+ * We got 'ack=1' packets from client side, it acks 'fin=1, ack=1'
+ * packet from server side. From this point, we can ensure that there
+ * will be no packets in the connection, except that, some errors
+ * happen between the path of 'filter object' and vNIC, if this rare
+ * case really happen, we can still create a new connection,
+ * So it is safe to remove the connection from connection_track_table.
+ *
+ */
+if ((conn->tcp_state == TCPS_LAST_ACK) &&
+(ntohl(tcp_pkt->th_ack) == (conn->fin_ack_seq + 1))) {
+g_hash_table_remove(rf->connection_track_table, key);
+}
+}
+/*
+ * Case 2:
+ * The *server* side of this connect is VM, *server* tries to close
+ * the connection.
+ *
+ * We got 'fin=1, ack=1' packet from client side, we need to
+ * record the seq of 'fin=1, ack=1' packet.
+ */
+if ((tcp_pkt->th_flags & (TH_ACK | TH_FIN)) == (TH_ACK | TH_FIN)) {
+conn->fin_ack_seq = htonl(tcp_pkt->th_seq);
+conn->tcp_state = TCPS_LAST_ACK;
  }
  
  return 0;

  }
  
  /* handle tcp packet from secondary guest */

-static int handle_secondary_tcp_pkt(NetFilterState *nf,
+static int handle_secondary_tcp_pkt(RewriterState *rf,
  Connection *conn,
-Packet *pkt)
+Packet *pkt, ConnectionKey *key)
  {
  struct tcphdr *tcp_pkt;
  
@@ -139,8 +168,34 @@ static int handle_secondary_tcp_pkt(NetFilterState *nf,

  net_checksum_calculate((uint8_t *)pkt->data + pkt->vnet_hdr_len,
 pkt->size - pkt->vnet_hdr_len);
  }
+/*
+ * Case 2:
+ * The *server* side of this connect is VM, *server* tries to close
+ * the connection.
+ *
+ * We got 'ack=1' packets from server side, it acks 'fin=1, ack=1'
+ * packet from client side. Like Case 1, there should be no packets
+ * in the connection from now know, But the difference here is
+ * if the packet is lost, We will get the resent 'fin=1,ack=1' packet.
+ * TODO: Fix above case.
+ */
+if ((conn->tcp_state == TCPS_LAST_ACK) &&
+(ntohl(tcp_pkt->th_ack) == (conn->fin_ack_seq + 1))) {
+g_hash_table_remove(rf->connection_track_table, key);
+}
+}
+/*
+ * Case 1:
+ * The *server* side of this connect is VM, *client* tries to close
+ * the connection.
+ *
+ * We got 'fin=1, ack=1' packet f

Re: [Qemu-devel] [RFC v2 0/2] kvm "fake DAX" device flushing

2018-06-03 Thread Pankaj Gupta


Hi Igor,

> 
> [...]
> > - Qemu virtio-pmem device
> >   It exposes a persistent memory range to KVM guest which
> >   at host side is file backed memory and works as persistent
> >   memory device. In addition to this it provides virtio
> >   device handling of flushing interface. KVM guest performs
> >   Qemu side asynchronous sync using this interface.
> a random high level question,
> Have you considered using a separate (from memory itself)
> virtio device as controller for exposing some memory, async flushing.
> And then just slaving pc-dimm devices to it with notification/ACPI
> code suppressed so that guest won't touch them?

No.

> 
> That way it might be more scale-able, you consume only 1 PCI slot
> for controller vs multiple for virtio-pmem devices.

That sounds like a good suggestion. I will note it as an
enhancement once we have other concerns related to basic working 
of 'flush' interface addressed. Then probably we can work on
things 'need to optimize' with robust core flush functionality. 

BTW any sample code doing this right now in Qemu? 

> 
> 
> > Changes from previous RFC[1]:
> > 
> > - Reuse existing 'pmem' code for registering persistent
> >   memory and other operations instead of creating an entirely
> >   new block driver.
> > - Use VIRTIO driver to register memory information with
> >   nvdimm_bus and create region_type accordingly.
> > - Call VIRTIO flush from existing pmem driver.
> > 
> > Details of project idea for 'fake DAX' flushing interface is
> > shared [2] & [3].
> > 
> > Pankaj Gupta (2):
> >Add virtio-pmem guest driver
> >pmem: device flush over VIRTIO
> > 
> > [1] https://marc.info/?l=linux-mm&m=150782346802290&w=2
> > [2] https://www.spinics.net/lists/kvm/msg149761.html
> > [3] https://www.spinics.net/lists/kvm/msg153095.html
> > 
> >  drivers/nvdimm/region_devs.c |7 ++
> >  drivers/virtio/Kconfig   |   12 +++
> >  drivers/virtio/Makefile  |1
> >  drivers/virtio/virtio_pmem.c |  118
> >  +++
> >  include/linux/libnvdimm.h|4 +
> >  include/uapi/linux/virtio_ids.h  |1
> >  include/uapi/linux/virtio_pmem.h |   58 +++
> >  7 files changed, 201 insertions(+)
> > 
> 
> 
> 



Re: [Qemu-devel] [PATCH 1/4] spapr: remove irq_hint parameter from spapr_irq_alloc()

2018-06-03 Thread Cédric Le Goater
>> The property is a public interface. Just because it's not used by
>> libvirt does not mean that nobody is using it. So yes, please follow the
>> rules and mark it as deprecated first for two release, before you really
>> remove it.
> 
> I don't see a section for 'Deprecated' properties in the qemu-doc files. 

So I added a new one, which gives us :

  B.7 Device options
B.7.1 Block device options
  B.7.1.1 "backing": "" (since 2.12.0)
B.7.2 vio-spapr-device device options
  B.7.2.1 "irq": "" (since 2.13.0)


Cheers,

C. 



Re: [Qemu-devel] [PATCH 7/8] virtio: get avail bytes check for packed ring

2018-06-03 Thread Wei Xu
On Wed, Apr 11, 2018 at 11:03:24AM +0800, Jason Wang wrote:
> 
> 
> On 2018年04月04日 20:54, w...@redhat.com wrote:
> >From: Wei Xu 
> >
> >mostly as same as 1.0, copy it separately for
> >prototype, need a refactoring.
> >
> >Signed-off-by: Wei Xu 
> >---
> >  hw/virtio/virtio.c | 142 
> > +++--
> >  1 file changed, 139 insertions(+), 3 deletions(-)
> >
> >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> >index def07c6..cf726f3 100644
> >--- a/hw/virtio/virtio.c
> >+++ b/hw/virtio/virtio.c
> >@@ -836,9 +836,9 @@ static int virtqueue_read_next_desc(VirtIODevice *vdev, 
> >VRingDesc *desc,
> >  return VIRTQUEUE_READ_DESC_MORE;
> >  }
> >-void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,
> >-   unsigned int *out_bytes,
> >-   unsigned max_in_bytes, unsigned 
> >max_out_bytes)
> >+static void virtqueue_get_avail_bytes_split(VirtQueue *vq,
> >+unsigned int *in_bytes, unsigned int *out_bytes,
> >+unsigned max_in_bytes, unsigned max_out_bytes)
> >  {
> >  VirtIODevice *vdev = vq->vdev;
> >  unsigned int max, idx;
> >@@ -961,6 +961,142 @@ err:
> >  goto done;
> >  }
> >+static void virtqueue_get_avail_bytes_packed(VirtQueue *vq,
> >+unsigned int *in_bytes, unsigned int *out_bytes,
> >+unsigned max_in_bytes, unsigned max_out_bytes)
> >+{
> >+VirtIODevice *vdev = vq->vdev;
> >+unsigned int max, idx;
> >+unsigned int total_bufs, in_total, out_total;
> >+MemoryRegionCache *desc_cache;
> >+VRingMemoryRegionCaches *caches;
> >+MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID;
> >+int64_t len = 0;
> >+VRingDescPacked desc;
> >+
> >+if (unlikely(!vq->packed.desc)) {
> >+if (in_bytes) {
> >+*in_bytes = 0;
> >+}
> >+if (out_bytes) {
> >+*out_bytes = 0;
> >+}
> >+return;
> >+}
> >+
> >+rcu_read_lock();
> >+idx = vq->last_avail_idx;
> >+total_bufs = in_total = out_total = 0;
> >+
> >+max = vq->packed.num;
> >+caches = vring_get_region_caches(vq);
> >+if (caches->desc.len < max * sizeof(VRingDescPacked)) {
> >+virtio_error(vdev, "Cannot map descriptor ring");
> >+goto err;
> >+}
> >+
> >+desc_cache = &caches->desc;
> >+vring_desc_read_packed(vdev, &desc, desc_cache, idx);
> >+while (is_desc_avail(&desc)) {
> >+unsigned int num_bufs;
> >+unsigned int i;
> >+
> >+num_bufs = total_bufs;
> >+
> >+if (desc.flags & VRING_DESC_F_INDIRECT) {
> >+if (desc.len % sizeof(VRingDescPacked)) {
> >+virtio_error(vdev, "Invalid size for indirect buffer 
> >table");
> >+goto err;
> >+}
> >+
> >+/* If we've got too many, that implies a descriptor loop. */
> >+if (num_bufs >= max) {
> >+virtio_error(vdev, "Looped descriptor");
> >+goto err;
> >+}
> >+
> >+/* loop over the indirect descriptor table */
> >+len = address_space_cache_init(&indirect_desc_cache,
> >+   vdev->dma_as,
> >+   desc.addr, desc.len, false);
> >+desc_cache = &indirect_desc_cache;
> >+if (len < desc.len) {
> >+virtio_error(vdev, "Cannot map indirect buffer");
> >+goto err;
> >+}
> >+
> >+max = desc.len / sizeof(VRingDescPacked);
> >+num_bufs = i = 0;
> >+vring_desc_read_packed(vdev, &desc, desc_cache, i);
> >+}
> >+
> >+do {
> >+/* If we've got too many, that implies a descriptor loop. */
> >+if (++num_bufs > max) {
> >+virtio_error(vdev, "Looped descriptor");
> >+goto err;
> >+}
> >+
> >+if (desc.flags & VRING_DESC_F_WRITE) {
> >+in_total += desc.len;
> >+} else {
> >+out_total += desc.len;
> >+}
> >+if (in_total >= max_in_bytes && out_total >= max_out_bytes) {
> >+goto done;
> >+}
> >+
> >+if (desc_cache == &indirect_desc_cache) {
> >+vring_desc_read_packed(vdev, &desc, desc_cache,
> >+   ++i % vq->packed.num);
> >+} else {
> >+vring_desc_read_packed(vdev, &desc, desc_cache,
> >+   ++idx % vq->packed.num);
> >+}
> 
> Need to make sure desc.flags was read before other fields, otherwise we may
> get stale address.

OK, need a barrier here, thanks.

Wei

> 
> Thanks
> 
> >+} while (desc.flags & VRING_DESC_F_NEXT);
> >+
> >+if (desc_cache == &indirect_des

Re: [Qemu-devel] [PATCH for-2.11.2] spapr: make pseries-2.11 the default machine type

2018-06-03 Thread David Gibson
On Tue, May 22, 2018 at 07:17:28PM +0200, Greg Kurz wrote:
> The spapr capability framework was introduced in QEMU 2.12. It allows
> to have an explicit control on how host features are exposed to the
> guest. This is especially needed to handle migration between hetero-
> geneous hosts (eg, POWER8 to POWER9). It is also used to expose fixes/
> workarounds against speculative execution vulnerabilities to guests.
> The framework was hence backported to QEMU 2.11.1, especially these
> commits:
> 
> 0fac4aa93074 spapr: Add pseries-2.12 machine type
> 9070f408f491 spapr: Treat Hardware Transactional Memory (HTM) as an
>  optional capability
> 
> 0fac4aa93074 has the confusing effect of making pseries-2.12 the default
> machine type for QEMU 2.11.1, instead of the expected pseries-2.11. This
> patch changes the default machine back to pseries-2.11.
> 
> Unfortunately, 9070f408f491 enforces the HTM capability for pseries-2.11.
> This isn't supported by TCG and breaks 'make check'. So this patch also
> adds a hack to turn HTM off when using TCG.
> 
> Signed-off-by: Greg Kurz 

Acked-by: David Gibson 

> ---
>  hw/ppc/spapr.c  |4 ++--
>  hw/ppc/spapr_caps.c |5 +
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 1a2dd1f597d9..6499a867520f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3820,7 +3820,7 @@ static void 
> spapr_machine_2_12_class_options(MachineClass *mc)
>  /* Defaults for the latest behaviour inherited from the base class */
>  }
>  
> -DEFINE_SPAPR_MACHINE(2_12, "2.12", true);
> +DEFINE_SPAPR_MACHINE(2_12, "2.12", false);
>  
>  /*
>   * pseries-2.11
> @@ -3842,7 +3842,7 @@ static void 
> spapr_machine_2_11_class_options(MachineClass *mc)
>  SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_11);
>  }
>  
> -DEFINE_SPAPR_MACHINE(2_11, "2.11", false);
> +DEFINE_SPAPR_MACHINE(2_11, "2.11", true);
>  
>  /*
>   * pseries-2.10
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 7b229517be38..82043e60e78b 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -285,6 +285,11 @@ static sPAPRCapabilities 
> default_caps_with_cpu(sPAPRMachineState *spapr,
>  
>  caps = smc->default_caps;
>  
> +/* HACK for 2.11.2: fix make check */
> +if (tcg_enabled()) {
> +caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF;
> +}
> +
>  if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_2_07,
>0, spapr->max_compat_pvr)) {
>  caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF;
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH 2/4] hw/ppc/spapr_drc: Replace error_setg(&error_abort) by error_report() + abort()

2018-06-03 Thread David Gibson
On Tue, May 29, 2018 at 02:48:19PM -0300, Philippe Mathieu-Daudé wrote:
> Use error_report() + abort() instead of error_setg(&error_abort),
> as suggested by the "qapi/error.h" documentation:
> 
> Please don't error_setg(&error_fatal, ...), use error_report() and
> exit(), because that's more obvious.
> Likewise, don't error_setg(&error_abort, ...), use assert().
> 
> Use abort() instead of the suggested assert() because the error message
> already got displayed.
> 
> Suggested-by: Eric Blake 
> Signed-off-by: Philippe Mathieu-Daudé 

Applied to ppc-for-2.13, thanks.

> ---
>  hw/ppc/spapr_drc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 8a045d6b93..2edb7d1e9c 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -366,7 +366,8 @@ static void prop_get_fdt(Object *obj, Visitor *v, const 
> char *name,
>  break;
>  }
>  default:
> -error_setg(&error_abort, "device FDT in unexpected state: %d", 
> tag);
> +error_report("device FDT in unexpected state: %d", tag);
> +abort();
>  }
>  fdt_offset = fdt_offset_next;
>  } while (fdt_depth != 0);

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH] target/ppc: Use proper logging function for possible guest errors

2018-06-03 Thread David Gibson
On Mon, May 28, 2018 at 08:11:19PM +0200, Thomas Huth wrote:
> fprintf() and qemu_log_separate() are frowned upon these days for printing
> logging information in QEMU. Accessing the wrong SPRs indicates wrong guest
> behaviour in most cases, and we've got a proper way to log such situations,
> which is the qemu_log_mask(LOG_GUEST_ERROR, ...) function. So use this
> function now for logging the bad SPR accesses instead.
> 
> Signed-off-by: Thomas Huth 

Applied to ppc-for-2.13, thanks.

> ---
>  target/ppc/translate.c | 37 -
>  1 file changed, 12 insertions(+), 25 deletions(-)
> 
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index e30d99f..0806ee0 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -3933,13 +3933,9 @@ static inline void gen_op_mfspr(DisasContext *ctx)
>   * allowing userland application to read the PVR
>   */
>  if (sprn != SPR_PVR) {
> -fprintf(stderr, "Trying to read privileged spr %d (0x%03x) 
> at "
> -TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 
> 4);
> -if (qemu_log_separate()) {
> -qemu_log("Trying to read privileged spr %d (0x%03x) at "
> - TARGET_FMT_lx "\n", sprn, sprn,
> - ctx->base.pc_next - 4);
> -}
> +qemu_log_mask(LOG_GUEST_ERROR, "Trying to read privileged 
> spr "
> +  "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, 
> sprn,
> +  ctx->base.pc_next - 4);
>  }
>  gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG);
>  }
> @@ -3951,12 +3947,9 @@ static inline void gen_op_mfspr(DisasContext *ctx)
>  return;
>  }
>  /* Not defined */
> -fprintf(stderr, "Trying to read invalid spr %d (0x%03x) at "
> -TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4);
> -if (qemu_log_separate()) {
> -qemu_log("Trying to read invalid spr %d (0x%03x) at "
> - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4);
> -}
> +qemu_log_mask(LOG_GUEST_ERROR,
> +  "Trying to read invalid spr %d (0x%03x) at "
> +  TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4);
>  
>  /* The behaviour depends on MSR:PR and SPR# bit 0x10,
>   * it can generate a priv, a hv emu or a no-op
> @@ -4097,12 +4090,9 @@ static void gen_mtspr(DisasContext *ctx)
>  (*write_cb)(ctx, sprn, rS(ctx->opcode));
>  } else {
>  /* Privilege exception */
> -fprintf(stderr, "Trying to write privileged spr %d (0x%03x) at "
> -TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4);
> -if (qemu_log_separate()) {
> -qemu_log("Trying to write privileged spr %d (0x%03x) at "
> - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 
> 4);
> -}
> +qemu_log_mask(LOG_GUEST_ERROR, "Trying to write privileged spr "
> +  "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn,
> +  ctx->base.pc_next - 4);
>  gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG);
>  }
>  } else {
> @@ -4114,12 +4104,9 @@ static void gen_mtspr(DisasContext *ctx)
>  }
>  
>  /* Not defined */
> -if (qemu_log_separate()) {
> -qemu_log("Trying to write invalid spr %d (0x%03x) at "
> - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4);
> -}
> -fprintf(stderr, "Trying to write invalid spr %d (0x%03x) at "
> -TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4);
> +qemu_log_mask(LOG_GUEST_ERROR,
> +  "Trying to write invalid spr %d (0x%03x) at "
> +  TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4);
>  
>  
>  /* The behaviour depends on MSR:PR and SPR# bit 0x10,

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH] 40p: remove pci_allow_0_address = true from 40p machine class

2018-06-03 Thread David Gibson
On Fri, May 25, 2018 at 10:15:23PM +0100, Mark Cave-Ayland wrote:
> The Linux sandalfoot zImage has an initialisation process which resets the
> VGA controller by setting all the BAR addresses to zero to access the VGA
> ioports at their legacy addresses.
> 
> Unfortunately setting the framebuffer BAR to address 0 makes the framebuffer
> memory overlap the internal VGA memory causing accesses to fail, and so
> prevents the kernel from switching successfully to text mode.
> 
> Since OpenHackWare configures the framebuffer BAR address outside of the 
> legacy
> VGA internal memory space, remove pci_allow_0_address from the 40p machine 
> class
> which causes the BAR reprogramming to zero to fail and so the VGA internal
> memory can be accessed correctly again.
> 
> Signed-off-by: Mark Cave-Ayland 

Applied, thanks.

> ---
>  hw/ppc/prep.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index be4db6a687..5ed0bcd862 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -884,7 +884,6 @@ static void ibm_40p_machine_init(MachineClass *mc)
>  mc->desc = "IBM RS/6000 7020 (40p)",
>  mc->init = ibm_40p_init;
>  mc->max_cpus = 1;
> -mc->pci_allow_0_address = true;
>  mc->default_ram_size = 128 * M_BYTE;
>  mc->block_default_type = IF_SCSI;
>  mc->default_boot_order = "c";

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH] prep: fix keyboard for the 40p machine

2018-06-03 Thread David Gibson
On Thu, May 24, 2018 at 06:39:58AM +0100, Mark Cave-Ayland wrote:
> Commit 72d3d8f052 "hw/isa/superio: Add a keyboard/mouse controller (8042)"
> added an 8042 keyboard device to the PC87312 superio device to replace that
> being used by the prep machine.
> 
> Unfortunately this commit didn't do the same for the 40p machine which broke
> the keyboard by registering two 8042 keyboard devices at the same address.
> 
> Resolve this by similarly removing the 8042 keyboard from the 40p machine as
> done for the prep machine in commit 72d3d8f052.
> 
> Signed-off-by: Mark Cave-Ayland 

Applied to ppc-for-3.0.

> ---
>  hw/ppc/prep.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index a1e7219db6..be4db6a687 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -770,7 +770,6 @@ static void ibm_40p_init(MachineState *machine)
>  
>  /* add some more devices */
>  if (defaults_enabled()) {
> -isa_create_simple(isa_bus, TYPE_I8042);
>  m48t59 = NVRAM(isa_create_simple(isa_bus, "isa-m48t59"));
>  
>  dev = DEVICE(isa_create(isa_bus, "cs4231a"));

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH V8 02/17] colo-compare: implement the process of checkpoint

2018-06-03 Thread Jason Wang




On 2018年06月03日 13:05, Zhang Chen wrote:

While do checkpoint, we need to flush all the unhandled packets,
By using the filter notifier mechanism, we can easily to notify
every compare object to do this process, which runs inside
of compare threads as a coroutine.

Signed-off-by: zhanghailiang 
Signed-off-by: Zhang Chen 
---
  include/migration/colo.h |  6 
  net/colo-compare.c   | 76 
  net/colo-compare.h   | 22 
  3 files changed, 104 insertions(+)
  create mode 100644 net/colo-compare.h

diff --git a/include/migration/colo.h b/include/migration/colo.h
index 2fe48ad353..fefb2fcf4c 100644
--- a/include/migration/colo.h
+++ b/include/migration/colo.h
@@ -16,6 +16,12 @@
  #include "qemu-common.h"
  #include "qapi/qapi-types-migration.h"
  
+enum colo_event {

+COLO_EVENT_NONE,
+COLO_EVENT_CHECKPOINT,
+COLO_EVENT_FAILOVER,
+};
+
  void colo_info_init(void);
  
  void migrate_start_colo_process(MigrationState *s);

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 23b2d2c4cc..7ff3ae8904 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -27,11 +27,16 @@
  #include "qemu/sockets.h"
  #include "net/colo.h"
  #include "sysemu/iothread.h"
+#include "net/colo-compare.h"
+#include "migration/colo.h"
  
  #define TYPE_COLO_COMPARE "colo-compare"

  #define COLO_COMPARE(obj) \
  OBJECT_CHECK(CompareState, (obj), TYPE_COLO_COMPARE)
  
+static QTAILQ_HEAD(, CompareState) net_compares =

+   QTAILQ_HEAD_INITIALIZER(net_compares);
+
  #define COMPARE_READ_LEN_MAX NET_BUFSIZE
  #define MAX_QUEUE_SIZE 1024
  
@@ -41,6 +46,10 @@

  /* TODO: Should be configurable */
  #define REGULAR_PACKET_CHECK_MS 3000
  
+static QemuMutex event_mtx;

+static QemuCond event_complete_cond;
+static int event_unhandled_count;
+
  /*
   *  + CompareState ++
   *  |   |
@@ -87,6 +96,11 @@ typedef struct CompareState {
  IOThread *iothread;
  GMainContext *worker_context;
  QEMUTimer *packet_check_timer;
+
+QEMUBH *event_bh;
+enum colo_event event;
+
+QTAILQ_ENTRY(CompareState) next;
  } CompareState;
  
  typedef struct CompareClass {

@@ -736,6 +750,25 @@ static void check_old_packet_regular(void *opaque)
  REGULAR_PACKET_CHECK_MS);
  }
  
+/* Public API, Used for COLO frame to notify compare event */

+void colo_notify_compares_event(void *opaque, int event, Error **errp)
+{
+CompareState *s;
+
+qemu_mutex_lock(&event_mtx);
+QTAILQ_FOREACH(s, &net_compares, next) {
+s->event = event;
+qemu_bh_schedule(s->event_bh);
+event_unhandled_count++;
+}
+/* Wait all compare threads to finish handling this event */
+while (event_unhandled_count > 0) {
+qemu_cond_wait(&event_complete_cond, &event_mtx);
+}
+
+qemu_mutex_unlock(&event_mtx);
+}
+
  static void colo_compare_timer_init(CompareState *s)
  {
  AioContext *ctx = iothread_get_aio_context(s->iothread);
@@ -756,6 +789,28 @@ static void colo_compare_timer_del(CompareState *s)
  }
   }
  
+static void colo_flush_packets(void *opaque, void *user_data);

+
+static void colo_compare_handle_event(void *opaque)
+{
+CompareState *s = opaque;
+
+switch (s->event) {
+case COLO_EVENT_CHECKPOINT:
+g_queue_foreach(&s->conn_list, colo_flush_packets, s);
+break;
+case COLO_EVENT_FAILOVER:
+break;
+default:
+break;
+}
+qemu_mutex_lock(&event_mtx);


Isn't this a deadlock? Since colo_notify_compares_event() won't release 
event_mtx until event_unhandled_count reaches zero.



+assert(event_unhandled_count > 0);
+event_unhandled_count--;
+qemu_cond_broadcast(&event_complete_cond);
+qemu_mutex_unlock(&event_mtx);
+}
+
  static void colo_compare_iothread(CompareState *s)
  {
  object_ref(OBJECT(s->iothread));
@@ -769,6 +824,7 @@ static void colo_compare_iothread(CompareState *s)
   s, s->worker_context, true);
  
  colo_compare_timer_init(s);

+s->event_bh = qemu_bh_new(colo_compare_handle_event, s);
  }
  
  static char *compare_get_pri_indev(Object *obj, Error **errp)

@@ -926,8 +982,13 @@ static void colo_compare_complete(UserCreatable *uc, Error 
**errp)
  net_socket_rs_init(&s->pri_rs, compare_pri_rs_finalize, s->vnet_hdr);
  net_socket_rs_init(&s->sec_rs, compare_sec_rs_finalize, s->vnet_hdr);
  
+QTAILQ_INSERT_TAIL(&net_compares, s, next);

+
  g_queue_init(&s->conn_list);
  
+qemu_mutex_init(&event_mtx);

+qemu_cond_init(&event_complete_cond);
+
  s->connection_track_table = g_hash_table_new_full(connection_key_hash,
connection_key_equal,
g_free,
@@ -990,6 +1051,7 @@ static void colo_compare_init(Object *obj)
  static void colo_compare_finalize(Object *obj)
  {
  CompareState *s = COLO_COMPARE(obj);
+CompareState *tmp = NULL;

Re: [Qemu-devel] [PATCH V8 03/17] colo-compare: use notifier to notify packets comparing result

2018-06-03 Thread Jason Wang




On 2018年06月03日 13:05, Zhang Chen wrote:

It's a good idea to use notifier to notify COLO frame of
inconsistent packets comparing.

Signed-off-by: Zhang Chen 
Signed-off-by: zhanghailiang 
---
  net/colo-compare.c | 32 +---
  net/colo-compare.h |  2 ++
  2 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 7ff3ae8904..05061cd1c4 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -29,6 +29,7 @@
  #include "sysemu/iothread.h"
  #include "net/colo-compare.h"
  #include "migration/colo.h"
+#include "migration/migration.h"
  
  #define TYPE_COLO_COMPARE "colo-compare"

  #define COLO_COMPARE(obj) \
@@ -37,6 +38,9 @@
  static QTAILQ_HEAD(, CompareState) net_compares =
 QTAILQ_HEAD_INITIALIZER(net_compares);
  
+static NotifierList colo_compare_notifiers =

+NOTIFIER_LIST_INITIALIZER(colo_compare_notifiers);
+
  #define COMPARE_READ_LEN_MAX NET_BUFSIZE
  #define MAX_QUEUE_SIZE 1024
  
@@ -561,8 +565,24 @@ static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)

  }
  }
  
+static void colo_compare_inconsistent_notify(void)

+{


Not good at English but inconsistency sounds better here.

Thanks


+notifier_list_notify(&colo_compare_notifiers,
+migrate_get_current());
+}
+
+void colo_compare_register_notifier(Notifier *notify)
+{
+notifier_list_add(&colo_compare_notifiers, notify);
+}
+
+void colo_compare_unregister_notifier(Notifier *notify)
+{
+notifier_remove(notify);
+}
+
  static int colo_old_packet_check_one_conn(Connection *conn,
-  void *user_data)
+   void *user_data)
  {
  GList *result = NULL;
  int64_t check_time = REGULAR_PACKET_CHECK_MS;
@@ -573,10 +593,7 @@ static int colo_old_packet_check_one_conn(Connection *conn,
  
  if (result) {

  /* Do checkpoint will flush old packet */
-/*
- * TODO: Notify colo frame to do checkpoint.
- * colo_compare_inconsistent_notify();
- */
+colo_compare_inconsistent_notify();
  return 0;
  }
  
@@ -620,11 +637,12 @@ static void colo_compare_packet(CompareState *s, Connection *conn,

  /*
   * If one packet arrive late, the secondary_list or
   * primary_list will be empty, so we can't compare it
- * until next comparison.
+ * until next comparison. If the packets in the list are
+ * timeout, it will trigger a checkpoint request.
   */
  trace_colo_compare_main("packet different");
  g_queue_push_head(&conn->primary_list, pkt);
-/* TODO: colo_notify_checkpoint();*/
+colo_compare_inconsistent_notify();
  break;
  }
  }
diff --git a/net/colo-compare.h b/net/colo-compare.h
index 1b1ce76aea..22ddd512e2 100644
--- a/net/colo-compare.h
+++ b/net/colo-compare.h
@@ -18,5 +18,7 @@
  #define QEMU_COLO_COMPARE_H
  
  void colo_notify_compares_event(void *opaque, int event, Error **errp);

+void colo_compare_register_notifier(Notifier *notify);
+void colo_compare_unregister_notifier(Notifier *notify);
  
  #endif /* QEMU_COLO_COMPARE_H */





Re: [Qemu-devel] [PATCH v4 00/19] reverse debugging

2018-06-03 Thread Pavel Dovgalyuk
Ping?



⁣Отправлено с помощью BlueMail ​

На 28 Май 2018 г., 10:13, в 10:13, Pavel Dovgalyuk  
написал:п>GDB remote protocol supports reverse debugging of the targets.
>It includes 'reverse step' and 'reverse continue' operations.
>The first one finds the previous step of the execution,
>and the second one is intended to stop at the last breakpoint that
>would happen when the program is executed normally.
>
>Reverse debugging is possible in the replay mode, when at least
>one snapshot was created at the record or replay phase.
>QEMU can use these snapshots for travelling back in time with GDB.
>
>Running the execution in replay mode allows using GDB reverse debugging
>commands:
> - reverse-stepi (or rsi): Steps one instruction to the past.
>   QEMU loads on of the prior snapshots and proceeds to the desired
>   instruction forward. When that step is reaches, execution stops.
> - reverse-continue (or rc): Runs execution "backwards".
>   QEMU tries to find breakpoint or watchpoint by loaded prior snapshot
>   and replaying the execution. Then QEMU loads snapshots again and
>   replays to the latest breakpoint. When there are no breakpoints in
>   the examined section of the execution, QEMU finds one more snapshot
>   and tries again. After the first snapshot is processed, execution
>   stops at this snapshot.
>
>The set of patches include the following modifications:
> - gdbstub update for reverse debugging support
> - functions that automatically perform reverse step and reverse
>   continue operations
> - hmp/qmp commands for manipulating the replay process
> - improvement of the snapshotting for saving the execution step
>   in the snapshot parameters
> - other record/replay fixes
>
>The patches are available in the repository:
>https://github.com/ispras/qemu/tree/rr-180524
>
>v4 changes:
>- changed 'since 2.13' to 'since 3.0' in json (as suggested by Eric
>Blake)
>
>v3 changes:
>- Fixed PS/2 bug with save/load vm, which caused failures of the
>replay.
>   The patch was sent separately.
> - Rebased to the new code base.
> - Minor fixes.
>
>v2 changes:
> - documented reverse debugging
> - fixed start vmstate loading in record mode
> - documented qcow2 changes (as suggested by Eric Blake)
> - made icount SnapshotInfo field optional (as suggested by Eric Blake)
> - renamed qmp commands (as suggested by Eric Blake)
> - minor changes
>
>---
>
>Pavel Dovgalyuk (19):
>  block: implement bdrv_snapshot_goto for blkreplay
>  replay: disable default snapshot for record/replay
>  replay: update docs for record/replay with block devices
>  replay: don't drain/flush bdrv queue while RR is working
>  replay: finish record/replay before closing the disks
>  qcow2: introduce icount field for snapshots
>  migration: introduce icount field for snapshots
>  replay: introduce info hmp/qmp command
>  replay: introduce breakpoint at the specified step
>   replay: implement replay-seek command to proceed to the desired step
>  replay: flush events when exiting
>  timer: remove replay clock probe in deadline calculation
>  replay: refine replay-time module
>  translator: fix breakpoint processing
>  replay: flush rr queue before loading the vmstate
>  gdbstub: add reverse step support in replay mode
>  gdbstub: add reverse continue support in replay mode
>  replay: describe reverse debugging in docs/replay.txt
>  replay: allow loading any snapshots before recording
>
>
> accel/tcg/translator.c|9 +
> block/blkreplay.c |8 +
> block/io.c|   22 +++
> block/qapi.c  |   17 ++-
> block/qcow2-snapshot.c|9 +
> block/qcow2.h |2 
> blockdev.c|   10 ++
> cpus.c|   19 ++-
> docs/interop/qcow2.txt|4 +
> docs/replay.txt   |   45 +++
> exec.c|6 +
> gdbstub.c |   50 +++-
> hmp-commands-info.hx  |   14 ++
> hmp-commands.hx   |   30 +
> hmp.h |3
> include/block/snapshot.h  |1
> include/sysemu/replay.h   |   18 +++
> migration/savevm.c|   15 +-
> qapi/block-core.json  |5 +
> qapi/block.json   |3
> qapi/misc.json|   68 +++
> replay/Makefile.objs  |3
>replay/replay-debugging.c |  287
>+
> replay/replay-events.c|   14 --
> replay/replay-internal.h  |   10 +-
> replay/replay-snapshot.c  |   17 ++-
> replay/replay-time.c  |   27 ++--
> replay/replay.c   |   22 +++
> stubs/replay.c|   10 ++
> util/qemu-timer.c |   11 --
> vl.c  |   18 ++-
> 31 files changed, 696 insertions(+), 81 deletions(-)
> create mode 100644 replay/replay-debugging.c
>
>--
>Pavel Dovgalyuk


Re: [Qemu-devel] [PATCH V8 14/17] filter: Add handle_event method for NetFilterClass

2018-06-03 Thread Jason Wang




On 2018年06月03日 13:05, Zhang Chen wrote:

Filter needs to process the event of checkpoint/failover or
other event passed by COLO frame.

Signed-off-by: zhanghailiang 
---
  include/net/filter.h |  5 +
  net/filter.c | 17 +
  net/net.c| 28 
  3 files changed, 50 insertions(+)

diff --git a/include/net/filter.h b/include/net/filter.h
index 435acd6f82..49da666ac0 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -38,6 +38,8 @@ typedef ssize_t (FilterReceiveIOV)(NetFilterState *nc,
  
  typedef void (FilterStatusChanged) (NetFilterState *nf, Error **errp);
  
+typedef void (FilterHandleEvent) (NetFilterState *nf, int event, Error **errp);

+
  typedef struct NetFilterClass {
  ObjectClass parent_class;
  
@@ -45,6 +47,7 @@ typedef struct NetFilterClass {

  FilterSetup *setup;
  FilterCleanup *cleanup;
  FilterStatusChanged *status_changed;
+FilterHandleEvent *handle_event;
  /* mandatory */
  FilterReceiveIOV *receive_iov;
  } NetFilterClass;
@@ -77,4 +80,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
  int iovcnt,
  void *opaque);
  
+void colo_notify_filters_event(int event, Error **errp);

+
  #endif /* QEMU_NET_FILTER_H */
diff --git a/net/filter.c b/net/filter.c
index 2fd7d7d663..0f17eba143 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -17,6 +17,8 @@
  #include "net/vhost_net.h"
  #include "qom/object_interfaces.h"
  #include "qemu/iov.h"
+#include "net/colo.h"
+#include "migration/colo.h"
  
  static inline bool qemu_can_skip_netfilter(NetFilterState *nf)

  {
@@ -245,11 +247,26 @@ static void netfilter_finalize(Object *obj)
  g_free(nf->netdev_id);
  }
  
+static void dummy_handle_event(NetFilterState *nf, int event, Error **errp)

+{
+switch (event) {
+case COLO_EVENT_CHECKPOINT:
+break;
+case COLO_EVENT_FAILOVER:
+object_property_set_str(OBJECT(nf), "off", "status", errp);
+break;
+default:
+break;
+}
+}
+
  static void netfilter_class_init(ObjectClass *oc, void *data)
  {
  UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+NetFilterClass *nfc = NETFILTER_CLASS(oc);
  
  ucc->complete = netfilter_complete;

+nfc->handle_event = dummy_handle_event;
  }
  
  static const TypeInfo netfilter_info = {

diff --git a/net/net.c b/net/net.c
index efb9eaf779..378cd2f9ec 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1329,6 +1329,34 @@ void hmp_info_network(Monitor *mon, const QDict *qdict)
  }
  }
  
+void colo_notify_filters_event(int event, Error **errp)

+{
+NetClientState *nc, *peer;
+NetClientDriver type;
+NetFilterState *nf;
+NetFilterClass *nfc = NULL;
+Error *local_err = NULL;
+
+QTAILQ_FOREACH(nc, &net_clients, next) {
+peer = nc->peer;
+type = nc->info->type;
+if (!peer || type != NET_CLIENT_DRIVER_TAP) {
+continue;


I think when COLO is enabled, qemu should reject all other types of 
backends.



+}
+QTAILQ_FOREACH(nf, &nc->filters, next) {
+nfc =  NETFILTER_GET_CLASS(OBJECT(nf));
+if (!nfc->handle_event) {
+continue;
+}


Is this still necessary?

Thanks


+nfc->handle_event(nf, event, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+}
+}
+}
+
  void qmp_set_link(const char *name, bool up, Error **errp)
  {
  NetClientState *ncs[MAX_QUEUE_NUM];