Re: [Qemu-devel] [PATCH 1/1] block: add block device shared field

[Fam Zheng]

On Tue, 09/05 10:56, Stefan Hajnoczi wrote:
> On Fri, Sep 01, 2017 at 08:10:54PM +, Brian Steffens wrote:
> > This adds a boolean option called 'shared' to block devices. It defaults
> > to off/false. When enabled for a particular block device, the 'shared' 
> > option
> > causes the block migration code to skip over syncing of that device. This
> > allows controlling exactly which block devices get synced during a 
> > migration.
> > 
> > Signed-off-by: Brian Steffens 
> > ---
> >  block.c   | 7 +++
> >  block/qapi.c  | 2 ++
> >  include/block/block.h | 1 +
> >  include/block/block_int.h | 3 +++
> >  migration/block.c | 4 
> >  qapi/block-core.json  | 2 +-
> >  6 files changed, 18 insertions(+), 1 deletion(-)
> 
> Thanks for the patch!  Please email the relevant maintainers:
> 
>   $ scripts/get_maintainer.pl -f block.c
>   Kevin Wolf  (supporter:Block layer core)
>   Max Reitz  (supporter:Block layer core)
>   qemu-bl...@nongnu.org (open list:Block layer core)
>   qemu-devel@nongnu.org (open list:All patches CC here)
> 
> I have CCed them so they see this email.
> 
> > diff --git a/migration/block.c b/migration/block.c
> > index 9171f60028..b347c3dc61 100644
> > --- a/migration/block.c
> > +++ b/migration/block.c
> > @@ -402,6 +402,10 @@ static int init_blk_migration(QEMUFile *f)
> >  bmds_bs = g_malloc0(num_bs * sizeof(*bmds_bs));
> >  
> >  for (i = 0, bs = bdrv_first(); bs; bs = bdrv_next(), i++) {
> > +if (bs->shared) {

If we go with this approach, I'd prefer a more specific name like
bs->skip_block_migration; "shared" has a lot meanings so is less readable.

Fam

> > +continue;
> > +}
> > +

[Qemu-devel] [Bug 1407813] Re: QEMU wrongly translates newlines on serial output

[Tomasz Rostanski]

I think this bug relates to:
https://bugs.launchpad.net/qemu/+bug/1715296

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

Title:
  QEMU wrongly translates newlines on serial output

Status in QEMU:
  New

Bug description:
  When using "-serial stdio", QEMU shows the guest serial port's output
  on the tty running qemu. As it should, QEMU sets the tty to raw mode.
  Or almost... Strangely, it neglects to remove one output-translation
  bit, ONLCR (see termios(3)) enabled on the tty. And it should have
  removed this output translation!

  The problem is that with this ONLCR, the guest has no way of
  outputting a bare linefeed ('\n') - every time the guest tries to
  output a bare linefeed to the serial port, the host tty will translate
  it to \r\n which will be sent to the underlying terminal (e.g.,
  xterm).

  In most cases, this issue doesn't cause a problem: When the guest is
  running a Unix-like operating system which is itself in cooked mode,
  the guest itself will always output \r\n, and the hosts second
  translation (to \r\r\n) does no harm. But in certain cases, the guest
  can *really* want to output just \n, and have this \n reach the
  terminal emulator and do what a linefeed is supposed to do without a
  carriage-return - namely - just go one line down in the same column.

  As an illustration of this bug, consider a guest running a Unix-like
  operating system running a curses-based application (e.g., "vi"). If
  you look at the output of "infocmp xterm", you'll notice that cud1=^J.
  This means that if the curses library decides to move one line down
  (it can happen in some cursor movement situations) it might decide to
  print a linefeed (\n) to move one line down. The guest's operating
  system will not mess with this linefeed (because the guest is in raw
  mode), but then qemu's tty, because it was wrongly left in ONLCR mode,
  will change this \n to \r\n before it reaches the terminal - causing
  wrong cursor movement (instead the cursor going straight down, it
  moves to the first column of the next line).

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

[Qemu-devel] [Bug 1715296] Re: qemu: invalid serial port configuration

[Tomasz Rostanski]

I believe the following bug is related:
https://bugs.launchpad.net/qemu/+bug/1407813

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

Title:
  qemu: invalid serial port configuration

Status in QEMU:
  New

Bug description:
  The tty_serial_init() function sets the port c_oflags as follows:
  tty.c_oflag |= OPOST not clearing ONLCR, ONLRET and others.
  The result is that the postprocess output is enabled and host translates 0xa 
(LF) to 0xd 0xa (CR LF) which breaks the binary transmissions on serial port 
even if you set the port to raw mode (no matters if on host and/or guest).
  The issue has been reported 11 years ago on qemu-devel mailing list:
  https://lists.nongnu.org/archive/html/qemu-devel/2006-06/msg00196.html
  There was also a FreeBSD patch including the fix:
  https://lists.freebsd.org/pipermail/freebsd-ports/2006-October/036390.html

  I think the correct port configuration is:
  tty.c_iflag &= ~(IGNBRK|BRKINT|PARMRK|ISTRIP|INLCR|IGNCR|ICRNL|IXON|IMAXBEL);
  tty.c_oflag &= ~OPOST;

  In such case the host will perform no output processing and will pass the 
data as is.
  And the guest will be able to configure input/output processing exactly as it 
wants.

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

[Qemu-devel] [Bug 1715296] [NEW] qemu: invalid serial port configuration

[Tomasz Rostanski]

Public bug reported:

The tty_serial_init() function sets the port c_oflags as follows:
tty.c_oflag |= OPOST not clearing ONLCR, ONLRET and others.
The result is that the postprocess output is enabled and host translates 0xa 
(LF) to 0xd 0xa (CR LF) which breaks the binary transmissions on serial port 
even if you set the port to raw mode (no matters if on host and/or guest).
The issue has been reported 11 years ago on qemu-devel mailing list:
https://lists.nongnu.org/archive/html/qemu-devel/2006-06/msg00196.html
There was also a FreeBSD patch including the fix:
https://lists.freebsd.org/pipermail/freebsd-ports/2006-October/036390.html

I think the correct port configuration is:
tty.c_iflag &= ~(IGNBRK|BRKINT|PARMRK|ISTRIP|INLCR|IGNCR|ICRNL|IXON|IMAXBEL);
tty.c_oflag &= ~OPOST;

In such case the host will perform no output processing and will pass the data 
as is.
And the guest will be able to configure input/output processing exactly as it 
wants.

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

Title:
  qemu: invalid serial port configuration

Status in QEMU:
  New

Bug description:
  The tty_serial_init() function sets the port c_oflags as follows:
  tty.c_oflag |= OPOST not clearing ONLCR, ONLRET and others.
  The result is that the postprocess output is enabled and host translates 0xa 
(LF) to 0xd 0xa (CR LF) which breaks the binary transmissions on serial port 
even if you set the port to raw mode (no matters if on host and/or guest).
  The issue has been reported 11 years ago on qemu-devel mailing list:
  https://lists.nongnu.org/archive/html/qemu-devel/2006-06/msg00196.html
  There was also a FreeBSD patch including the fix:
  https://lists.freebsd.org/pipermail/freebsd-ports/2006-October/036390.html

  I think the correct port configuration is:
  tty.c_iflag &= ~(IGNBRK|BRKINT|PARMRK|ISTRIP|INLCR|IGNCR|ICRNL|IXON|IMAXBEL);
  tty.c_oflag &= ~OPOST;

  In such case the host will perform no output processing and will pass the 
data as is.
  And the guest will be able to configure input/output processing exactly as it 
wants.

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

Re: [Qemu-devel] [PULL v2 13/47] tests/check-qlit: New, covering qobject/qlit.c

[Markus Armbruster]

Eric Blake  writes:

> On 09/01/2017 10:37 AM, Markus Armbruster wrote:
>> From: Marc-André Lureau 
>> 
>> Signed-off-by: Marc-André Lureau 
>> Message-Id: <20170825105913.4060-11-marcandre.lur...@redhat.com>
>> [Copyright notice correction squashed in, commit message tweaked]
>> Signed-off-by: Markus Armbruster 
>> ---
>>  tests/Makefile.include |  5 +++-
>>  tests/check-qlit.c | 64 
>> ++
>>  2 files changed, 68 insertions(+), 1 deletion(-)
>>  create mode 100644 tests/check-qlit.c
>
> Adds a new file that is not covered by tests/.gitignore when doing an
> in-tree build

Easy enough to fix.

>   (and is one of our few check-* names that stands out in
> comparison to the bulk of the tests named test-* or *-test).

It's called check-lit.c because to match the related tests check-qlit.c
check-qdict.c check-qnull.c check-qstring.c check-qjson.c check-qnum.c
check-qlist.c.

> Of course, the point may be moot if we take the patch for
> auto-generating tests/.gitignore on an in-tree build...

Re: [Qemu-devel] [RFC PATCH] osdep.h: Prohibit disabling assert() in supported builds

[Thomas Huth]

On 05.09.2017 21:50, Eric Blake wrote:
> On 08/24/2017 02:51 AM, Markus Armbruster wrote:
>> Eric Blake  writes:
>>
>>> On 08/22/2017 06:19 AM, Halil Pasic wrote:
[...]
>>> I'd prefer that if we are going to introduce our own construct that
>>> always evaluates side effects, and only has a compile-time switch on
>>> whether to abort() or (foolishly) plow on, that we name it something
>>> without 'assert' in the name, so that reviewers don't have to be
>>> confused about remembering which variant evaluates side effects.  Maybe:
>>>
>>> q_verify(cond)
>>
>> I vote for frying bigger fish.
>>
>> I also vote for using standard C when standard C is servicable.
> 
> So if it were up to me alone, the answer is:
> 
> I'm NOT going to add any new construct (whether spelled q_verify() or
> otherwise), and will merely document in the commit message that we
> discussed this as an alternative (so someone who wants to disable #error
> can get a git history of what went into the decision).
> 
> Also, it sounds like we want to keep it #error, not #warn.
> 
> But if anyone else has strong opinions before we promote this from RFC
> to actual patch, I'm still interested in your arguments.

You asked for opinions, so here's mine: I agree with you, please do
*not* add a new QEMU-specific construct here. assert() should be a
well-known C construct that every programmer should have understood. You
also need it for other projects. If you haven't understood that it's a
macro and has side-effects, you should learn it (e.g. during patch
review), not avoid it, otherwise you'll run into problems in another
project that is using it again.

But IMHO we should still try to get rid of wrong usage of assert() in
the QEMU sources. So maybe we could allow building with NDEBUG one day
for the brave people who need the extra percent of additional speed. But
as long as we're not there, I think this patch is a good thing to avoid
wrong expectations.

 Thomas



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 1/1] net: Add SunGEM device emulation as found on Apple UniNorth

[Mark Cave-Ayland]

On 06/09/17 04:16, David Gibson wrote:
> On Tue, Sep 05, 2017 at 11:13:43AM +1000, David Gibson wrote:
>> On Mon, Sep 04, 2017 at 07:39:38PM +0100, Mark Cave-Ayland wrote:
>>> From: Benjamin Herrenschmidt 
>>>
>>> This adds a simplistic emulation of the Sun GEM ethernet controller
>>> found in Apple ASICs.
>>>
>>> Currently we only support the Apple UniNorth 1.x variant, but the
>>> other Apple or Sun variants should mostly be a matter of adding
>>> PCI IDs options.
>>>
>>> We have a very primitive emulation of a single Broadcom 5201 PHY
>>> which is supported by the MacOS driver.
>>>
>>> This model brings out-of-the-box networking to MacOS 9, and all
>>> versions of OS X I tried with the mac99 platform.
>>>
>>> Further improvements from Mark:
>>> - Remove sungem.h file, moving constants into sungem.c as required
>>> - Switch to using tracepoints for debugging
>>> - Split register blocks into separate memory regions
>>> - Use arrays in SunGEMState to hold register values
>>> - Add state-saving support
>>>
>>> Signed-off-by: Benjamin Herrenschmidt 
>>> Signed-off-by: Mark Cave-Ayland 
>>
>> Applied to ppc-for-2.11.
> 
> Until I discovered that it breaks compile with
> --enable-trace-backend=ust.   So I've removed it again.

Oh that's interesting. I've had --enable-trace-backend=simple as part of
my default build for a long while now, so I'm quite surprised that the
trace-events are backend sensitive.

Any clue as to what the error might be before I go and start installing
the relevant libraries?


ATB,

Mark.

Re: [Qemu-devel] [PATCH] build-sys: generate tests/.gitignore

[Thomas Huth]

On 05.09.2017 12:42, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Sep 4, 2017 at 12:21 PM Thomas Huth  > wrote:
> 
> On 04.09.2017 11 :03, Marc-André Lureau
> wrote:
[...]
> >  # Build the help program automatically
> >
> >  all: $(QEMU_IOTESTS_HELPERS-y)
> >
> > +$(SRC_PATH)/tests/.gitignore: $(MAKEFILE_LIST)
> > +     $(call quiet-command, echo "$(tests-cleanfiles)"
> "$(tests-cleandirs)" | \
> > +             xargs -n1 | sort | uniq | sed -e s:^:/: >
> $@,"GEN","$(@F)")
> 
> Please do not use SRC_PATH here. I'm doing out of tree builds, and I
> don't want that these are touching my source folder!
> 
> I understand the feeling, I do also mostly out of tree build. However, I
> don't think it makes much sense to generate .gitignore in the build dir.

Why not? If you're doing out-of-tree builds, you don't need the
.gitignore in the source directory anyway, so it certainly does not make
sense to generate there such a file in the source directory.

> It's git related, and in the git source dir, you have .git that is
> writable already

It's unlikely, but I think it is perfectly legal to have your git source
directory e.g. mounted temporarily as a read-only file system. I'd then
still expect to be able to use my read-only sources to build QEMU out of
tree.

 Thomas


PS: Looks like you're sending HTML mails ... please check the setup of
your mail program.

Re: [Qemu-devel] [PATCH v3 0/3] three zpci patches

[Yi Min Zhao]

Thank you very much!


在 2017/9/5 下午7:58, Cornelia Huck 写道:

On Tue,  5 Sep 2017 12:12:57 +0200
Yi Min Zhao  wrote:


This patch set contains three small zpci patches to fixup different issues.
1) remove zpci idx from msix message, instead we could use PCIDevice's id to
find zpci device in kvm_arch_fixup_msi_route()
2) fixup ind_offset calculation for adapter interrupt routing entry
3) introduce our own iommu_replay callback

Yi Min Zhao (3):
   s390x/pci: remove idx from msix msg data
   s390x/pci: fixup ind_offset of msix routing entry
   s390x/pci: add iommu replay callback

  hw/s390x/s390-pci-bus.c  | 28 +---
  hw/s390x/s390-pci-bus.h  |  2 ++
  hw/s390x/s390-pci-inst.c | 24 
  hw/s390x/s390-pci-stub.c |  3 ++-
  target/s390x/kvm.c   | 14 --
  5 files changed, 29 insertions(+), 42 deletions(-)


Thanks, applied.

Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH] tests: Add a device_add/del HMP test

[Thomas Huth]

On 05.09.2017 20:37, Dr. David Alan Gilbert wrote:
> * Thomas Huth (th...@redhat.com) wrote:
>> On 05.09.2017 18:48, Dr. David Alan Gilbert wrote:
>>> * Markus Armbruster (arm...@redhat.com) wrote:
 Thomas Huth  writes:

> People tend to forget to mark internal devices with "user_creatable = 
> false
> or hotpluggable = false, and these devices can crash QEMU if added via the
> HMP monitor. So let's add a test to run through all devices and that tries
> to add them blindly (without arguments) to see whether this could crash 
> the
> QEMU instance.
>
> Signed-off-by: Thomas Huth 
> ---
[...]
>>> If I'm reading the code right it's creating the device with the same
>>> name as the device;  I wonder if that always works?
>>
>> Why not? The id is just an arbitrary string, isn't it?
> 
> I didn't know how arbitrary they were allowed to be and I was
> also worried they might clash with some existing id.
> As an example, I see there's at least one device (SUNW,fdtwo) with
> a , in it's name - is that legal for an id ?

Oh, right, I didn't think of comma :-/ ... I'll try to come up with a
better solution in the next version of the patch...

 Thomas

Re: [Qemu-devel] [PATCH 4/9] s390x: refactor error handling for SSCH and RSCH

[Dong Jia Shi]

* Halil Pasic  [2017-09-06 00:30:58 +0200]:

[...]

> > 
> >> But I guess, I was afraid of somebody blaming me for this
> >> comment (such TODOs in production code are a bit strange -- what
> >> does it mean: we did not bother to figure it out?).
> > 
> > For one, the TODO is preexisting... and we really should remember to
> > figure out if there's something better rather than just drop the
> > comment.
> > 
> > (And I sure hope nobody is using vfio-ccw in production yet...)
> >
> Since blame says the TODO has been around since 2017-05-17
> let me have a critical look at it.
> 
> At a first glance I would say addressing exception for SSCH
> is not what we want: the only possibility I see for address
> exception for SSCH is due to the ORB address. But if that's
> the case we will never reach the code in question.
Agree.

> So I suppose we can do better.
As the comment said, I'm (still) in the state of 'wondering'.

> 
> Adding Ren. @Ren: Do you agree with my analysis. If you do,
> I could come up with a proposal what to do -- after some reading.
If you have a better idea, and time, why not? ;)

> 
> Regards,
> Halil

-- 
Dong Jia Shi

Re: [Qemu-devel] [PATCH 1/1] net: Add SunGEM device emulation as found on Apple UniNorth

[David Gibson]

On Tue, Sep 05, 2017 at 11:13:43AM +1000, David Gibson wrote:
> On Mon, Sep 04, 2017 at 07:39:38PM +0100, Mark Cave-Ayland wrote:
> > From: Benjamin Herrenschmidt 
> > 
> > This adds a simplistic emulation of the Sun GEM ethernet controller
> > found in Apple ASICs.
> > 
> > Currently we only support the Apple UniNorth 1.x variant, but the
> > other Apple or Sun variants should mostly be a matter of adding
> > PCI IDs options.
> > 
> > We have a very primitive emulation of a single Broadcom 5201 PHY
> > which is supported by the MacOS driver.
> > 
> > This model brings out-of-the-box networking to MacOS 9, and all
> > versions of OS X I tried with the mac99 platform.
> > 
> > Further improvements from Mark:
> > - Remove sungem.h file, moving constants into sungem.c as required
> > - Switch to using tracepoints for debugging
> > - Split register blocks into separate memory regions
> > - Use arrays in SunGEMState to hold register values
> > - Add state-saving support
> > 
> > Signed-off-by: Benjamin Herrenschmidt 
> > Signed-off-by: Mark Cave-Ayland 
> 
> Applied to ppc-for-2.11.

Until I discovered that it breaks compile with
--enable-trace-backend=ust.   So I've removed it again.

> 
> > ---
> >  default-configs/ppc-softmmu.mak |1 +
> >  hw/net/Makefile.objs|1 +
> >  hw/net/sungem.c | 1447 
> > +++
> >  hw/net/trace-events |   44 ++
> >  hw/pci/pci.c|2 +
> >  include/hw/pci/pci_ids.h|1 +
> >  6 files changed, 1496 insertions(+)
> >  create mode 100644 hw/net/sungem.c
> > 
> > diff --git a/default-configs/ppc-softmmu.mak 
> > b/default-configs/ppc-softmmu.mak
> > index 1f1cd85..c12ba9e 100644
> > --- a/default-configs/ppc-softmmu.mak
> > +++ b/default-configs/ppc-softmmu.mak
> > @@ -17,6 +17,7 @@ CONFIG_PREP_PCI=y
> >  CONFIG_I82378=y
> >  CONFIG_PC87312=y
> >  CONFIG_MACIO=y
> > +CONFIG_SUNGEM=y
> >  CONFIG_PCSPK=y
> >  CONFIG_CS4231A=y
> >  CONFIG_CUDA=y
> > diff --git a/hw/net/Makefile.objs b/hw/net/Makefile.objs
> > index 5ddaffe..7e87d01 100644
> > --- a/hw/net/Makefile.objs
> > +++ b/hw/net/Makefile.objs
> > @@ -27,6 +27,7 @@ common-obj-$(CONFIG_CADENCE) += cadence_gem.o
> >  common-obj-$(CONFIG_STELLARIS_ENET) += stellaris_enet.o
> >  common-obj-$(CONFIG_LANCE) += lance.o
> >  common-obj-$(CONFIG_FTGMAC100) += ftgmac100.o
> > +common-obj-$(CONFIG_SUNGEM) += sungem.o
> >  
> >  obj-$(CONFIG_ETRAXFS) += etraxfs_eth.o
> >  obj-$(CONFIG_COLDFIRE) += mcf_fec.o
> > diff --git a/hw/net/sungem.c b/hw/net/sungem.c
> > new file mode 100644
> > index 000..8c2ca4a
> > --- /dev/null
> > +++ b/hw/net/sungem.c
> > @@ -0,0 +1,1447 @@
> > +/*
> > + * QEMU model of SUN GEM ethernet controller
> > + *
> > + * As found in Apple ASICs among others
> > + *
> > + * Copyright 2016 Ben Herrenschmidt
> > + * Copyright 2017 Mark Cave-Ayland
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/pci/pci.h"
> > +#include "qemu/log.h"
> > +#include "net/net.h"
> > +#include "net/checksum.h"
> > +#include "hw/net/mii.h"
> > +#include "sysemu/sysemu.h"
> > +#include "trace.h"
> > +/* For crc32 */
> > +#include 
> > +
> > +#define TYPE_SUNGEM "sungem"
> > +
> > +#define SUNGEM(obj) OBJECT_CHECK(SunGEMState, (obj), TYPE_SUNGEM)
> > +
> > +#define MAX_PACKET_SIZE 9016
> > +
> > +#define SUNGEM_MMIO_SIZE0x20
> > +
> > +/* Global registers */
> > +#define SUNGEM_MMIO_GREG_SIZE   0x2000
> > +
> > +#define GREG_SEBSTATE 0xUL/* SEB State Register */
> > +
> > +#define GREG_STAT 0x000CUL/* Status Register */
> > +#define GREG_STAT_TXINTME 0x0001/* TX INTME frame transferred 
> > */
> > +#define GREG_STAT_TXALL   0x0002/* All TX frames transferred */
> > +#define GREG_STAT_TXDONE  0x0004/* One TX frame transferred */
> > +#define GREG_STAT_RXDONE  0x0010/* One RX frame arrived */
> > +#define GREG_STAT_RXNOBUF 0x0020/* No free RX buffers 
> > available */
> > +#define GREG_STAT_RXTAGERR0x0040/* RX tag framing is corrupt */
> > +#define GREG_STAT_TXMAC   0x4000/* TX MAC signalled interrupt 
> > */
> > +#define GREG_STAT_RXMAC   0x8000/* RX MAC signalled interrupt 
> > */
> > +#define GREG_STAT_MAC 0x0001/* MAC Control signalled irq */
> > +#define GREG_STAT_TXNR0xfff8/* == TXDMA_TXDONE reg val */
> > +#define GREG_STAT_TXNR_SHIFT  19
> > +
> > +/* These interrupts are edge latches in the status register,
> > + * reading it (or writing the corresponding bit in IACK) will
> > + * clear them
> > + */
> > +#define GREG_STAT_LATCH   (GREG_STAT_TXALL  | GREG_STAT_TXINTME | \
> > +   GREG_STAT_RXDONE | GREG_STAT_RXDONE |  \
> > +   GREG_STAT_RXNOBUF | GREG_STAT_RXTAGERR)
> > +
> > +#define GREG_IMASK0x0010UL

[Qemu-devel] [PATCH v2 3/3] buildsys: Move gcrypt cflags/libs to per object

[Fam Zheng]

Signed-off-by: Fam Zheng 
---
 configure| 5 ++---
 crypto/Makefile.objs | 7 +++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index d24a7eab37..42622b98be 100755
--- a/configure
+++ b/configure
@@ -2566,9 +2566,6 @@ if test "$gcrypt" != "no"; then
 then
 gcrypt_libs="$gcrypt_libs -lgpg-error"
 fi
-libs_softmmu="$gcrypt_libs $libs_softmmu"
-libs_tools="$gcrypt_libs $libs_tools"
-QEMU_CFLAGS="$QEMU_CFLAGS $gcrypt_cflags"
 gcrypt="yes"
 if test -z "$nettle"; then
nettle="no"
@@ -5716,6 +5713,8 @@ if test "$gnutls_rnd" = "yes" ; then
 fi
 if test "$gcrypt" = "yes" ; then
   echo "CONFIG_GCRYPT=y" >> $config_host_mak
+  echo "GCRYPT_CFLAGS=$gcrypt_cflags" >> $config_host_mak
+  echo "GCRYPT_LIBS=$gcrypt_libs" >> $config_host_mak
   if test "$gcrypt_hmac" = "yes" ; then
 echo "CONFIG_GCRYPT_HMAC=y" >> $config_host_mak
   fi
diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 0cb5fa24b6..45f7c0c80d 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -34,6 +34,13 @@ crypto-obj-y += block.o
 crypto-obj-y += block-qcow.o
 crypto-obj-y += block-luks.o
 
