Re: [Qemu-devel] Patch causing qemu-system-ppc to crash

[Mark Cave-Ayland]

On 24/05/2019 03:48, Programmingkid wrote:

> Recently I have noticed that qemu-system-ppc is crashing while booting up my 
> Mac OS X VM. A bit of git bisecting shows this is the patch that causes this 
> issue:
> 
> commit 1e262b49b5331441f697461e4305fe06719758a7
> Author: Richard Henderson 
> Date:   Mon Mar 18 12:02:54 2019 -0700
> 
> tcg/i386: Implement tcg_out_dupm_vec
> 
> At the same time, improve tcg_out_dupi_vec wrt broadcast
> from the constant pool.
> 
> Signed-off-by: Richard Henderson https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03767.html help at all?


ATB,

Mark.

Re: [Qemu-devel] [PATCH 4/9] target/ppc: Fix lxvw4x, lxvh8x and lxvb16x

[Mark Cave-Ayland]

On 22/05/2019 07:10, David Gibson wrote:

> On Wed, May 22, 2019 at 05:37:47AM +0100, Mark Cave-Ayland wrote:
>> On 22/05/2019 01:49, David Gibson wrote:
>>
>>> On Wed, May 22, 2019 at 06:11:12AM +1000, Anton Blanchard wrote:
 Hi,

> I've now had a bit of time to look through this and I believe it is
> correct, so:
>
> Reviewed-by: Mark Cave-Ayland 

 Thanks Mark. David: any chance we could get this merged? I can't run a
 recent Ubuntu image successfully without it. sshd hangs when I try to
 ssh into it.
>>>
>>> I had a comment that was never addressed - it didn't look like the xth
>>> and xtl temporaries were initialized after the patch.
>>
>> If it helps, here was my analysis at the time (looks like you were also 
>> included on
>> the reply?): 
>> https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg01515.html.
> 
> Sorry, I missed that.  Looks reasonable, I think I failed to spot the
> generated load instructions which effectively initialize the temps.
> 
> This is all at some remove now, can you resend the patch on top of the
> latest tree please and I'll apply.  It's missed the pull request I
> sent today, obviously, but I know I have some other stuff I want to
> get in pretty soon, so I expect to send another one relatively soon.

All done - I've just sent a v2 rebased upon your ppc-for-4.1 branch with R-B 
and T-B
tags included.


ATB,

Mark.

[Qemu-devel] [PATCH v2] target/ppc: Fix lxvw4x, lxvh8x and lxvb16x

[Mark Cave-Ayland]

From: Anton Blanchard 

During the conversion these instructions were incorrectly treated as
stores. We need to use set_cpu_vsr* and not get_cpu_vsr*.

Fixes: 8b3b2d75c7c0 ("introduce get_cpu_vsr{l,h}() and set_cpu_vsr{l,h}() 
helpers for VSR register access")
Signed-off-by: Anton Blanchard 
Reviewed-by: Mark Cave-Ayland 
Tested-by: Greg Kurz 
Reviewed-by: Greg Kurz 
---
 target/ppc/translate/vsx-impl.inc.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/target/ppc/translate/vsx-impl.inc.c 
b/target/ppc/translate/vsx-impl.inc.c
index 199d22da97..cdb44b8b70 100644
--- a/target/ppc/translate/vsx-impl.inc.c
+++ b/target/ppc/translate/vsx-impl.inc.c
@@ -102,8 +102,7 @@ static void gen_lxvw4x(DisasContext *ctx)
 }
 xth = tcg_temp_new_i64();
 xtl = tcg_temp_new_i64();
-get_cpu_vsrh(xth, xT(ctx->opcode));
-get_cpu_vsrl(xtl, xT(ctx->opcode));
+
 gen_set_access_type(ctx, ACCESS_INT);
 EA = tcg_temp_new();
 
@@ -126,6 +125,8 @@ static void gen_lxvw4x(DisasContext *ctx)
 tcg_gen_addi_tl(EA, EA, 8);
 tcg_gen_qemu_ld_i64(xtl, EA, ctx->mem_idx, MO_BEQ);
 }
+set_cpu_vsrh(xT(ctx->opcode), xth);
+set_cpu_vsrl(xT(ctx->opcode), xtl);
 tcg_temp_free(EA);
 tcg_temp_free_i64(xth);
 tcg_temp_free_i64(xtl);
@@ -185,8 +186,6 @@ static void gen_lxvh8x(DisasContext *ctx)
 }
 xth = tcg_temp_new_i64();
 xtl = tcg_temp_new_i64();
-get_cpu_vsrh(xth, xT(ctx->opcode));
-get_cpu_vsrl(xtl, xT(ctx->opcode));
 gen_set_access_type(ctx, ACCESS_INT);
 
 EA = tcg_temp_new();
@@ -197,6 +196,8 @@ static void gen_lxvh8x(DisasContext *ctx)
 if (ctx->le_mode) {
 gen_bswap16x8(xth, xtl, xth, xtl);
 }
+set_cpu_vsrh(xT(ctx->opcode), xth);
+set_cpu_vsrl(xT(ctx->opcode), xtl);
 tcg_temp_free(EA);
 tcg_temp_free_i64(xth);
 tcg_temp_free_i64(xtl);
@@ -214,14 +215,14 @@ static void gen_lxvb16x(DisasContext *ctx)
 }
 xth = tcg_temp_new_i64();
 xtl = tcg_temp_new_i64();
-get_cpu_vsrh(xth, xT(ctx->opcode));
-get_cpu_vsrl(xtl, xT(ctx->opcode));
 gen_set_access_type(ctx, ACCESS_INT);
 EA = tcg_temp_new();
 gen_addr_reg_index(ctx, EA);
 tcg_gen_qemu_ld_i64(xth, EA, ctx->mem_idx, MO_BEQ);
 tcg_gen_addi_tl(EA, EA, 8);
 tcg_gen_qemu_ld_i64(xtl, EA, ctx->mem_idx, MO_BEQ);
+set_cpu_vsrh(xT(ctx->opcode), xth);
+set_cpu_vsrl(xT(ctx->opcode), xtl);
 tcg_temp_free(EA);
 tcg_temp_free_i64(xth);
 tcg_temp_free_i64(xtl);
-- 
2.11.0

[Qemu-devel] [PATCH 19/20] hw/i386/pc: Rename pc_build_feature_control() as generic fw_cfg_build_*

[Philippe Mathieu-Daudé]

Now that the pc_build_feature_control_file() function has been
refactored to not depend of PC specific types, rename it to a
more generic name.

Suggested-by: Samuel Ortiz 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0ff2cea1ee..4d333aba82 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1515,8 +1515,8 @@ void pc_cpus_init(PCMachineState *pcms)
 }
 }
 
-static void pc_build_feature_control_file(MachineState *ms,
-  FWCfgState *fw_cfg)
+static void fw_cfg_build_feature_control(MachineState *ms,
+ FWCfgState *fw_cfg)
 {
 X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
 CPUX86State *env = &cpu->env;
@@ -1587,7 +1587,7 @@ void pc_machine_done(Notifier *notifier, void *data)
 acpi_setup();
 if (pcms->fw_cfg) {
 fw_cfg_build_smbios(MACHINE(pcms), pcms->fw_cfg);
-pc_build_feature_control_file(MACHINE(pcms), pcms->fw_cfg);
+fw_cfg_build_feature_control(MACHINE(pcms), pcms->fw_cfg);
 /* update FW_CFG_NB_CPUS to account for -device added CPUs */
 fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
 }
-- 
2.20.1

[Qemu-devel] [PATCH 18/20] hw/i386/pc: Let pc_build_feature_control() take a MachineState argument

[Philippe Mathieu-Daudé]

Let the pc_build_feature_control_file() function take a generic MachineState
argument.

Suggested-by: Samuel Ortiz 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0db0ba3893..0ff2cea1ee 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1515,10 +1515,9 @@ void pc_cpus_init(PCMachineState *pcms)
 }
 }
 
-static void pc_build_feature_control_file(PCMachineState *pcms,
+static void pc_build_feature_control_file(MachineState *ms,
   FWCfgState *fw_cfg)
 {
-MachineState *ms = MACHINE(pcms);
 X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
 CPUX86State *env = &cpu->env;
 uint32_t unused, ecx, edx;
@@ -1588,7 +1587,7 @@ void pc_machine_done(Notifier *notifier, void *data)
 acpi_setup();
 if (pcms->fw_cfg) {
 fw_cfg_build_smbios(MACHINE(pcms), pcms->fw_cfg);
-pc_build_feature_control_file(pcms, pcms->fw_cfg);
+pc_build_feature_control_file(MACHINE(pcms), pcms->fw_cfg);
 /* update FW_CFG_NB_CPUS to account for -device added CPUs */
 fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
 }
-- 
2.20.1

[Qemu-devel] [PATCH 17/20] hw/i386/pc: Let pc_build_feature_control() take a FWCfgState argument

[Philippe Mathieu-Daudé]

Pass the FWCfgState object by argument, this will
allow us to remove the PCMachineState argument later.

Suggested-by: Samuel Ortiz 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2804c4dc1b..0db0ba3893 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1515,7 +1515,8 @@ void pc_cpus_init(PCMachineState *pcms)
 }
 }
 
-static void pc_build_feature_control_file(PCMachineState *pcms)
+static void pc_build_feature_control_file(PCMachineState *pcms,
+  FWCfgState *fw_cfg)
 {
 MachineState *ms = MACHINE(pcms);
 X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
@@ -1541,7 +1542,7 @@ static void pc_build_feature_control_file(PCMachineState 
*pcms)
 
 val = g_malloc(sizeof(*val));
 *val = cpu_to_le64(feature_control_bits | FEATURE_CONTROL_LOCKED);
-fw_cfg_add_file(pcms->fw_cfg, "etc/msr_feature_control", val, 
sizeof(*val));
+fw_cfg_add_file(fw_cfg, "etc/msr_feature_control", val, sizeof(*val));
 }
 
 static void rtc_set_cpus_count(ISADevice *rtc, uint16_t cpus_count)
@@ -1587,7 +1588,7 @@ void pc_machine_done(Notifier *notifier, void *data)
 acpi_setup();
 if (pcms->fw_cfg) {
 fw_cfg_build_smbios(MACHINE(pcms), pcms->fw_cfg);
-pc_build_feature_control_file(pcms);
+pc_build_feature_control_file(pcms, pcms->fw_cfg);
 /* update FW_CFG_NB_CPUS to account for -device added CPUs */
 fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
 }
-- 
2.20.1

[Qemu-devel] [PATCH 16/20] hw/i386/pc: Rename pc_build_smbios() as generic fw_cfg_build_smbios()

[Philippe Mathieu-Daudé]

Now that the pc_build_smbios() function has been refactored to not
depend of PC specific types, rename it to a more generic name.

Suggested-by: Samuel Ortiz 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index a79b344eb5..2804c4dc1b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -886,7 +886,7 @@ static uint32_t x86_cpu_apic_id_from_index(unsigned int 
cpu_index)
 }
 }
 
-static void pc_build_smbios(MachineState *ms, FWCfgState *fw_cfg)
+static void fw_cfg_build_smbios(MachineState *ms, FWCfgState *fw_cfg)
 {
 uint8_t *smbios_tables, *smbios_anchor;
 size_t smbios_tables_len, smbios_anchor_len;
@@ -1586,7 +1586,7 @@ void pc_machine_done(Notifier *notifier, void *data)
 
 acpi_setup();
 if (pcms->fw_cfg) {
-pc_build_smbios(MACHINE(pcms), pcms->fw_cfg);
+fw_cfg_build_smbios(MACHINE(pcms), pcms->fw_cfg);
 pc_build_feature_control_file(pcms);
 /* update FW_CFG_NB_CPUS to account for -device added CPUs */
 fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
-- 
2.20.1

[Qemu-devel] [PATCH 10/20] hw/i386/pc: Pass the boot_cpus value by argument

[Philippe Mathieu-Daudé]

The boot_cpus is used once. Pass it by argument, this will
allow us to remove the PCMachineState argument later.

Suggested-by: Samuel Ortiz 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 264074489b..01894b9875 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -928,7 +928,7 @@ static void pc_build_smbios(PCMachineState *pcms)
 }
 }
 
-static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms)
+static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms, uint16_t boot_cpus)
 {
 FWCfgState *fw_cfg;
 uint64_t *numa_fw_cfg;
@@ -938,7 +938,7 @@ static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms)
 
 fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,
 &address_space_memory);
-fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
+fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, boot_cpus);
 
 /* FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
  *
@@ -1762,7 +1762,7 @@ void pc_memory_init(PCMachineState *pcms,
 option_rom_mr,
 1);
 
-fw_cfg = x86_create_fw_cfg(pcms);
+fw_cfg = x86_create_fw_cfg(pcms, pcms->boot_cpus);
 
 rom_set_fw(fw_cfg);
 
-- 
2.20.1

[Qemu-devel] [PATCH 15/20] hw/i386/pc: Let pc_build_smbios() take a generic MachineState argument

[Philippe Mathieu-Daudé]

Let the pc_build_smbios() function take a generic MachineState
argument.

Suggested-by: Samuel Ortiz 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index b029aee6c7..a79b344eb5 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -886,13 +886,12 @@ static uint32_t x86_cpu_apic_id_from_index(unsigned int 
cpu_index)
 }
 }
 
-static void pc_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg)
+static void pc_build_smbios(MachineState *ms, FWCfgState *fw_cfg)
 {
 uint8_t *smbios_tables, *smbios_anchor;
 size_t smbios_tables_len, smbios_anchor_len;
 struct smbios_phys_mem_area *mem_array;
 unsigned i, array_count;
-MachineState *ms = MACHINE(pcms);
 X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
 
 /* tell smbios about cpuid version and features */
@@ -1587,7 +1586,7 @@ void pc_machine_done(Notifier *notifier, void *data)
 
 acpi_setup();
 if (pcms->fw_cfg) {
-pc_build_smbios(pcms, pcms->fw_cfg);
+pc_build_smbios(MACHINE(pcms), pcms->fw_cfg);
 pc_build_feature_control_file(pcms);
 /* update FW_CFG_NB_CPUS to account for -device added CPUs */
 fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
-- 
2.20.1

[Qemu-devel] [PATCH 06/20] hw/i386/pc: Use e820_get_num_entries() to access e820_entries

[Philippe Mathieu-Daudé]

To be able to extract the e820* code out of this file (in the next
patch), access e820_entries with its correct helper.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index ac8343c728..2e195049a5 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1022,7 +1022,7 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, 
PCMachineState *pcms)
 fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE,
  &e820_reserve, sizeof(e820_reserve));
 fw_cfg_add_file(fw_cfg, "etc/e820", e820_table,
-sizeof(struct e820_entry) * e820_entries);
+sizeof(struct e820_entry) * e820_get_num_entries());
 
 fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, &hpet_cfg, sizeof(hpet_cfg));
 /* allocate memory for the NUMA channel: one (64bit) word for the number
-- 
2.20.1

[Qemu-devel] [PATCH 14/20] hw/i386/pc: Let pc_build_smbios() take a FWCfgState argument

[Philippe Mathieu-Daudé]

Pass the FWCfgState object by argument, this will
allow us to remove the PCMachineState argument later.

Suggested-by: Samuel Ortiz 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e8813ef2b6..b029aee6c7 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -886,7 +886,7 @@ static uint32_t x86_cpu_apic_id_from_index(unsigned int 
cpu_index)
 }
 }
 
-static void pc_build_smbios(PCMachineState *pcms)
+static void pc_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg)
 {
 uint8_t *smbios_tables, *smbios_anchor;
 size_t smbios_tables_len, smbios_anchor_len;
@@ -900,7 +900,7 @@ static void pc_build_smbios(PCMachineState *pcms)
 
 smbios_tables = smbios_get_table_legacy(&smbios_tables_len);
 if (smbios_tables) {
-fw_cfg_add_bytes(pcms->fw_cfg, FW_CFG_SMBIOS_ENTRIES,
+fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES,
  smbios_tables, smbios_tables_len);
 }
 
@@ -921,9 +921,9 @@ static void pc_build_smbios(PCMachineState *pcms)
 g_free(mem_array);
 
 if (smbios_anchor) {
-fw_cfg_add_file(pcms->fw_cfg, "etc/smbios/smbios-tables",
+fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-tables",
 smbios_tables, smbios_tables_len);
-fw_cfg_add_file(pcms->fw_cfg, "etc/smbios/smbios-anchor",
+fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-anchor",
 smbios_anchor, smbios_anchor_len);
 }
 }
@@ -1587,7 +1587,7 @@ void pc_machine_done(Notifier *notifier, void *data)
 
 acpi_setup();
 if (pcms->fw_cfg) {
-pc_build_smbios(pcms);
+pc_build_smbios(pcms, pcms->fw_cfg);
 pc_build_feature_control_file(pcms);
 /* update FW_CFG_NB_CPUS to account for -device added CPUs */
 fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
-- 
2.20.1

[Qemu-devel] [PATCH 04/20] hw/i386/pc: Add the E820Type enum type

[Philippe Mathieu-Daudé]

This ensure we won't use an incorrect value.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc.c | 12 +++-
 include/hw/i386/pc.h | 16 ++--
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 1245028dd6..ac8343c728 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -868,9 +868,10 @@ static void handle_a20_line_change(void *opaque, int irq, 
int level)
 x86_cpu_set_a20(cpu, level);
 }
 
-ssize_t e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
+ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type type)
 {
 unsigned int index = le32_to_cpu(e820_reserve.count);
+uint32_t utype = (uint32_t)type;
 struct e820_entry *entry;
 
 if (type != E820_RAM) {
@@ -882,7 +883,7 @@ ssize_t e820_add_entry(uint64_t address, uint64_t length, 
uint32_t type)
 
 entry->address = cpu_to_le64(address);
 entry->length = cpu_to_le64(length);
-entry->type = cpu_to_le32(type);
+entry->type = cpu_to_le32(utype);
 
 e820_reserve.count = cpu_to_le32(index);
 }
@@ -891,7 +892,7 @@ ssize_t e820_add_entry(uint64_t address, uint64_t length, 
uint32_t type)
 e820_table = g_renew(struct e820_entry, e820_table, e820_entries + 1);
 e820_table[e820_entries].address = cpu_to_le64(address);
 e820_table[e820_entries].length = cpu_to_le64(length);
-e820_table[e820_entries].type = cpu_to_le32(type);
+e820_table[e820_entries].type = cpu_to_le32(utype);
 e820_entries++;
 
 return e820_entries;
@@ -902,10 +903,11 @@ size_t e820_get_num_entries(void)
 return e820_entries;
 }
 
-bool e820_get_entry(unsigned int idx, uint32_t type,
+bool e820_get_entry(unsigned int idx, E820Type type,
 uint64_t *address, uint64_t *length)
 {
-if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(type)) {
+uint32_t utype = (uint32_t)type;
+if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(utype)) {
 *address = le64_to_cpu(e820_table[idx].address);
 *length = le64_to_cpu(e820_table[idx].length);
 return true;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 2bc48c03c6..10e77a40ce 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -282,12 +282,16 @@ void pc_system_firmware_init(PCMachineState *pcms, 
MemoryRegion *rom_memory);
 void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
const CPUArchIdList *apic_ids, GArray *entry);
 
-/* e820 types */
-#define E820_RAM1
-#define E820_RESERVED   2
-#define E820_ACPI   3
-#define E820_NVS4
-#define E820_UNUSABLE   5
+/**
+ * E820Type: Type of the e820 address range.
+ */
+typedef enum {
+E820_RAM= 1,
+E820_RESERVED   = 2,
+E820_ACPI   = 3,
+E820_NVS= 4,
+E820_UNUSABLE   = 5
+} E820Type;
 
 ssize_t e820_add_entry(uint64_t, uint64_t, uint32_t);
 size_t e820_get_num_entries(void);
-- 
2.20.1

[Qemu-devel] [PATCH 09/20] hw/i386/pc: Rename bochs_bios_init() more generic as x86_create_fw_cfg()

[Philippe Mathieu-Daudé]

The bochs_bios_init() is not restricted to the Bochs BIOS and is
useful to other BIOS. Rename it to be more generic.

Suggested-by: Samuel Ortiz 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index a3936bb29d..264074489b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -928,7 +928,7 @@ static void pc_build_smbios(PCMachineState *pcms)
 }
 }
 
-static FWCfgState *bochs_bios_init(PCMachineState *pcms)
+static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms)
 {
 FWCfgState *fw_cfg;
 uint64_t *numa_fw_cfg;
@@ -1508,7 +1508,7 @@ void pc_cpus_init(PCMachineState *pcms)
  * Limit for the APIC ID value, so that all
  * CPU APIC IDs are < pcms->apic_id_limit.
  *
- * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
+ * This is used for FW_CFG_MAX_CPUS. See comments on x86_create_fw_cfg().
  */
 pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
 possible_cpus = mc->possible_cpu_arch_ids(ms);
@@ -1762,7 +1762,7 @@ void pc_memory_init(PCMachineState *pcms,
 option_rom_mr,
 1);
 
-fw_cfg = bochs_bios_init(pcms);
+fw_cfg = x86_create_fw_cfg(pcms);
 
 rom_set_fw(fw_cfg);
 
-- 
2.20.1

[Qemu-devel] [PATCH 13/20] hw/i386/pc: Let fw_cfg_init() use the generic MachineState

[Philippe Mathieu-Daudé]

We removed the PCMachineState access, we can now let the fw_cfg_init()
function to take a generic MachineState object.

Suggested-by: Samuel Ortiz 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 6cfdb09f34..e8813ef2b6 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -928,7 +928,7 @@ static void pc_build_smbios(PCMachineState *pcms)
 }
 }
 
-static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms, const CPUArchIdList 
*cpus,
+static FWCfgState *x86_create_fw_cfg(MachineState *ms, const CPUArchIdList 
*cpus,
  uint16_t boot_cpus, uint16_t 
apic_id_limit)
 {
 FWCfgState *fw_cfg;
@@ -1664,6 +1664,7 @@ void pc_memory_init(PCMachineState *pcms,
 MemoryRegion *ram_below_4g, *ram_above_4g;
 FWCfgState *fw_cfg;
 MachineState *machine = MACHINE(pcms);
+MachineClass *mc = MACHINE_GET_CLASS(machine);
 PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
 
 assert(machine->ram_size == pcms->below_4g_mem_size +
@@ -1760,7 +1761,7 @@ void pc_memory_init(PCMachineState *pcms,
 option_rom_mr,
 1);
 
-fw_cfg = x86_create_fw_cfg(pcms, mc->possible_cpu_arch_ids(machine),
+fw_cfg = x86_create_fw_cfg(machine, mc->possible_cpu_arch_ids(machine),
pcms->boot_cpus, pcms->apic_id_limit);
 
 rom_set_fw(fw_cfg);
-- 
2.20.1

[Qemu-devel] [PATCH 07/20] hw/i386/pc: Extract e820 memory layout code

[Philippe Mathieu-Daudé]

Suggested-by: Samuel Ortiz 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/Makefile.objs|  2 +-
 hw/i386/e820_memory_layout.c | 62 +
 hw/i386/e820_memory_layout.h | 76 
 hw/i386/pc.c | 64 +-
 include/hw/i386/pc.h | 48 ---
 target/i386/kvm.c|  1 +
 6 files changed, 141 insertions(+), 112 deletions(-)
 create mode 100644 hw/i386/e820_memory_layout.c
 create mode 100644 hw/i386/e820_memory_layout.h

diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 5d9c9efd5f..d3374e0831 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -1,5 +1,5 @@
 obj-$(CONFIG_KVM) += kvm/
-obj-y += multiboot.o
+obj-y += e820_memory_layout.o multiboot.o
 obj-y += pc.o
 obj-$(CONFIG_I440FX) += pc_piix.o
 obj-$(CONFIG_Q35) += pc_q35.o
diff --git a/hw/i386/e820_memory_layout.c b/hw/i386/e820_memory_layout.c
new file mode 100644
index 00..b9be08536c
--- /dev/null
+++ b/hw/i386/e820_memory_layout.c
@@ -0,0 +1,62 @@
+/*
+ * QEMU BIOS e820 routines
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/bswap.h"
+#include "e820_memory_layout.h"
+
+static size_t e820_entries;
+struct e820_table e820_reserve;
+struct e820_entry *e820_table;
+
+ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type type)
+{
+unsigned int index = le32_to_cpu(e820_reserve.count);
+uint32_t utype = (uint32_t)type;
+struct e820_entry *entry;
+
+if (type != E820_RAM) {
+/* old FW_CFG_E820_TABLE entry -- reservations only */
+if (index >= E820_NR_ENTRIES) {
+return -EBUSY;
+}
+entry = &e820_reserve.entry[index++];
+
+entry->address = cpu_to_le64(address);
+entry->length = cpu_to_le64(length);
+entry->type = cpu_to_le32(utype);
+
+e820_reserve.count = cpu_to_le32(index);
+}
+
+/* new "etc/e820" file -- include ram too */
+e820_table = g_renew(struct e820_entry, e820_table, e820_entries + 1);
+e820_table[e820_entries].address = cpu_to_le64(address);
+e820_table[e820_entries].length = cpu_to_le64(length);
+e820_table[e820_entries].type = cpu_to_le32(utype);
+e820_entries++;
+
+return e820_entries;
+}
+
+size_t e820_get_num_entries(void)
+{
+return e820_entries;
+}
+
+bool e820_get_entry(unsigned int idx, E820Type type,
+uint64_t *address, uint64_t *length)
+{
+uint32_t utype = (uint32_t)type;
+if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(utype)) {
+*address = le64_to_cpu(e820_table[idx].address);
+*length = le64_to_cpu(e820_table[idx].length);
+return true;
+}
+return false;
+}
diff --git a/hw/i386/e820_memory_layout.h b/hw/i386/e820_memory_layout.h
new file mode 100644
index 00..64e88e4772
--- /dev/null
+++ b/hw/i386/e820_memory_layout.h
@@ -0,0 +1,76 @@
+/*
+ * QEMU BIOS e820 routines
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#ifndef HW_I386_E820_H
+#define HW_I386_E820_H
+
+/**
+ * E820Type: Type of the e820 address range.
+ */
+typedef enum {
+E820_RAM= 1,
+E820_RESERVED   = 2,
+E820_ACPI   = 3,
+E820_NVS= 4,
+E820_UNUSABLE   = 5
+} E820Type;
+
+#define E820_NR_ENTRIES 16
+
+struct e820_entry {
+uint64_t address;
+uint64_t length;
+uint32_t type;
+} QEMU_PACKED __attribute((__aligned__(4)));
+
+struct e820_table {
+uint32_t count;
+struct e820_entry entry[E820_NR_ENTRIES];
+} QEMU_PACKED __attribute((__aligned__(4)));
+
+extern struct e820_table e820_reserve;
+extern struct e820_entry *e820_table;
+
+/**
+ * e820_add_entry: Add an #e820_entry to the @e820_table.
+ *
+ * Returns the number of entries of the e820_table on success,
+ * or a negative errno otherwise.
+ *
+ * @address: The base address of the structure which the BIOS is to fill in.
+ * @length: The length in bytes of the structure passed to the BIOS.
+ * @type: The #E820Type of the address range.
+ */
+ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type type);
+
+/**
+ * e820_get_num_entries: The number of entries of the @e820_table.
+ *
+ * Returns the number of entries of the e820_table.
+ */
+size_t e820_get_num_entries(void);
+
+/**
+ * e820_get_entry: Get the address/length of an #e820_entry.
+ *
+ * If the #e820_entry stored at @index is of #E820Type @type, fills @address
+ * and @length with the #e820_entry values and return @true.
+ * Return @false otherwise.
+ *
+ * @index: The index of the #e820_entry to get values.
+ * @type: The @E820Type of the address range expected.
+ * @address: Pointer to the base address of the #e820_entry structure to
+ *   be filled.
+ * @length: Pointer to the length (in bytes) of the #e820_entry structure
+ *  to be filled.
+ * @re

[Qemu-devel] [PATCH 03/20] hw/i386/pc: Let e820_add_entry() return a ssize_t type

[Philippe Mathieu-Daudé]

e820_add_entry() returns an array size on success, or a negative
value on error.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc.c | 2 +-
 include/hw/i386/pc.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index df8600ac24..1245028dd6 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -868,7 +868,7 @@ static void handle_a20_line_change(void *opaque, int irq, 
int level)
 x86_cpu_set_a20(cpu, level);
 }
 
-int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
+ssize_t e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
 {
 unsigned int index = le32_to_cpu(e820_reserve.count);
 struct e820_entry *entry;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 28b19173b0..2bc48c03c6 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -289,7 +289,7 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
 #define E820_NVS4
 #define E820_UNUSABLE   5
 
-int e820_add_entry(uint64_t, uint64_t, uint32_t);
+ssize_t e820_add_entry(uint64_t, uint64_t, uint32_t);
 size_t e820_get_num_entries(void);
 bool e820_get_entry(unsigned int, uint32_t, uint64_t *, uint64_t *);
 
-- 
2.20.1

[Qemu-devel] [PATCH 20/20] hw/i386/pc: Extract the x86 generic fw_cfg code

[Philippe Mathieu-Daudé]

Extract all the functions that are not PC-machine specific into
the (arch-specific) fw_cfg.c file. This will allow other X86-machine
to re-use these functions.

Suggested-by: Samuel Ortiz 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/fw_cfg.c | 135 +++
 hw/i386/fw_cfg.h |   6 +++
 hw/i386/pc.c | 128 +---
 3 files changed, 142 insertions(+), 127 deletions(-)

diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index 380a819230..bdd744c040 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -13,8 +13,15 @@
  */
 
 #include "qemu/osdep.h"
+#include "sysemu/numa.h"
+#include "hw/acpi/acpi.h"
+#include "hw/firmware/smbios.h"
+#include "hw/i386/pc.h"
 #include "hw/i386/fw_cfg.h"
+#include "hw/timer/hpet.h"
 #include "hw/nvram/fw_cfg.h"
+#include "e820_memory_layout.h"
+#include "kvm_i386.h"
 
 const char *fw_cfg_arch_key_name(uint16_t key)
 {
@@ -36,3 +43,131 @@ const char *fw_cfg_arch_key_name(uint16_t key)
 }
 return NULL;
 }
