Re: [PATCH v2] Implement configurable descriptor size in ftgmac100

2020-06-07 Thread Cédric Le Goater
On 6/6/20 11:03 AM, Erik Smit wrote:
> The hardware supports configurable descriptor sizes, configured in the DBLAC
> register.
> 
> Most drivers use the default 4 word descriptor, which is currently hardcoded,
> but Aspeed SDK configures 8 words to store extra data.
> 
> ---
> The implementation of the driver in Supermicro BMC SMT_X11_158 adds 4 extra
> word entries:
> https://github.com/ya-mouse/openwrt-linux-aspeed/blob/master/drivers/net/ftgmac100_26.h#L387-L391
> 
> And sets DBLAC to 0x44f97:
> https://github.com/ya-mouse/openwrt-linux-aspeed/blob/master/drivers/net/ftgmac100_26.c#L449
> 
> There's not a lot of public documentation on this hardware, but the
> current linux driver shows the meaning of these registers:
> 
> https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/faraday/ftgmac100.c#L280-L281
> 
> iowrite32(FTGMAC100_DBLAC_RXDES_SIZE(2) |   /* 2*8 bytes RX descs */
>   FTGMAC100_DBLAC_TXDES_SIZE(2) |   /* 2*8 bytes TX descs */
> 
> Without this patch, networking in SMT_X11_158 fails after the first packet.
> 
> changes since previous version:
> 
> - moved "* 8" into {R,T}XDES_SIZE macro
> - removed the RXFIFO and RX_THR_EN defines as they're AST2400-only and not 
> used
> - test setting of DBLAC register for validness
> 
> Signed-off-by: Erik Smit 

We don't need the parenthesis around FTGMAC100_DBLAC_RXDES_SIZE(). Anyhow,

Reviewed-by: Cédric Le Goater 

Thanks,

C. 

> ---
>  hw/net/ftgmac100.c | 26 --
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
> index 25ebee7ec2..83058497c4 100644
> --- a/hw/net/ftgmac100.c
> +++ b/hw/net/ftgmac100.c
> @@ -79,6 +79,16 @@
>  #define FTGMAC100_APTC_TXPOLL_CNT(x)(((x) >> 8) & 0xf)
>  #define FTGMAC100_APTC_TXPOLL_TIME_SEL  (1 << 12)
> 
> +/*
> + * DMA burst length and arbitration control register
> + */
> +#define FTGMAC100_DBLAC_RXBURST_SIZE(x) (((x) >> 8) & 0x3)
> +#define FTGMAC100_DBLAC_TXBURST_SIZE(x) (((x) >> 10) & 0x3)
> +#define FTGMAC100_DBLAC_RXDES_SIZE(x)   x) >> 12) & 0xf) * 8)
> +#define FTGMAC100_DBLAC_TXDES_SIZE(x)   x) >> 16) & 0xf) * 8)
> +#define FTGMAC100_DBLAC_IFG_CNT(x)  (((x) >> 20) & 0x7)
> +#define FTGMAC100_DBLAC_IFG_INC (1 << 23)
> +
>  /*
>   * PHY control register
>   */
> @@ -553,7 +563,7 @@ static void ftgmac100_do_tx(FTGMAC100State *s,
> uint32_t tx_ring,
>  if (bd.des0 & s->txdes0_edotr) {
>  addr = tx_ring;
>  } else {
> -addr += sizeof(FTGMAC100Desc);
> +addr += (FTGMAC100_DBLAC_TXDES_SIZE(s->dblac));
>  }
>  }
> 
> @@ -800,6 +810,18 @@ static void ftgmac100_write(void *opaque, hwaddr addr,
>  s->phydata = value & 0x;
>  break;
>  case FTGMAC100_DBLAC: /* DMA Burst Length and Arbitration Control */
> +if (FTGMAC100_DBLAC_TXDES_SIZE(s->dblac) < sizeof(FTGMAC100Desc)) {
> +qemu_log_mask(LOG_GUEST_ERROR,
> +  "%s: transmit descriptor too small : %d bytes\n",
> +  __func__, FTGMAC100_DBLAC_TXDES_SIZE(s->dblac));
> +break;
> +}
> +if (FTGMAC100_DBLAC_RXDES_SIZE(s->dblac) < sizeof(FTGMAC100Desc)) {
> +qemu_log_mask(LOG_GUEST_ERROR,
> +  "%s: receive descriptor too small : %d bytes\n",
> +  __func__, FTGMAC100_DBLAC_RXDES_SIZE(s->dblac));
> +break;
> +}
>  s->dblac = value;
>  break;
>  case FTGMAC100_REVR:  /* Feature Register */
> @@ -982,7 +1004,7 @@ static ssize_t ftgmac100_receive(NetClientState
> *nc, const uint8_t *buf,
>  if (bd.des0 & s->rxdes0_edorr) {
>  addr = s->rx_ring;
>  } else {
> -addr += sizeof(FTGMAC100Desc);
> +addr += (FTGMAC100_DBLAC_RXDES_SIZE(s->dblac));
>  }
>  }
>  s->rx_descriptor = addr;
> --
> 2.25.1
> 




[Bug 1874073] Re: openrisc_sim.c:87:42: error: 'cpu_irqs[0]' may be used uninitialized in this function [-Werror=maybe-uninitialized]

2020-06-07 Thread Philippe Mathieu-Daudé
** Changed in: qemu
   Status: New => Confirmed

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

Title:
  openrisc_sim.c:87:42: error: 'cpu_irqs[0]' may be used uninitialized
  in this function [-Werror=maybe-uninitialized]

Status in QEMU:
  Confirmed

Bug description:
  I see the warning since gcc10:

  static void openrisc_sim_init(MachineState *machine):
  ...
  qemu_irq *cpu_irqs[2];
  ...

  
  serial_mm_init(get_system_memory(), 0x9000, 0, serial_irq,
 115200, serial_hd(0), DEVICE_NATIVE_ENDIAN);

  I would initialize cpu_irqs[2] with {}.

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



Re: [PATCH] or1k: Fix compilation hiccup

2020-06-07 Thread Philippe Mathieu-Daudé
On 6/8/20 8:03 AM, Markus Armbruster wrote:
> Markus Armbruster  writes:
> 
>> Peter Maydell  writes:
>>
>>> On Fri, 29 May 2020 at 17:23, Christophe de Dinechin
>>>  wrote:
 On 2020-05-26 at 20:51 CEST, Eric Blake wrote...
> diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
> index d08ce6181199..95011a8015b4 100644
> --- a/hw/openrisc/openrisc_sim.c
> +++ b/hw/openrisc/openrisc_sim.c
> @@ -129,7 +129,7 @@ static void openrisc_sim_init(MachineState *machine)
>  const char *kernel_filename = machine->kernel_filename;
>  OpenRISCCPU *cpu = NULL;
>  MemoryRegion *ram;
> -qemu_irq *cpu_irqs[2];
> +qemu_irq *cpu_irqs[2] = {};

 Why is the value [2] correct here? The loop that initializes loops over
 machine->smp.cpus. Is it always less than 2 on this machine?
>>>
>>> Yes: openrisc_sim_machine_init() sets mc->max_cpus = 2.
>>> My suggestion of adding an assert() is essentially telling the
>>> compiler that indeed smp_cpus must always be in the range [1,2],
>>> which we can tell but it can't.
>>
>> Do we have a proper patch for this on the list?
> 
> Apparently not.
> 
> Philippe did try Peter's suggestion, found it works, but then posted it
> only to Launchpad.  Philippe, please post to the list, so we can finally
> get this fixed.

Sorry since it was Eric finding, I didn't understood I had to post it.
Will do.




Re: [PATCH v1 7/9] plugins: add API to return a name for a IO device

2020-06-07 Thread Philippe Mathieu-Daudé
On 6/8/20 5:45 AM, Emilio G. Cota wrote:
> On Tue, Jun 02, 2020 at 16:46:22 +0100, Alex Bennée wrote:
>> This may well end up being anonymous but it should always be unique.
>>
>> Signed-off-by: Alex Bennée 
>> ---
>>  include/qemu/qemu-plugin.h |  5 +
>>  plugins/api.c  | 18 ++
>>  2 files changed, 23 insertions(+)
>>
>> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
>> index bab8b0d4b3a..43c6a9e857f 100644
>> --- a/include/qemu/qemu-plugin.h
>> +++ b/include/qemu/qemu-plugin.h
>> @@ -335,6 +335,11 @@ struct qemu_plugin_hwaddr 
>> *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
>>  bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr);
>>  uint64_t qemu_plugin_hwaddr_device_offset(const struct qemu_plugin_hwaddr 
>> *haddr);
>>  
>> +/*
>> + * Returns a string representing the device. Plugin must free() it
>> + */
>> +char * qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr 
>> *haddr);
>> +
>>  typedef void
>>  (*qemu_plugin_vcpu_mem_cb_t)(unsigned int vcpu_index,
>>   qemu_plugin_meminfo_t info, uint64_t vaddr,
>> diff --git a/plugins/api.c b/plugins/api.c
>> index bbdc5a4eb46..3c73de8c1c2 100644
>> --- a/plugins/api.c
>> +++ b/plugins/api.c
>> @@ -303,6 +303,24 @@ uint64_t qemu_plugin_hwaddr_device_offset(const struct 
>> qemu_plugin_hwaddr *haddr
>>  return 0;
>>  }
>>  
>> +char * qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr 
>> *haddr)
>> +{
>> +#ifdef CONFIG_SOFTMMU
>> +if (haddr && haddr->is_io) {
>> +MemoryRegionSection *mrs = haddr->v.io.section;
>> +if (!mrs->mr->name) {
>> +return g_strdup_printf("anon%08lx", 0x & (uintptr_t) 
>> mrs->mr);
>> +} else {
>> +return g_strdup(mrs->mr->name);
>> +}
>> +} else {
>> +return g_strdup("RAM");
>> +}
>> +#else
>> +return g_strdup("Invalid");
>> +#endif
>> +}
> 
> I'd rather use asprintf(3) and strdup(3) here, so that plugins don't
> have to worry about glib, and on the QEMU side we don't have to worry
> about plugins calling free() instead of g_free().

It might make sense, but it should be documented in
include/qemu/qemu-plugin.h or docs/devel/tcg-plugins.rst.

> 
> Or given that this doesn't look perf-critical, perhaps an easier way out
> is to wrap the above with:
> 
> char *g_str = above();
> char *ret = strdup(g_str);
> g_free(g_str);

free() ;)

> return ret;
> 
> Not sure we should NULL-check ret, since I don't know whether
> mrs->mr->name is guaranteed to be non-NULL.
> 
> Thanks,
>   Emilio
> 




Re: [PATCH] block: Remove trailing newline in format used by error_report API

2020-06-07 Thread Philippe Mathieu-Daudé
On 6/8/20 6:45 AM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé  writes:
> 
>> On 2/28/20 6:32 PM, Markus Armbruster wrote:
[...]
>>> warn_reportf_err() is a convenience function to error_prepend(),
>>> warn_report() and free @local_err.
> [...]
>> Why warn_reportf_err() doesn't take a 'Error **err' instead, to set err
>> to NULL after freeing *err?
> 
> Why doesn't free() take a void ** argument, to set the pointer to null
> after freeing what it points to?  Why doesn't close() take an int *
> argument?

=)

Usually I see the code checking an Error* hasn't been set by a callee.
If it has, the caller usually returns.

You explained me warn_reportf_err() consume Error* and free() it.

So regarding the rest of our Error* use, a function calling
warn_reportf_err has to do extra care to set Error* to NULL.

Genuinely looks confuse or dangerous to me...

Note however I was not asking for a change, just asking 'why'
to better understand if there were not a design problem, or
o invalid use of different APIs.

> 
> [...]
> 




Re: [PATCH 10/16] qdev: Improve netdev property override error a bit

2020-06-07 Thread Philippe Mathieu-Daudé
On 6/5/20 4:56 PM, Markus Armbruster wrote:
> qdev_prop_set_netdev() fails when the property already has a non-null
> value.  Seems to go back to commit 30c367ed44
> "qdev-properties-system.c: Allow vlan or netdev for -device, not
> both", v1.7.0.  Board code doesn't expect failure, and crashes:
> 
> $ qemu-system-x86_64 --nodefaults -nic user -netdev user,id=nic0 -global 
> e1000.netdev=nic0
> Unexpected error in error_set_from_qdev_prop_error() at 
> /work/armbru/qemu/hw/core/qdev-properties.c:1101:
> qemu-system-x86_64: Property 'e1000.netdev' doesn't take value 
> '__org.qemu.nic0
> '
> Aborted (core dumped)
> 
> -device and device_add handle the failure:
> 
> $ qemu-system-x86_64 -nodefaults -netdev user,id=net0 -netdev 
> user,id=net1 -device e1000,netdev=net0,netdev=net1
> qemu-system-x86_64: -device e1000,netdev=net0,netdev=net1: Property 
> 'e1000.netdev' doesn't take value 'net1'
> $ qemu-system-x86_64 -nodefaults -S -display none -monitor stdio -netdev 
> user,id=net0 -netdev user,id=net1 -global e1000.netdev=net0
> QEMU 5.0.50 monitor - type 'help' for more information
> (qemu) qemu-system-x86_64: warning: netdev net0 has no peer
> qemu-system-x86_64: warning: netdev net1 has no peer
> device_add e1000,netdev=net1
> Error: Property 'e1000.netdev' doesn't take value 'net1'

If patch accepted as it, might be worth Cc'ing qemu-stable@.

