Re: [Qemu-devel] [PATCH v3 5/5] docs: update documentation considering PCIE-PCI bridge

[Laine Stump]

On 08/02/2017 01:58 PM, Marcel Apfelbaum wrote:
> On 02/08/2017 19:26, Michael S. Tsirkin wrote:
>> On Wed, Aug 02, 2017 at 06:36:29PM +0300, Marcel Apfelbaum wrote:
 Can dmi-pci support shpc? why doesn't it? For compatibility?
>>>
>>> I don't know why, but the fact that it doesn't is the reason libvirt
>>> settled on auto-creating a dmi-pci bridge and a pci-pci bridge under
>>> that for Q35. The reasoning was (IIRC Laine's words correctly)
>>> that the
>>> dmi-pci bridge cannot receive hotplugged devices, while the pci-pci
>>> bridge cannot be connected to the root complex. So both were needed.


At least that's what I was told :-) (seriously, 50% of the convoluted
rules encoded into libvirt's PCI bus topology construction and
connection rules come from trial and error, and the other 50% come from
advice and recommendations from others who (unlike me) actually know
something about PCI.)

Of course the whole setup of plugging a pci-bridge into a
dmi-to-pci-bridge was (at the time at least) an exercise in futility,
since hotplug didn't work properly on pci-bridge+Q35 anyway (that
initially wasn't explained to me; it was only after I had constructed
the odd bus topology and it was in released code that someone told me
"Oh, by the way, hotplug to pci-bridge doesn't work on Q35". At first it
was described as a bug, then later reclassified as a future feature.)

(I guess the upside is that all of the horrible complex/confusing code
needed to automatically add two controllers just to plug in a single
endpoint is now already in the code, and will "just work" if/when needed).

Now that I go back to look at this thread (qemu-devel is just too much
for me to try and read unless something has been Cc'ed to me - I really
don't know how you guys manage it!), I see that pcie-pci-bridge has been
implemented, and we (libvirt) will want to use that instead of
dmi-to-pci-bridge when available. And pcie-pci-bridge itself can have
endpoints hotplugged into it, correct? This means there will need to be
patches for libvirt that check for the presence of pcie-pci-bridge, and
if it's found they will replace any auto-added
dmi-to-pci-bridge+pci-bridge with a long pcie-pci-bridge.

>>>
>>> Thanks
>>> Laszlo
>>
>> OK. Is it true that dmi-pci + pci-pci under it will allow hotplug
>> on Q35 if we just flip the bit in _OSC?
>
> Marcel, what say you?... :)
>>>
>>> Good news, works with:
>>> -device i82801b11-bridge,id=b1
>>> -device pci-bridge,id=b2,bus=b1,chassis_nr=1,msi=off
>>
>> And presumably it works for modern windows?
>> OK, so it looks like patch 1 is merely a bugfix, I'll merge it for 2.10.
>>
> 
> Tested with Win10, I think is OK to merge if for 2.10.
> 
>>> Notice bridge's msi=off until the following kernel bug will be merged:
>>>https://www.spinics.net/lists/linux-pci/msg63052.html
>>
>> Does libvirt support msi=off as a work-around?

We have no explicit setting for msi on pci controllers. The only place
we explicitly set that is on the ivshmem device.

That doesn't mean that we couldn't add it. However, if we were going to
do it manually, that would mean adding another knob that we have to
support forever. And even if we wanted to do it automatically, we would
not only need to find something in qemu to key off of when deciding
whether or not to set it, but we would *still* have to explicitly store
the setting in the config so that migrations between hosts using
differing versions of qemu would preserve guest ABI. Are there really
enough people demanding (with actual concrete plans of *using*) hotplug
of legacy PCI devices on Q35 guests *immediately* that we want to
permanently pollute libvirt's code in this manner just for an interim
workaround?


I didn't have enough time/energy to fully parse all the rest of this
thread - is msi=off currently required for pcie-pci-bridge hotplug as
well? (not that it changes my opinion - just as we can tell people
"upgrade to a new qemu and libvirt if you want to hotplug legacy PCI
devices on Q35 guests", we can also tell them "Oh, and wait X weeks and
upgrade to a new kernel too".

Re: [Qemu-devel] [PATCH] libqtest: Fix typo in comments

[Jeff Cody]

On Wed, Aug 02, 2017 at 08:08:33PM -0500, Eric Blake wrote:
> s/continuosly/continuously/
> 
> Signed-off-by: Eric Blake 

Hardly seems like a trivial patch like this should need an R-b, but what the
heck:

Reviewed-by: Jeff Cody 


> ---
>  tests/libqtest.h | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/libqtest.h b/tests/libqtest.h
> index 38bc1e9953..3ae570927a 100644
> --- a/tests/libqtest.h
> +++ b/tests/libqtest.h
> @@ -117,7 +117,7 @@ QDict *qtest_qmp_receive(QTestState *s);
>   * @s: #QTestState instance to operate on.
>   * @s: #event event to wait for.
>   *
> - * Continuosly polls for QMP responses until it receives the desired event.
> + * Continuously polls for QMP responses until it receives the desired event.
>   */
>  void qtest_qmp_eventwait(QTestState *s, const char *event);
> 
> @@ -126,7 +126,7 @@ void qtest_qmp_eventwait(QTestState *s, const char 
> *event);
>   * @s: #QTestState instance to operate on.
>   * @s: #event event to wait for.
>   *
> - * Continuosly polls for QMP responses until it receives the desired event.
> + * Continuously polls for QMP responses until it receives the desired event.
>   * Returns a copy of the event for further investigation.
>   */
>  QDict *qtest_qmp_eventwait_ref(QTestState *s, const char *event);
> @@ -571,7 +571,7 @@ static inline QDict *qmp_receive(void)
>   * qmp_eventwait:
>   * @s: #event event to wait for.
>   *
> - * Continuosly polls for QMP responses until it receives the desired event.
> + * Continuously polls for QMP responses until it receives the desired event.
>   */
>  static inline void qmp_eventwait(const char *event)
>  {
> @@ -582,7 +582,7 @@ static inline void qmp_eventwait(const char *event)
>   * qmp_eventwait_ref:
>   * @s: #event event to wait for.
>   *
> - * Continuosly polls for QMP responses until it receives the desired event.
> + * Continuously polls for QMP responses until it receives the desired event.
>   * Returns a copy of the event for further investigation.
>   */
>  static inline QDict *qmp_eventwait_ref(const char *event)
> -- 
> 2.13.3
> 
> 

Re: [Qemu-devel] [PATCH] ppc: fix double-free in cpu_post_load()

[David Gibson]

On Wed, Aug 02, 2017 at 07:34:16PM +0200, Greg Kurz wrote:
> When running nested with KVM PR, ppc_set_compat() fails and QEMU crashes
> because of "double free or corruption (!prev)". The crash happens because
> error_report_err() has already called error_free().
> 
> Signed-off-by: Greg Kurz 

Oops, that's a bit embarassing.  Applied to ppc-for-2.10.

> ---
>  target/ppc/machine.c |1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index f578156dd411..abe0a1cdf021 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -239,7 +239,6 @@ static int cpu_post_load(void *opaque, int version_id)
>  ppc_set_compat(cpu, cpu->compat_pvr, _err);
>  if (local_err) {
>  error_report_err(local_err);
> -error_free(local_err);
>  return -1;
>  }
>  } else
> 

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH for-2.10 0/5] tests: acpi: make sure FADT is compared to reference table

[Michael S. Tsirkin]

On Wed, Aug 02, 2017 at 09:51:22AM +0200, Igor Mammedov wrote:
> On Wed, 2 Aug 2017 00:14:18 +0300
> "Michael S. Tsirkin"  wrote:
> 
> > On Mon, Jul 31, 2017 at 05:40:47PM +0200, Igor Mammedov wrote:
> > > While refactoring i386/FADT generation to build_append_int_noprefix() 
> > >
> > > and testing it, It turned out that FADT is only tested for valid  
> > >
> > > checksum but actual test for unintended changes isn't applied to it   
> > >
> > > even though we have reference tables in tree. 
> > >
> > > So here goes a couple of cleanups to reflect what fuctions do +   
> > >
> > > some comments and actual fix. 
> > >
> > >   
> > >
> > > Note to maintainer:   
> > >
> > >   FADT reference table is out of sync and should be updated along with
> > >
> > >   series applied. 
> > >
> > >   
> > >
> > > CC: "Michael S. Tsirkin" 
> > > 
> > > CC: Marcel Apfelbaum    
> > 
> > Absolutely good stuff, but not a bugfix as such (it's not that the
> > test is wrong, it's that we skip FADT for now)
> > so I don't think this is 2.10 material.
> Agreed, it could go in when 2.11 merge window is open.

thanks,pls remember to repost or ping then.

> > 
> > > Igor Mammedov (5):
> > >   tests: acpi: move tested tables array allocation outside of
> > > test_acpi_dsdt_table()
> > >   tests: acpi: init table descriptor in test_dst_table()
> > >   tests: acpi: rename test_acpi_tables()/test_dst_table() to reflect its
> > > usage
> > >   tests: acpi: add comments to fetch_rsdt_referenced_tables/data->tables
> > > usage
> > >   tests: acpi: fix FADT not being compared to reference table
> > > 
> > >  tests/bios-tables-test.c | 30 ++
> > >  1 file changed, 18 insertions(+), 12 deletions(-)
> > > 
> > > -- 
> > > 2.7.4  

Re: [Qemu-devel] [PATCH v2 1/2] build-sys: add --disable-vhost-user

[Michael S. Tsirkin]

On Fri, Jul 28, 2017 at 04:13:08PM +0200, Marc-André Lureau wrote:
> Learn to compile out vhost-user. Keep it enabled by default on
> non-win32, that is assumed to be POSIX. Fail if trying to enable it on
> win32.
> 
> When trying to make a vhost-user netdev, it gives the following error:
> 
> -netdev vhost-user,id=foo,chardev=chr-test: Parameter 'type' expects a netdev 
> backend type
> 
> And similar error with the HMP/QMP monitors.
> 
> While at it, rename CONFIG_VHOST_NET_TEST CONFIG_VHOST_USER_NET_TEST
> since it's a vhost-user specific variable.
> 
> Signed-off-by: Marc-André Lureau 

OK so pls address Cornelia's comment and post just patch 1 as
it seems appropriate for 2.10.

> ---
>  hw/virtio/virtio-pci.c|  4 ++--
>  configure | 29 +++--
>  default-configs/pci.mak   |  2 +-
>  default-configs/s390x-softmmu.mak |  2 +-
>  tests/Makefile.include|  6 +++---
>  5 files changed, 34 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 5d14bd66dc..8b0d6b69cd 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -2135,7 +2135,7 @@ static const TypeInfo vhost_scsi_pci_info = {
>  };
>  #endif
>  
> -#ifdef CONFIG_LINUX
> +#if defined(CONFIG_VHOST_USER) && defined(CONFIG_LINUX)
>  /* vhost-user-scsi-pci */
>  static Property vhost_user_scsi_pci_properties[] = {
>  DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
> @@ -2665,7 +2665,7 @@ static void virtio_pci_register_types(void)
>  #ifdef CONFIG_VHOST_SCSI
>  type_register_static(_scsi_pci_info);
>  #endif
> -#ifdef CONFIG_LINUX
> +#if defined(CONFIG_VHOST_USER) && defined(CONFIG_LINUX)
>  type_register_static(_user_scsi_pci_info);
>  #endif
>  #ifdef CONFIG_VHOST_VSOCK
> diff --git a/configure b/configure
> index 987f59ba88..efec1a613e 100755
> --- a/configure
> +++ b/configure
> @@ -306,6 +306,7 @@ tcg="yes"
>  vhost_net="no"
>  vhost_scsi="no"
>  vhost_vsock="no"
> +vhost_user=""
>  kvm="no"
>  hax="no"
>  rdma=""
> @@ -1282,6 +1283,15 @@ for opt do
>;;
>--enable-vxhs) vxhs="yes"
>;;
> +  --disable-vhost-user) vhost_user="no"
> +  ;;
> +  --enable-vhost-user)
> +  vhost_user="yes"
> +  if test "$mingw32" = "yes" ; then
> +  echo "ERROR: vhost-user isn't available on win32"
> +  exit 1
> +  fi
> +  ;;
>*)
>echo "ERROR: unknown option $opt"
>echo "Try '$0 --help' for more information"
> @@ -1290,6 +1300,14 @@ for opt do
>esac
>  done
>  
> +if test "$vhost_user" = ""; then
> +if test "$mingw32" = "yes" ; then
> +vhost_user="no"
> +else
> +vhost_user="yes"
> +fi
> +fi
> +
>  case "$cpu" in
>  ppc)
> CPU_CFLAGS="-m32"
> @@ -1518,6 +1536,7 @@ disabled with --disable-FEATURE, default is enabled if 
> available:
>tools   build qemu-io, qemu-nbd and qemu-image tools
>vxhsVeritas HyperScale vDisk backend support
>crypto-afalgLinux AF_ALG crypto backend driver
> +  vhost-user  vhost-user support
>  
>  NOTE: The object files are built at the place where configure is launched
>  EOF
> @@ -5348,6 +5367,7 @@ echo "libcap-ng support $cap_ng"
>  echo "vhost-net support $vhost_net"
>  echo "vhost-scsi support $vhost_scsi"
>  echo "vhost-vsock support $vhost_vsock"
> +echo "vhost-user support $vhost_user"
>  echo "Trace backends$trace_backends"
>  if have_backend "simple"; then
>  echo "Trace output file $trace_file-"
> @@ -5757,12 +5777,15 @@ fi
>  if test "$vhost_scsi" = "yes" ; then
>echo "CONFIG_VHOST_SCSI=y" >> $config_host_mak
>  fi
> -if test "$vhost_net" = "yes" ; then
> +if test "$vhost_net" = "yes" -a "$vhost_user" = "yes"; then
>echo "CONFIG_VHOST_NET_USED=y" >> $config_host_mak
>  fi
>  if test "$vhost_vsock" = "yes" ; then
>echo "CONFIG_VHOST_VSOCK=y" >> $config_host_mak
>  fi
> +if test "$vhost_user" = "yes" ; then
> +  echo "CONFIG_VHOST_USER=y" >> $config_host_mak
> +fi
>  if test "$blobs" = "yes" ; then
>echo "INSTALL_BLOBS=yes" >> $config_host_mak
>  fi
> @@ -6358,7 +6381,9 @@ if supported_kvm_target $target; then
>  echo "CONFIG_KVM=y" >> $config_target_mak
>  if test "$vhost_net" = "yes" ; then
>  echo "CONFIG_VHOST_NET=y" >> $config_target_mak
> -echo "CONFIG_VHOST_NET_TEST_$target_name=y" >> $config_host_mak
> +if test "$vhost_user" = "yes" ; then
> +echo "CONFIG_VHOST_USER_NET_TEST_$target_name=y" >> 
> $config_host_mak
> +fi
>  fi
>  fi
>  if supported_hax_target $target; then
> diff --git a/default-configs/pci.mak b/default-configs/pci.mak
> index acaa70301a..a758630d30 100644
> --- a/default-configs/pci.mak
> +++ b/default-configs/pci.mak
> @@ -43,4 +43,4 @@ CONFIG_VGA=y
>  CONFIG_VGA_PCI=y
>  CONFIG_IVSHMEM_DEVICE=$(CONFIG_IVSHMEM)
>  CONFIG_ROCKER=y
> -CONFIG_VHOST_USER_SCSI=$(CONFIG_LINUX)
> +CONFIG_VHOST_USER_SCSI=$(and 

[Qemu-devel] [PATCH] libqtest: Fix typo in comments

[Eric Blake]

s/continuosly/continuously/

Signed-off-by: Eric Blake 
---
 tests/libqtest.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/libqtest.h b/tests/libqtest.h
index 38bc1e9953..3ae570927a 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -117,7 +117,7 @@ QDict *qtest_qmp_receive(QTestState *s);
  * @s: #QTestState instance to operate on.
  * @s: #event event to wait for.
  *
- * Continuosly polls for QMP responses until it receives the desired event.
+ * Continuously polls for QMP responses until it receives the desired event.
  */
 void qtest_qmp_eventwait(QTestState *s, const char *event);

@@ -126,7 +126,7 @@ void qtest_qmp_eventwait(QTestState *s, const char *event);
  * @s: #QTestState instance to operate on.
  * @s: #event event to wait for.
  *
- * Continuosly polls for QMP responses until it receives the desired event.
+ * Continuously polls for QMP responses until it receives the desired event.
  * Returns a copy of the event for further investigation.
  */
 QDict *qtest_qmp_eventwait_ref(QTestState *s, const char *event);
@@ -571,7 +571,7 @@ static inline QDict *qmp_receive(void)
  * qmp_eventwait:
  * @s: #event event to wait for.
  *
- * Continuosly polls for QMP responses until it receives the desired event.
+ * Continuously polls for QMP responses until it receives the desired event.
  */
 static inline void qmp_eventwait(const char *event)
 {
@@ -582,7 +582,7 @@ static inline void qmp_eventwait(const char *event)
  * qmp_eventwait_ref:
  * @s: #event event to wait for.
  *
- * Continuosly polls for QMP responses until it receives the desired event.
+ * Continuously polls for QMP responses until it receives the desired event.
  * Returns a copy of the event for further investigation.
  */
 static inline QDict *qmp_eventwait_ref(const char *event)
-- 
2.13.3

[Qemu-devel] [PATCH v3 1/2] s390x/css: use macro for event-information pending error recover code

[Dong Jia Shi]

Let's use a macro for the ERC (error recover code) when generating a
Channel Subsystem Event-information pending CRW (channel report word).

While we are at it, let's also add all other ERCs.

Signed-off-by: Dong Jia Shi 
Reviewed-by: Halil Pasic 
---
 hw/s390x/css.c|  2 +-
 include/hw/s390x/ioinst.h | 12 ++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 6a42b95cee..5321ca016b 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -2103,7 +2103,7 @@ void css_generate_chp_crws(uint8_t cssid, uint8_t chpid)
 void css_generate_css_crws(uint8_t cssid)
 {
 if (!channel_subsys.sei_pending) {
-css_queue_crw(CRW_RSC_CSS, 0, 0, cssid);
+css_queue_crw(CRW_RSC_CSS, CRW_ERC_EVENT, 0, cssid);
 }
 channel_subsys.sei_pending = true;
 }
diff --git a/include/hw/s390x/ioinst.h b/include/hw/s390x/ioinst.h
index 92d15655e4..5f2db6949d 100644
--- a/include/hw/s390x/ioinst.h
+++ b/include/hw/s390x/ioinst.h
@@ -201,8 +201,16 @@ typedef struct CRW {
 #define CRW_FLAGS_MASK_A 0x0080
 #define CRW_FLAGS_MASK_ERC 0x003f
 
-#define CRW_ERC_INIT 0x02
-#define CRW_ERC_IPI  0x04
+#define CRW_ERC_EVENT0x00 /* event information pending */
+#define CRW_ERC_AVAIL0x01 /* available */
+#define CRW_ERC_INIT 0x02 /* initialized */
+#define CRW_ERC_TERROR   0x03 /* temporary error */
+#define CRW_ERC_IPI  0x04 /* installed parm initialized */
+#define CRW_ERC_TERM 0x05 /* terminal */
+#define CRW_ERC_PERRN0x06 /* perm. error, facility not init */
+#define CRW_ERC_PERRI0x07 /* perm. error, facility init */
+#define CRW_ERC_PMOD 0x08 /* installed parameters modified */
+#define CRW_ERC_IPR  0x0A /* installed parameters restored */
 
 #define CRW_RSC_SUBCH 0x3
 #define CRW_RSC_CHP   0x4
-- 
2.11.2

[Qemu-devel] [PATCH v3 2/2] s390x/css: generate solicited crw for rchp completion signaling

[Dong Jia Shi]

A successful completion of rchp should signal a solicited channel path
initialized CRW (channel report word), while the current implementation
always generates an un-solicited one. Let's fix this.

Reported-by: Halil Pasic 
Signed-off-by: Dong Jia Shi 
Reviewed-by: Halil Pasic 
---
 hw/s390x/css.c | 16 ++--
 include/hw/s390x/css.h |  3 ++-
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 5321ca016b..bfa56f4b12 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -1745,10 +1745,10 @@ int css_do_rchp(uint8_t cssid, uint8_t chpid)
 }
 
 /* We don't really use a channel path, so we're done here. */
-css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT,
+css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, 1,
   channel_subsys.max_cssid > 0 ? 1 : 0, chpid);
 if (channel_subsys.max_cssid > 0) {
-css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, 0, real_cssid << 8);
+css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, 1, 0, real_cssid << 8);
 }
 return 0;
 }
@@ -2028,7 +2028,8 @@ void css_subch_assign(uint8_t cssid, uint8_t ssid, 
uint16_t schid,
 }
 }
 
-void css_queue_crw(uint8_t rsc, uint8_t erc, int chain, uint16_t rsid)
+void css_queue_crw(uint8_t rsc, uint8_t erc, int solicited,
+   int chain, uint16_t rsid)
 {
 CrwContainer *crw_cont;
 
@@ -2040,6 +2041,9 @@ void css_queue_crw(uint8_t rsc, uint8_t erc, int chain, 
uint16_t rsid)
 return;
 }
 crw_cont->crw.flags = (rsc << 8) | erc;
+if (solicited) {
+crw_cont->crw.flags |= CRW_FLAGS_MASK_S;
+}
 if (chain) {
 crw_cont->crw.flags |= CRW_FLAGS_MASK_C;
 }
@@ -2086,9 +2090,9 @@ void css_generate_sch_crws(uint8_t cssid, uint8_t ssid, 
uint16_t schid,
 }
 chain_crw = (channel_subsys.max_ssid > 0) ||
 (channel_subsys.max_cssid > 0);
-css_queue_crw(CRW_RSC_SUBCH, CRW_ERC_IPI, chain_crw ? 1 : 0, schid);
+css_queue_crw(CRW_RSC_SUBCH, CRW_ERC_IPI, 0, chain_crw ? 1 : 0, schid);
 if (chain_crw) {
-css_queue_crw(CRW_RSC_SUBCH, CRW_ERC_IPI, 0,
+css_queue_crw(CRW_RSC_SUBCH, CRW_ERC_IPI, 0, 0,
   (guest_cssid << 8) | (ssid << 4));
 }
 /* RW_ERC_IPI --> clear pending interrupts */
@@ -2103,7 +2107,7 @@ void css_generate_chp_crws(uint8_t cssid, uint8_t chpid)
 void css_generate_css_crws(uint8_t cssid)
 {
 if (!channel_subsys.sei_pending) {
-css_queue_crw(CRW_RSC_CSS, CRW_ERC_EVENT, 0, cssid);
+css_queue_crw(CRW_RSC_CSS, CRW_ERC_EVENT, 0, 0, cssid);
 }
 channel_subsys.sei_pending = true;
 }
diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
index 5c5fe6b202..5b017e1fc3 100644
--- a/include/hw/s390x/css.h
+++ b/include/hw/s390x/css.h
@@ -150,7 +150,8 @@ void copy_scsw_to_guest(SCSW *dest, const SCSW *src);
 void css_inject_io_interrupt(SubchDev *sch);
 void css_reset(void);
 void css_reset_sch(SubchDev *sch);
-void css_queue_crw(uint8_t rsc, uint8_t erc, int chain, uint16_t rsid);
+void css_queue_crw(uint8_t rsc, uint8_t erc, int solicited,
+   int chain, uint16_t rsid);
 void css_generate_sch_crws(uint8_t cssid, uint8_t ssid, uint16_t schid,
int hotplugged, int add);
 void css_generate_chp_crws(uint8_t cssid, uint8_t chpid);
-- 
2.11.2

[Qemu-devel] [PATCH v3 0/2] ERC cleanup and CRW bugfix

[Dong Jia Shi]

This series is trying to:
1. clear up ERC related code
2. bugfix for channel path related CRW generation

Change log
--
v2->v3:
Added Halil's R-B on both patches.
Patch #1:
  Added ERC "installed parameters restored".

v1->v2:
Patch #1:
  Add all ERCs.
  Commit message update.
Patch #2:
  Rename the new added parameter to more speaking name -- solicited.

Dong Jia Shi (2):
  s390x/css: use macro for event-information pending error recover code
  s390x/css: generate solicited crw for rchp completion signaling

 hw/s390x/css.c| 16 ++--
 include/hw/s390x/css.h|  3 ++-
 include/hw/s390x/ioinst.h | 12 ++--
 3 files changed, 22 insertions(+), 9 deletions(-)

-- 
2.11.2

Re: [Qemu-devel] How to make ELF headers/symbol sections available for multiboot?

[Richard Henderson]

On 08/02/2017 03:00 PM, Anatol Pomozov wrote:
> What ELF specification says about it? Does it tell a loader to load
> only PT_LOAD segments?

Yes.  In https://refspecs.linuxfoundation.org/ there is a link to "System V ABI
Edition 4.1", which AFAIK is the latest version of the ELF "gABI" spec.
Section 5 describes program loading.

> Here is my current linker script:
> 
> ==
> ENTRY(start)
> 
> SECTIONS {
> . = 1M;
> 
> .text : {
> KEEP(*(.data.multiboot))
> *(.text .text*)
> }
> 
> .rodata : {
> *(.rodata .rodata*)
> }
> 
> .data : {
> *(.data .data.*)
> }
> 
> .bss : {
> __bss_start = .;
> *(.bss .bss*)
> . = ALIGN(8);
> __bss_end = .;
> }
> }
> ===

In the gnu ld manual, read about the PHDRS command, which describes all of the
ways you can manipulate the program header table.

Re-reading that now, I see FILEHDR and PHDR as keywords that can be used to
induce those portions of the file into the loadable segment, but I do not see
anything that could be used to load the section header.

You could force the symbol table and string table to be loaded, by referencing
their sections, but that won't affect the section header itself.


r~

[Qemu-devel] [PULL 0/2] slirp updates

[Samuel Thibault]

warning: redirection vers https://people.debian.org/~sthibault/qemu.git/
The following changes since commit aaaec6acad7cf97372d48c1b09126a09697519c8:

  Update version for v2.10.0-rc1 release (2017-08-02 16:36:32 +0100)

are available in the git repository at:

  http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault

for you to fetch changes up to 413d463f43fbc4dd3a601e80a5724aa384a265a0:

  slirp: check len against dhcp options array end (2017-08-03 00:26:44 +0200)


slirp updates


Hervé Poussineau (1):
  slirp: fill error when failing to initialize user network

Prasad J Pandit (1):
  slirp: check len against dhcp options array end

 net/slirp.c   | 134 --
 slirp/bootp.c |   3 ++
 2 files changed, 97 insertions(+), 40 deletions(-)

Re: [Qemu-devel] [PULL 2/2] slirp: check len against dhcp options array end

[Samuel Thibault]

Samuel Thibault, on jeu. 03 août 2017 00:26:07 +0200, wrote:
> From: Prasad J Pandit 
> 
> While parsing dhcp options string in 'dhcp_decode', if an options'
> length 'len' appeared towards the end of 'bp_vend' array, ensuing
> read could lead to an OOB memory access issue. Add check to avoid it.
> 
> Reported-by: Reno Robert 
> Signed-off-by: Prasad J Pandit 
> Signed-off-by: Samuel Thibault 

Oops, this should have mentioned the CVE, I'll redo the pull request,
sorry.

> ---
>  slirp/bootp.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/slirp/bootp.c b/slirp/bootp.c
> index 5a4646c182..5dd1a415b5 100644
> --- a/slirp/bootp.c
> +++ b/slirp/bootp.c
> @@ -123,6 +123,9 @@ static void dhcp_decode(const struct bootp_t *bp, int 
> *pmsg_type,
>  if (p >= p_end)
>  break;
>  len = *p++;
> +if (p + len > p_end) {
> +break;
> +}
>  DPRINTF("dhcp: tag=%d len=%d\n", tag, len);
>  
>  switch(tag) {
> -- 
> 2.13.2
> 

-- 
Samuel
"...[Linux's] capacity to talk via any medium except smoke signals."
(By Dr. Greg Wettstein, Roger Maris Cancer Center)

[Qemu-devel] [PULL 1/2] slirp: fill error when failing to initialize user network

[Samuel Thibault]

From: Hervé Poussineau 

With "-netdev user,id=net0,dns=1.2.3.4"
error was:
qemu-system-i386: -netdev user,id=net0,dns=1.2.3.4: Device 'user' could not be 
initialized

Error is now:
qemu-system-i386: -netdev user,id=net0,dns=1.2.3.4: DNS doesn't belong to 
network

Signed-off-by: Hervé Poussineau 
Signed-off-by: Samuel Thibault 
---
 net/slirp.c | 134 ++--
 1 file changed, 94 insertions(+), 40 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index 9fbc949e81..01ed21c006 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -91,15 +91,15 @@ static QTAILQ_HEAD(slirp_stacks, SlirpState) slirp_stacks =
 QTAILQ_HEAD_INITIALIZER(slirp_stacks);
 
 static int slirp_hostfwd(SlirpState *s, const char *redir_str,
- int legacy_format);
+ int legacy_format, Error **errp);
 static int slirp_guestfwd(SlirpState *s, const char *config_str,
-  int legacy_format);
+  int legacy_format, Error **errp);
 
 #ifndef _WIN32
 static const char *legacy_smb_export;
 
 static int slirp_smb(SlirpState *s, const char *exported_dir,
- struct in_addr vserver_addr);
+ struct in_addr vserver_addr, Error **errp);
 static void slirp_smb_cleanup(SlirpState *s);
 #else
 static inline void slirp_smb_cleanup(SlirpState *s) { }
