Re: [Qemu-devel] [PATCH v3 0/3] i.MX: Add the i.MX6UL SOC and a reference board.

[Thomas Huth]

On 2018-07-30 22:03, Jean-Christophe Dubois wrote:
> This series adds the i.MX6UL SOC from NXP/Freescale and the reference
> evaluation board.
> 
> This series was tested by booting linux 4.18 (built using imx_v6_v7_defconfig)
> on the emulated board (with the appropriate device tree).
> 
> Jean-Christophe Dubois (3):
>   i.MX6UL: Add i.MX6UL specific CCM device
>   i.MX6UL: Add i.MX6UL SOC
>   i.MX6UL: Add Freescale i.MX6 UltraLite 14x14 EVK Board
> 
>  default-configs/arm-softmmu.mak |   1 +
>  hw/arm/Makefile.objs|   1 +
>  hw/arm/fsl-imx6ul.c | 617 ++
>  hw/arm/mcimx6ul-evk.c   |  85 +++
>  hw/misc/Makefile.objs   |   1 +
>  hw/misc/imx6ul_ccm.c| 890 
>  hw/misc/trace-events|   7 +
>  include/hw/arm/fsl-imx6ul.h | 339 
>  include/hw/misc/imx6ul_ccm.h| 226 
>  9 files changed, 2167 insertions(+)
>  create mode 100644 hw/arm/fsl-imx6ul.c
>  create mode 100644 hw/arm/mcimx6ul-evk.c
>  create mode 100644 hw/misc/imx6ul_ccm.c
>  create mode 100644 include/hw/arm/fsl-imx6ul.h
>  create mode 100644 include/hw/misc/imx6ul_ccm.h

Can we please also get an entry to the MAINTAINERS file for this new
board / files?

 Thanks,
  Thomas

Re: [Qemu-devel] [PATCH 3/3] cirrus: mark as deprecated

[P J P]

  Hello Dan, all

+-- On Thu, 25 Oct 2018, Daniel P. Berrangé wrote --+
| On Thu, Oct 25, 2018 at 10:52:56AM +0200, Gerd Hoffmann wrote:
| > While being at it deprecate cirrus too.
| > 
| > Reason (short version): use stdvga instead.
| > Verbose version:
| > https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful
| 
| 
| I don't debate the points in the blog post above that stdvga is a
| better choice, but I don't think that's enough to justify deprecating
| cirrus at this point in time, because when it then gets deleted it
| will break way too many existing deployments.
| 
| We need to socialize info in that blog post above more widely and
| especially ensure that apps are not using that by default. I don't
| see it being viable to formally deprecate it in QEMU any time soon
| though given existing usage.

To note, IMO there are other devices/sources in QEMU which are potential 
candidates for deprecation, similar to adlib etc. It'll help if we could 
device a process to deprecate/remove such code base. Other than maintenance it 
invariably also becomes source of security issues.

Ex.(similar to Fedora) we could announce such candidate on qemu-devel list and 
after review over a period of say a month, candidate will be 
deprecated/expunged. (thinking aloud)

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH v1] arm: check bit index before usage

[Peter Maydell]