> 
> Perhaps netdev property override could be made to work.  Perhaps it
> should.  I'm not the right guy to figure this out.  What I can do is
> improve the error message a bit:
> 
> (qemu) device_add e1000,netdev=net1
> Error: -global e1000.netdev=... conflicts with netdev=net1
> 
> Cc: Jason Wang 
> Signed-off-by: Markus Armbruster 
> ---
>  include/hw/qdev-properties.h |  2 ++
>  hw/core/qdev-properties-system.c | 30 +++---
>  hw/core/qdev-properties.c| 17 +
>  3 files changed, 46 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index f161604fb6..566419f379 100644
> --- a/include/hw/qdev-properties.h
> +++ b/include/hw/qdev-properties.h
> @@ -248,6 +248,8 @@ void qdev_prop_set_macaddr(DeviceState *dev, const char 
> *name,
>  void qdev_prop_set_enum(DeviceState *dev, const char *name, int value);
>  
>  void qdev_prop_register_global(GlobalProperty *prop);
> +const GlobalProperty *qdev_find_global_prop(DeviceState *dev,
> +const char *name);
>  int qdev_prop_check_globals(void);
>  void qdev_prop_set_globals(DeviceState *dev);
>  void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
> diff --git a/hw/core/qdev-properties-system.c 
> b/hw/core/qdev-properties-system.c
> index 9aa80495ee..20fd58e8f9 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -25,6 +25,28 @@
>  #include "sysemu/iothread.h"
>  #include "sysemu/tpm_backend.h"
>  
> +static bool check_non_null(DeviceState *dev, const char *name,

I see this is a static qdev function, but can we have a name that
better match the content? Maybe qdev_global_prop_exists()?

> +   const void *old_val, const char *new_val,
> +   Error **errp)
> +{
> +const GlobalProperty *prop = qdev_find_global_prop(dev, name);
> +
> +if (!old_val) {
> +return true;
> +}
> +
> +if (prop) {
> +error_setg(errp, "-global %s.%s=... conflicts with %s=%s",
> +   prop->driver, prop->property, name, new_val);
> +} else {
> +/* Error message is vague, but a better one would be hard */
> +error_setg(errp, "%s=%s conflicts, and override is not implemented",
> +   name, new_val);
> +}
> +return false;
> +}
> +
> +
>  /* --- drive --- */
>  
>  static void get_drive(Object *obj, Visitor *v, const char *name, void 
> *opaque,
> @@ -299,14 +321,16 @@ static void set_netdev(Object *obj, Visitor *v, const 
> char *name,
>  }
>  
>  for (i = 0; i < queues; i++) {
> -
>  if (peers[i]->peer) {
>  err = -EEXIST;
>  goto out;
>  }
>  
> -if (ncs[i]) {
> -err = -EINVAL;
> +/*
> + * TODO Should this really be an error?  If no, the old value
> + * needs to be released before we store the new one.
> + */
> +if (!check_non_null(dev, name, ncs[i], str, errp)) {
>  goto out;
>  }
>  
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index cc924815da..8c8beb56b2 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -1181,6 +1181,23 @@ void qdev_prop_register_global(GlobalProperty *prop)
>  g_ptr_array_add(global_props(), prop);
>  }
>  
> +const GlobalProperty *qdev_find_global_prop(DeviceState *dev,
> +const char *name)

Do y

Re: [PATCH] or1k: Fix compilation hiccup

2020-06-07 Thread Markus Armbruster
Markus Armbruster  writes:

> Peter Maydell  writes:
>
>> On Fri, 29 May 2020 at 17:23, Christophe de Dinechin
>>  wrote:
>>> On 2020-05-26 at 20:51 CEST, Eric Blake wrote...
>>> > diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
>>> > index d08ce6181199..95011a8015b4 100644
>>> > --- a/hw/openrisc/openrisc_sim.c
>>> > +++ b/hw/openrisc/openrisc_sim.c
>>> > @@ -129,7 +129,7 @@ static void openrisc_sim_init(MachineState *machine)
>>> >  const char *kernel_filename = machine->kernel_filename;
>>> >  OpenRISCCPU *cpu = NULL;
>>> >  MemoryRegion *ram;
>>> > -qemu_irq *cpu_irqs[2];
>>> > +qemu_irq *cpu_irqs[2] = {};
>>>
>>> Why is the value [2] correct here? The loop that initializes loops over
>>> machine->smp.cpus. Is it always less than 2 on this machine?
>>
>> Yes: openrisc_sim_machine_init() sets mc->max_cpus = 2.
>> My suggestion of adding an assert() is essentially telling the
>> compiler that indeed smp_cpus must always be in the range [1,2],
>> which we can tell but it can't.
>
> Do we have a proper patch for this on the list?

Apparently not.

Philippe did try Peter's suggestion, found it works, but then posted it
only to Launchpad.  Philippe, please post to the list, so we can finally
get this fixed.




Re: [PATCH] util/oslib-posix : qemu_init_exec_dir implementation for MacOS

2020-06-07 Thread Roman Bolshakov
On Tue, May 26, 2020 at 09:40:27PM +0100, David CARLIER wrote:
> From b24a6702beb2a4e2a9c1c03b69c6d1dd07d4cf08 Mon Sep 17 00:00:00 2001
> From: David Carlier 
> Date: Tue, 26 May 2020 21:35:27 +0100
> Subject: [PATCH] util/oslib: current process full path resolution on MacOS
> 
> Using existing libproc to fill the path.
> 
> Signed-off-by: David Carlier 
> ---
>  util/oslib-posix.c | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 062236a1ab..96f0405ee6 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -55,6 +55,10 @@
>  #include 
>  #endif
> 
> +#ifdef __APPLE__
> +#include 
> +#endif
> +
>  #include "qemu/mmap-alloc.h"
> 
>  #ifdef CONFIG_DEBUG_STACK_USAGE
> @@ -366,6 +370,15 @@ void qemu_init_exec_dir(const char *argv0)
>  p = buf;
>  }
>  }
> +#elif defined(__APPLE__)
> +{
> +uint32_t len;
> +len = proc_pidpath(getpid(), buf, sizeof(buf) - 1);

Hi David,

proc_pidpath handler in the xnu kernel is indirectly calling
build_path_with_parent [1] and it includes NULL terminator into
buffersize, i.e. the patch seems to limit path length to one less
character than OS allows.

> +if (len > 0) {
> +buf[len] = 0;
> +p = buf;
> +}
> +}
>  #endif
>  /* If we don't have any way of figuring out the actual executable
> location then try argv[0].  */
> -- 
> 2.26.2
> 


The change looks okay but it's not clear why it is needed [2].

Also, libproc.h [3] has this comment:

/*
 * This header file contains private interfaces to obtain process information.  
 * These interfaces are subject to change in future releases.
 */

But iTerm2 [4] an Chromium [5] are using it. An alternative (if it works
and IMO less appealing) would be to retrieve process path using AppKit [6].

1. 
https://opensource.apple.com/source/xnu/xnu-6153.81.5/bsd/vfs/vfs_cache.c.auto.html
2. 
https://wiki.qemu.org/Contribute/SubmitAPatch#Write_a_meaningful_commit_message
3. 
https://opensource.apple.com/source/xnu/xnu-6153.81.5/libsyscall/wrappers/libproc/libproc.h.auto.html
4. 
https://gitlab.com/gnachman/iterm2/blob/872e3d63fbcaf7b4235c4f3b7273e09a7ba4c1d1/sources/iTermLSOF.m#L31
5. 
https://github.com/chromium/chromium/blob/2ca8c5037021c9d2ecc00b787d58a31ed8fc8bcb/base/process/process_handle_mac.cc#L31
6. https://developer.apple.com/documentation/appkit/nsworkspace?language=objc

Regards,
Roman



Re: [PATCH 14/16] arm/aspeed: Drop aspeed_board_init_flashes() parameter @errp

2020-06-07 Thread Philippe Mathieu-Daudé
On 6/5/20 4:56 PM, Markus Armbruster wrote:
> We always pass &error_abort.  Drop the parameter, use &error_abort
> directly.
> 
> Cc: Cédric Le Goater 
> Cc: Peter Maydell 
> Cc: Andrew Jeffery 
> Cc: Joel Stanley 
> Cc: qemu-...@nongnu.org
> Signed-off-by: Markus Armbruster 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  hw/arm/aspeed.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 5ffaf86b86..4ce6ca0ef5 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -215,8 +215,8 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, 
> size_t rom_size,
>  g_free(storage);
>  }
>  
> -static void aspeed_board_init_flashes(AspeedSMCState *s, const char 
> *flashtype,
> -  Error **errp)
> +static void aspeed_board_init_flashes(AspeedSMCState *s,
> +  const char *flashtype)
>  {
>  int i ;
>  
> @@ -227,8 +227,8 @@ static void aspeed_board_init_flashes(AspeedSMCState *s, 
> const char *flashtype,
>  
>  fl->flash = qdev_new(flashtype);
>  if (dinfo) {
> -qdev_prop_set_drive_err(fl->flash, "drive",
> -blk_by_legacy_dinfo(dinfo), errp);
> +qdev_prop_set_drive(fl->flash, "drive",
> +blk_by_legacy_dinfo(dinfo));
>  }
>  qdev_realize_and_unref(fl->flash, BUS(s->spi), &error_fatal);
>  
> @@ -314,8 +314,8 @@ static void aspeed_machine_init(MachineState *machine)
>"max_ram", max_ram_size  - ram_size);
>  memory_region_add_subregion(&bmc->ram_container, ram_size, 
> &bmc->max_ram);
>  
> -aspeed_board_init_flashes(&bmc->soc.fmc, amc->fmc_model, &error_abort);
> -aspeed_board_init_flashes(&bmc->soc.spi[0], amc->spi_model, 
> &error_abort);
> +aspeed_board_init_flashes(&bmc->soc.fmc, amc->fmc_model);
> +aspeed_board_init_flashes(&bmc->soc.spi[0], amc->spi_model);
>  
>  /* Install first FMC flash content as a boot rom. */
>  if (drive0) {
> 




Re: [PATCH 13/16] qdev: Make qdev_prop_set_drive() match the other helpers

2020-06-07 Thread Philippe Mathieu-Daudé
On 6/8/20 7:20 AM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé  writes:
> 
>> On 6/5/20 4:56 PM, Markus Armbruster wrote:
>>> qdev_prop_set_drive() can fail.  None of the other qdev_prop_set_FOO()
>>> can; they abort on error.
>>>
>>> To clean up this inconsistency, rename qdev_prop_set_drive() to
>>> qdev_prop_set_drive_err(), and create a qdev_prop_set_drive() that
>>> aborts on error.
>>>
>>> Coccinelle script to update callers:
>>>
>>> @ depends on !(file in "hw/core/qdev-properties-system.c")@
>>> expression dev, name, value;
>>> symbol error_abort;
>>> @@
>>> -qdev_prop_set_drive(dev, name, value, &error_abort);
>>> +qdev_prop_set_drive(dev, name, value);
>>
>> Why not open-code qdev_prop_set_drive_err(..., &error_abort)?
> 
> Consistency with qdev_prop_set_chr() and qdev_prop_set_netdev().
> 
> My starting point was "what makes block backends so different that they
> need error handling where nothing else does?"
> 
> After a considerable amount of digging, my answer is "nothing".
> qdev_prop_set_drive(), qdev_prop_set_chr() and qdev_prop_set_netdev()
> can all run into errors.  On closer examination, all programming errors
> (thus &error_abort), except for "backend is already in use", and to
> trigger that one, you have to get creative and steal the backend for
> another purpose, e.g. with -global.  This is the abridged version of a
> longwinded argument I didn't want to make in this series, so I left the
> error handling alone.
> 
> In the longer run, I want qdev_prop_set_drive_err() to die.

I agree with the longer run. I naively thought this could be done
in the same patch.

Reviewed-by: Philippe Mathieu-Daudé 

> 
>>
>>>
>>> @@
>>> expression dev, name, value, errp;
>>> @@
>>> -qdev_prop_set_drive(dev, name, value, errp);
>>> +qdev_prop_set_drive_err(dev, name, value, errp);
>>>
>> [...]
> 




Re: [PATCH v5 12/13] qcow2: QcowHeaderExtension print names for extension magics

2020-06-07 Thread Andrey Shinkevich



From: Vladimir Sementsov-Ogievskiy 
Sent: Saturday, June 6, 2020 11:18 AM
To: qemu-bl...@nongnu.org 
Cc: qemu-devel@nongnu.org ; mre...@redhat.com 
; kw...@redhat.com ; ebl...@redhat.com 
; Denis Lunev ; Andrey Shinkevich 
; Vladimir Sementsov-Ogievskiy 

Subject: [PATCH v5 12/13] qcow2: QcowHeaderExtension print names for extension 
magics

Suggested-by: Andrey Shinkevich 
Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/031.out | 22 +++---
 tests/qemu-iotests/036.out |  4 ++--
 tests/qemu-iotests/061.out | 14 +++---
 tests/qemu-iotests/qcow2_format.py | 17 -
 4 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index 32371e42da..40b5bf467b 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -39,6 +39,12 @@ class Flags64(Qcow2Field):
 return str(bits)


+class Enum(Qcow2Field):
+
+def __str__(self):
+return f'{self.value:#x} ({self.mapping.get(self.value, "")})'

I've got the error E0100: invalid syntax [pylama]

Andrey



Re: [PATCH 13/16] qdev: Make qdev_prop_set_drive() match the other helpers

2020-06-07 Thread Markus Armbruster
Philippe Mathieu-Daudé  writes:

> On 6/5/20 4:56 PM, Markus Armbruster wrote:
>> qdev_prop_set_drive() can fail.  None of the other qdev_prop_set_FOO()
>> can; they abort on error.
>> 
>> To clean up this inconsistency, rename qdev_prop_set_drive() to
>> qdev_prop_set_drive_err(), and create a qdev_prop_set_drive() that
>> aborts on error.
>> 
>> Coccinelle script to update callers:
>> 
>> @ depends on !(file in "hw/core/qdev-properties-system.c")@
>> expression dev, name, value;
>> symbol error_abort;
>> @@
>> -qdev_prop_set_drive(dev, name, value, &error_abort);
>> +qdev_prop_set_drive(dev, name, value);
>
> Why not open-code qdev_prop_set_drive_err(..., &error_abort)?

Consistency with qdev_prop_set_chr() and qdev_prop_set_netdev().

My starting point was "what makes block backends so different that they
need error handling where nothing else does?"

After a considerable amount of digging, my answer is "nothing".
qdev_prop_set_drive(), qdev_prop_set_chr() and qdev_prop_set_netdev()
can all run into errors.  On closer examination, all programming errors
(thus &error_abort), except for "backend is already in use", and to
trigger that one, you have to get creative and steal the backend for
another purpose, e.g. with -global.  This is the abridged version of a
longwinded argument I didn't want to make in this series, so I left the
error handling alone.

In the longer run, I want qdev_prop_set_drive_err() to die.

>
>> 
>> @@
>> expression dev, name, value, errp;
>> @@
>> -qdev_prop_set_drive(dev, name, value, errp);
>> +qdev_prop_set_drive_err(dev, name, value, errp);
>> 
> [...]




Re: [PULL 00/19] Linux user for 5.1 patches

2020-06-07 Thread Thomas Huth
On 05/06/2020 23.48, Richard Henderson wrote:
> On 6/5/20 1:32 PM, Peter Maydell wrote:
>> On Fri, 5 Jun 2020 at 20:20, Laurent Vivier  wrote:
>>> I was thinking this kind of problem would be detected by the travis-ci
>>> builds, but in fact ppc64 and s390 builds don't build other architecture
>>> linux-user targets.
>>
>> That's an unfortunate gap in the CI -- we should ideally cover
>> the whole tree with at least one big-endian platform.
> 
> Indeed.  Hopefully we can do this with our pending gitlab setup.
> 
> For travis, IIRC we only build this restricted set because we had problems 
> with
> timeouts on those machines.

Right. But we could add one more tests with
--target-list-exclude=${MAIN_SOFTMMU_TARGETS} --enable-user in Travis
... I'll have a look...

 Thomas




Re: [PATCH] block: Remove trailing newline in format used by error_report API

2020-06-07 Thread Markus Armbruster
Philippe Mathieu-Daudé  writes:

[...]
> Why warn_reportf_err() doesn't take a 'Error **err' instead, to set err
> to NULL after freeing *err?

Why doesn't free() take a void ** argument, to set the pointer to null
after freeing what it points to?  Why doesn't close() take an int *
argument?

[...]




Re: [PATCH for-5.1 V4 3/4] hw/mips: Add Loongson-3 machine support (with KVM)

2020-06-07 Thread Huacai Chen
Hi, Alexandar,

On Mon, Jun 8, 2020 at 4:00 AM Aleksandar Markovic
 wrote:
>
> On Sun, Jun 7, 2020 at 3:13 AM chen huacai  wrote:
> >
> > On Sat, Jun 6, 2020 at 4:01 AM Aleksandar Markovic
> >  wrote:
> > >
> > > суб, 6. јун 2020. у 09:32 Aleksandar Markovic
> > >  је написао/ла:
> > > >
> > > > уто, 2. јун 2020. у 04:40 Huacai Chen  је 
> > > > написао/ла:
> > > > >
> > > > > Add Loongson-3 based machine support, it use i8259 as the interrupt
> > > > > controler and use GPEX as the pci controller. Currently it can only
> > > > > work with KVM, but we will add TCG support in future.
> > > > >
> > > > > We already have a full functional Linux kernel (based on Linux-5.4.x 
> > > > > LTS
> > > > > but not upstream yet) here:
> > > > >
> > > > > https://github.com/chenhuacai/linux
> > > > >
> > > > > How to use QEMU/Loongson-3?
> > > > > 1, Download kernel source from the above URL;
> > > > > 2, Build a kernel with arch/mips/configs/loongson3_{def,hpc}config;
> > > > > 3, Boot the a Loongson-3A4000 host with this kernel;
> > > > > 4, Build QEMU-5.0.0 with this patchset;
> > > > > 5, modprobe kvm;
> > > > > 6, Use QEMU with TCG (available in future):
> > > > >qemu-system-mips64el -M loongson3,accel=tcg -cpu 
> > > > > Loongson-3A1000 -kernel  -append ...
> > > > >Use QEMU with KVM (available at present):
> > > > >qemu-system-mips64el -M loongson3,accel=kvm -cpu 
> > > > > Loongson-3A4000 -kernel  -append ...
> > > > >
> > > > >The "-cpu" parameter can be omitted here and QEMU will use the 
> > > > > correct type for TCG/KVM automatically.
> > > > >
> > > > > Signed-off-by: Huacai Chen 
> > > > > Co-developed-by: Jiaxun Yang 
> > > > > ---
> > > >
> > > > Reviewed-by: Aleksandar Markovic 
> > > >
> > >
> > > Just the license preamble should be harmonized with the template we use 
> > > for
> > > QEMU for MIPS. It is the same license you mention in this patch - GPL 2.0 
> > > or
> > > (at option) later, just in extended wording form.
> > >
> > > There is no need for respin, I will do it by myself while applying.
> > >
> > > Best Regards,
> > > Aleksandar
> >
> > I agree to use GPL v2, thanks.
> >
>
> Hi, Huacai.
>
> Patches 1 and 2 are applied to MIPS queue, while patches 3 and 4 will await
> corresponding kernel changes to be upstreamed.
>
> It looks you do not distinguish GPL 2.0 and GPL 2.0+. The latter allows usage
> of GPL later than 2.0 (at the option of the user), while the former doesn't.
>
> I don't blame you for that. We are engineers, not lawyers.
>
> I need to ask you again, do you agree with the following license preamble:
>
>  *  This program is free software: you can redistribute it and/or modify
>  *  it under the terms of the GNU General Public License as published by
>  *  the Free Software Foundation, either version 2 of the License, or
>  *  (at your option) any later version.
>  *
>  *  This program is distributed in the hope that it will be useful,
>  *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>  *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>  *  GNU General Public License for more details.
>  *
>  *  You should have received a copy of the GNU General Public License
>  *  along with this program. If not, see .
>
> Regards,
> Aleksandar
>
Yes, I totally agree to this version of license text.

Huacai
>
> > Huacai
> >
> > > > >  default-configs/mips64el-softmmu.mak |   1 +
> > > > >  hw/mips/Kconfig  |  10 +
> > > > >  hw/mips/Makefile.objs|   1 +
> > > > >  hw/mips/loongson3.c  | 901 
> > > > > +++
> > > > >  4 files changed, 913 insertions(+)
> > > > >  create mode 100644 hw/mips/loongson3.c
> > > > >
> > > > > diff --git a/default-configs/mips64el-softmmu.mak 
> > > > > b/default-configs/mips64el-softmmu.mak
> > > > > index 9f8a3ef..2a2a3fb 100644
> > > > > --- a/default-configs/mips64el-softmmu.mak
> > > > > +++ b/default-configs/mips64el-softmmu.mak
> > > > > @@ -3,6 +3,7 @@
> > > > >  include mips-softmmu-common.mak
> > > > >  CONFIG_IDE_VIA=y
> > > > >  CONFIG_FULOONG=y
> > > > > +CONFIG_LOONGSON3=y
> > > > >  CONFIG_ATI_VGA=y
> > > > >  CONFIG_RTL8139_PCI=y
> > > > >  CONFIG_JAZZ=y
> > > > > diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
> > > > > index 67d39c5..42931fd 100644
> > > > > --- a/hw/mips/Kconfig
> > > > > +++ b/hw/mips/Kconfig
> > > > > @@ -45,6 +45,16 @@ config FULOONG
> > > > >  bool
> > > > >  select PCI_BONITO
> > > > >
> > > > > +config LOONGSON3
> > > > > +bool
> > > > > +select PCKBD
> > > > > +select SERIAL
> > > > > +select ISA_BUS
> > > > > +select PCI_EXPRESS_GENERIC_BRIDGE
> > > > > +select VIRTIO_VGA
> > > > > +select QXL if SPICE
> > > > > +select MSI_NONBROKEN
> > > > > +
> > > > >  config MIPS_CPS
> > > > >  bool
> > > > >  select PTIMER
> > > > > diff --git a/hw/mips/Makefile.objs b/hw/mips/Makefile.objs
> > > > > index 3b3e6ea..31d

Re: [PATCH v2 13/13] tests: Disable select tests under TSan, which hit TSan issue.

2020-06-07 Thread Emilio G. Cota
On Fri, Jun 05, 2020 at 13:34:22 -0400, Robert Foley wrote:
> Disable a few tests under CONFIG_TSAN, which
> run into a known TSan issue that results in a hang.
> https://github.com/google/sanitizers/issues/1116
> 
> The disabled tests under TSan include all the qtests as well as
> the test-char, test-qga, and test-qdev-global-props.
> 
> Signed-off-by: Robert Foley 

Reviewed-by: Emilio G. Cota 

Thanks,

Emilio



Re: [PATCH v1 7/9] plugins: add API to return a name for a IO device

2020-06-07 Thread Emilio G. Cota
On Tue, Jun 02, 2020 at 16:46:22 +0100, Alex Bennée wrote:
> This may well end up being anonymous but it should always be unique.
> 
> Signed-off-by: Alex Bennée 
> ---
>  include/qemu/qemu-plugin.h |  5 +
>  plugins/api.c  | 18 ++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index bab8b0d4b3a..43c6a9e857f 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -335,6 +335,11 @@ struct qemu_plugin_hwaddr 
> *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
>  bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr);
>  uint64_t qemu_plugin_hwaddr_device_offset(const struct qemu_plugin_hwaddr 
> *haddr);
>  
> +/*
> + * Returns a string representing the device. Plugin must free() it
> + */
> +char * qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr 
> *haddr);
> +
>  typedef void
>  (*qemu_plugin_vcpu_mem_cb_t)(unsigned int vcpu_index,
>   qemu_plugin_meminfo_t info, uint64_t vaddr,
> diff --git a/plugins/api.c b/plugins/api.c
> index bbdc5a4eb46..3c73de8c1c2 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -303,6 +303,24 @@ uint64_t qemu_plugin_hwaddr_device_offset(const struct 
> qemu_plugin_hwaddr *haddr
>  return 0;
>  }
>  
> +char * qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr *haddr)
> +{
> +#ifdef CONFIG_SOFTMMU
> +if (haddr && haddr->is_io) {
> +MemoryRegionSection *mrs = haddr->v.io.section;
> +if (!mrs->mr->name) {
> +return g_strdup_printf("anon%08lx", 0x & (uintptr_t) 
> mrs->mr);
> +} else {
> +return g_strdup(mrs->mr->name);
> +}
> +} else {
> +return g_strdup("RAM");
> +}
> +#else
> +return g_strdup("Invalid");
> +#endif
> +}

I'd rather use asprintf(3) and strdup(3) here, so that plugins don't
have to worry about glib, and on the QEMU side we don't have to worry
about plugins calling free() instead of g_free().

Or given that this doesn't look perf-critical, perhaps an easier way out
is to wrap the above with:

char *g_str = above();
char *ret = strdup(g_str);
g_free(g_str);
return ret;

Not sure we should NULL-check ret, since I don't know whether
mrs->mr->name is guaranteed to be non-NULL.

Thanks,
Emilio