+
+void fw_cfg_build_smbios(MachineState *ms, FWCfgState *fw_cfg)
+{
+uint8_t *smbios_tables, *smbios_anchor;
+size_t smbios_tables_len, smbios_anchor_len;
+struct smbios_phys_mem_area *mem_array;
+unsigned i, array_count;
+X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
+
+/* tell smbios about cpuid version and features */
+smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
+
+smbios_tables = smbios_get_table_legacy(&smbios_tables_len);
+if (smbios_tables) {
+fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES,
+ smbios_tables, smbios_tables_len);
+}
+
+/* build the array of physical mem area from e820 table */
+mem_array = g_malloc0(sizeof(*mem_array) * e820_get_num_entries());
+for (i = 0, array_count = 0; i < e820_get_num_entries(); i++) {
+uint64_t addr, len;
+
+if (e820_get_entry(i, E820_RAM, &addr, &len)) {
+mem_array[array_count].address = addr;
+mem_array[array_count].length = len;
+array_count++;
+}
+}
+smbios_get_tables(mem_array, array_count,
+  &smbios_tables, &smbios_tables_len,
+  &smbios_anchor, &smbios_anchor_len);
+g_free(mem_array);
+
+if (smbios_anchor) {
+fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-tables",
+smbios_tables, smbios_tables_len);
+fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-anchor",
+smbios_anchor, smbios_anchor_len);
+}
+}
+
+void fw_cfg_build_feature_control(MachineState *ms, FWCfgState *fw_cfg)
+{
+X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
+CPUX86State *env = &cpu->env;
+uint32_t unused, ecx, edx;
+uint64_t feature_control_bits = 0;
+uint64_t *val;
+
+cpu_x86_cpuid(env, 1, 0, &unused, &unused, &ecx, &edx);
+if (ecx & CPUID_EXT_VMX) {
+feature_control_bits |= FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX;
+}
+
+if ((edx & (CPUID_EXT2_MCE | CPUID_EXT2_MCA)) ==
+(CPUID_EXT2_MCE | CPUID_EXT2_MCA) &&
+(env->mcg_cap & MCG_LMCE_P)) {
+feature_control_bits |= FEATURE_CONTROL_LMCE;
+}
+
+if (!feature_control_bits) {
+return;
+}
+
+val = g_malloc(sizeof(*val));
+*val = cpu_to_le64(feature_control_bits | FEATURE_CONTROL_LOCKED);
+fw_cfg_add_file(fw_cfg, "etc/msr_feature_control", val, sizeof(*val));
+}
+
+FWCfgState *x86_create_fw_cfg(MachineState *ms, const CPUArchIdList *cpus,
+  uint16_t boot_cpus, uint16_t apic_id_limit)
+{
+FWCfgState *fw_cfg;
+uint64_t *numa_fw_cfg;
+int i;
+
+fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,
+&address_space_memory);
+fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, boot_cpus);
+
+/*
+ * FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
+ *
+ * For machine types prior to 1.8, SeaBIOS needs FW_CFG_MAX_CPUS for
+ * building MPTable, ACPI MADT, ACPI CPU hotplug and ACPI SRAT table,
+ * that tables are based on xAPIC ID and QEMU<->SeaBIOS interface
+ * for CPU hotplug also uses APIC ID and not "CPU index".
+ * This means that FW_CFG_MAX_CPUS is not the "maximum number of CPUs",
+ * but the "limit to the APIC ID values SeaBIOS may see".
+ *
+ * So for compatibility reasons with old BIOSes we are stuck with
+ * "etc/max-cpus" actually being apic_id_limit
+ */
+fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, apic_id_limit);
+fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
+fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES,
+ acpi_tables, acpi_tables_len);
+fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
+
+fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE,
+ &e820_reserve, sizeof(e820_reserve));
+fw_cfg_add_file(fw_cfg

[Qemu-devel] [PATCH 12/20] hw/i386/pc: Pass the CPUArchIdList array by argument

[Philippe Mathieu-Daudé]

Pass the CPUArchIdList array by argument, this will
allow us to remove the PCMachineState argument later.

Suggested-by: Samuel Ortiz 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc.c | 18 --
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index fe07baeb1d..6cfdb09f34 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -928,14 +928,12 @@ static void pc_build_smbios(PCMachineState *pcms)
 }
 }
 
-static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms,
+static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms, const CPUArchIdList 
*cpus,
  uint16_t boot_cpus, uint16_t 
apic_id_limit)
 {
 FWCfgState *fw_cfg;
 uint64_t *numa_fw_cfg;
 int i;
-const CPUArchIdList *cpus;
-MachineClass *mc = MACHINE_GET_CLASS(pcms);
 
 fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,
 &address_space_memory);
@@ -953,7 +951,7 @@ static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms,
  * So for compatibility reasons with old BIOSes we are stuck with
  * "etc/max-cpus" actually being apic_id_limit
  */
-fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)pcms->apic_id_limit);
+fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, apic_id_limit);
 fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
 fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES,
  acpi_tables, acpi_tables_len);
@@ -969,20 +967,19 @@ static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms,
  * of nodes, one word for each VCPU->node and one word for each node to
  * hold the amount of memory.
  */
-numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes);
+numa_fw_cfg = g_new0(uint64_t, 1 + apic_id_limit + nb_numa_nodes);
 numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
-cpus = mc->possible_cpu_arch_ids(MACHINE(pcms));
 for (i = 0; i < cpus->len; i++) {
 unsigned int apic_id = cpus->cpus[i].arch_id;
-assert(apic_id < pcms->apic_id_limit);
+assert(apic_id < apic_id_limit);
 numa_fw_cfg[apic_id + 1] = cpu_to_le64(cpus->cpus[i].props.node_id);
 }
 for (i = 0; i < nb_numa_nodes; i++) {
-numa_fw_cfg[pcms->apic_id_limit + 1 + i] =
+numa_fw_cfg[apic_id_limit + 1 + i] =
 cpu_to_le64(numa_info[i].node_mem);
 }
 fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, numa_fw_cfg,
- (1 + pcms->apic_id_limit + nb_numa_nodes) *
+ (1 + apic_id_limit + nb_numa_nodes) *
  sizeof(*numa_fw_cfg));
 
 return fw_cfg;
@@ -1763,7 +1760,8 @@ void pc_memory_init(PCMachineState *pcms,
 option_rom_mr,
 1);
 
-fw_cfg = x86_create_fw_cfg(pcms, pcms->boot_cpus, pcms->apic_id_limit);
+fw_cfg = x86_create_fw_cfg(pcms, mc->possible_cpu_arch_ids(machine),
+   pcms->boot_cpus, pcms->apic_id_limit);
 
 rom_set_fw(fw_cfg);
 
-- 
2.20.1

[Qemu-devel] [PATCH 01/20] hw/i386/pc: Use unsigned type to index arrays

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc.c | 5 +++--
 include/hw/i386/pc.h | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2632b73f80..fc38b59d2d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -870,7 +870,7 @@ static void handle_a20_line_change(void *opaque, int irq, 
int level)
 
 int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
 {
-int index = le32_to_cpu(e820_reserve.count);
+unsigned int index = le32_to_cpu(e820_reserve.count);
 struct e820_entry *entry;
 
 if (type != E820_RAM) {
@@ -902,7 +902,8 @@ int e820_get_num_entries(void)
 return e820_entries;
 }
 
-bool e820_get_entry(int idx, uint32_t type, uint64_t *address, uint64_t 
*length)
+bool e820_get_entry(unsigned int idx, uint32_t type,
+uint64_t *address, uint64_t *length)
 {
 if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(type)) {
 *address = le64_to_cpu(e820_table[idx].address);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 43df7230a2..ad3a75d8fa 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -291,7 +291,7 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
 
 int e820_add_entry(uint64_t, uint64_t, uint32_t);
 int e820_get_num_entries(void);
-bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
+bool e820_get_entry(unsigned int, uint32_t, uint64_t *, uint64_t *);
 
 extern GlobalProperty pc_compat_4_0[];
 extern const size_t pc_compat_4_0_len;
-- 
2.20.1

[Qemu-devel] [PATCH 05/20] hw/i386/pc: Add documentation to the e820_*() functions

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/i386/pc.h | 37 +++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 10e77a40ce..95bf3278f2 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -293,9 +293,42 @@ typedef enum {
 E820_UNUSABLE   = 5
 } E820Type;
 
-ssize_t e820_add_entry(uint64_t, uint64_t, uint32_t);
+/**
+ * e820_add_entry: Add an #e820_entry to the @e820_table.
+ *
+ * Returns the number of entries of the e820_table on success,
+ * or a negative errno otherwise.
+ *
+ * @address: The base address of the structure which the BIOS is to fill in.
+ * @length: The length in bytes of the structure passed to the BIOS.
+ * @type: The #E820Type of the address range.
+ */
+ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type type);
+
+/**
+ * e820_get_num_entries: The number of entries of the @e820_table.
+ *
+ * Returns the number of entries of the e820_table.
+ */
 size_t e820_get_num_entries(void);
-bool e820_get_entry(unsigned int, uint32_t, uint64_t *, uint64_t *);
+
+/**
+ * e820_get_entry: Get the address/length of an #e820_entry.
+ *
+ * If the #e820_entry stored at @index is of #E820Type @type, fills @address
+ * and @length with the #e820_entry values and return @true.
+ * Return @false otherwise.
+ *
+ * @index: The index of the #e820_entry to get values.
+ * @type: The @E820Type of the address range expected.
+ * @address: Pointer to the base address of the #e820_entry structure to
+ *   be filled.
+ * @length: Pointer to the length (in bytes) of the #e820_entry structure
+ *  to be filled.
+ * @return: true if the entry was found, false otherwise.
+ */
+bool e820_get_entry(unsigned int index, E820Type type,
+uint64_t *address, uint64_t *length);
 
 extern GlobalProperty pc_compat_4_0[];
 extern const size_t pc_compat_4_0_len;
-- 
2.20.1

[Qemu-devel] [PATCH 11/20] hw/i386/pc: Pass the apic_id_limit value by argument

[Philippe Mathieu-Daudé]

Pass the apic_id_limit value by argument, this will
allow us to remove the PCMachineState argument later.

Suggested-by: Samuel Ortiz 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 01894b9875..fe07baeb1d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -928,7 +928,8 @@ static void pc_build_smbios(PCMachineState *pcms)
 }
 }
 
-static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms, uint16_t boot_cpus)
+static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms,
+ uint16_t boot_cpus, uint16_t 
apic_id_limit)
 {
 FWCfgState *fw_cfg;
 uint64_t *numa_fw_cfg;
@@ -1762,7 +1763,7 @@ void pc_memory_init(PCMachineState *pcms,
 option_rom_mr,
 1);
 
-fw_cfg = x86_create_fw_cfg(pcms, pcms->boot_cpus);
+fw_cfg = x86_create_fw_cfg(pcms, pcms->boot_cpus, pcms->apic_id_limit);
 
 rom_set_fw(fw_cfg);
 
-- 
2.20.1

[Qemu-devel] [PATCH 08/20] hw/i386/pc: Use address_space_memory in place

[Philippe Mathieu-Daudé]

The address_space_memory variable is used once.
Use it in place and remove the argument.

Suggested-by: Samuel Ortiz 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index fc22779ac1..a3936bb29d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -928,7 +928,7 @@ static void pc_build_smbios(PCMachineState *pcms)
 }
 }
 
-static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
+static FWCfgState *bochs_bios_init(PCMachineState *pcms)
 {
 FWCfgState *fw_cfg;
 uint64_t *numa_fw_cfg;
@@ -936,7 +936,8 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, 
PCMachineState *pcms)
 const CPUArchIdList *cpus;
 MachineClass *mc = MACHINE_GET_CLASS(pcms);
 
-fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4, as);
+fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,
+&address_space_memory);
 fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
 
 /* FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
@@ -1761,7 +1762,7 @@ void pc_memory_init(PCMachineState *pcms,
 option_rom_mr,
 1);
 
-fw_cfg = bochs_bios_init(&address_space_memory, pcms);
+fw_cfg = bochs_bios_init(pcms);
 
 rom_set_fw(fw_cfg);
 
-- 
2.20.1

[Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine

[Philippe Mathieu-Daudé]

Hi,

This is my take at salvaging some NEMU good work.
Samuel worked in adding the fw_cfg device to the x86-virt NEMU machine.
This series is inspired by NEMU's commit 3cb92d080835 [*] and adapted
to upstream style. The result makes the upstream codebase more
modularizable.
There are very little logical changes, this is mostly a cleanup
refactor.

Regards,

Phil.

[*] 
https://github.com/intel/nemu/commit/3cb92d080835ac8d47c8b713156338afa33cff5c

Philippe Mathieu-Daudé (20):
  hw/i386/pc: Use unsigned type to index arrays
  hw/i386/pc: Use size_t type to hold/return a size of array
  hw/i386/pc: Let e820_add_entry() return a ssize_t type
  hw/i386/pc: Add the E820Type enum type
  hw/i386/pc: Add documentation to the e820_*() functions
  hw/i386/pc: Use e820_get_num_entries() to access e820_entries
  hw/i386/pc: Extract e820 memory layout code
  hw/i386/pc: Use address_space_memory in place
  hw/i386/pc: Rename bochs_bios_init() more generic as
x86_create_fw_cfg()
  hw/i386/pc: Pass the boot_cpus value by argument
  hw/i386/pc: Pass the apic_id_limit value by argument
  hw/i386/pc: Pass the CPUArchIdList array by argument
  hw/i386/pc: Let fw_cfg_init() use the generic MachineState
  hw/i386/pc: Let pc_build_smbios() take a FWCfgState argument
  hw/i386/pc: Let pc_build_smbios() take a generic MachineState argument
  hw/i386/pc: Rename pc_build_smbios() as generic fw_cfg_build_smbios()
  hw/i386/pc: Let pc_build_feature_control() take a FWCfgState argument
  hw/i386/pc: Let pc_build_feature_control() take a MachineState
argument
  hw/i386/pc: Rename pc_build_feature_control() as generic
fw_cfg_build_*
  hw/i386/pc: Extract the x86 generic fw_cfg code

 hw/i386/Makefile.objs|   2 +-
 hw/i386/e820_memory_layout.c |  62 +++
 hw/i386/e820_memory_layout.h |  76 +
 hw/i386/fw_cfg.c | 135 +++
 hw/i386/fw_cfg.h |   6 ++
 hw/i386/pc.c | 201 ++-
 include/hw/i386/pc.h |  11 --
 target/i386/kvm.c|   1 +
 8 files changed, 289 insertions(+), 205 deletions(-)
 create mode 100644 hw/i386/e820_memory_layout.c
 create mode 100644 hw/i386/e820_memory_layout.h

-- 
2.20.1

[Qemu-devel] [PATCH 02/20] hw/i386/pc: Use size_t type to hold/return a size of array

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc.c | 4 ++--
 include/hw/i386/pc.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index fc38b59d2d..df8600ac24 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -104,7 +104,7 @@ struct e820_table {
 
 static struct e820_table e820_reserve;
 static struct e820_entry *e820_table;
-static unsigned e820_entries;
+static size_t e820_entries;
 struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
 
 /* Physical Address of PVH entry point read from kernel ELF NOTE */
@@ -897,7 +897,7 @@ int e820_add_entry(uint64_t address, uint64_t length, 
uint32_t type)
 return e820_entries;
 }
 
-int e820_get_num_entries(void)
+size_t e820_get_num_entries(void)
 {
 return e820_entries;
 }
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index ad3a75d8fa..28b19173b0 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -290,7 +290,7 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
 #define E820_UNUSABLE   5
 
 int e820_add_entry(uint64_t, uint64_t, uint32_t);
-int e820_get_num_entries(void);
+size_t e820_get_num_entries(void);
 bool e820_get_entry(unsigned int, uint32_t, uint64_t *, uint64_t *);
 
 extern GlobalProperty pc_compat_4_0[];
-- 
2.20.1

Re: [Qemu-devel] [PULL 00/38] ppc-for-4.1 queue 20190522

[David Gibson]

On Fri, May 24, 2019 at 12:17:25PM +1000, David Gibson wrote:
> On Fri, May 24, 2019 at 10:46:16AM +1000, David Gibson wrote:
> > On Thu, May 23, 2019 at 10:29:57AM +0100, Peter Maydell wrote:
> > > On Wed, 22 May 2019 at 05:46, David Gibson  
> > > wrote:
> > > >
> > > > The following changes since commit 
> > > > a4f667b6714916683408b983cfe0a615a725775f:
> > > >
> > > >   Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20190521-3' 
> > > > into staging (2019-05-21 16:30:13 +0100)
> > > >
> > > > are available in the Git repository at:
> > > >
> > > >   git://github.com/dgibson/qemu.git tags/ppc-for-4.1-20190522
> > > >
> > > > for you to fetch changes up to 885659103ad9e1b0460e89c548e9fb823b007b7e:
> > > >
> > > >   docs: provide documentation on the POWER9 XIVE interrupt controller 
> > > > (2019-05-22 10:38:22 +1000)
> > > >
> > > > 
> > > > ppc patch queue 2019-05-22
> > > >
> > > > Next pull request against qemu-4.1.  Highlights:
> > > >   * KVM accelerated support for the XIVE interrupt controller in PAPR
> > > > guests
> > > >   * A number of TCG vector fixes
> > > >   * Fixes for the PReP / 40p machine
> > > >
> > > > Other than that it's just a bunch of assorted fixes, cleanups and
> > > > minor improvements.
> > > >
> > > > This supersedes the pull request dated 2019-05-21.  I've dropped the
> > > > headers update, since it had a mistake, but is now made redundant by a
> > > > similar update in Cornelia's applied PR.  I've also added a patch with
> > > > extra documentation for the XIVE interrupt controller, and fixed a
> > > > couple of minor style errors in the previous PR.
> > > >
> > > > 
> > > 
> > > Hi -- this failed 'make check-tcg' for the linux-static build
> > > (x86-64 host):
> > > 
> > >   TESTlinux-test on ppc64abi32
> > > qemu-ppc64abi32:
> > > /home/petmay01/linaro/qemu-for-merges/accel/tcg/cpu-exec.c:700:
> > > cpu_exec: Assertion `!have_mmap_lock()' failed.
> > > qemu-ppc64abi32:
> > > /home/petmay01/linaro/qemu-for-merges/accel/tcg/cpu-exec.c:700:
> > > cpu_exec: Assertion `!have_mmap_lock()' failed.
> > > /home/petmay01/linaro/qemu-for-merges/tests/tcg/Makefile:122: recipe
> > > for target 'run-linux-test' failed
> > 
> > Bother.  I did run a make check-tcg on an x86_64 host, and didn't see
> > this.  Investigating...
> 
>   BUILD   TCG tests for ppc64abi32-linux-user
>   BUILD   ppc64abi32 guest-tests SKIPPED
>   RUN TCG tests for ppc64abi32-linux-user
>   RUN tests for ppc64abi32 SKIPPED
> 
> Well, that explains why I didn't see it.  I'm using the docker setup
> for check-tcg, but it's not clear to me what bits I need to run the
> ppc64abi32 tests.

What I can find seems to imply that it should use a a 32-bit ppc
target compiler, but if I do that the test SEGVs on master, not just
on my branch.

I'm having a lot of trouble even figuring out what the ppc64abi32
target is supposed to be about.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 1/7] virtio-pmem: add virtio device

[Pankaj Gupta]

> 
> On 5/23/19 5:24 AM, Pankaj Gupta wrote:
> > This is the implementation of virtio-pmem device. Support will require
> > machine changes for the architectures that will support it, so it will
> > not yet be compiled. It can be unlocked with VIRTIO_PMEM_SUPPORTED per
> > machine and disabled globally via VIRTIO_PMEM.
> > 
> > We cannot use the "addr" property as that is already used e.g. for
> > virtio-pci/pci devices. And we will have e.g. virtio-pmem-pci as a proxy.
> > So we have to choose a different one (unfortunately). "memaddr" it is.
> > That name should ideally be used by all other virtio-* based memory
> > devices in the future.
> > -device virtio-pmem-pci,id=p0,bus=bux0,addr=0x01,memaddr=0x100...
> > 
> > Acked-by: Markus Armbruster 
> > [ QAPI bits ]
> > Signed-off-by: Pankaj Gupta 
> > [ MemoryDevice/MemoryRegion changes, cleanups, addr property "memaddr",
> >   split up patches, unplug handler ]
> > Signed-off-by: David Hildenbrand 
> > ---
> 
> > +++ b/qapi/misc.json
> > @@ -2742,16 +2742,42 @@
> >}
> >  }
> >  
> > +##
> > +# @VirtioPMEMDeviceInfo:
> > +#
> > +# VirtioPMEM state information
> > +#
> > +# @id: device's ID
> > +#
> > +# @memaddr: physical address in memory, where device is mapped
> > +#
> > +# @size: size of memory that the device provides
> > +#
> > +# @memdev: memory backend linked with device
> > +#
> > +# Since: 4.0
> 
> You've missed 4.0; this should be 4.1.

Yes. Will update.

> 
> > +##
> > +{ 'struct': 'VirtioPMEMDeviceInfo',
> > +  'data': { '*id': 'str',
> 
> Why is id optional? Does it make sense to have a device without an id?

I think that's how it has been for both NVDIMM and DIMM. And it works
fine with optional 'id', but requires unique 'id' for underlying memory device. 
That means its okay to keep 'id' optional for root dimm/nvdimm/virtio-pmem 
devices. 

Thanks,
Pankaj

> 
> > +'memaddr': 'size',
> > +'size': 'size',
> > +'memdev': 'str'
> > +  }
> > +}
> > +
> >  ##
> >  # @MemoryDeviceInfo:
> >  #
> >  # Union containing information about a memory device
> >  #
> > +# nvdimm is included since 2.12. virtio-pmem is included since 4.0.
> 
> 4.1

o.k

> 
> > +#
> >  # Since: 2.1
> >  ##
> >  { 'union': 'MemoryDeviceInfo',
> >'data': { 'dimm': 'PCDIMMDeviceInfo',
> > -'nvdimm': 'PCDIMMDeviceInfo'
> > +'nvdimm': 'PCDIMMDeviceInfo',
> > +'virtio-pmem': 'VirtioPMEMDeviceInfo'
> >}
> >  }
> >  
> > 
> 
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
> 
> 

Re: [Qemu-devel] [RFC v4 5/7] tests: New make target check-source

[Philippe Mathieu-Daudé]

On 5/23/19 10:15 AM, Markus Armbruster wrote:
> Make target check-source is for checking the source code itself.  For
> now, there's just one such check, make target check-headers.  It
> checks basic header sanity: for each header "FOO.h", test whether
> 
>   #include "qemu/osdep.h"
>   #include "FOO.h"
>   #include "FOO.h"
> 
> compiles.
> 
> The test works only in a git tree, with git installed.  It is skipped
> unless $(SRC_PATH)/.git exists.
> 
> Third-party headers we don't intend to clean up are excluded from this
> test.  So are a few "funny" headers.  See make variable
> excluded-headers.
> 
> A large number of headers don't pass this test, by design or by
> accident.  To keep things more manageable, exclude all headers outside
> include/ for now.
> 
> Headers known to fail the test are marked with
> 
> /* FIXME Does not pass make check-headers, yet! */
> 
> Headers known to work only in certain configurations are marked like
> 
> /* FIXME Does not pass make check-headers without CONFIG_WIN32, yet! */
> 
> I tried to find and mark all of them by testing various
> configurations.  Still, "make check" might fail for configurations I
> didn't test.
> 
> Known issue: some of these don't actually need fixing; they're *meant*
> to work only in certain configurations.  We'll want to invent a
> suitable marker that doesn't claim FIXME.
> 
> Some headers may only be included into target-dependent code: they use
> identifiers poisoned by exec/poison.h, or include cpu.h.  These
> headers are marked with a comment
> 
> /* NOTE: May only be included into target-dependent code */
> 
> The test treats them specially.
> 
> Known issue: some of these are intended for specific targets.  The
> test should skip them for other targets, but doesn't.  They're marked
> FIXME instead, which is wrong.
> 
> New make target check-bad-headers runs the test for headers expected
> to fail it.  This helps with examining the failures.
> 
> Signed-off-by: Markus Armbruster 
> ---
[...]>
> diff --git a/Makefile b/Makefile
> index 59de8e2494..42f02c5ceb 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -416,6 +416,8 @@ dummy := $(call unnest-vars,, \
>  audio-obj-m \
>  trace-obj-y)
>  
> +RECURSIVE_TARGETS := all clean install
> +
>  include $(SRC_PATH)/tests/Makefile.include
>  
>  all: $(DOCS) $(if $(BUILD_DOCS),sphinxdocs) $(TOOLS) $(HELPERS-y) 
> recurse-all modules
> @@ -436,7 +438,7 @@ config-host.h-timestamp: config-host.mak
>  qemu-options.def: $(SRC_PATH)/qemu-options.hx $(SRC_PATH)/scripts/hxtool
>   $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > 
> $@,"GEN","$@")
>  
> -TARGET_DIRS_RULES := $(foreach t, all clean install, $(addsuffix /$(t), 
> $(TARGET_DIRS)))
> +TARGET_DIRS_RULES:=$(foreach t, $(RECURSIVE_TARGETS), $(addsuffix /$(t), 
> $(TARGET_DIRS)))
>  
>  SOFTMMU_ALL_RULES=$(filter %-softmmu/all, $(TARGET_DIRS_RULES))
>  $(SOFTMMU_ALL_RULES): $(authz-obj-y)
> diff --git a/Makefile.target b/Makefile.target
> index fdbe7c89f4..a46cfda580 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -41,6 +41,7 @@ STPFILES=
>  
>  # Makefile Tests
>  include $(SRC_PATH)/tests/tcg/Makefile.include
> +include $(SRC_PATH)/tests/check-headers.mak
>  
>  config-target.h: config-target.h-timestamp
>  config-target.h-timestamp: config-target.mak
> @@ -216,6 +217,22 @@ hmp-commands.h: $(SRC_PATH)/hmp-commands.hx 
> $(SRC_PATH)/scripts/hxtool
>  hmp-commands-info.h: $(SRC_PATH)/hmp-commands-info.hx 
> $(SRC_PATH)/scripts/hxtool
>   $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > 
> $@,"GEN","$(TARGET_DIR)$@")
>  
> +.PHONY: check-headers
> +ifeq ($(wildcard $(SRC_PATH)/.git),)
> +check-headers check-bad-headers:
> + @echo "  SKIP  $@ (requires a git tree)"
> +else
> +check-headers: $(check-target-header-tests:.c=.o)
> +
> +# Expected to fail:
> +check-bad-headers: $(check-bad-target-header-tests:.c=.o)
> +
> +.SECONDARY: $(check-target-header-tests)
> +$(check-target-header-tests) $(check-bad-target-header-tests): 
> tests/header-test-template.c
> + @mkdir -p $(dir $@)
> + @sed 's,@header@,$(subst tests/headers/,,$(@:.c=.h)),' <$< >$@
> +endif
> +
>  clean: clean-target
>   rm -f *.a *~ $(PROGS)
>   rm -f $(shell find . -name '*.[od]')
> @@ -238,3 +255,5 @@ endif
>  
>  generated-files-y += config-target.h
>  Makefile: $(generated-files-y)
> +
> +-include $(check-target-header-tests:.c=.d) 
> $(check-bad-target-header-tests:.c=.d)

$ make microblazeel-softmmu/tests/headers/include/exec/user/abitypes.o
./include/exec/user/abitypes.h:6:10: fatal error: cpu.h: No such file or
directory
make: *** [./rules.mak:69:
microblazeel-softmmu/tests/headers/include/exec/user/abitypes.o] Error 1

^ this one looks legit, it's arch-specific, right?

$ make tests/headers/include/hw/net/lance.o
  CC  tests/headers/include/hw/net/lance.o
In file included from tests/headers/include/hw/net/lance.c:14:
./include/hw/net/lance.h:42:5: error: unk

Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/8] spapr: Clean up dt creation for PCI buses

[Alexey Kardashevskiy]