On 25 October 2018 at 21:31, P J P  wrote:
> +-- On Thu, 25 Oct 2018, Peter Maydell wrote --+
> | Hi; thanks for this patch. Looking at the SA1110 manual,
> | it says that writes to the reserved bits [31:28] are
> | ignored. So I think that rather than doing this check
> | here, we should do what the strongarm_ppc_* code in the
> | same file does -- mask off the high bits for writes to
> | the direction and state registers. Then it will not
> | be possible for high bits to be set here that cause an
> | out-of-range array access.
>
> ===
> diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c
> index ec2627374d..dd8c4b1f2e 100644
> --- a/hw/arm/strongarm.c
> +++ b/hw/arm/strongarm.c
> @@ -587,12 +587,12 @@ static void strongarm_gpio_write(void *opaque, hwaddr
> offset,
>
>  switch (offset) {
>  case GPDR:/* GPIO Pin-Direction registers */
> -s->dir = value;
> +s->dir = value & 0x3f;
>  strongarm_gpio_handler_update(s);
>  break;
>
>  case GPSR:/* GPIO Pin-Output Set registers */
> -s->olevel |= value;
> +s->olevel |= value & 0x3f;
>  strongarm_gpio_handler_update(s);
>  break;
> ===
>
> does this seem okay?

Yes, that's what I had in mind.

thanks
-- PMM

Re: [Qemu-devel] [libvirt] [PATCH 2/3] adlib: mark as insecure and deprecated.

[P J P]

+-- On Thu, 25 Oct 2018, Daniel P. Berrangé wrote --+
| On Thu, Oct 25, 2018 at 04:26:16PM +0530, P J P wrote:
| > +-- On Thu, 25 Oct 2018, Gerd Hoffmann wrote --+
| > | We have a lovely, guest-triggerable buffer overflow in opl2 emulation.
| > | 
| > | Reproducer:
| > | outw(0xff60, 0x220);
| > | outw(0x1020, 0x220);
| > | outw(0xffb0, 0x220);
| > | Result:
| > | Will overflow FM_OPL->AR_TABLE[] (see hw/audio/fmopl.[ch])
| > 
| > + Reported-by: Wangjunqing 
| 
| So you have a CVE number for this ?

No, since the adlib device is not used as much and is being deprecated, I'm 
not inclined to get one.

--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH v8 00/38] target/mips: Limited support for the R5900

[Richard Henderson]

On 10/25/18 7:03 PM, Maciej W. Rozycki wrote:
> Overall this source file is clearly a modified copy of an ancient version 
> of the opcode table included with the opcodes library from binutils and I 
> think it would benefit from a refresh.

You can't do that because of GPL v3, sadly.


r~

[Qemu-devel] [PATCH v2] strongarm: mask off high[32:28] bits from dir and state registers

[P J P]

From: Prasad J Pandit 

The high[32:28] bits of 'direction' and 'state' registers of
SA-1100/SA-1110 device are reserved. Setting them may lead to
OOB 's->handler[]' array access issue. Mask off [32:28] bits to
avoid it.

Reported-by: Moguofang 
Signed-off-by: Prasad J Pandit 
---
 hw/arm/strongarm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Update v2: mask off high[32:28] bits
  -> https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg05746.html

diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c
index ec2627374d..dd8c4b1f2e 100644
--- a/hw/arm/strongarm.c
+++ b/hw/arm/strongarm.c
@@ -587,12 +587,12 @@ static void strongarm_gpio_write(void *opaque, hwaddr 
offset,
 
 switch (offset) {
 case GPDR:/* GPIO Pin-Direction registers */
-s->dir = value;
+s->dir = value & 0x3f;
 strongarm_gpio_handler_update(s);
 break;
 
 case GPSR:/* GPIO Pin-Output Set registers */
-s->olevel |= value;
+s->olevel |= value & 0x3f;
 strongarm_gpio_handler_update(s);
 break;
 
-- 
2.17.2

Re: [Qemu-devel] [PATCH 3/3] ppc/pnv: check size before data buffer access

[Cédric Le Goater]

Hello Prasad,

On 10/25/18 8:45 AM, P J P wrote:
>   Hello Cedric,
> 
> +-- On Wed, 24 Oct 2018, Cédric Le Goater wrote --+
> | I think using a data[8] would be more appropriate. It would make the 
> | pnv_lpc_do_eccb() routine a little more complex. I tried to rewrite it to 
> | have a common one with the P9 LPC model but could not find a common 
> pattern. 
> | P9 is purely MMIO based. Something on the TODO list.
> | 
> | 8 bytes accesses will then fail anyhow because all MemoryRegionOps have a 
> | max_access_size = 4.
> 
> Thank you for these inputs, appreciate it. To confirm,
> 
>  - You plan to send a v2 patch to fix the OOB access issue along with 
>refactoring pnv_lpc_do_eccb? 

we might improve pnv_lpc_do_eccb() when the P9 LPC model is sent. I am not
sure of that as this is for P8 only.

> If yes, kindly include me in CC please.

yes sure.  

> OR
> 
>  - While we refactor the routine for better, a patch below seem okay to fix 
>the OOB access issue?

I think it is fine. Please add something like :  

qemu_log_mask(LOG_GUEST_ERROR, "ECCB: invalid operation at @0x%08x" 
" size %d\n", opb_addr, sz);


Thanks,

C.

> ===
> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
> index d7721320a2..655d5f3d07 100644
> --- a/hw/ppc/pnv_lpc.c
> +++ b/hw/ppc/pnv_lpc.c
> @@ -155,9 +155,13 @@ static void pnv_lpc_do_eccb(PnvLpcController *lpc, 
> uint64_t cmd)
>  /* XXX Check for magic bits at the top, addr size etc... */
>  unsigned int sz = (cmd & ECCB_CTL_SZ_MASK) >> ECCB_CTL_SZ_LSH;
>  uint32_t opb_addr = cmd & ECCB_CTL_ADDR_MASK;
> -uint8_t data[4];
> +uint8_t data[8];
>  bool success;
>  
> +if (sz > sizeof(data)) {
> +return;
> +}
> +
>  if (cmd & ECCB_CTL_READ) {
>  success = opb_read(lpc, opb_addr, data, sz);
>  if (success) {
> ===
> 
> 
> Thank you.
> --
> Prasad J Pandit / Red Hat Product Security Team
> 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
> 

[Qemu-devel] [PATCH v3 10/16] gdbstub: add multiprocess support to 'D' packets

[Luc Michel]

'D' packets are used by GDB to detach from a process. In multiprocess
mode, the PID to detach from is sent in the request.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 60 ---
 1 file changed, 53 insertions(+), 7 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 39b1766f28..4d8474204f 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1036,24 +1036,39 @@ static int gdb_breakpoint_remove(target_ulong addr, 
target_ulong len, int type)
 default:
 return -ENOSYS;
 }
 }
 
+static inline void gdb_cpu_breakpoint_remove_all(CPUState *cpu)
+{
+cpu_breakpoint_remove_all(cpu, BP_GDB);
+#ifndef CONFIG_USER_ONLY
+cpu_watchpoint_remove_all(cpu, BP_GDB);
+#endif
+}
+
+static void gdb_process_breakpoint_remove_all(const GDBState *s, GDBProcess *p)
+{
+CPUState *cpu = get_first_cpu_in_process(s, p);
+
+while (cpu) {
+gdb_cpu_breakpoint_remove_all(cpu);
+cpu = gdb_next_cpu_in_process(s, cpu);
+}
+}
+
 static void gdb_breakpoint_remove_all(void)
 {
 CPUState *cpu;
 
 if (kvm_enabled()) {
 kvm_remove_all_breakpoints(gdbserver_state->c_cpu);
 return;
 }
 
 CPU_FOREACH(cpu) {
-cpu_breakpoint_remove_all(cpu, BP_GDB);
-#ifndef CONFIG_USER_ONLY
-cpu_watchpoint_remove_all(cpu, BP_GDB);
-#endif
+gdb_cpu_breakpoint_remove_all(cpu);
 }
 }
 
 static void gdb_set_cpu_pc(GDBState *s, target_ulong pc)
 {
@@ -1328,13 +1343,44 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 /* Kill the target */
 error_report("QEMU: Terminated via GDBstub");
 exit(0);
 case 'D':
 /* Detach packet */
-gdb_breakpoint_remove_all();
-gdb_syscall_mode = GDB_SYS_DISABLED;
-gdb_continue(s);
+pid = 1;
+
+if (s->multiprocess) {
+unsigned long lpid;
+if (*p != ';') {
+put_packet(s, "E22");
+break;
+}
+
+if (qemu_strtoul(p + 1, &p, 16, &lpid)) {
+put_packet(s, "E22");
+break;
+}
+
+pid = lpid;
+}
+
+process = gdb_get_process(s, pid);
+gdb_process_breakpoint_remove_all(s, process);
+process->attached = false;
+
+if (pid == gdb_get_cpu_pid(s, s->c_cpu)) {
+s->c_cpu = gdb_first_cpu(s);
+}
+
+if (pid == gdb_get_cpu_pid(s, s->g_cpu)) {
+s->g_cpu = gdb_first_cpu(s);
+}
+
+if (s->c_cpu == NULL) {
+/* No more process attached */
+gdb_syscall_mode = GDB_SYS_DISABLED;
+gdb_continue(s);
+}
 put_packet(s, "OK");
 break;
 case 's':
 if (*p != '\0') {
 addr = strtoull(p, (char **)&p, 16);
-- 
2.19.1

[Qemu-devel] [PATCH v3 15/16] gdbstub: add multiprocess extension support

[Luc Michel]

Add multiprocess extension support by enabling multiprocess mode when
the peer requests it, and by replying that we actually support it in the
qSupported reply packet.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index 09480df2bf..a1b63de34b 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1718,10 +1718,16 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 snprintf(buf, sizeof(buf), "PacketSize=%x", MAX_PACKET_LENGTH);
 cc = CPU_GET_CLASS(first_cpu);
 if (cc->gdb_core_xml_file != NULL) {
 pstrcat(buf, sizeof(buf), ";qXfer:features:read+");
 }
+
+if (strstr(p, "multiprocess+")) {
+s->multiprocess = true;
+}
+pstrcat(buf, sizeof(buf), ";multiprocess+");
+
 put_packet(s, buf);
 break;
 }
 if (strncmp(p, "Xfer:features:read:", 19) == 0) {
 const char *xml;
-- 
2.19.1

Re: [Qemu-devel] [PATCH 1/1] i386: Add PKU/OSPKE on Skylake-Server CPU model

[Eduardo Habkost]

On Fri, Oct 26, 2018 at 01:53:10PM +0800, Tao Xu wrote:
> On 10/25/18 9:28 PM, Eduardo Habkost wrote:
> > On Wed, Oct 17, 2018 at 12:48:58PM +0200, Paolo Bonzini wrote:
> > > On 17/10/2018 11:30, Tao Xu wrote:
> > > > As the release document ref below link (page 13):
> > > > https://software.intel.com/sites/default/files/managed/c5/15/\
> > > > architecture-instruction-set-extensions-programming-reference.pdf
> > > > 
> > > > PKU is supported in Skylake Server (Only Server) and later, and
> > > > on Intel(R) Xeon(R) Processor Scalable Family. OSPKE is to reads
> > > > the value of PKRU (Instruction of PKU) into EAX and clears EDX.
> > > > So PKU/OSPKE are supposed to be in Skylake-Server CPU model.
> > > > And PKU/OSPKE 's CPUID has been exposed to QEMU. But PKU/OSPKE
> > > > can't be find in Skylake-Server CPU model in the code.
> > > > So this patch will fix PKU/OSPKE this issue in Skylake-Server
> > > > CPU model.
> > > OSPKE is not needed, since it is added automatically based on CR4 (and
> > > is not set on boot).
> > Correct.
> > 
> > > Also, the guru of CPU model compatibility is Eduardo, so I'll wait for
> > > him to chime in anyway.
> > Sorry for taking so long to reply.  This can be safely done only
> > if every host that is able to run Skylake-Server today is
> > guaranteed to support PKU.  Is that the case?
> > 
> > You'll also need Skylake-Server-*-cpu.pku=off entries on
> > PC_COMPAT_3_0 to keep PKU disabled on pc-*-3.0 and older.
> > 
> Thank you Eduardo,
> 
> 
> But I can't find PC_COMPAT_3_0 in  include/hw/i386/pc.h. Will it exist on
> 
> QEMU 3.1 and will I add "pku=off" after QEMU 3.1  release?

PC_COMPAT_3_0 was added to qemu.git master a few weeks ago, by
commit 9b4cf107b09d18ac30f46fd1c4de8585ccba030c.

-- 
Eduardo

[Qemu-devel] [PATCH v3 11/16] gdbstub: add support for extended mode packet

[Luc Michel]

Add support for the '!' extended mode packet. This is required for the
multiprocess extension.

Signed-off-by: Luc Michel 
---
 gdbstub.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index 4d8474204f..9c239c1760 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1291,10 +1291,13 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 trace_gdbstub_io_command(line_buf);
 
 p = line_buf;
 ch = *p++;
 switch(ch) {
+case '!':
+put_packet(s, "OK");
+break;
 case '?':
 /* TODO: Make this return the correct value for user-mode.  */
 snprintf(buf, sizeof(buf), "T%02xthread:%s;", GDB_SIGNAL_TRAP,
  gdb_fmt_thread_id(s, s->c_cpu, thread_id, sizeof(thread_id)));
 put_packet(s, buf);
-- 
2.19.1

[Qemu-devel] [PATCH v3 04/16] gdbstub: add multiprocess support to 'H' and 'T' packets

[Luc Michel]

Add a couple of helper functions to cope with GDB threads and processes.

The gdb_get_process() function looks for a process given a pid.

The gdb_get_cpu() function returns the CPU corresponding to the (pid,
tid) pair given as parameters.

The read_thread_id() function parses the thread-id sent by the peer.
This function supports the multiprocess extension thread-id syntax.  The
return value specifies if the parsing failed, or if a special case was
encountered (all processes or all threads).

Use them in 'H' and 'T' packets handling to support the multiprocess
extension.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 152 +++---
 1 file changed, 134 insertions(+), 18 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 9ca743a4c6..c437af7aca 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -677,10 +677,73 @@ out:
 #else
 return s->processes[0].pid;
 #endif
 }
 
+static GDBProcess *gdb_get_process(const GDBState *s, uint32_t pid)
+{
+int i;
+
+if (!pid) {
+/* 0 means any process, we take the first one */
+return &s->processes[0];
+}
+
+for (i = 0; i < s->process_num; i++) {
+if (s->processes[i].pid == pid) {
+return &s->processes[i];
+}
+}
+
+return NULL;
+}
+
+static GDBProcess *gdb_get_cpu_process(const GDBState *s, CPUState *cpu)
+{
+return gdb_get_process(s, gdb_get_cpu_pid(s, cpu));
+}
+
+static CPUState *find_cpu(uint32_t thread_id)
+{
+CPUState *cpu;
+
+CPU_FOREACH(cpu) {
+if (cpu_gdb_index(cpu) == thread_id) {
+return cpu;
+}
+}
+
+return NULL;
+}
+
+static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
+{
+GDBProcess *process;
+CPUState *cpu = find_cpu(tid);
+
+if (!tid) {
+/* 0 means any thread, we take the first one */
+tid = 1;
+}
+
+if (cpu == NULL) {
+return NULL;
+}
+
+process = gdb_get_cpu_process(s, cpu);
+
+if (process->pid != pid) {
+return NULL;
+}
+
+if (!process->attached) {
+return NULL;
+}
+
+return cpu;
+}
+
 static const char *get_feature_xml(const char *p, const char **newp,
CPUClass *cc)
 {
 size_t len;
 int i;
@@ -933,23 +996,10 @@ static void gdb_set_cpu_pc(GDBState *s, target_ulong pc)
 
 cpu_synchronize_state(cpu);
 cpu_set_pc(cpu, pc);
 }
 
-static CPUState *find_cpu(uint32_t thread_id)
-{
-CPUState *cpu;
-
-CPU_FOREACH(cpu) {
-if (cpu_gdb_index(cpu) == thread_id) {
-return cpu;
-}
-}
-
-return NULL;
-}
-
 static char *gdb_fmt_thread_id(const GDBState *s, CPUState *cpu,
char *buf, size_t buf_size)
 {
 if (s->multiprocess) {
 snprintf(buf, buf_size, "p%02x.%02x",
@@ -959,10 +1009,64 @@ static char *gdb_fmt_thread_id(const GDBState *s, 
CPUState *cpu,
 }
 
 return buf;
 }
 
+typedef enum GDBThreadIdKind {
+GDB_ONE_THREAD = 0,
+GDB_ALL_THREADS, /* One process, all threads */
+GDB_ALL_PROCESSES,
+GDB_READ_THREAD_ERR
+} GDBThreadIdKind;
+
+static GDBThreadIdKind read_thread_id(const char *buf, const char **end_buf,
+  uint32_t *pid, uint32_t *tid)
+{
+unsigned long p, t;
+int ret;
+
+if (*buf == 'p') {
+buf++;
+ret = qemu_strtoul(buf, &buf, 16, &p);
+
+if (ret) {
+return GDB_READ_THREAD_ERR;
+}
+
+/* Skip '.' */
+buf++;
+} else {
+p = 1;
+}
+
+ret = qemu_strtoul(buf, &buf, 16, &t);
+
+if (ret) {
+return GDB_READ_THREAD_ERR;
+}
+
+*end_buf = buf;
+
+if (p == -1) {
+return GDB_ALL_PROCESSES;
+}
+
+if (pid) {
+*pid = p;
+}
+
+if (t == -1) {
+return GDB_ALL_THREADS;
+}
+
+if (tid) {
+*tid = t;
+}
+
+return GDB_ONE_THREAD;
+}
+
 static int is_query_packet(const char *p, const char *query, char separator)
 {
 unsigned int query_len = strlen(query);
 
 return strncmp(p, query, query_len) == 0 &&
@@ -1067,16 +1171,18 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 {
 CPUState *cpu;
 CPUClass *cc;
 const char *p;
 uint32_t thread;
+uint32_t pid, tid;
 int ch, reg_size, type, res;
 uint8_t mem_buf[MAX_PACKET_LENGTH];
 char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
 char thread_id[16];
 uint8_t *registers;
 target_ulong addr, len;
+GDBThreadIdKind thread_kind;
 
 trace_gdbstub_io_command(line_buf);
 
 p = line_buf;
 ch = *p++;
@@ -1280,16 +1386,22 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 else
 put_packet(s, "E22");
 break;
 case 'H':
 type = *p++;
-thread = strtoull(p, (char **)&p, 16);
-if (thread == -1 || thread == 0) {
+
+thread_kin

[Qemu-devel] [PATCH v3 16/16] arm/xlnx-zynqmp: put APUs and RPUs in separate CPU clusters

[Luc Michel]

Create two separate CPU clusters for APUs and RPUs.

Signed-off-by: Luc Michel 
---
 include/hw/arm/xlnx-zynqmp.h |  3 +++
 hw/arm/xlnx-zynqmp.c | 23 +++
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index 98f925ab84..591515c760 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -29,10 +29,11 @@
 #include "hw/dma/xlnx_dpdma.h"
 #include "hw/dma/xlnx-zdma.h"
 #include "hw/display/xlnx_dp.h"
 #include "hw/intc/xlnx-zynqmp-ipi.h"
 #include "hw/timer/xlnx-zynqmp-rtc.h"
+#include "hw/cpu/cluster.h"
 
 #define TYPE_XLNX_ZYNQMP "xlnx,zynqmp"
 #define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
TYPE_XLNX_ZYNQMP)
 
@@ -75,10 +76,12 @@
 typedef struct XlnxZynqMPState {
 /*< private >*/
 DeviceState parent_obj;
 
 /*< public >*/
+CPUClusterState apu_cluster;
+CPUClusterState rpu_cluster;
 ARMCPU apu_cpu[XLNX_ZYNQMP_NUM_APU_CPUS];
 ARMCPU rpu_cpu[XLNX_ZYNQMP_NUM_RPU_CPUS];
 GICState gic;
 MemoryRegion gic_mr[XLNX_ZYNQMP_GIC_REGIONS][XLNX_ZYNQMP_GIC_ALIASES];
 
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index c195040350..7cc01f0b77 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -176,16 +176,23 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, 
const char *boot_cpu,
 {
 Error *err = NULL;
 int i;
 int num_rpus = MIN(smp_cpus - XLNX_ZYNQMP_NUM_APU_CPUS, 
XLNX_ZYNQMP_NUM_RPU_CPUS);
 
+object_initialize_child(OBJECT(s), "rpu-cluster", &s->rpu_cluster,
+sizeof(s->rpu_cluster), TYPE_CPU_CLUSTER,
+&error_abort, NULL);
+
+qdev_prop_set_uint32(DEVICE(&s->rpu_cluster), "cluster-id", 1);
+qdev_init_nofail(DEVICE(&s->rpu_cluster));
+
 for (i = 0; i < num_rpus; i++) {
 char *name;
 
 object_initialize(&s->rpu_cpu[i], sizeof(s->rpu_cpu[i]),
   "cortex-r5f-" TYPE_ARM_CPU);
-object_property_add_child(OBJECT(s), "rpu-cpu[*]",
+object_property_add_child(OBJECT(&s->rpu_cluster), "rpu-cpu[*]",
   OBJECT(&s->rpu_cpu[i]), &error_abort);
 
 name = object_get_canonical_path_component(OBJECT(&s->rpu_cpu[i]));
 if (strcmp(name, boot_cpu)) {
 /* Secondary CPUs start in PSCI powered-down state */
@@ -211,14 +218,19 @@ static void xlnx_zynqmp_init(Object *obj)
 {
 XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
 int i;
 int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
 
+object_initialize_child(obj, "apu-cluster", &s->apu_cluster,
+sizeof(s->apu_cluster), TYPE_CPU_CLUSTER,
+&error_abort, NULL);
+
 for (i = 0; i < num_apus; i++) {
-object_initialize_child(obj, "apu-cpu[*]", &s->apu_cpu[i],
-sizeof(s->apu_cpu[i]),
-"cortex-a53-" TYPE_ARM_CPU, &error_abort, 
NULL);
+object_initialize_child(OBJECT(&s->apu_cluster), "apu-cpu[*]",
+&s->apu_cpu[i], sizeof(s->apu_cpu[i]),
+"cortex-a53-" TYPE_ARM_CPU, &error_abort,
+NULL);
 }
 
 sysbus_init_child_obj(obj, "gic", &s->gic, sizeof(s->gic),
   gic_class_name());
 
@@ -331,10 +343,13 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", num_apus);
 qdev_prop_set_bit(DEVICE(&s->gic), "has-security-extensions", s->secure);
 qdev_prop_set_bit(DEVICE(&s->gic),
   "has-virtualization-extensions", s->virt);
 
+qdev_prop_set_uint32(DEVICE(&s->apu_cluster), "cluster-id", 0);
+qdev_init_nofail(DEVICE(&s->apu_cluster));
+
 /* Realize APUs before realizing the GIC. KVM requires this.  */
 for (i = 0; i < num_apus; i++) {
 char *name;
 
 object_property_set_int(OBJECT(&s->apu_cpu[i]), QEMU_PSCI_CONDUIT_SMC,
-- 
2.19.1

[Qemu-devel] [PATCH v3 14/16] gdbstub: gdb_set_stop_cpu: ignore request when process is not attached

[Luc Michel]

When gdb_set_stop_cpu() is called with a CPU associated to a process
currently not attached by the GDB client, return without modifying the
stop CPU. Otherwise, GDB get confused if it receives packets with a
thread-id it does not know about.

Signed-off-by: Luc Michel 
---
 gdbstub.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index b3461eff9e..09480df2bf 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1784,10 +1784,19 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 return RS_IDLE;
 }
 
 void gdb_set_stop_cpu(CPUState *cpu)
 {
+GDBProcess *p = gdb_get_cpu_process(gdbserver_state, cpu);
+
+if (!p->attached) {
+/* Having a stop CPU corresponding to a process that is not attached
+ * confuses GDB. So we ignore the request.
+ */
+return;
+}
+
 gdbserver_state->c_cpu = cpu;
 gdbserver_state->g_cpu = cpu;
 }
 
 #ifndef CONFIG_USER_ONLY
-- 
2.19.1

[Qemu-devel] [PATCH v3 12/16] gdbstub: add support for vAttach packets

[Luc Michel]

Add support for the vAttach packets. In multiprocess mode, GDB sends
them to attach to additional processes.

Signed-off-by: Luc Michel 
---
 gdbstub.c | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index 9c239c1760..e5eddd8e2b 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1337,10 +1337,45 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 break;
 }
 goto unknown_command;
 }
 break;
+} else if (strncmp(p, "Attach;", 7) == 0) {
+unsigned long pid;
+
+p += 7;
+
+if (qemu_strtoul(p, &p, 16, &pid)) {
+put_packet(s, "E22");
+break;
+}
+
+process = gdb_get_process(s, pid);
+
+if (process == NULL) {
+put_packet(s, "E22");
+break;
+}
+
+cpu = get_first_cpu_in_process(s, process);
+
+if (cpu == NULL) {
+/* Refuse to attach an empty process */
+put_packet(s, "E22");
+break;
+}
+
+process->attached = true;
+
+s->g_cpu = cpu;
+s->c_cpu = cpu;
+
+snprintf(buf, sizeof(buf), "T%02xthread:%s;", GDB_SIGNAL_TRAP,
+ gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id)));
+
+put_packet(s, buf);
+break;
 } else {
 goto unknown_command;
 }
 case 'k':
 /* Kill the target */
-- 
2.19.1

[Qemu-devel] [PATCH v3 13/16] gdbstub: processes initialization on new peer connection

[Luc Michel]

When a new connection is established, we set the first process to be
attached, and the others detached. The first CPU of the first process
is selected as the current CPU.

Signed-off-by: Luc Michel 
Reviewed-by: Alistair Francis 
---
 gdbstub.c | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index e5eddd8e2b..b3461eff9e 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2242,13 +2242,14 @@ static bool gdb_accept(void)
 close(fd);
 return false;
 }
 
 s = g_malloc0(sizeof(GDBState));
-s->c_cpu = first_cpu;
-s->g_cpu = first_cpu;
 create_unique_process(s);
+s->processes[0].attached = true;
+s->c_cpu = gdb_first_cpu(s);
+s->g_cpu = s->c_cpu;
 s->fd = fd;
 gdb_has_xml = false;
 
 gdbserver_state = s;
 return true;
@@ -2330,12 +2331,23 @@ static void gdb_chr_receive(void *opaque, const uint8_t 
*buf, int size)
 }
 }
 
 static void gdb_chr_event(void *opaque, int event)
 {
+int i;
+GDBState *s = (GDBState *) opaque;
+
 switch (event) {
 case CHR_EVENT_OPENED:
+/* Start with first process attached, others detached */
+for (i = 0; i < s->process_num; i++) {
+s->processes[i].attached = !i;
+}
+
+s->c_cpu = gdb_first_cpu(s);
+s->g_cpu = s->c_cpu;
+
 vm_stop(RUN_STATE_PAUSED);
 gdb_has_xml = false;
 break;
 default:
 break;
@@ -2509,19 +2521,17 @@ int gdbserver_start(const char *device)
 mon_chr = s->mon_chr;
 cleanup_processes(s);
 memset(s, 0, sizeof(GDBState));
 s->mon_chr = mon_chr;
 }
-s->c_cpu = first_cpu;
-s->g_cpu = first_cpu;
 
 create_processes(s);
 
 if (chr) {
 qemu_chr_fe_init(&s->chr, chr, &error_abort);
 qemu_chr_fe_set_handlers(&s->chr, gdb_chr_can_receive, gdb_chr_receive,
- gdb_chr_event, NULL, NULL, NULL, true);
+ gdb_chr_event, NULL, s, NULL, true);
 }
 s->state = chr ? RS_IDLE : RS_INACTIVE;
 s->mon_chr = mon_chr;
 s->current_syscall_cb = NULL;
 
-- 
2.19.1

[Qemu-devel] [PATCH v3 02/16] gdbstub: introduce GDB processes

[Luc Michel]

Add a structure GDBProcess that represent processes from the GDB
semantic point of view.

CPUs can be split into different processes, by grouping them under
different cpu-cluster objects.  Each occurrence of a cpu-cluster object
implies the existence of the corresponding process in the GDB stub. The
GDB process ID is derived from the corresponding cluster ID as follows:

  GDB PID = cluster ID + 1

This is because PIDs -1 and 0 are reserved in GDB and cannot be used by
processes.

When no such container are found, all the CPUs are put in a unique GDB
process (create_unique_process()). This is also the case when compiled
in user mode, where multi-processes do not make much sense for now.

Signed-off-by: Luc Michel 
---
 gdbstub.c | 75 +++
 1 file changed, 75 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index c4e4f9f082..096ac8e99c 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -27,10 +27,11 @@
 #include "monitor/monitor.h"
 #include "chardev/char.h"
 #include "chardev/char-fe.h"
 #include "sysemu/sysemu.h"
 #include "exec/gdbstub.h"
+#include "hw/cpu/cluster.h"
 #endif
 
 #define MAX_PACKET_LENGTH 4096
 
 #include "qemu/sockets.h"
@@ -294,10 +295,15 @@ typedef struct GDBRegisterState {
 gdb_reg_cb set_reg;
 const char *xml;
 struct GDBRegisterState *next;
 } GDBRegisterState;
 
+typedef struct GDBProcess {
+uint32_t pid;
+bool attached;
+} GDBProcess;
+
 enum RSState {
 RS_INACTIVE,
 RS_IDLE,
 RS_GETLINE,
 RS_GETLINE_ESC,
@@ -322,10 +328,13 @@ typedef struct GDBState {
 int running_state;
 #else
 CharBackend chr;
 Chardev *mon_chr;
 #endif
+bool multiprocess;
+GDBProcess *processes;
+int process_num;
 char syscall_buf[256];
 gdb_syscall_complete_cb current_syscall_cb;
 } GDBState;
 
 /* By default use no IRQs and no timers while single stepping so as to
@@ -1749,10 +1758,24 @@ void gdb_exit(CPUArchState *env, int code)
 #ifndef CONFIG_USER_ONLY
   qemu_chr_fe_deinit(&s->chr, true);
 #endif
 }
 
+/*
+ * Create a unique process containing all the CPUs.
+ */
+static void create_unique_process(GDBState *s)
+{
+GDBProcess *process;
+
+s->processes = g_malloc0(sizeof(GDBProcess));
+s->process_num = 1;
+process = &s->processes[0];
+
+process->pid = 1;
+}
+
 #ifdef CONFIG_USER_ONLY
 int
 gdb_handlesig(CPUState *cpu, int sig)
 {
 GDBState *s;
@@ -1846,10 +1869,11 @@ static bool gdb_accept(void)
 }
 
 s = g_malloc0(sizeof(GDBState));
 s->c_cpu = first_cpu;
 s->g_cpu = first_cpu;
+create_unique_process(s);
 s->fd = fd;
 gdb_has_xml = false;
 
 gdbserver_state = s;
 return true;
@@ -2002,10 +2026,57 @@ static const TypeInfo char_gdb_type_info = {
 .name = TYPE_CHARDEV_GDB,
 .parent = TYPE_CHARDEV,
 .class_init = char_gdb_class_init,
 };
 
+static int find_cpu_clusters(Object *child, void *opaque)
+{
+if (object_dynamic_cast(child, TYPE_CPU_CLUSTER)) {
+GDBState *s = (GDBState *) opaque;
+CPUClusterState *cluster = CPU_CLUSTER(child);
+
+s->processes = g_renew(GDBProcess, s->processes, ++s->process_num);
+
+GDBProcess *process = &s->processes[s->process_num - 1];
+
+/* GDB process IDs -1 and 0 are reserved */
+process->pid = cluster->cluster_id + 1;
+process->attached = false;
+return 0;
+}
+
+return object_child_foreach(child, find_cpu_clusters, opaque);
+}
+
+static int pid_order(const void *a, const void *b)
+{
+GDBProcess *pa = (GDBProcess *) a;
+GDBProcess *pb = (GDBProcess *) b;
+
+return pa->pid - pb->pid;
+}
+
+static void create_processes(GDBState *s)
+{
+object_child_foreach(object_get_root(), find_cpu_clusters, s);
+
+if (!s->processes) {
+/* No CPU cluster specified by the machine */
+create_unique_process(s);
+} else {
+/* Sort by PID */
+qsort(s->processes, s->process_num, sizeof(s->processes[0]), 
pid_order);
+}
+}
+
+static void cleanup_processes(GDBState *s)
+{
+g_free(s->processes);
+s->process_num = 0;
+s->processes = NULL;
+}
+
 int gdbserver_start(const char *device)
 {
 trace_gdbstub_op_start(device);
 
 GDBState *s;
@@ -2058,15 +2129,19 @@ int gdbserver_start(const char *device)
NULL, &error_abort);
 monitor_init(mon_chr, 0);
 } else {
 qemu_chr_fe_deinit(&s->chr, true);
 mon_chr = s->mon_chr;
+cleanup_processes(s);
 memset(s, 0, sizeof(GDBState));
 s->mon_chr = mon_chr;
 }
 s->c_cpu = first_cpu;
 s->g_cpu = first_cpu;
+
+create_processes(s);
+
 if (chr) {
 qemu_chr_fe_init(&s->chr, chr, &error_abort);
 qemu_chr_fe_set_handlers(&s->chr, gdb_chr_can_receive, gdb_chr_receive,
  gdb_chr_event, NULL, NULL, NULL, true);
 }
-- 
2.19.1

[Qemu-devel] [PATCH v3 08/16] gdbstub: add multiprocess support to Xfer:features:read:

[Luc Michel]

Change the Xfer:features:read: packet handling to support the
multiprocess extension. This packet is used to request the XML
description of the CPU. In multiprocess mode, different descriptions can
be sent for different processes.

This function now takes the process to send the description for as a
parameter, and use a buffer in the process structure to store the
generated description.

It takes the first CPU of the process to generate the description.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 47 +++
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index b8aa4b34af..b7079eff4a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -298,10 +298,12 @@ typedef struct GDBRegisterState {
 } GDBRegisterState;
 
 typedef struct GDBProcess {
 uint32_t pid;
 bool attached;
+
+char target_xml[1024];
 } GDBProcess;
 
 enum RSState {
 RS_INACTIVE,
 RS_IDLE,
@@ -801,55 +803,57 @@ static CPUState *gdb_first_cpu(const GDBState *s)
 }
 
 return cpu;
 }
 
-static const char *get_feature_xml(const char *p, const char **newp,
-   CPUClass *cc)
+static const char *get_feature_xml(const GDBState *s, const char *p,
+   const char **newp, GDBProcess *process)
 {
 size_t len;
 int i;
 const char *name;
-static char target_xml[1024];
+CPUState *cpu = get_first_cpu_in_process(s, process);
+CPUClass *cc = CPU_GET_CLASS(cpu);
 
 len = 0;
 while (p[len] && p[len] != ':')
 len++;
 *newp = p + len;
 
 name = NULL;
 if (strncmp(p, "target.xml", len) == 0) {
+char *buf = process->target_xml;
+const size_t buf_sz = sizeof(process->target_xml);
+
 /* Generate the XML description for this CPU.  */
-if (!target_xml[0]) {
+if (!buf[0]) {
 GDBRegisterState *r;
-CPUState *cpu = first_cpu;
 
-pstrcat(target_xml, sizeof(target_xml),
+pstrcat(buf, buf_sz,
 ""
 ""
 "");
 if (cc->gdb_arch_name) {
 gchar *arch = cc->gdb_arch_name(cpu);
-pstrcat(target_xml, sizeof(target_xml), "");
-pstrcat(target_xml, sizeof(target_xml), arch);
-pstrcat(target_xml, sizeof(target_xml), "");
+pstrcat(buf, buf_sz, "");
+pstrcat(buf, buf_sz, arch);
+pstrcat(buf, buf_sz, "");
 g_free(arch);
 }
-pstrcat(target_xml, sizeof(target_xml), "gdb_core_xml_file);
-pstrcat(target_xml, sizeof(target_xml), "\"/>");
+pstrcat(buf, buf_sz, "gdb_core_xml_file);
+pstrcat(buf, buf_sz, "\"/>");
 for (r = cpu->gdb_regs; r; r = r->next) {
-pstrcat(target_xml, sizeof(target_xml), "xml);
-pstrcat(target_xml, sizeof(target_xml), "\"/>");
+pstrcat(buf, buf_sz, "xml);
+pstrcat(buf, buf_sz, "\"/>");
 }
-pstrcat(target_xml, sizeof(target_xml), "");
+pstrcat(buf, buf_sz, "");
 }
-return target_xml;
+return buf;
 }
 if (cc->gdb_get_dynamic_xml) {
-CPUState *cpu = first_cpu;
 char *xmlname = g_strndup(p, len);
 const char *xml = cc->gdb_get_dynamic_xml(cpu, xmlname);
 
 g_free(xmlname);
 if (xml) {
@@ -1255,10 +1259,11 @@ out:
 }
 
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
 CPUState *cpu;
+GDBProcess *process;
 CPUClass *cc;
 const char *p;
 uint32_t pid, tid;
 int ch, reg_size, type, res;
 uint8_t mem_buf[MAX_PACKET_LENGTH];
@@ -1636,18 +1641,19 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 }
 if (strncmp(p, "Xfer:features:read:", 19) == 0) {
 const char *xml;
 target_ulong total_len;
 
-cc = CPU_GET_CLASS(first_cpu);
+process = gdb_get_cpu_process(s, s->g_cpu);
+cc = CPU_GET_CLASS(s->g_cpu);
 if (cc->gdb_core_xml_file == NULL) {
 goto unknown_command;
 }
 
 gdb_has_xml = true;
 p += 19;
-xml = get_feature_xml(p, &p, cc);
+xml = get_feature_xml(s, p, &p, process);
 if (!xml) {
 snprintf(buf, sizeof(buf), "E00");
 put_packet(s, buf);
 break;
 }
@@ -2315,10 +2321,11 @@ static int find_cpu_clusters(Object *child, void 
*opaque)
 GDBProcess *process = &s->processes[s->process_num - 1];
 
 /* GDB process IDs -1 and 0 are reserved */
 process->pid = cluster->cluster_id + 1;
 process->attached = false;
+process->target_xml[0] = '\0';
 return 0;
 }
 
 return object_child_foreac

[Qemu-devel] [PATCH v3 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo

[Luc Michel]

Change the thread info related packets handling to support multiprocess
extension.

Add the CPUs class name in the extra info to help differentiate
them in multiprocess mode.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 35 +--
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 17fec8a41c..b8aa4b34af 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1257,11 +1257,10 @@ out:
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
 CPUState *cpu;
 CPUClass *cc;
 const char *p;
-uint32_t thread;
 uint32_t pid, tid;
 int ch, reg_size, type, res;
 uint8_t mem_buf[MAX_PACKET_LENGTH];
 char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
 char thread_id[16];
@@ -1553,30 +1552,46 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 snprintf(buf, sizeof(buf), "QC%s",
  gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id)));
 put_packet(s, buf);
 break;
 } else if (strcmp(p,"fThreadInfo") == 0) {
-s->query_cpu = first_cpu;
+s->query_cpu = gdb_first_cpu(s);
 goto report_cpuinfo;
 } else if (strcmp(p,"sThreadInfo") == 0) {
 report_cpuinfo:
 if (s->query_cpu) {
-snprintf(buf, sizeof(buf), "m%x", cpu_gdb_index(s->query_cpu));
+snprintf(buf, sizeof(buf), "m%s",
+ gdb_fmt_thread_id(s, s->query_cpu,
+   thread_id, sizeof(thread_id)));
 put_packet(s, buf);
-s->query_cpu = CPU_NEXT(s->query_cpu);
+s->query_cpu = gdb_next_cpu(s, s->query_cpu);
 } else
 put_packet(s, "l");
 break;
 } else if (strncmp(p,"ThreadExtraInfo,", 16) == 0) {
-thread = strtoull(p+16, (char **)&p, 16);
-cpu = find_cpu(thread);
+if (read_thread_id(p + 16, &p, &pid, &tid) == GDB_READ_THREAD_ERR) 
{
+put_packet(s, "E22");
+break;
+}
+cpu = gdb_get_cpu(s, pid, tid);
 if (cpu != NULL) {
 cpu_synchronize_state(cpu);
-/* memtohex() doubles the required space */
-len = snprintf((char *)mem_buf, sizeof(buf) / 2,
-   "CPU#%d [%s]", cpu->cpu_index,
-   cpu->halted ? "halted " : "running");
+
+if (s->multiprocess && (s->process_num > 1)) {
+/* Print the CPU model in multiprocess mode */
+ObjectClass *oc = object_get_class(OBJECT(cpu));
+const char *cpu_model = object_class_get_name(oc);
+len = snprintf((char *)mem_buf, sizeof(buf) / 2,
+   "CPU#%d %s [%s]", cpu->cpu_index,
+   cpu_model,
+   cpu->halted ? "halted " : "running");
+} else {
+/* memtohex() doubles the required space */
+len = snprintf((char *)mem_buf, sizeof(buf) / 2,
+   "CPU#%d [%s]", cpu->cpu_index,
+   cpu->halted ? "halted " : "running");
+}
 trace_gdbstub_op_extra_info((char *)mem_buf);
 memtohex(buf, mem_buf, len);
 put_packet(s, buf);
 }
 break;
-- 
2.19.1

[Qemu-devel] [PATCH v3 05/16] gdbstub: add multiprocess support to vCont packets

[Luc Michel]

Add the gdb_first_cpu() and gdb_next_cpu() to iterate over all
the CPUs in currently attached processes.

Add the gdb_first_cpu_in_process() and gdb_next_cpu_in_process() to
iterate over CPUs of a given process.

Use them to add multiprocess extension support to vCont packets.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 117 +++---
 1 file changed, 102 insertions(+), 15 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index c437af7aca..d1ec245d1d 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -713,10 +713,40 @@ static CPUState *find_cpu(uint32_t thread_id)
 }
 
 return NULL;
 }
 
+static CPUState *get_first_cpu_in_process(const GDBState *s,
+  GDBProcess *process)
+{
+CPUState *cpu;
+
+CPU_FOREACH(cpu) {
+if (gdb_get_cpu_pid(s, cpu) == process->pid) {
+return cpu;
+}
+}
+
+return NULL;
+}
+
+static CPUState *gdb_next_cpu_in_process(const GDBState *s, CPUState *cpu)
+{
+uint32_t pid = gdb_get_cpu_pid(s, cpu);
+cpu = CPU_NEXT(cpu);
+
+while (cpu) {
+if (gdb_get_cpu_pid(s, cpu) == pid) {
+break;
+}
+
+cpu = CPU_NEXT(cpu);
+}
+
+return cpu;
+}
+
 static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
 {
 GDBProcess *process;
 CPUState *cpu = find_cpu(tid);
 
@@ -740,10 +770,41 @@ static CPUState *gdb_get_cpu(const GDBState *s, uint32_t 
pid, uint32_t tid)
 }
 
 return cpu;
 }
 
+/* Return the cpu following @cpu, while ignoring
+ * unattached processes.
+ */
+static CPUState *gdb_next_cpu(const GDBState *s, CPUState *cpu)
+{
+cpu = CPU_NEXT(cpu);
+
+while (cpu) {
+if (gdb_get_cpu_process(s, cpu)->attached) {
+break;
+}
+
+cpu = CPU_NEXT(cpu);
+}
+
+return cpu;
+}
+
+/* Return the first attached cpu */
+static CPUState *gdb_first_cpu(const GDBState *s)
+{
+CPUState *cpu = first_cpu;
+GDBProcess *process = gdb_get_cpu_process(s, cpu);
+
+if (!process->attached) {
+return gdb_next_cpu(s, cpu);
+}
+
+return cpu;
+}
+
 static const char *get_feature_xml(const char *p, const char **newp,
CPUClass *cc)
 {
 size_t len;
 int i;
@@ -1078,14 +1139,16 @@ static int is_query_packet(const char *p, const char 
*query, char separator)
  * returns -ENOTSUP if a command is unsupported, -EINVAL or -ERANGE if there is
  * a format error, 0 on success.
  */
 static int gdb_handle_vcont(GDBState *s, const char *p)
 {
-int res, idx, signal = 0;
+int res, signal = 0;
 char cur_action;
 char *newstates;
 unsigned long tmp;
+uint32_t pid, tid;
+GDBProcess *process;
 CPUState *cpu;
 #ifdef CONFIG_USER_ONLY
 int max_cpus = 1; /* global variable max_cpus exists only in system mode */
 
 CPU_FOREACH(cpu) {
@@ -1124,29 +1187,52 @@ static int gdb_handle_vcont(GDBState *s, const char *p)
 } else if (cur_action != 'c' && cur_action != 's') {
 /* unknown/invalid/unsupported command */
 res = -ENOTSUP;
 goto out;
 }
-/* thread specification. special values: (none), -1 = all; 0 = any */
-if ((p[0] == ':' && p[1] == '-' && p[2] == '1') || (p[0] != ':')) {
-if (*p == ':') {
-p += 3;
-}
-for (idx = 0; idx < max_cpus; idx++) {
-if (newstates[idx] == 1) {
-newstates[idx] = cur_action;
+
+if (*p++ != ':') {
+res = -ENOTSUP;
+goto out;
+}
+
+switch (read_thread_id(p, &p, &pid, &tid)) {
+case GDB_READ_THREAD_ERR:
+res = -EINVAL;
+goto out;
+
+case GDB_ALL_PROCESSES:
+cpu = gdb_first_cpu(s);
+while (cpu) {
+if (newstates[cpu->cpu_index] == 1) {
+newstates[cpu->cpu_index] = cur_action;
 }
+
+cpu = gdb_next_cpu(s, cpu);
 }
-} else if (*p == ':') {
-p++;
-res = qemu_strtoul(p, &p, 16, &tmp);
-if (res) {
+break;
+
+case GDB_ALL_THREADS:
+process = gdb_get_process(s, pid);
+
+if (!process->attached) {
+res = -EINVAL;
 goto out;
 }
 
-/* 0 means any thread, so we pick the first valid CPU */
-cpu = tmp ? find_cpu(tmp) : first_cpu;
+cpu = get_first_cpu_in_process(s, process);
+while (cpu) {
+if (newstates[cpu->cpu_index] == 1) {
+newstates[cpu->cpu_index] = cur_action;
+}
+
+cpu = gdb_next_cpu_in_process(s, cpu);
+}
+break;
+
+case GDB_ONE_THREAD:
+cpu = gdb_get_cpu(s, pid, tid);
 
 /* invalid CP

[Qemu-devel] [PATCH v3 01/16] hw/cpu: introduce CPU clusters

[Luc Michel]

This commit adds the cpu-cluster type. It aims at gathering CPUs from
the same cluster in a machine.

For now it only has a `cluster-id` property.

Signed-off-by: Luc Michel 
---
 include/hw/cpu/cluster.h | 38 +++
 hw/cpu/cluster.c | 49 
 hw/cpu/Makefile.objs |  2 +-
 3 files changed, 88 insertions(+), 1 deletion(-)
 create mode 100644 include/hw/cpu/cluster.h
 create mode 100644 hw/cpu/cluster.c

diff --git a/include/hw/cpu/cluster.h b/include/hw/cpu/cluster.h
new file mode 100644
index 00..f233a47a4a
--- /dev/null
+++ b/include/hw/cpu/cluster.h
@@ -0,0 +1,38 @@
+/*
+ * QEMU CPU cluster
+ *
+ * Copyright (c) 2018 GreenSocs SAS
+ *
+ * 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
+ * 
+ */
+#ifndef QEMU_HW_CPU_CLUSTER_H
+#define QEMU_HW_CPU_CLUSTER_H
+
+#include "qemu/osdep.h"
+#include "hw/qdev.h"
+
+#define TYPE_CPU_CLUSTER "cpu-cluster"
+#define CPU_CLUSTER(obj) \
+OBJECT_CHECK(CPUClusterState, (obj), TYPE_CPU_CLUSTER)
+
+typedef struct CPUClusterState {
+/*< private >*/
+DeviceState parent_obj;
+
+/*< public >*/
+uint32_t cluster_id;
+} CPUClusterState;
+
+#endif
diff --git a/hw/cpu/cluster.c b/hw/cpu/cluster.c
new file mode 100644
index 00..11121e6f26
--- /dev/null
+++ b/hw/cpu/cluster.c
@@ -0,0 +1,49 @@
+/*
+ * QEMU CPU cluster
+ *
+ * Copyright (c) 2018 GreenSocs SAS
+ *
+ * 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 "hw/cpu/cluster.h"
+#include "qapi/error.h"
+#include "qemu/module.h"
+
+static Property cpu_cluster_properties[] = {
+DEFINE_PROP_UINT32("cluster-id", CPUClusterState, cluster_id, 0),
+DEFINE_PROP_END_OF_LIST()
+};
+
+static void cpu_cluster_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+
+dc->props = cpu_cluster_properties;
+}
+
+static const TypeInfo cpu_cluster_type_info = {
+.name = TYPE_CPU_CLUSTER,
+.parent = TYPE_DEVICE,
+.instance_size = sizeof(CPUClusterState),
+.class_init = cpu_cluster_class_init,
+};
+
+static void cpu_cluster_register_types(void)
+{
+type_register_static(&cpu_cluster_type_info);
+}
+
+type_init(cpu_cluster_register_types)
diff --git a/hw/cpu/Makefile.objs b/hw/cpu/Makefile.objs
index cd52d20b65..8db9e8a7b3 100644
--- a/hw/cpu/Makefile.objs
+++ b/hw/cpu/Makefile.objs
@@ -1,5 +1,5 @@
 obj-$(CONFIG_ARM11MPCORE) += arm11mpcore.o
 obj-$(CONFIG_REALVIEW) += realview_mpcore.o
 obj-$(CONFIG_A9MPCORE) += a9mpcore.o
 obj-$(CONFIG_A15MPCORE) += a15mpcore.o
-common-obj-y += core.o
+common-obj-y += core.o cluster.o
-- 
2.19.1

[Qemu-devel] [PATCH v3 00/16] gdbstub: support for the multiprocess extension

[Luc Michel]

changes since v2:
  - patch 1introducing the cpu-cluster type. I didn't opt for an
   Interface, but I can add one if you think it's necessary.
   For now this class inherits from Device and has a
   cluster-id property, used by the GDB stub to compute a
   PID.

  - patch 2removed GDB group related code as it has been replaced
   with CPU clusters

  - patch 2/8  moved GDBProcess target_xml field introduction into patch
   8 [Philippe]

  - patch 3gdb_get_cpu_pid() now search for CPU being a child of a
   cpu-cluster object. Use the cluster-id to compute the PID.

  - patch 4gdb_get_process() does not rely on s->processes array
   indices anymore as PIDs can now be sparse. Instead, iterate
   over the array to find the process.

  - patch 3/4  removed Reviewed-by tags because of substantial changes.

  - patch 4/7  read_thread_id() hardening [Philippe]

  - patch 12   safer vAttach packet parsing [Phillipe]

  - patch 16   put APUs and RPUs in different clusters instead of GDB
   groups

changes since v1:
  - rename qemu_get_thread_id() to gdb_fmt_thread_id() [Philippe]
  - check qemu_strtoul() return value for 'D' packets [Philippe]


This series adds support for the multiprocess extension of the GDB
remote protocol in the QEMU GDB stub.

This extension is useful to split QEMU emulated CPUs in different
processes from the point of view of the GDB client. It adds the
possibility to debug different kind of processors (e.g. an AArch64 and
an ARMv7 CPU) at the same time (it is not possible otherwise since GDB
expects an SMP view at the thread granularity.

CPUs are grouped using specially named QOM containers. CPUs that are
children of such a container are grouped under the same GDB process.

The last patch groups the CPUs of different model in the zynqmp machines
into separate processes.

To test this patchset, you can use the following commands:

(Note that this requires a recent enough GDB, I think GDB 7.2 is OK.
Also, it must be compiled to support both ARM and AArch64 architectures)

Run QEMU: (-smp 6 in xlnx-zcu102 enables both cortex-a53 and cortex-r5
CPUs)

qemu-system-aarch64 -M xlnx-zcu102 -gdb tcp::1234 -S -smp 6

Run the following commands in GDB:

target extended :1234
add-inferior
inferior 2
attach 2
info threads

I want to thanks the Xilinx's QEMU team who sponsored this work for
their collaboration and their prototype implementation.


Luc Michel (16):
  hw/cpu: introduce CPU clusters
  gdbstub: introduce GDB processes
  gdbstub: add multiprocess support to '?' packets
  gdbstub: add multiprocess support to 'H' and 'T' packets
  gdbstub: add multiprocess support to vCont packets
  gdbstub: add multiprocess support to 'sC' packets
  gdbstub: add multiprocess support to (f|s)ThreadInfo and
ThreadExtraInfo
  gdbstub: add multiprocess support to Xfer:features:read:
  gdbstub: add multiprocess support to gdb_vm_state_change()
  gdbstub: add multiprocess support to 'D' packets
  gdbstub: add support for extended mode packet
  gdbstub: add support for vAttach packets
  gdbstub: processes initialization on new peer connection
  gdbstub: gdb_set_stop_cpu: ignore request when process is not attached
  gdbstub: add multiprocess extension support
  arm/xlnx-zynqmp: put APUs and RPUs in separate GDB groups

 include/hw/arm/xlnx-zynqmp.h |   3 +
 include/hw/cpu/cluster.h |  38 +++
 gdbstub.c| 628 ++-
 hw/arm/xlnx-zynqmp.c |  23 +-
 hw/cpu/cluster.c |  49 +++
 hw/cpu/Makefile.objs |   2 +-
 6 files changed, 662 insertions(+), 81 deletions(-)
 create mode 100644 include/hw/cpu/cluster.h
 create mode 100644 hw/cpu/cluster.c

-- 
2.19.1

[Qemu-devel] [PATCH v3 06/16] gdbstub: add multiprocess support to 'sC' packets

[Luc Michel]

Change the sC packet handling to support the multiprocess extension.
Instead of returning the first thread, we return the first thread of the
current process.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 gdbstub.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index d1ec245d1d..17fec8a41c 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1543,13 +1543,18 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 type = strtoul(p, (char **)&p, 16);
 sstep_flags = type;
 put_packet(s, "OK");
 break;
 } else if (strcmp(p,"C") == 0) {
-/* "Current thread" remains vague in the spec, so always return
- *  the first CPU (gdb returns the first thread). */
-put_packet(s, "QC1");
+/* "Current thread" remains vague in the spec, so always return the
+ * first thread of the current process (gdb returns the first
+ * thread).
+ */
+cpu = get_first_cpu_in_process(s, gdb_get_cpu_process(s, 
s->g_cpu));
+snprintf(buf, sizeof(buf), "QC%s",
+ gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id)));
+put_packet(s, buf);
 break;
 } else if (strcmp(p,"fThreadInfo") == 0) {
 s->query_cpu = first_cpu;
 goto report_cpuinfo;
 } else if (strcmp(p,"sThreadInfo") == 0) {
-- 
2.19.1

Re: [Qemu-devel] [PULL v2 05/43] hw/timer/sun4v-rtc: Convert from DPRINTF() macro to trace events

[Eduardo Habkost]

On Thu, Oct 25, 2018 at 06:17:59PM +0100, David Gibson wrote:
> On Thu, Oct 25, 2018 at 10:32:23AM -0300, Eduardo Habkost wrote:
> > From: Philippe Mathieu-Daudé 
> > 
> > Signed-off-by: Philippe Mathieu-Daudé 
> > Reviewed-by: Artyom Tarasenko 
> > Reviewed-by: Cédric Le Goater 
> > Message-Id: <20181002212522.23303-3-f4...@amsat.org>
> > Signed-off-by: Eduardo Habkost 
> > ---
> >  hw/timer/sun4v-rtc.c  | 13 +++--
> >  hw/timer/trace-events |  4 
> >  2 files changed, 7 insertions(+), 10 deletions(-)
> > 
> > diff --git a/hw/timer/sun4v-rtc.c b/hw/timer/sun4v-rtc.c
> > index 310523225f..13be94f8da 100644
> > --- a/hw/timer/sun4v-rtc.c
> > +++ b/hw/timer/sun4v-rtc.c
> > @@ -14,15 +14,8 @@
> >  #include "hw/sysbus.h"
> >  #include "qemu/timer.h"
> >  #include "hw/timer/sun4v-rtc.h"
> > +#include "trace.h"
> >  
> > -//#define DEBUG_SUN4V_RTC
> > -
> > -#ifdef DEBUG_SUN4V_RTC
> > -#define DPRINTF(fmt, ...)   \
> > -do { printf("sun4v_rtc: " fmt , ## __VA_ARGS__); } while (0)
> > -#else
> > -#define DPRINTF(fmt, ...) do {} while (0)
> > -#endif
> >  
> >  #define TYPE_SUN4V_RTC "sun4v_rtc"
> >  #define SUN4V_RTC(obj) OBJECT_CHECK(Sun4vRtc, (obj), TYPE_SUN4V_RTC)
> > @@ -41,14 +34,14 @@ static uint64_t sun4v_rtc_read(void *opaque, hwaddr 
> > addr,
> >  /* accessing the high 32 bits */
> >  val >>= 32;
> >  }
> > -DPRINTF("read from " TARGET_FMT_plx " val %lx\n", addr, val);
> > +trace_sun4v_rtc_read(addr, val);
> >  return val;
> >  }
> >  
> >  static void sun4v_rtc_write(void *opaque, hwaddr addr,
> >   uint64_t val, unsigned size)
> >  {
> > -DPRINTF("write 0x%x to " TARGET_FMT_plx "\n", (unsigned)val, addr);
> > +trace_sun4v_rtc_read(addr, val);
> 
> Uh.. as in v1, it looks like this should be trace_sun4v_rtc_write().

Oops, my bad.  I can fix this manually in case there's a v3 pull
request, but I would really prefer that to be included in a
follow up patch instead of making this block the entire pull
request.

-- 
Eduardo

[Qemu-devel] [PATCH v3 09/16] gdbstub: add multiprocess support to gdb_vm_state_change()

[Luc Michel]

Add support for multiprocess extension in gdb_vm_state_change()
function.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index b7079eff4a..39b1766f28 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1710,10 +1710,11 @@ void gdb_set_stop_cpu(CPUState *cpu)
 static void gdb_vm_state_change(void *opaque, int running, RunState state)
 {
 GDBState *s = gdbserver_state;
 CPUState *cpu = s->c_cpu;
 char buf[256];
+char thread_id[16];
 const char *type;
 int ret;
 
 if (running || s->state == RS_INACTIVE) {
 return;
@@ -1721,10 +1722,18 @@ static void gdb_vm_state_change(void *opaque, int 
running, RunState state)
 /* Is there a GDB syscall waiting to be sent?  */
 if (s->current_syscall_cb) {
 put_packet(s, s->syscall_buf);
 return;
 }
+
+if (cpu == NULL) {
+/* No process attached */
+return;
+}
+
+gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id));
+
 switch (state) {
 case RUN_STATE_DEBUG:
 if (cpu->watchpoint_hit) {
 switch (cpu->watchpoint_hit->flags & BP_MEM_ACCESS) {
 case BP_MEM_READ:
@@ -1738,12 +1747,12 @@ static void gdb_vm_state_change(void *opaque, int 
running, RunState state)
 break;
 }
 trace_gdbstub_hit_watchpoint(type, cpu_gdb_index(cpu),
 (target_ulong)cpu->watchpoint_hit->vaddr);
 snprintf(buf, sizeof(buf),
- "T%02xthread:%02x;%swatch:" TARGET_FMT_lx ";",
- GDB_SIGNAL_TRAP, cpu_gdb_index(cpu), type,
+ "T%02xthread:%s;%swatch:" TARGET_FMT_lx ";",
+ GDB_SIGNAL_TRAP, thread_id, type,
  (target_ulong)cpu->watchpoint_hit->vaddr);
 cpu->watchpoint_hit = NULL;
 goto send_packet;
 } else {
 trace_gdbstub_hit_break();
@@ -1781,11 +1790,11 @@ static void gdb_vm_state_change(void *opaque, int 
running, RunState state)
 trace_gdbstub_hit_unknown(state);
 ret = GDB_SIGNAL_UNKNOWN;
 break;
 }
 gdb_set_stop_cpu(cpu);
-snprintf(buf, sizeof(buf), "T%02xthread:%02x;", ret, cpu_gdb_index(cpu));
+snprintf(buf, sizeof(buf), "T%02xthread:%s;", ret, thread_id);
 
 send_packet:
 put_packet(s, buf);
 
 /* disable single step if it was enabled */
-- 
2.19.1

Re: [Qemu-devel] [PATCH v5 3/3] x86: define a new MSR based feature word -- FEATURE_WORDS_ARCH_CAPABILITIES

[Eduardo Habkost]

On Fri, Oct 26, 2018 at 11:01:25AM +0800, Robert Hoo wrote:
> On Wed, 2018-10-24 at 07:06 -0300, Eduardo Habkost wrote:
> > On Mon, Oct 15, 2018 at 12:47:25PM +0800, Robert Hoo wrote:
> > > Note RSBA is specially treated -- no matter host support it or not,
> > > qemu
> > > pretends it is supported.
> > > 
> > > Signed-off-by: Robert Hoo 
> > 
> > I am now wondering what else we need to be able to remove
> > CPUID_7_0_EDX_ARCH_CAPABILITIES from
> > feature_word_info[FEAT_7_0_EDX].unmigratable_flags.
> > 
> > This series is necessary for that, be I think we still can't let
> > the VM be migrated if arch-capabilities is enabled and we're
> > running on a host that doesn't have MSR_IA32_ARCH_CAPABILITIES on
> > kvm_feature_msrs.
> > 
> > Reviewed-by: Eduardo Habkost 
> > 
> > > ---
> > >  target/i386/cpu.c | 31 ++-
> > >  target/i386/cpu.h |  8 
> > >  target/i386/kvm.c | 11 +++
> > >  3 files changed, 49 insertions(+), 1 deletion(-)
> > > 
> [...]
> > >  
> > >  typedef struct X86RegisterInfo32 {
> > > @@ -3696,7 +3717,15 @@ static uint32_t
> > > x86_cpu_get_supported_feature_word(FeatureWord w,
> > >  wi-
> > > >cpuid.reg);
> > >  break;
> > >  case MSR_FEATURE_WORD:
> > > -r = kvm_arch_get_supported_msr_feature(kvm_state, wi-
> > > >msr.index);
> > > +/* Special case:
> > > + * No matter host status, IA32_ARCH_CAPABILITIES.RSBA
> > > [bit 2]
> > > + * is always supported in guest.
> > > + */
> > > +if (wi->msr.index == MSR_IA32_ARCH_CAPABILITIES) {
> > > +r = MSR_ARCH_CAP_RSBA;
> > > +}
> > > +r |= kvm_arch_get_supported_msr_feature(kvm_state,
> > > +wi->msr.index);
> > >  break;
> After I add the filtering out MSR feature, whose CPUID dependency fails
> , in x86_cpu_filter_features(), 1 issue comes out here: 
> 
> If running on an old platform that doesn't have ARCH_CAPABILITIES MSR,
> but we still pretends it here, then qemu will always print out
> "warning: host doesn't support requested feature: MSR(10AH).rsba [bit
> 2]", with -cpu 'host', which does not look comfortable.
> How about remove this hunk for now? leave it to when we fully decide
> how to handle ARCH_CAPABILITIES live-migration safely. 

I will remove that hunk in x86-next, thanks for noting!

-- 
Eduardo

[Qemu-devel] [PATCH v3 03/16] gdbstub: add multiprocess support to '?' packets

[Luc Michel]

The gdb_get_cpu_pid() function does the PID lookup for the given CPU. It
checks if the CPU is a direct child of a CPU cluster. If it is, the
returned PID is the cluster ID plus one (cluster IDs start at 0, GDB
PIDs at 1). When the CPU is not a child of such a container, the PID of
the first process is returned.

The gdb_fmt_thread_id() function generates the string to be used to identify
a given thread, in a response packet for the peer. This function
supports generating thread IDs when multiprocess mode is enabled (in the
form `p.').

Use them in the reply to a '?' request.

Signed-off-by: Luc Michel 
---
 gdbstub.c | 57 +--
 1 file changed, 55 insertions(+), 2 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 096ac8e99c..9ca743a4c6 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -638,10 +638,49 @@ static int memtox(char *buf, const char *mem, int len)
 }
 }
 return p - buf;
 }
 
+static uint32_t gdb_get_cpu_pid(const GDBState *s, CPUState *cpu)
+{
+#ifndef CONFIG_USER_ONLY
+gchar *path, *name;
+Object *obj;
+CPUClusterState *cluster;
+uint32_t ret;
+
+path = object_get_canonical_path(OBJECT(cpu));
+name = object_get_canonical_path_component(OBJECT(cpu));
+
+if (path == NULL) {
+ret = s->processes[0].pid;
+goto out;
+}
+
+path[strlen(path) - strlen(name) - 1] = '\0';
+
+obj = object_resolve_path_type(path, TYPE_CPU_CLUSTER, NULL);
+
+if (obj == NULL) {
+ret = s->processes[0].pid;
+goto out;
+}
+
+cluster = CPU_CLUSTER(obj);
+ret = cluster->cluster_id + 1;
+
+out:
+g_free(name);
+g_free(path);
+
+return ret;
+
+#else
+return s->processes[0].pid;
+#endif
+}
+
 static const char *get_feature_xml(const char *p, const char **newp,
CPUClass *cc)
 {
 size_t len;
 int i;
@@ -907,10 +946,23 @@ static CPUState *find_cpu(uint32_t thread_id)
 }
 
 return NULL;
 }
 
+static char *gdb_fmt_thread_id(const GDBState *s, CPUState *cpu,
+   char *buf, size_t buf_size)
+{
+if (s->multiprocess) {
+snprintf(buf, buf_size, "p%02x.%02x",
+ gdb_get_cpu_pid(s, cpu), cpu_gdb_index(cpu));
+} else {
+snprintf(buf, buf_size, "%02x", cpu_gdb_index(cpu));
+}
+
+return buf;
+}
+
 static int is_query_packet(const char *p, const char *query, char separator)
 {
 unsigned int query_len = strlen(query);
 
 return strncmp(p, query, query_len) == 0 &&
@@ -1018,22 +1070,23 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 const char *p;
 uint32_t thread;
 int ch, reg_size, type, res;
 uint8_t mem_buf[MAX_PACKET_LENGTH];
 char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
+char thread_id[16];
 uint8_t *registers;
 target_ulong addr, len;
 
 trace_gdbstub_io_command(line_buf);
 
 p = line_buf;
 ch = *p++;
 switch(ch) {
 case '?':
 /* TODO: Make this return the correct value for user-mode.  */
-snprintf(buf, sizeof(buf), "T%02xthread:%02x;", GDB_SIGNAL_TRAP,
- cpu_gdb_index(s->c_cpu));
+snprintf(buf, sizeof(buf), "T%02xthread:%s;", GDB_SIGNAL_TRAP,
+ gdb_fmt_thread_id(s, s->c_cpu, thread_id, sizeof(thread_id)));
 put_packet(s, buf);
 /* Remove all the breakpoints when this query is issued,
  * because gdb is doing and initial connect and the state
  * should be cleaned up.
  */
-- 
2.19.1

Re: [Qemu-devel] [libvirt] [PATCH 3/3] cirrus: mark as deprecated

[Cole Robinson]

On 10/25/2018 09:37 PM, Daniel P. Berrangé wrote:

On Thu, Oct 25, 2018 at 10:52:56AM +0200, Gerd Hoffmann wrote:

While being at it deprecate cirrus too.

Reason (short version): use stdvga instead.
Verbose version:
 https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful


Every single one of my guests is using cirrus. This wasn't an explicit
choice on my part, so I believe it is being used as the default in
virt-install.



virt-manager/virt-install 2.0.0 will never explicitly set cirrus, but 
that release is quite new. The previous release's default for x86 was 
roughly:


if using spice graphics:
use qxl
elif guest os is windows:
use vga
else:
use cirrus

It was definitely sub optimal. Maybe your virt-install commands were 
explicitly setting --graphics vnc which would trigger cirrus in that case.



I don't debate the points in the blog post above that stdvga is a
better choice, but I don't think that's enough to justify deprecating
cirrus at this point in time, because when it then gets deleted it
will break way too many existing deployments.

We need to socialize info in that blog post above more widely and
especially ensure that apps are not using that by default. I don't
see it being viable to formally deprecate it in QEMU any time soon
though given existing usage.

Re: [Qemu-devel] [PATCH] cpu.h: fix a typo in comment

[Laurent Vivier]

On 05/09/2018 13:29, Li Qiang wrote:
> Found by reading the code.
> 
> Signed-off-by: Li Qiang 
> ---
>  include/qom/cpu.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index dc130cd307..5bb94a9f86 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -852,7 +852,7 @@ extern CPUInterruptHandler cpu_interrupt_handler;
>  /**
>   * cpu_interrupt:
>   * @cpu: The CPU to set an interrupt on.
> - * @mask: The interupts to set.
> + * @mask: The interrupts to set.
>   *
>   * Invokes the interrupt handler.
>   */
> 

Applied

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 2/3] adlib: mark as insecure and deprecated.

[Paolo Bonzini]

On 25/10/2018 10:52, Gerd Hoffmann wrote:
> We have a lovely, guest-triggerable buffer overflow in opl2 emulation.
> 
> Reproducer:
> outw(0xff60, 0x220);
> outw(0x1020, 0x220);
> outw(0xffb0, 0x220);
> Result:
> Will overflow FM_OPL->AR_TABLE[] (see hw/audio/fmopl.[ch])

I am dumb and I don't understand.  In set_ar_dr you get

v = 0xff
ar = 15
dr = 15

and OPL->AR_TABLE[60] is accessed.  The size of the array is 75, which
seems to be actually 14 more than required.  Likewise OPL->DR_TABLE[60]
is accessed.

The next accesses use SLOT->ksr which is 0 so it's fine too.

Paolo

[Qemu-devel] [PATCH v2] migration: avoid segmentfault when take a snapshot of a VM which being migrated

[Jia Lina]

During an active background migration, snapshot will trigger a
segmentfault. As snapshot clears the "current_migration" struct
and updates "to_dst_file" before it finds out that there is a
migration task, Migration accesses the null pointer in
"current_migration" struct and qemu crashes eventually.

Signed-off-by: Jia Lina 
Signed-off-by: Chai Wen 
Signed-off-by: Zhang Yu 
---
 migration/migration.c |  2 +-
 migration/migration.h |  2 ++
 migration/savevm.c| 19 +++
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index d6ae879dc8..b5e71c7bfc 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -711,7 +711,7 @@ MigrationParameters *qmp_query_migrate_parameters(Error 
**errp)
  * Return true if we're already in the middle of a migration
  * (i.e. any of the active or setup states)
  */
-static bool migration_is_setup_or_active(int state)
+bool migration_is_setup_or_active(int state)
 {
 switch (state) {
 case MIGRATION_STATUS_ACTIVE:
diff --git a/migration/migration.h b/migration/migration.h
index f7813f8261..e413d4d8b6 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -241,6 +241,8 @@ void migrate_fd_error(MigrationState *s, const Error 
*error);
 
 void migrate_fd_connect(MigrationState *s, Error *error_in);
 
+bool migration_is_setup_or_active(int state);
+
 void migrate_init(MigrationState *s);
 bool migration_is_blocked(Error **errp);
 /* True if outgoing migration has entered postcopy phase */
diff --git a/migration/savevm.c b/migration/savevm.c
index 2d10e45582..eeade8cb92 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1319,21 +1319,25 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
 MigrationState *ms = migrate_get_current();
 MigrationStatus status;
 
-migrate_init(ms);
-
-ms->to_dst_file = f;
+if (migration_is_setup_or_active(ms->state) ||
+ms->state == MIGRATION_STATUS_CANCELLING ||
+ms->state == MIGRATION_STATUS_COLO) {
+error_setg(errp, QERR_MIGRATION_ACTIVE);
+return -EINVAL;
+}
 
 if (migration_is_blocked(errp)) {
-ret = -EINVAL;
-goto done;
+return -EINVAL;
 }
 
 if (migrate_use_block()) {
 error_setg(errp, "Block migration and snapshots are incompatible");
-ret = -EINVAL;
-goto done;
+return -EINVAL;
 }
 
+migrate_init(ms);
+ms->to_dst_file = f;
+
 qemu_mutex_unlock_iothread();
 qemu_savevm_state_header(f);
 qemu_savevm_state_setup(f);
@@ -1355,7 +1359,6 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
 error_setg_errno(errp, -ret, "Error while writing VM state");
 }
 
-done:
 if (ret != 0) {
 status = MIGRATION_STATUS_FAILED;
 } else {
-- 
2.13.2.windows.1

Re: [Qemu-devel] [PATCH v3 0/9] iotests: Make them work for both Python 2 and 3

[Eduardo Habkost]

On Mon, Oct 22, 2018 at 02:52:58PM +0100, Max Reitz wrote:
> This series prepares the iotests to work with both Python 2 and 3.  In
> some places, it adds version-specific code and decides what to do based
> on the version (for instance, whether to import the StringIO or the
> BytesIO class from 'io' for use with the test runner), but most of the
> time, it just makes code work for both versions in general.
> 
> And when we make the switch to make Python 3 mandatory, we can simply
> drop the Python 2 branches.

Queued on python-next, thanks!

-- 
Eduardo

Re: [Qemu-devel] [PATCH v7 04/20] target/mips: Add and integrate MXU decoding engine placeholder

[Stefan Markovic]

On 24.10.18. 14:18, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
>
> Provide the placeholder and add the invocation logic for MXU
> decoding engine.
>
> Signed-off-by: Aleksandar Markovic 
> ---
>   target/mips/translate.c | 8 
>   1 file changed, 8 insertions(+)
>
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index fefe9ac..128cabe 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -23844,6 +23844,12 @@ static void decode_opc_special(CPUMIPSState *env, 
> DisasContext *ctx)
>   }
>   }
>   
> +static void decode_opc_mxu(CPUMIPSState *env, DisasContext *ctx)
> +{
> +MIPS_INVAL("decode_opc_mxu");
> +generate_exception_end(ctx, EXCP_RI);
> +}
> +
>   static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
>   {
>   int rs, rt, rd;
> @@ -26087,6 +26093,8 @@ static void decode_opc(CPUMIPSState *env, 
> DisasContext *ctx)
>   case OPC_SPECIAL2:
>   if ((ctx->insn_flags & INSN_R5900) && (ctx->insn_flags & ASE_MMI)) {
>   decode_tx79_mmi(env, ctx);
> +} else if (ctx->insn_flags & ASE_MXU) {
> +decode_opc_mxu(env, ctx);


Is the best way to implement this to include processing of MUL, CLZ, 
CLO, SDDP instructions into decode_opc_mxu as their encodings aren't 
overlaid by MXU instructions

considering MIPS SPECIAL2 instruction pool and MXU Instruction Set?


>   } else {
>   decode_opc_special2_legacy(env, ctx);
>   }

Re: [Qemu-devel] [PATCH v7 05/20] target/mips: Add MXU decoding engine

[Stefan Markovic]

On 24.10.18. 14:18, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
>
> Add MXU decoding engine: add handlers for all instruction pools,
> and main decode handler. The handlers, for now, for the purpose
> of this patch, contain only sceleton in the form of a single
> switch statement.
>
> Signed-off-by: Aleksandar Markovic 
> ---
>   target/mips/translate.c | 1143 
> ++-
>   1 file changed, 1141 insertions(+), 2 deletions(-)
>
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 128cabe..ed72b32 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -23844,12 +23844,1151 @@ static void decode_opc_special(CPUMIPSState *env, 
> DisasContext *ctx)
>   }
>   }
>   
> +/*
> + *
> + * Decode MXU pool00
> + *
> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +---+-+-+---+---+---+---+
> + *  |  SPECIAL2 |0 0 0 0 0|x x x|  XRc  |  XRb  |  XRa  |MXU__POOL00|
> + *  +---+-+-+---+---+---+---+
> + *
> + */
> +static void decode_opc_mxu__pool00(CPUMIPSState *env, DisasContext *ctx)
> +{
> +uint32_t opcode = extract32(ctx->opcode, 18, 3);
> +
> +switch (opcode) {
> +case OPC_MXU_S32MAX:
> +/* TODO: Implement emulation of S32MAX instruction. */
> +MIPS_INVAL("OPC_MXU_S32MAX");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_S32MIN:
> +/* TODO: Implement emulation of S32MIN instruction. */
> +MIPS_INVAL("OPC_MXU_S32MIN");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_D16MAX:
> +/* TODO: Implement emulation of D16MAX instruction. */
> +MIPS_INVAL("OPC_MXU_D16MAX");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_D16MIN:
> +/* TODO: Implement emulation of D16MIN instruction. */
> +MIPS_INVAL("OPC_MXU_D16MIN");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_Q8MAX:
> +/* TODO: Implement emulation of Q8MAX instruction. */
> +MIPS_INVAL("OPC_MXU_Q8MAX");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_Q8MIN:
> +/* TODO: Implement emulation of Q8MIN instruction. */
> +MIPS_INVAL("OPC_MXU_Q8MIN");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_Q8SLT:
> +/* TODO: Implement emulation of Q8SLT instruction. */
> +MIPS_INVAL("OPC_MXU_Q8SLT");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_Q8SLTU:
> +/* TODO: Implement emulation of Q8SLTU instruction. */
> +MIPS_INVAL("OPC_MXU_Q8SLTU");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +default:
> +MIPS_INVAL("decode_opc_mxu");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +}
> +}
> +
> +/*
> + *
> + * Decode MXU pool01
> + *
> + *  S32SLT, D16SLT, D16AVG, D16AVGR, Q8AVG, Q8AVGR:
> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +---+-+-+---+---+---+---+
> + *  |  SPECIAL2 |0 0 0 0 0|x x x|  XRc  |  XRb  |  XRa  |MXU__POOL01|
> + *  +---+-+-+---+---+---+---+
> + *
> + *  Q8MADD:


Q8ADD, instead of Q8MADD.

Otherwise:

Reviewed-by: Stefan Markovic 


> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +---+---+-+-+---+---+---+---+
> + *  |  SPECIAL2 |en2|0 0 0|x x x|  XRc  |  XRb  |  XRa  |MXU__POOL01|
> + *  +---+---+-+-+---+---+---+---+
> + *
> + */
> +static void decode_opc_mxu__pool01(CPUMIPSState *env, DisasContext *ctx)
> +{
> +uint32_t opcode = extract32(ctx->opcode, 18, 3);
> +
> +switch (opcode) {
> +case OPC_MXU_S32SLT:
> +/* TODO: Implement emulation of S32SLT instruction. */
> +MIPS_INVAL("OPC_MXU_S32SLT");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_D16SLT:
> +/* TODO: Implement emulation of D16SLT instruction. */
> +MIPS_INVAL("OPC_MXU_D16SLT");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_D16AVG:
> +/* TODO: Implement emulation of D16AVG instruction. */
> +MIPS_INVAL("OPC_MXU_D16AVG");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_D16AVGR:
> +/* TODO: Implement emulation of D16AVGR instruction. */
> +MIPS_INVAL("OPC_MXU_D16AVGR");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +case OPC_MXU_Q8AVG:
> +/* TODO: Implement emulation of Q8AVG instruction. */
> +MIPS_INVAL("OPC_MXU_Q8AVG");
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +c

Re: [Qemu-devel] [PATCH v7 06/20] target/mips: Add bit encoding for MXU accumulate add/sub 1-bit pattern 'aptn1'

[Stefan Markovic]

On 24.10.18. 14:18, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
>
> Add bit encoding for MXU accumulate add/subtract 1-bit pattern
> 'aptn1'.
>
> Signed-off-by: Aleksandar Markovic 
> ---
>   target/mips/translate.c | 6 ++
>   1 file changed, 6 insertions(+)


Reviewed-by: Stefan Markovic 


> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index ed72b32..f274ac1 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -23844,6 +23844,12 @@ static void decode_opc_special(CPUMIPSState *env, 
> DisasContext *ctx)
>   }
>   }
>   
> +
> +/* MXU accumulate add/subtract 1-bit pattern 'aptn1' */
> +#define MXU_APTN1_A0
> +#define MXU_APTN1_S1
> +
> +
>   /*
>*
>* Decode MXU pool00

Re: [Qemu-devel] [PATCH v7 08/20] target/mips: Add bit encoding for MXU execute add/sub pattern 'eptn2'

[Stefan Markovic]

On 24.10.18. 14:18, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
>
> Add bit encoding for MXU execute 2-bit add/subtract pattern 'eptn2'.
>
> Signed-off-by: Aleksandar Markovic 
> ---
>   target/mips/translate.c | 6 ++
>   1 file changed, 6 insertions(+)


Reviewed-by: Stefan Markovic 


> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 97fb2e0..665a584 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -23855,6 +23855,12 @@ static void decode_opc_special(CPUMIPSState *env, 
> DisasContext *ctx)
>   #define MXU_APTN2_SA2
>   #define MXU_APTN2_SS3
>   
> +/* MXU execute add/subtract 2-bit pattern 'eptn2' */
> +#define MXU_EPTN2_AA0
> +#define MXU_EPTN2_AS1
> +#define MXU_EPTN2_SA2
> +#define MXU_EPTN2_SS3
> +
>   
>   /*
>*

Re: [Qemu-devel] [PATCH] lsi53c895a: check message length value

[P J P]

+-- On Thu, 25 Oct 2018, Ameya More wrote --+
| While Mark and I reported this issue to you, it was actually discovered by 
| Dejvau Security and they should receive credit for reporting this issue. 
| http://www.dejavusecurity.com

I see; Would it be possible to share email-id of the original reporter to 
include in the commit log message?

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH 2/3] nvme: check size before memcpy

[Paolo Bonzini]

On 22/10/2018 14:14, P J P wrote:
> From: Prasad J Pandit 
> 
> While in nvme_mmio_read, memcpy could read past the 'n->bar'
> buffer, if addr offset was pointing towards its tail end.
> Add check to avoid OOB access.
> 
> Reported-by: Caihongzhu 
> Signed-off-by: Prasad J Pandit 
> ---
>  hw/block/nvme.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index fc7dacb816..87afc19b61 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1059,7 +1059,7 @@ static uint64_t nvme_mmio_read(void *opaque, hwaddr 
> addr, unsigned size)
>  /* should RAZ, fall through for now */
>  }
>  
> -if (addr < sizeof(n->bar)) {
> +if (addr + size <= sizeof(n->bar)) {
>  memcpy(&val, ptr + addr, size);
>  } else {
>  NVME_GUEST_ERR(nvme_ub_mmiord_invalid_ofs,
> 

Do you have a reproducer?  In particular, I think this cannot happen
because memory_region_dispatch_read will block accesses beyond 4 bytes,
and earlier code in this function already check that accesses are
aligned to 32 bits.

We could clarify it with

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index fc7dacb816..427e69a78d 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1167,7 +1167,7 @@ static const MemoryRegionOps nvme_mmio_ops = {
 .endianness = DEVICE_LITTLE_ENDIAN,
 .impl = {
 .min_access_size = 2,
-.max_access_size = 8,
+.max_access_size = 4,
 },
 };


but if my understanding is right then there is no bug.

Paolo

Re: [Qemu-devel] [PATCH 1/3] arm: check bit index before use

[Paolo Bonzini]

On 22/10/2018 14:09, P J P wrote:
> From: Prasad J Pandit 
> 
> While performing gpio write via strongarm_gpio_handler_update
> routine, the 'bit' index could access beyond s->handler[28] array.
> Add check to avoid OOB access.
> 
> Reported-by: Moguofang 
> Signed-off-by: Prasad J Pandit 
> ---
>  hw/arm/strongarm.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c
> index ec2627374d..3dda75feaf 100644
> --- a/hw/arm/strongarm.c
> +++ b/hw/arm/strongarm.c
> @@ -532,7 +532,9 @@ static void 
> strongarm_gpio_handler_update(StrongARMGPIOInfo *s)
>  
>  for (diff = s->prev_level ^ level; diff; diff ^= 1 << bit) {
>  bit = ctz32(diff);
> -qemu_set_irq(s->handler[bit], (level >> bit) & 1);
> +if (bit < sizeof(s->handler) / sizeof(s->handler[0])) {
> +qemu_set_irq(s->handler[bit], (level >> bit) & 1);
> +}
>  }
>  
>  s->prev_level = level;
> 

This is correct, but please use ARRAY_SIZE(s->handler).

Paolo

Re: [Qemu-devel] [PATCH 2/3] adlib: mark as insecure and deprecated.

[P J P]

+-- On Fri, 26 Oct 2018, Paolo Bonzini wrote --+
| I am dumb and I don't understand.  In set_ar_dr you get
| 
|   v = 0xff
|   ar = 15
|   dr = 15
| 
| and OPL->AR_TABLE[60] is accessed.  The size of the array is 75, which
| seems to be actually 14 more than required.  Likewise OPL->DR_TABLE[60]
| is accessed.
| 
| The next accesses use SLOT->ksr which is 0 so it's fine too.

In set_ar_dr

  SLOT->AR = ar ? &OPL->AR_TABLE[ar<<2] : RATE_0;

SLOT->AR is set to point to OPL->DR_TABLE[60] and while so if s->ksr is set to 
15, in CALC_FCSLOT()

  SLOT->evsa = SLOT->AR[ksr];  <= accesses OPL->AR_TABLE[60 + 15];

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH] lsi53c895a: check message length value

[P J P]

+-- On Fri, 26 Oct 2018, Paolo Bonzini wrote --+
| > -int msg_len;
| > +uint8_t msg_len;
| 
| Not wrong per se, but it's also not clear why it's needed.  I understand
| that you want to switch from signed to unsigned, but it is not mentioned
| in the commit message.

Changed to uint8_t because IIUC 'msg_len' is likely be < LSI_MAX_MSGIN_LEN=8. 
And uint32_t seems rather large for it.
 
| The switch to 8-bit, and below from VMSTATE_INT32 to VMSTATE_UINT8, is
| wrong because it changes the format of the live migration stream.

I see.

| I'm not sure it's appropriate to check for out of bounds reads here,
| because if s->msg_len is greater than LSI_MAX_MSGIN_LEN, then this test
| doesn't exclude you've already had an out of bounds write before.
| Indeed the msg_len is checked in lsi_add_msg_byte in order to avoid out
| of bounds accesses in either lsi_add_msg_byte or lsi_do_msgin. You
| could assert here that the variable is in range, I guess.

Okay.

| However, the out of bounds s->msg_len can actually happen in one other
| case: namely, if a malicious live migration stream includes a bogus
| s->msg_len.  Such live migration stream should be rejected; the fix for
| that is to add a lsi_post_load function, point to it in
| vmstate_lsi_scsi, and check there for s->msg_len <= LSI_MAX_MSGIN_LEN.

Okay, sending a revised patch v1 in a bit.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH v1] arm: check bit index before usage

[P J P]

+-- On Fri, 26 Oct 2018, Peter Maydell wrote --+
| > ===
| > diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c
| > index ec2627374d..dd8c4b1f2e 100644
| > --- a/hw/arm/strongarm.c
| > +++ b/hw/arm/strongarm.c
| > @@ -587,12 +587,12 @@ static void strongarm_gpio_write(void *opaque, hwaddr
| > offset,
| >
| >  switch (offset) {
| >  case GPDR:/* GPIO Pin-Direction registers */
| > -s->dir = value;
| > +s->dir = value & 0x3f;
| >  strongarm_gpio_handler_update(s);
| >  break;
| >
| >  case GPSR:/* GPIO Pin-Output Set registers */
| > -s->olevel |= value;
| > +s->olevel |= value & 0x3f;
| >  strongarm_gpio_handler_update(s);
| >  break;
| > ===
| >
| > does this seem okay?
| 
| Yes, that's what I had in mind.

Cool, patch sent.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH 3/3] ppc/pnv: check size before data buffer access

[P J P]

+-- On Fri, 26 Oct 2018, Cédric Le Goater wrote --+
| On 10/25/18 8:45 AM, P J P wrote:
| >  - While we refactor the routine for better, a patch below seem okay to fix 
| >the OOB access issue?
| 
| I think it is fine. Please add something like :  
| 
|   qemu_log_mask(LOG_GUEST_ERROR, "ECCB: invalid operation at @0x%08x" 
|   " size %d\n", opb_addr, sz);

Okay, will do.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH 3/3] cirrus: mark as deprecated

[Daniel P . Berrangé]

On Fri, Oct 26, 2018 at 12:33:55PM +0530, P J P wrote:
>   Hello Dan, all
> 
> +-- On Thu, 25 Oct 2018, Daniel P. Berrangé wrote --+
> | On Thu, Oct 25, 2018 at 10:52:56AM +0200, Gerd Hoffmann wrote:
> | > While being at it deprecate cirrus too.
> | > 
> | > Reason (short version): use stdvga instead.
> | > Verbose version:
> | > 
> https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful
> | 
> | 
> | I don't debate the points in the blog post above that stdvga is a
> | better choice, but I don't think that's enough to justify deprecating
> | cirrus at this point in time, because when it then gets deleted it
> | will break way too many existing deployments.
> | 
> | We need to socialize info in that blog post above more widely and
> | especially ensure that apps are not using that by default. I don't
> | see it being viable to formally deprecate it in QEMU any time soon
> | though given existing usage.
> 
> To note, IMO there are other devices/sources in QEMU which are potential 
> candidates for deprecation, similar to adlib etc. It'll help if we could 
> device a process to deprecate/remove such code base. Other than maintenance 
> it 
> invariably also becomes source of security issues.
> 
> Ex.(similar to Fedora) we could announce such candidate on qemu-devel list 
> and 
> after review over a period of say a month, candidate will be
> deprecated/expunged. (thinking aloud)

QEMU has a deprecation process:

  https://qemu.weilnetz.de/doc/qemu-doc.html#Deprecated-features

Most of the stuff deprecated is CLI args / monitor commands, etc where
mgmt apps just adjust the way they are calling QEMU, so end user's VMs
are largely not impacted.

Deprecating a device type that is widely used is not desirable because
that will cause breakage of existing guests.  Distros are free to disable
devices in their builds if they want to reduce the scope for CVEs in
packages they maintain, but again they should think carefully about how
many users they are going to break by doing so.

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

Re: [Qemu-devel] [libvirt] [PATCH 3/3] cirrus: mark as deprecated

[Daniel P . Berrangé]

On Fri, Oct 26, 2018 at 09:48:35AM +0100, Cole Robinson wrote:
> On 10/25/2018 09:37 PM, Daniel P. Berrangé wrote:
> > On Thu, Oct 25, 2018 at 10:52:56AM +0200, Gerd Hoffmann wrote:
> > > While being at it deprecate cirrus too.
> > > 
> > > Reason (short version): use stdvga instead.
> > > Verbose version:
> > >  
> > > https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful
> > 
> > Every single one of my guests is using cirrus. This wasn't an explicit
> > choice on my part, so I believe it is being used as the default in
> > virt-install.
> > 
> 
> virt-manager/virt-install 2.0.0 will never explicitly set cirrus, but that
> release is quite new. The previous release's default for x86 was roughly:
> 
> if using spice graphics:
> use qxl
> elif guest os is windows:
> use vga
> else:
> use cirrus
> 
> It was definitely sub optimal. Maybe your virt-install commands were
> explicitly setting --graphics vnc which would trigger cirrus in that case.

Yep, I've always tended to use vnc, since QXL has historically been a
pretty buggy & unreliable device model with guests often breaking.

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

Re: [Qemu-devel] [PATCH v7 13/20] target/mips: Move MUL, S32M2I, S32I2M handling out of main MXU switch

[Stefan Markovic]

On 24.10.18. 14:18, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
>
> Move MUL, S32M2I, S32I2M handling out of switch. These are all
> instructions that do not depend on MXU_EN flag of MXU_CR.
>
> Signed-off-by: Aleksandar Markovic 
> ---
>   target/mips/translate.c | 41 +++--
>   1 file changed, 23 insertions(+), 18 deletions(-)


See my comment for patch 04/20.

CLZ, CLO, SDDP are missing?


> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index c8c71c4..111affb 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -24859,6 +24859,29 @@ static void decode_opc_mxu(CPUMIPSState *env, 
> DisasContext *ctx)
>   {
>   uint32_t opcode = extract32(ctx->opcode, 0, 6);
>   
> +if (opcode == OPC__MXU_MUL) {
> +uint32_t  rs, rt, rd, op1;
> +
> +rs = extract32(ctx->opcode, 21, 5);
> +rt = extract32(ctx->opcode, 16, 5);
> +rd = extract32(ctx->opcode, 11, 5);
> +op1 = MASK_SPECIAL2(ctx->opcode);
> +
> +gen_arith(ctx, op1, rd, rs, rt);
> +
> +return;
> +}
> +
> +if (opcode == OPC_MXU_S32M2I) {
> +gen_mxu_s32m2i(ctx);
> +return;
> +}
> +
> +if (opcode == OPC_MXU_S32I2M) {
> +gen_mxu_s32i2m(ctx);
> +return;
> +}
> +
>   switch (opcode) {
>   case OPC_MXU_S32MADD:
>   /* TODO: Implement emulation of S32MADD instruction. */
> @@ -24870,18 +24893,6 @@ static void decode_opc_mxu(CPUMIPSState *env, 
> DisasContext *ctx)
>   MIPS_INVAL("OPC_MXU_S32MADDU");
>   generate_exception_end(ctx, EXCP_RI);
>   break;
> -case OPC__MXU_MUL: /* 0x2 - unused in MXU specs */
> -{
> -uint32_t  rs, rt, rd, op1;
> -
> -rs = extract32(ctx->opcode, 21, 5);
> -rt = extract32(ctx->opcode, 16, 5);
> -rd = extract32(ctx->opcode, 11, 5);
> -op1 = MASK_SPECIAL2(ctx->opcode);
> -
> -gen_arith(ctx, op1, rd, rs, rt);
> -}
> -break;
>   case OPC_MXU__POOL00:
>   decode_opc_mxu__pool00(env, ctx);
>   break;
> @@ -25033,12 +25044,6 @@ static void decode_opc_mxu(CPUMIPSState *env, 
> DisasContext *ctx)
>   MIPS_INVAL("OPC_MXU_S16SDI");
>   generate_exception_end(ctx, EXCP_RI);
>   break;
> -case OPC_MXU_S32M2I:
> -gen_mxu_s32m2i(ctx);
> -break;
> -case OPC_MXU_S32I2M:
> -gen_mxu_s32i2m(ctx);
> -break;
>   case OPC_MXU_D32SLL:
>   /* TODO: Implement emulation of D32SLL instruction. */
>   MIPS_INVAL("OPC_MXU_D32SLL");

Re: [Qemu-devel] [PATCH v7 19/20] target/mips: Move MXU_EN check one level higher

[Stefan Markovic]

On 24.10.18. 14:18, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
>
> Move MXU_EN check to the main MXU decoding function, to avoid code
> repetition.
>
> Signed-off-by: Aleksandar Markovic 
> ---
>   target/mips/translate.c | 509 
> ++--
>   1 file changed, 238 insertions(+), 271 deletions(-)


Reviewed-by: Stefan Markovic 


> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 61c1662..3620ae5 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -23960,23 +23960,16 @@ static void gen_mxu_s32m2i(DisasContext *ctx)
>   static void gen_mxu_s8ldd(DisasContext *ctx)
>   {
>   TCGv t0, t1;
> -TCGLabel *l0;
>   uint32_t XRa, Rb, s8, optn3;
>   
>   t0 = tcg_temp_new();
>   t1 = tcg_temp_new();
>   
> -l0 = gen_new_label();
> -
>   XRa = extract32(ctx->opcode, 6, 4);
>   s8 = extract32(ctx->opcode, 10, 8);
>   optn3 = extract32(ctx->opcode, 18, 3);
>   Rb = extract32(ctx->opcode, 21, 5);
>   
> -gen_load_mxu_cr(t0);
> -tcg_gen_andi_tl(t0, t0, MXU_CR_MXU_EN);
> -tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXU_CR_MXU_EN, l0);
> -
>   gen_load_gpr(t0, Rb);
>   tcg_gen_addi_tl(t0, t0, (int8_t)s8);
>   
> @@ -24034,8 +24027,6 @@ static void gen_mxu_s8ldd(DisasContext *ctx)
>   
>   gen_store_mxu_gpr(t0, XRa);
>   
> -gen_set_label(l0);
> -
>   tcg_temp_free(t0);
>   tcg_temp_free(t1);
>   }
> @@ -24046,7 +24037,6 @@ static void gen_mxu_s8ldd(DisasContext *ctx)
>   static void gen_mxu_d16mul(DisasContext *ctx)
>   {
>   TCGv t0, t1, t2, t3;
> -TCGLabel *l0;
>   uint32_t XRa, XRb, XRc, XRd, optn2;
>   
>   t0 = tcg_temp_new();
> @@ -24054,18 +24044,12 @@ static void gen_mxu_d16mul(DisasContext *ctx)
>   t2 = tcg_temp_new();
>   t3 = tcg_temp_new();
>   
> -l0 = gen_new_label();
> -
>   XRa = extract32(ctx->opcode, 6, 4);
>   XRb = extract32(ctx->opcode, 10, 4);
>   XRc = extract32(ctx->opcode, 14, 4);
>   XRd = extract32(ctx->opcode, 18, 4);
>   optn2 = extract32(ctx->opcode, 22, 2);
>   
> -gen_load_mxu_cr(t0);
> -tcg_gen_andi_tl(t0, t0, MXU_CR_MXU_EN);
> -tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXU_CR_MXU_EN, l0);
> -
>   gen_load_mxu_gpr(t1, XRb);
>   tcg_gen_sextract_tl(t0, t1, 0, 16);
>   tcg_gen_sextract_tl(t1, t1, 16, 16);
> @@ -24094,8 +24078,6 @@ static void gen_mxu_d16mul(DisasContext *ctx)
>   gen_store_mxu_gpr(t3, XRa);
>   gen_store_mxu_gpr(t2, XRd);
>   
> -gen_set_label(l0);
> -
>   tcg_temp_free(t0);
>   tcg_temp_free(t1);
>   tcg_temp_free(t2);
> @@ -24109,7 +24091,6 @@ static void gen_mxu_d16mul(DisasContext *ctx)
>   static void gen_mxu_d16mac(DisasContext *ctx)
>   {
>   TCGv t0, t1, t2, t3;
> -TCGLabel *l0;
>   uint32_t XRa, XRb, XRc, XRd, optn2, aptn2;
>   
>   t0 = tcg_temp_new();
> @@ -24117,8 +24098,6 @@ static void gen_mxu_d16mac(DisasContext *ctx)
>   t2 = tcg_temp_new();
>   t3 = tcg_temp_new();
>   
> -l0 = gen_new_label();
> -
>   XRa = extract32(ctx->opcode, 6, 4);
>   XRb = extract32(ctx->opcode, 10, 4);
>   XRc = extract32(ctx->opcode, 14, 4);
> @@ -24126,10 +24105,6 @@ static void gen_mxu_d16mac(DisasContext *ctx)
>   optn2 = extract32(ctx->opcode, 22, 2);
>   aptn2 = extract32(ctx->opcode, 24, 2);
>   
> -gen_load_mxu_cr(t0);
> -tcg_gen_andi_tl(t0, t0, MXU_CR_MXU_EN);
> -tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXU_CR_MXU_EN, l0);
> -
>   gen_load_mxu_gpr(t1, XRb);
>   tcg_gen_sextract_tl(t0, t1, 0, 16);
>   tcg_gen_sextract_tl(t1, t1, 16, 16);
> @@ -24180,8 +24155,6 @@ static void gen_mxu_d16mac(DisasContext *ctx)
>   gen_store_mxu_gpr(t3, XRa);
>   gen_store_mxu_gpr(t2, XRd);
>   
> -gen_set_label(l0);
> -
>   tcg_temp_free(t0);
>   tcg_temp_free(t1);
>   tcg_temp_free(t2);
> @@ -24195,7 +24168,6 @@ static void gen_mxu_d16mac(DisasContext *ctx)
>   static void gen_mxu_q8mul_q8mulsu(DisasContext *ctx)
>   {
>   TCGv t0, t1, t2, t3, t4, t5, t6, t7;
> -TCGLabel *l0;
>   uint32_t XRa, XRb, XRc, XRd, sel;
>   
>   t0 = tcg_temp_new();
> @@ -24207,18 +24179,12 @@ static void gen_mxu_q8mul_q8mulsu(DisasContext *ctx)
>   t6 = tcg_temp_new();
>   t7 = tcg_temp_new();
>   
> -l0 = gen_new_label();
> -
>   XRa = extract32(ctx->opcode, 6, 4);
>   XRb = extract32(ctx->opcode, 10, 4);
>   XRc = extract32(ctx->opcode, 14, 4);
>   XRd = extract32(ctx->opcode, 18, 4);
>   sel = extract32(ctx->opcode, 22, 2);
>   
> -gen_load_mxu_cr(t0);
> -tcg_gen_andi_tl(t0, t0, MXU_CR_MXU_EN);
> -tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXU_CR_MXU_EN, l0);
> -
>   gen_load_mxu_gpr(t3, XRb);
>   gen_load_mxu_gpr(t7, XRc);
>   
> @@ -24269,8 +24235,6 @@ static void gen_mxu_q8mul_q8mulsu(DisasContext *ctx)
>   gen_store_mxu_gpr(t0, XRd);
>   gen_store_mxu_gpr(t1, XRa);
>   
> -gen_set_label(l0);
> -
>

Re: [Qemu-devel] [libvirt] [PATCH 2/3] adlib: mark as insecure and deprecated.

[Daniel P . Berrangé]

On Fri, Oct 26, 2018 at 12:38:53PM +0530, P J P wrote:
> +-- On Thu, 25 Oct 2018, Daniel P. Berrangé wrote --+
> | On Thu, Oct 25, 2018 at 04:26:16PM +0530, P J P wrote:
> | > +-- On Thu, 25 Oct 2018, Gerd Hoffmann wrote --+
> | > | We have a lovely, guest-triggerable buffer overflow in opl2 emulation.
> | > | 
> | > | Reproducer:
> | > | outw(0xff60, 0x220);
> | > | outw(0x1020, 0x220);
> | > | outw(0xffb0, 0x220);
> | > | Result:
> | > | Will overflow FM_OPL->AR_TABLE[] (see hw/audio/fmopl.[ch])
> | > 
> | > + Reported-by: Wangjunqing 
> | 
> | So you have a CVE number for this ?
> 
> No, since the adlib device is not used as much and is being deprecated, I'm 
> not inclined to get one.

Any security issue that affects code in QEMU that is currently being
shipped by distros should have a CVE.

Whether we intend to deprecate & delete it later should not be a factor
because we are free to cancel the deprecation process at any time if we
find a reason to keep the feature around.

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

Re: [Qemu-devel] [PATCH v7 20/20] target/mips: Amend MXU ASE overview note

[Stefan Markovic]

On 24.10.18. 14:18, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
>
> Add prefix, suffix, operation descriptions, and other corrections
> and amendments to the comment that describes MXU ASE.
>
> Signed-off-by: Aleksandar Markovic 
> ---
>   target/mips/translate.c | 84 
> +++--
>   1 file changed, 74 insertions(+), 10 deletions(-)


Reviewed-by: Stefan Markovic 


> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 3620ae5..9bd5f27 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -1410,25 +1410,89 @@ enum {
>* MXU unit contains 17 registers called X0-X16. X0 is always zero, and X16 
> is
>* the control register.
>*
> - * The notation used in MXU assembler mnemonics:
> + * The notation used in MXU assembler mnemonics
> + * 
> + *
> + *  Registers:
>*
>*   XRa, XRb, XRc, XRd - MXU registers
>*   Rb, Rc, Rd, Rs, Rt - general purpose MIPS registers
> - *   s12- a subfield of an instruction code
> - *   strd2  - a subfield of an instruction code
> - *   eptn2  - a subfield of an instruction code
> - *   eptn3  - a subfield of an instruction code
> - *   optn2  - a subfield of an instruction code
> - *   optn3  - a subfield of an instruction code
> - *   sft4   - a subfield of an instruction code
> + *
> + *  Subfields:
> + *
> + *   aptn1  - 1-bit accumulate add/subtract pattern
> + *   aptn2  - 2-bit accumulate add/subtract pattern
> + *   eptn2  - 2-bit execute add/subtract pattern
> + *   optn2  - 2-bit operand pattern
> + *   optn3  - 3-bit operand pattern
> + *   sft4   - 4-bit shift amount
> + *   strd2  - 2-bit stride amount
> + *
> + *  Prefixes:
> + *
> + *   
> + * S 32
> + * D 16
> + * Q  8
> + *
> + *  Suffixes:
> + *
> + *   E - Expand results
> + *   F - Fixed point multiplication
> + *   L - Low part result
> + *   R - Doing rounding
> + *   V - Variable instead of immediate
> + *   W - Combine above L and V
> + *
> + *  Operations:
> + *
> + *   ADD   - Add or subtract
> + *   ADDC  - Add with carry-in
> + *   ACC   - Accumulate
> + *   ASUM  - Sum together then accumulate (add or subtract)
> + *   ASUMC - Sum together then accumulate (add or subtract) with carry-in
> + *   AVG   - Average between 2 operands
> + *   ABD   - Absolute difference
> + *   ALN   - Align data
> + *   AND   - Logical bitwise 'and' operation
> + *   CPS   - Copy sign
> + *   EXTR  - Extract bits
> + *   I2M   - Move from GPR register to MXU register
> + *   LDD   - Load data from memory to XRF
> + *   LDI   - Load data from memory to XRF (and increase the address base)
> + *   LUI   - Load unsigned immediate
> + *   MUL   - Multiply
> + *   MULU  - Unsigned multiply
> + *   MADD  - 64-bit operand add 32x32 product
> + *   MSUB  - 64-bit operand subtract 32x32 product
> + *   MAC   - Multiply and accumulate (add or subtract)
> + *   MAD   - Multiply and add or subtract
> + *   MAX   - Maximum between 2 operands
> + *   MIN   - Minimum between 2 operands
> + *   M2I   - Move from MXU register to GPR register
> + *   MOVZ  - Move if zero
> + *   MOVN  - Move if non-zero
> + *   NOR   - Logical bitwise 'nor' operation
> + *   OR- Logical bitwise 'or' operation
> + *   STD   - Store data from XRF to memory
> + *   SDI   - Store data from XRF to memory (and increase the address base)
> + *   SLT   - Set of less than comparison
> + *   SAD   - Sum of absolute differences
> + *   SLL   - Logical shift left
> + *   SLR   - Logical shift right
> + *   SAR   - Arithmetic shift right
> + *   SAT   - Saturation
> + *   SFL   - Shuffle
> + *   SCOP  - Calculate x’s scope (-1, means x<0; 0, means x==0; 1, means x>0)
> + *   XOR   - Logical bitwise 'exclusive or' operation
>*
>* Load/Store instructions   Multiplication instructions
>* ---   ---
>*
>*  S32LDD XRa, Rb, s12   S32MADD XRa, XRd, Rs, Rt
>*  S32STD XRa, Rb, s12   S32MADDU XRa, XRd, Rs, Rt
> - *  S32LDDV XRa, Rb, rc, strd2S32SUB XRa, XRd, Rs, Rt
> - *  S32STDV XRa, Rb, rc, strd2S32SUBU XRa, XRd, Rs, Rt
> + *  S32LDDV XRa, Rb, rc, strd2S32MSUB XRa, XRd, Rs, Rt
> + *  S32STDV XRa, Rb, rc, strd2S32MSUBU XRa, XRd, Rs, Rt
>*  S32LDI XRa, Rb, s12   S32MUL XRa, XRd, Rs, Rt
>*  S32SDI XRa, Rb, s12   S32MULU XRa, XRd, Rs, Rt
>*  S32LDIV XRa, Rb, rc, strd2D16MUL XRa, XRb, XRc, XRd, optn2

Re: [Qemu-devel] [PATCH 3/3] cirrus: mark as deprecated

[Daniel P . Berrangé]

On Fri, Oct 26, 2018 at 10:42:08AM +0100, Daniel P. Berrangé wrote:
> On Fri, Oct 26, 2018 at 12:33:55PM +0530, P J P wrote:
> >   Hello Dan, all
> > 
> > +-- On Thu, 25 Oct 2018, Daniel P. Berrangé wrote --+
> > | On Thu, Oct 25, 2018 at 10:52:56AM +0200, Gerd Hoffmann wrote:
> > | > While being at it deprecate cirrus too.
> > | > 
> > | > Reason (short version): use stdvga instead.
> > | > Verbose version:
> > | > 
> > https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful
> > | 
> > | 
> > | I don't debate the points in the blog post above that stdvga is a
> > | better choice, but I don't think that's enough to justify deprecating
> > | cirrus at this point in time, because when it then gets deleted it
> > | will break way too many existing deployments.
> > | 
> > | We need to socialize info in that blog post above more widely and
> > | especially ensure that apps are not using that by default. I don't
> > | see it being viable to formally deprecate it in QEMU any time soon
> > | though given existing usage.
> > 
> > To note, IMO there are other devices/sources in QEMU which are potential 
> > candidates for deprecation, similar to adlib etc. It'll help if we could 
> > device a process to deprecate/remove such code base. Other than maintenance 
> > it 
> > invariably also becomes source of security issues.
> > 
> > Ex.(similar to Fedora) we could announce such candidate on qemu-devel list 
> > and 
> > after review over a period of say a month, candidate will be
> > deprecated/expunged. (thinking aloud)
> 
> QEMU has a deprecation process:
> 
>   https://qemu.weilnetz.de/doc/qemu-doc.html#Deprecated-features
> 
> Most of the stuff deprecated is CLI args / monitor commands, etc where
> mgmt apps just adjust the way they are calling QEMU, so end user's VMs
> are largely not impacted.
> 
> Deprecating a device type that is widely used is not desirable because
> that will cause breakage of existing guests.  Distros are free to disable
> devices in their builds if they want to reduce the scope for CVEs in
> packages they maintain, but again they should think carefully about how
> many users they are going to break by doing so.

I should also say that QEMU as an upstream project has multiple goals.
Running KVM guests with modern PV hardware is only one of them, albeit
a widely used one. Being able to run old legacy OS with old hardware,
and running arbitrary embedded boards/devices with emulation are both
use cases that QEMU project aims to address. To eliminate all the old
"crufty" device emulation in name of improving security for KVM, would
be to eliminate core use cases of the project. THis is why we're trying
to persue the direction of making it easier for vendors to disable
features and devices they don't wish to support & thus limit their
downstream CVE exposure.

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

Re: [Qemu-devel] [PATCH 2/3] adlib: mark as insecure and deprecated.

[Paolo Bonzini]

On 26/10/2018 11:34, P J P wrote:
> +-- On Fri, 26 Oct 2018, Paolo Bonzini wrote --+
> | I am dumb and I don't understand.  In set_ar_dr you get
> | 
> | v = 0xff
> | ar = 15
> | dr = 15
> | 
> | and OPL->AR_TABLE[60] is accessed.  The size of the array is 75, which
> | seems to be actually 14 more than required.  Likewise OPL->DR_TABLE[60]
> | is accessed.
> | 
> | The next accesses use SLOT->ksr which is 0 so it's fine too.
> 
> In set_ar_dr
> 
>   SLOT->AR = ar ? &OPL->AR_TABLE[ar<<2] : RATE_0;
> 
> SLOT->AR is set to point to OPL->DR_TABLE[60] and while so if s->ksr is set 
> to 
> 15, in CALC_FCSLOT()
> 
>   SLOT->evsa = SLOT->AR[ksr];  <= accesses OPL->AR_TABLE[60 + 15];

Oh, thanks!  I said I was dumb. :)  So the fix is just this:

diff --git a/hw/audio/fmopl.h b/hw/audio/fmopl.h
index e7e578a48e..7199afaa3c 100644
--- a/hw/audio/fmopl.h
+++ b/hw/audio/fmopl.h
@@ -72,8 +72,8 @@ typedef struct fm_opl_f {
/* Rhythm sention */
uint8_t rhythm; /* Rhythm mode , key flag */
/* time tables */
-   int32_t AR_TABLE[75];   /* atttack rate tables */
-   int32_t DR_TABLE[75];   /* decay rate tables   */
+   int32_t AR_TABLE[76];   /* atttack rate tables */
+   int32_t DR_TABLE[76];   /* decay rate tables   */
uint32_t FN_TABLE[1024];  /* fnumber -> increment counter */
/* LFO */
int32_t *ams_table;

and init_timetables will just fill it with the right value?  (I checked
against another implementation at http://opl3.cozendey.com/).

Thanks,

Paolo

Re: [Qemu-devel] [PATCH 3/3] cirrus: mark as deprecated

[Paolo Bonzini]

On 26/10/2018 11:59, Daniel P. Berrangé wrote:
> I should also say that QEMU as an upstream project has multiple goals.
> Running KVM guests with modern PV hardware is only one of them, albeit
> a widely used one. Being able to run old legacy OS with old hardware,
> and running arbitrary embedded boards/devices with emulation are both
> use cases that QEMU project aims to address. To eliminate all the old
> "crufty" device emulation in name of improving security for KVM, would
> be to eliminate core use cases of the project. THis is why we're trying
> to persue the direction of making it easier for vendors to disable
> features and devices they don't wish to support & thus limit their
> downstream CVE exposure.

Indeed.  If we had to deprecate a feature just because it had an
off-by-one bug, no C program would grow beyond 1000 lines of code...

Paolo

[Qemu-devel] [PATCH 0/2] Deprecate the "collie" machine and Strongarm devices

[Thomas Huth]

These files lack an entry in the MAINTAINERS file, and according to
the initial commits, the board and devices are incomplete. Since there
have hardly been any commits in the past to really improve them, we
should consider to mark them as deprecated now.

Thomas Huth (2):
  hw/arm: Deprecate the "collie" board
  arm: Deprecate the Strongarm sa1100 and sa1110 processors

 hw/arm/collie.c  |  1 +
 qemu-deprecated.texi | 10 ++
 2 files changed, 11 insertions(+)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/2] hw/arm: Deprecate the "collie" board

[Thomas Huth]

"collie" has no entry in the MAINTAINERS file, and the initial commit
with ID c64b21d519a6ecae12f65625fa60f3035ed88644 said:

"Add very basic implementation of collie PDA emulation. The system lacks
 LoCoMo and graphics/sound emulation. Linux kernel boots up to mounting
 rootfs (theoretically it can be provided in pflash images)."

Thus the board is incomplete, since after that initial commit, there were
only the usual QEMU API-related rework patches applied to this file.
Unless someone speaks up and says that this board is still useful, we
should consider to remove it, so mark it as deprecated now.

Signed-off-by: Thomas Huth 
---
 hw/arm/collie.c  | 1 +
 qemu-deprecated.texi | 5 +
 2 files changed, 6 insertions(+)

diff --git a/hw/arm/collie.c b/hw/arm/collie.c
index 48b732c..e41bbc5 100644
--- a/hw/arm/collie.c
+++ b/hw/arm/collie.c
@@ -60,6 +60,7 @@ static void collie_machine_init(MachineClass *mc)
 mc->init = collie_init;
 mc->ignore_memory_transaction_failures = true;
 mc->default_cpu_type = ARM_CPU_TYPE_NAME("sa1110");
+mc->deprecation_reason = "board is incomplete and unmaintained";
 }
 
 DEFINE_MACHINE("collie", collie_machine_init)
diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 11b870c..acf9809 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -129,6 +129,11 @@ This machine type uses an unmaintained firmware, broken in 
lots of ways,
 and unable to start post-2004 operating systems. 40p machine type should be
 used instead.
 
+@subsection collie (ARM) (since 3.1)
+
+The board is incomplete and unmaintained. Use a different ARM board if
+possible instead.
+
 @section Device options
 
 @subsection Block device options
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/2] arm: Deprecate the Strongarm sa1100 and sa1110 processors

[Thomas Huth]

The deprecated "collie" board is the only user of the Strongarm
devices, so if "collie" goes away, we should remove the Strongarm
devices, too.

Signed-off-by: Thomas Huth 
---
 qemu-deprecated.texi | 5 +
 1 file changed, 5 insertions(+)

diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index acf9809..0de5c7f 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -116,6 +116,11 @@ The @option{[hub_id name]} parameter tuple of the 
'hostfwd_add' and
 The ``ivshmem'' device type is replaced by either the ``ivshmem-plain''
 or ``ivshmem-doorbell`` device types.
 
+@subsection Strongarm sa1100 and sa1110 and related devices (since 3.1)
+
+Without the deprecated "collie" board there is no other machine which is
+able to use these devices.
+
 @section System emulator machines
 
 @subsection pc-0.10 and pc-0.11 (since 3.0)
-- 
1.8.3.1

[Qemu-devel] [PATCH v1] xen: preserve COMPAT in CFLAGS

[Olaf Hering]

A given Qemu version can not predict what version of Xen it will run on.
There are some checks in configure to decide what Xen libraries and
functions are available. How exactly these functions must be accessed
has to be decided by configure and the user who is compiling Qemu.
In no way some random header file must override this decision.

Remove the breakage introduced by commit 5eeb39c24b, which would always
hide the libxc interfaces the given version of Qemu knows about.

The current symptom of such breakage is a build failure with qemu-2.9
and older, in combination with Xen 4.12.

Fixes: 5eeb39c24b7d4da5d129bfdd9c4fd21cfb3d28d6
Signed-off-by: Olaf Hering 
---
 include/hw/xen/xen_common.h | 9 -
 1 file changed, 9 deletions(-)

diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 5f1402b494..33fa2d3497 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -1,15 +1,6 @@
 #ifndef QEMU_HW_XEN_COMMON_H
 #define QEMU_HW_XEN_COMMON_H
 
-/*
- * If we have new enough libxenctrl then we do not want/need these compat
- * interfaces, despite what the user supplied cflags might say. They
- * must be undefined before including xenctrl.h
- */
-#undef XC_WANT_COMPAT_EVTCHN_API
-#undef XC_WANT_COMPAT_GNTTAB_API
-#undef XC_WANT_COMPAT_MAP_FOREIGN_API
-
 #include 
 #include 
 #include 

Re: [Qemu-devel] [PATCH 0/2] Deprecate the "collie" machine and Strongarm devices

[Peter Maydell]

On 26 October 2018 at 11:06, Thomas Huth  wrote:
> These files lack an entry in the MAINTAINERS file, and according to
> the initial commits, the board and devices are incomplete. Since there
> have hardly been any commits in the past to really improve them, we
> should consider to mark them as deprecated now.
>
> Thomas Huth (2):
>   hw/arm: Deprecate the "collie" board
>   arm: Deprecate the Strongarm sa1100 and sa1110 processors

Hi Guenter; there's a proposal here to deprecate (and eventually
remove) the 'collie' board (strongarm) from QEMU. Is that one of
the ones you're currently using in your automated testing of Linux
kernels on QEMU?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 3/3] cirrus: mark as deprecated

[Dr. David Alan Gilbert]

* Daniel P. Berrangé (berra...@redhat.com) wrote:
> On Fri, Oct 26, 2018 at 12:33:55PM +0530, P J P wrote:
> >   Hello Dan, all
> > 
> > +-- On Thu, 25 Oct 2018, Daniel P. Berrangé wrote --+
> > | On Thu, Oct 25, 2018 at 10:52:56AM +0200, Gerd Hoffmann wrote:
> > | > While being at it deprecate cirrus too.
> > | > 
> > | > Reason (short version): use stdvga instead.
> > | > Verbose version:
> > | > 
> > https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful
> > | 
> > | 
> > | I don't debate the points in the blog post above that stdvga is a
> > | better choice, but I don't think that's enough to justify deprecating
> > | cirrus at this point in time, because when it then gets deleted it
> > | will break way too many existing deployments.
> > | 
> > | We need to socialize info in that blog post above more widely and
> > | especially ensure that apps are not using that by default. I don't
> > | see it being viable to formally deprecate it in QEMU any time soon
> > | though given existing usage.
> > 
> > To note, IMO there are other devices/sources in QEMU which are potential 
> > candidates for deprecation, similar to adlib etc. It'll help if we could 
> > device a process to deprecate/remove such code base. Other than maintenance 
> > it 
> > invariably also becomes source of security issues.
> > 
> > Ex.(similar to Fedora) we could announce such candidate on qemu-devel list 
> > and 
> > after review over a period of say a month, candidate will be
> > deprecated/expunged. (thinking aloud)
> 
> QEMU has a deprecation process:
> 
>   https://qemu.weilnetz.de/doc/qemu-doc.html#Deprecated-features
> 
> Most of the stuff deprecated is CLI args / monitor commands, etc where
> mgmt apps just adjust the way they are calling QEMU, so end user's VMs
> are largely not impacted.
> 
> Deprecating a device type that is widely used is not desirable because
> that will cause breakage of existing guests.  Distros are free to disable
> devices in their builds if they want to reduce the scope for CVEs in
> packages they maintain, but again they should think carefully about how
> many users they are going to break by doing so.

I'm a bit mixed here;  until recent guest kernels I've always treated
Cirrus as the 'safe' one that's likely to work on anything - so losing
it worries me.  Having said that, I understand why we want to deprecate
it;  but we should give a much longer deprecation warning - a few years?
I don't see any harm warning that it's deprecated and you really should
try not to use it.

Dave

> Regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 02/29] targer/riscv: Activate decodetree and implemnt LUI & AUIPC

[Bastian Koppelmann]

On 10/25/18 6:58 PM, Palmer Dabbelt wrote:


Reviewed-by: Palmer Dabbelt 

How do you want to go about merging these?  It looks like it should be 
possible to merge the patch set piecemeal, which I'd actually be happy 
doing as it'll be easier to get these out for testing that way.  That 
way we can avoid having one day where everyone changes over to a new 
decoder.  If that's OK then I'll start slurping up this up into my 
weekly pull requests as I can review them, with the idea being that I 
won't re-order anything (in other words, I'll start at the first patch 
and take patches until I find one that isn't ready to go).



I think you can pick up everything up to the RVC conversion which still 
needs the work suggested by Richard. Thanks, for picking it up :)


Cheers,

Bastian

Re: [Qemu-devel] [PULL v2 05/43] hw/timer/sun4v-rtc: Convert from DPRINTF() macro to trace events

[Philippe Mathieu-Daudé]

On 26/10/18 10:27, Eduardo Habkost wrote:

On Thu, Oct 25, 2018 at 06:17:59PM +0100, David Gibson wrote:

On Thu, Oct 25, 2018 at 10:32:23AM -0300, Eduardo Habkost wrote:

From: Philippe Mathieu-Daudé 

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Artyom Tarasenko 
Reviewed-by: Cédric Le Goater 
Message-Id: <20181002212522.23303-3-f4...@amsat.org>
Signed-off-by: Eduardo Habkost 
---
  hw/timer/sun4v-rtc.c  | 13 +++--
  hw/timer/trace-events |  4 
  2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/hw/timer/sun4v-rtc.c b/hw/timer/sun4v-rtc.c
index 310523225f..13be94f8da 100644
--- a/hw/timer/sun4v-rtc.c
+++ b/hw/timer/sun4v-rtc.c
@@ -14,15 +14,8 @@
  #include "hw/sysbus.h"
  #include "qemu/timer.h"
  #include "hw/timer/sun4v-rtc.h"
+#include "trace.h"
  
-//#define DEBUG_SUN4V_RTC

-
-#ifdef DEBUG_SUN4V_RTC
-#define DPRINTF(fmt, ...)   \
-do { printf("sun4v_rtc: " fmt , ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) do {} while (0)
-#endif
  
  #define TYPE_SUN4V_RTC "sun4v_rtc"

  #define SUN4V_RTC(obj) OBJECT_CHECK(Sun4vRtc, (obj), TYPE_SUN4V_RTC)
@@ -41,14 +34,14 @@ static uint64_t sun4v_rtc_read(void *opaque, hwaddr addr,
  /* accessing the high 32 bits */
  val >>= 32;
  }
-DPRINTF("read from " TARGET_FMT_plx " val %lx\n", addr, val);
+trace_sun4v_rtc_read(addr, val);
  return val;
  }
  
  static void sun4v_rtc_write(void *opaque, hwaddr addr,

   uint64_t val, unsigned size)
  {
-DPRINTF("write 0x%x to " TARGET_FMT_plx "\n", (unsigned)val, addr);
+trace_sun4v_rtc_read(addr, val);


Uh.. as in v1, it looks like this should be trace_sun4v_rtc_write().


Oops, my bad.  I can fix this manually in case there's a v3 pull
request, but I would really prefer that to be included in a
follow up patch instead of making this block the entire pull
request.


Oops sorry... I noticed you fixed that in 605be3a8c0, thanks!

Regards,

Phil.

Re: [Qemu-devel] [PATCH v2 00/29] target/riscv: Convert to decodetree

[Bastian Koppelmann]

On 10/25/18 12:21 AM, Palmer Dabbelt wrote:
On Sat, 20 Oct 2018 00:14:22 PDT (-0700), 
kbast...@mail.uni-paderborn.de wrote:

Hi,

this patchset converts the RISC-V decoder to decodetree in three 
major steps:


1) Convert 32-bit instructions to decodetree [Patch 1-14]:
    Many of the gen_* functions are called by the decode functions 
for 16-bit

    and 32-bit functions. If we move translation code from the gen_*
    functions to the generated trans_* functions of decode-tree, we 
get a lot of
    duplication. Therefore, we mostly generate calls to the old gen_* 
function

    which are properly replaced after step 2).

    Each of the trans_ functions are grouped into files corresponding 
to their

    ISA extension, e.g. addi which is in RV32I is translated in the file
    'trans_rvi.inc.c'.

2) Convert 16-bit instructions to decodetree [Patch 15-17]:
    All 16 bit instructions have a direct mapping to a 32 bit 
instruction. Thus,
    we convert the arguments in the 16 bit trans_ function to the 
arguments of
    the corresponding 32 bit instruction and call the 32 bit trans_ 
function.


3) Remove old manual decoding in gen_* function [Patch 17-28]:
    this move all manual translation code into the trans_* 
instructions of

    decode tree, such that we can remove the old decode_* functions.

the full tree can be found here:
https://github.com/bkoppelmann/qemu/tree/riscv-dt-v2


Thanks!

I dropped this on top of master and it appears I'm getting a bunch of 
oops when trying to boot Linux.  They're fairly far into the boot 
process and may be a mistake on my end, I was just wondering if you'd 
booted Linux?


Are there non fatal oops in booting Linux? I only checked whether I 
could get a terminal on Fedora Linux.


Cheers,

Bastian

[Qemu-devel] [PATCH RFC] MAINTAINERS: clarify some of the tags

[Cornelia Huck]

The MAINTAINERS file is a bit sparse on information about what
the different designators are. Let's add some more information
to give contributors a better idea about what the different
roles are.

Signed-off-by: Cornelia Huck 
---

This came out of a discussion about what being a 'reviewer' listed in
this file actually means. A reviewer probably should already have a
track record of doing helpful reviews before being listed in here.

While at it, I also tried to add some more hints for the other entries.
This patch is supposed to be a starting point for further discussion.

---
 MAINTAINERS | 5 +
 1 file changed, 5 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 40672c4eba..7d9dbc6724 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12,9 +12,14 @@ consult qemu-devel and not any specific individual privately.
 Descriptions of section entries:
 
M: Mail patches to: FullName 
+  Maintainers are looking after a certain area and must be CCed on
+  patches. They are considered the main contact point.
R: Designated reviewer: FullName 
   These reviewers should be CCed on patches.
+  Reviewers are familiar with the subject matter and provide feedback
+  even though they are not maintainers.
L: Mailing list that is relevant to this area
+  These lists should be CCed on patches.
W: Web-page with status/info
Q: Patchwork web based patch tracking system site
T: SCM tree type and location.  Type is one of: git, hg, quilt, stgit.
-- 
2.14.4

Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU

[Aleksandar Markovic]

> I'm queueing your MADD and MADDU patch...

You don't queue, you submit.

Thanks,
Aleksandar


From: Fredrik Noring 
Sent: Wednesday, October 24, 2018 8:01:15 PM
To: Philippe Mathieu-Daudé
Cc: Maciej W . Rozycki; Richard Henderson; Aleksandar Markovic; Aurelien Jarno; 
qemu-devel@nongnu.org; Jürgen Urban
Subject: Re: [PATCH] target/mips: Support Toshiba specific three-operand MADD 
and MADDU

Hi Philippe,

> The three-operand MADD and MADDU are specific to the
> Toshiba TX19/TX39/TX79 cores.
>
> The "32-Bit TX System RISC TX39 Family Architecture manual"
> is available at https://wiki.qemu.org/File:DSAE0022432.pdf
>
> Signed-off-by: Philippe Mathieu-Daudé

I'm queueing your MADD and MADDU patch, with minor modifications as shown
below, to amend the support for the R5900, based on Aleksandar's latest
v2 tag:

https://github.com/AMarkovic/qemu tags/mips-queue-oct-2018-part-2-v2

It looks like gen_move_{low32,high32} do sign-extend properly. However,
their notes say "sign-extract" as in:

/* Sign-extract the low 32-bits to a target_long.  */
static inline void gen_move_low32(TCGv ret, TCGv_i64 arg)
...
/* Sign-extract the high 32-bits to a target_long.  */
static inline void gen_move_high32(TCGv ret, TCGv_i64 arg)

Perhaps these are typos? Also, looking at the code for tcg_gen_mulu2_i32
and tcg_gen_add2_i32, they don't appear to be particularly more efficient
anyway, in particular since more registers are needed, so let's go with
your version. (A subsequent patch will do MADD1 and MADDU1 as well.)

Thanks!

Fredrik

--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -4801,8 +4801,8 @@ static void gen_muldiv(DisasContext *ctx, uint32_t opc,
 }

 /*
- * These MULT and MULTU instructions implemented in for example the
- * Toshiba/Sony R5900 and the Toshiba TX19, TX39 and TX79 core
+ * These MULT[U] and MADD[U] instructions implemented in for example
+ * the Toshiba/Sony R5900 and the Toshiba TX19, TX39 and TX79 core
  * architectures are special three-operand variants with the syntax
  *
  * MULT[U][1] rd, rs, rt
@@ -4811,6 +4811,14 @@ static void gen_muldiv(DisasContext *ctx, uint32_t opc,
  *
  * (rd, LO, HI) <- rs * rt
  *
+ * and
+ *
+ * MADD[U]rd, rs, rt
+ *
+ * such that
+ *
+ * (rd, LO, HI) <- (LO, HI) + rs * rt
+ *
  * where the low-order 32-bits of the result is placed into both the
  * GPR rd and the special register LO. The high-order 32-bits of the
  * result is placed into the special register HI.
@@ -4867,8 +4875,48 @@ static void gen_mul_txx9(DisasContext *ctx, uint32_t opc,
 tcg_temp_free_i32(t3);
 }
 break;
+case TX79_MMI_MADD:
+{
+TCGv_i64 t2 = tcg_temp_new_i64();
+TCGv_i64 t3 = tcg_temp_new_i64();
+
+tcg_gen_ext_tl_i64(t2, t0);
+tcg_gen_ext_tl_i64(t3, t1);
+tcg_gen_mul_i64(t2, t2, t3);
+tcg_gen_concat_tl_i64(t3, cpu_LO[acc], cpu_HI[acc]);
+tcg_gen_add_i64(t2, t2, t3);
+tcg_temp_free_i64(t3);
+gen_move_low32(cpu_LO[acc], t2);
+gen_move_high32(cpu_HI[acc], t2);
+if (rd) {
+gen_move_low32(cpu_gpr[rd], t2);
+}
+tcg_temp_free_i64(t2);
+}
+break;
+case TX79_MMI_MADDU:
+{
+TCGv_i64 t2 = tcg_temp_new_i64();
+TCGv_i64 t3 = tcg_temp_new_i64();
+
+tcg_gen_ext32u_tl(t0, t0);
+tcg_gen_ext32u_tl(t1, t1);
+tcg_gen_extu_tl_i64(t2, t0);
+tcg_gen_extu_tl_i64(t3, t1);
+tcg_gen_mul_i64(t2, t2, t3);
+tcg_gen_concat_tl_i64(t3, cpu_LO[acc], cpu_HI[acc]);
+tcg_gen_add_i64(t2, t2, t3);
+tcg_temp_free_i64(t3);
+gen_move_low32(cpu_LO[acc], t2);
+gen_move_high32(cpu_HI[acc], t2);
+if (rd) {
+gen_move_low32(cpu_gpr[rd], t2);
+}
+tcg_temp_free_i64(t2);
+}
+break;
 default:
-MIPS_INVAL("mul TXx9");
+MIPS_INVAL("mul/madd TXx9");
 generate_exception_end(ctx, EXCP_RI);
 goto out;
 }
@@ -24699,6 +24747,8 @@ static void decode_tx79_mmi(CPUMIPSState *env, 
DisasContext *ctx)
 break;
 case TX79_MMI_MULT1:
 case TX79_MMI_MULTU1:
+case TX79_MMI_MADD:
+case TX79_MMI_MADDU:
 gen_mul_txx9(ctx, opc, rd, rs, rt);
 break;
 case TX79_MMI_DIV1:
@@ -24713,8 +24763,6 @@ static void decode_tx79_mmi(CPUMIPSState *env, 
DisasContext *ctx)
 case TX79_MMI_MFHI1:
 gen_HILO(ctx, opc, 1, rd);
 break;
-case TX79_MMI_MADD:  /* TODO: TX79_MMI_MADD */
-case TX79_MMI_MADDU: /* TODO: TX79_MMI_MADDU */
 case TX79_MMI_PLZCW: /* TODO: TX79_MMI_PLZCW */
 case TX79_MMI_MADD1: /* TODO: TX79_MMI_MADD1 */
 case TX79_MMI_MADDU1:/* TODO: TX79_MMI_MADDU1 */

Re: [Qemu-devel] [PATCH 2/3] adlib: mark as insecure and deprecated.

[P J P]

+-- On Fri, 26 Oct 2018, Paolo Bonzini wrote --+
| Oh, thanks!  I said I was dumb. :)  So the fix is just this:
| 
| diff --git a/hw/audio/fmopl.h b/hw/audio/fmopl.h
| index e7e578a48e..7199afaa3c 100644
| --- a/hw/audio/fmopl.h
| +++ b/hw/audio/fmopl.h
| @@ -72,8 +72,8 @@ typedef struct fm_opl_f {
|   /* Rhythm sention */
|   uint8_t rhythm; /* Rhythm mode , key flag */
|   /* time tables */
| - int32_t AR_TABLE[75];   /* atttack rate tables */
| - int32_t DR_TABLE[75];   /* decay rate tables   */
| + int32_t AR_TABLE[76];   /* atttack rate tables */
| + int32_t DR_TABLE[76];   /* decay rate tables   */
|   uint32_t FN_TABLE[1024];  /* fnumber -> increment counter */
|   /* LFO */
|   int32_t *ams_table;
| 
| and init_timetables will just fill it with the right value?  (I checked
| against another implementation at http://opl3.cozendey.com/).

Gerd has proposed to a patch to deprecate adlib, as it's not used as much. IMO 
deprecation is better option. But if that is not happening, above seems good.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [libvirt] [PATCH 2/3] adlib: mark as insecure and deprecated.

[P J P]

+-- On Fri, 26 Oct 2018, Daniel P. Berrangé wrote --+
| > No, since the adlib device is not used as much and is being deprecated, I'm 
| > not inclined to get one.
| 
| Any security issue that affects code in QEMU that is currently being
| shipped by distros should have a CVE.
| 
| Whether we intend to deprecate & delete it later should not be a factor
| because we are free to cancel the deprecation process at any time if we
| find a reason to keep the feature around.

Okay, will follow up with a CVE process. Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

[Qemu-devel] [PATCH v2] ppc/pnv: check size before data buffer access

[P J P]

From: Prasad J Pandit 

While performing PowerNV memory r/w operations, the access length
'sz' could exceed the data[4] buffer size. Add check to avoid OOB
access.

Reported-by: Moguofang 
Signed-off-by: Prasad J Pandit 
---
 hw/ppc/pnv_lpc.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Update v2: add error log message
  -> https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg05750.html

diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
index d7721320a2..172a915cfc 100644
--- a/hw/ppc/pnv_lpc.c
+++ b/hw/ppc/pnv_lpc.c
@@ -155,9 +155,15 @@ static void pnv_lpc_do_eccb(PnvLpcController *lpc, 
uint64_t cmd)
 /* XXX Check for magic bits at the top, addr size etc... */
 unsigned int sz = (cmd & ECCB_CTL_SZ_MASK) >> ECCB_CTL_SZ_LSH;
 uint32_t opb_addr = cmd & ECCB_CTL_ADDR_MASK;
-uint8_t data[4];
+uint8_t data[8];
 bool success;
 
+if (sz > sizeof(data)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+"ECCB: invalid operation at @0x%08x size %d\n", opb_addr, sz);
+return;
+}
+
 if (cmd & ECCB_CTL_READ) {
 success = opb_read(lpc, opb_addr, data, sz);
 if (success) {
-- 
2.17.2

Re: [Qemu-devel] [PATCH v3 2/7] qapi: correctly parse uint64_t values from strings

[David Hildenbrand]

> 
> It's not obvious to me why this looks so different from the code in
> parse_type_int64().  Should we be using qemu_strtoi64() in the
> pre-existing function, instead of what's there now?

The existing function has to be that complicated because it calls into
the same function used to parse ranges. We don't need ranges (or
create/modify) any, so this is not necessary.

This function is similar to the other parse functions (not parsing
ranges), e.g. parse_type_bool(). Thanks!

> 
>>  
>>  static void parse_type_size(Visitor *v, const char *name, uint64_t *obj,
> 


-- 

Thanks,

David / dhildenb

[Qemu-devel] [PATCH v6 3/3] x86: define a new MSR based feature word -- FEATURE_WORDS_ARCH_CAPABILITIES

[Robert Hoo]

Note RSBA is specially treated -- no matter host support it or not, qemu
pretends it is supported.

Changes in v6: filter out MSR features whose dependent CPUID enumeration is not
there.

Signed-off-by: Robert Hoo 
Reviewed-by: Eduardo Habkost 
---
 target/i386/cpu.c | 31 ++-
 target/i386/cpu.h |  8 
 target/i386/kvm.c | 11 +++
 3 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 0de21fa..6371722 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1141,6 +1141,27 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] 
= {
 },
 .tcg_features = ~0U,
 },
+/*Below are MSR exposed features*/
+[FEAT_ARCH_CAPABILITIES] = {
+.type = MSR_FEATURE_WORD,
+.feat_names = {
+"rdctl-no", "ibrs-all", "rsba", "skip-l1dfl-vmentry",
+"ssb-no", NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+},
+.msr = {
+.index = MSR_IA32_ARCH_CAPABILITIES,
+.cpuid_dep = {
+FEAT_7_0_EDX,
+CPUID_7_0_EDX_ARCH_CAPABILITIES
+}
+},
+},
 };
 
 typedef struct X86RegisterInfo32 {
@@ -3696,7 +3717,15 @@ static uint32_t 
x86_cpu_get_supported_feature_word(FeatureWord w,
 wi->cpuid.reg);
 break;
 case MSR_FEATURE_WORD:
-r = kvm_arch_get_supported_msr_feature(kvm_state, wi->msr.index);
+/* Special case:
+ * No matter host status, IA32_ARCH_CAPABILITIES.RSBA [bit 2]
+ * is always supported in guest.
+ */
+if (wi->msr.index == MSR_IA32_ARCH_CAPABILITIES) {
+r = MSR_ARCH_CAP_RSBA;
+}
+r |= kvm_arch_get_supported_msr_feature(kvm_state,
+wi->msr.index);
 break;
 }
 } else if (hvf_enabled()) {
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index dd3de97..ff1ae32 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -507,6 +507,7 @@ typedef enum FeatureWord {
 FEAT_XSAVE_COMP_LO, /* CPUID[EAX=0xd,ECX=0].EAX */
 FEAT_XSAVE_COMP_HI, /* CPUID[EAX=0xd,ECX=0].EDX */
 MSR_FEATURE_WORD_BEGIN, /* Define MSR feature words below */
+FEAT_ARCH_CAPABILITIES = MSR_FEATURE_WORD_BEGIN,
 FEATURE_WORDS,
 } FeatureWord;
 
@@ -735,6 +736,13 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
 #define CPUID_TOPOLOGY_LEVEL_SMT  (1U << 8)
 #define CPUID_TOPOLOGY_LEVEL_CORE (2U << 8)
 
+/* MSR Feature Bits */
+#define MSR_ARCH_CAP_RDCL_NO(1U << 0)
+#define MSR_ARCH_CAP_IBRS_ALL   (1U << 1)
+#define MSR_ARCH_CAP_RSBA   (1U << 2)
+#define MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY (1U << 3)
+#define MSR_ARCH_CAP_SSB_NO (1U << 4)
+
 #ifndef HYPERV_SPINLOCK_NEVER_RETRY
 #define HYPERV_SPINLOCK_NEVER_RETRY 0x
 #endif
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 161fc38..796a049 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1975,6 +1975,17 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
 }
 #endif
 
+/* If host supports feature MSR, write down. */
+if (kvm_feature_msrs) {
+int i;
+for (i = 0; i < kvm_feature_msrs->nmsrs; i++)
+if (kvm_feature_msrs->indices[i] == MSR_IA32_ARCH_CAPABILITIES) {
+kvm_msr_entry_add(cpu, MSR_IA32_ARCH_CAPABILITIES,
+  env->features[FEAT_ARCH_CAPABILITIES]);
+break;
+}
+}
+
 /*
  * The following MSRs have side effects on the guest or are too heavy
  * for normal writeback. Limit them to reset or full state updates.
-- 
1.8.3.1

[Qemu-devel] [PATCH v6 1/3] kvm: Add support to KVM_GET_MSR_FEATURE_INDEX_LIST and KVM_GET_MSRS system ioctl

[Robert Hoo]

Add kvm_get_supported_feature_msrs() to get supported MSR feature index list.
Add kvm_arch_get_supported_msr_feature() to get each MSR features value.

Signed-off-by: Robert Hoo 
Reviewed-by: Eduardo Habkost 
---
 include/sysemu/kvm.h |  2 ++
 target/i386/kvm.c| 80 
 2 files changed, 82 insertions(+)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 0b64b8e..97d8d9d 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -463,6 +463,8 @@ int kvm_vm_check_extension(KVMState *s, unsigned int 
extension);
 
 uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
   uint32_t index, int reg);
+uint32_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index);
+
 
 void kvm_set_sigmask_len(KVMState *s, unsigned int sigmask_len);
 
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 115d8b4..161fc38 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -107,6 +107,7 @@ static int has_pit_state2;
 static bool has_msr_mcg_ext_ctl;
 
 static struct kvm_cpuid2 *cpuid_cache;
+static struct kvm_msr_list *kvm_feature_msrs;
 
 int kvm_has_pit_state2(void)
 {
@@ -420,6 +421,42 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, 
uint32_t function,
 return ret;
 }
 
+uint32_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index)
+{
+struct {
+struct kvm_msrs info;
+struct kvm_msr_entry entries[1];
+} msr_data;
+uint32_t ret;
+
+if (kvm_feature_msrs == NULL) { /* Host doesn't support feature MSRs */
+return 0;
+}
+
+/* Check if requested MSR is supported feature MSR */
+int i;
+for (i = 0; i < kvm_feature_msrs->nmsrs; i++)
+if (kvm_feature_msrs->indices[i] == index) {
+break;
+}
+if (i == kvm_feature_msrs->nmsrs) {
+return 0; /* if the feature MSR is not supported, simply return 0 */
+}
+
+msr_data.info.nmsrs = 1;
+msr_data.entries[0].index = index;
+
+ret = kvm_ioctl(s, KVM_GET_MSRS, &msr_data);
+if (ret != 1) {
+error_report("KVM get MSR (index=0x%x) feature failed, %s",
+index, strerror(-ret));
+exit(1);
+}
+
+return msr_data.entries[0].data;
+}
+
+
 typedef struct HWPoisonPage {
 ram_addr_t ram_addr;
 QLIST_ENTRY(HWPoisonPage) list;
@@ -1286,6 +1323,47 @@ void kvm_arch_do_init_vcpu(X86CPU *cpu)
 }
 }
 
+static int kvm_get_supported_feature_msrs(KVMState *s)
+{
+int ret = 0;
+
+if (kvm_feature_msrs != NULL) {
+return 0;
+}
+
+if (!kvm_check_extension(s, KVM_CAP_GET_MSR_FEATURES)) {
+return 0;
+}
+
+struct kvm_msr_list msr_list;
+
+msr_list.nmsrs = 0;
+ret = kvm_ioctl(s, KVM_GET_MSR_FEATURE_INDEX_LIST, &msr_list);
+if (ret < 0 && ret != -E2BIG) {
+error_report("Fetch KVM feature MSR list failed: %s",
+strerror(-ret));
+return ret;
+}
+
+assert(msr_list.nmsrs > 0);
+kvm_feature_msrs = (struct kvm_msr_list *) \
+g_malloc0(sizeof(msr_list) +
+ msr_list.nmsrs * sizeof(msr_list.indices[0]));
+
+kvm_feature_msrs->nmsrs = msr_list.nmsrs;
+ret = kvm_ioctl(s, KVM_GET_MSR_FEATURE_INDEX_LIST, kvm_feature_msrs);
+
+if (ret < 0) {
+error_report("Fetch KVM feature MSR list failed: %s",
+strerror(-ret));
+g_free(kvm_feature_msrs);
+kvm_feature_msrs = NULL;
+return ret;
+}
+
+return 0;
+}
+
 static int kvm_get_supported_msrs(KVMState *s)
 {
 static int kvm_supported_msrs;
@@ -1439,6 +1517,8 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
 return ret;
 }
 
+kvm_get_supported_feature_msrs(s);
+
 uname(&utsname);
 lm_capable_kernel = strcmp(utsname.machine, "x86_64") == 0;
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v6 0/3] x86: QEMU side support on MSR based features

[Robert Hoo]

KVM side has added the framework (kvm.git:d1d93fa90) to support MSR based 
features.
Here is the QEMU part, including data structure changes/expanding, referring
functions changes, and the implementations on 
KVM_GET_MSR_FEATURE_INDEX_LIST and KVM_GET_MSRS system ioctl.

Changelog:
v6: In cpu feature filtering, filter out MSR features whose CPUID feature
dependency is not there.
Check feature word type for other accelerator, like hvf, for otherwise 
it
would return bogus EAX/ECX values in x86_cpu_get_supported_feature_word().
v5: Re-order patches. Complement feature MSR set routines.
v4:
Re-organize patch set to conform to request of individually build pass.
Add KVM capability check for KVM_GET_MSR_INDEX_LIST before fetch.
Special treatment for MSR_IA32_ARCH_CAPABILITIES.RSBA.
Use more convenient glib wrapper (g_strdup_printf) instead of native
(sprintf).

v3: patch 2&3 in v2 are corrupted. Re-format patches.
v2: coding style changes to pass ./scripts/checkpatch.pl.

Robert Hoo (3):
  kvm: Add support to KVM_GET_MSR_FEATURE_INDEX_LIST and KVM_GET_MSRS
system ioctl
  x86: Data structure changes to support MSR based features
  x86: define a new MSR based feature word --
FEATURE_WORDS_ARCH_CAPABILITIES

 include/sysemu/kvm.h |   2 +
 target/i386/cpu.c| 234 +++
 target/i386/cpu.h|  15 +++-
 target/i386/kvm.c|  91 
 4 files changed, 289 insertions(+), 53 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v6 2/3] x86: Data structure changes to support MSR based features

[Robert Hoo]

Add FeatureWordType indicator in struct FeatureWordInfo.
Change feature_word_info[] accordingly.
Change existing functions that refer to feature_word_info[] accordingly.

Signed-off-by: Robert Hoo 
---
 target/i386/cpu.c | 205 --
 target/i386/cpu.h |   7 +-
 2 files changed, 159 insertions(+), 53 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 1469a1b..0de21fa 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -770,17 +770,36 @@ static void x86_cpu_vendor_words2str(char *dst, uint32_t 
vendor1,
   /* missing:
   CPUID_XSAVE_XSAVEC, CPUID_XSAVE_XSAVES */
 
+typedef enum FeatureWordType {
+   CPUID_FEATURE_WORD,
+   MSR_FEATURE_WORD,
+} FeatureWordType;
+
 typedef struct FeatureWordInfo {
+FeatureWordType type;
 /* feature flags names are taken from "Intel Processor Identification and
  * the CPUID Instruction" and AMD's "CPUID Specification".
  * In cases of disagreement between feature naming conventions,
  * aliases may be added.
  */
 const char *feat_names[32];
-uint32_t cpuid_eax;   /* Input EAX for CPUID */
-bool cpuid_needs_ecx; /* CPUID instruction uses ECX as input */
-uint32_t cpuid_ecx;   /* Input ECX value for CPUID */
-int cpuid_reg;/* output register (R_* constant) */
+union {
+/* If type==CPUID_FEATURE_WORD */
+struct {
+uint32_t eax;   /* Input EAX for CPUID */
+bool needs_ecx; /* CPUID instruction uses ECX as input */
+uint32_t ecx;   /* Input ECX value for CPUID */
+int reg;/* output register (R_* constant) */
+} cpuid;
+/* If type==MSR_FEATURE_WORD */
+struct {
+uint32_t index;
+struct {   /*CPUID that enumerate this MSR*/
+FeatureWord cpuid_class;
+uint32_tcpuid_flag;
+} cpuid_dep;
+} msr;
+};
 uint32_t tcg_features; /* Feature flags supported by TCG */
 uint32_t unmigratable_flags; /* Feature flags known to be unmigratable */
 uint32_t migratable_flags; /* Feature flags known to be migratable */
@@ -790,6 +809,7 @@ typedef struct FeatureWordInfo {
 
 static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
 [FEAT_1_EDX] = {
+.type = CPUID_FEATURE_WORD,
 .feat_names = {
 "fpu", "vme", "de", "pse",
 "tsc", "msr", "pae", "mce",
@@ -800,10 +820,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = 
{
 "fxsr", "sse", "sse2", "ss",
 "ht" /* Intel htt */, "tm", "ia64", "pbe",
 },
-.cpuid_eax = 1, .cpuid_reg = R_EDX,
+.cpuid = {.eax = 1, .reg = R_EDX, },
 .tcg_features = TCG_FEATURES,
 },
 [FEAT_1_ECX] = {
+.type = CPUID_FEATURE_WORD,
 .feat_names = {
 "pni" /* Intel,AMD sse3 */, "pclmulqdq", "dtes64", "monitor",
 "ds-cpl", "vmx", "smx", "est",
@@ -814,7 +835,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
 "tsc-deadline", "aes", "xsave", NULL /* osxsave */,
 "avx", "f16c", "rdrand", "hypervisor",
 },
-.cpuid_eax = 1, .cpuid_reg = R_ECX,
+.cpuid = { .eax = 1, .reg = R_ECX, },
 .tcg_features = TCG_EXT_FEATURES,
 },
 /* Feature names that are already defined on feature_name[] but
@@ -823,6 +844,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
  * to features[FEAT_8000_0001_EDX] if and only if CPU vendor is AMD.
  */
 [FEAT_8000_0001_EDX] = {
+.type = CPUID_FEATURE_WORD,
 .feat_names = {
 NULL /* fpu */, NULL /* vme */, NULL /* de */, NULL /* pse */,
 NULL /* tsc */, NULL /* msr */, NULL /* pae */, NULL /* mce */,
@@ -833,10 +855,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = 
{
 NULL /* fxsr */, "fxsr-opt", "pdpe1gb", "rdtscp",
 NULL, "lm", "3dnowext", "3dnow",
 },
-.cpuid_eax = 0x8001, .cpuid_reg = R_EDX,
+.cpuid = { .eax = 0x8001, .reg = R_EDX, },
 .tcg_features = TCG_EXT2_FEATURES,
 },
 [FEAT_8000_0001_ECX] = {
+.type = CPUID_FEATURE_WORD,
 .feat_names = {
 "lahf-lm", "cmp-legacy", "svm", "extapic",
 "cr8legacy", "abm", "sse4a", "misalignsse",
@@ -847,7 +870,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
 "perfctr-nb", NULL, NULL, NULL,
 NULL, NULL, NULL, NULL,
 },
-.cpuid_eax = 0x8001, .cpuid_reg = R_ECX,
+.cpuid = { .eax = 0x8001, .reg = R_ECX, },
 .tcg_features = TCG_EXT3_FEATURES,
 /*
  * TOPOEXT is always allowed but can't be enabled blindly by
@@ -857,6 +880,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
 .no_autoenable_flags = CPUID_EXT3_TOPOEXT,
 },
 [FEAT_C000_0001_EDX] = {
+.t

Re: [Qemu-devel] [PATCH RFC 0/2] Fix migration issues

[Fei Li]

On 10/25/2018 08:55 PM, Dr. David Alan Gilbert wrote:

* Fei Li (f...@suse.com) wrote:

Hi,
these two patches are to fix live migration issues. The first is
about multifd, and the second is to fix some error handling.

But I have a question about using multifd migration.
In our current code, when multifd is used during migration, if there
is an error before the destination receives all new channels (I mean
multifd_recv_new_channel(ioc)), the destination does not exit but
keeps waiting (Hang in recvmsg() in qio_channel_socket_readv) until
the source exits.

My question is about the state of the destination host if fails during
this period. I did a test, after applying [1/2] patch, if
multifd_new_send_channel_async() fails, the destination host hangs for
a while then later pops up a window saying
 "'QEMU (...) [stopped]' is not responding.
 You may choose to wait a short while for it to continue or force
 the application to quit entirely."
But after closing the window by clicking, the qemu on the dest still
hangs there until I exclusively kill the qemu on the source.

That sounds like the main thread is blocked for some reason?

Yes, the main thread on  the dst is keeps looping.

But I don't
normally use the window setup;  if you try with -nographic and can see
the HMP (or a QMP) monitor, can you see if the monitor still responds?


Thanks for the `-nographic` reminder, I harvested an interesting 
phenonmenon:

If I do the `migrate -d tcp:ip_addr:port` before the guest's graphic appears
(it's dark now), there is no hang and the guest starts up properly later.
But if I do the live migration after the guest fully starts up, I mean when
I can operate something using my mouse inside the guest, the hang
situation is there.
This is true for using `-nographic` for both src and dst,
and using `-nographic` for only src or dst.


The hang phenonmenon is that the dst seems never responds (I
waited three minutes), and the cursor just keeps flashing. After I
exclusively kill the src, then the dst quit. Just as follows:
(Same result if gdb is not used in src)
src:
(qemu) ...
(qemu) q
(gdb) q
dst:
(qemu) Up to now, dst has received the 0 channel
Up to now, dst has received the 1 channel

(qemu)
(qemu)

To check the migtation state in the src:
(qemu) info migrate
globals:
store-global-state: on
only-migratable: off
send-configuration: on
send-section-footer: on
decompress-error-check: on
capabilities: xbzrle: off rdma-pin-all: off auto-converge: off 
zero-blocks: off compress: off events: off postcopy-ram: off x-colo: off 
release-ram: off block: off return-path: off pause-before-switchover: 
off x-multifd: on dirty-bitmaps: off postcopy-blocktime: off 
late-block-activate: off
Migration status: setup /* I added some codes to set the status to 
"failed", but still not working, details see below */

total time: 0 milliseconds

I guess maybe the source should to proactive to tell the dst and
disconnects from the source side, so I tried to set the above
"Migration status" to be "failed", and use qemu_fclose(s->to_dst_file)
when multifd_new_send_channel_async() fails.
(BTW: I even tried:
 if (s->vm_was_running) {   vm_start();   }   )
But the hang situation is still there.

If it doesn't then try and get a backtrace.

The monitor really shouldn't block, so it would be interesting to see.

Dave
I set two breakpoints and get the following backtrace, hope they can 
help. :)


Thread 1 "qemu-system-x86" hit Breakpoint 1, multifd_recv_new_channel (
    ioc=0x57995af0) at /build/gitcode/qemu-build/migration/ram.c:1368
1368    {
(gdb) c
Continuing.

Thread 1 "qemu-system-x86" hit Breakpoint 2, qio_channel_socket_readv (
    ioc=0x57995af0, iov=0x568777d0, niov=1, fds=0x0, nfds=0x0,
    errp=0x7fffdb38) at io/channel-socket.c:463
463    {
(gdb) n
464        QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(ioc);
(gdb)
..
483     retry:
(gdb)
484        ret = recvmsg(sioc->fd, &msg, sflags);
(gdb) bt
#0  qio_channel_socket_readv (ioc=0x57995af0, iov=0x568777d0, 
niov=1,

    fds=0x0, nfds=0x0, errp=0x7fffdb38) at io/channel-socket.c:484
#1  0x55d156c5 in qio_channel_readv_full (ioc=0x57995af0,
    iov=0x568777d0, niov=1, fds=0x0, nfds=0x0, errp=0x7fffdb38)
    at io/channel.c:65
#2  0x55d15b26 in qio_channel_readv (ioc=0x57995af0,
    iov=0x568777d0, niov=1, errp=0x7fffdb38) at io/channel.c:197
#3  0x55d15853 in qio_channel_readv_all_eof (ioc=0x57995af0,
    iov=0x7fffda70, niov=1, errp=0x7fffdb38) at io/channel.c:106
#4  0x55d1595c in qio_channel_readv_all (ioc=0x57995af0,
    iov=0x7fffda70, niov=1, errp=0x7fffdb38) at io/channel.c:142
#5  0x55d15d0c in qio_channel_read_all (ioc=0x57995af0,
    buf=0x7fffdad0 "\340\"zVUU", buflen=25, errp=0x7fffdb38)
    at io/channel.c:246
#6  0x5587695c in multifd_recv_initial_packet (c=0x57995af0,
    errp=0x7fffdb38) at /build/gitcode/qemu-bu

Re: [Qemu-devel] [PATCH RFC 0/2] Fix migration issues

[Fei Li]

On 10/25/2018 08:58 PM, Peter Xu wrote:

On Thu, Oct 25, 2018 at 05:04:00PM +0800, Fei Li wrote:

[...]


@@ -1325,22 +1325,24 @@ bool multifd_recv_all_channels_created(void)
  /* Return true if multifd is ready for the migration, otherwise false */
  bool multifd_recv_new_channel(QIOChannel *ioc)
  {
+    MigrationIncomingState *mis = migration_incoming_get_current();
  MultiFDRecvParams *p;
  Error *local_err = NULL;
  int id;

  id = multifd_recv_initial_packet(ioc, &local_err);
  if (id < 0) {
-    multifd_recv_terminate_threads(local_err);
-    return false;
+    error_reportf_err(local_err,
+  "failed to receive packet via multifd channel %x:
",
+  multifd_recv_state->count);
+    goto fail;
  }

  p = &multifd_recv_state->params[id];
  if (p->c != NULL) {
  error_setg(&local_err, "multifd: received id '%d' already setup'",
     id);
-    multifd_recv_terminate_threads(local_err);
-    return false;
+    goto fail;
  }
  p->c = ioc;
  object_ref(OBJECT(ioc));
@@ -1352,6 +1354,11 @@ bool multifd_recv_new_channel(QIOChannel *ioc)
     QEMU_THREAD_JOINABLE);
  atomic_inc(&multifd_recv_state->count);
  return multifd_recv_state->count == migrate_multifd_channels();
+fail:
+    multifd_recv_terminate_threads(local_err);
+    qemu_fclose(mis->from_src_file);
+    mis->from_src_file = NULL;
+    exit(EXIT_FAILURE);
  }

Yeah I think it makes sense to at least report some details when error
happens, but I'm not sure whether it's good to explicitly exit() here.
IMHO you can add an Error** in multifd_recv_new_channel() parameter
list to do that, and even through migration_ioc_process_incoming().
What do you think?

Regards,


You mean exit() in migration_ioc_process_incoming(), or further
caller migration_channel_process_incoming()? Actually either is
ok for me. :) But today I find if using postcopy and multifd together
to do live migration, it seems the hang still occurs even with the
above codes, so sad about that. I will keep debugging and see
how to fix this.

Have a nice day, thanks
Fei

Re: [Qemu-devel] [PATCH v8 00/38] target/mips: Limited support for the R5900

[Maciej W. Rozycki]

On Fri, 26 Oct 2018, Richard Henderson wrote:

> > Overall this source file is clearly a modified copy of an ancient version 
> > of the opcode table included with the opcodes library from binutils and I 
> > think it would benefit from a refresh.
> 
> You can't do that because of GPL v3, sadly.

 I've been aware of that, however the changes I mentioned are pretty 
mechanical and can be easily made from scratch by someone them who hasn't 
looked at binutils, even based on the description I already made.  You 
don't copyright an idea, only actual written code.

  Maciej

Re: [Qemu-devel] [libvirt] [PATCH 3/3] cirrus: mark as deprecated

[Christian Borntraeger]

On 10/26/2018 11:42 AM, Daniel P. Berrangé wrote:
> On Fri, Oct 26, 2018 at 12:33:55PM +0530, P J P wrote:
>>   Hello Dan, all
>>
>> +-- On Thu, 25 Oct 2018, Daniel P. Berrangé wrote --+
>> | On Thu, Oct 25, 2018 at 10:52:56AM +0200, Gerd Hoffmann wrote:
>> | > While being at it deprecate cirrus too.
>> | > 
>> | > Reason (short version): use stdvga instead.
>> | > Verbose version:
>> | > 
>> https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful
>> | 
>> | 
>> | I don't debate the points in the blog post above that stdvga is a
>> | better choice, but I don't think that's enough to justify deprecating
>> | cirrus at this point in time, because when it then gets deleted it
>> | will break way too many existing deployments.
>> | 
>> | We need to socialize info in that blog post above more widely and
>> | especially ensure that apps are not using that by default. I don't
>> | see it being viable to formally deprecate it in QEMU any time soon
>> | though given existing usage.
>>
>> To note, IMO there are other devices/sources in QEMU which are potential 
>> candidates for deprecation, similar to adlib etc. It'll help if we could 
>> device a process to deprecate/remove such code base. Other than maintenance 
>> it 
>> invariably also becomes source of security issues.
>>
>> Ex.(similar to Fedora) we could announce such candidate on qemu-devel list 
>> and 
>> after review over a period of say a month, candidate will be
>> deprecated/expunged. (thinking aloud)
> 
> QEMU has a deprecation process:
> 
>   https://qemu.weilnetz.de/doc/qemu-doc.html#Deprecated-features
> 
> Most of the stuff deprecated is CLI args / monitor commands, etc where
> mgmt apps just adjust the way they are calling QEMU, so end user's VMs
> are largely not impacted.
> 
> Deprecating a device type that is widely used is not desirable because
> that will cause breakage of existing guests.  Distros are free to disable
> devices in their builds if they want to reduce the scope for CVEs in
> packages they maintain, but again they should think carefully about how
> many users they are going to break by doing so.

I agree with what Daniel said. Deprecating something that is in heavy use 
by users just because we have trouble maintaining it not going to help the
QEMU project - quite the opposite. 

Re: [Qemu-devel] [PULL v2 00/28] pci, pc, virtio: fixes, features

[Singh, Brijesh]

On 10/25/2018 07:59 PM, Michael S. Tsirkin wrote:
> On Thu, Oct 25, 2018 at 08:16:44PM +0100, Peter Maydell wrote:
>> On 25 October 2018 at 01:52, Michael S. Tsirkin  wrote:
>>> The following changes since commit 13399aad4fa87b2878c49d02a5d3bafa6c966ba3:
>>>
>>>Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-10-22' 
>>> into staging (2018-10-23 17:20:23 +0100)
>>>
>>> are available in the Git repository at:
>>>
>>>git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>>>
>>> for you to fetch changes up to 6a9fb4e1ba5594cde7739068617ad88e6117db93:
>>>
>>>vhost-scsi: prevent using uninitialized vqs (2018-10-24 20:50:13 -0400)
>>>
>>> 
>>> pci, pc, virtio: fixes, features
>>>
>>> AMD IOMMU VAPIC support + fixes all over the place.
>>>
>>> Signed-off-by: Michael S. Tsirkin 
>>
>> Hi; I get some compile failures and a test assertion, I'm afraid:
>>
>> On 32-bit hosts (where uint64_t and size_t are not the same):
>>
>> /home/peter.maydell/qemu/include/qemu/compiler.h:80:35: error: invalid
>> operands to binary - (have 'uint64_t * {aka long long unsigned int *}'
>> and 'size_t * {aka unsigned int *}')
>>   #define type_check(t1,t2) ((t1*)0 - (t2*)0)
>> ^
>> /home/peter.maydell/qemu/include/hw/qdev-properties.h:77:15: note: in
>> expansion of macro 'type_check'
>>   + type_check(_type, typeof_field(_state, _field)),  \
>> ^
>> /home/peter.maydell/qemu/include/hw/qdev-properties.h:168:5: note: in
>> expansion of macro 'DEFINE_PROP_UNSIGNED'
>>   DEFINE_PROP_UNSIGNED(_n, _s, _f, _d, qdev_prop_size, uint64_t)
>>   ^
>> /home/peter.maydell/qemu/hw/misc/pci-testdev.c:322:5: note: in
>> expansion of macro 'DEFINE_PROP_SIZE'
>>   DEFINE_PROP_SIZE("membar", PCITestDevState, membar_size, 0),
>>   ^
>> /home/peter.maydell/qemu/rules.mak:69: recipe for target
>> 'hw/misc/pci-testdev.o' failed
>>
>> On the Windows w64 cross-compile:
>>
>> In file included from 
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c:26:0:
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c: In function
>> 'amdvi_int_remap_msi':
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.h:247:46: error: left
>> shift count >= width of type [-Werror=shift-count-overflow]
>>   #define AMDVI_DEV_NMI_PASS_MASK (1UL << 58)
>>^
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c:1281:25: note: in
>> expansion of macro 'AMDVI_DEV_NMI_PASS_MASK'
>>   pass = dte[3] & AMDVI_DEV_NMI_PASS_MASK;
>>   ^
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.h:245:46: error: left
>> shift count >= width of type [-Werror=shift-count-overflow]
>>   #define AMDVI_DEV_INT_PASS_MASK (1UL << 56)
>>^
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c:1285:25: note: in
>> expansion of macro 'AMDVI_DEV_INT_PASS_MASK'
>>   pass = dte[3] & AMDVI_DEV_INT_PASS_MASK;
>>   ^
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.h:246:46: error: left
>> shift count >= width of type [-Werror=shift-count-overflow]
>>   #define AMDVI_DEV_EINT_PASS_MASK(1UL << 57)
>>^
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c:1289:25: note: in
>> expansion of macro 'AMDVI_DEV_EINT_PASS_MASK'
>>   pass = dte[3] & AMDVI_DEV_EINT_PASS_MASK;
>>   ^
>>
>> These should presumably all be "ULL". (The "UL" suffix is
>> usually a bug, as it's either unnecessary or should be ULL.)
> 
> Yea.  Fixed. Brijesh could you start cleaning up that header generally?
> It has all kind of weird code like using bitfields for hardware
> accesses. That isn't portable - switch to full dword fields with shift
> and | to operate them and proper cpu_to_le APIs or similar please.
> 


Noted, I will look into cleaning up this and send patches for reviews.

thanks

Re: [Qemu-devel] Guest application exit point.

[Alex Bennée]

Rafael K. V. Maeda  writes:

> Hi,
>
> I am implementing a plugin that allocates several resources. I need to
> cleanup some of these resources when QEMU finishes executing the guest
> application. Where is the best exit point to place my cleanup
> functions?
>
> I have tried registering a function call "atexit" but it does not seem
> to work. Any suggestions? At the moment, I am looking for a solution
> for x86_64 (user emulation mode).

For linux-user have a look at preexit_cleanup() in linux-user/exit.c

>
> Kind regards,
> Rafael


--
Alex Bennée

Re: [Qemu-devel] [PATCH RFC 0/2] Fix migration issues

[Peter Xu]

On Fri, Oct 26, 2018 at 09:10:19PM +0800, Fei Li wrote:
> 
> 
> On 10/25/2018 08:58 PM, Peter Xu wrote:
> > On Thu, Oct 25, 2018 at 05:04:00PM +0800, Fei Li wrote:
> > 
> > [...]
> > 
> > > @@ -1325,22 +1325,24 @@ bool multifd_recv_all_channels_created(void)
> > >   /* Return true if multifd is ready for the migration, otherwise false */
> > >   bool multifd_recv_new_channel(QIOChannel *ioc)
> > >   {
> > > +    MigrationIncomingState *mis = migration_incoming_get_current();
> > >   MultiFDRecvParams *p;
> > >   Error *local_err = NULL;
> > >   int id;
> > > 
> > >   id = multifd_recv_initial_packet(ioc, &local_err);
> > >   if (id < 0) {
> > > -    multifd_recv_terminate_threads(local_err);
> > > -    return false;
> > > +    error_reportf_err(local_err,
> > > +  "failed to receive packet via multifd channel 
> > > %x:
> > > ",
> > > +  multifd_recv_state->count);
> > > +    goto fail;
> > >   }
> > > 
> > >   p = &multifd_recv_state->params[id];
> > >   if (p->c != NULL) {
> > >   error_setg(&local_err, "multifd: received id '%d' already 
> > > setup'",
> > >      id);
> > > -    multifd_recv_terminate_threads(local_err);
> > > -    return false;
> > > +    goto fail;
> > >   }
> > >   p->c = ioc;
> > >   object_ref(OBJECT(ioc));
> > > @@ -1352,6 +1354,11 @@ bool multifd_recv_new_channel(QIOChannel *ioc)
> > >      QEMU_THREAD_JOINABLE);
> > >   atomic_inc(&multifd_recv_state->count);
> > >   return multifd_recv_state->count == migrate_multifd_channels();
> > > +fail:
> > > +    multifd_recv_terminate_threads(local_err);
> > > +    qemu_fclose(mis->from_src_file);
> > > +    mis->from_src_file = NULL;
> > > +    exit(EXIT_FAILURE);
> > >   }
> > Yeah I think it makes sense to at least report some details when error
> > happens, but I'm not sure whether it's good to explicitly exit() here.
> > IMHO you can add an Error** in multifd_recv_new_channel() parameter
> > list to do that, and even through migration_ioc_process_incoming().
> > What do you think?
> > 
> > Regards,
> > 
> You mean exit() in migration_ioc_process_incoming(), or further
> caller migration_channel_process_incoming()? Actually either is
> ok for me. :) But today I find if using postcopy and multifd together
> to do live migration, it seems the hang still occurs even with the
> above codes, so sad about that. I will keep debugging and see
> how to fix this.

Maybe you can move the error_report_err() in
migration_channel_process_incoming() out of the TLS path so we can
report the error if either TLS or non-TLS case got something wrong.

And I don't even know whether multifd could work with postcopy...

Regards,

-- 
Peter Xu

Re: [Qemu-devel] [RFC v4 36/71] arm: convert to cpu_interrupt_request

[Alex Bennée]

Emilio G. Cota  writes:

> Cc: Peter Maydell 

This will need to catch-up in the next re-base as there is a merge conflict.

> Cc: qemu-...@nongnu.org
> Reviewed-by: Richard Henderson 
> Signed-off-by: Emilio G. Cota 
> ---
>  target/arm/cpu.c|  2 +-
>  target/arm/helper.c | 12 +---
>  2 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 9c5cda8eb7..7330c2dae1 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -49,7 +49,7 @@ static bool arm_cpu_has_work(CPUState *cs)
>  ARMCPU *cpu = ARM_CPU(cs);
>
>  return (cpu->power_state != PSCI_OFF)
> -&& cs->interrupt_request &
> +&& cpu_interrupt_request(cs) &
>  (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD
>   | CPU_INTERRUPT_VFIQ | CPU_INTERRUPT_VIRQ
>   | CPU_INTERRUPT_EXITTB);
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index c83f7c1109..454954a56c 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -1294,11 +1294,12 @@ static uint64_t isr_read(CPUARMState *env, const 
> ARMCPRegInfo *ri)
>  {
>  CPUState *cs = ENV_GET_CPU(env);
>  uint64_t ret = 0;
> +uint32_t interrupt_request = cpu_interrupt_request(cs);
>
> -if (cs->interrupt_request & CPU_INTERRUPT_HARD) {
> +if (interrupt_request & CPU_INTERRUPT_HARD) {
>  ret |= CPSR_I;
>  }
> -if (cs->interrupt_request & CPU_INTERRUPT_FIQ) {
> +if (interrupt_request & CPU_INTERRUPT_FIQ) {
>  ret |= CPSR_F;
>  }
>  /* External aborts are not possible in QEMU so A bit is always clear */
> @@ -8579,10 +8580,7 @@ void arm_cpu_do_interrupt(CPUState *cs)
>  return;
>  }
>
> -/* Hooks may change global state so BQL should be held, also the
> - * BQL needs to be held for any modification of
> - * cs->interrupt_request.
> - */
> +/* Hooks may change global state so BQL should be held */
>  g_assert(qemu_mutex_iothread_locked());
>
>  arm_call_pre_el_change_hook(cpu);
> @@ -8597,7 +8595,7 @@ void arm_cpu_do_interrupt(CPUState *cs)
>  arm_call_el_change_hook(cpu);
>
>  if (!kvm_enabled()) {
> -cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
> +cpu_interrupt_request_or(cs, CPU_INTERRUPT_EXITTB);
>  }
>  }


--
Alex Bennée

Re: [Qemu-devel] [PATCH v8 00/38] target/mips: Limited support for the R5900

[Fredrik Noring]

Hi Maciej,

>  I'm not sure if every single random vendor-specific instruction (or a 
> bunch of) deserves its own ASE designation, be it internal or externally 
> exposed.  I think the MMI set being a substantial architectural feature 
> makes sense to be shown in /proc/cpuinfo (in Linux), but I don't think 
> there's much more about it.  It's limited to 2 implementations only, so 
> internally I think it can well be handled with a macro or static inline 
> function (as appropriate) which boil down to (CPU_R5900 || CPU_TX79).

Are there benefits in leaving out features? Their utility, such as
in choosing compiler options, may not correlate with their (lack of)
architectural weight.

A random pc, for instance, comes fully dressed flying the flags of

fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca
cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm
pbe syscall nx rdtscp lm constant_tsc arch_perfmon pebs bts
rep_good nopl xtopology nonstop_tsc aperfmperf eagerfpu pni
pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 cx16
xtpr pdcm pcid sse4_1 sse4_2 x2apic popcnt tsc_deadline_timer
aes xsave avx lahf_lm epb kaiser tpr_shadow vnmi flexpriority
ept vpid xsaveopt dtherm ida arat pln pts

in its /proc/cpuinfo. It also has a bugs field with

cpu_meltdown spectre_v1 spectre_v2

where the R5900 could have an entry for its short loop bug.

>  And if you run out of bits for ASEs regardless, then I suggest just to 
> expand the field in question.  In QEMU you can rely on the presence of the 
> `uint64_t' data type, so with only 8 bits exhausted you're far from 
> getting into trouble.

DisasContext::insn_flags is already uint64_t, where bits 63..56 are
reserved for vendor-specific ASEs. Of course, one could organise them
differently, especially since they may be mutually exclusive, or one
could use a new ASE-specific field for them.

Fredrik

[Qemu-devel] [Bug 1800156] [NEW] windows 8.1 loose grab/leave window on windowed

[Valentin Fort]

Public bug reported:

Hello, i am new to QEMU and i encounter that annoying issue (windowed)
when i move the mouse a bit too much then it leave the window.

Windows 8.1, Latest QEMU (Windows binaries).

** Affects: qemu
 Importance: Undecided
 Status: New

** Summary changed:

- windows 8.1 loose grab on windowed
+ windows 8.1 loose grab/leave window on windowed

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

Title:
  windows 8.1 loose grab/leave window on windowed

Status in QEMU:
  New

Bug description:
  Hello, i am new to QEMU and i encounter that annoying issue (windowed)
  when i move the mouse a bit too much then it leave the window.

  Windows 8.1, Latest QEMU (Windows binaries).

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

Re: [Qemu-devel] [PATCH v2 02/29] targer/riscv: Activate decodetree and implemnt LUI & AUIPC

[Richard Henderson]

On 10/26/18 11:49 AM, Bastian Koppelmann wrote:
> I think you can pick up everything up to the RVC conversion which still needs
> the work suggested by Richard. Thanks, for picking it up :)

Even then I thought we were talking about splitting the RV64 insns
into a separate file, reducing the ifdefs, and renaming the arg-sets
to match the instruction formats described in the riscv spec.


r~

Re: [Qemu-devel] [PATCH 01/11] target/mips: Rename ASE_MMI to ASE_TOSHIBA_MMI, with Toshiba namespace

[Aleksandar Markovic]

> From: Fredrik Noring 
> Subject: [PATCH 01/11] target/mips: Rename ASE_MMI to ASE_TOSHIBA_MMI, with 
> Toshiba namespace
> 
> Several vendors have multimedia instruction (MMI) sets and other
> extensions of various kinds. ASE vendor namespaces make it clear these
> are not generic architectural features and also avoid name clashes.

ASE_XXX flags are not meant to identify a CPU or vendor. They are not wired to 
any configuration bit or CPU model. They are purely QEMU internal constructs, 
whose purpose was to make internal QEMU MIPS-specific code organization easier. 
In this case, ASE_MMI is an umbrella for all MMI-like ASEs, introduced with 
intent to make encapsulation of MMI-specific code better and easier. 
Differences between CPUs should be resolved by other means. Name 'ASE_MMI' is 
fine.

Thanks,
Aleksandar

Re: [Qemu-devel] [PATCH] lsi53c895a: check message length value

[Mark Kanda]

On 10/26/2018 4:25 AM, P J P wrote:

+-- On Thu, 25 Oct 2018, Ameya More wrote --+
| While Mark and I reported this issue to you, it was actually discovered by
| Dejvau Security and they should receive credit for reporting this issue.
| http://www.dejavusecurity.com

I see; Would it be possible to share email-id of the original reporter to
include in the commit log message?


Deja vu requested that we include the following text in the commit message:

Discovered by Deja vu Security. Reported by Oracle.

Would that be acceptable?

Thanks,

-Mark

[Qemu-devel] Minutes of KVM Forum BoF on deprecating stuff

[Markus Armbruster]

This is from my (imperfect) notes, corrections welcome.

Motivation: QEMU contains stuff of dubious value, which gets in the way
in various (sometimes painful and expensive) ways.

Deprecation is the marking of an external interface as "we intend to
remove this, you should stop using it" (preferably with advice on what
to use instead).  We have a deprecation policy to guide us through this
process.

Topics we covered, reordered for readability:

* Dropping features inconveniences their users.  Keeping them impedes
  forward movement, and thus inconveniences other users.  We need to
  engage with the tradeoffs.

* The cost of keeping both old and new for a deprecation grace period
  (currently two releases) can be painfully high.  Tradeoff again.
  However, there's rough consensus not to mess with the deprecation
  policy right now.

* When something has been broken for the customary deprecation grace
  period, removing it without going through the deprecation process
  should be okay.

* We may have to deprecate interfaces, but we may also have a need to
  deprecate guarantees interfaces provide.  Worse when the guarantees
  are tacit.  No good answers.  Let's attack less thorny problems first.

* One obvious class of candidates for removal is machines we don't know
  how to boot, or can't boot, say because we lack required firmware
  and/or OS.

  Of course, "can boot" should be an automated test.  As a first step
  towards that, we should at least document how to boot each machine.
  We're going to ask machine maintainers to do that.

* We need to communicate "you're using something that is deprecated".
  How?  Right now, we print a deprecation message.  Okay when humans use
  QEMU directly in a shell.  However, when QEMU sits at the bottom of a
  software stack, the message will likely end up in a log file that is
  effectively write-only.
 
  - The one way to get people read log files is crashing their
application.  A command line option --future could make QEMU crash
right after printing a deprecation message.  This could help with
finding use of deprecated features in a testing environment.

  - A less destructive way to grab people's attention is to make things
run really, really slow: have QEMU go to sleep for a while after
printing a deprecation message.

  - We can also pass the buck to the next layer up: emit a QMP event.

Sadly, by the time the next layer connects to QMP, plenty of stuff
already happened.  We'd have to buffer deprecation events somehow.

What would libvirt do with such an event?  Log it, taint the domain,
emit a (libvirt) event to pass it on to the next layer up.

  - A completely different idea is to have a configuratin linter.  To
support doing this at the libvirt level, QEMU could expose "is
deprecated" in interface introspection.  Feels feasible for QMP,
where we already have sufficiently expressive introspection.  For
CLI, we'd first have to provide that (but we want that anyway).

  - We might also want to dispay deprecation messages in QEMU's GUI
somehow, or on serial consoles.

Re: [Qemu-devel] [PATCH 3/3] cirrus: mark as deprecated

[Daniel P . Berrangé]

On Fri, Oct 26, 2018 at 12:03:35PM +0200, Paolo Bonzini wrote:
> On 26/10/2018 11:59, Daniel P. Berrangé wrote:
> > I should also say that QEMU as an upstream project has multiple goals.
> > Running KVM guests with modern PV hardware is only one of them, albeit
> > a widely used one. Being able to run old legacy OS with old hardware,
> > and running arbitrary embedded boards/devices with emulation are both
> > use cases that QEMU project aims to address. To eliminate all the old
> > "crufty" device emulation in name of improving security for KVM, would
> > be to eliminate core use cases of the project. THis is why we're trying
> > to persue the direction of making it easier for vendors to disable
> > features and devices they don't wish to support & thus limit their
> > downstream CVE exposure.
> 
> Indeed.  If we had to deprecate a feature just because it had an
> off-by-one bug, no C program would grow beyond 1000 lines of code...

One thing we should do, however, is to make it clear which of the
device models we consider secure, and which we consider only usable
in a friendly guest environment, as we have very different code
maintainership & quality standards for different parts of QEMU.

Essentially virtio devices, and then only a handful of the emulated
devices are things we consider suitable for usage in secure envs.
Likewise for machine types probably.


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

[Qemu-devel] [PULL 3/9] tests/vm: Do not abuse parallelism when KVM is not available

[Fam Zheng]

From: Philippe Mathieu-Daudé 

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20181013004034.6968-3-f4...@amsat.org>
Reviewed-by: Richard Henderson 
Signed-off-by: Fam Zheng 
---
 tests/vm/basevm.py | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 834bc90cc1..2bd32dc6ce 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -196,6 +196,13 @@ class BaseVM(object):
 return self._guest.qmp(*args, **kwargs)
 
 def parse_args(vm_name):
+
+def get_default_jobs():
+if kvm_available():
+return multiprocessing.cpu_count() / 2
+else:
+return 1
+
 parser = optparse.OptionParser(
 description="VM test utility.  Exit codes: "
 "0 = success, "
@@ -208,7 +215,7 @@ def parse_args(vm_name):
   help="image file name")
 parser.add_option("--force", "-f", action="store_true",
   help="force build image even if image exists")
-parser.add_option("--jobs", type=int, default=multiprocessing.cpu_count() 
/ 2,
+parser.add_option("--jobs", type=int, default=get_default_jobs(),
   help="number of virtual CPUs")
 parser.add_option("--verbose", "-V", action="store_true",
   help="Pass V=1 to builds within the guest")
-- 
2.17.1

[Qemu-devel] [PULL 1/9] tests: docker: update test-mingw for GTK+ 2.0 removal

[Fam Zheng]

From: Paolo Bonzini 

--with-gtkabi does not exist anymore; remove it from the configure invocation.

Fixes: 89d85cde75143325205e332dd97bf1bb8402d7c1
Signed-off-by: Paolo Bonzini 
Message-Id: <1539886203-33670-1-git-send-email-pbonz...@redhat.com>
Tested-by: Philippe Mathieu-Daudé 
Reviewed-by: Thomas Huth 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Fam Zheng 
---
 tests/docker/test-mingw | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/docker/test-mingw b/tests/docker/test-mingw
index 7cca7e16a6..b078f22879 100755
--- a/tests/docker/test-mingw
+++ b/tests/docker/test-mingw
@@ -28,8 +28,7 @@ for prefix in x86_64-w64-mingw32- i686-w64-mingw32-; do
 --enable-vnc \
 --enable-bzip2 \
 --enable-guest-agent \
---with-sdlabi=2.0 \
---with-gtkabi=3.0
+--with-sdlabi=2.0
 install_qemu
 make clean
 
-- 
2.17.1

[Qemu-devel] [PULL 9/9] tests/vm: Do not abuse parallelism when HOST != TARGET architecture

[Fam Zheng]

From: Philippe Mathieu-Daudé 

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20181013004034.6968-9-f4...@amsat.org>
Reviewed-by: Richard Henderson 
Signed-off-by: Fam Zheng 
---
 tests/vm/basevm.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 9f4794898a..5caf77d6b8 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -200,10 +200,10 @@ class BaseVM(object):
 def qmp(self, *args, **kwargs):
 return self._guest.qmp(*args, **kwargs)
 
-def parse_args(vm_name):
+def parse_args(vmcls):
 
 def get_default_jobs():
-if kvm_available():
+if kvm_available(vmcls.arch):
 return multiprocessing.cpu_count() / 2
 else:
 return 1
@@ -216,7 +216,7 @@ def parse_args(vm_name):
 "3 = test command failed")
 parser.add_option("--debug", "-D", action="store_true",
   help="enable debug output")
-parser.add_option("--image", "-i", default="%s.img" % vm_name,
+parser.add_option("--image", "-i", default="%s.img" % vmcls.name,
   help="image file name")
 parser.add_option("--force", "-f", action="store_true",
   help="force build image even if image exists")
@@ -237,7 +237,7 @@ def parse_args(vm_name):
 
 def main(vmcls):
 try:
-args, argv = parse_args(vmcls.name)
+args, argv = parse_args(vmcls)
 if not argv and not args.build_qemu and not args.build_image:
 print("Nothing to do?")
 return 1
-- 
2.17.1

[Qemu-devel] [PULL 6/9] tests/vm: Add a BaseVM::arch property

[Fam Zheng]

From: Philippe Mathieu-Daudé 

The 'arch' property gives a hint on which architecture the guest image runs.

This can be use to select the correct QEMU binary path.

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20181013004034.6968-6-f4...@amsat.org>
Reviewed-by: Richard Henderson 
Signed-off-by: Fam Zheng 
---
 tests/vm/basevm.py   | 4 +++-
 tests/vm/centos  | 1 +
 tests/vm/freebsd | 1 +
 tests/vm/netbsd  | 1 +
 tests/vm/openbsd | 1 +
 tests/vm/ubuntu.i386 | 1 +
 6 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 81a1cb05dd..b2e0de2022 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -42,6 +42,8 @@ class BaseVM(object):
 BUILD_SCRIPT = ""
 # The guest name, to be overridden by subclasses
 name = "#base"
+# The guest architecture, to be overridden by subclasses
+arch = "#arch"
 def __init__(self, debug=False, vcpus=None):
 self._guest = None
 self._tmpdir = os.path.realpath(tempfile.mkdtemp(prefix="vm-test-",
@@ -151,7 +153,7 @@ class BaseVM(object):
 "-device", "virtio-blk,drive=drive0,bootindex=0"]
 args += self._data_args + extra_args
 logging.debug("QEMU args: %s", " ".join(args))
-qemu_bin = os.environ.get("QEMU", "qemu-system-x86_64")
+qemu_bin = os.environ.get("QEMU", "qemu-system-" + self.arch)
 guest = QEMUMachine(binary=qemu_bin, args=args)
 try:
 guest.launch()
diff --git a/tests/vm/centos b/tests/vm/centos
index afd560c564..daa2dbca03 100755
--- a/tests/vm/centos
+++ b/tests/vm/centos
@@ -19,6 +19,7 @@ import time
 
 class CentosVM(basevm.BaseVM):
 name = "centos"
+arch = "x86_64"
 BUILD_SCRIPT = """
 set -e;
 cd $(mktemp -d);
diff --git a/tests/vm/freebsd b/tests/vm/freebsd
index b6983127d0..19a3729172 100755
--- a/tests/vm/freebsd
+++ b/tests/vm/freebsd
@@ -18,6 +18,7 @@ import basevm
 
 class FreeBSDVM(basevm.BaseVM):
 name = "freebsd"
+arch = "x86_64"
 BUILD_SCRIPT = """
 set -e;
 rm -rf /var/tmp/qemu-test.*
diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index a4e25820d5..fac6a7ce51 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -18,6 +18,7 @@ import basevm
 
 class NetBSDVM(basevm.BaseVM):
 name = "netbsd"
+arch = "x86_64"
 BUILD_SCRIPT = """
 set -e;
 rm -rf /var/tmp/qemu-test.*
diff --git a/tests/vm/openbsd b/tests/vm/openbsd
index 52500ee52b..cfe0572c59 100755
--- a/tests/vm/openbsd
+++ b/tests/vm/openbsd
@@ -18,6 +18,7 @@ import basevm
 
 class OpenBSDVM(basevm.BaseVM):
 name = "openbsd"
+arch = "x86_64"
 BUILD_SCRIPT = """
 set -e;
 rm -rf /var/tmp/qemu-test.*
diff --git a/tests/vm/ubuntu.i386 b/tests/vm/ubuntu.i386
index 3f6ed48b74..1b7e1ab8f0 100755
--- a/tests/vm/ubuntu.i386
+++ b/tests/vm/ubuntu.i386
@@ -19,6 +19,7 @@ import time
 
 class UbuntuX86VM(basevm.BaseVM):
 name = "ubuntu.i386"
+arch = "i386"
 BUILD_SCRIPT = """
 set -e;
 cd $(mktemp -d);
-- 
2.17.1

[Qemu-devel] [PULL 2/9] tests/vm: Extract the kvm_available() handy function

[Fam Zheng]

From: Philippe Mathieu-Daudé 

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20181013004034.6968-2-f4...@amsat.org>
Reviewed-by: Richard Henderson 
Signed-off-by: Fam Zheng 
---
 scripts/qemu.py| 4 
 tests/vm/basevm.py | 4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index f099ce7278..9fc0be4828 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -26,6 +26,10 @@ import tempfile
 LOG = logging.getLogger(__name__)
 
 
+def kvm_available(target_arch=None):
+return os.access("/dev/kvm", os.R_OK | os.W_OK)
+
+
 #: Maps machine types to the preferred console device types
 CONSOLE_DEV_TYPES = {
 r'^clipper$': 'isa-serial',
diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index cafbc6b3a5..834bc90cc1 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -18,7 +18,7 @@ import logging
 import time
 import datetime
 sys.path.append(os.path.join(os.path.dirname(__file__), "..", "..", "scripts"))
-from qemu import QEMUMachine
+from qemu import QEMUMachine, kvm_available
 import subprocess
 import hashlib
 import optparse
@@ -72,7 +72,7 @@ class BaseVM(object):
 "-serial", "file:%s" % os.path.join(self._tmpdir, "serial.out")]
 if vcpus:
 self._args += ["-smp", str(vcpus)]
-if os.access("/dev/kvm", os.R_OK | os.W_OK):
+if kvm_available():
 self._args += ["-enable-kvm"]
 else:
 logging.info("KVM not available, not using -enable-kvm")
-- 
2.17.1

[Qemu-devel] [PULL 5/9] tests/vm: Display remaining seconds to wait for a VM to start

[Fam Zheng]

From: Philippe Mathieu-Daudé 

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20181013004034.6968-5-f4...@amsat.org>
Reviewed-by: Richard Henderson 
Signed-off-by: Fam Zheng 
---
 tests/vm/basevm.py | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 9415e7c33a..81a1cb05dd 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -177,11 +177,14 @@ class BaseVM(object):
 
 def wait_ssh(self, seconds=300):
 starttime = datetime.datetime.now()
+endtime = starttime + datetime.timedelta(seconds=seconds)
 guest_up = False
-while (datetime.datetime.now() - starttime).total_seconds() < seconds:
+while datetime.datetime.now() < endtime:
 if self.ssh("exit 0") == 0:
 guest_up = True
 break
+seconds = (endtime - datetime.datetime.now()).total_seconds()
+logging.debug("%ds before timeout", seconds)
 time.sleep(1)
 if not guest_up:
 raise Exception("Timeout while waiting for guest ssh")
-- 
2.17.1

[Qemu-devel] [PULL 4/9] tests/vm: Do not use the -smp option with a single cpu

[Fam Zheng]

From: Philippe Mathieu-Daudé 

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20181013004034.6968-4-f4...@amsat.org>
Reviewed-by: Richard Henderson 
Signed-off-by: Fam Zheng 
---
 tests/vm/basevm.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 2bd32dc6ce..9415e7c33a 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -70,7 +70,7 @@ class BaseVM(object):
 "-device", "virtio-net-pci,netdev=vnet",
 "-vnc", "127.0.0.1:0,to=20",
 "-serial", "file:%s" % os.path.join(self._tmpdir, "serial.out")]
-if vcpus:
+if vcpus and vcpus > 1:
 self._args += ["-smp", str(vcpus)]
 if kvm_available():
 self._args += ["-enable-kvm"]
-- 
2.17.1

[Qemu-devel] [PULL 0/9] Testing patches

[Fam Zheng]

The following changes since commit 808ebd66e467f77c0d1f8c6346235f81e9c99cf2:

  Merge remote-tracking branch 'remotes/riscv/tags/riscv-for-master-3.1-sf0' 
into staging (2018-10-25 17:41:03 +0100)

are available in the Git repository at:

  git://github.com/famz/qemu.git tags/testing-pull-request

for you to fetch changes up to 63a24c5e2354833a84f18bdf0e857fad8812f65b:

  tests/vm: Do not abuse parallelism when HOST != TARGET architecture 
(2018-10-26 22:03:21 +0800)


Testing patches

One fix for mingw build and some improvements in VM based testing, many thanks
to Paolo and Phil.



Paolo Bonzini (1):
  tests: docker: update test-mingw for GTK+ 2.0 removal

Philippe Mathieu-Daudé (8):
  tests/vm: Extract the kvm_available() handy function
  tests/vm: Do not abuse parallelism when KVM is not available
  tests/vm: Do not use the -smp option with a single cpu
  tests/vm: Display remaining seconds to wait for a VM to start
  tests/vm: Add a BaseVM::arch property
  tests/vm: Let kvm_available() work in cross environments
  tests/vm: Do not use -enable-kvm if HOST != TARGET architecture
  tests/vm: Do not abuse parallelism when HOST != TARGET architecture

 scripts/qemu.py |  6 ++
 tests/docker/test-mingw |  3 +--
 tests/vm/basevm.py  | 30 +-
 tests/vm/centos |  1 +
 tests/vm/freebsd|  1 +
 tests/vm/netbsd |  1 +
 tests/vm/openbsd|  1 +
 tests/vm/ubuntu.i386|  1 +
 8 files changed, 33 insertions(+), 11 deletions(-)

-- 
2.17.1

Re: [Qemu-devel] Minutes of KVM Forum BoF on deprecating stuff

[Daniel P . Berrangé]

On Fri, Oct 26, 2018 at 04:03:51PM +0200, Markus Armbruster wrote:
> This is from my (imperfect) notes, corrections welcome.
> 
> Motivation: QEMU contains stuff of dubious value, which gets in the way
> in various (sometimes painful and expensive) ways.
>
> Deprecation is the marking of an external interface as "we intend to
> remove this, you should stop using it" (preferably with advice on what
> to use instead).  We have a deprecation policy to guide us through this
> process.


Something I meant to bring up but forgot is about the classification
of devices, especially with a view towards security. It is not directly
about deprecation, but it is somewhat related as it is related  to the
state of maintainence and quality level

We've got alot of devices, but only a subset are written and maintained
to a level where we'd consider them robust wrt malcious guests. Other
devices are only suitable for friendly guest environments. We should
clearly document which are the devices that we consider to provide
a secure boundary to guests, so users can make suitably informed choices.
I'd guess this means all virtio devices, and then few of the emulated
devices that are commonly used & maintained in a KVM environment.

This would be useful for distros/vendors/users who wish to limit their
potential attack surface once we have a KConfig system for fine grained
disablement of features.

> Topics we covered, reordered for readability:
> 
> * Dropping features inconveniences their users.  Keeping them impedes
>   forward movement, and thus inconveniences other users.  We need to
>   engage with the tradeoffs.
> 
> * The cost of keeping both old and new for a deprecation grace period
>   (currently two releases) can be painfully high.  Tradeoff again.
>   However, there's rough consensus not to mess with the deprecation
>   policy right now.
> 
> * When something has been broken for the customary deprecation grace
>   period, removing it without going through the deprecation process
>   should be okay.
> 
> * We may have to deprecate interfaces, but we may also have a need to
>   deprecate guarantees interfaces provide.  Worse when the guarantees
>   are tacit.  No good answers.  Let's attack less thorny problems first.
> 
> * One obvious class of candidates for removal is machines we don't know
>   how to boot, or can't boot, say because we lack required firmware
>   and/or OS.
> 
>   Of course, "can boot" should be an automated test.  As a first step
>   towards that, we should at least document how to boot each machine.
>   We're going to ask machine maintainers to do that.
> 
> * We need to communicate "you're using something that is deprecated".
>   How?  Right now, we print a deprecation message.  Okay when humans use
>   QEMU directly in a shell.  However, when QEMU sits at the bottom of a
>   software stack, the message will likely end up in a log file that is
>   effectively write-only.
>  
>   - The one way to get people read log files is crashing their
> application.  A command line option --future could make QEMU crash
> right after printing a deprecation message.  This could help with
> finding use of deprecated features in a testing environment.
> 
>   - A less destructive way to grab people's attention is to make things
> run really, really slow: have QEMU go to sleep for a while after
> printing a deprecation message.
> 
>   - We can also pass the buck to the next layer up: emit a QMP event.
> 
> Sadly, by the time the next layer connects to QMP, plenty of stuff
> already happened.  We'd have to buffer deprecation events somehow.
> 
> What would libvirt do with such an event?  Log it, taint the domain,
> emit a (libvirt) event to pass it on to the next layer up.
> 
>   - A completely different idea is to have a configuratin linter.  To
> support doing this at the libvirt level, QEMU could expose "is
> deprecated" in interface introspection.  Feels feasible for QMP,
> where we already have sufficiently expressive introspection.  For
> CLI, we'd first have to provide that (but we want that anyway).
> 
>   - We might also want to dispay deprecation messages in QEMU's GUI
> somehow, or on serial consoles.

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

[Qemu-devel] [PULL 8/9] tests/vm: Do not use -enable-kvm if HOST != TARGET architecture

[Fam Zheng]

From: Philippe Mathieu-Daudé 

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20181013004034.6968-8-f4...@amsat.org>
Reviewed-by: Richard Henderson 
Signed-off-by: Fam Zheng 
---
 tests/vm/basevm.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index b2e0de2022..9f4794898a 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -74,7 +74,7 @@ class BaseVM(object):
 "-serial", "file:%s" % os.path.join(self._tmpdir, "serial.out")]
 if vcpus and vcpus > 1:
 self._args += ["-smp", str(vcpus)]
-if kvm_available():
+if kvm_available(self.arch):
 self._args += ["-enable-kvm"]
 else:
 logging.info("KVM not available, not using -enable-kvm")
-- 
2.17.1

[Qemu-devel] [PULL 7/9] tests/vm: Let kvm_available() work in cross environments

[Fam Zheng]

From: Philippe Mathieu-Daudé 

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20181013004034.6968-7-f4...@amsat.org>
Reviewed-by: Richard Henderson 
Signed-off-by: Fam Zheng 
---
 scripts/qemu.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index 9fc0be4828..bcd24aad82 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -27,6 +27,8 @@ LOG = logging.getLogger(__name__)
 
 
 def kvm_available(target_arch=None):
+if target_arch and target_arch != os.uname()[4]:
+return False
 return os.access("/dev/kvm", os.R_OK | os.W_OK)
 
 
-- 
2.17.1

[Qemu-devel] [PATCH 0/6] target/mips: Add support for prctl() PR_GET_FP_MODE and PR_SET_FP_MODE

[Stefan Markovic]

From: Stefan Markovic 

This series includes support for prctl() PR_GET_FP_MODE and PR_SET_FP_MODE. 
This requires
extracting MIPS.abiflags section from ELF file and fp_abi value handling.

Stefan Markovic (6):
  Define MIPS_ABI_FP_UNKNOWN macro
  Extend image_info struct with MIPS specific fp_abi and
interp_fp_abi fields
  Extract MIPS abiflags from ELF file
  Read and set FP ABI value from MIPS abiflags
  Determine the desired FPU mode
  Add prctl() PR_SET_FP_MODE and PR_GET_FP_MODE implementations

 include/elf.h  |  2 +
 linux-user/elfload.c   | 37 +++
 linux-user/mips/cpu_loop.c | 75 ++
 linux-user/mips/target_syscall.h   |  2 +
 linux-user/mips64/target_syscall.h |  2 +
 linux-user/qemu.h  |  4 ++
 linux-user/syscall.c   | 62 +--
 7 files changed, 180 insertions(+), 4 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH 2/6] Extend image_info struct with MIPS specific fp_abi and interp_fp_abi fields

[Stefan Markovic]

From: Stefan Markovic 

Signed-off-by: Stefan Markovic 
---
 linux-user/qemu.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 1beb6a2..a752c1c 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -61,6 +61,10 @@ struct image_info {
 abi_ulong   interpreter_loadmap_addr;
 abi_ulong   interpreter_pt_dynamic_addr;
 struct image_info *other_info;
+#ifdef TARGET_MIPS
+int fp_abi;
+int interp_fp_abi;
+#endif
 };
 
 #ifdef TARGET_I386
-- 
1.9.1

[Qemu-devel] [PATCH 5/6] Determine the desired FPU mode

[Stefan Markovic]

From: Stefan Markovic 

Floating-point mode is calculated from MIPS.abiflags FP ABI value
(based on kernel implementation). Illegal combinations are rejected.

Signed-off-by: Stefan Markovic 
---
 linux-user/mips/cpu_loop.c | 75 ++
 1 file changed, 75 insertions(+)

diff --git a/linux-user/mips/cpu_loop.c b/linux-user/mips/cpu_loop.c
index c9c20cf..fd96e46 100644
--- a/linux-user/mips/cpu_loop.c
+++ b/linux-user/mips/cpu_loop.c
@@ -740,6 +740,34 @@ void target_cpu_copy_regs(CPUArchState *env, struct 
target_pt_regs *regs)
 struct image_info *info = ts->info;
 int i;
 
+struct mode_req {
+bool single;
+bool soft;
+bool fr1;
+bool frdefault;
+bool fre;
+};
+
+static const struct mode_req fpu_reqs[] = {
+[MIPS_ABI_FP_ANY]= { true,  true,  true,  true,  true  },
+[MIPS_ABI_FP_DOUBLE] = { false, false, false, true,  true  },
+[MIPS_ABI_FP_SINGLE] = { true,  false, false, false, false },
+[MIPS_ABI_FP_SOFT]   = { false, true,  false, false, false },
+[MIPS_ABI_FP_OLD_64] = { false, false, false, false, false },
+[MIPS_ABI_FP_XX] = { false, false, true,  true,  true  },
+[MIPS_ABI_FP_64] = { false, false, true,  false, false },
+[MIPS_ABI_FP_64A]= { false, false, true,  false, true  }
+};
+
+/*
+ * Mode requirements when .MIPS.abiflags is not present in the ELF.
+ * Not present means that everything is acceptable except FR1.
+ */
+static struct mode_req none_req = { true, true, false, true, true };
+
+struct mode_req prog_req;
+struct mode_req interp_req;
+
 for(i = 0; i < 32; i++) {
 env->active_tc.gpr[i] = regs->regs[i];
 }
@@ -747,6 +775,53 @@ void target_cpu_copy_regs(CPUArchState *env, struct 
target_pt_regs *regs)
 if (regs->cp0_epc & 1) {
 env->hflags |= MIPS_HFLAG_M16;
 }
+
+#ifdef TARGET_ABI_MIPSO32
+# define MAX_FP_ABI MIPS_ABI_FP_64A
+#else
+# define MAX_FP_ABI MIPS_ABI_FP_SOFT
+#endif
+ if ((info->fp_abi > MAX_FP_ABI && info->fp_abi != MIPS_ABI_FP_UNKNOWN)
+|| (info->interp_fp_abi > MAX_FP_ABI &&
+info->interp_fp_abi != MIPS_ABI_FP_UNKNOWN)) {
+fprintf(stderr, "qemu: Program and interpreter have "
+"unexpected FPU modes\n");
+exit(137);
+}
+
+prog_req = (info->fp_abi == MIPS_ABI_FP_UNKNOWN) ? none_req
+: fpu_reqs[info->fp_abi];
+interp_req = (info->interp_fp_abi == MIPS_ABI_FP_UNKNOWN) ? none_req
+: fpu_reqs[info->interp_fp_abi];
+
+prog_req.single &= interp_req.single;
+prog_req.soft &= interp_req.soft;
+prog_req.fr1 &= interp_req.fr1;
+prog_req.frdefault &= interp_req.frdefault;
+prog_req.fre &= interp_req.fre;
+
+bool cpu_has_mips_r2_r6 = env->insn_flags & ISA_MIPS32R2 ||
+  env->insn_flags & ISA_MIPS64R2 ||
+  env->insn_flags & ISA_MIPS32R6 ||
+  env->insn_flags & ISA_MIPS64R6;
+
+if (prog_req.fre && !prog_req.frdefault && !prog_req.fr1) {
+env->CP0_Config5 |= (1 << CP0C5_FRE);
+if (env->active_fpu.fcr0 & (1 << FCR0_FREP)) {
+env->hflags |= MIPS_HFLAG_FRE;
+}
+} else if ((prog_req.fr1 && prog_req.frdefault) ||
+ (prog_req.single && !prog_req.frdefault)) {
+if ((env->active_fpu.fcr0 & (1 << FCR0_F64)
+&& cpu_has_mips_r2_r6) || prog_req.fr1) {
+env->CP0_Status |= (1 << CP0St_FR);
+env->hflags |= MIPS_HFLAG_F64;
+}
+} else  if (!prog_req.fre && !prog_req.frdefault &&
+  !prog_req.fr1 && !prog_req.single && !prog_req.soft) {
+exit(137);
+}
+
 if (env->insn_flags & ISA_NANOMIPS32) {
 return;
 }
-- 
1.9.1