Re: [PATCH v8 30/62] target/riscv: Update fp_status when float rounding mode changes

2020-06-07 Thread LIU Zhiwei




On 2020/6/5 11:30, Richard Henderson wrote:

On 6/4/20 7:50 PM, LIU Zhiwei wrote:

So no scalar insns will require changes within a translation block.

Not true -- scalar insns can encode rm into the instruction.


I think there is a error in gen_set_rm

static void gen_set_rm(DisasContext *ctx, int rm)
{
     TCGv_i32 t0;

     if (ctx->frm == rm) {
     return;
     }
     ctx->frm = rm;
     t0 = tcg_const_i32(rm);
     gen_helper_set_rounding_mode(cpu_env, t0);
     tcg_temp_free_i32(t0);
}

I don't know why  updating ctx->frm in this function.

This is a cache of the current rm, as most recently stored in
env->fp_status.rounding_mode.

So if we have

fadd.s  ft0, ft0, ft0, rtz
fadd.s  ft0, ft0, ft0, rtz
fadd.s  ft0, ft0, ft0, rtz

we will only switch to round_to_zero once.

Get it, thanks.

Maybe I should only gen_set_rm(ctx, 7) for each vector float insn.
And the csr write method for frm or fcsr will not change.

So I will remove this patch in the next patch set.

Zhiwei


r~





Re: hw/char: a question about watch callback function in serial

2020-06-07 Thread LIU Zhiwei



On 2020/6/5 10:19, LIU Zhiwei wrote:



On 2020/6/4 21:32, Peter Maydell wrote:

On Thu, 4 Jun 2020 at 13:15, LIU Zhiwei  wrote:

I see many UART implementations have a G_IO_OUT | G_IO_HUP  callback function.

In hw/serial.c, it is serial_watch_cb, setting by the following code,

   s->watch_tag = qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,

   serial_watch_cb, s);

In hw/candence_uart.c, it is cadence_uart_xmit, setting by the following code,

 guint r = qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,

 cadence_uart_xmit, s);



I tried to call it with booting a Linux, but the interface will never be called.

Can someone give a reasonable answer why needs this interface, or a way to call 
it.

This code is here to handle the case where the UART wants to pass
data to the chardev which is acting as the backend to the UART
(which might be host stdio, a TCP port, etc), but the backend
cannot accept data.

Older UART code (eg hw/char/pl011.c) calls qemu_chr_fe_write_all()
to write data, but this is a blocking call and these calls are
usually marked with an XXX/TODO comment, because if the chardev
backend can't currently accept the data then execution of the
guest will be blocked until the backend does start to accept
data again.

The solution to this bug was the introduction of the non-blocking
qemu_chr_fe_write() call. But to use the non-blocking call, the
UART emulation code now needs to handle the case where
qemu_chr_fe_write() says "I couldn't write all the data you asked
me to". In that case, it must use qemu_chr_fe_add_watch() to
request a callback when the chardev is able to accept new data,
so that it can try again. (It also needs to emulate telling the
guest that the transmit FIFO is not yet empty via whatever status
registers the UART has for that, because in the meantime guest
execution will continue with some of the data still not sent to
the chardev, but sitting in the emulated FIFO; and it needs to
correctly emulate "guest tried to write more data to a full FIFO".
Older UART emulations that use the blocking write_all function
don't need to bother with these details because there the tx
FIFO is always empty -- from the guest's perspective data written
to the tx FIFO drains instantaneously.)

The common case execution path is "the chardev can accept the data
faster than the guest can feed it to the UART", in which case
qemu_chr_fe_write() will return 'wrote all the data' and the
UART never needs to call qemu_chr_fe_add_watch(). To exercise the
add-watch codepath you need to connect the UART to a chardev
that can be made to stop accepting data (for instance a pipe
or a unix domain socket where there's nothing on the other end
reading data.)

Hi Peter,

Thanks, it's really a reasonable answer. However I still have one 
question.


When I tried to verify the code-path, the callback is not triggered.
The serial I used is hw/serial.c, back ended with a named pipe.

The first step is make named pipe by
mkfifo xpipe
Then run a RISC-V Linux by the command
gdb --args qemu-system-riscv64 -M virt -kernel fw_jump.elf -device 
loader,file=Image,addr=0x8020 \
-append "rootwait root=/dev/vda ro" -drive file=rootfs.ext2,format=raw,id=hd0 \
-device virtio-blk-device,drive=hd0 -serial pipe:xpipe -smp 1
Set a breakpoint on serial_watch_cb before run the Linux
b serial_watch_cb
Then run the Linux. The breakpoint will never matched.  I think "there 
is nothing on the other end reading data".


Then I tried another way to verify. Read the pipe a little later after 
booting(There are some data in the FIFO already) by

cat xpipe
Now I can read some data out from the pipe. But it still can't match 
the breakpoint. I think the reading  will set the G_IO_OUT condition,

but it doesn't.

Is there something wrong? Or could you show me a case./
/

Hi folks,

I find it's the pipe buffer size is big enough to contain all the output 
of Linux booting.

So it will not call the qemu_chr_fe_add_watch.

If I output more characters more than 64KB,  it will call the 
qemu_chr_fe_add_watch and serial_watch_cb.


By this way, I verify the code-path.

Thanks Peter. It's really a right answer.

Zhiwei/
/

/
/Best Regards,
Zhiwei/
/

thanks
-- PMM






[PATCH] migration: fix xbzrle encoding rate calculation

2020-06-07 Thread Wei Wang
It's reported an error of implicit conversion from "unsigned long" to
"double" when compiling with Clang 10. Simply make the encoding rate 0
when the encoded_size is 0.

Fixes: e460a4b1a4
Reported-by: Richard Henderson 
Signed-off-by: Wei Wang 
---
 migration/ram.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 41cc530..069b6e3 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -913,10 +913,8 @@ static void migration_update_rates(RAMState *rs, int64_t 
end_time)
 unencoded_size = (xbzrle_counters.pages - rs->xbzrle_pages_prev) *
  TARGET_PAGE_SIZE;
 encoded_size = xbzrle_counters.bytes - rs->xbzrle_bytes_prev;
-if (xbzrle_counters.pages == rs->xbzrle_pages_prev) {
+if (xbzrle_counters.pages == rs->xbzrle_pages_prev || !encoded_size) {
 xbzrle_counters.encoding_rate = 0;
-} else if (!encoded_size) {
-xbzrle_counters.encoding_rate = UINT64_MAX;
 } else {
 xbzrle_counters.encoding_rate = unencoded_size / encoded_size;
 }
-- 
2.7.4




Re: Forward migration broken down since virt-4.2 machine type

2020-06-07 Thread Ying Fang

ping

On 6/4/2020 4:51 PM, Ying Fang wrote:

Hi Richard,

Recently we are doing some tests on forward migration based on
arm virt machine. And we found the patch below breaks forward
migration compatibility from virt-4.2 to virt-5.0 above machine
type. The patch which breaks this down given by git bisect is

commit f9506e162c33e87b609549157dd8431fcc732085
target/arm: Remove ARM_FEATURE_VFP*

QEMU may get crashed on the destination host loading cpu state.
Here goes my question since I am not familiar with the VFP feature.
1: Should we keep the forward migration compatibility here ?
2: If so how can we fixed it ?

Below is the crash stack:
Thread 1 "qemu-system-aar" received signal SIGSEGV, Segmentation fault.
[Switching to LWP 712330]
armv7m_nvic_neg_prio_requested (opaque=0x0, secure=secure@entry=false) 
at  qemu/hw/intc/armv7m_nvic.c:391

391    if (s->cpu->env.v7m.faultmask[secure]) {
#0  armv7m_nvic_neg_prio_requested (opaque=0x0, 
secure=secure@entry=false) at  qemu/hw/intc/armv7m_nvic.c:391
#1  0xaaae6f766510 in arm_v7m_mmu_idx_for_secstate_and_priv 
(env=0xaaae73456780, secstate=false, priv=true) at 
qemu/target/arm/m_helper.c:2711
#2  0xaaae6f7163f0 in arm_mmu_idx_el (env=env@entry=0xaaae73456780, 
el=el@entry=1) at  qemu/target/arm/helper.c:12386
#3  0xaaae6f717000 in rebuild_hflags_internal (env=0xaaae73456780) 
at  qemu/target/arm/helper.c:12611
#4  arm_rebuild_hflags (env=env@entry=0xaaae73456780) at 
qemu/target/arm/helper.c:12624
#5  0xaaae6f722940 in cpu_post_load (opaque=0xaaae7344ceb0, 
version_id=) at  qemu/target/arm/machine.c:767
#6  0xaaae6f9e0e78 in vmstate_load_state (f=f@entry=0xaaae73020260, 
vmsd=0xaaae6fe93178 , opaque=0xaaae7344ceb0, 
version_id=22) at migration/vmstate.c:168
#7  0xaaae6f9d9858 in vmstate_load (f=f@entry=0xaaae73020260, 
se=se@entry=0xaaae7302f750) at migration/savevm.c:885
#8  0xaaae6f9dab90 in qemu_loadvm_section_start_full 
(f=f@entry=0xaaae73020260, mis=0xaaae72fb88a0) at migration/savevm.c:2302
#9  0xaaae6f9dd248 in qemu_loadvm_state_main 
(f=f@entry=0xaaae73020260, mis=mis@entry=0xaaae72fb88a0) at 
migration/savevm.c:2486
#10 0xaaae6f9de3bc in qemu_loadvm_state (f=0xaaae73020260) at 
migration/savevm.c:2560
#11 0xaaae6f9d489c in process_incoming_migration_co 
(opaque=) at migration/migration.c:461
#12 0xaaae6fb59850 in coroutine_trampoline (i0=, 
i1=) at util/coroutine-ucontext.c:115

#13 0xfffdd6c16030 in ?? () from target:/usr/lib64/libc.so.6

#0  armv7m_nvic_neg_prio_requested (opaque=0x0, 
secure=secure@entry=false) at  qemu/hw/intc/armv7m_nvic.c:391

(gdb) p    s
$4 = (NVICState *) 0x0

Thanks.
Ying




RE: [RFC] hw: nios2: update interrupt_request when STATUS_PIE disabled

2020-06-07 Thread Wu, Wentong
@Chris Wulff @Marek Vasut could you please take a look this issue and patch? 
Thanks in advance!

-Original Message-
From: Thomas Huth  
Sent: Friday, June 5, 2020 3:07 PM
To: Wu, Wentong ; qemu-devel@nongnu.org
Cc: Chris Wulff ; Marek Vasut 
Subject: Re: [RFC] hw: nios2: update interrupt_request when STATUS_PIE disabled

On 05/06/2020 07.59, Wu, Wentong wrote:
> Hi all,
> 
> I’m running icount mode on qemu_nios2 with customized  platform(almost 
> same with 10m50_devboard),
> 
> but cpu abort happened(qemu: fatal: Raised interrupt while not in I/O
> function) when guest code changes
> 
> state register with wrctl instruction, add some debug code finding 
> that it’s caused by the interrupt_request
> 
> mismatch, so I made a patch as below, not sure if it’s right, hope I 
> can have some discussion with maintainers

 Hi,

please have a look at the MAINTAINERS file in the main directory of the 
sources, you can find the corresponding maintainers there. Thus if you have 
questions related to nios2, please make sure to put Chris and Marek into CC: so 
that your patch gets the right attention!

 Thanks,
  Thomas



