Re: [PATCH 5/8] Adds basic CPU hot-(un)plug support for Loongarch
Hi Igor Mammedov:
On 7/28/23 9:21 PM, Igor Mammedov wrote:
On Thu, 20 Jul 2023 15:15:10 +0800
xianglai li wrote:
1.Add CPU topology related functions
2.Add CPU hot-plug related hook functions
3.Update the in-place CPU creation process at machine initialization
patch is to large, split it at least on those ^^ 3 parts,
which would do a single distinct thing.
After that it will be easier to review this.
Ok, I'll split this patch further.
Also looking at hw/loongarch/acpi-build.c
you have cpu_index == arch_id == core_id /according to comments/
and they are mixed/used interchangeably. which is confusing
at least. So clean it up first to use arch_id consistently
then a separate patches to introduce socket/core/thread support
with proper documentation/pointers to specs as to how arch_id
should be calculated.
And once that is ready, add hotplug on top of it.
Okay, I'll do it according to your suggestion.
Cc: Xiaojuan Yang
Cc: Song Gao
Cc: "Michael S. Tsirkin"
Cc: Igor Mammedov
Cc: Ani Sinha
Cc: Paolo Bonzini
Cc: Richard Henderson
Cc: Eduardo Habkost
Cc: Marcel Apfelbaum
Cc: "Philippe Mathieu-Daudé"
Cc: Yanan Wang
Cc: "Daniel P. Berrangé"
Cc: Peter Xu
Cc: David Hildenbrand
Signed-off-by: xianglai li
---
hw/loongarch/virt.c | 381 ++--
include/hw/loongarch/virt.h | 11 +-
target/loongarch/cpu.h | 4 +
3 files changed, 382 insertions(+), 14 deletions(-)
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index e19b042ce8..5919389f42 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -46,6 +46,9 @@
#include "hw/block/flash.h"
#include "qemu/error-report.h"
+static int virt_get_socket_id(const MachineState *ms, int cpu_index);
+static int virt_get_core_id(const MachineState *ms, int cpu_index);
+static int virt_get_thread_id(const MachineState *ms, int cpu_index);
static void virt_flash_create(LoongArchMachineState *lams)
{
@@ -447,12 +450,12 @@ static DeviceState *create_acpi_ged(DeviceState *pch_pic,
LoongArchMachineState
{
DeviceState *dev;
MachineState *ms = MACHINE(lams);
-uint32_t event = ACPI_GED_PWR_DOWN_EVT;
+uint32_t event = ACPI_GED_PWR_DOWN_EVT | ACPI_GED_CPU_HOTPLUG_EVT;
if (ms->ram_slots) {
event |= ACPI_GED_MEM_HOTPLUG_EVT;
}
-dev = qdev_new(TYPE_ACPI_GED);
+dev = qdev_new(TYPE_ACPI_GED_LOONGARCH);
qdev_prop_set_uint32(dev, "ged-event", event);
/* ged event */
@@ -461,6 +464,7 @@ static DeviceState *create_acpi_ged(DeviceState *pch_pic,
LoongArchMachineState
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, VIRT_GED_MEM_ADDR);
/* ged regs used for reset and power down */
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, VIRT_GED_REG_ADDR);
+sysbus_mmio_map(SYS_BUS_DEVICE(dev), 3, VIRT_GED_CPUHP_ADDR);
sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
qdev_get_gpio_in(pch_pic, VIRT_SCI_IRQ -
VIRT_GSI_BASE));
@@ -583,6 +587,7 @@ static void loongarch_irq_init(LoongArchMachineState *lams)
extioi = qdev_new(TYPE_LOONGARCH_EXTIOI);
sysbus_realize_and_unref(SYS_BUS_DEVICE(extioi), &error_fatal);
+lams->extioi = extioi;
/*
* The connection of interrupts:
@@ -624,11 +629,11 @@ static void loongarch_irq_init(LoongArchMachineState
*lams)
sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi),
1));
/*
-* extioi iocsr memory region
-* only one extioi is added on loongarch virt machine
-* external device interrupt can only be routed to cpu 0-3
-*/
- if (cpu < EXTIOI_CPUS)
+ * extioi iocsr memory region
+ * only one extioi is added on loongarch virt machine
+ * external device interrupt can only be routed to cpu 0-3
+ */
+if (cpu < EXTIOI_CPUS)
memory_region_add_subregion(&env->system_iocsr, APIC_BASE,
sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi),
cpu));
@@ -789,7 +794,6 @@ static void loongarch_init(MachineState *machine)
NodeInfo *numa_info = machine->numa_state->nodes;
int i;
hwaddr fdt_base;
-const CPUArchIdList *possible_cpus;
MachineClass *mc = MACHINE_GET_CLASS(machine);
CPUState *cpu;
char *ramName = NULL;
@@ -810,12 +814,40 @@ static void loongarch_init(MachineState *machine)
create_fdt(lams);
/* Init CPUs */
-possible_cpus = mc->possible_cpu_arch_ids(machine);
-for (i = 0; i < possible_cpus->len; i++) {
-cpu = cpu_create(machine->cpu_type);
+mc->possible_cpu_arch_ids(machine);
+
+for (i = 0; i < machine->smp.cpus; i++) {
+Object *cpuobj;
+cpuobj = object_new(machine->cpu_type);
+
+cpu = CPU(cpuobj);
cpu->cpu_index = i;
I'd move this to foo_cpu_pre_plug()
I guess I don't need to assign a value to the cpu
Re: [PATCH v4 11/11] target/loongarch: Add loongarch32 cpu la132
On 2023/8/9 03:26, Richard Henderson wrote:
On 8/7/23 18:54, Jiajie Chen wrote:
+static void loongarch_la464_initfn(Object *obj)
+{
+ LoongArchCPU *cpu = LOONGARCH_CPU(obj);
+ CPULoongArchState *env = &cpu->env;
+
+ loongarch_cpu_initfn_common(env);
+
+ cpu->dtb_compatible = "loongarch,Loongson-3A5000";
+ env->cpucfg[0] = 0x14c010; /* PRID */
+
+ uint32_t data = env->cpucfg[1];
+ data = FIELD_DP32(data, CPUCFG1, ARCH, 2); /* LA64 */
+ data = FIELD_DP32(data, CPUCFG1, PALEN, 0x2f); /* 48 bits */
+ data = FIELD_DP32(data, CPUCFG1, VALEN, 0x2f); /* 48 bits */
+ data = FIELD_DP32(data, CPUCFG1, RI, 1);
+ data = FIELD_DP32(data, CPUCFG1, EP, 1);
+ data = FIELD_DP32(data, CPUCFG1, RPLV, 1);
+ env->cpucfg[1] = data;
+}
+
+static void loongarch_la132_initfn(Object *obj)
+{
+ LoongArchCPU *cpu = LOONGARCH_CPU(obj);
+ CPULoongArchState *env = &cpu->env;
+
+ loongarch_cpu_initfn_common(env);
+
+ cpu->dtb_compatible = "loongarch,Loongson-1C103";
+
+ uint32_t data = env->cpucfg[1];
+ data = FIELD_DP32(data, CPUCFG1, ARCH, 1); /* LA32 */
+ data = FIELD_DP32(data, CPUCFG1, PALEN, 0x1f); /* 32 bits */
+ data = FIELD_DP32(data, CPUCFG1, VALEN, 0x1f); /* 32 bits */
+ data = FIELD_DP32(data, CPUCFG1, RI, 0);
+ data = FIELD_DP32(data, CPUCFG1, EP, 0);
+ data = FIELD_DP32(data, CPUCFG1, RPLV, 0);
+ env->cpucfg[1] = data;
+}
The use of the loongarch_cpu_initfn_common function is not going to
scale.
Compare the set of *_initfn in target/arm/tcg/cpu32.c
In general, you want to copy data in bulk from the processor manual,
so that the reviewer can simply read through the table and see that
the code is correct, without having to check between multiple
functions to see that the combination is correct.
For our existing la464, that table is Table 54 in the 3A5000 manual.
Is there a public specification for the la132? I could not find one
in https://www.loongson.cn/EN/product/, but perhaps that's just the
english view.
There seems no, even from the chinese view.
r~
Re: [PATCH v4 1/9] virtio: Add shared memory capability
On Wed, Aug 09, 2023 at 10:11:00AM +0800, Gurchetan Singh wrote:
> From: "Dr. David Alan Gilbert"
>
> Define a new capability type 'VIRTIO_PCI_CAP_SHARED_MEMORY_CFG' to allow
> defining shared memory regions with sizes and offsets of 2^32 and more.
> Multiple instances of the capability are allowed and distinguished
> by a device-specific 'id'.
>
> Signed-off-by: Dr. David Alan Gilbert
> Signed-off-by: Antonio Caggiano
> Reviewed-by: Gurchetan Singh
> Signed-off-by: Gurchetan Singh
> Tested-by: Alyssa Ross
> Reviewed-by: Akihiko Odaki
Acked-and-Tested-by: Huang Rui
> ---
> hw/virtio/virtio-pci.c | 18 ++
> include/hw/virtio/virtio-pci.h | 4
> 2 files changed, 22 insertions(+)
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index edbc0daa18..da8c9ea12d 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1435,6 +1435,24 @@ static int virtio_pci_add_mem_cap(VirtIOPCIProxy
> *proxy,
> return offset;
> }
>
> +int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
> + uint8_t bar, uint64_t offset, uint64_t length,
> + uint8_t id)
> +{
> +struct virtio_pci_cap64 cap = {
> +.cap.cap_len = sizeof cap,
> +.cap.cfg_type = VIRTIO_PCI_CAP_SHARED_MEMORY_CFG,
> +};
> +
> +cap.cap.bar = bar;
> +cap.cap.length = cpu_to_le32(length);
> +cap.length_hi = cpu_to_le32(length >> 32);
> +cap.cap.offset = cpu_to_le32(offset);
> +cap.offset_hi = cpu_to_le32(offset >> 32);
> +cap.cap.id = id;
> +return virtio_pci_add_mem_cap(proxy, &cap.cap);
> +}
> +
> static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr,
> unsigned size)
> {
> diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
> index ab2051b64b..5a3f182f99 100644
> --- a/include/hw/virtio/virtio-pci.h
> +++ b/include/hw/virtio/virtio-pci.h
> @@ -264,4 +264,8 @@ unsigned virtio_pci_optimal_num_queues(unsigned
> fixed_queues);
> void virtio_pci_set_guest_notifier_fd_handler(VirtIODevice *vdev, VirtQueue
> *vq,
>int n, bool assign,
>bool with_irqfd);
> +
> +int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy, uint8_t bar, uint64_t
> offset,
> + uint64_t length, uint8_t id);
> +
> #endif
> --
> 2.41.0.640.ga95def55d0-goog
>
[PATCH for-8.2] dockerfiles: bump tricore cross compiler container to Debian 11
With the release of version 12 on June 10, 2023, Debian 10 is not supported anymore. Modify the cross compiler container to build on a newer version. Signed-off-by: Paolo Bonzini --- tests/docker/dockerfiles/debian-tricore-cross.docker | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/docker/dockerfiles/debian-tricore-cross.docker b/tests/docker/dockerfiles/debian-tricore-cross.docker index 269bfa8d423..5bd1963fb55 100644 --- a/tests/docker/dockerfiles/debian-tricore-cross.docker +++ b/tests/docker/dockerfiles/debian-tricore-cross.docker @@ -9,7 +9,7 @@ # # SPDX-License-Identifier: GPL-2.0-or-later # -FROM docker.io/library/debian:buster-slim +FROM docker.io/library/debian:11-slim MAINTAINER Philippe Mathieu-Daudé -- 2.41.0
[PATCH for-8.2] configure: fix and complete detection of tricore tools
The tricore tools are not detected when they are installed in
the host system, only if they are taken from an external
container. For this reason the build-tricore-softmmu job
was not running the TCG tests.
In addition the container provides all tools, not just as/ld/gcc,
so there is no need to special case tricore.
Signed-off-by: Paolo Bonzini
---
configure | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/configure b/configure
index 133f4e32351..f2bd8858d6c 100755
--- a/configure
+++ b/configure
@@ -1271,6 +1271,7 @@ fi
: ${cross_prefix_sh4="sh4-linux-gnu-"}
: ${cross_prefix_sparc64="sparc64-linux-gnu-"}
: ${cross_prefix_sparc="$cross_prefix_sparc64"}
+: ${cross_prefix_tricore="tricore-"}
: ${cross_prefix_x86_64="x86_64-linux-gnu-"}
: ${cross_cc_aarch64_be="$cross_cc_aarch64"}
@@ -1458,10 +1459,6 @@ probe_target_compiler() {
tricore)
container_image=debian-tricore-cross
container_cross_prefix=tricore-
-container_cross_as=tricore-as
-container_cross_ld=tricore-ld
-container_cross_cc=tricore-gcc
-break
;;
x86_64)
container_image=debian-amd64-cross
--
2.41.0
[PATCH v5 01/11] target/loongarch: Add function to check current arch
Add is_la64 function to check if the current cpucfg[1].arch equals to
2(LA64).
Signed-off-by: Jiajie Chen
Co-authored-by: Richard Henderson
Reviewed-by: Richard Henderson
---
target/loongarch/cpu.h | 10 ++
1 file changed, 10 insertions(+)
diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
index fa371ca8ba..5a71d64a04 100644
--- a/target/loongarch/cpu.h
+++ b/target/loongarch/cpu.h
@@ -132,6 +132,11 @@ FIELD(CPUCFG1, HP, 24, 1)
FIELD(CPUCFG1, IOCSR_BRD, 25, 1)
FIELD(CPUCFG1, MSG_INT, 26, 1)
+/* cpucfg[1].arch */
+#define CPUCFG1_ARCH_LA32R 0
+#define CPUCFG1_ARCH_LA321
+#define CPUCFG1_ARCH_LA642
+
/* cpucfg[2] bits */
FIELD(CPUCFG2, FP, 0, 1)
FIELD(CPUCFG2, FP_SP, 1, 1)
@@ -420,6 +425,11 @@ static inline int cpu_mmu_index(CPULoongArchState *env,
bool ifetch)
#endif
}
+static inline bool is_la64(CPULoongArchState *env)
+{
+return FIELD_EX32(env->cpucfg[1], CPUCFG1, ARCH) == CPUCFG1_ARCH_LA64;
+}
+
/*
* LoongArch CPUs hardware flags.
*/
--
2.41.0
[PATCH v5 08/11] target/loongarch: Reject la64-only instructions in la32 mode
LoongArch64-only instructions are marked with regard to the instruction manual Table 2. LSX instructions are not marked for now for lack of public manual. Signed-off-by: Jiajie Chen --- target/loongarch/insn_trans/trans_arith.c.inc | 30 .../loongarch/insn_trans/trans_atomic.c.inc | 76 +-- target/loongarch/insn_trans/trans_bit.c.inc | 28 +++ .../loongarch/insn_trans/trans_branch.c.inc | 4 +- target/loongarch/insn_trans/trans_extra.c.inc | 16 ++-- target/loongarch/insn_trans/trans_fmov.c.inc | 4 +- .../loongarch/insn_trans/trans_memory.c.inc | 68 - target/loongarch/insn_trans/trans_shift.c.inc | 14 ++-- target/loongarch/translate.h | 7 ++ 9 files changed, 127 insertions(+), 120 deletions(-) diff --git a/target/loongarch/insn_trans/trans_arith.c.inc b/target/loongarch/insn_trans/trans_arith.c.inc index 43d6cf261d..4c21d8b037 100644 --- a/target/loongarch/insn_trans/trans_arith.c.inc +++ b/target/loongarch/insn_trans/trans_arith.c.inc @@ -249,9 +249,9 @@ static bool trans_addu16i_d(DisasContext *ctx, arg_addu16i_d *a) } TRANS(add_w, gen_rrr, EXT_NONE, EXT_NONE, EXT_SIGN, tcg_gen_add_tl) -TRANS(add_d, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, tcg_gen_add_tl) +TRANS_64(add_d, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, tcg_gen_add_tl) TRANS(sub_w, gen_rrr, EXT_NONE, EXT_NONE, EXT_SIGN, tcg_gen_sub_tl) -TRANS(sub_d, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, tcg_gen_sub_tl) +TRANS_64(sub_d, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, tcg_gen_sub_tl) TRANS(and, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, tcg_gen_and_tl) TRANS(or, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, tcg_gen_or_tl) TRANS(xor, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, tcg_gen_xor_tl) @@ -261,32 +261,32 @@ TRANS(orn, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, tcg_gen_orc_tl) TRANS(slt, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_slt) TRANS(sltu, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_sltu) TRANS(mul_w, gen_rrr, EXT_SIGN, EXT_SIGN, EXT_SIGN, tcg_gen_mul_tl) -TRANS(mul_d, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, tcg_gen_mul_tl) +TRANS_64(mul_d, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, tcg_gen_mul_tl) TRANS(mulh_w, gen_rrr, EXT_SIGN, EXT_SIGN, EXT_NONE, gen_mulh_w) TRANS(mulh_wu, gen_rrr, EXT_ZERO, EXT_ZERO, EXT_NONE, gen_mulh_w) -TRANS(mulh_d, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_mulh_d) -TRANS(mulh_du, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_mulh_du) -TRANS(mulw_d_w, gen_rrr, EXT_SIGN, EXT_SIGN, EXT_NONE, tcg_gen_mul_tl) -TRANS(mulw_d_wu, gen_rrr, EXT_ZERO, EXT_ZERO, EXT_NONE, tcg_gen_mul_tl) +TRANS_64(mulh_d, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_mulh_d) +TRANS_64(mulh_du, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_mulh_du) +TRANS_64(mulw_d_w, gen_rrr, EXT_SIGN, EXT_SIGN, EXT_NONE, tcg_gen_mul_tl) +TRANS_64(mulw_d_wu, gen_rrr, EXT_ZERO, EXT_ZERO, EXT_NONE, tcg_gen_mul_tl) TRANS(div_w, gen_rrr, EXT_SIGN, EXT_SIGN, EXT_SIGN, gen_div_w) TRANS(mod_w, gen_rrr, EXT_SIGN, EXT_SIGN, EXT_SIGN, gen_rem_w) TRANS(div_wu, gen_rrr, EXT_ZERO, EXT_ZERO, EXT_SIGN, gen_div_du) TRANS(mod_wu, gen_rrr, EXT_ZERO, EXT_ZERO, EXT_SIGN, gen_rem_du) -TRANS(div_d, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_div_d) -TRANS(mod_d, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_rem_d) -TRANS(div_du, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_div_du) -TRANS(mod_du, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_rem_du) +TRANS_64(div_d, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_div_d) +TRANS_64(mod_d, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_rem_d) +TRANS_64(div_du, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_div_du) +TRANS_64(mod_du, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_rem_du) TRANS(slti, gen_rri_v, EXT_NONE, EXT_NONE, gen_slt) TRANS(sltui, gen_rri_v, EXT_NONE, EXT_NONE, gen_sltu) TRANS(addi_w, gen_rri_c, EXT_NONE, EXT_SIGN, tcg_gen_addi_tl) -TRANS(addi_d, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_addi_tl) +TRANS_64(addi_d, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_addi_tl) TRANS(alsl_w, gen_rrr_sa, EXT_NONE, EXT_SIGN, gen_alsl) -TRANS(alsl_wu, gen_rrr_sa, EXT_NONE, EXT_ZERO, gen_alsl) -TRANS(alsl_d, gen_rrr_sa, EXT_NONE, EXT_NONE, gen_alsl) +TRANS_64(alsl_wu, gen_rrr_sa, EXT_NONE, EXT_ZERO, gen_alsl) +TRANS_64(alsl_d, gen_rrr_sa, EXT_NONE, EXT_NONE, gen_alsl) TRANS(pcaddi, gen_pc, gen_pcaddi) TRANS(pcalau12i, gen_pc, gen_pcalau12i) TRANS(pcaddu12i, gen_pc, gen_pcaddu12i) -TRANS(pcaddu18i, gen_pc, gen_pcaddu18i) +TRANS_64(pcaddu18i, gen_pc, gen_pcaddu18i) TRANS(andi, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_andi_tl) TRANS(ori, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_ori_tl) TRANS(xori, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_xori_tl) diff --git a/target/loongarch/insn_trans/trans_atomic.c.inc b/target/loongarch/insn_trans/trans_atomic.c.inc index 612709f2a7..c69f31bc78 100644 --- a/target/loongarch/insn_trans/trans_atomic.c.inc +++ b/target/loongarch/insn_trans/trans_atomic.c.inc @@ -70,41 +70,41 @@ static bool gen_am(DisasContext *ctx, arg_rrr *a, TRANS(ll_w, gen_ll, MO_TESL) TRANS(sc_
[PATCH v5 00/11] Add la32 & va32 support for loongarch64-softmmu
This patch series allow qemu-system-loongarch64 to emulate a LoongArch32 machine. A new CPU model (la132) is added for loongarch32, however due to lack of public documentation, details will need to be added in the future. Initial GDB support is added. At the same time, VA32(32-bit virtual address) support is introduced for LoongArch64. LA32 support is tested using a small supervisor program at https://github.com/jiegec/supervisor-la32. VA32 mode under LA64 is not tested yet. Changes since v4: - Code refactor, thanks Richard Henderson for great advice - Truncate higher 32 bits of PC in VA32 mode - Revert la132 initfn refactor Changes since v3: - Support VA32 mode for LoongArch64 - Check the current arch from CPUCFG.ARCH - Reject la64-only instructions in la32 mode Changes since v2: - Fix typo in previous commit - Fix VPPN width in TLBEHI/TLBREHI Changes since v1: - No longer create a separate qemu-system-loongarch32 executable, but allow user to run loongarch32 emulation using qemu-system-loongarch64 - Add loongarch32 cpu support for virt machine Full changes: Jiajie Chen (11): target/loongarch: Add function to check current arch target/loongarch: Add new object class for loongarch32 cpus target/loongarch: Add GDB support for loongarch32 mode target/loongarch: Support LoongArch32 TLB entry target/loongarch: Support LoongArch32 DMW target/loongarch: Support LoongArch32 VPPN target/loongarch: Add LA64 & VA32 to DisasContext target/loongarch: Reject la64-only instructions in la32 mode target/loongarch: Truncate high 32 bits of address in VA32 mode target/loongarch: Sign extend results in VA32 mode target/loongarch: Add loongarch32 cpu la132 configs/targets/loongarch64-softmmu.mak | 2 +- gdb-xml/loongarch-base32.xml | 45 hw/loongarch/virt.c | 5 - target/loongarch/cpu-csr.h| 22 ++-- target/loongarch/cpu.c| 74 +++-- target/loongarch/cpu.h| 33 ++ target/loongarch/gdbstub.c| 34 -- target/loongarch/insn_trans/trans_arith.c.inc | 32 +++--- .../loongarch/insn_trans/trans_atomic.c.inc | 81 +++--- target/loongarch/insn_trans/trans_bit.c.inc | 28 ++--- .../loongarch/insn_trans/trans_branch.c.inc | 11 +- target/loongarch/insn_trans/trans_extra.c.inc | 16 +-- .../loongarch/insn_trans/trans_fmemory.c.inc | 30 ++ target/loongarch/insn_trans/trans_fmov.c.inc | 4 +- target/loongarch/insn_trans/trans_lsx.c.inc | 38 ++- .../loongarch/insn_trans/trans_memory.c.inc | 102 -- target/loongarch/insn_trans/trans_shift.c.inc | 14 +-- target/loongarch/op_helper.c | 4 +- target/loongarch/tlb_helper.c | 66 +--- target/loongarch/translate.c | 43 target/loongarch/translate.h | 9 ++ 21 files changed, 445 insertions(+), 248 deletions(-) create mode 100644 gdb-xml/loongarch-base32.xml -- 2.41.0
[PATCH v5 02/11] target/loongarch: Add new object class for loongarch32 cpus
Add object class for future loongarch32 cpus. It is derived from the
loongarch64 object class.
Signed-off-by: Jiajie Chen
---
target/loongarch/cpu.c | 19 +++
target/loongarch/cpu.h | 1 +
2 files changed, 20 insertions(+)
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index ad93ecac92..c6b73444b4 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -732,12 +732,22 @@ static void loongarch_cpu_class_init(ObjectClass *c, void
*data)
#endif
}
+static void loongarch32_cpu_class_init(ObjectClass *c, void *data)
+{
+}
+
#define DEFINE_LOONGARCH_CPU_TYPE(model, initfn) \
{ \
.parent = TYPE_LOONGARCH_CPU, \
.instance_init = initfn, \
.name = LOONGARCH_CPU_TYPE_NAME(model), \
}
+#define DEFINE_LOONGARCH32_CPU_TYPE(model, initfn) \
+{ \
+.parent = TYPE_LOONGARCH32_CPU, \
+.instance_init = initfn, \
+.name = LOONGARCH_CPU_TYPE_NAME(model), \
+}
static const TypeInfo loongarch_cpu_type_infos[] = {
{
@@ -750,6 +760,15 @@ static const TypeInfo loongarch_cpu_type_infos[] = {
.class_size = sizeof(LoongArchCPUClass),
.class_init = loongarch_cpu_class_init,
},
+{
+.name = TYPE_LOONGARCH32_CPU,
+.parent = TYPE_LOONGARCH_CPU,
+.instance_size = sizeof(LoongArchCPU),
+
+.abstract = true,
+.class_size = sizeof(LoongArchCPUClass),
+.class_init = loongarch32_cpu_class_init,
+},
DEFINE_LOONGARCH_CPU_TYPE("la464", loongarch_la464_initfn),
};
diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
index 5a71d64a04..2af4c414b0 100644
--- a/target/loongarch/cpu.h
+++ b/target/loongarch/cpu.h
@@ -382,6 +382,7 @@ struct ArchCPU {
};
#define TYPE_LOONGARCH_CPU "loongarch-cpu"
+#define TYPE_LOONGARCH32_CPU "loongarch32-cpu"
OBJECT_DECLARE_CPU_TYPE(LoongArchCPU, LoongArchCPUClass,
LOONGARCH_CPU)
--
2.41.0
[PATCH v5 03/11] target/loongarch: Add GDB support for loongarch32 mode
GPRs and PC are 32-bit wide in loongarch32 mode.
Signed-off-by: Jiajie Chen
Reviewed-by: Richard Henderson
---
configs/targets/loongarch64-softmmu.mak | 2 +-
gdb-xml/loongarch-base32.xml| 45 +
target/loongarch/cpu.c | 10 +-
target/loongarch/gdbstub.c | 32 ++
4 files changed, 80 insertions(+), 9 deletions(-)
create mode 100644 gdb-xml/loongarch-base32.xml
diff --git a/configs/targets/loongarch64-softmmu.mak
b/configs/targets/loongarch64-softmmu.mak
index 9abc99056f..f23780fdd8 100644
--- a/configs/targets/loongarch64-softmmu.mak
+++ b/configs/targets/loongarch64-softmmu.mak
@@ -1,5 +1,5 @@
TARGET_ARCH=loongarch64
TARGET_BASE_ARCH=loongarch
TARGET_SUPPORTS_MTTCG=y
-TARGET_XML_FILES= gdb-xml/loongarch-base64.xml gdb-xml/loongarch-fpu.xml
+TARGET_XML_FILES= gdb-xml/loongarch-base32.xml gdb-xml/loongarch-base64.xml
gdb-xml/loongarch-fpu.xml
TARGET_NEED_FDT=y
diff --git a/gdb-xml/loongarch-base32.xml b/gdb-xml/loongarch-base32.xml
new file mode 100644
index 00..af47bbd3da
--- /dev/null
+++ b/gdb-xml/loongarch-base32.xml
@@ -0,0 +1,45 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index c6b73444b4..30dd70571a 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -694,7 +694,13 @@ static const struct SysemuCPUOps loongarch_sysemu_ops = {
static gchar *loongarch_gdb_arch_name(CPUState *cs)
{
-return g_strdup("loongarch64");
+LoongArchCPU *cpu = LOONGARCH_CPU(cs);
+CPULoongArchState *env = &cpu->env;
+if (is_la64(env)) {
+return g_strdup("loongarch64");
+} else {
+return g_strdup("loongarch32");
+}
}
static void loongarch_cpu_class_init(ObjectClass *c, void *data)
@@ -734,6 +740,8 @@ static void loongarch_cpu_class_init(ObjectClass *c, void
*data)
static void loongarch32_cpu_class_init(ObjectClass *c, void *data)
{
+CPUClass *cc = CPU_CLASS(c);
+cc->gdb_core_xml_file = "loongarch-base32.xml";
}
#define DEFINE_LOONGARCH_CPU_TYPE(model, initfn) \
diff --git a/target/loongarch/gdbstub.c b/target/loongarch/gdbstub.c
index 0752fff924..a462e25737 100644
--- a/target/loongarch/gdbstub.c
+++ b/target/loongarch/gdbstub.c
@@ -34,16 +34,25 @@ int loongarch_cpu_gdb_read_register(CPUState *cs,
GByteArray *mem_buf, int n)
{
LoongArchCPU *cpu = LOONGARCH_CPU(cs);
CPULoongArchState *env = &cpu->env;
+uint64_t val;
if (0 <= n && n < 32) {
-return gdb_get_regl(mem_buf, env->gpr[n]);
+val = env->gpr[n];
} else if (n == 32) {
/* orig_a0 */
-return gdb_get_regl(mem_buf, 0);
+val = 0;
} else if (n == 33) {
-return gdb_get_regl(mem_buf, env->pc);
+val = env->pc;
} else if (n == 34) {
-return gdb_get_regl(mem_buf, env->CSR_BADV);
+val = env->CSR_BADV;
+}
+
+if (0 <= n && n <= 34) {
+if (is_la64(env)) {
+return gdb_get_reg64(mem_buf, val);
+} else {
+return gdb_get_reg32(mem_buf, val);
+}
}
return 0;
}
@@ -52,15 +61,24 @@ int loongarch_cpu_gdb_write_register(CPUState *cs, uint8_t
*mem_buf, int n)
{
LoongArchCPU *cpu = LOONGARCH_CPU(cs);
CPULoongArchState *env = &cpu->env;
-target_ulong tmp = ldtul_p(mem_buf);
+target_ulong tmp;
+int read_length;
int length = 0;
+if (is_la64(env)) {
+tmp = ldq_p(mem_buf);
+read_length = 8;
+} else {
+tmp = ldl_p(mem_buf);
+read_length = 4;
+}
+
if (0 <= n && n < 32) {
env->gpr[n] = tmp;
-length = sizeof(target_ulong);
+length = read_length;
} else if (n == 33) {
env->pc = tmp;
-length = sizeof(target_ulong);
+length = read_length;
}
return length;
}
--
2.41.0
[PATCH v5 05/11] target/loongarch: Support LoongArch32 DMW
LA32 uses a different encoding for CSR.DMW and a new direct mapping
mechanism.
Signed-off-by: Jiajie Chen
Reviewed-by: Richard Henderson
---
target/loongarch/cpu-csr.h| 7 +++
target/loongarch/tlb_helper.c | 26 +++---
2 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/target/loongarch/cpu-csr.h b/target/loongarch/cpu-csr.h
index 48ed2e0632..b93f99a9ef 100644
--- a/target/loongarch/cpu-csr.h
+++ b/target/loongarch/cpu-csr.h
@@ -188,10 +188,9 @@ FIELD(CSR_DMW, PLV1, 1, 1)
FIELD(CSR_DMW, PLV2, 2, 1)
FIELD(CSR_DMW, PLV3, 3, 1)
FIELD(CSR_DMW, MAT, 4, 2)
-FIELD(CSR_DMW, VSEG, 60, 4)
-
-#define dmw_va2pa(va) \
-(va & MAKE_64BIT_MASK(0, TARGET_VIRT_ADDR_SPACE_BITS))
+FIELD(CSR_DMW_32, PSEG, 25, 3)
+FIELD(CSR_DMW_32, VSEG, 29, 3)
+FIELD(CSR_DMW_64, VSEG, 60, 4)
/* Debug CSRs */
#define LOONGARCH_CSR_DBG0x500 /* debug config */
diff --git a/target/loongarch/tlb_helper.c b/target/loongarch/tlb_helper.c
index cef10e2257..1f8e7911c7 100644
--- a/target/loongarch/tlb_helper.c
+++ b/target/loongarch/tlb_helper.c
@@ -173,6 +173,18 @@ static int loongarch_map_address(CPULoongArchState *env,
hwaddr *physical,
return TLBRET_NOMATCH;
}
+static hwaddr dmw_va2pa(CPULoongArchState *env, target_ulong va,
+target_ulong dmw)
+{
+if (is_la64(env)) {
+return va & TARGET_VIRT_MASK;
+} else {
+uint32_t pseg = FIELD_EX32(dmw, CSR_DMW_32, PSEG);
+return (va & MAKE_64BIT_MASK(0, R_CSR_DMW_32_VSEG_SHIFT)) | \
+(pseg << R_CSR_DMW_32_VSEG_SHIFT);
+}
+}
+
static int get_physical_address(CPULoongArchState *env, hwaddr *physical,
int *prot, target_ulong address,
MMUAccessType access_type, int mmu_idx)
@@ -192,12 +204,20 @@ static int get_physical_address(CPULoongArchState *env,
hwaddr *physical,
}
plv = kernel_mode | (user_mode << R_CSR_DMW_PLV3_SHIFT);
-base_v = address >> R_CSR_DMW_VSEG_SHIFT;
+if (is_la64(env)) {
+base_v = address >> R_CSR_DMW_64_VSEG_SHIFT;
+} else {
+base_v = address >> R_CSR_DMW_32_VSEG_SHIFT;
+}
/* Check direct map window */
for (int i = 0; i < 4; i++) {
-base_c = FIELD_EX64(env->CSR_DMW[i], CSR_DMW, VSEG);
+if (is_la64(env)) {
+base_c = FIELD_EX64(env->CSR_DMW[i], CSR_DMW_64, VSEG);
+} else {
+base_c = FIELD_EX64(env->CSR_DMW[i], CSR_DMW_32, VSEG);
+}
if ((plv & env->CSR_DMW[i]) && (base_c == base_v)) {
-*physical = dmw_va2pa(address);
+*physical = dmw_va2pa(env, address, env->CSR_DMW[i]);
*prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
return TLBRET_MATCH;
}
--
2.41.0
[PATCH v5 06/11] target/loongarch: Support LoongArch32 VPPN
VPPN of TLBEHI/TLBREHI is limited to 19 bits in LA32.
Signed-off-by: Jiajie Chen
Reviewed-by: Richard Henderson
---
target/loongarch/cpu-csr.h| 6 --
target/loongarch/tlb_helper.c | 23 ++-
2 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/target/loongarch/cpu-csr.h b/target/loongarch/cpu-csr.h
index b93f99a9ef..c59d7a9fcb 100644
--- a/target/loongarch/cpu-csr.h
+++ b/target/loongarch/cpu-csr.h
@@ -57,7 +57,8 @@ FIELD(CSR_TLBIDX, PS, 24, 6)
FIELD(CSR_TLBIDX, NE, 31, 1)
#define LOONGARCH_CSR_TLBEHI 0x11 /* TLB EntryHi */
-FIELD(CSR_TLBEHI, VPPN, 13, 35)
+FIELD(CSR_TLBEHI_32, VPPN, 13, 19)
+FIELD(CSR_TLBEHI_64, VPPN, 13, 35)
#define LOONGARCH_CSR_TLBELO00x12 /* TLB EntryLo0 */
#define LOONGARCH_CSR_TLBELO10x13 /* TLB EntryLo1 */
@@ -164,7 +165,8 @@ FIELD(CSR_TLBRERA, PC, 2, 62)
#define LOONGARCH_CSR_TLBRELO1 0x8d /* TLB refill entrylo1 */
#define LOONGARCH_CSR_TLBREHI0x8e /* TLB refill entryhi */
FIELD(CSR_TLBREHI, PS, 0, 6)
-FIELD(CSR_TLBREHI, VPPN, 13, 35)
+FIELD(CSR_TLBREHI_32, VPPN, 13, 19)
+FIELD(CSR_TLBREHI_64, VPPN, 13, 35)
#define LOONGARCH_CSR_TLBRPRMD 0x8f /* TLB refill mode info */
FIELD(CSR_TLBRPRMD, PPLV, 0, 2)
FIELD(CSR_TLBRPRMD, PIE, 2, 1)
diff --git a/target/loongarch/tlb_helper.c b/target/loongarch/tlb_helper.c
index 1f8e7911c7..c8b8b0497f 100644
--- a/target/loongarch/tlb_helper.c
+++ b/target/loongarch/tlb_helper.c
@@ -300,8 +300,13 @@ static void raise_mmu_exception(CPULoongArchState *env,
target_ulong address,
if (tlb_error == TLBRET_NOMATCH) {
env->CSR_TLBRBADV = address;
-env->CSR_TLBREHI = FIELD_DP64(env->CSR_TLBREHI, CSR_TLBREHI, VPPN,
- extract64(address, 13, 35));
+if (is_la64(env)) {
+env->CSR_TLBREHI = FIELD_DP64(env->CSR_TLBREHI, CSR_TLBREHI_64,
+VPPN, extract64(address, 13, 35));
+} else {
+env->CSR_TLBREHI = FIELD_DP64(env->CSR_TLBREHI, CSR_TLBREHI_32,
+VPPN, extract64(address, 13, 19));
+}
} else {
if (!FIELD_EX64(env->CSR_DBG, CSR_DBG, DST)) {
env->CSR_BADV = address;
@@ -366,12 +371,20 @@ static void fill_tlb_entry(CPULoongArchState *env, int
index)
if (FIELD_EX64(env->CSR_TLBRERA, CSR_TLBRERA, ISTLBR)) {
csr_ps = FIELD_EX64(env->CSR_TLBREHI, CSR_TLBREHI, PS);
-csr_vppn = FIELD_EX64(env->CSR_TLBREHI, CSR_TLBREHI, VPPN);
+if (is_la64(env)) {
+csr_vppn = FIELD_EX64(env->CSR_TLBREHI, CSR_TLBREHI_64, VPPN);
+} else {
+csr_vppn = FIELD_EX64(env->CSR_TLBREHI, CSR_TLBREHI_32, VPPN);
+}
lo0 = env->CSR_TLBRELO0;
lo1 = env->CSR_TLBRELO1;
} else {
csr_ps = FIELD_EX64(env->CSR_TLBIDX, CSR_TLBIDX, PS);
-csr_vppn = FIELD_EX64(env->CSR_TLBEHI, CSR_TLBEHI, VPPN);
+if (is_la64(env)) {
+csr_vppn = FIELD_EX64(env->CSR_TLBEHI, CSR_TLBEHI_64, VPPN);
+} else {
+csr_vppn = FIELD_EX64(env->CSR_TLBEHI, CSR_TLBEHI_32, VPPN);
+}
lo0 = env->CSR_TLBELO0;
lo1 = env->CSR_TLBELO1;
}
@@ -491,7 +504,7 @@ void helper_tlbfill(CPULoongArchState *env)
if (pagesize == stlb_ps) {
/* Only write into STLB bits [47:13] */
-address = entryhi & ~MAKE_64BIT_MASK(0, R_CSR_TLBEHI_VPPN_SHIFT);
+address = entryhi & ~MAKE_64BIT_MASK(0, R_CSR_TLBEHI_64_VPPN_SHIFT);
/* Choose one set ramdomly */
set = get_random_tlb(0, 7);
--
2.41.0
[PATCH v5 11/11] target/loongarch: Add loongarch32 cpu la132
Add la132 as a loongarch32 cpu type and allow virt machine to be used
with la132 instead of la464.
Due to lack of public documentation of la132, it is currently a
synthetic loongarch32 cpu model. Details need to be added in the future.
Signed-off-by: Jiajie Chen
---
hw/loongarch/virt.c| 5 -
target/loongarch/cpu.c | 29 +
2 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index e19b042ce8..af15bf5aaa 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -798,11 +798,6 @@ static void loongarch_init(MachineState *machine)
cpu_model = LOONGARCH_CPU_TYPE_NAME("la464");
}
-if (!strstr(cpu_model, "la464")) {
-error_report("LoongArch/TCG needs cpu type la464");
-exit(1);
-}
-
if (ram_size < 1 * GiB) {
error_report("ram_size must be greater than 1G.");
exit(1);
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index bd980790f2..dd1cd7d7d2 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -439,6 +439,34 @@ static void loongarch_la464_initfn(Object *obj)
env->CSR_ASID = FIELD_DP64(0, CSR_ASID, ASIDBITS, 0xa);
}
+static void loongarch_la132_initfn(Object *obj)
+{
+LoongArchCPU *cpu = LOONGARCH_CPU(obj);
+CPULoongArchState *env = &cpu->env;
+
+int i;
+
+for (i = 0; i < 21; i++) {
+env->cpucfg[i] = 0x0;
+}
+
+cpu->dtb_compatible = "loongarch,Loongson-1C103";
+
+uint32_t data = 0;
+data = FIELD_DP32(data, CPUCFG1, ARCH, 1); /* LA32 */
+data = FIELD_DP32(data, CPUCFG1, PGMMU, 1);
+data = FIELD_DP32(data, CPUCFG1, IOCSR, 1);
+data = FIELD_DP32(data, CPUCFG1, PALEN, 0x1f); /* 32 bits */
+data = FIELD_DP32(data, CPUCFG1, VALEN, 0x1f); /* 32 bits */
+data = FIELD_DP32(data, CPUCFG1, UAL, 1);
+data = FIELD_DP32(data, CPUCFG1, RI, 0);
+data = FIELD_DP32(data, CPUCFG1, EP, 0);
+data = FIELD_DP32(data, CPUCFG1, RPLV, 0);
+data = FIELD_DP32(data, CPUCFG1, HP, 1);
+data = FIELD_DP32(data, CPUCFG1, IOCSR_BRD, 1);
+env->cpucfg[1] = data;
+}
+
static void loongarch_cpu_list_entry(gpointer data, gpointer user_data)
{
const char *typename = object_class_get_name(OBJECT_CLASS(data));
@@ -778,6 +806,7 @@ static const TypeInfo loongarch_cpu_type_infos[] = {
.class_init = loongarch32_cpu_class_init,
},
DEFINE_LOONGARCH_CPU_TYPE("la464", loongarch_la464_initfn),
+DEFINE_LOONGARCH32_CPU_TYPE("la132", loongarch_la132_initfn),
};
DEFINE_TYPES(loongarch_cpu_type_infos)
--
2.41.0
[PATCH v5 07/11] target/loongarch: Add LA64 & VA32 to DisasContext
Add LA64 and VA32(32-bit Virtual Address) to DisasContext to allow the
translator to reject doubleword instructions in LA32 mode for example.
Signed-off-by: Jiajie Chen
Reviewed-by: Richard Henderson
---
target/loongarch/cpu.h | 13 +
target/loongarch/translate.c | 3 +++
target/loongarch/translate.h | 2 ++
3 files changed, 18 insertions(+)
diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
index 2af4c414b0..0e02257f91 100644
--- a/target/loongarch/cpu.h
+++ b/target/loongarch/cpu.h
@@ -431,6 +431,17 @@ static inline bool is_la64(CPULoongArchState *env)
return FIELD_EX32(env->cpucfg[1], CPUCFG1, ARCH) == CPUCFG1_ARCH_LA64;
}
+static inline bool is_va32(CPULoongArchState *env)
+{
+/* VA32 if !LA64 or VA32L[1-3] */
+bool va32 = !is_la64(env);
+uint64_t plv = FIELD_EX64(env->CSR_CRMD, CSR_CRMD, PLV);
+if (plv >= 1 && (FIELD_EX64(env->CSR_MISC, CSR_MISC, VA32) & (1 << plv))) {
+va32 = true;
+}
+return va32;
+}
+
/*
* LoongArch CPUs hardware flags.
*/
@@ -438,6 +449,7 @@ static inline bool is_la64(CPULoongArchState *env)
#define HW_FLAGS_CRMD_PGR_CSR_CRMD_PG_MASK /* 0x10 */
#define HW_FLAGS_EUEN_FPE 0x04
#define HW_FLAGS_EUEN_SXE 0x08
+#define HW_FLAGS_VA32 0x20
static inline void cpu_get_tb_cpu_state(CPULoongArchState *env, vaddr *pc,
uint64_t *cs_base, uint32_t *flags)
@@ -447,6 +459,7 @@ static inline void cpu_get_tb_cpu_state(CPULoongArchState
*env, vaddr *pc,
*flags = env->CSR_CRMD & (R_CSR_CRMD_PLV_MASK | R_CSR_CRMD_PG_MASK);
*flags |= FIELD_EX64(env->CSR_EUEN, CSR_EUEN, FPE) * HW_FLAGS_EUEN_FPE;
*flags |= FIELD_EX64(env->CSR_EUEN, CSR_EUEN, SXE) * HW_FLAGS_EUEN_SXE;
+*flags |= is_va32(env) * HW_FLAGS_VA32;
}
void loongarch_cpu_list(void);
diff --git a/target/loongarch/translate.c b/target/loongarch/translate.c
index 3146a2d4ac..ac847745df 100644
--- a/target/loongarch/translate.c
+++ b/target/loongarch/translate.c
@@ -119,6 +119,9 @@ static void
loongarch_tr_init_disas_context(DisasContextBase *dcbase,
ctx->vl = LSX_LEN;
}
+ctx->la64 = is_la64(env);
+ctx->va32 = (ctx->base.tb->flags & HW_FLAGS_VA32) != 0;
+
ctx->zero = tcg_constant_tl(0);
}
diff --git a/target/loongarch/translate.h b/target/loongarch/translate.h
index 7f60090580..b6fa5df82d 100644
--- a/target/loongarch/translate.h
+++ b/target/loongarch/translate.h
@@ -33,6 +33,8 @@ typedef struct DisasContext {
uint16_t plv;
int vl; /* Vector length */
TCGv zero;
+bool la64; /* LoongArch64 mode */
+bool va32; /* 32-bit virtual address */
} DisasContext;
void generate_exception(DisasContext *ctx, int excp);
--
2.41.0
[PATCH v5 04/11] target/loongarch: Support LoongArch32 TLB entry
The TLB entry of LA32 lacks NR, NX and RPLV and they are hardwired to
zero in LoongArch32.
Signed-off-by: Jiajie Chen
Reviewed-by: Richard Henderson
---
target/loongarch/cpu-csr.h| 9 +
target/loongarch/tlb_helper.c | 17 -
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/target/loongarch/cpu-csr.h b/target/loongarch/cpu-csr.h
index f8f24032cb..48ed2e0632 100644
--- a/target/loongarch/cpu-csr.h
+++ b/target/loongarch/cpu-csr.h
@@ -66,10 +66,11 @@ FIELD(TLBENTRY, D, 1, 1)
FIELD(TLBENTRY, PLV, 2, 2)
FIELD(TLBENTRY, MAT, 4, 2)
FIELD(TLBENTRY, G, 6, 1)
-FIELD(TLBENTRY, PPN, 12, 36)
-FIELD(TLBENTRY, NR, 61, 1)
-FIELD(TLBENTRY, NX, 62, 1)
-FIELD(TLBENTRY, RPLV, 63, 1)
+FIELD(TLBENTRY_32, PPN, 8, 24)
+FIELD(TLBENTRY_64, PPN, 12, 36)
+FIELD(TLBENTRY_64, NR, 61, 1)
+FIELD(TLBENTRY_64, NX, 62, 1)
+FIELD(TLBENTRY_64, RPLV, 63, 1)
#define LOONGARCH_CSR_ASID 0x18 /* Address space identifier */
FIELD(CSR_ASID, ASID, 0, 10)
diff --git a/target/loongarch/tlb_helper.c b/target/loongarch/tlb_helper.c
index 6e00190547..cef10e2257 100644
--- a/target/loongarch/tlb_helper.c
+++ b/target/loongarch/tlb_helper.c
@@ -48,10 +48,17 @@ static int loongarch_map_tlb_entry(CPULoongArchState *env,
hwaddr *physical,
tlb_v = FIELD_EX64(tlb_entry, TLBENTRY, V);
tlb_d = FIELD_EX64(tlb_entry, TLBENTRY, D);
tlb_plv = FIELD_EX64(tlb_entry, TLBENTRY, PLV);
-tlb_ppn = FIELD_EX64(tlb_entry, TLBENTRY, PPN);
-tlb_nx = FIELD_EX64(tlb_entry, TLBENTRY, NX);
-tlb_nr = FIELD_EX64(tlb_entry, TLBENTRY, NR);
-tlb_rplv = FIELD_EX64(tlb_entry, TLBENTRY, RPLV);
+if (is_la64(env)) {
+tlb_ppn = FIELD_EX64(tlb_entry, TLBENTRY_64, PPN);
+tlb_nx = FIELD_EX64(tlb_entry, TLBENTRY_64, NX);
+tlb_nr = FIELD_EX64(tlb_entry, TLBENTRY_64, NR);
+tlb_rplv = FIELD_EX64(tlb_entry, TLBENTRY_64, RPLV);
+} else {
+tlb_ppn = FIELD_EX64(tlb_entry, TLBENTRY_32, PPN);
+tlb_nx = 0;
+tlb_nr = 0;
+tlb_rplv = 0;
+}
/* Check access rights */
if (!tlb_v) {
@@ -79,7 +86,7 @@ static int loongarch_map_tlb_entry(CPULoongArchState *env,
hwaddr *physical,
* tlb_entry contains ppn[47:12] while 16KiB ppn is [47:15]
* need adjust.
*/
-*physical = (tlb_ppn << R_TLBENTRY_PPN_SHIFT) |
+*physical = (tlb_ppn << R_TLBENTRY_64_PPN_SHIFT) |
(address & MAKE_64BIT_MASK(0, tlb_ps));
*prot = PAGE_READ;
if (tlb_d) {
--
2.41.0
[PATCH v5 10/11] target/loongarch: Sign extend results in VA32 mode
In VA32 mode, BL, JIRL and PC* instructions should sign-extend the low
32 bit result to 64 bits.
Signed-off-by: Jiajie Chen
---
target/loongarch/insn_trans/trans_arith.c.inc | 2 +-
target/loongarch/insn_trans/trans_branch.c.inc | 4 ++--
target/loongarch/translate.c | 8
3 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/target/loongarch/insn_trans/trans_arith.c.inc
b/target/loongarch/insn_trans/trans_arith.c.inc
index 4c21d8b037..e3b7979e15 100644
--- a/target/loongarch/insn_trans/trans_arith.c.inc
+++ b/target/loongarch/insn_trans/trans_arith.c.inc
@@ -72,7 +72,7 @@ static bool gen_pc(DisasContext *ctx, arg_r_i *a,
target_ulong (*func)(target_ulong, int))
{
TCGv dest = gpr_dst(ctx, a->rd, EXT_NONE);
-target_ulong addr = func(ctx->base.pc_next, a->imm);
+target_ulong addr = make_address_pc(ctx, func(ctx->base.pc_next, a->imm));
tcg_gen_movi_tl(dest, addr);
gen_set_gpr(a->rd, dest, EXT_NONE);
diff --git a/target/loongarch/insn_trans/trans_branch.c.inc
b/target/loongarch/insn_trans/trans_branch.c.inc
index b63058235d..cf035e44ff 100644
--- a/target/loongarch/insn_trans/trans_branch.c.inc
+++ b/target/loongarch/insn_trans/trans_branch.c.inc
@@ -12,7 +12,7 @@ static bool trans_b(DisasContext *ctx, arg_b *a)
static bool trans_bl(DisasContext *ctx, arg_bl *a)
{
-tcg_gen_movi_tl(cpu_gpr[1], ctx->base.pc_next + 4);
+tcg_gen_movi_tl(cpu_gpr[1], make_address_pc(ctx, ctx->base.pc_next + 4));
gen_goto_tb(ctx, 0, ctx->base.pc_next + a->offs);
ctx->base.is_jmp = DISAS_NORETURN;
return true;
@@ -25,7 +25,7 @@ static bool trans_jirl(DisasContext *ctx, arg_jirl *a)
TCGv addr = make_address_i(ctx, src1, a->imm);
tcg_gen_mov_tl(cpu_pc, addr);
-tcg_gen_movi_tl(dest, ctx->base.pc_next + 4);
+tcg_gen_movi_tl(dest, make_address_pc(ctx, ctx->base.pc_next + 4));
gen_set_gpr(a->rd, dest, EXT_NONE);
tcg_gen_lookup_and_goto_ptr();
ctx->base.is_jmp = DISAS_NORETURN;
diff --git a/target/loongarch/translate.c b/target/loongarch/translate.c
index 689da19ed0..de7c1c5d1f 100644
--- a/target/loongarch/translate.c
+++ b/target/loongarch/translate.c
@@ -236,6 +236,14 @@ static TCGv make_address_i(DisasContext *ctx, TCGv base,
target_long ofs)
return make_address_x(ctx, base, addend);
}
+static uint64_t make_address_pc(DisasContext *ctx, uint64_t addr)
+{
+if (ctx->va32) {
+addr = (int32_t)addr;
+}
+return addr;
+}
+
#include "decode-insns.c.inc"
#include "insn_trans/trans_arith.c.inc"
#include "insn_trans/trans_shift.c.inc"
--
2.41.0
[PATCH v5 09/11] target/loongarch: Truncate high 32 bits of address in VA32 mode
When running in VA32 mode(!LA64 or VA32L[1-3] matching PLV), virtual
address is truncated to 32 bits before address mapping.
Signed-off-by: Jiajie Chen
Co-authored-by: Richard Henderson
---
target/loongarch/cpu.c| 16
target/loongarch/cpu.h| 9 +
target/loongarch/gdbstub.c| 2 +-
.../loongarch/insn_trans/trans_atomic.c.inc | 5 ++-
.../loongarch/insn_trans/trans_branch.c.inc | 3 +-
.../loongarch/insn_trans/trans_fmemory.c.inc | 30 ---
target/loongarch/insn_trans/trans_lsx.c.inc | 38 +--
.../loongarch/insn_trans/trans_memory.c.inc | 34 +
target/loongarch/op_helper.c | 4 +-
target/loongarch/translate.c | 32
10 files changed, 85 insertions(+), 88 deletions(-)
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index 30dd70571a..bd980790f2 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -81,7 +81,7 @@ static void loongarch_cpu_set_pc(CPUState *cs, vaddr value)
LoongArchCPU *cpu = LOONGARCH_CPU(cs);
CPULoongArchState *env = &cpu->env;
-env->pc = value;
+set_pc(env, value);
}
static vaddr loongarch_cpu_get_pc(CPUState *cs)
@@ -168,7 +168,7 @@ static void loongarch_cpu_do_interrupt(CPUState *cs)
set_DERA:
env->CSR_DERA = env->pc;
env->CSR_DBG = FIELD_DP64(env->CSR_DBG, CSR_DBG, DST, 1);
-env->pc = env->CSR_EENTRY + 0x480;
+set_pc(env, env->CSR_EENTRY + 0x480);
break;
case EXCCODE_INT:
if (FIELD_EX64(env->CSR_DBG, CSR_DBG, DST)) {
@@ -249,7 +249,8 @@ static void loongarch_cpu_do_interrupt(CPUState *cs)
/* Find the highest-priority interrupt. */
vector = 31 - clz32(pending);
-env->pc = env->CSR_EENTRY + (EXCCODE_EXTERNAL_INT + vector) * vec_size;
+set_pc(env, env->CSR_EENTRY + \
+ (EXCCODE_EXTERNAL_INT + vector) * vec_size);
qemu_log_mask(CPU_LOG_INT,
"%s: PC " TARGET_FMT_lx " ERA " TARGET_FMT_lx
" cause %d\n" "A " TARGET_FMT_lx " D "
@@ -260,10 +261,9 @@ static void loongarch_cpu_do_interrupt(CPUState *cs)
env->CSR_ECFG, env->CSR_ESTAT);
} else {
if (tlbfill) {
-env->pc = env->CSR_TLBRENTRY;
+set_pc(env, env->CSR_TLBRENTRY);
} else {
-env->pc = env->CSR_EENTRY;
-env->pc += EXCODE_MCODE(cause) * vec_size;
+set_pc(env, env->CSR_EENTRY + EXCODE_MCODE(cause) * vec_size);
}
qemu_log_mask(CPU_LOG_INT,
"%s: PC " TARGET_FMT_lx " ERA " TARGET_FMT_lx
@@ -324,7 +324,7 @@ static void loongarch_cpu_synchronize_from_tb(CPUState *cs,
CPULoongArchState *env = &cpu->env;
tcg_debug_assert(!(cs->tcg_cflags & CF_PCREL));
-env->pc = tb->pc;
+set_pc(env, tb->pc);
}
static void loongarch_restore_state_to_opc(CPUState *cs,
@@ -334,7 +334,7 @@ static void loongarch_restore_state_to_opc(CPUState *cs,
LoongArchCPU *cpu = LOONGARCH_CPU(cs);
CPULoongArchState *env = &cpu->env;
-env->pc = data[0];
+set_pc(env, data[0]);
}
#endif /* CONFIG_TCG */
diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
index 0e02257f91..9f550793ca 100644
--- a/target/loongarch/cpu.h
+++ b/target/loongarch/cpu.h
@@ -442,6 +442,15 @@ static inline bool is_va32(CPULoongArchState *env)
return va32;
}
+static inline void set_pc(CPULoongArchState *env, uint64_t value)
+{
+if (is_va32(env)) {
+env->pc = (uint32_t)value;
+} else {
+env->pc = value;
+}
+}
+
/*
* LoongArch CPUs hardware flags.
*/
diff --git a/target/loongarch/gdbstub.c b/target/loongarch/gdbstub.c
index a462e25737..e20b20f99b 100644
--- a/target/loongarch/gdbstub.c
+++ b/target/loongarch/gdbstub.c
@@ -77,7 +77,7 @@ int loongarch_cpu_gdb_write_register(CPUState *cs, uint8_t
*mem_buf, int n)
env->gpr[n] = tmp;
length = read_length;
} else if (n == 33) {
-env->pc = tmp;
+set_pc(env, tmp);
length = read_length;
}
return length;
diff --git a/target/loongarch/insn_trans/trans_atomic.c.inc
b/target/loongarch/insn_trans/trans_atomic.c.inc
index c69f31bc78..d90312729b 100644
--- a/target/loongarch/insn_trans/trans_atomic.c.inc
+++ b/target/loongarch/insn_trans/trans_atomic.c.inc
@@ -7,9 +7,8 @@ static bool gen_ll(DisasContext *ctx, arg_rr_i *a, MemOp mop)
{
TCGv dest = gpr_dst(ctx, a->rd, EXT_NONE);
TCGv src1 = gpr_src(ctx, a->rj, EXT_NONE);
-TCGv t0 = tcg_temp_new();
+TCGv t0 = make_address_i(ctx, src1, a->imm);
-tcg_gen_addi_tl(t0, src1, a->imm);
tcg_gen_qemu_ld_i64(dest, t0, ctx->mem_idx, mop);
tcg_gen_st_tl(t0, cpu_env, offsetof(CPULoongArchState, lladdr));
tcg_gen_st_tl(dest, cpu_env, offsetof(CPULoongArchState, llval));
@@ -62,6 +61,8 @@ static bool gen_am(
Re: [RFC v4 10/11] tests/tcg/multiarch: Add nativecall.c test
Yeqi Fu writes:
> Introduce a new test for native calls to ensure their functionality.
> The process involves cross-compiling the test cases, building them
> as dynamically linked binaries, and running these binaries which
> necessitates the addition of the appropriate interpreter prefix.
>
> Signed-off-by: Yeqi Fu
> ---
> tests/tcg/multiarch/Makefile.target | 17 +
> tests/tcg/multiarch/native/nativecall.c | 98 +
> 2 files changed, 115 insertions(+)
> create mode 100644 tests/tcg/multiarch/native/nativecall.c
>
> diff --git a/tests/tcg/multiarch/Makefile.target
> b/tests/tcg/multiarch/Makefile.target
> index 43bddeaf21..5231df34ba 100644
> --- a/tests/tcg/multiarch/Makefile.target
> +++ b/tests/tcg/multiarch/Makefile.target
> @@ -138,5 +138,22 @@ run-plugin-semiconsole-with-%:
> TESTS += semihosting semiconsole
> endif
>
> +nativecall: native/nativecall.c
> + $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(filter-out
> -static,$(LDFLAGS))
> +
> +ifneq ($(LD_PREFIX),)
> +ifneq ($(wildcard $(LIBNATIVE)),)
> +run-nativecall: nativecall
> + $(call run-test,$<, $(QEMU) -L $(LD_PREFIX) --native-bypass
> $(LIBNATIVE) $<, "nativecall")
> +else
> +run-nativecall: nativecall
> + $(call skip-test, $<, "no native library found")
> +endif
> +else
> +run-nativecall: nativecall
> + $(call skip-test, $<, "no elf interpreter prefix found")
> +endif
> +EXTRA_RUNS += run-nativecall
> +
> # Update TESTS
> TESTS += $(MULTIARCH_TESTS)
> diff --git a/tests/tcg/multiarch/native/nativecall.c
> b/tests/tcg/multiarch/native/nativecall.c
> new file mode 100644
> index 00..d3f6f49ed0
> --- /dev/null
> +++ b/tests/tcg/multiarch/native/nativecall.c
> @@ -0,0 +1,98 @@
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +void compare_memory(const void *a, const void *b, size_t n)
> +{
> +const unsigned char *p1 = a;
> +const unsigned char *p2 = b;
> +for (size_t i = 0; i < n; i++) {
> +assert(p1[i] == p2[i]);
> +}
> +}
> +
> +void test_memcpy()
> +{
> +char src[] = "Hello, world!";
> +char dest[20];
> +memcpy(dest, src, 13);
> +compare_memory(dest, src, 13);
> +}
As discussed earlier expanding the range of sizes involved will help get
more coverage especially as routines are usually optimised to handle
various alignments and block sizes.
You can also use a --enable-gcov build and check how much coverage of
your new code is hit by the test cases.
> +
> +void test_strncpy()
> +{
> +char src[] = "Hello, world!";
> +char dest[20];
> +strncpy(dest, src, 13);
> +compare_memory(dest, src, 13);
> +}
> +
> +void test_strcpy()
> +{
> +char src[] = "Hello, world!";
> +char dest[20];
> +strcpy(dest, src);
> +compare_memory(dest, src, 13);
> +}
> +
> +void test_strcat()
> +{
> +char src[20] = "Hello, ";
> +char dst[] = "world!";
> +char str[] = "Hello, world!";
> +strcat(src, dest);
copy and paste fail here (dst/dest) means it doesn't compile.
It's always good practice to make sure your tree passes a make check (or
at least check-tcg in the user-mode case) to ensure no silly bugs are
present.
You can also create a gitlab account and take advantage of the CI. See
this run for example:
https://gitlab.com/stsquad/qemu/-/pipelines/959899240/failures
> +compare_memory(src, str, 13);
> +}
> +
> +void test_memcmp()
> +{
> +char str1[] = "abc";
> +char str2[] = "abc";
> +char str3[] = "def";
> +assert(memcmp(str1, str2, 3) == 0);
> +assert(memcmp(str1, str3, 3) != 0);
> +}
> +
> +void test_strncmp()
> +{
> +char str1[] = "abc";
> +char str2[] = "abc";
> +char str3[] = "def";
> +assert(strncmp(str1, str2, 2) == 0);
> +assert(strncmp(str1, str3, 2) != 0);
> +}
> +
> +void test_strcmp()
> +{
> +char str1[] = "abc";
> +char str2[] = "abc";
> +char str3[] = "def";
> +assert(strcmp(str1, str2) == 0);
> +assert(strcmp(str1, str3) != 0);
> +}
> +
> +void test_memset()
> +{
> +char buffer[10];
> +memset(buffer, 'A', 10);
> +int i;
> +for (i = 0; i < 10; i++) {
> +assert(buffer[i] == 'A');
> +}
> +}
> +
> +int main()
> +{
> +test_memset();
> +test_memcpy();
> +test_strncpy();
> +test_memcmp();
> +test_strncmp();
> +test_strcpy();
> +test_strcmp();
> +test_strcat();
> +
> +return EXIT_SUCCESS;
> +}
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH for-8.2] dockerfiles: bump tricore cross compiler container to Debian 11
Paolo Bonzini writes: > With the release of version 12 on June 10, 2023, Debian 10 is > not supported anymore. Modify the cross compiler container to > build on a newer version. > > Signed-off-by: Paolo Bonzini > --- > tests/docker/dockerfiles/debian-tricore-cross.docker | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/docker/dockerfiles/debian-tricore-cross.docker > b/tests/docker/dockerfiles/debian-tricore-cross.docker > index 269bfa8d423..5bd1963fb55 100644 > --- a/tests/docker/dockerfiles/debian-tricore-cross.docker > +++ b/tests/docker/dockerfiles/debian-tricore-cross.docker > @@ -9,7 +9,7 @@ > # > # SPDX-License-Identifier: GPL-2.0-or-later > # > -FROM docker.io/library/debian:buster-slim > +FROM docker.io/library/debian:11-slim > > MAINTAINER Philippe Mathieu-Daudé Acked-by: Alex Bennée -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH v2 1/7] tcg/ppc: Untabify tcg-target.c.inc
Acked-by: Nicholas Piggin
On Tue Aug 8, 2023 at 1:02 PM AEST, Richard Henderson wrote:
> Signed-off-by: Richard Henderson
> ---
> tcg/ppc/tcg-target.c.inc | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tcg/ppc/tcg-target.c.inc b/tcg/ppc/tcg-target.c.inc
> index 511e14b180..642d0fd128 100644
> --- a/tcg/ppc/tcg-target.c.inc
> +++ b/tcg/ppc/tcg-target.c.inc
> @@ -221,7 +221,7 @@ static inline bool in_range_b(tcg_target_long target)
> }
>
> static uint32_t reloc_pc24_val(const tcg_insn_unit *pc,
> -const tcg_insn_unit *target)
> + const tcg_insn_unit *target)
> {
> ptrdiff_t disp = tcg_ptr_byte_diff(target, pc);
> tcg_debug_assert(in_range_b(disp));
> @@ -241,7 +241,7 @@ static bool reloc_pc24(tcg_insn_unit *src_rw, const
> tcg_insn_unit *target)
> }
>
> static uint16_t reloc_pc14_val(const tcg_insn_unit *pc,
> -const tcg_insn_unit *target)
> + const tcg_insn_unit *target)
> {
> ptrdiff_t disp = tcg_ptr_byte_diff(target, pc);
> tcg_debug_assert(disp == (int16_t) disp);
> @@ -3587,7 +3587,7 @@ static void expand_vec_mul(TCGType type, unsigned vece,
> TCGv_vec v0,
>tcgv_vec_arg(t1), tcgv_vec_arg(t2));
> vec_gen_3(INDEX_op_ppc_pkum_vec, type, vece, tcgv_vec_arg(v0),
>tcgv_vec_arg(v0), tcgv_vec_arg(t1));
> - break;
> +break;
>
> case MO_32:
> tcg_debug_assert(!have_isa_2_07);
Re: [PATCH] block-migration: Ensure we don't crash during migration cleanup
On 8/8/23 19:08, Stefan Hajnoczi wrote:
> On Mon, Jul 31, 2023 at 05:33:38PM -0300, Fabiano Rosas wrote:
>> We can fail the blk_insert_bs() at init_blk_migration(), leaving the
>> BlkMigDevState without a dirty_bitmap and BlockDriverState. Account
>> for the possibly missing elements when doing cleanup.
>>
>> Fix the following crashes:
>>
>> Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
>> 0x55ec83ef in bdrv_release_dirty_bitmap (bitmap=0x0) at
>> ../block/dirty-bitmap.c:359
>> 359 BlockDriverState *bs = bitmap->bs;
>> #0 0x55ec83ef in bdrv_release_dirty_bitmap (bitmap=0x0) at
>> ../block/dirty-bitmap.c:359
>> #1 0x55bba331 in unset_dirty_tracking () at
>> ../migration/block.c:371
>> #2 0x55bbad98 in block_migration_cleanup_bmds () at
>> ../migration/block.c:681
>>
>> Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
>> 0x55e971ff in bdrv_op_unblock (bs=0x0,
>> op=BLOCK_OP_TYPE_BACKUP_SOURCE, reason=0x0) at ../block.c:7073
>> 7073QLIST_FOREACH_SAFE(blocker, &bs->op_blockers[op], list, next) {
>> #0 0x55e971ff in bdrv_op_unblock (bs=0x0,
>> op=BLOCK_OP_TYPE_BACKUP_SOURCE, reason=0x0) at ../block.c:7073
>> #1 0x55e9734a in bdrv_op_unblock_all (bs=0x0, reason=0x0) at
>> ../block.c:7095
>> #2 0x55bbae13 in block_migration_cleanup_bmds () at
>> ../migration/block.c:690
>>
>> Signed-off-by: Fabiano Rosas
>> ---
>> migration/block.c | 11 +--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> Sorry, I missed this patch!
>
> If this needs to be in QEMU 8.1 (-rc3 is being tagged today), please
> reply and provide a justification. At this point only security fixes and
> showstoppers will be merged. Thanks!
>
> Applied to my block-next tree for QEMU 8.2:
> https://gitlab.com/stefanha/qemu/commits/block-next
>
> Stefan
Thanks, and in my personal view I think it's ok for 8.2, IIUC it happens during
the migration to file work which is not in 8.1 anyway,
Fabiano correct me here if I am wrong,
Ciao,
Claudio
Re: [PATCH v2 2/7] tcg/ppc: Use PADDI in tcg_out_movi
On Tue Aug 8, 2023 at 1:02 PM AEST, Richard Henderson wrote:
> PADDI can load 34-bit immediates and 34-bit pc-relative addresses.
>
Reviewed-by: Nicholas Piggin
> Reviewed-by: Jordan Niethe
> Signed-off-by: Richard Henderson
> ---
> tcg/ppc/tcg-target.c.inc | 51
> 1 file changed, 51 insertions(+)
>
> diff --git a/tcg/ppc/tcg-target.c.inc b/tcg/ppc/tcg-target.c.inc
> index 642d0fd128..2141c0bc78 100644
> --- a/tcg/ppc/tcg-target.c.inc
> +++ b/tcg/ppc/tcg-target.c.inc
> @@ -707,6 +707,38 @@ static bool patch_reloc(tcg_insn_unit *code_ptr, int
> type,
> return true;
> }
>
> +/* Ensure that the prefixed instruction does not cross a 64-byte boundary. */
> +static bool tcg_out_need_prefix_align(TCGContext *s)
> +{
> +return ((uintptr_t)s->code_ptr & 0x3f) == 0x3c;
> +}
> +
> +static void tcg_out_prefix_align(TCGContext *s)
> +{
> +if (tcg_out_need_prefix_align(s)) {
> +tcg_out32(s, NOP);
> +}
> +}
> +
> +static ptrdiff_t tcg_pcrel_diff_for_prefix(TCGContext *s, const void *target)
> +{
> +return tcg_pcrel_diff(s, target) - (tcg_out_need_prefix_align(s) ? 4 :
> 0);
> +}
> +
> +/* Output Type 10 Prefix - Modified Load/Store Form (MLS:D) */
> +static void tcg_out_mls_d(TCGContext *s, tcg_insn_unit opc, unsigned rt,
> + unsigned ra, tcg_target_long imm, bool r)
> +{
> +tcg_insn_unit p, i;
> +
> +p = OPCD(1) | (2 << 24) | (r << 20) | ((imm >> 16) & 0x3);
> +i = opc | TAI(rt, ra, imm);
> +
> +tcg_out_prefix_align(s);
> +tcg_out32(s, p);
> +tcg_out32(s, i);
> +}
> +
> static void tcg_out_mem_long(TCGContext *s, int opi, int opx, TCGReg rt,
> TCGReg base, tcg_target_long offset);
>
> @@ -992,6 +1024,25 @@ static void tcg_out_movi_int(TCGContext *s, TCGType
> type, TCGReg ret,
> return;
> }
>
> +/*
> + * Load values up to 34 bits, and pc-relative addresses,
> + * with one prefixed insn.
> + */
> +if (have_isa_3_10) {
> +if (arg == sextract64(arg, 0, 34)) {
> +/* pli ret,value = paddi ret,0,value,0 */
> +tcg_out_mls_d(s, ADDI, ret, 0, arg, 0);
> +return;
> +}
> +
> +tmp = tcg_pcrel_diff_for_prefix(s, (void *)arg);
> +if (tmp == sextract64(tmp, 0, 34)) {
> +/* pla ret,value = paddi ret,0,value,1 */
> +tcg_out_mls_d(s, ADDI, ret, 0, tmp, 1);
> +return;
> +}
> +}
> +
> /* Load 32-bit immediates with two insns. Note that we've already
> eliminated bare ADDIS, so we know both insns are required. */
> if (TCG_TARGET_REG_BITS == 32 || arg == (int32_t)arg) {
Re: [PATCH] vfio/pci: hide ROM BAR on SFC9220 10/40G Ethernet Controller PF
On 8/8/23 17:40, Alex Williamson wrote:
> On Tue, 8 Aug 2023 16:59:16 +0200
> Laszlo Ersek wrote:
>
>> The Solarflare Communications SFC9220 NIC's physical function (PF) appears
>> to expose an expansion ROM with the following characteristics:
>>
>> (1) Single-image ROM, with only a legacy BIOS image (no UEFI driver).
>> Alex's rom-parser utility dumps it like this:
>>
>>> Valid ROM signature found @0h, PCIR offset 20h
>>> PCIR: type 0 (x86 PC-AT), vendor: 1924, device: 0a03, class: 02
>>> PCIR: revision 3, vendor revision: 1
>>> Last image
>>
>> (2) The BIOS image crashes when booted on i440fx.
>>
>> (3) The BIOS image prints the following messages on q35:
>>
>>> Solarflare Boot Manager (v5.2.2.1006)
>>> Solarflare Communications 2008-2019
>>> gPXE (http://etherboot.org) - [...] PCI[...] PnP PMM[...]
>>
>> So it appears like a modified derivative of old gPXE.
>>
>> Alex surmised in advance that the BIOS image could be accessing
>> host-physical addresses rather than guest-phys ones, leading to the crash
>> on i440fx.
>
> ROMs sometimes take shortcuts around the standard interfaces to the
> device and can therefore hit gaps in the virtualization, which is why
> that's suspect to me. However if it works on q35 but not 440fx it
> might be more that we're not matching a PCI topology expectation of the
> ROM. Was it only tested on 440fx attached to the root bus or does it
> also fail if the PF is attached downstream of a PCI-to-PCI bridge?
I don't know; I'll need to test both of these setups then.
>
>> Don't expose the option ROM BAR to the VM by default. While this prevents
>> netbooting the VM off the PF on q35/SeaBIOS (a relatively rare scenario),
>> it does not make any difference for UEFI, and at least the VM doesn't
>> crash during boot on i440fx/SeaBIOS (a relatively frequent scenario).
>> Users can restore the original behavior via the QEMU cmdline and the
>> libvirt domain XML.
>>
>> (In two years, we've not seen any customer interest in this bug, hence
>> there's no incentive to investigate (2).)
>>
>> Cc: Alex Williamson (supporter:VFIO)
>> Cc: "Cédric Le Goater" (supporter:VFIO)
>> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1975776
>> Signed-off-by: Laszlo Ersek
>> ---
>> hw/vfio/pci-quirks.c | 4
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
>> index f4ff83680572..270eb16b91fa 100644
>> --- a/hw/vfio/pci-quirks.c
>> +++ b/hw/vfio/pci-quirks.c
>> @@ -45,6 +45,10 @@ static const struct {
>> uint32_t device;
>> } rom_denylist[] = {
>> { 0x14e4, 0x168e }, /* Broadcom BCM 57810 */
>> +{ 0x1924, 0x0a03 }, /* Solarflare Communications
>> + * SFC9220 10/40G Ethernet Controller
>> + *
>> https://bugzilla.redhat.com/show_bug.cgi?id=1975776
>
> Unfortunately this is not a public bz so there's not much point in
> referencing it in public code or commit log :-\ Thanks,
The BZ is not public because it was originally (mis-)filed for the RH
kernel, and those BZs are private by default. I'd corrected the BZ
component yesterday, but didn't realize I should relax the BZ's
visibility. I'll do that now. (That's the right thing to do regardless
of whether this patch gets in or not.)
Thanks!
Laszlo
>
> Alex
>
>> + */
>> };
>>
>> bool vfio_opt_rom_in_denylist(VFIOPCIDevice *vdev)
>
Re: [PATCH 2/2] linux-user: Use ARRAY_SIZE with bitmask_transtbl
Richard Henderson writes:
> Rather than using a zero tuple to end the table, use a macro
> to apply ARRAY_SIZE and pass that on to the convert functions.
>
> This fixes two bugs in which the conversion functions required
> that both the target and host masks be non-zero in order to
> continue, rather than require both target and host masks be
> zero in order to terminate.
>
> This affected mmap_flags_tbl when the host does not support
> all of the flags we wish to convert (e.g. MAP_UNINITIALIZED).
> Mapping these flags to zero is good enough, and matches how
> the kernel ignores bits that are unknown.
>
> Fixes: 4b840f96 ("linux-user: Populate more bits in mmap_flags_tbl")
> Signed-off-by: Richard Henderson
Reviewed-by: Alex Bennée
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH 1/2] linux-user: Split out do_mmap
Richard Henderson writes:
> New function that rejects unsupported map types and flags.
> In 4b840f96 we should not have accepted MAP_SHARED_VALIDATE
> without actually validating the rest of the flags.
>
> Fixes: 4b840f96 ("linux-user: Populate more bits in mmap_flags_tbl")
> Signed-off-by: Richard Henderson
Reviewed-by: Alex Bennée
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH v1 1/3] softmmu/physmem: fallback to opening guest RAM file as readonly in a MAP_PRIVATE mapping
Hi Peter!
-fd = file_ram_open(mem_path, memory_region_name(mr), readonly, &created,
- errp);
+fd = file_ram_open(mem_path, memory_region_name(mr), readonly, &created);
+if (fd == -EACCES && !(ram_flags & RAM_SHARED) && !readonly) {
+/*
+ * We can have a writable MAP_PRIVATE mapping of a readonly file.
+ * However, some operations like ftruncate() or fallocate() might fail
+ * later, let's warn the user.
+ */
+fd = file_ram_open(mem_path, memory_region_name(mr), true, &created);
+if (fd >= 0) {
+warn_report("backing store %s for guest RAM (MAP_PRIVATE) opened"
+" readonly because the file is not writable",
mem_path);
I can understand the use case, but this will be slightly unwanted,
especially the user doesn't yet have a way to predict when will it happen.
Users can set the file permissions accordingly I guess. If they don't
want the file to never ever be modified via QEMU, set it R/O.
Meanwhile this changes the behavior, is it a concern that someone may want
to rely on current behavior of failing?
The scenario would be that someone passes a readonly file to "-mem-path"
or "-object memory-backend-file,share=off,readonly=off", with the
expectation that it would currently fail.
If it now doesn't fail (and we warn instead), what would happen is:
* In file_ram_alloc() we won't even try ftruncate(), because the file
already had a size > 0. So ftruncate() is not a concern as I now
realize.
* fallocate might fail later. AFAIKS, that only applies to
ram_block_discard_range().
-> virtio-mem performs an initial ram_block_discard_range() check and
fails gracefully early.
-> virtio-ballooon ignores any errors
-> ram_discard_range() in migration code fails early for postcopy in
init_range() and loadvm_postcopy_ram_handle_discard(), handling it
gracefully.
So mostly nothing "bad" would happen, it might just be undesirable, and
we properly warn.
Most importantly, we won't be corrupting/touching the original file in
any case, because it is R/O.
If we really want to be careful, we could clue that behavior to compat
machines. I'm not really sure yet if we really have to go down that path.
Any other alternatives? I'd like to avoid new flags where not really
required.
To think from a higher level of current use case, the ideal solution seems
to me that if the ram file can be put on a file system that supports CoW
itself (like btrfs), we can snapshot that ram file and make it RW for the
qemu instance. Then here it'll be able to open the file. We'll be able to
keep the interface working as before, meanwhile it'll work with fallocate
or truncations too I assume.
Would that be better instead of changing QEMU?
As I recently learned, using file-backed VMs (on real ssd/disks, not
shmem/hugetlb) is usually undesired, because the dirtied pages will
constantly get rewritten to disk by background writeback threads,
eventually resulting in bad performance and SSD wear.
So while using a COW filesystem sounds cleaner in theory, it's not
applicable in practice -- unless one disables any background writeback,
which has different side effects because it cannot be configured on a
per-file basis.
So for VM templating, it makes sense to capture the guest RAM and store
it in a file, to then use a COW (MAP_PRIVATE) mapping. Using a read-only
file makes perfect sense in that scenario IMHO.
[I'm curious at what point a filesystem will actually break COW. if it's
wired up to the writenotify infrastructure, it would happen when
actually writing to a page, not at mmap time. I know that filesystems
use writenotify for lazy allocation of disk blocks on file holes, maybe
they also do that for lazy allocation of disk blocks on COW]
Thanks!
--
Cheers,
David / dhildenb
Re: [PATCH 4/7] spapr: Fix record-replay machine reset consuming too many events
On Tue Aug 8, 2023 at 1:52 PM AEST, Pavel Dovgalyuk wrote: > On 08.08.2023 06:09, Nicholas Piggin wrote: > > On Sun Aug 6, 2023 at 9:46 PM AEST, Nicholas Piggin wrote: > >> On Fri Aug 4, 2023 at 6:50 PM AEST, Pavel Dovgalyuk wrote: > >>> BTW, there is a function qemu_register_reset_nosnapshotload that can be > >>> used in similar cases. > >>> Can you just use it without changing the code of the reset handler? > >> > >> I didn't know that, thanks for pointing it out. I'll take a closer look > >> at it before reposting. > > > > Seems a bit tricky because the device tree has to be rebuilt at reset > > time (including snapshot load), but it uses the random number. So > > It seems strange to me, that loading the existing configuration has to > randomize the device tree. Building the device tree requires a random number for one of the properties. Other architectures that don't have this "cas" firmware call that changes the device tree and so requires it is rebuilt at machine reset time just build the device tree once at machine creation time I believe. So spapr is already weird in that way. We could go the way that other archs have and just save that random number once at creation and then reuse it for each reset. I thought that was not so good because for a normal reset I think it is better to get a new random number each time, no? So I think it's natural enough to take a new random number for a regular reset, but keep the existing one for a snapshot reset. I could be misunderstanding something though. Thanks, Nick > > > having a second nosnapshotload reset function might not be called in > > the correct order, I think? For now I will keep it as is. > > Ok, let's wait for other reviewers. > > > Pavel Dovgalyuk
[PATCH 1/9] python: mkvenv: tweak the matching of --diagnose to depspecs
Move the matching between the "absent" array and dep_specs[0] inside
the loop, preparing for the possibility of having multiple canaries
among the installed packages.
Signed-off-by: Paolo Bonzini
---
python/scripts/mkvenv.py | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/python/scripts/mkvenv.py b/python/scripts/mkvenv.py
index a47f1eaf5d6..399659b22f1 100644
--- a/python/scripts/mkvenv.py
+++ b/python/scripts/mkvenv.py
@@ -806,6 +806,7 @@ def _do_ensure(
"""
absent = []
present = []
+canary = None
for spec in dep_specs:
matcher = distlib.version.LegacyMatcher(spec)
ver = _get_version(matcher.name)
@@ -817,6 +818,8 @@ def _do_ensure(
or not matcher.match(distlib.version.LegacyVersion(ver))
):
absent.append(spec)
+if spec == dep_specs[0]:
+canary = prog
else:
logger.info("found %s %s", matcher.name, ver)
present.append(matcher.name)
@@ -839,7 +842,7 @@ def _do_ensure(
absent[0],
online,
wheels_dir,
-prog if absent[0] == dep_specs[0] else None,
+canary,
)
return None
--
2.41.0
[PATCH 2/9] python: mkvenv: introduce TOML-like representation of dependencies
We would like to place all Python dependencies in the same file, so that
we can add more information without having long and complex command lines.
The plan is to have a TOML file with one entry per package, for example
[avocado]
avocado-framework = {
accepted = "(>=88.1, <93.0)",
installed = "88.1",
canary = "avocado"
}
Each TOML section will thus be a dictionary of dictionaries. Modify
mkvenv.py's workhorse function, _do_ensure, to already operate on such
a data structure. The "ensure" subcommand is modified to separate the
depspec into a name and a version part, and use the result (plus the
--diagnose argument) to build a dictionary for each command line argument.
Signed-off-by: Paolo Bonzini
---
python/scripts/mkvenv.py | 77 +++-
1 file changed, 61 insertions(+), 16 deletions(-)
diff --git a/python/scripts/mkvenv.py b/python/scripts/mkvenv.py
index 399659b22f1..96f506d7e22 100644
--- a/python/scripts/mkvenv.py
+++ b/python/scripts/mkvenv.py
@@ -46,6 +46,9 @@
"""
+# The duplication between importlib and pkg_resources does not help
+# pylint: disable=too-many-lines
+
# Copyright (C) 2022-2023 Red Hat, Inc.
#
# Authors:
@@ -69,6 +72,7 @@
from types import SimpleNamespace
from typing import (
Any,
+Dict,
Iterator,
Optional,
Sequence,
@@ -786,43 +790,67 @@ def pip_install(
)
+def _make_version_constraint(info: Dict[str, str], install: bool) -> str:
+"""
+Construct the version constraint part of a PEP 508 dependency
+specification (for example '>=0.61.5') from the accepted and
+installed keys of the provided dictionary.
+
+:param info: A dictionary corresponding to a TOML key-value list.
+:param install: True generates install constraints, False generates
+presence constraints
+"""
+if install and "installed" in info:
+return "==" + info["installed"]
+
+dep_spec = info.get("accepted", "")
+dep_spec = dep_spec.strip()
+# Double check that they didn't just use a version number
+if dep_spec and dep_spec[0] not in "!~><=(":
+raise Ouch(
+"invalid dependency specifier " + dep_spec + " in dependency file"
+)
+
+return dep_spec
+
+
def _do_ensure(
-dep_specs: Sequence[str],
+group: Dict[str, Dict[str, str]],
online: bool = False,
wheels_dir: Optional[Union[str, Path]] = None,
-prog: Optional[str] = None,
) -> Optional[Tuple[str, bool]]:
"""
-Use pip to ensure we have the package specified by @dep_specs.
+Use pip to ensure we have the packages specified in @group.
-If the package is already installed, do nothing. If online and
+If the packages are already installed, do nothing. If online and
wheels_dir are both provided, prefer packages found in wheels_dir
first before connecting to PyPI.
-:param dep_specs:
-PEP 508 dependency specifications. e.g. ['meson>=0.61.5'].
+:param group: A dictionary of dictionaries, corresponding to a
+section in a pythondeps.toml file.
:param online: If True, fall back to PyPI.
:param wheels_dir: If specified, search this path for packages.
"""
absent = []
present = []
canary = None
-for spec in dep_specs:
-matcher = distlib.version.LegacyMatcher(spec)
-ver = _get_version(matcher.name)
+for name, info in group.items():
+constraint = _make_version_constraint(info, False)
+matcher = distlib.version.LegacyMatcher(name + constraint)
+ver = _get_version(name)
if (
ver is None
# Always pass installed package to pip, so that they can be
# updated if the requested version changes
-or not _is_system_package(matcher.name)
+or not _is_system_package(name)
or not matcher.match(distlib.version.LegacyVersion(ver))
):
-absent.append(spec)
-if spec == dep_specs[0]:
-canary = prog
+absent.append(name + _make_version_constraint(info, True))
+if len(absent) == 1:
+canary = info.get("canary", None)
else:
-logger.info("found %s %s", matcher.name, ver)
-present.append(matcher.name)
+logger.info("found %s %s", name, ver)
+present.append(name)
if present:
generate_console_scripts(present)
@@ -875,7 +903,24 @@ def ensure(
if not HAVE_DISTLIB:
raise Ouch("a usable distlib could not be found, please install it")
-result = _do_ensure(dep_specs, online, wheels_dir, prog)
+# Convert the depspecs to a dictionary, as if they came
+# from a section in a pythondeps.toml file
+group: Dict[str, Dict[str, str]] = {}
+for spec in dep_specs:
+name = distlib.version.LegacyMatcher(spec).name
+group[name] = {}
+
+spec = spec.strip()
+pos = len(name)
+ver =
[PATCH 4/9] lcitool: bump libvirt-ci submodule and regenerate
This brings in a newer version of the pipewire mapping, so rename it. Python 3.9 and 3.10 do not seem to work in OpenSUSE LEAP 15.5 (weird, because 3.9 persisted from 15.3 to 15.4) so bump the Python runtime version to 3.11. Signed-off-by: Paolo Bonzini --- .../dockerfiles/debian-amd64-cross.docker | 2 +- .../dockerfiles/debian-arm64-cross.docker | 2 +- .../dockerfiles/debian-armel-cross.docker | 2 +- .../dockerfiles/debian-armhf-cross.docker | 2 +- .../dockerfiles/debian-mips64el-cross.docker | 2 +- .../dockerfiles/debian-mipsel-cross.docker| 2 +- .../dockerfiles/debian-ppc64el-cross.docker | 2 +- .../dockerfiles/debian-riscv64-cross.docker | 2 +- .../dockerfiles/debian-s390x-cross.docker | 2 +- .../dockerfiles/fedora-win32-cross.docker | 2 +- .../dockerfiles/fedora-win64-cross.docker | 2 +- tests/docker/dockerfiles/opensuse-leap.docker | 22 +-- tests/lcitool/libvirt-ci | 2 +- tests/lcitool/mappings.yml| 12 +- tests/lcitool/projects/qemu.yml | 2 +- tests/lcitool/targets/opensuse-leap-15.yml| 4 ++-- 16 files changed, 32 insertions(+), 32 deletions(-) diff --git a/tests/docker/dockerfiles/debian-amd64-cross.docker b/tests/docker/dockerfiles/debian-amd64-cross.docker index b7bdc012431..b9871f9c8c7 100644 --- a/tests/docker/dockerfiles/debian-amd64-cross.docker +++ b/tests/docker/dockerfiles/debian-amd64-cross.docker @@ -1,6 +1,6 @@ # THIS FILE WAS AUTO-GENERATED # -# $ lcitool dockerfile --layers all --cross x86_64 debian-11 qemu +# $ lcitool dockerfile --layers all --cross-arch x86_64 debian-11 qemu # # https://gitlab.com/libvirt/libvirt-ci diff --git a/tests/docker/dockerfiles/debian-arm64-cross.docker b/tests/docker/dockerfiles/debian-arm64-cross.docker index 68165c2f23e..3504c771a76 100644 --- a/tests/docker/dockerfiles/debian-arm64-cross.docker +++ b/tests/docker/dockerfiles/debian-arm64-cross.docker @@ -1,6 +1,6 @@ # THIS FILE WAS AUTO-GENERATED # -# $ lcitool dockerfile --layers all --cross aarch64 debian-11 qemu +# $ lcitool dockerfile --layers all --cross-arch aarch64 debian-11 qemu # # https://gitlab.com/libvirt/libvirt-ci diff --git a/tests/docker/dockerfiles/debian-armel-cross.docker b/tests/docker/dockerfiles/debian-armel-cross.docker index 2fb65308c7a..6d11c9510fd 100644 --- a/tests/docker/dockerfiles/debian-armel-cross.docker +++ b/tests/docker/dockerfiles/debian-armel-cross.docker @@ -1,6 +1,6 @@ # THIS FILE WAS AUTO-GENERATED # -# $ lcitool dockerfile --layers all --cross armv6l debian-11 qemu +# $ lcitool dockerfile --layers all --cross-arch armv6l debian-11 qemu # # https://gitlab.com/libvirt/libvirt-ci diff --git a/tests/docker/dockerfiles/debian-armhf-cross.docker b/tests/docker/dockerfiles/debian-armhf-cross.docker index df77ccb57bd..37ae575cf7e 100644 --- a/tests/docker/dockerfiles/debian-armhf-cross.docker +++ b/tests/docker/dockerfiles/debian-armhf-cross.docker @@ -1,6 +1,6 @@ # THIS FILE WAS AUTO-GENERATED # -# $ lcitool dockerfile --layers all --cross armv7l debian-11 qemu +# $ lcitool dockerfile --layers all --cross-arch armv7l debian-11 qemu # # https://gitlab.com/libvirt/libvirt-ci diff --git a/tests/docker/dockerfiles/debian-mips64el-cross.docker b/tests/docker/dockerfiles/debian-mips64el-cross.docker index 63a3d7aa3b9..26ed730165a 100644 --- a/tests/docker/dockerfiles/debian-mips64el-cross.docker +++ b/tests/docker/dockerfiles/debian-mips64el-cross.docker @@ -1,6 +1,6 @@ # THIS FILE WAS AUTO-GENERATED # -# $ lcitool dockerfile --layers all --cross mips64el debian-11 qemu +# $ lcitool dockerfile --layers all --cross-arch mips64el debian-11 qemu # # https://gitlab.com/libvirt/libvirt-ci diff --git a/tests/docker/dockerfiles/debian-mipsel-cross.docker b/tests/docker/dockerfiles/debian-mipsel-cross.docker index ac87bbb0956..ade2f37ed1d 100644 --- a/tests/docker/dockerfiles/debian-mipsel-cross.docker +++ b/tests/docker/dockerfiles/debian-mipsel-cross.docker @@ -1,6 +1,6 @@ # THIS FILE WAS AUTO-GENERATED # -# $ lcitool dockerfile --layers all --cross mipsel debian-11 qemu +# $ lcitool dockerfile --layers all --cross-arch mipsel debian-11 qemu # # https://gitlab.com/libvirt/libvirt-ci diff --git a/tests/docker/dockerfiles/debian-ppc64el-cross.docker b/tests/docker/dockerfiles/debian-ppc64el-cross.docker index def11f16933..08dcffa0a85 100644 --- a/tests/docker/dockerfiles/debian-ppc64el-cross.docker +++ b/tests/docker/dockerfiles/debian-ppc64el-cross.docker @@ -1,6 +1,6 @@ # THIS FILE WAS AUTO-GENERATED # -# $ lcitool dockerfile --layers all --cross ppc64le debian-11 qemu +# $ lcitool dockerfile --layers all --cross-arch ppc64le debian-11 qemu # # https://gitlab.com/libvirt/libvirt-ci diff --git a/tests/docker/dockerfiles/debian-riscv64-cross.docker b/tests/docker/dockerfiles/debian-riscv64-cross.docker index a2d879ee1fd..a26637ec4fb 100644 --- a/tests/docker/dockerfile
[PATCH 8/9] Revert "tests: Use separate virtual environment for avocado"
This reverts commit e8e4298feadae7924cf7600bb3bcc5b0a8d7cbe9. ensuregroup allows to specify both the acceptable versions of avocado, and a locked version to be used when avocado is not installed as a system pacakge. This lets us install avocado in pyvenv/ using "mkvenv.py" and reuse the distro package on Fedora and CentOS Stream (the only distros where it's available). ensuregroup's usage of "(>=..., <=...)" constraints when evaluating the distro package, and "==" constraints when installing it from PyPI, makes it possible to avoid conflicts between the known-good version and a package plugins included in the distro. This is because package plugins have "==" constraints on the version that is included in the distro, and, using "pip install avocado==88.1" on a venv that includes system packages will result in an error: avocado-framework-plugin-varianter-yaml-to-mux 98.0 requires avocado-framework==98.0, but you have avocado-framework 88.1 which is incompatible. avocado-framework-plugin-result-html 98.0 requires avocado-framework==98.0, but you have avocado-framework 88.1 which is incompatible. But at the same time, if the venv does not include a system distribution of avocado then we can install a known-good version and stick to LTS releases. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1663 Signed-off-by: Paolo Bonzini --- .gitlab-ci.d/buildtest.yml| 6 +++--- docs/devel/acpi-bits.rst | 6 +++--- docs/devel/testing.rst| 14 +++--- python/scripts/mkvenv.py | 13 + pythondeps.toml | 7 +++ .../org.centos/stream/8/x86_64/test-avocado | 4 ++-- scripts/device-crash-test | 2 +- tests/Makefile.include| 19 --- tests/requirements.txt| 6 -- tests/vm/Makefile.include | 2 +- 10 files changed, 37 insertions(+), 42 deletions(-) delete mode 100644 tests/requirements.txt diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml index 77dc83a6be0..aee91015077 100644 --- a/.gitlab-ci.d/buildtest.yml +++ b/.gitlab-ci.d/buildtest.yml @@ -103,7 +103,7 @@ crash-test-debian: script: - cd build - make NINJA=":" check-venv -- tests/venv/bin/python3 scripts/device-crash-test -q --tcg-only ./qemu-system-i386 +- pyvenv/bin/python3 scripts/device-crash-test -q --tcg-only ./qemu-system-i386 build-system-fedora: extends: @@ -146,8 +146,8 @@ crash-test-fedora: script: - cd build - make NINJA=":" check-venv -- tests/venv/bin/python3 scripts/device-crash-test -q ./qemu-system-ppc -- tests/venv/bin/python3 scripts/device-crash-test -q ./qemu-system-riscv32 +- pyvenv/bin/python3 scripts/device-crash-test -q ./qemu-system-ppc +- pyvenv/bin/python3 scripts/device-crash-test -q ./qemu-system-riscv32 build-system-centos: extends: diff --git a/docs/devel/acpi-bits.rst b/docs/devel/acpi-bits.rst index 22e2580200c..9677b0098f4 100644 --- a/docs/devel/acpi-bits.rst +++ b/docs/devel/acpi-bits.rst @@ -61,19 +61,19 @@ Under ``tests/avocado/`` as the root we have: :: $ make check-venv (needed only the first time to create the venv) - $ ./tests/venv/bin/avocado run -t acpi tests/avocado + $ ./pyvenv/bin/avocado run -t acpi tests/avocado The above will run all acpi avocado tests including this one. In order to run the individual tests, perform the following: :: - $ ./tests/venv/bin/avocado run tests/avocado/acpi-bits.py --tap - + $ ./pyvenv/bin/avocado run tests/avocado/acpi-bits.py --tap - The above will produce output in tap format. You can omit "--tap -" in the end and it will produce output like the following: :: - $ ./tests/venv/bin/avocado run tests/avocado/acpi-bits.py + $ ./pyvenv/bin/avocado run tests/avocado/acpi-bits.py Fetching asset from tests/avocado/acpi-bits.py:AcpiBitsTest.test_acpi_smbios_bits JOB ID : eab225724da7b64c012c65705dc2fa14ab1defef JOB LOG: /home/anisinha/avocado/job-results/job-2022-10-10T17.58-eab2257/job.log diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index b6ad21bed1c..5d1fc0aa95f 100644 --- a/docs/devel/testing.rst +++ b/docs/devel/testing.rst @@ -894,9 +894,9 @@ You can run the avocado tests simply by executing: make check-avocado -This involves the automatic creation of Python virtual environment -within the build tree (at ``tests/venv``) which will have all the -right dependencies, and will save tests results also within the +This involves the automatic installation, from PyPI, of all the +necessary avocado-framework dependencies into the QEMU venv within the +build tree (at ``./pyvenv``). Test results are also saved within the build tree (at ``tests/results``). Note: the build environment must be using a Python 3 stack, and have @
[PATCH 0/9] Use known good releases when installing in pyvenv
This series introduce a new installation command for mkvenv.py that retrieves the packages to be installed from a TOML file. This allows being more flexible in using the system version of a package, while at the same time using a known-good version when installing the package. This is important for packages that sometimes have backwards-incompatible changes or that depend on specific versions of their dependencies. For example, in the case of sphinx we can always use the last version that supports older versions of Python and especially docutils (Ubuntu 20.04 for example would not support sphinx 6.0), and in the case of Avocado we can avoid installing a package that conflicts with any plugins already existing in the system package path. This in turn enables using the same virtual environment for both the build and qemu-iotests. John has patches for this. For the configuration file, TOML was chosen because it was more human readable and easier to edit than JSON. A parser is available in the Python 3.11 standard library; for older versions, the API-compatible replacement tomli is very small (12k). I am introducing it as a vendored .whl file because it is not installed by default in most distros (unlike pip and setuptools which were introduced in 8.0) and because Debian 11 only has it in bullseye-backports. However, if preferred the patch "python: use vendored tomli" can be dropped. In that case, tomli will have to be installed from either PyPI or bullseye-backports. While tomli is bundled with pip, this is only true of recent versions of pip. Of all the supported OSes pretty much only FreeBSD has a recent enough version of pip while staying on Python <3.11. So we cannot use the same trick that is in place for distlib. In order to pick the tomli mapping, lcitool is updated to a recent version. As a side effect this updates from LEAP 15.4 to 15.5. Paolo Paolo Bonzini (9): python: mkvenv: tweak the matching of --diagnose to depspecs python: mkvenv: introduce TOML-like representation of dependencies python: mkvenv: add ensuregroup command lcitool: bump libvirt-ci submodule and regenerate configure: never use PyPI for Meson python: use vendored tomli configure: switch to ensuregroup Revert "tests: Use separate virtual environment for avocado" tests/docker: add python3-tomli dependency to containers .gitlab-ci.d/buildtest.yml| 6 +- .gitlab-ci.d/cirrus/freebsd-13.vars | 2 +- .gitlab-ci.d/cirrus/macos-12.vars | 2 +- configure | 22 +- docs/devel/acpi-bits.rst | 6 +- docs/devel/testing.rst| 14 +- python/scripts/mkvenv.py | 201 -- python/scripts/vendor.py | 5 +- python/setup.cfg | 9 + python/wheels/tomli-2.0.1-py3-none-any.whl| Bin 0 -> 12757 bytes pythondeps.toml | 32 +++ .../org.centos/stream/8/x86_64/test-avocado | 4 +- scripts/device-crash-test | 2 +- tests/Makefile.include| 19 +- tests/docker/dockerfiles/centos8.docker | 3 +- .../dockerfiles/debian-all-test-cross.docker | 7 +- .../dockerfiles/debian-amd64-cross.docker | 6 +- tests/docker/dockerfiles/debian-amd64.docker | 4 + .../dockerfiles/debian-arm64-cross.docker | 6 +- .../dockerfiles/debian-armel-cross.docker | 6 +- .../dockerfiles/debian-armhf-cross.docker | 6 +- .../dockerfiles/debian-hexagon-cross.docker | 6 +- .../dockerfiles/debian-mips64el-cross.docker | 6 +- .../dockerfiles/debian-mipsel-cross.docker| 6 +- .../dockerfiles/debian-ppc64el-cross.docker | 6 +- .../dockerfiles/debian-riscv64-cross.docker | 2 +- .../dockerfiles/debian-s390x-cross.docker | 6 +- .../dockerfiles/debian-tricore-cross.docker | 2 + .../dockerfiles/fedora-i386-cross.docker | 1 + .../dockerfiles/fedora-win32-cross.docker | 2 +- .../dockerfiles/fedora-win64-cross.docker | 2 +- tests/docker/dockerfiles/opensuse-leap.docker | 23 +- tests/docker/dockerfiles/ubuntu2004.docker| 4 +- tests/docker/dockerfiles/ubuntu2204.docker| 1 + tests/lcitool/libvirt-ci | 2 +- tests/lcitool/mappings.yml| 27 ++- tests/lcitool/projects/qemu.yml | 3 +- tests/lcitool/targets/opensuse-leap-15.yml| 4 +- tests/requirements.txt| 6 - tests/vm/Makefile.include | 2 +- tests/vm/generated/freebsd.json | 1 + 41 files changed, 371 insertions(+), 103 deletions(-) create mode 100644 python/wheels/tomli-2.0.1-py3-none-any.whl create mode 100644 pythondeps.toml delete mode 100644 tests/requirements.txt -- 2.41.0
[PATCH 6/9] python: use vendored tomli
Debian only introduced tomli in the bookworm release. Use a
vendored wheel to avoid requiring a package that is only in
bullseye-backports and is also absent in Ubuntu 20.04.
While at it, fix an issue in the vendor.py scripts which does
not add a newline after each package and hash.
Signed-off-by: Paolo Bonzini
---
configure | 6 ++
python/scripts/vendor.py | 5 -
python/wheels/tomli-2.0.1-py3-none-any.whl | Bin 0 -> 12757 bytes
3 files changed, 10 insertions(+), 1 deletion(-)
create mode 100644 python/wheels/tomli-2.0.1-py3-none-any.whl
diff --git a/configure b/configure
index f13f0662b98..347153702c1 100755
--- a/configure
+++ b/configure
@@ -1018,6 +1018,12 @@ fi
python="$python -B"
mkvenv="$python ${source_path}/python/scripts/mkvenv.py"
+# Finish preparing the virtual environment using vendored .whl files
+
+if $python -c 'import sys; sys.exit(sys.version_info >= (3,11))'; then
+$mkvenv ensure --dir "${source_path}/python/wheels" \
+'tomli>=1.2.0' || exit 1
+fi
if ! $mkvenv ensure \
--dir "${source_path}/python/wheels" \
--diagnose "meson" \
diff --git a/python/scripts/vendor.py b/python/scripts/vendor.py
index 34486a51f44..76274871170 100755
--- a/python/scripts/vendor.py
+++ b/python/scripts/vendor.py
@@ -43,13 +43,16 @@ def main() -> int:
packages = {
"meson==0.63.3":
"d677b809c4895dcbaac9bf6c43703fcb3609a4b24c6057c78f828590049cf43a",
+
+"tomli==2.0.1":
+"939de3e7a6161af0c887ef91b7d41a53e7c5a1ca976325f429cb46ea9bc30ecc",
}
vendor_dir = Path(__file__, "..", "..", "wheels").resolve()
with tempfile.NamedTemporaryFile(mode="w", encoding="utf-8") as file:
for dep_spec, checksum in packages.items():
-file.write(f"{dep_spec} --hash=sha256:{checksum}")
+print(f"{dep_spec} --hash=sha256:{checksum}", file=file)
file.flush()
cli_args = [
diff --git a/python/wheels/tomli-2.0.1-py3-none-any.whl
b/python/wheels/tomli-2.0.1-py3-none-any.whl
new file mode 100644
index
..29670b98d16e2bc770d4fea718582e1dc0dd8aca
GIT binary patch
literal 12757
zcmZ{K19T?cvi6(2v8{=1TN67I+qN;W?TKyMwlT47dy-6?{B!QT=Y02k_dnfhuU_4&
zpQrb(uBz4bbjeEt!O#Ez02H7}RYJMAoa`MF1OSNoGm!sWb+)sywqVfHv#_;r*3+Z6
zch`}hi0BtU>O7{2a9ah3PcNtq)hCA71L7L+!5ht;=`*8pC+Iw0f6veEbjEa6aZ!t{
z-7eZr5K1xTPCr1$E}2eui*cb3n04x0V575s9eHtd3AnP2al@G=Ow7|XTHLl>i@Uu~
zQ2;lqTW6Tfh`@q@wZc7Dx~3&*V^rA36cr>aDz~D#4UVmCEh*IfIByKl8D&D${Pjy<~={mz=uH0
z;OKxG#BXMAx=t8hwKuh0k+D=iu>QdmGLYQ_>$QTHK(gPU>jK%Hve4f5qUJ)GmUw5^
zilT(pi7l$g-h5`|Q5ZK@Iy|sLApdVx1nrKoh{6B>EYtx2q`$0aZ{X-;;`qmiL%gew
z>k{$TZxNPH{>B-G()C>|lj^cyTE!*UBx4BSTcyxf1Z1UB>@D~ux!JK>x35$}xoWyMldsqH<45+0P?zA!=C
zrmO~iH3qtRxp@te>kbb1y{B()f+NS~wt=ylY5e5Z@z!@|J%+wpvA{KvWSnT=G#FS9
zx`qzx-jZob`TJmBX$^<>dw!!_f*H6^ke
z+osYFw-rsfaF|qDyxGvI*_|vQ^>NGWh1~?hFJ~V(sT=WuCT4KQ^}VQrwHrE4@PeNa
zJ@3x2Uu>Q?ylKO2yADm5HTL2^D}yv6kP9_iN{F03?unuwnodf2mgb1uY)iSNRll9v
zXZ+n&d*q&2W6FEl)Eim{s+XBAt(HCe9O@hsU^(M;|h~dZgYrt!2
zm7oomE|;Ut2;g?~{N3WDXo5e3_L<}k1wsCeS5kjtK)m@r-yOb8gRns_Fw#8rG
zE=5nBoH$F!&665^%LM5GYn_l5||zSlb;T
zPfBsV-1?-Wb_u-H@@SvC@GGa&q2iPKgvZp;{OlgV^!`OwS1b&0`>$x3p7%)<*zJbL
zyFg7sq;WO;yuv~w%=ZDnrcdhcRqW|CsuNskql<)quW*4wBz1ujWy0xCtXouY
zkK9448_4P&GU{B`4KWU*h^L;JB-;dc)>&p*u-T?wANcCm&Yl;-C!57t)h7*LhkX*z
zoeYgBmWEI$_|=^}SH)Wa9m97qe53&sL>yT1`skfRvP6JgCO8P=41{4CBmzI6jst^K
z8J8wzB2qYS^xfcbK>RBi^(N_+FWM0`hgMTxE
zL|v$y=eVB49Qew9_W|LUFanUp!|!%7OaPh1&wro~!kV1zhD)C}-H0T!6uu^I?f^p|7ZDsKSgrTwNWJir;O)LV=?7bwZ~3-lAZrNHYo2k`4GU27
z`X04*5_Afhs%sN%;1Mmi0JX!e(AWEMbULIE@+6>(esH%$x}Jb(~vJWJ7@Qf{$44
z7e~`MjMB^Qul>&``bN2BBtbO&yYN&p;~W+Q)6}oI*a(bF=-i0mR1#y5_sE>#Sikl~
zgsA#kaDi~Ytf$zBvbkx!qnNJ+kr-bt=U1Q5>Qf?T7lSO?4Bigg1<{PF*CpLsb7x@a
zf|0%Z_q(M-4E05Ho@`*^va_Y>ir^La2nE}xYHb~4UHl2hi&x<9ifsjhIC8A^1NA*P
zae)d_0s_E`6^YE?dc<1pES|*%EB&&SHi+apoIDWuUbTU=qSOU|``o5SZlVu@|F)XX
zFOYuW0pw-mOX2&3RFpBMCTF5f>al_lZRZ$LMLv0Uict{f9(-I}S(|zTk8K4*Fghfu
zMW_@-9w6RFTX%*o4>EYs)IxqqPs}o9lvVF4i3j`^rG+hy$vB|#0>T<&#Gua^`S?$Q
z-M%0Oy?aos(hIu
zNyBgOO4B$lGAspX1-ABRdI1n)kREccqQQ%*&@pl4jt`vs3Pfr{utt-{gI7;tWIh8RC+#@megPSS`mU#fH7)V&_Opu`m;3Y82H>SSO@
zXwpwC!!D5^Bz`DbM1(GADdp+uudt_;GAk#5N0@?!sSyrGdTsU5^GW%1JUpST1Qx((
z=QqK@q*Z~C<*OjoQ-*Ufp0x%D4g2-iV(UQ&Xf_9#5#p3+EwO>w1<@vU&DYLpLNnIp
zR8%VpfUUZFuqV_z3Yp&lWOTmr$vt$c9Wr%EkV}pj>+7q7dr$(Eq`-{f=f0tv
z5TPhWfmG`}P=W_sTY4i>yd^?lZo!DC{wA~3p}x(pBLYmC#K3c{;ykf9Wx7}+XNhYd
z!8GJk*-1VHc!UF-81|bQ8I+m7@0-ihnjwAvt(7tYYvArt1@9%Sdpy3ue)|f-JdYe^
zVrx~anRs*l63W}}f0EvAi`<|TProaOq;QZe7m!O3Z#Gh?)e<$9i}Voaj9=I@;y*g=
zzzB(Q(74XIHs@v8$By_yK1a4nV2%P_|I<(N;5n7>E0yzLXc>OIfOJhHj<93{N0f;Z
zm1VoE(ZmCJ1CHsE8tIS;n<@brB(3*@GKj9!L!m2Pz1it#IG()=hYT@Q-U9gYfg=9(
z*0KhumaYq#jrr*mrP)u&8WZf0j6q(m7ilOV{eGC0nm^7sOxLlQO-Lom7y=uM4FCaaPhQVi1j8TY21d4uA-&zcAjg)WTa8$
zCbQG1WXx1bYxTQ{W@V{wy-ih>sw&T-;~v(O=RF!)JqqA;plP>jili1riMuqviFPv&
zbXddoJQZxbkT
z`^BTGlo-#6cq3B!Xa(4ic(TU#Ke|lib)r5#zXt0vhL$H##s?qMHJq$uiWUtq@_1C{
zNbouNw>boNqv4ff&!M6)Q$!5LPUJ&WBt5irMSS0)b%{ink)g-$V(%?|
[PATCH 9/9] tests/docker: add python3-tomli dependency to containers
Instead of having CI pick tomli from the vendored wheel at configure time, place it in the containers. Signed-off-by: Paolo Bonzini --- .gitlab-ci.d/cirrus/freebsd-13.vars | 2 +- .gitlab-ci.d/cirrus/macos-12.vars | 2 +- tests/docker/dockerfiles/centos8.docker | 3 ++- .../dockerfiles/debian-all-test-cross.docker | 7 ++- .../docker/dockerfiles/debian-amd64-cross.docker | 4 tests/docker/dockerfiles/debian-amd64.docker | 4 .../docker/dockerfiles/debian-arm64-cross.docker | 4 .../docker/dockerfiles/debian-armel-cross.docker | 4 .../docker/dockerfiles/debian-armhf-cross.docker | 4 .../dockerfiles/debian-hexagon-cross.docker | 6 +- .../dockerfiles/debian-mips64el-cross.docker | 4 .../docker/dockerfiles/debian-mipsel-cross.docker | 4 .../dockerfiles/debian-ppc64el-cross.docker | 4 .../docker/dockerfiles/debian-s390x-cross.docker | 4 .../dockerfiles/debian-tricore-cross.docker | 2 ++ tests/docker/dockerfiles/fedora-i386-cross.docker | 1 + tests/docker/dockerfiles/opensuse-leap.docker | 3 ++- tests/docker/dockerfiles/ubuntu2004.docker| 4 +++- tests/docker/dockerfiles/ubuntu2204.docker| 1 + tests/lcitool/mappings.yml| 15 +++ tests/lcitool/projects/qemu.yml | 1 + tests/vm/generated/freebsd.json | 1 + 22 files changed, 77 insertions(+), 7 deletions(-) diff --git a/.gitlab-ci.d/cirrus/freebsd-13.vars b/.gitlab-ci.d/cirrus/freebsd-13.vars index facb649f5bd..3785afca36d 100644 --- a/.gitlab-ci.d/cirrus/freebsd-13.vars +++ b/.gitlab-ci.d/cirrus/freebsd-13.vars @@ -11,6 +11,6 @@ MAKE='/usr/local/bin/gmake' NINJA='/usr/local/bin/ninja' PACKAGING_COMMAND='pkg' PIP3='/usr/local/bin/pip-3.8' -PKGS='alsa-lib bash bison bzip2 ca_root_nss capstone4 ccache cmocka ctags curl cyrus-sasl dbus diffutils dtc flex fusefs-libs3 gettext git glib gmake gnutls gsed gtk3 json-c libepoxy libffi libgcrypt libjpeg-turbo libnfs libslirp libspice-server libssh libtasn1 llvm lzo2 meson mtools ncurses nettle ninja opencv pixman pkgconf png py39-numpy py39-pillow py39-pip py39-sphinx py39-sphinx_rtd_theme py39-yaml python3 rpm2cpio sdl2 sdl2_image snappy sndio socat spice-protocol tesseract usbredir virglrenderer vte3 xorriso zstd' +PKGS='alsa-lib bash bison bzip2 ca_root_nss capstone4 ccache cmocka ctags curl cyrus-sasl dbus diffutils dtc flex fusefs-libs3 gettext git glib gmake gnutls gsed gtk3 json-c libepoxy libffi libgcrypt libjpeg-turbo libnfs libslirp libspice-server libssh libtasn1 llvm lzo2 meson mtools ncurses nettle ninja opencv pixman pkgconf png py39-numpy py39-pillow py39-pip py39-sphinx py39-sphinx_rtd_theme py39-tomli py39-yaml python3 rpm2cpio sdl2 sdl2_image snappy sndio socat spice-protocol tesseract usbredir virglrenderer vte3 xorriso zstd' PYPI_PKGS='' PYTHON='/usr/local/bin/python3' diff --git a/.gitlab-ci.d/cirrus/macos-12.vars b/.gitlab-ci.d/cirrus/macos-12.vars index ceb294e1539..80eadaab296 100644 --- a/.gitlab-ci.d/cirrus/macos-12.vars +++ b/.gitlab-ci.d/cirrus/macos-12.vars @@ -12,5 +12,5 @@ NINJA='/opt/homebrew/bin/ninja' PACKAGING_COMMAND='brew' PIP3='/opt/homebrew/bin/pip3' PKGS='bash bc bison bzip2 capstone ccache cmocka ctags curl dbus diffutils dtc flex gcovr gettext git glib gnu-sed gnutls gtk+3 jemalloc jpeg-turbo json-c libepoxy libffi libgcrypt libiscsi libnfs libpng libslirp libssh libtasn1 libusb llvm lzo make meson mtools ncurses nettle ninja pixman pkg-config python3 rpm2cpio sdl2 sdl2_image snappy socat sparse spice-protocol tesseract usbredir vde vte3 xorriso zlib zstd' -PYPI_PKGS='PyYAML numpy pillow sphinx sphinx-rtd-theme' +PYPI_PKGS='PyYAML numpy pillow sphinx sphinx-rtd-theme tomli' PYTHON='/opt/homebrew/bin/python3' diff --git a/tests/docker/dockerfiles/centos8.docker b/tests/docker/dockerfiles/centos8.docker index da7dc818fb6..fc1830966f4 100644 --- a/tests/docker/dockerfiles/centos8.docker +++ b/tests/docker/dockerfiles/centos8.docker @@ -133,7 +133,8 @@ RUN /usr/bin/pip3.8 install \ meson==0.63.2 \ pillow \ sphinx \ -sphinx-rtd-theme +sphinx-rtd-theme \ +tomli ENV CCACHE_WRAPPERSDIR "/usr/libexec/ccache-wrappers" ENV LANG "en_US.UTF-8" diff --git a/tests/docker/dockerfiles/debian-all-test-cross.docker b/tests/docker/dockerfiles/debian-all-test-cross.docker index f9f401544a0..54e957d5e74 100644 --- a/tests/docker/dockerfiles/debian-all-test-cross.docker +++ b/tests/docker/dockerfiles/debian-all-test-cross.docker @@ -58,7 +58,12 @@ RUN DEBIAN_FRONTEND=noninteractive eatmydata \ libc6-dev-sh4-cross \ gcc-sparc64-linux-gnu \ libc6-dev-sparc64-cross \ -python3-venv +python3-pip \ +python3-setuptools \ +python3-venv \ +
[PATCH 3/9] python: mkvenv: add ensuregroup command
Introduce a new subcommand that retrieves the packages to be installed
from a TOML file. This allows being more flexible in using the system
version of a package, while at the same time using a known-good version
when installing the package. This is important for packages that
sometimes have backwards-incompatible changes or that depend on
specific versions of their dependencies.
Compared to JSON, TOML is more human readable and easier to edit. A
parser is available in 3.11 but also available as a small (12k) package
for older versions, tomli. While tomli is bundled with pip, this is only
true of recent versions of pip. Of all the supported OSes pretty much
only FreeBSD has a recent enough version of pip while staying on Python
<3.11. So we cannot use the same trick that is in place for distlib.
Signed-off-by: Paolo Bonzini
---
python/scripts/mkvenv.py | 126 ++-
python/setup.cfg | 9 +++
pythondeps.toml | 17 ++
3 files changed, 151 insertions(+), 1 deletion(-)
create mode 100644 pythondeps.toml
diff --git a/python/scripts/mkvenv.py b/python/scripts/mkvenv.py
index 96f506d7e22..02bcd9a8c92 100644
--- a/python/scripts/mkvenv.py
+++ b/python/scripts/mkvenv.py
@@ -14,6 +14,8 @@
post_init
post-venv initialization
ensureEnsure that the specified package is installed.
+ensuregroup
+ Ensure that the specified package group is installed.
--
@@ -44,6 +46,19 @@
--onlineInstall packages from PyPI, if necessary.
--dir DIR Path to vendored packages where we may install from.
+--
+
+usage: mkvenv ensuregroup [-h] [--online] [--dir DIR] file group...
+
+positional arguments:
+ filepointer to a TOML file
+ group section name in the TOML file
+
+options:
+ -h, --help show this help message and exit
+ --onlineInstall packages from PyPI, if necessary.
+ --dir DIR Path to vendored packages where we may install from.
+
"""
# The duplication between importlib and pkg_resources does not help
@@ -99,6 +114,18 @@
except ImportError:
HAVE_DISTLIB = False
+# Try to load tomllib, with a fallback to tomli.
+# HAVE_TOMLLIB is checked below, just-in-time, so that mkvenv does not fail
+# outside the venv or before a potential call to ensurepip in checkpip().
+HAVE_TOMLLIB = True
+try:
+import tomllib
+except ImportError:
+try:
+import tomli as tomllib
+except ImportError:
+HAVE_TOMLLIB = False
+
# Do not add any mandatory dependencies from outside the stdlib:
# This script *must* be usable standalone!
@@ -837,6 +864,7 @@ def _do_ensure(
for name, info in group.items():
constraint = _make_version_constraint(info, False)
matcher = distlib.version.LegacyMatcher(name + constraint)
+print(f"mkvenv: checking for {matcher}", file=sys.stderr)
ver = _get_version(name)
if (
ver is None
@@ -898,7 +926,6 @@ def ensure(
be presented to the user. e.g., 'sphinx-build' can be used as a
bellwether for the presence of 'sphinx'.
"""
-print(f"mkvenv: checking for {', '.join(dep_specs)}", file=sys.stderr)
if not HAVE_DISTLIB:
raise Ouch("a usable distlib could not be found, please install it")
@@ -928,6 +955,64 @@ def ensure(
raise SystemExit(f"\n{result[0]}\n\n")
+def _parse_groups(file: str) -> Dict[str, Dict[str, Any]]:
+if not HAVE_TOMLLIB:
+if sys.version_info < (3, 11):
+raise Ouch("found no usable tomli, please install it")
+
+raise Ouch(
+"Python >=3.11 does not have tomllib... what have you done!?"
+)
+
+try:
+# Use loads() to support both tomli v1.2.x (Ubuntu 22.04,
+# Debian bullseye-backports) and v2.0.x
+with open(file, "r", encoding="ascii") as depfile:
+contents = depfile.read()
+return tomllib.loads(contents) # type: ignore
+except tomllib.TOMLDecodeError as exc:
+raise Ouch(f"parsing {file} failed: {exc}") from exc
+
+
+def ensure_group(
+file: str,
+groups: Sequence[str],
+online: bool = False,
+wheels_dir: Optional[Union[str, Path]] = None,
+) -> None:
+"""
+Use pip to ensure we have the package specified by @dep_specs.
+
+If the package is already installed, do nothing. If online and
+wheels_dir are both provided, prefer packages found in wheels_dir
+first before connecting to PyPI.
+
+:param dep_specs:
+PEP 508 dependency specifications. e.g. ['meson>=0.61.5'].
+:param online: If True, fall back to PyPI.
+:param wheels_dir: If specified, search this path for packages.
+"""
+
+if not HAVE_DISTLIB:
+raise Ouch("found no usable distlib, please install it")
+
+parsed_deps = _parse_groups(file)
+
+to_install: Dict[str, Dict[str,
[PATCH 5/9] configure: never use PyPI for Meson
Since there is a vendored copy, there is no point in choosing online
operation.
Signed-off-by: Paolo Bonzini
---
configure | 6 --
1 file changed, 6 deletions(-)
diff --git a/configure b/configure
index f2bd8858d6c..f13f0662b98 100755
--- a/configure
+++ b/configure
@@ -1018,13 +1018,7 @@ fi
python="$python -B"
mkvenv="$python ${source_path}/python/scripts/mkvenv.py"
-mkvenv_flags=""
-if test "$download" = "enabled" ; then
-mkvenv_flags="--online"
-fi
-
if ! $mkvenv ensure \
- $mkvenv_flags \
--dir "${source_path}/python/wheels" \
--diagnose "meson" \
"meson>=0.63.0" ;
--
2.41.0
[PATCH 7/9] configure: switch to ensuregroup
Using the new ensuregroup command, the desired versions of meson and
sphinx can be placed in pythondeps.toml rather than configure.
The meson.install entry in pythondeps.toml matches the version that is
found in python/wheels. This ensures that mkvenv.py uses the bundled
wheel even if PyPI is enabled; thus not introducing warnings or errors
from versions that are more recent than the one used in CI.
The sphinx entries match what is shipped in Fedora 38. It's the
last release that has support for older versions of Python (sphinx 6.0
requires Python 3.8) and especially docutils (of which sphinx 6.0 requires
version 0.18). This is important because Ubuntu 20.04 has docutils 0.14
and Debian 11 has docutils 0.16.
"mkvenv.py ensure" is only used to bootstrap tomli.
Signed-off-by: Paolo Bonzini
---
configure | 14 --
pythondeps.toml | 8
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/configure b/configure
index 347153702c1..e4d42d640e4 100755
--- a/configure
+++ b/configure
@@ -1024,13 +1024,8 @@ if $python -c 'import sys; sys.exit(sys.version_info >=
(3,11))'; then
$mkvenv ensure --dir "${source_path}/python/wheels" \
'tomli>=1.2.0' || exit 1
fi
-if ! $mkvenv ensure \
- --dir "${source_path}/python/wheels" \
- --diagnose "meson" \
- "meson>=0.63.0" ;
-then
-exit 1
-fi
+$mkvenv ensuregroup --dir "${source_path}/python/wheels" \
+ "${source_path}/pythondeps.toml" meson || exit 1
# At this point, we expect Meson to be installed and available.
# We expect mkvenv or pip to have created pyvenv/bin/meson for us.
@@ -1047,10 +1042,9 @@ if test "$download" = "enabled" -a "$docs" = "enabled" ;
then
fi
if test "$docs" != "disabled" ; then
-if ! $mkvenv ensure \
+if ! $mkvenv ensuregroup \
$mkvenv_flags \
- --diagnose "sphinx-build" \
- "sphinx>=1.6.0" "sphinx-rtd-theme>=0.5.0";
+ "${source_path}/pythondeps.toml" docs;
then
if test "$docs" = "enabled" ; then
exit 1
diff --git a/pythondeps.toml b/pythondeps.toml
index 362f63ff2c9..6be31dba301 100644
--- a/pythondeps.toml
+++ b/pythondeps.toml
@@ -15,3 +15,11 @@
# precise error diagnostics to the user. For example,
# 'sphinx-build' can be used as a bellwether for the
# presence of 'sphinx' in the system.
+
+[meson]
+# The install key should match the version in python/wheels/
+meson = { accepted = ">=0.63.0", installed = "0.63.3", canary = "meson" }
+
+[docs]
+sphinx = { accepted = ">=1.6", installed = "5.3.0", canary = "sphinx-build" }
+sphinx_rtd_theme = { accepted = ">=0.5", installed = "1.1.1" }
--
2.41.0
[PATCH] hw/ppc/e500: fix broken snapshot replay
ppce500_reset_device_tree is registered for system reset, but after
c4b075318eb1 this function rerandomizes rng-seed via
qemu_guest_getrandom_nofail. And when loading a snapshot, it tries to read
EVENT_RANDOM that doesn't exist, so we have an error:
qemu-system-ppc: Missing random event in the replay log
To fix this, use qemu_register_reset_nosnapshotload instead of
qemu_register_reset.
Reported-by: Vitaly Cheptsov
Fixes: c4b075318eb1 ("hw/ppc: pass random seed to fdt ")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1634
Signed-off-by: Maksim Kostin
---
hw/ppc/e500.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 67793a86f1..d5b6820d1d 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -712,7 +712,7 @@ static int ppce500_prep_device_tree(PPCE500MachineState
*machine,
p->kernel_base = kernel_base;
p->kernel_size = kernel_size;
-qemu_register_reset(ppce500_reset_device_tree, p);
+qemu_register_reset_nosnapshotload(ppce500_reset_device_tree, p);
p->notifier.notify = ppce500_init_notify;
qemu_add_machine_init_done_notifier(&p->notifier);
--
2.34.1
[PATCH v2] target/i386: Avoid cpu number overflow in legacy topology
The legacy topology enumerated by CPUID.1.EBX[23:16] is defined in SDM
Vol2:
Bits 23-16: Maximum number of addressable IDs for logical processors in
this physical package.
When launching the VM with -smp 256, the value written to EBX[23:16] is
0 because of data overflow. If the guest only supports legacy topology,
without V2 Extended Topology enumerated by CPUID.0x1f or Extended
Topology enumerated by CPUID.0x0b to support over 255 CPUs, the return
of the kernel invoking cpu_smt_allowed() is false and AP's bring-up will
fail. Then only CPU 0 is online, and others are offline.
To avoid this issue caused by overflow, limit the max value written to
EBX[23:16] to 255.
Signed-off-by: Qian Wen
---
Changes v1 -> v2:
- Revise the commit message and comment to more clearer.
- Rebased to v8.1.0-rc2.
---
target/i386/cpu.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 97ad229d8b..6e1d88fbd7 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6008,6 +6008,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
uint32_t count,
uint32_t die_offset;
uint32_t limit;
uint32_t signature[3];
+uint32_t threads_per_socket;
X86CPUTopoInfo topo_info;
topo_info.dies_per_pkg = env->nr_dies;
@@ -6049,8 +6050,19 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
uint32_t count,
*ecx |= CPUID_EXT_OSXSAVE;
}
*edx = env->features[FEAT_1_EDX];
-if (cs->nr_cores * cs->nr_threads > 1) {
-*ebx |= (cs->nr_cores * cs->nr_threads) << 16;
+/*
+ * Only bits [23:16] represent the maximum number of addressable
+ * IDs for logical processors in this physical package.
+ * When thread_per_socket > 255, it will 1) overwrite bits[31:24]
+ * which is apic_id, 2) bits [23:16] get truncated.
+ */
+threads_per_socket = cs->nr_cores * cs->nr_threads;
+if (threads_per_socket > 255) {
+threads_per_socket = 255;
+}
+
+if (threads_per_socket > 1) {
+*ebx |= threads_per_socket << 16;
*edx |= CPUID_HT;
}
if (!cpu->enable_pmu) {
--
2.25.1
[PATCH] hw/pci-host: Allow extended config space access for Designware PCIe host
In pcie_bus_realize(), a root bus is realized as a PCIe bus and a non-root bus is realized as a PCIe bus if its parent bus is a PCIe bus. However, the child bus "dw-pcie" is realized before the parent bus "pcie" which is the root PCIe bus. Thus, the extended configuration space is not accessible on "dw-pcie". The issue can be resolved by adding the PCI_BUS_EXTENDED_CONFIG_SPACE flag to "pcie" before "dw-pcie" is realized. Signed-off-by: Jason Chien --- hw/pci-host/designware.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c index 9e183caa48..388d252ee2 100644 --- a/hw/pci-host/designware.c +++ b/hw/pci-host/designware.c @@ -694,6 +694,7 @@ static void designware_pcie_host_realize(DeviceState *dev, Error **errp) &s->pci.io, 0, 4, TYPE_PCIE_BUS); +pci->bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE; memory_region_init(&s->pci.address_space_root, OBJECT(s), -- 2.17.1
Re: [PATCH 0/2] hw/nvme: two fixes
On Tue, Aug 08, 2023 at 05:16:12PM +0200, Klaus Jensen wrote: > From: Klaus Jensen > > Fix two potential accesses to null pointers. > > Klaus Jensen (2): > hw/nvme: fix null pointer access in directive receive > hw/nvme: fix null pointer access in ruh update > > hw/nvme/ctrl.c | 10 -- > 1 file changed, 8 insertions(+), 2 deletions(-) > > -- > 2.41.0 > > Reviewed-by: Jesper Wendel Devantier
Re: [PATCH for-8.1] tests/tcg: Disable filename test for info proc mappings
On Tue, 2023-08-08 at 16:49 -0700, Richard Henderson wrote:
> This test fails when host page size != guest page size,
> because qemu may not be able to directly map the file.
>
> Fixes: a6341482695 ("tests/tcg: Add a test for info proc mappings")
> Signed-off-by: Richard Henderson
> ---
> tests/tcg/multiarch/gdbstub/test-proc-mappings.py | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
> b/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
> index 7b596ac21b..5e3e5a2fb7 100644
> --- a/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
> +++ b/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
> @@ -33,7 +33,8 @@ def run_test():
> return
> raise
> report(isinstance(mappings, str), "Fetched the mappings from the
> inferior")
> - report("/sha1" in mappings, "Found the test binary name in the
> mappings")
> + # Broken with host page size > guest page size
> + # report("/sha1" in mappings, "Found the test binary name in the
> mappings")
>
>
> def main():
I'll try to find a way to check host/guest page sizes in Python for
8.2. I think it's fine to disable the check for 8.1.
Acked-by: Ilya Leoshkevich
Re: [PATCH v2 3/7] tcg/ppc: Use prefixed instructions in tcg_out_mem_long
On Tue Aug 8, 2023 at 1:02 PM AEST, Richard Henderson wrote:
> When the offset is out of range of the non-prefixed insn, but
> fits the 34-bit immediate of the prefixed insn, use that.
>
The switch will fall through in some cases (at least opi == 0).
Should it have a default: break; to make that obvious?
Reviewed-by: Nicholas Piggin
> Reviewed-by: Jordan Niethe
> Signed-off-by: Richard Henderson
> ---
> tcg/ppc/tcg-target.c.inc | 66
> 1 file changed, 66 insertions(+)
>
> diff --git a/tcg/ppc/tcg-target.c.inc b/tcg/ppc/tcg-target.c.inc
> index 2141c0bc78..61ae9d8ab7 100644
> --- a/tcg/ppc/tcg-target.c.inc
> +++ b/tcg/ppc/tcg-target.c.inc
> @@ -323,6 +323,15 @@ static bool tcg_target_const_match(int64_t val, TCGType
> type, int ct)
> #define STDX XO31(149)
> #define STQXO62( 2)
>
> +#define PLWA OPCD( 41)
> +#define PLDOPCD( 57)
> +#define PLXSD OPCD( 42)
> +#define PLXV OPCD(25 * 2 + 1) /* force tx=1 */
> +
> +#define PSTD OPCD( 61)
> +#define PSTXSD OPCD( 46)
> +#define PSTXV OPCD(27 * 2 + 1) /* force sx=1 */
> +
> #define ADDIC OPCD( 12)
> #define ADDI OPCD( 14)
> #define ADDIS OPCD( 15)
> @@ -725,6 +734,20 @@ static ptrdiff_t tcg_pcrel_diff_for_prefix(TCGContext
> *s, const void *target)
> return tcg_pcrel_diff(s, target) - (tcg_out_need_prefix_align(s) ? 4 :
> 0);
> }
>
> +/* Output Type 00 Prefix - 8-Byte Load/Store Form (8LS:D) */
> +static void tcg_out_8ls_d(TCGContext *s, tcg_insn_unit opc, unsigned rt,
> + unsigned ra, tcg_target_long imm, bool r)
> +{
> +tcg_insn_unit p, i;
> +
> +p = OPCD(1) | (r << 20) | ((imm >> 16) & 0x3);
> +i = opc | TAI(rt, ra, imm);
> +
> +tcg_out_prefix_align(s);
> +tcg_out32(s, p);
> +tcg_out32(s, i);
> +}
> +
> /* Output Type 10 Prefix - Modified Load/Store Form (MLS:D) */
> static void tcg_out_mls_d(TCGContext *s, tcg_insn_unit opc, unsigned rt,
>unsigned ra, tcg_target_long imm, bool r)
> @@ -1368,6 +1391,49 @@ static void tcg_out_mem_long(TCGContext *s, int opi,
> int opx, TCGReg rt,
> break;
> }
>
> +/* For unaligned or large offsets, use the prefixed form. */
> +if (have_isa_3_10
> +&& (offset != (int16_t)offset || (offset & align))
> +&& offset == sextract64(offset, 0, 34)) {
> +/*
> + * Note that the MLS:D insns retain their un-prefixed opcode,
> + * while the 8LS:D insns use a different opcode space.
> + */
> +switch (opi) {
> +case LBZ:
> +case LHZ:
> +case LHA:
> +case LWZ:
> +case STB:
> +case STH:
> +case STW:
> +case ADDI:
> +tcg_out_mls_d(s, opi, rt, base, offset, 0);
> +return;
> +case LWA:
> +tcg_out_8ls_d(s, PLWA, rt, base, offset, 0);
> +return;
> +case LD:
> +tcg_out_8ls_d(s, PLD, rt, base, offset, 0);
> +return;
> +case STD:
> +tcg_out_8ls_d(s, PSTD, rt, base, offset, 0);
> +return;
> +case LXSD:
> +tcg_out_8ls_d(s, PLXSD, rt & 31, base, offset, 0);
> +return;
> +case STXSD:
> +tcg_out_8ls_d(s, PSTXSD, rt & 31, base, offset, 0);
> +return;
> +case LXV:
> +tcg_out_8ls_d(s, PLXV, rt & 31, base, offset, 0);
> +return;
> +case STXV:
> +tcg_out_8ls_d(s, PSTXV, rt & 31, base, offset, 0);
> +return;
> +}
> +}
> +
> /* For unaligned, or very large offsets, use the indexed form. */
> if (offset & align || offset != (int32_t)offset || opi == 0) {
> if (rs == base) {
Re: [PATCH v2] target/i386: Avoid cpu number overflow in legacy topology
On Wed, 9 Aug 2023 18:27:32 +0800
Qian Wen wrote:
> The legacy topology enumerated by CPUID.1.EBX[23:16] is defined in SDM
> Vol2:
>
> Bits 23-16: Maximum number of addressable IDs for logical processors in
> this physical package.
>
> When launching the VM with -smp 256, the value written to EBX[23:16] is
> 0 because of data overflow. If the guest only supports legacy topology,
> without V2 Extended Topology enumerated by CPUID.0x1f or Extended
> Topology enumerated by CPUID.0x0b to support over 255 CPUs, the return
> of the kernel invoking cpu_smt_allowed() is false and AP's bring-up will
> fail. Then only CPU 0 is online, and others are offline.
>
> To avoid this issue caused by overflow, limit the max value written to
> EBX[23:16] to 255.
what happens on real hw or in lack of thereof what SDM says about this
value when there is more than 255 threads?.
> Signed-off-by: Qian Wen
> ---
> Changes v1 -> v2:
> - Revise the commit message and comment to more clearer.
> - Rebased to v8.1.0-rc2.
> ---
> target/i386/cpu.c | 16 ++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 97ad229d8b..6e1d88fbd7 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -6008,6 +6008,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
> uint32_t count,
> uint32_t die_offset;
> uint32_t limit;
> uint32_t signature[3];
> +uint32_t threads_per_socket;
> X86CPUTopoInfo topo_info;
>
> topo_info.dies_per_pkg = env->nr_dies;
> @@ -6049,8 +6050,19 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
> uint32_t count,
> *ecx |= CPUID_EXT_OSXSAVE;
> }
> *edx = env->features[FEAT_1_EDX];
> -if (cs->nr_cores * cs->nr_threads > 1) {
> -*ebx |= (cs->nr_cores * cs->nr_threads) << 16;
> +/*
> + * Only bits [23:16] represent the maximum number of addressable
> + * IDs for logical processors in this physical package.
> + * When thread_per_socket > 255, it will 1) overwrite bits[31:24]
> + * which is apic_id, 2) bits [23:16] get truncated.
> + */
> +threads_per_socket = cs->nr_cores * cs->nr_threads;
> +if (threads_per_socket > 255) {
> +threads_per_socket = 255;
> +}
> +
> +if (threads_per_socket > 1) {
> +*ebx |= threads_per_socket << 16;
> *edx |= CPUID_HT;
> }
> if (!cpu->enable_pmu) {
Re: [PATCH v2 4/7] tcg/ppc: Use PLD in tcg_out_movi for constant pool
On Tue Aug 8, 2023 at 1:02 PM AEST, Richard Henderson wrote:
> The prefixed instruction has a pc-relative form to use here.
I don't understand this code very well but going by existing
relocs it looks okay.
Reviewed-by: Nicholas Piggin
>
> Signed-off-by: Richard Henderson
> ---
> tcg/ppc/tcg-target.c.inc | 24
> 1 file changed, 24 insertions(+)
>
> diff --git a/tcg/ppc/tcg-target.c.inc b/tcg/ppc/tcg-target.c.inc
> index 61ae9d8ab7..b3b2e9874d 100644
> --- a/tcg/ppc/tcg-target.c.inc
> +++ b/tcg/ppc/tcg-target.c.inc
> @@ -101,6 +101,10 @@
> #define ALL_GENERAL_REGS 0xu
> #define ALL_VECTOR_REGS 0xull
>
> +#ifndef R_PPC64_PCREL34
> +#define R_PPC64_PCREL34 132
> +#endif
> +
> #define have_isel (cpuinfo & CPUINFO_ISEL)
>
> #ifndef CONFIG_SOFTMMU
> @@ -260,6 +264,19 @@ static bool reloc_pc14(tcg_insn_unit *src_rw, const
> tcg_insn_unit *target)
> return false;
> }
>
> +static bool reloc_pc34(tcg_insn_unit *src_rw, const tcg_insn_unit *target)
> +{
> +const tcg_insn_unit *src_rx = tcg_splitwx_to_rx(src_rw);
> +ptrdiff_t disp = tcg_ptr_byte_diff(target, src_rx);
> +
> +if (disp == sextract64(disp, 0, 34)) {
> +src_rw[0] = (src_rw[0] & ~0x3) | ((disp >> 16) & 0x3);
> +src_rw[1] = (src_rw[1] & ~0x) | (disp & 0x);
> +return true;
> +}
> +return false;
> +}
> +
> /* test if a constant matches the constraint */
> static bool tcg_target_const_match(int64_t val, TCGType type, int ct)
> {
> @@ -684,6 +701,8 @@ static bool patch_reloc(tcg_insn_unit *code_ptr, int type,
> return reloc_pc14(code_ptr, target);
> case R_PPC_REL24:
> return reloc_pc24(code_ptr, target);
> +case R_PPC64_PCREL34:
> +return reloc_pc34(code_ptr, target);
> case R_PPC_ADDR16:
> /*
> * We are (slightly) abusing this relocation type. In particular,
> @@ -,6 +1130,11 @@ static void tcg_out_movi_int(TCGContext *s, TCGType
> type, TCGReg ret,
> }
>
> /* Use the constant pool, if possible. */
> +if (have_isa_3_10) {
> +tcg_out_8ls_d(s, PLD, ret, 0, 0, 1);
> +new_pool_label(s, arg, R_PPC64_PCREL34, s->code_ptr - 2, 0);
> +return;
> +}
> if (!in_prologue && USE_REG_TB) {
> new_pool_label(s, arg, R_PPC_ADDR16, s->code_ptr,
> tcg_tbrel_diff(s, NULL));
Re: [PATCH v2 6/7] tcg/ppc: Disable USE_REG_TB for Power v3.1
On Tue Aug 8, 2023 at 1:02 PM AEST, Richard Henderson wrote: > With Power v3.1, we have pc-relative addressing and so > do not require a register holding the current TB. > Acked-by: Nicholas Piggin > Signed-off-by: Richard Henderson > --- > tcg/ppc/tcg-target.c.inc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tcg/ppc/tcg-target.c.inc b/tcg/ppc/tcg-target.c.inc > index 01ca5c9f39..63fe4ef995 100644 > --- a/tcg/ppc/tcg-target.c.inc > +++ b/tcg/ppc/tcg-target.c.inc > @@ -83,7 +83,7 @@ > #define TCG_VEC_TMP2TCG_REG_V1 > > #define TCG_REG_TB TCG_REG_R31 > -#define USE_REG_TB (TCG_TARGET_REG_BITS == 64) > +#define USE_REG_TB (TCG_TARGET_REG_BITS == 64 && !have_isa_3_10) > > /* Shorthand for size of a pointer. Avoid promotion to unsigned. */ > #define SZP ((int)sizeof(void *))
Re: [PATCH v2 7/7] tcg/ppc: Use prefixed instructions for tcg_out_goto_tb
On Tue Aug 8, 2023 at 1:02 PM AEST, Richard Henderson wrote:
> When a direct branch is out of range, we can load the destination for
> the indirect branch using PLA (for 16GB worth of buffer) and PLD from
> the TranslationBlock for everything larger.
>
> This means the patch affects exactly one instruction: B (plus filler),
> PLA or PLD. Which means we can update and execute the patch atomically.
>
Aside from changelog that Jordan pointed out,
Reviewed-by: Nicholas Piggin
> Signed-off-by: Richard Henderson
> ---
> tcg/ppc/tcg-target.c.inc | 31 +++
> 1 file changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/tcg/ppc/tcg-target.c.inc b/tcg/ppc/tcg-target.c.inc
> index 63fe4ef995..b686a68247 100644
> --- a/tcg/ppc/tcg-target.c.inc
> +++ b/tcg/ppc/tcg-target.c.inc
> @@ -2646,31 +2646,38 @@ static void tcg_out_goto_tb(TCGContext *s, int which)
> uintptr_t ptr = get_jmp_target_addr(s, which);
>
> if (USE_REG_TB) {
> +/*
> + * With REG_TB, we must always use indirect branching,
> + * so that the branch destination and TCG_REG_TB match.
> + */
> ptrdiff_t offset = tcg_tbrel_diff(s, (void *)ptr);
> tcg_out_mem_long(s, LD, LDX, TCG_REG_TB, TCG_REG_TB, offset);
> -
> -/* TODO: Use direct branches when possible. */
> -set_jmp_insn_offset(s, which);
> tcg_out32(s, MTSPR | RS(TCG_REG_TB) | CTR);
> -
> tcg_out32(s, BCCTR | BO_ALWAYS);
>
> /* For the unlinked case, need to reset TCG_REG_TB. */
> set_jmp_reset_offset(s, which);
> tcg_out_mem_long(s, ADDI, ADD, TCG_REG_TB, TCG_REG_TB,
> -tcg_current_code_size(s));
> -} else {
> -/* Direct branch will be patched by tb_target_set_jmp_target. */
> -set_jmp_insn_offset(s, which);
> -tcg_out32(s, NOP);
> +return;
> +}
>
> -/* When branch is out of range, fall through to indirect. */
> +/* Direct branch will be patched by tb_target_set_jmp_target. */
> +set_jmp_insn_offset(s, which);
> +tcg_out32(s, NOP);
> +
> +/* When branch is out of range, fall through to indirect. */
> +if (have_isa_3_10) {
> +ptrdiff_t offset = tcg_pcrel_diff_for_prefix(s, (void *)ptr);
> +tcg_out_8ls_d(s, PLD, TCG_REG_TMP1, 0, offset, 1);
> +} else {
> tcg_out_movi(s, TCG_TYPE_PTR, TCG_REG_TMP1, ptr - (int16_t)ptr);
> tcg_out_ld(s, TCG_TYPE_PTR, TCG_REG_TMP1, TCG_REG_TMP1,
> (int16_t)ptr);
> -tcg_out32(s, MTSPR | RS(TCG_REG_TMP1) | CTR);
> -tcg_out32(s, BCCTR | BO_ALWAYS);
> -set_jmp_reset_offset(s, which);
> }
> +
> +tcg_out32(s, MTSPR | RS(TCG_REG_TMP1) | CTR);
> +tcg_out32(s, BCCTR | BO_ALWAYS);
> +set_jmp_reset_offset(s, which);
> }
>
> void tb_target_set_jmp_target(const TranslationBlock *tb, int n,
Re: [PATCH] vfio/pci: hide ROM BAR on SFC9220 10/40G Ethernet Controller PF
On 8/8/23 17:40, Alex Williamson wrote:
> On Tue, 8 Aug 2023 16:59:16 +0200
> Laszlo Ersek wrote:
>
>> The Solarflare Communications SFC9220 NIC's physical function (PF) appears
>> to expose an expansion ROM with the following characteristics:
>>
>> (1) Single-image ROM, with only a legacy BIOS image (no UEFI driver).
>> Alex's rom-parser utility dumps it like this:
>>
>>> Valid ROM signature found @0h, PCIR offset 20h
>>> PCIR: type 0 (x86 PC-AT), vendor: 1924, device: 0a03, class: 02
>>> PCIR: revision 3, vendor revision: 1
>>> Last image
>>
>> (2) The BIOS image crashes when booted on i440fx.
>>
>> (3) The BIOS image prints the following messages on q35:
>>
>>> Solarflare Boot Manager (v5.2.2.1006)
>>> Solarflare Communications 2008-2019
>>> gPXE (http://etherboot.org) - [...] PCI[...] PnP PMM[...]
>>
>> So it appears like a modified derivative of old gPXE.
>>
>> Alex surmised in advance that the BIOS image could be accessing
>> host-physical addresses rather than guest-phys ones, leading to the crash
>> on i440fx.
>
> ROMs sometimes take shortcuts around the standard interfaces to the
> device and can therefore hit gaps in the virtualization, which is why
> that's suspect to me. However if it works on q35 but not 440fx it
> might be more that we're not matching a PCI topology expectation of the
> ROM. Was it only tested on 440fx attached to the root bus or does it
> also fail if the PF is attached downstream of a PCI-to-PCI bridge?
Turns out the oprom wants the NIC to have PCI device number 0,
regardless of the bus number, and regardless of the bus's location in
the PCI topology.
Please drop this patch; I've documented the workaround in the BZ for now
(which I've also opened up to the public).
We should probably find a more visible place for the documentation, though.
Thanks for pointing me in the right direction!
Laszlo
>
>> Don't expose the option ROM BAR to the VM by default. While this prevents
>> netbooting the VM off the PF on q35/SeaBIOS (a relatively rare scenario),
>> it does not make any difference for UEFI, and at least the VM doesn't
>> crash during boot on i440fx/SeaBIOS (a relatively frequent scenario).
>> Users can restore the original behavior via the QEMU cmdline and the
>> libvirt domain XML.
>>
>> (In two years, we've not seen any customer interest in this bug, hence
>> there's no incentive to investigate (2).)
>>
>> Cc: Alex Williamson (supporter:VFIO)
>> Cc: "Cédric Le Goater" (supporter:VFIO)
>> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1975776
>> Signed-off-by: Laszlo Ersek
>> ---
>> hw/vfio/pci-quirks.c | 4
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
>> index f4ff83680572..270eb16b91fa 100644
>> --- a/hw/vfio/pci-quirks.c
>> +++ b/hw/vfio/pci-quirks.c
>> @@ -45,6 +45,10 @@ static const struct {
>> uint32_t device;
>> } rom_denylist[] = {
>> { 0x14e4, 0x168e }, /* Broadcom BCM 57810 */
>> +{ 0x1924, 0x0a03 }, /* Solarflare Communications
>> + * SFC9220 10/40G Ethernet Controller
>> + *
>> https://bugzilla.redhat.com/show_bug.cgi?id=1975776
>
> Unfortunately this is not a public bz so there's not much point in
> referencing it in public code or commit log :-\ Thanks,
>
> Alex
>
>> + */
>> };
>>
>> bool vfio_opt_rom_in_denylist(VFIOPCIDevice *vdev)
>
Re: [RFC v1 0/3] Initial support for SPDM
On Tue, 8 Aug 2023 11:51:21 -0400 Alistair Francis wrote: > The Security Protocol and Data Model (SPDM) Specification defines > messages, data objects, and sequences for performing message exchanges > over a variety of transport and physical media. > - > https://www.dmtf.org/sites/default/files/standards/documents/DSP0274_1.3.0.pdf > > This series is a very initial start on adding SPDM support to QEMU. > > This series uses libspdm [1] which is a reference implementation of > SPDM. > > The series starts by adding support for building and linking with > libspdm. It then progresses to handling SPDM requests in the NVMe device > over the PCIe DOE protocol. > > This is a very early attempt. The code quality is not super high, the C > code hasn't been tested at all. This is really just an RFC to see if > QEMU will accept linking with libspdm. > > So, the main question is, how do people feel about linking with libspdm > to support SPDM? > > 1: https://github.com/DMTF/libspdm Hi Alastair, For reference / background we've had SPDM support for CXL emulated devices in our staging tree for quite a while - it's not upstream because of exactly this question (+ no one had upstreaming this as a high priority as out of tree was fine for developing the kernel stack - it's well isolated in the code so there was little cost in rebasing this feature) - and because libspdm is packaged by almost no one. There were problems building it with external crypto libraries etc. Looks like you have had a lot more success than I ever did in getting that to work. I tried a few times in the past and ended up sticking with the Avery design folks approach of a socket connection to spdm-emu Given you cc'd them I'm guessing you came across that work which is what we used for testing the kernel code Lukas is working on currently. https://lore.kernel.org/qemu-devel/20210629132520.0...@huawei.com/ https://gitlab.com/jic23/qemu/-/commit/9864fb29979e55c1e37c20edf00907d9524036e8 So I think we have 2 choices here. 1) Do what you have done and build the library as you are doing. - Can fix a version - so don't care if they change the ABI etc other than needing to move the version QEMU uses forwards when we need to for new features. - Cert management is going to add a lot of complexity into QEMU. I've not looked at how much infrastructure we can reuse from elsewhere. Maybe this is a solved problem. 2) Keep the SPDM emulation external. - I'm not sure the socket protocol used by spdm-emu is fixed in stone as people tend to change both ends. - Cert management and protocol options etc are already available. Personally I prefer the control option 1 gives us, even if it's a lot more code. Option 2 also rather limits our ability to do anything with the encrypted data as well. It's fine if the aim is just verification of simple flows, but if we need to inject particular measurements etc it doesn't really work. Jonathan > > Alistair Francis (3): > subprojects: libspdm: Initial support > hw: nvme: ctrl: Initial support for DOE > hw: nvme: ctrl: Process SPDM requests > > meson.build | 78 +++ > hw/nvme/nvme.h| 4 ++ > include/hw/pci/pcie_doe.h | 1 + > hw/nvme/ctrl.c| 35 > .gitmodules | 3 ++ > meson_options.txt | 3 ++ > scripts/meson-buildoptions.sh | 3 ++ > subprojects/.gitignore| 1 + > subprojects/libspdm.wrap | 5 +++ > 9 files changed, 133 insertions(+) > create mode 100644 subprojects/libspdm.wrap >
[PATCH v3 1/6] target/arm/ptw: Load stage-2 tables from realm physical space
In realm state, stage-2 translation tables are fetched from the realm
physical address space (R_PGRQD).
Signed-off-by: Jean-Philippe Brucker
Reviewed-by: Peter Maydell
---
target/arm/ptw.c | 26 ++
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index d1de934702..063adbd84a 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -157,22 +157,32 @@ static ARMMMUIdx ptw_idx_for_stage_2(CPUARMState *env,
ARMMMUIdx stage2idx)
/*
* We're OK to check the current state of the CPU here because
- * (1) we always invalidate all TLBs when the SCR_EL3.NS bit changes
+ * (1) we always invalidate all TLBs when the SCR_EL3.NS or SCR_EL3.NSE bit
+ * changes.
* (2) there's no way to do a lookup that cares about Stage 2 for a
* different security state to the current one for AArch64, and AArch32
* never has a secure EL2. (AArch32 ATS12NSO[UP][RW] allow EL3 to do
* an NS stage 1+2 lookup while the NS bit is 0.)
*/
-if (!arm_is_secure_below_el3(env) || !arm_el_is_aa64(env, 3)) {
+if (!arm_el_is_aa64(env, 3)) {
return ARMMMUIdx_Phys_NS;
}
-if (stage2idx == ARMMMUIdx_Stage2_S) {
-s2walk_secure = !(env->cp15.vstcr_el2 & VSTCR_SW);
-} else {
-s2walk_secure = !(env->cp15.vtcr_el2 & VTCR_NSW);
-}
-return s2walk_secure ? ARMMMUIdx_Phys_S : ARMMMUIdx_Phys_NS;
+switch (arm_security_space_below_el3(env)) {
+case ARMSS_NonSecure:
+return ARMMMUIdx_Phys_NS;
+case ARMSS_Realm:
+return ARMMMUIdx_Phys_Realm;
+case ARMSS_Secure:
+if (stage2idx == ARMMMUIdx_Stage2_S) {
+s2walk_secure = !(env->cp15.vstcr_el2 & VSTCR_SW);
+} else {
+s2walk_secure = !(env->cp15.vtcr_el2 & VTCR_NSW);
+}
+return s2walk_secure ? ARMMMUIdx_Phys_S : ARMMMUIdx_Phys_NS;
+default:
+g_assert_not_reached();
+}
}
static bool regime_translation_big_endian(CPUARMState *env, ARMMMUIdx mmu_idx)
--
2.41.0
[PATCH v3 4/6] target/arm: Pass security space rather than flag for AT instructions
At the moment we only handle Secure and Nonsecure security spaces for
the AT instructions. Add support for Realm and Root.
For AArch64, arm_security_space() gives the desired space. ARM DDI0487J
says (R_NYXTL):
If EL3 is implemented, then when an address translation instruction
that applies to an Exception level lower than EL3 is executed, the
Effective value of SCR_EL3.{NSE, NS} determines the target Security
state that the instruction applies to.
For AArch32, some instructions can access NonSecure space from Secure,
so we still need to pass the state explicitly to do_ats_write().
Signed-off-by: Jean-Philippe Brucker
Reviewed-by: Peter Maydell
---
target/arm/internals.h | 18 +-
target/arm/helper.c| 27 ---
target/arm/ptw.c | 12 ++--
3 files changed, 27 insertions(+), 30 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index fc90c364f7..cf13bb94f5 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1217,24 +1217,24 @@ bool get_phys_addr(CPUARMState *env, target_ulong
address,
__attribute__((nonnull));
/**
- * get_phys_addr_with_secure_nogpc: get the physical address for a virtual
- * address
+ * get_phys_addr_with_space_nogpc: get the physical address for a virtual
+ * address
* @env: CPUARMState
* @address: virtual address to get physical address for
* @access_type: 0 for read, 1 for write, 2 for execute
* @mmu_idx: MMU index indicating required translation regime
- * @is_secure: security state for the access
+ * @space: security space for the access
* @result: set on translation success.
* @fi: set to fault info if the translation fails
*
- * Similar to get_phys_addr, but use the given security regime and don't
perform
+ * Similar to get_phys_addr, but use the given security space and don't perform
* a Granule Protection Check on the resulting address.
*/
-bool get_phys_addr_with_secure_nogpc(CPUARMState *env, target_ulong address,
- MMUAccessType access_type,
- ARMMMUIdx mmu_idx, bool is_secure,
- GetPhysAddrResult *result,
- ARMMMUFaultInfo *fi)
+bool get_phys_addr_with_space_nogpc(CPUARMState *env, target_ulong address,
+MMUAccessType access_type,
+ARMMMUIdx mmu_idx, ARMSecuritySpace space,
+GetPhysAddrResult *result,
+ARMMMUFaultInfo *fi)
__attribute__((nonnull));
bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address,
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 427de6bd2a..fbb03c364b 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3357,7 +3357,7 @@ static int par_el1_shareability(GetPhysAddrResult *res)
static uint64_t do_ats_write(CPUARMState *env, uint64_t value,
MMUAccessType access_type, ARMMMUIdx mmu_idx,
- bool is_secure)
+ ARMSecuritySpace ss)
{
bool ret;
uint64_t par64;
@@ -3369,8 +3369,8 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t
value,
* I_MXTJT: Granule protection checks are not performed on the final
address
* of a successful translation.
*/
-ret = get_phys_addr_with_secure_nogpc(env, value, access_type, mmu_idx,
- is_secure, &res, &fi);
+ret = get_phys_addr_with_space_nogpc(env, value, access_type, mmu_idx, ss,
+ &res, &fi);
/*
* ATS operations only do S1 or S1+S2 translations, so we never
@@ -3535,7 +3535,7 @@ static void ats_write(CPUARMState *env, const
ARMCPRegInfo *ri, uint64_t value)
uint64_t par64;
ARMMMUIdx mmu_idx;
int el = arm_current_el(env);
-bool secure = arm_is_secure_below_el3(env);
+ARMSecuritySpace ss = arm_security_space(env);
switch (ri->opc2 & 6) {
case 0:
@@ -3543,10 +3543,9 @@ static void ats_write(CPUARMState *env, const
ARMCPRegInfo *ri, uint64_t value)
switch (el) {
case 3:
mmu_idx = ARMMMUIdx_E3;
-secure = true;
break;
case 2:
-g_assert(!secure); /* ARMv8.4-SecEL2 is 64-bit only */
+g_assert(ss != ARMSS_Secure); /* ARMv8.4-SecEL2 is 64-bit only */
/* fall through */
case 1:
if (ri->crm == 9 && (env->uncached_cpsr & CPSR_PAN)) {
@@ -3564,10 +3563,9 @@ static void ats_write(CPUARMState *env, const
ARMCPRegInfo *ri, uint64_t value)
switch (el) {
case 3:
mmu_idx = ARMMMUIdx_E10_0;
-secure = true;
break;
case 2:
-g_assert(!secure); /* ARMv8.4-SecEL2 is 64-bit only
[PATCH v3 3/6] target/arm: Skip granule protection checks for AT instructions
GPC checks are not performed on the output address for AT instructions,
as stated by ARM DDI 0487J in D8.12.2:
When populating PAR_EL1 with the result of an address translation
instruction, granule protection checks are not performed on the final
output address of a successful translation.
Rename get_phys_addr_with_secure(), since it's only used to handle AT
instructions.
Signed-off-by: Jean-Philippe Brucker
Reviewed-by: Peter Maydell
---
target/arm/internals.h | 25 ++---
target/arm/helper.c| 8 ++--
target/arm/ptw.c | 11 ++-
3 files changed, 26 insertions(+), 18 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 0f01bc32a8..fc90c364f7 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1190,12 +1190,11 @@ typedef struct GetPhysAddrResult {
} GetPhysAddrResult;
/**
- * get_phys_addr_with_secure: get the physical address for a virtual address
+ * get_phys_addr: get the physical address for a virtual address
* @env: CPUARMState
* @address: virtual address to get physical address for
* @access_type: 0 for read, 1 for write, 2 for execute
* @mmu_idx: MMU index indicating required translation regime
- * @is_secure: security state for the access
* @result: set on translation success.
* @fi: set to fault info if the translation fails
*
@@ -1212,26 +1211,30 @@ typedef struct GetPhysAddrResult {
* * for PSMAv5 based systems we don't bother to return a full FSR format
*value.
*/
-bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address,
- MMUAccessType access_type,
- ARMMMUIdx mmu_idx, bool is_secure,
- GetPhysAddrResult *result, ARMMMUFaultInfo *fi)
+bool get_phys_addr(CPUARMState *env, target_ulong address,
+ MMUAccessType access_type, ARMMMUIdx mmu_idx,
+ GetPhysAddrResult *result, ARMMMUFaultInfo *fi)
__attribute__((nonnull));
/**
- * get_phys_addr: get the physical address for a virtual address
+ * get_phys_addr_with_secure_nogpc: get the physical address for a virtual
+ * address
* @env: CPUARMState
* @address: virtual address to get physical address for
* @access_type: 0 for read, 1 for write, 2 for execute
* @mmu_idx: MMU index indicating required translation regime
+ * @is_secure: security state for the access
* @result: set on translation success.
* @fi: set to fault info if the translation fails
*
- * Similarly, but use the security regime of @mmu_idx.
+ * Similar to get_phys_addr, but use the given security regime and don't
perform
+ * a Granule Protection Check on the resulting address.
*/
-bool get_phys_addr(CPUARMState *env, target_ulong address,
- MMUAccessType access_type, ARMMMUIdx mmu_idx,
- GetPhysAddrResult *result, ARMMMUFaultInfo *fi)
+bool get_phys_addr_with_secure_nogpc(CPUARMState *env, target_ulong address,
+ MMUAccessType access_type,
+ ARMMMUIdx mmu_idx, bool is_secure,
+ GetPhysAddrResult *result,
+ ARMMMUFaultInfo *fi)
__attribute__((nonnull));
bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address,
diff --git a/target/arm/helper.c b/target/arm/helper.c
index a4c2c1bde5..427de6bd2a 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3365,8 +3365,12 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t
value,
ARMMMUFaultInfo fi = {};
GetPhysAddrResult res = {};
-ret = get_phys_addr_with_secure(env, value, access_type, mmu_idx,
-is_secure, &res, &fi);
+/*
+ * I_MXTJT: Granule protection checks are not performed on the final
address
+ * of a successful translation.
+ */
+ret = get_phys_addr_with_secure_nogpc(env, value, access_type, mmu_idx,
+ is_secure, &res, &fi);
/*
* ATS operations only do S1 or S1+S2 translations, so we never
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 063adbd84a..33179f3471 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -3418,16 +3418,17 @@ static bool get_phys_addr_gpc(CPUARMState *env,
S1Translate *ptw,
return false;
}
-bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address,
- MMUAccessType access_type, ARMMMUIdx mmu_idx,
- bool is_secure, GetPhysAddrResult *result,
- ARMMMUFaultInfo *fi)
+bool get_phys_addr_with_secure_nogpc(CPUARMState *env, target_ulong address,
+ MMUAccessType access_type,
+ ARMMMUIdx mmu_idx, bool is_secure,
+ GetPhysAddrResult *result,
+
[PATCH v3 6/6] target/arm/helper: Implement CNTHCTL_EL2.CNT[VP]MASK
When FEAT_RME is implemented, these bits override the value of
CNT[VP]_CTL_EL0.IMASK in Realm and Root state. Move the IRQ state update
into a new gt_update_irq() function and test those bits every time we
recompute the IRQ state.
Since we're removing the IRQ state from some trace events, add a new
trace event for gt_update_irq().
Signed-off-by: Jean-Philippe Brucker
---
target/arm/cpu.h| 4 +++
target/arm/cpu.c| 4 +++
target/arm/helper.c | 65 ++---
target/arm/trace-events | 7 +++--
4 files changed, 66 insertions(+), 14 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index bcd65a63ca..855a76ae81 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1115,6 +1115,7 @@ struct ArchCPU {
};
unsigned int gt_cntfrq_period_ns(ARMCPU *cpu);
+void gt_rme_post_el_change(ARMCPU *cpu, void *opaque);
void arm_cpu_post_init(Object *obj);
@@ -1743,6 +1744,9 @@ static inline void xpsr_write(CPUARMState *env, uint32_t
val, uint32_t mask)
#define HSTR_TTEE (1 << 16)
#define HSTR_TJDBX (1 << 17)
+#define CNTHCTL_CNTVMASK (1 << 18)
+#define CNTHCTL_CNTPMASK (1 << 19)
+
/* Return the current FPSCR value. */
uint32_t vfp_get_fpscr(CPUARMState *env);
void vfp_set_fpscr(CPUARMState *env, uint32_t val);
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 93c28d50e5..7df1f7600b 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2169,6 +2169,10 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
**errp)
set_feature(env, ARM_FEATURE_VBAR);
}
+if (cpu_isar_feature(aa64_rme, cpu)) {
+arm_register_el_change_hook(cpu, >_rme_post_el_change, 0);
+}
+
register_cp_regs_for_features(cpu);
arm_cpu_register_gdb_regs_for_features(cpu);
diff --git a/target/arm/helper.c b/target/arm/helper.c
index dbfe9f2f5e..86ce6a52bb 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2608,6 +2608,39 @@ static uint64_t gt_get_countervalue(CPUARMState *env)
return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) / gt_cntfrq_period_ns(cpu);
}
+static void gt_update_irq(ARMCPU *cpu, int timeridx)
+{
+CPUARMState *env = &cpu->env;
+uint64_t cnthctl = env->cp15.cnthctl_el2;
+ARMSecuritySpace ss = arm_security_space(env);
+/* ISTATUS && !IMASK */
+int irqstate = (env->cp15.c14_timer[timeridx].ctl & 6) == 4;
+
+/*
+ * If bit CNTHCTL_EL2.CNT[VP]MASK is set, it overrides IMASK.
+ * It is RES0 in Secure and NonSecure state.
+ */
+if ((ss == ARMSS_Root || ss == ARMSS_Realm) &&
+((timeridx == GTIMER_VIRT && (cnthctl & CNTHCTL_CNTVMASK)) ||
+ (timeridx == GTIMER_PHYS && (cnthctl & CNTHCTL_CNTPMASK {
+irqstate = 0;
+}
+
+qemu_set_irq(cpu->gt_timer_outputs[timeridx], irqstate);
+trace_arm_gt_update_irq(timeridx, irqstate);
+}
+
+void gt_rme_post_el_change(ARMCPU *cpu, void *ignored)
+{
+/*
+ * Changing security state between Root and Secure/NonSecure, which may
+ * happen when switching EL, can change the effective value of CNTHCTL_EL2
+ * mask bits. Update the IRQ state accordingly.
+ */
+gt_update_irq(cpu, GTIMER_VIRT);
+gt_update_irq(cpu, GTIMER_PHYS);
+}
+
static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
{
ARMGenericTimer *gt = &cpu->env.cp15.c14_timer[timeridx];
@@ -2623,13 +2656,9 @@ static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
/* Note that this must be unsigned 64 bit arithmetic: */
int istatus = count - offset >= gt->cval;
uint64_t nexttick;
-int irqstate;
gt->ctl = deposit32(gt->ctl, 2, 1, istatus);
-irqstate = (istatus && !(gt->ctl & 2));
-qemu_set_irq(cpu->gt_timer_outputs[timeridx], irqstate);
-
if (istatus) {
/* Next transition is when count rolls back over to zero */
nexttick = UINT64_MAX;
@@ -2648,14 +2677,14 @@ static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
} else {
timer_mod(cpu->gt_timer[timeridx], nexttick);
}
-trace_arm_gt_recalc(timeridx, irqstate, nexttick);
+trace_arm_gt_recalc(timeridx, nexttick);
} else {
/* Timer disabled: ISTATUS and timer output always clear */
gt->ctl &= ~4;
-qemu_set_irq(cpu->gt_timer_outputs[timeridx], 0);
timer_del(cpu->gt_timer[timeridx]);
trace_arm_gt_recalc_disabled(timeridx);
}
+gt_update_irq(cpu, timeridx);
}
static void gt_timer_reset(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -2759,10 +2788,8 @@ static void gt_ctl_write(CPUARMState *env, const
ARMCPRegInfo *ri,
* IMASK toggled: don't need to recalculate,
* just set the interrupt line based on ISTATUS
*/
-int irqstate = (oldval & 4) && !(value & 2);
-
-trace_arm_gt_imask_toggle(timeridx, irqstate);
-qemu_set_irq(cpu->gt_timer_outputs[timeridx], irqstate);
+trace_arm_gt_imask_toggle(timeridx);
[PATCH v3 0/6] target/arm: Fixes for RME
A few patches to fix RME support and allow booting a realm guest, based
on "[PATCH v2 00/15] target/arm/ptw: Cleanups and a few bugfixes"
https://lore.kernel.org/all/20230807141514.19075-1-peter.mayd...@linaro.org/
Since v2:
* Updated the comment in patch 5. I also removed the check for FEAT_RME,
because as pointed out in "target/arm: Catch illegal-exception-return
from EL3 with bad NSE/NS", the SCR_NSE bit can only be set with
FEAT_RME enabled. Because of this additional change, I didn't add the
Reviewed-by.
* Added an EL-change hook to patch 6, to update the timer IRQ
when changing the security state. I was wondering whether the
el_change function should filter security state changes, since we only
need to update IRQ state when switching between Root and
Secure/NonSecure. But with a small syscall benchmark exercising
EL0-EL1 switch with FEAT_RME enabled, I couldn't see any difference
with and without the el_change hook, so I kept it simple.
* Also added the .raw_write callback for CNTHCTL_EL2.
v2:
https://lore.kernel.org/all/20230802170157.401491-1-jean-phili...@linaro.org/
Jean-Philippe Brucker (6):
target/arm/ptw: Load stage-2 tables from realm physical space
target/arm/helper: Fix tlbmask and tlbbits for TLBI VAE2*
target/arm: Skip granule protection checks for AT instructions
target/arm: Pass security space rather than flag for AT instructions
target/arm/helper: Check SCR_EL3.{NSE,NS} encoding for AT instructions
target/arm/helper: Implement CNTHCTL_EL2.CNT[VP]MASK
target/arm/cpu.h| 4 +
target/arm/internals.h | 25 +++---
target/arm/cpu.c| 4 +
target/arm/helper.c | 184 ++--
target/arm/ptw.c| 39 ++---
target/arm/trace-events | 7 +-
6 files changed, 188 insertions(+), 75 deletions(-)
--
2.41.0
[PATCH v3 2/6] target/arm/helper: Fix tlbmask and tlbbits for TLBI VAE2*
When HCR_EL2.E2H is enabled, TLB entries are formed using the EL2&0
translation regime, instead of the EL2 translation regime. The TLB VAE2*
instructions invalidate the regime that corresponds to the current value
of HCR_EL2.E2H.
At the moment we only invalidate the EL2 translation regime. This causes
problems with RMM, which issues TLBI VAE2IS instructions with
HCR_EL2.E2H enabled. Update vae2_tlbmask() to take HCR_EL2.E2H into
account.
Add vae2_tlbbits() as well, since the top-byte-ignore configuration is
different between the EL2&0 and EL2 regime.
Signed-off-by: Jean-Philippe Brucker
Reviewed-by: Peter Maydell
---
target/arm/helper.c | 50 -
1 file changed, 40 insertions(+), 10 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 2959d27543..a4c2c1bde5 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -4663,6 +4663,21 @@ static int vae1_tlbmask(CPUARMState *env)
return mask;
}
+static int vae2_tlbmask(CPUARMState *env)
+{
+uint64_t hcr = arm_hcr_el2_eff(env);
+uint16_t mask;
+
+if (hcr & HCR_E2H) {
+mask = ARMMMUIdxBit_E20_2 |
+ ARMMMUIdxBit_E20_2_PAN |
+ ARMMMUIdxBit_E20_0;
+} else {
+mask = ARMMMUIdxBit_E2;
+}
+return mask;
+}
+
/* Return 56 if TBI is enabled, 64 otherwise. */
static int tlbbits_for_regime(CPUARMState *env, ARMMMUIdx mmu_idx,
uint64_t addr)
@@ -4689,6 +4704,25 @@ static int vae1_tlbbits(CPUARMState *env, uint64_t addr)
return tlbbits_for_regime(env, mmu_idx, addr);
}
+static int vae2_tlbbits(CPUARMState *env, uint64_t addr)
+{
+uint64_t hcr = arm_hcr_el2_eff(env);
+ARMMMUIdx mmu_idx;
+
+/*
+ * Only the regime of the mmu_idx below is significant.
+ * Regime EL2&0 has two ranges with separate TBI configuration, while EL2
+ * only has one.
+ */
+if (hcr & HCR_E2H) {
+mmu_idx = ARMMMUIdx_E20_2;
+} else {
+mmu_idx = ARMMMUIdx_E2;
+}
+
+return tlbbits_for_regime(env, mmu_idx, addr);
+}
+
static void tlbi_aa64_vmalle1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
@@ -4781,10 +4815,11 @@ static void tlbi_aa64_vae2_write(CPUARMState *env,
const ARMCPRegInfo *ri,
* flush-last-level-only.
*/
CPUState *cs = env_cpu(env);
-int mask = e2_tlbmask(env);
+int mask = vae2_tlbmask(env);
uint64_t pageaddr = sextract64(value << 12, 0, 56);
+int bits = vae2_tlbbits(env, pageaddr);
-tlb_flush_page_by_mmuidx(cs, pageaddr, mask);
+tlb_flush_page_bits_by_mmuidx(cs, pageaddr, mask, bits);
}
static void tlbi_aa64_vae3_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -4838,11 +4873,11 @@ static void tlbi_aa64_vae2is_write(CPUARMState *env,
const ARMCPRegInfo *ri,
uint64_t value)
{
CPUState *cs = env_cpu(env);
+int mask = vae2_tlbmask(env);
uint64_t pageaddr = sextract64(value << 12, 0, 56);
-int bits = tlbbits_for_regime(env, ARMMMUIdx_E2, pageaddr);
+int bits = vae2_tlbbits(env, pageaddr);
-tlb_flush_page_bits_by_mmuidx_all_cpus_synced(cs, pageaddr,
- ARMMMUIdxBit_E2, bits);
+tlb_flush_page_bits_by_mmuidx_all_cpus_synced(cs, pageaddr, mask, bits);
}
static void tlbi_aa64_vae3is_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -5014,11 +5049,6 @@ static void tlbi_aa64_rvae1is_write(CPUARMState *env,
do_rvae_write(env, value, vae1_tlbmask(env), true);
}
-static int vae2_tlbmask(CPUARMState *env)
-{
-return ARMMMUIdxBit_E2;
-}
-
static void tlbi_aa64_rvae2_write(CPUARMState *env,
const ARMCPRegInfo *ri,
uint64_t value)
--
2.41.0
[PATCH v3 5/6] target/arm/helper: Check SCR_EL3.{NSE, NS} encoding for AT instructions
The AT instruction is UNDEFINED if the {NSE,NS} configuration is
invalid. Add a function to check this on all AT instructions that apply
to an EL lower than 3.
Suggested-by: Peter Maydell
Signed-off-by: Jean-Philippe Brucker
---
target/arm/helper.c | 38 +++---
1 file changed, 27 insertions(+), 11 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index fbb03c364b..dbfe9f2f5e 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3616,6 +3616,22 @@ static void ats1h_write(CPUARMState *env, const
ARMCPRegInfo *ri,
#endif /* CONFIG_TCG */
}
+static CPAccessResult at_e012_access(CPUARMState *env, const ARMCPRegInfo *ri,
+ bool isread)
+{
+/*
+ * R_NYXTL: instruction is UNDEFINED if it applies to an Exception level
+ * lower than EL3 and the combination SCR_EL3.{NSE,NS} is reserved. This
can
+ * only happen when executing at EL3 because that combination also causes
an
+ * illegal exception return. We don't need to check FEAT_RME either,
because
+ * scr_write() ensures that the NSE bit is not set otherwise.
+ */
+if ((env->cp15.scr_el3 & (SCR_NSE | SCR_NS)) == SCR_NSE) {
+return CP_ACCESS_TRAP;
+}
+return CP_ACCESS_OK;
+}
+
static CPAccessResult at_s1e2_access(CPUARMState *env, const ARMCPRegInfo *ri,
bool isread)
{
@@ -3623,7 +3639,7 @@ static CPAccessResult at_s1e2_access(CPUARMState *env,
const ARMCPRegInfo *ri,
!(env->cp15.scr_el3 & (SCR_NS | SCR_EEL2))) {
return CP_ACCESS_TRAP;
}
-return CP_ACCESS_OK;
+return at_e012_access(env, ri, isread);
}
static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -5505,38 +5521,38 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
.opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 0,
.access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,
.fgt = FGT_ATS1E1R,
- .writefn = ats_write64 },
+ .accessfn = at_e012_access, .writefn = ats_write64 },
{ .name = "AT_S1E1W", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 1,
.access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,
.fgt = FGT_ATS1E1W,
- .writefn = ats_write64 },
+ .accessfn = at_e012_access, .writefn = ats_write64 },
{ .name = "AT_S1E0R", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 2,
.access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,
.fgt = FGT_ATS1E0R,
- .writefn = ats_write64 },
+ .accessfn = at_e012_access, .writefn = ats_write64 },
{ .name = "AT_S1E0W", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 3,
.access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,
.fgt = FGT_ATS1E0W,
- .writefn = ats_write64 },
+ .accessfn = at_e012_access, .writefn = ats_write64 },
{ .name = "AT_S12E1R", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 4,
.access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,
- .writefn = ats_write64 },
+ .accessfn = at_e012_access, .writefn = ats_write64 },
{ .name = "AT_S12E1W", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 5,
.access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,
- .writefn = ats_write64 },
+ .accessfn = at_e012_access, .writefn = ats_write64 },
{ .name = "AT_S12E0R", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 6,
.access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,
- .writefn = ats_write64 },
+ .accessfn = at_e012_access, .writefn = ats_write64 },
{ .name = "AT_S12E0W", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 7,
.access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,
- .writefn = ats_write64 },
+ .accessfn = at_e012_access, .writefn = ats_write64 },
/* AT S1E2* are elsewhere as they UNDEF from EL3 if EL2 is not present */
{ .name = "AT_S1E3R", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 6, .crn = 7, .crm = 8, .opc2 = 0,
@@ -8078,12 +8094,12 @@ static const ARMCPRegInfo ats1e1_reginfo[] = {
.opc0 = 1, .opc1 = 0, .crn = 7, .crm = 9, .opc2 = 0,
.access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,
.fgt = FGT_ATS1E1RP,
- .writefn = ats_write64 },
+ .accessfn = at_e012_access, .writefn = ats_write64 },
{ .name = "AT_S1E1WP", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 0, .crn = 7, .crm = 9, .opc2 = 1,
.access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,
.fgt = FGT_ATS1E1WP,
- .writefn = ats_write64 },
+ .accessfn = at_e012_access, .writefn = ats_write64 },
};
static const ARMCPRegInfo ats1cp_reginfo[] = {
--
2.41.0
Re: [RFC v4 01/11] build: Implement logic for sharing cross-building config files
This patch needs a detailed commit message, since it's not obvious why
these changes are made at all. It'd also be helpful for reviewing.
General style comment for shell scripts: Always put curly braces around
variables even if they are unnecessary. a $source_path could become
$source_pathPREFIX in the future and instead of ${source_path} it would
expand to ${source_pathPREFIX}.
On Tue, 08 Aug 2023 16:17, Yeqi Fu wrote:
+tcg_tests_targets=
+for target in $target_list; do
+ case $target in
+*-softmmu)
+ test -f "$source_path/tests/tcg/$arch/Makefile.softmmu-target" ||
continue
+ ;;
+ esac
+ if test -f cross-build/$target/config-target.mak; then
targets will never have spaces but I'd still double quote ${target} for
consistency and style
+ mkdir -p "tests/tcg/$target"
+ ln -srf cross-build/$target/config-target.mak
tests/tcg/$target/config-target.mak
+ ln -sf $source_path/tests/tcg/Makefile.target tests/tcg/$target/Makefile
This ln definitely needs double quoting.
echo "run-tcg-tests-$target: $qemu\$(EXESUF)" >> Makefile.prereqs
tcg_tests_targets="$tcg_tests_targets $target"
fi
--
2.34.1
Re: [RFC v4 11/11] docs/user: Add doc for native library calls
On Tue, 08 Aug 2023 16:17, Yeqi Fu wrote: +arm and aarch64 +--- +HLT is an invalid instruction for userspace and usefully has 16 +bits of spare immeadiate data which we can stuff data in. s/immeadiate/immediate With that fix, you can add Reviewed-by: Emmanouil Pitsidianakis
Re: [PATCH v2 10/19] target/ppc: Migrate DECR SPR
Hello Nick,
On 8/8/23 06:19, Nicholas Piggin wrote:
TCG does not maintain the DEC reigster in the SPR array, so it does get
migrated. TCG also needs to re-start the decrementer timer on the
destination machine.
Load and store the decrementer into the SPR when migrating. This works
for the level-triggered (book3s) decrementer, and should be compatible
with existing KVM machines that do keep the DEC value there.
This fixes lost decrementer interrupt on migration that can cause
hangs, as well as other problems including record-replay bugs.
Signed-off-by: Nicholas Piggin
---
target/ppc/machine.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index 8234e35d69..8a190c4853 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -209,6 +209,14 @@ static int cpu_pre_save(void *opaque)
/* Used to retain migration compatibility for pre 6.0 for 601 machines. */
env->hflags_compat_nmsr = 0;
+if (tcg_enabled()) {
+/*
+ * TCG does not maintain the DECR spr (unlike KVM) so have to save
+ * it here.
+ */
+env->spr[SPR_DECR] = cpu_ppc_load_decr(env);
+}
+
return 0;
}
@@ -319,6 +327,12 @@ static int cpu_post_load(void *opaque, int version_id)
ppc_update_ciabr(env);
ppc_update_daw0(env);
#endif
+/*
+ * TCG needs to re-start the decrementer timer and/or raise the
+ * interrupt. This works for level-triggered decrementer. Edge
+ * triggered types (including HDEC) would need to carry more state.
+ */
+cpu_ppc_store_decr(env, env->spr[SPR_DECR]);
pmu_mmcr01_updated(env);
}
This doesn't apply. I am missing some patch ?
Thanks,
C.
RE: [PATCH] Fix SEGFAULT on getting physical address of MMIO region.
> On 8/2/23 06:08, Mikhail Tyutin wrote: > > The fix is to clear TLB_INVALID_MASK bit in tlb_addr, as it happens in > > other places e.g. > > load_helper(). > > > > Signed-off-by: Dmitriy Solovev > > Signed-off-by: Mikhail Tyutin > > --- > > accel/tcg/cputlb.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > The other places in load_helper happen only directly after tlb_fill has > succeeded. Here > you have no such guarantee. > > I think perhaps the save_iotlb_data() call should be applied to loads as > well, and then > tlb_plugin_lookup simplified. > Hello Richard, We performed testing on more scenarios and noticed that patch when save_iotlb_data() call is added to io_readx (https://patchew.org/QEMU/20230804110903.19968-1-m.tyu...@yadro.com/). It doesn't work for addresses in OCRAM region. Those accessed bypass io_writex/io_readx function and therefore don’t invoke save_iotlb_data(). So we observe the wrong value of cpu->saved_iotlb for it. Would not be better to get back to initial v1 approach when we clean TLB_INVALID_MASK flag in tlb_plugin_lookup()? It works well for those regions. (https://patchew.org/QEMU/bf8ae2fd-158a-57b6-6270-2e56b6506...@yadro.com)
Re: [PATCH v2] target/i386: Avoid cpu number overflow in legacy topology
On 8/9/2023 7:14 PM, Igor Mammedov wrote:
> On Wed, 9 Aug 2023 18:27:32 +0800
> Qian Wen wrote:
>
>> The legacy topology enumerated by CPUID.1.EBX[23:16] is defined in SDM
>> Vol2:
>>
>> Bits 23-16: Maximum number of addressable IDs for logical processors in
>> this physical package.
>>
>> When launching the VM with -smp 256, the value written to EBX[23:16] is
>> 0 because of data overflow. If the guest only supports legacy topology,
>> without V2 Extended Topology enumerated by CPUID.0x1f or Extended
>> Topology enumerated by CPUID.0x0b to support over 255 CPUs, the return
>> of the kernel invoking cpu_smt_allowed() is false and AP's bring-up will
>> fail. Then only CPU 0 is online, and others are offline.
>>
>> To avoid this issue caused by overflow, limit the max value written to
>> EBX[23:16] to 255.
> what happens on real hw or in lack of thereof what SDM says about this
> value when there is more than 255 threads?.
>
Current SDM doesn't specify what the value should be when APIC IDs per package
exceeds 255. So we asked the internal HW architect, the response is that
EBX[23:16] will report 255 instead of being truncated to a smaller value.
Thanks,
Qian
>> Signed-off-by: Qian Wen
>> ---
>> Changes v1 -> v2:
>> - Revise the commit message and comment to more clearer.
>> - Rebased to v8.1.0-rc2.
>> ---
>> target/i386/cpu.c | 16 ++--
>> 1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index 97ad229d8b..6e1d88fbd7 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -6008,6 +6008,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
>> uint32_t count,
>> uint32_t die_offset;
>> uint32_t limit;
>> uint32_t signature[3];
>> +uint32_t threads_per_socket;
>> X86CPUTopoInfo topo_info;
>>
>> topo_info.dies_per_pkg = env->nr_dies;
>> @@ -6049,8 +6050,19 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
>> uint32_t count,
>> *ecx |= CPUID_EXT_OSXSAVE;
>> }
>> *edx = env->features[FEAT_1_EDX];
>> -if (cs->nr_cores * cs->nr_threads > 1) {
>> -*ebx |= (cs->nr_cores * cs->nr_threads) << 16;
>> +/*
>> + * Only bits [23:16] represent the maximum number of addressable
>> + * IDs for logical processors in this physical package.
>> + * When thread_per_socket > 255, it will 1) overwrite bits[31:24]
>> + * which is apic_id, 2) bits [23:16] get truncated.
>> + */
>> +threads_per_socket = cs->nr_cores * cs->nr_threads;
>> +if (threads_per_socket > 255) {
>> +threads_per_socket = 255;
>> +}
>> +
>> +if (threads_per_socket > 1) {
>> +*ebx |= threads_per_socket << 16;
>> *edx |= CPUID_HT;
>> }
>> if (!cpu->enable_pmu) {
[PULL 0/2] hw/nvme: more fixes
From: Klaus Jensen Hi, The following changes since commit a8fc5165aab02f328ccd148aafec1e59fd1426eb: Merge tag 'nvme-next-pull-request' of https://gitlab.com/birkelund/qemu into staging (2023-08-08 16:39:20 -0700) are available in the Git repository at: https://gitlab.com/birkelund/qemu.git tags/nvme-fixes-pull-request for you to fetch changes up to 3439ba9c5da943d96f7a3c86e0a7eb2ff48de41c: hw/nvme: fix null pointer access in ruh update (2023-08-09 15:32:32 +0200) hw/nvme: fixes -BEGIN PGP SIGNATURE- iQEzBAABCgAdFiEEUigzqnXi3OaiR2bATeGvMW1PDekFAmTTlmcACgkQTeGvMW1P DemjjggAnhEvaJ4fgS9rsvtxCwtzLNc405xMpNxh6rPaxa+sL3RXPIrW6vWG13+W VcHw8DI8EV4DzAFP919ZmTUq9/boRbgxx84bStlILUPHWol8+eGYVVfT75wFKszx d4Vi3nyPSGlrxieSrosARqimcUDtFtDGGAxjvEcKgzhkcU3a8DVYAOmx/hdlWJJQ KSk4h/E1pKItFbvv+w9yszsbToeZN65oIy7kQtFgx0JOULyWvEYSVygotw/AruF3 FPQ0nrJuZ115w3cJWDszznVJ6+3EcWbD3luQc3zE1FOPp76EkAOkcnPh1XbBJrE2 2BsCX/XnXcZT7BWSJbEzGXLsHjqsPg== =Zy0+ -END PGP SIGNATURE- Klaus Jensen (2): hw/nvme: fix null pointer access in directive receive hw/nvme: fix null pointer access in ruh update hw/nvme/ctrl.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) -- 2.41.0
Re: [PATCH v2 2/3] hw/smbios: Fix thread count in type4
On Mon, 7 Aug 2023 22:31:35 +0800 Zhao Liu wrote: > Hi Igor, > > On Mon, Aug 07, 2023 at 12:11:29PM +0200, Igor Mammedov wrote: > > Date: Mon, 7 Aug 2023 12:11:29 +0200 > > From: Igor Mammedov > > Subject: Re: [PATCH v2 2/3] hw/smbios: Fix thread count in type4 > > X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; x86_64-redhat-linux-gnu) > > > > On Mon, 7 Aug 2023 13:06:47 +0300 > > Michael Tokarev wrote: > > > > > 07.08.2023 12:56, Igor Mammedov wrote: > > > > On Sat, 5 Aug 2023 09:00:41 +0300 > > > > Michael Tokarev wrote: > > [...] > > > The whole thing - provided the preparational patch a1d027be95 > > > "machine: Add helpers to get cores/threads per socket" is also > > > picked up - applies cleanly and in a stright-forward way to 8.0 > > > and even to 7.2, and passes the usual qemu testsuite. Sure thing > > > since the issues weren't noticed before, the testsuite does not > > > cover this area. It'd be nice to have some verifier to check if > > > the whole thing actually works after applying the patchset. > > > > Zhao Liu, > > can you help us out with adding test cases to cover the code > > you are touching? > > Yes, sure. > > Just double check, I should add these 2 test cases: > 1. in "bios-tables-test.c" to test smbios type4 topology related things, and > 2. also in "test-smp-parse.c" to test our new topology helpers. > > Do I understand correctly? yep, I'd do both. Also looking at test cases I don't see any test cases that check topo end-to-end path (at least for x86). I mean checking related CPUID values. One possible place to it without writing testcase from scratch could be test-x86-cpuid-compat.c. Where I'd add test cases for some CPUID leaves at certain topo configurations (values to check could be hardcoded magic numbers as long as they are properly documented/reference specs) . > > -Zhao > > > > > [...] > > >
[PULL 2/2] hw/nvme: fix null pointer access in ruh update
From: Klaus Jensen
The Reclaim Unit Update operation in I/O Management Receive does not
verify the presence of a configured endurance group prior to accessing
it.
Fix this.
Cc: qemu-sta...@nongnu.org
Fixes: 73064edfb864 ("hw/nvme: flexible data placement emulation")
Reviewed-by: Jesper Wendel Devantier
Signed-off-by: Klaus Jensen
---
hw/nvme/ctrl.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index e5b5c7034d2b..539d27355313 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -4361,7 +4361,13 @@ static uint16_t nvme_io_mgmt_send_ruh_update(NvmeCtrl
*n, NvmeRequest *req)
uint32_t npid = (cdw10 >> 1) + 1;
unsigned int i = 0;
g_autofree uint16_t *pids = NULL;
-uint32_t maxnpid = n->subsys->endgrp.fdp.nrg * n->subsys->endgrp.fdp.nruh;
+uint32_t maxnpid;
+
+if (!ns->endgrp || !ns->endgrp->fdp.enabled) {
+return NVME_FDP_DISABLED | NVME_DNR;
+}
+
+maxnpid = n->subsys->endgrp.fdp.nrg * n->subsys->endgrp.fdp.nruh;
if (unlikely(npid >= MIN(NVME_FDP_MAXPIDS, maxnpid))) {
return NVME_INVALID_FIELD | NVME_DNR;
--
2.41.0
[PULL 1/2] hw/nvme: fix null pointer access in directive receive
From: Klaus Jensen
nvme_directive_receive() does not check if an endurance group has been
configured (set) prior to testing if flexible data placement is enabled
or not.
Fix this.
Cc: qemu-sta...@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1815
Fixes: 73064edfb864 ("hw/nvme: flexible data placement emulation")
Reviewed-by: Jesper Wendel Devantier
Signed-off-by: Klaus Jensen
---
hw/nvme/ctrl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index d217ae91b506..e5b5c7034d2b 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -6900,7 +6900,7 @@ static uint16_t nvme_directive_receive(NvmeCtrl *n,
NvmeRequest *req)
case NVME_DIRECTIVE_IDENTIFY:
switch (doper) {
case NVME_DIRECTIVE_RETURN_PARAMS:
-if (ns->endgrp->fdp.enabled) {
+if (ns->endgrp && ns->endgrp->fdp.enabled) {
id.supported |= 1 << NVME_DIRECTIVE_DATA_PLACEMENT;
id.enabled |= 1 << NVME_DIRECTIVE_DATA_PLACEMENT;
id.persistent |= 1 << NVME_DIRECTIVE_DATA_PLACEMENT;
--
2.41.0
Re: [PATCH v2] target/i386: Avoid cpu number overflow in legacy topology
On Wed, 9 Aug 2023 21:20:48 +0800
"Wen, Qian" wrote:
> On 8/9/2023 7:14 PM, Igor Mammedov wrote:
> > On Wed, 9 Aug 2023 18:27:32 +0800
> > Qian Wen wrote:
> >
> >> The legacy topology enumerated by CPUID.1.EBX[23:16] is defined in SDM
> >> Vol2:
> >>
> >> Bits 23-16: Maximum number of addressable IDs for logical processors in
> >> this physical package.
> >>
> >> When launching the VM with -smp 256, the value written to EBX[23:16] is
> >> 0 because of data overflow. If the guest only supports legacy topology,
> >> without V2 Extended Topology enumerated by CPUID.0x1f or Extended
> >> Topology enumerated by CPUID.0x0b to support over 255 CPUs, the return
> >> of the kernel invoking cpu_smt_allowed() is false and AP's bring-up will
> >> fail. Then only CPU 0 is online, and others are offline.
> >>
> >> To avoid this issue caused by overflow, limit the max value written to
> >> EBX[23:16] to 255.
> > what happens on real hw or in lack of thereof what SDM says about this
> > value when there is more than 255 threads?.
> >
>
> Current SDM doesn't specify what the value should be when APIC IDs per
> package exceeds 255. So we asked the internal HW architect, the response is
> that EBX[23:16] will report 255 instead of being truncated to a smaller value.
then mention it in commit log so one wouldn't wonder where the value came from.
>
> Thanks,
> Qian
>
> >> Signed-off-by: Qian Wen
> >> ---
> >> Changes v1 -> v2:
> >> - Revise the commit message and comment to more clearer.
> >> - Rebased to v8.1.0-rc2.
> >> ---
> >> target/i386/cpu.c | 16 ++--
> >> 1 file changed, 14 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> >> index 97ad229d8b..6e1d88fbd7 100644
> >> --- a/target/i386/cpu.c
> >> +++ b/target/i386/cpu.c
> >> @@ -6008,6 +6008,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
> >> uint32_t count,
> >> uint32_t die_offset;
> >> uint32_t limit;
> >> uint32_t signature[3];
> >> +uint32_t threads_per_socket;
> >> X86CPUTopoInfo topo_info;
> >>
> >> topo_info.dies_per_pkg = env->nr_dies;
> >> @@ -6049,8 +6050,19 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t
> >> index, uint32_t count,
> >> *ecx |= CPUID_EXT_OSXSAVE;
> >> }
> >> *edx = env->features[FEAT_1_EDX];
> >> -if (cs->nr_cores * cs->nr_threads > 1) {
> >> -*ebx |= (cs->nr_cores * cs->nr_threads) << 16;
> >> +/*
> >> + * Only bits [23:16] represent the maximum number of addressable
> >> + * IDs for logical processors in this physical package.
> >> + * When thread_per_socket > 255, it will 1) overwrite bits[31:24]
> >> + * which is apic_id, 2) bits [23:16] get truncated.
> >> + */
> >> +threads_per_socket = cs->nr_cores * cs->nr_threads;
> >> +if (threads_per_socket > 255) {
> >> +threads_per_socket = 255;
> >> +}
> >> +
> >> +if (threads_per_socket > 1) {
> >> +*ebx |= threads_per_socket << 16;
^
more robust would be mask out non-relevant fields at rhs
also perhaps double check if we could do induce similar overflow
tweaking other -smp properties (todo for another patch[es] if there are such
places).
> >> *edx |= CPUID_HT;
> >> }
> >> if (!cpu->enable_pmu) {
Re: [PATCH for-8.2] configure: fix and complete detection of tricore tools
On Wed, Aug 09, 2023 at 10:29:44AM +0200, Paolo Bonzini wrote: > The tricore tools are not detected when they are installed in > the host system, only if they are taken from an external > container. For this reason the build-tricore-softmmu job > was not running the TCG tests. > > In addition the container provides all tools, not just as/ld/gcc, > so there is no need to special case tricore. > > Signed-off-by: Paolo Bonzini > --- > configure | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) I think it would be better to skip debian-11 and use 12 instead. Cheers, Bastian
Re: [PATCH for-8.2] configure: fix and complete detection of tricore tools
On Wed, Aug 09, 2023 at 03:49:01PM +0200, Bastian Koppelmann wrote: > On Wed, Aug 09, 2023 at 10:29:44AM +0200, Paolo Bonzini wrote: > > The tricore tools are not detected when they are installed in > > the host system, only if they are taken from an external > > container. For this reason the build-tricore-softmmu job > > was not running the TCG tests. > > > > In addition the container provides all tools, not just as/ld/gcc, > > so there is no need to special case tricore. > > > > Signed-off-by: Paolo Bonzini > > --- > > configure | 5 + > > 1 file changed, 1 insertion(+), 4 deletions(-) > > I think it would be better to skip debian-11 and use 12 instead. Whoops I mixed it up with the other patch. For this one you can have Reviewed-by: Bastian Koppelmann Cheers, Bastian > > Cheers, > Bastian
Re: [PATCH for-8.2] dockerfiles: bump tricore cross compiler container to Debian 11
On Wed, Aug 09, 2023 at 10:29:45AM +0200, Paolo Bonzini wrote: > With the release of version 12 on June 10, 2023, Debian 10 is > not supported anymore. Modify the cross compiler container to > build on a newer version. > > Signed-off-by: Paolo Bonzini > --- > tests/docker/dockerfiles/debian-tricore-cross.docker | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/docker/dockerfiles/debian-tricore-cross.docker > b/tests/docker/dockerfiles/debian-tricore-cross.docker > index 269bfa8d423..5bd1963fb55 100644 > --- a/tests/docker/dockerfiles/debian-tricore-cross.docker > +++ b/tests/docker/dockerfiles/debian-tricore-cross.docker > @@ -9,7 +9,7 @@ > # > # SPDX-License-Identifier: GPL-2.0-or-later > # > -FROM docker.io/library/debian:buster-slim > +FROM docker.io/library/debian:11-slim I think it would be better to skip debian-11 and use 12 instead. Cheers, Bastian
Re: [PATCH v2] migration/calc-dirty-rate: millisecond-granularity period
On Sun, Aug 06, 2023 at 02:31:43PM +0800, Yong Huang wrote: > On Sat, Aug 5, 2023 at 2:05 AM Markus Armbruster wrote: > > > Andrei Gudkov writes: > > > > > Introduces alternative argument calc-time-ms, which is the > > > the same as calc-time but accepts millisecond value. > > > Millisecond granularity allows to make predictions whether > > > migration will succeed or not. To do this, calculate dirty > > > rate with calc-time-ms set to max allowed downtime, convert > > > measured rate into volume of dirtied memory, and divide by > > > network throughput. If the value is lower than max allowed > > > downtime, then migration will converge. > > > > > > Measurement results for single thread randomly writing to > > > a 1/4/24GiB memory region: > > > > > > +--+---+ > > > | calc-time-ms |dirty rate MiB/s | > > > | ++---+--+ > > > | | theoretical| page-sampling | dirty-bitmap | > > > | | (at 3M wr/sec) | | | > > > +--++---+--+ > > > | 1GiB | > > > +--++---+--+ > > > | 100 | 6996 | 7100 | 3192 | > > > | 200 | 4606 | 4660 | 2655 | > > > | 300 | 3305 | 3280 | 2371 | > > > | 400 | 2534 | 2525 | 2154 | > > > | 500 | 2041 | 2044 | 1871 | > > > | 750 | 1365 | 1341 | 1358 | > > > | 1000 | 1024 | 1052 | 1025 | > > > | 1500 |683 | 678 | 684 | > > > | 2000 |512 | 507 | 513 | > > > +--++---+--+ > > > | 4GiB | > > > +--++---+--+ > > > | 100 | 10232 | 8880 | 4070 | > > > | 200 | 8954 | 8049 | 3195 | > > > | 300 | 7889 | 7193 | 2881 | > > > | 400 | 6996 | 6530 | 2700 | > > > | 500 | 6245 | 5772 | 2312 | > > > | 750 | 4829 | 4586 | 2465 | > > > | 1000 | 3865 | 3780 | 2178 | > > > | 1500 | 2694 | 2633 | 2004 | > > > | 2000 | 2041 | 2031 | 1789 | > > > +--++---+--+ > > > | 24GiB| > > > +--++---+--+ > > > | 100 | 11495 | 8640 | 5597 | > > > | 200 | 11226 | 8616 | 3527 | > > > | 300 | 10965 | 8386 | 2355 | > > > | 400 | 10713 | 8370 | 2179 | > > > | 500 | 10469 | 8196 | 2098 | > > > | 750 | 9890 | 7885 | 2556 | > > > | 1000 | 9354 | 7506 | 2084 | > > > | 1500 | 8397 | 6944 | 2075 | > > > | 2000 | 7574 | 6402 | 2062 | > > > +--++---+--+ > > > > > > Theoretical values are computed according to the following formula: > > > size * (1 - (1-(4096/size))^(time*wps)) / (time * 2^20), > > > where size is in bytes, time is in seconds, and wps is number of > > > writes per second. > > > > > > Signed-off-by: Andrei Gudkov > > > --- > > > qapi/migration.json | 14 ++-- > > > migration/dirtyrate.h | 12 --- > > > migration/dirtyrate.c | 81 +-- > > > 3 files changed, 67 insertions(+), 40 deletions(-) > > > > > > diff --git a/qapi/migration.json b/qapi/migration.json > > > index 8843e74b59..82493d6a57 100644 > > > --- a/qapi/migration.json > > > +++ b/qapi/migration.json > > > @@ -1849,7 +1849,11 @@ > > > # @start-time: start time in units of second for calculation > > > # > > > # @calc-time: time period for which dirty page rate was measured > > > -# (in seconds) > > > +# (rounded down to seconds). > > > +# > > > +# @calc-time-ms: actual time period for which dirty page rate was > > > +# measured (in milliseconds). Value may be larger than requested > > > +# time period due to measurement overhead. > > > # > > > # @sample-pages: number of sampled pages per GiB of guest memory. > > > # Valid only in page-sam
Re: [PATCH for-8.2] dockerfiles: bump tricore cross compiler container to Debian 11
On Wed, Aug 9, 2023 at 3:53 PM Bastian Koppelmann wrote: > > diff --git a/tests/docker/dockerfiles/debian-tricore-cross.docker > > b/tests/docker/dockerfiles/debian-tricore-cross.docker > > index 269bfa8d423..5bd1963fb55 100644 > > --- a/tests/docker/dockerfiles/debian-tricore-cross.docker > > +++ b/tests/docker/dockerfiles/debian-tricore-cross.docker > > @@ -9,7 +9,7 @@ > > # > > # SPDX-License-Identifier: GPL-2.0-or-later > > # > > -FROM docker.io/library/debian:buster-slim > > +FROM docker.io/library/debian:11-slim > > I think it would be better to skip debian-11 and use 12 instead. I picked 11 in order to keep the container in sync with the lcitool-generated dockerfiles. Once we switch lcitool from 11 to 12, it is easier to see what the changes are and replicate them in debian-tricore-cross and friends. lcitool support for Debian 12 has already landed, but usually we try to keep the containers on the oldest supported release of a distro, and that is currently Debian 11. Paolo
Re: [PATCH] Fix SEGFAULT on getting physical address of MMIO region.
On 8/9/23 06:17, Mikhail Tyutin wrote: Would not be better to get back to initial v1 approach when we clean TLB_INVALID_MASK flag in tlb_plugin_lookup()? It works well for those regions. You're just as likely to get invalid data. r~
Re: [PATCH v5 02/11] target/loongarch: Add new object class for loongarch32 cpus
On 8/9/23 01:26, Jiajie Chen wrote: Add object class for future loongarch32 cpus. It is derived from the loongarch64 object class. Signed-off-by: Jiajie Chen --- target/loongarch/cpu.c | 19 +++ target/loongarch/cpu.h | 1 + 2 files changed, 20 insertions(+) Reviewed-by: Richard Henderson r~
Re: [RFC v4 01/11] build: Implement logic for sharing cross-building config files
Yeqi Fu writes:
> Signed-off-by: Yeqi Fu
> ---
> configure | 57 +--
> 1 file changed, 34 insertions(+), 23 deletions(-)
>
> diff --git a/configure b/configure
> index 2b41c49c0d..a076583141 100755
> --- a/configure
> +++ b/configure
> @@ -1751,56 +1751,67 @@ if test "$ccache_cpp2" = "yes"; then
>echo "export CCACHE_CPP2=y" >> $config_host_mak
> fi
>
> -# tests/tcg configuration
> -(config_host_mak=tests/tcg/config-host.mak
> -mkdir -p tests/tcg
> -echo "# Automatically generated by configure - do not modify" >
> $config_host_mak
> -echo "SRC_PATH=$source_path" >> $config_host_mak
> -echo "HOST_CC=$host_cc" >> $config_host_mak
> +# Prepare the config files for cross building.
> +# This process generates 'cross-build//config-target.mak' files.
> +# These files are then symlinked to the directories that need them which
> +# including the TCG tests (tests/tcg/) and the libnative library
> +# for linux-user (common/native//).
> +mkdir -p cross-build
>
> -# versioned checked in the main config_host.mak above
> -if test -n "$gdb_bin"; then
> -echo "HAVE_GDB_BIN=$gdb_bin" >> $config_host_mak
> -fi
> -if test "$plugins" = "yes" ; then
> -echo "CONFIG_PLUGIN=y" >> $config_host_mak
> -fi
I think there is a merge conflict here because a bunch of the
config-host.mak output has been squashed. This disabled plugins and gdb
testing.
> -
> -tcg_tests_targets=
> for target in $target_list; do
>arch=${target%%-*}
> -
>case $target in
> xtensa*-linux-user)
> - # the toolchain is not complete with headers, only build softmmu tests
> + # the toolchain for tests/tcg is not complete with headers
>continue
>;;
> *-softmmu)
> - test -f "$source_path/tests/tcg/$arch/Makefile.softmmu-target" ||
> continue
We still want to skip linking tests/tcg/foo-softmmu/config-target.mak
when there are no softmmu tests to build (only a few targets currently
have softmmu tests). I think this is triggering failures like:
➜ make run-tcg-tests-m68k-softmmu V=1
make -C tests/tcg/m68k-softmmu
make[1]: Entering directory
'/home/alex/lsrc/qemu.git/builds/all/tests/tcg/m68k-softmmu'
make[1]: Nothing to be done for 'all'.
make[1]: Leaving directory
'/home/alex/lsrc/qemu.git/builds/all/tests/tcg/m68k-softmmu'
make -C tests/tcg/m68k-softmmu SPEED=quick run
make[1]: Entering directory
'/home/alex/lsrc/qemu.git/builds/all/tests/tcg/m68k-softmmu'
make[1]: *** No rule to make target 'hello', needed by
'run-plugin-hello-with-libbb.so'. Stop.
make[1]: Leaving directory
'/home/alex/lsrc/qemu.git/builds/all/tests/tcg/m68k-softmmu'
make: *** [/home/alex/lsrc/qemu.git/tests/Makefile.include:56:
run-tcg-tests-m68k-softmmu] Erro
>qemu="qemu-system-$arch"
>;;
> *-linux-user|*-bsd-user)
>qemu="qemu-$arch"
>;;
>esac
> -
>if probe_target_compiler $target || test -n "$container_image"; then
>test -n "$container_image" && build_static=y
> - mkdir -p "tests/tcg/$target"
> - config_target_mak=tests/tcg/$target/config-target.mak
> - ln -sf "$source_path/tests/tcg/Makefile.target"
> "tests/tcg/$target/Makefile"
> + mkdir -p "cross-build/$target"
> + config_target_mak=cross-build/$target/config-target.mak
>echo "# Automatically generated by configure - do not modify" >
> "$config_target_mak"
>echo "TARGET_NAME=$arch" >> "$config_target_mak"
>echo "TARGET=$target" >> "$config_target_mak"
> - write_target_makefile "build-tcg-tests-$target" >> "$config_target_mak"
> + write_target_makefile "$target" >> "$config_target_mak"
>echo "BUILD_STATIC=$build_static" >> "$config_target_mak"
>echo "QEMU=$PWD/$qemu" >> "$config_target_mak"
>
> + # get the interpreter prefix and the path of libnative required for
> native call tests
> + if [ -d "/usr/$(echo "$target_cc" | sed 's/-gcc//')" ]; then
> + echo "LD_PREFIX=/usr/$(echo "$target_cc" | sed 's/-gcc//')" >>
> "$config_target_mak"
> + fi
> +
We should only emit LD_PREFIX for -user targets.
># will GDB work with these binaries?
>if test "${gdb_arches#*$arch}" != "$gdb_arches"; then
>echo "HOST_GDB_SUPPORTS_ARCH=y" >> "$config_target_mak"
>fi
> + fi
> +done
> +
> +# tests/tcg configuration
> +(mkdir -p tests/tcg
> +# create a symlink to the config-host.mak file in the tests/tcg
> +ln -srf $config_host_mak tests/tcg/config-host.mak
> +
> +tcg_tests_targets=
> +for target in $target_list; do
> + case $target in
> +*-softmmu)
> + test -f "$source_path/tests/tcg/$arch/Makefile.softmmu-target" ||
> continue
> + ;;
> + esac
>
> + if test -f cross-build/$target/config-target.mak; then
> + mkdir -p "tests/tcg/$target"
> + ln -srf cross-build/$target/config-target.mak
> tests/tcg/$target/config-target.mak
> + ln -sf $source_path/tests/tcg/Makefile.target
> tests/tc
Re: [PATCH for-8.2 2/3] pnv/lpc: Hook up xscom region for P9/P10
Hello Joel,
So we're re-using the same xscom ops as on P8. A quick look at the
definition of those 4 registers on P8 (0xb0020) and on P9/P10
(0x00090040) seem to show they are not the same though. Am i missing
something?
Fred
On 08/08/2023 10:34, Joel Stanley wrote:
From P9 on the LPC bus is memory mapped. However the xscom access still
is possible, so add it too.
Signed-off-by: Joel Stanley
---
include/hw/ppc/pnv_xscom.h | 6 ++
hw/ppc/pnv.c | 4
hw/ppc/pnv_lpc.c | 6 ++
3 files changed, 16 insertions(+)
diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
index 9bc64635471e..42601bdf419d 100644
--- a/include/hw/ppc/pnv_xscom.h
+++ b/include/hw/ppc/pnv_xscom.h
@@ -96,6 +96,9 @@ struct PnvXScomInterfaceClass {
#define PNV9_XSCOM_SBE_CTRL_BASE 0x00050008
#define PNV9_XSCOM_SBE_CTRL_SIZE 0x1
+#define PNV9_XSCOM_LPC_BASE 0x00090040
+#define PNV9_XSCOM_LPC_SIZE PNV_XSCOM_LPC_SIZE
+
#define PNV9_XSCOM_SBE_MBOX_BASE 0x000D0050
#define PNV9_XSCOM_SBE_MBOX_SIZE 0x16
@@ -155,6 +158,9 @@ struct PnvXScomInterfaceClass {
#define PNV10_XSCOM_SBE_CTRL_BASE PNV9_XSCOM_SBE_CTRL_BASE
#define PNV10_XSCOM_SBE_CTRL_SIZE PNV9_XSCOM_SBE_CTRL_SIZE
+#define PNV10_XSCOM_LPC_BASE PNV9_XSCOM_LPC_BASE
+#define PNV10_XSCOM_LPC_SIZE PNV9_XSCOM_LPC_SIZE
+
#define PNV10_XSCOM_SBE_MBOX_BASE PNV9_XSCOM_SBE_MBOX_BASE
#define PNV10_XSCOM_SBE_MBOX_SIZE PNV9_XSCOM_SBE_MBOX_SIZE
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index afdaa25c2b26..a5db655b41b6 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1566,6 +1566,8 @@ static void pnv_chip_power9_realize(DeviceState *dev,
Error **errp)
}
memory_region_add_subregion(get_system_memory(), PNV9_LPCM_BASE(chip),
&chip9->lpc.mmio_regs);
+pnv_xscom_add_subregion(chip, PNV9_XSCOM_LPC_BASE,
+&chip9->lpc.xscom_regs);
chip->fw_mr = &chip9->lpc.isa_fw;
chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
@@ -1785,6 +1787,8 @@ static void pnv_chip_power10_realize(DeviceState *dev,
Error **errp)
}
memory_region_add_subregion(get_system_memory(), PNV10_LPCM_BASE(chip),
&chip10->lpc.mmio_regs);
+pnv_xscom_add_subregion(chip, PNV10_XSCOM_LPC_BASE,
+&chip10->lpc.xscom_regs);
chip->fw_mr = &chip10->lpc.isa_fw;
chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
index caf5e10a5f96..6c6a3134087f 100644
--- a/hw/ppc/pnv_lpc.c
+++ b/hw/ppc/pnv_lpc.c
@@ -666,6 +666,12 @@ static void pnv_lpc_power9_realize(DeviceState *dev, Error
**errp)
/* P9 uses a MMIO region */
memory_region_init_io(&lpc->mmio_regs, OBJECT(lpc), &pnv_lpc_mmio_ops,
lpc, "lpcm", PNV9_LPCM_SIZE);
+
+/* but the XSCOM region still exists */
+pnv_xscom_region_init(&lpc->xscom_regs, OBJECT(lpc),
+ &pnv_lpc_xscom_ops, lpc, "xscom-lpc",
+ PNV_XSCOM_LPC_SIZE);
+
}
static void pnv_lpc_power9_class_init(ObjectClass *klass, void *data)
Re: [PATCH v5 08/11] target/loongarch: Reject la64-only instructions in la32 mode
On 8/9/23 01:26, Jiajie Chen wrote: LoongArch64-only instructions are marked with regard to the instruction manual Table 2. LSX instructions are not marked for now for lack of public manual. I would expect LSX not to be affected by CPUCFG.1.ARCH, but only by CPUCFG.2.LSX. Note that there appears to be a bug with respect to LSX, in that CPUCFG.2.LSX is not checked. The manual is not clear, but I would expect CPUCFG.2.LSX == 0 to trigger an illegal instruction exception before the check for EUEN.SXE == 0 to trigger an instruction disable exception. Also, are bit in EUEN allowed to be set to non-zero values when the corresponding expansion is not present? But that is not a problem with this patch, so: Reviewed-by: Richard Henderson r~
Re: [PATCH v2] migration/calc-dirty-rate: millisecond-granularity period
On Sun, Aug 06, 2023 at 02:16:34PM +0800, Yong Huang wrote: > On Fri, Aug 4, 2023 at 11:03 PM Andrei Gudkov > wrote: > > > Introduces alternative argument calc-time-ms, which is the > > the same as calc-time but accepts millisecond value. > > Millisecond granularity allows to make predictions whether > > migration will succeed or not. To do this, calculate dirty > > rate with calc-time-ms set to max allowed downtime, convert > > measured rate into volume of dirtied memory, and divide by > > network throughput. If the value is lower than max allowed > > > Not for the patch, I'm just curious about how the predication > decides the network throughput, I mean QEMU predicts > if migration will converge based on how fast it sends the data, > not the actual bandwidth of the interface, which one the > prediction uses? > Currently I use network nominal bandwidth, e.g. 1gbps. It would be nice, of course, to use measured throughput since it can take into account network headers overhead (as Wang Lei previously mentioned), compression, etc., but it looks too complicated to implement outside of migration process. > > downtime, then migration will converge. > > > > Measurement results for single thread randomly writing to > > a 1/4/24GiB memory region: > > > > +--+---+ > > | calc-time-ms |dirty rate MiB/s | > > | ++---+--+ > > | | theoretical| page-sampling | dirty-bitmap | > > | | (at 3M wr/sec) | | | > > +--++---+--+ > > | 1GiB | > > +--++---+--+ > > | 100 | 6996 | 7100 | 3192 | > > | 200 | 4606 | 4660 | 2655 | > > | 300 | 3305 | 3280 | 2371 | > > | 400 | 2534 | 2525 | 2154 | > > | 500 | 2041 | 2044 | 1871 | > > | 750 | 1365 | 1341 | 1358 | > > | 1000 | 1024 | 1052 | 1025 | > > | 1500 |683 | 678 | 684 | > > | 2000 |512 | 507 | 513 | > > +--++---+--+ > > | 4GiB | > > +--++---+--+ > > | 100 | 10232 | 8880 | 4070 | > > | 200 | 8954 | 8049 | 3195 | > > | 300 | 7889 | 7193 | 2881 | > > | 400 | 6996 | 6530 | 2700 | > > | 500 | 6245 | 5772 | 2312 | > > | 750 | 4829 | 4586 | 2465 | > > | 1000 | 3865 | 3780 | 2178 | > > | 1500 | 2694 | 2633 | 2004 | > > | 2000 | 2041 | 2031 | 1789 | > > +--++---+--+ > > | 24GiB| > > +--++---+--+ > > | 100 | 11495 | 8640 | 5597 | > > | 200 | 11226 | 8616 | 3527 | > > | 300 | 10965 | 8386 | 2355 | > > | 400 | 10713 | 8370 | 2179 | > > | 500 | 10469 | 8196 | 2098 | > > | 750 | 9890 | 7885 | 2556 | > > | 1000 | 9354 | 7506 | 2084 | > > | 1500 | 8397 | 6944 | 2075 | > > | 2000 | 7574 | 6402 | 2062 | > > +--++---+--+ > > > > Theoretical values are computed according to the following formula: > > size * (1 - (1-(4096/size))^(time*wps)) / (time * 2^20), > > where size is in bytes, time is in seconds, and wps is number of > > writes per second. > > > > Signed-off-by: Andrei Gudkov > > --- > > qapi/migration.json | 14 ++-- > > migration/dirtyrate.h | 12 --- > > migration/dirtyrate.c | 81 +-- > > 3 files changed, 67 insertions(+), 40 deletions(-) > > > > [...] > > > diff --git a/qapi/migration.json b/qapi/migration.json > > index 8843e74b59..82493d6a57 100644 > > --- a/qapi/migration.json > > +++ b/qapi/migration.json > > @@ -1849,7 +1849,11 @@ > > # @start-time: start time in units of second for calculation > > # > > # @calc-time: time period for which dirty page rate was measured
Re: [PATCH v5 09/11] target/loongarch: Truncate high 32 bits of address in VA32 mode
On 8/9/23 01:26, Jiajie Chen wrote: When running in VA32 mode(!LA64 or VA32L[1-3] matching PLV), virtual address is truncated to 32 bits before address mapping. Signed-off-by: Jiajie Chen Co-authored-by: Richard Henderson --- target/loongarch/cpu.c| 16 target/loongarch/cpu.h| 9 + target/loongarch/gdbstub.c| 2 +- .../loongarch/insn_trans/trans_atomic.c.inc | 5 ++- .../loongarch/insn_trans/trans_branch.c.inc | 3 +- .../loongarch/insn_trans/trans_fmemory.c.inc | 30 --- target/loongarch/insn_trans/trans_lsx.c.inc | 38 +-- .../loongarch/insn_trans/trans_memory.c.inc | 34 + target/loongarch/op_helper.c | 4 +- target/loongarch/translate.c | 32 10 files changed, 85 insertions(+), 88 deletions(-) Much better, thanks. Reviewed-by: Richard Henderson r~
Re: [PATCH v5 10/11] target/loongarch: Sign extend results in VA32 mode
On 8/9/23 01:26, Jiajie Chen wrote: In VA32 mode, BL, JIRL and PC* instructions should sign-extend the low 32 bit result to 64 bits. Signed-off-by: Jiajie Chen --- target/loongarch/insn_trans/trans_arith.c.inc | 2 +- target/loongarch/insn_trans/trans_branch.c.inc | 4 ++-- target/loongarch/translate.c | 8 3 files changed, 11 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson r~
Fix interval_tree_iter_first() to check root node value
Fix a crash in qemu-user when running
cat /proc/self/maps
in a chroot, where /proc isn't mounted.
The problem was introduced by commit 3ce3dd8ca965 ("util/selfmap:
Rewrite using qemu/interval-tree.h") where in open_self_maps_1() the
function read_self_maps() is called and which returns NULL if it can't
read the hosts /proc/self/maps file. Afterwards that NULL is fed into
interval_tree_iter_first() which doesn't check if the root node is NULL.
Fix it by adding a check if root is NULL and return NULL in that case.
Signed-off-by: Helge Deller
Fixes: 3ce3dd8ca965 ("util/selfmap: Rewrite using qemu/interval-tree.h")
diff --git a/util/interval-tree.c b/util/interval-tree.c
index f2866aa7d3..53465182e6 100644
--- a/util/interval-tree.c
+++ b/util/interval-tree.c
@@ -797,7 +797,7 @@ IntervalTreeNode *interval_tree_iter_first(IntervalTreeRoot
*root,
{
IntervalTreeNode *node, *leftmost;
-if (!root->rb_root.rb_node) {
+if (!root || !root->rb_root.rb_node) {
return NULL;
}
Re: [PATCH v5 11/11] target/loongarch: Add loongarch32 cpu la132
On 8/9/23 01:26, Jiajie Chen wrote: Add la132 as a loongarch32 cpu type and allow virt machine to be used with la132 instead of la464. Due to lack of public documentation of la132, it is currently a synthetic loongarch32 cpu model. Details need to be added in the future. Signed-off-by: Jiajie Chen --- hw/loongarch/virt.c| 5 - target/loongarch/cpu.c | 29 + 2 files changed, 29 insertions(+), 5 deletions(-) Acked-by: Richard Henderson r~
Re: [PATCH v1 1/3] softmmu/physmem: fallback to opening guest RAM file as readonly in a MAP_PRIVATE mapping
On Wed, Aug 09, 2023 at 11:20:11AM +0200, David Hildenbrand wrote:
> Hi Peter!
Hi, David,
>
> > > -fd = file_ram_open(mem_path, memory_region_name(mr), readonly,
> > > &created,
> > > - errp);
> > > +fd = file_ram_open(mem_path, memory_region_name(mr), readonly,
> > > &created);
> > > +if (fd == -EACCES && !(ram_flags & RAM_SHARED) && !readonly) {
> > > +/*
> > > + * We can have a writable MAP_PRIVATE mapping of a readonly file.
> > > + * However, some operations like ftruncate() or fallocate()
> > > might fail
> > > + * later, let's warn the user.
> > > + */
> > > +fd = file_ram_open(mem_path, memory_region_name(mr), true,
> > > &created);
> > > +if (fd >= 0) {
> > > +warn_report("backing store %s for guest RAM (MAP_PRIVATE)
> > > opened"
> > > +" readonly because the file is not writable",
> > > mem_path);
> >
> > I can understand the use case, but this will be slightly unwanted,
> > especially the user doesn't yet have a way to predict when will it happen.
>
> Users can set the file permissions accordingly I guess. If they don't want
> the file to never ever be modified via QEMU, set it R/O.
>
> >
> > Meanwhile this changes the behavior, is it a concern that someone may want
> > to rely on current behavior of failing?
>
> The scenario would be that someone passes a readonly file to "-mem-path" or
> "-object memory-backend-file,share=off,readonly=off", with the expectation
> that it would currently fail.
>
> If it now doesn't fail (and we warn instead), what would happen is:
> * In file_ram_alloc() we won't even try ftruncate(), because the file
> already had a size > 0. So ftruncate() is not a concern as I now
> realize.
> * fallocate might fail later. AFAIKS, that only applies to
> ram_block_discard_range().
> -> virtio-mem performs an initial ram_block_discard_range() check and
> fails gracefully early.
> -> virtio-ballooon ignores any errors
> -> ram_discard_range() in migration code fails early for postcopy in
> init_range() and loadvm_postcopy_ram_handle_discard(), handling it
> gracefully.
>
> So mostly nothing "bad" would happen, it might just be undesirable, and we
> properly warn.
Indeed, all of them can fail gracefully, while balloon one is harmless.
Definitely good news.
>
> Most importantly, we won't be corrupting/touching the original file in any
> case, because it is R/O.
>
> If we really want to be careful, we could clue that behavior to compat
> machines. I'm not really sure yet if we really have to go down that path.
>
> Any other alternatives? I'd like to avoid new flags where not really
> required.
I was just thinking of a new flag. :) So have you already discussed that
possibility and decided that not a good idea?
The root issue to me here is we actually have two resources (memory map of
the process, and the file) but we only have one way to describe the
permissions upon the two objects. I'd think it makes a lot more sense if a
new flag is added, when there's a need to differentiate the two.
Consider if you see a bunch of qemu instances with:
-mem-path $RAM_FILE
On the same host, which can be as weird as it could be to me.. At least
'-mem-path' looks still like a way to exclusively own a ram file for an
instance. I hesitate the new fallback can confuse people too, while that's
so far not the major use case.
Nobody may really rely on any existing behavior of the failure, but
changing existing behavior is just always not wanted. The guideline here
to me is: whether we want existing "-mem-path XXX" users to start using the
fallback in general? If it's "no", then maybe it implies a new flag is
better?
>
> >
> > To think from a higher level of current use case, the ideal solution seems
> > to me that if the ram file can be put on a file system that supports CoW
> > itself (like btrfs), we can snapshot that ram file and make it RW for the
> > qemu instance. Then here it'll be able to open the file. We'll be able to
> > keep the interface working as before, meanwhile it'll work with fallocate
> > or truncations too I assume.
> >
> > Would that be better instead of changing QEMU?
>
> As I recently learned, using file-backed VMs (on real ssd/disks, not
> shmem/hugetlb) is usually undesired, because the dirtied pages will
> constantly get rewritten to disk by background writeback threads, eventually
> resulting in bad performance and SSD wear.
>
> So while using a COW filesystem sounds cleaner in theory, it's not
> applicable in practice -- unless one disables any background writeback,
> which has different side effects because it cannot be configured on a
> per-file basis.
>
> So for VM templating, it makes sense to capture the guest RAM and store it
> in a file, to then use a COW (MAP_PRIVATE) mapping. Using a read-only file
> makes perfect sense in that scenario IMHO.
Valid point.
>
> [I'm curious at wha
Re: Fix interval_tree_iter_first() to check root node value
On 8/9/23 08:11, Helge Deller wrote:
Fix a crash in qemu-user when running
cat /proc/self/maps
in a chroot, where /proc isn't mounted.
The problem was introduced by commit 3ce3dd8ca965 ("util/selfmap:
Rewrite using qemu/interval-tree.h") where in open_self_maps_1() the
function read_self_maps() is called and which returns NULL if it can't
read the hosts /proc/self/maps file. Afterwards that NULL is fed into
interval_tree_iter_first() which doesn't check if the root node is NULL.
Fix it by adding a check if root is NULL and return NULL in that case.
Signed-off-by: Helge Deller
Fixes: 3ce3dd8ca965 ("util/selfmap: Rewrite using qemu/interval-tree.h")
diff --git a/util/interval-tree.c b/util/interval-tree.c
index f2866aa7d3..53465182e6 100644
--- a/util/interval-tree.c
+++ b/util/interval-tree.c
@@ -797,7 +797,7 @@ IntervalTreeNode *interval_tree_iter_first(IntervalTreeRoot
*root,
{
IntervalTreeNode *node, *leftmost;
-if (!root->rb_root.rb_node) {
+if (!root || !root->rb_root.rb_node) {
I guess this is good enough for 8.1. Before the conversion to interval-tree we would also
emit nothing.
I've already done a rewrite for 8.2, and I noticed this problem. There I emit what
mapping information that I have, which is everything except for the device+path data.
Reviewed-by: Richard Henderson
r~
Re: [RFC v4 02/11] build: Implement libnative library and the build machinery for libnative
Yeqi Fu writes:
> This commit implements a shared library, where native functions are
> rewritten as special instructions. At runtime, user programs load
> the shared library, and special instructions are executed when
> native functions are called.
>
> Signed-off-by: Yeqi Fu
> ---
> Makefile| 2 +
> common-user/native/Makefile.include | 9
> common-user/native/Makefile.target | 22 ++
> common-user/native/libnative.c | 67 +
> configure | 39 +
> include/native/libnative.h | 8
> 6 files changed, 147 insertions(+)
> create mode 100644 common-user/native/Makefile.include
> create mode 100644 common-user/native/Makefile.target
> create mode 100644 common-user/native/libnative.c
> create mode 100644 include/native/libnative.h
>
> diff --git a/Makefile b/Makefile
> index 5d48dfac18..6f6147b40f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -182,6 +182,8 @@ SUBDIR_MAKEFLAGS=$(if $(V),,--no-print-directory --quiet)
>
> include $(SRC_PATH)/tests/Makefile.include
>
> +include $(SRC_PATH)/common-user/native/Makefile.include
> +
> all: recurse-all
>
> ROMS_RULES=$(foreach t, all clean distclean, $(addsuffix /$(t), $(ROMS)))
> diff --git a/common-user/native/Makefile.include
> b/common-user/native/Makefile.include
> new file mode 100644
> index 00..40d20bcd4c
> --- /dev/null
> +++ b/common-user/native/Makefile.include
> @@ -0,0 +1,9 @@
> +.PHONY: build-native
> +build-native: $(NATIVE_TARGETS:%=build-native-library-%)
> +$(NATIVE_TARGETS:%=build-native-library-%): build-native-library-%:
> + $(call quiet-command, \
> + $(MAKE) -C common-user/native/$* $(SUBDIR_MAKEFLAGS), \
> + "BUILD","$* native library")
> +# endif
> +
> +all: build-native
> diff --git a/common-user/native/Makefile.target
> b/common-user/native/Makefile.target
> new file mode 100644
> index 00..0c1241b368
> --- /dev/null
> +++ b/common-user/native/Makefile.target
> @@ -0,0 +1,22 @@
> +# -*- Mode: makefile -*-
> +#
> +# Library for native calls
> +#
> +
> +all:
> +-include ../../config-host.mak
This is sensitive to the out of tree build structure the user chooses. For
example:
➜ pwd
/home/alex/lsrc/qemu.git/builds/user/common-user/native/aarch64-linux-user
🕙16:20:08 alex@zen:common-user/native/aarch64-linux-user on
review/native-lib-calls-v4 [$!?]
➜ make libnative.so
make: *** No rule to make target '/common-user/native/libnative.c', needed by
'libnative.so'. Stop.
🕙16:20:13 alex@zen:common-user/native/aarch64-linux-user on
review/native-lib-calls-v4 [$!?] [🔴 USAGE]
✗
I think this can be solved the same way as we do for tests/tcg by
symlinking the config-host.mak into place and referring to it directly
or adjusting the include to ../../../config-host.mak because the top of
the build tree has a symlinked copy as well.
> +-include config-target.mak
> +
> +CFLAGS+=-O1 -fPIC -shared -fno-stack-protector -I$(SRC_PATH)/include
> -D$(TARGET_NAME)
> +LDFLAGS+=
> +
> +SRC = $(SRC_PATH)/common-user/native/libnative.c
> +LIBNATIVE = libnative.so
> +
> +all: $(LIBNATIVE)
> +
> +$(LIBNATIVE): $(SRC)
> + $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $(EXTRA_NATIVE_CALL_FLAGS) $< -o $@
> $(LDFLAGS)
> +
> +clean:
> + rm -f $(LIBNATIVE)
> diff --git a/common-user/native/libnative.c b/common-user/native/libnative.c
> new file mode 100644
> index 00..662ae6fbfe
> --- /dev/null
> +++ b/common-user/native/libnative.c
> @@ -0,0 +1,67 @@
> +#include
> +#include
> +#include
> +
> +#include "native/libnative.h"
> +
> +#define WRAP_NATIVE() \
> +do { \
> +__asm__ volatile(__CALL_EXPR : : : "memory"); \
> +} while (0)
> +
> +#if defined(i386) || defined(x86_64)
> +/*
> + * An unused instruction is utilized to mark a native call.
> + */
> +#define __CALL_EXPR ".byte 0x0f, 0xff;"
> +#endif
> +
> +#if defined(arm) || defined(aarch64)
> +/*
> + * HLT is an invalid instruction for userspace and usefully has 16
> + * bits of spare immeadiate data which we can stuff data in.
> + */
> +#define __CALL_EXPR "hlt 0x;"
> +#endif
> +
> +#if defined(mips) || defined(mips64)
> +/*
> + * The syscall instruction contains 20 unused bits, which are typically
> + * set to 0. These bits can be used to store non-zero data,
> + * distinguishing them from a regular syscall instruction.
> + */
> +#define __CALL_EXPR "syscall 0x;"
> +#endif
> +
> +void *memcpy(void *dest, const void *src, size_t n)
> +{
> +WRAP_NATIVE();
> +}
> +int memcmp(const void *s1, const void *s2, size_t n)
> +{
> +WRAP_NATIVE();
> +}
> +void *memset(void *s, int c, size_t n)
> +{
> +WRAP_NATIVE();
> +}
> +char *strncpy(char *dest, const char *src, size_t n)
> +{
> +WRAP_NATIVE();
> +}
> +int strncmp(const char *s1, const char *s2, size_t n)
> +{
> +WRAP_NATIVE();
> +}
> +char *strcpy
Re: [RFC PATCH 15/19] kvm: handle KVM_EXIT_MEMORY_FAULT
On 2023-07-31 at 12:21:57 -0400, Xiaoyao Li wrote:
> From: Chao Peng
>
> Currently only KVM_MEMORY_EXIT_FLAG_PRIVATE in flags is valid when
> KVM_EXIT_MEMORY_FAULT happens. It indicates userspace needs to do
> the memory conversion on the RAMBlock to turn the memory into desired
> attribute, i.e., private/shared.
>
> Note, KVM_EXIT_MEMORY_FAULT makes sense only when the RAMBlock has
> gmem memory backend.
>
> Signed-off-by: Chao Peng
> Signed-off-by: Xiaoyao Li
> ---
> accel/kvm/kvm-all.c | 52 +
> 1 file changed, 52 insertions(+)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index f9b5050b8885..72d50b923bf2 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -3040,6 +3040,48 @@ static void kvm_eat_signals(CPUState *cpu)
> } while (sigismember(&chkset, SIG_IPI));
> }
>
> +static int kvm_convert_memory(hwaddr start, hwaddr size, bool to_private)
> +{
> +MemoryRegionSection section;
> +void *addr;
> +RAMBlock *rb;
> +ram_addr_t offset;
> +int ret = -1;
> +
> +section = memory_region_find(get_system_memory(), start, size);
> +if (!section.mr) {
> +return ret;
> +}
> +
> +if (memory_region_can_be_private(section.mr)) {
> +if (to_private) {
> +ret = kvm_set_memory_attributes_private(start, size);
> +} else {
> +ret = kvm_set_memory_attributes_shared(start, size);
> +}
> +
> +if (ret) {
> +return ret;
Should we unref the memory region before return?
Thanks,
Yilun
> +}
> +
> +addr = memory_region_get_ram_ptr(section.mr) +
> + section.offset_within_region;
> +rb = qemu_ram_block_from_host(addr, false, &offset);
> +/*
> + * With KVM_SET_MEMORY_ATTRIBUTES by kvm_set_memory_attributes(),
> + * operation on underlying file descriptor is only for releasing
> + * unnecessary pages.
> + */
> +ram_block_convert_range(rb, offset, size, to_private);
> +} else {
> +warn_report("Convert non guest-memfd backed memory region
> (0x%"HWADDR_PRIx" ,+ 0x%"HWADDR_PRIx") to %s",
> +start, size, to_private ? "private" : "shared");
> +}
> +
> +memory_region_unref(section.mr);
> +return ret;
> +}
Re: [RFC v4 01/11] build: Implement logic for sharing cross-building config files
Alex Bennée writes: > Yeqi Fu writes: > >> Signed-off-by: Yeqi Fu >> --- >> configure | 57 +-- >> 1 file changed, 34 insertions(+), 23 deletions(-) >> >> diff --git a/configure b/configure >> index 2b41c49c0d..a076583141 100755 >> --- a/configure >> +++ b/configure >> @@ -1751,56 +1751,67 @@ if test "$ccache_cpp2" = "yes"; then >>echo "export CCACHE_CPP2=y" >> $config_host_mak >> fi >> >> -# tests/tcg configuration >> -(config_host_mak=tests/tcg/config-host.mak >> -mkdir -p tests/tcg >> -echo "# Automatically generated by configure - do not modify" > >> $config_host_mak >> -echo "SRC_PATH=$source_path" >> $config_host_mak >> -echo "HOST_CC=$host_cc" >> $config_host_mak >> +# Prepare the config files for cross building. >> +# This process generates 'cross-build//config-target.mak' files. >> +# These files are then symlinked to the directories that need them which >> +# including the TCG tests (tests/tcg/) and the libnative library >> +# for linux-user (common/native//). >> +mkdir -p cross-build >> >> -# versioned checked in the main config_host.mak above >> -if test -n "$gdb_bin"; then >> -echo "HAVE_GDB_BIN=$gdb_bin" >> $config_host_mak >> -fi >> -if test "$plugins" = "yes" ; then >> -echo "CONFIG_PLUGIN=y" >> $config_host_mak >> -fi > > I think there is a merge conflict here because a bunch of the > config-host.mak output has been squashed. This disabled plugins and gdb > testing. Ahh I see now this was intentional because we symlink however it was lost in the noise of the diff. As Manos pointed out detailing the movement in the commit message aids reviewers in tracing what has changed. -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH v2] migration/calc-dirty-rate: millisecond-granularity period
On Wed, Aug 09, 2023 at 06:02:52PM +0300, gudkov.and...@huawei.com wrote: > > Not for the patch, I'm just curious about how the predication > > decides the network throughput, I mean QEMU predicts > > if migration will converge based on how fast it sends the data, > > not the actual bandwidth of the interface, which one the > > prediction uses? > > > Currently I use network nominal bandwidth, e.g. 1gbps. It would > be nice, of course, to use measured throughput since it can take > into account network headers overhead (as Wang Lei previously > mentioned), compression, etc., but it looks too complicated to > implement outside of migration process. Using line speed is definitely wise, but qemu may be stupid enough to do wrong calculations and when it happens migration may not switchover.. See this: https://lore.kernel.org/r/20230803155344.11450-3-pet...@redhat.com Probably should be used together with your ways of prediction to be even more accurate on some workloads where mbps reports insane numbers. Thanks, -- Peter Xu
[PATCH QEMU] docs/migration: Add the dirty limit section
From: Hyman Huang(黄勇) The dirty limit feature has been introduced since the 8.1 QEMU release but has not reflected in the document, add a section for that. Signed-off-by: Hyman Huang(黄勇) --- docs/devel/migration.rst | 70 1 file changed, 70 insertions(+) diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst index c3e1400c0c..4cc83adc8e 100644 --- a/docs/devel/migration.rst +++ b/docs/devel/migration.rst @@ -588,6 +588,76 @@ path. Return path - opened by main thread, written by main thread AND postcopy thread (protected by rp_mutex) +Dirty limit += +The dirty limit, short for dirty page rate upper limit, is a new capability +introduced in the 8.1 QEMU release that uses a new algorithm based on the KVM +dirty ring to throttle down the guest during live migration. + +The algorithm framework is as follows: + +:: + + --- + main --> throttle thread > PREPARE(1) < + thread \| | + \ | | +\ V | + -\CALCULATE(2) | + \ | | +\ | | + \ V | + \SET PENALTY(3) - + -\ | + \ | + \V + -> virtual CPU thread ---> ACCEPT PENALTY(4) + --- +When the qmp command qmp_set_vcpu_dirty_limit is called for the first time, +the QEMU main thread starts the throttle thread. The throttle thread, once +launched, executes the loop, which consists of three steps: + + - PREPARE (1) + + The entire work of PREPARE (1) is prepared for the second stage, + CALCULATE(2), as the name implies. It involves preparing the dirty + page rate value and the corresponding upper limit of the VM: + The dirty page rate is calculated via the KVM dirty ring mechanism, + which tells QEMU how many dirty pages a virtual CPU has had since the + last KVM_EXIT_DIRTY_RING_RULL exception; The dirty page rate upper + limit is specified by caller, therefore fetch it directly. + + - CALCULATE (2) + + Calculate a suitable sleep period for each virtual CPU, which will be + used to determine the penalty for the target virtual CPU. The + computation must be done carefully in order to reduce the dirty page + rate progressively down to the upper limit without oscillation. To + achieve this, two strategies are provided: the first is to add or + subtract sleep time based on the ratio of the current dirty page rate + to the limit, which is used when the current dirty page rate is far + from the limit; the second is to add or subtract a fixed time when + the current dirty page rate is close to the limit. + + - SET PENALTY (3) + + Set the sleep time for each virtual CPU that should be penalized based + on the results of the calculation supplied by step CALCULATE (2). + +After completing the three above stages, the throttle thread loops back +to step PREPARE (1) until the dirty limit is reached. + +On the other hand, each virtual CPU thread reads the sleep duration and +sleeps in the path of the KVM_EXIT_DIRTY_RING_RULL exception handler, that +is ACCEPT PENALTY (4). Virtual CPUs tied with writing processes will +obviously exit to the path and get penalized, whereas virtual CPUs involved +with read processes will not. + +In summary, thanks to the KVM dirty ring technology, the dirty limit +algorithm will restrict virtual CPUs as needed to keep their dirty page +rate inside the limit. This leads to more steady reading performance during +live migration and can aid in improving large guest responsiveness. + Postcopy -- 2.38.5
Re: [RFC v4 04/11] linux-user: Implement native-bypass option support
On 8/8/23 07:17, Yeqi Fu wrote:
+#define native_bypass_enabled() native_lib_path ? true : false
Need parenthesis for the expression, and possibly better as
(native_lib_path != NULL)
rather than ternary expression.
+#if defined(CONFIG_NATIVE_CALL)
+/* Set the library for native bypass */
+if (native_lib_path) {
+if (g_file_test(native_lib_path, G_FILE_TEST_EXISTS)) {
G_FILE_TEST_EXISTS may be a directory.
Better with G_FILE_TEST_IS_REGULAR, I guess?
r~
Re: [RFC v4 03/11] linux-user: Implement envlist_appendenv and add tests for envlist
On 8/8/23 07:17, Yeqi Fu wrote:
Signed-off-by: Yeqi Fu
---
include/qemu/envlist.h| 13 ++
tests/unit/meson.build| 1 +
tests/unit/test-envlist.c | 94 +++
util/envlist.c| 71 -
4 files changed, 169 insertions(+), 10 deletions(-)
create mode 100644 tests/unit/test-envlist.c
diff --git a/include/qemu/envlist.h b/include/qemu/envlist.h
index 6006dfae44..9eb1459e79 100644
--- a/include/qemu/envlist.h
+++ b/include/qemu/envlist.h
@@ -1,12 +1,25 @@
#ifndef ENVLIST_H
#define ENVLIST_H
+#include "qemu/queue.h"
+
+struct envlist_entry {
+const char *ev_var;/* actual env value */
+QLIST_ENTRY(envlist_entry) ev_link;
+};
+
+struct envlist {
+QLIST_HEAD(, envlist_entry) el_entries; /* actual entries */
+size_t el_count;/* number of entries */
+};
+
Why are you exposing the structures?
+static void envlist_parse_set_unset_test(void)
+{
+envlist_t *testenvlist;
+const char *env = "TEST1=123,TEST2=456";
+
+testenvlist = envlist_create();
+g_assert(envlist_parse_set(testenvlist, env) == 0);
+g_assert(testenvlist->el_count == 2);
+g_assert(envlist_parse_unset(testenvlist, "TEST1,TEST2") == 0);
+g_assert(testenvlist->el_count == 0);
If it's just for the count, then add an envlist_length() function.
r~
Re: [RFC v4 03/11] linux-user: Implement envlist_appendenv and add tests for envlist
Yeqi Fu writes:
> Signed-off-by: Yeqi Fu
> ---
> include/qemu/envlist.h| 13 ++
> tests/unit/meson.build| 1 +
> tests/unit/test-envlist.c | 94 +++
> util/envlist.c| 71 -
> 4 files changed, 169 insertions(+), 10 deletions(-)
> create mode 100644 tests/unit/test-envlist.c
Thanks for adding the unit test.
>
> +/*
> + * Appends environment value to envlist. If the environment
> + * variable already exists, the new value is appended to the
> + * existing one.
> + *
> + * Returns 0 in success, errno otherwise.
> + */
> +int
> +envlist_appendenv(envlist_t *envlist, const char *env, const char *separator)
> +{
> +struct envlist_entry *entry = NULL;
> +const char *eq_sign;
> +size_t envname_len;
> +
> +if ((envlist == NULL) || (env == NULL) || (separator == NULL)) {
> +return -EINVAL;
> +}
> +
> +/* find out first equals sign in given env */
> +eq_sign = strchr(env, '=');
> +if (eq_sign == NULL) {
> +return -EINVAL;
> +}
> +
> +if (strchr(eq_sign + 1, '=') != NULL) {
> +return -EINVAL;
> +}
> +
> +envname_len = eq_sign - env + 1;
> +
> +/*
> + * If there already exists variable with given name,
> + * we append the new value to the existing one.
> + */
> +for (entry = envlist->el_entries.lh_first; entry != NULL;
> +entry = entry->ev_link.le_next) {
> +if (strncmp(entry->ev_var, env, envname_len) == 0) {
> +break;
> +}
> +}
> +
> +if (entry != NULL) {
> +char *new_env_value = NULL;
> +size_t new_env_len = strlen(entry->ev_var) + strlen(eq_sign)
> ++ strlen(separator) + 1;
> +new_env_value = g_malloc(new_env_len);
> +strcpy(new_env_value, entry->ev_var);
> +strcat(new_env_value, separator);
> +strcat(new_env_value, eq_sign + 1);
> +g_free((char *)entry->ev_var);
> +entry->ev_var = new_env_value;
You could probably avoid messing about with strlens if you used the
handy glib functions:
if (entry != NULL) {
GString *new_val = g_string_new(entry->ev_var);
g_string_append(new_val, separator);
g_string_append(new_val, eq_sign + 1);
g_free(entry->ev_var);
entry->ev_var = g_string_free(new_val, false);
} else {
Also the fact you needed to cast entry->ev_var to g_free() should have
pointed out that now we can change env entries we need to drop the const
from the char *ev_var; definition in envlist_entry.
Otherwise with those fixes:
Reviewed-by: Alex Bennée
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [RFC v4 04/11] linux-user: Implement native-bypass option support
Yeqi Fu writes:
> This commit implements the -native-bypass support in linux-user. The
> native_calls_enabled() function can be true only when the
> '-native-bypass' option is given.
>
> Signed-off-by: Yeqi Fu
> ---
> include/native/native.h | 9 +
> linux-user/main.c | 38 ++
> 2 files changed, 47 insertions(+)
> create mode 100644 include/native/native.h
>
> diff --git a/include/native/native.h b/include/native/native.h
> new file mode 100644
> index 00..62951fafb1
> --- /dev/null
> +++ b/include/native/native.h
> @@ -0,0 +1,9 @@
> +/*
> + * Check if the native bypass feature is enabled.
> + */
> +#if defined(CONFIG_USER_ONLY) && defined(CONFIG_NATIVE_CALL)
> +extern char *native_lib_path;
> +#define native_bypass_enabled() native_lib_path ? true : false
> +#else
> +#define native_bypass_enabled() false
> +#endif
> diff --git a/linux-user/main.c b/linux-user/main.c
> index dba67ffa36..86ea0191f7 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -60,6 +60,11 @@
> #include "semihosting/semihost.h"
> #endif
>
> +#if defined(CONFIG_NATIVE_CALL)
> +#include "native/native.h"
> +char *native_lib_path;
> +#endif
> +
> #ifndef AT_FLAGS_PRESERVE_ARGV0
> #define AT_FLAGS_PRESERVE_ARGV0_BIT 0
> #define AT_FLAGS_PRESERVE_ARGV0 (1 << AT_FLAGS_PRESERVE_ARGV0_BIT)
> @@ -293,6 +298,17 @@ static void handle_arg_set_env(const char *arg)
> free(r);
> }
>
> +#if defined(CONFIG_NATIVE_CALL)
> +static void handle_arg_native_bypass(const char *arg)
> +{
> +if (access(arg, F_OK) != 0) {
> +fprintf(stderr, "native library %s does not exist\n", arg);
> +exit(EXIT_FAILURE);
> +}
> +native_lib_path = strdup(arg);
Although we never free this the coding style states:
Because of the memory management rules, you must use g_strdup/g_strndup
instead of plain strdup/strndup.
We do still have a few legacy strdup's to eliminate from the code base
though.
> +}
> +#endif
> +
> static void handle_arg_unset_env(const char *arg)
> {
> char *r, *p, *token;
> @@ -522,6 +538,10 @@ static const struct qemu_argument arg_table[] = {
> "", "Generate a /tmp/perf-${pid}.map file for perf"},
> {"jitdump","QEMU_JITDUMP", false, handle_arg_jitdump,
> "", "Generate a jit-${pid}.dump file for perf"},
> +#if defined(CONFIG_NATIVE_CALL)
> +{"native-bypass", "QEMU_NATIVE_BYPASS", true, handle_arg_native_bypass,
> + "", "native bypass for library calls in user mode only."},
> +#endif
You can drop " in user mode only" because this help text will only show
up on linux-user binaries with support for native bypass.
Otherwise:
Reviewed-by: Alex Bennée
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: Fix interval_tree_iter_first() to check root node value
On 8/9/23 17:23, Richard Henderson wrote:
On 8/9/23 08:11, Helge Deller wrote:
Fix a crash in qemu-user when running
cat /proc/self/maps
in a chroot, where /proc isn't mounted.
The problem was introduced by commit 3ce3dd8ca965 ("util/selfmap:
Rewrite using qemu/interval-tree.h") where in open_self_maps_1() the
function read_self_maps() is called and which returns NULL if it can't
read the hosts /proc/self/maps file. Afterwards that NULL is fed into
interval_tree_iter_first() which doesn't check if the root node is NULL.
Fix it by adding a check if root is NULL and return NULL in that case.
Signed-off-by: Helge Deller
Fixes: 3ce3dd8ca965 ("util/selfmap: Rewrite using qemu/interval-tree.h")
diff --git a/util/interval-tree.c b/util/interval-tree.c
index f2866aa7d3..53465182e6 100644
--- a/util/interval-tree.c
+++ b/util/interval-tree.c
@@ -797,7 +797,7 @@ IntervalTreeNode *interval_tree_iter_first(IntervalTreeRoot
*root,
{
IntervalTreeNode *node, *leftmost;
- if (!root->rb_root.rb_node) {
+ if (!root || !root->rb_root.rb_node) {
I guess this is good enough for 8.1. Before the conversion to interval-tree we
would also emit nothing.
Yes and yes.
I've already done a rewrite for 8.2, and I noticed this problem.
There I emit what mapping information that I have, which is
everything except for the device+path data.
nice.
Reviewed-by: Richard Henderson
Shall I send a pull request?
If so, is it OK that I include this patch in the pull-request as well?
linux-user: Fix openat() emulation to correctly detect accesses to /proc
https://lists.nongnu.org/archive/html/qemu-devel/2023-08/msg00165.html
which already has been R-b: Daniel P. Berrangé
Helge
Re: [RFC v4 03/11] linux-user: Implement envlist_appendenv and add tests for envlist
On 8/8/23 07:17, Yeqi Fu wrote: +char *new_env_value = NULL; +size_t new_env_len = strlen(entry->ev_var) + strlen(eq_sign) ++ strlen(separator) + 1; +new_env_value = g_malloc(new_env_len); +strcpy(new_env_value, entry->ev_var); +strcat(new_env_value, separator); +strcat(new_env_value, eq_sign + 1); This is new_env_value = g_strconcat(entry->ev_var, separator, eq_sign + 1, NULL); r~
Re: [RFC v4 02/11] build: Implement libnative library and the build machinery for libnative
On 8/8/23 07:17, Yeqi Fu wrote: +#if defined(i386) || defined(x86_64) +/* + * An unused instruction is utilized to mark a native call. + */ +#define __CALL_EXPR ".byte 0x0f, 0xff;" +#endif This is 2 of the 3 (or more) bytes of the UD0 instruction. At minimum you should include a third byte for the modrm. For example, 0F FF C0ud0 %eax, %eax If you want to encode more data, or simply magic numbers, you can use 0F FF 80 78 56 34 12 ud0 0x12345678(%eax), %eax or with modrm + sib bytes, 0F FF 84 00 78 56 34 12 ud0 0x12345678(%eax, %eax, 0), %eax So you have up to 32 (displacement) + 3 * 3 (registers) + 2 (shift) = 43 bits that you can vary while staying within the encoding of UD0. You can even have the assembler help encode a displacement to associated data: .text 0: ud0 label-0b(%eax), %eax .rodata label: .byte some stuff r~
Re: [RFC v4 05/11] linux-user/elfload: Add support for parsing symbols of native libraries.
On 8/8/23 07:17, Yeqi Fu wrote: This commit addresses the need to parse symbols of native libraries. The base address of a shared library is determined by the dynamic linker. To simplify the process, we focus on the last three digits, which reside within the same page and remain unaffected by the base address. Signed-off-by: Yeqi Fu --- linux-user/elfload.c | 85 +--- 1 file changed, 80 insertions(+), 5 deletions(-) I'm not keen on this. I would much prefer the native library to be self-contained and not rely on symbols in this fashion. r~
Re: Fix interval_tree_iter_first() to check root node value
On 8/9/23 08:53, Helge Deller wrote:
On 8/9/23 17:23, Richard Henderson wrote:
On 8/9/23 08:11, Helge Deller wrote:
Fix a crash in qemu-user when running
cat /proc/self/maps
in a chroot, where /proc isn't mounted.
The problem was introduced by commit 3ce3dd8ca965 ("util/selfmap:
Rewrite using qemu/interval-tree.h") where in open_self_maps_1() the
function read_self_maps() is called and which returns NULL if it can't
read the hosts /proc/self/maps file. Afterwards that NULL is fed into
interval_tree_iter_first() which doesn't check if the root node is NULL.
Fix it by adding a check if root is NULL and return NULL in that case.
Signed-off-by: Helge Deller
Fixes: 3ce3dd8ca965 ("util/selfmap: Rewrite using qemu/interval-tree.h")
diff --git a/util/interval-tree.c b/util/interval-tree.c
index f2866aa7d3..53465182e6 100644
--- a/util/interval-tree.c
+++ b/util/interval-tree.c
@@ -797,7 +797,7 @@ IntervalTreeNode *interval_tree_iter_first(IntervalTreeRoot
*root,
{
IntervalTreeNode *node, *leftmost;
- if (!root->rb_root.rb_node) {
+ if (!root || !root->rb_root.rb_node) {
I guess this is good enough for 8.1. Before the conversion to interval-tree we would
also emit nothing.
Yes and yes.
I've already done a rewrite for 8.2, and I noticed this problem.
There I emit what mapping information that I have, which is
everything except for the device+path data.
nice.
Reviewed-by: Richard Henderson
Shall I send a pull request?
If so, is it OK that I include this patch in the pull-request as well?
linux-user: Fix openat() emulation to correctly detect accesses to /proc
https://lists.nongnu.org/archive/html/qemu-devel/2023-08/msg00165.html
which already has been R-b: Daniel P. Berrangé
I can pick them both up -- I have other linux-user patches to send.
r~
Re: [RFC v4 07/11] target/i386: Add support for native library calls
On 8/8/23 07:17, Yeqi Fu wrote:
This commit introduces support for native library calls on the
i386 target. When special instructions reserved for native calls
are encountered, the code now performs address translation and
generates the corresponding native call.
Signed-off-by: Yeqi Fu
---
configs/targets/i386-linux-user.mak | 1 +
configs/targets/x86_64-linux-user.mak | 1 +
target/i386/tcg/translate.c | 27 +++
3 files changed, 29 insertions(+)
diff --git a/configs/targets/i386-linux-user.mak
b/configs/targets/i386-linux-user.mak
index 5b2546a430..2d8bca8f93 100644
--- a/configs/targets/i386-linux-user.mak
+++ b/configs/targets/i386-linux-user.mak
@@ -2,3 +2,4 @@ TARGET_ARCH=i386
TARGET_SYSTBL_ABI=i386
TARGET_SYSTBL=syscall_32.tbl
TARGET_XML_FILES= gdb-xml/i386-32bit.xml
+CONFIG_NATIVE_CALL=y
diff --git a/configs/targets/x86_64-linux-user.mak
b/configs/targets/x86_64-linux-user.mak
index 9ceefbb615..a53b017454 100644
--- a/configs/targets/x86_64-linux-user.mak
+++ b/configs/targets/x86_64-linux-user.mak
@@ -3,3 +3,4 @@ TARGET_BASE_ARCH=i386
TARGET_SYSTBL_ABI=common,64
TARGET_SYSTBL=syscall_64.tbl
TARGET_XML_FILES= gdb-xml/i386-64bit.xml
+CONFIG_NATIVE_CALL=y
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 90c7b32f36..28bf4477fb 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -33,6 +33,7 @@
#include "helper-tcg.h"
#include "exec/log.h"
+#include "native/native.h"
#define HELPER_H "helper.h"
#include "exec/helper-info.c.inc"
@@ -6810,6 +6811,32 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
case 0x1d0 ... 0x1fe:
disas_insn_new(s, cpu, b);
break;
+case 0x1ff:
+if (native_bypass_enabled()) {
+TCGv ret = tcg_temp_new();
+TCGv arg1 = tcg_temp_new();
+TCGv arg2 = tcg_temp_new();
+TCGv arg3 = tcg_temp_new();
+const char *fun_name = lookup_symbol((s->base.pc_next) & 0xfff);
I'm not keen on this lookup_symbol interface.
I would much rather there be some data encoded in the native.so.
+uintptr_t ra = GETPC();
+uint32_t a1 = cpu_ldl_data_ra(env, env->regs[R_ESP] + 4, ra);
+uint32_t a2 = cpu_ldl_data_ra(env, env->regs[R_ESP] + 8, ra);
+uint32_t a3 = cpu_ldl_data_ra(env, env->regs[R_ESP] + 12, ra);
+tcg_gen_movi_tl(arg1, a1);
+tcg_gen_movi_tl(arg2, a2);
+tcg_gen_movi_tl(arg3, a3);
This is wrong. You are performing the stack load at translation time, but it must be done
at execution time. You need
tcg_gen_addi_tl(arg1, cpu_regs[R_ESP], 4); /* arg1 = esp + 4 */
gen_op_ld_v(s, MO_UL, arg1, arg1); /* arg1 = *arg1 */
etc.
r~
Re: [PATCH for-8.2] dockerfiles: bump tricore cross compiler container to Debian 11
On Wed, Aug 09, 2023 at 04:33:37PM +0200, Paolo Bonzini wrote: > On Wed, Aug 9, 2023 at 3:53 PM Bastian Koppelmann > wrote: > > > diff --git a/tests/docker/dockerfiles/debian-tricore-cross.docker > > > b/tests/docker/dockerfiles/debian-tricore-cross.docker > > > index 269bfa8d423..5bd1963fb55 100644 > > > --- a/tests/docker/dockerfiles/debian-tricore-cross.docker > > > +++ b/tests/docker/dockerfiles/debian-tricore-cross.docker > > > @@ -9,7 +9,7 @@ > > > # > > > # SPDX-License-Identifier: GPL-2.0-or-later > > > # > > > -FROM docker.io/library/debian:buster-slim > > > +FROM docker.io/library/debian:11-slim > > > > I think it would be better to skip debian-11 and use 12 instead. > > I picked 11 in order to keep the container in sync with the > lcitool-generated dockerfiles. Once we switch lcitool from 11 to > 12, it is easier to see what the changes are and replicate them in > debian-tricore-cross and friends. > > lcitool support for Debian 12 has already landed, but usually we > try to keep the containers on the oldest supported release of a > distro, and that is currently Debian 11. Thanks for the explanation. Acked-by: Bastian Koppelmann Cheers, Bastian
Re: [RFC v1 0/3] Initial support for SPDM
On Wed, Aug 9, 2023 at 8:11 AM Jonathan Cameron wrote: > > On Tue, 8 Aug 2023 11:51:21 -0400 > Alistair Francis wrote: > > > The Security Protocol and Data Model (SPDM) Specification defines > > messages, data objects, and sequences for performing message exchanges > > over a variety of transport and physical media. > > - > > https://www.dmtf.org/sites/default/files/standards/documents/DSP0274_1.3.0.pdf > > > > This series is a very initial start on adding SPDM support to QEMU. > > > > This series uses libspdm [1] which is a reference implementation of > > SPDM. > > > > The series starts by adding support for building and linking with > > libspdm. It then progresses to handling SPDM requests in the NVMe device > > over the PCIe DOE protocol. > > > > This is a very early attempt. The code quality is not super high, the C > > code hasn't been tested at all. This is really just an RFC to see if > > QEMU will accept linking with libspdm. > > > > So, the main question is, how do people feel about linking with libspdm > > to support SPDM? > > > > 1: https://github.com/DMTF/libspdm > > Hi Alastair, > > For reference / background we've had SPDM support for CXL emulated devices > in our staging tree for quite a while - it's not upstream because of > exactly this question (+ no one had upstreaming this as a high priority > as out of tree was fine for developing the kernel stack - it's well > isolated in the code so there was little cost in rebasing this feature) > - and because libspdm is packaged by almost no one. There were problems > building it with external crypto libraries etc. Thanks for pointing that out! I didn't realise there was existing QEMU work. I'm glad others are looking into it Building with libspdm is difficult. Even this series does have weird issues with openssl's crypto library. I have been working towards packaging libspdm into buildroot, which has helped cleanup libspdm to be more user friendly. libspdm 3.0 for example is now a lot easier to build/package, but I expect to continue to improve things. There will need to be more improvements to libspdm before this series could be merged. > > Looks like you have had a lot more success than I ever did in getting that > to work. I tried a few times in the past and ended up sticking with > the Avery design folks approach of a socket connection to spdm-emu > Given you cc'd them I'm guessing you came across that work which is what > we used for testing the kernel code Lukas is working on currently. > > https://lore.kernel.org/qemu-devel/20210629132520.0...@huawei.com/ > > https://gitlab.com/jic23/qemu/-/commit/9864fb29979e55c1e37c20edf00907d9524036e8 Thanks for that! In the past the QEMU community has refused to accept upstream changes that expose QEMU internals via sockets. So I suspect linking with libspdm will be a more upstreamable approach. On top of that requiring user to run an external socket application is clunky. > > So I think we have 2 choices here. > 1) Do what you have done and build the library as you are doing. > - Can fix a version - so don't care if they change the ABI etc other >than needing to move the version QEMU uses forwards when we need >to for new features. I agree > - Cert management is going to add a lot of complexity into QEMU. >I've not looked at how much infrastructure we can reuse from elsewhere. >Maybe this is a solved problem. Could we not just point to a cert when running QEMU? > > 2) Keep the SPDM emulation external. > - I'm not sure the socket protocol used by spdm-emu is fixed in stone >as people tend to change both ends. > - Cert management and protocol options etc are already available. I suspect this will never get upstream though. My aim is to get something upstream, so this is probably a no go (unless someone jumps in and says this is ok). > > Personally I prefer the control option 1 gives us, even if it's a lot more > code. Option 2 also rather limits our ability to do anything with > the encrypted data as well. It's fine if the aim is just verification > of simple flows, but if we need to inject particular measurements etc > it doesn't really work. I like option 1 as well :) I don't envisage QEMU supporting extremely complex flows. I was more aiming for a certificate and some measurement data and maybe a manifest. My hope was basic SPDM support, not a complete test suite. Alistair > > Jonathan > > > > > > > Alistair Francis (3): > > subprojects: libspdm: Initial support > > hw: nvme: ctrl: Initial support for DOE > > hw: nvme: ctrl: Process SPDM requests > > > > meson.build | 78 +++ > > hw/nvme/nvme.h| 4 ++ > > include/hw/pci/pcie_doe.h | 1 + > > hw/nvme/ctrl.c| 35 > > .gitmodules | 3 ++ > > meson_options.txt | 3 ++ > > scripts/meson-buildoptions.sh | 3 ++ > > subprojects/.gitignore| 1 + > > subpro
