Re: [PATCH 8/8] aspeed: Add AST1030 (BIC) to fby35

[Cédric Le Goater]

On 7/4/22 23:54, Peter Delevoryas wrote:

With the BIC, the easiest way to run everything is to create two pty's
for each SoC and reserve stdin/stdout for the monitor:

 wget 
https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
 wget 
https://github.com/peterdelevoryas/OpenBIC/releases/download/oby35-cl-2022.13.01/Y35BCL.elf
 qemu-system-arm -machine fby35 \
 -drive file=fby35.mtd,format=raw,if=mtd \
 -device loader,file=fby35.mtd,addr=0,cpu-num=0 \
 -serial pty -serial pty -serial mon:stdio -display none -S

 screen /dev/ttys0
 screen /dev/ttys1
 (qemu) c

Signed-off-by: Peter Delevoryas 



Reviewed-by: Cédric Le Goater 

Thanks,

C.


---
  hw/arm/fby35.c | 27 ++-
  1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
index d3edfa3b10..031602800f 100644
--- a/hw/arm/fby35.c
+++ b/hw/arm/fby35.c
@@ -11,7 +11,9 @@
  #include "sysemu/sysemu.h"
  #include "sysemu/block-backend.h"
  #include "hw/boards.h"
+#include "hw/qdev-clock.h"
  #include "hw/arm/aspeed_soc.h"
+#include "hw/arm/boot.h"
  
  #define TYPE_FBY35 MACHINE_TYPE_NAME("fby35")

  OBJECT_DECLARE_SIMPLE_TYPE(Fby35State, FBY35);
@@ -22,8 +24,11 @@ struct Fby35State {
  MemoryRegion bmc_memory;
  MemoryRegion bmc_dram;
  MemoryRegion bmc_boot_rom;
+MemoryRegion bic_memory;
+Clock *bic_sysclk;
  
  AspeedSoCState bmc;

+AspeedSoCState bic;
  
  bool mmio_exec;

  };
@@ -110,11 +115,31 @@ static void fby35_bmc_init(Fby35State *s)
  }
  }
  
+static void fby35_bic_init(Fby35State *s)

+{
+s->bic_sysclk = clock_new(OBJECT(s), "SYSCLK");
+clock_set_hz(s->bic_sysclk, 2ULL);
+
+memory_region_init(>bic_memory, OBJECT(s), "bic-memory", UINT64_MAX);
+
+object_initialize_child(OBJECT(s), "bic", >bic, "ast1030-a1");
+qdev_connect_clock_in(DEVICE(>bic), "sysclk", s->bic_sysclk);
+object_property_set_link(OBJECT(>bic), "memory", OBJECT(>bic_memory),
+ _abort);
+aspeed_soc_uart_set_chr(>bic, ASPEED_DEV_UART5, serial_hd(1));
+qdev_realize(DEVICE(>bic), NULL, _abort);
+
+aspeed_board_init_flashes(>bic.fmc, "sst25vf032b", 2, 2);
+aspeed_board_init_flashes(>bic.spi[0], "sst25vf032b", 2, 4);
+aspeed_board_init_flashes(>bic.spi[1], "sst25vf032b", 2, 6);
+}
+
  static void fby35_init(MachineState *machine)
  {
  Fby35State *s = FBY35(machine);
  
  fby35_bmc_init(s);

+fby35_bic_init(s);
  }
  
  
@@ -141,7 +166,7 @@ static void fby35_class_init(ObjectClass *oc, void *data)

  mc->init = fby35_init;
  mc->no_floppy = 1;
  mc->no_cdrom = 1;
-mc->min_cpus = mc->max_cpus = mc->default_cpus = 2;
+mc->min_cpus = mc->max_cpus = mc->default_cpus = 3;
  
  object_class_property_add_bool(oc, "execute-in-place",

 fby35_get_mmio_exec,

Re: [PATCH 3/8] aspeed: Refactor UART init for multi-SoC machines

[Cédric Le Goater]

On 7/4/22 23:54, Peter Delevoryas wrote:

This change moves the code that connects the SoC UART's to serial_hd's
to the machine.

It makes each UART a proper child member of the SoC, and then allows the
machine to selectively initialize the chardev for each UART with a
serial_hd.

This should preserve backwards compatibility, but also allow multi-SoC
boards to completely change the wiring of serial devices from the
command line to specific SoC UART's.

This also removes the uart-default property from the SoC, since the SoC
doesn't need to know what UART is the "default" on the machine anymore.

I tested this using the images and commands from the previous
refactoring, and another test image for the ast1030:

 wget 
https://github.com/facebook/openbmc/releases/download/v2021.49.0/fuji.mtd
 wget 
https://github.com/facebook/openbmc/releases/download/v2021.49.0/wedge100.mtd
 wget 
https://github.com/peterdelevoryas/OpenBIC/releases/download/oby35-cl-2022.13.01/Y35BCL.elf

Fuji uses UART1:

 qemu-system-arm -machine fuji-bmc \
 -drive file=fuji.mtd,format=raw,if=mtd \
 -nographic

ast2600-evb uses uart-default=UART5:

 qemu-system-arm -machine ast2600-evb \
 -drive file=fuji.mtd,format=raw,if=mtd \
 -serial null -serial mon:stdio -display none

Wedge100 uses UART3:

 qemu-system-arm -machine palmetto-bmc \
 -drive file=wedge100.mtd,format=raw,if=mtd \
 -serial null -serial null -serial null \
 -serial mon:stdio -display none

AST1030 EVB uses UART5:

 qemu-system-arm -machine ast1030-evb \
 -kernel Y35BCL.elf -nographic

Fixes: 6827ff20b2975 ("hw: aspeed: Init all UART's with serial devices")
Signed-off-by: Peter Delevoryas 



Reviewed-by: Cédric Le Goater 

Thanks,

C.



---
  hw/arm/aspeed.c | 22 +---
  hw/arm/aspeed_ast10x0.c |  8 +-
  hw/arm/aspeed_ast2600.c |  8 +-
  hw/arm/aspeed_soc.c | 50 +
  include/hw/arm/aspeed_soc.h |  7 --
  5 files changed, 71 insertions(+), 24 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index bee8a748ec..cc395f988c 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -26,6 +26,7 @@
  #include "qemu/error-report.h"
  #include "qemu/units.h"
  #include "hw/qdev-clock.h"
+#include "sysemu/sysemu.h"
  
  static struct arm_boot_info aspeed_board_binfo = {

  .board_id = -1, /* device-tree-only board */
@@ -301,6 +302,21 @@ static void sdhci_attach_drive(SDHCIState *sdhci, 
DriveInfo *dinfo)
 _fatal);
  }
  
+static void connect_serial_hds_to_uarts(AspeedMachineState *bmc)

+{
+AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(bmc);
+AspeedSoCState *s = >soc;
+AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
+
+aspeed_soc_uart_set_chr(s, amc->uart_default, serial_hd(0));
+for (int i = 1, uart = ASPEED_DEV_UART1; i < sc->uarts_num; i++, uart++) {
+if (uart == amc->uart_default) {
+continue;
+}
+aspeed_soc_uart_set_chr(s, uart, serial_hd(i));
+}
+}
+
  static void aspeed_machine_init(MachineState *machine)
  {
  AspeedMachineState *bmc = ASPEED_MACHINE(machine);
@@ -346,8 +362,7 @@ static void aspeed_machine_init(MachineState *machine)
  object_property_set_int(OBJECT(>soc), "hw-prot-key",
  ASPEED_SCU_PROT_KEY, _abort);
  }
-qdev_prop_set_uint32(DEVICE(>soc), "uart-default",
- amc->uart_default);
+connect_serial_hds_to_uarts(bmc);
  qdev_realize(DEVICE(>soc), NULL, _abort);
  
  aspeed_board_init_flashes(>soc.fmc,

@@ -1372,8 +1387,7 @@ static void aspeed_minibmc_machine_init(MachineState 
*machine)
  
  object_property_set_link(OBJECT(>soc), "memory",

   OBJECT(get_system_memory()), _abort);
-qdev_prop_set_uint32(DEVICE(>soc), "uart-default",
- amc->uart_default);
+connect_serial_hds_to_uarts(bmc);
  qdev_realize(DEVICE(>soc), NULL, _abort);
  
  aspeed_board_init_flashes(>soc.fmc,

diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index 677699e54c..4d0b9b115f 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -144,6 +144,10 @@ static void aspeed_soc_ast1030_init(Object *obj)
  object_initialize_child(obj, "wdt[*]", >wdt[i], typename);
  }
  
+for (i = 0; i < sc->uarts_num; i++) {

+object_initialize_child(obj, "uart[*]", >uart[i], TYPE_SERIAL_MM);
+}
+
  snprintf(typename, sizeof(typename), "aspeed.gpio-%s", socname);
  object_initialize_child(obj, "gpio", >gpio, typename);
  
@@ -255,7 +259,9 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)

  sc->irqmap[ASPEED_DEV_KCS] + 
aspeed_lpc_kcs_4));
  
  /* UART */

-aspeed_soc_uart_init(s);
+if (!aspeed_soc_uart_realize(s, errp)) {
+return;
+   

Re: [PATCH 6/8] aspeed: Add AST2600 (BMC) to fby35

[Cédric Le Goater]

On 7/4/22 23:54, Peter Delevoryas wrote:

You can test booting the BMC with both '-device loader' and '-drive
file'. This is necessary because of how the fb-openbmc boot sequence
works (jump to 0x2000 after U-Boot SPL).

 wget 
https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
 qemu-system-arm -machine fby35 -nographic \
 -device loader,file=fby35.mtd,addr=0,cpu-num=0 -drive 
file=fby35.mtd,format=raw,if=mtd

Signed-off-by: Peter Delevoryas 


Reviewed-by: Cédric Le Goater 

Thanks,

C.



---
  hw/arm/fby35.c | 41 +
  1 file changed, 41 insertions(+)

diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
index 03b458584c..5c5224d374 100644
--- a/hw/arm/fby35.c
+++ b/hw/arm/fby35.c
@@ -6,17 +6,55 @@
   */
  
  #include "qemu/osdep.h"

+#include "qemu/units.h"
+#include "qapi/error.h"
+#include "sysemu/sysemu.h"
  #include "hw/boards.h"
+#include "hw/arm/aspeed_soc.h"
  
  #define TYPE_FBY35 MACHINE_TYPE_NAME("fby35")

  OBJECT_DECLARE_SIMPLE_TYPE(Fby35State, FBY35);
  
  struct Fby35State {

  MachineState parent_obj;
+
+MemoryRegion bmc_memory;
+MemoryRegion bmc_dram;
+MemoryRegion bmc_boot_rom;
+
+AspeedSoCState bmc;
  };
  
+#define FBY35_BMC_RAM_SIZE (2 * GiB)

+
+static void fby35_bmc_init(Fby35State *s)
+{
+memory_region_init(>bmc_memory, OBJECT(s), "bmc-memory", UINT64_MAX);
+memory_region_init_ram(>bmc_dram, OBJECT(s), "bmc-dram",
+   FBY35_BMC_RAM_SIZE, _abort);
+
+object_initialize_child(OBJECT(s), "bmc", >bmc, "ast2600-a3");
+object_property_set_int(OBJECT(>bmc), "ram-size", FBY35_BMC_RAM_SIZE,
+_abort);
+object_property_set_link(OBJECT(>bmc), "memory", OBJECT(>bmc_memory),
+ _abort);
+object_property_set_link(OBJECT(>bmc), "dram", OBJECT(>bmc_dram),
+ _abort);
+object_property_set_int(OBJECT(>bmc), "hw-strap1", 0x00C0,
+_abort);
+object_property_set_int(OBJECT(>bmc), "hw-strap2", 0x0003,
+_abort);
+aspeed_soc_uart_set_chr(>bmc, ASPEED_DEV_UART5, serial_hd(0));
+qdev_realize(DEVICE(>bmc), NULL, _abort);
+
+aspeed_board_init_flashes(>bmc.fmc, "n25q00", 2, 0);
+}
+
  static void fby35_init(MachineState *machine)
  {
+Fby35State *s = FBY35(machine);
+
+fby35_bmc_init(s);
  }
  
  static void fby35_class_init(ObjectClass *oc, void *data)

@@ -25,6 +63,9 @@ static void fby35_class_init(ObjectClass *oc, void *data)
  
  mc->desc = "Meta Platforms fby35";

  mc->init = fby35_init;
+mc->no_floppy = 1;
+mc->no_cdrom = 1;
+mc->min_cpus = mc->max_cpus = mc->default_cpus = 2;
  }
  
  static const TypeInfo fby35_types[] = {

Re: [PATCH 4/8] aspeed: Make aspeed_board_init_flashes public

[Cédric Le Goater]

On 7/4/22 23:54, Peter Delevoryas wrote:

Signed-off-by: Peter Delevoryas 


Reviewed-by: Cédric Le Goater 

Thanks,

C.



---
  hw/arm/aspeed.c | 2 +-
  include/hw/arm/aspeed_soc.h | 2 ++
  2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index cc395f988c..b2486a9e78 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -262,7 +262,7 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, 
size_t rom_size,
  rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
  }
  
-static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,

+void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
unsigned int count, int unit0)
  {
  int i;
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 68e907cd64..8389200b2d 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -184,5 +184,7 @@ void aspeed_mmio_map(AspeedSoCState *s, SysBusDevice *dev, 
int n, hwaddr addr);
  void aspeed_mmio_map_unimplemented(AspeedSoCState *s, SysBusDevice *dev,
 const char *name, hwaddr addr,
 uint64_t size);
+void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
+   unsigned int count, int unit0);
  
  #endif /* ASPEED_SOC_H */

Re: [PATCH 2/8] aspeed: Create SRAM name from first CPU index

[Cédric Le Goater]

On 7/4/22 23:54, Peter Delevoryas wrote:

To support multiple SoC's running simultaneously, we need a unique name for
each RAM region. DRAM is created by the machine, but SRAM is created by the
SoC, since in hardware it is part of the SoC's internals.

We need a way to uniquely identify each SRAM region though, for VM
migration. Since each of the SoC's CPU's has an index which identifies it
uniquely from other CPU's in the machine, we can use the index of any of the
CPU's in the SoC to uniquely identify differentiate the SRAM name from other
SoC SRAM's. In this change, I just elected to use the index of the first CPU
in each SoC.

Signed-off-by: Peter Delevoryas 


Reviewed-by: Cédric Le Goater 

Thanks,

C.


---
  hw/arm/aspeed_ast10x0.c | 5 -
  hw/arm/aspeed_ast2600.c | 5 +++--
  hw/arm/aspeed_soc.c | 5 +++--
  3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index 33ef331771..677699e54c 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -159,6 +159,7 @@ static void aspeed_soc_ast1030_realize(DeviceState 
*dev_soc, Error **errp)
  DeviceState *armv7m;
  Error *err = NULL;
  int i;
+g_autofree char *sram_name = NULL;
  
  if (!clock_has_source(s->sysclk)) {

  error_setg(errp, "sysclk clock must be wired up by the board code");
@@ -183,7 +184,9 @@ static void aspeed_soc_ast1030_realize(DeviceState 
*dev_soc, Error **errp)
  sysbus_realize(SYS_BUS_DEVICE(>armv7m), _abort);
  
  /* Internal SRAM */

-memory_region_init_ram(>sram, NULL, "aspeed.sram", sc->sram_size, );
+sram_name = g_strdup_printf("aspeed.sram.%d",
+CPU(s->armv7m.cpu)->cpu_index);
+memory_region_init_ram(>sram, OBJECT(s), sram_name, sc->sram_size, 
);
  if (err != NULL) {
  error_propagate(errp, err);
  return;
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 3f0611ac11..64eb5a7b26 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -276,6 +276,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
  AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
  Error *err = NULL;
  qemu_irq irq;
+g_autofree char *sram_name = NULL;
  
  /* IO space */

  aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(>iomem), "aspeed.io",
@@ -335,8 +336,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
  }
  
  /* SRAM */

-memory_region_init_ram(>sram, OBJECT(dev), "aspeed.sram",
-   sc->sram_size, );
+sram_name = g_strdup_printf("aspeed.sram.%d", CPU(>cpu[0])->cpu_index);
+memory_region_init_ram(>sram, OBJECT(s), sram_name, sc->sram_size, 
);
  if (err) {
  error_propagate(errp, err);
  return;
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 0f675e7fcd..0bb6a2f092 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -239,6 +239,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error 
**errp)
  AspeedSoCState *s = ASPEED_SOC(dev);
  AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
  Error *err = NULL;
+g_autofree char *sram_name = NULL;
  
  /* IO space */

  aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(>iomem), "aspeed.io",
@@ -259,8 +260,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error 
**errp)
  }
  
  /* SRAM */

-memory_region_init_ram(>sram, OBJECT(dev), "aspeed.sram",
-   sc->sram_size, );
+sram_name = g_strdup_printf("aspeed.sram.%d", CPU(>cpu[0])->cpu_index);
+memory_region_init_ram(>sram, OBJECT(s), sram_name, sc->sram_size, 
);
  if (err) {
  error_propagate(errp, err);
  return;

Re: [PATCH 1/8] hw/i2c/pca954x: Add method to get channels

[Cédric Le Goater]

On 7/4/22 23:54, Peter Delevoryas wrote:

I added this helper in the Aspeed machine file a while ago to help
initialize fuji-bmc i2c devices. This moves it to the official pca954x
file so that other files can use it.

This does something very similar to pca954x_i2c_get_bus, but I think
this is useful when you have a very complicated dts with a lot of
switches, like the fuji dts.

This convenience method lets you write code that produces a flat array
of I2C buses that matches the naming in the dts. After that you can just
add individual sensors using the flat array of I2C buses.

See fuji_bmc_i2c_init to understand this point further.

The fuji dts is here for reference:

https://github.com/torvalds/linux/blob/40cb6373b46/arch/arm/boot/dts/aspeed-bmc-facebook-fuji.dts

Signed-off-by: Peter Delevoryas 


Reviewed-by: Cédric Le Goater 

Thanks,

C.



---
  hw/arm/aspeed.c  | 29 +
  hw/i2c/i2c_mux_pca954x.c | 10 ++
  include/hw/i2c/i2c_mux_pca954x.h | 13 +
  3 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 6fe9b13548..bee8a748ec 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -793,15 +793,6 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
  create_pca9552(soc, 15, 0x60);
  }
  
-static void get_pca9548_channels(I2CBus *bus, uint8_t mux_addr,

- I2CBus **channels)
-{
-I2CSlave *mux = i2c_slave_create_simple(bus, "pca9548", mux_addr);
-for (int i = 0; i < 8; i++) {
-channels[i] = pca954x_i2c_get_bus(mux, i);
-}
-}
-
  #define TYPE_LM75 TYPE_TMP105
  #define TYPE_TMP75 TYPE_TMP105
  #define TYPE_TMP422 "tmp422"
@@ -814,20 +805,18 @@ static void fuji_bmc_i2c_init(AspeedMachineState *bmc)
  for (int i = 0; i < 16; i++) {
  i2c[i] = aspeed_i2c_get_bus(>i2c, i);
  }
-I2CBus *i2c180 = i2c[2];
-I2CBus *i2c480 = i2c[8];
-I2CBus *i2c600 = i2c[11];
  
-get_pca9548_channels(i2c180, 0x70, [16]);

-get_pca9548_channels(i2c480, 0x70, [24]);
+pca954x_i2c_get_channels(i2c[2], 0x70, "pca9548", [16]);
+pca954x_i2c_get_channels(i2c[8], 0x70, "pca9548", [24]);
  /* NOTE: The device tree skips [32, 40) in the alias numbering */
-get_pca9548_channels(i2c600, 0x77, [40]);
-get_pca9548_channels(i2c[24], 0x71, [48]);
-get_pca9548_channels(i2c[25], 0x72, [56]);
-get_pca9548_channels(i2c[26], 0x76, [64]);
-get_pca9548_channels(i2c[27], 0x76, [72]);
+pca954x_i2c_get_channels(i2c[11], 0x77, "pca9548", [40]);
+pca954x_i2c_get_channels(i2c[24], 0x71, "pca9548", [48]);
+pca954x_i2c_get_channels(i2c[25], 0x72, "pca9548", [56]);
+pca954x_i2c_get_channels(i2c[26], 0x76, "pca9548", [64]);
+pca954x_i2c_get_channels(i2c[27], 0x76, "pca9548", [72]);
  for (int i = 0; i < 8; i++) {
-get_pca9548_channels(i2c[40 + i], 0x76, [80 + i * 8]);
+pca954x_i2c_get_channels(i2c[40 + i], 0x76, "pca9548",
+ [80 + i * 8]);
  }
  
  i2c_slave_create_simple(i2c[17], TYPE_LM75, 0x4c);

diff --git a/hw/i2c/i2c_mux_pca954x.c b/hw/i2c/i2c_mux_pca954x.c
index 3945de795c..6b07804546 100644
--- a/hw/i2c/i2c_mux_pca954x.c
+++ b/hw/i2c/i2c_mux_pca954x.c
@@ -169,6 +169,16 @@ I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel)
  return pca954x->bus[channel];
  }
  
+void pca954x_i2c_get_channels(I2CBus *bus, uint8_t address,

+  const char *type_name, I2CBus **channels)
+{
+I2CSlave *mux = i2c_slave_create_simple(bus, type_name, address);
+Pca954xClass *pc = PCA954X_GET_CLASS(mux);
+Pca954xState *pca954x = PCA954X(mux);
+
+memcpy(channels, pca954x->bus, pc->nchans * sizeof(channels[0]));
+}
+
  static void pca9546_class_init(ObjectClass *klass, void *data)
  {
  Pca954xClass *s = PCA954X_CLASS(klass);
diff --git a/include/hw/i2c/i2c_mux_pca954x.h b/include/hw/i2c/i2c_mux_pca954x.h
index 3dd25ec983..3a676a30a9 100644
--- a/include/hw/i2c/i2c_mux_pca954x.h
+++ b/include/hw/i2c/i2c_mux_pca954x.h
@@ -16,4 +16,17 @@
   */
  I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel);
  
+/**

+ * Creates an i2c mux and retrieves all of the channels associated with it.
+ *
+ * @bus: the i2c bus where the i2c mux resides.
+ * @address: the address of the i2c mux on the aforementioned i2c bus.
+ * @type_name: name of the i2c mux type to create.
+ * @channels: an output parameter specifying where to return the channels.
+ *
+ * Returns: None
+ */
+void pca954x_i2c_get_channels(I2CBus *bus, uint8_t address,
+  const char *type_name, I2CBus **channels);
+
  #endif

Re: [PATCH v9 0/2] QEMU RISC-V nested virtualization fixes

[Alistair Francis]

On Thu, Jun 30, 2022 at 4:27 PM Anup Patel  wrote:
>
> On Thu, Jun 30, 2022 at 11:42 AM Anup Patel  wrote:
> >
> > This series does fixes and improvements to have nested virtualization
> > on QEMU RISC-V.
> >
> > These patches can also be found in riscv_nested_fixes_v9 branch at:
> > https://github.com/avpatel/qemu.git
> >
> > The RISC-V nested virtualization was tested on QEMU RISC-V using
> > Xvisor RISC-V which has required hypervisor support to run another
> > hypervisor as Guest/VM.
>
> Changes since v8:
>  - Drop first two patches because Alistair has already taken care of it.
>  - Include instruction immediate offset in "Addr. Offset" for PATCH1
>
> Regards,
> Anup
>
> >
> > Changes since v7:
> >  - Improve tinst "Addr. Offset" in PATCH3
> >
> > Changes since v6:
> >  - Droppred original PATCH1 and PATCH2 since these are already merged
> >  - Added PATCH1 to revert dummy mcountinhibit CSR
> >  - Added PATCH2 to fix minimum priv spec version for mcountinhibit CSR
> >  - Fixed checkpatch error in PATCH3
> >  - Fixed compile error in PATCH4
> >
> > Changes since v5:
> >  - Correctly set "Addr. Offset" for misaligned load/store traps in PATCH3
> >  - Use offsetof() instead of pointer in PATCH4
> >
> > Changes since v4:
> >  - Updated commit description in PATCH1, PATCH2, and PATCH4
> >  - Use "const" for local array in PATCH5
> >
> > Changes since v3:
> >  - Updated PATCH3 to set special pseudoinstructions in htinst for
> >guest page faults which result due to VS-stage page table walks
> >  - Updated warning message in PATCH4
> >
> > Changes since v2:
> >  - Dropped the patch which are already in Alistair's next branch
> >  - Set "Addr. Offset" in the transformed instruction for PATCH3
> >  - Print warning in riscv_cpu_realize() if we are disabling an
> >extension due to privilege spec verions mismatch for PATCH4
> >
> > Changes since v1:
> >  - Set write_gva to env->two_stage_lookup which ensures that for
> >HS-mode to HS-mode trap write_gva is true only for HLV/HSV
> >instructions
> >  - Included "[PATCH 0/3] QEMU RISC-V priv spec version fixes"
> >patches in this series for easy review
> >  - Re-worked PATCH7 to force disable extensions if required
> >priv spec version is not staisfied
> >  - Added new PATCH8 to fix "aia=aplic-imsic" mode of virt machine
> >
> > Anup Patel (2):
> >   target/riscv: Update [m|h]tinst CSR in riscv_cpu_do_interrupt()
> >   target/riscv: Force disable extensions if priv spec version does not
> > match
> >
> >  target/riscv/cpu.c| 150 ++-
> >  target/riscv/cpu.h|   5 +
> >  target/riscv/cpu_helper.c | 252 +-
> >  target/riscv/instmap.h|  45 +++
> >  4 files changed, 390 insertions(+), 62 deletions(-)

Thanks!

Applied to riscv-to-apply.next

Alistair

> >
> > --
> > 2.34.1
> >
>

Re: [PATCH v3 2/3] Add dirty-sync-missed-zero-copy migration stat

[Markus Armbruster]

Leonardo Bras  writes:

> Signed-off-by: Leonardo Bras 

Acked-by: Markus Armbruster 

Re: [PATCH v2 2/3] Add zero-copy-copied migration stat

[Markus Armbruster]

Leonardo Brás  writes:

> Thanks Daniel, Markus and Dave for the feedback!
>
> On Mon, 2022-07-04 at 14:14 +0100, Daniel P. Berrangé wrote:
>> On Mon, Jul 04, 2022 at 02:59:50PM +0200, Markus Armbruster wrote:
>> > Daniel P. Berrangé  writes:
>> > 
>> > > On Mon, Jul 04, 2022 at 02:04:41PM +0200, Markus Armbruster wrote:
>> > 
> [...]
>
>> 
>> since 7.1, unless you're planning for really tortuous review :)
>> 
>
> Ok, updated :)
>
>> > [...]
>> > 
>> > > > Please rephrase the documentation of @zero-copy-copied in terms of
>> > > > @dirty-sync-count.  Here's my attempt.
>> > > > 
>> > > > # @zero-copy-copied: Number of times dirty RAM synchronization could 
>> > > > not
>> > > > #    avoid copying zero pages.  This is between 0 and
>> > > > #    @dirty-sync-count.  (since 7.1)
>
> That's a great description! And it's almost 100% correct.

That's why we do patch review :)

> IIUC dirty-sync-count increments on migration_bitmap_sync() once after each 
> full
> dirty-bitmap scan, and even with multifd syncing at the same point, it could
> potentially have a increment per multifd channel.
>
> The only change would be having:
> # This is between 0 and @dirty-sync-count * @multifd-channel.

Makes sense to me.

[...]

Re: [PATCH] include/qemu/host-utils: Remove unused code in the *_overflow wrappers

[Richard Henderson]

On 7/1/22 08:21, Thomas Huth wrote:

According to commit cec07c0b612975 the code in the #else paths was required
for GCC < 5.0 and Clang < 3.8. We don't support such old compilers
at all anymore, so we can remove these lines now. We keep the wrapper
function, though, since they are easier to read and help to make sure that
the parameters have the right types.

Signed-off-by: Thomas Huth
---
  include/qemu/host-utils.h | 65 ---
  1 file changed, 65 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v4 37/45] linux-user/aarch64: Do not allow duplicate or short sve records

[Richard Henderson]

On 7/5/22 09:00, Richard Henderson wrote:

On 7/4/22 17:38, Peter Maydell wrote:

  case TARGET_SVE_MAGIC:
+    if (sve || size < sizeof(struct target_sve_context)) {
+    goto err;
+    }
  if (cpu_isar_feature(aa64_sve, env_archcpu(env))) {
  vq = sve_vq(env);
  sve_size = QEMU_ALIGN_UP(TARGET_SVE_SIG_CONTEXT_SIZE(vq), 16);
-    if (!sve && size == sve_size) {
+    if (size == sve_size) {
  sve = (struct target_sve_context *)ctx;
  break;
  }


On the other hand, the kernel seems to happily allow records
which are larger than the SVE_SIG_CONTEXT_SIZE, whereas we
ignore the record unless there's an exact size match.


Yeah, this gets fixed properly in patch 39.
Perhaps I should simply squash this with that?


Bah!  No, those are two separate checks: the minimum size to contain vq and flags 
(target_sve_context) and the minimum size to contain all of the vector data 
(TARGET_SVE_SIG_CONTEXT_SIZE).


The latter *is* fixed in patch 39, but this one should stay as-is.


r~

Re: [PATCH v4 37/45] linux-user/aarch64: Do not allow duplicate or short sve records

[Richard Henderson]

On 7/4/22 17:38, Peter Maydell wrote:

  case TARGET_SVE_MAGIC:
+if (sve || size < sizeof(struct target_sve_context)) {
+goto err;
+}
  if (cpu_isar_feature(aa64_sve, env_archcpu(env))) {
  vq = sve_vq(env);
  sve_size = QEMU_ALIGN_UP(TARGET_SVE_SIG_CONTEXT_SIZE(vq), 16);
-if (!sve && size == sve_size) {
+if (size == sve_size) {
  sve = (struct target_sve_context *)ctx;
  break;
  }


On the other hand, the kernel seems to happily allow records
which are larger than the SVE_SIG_CONTEXT_SIZE, whereas we
ignore the record unless there's an exact size match.


Yeah, this gets fixed properly in patch 39.
Perhaps I should simply squash this with that?


r~

Re: [PATCH v4 37/45] linux-user/aarch64: Do not allow duplicate or short sve records

[Richard Henderson]

On 7/4/22 17:38, Peter Maydell wrote:

I notice the kernel has a bunch of signal frame test
cases in  tools/testing/selftests/arm64/signal/testcases --
do we pass those ?


Most but not all of them.  The ones we don't pass are those for which SVE state has been 
discarded across a syscall and so the signal frame record is expected to be missing.  I 
thought about fixing those, but decided not to do within this series.



r~

Re: [PATCH v4 35/45] linux-user/aarch64: Add SM bit to SVE signal context

[Richard Henderson]

On 7/4/22 17:32, Peter Maydell wrote:

@@ -177,9 +180,13 @@ static void target_setup_sve_record(struct 
target_sve_context *sve,
  {
  int i, j;

+memset(sve, 0, sizeof(*sve));
  __put_user(TARGET_SVE_MAGIC, >head.magic);
  __put_user(size, >head.size);
  __put_user(vq * TARGET_SVE_VQ_BYTES, >vl);
+if (FIELD_EX64(env->svcr, SVCR, SM)) {
+__put_user(TARGET_SVE_SIG_FLAG_SM, >flags);
+}



The kernel documentation says that if this is set then the SVE
record contains the streaming vector length. Does that happen
automatically (ie vq is the right thing for both streaming
and non-streaming) or do we need to do something there?


It is automatically correct (modulo the typo you found in patch 40).  The two helpers we 
have are for VL and SVL, with no direct access to NVL.



I gather that the other half of handling this bit (allowing
it to be changed on signal-return) is in a later patch.


Yes.


r~

Re: [PATCH v4 23/45] target/arm: Implement SME ADDHA, ADDVA

[Richard Henderson]

On 7/4/22 16:20, Peter Maydell wrote:

+void HELPER(sme_addha_d)(void *vzda, void *vzn, void *vpn,
+ void *vpm, uint32_t desc)
+{
+intptr_t row, col, oprsz = simd_oprsz(desc) / 8;
+uint8_t *pn = vpn, *pm = vpm;
+uint64_t *zda = vzda, *zn = vzn;
+
+for (row = 0; row < oprsz; ++row) {
+if (pn[H1(row)] & 1) {
+for (col = 0; col < oprsz; ++col) {
+if (pm[H1(col)] & 1) {
+zda[row * sizeof(ARMVectorReg) + col] += zn[col];
+}
+}
+}
+}
+}


These array index calculations look wrong again?
Should be 'row * (sizeof(ARMVectorReg) / 8) + col' or equivalent,
I think.


The tile rows are N physical rows apart.


r~

Re: [PATCH v4 20/45] target/arm: Implement SME LD1, ST1

[Richard Henderson]

On 7/4/22 16:09, Peter Maydell wrote:

+static void clear_vertical_h(void *vptr, size_t off, size_t len)
+{
+uint16_t *ptr = vptr;
+size_t i;
+
+for (i = 0; i < len / 2; ++i) {
+ptr[(i + off) * sizeof(ARMVectorReg)] = 0;
+}


The code for the bigger-than-byte vertical actions seems wrong:
because 'ptr' is a uint16_t here this expression is mixing
byte offsets (off, the multiply by sizeof(ARMVectorReg) with
halfword offsets (i, the fact we're calculating an index value
for a uint16_t array).


I agree these are wrong, because they mix 'i' as index and 'off' as byte offset.  I think 
the correct addressing is always the same as byte addressing.  I.e.


for (i = 0; i < len; i += N) {
uintN_t *ptr = vptr + (i + off) * sizeof(ARMVectorReg);
*ptr = 0;
}

so that every iteration advances N rows and writes N bytes.


+static void copy_vertical_h(void *vdst, const void *vsrc, size_t len)
+{
+const uint16_t *src = vsrc;
+uint16_t *dst = vdst;
+size_t i;
+
+for (i = 0; i < len / 2; ++i) {
+dst[i * sizeof(ARMVectorReg)] = src[i];


Similarly the array index calculation for dst[] here looks wrong.


I don't think so in this case.  I'm not mixing indexes and byte offsets like I 
was above.

Recall that the next vertical tile element is not in the next physical row, but in the Nth 
physical row.  Therefore there are always sizeof(ARMVectorReg) elements in the host layout 
between vertical tile elements.


I agree it looks strange.


+#define DO_LD(NAME, TYPE, HOST, TLB)\
+static inline void sme_##NAME##_v_host(void *za, intptr_t off, void *host)  \
+{   \
+TYPE val = HOST(host);  \
+*(TYPE *)(za + off * sizeof(ARMVectorReg)) = val;   \
+}   \
+static inline void sme_##NAME##_v_tlb(CPUARMState *env, void *za,   \
+intptr_t off, target_ulong addr, uintptr_t ra)  \
+{   \
+TYPE val = TLB(env, useronly_clean_ptr(addr), ra);  \
+*(TYPE *)(za + off * sizeof(ARMVectorReg)) = val;   \
+}


So in these functions is 'za' pre-adjusted to the start address of the
vertical column?


Yes.  That's true of all of these routines, and what I compute in 
get_tile_colrow.

Is 'off' a byte offset here 


Yes.


(in which case the arithmetic is wrong for anything except byte columns)


I don't think so in this case.  This is all byte arithmetic.  Just as for copy_vertical_*, 
there are N rows between elements.


Consider a vertical tile slice of uint64_t:

  Element 0 is off=0 -> za + row 0.
  Element 1 is off=8 -> za + row 8.


+tcg_gen_shli_i64(addr, cpu_reg(s, a->rm), a->esz);
+tcg_gen_add_i64(addr, addr, cpu_reg_sp(s, a->rn));


Theoretically we ought to call gen_check_sp_alignment() here
for rn == 31, but I guess we didn't do that for any of the
non-base-A64 instructions like SVE either.


Oh yeah.  Some day we should make gen_check_sp_alignment do something too.


r~

Re: [PATCH v10 08/12] target/riscv: Add sscofpmf extension support

[Weiwei Li]

在 2022/6/21 上午7:15, Atish Patra 写道:

The Sscofpmf ('Ss' for Privileged arch and Supervisor-level extensions,
and 'cofpmf' for Count OverFlow and Privilege Mode Filtering)
extension allows the perf to handle overflow interrupts and filtering
support. This patch provides a framework for programmable
counters to leverage the extension. As the extension doesn't have any
provision for the overflow bit for fixed counters, the fixed events
can also be monitoring using programmable counters. The underlying
counters for cycle and instruction counters are always running. Thus,
a separate timer device is programmed to handle the overflow.

Signed-off-by: Atish Patra 
Signed-off-by: Atish Patra 
---
  target/riscv/cpu.c  |  11 ++
  target/riscv/cpu.h  |  25 +++
  target/riscv/cpu_bits.h |  55 +++
  target/riscv/csr.c  | 165 ++-
  target/riscv/machine.c  |   1 +
  target/riscv/pmu.c  | 357 +++-
  target/riscv/pmu.h  |   7 +
  7 files changed, 610 insertions(+), 11 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index d12c6dc630ca..7d9e2aca12a9 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -22,6 +22,7 @@
  #include "qemu/ctype.h"
  #include "qemu/log.h"
  #include "cpu.h"
+#include "pmu.h"
  #include "internals.h"
  #include "exec/exec-all.h"
  #include "qapi/error.h"
@@ -775,6 +776,15 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
  set_misa(env, env->misa_mxl, ext);
  }
  
+#ifndef CONFIG_USER_ONLY

+if (cpu->cfg.pmu_num) {
+if (!riscv_pmu_init(cpu, cpu->cfg.pmu_num) && cpu->cfg.ext_sscofpmf) {
+cpu->pmu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
+  riscv_pmu_timer_cb, cpu);
+}
+ }
+#endif
+
  riscv_cpu_register_gdb_regs_for_features(cs);
  
  qemu_init_vcpu(cs);

@@ -879,6 +889,7 @@ static Property riscv_cpu_extensions[] = {
  DEFINE_PROP_BOOL("v", RISCVCPU, cfg.ext_v, false),
  DEFINE_PROP_BOOL("h", RISCVCPU, cfg.ext_h, true),
  DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),
+DEFINE_PROP_BOOL("sscofpmf", RISCVCPU, cfg.ext_sscofpmf, false),
  DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
  DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
  DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 5c7acc055ac9..db193c3d 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -137,6 +137,8 @@ typedef struct PMUCTRState {
  /* Snapshort value of a counter in RV32 */
  target_ulong mhpmcounterh_prev;
  bool started;
+/* Value beyond UINT32_MAX/UINT64_MAX before overflow interrupt trigger */
+target_ulong irq_overflow_left;
  } PMUCTRState;
  
  struct CPUArchState {

@@ -297,6 +299,9 @@ struct CPUArchState {
  /* PMU event selector configured values. First three are unused*/
  target_ulong mhpmevent_val[RV_MAX_MHPMEVENTS];
  
+/* PMU event selector configured values for RV32*/

+target_ulong mhpmeventh_val[RV_MAX_MHPMEVENTS];
+
  target_ulong sscratch;
  target_ulong mscratch;
  
@@ -433,6 +438,7 @@ struct RISCVCPUConfig {

  bool ext_zve32f;
  bool ext_zve64f;
  bool ext_zmmul;
+bool ext_sscofpmf;
  bool rvv_ta_all_1s;
  
  uint32_t mvendorid;

@@ -479,6 +485,12 @@ struct ArchCPU {
  
  /* Configuration Settings */

  RISCVCPUConfig cfg;
+
+QEMUTimer *pmu_timer;
+/* A bitmask of Available programmable counters */
+uint32_t pmu_avail_ctrs;
+/* Mapping of events to counters */
+GHashTable *pmu_event_ctr_map;
  };
  
  static inline int riscv_has_ext(CPURISCVState *env, target_ulong ext)

@@ -738,6 +750,19 @@ enum {
  CSR_TABLE_SIZE = 0x1000
  };
  
+/**

+ * The event id are encoded based on the encoding specified in the
+ * SBI specification v0.3
+ */
+
+enum riscv_pmu_event_idx {
+RISCV_PMU_EVENT_HW_CPU_CYCLES = 0x01,
+RISCV_PMU_EVENT_HW_INSTRUCTIONS = 0x02,
+RISCV_PMU_EVENT_CACHE_DTLB_READ_MISS = 0x10019,
+RISCV_PMU_EVENT_CACHE_DTLB_WRITE_MISS = 0x1001B,
+RISCV_PMU_EVENT_CACHE_ITLB_PREFETCH_MISS = 0x10021,
+};
+
  /* CSR function table */
  extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
  
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h

index b3f7fa713000..d94abefdaa0f 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -400,6 +400,37 @@
  #define CSR_MHPMEVENT29 0x33d
  #define CSR_MHPMEVENT30 0x33e
  #define CSR_MHPMEVENT31 0x33f
+
+#define CSR_MHPMEVENT3H 0x723
+#define CSR_MHPMEVENT4H 0x724
+#define CSR_MHPMEVENT5H 0x725
+#define CSR_MHPMEVENT6H 0x726
+#define CSR_MHPMEVENT7H 0x727
+#define CSR_MHPMEVENT8H 0x728
+#define CSR_MHPMEVENT9H 0x729
+#define CSR_MHPMEVENT10H0x72a
+#define CSR_MHPMEVENT11H0x72b
+#define CSR_MHPMEVENT12H0x72c
+#define CSR_MHPMEVENT13H

Re: [PATCH] hw/riscv: virt: pass random seed to fdt

[Jason A. Donenfeld]

Hi Alistair,

On Wed, Jun 29, 2022 at 4:09 AM Alistair Francis  wrote:
> I have a Linux 5.8 test case that is failing due to this patch.

Before I started fixing things in random.c, there were a lot of early
boot bugs with the RNG in Linux. I backported the fixes for these to
all stable kernels. It's a bummer that risc-v got hit by these bugs,
but I think that's just the way things go unfortunately.

Jason

[Qemu] how to use viriofs in qemu without NUMA

[Zhao, Shirley]

Hi, all,

I want to use virtiofs to share folder between host and guest.
>From the guide, it must set the NUMA node. 
>https://virtio-fs.gitlab.io/howto-qemu.html

But my guest doesn't support NUMA.

Is there any guide to use qemu + virtiofs without NUMA?
Or does qemu have any plan to support it?

Thanks.

  *   Shirley

Re: [PATCH 02/62] target/arm: Enable PageEntryExtra

[Richard Henderson]

On 7/4/22 20:52, Peter Maydell wrote:

On Sun, 3 Jul 2022 at 09:25, Richard Henderson
 wrote:


Copy attrs, sharability, and the NS bit into the TLB.

Signed-off-by: Richard Henderson 
---
  target/arm/cpu-param.h  |  8 
  target/arm/internals.h  |  5 +
  target/arm/tlb_helper.c | 14 --
  3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h
index 68ffb12427..a14f167d11 100644
--- a/target/arm/cpu-param.h
+++ b/target/arm/cpu-param.h
@@ -30,6 +30,14 @@
   */
  # define TARGET_PAGE_BITS_VARY
  # define TARGET_PAGE_BITS_MIN  10
+/*
+ * Extra information stored in softmmu page tables.
+ */
+# define TARGET_PAGE_ENTRY_EXTRA
+struct PageEntryExtra {
+/* See PAGEENTRYEXTRA fields in cpu.h */
+uint64_t x;
+};
  #endif

  #define NB_MMU_MODES 15
diff --git a/target/arm/internals.h b/target/arm/internals.h
index c66f74a0db..2b38a83574 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -74,6 +74,11 @@ FIELD(V7M_EXCRET, DCRS, 5, 1)
  FIELD(V7M_EXCRET, S, 6, 1)
  FIELD(V7M_EXCRET, RES1, 7, 25) /* including the must-be-1 prefix */

+/* Bit definitions for PageEntryExtra */
+FIELD(PAGEENTRYEXTRA, ATTRS, 0, 8)
+FIELD(PAGEENTRYEXTRA, SHAREABILITY, 8, 2)
+FIELD(PAGEENTRYEXTRA, PA, 12, 52)


So why do we want these things in particular? It would be
helpful to describe the intended uses in the commit message
to save the reader having to read the next 60 patches to
find out :-)


Heh, yes.  Basically, it's what S1_ptw_translate requires (pa, attrs), so that we can 
report a stage1 ptw failure, and what do_ats_write requires (pa, sh, attrs) for filling in 
PAR_EL1.  Although within these 62 patches I didn't came back to finish converting 
do_ats_write to use probe_access_flags_extra instead of using get_phys_addr directly, it 
was a goal.



Is wanting to cache the physaddr an Arm-specific thing, or is it
something we should consider having in the core softmmu code?


I'm not sure what other targets require for their 2-stage page table walks.  I guess I 
should have a look (riscv, i386, ?).


It *is* possible to recover the phys addr from the iommutlb, because I was doing that in 
mte_helper.c (see code removed in patch 5), but it's certainly not simple.



  if (likely(!ret)) {
+PageEntryExtra extra = {};
+
  /*
   * Map a single [sub]page. Regions smaller than our declared
   * target page size are handled specially, so for those we
- * pass in the exact addresses.
+ * pass in the exact addresses.  This only happens for M-profile,
+ * which does not use or require PageEntryExtra.
   */


Do we have to exclude M-profile here because the PageEntryExtra
data is strictly-per-page, or because the way we've formatted
our extra uint64_t requires the physaddr to be page-aligned, or both?


Because our extra uint64_t requires page alignment, and reuses those bits.


r~

Re: [PATCH] e1000: set RX descriptor status in a separate operation

[Jason Wang]

On Mon, Jul 4, 2022 at 5:05 PM Ding Hui  wrote:
>
> On 2022/7/4 15:10, Jason Wang wrote:
> >
> > 在 2022/6/29 17:40, Ding Hui 写道:
> >> @@ -1013,6 +1013,9 @@ e1000_receive_iov(NetClientState *nc, const
> >> struct iovec *iov, int iovcnt)
> >>   DBGOUT(RX, "Null RX descriptor!!\n");
> >>   }
> >>   pci_dma_write(d, base, , sizeof(desc));
> >> +desc.status |= (vlan_status | E1000_RXD_STAT_DD);
> >> +pci_dma_write(d, base + offsetof(struct e1000_rx_desc, status),
> >> +  , sizeof(desc.status));
> >
> >
> > Good catch, but to be more safe, I wonder if we can simply use
> > stx_le_pci_dma() here?
> >
>
> Do you mean stb_le_pci_dma(d, base + offsetof(struct e1000_rx_desc,
> status), desc.status, MEMTXATTRS_UNSPECIFIED)?
>
> I checked both pci_dma_write() and stb_le_pci_dma(), there is no
> difference between them,

I think the difference is that the stx_xxx() can guarantee the atomicy
when it is allowed by the arch.

> but I'm not sure whether it is proper to mixed
> use address based api and value based api, besides that it's OK to me.
>
> Thanks for reply.

I apply this patch as is.

Thanks

>
> --
> Thanks,
> - Ding Hui
>
>

Re: [PATCH v2 03/11] goldfish_rtc: Add endianness property

[Jason A. Donenfeld]

On Tue, Jul 05, 2022 at 05:40:31AM +0900, Stafford Horne wrote:
>   riscv{LE}--->goldfish_rtc{LE}
>   mips-longsoon3{LE}-->goldfish_rtc{LE}
>   openrisc{BE}>goldfish_rtc{LE} (LE to BE conversion done in 
> driver)
>   m68k{BE}>goldfish_rtc{BE} (only big-endian user)

I wish the powers that be would lighten up a little bit and let us
change m68k to be LE, and then we could avoid all this...

Just a last grumble, I guess.

Jason

Re: [PATCH v2] hw/arm/virt: dt: add rng-seed property

[Jason A. Donenfeld]

Hi Peter,

On Mon, Jul 04, 2022 at 03:42:55PM +0100, Peter Maydell wrote:
> We should also add a section to docs/about/deprecated.rst
> noting that the old name is deprecated in favour of the new one.
> I'll fold that in when I add the patch to target-arm.next, to
> save you doing a respin:

Thanks for doing that. Your text looks good to me.

Jason

Re: [PATCH v10 04/12] target/riscv: pmu: Make number of counters configurable

[Weiwei Li]

在 2022/7/4 下午11:26, Weiwei Li 写道:


在 2022/6/21 上午7:15, Atish Patra 写道:

The RISC-V privilege specification provides flexibility to implement
any number of counters from 29 programmable counters. However, the QEMU
implements all the counters.

Make it configurable through pmu config parameter which now will 
indicate

how many programmable counters should be implemented by the cpu.

Reviewed-by: Bin Meng 
Reviewed-by: Alistair Francis 
Signed-off-by: Atish Patra 
Signed-off-by: Atish Patra 
---
  target/riscv/cpu.c |  3 +-
  target/riscv/cpu.h |  2 +-
  target/riscv/csr.c | 94 ++
  3 files changed, 63 insertions(+), 36 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1b57b3c43980..d12c6dc630ca 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -851,7 +851,6 @@ static void riscv_cpu_init(Object *obj)
  {
  RISCVCPU *cpu = RISCV_CPU(obj);
  -    cpu->cfg.ext_pmu = true;
  cpu->cfg.ext_ifencei = true;
  cpu->cfg.ext_icsr = true;
  cpu->cfg.mmu = true;
@@ -879,7 +878,7 @@ static Property riscv_cpu_extensions[] = {
  DEFINE_PROP_BOOL("u", RISCVCPU, cfg.ext_u, true),
  DEFINE_PROP_BOOL("v", RISCVCPU, cfg.ext_v, false),
  DEFINE_PROP_BOOL("h", RISCVCPU, cfg.ext_h, true),
-    DEFINE_PROP_BOOL("pmu", RISCVCPU, cfg.ext_pmu, true),
+    DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),


I think It's better to add  a check on cfg.pmu_num to  <= 29.


OK, I find this check in the following patch.

  DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
  DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
  DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 252c30a55d78..ffee54ea5c27 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -397,7 +397,6 @@ struct RISCVCPUConfig {
  bool ext_zksed;
  bool ext_zksh;
  bool ext_zkt;
-    bool ext_pmu;
  bool ext_ifencei;
  bool ext_icsr;
  bool ext_svinval;
@@ -421,6 +420,7 @@ struct RISCVCPUConfig {
  /* Vendor-specific custom extensions */
  bool ext_XVentanaCondOps;
  +    uint8_t pmu_num;
  char *priv_spec;
  char *user_spec;
  char *bext_spec;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 0ca05c77883c..b4a8e15f498f 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -73,9 +73,17 @@ static RISCVException ctr(CPURISCVState *env, int 
csrno)

  CPUState *cs = env_cpu(env);
  RISCVCPU *cpu = RISCV_CPU(cs);
  int ctr_index;
+    int base_csrno = CSR_HPMCOUNTER3;
+    bool rv32 = riscv_cpu_mxl(env) == MXL_RV32 ? true : false;
  -    if (!cpu->cfg.ext_pmu) {
-    /* The PMU extension is not enabled */
+    if (rv32 && csrno >= CSR_CYCLEH) {
+    /* Offset for RV32 hpmcounternh counters */
+    base_csrno += 0x80;
+    }
+    ctr_index = csrno - base_csrno;
+
+    if (!cpu->cfg.pmu_num || ctr_index >= (cpu->cfg.pmu_num)) {
+    /* No counter is enabled in PMU or the counter is out of 
range */


I seems unnecessary to add '!cpu->cfg.pmu_num ' here, 'ctr_index >= 
(cpu->cfg.pmu_num)' is true

Typo.  I -> It


when cpu->cfg.pmu_num is zero if the problem for base_csrno is fixed.

Ragards,

Weiwei Li


  return RISCV_EXCP_ILLEGAL_INST;
  }
  @@ -103,7 +111,7 @@ static RISCVException ctr(CPURISCVState *env, 
int csrno)

  }
  break;
  }
-    if (riscv_cpu_mxl(env) == MXL_RV32) {
+    if (rv32) {
  switch (csrno) {
  case CSR_CYCLEH:
  if (!get_field(env->mcounteren, COUNTEREN_CY)) {
@@ -158,7 +166,7 @@ static RISCVException ctr(CPURISCVState *env, int 
csrno)

  }
  break;
  }
-    if (riscv_cpu_mxl(env) == MXL_RV32) {
+    if (rv32) {
  switch (csrno) {
  case CSR_CYCLEH:
  if (!get_field(env->hcounteren, COUNTEREN_CY) &&
@@ -202,6 +210,26 @@ static RISCVException ctr32(CPURISCVState *env, 
int csrno)

  }
    #if !defined(CONFIG_USER_ONLY)
+static RISCVException mctr(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    int ctr_index;
+    int base_csrno = CSR_MHPMCOUNTER3;
+
+    if ((riscv_cpu_mxl(env) == MXL_RV32) && csrno >= CSR_MCYCLEH) {
+    /* Offset for RV32 mhpmcounternh counters */
+    base_csrno += 0x80;
+    }
+    ctr_index = csrno - base_csrno;
+    if (!cpu->cfg.pmu_num || ctr_index >= cpu->cfg.pmu_num) {
+    /* The PMU is not enabled or counter is out of range*/
+    return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return RISCV_EXCP_NONE;
+}
+
  static RISCVException any(CPURISCVState *env, int csrno)
  {
  return RISCV_EXCP_NONE;
@@ -3687,35 +3715,35 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
  [CSR_HPMCOUNTER30]   = { "hpmcounter30",   ctr, read_zero },
  [CSR_HPMCOUNTER31]   = { "hpmcounter31",   ctr, 

Re: [PATCH 01/62] accel/tcg: Introduce PageEntryExtra

[Richard Henderson]

On 7/4/22 20:58, Peter Maydell wrote:

+ * Returns a pointer to the iotlb entry, with env_tlb(env)->c.lock
+ * still locked, for final additions to the iotlb entry.  The caller
+ * must unlock the lock.
   */
-void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr,
- hwaddr paddr, MemTxAttrs attrs, int prot,
- int mmu_idx, target_ulong size)
+void tlb_set_page_with_extra(CPUState *cpu, target_ulong vaddr, hwaddr paddr,
+ MemTxAttrs attrs, PageEntryExtra extra,
+ int prot, int mmu_idx, target_ulong size)


The comment claims it returns a pointer to the iotlb entry, but
the code says it returns void... leftover from a previous design?


Whoops, yes.  In the end I thought that was a bit too much internals exposed.


r~

Re: [PATCH v10 08/12] target/riscv: Add sscofpmf extension support

[Weiwei Li]

在 2022/6/21 上午7:15, Atish Patra 写道:

The Sscofpmf ('Ss' for Privileged arch and Supervisor-level extensions,
and 'cofpmf' for Count OverFlow and Privilege Mode Filtering)
extension allows the perf to handle overflow interrupts and filtering
support. This patch provides a framework for programmable
counters to leverage the extension. As the extension doesn't have any
provision for the overflow bit for fixed counters, the fixed events
can also be monitoring using programmable counters. The underlying
counters for cycle and instruction counters are always running. Thus,
a separate timer device is programmed to handle the overflow.

Signed-off-by: Atish Patra 
Signed-off-by: Atish Patra 
---
  target/riscv/cpu.c  |  11 ++
  target/riscv/cpu.h  |  25 +++
  target/riscv/cpu_bits.h |  55 +++
  target/riscv/csr.c  | 165 ++-
  target/riscv/machine.c  |   1 +
  target/riscv/pmu.c  | 357 +++-
  target/riscv/pmu.h  |   7 +
  7 files changed, 610 insertions(+), 11 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index d12c6dc630ca..7d9e2aca12a9 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -22,6 +22,7 @@
  #include "qemu/ctype.h"
  #include "qemu/log.h"
  #include "cpu.h"
+#include "pmu.h"
  #include "internals.h"
  #include "exec/exec-all.h"
  #include "qapi/error.h"
@@ -775,6 +776,15 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
  set_misa(env, env->misa_mxl, ext);
  }
  
+#ifndef CONFIG_USER_ONLY

+if (cpu->cfg.pmu_num) {
+if (!riscv_pmu_init(cpu, cpu->cfg.pmu_num) && cpu->cfg.ext_sscofpmf) {
+cpu->pmu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
+  riscv_pmu_timer_cb, cpu);
+}
+ }
+#endif
+
  riscv_cpu_register_gdb_regs_for_features(cs);
  
  qemu_init_vcpu(cs);

@@ -879,6 +889,7 @@ static Property riscv_cpu_extensions[] = {
  DEFINE_PROP_BOOL("v", RISCVCPU, cfg.ext_v, false),
  DEFINE_PROP_BOOL("h", RISCVCPU, cfg.ext_h, true),
  DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),
+DEFINE_PROP_BOOL("sscofpmf", RISCVCPU, cfg.ext_sscofpmf, false),
  DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
  DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
  DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 5c7acc055ac9..db193c3d 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -137,6 +137,8 @@ typedef struct PMUCTRState {
  /* Snapshort value of a counter in RV32 */
  target_ulong mhpmcounterh_prev;
  bool started;
+/* Value beyond UINT32_MAX/UINT64_MAX before overflow interrupt trigger */
+target_ulong irq_overflow_left;
  } PMUCTRState;
  
  struct CPUArchState {

@@ -297,6 +299,9 @@ struct CPUArchState {
  /* PMU event selector configured values. First three are unused*/
  target_ulong mhpmevent_val[RV_MAX_MHPMEVENTS];
  
+/* PMU event selector configured values for RV32*/

+target_ulong mhpmeventh_val[RV_MAX_MHPMEVENTS];
+
  target_ulong sscratch;
  target_ulong mscratch;
  
@@ -433,6 +438,7 @@ struct RISCVCPUConfig {

  bool ext_zve32f;
  bool ext_zve64f;
  bool ext_zmmul;
+bool ext_sscofpmf;
  bool rvv_ta_all_1s;
  
  uint32_t mvendorid;

@@ -479,6 +485,12 @@ struct ArchCPU {
  
  /* Configuration Settings */

  RISCVCPUConfig cfg;
+
+QEMUTimer *pmu_timer;
+/* A bitmask of Available programmable counters */
+uint32_t pmu_avail_ctrs;
+/* Mapping of events to counters */
+GHashTable *pmu_event_ctr_map;
  };
  
  static inline int riscv_has_ext(CPURISCVState *env, target_ulong ext)

@@ -738,6 +750,19 @@ enum {
  CSR_TABLE_SIZE = 0x1000
  };
  
+/**

+ * The event id are encoded based on the encoding specified in the
+ * SBI specification v0.3
+ */
+
+enum riscv_pmu_event_idx {
+RISCV_PMU_EVENT_HW_CPU_CYCLES = 0x01,
+RISCV_PMU_EVENT_HW_INSTRUCTIONS = 0x02,
+RISCV_PMU_EVENT_CACHE_DTLB_READ_MISS = 0x10019,
+RISCV_PMU_EVENT_CACHE_DTLB_WRITE_MISS = 0x1001B,
+RISCV_PMU_EVENT_CACHE_ITLB_PREFETCH_MISS = 0x10021,
+};
+
  /* CSR function table */
  extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
  
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h

index b3f7fa713000..d94abefdaa0f 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -400,6 +400,37 @@
  #define CSR_MHPMEVENT29 0x33d
  #define CSR_MHPMEVENT30 0x33e
  #define CSR_MHPMEVENT31 0x33f
+
+#define CSR_MHPMEVENT3H 0x723
+#define CSR_MHPMEVENT4H 0x724
+#define CSR_MHPMEVENT5H 0x725
+#define CSR_MHPMEVENT6H 0x726
+#define CSR_MHPMEVENT7H 0x727
+#define CSR_MHPMEVENT8H 0x728
+#define CSR_MHPMEVENT9H 0x729
+#define CSR_MHPMEVENT10H0x72a
+#define CSR_MHPMEVENT11H0x72b
+#define CSR_MHPMEVENT12H0x72c
+#define CSR_MHPMEVENT13H   

Re: [PATCH 2/9] target/ppc: add errp to kvmppc_read_int_cpu_dt()

[Daniel Henrique Barboza]

On 7/4/22 14:34, Cédric Le Goater wrote:

On 7/2/22 15:34, Daniel Henrique Barboza wrote:



On 7/2/22 03:24, Cédric Le Goater wrote:

On 6/30/22 21:42, Daniel Henrique Barboza wrote:

The function can't just return 0 whether an error happened and call it a
day. We must provide a way of letting callers know if the zero return is
legitimate or due to an error.

Add an Error pointer to kvmppc_read_int_cpu_dt() that will be filled
with an appropriate error, if one occurs. Callers are then free to pass
an Error pointer and handle it.

Signed-off-by: Daniel Henrique Barboza 
---
  target/ppc/kvm.c | 16 +---
  1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 109823136d..bc17437097 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -1925,15 +1925,17 @@ static uint64_t kvmppc_read_int_dt(const char *filename)
  /*
   * Read a CPU node property from the host device tree that's a single
- * integer (32-bit or 64-bit).  Returns 0 if anything goes wrong
- * (can't find or open the property, or doesn't understand the format)
+ * integer (32-bit or 64-bit).  Returns 0 and set errp if anything goes
+ * wrong (can't find or open the property, or doesn't understand the
+ * format)
   */
-static uint64_t kvmppc_read_int_cpu_dt(const char *propname)
+static uint64_t kvmppc_read_int_cpu_dt(const char *propname, Error **errp)
  {
  char buf[PATH_MAX], *tmp;
  uint64_t val;
  if (kvmppc_find_cpu_dt(buf, sizeof(buf))) {
+    error_setg(errp, "Failed to read CPU property %s", propname);
  return 0;
  }
@@ -1946,12 +1948,12 @@ static uint64_t kvmppc_read_int_cpu_dt(const char 
*propname)
  uint64_t kvmppc_get_clockfreq(void)
  {
-    return kvmppc_read_int_cpu_dt("clock-frequency");
+    return kvmppc_read_int_cpu_dt("clock-frequency", NULL);



This should be fatal. no ?



I'm not sure. I went under the assumption that there might be some weird
condition where 'clock-frequency' might be missing from the DT, and this
is why we're not exiting out immediately.

That said, the advantage of turning this into fatal is that we won't need
all the other patches that handles error on this function. We're going to
assume that if 'clock-frequency' is missing then we can't boot. I'm okay
with that.


I think so. Some machines behave badly when 'clock-frequency' is bogus,
division by zero, no console, etc. We could check easily with pseries
which is the only KVM PPC platform.



I can assert that with pSeries we can safely error_fatal if the DT doesn't
contain 'clock-frequency' since it's on PAPR under this section:

"C.6.2 OF Root Note

This section defines additional properties and methods associated with PAPR
platforms that OSs expect to find in the root node."

I interpret this as "if there's no clock-frequency just bail out".


Thanks,


Daniel




C.

[RFC] gitlab: introduce s390x wasmtime job

[Ilya Leoshkevich]

wasmtime is a WebAssembly runtime, which includes a large testsuite.
This testsuite uses qemu-user (aarch64 and s390x are supported) in
order to exercise foreign architectures. Over time it found several
regressions in qemu itself, and it would be beneficial to catch the
similar ones earlier.

To this end, this patch introduces a job that runs stable wasmtime
testsuite against qemu-s390x. The job is split into the following
components:

- A script for running the tests. Usable on developers' machines:

qemu$ mkdir build
qemu$ cd build
qemu/build$ ../tests/wasmtime/test s390x

- A script for building the tests (build-toolchain.sh).

- A dockerfile describing an image with the prebuilt testsuite
  (debian-s390x-wasmtime-cross.docker).

- gitlab job definition for building the image.

- gitlab job definition for using the image to run the tests.

It's possible to use this with aarch64 as well, but it segfaults at
the moment, therefore this patch does not provide job definitions for
it. This needs to be investigated separately.

The example of a resulting pipeline can be seen at [1].

The test job runs for about 30 minutes mostly due to unnecessary
rebuilds. They will be gone once [2] is integrated and makes it to a
stable release.

This patch depends on madvise(MADV_DONTNEED) passthrough support [3].

[1] https://gitlab.com/iii-i/qemu/-/pipelines/579677396
[2] https://github.com/bytecodealliance/wasmtime/pull/4377
[3] https://lists.gnu.org/archive/html/qemu-devel/2022-07/msg00112.html

Signed-off-by: Ilya Leoshkevich 
---
 .gitlab-ci.d/container-cross.yml  | 10 +++
 .gitlab-ci.d/container-template.yml   |  2 +-
 .gitlab-ci.d/qemu-project.yml |  1 +
 .gitlab-ci.d/wasmtime-template.yml|  6 ++
 .gitlab-ci.d/wasmtime.yml |  9 ++
 tests/docker/Makefile.include |  6 ++
 .../build-toolchain.sh| 83 +++
 .../debian-s390x-wasmtime-cross.docker| 16 
 tests/wasmtime/test   | 39 +
 9 files changed, 171 insertions(+), 1 deletion(-)
 create mode 100644 .gitlab-ci.d/wasmtime-template.yml
 create mode 100644 .gitlab-ci.d/wasmtime.yml
 create mode 100755 
tests/docker/dockerfiles/debian-s390x-wasmtime-cross.d/build-toolchain.sh
 create mode 100644 tests/docker/dockerfiles/debian-s390x-wasmtime-cross.docker
 create mode 100755 tests/wasmtime/test

diff --git a/.gitlab-ci.d/container-cross.yml b/.gitlab-ci.d/container-cross.yml
index b7963498a3..b3c4b76a16 100644
--- a/.gitlab-ci.d/container-cross.yml
+++ b/.gitlab-ci.d/container-cross.yml
@@ -138,6 +138,16 @@ s390x-debian-cross-container:
   variables:
 NAME: debian-s390x-cross
 
+s390x-debian-wasmtime-cross-container:
+  extends: .container_job_template
+  stage: containers
+  needs: ['s390x-debian-cross-container']
+  variables:
+NAME: debian-s390x-wasmtime-cross
+DOCKER_SCRIPT_ARGS: >
+  --extra-files
+  tests/docker/dockerfiles/debian-s390x-wasmtime-cross.d/build-toolchain.sh
+
 sh4-debian-cross-container:
   extends: .container_job_template
   stage: containers-layer2
diff --git a/.gitlab-ci.d/container-template.yml 
b/.gitlab-ci.d/container-template.yml
index c434b9c8f3..8654f89a15 100644
--- a/.gitlab-ci.d/container-template.yml
+++ b/.gitlab-ci.d/container-template.yml
@@ -15,7 +15,7 @@
 - echo "COMMON_TAG:$COMMON_TAG"
 - ./tests/docker/docker.py --engine docker build
   -t "qemu/$NAME" -f "tests/docker/dockerfiles/$NAME.docker"
-  -r $CI_REGISTRY/qemu-project/qemu
+  -r $CI_REGISTRY/qemu-project/qemu $DOCKER_SCRIPT_ARGS
 - docker tag "qemu/$NAME" "$TAG"
 - docker push "$TAG"
   after_script:
diff --git a/.gitlab-ci.d/qemu-project.yml b/.gitlab-ci.d/qemu-project.yml
index 691d9bf5dc..712a27f7a0 100644
--- a/.gitlab-ci.d/qemu-project.yml
+++ b/.gitlab-ci.d/qemu-project.yml
@@ -13,3 +13,4 @@ include:
   - local: '/.gitlab-ci.d/custom-runners.yml'
   - local: '/.gitlab-ci.d/cirrus.yml'
   - local: '/.gitlab-ci.d/windows.yml'
+  - local: '/.gitlab-ci.d/wasmtime.yml'
diff --git a/.gitlab-ci.d/wasmtime-template.yml 
b/.gitlab-ci.d/wasmtime-template.yml
new file mode 100644
index 00..cbf89c39eb
--- /dev/null
+++ b/.gitlab-ci.d/wasmtime-template.yml
@@ -0,0 +1,6 @@
+.wasmtime_job_template:
+  extends: .base_job_template
+  image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:latest
+  stage: test
+  script:
+- srcdir=$(pwd) && cd /build && "$srcdir"/tests/wasmtime/test "$ARCH"
diff --git a/.gitlab-ci.d/wasmtime.yml b/.gitlab-ci.d/wasmtime.yml
new file mode 100644
index 00..2647f28bb1
--- /dev/null
+++ b/.gitlab-ci.d/wasmtime.yml
@@ -0,0 +1,9 @@
+include:
+  - local: '/.gitlab-ci.d/wasmtime-template.yml'
+
+wasmtime-s390x:
+  extends: .wasmtime_job_template
+  needs: ['s390x-debian-wasmtime-cross-container']
+  variables:
+IMAGE: debian-s390x-wasmtime-cross
+ARCH: s390x
diff --git a/tests/docker/Makefile.include 

[PATCH 6/8] aspeed: Add AST2600 (BMC) to fby35

[Peter Delevoryas]

You can test booting the BMC with both '-device loader' and '-drive
file'. This is necessary because of how the fb-openbmc boot sequence
works (jump to 0x2000 after U-Boot SPL).

wget 
https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
qemu-system-arm -machine fby35 -nographic \
-device loader,file=fby35.mtd,addr=0,cpu-num=0 -drive 
file=fby35.mtd,format=raw,if=mtd

Signed-off-by: Peter Delevoryas 
---
 hw/arm/fby35.c | 41 +
 1 file changed, 41 insertions(+)

diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
index 03b458584c..5c5224d374 100644
--- a/hw/arm/fby35.c
+++ b/hw/arm/fby35.c
@@ -6,17 +6,55 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/units.h"
+#include "qapi/error.h"
+#include "sysemu/sysemu.h"
 #include "hw/boards.h"
+#include "hw/arm/aspeed_soc.h"
 
 #define TYPE_FBY35 MACHINE_TYPE_NAME("fby35")
 OBJECT_DECLARE_SIMPLE_TYPE(Fby35State, FBY35);
 
 struct Fby35State {
 MachineState parent_obj;
+
+MemoryRegion bmc_memory;
+MemoryRegion bmc_dram;
+MemoryRegion bmc_boot_rom;
+
+AspeedSoCState bmc;
 };
 
+#define FBY35_BMC_RAM_SIZE (2 * GiB)
+
+static void fby35_bmc_init(Fby35State *s)
+{
+memory_region_init(>bmc_memory, OBJECT(s), "bmc-memory", UINT64_MAX);
+memory_region_init_ram(>bmc_dram, OBJECT(s), "bmc-dram",
+   FBY35_BMC_RAM_SIZE, _abort);
+
+object_initialize_child(OBJECT(s), "bmc", >bmc, "ast2600-a3");
+object_property_set_int(OBJECT(>bmc), "ram-size", FBY35_BMC_RAM_SIZE,
+_abort);
+object_property_set_link(OBJECT(>bmc), "memory", OBJECT(>bmc_memory),
+ _abort);
+object_property_set_link(OBJECT(>bmc), "dram", OBJECT(>bmc_dram),
+ _abort);
+object_property_set_int(OBJECT(>bmc), "hw-strap1", 0x00C0,
+_abort);
+object_property_set_int(OBJECT(>bmc), "hw-strap2", 0x0003,
+_abort);
+aspeed_soc_uart_set_chr(>bmc, ASPEED_DEV_UART5, serial_hd(0));
+qdev_realize(DEVICE(>bmc), NULL, _abort);
+
+aspeed_board_init_flashes(>bmc.fmc, "n25q00", 2, 0);
+}
+
 static void fby35_init(MachineState *machine)
 {
+Fby35State *s = FBY35(machine);
+
+fby35_bmc_init(s);
 }
 
 static void fby35_class_init(ObjectClass *oc, void *data)
@@ -25,6 +63,9 @@ static void fby35_class_init(ObjectClass *oc, void *data)
 
 mc->desc = "Meta Platforms fby35";
 mc->init = fby35_init;
+mc->no_floppy = 1;
+mc->no_cdrom = 1;
+mc->min_cpus = mc->max_cpus = mc->default_cpus = 2;
 }
 
 static const TypeInfo fby35_types[] = {
-- 
2.37.0

[PATCH 8/8] aspeed: Add AST1030 (BIC) to fby35

[Peter Delevoryas]

With the BIC, the easiest way to run everything is to create two pty's
for each SoC and reserve stdin/stdout for the monitor:

wget 
https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
wget 
https://github.com/peterdelevoryas/OpenBIC/releases/download/oby35-cl-2022.13.01/Y35BCL.elf
qemu-system-arm -machine fby35 \
-drive file=fby35.mtd,format=raw,if=mtd \
-device loader,file=fby35.mtd,addr=0,cpu-num=0 \
-serial pty -serial pty -serial mon:stdio -display none -S

screen /dev/ttys0
screen /dev/ttys1
(qemu) c

Signed-off-by: Peter Delevoryas 
---
 hw/arm/fby35.c | 27 ++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
index d3edfa3b10..031602800f 100644
--- a/hw/arm/fby35.c
+++ b/hw/arm/fby35.c
@@ -11,7 +11,9 @@
 #include "sysemu/sysemu.h"
 #include "sysemu/block-backend.h"
 #include "hw/boards.h"
+#include "hw/qdev-clock.h"
 #include "hw/arm/aspeed_soc.h"
+#include "hw/arm/boot.h"
 
 #define TYPE_FBY35 MACHINE_TYPE_NAME("fby35")
 OBJECT_DECLARE_SIMPLE_TYPE(Fby35State, FBY35);
@@ -22,8 +24,11 @@ struct Fby35State {
 MemoryRegion bmc_memory;
 MemoryRegion bmc_dram;
 MemoryRegion bmc_boot_rom;
+MemoryRegion bic_memory;
+Clock *bic_sysclk;
 
 AspeedSoCState bmc;
+AspeedSoCState bic;
 
 bool mmio_exec;
 };
@@ -110,11 +115,31 @@ static void fby35_bmc_init(Fby35State *s)
 }
 }
 
+static void fby35_bic_init(Fby35State *s)
+{
+s->bic_sysclk = clock_new(OBJECT(s), "SYSCLK");
+clock_set_hz(s->bic_sysclk, 2ULL);
+
+memory_region_init(>bic_memory, OBJECT(s), "bic-memory", UINT64_MAX);
+
+object_initialize_child(OBJECT(s), "bic", >bic, "ast1030-a1");
+qdev_connect_clock_in(DEVICE(>bic), "sysclk", s->bic_sysclk);
+object_property_set_link(OBJECT(>bic), "memory", OBJECT(>bic_memory),
+ _abort);
+aspeed_soc_uart_set_chr(>bic, ASPEED_DEV_UART5, serial_hd(1));
+qdev_realize(DEVICE(>bic), NULL, _abort);
+
+aspeed_board_init_flashes(>bic.fmc, "sst25vf032b", 2, 2);
+aspeed_board_init_flashes(>bic.spi[0], "sst25vf032b", 2, 4);
+aspeed_board_init_flashes(>bic.spi[1], "sst25vf032b", 2, 6);
+}
+
 static void fby35_init(MachineState *machine)
 {
 Fby35State *s = FBY35(machine);
 
 fby35_bmc_init(s);
+fby35_bic_init(s);
 }
 
 
@@ -141,7 +166,7 @@ static void fby35_class_init(ObjectClass *oc, void *data)
 mc->init = fby35_init;
 mc->no_floppy = 1;
 mc->no_cdrom = 1;
-mc->min_cpus = mc->max_cpus = mc->default_cpus = 2;
+mc->min_cpus = mc->max_cpus = mc->default_cpus = 3;
 
 object_class_property_add_bool(oc, "execute-in-place",
fby35_get_mmio_exec,
-- 
2.37.0

[PATCH 3/8] aspeed: Refactor UART init for multi-SoC machines

[Peter Delevoryas]

This change moves the code that connects the SoC UART's to serial_hd's
to the machine.

It makes each UART a proper child member of the SoC, and then allows the
machine to selectively initialize the chardev for each UART with a
serial_hd.

This should preserve backwards compatibility, but also allow multi-SoC
boards to completely change the wiring of serial devices from the
command line to specific SoC UART's.

This also removes the uart-default property from the SoC, since the SoC
doesn't need to know what UART is the "default" on the machine anymore.

I tested this using the images and commands from the previous
refactoring, and another test image for the ast1030:

wget 
https://github.com/facebook/openbmc/releases/download/v2021.49.0/fuji.mtd
wget 
https://github.com/facebook/openbmc/releases/download/v2021.49.0/wedge100.mtd
wget 
https://github.com/peterdelevoryas/OpenBIC/releases/download/oby35-cl-2022.13.01/Y35BCL.elf

Fuji uses UART1:

qemu-system-arm -machine fuji-bmc \
-drive file=fuji.mtd,format=raw,if=mtd \
-nographic

ast2600-evb uses uart-default=UART5:

qemu-system-arm -machine ast2600-evb \
-drive file=fuji.mtd,format=raw,if=mtd \
-serial null -serial mon:stdio -display none

Wedge100 uses UART3:

qemu-system-arm -machine palmetto-bmc \
-drive file=wedge100.mtd,format=raw,if=mtd \
-serial null -serial null -serial null \
-serial mon:stdio -display none

AST1030 EVB uses UART5:

qemu-system-arm -machine ast1030-evb \
-kernel Y35BCL.elf -nographic

Fixes: 6827ff20b2975 ("hw: aspeed: Init all UART's with serial devices")
Signed-off-by: Peter Delevoryas 
---
 hw/arm/aspeed.c | 22 +---
 hw/arm/aspeed_ast10x0.c |  8 +-
 hw/arm/aspeed_ast2600.c |  8 +-
 hw/arm/aspeed_soc.c | 50 +
 include/hw/arm/aspeed_soc.h |  7 --
 5 files changed, 71 insertions(+), 24 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index bee8a748ec..cc395f988c 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -26,6 +26,7 @@
 #include "qemu/error-report.h"
 #include "qemu/units.h"
 #include "hw/qdev-clock.h"
+#include "sysemu/sysemu.h"
 
 static struct arm_boot_info aspeed_board_binfo = {
 .board_id = -1, /* device-tree-only board */
@@ -301,6 +302,21 @@ static void sdhci_attach_drive(SDHCIState *sdhci, 
DriveInfo *dinfo)
_fatal);
 }
 
+static void connect_serial_hds_to_uarts(AspeedMachineState *bmc)
+{
+AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(bmc);
+AspeedSoCState *s = >soc;
+AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
+
+aspeed_soc_uart_set_chr(s, amc->uart_default, serial_hd(0));
+for (int i = 1, uart = ASPEED_DEV_UART1; i < sc->uarts_num; i++, uart++) {
+if (uart == amc->uart_default) {
+continue;
+}
+aspeed_soc_uart_set_chr(s, uart, serial_hd(i));
+}
+}
+
 static void aspeed_machine_init(MachineState *machine)
 {
 AspeedMachineState *bmc = ASPEED_MACHINE(machine);
@@ -346,8 +362,7 @@ static void aspeed_machine_init(MachineState *machine)
 object_property_set_int(OBJECT(>soc), "hw-prot-key",
 ASPEED_SCU_PROT_KEY, _abort);
 }
-qdev_prop_set_uint32(DEVICE(>soc), "uart-default",
- amc->uart_default);
+connect_serial_hds_to_uarts(bmc);
 qdev_realize(DEVICE(>soc), NULL, _abort);
 
 aspeed_board_init_flashes(>soc.fmc,
@@ -1372,8 +1387,7 @@ static void aspeed_minibmc_machine_init(MachineState 
*machine)
 
 object_property_set_link(OBJECT(>soc), "memory",
  OBJECT(get_system_memory()), _abort);
-qdev_prop_set_uint32(DEVICE(>soc), "uart-default",
- amc->uart_default);
+connect_serial_hds_to_uarts(bmc);
 qdev_realize(DEVICE(>soc), NULL, _abort);
 
 aspeed_board_init_flashes(>soc.fmc,
diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index 677699e54c..4d0b9b115f 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -144,6 +144,10 @@ static void aspeed_soc_ast1030_init(Object *obj)
 object_initialize_child(obj, "wdt[*]", >wdt[i], typename);
 }
 
+for (i = 0; i < sc->uarts_num; i++) {
+object_initialize_child(obj, "uart[*]", >uart[i], TYPE_SERIAL_MM);
+}
+
 snprintf(typename, sizeof(typename), "aspeed.gpio-%s", socname);
 object_initialize_child(obj, "gpio", >gpio, typename);
 
@@ -255,7 +259,9 @@ static void aspeed_soc_ast1030_realize(DeviceState 
*dev_soc, Error **errp)
 sc->irqmap[ASPEED_DEV_KCS] + 
aspeed_lpc_kcs_4));
 
 /* UART */
-aspeed_soc_uart_init(s);
+if (!aspeed_soc_uart_realize(s, errp)) {
+return;
+}
 
 /* Timer */
 object_property_set_link(OBJECT(>timerctrl), "scu", OBJECT(>scu),
diff --git a/hw/arm/aspeed_ast2600.c 

[PATCH 5/8] aspeed: Add fby35 skeleton

[Peter Delevoryas]

Signed-off-by: Peter Delevoryas 
Reviewed-by: Cédric Le Goater 
---
 MAINTAINERS|  1 +
 hw/arm/fby35.c | 39 +++
 hw/arm/meson.build |  3 ++-
 3 files changed, 42 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/fby35.c

diff --git a/MAINTAINERS b/MAINTAINERS
index d9378511b7..147330ddd7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1067,6 +1067,7 @@ F: hw/net/ftgmac100.c
 F: include/hw/net/ftgmac100.h
 F: docs/system/arm/aspeed.rst
 F: tests/qtest/*aspeed*
+F: hw/arm/fby35.c
 
 NRF51
 M: Joel Stanley 
diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
new file mode 100644
index 00..03b458584c
--- /dev/null
+++ b/hw/arm/fby35.c
@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates. (http://www.meta.com)
+ *
+ * This code is licensed under the GPL version 2 or later. See the COPYING
+ * file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/boards.h"
+
+#define TYPE_FBY35 MACHINE_TYPE_NAME("fby35")
+OBJECT_DECLARE_SIMPLE_TYPE(Fby35State, FBY35);
+
+struct Fby35State {
+MachineState parent_obj;
+};
+
+static void fby35_init(MachineState *machine)
+{
+}
+
+static void fby35_class_init(ObjectClass *oc, void *data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+
+mc->desc = "Meta Platforms fby35";
+mc->init = fby35_init;
+}
+
+static const TypeInfo fby35_types[] = {
+{
+.name = MACHINE_TYPE_NAME("fby35"),
+.parent = TYPE_MACHINE,
+.class_init = fby35_class_init,
+.instance_size = sizeof(Fby35State),
+},
+};
+
+DEFINE_TYPES(fby35_types);
diff --git a/hw/arm/meson.build b/hw/arm/meson.build
index 2d8381339c..92f9f6e000 100644
--- a/hw/arm/meson.build
+++ b/hw/arm/meson.build
@@ -51,7 +51,8 @@ arm_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files(
   'aspeed_soc.c',
   'aspeed.c',
   'aspeed_ast2600.c',
-  'aspeed_ast10x0.c'))
+  'aspeed_ast10x0.c',
+  'fby35.c'))
 arm_ss.add(when: 'CONFIG_MPS2', if_true: files('mps2.c'))
 arm_ss.add(when: 'CONFIG_MPS2', if_true: files('mps2-tz.c'))
 arm_ss.add(when: 'CONFIG_MSF2', if_true: files('msf2-soc.c'))
-- 
2.37.0

[PATCH 7/8] aspeed: fby35: Add a bootrom for the BMC

[Peter Delevoryas]

From: Cédric Le Goater 

The BMC boots from the first flash device by fetching instructions
from the flash contents. Add an alias region on 0x0 for this
purpose. There are currently performance issues with this method (TBs
being flushed too often), so as a faster alternative, install the
flash contents as a ROM in the BMC memory space.

See commit 1a15311a12fa ("hw/arm/aspeed: add a 'execute-in-place'
property to boot directly from CE0")

Signed-off-by: Cédric Le Goater 
Signed-off-by: Peter Delevoryas 
---
 hw/arm/fby35.c | 83 ++
 1 file changed, 83 insertions(+)

diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
index 5c5224d374..d3edfa3b10 100644
--- a/hw/arm/fby35.c
+++ b/hw/arm/fby35.c
@@ -9,6 +9,7 @@
 #include "qemu/units.h"
 #include "qapi/error.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/block-backend.h"
 #include "hw/boards.h"
 #include "hw/arm/aspeed_soc.h"
 
@@ -23,12 +24,49 @@ struct Fby35State {
 MemoryRegion bmc_boot_rom;
 
 AspeedSoCState bmc;
+
+bool mmio_exec;
 };
 
 #define FBY35_BMC_RAM_SIZE (2 * GiB)
+#define FBY35_BMC_FIRMWARE_ADDR 0x0
+
+static void fby35_bmc_write_boot_rom(DriveInfo *dinfo, MemoryRegion *mr,
+ hwaddr offset, size_t rom_size,
+ Error **errp)
+{
+BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
+g_autofree void *storage = NULL;
+int64_t size;
+
+/*
+ * The block backend size should have already been 'validated' by
+ * the creation of the m25p80 object.
+ */
+size = blk_getlength(blk);
+if (size <= 0) {
+error_setg(errp, "failed to get flash size");
+return;
+}
+
+if (rom_size > size) {
+rom_size = size;
+}
+
+storage = g_malloc0(rom_size);
+if (blk_pread(blk, 0, storage, rom_size) < 0) {
+error_setg(errp, "failed to read the initial flash content");
+return;
+}
+
+/* TODO: find a better way to install the ROM */
+memcpy(memory_region_get_ram_ptr(mr) + offset, storage, rom_size);
+}
 
 static void fby35_bmc_init(Fby35State *s)
 {
+DriveInfo *drive0 = drive_get(IF_MTD, 0, 0);
+
 memory_region_init(>bmc_memory, OBJECT(s), "bmc-memory", UINT64_MAX);
 memory_region_init_ram(>bmc_dram, OBJECT(s), "bmc-dram",
FBY35_BMC_RAM_SIZE, _abort);
@@ -48,6 +86,28 @@ static void fby35_bmc_init(Fby35State *s)
 qdev_realize(DEVICE(>bmc), NULL, _abort);
 
 aspeed_board_init_flashes(>bmc.fmc, "n25q00", 2, 0);
+
+/* Install first FMC flash content as a boot rom. */
+if (drive0) {
+AspeedSMCFlash *fl = >bmc.fmc.flashes[0];
+MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
+uint64_t size = memory_region_size(>mmio);
+
+if (s->mmio_exec) {
+memory_region_init_alias(boot_rom, NULL, "aspeed.boot_rom",
+ >mmio, 0, size);
+memory_region_add_subregion(>bmc_memory, 
FBY35_BMC_FIRMWARE_ADDR,
+boot_rom);
+} else {
+
+memory_region_init_rom(boot_rom, NULL, "aspeed.boot_rom",
+   size, _abort);
+memory_region_add_subregion(>bmc_memory, 
FBY35_BMC_FIRMWARE_ADDR,
+boot_rom);
+fby35_bmc_write_boot_rom(drive0, boot_rom, FBY35_BMC_FIRMWARE_ADDR,
+ size, _abort);
+}
+}
 }
 
 static void fby35_init(MachineState *machine)
@@ -57,6 +117,22 @@ static void fby35_init(MachineState *machine)
 fby35_bmc_init(s);
 }
 
+
+static bool fby35_get_mmio_exec(Object *obj, Error **errp)
+{
+return FBY35(obj)->mmio_exec;
+}
+
+static void fby35_set_mmio_exec(Object *obj, bool value, Error **errp)
+{
+FBY35(obj)->mmio_exec = value;
+}
+
+static void fby35_instance_init(Object *obj)
+{
+FBY35(obj)->mmio_exec = false;
+}
+
 static void fby35_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
@@ -66,6 +142,12 @@ static void fby35_class_init(ObjectClass *oc, void *data)
 mc->no_floppy = 1;
 mc->no_cdrom = 1;
 mc->min_cpus = mc->max_cpus = mc->default_cpus = 2;
+
+object_class_property_add_bool(oc, "execute-in-place",
+   fby35_get_mmio_exec,
+   fby35_set_mmio_exec);
+object_class_property_set_description(oc, "execute-in-place",
+   "boot directly from CE0 flash device");
 }
 
 static const TypeInfo fby35_types[] = {
@@ -74,6 +156,7 @@ static const TypeInfo fby35_types[] = {
 .parent = TYPE_MACHINE,
 .class_init = fby35_class_init,
 .instance_size = sizeof(Fby35State),
+.instance_init = fby35_instance_init,
 },
 };
 
-- 
2.37.0

[PATCH 4/8] aspeed: Make aspeed_board_init_flashes public

[Peter Delevoryas]

Signed-off-by: Peter Delevoryas 
---
 hw/arm/aspeed.c | 2 +-
 include/hw/arm/aspeed_soc.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index cc395f988c..b2486a9e78 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -262,7 +262,7 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, 
size_t rom_size,
 rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
 }
 
-static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
+void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
   unsigned int count, int unit0)
 {
 int i;
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 68e907cd64..8389200b2d 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -184,5 +184,7 @@ void aspeed_mmio_map(AspeedSoCState *s, SysBusDevice *dev, 
int n, hwaddr addr);
 void aspeed_mmio_map_unimplemented(AspeedSoCState *s, SysBusDevice *dev,
const char *name, hwaddr addr,
uint64_t size);
+void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
+   unsigned int count, int unit0);
 
 #endif /* ASPEED_SOC_H */
-- 
2.37.0

[PATCH 2/8] aspeed: Create SRAM name from first CPU index

[Peter Delevoryas]

To support multiple SoC's running simultaneously, we need a unique name for
each RAM region. DRAM is created by the machine, but SRAM is created by the
SoC, since in hardware it is part of the SoC's internals.

We need a way to uniquely identify each SRAM region though, for VM
migration. Since each of the SoC's CPU's has an index which identifies it
uniquely from other CPU's in the machine, we can use the index of any of the
CPU's in the SoC to uniquely identify differentiate the SRAM name from other
SoC SRAM's. In this change, I just elected to use the index of the first CPU
in each SoC.

Signed-off-by: Peter Delevoryas 
---
 hw/arm/aspeed_ast10x0.c | 5 -
 hw/arm/aspeed_ast2600.c | 5 +++--
 hw/arm/aspeed_soc.c | 5 +++--
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index 33ef331771..677699e54c 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -159,6 +159,7 @@ static void aspeed_soc_ast1030_realize(DeviceState 
*dev_soc, Error **errp)
 DeviceState *armv7m;
 Error *err = NULL;
 int i;
+g_autofree char *sram_name = NULL;
 
 if (!clock_has_source(s->sysclk)) {
 error_setg(errp, "sysclk clock must be wired up by the board code");
@@ -183,7 +184,9 @@ static void aspeed_soc_ast1030_realize(DeviceState 
*dev_soc, Error **errp)
 sysbus_realize(SYS_BUS_DEVICE(>armv7m), _abort);
 
 /* Internal SRAM */
-memory_region_init_ram(>sram, NULL, "aspeed.sram", sc->sram_size, );
+sram_name = g_strdup_printf("aspeed.sram.%d",
+CPU(s->armv7m.cpu)->cpu_index);
+memory_region_init_ram(>sram, OBJECT(s), sram_name, sc->sram_size, 
);
 if (err != NULL) {
 error_propagate(errp, err);
 return;
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 3f0611ac11..64eb5a7b26 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -276,6 +276,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
 AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
 Error *err = NULL;
 qemu_irq irq;
+g_autofree char *sram_name = NULL;
 
 /* IO space */
 aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(>iomem), "aspeed.io",
@@ -335,8 +336,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
 }
 
 /* SRAM */
-memory_region_init_ram(>sram, OBJECT(dev), "aspeed.sram",
-   sc->sram_size, );
+sram_name = g_strdup_printf("aspeed.sram.%d", CPU(>cpu[0])->cpu_index);
+memory_region_init_ram(>sram, OBJECT(s), sram_name, sc->sram_size, 
);
 if (err) {
 error_propagate(errp, err);
 return;
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 0f675e7fcd..0bb6a2f092 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -239,6 +239,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error 
**errp)
 AspeedSoCState *s = ASPEED_SOC(dev);
 AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
 Error *err = NULL;
+g_autofree char *sram_name = NULL;
 
 /* IO space */
 aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(>iomem), "aspeed.io",
@@ -259,8 +260,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error 
**errp)
 }
 
 /* SRAM */
-memory_region_init_ram(>sram, OBJECT(dev), "aspeed.sram",
-   sc->sram_size, );
+sram_name = g_strdup_printf("aspeed.sram.%d", CPU(>cpu[0])->cpu_index);
+memory_region_init_ram(>sram, OBJECT(s), sram_name, sc->sram_size, 
);
 if (err) {
 error_propagate(errp, err);
 return;
-- 
2.37.0

[PATCH 1/8] hw/i2c/pca954x: Add method to get channels

[Peter Delevoryas]

I added this helper in the Aspeed machine file a while ago to help
initialize fuji-bmc i2c devices. This moves it to the official pca954x
file so that other files can use it.

This does something very similar to pca954x_i2c_get_bus, but I think
this is useful when you have a very complicated dts with a lot of
switches, like the fuji dts.

This convenience method lets you write code that produces a flat array
of I2C buses that matches the naming in the dts. After that you can just
add individual sensors using the flat array of I2C buses.

See fuji_bmc_i2c_init to understand this point further.

The fuji dts is here for reference:

https://github.com/torvalds/linux/blob/40cb6373b46/arch/arm/boot/dts/aspeed-bmc-facebook-fuji.dts

Signed-off-by: Peter Delevoryas 
---
 hw/arm/aspeed.c  | 29 +
 hw/i2c/i2c_mux_pca954x.c | 10 ++
 include/hw/i2c/i2c_mux_pca954x.h | 13 +
 3 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 6fe9b13548..bee8a748ec 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -793,15 +793,6 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
 create_pca9552(soc, 15, 0x60);
 }
 
-static void get_pca9548_channels(I2CBus *bus, uint8_t mux_addr,
- I2CBus **channels)
-{
-I2CSlave *mux = i2c_slave_create_simple(bus, "pca9548", mux_addr);
-for (int i = 0; i < 8; i++) {
-channels[i] = pca954x_i2c_get_bus(mux, i);
-}
-}
-
 #define TYPE_LM75 TYPE_TMP105
 #define TYPE_TMP75 TYPE_TMP105
 #define TYPE_TMP422 "tmp422"
@@ -814,20 +805,18 @@ static void fuji_bmc_i2c_init(AspeedMachineState *bmc)
 for (int i = 0; i < 16; i++) {
 i2c[i] = aspeed_i2c_get_bus(>i2c, i);
 }
-I2CBus *i2c180 = i2c[2];
-I2CBus *i2c480 = i2c[8];
-I2CBus *i2c600 = i2c[11];
 
-get_pca9548_channels(i2c180, 0x70, [16]);
-get_pca9548_channels(i2c480, 0x70, [24]);
+pca954x_i2c_get_channels(i2c[2], 0x70, "pca9548", [16]);
+pca954x_i2c_get_channels(i2c[8], 0x70, "pca9548", [24]);
 /* NOTE: The device tree skips [32, 40) in the alias numbering */
-get_pca9548_channels(i2c600, 0x77, [40]);
-get_pca9548_channels(i2c[24], 0x71, [48]);
-get_pca9548_channels(i2c[25], 0x72, [56]);
-get_pca9548_channels(i2c[26], 0x76, [64]);
-get_pca9548_channels(i2c[27], 0x76, [72]);
+pca954x_i2c_get_channels(i2c[11], 0x77, "pca9548", [40]);
+pca954x_i2c_get_channels(i2c[24], 0x71, "pca9548", [48]);
+pca954x_i2c_get_channels(i2c[25], 0x72, "pca9548", [56]);
+pca954x_i2c_get_channels(i2c[26], 0x76, "pca9548", [64]);
+pca954x_i2c_get_channels(i2c[27], 0x76, "pca9548", [72]);
 for (int i = 0; i < 8; i++) {
-get_pca9548_channels(i2c[40 + i], 0x76, [80 + i * 8]);
+pca954x_i2c_get_channels(i2c[40 + i], 0x76, "pca9548",
+ [80 + i * 8]);
 }
 
 i2c_slave_create_simple(i2c[17], TYPE_LM75, 0x4c);
diff --git a/hw/i2c/i2c_mux_pca954x.c b/hw/i2c/i2c_mux_pca954x.c
index 3945de795c..6b07804546 100644
--- a/hw/i2c/i2c_mux_pca954x.c
+++ b/hw/i2c/i2c_mux_pca954x.c
@@ -169,6 +169,16 @@ I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel)
 return pca954x->bus[channel];
 }
 
+void pca954x_i2c_get_channels(I2CBus *bus, uint8_t address,
+  const char *type_name, I2CBus **channels)
+{
+I2CSlave *mux = i2c_slave_create_simple(bus, type_name, address);
+Pca954xClass *pc = PCA954X_GET_CLASS(mux);
+Pca954xState *pca954x = PCA954X(mux);
+
+memcpy(channels, pca954x->bus, pc->nchans * sizeof(channels[0]));
+}
+
 static void pca9546_class_init(ObjectClass *klass, void *data)
 {
 Pca954xClass *s = PCA954X_CLASS(klass);
diff --git a/include/hw/i2c/i2c_mux_pca954x.h b/include/hw/i2c/i2c_mux_pca954x.h
index 3dd25ec983..3a676a30a9 100644
--- a/include/hw/i2c/i2c_mux_pca954x.h
+++ b/include/hw/i2c/i2c_mux_pca954x.h
@@ -16,4 +16,17 @@
  */
 I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel);
 
+/**
+ * Creates an i2c mux and retrieves all of the channels associated with it.
+ *
+ * @bus: the i2c bus where the i2c mux resides.
+ * @address: the address of the i2c mux on the aforementioned i2c bus.
+ * @type_name: name of the i2c mux type to create.
+ * @channels: an output parameter specifying where to return the channels.
+ *
+ * Returns: None
+ */
+void pca954x_i2c_get_channels(I2CBus *bus, uint8_t address,
+  const char *type_name, I2CBus **channels);
+
 #endif
-- 
2.37.0

Re: [PATCH 1/2] hw: m25p80: Add Block Protect and Top Bottom bits for write protect

[Cédric Le Goater]

On 6/27/22 20:52, Iris Chen wrote:

Signed-off-by: Iris Chen 
---
  hw/block/m25p80.c | 74 +++
  1 file changed, 62 insertions(+), 12 deletions(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 50b523e5b1..0156a70f5e 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -38,21 +38,19 @@
  #include "trace.h"
  #include "qom/object.h"
  
-/* Fields for FlashPartInfo->flags */

-
-/* erase capabilities */
-#define ER_4K 1
-#define ER_32K 2
-/* set to allow the page program command to write 0s back to 1. Useful for
- * modelling EEPROM with SPI flash command set
- */
-#define EEPROM 0x100
-
  /* 16 MiB max in 3 byte address mode */
  #define MAX_3BYTES_SIZE 0x100
-
  #define SPI_NOR_MAX_ID_LEN 6
  
+/* Fields for FlashPartInfo->flags */

+enum spi_nor_option_flags {
+ER_4K  = BIT(0),
+ER_32K = BIT(1),
+EEPROM = BIT(2),
+SNOR_F_HAS_SR_TB   = BIT(3),
+SNOR_F_HAS_SR_BP3_BIT6 = BIT(4),
+};
+
  typedef struct FlashPartInfo {
  const char *part_name;
  /*
@@ -253,7 +251,8 @@ static const FlashPartInfo known_devices[] = {
  { INFO("n25q512a11",  0x20bb20,  0,  64 << 10, 1024, ER_4K) },
  { INFO("n25q512a13",  0x20ba20,  0,  64 << 10, 1024, ER_4K) },
  { INFO("n25q128", 0x20ba18,  0,  64 << 10, 256, 0) },
-{ INFO("n25q256a",0x20ba19,  0,  64 << 10, 512, ER_4K) },
+{ INFO("n25q256a",0x20ba19,  0,  64 << 10, 512,
+   ER_4K | SNOR_F_HAS_SR_BP3_BIT6 | SNOR_F_HAS_SR_TB) },
  { INFO("n25q512a",0x20ba20,  0,  64 << 10, 1024, ER_4K) },
  { INFO("n25q512ax3",  0x20ba20,  0x1000,  64 << 10, 1024, ER_4K) },
  { INFO("mt25ql512ab", 0x20ba20, 0x1044, 64 << 10, 1024, ER_4K | ER_32K) },
@@ -480,6 +479,11 @@ struct Flash {
  bool reset_enable;
  bool quad_enable;
  bool aai_enable;
+bool block_protect0;
+bool block_protect1;
+bool block_protect2;
+bool block_protect3;
+bool top_bottom_bit;
  bool status_register_write_disabled;
  uint8_t ear;
  
@@ -630,6 +634,29 @@ void flash_write8(Flash *s, uint32_t addr, uint8_t data)

  qemu_log_mask(LOG_GUEST_ERROR, "M25P80: write with write protect!\n");
  return;
  }
+uint32_t block_protect_value = (s->block_protect3 << 3) |
+   (s->block_protect2 << 2) |
+   (s->block_protect1 << 1) |
+   (s->block_protect0 << 0);
+
+ uint32_t num_protected_sectors = 1 << (block_protect_value - 1);


Something wrong will occur if all block_protect[0123] are zeroes.

C.


+ uint32_t sector = addr / s->pi->sector_size;
+
+ /* top_bottom_bit == 0 means TOP */
+if (!s->top_bottom_bit) {
+if (block_protect_value > 0 &&
+s->pi->n_sectors <= sector + num_protected_sectors) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "M25P80: write with write protect!\n");
+return;
+}
+} else {
+if (block_protect_value > 0 && sector < num_protected_sectors) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "M25P80: write with write protect!\n");
+return;
+}
+}
  
  if ((prev ^ data) & data) {

  trace_m25p80_programming_zero_to_one(s, addr, prev, data);
@@ -728,6 +755,15 @@ static void complete_collecting_data(Flash *s)
  break;
  case WRSR:
  s->status_register_write_disabled = extract32(s->data[0], 7, 1);
+s->block_protect0 = extract32(s->data[0], 2, 1);
+s->block_protect1 = extract32(s->data[0], 3, 1);
+s->block_protect2 = extract32(s->data[0], 4, 1);
+if (s->pi->flags & SNOR_F_HAS_SR_TB) {
+s->top_bottom_bit = extract32(s->data[0], 5, 1);
+}
+if (s->pi->flags & SNOR_F_HAS_SR_BP3_BIT6) {
+s->block_protect3 = extract32(s->data[0], 6, 1);
+}
  
  switch (get_man(s)) {

  case MAN_SPANSION:
@@ -1213,6 +1249,15 @@ static void decode_new_cmd(Flash *s, uint32_t value)
  case RDSR:
  s->data[0] = (!!s->write_enable) << 1;
  s->data[0] |= (!!s->status_register_write_disabled) << 7;
+s->data[0] |= (!!s->block_protect0) << 2;
+s->data[0] |= (!!s->block_protect1) << 3;
+s->data[0] |= (!!s->block_protect2) << 4;
+if (s->pi->flags & SNOR_F_HAS_SR_TB) {
+s->data[0] |= (!!s->top_bottom_bit) << 5;
+}
+if (s->pi->flags & SNOR_F_HAS_SR_BP3_BIT6) {
+s->data[0] |= (!!s->block_protect3) << 6;
+}
  
  if (get_man(s) == MAN_MACRONIX || get_man(s) == MAN_ISSI) {

  s->data[0] |= (!!s->quad_enable) << 6;
@@ -1553,6 +1598,11 @@ static void m25p80_reset(DeviceState *d)
  
  s->wp_level = true;

  s->status_register_write_disabled = false;
+s->block_protect0 = false;
+

Re: [PATCH v2 03/11] goldfish_rtc: Add endianness property

[Stafford Horne]

On Mon, Jul 04, 2022 at 12:23:23PM +0200, Laurent Vivier wrote:
> On 04/07/2022 12:21, Richard Henderson wrote:
> > On 7/4/22 15:46, Laurent Vivier wrote:
> > > On 04/07/2022 11:59, Richard Henderson wrote:
> > > > On 7/4/22 02:58, Stafford Horne wrote:
> > > > > -static const MemoryRegionOps goldfish_rtc_ops = {
> > > > > -    .read = goldfish_rtc_read,
> > > > > -    .write = goldfish_rtc_write,
> > > > > -    .endianness = DEVICE_NATIVE_ENDIAN,
> > > > > -    .valid = {
> > > > > -    .min_access_size = 4,
> > > > > -    .max_access_size = 4
> > > > > -    }
> > > > > +static const MemoryRegionOps goldfish_rtc_ops[3] = {
> > > > > +    [DEVICE_NATIVE_ENDIAN] = {
> > > > > +    .read = goldfish_rtc_read,
> > > > > +    .write = goldfish_rtc_write,
> > > > > +    .endianness = DEVICE_NATIVE_ENDIAN,
> > > > > +    .valid = {
> > > > > +    .min_access_size = 4,
> > > > > +    .max_access_size = 4
> > > > > +    }
> > > > > +    },
> > > > > +    [DEVICE_LITTLE_ENDIAN] = {
> > > > > +    .read = goldfish_rtc_read,
> > > > > +    .write = goldfish_rtc_write,
> > > > > +    .endianness = DEVICE_LITTLE_ENDIAN,
> > > > > +    .valid = {
> > > > > +    .min_access_size = 4,
> > > > > +    .max_access_size = 4
> > > > > +    }
> > > > > +    },
> > > > > +    [DEVICE_BIG_ENDIAN] = {
> > > > > +    .read = goldfish_rtc_read,
> > > > > +    .write = goldfish_rtc_write,
> > > > > +    .endianness = DEVICE_BIG_ENDIAN,
> > > > > +    .valid = {
> > > > > +    .min_access_size = 4,
> > > > > +    .max_access_size = 4
> > > > > +    }
> > > > > +    },
> > > > >   };
> > > > 
> > > > You don't need 3 copies, only big and little.
> > > > 
> > > > > +static Property goldfish_rtc_properties[] = {
> > > > > +    DEFINE_PROP_UINT8("endianness", GoldfishRTCState, endianness,
> > > > > +  DEVICE_NATIVE_ENDIAN),
> > > > > +    DEFINE_PROP_END_OF_LIST(),
> > > > > +};
> > > > 
> > > > ... and I think the clear desire for default is little-endian. 
> > > > I would make the property be bool, and add a comment that this
> > > > is only for m68k compatibility, so don't use it in new code.
> > > 
> > > m68k doesn't really need this.
> > > 
> > > kernel with the m68k virt machine and goldfish device supports
> > > "native" mode so I think it's not needed to add another layer of
> > > complexity for it.
> > 
> > "Another level"?  I'm talking about removing "native", and only having
> > "big" and "little", which is less complexity.
> 
> "Less complexity" is to keep only native. I'm not against the change, I'm
> just saying it's not needed by m68k.

Hi Laurent,

I would agree if we only had m68k.  But I am making this change so that OpenRISC
(another big-endian architecture) could use this.  In the OpenRISC case we want
to use this as little-endian so no kernel updates would be needed.

So in the end we will have the following qemu platforms:

  riscv{LE}--->goldfish_rtc{LE}
  mips-longsoon3{LE}-->goldfish_rtc{LE}
  openrisc{BE}>goldfish_rtc{LE} (LE to BE conversion done in driver)
  m68k{BE}>goldfish_rtc{BE} (only big-endian user)

-Stafford

Re: [PATCH v2 03/11] goldfish_rtc: Add endianness property

[Stafford Horne]

On Mon, Jul 04, 2022 at 03:29:57PM +0530, Richard Henderson wrote:
> On 7/4/22 02:58, Stafford Horne wrote:
> > -static const MemoryRegionOps goldfish_rtc_ops = {
> > -.read = goldfish_rtc_read,
> > -.write = goldfish_rtc_write,
> > -.endianness = DEVICE_NATIVE_ENDIAN,
> > -.valid = {
> > -.min_access_size = 4,
> > -.max_access_size = 4
> > -}
> > +static const MemoryRegionOps goldfish_rtc_ops[3] = {
> > +[DEVICE_NATIVE_ENDIAN] = {
> > +.read = goldfish_rtc_read,
> > +.write = goldfish_rtc_write,
> > +.endianness = DEVICE_NATIVE_ENDIAN,
> > +.valid = {
> > +.min_access_size = 4,
> > +.max_access_size = 4
> > +}
> > +},
> > +[DEVICE_LITTLE_ENDIAN] = {
> > +.read = goldfish_rtc_read,
> > +.write = goldfish_rtc_write,
> > +.endianness = DEVICE_LITTLE_ENDIAN,
> > +.valid = {
> > +.min_access_size = 4,
> > +.max_access_size = 4
> > +}
> > +},
> > +[DEVICE_BIG_ENDIAN] = {
> > +.read = goldfish_rtc_read,
> > +.write = goldfish_rtc_write,
> > +.endianness = DEVICE_BIG_ENDIAN,
> > +.valid = {
> > +.min_access_size = 4,
> > +.max_access_size = 4
> > +}
> > +},
> >   };
> 
> You don't need 3 copies, only big and little.
> 
> > +static Property goldfish_rtc_properties[] = {
> > +DEFINE_PROP_UINT8("endianness", GoldfishRTCState, endianness,
> > +  DEVICE_NATIVE_ENDIAN),
> > +DEFINE_PROP_END_OF_LIST(),
> > +};
> 
> ... and I think the clear desire for default is little-endian.  I would make
> the property be bool, and add a comment that this is only for m68k
> compatibility, so don't use it in new code.

Yeah, that makes sense.

-Stafford

Re: [PATCH v2 06/11] hw/openrisc: Initialize timer time at startupi

[Stafford Horne]

On Mon, Jul 04, 2022 at 03:33:26PM +0530, Richard Henderson wrote:
> On 7/4/22 02:58, Stafford Horne wrote:
> > The last_clk time was initialized at zero, this means when we calculate
> > the first delta we will calculate 0 vs current time which could cause
> > unnecessary hops.
> > 
> > Initialize last_clk to the qemu clock on initialization.
> > 
> > Signed-off-by: Stafford Horne 
> > ---
> >   hw/openrisc/cputimer.c | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/hw/openrisc/cputimer.c b/hw/openrisc/cputimer.c
> > index 93268815d8..4dbba3a3d4 100644
> > --- a/hw/openrisc/cputimer.c
> > +++ b/hw/openrisc/cputimer.c
> > @@ -140,6 +140,7 @@ void cpu_openrisc_clock_init(OpenRISCCPU *cpu)
> >   if (or1k_timer == NULL) {
> >   or1k_timer = g_new0(OR1KTimerState, 1);
> > +or1k_timer->last_clk = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> >   vmstate_register(NULL, 0, _or1k_timer, or1k_timer);
> >   }
> >   }
> 
> Init doesn't seem right.  Should be in reset?

Good point, I think reset would be better.

Re: [PATCH v2 08/11] target/openrisc: Enable MTTCG

[Stafford Horne]

On Mon, Jul 04, 2022 at 03:37:04PM +0530, Richard Henderson wrote:
> On 7/4/22 02:58, Stafford Horne wrote:
> >   case TO_SPR(10, 1): /* TTCR */
> > -cpu_openrisc_count_update(cpu);
> > +if (cpu_openrisc_timer_has_advanced(cpu)) {
> > +qemu_mutex_lock_iothread();
> > +cpu_openrisc_count_update(cpu);
> > +qemu_mutex_unlock_iothread();
> > +}
> 
> Lock around the whole if, I think.  Otherwise looks good.

Well, actually the cpu_openrisc_timer_has_advanced read is done once outside the
lock as an optimization to avoid taking the lock when it is not needed. i.e. if
we have 4 cores that all try to update the clock at the same time in theory only
one will have to take the lock and update the shared timer.

But I do see that could be flawed as after it takes the lock the timer could
have been updated by then.  Ill move it inside and see if there is any
perfromance hit / increase in the sync-profile.

> Reviewed-by: Richard Henderson 
> 
> 
> r~

Re: [PATCH v2 07/11] target/openrisc: Add interrupted CPU to log

[Stafford Horne]

On Mon, Jul 04, 2022 at 03:34:52PM +0530, Richard Henderson wrote:
> On 7/4/22 02:58, Stafford Horne wrote:
> > When we are tracing it's helpful to know which CPU's are getting
> > interrupted, att that detail to the log line.
> 
> "at".
> 
> Reviewed-by: Richard Henderson 

Actually it should be "add", thanks I fixed it.

-Stafford

[PATCH v3 3/3] migration/multifd: Warn user when zerocopy not working

[Leonardo Bras]

Some errors, like the lack of Scatter-Gather support by the network
interface(NETIF_F_SG) may cause sendmsg(...,MSG_ZEROCOPY) to fail on using
zero-copy, which causes it to fall back to the default copying mechanism.

After each full dirty-bitmap scan there should be a zero-copy flush
happening, which checks for errors each of the previous calls to
sendmsg(...,MSG_ZEROCOPY). If all of them failed to use zero-copy, then
increment dirty_sync_missed_zero_copy migration stat to let the user know
about it.

Signed-off-by: Leonardo Bras 
---
 migration/ram.h | 2 ++
 migration/multifd.c | 2 ++
 migration/ram.c | 5 +
 3 files changed, 9 insertions(+)

diff --git a/migration/ram.h b/migration/ram.h
index ded0a3a086..d3c7eb96f5 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -87,4 +87,6 @@ void ram_write_tracking_prepare(void);
 int ram_write_tracking_start(void);
 void ram_write_tracking_stop(void);
 
+void dirty_sync_missed_zero_copy(void);
+
 #endif
diff --git a/migration/multifd.c b/migration/multifd.c
index 684c014c86..3909b34967 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -624,6 +624,8 @@ int multifd_send_sync_main(QEMUFile *f)
 if (ret < 0) {
 error_report_err(err);
 return -1;
+} else if (ret == 1) {
+dirty_sync_missed_zero_copy();
 }
 }
 }
diff --git a/migration/ram.c b/migration/ram.c
index 01f9cc1d72..db948c4787 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -407,6 +407,11 @@ static void ram_transferred_add(uint64_t bytes)
 ram_counters.transferred += bytes;
 }
 
+void dirty_sync_missed_zero_copy(void)
+{
+ram_counters.dirty_sync_missed_zero_copy++;
+}
+
 /* used by the search for pages to send */
 struct PageSearchStatus {
 /* Current block being searched */
-- 
2.36.1

[PATCH v3 2/3] Add dirty-sync-missed-zero-copy migration stat

[Leonardo Bras]

Signed-off-by: Leonardo Bras 
---
 qapi/migration.json   | 7 ++-
 migration/migration.c | 2 ++
 monitor/hmp-cmds.c| 4 
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/qapi/migration.json b/qapi/migration.json
index 7102e474a6..fed08b9b88 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -55,6 +55,10 @@
 # @postcopy-bytes: The number of bytes sent during the post-copy phase
 #  (since 7.0).
 #
+# @dirty-sync-missed-zero-copy: Number of times dirty RAM synchronization could
+#   not avoid copying zero pages.  This is between 0
+#   and @dirty-sync-count * @multifd-channels.
+#   (since 7.1)
 # Since: 0.14
 ##
 { 'struct': 'MigrationStats',
@@ -65,7 +69,8 @@
'postcopy-requests' : 'int', 'page-size' : 'int',
'multifd-bytes' : 'uint64', 'pages-per-second' : 'uint64',
'precopy-bytes' : 'uint64', 'downtime-bytes' : 'uint64',
-   'postcopy-bytes' : 'uint64' } }
+   'postcopy-bytes' : 'uint64',
+   'dirty-sync-missed-zero-copy' : 'uint64' } }
 
 ##
 # @XBZRLECacheStats:
diff --git a/migration/migration.c b/migration/migration.c
index 78f5057373..048f7f8bdb 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1027,6 +1027,8 @@ static void populate_ram_info(MigrationInfo *info, 
MigrationState *s)
 info->ram->normal_bytes = ram_counters.normal * page_size;
 info->ram->mbps = s->mbps;
 info->ram->dirty_sync_count = ram_counters.dirty_sync_count;
+info->ram->dirty_sync_missed_zero_copy =
+ram_counters.dirty_sync_missed_zero_copy;
 info->ram->postcopy_requests = ram_counters.postcopy_requests;
 info->ram->page_size = page_size;
 info->ram->multifd_bytes = ram_counters.multifd_bytes;
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index ca98df0495..5f3be9e405 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -307,6 +307,10 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
 monitor_printf(mon, "postcopy ram: %" PRIu64 " kbytes\n",
info->ram->postcopy_bytes >> 10);
 }
+if (info->ram->dirty_sync_missed_zero_copy) {
+monitor_printf(mon, "missed zero-copy on: %" PRIu64 " 
iterations\n",
+   info->ram->dirty_sync_missed_zero_copy);
+}
 }
 
 if (info->has_disk) {
-- 
2.36.1

[PATCH v3 1/3] QIOChannelSocket: Fix zero-copy flush returning code 1 when nothing sent

[Leonardo Bras]

If flush is called when no buffer was sent with MSG_ZEROCOPY, it currently
returns 1. This return code should be used only when Linux fails to use
MSG_ZEROCOPY on a lot of sendmsg().

Fix this by returning early from flush if no sendmsg(...,MSG_ZEROCOPY)
was attempted.

Fixes: 2bc58ffc2926 ("QIOChannelSocket: Implement io_writev zero copy flag & 
io_flush for CONFIG_LINUX")
Signed-off-by: Leonardo Bras 
---
 io/channel-socket.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/io/channel-socket.c b/io/channel-socket.c
index 4466bb1cd4..698c086b70 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -716,12 +716,18 @@ static int qio_channel_socket_flush(QIOChannel *ioc,
 struct cmsghdr *cm;
 char control[CMSG_SPACE(sizeof(*serr))];
 int received;
-int ret = 1;
+int ret;
+
+if (!sioc->zero_copy_queued) {
+return 0;
+}
 
 msg.msg_control = control;
 msg.msg_controllen = sizeof(control);
 memset(control, 0, sizeof(control));
 
+ret = 1;
+
 while (sioc->zero_copy_sent < sioc->zero_copy_queued) {
 received = recvmsg(sioc->fd, , MSG_ERRQUEUE);
 if (received < 0) {
-- 
2.36.1

[PATCH v3 0/3] Zero copy improvements (QIOChannel + multifd)

[Leonardo Bras]

The first patch avoid spuriously returning 1 [*] when zero-copy flush is
attempted before any buffer was sent using MSG_ZEROCOPY.

[*] zero-copy not being used, even though it's enabled and supported
by kernel

The second patch introduces a new migration stat
(dirty-sync-missed-zero-copy) that will be used to keep track of [*]. 

The third patch keeps track of how many zero-copy flushes retured 1 [*]

Changes since v2:
- Documentation release number changed from 7.2 to 7.1
- migration stat renamed from zero-copy-copied to 
  dirty-sync-missed-zero-copy
- Updated documentation to make it more user-friendly

Changes since v1:
- Idea of using a warning replaced by using a migration stat counter

Leonardo Bras (3):
  QIOChannelSocket: Fix zero-copy flush returning code 1 when nothing
sent
  Add dirty-sync-missed-zero-copy migration stat
  migration/multifd: Warn user when zerocopy not working

 qapi/migration.json   | 7 ++-
 migration/ram.h   | 2 ++
 io/channel-socket.c   | 8 +++-
 migration/migration.c | 2 ++
 migration/multifd.c   | 2 ++
 migration/ram.c   | 5 +
 monitor/hmp-cmds.c| 4 
 7 files changed, 28 insertions(+), 2 deletions(-)

-- 
2.36.1

[PATCH 1/2] hw/i2c/aspeed: Allow machines to set I2CBus

[Peter Delevoryas]

In a multi-SoC board, we want to allow machines to construct shared
I2CBus's, so that we can have two SoC I2C controllers attached to a single
I2CBus. We already expose read-only access, this just adds a method for
setting and using an external I2CBus in the Aspeed I2C bus controller.

One issue is that in order to use these methods, the machine needs to reach
into the SoC and call these methods on the I2C controller, and we would
prefer to keep the abstraction at the SoC level. If we create a set of
"aspeed_soc_i2c_get_bus/set_bus" methods though, they will just be
one-liners that don't do anything interesting. I would prefer to avoid that
if possible, because that doesn't seem scalable if we need to do the same
thing for all of the peripherals later.

In addition, we are already reaching into the Aspeed SoC to access the flash
controller to determine the boot rom size, so there is a precedent that we
need to reach into SoC peripherals for data sometimes.

Signed-off-by: Peter Delevoryas 
---
 hw/i2c/aspeed_i2c.c | 16 +++-
 include/hw/i2c/aspeed_i2c.h |  1 +
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
index 42c6d69b82..00bf58c7a3 100644
--- a/hw/i2c/aspeed_i2c.c
+++ b/hw/i2c/aspeed_i2c.c
@@ -1236,7 +1236,12 @@ static void aspeed_i2c_bus_realize(DeviceState *dev, 
Error **errp)
 
 sysbus_init_irq(SYS_BUS_DEVICE(dev), >irq);
 
-s->bus = i2c_init_bus(dev, name);
+/*
+ * If a bus hasn't been provided to the controller, create one from 
scratch.
+ */
+if (!s->bus) {
+s->bus = i2c_init_bus(dev, name);
+}
 s->slave = i2c_slave_create_simple(s->bus, TYPE_ASPEED_I2C_BUS_SLAVE,
0xff);
 
@@ -1420,3 +1425,12 @@ I2CBus *aspeed_i2c_get_bus(AspeedI2CState *s, int busnr)
 
 return bus;
 }
+
+void aspeed_i2c_set_bus(AspeedI2CState *s, int busnr, I2CBus *bus)
+{
+AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(s);
+
+if (busnr >= 0 && busnr < aic->num_busses) {
+s->busses[busnr].bus = bus;
+}
+}
diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
index 300a89b343..c60f8b291d 100644
--- a/include/hw/i2c/aspeed_i2c.h
+++ b/include/hw/i2c/aspeed_i2c.h
@@ -376,5 +376,6 @@ static inline bool aspeed_i2c_bus_is_enabled(AspeedI2CBus 
*bus)
 }
 
 I2CBus *aspeed_i2c_get_bus(AspeedI2CState *s, int busnr);
+void aspeed_i2c_set_bus(AspeedI2CState *s, int busnr, I2CBus *bus);
 
 #endif /* ASPEED_I2C_H */
-- 
2.37.0

[PATCH 2/2] fby35: Connect BMC to slot 0 BIC over I2C

[Peter Delevoryas]

Signed-off-by: Peter Delevoryas 
---
 hw/arm/fby35.c | 24 +++-
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
index 1972948318..88923d88eb 100644
--- a/hw/arm/fby35.c
+++ b/hw/arm/fby35.c
@@ -15,6 +15,7 @@
 #include "hw/arm/aspeed_soc.h"
 #include "hw/arm/boot.h"
 #include "hw/arm/fby35.h"
+#include "hw/i2c/i2c.h"
 #include "hw/i2c/i2c_mux_pca954x.h"
 
 #define TYPE_FBY35 MACHINE_TYPE_NAME("fby35")
@@ -83,7 +84,6 @@ static void fby35_bmc_init(Fby35State *s)
 memory_region_init_ram(>bmc_dram, OBJECT(s), "bmc-dram",
FBY35_BMC_RAM_SIZE, _abort);
 
-object_initialize_child(OBJECT(s), "bmc", >bmc, "ast2600-a3");
 object_property_set_int(OBJECT(>bmc), "ram-size", FBY35_BMC_RAM_SIZE,
 _abort);
 object_property_set_link(OBJECT(>bmc), "memory", OBJECT(>bmc_memory),
@@ -129,7 +129,6 @@ static void fby35_bic_init(Fby35State *s)
 
 memory_region_init(>bic_memory, OBJECT(s), "bic-memory", UINT64_MAX);
 
-object_initialize_child(OBJECT(s), "bic", >bic, "ast1030-a1");
 qdev_connect_clock_in(DEVICE(>bic), "sysclk", s->bic_sysclk);
 object_property_set_link(OBJECT(>bic), "memory", OBJECT(>bic_memory),
  _abort);
@@ -167,20 +166,27 @@ void fby35_cl_bic_i2c_init(AspeedSoCState *s)
 for (int i = 0; i < 8; i++) {
 i2c_slave_create_simple(ssd[i], "tmp105", 0x6a);
 }
-
-/*
- * FIXME: This should actually be the BMC, but both the ME and the BMC
- * are IPMB endpoints, and the current ME implementation is generic
- * enough to respond normally to some things.
- */
-i2c_slave_create_simple(i2c[6], "intel-me", 0x10);
 }
 
 static void fby35_init(MachineState *machine)
 {
 Fby35State *s = FBY35(machine);
+I2CBus *slot_i2c[4];
+
+object_initialize_child(OBJECT(s), "bmc", >bmc, "ast2600-a3");
+object_initialize_child(OBJECT(s), "bic", >bic, "ast1030-a1");
 
 fby35_bmc_init(s);
+
+for (int i = 0; i < ARRAY_SIZE(slot_i2c); i++) {
+slot_i2c[i] = aspeed_i2c_get_bus(>bmc.i2c, i);
+}
+
+/*
+ * There are 4 server board slots in fby35, and the first 4 I2C buses of 
the
+ * BMC are routed to each server board's BIC.
+ */
+aspeed_i2c_set_bus(>bic.i2c, 6, slot_i2c[0]);
 fby35_bic_init(s);
 }
 
-- 
2.37.0

Re: [RFC 0/8] Introduce an extensible static analyzer

[Alberto Faria]

On Mon, Jul 4, 2022 at 5:28 PM Daniel P. Berrangé  wrote:
> Have you done any measurement see how much of the overhead is from
> the checks you implemented, vs how much is inherantly forced on us
> by libclang ? ie what does it look like if you just load the libclang
> framework and run it actross all source files, without doing any
> checks in python.

Running the script with all checks disabled, i.e., doing nothing after
TranslationUnit.from_source():

$ time ./static-analyzer.py build
[...]
Analyzed 8775 translation units in 274.0 seconds.

real4m34.537s
user49m32.555s
sys 1m18.731s

$ time ./static-analyzer.py build block util
Analyzed 162 translation units in 4.2 seconds.

real0m4.804s
user0m40.389s
sys 0m1.690s

This is still with 12 threads on a 12-hardware thread laptop, but
scalability is near perfect. (The time reported by the script doesn't
include loading and inspection of the compilation database.)

So, not great. What's more, TranslationUnit.from_source() delegates
all work to clang_parseTranslationUnit(), so I suspect C libclang
wouldn't do much better.

And with all checks enabled:

$ time ./static-analyzer.py build block util
[...]
Analyzed 162 translation units in 86.4 seconds.

real1m26.999s
user14m51.163s
sys 0m2.205s

Yikes. Also not great at all, although the current implementation does
many inefficient things, like redundant AST traversals. Cutting
through some of clang.cindex's abstractions should also help, e.g.,
using libclang's visitor API properly instead of calling
clang_visitChildren() for every get_children().

Perhaps we should set a target for how slow we can tolerate this thing
to be, as a percentage of total build time, and determine if the
libclang approach is viable. I'm thinking maybe 10%?

> If i run 'clang-tidy' across the entire source tree, it takes 3 minutes
> on my machine, but there's overhead of repeatedly starting the process
> in there.

Is that parallelized in some way? It seems strange that clang-tidy
would be so much faster than libclang.

> I think anything that actually fully parses the source files is going
> to have a decent sized unavoidable overhead, when run across the whole
> source tree.
>
> Still having a properly parsed abstract source tree is an inherantly
> useful thing. for certain types of static analysis check. Some things
> get unreliable real fast if you try to anlyse using regex matches and
> similar approaches that are the common alternative. So libclang is
> understandably appealing in this respect.
>
> > The script takes a path to the build directory, and any number of paths
> > to directories or files to analyze. Example run on a 12-thread laptop:
> >
> > $ time ./static-analyzer.py build block
> > block/commit.c:525:15: non-coroutine_fn function calls coroutine_fn
> > block/nbd.c:206:5: non-coroutine_fn function calls coroutine_fn
> > [...]
> > block/ssh.c:1167:13: non-coroutine_fn function calls coroutine_fn
> > block/nfs.c:229:27: non-coroutine_fn function calls coroutine_fn
> > Analyzed 79 translation units.
> >
> > real0m45.277s
> > user7m55.496s
> > sys 0m1.445s
>
> Ouch, that is incredibly slow :-(

Yup :-(

Alberto

Re: [PATCH v8 00/12] qtests: Check accelerator available at runtime via QMP 'query-accels'

[Antonio Huete Jimenez]

Are all these changes OK or is there anything else missing? I was 
thinking in adding a QMP 'query-nvmm' command but I got pointed to this 
thread as a better alternative to having a per-accelerator query command.

Re: [RFC 5/8] static-analyzer: Enforce coroutine_fn restrictions on function pointers

[Víctor Colombo]

On 04/07/2022 15:04, Alberto Faria wrote:

On Mon, Jul 4, 2022 at 6:46 PM Víctor Colombo
 wrote:

Yes, this line is present at the beginning of the output
Is this caused by problems with the code being analyzed or is it because
libclang is getting confused with something that is outside of our
control?


I think I found the problem: the commands in the compilation database
weren't being parsed properly. I switched to shlex.split() and it
seems to be working now. The WIP v2 is available at [1], if you want
to give it a try.

Thanks for reporting this!

Alberto

[1] https://gitlab.com/albertofaria/qemu/-/tree/static-analysis



I tested the version from the WIP v2 and seems to be working now.
Thanks!

--
Víctor Cora Colombo
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

Re: [PATCH v2 2/3] Add zero-copy-copied migration stat

[Leonardo Brás]

Thanks Daniel, Markus and Dave for the feedback!

On Mon, 2022-07-04 at 14:14 +0100, Daniel P. Berrangé wrote:
> On Mon, Jul 04, 2022 at 02:59:50PM +0200, Markus Armbruster wrote:
> > Daniel P. Berrangé  writes:
> > 
> > > On Mon, Jul 04, 2022 at 02:04:41PM +0200, Markus Armbruster wrote:
> > 
[...]

> 
> since 7.1, unless you're planning for really tortuous review :)
> 

Ok, updated :)

> > [...]
> > 
> > > > Please rephrase the documentation of @zero-copy-copied in terms of
> > > > @dirty-sync-count.  Here's my attempt.
> > > > 
> > > > # @zero-copy-copied: Number of times dirty RAM synchronization could not
> > > > #    avoid copying zero pages.  This is between 0 and
> > > > #    @dirty-sync-count.  (since 7.1)

That's a great description! And it's almost 100% correct.

IIUC dirty-sync-count increments on migration_bitmap_sync() once after each full
dirty-bitmap scan, and even with multifd syncing at the same point, it could
potentially have a increment per multifd channel.

The only change would be having:
# This is between 0 and @dirty-sync-count * @multifd-channel.


> > > 
> > > Any one have preferences on the name - i get slight put off by the
> > > repeated word in the property name here.
> > > 
> > >    @zero-copy-rejects ?
> > >    @zero-copy-misses ?
> > >    @zero-copy-fails ?
> > 
> > I'd consider any of these an improvement.  Not a native speaker, but
> > perhaps "failures" instead of "fails".
> > 
> > We could also express the connection to @dirty-sync-count right in the
> > name, like @dirty-sync-rejected-zero-copy, @dirty-sync-missed-zero-copy,
> > @dirty-sync-failed-zero-copy.  Or maybe -copies.
> 
> Yeah, @dirty-sync-missed-zero-copy   is probably my favourite.

Ok then, this one there is :)

I will update my patchset with the suggestions, and I should publish a v3
shortly. 

Best regards,
Leo

> 
> With regards,
> Daniel

Re: [RFC 5/8] static-analyzer: Enforce coroutine_fn restrictions on function pointers

[Alberto Faria]

On Mon, Jul 4, 2022 at 6:46 PM Víctor Colombo
 wrote:
> Yes, this line is present at the beginning of the output
> Is this caused by problems with the code being analyzed or is it because
> libclang is getting confused with something that is outside of our
> control?

I think I found the problem: the commands in the compilation database
weren't being parsed properly. I switched to shlex.split() and it
seems to be working now. The WIP v2 is available at [1], if you want
to give it a try.

Thanks for reporting this!

Alberto

[1] https://gitlab.com/albertofaria/qemu/-/tree/static-analysis

Re: [RFC 5/8] static-analyzer: Enforce coroutine_fn restrictions on function pointers

[Víctor Colombo]

On 04/07/2022 13:57, Alberto Faria wrote:

Hi Víctor,

On Mon, Jul 4, 2022 at 3:18 PM Víctor Colombo
 wrote:

And I receive an exception on the line above saying that node is of type
NoneType. Seems that `node = node.referenced` is setting `node` to None
in this case.

I was unable to understand the root cause of it. Is this an incorrect
usage of the tool from my part? Full error message below


Unfortunately there seem to be a lot of corner cases that libclang can
throw at us. I hadn't come across this one before. I expected that
DECL_REF_EXPR/MEMBER_REF_EXPR would always reference something.

This may be due to some build error -- libclang tries to continue
processing a translation unit by dropping subtrees or nodes that have
problems. Is there a "too many errors emitted, stopping now; this may
lead to false positives and negatives" line at the top of the script's
output?



Yes, this line is present at the beginning of the output
Is this caused by problems with the code being analyzed or is it because
libclang is getting confused with something that is outside of our
control?

--
Víctor Cora Colombo
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

Re: [PATCH 2/9] target/ppc: add errp to kvmppc_read_int_cpu_dt()

[Cédric Le Goater]

On 7/2/22 15:34, Daniel Henrique Barboza wrote:



On 7/2/22 03:24, Cédric Le Goater wrote:

On 6/30/22 21:42, Daniel Henrique Barboza wrote:

The function can't just return 0 whether an error happened and call it a
day. We must provide a way of letting callers know if the zero return is
legitimate or due to an error.

Add an Error pointer to kvmppc_read_int_cpu_dt() that will be filled
with an appropriate error, if one occurs. Callers are then free to pass
an Error pointer and handle it.

Signed-off-by: Daniel Henrique Barboza 
---
  target/ppc/kvm.c | 16 +---
  1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 109823136d..bc17437097 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -1925,15 +1925,17 @@ static uint64_t kvmppc_read_int_dt(const char *filename)
  /*
   * Read a CPU node property from the host device tree that's a single
- * integer (32-bit or 64-bit).  Returns 0 if anything goes wrong
- * (can't find or open the property, or doesn't understand the format)
+ * integer (32-bit or 64-bit).  Returns 0 and set errp if anything goes
+ * wrong (can't find or open the property, or doesn't understand the
+ * format)
   */
-static uint64_t kvmppc_read_int_cpu_dt(const char *propname)
+static uint64_t kvmppc_read_int_cpu_dt(const char *propname, Error **errp)
  {
  char buf[PATH_MAX], *tmp;
  uint64_t val;
  if (kvmppc_find_cpu_dt(buf, sizeof(buf))) {
+    error_setg(errp, "Failed to read CPU property %s", propname);
  return 0;
  }
@@ -1946,12 +1948,12 @@ static uint64_t kvmppc_read_int_cpu_dt(const char 
*propname)
  uint64_t kvmppc_get_clockfreq(void)
  {
-    return kvmppc_read_int_cpu_dt("clock-frequency");
+    return kvmppc_read_int_cpu_dt("clock-frequency", NULL);



This should be fatal. no ?



I'm not sure. I went under the assumption that there might be some weird
condition where 'clock-frequency' might be missing from the DT, and this
is why we're not exiting out immediately.

That said, the advantage of turning this into fatal is that we won't need
all the other patches that handles error on this function. We're going to
assume that if 'clock-frequency' is missing then we can't boot. I'm okay
with that.


I think so. Some machines behave badly when 'clock-frequency' is bogus,
division by zero, no console, etc. We could check easily with pseries
which is the only KVM PPC platform.

C.

Re: [RFC 5/8] static-analyzer: Enforce coroutine_fn restrictions on function pointers

[Alberto Faria]

Hi Víctor,

On Mon, Jul 4, 2022 at 3:18 PM Víctor Colombo
 wrote:
> And I receive an exception on the line above saying that node is of type
> NoneType. Seems that `node = node.referenced` is setting `node` to None
> in this case.
>
> I was unable to understand the root cause of it. Is this an incorrect
> usage of the tool from my part? Full error message below

Unfortunately there seem to be a lot of corner cases that libclang can
throw at us. I hadn't come across this one before. I expected that
DECL_REF_EXPR/MEMBER_REF_EXPR would always reference something.

This may be due to some build error -- libclang tries to continue
processing a translation unit by dropping subtrees or nodes that have
problems. Is there a "too many errors emitted, stopping now; this may
lead to false positives and negatives" line at the top of the script's
output?

Thanks,
Alberto

Re: [PATCH] multifd: Copy pages before compressing them with zlib

[Juan Quintela]

Ilya Leoshkevich  wrote:
> zlib_send_prepare() compresses pages of a running VM. zlib does not
> make any thread-safety guarantees with respect to changing deflate()
> input concurrently with deflate() [1].
>
> One can observe problems due to this with the IBM zEnterprise Data
> Compression accelerator capable zlib [2]. When the hardware
> acceleration is enabled, migration/multifd/tcp/plain/zlib test fails
> intermittently [3] due to sliding window corruption. The accelerator's
> architecture explicitly discourages concurrent accesses [4]:
>
> Page 26-57, "Other Conditions":
>
> As observed by this CPU, other CPUs, and channel
> programs, references to the parameter block, first,
> second, and third operands may be multiple-access
> references, accesses to these storage locations are
> not necessarily block-concurrent, and the sequence
> of these accesses or references is undefined.
>
> Mark Adler pointed out that vanilla zlib performs double fetches under
> certain circumstances as well [5], therefore we need to copy data
> before passing it to deflate().
>
> [1] https://zlib.net/manual.html
> [2] https://github.com/madler/zlib/pull/410
> [3] https://lists.nongnu.org/archive/html/qemu-devel/2022-03/msg03988.html
> [4] http://publibfp.dhe.ibm.com/epubs/pdf/a227832c.pdf
> [5] https://gitlab.com/qemu-project/qemu/-/issues/1099
>
> Signed-off-by: Ilya Leoshkevich 

Reviewed-by: Juan Quintela 

And now I wonder if we need this for zstd.

Once told that, compression (not multifd one) has always operated the
other way, sniff.

[PATCH] multifd: Copy pages before compressing them with zlib

[Ilya Leoshkevich]

zlib_send_prepare() compresses pages of a running VM. zlib does not
make any thread-safety guarantees with respect to changing deflate()
input concurrently with deflate() [1].

One can observe problems due to this with the IBM zEnterprise Data
Compression accelerator capable zlib [2]. When the hardware
acceleration is enabled, migration/multifd/tcp/plain/zlib test fails
intermittently [3] due to sliding window corruption. The accelerator's
architecture explicitly discourages concurrent accesses [4]:

Page 26-57, "Other Conditions":

As observed by this CPU, other CPUs, and channel
programs, references to the parameter block, first,
second, and third operands may be multiple-access
references, accesses to these storage locations are
not necessarily block-concurrent, and the sequence
of these accesses or references is undefined.

Mark Adler pointed out that vanilla zlib performs double fetches under
certain circumstances as well [5], therefore we need to copy data
before passing it to deflate().

[1] https://zlib.net/manual.html
[2] https://github.com/madler/zlib/pull/410
[3] https://lists.nongnu.org/archive/html/qemu-devel/2022-03/msg03988.html
[4] http://publibfp.dhe.ibm.com/epubs/pdf/a227832c.pdf
[5] https://gitlab.com/qemu-project/qemu/-/issues/1099

Signed-off-by: Ilya Leoshkevich 
---

v1: https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg06841.html
v1 -> v2: Rebase, mention Mark Adler's reply in the commit message.

 migration/multifd-zlib.c | 35 ++-
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
index 3a7ae44485..b6b22b7d1f 100644
--- a/migration/multifd-zlib.c
+++ b/migration/multifd-zlib.c
@@ -27,6 +27,8 @@ struct zlib_data {
 uint8_t *zbuff;
 /* size of compressed buffer */
 uint32_t zbuff_len;
+/* uncompressed buffer */
+uint8_t buf[];
 };
 
 /* Multifd zlib compression */
@@ -43,9 +45,18 @@ struct zlib_data {
  */
 static int zlib_send_setup(MultiFDSendParams *p, Error **errp)
 {
-struct zlib_data *z = g_new0(struct zlib_data, 1);
-z_stream *zs = >zs;
+/* This is the maximum size of the compressed buffer */
+uint32_t zbuff_len = compressBound(MULTIFD_PACKET_SIZE);
+size_t buf_len = qemu_target_page_size();
+struct zlib_data *z;
+z_stream *zs;
 
+z = g_try_malloc0(sizeof(struct zlib_data) + buf_len + zbuff_len);
+if (!z) {
+error_setg(errp, "multifd %u: out of memory for zlib_data", p->id);
+return -1;
+}
+zs = >zs;
 zs->zalloc = Z_NULL;
 zs->zfree = Z_NULL;
 zs->opaque = Z_NULL;
@@ -54,15 +65,8 @@ static int zlib_send_setup(MultiFDSendParams *p, Error 
**errp)
 error_setg(errp, "multifd %u: deflate init failed", p->id);
 return -1;
 }
-/* This is the maxium size of the compressed buffer */
-z->zbuff_len = compressBound(MULTIFD_PACKET_SIZE);
-z->zbuff = g_try_malloc(z->zbuff_len);
-if (!z->zbuff) {
-deflateEnd(>zs);
-g_free(z);
-error_setg(errp, "multifd %u: out of memory for zbuff", p->id);
-return -1;
-}
+z->zbuff_len = zbuff_len;
+z->zbuff = z->buf + buf_len;
 p->data = z;
 return 0;
 }
@@ -80,7 +84,6 @@ static void zlib_send_cleanup(MultiFDSendParams *p, Error 
**errp)
 struct zlib_data *z = p->data;
 
 deflateEnd(>zs);
-g_free(z->zbuff);
 z->zbuff = NULL;
 g_free(p->data);
 p->data = NULL;
@@ -114,8 +117,14 @@ static int zlib_send_prepare(MultiFDSendParams *p, Error 
**errp)
 flush = Z_SYNC_FLUSH;
 }
 
+/*
+ * Since the VM might be running, the page may be changing concurrently
+ * with compression. zlib does not guarantee that this is safe,
+ * therefore copy the page before calling deflate().
+ */
+memcpy(z->buf, p->pages->block->host + p->normal[i], page_size);
 zs->avail_in = page_size;
-zs->next_in = p->pages->block->host + p->normal[i];
+zs->next_in = z->buf;
 
 zs->avail_out = available;
 zs->next_out = z->zbuff + out_size;
-- 
2.35.3

Re: [RFC 0/8] Introduce an extensible static analyzer

[Daniel P . Berrangé]

On Sat, Jul 02, 2022 at 12:33:23PM +0100, Alberto Faria wrote:
> This series introduces a static analyzer for QEMU. It consists of a
> single static-analyzer.py script that relies on libclang's Python
> bindings, and provides a common framework on which arbitrary static
> analysis checks can be developed and run against QEMU's code base.
> 
> Summary of the series:
> 
>   - Patch 1 adds the base static analyzer, along with a simple check
> that finds static functions whose return value is never used, and
> patch 2 fixes some occurrences of this.
> 
>   - Patch 3 adds a check to ensure that non-coroutine_fn functions don't
> perform direct calls to coroutine_fn functions, and patch 4 fixes
> some violations of this rule.
> 
>   - Patch 5 adds a check to enforce coroutine_fn restrictions on
> function pointers, namely around assignment and indirect calls, and
> patch 6 fixes some problems it detects. (Implementing this check
> properly is complicated, since AFAICT annotation attributes cannot
> be applied directly to types. This part still needs a lot of work.)
> 
>   - Patch 7 introduces a no_coroutine_fn marker for functions that
> should not be called from coroutines, makes generated_co_wrapper
> evaluate to no_coroutine_fn, and adds a check enforcing this rule.
> Patch 8 fixes some violations that it finds.
> 
> The current primary motivation for this work is enforcing rules around
> block layer coroutines, which is why most of the series focuses on that.
> However, the static analyzer is intended to be sufficiently generic to
> satisfy other present and future QEMU static analysis needs.
> 
> This is very early work-in-progress, and a lot is missing. One notable
> omission is build system integration, including keeping track of which
> translation units have been modified and need re-analyzing.
> 
> Performance is bad, but there is a lot of potential for optimization,
> such as avoiding redundant AST traversals. Switching to C libclang is
> also a possibility, although Python makes it easy to quickly prototype
> new checks, which should encourage adoption and contributions.

Have you done any measurement see how much of the overhead is from
the checks you implemented, vs how much is inherantly forced on us
by libclang ? ie what does it look like if you just load the libclang
framework and run it actross all source files, without doing any
checks in python.

If i run 'clang-tidy' across the entire source tree, it takes 3 minutes
on my machine, but there's overhead of repeatedly starting the process
in there.

I think anything that actually fully parses the source files is going
to have a decent sized unavoidable overhead, when run across the whole
source tree.

Still having a properly parsed abstract source tree is an inherantly
useful thing. for certain types of static analysis check. Some things
get unreliable real fast if you try to anlyse using regex matches and
similar approaches that are the common alternative. So libclang is
understandably appealing in this respect.

> The script takes a path to the build directory, and any number of paths
> to directories or files to analyze. Example run on a 12-thread laptop:
> 
> $ time ./static-analyzer.py build block
> block/commit.c:525:15: non-coroutine_fn function calls coroutine_fn
> block/nbd.c:206:5: non-coroutine_fn function calls coroutine_fn
> [...]
> block/ssh.c:1167:13: non-coroutine_fn function calls coroutine_fn
> block/nfs.c:229:27: non-coroutine_fn function calls coroutine_fn
> Analyzed 79 translation units.
> 
> real0m45.277s
> user7m55.496s
> sys 0m1.445s

Ouch, that is incredibly slow :-(

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

Re: [PATCH 01/18] block: Make blk_{pread, pwrite}() return 0 on success

[Alberto Faria]

On Mon, Jul 4, 2022 at 2:52 PM Hanna Reitz  wrote:
> There are a couple of places where you decided to replace “*len”
> variables that used to store the return value by a plain “ret”. That
> seems good to me, given how these functions no longer return length
> values, but you haven’t done so consistently.  Below, I have noted
> places where this wasn’t done; I wonder why, but my R-b stands
> regardless of whether you change them too or not.

Thanks, this was just an oversight on my part. I'll fix it.

> > diff --git a/qemu-img.c b/qemu-img.c
> > index 4cf4d2423d..9d98ef63ac 100644
> > --- a/qemu-img.c
> > +++ b/qemu-img.c
> > @@ -5120,30 +5120,27 @@ static int img_dd(int argc, char **argv)
> >   in.buf = g_new(uint8_t, in.bsz);
> >
> >   for (out_pos = 0; in_pos < size; block_count++) {
> > -int in_ret, out_ret;
> > +int bytes, in_ret, out_ret;
> >
> > -if (in_pos + in.bsz > size) {
> > -in_ret = blk_pread(blk1, in_pos, in.buf, size - in_pos);
> > -} else {
> > -in_ret = blk_pread(blk1, in_pos, in.buf, in.bsz);
> > -}
> > +bytes = (in_pos + in.bsz > size) ? size - in_pos : in.bsz;
> > +
> > +in_ret = blk_pread(blk1, in_pos, in.buf, bytes);
> >   if (in_ret < 0) {
> >   error_report("error while reading from input image file: %s",
> >strerror(-in_ret));
> >   ret = -1;
> >   goto out;
> >   }
> > -in_pos += in_ret;
> > -
> > -out_ret = blk_pwrite(blk2, out_pos, in.buf, in_ret, 0);
> > +in_pos += bytes;
> >
> > +out_ret = blk_pwrite(blk2, out_pos, in.buf, bytes, 0);
> >   if (out_ret < 0) {
> >   error_report("error while writing to output image file: %s",
> >strerror(-out_ret));
> >   ret = -1;
> >   goto out;
> >   }
> > -out_pos += out_ret;
> > +out_pos += bytes;
> >   }
> >
> >   out:
>
> We could use this opportunity to drop in_ret and out_ret altogether (but
> we can also decide not to).

Dropping them sounds good to me.

Alberto

Re: [PATCH 5/5] multifd: Only sync once each full round of memory

[Juan Quintela]

Leonardo Brás  wrote:
> Hello Juan,
>
> On Tue, 2022-06-21 at 16:05 +0200, Juan Quintela wrote:
>> We need to add a new flag to mean to sync at that point.
>> Notice that we still synchronize at the end of setup and at the end of
>> complete stages.
>> 
>> Signed-off-by: Juan Quintela 
>> ---
>>  migration/migration.c |  2 +-
>>  migration/ram.c   | 42 ++
>>  2 files changed, 31 insertions(+), 13 deletions(-)
>> 




>> @@ -2943,11 +2953,13 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>>  ram_control_before_iterate(f, RAM_CONTROL_SETUP);
>>  ram_control_after_iterate(f, RAM_CONTROL_SETUP);
>>  
>> -    if (migrate_multifd_sync_each_iteration()) {
>> -    ret =  multifd_send_sync_main(f);
>> -    if (ret < 0) {
>> -    return ret;
>> -    }
>
> (1) IIUC, the above lines were changed in 2/5 to be reverted now.
> Is that correct? was it expected?

Yeap.  The problem here is that (withouth this patchset) we synchrconize
in three places:

- ram_save_setup()
- ram_save_iterate()
- ram_save_complete()

And we want to change it to:

- ram_save_setup()
- ram_save_complete()
- And each time that we pass through the end of memory. (much less times
  than calls to ram_save_iterate).

In the three places we send:

   RAM_SAVE_FLAG_EOS

And that is what cause the synchronization.

As we can't piggyback on RAM_SAVE_FLAG_EOS anymore, I added a new flag
to synchronize it.

The problem is that now (on setup and complete)  we need to synchronize
independently if we do the older way or the new one.  On iterate we only
synchronize on the old code, and on new code only when we reach the end
of the memory.

I *thought* it was clear this way, but I can do without the change if
people think it is easier.


>> +    ret =  multifd_send_sync_main(f);
>> +    if (ret < 0) {
>> +    return ret;
>> +    }
>> +
>> +    if (!migrate_multifd_sync_each_iteration()) {
>> +    qemu_put_be64(f, RAM_SAVE_FLAG_MULTIFD_SYNC);
>
> (2) I have done some testing with this patchset (because of MSG_ZEROCOPY) and 
> it
> seems this part here is breaking migration from this build to 'older' builds
> (same commits except for this patchset):
>
> qemu-system-x86_64: Unknown combination of migration flags: 0x200
> qemu-system-x86_64: error while loading state section id 2(ram)
> qemu-system-x86_64: load of migration failed: Invalid argument

You can't do that O:-) (TM), that is the whole point that I added the
multifd-sync-each-iteration property.  It is true for old machine types,
it is false for new machine types.  If you try to play with that
property, it is better than you know what you are doing (TM).

> Which makes sense, since there is no RAM_SAVE_FLAG_MULTIFD_SYNC in older
> versions. Is this expected / desired ?

See previous paragraph.

> Strange enough, it seems to be breaking even with this set in the sending 
> part: 
> --global migration.multifd-sync-each-iteration=on
>
> Was the idea of this config to allow migration to older qemu builds?

If you set it to on, it should work against and old qemu.  By default it
is set to on for old machine types, and only to on for new machine
types.  So you should have it right if you use new machine types.  (If
you use "pc" or "q35" machine types, you should now what you are doing
for migrating machines.  We do this kind of change all the time).

>>  }
>>  qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
>>  qemu_fflush(f);
>> @@ -3127,13 +3139,14 @@ static int ram_save_complete(QEMUFile *f, void
>> *opaque)
>>  return ret;
>>  }
>>  
>> -    if (migrate_multifd_sync_each_iteration()) {
>> -    ret = multifd_send_sync_main(rs->f);
>> -    if (ret < 0) {
>> -    return ret;
>> -    }
>> +    ret = multifd_send_sync_main(rs->f);
>> +    if (ret < 0) {
>> +    return ret;
>>  }
>
> (3) Same as (1)

Yeap, this is on purpose.  If you feel that it is clearer the other way,
I can change the patchset.

Later, Juan.

Re: [PATCH v2 7/7] tests/style: check qemu/osdep.h is included in all .c files

[Daniel P . Berrangé]

On Mon, Jul 04, 2022 at 04:55:45PM +0100, Peter Maydell wrote:
> On Mon, 4 Jul 2022 at 16:50, Daniel P. Berrangé  wrote:
> >
> > On Mon, Jul 04, 2022 at 04:47:16PM +0100, Peter Maydell wrote:
> > > On Mon, 4 Jul 2022 at 16:23, Daniel P. Berrangé  
> > > wrote:
> > > >
> > > > Signed-off-by: Daniel P. Berrangé 
> > >
> > > > +
> > > > +sc_c_file_osdep_h:
> > > > +   @require='#include "qemu/osdep.h"' \
> > > > +   in_vc_files='\.c$$' \
> > > > +   halt='all C files must include qemu/osdep.h' \
> > > > +   $(_sc_search_regexp)
> > >
> > > The rule is not just "included in all C files", but "included
> > > as the *first* include in all C files".
> >
> > Oh right, so we can copy a rule from libvirt to validate that.
> >
> > It would look like this, but s,config.h,qemu/osdep.h,
> >
> >
> > # Print each file name for which the first #include does not match
> > # $(config_h_header).  Like grep -m 1, this only looks at the first match.
> > perl_config_h_first_ = \
> >   -e 'BEGIN {$$ret = 0}' \
> >   -e 'if (/^\# *include\b/) {' \
> >   -e '  if (not m{^\# *include $(config_h_header)}) {' \
> >   -e 'print "$$ARGV\n";' \
> >   -e '$$ret = 1;' \
> >   -e '  }' \
> >   -e '  \# Move on to next file after first include' \
> >   -e '  close ARGV;' \
> >   -e '}' \
> >   -e 'END {exit $$ret}'
> >
> > # You must include  before including any other header file.
> > # This can possibly be via a package-specific header, if given by 
> > syntax-check.mk.
> > sc_require_config_h_first:
> > @if $(VC_LIST_EXCEPT) | $(GREP) '\.c$$' > /dev/null; then \
> >   files=$$($(VC_LIST_EXCEPT) | $(GREP) '\.c$$') && \
> >   perl -n $(perl_config_h_first_) $$files || \
> > { echo 'the above files include some other header' \
> > 'before ' 1>&2; exit 1; } || :; \
> > else :; \
> > fi
> 
> As an example syntax checking rule I think this makes a pretty
> convincing case for the argument "make is the wrong language/framework
> for this job"...

Matching contextually across multiple lines of text is admittedly hard.
IME most of the usage of this syntax checking facility we had in libvirt
can be handled using single line matches, which are trivial to provide.

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

Re: [PATCH v2 1/7] tests: introduce tree-wide code style checking

[Daniel P . Berrangé]

On Mon, Jul 04, 2022 at 04:46:53PM +0100, Peter Maydell wrote:
> On Mon, 4 Jul 2022 at 16:23, Daniel P. Berrangé  wrote:
> >
> > Historically QEMU has used the 'scripts/checkpatch.pl' script to
> > validate various style rules but there are a number of issues:
> 
> >  meson.build  |   3 +
> >  tests/Makefile.include   |   3 +-
> >  tests/meson.build|  19 +++
> >  tests/style-excludes.mak |   4 +
> >  tests/style-infra.mak| 265 +++
> >  tests/style.mak  |  24 
> 
> From my point of view the main issue with checkpatch.pl is
> that nobody in the QEMU developers particularly understands
> it or is enthusiastic about trying to add more tests to it
> or adjust the existing ones where QEMU style diverges from
> the kernel style (but nor are we tracking and upgrading to
> newer versions of the kernel's script).
> 
> This seems to be aiming to replace a complex and hard to
> understand perl script with a complex and hard to understand
> makefile. I can't say I'm terribly enthusiastic :-/

I think the downsides comapred here are rather different orders of
magnitude. The checkpatch.pl script is 3000 lines of code where we
have years of experiance that no one in QEMU likes touching it.

The makefile here is 265 lines of which 50% is comments of license
text.  In terms of what contributors most care about though, is
how you add new rules, and most of the time that's involves just
adding a 3 line make rule based off a regex to match the code
pattern you want to prohibit. Some of this is a bit crufty to
look at, but we've got years of experiance in libvirt with many
contributors frequently adding new tests.

It only gets hairy if the pattern you're trying to forbid needs
to match across multiple lines of text - hence the difference in
complexity between matching 'osdep.h' exists in .c, vs 'osdep.h'
exists as the very first #include.  IME, the single-line matches
are most typical need that is addressed.

So while I wont claim this proposal is perfect, IMHO this would
be a significant step fowards over checkpatch.pl.

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

Re: [PATCH v2 6/7] misc: ensure qemu/osdep.h is included in all .c files

[Warner Losh]

On Mon, Jul 4, 2022, 9:46 AM Daniel P. Berrangé  wrote:

> On Mon, Jul 04, 2022 at 09:38:46AM -0600, Warner Losh wrote:
> > On Mon, Jul 4, 2022 at 9:28 AM Daniel P. Berrangé 
> > wrote:
> >
> > > A few files relied on qemu/osdep.h being included via a common
> > > header. Another file didn't need it because it was actually an
> > > included file, so ought to have been named .c.inc
> > >
> > > Signed-off-by: Daniel P. Berrangé 
> > > ---
> > >  bsd-user/arm/signal.c | 2 ++
> > >  bsd-user/arm/target_arch_cpu.c| 3 +++
> > >  bsd-user/{elfcore.c => elfcore.c.inc} | 0
> > >  bsd-user/elfload.c| 2 +-
> > >  bsd-user/freebsd/os-sys.c | 2 ++
> > >  bsd-user/i386/signal.c| 2 ++
> > >  bsd-user/qemu.h   | 1 -
> > >  bsd-user/x86_64/signal.c  | 2 ++
> > >  crypto/rsakey.c   | 1 +
> > >  qga/cutils.c  | 2 ++
> > >  10 files changed, 15 insertions(+), 2 deletions(-)
> > >  rename bsd-user/{elfcore.c => elfcore.c.inc} (100%)
> > >
> >
> > The change to bsd-user is fine, though will cause many ripples in the
> > upstream
> > branch when I merge it. The ripples likely are worth it in the long run,
> > and knowing
> > they are coming and helps me prepare the tree for the merge.
>
> If you prefer to delay these changes I don't mind. It just means that
> it would need a 'bsd-user/.*' exclude rule in the next patch to
> temporarily skip this chck for bsd-user code.
>

No. Go ahead. I know I need to do something when I do the next merge.
And it shouldn't be a big deal to do now, otherwise I'll forget...

Warner


> It also reminds me that once I'm done upstreaming, there's likely benefit
> > from having
> > a common elf loader / core generator as much of this code is copied from
> > linux-user
> > with the qemu style layered on top
> >
> > Reviewed-by: Warner Losh 
>
>
> With regards,
> Daniel
> --
> |: https://berrange.com  -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-
> https://www.instagram.com/dberrange :|
>
>

Re: [PATCH 1/5] multifd: Create property multifd-sync-each-iteration

[Juan Quintela]

"Dr. David Alan Gilbert"  wrote:
> * Juan Quintela (quint...@redhat.com) wrote:
>> We used to synchronize all channels at the end of each RAM section
>> sent.  That is not needed, so preparing to only synchronize once every
>> full round in latests patches.
>> 
>> Notice that we initialize the property as true.  We will change the
>> default when we introduce the new mechanism.
>
> I don't understand why this is a property - does it break the actual
> stream format?

Yeap.

You can see on following patches.  The problem is that we synchronize
each time that we sent/receive a

RAM_SAVE_FLAG_EOS

And that is way too much.  As Leo showed, it can be as much as 20 times
a second.

Later, Juan.

Re: [PATCH 00/62] target/arm: Implement FEAT_HAFDBS

[Peter Maydell]

On Mon, 4 Jul 2022 at 15:58, Richard Henderson
 wrote:
>
> On 7/4/22 20:24, Peter Maydell wrote:
> >> Previously, we had A-profile allocate separate mmu_idx for secure
> >> vs non-secure.  I've done away with that.  Now, I flush all mmu_idx
> >> when SCR_EL3.NS is changed.  I did not see how we could reasonably
> >> add 8 more mmu_idx for Realm.  Moreover, I had a look through ARM
> >> Trusted Firmware, at the code paths used to change between Secure
> >> and Nonsecure.  We wind up flushing all of these mmu_idx anyway while
> >> swapping the EL1+EL2 cpregs, so there is no gain at all in attempting
> >> to keep them live at the same time within qemu.
> >
> > Is there no SMC/interrupt/etc at all which is handled as a "just do the
> > thing at EL3" without dropping down to secure EL2/EL1 ?
>
> I'm sure there is, but it's only swapping between S EL[012] and NS EL[012] 
> that concerned
> me.  Is there something that I'm missing?

Oh, right, EL3 remains its own mmu_idx, of course. (And I guess
also Monitor mode for AArch32 EL3, though the degree to which we
care about performance of emulation there is decreasing I suspect.)

thanks
-- PMM

Re: [PATCH v2 7/7] tests/style: check qemu/osdep.h is included in all .c files

[Peter Maydell]

On Mon, 4 Jul 2022 at 16:50, Daniel P. Berrangé  wrote:
>
> On Mon, Jul 04, 2022 at 04:47:16PM +0100, Peter Maydell wrote:
> > On Mon, 4 Jul 2022 at 16:23, Daniel P. Berrangé  wrote:
> > >
> > > Signed-off-by: Daniel P. Berrangé 
> >
> > > +
> > > +sc_c_file_osdep_h:
> > > +   @require='#include "qemu/osdep.h"' \
> > > +   in_vc_files='\.c$$' \
> > > +   halt='all C files must include qemu/osdep.h' \
> > > +   $(_sc_search_regexp)
> >
> > The rule is not just "included in all C files", but "included
> > as the *first* include in all C files".
>
> Oh right, so we can copy a rule from libvirt to validate that.
>
> It would look like this, but s,config.h,qemu/osdep.h,
>
>
> # Print each file name for which the first #include does not match
> # $(config_h_header).  Like grep -m 1, this only looks at the first match.
> perl_config_h_first_ = \
>   -e 'BEGIN {$$ret = 0}' \
>   -e 'if (/^\# *include\b/) {' \
>   -e '  if (not m{^\# *include $(config_h_header)}) {' \
>   -e 'print "$$ARGV\n";' \
>   -e '$$ret = 1;' \
>   -e '  }' \
>   -e '  \# Move on to next file after first include' \
>   -e '  close ARGV;' \
>   -e '}' \
>   -e 'END {exit $$ret}'
>
> # You must include  before including any other header file.
> # This can possibly be via a package-specific header, if given by 
> syntax-check.mk.
> sc_require_config_h_first:
> @if $(VC_LIST_EXCEPT) | $(GREP) '\.c$$' > /dev/null; then \
>   files=$$($(VC_LIST_EXCEPT) | $(GREP) '\.c$$') && \
>   perl -n $(perl_config_h_first_) $$files || \
> { echo 'the above files include some other header' \
> 'before ' 1>&2; exit 1; } || :; \
> else :; \
> fi

As an example syntax checking rule I think this makes a pretty
convincing case for the argument "make is the wrong language/framework
for this job"...

thanks
-- PMM

Re: [PATCH v2 4/7] misc: fix commonly doubled up words

[Peter Maydell]

On Mon, 4 Jul 2022 at 16:23, Daniel P. Berrangé  wrote:
>
> Signed-off-by: Daniel P. Berrangé 
> ---

> --- a/docs/tools/qemu-pr-helper.rst
> +++ b/docs/tools/qemu-pr-helper.rst
> @@ -22,7 +22,7 @@ storage fabric. QEMU's SCSI passthrough devices 
> ``scsi-block``
>  and ``scsi-generic`` support passing guest persistent reservation
>  requests to a privileged external helper program. :program:`qemu-pr-helper`
>  is that external helper; it creates a socket which QEMU can
> -connect to to communicate with it.
> +connect to communicate with it.

This text is correct as it stands, and the change is wrong.

> diff --git a/tests/qtest/microbit-test.c b/tests/qtest/microbit-test.c
> index 04e199ec33..6cc1f5d932 100644
> --- a/tests/qtest/microbit-test.c
> +++ b/tests/qtest/microbit-test.c
> @@ -449,9 +449,9 @@ static void test_nrf51_timer(void)
>  timer_set_prescaler(qts, 0);
>  /* Swept over in first step */
>  timer_set_cc(qts, 0, 2);
> -/* Barely miss on first step */
> +/* Barely miss in first step */
>  timer_set_cc(qts, 1, 162);
> -/* Spot on on third step */
> +/* Spot on in third step */
>  timer_set_cc(qts, 2, 480);

These changes also look wrong.

The rest seems OK.

thanks
-- PMM

Re: [PATCH v2 7/7] tests/style: check qemu/osdep.h is included in all .c files

[Daniel P . Berrangé]

On Mon, Jul 04, 2022 at 04:47:16PM +0100, Peter Maydell wrote:
> On Mon, 4 Jul 2022 at 16:23, Daniel P. Berrangé  wrote:
> >
> > Signed-off-by: Daniel P. Berrangé 
> 
> > +
> > +sc_c_file_osdep_h:
> > +   @require='#include "qemu/osdep.h"' \
> > +   in_vc_files='\.c$$' \
> > +   halt='all C files must include qemu/osdep.h' \
> > +   $(_sc_search_regexp)
> 
> The rule is not just "included in all C files", but "included
> as the *first* include in all C files".

Oh right, so we can copy a rule from libvirt to validate that.

It would look like this, but s,config.h,qemu/osdep.h,


# Print each file name for which the first #include does not match
# $(config_h_header).  Like grep -m 1, this only looks at the first match.
perl_config_h_first_ = \
  -e 'BEGIN {$$ret = 0}' \
  -e 'if (/^\# *include\b/) {' \
  -e '  if (not m{^\# *include $(config_h_header)}) {' \
  -e 'print "$$ARGV\n";' \
  -e '$$ret = 1;' \
  -e '  }' \
  -e '  \# Move on to next file after first include' \
  -e '  close ARGV;' \
  -e '}' \
  -e 'END {exit $$ret}'

# You must include  before including any other header file.
# This can possibly be via a package-specific header, if given by 
syntax-check.mk.
sc_require_config_h_first:
@if $(VC_LIST_EXCEPT) | $(GREP) '\.c$$' > /dev/null; then \
  files=$$($(VC_LIST_EXCEPT) | $(GREP) '\.c$$') && \
  perl -n $(perl_config_h_first_) $$files || \
{ echo 'the above files include some other header' \
'before ' 1>&2; exit 1; } || :; \
else :; \
fi



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

Re: [PATCH v2 7/7] tests/style: check qemu/osdep.h is included in all .c files

[Peter Maydell]

On Mon, 4 Jul 2022 at 16:23, Daniel P. Berrangé  wrote:
>
> Signed-off-by: Daniel P. Berrangé 

> +
> +sc_c_file_osdep_h:
> +   @require='#include "qemu/osdep.h"' \
> +   in_vc_files='\.c$$' \
> +   halt='all C files must include qemu/osdep.h' \
> +   $(_sc_search_regexp)

The rule is not just "included in all C files", but "included
as the *first* include in all C files".

thanks
-- PMM

Re: [PATCH v2 1/7] tests: introduce tree-wide code style checking

[Peter Maydell]

On Mon, 4 Jul 2022 at 16:23, Daniel P. Berrangé  wrote:
>
> Historically QEMU has used the 'scripts/checkpatch.pl' script to
> validate various style rules but there are a number of issues:

>  meson.build  |   3 +
>  tests/Makefile.include   |   3 +-
>  tests/meson.build|  19 +++
>  tests/style-excludes.mak |   4 +
>  tests/style-infra.mak| 265 +++
>  tests/style.mak  |  24 

>From my point of view the main issue with checkpatch.pl is
that nobody in the QEMU developers particularly understands
it or is enthusiastic about trying to add more tests to it
or adjust the existing ones where QEMU style diverges from
the kernel style (but nor are we tracking and upgrading to
newer versions of the kernel's script).

This seems to be aiming to replace a complex and hard to
understand perl script with a complex and hard to understand
makefile. I can't say I'm terribly enthusiastic :-/

thanks
-- PMM

Re: [PATCH v2 6/7] misc: ensure qemu/osdep.h is included in all .c files

[Daniel P . Berrangé]

On Mon, Jul 04, 2022 at 09:38:46AM -0600, Warner Losh wrote:
> On Mon, Jul 4, 2022 at 9:28 AM Daniel P. Berrangé 
> wrote:
> 
> > A few files relied on qemu/osdep.h being included via a common
> > header. Another file didn't need it because it was actually an
> > included file, so ought to have been named .c.inc
> >
> > Signed-off-by: Daniel P. Berrangé 
> > ---
> >  bsd-user/arm/signal.c | 2 ++
> >  bsd-user/arm/target_arch_cpu.c| 3 +++
> >  bsd-user/{elfcore.c => elfcore.c.inc} | 0
> >  bsd-user/elfload.c| 2 +-
> >  bsd-user/freebsd/os-sys.c | 2 ++
> >  bsd-user/i386/signal.c| 2 ++
> >  bsd-user/qemu.h   | 1 -
> >  bsd-user/x86_64/signal.c  | 2 ++
> >  crypto/rsakey.c   | 1 +
> >  qga/cutils.c  | 2 ++
> >  10 files changed, 15 insertions(+), 2 deletions(-)
> >  rename bsd-user/{elfcore.c => elfcore.c.inc} (100%)
> >
> 
> The change to bsd-user is fine, though will cause many ripples in the
> upstream
> branch when I merge it. The ripples likely are worth it in the long run,
> and knowing
> they are coming and helps me prepare the tree for the merge.

If you prefer to delay these changes I don't mind. It just means that
it would need a 'bsd-user/.*' exclude rule in the next patch to
temporarily skip this chck for bsd-user code.

> It also reminds me that once I'm done upstreaming, there's likely benefit
> from having
> a common elf loader / core generator as much of this code is copied from
> linux-user
> with the qemu style layered on top
> 
> Reviewed-by: Warner Losh 


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

Re: [PATCH v2 2/7] misc: fix mixups of bool constants with int variables

[Peter Maydell]

On Mon, 4 Jul 2022 at 16:23, Daniel P. Berrangé  wrote:
>
> Signed-off-by: Daniel P. Berrangé 
> ---
>  block/vhdx-log.c   | 2 +-
>  hw/xtensa/sim.c| 4 ++--
>  nbd/client.c   | 4 ++--
>  target/i386/cpu-dump.c | 3 ++-
>  ui/spice-display.c | 4 ++--
>  5 files changed, 9 insertions(+), 8 deletions(-)

> diff --git a/nbd/client.c b/nbd/client.c
> index 30d5383cb1..fee3959d24 100644
> --- a/nbd/client.c
> +++ b/nbd/client.c
> @@ -832,8 +832,8 @@ static int nbd_list_meta_contexts(QIOChannel *ioc,
>Error **errp)
>  {
>  int ret;
> -int seen_any = false;
> -int seen_qemu = false;
> +bool seen_any = false;
> +bool seen_qemu = false;
>
>  if (nbd_send_meta_query(ioc, NBD_OPT_LIST_META_CONTEXT,
>  info->name, NULL, errp) < 0) {

The code in this function does a bitwise OR into seen_qemu later --
bitwise OR into a bool also seems like bad style that something
is probably going to complain about. (I guess currently -Wbool-operation
doesn't care about it or else we don't enable that, but it might
in future.)

The other changes look OK.

-- PMM

Re: [PATCH v2 6/7] misc: ensure qemu/osdep.h is included in all .c files

[Warner Losh]

On Mon, Jul 4, 2022 at 9:28 AM Daniel P. Berrangé 
wrote:

> A few files relied on qemu/osdep.h being included via a common
> header. Another file didn't need it because it was actually an
> included file, so ought to have been named .c.inc
>
> Signed-off-by: Daniel P. Berrangé 
> ---
>  bsd-user/arm/signal.c | 2 ++
>  bsd-user/arm/target_arch_cpu.c| 3 +++
>  bsd-user/{elfcore.c => elfcore.c.inc} | 0
>  bsd-user/elfload.c| 2 +-
>  bsd-user/freebsd/os-sys.c | 2 ++
>  bsd-user/i386/signal.c| 2 ++
>  bsd-user/qemu.h   | 1 -
>  bsd-user/x86_64/signal.c  | 2 ++
>  crypto/rsakey.c   | 1 +
>  qga/cutils.c  | 2 ++
>  10 files changed, 15 insertions(+), 2 deletions(-)
>  rename bsd-user/{elfcore.c => elfcore.c.inc} (100%)
>

The change to bsd-user is fine, though will cause many ripples in the
upstream
branch when I merge it. The ripples likely are worth it in the long run,
and knowing
they are coming and helps me prepare the tree for the merge.

It also reminds me that once I'm done upstreaming, there's likely benefit
from having
a common elf loader / core generator as much of this code is copied from
linux-user
with the qemu style layered on top

Reviewed-by: Warner Losh 


> diff --git a/bsd-user/arm/signal.c b/bsd-user/arm/signal.c
> index 2b1dd745d1..eca20ac4d7 100644
> --- a/bsd-user/arm/signal.c
> +++ b/bsd-user/arm/signal.c
> @@ -17,6 +17,8 @@
>   *  along with this program; if not, see .
>   */
>
> +#include "qemu/osdep.h"
> +
>  #include "qemu.h"
>
>  /*
> diff --git a/bsd-user/arm/target_arch_cpu.c
> b/bsd-user/arm/target_arch_cpu.c
> index 02bf9149d5..186cf43fb9 100644
> --- a/bsd-user/arm/target_arch_cpu.c
> +++ b/bsd-user/arm/target_arch_cpu.c
> @@ -16,6 +16,9 @@
>   *  You should have received a copy of the GNU General Public License
>   *  along with this program; if not, see .
>   */
> +
> +#include "qemu/osdep.h"
> +
>  #include "target_arch.h"
>
>  void target_cpu_set_tls(CPUARMState *env, target_ulong newtls)
> diff --git a/bsd-user/elfcore.c b/bsd-user/elfcore.c.inc
> similarity index 100%
> rename from bsd-user/elfcore.c
> rename to bsd-user/elfcore.c.inc
> diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
> index f8edb22f2a..1717a454dc 100644
> --- a/bsd-user/elfload.c
> +++ b/bsd-user/elfload.c
> @@ -121,7 +121,7 @@ static void bswap_note(struct elf_note *en) { }
>
>  #endif /* ! BSWAP_NEEDED */
>
> -#include "elfcore.c"
> +#include "elfcore.c.inc"
>
>  /*
>   * 'copy_elf_strings()' copies argument/envelope strings from user
> diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
> index 309e27b9d6..1eab1be6f6 100644
> --- a/bsd-user/freebsd/os-sys.c
> +++ b/bsd-user/freebsd/os-sys.c
> @@ -17,6 +17,8 @@
>   *  along with this program; if not, see .
>   */
>
> +#include "qemu/osdep.h"
> +
>  #include "qemu.h"
>  #include "target_arch_sysarch.h"
>
> diff --git a/bsd-user/i386/signal.c b/bsd-user/i386/signal.c
> index 5dd975ce56..db5b774213 100644
> --- a/bsd-user/i386/signal.c
> +++ b/bsd-user/i386/signal.c
> @@ -17,6 +17,8 @@
>   *  along with this program; if not, see .
>   */
>
> +#include "qemu/osdep.h"
> +
>  #include "qemu.h"
>
>  /*
> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index be6105385e..0ceecfb6df 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -17,7 +17,6 @@
>  #ifndef QEMU_H
>  #define QEMU_H
>
> -#include "qemu/osdep.h"
>  #include "cpu.h"
>  #include "qemu/units.h"
>  #include "exec/cpu_ldst.h"
> diff --git a/bsd-user/x86_64/signal.c b/bsd-user/x86_64/signal.c
> index c3875bc4c6..217f9ceb66 100644
> --- a/bsd-user/x86_64/signal.c
> +++ b/bsd-user/x86_64/signal.c
> @@ -16,6 +16,8 @@
>   *  along with this program; if not, see .
>   */
>
> +#include "qemu/osdep.h"
> +
>  #include "qemu.h"
>
>  /*
> diff --git a/crypto/rsakey.c b/crypto/rsakey.c
> index cc40e072f0..dcdbd9ec57 100644
> --- a/crypto/rsakey.c
> +++ b/crypto/rsakey.c
> @@ -19,6 +19,7 @@
>   *
>   */
>
> +#include "qemu/osdep.h"
>  #include "rsakey.h"
>
>  void qcrypto_akcipher_rsakey_free(QCryptoAkCipherRSAKey *rsa_key)
> diff --git a/qga/cutils.c b/qga/cutils.c
> index b8e142ef64..c53dd418c7 100644
> --- a/qga/cutils.c
> +++ b/qga/cutils.c
> @@ -2,6 +2,8 @@
>   * 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 "cutils.h"
>
>  #include "qapi/error.h"
> --
> 2.36.1
>
>
>

Re: [PATCH] target/ppc: Fix MPC8555 and MPC8560 core type to e500v1

[Yonggang Luo]

On Mon, Jul 4, 2022 at 6:20 PM Pali Rohár  wrote:
>
> On Sunday 03 July 2022 21:50:29 Pali Rohár wrote:
> > Commit 80d11f4467c4 ("Add definitions for Freescale PowerPC
implementations")
> > changed core type of MPC8555 and MPC8560 from e500v1 to e500v2.
> >
> > But both MPC8555 and MPC8560 have just e500v1 cores, there are no
features
> > of e500v2 cores. It can be verified by reading NXP documentations:
> > https://www.nxp.com/docs/en/data-sheet/MPC8555EEC.pdf
> > https://www.nxp.com/docs/en/data-sheet/MPC8560EC.pdf
> > https://www.nxp.com/docs/en/reference-manual/MPC8555ERM.pdf
> > https://www.nxp.com/docs/en/reference-manual/MPC8560RM.pdf
> >
> > Therefore fix core type of MPC8555 and MPC8560 back to e500v1.
> >
> > Fixes: 80d11f4467c4 ("Add definitions for Freescale PowerPC
implementations")
> > Signed-off-by: Pali Rohár 
> > ---
>
> Just for completeness, here is list of all Motorola/Freescale/NXP
> processors which were released and have e500v1 or e500v2 cores.
>
> e500v1:
> MPC8540
> MPC8541
> MPC8555
> MPC8560
>
> e500v2:
> BSC9131
> BSC9132
> C291
> C292
> C293
> MPC8533
> MPC8535
> MPC8536
> MPC8543
> MPC8544
> MPC8545
> MPC8547
> MPC8548
> MPC8567
> MPC8568
> MPC8569
> MPC8572
> P1010
> P1011
> P1012
> P1013
> P1014
> P1015
> P1016
> P1020
> P1021
> P1022
> P1024
> P1025
> P2010
> P2020

I'll suggest add this list into comment or commit message for record

>
> (sorted alphabetically; not by release date / generation / feature set)
>
> All this is from public information available on NXP website.
>
> Seems that qemu has support only for some subset of MPC85?? processors.
>
> Historically processors with e500 cores have mpc85xx family codename and
> lot of software have them in mpc85xx architecture subdirectory.
>
> Note that GCC uses -mcpu=8540 option for specifying e500v1 core and
> -mcpu=8548 option for specifying e500v2 core.
>
> So sometimes (mpc)8540 is alias for e500v1 and (mpc)8548 is alias for
> e500v2.
>
> >  target/ppc/cpu-models.c | 14 +++---
> >  target/ppc/cpu-models.h | 14 +++---
> >  2 files changed, 14 insertions(+), 14 deletions(-)
> >
> > diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
> > index 976be5e0d171..44a2710c5843 100644
> > --- a/target/ppc/cpu-models.c
> > +++ b/target/ppc/cpu-models.c
> > @@ -385,19 +385,19 @@
> >  POWERPC_DEF_SVR("mpc8548e_v21", "MPC8548E v2.1",
> >  CPU_POWERPC_MPC8548E_v21, POWERPC_SVR_8548E_v21,
e500v2)
> >  POWERPC_DEF_SVR("mpc8555_v10", "MPC8555 v1.0",
> > -CPU_POWERPC_MPC8555_v10,  POWERPC_SVR_8555_v10,
 e500v2)
> > +CPU_POWERPC_MPC8555_v10,  POWERPC_SVR_8555_v10,
 e500v1)
> >  POWERPC_DEF_SVR("mpc8555_v11", "MPC8555 v1.1",
> > -CPU_POWERPC_MPC8555_v11,  POWERPC_SVR_8555_v11,
 e500v2)
> > +CPU_POWERPC_MPC8555_v11,  POWERPC_SVR_8555_v11,
 e500v1)
> >  POWERPC_DEF_SVR("mpc8555e_v10", "MPC8555E v1.0",
> > -CPU_POWERPC_MPC8555E_v10, POWERPC_SVR_8555E_v10,
e500v2)
> > +CPU_POWERPC_MPC8555E_v10, POWERPC_SVR_8555E_v10,
e500v1)
> >  POWERPC_DEF_SVR("mpc8555e_v11", "MPC8555E v1.1",
> > -CPU_POWERPC_MPC8555E_v11, POWERPC_SVR_8555E_v11,
e500v2)
> > +CPU_POWERPC_MPC8555E_v11, POWERPC_SVR_8555E_v11,
e500v1)
> >  POWERPC_DEF_SVR("mpc8560_v10", "MPC8560 v1.0",
> > -CPU_POWERPC_MPC8560_v10,  POWERPC_SVR_8560_v10,
 e500v2)
> > +CPU_POWERPC_MPC8560_v10,  POWERPC_SVR_8560_v10,
 e500v1)
> >  POWERPC_DEF_SVR("mpc8560_v20", "MPC8560 v2.0",
> > -CPU_POWERPC_MPC8560_v20,  POWERPC_SVR_8560_v20,
 e500v2)
> > +CPU_POWERPC_MPC8560_v20,  POWERPC_SVR_8560_v20,
 e500v1)
> >  POWERPC_DEF_SVR("mpc8560_v21", "MPC8560 v2.1",
> > -CPU_POWERPC_MPC8560_v21,  POWERPC_SVR_8560_v21,
 e500v2)
> > +CPU_POWERPC_MPC8560_v21,  POWERPC_SVR_8560_v21,
 e500v1)
> >  POWERPC_DEF_SVR("mpc8567", "MPC8567",
> >  CPU_POWERPC_MPC8567,  POWERPC_SVR_8567,
 e500v2)
> >  POWERPC_DEF_SVR("mpc8567e", "MPC8567E",
> > diff --git a/target/ppc/cpu-models.h b/target/ppc/cpu-models.h
> > index 76775a74a9b6..1326493a9a05 100644
> > --- a/target/ppc/cpu-models.h
> > +++ b/target/ppc/cpu-models.h
> > @@ -184,13 +184,13 @@ enum {
> >  #define CPU_POWERPC_MPC8548E_v11 CPU_POWERPC_e500v2_v11
> >  #define CPU_POWERPC_MPC8548E_v20 CPU_POWERPC_e500v2_v20
> >  #define CPU_POWERPC_MPC8548E_v21 CPU_POWERPC_e500v2_v21
> > -#define CPU_POWERPC_MPC8555_v10  CPU_POWERPC_e500v2_v10
> > -#define CPU_POWERPC_MPC8555_v11  CPU_POWERPC_e500v2_v11
> > -#define CPU_POWERPC_MPC8555E_v10 CPU_POWERPC_e500v2_v10
> > -#define CPU_POWERPC_MPC8555E_v11 CPU_POWERPC_e500v2_v11
> > -#define CPU_POWERPC_MPC8560_v10  CPU_POWERPC_e500v2_v10
> > -#define CPU_POWERPC_MPC8560_v20  CPU_POWERPC_e500v2_v20
> > -#define 

Re: [PATCH v2 02/10] tests/qemu-iotests: skip 108 when FUSE is not loaded

[Hanna Reitz]

On 01.07.22 18:15, John Snow wrote:

On Fri, Jul 1, 2022 at 4:05 AM Hanna Reitz  wrote:

On 16.06.22 16:26, John Snow wrote:

In certain container environments we may not have FUSE at all, so skip
the test in this circumstance too.

Signed-off-by: John Snow 
---
   tests/qemu-iotests/108 | 5 +
   1 file changed, 5 insertions(+)

Reviewed-by: Hanna Reitz 


Hanna, if you want to take just the first two, be my guest.


Thanks for the invite, always happy to be your guest!

(Applied to my block branch:)

https://gitlab.com/hreitz/qemu/-/commits/block

Hanna

Re: [PATCH v10 05/12] target/riscv: Implement mcountinhibit CSR

[Weiwei Li]

在 2022/6/21 上午7:15, Atish Patra 写道:

From: Atish Patra 

As per the privilege specification v1.11, mcountinhibit allows to start/stop
a pmu counter selectively.

Reviewed-by: Bin Meng 
Reviewed-by: Alistair Francis 
Signed-off-by: Atish Patra 
Signed-off-by: Atish Patra 
---
  target/riscv/cpu.h  |  2 ++
  target/riscv/cpu_bits.h |  4 
  target/riscv/csr.c  | 25 +
  target/riscv/machine.c  |  1 +
  4 files changed, 32 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index ffee54ea5c27..0a916db9f614 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -275,6 +275,8 @@ struct CPUArchState {
  target_ulong scounteren;
  target_ulong mcounteren;
  
+target_ulong mcountinhibit;

+
  target_ulong sscratch;
  target_ulong mscratch;
  
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h

index 4d04b20d064e..b3f7fa713000 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -367,6 +367,10 @@
  #define CSR_MHPMCOUNTER29   0xb1d
  #define CSR_MHPMCOUNTER30   0xb1e
  #define CSR_MHPMCOUNTER31   0xb1f
+
+/* Machine counter-inhibit register */
+#define CSR_MCOUNTINHIBIT   0x320
+
  #define CSR_MHPMEVENT3  0x323
  #define CSR_MHPMEVENT4  0x324
  #define CSR_MHPMEVENT5  0x325
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index b4a8e15f498f..94d39a4ce1c5 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1475,6 +1475,28 @@ static RISCVException write_mtvec(CPURISCVState *env, 
int csrno,
  return RISCV_EXCP_NONE;
  }
  
+static RISCVException read_mcountinhibit(CPURISCVState *env, int csrno,

+ target_ulong *val)
+{
+if (env->priv_ver < PRIV_VERSION_1_11_0) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+


This seems can be done by add  .min_priv_ver=PRIV_VERSION_1_11_0 in 
csr_ops table.


Regards,

Weiwei Li


+*val = env->mcountinhibit;
+return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_mcountinhibit(CPURISCVState *env, int csrno,
+  target_ulong val)
+{
+if (env->priv_ver < PRIV_VERSION_1_11_0) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+env->mcountinhibit = val;
+return RISCV_EXCP_NONE;
+}
+
  static RISCVException read_mcounteren(CPURISCVState *env, int csrno,
target_ulong *val)
  {
@@ -3745,6 +3767,9 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
  [CSR_MHPMCOUNTER30]  = { "mhpmcounter30",  mctr,   read_zero },
  [CSR_MHPMCOUNTER31]  = { "mhpmcounter31",  mctr,   read_zero },
  
+[CSR_MCOUNTINHIBIT]  = { "mcountinhibit",   any,read_mcountinhibit,

+   write_mcountinhibit },
+
  [CSR_MHPMEVENT3] = { "mhpmevent3", any,read_zero },
  [CSR_MHPMEVENT4] = { "mhpmevent4", any,read_zero },
  [CSR_MHPMEVENT5] = { "mhpmevent5", any,read_zero },
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 2a437b29a1ce..87cd55bfd3a7 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -330,6 +330,7 @@ const VMStateDescription vmstate_riscv_cpu = {
  VMSTATE_UINTTL(env.siselect, RISCVCPU),
  VMSTATE_UINTTL(env.scounteren, RISCVCPU),
  VMSTATE_UINTTL(env.mcounteren, RISCVCPU),
+VMSTATE_UINTTL(env.mcountinhibit, RISCVCPU),
  VMSTATE_UINTTL(env.sscratch, RISCVCPU),
  VMSTATE_UINTTL(env.mscratch, RISCVCPU),
  VMSTATE_UINT64(env.mfromhost, RISCVCPU),

Re: [PATCH 05/18] block: Make blk_co_pwrite() take a const buffer

[Hanna Reitz]

On 17.05.22 13:38, Alberto Faria wrote:

It does not mutate the buffer.

Signed-off-by: Alberto Faria 
---
  include/sysemu/block-backend-io.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Hanna Reitz 

Re: [PATCH 06/18] block: Implement blk_{pread,pwrite}() using generated_co_wrapper

[Hanna Reitz]

On 17.05.22 13:38, Alberto Faria wrote:

We need to add include/sysemu/block-backend-io.h to the inputs of the
block-gen.c target defined in block/meson.build.

Signed-off-by: Alberto Faria 
---
  block/block-backend.c | 23 ---
  block/coroutines.h|  4 
  block/meson.build |  1 +
  include/sysemu/block-backend-io.h | 10 ++
  4 files changed, 7 insertions(+), 31 deletions(-)


Reviewed-by: Hanna Reitz 

Re: [PATCH 01/62] accel/tcg: Introduce PageEntryExtra

[Peter Maydell]

On Sun, 3 Jul 2022 at 09:25, Richard Henderson
 wrote:
>
> Add an optional structure, controlled by TARGET_PAGE_ENTRY_EXTRA,
> that allows arbitrary extra data to be saved in the TLB for a
> given page.  Set it with tlb_set_page_with_extra() and fetch it
> with probe_access_extra().
>
> Signed-off-by: Richard Henderson 
> ---



> -/* Add a new TLB entry. At most one entry for a given virtual address
> +/*
> + * Add a new TLB entry. At most one entry for a given virtual address
>   * is permitted. Only a single TARGET_PAGE_SIZE region is mapped, the
>   * supplied size is only used by tlb_flush_page.
>   *
>   * Called from TCG-generated code, which is under an RCU read-side
>   * critical section.
> + *
> + * Returns a pointer to the iotlb entry, with env_tlb(env)->c.lock
> + * still locked, for final additions to the iotlb entry.  The caller
> + * must unlock the lock.
>   */
> -void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr,
> - hwaddr paddr, MemTxAttrs attrs, int prot,
> - int mmu_idx, target_ulong size)
> +void tlb_set_page_with_extra(CPUState *cpu, target_ulong vaddr, hwaddr paddr,
> + MemTxAttrs attrs, PageEntryExtra extra,
> + int prot, int mmu_idx, target_ulong size)

The comment claims it returns a pointer to the iotlb entry, but
the code says it returns void... leftover from a previous design?

>  {
>  CPUArchState *env = cpu->env_ptr;
>  CPUTLB *tlb = env_tlb(env);

-- PMM

Re: [PATCH v10 04/12] target/riscv: pmu: Make number of counters configurable

[Weiwei Li]

在 2022/6/21 上午7:15, Atish Patra 写道:

The RISC-V privilege specification provides flexibility to implement
any number of counters from 29 programmable counters. However, the QEMU
implements all the counters.

Make it configurable through pmu config parameter which now will indicate
how many programmable counters should be implemented by the cpu.

Reviewed-by: Bin Meng 
Reviewed-by: Alistair Francis 
Signed-off-by: Atish Patra 
Signed-off-by: Atish Patra 
---
  target/riscv/cpu.c |  3 +-
  target/riscv/cpu.h |  2 +-
  target/riscv/csr.c | 94 ++
  3 files changed, 63 insertions(+), 36 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1b57b3c43980..d12c6dc630ca 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -851,7 +851,6 @@ static void riscv_cpu_init(Object *obj)
  {
  RISCVCPU *cpu = RISCV_CPU(obj);
  
-cpu->cfg.ext_pmu = true;

  cpu->cfg.ext_ifencei = true;
  cpu->cfg.ext_icsr = true;
  cpu->cfg.mmu = true;
@@ -879,7 +878,7 @@ static Property riscv_cpu_extensions[] = {
  DEFINE_PROP_BOOL("u", RISCVCPU, cfg.ext_u, true),
  DEFINE_PROP_BOOL("v", RISCVCPU, cfg.ext_v, false),
  DEFINE_PROP_BOOL("h", RISCVCPU, cfg.ext_h, true),
-DEFINE_PROP_BOOL("pmu", RISCVCPU, cfg.ext_pmu, true),
+DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),


I think It's better to add  a check on cfg.pmu_num to  <= 29.


  DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
  DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
  DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 252c30a55d78..ffee54ea5c27 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -397,7 +397,6 @@ struct RISCVCPUConfig {
  bool ext_zksed;
  bool ext_zksh;
  bool ext_zkt;
-bool ext_pmu;
  bool ext_ifencei;
  bool ext_icsr;
  bool ext_svinval;
@@ -421,6 +420,7 @@ struct RISCVCPUConfig {
  /* Vendor-specific custom extensions */
  bool ext_XVentanaCondOps;
  
+uint8_t pmu_num;

  char *priv_spec;
  char *user_spec;
  char *bext_spec;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 0ca05c77883c..b4a8e15f498f 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -73,9 +73,17 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
  CPUState *cs = env_cpu(env);
  RISCVCPU *cpu = RISCV_CPU(cs);
  int ctr_index;
+int base_csrno = CSR_HPMCOUNTER3;
+bool rv32 = riscv_cpu_mxl(env) == MXL_RV32 ? true : false;
  
-if (!cpu->cfg.ext_pmu) {

-/* The PMU extension is not enabled */
+if (rv32 && csrno >= CSR_CYCLEH) {
+/* Offset for RV32 hpmcounternh counters */
+base_csrno += 0x80;
+}
+ctr_index = csrno - base_csrno;
+
+if (!cpu->cfg.pmu_num || ctr_index >= (cpu->cfg.pmu_num)) {
+/* No counter is enabled in PMU or the counter is out of range */


I seems unnecessary to add '!cpu->cfg.pmu_num ' here,   'ctr_index >= 
(cpu->cfg.pmu_num)' is true


when cpu->cfg.pmu_num is zero if the problem for base_csrno is fixed.

Ragards,

Weiwei Li


  return RISCV_EXCP_ILLEGAL_INST;
  }
  
@@ -103,7 +111,7 @@ static RISCVException ctr(CPURISCVState *env, int csrno)

  }
  break;
  }
-if (riscv_cpu_mxl(env) == MXL_RV32) {
+if (rv32) {
  switch (csrno) {
  case CSR_CYCLEH:
  if (!get_field(env->mcounteren, COUNTEREN_CY)) {
@@ -158,7 +166,7 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
  }
  break;
  }
-if (riscv_cpu_mxl(env) == MXL_RV32) {
+if (rv32) {
  switch (csrno) {
  case CSR_CYCLEH:
  if (!get_field(env->hcounteren, COUNTEREN_CY) &&
@@ -202,6 +210,26 @@ static RISCVException ctr32(CPURISCVState *env, int csrno)
  }
  
  #if !defined(CONFIG_USER_ONLY)

+static RISCVException mctr(CPURISCVState *env, int csrno)
+{
+CPUState *cs = env_cpu(env);
+RISCVCPU *cpu = RISCV_CPU(cs);
+int ctr_index;
+int base_csrno = CSR_MHPMCOUNTER3;
+
+if ((riscv_cpu_mxl(env) == MXL_RV32) && csrno >= CSR_MCYCLEH) {
+/* Offset for RV32 mhpmcounternh counters */
+base_csrno += 0x80;
+}
+ctr_index = csrno - base_csrno;
+if (!cpu->cfg.pmu_num || ctr_index >= cpu->cfg.pmu_num) {
+/* The PMU is not enabled or counter is out of range*/
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+return RISCV_EXCP_NONE;
+}
+
  static RISCVException any(CPURISCVState *env, int csrno)
  {
  return RISCV_EXCP_NONE;
@@ -3687,35 +3715,35 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
  [CSR_HPMCOUNTER30]   = { "hpmcounter30",   ctr,read_zero },
  [CSR_HPMCOUNTER31]   = { "hpmcounter31",   ctr,read_zero },
  
-[CSR_MHPMCOUNTER3]   = { "mhpmcounter3",   any,read_zero },

-

Re: [PATCH 04/18] block: Make 'bytes' param of blk_{pread,pwrite}() an int64_t

[Hanna Reitz]

On 17.05.22 13:37, Alberto Faria wrote:

For consistency with other I/O functions, and in preparation to
implement them using generated_co_wrapper.

Signed-off-by: Alberto Faria 
---
  block/block-backend.c | 6 +++---
  include/sysemu/block-backend-io.h | 6 +++---
  2 files changed, 6 insertions(+), 6 deletions(-)


Reviewed-by: Hanna Reitz 

[PATCH v2 6/7] misc: ensure qemu/osdep.h is included in all .c files

[Daniel P . Berrangé]

A few files relied on qemu/osdep.h being included via a common
header. Another file didn't need it because it was actually an
included file, so ought to have been named .c.inc

Signed-off-by: Daniel P. Berrangé 
---
 bsd-user/arm/signal.c | 2 ++
 bsd-user/arm/target_arch_cpu.c| 3 +++
 bsd-user/{elfcore.c => elfcore.c.inc} | 0
 bsd-user/elfload.c| 2 +-
 bsd-user/freebsd/os-sys.c | 2 ++
 bsd-user/i386/signal.c| 2 ++
 bsd-user/qemu.h   | 1 -
 bsd-user/x86_64/signal.c  | 2 ++
 crypto/rsakey.c   | 1 +
 qga/cutils.c  | 2 ++
 10 files changed, 15 insertions(+), 2 deletions(-)
 rename bsd-user/{elfcore.c => elfcore.c.inc} (100%)

diff --git a/bsd-user/arm/signal.c b/bsd-user/arm/signal.c
index 2b1dd745d1..eca20ac4d7 100644
--- a/bsd-user/arm/signal.c
+++ b/bsd-user/arm/signal.c
@@ -17,6 +17,8 @@
  *  along with this program; if not, see .
  */
 
+#include "qemu/osdep.h"
+
 #include "qemu.h"
 
 /*
diff --git a/bsd-user/arm/target_arch_cpu.c b/bsd-user/arm/target_arch_cpu.c
index 02bf9149d5..186cf43fb9 100644
--- a/bsd-user/arm/target_arch_cpu.c
+++ b/bsd-user/arm/target_arch_cpu.c
@@ -16,6 +16,9 @@
  *  You should have received a copy of the GNU General Public License
  *  along with this program; if not, see .
  */
+
+#include "qemu/osdep.h"
+
 #include "target_arch.h"
 
 void target_cpu_set_tls(CPUARMState *env, target_ulong newtls)
diff --git a/bsd-user/elfcore.c b/bsd-user/elfcore.c.inc
similarity index 100%
rename from bsd-user/elfcore.c
rename to bsd-user/elfcore.c.inc
diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
index f8edb22f2a..1717a454dc 100644
--- a/bsd-user/elfload.c
+++ b/bsd-user/elfload.c
@@ -121,7 +121,7 @@ static void bswap_note(struct elf_note *en) { }
 
 #endif /* ! BSWAP_NEEDED */
 
-#include "elfcore.c"
+#include "elfcore.c.inc"
 
 /*
  * 'copy_elf_strings()' copies argument/envelope strings from user
diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
index 309e27b9d6..1eab1be6f6 100644
--- a/bsd-user/freebsd/os-sys.c
+++ b/bsd-user/freebsd/os-sys.c
@@ -17,6 +17,8 @@
  *  along with this program; if not, see .
  */
 
+#include "qemu/osdep.h"
+
 #include "qemu.h"
 #include "target_arch_sysarch.h"
 
diff --git a/bsd-user/i386/signal.c b/bsd-user/i386/signal.c
index 5dd975ce56..db5b774213 100644
--- a/bsd-user/i386/signal.c
+++ b/bsd-user/i386/signal.c
@@ -17,6 +17,8 @@
  *  along with this program; if not, see .
  */
 
+#include "qemu/osdep.h"
+
 #include "qemu.h"
 
 /*
diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index be6105385e..0ceecfb6df 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -17,7 +17,6 @@
 #ifndef QEMU_H
 #define QEMU_H
 
-#include "qemu/osdep.h"
 #include "cpu.h"
 #include "qemu/units.h"
 #include "exec/cpu_ldst.h"
diff --git a/bsd-user/x86_64/signal.c b/bsd-user/x86_64/signal.c
index c3875bc4c6..217f9ceb66 100644
--- a/bsd-user/x86_64/signal.c
+++ b/bsd-user/x86_64/signal.c
@@ -16,6 +16,8 @@
  *  along with this program; if not, see .
  */
 
+#include "qemu/osdep.h"
+
 #include "qemu.h"
 
 /*
diff --git a/crypto/rsakey.c b/crypto/rsakey.c
index cc40e072f0..dcdbd9ec57 100644
--- a/crypto/rsakey.c
+++ b/crypto/rsakey.c
@@ -19,6 +19,7 @@
  *
  */
 
+#include "qemu/osdep.h"
 #include "rsakey.h"
 
 void qcrypto_akcipher_rsakey_free(QCryptoAkCipherRSAKey *rsa_key)
diff --git a/qga/cutils.c b/qga/cutils.c
index b8e142ef64..c53dd418c7 100644
--- a/qga/cutils.c
+++ b/qga/cutils.c
@@ -2,6 +2,8 @@
  * 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 "cutils.h"
 
 #include "qapi/error.h"
-- 
2.36.1

[PATCH v2 5/7] tests/style: check for commonly doubled up words

[Daniel P . Berrangé]

This style check looks for cases where the words

  the then in an on if is it but for or at and do to

are repeated in a sentence. It doesn't use the simple regex match logic
because it needs to match repeats across lines, so has a custom crafted
rule.

Signed-off-by: Daniel P. Berrangé 
---
 tests/style-excludes.mak | 12 
 tests/style.mak  | 24 
 2 files changed, 36 insertions(+)

diff --git a/tests/style-excludes.mak b/tests/style-excludes.mak
index 32c0e9c328..931dd03a64 100644
--- a/tests/style-excludes.mak
+++ b/tests/style-excludes.mak
@@ -2,3 +2,15 @@
 #
 # Filenames that should be excluded from specific
 # style checks performed by style.mak
+
+exclude_file_name_regexp--sc_prohibit_doubled_word = \
+   disas/sparc\.c \
+   hw/char/terminal3270\.c \
+   include/crypto/afsplit\.h \
+   qemu-options\.hx \
+   scripts/checkpatch\.pl \
+   target/s390x/tcg/insn-data\.def \
+   pc-bios/slof\.bin \
+   tests/qemu-iotests/142(\.out)? \
+   tests/qtest/arm-cpu-features\.c \
+   ui/cursor\.c
diff --git a/tests/style.mak b/tests/style.mak
index ae658395c9..4056bde619 100644
--- a/tests/style.mak
+++ b/tests/style.mak
@@ -28,3 +28,27 @@ sc_int_assign_bool:
@prohibit='\.*= *(true|false)\b' \
halt='use bool type for boolean values' \
$(_sc_search_regexp)
+
+prohibit_doubled_words_ = \
+the then in an on if is it but for or at and do to can
+# expand the regex before running the check to avoid using expensive captures
+prohibit_doubled_word_expanded_ = \
+$(join $(prohibit_doubled_words_),$(addprefix 
\s+,$(prohibit_doubled_words_)))
+prohibit_doubled_word_RE_ ?= \
+/\b(?:$(subst $(_sp),|,$(prohibit_doubled_word_expanded_)))\b/gims
+prohibit_doubled_word_ =   \
+-e 'while ($(prohibit_doubled_word_RE_))'  \
+$(perl_filename_lineno_text_)
+
+# Define this to a regular expression that matches
+# any filename:dd:match lines you want to ignore.
+# The default is to ignore no matches.
+ignore_doubled_word_match_RE_ ?= ^$$
+
+sc_prohibit_doubled_word:
+   @$(VC_LIST_EXCEPT)  \
+ | xargs perl -n -0777 $(prohibit_doubled_word_)   \
+ | $(GREP) -vE '$(ignore_doubled_word_match_RE_)'  \
+ | $(GREP) .   \
+ && { echo '$(ME): doubled words' 1>&2; exit 1; }  \
+ || :
-- 
2.36.1

[PATCH v2 4/7] misc: fix commonly doubled up words

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 block/linux-aio.c  | 2 +-
 block/qcow2-bitmap.c   | 8 
 contrib/plugins/cache.c| 2 +-
 disas/libvixl/vixl/invalset.h  | 2 +-
 docs/devel/qom.rst | 4 ++--
 docs/interop/live-block-operations.rst | 4 ++--
 docs/system/arm/cpu-features.rst   | 2 +-
 docs/system/devices/cxl.rst| 2 +-
 docs/system/s390x/bootdevices.rst  | 2 +-
 docs/system/tls.rst| 2 +-
 docs/tools/qemu-pr-helper.rst  | 2 +-
 hw/core/clock.c| 2 +-
 hw/intc/arm_gicv3_redist.c | 2 +-
 hw/misc/iotkit-secctl.c| 2 +-
 hw/misc/iotkit-sysctl.c| 4 ++--
 hw/s390x/s390-ccw.c| 2 +-
 hw/usb/u2f.h   | 2 +-
 include/hw/qdev-core.h | 2 +-
 include/user/safe-syscall.h| 2 +-
 linux-user/i386/cpu_loop.c | 2 +-
 pc-bios/s390-ccw/virtio-scsi.c | 2 +-
 python/Makefile| 2 +-
 python/qemu/utils/__init__.py  | 2 +-
 target/arm/translate.c | 2 +-
 target/i386/cpu.c  | 2 +-
 tcg/i386/tcg-target.c.inc  | 2 +-
 tests/qtest/microbit-test.c| 4 ++--
 tools/virtiofsd/fuse_virtio.c  | 2 +-
 28 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/block/linux-aio.c b/block/linux-aio.c
index 9c2393a2f7..d2cfb7f523 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -461,7 +461,7 @@ LinuxAioState *laio_init(Error **errp)
 s = g_malloc0(sizeof(*s));
 rc = event_notifier_init(>e, false);
 if (rc < 0) {
-error_setg_errno(errp, -rc, "failed to to initialize event notifier");
+error_setg_errno(errp, -rc, "failed to initialize event notifier");
 goto out_free_state;
 }
 
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 8fb4731551..5529368df4 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -787,10 +787,10 @@ static int bitmap_list_store(BlockDriverState *bs, 
Qcow2BitmapList *bm_list,
 }
 }
 
-/* Actually, even in in-place case ignoring QCOW2_OL_BITMAP_DIRECTORY is 
not
- * necessary, because we drop QCOW2_AUTOCLEAR_BITMAPS when updating bitmap
- * directory in-place (actually, turn-off the extension), which is checked
- * in qcow2_check_metadata_overlap() */
+/* Actually, even in the in-place case ignoring QCOW2_OL_BITMAP_DIRECTORY
+ * is not necessary, because we drop QCOW2_AUTOCLEAR_BITMAPS when updating
+ * bitmap directory in-place (actually, turn-off the extension), which is
+ * checked in qcow2_check_metadata_overlap() */
 ret = qcow2_pre_write_overlap_check(
 bs, in_place ? QCOW2_OL_BITMAP_DIRECTORY : 0, dir_offset, dir_size,
 false);
diff --git a/contrib/plugins/cache.c b/contrib/plugins/cache.c
index b9226e7c40..ac1510aaa1 100644
--- a/contrib/plugins/cache.c
+++ b/contrib/plugins/cache.c
@@ -38,7 +38,7 @@ enum EvictionPolicy policy;
  * put in any of the blocks inside the set. The number of block per set is
  * called the associativity (assoc).
  *
- * Each block contains the the stored tag and a valid bit. Since this is not
+ * Each block contains the stored tag and a valid bit. Since this is not
  * a functional simulator, the data itself is not stored. We only identify
  * whether a block is in the cache or not by searching for its tag.
  *
diff --git a/disas/libvixl/vixl/invalset.h b/disas/libvixl/vixl/invalset.h
index 2e0871f8c3..f5d6c43d81 100644
--- a/disas/libvixl/vixl/invalset.h
+++ b/disas/libvixl/vixl/invalset.h
@@ -102,7 +102,7 @@ template class InvalSet {
   size_t size() const;
 
   // Returns true if no elements are stored in the set.
-  // Note that this does not mean the the backing storage is empty: it can 
still
+  // Note that this does not mean the backing storage is empty: it can still
   // contain invalid elements.
   bool empty() const;
 
diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst
index e5fe3597cd..62c39c9c88 100644
--- a/docs/devel/qom.rst
+++ b/docs/devel/qom.rst
@@ -372,8 +372,8 @@ This accepts an array of interface type names.
   { TYPE_USER_CREATABLE },
   { NULL })
 
-If the type is not intended to be instantiated, then then
-the OBJECT_DEFINE_ABSTRACT_TYPE() macro can be used instead:
+If the type is not intended to be instantiated, then the
+OBJECT_DEFINE_ABSTRACT_TYPE() macro can be used instead:
 
 .. code-block:: c
:caption: Defining a simple abstract type
diff --git a/docs/interop/live-block-operations.rst 
b/docs/interop/live-block-operations.rst
index 39e62c9915..135784ab33 100644
--- a/docs/interop/live-block-operations.rst
+++ b/docs/interop/live-block-operations.rst
@@ -53,7 +53,7 @@ files in a disk image backing chain:
 
 (1) Directional: 'base' and 'top'.  Given the 

[PATCH v2 7/7] tests/style: check qemu/osdep.h is included in all .c files

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 tests/style-excludes.mak | 17 +
 tests/style.mak  |  6 ++
 2 files changed, 23 insertions(+)

diff --git a/tests/style-excludes.mak b/tests/style-excludes.mak
index 931dd03a64..df158e1d9d 100644
--- a/tests/style-excludes.mak
+++ b/tests/style-excludes.mak
@@ -14,3 +14,20 @@ exclude_file_name_regexp--sc_prohibit_doubled_word = \
tests/qemu-iotests/142(\.out)? \
tests/qtest/arm-cpu-features\.c \
ui/cursor\.c
+
+exclude_file_name_regexp--sc_c_file_osdep_h = \
+   contrib/plugins/.* \
+   linux-user/(mips64|x86_64)/(signal|cpu_loop)\.c \
+   pc-bios/.* \
+   scripts/coverity-scan/model\.c \
+   scripts/xen-detect\.c \
+   subprojects/.* \
+   target/hexagon/(gen_semantics|gen_dectree_import)\.c \
+   target/s390x/gen-features\.c \
+   tests/migration/s390x/a-b-bios\.c \
+   tests/multiboot/.* \
+   tests/plugin/.* \
+   tests/tcg/.* \
+   tests/uefi-test-tools/.* \
+   tests/unit/test-rcu-(simpleq|slist|tailq)\.c \
+   tools/ebpf/rss.bpf.c
diff --git a/tests/style.mak b/tests/style.mak
index 4056bde619..301d978155 100644
--- a/tests/style.mak
+++ b/tests/style.mak
@@ -52,3 +52,9 @@ sc_prohibit_doubled_word:
  | $(GREP) .   \
  && { echo '$(ME): doubled words' 1>&2; exit 1; }  \
  || :
+
+sc_c_file_osdep_h:
+   @require='#include "qemu/osdep.h"' \
+   in_vc_files='\.c$$' \
+   halt='all C files must include qemu/osdep.h' \
+   $(_sc_search_regexp)
-- 
2.36.1

[PATCH v2 0/7] tests: introduce a tree-wide code style checking facility

[Daniel P . Berrangé]

The first patch gives a detailed description, but the overall goal here
is to provide a code style checking facility to augment (and ideally
eventually replace) checkpatch.pl. The key conceptual differences are:

 - Always applies to all code in tree, not merely patches
 - Failures are always fatal, exceptions must be recorded
 - Always runs as part of 'make check'

These combine to ensure that the in-tree code quality will be kept
at a high bar on an ongoing basis, with no silently ignoring rules,
any exceptions must be recorded explicitly to allow the check to
pass.

The first patch introduces the infrastructure, the remaining patches
illustrate its usage for three rules

 - Prevent initializing an 'int' variable with 'true' / 'false'
 - Look for commonly repeated words (ie the the)
 - Ensure qemu/osdep.h is listed in all .c files

As noted above, it integrates with 'make check' via a new test suite
called 'style', so you can invoke it individually too:

$ make check-style
changing dir to build for make "check-style"...
/usr/bin/meson test  --no-rebuild -t 0  --num-processes 1 --print-errorlogs 
 --suite style
1/3 qemu:style / int_assign_bool  OK  0.28s
2/3 qemu:style / prohibit_doubled_wordOK  1.78s
3/3 qemu:style / c_file_osdep_h   OK  0.08s

Ok: 3
Expected Fail:  0
Fail:   0
Unexpected Pass:0
Skipped:0
Timeout:0

Example of what it looks like when it fails:

$ make check-style
changing dir to build for make "check-style"...
make[1]: Entering directory '/home/berrange/src/virt/qemu/build'
  GIT ui/keycodemapdb tests/fp/berkeley-testfloat-3 
tests/fp/berkeley-softfloat-3 dtc slirp
/usr/bin/meson test  --no-rebuild -t 0  --num-processes 1 --print-errorlogs 
 --suite style
1/3 qemu:style / int_assign_bool  OK  0.27s
2/3 qemu:style / prohibit_doubled_wordOK  1.77s
3/3 qemu:style / c_file_osdep_h   FAIL0.07s   exit 
status 2
>>> MALLOC_PERTURB_=217 /usr/bin/make -f 
/home/berrange/src/virt/qemu/build/../tests/style.mak sc_c_file_osdep_h
― ✀  
――
stdout:
make[2]: Entering directory '/home/berrange/src/virt/qemu'
c_file_osdep_h
scripts/coverity-scan/model.c
scripts/xen-detect.c
subprojects/libvduse/libvduse.c
subprojects/libvhost-user/libvhost-user-glib.c
subprojects/libvhost-user/libvhost-user.c
subprojects/libvhost-user/link-test.c
make[2]: Leaving directory '/home/berrange/src/virt/qemu'
stderr:
tests/style.mak: all C files must include qemu/osdep.h
make[2]: *** [/home/berrange/src/virt/qemu/build/../tests/style.mak:57: 
sc_c_file_osdep_h] Error 1

―――

Summary of Failures:

3/3 qemu:style / c_file_osdep_hFAIL0.07s   exit status 2

Ok: 2
Expected Fail:  0
Fail:   1
Unexpected Pass:0
Skipped:0
Timeout:0


If debugging new tests it can be preferrable to directly invoke it
bypassing meson:

$ make -f tests/style.mak
c_file_osdep_h
0.04 c_file_osdep_h
int_assign_bool
0.23 int_assign_bool
prohibit_doubled_word
1.73 prohibit_doubled_word

or

$ make -f tests/style.mak  sc_c_file_osdep_h
c_file_osdep_h
scripts/coverity-scan/model.c
scripts/xen-detect.c
subprojects/libvduse/libvduse.c
subprojects/libvhost-user/libvhost-user-glib.c
subprojects/libvhost-user/libvhost-user.c
subprojects/libvhost-user/link-test.c
tests/style.mak: all C files must include qemu/osdep.h
make: *** [tests/style.mak:57: sc_c_file_osdep_h] Error 1

The speed of the test suite is largely driven by how quickly
'grep' can match through *every* file in the soruce tree (as
reported by 'git ls-tree').

The 'prohibit_doubled_word' test illustrates a more complex
check that uses perl. That runs a little more slowly but is
more flexible in what it checks for.

This style checking scheme is that same as that used by libvirt
and many other projects that historically used gnulib. Fortunately
the style check rules were easy to extract from gnulib, so libvirt
kept using them after droppping gnulib.

Daniel P. Berrangé (7):
  tests: introduce tree-wide code style checking
  misc: fix mixups of bool constants with int variables
  tests/style: check for mixups of bool constants with int variables
  misc: fix commonly doubled up words
  tests/style: check for commonly doubled up words
  misc: ensure qemu/osdep.h is included in all .c files
  tests/style: check qemu/osdep.h is included in all .c files

 

[PATCH v2 1/7] tests: introduce tree-wide code style checking

[Daniel P . Berrangé]

Historically QEMU has used the 'scripts/checkpatch.pl' script to
validate various style rules but there are a number of issues:

 - It requires the contributor to remember to run it as it
   is not wired into 'make check'

 - While it can be told to check whole files, in practice
   it is usually only used to check patch diffs, because the
   former would flag up pages of pre-existing violations that
   have never been fixed

 - It is said to be OK to ignore some things reported by the
   script, but don't record these exceptional cases anywere.
   Thus contributors looking at existing violations in tree
   are never sure whether they are intentional or historical
   unfixed technical debt.

 - There is no distinct reporting for each type of check
   performed and as a consequence there is also no way to
   mark particular files to be skipped for particular checks

This commit aims to give us a better approach to checking many types
of code style problems by introducing a flexible way to define whole
tree style checks.

The logic is essentially an import of the 'top/maint.mk' file from
GNULIB, with the following changes applied

 - Logic unrelated to the GNULIB syntax-check functionality
   is removed.

 - sc_excl is redefined, so that whitespace is turned into
   an '|' condition. This allows filename exclusion lists
   to span multiple lines making it more readable.

 - VC_LIST is changed to directly invoke git instead of
   indirectly via the vc-list script, since QEMU does not
   need portaility across many source control systems.

 - .DEFAULT_GOAL is set to 'syntax-check' since this is being
   used in a self-contained manner and thus doesn't need to
   play nice with our makefile rules QEMU has

This commit does the bare minimum introducing the basic infra:

 - tests/style-infra.mak - the cut down import of maint.mk
 - tests/style-excludes.mak - where the list of filename
   exclusions per test will be maintained (empty)
 - tests/style.mak - where the interesting tests live (empty)

As a general rule new checks can be implemented by merely defining
a regex matching the code pattern that should be blocked.

For example, consider we want to stop people using the 'bool' type
entirely. A rule starting with the prefix 'sc_' is defined in the
style.mak file:

  sc_prohibit_bool:
prohibit='\' \
halt='do not use the bool type' \
$(_sc_search_regexp)

Where '$(_sc_search_regexp)' (acquired from style-infra.mak)
contains all the magic to perform the tree-wide search for the
offending code pattern, reporting '$(halt)' as the error message
if violations are found.

If certain files need to be skipped for certain tests this can
be achieved by defining a variable with 'exclude_file_name_regexp--'
followed by the name of the check

  exclude_file_name_regexp--sc_prohibit_bool = \
i-am-allowed-to-use-bool.c \
and-so-am-i.c

Some checks can't be easily implemented by a simple per-line matching
regex. Since the checks are just implemented in make/shell, custom
rules can run essentially arbitrary logic.

Note that the checks require the use of 'git' to detect the list of
source files to search. Thus the check is skipped when not running
from a git repository.

Signed-off-by: Daniel P. Berrangé 
---
 meson.build  |   3 +
 tests/Makefile.include   |   3 +-
 tests/meson.build|  19 +++
 tests/style-excludes.mak |   4 +
 tests/style-infra.mak| 265 +++
 tests/style.mak  |  24 
 6 files changed, 317 insertions(+), 1 deletion(-)
 create mode 100644 tests/style-excludes.mak
 create mode 100644 tests/style-infra.mak
 create mode 100644 tests/style.mak

diff --git a/meson.build b/meson.build
index 65a885ea69..420102353e 100644
--- a/meson.build
+++ b/meson.build
@@ -18,6 +18,9 @@ config_host = keyval.load(meson.current_build_dir() / 
'config-host.mak')
 enable_modules = 'CONFIG_MODULES' in config_host
 enable_static = 'CONFIG_STATIC' in config_host
 
+make = find_program(config_host['MAKE'])
+in_gitrepo = run_command('test', '-d', '.git', check: false).returncode() == 0
+
 # Allow both shared and static libraries unless --enable-static
 static_kwargs = enable_static ? {'static': true} : {}
 
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3accb83b13..f7c1d2644e 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -3,12 +3,13 @@
 .PHONY: check-help
 check-help:
@echo "Regression testing targets:"
-   @echo " $(MAKE) check  Run block, qapi-schema, unit, 
softfloat, qtest and decodetree tests"
+   @echo " $(MAKE) check  Run block, qapi-schema, unit, 
style, softfloat, qtest and decodetree tests"
@echo " $(MAKE) bench  Run speed tests"
@echo
@echo "Individual test suites:"
@echo " $(MAKE) check-qtest-TARGET Run qtest tests for given target"
@echo " $(MAKE) check-qtestRun 

[PATCH v2 2/7] misc: fix mixups of bool constants with int variables

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 block/vhdx-log.c   | 2 +-
 hw/xtensa/sim.c| 4 ++--
 nbd/client.c   | 4 ++--
 target/i386/cpu-dump.c | 3 ++-
 ui/spice-display.c | 4 ++--
 5 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/block/vhdx-log.c b/block/vhdx-log.c
index ff0d4e0da0..8f34755a6f 100644
--- a/block/vhdx-log.c
+++ b/block/vhdx-log.c
@@ -215,7 +215,7 @@ exit:
 static bool vhdx_log_hdr_is_valid(VHDXLogEntries *log, VHDXLogEntryHeader *hdr,
   BDRVVHDXState *s)
 {
-int valid = false;
+bool valid = false;
 
 if (hdr->signature != VHDX_LOG_SIGNATURE) {
 goto exit;
diff --git a/hw/xtensa/sim.c b/hw/xtensa/sim.c
index 946c71cb5b..70fce7fb85 100644
--- a/hw/xtensa/sim.c
+++ b/hw/xtensa/sim.c
@@ -97,9 +97,9 @@ void xtensa_sim_load_kernel(XtensaCPU *cpu, MachineState 
*machine)
 {
 const char *kernel_filename = machine->kernel_filename;
 #if TARGET_BIG_ENDIAN
-int big_endian = true;
+int big_endian = 1;
 #else
-int big_endian = false;
+int big_endian = 0;
 #endif
 
 if (kernel_filename) {
diff --git a/nbd/client.c b/nbd/client.c
index 30d5383cb1..fee3959d24 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -832,8 +832,8 @@ static int nbd_list_meta_contexts(QIOChannel *ioc,
   Error **errp)
 {
 int ret;
-int seen_any = false;
-int seen_qemu = false;
+bool seen_any = false;
+bool seen_qemu = false;
 
 if (nbd_send_meta_query(ioc, NBD_OPT_LIST_META_CONTEXT,
 info->name, NULL, errp) < 0) {
diff --git a/target/i386/cpu-dump.c b/target/i386/cpu-dump.c
index 08ac957e99..43521c74c8 100644
--- a/target/i386/cpu-dump.c
+++ b/target/i386/cpu-dump.c
@@ -275,7 +275,8 @@ static void dump_apic_icr(APICCommonState *s, CPUX86State 
*env)
 static void dump_apic_interrupt(const char *name, uint32_t *ireg_tab,
 uint32_t *tmr_tab)
 {
-int i, empty = true;
+int i;
+bool empty = true;
 
 qemu_printf("%s\t ", name);
 for (i = 0; i < 256; i++) {
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 494168e7fe..5d3b64413f 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -541,14 +541,14 @@ static int interface_get_command(QXLInstance *sin, 
QXLCommandExt *ext)
 {
 SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
 SimpleSpiceUpdate *update;
-int ret = false;
+int ret = 0;
 
 qemu_mutex_lock(>lock);
 update = QTAILQ_FIRST(>updates);
 if (update != NULL) {
 QTAILQ_REMOVE(>updates, update, next);
 *ext = update->ext;
-ret = true;
+ret = 1;
 }
 qemu_mutex_unlock(>lock);
 
-- 
2.36.1

[PATCH v2 3/7] tests/style: check for mixups of bool constants with int variables

[Daniel P . Berrangé]

The 'true' and 'false' constants should only ever be used with the
'bool' type, never 'int'.

Signed-off-by: Daniel P. Berrangé 
---
 tests/style.mak | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/tests/style.mak b/tests/style.mak
index 32c7e706ba..ae658395c9 100644
--- a/tests/style.mak
+++ b/tests/style.mak
@@ -22,3 +22,9 @@
 
 include tests/style-infra.mak
 include tests/style-excludes.mak
+
+# Use 'bool', not 'int', when assigning true or false
+sc_int_assign_bool:
+   @prohibit='\.*= *(true|false)\b' \
+   halt='use bool type for boolean values' \
+   $(_sc_search_regexp)
-- 
2.36.1

Re: [PATCH 02/62] target/arm: Enable PageEntryExtra

[Peter Maydell]

On Sun, 3 Jul 2022 at 09:25, Richard Henderson
 wrote:
>
> Copy attrs, sharability, and the NS bit into the TLB.
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/cpu-param.h  |  8 
>  target/arm/internals.h  |  5 +
>  target/arm/tlb_helper.c | 14 --
>  3 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h
> index 68ffb12427..a14f167d11 100644
> --- a/target/arm/cpu-param.h
> +++ b/target/arm/cpu-param.h
> @@ -30,6 +30,14 @@
>   */
>  # define TARGET_PAGE_BITS_VARY
>  # define TARGET_PAGE_BITS_MIN  10
> +/*
> + * Extra information stored in softmmu page tables.
> + */
> +# define TARGET_PAGE_ENTRY_EXTRA
> +struct PageEntryExtra {
> +/* See PAGEENTRYEXTRA fields in cpu.h */
> +uint64_t x;
> +};
>  #endif
>
>  #define NB_MMU_MODES 15
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index c66f74a0db..2b38a83574 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -74,6 +74,11 @@ FIELD(V7M_EXCRET, DCRS, 5, 1)
>  FIELD(V7M_EXCRET, S, 6, 1)
>  FIELD(V7M_EXCRET, RES1, 7, 25) /* including the must-be-1 prefix */
>
> +/* Bit definitions for PageEntryExtra */
> +FIELD(PAGEENTRYEXTRA, ATTRS, 0, 8)
> +FIELD(PAGEENTRYEXTRA, SHAREABILITY, 8, 2)
> +FIELD(PAGEENTRYEXTRA, PA, 12, 52)

So why do we want these things in particular? It would be
helpful to describe the intended uses in the commit message
to save the reader having to read the next 60 patches to
find out :-)

Is wanting to cache the physaddr an Arm-specific thing, or is it
something we should consider having in the core softmmu code?

> +
>  /* Minimum value which is a magic number for exception return */
>  #define EXC_RETURN_MIN_MAGIC 0xff00
>  /* Minimum number which is a magic number for function or exception return
> diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c
> index 7d8a86b3c4..9de3099153 100644
> --- a/target/arm/tlb_helper.c
> +++ b/target/arm/tlb_helper.c
> @@ -226,21 +226,31 @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int 
> size,
>  _addr, , , _size,
>  , );
>  if (likely(!ret)) {
> +PageEntryExtra extra = {};
> +
>  /*
>   * Map a single [sub]page. Regions smaller than our declared
>   * target page size are handled specially, so for those we
> - * pass in the exact addresses.
> + * pass in the exact addresses.  This only happens for M-profile,
> + * which does not use or require PageEntryExtra.
>   */

Do we have to exclude M-profile here because the PageEntryExtra
data is strictly-per-page, or because the way we've formatted
our extra uint64_t requires the physaddr to be page-aligned, or both?

>  if (page_size >= TARGET_PAGE_SIZE) {
>  phys_addr &= TARGET_PAGE_MASK;
>  address &= TARGET_PAGE_MASK;
> +
> +/* Record some particulars for later lookup. */
> +extra.x = phys_addr;
> +extra.x = FIELD_DP64(extra.x, PAGEENTRYEXTRA, ATTRS,
> + cacheattrs.attrs);
> +extra.x = FIELD_DP64(extra.x, PAGEENTRYEXTRA, SHAREABILITY,
> + cacheattrs.shareability);
>  }
>  /* Notice and record tagged memory. */
>  if (cpu_isar_feature(aa64_mte, cpu) && cacheattrs.attrs == 0xf0) {
>  arm_tlb_mte_tagged() = true;
>  }
>
> -tlb_set_page_with_attrs(cs, address, phys_addr, attrs,
> +tlb_set_page_with_extra(cs, address, phys_addr, attrs, extra,
>  prot, mmu_idx, page_size);
>  return true;
>  } else if (probe) {
> --
> 2.34.1

thanks
-- PMM

Re: [PATCH 03/18] block: Change blk_{pread,pwrite}() param order

[Hanna Reitz]

On 17.05.22 13:37, Alberto Faria wrote:

Swap 'buf' and 'bytes' around for consistency with
blk_co_{pread,pwrite}(), and in preparation to implement these functions
using generated_co_wrapper.

Callers were updated using this Coccinelle script:

 @@ expression blk, offset, buf, bytes, flags; @@
 - blk_pread(blk, offset, buf, bytes, flags)
 + blk_pread(blk, offset, bytes, buf, flags)

 @@ expression blk, offset, buf, bytes, flags; @@
 - blk_pwrite(blk, offset, buf, bytes, flags)
 + blk_pwrite(blk, offset, bytes, buf, flags)

It had no effect on hw/block/nand.c, presumably due to the #if, so that
file was updated manually.

Overly-long lines were then fixed by hand.

Signed-off-by: Alberto Faria 
---
  block.c   |  2 +-
  block/block-backend.c |  4 +--
  block/commit.c|  4 +--
  block/crypto.c|  2 +-
  block/export/fuse.c   |  4 +--
  block/parallels.c |  2 +-
  block/qcow.c  |  8 +++---
  block/qcow2.c |  4 +--
  block/qed.c   |  8 +++---
  block/vdi.c   |  4 +--
  block/vhdx.c  | 20 ++---
  block/vmdk.c  | 10 +++
  block/vpc.c   | 12 
  hw/arm/allwinner-h3.c |  2 +-
  hw/arm/aspeed.c   |  2 +-
  hw/block/block.c  |  2 +-
  hw/block/fdc.c| 20 ++---
  hw/block/hd-geometry.c|  2 +-
  hw/block/m25p80.c |  2 +-
  hw/block/nand.c   | 47 ---
  hw/block/onenand.c| 32 ++---
  hw/block/pflash_cfi01.c   |  4 +--
  hw/block/pflash_cfi02.c   |  4 +--
  hw/ide/atapi.c|  4 +--
  hw/misc/mac_via.c |  4 +--
  hw/misc/sifive_u_otp.c| 14 -
  hw/nvram/eeprom_at24c.c   |  4 +--
  hw/nvram/spapr_nvram.c|  6 ++--
  hw/nvram/xlnx-bbram.c |  4 +--
  hw/nvram/xlnx-efuse.c |  4 +--
  hw/ppc/pnv_pnor.c |  6 ++--
  hw/sd/sd.c|  4 +--
  include/sysemu/block-backend-io.h |  4 +--
  migration/block.c |  6 ++--
  nbd/server.c  |  8 +++---
  qemu-img.c| 18 ++--
  qemu-io-cmds.c|  4 +--
  tests/unit/test-block-iothread.c  |  8 +++---
  38 files changed, 150 insertions(+), 149 deletions(-)


Reviewed-by: Hanna Reitz 

Re: [PATCH v10 09/12] target/riscv: Simplify counter predicate function

[Weiwei Li]

在 2022/6/21 上午7:15, Atish Patra 写道:

All the hpmcounters and the fixed counters (CY, IR, TM) can be represented
as a unified counter. Thus, the predicate function doesn't need handle each
case separately.

Simplify the predicate function so that we just handle things differently
between RV32/RV64 and S/HS mode.

Reviewed-by: Bin Meng 
Acked-by: Alistair Francis 
Signed-off-by: Atish Patra 
---
  target/riscv/csr.c | 112 +
  1 file changed, 11 insertions(+), 101 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 2664ce265784..9367e2af9b90 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -74,6 +74,7 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
  CPUState *cs = env_cpu(env);
  RISCVCPU *cpu = RISCV_CPU(cs);
  int ctr_index;
+target_ulong ctr_mask;
  int base_csrno = CSR_CYCLE;
  bool rv32 = riscv_cpu_mxl(env) == MXL_RV32 ? true : false;
  
@@ -82,122 +83,31 @@ static RISCVException ctr(CPURISCVState *env, int csrno)

  base_csrno += 0x80;
  }
  ctr_index = csrno - base_csrno;
+ctr_mask = BIT(ctr_index);
  
  if ((csrno >= CSR_CYCLE && csrno <= CSR_INSTRET) ||

  (csrno >= CSR_CYCLEH && csrno <= CSR_INSTRETH)) {
  goto skip_ext_pmu_check;
  }
  
-if ((!cpu->cfg.pmu_num || !(cpu->pmu_avail_ctrs & BIT(ctr_index {

+if ((!cpu->cfg.pmu_num || !(cpu->pmu_avail_ctrs & ctr_mask))) {
  /* No counter is enabled in PMU or the counter is out of range */
  return RISCV_EXCP_ILLEGAL_INST;
  }
  
  skip_ext_pmu_check:
  
-if (env->priv == PRV_S) {

-switch (csrno) {
-case CSR_CYCLE:
-if (!get_field(env->mcounteren, COUNTEREN_CY)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-case CSR_TIME:
-if (!get_field(env->mcounteren, COUNTEREN_TM)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-case CSR_INSTRET:
-if (!get_field(env->mcounteren, COUNTEREN_IR)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-case CSR_HPMCOUNTER3...CSR_HPMCOUNTER31:
-if (!get_field(env->mcounteren, 1 << ctr_index)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-}
-if (rv32) {
-switch (csrno) {
-case CSR_CYCLEH:
-if (!get_field(env->mcounteren, COUNTEREN_CY)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-case CSR_TIMEH:
-if (!get_field(env->mcounteren, COUNTEREN_TM)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-case CSR_INSTRETH:
-if (!get_field(env->mcounteren, COUNTEREN_IR)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-case CSR_HPMCOUNTER3H...CSR_HPMCOUNTER31H:
-if (!get_field(env->mcounteren, 1 << ctr_index)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-}
-}
+if (((env->priv == PRV_S) && (!get_field(env->mcounteren, ctr_mask))) ||
+   ((env->priv == PRV_U) && (!get_field(env->scounteren, ctr_mask {
+return RISCV_EXCP_ILLEGAL_INST;
  }


Sorry. I didn't realize this simplification and sent a similar patch to 
fix the problems in Xcounteren


related check I found when I tried to learn the patchset for state 
enable extension two days ago.


I think there are several difference between our understanding, 
following is my modifications:


+if (csrno <= CSR_HPMCOUNTER31 && csrno >= CSR_CYCLE) {
+field = 1 << (csrno - CSR_CYCLE);
+} else if (riscv_cpu_mxl(env) == MXL_RV32 && csrno <= CSR_HPMCOUNTER31H &&
+   csrno >= CSR_CYCLEH) {
+field = 1 << (csrno - CSR_CYCLEH);
+}
+
+if (env->priv < PRV_M && !get_field(env->mcounteren, field)) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+if (riscv_cpu_virt_enabled(env) && !get_field(env->hcounteren, field)) {
+return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+}
+
+if (riscv_has_ext(env, RVS) && env->priv == PRV_U &&
+!get_field(env->scounteren, field)) {
+if (riscv_cpu_virt_enabled(env)) {
+return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+} else {
+return RISCV_EXCP_ILLEGAL_INST;
 }
 }


1) For any less-privileged mode under M, illegal exception is raised if matching
bit in mcounteren is zero.

2) For VS/VU mode('H' extension is supported implicitly), virtual instruction
exception is raised if matching bit in hcounteren is zero.

3) scounteren csr only works in U/VU mode when 'S' extension is supported:
   For U mode, illegal exception is raised if matching bit in 

Re: [PULL 00/23] loongarch64 patch queue

[Richard Henderson]

On 7/4/22 15:03, Richard Henderson wrote:

The following changes since commit e8e86b484eac70cd86e15fa10a2f0038a536cbba:

   Merge tag 'pull-riscv-to-apply-20220703-1' of github.com:alistair23/qemu 
into staging (2022-07-03 06:29:02 +0530)

are available in the Git repository at:

   https://gitlab.com/rth7680/qemu.git tags/pull-la-20220704

for you to fetch changes up to eb1e9ff8bba91674b4321f2b075c55aa8d9948cc:

   target/loongarch: Add lock when writing timer clear reg (2022-07-04 11:08:58 
+0530)


LoongArch patch queue:
   Support linux-user.
   Fixes for CSR BADV.
   Fix ASRT{LE,GT} exception.
   Fixes for LS7A RTC.
   Fix for interrupt vector spacing.


Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/7.1 as 
appropriate.


r~





Mao Bibo (1):
   hw/intc/loongarch_pch_msi: Fix msi vector convertion

Song Gao (13):
   linux-user: Add LoongArch generic header files
   linux-user: Add LoongArch signal support
   linux-user: Add LoongArch elf support
   linux-user: Add LoongArch syscall support
   linux-user: Add LoongArch cpu_loop support
   scripts: add loongarch64 binfmt config
   target/loongarch: remove badaddr from CPULoongArch
   target/loongarch: Fix missing update CSR_BADV
   target/loongarch: Fix helper_asrtle_d/asrtgt_d raise wrong exception
   target/loongarch: remove unused include hw/loader.h
   target/loongarch: Adjust functions and structure to support user-mode
   default-configs: Add loongarch linux-user support
   target/loongarch: Update README

Xiaojuan Yang (9):
   hw/rtc/ls7a_rtc: Fix uninitialied bugs and toymatch writing function
   hw/rtc/ls7a_rtc: Fix timer call back function
   hw/rtc/ls7a_rtc: Remove unimplemented device in realized function
   hw/rtc/ls7a_rtc: Add reset function
   hw/rtc/ls7a_rtc: Fix rtc enable and disable function
   hw/rtc/ls7a_rtc: Use tm struct pointer as arguments in toy_time_to_val()
   hw/rtc/ls7a_rtc: Fix 'calculate' spelling errors
   target/loongarch: Fix the meaning of ECFG reg's VS field
   target/loongarch: Add lock when writing timer clear reg

  configs/targets/loongarch64-linux-user.mak |   3 +
  include/hw/intc/loongarch_pch_msi.h|   2 +
  linux-user/loongarch64/sockbits.h  |  11 +
  linux-user/loongarch64/syscall_nr.h| 312 +++
  linux-user/loongarch64/target_cpu.h|  34 +++
  linux-user/loongarch64/target_elf.h|  12 +
  linux-user/loongarch64/target_errno_defs.h |  12 +
  linux-user/loongarch64/target_fcntl.h  |  11 +
  linux-user/loongarch64/target_prctl.h  |   1 +
  linux-user/loongarch64/target_resource.h   |  11 +
  linux-user/loongarch64/target_signal.h |  13 +
  linux-user/loongarch64/target_structs.h|  11 +
  linux-user/loongarch64/target_syscall.h|  48 +++
  linux-user/loongarch64/termbits.h  |  11 +
  linux-user/syscall_defs.h  |   6 +-
  target/loongarch/cpu.h |   8 +-
  target/loongarch/helper.h  |   2 +
  target/loongarch/internals.h   |   2 +
  hw/intc/loongarch_pch_msi.c|  22 +-
  hw/loongarch/loongson3.c   |   1 +
  hw/rtc/ls7a_rtc.c  | 131 
  linux-user/elfload.c   |  91 ++
  linux-user/loongarch64/cpu_loop.c  |  96 ++
  linux-user/loongarch64/signal.c| 335 +
  target/loongarch/cpu.c |  38 ++-
  target/loongarch/csr_helper.c  |   2 +
  target/loongarch/gdbstub.c |   2 +-
  target/loongarch/op_helper.c   |  10 +-
  target/loongarch/insn_trans/trans_privileged.c.inc |  36 +++
  scripts/gensyscalls.sh |   2 +
  scripts/qemu-binfmt-conf.sh|   6 +-
  target/loongarch/README|  39 ++-
  32 files changed, 1226 insertions(+), 95 deletions(-)
  create mode 100644 configs/targets/loongarch64-linux-user.mak
  create mode 100644 linux-user/loongarch64/sockbits.h
  create mode 100644 linux-user/loongarch64/syscall_nr.h
  create mode 100644 linux-user/loongarch64/target_cpu.h
  create mode 100644 linux-user/loongarch64/target_elf.h
  create mode 100644 linux-user/loongarch64/target_errno_defs.h
  create mode 100644 linux-user/loongarch64/target_fcntl.h
  create mode 100644 linux-user/loongarch64/target_prctl.h
  create mode 100644 linux-user/loongarch64/target_resource.h
  create mode 100644 linux-user/loongarch64/target_signal.h
  create mode 100644 linux-user/loongarch64

Re: [PATCH v3 0/4] rSTify a few more docs; move them to QEMU Git

[Thomas Huth]

On 01/07/2022 10.53, Kashyap Chamarthy wrote:

Ping.

Thomas/Peter: when you get some time, please have a look at this.


I still had a question/request here that was not answered:

https://lore.kernel.org/qemu-devel/2fca7b7e-f95d-eae1-9973-9ede30cac...@redhat.com/

Also, for the first patch, I'd like to see a Reviewed-by by one of the 
security folks whether it's ok for them to move this page to a location that 
has different access rights.


 Thomas



On Mon, Jun 06, 2022 at 06:55:47PM +0200, Kashyap Chamarthy wrote:

On Mon, Jun 06, 2022 at 06:49:49PM +0200, Kashyap Chamarthy wrote:

Oops, messed up v3's cover-letter subject; now fixed.  Sorry.


Sigh, instead of "v3", I accidentally wrote "v4" in the cover-letter
subject.  Now fix that too; sorry for the noise.


On Mon, Jun 06, 2022 at 06:43:32PM +0200, Kashyap Chamarthy wrote:

- Add back the "" fragment in security-process.rst
   [Thomas]
- Add a docs/about/contacting-the-project.rst as per Peter's feedback
   here:
   https://lists.nongnu.org/archive/html/qemu-devel/2022-03/msg05178.html
   [pm215]
- Keep Thomas' R-by for "docs: rSTify MailingLists wiki; move it to QEMU
   Git"

v2 cover letter:
---
rSTify a few more docs; move them to QEMU Git

This series rST-ifies:

   - security-process[1]
   - MailingLists[2]
   - GettingStartedDevelopers[3]

The 'security-process' page is from the QEMU web and is moved to
docs/devel/ in QEMU Git.  This is based on Paolo's feedback here[4].
The next two docs are converted from the Wiki.

[1] https://www.qemu.org/contribute/security-process
[2] https://wiki.qemu.org/Contribute/MailingLists
[3] https://wiki.qemu.org/Documentation/GettingStartedDevelopers
[4] https://lists.nongnu.org/archive/html/qemu-devel/2021-11/msg04002.html
---

Kashyap Chamarthy (4):
   docs: rSTify "security-process" page; move it to QEMU Git
   docs: rSTify MailingLists wiki; move it to QEMU Git
   docs: rSTify GettingStartedDevelopers wiki; move it to QEMU Git
   Add a new doc "contacting-the-project.rst"

  docs/about/contacting-the-project.rst |  16 ++
  docs/about/index.rst  |   1 +
  docs/devel/getting-started-developers.rst | 200 ++
  docs/devel/index.rst  |   3 +
  docs/devel/mailing-lists.rst  |  51 ++
  docs/devel/security-process.rst   | 190 
  6 files changed, 461 insertions(+)
  create mode 100644 docs/about/contacting-the-project.rst
  create mode 100644 docs/devel/getting-started-developers.rst
  create mode 100644 docs/devel/mailing-lists.rst
  create mode 100644 docs/devel/security-process.rst

--
2.36.1



--
/kashyap


--
/kashyap

Re: [PULL v2 0/9] Block jobs & NBD patches

[Vladimir Sementsov-Ogievskiy]

On 7/1/22 20:02, John Snow wrote:

On Wed, Jun 29, 2022 at 7:18 PM Richard Henderson
 wrote:


On 6/29/22 13:45, Vladimir Sementsov-Ogievskiy wrote:

The following changes since commit ad4c7f529a279685da84297773b4ec8080153c2d:

Merge tag 'pull-semi-20220628' of https://gitlab.com/rth7680/qemu into 
staging (2022-06-28 10:24:31 +0530)

are available in the Git repository at:

https://gitlab.com/vsementsov/qemu.git tags/pull-block-2022-06-14-v2

for you to fetch changes up to 1b8f777673985af366de099ad4e41d334b36fb12:

block: use 'unsigned' for in_flight field on driver state (2022-06-29 
10:57:02 +0300)


Block jobs & NBD patches

v2: - add arguments to QEMUMachine constructor in test, to make it work
on arm in gitlab pipeline
  - use bdrv_inc_in_flight() / bdrv_dec_in_flight() instead of direct
manipulation with bs->in_flight


Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/7.1 as 
appropriate.


r~




- add new options for copy-before-write filter
- new trace points for NBD
- prefer unsigned type for some 'in_flight' fields

Denis V. Lunev (2):
nbd: trace long NBD operations
block: use 'unsigned' for in_flight field on driver state

Vladimir Sementsov-Ogievskiy (7):
block/copy-before-write: refactor option parsing
block/copy-before-write: add on-cbw-error open parameter
iotests: add copy-before-write: on-cbw-error tests
util: add qemu-co-timeout
block/block-copy: block_copy(): add timeout_ns parameter
block/copy-before-write: implement cbw-timeout option
iotests: copy-before-write: add cases for cbw-timeout option

   block/block-copy.c|  33 ++-
   block/copy-before-write.c | 110 ++---
   block/mirror.c|   2 +-
   block/nbd.c   |   8 +-
   block/trace-events|   2 +
   include/block/block-copy.h|   4 +-
   include/qemu/coroutine.h  |  13 ++
   nbd/client-connection.c   |   2 +
   nbd/trace-events  |   3 +
   qapi/block-core.json  |  31 ++-
   tests/qemu-iotests/pylintrc   |   5 +
   tests/qemu-iotests/tests/copy-before-write| 216 ++
   .../qemu-iotests/tests/copy-before-write.out  |   5 +
   util/meson.build  |   1 +
   util/qemu-co-timeout.c|  89 
   15 files changed, 482 insertions(+), 42 deletions(-)
   create mode 100755 tests/qemu-iotests/tests/copy-before-write
   create mode 100644 tests/qemu-iotests/tests/copy-before-write.out
   create mode 100644 util/qemu-co-timeout.c






iotests: copy-before-write: add cases for cbw-timeout option

- This is causing the FreeBSD VM tests to regress for me, because the
new iotest is failing there. Haven't diagnosed further yet, but I will
update this thread if I get better info.



Like other problems around this test (I had a hard debugging session for the 
problem that reproduces only on gitlab pipline :/, it may relate to the fact 
that I use QEMUMachine class directly and avoid -accel qtest. Also, to fix test 
on ARM, I've added -machine none.


--
Best regards,
Vladimir

Re: [PATCH 00/62] target/arm: Implement FEAT_HAFDBS

[Peter Maydell]

On Sun, 3 Jul 2022 at 09:25, Richard Henderson
 wrote:
>
> This is a major reorg to arm page table walking.  While the result
> here is "merely" Hardware-assited Access Flag and Dirty Bit Setting
> (HAFDBS), the ultimate goal is the Realm Management Extension (RME).
> RME "recommends" that HAFDBS be implemented (I_CSLWZ).
>
> For HAFDBS, being able to find a host pointer for the ram that
> backs a given page table entry is required in order to perform the
> atomic update to that PTE.  The easiest way to find a host pointer
> is to use the existing softtlb mechanism.  Thus all of the page
> table walkers have been adjusted to take an mmu_idx that corresponds
> to the regime in which the page table is stored.  In some cases,
> this is a new "physical" mmu_idx that has a permanent 1-1 mapping.
>
> For RME, "physical" addresses also have page permissions, coming
> from the Root realm Granule Protection Table, which can be thought
> of as a third stage page table lookup.  So eventually the new
> Secure and Nonsecure physical mmu indexes will joined by
> Realm and Root physical mmu indexes, and all of them will take
> the new Granule Page Table into account.
>
> Previously, we had A-profile allocate separate mmu_idx for secure
> vs non-secure.  I've done away with that.  Now, I flush all mmu_idx
> when SCR_EL3.NS is changed.  I did not see how we could reasonably
> add 8 more mmu_idx for Realm.  Moreover, I had a look through ARM
> Trusted Firmware, at the code paths used to change between Secure
> and Nonsecure.  We wind up flushing all of these mmu_idx anyway while
> swapping the EL1+EL2 cpregs, so there is no gain at all in attempting
> to keep them live at the same time within qemu.

Is there no SMC/interrupt/etc at all which is handled as a "just do the
thing at EL3" without dropping down to secure EL2/EL1 ?

thanks
-- PMM

Re: [PATCH 00/62] target/arm: Implement FEAT_HAFDBS

[Richard Henderson]

On 7/4/22 20:24, Peter Maydell wrote:

Previously, we had A-profile allocate separate mmu_idx for secure
vs non-secure.  I've done away with that.  Now, I flush all mmu_idx
when SCR_EL3.NS is changed.  I did not see how we could reasonably
add 8 more mmu_idx for Realm.  Moreover, I had a look through ARM
Trusted Firmware, at the code paths used to change between Secure
and Nonsecure.  We wind up flushing all of these mmu_idx anyway while
swapping the EL1+EL2 cpregs, so there is no gain at all in attempting
to keep them live at the same time within qemu.


Is there no SMC/interrupt/etc at all which is handled as a "just do the
thing at EL3" without dropping down to secure EL2/EL1 ?


I'm sure there is, but it's only swapping between S EL[012] and NS EL[012] that concerned 
me.  Is there something that I'm missing?



r~

Re: [PATCH] stubs: update replay-tools to match replay.h types

[Thomas Huth]

On 04/07/2022 09.58, Claudio Fontana wrote:

detected with GCC 13 [-Werror=enum-int-mismatch]

Solves Issue #1096.

Signed-off-by: Claudio Fontana 
Cc: Pavel Dovgalyuk 
---
  stubs/replay-tools.c | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/stubs/replay-tools.c b/stubs/replay-tools.c
index 43296b3d4e..f2e72bb225 100644
--- a/stubs/replay-tools.c
+++ b/stubs/replay-tools.c
@@ -7,13 +7,14 @@ bool replay_events_enabled(void)
  return false;
  }
  
-int64_t replay_save_clock(unsigned int kind, int64_t clock, int64_t raw_icount)

+int64_t replay_save_clock(ReplayClockKind kind,
+  int64_t clock, int64_t raw_icount)
  {
  abort();
  return 0;
  }
  
-int64_t replay_read_clock(unsigned int kind, int64_t raw_icount)

+int64_t replay_read_clock(ReplayClockKind kind, int64_t raw_icount)
  {
  abort();
  return 0;
@@ -48,11 +49,11 @@ void replay_mutex_unlock(void)
  {
  }
  
-void replay_register_char_driver(Chardev *chr)

+void replay_register_char_driver(struct Chardev *chr)
  {
  }
  
-void replay_chr_be_write(Chardev *s, uint8_t *buf, int len)

+void replay_chr_be_write(struct Chardev *s, uint8_t *buf, int len)
  {
  abort();
  }


Reviewed-by: Thomas Huth 

Re: [PATCH] po: add ukrainian translation

[Thomas Huth]

On 13/06/2022 14.37, Andrij Mizyk wrote:

Signed-off-by: Andrij Mizyk 
---
  po/LINGUAS |  1 +
  po/uk.po   | 75 ++
  2 files changed, 76 insertions(+)
  create mode 100644 po/uk.po


Thanks!

Unfortunately, it seems like we currently don't have a maintainer for the 
.po files anymore ... so maybe this patch can go via the trivial tree (now 
in CC:), or I'll add it to my next pull request if I don't forget it...


 Thomas