> commit efdb3da4e145a7a34ba8b3ab1cdcfc346ae20a11 (HEAD -> master)
> 
> Author: Wentong Wu 
> 
> Date:   Fri Jun 5 09:29:43 2020 -0400
> 
>  
> 
>     hw: nios2: update interrupt_request when CR_STATUS_PIE disabled
> 
>  
> 
>     Update interrupt_request when external interupt pends for 
> STATUS_PIE
> 
>     disabled. Otherwise on icount enabled nios2 target there will be 
> cpu
> 
>     abort when guest code changes state register with wrctl instruction.
> 
>  
> 
>     Signed-off-by: Wentong Wu 
> 
>  
> 
> diff --git a/hw/nios2/cpu_pic.c b/hw/nios2/cpu_pic.c
> 
> index 1c1989d5..b04db4d7 100644
> 
> --- a/hw/nios2/cpu_pic.c
> 
> +++ b/hw/nios2/cpu_pic.c
> 
> @@ -42,7 +42,9 @@ static void nios2_pic_cpu_handler(void *opaque, int 
> irq, int level)
> 
>  } else if (!level) {
> 
>  env->irq_pending = 0;
> 
>  cpu_reset_interrupt(cs, type);
> 
> -    }
> 
> +    } else {
> 
> +    cs->interrupt_request |= type;
> 
> +   }
> 
>  } else {
> 
>  if (level) {
> 
>  cpu_interrupt(cs, type);
> 



Re: [PATCH] net: tulip: Set PCI revision to match dec21143

2020-06-07 Thread Philippe Mathieu-Daudé
Hi Sven, could you review thiw one-line patch?

On 4/18/20 2:25 AM, Marek Vasut wrote:
> The tulip driver claims to emulate dec21143 and it does not emulate dec21142.
> The dec21142 and dec21143 can be discerned by the PCI revision register,
> where dec21142 reports value < 0x20 and dec21143 value >= 0x20. E.g. the
> U-Boot 'tulip' driver also only supports dec21143 and verifies that the
> PCI revision ID is >= 0x20, otherwise refuses to operate such a card.
> 
> This patch sets the PCI revision ID to 0x20 to match the dec21143 and
> thus also permits e.g. U-Boot to work with the tulip emulation.
> 
> Fixes: 34ea023d4b95 ("net: add tulip (dec21143) driver")
> Signed-off-by: Marek Vasut 
> Cc: Marc-André Lureau 
> Cc: Paolo Bonzini 
> Cc: Peter Maydell 
> Cc: Prasad J Pandit 
> Cc: Sven Schnelle 
> ---
>  hw/net/tulip.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/net/tulip.c b/hw/net/tulip.c
> index 1295f51d07..ffb6c2479a 100644
> --- a/hw/net/tulip.c
> +++ b/hw/net/tulip.c
> @@ -962,6 +962,8 @@ static void pci_tulip_realize(PCIDevice *pci_dev, Error 
> **errp)
>  
>  pci_conf = s->dev.config;
>  pci_conf[PCI_INTERRUPT_PIN] = 1; /* interrupt pin A */
> +/* Anything with revision < 0x20 is DC21142, anything >= 0x20 is DC21143 
> */
> +pci_conf[PCI_REVISION_ID] = 0x20;
>  
>  s->eeprom = eeprom93xx_new(&pci_dev->qdev, 64);
>  tulip_fill_eeprom(s);
> 




[PATCH v5] tcg: Sanitize shift constants on ppc64le so that shift operations with large constants don't generate invalid instructions.

2020-06-07 Thread agrecascino123
From: "Catherine A. Frederick" 

Signed-off-by: Catherine A. Frederick 
---
Okay, I removed the bad "fix" on sar_i64, and the asserts in the various 
functions. 
Crossing my fingers here.

 tcg/ppc/tcg-target.inc.c | 34 +-
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index 7da67086c6..5cb1556912 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -2610,21 +2610,33 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, 
const TCGArg *args,
 
 case INDEX_op_shl_i32:
 if (const_args[2]) {
-tcg_out_shli32(s, args[0], args[1], args[2]);
+/*
+ * Limit shift immediate to prevent illegal instruction
+ * from bitmask corruption
+ */
+tcg_out_shli32(s, args[0], args[1], args[2] & 31);
 } else {
 tcg_out32(s, SLW | SAB(args[1], args[0], args[2]));
 }
 break;
 case INDEX_op_shr_i32:
 if (const_args[2]) {
-tcg_out_shri32(s, args[0], args[1], args[2]);
+/*
+ * Both use RLWINM, which has a 5 bit field for the
+ * shift mask.
+ */
+tcg_out_shri32(s, args[0], args[1], args[2] & 31);
 } else {
 tcg_out32(s, SRW | SAB(args[1], args[0], args[2]));
 }
 break;
 case INDEX_op_sar_i32:
 if (const_args[2]) {
-tcg_out32(s, SRAWI | RS(args[1]) | RA(args[0]) | SH(args[2]));
+/*
+ * SRAWI has a 5 bit sized field for the shift mask
+ * as well.
+ */
+tcg_out32(s, SRAWI | RS(args[1]) | RA(args[0]) | SH(args[2] & 31));
 } else {
 tcg_out32(s, SRAW | SAB(args[1], args[0], args[2]));
 }
@@ -2696,20 +2708,32 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, 
const TCGArg *args,
 
 case INDEX_op_shl_i64:
 if (const_args[2]) {
-tcg_out_shli64(s, args[0], args[1], args[2]);
+/*
+ * Limit shift immediate to prevent illegal instruction from
+ * from bitmask corruption
+ */
+tcg_out_shli64(s, args[0], args[1], args[2] & 63);
 } else {
 tcg_out32(s, SLD | SAB(args[1], args[0], args[2]));
 }
 break;
 case INDEX_op_shr_i64:
 if (const_args[2]) {
-tcg_out_shri64(s, args[0], args[1], args[2]);
+/*
+ * Same applies here, as both RLDICL, and RLDICR have a
+ * 6 bit large mask for the shift value
+ */
+tcg_out_shri64(s, args[0], args[1], args[2] & 63);
 } else {
 tcg_out32(s, SRD | SAB(args[1], args[0], args[2]));
 }
 break;
 case INDEX_op_sar_i64:
 if (const_args[2]) {
+/*
+ * Already done here, as it's a split field, and
+ * somebody noticed it would have overflowed.
+ */
 int sh = SH(args[2] & 0x1f) | (((args[2] >> 5) & 1) << 1);
 tcg_out32(s, SRADI | RA(args[0]) | RS(args[1]) | sh);
 } else {
-- 
2.26.2




Re: [PULL 00/21] MIPS queue for June 7th, 2020

2020-06-07 Thread no-reply
Patchew URL: 
https://patchew.org/QEMU/1591559185-31287-1-git-send-email-aleksandar.qemu.de...@gmail.com/



Hi,

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

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

--- /tmp/qemu-test/src/tests/qemu-iotests/040.out   2020-06-07 
19:56:39.0 +
+++ /tmp/qemu-test/build/tests/qemu-iotests/040.out.bad 2020-06-07 
20:21:34.803254929 +
@@ -1,3 +1,5 @@
+WARNING:qemu.machine:qemu received signal 9: 
/tmp/qemu-test/build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64 
-display none -vga none -chardev 
socket,id=mon,path=/tmp/tmp.G5tFM6qSxm/qemu-20664-monitor.sock -mon 
chardev=mon,mode=control -qtest 
unix:path=/tmp/tmp.G5tFM6qSxm/qemu-20664-qtest.sock -accel qtest -nodefaults 
-display none -accel qtest
+WARNING:qemu.machine:qemu received signal 9: 
/tmp/qemu-test/build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64 
-display none -vga none -chardev 
socket,id=mon,path=/tmp/tmp.G5tFM6qSxm/qemu-20664-monitor.sock -mon 
chardev=mon,mode=control -qtest 
unix:path=/tmp/tmp.G5tFM6qSxm/qemu-20664-qtest.sock -accel qtest -nodefaults 
-display none -accel qtest
 ...
 --
 Ran 59 tests
---
Not run: 259
Failures: 040
Failed 1 of 119 iotests
make: *** [check-tests/check-block.sh] Error 1
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 665, in 
sys.exit(main())
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', 
'--label', 'com.qemu.instance.uuid=f770c887f5824103a4bf9eb1cb4665d2', '-u', 
'1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', 
'-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 
'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
'/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', 
'/var/tmp/patchew-tester-tmp-4e7hhqlp/src/docker-src.2020-06-07-16.13.37.32764:/var/tmp/qemu:z,ro',
 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit 
status 2.
filter=--filter=label=com.qemu.instance.uuid=f770c887f5824103a4bf9eb1cb4665d2
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-4e7hhqlp/src'
make: *** [docker-run-test-quick@centos7] Error 2

real15m14.937s
user0m9.503s


The full log is available at
http://patchew.org/logs/1591559185-31287-1-git-send-email-aleksandar.qemu.de...@gmail.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH for-5.1 V4 3/4] hw/mips: Add Loongson-3 machine support (with KVM)

2020-06-07 Thread Aleksandar Markovic
On Sun, Jun 7, 2020 at 3:13 AM chen huacai  wrote:
>
> On Sat, Jun 6, 2020 at 4:01 AM Aleksandar Markovic
>  wrote:
> >
> > суб, 6. јун 2020. у 09:32 Aleksandar Markovic
> >  је написао/ла:
> > >
> > > уто, 2. јун 2020. у 04:40 Huacai Chen  је 
> > > написао/ла:
> > > >
> > > > Add Loongson-3 based machine support, it use i8259 as the interrupt
> > > > controler and use GPEX as the pci controller. Currently it can only
> > > > work with KVM, but we will add TCG support in future.
> > > >
> > > > We already have a full functional Linux kernel (based on Linux-5.4.x LTS
> > > > but not upstream yet) here:
> > > >
> > > > https://github.com/chenhuacai/linux
> > > >
> > > > How to use QEMU/Loongson-3?
> > > > 1, Download kernel source from the above URL;
> > > > 2, Build a kernel with arch/mips/configs/loongson3_{def,hpc}config;
> > > > 3, Boot the a Loongson-3A4000 host with this kernel;
> > > > 4, Build QEMU-5.0.0 with this patchset;
> > > > 5, modprobe kvm;
> > > > 6, Use QEMU with TCG (available in future):
> > > >qemu-system-mips64el -M loongson3,accel=tcg -cpu Loongson-3A1000 
> > > > -kernel  -append ...
> > > >Use QEMU with KVM (available at present):
> > > >qemu-system-mips64el -M loongson3,accel=kvm -cpu Loongson-3A4000 
> > > > -kernel  -append ...
> > > >
> > > >The "-cpu" parameter can be omitted here and QEMU will use the 
> > > > correct type for TCG/KVM automatically.
> > > >
> > > > Signed-off-by: Huacai Chen 
> > > > Co-developed-by: Jiaxun Yang 
> > > > ---
> > >
> > > Reviewed-by: Aleksandar Markovic 
> > >
> >
> > Just the license preamble should be harmonized with the template we use for
> > QEMU for MIPS. It is the same license you mention in this patch - GPL 2.0 or
> > (at option) later, just in extended wording form.
> >
> > There is no need for respin, I will do it by myself while applying.
> >
> > Best Regards,
> > Aleksandar
>
> I agree to use GPL v2, thanks.
>

Hi, Huacai.

Patches 1 and 2 are applied to MIPS queue, while patches 3 and 4 will await
corresponding kernel changes to be upstreamed.

It looks you do not distinguish GPL 2.0 and GPL 2.0+. The latter allows usage
of GPL later than 2.0 (at the option of the user), while the former doesn't.

I don't blame you for that. We are engineers, not lawyers.

I need to ask you again, do you agree with the following license preamble:

 *  This program is free software: you can redistribute it and/or modify
 *  it under the terms of the GNU General Public License as published by
 *  the Free Software Foundation, either version 2 of the License, or
 *  (at your option) any later version.
 *
 *  This program is distributed in the hope that it will be useful,
 *  but WITHOUT ANY WARRANTY; without even the implied warranty of
 *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 *  GNU General Public License for more details.
 *
 *  You should have received a copy of the GNU General Public License
 *  along with this program. If not, see .

Regards,
Aleksandar


> Huacai
>
> > > >  default-configs/mips64el-softmmu.mak |   1 +
> > > >  hw/mips/Kconfig  |  10 +
> > > >  hw/mips/Makefile.objs|   1 +
> > > >  hw/mips/loongson3.c  | 901 
> > > > +++
> > > >  4 files changed, 913 insertions(+)
> > > >  create mode 100644 hw/mips/loongson3.c
> > > >
> > > > diff --git a/default-configs/mips64el-softmmu.mak 
> > > > b/default-configs/mips64el-softmmu.mak
> > > > index 9f8a3ef..2a2a3fb 100644
> > > > --- a/default-configs/mips64el-softmmu.mak
> > > > +++ b/default-configs/mips64el-softmmu.mak
> > > > @@ -3,6 +3,7 @@
> > > >  include mips-softmmu-common.mak
> > > >  CONFIG_IDE_VIA=y
> > > >  CONFIG_FULOONG=y
> > > > +CONFIG_LOONGSON3=y
> > > >  CONFIG_ATI_VGA=y
> > > >  CONFIG_RTL8139_PCI=y
> > > >  CONFIG_JAZZ=y
> > > > diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
> > > > index 67d39c5..42931fd 100644
> > > > --- a/hw/mips/Kconfig
> > > > +++ b/hw/mips/Kconfig
> > > > @@ -45,6 +45,16 @@ config FULOONG
> > > >  bool
> > > >  select PCI_BONITO
> > > >
> > > > +config LOONGSON3
> > > > +bool
> > > > +select PCKBD
> > > > +select SERIAL
> > > > +select ISA_BUS
> > > > +select PCI_EXPRESS_GENERIC_BRIDGE
> > > > +select VIRTIO_VGA
> > > > +select QXL if SPICE
> > > > +select MSI_NONBROKEN
> > > > +
> > > >  config MIPS_CPS
> > > >  bool
> > > >  select PTIMER
> > > > diff --git a/hw/mips/Makefile.objs b/hw/mips/Makefile.objs
> > > > index 3b3e6ea..31dedcb 100644
> > > > --- a/hw/mips/Makefile.objs
> > > > +++ b/hw/mips/Makefile.objs
> > > > @@ -4,5 +4,6 @@ obj-$(CONFIG_MALTA) += gt64xxx_pci.o malta.o
> > > >  obj-$(CONFIG_MIPSSIM) += mipssim.o
> > > >  obj-$(CONFIG_JAZZ) += jazz.o
> > > >  obj-$(CONFIG_FULOONG) += fuloong2e.o
> > > > +obj-$(CONFIG_LOONGSON3) += loongson3.o
> > > >  obj-$(CONFIG_MIPS_CPS) += cps.o
> > > >  obj-$(CONFIG_MIPS_BOSTON) += boston.

[PATCH V3 2/3] migration/colo: Update checkpoint time lately

2020-06-07 Thread Zhang Chen
From: Zhang Chen 

Previous operation(like vm_start and replication_start_all) will consume
extra time for first forced synchronization, so reduce it in this patch.

Signed-off-by: Zhang Chen 
Reviewed-by: zhanghailiang 
Reviewed-by: Lukas Straub 
Tested-by: Lukas Straub 
---
 migration/colo.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/migration/colo.c b/migration/colo.c
index 91c76789fa..2b837e1255 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -532,7 +532,6 @@ static void colo_process_checkpoint(MigrationState *s)
 {
 QIOChannelBuffer *bioc;
 QEMUFile *fb = NULL;
-int64_t current_time = qemu_clock_get_ms(QEMU_CLOCK_HOST);
 Error *local_err = NULL;
 int ret;
 
@@ -581,8 +580,8 @@ static void colo_process_checkpoint(MigrationState *s)
 qemu_mutex_unlock_iothread();
 trace_colo_vm_state_change("stop", "run");
 
-timer_mod(s->colo_delay_timer,
-current_time + s->parameters.x_checkpoint_delay);
+timer_mod(s->colo_delay_timer, qemu_clock_get_ms(QEMU_CLOCK_HOST) +
+  s->parameters.x_checkpoint_delay);
 
 while (s->state == MIGRATION_STATUS_COLO) {
 if (failover_get_state() != FAILOVER_STATUS_NONE) {
-- 
2.17.1




[PULL 20/21] target/mips: Add Loongson-3 CPU definition

2020-06-07 Thread Aleksandar Markovic
From: Huacai Chen 

Loongson-3 CPU family include Loongson-3A R1/R2/R3/R4 and Loongson-3B
R1/R2. Loongson-3A R1 is the oldest and its ISA is the smallest, while
Loongson-3A R4 is the newest and its ISA is almost the superset of all
others. To reduce complexity, we just define two CPU types:

1) "Loongson-3A1000" CPU which is corresponding to Loongson-3A R1. It is
   suitable for TCG because Loongson-3A R1 has fewest ASE.
2) "Loongson-3A4000" CPU which is corresponding to Loongson-3A R4. It is
   suitable for KVM because Loongson-3A R4 has the VZ ASE.

Loongson-3A has CONFIG6 and CONFIG7, so add their bit-fields as well.

[AM: Rearranged insn_flags, added comments, renamed lmi_helper.c,
improved commit message, fixed checkpatch warnings]

Signed-off-by: Huacai Chen 
Co-developed-by: Jiaxun Yang 
Reviewed-by: Aleksandar Markovic 
Signed-off-by: Aleksandar Markovic 
Message-Id: <1591065557-9174-3-git-send-email-che...@lemote.com>
---
 target/mips/cpu.h   | 32 ++-
 target/mips/internal.h  |  2 +
 target/mips/mips-defs.h | 45 ---
 target/mips/{lmi_helper.c => lmmi_helper.c} |  0
 target/mips/translate.c |  2 +
 target/mips/translate_init.inc.c| 86 +
 target/mips/Makefile.objs   |  2 +-
 7 files changed, 146 insertions(+), 23 deletions(-)
 rename target/mips/{lmi_helper.c => lmmi_helper.c} (100%)

diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 94d01ea..7cf7f52 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -198,8 +198,8 @@ typedef struct mips_def_t mips_def_t;
  * 3   Config3 WatchLo3  WatchHi
  * 4   Config4 WatchLo4  WatchHi
  * 5   Config5 WatchLo5  WatchHi
- * 6   WatchLo6  WatchHi
- * 7   WatchLo7  WatchHi
+ * 6   Config6 WatchLo6  WatchHi
+ * 7   Config7 WatchLo7  WatchHi
  *
  *
  * Register 20   Register 21   Register 22   Register 23
@@ -940,7 +940,35 @@ struct CPUMIPSState {
 #define CP0C5_UFR  2
 #define CP0C5_NFExists 0
 int32_t CP0_Config6;
+int32_t CP0_Config6_rw_bitmask;
+#define CP0C6_BPPASS  31
+#define CP0C6_KPOS24
+#define CP0C6_KE  23
+#define CP0C6_VTLBONLY22
+#define CP0C6_LASX21
+#define CP0C6_SSEN20
+#define CP0C6_DISDRTIME   19
+#define CP0C6_PIXNUEN 18
+#define CP0C6_SCRAND  17
+#define CP0C6_LLEXCEN 16
+#define CP0C6_DISVC   15
+#define CP0C6_VCLRU   14
+#define CP0C6_DCLRU   13
+#define CP0C6_PIXUEN  12
+#define CP0C6_DISBLKLYEN  11
+#define CP0C6_UMEMUALEN   10
+#define CP0C6_SFBEN   8
+#define CP0C6_FLTINT  7
+#define CP0C6_VLTINT  6
+#define CP0C6_DISBTB  5
+#define CP0C6_STPREFCTL   2
+#define CP0C6_INSTPREF1
+#define CP0C6_DATAPREF0
 int32_t CP0_Config7;
+int64_t CP0_Config7_rw_bitmask;
+#define CP0C7_NAPCGEN   2
+#define CP0C7_UNIMUEN   1
+#define CP0C7_VFPUCGEN  0
 uint64_t CP0_LLAddr;
 uint64_t CP0_MAAR[MIPS_MAAR_MAX];
 int32_t CP0_MAARI;
diff --git a/target/mips/internal.h b/target/mips/internal.h
index 684356e..7f159a9 100644
--- a/target/mips/internal.h
+++ b/target/mips/internal.h
@@ -36,7 +36,9 @@ struct mips_def_t {
 int32_t CP0_Config5;
 int32_t CP0_Config5_rw_bitmask;
 int32_t CP0_Config6;
+int32_t CP0_Config6_rw_bitmask;
 int32_t CP0_Config7;
+int32_t CP0_Config7_rw_bitmask;
 target_ulong CP0_LLAddr_rw_bitmask;
 int CP0_LLAddr_shift;
 int32_t SYNCI_Step;
diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h
index a831bb4..0c12910 100644
--- a/target/mips/mips-defs.h
+++ b/target/mips/mips-defs.h
@@ -15,7 +15,7 @@
  * 
  */
 /*
- *   bits 0-31: MIPS base instruction sets
+ *   bits 0-23: MIPS base instruction sets
  */
 #define ISA_MIPS1 0x0001ULL
 #define ISA_MIPS2 0x0002ULL
@@ -34,30 +34,33 @@
 #define ISA_MIPS64R6  0x4000ULL
 #define ISA_NANOMIPS320x8000ULL
 /*
- *   bits 32-47: MIPS ASEs
+ *   bits 24-39: MIPS ASEs
  */
-#define ASE_MIPS160x0001ULL
-#define ASE_MIPS3D0x0002ULL
-#define ASE_MDMX  0x0004ULL
-#define ASE_DSP   0x0008ULL
-#define ASE_DSP_R20x0010ULL
-#define ASE_DSP_R30x0020ULL
-#define ASE_MT0x0040ULL
-#define ASE_SMARTMIPS 0x0080ULL
-#define ASE_MICROMIPS 0x0100ULL
-#define ASE_MSA   0x0200ULL
+#define ASE_MI

[PULL 21/21] target/mips: Enable hardware page table walker and CMGCR features for P5600

2020-06-07 Thread Aleksandar Markovic
From: Andrea Oliveri 

Enable hardware page table walker and CMGCR features for P5600 that
supports both.

Signed-off-by: Andrea Oliveri 
Reviewed-by: Aleksandar Markovic 
Signed-off-by: Aleksandar Markovic 
Message-Id: 
---
 target/mips/translate_init.inc.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/target/mips/translate_init.inc.c b/target/mips/translate_init.inc.c
index ffae10d..637cacc 100644
--- a/target/mips/translate_init.inc.c
+++ b/target/mips/translate_init.inc.c
@@ -366,7 +366,7 @@ const mips_def_t mips_defs[] =
 },
 {
 /* FIXME:
- * Config3: CMGCR, PW, VZ, CTXTC, CDMM, TL
+ * Config3: VZ, CTXTC, CDMM, TL
  * Config4: MMUExtDef
  * Config5: MRP
  * FIR(FCR0): Has2008
@@ -380,10 +380,11 @@ const mips_def_t mips_defs[] =
(2 << CP0C1_DS) | (4 << CP0C1_DL) | (3 << CP0C1_DA) |
(1 << CP0C1_PC) | (1 << CP0C1_FP),
 .CP0_Config2 = MIPS_CONFIG2,
-.CP0_Config3 = MIPS_CONFIG3 | (1U << CP0C3_M) | (1 << CP0C3_MSAP) |
+.CP0_Config3 = MIPS_CONFIG3 | (1U << CP0C3_M) |
+   (1 << CP0C3_CMGCR) | (1 << CP0C3_MSAP) |
(1 << CP0C3_BP) | (1 << CP0C3_BI) | (1 << CP0C3_SC) |
-   (1 << CP0C3_ULRI) | (1 << CP0C3_RXI) | (1 << CP0C3_LPA) 
|
-   (1 << CP0C3_VInt),
+   (1 << CP0C3_PW) | (1 << CP0C3_ULRI) | (1 << CP0C3_RXI) |
+   (1 << CP0C3_LPA) | (1 << CP0C3_VInt),
 .CP0_Config4 = MIPS_CONFIG4 | (1U << CP0C4_M) | (2 << CP0C4_IE) |
(0x1c << CP0C4_KScrExist),
 .CP0_Config4_rw_bitmask = 0,
-- 
2.7.4




[PULL 18/21] target/mips: fpu: Refactor conversion from ieee to mips exception flags

2020-06-07 Thread Aleksandar Markovic
The original coversion function is used for regular and MSA floating
point instructions handling. Since there are some nuanced differences
between regular and MSA floating point exception handling, provide two
instances of the conversion function, rather than just a single common
one. Inline both instances of this function instances for the sake of
performance. Improve variable naming in surrounding code for clarity.

Reviewed-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Message-Id: <20200518200920.17344-17-aleksandar.qemu.de...@gmail.com>
---
 target/mips/internal.h   |  1 -
 target/mips/fpu_helper.c | 55 ++
 target/mips/msa_helper.c | 77 
 3 files changed, 82 insertions(+), 51 deletions(-)

diff --git a/target/mips/internal.h b/target/mips/internal.h
index 1bf274b..684356e 100644
--- a/target/mips/internal.h
+++ b/target/mips/internal.h
@@ -224,7 +224,6 @@ uint32_t float_class_s(uint32_t arg, float_status *fst);
 uint64_t float_class_d(uint64_t arg, float_status *fst);
 
 extern unsigned int ieee_rm[];
-int ieee_ex_to_mips(int xcpt);
 void update_pagemask(CPUMIPSState *env, target_ulong arg1, int32_t *pagemask);
 
 static inline void restore_rounding_mode(CPUMIPSState *env)
diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
index dbb8ca5..7a3a61c 100644
--- a/target/mips/fpu_helper.c
+++ b/target/mips/fpu_helper.c
@@ -189,43 +189,48 @@ void helper_ctc1(CPUMIPSState *env, target_ulong arg1, 
uint32_t fs, uint32_t rt)
 }
 }
 
-int ieee_ex_to_mips(int xcpt)
+static inline int ieee_to_mips_xcpt(int ieee_xcpt)
 {
-int ret = 0;
-if (xcpt) {
-if (xcpt & float_flag_invalid) {
-ret |= FP_INVALID;
-}
-if (xcpt & float_flag_overflow) {
-ret |= FP_OVERFLOW;
-}
-if (xcpt & float_flag_underflow) {
-ret |= FP_UNDERFLOW;
-}
-if (xcpt & float_flag_divbyzero) {
-ret |= FP_DIV0;
-}
-if (xcpt & float_flag_inexact) {
-ret |= FP_INEXACT;
-}
+int mips_xcpt = 0;
+
+if (ieee_xcpt & float_flag_invalid) {
+mips_xcpt |= FP_INVALID;
+}
+if (ieee_xcpt & float_flag_overflow) {
+mips_xcpt |= FP_OVERFLOW;
 }
-return ret;
+if (ieee_xcpt & float_flag_underflow) {
+mips_xcpt |= FP_UNDERFLOW;
+}
+if (ieee_xcpt & float_flag_divbyzero) {
+mips_xcpt |= FP_DIV0;
+}
+if (ieee_xcpt & float_flag_inexact) {
+mips_xcpt |= FP_INEXACT;
+}
+
+return mips_xcpt;
 }
 
 static inline void update_fcr31(CPUMIPSState *env, uintptr_t pc)
 {
-int tmp = ieee_ex_to_mips(get_float_exception_flags(
-  &env->active_fpu.fp_status));
+int ieee_exception_flags = get_float_exception_flags(
+   &env->active_fpu.fp_status);
+int mips_exception_flags = 0;
+
+if (ieee_exception_flags) {
+mips_exception_flags = ieee_to_mips_xcpt(ieee_exception_flags);
+}
 
-SET_FP_CAUSE(env->active_fpu.fcr31, tmp);
+SET_FP_CAUSE(env->active_fpu.fcr31, mips_exception_flags);
 
-if (tmp) {
+if (mips_exception_flags)  {
 set_float_exception_flags(0, &env->active_fpu.fp_status);
 
-if (GET_FP_ENABLE(env->active_fpu.fcr31) & tmp) {
+if (GET_FP_ENABLE(env->active_fpu.fcr31) & mips_exception_flags) {
 do_raise_exception(env, EXCP_FPE, pc);
 } else {
-UPDATE_FP_FLAGS(env->active_fpu.fcr31, tmp);
+UPDATE_FP_FLAGS(env->active_fpu.fcr31, mips_exception_flags);
 }
 }
 }
diff --git a/target/mips/msa_helper.c b/target/mips/msa_helper.c
index 3c7012c..c3b2719 100644
--- a/target/mips/msa_helper.c
+++ b/target/mips/msa_helper.c
@@ -5419,54 +5419,81 @@ static inline void check_msacsr_cause(CPUMIPSState 
*env, uintptr_t retaddr)
 #define CLEAR_IS_INEXACT   2
 #define RECIPROCAL_INEXACT 4
 
-static inline int update_msacsr(CPUMIPSState *env, int action, int denormal)
+
+static inline int ieee_to_mips_xcpt_msa(int ieee_xcpt)
 {
-int ieee_ex;
+int mips_xcpt = 0;
 
-int c;
+if (ieee_xcpt & float_flag_invalid) {
+mips_xcpt |= FP_INVALID;
+}
+if (ieee_xcpt & float_flag_overflow) {
+mips_xcpt |= FP_OVERFLOW;
+}
+if (ieee_xcpt & float_flag_underflow) {
+mips_xcpt |= FP_UNDERFLOW;
+}
+if (ieee_xcpt & float_flag_divbyzero) {
+mips_xcpt |= FP_DIV0;
+}
+if (ieee_xcpt & float_flag_inexact) {
+mips_xcpt |= FP_INEXACT;
+}
+
+return mips_xcpt;
+}
+
+static inline int update_msacsr(CPUMIPSState *env, int action, int denormal)
+{
+int ieee_exception_flags;
+int mips_exception_flags = 0;
 int cause;
 int enable;
 
-ieee_ex = get_float_exception_flags(&env->active_tc.msa_fp_status);
+ieee_exception_flags = get_float_exception_flags(
+   &env->active_tc.m

[PULL 16/21] target/mips: fpu: Remove now unused FLOAT_RINT macro

2020-06-07 Thread Aleksandar Markovic
After demacroing RINT., this macro is not needed anymore.

Reviewed-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Message-Id: <20200518200920.17344-15-aleksandar.qemu.de...@gmail.com>
---
 target/mips/fpu_helper.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
index dae1331..56ba491 100644
--- a/target/mips/fpu_helper.c
+++ b/target/mips/fpu_helper.c
@@ -1102,19 +1102,6 @@ uint64_t helper_float_rsqrt1_ps(CPUMIPSState *env, 
uint64_t fdt0)
 return ((uint64_t)fsth2 << 32) | fst2;
 }
 
-#define FLOAT_RINT(name, bits)  \
-uint ## bits ## _t helper_float_ ## name(CPUMIPSState *env, \
- uint ## bits ## _t fs) \
-{   \
-uint ## bits ## _t fdret;   \
-\
-fdret = float ## bits ## _round_to_int(fs, &env->active_fpu.fp_status); \
-update_fcr31(env, GETPC()); \
-return fdret;   \
-}
-
-#undef FLOAT_RINT
-
 uint64_t helper_float_rint_d(CPUMIPSState *env, uint64_t fs)
 {
 uint64_t fdret;
-- 
2.7.4




[PULL 15/21] target/mips: fpu: Demacro RINT.

2020-06-07 Thread Aleksandar Markovic
This is just a cosmetic change to enable tools like gcov, gdb,
callgrind, etc. to better display involved source code.

Reviewed-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Message-Id: <20200518200920.17344-14-aleksandar.qemu.de...@gmail.com>
---
 target/mips/fpu_helper.c | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
index e227e53..dae1331 100644
--- a/target/mips/fpu_helper.c
+++ b/target/mips/fpu_helper.c
@@ -1113,10 +1113,26 @@ uint ## bits ## _t helper_float_ ## name(CPUMIPSState 
*env, \
 return fdret;   \
 }
 
-FLOAT_RINT(rint_s, 32)
-FLOAT_RINT(rint_d, 64)
 #undef FLOAT_RINT
 
+uint64_t helper_float_rint_d(CPUMIPSState *env, uint64_t fs)
+{
+uint64_t fdret;
+
+fdret = float64_round_to_int(fs, &env->active_fpu.fp_status);
+update_fcr31(env, GETPC());
+return fdret;
+}
+
+uint32_t helper_float_rint_s(CPUMIPSState *env, uint32_t fs)
+{
+uint32_t fdret;
+
+fdret = float32_round_to_int(fs, &env->active_fpu.fp_status);
+update_fcr31(env, GETPC());
+return fdret;
+}
+
 #define FLOAT_CLASS_SIGNALING_NAN  0x001
 #define FLOAT_CLASS_QUIET_NAN  0x002
 #define FLOAT_CLASS_NEGATIVE_INFINITY  0x004
-- 
2.7.4




[PATCH V3 3/3] migration/migration.c: Remove MIGRATION_STATUS_ACTIVE in migration_iteration_finish

2020-06-07 Thread Zhang Chen
From: Zhang Chen 

MIGRATION_STATUS_ACTIVE is invalid here, handle it by default case.

Suggested-by: Lukas Straub 
Signed-off-by: Zhang Chen 
---
 migration/migration.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 9059238e3d..9958b15202 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3319,11 +3319,6 @@ static void migration_iteration_finish(MigrationState *s)
  */
 s->vm_was_running = true;
 /* Fallthrough */
-case MIGRATION_STATUS_ACTIVE:
-/*
- * We should really assert here, but since it's during
- * migration, let's try to reduce the usage of assertions.
- */
 case MIGRATION_STATUS_FAILED:
 case MIGRATION_STATUS_CANCELLED:
 case MIGRATION_STATUS_CANCELLING:
-- 
2.17.1




[PATCH V3 1/3] migration/colo: Optimize COLO boot code path

2020-06-07 Thread Zhang Chen
From: Zhang Chen 

No need to reuse MIGRATION_STATUS_ACTIVE boot COLO.

Signed-off-by: Zhang Chen 
Reviewed-by: zhanghailiang 
---
 migration/colo.c  |  2 --
 migration/migration.c | 17 ++---
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/migration/colo.c b/migration/colo.c
index ea7d1e9d4e..91c76789fa 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -670,8 +670,6 @@ void migrate_start_colo_process(MigrationState *s)
 colo_checkpoint_notify, s);
 
 qemu_sem_init(&s->colo_exit_sem, 0);
-migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE,
-  MIGRATION_STATUS_COLO);
 colo_process_checkpoint(s);
 qemu_mutex_lock_iothread();
 }
diff --git a/migration/migration.c b/migration/migration.c
index b63ad91d34..9059238e3d 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2972,7 +2972,10 @@ static void migration_completion(MigrationState *s)
 goto fail_invalidate;
 }
 
-if (!migrate_colo_enabled()) {
+if (migrate_colo_enabled()) {
+migrate_set_state(&s->state, current_active_state,
+  MIGRATION_STATUS_COLO);
+} else {
 migrate_set_state(&s->state, current_active_state,
   MIGRATION_STATUS_COMPLETED);
 }
@@ -3304,12 +3307,7 @@ static void migration_iteration_finish(MigrationState *s)
 migration_calculate_complete(s);
 runstate_set(RUN_STATE_POSTMIGRATE);
 break;
-
-case MIGRATION_STATUS_ACTIVE:
-/*
- * We should really assert here, but since it's during
- * migration, let's try to reduce the usage of assertions.
- */
+case MIGRATION_STATUS_COLO:
 if (!migrate_colo_enabled()) {
 error_report("%s: critical error: calling COLO code without "
  "COLO enabled", __func__);
@@ -3321,6 +3319,11 @@ static void migration_iteration_finish(MigrationState *s)
  */
 s->vm_was_running = true;
 /* Fallthrough */
+case MIGRATION_STATUS_ACTIVE:
+/*
+ * We should really assert here, but since it's during
+ * migration, let's try to reduce the usage of assertions.
+ */
 case MIGRATION_STATUS_FAILED:
 case MIGRATION_STATUS_CANCELLED:
 case MIGRATION_STATUS_CANCELLING:
-- 
2.17.1




[PULL 14/21] target/mips: fpu: Remove now unused FLOAT_CLASS macro

2020-06-07 Thread Aleksandar Markovic
After demacroing CLASS., this macro is not needed anymore.

Reviewed-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Message-Id: <20200518200920.17344-13-aleksandar.qemu.de...@gmail.com>
---
 target/mips/fpu_helper.c | 39 ---
 1 file changed, 39 deletions(-)

diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
index b3903f5..e227e53 100644
--- a/target/mips/fpu_helper.c
+++ b/target/mips/fpu_helper.c
@@ -1128,45 +1128,6 @@ FLOAT_RINT(rint_d, 64)
 #define FLOAT_CLASS_POSITIVE_SUBNORMAL 0x100
 #define FLOAT_CLASS_POSITIVE_ZERO  0x200
 
-#define FLOAT_CLASS(name, bits)  \
-uint ## bits ## _t float_ ## name(uint ## bits ## _t arg,\
-  float_status *status)  \
-{\
-if (float ## bits ## _is_signaling_nan(arg, status)) {   \
-return FLOAT_CLASS_SIGNALING_NAN;\
-} else if (float ## bits ## _is_quiet_nan(arg, status)) {\
-return FLOAT_CLASS_QUIET_NAN;\
-} else if (float ## bits ## _is_neg(arg)) {  \
-if (float ## bits ## _is_infinity(arg)) {\
-return FLOAT_CLASS_NEGATIVE_INFINITY;\
-} else if (float ## bits ## _is_zero(arg)) { \
-return FLOAT_CLASS_NEGATIVE_ZERO;\
-} else if (float ## bits ## _is_zero_or_denormal(arg)) { \
-return FLOAT_CLASS_NEGATIVE_SUBNORMAL;   \
-} else { \
-return FLOAT_CLASS_NEGATIVE_NORMAL;  \
-}\
-} else { \
-if (float ## bits ## _is_infinity(arg)) {\
-return FLOAT_CLASS_POSITIVE_INFINITY;\
-} else if (float ## bits ## _is_zero(arg)) { \
-return FLOAT_CLASS_POSITIVE_ZERO;\
-} else if (float ## bits ## _is_zero_or_denormal(arg)) { \
-return FLOAT_CLASS_POSITIVE_SUBNORMAL;   \
-} else { \
-return FLOAT_CLASS_POSITIVE_NORMAL;  \
-}\
-}\
-}\
- \
-uint ## bits ## _t helper_float_ ## name(CPUMIPSState *env,  \
- uint ## bits ## _t arg) \
-{\
-return float_ ## name(arg, &env->active_fpu.fp_status);  \
-}
-
-#undef FLOAT_CLASS
-
 uint64_t float_class_d(uint64_t arg, float_status *status)
 {
 if (float64_is_signaling_nan(arg, status)) {
-- 
2.7.4




[PULL 12/21] target/mips: fpu: Remove now unused UNFUSED_FMA and FLOAT_FMA macros

2020-06-07 Thread Aleksandar Markovic
After demacroing ., these macros
are not needed anymore.

Reviewed-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Message-Id: <20200518200920.17344-11-aleksandar.qemu.de...@gmail.com>
---
 target/mips/fpu_helper.c | 50 
 1 file changed, 50 deletions(-)

diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
index 927bac2..e8e50e4 100644
--- a/target/mips/fpu_helper.c
+++ b/target/mips/fpu_helper.c
@@ -1446,56 +1446,6 @@ FLOAT_MINMAX(mina_d, 64, minnummag)
 #undef FLOAT_MINMAX
 
 /* ternary operations */
-#define UNFUSED_FMA(prefix, a, b, c, flags)  \
-{\
-a = prefix##_mul(a, b, &env->active_fpu.fp_status);  \
-if ((flags) & float_muladd_negate_c) {   \
-a = prefix##_sub(a, c, &env->active_fpu.fp_status);  \
-} else { \
-a = prefix##_add(a, c, &env->active_fpu.fp_status);  \
-}\
-if ((flags) & float_muladd_negate_result) {  \
-a = prefix##_chs(a); \
-}\
-}
-
-/* FMA based operations */
-#define FLOAT_FMA(name, type)\
-uint64_t helper_float_ ## name ## _d(CPUMIPSState *env,  \
- uint64_t fdt0, uint64_t fdt1,   \
- uint64_t fdt2)  \
-{\
-UNFUSED_FMA(float64, fdt0, fdt1, fdt2, type);\
-update_fcr31(env, GETPC());  \
-return fdt0; \
-}\
- \
-uint32_t helper_float_ ## name ## _s(CPUMIPSState *env,  \
- uint32_t fst0, uint32_t fst1,   \
- uint32_t fst2)  \
-{\
-UNFUSED_FMA(float32, fst0, fst1, fst2, type);\
-update_fcr31(env, GETPC());  \
-return fst0; \
-}\
- \
-uint64_t helper_float_ ## name ## _ps(CPUMIPSState *env, \
-  uint64_t fdt0, uint64_t fdt1,  \
-  uint64_t fdt2) \
-{\
-uint32_t fst0 = fdt0 & 0X;   \
-uint32_t fsth0 = fdt0 >> 32; \
-uint32_t fst1 = fdt1 & 0X;   \
-uint32_t fsth1 = fdt1 >> 32; \
-uint32_t fst2 = fdt2 & 0X;   \
-uint32_t fsth2 = fdt2 >> 32; \
- \
-UNFUSED_FMA(float32, fst0, fst1, fst2, type);\
-UNFUSED_FMA(float32, fsth0, fsth1, fsth2, type); \
-update_fcr31(env, GETPC());  \
-return ((uint64_t)fsth0 << 32) | fst0;   \
-}
-#undef FLOAT_FMA
 
 uint64_t helper_float_madd_d(CPUMIPSState *env, uint64_t fst0,
  uint64_t fst1, uint64_t fst2)
-- 
2.7.4




[PATCH V3 0/3] migration/colo: Optimize COLO framework code

2020-06-07 Thread Zhang Chen
From: Zhang Chen 

This series optimize some code of COLO, please review.

Zhang Chen (3):
  migration/colo: Optimize COLO boot code path
  migration/colo: Update checkpoint time lately
  migration/migration.c: Remove MIGRATION_STATUS_ACTIVE in
migration_iteration_finish

 migration/colo.c  |  7 ++-
 migration/migration.c | 12 +---
 2 files changed, 7 insertions(+), 12 deletions(-)

-- 
2.17.1




[PULL 19/21] hw/mips: Implement the kvm_type() hook in MachineClass

2020-06-07 Thread Aleksandar Markovic
From: Huacai Chen 

MIPS has two types of KVM: TE & VZ, and TE is the default type. Now we
can't create a VZ guest in QEMU because it lacks the kvm_type() hook in
MachineClass. Besides, libvirt uses a null-machine to detect the kvm
capability, so by default it will return "KVM not supported" on a VZ
platform. Thus, null-machine also need the kvm_type() hook.

Reviewed-by: Aleksandar Markovic 
Signed-off-by: Huacai Chen 
Co-developed-by: Jiaxun Yang 
Signed-off-by: Aleksandar Markovic 
Message-Id: <1591065557-9174-2-git-send-email-che...@lemote.com>
---
 include/hw/mips/mips.h |  3 +++
 hw/core/null-machine.c |  4 
 hw/mips/common.c   | 55 ++
 hw/core/Makefile.objs  |  2 +-
 hw/mips/Makefile.objs  |  2 +-
 5 files changed, 64 insertions(+), 2 deletions(-)
 create mode 100644 hw/mips/common.c

diff --git a/include/hw/mips/mips.h b/include/hw/mips/mips.h
index 0af4c3d..2ac0580 100644
--- a/include/hw/mips/mips.h
+++ b/include/hw/mips/mips.h
@@ -20,4 +20,7 @@ void rc4030_dma_write(void *dma, uint8_t *buf, int len);
 
 DeviceState *rc4030_init(rc4030_dma **dmas, IOMMUMemoryRegion **dma_mr);
 
+/* common.c */
+int mips_kvm_type(MachineState *machine, const char *vm_type);
+
 #endif
diff --git a/hw/core/null-machine.c b/hw/core/null-machine.c
index cb47d9d..94a36f9 100644
--- a/hw/core/null-machine.c
+++ b/hw/core/null-machine.c
@@ -17,6 +17,7 @@
 #include "sysemu/sysemu.h"
 #include "exec/address-spaces.h"
 #include "hw/core/cpu.h"
+#include "hw/mips/mips.h"
 
 static void machine_none_init(MachineState *mch)
 {
@@ -50,6 +51,9 @@ static void machine_none_machine_init(MachineClass *mc)
 mc->max_cpus = 1;
 mc->default_ram_size = 0;
 mc->default_ram_id = "ram";
+#ifdef TARGET_MIPS
+mc->kvm_type = mips_kvm_type;
+#endif
 }
 
 DEFINE_MACHINE("none", machine_none_machine_init)
diff --git a/hw/mips/common.c b/hw/mips/common.c
new file mode 100644
index 000..fd7e10a
--- /dev/null
+++ b/hw/mips/common.c
@@ -0,0 +1,55 @@
+/*
+ *  Common MIPS routines
+ *
+ *  Copyright (C) 2020  Huacai Chen 
+ *
+ *  This program is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program. If not, see .
+ *
+ */
+
+#include 
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "hw/boards.h"
+#include "hw/mips/mips.h"
+#include "sysemu/kvm_int.h"
+
+#ifndef CONFIG_KVM
+
+int mips_kvm_type(MachineState *machine, const char *vm_type)
+{
+return 0;
+}
+
+#else
+
+int mips_kvm_type(MachineState *machine, const char *vm_type)
+{
+int r;
+KVMState *s = KVM_STATE(machine->accelerator);
+
+r = kvm_check_extension(s, KVM_CAP_MIPS_VZ);
+if (r > 0) {
+return KVM_VM_MIPS_VZ;
+}
+
+r = kvm_check_extension(s, KVM_CAP_MIPS_TE);
+if (r > 0) {
+return KVM_VM_MIPS_TE;
+}
+
+return -1;
+}
+
+#endif
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index 1d540ed..b5672f4 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -17,11 +17,11 @@ common-obj-$(CONFIG_SOFTMMU) += vm-change-state-handler.o
 common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o
 common-obj-$(CONFIG_SOFTMMU) += sysbus.o
 common-obj-$(CONFIG_SOFTMMU) += machine.o
-common-obj-$(CONFIG_SOFTMMU) += null-machine.o
 common-obj-$(CONFIG_SOFTMMU) += loader.o
 common-obj-$(CONFIG_SOFTMMU) += machine-hmp-cmds.o
 common-obj-$(CONFIG_SOFTMMU) += numa.o
 common-obj-$(CONFIG_SOFTMMU) += clock-vmstate.o
+obj-$(CONFIG_SOFTMMU) += null-machine.o
 obj-$(CONFIG_SOFTMMU) += machine-qmp-cmds.o
 
 common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
diff --git a/hw/mips/Makefile.objs b/hw/mips/Makefile.objs
index 739e2b7..3b3e6ea 100644
--- a/hw/mips/Makefile.objs
+++ b/hw/mips/Makefile.objs
@@ -1,4 +1,4 @@
-obj-y += addr.o mips_int.o
+obj-y += addr.o common.o mips_int.o
 obj-$(CONFIG_R4K) += r4k.o
 obj-$(CONFIG_MALTA) += gt64xxx_pci.o malta.o
 obj-$(CONFIG_MIPSSIM) += mipssim.o
-- 
2.7.4




[PULL 17/21] target/mips: fpu: Name better paired-single variables

2020-06-07 Thread Aleksandar Markovic
Use consistently 'l' and 'h' for low and high halves.

Reviewed-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Message-Id: <20200518200920.17344-16-aleksandar.qemu.de...@gmail.com>
---
 target/mips/fpu_helper.c | 62 
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
index 56ba491..dbb8ca5 100644
--- a/target/mips/fpu_helper.c
+++ b/target/mips/fpu_helper.c
@@ -1059,14 +1059,14 @@ uint32_t helper_float_recip1_s(CPUMIPSState *env, 
uint32_t fst0)
 
 uint64_t helper_float_recip1_ps(CPUMIPSState *env, uint64_t fdt0)
 {
-uint32_t fst2;
+uint32_t fstl2;
 uint32_t fsth2;
 
-fst2 = float32_div(float32_one, fdt0 & 0X,
-   &env->active_fpu.fp_status);
+fstl2 = float32_div(float32_one, fdt0 & 0X,
+&env->active_fpu.fp_status);
 fsth2 = float32_div(float32_one, fdt0 >> 32, &env->active_fpu.fp_status);
 update_fcr31(env, GETPC());
-return ((uint64_t)fsth2 << 32) | fst2;
+return ((uint64_t)fsth2 << 32) | fstl2;
 }
 
 uint64_t helper_float_rsqrt1_d(CPUMIPSState *env, uint64_t fdt0)
@@ -1091,15 +1091,15 @@ uint32_t helper_float_rsqrt1_s(CPUMIPSState *env, 
uint32_t fst0)
 
 uint64_t helper_float_rsqrt1_ps(CPUMIPSState *env, uint64_t fdt0)
 {
-uint32_t fst2;
+uint32_t fstl2;
 uint32_t fsth2;
 
-fst2 = float32_sqrt(fdt0 & 0X, &env->active_fpu.fp_status);
+fstl2 = float32_sqrt(fdt0 & 0X, &env->active_fpu.fp_status);
 fsth2 = float32_sqrt(fdt0 >> 32, &env->active_fpu.fp_status);
-fst2 = float32_div(float32_one, fst2, &env->active_fpu.fp_status);
+fstl2 = float32_div(float32_one, fstl2, &env->active_fpu.fp_status);
 fsth2 = float32_div(float32_one, fsth2, &env->active_fpu.fp_status);
 update_fcr31(env, GETPC());
-return ((uint64_t)fsth2 << 32) | fst2;
+return ((uint64_t)fsth2 << 32) | fstl2;
 }
 
 uint64_t helper_float_rint_d(CPUMIPSState *env, uint64_t fs)
@@ -1367,19 +1367,19 @@ uint32_t helper_float_recip2_s(CPUMIPSState *env, 
uint32_t fst0, uint32_t fst2)
 
 uint64_t helper_float_recip2_ps(CPUMIPSState *env, uint64_t fdt0, uint64_t 
fdt2)
 {
-uint32_t fst0 = fdt0 & 0X;
+uint32_t fstl0 = fdt0 & 0X;
 uint32_t fsth0 = fdt0 >> 32;
-uint32_t fst2 = fdt2 & 0X;
+uint32_t fstl2 = fdt2 & 0X;
 uint32_t fsth2 = fdt2 >> 32;
 
-fst2 = float32_mul(fst0, fst2, &env->active_fpu.fp_status);
+fstl2 = float32_mul(fstl0, fstl2, &env->active_fpu.fp_status);
 fsth2 = float32_mul(fsth0, fsth2, &env->active_fpu.fp_status);
-fst2 = float32_chs(float32_sub(fst2, float32_one,
+fstl2 = float32_chs(float32_sub(fstl2, float32_one,
&env->active_fpu.fp_status));
 fsth2 = float32_chs(float32_sub(fsth2, float32_one,
&env->active_fpu.fp_status));
 update_fcr31(env, GETPC());
-return ((uint64_t)fsth2 << 32) | fst2;
+return ((uint64_t)fsth2 << 32) | fstl2;
 }
 
 uint64_t helper_float_rsqrt2_d(CPUMIPSState *env, uint64_t fdt0, uint64_t fdt2)
@@ -1404,51 +1404,51 @@ uint32_t helper_float_rsqrt2_s(CPUMIPSState *env, 
uint32_t fst0, uint32_t fst2)
 
 uint64_t helper_float_rsqrt2_ps(CPUMIPSState *env, uint64_t fdt0, uint64_t 
fdt2)
 {
-uint32_t fst0 = fdt0 & 0X;
+uint32_t fstl0 = fdt0 & 0X;
 uint32_t fsth0 = fdt0 >> 32;
-uint32_t fst2 = fdt2 & 0X;
+uint32_t fstl2 = fdt2 & 0X;
 uint32_t fsth2 = fdt2 >> 32;
 
-fst2 = float32_mul(fst0, fst2, &env->active_fpu.fp_status);
+fstl2 = float32_mul(fstl0, fstl2, &env->active_fpu.fp_status);
 fsth2 = float32_mul(fsth0, fsth2, &env->active_fpu.fp_status);
-fst2 = float32_sub(fst2, float32_one, &env->active_fpu.fp_status);
+fstl2 = float32_sub(fstl2, float32_one, &env->active_fpu.fp_status);
 fsth2 = float32_sub(fsth2, float32_one, &env->active_fpu.fp_status);
-fst2 = float32_chs(float32_div(fst2, FLOAT_TWO32,
+fstl2 = float32_chs(float32_div(fstl2, FLOAT_TWO32,
&env->active_fpu.fp_status));
 fsth2 = float32_chs(float32_div(fsth2, FLOAT_TWO32,
&env->active_fpu.fp_status));
 update_fcr31(env, GETPC());
-return ((uint64_t)fsth2 << 32) | fst2;
+return ((uint64_t)fsth2 << 32) | fstl2;
 }
 
 uint64_t helper_float_addr_ps(CPUMIPSState *env, uint64_t fdt0, uint64_t fdt1)
 {
-uint32_t fst0 = fdt0 & 0X;
+uint32_t fstl0 = fdt0 & 0X;
 uint32_t fsth0 = fdt0 >> 32;
-uint32_t fst1 = fdt1 & 0X;
+uint32_t fstl1 = fdt1 & 0X;
 uint32_t fsth1 = fdt1 >> 32;
-uint32_t fst2;
+uint32_t fstl2;
 uint32_t fsth2;
 
-fst2 = float32_add(fst0, fsth0, &env->active_fpu.fp_status);
-fsth2 = float32_add(fst1, fsth1, &env->active_fpu.fp_status);
+fstl2 

[PULL 11/21] target/mips: fpu: Demacro NMSUB.

2020-06-07 Thread Aleksandar Markovic
This is just a cosmetic change to enable tools like gcov, gdb,
callgrind, etc. to better display involved source code.

Reviewed-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Message-Id: <20200518200920.17344-10-aleksandar.qemu.de...@gmail.com>
---
 target/mips/fpu_helper.c | 44 +++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
index d4c065f..927bac2 100644
--- a/target/mips/fpu_helper.c
+++ b/target/mips/fpu_helper.c
@@ -1495,7 +1495,6 @@ uint64_t helper_float_ ## name ## _ps(CPUMIPSState *env,  
   \
 update_fcr31(env, GETPC());  \
 return ((uint64_t)fsth0 << 32) | fst0;   \
 }
-FLOAT_FMA(nmsub, float_muladd_negate_result | float_muladd_negate_c)
 #undef FLOAT_FMA
 
 uint64_t helper_float_madd_d(CPUMIPSState *env, uint64_t fst0,
@@ -1619,6 +1618,49 @@ uint64_t helper_float_nmadd_ps(CPUMIPSState *env, 
uint64_t fdt0,
 return ((uint64_t)fsth0 << 32) | fstl0;
 }
 
+uint64_t helper_float_nmsub_d(CPUMIPSState *env, uint64_t fst0,
+ uint64_t fst1, uint64_t fst2)
+{
+fst0 = float64_mul(fst0, fst1, &env->active_fpu.fp_status);
+fst0 = float64_sub(fst0, fst2, &env->active_fpu.fp_status);
+fst0 = float64_chs(fst0);
+
+update_fcr31(env, GETPC());
+return fst0;
+}
+
+uint32_t helper_float_nmsub_s(CPUMIPSState *env, uint32_t fst0,
+ uint32_t fst1, uint32_t fst2)
+{
+fst0 = float32_mul(fst0, fst1, &env->active_fpu.fp_status);
+fst0 = float32_sub(fst0, fst2, &env->active_fpu.fp_status);
+fst0 = float32_chs(fst0);
+
+update_fcr31(env, GETPC());
+return fst0;
+}
+
+uint64_t helper_float_nmsub_ps(CPUMIPSState *env, uint64_t fdt0,
+  uint64_t fdt1, uint64_t fdt2)
+{
+uint32_t fstl0 = fdt0 & 0X;
+uint32_t fsth0 = fdt0 >> 32;
+uint32_t fstl1 = fdt1 & 0X;
+uint32_t fsth1 = fdt1 >> 32;
+uint32_t fstl2 = fdt2 & 0X;
+uint32_t fsth2 = fdt2 >> 32;
+
+fstl0 = float32_mul(fstl0, fstl1, &env->active_fpu.fp_status);
+fstl0 = float32_sub(fstl0, fstl2, &env->active_fpu.fp_status);
+fstl0 = float32_chs(fstl0);
+fsth0 = float32_mul(fsth0, fsth1, &env->active_fpu.fp_status);
+fsth0 = float32_sub(fsth0, fsth2, &env->active_fpu.fp_status);
+fsth0 = float32_chs(fsth0);
+
+update_fcr31(env, GETPC());
+return ((uint64_t)fsth0 << 32) | fstl0;
+}
+
 
 #define FLOAT_FMADDSUB(name, bits, muladd_arg)  \
 uint ## bits ## _t helper_float_ ## name(CPUMIPSState *env, \
-- 
2.7.4




[PULL 09/21] target/mips: fpu: Demacro MSUB.

2020-06-07 Thread Aleksandar Markovic
This is just a cosmetic change to enable tools like gcov, gdb,
callgrind, etc. to better display involved source code.

Reviewed-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Message-Id: <20200518200920.17344-8-aleksandar.qemu.de...@gmail.com>
---
 target/mips/fpu_helper.c | 40 +++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
index c070081..e37fc40 100644
--- a/target/mips/fpu_helper.c
+++ b/target/mips/fpu_helper.c
@@ -1495,7 +1495,6 @@ uint64_t helper_float_ ## name ## _ps(CPUMIPSState *env,  
   \
 update_fcr31(env, GETPC());  \
 return ((uint64_t)fsth0 << 32) | fst0;   \
 }
-FLOAT_FMA(msub, float_muladd_negate_c)
 FLOAT_FMA(nmadd, float_muladd_negate_result)
 FLOAT_FMA(nmsub, float_muladd_negate_result | float_muladd_negate_c)
 #undef FLOAT_FMA
@@ -1539,6 +1538,45 @@ uint64_t helper_float_madd_ps(CPUMIPSState *env, 
uint64_t fdt0,
 return ((uint64_t)fsth0 << 32) | fstl0;
 }
 
+uint64_t helper_float_msub_d(CPUMIPSState *env, uint64_t fst0,
+ uint64_t fst1, uint64_t fst2)
+{
+fst0 = float64_mul(fst0, fst1, &env->active_fpu.fp_status);
+fst0 = float64_sub(fst0, fst2, &env->active_fpu.fp_status);
+
+update_fcr31(env, GETPC());
+return fst0;
+}
+
+uint32_t helper_float_msub_s(CPUMIPSState *env, uint32_t fst0,
+ uint32_t fst1, uint32_t fst2)
+{
+fst0 = float32_mul(fst0, fst1, &env->active_fpu.fp_status);
+fst0 = float32_sub(fst0, fst2, &env->active_fpu.fp_status);
+
+update_fcr31(env, GETPC());
+return fst0;
+}
+
+uint64_t helper_float_msub_ps(CPUMIPSState *env, uint64_t fdt0,
+  uint64_t fdt1, uint64_t fdt2)
+{
+uint32_t fstl0 = fdt0 & 0X;
+uint32_t fsth0 = fdt0 >> 32;
+uint32_t fstl1 = fdt1 & 0X;
+uint32_t fsth1 = fdt1 >> 32;
+uint32_t fstl2 = fdt2 & 0X;
+uint32_t fsth2 = fdt2 >> 32;
+
+fstl0 = float32_mul(fstl0, fstl1, &env->active_fpu.fp_status);
+fstl0 = float32_sub(fstl0, fstl2, &env->active_fpu.fp_status);
+fsth0 = float32_mul(fsth0, fsth1, &env->active_fpu.fp_status);
+fsth0 = float32_sub(fsth0, fsth2, &env->active_fpu.fp_status);
+
+update_fcr31(env, GETPC());
+return ((uint64_t)fsth0 << 32) | fstl0;
+}
+
 
 #define FLOAT_FMADDSUB(name, bits, muladd_arg)  \
 uint ## bits ## _t helper_float_ ## name(CPUMIPSState *env, \
-- 
2.7.4




[PULL 10/21] target/mips: fpu: Demacro NMADD.

2020-06-07 Thread Aleksandar Markovic
This is just a cosmetic change to enable tools like gcov, gdb,
callgrind, etc. to better display involved source code.

Reviewed-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Message-Id: <20200518200920.17344-9-aleksandar.qemu.de...@gmail.com>
---
 target/mips/fpu_helper.c | 44 +++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
index e37fc40..d4c065f 100644
--- a/target/mips/fpu_helper.c
+++ b/target/mips/fpu_helper.c
@@ -1495,7 +1495,6 @@ uint64_t helper_float_ ## name ## _ps(CPUMIPSState *env,  
   \
 update_fcr31(env, GETPC());  \
 return ((uint64_t)fsth0 << 32) | fst0;   \
 }
-FLOAT_FMA(nmadd, float_muladd_negate_result)
 FLOAT_FMA(nmsub, float_muladd_negate_result | float_muladd_negate_c)
 #undef FLOAT_FMA
 
@@ -1577,6 +1576,49 @@ uint64_t helper_float_msub_ps(CPUMIPSState *env, 
uint64_t fdt0,
 return ((uint64_t)fsth0 << 32) | fstl0;
 }
 
+uint64_t helper_float_nmadd_d(CPUMIPSState *env, uint64_t fst0,
+ uint64_t fst1, uint64_t fst2)
+{
+fst0 = float64_mul(fst0, fst1, &env->active_fpu.fp_status);
+fst0 = float64_add(fst0, fst2, &env->active_fpu.fp_status);
+fst0 = float64_chs(fst0);
+
+update_fcr31(env, GETPC());
+return fst0;
+}
+
+uint32_t helper_float_nmadd_s(CPUMIPSState *env, uint32_t fst0,
+ uint32_t fst1, uint32_t fst2)
+{
+fst0 = float32_mul(fst0, fst1, &env->active_fpu.fp_status);
+fst0 = float32_add(fst0, fst2, &env->active_fpu.fp_status);
+fst0 = float32_chs(fst0);
+
+update_fcr31(env, GETPC());
+return fst0;
+}
+
+uint64_t helper_float_nmadd_ps(CPUMIPSState *env, uint64_t fdt0,
+  uint64_t fdt1, uint64_t fdt2)
+{
+uint32_t fstl0 = fdt0 & 0X;
+uint32_t fsth0 = fdt0 >> 32;
+uint32_t fstl1 = fdt1 & 0X;
+uint32_t fsth1 = fdt1 >> 32;
+uint32_t fstl2 = fdt2 & 0X;
+uint32_t fsth2 = fdt2 >> 32;
+
+fstl0 = float32_mul(fstl0, fstl1, &env->active_fpu.fp_status);
+fstl0 = float32_add(fstl0, fstl2, &env->active_fpu.fp_status);
+fstl0 = float32_chs(fstl0);
+fsth0 = float32_mul(fsth0, fsth1, &env->active_fpu.fp_status);
+fsth0 = float32_add(fsth0, fsth2, &env->active_fpu.fp_status);
+fsth0 = float32_chs(fsth0);
+
+update_fcr31(env, GETPC());
+return ((uint64_t)fsth0 << 32) | fstl0;
+}
+
 
 #define FLOAT_FMADDSUB(name, bits, muladd_arg)  \
 uint ## bits ## _t helper_float_ ## name(CPUMIPSState *env, \
-- 
2.7.4




[PULL 08/21] target/mips: fpu: Demacro MADD.

2020-06-07 Thread Aleksandar Markovic
This is just a cosmetic change to enable tools like gcov, gdb,
callgrind, etc. to better display involved source code.

Reviewed-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Message-Id: <20200518200920.17344-7-aleksandar.qemu.de...@gmail.com>
---
 target/mips/fpu_helper.c | 41 -
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
index a3a3968..c070081 100644
--- a/target/mips/fpu_helper.c
+++ b/target/mips/fpu_helper.c
@@ -1495,12 +1495,51 @@ uint64_t helper_float_ ## name ## _ps(CPUMIPSState 
*env, \
 update_fcr31(env, GETPC());  \
 return ((uint64_t)fsth0 << 32) | fst0;   \
 }
-FLOAT_FMA(madd, 0)
 FLOAT_FMA(msub, float_muladd_negate_c)
 FLOAT_FMA(nmadd, float_muladd_negate_result)
 FLOAT_FMA(nmsub, float_muladd_negate_result | float_muladd_negate_c)
 #undef FLOAT_FMA
 
+uint64_t helper_float_madd_d(CPUMIPSState *env, uint64_t fst0,
+ uint64_t fst1, uint64_t fst2)
+{
+fst0 = float64_mul(fst0, fst1, &env->active_fpu.fp_status);
+fst0 = float64_add(fst0, fst2, &env->active_fpu.fp_status);
+
+update_fcr31(env, GETPC());
+return fst0;
+}
+
+uint32_t helper_float_madd_s(CPUMIPSState *env, uint32_t fst0,
+ uint32_t fst1, uint32_t fst2)
+{
+fst0 = float32_mul(fst0, fst1, &env->active_fpu.fp_status);
+fst0 = float32_add(fst0, fst2, &env->active_fpu.fp_status);
+
+update_fcr31(env, GETPC());
+return fst0;
+}
+
+uint64_t helper_float_madd_ps(CPUMIPSState *env, uint64_t fdt0,
+  uint64_t fdt1, uint64_t fdt2)
+{
+uint32_t fstl0 = fdt0 & 0X;
+uint32_t fsth0 = fdt0 >> 32;
+uint32_t fstl1 = fdt1 & 0X;
+uint32_t fsth1 = fdt1 >> 32;
+uint32_t fstl2 = fdt2 & 0X;
+uint32_t fsth2 = fdt2 >> 32;
+
+fstl0 = float32_mul(fstl0, fstl1, &env->active_fpu.fp_status);
+fstl0 = float32_add(fstl0, fstl2, &env->active_fpu.fp_status);
+fsth0 = float32_mul(fsth0, fsth1, &env->active_fpu.fp_status);
+fsth0 = float32_add(fsth0, fsth2, &env->active_fpu.fp_status);
+
+update_fcr31(env, GETPC());
+return ((uint64_t)fsth0 << 32) | fstl0;
+}
+
+
 #define FLOAT_FMADDSUB(name, bits, muladd_arg)  \
 uint ## bits ## _t helper_float_ ## name(CPUMIPSState *env, \
  uint ## bits ## _t fs, \
-- 
2.7.4




[PULL 13/21] target/mips: fpu: Demacro CLASS.

2020-06-07 Thread Aleksandar Markovic
This is just a cosmetic change to enable tools like gcov, gdb,
callgrind, etc. to better display involved source code.

Reviewed-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Message-Id: <20200518200920.17344-12-aleksandar.qemu.de...@gmail.com>
---
 target/mips/fpu_helper.c | 70 ++--
 1 file changed, 68 insertions(+), 2 deletions(-)

diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
index e8e50e4..b3903f5 100644
--- a/target/mips/fpu_helper.c
+++ b/target/mips/fpu_helper.c
@@ -1165,10 +1165,76 @@ uint ## bits ## _t helper_float_ ## name(CPUMIPSState 
*env,  \
 return float_ ## name(arg, &env->active_fpu.fp_status);  \
 }
 
-FLOAT_CLASS(class_s, 32)
-FLOAT_CLASS(class_d, 64)
 #undef FLOAT_CLASS
 
+uint64_t float_class_d(uint64_t arg, float_status *status)
+{
+if (float64_is_signaling_nan(arg, status)) {
+return FLOAT_CLASS_SIGNALING_NAN;
+} else if (float64_is_quiet_nan(arg, status)) {
+return FLOAT_CLASS_QUIET_NAN;
+} else if (float64_is_neg(arg)) {
+if (float64_is_infinity(arg)) {
+return FLOAT_CLASS_NEGATIVE_INFINITY;
+} else if (float64_is_zero(arg)) {
+return FLOAT_CLASS_NEGATIVE_ZERO;
+} else if (float64_is_zero_or_denormal(arg)) {
+return FLOAT_CLASS_NEGATIVE_SUBNORMAL;
+} else {
+return FLOAT_CLASS_NEGATIVE_NORMAL;
+}
+} else {
+if (float64_is_infinity(arg)) {
+return FLOAT_CLASS_POSITIVE_INFINITY;
+} else if (float64_is_zero(arg)) {
+return FLOAT_CLASS_POSITIVE_ZERO;
+} else if (float64_is_zero_or_denormal(arg)) {
+return FLOAT_CLASS_POSITIVE_SUBNORMAL;
+} else {
+return FLOAT_CLASS_POSITIVE_NORMAL;
+}
+}
+}
+
+uint64_t helper_float_class_d(CPUMIPSState *env, uint64_t arg)
+{
+return float_class_d(arg, &env->active_fpu.fp_status);
+}
+
+uint32_t float_class_s(uint32_t arg, float_status *status)
+{
+if (float32_is_signaling_nan(arg, status)) {
+return FLOAT_CLASS_SIGNALING_NAN;
+} else if (float32_is_quiet_nan(arg, status)) {
+return FLOAT_CLASS_QUIET_NAN;
+} else if (float32_is_neg(arg)) {
+if (float32_is_infinity(arg)) {
+return FLOAT_CLASS_NEGATIVE_INFINITY;
+} else if (float32_is_zero(arg)) {
+return FLOAT_CLASS_NEGATIVE_ZERO;
+} else if (float32_is_zero_or_denormal(arg)) {
+return FLOAT_CLASS_NEGATIVE_SUBNORMAL;
+} else {
+return FLOAT_CLASS_NEGATIVE_NORMAL;
+}
+} else {
+if (float32_is_infinity(arg)) {
+return FLOAT_CLASS_POSITIVE_INFINITY;
+} else if (float32_is_zero(arg)) {
+return FLOAT_CLASS_POSITIVE_ZERO;
+} else if (float32_is_zero_or_denormal(arg)) {
+return FLOAT_CLASS_POSITIVE_SUBNORMAL;
+} else {
+return FLOAT_CLASS_POSITIVE_NORMAL;
+}
+}
+}
+
+uint32_t helper_float_class_s(CPUMIPSState *env, uint32_t arg)
+{
+return float_class_s(arg, &env->active_fpu.fp_status);
+}
+
 /* binary operations */
 
 uint64_t helper_float_add_d(CPUMIPSState *env,
-- 
2.7.4




[PULL 04/21] target/mips: fpu: Demacro SUB.

2020-06-07 Thread Aleksandar Markovic
This is just a cosmetic change to enable tools like gcov, gdb,
callgrind, etc. to better display involved source code.

Reviewed-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Message-Id: <20200518200920.17344-3-aleksandar.qemu.de...@gmail.com>
---
 target/mips/fpu_helper.c | 37 -
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
index 984f3f4..715a872 100644
--- a/target/mips/fpu_helper.c
+++ b/target/mips/fpu_helper.c
@@ -1208,7 +1208,6 @@ uint64_t helper_float_ ## name ## _ps(CPUMIPSState *env,  
 \
 return ((uint64_t)wth2 << 32) | wt2;   \
 }
 
-FLOAT_BINOP(sub)
 FLOAT_BINOP(mul)
 FLOAT_BINOP(div)
 #undef FLOAT_BINOP
@@ -1249,6 +1248,42 @@ uint64_t helper_float_add_ps(CPUMIPSState *env,
 return ((uint64_t)wth2 << 32) | wtl2;
 }
 
+uint64_t helper_float_sub_d(CPUMIPSState *env,
+uint64_t fdt0, uint64_t fdt1)
+{
+uint64_t dt2;
+
+dt2 = float64_sub(fdt0, fdt1, &env->active_fpu.fp_status);
+update_fcr31(env, GETPC());
+return dt2;
+}
+
+uint32_t helper_float_sub_s(CPUMIPSState *env,
+uint32_t fst0, uint32_t fst1)
+{
+uint32_t wt2;
+
+wt2 = float32_sub(fst0, fst1, &env->active_fpu.fp_status);
+update_fcr31(env, GETPC());
+return wt2;
+}
+
+uint64_t helper_float_sub_ps(CPUMIPSState *env,
+ uint64_t fdt0, uint64_t fdt1)
+{
+uint32_t fstl0 = fdt0 & 0X;
+uint32_t fsth0 = fdt0 >> 32;
+uint32_t fstl1 = fdt1 & 0X;
+uint32_t fsth1 = fdt1 >> 32;
+uint32_t wtl2;
+uint32_t wth2;
+
+wtl2 = float32_sub(fstl0, fstl1, &env->active_fpu.fp_status);
+wth2 = float32_sub(fsth0, fsth1, &env->active_fpu.fp_status);
+update_fcr31(env, GETPC());
+return ((uint64_t)wth2 << 32) | wtl2;
+}
+
 
 /* MIPS specific binary operations */
 uint64_t helper_float_recip2_d(CPUMIPSState *env, uint64_t fdt0, uint64_t fdt2)
-- 
2.7.4




[PULL 07/21] target/mips: fpu: Remove now unused macro FLOAT_BINOP

2020-06-07 Thread Aleksandar Markovic
After demacroing ., this macro is not
needed anymore.

Reviewed-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Message-Id: <20200518200920.17344-6-aleksandar.qemu.de...@gmail.com>
---
 target/mips/fpu_helper.c | 39 ---
 1 file changed, 39 deletions(-)

diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
index 2759c99..a3a3968 100644
--- a/target/mips/fpu_helper.c
+++ b/target/mips/fpu_helper.c
@@ -1170,45 +1170,6 @@ FLOAT_CLASS(class_d, 64)
 #undef FLOAT_CLASS
 
 /* binary operations */
-#define FLOAT_BINOP(name)  \
-uint64_t helper_float_ ## name ## _d(CPUMIPSState *env,\
- uint64_t fdt0, uint64_t fdt1) \
-{  \
-uint64_t dt2;  \
-   \
-dt2 = float64_ ## name(fdt0, fdt1, &env->active_fpu.fp_status);\
-update_fcr31(env, GETPC());\
-return dt2;\
-}  \
-   \
-uint32_t helper_float_ ## name ## _s(CPUMIPSState *env,\
- uint32_t fst0, uint32_t fst1) \
-{  \
-uint32_t wt2;  \
-   \
-wt2 = float32_ ## name(fst0, fst1, &env->active_fpu.fp_status);\
-update_fcr31(env, GETPC());\
-return wt2;\
-}  \
-   \
-uint64_t helper_float_ ## name ## _ps(CPUMIPSState *env,   \
-  uint64_t fdt0,   \
-  uint64_t fdt1)   \
-{  \
-uint32_t fst0 = fdt0 & 0X; \
-uint32_t fsth0 = fdt0 >> 32;   \
-uint32_t fst1 = fdt1 & 0X; \
-uint32_t fsth1 = fdt1 >> 32;   \
-uint32_t wt2;  \
-uint32_t wth2; \
-   \
-wt2 = float32_ ## name(fst0, fst1, &env->active_fpu.fp_status); \
-wth2 = float32_ ## name(fsth0, fsth1, &env->active_fpu.fp_status);  \
-update_fcr31(env, GETPC());\
-return ((uint64_t)wth2 << 32) | wt2;   \
-}
-
-#undef FLOAT_BINOP
 
 uint64_t helper_float_add_d(CPUMIPSState *env,
 uint64_t fdt0, uint64_t fdt1)