On 23/05/2019 15:29, David Gibson wrote:
> Device nodes for PCI bridges (both host and P2P) describe both the bridge
> device itself and the bus hanging off it, handling of this is a bit of a
> mess.
> 
> spapr_dt_pci_device() has a few things it only adds for non-bridges, but
> always adds #address-cells and #size-cells which should only appear for
> bridges.  But the walking down the subordinate PCI bus is done in one of
> its callers spapr_populate_pci_devices_dt().  The PHB dt creation in
> spapr_populate_pci_dt() open codes some similar logic to the bridge case.
> 
> This patch consolidates things in a bunch of ways:
>  * Bus specific dt info is now created in spapr_dt_pci_bus() used for both
>P2P bridges and the host bridge.  This includes walking subordinate
>devices
>  * spapr_dt_pci_device() now calls spapr_dt_pci_bus() when called on a
>P2P bridge
>  * We do detection of bridges with the is_bridge field of the device class,
>rather than checking PCI config space directly, for consistency with
>qemu's core PCI code.
>  * Several things are renamed for brevity and clarity
> 
> Signed-off-by: David Gibson 
> ---
>  hw/ppc/spapr.c  |   7 +-
>  hw/ppc/spapr_pci.c  | 140 +++-
>  include/hw/pci-host/spapr.h |   4 +-
>  3 files changed, 79 insertions(+), 72 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index e2b33e5890..44573adce7 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1309,8 +1309,7 @@ static void *spapr_build_fdt(SpaprMachineState *spapr)
>  }
>  
>  QLIST_FOREACH(phb, &spapr->phbs, list) {
> -ret = spapr_populate_pci_dt(phb, PHANDLE_INTC, fdt,
> -spapr->irq->nr_msis, NULL);
> +ret = spapr_dt_phb(phb, PHANDLE_INTC, fdt, spapr->irq->nr_msis, 
> NULL);
>  if (ret < 0) {
>  error_report("couldn't setup PCI devices in fdt");
>  exit(1);
> @@ -3917,8 +3916,8 @@ int spapr_phb_dt_populate(SpaprDrc *drc, 
> SpaprMachineState *spapr,
>  return -1;
>  }
>  
> -if (spapr_populate_pci_dt(sphb, intc_phandle, fdt, spapr->irq->nr_msis,
> -  fdt_start_offset)) {
> +if (spapr_dt_phb(sphb, intc_phandle, fdt, spapr->irq->nr_msis,
> + fdt_start_offset)) {
>  error_setg(errp, "unable to create FDT node for PHB %d", 
> sphb->index);
>  return -1;
>  }
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 4075b433fd..c166df4145 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1219,6 +1219,60 @@ static const char *dt_name_from_class(uint8_t class, 
> uint8_t subclass,
>  static uint32_t spapr_phb_get_pci_drc_index(SpaprPhbState *phb,
>  PCIDevice *pdev);
>  
> +typedef struct PciWalkFdt {
> +void *fdt;
> +int offset;
> +SpaprPhbState *sphb;
> +int err;
> +} PciWalkFdt;
> +
> +static int spapr_dt_pci_device(SpaprPhbState *sphb, PCIDevice *dev,
> +   void *fdt, int parent_offset);
> +
> +static void spapr_dt_pci_device_cb(PCIBus *bus, PCIDevice *pdev,
> +   void *opaque)
> +{
> +PciWalkFdt *p = opaque;
> +int err;
> +
> +if (p->err) {
> +/* Something's already broken, don't keep going */
> +return;
> +}
> +
> +err = spapr_dt_pci_device(p->sphb, pdev, p->fdt, p->offset);
> +if (err < 0) {
> +p->err = err;
> +}
> +}
> +
> +/* Augment PCI device node with bridge specific information */
> +static int spapr_dt_pci_bus(SpaprPhbState *sphb, PCIBus *bus,
> +   void *fdt, int offset)
> +{
> +PciWalkFdt cbinfo = {
> +.fdt = fdt,
> +.offset = offset,
> +.sphb = sphb,
> +.err = 0,
> +};
> +
> +_FDT(fdt_setprop_cell(fdt, offset, "#address-cells",
> +  RESOURCE_CELLS_ADDRESS));
> +_FDT(fdt_setprop_cell(fdt, offset, "#size-cells",
> +  RESOURCE_CELLS_SIZE));
> +
> +if (bus) {
> +pci_for_each_device_reverse(bus, pci_bus_num(bus),
> +spapr_dt_pci_device_cb, &cbinfo);
> +if (cbinfo.err) {
> +return cbinfo.err;
> +}
> +}
> +
> +return offset;


This spapr_dt_pci_bus() returns 0 or an offset or an error.

But:
spapr_dt_phb() returns 0 or error; and so does spapr_dt_drc().

Not extremely consistent.

It looks like spapr_dt_pci_bus() does not need to return @offset as it
does not change it and the caller - spapr_dt_pci_device() - can have 1
"return offset" in the end.



> +}
> +
>  /* create OF node for pci device and required OF DT properties */
>  static int spapr_dt_pci_device(SpaprPhbState *sphb, PCIDevice *dev,
> void *fdt, int parent_offset)
> @@ -1228,10 +1282,9 @@ static int spapr_dt_pci_device(SpaprPhbState *sphb, 
> PCI

Re: [Qemu-devel] [PATCH] hw/rdma: Delete unused headers inclusion

[Marcel Apfelbaum]

Hi Markus,

On 5/23/19 6:24 PM, Markus Armbruster wrote:

Looks stuck.  Let's try qemu-trivial.


Thanks, that is a great idea.

I didn't want to send a single-patch pull request,
especially while another is waiting for reviews.
Anyway, trivial queue can be used indeed.

Thanks,
Marcel



Yuval Shaia  writes:


This is a trivial cleanup patch.

Signed-off-by: Yuval Shaia 
---
  hw/rdma/rdma_backend.c | 7 ---
  1 file changed, 7 deletions(-)

diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index d1660b6474..05f6b03221 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -14,16 +14,9 @@
   */
  
  #include "qemu/osdep.h"

-#include "sysemu/sysemu.h"
-#include "qapi/error.h"
-#include "qapi/qmp/qlist.h"
-#include "qapi/qmp/qnum.h"
  #include "qapi/qapi-events-rdma.h"
  
  #include 

-#include 
-#include 
-#include 
  
  #include "contrib/rdmacm-mux/rdmacm-mux.h"

  #include "trace.h"

Re: [Qemu-devel] [RFC v4 3/7] Makefile: Rename targets for make recursion

[Philippe Mathieu-Daudé]

On 5/23/19 10:15 AM, Markus Armbruster wrote:
> We make a few sub-directories recursively, in particular
> $(TARGET_DIRS).
> 
> For goal "all", we do it the nice way: "all" has a prerequisite
> subdir-T for each T in $(TARGET_DIRS), and T's recipe runs make
> recursively.  Behaves nicely with -j and -k.
> 
> For other goals such as "clean" and "install", the recipe runs make
> recursively in a for loop.  Ignores -j and -k.
> 
> The next commit will fix that for "clean" and "install".  This commit
> prepares the ground by renaming the targets we use for "all" to
> include the goal for the sub-make.  This will permit reusing them for
> goals other than "all".
> 
> Targets subdir-T for T in $(TARGET_DIRS) run "make all" in T.  Rename
> to T/all, and declare phony.
> 
> Targets romsubdir-R for R in $(ROMS) run "make" in pc-bios/R.  Default
> goal is "all" for all R.  Rename to pc-bios/R/all, and declare phony.
> 
> The remainder are renamed just for consistency.
> 
> Target subdir-dtc runs "make libbft/libfdt.a" in dtc.  Rename to
> dtc/all, and declare phony.
> 
> Target subdir-capstone runs make $(BUILD_DIR)/capstone/$(LIBCAPSTONE)
> in $(SRC_PATH)/capstone.  Rename to capstone/all, and declare phony.
> 
> Target subdir-slirp runs "make" in $(SRC_PATH)/slirp.  Default goal is
> all, which builds $(BUILD_DIR)/libslirp.a.  Rename to slirp/all, and
> declare phony.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  Makefile   | 30 ++
>  configure  |  6 +++---
>  tests/Makefile.include |  3 ++-
>  3 files changed, 23 insertions(+), 16 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 12e470fa03..09c726bcc2 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -436,8 +436,8 @@ config-host.h-timestamp: config-host.mak
>  qemu-options.def: $(SRC_PATH)/qemu-options.hx $(SRC_PATH)/scripts/hxtool
>   $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > 
> $@,"GEN","$@")
>  
> -SUBDIR_RULES=$(patsubst %,subdir-%, $(TARGET_DIRS))
> -SOFTMMU_SUBDIR_RULES=$(filter %-softmmu,$(SUBDIR_RULES))
> +SUBDIR_RULES=$(addsuffix /all, $(TARGET_DIRS))
> +SOFTMMU_SUBDIR_RULES=$(filter %-softmmu/all,$(SUBDIR_RULES))
>  
>  $(SOFTMMU_SUBDIR_RULES): $(authz-obj-y)
>  $(SOFTMMU_SUBDIR_RULES): $(block-obj-y)
> @@ -447,14 +447,16 @@ $(SOFTMMU_SUBDIR_RULES): $(io-obj-y)
>  $(SOFTMMU_SUBDIR_RULES): config-all-devices.mak
>  $(SOFTMMU_SUBDIR_RULES): $(edk2-decompressed)
>  
> -subdir-%:
> - $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $* V="$(V)" 
> TARGET_DIR="$*/" all,)
> +.PHONY: $(SUBDIR_RULES)
> +$(SUBDIR_RULES):
> + $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $(dir $@) V="$(V)" 
> TARGET_DIR="$(dir $@)" all,)
>  
>  DTC_MAKE_ARGS=-I$(SRC_PATH)/dtc VPATH=$(SRC_PATH)/dtc -C dtc V="$(V)" 
> LIBFDT_srcdir=$(SRC_PATH)/dtc/libfdt
>  DTC_CFLAGS=$(CFLAGS) $(QEMU_CFLAGS)
>  DTC_CPPFLAGS=-I$(BUILD_DIR)/dtc -I$(SRC_PATH)/dtc -I$(SRC_PATH)/dtc/libfdt
>  
> -subdir-dtc: .git-submodule-status dtc/libfdt dtc/tests
> +.PHONY: dtc/all
> +dtc/all: .git-submodule-status dtc/libfdt dtc/tests
>   $(call quiet-command,$(MAKE) $(DTC_MAKE_ARGS) 
> CPPFLAGS="$(DTC_CPPFLAGS)" CFLAGS="$(DTC_CFLAGS)" LDFLAGS="$(LDFLAGS)" 
> ARFLAGS="$(ARFLAGS)" CC="$(CC)" AR="$(AR)" LD="$(LD)" $(SUBDIR_MAKEFLAGS) 
> libfdt/libfdt.a,)

OK

>  
>  dtc/%: .git-submodule-status
> @@ -472,21 +474,25 @@ CAP_CFLAGS += -DCAPSTONE_HAS_ARM64
>  CAP_CFLAGS += -DCAPSTONE_HAS_POWERPC
>  CAP_CFLAGS += -DCAPSTONE_HAS_X86
>  
> -subdir-capstone: .git-submodule-status
> +.PHONY: capstone/all
> +capstone/all: .git-submodule-status
>   $(call quiet-command,$(MAKE) -C $(SRC_PATH)/capstone CAPSTONE_SHARED=no 
> BUILDDIR="$(BUILD_DIR)/capstone" CC="$(CC)" AR="$(AR)" LD="$(LD)" 
> RANLIB="$(RANLIB)" CFLAGS="$(CAP_CFLAGS)" $(SUBDIR_MAKEFLAGS) 
> $(BUILD_DIR)/capstone/$(LIBCAPSTONE))

OK

>  
> -subdir-slirp: .git-submodule-status
> +.PHONY: slirp/all
> +slirp/all: .git-submodule-status
>   $(call quiet-command,$(MAKE) -C $(SRC_PATH)/slirp 
> BUILD_DIR="$(BUILD_DIR)/slirp" CC="$(CC)" AR="$(AR)" LD="$(LD)" 
> RANLIB="$(RANLIB)" CFLAGS="$(QEMU_CFLAGS) $(CFLAGS)" LDFLAGS="$(LDFLAGS)")

OK

>  
>  $(SUBDIR_RULES): libqemuutil.a $(common-obj-y) \
>   $(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY))
>  
> -ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS))
> +ROM_DIRS = $(addprefix pc-bios/, $(ROMS))
> +ROMSUBDIR_RULES=$(addsuffix /all, $(ROM_DIRS))
>  # Only keep -O and -g cflags
> -romsubdir-%:
> - $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C pc-bios/$* V="$(V)" 
> TARGET_DIR="$*/" CFLAGS="$(filter -O% -g%,$(CFLAGS))",)
> +.PHONY: $(ROMSUBDIR_RULES)
> +$(ROMSUBDIR_RULES):
> + $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $(dir $@) V="$(V)" 
> TARGET_DIR="$(dir $@)" CFLAGS="$(filter -O% -g%,$(CFLAGS))",)

OK

>  
> -ALL_SUBDIRS=$(TARGET_DIRS) $(patsubst %,pc-bios/%, $(ROMS))
> +ALL_SUBDIRS=$(TARGET_DIRS) $(ROM_DIRS)
>  
>  recurse-all: $(SUBDIR_RULES) $(ROMSUBDIR_RULES)
>  
> @@ -1087,7 +1

Re: [Qemu-devel] [PATCH] hw/rdma: Add support for GID state changes for non-qmp frameworks

[Marcel Apfelbaum]

Hi Yuval,

On 5/5/19 1:55 PM, Yuval Shaia wrote:

Any GID change in guest must be propogate to host. This is already done
by firing QMP event to managment system such as libvirt which in turn
will update the host with the relevant change.


Agreed, *any* management software can do that.



When qemu is executed on non-qmp framework (ex from command-line) we
need to update the host instead.
Fix it by adding support to update the RoCE device's Ethernet function
IP list from qemu via netlink.


I am not sure this is the right approach. I don't think QEMU should 
actively change

the host network configuration.
As you pointed out yourself, the management software should make such 
changes.


I agree you cannot always assume the QEMU instance is managed by libvirt,
what about adding this functionality to rdma-multiplexer? The 
multiplexer may

register to the same QMP event.

Even if you think the multiplexer is not the right way to do it, even a 
simple bash script

disguised  as a systemd service can subscribe to the QMP event and make the
change on the host.

What do you think?

Thanks,
Marcel



Signed-off-by: Yuval Shaia 
---
  configure  |  6 
  hw/rdma/rdma_backend.c | 74 +-
  2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 5b183c2e39..1f707b1a62 100755
--- a/configure
+++ b/configure
@@ -3132,6 +3132,8 @@ fi
  
  cat > $TMPC <
  #include 
+#include 
+#include 
  
  int

  main(void)
@@ -3144,10 +3146,13 @@ main(void)
  }
  EOF
  
+pvrdma_libs="-lmnl"

+
  if test "$rdma" = "yes" ; then
  case "$pvrdma" in
  "")
  if compile_prog "" ""; then
+libs_softmmu="$libs_softmmu $pvrdma_libs"
  pvrdma="yes"
  else
  pvrdma="no"
@@ -3156,6 +3161,7 @@ if test "$rdma" = "yes" ; then
  "yes")
  if ! compile_prog "" ""; then
  error_exit "PVRDMA is not supported since mremap is not 
implemented"
+" or libmnl-devel is not installed"
  fi
  pvrdma="yes"
  ;;
diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index 05f6b03221..bc57b1a624 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -16,6 +16,11 @@
  #include "qemu/osdep.h"
  #include "qapi/qapi-events-rdma.h"
  
+#include "linux/if_addr.h"

+#include "libmnl/libmnl.h"
+#include "linux/rtnetlink.h"
+#include "net/if.h"
+
  #include 
  
  #include "contrib/rdmacm-mux/rdmacm-mux.h"

@@ -47,6 +52,61 @@ static void dummy_comp_handler(void *ctx, struct ibv_wc *wc)
  rdma_error_report("No completion handler is registered");
  }
  