@@ -155,7 +155,7 @@ static int net_slirp_init(NetClientState *peer, const char 
*model,
   const char *bootfile, const char *vdhcp_start,
   const char *vnameserver, const char *vnameserver6,
   const char *smb_export, const char *vsmbserver,
-  const char **dnssearch)
+  const char **dnssearch, Error **errp)
 {
 /* default settings according to historic slirp */
 struct in_addr net  = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
@@ -178,15 +178,18 @@ static int net_slirp_init(NetClientState *peer, const 
char *model,
 struct slirp_config_str *config;
 
 if (!ipv4 && (vnetwork || vhost || vnameserver)) {
+error_setg(errp, "IPv4 disabled but netmask/host/dns provided");
 return -1;
 }
 
 if (!ipv6 && (vprefix6 || vhost6 || vnameserver6)) {
+error_setg(errp, "IPv6 disabled but prefix/host6/dns6 provided");
 return -1;
 }
 
 if (!ipv4 && !ipv6) {
 /* It doesn't make sense to disable both */
+error_setg(errp, "IPv4 and IPv6 disabled");
 return -1;
 }
 
@@ -200,6 +203,7 @@ static int net_slirp_init(NetClientState *peer, const char 
*model,
 if (vnetwork) {
 if (get_str_sep(buf, sizeof(buf), , '/') < 0) {
 if (!inet_aton(vnetwork, )) {
+error_setg(errp, "Failed to parse netmask");
 return -1;
 }
 addr = ntohl(net.s_addr);
@@ -220,14 +224,19 @@ static int net_slirp_init(NetClientState *peer, const 
char *model,
 }
 } else {
 if (!inet_aton(buf, )) {
+error_setg(errp, "Failed to parse netmask");
 return -1;
 }
 shift = strtol(vnetwork, , 10);
 if (*end != '\0') {
 if (!inet_aton(vnetwork, )) {
+error_setg(errp,
+   "Failed to parse netmask (trailing chars)");
 return -1;
 }
 } else if (shift < 4 || shift > 32) {
+error_setg(errp,
+   "Invalid netmask provided (must be in range 4-32)");
 return -1;
 } else {
 mask.s_addr = htonl(0x << (32 - shift));
@@ -240,30 +249,43 @@ static int net_slirp_init(NetClientState *peer, const 
char *model,
 }
 
 if (vhost && !inet_aton(vhost, )) {
+error_setg(errp, "Failed to parse host");
 return -1;
 }
 if ((host.s_addr & mask.s_addr) != net.s_addr) {
+error_setg(errp, "Host doesn't belong to network");
 return -1;
 }
 
 if (vnameserver && !inet_aton(vnameserver, )) {
+error_setg(errp, "Failed to parse DNS");
 return -1;
 }
-if ((dns.s_addr & mask.s_addr) != net.s_addr ||
-dns.s_addr == host.s_addr) {
+if ((dns.s_addr & mask.s_addr) != net.s_addr) {
+error_setg(errp, "DNS doesn't belong to network");
+return -1;
+}
+if (dns.s_addr == host.s_addr) {
+error_setg(errp, "DNS must be different from host");
 return -1;
 }
 
 if (vdhcp_start && !inet_aton(vdhcp_start, )) {
+error_setg(errp, "Failed to parse DHCP start address");
 return -1;
 }
-if ((dhcp.s_addr & mask.s_addr) != net.s_addr ||
-dhcp.s_addr == host.s_addr || dhcp.s_addr == dns.s_addr) {
+if ((dhcp.s_addr & 

[Qemu-devel] [PULL 2/2] slirp: check len against dhcp options array end

[Samuel Thibault]

From: Prasad J Pandit 

While parsing dhcp options string in 'dhcp_decode', if an options'
length 'len' appeared towards the end of 'bp_vend' array, ensuing
read could lead to an OOB memory access issue. Add check to avoid it.

This is CVE-2017-11434.

Reported-by: Reno Robert 
Signed-off-by: Prasad J Pandit 
Signed-off-by: Samuel Thibault 
---
 slirp/bootp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/slirp/bootp.c b/slirp/bootp.c
index 5a4646c182..5dd1a415b5 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -123,6 +123,9 @@ static void dhcp_decode(const struct bootp_t *bp, int 
*pmsg_type,
 if (p >= p_end)
 break;
 len = *p++;
+if (p + len > p_end) {
+break;
+}
 DPRINTF("dhcp: tag=%d len=%d\n", tag, len);
 
 switch(tag) {
-- 
2.13.2

[Qemu-devel] [PULL 1/2] slirp: fill error when failing to initialize user network

[Samuel Thibault]

From: Hervé Poussineau 

With "-netdev user,id=net0,dns=1.2.3.4"
error was:
qemu-system-i386: -netdev user,id=net0,dns=1.2.3.4: Device 'user' could not be 
initialized

Error is now:
qemu-system-i386: -netdev user,id=net0,dns=1.2.3.4: DNS doesn't belong to 
network

Signed-off-by: Hervé Poussineau 
Signed-off-by: Samuel Thibault 
---
 net/slirp.c | 134 ++--
 1 file changed, 94 insertions(+), 40 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index 9fbc949e81..01ed21c006 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -91,15 +91,15 @@ static QTAILQ_HEAD(slirp_stacks, SlirpState) slirp_stacks =
 QTAILQ_HEAD_INITIALIZER(slirp_stacks);
 
 static int slirp_hostfwd(SlirpState *s, const char *redir_str,
- int legacy_format);
+ int legacy_format, Error **errp);
 static int slirp_guestfwd(SlirpState *s, const char *config_str,
-  int legacy_format);
+  int legacy_format, Error **errp);
 
 #ifndef _WIN32
 static const char *legacy_smb_export;
 
 static int slirp_smb(SlirpState *s, const char *exported_dir,
- struct in_addr vserver_addr);
+ struct in_addr vserver_addr, Error **errp);
 static void slirp_smb_cleanup(SlirpState *s);
 #else
 static inline void slirp_smb_cleanup(SlirpState *s) { }
@@ -155,7 +155,7 @@ static int net_slirp_init(NetClientState *peer, const char 
*model,
   const char *bootfile, const char *vdhcp_start,
   const char *vnameserver, const char *vnameserver6,
   const char *smb_export, const char *vsmbserver,
-  const char **dnssearch)
+  const char **dnssearch, Error **errp)
 {
 /* default settings according to historic slirp */
 struct in_addr net  = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
@@ -178,15 +178,18 @@ static int net_slirp_init(NetClientState *peer, const 
char *model,
 struct slirp_config_str *config;
 
 if (!ipv4 && (vnetwork || vhost || vnameserver)) {
+error_setg(errp, "IPv4 disabled but netmask/host/dns provided");
 return -1;
 }
 
 if (!ipv6 && (vprefix6 || vhost6 || vnameserver6)) {
+error_setg(errp, "IPv6 disabled but prefix/host6/dns6 provided");
 return -1;
 }
 
 if (!ipv4 && !ipv6) {
 /* It doesn't make sense to disable both */
+error_setg(errp, "IPv4 and IPv6 disabled");
 return -1;
 }
 
@@ -200,6 +203,7 @@ static int net_slirp_init(NetClientState *peer, const char 
*model,
 if (vnetwork) {
 if (get_str_sep(buf, sizeof(buf), , '/') < 0) {
 if (!inet_aton(vnetwork, )) {
+error_setg(errp, "Failed to parse netmask");
 return -1;
 }
 addr = ntohl(net.s_addr);
@@ -220,14 +224,19 @@ static int net_slirp_init(NetClientState *peer, const 
char *model,
 }
 } else {
 if (!inet_aton(buf, )) {
+error_setg(errp, "Failed to parse netmask");
 return -1;
 }
 shift = strtol(vnetwork, , 10);
 if (*end != '\0') {
 if (!inet_aton(vnetwork, )) {
+error_setg(errp,
+   "Failed to parse netmask (trailing chars)");
 return -1;
 }
 } else if (shift < 4 || shift > 32) {
+error_setg(errp,
+   "Invalid netmask provided (must be in range 4-32)");
 return -1;
 } else {
 mask.s_addr = htonl(0x << (32 - shift));
@@ -240,30 +249,43 @@ static int net_slirp_init(NetClientState *peer, const 
char *model,
 }
 
 if (vhost && !inet_aton(vhost, )) {
+error_setg(errp, "Failed to parse host");
 return -1;
 }
 if ((host.s_addr & mask.s_addr) != net.s_addr) {
+error_setg(errp, "Host doesn't belong to network");
 return -1;
 }
 
 if (vnameserver && !inet_aton(vnameserver, )) {
+error_setg(errp, "Failed to parse DNS");
 return -1;
 }
-if ((dns.s_addr & mask.s_addr) != net.s_addr ||
-dns.s_addr == host.s_addr) {
+if ((dns.s_addr & mask.s_addr) != net.s_addr) {
+error_setg(errp, "DNS doesn't belong to network");
+return -1;
+}
+if (dns.s_addr == host.s_addr) {
+error_setg(errp, "DNS must be different from host");
 return -1;
 }
 
 if (vdhcp_start && !inet_aton(vdhcp_start, )) {
+error_setg(errp, "Failed to parse DHCP start address");
 return -1;
 }
-if ((dhcp.s_addr & mask.s_addr) != net.s_addr ||
-dhcp.s_addr == host.s_addr || dhcp.s_addr == dns.s_addr) {
+if ((dhcp.s_addr & 

[Qemu-devel] [PULL 0/2] slirp updates

[Samuel Thibault]

warning: redirection vers https://people.debian.org/~sthibault/qemu.git/
The following changes since commit aaaec6acad7cf97372d48c1b09126a09697519c8:

  Update version for v2.10.0-rc1 release (2017-08-02 16:36:32 +0100)

are available in the git repository at:

  http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault

for you to fetch changes up to 2fe00b96b750f8c7a48dd3a98c5cb7370947264e:

  slirp: check len against dhcp options array end (2017-08-03 00:24:31 +0200)


slirp updates


Hervé Poussineau (1):
  slirp: fill error when failing to initialize user network

Prasad J Pandit (1):
  slirp: check len against dhcp options array end

 net/slirp.c   | 134 --
 slirp/bootp.c |   3 ++
 2 files changed, 97 insertions(+), 40 deletions(-)

[Qemu-devel] [PULL 2/2] slirp: check len against dhcp options array end

[Samuel Thibault]

From: Prasad J Pandit 

While parsing dhcp options string in 'dhcp_decode', if an options'
length 'len' appeared towards the end of 'bp_vend' array, ensuing
read could lead to an OOB memory access issue. Add check to avoid it.

Reported-by: Reno Robert 
Signed-off-by: Prasad J Pandit 
Signed-off-by: Samuel Thibault 
---
 slirp/bootp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/slirp/bootp.c b/slirp/bootp.c
index 5a4646c182..5dd1a415b5 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -123,6 +123,9 @@ static void dhcp_decode(const struct bootp_t *bp, int 
*pmsg_type,
 if (p >= p_end)
 break;
 len = *p++;
+if (p + len > p_end) {
+break;
+}
 DPRINTF("dhcp: tag=%d len=%d\n", tag, len);
 
 switch(tag) {
-- 
2.13.2

Re: [Qemu-devel] [PATCH 1/6] tests/tcg/test_path.c: include utils/bufferiszero.c

[Cleber Rosa]

On 08/02/2017 05:36 PM, Philippe Mathieu-Daudé wrote:
> Hi Cleber,
> 
> On 08/02/2017 05:15 PM, Cleber Rosa wrote:
>> Which contains one specific function used by iov.c.
>>
>> Without this, "make -C tests/tcg test_path" (and consequently
>> "make -C tests/tcg" or simply "make test") fails quite early.
>>
>> Signed-off-by: Cleber Rosa 
>> ---
>>   tests/tcg/test_path.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/tests/tcg/test_path.c b/tests/tcg/test_path.c
>> index 1c29bce..74dbdaf 100644
>> --- a/tests/tcg/test_path.c
>> +++ b/tests/tcg/test_path.c
>> @@ -3,6 +3,7 @@
>>   #include "util/cutils.c"
>>   #include "util/hexdump.c"
>>   #include "util/iov.c"
>> +#include "util/bufferiszero.c"
> 
> This fixes the build, however why not take this opportunity to fix it
> through the Makefile instead of including .c?
> 

Do you mean for all of the .c includes?  I just took a baby step, which
seemed more consistent with the mission (get it to build), without
changing a lot.

> Regards,
> 
> Phil.
> 

Thanks for the review!
- Cleber.

>>   #include "util/path.c"
>>   #include "util/qemu-timer-common.c"
>>   #include 
>>

-- 
Cleber Rosa
[ Sr Software Engineer - Virtualization Team - Red Hat ]
[ Avocado Test Framework - avocado-framework.github.io ]
[  7ABB 96EB 8B46 B94D 5E0F  E9BB 657E 8D33 A5F2 09F3  ]



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] test-qga: Kill broken and dead QGA_TEST_SIDE_EFFECTING code

[Marc-André Lureau]

- Original Message -
> Back when the test was introduced, in commit 62c39b307, the
> test was set up to run qemu-ga directly on the host performing
> the test, and defaults to limiting itself to safe commands.  At
> the time, it was envisioned that setting QGA_TEST_SIDE_EFFECTING
> in the environment could cover a few more commands, while noting
> the potential danger of those side effects running in the host.
> 
> But this has NEVER been tested: if you enable the environment
> variable, the test WILL fail.  One obvious reason: if you are not
> running as root, you'll probably get a permission failure when
> trying to freeze the file systems, or when changing system time.
> Less obvious: if you run the test as root (wow, you're brave), you
> could end up hanging if the test tries to log things to a
> temporarily frozen filesystem.  But the cutest reason of all: if
> you get past the above hurdles, the test uses invalid JSON in
> test_qga_fstrim() (missing '' around the dictionary key 'minimum'),
> and will thus fail an assertion in qmp_fd().
> 
> Rather than leave this untested time-bomb in place, rip it out.
> Hopefully, as originally envisioned, we can find an opportunity
> to test an actual sandboxed guest where the guest-agent has
> full permissions and will not unduly affect the host running
> the test - if so, 'git revert' can be used if desired, for
> salvaging any useful parts of this attempt.
> 
> Signed-off-by: Eric Blake 

I think it used to work in a vm (as qemu-ga user), but I don't mind to revert 
it if we need it back:

Signed-off-by: Marc-André Lureau 


> ---
>  tests/test-qga.c | 90
>  
>  1 file changed, 90 deletions(-)
> 
> diff --git a/tests/test-qga.c b/tests/test-qga.c
> index 06783e7585..fd6bc7690f 100644
> --- a/tests/test-qga.c
> +++ b/tests/test-qga.c
> @@ -642,65 +642,6 @@ static void test_qga_get_time(gconstpointer fix)
>  QDECREF(ret);
>  }
> 
> -static void test_qga_set_time(gconstpointer fix)
> -{
> -const TestFixture *fixture = fix;
> -QDict *ret;
> -int64_t current, time;
> -gchar *cmd;
> -
> -/* get current time */
> -ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-time'}");
> -g_assert_nonnull(ret);
> -qmp_assert_no_error(ret);
> -current = qdict_get_int(ret, "return");
> -g_assert_cmpint(current, >, 0);
> -QDECREF(ret);
> -
> -/* set some old time */
> -ret = qmp_fd(fixture->fd, "{'execute': 'guest-set-time',"
> - " 'arguments': { 'time': 1000 } }");
> -g_assert_nonnull(ret);
> -qmp_assert_no_error(ret);
> -QDECREF(ret);
> -
> -/* check old time */
> -ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-time'}");
> -g_assert_nonnull(ret);
> -qmp_assert_no_error(ret);
> -time = qdict_get_int(ret, "return");
> -g_assert_cmpint(time / 1000, <, G_USEC_PER_SEC * 10);
> -QDECREF(ret);
> -
> -/* set back current time */
> -cmd = g_strdup_printf("{'execute': 'guest-set-time',"
> -  " 'arguments': { 'time': %" PRId64 " } }",
> -  current + time * 1000);
> -ret = qmp_fd(fixture->fd, cmd);
> -g_free(cmd);
> -g_assert_nonnull(ret);
> -qmp_assert_no_error(ret);
> -QDECREF(ret);
> -}
> -
> -static void test_qga_fstrim(gconstpointer fix)
> -{
> -const TestFixture *fixture = fix;
> -QDict *ret;
> -QList *list;
> -const QListEntry *entry;
> -
> -ret = qmp_fd(fixture->fd, "{'execute': 'guest-fstrim',"
> - " arguments: { minimum: 4194304 } }");
> -g_assert_nonnull(ret);
> -qmp_assert_no_error(ret);
> -list = qdict_get_qlist(ret, "return");
> -entry = qlist_first(list);
> -g_assert(qdict_haskey(qobject_to_qdict(entry->value), "paths"));
> -
> -QDECREF(ret);
> -}
> -
>  static void test_qga_blacklist(gconstpointer data)
>  {
>  TestFixture fix;
> @@ -831,30 +772,6 @@ static void test_qga_fsfreeze_status(gconstpointer fix)
>  QDECREF(ret);
>  }
> 
> -static void test_qga_fsfreeze_and_thaw(gconstpointer fix)
> -{
> -const TestFixture *fixture = fix;
> -QDict *ret;
> -const gchar *status;
> -
> -ret = qmp_fd(fixture->fd, "{'execute': 'guest-fsfreeze-freeze'}");
> -g_assert_nonnull(ret);
> -qmp_assert_no_error(ret);
> -QDECREF(ret);
> -
> -ret = qmp_fd(fixture->fd, "{'execute': 'guest-fsfreeze-status'}");
> -g_assert_nonnull(ret);
> -qmp_assert_no_error(ret);
> -status = qdict_get_try_str(ret, "return");
> -g_assert_cmpstr(status, ==, "frozen");
> -QDECREF(ret);
> -
> -ret = qmp_fd(fixture->fd, "{'execute': 'guest-fsfreeze-thaw'}");
> -g_assert_nonnull(ret);
> -qmp_assert_no_error(ret);
> -QDECREF(ret);
> -}
> -
>  static void test_qga_guest_exec(gconstpointer fix)
>  {
>  const TestFixture *fixture = fix;
> @@ -1029,13 +946,6 @@ int main(int argc, char **argv)

Re: [Qemu-devel] How to make ELF headers/symbol sections available for multiboot?

[Anatol Pomozov]

Hello Richard

Thank you for this useful information. I still learning about ELF and
a lot of things are still unclear for me.

On Mon, Jul 31, 2017 at 11:20 AM, Richard Henderson  wrote:
> On 07/31/2017 10:21 AM, Anatol Pomozov wrote:
>> ELF sections info is needed for an OS to map address space properly.
>
> No, ELF *program header* info is needed for an OS to map the address space
> properly.  For example:
>
> $ readelf -hl vmlinux-4.9.0-3-5kc-malta

Thanks for this information about program headers. I reread elf_ops.h
and now I see that QEMU loads all PT_LOAD segments. I wonder why GRUB
bootloader loads also sections that are not in this segment (e.g. GRUB
loads content of .shstrtab into target's memory despite my elf does
not keep it in PT_LOAD segment).

What ELF specification says about it? Does it tell a loader to load
only PT_LOAD segments? In this case bootloaders should follow it as
well and current QEMU behavior is correct.

But multiboot expects section headers info and .shstrtab section to be
loaded to the target memory. I believe I need to modify my linker
script and add this information into the loadable segment.

Here is my current linker script:

==
ENTRY(start)

SECTIONS {
. = 1M;

.text : {
KEEP(*(.data.multiboot))
*(.text .text*)
}

.rodata : {
*(.rodata .rodata*)
}

.data : {
*(.data .data.*)
}

.bss : {
__bss_start = .;
*(.bss .bss*)
. = ALIGN(8);
__bss_end = .;
}
}
===

With my linker script only .text .rodata .data and .bss are included
into the loadable segment.


So my questions: how to tell the linker to include "section headers"
and ".shstrtab" section into loadable segment? Once it is done I can
try to modify QEMU to pass its addresses to the target.

>
> Using a mips kernel binary I happend to have handy; it would be the same for
> x86_64, prior to being bundled in the (imo superfluous bzImage) format.
>
> ELF Header:
>   Magic:   7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00
>   Class: ELF64
>   Data:  2's complement, little endian
>   Version:   1 (current)
>   OS/ABI:UNIX - System V
>   ABI Version:   0
>   Type:  EXEC (Executable file)
>   Machine:   MIPS R3000
>   Version:   0x1
>   Entry point address:   0x806ed590
>   Start of program headers:  64 (bytes into file)
>   Start of section headers:  13351328 (bytes into file)
>   Flags: 0x8001, noreorder, mips64r2
>   Size of this header:   64 (bytes)
>   Size of program headers:   56 (bytes)
>   Number of program headers: 2
>   Size of section headers:   64 (bytes)
>   Number of section headers: 28
>   Section header string table index: 27
>
> The ELF file header, always at file offset 0.  The relevant info is the
> encoding (64-bit little-endian), cpu (mips), and start of program headers.
>
> Program Headers:
>   Type   Offset VirtAddr   PhysAddr
>  FileSizMemSiz  Flags  Align
>   LOAD   0x1000 0x8010 0x8010
>  0x009fefb4 0x00a5a150  RWE1000
>   NOTE   0x005fecb0 0x806fdcb0 0x806fdcb0
>  0x0024 0x0024  R  4
>
> The ELF program header.  There is one segment to be loaded, at a given 
> physical
> address (which happens to match the virtual address, but that need not have
> been so).
>
> The segment consists of 0x9fefb4 bytes of data to be loaded from the file, and
> occupies 0xa5a150 in memory.  The difference between the two sizes is "bss",
> and should be zeroed.
>
> A proper ELF loader will process *all* LOAD segments, however many are 
> required
> by the binary.  Though in practice there will probably only be 1 or 2.
>
> Section to Segment mapping:
>   Segment Sections...
>   00 .text __ex_table __dbe_table .notes .rodata .pci_fixup __ksymtab
> __ksymtab_gpl __kcrctab __kcrctab_gpl __ksymtab_strings __param __modver .data
> .data..page_aligned .init.text .init.data .exit.text .data.reloc .sbss .bss
>   01 __dbe_table .notes
>
> This mapping is provided by readelf for convenience, not actually present in
> the ELF file.  But it is handy when debugging a linker script.
>
>> Quoting Multiboot specification
>> https://www.gnu.org/software/grub/manual/multiboot/multiboot.html
>
> I'm not sure why someone felt the need to re-invent the wheel, especially
> considering its introduction section talks about trying to stop reinventing 
> the
> wheel...
>
> But... whatever.  I'm not sure why this is relevant to $SUBJECT, since it does

Re: [Qemu-devel] [Qemu-arm] [PATCH 01/15] target/arm: Use MMUAccessType enum rather than int

[Philippe Mathieu-Daudé]

On 08/02/2017 01:43 PM, Peter Maydell wrote:

In the ARM get_phys_addr() code, switch to using the MMUAccessType
enum and its MMU_* values rather than int and literal 0/1/2.

Signed-off-by: Peter Maydell 


Reviewed-by: Philippe Mathieu-Daudé 


---
  target/arm/helper.c| 30 +++---
  target/arm/internals.h |  3 ++-
  2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index fa60040..b78d277 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -20,13 +20,13 @@
  
  #ifndef CONFIG_USER_ONLY

  static bool get_phys_addr(CPUARMState *env, target_ulong address,
-  int access_type, ARMMMUIdx mmu_idx,
+  MMUAccessType access_type, ARMMMUIdx mmu_idx,
hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
target_ulong *page_size, uint32_t *fsr,
ARMMMUFaultInfo *fi);
  
  static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,

-   int access_type, ARMMMUIdx mmu_idx,
+   MMUAccessType access_type, ARMMMUIdx mmu_idx,
 hwaddr *phys_ptr, MemTxAttrs *txattrs, int 
*prot,
 target_ulong *page_size_ptr, uint32_t *fsr,
 ARMMMUFaultInfo *fi);
@@ -2135,7 +2135,7 @@ static CPAccessResult ats_access(CPUARMState *env, const 
ARMCPRegInfo *ri,
  }
  
  static uint64_t do_ats_write(CPUARMState *env, uint64_t value,

- int access_type, ARMMMUIdx mmu_idx)
+ MMUAccessType access_type, ARMMMUIdx mmu_idx)
  {
  hwaddr phys_addr;
  target_ulong page_size;
@@ -2194,7 +2194,7 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t 
value,
  
  static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)

  {
-int access_type = ri->opc2 & 1;
+MMUAccessType access_type = ri->opc2 & 1 ? MMU_DATA_STORE : MMU_DATA_LOAD;
  uint64_t par64;
  ARMMMUIdx mmu_idx;
  int el = arm_current_el(env);
@@ -2253,7 +2253,7 @@ static void ats_write(CPUARMState *env, const 
ARMCPRegInfo *ri, uint64_t value)
  static void ats1h_write(CPUARMState *env, const ARMCPRegInfo *ri,
  uint64_t value)
  {
-int access_type = ri->opc2 & 1;
+MMUAccessType access_type = ri->opc2 & 1 ? MMU_DATA_STORE : MMU_DATA_LOAD;
  uint64_t par64;
  
  par64 = do_ats_write(env, value, access_type, ARMMMUIdx_S2NS);

@@ -2273,7 +2273,7 @@ static CPAccessResult at_s1e2_access(CPUARMState *env, 
const ARMCPRegInfo *ri,
  static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri,
  uint64_t value)
  {
-int access_type = ri->opc2 & 1;
+MMUAccessType access_type = ri->opc2 & 1 ? MMU_DATA_STORE : MMU_DATA_LOAD;
  ARMMMUIdx mmu_idx;
  int secure = arm_is_secure_below_el3(env);
  
@@ -7510,7 +7510,7 @@ static uint64_t arm_ldq_ptw(CPUState *cs, hwaddr addr, bool is_secure,

  }
  
  static bool get_phys_addr_v5(CPUARMState *env, uint32_t address,

- int access_type, ARMMMUIdx mmu_idx,
+ MMUAccessType access_type, ARMMMUIdx mmu_idx,
   hwaddr *phys_ptr, int *prot,
   target_ulong *page_size, uint32_t *fsr,
   ARMMMUFaultInfo *fi)
@@ -7626,7 +7626,7 @@ do_fault:
  }
  
  static bool get_phys_addr_v6(CPUARMState *env, uint32_t address,

- int access_type, ARMMMUIdx mmu_idx,
+ MMUAccessType access_type, ARMMMUIdx mmu_idx,
   hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
   target_ulong *page_size, uint32_t *fsr,
   ARMMMUFaultInfo *fi)
@@ -7733,7 +7733,7 @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t 
address,
  if (pxn && !regime_is_user(env, mmu_idx)) {
  xn = 1;
  }
-if (xn && access_type == 2)
+if (xn && access_type == MMU_INST_FETCH)
  goto do_fault;
  
  if (arm_feature(env, ARM_FEATURE_V6K) &&

@@ -7848,7 +7848,7 @@ static bool check_s2_mmu_setup(ARMCPU *cpu, bool is_aa64, 
int level,
  }
  
  static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,

-   int access_type, ARMMMUIdx mmu_idx,
+   MMUAccessType access_type, ARMMMUIdx mmu_idx,
 hwaddr *phys_ptr, MemTxAttrs *txattrs, int 
*prot,
 target_ulong *page_size_ptr, uint32_t *fsr,
 ARMMMUFaultInfo *fi)
@@ -8256,7 +8256,7 @@ static inline bool m_is_system_region(CPUARMState *env, 
uint32_t address)
  }
  
  static bool 

Re: [Qemu-devel] [PATCH 03/15] target/arm: Consolidate PMSA handling in get_phys_addr()

[Philippe Mathieu-Daudé]

On 08/02/2017 01:43 PM, Peter Maydell wrote:

Currently get_phys_addr() has PMSAv7 handling before the
"is translation disabled?" check, and then PMSAv5 after it.
Tidy this up by making the PMSAv5 code handle the "MPU disabled"
case itself, so that we have all the PMSA code in one place.
This will make adding the PMSAv8 code slightly cleaner, and
also means that pre-v7 PMSA cores benefit from the MPU lookup
logging that the PMSAv7 codepath had.

Signed-off-by: Peter Maydell 


Reviewed-by: Philippe Mathieu-Daudé 


---
  target/arm/helper.c | 38 ++
  1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index b78d277..fd83a21 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8423,6 +8423,13 @@ static bool get_phys_addr_pmsav5(CPUARMState *env, 
uint32_t address,
  uint32_t base;
  bool is_user = regime_is_user(env, mmu_idx);
  
+if (regime_translation_disabled(env, mmu_idx)) {

+/* MPU disabled.  */
+*phys_ptr = address;
+*prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
+return false;
+}
+
  *phys_ptr = address;
  for (n = 7; n >= 0; n--) {
  base = env->cp15.c6_region[n];
@@ -8572,16 +8579,20 @@ static bool get_phys_addr(CPUARMState *env, 
target_ulong address,
  }
  }
  
-/* pmsav7 has special handling for when MPU is disabled so call it before

- * the common MMU/MPU disabled check below.
- */
-if (arm_feature(env, ARM_FEATURE_PMSA) &&
-arm_feature(env, ARM_FEATURE_V7)) {
+if (arm_feature(env, ARM_FEATURE_PMSA)) {
  bool ret;
  *page_size = TARGET_PAGE_SIZE;
-ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx,
-   phys_ptr, prot, fsr);
-qemu_log_mask(CPU_LOG_MMU, "PMSAv7 MPU lookup for %s at 0x%08" PRIx32
+
+if (arm_feature(env, ARM_FEATURE_V7)) {
+/* PMSAv7 */
+ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx,
+   phys_ptr, prot, fsr);
+} else {
+/* Pre-v7 MPU */
+ret = get_phys_addr_pmsav5(env, address, access_type, mmu_idx,
+   phys_ptr, prot, fsr);
+}
+qemu_log_mask(CPU_LOG_MMU, "PMSA MPU lookup for %s at 0x%08" PRIx32
" mmu_idx %u -> %s (prot %c%c%c)\n",
access_type == MMU_DATA_LOAD ? "reading" :
(access_type == MMU_DATA_STORE ? "writing" : "execute"),
@@ -8594,21 +8605,16 @@ static bool get_phys_addr(CPUARMState *env, 
target_ulong address,
  return ret;
  }
  
+/* Definitely a real MMU, not an MPU */

+
  if (regime_translation_disabled(env, mmu_idx)) {
-/* MMU/MPU disabled.  */
+/* MMU disabled. */
  *phys_ptr = address;
  *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
  *page_size = TARGET_PAGE_SIZE;
  return 0;
  }
  
-if (arm_feature(env, ARM_FEATURE_PMSA)) {

-/* Pre-v7 MPU */
-*page_size = TARGET_PAGE_SIZE;
-return get_phys_addr_pmsav5(env, address, access_type, mmu_idx,
-phys_ptr, prot, fsr);
-}
-
  if (regime_using_lpae_format(env, mmu_idx)) {
  return get_phys_addr_lpae(env, address, access_type, mmu_idx, 
phys_ptr,
attrs, prot, page_size, fsr, fi);

Re: [Qemu-devel] [Qemu-arm] [PATCH 14/15] armv7m_nvic.h: Move from include/hw/arm to include/hw/intc

[Philippe Mathieu-Daudé]

On 08/02/2017 01:44 PM, Peter Maydell wrote:

The armv7m_nvic.h header file was accidentally placed in
include/hw/arm; move it to include/hw/intc to match where
its corresponding .c file lives.

Signed-off-by: Peter Maydell 


Reviewed-by: Philippe Mathieu-Daudé 


---
  hw/intc/armv7m_nvic.c  | 2 +-
  include/hw/arm/armv7m.h| 2 +-
  include/hw/{arm => intc}/armv7m_nvic.h | 0
  3 files changed, 2 insertions(+), 2 deletions(-)
  rename include/hw/{arm => intc}/armv7m_nvic.h (100%)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 343bc16..5a18025 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -17,7 +17,7 @@
  #include "hw/sysbus.h"
  #include "qemu/timer.h"
  #include "hw/arm/arm.h"
-#include "hw/arm/armv7m_nvic.h"
+#include "hw/intc/armv7m_nvic.h"
  #include "target/arm/cpu.h"
  #include "exec/exec-all.h"
  #include "qemu/log.h"
diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
index a9b3f2a..10eb058 100644
--- a/include/hw/arm/armv7m.h
+++ b/include/hw/arm/armv7m.h
@@ -11,7 +11,7 @@
  #define HW_ARM_ARMV7M_H
  
  #include "hw/sysbus.h"

-#include "hw/arm/armv7m_nvic.h"
+#include "hw/intc/armv7m_nvic.h"
  
  #define TYPE_BITBAND "ARM,bitband-memory"

  #define BITBAND(obj) OBJECT_CHECK(BitBandState, (obj), TYPE_BITBAND)
diff --git a/include/hw/arm/armv7m_nvic.h b/include/hw/intc/armv7m_nvic.h
similarity index 100%
rename from include/hw/arm/armv7m_nvic.h
rename to include/hw/intc/armv7m_nvic.h

Re: [Qemu-devel] [Qemu-arm] [PATCH 13/15] target/arm: Create and use new function arm_v7m_is_handler_mode()

[Philippe Mathieu-Daudé]

On 08/02/2017 01:43 PM, Peter Maydell wrote:

Add a utility function for testing whether the CPU is in Handler
mode; this is just a check whether v7m.exception is non-zero, but
we do it in several places and it makes the code a bit easier
to read to not have to mentally figure out what the test is testing.


<3 <3 <3



Signed-off-by: Peter Maydell 


Reviewed-by: Philippe Mathieu-Daudé 


---
  target/arm/cpu.h| 10 --
  target/arm/helper.c |  8 
  2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index da90b7a..a3b4b78 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1630,13 +1630,19 @@ static inline int arm_highest_el(CPUARMState *env)
  return 1;
  }
  
+/* Return true if a v7M CPU is in Handler mode */

+static inline bool arm_v7m_is_handler_mode(CPUARMState *env)
+{
+return env->v7m.exception != 0;
+}
+
  /* Return the current Exception Level (as per ARMv8; note that this differs
   * from the ARMv7 Privilege Level).
   */
  static inline int arm_current_el(CPUARMState *env)
  {
  if (arm_feature(env, ARM_FEATURE_M)) {
-return !((env->v7m.exception == 0) && (env->v7m.control & 1));
+return arm_v7m_is_handler_mode(env) || !(env->v7m.control & 1);
  }
  
  if (is_a64(env)) {

@@ -2636,7 +2642,7 @@ static inline void cpu_get_tb_cpu_state(CPUARMState *env, 
target_ulong *pc,
  }
  *flags |= fp_exception_el(env) << ARM_TBFLAG_FPEXC_EL_SHIFT;
  
-if (env->v7m.exception != 0) {

+if (arm_v7m_is_handler_mode(env)) {
  *flags |= ARM_TBFLAG_HANDLER_MASK;
  }
  
diff --git a/target/arm/helper.c b/target/arm/helper.c

index 0ecc8f1..7920153 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6147,7 +6147,7 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
   * that jumps to magic addresses don't have magic behaviour unless
   * we're in Handler mode (compare pseudocode BXWritePC()).
   */
-assert(env->v7m.exception != 0);
+assert(arm_v7m_is_handler_mode(env));
  
  /* In the spec pseudocode ExceptionReturn() is called directly

   * from BXWritePC() and gets the full target PC value including
@@ -6254,7 +6254,7 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
   * resuming in Thread mode. If that doesn't match what the
   * exception return type specified then this is a UsageFault.
   */
-if (return_to_handler == (env->v7m.exception == 0)) {
+if (return_to_handler != arm_v7m_is_handler_mode(env)) {
  /* Take an INVPC UsageFault by pushing the stack again. */
  armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE);
  env->v7m.cfsr |= R_V7M_CFSR_INVPC_MASK;
@@ -6405,7 +6405,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
  if (env->v7m.control & R_V7M_CONTROL_SPSEL_MASK) {
  lr |= 4;
  }
-if (env->v7m.exception == 0) {
+if (!arm_v7m_is_handler_mode(env)) {
  lr |= 8;
  }
  
@@ -8798,7 +8798,7 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val)

   * switch_v7m_sp() deals with updating the SPSEL bit in
   * env->v7m.control, so we only need update the others.
   */
-if (env->v7m.exception == 0) {
+if (!arm_v7m_is_handler_mode(env)) {
  switch_v7m_sp(env, (val & R_V7M_CONTROL_SPSEL_MASK) != 0);
  }
  env->v7m.control &= ~R_V7M_CONTROL_NPRIV_MASK;

Re: [Qemu-devel] [Qemu-arm] [PATCH 12/15] target/arm: Don't calculate lr in arm_v7m_cpu_do_interrupt() until needed

[Philippe Mathieu-Daudé]

On 08/02/2017 01:43 PM, Peter Maydell wrote:

Move the code in arm_v7m_cpu_do_interrupt() that calculates the
magic LR value down to when we're actually going to use it.
Having the calculation and use so far apart makes the code
a little harder to understand than it needs to be.

Signed-off-by: Peter Maydell 


Reviewed-by: Philippe Mathieu-Daudé 


---
  target/arm/helper.c | 15 ---
  1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index b64ddb1..0ecc8f1 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6311,13 +6311,6 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
  
  arm_log_exception(cs->exception_index);
  
-lr = 0xfff1;

-if (env->v7m.control & R_V7M_CONTROL_SPSEL_MASK) {
-lr |= 4;
-}
-if (env->v7m.exception == 0)
-lr |= 8;
-
  /* For exceptions we just mark as pending on the NVIC, and let that
 handle it.  */
  switch (cs->exception_index) {
@@ -6408,6 +6401,14 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
  return; /* Never happens.  Keep compiler happy.  */
  }
  
+lr = 0xfff1;

+if (env->v7m.control & R_V7M_CONTROL_SPSEL_MASK) {
+lr |= 4;
+}
+if (env->v7m.exception == 0) {
+lr |= 8;
+}
+
  v7m_push_stack(cpu);
  v7m_exception_taken(cpu, lr);
  qemu_log_mask(CPU_LOG_INT, "... as %d\n", env->v7m.exception);

Re: [Qemu-devel] [PATCH for-2.10 2/3] target/mips: Drop redundant gen_io_start/stop()

[Philippe Mathieu-Daudé]

On 08/02/2017 06:59 AM, James Hogan wrote:

DMTC0 CP0_Cause does a redundant gen_io_start() and gen_io_end() pair,
even though this is done for all DMTC0 operations outside of the switch
statement. Remove these redundant calls.

Fixes: 5dc5d9f055c5 ("mips: more fixes to the MIPS interrupt glue logic")
Signed-off-by: James Hogan 


Reviewed-by: Philippe Mathieu-Daudé 


Cc: Yongbok Kim 
Cc: Aurelien Jarno 
---
  target/mips/translate.c | 8 
  1 file changed, 0 insertions(+), 8 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 6b41f7b65e00..6e724ac71dcd 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -7403,15 +7403,7 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int 
reg, int sel)
  switch (sel) {
  case 0:
  save_cpu_state(ctx, 1);
-/* Mark as an IO operation because we may trigger a software
-   interrupt.  */
-if (ctx->tb->cflags & CF_USE_ICOUNT) {
-gen_io_start();
-}
  gen_helper_mtc0_cause(cpu_env, arg);
-if (ctx->tb->cflags & CF_USE_ICOUNT) {
-gen_io_end();
-}
  /* Stop translation as we may have triggered an intetrupt. BS_STOP
   * isn't sufficient, we need to ensure we break out of translated
   * code to check for pending interrupts.  */

Re: [Qemu-devel] [PATCH 3/6] tests/tcg/linux-test.c: include definitions for getrusage()

[Philippe Mathieu-Daudé]

On Wed, Aug 2, 2017 at 6:28 PM, Philippe Mathieu-Daudé  wrote:
> Hi Cleber,
>
> On 08/02/2017 05:15 PM, Cleber Rosa wrote:
>>
>> A include for  is missing, and prevents
>> tests/tcg/linux-test from compiling.
>
>
> getrusage() I presume, don't know if worth adding in commit message.

Sorry I missed it from the commit subject :/

>
>>
>> Signed-off-by: Cleber Rosa 
>
>
> Reviewed-by: Philippe Mathieu-Daudé 
>
>
>> ---
>>   tests/tcg/linux-test.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/tests/tcg/linux-test.c b/tests/tcg/linux-test.c
>> index 1c6c013..15c9d7f 100644
>> --- a/tests/tcg/linux-test.c
>> +++ b/tests/tcg/linux-test.c
>> @@ -39,6 +39,7 @@
>>   #include 
>>   #include 
>>   #include 
>> +#include 
>> #define TESTPATH "/tmp/linux-test.tmp"
>>   #define TESTPORT 7654
>>
>

Re: [Qemu-devel] [PATCH 1/6] tests/tcg/test_path.c: include utils/bufferiszero.c

[Philippe Mathieu-Daudé]

Hi Cleber,

On 08/02/2017 05:15 PM, Cleber Rosa wrote:

Which contains one specific function used by iov.c.

Without this, "make -C tests/tcg test_path" (and consequently
"make -C tests/tcg" or simply "make test") fails quite early.

Signed-off-by: Cleber Rosa 
---
  tests/tcg/test_path.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/tests/tcg/test_path.c b/tests/tcg/test_path.c
index 1c29bce..74dbdaf 100644
--- a/tests/tcg/test_path.c
+++ b/tests/tcg/test_path.c
@@ -3,6 +3,7 @@
  #include "util/cutils.c"
  #include "util/hexdump.c"
  #include "util/iov.c"
+#include "util/bufferiszero.c"


This fixes the build, however why not take this opportunity to fix it 
through the Makefile instead of including .c?


Regards,

Phil.


  #include "util/path.c"
  #include "util/qemu-timer-common.c"
  #include 

Re: [Qemu-devel] [PATCH] cpu: don't allow negative core id

[Eduardo Habkost]

On Wed, Aug 02, 2017 at 03:50:36PM +0200, Laurent Vivier wrote:
> On 02/08/2017 15:42, Philippe Mathieu-Daudé wrote:
> > Hi Laurent,
> > 
> > On Wed, Aug 2, 2017 at 7:32 AM, Laurent Vivier  wrote:
> >> With pseries machine type a negative core-id is not managed properly:
> >> -1 gives an inaccurate error message ("core -1 already populated"),
> >> -2 crashes QEMU (core dump)
> >>
> >> As it seems a negative value is invalid for any architecture,
> >> instead of checking this in spapr_core_pre_plug() I think it's better
> >> to check this in the generic part, core_prop_set_core_id()
> > 
> > Why is this property signed? If there is not reason to use it negative,
> > is it possible to use object_property_add(.."uint"..)?
> 
> You should be right:
> 
> { 'struct': 'NumaNodeOptions',
>   'data': {
>'*nodeid': 'uint16',
>'*cpus':   ['uint16'],
>'*mem':'size',
>'*memdev': 'str' }}
> 
> but
> 
> { 'struct': 'CpuInstanceProperties',
>   'data': { '*node-id': 'int',
> '*socket-id': 'int',
> '*core-id': 'int',
> '*thread-id': 'int'
>   }
> }
> 
> But I'm not sure it's a good idea to change the API now.

Property parsing is not affected by the QAPI schema at all, so
touching the schema wouldn't fix the bug.

The same applies to the 'type' argument to object_property_add():
it is ignored everywhere.

However, the property setter can simply use a visitor for unsigned values, and
it will reject negative values automatically, e.g.:

  diff --git a/hw/cpu/core.c b/hw/cpu/core.c
  index 2bf960d..b5af2bf 100644
  --- a/hw/cpu/core.c
  +++ b/hw/cpu/core.c
  @@ -25,9 +25,9 @@ static void core_prop_set_core_id(Object *obj, Visitor *v, 
const char *name,
   {
   CPUCore *core = CPU_CORE(obj);
   Error *local_err = NULL;
  -int64_t value;
  +uint32_t value;
   
  -visit_type_int(v, name, , _err);
  +visit_type_uint32(v, name, , _err);
   if (local_err) {
   error_propagate(errp, local_err);
   return;


  $ ./ppc64-softmmu/qemu-system-ppc64 -device 
POWER8_v2.0-spapr-cpu-core,core-id=-2
  qemu-system-ppc64: -device POWER8_v2.0-spapr-cpu-core,core-id=-2: Parameter 
'core-id' expects uint32_t


I would suggest changing the CPUCore struct fields to uint32_t or
uint64_t, but it would be more intrusive and we're past hard
freeze.  Your patch looks good for 2.10.

Reviewed-by: Eduardo Habkost 

I'm queueing it on machine-next.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 3/6] tests/tcg/linux-test.c: include definitions for getrusage()

[Philippe Mathieu-Daudé]

Hi Cleber,

On 08/02/2017 05:15 PM, Cleber Rosa wrote:

A include for  is missing, and prevents
tests/tcg/linux-test from compiling.


getrusage() I presume, don't know if worth adding in commit message.



Signed-off-by: Cleber Rosa 


Reviewed-by: Philippe Mathieu-Daudé 


---
  tests/tcg/linux-test.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/tests/tcg/linux-test.c b/tests/tcg/linux-test.c
index 1c6c013..15c9d7f 100644
--- a/tests/tcg/linux-test.c
+++ b/tests/tcg/linux-test.c
@@ -39,6 +39,7 @@
  #include 
  #include 
  #include 
+#include 
  
  #define TESTPATH "/tmp/linux-test.tmp"

  #define TESTPORT 7654

Re: [Qemu-devel] [PATCH] firmware: add const to bin_attribute structures

[Michael S. Tsirkin]

On Wed, Aug 02, 2017 at 02:11:35PM +0530, Bhumika Goyal wrote:
> Add const to bin_attribute structures as they are only passed to the
> functions sysfs_{remove/create}_bin_file. The arguments passed are of
> type const, so declare the structures to be const.
> 
> Done using Coccinelle.
> 
> @m disable optional_qualifier@
> identifier s;
> position p;
> @@
> static struct bin_attribute s@p={...};
> 
> @okay1@
> position p;
> identifier m.s;
> @@
> (
> sysfs_create_bin_file(...,@p,...)
> |
> sysfs_remove_bin_file(...,@p,...)
> )
> 
> @bad@
> position p!={m.p,okay1.p};
> identifier m.s;
> @@
> s@p
> 
> @change depends on !bad disable optional_qualifier@
> identifier m.s;
> @@
> static
> +const
> struct bin_attribute s={...};
> 
> Signed-off-by: Bhumika Goyal 

For qemu bits:

Acked-by: Michael S. Tsirkin 

Feel free to merge.,

> ---
>  drivers/firmware/dell_rbu.c  | 6 +++---
>  drivers/firmware/dmi-sysfs.c | 2 +-
>  drivers/firmware/google/gsmi.c   | 2 +-
>  drivers/firmware/google/memconsole.c | 2 +-
>  drivers/firmware/qemu_fw_cfg.c   | 2 +-
>  5 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/firmware/dell_rbu.c b/drivers/firmware/dell_rbu.c
> index 2f452f1..a771360 100644
> --- a/drivers/firmware/dell_rbu.c
> +++ b/drivers/firmware/dell_rbu.c
> @@ -675,18 +675,18 @@ static ssize_t write_rbu_packet_size(struct file *filp, 
> struct kobject *kobj,
>   return count;
>  }
>  
> -static struct bin_attribute rbu_data_attr = {
> +static const struct bin_attribute rbu_data_attr = {
>   .attr = {.name = "data", .mode = 0444},
>   .read = read_rbu_data,
>  };
>  
> -static struct bin_attribute rbu_image_type_attr = {
> +static const struct bin_attribute rbu_image_type_attr = {
>   .attr = {.name = "image_type", .mode = 0644},
>   .read = read_rbu_image_type,
>   .write = write_rbu_image_type,
>  };
>  
> -static struct bin_attribute rbu_packet_size_attr = {
> +static const struct bin_attribute rbu_packet_size_attr = {
>   .attr = {.name = "packet_size", .mode = 0644},
>   .read = read_rbu_packet_size,
>   .write = write_rbu_packet_size,
> diff --git a/drivers/firmware/dmi-sysfs.c b/drivers/firmware/dmi-sysfs.c
> index d5de6ee..936e656 100644
> --- a/drivers/firmware/dmi-sysfs.c
> +++ b/drivers/firmware/dmi-sysfs.c
> @@ -440,7 +440,7 @@ static ssize_t dmi_sel_raw_read(struct file *filp, struct 
> kobject *kobj,
>   return find_dmi_entry(entry, dmi_sel_raw_read_helper, );
>  }
>  
> -static struct bin_attribute dmi_sel_raw_attr = {
> +static const struct bin_attribute dmi_sel_raw_attr = {
>   .attr = {.name = "raw_event_log", .mode = 0400},
>   .read = dmi_sel_raw_read,
>  };
> diff --git a/drivers/firmware/google/gsmi.c b/drivers/firmware/google/gsmi.c
> index c8f169b..1ad29f7 100644
> --- a/drivers/firmware/google/gsmi.c
> +++ b/drivers/firmware/google/gsmi.c
> @@ -508,7 +508,7 @@ static ssize_t eventlog_write(struct file *filp, struct 
> kobject *kobj,
>  
>  }
>  
> -static struct bin_attribute eventlog_bin_attr = {
> +static const struct bin_attribute eventlog_bin_attr = {
>   .attr = {.name = "append_to_eventlog", .mode = 0200},
>   .write = eventlog_write,
>  };
> diff --git a/drivers/firmware/google/memconsole.c 
> b/drivers/firmware/google/memconsole.c
> index 166f07c..b11a02c 100644
> --- a/drivers/firmware/google/memconsole.c
> +++ b/drivers/firmware/google/memconsole.c
> @@ -33,7 +33,7 @@ static ssize_t memconsole_read(struct file *filp, struct 
> kobject *kobp,
>   return memconsole_read_func(buf, pos, count);
>  }
>  
> -static struct bin_attribute memconsole_bin_attr = {
> +static const struct bin_attribute memconsole_bin_attr = {
>   .attr = {.name = "log", .mode = 0444},
>   .read = memconsole_read,
>  };
> diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
> index 0e20116..f254977 100644
> --- a/drivers/firmware/qemu_fw_cfg.c
> +++ b/drivers/firmware/qemu_fw_cfg.c
> @@ -343,7 +343,7 @@ static ssize_t fw_cfg_sysfs_read_raw(struct file *filp, 
> struct kobject *kobj,
>   return count;
>  }
>  
> -static struct bin_attribute fw_cfg_sysfs_attr_raw = {
> +static const struct bin_attribute fw_cfg_sysfs_attr_raw = {
>   .attr = { .name = "raw", .mode = S_IRUSR },
>   .read = fw_cfg_sysfs_read_raw,
>  };
> -- 
> 1.9.1

Re: [Qemu-devel] [PATCH] docs/pcie.txt: Replace ioh3420 with pcie-root-port

[Michael S. Tsirkin]

On Wed, Aug 02, 2017 at 06:51:13PM +0300, Marcel Apfelbaum wrote:
> Do not mention ioh3420 in the "how to" doc.
> The device still works and can be used by already
> existing setups, but no need to be mentioned.
> 
> Suggested-by: Andrew Jones 
> Signed-off-by: Marcel Apfelbaum 
> ---

Do we need this in 2.10? I'm inclined to say no, it can wait
until 2.11. Thoughts?

>  docs/pcie.txt | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/docs/pcie.txt b/docs/pcie.txt
> index 5bada24..f990033 100644
> --- a/docs/pcie.txt
> +++ b/docs/pcie.txt
> @@ -43,8 +43,8 @@ Place only the following kinds of devices directly on the 
> Root Complex:
>  strangely when PCI Express devices are integrated
>  with the Root Complex.
>  
> -(2) PCI Express Root Ports (ioh3420), for starting exclusively PCI 
> Express
> -hierarchies.
> +(2) PCI Express Root Ports (pcie-root-port), for starting exclusively
> +PCI Express hierarchies.
>  
>  (3) DMI-PCI Bridges (i82801b11-bridge), for starting legacy PCI
>  hierarchies.
> @@ -65,7 +65,7 @@ Place only the following kinds of devices directly on the 
> Root Complex:
>-device pxb-pcie,id=pcie.1,bus_nr=x[,numa_node=y][,addr=z]
>Only PCI Express Root Ports and DMI-PCI bridges can be connected
>to the pcie.1 bus:
> -  -device 
> ioh3420,id=root_port1[,bus=pcie.1][,chassis=x][,slot=y][,addr=z]  
>\
> +  -device 
> pcie-root-port,id=root_port1[,bus=pcie.1][,chassis=x][,slot=y][,addr=z]   
>   \
>-device i82801b11-bridge,id=dmi_pci_bridge1,bus=pcie.1
>  
>  
> @@ -107,14 +107,14 @@ Plug only PCI Express devices into PCI Express Ports.
>   
>  
>  2.2.1 Plugging a PCI Express device into a PCI Express Root Port:
> -  -device 
> ioh3420,id=root_port1,chassis=x,slot=y[,bus=pcie.0][,addr=z]  \
> +  -device 
> pcie-root-port,id=root_port1,chassis=x,slot=y[,bus=pcie.0][,addr=z]  \
>-device ,bus=root_port1
>  2.2.2 Using multi-function PCI Express Root Ports:
> -  -device 
> ioh3420,id=root_port1,multifunction=on,chassis=x,addr=z.0[,slot=y][,bus=pcie.0]
>  \
> -  -device 
> ioh3420,id=root_port2,chassis=x1,addr=z.1[,slot=y1][,bus=pcie.0] \
> -  -device 
> ioh3420,id=root_port3,chassis=x2,addr=z.2[,slot=y2][,bus=pcie.0] \
> +  -device 
> pcie-root-port,id=root_port1,multifunction=on,chassis=x,addr=z.0[,slot=y][,bus=pcie.0]
>  \
> +  -device 
> pcie-root-port,id=root_port2,chassis=x1,addr=z.1[,slot=y1][,bus=pcie.0] \
> +  -device 
> pcie-root-port,id=root_port3,chassis=x2,addr=z.2[,slot=y2][,bus=pcie.0] \
>  2.2.3 Plugging a PCI Express device into a Switch:
> -  -device ioh3420,id=root_port1,chassis=x,slot=y[,bus=pcie.0][,addr=z]  \
> +  -device 
> pcie-root-port,id=root_port1,chassis=x,slot=y[,bus=pcie.0][,addr=z]  \
>-device x3130-upstream,id=upstream_port1,bus=root_port1[,addr=x]   
>\
>-device 
> xio3130-downstream,id=downstream_port1,bus=upstream_port1,chassis=x1,slot=y1[,addr=z1]]
>  \
>-device ,bus=downstream_port1
> -- 
> 2.9.4

Re: [Qemu-devel] [PATCH for-2.10?] test-qga: Kill broken and dead QGA_TEST_SIDE_EFFECTING code

[Eric Blake]

On 08/02/2017 03:19 PM, Eric Blake wrote:
> Back when the test was introduced, in commit 62c39b307, the
> test was set up to run qemu-ga directly on the host performing
> the test, and defaults to limiting itself to safe commands.  At
> the time, it was envisioned that setting QGA_TEST_SIDE_EFFECTING
> in the environment could cover a few more commands, while noting
> the potential danger of those side effects running in the host.

> Rather than leave this untested time-bomb in place, rip it out.

Oh, I meant to add that I'm wondering if this should be considered 2.10
material.

-- 
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] test-qga: Kill broken and dead QGA_TEST_SIDE_EFFECTING code

[Eric Blake]

Back when the test was introduced, in commit 62c39b307, the
test was set up to run qemu-ga directly on the host performing
the test, and defaults to limiting itself to safe commands.  At
the time, it was envisioned that setting QGA_TEST_SIDE_EFFECTING
in the environment could cover a few more commands, while noting
the potential danger of those side effects running in the host.

But this has NEVER been tested: if you enable the environment
variable, the test WILL fail.  One obvious reason: if you are not
running as root, you'll probably get a permission failure when
trying to freeze the file systems, or when changing system time.
Less obvious: if you run the test as root (wow, you're brave), you
could end up hanging if the test tries to log things to a
temporarily frozen filesystem.  But the cutest reason of all: if
you get past the above hurdles, the test uses invalid JSON in
test_qga_fstrim() (missing '' around the dictionary key 'minimum'),
and will thus fail an assertion in qmp_fd().

Rather than leave this untested time-bomb in place, rip it out.
Hopefully, as originally envisioned, we can find an opportunity
to test an actual sandboxed guest where the guest-agent has
full permissions and will not unduly affect the host running
the test - if so, 'git revert' can be used if desired, for
salvaging any useful parts of this attempt.

Signed-off-by: Eric Blake 
---
 tests/test-qga.c | 90 
 1 file changed, 90 deletions(-)

diff --git a/tests/test-qga.c b/tests/test-qga.c
index 06783e7585..fd6bc7690f 100644
--- a/tests/test-qga.c
+++ b/tests/test-qga.c
@@ -642,65 +642,6 @@ static void test_qga_get_time(gconstpointer fix)
 QDECREF(ret);
 }

-static void test_qga_set_time(gconstpointer fix)
-{
-const TestFixture *fixture = fix;
-QDict *ret;
-int64_t current, time;
-gchar *cmd;
-
-/* get current time */
-ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-time'}");
-g_assert_nonnull(ret);
-qmp_assert_no_error(ret);
-current = qdict_get_int(ret, "return");
-g_assert_cmpint(current, >, 0);
-QDECREF(ret);
-
-/* set some old time */
-ret = qmp_fd(fixture->fd, "{'execute': 'guest-set-time',"
- " 'arguments': { 'time': 1000 } }");
-g_assert_nonnull(ret);
-qmp_assert_no_error(ret);
-QDECREF(ret);
-
-/* check old time */
-ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-time'}");
-g_assert_nonnull(ret);
-qmp_assert_no_error(ret);
-time = qdict_get_int(ret, "return");
-g_assert_cmpint(time / 1000, <, G_USEC_PER_SEC * 10);
-QDECREF(ret);
-
-/* set back current time */
-cmd = g_strdup_printf("{'execute': 'guest-set-time',"
-  " 'arguments': { 'time': %" PRId64 " } }",
-  current + time * 1000);
-ret = qmp_fd(fixture->fd, cmd);
-g_free(cmd);
-g_assert_nonnull(ret);
-qmp_assert_no_error(ret);
-QDECREF(ret);
-}
-
-static void test_qga_fstrim(gconstpointer fix)
-{
-const TestFixture *fixture = fix;
-QDict *ret;
-QList *list;
-const QListEntry *entry;
-
-ret = qmp_fd(fixture->fd, "{'execute': 'guest-fstrim',"
- " arguments: { minimum: 4194304 } }");
-g_assert_nonnull(ret);
-qmp_assert_no_error(ret);
-list = qdict_get_qlist(ret, "return");
-entry = qlist_first(list);
-g_assert(qdict_haskey(qobject_to_qdict(entry->value), "paths"));
-
-QDECREF(ret);
-}
-
 static void test_qga_blacklist(gconstpointer data)
 {
 TestFixture fix;
@@ -831,30 +772,6 @@ static void test_qga_fsfreeze_status(gconstpointer fix)
 QDECREF(ret);
 }

-static void test_qga_fsfreeze_and_thaw(gconstpointer fix)
-{
-const TestFixture *fixture = fix;
-QDict *ret;
-const gchar *status;
-
-ret = qmp_fd(fixture->fd, "{'execute': 'guest-fsfreeze-freeze'}");
-g_assert_nonnull(ret);
-qmp_assert_no_error(ret);
-QDECREF(ret);
-
-ret = qmp_fd(fixture->fd, "{'execute': 'guest-fsfreeze-status'}");
-g_assert_nonnull(ret);
-qmp_assert_no_error(ret);
-status = qdict_get_try_str(ret, "return");
-g_assert_cmpstr(status, ==, "frozen");
-QDECREF(ret);
-
-ret = qmp_fd(fixture->fd, "{'execute': 'guest-fsfreeze-thaw'}");
-g_assert_nonnull(ret);
-qmp_assert_no_error(ret);
-QDECREF(ret);
-}
-
 static void test_qga_guest_exec(gconstpointer fix)
 {
 const TestFixture *fixture = fix;
@@ -1029,13 +946,6 @@ int main(int argc, char **argv)
 g_test_add_data_func("/qga/guest-get-osinfo", ,
  test_qga_guest_get_osinfo);

-if (g_getenv("QGA_TEST_SIDE_EFFECTING")) {
-g_test_add_data_func("/qga/fsfreeze-and-thaw", ,
- test_qga_fsfreeze_and_thaw);
-g_test_add_data_func("/qga/set-time", , test_qga_set_time);
-g_test_add_data_func("/qga/fstrim", , test_qga_fstrim);
-}
-
 ret = g_test_run();

 fixture_tear_down(, 

[Qemu-devel] [PATCH 6/6] tests/tcg/test-i386-fprem.c: compilation fix for -Werror=unused-const-variable=

[Cleber Rosa]

A clean up of unused code to make the compiler happy.

Signed-off-by: Cleber Rosa 
---
 tests/tcg/test-i386-fprem.c | 15 ---
 1 file changed, 15 deletions(-)

diff --git a/tests/tcg/test-i386-fprem.c b/tests/tcg/test-i386-fprem.c
index f70363d..1dceda0 100644
--- a/tests/tcg/test-i386-fprem.c
+++ b/tests/tcg/test-i386-fprem.c
@@ -54,14 +54,6 @@ union float80u {
 
 #define IEEE854_LONG_DOUBLE_BIAS 0x3fff
 
-static const union float80u q_nan = {
-.ieee_nan.negative = 0,  /* X */
-.ieee_nan.exponent = 0x7fff,
-.ieee_nan.one = 1,
-.ieee_nan.quiet_nan = 1,
-.ieee_nan.mantissa = 0,
-};
-
 static const union float80u s_nan = {
 .ieee_nan.negative = 0,  /* X */
 .ieee_nan.exponent = 0x7fff,
@@ -91,13 +83,6 @@ static const union float80u pos_denorm = {
 .ieee.mantissa = 1,
 };
 
-static const union float80u smallest_positive_norm = {
-.ieee.negative = 0,
-.ieee.exponent = 1,
-.ieee.one = 1,
-.ieee.mantissa = 0,
-};
-
 static void fninit(void)
 {
 asm volatile ("fninit\n");
-- 
2.9.4

[Qemu-devel] [PATCH 4/6] tests/tcg/test-i386-fprem: build with $(QEMU_CFLAGS)

[Cleber Rosa]

So that glib.h can be found.

Signed-off-by: Cleber Rosa 
---
 tests/tcg/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/tcg/Makefile b/tests/tcg/Makefile
index 89e3342..c946fde 100644
--- a/tests/tcg/Makefile
+++ b/tests/tcg/Makefile
@@ -98,7 +98,7 @@ test-i386: test-i386.c test-i386-code16.S test-i386-vm86.S \
   $(

[Qemu-devel] [PATCH 5/6] tests/tcg/test-i386-fprem.c: compilation fix for -Werror=strict-prototype

[Cleber Rosa]

A trivial fix to make the compiler happy.

Signed-off-by: Cleber Rosa 
---
 tests/tcg/test-i386-fprem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/tcg/test-i386-fprem.c b/tests/tcg/test-i386-fprem.c
index 1a71623..f70363d 100644
--- a/tests/tcg/test-i386-fprem.c
+++ b/tests/tcg/test-i386-fprem.c
@@ -98,7 +98,7 @@ static const union float80u smallest_positive_norm = {
 .ieee.mantissa = 0,
 };
 
-static void fninit()
+static void fninit(void)
 {
 asm volatile ("fninit\n");
 }
-- 
2.9.4

[Qemu-devel] [PATCH 0/6] Enable building and running tcg tests on i386

[Cleber Rosa]

The primary motivation of this patch series is to get "make -C
tests/tcg", on i386, to run.  Having all of the individual i386 tcg
tests passing is beyond the scope of this patch series, though.

The secondary motivation is to gather feedback on the status of the
tcg tests.  If you have strong opinions about it, be that they need to
be nourished and cared for, that they should be eliminated completely,
or anything in between, I'd love to hear about it.

Cleber Rosa(6):
   tests/tcg/test_path.c: include utils/bufferiszero.c
   tests/tcg/linux-test.c: remove unused include of "qemu/cutils.h"
   tests/tcg/linux-test.c: include definitions for getrusage()
   tests/tcg/test-i386-fprem: build with $(QEMU_CFLAGS)
   tests/tcg/test-i386-fprem.c: compilation fix for -Werror=strict-prototype
   tests/tcg/test-i386-fprem.c: compilation fix for 
-Werror=unused-const-variable=

 tests/tcg/Makefile  |  2 +-
 tests/tcg/linux-test.c  |  2 +-
 tests/tcg/test-i386-fprem.c | 17 +
 tests/tcg/test_path.c   |  1 +
4 files changed, 4 insertions(+), 18 deletions(-)

[Qemu-devel] [PATCH 3/6] tests/tcg/linux-test.c: include definitions for getrusage()

[Cleber Rosa]

A include for  is missing, and prevents
tests/tcg/linux-test from compiling.

Signed-off-by: Cleber Rosa 
---
 tests/tcg/linux-test.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/tcg/linux-test.c b/tests/tcg/linux-test.c
index 1c6c013..15c9d7f 100644
--- a/tests/tcg/linux-test.c
+++ b/tests/tcg/linux-test.c
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define TESTPATH "/tmp/linux-test.tmp"
 #define TESTPORT 7654
-- 
2.9.4

[Qemu-devel] [PATCH 1/6] tests/tcg/test_path.c: include utils/bufferiszero.c

[Cleber Rosa]

Which contains one specific function used by iov.c.

Without this, "make -C tests/tcg test_path" (and consequently
"make -C tests/tcg" or simply "make test") fails quite early.

Signed-off-by: Cleber Rosa 
---
 tests/tcg/test_path.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/tcg/test_path.c b/tests/tcg/test_path.c
index 1c29bce..74dbdaf 100644
--- a/tests/tcg/test_path.c
+++ b/tests/tcg/test_path.c
@@ -3,6 +3,7 @@
 #include "util/cutils.c"
 #include "util/hexdump.c"
 #include "util/iov.c"
+#include "util/bufferiszero.c"
 #include "util/path.c"
 #include "util/qemu-timer-common.c"
 #include 
-- 
2.9.4

[Qemu-devel] [PATCH 2/6] tests/tcg/linux-test.c: remove unused include of "qemu/cutils.h"

[Cleber Rosa]

Building tests/tcg/linux-test is not currently possible because
$(QEMU_INCLUDES) is not being passed to $(CC_I386).  But, since
it's not really used, instead of adding the $(QEMU_INCLUDES),
let's remove the "qemu/ctuils.h" include instead.

Signed-off-by: Cleber Rosa 
---
 tests/tcg/linux-test.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/tcg/linux-test.c b/tests/tcg/linux-test.c
index 5070d31..1c6c013 100644
--- a/tests/tcg/linux-test.c
+++ b/tests/tcg/linux-test.c
@@ -39,7 +39,6 @@
 #include 
 #include 
 #include 
-#include "qemu/cutils.h"
 
 #define TESTPATH "/tmp/linux-test.tmp"
 #define TESTPORT 7654
-- 
2.9.4

Re: [Qemu-devel] [ANNOUNCE] QEMU 2.10.0-rc1 is now available

[Michael Roth]

Quoting Michael Roth (2017-08-02 14:25:48)
> Hello,
> 
> On behalf of the QEMU Team, I'd like to announce the availability of the
> second release candidate for the QEMU 2.10 release.  This release is meant
> for testing purposes and should not be used in a production environment.
> 
>   http://download.qemu-project.org/qemu-2.10.0-rc1.tar.xz
>   http://download.qemu-project.org/qemu-2.10.0-rc1.tar.xz.sig
> 
> You can help improve the quality of the QEMU 2.10 release by testing this
> release and reporting bugs on Launchpad:
> 
>   https://bugs.launchpad.net/qemu/
> 
> The release plan, as well a documented known issues for release
> candidates, are available at:
> 
>   http://wiki.qemu.org/Planning/2.10
> 
> Please add entries to the ChangeLog for the 2.10 release below:
> 
>   http://wiki.qemu.org/ChangeLog/2.10
> 
> 

Changes since rc0:

aaaec6a: Update version for v2.10.0-rc1 release (Peter Maydell)
bbcee37: tests/hmp: Fix typo in the 'chardev-send-break' test (Thomas Huth)
8bd9c4e: io: fix qio_channel_socket_accept err handling (Peter Xu)
2dfaf12: migration: fix comment disorder in RAMState (Peter Xu)
b91bf5e: migration: fix small leaks (Marc-André Lureau)
3a3fcc7: pc: acpi: force FADT rev1 for 440fx based machine types (Igor Mammedov)
208fa0e: pc: make 'pc.rom' readonly when machine has PCI enabled (Igor Mammedov)
41d4e5e: vhost-user: fix watcher need be removed when vhost-user hotplug 
(Yunjian Wang)
91c38e0: tests/bios-tables-test: Compiler warning fix (Dr. David Alan Gilbert)
2f6b38d: accel: cleanup error output (Laurent Vivier)
07f7b73: intel_iommu: use access_flags for iotlb (Peter Xu)
892721d: intel_iommu: fix iova for pt (Peter Xu)
5df04f1: vhost-user: fix legacy cross-endian configurations (Felipe Franciosi)
08b9e0b: vhost: fix a memory leak (Peng Hao)
2cef91c: tests: switch pxe and vm gen id tests to use kvm (Michael S. Tsirkin)
8e8eb0a: block/qapi: Remove redundant NULL check to silence Coverity (Kevin 
Wolf)
59fa68f: qemu-iotests/059: Fix leaked image files (Kevin Wolf)
1803f3f: qemu-iotests/063: Fix leaked image (Kevin Wolf)
a8e9c84: qemu-iotests/162: Fix leaked temporary files (Kevin Wolf)
6a1e909: qemu-iotests/153: Fix leaked scratch images (Kevin Wolf)
0a1505d: qemu-iotests/141: Fix image cleanup (Kevin Wolf)
0e59607: qemu-iotests: Remove blkdebug.conf after tests (Kevin Wolf)
db11d1e: qemu-iotests/041: Fix leaked scratch images (Kevin Wolf)
180ca19: block: fix leaks in bdrv_open_driver() (Manos Pitsidianakis)
998cbd6: block: fix dangling bs->explicit_options in block.c (Manos 
Pitsidianakis)
b81b74b: iotests: Add test of recent fix to 'qemu-img measure' (Eric Blake)
1e2b1f6: iotests: Check dirty bitmap statistics in 124 (Eric Blake)
c09bd34: iotests: Redirect stderr to stdout in 186 (Max Reitz)
645acdc: iotests: Fix test 156 (Max Reitz)
33f21e4: mc146818rtc: implement UIP latching as intended (Paolo Bonzini)
6a51d83: mc146818rtc: simplify check_update_timer (Paolo Bonzini)
da3a392: rtc-test: introduce more update tests (Paolo Bonzini)
bc706fa: rtc-test: cleanup register_b_set_flag test (Paolo Bonzini)
fafeb41: hw/scsi/vmw_pvscsi: Convert to realize (Mao Zhongyi)
dfaea0c: hw/scsi/vmw_pvscsi: Remove the dead error handling (Mao Zhongyi)
1931076: migration: optimize the downtime (Jay Zhou)
8bfce83: qemu-options: document existance of versioned machine types (Daniel P. 
Berrange)
393c13b: bt: stop the sdp memory allocation craziness (Paolo Bonzini)
f5aa69b: exec: Add lock parameter to qemu_ram_ptr_length (Anthony PERARD)
4fadfa0: target-i386: kvm_get/put_vcpu_events don't handle sipi_vector (Peng 
Hao)
eb22aec: docs: document deprecation policy & deprecated features in appendix 
(Daniel P. Berrange)
0ec846b: char: don't exit on hmp 'chardev-add help' (Anton Nefedov)
4db0db1: char-fd: remove useless chr pointer (Marc-André Lureau)
d73f024: accel: cleanup error output (Laurent Vivier)
f70d345: cpu_physical_memory_sync_dirty_bitmap: Fix alignment check (Dr. David 
Alan Gilbert)
452589b: vl.c/exit: pause cpus before closing block devices (Dr. David Alan 
Gilbert)
bd6952a: monitor: Reduce handle_qmp_command() tracing overhead (Denis V. Lunev)
8908eb1: trace-events: fix code style: print 0x before hex numbers (Vladimir 
Sementsov-Ogievskiy)
c3e5875: checkpatch: check trace-events code style (Vladimir 
Sementsov-Ogievskiy)
db73ee4: trace-events: fix code style: %# -> 0x% (Vladimir Sementsov-Ogievskiy)
44c6d63: coding_style: add point about 0x in trace-events (Vladimir 
Sementsov-Ogievskiy)
d87aa13: trace: add trace_event_get_state_backends() (Stefan Hajnoczi)
3932ef3: trace: add TRACE__BACKEND_DSTATE() (Stefan Hajnoczi)
ea1ff54: trace: ensure unique function / variable names per .stp file (Daniel 
P. Berrange)
ed86505: trace: ensure .stp files are rebuilt if trace tool source changes 
(Daniel P. Berrange)
bdf211f: Revert "syscall: fix dereference of undefined pointer" (Peter Maydell)
89cbc37: hw/mps2_scc: fix incorrect properties (Philippe Mathieu-Daudé)
f1a4694: target/arm: Migrate MPU_RNR 

[Qemu-devel] [ANNOUNCE] QEMU 2.10.0-rc1 is now available

[Michael Roth]

Hello,

On behalf of the QEMU Team, I'd like to announce the availability of the
second release candidate for the QEMU 2.10 release.  This release is meant
for testing purposes and should not be used in a production environment.

  http://download.qemu-project.org/qemu-2.10.0-rc1.tar.xz
  http://download.qemu-project.org/qemu-2.10.0-rc1.tar.xz.sig

You can help improve the quality of the QEMU 2.10 release by testing this
release and reporting bugs on Launchpad:

  https://bugs.launchpad.net/qemu/

The release plan, as well a documented known issues for release
candidates, are available at:

  http://wiki.qemu.org/Planning/2.10

Please add entries to the ChangeLog for the 2.10 release below:

  http://wiki.qemu.org/ChangeLog/2.10

Re: [Qemu-devel] [PATCH] target-mips: apply CP0.PageMask before writing into TLB entry

[Philippe Mathieu-Daudé]

Hi Leon,

On 08/02/2017 10:58 AM, Yongbok Kim wrote:

From: Leon Alrae 

PFN0 and PFN1 have to be masked out with PageMask_Mask.

Signed-off-by: Leon Alrae 
Reviewed-by: Yongbok Kim 
[Yongbok Kim:
   Added commit message]
Signed-off-by: Yongbok Kim 
---
  target/mips/op_helper.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
index 526f8e4..320f2b0 100644
--- a/target/mips/op_helper.c
+++ b/target/mips/op_helper.c
@@ -2008,6 +2008,7 @@ static inline uint64_t get_tlb_pfn_from_entrylo(uint64_t 
entrylo)
  static void r4k_fill_tlb(CPUMIPSState *env, int idx)
  {
  r4k_tlb_t *tlb;
+uint64_t mask = env->CP0_PageMask >> (TARGET_PAGE_BITS + 1);
  
  /* XXX: detect conflicting TLBs and raise a MCHECK exception when needed */

  tlb = >tlb->mmu.r4k.tlb[idx];
@@ -2028,13 +2029,13 @@ static void r4k_fill_tlb(CPUMIPSState *env, int idx)
  tlb->C0 = (env->CP0_EntryLo0 >> 3) & 0x7;
  tlb->XI0 = (env->CP0_EntryLo0 >> CP0EnLo_XI) & 1;
  tlb->RI0 = (env->CP0_EntryLo0 >> CP0EnLo_RI) & 1;
-tlb->PFN[0] = get_tlb_pfn_from_entrylo(env->CP0_EntryLo0) << 12;
+tlb->PFN[0] = (get_tlb_pfn_from_entrylo(env->CP0_EntryLo0) & ~mask) << 12;
  tlb->V1 = (env->CP0_EntryLo1 & 2) != 0;
  tlb->D1 = (env->CP0_EntryLo1 & 4) != 0;
  tlb->C1 = (env->CP0_EntryLo1 >> 3) & 0x7;
  tlb->XI1 = (env->CP0_EntryLo1 >> CP0EnLo_XI) & 1;
  tlb->RI1 = (env->CP0_EntryLo1 >> CP0EnLo_RI) & 1;
-tlb->PFN[1] = get_tlb_pfn_from_entrylo(env->CP0_EntryLo1) << 12;
+tlb->PFN[1] = (get_tlb_pfn_from_entrylo(env->CP0_EntryLo1) & ~mask) << 12;
  }
  
  void r4k_helper_tlbinv(CPUMIPSState *env)




What about refactoring get_tlb_pfn_from_entrylo(uint64_t entrylo) -> 
r4k_get_tlb_pfn_from_entrylo(uint64_t entrylo, uint64_t pagemask) to 
directly masked pfn?


Regards,

Phil.

Re: [Qemu-devel] [PATCH] ppc: fix double-free in cpu_post_load()

[Philippe Mathieu-Daudé]

On 08/02/2017 02:34 PM, Greg Kurz wrote:

When running nested with KVM PR, ppc_set_compat() fails and QEMU crashes
because of "double free or corruption (!prev)". The crash happens because
error_report_err() has already called error_free().

Signed-off-by: Greg Kurz 


Reviewed-by: Philippe Mathieu-Daudé 


---
  target/ppc/machine.c |1 -
  1 file changed, 1 deletion(-)

diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index f578156dd411..abe0a1cdf021 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -239,7 +239,6 @@ static int cpu_post_load(void *opaque, int version_id)
  ppc_set_compat(cpu, cpu->compat_pvr, _err);
  if (local_err) {
  error_report_err(local_err);
-error_free(local_err);
  return -1;
  }
  } else

Re: [Qemu-devel] [PATCH] docs/pcie.txt: Replace ioh3420 with pcie-root-port

[Marcel Apfelbaum]

On 02/08/2017 19:25, Laszlo Ersek wrote:

On 08/02/17 17:51, Marcel Apfelbaum wrote:

Do not mention ioh3420 in the "how to" doc.
The device still works and can be used by already
existing setups, but no need to be mentioned.

Suggested-by: Andrew Jones 
Signed-off-by: Marcel Apfelbaum 
---
  docs/pcie.txt | 16 
  1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/docs/pcie.txt b/docs/pcie.txt
index 5bada24..f990033 100644
--- a/docs/pcie.txt
+++ b/docs/pcie.txt
@@ -43,8 +43,8 @@ Place only the following kinds of devices directly on the 
Root Complex:
  strangely when PCI Express devices are integrated
  with the Root Complex.
  
-(2) PCI Express Root Ports (ioh3420), for starting exclusively PCI Express

-hierarchies.
+(2) PCI Express Root Ports (pcie-root-port), for starting exclusively
+PCI Express hierarchies.
  
  (3) DMI-PCI Bridges (i82801b11-bridge), for starting legacy PCI

  hierarchies.
@@ -65,7 +65,7 @@ Place only the following kinds of devices directly on the 
Root Complex:
-device pxb-pcie,id=pcie.1,bus_nr=x[,numa_node=y][,addr=z]
Only PCI Express Root Ports and DMI-PCI bridges can be connected
to the pcie.1 bus:
-  -device 
ioh3420,id=root_port1[,bus=pcie.1][,chassis=x][,slot=y][,addr=z]
 \
+  -device 
pcie-root-port,id=root_port1[,bus=pcie.1][,chassis=x][,slot=y][,addr=z] 
\
-device i82801b11-bridge,id=dmi_pci_bridge1,bus=pcie.1
  
  
@@ -107,14 +107,14 @@ Plug only PCI Express devices into PCI Express Ports.

   
  
  2.2.1 Plugging a PCI Express device into a PCI Express Root Port:

-  -device ioh3420,id=root_port1,chassis=x,slot=y[,bus=pcie.0][,addr=z] 
 \
+  -device 
pcie-root-port,id=root_port1,chassis=x,slot=y[,bus=pcie.0][,addr=z]  \
-device ,bus=root_port1
  2.2.2 Using multi-function PCI Express Root Ports:
-  -device 
ioh3420,id=root_port1,multifunction=on,chassis=x,addr=z.0[,slot=y][,bus=pcie.0] 
\
-  -device ioh3420,id=root_port2,chassis=x1,addr=z.1[,slot=y1][,bus=pcie.0] 
\
-  -device ioh3420,id=root_port3,chassis=x2,addr=z.2[,slot=y2][,bus=pcie.0] 
\
+  -device 
pcie-root-port,id=root_port1,multifunction=on,chassis=x,addr=z.0[,slot=y][,bus=pcie.0]
 \
+  -device 
pcie-root-port,id=root_port2,chassis=x1,addr=z.1[,slot=y1][,bus=pcie.0] \
+  -device 
pcie-root-port,id=root_port3,chassis=x2,addr=z.2[,slot=y2][,bus=pcie.0] \
  2.2.3 Plugging a PCI Express device into a Switch:
-  -device ioh3420,id=root_port1,chassis=x,slot=y[,bus=pcie.0][,addr=z]  \
+  -device 
pcie-root-port,id=root_port1,chassis=x,slot=y[,bus=pcie.0][,addr=z]  \
-device x3130-upstream,id=upstream_port1,bus=root_port1[,addr=x]
  \
-device 
xio3130-downstream,id=downstream_port1,bus=upstream_port1,chassis=x1,slot=y1[,addr=z1]]
 \
-device ,bus=downstream_port1



I trust that you found all occurrences of "ioh3420" in the doc :)



I did my best... thanks for the fast review!
Marcel


Reviewed-by: Laszlo Ersek 

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v3 5/5] docs: update documentation considering PCIE-PCI bridge

[Marcel Apfelbaum]

On 02/08/2017 19:26, Michael S. Tsirkin wrote:

On Wed, Aug 02, 2017 at 06:36:29PM +0300, Marcel Apfelbaum wrote:

Can dmi-pci support shpc? why doesn't it? For compatibility?


I don't know why, but the fact that it doesn't is the reason libvirt
settled on auto-creating a dmi-pci bridge and a pci-pci bridge under
that for Q35. The reasoning was (IIRC Laine's words correctly) that the
dmi-pci bridge cannot receive hotplugged devices, while the pci-pci
bridge cannot be connected to the root complex. So both were needed.

Thanks
Laszlo


OK. Is it true that dmi-pci + pci-pci under it will allow hotplug
on Q35 if we just flip the bit in _OSC?


Marcel, what say you?... :)


Good news, works with:
-device i82801b11-bridge,id=b1
-device pci-bridge,id=b2,bus=b1,chassis_nr=1,msi=off


And presumably it works for modern windows?
OK, so it looks like patch 1 is merely a bugfix, I'll merge it for 2.10.



Tested with Win10, I think is OK to merge if for 2.10.


Notice bridge's msi=off until the following kernel bug will be merged:
   https://www.spinics.net/lists/linux-pci/msg63052.html


Does libvirt support msi=off as a work-around?



Adding Laine, maybe he has the answer.

Thanks,
Marcel





Thanks,
Marcel

Re: [Qemu-devel] [PATCH] ppc: fix double-free in cpu_post_load()

[Eric Blake]

On 08/02/2017 12:34 PM, Greg Kurz wrote:
> When running nested with KVM PR, ppc_set_compat() fails and QEMU crashes
> because of "double free or corruption (!prev)". The crash happens because
> error_report_err() has already called error_free().
> 
> Signed-off-by: Greg Kurz 
> ---
>  target/ppc/machine.c |1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Eric Blake 

> 
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index f578156dd411..abe0a1cdf021 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -239,7 +239,6 @@ static int cpu_post_load(void *opaque, int version_id)
>  ppc_set_compat(cpu, cpu->compat_pvr, _err);
>  if (local_err) {
>  error_report_err(local_err);
> -error_free(local_err);
>  return -1;
>  }
>  } else
> 
> 
> 

-- 
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 05/15] hw/intc/armv7m_nvic.c: Remove out of date comment

[Edgar E. Iglesias]

On Wed, Aug 02, 2017 at 05:43:51PM +0100, Peter Maydell wrote:
> Remove an out of date comment which says there's only one
> item in the NVIC container region -- we put systick into its
> own device object a while back and so now there are two
> things in the container.
> 
> Signed-off-by: Peter Maydell 

Reviewed-by: Edgar E. Iglesias 


> ---
>  hw/intc/armv7m_nvic.c | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index 323e2d4..2e8166a 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -1036,10 +1036,6 @@ static void armv7m_nvic_realize(DeviceState *dev, 
> Error **errp)
>   *  0xd00..0xd3c - SCS registers
>   *  0xd40..0xeff - Reserved or Not implemented
>   *  0xf00 - STIR
> - *
> - * At the moment there is only one thing in the container region,
> - * but we leave it in place to allow us to pull systick out into
> - * its own device object later.
>   */
>  memory_region_init(>container, OBJECT(s), "nvic", 0x1000);
>  /* The system register region goes at the bottom of the priority
> -- 
> 2.7.4
> 
> 

Re: [Qemu-devel] [PATCH for-2.10 5/5] tests: acpi: fix FADT not being compared to reference table

[Marcel Apfelbaum]

On 02/08/2017 17:10, Igor Mammedov wrote:

On Wed, 2 Aug 2017 16:15:10 +0300
Marcel Apfelbaum  wrote:


On 31/07/2017 18:40, Igor Mammedov wrote:

It turns out that FADT isn't actually tested for changes
against reference table, since it happens to be the 1st
table in RSDT which is currently ignored.
Fix it by making sure that all tables from RSDT are added
to test list.
   


Hi Igor,


Signed-off-by: Igor Mammedov 
---
   tests/bios-tables-test.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index a2a90d7..129ef46 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -243,13 +243,13 @@ static void test_acpi_dsdt_table(test_data *data)
   /* Load all tables and add to test list directly RSDT referenced tables */
   static void fetch_rsdt_referenced_tables(test_data *data)
   {
-int tables_nr = data->rsdt_tables_nr - 1; /* fadt is first */
+int tables_nr = data->rsdt_tables_nr;
   int i;
   
   for (i = 0; i < tables_nr; i++) {

   AcpiSdtTable ssdt_table;
   
-uint32_t addr = data->rsdt_tables_addr[i + 1]; /* fadt is first */

+uint32_t addr = data->rsdt_tables_addr[i];
   fetch_table(_table, addr);
   
   /* Add table to ASL test tables list */
   


For some reason I decided not to test it... strange.
Anyway, we should also add the expected file, right?
(the rebuild script works only for existent tables, I think)

they are there see: tests/acpi-test-data/*/FACP
files obviously stale as they haven't been actually used.
Michael should update them when he applies this series.



Reviewed-by: Marcel Apfelbaum 

Thanks,
Marcel



Thanks,
Marcel

Re: [Qemu-devel] [Qemu-arm] [PATCH 04/15] target/arm: Tighten up Thumb decode where new v8M insns will be

[Edgar E. Iglesias]

On Wed, Aug 02, 2017 at 05:43:50PM +0100, Peter Maydell wrote:
> Tighten up the T32 decoder in the places where new v8M instructions
> will be:
>  * TT/TTT/TTA/TTAT are in what was nominally LDREX/STREX r15, ...
>which is UNPREDICTABLE:
>make the UNPREDICTABLE behaviour be to UNDEF
>  * BXNS/BLXNS are distinguished from BX/BLX via the low 3 bits,
>which in previous architectural versions are SBZ:
>enforce the SBZ via UNDEF rather than ignoring it, and move
>the "ARCH(5)" UNDEF case up so we don't leak a TCG temporary
>  * SG is in the encoding which would be LDRD/STRD with rn = r15;
>this is UNPREDICTABLE and we currently UNDEF:
>move this check further up the code so that we don't leak
>TCG temporaries in the UNDEF case and have a better place
>to put the SG decode.
> 
> This means that if a v8M binary is accidentally run on v7M
> or if a test case hits something that we haven't implemented
> yet the behaviour will be obvious (UNDEF) rather than obscure
> (plough on treating it as a different instruction).
> 
> In the process, add some comments about the instruction patterns
> at these points in the decode. Our Thumb and ARM decoders are
> very difficult to understand currently, but gradually adding
> comments like this should help to clarify what exactly has
> been decoded when.
> 
> Signed-off-by: Peter Maydell 

Reviewed-by: Edgar E. Iglesias 


> ---
>  target/arm/translate.c | 48 +++-
>  1 file changed, 39 insertions(+), 9 deletions(-)
> 
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index d1a5f56..3c14cb0 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -9735,10 +9735,23 @@ static int disas_thumb2_insn(CPUARMState *env, 
> DisasContext *s, uint16_t insn_hw
>  abort();
>  case 4:
>  if (insn & (1 << 22)) {
> -/* Other load/store, table branch.  */
> +/* 0b1110_100x_x1xx_____
> + * - load/store doubleword, load/store exclusive, ldacq/strel,
> + *   table branch.
> + */
>  if (insn & 0x0120) {
> -/* Load/store doubleword.  */
> +/* 0b1110_1000_x11x_____
> + *  - load/store dual (post-indexed)
> + * 0b_1001_x10x_____
> + *  - load/store dual (literal and immediate)
> + * 0b_1001_x11x_____
> + *  - load/store dual (pre-indexed)
> + */
>  if (rn == 15) {
> +if (insn & (1 << 21)) {
> +/* UNPREDICTABLE */
> +goto illegal_op;
> +}
>  addr = tcg_temp_new_i32();
>  tcg_gen_movi_i32(addr, s->pc & ~3);
>  } else {
> @@ -9772,15 +9785,18 @@ static int disas_thumb2_insn(CPUARMState *env, 
> DisasContext *s, uint16_t insn_hw
>  }
>  if (insn & (1 << 21)) {
>  /* Base writeback.  */
> -if (rn == 15)
> -goto illegal_op;
>  tcg_gen_addi_i32(addr, addr, offset - 4);
>  store_reg(s, rn, addr);
>  } else {
>  tcg_temp_free_i32(addr);
>  }
>  } else if ((insn & (1 << 23)) == 0) {
> -/* Load/store exclusive word.  */
> +/* 0b1110_1000_010x_____
> + * - load/store exclusive word
> + */
> +if (rs == 15) {
> +goto illegal_op;
> +}
>  addr = tcg_temp_local_new_i32();
>  load_reg_var(s, addr, rn);
>  tcg_gen_addi_i32(addr, addr, (insn & 0xff) << 2);
> @@ -11137,7 +11153,9 @@ static void disas_thumb_insn(CPUARMState *env, 
> DisasContext *s)
>  break;
>  }
>  if (insn & (1 << 10)) {
> -/* data processing extended or blx */
> +/* 0b0100_01xx__
> + * - data processing extended, branch and exchange
> + */
>  rd = (insn & 7) | ((insn >> 4) & 8);
>  rm = (insn >> 3) & 0xf;
>  op = (insn >> 8) & 3;
> @@ -11160,10 +11178,21 @@ static void disas_thumb_insn(CPUARMState *env, 
> DisasContext *s)
>  tmp = load_reg(s, rm);
>  store_reg(s, rd, tmp);
>  break;
> -case 3:/* branch [and link] exchange thumb register */
> -tmp = load_reg(s, rm);
> -if (insn & (1 << 7)) {
> +case 3:
> +{
> +/* 0b0100_0111__
> + * - branch [and link] exchange thumb register
> +   

Re: [Qemu-devel] [PATCH 03/15] target/arm: Consolidate PMSA handling in get_phys_addr()

[Edgar E. Iglesias]

On Wed, Aug 02, 2017 at 05:43:49PM +0100, Peter Maydell wrote:
> Currently get_phys_addr() has PMSAv7 handling before the
> "is translation disabled?" check, and then PMSAv5 after it.
> Tidy this up by making the PMSAv5 code handle the "MPU disabled"
> case itself, so that we have all the PMSA code in one place.
> This will make adding the PMSAv8 code slightly cleaner, and
> also means that pre-v7 PMSA cores benefit from the MPU lookup
> logging that the PMSAv7 codepath had.
> 
> Signed-off-by: Peter Maydell 

Reviewed-by: Edgar E. Iglesias 


> ---
>  target/arm/helper.c | 38 ++
>  1 file changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index b78d277..fd83a21 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -8423,6 +8423,13 @@ static bool get_phys_addr_pmsav5(CPUARMState *env, 
> uint32_t address,
>  uint32_t base;
>  bool is_user = regime_is_user(env, mmu_idx);
>  
> +if (regime_translation_disabled(env, mmu_idx)) {
> +/* MPU disabled.  */
> +*phys_ptr = address;
> +*prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> +return false;
> +}
> +
>  *phys_ptr = address;
>  for (n = 7; n >= 0; n--) {
>  base = env->cp15.c6_region[n];
> @@ -8572,16 +8579,20 @@ static bool get_phys_addr(CPUARMState *env, 
> target_ulong address,
>  }
>  }
>  
> -/* pmsav7 has special handling for when MPU is disabled so call it before
> - * the common MMU/MPU disabled check below.
> - */
> -if (arm_feature(env, ARM_FEATURE_PMSA) &&
> -arm_feature(env, ARM_FEATURE_V7)) {
> +if (arm_feature(env, ARM_FEATURE_PMSA)) {
>  bool ret;
>  *page_size = TARGET_PAGE_SIZE;
> -ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx,
> -   phys_ptr, prot, fsr);
> -qemu_log_mask(CPU_LOG_MMU, "PMSAv7 MPU lookup for %s at 0x%08" PRIx32
> +
> +if (arm_feature(env, ARM_FEATURE_V7)) {
> +/* PMSAv7 */
> +ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx,
> +   phys_ptr, prot, fsr);
> +} else {
> +/* Pre-v7 MPU */
> +ret = get_phys_addr_pmsav5(env, address, access_type, mmu_idx,
> +   phys_ptr, prot, fsr);
> +}
> +qemu_log_mask(CPU_LOG_MMU, "PMSA MPU lookup for %s at 0x%08" PRIx32
>" mmu_idx %u -> %s (prot %c%c%c)\n",
>access_type == MMU_DATA_LOAD ? "reading" :
>(access_type == MMU_DATA_STORE ? "writing" : 
> "execute"),
> @@ -8594,21 +8605,16 @@ static bool get_phys_addr(CPUARMState *env, 
> target_ulong address,
>  return ret;
>  }
>  
> +/* Definitely a real MMU, not an MPU */
> +
>  if (regime_translation_disabled(env, mmu_idx)) {
> -/* MMU/MPU disabled.  */
> +/* MMU disabled. */
>  *phys_ptr = address;
>  *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
>  *page_size = TARGET_PAGE_SIZE;
>  return 0;
>  }
>  
> -if (arm_feature(env, ARM_FEATURE_PMSA)) {
> -/* Pre-v7 MPU */
> -*page_size = TARGET_PAGE_SIZE;
> -return get_phys_addr_pmsav5(env, address, access_type, mmu_idx,
> -phys_ptr, prot, fsr);
> -}
> -
>  if (regime_using_lpae_format(env, mmu_idx)) {
>  return get_phys_addr_lpae(env, address, access_type, mmu_idx, 
> phys_ptr,
>attrs, prot, page_size, fsr, fi);
> -- 
> 2.7.4
> 
> 

[Qemu-devel] [PATCH] ppc: fix double-free in cpu_post_load()

[Greg Kurz]

When running nested with KVM PR, ppc_set_compat() fails and QEMU crashes
because of "double free or corruption (!prev)". The crash happens because
error_report_err() has already called error_free().

Signed-off-by: Greg Kurz 
---
 target/ppc/machine.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index f578156dd411..abe0a1cdf021 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -239,7 +239,6 @@ static int cpu_post_load(void *opaque, int version_id)
 ppc_set_compat(cpu, cpu->compat_pvr, _err);
 if (local_err) {
 error_report_err(local_err);
-error_free(local_err);
 return -1;
 }
 } else

Re: [Qemu-devel] [PATCH 02/15] target/arm: Don't trap WFI/WFE for M profile

[Edgar E. Iglesias]

On Wed, Aug 02, 2017 at 05:43:48PM +0100, Peter Maydell wrote:
> M profile cores can never trap on WFI or WFE instructions. Check for
> M profile in check_wfx_trap() to ensure this.
> 
> The existing code will do the right thing for v7M cores because
> the hcr_el2 and scr_el3 registers will be all-zeroes and so we
> won't attempt to trap, but when we start setting ARM_FEATURE_V8
> for v8M cores the v8A handling of SCTLR.nTWE and .nTWI will not
> give the right results.
> 
> Signed-off-by: Peter Maydell 

Reviewed-by: Edgar E. Iglesias 


> ---
>  target/arm/op_helper.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
> index 2a85666..5a94a5f 100644
> --- a/target/arm/op_helper.c
> +++ b/target/arm/op_helper.c
> @@ -370,6 +370,11 @@ static inline int check_wfx_trap(CPUARMState *env, bool 
> is_wfe)
>  int cur_el = arm_current_el(env);
>  uint64_t mask;
>  
> +if (arm_feature(env, ARM_FEATURE_M)) {
> +/* M profile cores can never trap WFI/WFE. */
> +return 0;
> +}
> +
>  /* If we are currently in EL0 then we need to check if SCTLR is set up 
> for
>   * WFx instructions being trapped to EL1. These trap bits don't exist in 
> v7.
>   */
> -- 
> 2.7.4
> 
> 

[Qemu-devel] [Bug 1708215] Re: Windows 10 clipboard bug

[Gheorghe Ungureanu]

UPDATE:
Restarting "SPICE VDAagent" within the VM allows me to paste again from host to 
VM, however as soon as I use copy within the VM, it stops working again.

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

Title:
  Windows 10 clipboard bug

Status in QEMU:
  New

Bug description:
  Hello,

  I am using qemu on arch:
  pacman -Q libvirt qemu linux virt-manager
  libvirt 3.5.0-1
  qemu 2.9.0-2
  linux 4.12.3-1
  virt-manager 1.4.1-2

  I have a windows 10 Guest, with all updates and the following packages 
installed in the guest:
  - QEMU guest agent 7.3.2
  - SPICE Guest Tools 0.132

  When I start the VM, I can copy/paste from the host to the guest.
  However, after I use COPY inside the VM, copy/paste is not working any
  more from host to guest. However, I can still copy/paste from guest to
  host.

  To summarize:
  - copy/paste from guest to host works always
  - copy/paste from host to guest works only if copy was not previously used in 
guest.

  If this bug needs to be reported using another portal or if I can
  provide any further information, please contact me.

  Best Regards,
  gxgung

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

Re: [Qemu-devel] [Qemu-arm] [PATCH 01/15] target/arm: Use MMUAccessType enum rather than int

[Edgar E. Iglesias]

On Wed, Aug 02, 2017 at 05:43:47PM +0100, Peter Maydell wrote:
> In the ARM get_phys_addr() code, switch to using the MMUAccessType
> enum and its MMU_* values rather than int and literal 0/1/2.
> 
> Signed-off-by: Peter Maydell 

Reviewed-by: Edgar E. Iglesias 


> ---
>  target/arm/helper.c| 30 +++---
>  target/arm/internals.h |  3 ++-
>  2 files changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index fa60040..b78d277 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -20,13 +20,13 @@
>  
>  #ifndef CONFIG_USER_ONLY
>  static bool get_phys_addr(CPUARMState *env, target_ulong address,
> -  int access_type, ARMMMUIdx mmu_idx,
> +  MMUAccessType access_type, ARMMMUIdx mmu_idx,
>hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
>target_ulong *page_size, uint32_t *fsr,
>ARMMMUFaultInfo *fi);
>  
>  static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
> -   int access_type, ARMMMUIdx mmu_idx,
> +   MMUAccessType access_type, ARMMMUIdx mmu_idx,
> hwaddr *phys_ptr, MemTxAttrs *txattrs, int 
> *prot,
> target_ulong *page_size_ptr, uint32_t *fsr,
> ARMMMUFaultInfo *fi);
> @@ -2135,7 +2135,7 @@ static CPAccessResult ats_access(CPUARMState *env, 
> const ARMCPRegInfo *ri,
>  }
>  
>  static uint64_t do_ats_write(CPUARMState *env, uint64_t value,
> - int access_type, ARMMMUIdx mmu_idx)
> + MMUAccessType access_type, ARMMMUIdx mmu_idx)
>  {
>  hwaddr phys_addr;
>  target_ulong page_size;
> @@ -2194,7 +2194,7 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t 
> value,
>  
>  static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t 
> value)
>  {
> -int access_type = ri->opc2 & 1;
> +MMUAccessType access_type = ri->opc2 & 1 ? MMU_DATA_STORE : 
> MMU_DATA_LOAD;
>  uint64_t par64;
>  ARMMMUIdx mmu_idx;
>  int el = arm_current_el(env);
> @@ -2253,7 +2253,7 @@ static void ats_write(CPUARMState *env, const 
> ARMCPRegInfo *ri, uint64_t value)
>  static void ats1h_write(CPUARMState *env, const ARMCPRegInfo *ri,
>  uint64_t value)
>  {
> -int access_type = ri->opc2 & 1;
> +MMUAccessType access_type = ri->opc2 & 1 ? MMU_DATA_STORE : 
> MMU_DATA_LOAD;
>  uint64_t par64;
>  
>  par64 = do_ats_write(env, value, access_type, ARMMMUIdx_S2NS);
> @@ -2273,7 +2273,7 @@ static CPAccessResult at_s1e2_access(CPUARMState *env, 
> const ARMCPRegInfo *ri,
>  static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri,
>  uint64_t value)
>  {
> -int access_type = ri->opc2 & 1;
> +MMUAccessType access_type = ri->opc2 & 1 ? MMU_DATA_STORE : 
> MMU_DATA_LOAD;
>  ARMMMUIdx mmu_idx;
>  int secure = arm_is_secure_below_el3(env);
>  
> @@ -7510,7 +7510,7 @@ static uint64_t arm_ldq_ptw(CPUState *cs, hwaddr addr, 
> bool is_secure,
>  }
>  
>  static bool get_phys_addr_v5(CPUARMState *env, uint32_t address,
> - int access_type, ARMMMUIdx mmu_idx,
> + MMUAccessType access_type, ARMMMUIdx mmu_idx,
>   hwaddr *phys_ptr, int *prot,
>   target_ulong *page_size, uint32_t *fsr,
>   ARMMMUFaultInfo *fi)
> @@ -7626,7 +7626,7 @@ do_fault:
>  }
>  
>  static bool get_phys_addr_v6(CPUARMState *env, uint32_t address,
> - int access_type, ARMMMUIdx mmu_idx,
> + MMUAccessType access_type, ARMMMUIdx mmu_idx,
>   hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
>   target_ulong *page_size, uint32_t *fsr,
>   ARMMMUFaultInfo *fi)
> @@ -7733,7 +7733,7 @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t 
> address,
>  if (pxn && !regime_is_user(env, mmu_idx)) {
>  xn = 1;
>  }
> -if (xn && access_type == 2)
> +if (xn && access_type == MMU_INST_FETCH)
>  goto do_fault;
>  
>  if (arm_feature(env, ARM_FEATURE_V6K) &&
> @@ -7848,7 +7848,7 @@ static bool check_s2_mmu_setup(ARMCPU *cpu, bool 
> is_aa64, int level,
>  }
>  
>  static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
> -   int access_type, ARMMMUIdx mmu_idx,
> +   MMUAccessType access_type, ARMMMUIdx mmu_idx,
> hwaddr *phys_ptr, MemTxAttrs *txattrs, int 
> *prot,
> target_ulong *page_size_ptr, 

[Qemu-devel] [Bug 1708215] [NEW] Windows 10 clipboard bug

[Gheorghe Ungureanu]

Public bug reported:

Hello,

I am using qemu on arch:
pacman -Q libvirt qemu linux virt-manager
libvirt 3.5.0-1
qemu 2.9.0-2
linux 4.12.3-1
virt-manager 1.4.1-2

I have a windows 10 Guest, with all updates and the following packages 
installed in the guest:
- QEMU guest agent 7.3.2
- SPICE Guest Tools 0.132

When I start the VM, I can copy/paste from the host to the guest.
However, after I use COPY inside the VM, copy/paste is not working any
more from host to guest. However, I can still copy/paste from guest to
host.

To summarize:
- copy/paste from guest to host works always
- copy/paste from host to guest works only if copy was not previously used in 
guest.

If this bug needs to be reported using another portal or if I can
provide any further information, please contact me.

Best Regards,
gxgung

** Affects: qemu
 Importance: Undecided
 Status: New


** Tags: spice windows

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

Title:
  Windows 10 clipboard bug

Status in QEMU:
  New

Bug description:
  Hello,

  I am using qemu on arch:
  pacman -Q libvirt qemu linux virt-manager
  libvirt 3.5.0-1
  qemu 2.9.0-2
  linux 4.12.3-1
  virt-manager 1.4.1-2

  I have a windows 10 Guest, with all updates and the following packages 
installed in the guest:
  - QEMU guest agent 7.3.2
  - SPICE Guest Tools 0.132

  When I start the VM, I can copy/paste from the host to the guest.
  However, after I use COPY inside the VM, copy/paste is not working any
  more from host to guest. However, I can still copy/paste from guest to
  host.

  To summarize:
  - copy/paste from guest to host works always
  - copy/paste from host to guest works only if copy was not previously used in 
guest.

  If this bug needs to be reported using another portal or if I can
  provide any further information, please contact me.

  Best Regards,
  gxgung

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

Re: [Qemu-devel] [PATCH 3/3] block: remove legacy I/O throttling

[Manos Pitsidianakis]

On Wed, Aug 02, 2017 at 12:33:19PM +0200, Kevin Wolf wrote:

Am 02.08.2017 um 12:07 hat Stefan Hajnoczi geschrieben:

On Tue, Aug 01, 2017 at 04:49:07PM +0300, Manos Pitsidianakis wrote:
> diff --git a/block.c b/block.c
> index 9ebdba28b0..c6aad25286 100644
> --- a/block.c
> +++ b/block.c
> @@ -1975,6 +1975,7 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState 
*child_bs,
>  child = g_new(BdrvChild, 1);
>  *child = (BdrvChild) {
>  .bs = NULL,
> +.parent_bs  = NULL,
>  .name   = g_strdup(child_name),
>  .role   = child_role,
>  .perm   = perm,
> @@ -2009,6 +2010,7 @@ BdrvChild *bdrv_attach_child(BlockDriverState 
*parent_bs,
>  if (child == NULL) {
>  return NULL;
>  }
> +child->parent_bs = parent_bs;
>
>  QLIST_INSERT_HEAD(_bs->children, child, next);
>  return child;
> @@ -3729,6 +3731,12 @@ const char *bdrv_get_parent_name(const 
BlockDriverState *bs)
>  return name;
>  }
>  }
> +if (c->parent_bs && c->parent_bs->implicit) {
> +name = bdrv_get_parent_name(c->parent_bs);
> +if (name && *name) {
> +return name;
> +}
> +}
>  }
>
>  return NULL;

This should be a separate patch.

Who updates parent_bs if the parent is changed (e.g.
bdrv_replace_node())?

We already have bs->parents.  Why is BdrvChild->parent_bs needed?


I haven't look at the whole patch yet, but BdrvChild->parent_bs is a
thing that intentionally doesn't exist. A node simply has no business
knowing its parents - which may or may not be BlockDriverStates (the
obvious example where they aren't BDSes are BlockBackends, but block
jobs own some BdrvChild objects, too).

Usually the replacement is a BdrvChildRole callback.

Kevin


I think accessing the parent bs is necessary for the reasons I specified 
in my reply to Stefan. Will a callback that returns BdrvChild->opaque 
(parent_bs) for child_file and child_backing be okay?




signature.asc
Description: PGP signature

[Qemu-devel] [PATCH 05/15] hw/intc/armv7m_nvic.c: Remove out of date comment

[Peter Maydell]

Remove an out of date comment which says there's only one
item in the NVIC container region -- we put systick into its
own device object a while back and so now there are two
things in the container.

Signed-off-by: Peter Maydell 
---
 hw/intc/armv7m_nvic.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 323e2d4..2e8166a 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -1036,10 +1036,6 @@ static void armv7m_nvic_realize(DeviceState *dev, Error 
**errp)
  *  0xd00..0xd3c - SCS registers
  *  0xd40..0xeff - Reserved or Not implemented
  *  0xf00 - STIR
- *
- * At the moment there is only one thing in the container region,
- * but we leave it in place to allow us to pull systick out into
- * its own device object later.
  */
 memory_region_init(>container, OBJECT(s), "nvic", 0x1000);
 /* The system register region goes at the bottom of the priority
-- 
2.7.4

[Qemu-devel] [PATCH 06/15] target/arm: Remove incorrect comment about MPU_CTRL

[Peter Maydell]

Remove the comment that claims that some MPU_CTRL bits are stored
in sctlr_el[1]. This has never been true since MPU_CTRL was added
in commit 29c483a50607 -- the comment is a leftover from
Michael Davidsaver's original implementation, which I modified
not to use sctlr_el[1]; I forgot to delete the comment then.

Signed-off-by: Peter Maydell 
---
 target/arm/cpu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index b39d64a..b64474c 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -416,7 +416,7 @@ typedef struct CPUARMState {
 uint32_t dfsr; /* Debug Fault Status Register */
 uint32_t mmfar; /* MemManage Fault Address */
 uint32_t bfar; /* BusFault Address */
-unsigned mpu_ctrl; /* MPU_CTRL (some bits kept in sctlr_el[1]) */
+unsigned mpu_ctrl; /* MPU_CTRL */
 int exception;
 } v7m;
 
-- 
2.7.4

[Qemu-devel] [PATCH 00/15] v7M: cleanups and bugfixes prior to v8M

[Peter Maydell]

(This is 2.11 material, obviously, but it's a coherent and
large enough set of patches that I figured I might as well
push it out for review now.)

This patchset is a collection of cleanups, bugfixes, etc to
the existing v7M code which are either necessary preliminary
to implementing v8M or just things I noticed along the way.
The non-trivial stuff is:
 * migration for M profile is shifted to not use read_cpsr()
   and write_cpsr() which assume A profile semantics
   (back compatibility with old migration state is maintained)
 * we implement the "user accesses should BusFault" behaviour
   for the memory mapped registers in the SCS, though this
   won't actually kick in until we turn MEMTX_ERROR into a
   BusFault (I have patches for that)

thanks
-- PMM

Peter Maydell (15):
  target/arm: Use MMUAccessType enum rather than int
  target/arm: Don't trap WFI/WFE for M profile
  target/arm: Consolidate PMSA handling in get_phys_addr()
  target/arm: Tighten up Thumb decode where new v8M insns will be
  hw/intc/armv7m_nvic.c: Remove out of date comment
  target/arm: Remove incorrect comment about MPU_CTRL
  target/arm: Fix outdated comment about exception exit
  target/arm: Define and use XPSR bit masks
  target/arm: Don't store M profile PRIMASK and FAULTMASK in daif
  target/arm: Don't use cpsr_write/cpsr_read to transfer M profile XPSR
  target/arm: Make arm_cpu_dump_state() handle the M-profile XPSR
  target/arm: Don't calculate lr in arm_v7m_cpu_do_interrupt() until
needed
  target/arm: Create and use new function arm_v7m_is_handler_mode()
  armv7m_nvic.h: Move from include/hw/arm to include/hw/intc
  nvic: Implement "user accesses BusFault" SCS region behaviour

 hw/intc/armv7m_nvic.c  |  68 +++---
 include/hw/arm/armv7m.h|   2 +-
 include/hw/{arm => intc}/armv7m_nvic.h |   0
 target/arm/cpu.c   |   5 --
 target/arm/cpu.h   |  54 ++
 target/arm/helper.c| 124 -
 target/arm/internals.h |   3 +-
 target/arm/machine.c   |  54 +-
 target/arm/op_helper.c |   5 ++
 target/arm/translate.c | 106 +---
 10 files changed, 286 insertions(+), 135 deletions(-)
 rename include/hw/{arm => intc}/armv7m_nvic.h (100%)

-- 
2.7.4

[Qemu-devel] [PATCH 11/15] target/arm: Make arm_cpu_dump_state() handle the M-profile XPSR

[Peter Maydell]

Make the arm_cpu_dump_state() debug logging handle the M-profile XPSR
rather than assuming it's an A-profile CPSR.  On M profile the PSR
line of a register dump will now look like this:

XPSR=4100 -Z-- T priv-thread

Signed-off-by: Peter Maydell 
---
 target/arm/translate.c | 58 ++
 1 file changed, 40 insertions(+), 18 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index 3c14cb0..e52a6d7 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -12215,8 +12215,6 @@ void arm_cpu_dump_state(CPUState *cs, FILE *f, 
fprintf_function cpu_fprintf,
 ARMCPU *cpu = ARM_CPU(cs);
 CPUARMState *env = >env;
 int i;
-uint32_t psr;
-const char *ns_status;
 
 if (is_a64(env)) {
 aarch64_cpu_dump_state(cs, f, cpu_fprintf, flags);
@@ -12230,24 +12228,48 @@ void arm_cpu_dump_state(CPUState *cs, FILE *f, 
fprintf_function cpu_fprintf,
 else
 cpu_fprintf(f, " ");
 }
-psr = cpsr_read(env);
 
-if (arm_feature(env, ARM_FEATURE_EL3) &&
-(psr & CPSR_M) != ARM_CPU_MODE_MON) {
-ns_status = env->cp15.scr_el3 & SCR_NS ? "NS " : "S ";
+if (arm_feature(env, ARM_FEATURE_M)) {
+uint32_t xpsr = xpsr_read(env);
+const char *mode;
+
+if (xpsr & XPSR_EXCP) {
+mode = "handler";
+} else {
+if (env->v7m.control & R_V7M_CONTROL_NPRIV_MASK) {
+mode = "unpriv-thread";
+} else {
+mode = "priv-thread";
+}
+}
+
+cpu_fprintf(f, "XPSR=%08x %c%c%c%c %c %s\n",
+xpsr,
+xpsr & XPSR_N ? 'N' : '-',
+xpsr & XPSR_Z ? 'Z' : '-',
+xpsr & XPSR_C ? 'C' : '-',
+xpsr & XPSR_V ? 'V' : '-',
+xpsr & XPSR_T ? 'T' : 'A',
+mode);
 } else {
-ns_status = "";
-}
-
-cpu_fprintf(f, "PSR=%08x %c%c%c%c %c %s%s%d\n",
-psr,
-psr & (1 << 31) ? 'N' : '-',
-psr & (1 << 30) ? 'Z' : '-',
-psr & (1 << 29) ? 'C' : '-',
-psr & (1 << 28) ? 'V' : '-',
-psr & CPSR_T ? 'T' : 'A',
-ns_status,
-cpu_mode_names[psr & 0xf], (psr & 0x10) ? 32 : 26);
+uint32_t psr = cpsr_read(env);
+const char *ns_status = "";
+
+if (arm_feature(env, ARM_FEATURE_EL3) &&
+(psr & CPSR_M) != ARM_CPU_MODE_MON) {
+ns_status = env->cp15.scr_el3 & SCR_NS ? "NS " : "S ";
+}
+
+cpu_fprintf(f, "PSR=%08x %c%c%c%c %c %s%s%d\n",
+psr,
+psr & CPSR_N ? 'N' : '-',
+psr & CPSR_Z ? 'Z' : '-',
+psr & CPSR_C ? 'C' : '-',
+psr & CPSR_V ? 'V' : '-',
+psr & CPSR_T ? 'T' : 'A',
+ns_status,
+cpu_mode_names[psr & 0xf], (psr & 0x10) ? 32 : 26);
+}
 
 if (flags & CPU_DUMP_FPU) {
 int numvfpregs = 0;
-- 
2.7.4

[Qemu-devel] [PATCH 03/15] target/arm: Consolidate PMSA handling in get_phys_addr()

[Peter Maydell]

Currently get_phys_addr() has PMSAv7 handling before the
"is translation disabled?" check, and then PMSAv5 after it.
Tidy this up by making the PMSAv5 code handle the "MPU disabled"
case itself, so that we have all the PMSA code in one place.
This will make adding the PMSAv8 code slightly cleaner, and
also means that pre-v7 PMSA cores benefit from the MPU lookup
logging that the PMSAv7 codepath had.

Signed-off-by: Peter Maydell 
---
 target/arm/helper.c | 38 ++
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index b78d277..fd83a21 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8423,6 +8423,13 @@ static bool get_phys_addr_pmsav5(CPUARMState *env, 
uint32_t address,
 uint32_t base;
 bool is_user = regime_is_user(env, mmu_idx);
 
+if (regime_translation_disabled(env, mmu_idx)) {
+/* MPU disabled.  */
+*phys_ptr = address;
+*prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
+return false;
+}
+
 *phys_ptr = address;
 for (n = 7; n >= 0; n--) {
 base = env->cp15.c6_region[n];
@@ -8572,16 +8579,20 @@ static bool get_phys_addr(CPUARMState *env, 
target_ulong address,
 }
 }
 
-/* pmsav7 has special handling for when MPU is disabled so call it before
- * the common MMU/MPU disabled check below.
- */
-if (arm_feature(env, ARM_FEATURE_PMSA) &&
-arm_feature(env, ARM_FEATURE_V7)) {
+if (arm_feature(env, ARM_FEATURE_PMSA)) {
 bool ret;
 *page_size = TARGET_PAGE_SIZE;
-ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx,
-   phys_ptr, prot, fsr);
-qemu_log_mask(CPU_LOG_MMU, "PMSAv7 MPU lookup for %s at 0x%08" PRIx32
+
+if (arm_feature(env, ARM_FEATURE_V7)) {
+/* PMSAv7 */
+ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx,
+   phys_ptr, prot, fsr);
+} else {
+/* Pre-v7 MPU */
+ret = get_phys_addr_pmsav5(env, address, access_type, mmu_idx,
+   phys_ptr, prot, fsr);
+}
+qemu_log_mask(CPU_LOG_MMU, "PMSA MPU lookup for %s at 0x%08" PRIx32
   " mmu_idx %u -> %s (prot %c%c%c)\n",
   access_type == MMU_DATA_LOAD ? "reading" :
   (access_type == MMU_DATA_STORE ? "writing" : "execute"),
@@ -8594,21 +8605,16 @@ static bool get_phys_addr(CPUARMState *env, 
target_ulong address,
 return ret;
 }
 
+/* Definitely a real MMU, not an MPU */
+
 if (regime_translation_disabled(env, mmu_idx)) {
-/* MMU/MPU disabled.  */
+/* MMU disabled. */
 *phys_ptr = address;
 *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
 *page_size = TARGET_PAGE_SIZE;
 return 0;
 }
 
-if (arm_feature(env, ARM_FEATURE_PMSA)) {
-/* Pre-v7 MPU */
-*page_size = TARGET_PAGE_SIZE;
-return get_phys_addr_pmsav5(env, address, access_type, mmu_idx,
-phys_ptr, prot, fsr);
-}
-
 if (regime_using_lpae_format(env, mmu_idx)) {
 return get_phys_addr_lpae(env, address, access_type, mmu_idx, phys_ptr,
   attrs, prot, page_size, fsr, fi);
-- 
2.7.4

[Qemu-devel] [PATCH 02/15] target/arm: Don't trap WFI/WFE for M profile

[Peter Maydell]

M profile cores can never trap on WFI or WFE instructions. Check for
M profile in check_wfx_trap() to ensure this.

The existing code will do the right thing for v7M cores because
the hcr_el2 and scr_el3 registers will be all-zeroes and so we
won't attempt to trap, but when we start setting ARM_FEATURE_V8
for v8M cores the v8A handling of SCTLR.nTWE and .nTWI will not
give the right results.

Signed-off-by: Peter Maydell 
---
 target/arm/op_helper.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index 2a85666..5a94a5f 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -370,6 +370,11 @@ static inline int check_wfx_trap(CPUARMState *env, bool 
is_wfe)
 int cur_el = arm_current_el(env);
 uint64_t mask;
 
+if (arm_feature(env, ARM_FEATURE_M)) {
+/* M profile cores can never trap WFI/WFE. */
+return 0;
+}
+
 /* If we are currently in EL0 then we need to check if SCTLR is set up for
  * WFx instructions being trapped to EL1. These trap bits don't exist in 
v7.
  */
-- 
2.7.4

[Qemu-devel] [PATCH 04/15] target/arm: Tighten up Thumb decode where new v8M insns will be

[Peter Maydell]

Tighten up the T32 decoder in the places where new v8M instructions
will be:
 * TT/TTT/TTA/TTAT are in what was nominally LDREX/STREX r15, ...
   which is UNPREDICTABLE:
   make the UNPREDICTABLE behaviour be to UNDEF
 * BXNS/BLXNS are distinguished from BX/BLX via the low 3 bits,
   which in previous architectural versions are SBZ:
   enforce the SBZ via UNDEF rather than ignoring it, and move
   the "ARCH(5)" UNDEF case up so we don't leak a TCG temporary
 * SG is in the encoding which would be LDRD/STRD with rn = r15;
   this is UNPREDICTABLE and we currently UNDEF:
   move this check further up the code so that we don't leak
   TCG temporaries in the UNDEF case and have a better place
   to put the SG decode.

This means that if a v8M binary is accidentally run on v7M
or if a test case hits something that we haven't implemented
yet the behaviour will be obvious (UNDEF) rather than obscure
(plough on treating it as a different instruction).

In the process, add some comments about the instruction patterns
at these points in the decode. Our Thumb and ARM decoders are
very difficult to understand currently, but gradually adding
comments like this should help to clarify what exactly has
been decoded when.

Signed-off-by: Peter Maydell 
---
 target/arm/translate.c | 48 +++-
 1 file changed, 39 insertions(+), 9 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index d1a5f56..3c14cb0 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -9735,10 +9735,23 @@ static int disas_thumb2_insn(CPUARMState *env, 
DisasContext *s, uint16_t insn_hw
 abort();
 case 4:
 if (insn & (1 << 22)) {
-/* Other load/store, table branch.  */
+/* 0b1110_100x_x1xx_____
+ * - load/store doubleword, load/store exclusive, ldacq/strel,
+ *   table branch.
+ */
 if (insn & 0x0120) {
-/* Load/store doubleword.  */
+/* 0b1110_1000_x11x_____
+ *  - load/store dual (post-indexed)
+ * 0b_1001_x10x_____
+ *  - load/store dual (literal and immediate)
+ * 0b_1001_x11x_____
+ *  - load/store dual (pre-indexed)
+ */
 if (rn == 15) {
+if (insn & (1 << 21)) {
+/* UNPREDICTABLE */
+goto illegal_op;
+}
 addr = tcg_temp_new_i32();
 tcg_gen_movi_i32(addr, s->pc & ~3);
 } else {
@@ -9772,15 +9785,18 @@ static int disas_thumb2_insn(CPUARMState *env, 
DisasContext *s, uint16_t insn_hw
 }
 if (insn & (1 << 21)) {
 /* Base writeback.  */
-if (rn == 15)
-goto illegal_op;
 tcg_gen_addi_i32(addr, addr, offset - 4);
 store_reg(s, rn, addr);
 } else {
 tcg_temp_free_i32(addr);
 }
 } else if ((insn & (1 << 23)) == 0) {
-/* Load/store exclusive word.  */
+/* 0b1110_1000_010x_____
+ * - load/store exclusive word
+ */
+if (rs == 15) {
+goto illegal_op;
+}
 addr = tcg_temp_local_new_i32();
 load_reg_var(s, addr, rn);
 tcg_gen_addi_i32(addr, addr, (insn & 0xff) << 2);
@@ -11137,7 +11153,9 @@ static void disas_thumb_insn(CPUARMState *env, 
DisasContext *s)
 break;
 }
 if (insn & (1 << 10)) {
-/* data processing extended or blx */
+/* 0b0100_01xx__
+ * - data processing extended, branch and exchange
+ */
 rd = (insn & 7) | ((insn >> 4) & 8);
 rm = (insn >> 3) & 0xf;
 op = (insn >> 8) & 3;
@@ -11160,10 +11178,21 @@ static void disas_thumb_insn(CPUARMState *env, 
DisasContext *s)
 tmp = load_reg(s, rm);
 store_reg(s, rd, tmp);
 break;
-case 3:/* branch [and link] exchange thumb register */
-tmp = load_reg(s, rm);
-if (insn & (1 << 7)) {
+case 3:
+{
+/* 0b0100_0111__
+ * - branch [and link] exchange thumb register
+ */
+bool link = insn & (1 << 7);
+
+if (insn & 7) {
+goto undef;
+}
+if (link) {
 ARCH(5);
+}
+tmp = load_reg(s, rm);
+if (link) {
 val = (uint32_t)s->pc | 1;

[Qemu-devel] [PATCH 12/15] target/arm: Don't calculate lr in arm_v7m_cpu_do_interrupt() until needed

[Peter Maydell]

Move the code in arm_v7m_cpu_do_interrupt() that calculates the
magic LR value down to when we're actually going to use it.
Having the calculation and use so far apart makes the code
a little harder to understand than it needs to be.

Signed-off-by: Peter Maydell 
---
 target/arm/helper.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index b64ddb1..0ecc8f1 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6311,13 +6311,6 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
 
 arm_log_exception(cs->exception_index);
 
-lr = 0xfff1;
-if (env->v7m.control & R_V7M_CONTROL_SPSEL_MASK) {
-lr |= 4;
-}
-if (env->v7m.exception == 0)
-lr |= 8;
-
 /* For exceptions we just mark as pending on the NVIC, and let that
handle it.  */
 switch (cs->exception_index) {
@@ -6408,6 +6401,14 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
 return; /* Never happens.  Keep compiler happy.  */
 }
 
+lr = 0xfff1;
+if (env->v7m.control & R_V7M_CONTROL_SPSEL_MASK) {
+lr |= 4;
+}
+if (env->v7m.exception == 0) {
+lr |= 8;
+}
+
 v7m_push_stack(cpu);
 v7m_exception_taken(cpu, lr);
 qemu_log_mask(CPU_LOG_INT, "... as %d\n", env->v7m.exception);
-- 
2.7.4

[Qemu-devel] [PATCH 09/15] target/arm: Don't store M profile PRIMASK and FAULTMASK in daif

[Peter Maydell]

We currently store the M profile CPU register state PRIMASK and
FAULTMASK in the daif field of the CPU state in its I and F
bits. This is a legacy from the original implementation, which
tried to share the cpu_exec_interrupt code between A profile
and M profile. We've since separated out the two cases because
they are significantly different, so now there is no common
code between M and A profile which looks at env->daif: all the
uses are either in A-only or M-only code paths. Sharing the state
fields now is just confusing, and will make things awkward
when we implement v8M, where the PRIMASK and FAULTMASK
registers are banked between security states.

Switch M profile over to using v7m.faultmask and v7m.primask
fields for these registers.

Signed-off-by: Peter Maydell 
---
 hw/intc/armv7m_nvic.c |  4 ++--
 target/arm/cpu.c  |  5 -
 target/arm/cpu.h  |  4 +++-
 target/arm/helper.c   | 18 +-
 target/arm/machine.c  | 33 +
 5 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 2e8166a..343bc16 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -167,9 +167,9 @@ static inline int nvic_exec_prio(NVICState *s)
 CPUARMState *env = >cpu->env;
 int running;
 
-if (env->daif & PSTATE_F) { /* FAULTMASK */
+if (env->v7m.faultmask) {
 running = -1;
-} else if (env->daif & PSTATE_I) { /* PRIMASK */
+} else if (env->v7m.primask) {
 running = 0;
 } else if (env->v7m.basepri > 0) {
 running = env->v7m.basepri & nvic_gprio_mask(s);
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 05c038b..b241a63 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -185,11 +185,6 @@ static void arm_cpu_reset(CPUState *s)
 uint32_t initial_pc; /* Loaded from 0x4 */
 uint8_t *rom;
 
-/* For M profile we store FAULTMASK and PRIMASK in the
- * PSTATE F and I bits; these are both clear at reset.
- */
-env->daif &= ~(PSTATE_I | PSTATE_F);
-
 /* The reset value of this bit is IMPDEF, but ARM recommends
  * that it resets to 1, so QEMU always does that rather than making
  * it dependent on CPU model.
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 1f06de0..da90b7a 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -418,6 +418,8 @@ typedef struct CPUARMState {
 uint32_t bfar; /* BusFault Address */
 unsigned mpu_ctrl; /* MPU_CTRL */
 int exception;
+uint32_t primask;
+uint32_t faultmask;
 } v7m;
 
 /* Information associated with an exception about to be taken:
@@ -2179,7 +2181,7 @@ static inline int cpu_mmu_index(CPUARMState *env, bool 
ifetch)
  * we're in a HardFault or NMI handler.
  */
 if ((env->v7m.exception > 0 && env->v7m.exception <= 3)
-|| env->daif & PSTATE_F) {
+|| env->v7m.faultmask) {
 return arm_to_core_mmu_idx(ARMMMUIdx_MNegPri);
 }
 
diff --git a/target/arm/helper.c b/target/arm/helper.c
index f087d42..b64ddb1 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6172,7 +6172,7 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
 
 if (env->v7m.exception != ARMV7M_EXCP_NMI) {
 /* Auto-clear FAULTMASK on return from other than NMI */
-env->daif &= ~PSTATE_F;
+env->v7m.faultmask = 0;
 }
 
 switch (armv7m_nvic_complete_irq(env->nvic, env->v7m.exception)) {
@@ -8718,12 +8718,12 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
 return (env->v7m.control & R_V7M_CONTROL_SPSEL_MASK) ?
 env->regs[13] : env->v7m.other_sp;
 case 16: /* PRIMASK */
-return (env->daif & PSTATE_I) != 0;
+return env->v7m.primask;
 case 17: /* BASEPRI */
 case 18: /* BASEPRI_MAX */
 return env->v7m.basepri;
 case 19: /* FAULTMASK */
-return (env->daif & PSTATE_F) != 0;
+return env->v7m.faultmask;
 default:
 qemu_log_mask(LOG_GUEST_ERROR, "Attempt to read unknown special"
" register %d\n", reg);
@@ -8778,11 +8778,7 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, 
uint32_t val)
 }
 break;
 case 16: /* PRIMASK */
-if (val & 1) {
-env->daif |= PSTATE_I;
-} else {
-env->daif &= ~PSTATE_I;
-}
+env->v7m.primask = val & 1;
 break;
 case 17: /* BASEPRI */
 env->v7m.basepri = val & 0xff;
@@ -8793,11 +8789,7 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, 
uint32_t val)
 env->v7m.basepri = val;
 break;
 case 19: /* FAULTMASK */
-if (val & 1) {
-env->daif |= PSTATE_F;
-} else {
-env->daif &= ~PSTATE_F;
-}
+env->v7m.faultmask = val & 1;
 break;
 case 20: /* CONTROL */
   

[Qemu-devel] [PATCH 08/15] target/arm: Define and use XPSR bit masks

[Peter Maydell]

The M profile XPSR is almost the same format as the A profile CPSR,
but not quite. Define some XPSR_* macros and use them where we
definitely dealing with an XPSR rather than reusing the CPSR ones.

Signed-off-by: Peter Maydell 
---
 target/arm/cpu.h| 38 --
 target/arm/helper.c | 15 ---
 2 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index b64474c..1f06de0 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -883,6 +883,22 @@ void pmccntr_sync(CPUARMState *env);
 /* Mask of bits which may be set by exception return copying them from SPSR */
 #define CPSR_ERET_MASK (~CPSR_RESERVED)
 
+/* Bit definitions for M profile XPSR. Most are the same as CPSR. */
+#define XPSR_EXCP 0x1ffU
+#define XPSR_SPREALIGN (1U << 9) /* Only set in exception stack frames */
+#define XPSR_IT_2_7 CPSR_IT_2_7
+#define XPSR_GE CPSR_GE
+#define XPSR_SFPA (1U << 20) /* Only set in exception stack frames */
+#define XPSR_T (1U << 24) /* Not the same as CPSR_T ! */
+#define XPSR_IT_0_1 CPSR_IT_0_1
+#define XPSR_Q CPSR_Q
+#define XPSR_V CPSR_V
+#define XPSR_C CPSR_C
+#define XPSR_Z CPSR_Z
+#define XPSR_N CPSR_N
+#define XPSR_NZCV CPSR_NZCV
+#define XPSR_IT CPSR_IT
+
 #define TTBCR_N  (7U << 0) /* TTBCR.EAE==0 */
 #define TTBCR_T0SZ   (7U << 0) /* TTBCR.EAE==1 */
 #define TTBCR_PD0(1U << 4)
@@ -987,26 +1003,28 @@ static inline uint32_t xpsr_read(CPUARMState *env)
 /* Set the xPSR.  Note that some bits of mask must be all-set or all-clear.  */
 static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
 {
-if (mask & CPSR_NZCV) {
-env->ZF = (~val) & CPSR_Z;
+if (mask & XPSR_NZCV) {
+env->ZF = (~val) & XPSR_Z;
 env->NF = val;
 env->CF = (val >> 29) & 1;
 env->VF = (val << 3) & 0x8000;
 }
-if (mask & CPSR_Q)
-env->QF = ((val & CPSR_Q) != 0);
-if (mask & (1 << 24))
-env->thumb = ((val & (1 << 24)) != 0);
-if (mask & CPSR_IT_0_1) {
+if (mask & XPSR_Q) {
+env->QF = ((val & XPSR_Q) != 0);
+}
+if (mask & XPSR_T) {
+env->thumb = ((val & XPSR_T) != 0);
+}
+if (mask & XPSR_IT_0_1) {
 env->condexec_bits &= ~3;
 env->condexec_bits |= (val >> 25) & 3;
 }
-if (mask & CPSR_IT_2_7) {
+if (mask & XPSR_IT_2_7) {
 env->condexec_bits &= 3;
 env->condexec_bits |= (val >> 8) & 0xfc;
 }
-if (mask & 0x1ff) {
-env->v7m.exception = val & 0x1ff;
+if (mask & XPSR_EXCP) {
+env->v7m.exception = val & XPSR_EXCP;
 }
 }
 
diff --git a/target/arm/helper.c b/target/arm/helper.c
index cb88c66..f087d42 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6119,7 +6119,7 @@ static void v7m_push_stack(ARMCPU *cpu)
 /* Align stack pointer if the guest wants that */
 if ((env->regs[13] & 4) && (env->v7m.ccr & R_V7M_CCR_STKALIGN_MASK)) {
 env->regs[13] -= 4;
-xpsr |= 0x200;
+xpsr |= XPSR_SPREALIGN;
 }
 /* Switch to the handler mode.  */
 v7m_push(env, xpsr);
@@ -6244,10 +6244,11 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
 env->regs[15] &= ~1U;
 }
 xpsr = v7m_pop(env);
-xpsr_write(env, xpsr, 0xfdff);
+xpsr_write(env, xpsr, ~XPSR_SPREALIGN);
 /* Undo stack alignment.  */
-if (xpsr & 0x200)
+if (xpsr & XPSR_SPREALIGN) {
 env->regs[13] |= 4;
+}
 
 /* The restored xPSR exception field will be zero if we're
  * resuming in Thread mode. If that doesn't match what the
@@ -8693,10 +8694,10 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
 case 0 ... 7: /* xPSR sub-fields */
 mask = 0;
 if ((reg & 1) && el) {
-mask |= 0x01ff; /* IPSR (unpriv. reads as zero) */
+mask |= XPSR_EXCP; /* IPSR (unpriv. reads as zero) */
 }
 if (!(reg & 4)) {
-mask |= 0xf800; /* APSR */
+mask |= XPSR_NZCV | XPSR_Q; /* APSR */
 }
 /* EPSR reads as zero */
 return xpsr_read(env) & mask;
@@ -8754,10 +8755,10 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t 
maskreg, uint32_t val)
 uint32_t apsrmask = 0;
 
 if (mask & 8) {
-apsrmask |= 0xf800; /* APSR NZCVQ */
+apsrmask |= XPSR_NZCV | XPSR_Q;
 }
 if ((mask & 4) && arm_feature(env, ARM_FEATURE_THUMB_DSP)) {
-apsrmask |= 0x000f; /* APSR GE[3:0] */
+apsrmask |= XPSR_GE;
 }
 xpsr_write(env, val, apsrmask);
 }
-- 
2.7.4

[Qemu-devel] [PATCH 13/15] target/arm: Create and use new function arm_v7m_is_handler_mode()

[Peter Maydell]

Add a utility function for testing whether the CPU is in Handler
mode; this is just a check whether v7m.exception is non-zero, but
we do it in several places and it makes the code a bit easier
to read to not have to mentally figure out what the test is testing.

Signed-off-by: Peter Maydell 
---
 target/arm/cpu.h| 10 --
 target/arm/helper.c |  8 
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index da90b7a..a3b4b78 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1630,13 +1630,19 @@ static inline int arm_highest_el(CPUARMState *env)
 return 1;
 }
 
+/* Return true if a v7M CPU is in Handler mode */
+static inline bool arm_v7m_is_handler_mode(CPUARMState *env)
+{
+return env->v7m.exception != 0;
+}
+
 /* Return the current Exception Level (as per ARMv8; note that this differs
  * from the ARMv7 Privilege Level).
  */
 static inline int arm_current_el(CPUARMState *env)
 {
 if (arm_feature(env, ARM_FEATURE_M)) {
-return !((env->v7m.exception == 0) && (env->v7m.control & 1));
+return arm_v7m_is_handler_mode(env) || !(env->v7m.control & 1);
 }
 
 if (is_a64(env)) {
@@ -2636,7 +2642,7 @@ static inline void cpu_get_tb_cpu_state(CPUARMState *env, 
target_ulong *pc,
 }
 *flags |= fp_exception_el(env) << ARM_TBFLAG_FPEXC_EL_SHIFT;
 
-if (env->v7m.exception != 0) {
+if (arm_v7m_is_handler_mode(env)) {
 *flags |= ARM_TBFLAG_HANDLER_MASK;
 }
 
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 0ecc8f1..7920153 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6147,7 +6147,7 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
  * that jumps to magic addresses don't have magic behaviour unless
  * we're in Handler mode (compare pseudocode BXWritePC()).
  */
-assert(env->v7m.exception != 0);
+assert(arm_v7m_is_handler_mode(env));
 
 /* In the spec pseudocode ExceptionReturn() is called directly
  * from BXWritePC() and gets the full target PC value including
@@ -6254,7 +6254,7 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
  * resuming in Thread mode. If that doesn't match what the
  * exception return type specified then this is a UsageFault.
  */
-if (return_to_handler == (env->v7m.exception == 0)) {
+if (return_to_handler != arm_v7m_is_handler_mode(env)) {
 /* Take an INVPC UsageFault by pushing the stack again. */
 armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE);
 env->v7m.cfsr |= R_V7M_CFSR_INVPC_MASK;
@@ -6405,7 +6405,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
 if (env->v7m.control & R_V7M_CONTROL_SPSEL_MASK) {
 lr |= 4;
 }
-if (env->v7m.exception == 0) {
+if (!arm_v7m_is_handler_mode(env)) {
 lr |= 8;
 }
 
@@ -8798,7 +8798,7 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, 
uint32_t val)
  * switch_v7m_sp() deals with updating the SPSEL bit in
  * env->v7m.control, so we only need update the others.
  */
-if (env->v7m.exception == 0) {
+if (!arm_v7m_is_handler_mode(env)) {
 switch_v7m_sp(env, (val & R_V7M_CONTROL_SPSEL_MASK) != 0);
 }
 env->v7m.control &= ~R_V7M_CONTROL_NPRIV_MASK;
-- 
2.7.4

[Qemu-devel] [PATCH 07/15] target/arm: Fix outdated comment about exception exit

[Peter Maydell]

When we switched our handling of exception exit to detect
the magic addresses at translate time rather than via
a do_unassigned_access hook, we forgot to update a
comment; correct the omission.

Signed-off-by: Peter Maydell 
---
 target/arm/helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index fd83a21..cb88c66 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6143,7 +6143,7 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
 bool rettobase = false;
 
 /* We can only get here from an EXCP_EXCEPTION_EXIT, and
- * arm_v7m_do_unassigned_access() enforces the architectural rule
+ * gen_bx_excret() enforces the architectural rule
  * that jumps to magic addresses don't have magic behaviour unless
  * we're in Handler mode (compare pseudocode BXWritePC()).
  */
-- 
2.7.4

[Qemu-devel] [PATCH 10/15] target/arm: Don't use cpsr_write/cpsr_read to transfer M profile XPSR

[Peter Maydell]

For M profile the XPSR is a similar but not identical format to the
A profile CPSR/SPSR. (For instance the Thumb bit is in a different
place.) For guest accesses we make the M profile code go through
xpsr_read() and xpsr_write() which handle the different layout.
However for migration we use cpsr_read() and cpsr_write() to
marshal state into and out of the migration data stream. This
is pretty confusing and works more by luck than anything else.
Make M profile migration use xpsr_read() and xpsr_write() instead.

The most complicated part of this is handling the possibility
that the migration source is an older QEMU which hands us a
CPSR format value; helpfully we can always tell the two apart.

Signed-off-by: Peter Maydell 
---
 target/arm/machine.c | 49 ++---
 1 file changed, 34 insertions(+), 15 deletions(-)

diff --git a/target/arm/machine.c b/target/arm/machine.c
index 2fb4b76..3193b00 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -217,21 +217,37 @@ static int get_cpsr(QEMUFile *f, void *opaque, size_t 
size,
 uint32_t val = qemu_get_be32(f);
 
 if (arm_feature(env, ARM_FEATURE_M)) {
-/* If the I or F bits are set then this is a migration from
- * an old QEMU which still stored the M profile FAULTMASK
- * and PRIMASK in env->daif. Set v7m.faultmask and v7m.primask
- * accordingly, and then clear the bits so they don't confuse
- * cpsr_write(). For a new QEMU, the bits here will always be
- * clear, and the data is transferred using the
- * vmstate_m_faultmask_primask subsection.
- */
-if (val & CPSR_F) {
-env->v7m.faultmask = 1;
-}
-if (val & CPSR_I) {
-env->v7m.primask = 1;
+if (val & XPSR_EXCP) {
+/* This is a CPSR format value from an older QEMU. (We can tell
+ * because values transferred in XPSR format always have zero
+ * for the EXCP field, and CPSR format will always have bit 4
+ * set in CPSR_M.) Rearrange it into XPSR format. The significant
+ * differences are that the T bit is not in the same place, the
+ * primask/faultmask info may be in the CPSR I and F bits, and
+ * we do not want the mode bits.
+ */
+uint32_t newval = val;
+
+newval &= (CPSR_NZCV | CPSR_Q | CPSR_IT | CPSR_GE);
+if (val & CPSR_T) {
+newval |= XPSR_T;
+}
+/* If the I or F bits are set then this is a migration from
+ * an old QEMU which still stored the M profile FAULTMASK
+ * and PRIMASK in env->daif. For a new QEMU, the data is
+ * transferred using the vmstate_m_faultmask_primask subsection.
+ */
+if (val & CPSR_F) {
+env->v7m.faultmask = 1;
+}
+if (val & CPSR_I) {
+env->v7m.primask = 1;
+}
+val = newval;
 }
-val &= ~(CPSR_F | CPSR_I);
+/* Ignore the low bits, they are handled by vmstate_m. */
+xpsr_write(env, val, ~XPSR_EXCP);
+return 0;
 }
 
 env->aarch64 = ((val & PSTATE_nRW) == 0);
@@ -252,7 +268,10 @@ static int put_cpsr(QEMUFile *f, void *opaque, size_t size,
 CPUARMState *env = >env;
 uint32_t val;
 
-if (is_a64(env)) {
+if (arm_feature(env, ARM_FEATURE_M)) {
+/* The low 9 bits are v7m.exception, which is handled by vmstate_m. */
+val = xpsr_read(env) & ~XPSR_EXCP;
+} else if (is_a64(env)) {
 val = pstate_read(env);
 } else {
 val = cpsr_read(env);
-- 
2.7.4

[Qemu-devel] [PATCH 01/15] target/arm: Use MMUAccessType enum rather than int

[Peter Maydell]

In the ARM get_phys_addr() code, switch to using the MMUAccessType
enum and its MMU_* values rather than int and literal 0/1/2.

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

diff --git a/target/arm/helper.c b/target/arm/helper.c
index fa60040..b78d277 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -20,13 +20,13 @@
 
 #ifndef CONFIG_USER_ONLY
 static bool get_phys_addr(CPUARMState *env, target_ulong address,
-  int access_type, ARMMMUIdx mmu_idx,
+  MMUAccessType access_type, ARMMMUIdx mmu_idx,
   hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
   target_ulong *page_size, uint32_t *fsr,
   ARMMMUFaultInfo *fi);
 
 static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
-   int access_type, ARMMMUIdx mmu_idx,
+   MMUAccessType access_type, ARMMMUIdx mmu_idx,
hwaddr *phys_ptr, MemTxAttrs *txattrs, int 
*prot,
target_ulong *page_size_ptr, uint32_t *fsr,
ARMMMUFaultInfo *fi);
@@ -2135,7 +2135,7 @@ static CPAccessResult ats_access(CPUARMState *env, const 
ARMCPRegInfo *ri,
 }
 
 static uint64_t do_ats_write(CPUARMState *env, uint64_t value,
- int access_type, ARMMMUIdx mmu_idx)
+ MMUAccessType access_type, ARMMMUIdx mmu_idx)
 {
 hwaddr phys_addr;
 target_ulong page_size;
@@ -2194,7 +2194,7 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t 
value,
 
 static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
 {
-int access_type = ri->opc2 & 1;
+MMUAccessType access_type = ri->opc2 & 1 ? MMU_DATA_STORE : MMU_DATA_LOAD;
 uint64_t par64;
 ARMMMUIdx mmu_idx;
 int el = arm_current_el(env);
@@ -2253,7 +2253,7 @@ static void ats_write(CPUARMState *env, const 
ARMCPRegInfo *ri, uint64_t value)
 static void ats1h_write(CPUARMState *env, const ARMCPRegInfo *ri,
 uint64_t value)
 {
-int access_type = ri->opc2 & 1;
+MMUAccessType access_type = ri->opc2 & 1 ? MMU_DATA_STORE : MMU_DATA_LOAD;
 uint64_t par64;
 
 par64 = do_ats_write(env, value, access_type, ARMMMUIdx_S2NS);
@@ -2273,7 +2273,7 @@ static CPAccessResult at_s1e2_access(CPUARMState *env, 
const ARMCPRegInfo *ri,
 static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri,
 uint64_t value)
 {
-int access_type = ri->opc2 & 1;
+MMUAccessType access_type = ri->opc2 & 1 ? MMU_DATA_STORE : MMU_DATA_LOAD;
 ARMMMUIdx mmu_idx;
 int secure = arm_is_secure_below_el3(env);
 
@@ -7510,7 +7510,7 @@ static uint64_t arm_ldq_ptw(CPUState *cs, hwaddr addr, 
bool is_secure,
 }
 
 static bool get_phys_addr_v5(CPUARMState *env, uint32_t address,
- int access_type, ARMMMUIdx mmu_idx,
+ MMUAccessType access_type, ARMMMUIdx mmu_idx,
  hwaddr *phys_ptr, int *prot,
  target_ulong *page_size, uint32_t *fsr,
  ARMMMUFaultInfo *fi)
@@ -7626,7 +7626,7 @@ do_fault:
 }
 
 static bool get_phys_addr_v6(CPUARMState *env, uint32_t address,
- int access_type, ARMMMUIdx mmu_idx,
+ MMUAccessType access_type, ARMMMUIdx mmu_idx,
  hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
  target_ulong *page_size, uint32_t *fsr,
  ARMMMUFaultInfo *fi)
@@ -7733,7 +7733,7 @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t 
address,
 if (pxn && !regime_is_user(env, mmu_idx)) {
 xn = 1;
 }
-if (xn && access_type == 2)
+if (xn && access_type == MMU_INST_FETCH)
 goto do_fault;
 
 if (arm_feature(env, ARM_FEATURE_V6K) &&
@@ -7848,7 +7848,7 @@ static bool check_s2_mmu_setup(ARMCPU *cpu, bool is_aa64, 
int level,
 }
 
 static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
-   int access_type, ARMMMUIdx mmu_idx,
+   MMUAccessType access_type, ARMMMUIdx mmu_idx,
hwaddr *phys_ptr, MemTxAttrs *txattrs, int 
*prot,
target_ulong *page_size_ptr, uint32_t *fsr,
ARMMMUFaultInfo *fi)
@@ -8256,7 +8256,7 @@ static inline bool m_is_system_region(CPUARMState *env, 
uint32_t address)
 }
 
 static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
- int access_type, ARMMMUIdx mmu_idx,
+ MMUAccessType 

[Qemu-devel] [PATCH 15/15] nvic: Implement "user accesses BusFault" SCS region behaviour

[Peter Maydell]

The ARMv7M architecture specifies that most of the addresses in the
PPB region (which includes the NVIC, systick and system registers)
are not accessible to unprivileged accesses, which should
BusFault with a few exceptions:
 * the STIR is configurably user-accessible
 * the ITM (which we don't implement at all) is always
   user-accessible

Implement this by switching the register access functions
to the _with_attrs scheme that lets us distinguish user
mode accesses.

This allows us to pull the handling of the CCR.USERSETMPEND
flag up to the level where we can make it generate a BusFault
as it should for non-permitted accesses.

Note that until the core ARM CPU code implements turning
MEMTX_ERROR into a BusFault the registers will continue to
act as RAZ/WI to user accesses.

Signed-off-by: Peter Maydell 
---
 hw/intc/armv7m_nvic.c | 58 ---
 1 file changed, 41 insertions(+), 17 deletions(-)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 5a18025..bbfe2d5 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -733,11 +733,8 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value)
 }
 case 0xf00: /* Software Triggered Interrupt Register */
 {
-/* user mode can only write to STIR if CCR.USERSETMPEND permits it */
 int excnum = (value & 0x1ff) + NVIC_FIRST_IRQ;
-if (excnum < s->num_irq &&
-(arm_current_el(>env) ||
- (cpu->env.v7m.ccr & R_V7M_CCR_USERSETMPEND_MASK))) {
+if (excnum < s->num_irq) {
 armv7m_nvic_set_pending(s, excnum);
 }
 break;
@@ -748,14 +745,32 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value)
 }
 }
 
-static uint64_t nvic_sysreg_read(void *opaque, hwaddr addr,
- unsigned size)
+static bool nvic_user_access_ok(NVICState *s, hwaddr offset)
+{
+/* Return true if unprivileged access to this register is permitted. */
+switch (offset) {
+case 0xf00: /* STIR: accessible only if CCR.USERSETMPEND permits */
+return s->cpu->env.v7m.ccr & R_V7M_CCR_USERSETMPEND_MASK;
+default:
+/* All other user accesses cause a BusFault unconditionally */
+return false;
+}
+}
+
+static MemTxResult nvic_sysreg_read(void *opaque, hwaddr addr,
+uint64_t *data, unsigned size,
+MemTxAttrs attrs)
 {
 NVICState *s = (NVICState *)opaque;
 uint32_t offset = addr;
 unsigned i, startvec, end;
 uint32_t val;
 
+if (attrs.user && !nvic_user_access_ok(s, addr)) {
+/* Generate BusFault for unprivileged accesses */
+return MEMTX_ERROR;
+}
+
 switch (offset) {
 /* reads of set and clear both return the status */
 case 0x100 ... 0x13f: /* NVIC Set enable */
@@ -826,11 +841,13 @@ static uint64_t nvic_sysreg_read(void *opaque, hwaddr 
addr,
 }
 
 trace_nvic_sysreg_read(addr, val, size);
-return val;
+*data = val;
+return MEMTX_OK;
 }
 
-static void nvic_sysreg_write(void *opaque, hwaddr addr,
-  uint64_t value, unsigned size)
+static MemTxResult nvic_sysreg_write(void *opaque, hwaddr addr,
+ uint64_t value, unsigned size,
+ MemTxAttrs attrs)
 {
 NVICState *s = (NVICState *)opaque;
 uint32_t offset = addr;
@@ -839,6 +856,11 @@ static void nvic_sysreg_write(void *opaque, hwaddr addr,
 
 trace_nvic_sysreg_write(addr, value, size);
 
+if (attrs.user && !nvic_user_access_ok(s, addr)) {
+/* Generate BusFault for unprivileged accesses */
+return MEMTX_ERROR;
+}
+
 switch (offset) {
 case 0x100 ... 0x13f: /* NVIC Set enable */
 offset += 0x80;
@@ -853,7 +875,7 @@ static void nvic_sysreg_write(void *opaque, hwaddr addr,
 }
 }
 nvic_irq_update(s);
-return;
+return MEMTX_OK;
 case 0x200 ... 0x23f: /* NVIC Set pend */
 /* the special logic in armv7m_nvic_set_pending()
  * is not needed since IRQs are never escalated
@@ -870,9 +892,9 @@ static void nvic_sysreg_write(void *opaque, hwaddr addr,
 }
 }
 nvic_irq_update(s);
-return;
+return MEMTX_OK;
 case 0x300 ... 0x33f: /* NVIC Active */
-return; /* R/O */
+return MEMTX_OK; /* R/O */
 case 0x400 ... 0x5ef: /* NVIC Priority */
 startvec = 8 * (offset - 0x400) + NVIC_FIRST_IRQ; /* vector # */
 
@@ -880,26 +902,28 @@ static void nvic_sysreg_write(void *opaque, hwaddr addr,
 set_prio(s, startvec + i, (value >> (i * 8)) & 0xff);
 }
 nvic_irq_update(s);
-return;
+return MEMTX_OK;
 case 0xd18 ... 0xd23: /* System Handler Priority.  */
 for (i = 0; i < size; i++) {
 unsigned hdlidx = (offset - 0xd14) 

[Qemu-devel] [PATCH 14/15] armv7m_nvic.h: Move from include/hw/arm to include/hw/intc

[Peter Maydell]

The armv7m_nvic.h header file was accidentally placed in
include/hw/arm; move it to include/hw/intc to match where
its corresponding .c file lives.

Signed-off-by: Peter Maydell 
---
 hw/intc/armv7m_nvic.c  | 2 +-
 include/hw/arm/armv7m.h| 2 +-
 include/hw/{arm => intc}/armv7m_nvic.h | 0
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename include/hw/{arm => intc}/armv7m_nvic.h (100%)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 343bc16..5a18025 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -17,7 +17,7 @@
 #include "hw/sysbus.h"
 #include "qemu/timer.h"
 #include "hw/arm/arm.h"
-#include "hw/arm/armv7m_nvic.h"
+#include "hw/intc/armv7m_nvic.h"
 #include "target/arm/cpu.h"
 #include "exec/exec-all.h"
 #include "qemu/log.h"
diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
index a9b3f2a..10eb058 100644
--- a/include/hw/arm/armv7m.h
+++ b/include/hw/arm/armv7m.h
@@ -11,7 +11,7 @@
 #define HW_ARM_ARMV7M_H
 
 #include "hw/sysbus.h"
-#include "hw/arm/armv7m_nvic.h"
+#include "hw/intc/armv7m_nvic.h"
 
 #define TYPE_BITBAND "ARM,bitband-memory"
 #define BITBAND(obj) OBJECT_CHECK(BitBandState, (obj), TYPE_BITBAND)
diff --git a/include/hw/arm/armv7m_nvic.h b/include/hw/intc/armv7m_nvic.h
similarity index 100%
rename from include/hw/arm/armv7m_nvic.h
rename to include/hw/intc/armv7m_nvic.h
-- 
2.7.4

Re: [Qemu-devel] [PATCH v3 5/5] docs: update documentation considering PCIE-PCI bridge

[Michael S. Tsirkin]

On Wed, Aug 02, 2017 at 06:36:29PM +0300, Marcel Apfelbaum wrote:
> > > > > > Can dmi-pci support shpc? why doesn't it? For compatibility?
> > > > > 
> > > > > I don't know why, but the fact that it doesn't is the reason libvirt
> > > > > settled on auto-creating a dmi-pci bridge and a pci-pci bridge under
> > > > > that for Q35. The reasoning was (IIRC Laine's words correctly) that 
> > > > > the
> > > > > dmi-pci bridge cannot receive hotplugged devices, while the pci-pci
> > > > > bridge cannot be connected to the root complex. So both were needed.
> > > > > 
> > > > > Thanks
> > > > > Laszlo
> > > > 
> > > > OK. Is it true that dmi-pci + pci-pci under it will allow hotplug
> > > > on Q35 if we just flip the bit in _OSC?
> > > 
> > > Marcel, what say you?... :)
> 
> Good news, works with:
>-device i82801b11-bridge,id=b1
>-device pci-bridge,id=b2,bus=b1,chassis_nr=1,msi=off

And presumably it works for modern windows?
OK, so it looks like patch 1 is merely a bugfix, I'll merge it for 2.10.

> Notice bridge's msi=off until the following kernel bug will be merged:
>   https://www.spinics.net/lists/linux-pci/msg63052.html

Does libvirt support msi=off as a work-around?

> 
> Thanks,
> Marcel

-- 
MST

Re: [Qemu-devel] [PATCH] docs/pcie.txt: Replace ioh3420 with pcie-root-port

[Laszlo Ersek]

On 08/02/17 17:51, Marcel Apfelbaum wrote:
> Do not mention ioh3420 in the "how to" doc.
> The device still works and can be used by already
> existing setups, but no need to be mentioned.
> 
> Suggested-by: Andrew Jones 
> Signed-off-by: Marcel Apfelbaum 
> ---
>  docs/pcie.txt | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/docs/pcie.txt b/docs/pcie.txt
> index 5bada24..f990033 100644
> --- a/docs/pcie.txt
> +++ b/docs/pcie.txt
> @@ -43,8 +43,8 @@ Place only the following kinds of devices directly on the 
> Root Complex:
>  strangely when PCI Express devices are integrated
>  with the Root Complex.
>  
> -(2) PCI Express Root Ports (ioh3420), for starting exclusively PCI 
> Express
> -hierarchies.
> +(2) PCI Express Root Ports (pcie-root-port), for starting exclusively
> +PCI Express hierarchies.
>  
>  (3) DMI-PCI Bridges (i82801b11-bridge), for starting legacy PCI
>  hierarchies.
> @@ -65,7 +65,7 @@ Place only the following kinds of devices directly on the 
> Root Complex:
>-device pxb-pcie,id=pcie.1,bus_nr=x[,numa_node=y][,addr=z]
>Only PCI Express Root Ports and DMI-PCI bridges can be connected
>to the pcie.1 bus:
> -  -device 
> ioh3420,id=root_port1[,bus=pcie.1][,chassis=x][,slot=y][,addr=z]  
>\
> +  -device 
> pcie-root-port,id=root_port1[,bus=pcie.1][,chassis=x][,slot=y][,addr=z]   
>   \
>-device i82801b11-bridge,id=dmi_pci_bridge1,bus=pcie.1
>  
>  
> @@ -107,14 +107,14 @@ Plug only PCI Express devices into PCI Express Ports.
>   
>  
>  2.2.1 Plugging a PCI Express device into a PCI Express Root Port:
> -  -device 
> ioh3420,id=root_port1,chassis=x,slot=y[,bus=pcie.0][,addr=z]  \
> +  -device 
> pcie-root-port,id=root_port1,chassis=x,slot=y[,bus=pcie.0][,addr=z]  \
>-device ,bus=root_port1
>  2.2.2 Using multi-function PCI Express Root Ports:
> -  -device 
> ioh3420,id=root_port1,multifunction=on,chassis=x,addr=z.0[,slot=y][,bus=pcie.0]
>  \
> -  -device 
> ioh3420,id=root_port2,chassis=x1,addr=z.1[,slot=y1][,bus=pcie.0] \
> -  -device 
> ioh3420,id=root_port3,chassis=x2,addr=z.2[,slot=y2][,bus=pcie.0] \
> +  -device 
> pcie-root-port,id=root_port1,multifunction=on,chassis=x,addr=z.0[,slot=y][,bus=pcie.0]
>  \
> +  -device 
> pcie-root-port,id=root_port2,chassis=x1,addr=z.1[,slot=y1][,bus=pcie.0] \
> +  -device 
> pcie-root-port,id=root_port3,chassis=x2,addr=z.2[,slot=y2][,bus=pcie.0] \
>  2.2.3 Plugging a PCI Express device into a Switch:
> -  -device ioh3420,id=root_port1,chassis=x,slot=y[,bus=pcie.0][,addr=z]  \
> +  -device 
> pcie-root-port,id=root_port1,chassis=x,slot=y[,bus=pcie.0][,addr=z]  \
>-device x3130-upstream,id=upstream_port1,bus=root_port1[,addr=x]   
>\
>-device 
> xio3130-downstream,id=downstream_port1,bus=upstream_port1,chassis=x1,slot=y1[,addr=z1]]
>  \
>-device ,bus=downstream_port1
> 

I trust that you found all occurrences of "ioh3420" in the doc :)

Reviewed-by: Laszlo Ersek 

Thanks
Laszlo

[Qemu-devel] [PATCH] docs/pcie.txt: Replace ioh3420 with pcie-root-port

[Marcel Apfelbaum]

Do not mention ioh3420 in the "how to" doc.
The device still works and can be used by already
existing setups, but no need to be mentioned.

Suggested-by: Andrew Jones 
Signed-off-by: Marcel Apfelbaum 
---
 docs/pcie.txt | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/docs/pcie.txt b/docs/pcie.txt
index 5bada24..f990033 100644
--- a/docs/pcie.txt
+++ b/docs/pcie.txt
@@ -43,8 +43,8 @@ Place only the following kinds of devices directly on the 
Root Complex:
 strangely when PCI Express devices are integrated
 with the Root Complex.
 
-(2) PCI Express Root Ports (ioh3420), for starting exclusively PCI Express
-hierarchies.
+(2) PCI Express Root Ports (pcie-root-port), for starting exclusively
+PCI Express hierarchies.
 
 (3) DMI-PCI Bridges (i82801b11-bridge), for starting legacy PCI
 hierarchies.
@@ -65,7 +65,7 @@ Place only the following kinds of devices directly on the 
Root Complex:
   -device pxb-pcie,id=pcie.1,bus_nr=x[,numa_node=y][,addr=z]
   Only PCI Express Root Ports and DMI-PCI bridges can be connected
   to the pcie.1 bus:
-  -device 
ioh3420,id=root_port1[,bus=pcie.1][,chassis=x][,slot=y][,addr=z]
 \
+  -device 
pcie-root-port,id=root_port1[,bus=pcie.1][,chassis=x][,slot=y][,addr=z] 
\
   -device i82801b11-bridge,id=dmi_pci_bridge1,bus=pcie.1
 
 
@@ -107,14 +107,14 @@ Plug only PCI Express devices into PCI Express Ports.
  
 
 2.2.1 Plugging a PCI Express device into a PCI Express Root Port:
-  -device ioh3420,id=root_port1,chassis=x,slot=y[,bus=pcie.0][,addr=z] 
 \
+  -device 
pcie-root-port,id=root_port1,chassis=x,slot=y[,bus=pcie.0][,addr=z]  \
   -device ,bus=root_port1
 2.2.2 Using multi-function PCI Express Root Ports:
-  -device 
ioh3420,id=root_port1,multifunction=on,chassis=x,addr=z.0[,slot=y][,bus=pcie.0] 
\
-  -device ioh3420,id=root_port2,chassis=x1,addr=z.1[,slot=y1][,bus=pcie.0] 
\
-  -device ioh3420,id=root_port3,chassis=x2,addr=z.2[,slot=y2][,bus=pcie.0] 
\
+  -device 
pcie-root-port,id=root_port1,multifunction=on,chassis=x,addr=z.0[,slot=y][,bus=pcie.0]
 \
+  -device 
pcie-root-port,id=root_port2,chassis=x1,addr=z.1[,slot=y1][,bus=pcie.0] \
+  -device 
pcie-root-port,id=root_port3,chassis=x2,addr=z.2[,slot=y2][,bus=pcie.0] \
 2.2.3 Plugging a PCI Express device into a Switch:
-  -device ioh3420,id=root_port1,chassis=x,slot=y[,bus=pcie.0][,addr=z]  \
+  -device 
pcie-root-port,id=root_port1,chassis=x,slot=y[,bus=pcie.0][,addr=z]  \
   -device x3130-upstream,id=upstream_port1,bus=root_port1[,addr=x] 
 \
   -device 
xio3130-downstream,id=downstream_port1,bus=upstream_port1,chassis=x1,slot=y1[,addr=z1]]
 \
   -device ,bus=downstream_port1
-- 
2.9.4

Re: [Qemu-devel] [PATCH v3 5/5] docs: update documentation considering PCIE-PCI bridge

[Marcel Apfelbaum]

On 02/08/2017 17:21, Marcel Apfelbaum wrote:

On 02/08/2017 17:16, Laszlo Ersek wrote:

On 08/02/17 15:47, Michael S. Tsirkin wrote:

On Wed, Aug 02, 2017 at 12:23:46AM +0200, Laszlo Ersek wrote:

On 08/01/17 23:39, Michael S. Tsirkin wrote:

On Wed, Aug 02, 2017 at 12:33:12AM +0300, Alexander Bezzubikov wrote:

2017-08-01 23:31 GMT+03:00 Laszlo Ersek :
(Whenever my comments conflict with Michael's or Marcel's, I 
defer to them.)


On 07/29/17 01:37, Aleksandr Bezzubikov wrote:

Signed-off-by: Aleksandr Bezzubikov 
---
  docs/pcie.txt|  46 ++
  docs/pcie_pci_bridge.txt | 121 
+++

  2 files changed, 147 insertions(+), 20 deletions(-)
  create mode 100644 docs/pcie_pci_bridge.txt

diff --git a/docs/pcie.txt b/docs/pcie.txt
index 5bada24..338b50e 100644
--- a/docs/pcie.txt
+++ b/docs/pcie.txt
@@ -46,7 +46,7 @@ Place only the following kinds of devices 
directly on the Root Complex:
  (2) PCI Express Root Ports (ioh3420), for starting 
exclusively PCI Express

  hierarchies.

-(3) DMI-PCI Bridges (i82801b11-bridge), for starting legacy 
PCI

+(3) PCIE-PCI Bridge (pcie-pci-bridge), for starting legacy PCI
  hierarchies.

  (4) Extra Root Complexes (pxb-pcie), if multiple PCI 
Express Root Buses


When reviewing previous patches modifying / adding this file, I
requested that we spell out "PCI Express" every single time. I'd 
like to

see the same in this patch, if possible.


OK, I didn't know it.



@@ -55,18 +55,18 @@ Place only the following kinds of devices 
directly on the Root Complex:

 pcie.0 bus
 
 

  ||
|  |
-   ---   --   --   
--
-   | PCI Dev |   | PCIe Root Port |   | DMI-PCI Bridge |   |  
pxb-pcie  |
-   ---   --   --   
--
+   ---   --   ---   
--
+   | PCI Dev |   | PCIe Root Port |   | PCIE-PCI Bridge |   |  
pxb-pcie  |
+   ---   --   ---   
--


  2.1.1 To plug a device into pcie.0 as a Root Complex 
Integrated Endpoint use:

-device [,bus=pcie.0]
  2.1.2 To expose a new PCI Express Root Bus use:
-device 
pxb-pcie,id=pcie.1,bus_nr=x[,numa_node=y][,addr=z]
-  Only PCI Express Root Ports and DMI-PCI bridges can be 
connected
+  Only PCI Express Root Ports, PCIE-PCI bridges and DMI-PCI 
bridges can be connected


It would be nice if we could keep the flowing text wrapped to 80 
chars.


Also, here you add the "PCI Express-PCI" bridge to the list of 
allowed

controllers (and you keep DMI-PCI as permitted), but above DMI was
replaced. I think these should be made consistent -- we should 
make up
our minds if we continue to recommend the DMI-PCI bridge or not. 
If not,

then we should eradicate all traces of it. If we want to keep it at
least for compatibility, then it should remain as fully 
documented as it

is now.


Now I'm beginning to think that we shouldn't keep the DMI-PCI bridge
even for compatibility and may want to use a new PCIE-PCI bridge
everywhere (of course, except some cases when users are
sure they need exactly DMI-PCI bridge for some reason)


Can dmi-pci support shpc? why doesn't it? For compatibility?


I don't know why, but the fact that it doesn't is the reason libvirt
settled on auto-creating a dmi-pci bridge and a pci-pci bridge under
that for Q35. The reasoning was (IIRC Laine's words correctly) that the
dmi-pci bridge cannot receive hotplugged devices, while the pci-pci
bridge cannot be connected to the root complex. So both were needed.

Thanks
Laszlo


OK. Is it true that dmi-pci + pci-pci under it will allow hotplug
on Q35 if we just flip the bit in _OSC?


Marcel, what say you?... :)


Good news, works with:
   -device i82801b11-bridge,id=b1
   -device pci-bridge,id=b2,bus=b1,chassis_nr=1,msi=off

Notice bridge's msi=off until the following kernel bug will be merged:
  https://www.spinics.net/lists/linux-pci/msg63052.html


Thanks,
Marcel





Will test and get back to you (it may actually work)

Thanks,
Marcel

Re: [Qemu-devel] [PATCH] tests/hmp: Fix typo in the 'chardev-send-break' test

[Peter Maydell]

On 27 July 2017 at 10:55, Dr. David Alan Gilbert  wrote:
> * Thomas Huth (th...@redhat.com) wrote:
>> testchardev2 is not a valid chardev id here. Use testchardev1
>> instead which has been created with chardev-add right before
>> the 'chardev-send-break' line.
>> And while we're at it, add the test-hmp.c file to the MAINTAINERS
>> file, too.
>>
>> Signed-off-by: Thomas Huth 
>
>
> Reviewed-by: Dr. David Alan Gilbert 

Thanks, applied to master.

-- PMM

Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation

[Lluís Vilanova]

Stefan Hajnoczi writes:

> On Wed, Aug 02, 2017 at 12:10:14PM +0100, Peter Maydell wrote:
>> On 2 August 2017 at 12:04, Stefan Hajnoczi  wrote:
>> > On Tue, Aug 01, 2017 at 02:54:29PM +0100, Peter Maydell wrote:
>> >> and I don't need the TCG engine to be a library to do that...
>> >
>> > You do need TCG APIs if you want TCG-level instrumentation, tuning
>> > options, callbacks, etc.
>> 
>> I need an API; that doesn't necessarily look like the kind
>> of API you want to be able to embed the TCG engine into
>> other things, I think.
>> 
>> >> I agree that we want to provide something that is at least
>> >> closer to a stable API than "just expose trace events",
>> >> though.
>> >
>> > libqemu has at least three parts:
>> >
>> > 1. VM API (i.e. qemu_init(argc, argv), qemu_run(), qemu_vcpu_get_reg32())
>> > 2. TCG engine
>> > 3. Device models
>> >
>> > Like I said in my email, start with what matters for the instrumentation
>> > use case (VM API at a minimum to control guest execution).  Other people
>> > can flesh out the other parts later, as needed.
>> >
>> > Other attempts to provide a stable API will be essentially the same
>> > thing as libqemu.
>> 
>> I don't think this is the case -- you could have a stable
>> instrumentation API without it looking anything like
>> libqemu. In particular I don't think you need to have
>> something that sits at the top level and says 'run'.
>> 
>> In particular I think that pulling TCG out of QEMU
>> is an enormous and painful undertaking that you just
>> don't need to do at all to allow this kind of
>> instrumentation API.

> Please post an example of the API you'd like.

In my opinion, the instrumentation support in this series provides an API that
works in the opposite way you're suggesting (let's ignore the fact that it's
built on tracing events).

When QEMU loads an instrumentation library (which can happen at any time), some
initialization function is called on the library so that it can establish what
events to instrument. This also has the advantage that a user can hook into a
running QEMU instance at any time to perform some instrumentation.

I think this is the bare minimum necessary to make it work, and has the upside
of being completely orthogonal to the libqemu approach. We could reuse most of
the stable instrumentation API there too, except for the instrumentation code
initialization.

That being said, the libqemu approach *might* make it a bit easier to provide an
API for things such as "run for this many instructions and return control to
instrumentor", but I don't think that's mandatory for a first prototype (and can
definitely be implemented using both approaches).

Cheers,
  Lluis

Re: [Qemu-devel] [PATCH for-2.10 2/3] target/mips: Drop redundant gen_io_start/stop()

[Richard Henderson]

On 08/02/2017 02:59 AM, James Hogan wrote:

DMTC0 CP0_Cause does a redundant gen_io_start() and gen_io_end() pair,
even though this is done for all DMTC0 operations outside of the switch
statement. Remove these redundant calls.

Fixes: 5dc5d9f055c5 ("mips: more fixes to the MIPS interrupt glue logic")
Signed-off-by: James Hogan
Cc: Yongbok Kim
Cc: Aurelien Jarno
---
  target/mips/translate.c | 8 
  1 file changed, 0 insertions(+), 8 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH for-2.10 3/3] target/mips: Fix RDHWR CC with icount

[Richard Henderson]

On 08/02/2017 02:59 AM, James Hogan wrote:

RDHWR CC reads the CPU timer like MFC0 CP0_Count, so with icount enabled
it must set can_do_io while it calls the helper to avoid the "Bad icount
read" error. It should also break out of the translation loop to ensure
that timer interrupts are immediately handled.

Fixes: 2e70f6efa8b9 ("Add instruction counter.")
Signed-off-by: James Hogan
Cc: Aurelien Jarno
Cc: Yongbok Kim
---
I've based this on MFC0 Count, but this instruction is also available to
usermode (e.g. CONFIG_USER_ONLY), which I presume is still fine.
---
  target/mips/translate.c | 11 +++
  1 file changed, 11 insertions(+), 0 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH for-2.10 1/3] target/mips: Use BS_EXCP where interrupts are expected

[Richard Henderson]

On 08/02/2017 02:59 AM, James Hogan wrote:

Commit e350d8ca3ac7 ("target/mips: optimize indirect branches") made
indirect branches able to directly find the next TB and jump straight to
it without breaking out of translated code and going around the main
execution loop. This breaks the assumption in target/mips/translate.c
that BS_STOP is sufficient to cause pending interrupts to be handled,
since interrupts are only checked in the main loop.

Fix a few of these assumptions by using gen_save_pc to update the saved
PC and using BS_EXCP instead of BS_STOP:


I don't like the name, BS_EXCP, because this isn't an exception.

But, since one of the first things we'll do in 2.11 is land LLuis' generic 
translation framework, and rename all of these exit conditions, this is ok.  It 
certainly is the minimal change for 2.10, and this does work.


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PULL 0/3] migration queue

[Peter Maydell]

On 2 August 2017 at 14:58, Dr. David Alan Gilbert (git)
 wrote:
> From: "Dr. David Alan Gilbert" 
>
> The following changes since commit d3d183a638d6a3ead515618a6547b3f80d39fcb9:
>
>   Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging 
> (2017-08-02 09:49:02 +0100)
>
> are available in the git repository at:
>
>   git://github.com/dagrh/qemu.git tags/pull-migration-20170802a
>
> for you to fetch changes up to 8bd9c4e6c565c566a6cba3470cb2d4ea63994143:
>
>   io: fix qio_channel_socket_accept err handling (2017-08-02 11:27:44 +0100)
>
> 
> Migration pull 2017-08-02
>
> Just minor fixes for 2.10

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation

[Stefan Hajnoczi]

On Wed, Aug 02, 2017 at 12:10:14PM +0100, Peter Maydell wrote:
> On 2 August 2017 at 12:04, Stefan Hajnoczi  wrote:
> > On Tue, Aug 01, 2017 at 02:54:29PM +0100, Peter Maydell wrote:
> >> and I don't need the TCG engine to be a library to do that...
> >
> > You do need TCG APIs if you want TCG-level instrumentation, tuning
> > options, callbacks, etc.
> 
> I need an API; that doesn't necessarily look like the kind
> of API you want to be able to embed the TCG engine into
> other things, I think.
> 
> >> I agree that we want to provide something that is at least
> >> closer to a stable API than "just expose trace events",
> >> though.
> >
> > libqemu has at least three parts:
> >
> > 1. VM API (i.e. qemu_init(argc, argv), qemu_run(), qemu_vcpu_get_reg32())
> > 2. TCG engine
> > 3. Device models
> >
> > Like I said in my email, start with what matters for the instrumentation
> > use case (VM API at a minimum to control guest execution).  Other people
> > can flesh out the other parts later, as needed.
> >
> > Other attempts to provide a stable API will be essentially the same
> > thing as libqemu.
> 
> I don't think this is the case -- you could have a stable
> instrumentation API without it looking anything like
> libqemu. In particular I don't think you need to have
> something that sits at the top level and says 'run'.
> 
> In particular I think that pulling TCG out of QEMU
> is an enormous and painful undertaking that you just
> don't need to do at all to allow this kind of
> instrumentation API.

Please post an example of the API you'd like.

Stefan


signature.asc
Description: PGP signature

[Qemu-devel] KVM call for 2017-08-15

[Juan Quintela]

Hi

Please, send any topic that you are interested in covering.

At the end of Monday I will send an email with the agenda or the
cancellation of the call, so hurry up.

After discussions on the QEMU Summit, we are going to have always open a
KVM call where you can add topics.

 Call details:

By popular demand, a google calendar public entry with it

  
https://www.google.com/calendar/embed?src=dG9iMXRqcXAzN3Y4ZXZwNzRoMHE4a3BqcXNAZ3JvdXAuY2FsZW5kYXIuZ29vZ2xlLmNvbQ

(Let me know if you have any problems with the calendar entry.  I just
gave up about getting right at the same time CEST, CET, EDT and DST).

If you need phone number details,  contact me privately

Thanks, Juan.

[Qemu-devel] [PATCH] kvm: workaround build break on gcc-7.1.1 / fedora26

[Greg Kurz]

Building QEMU on fedora26 with the latest gcc package fails:

  CC  ppc64-softmmu/target/ppc/kvm.o
In file included from include/sysemu/hw_accel.h:16:0,
 from target/ppc/kvm.c:31:
target/ppc/kvm.c: In function ‘kvmppc_booke_watchdog_enable’:
include/sysemu/kvm.h:449:35: error: ‘args_tmp[i]’ may be used uninitialized
 in this function [-Werror=maybe-uninitialized]
 cap.args[i] = args_tmp[i];   \
   ^
target/ppc/kvm.c: In function ‘kvmppc_set_papr’:
include/sysemu/kvm.h:449:35: error: ‘args_tmp[i]’ may be used uninitialized
 in this function [-Werror=maybe-uninitialized]
cc1: all warnings being treated as errors

$ rpm -q gcc
gcc-7.1.1-3.fc26.ppc64le

Testing the size of args_tmp seems to be enough to prevent the warning
to pop up.

Signed-off-by: Greg Kurz 
---
 include/sysemu/kvm.h |   12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 91fc07ee9afe..41fc1005a9ea 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -429,9 +429,11 @@ int kvm_vm_check_extension(KVMState *s, unsigned int 
extension);
 };   \
 uint64_t args_tmp[] = { __VA_ARGS__ };   \
 int i;   \
-for (i = 0; i < (int)ARRAY_SIZE(args_tmp) && \
+if (sizeof(args_tmp)) {  \
+for (i = 0; i < (int)ARRAY_SIZE(args_tmp) && \
  i < ARRAY_SIZE(cap.args); i++) {\
-cap.args[i] = args_tmp[i];   \
+cap.args[i] = args_tmp[i];   \
+}\
 }\
 kvm_vm_ioctl(s, KVM_ENABLE_CAP, );   \
 })
@@ -444,9 +446,11 @@ int kvm_vm_check_extension(KVMState *s, unsigned int 
extension);
 };   \
 uint64_t args_tmp[] = { __VA_ARGS__ };   \
 int i;   \
-for (i = 0; i < (int)ARRAY_SIZE(args_tmp) && \
+if (sizeof(args_tmp)) {  \
+for (i = 0; i < (int)ARRAY_SIZE(args_tmp) && \
  i < ARRAY_SIZE(cap.args); i++) {\
-cap.args[i] = args_tmp[i];   \
+cap.args[i] = args_tmp[i];   \
+}\
 }\
 kvm_vcpu_ioctl(cpu, KVM_ENABLE_CAP, );   \
 })

Re: [Qemu-devel] [PATCH v3 4/7] block: convert ThrottleGroup to object with QOM

[Stefan Hajnoczi]

On Wed, Aug 02, 2017 at 01:57:04PM +0300, Manos Pitsidianakis wrote:
> On Wed, Aug 02, 2017 at 11:39:22AM +0100, Stefan Hajnoczi wrote:
> > On Tue, Aug 01, 2017 at 07:49:33PM +0300, Manos Pitsidianakis wrote:
> > > On Tue, Aug 01, 2017 at 04:47:03PM +0100, Stefan Hajnoczi wrote:
> > > > On Mon, Jul 31, 2017 at 12:54:40PM +0300, Manos Pitsidianakis wrote:
> > > > > ThrottleGroup is converted to an object. This will allow the future
> > > > > throttle block filter drive easy creation and configuration of 
> > > > > throttle
> > > > > groups in QMP and cli.
> > > > >
> > > > > A new QAPI struct, ThrottleLimits, is introduced to provide a shared
> > > > > struct for all throttle configuration needs in QMP.
> > > > >
> > > > > ThrottleGroups can be created via CLI as
> > > > > -object throttle-group,id=foo,x-iops-total=100,x-..
> > > > > where x-* are individual limit properties. Since we can't add 
> > > > > non-scalar
> > > > > properties in -object this interface must be used instead. However,
> > > > > setting these properties must be disabled after initialization because
> > > > > certain combinations of limits are forbidden and thus configuration
> > > > > changes should be done in one transaction. The individual properties
> > > > > will go away when support for non-scalar values in CLI is implemented
> > > > > and thus are marked as experimental.
> > > > >
> > > > > ThrottleGroup also has a `limits` property that uses the 
> > > > > ThrottleLimits
> > > > > struct.  It can be used to create ThrottleGroups or set the
> > > > > configuration in existing groups as follows:
> > > > >
> > > > > { "execute": "object-add",
> > > > >   "arguments": {
> > > > > "qom-type": "throttle-group",
> > > > > "id": "foo",
> > > > > "props" : {
> > > > >   "limits": {
> > > > >   "iops-total": 100
> > > > >   }
> > > > > }
> > > > >   }
> > > > > }
> > > > > { "execute" : "qom-set",
> > > > > "arguments" : {
> > > > > "path" : "foo",
> > > > > "property" : "limits",
> > > > > "value" : {
> > > > > "iops-total" : 99
> > > > > }
> > > > > }
> > > > > }
> > > > >
> > > > > This also means a group's configuration can be fetched with qom-get.
> > > > >
> > > > > ThrottleGroups can be anonymous which means they can't get accessed by
> > > > > other users ie they will always be units instead of group (Because 
> > > > > they
> > > > > have one ThrottleGroupMember).
> > > >
> > > > blockdev.c automatically assigns -drive id= to the group name if
> > > > throttling.group= wasn't given.  So who will use anonymous single-drive
> > > > mode?
> > > 
> > > Manual filter nodes. It's possible to not pass a group name value and the
> > > resulting group will be anonymous. Are you suggesting to move this change 
> > > to
> > > the throttle filter patch?
> > 
> > What is the use case?  Human users will stick to the legacy syntax
> > because it's convenient.  Management tools will use the filter
> > explicitly in the future, and it's easy for them to choose a name.
> > 
> > Unless there is a need for this case I'd prefer to make the group name
> > mandatory.  That way there are less code paths to worry about.
> 
> I think Kevin requested this though I don't really remember the use case.

Kevin: Do you still want this?

> > 
> > > > > @@ -87,32 +99,30 @@ ThrottleState *throttle_group_incref(const char 
> > > > > *name)
> > > > >
> > > > >  qemu_mutex_lock(_groups_lock);
> > > > >
> > > > > -/* Look for an existing group with that name */
> > > > > -QTAILQ_FOREACH(iter, _groups, list) {
> > > > > -if (!strcmp(name, iter->name)) {
> > > > > -tg = iter;
> > > > > -break;
> > > > > +if (name) {
> > > > > +/* Look for an existing group with that name */
> > > > > +QTAILQ_FOREACH(iter, _groups, list) {
> > > > > +if (!g_strcmp0(name, iter->name)) {
> > > > > +tg = iter;
> > > > > +break;
> > > > > +}
> > > > >  }
> > > > >  }
> > > > >
> > > > >  /* Create a new one if not found */
> > > > >  if (!tg) {
> > > > > -tg = g_new0(ThrottleGroup, 1);
> > > > > +/* new ThrottleGroup obj will have a refcnt = 1 */
> > > > > +tg = THROTTLE_GROUP(object_new(TYPE_THROTTLE_GROUP));
> > > > >  tg->name = g_strdup(name);
> > > > > -tg->clock_type = QEMU_CLOCK_REALTIME;
> > > > > -
> > > > > -if (qtest_enabled()) {
> > > > > -/* For testing block IO throttling only */
> > > > > -tg->clock_type = QEMU_CLOCK_VIRTUAL;
> > > > > -}
> > > > > -qemu_mutex_init(>lock);
> > > > > -throttle_init(>ts);
> > > > > -QLIST_INIT(>head);
> > > > > -
> > > > > -QTAILQ_INSERT_TAIL(_groups, tg, list);
> > > > > +throttle_group_obj_complete((UserCreatable *)tg, 
> > > > > _abort);
> > > > >  }
> > > > >
> > > > > -tg->refcount++;
> > 

Re: [Qemu-devel] [PATCH] vl.c/exit: pause cpus before closing block devices

[Alberto Garcia]

On Thu, Jul 13, 2017 at 08:01:16PM +0100, Dr. David Alan Gilbert (git) wrote:
> --- a/vl.c
> +++ b/vl.c
> @@ -4787,8 +4787,8 @@ int main(int argc, char **argv, char **envp)
>  replay_disable_events();
>  iothread_stop_all();
>  
> -bdrv_close_all();
>  pause_all_vcpus();
> +bdrv_close_all();
>  res_free();

I haven't debugged it yet, but in my computer iotest 093 stops working
(it never finishes) after this change.

Berto

Re: [Qemu-devel] [PATCH 3/3] block: remove legacy I/O throttling

[Stefan Hajnoczi]

On Wed, Aug 02, 2017 at 01:34:46PM +0300, Manos Pitsidianakis wrote:
> On Wed, Aug 02, 2017 at 11:07:24AM +0100, Stefan Hajnoczi wrote:
> > On Tue, Aug 01, 2017 at 04:49:07PM +0300, Manos Pitsidianakis wrote:
> > > @@ -3729,6 +3731,12 @@ const char *bdrv_get_parent_name(const 
> > > BlockDriverState *bs)
> > >  return name;
> > >  }
> > >  }
> > > +if (c->parent_bs && c->parent_bs->implicit) {
> > > +name = bdrv_get_parent_name(c->parent_bs);
> > > +if (name && *name) {
> > > +return name;
> > > +}
> > > +}
> > >  }
> > > 
> > >  return NULL;
> > 
> > This should be a separate patch.
> > 
> > Who updates parent_bs if the parent is changed (e.g.
> > bdrv_replace_node())?
> > 
> > We already have bs->parents.  Why is BdrvChild->parent_bs needed?
> > 
> 
> If I haven't misunderstood this, BdrvChild holds only the child part of the
> parent-child relationship and there's no way to access a parent from
> bs->parents. bdrv_replace_node() will thus only replace the child part in
> BdrvChild from the aspect of the parent. In the old child bs's perspective,
> one of the nodes of bs->parents is removed and in the new child bs's
> perspective a new node in bs->parents was inserted. parent_bs thus remains
> immutable.
> 
> child->parent_bs is needed in this patch because in jobs if a job-ID is not
> specified the parent name is used, but this fails if the parent is an
> implicit node instead of BlockBackend and causes a regression (certain job
> setups suddenly need an explicit job ID instead of just working).

Please see Kevin's reply to my email.

> > > -throttle_group_unregister_tgm(>public.throttle_group_member);
> > > -bdrv_drained_end(blk_bs(blk));
> > > +BlockDriverState *bs, *throttle_node;
> > > +
> > > +throttle_node = blk_get_public(blk)->throttle_node;
> > 
> > Is blk_get_public() still necessary?  Perhaps we can do away with the
> > concept of the public struct now.  It doesn't need to be done in this
> > patch though.
> 
> I can include a patch to move throttle_node to BlockBackend and remove all
> BlockBackendPublic code, is that okay?

That would be a nice cleanup, thanks!


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] cpu: don't allow negative core id

[David Gibson]

On Wed, Aug 02, 2017 at 12:32:59PM +0200, Laurent Vivier wrote:
> With pseries machine type a negative core-id is not managed properly:
> -1 gives an inaccurate error message ("core -1 already populated"),
> -2 crashes QEMU (core dump)
> 
> As it seems a negative value is invalid for any architecture,
> instead of checking this in spapr_core_pre_plug() I think it's better
> to check this in the generic part, core_prop_set_core_id()
> 
> Signed-off-by: Laurent Vivier 

Reviewed-by: David Gibson 

> ---
>  hw/cpu/core.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/cpu/core.c b/hw/cpu/core.c
> index 2bf960d..bd578ab 100644
> --- a/hw/cpu/core.c
> +++ b/hw/cpu/core.c
> @@ -33,6 +33,11 @@ static void core_prop_set_core_id(Object *obj, Visitor *v, 
> const char *name,
>  return;
>  }
>  
> +if (value < 0) {
> +error_setg(errp, "Invalid core id %"PRId64, value);
> +return;
> +}
> +
>  core->core_id = value;
>  }
>  

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [for-2.10 PATCH] spapr_drc: fix realize and unrealize

[David Gibson]

On Wed, Aug 02, 2017 at 10:14:56AM +0200, Greg Kurz wrote:
> On Fri, 28 Jul 2017 14:27:45 +1000
> David Gibson  wrote:
> 
> > On Thu, Jul 27, 2017 at 03:50:37PM -0500, Michael Roth wrote:
> > > Quoting Greg Kurz (2017-07-27 08:45:47)  
> > > > If object_property_add_alias() returns an error in realize(), we should
> > > > propagate it to the caller and certainly not unref the DRC.  
> > > 
> > > Indeed. I think that was the result of this part of the code
> > > originally living in spapr_dr_connector_new() during development,
> > > where it had previously made sense to free the object if there was a
> > > failure and then return NULL. We definitely shouldn't be during it
> > > now...  
> > 
> > Yes, I think that's right.
> > 
> > I've applied this fix to ppc-for-2.10.
> > 
> > >   
> > > > 
> > > > Same thing goes for unrealize(). Since object_property_del() is the last
> > > > call, we can even get rid of the intermediate Error *.
> > > > 
> > > > And finally, unrealize() should undo all registrations performed by
> > > > realize().
> > > > 
> > > > Signed-off-by: Greg Kurz 
> > > > ---
> > > > 
> > > > David,
> > > > 
> > > > As agreed on the list, here's the version of the fix for 2.10. I'll 
> > > > respin
> > > > my PHB hotplug series on top of it.
> > > >   
> > > 
> > > 
> > > Since spapr_dr_connector_new() calls realize() with errp = NULL, we
> > > basically lose the error now. I think maybe we should at least report it
> > > still, but even better would be to have _new() call object_property_* with
> > > error_abort, since callers of spapr_dr_connector_new() don't all check
> > > for the return value and even if they did the appropriate action would
> > > always be to report+abort() anyway.  
> > 
> > I agree.  Well, except I think it should be _fatal, not
> > _abort.  But that can be a follow up patch.
> > 
> 
> Hmmm... when we'll hotplug a PHB, an error during the realization of
> a PCI DRC would then terminate QEMU. I'd rather add an errp argument
> to spapr_dr_connector_new() and propagate the error instead.

Ah, good point.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v3 5/5] docs: update documentation considering PCIE-PCI bridge

[Marcel Apfelbaum]

On 02/08/2017 17:16, Laszlo Ersek wrote:

On 08/02/17 15:47, Michael S. Tsirkin wrote:

On Wed, Aug 02, 2017 at 12:23:46AM +0200, Laszlo Ersek wrote:

On 08/01/17 23:39, Michael S. Tsirkin wrote:

On Wed, Aug 02, 2017 at 12:33:12AM +0300, Alexander Bezzubikov wrote:

2017-08-01 23:31 GMT+03:00 Laszlo Ersek :

(Whenever my comments conflict with Michael's or Marcel's, I defer to them.)

On 07/29/17 01:37, Aleksandr Bezzubikov wrote:

Signed-off-by: Aleksandr Bezzubikov 
---
  docs/pcie.txt|  46 ++
  docs/pcie_pci_bridge.txt | 121 +++
  2 files changed, 147 insertions(+), 20 deletions(-)
  create mode 100644 docs/pcie_pci_bridge.txt

diff --git a/docs/pcie.txt b/docs/pcie.txt
index 5bada24..338b50e 100644
--- a/docs/pcie.txt
+++ b/docs/pcie.txt
@@ -46,7 +46,7 @@ Place only the following kinds of devices directly on the 
Root Complex:
  (2) PCI Express Root Ports (ioh3420), for starting exclusively PCI Express
  hierarchies.

-(3) DMI-PCI Bridges (i82801b11-bridge), for starting legacy PCI
+(3) PCIE-PCI Bridge (pcie-pci-bridge), for starting legacy PCI
  hierarchies.

  (4) Extra Root Complexes (pxb-pcie), if multiple PCI Express Root Buses


When reviewing previous patches modifying / adding this file, I
requested that we spell out "PCI Express" every single time. I'd like to
see the same in this patch, if possible.


OK, I didn't know it.




@@ -55,18 +55,18 @@ Place only the following kinds of devices directly on the 
Root Complex:
 pcie.0 bus
 

  |||  |
-   ---   --   --   --
-   | PCI Dev |   | PCIe Root Port |   | DMI-PCI Bridge |   |  pxb-pcie  |
-   ---   --   --   --
+   ---   --   ---   --
+   | PCI Dev |   | PCIe Root Port |   | PCIE-PCI Bridge |   |  pxb-pcie  |
+   ---   --   ---   --

  2.1.1 To plug a device into pcie.0 as a Root Complex Integrated Endpoint use:
-device [,bus=pcie.0]
  2.1.2 To expose a new PCI Express Root Bus use:
-device pxb-pcie,id=pcie.1,bus_nr=x[,numa_node=y][,addr=z]
-  Only PCI Express Root Ports and DMI-PCI bridges can be connected
+  Only PCI Express Root Ports, PCIE-PCI bridges and DMI-PCI bridges can be 
connected


It would be nice if we could keep the flowing text wrapped to 80 chars.

Also, here you add the "PCI Express-PCI" bridge to the list of allowed
controllers (and you keep DMI-PCI as permitted), but above DMI was
replaced. I think these should be made consistent -- we should make up
our minds if we continue to recommend the DMI-PCI bridge or not. If not,
then we should eradicate all traces of it. If we want to keep it at
least for compatibility, then it should remain as fully documented as it
is now.


Now I'm beginning to think that we shouldn't keep the DMI-PCI bridge
even for compatibility and may want to use a new PCIE-PCI bridge
everywhere (of course, except some cases when users are
sure they need exactly DMI-PCI bridge for some reason)


Can dmi-pci support shpc? why doesn't it? For compatibility?


I don't know why, but the fact that it doesn't is the reason libvirt
settled on auto-creating a dmi-pci bridge and a pci-pci bridge under
that for Q35. The reasoning was (IIRC Laine's words correctly) that the
dmi-pci bridge cannot receive hotplugged devices, while the pci-pci
bridge cannot be connected to the root complex. So both were needed.

Thanks
Laszlo


OK. Is it true that dmi-pci + pci-pci under it will allow hotplug
on Q35 if we just flip the bit in _OSC?


Marcel, what say you?... :)



Will test and get back to you (it may actually work)

Thanks,
Marcel

Re: [Qemu-devel] [PATCH v3 5/5] docs: update documentation considering PCIE-PCI bridge

[Laszlo Ersek]

On 08/02/17 15:47, Michael S. Tsirkin wrote:
> On Wed, Aug 02, 2017 at 12:23:46AM +0200, Laszlo Ersek wrote:
>> On 08/01/17 23:39, Michael S. Tsirkin wrote:
>>> On Wed, Aug 02, 2017 at 12:33:12AM +0300, Alexander Bezzubikov wrote:
 2017-08-01 23:31 GMT+03:00 Laszlo Ersek :
> (Whenever my comments conflict with Michael's or Marcel's, I defer to 
> them.)
>
> On 07/29/17 01:37, Aleksandr Bezzubikov wrote:
>> Signed-off-by: Aleksandr Bezzubikov 
>> ---
>>  docs/pcie.txt|  46 ++
>>  docs/pcie_pci_bridge.txt | 121 
>> +++
>>  2 files changed, 147 insertions(+), 20 deletions(-)
>>  create mode 100644 docs/pcie_pci_bridge.txt
>>
>> diff --git a/docs/pcie.txt b/docs/pcie.txt
>> index 5bada24..338b50e 100644
>> --- a/docs/pcie.txt
>> +++ b/docs/pcie.txt
>> @@ -46,7 +46,7 @@ Place only the following kinds of devices directly on 
>> the Root Complex:
>>  (2) PCI Express Root Ports (ioh3420), for starting exclusively PCI 
>> Express
>>  hierarchies.
>>
>> -(3) DMI-PCI Bridges (i82801b11-bridge), for starting legacy PCI
>> +(3) PCIE-PCI Bridge (pcie-pci-bridge), for starting legacy PCI
>>  hierarchies.
>>
>>  (4) Extra Root Complexes (pxb-pcie), if multiple PCI Express Root 
>> Buses
>
> When reviewing previous patches modifying / adding this file, I
> requested that we spell out "PCI Express" every single time. I'd like to
> see the same in this patch, if possible.

 OK, I didn't know it.

>
>> @@ -55,18 +55,18 @@ Place only the following kinds of devices directly 
>> on the Root Complex:
>> pcie.0 bus
>> 
>> 
>>  |||  |
>> -   ---   --   --   
>> --
>> -   | PCI Dev |   | PCIe Root Port |   | DMI-PCI Bridge |   |  pxb-pcie  
>> |
>> -   ---   --   --   
>> --
>> +   ---   --   ---   
>> --
>> +   | PCI Dev |   | PCIe Root Port |   | PCIE-PCI Bridge |   |  pxb-pcie 
>>  |
>> +   ---   --   ---   
>> --
>>
>>  2.1.1 To plug a device into pcie.0 as a Root Complex Integrated 
>> Endpoint use:
>>-device [,bus=pcie.0]
>>  2.1.2 To expose a new PCI Express Root Bus use:
>>-device pxb-pcie,id=pcie.1,bus_nr=x[,numa_node=y][,addr=z]
>> -  Only PCI Express Root Ports and DMI-PCI bridges can be connected
>> +  Only PCI Express Root Ports, PCIE-PCI bridges and DMI-PCI bridges 
>> can be connected
>
> It would be nice if we could keep the flowing text wrapped to 80 chars.
>
> Also, here you add the "PCI Express-PCI" bridge to the list of allowed
> controllers (and you keep DMI-PCI as permitted), but above DMI was
> replaced. I think these should be made consistent -- we should make up
> our minds if we continue to recommend the DMI-PCI bridge or not. If not,
> then we should eradicate all traces of it. If we want to keep it at
> least for compatibility, then it should remain as fully documented as it
> is now.

 Now I'm beginning to think that we shouldn't keep the DMI-PCI bridge
 even for compatibility and may want to use a new PCIE-PCI bridge
 everywhere (of course, except some cases when users are
 sure they need exactly DMI-PCI bridge for some reason)
>>>
>>> Can dmi-pci support shpc? why doesn't it? For compatibility?
>>
>> I don't know why, but the fact that it doesn't is the reason libvirt
>> settled on auto-creating a dmi-pci bridge and a pci-pci bridge under
>> that for Q35. The reasoning was (IIRC Laine's words correctly) that the
>> dmi-pci bridge cannot receive hotplugged devices, while the pci-pci
>> bridge cannot be connected to the root complex. So both were needed.
>>
>> Thanks
>> Laszlo
> 
> OK. Is it true that dmi-pci + pci-pci under it will allow hotplug
> on Q35 if we just flip the bit in _OSC?

Marcel, what say you?... :)

Re: [Qemu-devel] [PATCH for-2.10 5/5] tests: acpi: fix FADT not being compared to reference table

[Igor Mammedov]

On Wed, 2 Aug 2017 16:15:10 +0300
Marcel Apfelbaum  wrote:

> On 31/07/2017 18:40, Igor Mammedov wrote:
> > It turns out that FADT isn't actually tested for changes
> > against reference table, since it happens to be the 1st
> > table in RSDT which is currently ignored.
> > Fix it by making sure that all tables from RSDT are added
> > to test list.
> >   
> 
> Hi Igor,
> 
> > Signed-off-by: Igor Mammedov 
> > ---
> >   tests/bios-tables-test.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> > index a2a90d7..129ef46 100644
> > --- a/tests/bios-tables-test.c
> > +++ b/tests/bios-tables-test.c
> > @@ -243,13 +243,13 @@ static void test_acpi_dsdt_table(test_data *data)
> >   /* Load all tables and add to test list directly RSDT referenced tables */
> >   static void fetch_rsdt_referenced_tables(test_data *data)
> >   {
> > -int tables_nr = data->rsdt_tables_nr - 1; /* fadt is first */
> > +int tables_nr = data->rsdt_tables_nr;
> >   int i;
> >   
> >   for (i = 0; i < tables_nr; i++) {
> >   AcpiSdtTable ssdt_table;
> >   
> > -uint32_t addr = data->rsdt_tables_addr[i + 1]; /* fadt is first */
> > +uint32_t addr = data->rsdt_tables_addr[i];
> >   fetch_table(_table, addr);
> >   
> >   /* Add table to ASL test tables list */
> >   
> 
> For some reason I decided not to test it... strange.
> Anyway, we should also add the expected file, right?
> (the rebuild script works only for existent tables, I think)
they are there see: tests/acpi-test-data/*/FACP
files obviously stale as they haven't been actually used.
Michael should update them when he applies this series.

> 
> Thanks,
> Marcel
> 
> 
> 

Re: [Qemu-devel] [PATCH] slirp: check len against dhcp options array end

[Michael Tokarev]

17.07.2017 17:48, Samuel Thibault wrote:
> P J P, on lun. 17 juil. 2017 17:33:26 +0530, wrote:
>> From: Prasad J Pandit 
>>
>> While parsing dhcp options string in 'dhcp_decode', if an options'
>> length 'len' appeared towards the end of 'bp_vend' array, ensuing
>> read could lead to an OOB memory access issue. Add check to avoid it.
>>
>> Reported-by: Reno Robert 
>> Signed-off-by: Prasad J Pandit 
> 
> Oops, sure, applied to my tree, thanks!

Can we have it in 2.10 please?

This is CVE-2017-11434, which can be mentioned in the commit message.

Thanks,

/mjt

[Qemu-devel] [PULL 1/3] migration: fix small leaks

[Dr. David Alan Gilbert (git)]

From: Marc-André Lureau 

Spotted thanks to valgrind and tests/device-introspect-test:

==11711== 1 bytes in 1 blocks are definitely lost in loss record 6 of 14,537
==11711==at 0x4C2EB6B: malloc (vg_replace_malloc.c:299)
==11711==by 0x1E0CDBD8: g_malloc (gmem.c:94)
==11711==by 0x1E0E696E: g_strdup (gstrfuncs.c:363)
==11711==by 0x695693: migration_instance_init (migration.c:2226)
==11711==by 0x717C4B: object_init_with_type (object.c:344)
==11711==by 0x717E80: object_initialize_with_type (object.c:375)
==11711==by 0x7182EB: object_new_with_type (object.c:483)
==11711==by 0x718328: object_new (object.c:493)
==11711==by 0x4B8A29: qmp_device_list_properties (qmp.c:542)
==11711==by 0x4A9561: qmp_marshal_device_list_properties 
(qmp-marshal.c:1425)
==11711==by 0x819D4A: do_qmp_dispatch (qmp-dispatch.c:104)
==11711==by 0x819E82: qmp_dispatch (qmp-dispatch.c:131)

Signed-off-by: Marc-André Lureau 
Message-Id: <20170801160419.14180-1-marcandre.lur...@redhat.com>
Reviewed-by: Peter Xu 
Reviewed-by: Juan Quintela 
Signed-off-by: Dr. David Alan Gilbert 
---
 migration/migration.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/migration/migration.c b/migration/migration.c
index 085c32c994..c3fe0ed9ca 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2214,6 +2214,15 @@ static void migration_class_init(ObjectClass *klass, 
void *data)
 dc->props = migration_properties;
 }
 
+static void migration_instance_finalize(Object *obj)
+{
+MigrationState *ms = MIGRATION_OBJ(obj);
+MigrationParameters *params = >parameters;
+
+g_free(params->tls_hostname);
+g_free(params->tls_creds);
+}
+
 static void migration_instance_init(Object *obj)
 {
 MigrationState *ms = MIGRATION_OBJ(obj);
@@ -2282,6 +2291,7 @@ static const TypeInfo migration_type = {
 .class_size = sizeof(MigrationClass),
 .instance_size = sizeof(MigrationState),
 .instance_init = migration_instance_init,
+.instance_finalize = migration_instance_finalize,
 };
 
 static void register_migration_types(void)
-- 
2.13.3

[Qemu-devel] [PULL 3/3] io: fix qio_channel_socket_accept err handling

[Dr. David Alan Gilbert (git)]

From: Peter Xu 

When accept failed, we should setup errp with the reason. More
importantly, the caller may assume errp be non-NULL when error happens,
and not setting the errp may crash QEMU.

At the same time, move the trace_qio_channel_socket_accept_fail() after
the if check on EINTR. Two reasons:

1. when EINTR happened, it's not really a fault (we should just try
   again), so we should not log with an "accept failure".

2. trace_*() functions may overwrite errno, then the old errno will be
   missing. We need to either check errno before trace_*() calls, or
   reserve the errno.

Signed-off-by: Peter Xu 
Message-Id: <1501666880-10159-3-git-send-email-pet...@redhat.com>
Reviewed-by: Daniel P. Berrange 
Signed-off-by: Dr. David Alan Gilbert 
---
 io/channel-socket.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/io/channel-socket.c b/io/channel-socket.c
index 53386b7ba3..591d27e8c3 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -340,10 +340,11 @@ qio_channel_socket_accept(QIOChannelSocket *ioc,
 cioc->fd = qemu_accept(ioc->fd, (struct sockaddr *)>remoteAddr,
>remoteAddrLen);
 if (cioc->fd < 0) {
-trace_qio_channel_socket_accept_fail(ioc);
 if (errno == EINTR) {
 goto retry;
 }
+error_setg_errno(errp, errno, "Unable to accept connection");
+trace_qio_channel_socket_accept_fail(ioc);
 goto error;
 }
 
-- 
2.13.3

[Qemu-devel] [PULL 2/3] migration: fix comment disorder in RAMState

[Dr. David Alan Gilbert (git)]

From: Peter Xu 

Comments for "migration_dirty_pages" and "bitmap_mutex" are switched.
Fix it.

Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Juan Quintela 
Signed-off-by: Peter Xu 
Message-Id: <1501666880-10159-2-git-send-email-pet...@redhat.com>
Signed-off-by: Dr. David Alan Gilbert 
---
 migration/ram.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 1b08296d1b..e18b3e2d4f 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -188,9 +188,9 @@ struct RAMState {
 uint64_t iterations_prev;
 /* Iterations since start */
 uint64_t iterations;
-/* protects modification of the bitmap */
-uint64_t migration_dirty_pages;
 /* number of dirty bits in the bitmap */
+uint64_t migration_dirty_pages;
+/* protects modification of the bitmap */
 QemuMutex bitmap_mutex;
 /* The RAMBlock used in the last src_page_requests */
 RAMBlock *last_req_rb;
-- 
2.13.3

[Qemu-devel] [PULL 0/3] migration queue

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

The following changes since commit d3d183a638d6a3ead515618a6547b3f80d39fcb9:

  Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging 
(2017-08-02 09:49:02 +0100)

are available in the git repository at:

  git://github.com/dagrh/qemu.git tags/pull-migration-20170802a

for you to fetch changes up to 8bd9c4e6c565c566a6cba3470cb2d4ea63994143:

  io: fix qio_channel_socket_accept err handling (2017-08-02 11:27:44 +0100)


Migration pull 2017-08-02

Just minor fixes for 2.10


Marc-André Lureau (1):
  migration: fix small leaks

Peter Xu (2):
  migration: fix comment disorder in RAMState
  io: fix qio_channel_socket_accept err handling

 io/channel-socket.c   |  3 ++-
 migration/migration.c | 10 ++
 migration/ram.c   |  4 ++--
 3 files changed, 14 insertions(+), 3 deletions(-)