-- 
2.7.4




[PULL 06/21] target/mips: fpu: Demacro DIV.

2020-06-07 Thread Aleksandar Markovic
This is just a cosmetic change to enable tools like gcov, gdb,
callgrind, etc. to better display involved source code.

Reviewed-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Message-Id: <20200518200920.17344-5-aleksandar.qemu.de...@gmail.com>
---
 target/mips/fpu_helper.c | 37 -
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
index 449e945..2759c99 100644
--- a/target/mips/fpu_helper.c
+++ b/target/mips/fpu_helper.c
@@ -1208,7 +1208,6 @@ uint64_t helper_float_ ## name ## _ps(CPUMIPSState *env,  
 \
 return ((uint64_t)wth2 << 32) | wt2;   \
 }
 
-FLOAT_BINOP(div)
 #undef FLOAT_BINOP
 
 uint64_t helper_float_add_d(CPUMIPSState *env,
@@ -1319,6 +1318,42 @@ uint64_t helper_float_mul_ps(CPUMIPSState *env,
 return ((uint64_t)wth2 << 32) | wtl2;
 }
 
+uint64_t helper_float_div_d(CPUMIPSState *env,
+uint64_t fdt0, uint64_t fdt1)
+{
+uint64_t dt2;
+
+dt2 = float64_div(fdt0, fdt1, &env->active_fpu.fp_status);
+update_fcr31(env, GETPC());
+return dt2;
+}
+
+uint32_t helper_float_div_s(CPUMIPSState *env,
+uint32_t fst0, uint32_t fst1)
+{
+uint32_t wt2;
+
+wt2 = float32_div(fst0, fst1, &env->active_fpu.fp_status);
+update_fcr31(env, GETPC());
+return wt2;
+}
+
+uint64_t helper_float_div_ps(CPUMIPSState *env,
+ uint64_t fdt0, uint64_t fdt1)
+{
+uint32_t fstl0 = fdt0 & 0X;
+uint32_t fsth0 = fdt0 >> 32;
+uint32_t fstl1 = fdt1 & 0X;
+uint32_t fsth1 = fdt1 >> 32;
+uint32_t wtl2;
+uint32_t wth2;
+
+wtl2 = float32_div(fstl0, fstl1, &env->active_fpu.fp_status);
+wth2 = float32_div(fsth0, fsth1, &env->active_fpu.fp_status);
+update_fcr31(env, GETPC());
+return ((uint64_t)wth2 << 32) | wtl2;
+}
+
 
 /* MIPS specific binary operations */
 uint64_t helper_float_recip2_d(CPUMIPSState *env, uint64_t fdt0, uint64_t fdt2)
-- 
2.7.4




[PULL 05/21] target/mips: fpu: Demacro MUL.

2020-06-07 Thread Aleksandar Markovic
This is just a cosmetic change to enable tools like gcov, gdb,
callgrind, etc. to better display involved source code.

Reviewed-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Message-Id: <20200518200920.17344-4-aleksandar.qemu.de...@gmail.com>
---
 target/mips/fpu_helper.c | 37 -
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
index 715a872..449e945 100644
--- a/target/mips/fpu_helper.c
+++ b/target/mips/fpu_helper.c
@@ -1208,7 +1208,6 @@ uint64_t helper_float_ ## name ## _ps(CPUMIPSState *env,  
 \
 return ((uint64_t)wth2 << 32) | wt2;   \
 }
 