+static int netlink_route_update(const char *ifname, union ibv_gid *gid,

+__u16 type)
+{
+char buf[MNL_SOCKET_BUFFER_SIZE];
+struct nlmsghdr *nlh;
+struct ifaddrmsg *ifm;
+struct mnl_socket *nl;
+int ret;
+uint32_t ipv4;
+
+nl = mnl_socket_open(NETLINK_ROUTE);
+if (!nl) {
+rdma_error_report("Fail to connect to netlink\n");
+return -EIO;
+}
+
+ret = mnl_socket_bind(nl, 0, MNL_SOCKET_AUTOPID);
+if (ret < 0) {
+rdma_error_report("Fail to bind to netlink\n");
+goto out;
+}
+
+nlh = mnl_nlmsg_put_header(buf);
+nlh->nlmsg_type = type;
+nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL;
+nlh->nlmsg_seq = 1;
+
+ifm = mnl_nlmsg_put_extra_header(nlh, sizeof(*ifm));
+ifm->ifa_index = if_nametoindex(ifname);
+if (gid->global.subnet_prefix) {
+ifm->ifa_family = AF_INET6;
+ifm->ifa_prefixlen = 64;
+ifm->ifa_flags = IFA_F_PERMANENT;
+ifm->ifa_scope = RT_SCOPE_UNIVERSE;
+mnl_attr_put(nlh, IFA_ADDRESS, sizeof(*gid), gid);
+} else {
+ifm->ifa_family = AF_INET;
+ifm->ifa_prefixlen = 24;
+memcpy(&ipv4, (char *)&gid->global.interface_id + 4, sizeof(ipv4));
+mnl_attr_put(nlh, IFA_LOCAL, 4, &ipv4);
+}
+
+ret = mnl_socket_sendto(nl, nlh, nlh->nlmsg_len);
+if (ret < 0) {
+rdma_error_report("Fail to send msg to to netlink\n");
+goto out;
+}
+
+ret = 0;
+
+out:
+mnl_socket_close(nl);
+return ret;
+}
+
  static inline void complete_work(enum ibv_wc_status status, uint32_t 
vendor_err,
   void *ctx)
  {
@@ -1123,7 +1183,13 @@ int rdma_backend_add_gid(RdmaBackendDev *backend_dev, 
const char *ifname,
  gid->global.subnet_prefix,
  gid->global.interface_id);
  
-return ret;

+/*
+ * We ignore return value since operation might completed sucessfully
+ * by the QMP consumer
+ */
+netlink_route_update(ifname, gid, RTM_NEWADDR);
+
+return 0;
  }
  
  int rdma_backend_del_gid(RdmaBackendDev *backend_dev, const char *ifname,

@@ -1149,6 +1215,12 @@ int rdma_backend_del_gid(RdmaBackendDev *backend_dev, 
const c

Re: [Qemu-devel] [RFC v4 2/7] Makefile: Drop bogus cleaning of $(ALL_SUBDIRS)/qemu-options.def

[Philippe Mathieu-Daudé]

On 5/23/19 10:15 AM, Markus Armbruster wrote:
> When commit df2943ba3c7 moved "rm -f qemu-options.def" from distclean
> to clean, it also added "rm -f $$d/qemu-options.def" to the for d in
> $(ALL_SUBDIRS) loop.  That file doesn't exist.  Remove the mistaken
> rm.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  Makefile | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 8ec833a5fb..12e470fa03 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -647,7 +647,6 @@ clean:
>   rm -rf qga/qapi-generated
>   for d in $(ALL_SUBDIRS); do \
>   if test -d $$d; then $(MAKE) -C $$d $@ || exit 1; fi; \
> - rm -f $$d/qemu-options.def; \
>  done
>   rm -f config-all-devices.mak
>  
> 

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [RFC v4 1/7] Makefile: Remove code to smooth transition to config.status

[Philippe Mathieu-Daudé]

Hi Markus,

On 5/23/19 10:15 AM, Markus Armbruster wrote:
> When commit bdf523e6923 made configure generate config.status, it
> added a fallback to Makefile to smooth the transition, with a TODO
> "code can be removed after QEMU 1.7."  It's been more than five years.
> Remove it.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  Makefile | 9 +
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 155f066a20..8ec833a5fb 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -69,14 +69,7 @@ CONFIG_ALL=y
>  
>  config-host.mak: $(SRC_PATH)/configure $(SRC_PATH)/pc-bios 
> $(SRC_PATH)/VERSION
>   @echo $@ is out-of-date, running configure
> - @# TODO: The next lines include code which supports a smooth
> - @# transition from old configurations without config.status.
> - @# This code can be removed after QEMU 1.7.
> - @if test -x config.status; then \
> - ./config.status; \
> -else \
> - sed -n "/.*Configured with/s/[^:]*: //p" $@ | sh; \
> - fi
> + ./config.status

We could prepend a '@' since we already describe what this rule does
("config-host.mak is out-of-date, running configure").

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 

>  else
>  config-host.mak:
>  ifneq ($(filter-out $(UNCHECKED_GOALS),$(MAKECMDGOALS)),$(if 
> $(MAKECMDGOALS),,fail))
> 

Re: [Qemu-devel] [PATCH 0/4] tests/docker: add podman support

[Gerd Hoffmann]

On Fri, May 24, 2019 at 01:40:07AM +0200, Marc-André Lureau wrote:
> Hi,
> 
> podman allows to run containers in a similar fashion as docker, but
> without daemon or root privileges. Thank you podman!

Well, I saw a rather dramatic increase on disk usage when running podman
rootless.  Looked like podman did a full container image copy for each
docker file step instead of properly stacking incremental changes.
Didn't investigate why.

Therefore my "docker" looks like this:

   kraxel@sirius ~# cat bin/docker
   #!/bin/sh
   proxy="https_proxy,http_proxy,ftp_proxy,no_proxy"
   exec /usr/bin/sudo --preserve-env="${proxy}" /usr/bin/podman "$@"

So, yes, podman can run our docker tests just fine, but the rootless
mode has some hickups still.

cheers,
  Gerd

Re: [Qemu-devel] [PULL 00/38] ppc-for-4.1 queue 20190522

[David Gibson]

On Fri, May 24, 2019 at 10:46:16AM +1000, David Gibson wrote:
> On Thu, May 23, 2019 at 10:29:57AM +0100, Peter Maydell wrote:
> > On Wed, 22 May 2019 at 05:46, David Gibson  
> > wrote:
> > >
> > > The following changes since commit 
> > > a4f667b6714916683408b983cfe0a615a725775f:
> > >
> > >   Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20190521-3' 
> > > into staging (2019-05-21 16:30:13 +0100)
> > >
> > > are available in the Git repository at:
> > >
> > >   git://github.com/dgibson/qemu.git tags/ppc-for-4.1-20190522
> > >
> > > for you to fetch changes up to 885659103ad9e1b0460e89c548e9fb823b007b7e:
> > >
> > >   docs: provide documentation on the POWER9 XIVE interrupt controller 
> > > (2019-05-22 10:38:22 +1000)
> > >
> > > 
> > > ppc patch queue 2019-05-22
> > >
> > > Next pull request against qemu-4.1.  Highlights:
> > >   * KVM accelerated support for the XIVE interrupt controller in PAPR
> > > guests
> > >   * A number of TCG vector fixes
> > >   * Fixes for the PReP / 40p machine
> > >
> > > Other than that it's just a bunch of assorted fixes, cleanups and
> > > minor improvements.
> > >
> > > This supersedes the pull request dated 2019-05-21.  I've dropped the
> > > headers update, since it had a mistake, but is now made redundant by a
> > > similar update in Cornelia's applied PR.  I've also added a patch with
> > > extra documentation for the XIVE interrupt controller, and fixed a
> > > couple of minor style errors in the previous PR.
> > >
> > > 
> > 
> > Hi -- this failed 'make check-tcg' for the linux-static build
> > (x86-64 host):
> > 
> >   TESTlinux-test on ppc64abi32
> > qemu-ppc64abi32:
> > /home/petmay01/linaro/qemu-for-merges/accel/tcg/cpu-exec.c:700:
> > cpu_exec: Assertion `!have_mmap_lock()' failed.
> > qemu-ppc64abi32:
> > /home/petmay01/linaro/qemu-for-merges/accel/tcg/cpu-exec.c:700:
> > cpu_exec: Assertion `!have_mmap_lock()' failed.
> > /home/petmay01/linaro/qemu-for-merges/tests/tcg/Makefile:122: recipe
> > for target 'run-linux-test' failed
> 
> Bother.  I did run a make check-tcg on an x86_64 host, and didn't see
> this.  Investigating...

  BUILD   TCG tests for ppc64abi32-linux-user
  BUILD   ppc64abi32 guest-tests SKIPPED
  RUN TCG tests for ppc64abi32-linux-user
  RUN tests for ppc64abi32 SKIPPED

Well, that explains why I didn't see it.  I'm using the docker setup
for check-tcg, but it's not clear to me what bits I need to run the
ppc64abi32 tests.

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


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH] BootLinuxConsoleTest: Test the RX-Virt machine

[Philippe Mathieu-Daudé]

Add two tests for the rx-virt machine, based on the recommended test
setup from Yoshinori Sato:
https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03586.html

- U-Boot prompt
- Linux kernel with Sash shell

These are very quick tests:

  $ avocado run -t arch:rx tests/acceptance/boot_linux_console.py
  JOB ID : 84a6ef01c0b87975ecbfcb31a920afd735753ace
  JOB LOG: 
/home/phil/avocado/job-results/job-2019-05-24T05.02-84a6ef0/job.log
   (1/2) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_rx_uboot: 
PASS (0.11 s)
   (2/2) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_rx_linux: 
PASS (0.45 s)
  RESULTS: PASS 2 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | 
CANCEL 0

Tests can also be run with:

  $ avocado --show=console run -t arch:rx tests/acceptance/boot_linux_console.py
  console: U-Boot 2016.05-rc3-23705-ga1ef3c71cb-dirty (Feb 05 2019 - 21:56:06 
+0900)
  console: Linux version 4.19.0+ (yo-satoh@yo-satoh-debian) (gcc version 9.0.0 
20181105 (experimental) (GCC)) #137 Wed Feb 20 23:20:02 JST 2019
  console: Built 1 zonelists, mobility grouping on.  Total pages: 8128
  ...
  console: SuperH (H)SCI(F) driver initialized
  console: 88240.serial: ttySC0 at MMIO 0x88240 (irq = 215, base_baud = 0) is a 
sci
  console: console [ttySC0] enabled
  console: 88248.serial: ttySC1 at MMIO 0x88248 (irq = 219, base_baud = 0) is a 
sci

Signed-off-by: Philippe Mathieu-Daudé 
---
Based-on: 20190517045136.3509-1-richard.hender...@linaro.org
"RX architecture support"
---
 tests/acceptance/boot_linux_console.py | 51 ++
 1 file changed, 51 insertions(+)

diff --git a/tests/acceptance/boot_linux_console.py 
b/tests/acceptance/boot_linux_console.py
index d5c500ea30..f68aab1df8 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -45,6 +45,11 @@ class BootLinuxConsole(Test):
 fail = 'Failure message found in console: %s' % failure_message
 self.fail(fail)
 
+def exec_command_and_wait_for_pattern(self, command, success_message):
+command += '\n'
+self.vm.console_socket.sendall(command.encode())
+self.wait_for_console_pattern(success_message)
+
 def extract_from_deb(self, deb, path):
 """
 Extracts a file from a deb package into the test workdir
@@ -217,3 +222,49 @@ class BootLinuxConsole(Test):
 self.vm.launch()
 console_pattern = 'Kernel command line: %s' % kernel_command_line
 self.wait_for_console_pattern(console_pattern)
+
+def test_rx_uboot(self):
+"""
+:avocado: tags=arch:rx
+:avocado: tags=machine:rx-virt
+:avocado: tags=endian:little
+"""
+uboot_url = ('https://acc.dl.osdn.jp/users/23/23888/u-boot.bin.gz')
+uboot_hash = '9b78dbd43b40b2526848c0b1ce9de02c24f4dcdb'
+uboot_path = self.fetch_asset(uboot_url, asset_hash=uboot_hash)
+uboot_path = archive.uncompress(uboot_path, self.workdir)
+
+self.vm.set_machine('rx-virt')
+self.vm.set_console()
+self.vm.add_args('-bios', uboot_path,
+ '-no-reboot')
+self.vm.launch()
+uboot_version = 'U-Boot 2016.05-rc3-23705-ga1ef3c71cb-dirty'
+self.wait_for_console_pattern(uboot_version)
+gcc_version = 'rx-unknown-linux-gcc (GCC) 9.0.0 20181105 
(experimental)'
+# FIXME limit baudrate on chardev, else we type too fast
+#self.exec_command_and_wait_for_pattern('version', gcc_version)
+
+def test_rx_linux(self):
+"""
+:avocado: tags=arch:rx
+:avocado: tags=machine:rx-virt
+:avocado: tags=endian:little
+"""
+dtb_url = ('https://acc.dl.osdn.jp/users/23/23887/rx-qemu.dtb')
+dtb_hash = '7b4e4e2c71905da44e86ce47adee2210b026ac18'
+dtb_path = self.fetch_asset(dtb_url, asset_hash=dtb_hash)
+kernel_url = ('http://acc.dl.osdn.jp/users/23/23845/zImage')
+kernel_hash = '39a81067f8d72faad90866ddfefa19165d68fc99'
+kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
+
+self.vm.set_machine('rx-virt')
+self.vm.set_console()
+kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'earlycon'
+self.vm.add_args('-kernel', kernel_path,
+ '-dtb', dtb_path,
+ '-no-reboot')
+self.vm.launch()
+self.wait_for_console_pattern('Sash command shell (version 1.1.1)')
+self.exec_command_and_wait_for_pattern('printenv',
+   'TERM=linux')
-- 
2.19.1

Re: [Qemu-devel] [PULL 07/13] hw/char: RX62N serial communication interface (SCI)

[Philippe Mathieu-Daudé]

Hi Paolo, Marc-André,

On 5/23/19 4:00 PM, Richard Henderson wrote:
> From: Yoshinori Sato 
> 
> This module supported only non FIFO type.
> Hardware manual.
> https://www.renesas.com/us/en/doc/products/mpumcu/doc/rx_family/r01uh0033ej0140_rx62n.pdf
> 
> Signed-off-by: Yoshinori Sato 
> Reviewed-by: Alex Bennée 
> Reviewed-by: Philippe Mathieu-Daudé 
> Message-Id: <20190516055244.95559-8-ys...@users.sourceforge.jp>
> Signed-off-by: Richard Henderson 
> ---
>  include/hw/char/renesas_sci.h |  45 +
>  hw/char/renesas_sci.c | 340 ++
>  hw/char/Kconfig   |   3 +
>  hw/char/Makefile.objs |   1 +
>  4 files changed, 389 insertions(+)
>  create mode 100644 include/hw/char/renesas_sci.h
>  create mode 100644 hw/char/renesas_sci.c
> 
> diff --git a/include/hw/char/renesas_sci.h b/include/hw/char/renesas_sci.h
> new file mode 100644
> index 00..50d1336944
> --- /dev/null
> +++ b/include/hw/char/renesas_sci.h
> @@ -0,0 +1,45 @@
> +/*
> + * Renesas Serial Communication Interface
> + *
> + * Copyright (c) 2018 Yoshinori Sato
> + *
> + * This code is licensed under the GPL version 2 or later.
> + *
> + */
> +
> +#include "chardev/char-fe.h"
> +#include "qemu/timer.h"
> +#include "hw/sysbus.h"
> +
> +#define TYPE_RENESAS_SCI "renesas-sci"
> +#define RSCI(obj) OBJECT_CHECK(RSCIState, (obj), TYPE_RENESAS_SCI)
> +
> +enum {
> +ERI = 0,
> +RXI = 1,
> +TXI = 2,
> +TEI = 3,
> +SCI_NR_IRQ = 4,
> +};
> +
> +typedef struct {
> +SysBusDevice parent_obj;
> +MemoryRegion memory;
> +
> +uint8_t smr;
> +uint8_t brr;
> +uint8_t scr;
> +uint8_t tdr;
> +uint8_t ssr;
> +uint8_t rdr;
> +uint8_t scmr;
> +uint8_t semr;
> +
> +uint8_t read_ssr;
> +int64_t trtime;
> +int64_t rx_next;
> +QEMUTimer *timer;
> +CharBackend chr;
> +uint64_t input_freq;
> +qemu_irq irq[SCI_NR_IRQ];
> +} RSCIState;
> diff --git a/hw/char/renesas_sci.c b/hw/char/renesas_sci.c
> new file mode 100644
> index 00..6298cbf43a
> --- /dev/null
> +++ b/hw/char/renesas_sci.c
> @@ -0,0 +1,340 @@
> +/*
> + * Renesas Serial Communication Interface
> + *
> + * Datasheet: RX62N Group, RX621 Group User's Manual: Hardware
> + * (Rev.1.40 R01UH0033EJ0140)
> + *
> + * Copyright (c) 2019 Yoshinori Sato
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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 "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "qapi/error.h"
> +#include "qemu-common.h"
> +#include "cpu.h"
> +#include "hw/hw.h"
> +#include "hw/sysbus.h"
> +#include "hw/registerfields.h"
> +#include "hw/char/renesas_sci.h"
> +#include "qemu/error-report.h"
> +
> +/* SCI register map */
> +REG8(SMR, 0)
> +  FIELD(SMR, CKS,  0, 2)
> +  FIELD(SMR, MP,   2, 1)
> +  FIELD(SMR, STOP, 3, 1)
> +  FIELD(SMR, PM,   4, 1)
> +  FIELD(SMR, PE,   5, 1)
> +  FIELD(SMR, CHR,  6, 1)
> +  FIELD(SMR, CM,   7, 1)
> +REG8(BRR, 1)
> +REG8(SCR, 2)
> +  FIELD(SCR, CKE, 0, 2)
> +  FIELD(SCR, TEIE, 2, 1)
> +  FIELD(SCR, MPIE, 3, 1)
> +  FIELD(SCR, RE,   4, 1)
> +  FIELD(SCR, TE,   5, 1)
> +  FIELD(SCR, RIE,  6, 1)
> +  FIELD(SCR, TIE,  7, 1)
> +REG8(TDR, 3)
> +REG8(SSR, 4)
> +  FIELD(SSR, MPBT, 0, 1)
> +  FIELD(SSR, MPB,  1, 1)
> +  FIELD(SSR, TEND, 2, 1)
> +  FIELD(SSR, ERR, 3, 3)
> +FIELD(SSR, PER,  3, 1)
> +FIELD(SSR, FER,  4, 1)
> +FIELD(SSR, ORER, 5, 1)
> +  FIELD(SSR, RDRF, 6, 1)
> +  FIELD(SSR, TDRE, 7, 1)
> +REG8(RDR, 5)
> +REG8(SCMR, 6)
> +  FIELD(SCMR, SMIF, 0, 1)
> +  FIELD(SCMR, SINV, 2, 1)
> +  FIELD(SCMR, SDIR, 3, 1)
> +  FIELD(SCMR, BCP2, 7, 1)
> +REG8(SEMR, 7)
> +  FIELD(SEMR, ACS0, 0, 1)
> +  FIELD(SEMR, ABCS, 4, 1)
> +
> +static int can_receive(void *opaque)
> +{
> +RSCIState *sci = RSCI(opaque);
> +if (sci->rx_next > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)) {
> +return 0;
> +} else {
> +return FIELD_EX8(sci->scr, SCR, RE);
> +}

I previously tested this device manually, now I'm trying to add a test
and I see the serial dropping characters. Is it possible to set a
baudrate limit on a chardev?

* manual use:

=> version
U-Boot 2016.05-rc3-23705-ga1ef3c71cb-dirty (Feb 05 2019 - 21:56:06+0900)
rx-unknown-linux-gcc (GCC) 9.0.0 20181105 (experimental)
GNU ld (GNU Binutils) 2.31.51.20180703

* script use (echo 'version' | qemu):

=> vion
Unknown command 'vion' - try 'help'

Thanks!

Phil.

> +}
> +
> +static void receive(void *opaqu

[Qemu-devel] Patch causing qemu-system-ppc to crash

[Programmingkid]

Recently I have noticed that qemu-system-ppc is crashing while booting up my 
Mac OS X VM. A bit of git bisecting shows this is the patch that causes this 
issue:

commit 1e262b49b5331441f697461e4305fe06719758a7
Author: Richard Henderson 
Date:   Mon Mar 18 12:02:54 2019 -0700

tcg/i386: Implement tcg_out_dupm_vec

At the same time, improve tcg_out_dupi_vec wrt broadcast
from the constant pool.

Signed-off-by: Richard Henderson 

Re: [Qemu-devel] [PULL 05/15] hw/ppc: Implement fw_cfg_arch_key_name()

[David Gibson]

On Thu, May 23, 2019 at 02:43:10PM +0200, Philippe Mathieu-Daudé wrote:
> Implement fw_cfg_arch_key_name(), which returns the name of a
> ppc-specific key.
> 
> The fw_cfg device is used by the machine using OpenBIOS:
> - 40p
> - mac99 (oldworld)
> - g3beige (newworld)
> 
> Reviewed-by: Laszlo Ersek 
> Message-Id: <20190422195020.1494-6-phi...@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé 

Acked-by: David Gibson 

> ---
>  hw/ppc/Makefile.objs |  2 +-
>  hw/ppc/fw_cfg.c  | 45 
>  2 files changed, 46 insertions(+), 1 deletion(-)
>  create mode 100644 hw/ppc/fw_cfg.c
> 
> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> index 636e717f20..9da93af905 100644
> --- a/hw/ppc/Makefile.objs
> +++ b/hw/ppc/Makefile.objs
> @@ -1,5 +1,5 @@
>  # shared objects
> -obj-y += ppc.o ppc_booke.o fdt.o
> +obj-y += ppc.o ppc_booke.o fdt.o fw_cfg.o
>  # IBM pSeries (sPAPR)
>  obj-$(CONFIG_PSERIES) += spapr.o spapr_caps.o spapr_vio.o spapr_events.o
>  obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
> diff --git a/hw/ppc/fw_cfg.c b/hw/ppc/fw_cfg.c
> new file mode 100644
> index 00..a88b5c4bde
> --- /dev/null
> +++ b/hw/ppc/fw_cfg.c
> @@ -0,0 +1,45 @@
> +/*
> + * fw_cfg helpers (PPC specific)
> + *
> + * Copyright (c) 2019 Red Hat, Inc.
> + *
> + * Author:
> + *   Philippe Mathieu-Daudé 
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/ppc/ppc.h"
> +#include "hw/nvram/fw_cfg.h"
> +
> +const char *fw_cfg_arch_key_name(uint16_t key)
> +{
> +static const struct {
> +uint16_t key;
> +const char *name;
> +} fw_cfg_arch_wellknown_keys[] = {
> +{FW_CFG_PPC_WIDTH, "width"},
> +{FW_CFG_PPC_HEIGHT, "height"},
> +{FW_CFG_PPC_DEPTH, "depth"},
> +{FW_CFG_PPC_TBFREQ, "tbfreq"},
> +{FW_CFG_PPC_CLOCKFREQ, "clockfreq"},
> +{FW_CFG_PPC_IS_KVM, "is_kvm"},
> +{FW_CFG_PPC_KVM_HC, "kvm_hc"},
> +{FW_CFG_PPC_KVM_PID, "pid"},
> +{FW_CFG_PPC_NVRAM_ADDR, "nvram_addr"},
> +{FW_CFG_PPC_BUSFREQ, "busfreq"},
> +{FW_CFG_PPC_NVRAM_FLAT, "nvram_flat"},
> +{FW_CFG_PPC_VIACONFIG, "viaconfig"},
> +};
> +
> +for (size_t i = 0; i < ARRAY_SIZE(fw_cfg_arch_wellknown_keys); i++) {
> +if (fw_cfg_arch_wellknown_keys[i].key == key) {
> +return fw_cfg_arch_wellknown_keys[i].name;
> +}
> +}
> +return NULL;
> +}

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PULL 00/38] ppc-for-4.1 queue 20190522

[David Gibson]

On Thu, May 23, 2019 at 10:29:57AM +0100, Peter Maydell wrote:
> On Wed, 22 May 2019 at 05:46, David Gibson  
> wrote:
> >
> > The following changes since commit a4f667b6714916683408b983cfe0a615a725775f:
> >
> >   Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20190521-3' into 
> > staging (2019-05-21 16:30:13 +0100)
> >
> > are available in the Git repository at:
> >
> >   git://github.com/dgibson/qemu.git tags/ppc-for-4.1-20190522
> >
> > for you to fetch changes up to 885659103ad9e1b0460e89c548e9fb823b007b7e:
> >
> >   docs: provide documentation on the POWER9 XIVE interrupt controller 
> > (2019-05-22 10:38:22 +1000)
> >
> > 
> > ppc patch queue 2019-05-22
> >
> > Next pull request against qemu-4.1.  Highlights:
> >   * KVM accelerated support for the XIVE interrupt controller in PAPR
> > guests
> >   * A number of TCG vector fixes
> >   * Fixes for the PReP / 40p machine
> >
> > Other than that it's just a bunch of assorted fixes, cleanups and
> > minor improvements.
> >
> > This supersedes the pull request dated 2019-05-21.  I've dropped the
> > headers update, since it had a mistake, but is now made redundant by a
> > similar update in Cornelia's applied PR.  I've also added a patch with
> > extra documentation for the XIVE interrupt controller, and fixed a
> > couple of minor style errors in the previous PR.
> >
> > 
> 
> Hi -- this failed 'make check-tcg' for the linux-static build
> (x86-64 host):
> 
>   TESTlinux-test on ppc64abi32
> qemu-ppc64abi32:
> /home/petmay01/linaro/qemu-for-merges/accel/tcg/cpu-exec.c:700:
> cpu_exec: Assertion `!have_mmap_lock()' failed.
> qemu-ppc64abi32:
> /home/petmay01/linaro/qemu-for-merges/accel/tcg/cpu-exec.c:700:
> cpu_exec: Assertion `!have_mmap_lock()' failed.
> /home/petmay01/linaro/qemu-for-merges/tests/tcg/Makefile:122: recipe
> for target 'run-linux-test' failed

Bother.  I did run a make check-tcg on an x86_64 host, and didn't see
this.  Investigating...

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


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH 4/4] qemu-sockets: do not require configured ipv4/ipv6 address

[Marc-André Lureau]

podman containers without network don't have ipv4/ipv6 addresses other
than loopback address. However, some of our tests require
getaddrinfo("127.0.0.1") to succeed.

Alternatively, we may want to treat 127.0.0.1 as a special case, to
keep the AI_ADDRCONFIG convenience.

Signed-off-by: Marc-André Lureau 
---
 util/qemu-sockets.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 8850a280a8..f9c1392a05 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -31,10 +31,6 @@
 #include "qapi/qobject-output-visitor.h"
 #include "qemu/cutils.h"
 
-#ifndef AI_ADDRCONFIG
-# define AI_ADDRCONFIG 0
-#endif
-
 #ifndef AI_V4MAPPED
 # define AI_V4MAPPED 0
 #endif
@@ -385,7 +381,7 @@ static struct addrinfo 
*inet_parse_connect_saddr(InetSocketAddress *saddr,
 
 memset(&ai, 0, sizeof(ai));
 
-ai.ai_flags = AI_CANONNAME | AI_ADDRCONFIG;
+ai.ai_flags = AI_CANONNAME;
 if (atomic_read(&useV4Mapped)) {
 ai.ai_flags |= AI_V4MAPPED;
 }
@@ -472,7 +468,7 @@ static int inet_dgram_saddr(InetSocketAddress *sraddr,
 
 /* lookup peer addr */
 memset(&ai,0, sizeof(ai));
-ai.ai_flags = AI_CANONNAME | AI_V4MAPPED | AI_ADDRCONFIG;
+ai.ai_flags = AI_CANONNAME | AI_V4MAPPED;
 ai.ai_family = inet_ai_family_from_address(sraddr, &err);
 ai.ai_socktype = SOCK_DGRAM;
 
-- 
2.22.0.rc1.1.g079e7d2849.dirty

[Qemu-devel] [PATCH 2/4] tests/docker: add podman support

[Marc-André Lureau]

Allow to specify the container engine to run with ENGINE variable.

By default, ENGINE=auto and will select either podman or docker.

With current podman, we have to use a uidmap trick in order to be able
to rw-share the ccache directory with the container user.

With a user 1000, the default mapping is:
1000 (host) -> 0 (container).

So write access to /var/tmp/ccache ends will end with permission
denied error.

With "--uidmap 1000:0:1 --uidmap 0:1:1000", the mapping is:
1000 (host) -> 0 (container, 1st namespace) -> 1000 (container, 2nd namespace).

(the rest is mumbo jumbo to avoid holes in the range of UIDs)

A future podman version may have an option such as --userns-keep-uid.
Thanks to Debarshi Ray for the help!

Cc: Debarshi Ray 
Signed-off-by: Marc-André Lureau 
---
 Makefile  |  2 +-
 tests/docker/Makefile.include | 17 ++---
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index e02b88bcb1..e3a150ac4c 100644
--- a/Makefile
+++ b/Makefile
@@ -1118,7 +1118,7 @@ endif
@echo  ''
@echo  'Test targets:'
@echo  '  check   - Run all tests (check-help for details)'
-   @echo  '  docker  - Help about targets running tests inside 
Docker containers'
+   @echo  '  docker  - Help about targets running tests inside 
containers'
@echo  '  vm-test - Help about targets running tests inside VM'
@echo  ''
@echo  'Documentation targets:'
diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index c0e1bf57a3..2bf679767e 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -17,7 +17,9 @@ DOCKER_TESTS := $(notdir $(shell \
 
 DOCKER_TOOLS := travis
 
-DOCKER_SCRIPT=$(SRC_PATH)/tests/docker/docker.py
+ENGINE := auto
+
+DOCKER_SCRIPT=$(SRC_PATH)/tests/docker/docker.py --engine $(ENGINE)
 
 TESTS ?= %
 IMAGES ?= %
@@ -145,7 +147,7 @@ $(foreach i,$(filter-out 
$(DOCKER_PARTIAL_IMAGES),$(DOCKER_IMAGES) $(DOCKER_DEPR
 )
 
 docker:
-   @echo 'Build QEMU and run tests inside Docker containers'
+   @echo 'Build QEMU and run tests inside Docker or Podman containers'
@echo
@echo 'Available targets:'
@echo
@@ -192,6 +194,14 @@ endif
@echo 'EXECUTABLE=Include executable in image.'
@echo 'EXTRA_FILES=" [... ]"'
@echo ' Include extra files in image.'
+   @echo 'ENGINE=auto/docker/podman'
+   @echo ' Specify which container engine to run.'
+
+UID=$(shell id -u)
+UID1=$(shell expr $(UID) + 1)
+ifeq ($(shell $(DOCKER_SCRIPT) probe),podman)
+PODMAN=1
+endif
 
 # This rule if for directly running against an arbitrary docker target.
 # It is called by the expanded docker targets (e.g. make
@@ -211,7 +221,8 @@ docker-run: docker-qemu-src
"  COPYING $(EXECUTABLE) to $(IMAGE)"))
$(call quiet-command,   \
$(DOCKER_SCRIPT) run\
-   $(if $(NOUSER),,-u $(shell id -u))  \
+   $(if $(NOUSER),,-u $(UID)   \
+   $(if $(PODMAN),--uidmap $(UID):0:1 --uidmap 
0:1:$(UID) --uidmap $(UID1):$(UID1):64536)) \
--security-opt seccomp=unconfined   \
$(if $V,,--rm)  \
$(if $(DEBUG),-ti,) \
-- 
2.22.0.rc1.1.g079e7d2849.dirty

[Qemu-devel] [PATCH 3/4] docker: update fedora to f30

[Marc-André Lureau]

Released last month.

Signed-off-by: Marc-André Lureau 
---
 tests/docker/dockerfiles/fedora.docker | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/docker/dockerfiles/fedora.docker 
b/tests/docker/dockerfiles/fedora.docker
index 69d4a7f5d7..1496b68ba1 100644
--- a/tests/docker/dockerfiles/fedora.docker
+++ b/tests/docker/dockerfiles/fedora.docker
@@ -1,4 +1,4 @@
-FROM fedora:29
+FROM fedora:30
 ENV PACKAGES \
 bc \
 bison \
-- 
2.22.0.rc1.1.g079e7d2849.dirty

[Qemu-devel] [PATCH 0/4] tests/docker: add podman support

[Marc-André Lureau]

Hi,

podman allows to run containers in a similar fashion as docker, but
without daemon or root privileges. Thank you podman!

I haven't done extensive testing. Basic make docker-test rules
work. There seems to be a few issues with permissions at run time
(podman ps fails), but that seems not directly related to this series.

There was also a small issue running make check, due to not having
network address at all by default. See "qemu-sockets: do not require
configured ipv4/ipv6 address" patch for the proposed solution.

Marc-André Lureau (4):
  docker.py: add podman support
  tests/docker: add podman support
  docker: update fedora to f30
  qemu-sockets: do not require configured ipv4/ipv6 address

 util/qemu-sockets.c|  8 ++---
 Makefile   |  2 +-
 tests/docker/Makefile.include  | 17 --
 tests/docker/docker.py | 43 +++---
 tests/docker/dockerfiles/fedora.docker |  2 +-
 5 files changed, 56 insertions(+), 16 deletions(-)

-- 
2.22.0.rc1.1.g079e7d2849.dirty

[Qemu-devel] [PATCH 1/4] docker.py: add podman support

[Marc-André Lureau]

Add a --engine option to select either docker, podman or auto.

Among other advantages, podman allows to run rootless & daemonless
containers, fortunately sharing compatible CLI with docker.

Signed-off-by: Marc-André Lureau 
---
 tests/docker/docker.py | 43 +-
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 53a8c9c801..1f59a78b10 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -20,6 +20,7 @@ import hashlib
 import atexit
 import uuid
 import argparse
+import enum
 import tempfile
 import re
 import signal
@@ -38,6 +39,26 @@ FILTERED_ENV_NAMES = ['ftp_proxy', 'http_proxy', 
'https_proxy']
 
 DEVNULL = open(os.devnull, 'wb')
 
+class EngineEnum(enum.IntEnum):
+AUTO = 1
+DOCKER = 2
+PODMAN = 3
+
+def __str__(self):
+return self.name.lower()
+
+def __repr__(self):
+return str(self)
+
+@staticmethod
+def argparse(s):
+try:
+return EngineEnum[s.upper()]
+except KeyError:
+return s
+
+
+USE_ENGINE = EngineEnum.AUTO
 
 def _text_checksum(text):
 """Calculate a digest string unique to the text content"""
@@ -48,9 +69,14 @@ def _file_checksum(filename):
 return _text_checksum(open(filename, 'rb').read())
 
 
-def _guess_docker_command():
-""" Guess a working docker command or raise exception if not found"""
-commands = [["docker"], ["sudo", "-n", "docker"]]
+def _guess_engine_command():
+""" Guess a working engine command or raise exception if not found"""
+commands = []
+
+if USE_ENGINE in [EngineEnum.AUTO, EngineEnum.PODMAN]:
+commands += [["podman"]]
+if USE_ENGINE in [EngineEnum.AUTO, EngineEnum.DOCKER]:
+commands += [["docker"], ["sudo", "-n", "docker"]]
 for cmd in commands:
 try:
 # docker version will return the client details in stdout
@@ -61,7 +87,7 @@ def _guess_docker_command():
 except OSError:
 pass
 commands_txt = "\n".join(["  " + " ".join(x) for x in commands])
-raise Exception("Cannot find working docker command. Tried:\n%s" %
+raise Exception("Cannot find working engine command. Tried:\n%s" %
 commands_txt)
 
 
@@ -190,7 +216,7 @@ def _dockerfile_preprocess(df):
 class Docker(object):
 """ Running Docker commands """
 def __init__(self):
-self._command = _guess_docker_command()
+self._command = _guess_engine_command()
 self._instances = []
 atexit.register(self._kill_instances)
 signal.signal(signal.SIGTERM, self._kill_instances)
@@ -502,6 +528,8 @@ class ProbeCommand(SubCommand):
 print("yes")
 elif docker._command[0] == "sudo":
 print("sudo")
+elif docker._command[0] == "podman":
+print("podman")
 except Exception:
 print("no")
 
@@ -597,9 +625,13 @@ class CheckCommand(SubCommand):
 
 
 def main():
+global USE_ENGINE
+
 parser = argparse.ArgumentParser(description="A Docker helper",
  usage="%s  ..." %
  os.path.basename(sys.argv[0]))
+parser.add_argument("--engine", type=EngineEnum.argparse, 
choices=list(EngineEnum),
+help="specify which container engine to use")
 subparsers = parser.add_subparsers(title="subcommands", help=None)
 for cls in SubCommand.__subclasses__():
 cmd = cls()
@@ -608,6 +640,7 @@ def main():
 cmd.args(subp)
 subp.set_defaults(cmdobj=cmd)
 args, argv = parser.parse_known_args()
+USE_ENGINE = args.engine
 return args.cmdobj.run(args, argv)
 
 
-- 
2.22.0.rc1.1.g079e7d2849.dirty

Re: [Qemu-devel] [PATCH 0/4] mips: Add more Avocado tests

[Eduardo Habkost]

On Thu, May 23, 2019 at 09:28:00AM -0400, Cleber Rosa wrote:
> 
> 
> - Original Message -
> > From: "Philippe Mathieu-Daudé" 
> > To: "Eduardo Habkost" , "Cleber Rosa" 
> > 
> > Cc: "Aleksandar Rikalo" , "Philippe Mathieu-Daudé" 
> > , "Wainer dos Santos
> > Moschetta" , qemu-devel@nongnu.org, "Aleksandar 
> > Markovic" ,
> > "Aleksandar Markovic" , "Aurelien Jarno" 
> > 
> > Sent: Thursday, May 23, 2019 5:38:34 AM
> > Subject: Re: [Qemu-devel] [PATCH 0/4] mips: Add more Avocado tests
> > 
> > On 5/23/19 1:07 AM, Eduardo Habkost wrote:
> > > On Wed, May 22, 2019 at 05:46:06PM -0400, Cleber Rosa wrote:
> > >> - Original Message -
> > >>> From: "Eduardo Habkost" 
> > >>> On Tue, May 21, 2019 at 01:19:06AM +0200, Philippe Mathieu-Daudé wrote:
> >  Hi,
> > 
> >  It was a rainy week-end here, so I invested it to automatize some
> >  of my MIPS tests.
> > 
> >  The BootLinuxSshTest is not Global warming friendly, it is not
> >  meant to run on a CI system but rather on a workstation previous
> >  to post a pull request.
> >  It can surely be improved, but it is a good starting point.
> > >>>
> > >>> Until we actually have a mechanism to exclude the test case on
> > >>> travis-ci, I will remove patch 4/4 from the queue.  Aleksandar,
> > >>> please don't merge patch 4/4 yet or it will break travis-ci.
> > >>>
> > >>> Cleber, Wainer, is it already possible to make "avocado run" skip
> > >>> tests tagged with "slow"?
> > >>>
> > >>
> > >> The mechanism exists, but we haven't tagged any test so far as slow.
> > >>
> > >> Should we define/document a criteria for a test to be slow?  Given
> > >> that this is highly subjective, we have to think of:
> > >>
> > >>  * Will we consider the average or maximum run time (the timeout
> > >>definition)?
> > >>  
> > >>  * For a single test, what is "slow"? Some rough numbers from Travis
> > >>CI[1] to help us with guidelines:
> > >>- boot_linux_console.py:BootLinuxConsole.test_x86_64_pc:  PASS (6.04 
> > >> s)
> > >>- boot_linux_console.py:BootLinuxConsole.test_arm_virt:  PASS (2.91 s)
> > >>-
> > >>
> > >> linux_initrd.py:LinuxInitrd.test_with_2gib_file_should_work_with_linux_v4_16:
> > >>PASS (18.14 s)
> > >>- boot_linux.py:BootLinuxAarch64.test_virt:  PASS (396.88 s)
> > > 
> > > I don't think we need to overthink this.  Whatever objective
> > > criteria we choose, I'm sure we'll have to adapt them later due
> > > to real world problems.
> > > 
> > > e.g.: is 396 seconds too slow?  I don't know, it depends: does it
> > > break Travis and other CI systems often because of timeouts?  If
> > > yes, then we should probably tag it as slow.
> > > 
> > > If having subjective criteria is really a problem (I don't think
> > > it is), then we can call the tag "skip_travis", and stop worrying
> > > about defining what exactly is "slow".
> > 
> > I'd go with a simpler "tags:travis-ci" whitelisting any job expecting to
> > run smoothly there.
> > 
> 
> My concern is what becomes of "make check-acceptance".  Should we introduce
> another target, say, "make check-acceptance-ci" or just change its meaning
> and reuse it?

What about "make check-acceptance TAG=travis-ci"?

> 
> > Then we can add "slow" tests without having to worry about blacklisting
> > for Travis CI.
> > Also, Other CI can set different timeouts.
> > 
> > I'd like maintainers to add as many tests as they want to upstream, so
> > these tests can eventually run by anyone, then each maintainer is free
> > to select which particular set he wants to run as default.
> > 
> 
> OK, so this matches the idea of carefully curating a set of tests for
> CI.  WRT white or blacklisting, I favor the approach that requires the
> least effort from the developer to have its test enabled, so I'd go
> with blacklisting.  I fear that simple tests will just sit on the repo
> without being properly exercised if we need to whitelist them.
> 

I agree.  I'd prefer the default case to be simple and not
require extra tags.  (i.e. tests without any tags would be run in
Travis by default).

-- 
Eduardo

Re: [Qemu-devel] [PATCH] i386: Fix nested SVM on older Opterons

[Eduardo Habkost]

On Thu, May 23, 2019 at 07:57:38PM +0100, Dr. David Alan Gilbert wrote:
> * Bernhard M. Wiedemann (bwiedem...@suse.de) wrote:
> > Without this patch, a VM on a Opteron G3 host will have the svm flag, but
> > the kvm-amd module fails to load in there, complaining that it needs
> > cpuid 0x800a
> > 
> > I have successfully built and tested this for 3+ years in production
> > on Opteron G3 servers.

Have you reproduced the bug on QEMU 2.8 or newer?  The problem
you describe should be fixed by the following commit (from ~2.5
years ago).

commit 0c3d7c0051576d220e6da0a8ac08f2d8482e2f0b
Author: Eduardo Habkost 
Date:   Wed Sep 21 15:01:35 2016 -0300

target-i386: Enable CPUID[0x800A] if SVM is enabled

SVM needs CPUID[0x800A] to be available. So if SVM is enabled
in a CPU model or explicitly in the command-line, adjust CPUID
xlevel to expose the CPUID[0x800A] leaf.

Reviewed-by: Richard Henderson 
Signed-off-by: Eduardo Habkost 

-- 
Eduardo

Re: [Qemu-devel] [PULL v3 47/55] linux headers: update against Linux 5.2-rc1

[Aleksandar Markovic]

On May 22, 2019 3:50 PM, "Cornelia Huck"  wrote:
>
> On Wed, 22 May 2019 15:22:23 +0200
> Aleksandar Markovic  wrote:
>
> > On May 22, 2019 2:24 PM, "Cornelia Huck"  wrote:
> > >
> > > On Wed, 22 May 2019 14:10:39 +0200
> > > Laurent Vivier  wrote:
> > >
> > > > On 22/05/2019 14:07, Cornelia Huck wrote:
> > > > > On Wed, 22 May 2019 13:47:25 +0200
> > > > > Philippe Mathieu-Daudé  wrote:
> > > > >
> > > > >> On 5/21/19 5:28 PM, Cornelia Huck wrote:
> > > > >>> commit a188339ca5a396acc588e5851ed7e19f66b0ebd9
> > > > >>>
> > > > >>> Signed-off-by: Cornelia Huck 
> > > > >>> ---
> > > > >> [...]
> > > > >>>   #define __NR_mq_notify 184
> > > > >>>   __SC_COMP(__NR_mq_notify, sys_mq_notify, compat_sys_mq_notify)
> > > > >>>   #define __NR_mq_getsetattr 185
> > > > >>> @@ -536,8 +567,10 @@ __SC_COMP(__NR_msgsnd, sys_msgsnd,
> > compat_sys_msgsnd)
> > > > >>>   __SYSCALL(__NR_semget, sys_semget)
> > > > >>>   #define __NR_semctl 191
> > > > >>>   __SC_COMP(__NR_semctl, sys_semctl, compat_sys_semctl)
> > > > >>> +#if defined(__ARCH_WANT_TIME32_SYSCALLS) || __BITS_PER_LONG !=
32
> > > > >
> > > > > Eww. It seems only aarch64 sets __ARCH_WANT_TIME32_SYSCALLS, and
the
> > > > > second condition probably catches others but not mipsel.
> > > > >
> > > > >>>   #define __NR_semtimedop 192
> > > > >>> -__SC_COMP(__NR_semtimedop, sys_semtimedop,
compat_sys_semtimedop)
> > > > >>> +__SC_COMP(__NR_semtimedop, sys_semtimedop,
sys_semtimedop_time32)
> > > > >>> +#endif
> > > > >>>   #define __NR_semop 193
> > > > >>>   __SYSCALL(__NR_semop, sys_semop)
> > > > >> [...]
> > > > >>
> > > > >>
https://app.shippable.com/github/qemu/qemu/runs/1703/summary/console
> > > > >>
> > > > >> It seems this commit introduce a regression on mips32:
> > > > >>
> > > > >>CC  mipsel-linux-user/linux-user/syscall.o
> > > > >> ./linux-user/syscall.c: In function 'safe_semtimedop':
> > > > >> ./linux-user/syscall.c:697:25: error: '__NR_semtimedop'
undeclared
> > > > >> (first use in this function)
> > > > >>   return safe_syscall(__NR_##name, arg1, arg2, arg3, arg4);
\
> > > > >
> > > > > So, we unconditionally deal with this syscall, i.e. we assume it
is
> > > > > always present? (I'm not sure of the logic in linux-user code.)
> > > > >
> > > >
> > > > linux-user assumes it is present if __NR_msgsnd is present.
> > >
> > > Hm. The kernel change seems to break that assumption. Does anyone with
> > > mips knowledge have an idea whether that was intentional (and the
> > > linux-user code needs to be changed), or whether that's an issue on
the
> > > kernel side?
> > >
> >
> > Hi, Cornelia.
> >
> > Thanks for your involving into this issue!
> >
> > It could be that (not-originating-from-MIPS) kernel commit:
> >
> >
https://github.com/torvalds/linux/commit/1a787fc5ba18ac767e635c58d06a0b46876184e3
> >
> > made a mess with system call availability for MIPS (I will forward this
to
> > MIPS kernel maintainer Paul Burton). My impression is that this was not
> > intentional, and is a temporary instability of kernel interface.
>
> I don't think that's the problematic commit; that one seems to be a
> follow-up on c8ce48f06503 ("asm-generic: Make time32 syscall numbers
> optional") for tools usage (we sync from the 'normal' headers).
>
> The stated intention of the asm-generic commit is to keep 32 bit
> architectures working as before via defining
> __ARCH_WANT_TIME32_SYSCALLS, but it seems that was not done for mips
> (but it should, right?)
>
> > However, I think that QEMU nevertheless should not make the assumption
that
> > if __NR_MSGSND, than semtimedop() is present. It could be true, but it
is
> > still just self-imposed belief in QEMU, kernel never guarantied such
things.
>
> I'm not too familiar with that family of syscalls; is there a better
> way to check for syscall availability here?
>
> > The alternative way of invoking via IPCV6 (else part of “ifdef
> > __NR_MSGSND”) should work for MIPS in the present stage of headers and
> > kernel.
>
> If my assumption above (mips skipped by accident) is correct, we need
> to fix the kernel headers instead :/ -- unless we want to add a
> temporary build fix.
>
> > As a side note, perhaps we shoul update kernel headers only off of
stable
> > kernel releases.
>
> In the past, we have even updated the kernel headers against
> non-mainline (kvm) versions :)
>
> Breakage here seems to be rare (and if this is a real kernel interface
> bug, it'd be a good thing that we caught it);

Definitely a good thing. I think it is fair to say that you found three or
even more bugs, in two separate software projects, and all of this using a
single patch only. :-o Perhaps you should do it more often. ;)

> I believe getting support
> for new features into QEMU quicker makes that a good trade-off.

[Qemu-devel] [PATCH v2 2/2] capstone: Enable disassembly for s390x

[Richard Henderson]

Enable s390x, aka SYSZ, in the git submodule build.
Set the capstone parameters for both s390x host and guest.

Signed-off-by: Richard Henderson 
---
 Makefile   | 1 +
 disas.c| 3 +++
 target/s390x/cpu.c | 4 
 3 files changed, 8 insertions(+)

diff --git a/Makefile b/Makefile
index e02b88bcb1..3b49eed664 100644
--- a/Makefile
+++ b/Makefile
@@ -478,6 +478,7 @@ CAP_CFLAGS += -DCAPSTONE_USE_SYS_DYN_MEM
 CAP_CFLAGS += -DCAPSTONE_HAS_ARM
 CAP_CFLAGS += -DCAPSTONE_HAS_ARM64
 CAP_CFLAGS += -DCAPSTONE_HAS_POWERPC
+CAP_CFLAGS += -DCAPSTONE_HAS_SYSZ
 CAP_CFLAGS += -DCAPSTONE_HAS_X86
 
 subdir-capstone: .git-submodule-status
diff --git a/disas.c b/disas.c
index 41ad0102e2..4a63586af0 100644
--- a/disas.c
+++ b/disas.c
@@ -551,6 +551,9 @@ void disas(FILE *out, void *code, unsigned long size)
 print_insn = print_insn_m68k;
 #elif defined(__s390__)
 print_insn = print_insn_s390;
+s.info.cap_arch = CS_ARCH_SYSZ;
+s.info.cap_insn_unit = 2;
+s.info.cap_insn_split = 6;
 #elif defined(__hppa__)
 print_insn = print_insn_hppa;
 #endif
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index b1df63d82c..553571d86b 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -41,6 +41,7 @@
 #include "sysemu/sysemu.h"
 #endif
 #include "fpu/softfloat.h"
+#include "disas/capstone.h"
 
 #define CR0_RESET   0xE0UL
 #define CR14_RESET  0xC200UL;
@@ -175,6 +176,9 @@ static void s390_cpu_disas_set_info(CPUState *cpu, 
disassemble_info *info)
 {
 info->mach = bfd_mach_s390_64;
 info->print_insn = print_insn_s390;
+info->cap_arch = CS_ARCH_SYSZ;
+info->cap_insn_unit = 2;
+info->cap_insn_split = 6;
 }
 
 static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
-- 
2.17.1

[Qemu-devel] [PATCH v2 0/2] Update capstone module

[Richard Henderson]

There has recently been some good progress in the upstream
capstone repository, syncing the instruction sets from the
(further) upstream llvm.

In particular, there are Power9 and System z13 instructions.
Both of which were missing from our current snapshot and from
our (ancient) binutils opcodes snapshots.

Changes for v2:
  * Drop the installed directory change.  This does force a
different include change when building from git.
  * Drop the s390 skipdata callback for now.


r~


Richard Henderson (2):
  capstone: Update to master
  capstone: Enable disassembly for s390x

 Makefile   | 1 +
 disas.c| 3 +++
 target/s390x/cpu.c | 4 
 capstone   | 2 +-
 configure  | 2 +-
 5 files changed, 10 insertions(+), 2 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH v2 1/2] capstone: Update to master

[Richard Henderson]

Update to fbb20ea83c5a.  Choose this over the 4.0.1 tag because
master now includes the s390x z13 vector opcodes.

Acked-by: David Hildenbrand 
Tested-by: Philippe Mathieu-Daudé 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 capstone  | 2 +-
 configure | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/capstone b/capstone
index 22ead3e0bf..fbb20ea83c 16
--- a/capstone
+++ b/capstone
@@ -1 +1 @@
-Subproject commit 22ead3e0bfdb87516656453336160e0a37b066bf
+Subproject commit fbb20ea83c5af4f29b40c17fbadb1f71b0a08fae
diff --git a/configure b/configure
index 528b9ff705..d3cbdd595c 100755
--- a/configure
+++ b/configure
@@ -5022,7 +5022,7 @@ case "$capstone" in
   git_submodules="${git_submodules} capstone"
 fi
 mkdir -p capstone
-QEMU_CFLAGS="$QEMU_CFLAGS -I\$(SRC_PATH)/capstone/include"
+QEMU_CFLAGS="$QEMU_CFLAGS -I\$(SRC_PATH)/capstone/include/capstone"
 if test "$mingw32" = "yes"; then
   LIBCAPSTONE=capstone.lib
 else
-- 
2.17.1

[Qemu-devel] [RISU v3 09/11] i386: Add avx512 state to reginfo_t

[Jan Bobek]

From: Richard Henderson 

The state expected for a given test must be specifically requested
with the --xfeatures=mask command-line argument.  This is recorded
with the saved state so that it is obvious if the apprentice is given
a different argument.  Any features beyond what are present on the
running cpu will read as zero.

Signed-off-by: Richard Henderson 
---
 risu_reginfo_i386.h |  14 +++
 risu_reginfo_i386.c | 228 ++--
 test_i386.S |  39 
 3 files changed, 273 insertions(+), 8 deletions(-)

diff --git a/risu_reginfo_i386.h b/risu_reginfo_i386.h
index e350f01..b468f79 100644
--- a/risu_reginfo_i386.h
+++ b/risu_reginfo_i386.h
@@ -12,6 +12,10 @@
 #ifndef RISU_REGINFO_I386_H
 #define RISU_REGINFO_I386_H
 
+struct avx512_reg {
+uint64_t q[8];
+};
+
 /*
  * This is the data structure we pass over the socket.
  * It is a simplified and reduced subset of what can
@@ -19,7 +23,17 @@
  */
 struct reginfo {
 uint32_t faulting_insn;
+uint32_t mxcsr;
+uint64_t xfeatures;
+
 gregset_t gregs;
+
+#ifdef __x86_64__
+struct avx512_reg vregs[32];
+#else
+struct avx512_reg vregs[8];
+#endif
+uint64_t kregs[8];
 };
 
 /*
diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
index c4dc14a..83f9541 100644
--- a/risu_reginfo_i386.c
+++ b/risu_reginfo_i386.c
@@ -11,19 +11,32 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
+#include 
 
 #include "risu.h"
 #include "risu_reginfo_i386.h"
 
-const struct option * const arch_long_opts;
-const char * const arch_extra_help;
+#include 
+
+static uint64_t xfeatures = 3;  /* SSE */
+
+static const struct option extra_ops[] = {
+{"xfeatures", required_argument, NULL, FIRST_ARCH_OPT },
+{0, 0, 0, 0}
+};
+
+const struct option * const arch_long_opts = extra_ops;
+const char * const arch_extra_help
+= "  --xfeatures=  Use features in mask for XSAVE\n";
 
 void process_arch_opt(int opt, const char *arg)
 {
-abort();
+assert(opt == FIRST_ARCH_OPT);
+xfeatures = strtoull(arg, 0, 0);
 }
 
 const int reginfo_size(void)
@@ -31,13 +44,37 @@ const int reginfo_size(void)
 return sizeof(struct reginfo);
 }
 
+static void *xsave_feature_buf(struct _xstate *xs, int feature)
+{
+unsigned int eax, ebx, ecx, edx;
+int ok;
+
+/*
+ * Get the location of the XSAVE feature from the cpuid leaf.
+ * Given that we know the xfeature bit is set, this must succeed.
+ */
+ok = __get_cpuid_count(0xd, feature, &eax, &ebx, &ecx, &edx);
+assert(ok);
+
+/* Sanity check that the frame stored by the kernel contains the data. */
+assert(xs->fpstate.sw_reserved.extended_size >= eax + ebx);
+
+return (void *)xs + ebx;
+}
+
 /* reginfo_init: initialize with a ucontext */
 void reginfo_init(struct reginfo *ri, ucontext_t *uc)
 {
-int i;
+int i, nvecregs;
+struct _fpstate *fp;
+struct _xstate *xs;
+uint64_t features;
 
 memset(ri, 0, sizeof(*ri));
 
+/* Require master and apprentice to be given the same arguments.  */
+ri->xfeatures = xfeatures;
+
 for (i = 0; i < NGREG; i++) {
 switch (i) {
 case REG_E(IP):
@@ -79,12 +116,89 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
  * distinguish 'do compare' from 'stop'.
  */
 ri->faulting_insn = *(uint32_t *)uc->uc_mcontext.gregs[REG_E(IP)];
+
+/*
+ * FP state is omitted if unused (aka in init state).
+ * Use the  struct for access to AVX state.
+ */
+
+fp = (struct _fpstate *)uc->uc_mcontext.fpregs;
+if (fp == NULL) {
+return;
+}
+
+#ifdef __x86_64__
+nvecregs = 16;
+#else
+/* We don't (currently) care about the 80387 state, only SSE+.  */
+if (fp->magic != X86_FXSR_MAGIC) {
+return;
+}
+nvecregs = 8;
+#endif
+
+/*
+ * Now we know that _fpstate contains FXSAVE data.
+ */
+ri->mxcsr = fp->mxcsr;
+
+for (i = 0; i < nvecregs; ++i) {
+#ifdef __x86_64__
+memcpy(&ri->vregs[i], &fp->xmm_space[i * 4], 16);
+#else
+memcpy(&ri->vregs[i], &fp->_xmm[i], 16);
+#endif
+}
+
+if (fp->sw_reserved.magic1 != FP_XSTATE_MAGIC1) {
+return;
+}
+xs = (struct _xstate *)fp;
+features = xfeatures & xs->xstate_hdr.xfeatures;
+
+/*
+ * Now we know that _fpstate contains XSAVE data.
+ */
+
+if (features & (1 << 2)) {
+/* YMM_Hi128 state */
+void *buf = xsave_feature_buf(xs, 2);
+for (i = 0; i < nvecregs; ++i) {
+memcpy(&ri->vregs[i].q[2], buf + 16 * i, 16);
+}
+}
+
+if (features & (1 << 5)) {
+/* Opmask state */
+uint64_t *buf = xsave_feature_buf(xs, 5);
+for (i = 0; i < 8; ++i) {
+ri->kregs[i] = buf[i];
+}
+}
+
+if (features & (1 << 6)) {
+/* ZMM_Hi256 state */
+void *buf = xsave_feature_buf(xs, 6);
+for (i = 0; i < nvecregs; ++i) {
+memcpy(&ri->vregs[i].q[4], buf + 32 * i, 32)

[Qemu-devel] [RISU v3 11/11] risu_reginfo_i386: rework --xfeatures value parsing

[Jan Bobek]

Have the --xfeatures option accept "sse", "avx" and "avx512" in
addition to a plain numerical value, purely for users' convenience.
Don't fail silently when an incorrect value is specified, to avoid
confusion.

Suggested-by: Richard Henderson 
Signed-off-by: Jan Bobek 
---
 risu_reginfo_i386.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
index 01ea179..194e0ad 100644
--- a/risu_reginfo_i386.c
+++ b/risu_reginfo_i386.c
@@ -53,8 +53,25 @@ const char * const arch_extra_help
 
 void process_arch_opt(int opt, const char *arg)
 {
+char *endptr;
+
 assert(opt == FIRST_ARCH_OPT);
-xfeatures = strtoull(arg, 0, 0);
+
+if (!strcmp(arg, "sse")) {
+xfeatures = XFEAT_X87 | XFEAT_SSE;
+} else if (!strcmp(arg, "avx")) {
+xfeatures = XFEAT_X87 | XFEAT_SSE | XFEAT_AVX;
+} else if (!strcmp(arg, "avx512")) {
+xfeatures = XFEAT_X87 | XFEAT_SSE | XFEAT_AVX | XFEAT_AVX512;
+} else {
+xfeatures = strtoull(arg, &endptr, 0);
+if (*endptr) {
+fprintf(stderr,
+"Unable to parse '%s' in '%s' into an xfeatures integer 
mask\n",
+endptr, arg);
+exit(1);
+}
+}
 }
 
 const int reginfo_size(void)
-- 
2.20.1

[Qemu-devel] [RISU v3 07/11] test_i386: change syntax from nasm to gas

[Jan Bobek]

This allows us to drop dependency on NASM and build the test image
with GCC only. Adds support for x86_64, too.

Suggested-by: Richard Henderson 
Reviewed-by: Richard Henderson 
Signed-off-by: Jan Bobek 
---
 Makefile|  3 +++
 test_i386.S | 41 +
 test_i386.s | 27 ---
 3 files changed, 44 insertions(+), 27 deletions(-)
 create mode 100644 test_i386.S
 delete mode 100644 test_i386.s

diff --git a/Makefile b/Makefile
index b362dbe..6ab014a 100644
--- a/Makefile
+++ b/Makefile
@@ -49,6 +49,9 @@ $(PROG): $(OBJS)
 %_$(ARCH).elf: %_$(ARCH).s
$(AS) -o $@ $<
 
+%_$(ARCH).elf: %_$(ARCH).S
+   $(CC) $(CPPFLAGS) -o $@ -c $<
+
 clean:
rm -f $(PROG) $(OBJS) $(BINS)
 
diff --git a/test_i386.S b/test_i386.S
new file mode 100644
index 000..456b99c
--- /dev/null
+++ b/test_i386.S
@@ -0,0 +1,41 @@
+/*#
+ * Copyright (c) 2010 Linaro Limited
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Peter Maydell (Linaro) - initial implementation
+ *###*/
+
+/* A trivial test image for x86 */
+
+/* Initialise the registers to avoid spurious mismatches */
+   xor %eax, %eax
+   sahf/* init eflags */
+
+   mov $0x12345678, %eax
+   mov $0x9abcdef0, %ebx
+   mov $0x97361234, %ecx
+   mov $0x84310284, %edx
+   mov $0x83624173, %edi
+   mov $0xfaebfaeb, %esi
+   mov $0x84610123, %ebp
+
+#ifdef __x86_64__
+   movq$0x123456789abcdef0, %r8
+   movq$0x, %r9
+   movq$0x1010101010101010, %r10
+   movq$0x, %r11
+   movq$0x1212121212121212, %r12
+   movq$0x1313131313131313, %r13
+   movq$0x1414141414141414, %r14
+   movq$0x1515151515151515, %r15
+#endif
+
+/* do compare */
+   ud1 %eax, %eax
+
+/* exit test */
+   ud1 %ecx, %eax
diff --git a/test_i386.s b/test_i386.s
deleted file mode 100644
index a2140a0..000
--- a/test_i386.s
+++ /dev/null
@@ -1,27 +0,0 @@
-;###
-;# Copyright (c) 2010 Linaro Limited
-;# All rights reserved. This program and the accompanying materials
-;# are made available under the terms of the Eclipse Public License v1.0
-;# which accompanies this distribution, and is available at
-;# http://www.eclipse.org/legal/epl-v10.html
-;#
-;# Contributors:
-;# Peter Maydell (Linaro) - initial implementation
-;###
-
-; A trivial test image for x86
-
-BITS 32
-; Initialise the registers to avoid spurious mismatches
-mov eax, 0x12345678
-mov ebx, 0x9abcdef0
-mov ecx, 0x97361234
-mov edx, 0x84310284
-mov edi, 0x83624173
-mov esi, 0xfaebfaeb
-mov ebp, 0x84610123
-; UD1 : do compare
-UD1
-
-; UD2 : exit test
-UD2
-- 
2.20.1

[Qemu-devel] [RISU v3 10/11] risu_reginfo_i386: replace xfeature constants with symbolic names

[Jan Bobek]

The original code used "magic numbers", which made it unclear in
some places. Include a reference to the Intel manual where the
constants' meaning is discussed.

Reviewed-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Signed-off-by: Jan Bobek 
---
 risu_reginfo_i386.c | 48 +++--
 1 file changed, 33 insertions(+), 15 deletions(-)

diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
index 83f9541..01ea179 100644
--- a/risu_reginfo_i386.c
+++ b/risu_reginfo_i386.c
@@ -22,7 +22,25 @@
 
 #include 
 
-static uint64_t xfeatures = 3;  /* SSE */
+/*
+ * Refer to "Intel(R) 64 and IA-32 Architectures Software Developer's
+ * Manual", Volume 1, Section 13.1 "XSAVE-Supported Features and
+ * State-Component Bitmaps" for detailed discussion of these constants
+ * and their meaning.
+ */
+enum {
+XFEAT_X87  = 1 << 0,
+XFEAT_SSE  = 1 << 1,
+XFEAT_AVX  = 1 << 2,
+XFEAT_AVX512_OPMASK= 1 << 5,
+XFEAT_AVX512_ZMM_HI256 = 1 << 6,
+XFEAT_AVX512_HI16_ZMM  = 1 << 7,
+XFEAT_AVX512   = XFEAT_AVX512_OPMASK
+   | XFEAT_AVX512_ZMM_HI256
+   | XFEAT_AVX512_HI16_ZMM
+};
+
+static uint64_t xfeatures = XFEAT_X87 | XFEAT_SSE;
 
 static const struct option extra_ops[] = {
 {"xfeatures", required_argument, NULL, FIRST_ARCH_OPT },
@@ -160,34 +178,34 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
  * Now we know that _fpstate contains XSAVE data.
  */
 
-if (features & (1 << 2)) {
+if (features & XFEAT_AVX) {
 /* YMM_Hi128 state */
-void *buf = xsave_feature_buf(xs, 2);
+void *buf = xsave_feature_buf(xs, XFEAT_AVX);
 for (i = 0; i < nvecregs; ++i) {
 memcpy(&ri->vregs[i].q[2], buf + 16 * i, 16);
 }
 }
 
-if (features & (1 << 5)) {
+if (features & XFEAT_AVX512_OPMASK) {
 /* Opmask state */
-uint64_t *buf = xsave_feature_buf(xs, 5);
+uint64_t *buf = xsave_feature_buf(xs, XFEAT_AVX512_OPMASK);
 for (i = 0; i < 8; ++i) {
 ri->kregs[i] = buf[i];
 }
 }
 
-if (features & (1 << 6)) {
+if (features & XFEAT_AVX512_ZMM_HI256) {
 /* ZMM_Hi256 state */
-void *buf = xsave_feature_buf(xs, 6);
+void *buf = xsave_feature_buf(xs, XFEAT_AVX512_ZMM_HI256);
 for (i = 0; i < nvecregs; ++i) {
 memcpy(&ri->vregs[i].q[4], buf + 32 * i, 32);
 }
 }
 
 #ifdef __x86_64__
-if (features & (1 << 7)) {
+if (features & XFEAT_AVX512_HI16_ZMM) {
 /* Hi16_ZMM state */
-void *buf = xsave_feature_buf(xs, 7);
+void *buf = xsave_feature_buf(xs, XFEAT_AVX512_HI16_ZMM);
 for (i = 0; i < 16; ++i) {
 memcpy(&ri->vregs[i + 16], buf + 64 * i, 64);
 }
@@ -243,7 +261,7 @@ static const char *const regname[NGREG] = {
 static int get_nvecregs(uint64_t features)
 {
 #ifdef __x86_64__
-return features & (1 << 7) ? 32 : 16;
+return features & XFEAT_AVX512_HI16_ZMM ? 32 : 16;
 #else
 return 8;
 #endif
@@ -251,9 +269,9 @@ static int get_nvecregs(uint64_t features)
 
 static int get_nvecquads(uint64_t features)
 {
-if (features & (1 << 6)) {
+if (features & XFEAT_AVX512_ZMM_HI256) {
 return 8;
-} else if (features & (1 << 2)) {
+} else if (features & XFEAT_AVX) {
 return 4;
 } else {
 return 2;
@@ -262,9 +280,9 @@ static int get_nvecquads(uint64_t features)
 
 static char get_vecletter(uint64_t features)
 {
-if (features & (1 << 6 | 1 << 7)) {
+if (features & (XFEAT_AVX512_ZMM_HI256 | XFEAT_AVX512_HI16_ZMM)) {
 return 'z';
-} else if (features & (1 << 2)) {
+} else if (features & XFEAT_AVX) {
 return 'y';
 } else {
 return 'x';
@@ -301,7 +319,7 @@ int reginfo_dump(struct reginfo *ri, FILE *f)
 }
 }
 
-if (features & (1 << 5)) {
+if (features & XFEAT_AVX512_OPMASK) {
 for (i = 0; i < 8; i++) {
 fprintf(f, "  k%-5d: %016" PRIx64 "\n", i, ri->kregs[i]);
 }
-- 
2.20.1

[Qemu-devel] [RISU v3 04/11] risu_reginfo_i386: implement arch-specific reginfo interface

[Jan Bobek]

CPU-specific code in risu_reginfo_* is expected to define and export
the following symbols:

- arch_long_opts, arch_extra_help, process_arch_opt
- reginfo_size
- reginfo_init
- reginfo_is_eq
- reginfo_dump, reginfo_dump_mismatch

Make risu_reginfo_i386.c implement this interface; and while we're at
it, expand the support to x86_64 as well.

Suggested-by: Richard Henderson 
Reviewed-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Signed-off-by: Jan Bobek 
---
 risu_reginfo_i386.h |  24 
 risu_reginfo_i386.c | 147 ++--
 2 files changed, 127 insertions(+), 44 deletions(-)

diff --git a/risu_reginfo_i386.h b/risu_reginfo_i386.h
index 5bba439..e350f01 100644
--- a/risu_reginfo_i386.h
+++ b/risu_reginfo_i386.h
@@ -12,7 +12,8 @@
 #ifndef RISU_REGINFO_I386_H
 #define RISU_REGINFO_I386_H
 
-/* This is the data structure we pass over the socket.
+/*
+ * This is the data structure we pass over the socket.
  * It is a simplified and reduced subset of what can
  * be obtained with a ucontext_t*
  */
@@ -21,17 +22,14 @@ struct reginfo {
 gregset_t gregs;
 };
 
-#ifndef REG_GS
-/* Assume that either we get all these defines or none */
-#   define REG_GS  0
-#   define REG_FS  1
-#   define REG_ES  2
-#   define REG_DS  3
-#   define REG_ESP 7
-#   define REG_TRAPNO 12
-#   define REG_EIP14
-#   define REG_EFL16
-#   define REG_UESP   17
-#endif /* !defined(REG_GS) */
+/*
+ * For i386, the defines are named REG_EAX, etc.
+ * For x86_64, the defines are named REG_RAX, etc.
+ */
+#ifdef __x86_64__
+# define REG_E(X)   REG_R##X
+#else
+# define REG_E(X)   REG_E##X
+#endif
 
 #endif /* RISU_REGINFO_I386_H */
diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
index e8d671f..c4dc14a 100644
--- a/risu_reginfo_i386.c
+++ b/risu_reginfo_i386.c
@@ -10,59 +10,144 @@
  
**/
 
 #include 
+#include 
+#include 
 #include 
+#include 
 
 #include "risu.h"
 #include "risu_reginfo_i386.h"
 
-static void fill_reginfo(struct reginfo *ri, ucontext_t * uc)
+const struct option * const arch_long_opts;
+const char * const arch_extra_help;
+
+void process_arch_opt(int opt, const char *arg)
+{
+abort();
+}
+
+const int reginfo_size(void)
+{
+return sizeof(struct reginfo);
+}
+
+/* reginfo_init: initialize with a ucontext */
+void reginfo_init(struct reginfo *ri, ucontext_t *uc)
 {
 int i;
+
+memset(ri, 0, sizeof(*ri));
+
 for (i = 0; i < NGREG; i++) {
 switch (i) {
-case REG_ESP:
-case REG_UESP:
-case REG_GS:
-case REG_FS:
-case REG_ES:
-case REG_DS:
-case REG_TRAPNO:
-case REG_EFL:
-/* Don't store these registers as it results in mismatches.
- * In particular valgrind has different values for some
- * segment registers, and they're boring anyway.
- * We really shouldn't be ignoring EFL but valgrind doesn't
- * seem to set it right and I don't care to investigate.
- */
-ri->gregs[i] = 0xDEADBEEF;
-break;
-case REG_EIP:
-/* Store the offset from the start of the test image */
+case REG_E(IP):
+/* Store the offset from the start of the test image.  */
 ri->gregs[i] = uc->uc_mcontext.gregs[i] - image_start_address;
 break;
-default:
+case REG_EFL:
+/* Store only the "flaggy" bits: SF, ZF, AF, PF, CF.  */
+ri->gregs[i] = uc->uc_mcontext.gregs[i] & 0xd5;
+break;
+case REG_E(SP):
+/* Ignore the stack.  */
+ri->gregs[i] = 0xdeadbeef;
+break;
+case REG_E(AX):
+case REG_E(BX):
+case REG_E(CX):
+case REG_E(DX):
+case REG_E(DI):
+case REG_E(SI):
+case REG_E(BP):
+#ifdef __x86_64__
+case REG_R8:
+case REG_R9:
+case REG_R10:
+case REG_R11:
+case REG_R12:
+case REG_R13:
+case REG_R14:
+case REG_R15:
+#endif
 ri->gregs[i] = uc->uc_mcontext.gregs[i];
 break;
 }
 }
-/* x86 insns aren't 32 bit but we're not really testing x86 so
- * this is just to distinguish 'do compare' from 'stop'
+
+/*
+ * x86 insns aren't 32 bit but 3 bytes are sufficient to
+ * distinguish 'do compare' from 'stop'.
  */
-ri->faulting_insn = *((uint32_t *) uc->uc_mcontext.gregs[REG_EIP]);
+ri->faulting_insn = *(uint32_t *)uc->uc_mcontext.gregs[REG_E(IP)];
 }
 
-static char *regname[] = {
-"GS", "FS", "ES", "DS", "EDI", "ESI", "EBP", "ESP",
-"EBX", "EDX", "ECX", "EAX", "TRAPNO", "ERR", "EIP",
-"CS", "EFL", "UESP", "SS", 0
+/* reginfo_is_eq: compare the reginfo structs, returns nonzero if equal */
+int reginfo_is_eq(struct reginfo *m, struct reginfo *a)
+{
+return 0 == memcmp(m, a, sizeof(*m));
+

[Qemu-devel] [RISU v3 05/11] risu_i386: implement missing CPU-specific functions

[Jan Bobek]

risu_i386.c is expected to implement the following functions:

- advance_pc
- get_reginfo_paramreg, set_ucontext_paramreg
- get_risuop
- get_pc

This patch adds the necessary code. We use EAX as the parameter
register and opcode "UD1 %xxx,%eax" for triggering RISU actions.

Suggested-by: Richard Henderson 
Reviewed-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Signed-off-by: Jan Bobek 
---
 risu_i386.c | 35 ++-
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/risu_i386.c b/risu_i386.c
index 2d2f325..06d95e5 100644
--- a/risu_i386.c
+++ b/risu_i386.c
@@ -25,12 +25,37 @@ static int insn_is_ud2(uint32_t insn)
 
 void advance_pc(void *vuc)
 {
-/* We assume that this is either UD1 or UD2.
- * This would need tweaking if we want to test
- * expected undefs on x86.
+ucontext_t *uc = (ucontext_t *) vuc;
+
+/*
+ * We assume that this is UD1 as per get_risuop below.
+ * This would need tweaking if we want to test expected undefs.
  */
-ucontext_t *uc = vuc;
-uc->uc_mcontext.gregs[REG_EIP] += 2;
+uc->uc_mcontext.gregs[REG_E(IP)] += 3;
+}
+
+void set_ucontext_paramreg(void *vuc, uint64_t value)
+{
+ucontext_t *uc = (ucontext_t *) vuc;
+uc->uc_mcontext.gregs[REG_E(AX)] = value;
+}
+
+uint64_t get_reginfo_paramreg(struct reginfo *ri)
+{
+return ri->gregs[REG_E(AX)];
+}
+
+int get_risuop(struct reginfo *ri)
+{
+if ((ri->faulting_insn & 0xf8) == 0xc0b90f) { /* UD1 %xxx,%eax */
+return (ri->faulting_insn >> 16) & 7;
+}
+return -1;
+}
+
+uintptr_t get_pc(struct reginfo *ri)
+{
+return ri->gregs[REG_E(IP)];
 }
 
 int send_register_info(int sock, void *uc)
-- 
2.20.1

[Qemu-devel] [RISU v3 02/11] risu_i386: move reginfo_t and related defines to risu_reginfo_i386.h

[Jan Bobek]

In order to build risu successfully for i386, we need files
risu_reginfo_i386.{h,c}; this patch adds the former by extracting the
relevant code from risu_i386.c.

This patch is pure code motion; no functional changes were made.

Reviewed-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Signed-off-by: Jan Bobek 
---
 risu_reginfo_i386.h | 37 +
 risu_i386.c | 23 +--
 2 files changed, 38 insertions(+), 22 deletions(-)
 create mode 100644 risu_reginfo_i386.h

diff --git a/risu_reginfo_i386.h b/risu_reginfo_i386.h
new file mode 100644
index 000..5bba439
--- /dev/null
+++ b/risu_reginfo_i386.h
@@ -0,0 +1,37 @@
+/***
+ * Copyright (c) 2010 Linaro Limited
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Peter Maydell (Linaro) - initial implementation
+ 
**/
+
+#ifndef RISU_REGINFO_I386_H
+#define RISU_REGINFO_I386_H
+
+/* This is the data structure we pass over the socket.
+ * It is a simplified and reduced subset of what can
+ * be obtained with a ucontext_t*
+ */
+struct reginfo {
+uint32_t faulting_insn;
+gregset_t gregs;
+};
+
+#ifndef REG_GS
+/* Assume that either we get all these defines or none */
+#   define REG_GS  0
+#   define REG_FS  1
+#   define REG_ES  2
+#   define REG_DS  3
+#   define REG_ESP 7
+#   define REG_TRAPNO 12
+#   define REG_EIP14
+#   define REG_EFL16
+#   define REG_UESP   17
+#endif /* !defined(REG_GS) */
+
+#endif /* RISU_REGINFO_I386_H */
diff --git a/risu_i386.c b/risu_i386.c
index 5e7e01d..6798a78 100644
--- a/risu_i386.c
+++ b/risu_i386.c
@@ -14,28 +14,7 @@
 #include 
 
 #include "risu.h"
-
-/* This is the data structure we pass over the socket.
- * It is a simplified and reduced subset of what can
- * be obtained with a ucontext_t*
- */
-struct reginfo {
-uint32_t faulting_insn;
-gregset_t gregs;
-};
-
-#ifndef REG_GS
-/* Assume that either we get all these defines or none */
-#define REG_GS 0
-#define REG_FS 1
-#define REG_ES 2
-#define REG_DS 3
-#define REG_ESP 7
-#define REG_TRAPNO 12
-#define REG_EIP 14
-#define REG_EFL 16
-#define REG_UESP 17
-#endif
+#include "risu_reginfo_i386.h"
 
 struct reginfo master_ri, apprentice_ri;
 
-- 
2.20.1

[Qemu-devel] [RISU v3 08/11] configure: add i386/x86_64 architectures

[Jan Bobek]

Now that i386 and x86_64 architectures are supported by RISU, we want
to detect them and build RISU for them automatically.

Suggested-by: Richard Henderson 
Reviewed-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Signed-off-by: Jan Bobek 
---
 configure | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index 65e1819..ca2d7db 100755
--- a/configure
+++ b/configure
@@ -48,12 +48,14 @@ EOF
 }
 
 guess_arch() {
-if check_define __m68k__ ; then
-ARCH="m68k"
+if check_define __aarch64__ ; then
+ARCH="aarch64"
 elif check_define __arm__ ; then
 ARCH="arm"
-elif check_define __aarch64__ ; then
-ARCH="aarch64"
+elif check_define __i386__ || check_define __x86_64__ ; then
+ARCH="i386"
+elif check_define __m68k__ ; then
+ARCH="m68k"
 elif check_define __powerpc64__ ; then
 ARCH="ppc64"
 else
-- 
2.20.1

[Qemu-devel] [RISU v3 03/11] risu_i386: move reginfo-related code to risu_reginfo_i386.c

[Jan Bobek]

In order to build risu successfully for i386, we need files
risu_reginfo_i386.{h,c}; this patch adds the latter by extracting the
relevant code from risu_i386.c.

This patch is pure code motion; no functional changes were made.

Reviewed-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Signed-off-by: Jan Bobek 
---
 risu_i386.c | 54 ---
 risu_reginfo_i386.c | 68 +
 2 files changed, 68 insertions(+), 54 deletions(-)
 create mode 100644 risu_reginfo_i386.c

diff --git a/risu_i386.c b/risu_i386.c
index 6798a78..2d2f325 100644
--- a/risu_i386.c
+++ b/risu_i386.c
@@ -33,43 +33,6 @@ void advance_pc(void *vuc)
 uc->uc_mcontext.gregs[REG_EIP] += 2;
 }
 
-static void fill_reginfo(struct reginfo *ri, ucontext_t * uc)
-{
-int i;
-for (i = 0; i < NGREG; i++) {
-switch (i) {
-case REG_ESP:
-case REG_UESP:
-case REG_GS:
-case REG_FS:
-case REG_ES:
-case REG_DS:
-case REG_TRAPNO:
-case REG_EFL:
-/* Don't store these registers as it results in mismatches.
- * In particular valgrind has different values for some
- * segment registers, and they're boring anyway.
- * We really shouldn't be ignoring EFL but valgrind doesn't
- * seem to set it right and I don't care to investigate.
- */
-ri->gregs[i] = 0xDEADBEEF;
-break;
-case REG_EIP:
-/* Store the offset from the start of the test image */
-ri->gregs[i] = uc->uc_mcontext.gregs[i] - image_start_address;
-break;
-default:
-ri->gregs[i] = uc->uc_mcontext.gregs[i];
-break;
-}
-}
-/* x86 insns aren't 32 bit but we're not really testing x86 so
- * this is just to distinguish 'do compare' from 'stop'
- */
-ri->faulting_insn = *((uint32_t *) uc->uc_mcontext.gregs[REG_EIP]);
-}
-
-
 int send_register_info(int sock, void *uc)
 {
 struct reginfo ri;
@@ -100,23 +63,6 @@ int recv_and_compare_register_info(int sock, void *uc)
 return resp;
 }
 
-static char *regname[] = {
-"GS", "FS", "ES", "DS", "EDI", "ESI", "EBP", "ESP",
-"EBX", "EDX", "ECX", "EAX", "TRAPNO", "ERR", "EIP",
-"CS", "EFL", "UESP", "SS", 0
-};
-
-static void dump_reginfo(struct reginfo *ri)
-{
-int i;
-fprintf(stderr, "  faulting insn %x\n", ri->faulting_insn);
-for (i = 0; i < NGREG; i++) {
-fprintf(stderr, "  %s: %x\n", regname[i] ? regname[i] : "???",
-ri->gregs[i]);
-}
-}
-
-
 /* Print a useful report on the status of the last comparison
  * done in recv_and_compare_register_info(). This is called on
  * exit, so need not restrict itself to signal-safe functions.
diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
new file mode 100644
index 000..e8d671f
--- /dev/null
+++ b/risu_reginfo_i386.c
@@ -0,0 +1,68 @@
+/***
+ * Copyright (c) 2010 Linaro Limited
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Peter Maydell (Linaro) - initial implementation
+ 
**/
+
+#include 
+#include 
+
+#include "risu.h"
+#include "risu_reginfo_i386.h"
+
+static void fill_reginfo(struct reginfo *ri, ucontext_t * uc)
+{
+int i;
+for (i = 0; i < NGREG; i++) {
+switch (i) {
+case REG_ESP:
+case REG_UESP:
+case REG_GS:
+case REG_FS:
+case REG_ES:
+case REG_DS:
+case REG_TRAPNO:
+case REG_EFL:
+/* Don't store these registers as it results in mismatches.
+ * In particular valgrind has different values for some
+ * segment registers, and they're boring anyway.
+ * We really shouldn't be ignoring EFL but valgrind doesn't
+ * seem to set it right and I don't care to investigate.
+ */
+ri->gregs[i] = 0xDEADBEEF;
+break;
+case REG_EIP:
+/* Store the offset from the start of the test image */
+ri->gregs[i] = uc->uc_mcontext.gregs[i] - image_start_address;
+break;
+default:
+ri->gregs[i] = uc->uc_mcontext.gregs[i];
+break;
+}
+}
+/* x86 insns aren't 32 bit but we're not really testing x86 so
+ * this is just to distinguish 'do compare' from 'stop'
+ */
+ri->faulting_insn = *((uint32_t *) uc->uc_mcontext.gregs[REG_EIP]);
+}
+
+static char *regname[] = {
+"GS", "FS", "ES", "DS", "EDI", "ESI", "EBP", "ESP",
+"EBX", "EDX", "ECX", "EAX", "TRAPNO", "ERR", "EIP",
+"CS", "

[Qemu-devel] [RISU v3 01/11] Makefile: undefine the arch name symbol

[Jan Bobek]

At least GCC defines the symbol "i386" to 1 to signal the target
platform. We need to use "i386" as an undefined symbol in order to
correctly include risu_reginfo_i386.h from risu.h. Add an -U option to
the build command to make sure the symbol remains undefined.

Suggested-by: Richard Henderson 
Reviewed-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Signed-off-by: Jan Bobek 
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 4aad448..b362dbe 100644
--- a/Makefile
+++ b/Makefile
@@ -17,7 +17,7 @@ VPATH=$(SRCDIR)
 
 CFLAGS ?= -g
 
-ALL_CFLAGS = -Wall -D_GNU_SOURCE -DARCH=$(ARCH) $(BUILD_INC) $(CFLAGS) 
$(EXTRA_CFLAGS)
+ALL_CFLAGS = -Wall -D_GNU_SOURCE -DARCH=$(ARCH) -U$(ARCH) $(BUILD_INC) 
$(CFLAGS) $(EXTRA_CFLAGS)
 
 PROG=risu
 SRCS=risu.c comms.c reginfo.c risu_$(ARCH).c risu_reginfo_$(ARCH).c
-- 
2.20.1

[Qemu-devel] [RISU v3 06/11] risu_i386: remove old unused code

[Jan Bobek]

The code being removed is a remnant of the past implementation; it has
since been replaced by its more powerful, architecture-independent
counterpart in reginfo.c.

Reviewed-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Signed-off-by: Jan Bobek 
---
 risu_i386.c | 58 -
 1 file changed, 58 deletions(-)

diff --git a/risu_i386.c b/risu_i386.c
index 06d95e5..9962b8f 100644
--- a/risu_i386.c
+++ b/risu_i386.c
@@ -16,13 +16,6 @@
 #include "risu.h"
 #include "risu_reginfo_i386.h"
 
-struct reginfo master_ri, apprentice_ri;
-
-static int insn_is_ud2(uint32_t insn)
-{
-return ((insn & 0x) == 0x0b0f);
-}
-
 void advance_pc(void *vuc)
 {
 ucontext_t *uc = (ucontext_t *) vuc;
@@ -57,54 +50,3 @@ uintptr_t get_pc(struct reginfo *ri)
 {
 return ri->gregs[REG_E(IP)];
 }
-
-int send_register_info(int sock, void *uc)
-{
-struct reginfo ri;
-fill_reginfo(&ri, uc);
-return send_data_pkt(sock, &ri, sizeof(ri));
-}
-
-/* Read register info from the socket and compare it with that from the
- * ucontext. Return 0 for match, 1 for end-of-test, 2 for mismatch.
- * NB: called from a signal handler.
- */
-int recv_and_compare_register_info(int sock, void *uc)
-{
-int resp;
-fill_reginfo(&master_ri, uc);
-recv_data_pkt(sock, &apprentice_ri, sizeof(apprentice_ri));
-if (memcmp(&master_ri, &apprentice_ri, sizeof(master_ri)) != 0) {
-/* mismatch */
-resp = 2;
-} else if (insn_is_ud2(master_ri.faulting_insn)) {
-/* end of test */
-resp = 1;
-} else {
-/* either successful match or expected undef */
-resp = 0;
-}
-send_response_byte(sock, resp);
-return resp;
-}
-
-/* Print a useful report on the status of the last comparison
- * done in recv_and_compare_register_info(). This is called on
- * exit, so need not restrict itself to signal-safe functions.
- * Should return 0 if it was a good match (ie end of test)
- * and 1 for a mismatch.
- */
-int report_match_status(void)
-{
-fprintf(stderr, "match status...\n");
-fprintf(stderr, "master reginfo:\n");
-dump_reginfo(&master_ri);
-fprintf(stderr, "apprentice reginfo:\n");
-dump_reginfo(&apprentice_ri);
-if (memcmp(&master_ri, &apprentice_ri, sizeof(master_ri)) == 0) {
-fprintf(stderr, "match!\n");
-return 0;
-}
-fprintf(stderr, "mismatch!\n");
-return 1;
-}
-- 
2.20.1

[Qemu-devel] [RISU v3 00/11] Support for i386/x86_64 with vector extensions

[Jan Bobek]

This patch series adds support for i386 and x86_64 architectures to
RISU. Notably, vector registers (SSE, AVX, AVX-512) are supported for
verification of the apprentice. This is V3 of the series posted in [1]
and [2].

Changes is V3:
  - fix a matching bug caused by misplaced index multiplication [3]
  - print an error and exit when parse of the --xfeatures option fails [4]

References:
  1. https://lists.nongnu.org/archive/html/qemu-devel/2019-04/msg01294.html
  2. https://lists.nongnu.org/archive/html/qemu-devel/2019-05/msg04089.html
  3. https://lists.nongnu.org/archive/html/qemu-devel/2019-05/msg04922.html
  4. https://lists.nongnu.org/archive/html/qemu-devel/2019-05/msg04903.html

Jan Bobek (10):
  Makefile: undefine the arch name symbol
  risu_i386: move reginfo_t and related defines to risu_reginfo_i386.h
  risu_i386: move reginfo-related code to risu_reginfo_i386.c
  risu_reginfo_i386: implement arch-specific reginfo interface
  risu_i386: implement missing CPU-specific functions
  risu_i386: remove old unused code
  test_i386: change syntax from nasm to gas
  configure: add i386/x86_64 architectures
  risu_reginfo_i386: replace xfeature constants with symbolic names
  risu_reginfo_i386: rework --xfeatures value parsing

Richard Henderson (1):
  i386: Add avx512 state to reginfo_t

 configure   |  10 +-
 Makefile|   5 +-
 risu_reginfo_i386.h |  49 ++
 risu_i386.c | 142 ++--
 risu_reginfo_i386.c | 400 
 test_i386.S |  80 +
 test_i386.s |  27 ---
 7 files changed, 556 insertions(+), 157 deletions(-)
 create mode 100644 risu_reginfo_i386.h
 create mode 100644 risu_reginfo_i386.c
 create mode 100644 test_i386.S
 delete mode 100644 test_i386.s

-- 
2.20.1

Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/1] spapr: Do not re-read the clock on pre_save handler on migration

[Maxiwell S. Garcia]

On Thu, May 23, 2019 at 09:29:52AM +1000, David Gibson wrote:
> On Mon, May 20, 2019 at 05:43:40PM -0300, Maxiwell S. Garcia wrote:
> > This handler was added in the commit:
> >   42043e4f1241: spapr: clock should count only if vm is running
> > 
> > In a scenario without migration, this pre_save handler is not
> > triggered, so the 'stop/cont' commands save and restore the clock
> > in the function 'cpu_ppc_clock_vm_state_change.' The SW clock
> > in the guest doesn't know about this pause.
> > 
> > If the command 'migrate' is called between 'stop' and 'cont',
> > the pre_save handler re-read the clock, and the SW clock in the
> > guest will know about the pause between 'stop' and 'migrate.'
> > If the guest is running a workload like HTC, a side-effect of
> > this is a lot of process stall messages (with call traces) in
> > the kernel guest.
> > 
> > Signed-off-by: Maxiwell S. Garcia 
> 
> What affect will this have on the clock for the case of migrations
> without a stop/cont around?

The guest timebase is saved when the VM stop running and restored when
the VM starts running again (cpu_ppc_clock_vm_state_change handler).
Migrations without stop/cont save the clock when the VM go to the
FINISH_MIGRATE state.

> The complicated thing here is that for
> *explicit* stops/continues we want to freeze the clock, however for
> the implicit stop/continue during migration downtime, we want to keep
> the clock running (logically), so that the guest time of day doesn't
> get out of sync on migration.
> 

Not sure if the *implicit* word here is about commands from the libvirt
or any other orchestrator. QEMU itself doesn't know the intent behind the
command stop/cont. So, If we are using a guest to process a workload and
the manager tool decide to migrate our VM transparently, it's unpleasant
to see a lot of process stalls with call traces in the kernel log.
The high-level tools could sync the SW clock with the HW clock if this
behavior is required, keeping the QEMU stop/cont and stop/migrate/cont
consistent.

> > ---
> >  hw/ppc/ppc.c | 24 
> >  1 file changed, 24 deletions(-)
> > 
> > diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> > index ad20584f26..3fb50cbeee 100644
> > --- a/hw/ppc/ppc.c
> > +++ b/hw/ppc/ppc.c
> > @@ -1056,35 +1056,11 @@ void cpu_ppc_clock_vm_state_change(void *opaque, 
> > int running,
> >  }
> >  }
> >  
> > -/*
> > - * When migrating, read the clock just before migration,
> > - * so that the guest clock counts during the events
> > - * between:
> > - *
> > - *  * vm_stop()
> > - *  *
> > - *  * pre_save()
> > - *
> > - *  This reduces clock difference on migration from 5s
> > - *  to 0.1s (when max_downtime == 5s), because sending the
> > - *  final pages of memory (which happens between vm_stop()
> > - *  and pre_save()) takes max_downtime.
> 
> Urgh.. this comment is confusing - 5s would be a ludicrously long
> max_downtime by modern standards.
> 
> > - */
> > -static int timebase_pre_save(void *opaque)
> > -{
> > -PPCTimebase *tb = opaque;
> > -
> > -timebase_save(tb);
> > -
> > -return 0;
> > -}
> > -
> >  const VMStateDescription vmstate_ppc_timebase = {
> >  .name = "timebase",
> >  .version_id = 1,
> >  .minimum_version_id = 1,
> >  .minimum_version_id_old = 1,
> > -.pre_save = timebase_pre_save,
> >  .fields  = (VMStateField []) {
> >  VMSTATE_UINT64(guest_timebase, PPCTimebase),
> >  VMSTATE_INT64(time_of_the_day_ns, PPCTimebase),
> 
> -- 
> David Gibson  | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au| minimalist, thank you.  NOT _the_ 
> _other_
>   | _way_ _around_!
> http://www.ozlabs.org/~dgibson

Re: [Qemu-devel] [PATCH v2 11/28] MAINTAINERS: update for semihostings new home

[Richard Henderson]

On 5/23/19 6:25 AM, Alex Bennée wrote:
> Seeing as I touched it I should at least keep an eye on it.
> 
> Signed-off-by: Alex Bennée 
> Reviewed-by: Philippe Mathieu-Daudé 
> ---
>  MAINTAINERS | 7 +++
>  1 file changed, 7 insertions(+)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 17/28] tests/tcg/aarch64: add system boot.S

[Richard Henderson]

On 5/23/19 6:25 AM, Alex Bennée wrote:
> This provides the bootstrap and low level helper functions for an
> aarch64 kernel. We use semihosting to handle test output and exiting
> the emulation. semihosting's parameter passing is a little funky so we
> end up using the stack and pointing to that as the parameter block.
> 
> Signed-off-by: Alex Bennée 
> 
> ---
> v2
>   - fix tabs
>   - 2 stage table lookup with ro and nx sections
>   - set stack to back
>   - moar comments
> ---
>  tests/tcg/aarch64/Makefile.softmmu-target |  32 +++
>  tests/tcg/aarch64/system/boot.S   | 239 ++
>  tests/tcg/aarch64/system/kernel.ld|  24 +++
>  3 files changed, 295 insertions(+)
>  create mode 100644 tests/tcg/aarch64/Makefile.softmmu-target
>  create mode 100644 tests/tcg/aarch64/system/boot.S
>  create mode 100644 tests/tcg/aarch64/system/kernel.ld

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 09/28] target/mips: only build mips-semi for softmmu

[Richard Henderson]

On 5/23/19 6:25 AM, Alex Bennée wrote:
> The is_uhi gates all semihosting calls and always returns false for
> CONFIG_USER_ONLY builds. There is no reason to build and link
> mips-semi for these builds so lets fix that.
> 
> Signed-off-by: Alex Bennée 
> Reviewed-by: Philippe Mathieu-Daudé 
> Reviewed-by: Aleksandar Markovic 
> ---
>  target/mips/Makefile.objs | 3 ++-
>  target/mips/helper.h  | 2 ++
>  target/mips/translate.c   | 8 
>  3 files changed, 12 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [RFC v4 5/7] tests: New make target check-source

[Markus Armbruster]

Paolo Bonzini  writes:

> On 23/05/19 10:15, Markus Armbruster wrote:
>> A large number of headers don't pass this test, by design or by
>> accident.  To keep things more manageable, exclude all headers outside
>> include/ for now.
>
> A lot of these, either in include/ or outside, are _meant_ to be
> included many times.  What about renaming those to .inc.h and
> whitelisting them in the script?

Yes, that would be nice.

Re: [Qemu-devel] [PATCH 00/15] block: AioContext management, part 2

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190523160104.21258-1-kw...@redhat.com/



Hi,

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

Subject: [Qemu-devel] [PATCH 00/15] block: AioContext management, part 2
Type: series
Message-id: 20190523160104.21258-1-kw...@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
6ca27a5 block: Remove bdrv_set_aio_context()
984ba6d test-bdrv-drain: Use bdrv_try_set_aio_context()
b054561 iotests: Attach new devices to node in non-default iothread
98252b3 virtio-scsi-test: Test attaching new overlay with iothreads
7f6cd93 block: Remove wrong bdrv_set_aio_context() calls
0419720 blockdev: Use bdrv_try_set_aio_context() for monitor commands
26a1acf block: Move node without parents to main AioContext
cd2b0d7 test-block-iothread: BlockBackend AioContext across root node change
5a4c60c test-block-iothread: Test adding parent to iothread node
1767a79 block: Adjust AioContexts when attaching nodes
16a9345 scsi-disk: Use qdev_prop_drive_iothread
cc6b17c block: Add qdev_prop_drive_iothread property type
2e30704 block: Add BlockBackend.ctx
15a3e30 block: Add Error to blk_set_aio_context()
88715f0 test-block-iothread: Check filter node in test_propagate_mirror

=== OUTPUT BEGIN ===
1/15 Checking commit 88715f0ee897 (test-block-iothread: Check filter node in 
test_propagate_mirror)
2/15 Checking commit 15a3e308457c (block: Add Error to blk_set_aio_context())
WARNING: Block comments use a leading /* on a separate line
#104: FILE: hw/block/dataplane/virtio-blk.c:289:
+/* Drain and try to switch bs back to the QEMU main loop. If other users

WARNING: Block comments use a trailing */ on a separate line
#105: FILE: hw/block/dataplane/virtio-blk.c:290:
+ * keep the BlockBackend in the iothread, that's ok */

total: 0 errors, 2 warnings, 259 lines checked

Patch 2/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/15 Checking commit 2e30704b8e57 (block: Add BlockBackend.ctx)
WARNING: Block comments use a leading /* on a separate line
#96: FILE: block/block-backend.c:1865:
+/* FIXME The AioContext of bs and blk can be inconsistent. For the moment,

WARNING: Block comments use a trailing */ on a separate line
#97: FILE: block/block-backend.c:1866:
+ * we prefer the one of bs for compatibility. */

WARNING: Block comments use a leading /* on a separate line
#405: FILE: hw/core/qdev-properties-system.c:83:
+/* BlockBackends of devices start in the main context and are only

WARNING: Block comments use a trailing */ on a separate line
#406: FILE: hw/core/qdev-properties-system.c:84:
+ * later moved into another context if the device supports that. */

WARNING: line over 80 characters
#647: FILE: tests/test-block-backend.c:40:
+BlockBackend *blk = blk_new(qemu_get_aio_context(), BLK_PERM_ALL, 
BLK_PERM_ALL);

WARNING: line over 80 characters
#656: FILE: tests/test-block-backend.c:56:
+BlockBackend *blk = blk_new(qemu_get_aio_context(), BLK_PERM_ALL, 
BLK_PERM_ALL);

total: 0 errors, 6 warnings, 533 lines checked

Patch 3/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/15 Checking commit cc6b17c2c35e (block: Add qdev_prop_drive_iothread property 
type)
ERROR: Macros with complex values should be enclosed in parenthesis
#133: FILE: include/hw/block/block.h:61:
+#define DEFINE_BLOCK_PROPERTIES(_state, _conf)  \
+DEFINE_PROP_DRIVE("drive", _state, _conf.blk),  \
+DEFINE_BLOCK_PROPERTIES_BASE(_state, _conf)

total: 1 errors, 0 warnings, 107 lines checked

Patch 4/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/15 Checking commit 16a934533950 (scsi-disk: Use qdev_prop_drive_iothread)
ERROR: Macros with complex values should be enclosed in parenthesis
#49: FILE: hw/scsi/scsi-disk.c:2939:
+#define DEFINE_SCSI_DISK_PROPERTIES()   \
+DEFINE_PROP_DRIVE_IOTHREAD("drive", SCSIDiskState, qdev.conf.blk),  \
+DEFINE_BLOCK_PROPERTIES_BASE(SCSIDiskState, qdev.conf), \
+DEFINE_BLOCK_ERROR_PROPERTIES(SCSIDiskState, qdev.conf),\
+DEFINE_PROP_STRING("ver", SCSIDiskState, version),  \
+DEFINE_PROP_STRING("serial", SCSIDiskState, serial),\
+DEFINE_PROP_STRING("vendor", SCSIDiskState, vendor),\
+DEFINE_PROP_STRING("product", SCSIDiskState, product),  \

Re: [Qemu-devel] [PATCH 3/3] capstone: Enable disassembly for s390x

[Richard Henderson]

On 5/23/19 3:26 PM, Richard Henderson wrote:
> On 5/22/19 10:42 PM, Richard Henderson wrote:
>> Enable s390x, aka SYSZ, in the git submodule build.
>> Set the capstone parameters for both s390x host and guest.
>> Install a skipdata hook to keep capstone in sync with the
>> instruction stream for unknown opcodes.
>>
>> Signed-off-by: Richard Henderson 
>> ---
>>  Makefile   |  1 +
>>  disas.c| 40 
>>  target/s390x/cpu.c |  4 
>>  3 files changed, 45 insertions(+)
> 
> I'm going to put this patch on hold for now, as I'm trying to get this fixed
> upstream.  If that fails, I'll re-introduce it here.

Bah.  What I meant is the skipdata hook portion of the patch.  I'll split out
the bare "enable s390" portion for v2.


r~

Re: [Qemu-devel] [PATCH 3/3] capstone: Enable disassembly for s390x

[Richard Henderson]

On 5/22/19 10:42 PM, Richard Henderson wrote:
> Enable s390x, aka SYSZ, in the git submodule build.
> Set the capstone parameters for both s390x host and guest.
> Install a skipdata hook to keep capstone in sync with the
> instruction stream for unknown opcodes.
> 
> Signed-off-by: Richard Henderson 
> ---
>  Makefile   |  1 +
>  disas.c| 40 
>  target/s390x/cpu.c |  4 
>  3 files changed, 45 insertions(+)

I'm going to put this patch on hold for now, as I'm trying to get this fixed
upstream.  If that fails, I'll re-introduce it here.


r~

Re: [Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

[Richard Henderson]

On 5/23/19 7:07 AM, Daniel P. Berrangé wrote:
> On Wed, May 22, 2019 at 10:42:27PM -0400, Richard Henderson wrote:
>> Since v4.0, capstone.h has moved to .
> NB this was a regression bug in capstone pkg-config file which has been
> fixed upstream
> 
>https://github.com/aquynh/capstone/pull/1276
> 
> In Fedora we pulled in the fix to our v4.0 builds and I'd suggest
> other distros should do the same
> 

It seems this fix is present in the tagged 4.0 release.

This would have only been present if a distro packaged snapshots.
At least one may have done so, based on

https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg00329.html

but there's no further information to go on.

I've checked Fedora 30 and Debian Buster, which are the only two I could
immediately identify that shipped 4.0.1, as opposed to some 3.x version.  Both
have the pkg-config bug fixed.

Since I cannot test any fixup path, I'm going to drop this patch entirely.


r~

Re: [Qemu-devel] [PULL v3 47/55] linux headers: update against Linux 5.2-rc1

[Laurent Vivier]

On 23/05/2019 21:16, Alex Bennée wrote:
> 
> Laurent Vivier  writes:
> 
>> On 23/05/2019 13:56, Cornelia Huck wrote:
>>> On Wed, 22 May 2019 15:22:23 +0200
>>> Aleksandar Markovic  wrote:
>>>
 The alternative way of invoking via IPCV6 (else part of “ifdef
 __NR_MSGSND”) should work for MIPS in the present stage of headers and
 kernel.
>>>
>>> I tried to do that so that we have at least a workaround for now; but
>>> this fails building on my x86 laptop (the safe_syscall6 for ipc
>>> complains about missing __NR_ipc). Maybe I'm holding it wrong (should
>>> that be conditional on the host?), but I think that really needs to be
>>> done by the mips maintainers...
>>>
>>
>> Perhaps a simple workaround could be:
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index e311fcda0517..5b431736032c 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -761,14 +761,8 @@ safe_syscall2(int, nanosleep, const struct timespec *, 
>> req,
>>   safe_syscall4(int, clock_nanosleep, const clockid_t, clock, int, flags,
>> const struct timespec *, req, struct timespec *, rem)
>>   #endif
>> -#ifdef __NR_msgsnd
>> -safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
>> -  int, flags)
>> -safe_syscall5(int, msgrcv, int, msgid, void *, msgp, size_t, sz,
>> -  long, msgtype, int, flags)
>> -safe_syscall4(int, semtimedop, int, semid, struct sembuf *, tsops,
>> -  unsigned, nsops, const struct timespec *, timeout)
>> -#else
>> +
>> +#ifdef __NR_ipc
>>   /* This host kernel architecture uses a single ipc syscall; fake up
>>* wrappers for the sub-operations to hide this implementation detail.
>>* Annoyingly we can't include linux/ipc.h to get the constant definitions
>> @@ -783,14 +777,30 @@ safe_syscall4(int, semtimedop, int, semid, struct 
>> sembuf *, tsops,
>>
>>   safe_syscall6(int, ipc, int, call, long, first, long, second, long, third,
>> void *, ptr, long, fifth)
>> +#endif
> 
> *sigh* almost but for arches we get complaints when the ipc syscall is
>   defined but not used
> 
>https://app.shippable.com/github/stsquad/qemu/runs/835/summary/console

Yes, I've know.

I have sent a patch with an updated #if:

  #if !defined(__NR_msgsnd) || !defined(__NR_msgrcv) || 
!defined(__NR_semtimedop)

And it should work on any arch.

Thanks,
Laurent

Re: [Qemu-devel] [PULL v3 47/55] linux headers: update against Linux 5.2-rc1

[Alex Bennée]

Laurent Vivier  writes:

> On 23/05/2019 13:56, Cornelia Huck wrote:
>> On Wed, 22 May 2019 15:22:23 +0200
>> Aleksandar Markovic  wrote:
>>
>>> The alternative way of invoking via IPCV6 (else part of “ifdef
>>> __NR_MSGSND”) should work for MIPS in the present stage of headers and
>>> kernel.
>>
>> I tried to do that so that we have at least a workaround for now; but
>> this fails building on my x86 laptop (the safe_syscall6 for ipc
>> complains about missing __NR_ipc). Maybe I'm holding it wrong (should
>> that be conditional on the host?), but I think that really needs to be
>> done by the mips maintainers...
>>
>
> Perhaps a simple workaround could be:
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index e311fcda0517..5b431736032c 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -761,14 +761,8 @@ safe_syscall2(int, nanosleep, const struct timespec *, 
> req,
>  safe_syscall4(int, clock_nanosleep, const clockid_t, clock, int, flags,
>const struct timespec *, req, struct timespec *, rem)
>  #endif
> -#ifdef __NR_msgsnd
> -safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
> -  int, flags)
> -safe_syscall5(int, msgrcv, int, msgid, void *, msgp, size_t, sz,
> -  long, msgtype, int, flags)
> -safe_syscall4(int, semtimedop, int, semid, struct sembuf *, tsops,
> -  unsigned, nsops, const struct timespec *, timeout)
> -#else
> +
> +#ifdef __NR_ipc
>  /* This host kernel architecture uses a single ipc syscall; fake up
>   * wrappers for the sub-operations to hide this implementation detail.
>   * Annoyingly we can't include linux/ipc.h to get the constant definitions
> @@ -783,14 +777,30 @@ safe_syscall4(int, semtimedop, int, semid, struct 
> sembuf *, tsops,
>
>  safe_syscall6(int, ipc, int, call, long, first, long, second, long, third,
>void *, ptr, long, fifth)
> +#endif

*sigh* almost but for arches we get complaints when the ipc syscall is
 defined but not used

  https://app.shippable.com/github/stsquad/qemu/runs/835/summary/console

> +
> +#ifdef __NR_msgsnd
> +safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
> +  int, flags)
> +#else
>  static int safe_msgsnd(int msgid, const void *msgp, size_t sz, int flags)
>  {
>  return safe_ipc(Q_IPCCALL(0, Q_MSGSND), msgid, sz, flags, (void *)msgp, 
> 0);
>  }
> +#endif
> +#ifdef __NR_msgrcv
> +safe_syscall5(int, msgrcv, int, msgid, void *, msgp, size_t, sz,
> +  long, msgtype, int, flags)
> +#else
>  static int safe_msgrcv(int msgid, void *msgp, size_t sz, long type, int 
> flags)
>  {
>  return safe_ipc(Q_IPCCALL(1, Q_MSGRCV), msgid, sz, flags, msgp, type);
>  }
> +#endif
> +#ifdef __NR_semtimedop
> +safe_syscall4(int, semtimedop, int, semid, struct sembuf *, tsops,
> +  unsigned, nsops, const struct timespec *, timeout)
> +#else
>  static int safe_semtimedop(int semid, struct sembuf *tsops, unsigned nsops,
> const struct timespec *timeout)
>  {


--
Alex Bennée

Re: [Qemu-devel] [PATCH] i386: Fix nested SVM on older Opterons

[Dr. David Alan Gilbert]

* Bernhard M. Wiedemann (bwiedem...@suse.de) wrote:
> Without this patch, a VM on a Opteron G3 host will have the svm flag, but
> the kvm-amd module fails to load in there, complaining that it needs
> cpuid 0x800a
> 
> I have successfully built and tested this for 3+ years in production
> on Opteron G3 servers.
> 
> Signed-off-by: Bernhard M. Wiedemann 
> ---
>  target/i386/cpu.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 722c5514d4..df1d81ded8 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -2723,7 +2723,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>  CPUID_EXT_SSE3,
>  .features[FEAT_8000_0001_EDX] =
>  CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
> -.xlevel = 0x8008,
> +.xlevel = 0x800A,
>  .model_id = "AMD Opteron 240 (Gen 1 Class Opteron)",
>  },
>  {
> @@ -2745,7 +2745,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>  CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
>  .features[FEAT_8000_0001_ECX] =
>  CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
> -.xlevel = 0x8008,
> +.xlevel = 0x800A,
>  .model_id = "AMD Opteron 22xx (Gen 2 Class Opteron)",
>  },
>  {
> @@ -2770,7 +2770,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>  .features[FEAT_8000_0001_ECX] =
>  CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A |
>  CPUID_EXT3_ABM | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
> -.xlevel = 0x8008,
> +.xlevel = 0x800A,
>  .model_id = "AMD Opteron 23xx (Gen 3 Class Opteron)",
>  },
>  {

cc'ing in Bandan and Eduardo,

I suspect these need to have compatibility entries on the machine types
so that old machine types don't notice the improvement.

Dave

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

[Qemu-devel] [PATCH] bios-tables-test: add diff allowed list

[Michael S. Tsirkin]

Expected table change is then handled like this:
1. add table to diff allowed list
2. change generating code (can be combined with 1)
3. maintainer runs a script to update expected +
   blows away allowed diff list

Signed-off-by: Michael S. Tsirkin 
---

So I got tired of manual "remember to update AML files"
reminders. With this an AML diff asserts unless it's
explicitly allowed.

Comments?

 tests/bios-tables-test-allowed-diff.h |  1 +
 tests/bios-tables-test.c  | 20 +++-
 2 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 tests/bios-tables-test-allowed-diff.h

diff --git a/tests/bios-tables-test-allowed-diff.h 
b/tests/bios-tables-test-allowed-diff.h
new file mode 100644
index 00..dfb8523c8b
--- /dev/null
+++ b/tests/bios-tables-test-allowed-diff.h
@@ -0,0 +1 @@
+/* List of comma-separated changed AML files to ignore */
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 11e07be093..93db1a7265 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -342,6 +342,23 @@ try_again:
 return exp_tables;
 }
 
+static bool test_acpi_find_diff_allowed(AcpiSdtTable *sdt)
+{
+const gchar *allowed_diff_file[] = {
+#include "bios-tables-test-allowed-diff.h"
+NULL
+};
+int offset = strlen(data_dir) + 1;
+const gchar **f;
+
+for (f = allowed_diff_file; *f; ++f) {
+if (!g_strcmp0(sdt->aml_file + offset, *f)) {
+return true;
+}
+}
+return false;
+}
+
 /* test the list of tables in @data->tables against reference tables */
 static void test_acpi_asl(test_data *data)
 {
@@ -396,7 +413,8 @@ static void test_acpi_asl(test_data *data)
 "see ASL difference.");
 }
 }
-  }
+}
+g_assert(test_acpi_find_diff_allowed(sdt));
 }
 g_string_free(asl, true);
 g_string_free(exp_asl, true);
-- 
MST

Re: [Qemu-devel] [PATCH 2/4] qemu-common: Move qemu_isalnum() etc. to qemu/ctype.h

[Richard Henderson]

On 5/23/19 10:35 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster 
> ---
>  block/ssh.c|  1 +
>  block/vvfat.c  |  1 +
>  gdbstub.c  |  2 ++
>  hw/core/bus.c  |  2 +-
>  hw/core/qdev-properties.c  |  1 +
>  hw/s390x/s390-virtio-ccw.c |  1 +
>  hw/scsi/scsi-generic.c |  2 +-
>  include/qemu-common.h  | 16 
>  include/qemu/ctype.h   | 27 +++
>  monitor.c  |  1 +
>  net/net.c  |  1 +
>  net/tap-solaris.c  |  1 +
>  qapi/qapi-util.c   |  2 +-
>  qobject/json-parser.c  |  2 +-
>  target/ppc/monitor.c   |  2 ++
>  target/riscv/cpu.c |  1 +
>  tests/libqtest.c   |  1 +
>  tests/vhost-user-bridge.c  |  2 +-
>  ui/keymaps.c   |  1 +
>  util/cutils.c  |  3 ++-
>  util/id.c  |  2 +-
>  util/readline.c|  2 +-
>  22 files changed, 50 insertions(+), 24 deletions(-)
>  create mode 100644 include/qemu/ctype.h

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 1/4] qemu-common: Move tcg_enabled() etc. to sysemu/tcg.h

[Richard Henderson]

On 5/23/19 10:35 AM, Markus Armbruster wrote:
> Other accelerators have their own headers: sysemu/hax.h, sysemu/hvf.h,
> sysemu/kvm.h, sysemu/whpx.h.  Only tcg_enabled() & friends sit in
> qemu-common.h.  This necessitates inclusion of qemu-common.h into
> headers, which is against the rules spelled out in qemu-common.h's
> file comment.
> 
> Move tcg_enabled() & friends into their own header sysemu/tcg.h, and
> adjust #include directives.
> 
> Cc: Richard Henderson 
> Cc: Paolo Bonzini 
> Signed-off-by: Markus Armbruster 
> ---

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] Running linux on qemu omap

[Aaro Koskinen]

Hi,

On Thu, May 23, 2019 at 02:00:41PM +0200, Philippe Mathieu-Daudé wrote:
> On 5/23/19 1:27 PM, Thomas Huth wrote:
> > On 22/05/2019 20.19, Aaro Koskinen wrote:
> >> On Wed, May 22, 2019 at 11:33:41AM +0200, Corentin Labbe wrote:
> >>> qemu-system-arm -M help |grep OMAP
> >>> cheetah  Palm Tungsten|E aka. Cheetah PDA (OMAP310)
> >>> n800 Nokia N800 tablet aka. RX-34 (OMAP2420)
> >>> n810 Nokia N810 tablet aka. RX-44 (OMAP2420)
> >>> sx1  Siemens SX1 (OMAP310) V2
> >>> sx1-v1   Siemens SX1 (OMAP310) V1
> >>>
> > The maximum I can get with omap1_defconfig is
> > qemu-system-arm -kernel zImage -nographic -machine cheetah -append 
> > 'root=/dev/ram0 console=ttyO0'
> > Uncompressing Linux... done, booting the kernel.
> > then nothing more.
> >>
> >> With N800/N810 omap2plus_defconfig should be used instead. However,
> >> I don't think that works either (but haven't tried recently). Also with
> >> N800/N810 you need to append the DTB file to the kernel image.
> > 
> > FWIW, Philippe recently posted a mail how to run older kernels on n810:
> > 
> > https://www.mail-archive.com/qemu-devel@nongnu.org/msg610653.html

So it seems the issue with N8x0 is that serial console does not work.
And we are missing the display support in the mainline kernel.

> However I can see than none of the board listed by Corentin are tested
> ... That reminder me I never succeed at using the Cheetah PDA. So the
> OMAP310 is probably bitroting in QEMU...

Cheetah works with serial console. I tried with console on display,
and it seems to boot up, and the frame buffer window gets correctly
sized but for some reason it just stays blank.

A.

Re: [Qemu-devel] [PATCH] linux-user: fix __NR_semtimedop undeclared error

[Philippe Mathieu-Daudé]

On Thu, May 23, 2019 at 8:29 PM Philippe Mathieu-Daudé
 wrote:
> On 5/23/19 7:54 PM, Laurent Vivier wrote:
> > In current code, __NR_msgrcv and__NR_semtimedop are supposed to be
> > defined if __NR_msgsnd is defined.
> >
> > But linux headers 5.2-rc1 for MIPS define __NR_msgsnd without defining
> > __NR_semtimedop and it breaks the QEMU build.
> >
> > __NR_semtimedop is defined in asm-mips/unistd_n64.h and 
> > asm-mips/unistd_n32.h
> > but not in asm-mips/unistd_o32.h.
> >
> > Commit d9cb4336159a ("linux headers: update against Linux 5.2-rc1") has
> > updated asm-mips/unistd_o32.h and added __NR_msgsnd but not __NR_semtimedop.
> > It introduces __NR_semtimedop_time64 instead.
> >
> > This patch fixes the problem by checking for each __NR_XXX symbol
> > before defining the corresponding syscall.
>
> Thanks for the quick fix Laurent.
>
> >
> > Fixes: d9cb4336159a ("linux headers: update against Linux 5.2-rc1")
> > Reported-by: Philippe Mathieu-Daudé 
> > Signed-off-by: Laurent Vivier 
>
> Tested-by: Philippe Mathieu-Daudé 

Aleksandar, you have a pull request in preparation, if you agree with
this patch you might want to include it ;)

Regards,

Phil.

Re: [Qemu-devel] [PATCH] linux-user: fix __NR_semtimedop undeclared error

[Philippe Mathieu-Daudé]

On 5/23/19 7:54 PM, Laurent Vivier wrote:
> In current code, __NR_msgrcv and__NR_semtimedop are supposed to be
> defined if __NR_msgsnd is defined.
> 
> But linux headers 5.2-rc1 for MIPS define __NR_msgsnd without defining
> __NR_semtimedop and it breaks the QEMU build.
> 
> __NR_semtimedop is defined in asm-mips/unistd_n64.h and asm-mips/unistd_n32.h
> but not in asm-mips/unistd_o32.h.
> 
> Commit d9cb4336159a ("linux headers: update against Linux 5.2-rc1") has
> updated asm-mips/unistd_o32.h and added __NR_msgsnd but not __NR_semtimedop.
> It introduces __NR_semtimedop_time64 instead.
> 
> This patch fixes the problem by checking for each __NR_XXX symbol
> before defining the corresponding syscall.

Thanks for the quick fix Laurent.

> 
> Fixes: d9cb4336159a ("linux headers: update against Linux 5.2-rc1")
> Reported-by: Philippe Mathieu-Daudé 
> Signed-off-by: Laurent Vivier 

Tested-by: Philippe Mathieu-Daudé 

> ---
>  linux-user/syscall.c | 24 
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index e311fcda0517..d316de25c9f2 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -761,14 +761,7 @@ safe_syscall2(int, nanosleep, const struct timespec *, 
> req,
>  safe_syscall4(int, clock_nanosleep, const clockid_t, clock, int, flags,
>const struct timespec *, req, struct timespec *, rem)
>  #endif
> -#ifdef __NR_msgsnd
> -safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
> -  int, flags)
> -safe_syscall5(int, msgrcv, int, msgid, void *, msgp, size_t, sz,
> -  long, msgtype, int, flags)
> -safe_syscall4(int, semtimedop, int, semid, struct sembuf *, tsops,
> -  unsigned, nsops, const struct timespec *, timeout)
> -#else
> +#if !defined(__NR_msgsnd) || !defined(__NR_msgrcv) || 
> !defined(__NR_semtimedop)
>  /* This host kernel architecture uses a single ipc syscall; fake up
>   * wrappers for the sub-operations to hide this implementation detail.
>   * Annoyingly we can't include linux/ipc.h to get the constant definitions
> @@ -783,14 +776,29 @@ safe_syscall4(int, semtimedop, int, semid, struct 
> sembuf *, tsops,
>  
>  safe_syscall6(int, ipc, int, call, long, first, long, second, long, third,
>void *, ptr, long, fifth)
> +#endif
> +#ifdef __NR_msgsnd
> +safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
> +  int, flags)
> +#else
>  static int safe_msgsnd(int msgid, const void *msgp, size_t sz, int flags)
>  {
>  return safe_ipc(Q_IPCCALL(0, Q_MSGSND), msgid, sz, flags, (void *)msgp, 
> 0);
>  }
> +#endif
> +#ifdef __NR_msgrcv
> +safe_syscall5(int, msgrcv, int, msgid, void *, msgp, size_t, sz,
> +  long, msgtype, int, flags)
> +#else
>  static int safe_msgrcv(int msgid, void *msgp, size_t sz, long type, int 
> flags)
>  {
>  return safe_ipc(Q_IPCCALL(1, Q_MSGRCV), msgid, sz, flags, msgp, type);
>  }
> +#endif
> +#ifdef __NR_semtimedop
> +safe_syscall4(int, semtimedop, int, semid, struct sembuf *, tsops,
> +  unsigned, nsops, const struct timespec *, timeout)
> +#else
>  static int safe_semtimedop(int semid, struct sembuf *tsops, unsigned nsops,
> const struct timespec *timeout)
>  {
> 

Re: [Qemu-devel] [RISU v2 00/11] Support for i386/x86_64 with vector extensions

[Richard Henderson]

On 5/23/19 2:03 PM, Jan Bobek wrote:
> I ran some experiments on my laptop's Intel(R) Core(TM) i5-4210U to
> find out what actually happens. This CPU supports AVX (but not
> AVX-512) and doesn't support XSAVEC, so I only looked at XSAVE and
> XSAVEOPT (XSAVES is kernel-mode only). I found out that:
> 
> 1. both XSAVE and XSAVEOPT do not include the AVX state if it is in
>the initial configuration (not only XSAVEOPT),
> 2. return to initial configuration is not detected, i.e. the AVX state
>is included even though it has been set to all zeros via vxorps,
> 3. the AVX state can be brought back to the initial configuration via
>XRSTOR with the AVX component removed.

Thanks, for doing the tests.  I'm glad what I understood from my reading
matches experimental results.  :-)


r~

Re: [Qemu-devel] [RISU v2 00/11] Support for i386/x86_64 with vector extensions

[Jan Bobek]

On 5/21/19 12:49 PM, Richard Henderson wrote:
> SSE2 is a mandatory part of the x86_64 ABI.
> 
> I sincerely doubt we care about testing 32-bit that does not have SSE, but 
> even
> then this patch set will not fail, as the kernel will not include the SSE
> registers into the signal frame.  It would be the actual test cases for SSE
> instructions that would SIGILL when run on a 32-bit guest w/o SSE.

Fair enough. I had no idea that SSE2 is mandatory in x86_64, that's
good to know. Let's keep the SSE default then.

> No, the assert is really an assert, because we have also masked the 
> --xfeatures
> value against the set of features stored in the signal frame.  If the kernel
> reports a feature in the signal frame for which there is no cpuid leaf, then
> something is very confused somewhere.

Ah, I see. Makes complete sense.

> I am not sure that we can validate that the features in the signal frame match
> the --xfeatures value, because I *think* that features are omitted from the
> XSAVE data structure when they are in init state.  E.g. when we have not yet
> exercised the feature.
> 
> This caveat is definitely true of ARM SVE, and I found that if I asserted that
> all of the SVE state was in the signal frame that the generated RISU test 
> which
> uses memory would fail the 1st checkpoint, because no SVE instructions had yet
> been executed.
> 
> A careful reading of the XSAVE documentation, plus some experimentation, is
> definitely required.  Maybe hand-craft a test case using XRSTOR, giving it a
> save area that specifies all features to be reset to init state.

tl;dr Richard is exactly right; a component may be missing from the
XSAVE region if it's in the "initial configuration." I'd just leave it
as it is now: it appears everything more advanced than the SSE state
is in the initial configuration when the test image starts
executing. It won't show up until there's instructions which actually
touch it, but by then we get a SIGILL if the HW doesn't support it.

Long story: The Intel manual definitely has a notion of "init
optimization," in addition to "modified optimization." Vol. 1, Section
13.6 "Processor Tracking of XSAVE Managed State" says:

  The XSAVEOPT, XSAVEC, and XSAVES instructions use two optimizations
  to reduce the amount of data that they write to memory. They avoid
  writing data for any state component known to be in its initial
  configuration (the init optimization). In addition, if either
  XSAVEOPT or XSAVES is using the same XSAVE area as that used by the
  most recent execution of XRSTOR or XRSTORS, it may avoid writing
  data for any state component whose configuration is known not to
  have been modified since then (the modified optimization). (XSAVE
  does not use these optimizations, and XSAVEC does not use the
  modified optimization.)

So, XSAVE does not use any optimizations, whereas all other XSAVE
variants use at least the init optimization. Furthermore,

  The following notation describes the state of the init and modified
  optimizations:

  * XINUSE denotes the state-component bitmap corresponding to the
init optimization. If XINUSE[i] = 0, state component i is known to
be in its initial configuration; otherwise XINUSE[i] = 1. It is
possible for XINUSE[i] to be 1 even when state component i is in
its initial configuration. On a processor that does not support
the init optimization, XINUSE[i] is always 1 for every value of i.

  [...]

The processor does not need to detect "return" to the initial
configuration; this makes more sense once it's clear what the initial
configuration is:

  The following items specify the initial configuration each state
  component (for the purposes of defining the XINUSE bitmap):

  * SSE state. In 64-bit mode, SSE state is in its initial
configuration if each of XMM0–XMM15 is 0. Outside 64-bit mode, SSE
state is in its initial configuration if each of XMM0–XMM7 is
0. [...]

  * AVX state. In 64-bit mode, AVX state is in its initial
configuration if each of YMM0_H–YMM15_H is 0. Outside 64-bit mode,
AVX state is in its initial configuration if each of YMM0_H–YMM7_H
is 0. [...]

  [...]

No surprise here; the initial configuration is just all zeros.

I ran some experiments on my laptop's Intel(R) Core(TM) i5-4210U to
find out what actually happens. This CPU supports AVX (but not
AVX-512) and doesn't support XSAVEC, so I only looked at XSAVE and
XSAVEOPT (XSAVES is kernel-mode only). I found out that:

1. both XSAVE and XSAVEOPT do not include the AVX state if it is in
   the initial configuration (not only XSAVEOPT),
2. return to initial configuration is not detected, i.e. the AVX state
   is included even though it has been set to all zeros via vxorps,
3. the AVX state can be brought back to the initial configuration via
   XRSTOR with the AVX component removed.

Cheers,
-Jan



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 3/5] QEMUMachine: add events_wait method

[John Snow]

On 5/23/19 1:49 PM, Max Reitz wrote:
> On 23.05.19 19:06, John Snow wrote:
>> Instead of event_wait which looks for a single event, add an events_wait
>> which can look for any number of events simultaneously. However, it
>> will still only return one at a time, whichever happens first.
>>
>> Signed-off-by: John Snow 
>> ---
>>  python/qemu/__init__.py | 69 +
>>  1 file changed, 49 insertions(+), 20 deletions(-)
>>
>> diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py
>> index 81d9657ec0..98ed8a2e28 100644
>> --- a/python/qemu/__init__.py
>> +++ b/python/qemu/__init__.py
>> @@ -402,42 +402,71 @@ class QEMUMachine(object):
>>  self._qmp.clear_events()
>>  return events
>>  
>> -def event_wait(self, name, timeout=60.0, match=None):
>> +@staticmethod
>> +def event_match(event, match=None):
>>  """
>> -Wait for specified timeout on named event in QMP; optionally filter
>> -results by match.
>> +Check if an event matches optional match criteria.
>>  
>> -The 'match' is checked to be a recursive subset of the 'event'; 
>> skips
>> -branch processing on match's value None
>> -   {"foo": {"bar": 1}} matches {"foo": None}
>> -   {"foo": {"bar": 1}} does not matches {"foo": {"baz": None}}
>> +The match criteria takes the form of a matching subdict. The event 
>> is
>> +checked to be a superset of the subdict, recursively, with matching
>> +values whenever those values are not None.
>> +
>> +Examples, with the subdict queries on the left:
>> + - None matches any object.
>> + - {"foo": None} matches {"foo": {"bar": 1}}
>> + - {"foo": {"baz": None}} does not match {"foo": {"bar": 1}}
> 
> Pre-existing, but the difference between “bar” and “baz” confused me
> quite a bit.
> 
> Also, I wonder...  {"foo": None} would not match {"foo": 1}, right?
> Does that make sense?  Shouldn’t None be the wildcard here in general?
> (Also pre-existing of course.)
> 
> But this patch doesn’t make things worse, so:
> 
> Reviewed-by: Max Reitz 
> 
> (I’d still like your opinion.)
> 

I knew I was inviting trouble by trying to re-document this.

The intention I had when writing the docs, which I think are wrong now,
was for {"foo": None} to match {"foo": 1}, but I think you're right that
it won't because '1' isn't a dict, so it tests for equality instead.

So I need to fix this one up a little bit, but I'll take the review as a
sign that this approach seems workable.

--js

[Qemu-devel] [PATCH] linux-user: fix __NR_semtimedop undeclared error

[Laurent Vivier]

In current code, __NR_msgrcv and__NR_semtimedop are supposed to be
defined if __NR_msgsnd is defined.

But linux headers 5.2-rc1 for MIPS define __NR_msgsnd without defining
__NR_semtimedop and it breaks the QEMU build.

__NR_semtimedop is defined in asm-mips/unistd_n64.h and asm-mips/unistd_n32.h
but not in asm-mips/unistd_o32.h.

Commit d9cb4336159a ("linux headers: update against Linux 5.2-rc1") has
updated asm-mips/unistd_o32.h and added __NR_msgsnd but not __NR_semtimedop.
It introduces __NR_semtimedop_time64 instead.

This patch fixes the problem by checking for each __NR_XXX symbol
before defining the corresponding syscall.

Fixes: d9cb4336159a ("linux headers: update against Linux 5.2-rc1")
Reported-by: Philippe Mathieu-Daudé 
Signed-off-by: Laurent Vivier 
---
 linux-user/syscall.c | 24 
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index e311fcda0517..d316de25c9f2 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -761,14 +761,7 @@ safe_syscall2(int, nanosleep, const struct timespec *, req,
 safe_syscall4(int, clock_nanosleep, const clockid_t, clock, int, flags,
   const struct timespec *, req, struct timespec *, rem)
 #endif
-#ifdef __NR_msgsnd
-safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
-  int, flags)
-safe_syscall5(int, msgrcv, int, msgid, void *, msgp, size_t, sz,
-  long, msgtype, int, flags)
-safe_syscall4(int, semtimedop, int, semid, struct sembuf *, tsops,
-  unsigned, nsops, const struct timespec *, timeout)
-#else
+#if !defined(__NR_msgsnd) || !defined(__NR_msgrcv) || !defined(__NR_semtimedop)
 /* This host kernel architecture uses a single ipc syscall; fake up
  * wrappers for the sub-operations to hide this implementation detail.
  * Annoyingly we can't include linux/ipc.h to get the constant definitions
@@ -783,14 +776,29 @@ safe_syscall4(int, semtimedop, int, semid, struct sembuf 
*, tsops,
 
 safe_syscall6(int, ipc, int, call, long, first, long, second, long, third,
   void *, ptr, long, fifth)
+#endif
+#ifdef __NR_msgsnd
+safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
+  int, flags)
+#else
 static int safe_msgsnd(int msgid, const void *msgp, size_t sz, int flags)
 {
 return safe_ipc(Q_IPCCALL(0, Q_MSGSND), msgid, sz, flags, (void *)msgp, 0);
 }
+#endif
+#ifdef __NR_msgrcv
+safe_syscall5(int, msgrcv, int, msgid, void *, msgp, size_t, sz,
+  long, msgtype, int, flags)
+#else
 static int safe_msgrcv(int msgid, void *msgp, size_t sz, long type, int flags)
 {
 return safe_ipc(Q_IPCCALL(1, Q_MSGRCV), msgid, sz, flags, msgp, type);
 }
+#endif
+#ifdef __NR_semtimedop
+safe_syscall4(int, semtimedop, int, semid, struct sembuf *, tsops,
+  unsigned, nsops, const struct timespec *, timeout)
+#else
 static int safe_semtimedop(int semid, struct sembuf *tsops, unsigned nsops,
const struct timespec *timeout)
 {
-- 
2.20.1

Re: [Qemu-devel] [PATCH v3 0/5] blockdev-backup: don't check aio_context too early

[Max Reitz]

On 23.05.19 19:06, John Snow wrote:
> See patch one's commit message for justification.
> Patches 2-5 are for testing, because that's always how these things go.
> 
> 001/5:[] [--] 'blockdev-backup: don't check aio_context too early'
> 002/5:[0004] [FC] 'iotests.py: do not use infinite waits'
> 003/5:[down]  'QEMUMachine: add events_wait method'
> 004/5:[0022] [FC] 'iotests.py: rewrite run_job to be pickier'
> 005/5:[0017] [FC] 'iotests: add iotest 250 for testing blockdev-backup
>across iothread contexts'
> 
> v3: Rebased on Max's staging branch:
> Rebase patch 2
> added patch 3, to add events_wait.
> Rework patch 4 to make run_job consume legacy events, too
> Minorly edit patch 5 due to the two above.
> v2: added patch 4, with iotest framework adjustments in patches 2/3.
> 
> John Snow (5):
>   blockdev-backup: don't check aio_context too early
>   iotests.py: do not use infinite waits
>   QEMUMachine: add events_wait method
>   iotests.py: rewrite run_job to be pickier
>   iotests: add iotest 250 for testing blockdev-backup across iothread
> contexts
> 
>  blockdev.c|   4 --
>  python/qemu/__init__.py   |  69 +--
>  tests/qemu-iotests/250| 122 ++
>  tests/qemu-iotests/250.out| 119 +
>  tests/qemu-iotests/group  |   1 +
>  tests/qemu-iotests/iotests.py |  60 ++---
>  6 files changed, 326 insertions(+), 49 deletions(-)
>  create mode 100755 tests/qemu-iotests/250
>  create mode 100644 tests/qemu-iotests/250.out

Looks good to me (if it helps:

Reviewed-by: Max Reitz 

), just a question on patch 3 on pre-existing quirks.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 3/5] QEMUMachine: add events_wait method

[Max Reitz]

On 23.05.19 19:06, John Snow wrote:
> Instead of event_wait which looks for a single event, add an events_wait
> which can look for any number of events simultaneously. However, it
> will still only return one at a time, whichever happens first.
> 
> Signed-off-by: John Snow 
> ---
>  python/qemu/__init__.py | 69 +
>  1 file changed, 49 insertions(+), 20 deletions(-)
> 
> diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py
> index 81d9657ec0..98ed8a2e28 100644
> --- a/python/qemu/__init__.py
> +++ b/python/qemu/__init__.py
> @@ -402,42 +402,71 @@ class QEMUMachine(object):
>  self._qmp.clear_events()
>  return events
>  
> -def event_wait(self, name, timeout=60.0, match=None):
> +@staticmethod
> +def event_match(event, match=None):
>  """
> -Wait for specified timeout on named event in QMP; optionally filter
> -results by match.
> +Check if an event matches optional match criteria.
>  
> -The 'match' is checked to be a recursive subset of the 'event'; skips
> -branch processing on match's value None
> -   {"foo": {"bar": 1}} matches {"foo": None}
> -   {"foo": {"bar": 1}} does not matches {"foo": {"baz": None}}
> +The match criteria takes the form of a matching subdict. The event is
> +checked to be a superset of the subdict, recursively, with matching
> +values whenever those values are not None.
> +
> +Examples, with the subdict queries on the left:
> + - None matches any object.
> + - {"foo": None} matches {"foo": {"bar": 1}}
> + - {"foo": {"baz": None}} does not match {"foo": {"bar": 1}}

Pre-existing, but the difference between “bar” and “baz” confused me
quite a bit.

Also, I wonder...  {"foo": None} would not match {"foo": 1}, right?
Does that make sense?  Shouldn’t None be the wildcard here in general?
(Also pre-existing of course.)

But this patch doesn’t make things worse, so:

Reviewed-by: Max Reitz 

(I’d still like your opinion.)



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 0/4] mips: Add more Avocado tests

[Aleksandar Markovic]

On May 23, 2019 7:27 PM, "Philippe Mathieu-Daudé"  wrote:
>
> On 5/23/19 7:11 PM, Aleksandar Markovic wrote:
> > On May 23, 2019 3:45 PM, "Cleber Rosa"  wrote:
> >>> From: "Aleksandar Markovic" 
> >>> On May 22, 2019 11:46 PM, "Cleber Rosa"  wrote:
> > From: "Eduardo Habkost" 
> >
> > Until we actually have a mechanism to exclude the test case on
> > travis-ci, I will remove patch 4/4 from the queue.  Aleksandar,
> > please don't merge patch 4/4 yet or it will break travis-ci.
> >
> > Cleber, Wainer, is it already possible to make "avocado run" skip
> > tests tagged with "slow"?
> >
> 
>  The mechanism exists, but we haven't tagged any test so far as slow.
> 
> >>>
> >>> Cleber,
> >>>
> >>> For the test from patch 4/4, there is no dilemma - it should be in the
> >>> “slow” group, as Philippe envisioned and said, so that it is not
> > humpered
> >>> with stricter requirements for “fast” (default) group. Could you
> > explain us
> >>> how to do it, so that we can hopefully finally proceed?
> >>>
> >>
> >> Hi Aleksandar,
> >>
> >> The point is that there's no "group" definition at this point.  This is
> > the
> >> core of the discussion.
> >>
> >> I think we're close to converging to something simple and effective.
> > Please
> >> let us know what you think of the proposals given.
> >>
> >> Thanks!
> >> - Cleber.
> >>
> >
> > Cleber, hi.
> >
> > Thanks for responding.
> >
> > My views are very similar to Philippe's, but I will provide you with
more
> > details of our (mips) perspective.
> >
> > As far as black/whitelist issues that is a moot point for us - we only
want
> > to be able to have a way to tag a test within the test itself (so,
without
> > updating some common files, external lists,etc.)
> >
> > In general, we would like to have a test environment where we would be
able
> > to test what WE deem suitable for us, without feeling that we bother
you or
> > anybody else, or that we are bothered by others.
> >
> > Let me give you a little extreme example: Let's say we want a complex
test
> > that downloads components from let's say fifty internet location,
executes
> > zillion test cases, and last two days. I wouldn't like anybody to ask me
> > “Why would you that?” or tell me “You can't do this.” or say “No, we did
> > not anticipate such tests, patch rejected.” I (we, people from mips)
should
> > be able to define what I (we) need.
>
> Maybe we can use subdirectory like we do for the TCG tests (Aleksandar
> maintains tests/tcg/mips/). We should try to keep contribution upstream,
> so good idea/pattern can be reused by others.
>
> What I'd like to have with those tests is, at least:
>
> 1/ we don't need to run all the tests (but there is a set of 'quick'
> tests we can run on daily basis)
>
> 2/ maintainers can run their default tests easily (using a combination
> of Avocado tags)
>
> 3/ if a developer working on the PCI subsystem has to modify the MIPS
> subsystem (for example), he should be able to run the MIPS tests before
> sending his series.
>

Exactly! Excellent ideas and examples!

> > Having such test would be a big deal for me, not only that I could run
it
> > manually or automatically every weekend, but I could ask submitters of
> > critical changes: “Did you run this test that we have in Avocado dir?”,
> > without specifying test details, procedures, etc. All this is a BIG deal
> > for me.
> >
> > On the other hand, I agree that certain group of tests (envisioned for
> > daily or so Travis CI) should have some stricter limitations and
structure.
> > But right now I feel humpered by it, and this is counterproductive.
> >
> > So, we want freedom, responsibility and ownersheep of our tests. Please
> > give us the opportunity to get down on business and start writing tests
and
> > start testing.
> >
> > Yours,
> > Aleksandar

Re: [Qemu-devel] [PATCH v5 0/3] qemu-img: rebase: Improve/optimize rebase operation

[Max Reitz]

On 23.05.19 18:33, Sam Eiderman wrote:
> This patch series aims to improve the speed of qemu-img rebase.
> 
>   1. Mainly by removing unnecessary reads when rebasing on the same
>  chain.
>   2. But also by minimizing the number of bdrv_open calls rebase
>  requires.
> 
> v2:
> 
> * Added missing g_free in
>   "qemu-img: rebase: Reuse in-chain BlockDriverState"
> 
> v3:
> 
> * Rebased on top of "Allow rebase with no input base" series
> 
> v4 & v5:
> 
> * Using backing_bs(bs) when a prefix was detected since bs was
>   already checked for allocation.
> 
> Sam Eiderman (3):
>   qemu-img: rebase: Reuse parent BlockDriverState
>   qemu-img: rebase: Reduce reads on in-chain rebase
>   qemu-img: rebase: Reuse in-chain BlockDriverState
> 
>  qemu-img.c | 85 
> --
>  1 file changed, 55 insertions(+), 30 deletions(-)

Thanks, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PULL 00/10] Linux user for 4.1 patches

[Aleksandar Markovic]

On May 23, 2019 6:51 PM,  wrote:
>
> Patchew URL:
https://patchew.org/QEMU/20190523144336.13960-1-laur...@vivier.eu/
>
>
>
> Hi,
>
> This series seems to have some coding style problems. See output below for
> more information:
>
> Message-id: 20190523144336.13960-1-laur...@vivier.eu
> Type: series
> Subject: [Qemu-devel] [PULL 00/10] Linux user for 4.1 patches
>
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
> === TEST SCRIPT END ===
>
> From https://github.com/patchew-project/qemu
>d418238dca..8dc7fd56dd  master -> master
> From https://github.com/patchew-project/qemu
>  * [new tag]   patchew/
20190523144336.13960-1-laur...@vivier.eu -> patchew/
20190523144336.13960-1-laur...@vivier.eu
> Switched to a new branch 'test'
> 4770ccf734 linux-user: Pass through nanosecond timestamp components for
stat syscalls
> 1b5b9faa88 linux-user: Align mmap_find_vma to host page size
> 874caa8bfb linux-user: Fix shmat emulation by honoring host SHMLBA
> d907278358 linux-user: Add support for setsockopt() options
IPV6__MEMBERSHIP
> 57d45df330 linux-user: Sanitize interp_info and, for mips only, init
field fp_abi
> 7267248384 linux-user: Add support for SIOCIFPFLAGS ioctls for all
targets
> 0eac2d71cc linux-user: Add support for SIOCSPGRP ioctl for all targets
> f0e6b29b94 linux-user: Fix support for SIOCATMARK and SIOCGPGRP ioctls
for xtensa
> bd5d878a8c linux-user: add pseudo /proc/hardware for m68k
> 1384b0 linux-user: add pseudo /proc/cpuinfo for sparc
>
> === OUTPUT BEGIN ===
> 1/10 Checking commit 1384b0b5 (linux-user: add pseudo /proc/cpuinfo
for sparc)
> 2/10 Checking commit bd5d878a8cee (linux-user: add pseudo /proc/hardware
for m68k)
> 3/10 Checking commit f0e6b29b94d6 (linux-user: Fix support for SIOCATMARK
and SIOCGPGRP ioctls for xtensa)
> 4/10 Checking commit 0eac2d71ccfa (linux-user: Add support for SIOCSPGRP
ioctl for all targets)
> 5/10 Checking commit 72672483844f (linux-user: Add support for
SIOCIFPFLAGS ioctls for all targets)
> 6/10 Checking commit 57d45df3304f (linux-user: Sanitize interp_info and,
for mips only, init field fp_abi)
> 7/10 Checking commit d907278358c4 (linux-user: Add support for
setsockopt() options IPV6__MEMBERSHIP)
> WARNING: architecture specific defines should be avoided
> #70: FILE: linux-user/syscall.c:1929:
> +#if __UAPI_DEF_IPV6_MREQ
>
> total: 0 errors, 1 warnings, 29 lines checked
>

This warning was known to us, but we can't do anything about it, the line
in question is a “necessary evil” .

> Patch 7/10 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 8/10 Checking commit 874caa8bfb4f (linux-user: Fix shmat emulation by
honoring host SHMLBA)
> WARNING: Block comments use a leading /* on a separate line
> #54: FILE: linux-user/elfload.c:2070:
> +/* The same thing again, but with extra
>
> total: 0 errors, 1 warnings, 221 lines checked
>
> Patch 8/10 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 9/10 Checking commit 1b5b9faa88bd (linux-user: Align mmap_find_vma to
host page size)
> 10/10 Checking commit 4770ccf73400 (linux-user: Pass through nanosecond
timestamp components for stat syscalls)
> ERROR: code indent should never use tabs
> #105: FILE: linux-user/syscall_defs.h:1203:
> +^Iabi_ulong  target_st_atime_nsec;$
>
> ERROR: code indent should never use tabs
> #108: FILE: linux-user/syscall_defs.h:1205:
> +^Iabi_ulong  target_st_mtime_nsec;$
>
> ERROR: code indent should never use tabs
> #111: FILE: linux-user/syscall_defs.h:1207:
> +^Iabi_ulong  target_st_ctime_nsec;$
>
> ERROR: code indent should never use tabs
> #120: FILE: linux-user/syscall_defs.h:1239:
> +^Iabi_ulong^Itarget_st_atime_nsec;$
>
> ERROR: code indent should never use tabs
> #124: FILE: linux-user/syscall_defs.h:1242:
> +^Iabi_ulong^Itarget_st_mtime_nsec;$
>
> ERROR: code indent should never use tabs
> #128: FILE: linux-user/syscall_defs.h:1245:
> +^Iabi_ulong^Itarget_st_ctime_nsec;$
>
> ERROR: code indent should never use tabs
> #137: FILE: linux-user/syscall_defs.h:1324:
> +^Iabi_ulong^Itarget_st_atime_nsec;$
>
> ERROR: code indent should never use tabs
> #141: FILE: linux-user/syscall_defs.h:1327:
> +^Iabi_ulong^Itarget_st_mtime_nsec;$
>
> ERROR: code indent should never use tabs
> #145: FILE: linux-user/syscall_defs.h:1330:
> +^Iabi_ulong^Itarget_st_ctime_nsec;$
>
> ERROR: code indent should never use tabs
> #161: FILE: linux-user/syscall_defs.h:1348:
> +^Iabi_ulong^Itarget_st_atime_nsec;$
>
> ERROR: code indent should never use tabs
> #164: FILE: linux-user/syscall_defs.h:1350:
> +^Iabi_ulong^Itarget_st_mtime_nsec;$
>
> ERROR: code indent should never use tabs
>

Re: [Qemu-devel] [PATCH v4 02/11] block: Filtered children access functions

[Max Reitz]

On 17.05.19 16:50, Vladimir Sementsov-Ogievskiy wrote:
> 10.04.2019 23:20, Max Reitz wrote:
>> What bs->file and bs->backing mean depends on the node.  For filter
>> nodes, both signify a node that will eventually receive all R/W
>> accesses.  For format nodes, bs->file contains metadata and data, and
>> bs->backing will not receive writes -- instead, writes are COWed to
>> bs->file.  Usually.
>>
>> In any case, it is not trivial to guess what a child means exactly with
>> our currently limited form of expression.  It is better to introduce
>> some functions that actually guarantee a meaning:
>>
>> - bdrv_filtered_cow_child() will return the child that receives requests
>>filtered through COW.  That is, reads may or may not be forwarded
>>(depending on the overlay's allocation status), but writes never go to
>>this child.
>>
>> - bdrv_filtered_rw_child() will return the child that receives requests
>>filtered through some very plain process.  Reads and writes issued to
>>the parent will go to the child as well (although timing, etc. may be
>>modified).
>>
>> - All drivers but quorum (but quorum is pretty opaque to the general
>>block layer anyway) always only have one of these children: All read
>>requests must be served from the filtered_rw_child (if it exists), so
>>if there was a filtered_cow_child in addition, it would not receive
>>any requests at all.
>>(The closest here is mirror, where all requests are passed on to the
>>source, but with write-blocking, write requests are "COWed" to the
>>target.  But that just means that the target is a special child that
>>cannot be introspected by the generic block layer functions, and that
>>source is a filtered_rw_child.)
>>Therefore, we can also add bdrv_filtered_child() which returns that
>>one child (or NULL, if there is no filtered child).
>>
>> Also, many places in the current block layer should be skipping filters
>> (all filters or just the ones added implicitly, it depends) when going
>> through a block node chain.  They do not do that currently, but this
>> patch makes them.
>>
>> One example for this is qemu-img map, which should skip filters and only
>> look at the COW elements in the graph.  The change to iotest 204's
>> reference output shows how using blkdebug on top of a COW node used to
>> make qemu-img map disregard the rest of the backing chain, but with this
>> patch, the allocation in the base image is reported correctly.
>>
>> Furthermore, a note should be made that sometimes we do want to access
>> bs->backing directly.  This is whenever the operation in question is not
>> about accessing the COW child, but the "backing" child, be it COW or
>> not.  This is the case in functions such as bdrv_open_backing_file() or
>> whenever we have to deal with the special behavior of @backing as a
>> blockdev option, which is that it does not default to null like all
>> other child references do.
>>
>> Finally, the query functions (query-block and query-named-block-nodes)
>> are modified to return any filtered child under "backing", not just
>> bs->backing or COW children.  This is so that filters do not interrupt
>> the reported backing chain.  This changes the output of iotest 184, as
>> the throttled node now appears as a backing child.
>>
>> Signed-off-by: Max Reitz 
>> ---
> 
> [..]
> 
>> --- a/block/mirror.c
>> +++ b/block/mirror.c

[...]

>> @@ -1650,7 +1651,9 @@ static void mirror_start_job(const char *job_id, 
>> BlockDriverState *bs,
>>* any jobs in them must be blocked */
>>   if (target_is_backing) {
>>   BlockDriverState *iter;
>> -for (iter = backing_bs(bs); iter != target; iter = 
>> backing_bs(iter)) {
>> +for (iter = bdrv_filtered_bs(bs); iter != target;
> 
> should it be filtered_target too?

Hmm...  The comment says that all nodes that disappear must be blocked.
 I don’t even know by heart which nodes I let disappear. :-/

I suppose we should start at the first explicit node, filter or not...?

>> + iter = bdrv_filtered_bs(iter))
>> +{
>>   /* XXX BLK_PERM_WRITE needs to be allowed so we don't block
>>* ourselves at s->base (if writes are blocked for a node, 
>> they are
>>* also blocked for its backing file). The other options would 
>> be a

[...]

>> @@ -1707,14 +1710,14 @@ void mirror_start(const char *job_id, 
>> BlockDriverState *bs,
>> MirrorCopyMode copy_mode, Error **errp)
>>   {
>>   bool is_none_mode;
>> -BlockDriverState *base;
>> +BlockDriverState *base = NULL;
> 
> dead assignment

Now I wonder why I even have that.  Probably an artifact from some
intermediate point.

>>   
>>   if (mode == MIRROR_SYNC_MODE_INCREMENTAL) {
>>   error_setg(errp, "Sync mode 'incremental' not supported");
>>   return;
>>   }
>>   is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
>> -base = mode == MIRROR_SYNC_MODE_TO

Re: [Qemu-devel] [PATCH 0/4] mips: Add more Avocado tests

[Philippe Mathieu-Daudé]

On 5/23/19 7:11 PM, Aleksandar Markovic wrote:
> On May 23, 2019 3:45 PM, "Cleber Rosa"  wrote:
>>> From: "Aleksandar Markovic" 
>>> On May 22, 2019 11:46 PM, "Cleber Rosa"  wrote:
> From: "Eduardo Habkost" 
>
> Until we actually have a mechanism to exclude the test case on
> travis-ci, I will remove patch 4/4 from the queue.  Aleksandar,
> please don't merge patch 4/4 yet or it will break travis-ci.
>
> Cleber, Wainer, is it already possible to make "avocado run" skip
> tests tagged with "slow"?
>

 The mechanism exists, but we haven't tagged any test so far as slow.

>>>
>>> Cleber,
>>>
>>> For the test from patch 4/4, there is no dilemma - it should be in the
>>> “slow” group, as Philippe envisioned and said, so that it is not
> humpered
>>> with stricter requirements for “fast” (default) group. Could you
> explain us
>>> how to do it, so that we can hopefully finally proceed?
>>>
>>
>> Hi Aleksandar,
>>
>> The point is that there's no "group" definition at this point.  This is
> the
>> core of the discussion.
>>
>> I think we're close to converging to something simple and effective.
> Please
>> let us know what you think of the proposals given.
>>
>> Thanks!
>> - Cleber.
>>
> 
> Cleber, hi.
> 
> Thanks for responding.
> 
> My views are very similar to Philippe's, but I will provide you with more
> details of our (mips) perspective.
> 
> As far as black/whitelist issues that is a moot point for us - we only want
> to be able to have a way to tag a test within the test itself (so, without
> updating some common files, external lists,etc.)
> 
> In general, we would like to have a test environment where we would be able
> to test what WE deem suitable for us, without feeling that we bother you or
> anybody else, or that we are bothered by others.
> 
> Let me give you a little extreme example: Let's say we want a complex test
> that downloads components from let's say fifty internet location, executes
> zillion test cases, and last two days. I wouldn't like anybody to ask me
> “Why would you that?” or tell me “You can't do this.” or say “No, we did
> not anticipate such tests, patch rejected.” I (we, people from mips) should
> be able to define what I (we) need.

Maybe we can use subdirectory like we do for the TCG tests (Aleksandar
maintains tests/tcg/mips/). We should try to keep contribution upstream,
so good idea/pattern can be reused by others.

What I'd like to have with those tests is, at least:

1/ we don't need to run all the tests (but there is a set of 'quick'
tests we can run on daily basis)

2/ maintainers can run their default tests easily (using a combination
of Avocado tags)

3/ if a developer working on the PCI subsystem has to modify the MIPS
subsystem (for example), he should be able to run the MIPS tests before
sending his series.

> Having such test would be a big deal for me, not only that I could run it
> manually or automatically every weekend, but I could ask submitters of
> critical changes: “Did you run this test that we have in Avocado dir?”,
> without specifying test details, procedures, etc. All this is a BIG deal
> for me.
> 
> On the other hand, I agree that certain group of tests (envisioned for
> daily or so Travis CI) should have some stricter limitations and structure.
> But right now I feel humpered by it, and this is counterproductive.
> 
> So, we want freedom, responsibility and ownersheep of our tests. Please
> give us the opportunity to get down on business and start writing tests and
> start testing.
> 
> Yours,
> Aleksandar

[Qemu-devel] [PATCH v3 5/5] iotests: add iotest 250 for testing blockdev-backup across iothread contexts

[John Snow]

Signed-off-by: John Snow 
---
 tests/qemu-iotests/250 | 122 +
 tests/qemu-iotests/250.out | 119 
 tests/qemu-iotests/group   |   1 +
 3 files changed, 242 insertions(+)
 create mode 100755 tests/qemu-iotests/250
 create mode 100644 tests/qemu-iotests/250.out

diff --git a/tests/qemu-iotests/250 b/tests/qemu-iotests/250
new file mode 100755
index 00..c594a43205
--- /dev/null
+++ b/tests/qemu-iotests/250
@@ -0,0 +1,122 @@
+#!/usr/bin/env python
+#
+# Test incremental/backup across iothread contexts
+#
+# Copyright (c) 2019 John Snow for Red Hat, Inc.
+#
+# 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 .
+#
+# owner=js...@redhat.com
+
+import os
+import iotests
+from iotests import log
+
+iotests.verify_image_format(supported_fmts=['qcow2'])
+size = 64 * 1024 * 1024
+
+with iotests.FilePath('img0') as img0_path, \
+ iotests.FilePath('img1') as img1_path, \
+ iotests.FilePath('img0-full') as img0_full_path, \
+ iotests.FilePath('img1-full') as img1_full_path, \
+ iotests.FilePath('img0-incr') as img0_incr_path, \
+ iotests.FilePath('img1-incr') as img1_incr_path, \
+ iotests.VM() as vm:
+
+def create_target(filepath, name, size):
+basename = os.path.basename(filepath)
+nodename = "file_{}".format(basename)
+log(vm.command('blockdev-create', job_id='job1',
+   options={
+   'driver': 'file',
+   'filename': filepath,
+   'size': 0,
+   }))
+vm.run_job('job1')
+log(vm.command('blockdev-add', driver='file',
+   node_name=nodename, filename=filepath))
+log(vm.command('blockdev-create', job_id='job2',
+   options={
+   'driver': iotests.imgfmt,
+   'file': nodename,
+   'size': size,
+   }))
+vm.run_job('job2')
+log(vm.command('blockdev-add', driver=iotests.imgfmt,
+   node_name=name,
+   file=nodename))
+
+log('--- Preparing images & VM ---\n')
+vm.add_object('iothread,id=iothread0')
+vm.add_object('iothread,id=iothread1')
+vm.add_device('virtio-scsi-pci,id=scsi0,iothread=iothread0')
+vm.add_device('virtio-scsi-pci,id=scsi1,iothread=iothread1')
+iotests.qemu_img_create('-f', iotests.imgfmt, img0_path, str(size))
+iotests.qemu_img_create('-f', iotests.imgfmt, img1_path, str(size))
+vm.add_drive(img0_path, interface='none')
+vm.add_device('scsi-hd,id=device0,drive=drive0,bus=scsi0.0')
+vm.add_drive(img1_path, interface='none')
+vm.add_device('scsi-hd,id=device1,drive=drive1,bus=scsi1.0')
+
+log('--- Starting VM ---\n')
+vm.launch()
+
+log('--- Create Targets & Full Backups ---\n')
+create_target(img0_full_path, 'img0-full', size)
+create_target(img1_full_path, 'img1-full', size)
+ret = vm.qmp_log('transaction', indent=2, actions=[
+{ 'type': 'block-dirty-bitmap-add',
+  'data': { 'node': 'drive0', 'name': 'bitmap0' }},
+{ 'type': 'block-dirty-bitmap-add',
+  'data': { 'node': 'drive1', 'name': 'bitmap1' }},
+{ 'type': 'blockdev-backup',
+  'data': { 'device': 'drive0',
+'target': 'img0-full',
+'sync': 'full',
+'job-id': 'j0' }},
+{ 'type': 'blockdev-backup',
+  'data': { 'device': 'drive1',
+'target': 'img1-full',
+'sync': 'full',
+'job-id': 'j1' }}
+])
+if "error" in ret:
+raise Exception(ret['error']['desc'])
+vm.run_job('j0', auto_dismiss=True)
+vm.run_job('j1', auto_dismiss=True)
+
+log('\n--- Create Targets & Incremental Backups ---\n')
+create_target(img0_incr_path, 'img0-incr', size)
+create_target(img1_incr_path, 'img1-incr', size)
+ret = vm.qmp_log('transaction', indent=2, actions=[
+{ 'type': 'blockdev-backup',
+  'data': { 'device': 'drive0',
+'target': 'img0-incr',
+'sync': 'incremental',
+'bitmap': 'bitmap0',
+'job-id': 'j2' }},
+{ 'type': 'blockdev-backup',
+  'data': { '

[Qemu-devel] [PATCH v3 3/5] QEMUMachine: add events_wait method

[John Snow]

Instead of event_wait which looks for a single event, add an events_wait
which can look for any number of events simultaneously. However, it
will still only return one at a time, whichever happens first.

Signed-off-by: John Snow 
---
 python/qemu/__init__.py | 69 +
 1 file changed, 49 insertions(+), 20 deletions(-)

diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py
index 81d9657ec0..98ed8a2e28 100644
--- a/python/qemu/__init__.py
+++ b/python/qemu/__init__.py
@@ -402,42 +402,71 @@ class QEMUMachine(object):
 self._qmp.clear_events()
 return events
 
-def event_wait(self, name, timeout=60.0, match=None):
+@staticmethod
+def event_match(event, match=None):
 """
-Wait for specified timeout on named event in QMP; optionally filter
-results by match.
+Check if an event matches optional match criteria.
 
-The 'match' is checked to be a recursive subset of the 'event'; skips
-branch processing on match's value None
-   {"foo": {"bar": 1}} matches {"foo": None}
-   {"foo": {"bar": 1}} does not matches {"foo": {"baz": None}}
+The match criteria takes the form of a matching subdict. The event is
+checked to be a superset of the subdict, recursively, with matching
+values whenever those values are not None.
+
+Examples, with the subdict queries on the left:
+ - None matches any object.
+ - {"foo": None} matches {"foo": {"bar": 1}}
+ - {"foo": {"baz": None}} does not match {"foo": {"bar": 1}}
+ - {"foo": {"baz": 2}} matches {"foo": {"bar": 1, "baz": 2}}
 """
-def event_match(event, match=None):
-if match is None:
-return True
+if match is None:
+return True
 
-for key in match:
-if key in event:
-if isinstance(event[key], dict):
-if not event_match(event[key], match[key]):
-return False
-elif event[key] != match[key]:
+for key in match:
+if key in event:
+if isinstance(event[key], dict):
+if not QEMUMachine.event_match(event[key], match[key]):
 return False
-else:
+elif event[key] != match[key]:
 return False
+else:
+return False
+return True
 
-return True
+def event_wait(self, name, timeout=60.0, match=None):
+"""
+event_wait waits for and returns a named event from QMP with a timeout.
+
+name: The event to wait for.
+timeout: QEMUMonitorProtocol.pull_event timeout parameter.
+match: Optional match criteria. See event_match for details.
+"""
+return self.events_wait([(name, match)], timeout)
+
+def events_wait(self, events, timeout=60.0):
+"""
+events_wait waits for and returns a named event from QMP with a 
timeout.
+
+events: a sequence of (name, match_criteria) tuples.
+The match criteria are optional and may be None.
+See event_match for details.
+timeout: QEMUMonitorProtocol.pull_event timeout parameter.
+"""
+def _match(event):
+for name, match in events:
+if (event['event'] == name and
+self.event_match(event, match)):
+return True
+return False
 
 # Search cached events
 for event in self._events:
-if (event['event'] == name) and event_match(event, match):
+if _match(event):
 self._events.remove(event)
 return event
 
 # Poll for new events
 while True:
 event = self._qmp.pull_event(wait=timeout)
-if (event['event'] == name) and event_match(event, match):
+if _match(event):
 return event
 self._events.append(event)
 
-- 
2.20.1

Re: [Qemu-devel] [PATCH 0/4] mips: Add more Avocado tests

[Aleksandar Markovic]

On May 23, 2019 3:45 PM, "Cleber Rosa"  wrote:
>
>
>
> - Original Message -
> > From: "Aleksandar Markovic" 
> > To: "Cleber Rosa" 
> > Cc: "Wainer dos Santos Moschetta" , "Aleksandar
Markovic" ,
> > qemu-devel@nongnu.org, "Aleksandar Rikalo" ,
"Eduardo Habkost" ,
> > "Aurelien Jarno" , "Philippe Mathieu-Daudé" <
f4...@amsat.org>
> > Sent: Wednesday, May 22, 2019 6:43:54 PM
> > Subject: Re: [Qemu-devel] [PATCH 0/4] mips: Add more Avocado tests
> >
> > On May 22, 2019 11:46 PM, "Cleber Rosa"  wrote:
> > >
> > >
> > >
> > > - Original Message -
> > > > From: "Eduardo Habkost" 
> > > > To: "Philippe Mathieu-Daudé" 
> > > > Cc: qemu-devel@nongnu.org, "Aleksandar Rikalo" ,
> > "Aleksandar Markovic"
> > > > , "Aleksandar Markovic" <
> > amarko...@wavecomp.com>, "Cleber Rosa" ,
> > > > "Aurelien Jarno" , "Wainer dos Santos
Moschetta" <
> > waine...@redhat.com>
> > > > Sent: Wednesday, May 22, 2019 5:12:30 PM
> > > > Subject: Re: [Qemu-devel] [PATCH 0/4] mips: Add more Avocado tests
> > > >
> > > > On Tue, May 21, 2019 at 01:19:06AM +0200, Philippe Mathieu-Daudé
wrote:
> > > > > Hi,
> > > > >
> > > > > It was a rainy week-end here, so I invested it to automatize some
> > > > > of my MIPS tests.
> > > > >
> > > > > The BootLinuxSshTest is not Global warming friendly, it is not
> > > > > meant to run on a CI system but rather on a workstation previous
> > > > > to post a pull request.
> > > > > It can surely be improved, but it is a good starting point.
> > > >
> > > > Until we actually have a mechanism to exclude the test case on
> > > > travis-ci, I will remove patch 4/4 from the queue.  Aleksandar,
> > > > please don't merge patch 4/4 yet or it will break travis-ci.
> > > >
> > > > Cleber, Wainer, is it already possible to make "avocado run" skip
> > > > tests tagged with "slow"?
> > > >
> > >
> > > The mechanism exists, but we haven't tagged any test so far as slow.
> > >
> >
> > Cleber,
> >
> > For the test from patch 4/4, there is no dilemma - it should be in the
> > “slow” group, as Philippe envisioned and said, so that it is not
humpered
> > with stricter requirements for “fast” (default) group. Could you
explain us
> > how to do it, so that we can hopefully finally proceed?
> >
>
> Hi Aleksandar,
>
> The point is that there's no "group" definition at this point.  This is
the
> core of the discussion.
>
> I think we're close to converging to something simple and effective.
Please
> let us know what you think of the proposals given.
>
> Thanks!
> - Cleber.
>

Cleber, hi.

Thanks for responding.

My views are very similar to Philippe's, but I will provide you with more
details of our (mips) perspective.

As far as black/whitelist issues that is a moot point for us - we only want
to be able to have a way to tag a test within the test itself (so, without
updating some common files, external lists,etc.)

In general, we would like to have a test environment where we would be able
to test what WE deem suitable for us, without feeling that we bother you or
anybody else, or that we are bothered by others.

Let me give you a little extreme example: Let's say we want a complex test
that downloads components from let's say fifty internet location, executes
zillion test cases, and last two days. I wouldn't like anybody to ask me
“Why would you that?” or tell me “You can't do this.” or say “No, we did
not anticipate such tests, patch rejected.” I (we, people from mips) should
be able to define what I (we) need.

Having such test would be a big deal for me, not only that I could run it
manually or automatically every weekend, but I could ask submitters of
critical changes: “Did you run this test that we have in Avocado dir?”,
without specifying test details, procedures, etc. All this is a BIG deal
for me.

On the other hand, I agree that certain group of tests (envisioned for
daily or so Travis CI) should have some stricter limitations and structure.
But right now I feel humpered by it, and this is counterproductive.

So, we want freedom, responsibility and ownersheep of our tests. Please
give us the opportunity to get down on business and start writing tests and
start testing.

Yours,
Aleksandar

> > Gratefully,
> > Aleksandar
> >
> > > Should we define/document a criteria for a test to be slow?  Given
> > > that this is highly subjective, we have to think of:
> > >
> > >  * Will we consider the average or maximum run time (the timeout
> > >definition)?
> > >
> > >  * For a single test, what is "slow"? Some rough numbers from Travis
> > >CI[1] to help us with guidelines:
> > >- boot_linux_console.py:BootLinuxConsole.test_x86_64_pc:  PASS
(6.04 s)
> > >- boot_linux_console.py:BootLinuxConsole.test_arm_virt:  PASS
(2.91 s)
> > >-
> >
linux_initrd.py:LinuxInitrd.test_with_2gib_file_should_work_with_linux_v4_16:
> > PASS (18.14 s)
> > >- boot_linux.py:BootLinuxAarch64.test_virt:  PASS (396.88 s)
> > >
> > >  * Do we want to set a maximum job timeout?  This way we can skip
> > >tests

[Qemu-devel] [PATCH v3 2/5] iotests.py: do not use infinite waits

[John Snow]

Cap waits to 60 seconds so that iotests can fail gracefully if something
goes wrong.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 6bcddd8870..6e17c040dc 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -521,7 +521,7 @@ class VM(qtest.QEMUQtestMachine):
 output_list += [key + '=' + obj[key]]
 return ','.join(output_list)
 
-def get_qmp_events_filtered(self, wait=True):
+def get_qmp_events_filtered(self, wait=60.0):
 result = []
 for ev in self.get_qmp_events(wait=wait):
 result.append(filter_qmp_event(ev))
@@ -539,10 +539,10 @@ class VM(qtest.QEMUQtestMachine):
 
 # Returns None on success, and an error string on failure
 def run_job(self, job, auto_finalize=True, auto_dismiss=False,
-pre_finalize=None):
+pre_finalize=None, wait=60.0):
 error = None
 while True:
-for ev in self.get_qmp_events_filtered(wait=True):
+for ev in self.get_qmp_events_filtered(wait=wait):
 if ev['event'] == 'JOB_STATUS_CHANGE':
 status = ev['data']['status']
 if status == 'aborting':
@@ -647,7 +647,7 @@ class QMPTestCase(unittest.TestCase):
 
self.assertEqual(self.vm.flatten_qmp_object(json.loads(json_filename[5:])),
  self.vm.flatten_qmp_object(reference))
 
-def cancel_and_wait(self, drive='drive0', force=False, resume=False):
+def cancel_and_wait(self, drive='drive0', force=False, resume=False, 
wait=60.0):
 '''Cancel a block job and wait for it to finish, returning the event'''
 result = self.vm.qmp('block-job-cancel', device=drive, force=force)
 self.assert_qmp(result, 'return', {})
@@ -658,7 +658,7 @@ class QMPTestCase(unittest.TestCase):
 cancelled = False
 result = None
 while not cancelled:
-for event in self.vm.get_qmp_events(wait=True):
+for event in self.vm.get_qmp_events(wait=wait):
 if event['event'] == 'BLOCK_JOB_COMPLETED' or \
event['event'] == 'BLOCK_JOB_CANCELLED':
 self.assert_qmp(event, 'data/device', drive)
@@ -671,10 +671,10 @@ class QMPTestCase(unittest.TestCase):
 self.assert_no_active_block_jobs()
 return result
 
-def wait_until_completed(self, drive='drive0', check_offset=True):
+def wait_until_completed(self, drive='drive0', check_offset=True, 
wait=60.0):
 '''Wait for a block job to finish, returning the event'''
 while True:
-for event in self.vm.get_qmp_events(wait=True):
+for event in self.vm.get_qmp_events(wait=wait):
 if event['event'] == 'BLOCK_JOB_COMPLETED':
 self.assert_qmp(event, 'data/device', drive)
 self.assert_qmp_absent(event, 'data/error')
-- 
2.20.1

[Qemu-devel] [PATCH v3 0/5] blockdev-backup: don't check aio_context too early

[John Snow]

See patch one's commit message for justification.
Patches 2-5 are for testing, because that's always how these things go.

001/5:[] [--] 'blockdev-backup: don't check aio_context too early'
002/5:[0004] [FC] 'iotests.py: do not use infinite waits'
003/5:[down]  'QEMUMachine: add events_wait method'
004/5:[0022] [FC] 'iotests.py: rewrite run_job to be pickier'
005/5:[0017] [FC] 'iotests: add iotest 250 for testing blockdev-backup
   across iothread contexts'

v3: Rebased on Max's staging branch:
Rebase patch 2
added patch 3, to add events_wait.
Rework patch 4 to make run_job consume legacy events, too
Minorly edit patch 5 due to the two above.
v2: added patch 4, with iotest framework adjustments in patches 2/3.

John Snow (5):
  blockdev-backup: don't check aio_context too early
  iotests.py: do not use infinite waits
  QEMUMachine: add events_wait method
  iotests.py: rewrite run_job to be pickier
  iotests: add iotest 250 for testing blockdev-backup across iothread
contexts

 blockdev.c|   4 --
 python/qemu/__init__.py   |  69 +--
 tests/qemu-iotests/250| 122 ++
 tests/qemu-iotests/250.out| 119 +
 tests/qemu-iotests/group  |   1 +
 tests/qemu-iotests/iotests.py |  60 ++---
 6 files changed, 326 insertions(+), 49 deletions(-)
 create mode 100755 tests/qemu-iotests/250
 create mode 100644 tests/qemu-iotests/250.out

-- 
2.20.1

[Qemu-devel] [PATCH v3 4/5] iotests.py: rewrite run_job to be pickier

[John Snow]

Don't pull events out of the queue that don't belong to us;
be choosier so that we can use this method to drive jobs that
were launched by transactions that may have more jobs.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 48 +--
 1 file changed, 29 insertions(+), 19 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 6e17c040dc..dc77d3fba0 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -540,27 +540,37 @@ class VM(qtest.QEMUQtestMachine):
 # Returns None on success, and an error string on failure
 def run_job(self, job, auto_finalize=True, auto_dismiss=False,
 pre_finalize=None, wait=60.0):
+match_device = {'data': {'device': job}}
+match_id = {'data': {'id': job}}
+events = [
+('BLOCK_JOB_COMPLETED', match_device),
+('BLOCK_JOB_CANCELLED', match_device),
+('BLOCK_JOB_ERROR', match_device),
+('BLOCK_JOB_READY', match_device),
+('BLOCK_JOB_PENDING', match_id),
+('JOB_STATUS_CHANGE', match_id)
+]
 error = None
 while True:
-for ev in self.get_qmp_events_filtered(wait=wait):
-if ev['event'] == 'JOB_STATUS_CHANGE':
-status = ev['data']['status']
-if status == 'aborting':
-result = self.qmp('query-jobs')
-for j in result['return']:
-if j['id'] == job:
-error = j['error']
-log('Job failed: %s' % (j['error']))
-elif status == 'pending' and not auto_finalize:
-if pre_finalize:
-pre_finalize()
-self.qmp_log('job-finalize', id=job)
-elif status == 'concluded' and not auto_dismiss:
-self.qmp_log('job-dismiss', id=job)
-elif status == 'null':
-return error
-else:
-log(ev)
+ev = filter_qmp_event(self.events_wait(events))
+if ev['event'] != 'JOB_STATUS_CHANGE':
+log(ev)
+continue
+status = ev['data']['status']
+if status == 'aborting':
+result = self.qmp('query-jobs')
+for j in result['return']:
+if j['id'] == job:
+error = j['error']
+log('Job failed: %s' % (j['error']))
+elif status == 'pending' and not auto_finalize:
+if pre_finalize:
+pre_finalize()
+self.qmp_log('job-finalize', id=job)
+elif status == 'concluded' and not auto_dismiss:
+self.qmp_log('job-dismiss', id=job)
+elif status == 'null':
+return error
 
 def node_info(self, node_name):
 nodes = self.qmp('query-named-block-nodes')
-- 
2.20.1

[Qemu-devel] [PATCH v3 1/5] blockdev-backup: don't check aio_context too early

[John Snow]

in blockdev_backup_prepare, we check to make sure that the target is
associated with a compatible aio context. However, do_blockdev_backup is
called later and has some logic to move the target to a compatible
aio_context. The transaction version will fail certain commands
needlessly early as a result.

Allow blockdev_backup_prepare to simply call do_blockdev_backup, which
will ultimately decide if the contexts are compatible or not.

Note: the transaction version has always disallowed this operation since
its initial commit bd8baecd (2014), whereas the version of
qmp_blockdev_backup at the time, from commit c29c1dd312f, tried to
enforce the aio_context switch instead. It's not clear, and I can't see
from the mailing list archives at the time, why the two functions take a
different approach. It wasn't until later in efd7556708b (2016) that the
standalone version tried to determine if it could set the context or
not.

Reported-by: aihua liang 
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1683498
Signed-off-by: John Snow 
---
 blockdev.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index e856ca4be9..01a48a2a5a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1872,10 +1872,6 @@ static void blockdev_backup_prepare(BlkActionState 
*common, Error **errp)
 }
 
 aio_context = bdrv_get_aio_context(bs);
-if (aio_context != bdrv_get_aio_context(target)) {
-error_setg(errp, "Backup between two IO threads is not implemented");
-return;
-}
 aio_context_acquire(aio_context);
 state->bs = bs;
 
-- 
2.20.1

Re: [Qemu-devel] [PATCH] vmstate: Add VMSTATE_OPAQUE to save/load complex data structures

[Roman Kiryanov via Qemu-devel]

Hi Dave, thank you for looking.

> Can you give me an example of where you would use it?

We use it in our host memory sharing device. I used the existing
macros for all fields I could, but unfortunately some state does not fit
into them. We use this new macro to save/load memory
allocators (for now we have malloc, but we are working on adding
Vulkan calls). For now the state looks this way:

class Allocator;
unordered_map state;

class MallocAllocator: public Allocator {
unordered_map> state;
};

class VulkanAllocator: public Allocator {
// TBD
};

>  I've been trying to get rid as many of the open-coded cases as possible

I saw this in the migration document. I used VMSTATE_INT32,
VMSTATE_STRUCT and VMSTATE_STRUCT_VARRAY_ALLOC
where I could.

> Having said that;  would it be easier to pass the get/put functions
> rather than the info ?

Sure, function pointer will be even better, but since VMStateField
takes "const VMStateInfo *", I added this way.

Regards,
Roman.