+$(if $(CONFIG_GCRYPT), \
+   $(foreach x, init cipher hash-gcrypt hmac-gcrypt random-gcrypt \
+   pbkdf-gcrypt, \
+   $(eval $x.o-cflags := $(NETTLE_CFLAGS)) \
+   $(eval $x.o-libs := $(NETTLE_LIBS))) \
+)
+
 $(if $(CONFIG_NETTLE), \
$(foreach x, cipher hash-nettle hmac-nettle pbkdf-nettle, \
$(eval $x.o-cflags := $(NETTLE_CFLAGS)) \
-- 
2.13.5

[Qemu-devel] [PATCH v2 0/3] buildsys: Move crypto libraries to per object variables

[Fam Zheng]

v2: Add gcrypt as well. [Daniel]

Not all targets need the flags and libs of gcrypt/nettle/gnutls, so move them
out from global variables, as done with other libraries.

There are still other libraries in ./configure that can be converted this way,
which will be sent separatedly.

Fam Zheng (3):
  buildsys: Move nettle cflags/libs to per object
  buildsys: Move gnutls cflags/libs to per object
  buildsys: Move gcrypt cflags/libs to per object

 configure  | 14 ++
 crypto/Makefile.objs   | 20 
 tests/Makefile.include | 10 +-
 3 files changed, 31 insertions(+), 13 deletions(-)

-- 
2.13.5

[Qemu-devel] [PATCH v2 1/3] buildsys: Move nettle cflags/libs to per object

[Fam Zheng]

Signed-off-by: Fam Zheng 
---
 configure| 5 ++---
 crypto/Makefile.objs | 6 ++
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index fb7e34a901..6a4cb3832b 100755
--- a/configure
+++ b/configure
@@ -2616,9 +2616,6 @@ if test "$nettle" != "no"; then
 nettle_cflags=$($pkg_config --cflags nettle)
 nettle_libs=$($pkg_config --libs nettle)
 nettle_version=$($pkg_config --modversion nettle)
-libs_softmmu="$nettle_libs $libs_softmmu"
-libs_tools="$nettle_libs $libs_tools"
-QEMU_CFLAGS="$QEMU_CFLAGS $nettle_cflags"
 nettle="yes"
 
 cat > $TMPC << EOF
@@ -5732,6 +5729,8 @@ if test "$nettle" = "yes" ; then
   if test "$nettle_kdf" = "yes" ; then
 echo "CONFIG_NETTLE_KDF=y" >> $config_host_mak
   fi
+  echo "NETTLE_CFLAGS=$nettle_cflags" >> $config_host_mak
+  echo "NETTLE_LIBS=$nettle_libs" >> $config_host_mak
 fi
 if test "$tasn1" = "yes" ; then
   echo "CONFIG_TASN1=y" >> $config_host_mak
diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 2b99e08062..a936957d03 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -34,6 +34,12 @@ crypto-obj-y += block.o
 crypto-obj-y += block-qcow.o
 crypto-obj-y += block-luks.o
 
+$(if $(CONFIG_NETTLE), \
+   $(foreach x, cipher hash-nettle hmac-nettle pbkdf-nettle, \
+   $(eval $x.o-cflags := $(NETTLE_CFLAGS)) \
+   $(eval $x.o-libs := $(NETTLE_LIBS))) \
+)
+
 # Let the userspace emulators avoid linking gnutls/etc
 crypto-aes-obj-y = aes.o
 
-- 
2.13.5

[Qemu-devel] [PATCH v2 2/3] buildsys: Move gnutls cflags/libs to per object

[Fam Zheng]

Signed-off-by: Fam Zheng 
---
 configure  |  4 ++--
 crypto/Makefile.objs   |  7 +++
 tests/Makefile.include | 10 +-
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/configure b/configure
index 6a4cb3832b..d24a7eab37 100755
--- a/configure
+++ b/configure
@@ -2472,8 +2472,6 @@ if test "$gnutls" != "no"; then
 if gnutls_works; then
 gnutls_cflags=$($pkg_config --cflags gnutls)
 gnutls_libs=$($pkg_config --libs gnutls)
-libs_softmmu="$gnutls_libs $libs_softmmu"
-libs_tools="$gnutls_libs $libs_tools"
QEMU_CFLAGS="$QEMU_CFLAGS $gnutls_cflags"
 gnutls="yes"
 
@@ -5710,6 +5708,8 @@ fi
 echo "CONFIG_TLS_PRIORITY=\"$tls_priority\"" >> $config_host_mak
 if test "$gnutls" = "yes" ; then
   echo "CONFIG_GNUTLS=y" >> $config_host_mak
+  echo "GNUTLS_CFLAGS=$gnutls_cflags" >> $config_host_mak
+  echo "GNUTLS_LIBS=$gnutls_libs" >> $config_host_mak
 fi
 if test "$gnutls_rnd" = "yes" ; then
   echo "CONFIG_GNUTLS_RND=y" >> $config_host_mak
diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index a936957d03..0cb5fa24b6 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -40,6 +40,13 @@ $(if $(CONFIG_NETTLE), \
$(eval $x.o-libs := $(NETTLE_LIBS))) \
 )
 
+$(if $(CONFIG_GNUTLS), \
+   $(foreach x, init random-gnutls tlscreds tlscredsanon \
+   tlscredsx509 tlssession, \
+   $(eval $x.o-cflags := $(GNUTLS_CFLAGS)) \
+   $(eval $x.o-libs := $(GNUTLS_LIBS))) \
+)
+
 # Let the userspace emulators avoid linking gnutls/etc
 crypto-aes-obj-y = aes.o
 
diff --git a/tests/Makefile.include b/tests/Makefile.include
index fae5715e9c..d46c22d1ec 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -677,15 +677,15 @@ tests/benchmark-crypto-cipher$(EXESUF): 
tests/benchmark-crypto-cipher.o $(test-c
 tests/test-crypto-secret$(EXESUF): tests/test-crypto-secret.o 
$(test-crypto-obj-y)
 tests/test-crypto-xts$(EXESUF): tests/test-crypto-xts.o $(test-crypto-obj-y)
 
-tests/crypto-tls-x509-helpers.o-cflags := $(TASN1_CFLAGS)
-tests/crypto-tls-x509-helpers.o-libs := $(TASN1_LIBS)
-tests/pkix_asn1_tab.o-cflags := $(TASN1_CFLAGS)
+tests/crypto-tls-x509-helpers.o-cflags := $(TASN1_CFLAGS) $(GNUTLS_CFLAGS)
+tests/crypto-tls-x509-helpers.o-libs := $(TASN1_LIBS) $(GNUTLS_LIBS)
+tests/pkix_asn1_tab.o-cflags := $(TASN1_CFLAGS) $(GNUTLS_LIBS)
 
-tests/test-crypto-tlscredsx509.o-cflags := $(TASN1_CFLAGS)
+tests/test-crypto-tlscredsx509.o-cflags := $(TASN1_CFLAGS) $(GNUTLS_CFLAGS)
 tests/test-crypto-tlscredsx509$(EXESUF): tests/test-crypto-tlscredsx509.o \
tests/crypto-tls-x509-helpers.o tests/pkix_asn1_tab.o 
$(test-crypto-obj-y)
 
-tests/test-crypto-tlssession.o-cflags := $(TASN1_CFLAGS)
+tests/test-crypto-tlssession.o-cflags := $(TASN1_CFLAGS) $(GNUTLS_CFLAGS)
 tests/test-crypto-tlssession$(EXESUF): tests/test-crypto-tlssession.o \
tests/crypto-tls-x509-helpers.o tests/pkix_asn1_tab.o 
$(test-crypto-obj-y)
 tests/test-io-task$(EXESUF): tests/test-io-task.o $(test-io-obj-y)
-- 
2.13.5

Re: [Qemu-devel] [PATCH v3 08/14] hvf: add compilation rules to Makefile.objs

[Sergio Andrés Gómez del Real]

Stefan, is this acceptable? I mean delaying the compiling; up until patch 7
no hvf code is compiled. Or should I just squash everything from patch 2 to
7?

El 4/09/2017 23:03, "Sergio Andrés Gómez del Real" <
sergio.g.delr...@gmail.com> escribió:

Note that until this patch (8/14) there is no compiling. I tried to put
this patch earlier in the patchset but found difficulties because some
things need to be done before the hvf code compiles well.
Also, the commit message needs to be updated (remove the -enable-hvf part).

On Mon, Sep 4, 2017 at 10:54 PM, Sergio Andres Gomez Del Real <
sergio.g.delr...@gmail.com> wrote:

> This commit adds to target/i386/Makefile.objs the necessary rules so
> that the new files for hvf are compiled by the build system.
> It also adds handling of the -enable-hvf argument in the main function
> in vl.c.
>
> Signed-off-by: Sergio Andres Gomez Del Real 
> ---
>  target/i386/Makefile.objs | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target/i386/Makefile.objs b/target/i386/Makefile.objs
> index 6a26e9d9f0..0bef89c099 100644
> --- a/target/i386/Makefile.objs
> +++ b/target/i386/Makefile.objs
> @@ -12,4 +12,5 @@ obj-$(CONFIG_HAX) += hax-all.o hax-mem.o hax-windows.o
>  endif
>  ifdef CONFIG_DARWIN
>  obj-$(CONFIG_HAX) += hax-all.o hax-mem.o hax-darwin.o
> +obj-$(CONFIG_HVF) += hvf-utils/ hvf-all.o
>  endif
> --
> 2.14.1
>
>

Re: [Qemu-devel] [PATCHv5 02/03] colo-compare: Use IOThread to Check old packet regularly and Process pactkets of the primary

[wang.yong155]

>Remove the task which check old packet in the comparing thread,

>then use IOthread context timer to handle it.

>

>Process pactkets in the IOThread which arrived over the socket.

>we use iothread_get_g_main_context to create a new g_main_loop in

>the IOThread.then the packets from the primary and the secondary

>are processed in the IOThread.

>

>Finally remove the colo-compare thread using the IOThread instead.

>

>Signed-off-by: Wang Yong 

>Signed-off-by: Wang Guang 

>---

> net/colo-compare.c | 83 +-

> 1 file changed, 45 insertions(+), 38 deletions(-)




Hi Jason,

How about this series?




Thanks






原始邮件



发件人：王勇10170530
收件人：    
  

抄送人：王勇10170530王广10165992  
日 期 ：2017年08月29日 15:22
主 题 ：[PATCHv5 02/03] colo-compare: Use IOThread to Check old packet regularly 
and Process pactkets of the primary





From: Wang Yong 

Remove the task which check old packet in the comparing thread,
then use IOthread context timer to handle it.

Process pactkets in the IOThread which arrived over the socket.
we use iothread_get_g_main_context to create a new g_main_loop in
the IOThread.then the packets from the primary and the secondary
are processed in the IOThread.

Finally remove the colo-compare thread using the IOThread instead.

Signed-off-by: Wang Yong 
Signed-off-by: Wang Guang 
---
 net/colo-compare.c | 83 +-
 1 file changed, 45 insertions(+), 38 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 5fe8e3f..b2a2a13 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -29,6 +29,7 @@
 #include "qemu/sockets.h"
 #include "qapi-visit.h"
 #include "net/colo.h"
+#include "sysemu/iothread.h"
 
 #define TYPE_COLO_COMPARE "colo-compare"
 #define COLO_COMPARE(obj) \
@@ -82,11 +83,10 @@ typedef struct CompareState {
 GQueue conn_list
 /* hashtable to save connection */
 GHashTable *connection_track_table
-/* compare thread, a thread for each NIC */
-QemuThread thread
 
+IOThread *iothread
 GMainContext *worker_context
-GMainLoop *compare_loop
+QEMUTimer *packet_check_timer
 } CompareState
 
 typedef struct CompareClass {
@@ -597,22 +597,40 @@ static void compare_sec_chr_in(void *opaque, const 
uint8_t *buf, int size)
  * Check old packet regularly so it can watch for any packets
  * that the secondary hasn't produced equivalents of.
  */
-static gboolean check_old_packet_regular(void *opaque)
+static void check_old_packet_regular(void *opaque)
 {
 CompareState *s = opaque
 
 /* if have old packet we will notify checkpoint */
 colo_old_packet_check(s)
+timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
+REGULAR_PACKET_CHECK_MS)
+}
+
+static void colo_compare_timer_init(CompareState *s)
+{
+AioContext *ctx = iothread_get_aio_context(s->iothread)
 
-return TRUE
+s->packet_check_timer = aio_timer_new(ctx, QEMU_CLOCK_VIRTUAL,
+SCALE_MS, check_old_packet_regular,
+s)
+timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
+REGULAR_PACKET_CHECK_MS)
 }
 
-static void *colo_compare_thread(void *opaque)
+static void colo_compare_timer_del(CompareState *s)
 {
-CompareState *s = opaque
-GSource *timeout_source
+if (s->packet_check_timer) {
+timer_del(s->packet_check_timer)
+timer_free(s->packet_check_timer)
+s->packet_check_timer = NULL
+}
+ }
 
-s->worker_context = g_main_context_new()
+static void colo_compare_iothread(CompareState *s)
+{
+object_ref(OBJECT(s->iothread))
+s->worker_context = iothread_get_g_main_context(s->iothread)
 
 qemu_chr_fe_set_handlers(>chr_pri_in, compare_chr_can_read,
  compare_pri_chr_in, NULL, NULL,
@@ -621,20 +639,7 @@ static void *colo_compare_thread(void *opaque)
  compare_sec_chr_in, NULL, NULL,
  s, s->worker_context, true)
 
-s->compare_loop = g_main_loop_new(s->worker_context, FALSE)
-
-/* To kick any packets that the secondary doesn't match */
-timeout_source = g_timeout_source_new(REGULAR_PACKET_CHECK_MS)
-g_source_set_callback(timeout_source,
-  (GSourceFunc)check_old_packet_regular, s, NULL)
-g_source_attach(timeout_source, s->worker_context)
-
-g_main_loop_run(s->compare_loop)
-
-g_source_unref(timeout_source)
-g_main_loop_unref(s->compare_loop)
-g_main_context_unref(s->worker_context)
-return NULL
+colo_compare_timer_init(s)
 }
 

Re: [Qemu-devel] [PATCH v7 0/4] Generic PCIE-PCI Bridge

[Aleksandr Bezzubikov]

ср, 23 авг. 2017 г. в 5:46, Michael S. Tsirkin :

> On Tue, Aug 22, 2017 at 02:43:39PM +0300, Marcel Apfelbaum wrote:
> > On 18/08/2017 2:36, Aleksandr Bezzubikov wrote:
> > > This series introduces a new device - Generic PCI Express to PCI
> bridge,
> > > and also makes all necessary changes to enable hotplug of the bridge
> itself
> > > and any device into the bridge.
> > >
> >
> > Hi,
> >
> > Series
> >Tested-by: Marcel Apfelbaum 
> > (focused on changes from v6)
> >
> >
> > Michael, will Alecsandr need to re-send it after freeze?
>
> re-send or ping pls.


Michael, it seems that freeze is over, so you can pick this patches.


>
> > I am asking because the GSOC project is ending in a week or so.
> >
> >
> > Thanks,
> > Marcel
> >
> > > Changes v6->v7:
> > > Change IO/MEM/PREF reservation properties type to SIZE.
> > >
> > > Changes v5->v6:
> > > 1. Fix indentation in the cap creation function (addresses Marcel's
> comment)
> > > 2. Simplify capability pref_mem_* fields assignment (addresses
> Marcel's comment)
> > > 3. Documentation fixes:
> > > - fix mutually exclusive fields definition (addresses Laszlo's
> comment)
> > > - fix pcie-pci-bridge usage example (addresses Marcel's comment)
> > >
> > > Changes v4->v5:
> > > 1. Change PCIE-PCI Bridge license (addresses Marcel's comment)
> > > 2. The capability layout changes (adress Laszlo' comments):
> > >  - separate pref_mem into pref_mem_32 and pref_mem_64 fields
> (SeaBIOS side has the same changes)
> > >  - accordingly change the Generic Root Port's properties
> > > 3. Do not add the capability to the root port if no valid values are
> provided (adresses Michael's comment)
> > > 4. Rename the capability type to 'RESOURCE_RESERVE' (addresses
> Marcel's comment)
> > > 5. Remove shpc_present check function (addresses Marcel's comment)
> > > 6. Fix the 4th patch message (adresses Michael's comment)
> > > 7. Patch for SHPC enabling in _OSC method has been already merged
> > >
> > > Changes v3->v4:
> > > 1. PCIE-PCI Bridge device: "msi_enable"->"msi", "shpc"->"shpc_bar",
> remove local_err,
> > > make "msi" property OnOffAuto, shpc_present() is still here
> > > to avoid SHPC_VMSTATE refactoring (address Marcel's comments).
> > > 2. Change QEMU PCI capability layout (SeaBIOS side has the same
> changes):
> > >- change reservation fields types: bus_res - uint32_t, others -
> uint64_t
> > >- rename 'non_pref' and 'pref' fields
> > >- interpret -1 value as 'ignore'
> > > 3. Use parent_realize in Generic PCI Express Root Port properly.
> > > 4. Fix documentation: fully replace the DMI-PCI bridge references with
> the new PCIE-PCI bridge,
> > > "PCIE"->"PCI Express", small mistakes and typos - address Laszlo's and
> Marcel's comments.
> > > 5. Rename QEMU PCI cap creation fucntion - addresses Marcel's comment.
> > >
> > > Changes v2->v3:
> > > (0). 'do_not_use' capability field flag is still _not_ in here since
> we haven't come to consesus on it yet.
> > > 1. Merge commits 5 (bus_reserve property creation) and 6 (property
> usage) together - addresses Michael's comment.
> > > 2. Add 'bus_reserve' property and QEMU PCI capability only to Generic
> PCIE Root Port - addresses Michael's and Marcel's comments.
> > > 3. Change 'bus_reserve' property's default value to 0 - addresses
> Michael's comment.
> > > 4. Rename QEMU bridge-specific PCI capability creation function -
> addresses Michael's comment.
> > > 5. Init the whole QEMU PCI capability with zeroes - addresses
> Michael's and Laszlo's comments.
> > > 6. Change QEMU PCI capability layout (SeaBIOS side has the same
> changes)
> > >- add 'type' field to distinguish multiple
> > >  RedHat-specific capabilities - addresses Michael's comment
> > >- do not mimiс PCI Config space register layout, but use mutually
> exclusive differently
> > >  sized fields for IO and prefetchable memory limits - addresses
> Laszlo's comment
> > > 7. Correct error handling in PCIE-PCI bridge realize function.
> > > 8. Replace a '2' constant with PCI_CAP_FLAGS in the capability
> creation function - addresses Michael's comment.
> > > 9. Remove a comment on _OSC which isn't correct anymore - address
> Marcel's comment.
> > > 10. Add documentation for the Generic PCIE-PCI Bridge and QEMU PCI
> capability - addresses Michael's comment.
> > >
> > > Changes v1->v2:
> > > 1. Enable SHPC for the bridge.
> > > 2. Enable SHPC support for the Q35 machine (ACPI stuff).
> > > 3. Introduce PCI capability to help firmware on the system init.
> > > This allows the bridge to be hotpluggable. Now it's supported
> > > only for pcie-root-port. Now it's supposed to used with
> > > SeaBIOS only, look at the SeaBIOS corresponding series
> > > "Allow RedHat PCI bridges reserve more buses than necessary during
> init".
> > >
> > > Aleksandr Bezzubikov (4):
> > >hw/pci: introduce pcie-pci-bridge device
> > >hw/pci: introduce bridge-only 

Re: [Qemu-devel] [PATCH 1/5] target/arm: Remove stale comment

[Pranith Kumar]

Hi Alex,

On Tue, Sep 5, 2017 at 8:02 AM, Alex Bennée  wrote:
>
> Pranith Kumar  writes:
>
>> Update the comment which is not true since MTTCG.
>
> What happened to the cover letter? We seem to have a mix of patches but
> no summary of the overall outcome.
>

These are a bunch of unrelated patches, so there is no theme. I will
include a cover letter saying so from now on.

Thanks,
-- 
Pranith

Re: [Qemu-devel] [PATCH] tcg/softmmu: Increase size of TLB caches

[Pranith Kumar]

On Tue, Sep 5, 2017 at 5:50 PM, Richard Henderson  wrote:
> On 08/29/2017 10:23 AM, Pranith Kumar wrote:
>> This patch increases the number of entries cached in the TLB. I went
>> over a few architectures to see if increasing it is problematic. Only
>> armv6 seems to have a limitation that only 8 bits can be used for
>> indexing these entries. For other architectures, the number of TLB
>> entries is increased to a 4K-sized cache. The patch also doubles the
>> number of victim TLB entries.
>>
>> Some statistics collected from a build benchmark for various cache
>> sizes is listed below:
>>
>> | TLB bits\vTLB entires | 8 |16  |32 |
>> | 8 | 952.94(+0.0%) | 929.99(+2.4%)  | 919.02(+3.6%) |
>> |10 | 898.92(+5.6%) | 886.13(+7.0%)  | 887.03(+6.9%) |
>> |12 | 878.56(+7.8%) | 873.53(+8.3%)* | 875.34(+8.1%) |
>>
>> The best combination for this workload came out to be 12 bits for the
>> TLB and a 16 entry vTLB cache.
>
> This significantly degrades performance of alpha-softmmu.
> It spends about 25% of all cpu time in memset.

What workload does it degrade for? I will try to reproduce and see
which memset is causing this.

-- 
Pranith

Re: [Qemu-devel] [PATCH 6/6] arm: drop intermadiate cpu_model -> cpu type parsing and use cpu type directly

[Alistair Francis]

On Tue, Sep 5, 2017 at 3:46 PM, Alistair Francis  wrote:
> On Tue, Sep 5, 2017 at 3:12 PM, Eduardo Habkost  wrote:
>> On Tue, Sep 05, 2017 at 02:47:52PM -0700, Alistair Francis wrote:
>>> On Tue, Sep 5, 2017 at 2:31 PM, Eduardo Habkost  wrote:
>> [...]
>>> >> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
>>> >> index f61e735..1cd6374 100644
>>> >> --- a/hw/arm/stm32f205_soc.c
>>> >> +++ b/hw/arm/stm32f205_soc.c
>>> >> @@ -112,7 +112,7 @@ static void stm32f205_soc_realize(DeviceState 
>>> >> *dev_soc, Error **errp)
>>> >>
>>> >>  armv7m = DEVICE(>armv7m);
>>> >>  qdev_prop_set_uint32(armv7m, "num-irq", 96);
>>> >> -qdev_prop_set_string(armv7m, "cpu-model", s->cpu_model);
>>> >> +qdev_prop_set_string(armv7m, "cpu-type", s->cpu_type);
>>> >>  object_property_set_link(OBJECT(>armv7m), 
>>> >> OBJECT(get_system_memory()),
>>> >>   "memory", _abort);
>>> >>  object_property_set_bool(OBJECT(>armv7m), true, "realized", 
>>> >> );
>>> >> @@ -200,7 +200,7 @@ static void stm32f205_soc_realize(DeviceState 
>>> >> *dev_soc, Error **errp)
>>> >>  }
>>> >>
>>> >>  static Property stm32f205_soc_properties[] = {
>>> >> -DEFINE_PROP_STRING("cpu-model", STM32F205State, cpu_model),
>>> >> +DEFINE_PROP_STRING("cpu-type", STM32F205State, cpu_type),
>>> >
>>> > Same as armv7m: are we 100% sure users are not setting this
>>> > manually?
>>>
>>> In an embedded board like this it really doesn't make sense to let the
>>> user overwrite the CPU. The SoC will take it as an option, but the
>>> board (which creates the SoC) just blindly always uses the same CPU.
>>> That feature is more for QOMificatoion then any real reason though.
>>>
>>
>> I'm not talking about -cpu (no user-visible change in the
>> handling of -cpu should result from this patch), but about
>> possible cases where the user set the "cpu-model" property using
>> another mechanism, like -global.  Probably it's impossible for an
>> user to override the property successfully, but I would like to
>> be sure.
>
> Ah, that is trickier.
>
> I guess that is possible to do, but the object setting logic should
> handle the error gracefully and inform the user of the error.
>
>>
>>
>>> In saying that I think a warning if the user tries to set the CPU
>>> would make sense. I know that this issues comes up in other ARM boards
>>> (Zynq-7000 has the same issue as well) so maybe a machine property
>>> saying that the board doesn't accept custom CPUs would be a good idea.
>>
>> Yeah, there are multiple cases in this patch where boards are
>> validating the CPU model, but not all boards do that.  A generic
>> MachineClass::valid_cpu_types[] field would be useful.

I just sent a RFC out that does this, let me know what you think.

The cover letter is called: "Add a valid_cpu_types property"

Thanks,
Alistair

>>
>>>
>>> Overall I think this patch is moving in the right direction though and
>>> this CPU option being ignored existed before this series.
>>
>> I agree this is going on the right direction.  However, I don't
>> see any board that ignore the CPU option: all of them seem to use
>> cpu_model when creating the CPUs, already.
>
> The Netduino2 will ignore any CPU options and always use a Cortex-m3.
> I was wrong about Zynq-7000 though, it does respect the -cpu option.
>
> Thanks,
> Alistair
>
>>
>> --
>> Eduardo

[Qemu-devel] [RFC v1 1/2] machine: Add a valid_cpu_types property

[Alistair Francis]

Signed-off-by: Alistair Francis 
---

 hw/core/machine.c   | 27 +++
 include/hw/boards.h |  1 +
 2 files changed, 28 insertions(+)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 41b53a17ad..de0f127d27 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -758,6 +758,33 @@ void machine_run_board_init(MachineState *machine)
 machine_numa_finish_init(machine);
 }
 machine_class->init(machine);
+
+if (machine_class->valid_cpu_types && machine->cpu_model) {
+const char *temp;
+int i, len = machine_class->valid_cpu_types->len;
+
+for (i = 0; i < len; i++) {
+temp = g_array_index(machine_class->valid_cpu_types, char *, i);
+if (!strcmp(machine->cpu_model, temp)) {
+/* The user specificed CPU is in the valid field, we are
+ * good to go.
+ */
+g_array_free(machine_class->valid_cpu_types, true);
+return;
+}
+}
+/* The user specified CPU must not be a valid CPU, print a sane error 
*/
+temp = g_array_index(machine_class->valid_cpu_types, char *, 0);
+error_report("Invalid CPU: %s", machine->cpu_model);
+error_printf("The valid options are: %s", temp);
+for (i = 1; i < len; i++) {
+temp = g_array_index(machine_class->valid_cpu_types, char *, i);
+error_printf(", %s", temp);
+}
+error_printf("\n");
+g_array_free(machine_class->valid_cpu_types, true);
+exit(1);
+}
 }
 
 static void machine_class_finalize(ObjectClass *klass, void *data)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 3363dd19fd..78678f84a9 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -172,6 +172,7 @@ struct MachineClass {
 int minimum_page_bits;
 bool has_hotpluggable_cpus;
 int numa_mem_align_shift;
+GArray *valid_cpu_types;
 void (*numa_auto_assign_ram)(MachineClass *mc, NodeInfo *nodes,
  int nb_nodes, ram_addr_t size);
 
-- 
2.11.0

[Qemu-devel] [RFC v1 0/2] Add a valid_cpu_types property

[Alistair Francis]

There are numorous QEMU machines that only have a single or a handful of
valid CPU options. To simplyfy the management of specificying which CPU
is/isn't valid let's create a property that can be set in the machine
init. We can then check to see if the user supplied CPU is in that list
or not.

This is just a quick setup, if this method is agreed apon I can add a
nice macro to add the valid CPU options (similar to SET_MACHINE_COMPAT)
and improve the error messages. I just wanted to get some input before I
spent too much time on that.

Alistair Francis (2):
  machine: Add a valid_cpu_types property
  netduino2: Specify the valid CPUs

 hw/arm/netduino2.c  |  5 +
 hw/core/machine.c   | 27 +++
 include/hw/boards.h |  1 +
 3 files changed, 33 insertions(+)

-- 
2.11.0

[Qemu-devel] [RFC v1 2/2] netduino2: Specify the valid CPUs

[Alistair Francis]

Signed-off-by: Alistair Francis 
---

 hw/arm/netduino2.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
index 3cfe332dd1..accca344fd 100644
--- a/hw/arm/netduino2.c
+++ b/hw/arm/netduino2.c
@@ -43,8 +43,13 @@ static void netduino2_init(MachineState *machine)
 
 static void netduino2_machine_init(MachineClass *mc)
 {
+const char *val = "cortex-m3";
+
 mc->desc = "Netduino 2 Machine";
 mc->init = netduino2_init;
+
+mc->valid_cpu_types = g_array_new(false, false, sizeof(char *));
+g_array_append_val(mc->valid_cpu_types, val);
 }
 
 DEFINE_MACHINE("netduino2", netduino2_machine_init)
-- 
2.11.0

Re: [Qemu-devel] [PATCH 03/20] target/arm: Add state field, feature bit and migration for v8M secure state

[Philippe Mathieu-Daudé]

On 08/29/2017 12:28 PM, Richard Henderson wrote:

On 08/22/2017 08:08 AM, Peter Maydell wrote:

As the first step in implementing ARM v8M's security extension:
  * add a new feature bit ARM_FEATURE_M_SECURITY
  * add the CPU state field that indicates whether the CPU is
currently in the secure state
  * add a migration subsection for this new state
(we will add the Secure copies of banked register state
to this subsection in later patches)
  * add a #define for the one new-in-v8M exception type
  * make the CPU debug log print S/NS status

Signed-off-by: Peter Maydell 
---
  target/arm/cpu.h   |  3 +++
  target/arm/cpu.c   |  4 
  target/arm/machine.c   | 20 
  target/arm/translate.c |  8 +++-
  4 files changed, 34 insertions(+), 1 deletion(-)


Reviewed-by: Richard Henderson 


Reviewed-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [Qemu-arm] [PATCH 17/20] target/arm: Make MMFAR banked for v8M

[Philippe Mathieu-Daudé]

On 08/29/2017 01:10 PM, Richard Henderson wrote:

On 08/22/2017 08:08 AM, Peter Maydell wrote:

Make the MMFAR register banked if v8M security extensions are
enabled.

Signed-off-by: Peter Maydell 
---
  target/arm/cpu.h  | 2 +-
  hw/intc/armv7m_nvic.c | 4 ++--
  target/arm/helper.c   | 4 ++--
  target/arm/machine.c  | 3 ++-
  4 files changed, 7 insertions(+), 6 deletions(-)



Reviewed-by: Richard Henderson 


Reviewed-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [Qemu-arm] [PATCH 13/20] target/arm: Make MPU_RBAR, MPU_RLAR banked for v8M

[Philippe Mathieu-Daudé]

On 08/22/2017 12:08 PM, Peter Maydell wrote:

Make the MPU registers MPU_MAIR0 and MPU_MAIR1 banked if v8M security
extensions are enabled.

We can freely add more items to vmstate_m_security without
breaking migration compatibility, because no CPU currently
has the ARM_FEATURE_M_SECURITY bit enabled and so this
subsection is not yet used by anything.

Signed-off-by: Peter Maydell 
---
  target/arm/cpu.h  |  4 ++--
  hw/intc/armv7m_nvic.c |  8 
  target/arm/cpu.c  | 26 --
  target/arm/helper.c   | 11 ++-
  target/arm/machine.c  | 12 
  5 files changed, 40 insertions(+), 21 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 2f59828..12fa95e 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -543,8 +543,8 @@ typedef struct CPUARMState {
   *  pmsav7.rnr (region number register)
   *  pmsav7_dregion (number of configured regions)
   */
-uint32_t *rbar;
-uint32_t *rlar;
+uint32_t *rbar[2];
+uint32_t *rlar[2];
  uint32_t mair0[2];
  uint32_t mair1[2];
  } pmsav8;
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index e98eb95..9ced7af 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -564,7 +564,7 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, 
MemTxAttrs attrs)
  if (region >= cpu->pmsav7_dregion) {
  return 0;
  }
-return cpu->env.pmsav8.rbar[region];
+return cpu->env.pmsav8.rbar[attrs.secure][region];
  }
  
  if (region >= cpu->pmsav7_dregion) {

@@ -591,7 +591,7 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, 
MemTxAttrs attrs)
  if (region >= cpu->pmsav7_dregion) {
  return 0;
  }
-return cpu->env.pmsav8.rlar[region];
+return cpu->env.pmsav8.rlar[attrs.secure][region];
  }
  
  if (region >= cpu->pmsav7_dregion) {

@@ -756,7 +756,7 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value,
  if (region >= cpu->pmsav7_dregion) {
  return;
  }
-cpu->env.pmsav8.rbar[region] = value;
+cpu->env.pmsav8.rbar[attrs.secure][region] = value;
  tlb_flush(CPU(cpu));
  return;
  }
@@ -806,7 +806,7 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value,
  if (region >= cpu->pmsav7_dregion) {
  return;
  }
-cpu->env.pmsav8.rlar[region] = value;
+cpu->env.pmsav8.rlar[attrs.secure][region] = value;
  tlb_flush(CPU(cpu));
  return;
  }
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index ae8af19..333029c 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -235,10 +235,20 @@ static void arm_cpu_reset(CPUState *s)
  if (arm_feature(env, ARM_FEATURE_PMSA)) {
  if (cpu->pmsav7_dregion > 0) {
  if (arm_feature(env, ARM_FEATURE_V8)) {
-memset(env->pmsav8.rbar, 0,
-   sizeof(*env->pmsav8.rbar) * cpu->pmsav7_dregion);
-memset(env->pmsav8.rlar, 0,
-   sizeof(*env->pmsav8.rlar) * cpu->pmsav7_dregion);
+memset(env->pmsav8.rbar[M_REG_NS], 0,
+   sizeof(*env->pmsav8.rbar[M_REG_NS])
+   * cpu->pmsav7_dregion);
+memset(env->pmsav8.rlar[M_REG_NS], 0,
+   sizeof(*env->pmsav8.rlar[M_REG_NS])
+   * cpu->pmsav7_dregion);
+if (arm_feature(env, ARM_FEATURE_M_SECURITY)) {
+memset(env->pmsav8.rbar[M_REG_S], 0,
+   sizeof(*env->pmsav8.rbar[M_REG_S])
+   * cpu->pmsav7_dregion);
+memset(env->pmsav8.rlar[M_REG_S], 0,
+   sizeof(*env->pmsav8.rlar[M_REG_S])
+   * cpu->pmsav7_dregion);
+}
  } else if (arm_feature(env, ARM_FEATURE_V7)) {
  memset(env->pmsav7.drbar, 0,
 sizeof(*env->pmsav7.drbar) * cpu->pmsav7_dregion);
@@ -823,8 +833,12 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
**errp)
  if (nr) {
  if (arm_feature(env, ARM_FEATURE_V8)) {
  /* PMSAv8 */
-env->pmsav8.rbar = g_new0(uint32_t, nr);
-env->pmsav8.rlar = g_new0(uint32_t, nr);
+env->pmsav8.rbar[M_REG_NS] = g_new0(uint32_t, nr);
+env->pmsav8.rlar[M_REG_NS] = g_new0(uint32_t, nr);
+if (arm_feature(env, ARM_FEATURE_M_SECURITY)) {
+env->pmsav8.rbar[M_REG_S] = g_new0(uint32_t, nr);
+env->pmsav8.rlar[M_REG_S] = g_new0(uint32_t, nr);
+}
  } else {
  

Re: [Qemu-devel] [PATCH 12/20] target/arm: Make MPU_MAIR0, MPU_MAIR1 registers banked for v8M

[Philippe Mathieu-Daudé]

On 08/22/2017 12:08 PM, Peter Maydell wrote:

Make the MPU registers MPU_MAIR0 and MPU_MAIR1 banked if v8M security
extensions are enabled.

Signed-off-by: Peter Maydell 
---
  target/arm/cpu.h  | 4 ++--
  hw/intc/armv7m_nvic.c | 8 
  target/arm/cpu.c  | 4 ++--
  target/arm/machine.c  | 6 --
  4 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index d0b0936..2f59828 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -545,8 +545,8 @@ typedef struct CPUARMState {
   */
  uint32_t *rbar;
  uint32_t *rlar;
-uint32_t mair0;
-uint32_t mair1;
+uint32_t mair0[2];
+uint32_t mair1[2];


I'm tempted to ask:

uint32_t mair[2][2];


  } pmsav8;
  
  void *nvic;

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 3a1f02d..e98eb95 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -604,12 +604,12 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, 
MemTxAttrs attrs)
  if (!arm_feature(>env, ARM_FEATURE_V8)) {
  goto bad_offset;
  }
-return cpu->env.pmsav8.mair0;
+return cpu->env.pmsav8.mair0[attrs.secure];


   return cpu->env.pmsav8.mair[0][attrs.secure];


  case 0xdc4: /* MPU_MAIR1 */
  if (!arm_feature(>env, ARM_FEATURE_V8)) {
  goto bad_offset;
  }
-return cpu->env.pmsav8.mair1;
+return cpu->env.pmsav8.mair1[attrs.secure];


   return cpu->env.pmsav8.mair[1][attrs.secure];


  default:
  bad_offset:
  qemu_log_mask(LOG_GUEST_ERROR, "NVIC: Bad read offset 0x%x\n", 
offset);
@@ -826,7 +826,7 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value,
  }
  if (cpu->pmsav7_dregion) {
  /* Register is RES0 if no MPU regions are implemented */
-cpu->env.pmsav8.mair0 = value;
+cpu->env.pmsav8.mair0[attrs.secure] = value;


   cpu->env.pmsav8.mair[0][attrs.secure] = value;


  }
  /* We don't need to do anything else because memory attributes
   * only affect cacheability, and we don't implement caching.
@@ -838,7 +838,7 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value,
  }
  if (cpu->pmsav7_dregion) {
  /* Register is RES0 if no MPU regions are implemented */
-cpu->env.pmsav8.mair1 = value;
+cpu->env.pmsav8.mair1[attrs.secure] = value;


   cpu->env.pmsav8.mair[1][attrs.secure] = value;


  }
  /* We don't need to do anything else because memory attributes
   * only affect cacheability, and we don't implement caching.
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index ae866be..ae8af19 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -249,8 +249,8 @@ static void arm_cpu_reset(CPUState *s)
  }
  }
  env->pmsav7.rnr = 0;
-env->pmsav8.mair0 = 0;
-env->pmsav8.mair1 = 0;
+memset(env->pmsav8.mair0, 0, sizeof(env->pmsav8.mair0));
+memset(env->pmsav8.mair1, 0, sizeof(env->pmsav8.mair1));

   memset(env->pmsav8.mair, 0, sizeof(env->pmsav8.mair));


  }
  
  set_flush_to_zero(1, >vfp.standard_fp_status);

diff --git a/target/arm/machine.c b/target/arm/machine.c
index cd6b6af..414a879 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -229,8 +229,8 @@ static const VMStateDescription vmstate_pmsav8 = {
vmstate_info_uint32, uint32_t),
  VMSTATE_VARRAY_UINT32(env.pmsav8.rlar, ARMCPU, pmsav7_dregion, 0,
vmstate_info_uint32, uint32_t),
-VMSTATE_UINT32(env.pmsav8.mair0, ARMCPU),
-VMSTATE_UINT32(env.pmsav8.mair1, ARMCPU),
+VMSTATE_UINT32(env.pmsav8.mair0[M_REG_NS], ARMCPU),
+VMSTATE_UINT32(env.pmsav8.mair1[M_REG_NS], ARMCPU),


etc...

matter of taste, so:
Reviewed-by: Philippe Mathieu-Daudé 


  VMSTATE_END_OF_LIST()
  }
  };
@@ -255,6 +255,8 @@ static const VMStateDescription vmstate_m_security = {
  VMSTATE_UINT32(env.v7m.faultmask[M_REG_S], ARMCPU),
  VMSTATE_UINT32(env.v7m.control[M_REG_S], ARMCPU),
  VMSTATE_UINT32(env.v7m.vecbase[M_REG_S], ARMCPU),
+VMSTATE_UINT32(env.pmsav8.mair0[M_REG_S], ARMCPU),
+VMSTATE_UINT32(env.pmsav8.mair1[M_REG_S], ARMCPU),
  VMSTATE_END_OF_LIST()
  }
  };

Re: [Qemu-devel] [Qemu-arm] [PATCH 09/20] target/arm: Make CONTROL register banked for v8M

[Philippe Mathieu-Daudé]

On 08/29/2017 12:43 PM, Richard Henderson wrote:

On 08/22/2017 08:08 AM, Peter Maydell wrote:

Make the CONTROL register banked if v8M security extensions are enabled.

Signed-off-by: Peter Maydell 
---
  target/arm/cpu.h   |  5 +++--
  target/arm/helper.c| 21 +++--
  target/arm/machine.c   |  3 ++-
  target/arm/translate.c |  2 +-
  4 files changed, 17 insertions(+), 14 deletions(-)


Reviewed-by: Richard Henderson 


Reviewed-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [Qemu-arm] [PATCH 07/20] target/arm: Make PRIMASK register banked for v8M

[Philippe Mathieu-Daudé]

On 08/22/2017 08:08 AM, Peter Maydell wrote:

Make the PRIMASK register banked if v8M security extensions are enabled.

Note that we do not yet implement the functionality of the new
AIRCR.PRIS bit (which allows the effect of the NS copy of PRIMASK to
be restricted).

Signed-off-by: Peter Maydell 
---
  target/arm/cpu.h  | 2 +-
  hw/intc/armv7m_nvic.c | 2 +-
  target/arm/helper.c   | 4 ++--
  target/arm/machine.c  | 9 +++--
  4 files changed, 11 insertions(+), 6 deletions(-)


Reviewed-by: Richard Henderson 


Reviewed-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [Qemu-arm] [PATCH 06/20] target/arm: Make BASEPRI register banked for v8M

[Philippe Mathieu-Daudé]

On 08/22/2017 12:08 PM, Peter Maydell wrote:

Make the BASEPRI register banked if v8M security extensions are enabled.

Note that we do not yet implement the functionality of the new
AIRCR.PRIS bit (which allows the effect of the NS copy of BASEPRI to
be restricted).

Signed-off-by: Peter Maydell 


(forgot to add)

Anyway:
Reviewed-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [Qemu-arm] [PATCH 19/20] target/arm: Move regime_is_secure() to target/arm/internals.h

[Philippe Mathieu-Daudé]

On 08/29/2017 01:12 PM, Richard Henderson wrote:

On 08/22/2017 08:08 AM, Peter Maydell wrote:

Move the regime_is_secure() utility function to internals.h;
we are going to want to call it from translate.c.

Signed-off-by: Peter Maydell 
---
  target/arm/internals.h | 26 ++
  target/arm/helper.c| 26 --
  2 files changed, 26 insertions(+), 26 deletions(-)


Reviewed-by: Richard Henderson 


Reviewed-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [PATCH 6/6] arm: drop intermadiate cpu_model -> cpu type parsing and use cpu type directly

[Alistair Francis]

On Tue, Sep 5, 2017 at 3:12 PM, Eduardo Habkost  wrote:
> On Tue, Sep 05, 2017 at 02:47:52PM -0700, Alistair Francis wrote:
>> On Tue, Sep 5, 2017 at 2:31 PM, Eduardo Habkost  wrote:
> [...]
>> >> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
>> >> index f61e735..1cd6374 100644
>> >> --- a/hw/arm/stm32f205_soc.c
>> >> +++ b/hw/arm/stm32f205_soc.c
>> >> @@ -112,7 +112,7 @@ static void stm32f205_soc_realize(DeviceState 
>> >> *dev_soc, Error **errp)
>> >>
>> >>  armv7m = DEVICE(>armv7m);
>> >>  qdev_prop_set_uint32(armv7m, "num-irq", 96);
>> >> -qdev_prop_set_string(armv7m, "cpu-model", s->cpu_model);
>> >> +qdev_prop_set_string(armv7m, "cpu-type", s->cpu_type);
>> >>  object_property_set_link(OBJECT(>armv7m), 
>> >> OBJECT(get_system_memory()),
>> >>   "memory", _abort);
>> >>  object_property_set_bool(OBJECT(>armv7m), true, "realized", );
>> >> @@ -200,7 +200,7 @@ static void stm32f205_soc_realize(DeviceState 
>> >> *dev_soc, Error **errp)
>> >>  }
>> >>
>> >>  static Property stm32f205_soc_properties[] = {
>> >> -DEFINE_PROP_STRING("cpu-model", STM32F205State, cpu_model),
>> >> +DEFINE_PROP_STRING("cpu-type", STM32F205State, cpu_type),
>> >
>> > Same as armv7m: are we 100% sure users are not setting this
>> > manually?
>>
>> In an embedded board like this it really doesn't make sense to let the
>> user overwrite the CPU. The SoC will take it as an option, but the
>> board (which creates the SoC) just blindly always uses the same CPU.
>> That feature is more for QOMificatoion then any real reason though.
>>
>
> I'm not talking about -cpu (no user-visible change in the
> handling of -cpu should result from this patch), but about
> possible cases where the user set the "cpu-model" property using
> another mechanism, like -global.  Probably it's impossible for an
> user to override the property successfully, but I would like to
> be sure.

Ah, that is trickier.

I guess that is possible to do, but the object setting logic should
handle the error gracefully and inform the user of the error.

>
>
>> In saying that I think a warning if the user tries to set the CPU
>> would make sense. I know that this issues comes up in other ARM boards
>> (Zynq-7000 has the same issue as well) so maybe a machine property
>> saying that the board doesn't accept custom CPUs would be a good idea.
>
> Yeah, there are multiple cases in this patch where boards are
> validating the CPU model, but not all boards do that.  A generic
> MachineClass::valid_cpu_types[] field would be useful.
>
>>
>> Overall I think this patch is moving in the right direction though and
>> this CPU option being ignored existed before this series.
>
> I agree this is going on the right direction.  However, I don't
> see any board that ignore the CPU option: all of them seem to use
> cpu_model when creating the CPUs, already.

The Netduino2 will ignore any CPU options and always use a Cortex-m3.
I was wrong about Zynq-7000 though, it does respect the -cpu option.

Thanks,
Alistair

>
> --
> Eduardo

Re: [Qemu-devel] [Qemu-arm] [PATCH 06/20] target/arm: Make BASEPRI register banked for v8M

[Philippe Mathieu-Daudé]

Hi Peter,

On 08/22/2017 12:08 PM, Peter Maydell wrote:

Make the BASEPRI register banked if v8M security extensions are enabled.

Note that we do not yet implement the functionality of the new
AIRCR.PRIS bit (which allows the effect of the NS copy of BASEPRI to
be restricted).

Signed-off-by: Peter Maydell 
---
  target/arm/cpu.h  | 14 +-
  hw/intc/armv7m_nvic.c |  4 ++--
  target/arm/helper.c   | 10 ++
  target/arm/machine.c  |  3 ++-
  4 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 436ca0d..0c28dfd 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -72,6 +72,18 @@
  #define ARMV7M_EXCP_PENDSV  14
  #define ARMV7M_EXCP_SYSTICK 15
  
+/* For M profile, some registers are banked secure vs non-secure;

+ * these are represented as a 2-element array where the first element
+ * is the non-secure copy and the second is the secure copy.
+ * When the CPU does not have implement the security extension then
+ * only the first element is used.
+ * This means that the copy for the current security state can be
+ * accessed via env->registerfield[env->v7m.secure] (whether the security
+ * extension is implemented or not).
+ */
+#define M_REG_NS 0
+#define M_REG_S 1


enum {
M_REG_NS = 0,
M_REG_S = 1,
M_BANKED_SECURE_REG_COUNT = 2;
};

or directly this?

#define M_BANKED_SECURE_REG_COUNT 2

shorter nicer...

#define M_BANKED_SECURE_REGS 2


+
  /* ARM-specific interrupt pending bits.  */
  #define CPU_INTERRUPT_FIQ   CPU_INTERRUPT_TGT_EXT_1
  #define CPU_INTERRUPT_VIRQ  CPU_INTERRUPT_TGT_EXT_2
@@ -409,7 +421,7 @@ typedef struct CPUARMState {
  struct {
  uint32_t other_sp;
  uint32_t vecbase;
-uint32_t basepri;
+uint32_t basepri[2];


uint32_t basepri[M_BANKED_SECURE_REGS];

(and following patches of this series...)


  uint32_t control;
  uint32_t ccr; /* Configuration and Control */
  uint32_t cfsr; /* Configurable Fault Status */
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index c0dbbad..2a41e5d 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -171,8 +171,8 @@ static inline int nvic_exec_prio(NVICState *s)
  running = -1;
  } else if (env->v7m.primask) {
  running = 0;
-} else if (env->v7m.basepri > 0) {
-running = env->v7m.basepri & nvic_gprio_mask(s);
+} else if (env->v7m.basepri[env->v7m.secure] > 0) {
+running = env->v7m.basepri[env->v7m.secure] & nvic_gprio_mask(s);
  } else {
  running = NVIC_NOEXC_PRIO; /* lower than any possible priority */
  }
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 1debebc..1087f19 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8838,7 +8838,7 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
  return env->v7m.primask;
  case 17: /* BASEPRI */
  case 18: /* BASEPRI_MAX */
-return env->v7m.basepri;
+return env->v7m.basepri[env->v7m.secure];
  case 19: /* FAULTMASK */
  return env->v7m.faultmask;
  default:
@@ -8898,12 +8898,14 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t 
maskreg, uint32_t val)
  env->v7m.primask = val & 1;
  break;
  case 17: /* BASEPRI */
-env->v7m.basepri = val & 0xff;
+env->v7m.basepri[env->v7m.secure] = val & 0xff;
  break;
  case 18: /* BASEPRI_MAX */
  val &= 0xff;
-if (val != 0 && (val < env->v7m.basepri || env->v7m.basepri == 0))
-env->v7m.basepri = val;
+if (val != 0 && (val < env->v7m.basepri[env->v7m.secure]
+ || env->v7m.basepri[env->v7m.secure] == 0)) {
+env->v7m.basepri[env->v7m.secure] = val;
+}
  break;
  case 19: /* FAULTMASK */
  env->v7m.faultmask = val & 1;
diff --git a/target/arm/machine.c b/target/arm/machine.c
index 745adae..8476efd 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -115,7 +115,7 @@ static const VMStateDescription vmstate_m = {
  .needed = m_needed,
  .fields = (VMStateField[]) {
  VMSTATE_UINT32(env.v7m.vecbase, ARMCPU),
-VMSTATE_UINT32(env.v7m.basepri, ARMCPU),
+VMSTATE_UINT32(env.v7m.basepri[M_REG_NS], ARMCPU),
  VMSTATE_UINT32(env.v7m.control, ARMCPU),
  VMSTATE_UINT32(env.v7m.ccr, ARMCPU),
  VMSTATE_UINT32(env.v7m.cfsr, ARMCPU),
@@ -250,6 +250,7 @@ static const VMStateDescription vmstate_m_security = {
  .needed = m_security_needed,
  .fields = (VMStateField[]) {
  VMSTATE_UINT32(env.v7m.secure, ARMCPU),
+VMSTATE_UINT32(env.v7m.basepri[M_REG_S], ARMCPU),
  VMSTATE_END_OF_LIST()
  }
  };

Re: [Qemu-devel] [PATCH v3] vhost-user: disable the *broken* subprocess tests

[Philippe Mathieu-Daudé]

not vhost-user-test, but this still failed:

ERROR:tests/rcutorture.c:388:gtest_stress: assertion failed (n_mberror
== 0): (1 == 0)
GTester: last random seed: R02S8905ef6a0d8ed82670297d559c40e919
make: *** [check-tests/rcutorture] Error 1

CONFIG="--enable-debug --enable-debug-tcg --enable-trace-backends=log"

https://travis-ci.org/philmd/qemu/jobs/272168998


[...]


Not so, at least I can't reproduce locally yet.

If it's only reproducible on travis for now, shouldn't the failing tests 
be disabled if getenv("TRAVIS") instead?


I restarted the Travis job and it succeeded so this seems to be another 
problem, not the vhost-user-test one :)


https://travis-ci.org/philmd/qemu/builds/272168994

Re: [Qemu-devel] [PATCH 4/9] s390x: refactor error handling for SSCH and RSCH

[Halil Pasic]

On 09/05/2017 06:25 PM, Cornelia Huck wrote:
> On Tue, 5 Sep 2017 17:55:17 +0200
> Halil Pasic  wrote:
> 
>> On 08/31/2017 11:55 AM, Cornelia Huck wrote:
>>> On Wed, 30 Aug 2017 18:36:04 +0200
>>> Halil Pasic  wrote:
>>>   
 Simplify the error handling of the SSCH and RSCH handler avoiding
 arbitrary and cryptic error codes being mapped to what a subchannel is
 supposed to do.  Let the code detecting the condition tell how it's to be
 handled in a less ambiguous way.  It's best to handle SSCH and RSCH in
 one go as the emulation of the two shares a lot of code.  
>>>
>>> So determining the return code at the point in time where we can
>>> instead of trying to map to return codes and back again makes quite a
>>> bit of sense, but I'll have to look at the rest of this.  
>>
>>
>> Looging forward to.
> 
> Once I manage to find some quiet time for thinking :(

I think it's the best if you ignore the rest until v2.

Since we agreed that cc 3 is not the way to go and that the basic
idea is sane, IMHO it does not make much sense to do a thorough
review of this any more.

Not diverting valuable maintainer resources from my indirect data 
access patch set is also a point (IDA is something I have to deliver, while
this is just for fun).
 
> 
 -ret = s390_ccw_cmd_request(orb, s, sch->driver_data);
 -switch (ret) {
 -/* Currently we don't update control block and just return the cc 
 code. */
 -case 0:
 -break;
 -case -EBUSY:
 -break;
 -case -ENODEV:
 -break;
 -case -EFAULT:
 - break;
 -case -EACCES:
 -/* Let's reflect an inaccessible host device by cc 3. */
 -default:
 -/* Let's make all other return codes map to cc 3.  */
 -ret = -ENODEV;
 -};
 -
 -return ret;
 +s390_ccw_cmd_request(sch);  
>>>
>>> As you change the handling anyway: Don't change this logic in prior
>>> patches?  
>>
>> I preserve the logic as altered by the previous patches (at least,
>> that is the intention). This is the mapping around part which is going
>> away if we push the handling down.
> 
> My point is that you touch the code path twice. But we disagreed on the
> original change anyway :)

Nod.

> 
 -int do_subchannel_work_passthrough(SubchDev *sch)
 +void do_subchannel_work_passthrough(SubchDev *sch)
  {
 -int ret;
  SCSW *s = >curr_status.scsw;
 
  if (s->ctrl & SCSW_FCTL_CLEAR_FUNC) {
  /* TODO: Clear handling */
  sch_handle_clear_func(sch);
 -ret = 0;
  } else if (s->ctrl & SCSW_FCTL_HALT_FUNC) {
  /* TODO: Halt handling */
  sch_handle_halt_func(sch);
 -ret = 0;
  } else if (s->ctrl & SCSW_FCTL_START_FUNC) {
 -ret = sch_handle_start_func_passthrough(sch);
 +sch_handle_start_func_passthrough(sch);
  } else {
  /* Cannot happen. */
 -return -ENODEV;
 +sch->iret.cc = 3;  
>>>
>>> ftcl == 0 should have been rejected already by the function handlers
>>> here as well, shouldn't it?  
>>
>> Which function handlers. Basically the instruction handlers set fctl
>> and call do_subchannel_work to get the indicated work done.
> 
> Or set. My point is that we can't get here with fctl == 0. So an assert
> sounds better to me.
> 
Yeah, I agree assert is the way to go here.

> 
 diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
 index 5c5fe6b202..d093181a9e 100644
 --- a/include/hw/s390x/css.h
 +++ b/include/hw/s390x/css.h
 @@ -94,13 +94,31 @@ struct SubchDev {
  /* transport-provided data: */
  int (*ccw_cb) (SubchDev *, CCW1);
  void (*disable_cb)(SubchDev *);
 -int (*do_subchannel_work) (SubchDev *);
 +void (*do_subchannel_work) (SubchDev *);
  SenseId id;
  void *driver_data;
 +/* io instructions conclude according to iret */
 +struct {
 +/*
 + * General semantic of cc codes of IO instructions is (brief):
 + * 0 -- produced expected result
 + * 1 -- produced alternate result
 + * 2 -- ineffective, because busy with previously initiated 
 function
 + * 3 -- ineffective, not operational  
>>>
>>> I'm not sure you can summarize this that way in all cases.
>>>
>>> Also, what does "ineffective" mean? If I get a cc 1 for, say, ssch, I
>>> don't expect something either as the subchannel was already status
>>> pending.  
>>
>> You are right cc 1 would have been better off with
>>  'produced alternate result or status pending'
>>
>> I've tried to make this shorter (from PoP 14-2):
>> """
>> Condition Code 0: Instruction execution produced
>> the expected or most probable result. (See “Deferred
>> Condition Code (CC)” on page 9 for 

Re: [Qemu-devel] [PATCH v6 08/29] libqtest: Let socket_send() compute length

[Eric Blake]

On 09/05/2017 04:54 AM, Markus Armbruster wrote:
> Thomas Huth  writes:
> 
>> On 01.09.2017 20:03, Eric Blake wrote:
>>> Rather than make multiple callers call strlen(), it's easier if
>>> socket_send() itself can compute a length via strlen() if none
>>> was provided (caller passes -1).  Callers that can get at the
>>> length more efficiently are left that way.
>>>
>>> Signed-off-by: Eric Blake 
>>> ---
>>>  tests/libqtest.c | 10 ++
>>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> I have to say that I don't like this idea very much. socket_send()
>> should IMHO not know about the type of the data that should be sent,
>> i.e. it should not assume that the content is a zero-terminated string.
> 
> I agree.

It doesn't assume that the content is zero-terminated unless you pass a
negative length.

> 
>> This also could lead to some hard to detect bugs later in case somebody
>> is calling the function like this:
>>
>>   size = someotherfunction();
>>   socket_send(fd, buf, size);
>>
>> ... and the someotherfunction() returned a negative error code instead
>> of a correct size.
>>
>> So I'd like to suggest to simply drop this patch.
> 
> A separate wrapper function for sending zero-terminated strings would be
> fine with me.

I'm fine dropping the patch; computing the length in the callers is not
that much more onerous (there aren't that many), so I don't think
another wrapper is needed.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 6/6] arm: drop intermadiate cpu_model -> cpu type parsing and use cpu type directly

[Eduardo Habkost]

On Tue, Sep 05, 2017 at 02:47:52PM -0700, Alistair Francis wrote:
> On Tue, Sep 5, 2017 at 2:31 PM, Eduardo Habkost  wrote:
[...]
> >> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
> >> index f61e735..1cd6374 100644
> >> --- a/hw/arm/stm32f205_soc.c
> >> +++ b/hw/arm/stm32f205_soc.c
> >> @@ -112,7 +112,7 @@ static void stm32f205_soc_realize(DeviceState 
> >> *dev_soc, Error **errp)
> >>
> >>  armv7m = DEVICE(>armv7m);
> >>  qdev_prop_set_uint32(armv7m, "num-irq", 96);
> >> -qdev_prop_set_string(armv7m, "cpu-model", s->cpu_model);
> >> +qdev_prop_set_string(armv7m, "cpu-type", s->cpu_type);
> >>  object_property_set_link(OBJECT(>armv7m), 
> >> OBJECT(get_system_memory()),
> >>   "memory", _abort);
> >>  object_property_set_bool(OBJECT(>armv7m), true, "realized", );
> >> @@ -200,7 +200,7 @@ static void stm32f205_soc_realize(DeviceState 
> >> *dev_soc, Error **errp)
> >>  }
> >>
> >>  static Property stm32f205_soc_properties[] = {
> >> -DEFINE_PROP_STRING("cpu-model", STM32F205State, cpu_model),
> >> +DEFINE_PROP_STRING("cpu-type", STM32F205State, cpu_type),
> >
> > Same as armv7m: are we 100% sure users are not setting this
> > manually?
> 
> In an embedded board like this it really doesn't make sense to let the
> user overwrite the CPU. The SoC will take it as an option, but the
> board (which creates the SoC) just blindly always uses the same CPU.
> That feature is more for QOMificatoion then any real reason though.
> 

I'm not talking about -cpu (no user-visible change in the
handling of -cpu should result from this patch), but about
possible cases where the user set the "cpu-model" property using
another mechanism, like -global.  Probably it's impossible for an
user to override the property successfully, but I would like to
be sure.


> In saying that I think a warning if the user tries to set the CPU
> would make sense. I know that this issues comes up in other ARM boards
> (Zynq-7000 has the same issue as well) so maybe a machine property
> saying that the board doesn't accept custom CPUs would be a good idea.

Yeah, there are multiple cases in this patch where boards are
validating the CPU model, but not all boards do that.  A generic
MachineClass::valid_cpu_types[] field would be useful.

> 
> Overall I think this patch is moving in the right direction though and
> this CPU option being ignored existed before this series.

I agree this is going on the right direction.  However, I don't
see any board that ignore the CPU option: all of them seem to use
cpu_model when creating the CPUs, already.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3] vhost-user: disable the *broken* subprocess tests

[Marc-André Lureau]

Hi

On Tue, Sep 5, 2017 at 9:41 PM Philippe Mathieu-Daudé 
wrote:

> not vhost-user-test, but this still failed:
>
> ERROR:tests/rcutorture.c:388:gtest_stress: assertion failed (n_mberror
> == 0): (1 == 0)
> GTester: last random seed: R02S8905ef6a0d8ed82670297d559c40e919
> make: *** [check-tests/rcutorture] Error 1
>
> CONFIG="--enable-debug --enable-debug-tcg --enable-trace-backends=log"
>
> https://travis-ci.org/philmd/qemu/jobs/272168998
>
> On 09/05/2017 03:06 PM, Philippe Mathieu-Daudé wrote:
> > tests/vhost-user-test keeps failing on build-system since Aug 15:
> >
> >ERROR:tests/vhost-user-test.c:835:test_flags_mismatch: child process
> (/i386/vhost-user/flags-mismatch/subprocess [4836]) failed unexpectedly
> > ...
> >ERROR:tests/vhost-user-test.c:807:test_connect_fail: child process
> (/x86_64/vhost-user/connect-fail/subprocess [58910]) failed unexpectedly
> >
> > Suggested-by: Peter Maydell 
> > Suggested-by: Daniel P. Berrange 
> > Signed-off-by: Philippe Mathieu-Daudé 
> > ---
> > v3: only disable tests using glib subprocess,
> >  migrate/multiqueue/guest-mem are still tested.
> >
> > v2: remove unuseful warning
> >
> >   tests/vhost-user-test.c | 17 ++---
> >   1 file changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
> > index d4da09f147..4b98018478 100644
> > --- a/tests/vhost-user-test.c
> > +++ b/tests/vhost-user-test.c
> > @@ -31,6 +31,8 @@
> >   #include 
> >   #include 
> >
> > +#define VHOST_USER_NET_TESTS_WORKING 0 /* broken as of 2.10.0 */
>

Not so, at least I can't reproduce locally yet.

If it's only reproducible on travis for now, shouldn't the failing tests be
disabled if getenv("TRAVIS") instead?

> +
> >   /* GLIB version compatibility flags */
> >   #if !GLIB_CHECK_VERSION(2, 26, 0)
> >   #define G_TIME_SPAN_SECOND  (G_GINT64_CONSTANT(100))
> > @@ -472,11 +474,6 @@ static void test_server_listen(TestServer *server)
> >   test_server_create_chr(server, ",server,nowait");
> >   }
> >
> > -static inline void test_server_connect(TestServer *server)
> > -{
> > -test_server_create_chr(server, ",reconnect=1");
> > -}
> > -
> >   #define GET_QEMU_CMD(s) \
> >   g_strdup_printf(QEMU_CMD, 512, 512, (root), (s)->chr_name,  \
> >   (s)->socket_path, "", (s)->chr_name)
> > @@ -722,7 +719,12 @@ static void wait_for_rings_started(TestServer *s,
> size_t count)
> >   g_mutex_unlock(>data_mutex);
> >   }
> >
> > -#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
> > +#if VHOST_USER_NET_TESTS_WORKING &&
> defined(CONFIG_HAS_GLIB_SUBPROCESS_TESTS)
> > +static inline void test_server_connect(TestServer *server)
> > +{
> > +test_server_create_chr(server, ",reconnect=1");
> > +}
> > +
> >   static gboolean
> >   reconnect_cb(gpointer user_data)
> >   {
> > @@ -962,7 +964,8 @@ int main(int argc, char **argv)
> >   qtest_add_data_func("/vhost-user/read-guest-mem", server,
> read_guest_mem);
> >   qtest_add_func("/vhost-user/migrate", test_migrate);
> >   qtest_add_func("/vhost-user/multiqueue", test_multiqueue);
> > -#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
> > +
> > +#if VHOST_USER_NET_TESTS_WORKING &&
> defined(CONFIG_HAS_GLIB_SUBPROCESS_TESTS)
> >   qtest_add_func("/vhost-user/reconnect/subprocess",
> >  test_reconnect_subprocess);
> >   qtest_add_func("/vhost-user/reconnect", test_reconnect);
> >
>
> --
Marc-André Lureau

Re: [Qemu-devel] [PATCH] tcg/softmmu: Increase size of TLB caches

[Richard Henderson]

On 08/29/2017 10:23 AM, Pranith Kumar wrote:
> This patch increases the number of entries cached in the TLB. I went
> over a few architectures to see if increasing it is problematic. Only
> armv6 seems to have a limitation that only 8 bits can be used for
> indexing these entries. For other architectures, the number of TLB
> entries is increased to a 4K-sized cache. The patch also doubles the
> number of victim TLB entries.
> 
> Some statistics collected from a build benchmark for various cache
> sizes is listed below:
> 
> | TLB bits\vTLB entires | 8 |16  |32 |
> | 8 | 952.94(+0.0%) | 929.99(+2.4%)  | 919.02(+3.6%) |
> |10 | 898.92(+5.6%) | 886.13(+7.0%)  | 887.03(+6.9%) |
> |12 | 878.56(+7.8%) | 873.53(+8.3%)* | 875.34(+8.1%) |
> 
> The best combination for this workload came out to be 12 bits for the
> TLB and a 16 entry vTLB cache.

This significantly degrades performance of alpha-softmmu.
It spends about 25% of all cpu time in memset.


r~

Re: [Qemu-devel] [PATCH 6/6] arm: drop intermadiate cpu_model -> cpu type parsing and use cpu type directly

[Alistair Francis]

On Tue, Sep 5, 2017 at 2:31 PM, Eduardo Habkost  wrote:
> On Mon, Sep 04, 2017 at 04:01:02PM +0200, Igor Mammedov wrote:
>> there are 2 use cases to deal with:
>>   1: fixed CPU models per board/soc
>>   2: boards with user configurable cpu_model and fallback to
>>  default cpu_model if user hasn't specified one explicitly
>>
>> For the 1st
>>   drop intermediate cpu_model parsing and use const cpu type
>>   directly, which replaces:
>>  typename = object_class_get_name(
>>cpu_class_by_name(TYPE_ARM_CPU, cpu_model))
>>  object_new(typename)
>>   with
>>  object_new(FOO_CPU_TYPE_NAME)
>>   or
>>  cpu_generic_init(BASE_CPU_TYPE, "my cpu model")
>>   with
>>  cpu_create(FOO_CPU_TYPE_NAME)
>>
>> as result 1st use case doesn't have to invoke not necessary
>> translation and not needed code is removed.
>>
>> For the 2nd
>>  1: set default cpu type with MachineClass::default_cpu_type and
>>  2: use generic cpu_model parsing that done before machine_init()
>> is run and:
>> 2.1: drop custom cpu_model parsing where pattern is:
>>typename = object_class_get_name(
>>cpu_class_by_name(TYPE_ARM_CPU, cpu_model))
>>[parse_features(typename, cpu_model, ) ]
>>
>> 2.2: or replace cpu_generic_init() which does what
>>  2.1 does + create_cpu(typename) with just
>>  create_cpu(machine->cpu_type)
>> as result cpu_name -> cpu_type translation is done using
>> generic machine code one including parsing optional features
>> if supported/present (removes a bunch of duplicated cpu_model
>> parsing code) and default cpu type is defined in an uniform way
>> within machine_class_init callbacks instead of adhoc places
>> in boadr's machine_init code.
>>
>> Signed-off-by: Igor Mammedov 
>> ---
>> CC: Peter Maydell 
>> CC: Igor Mitsyanko 
>> CC: Rob Herring 
>> CC: Andrzej Zaborowski 
>> CC: Jan Kiszka 
>> CC: Alistair Francis 
>> CC: "Edgar E. Iglesias" 
>> CC: qemu-...@nongnu.org
>> ---
>>  include/hw/arm/armv7m.h|  2 +-
>>  include/hw/arm/aspeed_soc.h|  2 +-
>>  include/hw/arm/stm32f205_soc.h |  2 +-
>>  target/arm/cpu.h   |  3 +++
>>  hw/arm/armv7m.c| 40 +---
>>  hw/arm/aspeed_soc.c| 13 +---
>>  hw/arm/collie.c| 10 +++--
>>  hw/arm/exynos4210.c|  6 +-
>>  hw/arm/gumstix.c   |  5 +++--
>>  hw/arm/highbank.c  | 10 -
>>  hw/arm/integratorcp.c  | 30 ++-
>>  hw/arm/mainstone.c |  9 -
>>  hw/arm/mps2.c  | 17 +++-
>>  hw/arm/musicpal.c  |  7 ++-
>>  hw/arm/netduino2.c |  2 +-
>>  hw/arm/nseries.c   |  4 +++-
>>  hw/arm/omap1.c |  7 ++-
>>  hw/arm/omap2.c |  4 ++--
>>  hw/arm/omap_sx1.c  |  5 -
>>  hw/arm/palm.c  |  5 +++--
>>  hw/arm/pxa2xx.c| 10 -
>>  hw/arm/realview.c  | 25 +--
>>  hw/arm/spitz.c | 12 ++-
>>  hw/arm/stellaris.c | 16 +++
>>  hw/arm/stm32f205_soc.c |  4 ++--
>>  hw/arm/strongarm.c | 10 +++--
>>  hw/arm/tosa.c  |  4 
>>  hw/arm/versatilepb.c   | 15 +++---
>>  hw/arm/vexpress.c  | 32 +
>>  hw/arm/virt.c  | 46 
>> +-
>>  hw/arm/xilinx_zynq.c   | 10 ++---
>>  hw/arm/z2.c|  9 +++--
>>  target/arm/cpu.c   |  2 +-
>>  33 files changed, 114 insertions(+), 264 deletions(-)
>>
>> diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
>> index a9b3f2a..8496743 100644
>> --- a/include/hw/arm/armv7m.h
>> +++ b/include/hw/arm/armv7m.h
>> @@ -55,7 +55,7 @@ typedef struct ARMv7MState {
>>  MemoryRegion container;
>>
>>  /* Properties */
>> -char *cpu_model;
>> +char *cpu_type;
>>  /* MemoryRegion the board provides to us (with its devices, RAM, etc) */
>>  MemoryRegion *board_memory;
>>  } ARMv7MState;
>> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
>> index 0b88baa..f26914a 100644
>> --- a/include/hw/arm/aspeed_soc.h
>> +++ b/include/hw/arm/aspeed_soc.h
>> @@ -49,7 +49,7 @@ typedef struct AspeedSoCState {
>>
>>  typedef struct AspeedSoCInfo {
>>  const char *name;
>> -const char *cpu_model;
>> +const char *cpu_type;
>>  uint32_t silicon_rev;
>>  hwaddr sdram_base;
>>  uint64_t sram_size;
>> diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h
>> index e2dce11..922a733 100644
>> --- 

Re: [Qemu-devel] [PATCH 6/6] arm: drop intermadiate cpu_model -> cpu type parsing and use cpu type directly

[Eduardo Habkost]

On Mon, Sep 04, 2017 at 04:01:02PM +0200, Igor Mammedov wrote:
> there are 2 use cases to deal with:
>   1: fixed CPU models per board/soc
>   2: boards with user configurable cpu_model and fallback to
>  default cpu_model if user hasn't specified one explicitly
> 
> For the 1st
>   drop intermediate cpu_model parsing and use const cpu type
>   directly, which replaces:
>  typename = object_class_get_name(
>cpu_class_by_name(TYPE_ARM_CPU, cpu_model))
>  object_new(typename)
>   with
>  object_new(FOO_CPU_TYPE_NAME)
>   or
>  cpu_generic_init(BASE_CPU_TYPE, "my cpu model")
>   with
>  cpu_create(FOO_CPU_TYPE_NAME)
> 
> as result 1st use case doesn't have to invoke not necessary
> translation and not needed code is removed.
> 
> For the 2nd
>  1: set default cpu type with MachineClass::default_cpu_type and
>  2: use generic cpu_model parsing that done before machine_init()
> is run and:
> 2.1: drop custom cpu_model parsing where pattern is:
>typename = object_class_get_name(
>cpu_class_by_name(TYPE_ARM_CPU, cpu_model))
>[parse_features(typename, cpu_model, ) ]
> 
> 2.2: or replace cpu_generic_init() which does what
>  2.1 does + create_cpu(typename) with just
>  create_cpu(machine->cpu_type)
> as result cpu_name -> cpu_type translation is done using
> generic machine code one including parsing optional features
> if supported/present (removes a bunch of duplicated cpu_model
> parsing code) and default cpu type is defined in an uniform way
> within machine_class_init callbacks instead of adhoc places
> in boadr's machine_init code.
> 
> Signed-off-by: Igor Mammedov 
> ---
> CC: Peter Maydell  
> CC: Igor Mitsyanko  
> CC: Rob Herring  
> CC: Andrzej Zaborowski  
> CC: Jan Kiszka  
> CC: Alistair Francis  
> CC: "Edgar E. Iglesias"  
> CC: qemu-...@nongnu.org 
> ---
>  include/hw/arm/armv7m.h|  2 +-
>  include/hw/arm/aspeed_soc.h|  2 +-
>  include/hw/arm/stm32f205_soc.h |  2 +-
>  target/arm/cpu.h   |  3 +++
>  hw/arm/armv7m.c| 40 +---
>  hw/arm/aspeed_soc.c| 13 +---
>  hw/arm/collie.c| 10 +++--
>  hw/arm/exynos4210.c|  6 +-
>  hw/arm/gumstix.c   |  5 +++--
>  hw/arm/highbank.c  | 10 -
>  hw/arm/integratorcp.c  | 30 ++-
>  hw/arm/mainstone.c |  9 -
>  hw/arm/mps2.c  | 17 +++-
>  hw/arm/musicpal.c  |  7 ++-
>  hw/arm/netduino2.c |  2 +-
>  hw/arm/nseries.c   |  4 +++-
>  hw/arm/omap1.c |  7 ++-
>  hw/arm/omap2.c |  4 ++--
>  hw/arm/omap_sx1.c  |  5 -
>  hw/arm/palm.c  |  5 +++--
>  hw/arm/pxa2xx.c| 10 -
>  hw/arm/realview.c  | 25 +--
>  hw/arm/spitz.c | 12 ++-
>  hw/arm/stellaris.c | 16 +++
>  hw/arm/stm32f205_soc.c |  4 ++--
>  hw/arm/strongarm.c | 10 +++--
>  hw/arm/tosa.c  |  4 
>  hw/arm/versatilepb.c   | 15 +++---
>  hw/arm/vexpress.c  | 32 +
>  hw/arm/virt.c  | 46 
> +-
>  hw/arm/xilinx_zynq.c   | 10 ++---
>  hw/arm/z2.c|  9 +++--
>  target/arm/cpu.c   |  2 +-
>  33 files changed, 114 insertions(+), 264 deletions(-)
> 
> diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
> index a9b3f2a..8496743 100644
> --- a/include/hw/arm/armv7m.h
> +++ b/include/hw/arm/armv7m.h
> @@ -55,7 +55,7 @@ typedef struct ARMv7MState {
>  MemoryRegion container;
>  
>  /* Properties */
> -char *cpu_model;
> +char *cpu_type;
>  /* MemoryRegion the board provides to us (with its devices, RAM, etc) */
>  MemoryRegion *board_memory;
>  } ARMv7MState;
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 0b88baa..f26914a 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -49,7 +49,7 @@ typedef struct AspeedSoCState {
>  
>  typedef struct AspeedSoCInfo {
>  const char *name;
> -const char *cpu_model;
> +const char *cpu_type;
>  uint32_t silicon_rev;
>  hwaddr sdram_base;
>  uint64_t sram_size;
> diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h
> index e2dce11..922a733 100644
> --- a/include/hw/arm/stm32f205_soc.h
> +++ b/include/hw/arm/stm32f205_soc.h
> @@ -52,7 +52,7 @@ typedef struct STM32F205State {
>  SysBusDevice parent_obj;
>  /*< public >*/
>  
> -char 

Re: [Qemu-devel] [Qemu-arm] [PATCH 01/20] target/arm: Implement ARMv8M's PMSAv8 registers

[Peter Maydell]

On 5 September 2017 at 20:16, Philippe Mathieu-Daudé  wrote:
> Hi Peter,
>
>
> On 08/22/2017 12:08 PM, Peter Maydell wrote:
>>   diff --git a/target/arm/machine.c b/target/arm/machine.c
>> index 3193b00..05e2909 100644
>> --- a/target/arm/machine.c
>> +++ b/target/arm/machine.c
>> @@ -159,7 +159,8 @@ static bool pmsav7_needed(void *opaque)
>>   CPUARMState *env = >env;
>> return arm_feature(env, ARM_FEATURE_PMSA) &&
>> -   arm_feature(env, ARM_FEATURE_V7);
>> +   arm_feature(env, ARM_FEATURE_V7) &&
>> +   !arm_feature(env, ARM_FEATURE_V8);
>>   }
>> static bool pmsav7_rgnr_vmstate_validate(void *opaque, int version_id)
>> @@ -209,6 +210,31 @@ static const VMStateDescription vmstate_pmsav7_rnr =
>> {
>>   }
>>   };
>>   +static bool pmsav8_needed(void *opaque)
>> +{
>> +ARMCPU *cpu = opaque;
>> +CPUARMState *env = >env;
>> +
>> +return arm_feature(env, ARM_FEATURE_PMSA) &&
>> +arm_feature(env, ARM_FEATURE_V8);
>> +}
>> +
>> +static const VMStateDescription vmstate_pmsav8 = {
>> +.name = "cpu/pmsav8",
>> +.version_id = 1,
>> +.minimum_version_id = 1,
>> +.needed = pmsav8_needed,
>> +.fields = (VMStateField[]) {
>> +VMSTATE_VARRAY_UINT32(env.pmsav8.rbar, ARMCPU, pmsav7_dregion, 0,
>> +  vmstate_info_uint32, uint32_t),
>> +VMSTATE_VARRAY_UINT32(env.pmsav8.rlar, ARMCPU, pmsav7_dregion, 0,
>> +  vmstate_info_uint32, uint32_t),
>> +VMSTATE_UINT32(env.pmsav8.mair0, ARMCPU),
>> +VMSTATE_UINT32(env.pmsav8.mair1, ARMCPU),
>> +VMSTATE_END_OF_LIST()
>> +}
>> +};
>> +
>>   static int get_cpsr(QEMUFile *f, void *opaque, size_t size,
>>   VMStateField *field)
>>   {
>>
>
> maybe you plan to add migration between version 22 and 23 in a later patch,
> else
>
> ...
> const VMStateDescription vmstate_arm_cpu = {
> ...
> .subsections = (const VMStateDescription*[]) {
> ...
> _pmsav8,
>
> do not forget this ^

Whoops, yes, I forgot that bit. I'm surprised the compiler
didn't complain about the unused variables...

There's no need to worry about migration compat in any
of these v8M-specific vmstate structs, because right now
there are no QEMU CPUs with v8M enabled and so the
vmstate struct can't be used. That means we can freely
add fields to them in later patches without having to
bump version numbers or otherwise keep compatibility.
Once the patch which adds a cortex-m33 CPU lands in
master the rules will change and we'll need to be more
careful.


thanks
-- PMM

Re: [Qemu-devel] [PATCH v10 0/6] fsdev: qmp interface for io throttling

[Eric Blake]

On 09/04/2017 11:07 AM, Pradeep Jagadeesh wrote:
> These patches provide the qmp interface, to query the io throttle 
> status of the all fsdev devices that are present in a vm.
> also, it provides an interface to set the io throttle parameters of a
> fsdev to a required value. some of the patches also remove the duplicate
> code that was present in block and fsdev files. 
> 
> Pradeep Jagadeesh (6):
>   throttle: factor out duplicate code
>   qmp: Create IOThrottle structure
>   throttle: move out function to reuse the code
>   hmp: create a throttle initialization function for code reusability
>   fsdev: QMP interface for throttling
>   fsdev: hmp interface for throttling

I know you're already up to v10, but your code changes conflict with
Manos throttling work, which has now landed on Kevin's branch:

https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg01051.html

T: git git://repo.or.cz/qemu/kevin.git block

How hard is it for you to post a rebased v11 on top of Manos' work?

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] target/arm: Add Jazelle feature

[Portia Stephens]

This adds the Jazelle execution state as a feature if ARM_FEATURE_V6 is
set or if the processor is arm926 or arm1026. This fixes the issue that any
BXJ instruction will result in an illegal_op. BXJ instructions will now
check if the architecture supports ARM_FEATURE_JAZELLE.

Signed-off-by: Portia Stephens 
Reviewed-by: Alistair Francis 
---

This is a rewrite from my previous patch, "target/arm: Remove 5J
architecture"
-Instead of removing 5J architecture, added a Jazelle feature.

 target/arm/cpu.c   | 3 +++
 target/arm/cpu.h   | 1 +
 target/arm/translate.c | 2 +-
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 41ae6ba3c2..0fb2fddae3 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -681,6 +681,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
**errp)
 }
 if (arm_feature(env, ARM_FEATURE_V6)) {
 set_feature(env, ARM_FEATURE_V5);
+set_feature(env, ARM_FEATURE_JAZELLE);
 if (!arm_feature(env, ARM_FEATURE_M)) {
 set_feature(env, ARM_FEATURE_AUXCR);
 }
@@ -887,6 +888,7 @@ static void arm926_initfn(Object *obj)
 set_feature(>env, ARM_FEATURE_VFP);
 set_feature(>env, ARM_FEATURE_DUMMY_C15_REGS);
 set_feature(>env, ARM_FEATURE_CACHE_TEST_CLEAN);
+set_feature(>env, ARM_FEATURE_JAZELLE);
 cpu->midr = 0x41069265;
 cpu->reset_fpsid = 0x41011090;
 cpu->ctr = 0x1dd20d2;
@@ -916,6 +918,7 @@ static void arm1026_initfn(Object *obj)
 set_feature(>env, ARM_FEATURE_AUXCR);
 set_feature(>env, ARM_FEATURE_DUMMY_C15_REGS);
 set_feature(>env, ARM_FEATURE_CACHE_TEST_CLEAN);
+set_feature(>env, ARM_FEATURE_JAZELLE);
 cpu->midr = 0x4106a262;
 cpu->reset_fpsid = 0x410110a0;
 cpu->ctr = 0x1dd20d2;
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 92771d3790..daa99169fd 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1250,6 +1250,7 @@ enum arm_features {
 ARM_FEATURE_THUMB_DSP, /* DSP insns supported in the Thumb encodings */
 ARM_FEATURE_PMU, /* has PMU support */
 ARM_FEATURE_VBAR, /* has cp15 VBAR */
+ARM_FEATURE_JAZELLE, /* has Jazelle execution state */
 };
 
 static inline int arm_feature(CPUARMState *env, int feature)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index e52a6d7622..3a9142cd7d 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -41,7 +41,7 @@
 #define ENABLE_ARCH_5 arm_dc_feature(s, ARM_FEATURE_V5)
 /* currently all emulated v5 cores are also v5TE, so don't bother */
 #define ENABLE_ARCH_5TE   arm_dc_feature(s, ARM_FEATURE_V5)
-#define ENABLE_ARCH_5J0
+#define ENABLE_ARCH_5Jarm_dc_feature(s, ARM_FEATURE_JAZELLE)
 #define ENABLE_ARCH_6 arm_dc_feature(s, ARM_FEATURE_V6)
 #define ENABLE_ARCH_6Karm_dc_feature(s, ARM_FEATURE_V6K)
 #define ENABLE_ARCH_6T2   arm_dc_feature(s, ARM_FEATURE_THUMB2)
-- 
2.14.1

Re: [Qemu-devel] [Qemu-block] [PATCH v9 6/6] qemu-iotests: add 184 for throttle filter driver

[Eric Blake]

On 09/05/2017 02:06 PM, Kevin Wolf wrote:
> Am 05.09.2017 um 18:16 hat Kevin Wolf geschrieben:
>> Am 25.08.2017 um 15:20 hat Manos Pitsidianakis geschrieben:
>>> Reviewed-by: Alberto Garcia 
>>> Signed-off-by: Manos Pitsidianakis 
>>
>> Does this test actually (still) pass for you? I can't see that it's
>> related to any recent change in master, but this is the diff that I get.
>>
>> I can update the reference output while applying, but obviously if it's
>> currently passing for you, it will fail after I "fix" it.
> 
> For the record, we discussed this on IRC. The test works correctly on
> master, but on my block branch there is a conflict with "block: pass
> bdrv_* methods to bs->file by default in block filters".
> 
> The correct action is to merge this throttle driver series after the
> conflicting patch because throttle doesn't implement .bdrv_get_info and
> needs the forwarding that the other patch implements.
> 
> I updated the test output accordingly and applied the series to my block
> branch.

Could you also squash this in to 5/6? (as long as we're intentionally
basing throttle on top of defaults, then we should use the right default
instead of duplicating things)

diff --git i/block/throttle.c w/block/throttle.c
index 7b33613372..5bca76300f 100644
--- i/block/throttle.c
+++ w/block/throttle.c
@@ -197,19 +197,6 @@ static bool
throttle_recurse_is_first_non_filter(BlockDriverState *bs,
 return bdrv_recurse_is_first_non_filter(bs->file->bs, candidate);
 }

-static int64_t coroutine_fn
throttle_co_get_block_status(BlockDriverState *bs,
- int64_t
sector_num,
- int nb_sectors,
- int *pnum,
-
BlockDriverState **file)
-{
-assert(bs->file && bs->file->bs);
-*pnum = nb_sectors;
-*file = bs->file->bs;
-return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID |
-   (sector_num << BDRV_SECTOR_BITS);
-}
-
 static BlockDriver bdrv_throttle = {
 .format_name=   "throttle",
 .protocol_name  =   "throttle",
@@ -237,7 +224,7 @@ static BlockDriver bdrv_throttle = {
 .bdrv_reopen_prepare=   throttle_reopen_prepare,
 .bdrv_reopen_commit =   throttle_reopen_commit,
 .bdrv_reopen_abort  =   throttle_reopen_abort,
-.bdrv_co_get_block_status   =   throttle_co_get_block_status,
+.bdrv_co_get_block_status   =
bdrv_co_get_block_status_from_file,

 .is_filter  =   true,
 };


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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] build-sys: generate tests/.gitignore

[Eric Blake]

On 09/05/2017 05:42 AM, Marc-André Lureau wrote:
> 
> I understand the feeling, I do also mostly out of tree build. However, I
> don't think it makes much sense to generate .gitignore in the build dir.
> It's git related, and in the git source dir, you have .git that is writable
> already: it'd be odd that the git wortree/srcdir is read-only, and the
> point is to generate .gitignore. Not so true with tarballs though.
> 
> I wrote this patch inspired by how the
> https://github.com/behdad/git.mk/blob/master/git.mk rules does it (it is
> uses by many GNOME projects and others with autoconf). The way it gets away
> with not touching the srcdir in non-git tree, is that the git.mk file is
> not shipped in distributed tarball. We could have a similar approach if
> necessary.

Shipping .gitignore in the tarball is not necessary; it's main benefit
is for in-tree development.

> 
> Another alternative, which I find much less appealing, is that qemu tree
> keeps shipping .gitignore, and we just add a manual rule to maintain it.

Keeping .gitignore in git, if it is generated, is a pain, because then
we have to manually resync it every time it gets out of date (and we're
already demonstrating that we forget to update .gitignore with new
tests).  This patch is only worthwhile if it reduces maintainer
overhead, but not if it trades one task for another.

> 
> You suggested using more test* *test etc wildcards. I think this is bad, I
> would rather have a precise .gitignore, and not ignore extra files that are
> not part of the build-system. I couldn't find a simple way to generate
> precise rules for intermediary files (like .o etc), but for end targets,
> this patch demonstrates it can be done quite easily.
> 
> Finally, there is this trend these days with meson to not even allow
> in-tree build, and thus no need for .gitignore..

Please don't go the direction of forbidding in-tree builds.  While I am
a BIG fan of VPATH builds, I'm also a big fan of in-tree builds (it's
slightly faster to do a one-off in-tree build of a fresh git checkout
than it is to set up yet another VPATH build directory).  Projects that
force one way or the other are annoying.  I see no reason why we should
not keep both approaches working, particularly if we have patchew or
other automation that covers both styles.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] build-sys: generate tests/.gitignore

[Eric Blake]

On 09/04/2017 04:03 AM, Marc-André Lureau wrote:
> tests/.gitignore is often out of date. Let's generate it based on the
> files and directories to clean.

In fact, it got out-of-date again with the recent cbb6540.

> 
> Note: I didn't succeed yet at generalizing this approach for the rest
> of qemu .gitignore files, but I hope it may eventually happen.
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  Makefile   |  7 +++-
>  tests/.gitignore   | 97 
> --
>  tests/Makefile.include | 39 +--
>  tests/migration/.gitignore |  2 -
>  4 files changed, 41 insertions(+), 104 deletions(-)
>  delete mode 100644 tests/.gitignore
>  delete mode 100644 tests/migration/.gitignore
> 

> @@ -14,6 +14,11 @@ ifneq ($(wildcard config-host.mak),)
>  all:
>  include config-host.mak
>  
> +.PHONY: gitignore
> +ifneq ($(filter-out $(UNCHECKED_GOALS),$(MAKECMDGOALS)),$(if 
> $(MAKECMDGOALS),,fail))
> +all $(MAKECMDGOALS): gitignore
> +endif

As others have mentioned, can we make this conditional on being an
in-tree build, so that a VPATH build isn't modifying non-local files
(after all, .gitignore only matters for an in-tree build)?

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PULL v2 13/47] tests/check-qlit: New, covering qobject/qlit.c

[Eric Blake]

On 09/01/2017 10:37 AM, Markus Armbruster wrote:
> From: Marc-André Lureau 
> 
> Signed-off-by: Marc-André Lureau 
> Message-Id: <20170825105913.4060-11-marcandre.lur...@redhat.com>
> [Copyright notice correction squashed in, commit message tweaked]
> Signed-off-by: Markus Armbruster 
> ---
>  tests/Makefile.include |  5 +++-
>  tests/check-qlit.c | 64 
> ++
>  2 files changed, 68 insertions(+), 1 deletion(-)
>  create mode 100644 tests/check-qlit.c

Adds a new file that is not covered by tests/.gitignore when doing an
in-tree build (and is one of our few check-* names that stands out in
comparison to the bulk of the tests named test-* or *-test).

Of course, the point may be moot if we take the patch for
auto-generating tests/.gitignore on an in-tree build...

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2] watchdog: Allow setting action on the fly

[Eric Blake]

On 09/05/2017 08:22 AM, Michal Privoznik wrote:
> Currently, the only time that users can set watchdog action is at
> the start as all we expose is this -watchdog-action command line
> argument. This is suboptimal when users want to plug the device
> later via monitor. Alternatively, they might want to change the
> action for already existing device on the fly.
> 
> At the same time, drop local redefinition of the actions eum in

s/eum/enum/

> watchdog.h in favour of the ones defined in schema.
> 
> Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1447169
> 
> Signed-off-by: Michal Privoznik 
> ---

> @@ -77,27 +80,17 @@ int select_watchdog(const char *p)
>  
>  int select_watchdog_action(const char *p)
>  {

> +int action;
>  
> +action = qapi_enum_parse(WatchdogExpirationAction_lookup, p,
> + WATCHDOG_EXPIRATION_ACTION__MAX, -1, NULL);

Semantic conflict; you need to rebase now that the qapi enum lookup code
has landed (see commit ebf677c8 and parents)

> +
> +case WATCHDOG_EXPIRATION_ACTION__MAX:
> +/* keep gcc happy */
> +break;

Could use g_assert_not_reached() here instead.

> +++ b/qapi-schema.json
> @@ -6539,3 +6539,12 @@
>  # Since 2.9
>  ##
>  { 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }
> +
> +##
> +# @watchdog-set-action:
> +#
> +# Set watchdog action
> +#
> +# Since 2.11
> +##
> +{ 'command': 'watchdog-set-action', 'data' : {'action': 
> 'WatchdogExpirationAction'} }

Can a machine have more than one watchdog device, in which case you'd
want a device name to select which watchdog gets which action?  But the
command-line version that you are replacing seems to be global, so I
guess you're okay with this interface.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 2/6] cpu: make cpu_generic_init() abort QEMU on error

[Eduardo Habkost]

On Mon, Sep 04, 2017 at 04:00:58PM +0200, Igor Mammedov wrote:
> Almost every user of cpu_generic_init() checks for
> returned NULL and then reports failure in a custom way
> and aborts process.
> Some users assume that call can't fail and don't check
> for failure, though they should have checked for it.
> 
> In either cases cpu_generic_init() failure is fatal,
> so instead of checking for failure and reporting
> it various ways, make cpu_generic_init() report
> errors in consistent way and terminate QEMU on failure.
> 
> Signed-off-by: Igor Mammedov 
> ---
> Even though it's tree wide change, it's trivial so all
> affected call sites are included within one patch.
> 
[...]
> diff --git a/qom/cpu.c b/qom/cpu.c
> index d715890..307d638 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -61,7 +61,7 @@ CPUState *cpu_create(const char *typename)
>  if (err != NULL) {
>  error_report_err(err);
>  object_unref(OBJECT(cpu));
> -return NULL;
> +exit(EXIT_FAILURE);

Isn't it simpler to use _fatal?

-- 
Eduardo

Re: [Qemu-devel] [RFC PATCH] osdep.h: Prohibit disabling assert() in supported builds

[Eric Blake]

On 08/24/2017 02:51 AM, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> On 08/22/2017 06:19 AM, Halil Pasic wrote:
>>
>>> OTOH I do think this is to some degree institutionalizing a bad practice
>>> (you say we do not want to do that, but IMHO refusing to build with
>>> NDEBUG makes only sense if we want to alter the semantic of assert so
>>> that once bad becomes acceptable). I can live with that, but I'm not
>>> happy about it. Have we considered rolling our own construct which is
>>> designed to exhibit the properties we desire?
>>

>>
>> I'd prefer that if we are going to introduce our own construct that
>> always evaluates side effects, and only has a compile-time switch on
>> whether to abort() or (foolishly) plow on, that we name it something
>> without 'assert' in the name, so that reviewers don't have to be
>> confused about remembering which variant evaluates side effects.  Maybe:
>>
>> q_verify(cond)
>>

> 
> I vote for frying bigger fish.
> 
> I also vote for using standard C when standard C is servicable.

So if it were up to me alone, the answer is:

I'm NOT going to add any new construct (whether spelled q_verify() or
otherwise), and will merely document in the commit message that we
discussed this as an alternative (so someone who wants to disable #error
can get a git history of what went into the decision).

Also, it sounds like we want to keep it #error, not #warn.

But if anyone else has strong opinions before we promote this from RFC
to actual patch, I'm still interested in your arguments.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC PATCH] osdep.h: Prohibit disabling assert() in supported builds

[Eric Blake]

On 08/21/2017 05:08 AM, Peter Maydell wrote:
> On 21 August 2017 at 10:31, Markus Armbruster  wrote:
>> I'm fine with project-wide, but does it have to be a hard error?  Can we
>> make it a warning?  Same effect by default, but it lets people
>> experiment more easily with --disable-werror.
> 
> It won't be the same effect by default for our releases, which
> don't use -Werror.
> 
> I don't think we want to make it easy for people to turn this
> off -- and it's only commenting out a single #error line if
> anybody really does want to mess about with it.

Two lines, actually, since we are also prohibiting disabling g_assert().

Anyways, as the topic came up again on the list, should I promote this
patch from RFC to actual patch? Whose tree would it go in through?

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] 答复: [PATCH] intc: arm_gicv3: limit GICR ipriority index

[Eric Blake]

On 09/05/2017 07:35 AM, Peter Maydell wrote:
> On 5 September 2017 at 13:30, niuguoxiang  wrote:
>> I think only assert is not enough, because assert() depends on NDEBUG
>> preprocessing
> 

> Incidentally, QEMU can never be compiled with NDEBUG not
> set -- we will #error in the compilation if it is not set.

Well, right now, we only #error if you happen to compile certain
devices, although the proposal has been made to #error for ALL builds:

https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg03608.html

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3] vhost-user: disable the *broken* subprocess tests

[Philippe Mathieu-Daudé]

not vhost-user-test, but this still failed:

ERROR:tests/rcutorture.c:388:gtest_stress: assertion failed (n_mberror 
== 0): (1 == 0)

GTester: last random seed: R02S8905ef6a0d8ed82670297d559c40e919
make: *** [check-tests/rcutorture] Error 1

CONFIG="--enable-debug --enable-debug-tcg --enable-trace-backends=log"

https://travis-ci.org/philmd/qemu/jobs/272168998

On 09/05/2017 03:06 PM, Philippe Mathieu-Daudé wrote:

tests/vhost-user-test keeps failing on build-system since Aug 15:

   ERROR:tests/vhost-user-test.c:835:test_flags_mismatch: child process 
(/i386/vhost-user/flags-mismatch/subprocess [4836]) failed unexpectedly
...
   ERROR:tests/vhost-user-test.c:807:test_connect_fail: child process 
(/x86_64/vhost-user/connect-fail/subprocess [58910]) failed unexpectedly

Suggested-by: Peter Maydell 
Suggested-by: Daniel P. Berrange 
Signed-off-by: Philippe Mathieu-Daudé 
---
v3: only disable tests using glib subprocess,
 migrate/multiqueue/guest-mem are still tested.

v2: remove unuseful warning

  tests/vhost-user-test.c | 17 ++---
  1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index d4da09f147..4b98018478 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -31,6 +31,8 @@
  #include 
  #include 
  
+#define VHOST_USER_NET_TESTS_WORKING 0 /* broken as of 2.10.0 */

+
  /* GLIB version compatibility flags */
  #if !GLIB_CHECK_VERSION(2, 26, 0)
  #define G_TIME_SPAN_SECOND  (G_GINT64_CONSTANT(100))
@@ -472,11 +474,6 @@ static void test_server_listen(TestServer *server)
  test_server_create_chr(server, ",server,nowait");
  }
  
-static inline void test_server_connect(TestServer *server)

-{
-test_server_create_chr(server, ",reconnect=1");
-}
-
  #define GET_QEMU_CMD(s) \
  g_strdup_printf(QEMU_CMD, 512, 512, (root), (s)->chr_name,  \
  (s)->socket_path, "", (s)->chr_name)
@@ -722,7 +719,12 @@ static void wait_for_rings_started(TestServer *s, size_t 
count)
  g_mutex_unlock(>data_mutex);
  }
  
-#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS

+#if VHOST_USER_NET_TESTS_WORKING && defined(CONFIG_HAS_GLIB_SUBPROCESS_TESTS)
+static inline void test_server_connect(TestServer *server)
+{
+test_server_create_chr(server, ",reconnect=1");
+}
+
  static gboolean
  reconnect_cb(gpointer user_data)
  {
@@ -962,7 +964,8 @@ int main(int argc, char **argv)
  qtest_add_data_func("/vhost-user/read-guest-mem", server, read_guest_mem);
  qtest_add_func("/vhost-user/migrate", test_migrate);
  qtest_add_func("/vhost-user/multiqueue", test_multiqueue);
-#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
+
+#if VHOST_USER_NET_TESTS_WORKING && defined(CONFIG_HAS_GLIB_SUBPROCESS_TESTS)
  qtest_add_func("/vhost-user/reconnect/subprocess",
 test_reconnect_subprocess);
  qtest_add_func("/vhost-user/reconnect", test_reconnect);

Re: [Qemu-devel] [PATCH] qemu-iotests: Make test 192 use QMP; convert it to Python

[Eric Blake]

On 09/04/2017 08:28 AM, Kashyap Chamarthy wrote:
> On Mon, Sep 04, 2017 at 08:55:23PM +0800, Fam Zheng wrote:
>> Hi Kashyap,
>>
>> On Mon, 09/04 13:16, Kashyap Chamarthy wrote:

>>> The test 192 ("Test NBD export with '-incoming' (non-shared
>>> storage migration use case from libvirt")) is currently using HMP.
>>> Replace the HMP usage with QMP, as the upstream preference seems to be:
>>> "Use QMP where possible, unless you're explicitly testing something
>>> related to HMP".

Kevin actually argued that keeping some HMP coverage is a GOOD thing:

https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg00798.html

>>
>> As an improvement maybe you could rebase to Stefan's "iotests: clean up
>> resources using context managers" series and switch to "with" for the temp 
>> file
>> and VM.
> 
> Good point.  I did notice that thread[*] about resource clean up.  And I
> see it's already applied to 'block-next'.  Will rebase and change to the
> 'with' statement.

I'm not sure if this patch helps us any; I personally find the unit-test
style python iotests harder to debug than the ones that produce
diff-able nnn.out files (debugging an nnn.out file that has only a list
of . doesn't make it easy to reproduce).  If the test already runs
well in shell, what does the conversion to Python actually buy us?

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for-2.10 0/3] qdev/vfio: defer DEVICE_DEL to avoid races with libvirt

[Greg Kurz]

On Wed, 9 Aug 2017 15:08:42 +1000
David Gibson  wrote:

> On Tue, Aug 08, 2017 at 01:40:08PM -0600, Alex Williamson wrote:
> > On Thu, 27 Jul 2017 12:50:42 +0100
> > "Daniel P. Berrange"  wrote:
> >   
> > > On Thu, Jul 27, 2017 at 08:53:48PM +1000, David Gibson wrote:  
> > > > On Thu, Jul 27, 2017 at 10:11:48AM +0100, Peter Maydell wrote:
> > > > > On 27 July 2017 at 02:30, Michael Roth  
> > > > > wrote:
> > > > > > In particular, Mellanox CX4 adapters on PowerNV hosts might not be 
> > > > > > fully
> > > > > > quiesced by vfio-pci's finalize() routine until up to 6s after the
> > > > > > DEVICE_DELETED was emitted, leading to detach-device on the libvirt 
> > > > > > side pretty
> > > > > > much always crashing the host.
> > > > > 
> > > > > My initial naive thought is that if the host kernel can crash then
> > > > > this is a host kernel bug... shouldn't the host kernel refuse
> > > > > the subsequent libvirt rebind if it would cause a crash ?
> > > > 
> > > > I think so too, but I haven't been able to convince Alex.  Nor
> > > > find time to fix it in the kernel myself.
> > > 
> > > I think we need to fix both the QEMU premature sending of DEVICE_DELETED
> > > and the kernel bug that allowed the crash.  
> > 
> > 
> > Where do we stand on this for v2.10?  I'd like to see it get in.  There
> > may be things to fix in the kernel, some of them may already be fixed
> > in the latest development kernel, but ultimately the kernel considers
> > driver binding to be a trusted operation and if userspace doesn't
> > understand all the dependencies, they shouldn't be doing it.  In this
> > case libvirt is using the DEVICE_DELETED signal with the assumption
> > that the device has been fully released by QEMU, which is of course not
> > accurate (libvirt could test this, but chooses not to).  libvirt
> > therefore begins trying to unbind a device that is still in use, we try
> > to handle it, but see official kernel stance that userspace is
> > responsible for understanding device dependencies, so we can only do so
> > much.
> > 
> > IMO, the next step along those lines would be that libvirt needs to
> > understand that even once a device is fully released from QEMU, it's
> > not necessarily safe to re-bind the device to a host driver.  If the
> > device is a member of a group where other devices are still in use by
> > userspace, this will violate user/host device isolation and the kernel
> > will crash to protect itself.  At best I may be able to improve this to
> > killing the userspace process making use of the conflicting device, but
> > the kernel view is that userspace (libvirt) has mandated to bind the
> > device to the host driver and we must make it so, the user is
> > responsible for the consequences.  Thanks,  
> 
> Merging it for 2.10 seems like a good idea to me to, but it's not
> really my area of expertise, and therefore not my call.
> 

It looks like there was some kind of consensus on this series, but it
didn't make it for 2.10. Is there something more to discuss ?

Cheers,

--
Greg


pgph8QIXE9MmN.pgp
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] qcow2: move qcow2_store_persistent_dirty_bitmaps() before cache flushing

[Eric Blake]

On 09/04/2017 05:18 AM, Pavel Butsykin wrote:
> After calling qcow2_inactivate(), all qcow2 caches must be flushed, but this
> may not happen, because the last call qcow2_store_persistent_dirty_bitmaps()
> can lead to marking l2/refcont cache as dirty.
> 
> Let's move qcow2_store_persistent_dirty_bitmaps() before the caсhe flushing
> to fix it.
> 
> Signed-off-by: Pavel Butsykin 
> ---
>  block/qcow2.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 

Should this cc: qemu-stable?

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-arm] [PATCH 01/20] target/arm: Implement ARMv8M's PMSAv8 registers

[Philippe Mathieu-Daudé]

Hi Peter,

On 08/22/2017 12:08 PM, Peter Maydell wrote:

As part of ARMv8M, we need to add support for the PMSAv8 MPU
architecture.

PMSAv8 differs from PMSAv7 both in register/data layout (for instance
using base and limit registers rather than base and size) and also in
behaviour (for example it does not have subregions); rather than
trying to wedge it into the existing PMSAv7 code and data structures,
we define separate ones.

This commit adds the data structures which hold the state for a
PMSAv8 MPU and the register interface to it.  The implementation of
the MPU behaviour will be added in a subsequent commit.

Signed-off-by: Peter Maydell 
---
  target/arm/cpu.h  |  13 ++
  hw/intc/armv7m_nvic.c | 122 ++
  target/arm/cpu.c  |  36 ++-
  target/arm/machine.c  |  28 +++-
  4 files changed, 179 insertions(+), 20 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index fe6edb7..b6bb78a 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -522,6 +522,19 @@ typedef struct CPUARMState {
  uint32_t rnr;
  } pmsav7;
  
+/* PMSAv8 MPU */

+struct {
+/* The PMSAv8 implementation also shares some PMSAv7 config
+ * and state:
+ *  pmsav7.rnr (region number register)
+ *  pmsav7_dregion (number of configured regions)
+ */
+uint32_t *rbar;
+uint32_t *rlar;
+uint32_t mair0;
+uint32_t mair1;
+} pmsav8;
+
  void *nvic;
  const struct arm_boot_info *boot_info;
  /* Store GICv3CPUState to access from this struct */
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index bbfe2d5..c0dbbad 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -544,25 +544,67 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset)
  {
  int region = cpu->env.pmsav7.rnr;
  
+if (arm_feature(>env, ARM_FEATURE_V8)) {

+/* PMSAv8M handling of the aliases is different from v7M:
+ * aliases A1, A2, A3 override the low two bits of the region
+ * number in MPU_RNR, and there is no 'region' field in the
+ * RBAR register.
+ */
+int aliasno = (offset - 0xd9c) / 8; /* 0..3 */
+if (aliasno) {
+region = deposit32(region, 0, 2, aliasno);
+}
+if (region >= cpu->pmsav7_dregion) {
+return 0;
+}
+return cpu->env.pmsav8.rbar[region];
+}
+
  if (region >= cpu->pmsav7_dregion) {
  return 0;
  }
  return (cpu->env.pmsav7.drbar[region] & 0x1f) | (region & 0xf);
  }
-case 0xda0: /* MPU_RASR */
-case 0xda8: /* MPU_RASR_A1 */
-case 0xdb0: /* MPU_RASR_A2 */
-case 0xdb8: /* MPU_RASR_A3 */
+case 0xda0: /* MPU_RASR (v7M), MPU_RLAR (v8M) */
+case 0xda8: /* MPU_RASR_A1 (v7M), MPU_RLAR_A1 (v8M) */
+case 0xdb0: /* MPU_RASR_A2 (v7M), MPU_RLAR_A2 (v8M) */
+case 0xdb8: /* MPU_RASR_A3 (v7M), MPU_RLAR_A3 (v8M) */
  {
  int region = cpu->env.pmsav7.rnr;
  
+if (arm_feature(>env, ARM_FEATURE_V8)) {

+/* PMSAv8M handling of the aliases is different from v7M:
+ * aliases A1, A2, A3 override the low two bits of the region
+ * number in MPU_RNR.
+ */
+int aliasno = (offset - 0xda0) / 8; /* 0..3 */
+if (aliasno) {
+region = deposit32(region, 0, 2, aliasno);
+}
+if (region >= cpu->pmsav7_dregion) {
+return 0;
+}
+return cpu->env.pmsav8.rlar[region];
+}
+
  if (region >= cpu->pmsav7_dregion) {
  return 0;
  }
  return ((cpu->env.pmsav7.dracr[region] & 0x) << 16) |
  (cpu->env.pmsav7.drsr[region] & 0x);
  }
+case 0xdc0: /* MPU_MAIR0 */
+if (!arm_feature(>env, ARM_FEATURE_V8)) {
+goto bad_offset;
+}
+return cpu->env.pmsav8.mair0;
+case 0xdc4: /* MPU_MAIR1 */
+if (!arm_feature(>env, ARM_FEATURE_V8)) {
+goto bad_offset;
+}
+return cpu->env.pmsav8.mair1;
  default:
+bad_offset:
  qemu_log_mask(LOG_GUEST_ERROR, "NVIC: Bad read offset 0x%x\n", 
offset);
  return 0;
  }
@@ -691,6 +733,26 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value)
  {
  int region;
  
+if (arm_feature(>env, ARM_FEATURE_V8)) {

+/* PMSAv8M handling of the aliases is different from v7M:
+ * aliases A1, A2, A3 override the low two bits of the region
+ * number in MPU_RNR, and there is no 'region' field in the
+ * RBAR register.
+ */
+int aliasno = (offset - 0xd9c) / 8; /* 0..3 */
+
+region = cpu->env.pmsav7.rnr;
+if (aliasno) {
+ 

[Qemu-devel] [PATCH 3/3] nbd: Use new qio_channel_*_all() functions

[Eric Blake]

Rather than open-coding our own read/write-all functions, we
can make use of the recently-added qio code.  It slightly
changes the error message in one of the iotests.

Signed-off-by: Eric Blake 
---
 include/block/nbd.h|  2 --
 nbd/nbd-internal.h | 41 +++--
 block/nbd-client.c | 15 +++
 nbd/common.c   | 45 -
 tests/qemu-iotests/083.out |  8 
 5 files changed, 18 insertions(+), 93 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index 040cdd2e60..707fd37575 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -155,8 +155,6 @@ struct NBDExportInfo {
 };
 typedef struct NBDExportInfo NBDExportInfo;

-ssize_t nbd_rwv(QIOChannel *ioc, struct iovec *iov, size_t niov, size_t length,
-bool do_read, Error **errp);
 int nbd_receive_negotiate(QIOChannel *ioc, const char *name,
   QCryptoTLSCreds *tlscreds, const char *hostname,
   QIOChannel **outioc, NBDExportInfo *info,
diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
index 03549e3f39..8a609a227f 100644
--- a/nbd/nbd-internal.h
+++ b/nbd/nbd-internal.h
@@ -85,28 +85,14 @@
 static inline int nbd_read_eof(QIOChannel *ioc, void *buffer, size_t size,
Error **errp)
 {
-struct iovec iov = { .iov_base = buffer, .iov_len = size };
-ssize_t ret;
-
-/* Sockets are kept in blocking mode in the negotiation phase.  After
- * that, a non-readable socket simply means that another thread stole
- * our request/reply.  Synchronization is done with recv_coroutine, so
- * that this is coroutine-safe.
- */
+int ret;

 assert(size);
-
-ret = nbd_rwv(ioc, , 1, size, true, errp);
-if (ret <= 0) {
-return ret;
+ret = qio_channel_read_all_eof(ioc, buffer, size, errp);
+if (ret < 0) {
+ret = -EIO;
 }
-
-if (ret != size) {
-error_setg(errp, "End of file");
-return -EINVAL;
-}
-
-return 1;
+return ret;
 }

 /* nbd_read
@@ -115,14 +101,7 @@ static inline int nbd_read_eof(QIOChannel *ioc, void 
*buffer, size_t size,
 static inline int nbd_read(QIOChannel *ioc, void *buffer, size_t size,
Error **errp)
 {
-int ret = nbd_read_eof(ioc, buffer, size, errp);
-
-if (ret == 0) {
-ret = -EINVAL;
-error_setg(errp, "End of file");
-}
-
-return ret < 0 ? ret : 0;
+return qio_channel_read_all(ioc, buffer, size, errp) < 0 ? -EIO : 0;
 }

 /* nbd_write
@@ -131,13 +110,7 @@ static inline int nbd_read(QIOChannel *ioc, void *buffer, 
size_t size,
 static inline int nbd_write(QIOChannel *ioc, const void *buffer, size_t size,
 Error **errp)
 {
-struct iovec iov = { .iov_base = (void *) buffer, .iov_len = size };
-
-ssize_t ret = nbd_rwv(ioc, , 1, size, false, errp);
-
-assert(ret < 0 || ret == size);
-
-return ret < 0 ? ret : 0;
+return qio_channel_write_all(ioc, buffer, size, errp) < 0 ? -EIO : 0;
 }

 struct NBDTLSHandshakeData {
diff --git a/block/nbd-client.c b/block/nbd-client.c
index f0dbea24d3..ee7f758e68 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -121,7 +121,7 @@ static int nbd_co_send_request(BlockDriverState *bs,
QEMUIOVector *qiov)
 {
 NBDClientSession *s = nbd_get_client_session(bs);
-int rc, ret, i;
+int rc, i;

 qemu_co_mutex_lock(>send_mutex);
 while (s->in_flight == MAX_NBD_REQUESTS) {
@@ -156,9 +156,9 @@ static int nbd_co_send_request(BlockDriverState *bs,
 qio_channel_set_cork(s->ioc, true);
 rc = nbd_send_request(s->ioc, request);
 if (rc >= 0 && !s->quit) {
-ret = nbd_rwv(s->ioc, qiov->iov, qiov->niov, request->len, false,
-  NULL);
-if (ret != request->len) {
+assert(request->len == iov_size(qiov->iov, qiov->niov));
+if (qio_channel_writev_all(s->ioc, qiov->iov, qiov->niov,
+   NULL) < 0) {
 rc = -EIO;
 }
 }
@@ -184,7 +184,6 @@ static void nbd_co_receive_reply(NBDClientSession *s,
  QEMUIOVector *qiov)
 {
 int i = HANDLE_TO_INDEX(s, request->handle);
-int ret;

 /* Wait until we're woken up by nbd_read_reply_entry.  */
 s->requests[i].receiving = true;
@@ -195,9 +194,9 @@ static void nbd_co_receive_reply(NBDClientSession *s,
 reply->error = EIO;
 } else {
 if (qiov && reply->error == 0) {
-ret = nbd_rwv(s->ioc, qiov->iov, qiov->niov, request->len, true,
-  NULL);
-if (ret != request->len) {
+assert(request->len == iov_size(qiov->iov, qiov->niov));
+if (qio_channel_readv_all(s->ioc, qiov->iov, qiov->niov,
+

[Qemu-devel] [PATCH 2/3] io: Add new qio_channel_read{, v}_all_eof functions

[Eric Blake]

Some callers want to distinguish between clean EOF (no bytes read)
vs. a short read (at least one byte read, but EOF encountered
before reaching the desired length), as it allows clients the
ability to do a graceful shutdown when a server shuts down at
defined safe points in the protocol, rather than treating all
shutdown scenarios as an error due to EOF.  However, we don't want
to require all callers to have to check for early EOF.  So add
another wrapper function that can be used by the callers that care
about the distinction.

Signed-off-by: Eric Blake 
---
 include/io/channel.h | 53 
 io/channel.c | 48 +++
 2 files changed, 93 insertions(+), 8 deletions(-)

diff --git a/include/io/channel.h b/include/io/channel.h
index 8f25893c45..3995e243a3 100644
--- a/include/io/channel.h
+++ b/include/io/channel.h
@@ -269,6 +269,36 @@ ssize_t qio_channel_writev_full(QIOChannel *ioc,
 Error **errp);

 /**
+ * qio_channel_readv_all_eof:
+ * @ioc: the channel object
+ * @iov: the array of memory regions to read data into
+ * @niov: the length of the @iov array
+ * @errp: pointer to a NULL-initialized error object
+ *
+ * Read data from the IO channel, storing it in the
+ * memory regions referenced by @iov. Each element
+ * in the @iov will be fully populated with data
+ * before the next one is used. The @niov parameter
+ * specifies the total number of elements in @iov.
+ *
+ * The function will wait for all requested data
+ * to be read, yielding from the current coroutine
+ * if required.
+ *
+ * If end-of-file occurs before any data is read,
+ * no error is reported; otherwise, if it occurs
+ * before all requested data has been read, an error
+ * will be reported.
+ *
+ * Returns: 1 if all bytes were read, 0 if end-of-file
+ *  occurs without data, or -1 on error
+ */
+int qio_channel_readv_all_eof(QIOChannel *ioc,
+  const struct iovec *iov,
+  size_t niov,
+  Error **errp);
+
+/**
  * qio_channel_readv_all:
  * @ioc: the channel object
  * @iov: the array of memory regions to read data into
@@ -383,6 +413,28 @@ ssize_t qio_channel_write(QIOChannel *ioc,
   Error **errp);

 /**
+ * qio_channel_read_all_eof:
+ * @ioc: the channel object
+ * @buf: the memory region to read data into
+ * @buflen: the number of bytes to @buf
+ * @errp: pointer to a NULL-initialized error object
+ *
+ * Reads @buflen bytes into @buf, possibly blocking or (if the
+ * channel is non-blocking) yielding from the current coroutine
+ * multiple times until the entire content is read. If end-of-file
+ * occurs immediately it is not an error, but if it occurs after
+ * data has been read it will return an error rather than a
+ * short-read. Otherwise behaves as qio_channel_read().
+ *
+ * Returns: 1 if all bytes were read, 0 if end-of-file occurs
+ *  without data, or -1 on error
+ */
+int qio_channel_read_all_eof(QIOChannel *ioc,
+ char *buf,
+ size_t buflen,
+ Error **errp);
+
+/**
  * qio_channel_read_all:
  * @ioc: the channel object
  * @buf: the memory region to read data into
@@ -401,6 +453,7 @@ int qio_channel_read_all(QIOChannel *ioc,
  char *buf,
  size_t buflen,
  Error **errp);
+
 /**
  * qio_channel_write_all:
  * @ioc: the channel object
diff --git a/io/channel.c b/io/channel.c
index 9e62794cab..ec4b86de7c 100644
--- a/io/channel.c
+++ b/io/channel.c
@@ -86,16 +86,16 @@ ssize_t qio_channel_writev_full(QIOChannel *ioc,
 }


-
-int qio_channel_readv_all(QIOChannel *ioc,
-  const struct iovec *iov,
-  size_t niov,
-  Error **errp)
+int qio_channel_readv_all_eof(QIOChannel *ioc,
+  const struct iovec *iov,
+  size_t niov,
+  Error **errp)
 {
 int ret = -1;
 struct iovec *local_iov = g_new(struct iovec, niov);
 struct iovec *local_iov_head = local_iov;
 unsigned int nlocal_iov = niov;
+bool partial = false;

 nlocal_iov = iov_copy(local_iov, nlocal_iov,
   iov, niov,
@@ -114,21 +114,43 @@ int qio_channel_readv_all(QIOChannel *ioc,
 } else if (len < 0) {
 goto cleanup;
 } else if (len == 0) {
-error_setg(errp,
-   "Unexpected end-of-file before all bytes were read");
+if (partial) {
+error_setg(errp,
+   "Unexpected end-of-file before all bytes were 
read");
+} else {
+ret = 0;
+}
 goto cleanup;
 }

+partial = true;
 

[Qemu-devel] [PATCH 0/3] nbd: Use common read/write-all qio functions

[Eric Blake]

Now that Daniel just added convenience qio channel looping functions,
we might as well use it in NBD instead of open-coding our own.  In
doing so, fix a couple of short-falls noticed in the qio code.

If Dan is happy with this, I can take the series through the NBD queue.

[The diffstat is slightly misleading - it's a net reduction in lines
of code, but the added comments hide that fact]

Eric Blake (3):
  io: Yield rather than wait when already in coroutine
  io: Add new qio_channel_read{,v}_all_eof functions
  nbd: Use new qio_channel_*_all() functions

 include/block/nbd.h|  2 --
 include/io/channel.h   | 53 
 nbd/nbd-internal.h | 41 ++-
 block/nbd-client.c | 15 ++--
 io/channel.c   | 60 ++
 nbd/common.c   | 45 --
 tests/qemu-iotests/083.out |  8 +++
 7 files changed, 121 insertions(+), 103 deletions(-)

-- 
2.13.5

[Qemu-devel] [PATCH 1/3] io: Yield rather than wait when already in coroutine

[Eric Blake]

The new qio_channel_{read,write}{,v}_all functions are documented
as yielding until data is available.  When used on a blocking
channel, this yield is done via qio_channel_wait() which spawns
a new coroutine under the hood (so it is the new entry point that
yields as needed); but if we are already in a coroutine (at which
point QIO_CHANNEL_ERR_BLOCK is only possible if we are a
non-blocking channel), we want to yield the current coroutine
instead of spawning a new one.

Signed-off-by: Eric Blake 
---
 io/channel.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/io/channel.c b/io/channel.c
index 5e8c2f0a91..9e62794cab 100644
--- a/io/channel.c
+++ b/io/channel.c
@@ -105,7 +105,11 @@ int qio_channel_readv_all(QIOChannel *ioc,
 ssize_t len;
 len = qio_channel_readv(ioc, local_iov, nlocal_iov, errp);
 if (len == QIO_CHANNEL_ERR_BLOCK) {
-qio_channel_wait(ioc, G_IO_IN);
+if (qemu_in_coroutine()) {
+qio_channel_yield(ioc, G_IO_IN);
+} else {
+qio_channel_wait(ioc, G_IO_IN);
+}
 continue;
 } else if (len < 0) {
 goto cleanup;
@@ -143,7 +147,11 @@ int qio_channel_writev_all(QIOChannel *ioc,
 ssize_t len;
 len = qio_channel_writev(ioc, local_iov, nlocal_iov, errp);
 if (len == QIO_CHANNEL_ERR_BLOCK) {
-qio_channel_wait(ioc, G_IO_OUT);
+if (qemu_in_coroutine()) {
+qio_channel_yield(ioc, G_IO_OUT);
+} else {
+qio_channel_wait(ioc, G_IO_OUT);
+}
 continue;
 }
 if (len < 0) {
-- 
2.13.5

[Qemu-devel] [Bug 1715203] Re: Maintain Haiku support

[kallisti5]

Contacting our board of directors to see if I can get a VM deployed at
Vultr. I'll keep this ticket updated.

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

Title:
  Maintain Haiku support

Status in QEMU:
  New

Bug description:
  It was pointed out that the 2.10 release notes are pushing to drop
  Haiku support.  The qemu port is currently working as-is under Haiku.

  Was there a reason this was recommended? Is there anything Haiku can
  do to keep it from being dropped?

  We're working on a docker container to cross-compile rust-lang for
  Haiku, could this be of some use to qemu when complete?

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

Re: [Qemu-devel] [Qemu-block] [PATCH v9 6/6] qemu-iotests: add 184 for throttle filter driver

[Kevin Wolf]

Am 05.09.2017 um 18:16 hat Kevin Wolf geschrieben:
> Am 25.08.2017 um 15:20 hat Manos Pitsidianakis geschrieben:
> > Reviewed-by: Alberto Garcia 
> > Signed-off-by: Manos Pitsidianakis 
> 
> Does this test actually (still) pass for you? I can't see that it's
> related to any recent change in master, but this is the diff that I get.
> 
> I can update the reference output while applying, but obviously if it's
> currently passing for you, it will fail after I "fix" it.

For the record, we discussed this on IRC. The test works correctly on
master, but on my block branch there is a conflict with "block: pass
bdrv_* methods to bs->file by default in block filters".

The correct action is to merge this throttle driver series after the
conflicting patch because throttle doesn't implement .bdrv_get_info and
needs the forwarding that the other patch implements.

I updated the test output accordingly and applied the series to my block
branch.

Kevin

Re: [Qemu-devel] [RFC v2 2/8] monitor: allow monitor to create thread to poll

[Dr. David Alan Gilbert]

* Peter Xu (pet...@redhat.com) wrote:
> On Mon, Aug 28, 2017 at 12:11:38PM +0200, Marc-André Lureau wrote:
> > Hi
> > 
> > On Mon, Aug 28, 2017 at 5:05 AM, Peter Xu  wrote:
> > > On Fri, Aug 25, 2017 at 04:07:34PM +, Marc-André Lureau wrote:
> > >> On Fri, Aug 25, 2017 at 5:33 PM Dr. David Alan Gilbert 
> > >> 
> > >> wrote:
> > >>
> > >> > * Marc-André Lureau (marcandre.lur...@gmail.com) wrote:
> > >> > > Hi
> > >> > >
> > >> > > On Wed, Aug 23, 2017 at 8:52 AM Peter Xu  wrote:
> > >> > >
> > >> > > > Firstly, introduce Monitor.use_thread, and set it for monitors 
> > >> > > > that are
> > >> > > > using non-mux typed backend chardev.  We only do this for 
> > >> > > > monitors, so
> > >> > > > mux-typed chardevs are not suitable (when it connects to, e.g., 
> > >> > > > serials
> > >> > > > and the monitor together).
> > >> > > >
> > >> > > > When use_thread is set, we create standalone thread to poll the 
> > >> > > > monitor
> > >> > > > events, isolated from the main loop thread.  Here we still need to 
> > >> > > > take
> > >> > > > the BQL before dispatching the tasks since some of the monitor 
> > >> > > > commands
> > >> > > > are not allowed to execute without the protection of BQL.  Then 
> > >> > > > this
> > >> > > > gives us the chance to avoid taking the BQL for some monitor 
> > >> > > > commands
> > >> > in
> > >> > > > the future.
> > >> > > >
> > >> > > > * Why this change?
> > >> > > >
> > >> > > > We need these per-monitor threads to make sure we can have at 
> > >> > > > least one
> > >> > > > monitor that will never stuck (that can receive further monitor
> > >> > > > commands).
> > >> > > >
> > >> > > > * So when will monitors stuck?  And, how do they stuck?
> > >> > > >
> > >> > > > After we have postcopy and remote page faults, it's simple to 
> > >> > > > achieve a
> > >> > > > stuck in the monitor (which is also a stuck in main loop thread):
> > >> > > >
> > >> > > > (1) Monitor deadlock on BQL
> > >> > > >
> > >> > > > As we may know, when postcopy is running on destination VM, the 
> > >> > > > vcpu
> > >> > > > threads can stuck merely any time as long as it tries to access an
> > >> > > > uncopied guest page.  Meanwhile, when the stuck happens, it is 
> > >> > > > possible
> > >> > > > that the vcpu thread is holding the BQL.  If the page fault is not
> > >> > > > handled quickly, you'll find that monitors stop working, which is
> > >> > trying
> > >> > > > to take the BQL.
> > >> > > >
> > >> > > > If the page fault cannot be handled correctly (one case is a paused
> > >> > > > postcopy, when network is temporarily down), monitors will hang
> > >> > > > forever.  Without current patch, that means the main loop hanged.
> > >> > We'll
> > >> > > > never find a way to talk to VM again.
> > >> > > >
> > >> > >
> > >> > > Could the BQL be pushed down to the monitor commands level instead? 
> > >> > > That
> > >> > > way we wouldn't need a seperate thread to solve the hang on commands 
> > >> > > that
> > >> > > do not need BQL.
> > >> >
> > >> > If the main thread is stuck though I don't see how that helps you; you
> > >> > have to be able to run these commands on another thread.
> > >> >
> > >>
> > >> Why would the main thread be stuck? In (1) If the vcpu thread takes the 
> > >> BQL
> > >> and the command doesn't need it, it would work.  In (2),  info cpus
> > >> shouldn't keep the BQL (my qapi-async series would probably help here)
> > >
> > > (Thanks for joining the discussion)
> > >
> > > AFAIK the main thread can be stuck for many reasons.  I have seen one
> > > stack when the VGA code (IIUC) was trying to writting to guest graphic
> > > memory in main loop thread but luckily that guest page is still not
> > > copied yet from source.  As long as the main thread is stuck for any
> > > reason, no chance for monitor commands, even if the commands support
> > > async operations.
> > 
> > If that command becomes async (it probably should, any command doing
> > IO probaly should), then the main loop can keep running.
> 
> The problem is that, it's not blocked at "a command", but a task
> running on the main thread.  The task can access guest memory, and
> when the guest page is not there, the main thread hangs.  Then it
> hangs every monitors, and all other tasks that are bounded to main
> thread.

This is my main reason for believing we need this approach; I don't
see how the async-command solution solves it.

(I don't have anything against the async command stuff, I just don't
think it solves this problem)

Dave

> > 
> > >
> > > So IMHO the only solution is doing these things in separate threads,
> > > rather than all in a single one.
> > 
> > I wouldn't say it's the only solution. I think the monitor can touch
> > many areas that haven't been written with multi-threading in mind. My
> > proposal is probably safer, although I don't know how hard it would be
> > to push the BQL down to QMP commands, and make async 

[Qemu-devel] [Bug 1714331] Re: Virtual machines not working anymore on 2.10

[Chris Unsworth]

I think my issue looks like the same.  Sometimes I just get spinning
dots, and sometimes there is the message about doing an automatic repair
below the spinning dots before it stops and uses 100% cpu.  I just did a
git bisect:

# git bisect log
# bad: [1ab5eb4efb91a3d4569b0df6e824cc08ab4bd8ec] Update version for v2.10.0 
release
# good: [6c02258e143700314ebf268dae47eb23db17d1cf] Update version for v2.9.0 
release
git bisect start 'v2.10.0' 'v2.9.0'
# bad: [269c20b2bbd2aa8531e0cdc741fb166f290d7a2b] tests/qdict: check more 
get_try_int() cases
git bisect bad 269c20b2bbd2aa8531e0cdc741fb166f290d7a2b
# bad: [eba0161990af8509608332450ee7e338273cf5df] Merge remote-tracking branch 
'rth/tags/pull-s390-20170512' into staging
git bisect bad eba0161990af8509608332450ee7e338273cf5df
# good: [9ea5ada76f34a0ef048b131c3a166d8564199bdb] audio: Use ARRAY_SIZE from 
qemu/osdep.h
git bisect good 9ea5ada76f34a0ef048b131c3a166d8564199bdb
# bad: [1effe6ad5eac1b2e50a077695ac801d172891d6a] Merge remote-tracking branch 
'danpb/tags/pull-qcrypto-2017-05-09-1' into staging
git bisect bad 1effe6ad5eac1b2e50a077695ac801d172891d6a
# good: [f03f9f0c10dcfadee5811d43240f0a6af230f1ce] Merge remote-tracking branch 
'cohuck/tags/s390x-3270-20170504' into staging
git bisect good f03f9f0c10dcfadee5811d43240f0a6af230f1ce
# good: [6c02258e143700314ebf268dae47eb23db17d1cf] qobject-input-visitor: 
Document full_name_nth()
git bisect good 6c02258e143700314ebf268dae47eb23db17d1cf
# bad: [95615ce5a1be1a5dd3597d8cb6ba83f0010e] vhost-scsi: create a 
vhost-scsi-common abstraction
git bisect bad 95615ce5a1be1a5dd3597d8cb6ba83f0010e
# bad: [31f5a726b59bda5580e2f9413867893501dd7d93] trace: add qemu mutex lock 
and unlock trace events
git bisect bad 31f5a726b59bda5580e2f9413867893501dd7d93
# bad: [49e00a18708e27c815828d9440d5c9300d19547c] use _Static_assert in 
QEMU_BUILD_BUG_ON
git bisect bad 49e00a18708e27c815828d9440d5c9300d19547c
# bad: [6103451aeb749e92bf7d730429985189c6921c32] hw/i386: Build-time assertion 
on pc/q35 reset register being identical.
git bisect bad 6103451aeb749e92bf7d730429985189c6921c32
# bad: [77af8a2b95b79699de650965d5228772743efe84] hw/i386: Use Rev3 FADT (ACPI 
2.0) instead of Rev1 to improve guest OS support.
git bisect bad 77af8a2b95b79699de650965d5228772743efe84
# first bad commit: [77af8a2b95b79699de650965d5228772743efe84] hw/i386: Use 
Rev3 FADT (ACPI 2.0) instead of Rev1 to improve guest OS support.


77af8a2b95b79699de650965d5228772743efe84 is the first bad commit
commit 77af8a2b95b79699de650965d5228772743efe84
Author: Phil Dennis-Jordan 
Date:   Wed Mar 15 19:20:26 2017 +1300

hw/i386: Use Rev3 FADT (ACPI 2.0) instead of Rev1 to improve guest
OS support.

This updates the FADT generated for x86/64 machine types from
Revision 1 to 3. (Based on ACPI standard 2.0 instead of 1.0) The
intention is to expose the reset register information to guest operating
systems which require it, specifically OS X/macOS. Revision 1 FADTs do
not contain the fields relating to the reset register.

The new layout and contents remains backwards-compatible with
operating systems which only support ACPI 1.0, as the existing fields
are not modified by this change, as the 64-bit and 32-bit variants are
allowed to co-exist according to the ACPI 2.0 standard. No regressions
became apparent in tests with a range of Windows (XP-10) and Linux
versions.

The BIOS tables test suite's FADT checksum test has also been
updated to reflect the new FADT layout and content.

Signed-off-by: Phil Dennis-Jordan 
Message-Id: <1489558827-28971-2-git-send-email-p...@philjordan.eu>
Signed-off-by: Paolo Bonzini 

:04 04 40063761c0b86f87e798e03ea48eff9ea0753425 
6d2a94150cf1eafb16f0ccf6325281415fef64a6 M  hw
:04 04 fe3f1480a91b76fea238c765f0725e715932d96d 
68f9368d8d78fd3267f609b603f97e8a74bdf528 M  include
:04 04 895e961b0a160100aa95b2f557cfe6b87a7d9bff 
8ed08cef10fddee7814e38ad62be11371592a75a M  tests

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

Title:
  Virtual machines not working anymore on 2.10

Status in QEMU:
  New

Bug description:
  Using 2.10, my virtual machine(s) don't work anymore. This happens
  100% of the times.

  -

  I use QEMU compiling it from source, on Ubuntu 16.04 amd64. This is
  the configure command:

  configure --target-list=x86_64-softmmu --enable-debug --enable-gtk
  --enable-spice --audio-drv-list=pa

  I have one virtual disk, with a Windows 10 64-bit, which I launch in
  two different ways; both work perfectly on 2.9 (and used to do on 2.8,
  but I haven't used it for a long time).

  This is the first way:

  qemu-system-x86_64
-drive if=pflash,format=raw,readonly,file=/path/to/OVMF_CODE.fd
-drive if=pflash,format=raw,file=/tmp/OVMF_VARS.fd.tmp
-enable-kvm
  

Re: [Qemu-devel] Questions regarding emulated UART in VersatilePB board

[Ramy Sameh]

Thank you very much Peter.

I will check the documentation for VersatilePB and ARM926EJ-S for more
understanding of interrupts handling.



On Tue, Sep 5, 2017 at 8:06 PM, Peter Maydell 
wrote:

> On 5 September 2017 at 18:56, Ramy Sameh  wrote:
> > Are there any documentation or source of information, that can describe
> how
> > interrupts are implemented, and how to use them (where is the vector
> table
> > to put the ISR ... etc) ?
>
> The source code is it.
>
> Note that the pl011 is only a UART. It is used in a variety
> of different boards, which have different interrupt controllers,
> and with different CPUs. Things like ISR vector tables are
> generally part of the CPU or interrupt controller emulation.
>
> The pl011's part of this is simply to raise and lower its outbound IRQ line
> when the conditions are right -- this happens by calling qemu_set_irq()
> in pl011_update(). This corresponds to the hardware's UARTINTR line.
> In QEMU that IRQ line is connected up to an emulated interrupt controller
> which in turn is connected to an emulated CPU, just as in hardware
> the pl011 UARTINTR line is connected to a hardware interrupt controller
> and thus to a CPU.
>
> For information on how guest code should use a UART, how it should
> set up interrupts and so on, you should consult the documentation
> on how the hardware behaves.
>
> > In addition, any source of info that describes the code workflow when
> > reading or writing to the uart would be very helpful.
> > (because I put printfs in the functions to understand the workflow, but
> the
> > order of these printfs made me very confused).
> >
> > Regarding the registers, I have checked pl011_read and pl011_write, and
> > found them, thanks for that.
> > But I couldn't find the register "UARTICR" (Interrupt clear register)
> > I think this might be related to my lack of understanding of how
> interrupts
> > are implemented in emulated pl011.
>
> UARTICR is a write-only register, so it is handled only in
> pl011_write():
>
> case 17: /* UARTICR */
> s->int_level &= ~value;
> pl011_update(s);
> break;
>
> (It's write-1-to-clear, hence the &= ~.)
>
> > Regarding the ID registers, whose values are hard-wired in the board, and
> > there was no need to implement modifiable states for them.
> > Do you mean Peripheral identification registers, UARTPeriphID0-3 and
> > PrimeCell identification registers, UARTPCellID0-3 ?
>
> Yes. (See the pl011_id_arm[] array which has the values.)
>
> thanks
> -- PMM
>



-- 
Best Regards,
Ramy Sameh
Embedded Software Engineer
+2-010-172-777-14

Re: [Qemu-devel] [PATCH] multiboot: validate multiboot header address values

[Thomas Garnier via Qemu-devel]

On Tue, Sep 5, 2017 at 10:49 AM, P J P  wrote:
> From: Prasad J Pandit 
>
> While loading kernel via multiboot-v1 image, (flags & 0x0001)
> indicates that multiboot header contains valid addresses to load
> the kernel image. These addresses are used to compute kernel
> size and kernel text offset in the OS image. Validate these
> address values to avoid an OOB access issue.
>
> Reported-by: Thomas Garnier 
> Signed-off-by: Prasad J Pandit 
> ---

Looks good, tested.

Tested-by: Thomas Garnier 

>  hw/i386/multiboot.c | 19 +++
>  1 file changed, 19 insertions(+)
>
> diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
> index 6001f4caa2..c7b70c91d5 100644
> --- a/hw/i386/multiboot.c
> +++ b/hw/i386/multiboot.c
> @@ -221,15 +221,34 @@ int load_multiboot(FWCfgState *fw_cfg,
>  uint32_t mh_header_addr = ldl_p(header+i+12);
>  uint32_t mh_load_end_addr = ldl_p(header+i+20);
>  uint32_t mh_bss_end_addr = ldl_p(header+i+24);
> +
>  mh_load_addr = ldl_p(header+i+16);
> +if (mh_header_addr < mh_load_addr) {
> +fprintf(stderr, "invalid mh_load_addr address\n");
> +exit(1);
> +}
> +
>  uint32_t mb_kernel_text_offset = i - (mh_header_addr - mh_load_addr);
>  uint32_t mb_load_size = 0;
>  mh_entry_addr = ldl_p(header+i+28);
>
>  if (mh_load_end_addr) {
> +if (mh_bss_end_addr < mh_load_addr) {
> +fprintf(stderr, "invalid mh_bss_end_addr address\n");
> +exit(1);
> +}
>  mb_kernel_size = mh_bss_end_addr - mh_load_addr;
> +
> +if (mh_load_end_addr < mh_load_addr) {
> +fprintf(stderr, "invalid mh_load_end_addr address\n");
> +exit(1);
> +}
>  mb_load_size = mh_load_end_addr - mh_load_addr;
>  } else {
> +if (kernel_file_size < mb_kernel_text_offset) {
> +fprintf(stderr, "invalid kernel_file_size\n");
> +exit(1);
> +}
>  mb_kernel_size = kernel_file_size - mb_kernel_text_offset;
>  mb_load_size = mb_kernel_size;
>  }
> --
> 2.13.5
>



-- 
Thomas

Re: [Qemu-devel] [PATCH] multiboot: validate multiboot header address values

[Thomas Garnier via Qemu-devel]

On Tue, Sep 5, 2017 at 11:12 AM, Thomas Garnier  wrote:
> On Tue, Sep 5, 2017 at 10:49 AM, P J P  wrote:
>> From: Prasad J Pandit 
>>
>> While loading kernel via multiboot-v1 image, (flags & 0x0001)
>> indicates that multiboot header contains valid addresses to load
>> the kernel image. These addresses are used to compute kernel
>> size and kernel text offset in the OS image. Validate these
>> address values to avoid an OOB access issue.
>>
>> Reported-by: Thomas Garnier 
>> Signed-off-by: Prasad J Pandit 
>> ---
>
> Looks good, tested.
>
> Tested-by: Thomas Garnier 

Btw, can you open a CVE for that? (and reference it in the commit).

>
>>  hw/i386/multiboot.c | 19 +++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
>> index 6001f4caa2..c7b70c91d5 100644
>> --- a/hw/i386/multiboot.c
>> +++ b/hw/i386/multiboot.c
>> @@ -221,15 +221,34 @@ int load_multiboot(FWCfgState *fw_cfg,
>>  uint32_t mh_header_addr = ldl_p(header+i+12);
>>  uint32_t mh_load_end_addr = ldl_p(header+i+20);
>>  uint32_t mh_bss_end_addr = ldl_p(header+i+24);
>> +
>>  mh_load_addr = ldl_p(header+i+16);
>> +if (mh_header_addr < mh_load_addr) {
>> +fprintf(stderr, "invalid mh_load_addr address\n");
>> +exit(1);
>> +}
>> +
>>  uint32_t mb_kernel_text_offset = i - (mh_header_addr - 
>> mh_load_addr);
>>  uint32_t mb_load_size = 0;
>>  mh_entry_addr = ldl_p(header+i+28);
>>
>>  if (mh_load_end_addr) {
>> +if (mh_bss_end_addr < mh_load_addr) {
>> +fprintf(stderr, "invalid mh_bss_end_addr address\n");
>> +exit(1);
>> +}
>>  mb_kernel_size = mh_bss_end_addr - mh_load_addr;
>> +
>> +if (mh_load_end_addr < mh_load_addr) {
>> +fprintf(stderr, "invalid mh_load_end_addr address\n");
>> +exit(1);
>> +}
>>  mb_load_size = mh_load_end_addr - mh_load_addr;
>>  } else {
>> +if (kernel_file_size < mb_kernel_text_offset) {
>> +fprintf(stderr, "invalid kernel_file_size\n");
>> +exit(1);
>> +}
>>  mb_kernel_size = kernel_file_size - mb_kernel_text_offset;
>>  mb_load_size = mb_kernel_size;
>>  }
>> --
>> 2.13.5
>>
>
>
>
> --
> Thomas



-- 
Thomas

Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH] tests: Add a device_add/del HMP test

[Dr. David Alan Gilbert]

* Thomas Huth (th...@redhat.com) wrote:
> On 05.09.2017 18:48, Dr. David Alan Gilbert wrote:
> > * Markus Armbruster (arm...@redhat.com) wrote:
> >> Thomas Huth  writes:
> >>
> >>> People tend to forget to mark internal devices with "user_creatable = 
> >>> false
> >>> or hotpluggable = false, and these devices can crash QEMU if added via the
> >>> HMP monitor. So let's add a test to run through all devices and that tries
> >>> to add them blindly (without arguments) to see whether this could crash 
> >>> the
> >>> QEMU instance.
> >>>
> >>> Signed-off-by: Thomas Huth 
> >>> ---
> >>>  I've marked the patch as RFC since not all of the required device bug
> >>>  fixes have been merged yet (so this patch can not be included yet without
> >>>  breaking "make check"). It's also sad that "macio-oldworld" currently
> >>>  has to be blacklisted - I tried to find a fix for that device,  but I was
> >>>  not able to spot the exact problem so far. So help for fixing that device
> >>>  is very very welcome! The crash can be reproduced like this:
> >>>
> >>>  $ qemu-system-ppc64 -nographic -S -monitor stdio -serial none
> >>>  QEMU 2.10.50 monitor - type 'help' for more information
> >>>  (qemu) device_add macio-oldworld,id=x
> >>>  (qemu) device_del x
> >>>  (qemu) **
> >>>  ERROR:qom/object.c:1611:object_get_canonical_path_component:
> >>>   assertion failed: (obj->parent != NULL)
> >>>  Aborted (core dumped)
> >>>
> >>>  tests/test-hmp.c | 60 
> >>> +++-
> >>>  1 file changed, 59 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/tests/test-hmp.c b/tests/test-hmp.c
> >>> index 7a38cdc..e8a25c4 100644
> >>> --- a/tests/test-hmp.c
> >>> +++ b/tests/test-hmp.c
> >>> @@ -28,7 +28,6 @@ static const char *hmp_cmds[] = {
> >>>  "commit all",
> >>>  "cpu-add 1",
> >>>  "cpu 0",
> >>> -"device_add ?",
> >>>  "device_add usb-mouse,id=mouse1",
> >>>  "mouse_button 7",
> >>>  "mouse_move 10 10",
> >>> @@ -116,6 +115,64 @@ static void test_info_commands(void)
> >>>  g_free(info_buf);
> >>>  }
> >>>  
> >>> +/*
> >>> + * People tend to forget to mark internal devices with "user_creatable = 
> >>> false
> >>> + * and these devices can crash QEMU if added via the HMP monitor. So 
> >>> let's run
> >>> + * through all devices and try to add them blindly (without arguments) 
> >>> to see
> >>> + * whether this could crash the QEMU instance.
> >>> + */
> >>> +static void test_device_add_commands(void)
> >>> +{
> >>> +char *resp, *devices, *devices_buf, *endp;
> >>> +
> >>> +devices = devices_buf = hmp("device_add help");
> >>> +
> >>> +while (*devices) {
> >>> +/* Ignore header lines etc. */
> >>> +if (strncmp(devices, "name \"", 6)) {
> >>> +devices = strchr(devices, '\n');
> >>> +if (!devices) {
> >>> +break;
> >>> +}
> >>> +devices += 1;
> >>> +continue;
> >>> +}
> >>> +/* Extract the device name, ignore parameters and description */
> >>> +devices += 6;
> >>> +endp = strchr(devices, '"');
> >>> +g_assert(endp != NULL);
> >>> +*endp = '\0';
> >>> +/* Check whether it is blacklisted... */
> >>> +if (g_str_equal("macio-oldworld", devices)) {
> >>> +devices = strchr(endp + 1, '\n');
> >>> +if (!devices) {
> >>> +break;
> >>> +}
> >>> +devices += 1;
> >>> +continue;
> >>> +}
> >>> +/* Now run the device_add + device_del commands */
> >>> +if (verbose) {
> >>> +fprintf(stderr, "\tdevice_add %s,id=%s\n", devices, devices);
> >>> +}
> >>> +resp = hmp("device_add %s,id=%s", devices, devices);
> >>> +g_free(resp);
> >>> +if (verbose) {
> >>> +fprintf(stderr, "\tdevice_del %s\n", devices);
> >>> +}
> >>> +resp = hmp("device_del %s", devices);
> >>> +g_free(resp);
> >>> +/* And move forward to the next line */
> >>> +devices = strchr(endp + 1, '\n');
> >>> +if (!devices) {
> >>> +break;
> >>> +}
> >>> +devices += 1;
> >>> +}
> >>> +
> >>> +g_free(devices_buf);
> >>> +}
> >>> +
> >>>  static void test_machine(gconstpointer data)
> >>>  {
> >>>  const char *machine = data;
> >>> @@ -125,6 +182,7 @@ static void test_machine(gconstpointer data)
> >>>  qtest_start(args);
> >>>  
> >>>  test_info_commands();
> >>> +test_device_add_commands();
> >>>  test_commands();
> >>>  
> >>>  qtest_end();
> >>
> >> This finds devices by parsing output of HMP help.  I think you should
> >> use introspection instead, like device-introspect-test does.  You may
> >> want to extract common utility code from there.
> 
> Well, I wrote a HMP test, to simulate what users can do at the HMP
> prompt. But ok, it's likely to 

Re: [Qemu-devel] [PATCH v2] io: add new qio_channel_{readv, writev, read, write}_all functions

[Eric Blake]

On 08/31/2017 04:46 AM, Daniel P. Berrange wrote:
> These functions wait until they are able to read / write the full
> requested data buffer(s).
> 
> Signed-off-by: Daniel P. Berrange 
> ---
> 
> Changed in v2:
> 
>  - Remove 'coroutine_fn' annotation (Stefan)
>  - Fix docs typos (Eric)
>  - Remove bogus error overwriting (Eric)
> 
>  include/io/channel.h   |  90 +++
>  io/channel.c   |  94 +
>  tests/io-channel-helpers.c | 102 
> -
>  3 files changed, 193 insertions(+), 93 deletions(-)

Already applied, but just now noticing two things:

> 
> diff --git a/include/io/channel.h b/include/io/channel.h
> index 54f3dc252f..8f25893c45 100644
> --- a/include/io/channel.h
> +++ b/include/io/channel.h
> @@ -269,6 +269,58 @@ ssize_t qio_channel_writev_full(QIOChannel *ioc,
>  Error **errp);
>  
>  /**
> + * qio_channel_readv_all:
> + * @ioc: the channel object
> + * @iov: the array of memory regions to read data into
> + * @niov: the length of the @iov array
> + * @errp: pointer to a NULL-initialized error object
> + *
> + * Read data from the IO channel, storing it in the
> + * memory regions referenced by @iov. Each element
> + * in the @iov will be fully populated with data
> + * before the next one is used. The @niov parameter
> + * specifies the total number of elements in @iov.
> + *
> + * The function will wait for all requested data
> + * to be read, yielding from the current coroutine
> + * if required.

[1] This states we yield...

> + *
> + * If end-of-file occurs before all requested data
> + * has been read, an error will be reported.

[2] nbd_read_eof() can't use this function, because it wants to
distinguish between early EOF (no data at all - server quit at a sane
point in the protocol, so the client doesn't need to report an error but
just start clean shutdown) and short read (EOF encountered after at
least one byte was read - server quit mid-message and client must report
the error).  But we don't want all callers to have to check for this
corner-case.  Obvious solution: add qio_channel_readv_all_eof() (I've
got the patch ready to submit, and can take it through my NBD tree since
I'm also patch nbd to take advantage of these new functions).

> +int qio_channel_readv_all(QIOChannel *ioc,

> + *
> + * The function will wait for all requested data
> + * to be written, yielding from the current coroutine
> + * if required.

[1] again

> +++ b/io/channel.c
> @@ -22,6 +22,7 @@
>  #include "io/channel.h"
>  #include "qapi/error.h"
>  #include "qemu/main-loop.h"
> +#include "qemu/iov.h"
>  
>  bool qio_channel_has_feature(QIOChannel *ioc,
>   QIOChannelFeature feature)
> @@ -85,6 +86,79 @@ ssize_t qio_channel_writev_full(QIOChannel *ioc,
>  }
>  
>  
> +
> +int qio_channel_readv_all(QIOChannel *ioc,
> +  const struct iovec *iov,
> +  size_t niov,
> +  Error **errp)
> +{
> +int ret = -1;
> +struct iovec *local_iov = g_new(struct iovec, niov);
> +struct iovec *local_iov_head = local_iov;
> +unsigned int nlocal_iov = niov;
> +
> +nlocal_iov = iov_copy(local_iov, nlocal_iov,
> +  iov, niov,
> +  0, iov_size(iov, niov));
> +
> +while (nlocal_iov > 0) {
> +ssize_t len;
> +len = qio_channel_readv(ioc, local_iov, nlocal_iov, errp);
> +if (len == QIO_CHANNEL_ERR_BLOCK) {
> +qio_channel_wait(ioc, G_IO_IN);

[1] Wait a minute. qio_channel_wait() spawns a NEW coroutine, rather
than yielding the current coroutine. nbd_rwv() was using
qio_channel_yield() here (after asserting that QIO_CHANNEL_ERR_BLOCK is
only possible for a non-blocking channel).  Keeping the wait in place
breaks iotests (things hang), while s/wait/yield/ lets NBD get through
iotests, but then breaks check-tests/test-io/channel-socket.  So I think
we have to make the code choose between the two conditions according to
whether it is currently in a coroutine.  I'll propose that patch, and
see what you say about it :)

> +continue;
> +} else if (len < 0) {
> +goto cleanup;
> +} else if (len == 0) {
> +error_setg(errp,
> +   "Unexpected end-of-file before all bytes were read");

[2] This is where the special-casing for early EOF vs. short read fits.


> +int qio_channel_writev_all(QIOChannel *ioc,
> +   const struct iovec *iov,
> +   size_t niov,
> +   Error **errp)
> +{
> +int ret = -1;
> +struct iovec *local_iov = g_new(struct iovec, niov);
> +struct iovec *local_iov_head = local_iov;
> +unsigned int nlocal_iov = niov;
> +
> +nlocal_iov = iov_copy(local_iov, nlocal_iov,
> +  

Re: [Qemu-devel] [Bug 1715203] Re: Maintain Haiku support

[Peter Maydell]

On 5 September 2017 at 19:14, kallisti5 <1715...@bugs.launchpad.net> wrote:
> Haiku recently got a virtio driver, so running a Haiku system in cloud
> infrastructure seems possible now. (I've personally run it on vultr.com)
> Does QEMU have some infrastructure available so we can stand up a x86_64
> system?

Nope. We would be looking either for ssh login on a system somebody
else admins, or detailed instructions on how to set up a VM for it,
like the ones we have for BSD at https://wiki.qemu.org/index.php/Hosts/BSD

thanks
-- PMM

[Qemu-devel] [Bug 1715203] Re: Maintain Haiku support

[kallisti5]

Haiku recently got a virtio driver, so running a Haiku system in cloud
infrastructure seems possible now. (I've personally run it on vultr.com)
Does QEMU have some infrastructure available so we can stand up a x86_64
system?

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

Title:
  Maintain Haiku support

Status in QEMU:
  New

Bug description:
  It was pointed out that the 2.10 release notes are pushing to drop
  Haiku support.  The qemu port is currently working as-is under Haiku.

  Was there a reason this was recommended? Is there anything Haiku can
  do to keep it from being dropped?

  We're working on a docker container to cross-compile rust-lang for
  Haiku, could this be of some use to qemu when complete?

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

Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH] tests: Add a device_add/del HMP test

[Thomas Huth]

On 05.09.2017 18:48, Dr. David Alan Gilbert wrote:
> * Markus Armbruster (arm...@redhat.com) wrote:
>> Thomas Huth  writes:
>>
>>> People tend to forget to mark internal devices with "user_creatable = false
>>> or hotpluggable = false, and these devices can crash QEMU if added via the
>>> HMP monitor. So let's add a test to run through all devices and that tries
>>> to add them blindly (without arguments) to see whether this could crash the
>>> QEMU instance.
>>>
>>> Signed-off-by: Thomas Huth 
>>> ---
>>>  I've marked the patch as RFC since not all of the required device bug
>>>  fixes have been merged yet (so this patch can not be included yet without
>>>  breaking "make check"). It's also sad that "macio-oldworld" currently
>>>  has to be blacklisted - I tried to find a fix for that device,  but I was
>>>  not able to spot the exact problem so far. So help for fixing that device
>>>  is very very welcome! The crash can be reproduced like this:
>>>
>>>  $ qemu-system-ppc64 -nographic -S -monitor stdio -serial none
>>>  QEMU 2.10.50 monitor - type 'help' for more information
>>>  (qemu) device_add macio-oldworld,id=x
>>>  (qemu) device_del x
>>>  (qemu) **
>>>  ERROR:qom/object.c:1611:object_get_canonical_path_component:
>>>   assertion failed: (obj->parent != NULL)
>>>  Aborted (core dumped)
>>>
>>>  tests/test-hmp.c | 60 
>>> +++-
>>>  1 file changed, 59 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/test-hmp.c b/tests/test-hmp.c
>>> index 7a38cdc..e8a25c4 100644
>>> --- a/tests/test-hmp.c
>>> +++ b/tests/test-hmp.c
>>> @@ -28,7 +28,6 @@ static const char *hmp_cmds[] = {
>>>  "commit all",
>>>  "cpu-add 1",
>>>  "cpu 0",
>>> -"device_add ?",
>>>  "device_add usb-mouse,id=mouse1",
>>>  "mouse_button 7",
>>>  "mouse_move 10 10",
>>> @@ -116,6 +115,64 @@ static void test_info_commands(void)
>>>  g_free(info_buf);
>>>  }
>>>  
>>> +/*
>>> + * People tend to forget to mark internal devices with "user_creatable = 
>>> false
>>> + * and these devices can crash QEMU if added via the HMP monitor. So let's 
>>> run
>>> + * through all devices and try to add them blindly (without arguments) to 
>>> see
>>> + * whether this could crash the QEMU instance.
>>> + */
>>> +static void test_device_add_commands(void)
>>> +{
>>> +char *resp, *devices, *devices_buf, *endp;
>>> +
>>> +devices = devices_buf = hmp("device_add help");
>>> +
>>> +while (*devices) {
>>> +/* Ignore header lines etc. */
>>> +if (strncmp(devices, "name \"", 6)) {
>>> +devices = strchr(devices, '\n');
>>> +if (!devices) {
>>> +break;
>>> +}
>>> +devices += 1;
>>> +continue;
>>> +}
>>> +/* Extract the device name, ignore parameters and description */
>>> +devices += 6;
>>> +endp = strchr(devices, '"');
>>> +g_assert(endp != NULL);
>>> +*endp = '\0';
>>> +/* Check whether it is blacklisted... */
>>> +if (g_str_equal("macio-oldworld", devices)) {
>>> +devices = strchr(endp + 1, '\n');
>>> +if (!devices) {
>>> +break;
>>> +}
>>> +devices += 1;
>>> +continue;
>>> +}
>>> +/* Now run the device_add + device_del commands */
>>> +if (verbose) {
>>> +fprintf(stderr, "\tdevice_add %s,id=%s\n", devices, devices);
>>> +}
>>> +resp = hmp("device_add %s,id=%s", devices, devices);
>>> +g_free(resp);
>>> +if (verbose) {
>>> +fprintf(stderr, "\tdevice_del %s\n", devices);
>>> +}
>>> +resp = hmp("device_del %s", devices);
>>> +g_free(resp);
>>> +/* And move forward to the next line */
>>> +devices = strchr(endp + 1, '\n');
>>> +if (!devices) {
>>> +break;
>>> +}
>>> +devices += 1;
>>> +}
>>> +
>>> +g_free(devices_buf);
>>> +}
>>> +
>>>  static void test_machine(gconstpointer data)
>>>  {
>>>  const char *machine = data;
>>> @@ -125,6 +182,7 @@ static void test_machine(gconstpointer data)
>>>  qtest_start(args);
>>>  
>>>  test_info_commands();
>>> +test_device_add_commands();
>>>  test_commands();
>>>  
>>>  qtest_end();
>>
>> This finds devices by parsing output of HMP help.  I think you should
>> use introspection instead, like device-introspect-test does.  You may
>> want to extract common utility code from there.

Well, I wrote a HMP test, to simulate what users can do at the HMP
prompt. But ok, it's likely to crash QEMU also when using the QMP
interface instead ... but then the code should also go into a different
.c file, I think.

>> The actual device_add and device_del also use HMP.  Failures are
>> ignored.  A few device_add failures I'd expect:
>>
>> * There is no suitable bus.
>>
>> * There are suitable 

Re: [Qemu-devel] [Bug 1715203] [NEW] Maintain Haiku support

[Peter Maydell]

On 5 September 2017 at 18:55, kallisti5 <1715...@bugs.launchpad.net> wrote:
> It was pointed out that the 2.10 release notes are pushing to drop Haiku
> support.  The qemu port is currently working as-is under Haiku.
>
> Was there a reason this was recommended? Is there anything Haiku can do
> to keep it from being dropped?

Basically we don't want to try to support host systems where we
have no access to hardware that will let us compile and run
the test suite, and where there's nobody interacting with us
upstream to help fix issues that are Haiku related.

If there's genuinely a community of Haiku QEMU users out there
who want to help us maintain the support in the QEMU codebase that's
great. What we don't want is to be carrying around code we can't
test and where we don't seem to have anybody using it. (For instance
we're about to drop support for AIX...)

> We're working on a docker container to cross-compile rust-lang for
> Haiku, could this be of some use to qemu when complete?

Cross-compilation won't let us run the test suite.

thanks
-- PMM

Re: [Qemu-devel] Questions regarding emulated UART in VersatilePB board

[Peter Maydell]

On 5 September 2017 at 18:56, Ramy Sameh  wrote:
> Are there any documentation or source of information, that can describe how
> interrupts are implemented, and how to use them (where is the vector table
> to put the ISR ... etc) ?

The source code is it.

Note that the pl011 is only a UART. It is used in a variety
of different boards, which have different interrupt controllers,
and with different CPUs. Things like ISR vector tables are
generally part of the CPU or interrupt controller emulation.

The pl011's part of this is simply to raise and lower its outbound IRQ line
when the conditions are right -- this happens by calling qemu_set_irq()
in pl011_update(). This corresponds to the hardware's UARTINTR line.
In QEMU that IRQ line is connected up to an emulated interrupt controller
which in turn is connected to an emulated CPU, just as in hardware
the pl011 UARTINTR line is connected to a hardware interrupt controller
and thus to a CPU.

For information on how guest code should use a UART, how it should
set up interrupts and so on, you should consult the documentation
on how the hardware behaves.

> In addition, any source of info that describes the code workflow when
> reading or writing to the uart would be very helpful.
> (because I put printfs in the functions to understand the workflow, but the
> order of these printfs made me very confused).
>
> Regarding the registers, I have checked pl011_read and pl011_write, and
> found them, thanks for that.
> But I couldn't find the register "UARTICR" (Interrupt clear register)
> I think this might be related to my lack of understanding of how interrupts
> are implemented in emulated pl011.

UARTICR is a write-only register, so it is handled only in
pl011_write():

case 17: /* UARTICR */
s->int_level &= ~value;
pl011_update(s);
break;

(It's write-1-to-clear, hence the &= ~.)

> Regarding the ID registers, whose values are hard-wired in the board, and
> there was no need to implement modifiable states for them.
> Do you mean Peripheral identification registers, UARTPeriphID0-3 and
> PrimeCell identification registers, UARTPCellID0-3 ?

Yes. (See the pl011_id_arm[] array which has the values.)

thanks
-- PMM

[Qemu-devel] [PATCH v3] vhost-user: disable the *broken* subprocess tests

[Philippe Mathieu-Daudé]

tests/vhost-user-test keeps failing on build-system since Aug 15:

  ERROR:tests/vhost-user-test.c:835:test_flags_mismatch: child process 
(/i386/vhost-user/flags-mismatch/subprocess [4836]) failed unexpectedly
...
  ERROR:tests/vhost-user-test.c:807:test_connect_fail: child process 
(/x86_64/vhost-user/connect-fail/subprocess [58910]) failed unexpectedly

Suggested-by: Peter Maydell 
Suggested-by: Daniel P. Berrange 
Signed-off-by: Philippe Mathieu-Daudé 
---
v3: only disable tests using glib subprocess,
migrate/multiqueue/guest-mem are still tested.

v2: remove unuseful warning

 tests/vhost-user-test.c | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index d4da09f147..4b98018478 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -31,6 +31,8 @@
 #include 
 #include 
 
+#define VHOST_USER_NET_TESTS_WORKING 0 /* broken as of 2.10.0 */
+
 /* GLIB version compatibility flags */
 #if !GLIB_CHECK_VERSION(2, 26, 0)
 #define G_TIME_SPAN_SECOND  (G_GINT64_CONSTANT(100))
@@ -472,11 +474,6 @@ static void test_server_listen(TestServer *server)
 test_server_create_chr(server, ",server,nowait");
 }
 
-static inline void test_server_connect(TestServer *server)
-{
-test_server_create_chr(server, ",reconnect=1");
-}
-
 #define GET_QEMU_CMD(s) \
 g_strdup_printf(QEMU_CMD, 512, 512, (root), (s)->chr_name,  \
 (s)->socket_path, "", (s)->chr_name)
@@ -722,7 +719,12 @@ static void wait_for_rings_started(TestServer *s, size_t 
count)
 g_mutex_unlock(>data_mutex);
 }
 
-#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
+#if VHOST_USER_NET_TESTS_WORKING && defined(CONFIG_HAS_GLIB_SUBPROCESS_TESTS)
+static inline void test_server_connect(TestServer *server)
+{
+test_server_create_chr(server, ",reconnect=1");
+}
+
 static gboolean
 reconnect_cb(gpointer user_data)
 {
@@ -962,7 +964,8 @@ int main(int argc, char **argv)
 qtest_add_data_func("/vhost-user/read-guest-mem", server, read_guest_mem);
 qtest_add_func("/vhost-user/migrate", test_migrate);
 qtest_add_func("/vhost-user/multiqueue", test_multiqueue);
-#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
+
+#if VHOST_USER_NET_TESTS_WORKING && defined(CONFIG_HAS_GLIB_SUBPROCESS_TESTS)
 qtest_add_func("/vhost-user/reconnect/subprocess",
test_reconnect_subprocess);
 qtest_add_func("/vhost-user/reconnect", test_reconnect);
-- 
2.14.1

[Qemu-devel] [Bug 1715203] [NEW] Maintain Haiku support

[kallisti5]

Public bug reported:

It was pointed out that the 2.10 release notes are pushing to drop Haiku
support.  The qemu port is currently working as-is under Haiku.

Was there a reason this was recommended? Is there anything Haiku can do
to keep it from being dropped?

We're working on a docker container to cross-compile rust-lang for
Haiku, could this be of some use to qemu when complete?

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

Title:
  Maintain Haiku support

Status in QEMU:
  New

Bug description:
  It was pointed out that the 2.10 release notes are pushing to drop
  Haiku support.  The qemu port is currently working as-is under Haiku.

  Was there a reason this was recommended? Is there anything Haiku can
  do to keep it from being dropped?

  We're working on a docker container to cross-compile rust-lang for
  Haiku, could this be of some use to qemu when complete?

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

Re: [Qemu-devel] [PATCH v10 6/6] fsdev: hmp interface for throttling

[Dr. David Alan Gilbert]

* Pradeep Jagadeesh (pradeepkiruv...@gmail.com) wrote:
> This patch introduces hmp interfaces for the fsdev
> devices.
> 
> Signed-off-by: Pradeep Jagadeesh 
> ---
>  hmp-commands-info.hx | 18 
>  hmp-commands.hx  | 19 +
>  hmp.c| 60 
> 
>  hmp.h|  4 
>  4 files changed, 101 insertions(+)
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index d9df238..07ed338 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -84,6 +84,24 @@ STEXI
>  Show block device statistics.
>  ETEXI
>  
> +#if defined(CONFIG_VIRTFS)
> +
> +{
> +.name   = "fsdev_iothrottle",
> +.args_type  = "",
> +.params = "",
> +.help   = "show fsdev iothrottle information",
> +.cmd= hmp_info_fsdev_iothrottle,
> +},
> +
> +#endif
> +
> +STEXI
> +@item info fsdev_iothrottle
> +@findex fsdev_iothrottle
> +Show fsdev device throttle info.
> +ETEXI
> +
>  {
>  .name   = "block-jobs",
>  .args_type  = "",
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 1941e19..aef9f79 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1680,6 +1680,25 @@ STEXI
>  Change I/O throttle limits for a block drive to @var{bps} @var{bps_rd} 
> @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
>  ETEXI
>  
> +#if defined(CONFIG_VIRTFS)
> +
> +{
> +.name   = "fsdev_set_io_throttle",
> +.args_type  = 
> "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l",
> +.params = "device bps bps_rd bps_wr iops iops_rd iops_wr",
> +.help   = "change I/O throttle limits for a fs devices",
> +.cmd= hmp_fsdev_set_io_throttle,
> +},
> +
> +#endif
> +
> +STEXI
> +@item fsdev_set_io_throttle @var{device} @var{bps} @var{bps_rd} @var{bps_wr} 
> @var{iops} @var{iops_rd} @var{iops_wr}
> +@findex fsdev_set_io_throttle
> +Change I/O throttle limits for a fs devices to @var{bps} @var{bps_rd} 
> @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
> +ETEXI

I'm OK with this, and I see it's a copy of the equivalent
block_set_io_throttle, but can you clarify the meaning - for
example if a value is 0 what does it mean? Does it mean
there's no throttle set for that variable?
Also is the 'bps' bytes/second (as opposed to bits or blocks).
Clarifying both of these in the text would be good.

Dave

> +
> +
>  {
>  .name   = "set_password",
>  .args_type  = "protocol:s,password:s,connected:s?",
> diff --git a/hmp.c b/hmp.c
> index 2dbfb80..6e63cf1 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1783,6 +1783,66 @@ void hmp_block_set_io_throttle(Monitor *mon, const 
> QDict *qdict)
>  hmp_handle_error(mon, );
>  }
>  
> +#ifdef CONFIG_VIRTFS
> +
> +void hmp_fsdev_set_io_throttle(Monitor *mon, const QDict *qdict)
> +{
> +Error *err = NULL;
> +IOThrottle throttle = {
> +.has_id = true,
> +.id = (char *) qdict_get_str(qdict, "device"),
> +};
> +
> +hmp_initialize_io_throttle(, qdict);
> +qmp_fsdev_set_io_throttle(, );
> +hmp_handle_error(mon, );
> +}
> +
> +static void print_fsdev_throttle_config(Monitor *mon, IOThrottle *fscfg)
> +{
> +monitor_printf(mon, "%s", fscfg->id);
> +monitor_printf(mon, "I/O throttling:"
> +   " bps=%" PRId64
> +   " bps_rd=%" PRId64  " bps_wr=%" PRId64
> +   " bps_max=%" PRId64
> +   " bps_rd_max=%" PRId64
> +   " bps_wr_max=%" PRId64
> +   " iops=%" PRId64 " iops_rd=%" PRId64
> +   " iops_wr=%" PRId64
> +   " iops_max=%" PRId64
> +   " iops_rd_max=%" PRId64
> +   " iops_wr_max=%" PRId64
> +   " iops_size=%" PRId64
> +   "\n",
> +   fscfg->bps,
> +   fscfg->bps_rd,
> +   fscfg->bps_wr,
> +   fscfg->bps_max,
> +   fscfg->bps_rd_max,
> +   fscfg->bps_wr_max,
> +   fscfg->iops,
> +   fscfg->iops_rd,
> +   fscfg->iops_wr,
> +   fscfg->iops_max,
> +   fscfg->iops_rd_max,
> +   fscfg->iops_wr_max,
> +   fscfg->iops_size);
> +}
> +
> +void hmp_info_fsdev_iothrottle(Monitor *mon, const QDict *qdict)
> +{
> +Error *err = NULL;
> +IOThrottleList *fsdev_list, *info;
> +fsdev_list = qmp_query_fsdev_io_throttle();
> +
> +for (info = fsdev_list; info; info = info->next) {
> +print_fsdev_throttle_config(mon, info->value);
> +}
> +qapi_free_IOThrottleList(fsdev_list);
> +}
> +
> +#endif
> +
>  void hmp_block_stream(Monitor *mon, const QDict *qdict)
>  {
>  Error *error = NULL;
> diff --git a/hmp.h b/hmp.h
> index 1ff4552..d700d7d 100644
> 

Re: [Qemu-devel] Questions regarding emulated UART in VersatilePB board

[Ramy Sameh]

Thanks Peter for your help.

Are there any documentation or source of information, that can describe how
interrupts are implemented, and how to use them (where is the vector table
to put the ISR ... etc) ?
In addition, any source of info that describes the code workflow when
reading or writing to the uart would be very helpful.
(because I put printfs in the functions to understand the workflow, but the
order of these printfs made me very confused).

Regarding the registers, I have checked pl011_read and pl011_write, and
found them, thanks for that.
But I couldn't find the register "UARTICR" (*Interrupt clear register*)
I think this might be related to my lack of understanding of how interrupts
are implemented in emulated pl011.

Regarding the ID registers, whose values are hard-wired in the board, and
there was no need to implement modifiable states for them.
Do you mean *Peripheral identification registers, UARTPeriphID0-3* and
*PrimeCell
identification registers, UARTPCellID0-3* ?

Thanks in advance for your help.


On Mon, Sep 4, 2017 at 10:50 AM, Peter Maydell 
wrote:

> On 4 September 2017 at 01:27, Ramy Sameh  wrote:
> > *My 2 questions are:*
> >
> > *First:*
> > Are interrupts activated in the emulated pl011 ?
> > I mean, if I enabled the interrupt bits for UARTTXINTR, will this trigger
> > an interrupt when the FIFO reaches a certain level?
>
> Yes, we implement interrupts.
>
> > *Second:*
> > Another problem is that I can't find some of the UART registers (which
> are
> > in the data sheet PrimeCell UART (PL011)), in QEMU's emulated UART pl011.
> >
> > *These are the registers which I can't find their matches in the
> structure
> > PL011State:*
> >
> > 1.) Interrupt FIFO level select register, UARTIFLS
> > 2.) Raw interrupt status register, UARTRIS
> > 3.) Masked interrupt status register, UARTMIS
> > 4.) Interrupt clear register, UARTICR
> > 5.) Peripheral identification registers, UARTPeriphID0-3
> > 6.) PrimeCell identification registers, UARTPCellID0-3
>
> All these registers are implemented. You can find the code that
> implements them in the pl011_read() and pl011_write() functions.
>
> > *This is the structure where I get pl011 emulated registers:*
> > typedef struct PL011State
>
> This structure defines (among other things) fields which
> hold the underlying state of the device (in hardware terms,
> usually information which is in a flipflop or otherwise
> stored). This is not the same as a guest-visible register,
> although there is very often overlap. For instance, the
> ID registers are constant and read-only, so they do not
> have any modifiable state that we need to put in the struct
> (in hardware, they'll be implemented by just tying off
> lines to 0 or 1, not with flops). Sometimes a register
> value as seen by the guest is just a logical combination
> of state used by other registers, eg UARTMIS is just the
> logical OR of UARTRIS and UARTIMSC, so it doesn't need
> any extra state in the struct.
>
> Some of the state fields have names that don't correspond
> to the register names for historical reasons (eg UARTIFLS
> is in s->ifl); you should always start with the read and
> write functions to look at how the register behaviour
> is implemented, which will show you which state fields
> if any are involved.
>
> thanks
> -- PMM
>



-- 
Best Regards,
Ramy Sameh
Embedded Software Engineer
+2-010-172-777-14

Re: [Qemu-devel] [PULL 24/29] kvm: use DIV_ROUND_UP

[Peter Maydell]

On 31 August 2017 at 11:34, Marc-André Lureau
 wrote:
> I used the clang-tidy qemu-round check to generate the fix:
> https://github.com/elmarco/clang-tools-extra
>
> Signed-off-by: Marc-André Lureau 
> Reviewed-by: Richard Henderson 
> ---
>  linux-headers/asm-x86/kvm.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/linux-headers/asm-x86/kvm.h b/linux-headers/asm-x86/kvm.h
> index c2824d02ba..1930b95bcb 100644
> --- a/linux-headers/asm-x86/kvm.h
> +++ b/linux-headers/asm-x86/kvm.h
> @@ -153,7 +153,7 @@ struct kvm_sregs {
> __u64 cr0, cr2, cr3, cr4, cr8;
> __u64 efer;
> __u64 apic_base;
> -   __u64 interrupt_bitmap[(KVM_NR_INTERRUPTS + 63) / 64];
> +   __u64 interrupt_bitmap[DIV_ROUND_UP(KVM_NR_INTERRUPTS, 64)];
>  };
>
>  /* for KVM_GET_FPU and KVM_SET_FPU */

Hi -- I just noticed that this commit makes a change to linux-headers/.
This directory is for kernel headers which we keep in sync with the
upstream kernel via scripts/update-linux-headers.sh, so we shouldn't
be applying our code cleanups to it.

I'm going to revert it (206a0fc75d5f54886c1b3f3a65782a75e36b6b97).

thanks
-- PMM

Re: [Qemu-devel] [PATCH v11 6/6] target-arm: kvm64: Handle SError interrupt for the guest OS

[Peter Maydell]

On 18 August 2017 at 15:23, Dongjiu Geng  wrote:
> When guest OS happens SError interrupt(SEI), it will trap to host.
> Host firstly calls memory failure to deal with this error and decide
> whether it needs to deliver SIGBUS signal to userspace. The advantage
> that using signal to notify is that it can make the notification method
> is general, non-KVM user can also use it. when userspace gets this
> signal and knows this is SError interrupt, it will translate the
> delivered host VA to PA and record this PA to GHES.
>
> Because ARMv8.2 adds an extension to RAS to allow system software insert
> implicit Error Synchronization Barrier operations to isolate the error and
> allow passes specified syndrome to guest OS, so after record the CPER, user
> space calls IOCTL to pass a specified syndrome to KVM, then switch to guest
> OS, guest OS can use the recorded CPER record and syndrome information to
> do the recovery.
>
> The steps are shown below:
> 1. translate the host VA to guest OS PA and record this error PA to HEST 
> table.
> 2. set specified virtual SError syndrome and pass the value to KVM.
>
> Signed-off-by: Dongjiu Geng 
> Signed-off-by: Quanming Wu 
> ---
>  linux-headers/linux/kvm.h |  1 +
>  target/arm/internals.h|  1 +
>  target/arm/kvm64.c| 28 
>  3 files changed, 30 insertions(+)
>
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index 2aa176e..10dfcab 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -1356,6 +1356,7 @@ struct kvm_s390_ucas_mapping {
>  /* Available with KVM_CAP_S390_CMMA_MIGRATION */
>  #define KVM_S390_GET_CMMA_BITS  _IOWR(KVMIO, 0xb8, struct 
> kvm_s390_cmma_log)
>  #define KVM_S390_SET_CMMA_BITS  _IOW(KVMIO, 0xb9, struct 
> kvm_s390_cmma_log)
> +#define KVM_ARM_SEI _IO(KVMIO,   0xb10)
>
>  #define KVM_DEV_ASSIGN_ENABLE_IOMMU(1 << 0)
>  #define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index fc0ad6d..18b1cbc 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -237,6 +237,7 @@ enum arm_exception_class {
>  #define ARM_EL_ISV (1 << ARM_EL_ISV_SHIFT)
>  #define ARM_EL_EC_MASK  ((0x3F) << ARM_EL_EC_SHIFT)
>  #define ARM_EL_FSC_TYPE (0x3C)
> +#define ARM_EL_ISS_MASK ((1 << ARM_EL_IL_SHIFT) - 1)
>
>  #define FSC_SEA (0x10)
>  #define FSC_SEA_TTW0(0x14)
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index d3bdab2..b84cb49 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -616,6 +616,22 @@ static int kvm_arm_cpreg_value(ARMCPU *cpu, ptrdiff_t 
> fieldoffset)
>  return -EINVAL;
>  }
>
> +static int kvm_inject_arm_sei(CPUState *cs)
> +{
> +ARMCPU *cpu = ARM_CPU(cs);
> +CPUARMState *env = >env;
> +
> +unsigned long syndrome = env->exception.vaddress;
> +/* set virtual SError syndrome */
> +if (arm_feature(env, ARM_FEATURE_RAS_EXTENSION)) {
> +syndrome = syndrome & ARM_EL_ISS_MASK;
> +} else {
> +syndrome = 0;
> +}
> +
> +return  kvm_vcpu_ioctl(CPU(cpu), KVM_ARM_SEI, );

This looks odd. If we don't have the RAS extension why do
we need to do anything at all here ?

> +}
> +
>  /* Inject synchronous external abort */
>  static int kvm_inject_arm_sea(CPUState *c)
>  {
> @@ -1007,6 +1023,15 @@ static bool is_abort_sea(unsigned long syndrome)
>  }
>  }
>
> +static bool is_abort_sei(unsigned long syndrome)
> +{
> +uint8_t ec = ((syndrome & ARM_EL_EC_MASK) >> ARM_EL_EC_SHIFT);

You don't need to bother masking here -- in other places in
QEMU we assume that the EC field is at the top of the word,
so just "syndrome >> ARM_EL_EC_SHIFT" is sufficient.

> +if ((ec != EC_SERROR))
> +return false;
> +else
> +return true;

scripts/checkpatch.pl should tell you that this if needs braces
(it's good to get in the habit of running it on all patches; it
is not always correct, so judgement is required, but it will flag
up some common mistakes).

In this particular case, you should just
   return ec == EC_SERROR;
though.

> +}
> +
>  void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
>  {
>  ram_addr_t ram_addr;
> @@ -1024,6 +1049,9 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, 
> void *addr)
>  if (is_abort_sea(env->exception.syndrome)) {
>  ghes_update_guest(ACPI_HEST_NOTIFY_SEA, paddr);
>  kvm_inject_arm_sea(c);
> +} else if (is_abort_sei(env->exception.syndrome)) {
> +ghes_update_guest(ACPI_HEST_NOTIFY_SEI, paddr);
> +kvm_inject_arm_sei(c);
>  }
>  return;
>  }
> --
> 1.8.3.1

thanks
-- PMM

[Qemu-devel] [PATCH] multiboot: validate multiboot header address values

[P J P]

From: Prasad J Pandit 

While loading kernel via multiboot-v1 image, (flags & 0x0001)
indicates that multiboot header contains valid addresses to load
the kernel image. These addresses are used to compute kernel
size and kernel text offset in the OS image. Validate these
address values to avoid an OOB access issue.

Reported-by: Thomas Garnier 
Signed-off-by: Prasad J Pandit 
---
 hw/i386/multiboot.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index 6001f4caa2..c7b70c91d5 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -221,15 +221,34 @@ int load_multiboot(FWCfgState *fw_cfg,
 uint32_t mh_header_addr = ldl_p(header+i+12);
 uint32_t mh_load_end_addr = ldl_p(header+i+20);
 uint32_t mh_bss_end_addr = ldl_p(header+i+24);
+
 mh_load_addr = ldl_p(header+i+16);
+if (mh_header_addr < mh_load_addr) {
+fprintf(stderr, "invalid mh_load_addr address\n");
+exit(1);
+}
+
 uint32_t mb_kernel_text_offset = i - (mh_header_addr - mh_load_addr);
 uint32_t mb_load_size = 0;
 mh_entry_addr = ldl_p(header+i+28);
 
 if (mh_load_end_addr) {
+if (mh_bss_end_addr < mh_load_addr) {
+fprintf(stderr, "invalid mh_bss_end_addr address\n");
+exit(1);
+}
 mb_kernel_size = mh_bss_end_addr - mh_load_addr;
+
+if (mh_load_end_addr < mh_load_addr) {
+fprintf(stderr, "invalid mh_load_end_addr address\n");
+exit(1);
+}
 mb_load_size = mh_load_end_addr - mh_load_addr;
 } else {
+if (kernel_file_size < mb_kernel_text_offset) {
+fprintf(stderr, "invalid kernel_file_size\n");
+exit(1);
+}
 mb_kernel_size = kernel_file_size - mb_kernel_text_offset;
 mb_load_size = mb_kernel_size;
 }
-- 
2.13.5

Re: [Qemu-devel] [PATCH v3 3/7] Convert single line fprintf(.../n) to warn_report()

[Alistair Francis]

On Fri, Sep 1, 2017 at 1:52 PM, James Hogan  wrote:
> On Fri, Sep 01, 2017 at 09:51:15AM -0700, Alistair Francis wrote:
>> diff --git a/target/mips/kvm.c b/target/mips/kvm.c
>> index 3317905e71..a23aa438d2 100644
>> --- a/target/mips/kvm.c
>> +++ b/target/mips/kvm.c
>> @@ -95,11 +95,11 @@ void kvm_mips_reset_vcpu(MIPSCPU *cpu)
>>  CPUMIPSState *env = >env;
>>
>>  if (!kvm_mips_fpu_cap && env->CP0_Config1 & (1 << CP0C1_FP)) {
>> -fprintf(stderr, "Warning: KVM does not support FPU, disabling\n");
>> +warn_report("KVM does not support FPU, disabling");
>>  env->CP0_Config1 &= ~(1 << CP0C1_FP);
>>  }
>>  if (!kvm_mips_msa_cap && env->CP0_Config3 & (1 << CP0C3_MSAP)) {
>> -fprintf(stderr, "Warning: KVM does not support MSA, disabling\n");
>> +warn_report("KVM does not support MSA, disabling");
>>  env->CP0_Config3 &= ~(1 << CP0C3_MSAP);
>>  }
>
> That looks sensible.
> Reviewed-by: James Hogan  [mips]

Thanks

>
> The 3 fprintfs in kvm_mips_update_state() are also pretty much non-fatal
> warnings too (though only because old kernels v3.10-v3.15 didn't
> implement the timer saving API). Feel free to convert those too if you
> like, otherwise I'll submit along with some other changes.

Ok, I'll add an extra patch to this series which converts those.

Thanks,
Alistair

>
> Cheers
> James

Re: [Qemu-devel] [PATCH v11 5/6] target-arm: kvm64: handle SIGBUS signal for synchronous External Abort

[Peter Maydell]

On 18 August 2017 at 15:23, Dongjiu Geng  wrote:
> Add SIGBUS signal handler. In this handler, it checks
> the exception type, translates the host VA which is
> delivered by host or KVM to guest PA, then fills this
> PA to CPER, finally injects a Error to guest OS through
> KVM.
>
> Add synchronous external abort injection logic, setup
> spsr_elx, esr_elx, PSTATE, far_elx, elr_elx etc, when
> switch to guest OS, it will jump to the synchronous
> external abort vector table entry.
>
> Signed-off-by: Dongjiu Geng 
> Signed-off-by: Quanming Wu 
> ---
>  include/sysemu/kvm.h  |   2 +-
>  linux-headers/asm-arm64/kvm.h |   5 ++
>  target/arm/internals.h|  13 
>  target/arm/kvm.c  |  34 ++
>  target/arm/kvm64.c| 150 
> ++
>  target/arm/kvm_arm.h  |   1 +
>  6 files changed, 204 insertions(+), 1 deletion(-)

Have you tested whether this patchset builds OK on aarch32 ?

> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 3a458f5..90c1605 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -361,7 +361,7 @@ bool kvm_vcpu_id_is_valid(int vcpu_id);
>  /* Returns VCPU ID to be used on KVM_CREATE_VCPU ioctl() */
>  unsigned long kvm_arch_vcpu_id(CPUState *cpu);
>
> -#ifdef TARGET_I386
> +#if defined(TARGET_I386) || defined(TARGET_AARCH64)
>  #define KVM_HAVE_MCE_INJECTION 1
>  void kvm_arch_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
>  #endif
> diff --git a/linux-headers/asm-arm64/kvm.h b/linux-headers/asm-arm64/kvm.h
> index d254700..5909c30 100644
> --- a/linux-headers/asm-arm64/kvm.h
> +++ b/linux-headers/asm-arm64/kvm.h
> @@ -181,6 +181,11 @@ struct kvm_arch_memory_slot {
>  #define KVM_REG_ARM64_SYSREG_OP2_MASK  0x0007
>  #define KVM_REG_ARM64_SYSREG_OP2_SHIFT 0
>
> +/* AArch64 fault registers */
> +#define KVM_REG_ARM64_FAULT (0x0014 << KVM_REG_ARM_COPROC_SHIFT)
> +#define KVM_REG_ARM64_FAULT_ESR_EC  (0)
> +#define KVM_REG_ARM64_FAULT_FAR (1)
> +
>  #define ARM64_SYS_REG_SHIFT_MASK(x,n) \
> (((x) << KVM_REG_ARM64_SYSREG_ ## n ## _SHIFT) & \
> KVM_REG_ARM64_SYSREG_ ## n ## _MASK)

Again, linux-headers changes need to go in their own header sync patch.

> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index 1f6efef..fc0ad6d 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -235,6 +235,19 @@ enum arm_exception_class {
>  #define ARM_EL_ISV_SHIFT 24
>  #define ARM_EL_IL (1 << ARM_EL_IL_SHIFT)
>  #define ARM_EL_ISV (1 << ARM_EL_ISV_SHIFT)
> +#define ARM_EL_EC_MASK  ((0x3F) << ARM_EL_EC_SHIFT)
> +#define ARM_EL_FSC_TYPE (0x3C)
> +
> +#define FSC_SEA (0x10)
> +#define FSC_SEA_TTW0(0x14)
> +#define FSC_SEA_TTW1(0x15)
> +#define FSC_SEA_TTW2(0x16)
> +#define FSC_SEA_TTW3(0x17)
> +#define FSC_SECC(0x18)
> +#define FSC_SECC_TTW0   (0x1c)
> +#define FSC_SECC_TTW1   (0x1d)
> +#define FSC_SECC_TTW2   (0x1e)
> +#define FSC_SECC_TTW3   (0x1f)
>
>  /* Utility functions for constructing various kinds of syndrome value.
>   * Note that in general we follow the AArch64 syndrome values; in a
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 7c17f0d..2e1716a 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -129,6 +129,39 @@ void kvm_arm_destroy_scratch_host_vcpu(int *fdarray)
>  }
>  }
>
> +typedef struct HWPoisonPage {
> +ram_addr_t ram_addr;
> +QLIST_ENTRY(HWPoisonPage) list;
> +} HWPoisonPage;
> +
> +static QLIST_HEAD(, HWPoisonPage) hwpoison_page_list =
> +QLIST_HEAD_INITIALIZER(hwpoison_page_list);
> +
> +static void kvm_unpoison_all(void *param)
> +{
> +HWPoisonPage *page, *next_page;
> +
> +QLIST_FOREACH_SAFE(page, _page_list, list, next_page) {
> +QLIST_REMOVE(page, list);
> +qemu_ram_remap(page->ram_addr, TARGET_PAGE_SIZE);
> +g_free(page);
> +}
> +}
> +
> +void kvm_hwpoison_page_add(ram_addr_t ram_addr)
> +{
> +HWPoisonPage *page;
> +
> +QLIST_FOREACH(page, _page_list, list) {
> +if (page->ram_addr == ram_addr) {
> +return;
> +}
> +}
> +page = g_new(HWPoisonPage, 1);
> +page->ram_addr = ram_addr;
> +QLIST_INSERT_HEAD(_page_list, page, list);
> +}

This code has all just been copied-and-pasted from target/i386/kvm.c.
Please instead abstract it out properly into a cpu-independent
source file.

>  static void kvm_arm_host_cpu_class_init(ObjectClass *oc, void *data)
>  {
>  ARMHostCPUClass *ahcc = ARM_HOST_CPU_CLASS(oc);
> @@ -182,6 +215,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>
>  cap_has_mp_state = kvm_check_extension(s, KVM_CAP_MP_STATE);
>
> +qemu_register_reset(kvm_unpoison_all, NULL);
>  type_register_static(_arm_cpu_type_info);
>
>  return 0;
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 0781367..d3bdab2 100644
> --- 

Re: [Qemu-devel] [PATCH v2 28/54] qapi: do not define enumeration value explicitely

[Markus Armbruster]

Marc-André Lureau  writes:

> The C standard has the initial value at 0 and the subsequent values
> incremented by 1. No need to set this explicitely.
>
> This will prevent from artificial "gaps" when compiling out some enum
> values and having unnecessarily large MAX values & enums arrays.

Yes, but it also risks entertaining mishaps like compiling this one

typedef enum Color {
COLOR_WHITE,
#if defined(NEED_CPU_H)
#if defined(TARGET_S390X)
COLOR_BLUE,
#endif /* defined(TARGET_S390X) */
#endif /* defined(NEED_CPU_H) */
COLOR_BLACK,
} Color;

in s390x-code (COLOR_BLACK = 2) and in target-independent code
(COLOR_BLACK = 1), then linking the two together.

> Signed-off-by: Marc-André Lureau 
> ---
>  scripts/qapi.py | 7 ++-
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 52099332f1..9d075440d3 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -1979,14 +1979,11 @@ typedef enum %(c_name)s {
>  ''',
>  c_name=c_name(name))
>  
> -i = 0
>  for value in enum_values:
>  ret += mcgen('''
> -%(c_enum)s = %(i)d,
> +%(c_enum)s,
>  ''',
> - c_enum=c_enum_const(name, value, prefix),
> - i=i)
> -i += 1
> + c_enum=c_enum_const(name, value, prefix))
>  
>  ret += mcgen('''
>  } %(c_name)s;

Re: [Qemu-devel] [PATCH v2 2/6] xlnx-zynqmp-pmu: Add the CPU and memory

[Alistair Francis]

On Mon, Sep 4, 2017 at 12:32 AM, Igor Mammedov  wrote:
> On Fri, 1 Sep 2017 14:00:37 -0700
> Alistair Francis  wrote:
>
>> Connect the MicroBlaze CPU and the ROM and RAM memory regions.
>>
>> Signed-off-by: Alistair Francis 
>> ---
>>
>>  hw/microblaze/xlnx-zynqmp-pmu.c | 65 
>> +++--
>>  1 file changed, 63 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/microblaze/xlnx-zynqmp-pmu.c 
>> b/hw/microblaze/xlnx-zynqmp-pmu.c
>> index fc3c8b236f..33584cfa4d 100644
>> --- a/hw/microblaze/xlnx-zynqmp-pmu.c
>> +++ b/hw/microblaze/xlnx-zynqmp-pmu.c
>> @@ -18,8 +18,11 @@
>>  #include "qemu/osdep.h"
>>  #include "qapi/error.h"
>>  #include "qemu-common.h"
>> +#include "exec/address-spaces.h"
>>  #include "hw/boards.h"
>> +#include "hw/qdev-properties.h"
>>  #include "cpu.h"
>> +#include "boot.h"
>>
>>  /* Define the PMU device */
>>
>> @@ -27,21 +30,51 @@
>>  #define XLNX_ZYNQMP_PMU(obj) OBJECT_CHECK(XlnxZynqMPPMUState, (obj), \
>>TYPE_XLNX_ZYNQMP_PMU)
>>
>> +#define XLNX_ZYNQMP_PMU_ROM_SIZE0x8000
>> +#define XLNX_ZYNQMP_PMU_ROM_ADDR0xFFD0
>> +#define XLNX_ZYNQMP_PMU_RAM_ADDR0xFFDC
>> +
>>  typedef struct XlnxZynqMPPMUState {
>>  /*< private >*/
>>  DeviceState parent_obj;
>>
>>  /*< public >*/
>> +MicroBlazeCPU cpu;
>>  }  XlnxZynqMPPMUState;
>>
>>  static void xlnx_zynqmp_pmu_init(Object *obj)
>>  {
>> +XlnxZynqMPPMUState *s = XLNX_ZYNQMP_PMU(obj);
>>
>> +object_initialize(>cpu, sizeof(s->cpu),
>> +  TYPE_MICROBLAZE_CPU);
>> +object_property_add_child(obj, "pmu-cpu[*]", OBJECT(>cpu),
>   ^^^ why do you use this syntax 
> here?
>

Woops, that was a left over from the ARM ZynqMP. I'll remove the '[*]'

>> +  _abort);
>>  }
>>
>>  static void xlnx_zynqmp_pmu_realize(DeviceState *dev, Error **errp)
>>  {
>> -
>> +XlnxZynqMPPMUState *s = XLNX_ZYNQMP_PMU(dev);
>> +
>> +object_property_set_uint(OBJECT(>cpu), XLNX_ZYNQMP_PMU_ROM_ADDR,
>> + "base-vectors", _abort);
>> +object_property_set_bool(OBJECT(>cpu), true, "use-stack-protection",
>> + _abort);
>> +object_property_set_uint(OBJECT(>cpu), 0, "use-fpu", _abort);
>> +object_property_set_uint(OBJECT(>cpu), 0, "use-hw-mul", 
>> _abort);
>> +object_property_set_bool(OBJECT(>cpu), true, "use-barrel",
>> + _abort);
>> +object_property_set_bool(OBJECT(>cpu), true, "use-msr-instr",
>> + _abort);
>> +object_property_set_bool(OBJECT(>cpu), true, "use-pcmp-instr",
>> + _abort);
>> +object_property_set_bool(OBJECT(>cpu), false, "use-mmu", 
>> _abort);
>> +object_property_set_bool(OBJECT(>cpu), true, "endianness",
>> + _abort);
>> +object_property_set_str(OBJECT(>cpu), "8.40.b", "version",
>> +_abort);
>> +object_property_set_uint(OBJECT(>cpu), 0, "pvr", _abort);
>> +object_property_set_bool(OBJECT(>cpu), true, "realized", 
>> _fatal);
> I'd replace error_fatal here with errp

Will do.

Thanks,
Alistair

>
>>  }
>>
>>  static void xlnx_zynqmp_pmu_class_init(ObjectClass *oc, void *data)
>> @@ -70,7 +103,35 @@ type_init(xlnx_zynqmp_pmu_register_types)
>>
>>  static void xlnx_zcu102_pmu_init(MachineState *machine)
>>  {
>> -
>> +XlnxZynqMPPMUState *pmu = g_new0(XlnxZynqMPPMUState, 1);
>> +MemoryRegion *address_space_mem = get_system_memory();
>> +MemoryRegion *pmu_rom = g_new(MemoryRegion, 1);
>> +MemoryRegion *pmu_ram = g_new(MemoryRegion, 1);
>> +
>> +/* Create the ROM */
>> +memory_region_init_rom(pmu_rom, NULL, "xlnx-zcu102-pmu.rom",
>> +   XLNX_ZYNQMP_PMU_ROM_SIZE, _fatal);
>> +memory_region_add_subregion(address_space_mem, XLNX_ZYNQMP_PMU_ROM_ADDR,
>> +pmu_rom);
>> +
>> +/* Create the RAM */
>> +memory_region_init_ram(pmu_ram, NULL, "xlnx-zcu102-pmu.ram",
>> +   machine->ram_size, _fatal);
>> +memory_region_add_subregion(address_space_mem, XLNX_ZYNQMP_PMU_RAM_ADDR,
>> +pmu_ram);
>> +
>> +/* Create the PMU device */
>> +object_initialize(pmu, sizeof(XlnxZynqMPPMUState), 
>> TYPE_XLNX_ZYNQMP_PMU);
>> +object_property_add_child(OBJECT(machine), "pmu", OBJECT(pmu),
>> +  _abort);
>> +object_property_set_bool(OBJECT(pmu), true, "realized", _fatal);
>> +
>> +/* Load the kernel */
>> +microblaze_load_kernel(>cpu, XLNX_ZYNQMP_PMU_RAM_ADDR,
>> +   machine->ram_size,
>> +   machine->kernel_filename,
>> +   machine->dtb,
>> +   NULL);
>>  }
>>
>>  static 

Re: [Qemu-devel] [PATCH v3 1/7] hw/i386: Improve some of the warning messages

[Alistair Francis]

On Mon, Sep 4, 2017 at 7:17 AM, Eduardo Habkost  wrote:
> On Fri, Sep 01, 2017 at 09:51:05AM -0700, Alistair Francis wrote:
> [...]
>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>> index 169a214d50..435eb2c458 100644
>> --- a/hw/i386/pc_q35.c
>> +++ b/hw/i386/pc_q35.c
>> @@ -101,9 +101,11 @@ static void pc_q35_init(MachineState *machine)
>>  lowmem = pcms->max_ram_below_4g;
>>  if (machine->ram_size - lowmem > lowmem &&
>>  lowmem & ((1ULL << 30) - 1)) {
>> -warn_report("Large machine and max_ram_below_4g(%"PRIu64
>> -") not a multiple of 1G; possible bad performance.",
>> -pcms->max_ram_below_4g);
>> +warn_report("Large machine as the ram size (0x%" PRIx64 ") is 
>> more"
>> +" then twice the size of the internal limit"
>> +" (0x%" PRIx64 ") and max-ram-below-4g (%"PRIu64")"
>> +" note a multiple of 1G; possible bad performance.",
>> +machine->ram_size, lowmem, pcms->max_ram_below_4g);
>
> Here lowmem and max_ram_below_4g have exactly the same value,
> don't they?  There's no internal limit involved in this logic,
> only max_ram_below_4g and ram_size.

Good point, I'll change it to this:

warn_report("There is possibly poor performance as the ram size "
" (0x%" PRIx64 ") is more then twice the size of"
" max-ram-below-4g (%"PRIu64") and"
" max-ram-below-4g is not a multiple of 1G.",
machine->ram_size, pcms->max_ram_below_4g);

Thanks,
Alistair

>
> --
> Eduardo

[Qemu-devel] [PATCH v2] vhost-user: disable broken test

[Philippe Mathieu-Daudé]

tests/vhost-user-test keeps failing on build-system since Aug 15:

  ERROR:tests/vhost-user-test.c:835:test_flags_mismatch: child process 
(/i386/vhost-user/flags-mismatch/subprocess [4836]) failed unexpectedly
...
  ERROR:tests/vhost-user-test.c:807:test_connect_fail: child process 
(/x86_64/vhost-user/connect-fail/subprocess [58910]) failed unexpectedly

The test can still be build for debug session using:

  $ FORCE_VHOST_USER_NET_TEST=1 ./configure ...

Suggested-by: Peter Maydell 
Signed-off-by: Philippe Mathieu-Daudé 
---
v2: remove unuseful warning

 configure | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index fb7e34a901..921b3e1cff 100755
--- a/configure
+++ b/configure
@@ -6386,7 +6386,11 @@ if supported_kvm_target $target; then
 if test "$vhost_net" = "yes" ; then
 echo "CONFIG_VHOST_NET=y" >> $config_target_mak
 if test "$vhost_user" = "yes" ; then
-echo "CONFIG_VHOST_USER_NET_TEST_$target_name=y" >> 
$config_host_mak
+# test currently broken (v2.10.0) but can still be forced using:
+#   FORCE_VHOST_USER_NET_TEST=1 ./configure [...]
+if test -n "${FORCE_VHOST_USER_NET_TEST}"; then
+echo "CONFIG_VHOST_USER_NET_TEST_$target_name=y" >> 
$config_host_mak
+fi
 fi
 fi
 fi
-- 
2.14.1

Re: [Qemu-devel] [PATCH v11 4/6] target-arm: kvm64: detect guest RAS EXTENSION feature

[Peter Maydell]

On 18 August 2017 at 15:23, Dongjiu Geng  wrote:
> check if kvm supports guest RAS EXTENSION. if so, set
> corresponding feature bit for vcpu.
>
> Signed-off-by: Dongjiu Geng 
> ---
>  linux-headers/linux/kvm.h | 1 +
>  target/arm/cpu.h  | 3 +++
>  target/arm/kvm64.c| 8 
>  3 files changed, 12 insertions(+)
>
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index 7971a4f..2aa176e 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -929,6 +929,7 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_CAP_PPC_SMT_POSSIBLE 147
>  #define KVM_CAP_HYPERV_SYNIC2 148
>  #define KVM_CAP_HYPERV_VP_INDEX 149
> +#define KVM_CAP_ARM_RAS_EXTENSION 150
>
>  #ifdef KVM_CAP_IRQ_ROUTING
>

Hi. Changes to linux-headers need to be done as a patch of their
own created using scripts/update-linux-headers.sh run against a
mainline kernel tree (and with a commit message that quotes the
kernel commit hash used). This ensures that we have a consistent
set of headers that don't diverge from the kernel copy.

> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index b39d64a..6b0961b 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -611,6 +611,8 @@ struct ARMCPU {
>
>  /* CPU has memory protection unit */
>  bool has_mpu;
> +/* CPU has ras extension unit */
> +bool has_ras_extension;
>  /* PMSAv7 MPU number of supported regions */
>  uint32_t pmsav7_dregion;
>
> @@ -1229,6 +1231,7 @@ enum arm_features {
>  ARM_FEATURE_THUMB_DSP, /* DSP insns supported in the Thumb encodings */
>  ARM_FEATURE_PMU, /* has PMU support */
>  ARM_FEATURE_VBAR, /* has cp15 VBAR */
> +ARM_FEATURE_RAS_EXTENSION, /*has RAS extension support */

Missing space after '/*' ?

>  };
>
>  static inline int arm_feature(CPUARMState *env, int feature)
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index a16abc8..0781367 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -518,6 +518,14 @@ int kvm_arch_init_vcpu(CPUState *cs)
>  unset_feature(>features, ARM_FEATURE_PMU);
>  }
>
> +if (kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_RAS_EXTENSION)) {
> +cpu->has_ras_extension = true;
> +set_feature(>features, ARM_FEATURE_RAS_EXTENSION);
> +} else {
> +cpu->has_ras_extension = false;
> +unset_feature(>features, ARM_FEATURE_RAS_EXTENSION);
> +}
> +

Shouldn't we need to also tell the kernel that we actually want
it to expose RAS to the guest? Compare the PMU code in this
function, where we set a kvm_init_features bit to do this.
(This suggests that your ABI for the kernel part of this feature
may not be correct?)

You should also not be calling set_feature() here -- if the
CPU features bit doesn't say "this CPU should have the RAS
extensions" we shouldn't create a CPU with them. Instead
you should set it in kvm_arm_get_host_cpu_features() (again,
compare the PMU code).

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/9] s390x: fix invalid use of cc 1 for SSCH

[Halil Pasic]

On 09/05/2017 05:46 PM, Cornelia Huck wrote:
> On Tue, 5 Sep 2017 17:24:19 +0200
> Halil Pasic  wrote:
> 
>> My problem with a program check (indicated by SCSW word 2 bit 10) is
>> that, in my reading of the architecture, the semantic behind it is: The
>> channel subsystem (not the cu or device) has detected, that the 
>> the channel program (previously submitted as an ORB) is erroneous. Which
>> programs are erroneous is specified by the architecture. What we have
>> here does not qualify.
>>
>> My idea was to rather blame the virtual hardware (device) and put no blame
>> on the program nor he channel subsystem. This could be done using device
>> status (unit check with command reject, maybe unit exception) or interface
>> check. My train of thought was, the problem is not consistent across a
>> device type, so it has to be device specific.
> 
> Unit exception might be a better way to express what is happening here.
> At least, it moves us away from cc 1 and not towards cc 3 :)
> 

I will do a follow up patch pursuing device exception.

>>
>> Of course blaming the device could mislead the person encountering the
>> problem, and make him believe it's an non-virtual hardware problem.
>>
>> About the misleading, I think the best we can do is log out a message
>> indicating what really happened.
> 
> Just document it in the code? If it doesn't happen with Linux as a
> guest, it is highly unlikely to be seen in the wild.
> 


Well we have two problems here:
1) Unit exception can be already defined by the device type for the
command (reference: 
http://publibfp.dhe.ibm.com/cgi-bin/bookmgr/BOOKS/dz9ar110/2.6.10?DT=19920904110920).
I think this one is what you mean. And I agree that's best handled
with comment in code.
2) The poor user/programmer is trying to figure out why things
don't work (why are we getting the unit exception)? I think that's
best remedied with producing something for the log (maybe a warning
with warn_report which states that the implementation vfio-ccw requires
the given flags).

[..] 
>> @@ -1115,7 +1112,7 @@ static int do_subchannel_work(SubchDev *sch)
>>  if (sch->do_subchannel_work) {
>>  return sch->do_subchannel_work(sch);
>>  } else {
>> -return -EINVAL;
>> +return -ENODEV;
>
> This rather seems like a job for an assert? If we don't have a function
> for the 'asynchronous' handling of the various functions assigned for a
> subchannel, that looks like an internal error.
> 

 IMHO it depends. Aborting qemu is heavy handed, and as an user I would not
 be happy about it. But certainly it is an assert situation.  We can look 
 for
 an even better solution, but I think this is an improvement. The logic 
 behind
 is that the device is broken and can't be talked to properly.  
>>>
>>> We currently don't have a vast array of subchannel types (and are
>>> unlikely to get more types that need a different handler function). We
>>> know the current ones are fine, and an assert would catch programming
>>> errors early.
>>>   
>>
>> Despite of that we already had a problem of this type: see 1728cff2ab
>> ("s390x/3270: fix instruction interception handler", 2017-06-09) by 
>> Dong Jia. If we had some automated testing covering all the asserts
>> I would not think twice about using an assert here. But I don't think
>> we do and I'm reluctant (not positive that assert is superior to what
>> we have now). Maybe we could agree on reported by again.
> 
> Yes, we (as in generally 'we') are really lacking automated testing...
> (it is somewhere on my todo list).
> 
> Either leave it as-is, or do an assert. -ENODEV just feels wrong.
> 

I think I will leave this one as is and maybe try to discuss with
the folks here about reliable test coverage. Just spoke with Marc H.,
and according to that we have a long way to go.

Re: [Qemu-devel] [PATCH] vhost-user: disable broken test

[Daniel P. Berrange]

On Tue, Sep 05, 2017 at 02:08:22PM -0300, Philippe Mathieu-Daudé wrote:
> tests/vhost-user-test keeps failing on build-system since Aug 15:
> 
>   ERROR:tests/vhost-user-test.c:835:test_flags_mismatch: child process 
> (/i386/vhost-user/flags-mismatch/subprocess [4836]) failed unexpectedly
> ...
>   ERROR:tests/vhost-user-test.c:807:test_connect_fail: child process 
> (/x86_64/vhost-user/connect-fail/subprocess [58910]) failed unexpectedly

Seems like it is always the subprocess tests which fail
from what I see. So rather than disable the entire test,
just put

   if (1) {
   g_test_skip("Skipping unreliable subprocess test");
   return;
   }

at the top of each test_*_subprocess() method

> 
> The test can still be build for debug session using:
> 
>   $ FORCE_VHOST_USER_NET_TEST=1 ./configure ...
> 
> Suggested-by: Peter Maydell 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  configure | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index fb7e34a901..d3b895b167 100755
> --- a/configure
> +++ b/configure
> @@ -6386,7 +6386,12 @@ if supported_kvm_target $target; then
>  if test "$vhost_net" = "yes" ; then
>  echo "CONFIG_VHOST_NET=y" >> $config_target_mak
>  if test "$vhost_user" = "yes" ; then
> -echo "CONFIG_VHOST_USER_NET_TEST_$target_name=y" >> 
> $config_host_mak
> +if test -n "${FORCE_VHOST_USER_NET_TEST}"; then
> +echo "CONFIG_VHOST_USER_NET_TEST_$target_name=y" >> 
> $config_host_mak
> +elif test -z "${VHOST_USER_NET_TEST_WARNED}"; then
> +echo "warning: vhost-user tests disabled" >&2
> +VHOST_USER_NET_TEST_WARNED="warned"
> +fi
>  fi
>  fi
>  fi
> -- 
> 2.14.1
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PULL v2 0/4] 9pfs/virtfs patches for 2.11 20170905

[Peter Maydell]

On 5 September 2017 at 17:00, Greg Kurz  wrote:
> The following changes since commit 53e2c48d3f0db6a1598f49baf0b56dd4975e53a7:
>
>   Merge remote-tracking branch 'remotes/mcayland/tags/qemu-sparc-signed' into 
> staging (2017-09-04 18:53:46 +0100)
>
> are available in the git repository at:
>
>   https://github.com/gkurz/qemu.git tags/for-upstream
>
> for you to fetch changes up to 32b6943699948f7adc35ada233fbd25daffad5e9:
>
>   virtfs: error out gracefully when mandatory suboptions are missing 
> (2017-09-05 17:56:58 +0200)
>
> 
> Some trivial fixes/cleanup and a fix to cause QEMU to error out gracefully
> instead of aborting.
>
> 
> Greg Kurz (2):
>   9pfs: local: clarify fchmodat_nofollow() implementation
>   virtfs: error out gracefully when mandatory suboptions are missing
>
> Philippe Mathieu-Daudé (1):
>   9pfs: avoid sign conversion error simplifying the code
>
> ZhiPeng Lu (1):
>   fsdev: fix memory leak in main()

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH] vhost-user: disable broken test

[Peter Maydell]

On 5 September 2017 at 18:08, Philippe Mathieu-Daudé  wrote:
> tests/vhost-user-test keeps failing on build-system since Aug 15:
>
>   ERROR:tests/vhost-user-test.c:835:test_flags_mismatch: child process 
> (/i386/vhost-user/flags-mismatch/subprocess [4836]) failed unexpectedly
> ...
>   ERROR:tests/vhost-user-test.c:807:test_connect_fail: child process 
> (/x86_64/vhost-user/connect-fail/subprocess [58910]) failed unexpectedly
>
> The test can still be build for debug session using:
>
>   $ FORCE_VHOST_USER_NET_TEST=1 ./configure ...
>
> Suggested-by: Peter Maydell 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  configure | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index fb7e34a901..d3b895b167 100755
> --- a/configure
> +++ b/configure
> @@ -6386,7 +6386,12 @@ if supported_kvm_target $target; then
>  if test "$vhost_net" = "yes" ; then
>  echo "CONFIG_VHOST_NET=y" >> $config_target_mak
>  if test "$vhost_user" = "yes" ; then
> -echo "CONFIG_VHOST_USER_NET_TEST_$target_name=y" >> 
> $config_host_mak
> +if test -n "${FORCE_VHOST_USER_NET_TEST}"; then
> +echo "CONFIG_VHOST_USER_NET_TEST_$target_name=y" >> 
> $config_host_mak
> +elif test -z "${VHOST_USER_NET_TEST_WARNED}"; then
> +echo "warning: vhost-user tests disabled" >&2
> +VHOST_USER_NET_TEST_WARNED="warned"
> +fi

Please don't make this print a warning -- that will just create
noise in my test logs to no useful effect.

thanks
-- PMM

[Qemu-devel] [Bug 1715186] Re: websockets: Improve error messages

[Daniel Berrange]

At very least we should also use 404 if given a invalid path

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

Title:
  websockets: Improve error messages

Status in QEMU:
  New

Bug description:
  Since 2.9 / 07e95cd529af345fdeea230913f68eff5b925bb6 , whenever the
  VNC websocket server finds an error with the incoming connection
  request, it just closes the socket with no further information.

  This makes figuring out what's wrong with the request nearly
  impossible.

  I would be nice if:

  * HTTP 400 were returned to the client, with an appropriate error message
  * Maybe something written to the log as well?

  Currently, I'm resorting to looking at my request and the websocket
  source and hoping I can figure out what's wrong.

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

Re: [Qemu-devel] [PATCH 10/20] nvic: Add NS alias SCS region

[Peter Maydell]

On 5 September 2017 at 17:48, Richard Henderson
 wrote:
> On 09/05/2017 09:26 AM, Peter Maydell wrote:
>> We don't map the hole. The container is 0x21000 in size, the normal
>> nvic_sysregs region is 0x1000 at offset 0x0 (which will be 0xe000e000
>> in the system address space), and the NS alias region
>> is 0x1000 at offset 0x2 (0xe002e000 in the system address space).
>> There's nothing mapped in the hole in the container, so accesses
>> there will busfault, as they will for other PPB accesses before or
>> after the SCSes.
>
> Ok, it's not wrong, but I still don't understand the need for the container.

It lets us assemble all the parts of this bit of the address space --
SCS, systick (which sits on top of some of the SCS), NS SCS --
in the right place, rather than requiring any caller that wants
to instantiate an NVIC to do it (and to check whether the cpu
has the security extension to figure out whether the NS alias
exists, and so on).

Now that we've QOMified hw/arm/armv7m.c it's less important that
the NVIC in particular does that, and it's partly historical legacy
that most of this is done in the NVIC realize function rather than
in armv7m_instance_init() (we used to implement systick directly
in the NVIC source file rather than as its own device). It doesn't
seem worth shuffling the code around now, though (the container
already existed prior to this patch, we're just making it a bit
bigger and adding another thing to it.)

thanks
-- PMM

[Qemu-devel] [PATCH] vhost-user: disable broken test

[Philippe Mathieu-Daudé]

tests/vhost-user-test keeps failing on build-system since Aug 15:

  ERROR:tests/vhost-user-test.c:835:test_flags_mismatch: child process 
(/i386/vhost-user/flags-mismatch/subprocess [4836]) failed unexpectedly
...
  ERROR:tests/vhost-user-test.c:807:test_connect_fail: child process 
(/x86_64/vhost-user/connect-fail/subprocess [58910]) failed unexpectedly

The test can still be build for debug session using:

  $ FORCE_VHOST_USER_NET_TEST=1 ./configure ...

Suggested-by: Peter Maydell 
Signed-off-by: Philippe Mathieu-Daudé 
---
 configure | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index fb7e34a901..d3b895b167 100755
--- a/configure
+++ b/configure
@@ -6386,7 +6386,12 @@ if supported_kvm_target $target; then
 if test "$vhost_net" = "yes" ; then
 echo "CONFIG_VHOST_NET=y" >> $config_target_mak
 if test "$vhost_user" = "yes" ; then
-echo "CONFIG_VHOST_USER_NET_TEST_$target_name=y" >> 
$config_host_mak
+if test -n "${FORCE_VHOST_USER_NET_TEST}"; then
+echo "CONFIG_VHOST_USER_NET_TEST_$target_name=y" >> 
$config_host_mak
+elif test -z "${VHOST_USER_NET_TEST_WARNED}"; then
+echo "warning: vhost-user tests disabled" >&2
+VHOST_USER_NET_TEST_WARNED="warned"
+fi
 fi
 fi
 fi
-- 
2.14.1

Re: [Qemu-devel] [PATCH] virtfs: error out gracefully when mandatory suboptions are missing

[Greg Kurz]

On Tue, 05 Sep 2017 10:20:22 -0500
Michael Roth  wrote:
[...]
> > 
> > Well, it's been there forever and it isn't a critical fix... but the
> > thread on qemu-discuss the other day showed that it was confusing
> > people, and the fix is trivial. So I guess it doesn't hurt to have
> > this in stable. :)
> > 
> > Michael,
> > 
> > I'll send a pull req later today. If it's not too late, maybe you can add
> > this patch to 2.9.1 when it's merged in master?  
> 
> We're in freeze as of yesterday with release set for Thursday, so it's a
> bit late. OTOH I don't have test coverage for virtfs and the patch seems
> isolated enough that I don't see any harm in slipping it in if it
> lands in master by tomorrow. Otherwise it'll have to wait for 2.10.1,
> which I'm hoping to get out this month.
> 

The fix just got merged.

commit 32b6943699948f7adc35ada233fbd25daffad5e9
Author: Greg Kurz 
Date:   Mon Sep 4 09:59:01 2017 +0200

virtfs: error out gracefully when mandatory suboptions are missing

Cheers,

--
Greg


pgpy_SQx0FNfV.pgp
Description: OpenPGP digital signature