-FLOAT_BINOP(mul)
 FLOAT_BINOP(div)
 #undef FLOAT_BINOP
 
@@ -1284,6 +1283,42 @@ uint64_t helper_float_sub_ps(CPUMIPSState *env,
 return ((uint64_t)wth2 << 32) | wtl2;
 }
 
+uint64_t helper_float_mul_d(CPUMIPSState *env,
+uint64_t fdt0, uint64_t fdt1)
+{
+uint64_t dt2;
+
+dt2 = float64_mul(fdt0, fdt1, &env->active_fpu.fp_status);
+update_fcr31(env, GETPC());
+return dt2;
+}
+
+uint32_t helper_float_mul_s(CPUMIPSState *env,
+uint32_t fst0, uint32_t fst1)
+{
+uint32_t wt2;
+
+wt2 = float32_mul(fst0, fst1, &env->active_fpu.fp_status);
+update_fcr31(env, GETPC());
+return wt2;
+}
+
+uint64_t helper_float_mul_ps(CPUMIPSState *env,
+ uint64_t fdt0, uint64_t fdt1)
+{
+uint32_t fstl0 = fdt0 & 0X;
+uint32_t fsth0 = fdt0 >> 32;
+uint32_t fstl1 = fdt1 & 0X;
+uint32_t fsth1 = fdt1 >> 32;
+uint32_t wtl2;
+uint32_t wth2;
+
+wtl2 = float32_mul(fstl0, fstl1, &env->active_fpu.fp_status);
+wth2 = float32_mul(fsth0, fsth1, &env->active_fpu.fp_status);
+update_fcr31(env, GETPC());
+return ((uint64_t)wth2 << 32) | wtl2;
+}
+
 
 /* MIPS specific binary operations */
 uint64_t helper_float_recip2_d(CPUMIPSState *env, uint64_t fdt0, uint64_t fdt2)
-- 
2.7.4




[PULL 00/21] MIPS queue for June 7th, 2020

2020-06-07 Thread Aleksandar Markovic
The following changes since commit 175198ad91d8bac540159705873b4ffe4fb94eab:

  Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20200605' into 
staging (2020-06-05 17:45:59 +0100)

are available in the git repository at:

  https://github.com/AMarkovic/qemu tags/mips-queue-june-07-2020

for you to fetch changes up to ffbd8a88e8872d61fa5622a0075eddbe71951067:

  target/mips: Enable hardware page table walker and CMGCR features for P5600 
(2020-06-07 21:34:14 +0200)



MIPS queue for June 7th, 2020

Highlights:

  - Registring change of email address for two contributors
  - Cleanup and improvements of FPU helpers
  - Enabling some features of P5600
  - Adding two Loongson-3A CPU definitions
  - Moving futher towards Loongson-3A KVM support
  - Two checkpatch warnings are known and should be ignored




Aleksandar Markovic (18):
  mailmap: Change email address of Filip Bozuta
  mailmap: Change email address of Stefan Brankovic
  target/mips: fpu: Demacro ADD.
  target/mips: fpu: Demacro SUB.
  target/mips: fpu: Demacro MUL.
  target/mips: fpu: Demacro DIV.
  target/mips: fpu: Remove now unused macro FLOAT_BINOP
  target/mips: fpu: Demacro MADD.
  target/mips: fpu: Demacro MSUB.
  target/mips: fpu: Demacro NMADD.
  target/mips: fpu: Demacro NMSUB.
  target/mips: fpu: Remove now unused UNFUSED_FMA and FLOAT_FMA macros
  target/mips: fpu: Demacro CLASS.
  target/mips: fpu: Remove now unused FLOAT_CLASS macro
  target/mips: fpu: Demacro RINT.
  target/mips: fpu: Remove now unused FLOAT_RINT macro
  target/mips: fpu: Name better paired-single variables
  target/mips: fpu: Refactor conversion from ieee to mips exception
flags

Andrea Oliveri (1):
  target/mips: Enable hardware page table walker and CMGCR features for
P5600

Huacai Chen (2):
  hw/mips: Implement the kvm_type() hook in MachineClass
  target/mips: Add Loongson-3 CPU definition

 include/hw/mips/mips.h  |   3 +
 target/mips/cpu.h   |  32 +-
 target/mips/internal.h  |   3 +-
 target/mips/mips-defs.h |  45 +-
 hw/core/null-machine.c  |   4 +
 hw/mips/common.c|  55 +++
 target/mips/fpu_helper.c| 658 +++-
 target/mips/{lmi_helper.c => lmmi_helper.c} |   0
 target/mips/msa_helper.c|  77 ++--
 target/mips/translate.c |   2 +
 target/mips/translate_init.inc.c|  95 +++-
 .mailmap|   2 +
 hw/core/Makefile.objs   |   2 +-
 hw/mips/Makefile.objs   |   2 +-
 target/mips/Makefile.objs   |   2 +-
 15 files changed, 722 insertions(+), 260 deletions(-)
 create mode 100644 hw/mips/common.c
 rename target/mips/{lmi_helper.c => lmmi_helper.c} (100%)

-- 
2.7.4




[PULL 02/21] mailmap: Change email address of Stefan Brankovic

2020-06-07 Thread Aleksandar Markovic
Stefan Brankovic wants to use his new email address for his future
work in QEMU.

CC: Stefan Brankovic 
Signed-off-by: Aleksandar Markovic 
Reviewed-by: Stefan Brankovic 
Message-Id: <20200602085215.12585-3-aleksandar.qemu.de...@gmail.com>
---
 .mailmap | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.mailmap b/.mailmap
index 9f2a3a5..84f3659 100644
--- a/.mailmap
+++ b/.mailmap
@@ -52,6 +52,7 @@ Paul Burton  
 Paul Burton  
 Paul Burton  
 Philippe Mathieu-Daudé  
+Stefan Brankovic  
 Yongbok Kim  
 
 # Also list preferred name forms where people have changed their
-- 
2.7.4




[PULL 03/21] target/mips: fpu: Demacro ADD.

2020-06-07 Thread Aleksandar Markovic
This is just a cosmetic change to enable tools like gcov, gdb,
callgrind, etc. to better display involved source code.

Reviewed-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Message-Id: <20200518200920.17344-2-aleksandar.qemu.de...@gmail.com>
---
 target/mips/fpu_helper.c | 38 +-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
index 5287c86..984f3f4 100644
--- a/target/mips/fpu_helper.c
+++ b/target/mips/fpu_helper.c
@@ -1208,12 +1208,48 @@ uint64_t helper_float_ ## name ## _ps(CPUMIPSState 
*env,   \
 return ((uint64_t)wth2 << 32) | wt2;   \
 }
 
-FLOAT_BINOP(add)
 FLOAT_BINOP(sub)
 FLOAT_BINOP(mul)
 FLOAT_BINOP(div)
 #undef FLOAT_BINOP
 
+uint64_t helper_float_add_d(CPUMIPSState *env,
+uint64_t fdt0, uint64_t fdt1)
+{
+uint64_t dt2;
+
+dt2 = float64_add(fdt0, fdt1, &env->active_fpu.fp_status);
+update_fcr31(env, GETPC());
+return dt2;
+}
+
+uint32_t helper_float_add_s(CPUMIPSState *env,
+uint32_t fst0, uint32_t fst1)
+{
+uint32_t wt2;
+
+wt2 = float32_sub(fst0, fst1, &env->active_fpu.fp_status);
+update_fcr31(env, GETPC());
+return wt2;
+}
+
+uint64_t helper_float_add_ps(CPUMIPSState *env,
+ uint64_t fdt0, uint64_t fdt1)
+{
+uint32_t fstl0 = fdt0 & 0X;
+uint32_t fsth0 = fdt0 >> 32;
+uint32_t fstl1 = fdt1 & 0X;
+uint32_t fsth1 = fdt1 >> 32;
+uint32_t wtl2;
+uint32_t wth2;
+
+wtl2 = float32_add(fstl0, fstl1, &env->active_fpu.fp_status);
+wth2 = float32_add(fsth0, fsth1, &env->active_fpu.fp_status);
+update_fcr31(env, GETPC());
+return ((uint64_t)wth2 << 32) | wtl2;
+}
+
+
 /* MIPS specific binary operations */
 uint64_t helper_float_recip2_d(CPUMIPSState *env, uint64_t fdt0, uint64_t fdt2)
 {
-- 
2.7.4




[PULL 01/21] mailmap: Change email address of Filip Bozuta

2020-06-07 Thread Aleksandar Markovic
Filip Bozuta wants to use his new email address for his future
work in QEMU.

CC: Filip Bozuta 
Signed-off-by: Aleksandar Markovic 
Reviewed-by: Filip Bozuta 
Message-Id: <20200602085215.12585-2-aleksandar.qemu.de...@gmail.com>
---
 .mailmap | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.mailmap b/.mailmap
index e3628c7..9f2a3a5 100644
--- a/.mailmap
+++ b/.mailmap
@@ -45,6 +45,7 @@ Aleksandar Markovic  

 Aleksandar Rikalo  
 Aleksandar Rikalo  
 Anthony Liguori  Anthony Liguori 
+Filip Bozuta  
 James Hogan  
 Leif Lindholm  
 Paul Burton  
-- 
2.7.4




RE: [PATCH V2 1/2] migration/colo: Optimize COLO boot code path

2020-06-07 Thread Zhang, Chen



> -Original Message-
> From: Lukas Straub 
> Sent: Sunday, June 7, 2020 3:57 AM
> To: Zhang, Chen 
> Cc: Dr . David Alan Gilbert ; qemu-dev  de...@nongnu.org>; Zhanghailiang ;
> Zhang Chen 
> Subject: Re: [PATCH V2 1/2] migration/colo: Optimize COLO boot code path
> 
> On Thu,  4 Jun 2020 16:55:32 +0800
> Zhang Chen  wrote:
> 
> > From: Zhang Chen 
> >
> > No need to reuse MIGRATION_STATUS_ACTIVE boot COLO.
> >
> > Signed-off-by: Zhang Chen 
> > Reviewed-by: zhanghailiang 
> > ---
> >  migration/colo.c  |  2 --
> >  migration/migration.c | 17 ++---
> >  2 files changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/migration/colo.c b/migration/colo.c index
> > ea7d1e9d4e..91c76789fa 100644
> > --- a/migration/colo.c
> > +++ b/migration/colo.c
> > @@ -670,8 +670,6 @@ void migrate_start_colo_process(MigrationState *s)
> >  colo_checkpoint_notify, s);
> >
> >  qemu_sem_init(&s->colo_exit_sem, 0);
> > -migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE,
> > -  MIGRATION_STATUS_COLO);
> >  colo_process_checkpoint(s);
> >  qemu_mutex_lock_iothread();
> >  }
> > diff --git a/migration/migration.c b/migration/migration.c index
> > b63ad91d34..7f3f82a715 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -2972,7 +2972,10 @@ static void migration_completion(MigrationState
> *s)
> >  goto fail_invalidate;
> >  }
> >
> > -if (!migrate_colo_enabled()) {
> > +if (migrate_colo_enabled()) {
> > +migrate_set_state(&s->state, current_active_state,
> > +  MIGRATION_STATUS_COLO);
> > +} else {
> >  migrate_set_state(&s->state, current_active_state,
> >MIGRATION_STATUS_COMPLETED);
> >  }
> > @@ -3304,12 +3307,7 @@ static void
> migration_iteration_finish(MigrationState *s)
> >  migration_calculate_complete(s);
> >  runstate_set(RUN_STATE_POSTMIGRATE);
> >  break;
> > -
> > -case MIGRATION_STATUS_ACTIVE:
> > -/*
> > - * We should really assert here, but since it's during
> > - * migration, let's try to reduce the usage of assertions.
> > - */
> > +case MIGRATION_STATUS_COLO:
> >  if (!migrate_colo_enabled()) {
> >  error_report("%s: critical error: calling COLO code without "
> >   "COLO enabled", __func__); @@ -3319,6
> > +3317,11 @@ static void migration_iteration_finish(MigrationState *s)
> >   * Fixme: we will run VM in COLO no matter its old running state.
> >   * After exited COLO, we will keep running.
> >   */
> > +case MIGRATION_STATUS_ACTIVE:
> > +/*
> > + * We should really assert here, but since it's during
> > + * migration, let's try to reduce the usage of assertions.
> > + */
> 
> I think this case should be removed, because arriving here with
> MIGRATION_STATUS_ACTIVE is invalid. It should be handled by the default
> case.

OK, looks good here. I will add this part in V3.

Thanks
Zhang Chen

> 
> Regards,
> Lukas Straub
> 
> >  s->vm_was_running = true;
> >  /* Fallthrough */
> >  case MIGRATION_STATUS_FAILED:




Re: [PATCH v6 00/20] nvme: small fixes, refactoring and cleanups

2020-06-07 Thread Maxim Levitsky
On Sun, 2020-06-07 at 15:51 +0300, Maxim Levitsky wrote:
> On Tue, 2020-06-02 at 17:31 +0200, Kevin Wolf wrote:
> > Am 14.05.2020 um 06:45 hat Klaus Jensen geschrieben:
> > > From: Klaus Jensen 
> > > 
> > > Changes since v5
> > > 
> > > * Prefixed all patches with "hw/block/nvme" to avoid confusion with the
> > >   nvme block driver.
> > > 
> > > * Added patch two patches:
> > > 
> > > hw/block/nvme: fix pin-based interrupt behavior
> > > hw/block/nvme: allow use of any valid msix vector
> > > 
> > >   These were previously posted separately, but I'm including them in this
> > >   series since I didnt get any response on the separate series anyway.
> > > 
> > > * Fixed Maxim's email in the R-b on "hw/block/nvme: refactor
> > >   nvme_addr_read"
> > 
> > Thanks, applied to the block branch.
> 
> Great to see that code merged!
> 
> Note that while I kind of didn't review some of
> the latest patches mostly becasue I was doing other things,
> I do plan to keep on reviewing following patches in this activety.

And I am really tired today, when I look at all of the spelling mistakes I make 
today :-)

Best regards,
Maxim Levitsky

> 
> Best regards,
>   Maxim Levitsky
> 
> > Kevin
> > 
> > 





Re: [PATCH v7 00/14] LUKS: encryption slot management using amend interface

2020-06-07 Thread Maxim Levitsky
On Tue, 2020-06-02 at 18:29 +0200, Max Reitz wrote:
> On 18.05.20 14:20, Maxim Levitsky wrote:
> > Hi!
> > Here is the updated series of my patches, incorporating all the feedback I 
> > received.
> 
> You asked me on IRC what to do to get this series to move forward;
> considering I don’t think there were objections from anyone but me on
> v6, there are no further (substantial) objections from anyone on v7, and
> I gave all feedback I had on v6, I don’t think there’s much you can do
> right now.  (Sorry for the delay, but, well, I was on PTO as you know.)
> 
> As usual, I’ll try to get around to see whether I can rebase your series
> myself (because Dan hinted at some conflicts), and whether I feel like
> my comments were appropriately addressed (which I have little doubt
> about).  That’s the plan.
Sorry in advance if I missed any feedback from you. That can happen,
I do make mistakes like anybody of us.


> 
> Note from a couple minutes later: From what I can see, the rebase
> conflicts don’t look too wild, but I don’t feel quite comfortable
> addressing all of them.  There’s a functional one I could address in
> qemu-img.c (patch 3), where we need to bump OPTION_FORCE from 269 to
> 276.  I could do that, but that’s not the only one, unfortunately.
Yes, that conflict is easy.

> 
> Patch 7 needs a bit more extensive modification: First, we need
> %s/bdrv_filter_default_perms/bdrv_default_perms/.  Second, this will
> still not compile, because the .bdrv_child_perm interface has changed.
> BdrvChildRole is now an integer, so we also need
> s/const BdrvChildRole \*/BdrvChildRole /.
> (That gives us the nice side effect of being able to align the second
> line of the bdrv_default_perms() parameters (called from
> block_crypto_child_perms()) on the opening parenthesis.)
I did this.

> 
> Third, it becomes really interesting.  With these changes, it would be
> wrong, because bdrv_default_perms() will then not use the permissions
> for a filter but for an image file with metadata – because that’s what
> the LUKS file child is.
> 
> But that’s actually a bug that’s already there (and that I introduced).
>  It makese sense to fix it in your series here, because to fix it we
> need a dedicated block_crypto_child_perms() function anyway.
> 
> So, well.  Do we want to cheat?  Just let block_crypto_child_perms()
> call bdrv_default_perms() with role=BDRV_CHILD_FILTERED?  That would be
> the previous behavior, but it would also be bad cheating.
> 
> The comment that exists (before patch 7) above
> bdrv_crypto_luks.bdrv_child_perm says that we just want to allow
> share-rw=on, and we could achieve that simply by force-sharing the WRITE
> permission after invoking bdrv_default_perms() with the actual role
> (which is BDRV_CHILD_IMAGE).
> 
> But what about the other stuff that sets
> bdrv_default_perms_for_storage() apart from bdrv_filter_default_perms()?
>  I.e., force-taking WRITE and RESIZE permissions if the file is
> writable; force-taking the CONSISTENT_READ permission because we need
> the metadata; and unsharing the RESIZE permission?
> 
> I think the best thing to do would be to call bdrv_default_perms() with
> the @role passed to block_crypto_child_perms() (i.e., BDRV_CHILD_IMAGE),
> and then relaxing the permissions, that is to share the WRITE and RESIZE
> persmissions, and to perhaps restore the WRITE and RESIZE permissions
> from what’s given to block_crypto_child_perms() (i.e., *nperm &= ~(WRITE
> > RESIZE); *nperm |= perm & (WRITE | RESIZE);), because LUKS doesn’t
> need them unless the image may actually be written.
> 
> (OTOH, force-taking CONSISTENT_READ seems entirely reasonable.)

Could we talk about this on IRC tomorrow? I don't fully understand you here,
and I sort of understood this stuff back when I learned it, but
that was long ago, and since we are talking about permissions here,
plus an necessary hack that luks now have to make, I would like
to understand exactly what I am doing.

Other than that, I rebased the series, and all iotests (with the permission
fix below) are passing.
I'll send the rebased version once we finish the permission thing.


Best regards,
Maxim Levitsky


PS: This code which is roughtly based on your suggestions,
does pass my test write sharing test, but I don't have much confidence that it 
is correct:

For example, I don't think that resize permission is needed to be touched,
since resize of the 'luks' image shoudn't have any affect on encryption keys
(since luks image is basically the underlying file minus the header, decrypted,
and we don't really change the encryption key, but a encrypted keyslot which 
contains it).


BlockCrypto *crypto = bs->opaque;

bdrv_default_perms(bs, c, role, reopen_queue, perm, shared, nperm, nshared);
/*
 * Ask for consistent read permission so that if
 * someone else tries to open this image with this permission
 * neither will be able to edit encryption keys, since
 * we will unshare that permission wh

Re: [PATCH] MAINTAINERS: Fix the classification of bios-tables-test-allowed-diff.h

2020-06-07 Thread Philippe Mathieu-Daudé
On Sun, Jun 7, 2020 at 7:21 AM Thomas Huth  wrote:
>
> The file tests/qtest/bios-tables-test-allowed-diff.h is currently only
> assigned to the qtest section according MAINTAINERS. However, this file
> normally only gets updated when the ACPI tables changed - something the
> qtest maintainers don't have much clue of. Thus this file should rather
> be assigned to the ACPI maintainers instead.
>
> Signed-off-by: Thomas Huth 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  MAINTAINERS | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6e7890ce82..0c5ed09ad5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1531,7 +1531,7 @@ F: hw/acpi/*
>  F: hw/smbios/*
>  F: hw/i386/acpi-build.[hc]
>  F: hw/arm/virt-acpi-build.c
> -F: tests/qtest/bios-tables-test.c
> +F: tests/qtest/bios-tables-test*
>  F: tests/qtest/acpi-utils.[hc]
>  F: tests/data/acpi/
>
> @@ -2321,6 +2321,7 @@ S: Maintained
>  F: qtest.c
>  F: accel/qtest.c
>  F: tests/qtest/
> +X: tests/qtest/bios-tables-test-allowed-diff.h
>
>  Device Fuzzing
>  M: Alexander Bulekov 
> --
> 2.18.1
>
>




Re: [PATCH v6 00/20] nvme: small fixes, refactoring and cleanups

2020-06-07 Thread Maxim Levitsky
On Tue, 2020-06-02 at 17:31 +0200, Kevin Wolf wrote:
> Am 14.05.2020 um 06:45 hat Klaus Jensen geschrieben:
> > From: Klaus Jensen 
> > 
> > Changes since v5
> > 
> > * Prefixed all patches with "hw/block/nvme" to avoid confusion with the
> >   nvme block driver.
> > 
> > * Added patch two patches:
> > 
> > hw/block/nvme: fix pin-based interrupt behavior
> > hw/block/nvme: allow use of any valid msix vector
> > 
> >   These were previously posted separately, but I'm including them in this
> >   series since I didnt get any response on the separate series anyway.
> > 
> > * Fixed Maxim's email in the R-b on "hw/block/nvme: refactor
> >   nvme_addr_read"
> 
> Thanks, applied to the block branch.

Great to see that code merged!

Note that while I kind of didn't review some of
the latest patches mostly becasue I was doing other things,
I do plan to keep on reviewing following patches in this activety.

Best regards,
Maxim Levitsky

> 
> Kevin
> 
> 





Re: [PATCH] Makefile: set PYTHON to python2 instead of python

2020-06-07 Thread Frédéric Pierret
I should have included [SeaBIOS]. Sorry.

On 2020-06-07 10:56, Frédéric Pierret (fepitre) wrote:
> Newer distro like CentOS 8 does not include any reference
> to 'python' binary but only 'python2'.
> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index c20be15..995dc86 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -22,7 +22,7 @@ LD=$(CROSS_PREFIX)ld
>  OBJCOPY=$(CROSS_PREFIX)objcopy
>  OBJDUMP=$(CROSS_PREFIX)objdump
>  STRIP=$(CROSS_PREFIX)strip
> -PYTHON=python
> +PYTHON=python2
>  CPP=cpp
>  IASL:=iasl
>  LD32BIT_FLAG:=-melf_i386
> 



signature.asc
Description: OpenPGP digital signature


[PATCH] Makefile: set PYTHON to python2 instead of python

2020-06-07 Thread fepitre
Newer distro like CentOS 8 does not include any reference
to 'python' binary but only 'python2'.
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index c20be15..995dc86 100644
--- a/Makefile
+++ b/Makefile
@@ -22,7 +22,7 @@ LD=$(CROSS_PREFIX)ld
 OBJCOPY=$(CROSS_PREFIX)objcopy
 OBJDUMP=$(CROSS_PREFIX)objdump
 STRIP=$(CROSS_PREFIX)strip
-PYTHON=python
+PYTHON=python2
 CPP=cpp
 IASL:=iasl
 LD32BIT_FLAG:=-melf_i386
-- 
2.25.4





Re: gitlab-ci: Do not use the standard container images from gitlab

2020-06-07 Thread Sam Eiderman
I see, thanks for the clarification.

However sometimes builds usually do tend to work on Ubuntu but fail to
work on Debian since it's not always a 1-1 (as in this case) - so you
might want to consider to keep testing Debian together with Ubuntu.

Regarding the Ubuntu 20 problem - have you tried "export
DEBIAN_FRONTEND=noninteractive"? didn't see it in the logs

Sam


On Sun, Jun 7, 2020 at 8:39 AM Thomas Huth  wrote:
>
> On 06/06/2020 14.38, Sam Eiderman wrote:
> > Thanks for the link
> >
> > I do believe that the correct approach for me is to rename
> > BITS_PER_LONG to __BITS_PER_LONG (I just added a sed command in my
> > Dockerfile) and move on with my particular usage, however I am just
> > wondering whether dropping debian10/ubuntu20 in the official qemu ci/
> > pipeline until it's fixed is the correct approach instead of keep
> > failing it until the error resolves, in a way we want to always know
> > on which OSs the compilation fails for visibility, no?
>
>  Hi,
>
> that bug was only one reason to move the pipelines to another OS. The
> other reason is that we are already extensively testing various Ubuntu
> (and thus Debian-based) versions in the Travis CI - but did not test any
> RPM-based distros in the CI yet. Since Travis is bound to Ubuntu, we can
> not test Fedora/CentOS there, thus the Gitlab CI pipelines have now been
> moved to RPM-based distros (except for the "build-user" pipeline which
> is still using Debian, and the "build-system1" which is now using Ubuntu
> 19.04 instead, so I think we still have a good mix there).
>
> Note that the problem with Ubuntu 20.04 is also something completely
> different: It hangs in an interactive prompt during update and waits for
> user input, so that the pipelines finally times out:
>
>  https://gitlab.com/huth/qemu/-/jobs/584573287#L800
>
> If you know a work-around for that, we can move the build-system1
> pipeline from 19.04 to 20.04 ... or if Debian gets finally fixed, we can
> also move that pipeline back to Debian. I'm fine either way, as long as
> the pipelines do not fail due to non-QEMU bugs in the distros.
>
>  Thomas
>